Skip to content

fix: send review to terminal respects repo default agent setting - #17

Merged
Ziinc merged 7 commits into
mainfrom
claude/review-terminal-agent-default-qth3jc
Jul 30, 2026
Merged

fix: send review to terminal respects repo default agent setting#17
Ziinc merged 7 commits into
mainfrom
claude/review-terminal-agent-default-qth3jc

Conversation

@Ziinc

@Ziinc Ziinc commented Jul 6, 2026

Copy link
Copy Markdown
Collaborator

handleCreateAgentWithReview in ShowWorkspace was calling onSessionCreated
without an agent field, so the session always launched with the claude
agent regardless of the repo or app default_agent setting.
Now it reads the repo-level default_agent first (falling back to the
app-level default), and passes the resolved agent to onSessionCreated,
matching the same resolution logic used when creating sessions from the
task input.
Also adds getRepoSetting to the api mock in ShowWorkspace test files to
avoid falling through to the real implementation when the review handler
is invoked in tests.

@Ziinc
Ziinc force-pushed the claude/review-terminal-agent-default-qth3jc branch from 9044c6e to e444558 Compare July 10, 2026 03:02
@Ziinc
Ziinc force-pushed the claude/review-terminal-agent-default-qth3jc branch from e444558 to 344047b Compare July 26, 2026 10:51
@github-actions

github-actions Bot commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

📸 App QA screenshots

Re-ran the flow whose spec this PR adds or modifies — 2 captures. Other specs in the library were not run.

⬇️ Download the screenshots

send-review-to-terminal-default-agent
  • send-review-codex-01-popover.png
    • The "Finish review" popover is open showing the "Finish your review" heading.
    • Three action buttons are visible: "Copy" on the left, "Plan" and "Edit" on the right.
    • The diff pane for example.ts is visible in the background.
  • send-review-codex-02-terminal.png
    • The terminal pane at the bottom is open.
    • A session tab labelled "Code Review" is visible in the terminal pane.
    • The "Code Review" tab has a Sparkles (✨) icon — the codex agent icon — not a Bot icon (which would indicate the claude agent was launched instead).

Each bullet under a capture is what the spec claims that image should show — open the PNG and check it.


commit 6b40227 · workflow run — this comment is updated in place on each run.

@Ziinc
Ziinc force-pushed the claude/review-terminal-agent-default-qth3jc branch from 3cb4500 to 1461ca8 Compare July 27, 2026 19:55
claude added 7 commits July 29, 2026 12:26
handleCreateAgentWithReview in ShowWorkspace was calling onSessionCreated
without an agent field, so the session always launched with the claude
agent regardless of the repo or app default_agent setting.

Now it reads the repo-level default_agent first (falling back to the
app-level default), and passes the resolved agent to onSessionCreated,
matching the same resolution logic used when creating sessions from the
task input.

Also adds getRepoSetting to the api mock in ShowWorkspace test files to
avoid falling through to the real implementation when the review handler
is invoked in tests.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EA8qrRVMGszi1yK9gnjXKW
Adds a screenshot spec verifying that clicking "Plan" in the Finish Review
popover opens a Codex terminal (Sparkles icon) when the repo default_agent
is set to "codex", instead of always opening a Claude terminal (Bot icon).

Drives the full UI flow: workspace Review tab → add inline comment →
Finish Review popover → Plan → terminal pane with codex session.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EA8qrRVMGszi1yK9gnjXKW
…waitFor callbacks

- Each test was calling findByTestId("changes-viewer") without first
  clicking the Review tab, so ChangesDiffViewer was never mounted in CI
- Collapsed multi-line waitFor arrow callbacks to satisfy biome format check
… drop completes synchronously

tracing_appender's WorkerGuard only waits a bounded amount of time for the
non-blocking writer's background thread to flush on drop. Under CI load that
window can be missed even though records were queued, causing an intermittent
"expected one JSON line per forwarded record" failure. Poll the file for up
to 1s instead of reading it once immediately after the guards drop.
…og line count

The OTel SDK emits its own self-diagnostic record ("Last reference of
LoggerProvider dropped, initiating shutdown.") through the same file
pipeline when SdkLoggerProvider is dropped. Asserting exactly 5 raw lines
made the test fail whenever that diagnostic showed up. Filter to lines
whose body matches our own forwarded records before counting/asserting.
@Ziinc
Ziinc force-pushed the claude/review-terminal-agent-default-qth3jc branch from 6b40227 to 92bca64 Compare July 29, 2026 12:26
@Ziinc
Ziinc merged commit f4679c4 into main Jul 30, 2026
7 checks passed
@Ziinc
Ziinc deleted the claude/review-terminal-agent-default-qth3jc branch July 30, 2026 20:00
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.

2 participants