feat(projects): expose code reviews as a repo session outcome - #917
Conversation
There was a problem hiding this comment.
💡 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".
| if let Some(ref provider) = session_provider { | ||
| session = session.with_provider(provider); | ||
| } | ||
| if let Some(selection) = self.acp_config_selection.clone() { |
There was a problem hiding this comment.
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 👍 / 👎.
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>
7d0627f to
f4e8a97
Compare
Project note sessions could only start repo-level sessions with the
note_in_repoorcommitoutcomes, so an agent orchestrating work across a project had no way to ask for a code review. This addscode_reviewto thestart_repo_sessionMCP tool.What's in it
code_reviewoutcome. Enqueues a branch review session and creates a review stub (itscommit_shais filled in at drain time, like queued user review sessions). The completed payload reports the review's confidence title and inline comments.code_review(a requested review would duplicate it) andcommit(a commit invalidates it and triggers a fresh one once it lands) — agent-triggered commits create their session row directly and previously skipped this.resolve_inherited_review_providerkeeps 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.drain_queued_sessions_for_branchis the inherited provider, so a review's fallback agent can't pull unrelated queued sessions off the project session's provider.start_line/end_lineimply, matching what review comments already get when posted to GitHub. Empty spans collapse onto the line they anchor to.instructionsfield, and the project session instruction text all cover the new outcome. For a standard review,instructionsshould 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