Skip to content

feat(projects): expose code reviews as a repo session outcome - #917

Merged
matt2e merged 5 commits into
mainfrom
allow-agent-triggered-reviews
Aug 13, 2026
Merged

feat(projects): expose code reviews as a repo session outcome#917
matt2e merged 5 commits into
mainfrom
allow-agent-triggered-reviews

Conversation

@matt2e

@matt2e matt2e commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Project note sessions could only start repo-level sessions with the note_in_repo or commit outcomes, so an agent orchestrating work across a project had no way to ask for a code review. This adds code_review to the start_repo_session MCP tool.

What's in it

  • New code_review outcome. Enqueues a branch review session and creates a review stub (its commit_sha is filled in at drain time, like queued user review sessions). The completed payload reports the review's confidence title and inline comments.
  • Cancel the in-flight auto review first, matching user-initiated sessions. Applied to both code_review (a requested review would duplicate it) and commit (a commit invalidates it and triggers a fresh one once it lands) — agent-triggered commits create their session row directly and previously skipped this.
  • Resilient provider resolution. A review's provider is inherited from the parent project session, not chosen, and the calling agent has no parameter to override it. resolve_inherited_review_provider keeps the inherited provider when it can review and otherwise falls back to the preferred review-capable provider, still erroring when no provider is available at all. Resolution happens up front so a queued review can't strand the branch drain, and runs on a blocking thread because local resolution probes every known agent through a login shell.
  • The review-resolved provider stays scoped to the review session. The branch-wide default passed to drain_queued_sessions_for_branch is the inherited provider, so a review's fallback agent can't pull unrelated queued sessions off the project session's provider.
  • 1-indexed inclusive comment lines. Comment spans are stored 0-indexed with an exclusive end; the payload now converts them to what start_line/end_line imply, matching what review comments already get when posted to GitHub. Empty spans collapse onto the line they anchor to.
  • Docs. The tool description, the instructions field, and the project session instruction text all cover the new outcome. For a standard review, instructions should be left empty — provide them only to flag a specific concern or focus area.

Testing

Unit tests cover the line-range conversion, the review handle round-trip, inherited-provider fallback (falls back, keeps a capable provider, still errors with none available), and the tool/schema/prompt text listing all three outcomes.

🤖 Generated with Claude Code

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 21ccc8aff0

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment on lines 585 to 588
if let Some(ref provider) = session_provider {
session = session.with_provider(provider);
}
if let Some(selection) = self.acp_config_selection.clone() {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Drop stale config when falling back providers

When expected_outcome is code_review and resolve_inherited_review_provider replaces the inherited provider with a different review-capable one (for example, a remote repo review from a Codex project session falling back to Claude/Goose), this stores the fallback provider but the next block still copies the parent session's ACP model/effort selection. Those selections carry provider-specific config_id/value_ids and are later forwarded unchanged to SessionConfig, so the fallback review can fail with an unavailable config before producing a review. Clear or re-resolve the ACP selection when session_provider differs from self.provider.

Useful? React with 👍 / 👎.

matt2e and others added 5 commits August 14, 2026 09:52
Project note sessions could previously only start repo-level sessions
with note_in_repo or commit outcomes. Add a code_review outcome to the
start_repo_session MCP tool that enqueues a branch code review session:
the review provider is resolved and validated up front so a queued
review can't strand the branch queue, any in-flight auto review is
cancelled first (matching user-initiated review sessions), and the
completed payload includes the review's confidence title and inline
comments. The tool description and the project session instruction
text now explain the new outcome alongside the existing two.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Matt Toohey <contact@matttoohey.com>
…d provider

Review sessions started via start_repo_session resolved their provider
with the same explicit-provider semantics as user-chosen reviews, so a
project session running on an agent that can't review — notably any
provider outside the remote workstation set on a remote branch — failed
the call with no provider parameter the agent could use to work around
it. Add resolve_inherited_review_provider, which keeps the inherited
provider when it can review and otherwise falls back to the preferred
review-capable provider, still erroring when no provider is available at
all. Resolution stays eager so a queued review can't strand the branch
queue.

Also convert the review comment spans in the repo session payload to the
1-indexed inclusive lines start_line/end_line imply. They were passed
through as stored (0-indexed, exclusive end), inviting off-by-one errors
when the parent agent relays comment locations; the conversion matches
what review comments already get when posted to GitHub, and empty spans
collapse onto the line they anchor to.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Signed-off-by: Matt Toohey <contact@matttoohey.com>
Resolve the code review comments on the agent-triggered review plumbing.

The review-resolved provider was passed to drain_queued_sessions_for_branch
as the branch-wide default, which the drain applies to every queued session
without a stored provider — an unrelated queued note or commit would run on
the review's fallback agent instead of the project session's. Pass the
inherited provider instead; the review session carries its resolved provider
on its own row.

A commit invalidates the auto review being generated for the same branch,
and the user-facing path cancels it for commit sessions as well as reviews.
Agent-triggered commits create their session row directly, so they skipped
that; widen the cancellation to both outcomes.

Document what `instructions` means for a code_review outcome — the review
prompt is supplied by the app and these instructions are appended to it, so
they should carry focus areas rather than a restated request or an output
format that fights the parser.

Finally, run review provider resolution on a blocking thread. Local
resolution probes every known agent through a login shell, and the MCP
handler that calls it promises to return immediately; the other
discover_providers callers already wrap it the same way.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Signed-off-by: Matt Toohey <contact@matttoohey.com>
The instructions doc for the code_review outcome explained how the
built-in review prompt is assembled (diff strategy, comment taxonomy,
output format) — detail the parent agent has no use for and shouldn't
be steering. Replace it: leave instructions empty for a standard
review, and provide them only when something specific should be looked
into or is a concern. Rename the schema test to match.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Matt Toohey <contact@matttoohey.com>
… falls back

An agent-triggered review that falls back to a different review-capable
provider still had the parent project session's ACP config selection
stamped onto its row. The selection names config and value IDs specific
to the inherited provider, so the fallback agent fails config
application at session start — and since a retried start_repo_session
re-applies the handler's selection to a fresh row, the failure repeats
on every retry instead of self-healing.

Inherit the selection only when the repo session runs on the parent's
own provider. Non-review outcomes pass the inherited provider through
unchanged, so their behavior is untouched; a review that resolves with
no inherited provider to compare against also drops the selection,
since there's no way to tell which agent it was made for.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Matt Toohey <contact@matttoohey.com>
@matt2e
matt2e force-pushed the allow-agent-triggered-reviews branch from 7d0627f to f4e8a97 Compare August 13, 2026 23:53
@matt2e
matt2e merged commit de68eb3 into main Aug 13, 2026
2 checks passed
@matt2e
matt2e deleted the allow-agent-triggered-reviews branch August 13, 2026 23:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant