Repository navigation
fix(chat): keep resumed Claude messages deliverable after settings reports - #30982
Conversation
|
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:
📝 Walkthrough
Priority: ⚪ Pending latest changes Merge Risk: 🔵 Low · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 46.88% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 32 functions across 28 files. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Title check The title clearly identifies the main change: keeping resumed Claude messages deliverable after settings reports. 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 The description is detailed and covers the user impact, mechanism, rationale, testing, visual proof, review context, and regression scope. It includes the required major sections, but it does not prov… Full details: Description check
-
Explanation
The description is detailed and covers the user impact, mechanism, rationale, testing, visual proof, review context, and regression scope. It includes the required major sections, but it does not provide a direct issue link in the Linked Issue section and omits the template checklist and Agent skill upstream boundary responses. These omissions do not prevent approval because the description is otherwise complete and focused.
-
✨ 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.
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.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
470edbc7-4f06-4953-998e-7a5ac92fac2a
📒 Files selected for processing: 19
src/main/claude/claude-stream-json-connection.test.tssrc/main/claude/claude-structured-dispatch-test-support.tssrc/main/claude/claude-structured-effort-reporting.test.tssrc/main/claude/claude-structured-message-options.test.tssrc/main/claude/claude-structured-message-options.tssrc/main/claude/claude-structured-model-confirmation.test.tssrc/main/claude/claude-structured-model-preflight.test.tssrc/main/claude/claude-structured-option-confirmation.test.tssrc/main/claude/claude-structured-options.test.tssrc/main/claude/claude-structured-options.tssrc/main/claude/claude-structured-retired-model.test.tssrc/main/claude/claude-structured-retired-model.tssrc/main/claude/claude-structured-session-acquisition.tssrc/main/claude/claude-structured-session-adapter.tssrc/main/claude/claude-structured-session-options.tssrc/main/claude/claude-structured-session-publication.tssrc/main/claude/claude-structured-session-startup.tssrc/main/claude/claude-structured-session-state.tssrc/main/claude/claude-structured-spawn-options.ts
💤 Files with no reviewable changes: 3
- src/main/claude/claude-structured-session-acquisition.ts
- src/main/claude/claude-structured-session-startup.ts
- src/main/claude/claude-structured-session-options.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 1 remain after this review.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to GitHub limitations.
🟡 Minor · Use a per-key fence for appliedInputOptions. · claude-structured-options.ts:119-135
src/main/claude/claude-structured-options.ts:119-135
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse a per-key fence for
appliedInputOptions.When a model write awaits
setModel, an overlapping effort write advances the shared mutation sequence. The model write can succeed but skip itsappliedInputOptionsmarker. The next dispatch then sendsset_modelagain becauseapplyClaudeMessageOptionsdoes not find the model marker.Use a per-key sequence for the marker. Keep the existing shared fence for stale same-key writes and other bookkeeping.
Suggested fix
const OPTION_ORDER = ['model', 'effort', 'fastMode'] as const +const appliedInputOptionSequences = new WeakMap<ClaudeSession, Map<string, number>>() + +function nextAppliedInputOptionSequence(session: ClaudeSession, key: string): number { + let sequences = appliedInputOptionSequences.get(session) + if (!sequences) { + sequences = new Map() + appliedInputOptionSequences.set(session, sequences) + } + const sequence = (sequences.get(key) ?? 0) + 1 + sequences.set(key, sequence) + return sequence +} + +function isCurrentAppliedInputOptionSequence( + session: ClaudeSession, + key: string, + sequence: number +): boolean { + return appliedInputOptionSequences.get(session)?.get(key) === sequence +} + ... session.appliedInputOptions.delete(input.key) const mutationSequence = ++session.optionMutationSequence + const appliedInputOptionSequence = nextAppliedInputOptionSequence(session, input.key) ... - if (mutationSequence === session.optionMutationSequence) { + if ( + isCurrentAppliedInputOptionSequence(session, input.key, appliedInputOptionSequence) + ) { session.appliedInputOptions.set(input.key, input.value) }
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
980ec7e7-ac14-4637-93ce-1f373a32ce0f
📒 Files selected for processing: 7
src/main/claude/claude-structured-message-options.test.tssrc/main/claude/claude-structured-message-options.tssrc/main/claude/claude-structured-session-adapter-turns.test.tssrc/main/claude/claude-structured-session-adapter.tssrc/main/claude/claude-structured-session-options.tssrc/main/claude/claude-structured-session-publication.tssrc/main/claude/claude-structured-session-startup.ts
💤 Files with no reviewable changes: 2
- src/main/claude/claude-structured-session-startup.ts
- src/main/claude/claude-structured-session-options.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 6 remain after this review.
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
6e71db60-3c6b-4323-a306-0645fb9b54c5
📒 Files selected for processing: 14
src/main/claude/claude-structured-applied-options.test.tssrc/main/claude/claude-structured-execution-observations.tssrc/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-structured-options.tssrc/main/claude/claude-structured-retired-model.test.tssrc/main/claude/claude-structured-retired-model.tssrc/main/claude/claude-structured-session-acquisition.tssrc/main/claude/claude-structured-session-options.tssrc/main/claude/claude-structured-session-startup.tssrc/main/claude/claude-structured-session-state.tssrc/main/native-chat/agent-session-wire/structured-agent-session-message-choices.test.tssrc/main/native-chat/agent-session-wire/structured-agent-session-ran-options.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.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments: 1
src/main/claude/claude-structured-options.ts: 1
178-180: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the duplicate Fast-off cleanup.
Lines 178-180 and Lines 188-190 both delete
session.fastModeReportedMutationwhen the readback reportsfastMode === false. The second block already covers the case, becauseadoptedisreadClaudeSettingsFastMode(settings)when the key isfastMode. The first block also runs for effort writes, so it is the only path that clears the marker there. Keep one block, and make its scope explicit.
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
1b8a12c7-8de9-43bc-a392-4b63eeb1cca8
📒 Files selected for processing: 9
src/main/claude/claude-structured-execution-recording.test.tssrc/main/claude/claude-structured-message-options.tssrc/main/claude/claude-structured-options.tssrc/main/claude/claude-structured-session-acquisition.tssrc/main/native-chat/agent-session-wire/structured-agent-session-adapter.tssrc/main/native-chat/agent-session-wire/structured-agent-session-handover.tssrc/main/native-chat/agent-session-wire/structured-agent-session-host-delivery.tssrc/main/native-chat/agent-session-wire/structured-agent-session-message-choices.test.tssrc/main/native-chat/agent-session-wire/structured-agent-session-ran-options.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 0 remain after this review.
|
Review loop status: done on 5705788 (f95df94 plus a clean merge of current main)
|
…t-cause-architecture
ELI5
After Orca restarted a Claude chat, the next message (and sometimes every message after the first one) showed “Claude didn’t accept this message.” Claude never saw those messages: Orca refused them itself before sending. Users hit it repeatedly; it blocked chats saved with model aliases such as
opus[1m].Regression
Introduced by #28046, merged October 19 at 10:53 PT in commit
53191250e87, and first shipped in v1.4.231; v1.4.232 and v1.4.233 also contain it. That change made Orca forget the model, effort and Fast choices it had already applied whenever Claude reported its model under a different name. A savedopus[1m]always comes back asclaude-opus-5-5[1m], so a restarted chat either refused its first message or accepted one message and refused every follow-up. Before #28046 there was no such check, and every one of these messages was delivered.What changes for users
Restarted and resumed Claude chats send normally again, including chats saved before this fix on an alias like
opus[1m]with high effort. Choosing a different model or effort still applies it before the next message, and a real refusal from Claude still shows as a failure. Nothing about saved chats, settings or the data exchanged with remote hosts changes.What changed (mechanism)
Why this approach
The bug came from one record doing two jobs: it was both "what we applied" and "what the agent reported", so any differently worded report erased what had been applied and the next send was refused. Keeping applied input separate from reports follows the common pattern and removes the whole failure mechanism for Claude rather than adding an exception for aliases.
Differences from the common pattern
--settingsAgent Argument plus saved Fast on a model the list does not identify (the picker never offers Fast for such a model).Linked Issue
User-reported incident (repeated "Claude didn't accept this message." on October 19 and 20). Release cherry-pick tracked in STA-11434.
Visual Proof
Real Electron QA on the exact final head
f95df9417730, hidden isolated test copy on macOS, real agents signed in through the team's load balancers. Each chat was created and saved on a build from before this fix (main3be5039, which still has the bug), then opened on this head, the app was restarted, and two ordinary messages were sent in the same conversation.opus[1m]/ High / Fast off)No "didn't accept this message" appeared for any agent, and every send was accepted. The saved choices stayed the user's literal picks; nothing written back came from an unconfirmed request. Real Claude CLI suites passed 15/15 on this head.
Claude chat saved before the fix, after restart (two follow-ups answered):
Claude after changing model and effort:
Codex, Pi, OpenCode and OMP chats saved before the fix, after restart:
Not tested (evidence):
Testing
opus[1m]/ high / Fast off, Claude reporting the resolved model name, missing settings readback, two sends): they fail on Accept messages while any agent starts, and bound each startup step #28046's commit and on main and pass here. A probe of 10 incident combinations is refused 10/10 on main and admitted on this branch; the commit before Accept messages while any agent starts, and bound each startup step #28046 admits all of them.claude-structured-real-cli*.test.ts): 15/15 on the final head.Review
Full review path: two broad review rounds, an architecture panel for the main-merge integration, a bounded final correction, and a fresh final review plus reference comparison on the final head with every difference labelled above.