Add AI-assisted triage - #3
Conversation
Also make the agent perma prompt part of the package
- make it part of the markdown when that is used - save to separate file if not used - print to stdout for ai-triage command
grayson-wolf
left a comment
There was a problem hiding this comment.
looks good, to me, just a few suggestions / nit-questions
TheJJ
left a comment
There was a problem hiding this comment.
coming together pretty nicely. the overall architecture is good, i'd say.
one thing that worries me is the implicit assumption this is run in a secure container now:
how about a text-only analysis without funny containers as the first agent, and when it decides we wanna run shell stuff, it spins up a subagent in an lxd container (using the same mechanism we use for git ubuntu deltarebase resolving - we can hopefully create a shared ai lxd containering library so we don't have to duplicate/reinvent stuff)
otherwise many nits and naming issues, you'll see. great work, overall :)
Accept only a bare/#-prefixed integer or a genuine Launchpad bug URL (.../+bug/<n> or /bugs/<n>) and raise ValueError otherwise.
Plain 'analyze <bug>' now prints the bug metadata (status, tags, affected targets, description, comments) with no provider or credentials required; 'analyze --ai <bug>' runs the agent as the old 'ai-triage' command did.
triage --ai no longer writes a separate autotriage-YYYY-MM-DD.md file. Without --markdown the AI section is printed to stdout after the normal triage output; with --markdown it is folded into that same file behind the review notice, giving one cohesive report. Drops the now-unused report_filename/write_report/resolve_report_dir helpers and their tests, and updates the prompt and README accordingly.
|
@TheJJ I addressed most of your review points, responded to a couple inline. I see your point with the two-step agent run, but I would ask you if we could not do this in a future iteration. You mentioned there may be some overlap in the "Run this container and do stuff" tasks - let's go for the shared LXD library then. But the size and scope of this PR make me thing later is better. For now we can tell users that this is potentially unsafe, and ask them to please containerize (or isolate the environment right) before running AI commands. WDYT? |
TheJJ
left a comment
There was a problem hiding this comment.
just minor things left, i think for a first version we are nearly ready.
this runs the ai analysis within the snap confinement, but as future work we should add properly containerized ai execution - maybe open an issue for that then :)
| def _build_ai_provider(config: StarTriageConfig) -> Provider | None: | ||
| """Build the AI provider, printing a friendly hint and returning None on misconfig.""" | ||
| from .ai import build_provider | ||
|
|
||
| try: | ||
| return build_provider(config.ai) | ||
| except AIConfigError as exc: | ||
| print(f"error: {exc}", file=sys.stderr) | ||
| return None |
There was a problem hiding this comment.
shouldn't be part of cli.py, rather ai/provider.py. return value should always be Provider, and if that fails, let the AIConfiError be raised to top for crash.
| def _emit_ai_report(report: str, markdown_path: Path | None) -> None: | ||
| """Emit an AI ``report`` for a ``triage --ai`` run. | ||
|
|
||
| With ``--markdown`` the AI section is appended (behind a review notice) to | ||
| that same file, so the human triage report and the AI aid live in one | ||
| cohesive document. Without ``--markdown`` the report is printed to stdout, | ||
| together with the normal triage output that already went there. | ||
| """ | ||
| from .ai import append_report | ||
| from .ai.render import AI_APPEND_NOTICE | ||
|
|
||
| if markdown_path is not None: | ||
| append_report(markdown_path, report) | ||
| print(f"AI triage appended to {markdown_path}") | ||
| else: | ||
| print("\n" + AI_APPEND_NOTICE + report) |
| return self.github_token or _first_env(COPILOT_TOKEN_ENV_VARS) | ||
| return self.openrouter_api_key or _first_env(OPENROUTER_KEY_ENV_VARS) | ||
|
|
||
| def require_configured(self) -> None: |
There was a problem hiding this comment.
@model_validator(mode='after')
def check_token(self) -> Self:
if self.resolve_token() is not None:
return self
match self.provider:
case AIProvider.copilot:
...
raise ValueError('bla')
case _:
raise RuntimeError("unhandled provider")Move the missing-credential check from AIConfig.require_configured() onto a model validator that only enforces when validated with a require_ai context. This keeps the [ai] section optional for non-AI commands while build_provider re-validates with that context so a misconfigured provider fails before a session starts. AIConfigError is not a ValueError, so pydantic propagates it unwrapped to the top.
|
@TheJJ issue open Claude is worried that the model validator will break users who have no [ai] configured when trying to run non-ai commands, so it proposed something in-the-middle. |
TheJJ
left a comment
There was a problem hiding this comment.
great, now the only missing piece is the discussed --yolo flag to grant/deny execution rights to the agent.
to implement it, we could turn --ai into having a parameter.
--ai=deny (Default)
--ai=always
--ai=ask
and this flag could then be in a parent parser, as mentioned below.
--ai now takes a required level (restricted/full/ask) instead of being a plain on/off switch, and lives on a parent parser shared by triage and analyze. The level is mandatory so --ai never swallows a following bug number as its value, keeping 'analyze --ai 123' unambiguous (it rejects 123 as an invalid level). A new AIPermission enum carries the level through to build_provider.
build_provider takes the permission level and threads it to CopilotProvider, which selects the session's on_permission_request handler: full auto-approves every tool, restricted rejects them all (and appends a text-only note to the prompt), ask prompts on the terminal per call.
|
Claude iteration done Then fire the cannons and here we are |
TheJJ
left a comment
There was a problem hiding this comment.
great, for a first version we can merge and test this for further improvements.
This adds the whole AI-assisted functionality to startriage. It's full of abstractions and details but it works well.
--aifor usual triage second opinionai-triagefor single bugs