Repository navigation
Keep Claude approval choices across model changes without freezing delivery - #34031
Conversation
…ithout freezing delivery A Claude chat saved as "Approve for me" could reopen asking for every approval, because Orca read the folder's saved model list as launch policy, and a model change could keep running in a weaker approval mode Claude opened with. The first fix for this was reverted after it froze the app: restored history left a turn reading as running, a send was refused against it, and delivery retried the same send with nothing changed. - The saved approval choice goes to Claude as-is; the menu offers every mode. - Orca records the mode Claude actually opened in. A send that picks a new model reopens Claude first when that mode differs from the one requested, only when the chat owes no work; Orca mail notices never reopen it. - A message sent while a Claude turn is running joins that turn on its current choices; new choices apply at the next new turn. Nothing is refused. - History catch-up closes every turn it opens, in the thread it opened in. - At startup, a turn reading as running under the new agent that the agent is not running is ended as interrupted. - Delivery goes round again only after a step shows what it changed (a commit, a different child, a start it joined ending, or a wake); otherwise it waits for the next change instead of repeating the same step. Tracked in STA-10953.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review.
 |
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Description check |  |
The description provides detailed scope, behavior, testing, review, and agent-boundary information. It does not follow the required template because ELI5, Why, Visual Proof, AI Disclosure, Notes, and … | Add the missing template sections. Include a plain-language ELI5 summary, an explicit Why section, Visual Proof with before/after attachments or N/A and a reason, AI Disclosure or the required internal-contributor handling, Notes, and the… |
-
Check name Status Explanation Title check The title clearly summarizes the primary change: preserving Claude approval choices across model changes while preventing delivery freezes. 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.
Full details: Description check
-
Explanation
The description provides detailed scope, behavior, testing, review, and agent-boundary information. It does not follow the required template because ELI5, Why, Visual Proof, AI Disclosure, Notes, and Checklist sections are missing. The Linked Issue requirement is also not met: it references STA-10953 but does not provide an issue link or a Fixes reference.
Resolution
Add the missing template sections. Include a plain-language ELI5 summary, an explicit Why section, Visual Proof with before/after attachments or
N/Aand a reason, AI Disclosure or the required internal-contributor handling, Notes, and the completed Checklist. Link the actual issue, preferably withFixes #<issue-number>or the repository’s accepted issue-link format.
Finishing Touches: 💡 2
-
📝 Generate docstrings: 💡
-
- Commit to this branch
- Create a new PR
🛠️ Fix failing CI checks: 💡
-
- 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.
Comment @coderabbitai help to get the list of available commands.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments: 1
src/main/native-chat/agent-session-wire/structured-agent-session-launch-permission.test.ts: 1
20-22:Trivial · Maintainability & Code Quality · Quick win Cover saved Claude
accept-editsat launch.The existing integration test already detects a saved
autovalue being replaced withaskon the next start. No test covers saved Claudeaccept-editsat this launch-permission boundary. A regression for that value can pass the current launch-permission test.Suggested test
it('delegates saved own settings', async () => { for (const provider of ['claude', 'codex']) { await expect(launchPermission(provider, { permissionMode: 'own' })).resolves.toBe('own') } }) + it('delegates saved Claude accept-edits settings', async () => { + await expect( + launchPermission('claude', { permissionMode: 'accept-edits' }) + ).resolves.toBe('accept-edits') + }) +
-
-
- Source mode: Auto
- Repository: Unavailable
- Branch: Unavailable
- File: No configuration file from this repository applied
- Commit: Unavailable
- Layers: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
b5d1d7d0-86b4-4c3c-a61d-aedb8e61c8bf
-
src/main/claude/claude-structured-execution-recording.test.tssrc/main/claude/claude-structured-message-options.test.tssrc/main/claude/claude-structured-message-options.tssrc/main/claude/claude-turn-opening.tssrc/main/native-chat/agent-session-wire/structured-agent-session-claude-opening-send-hold.test.tssrc/main/native-chat/agent-session-wire/structured-agent-session-delivery-loop-deps.tssrc/main/native-chat/agent-session-wire/structured-agent-session-delivery-loop.test.tssrc/main/native-chat/agent-session-wire/structured-agent-session-delivery-loop.tssrc/main/native-chat/agent-session-wire/structured-agent-session-delivery-progress.tssrc/main/native-chat/agent-session-wire/structured-agent-session-launch-permission.test.tssrc/main/native-chat/agent-session-wire/structured-agent-session-outgrown-child.test.tssrc/main/native-chat/agent-session-wire/structured-agent-session-ran-options.tssrc/main/runtime/claude-structured-retired-saved-model.test.tssrc/shared/structured-agent-session-permission-option.test.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 5 remain after this review.
-
There was a problem hiding this comment.
🧹 Nitpick comments: 1
tests/e2e/claude-auto-mode-host-catalog.unit.test.ts: 1
222-224:Trivial · Functional Correctness · Quick win Wait for a catalog-derived hook update before the final assertion.
The direct
mocks.rpc(undefined, 'agentSession.modelCatalog', { waitForListing: true })waits for the service response. It does not wait foruseHostModelCatalogUpgradeto apply that response and for React to commit the newoptionSnapshot. Since the final assertion already matches the pre-listing snapshot, it can pass without checking the after-listing state.Add a
waitForfor a catalog-derived hook change before asserting that the permission choices remain unchanged. This test is explicitly intended to cover the choices both before and after the background listing lands.
-
-
- Source mode: Auto
- Repository: Unavailable
- Branch: Unavailable
- File: No configuration file from this repository applied
- Commit: Unavailable
- Layers: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
56806e01-559f-4e05-b11d-bc079559a505
-
src/main/native-chat/agent-session-wire/structured-agent-session-adapter-router.tssrc/main/native-chat/agent-session-wire/structured-agent-session-adapter.tssrc/main/native-chat/agent-session-wire/structured-agent-session-delivery-loop-deps.tssrc/main/native-chat/agent-session-wire/structured-agent-session-delivery-loop.test.tssrc/main/native-chat/agent-session-wire/structured-agent-session-delivery-loop.tssrc/main/native-chat/agent-session-wire/structured-agent-session-host-delivery.tssrc/main/native-chat/agent-session-wire/structured-agent-session-launch-permission.tssrc/main/native-chat/agent-session-wire/structured-agent-session-outgrown-child.test.tssrc/main/native-chat/agent-session-wire/structured-agent-session-outgrown-child.tstests/e2e/claude-auto-mode-host-catalog.unit.test.ts
-
- src/main/native-chat/agent-session-wire/structured-agent-session-outgrown-child.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.
-
|
Claude’s saved Approve for me choice could silently become Ask, or stay downgraded after changing models. The previous fix was reverted after restored history left a turn marked running and delivery retried a waiting message without making progress, freezing the app. This restores saved approval intent, distinguishes actual process initialization and live work from imported history, and parks delivery when a step makes no progress. A message joining live Claude work retains that turn’s choices; the next idle message applies its own choices. Existing result grouping and combined quota-warning detail are preserved. The full loop finished in three production review rounds, with the end-of-round fresh decider ruling KEEP. Fixes covered partial execution readback, the attachment-preparation boundary, API-error-only output being mistaken for actual work, and delivery progress/ownership. Fresh correctness, consumer, whole-mechanism common-pattern comparison, readiness/performance and final QA evidence audits passed. No open proven finding or new P2/P3 deferral remains. Pre-existing journal-pressure ending loss, default-option rejection, approval display and unrelated lifecycle work remain outside this change; related work is tracked in STA-11280 and STA-11451. Verification:
For Brennan’s review — whole-window captures, visually inspected. Baseline images show the actual previous build, not a manufactured failure. Recovery result images also show the carried two-reply evidence.
Original restored Claude card before Steer; completed corrected live followup; carried Auto model-change A1/A2 replies at 845055c8. Desktop permission menu (845055c, unchanged UI):
Native phone permission menu (Auto offered, Full access selected) and live picker lock: 
Three code spots where judgment matters: actual work owns joining-message choices, replace only with no work owed, owned progress decides whether to retry or park. |
A Claude chat saved as Approve for me could reopen asking for every approval, or keep an earlier approval downgrade after a model change. The previous fix (#30914) was reverted in #33823 after an update could freeze the app with a message waiting. This restores saved approval intent and repairs the recovery and delivery mechanisms behind that freeze. Tracked in STA-10953.
What changes
The latest repair keeps main's opener, replay/fold and result paths unchanged. The independent actual-work witness is used only for input choices. Existing quota/error grouping and combined warning detail are preserved. No result registry, saved field, wire format, setting or paired-server minimum is added.
Intended behavior and limits
The common pattern separates requested choices, actual process initialization and live work from restored history. This follows that separation using Orca's existing journal and process ownership.
Orca retains its existing no-new-refusal path when work remains owed: messages go to the running process and an incompatible model transition waits for an idle opportunity. Mail keeps current choices. If the turn ends after preparation but before Claude consumes a joining message, that message can become the next prompt on the retained choices; it is neither refused nor automatically resent. Older CLIs that reject the requested mode keep the existing startup failure behavior; the saved choice is not rewritten.
The bounded quota/ordinary-output ordering proofs match main; they do not prove universal late-result attribution or external execution fate. Later live endings dropped under heavy journal load remain outside this change. Approval display and other lifecycle work remain tracked separately in STA-11280 and STA-11451.
Existing behavior, unchanged here: if Orca is quit or updated while a message has been handed to Claude but Claude has not acknowledged it yet, the message is marked unknown ("provider closed before acknowledgement") and is not resent after restart, so it shows one prompt with no reply. This avoids sending it twice. The graceful close settles it in
structured-agent-session-child-exit.ts:229-230andstructured-agent-session-dead-generation-settlement.ts:76,95. Recovery skips it becausejournal-pending-submission-recovery.ts:24-26andagent-session-queued-submission.ts:11treat only messages never handed over as queued. Delivery picks only those (structured-agent-session-start-failure-row.ts:149). These files are identical on main and this branch. A message never handed over survives the update and is delivered once.Verification
Review
Full path: native chat lifecycle, recovery, ordering, prior P0 incident and revert. Three production review rounds converged with fresh KEEP, whole-mechanism reference comparison and readiness/performance passes; the main integration was checked narrowly. An independent final evidence audit passes the adopted QA scope. No new proven finding is deferred. Current head is 14a562f; actual affected QA head ae74ff2 carries across that main-only merge. The loop status comment includes inspected screenshots, exact limits and the completion marker.
Agent skill upstream boundary
Author X handle: @BrennanKB5