[brennanb2025]

	Files	Added	Deleted	Net
Test	4	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​387	$\color{#cf222e}{\Huge{\mathbf{−}}}$​123	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​264
Prod	0	0	0	0

ELI5

Orca's Claude chats talk to the user's own claude program. CI checks that conversation only against a fake claude that replays recorded answers, so it can't tell when a real claude, or a new version of the library Orca uses to talk to it, behaves differently. The repo has a second set of tests that drive the real claude (opt-in with ORCA_REAL_CLAUDE_CLI_TEST=1; CI skips them). They had drifted. On a real claude 2.1.302, signed in with a throwaway Claude home, these failed for reasons that had nothing to do with Orca:

• Two relied on the machine's real Claude settings for a SessionStart hook, and one also for an effort level. They only passed on a developer machine with Orca's hooks installed.
• Three (supervised stop) launched Claude under one Claude home and looked for its transcript under another. With a throwaway home, Claude wrote into the developer's real ~/.claude, ran their real hooks, and then the test timed out looking in the wrong place. The stops themselves worked.
• One broke on current main: since A chat's model, effort and Fast belong to the chat, not to a model #30561, a send also reports the options it ran with, and the stop test compared the whole result exactly.

(Two more failed on stale model ids; main fixed those in #30561 and #30853 while this was being written.)

Several requests behind the chat pickers also had no real-claude test at all: model and effort per message, the model list, the context meter, and changing permission mode. CI meets them only through stand-ins.

This removes those fixture dependencies and adds one live test for the option requests behind those pickers. The isolated runner verified the suite on Claude 2.1.302 with the current SDK; it does not prove every account, CLI version or host configuration. This provides coverage for checking Claude SDK updates (the next one is tracked in #30814).

What changed for users

Nothing. Tests only; no app code changes.

What changed

• Tests bring their own settings. The tests that need a SessionStart hook create it in their own temporary project, using one small shared helper (a neighbouring test already did this inline). The effort test sets effortLevel: low there and checks for exactly low. The adopted-model test also checks that Orca marks the model as confirmed.
• The supervised-stop test launches and reads under one Claude home, the way the app starts a chat (realClaudeLaunchHome() plus the config-dir pin), so launch and lookup use the same selected Claude home. With the isolated runner, that is the throwaway home; without an explicit config directory, the existing opt-in contract still uses the default Claude home.
• The stop test accepts the options a send now reports, and checks only that the send was admitted.
• Merged with main. One conflict (the stop test) took main's identical fix. The commit hook also reformatted main's src/renderer/src/components/terminal-pane/pty-connection-split-spawn.spec.ts; that change is formatting only.
• New: claude-structured-real-cli-options.test.ts, one flow over two short turns:
  1. The live model list has haiku and sonnet rows with resolved ids, and Orca's model picker lists them.
  2. A message sent with a model and an effort goes through the real send path (model list, set model, apply effort, read settings). It reports running with exactly that effort; the model it reports is either absent or exactly the pick (since fix(chat): keep resumed Claude messages deliverable after settings reports #30982 the report only carries what the CLI said, and the CLI names the resolved id, so a short-name pick like haiku reports no model; the saved chat options still record haiku). Its turn reports the resolved model, no message carries an effort field, and Orca shows both as confirmed.
  3. The turn ends with a real context-usage report (tokens used, window, model).
  4. Permission mode changes the way Orca does it: the chat closes and resumes with the new mode and the saved picks. The resumed turn runs in acceptEdits on the same conversation and model, and the effort survives.

Why

The fixtures now supply the hook and effort they require, and transcript lookup follows the home selected for launch. One flow test covers the adapter option requests end to end in two short turns; it does not exercise the rendered picker or prove conversational recall after resume.

Plan

Keep the changes in tests: give hook-dependent tests their own temporary project settings, use the same selected Claude home for launch and transcript lookup, and exercise the chat option requests through one live two-turn flow. No app code, saved user settings or defaults change.

Review path: full

• Small: 0 production lines; 3 test files.
• Outside shipped risky areas: no app lifecycle, storage, protocol, process or default changes. The tests exercise existing behavior.
• Repeat criterion fails: A chat's model, effort and Fast belong to the chat, not to a model #30561 already updated admitted-result assertions in this test file, and Fix admission of later Claude question rows and real CLI verification #25119 fixed authentication environment handling in its fixtures. The review therefore uses the full path, including an independent architecture challenge.
• Local effect: test-only. QA marker qa=none; app QA and screenshots do not apply.

Linked Issue

N/A: groundwork for checking Claude SDK updates (the next one is tracked in #30814).

Visual Proof

N/A: tests only, no UI.

Testing

• Automated tests added/updated, or explained why not below
• I manually tested these changes locally

Real claude 2.1.302 through the team load balancer, with a throwaway Claude home (CLAUDE_CONFIG_DIR) and the calling session's own Claude environment variables cleared, on head 07576e1c779 in an independent review run admitted by qa-gate:

• 21 of 21 passed: 20 existing tests plus the new flow test. The reviewer checked the per-test rows and explicit Vitest exit=0.
• The real ~/.claude/settings.json was unchanged, and the runner found no transcript folder matching this isolated run in the real ~/.claude/projects. This verifies the configured Claude root, rather than a full filesystem sandbox.

Before this change, on the same machine: 11 of 19 passed, and the same 8 failed on SDK 0.3.284 and 0.3.302 alike.

AI Disclosure

Written with Claude.

Agent skill upstream boundary

• Not applicable, or this change copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.

Checklist

• This PR is small and focused
• I explained what changed and why (ELI5, the user-facing before/after, the mechanism, and why over the alternatives)
• Before/after screenshots or videos attached for UI changes, or N/A with reason
• Self-reviewed for correctness, security, and performance
• Cross-platform, SSH/remote, and path/shortcut impact considered (or N/A)
• pnpm lint, pnpm typecheck, pnpm test, and pnpm build pass (or CI will cover; local preferred)

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Repository UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 3d98f7de-d43d-44cf-8e96-a060f85721bd

📥 Commits

Reviewing files that changed from the base of the PR and between db53e47 and 8a6580a.

📒 Files selected for processing: 1

• src/main/claude/claude-structured-real-cli-options.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 4 remain after this review.

📝 Walkthrough

The changes add a real Claude CLI test for model and effort selection, context usage, and session resumption with a different permission mode. Existing structured CLI tests now use controlled project settings and stronger effort and model confirmation checks. The supervised-stop test uses shared Claude home and configuration-directory helpers. One PTY test is reformatted without behavior changes.

Priority: ⬇️ Low

Merge Risk: ⚪ Minimal · up to 8a658

This PR adds opt-in test coverage without an established user-facing or merge-blocking defect.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 11.11% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 9 functions across 4 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Title check		The title clearly identifies the main changes: isolating real Claude CLI test fixtures and adding coverage for chat option requests.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.
Description check		PASS. The description covers the required scope, motivation, changes, testing, visual proof, AI disclosure, review notes, compatibility considerations, and checklist. The Linked Issue section uses N/A…

Full details: Description check

Explanation

PASS. The description covers the required scope, motivation, changes, testing, visual proof, AI disclosure, review notes, compatibility considerations, and checklist. The Linked Issue section uses N/A instead of an actual issue link, and manual testing remains unchecked, but the author provides a clear rationale and detailed automated test results.

✨ Finishing Touches: 💡 1

📝 Generate docstrings: 💡

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[brennanb2025]

The opt-in tests against real Claude had drifted: some required the developer’s hooks and effort setting, and supervised-stop launched Claude under a different home from the one searched for its transcript. This PR gives those tests their own required settings, uses one selected Claude home for launch and transcript lookup, and adds a two-turn contract test for model, effort, context usage and permission-mode resume. App behavior is unchanged; all four changed files are tests, including a renderer test reflow during the main merge whose 4,250 tokens and syntax tree were verified identical.

The full review included an independent correctness reviewer, architecture challenger and dedicated resource scan, followed by a scoped review of the final main merge and test-assertion update. No proven code defects required fixes. The final-head delta review found no new defects. The final private reference table records each checked value’s source and whether it can gate a user action; this PR adds test assertions and no shipped action gate. The review corrected the PR’s initial light-path classification because recent commits already fixed the same test mechanisms, and narrowed claims about isolation and coverage.

Verified on 8a6580a60e50d4ef9ed5e50ac8317e834ff3a11e: the lane’s final-head run using Claude 2.1.302 and SDK 0.3.284 passed 21/21 tests, including all supervised-stop cases and the new options flow. The run used the authorized isolated runner under QA admission ticket 1011; the actual Vitest summary and exit=0 were checked. The real Claude settings hash was unchanged, and the runner found no transcript folder matching this run under the default home. The readiness checklist found no blocking issue. The final assertion accepts an omitted dispatch model for an alias pick, as current dispatch does, while still requiring exact effort, the real init’s resolved model and confirmed model/effort readback. The full PR Checks workflow (.github/workflows/pr.yml, run 39541664770) and its “PR checks passed” aggregate succeeded on this exact head.

Deferred limits: the runner isolates the selected Claude configuration root rather than the entire filesystem, and its transcript-leak search checks matching run folders. The external runner now propagates Vitest failure status; the raw results were also inspected. The resume test checks the same session id, saved picks and a successful reply; it does not prove conversational recall or exercise the rendered picker. A near-timeout slow provider can exceed the overall test timeout before its individual waits expire; no lingering process or introduced app resource regression was demonstrated. Windows/Linux/SSH live runs and other CLI/account combinations were not exercised. App QA and screenshots are skipped (qa=none) because no shipped code changed.

For Brennan’s review: no app screenshots apply. The three useful inspection points are the temporary hook settings, shared launch home, and resume contract.