Skip to content

feat(staged): pass agent preference to autoreviews and validate on adoption#639

Open
matt2e wants to merge 6 commits intomainfrom
agent-for-autoreviews
Open

feat(staged): pass agent preference to autoreviews and validate on adoption#639
matt2e wants to merge 6 commits intomainfrom
agent-for-autoreviews

Conversation

@matt2e
Copy link
Copy Markdown
Contributor

@matt2e matt2e commented Apr 16, 2026

Summary

  • Pass the completing session's provider to auto-reviews so they run on the same agent the user chose for the commit
  • Validate that an auto-review's agent matches the user's current preferred agent before adopting it; skip adoption if they differ so a fresh review starts with the correct agent
  • Replace the positional (session_id, status, completion_reason) tuple with a ResolvedSession struct and include the provider field
  • Remove per-project agent preference scoping (setProjectAiAgent, getPreferredAgentForProject) in favor of the single global preference

Test plan

  • Verify auto-reviews use the same agent provider as the triggering commit session
  • Verify switching preferred agent causes stale auto-reviews to be skipped rather than adopted
  • Verify the global agent selector still works correctly in project sections
  • Verify timeline items correctly display session provider information for reviews

🤖 Generated with Claude Code

matt2e and others added 4 commits April 15, 2026 13:49
…option

Autoreviews now use the committing session's agent provider instead of
always defaulting to None, and tryAdoptAutoReview checks that the
autoreview's agent matches the user's current preference before adopting.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Project-level agent tracking was broken, so simplify to use only the
global last-used agent preference everywhere. Removes setProjectAiAgent,
getPreferredAgentForProject, projectAgents state, and the projectId prop
from AgentSelector.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…inate double-fetch

Expand resolve_session_status() to return the session provider as a
fourth tuple element, removing the redundant get_session() call in the
reviews timeline builder that previously fetched the session twice.

Also simplify the agent comparison in tryAdoptAutoReview by removing
the redundant `?? null` coercion — both sides are already
`string | null`, so strict equality handles the null match correctly.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…truct

Address code review feedback from fbb77e6:

- Replace the unwieldy 4-tuple (Option<String>, Option<String>,
  Option<String>, Option<String>) returned by resolve_session_status()
  with a self-documenting ResolvedSession struct, making each field's
  purpose clear at all 6 call sites in timeline.rs.

- Eliminate the extra getSession IPC round-trip in tryAdoptAutoReview()
  by looking up the review's sessionProvider from the already-fetched
  timeline data instead of making a redundant backend call.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@matt2e matt2e requested review from baxen and wesbillman as code owners April 16, 2026 03:44
Copy link
Copy Markdown

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

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: cf7096cb17

ℹ️ 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 +197 to +199
const timelineReview = this.getTimeline()?.reviews.find((r) => r.id === review.id);
const reviewProvider = timelineReview?.sessionProvider ?? null;
if (preferredAgent !== reviewProvider) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Resolve auto-review provider from source of truth

This provider check reads from this.getTimeline()?.reviews instead of the freshly fetched review/session, but BranchCard intentionally does not call loadTimeline() for isAutoReview running events, so the timeline is often stale when adoption is attempted. In that common path, timelineReview is missing, reviewProvider becomes null, and adoption is incorrectly skipped even when the auto-review was generated with the preferred agent, which then triggers an unnecessary new review run.

Useful? React with 👍 / 👎.

matt2e and others added 2 commits April 16, 2026 14:11
…f stale timeline

The timeline is intentionally not refreshed during auto-review lifecycle
events, so the previous timeline-based provider lookup always resolved to
null—causing adoption to be skipped whenever the user had an agent
selected. Fix by fetching the session's provider in the backend
find_fresh_auto_review query and returning it on the Review payload,
removing the stale-timeline dependency entirely.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ssion lookup

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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