Repository navigation
fix(native-chat): preserve accepted command outcomes across agent startup - #31605
Conversation
Setting a goal starts the agent when none is running, which moves the session's runtime fence before the host answers. The composer dropped that successful answer as stale, so the objective and Goal chip stayed. Keep the reply for goal changes as for the other writes that may start the agent.
|
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. 🎉 ℹ️ Recent review info
📝 Walkthrough
Priority: ➖ Normal Merge Risk: 🔵 Low · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 29.41% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 34 functions across 34 files. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Title check The title clearly and concisely describes the main change: preserving accepted native chat command outcomes across agent startup. 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 directly explains the problem, solution, scope, compatibility behavior, testing, CI status, QA limitations, and related issues. It does not follow every template headin… Full details: Description check
-
Explanation
The description is detailed and directly explains the problem, solution, scope, compatibility behavior, testing, CI status, QA limitations, and related issues. It does not follow every template heading: the ELI5, explicit Visual Proof, AI Disclosure, Review, Agent skill boundary, Notes, and Checklist sections are not separately completed, and no
Fixes #issue reference is provided. These omissions are non-critical because the required context is substantially present.
-
✨ 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
Caution
Some comments are outside the diff and can’t be posted inline due to GitHub limitations.
🟠 Major · Add capturePresentation to the default… · NativeChatInterruptedContinue.test.tsx:137-146
src/renderer/src/components/native-chat/NativeChatInterruptedContinue.test.tsx:137-146
🎯 Functional Correctness | 🟠 Major | ⚡ Quick winAdd
capturePresentationto the default transport inHarness.
capturePresentationis now a required property ofNativeChatStructuredComposerTransport. The fallback transport object inHarness(theprops.transport ?? {...}branch) does not define it. Typecheck fails with TS2322 at line 137, as the pipeline log shows. This blocks thestatic analysis and typecheckjob.The other fixtures in this file already received the stub at lines 391 and 518. Add it to this fallback too.
🐛 Proposed fix
transport: props.transport ?? { + capturePresentation: () => () => true, send: () => true, dispatchCommand: async () => ({ handled: false, accepted: false, error: null }),Source: Pipeline failures
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
3383cf42-bdd2-466a-90b5-9646b97f8fd2
📒 Files selected for processing: 28
src/renderer/src/components/native-chat/NativeChatComposer.resume-command.test.tsxsrc/renderer/src/components/native-chat/NativeChatInterruptedContinue.test.tsxsrc/renderer/src/components/native-chat/NativeChatQueuedMessageList.in-flight.test.tsxsrc/renderer/src/components/native-chat/NativeChatQueuedMessageList.test.tsxsrc/renderer/src/components/native-chat/native-chat-composer-draft-lifecycle.test.tsxsrc/renderer/src/components/native-chat/native-chat-composer-types.tssrc/renderer/src/components/native-chat/native-chat-image-references.test.tsxsrc/renderer/src/components/native-chat/native-chat-structured-send-composition-clear.test.tsxsrc/renderer/src/components/native-chat/native-chat-submission-navigation.test.tsxsrc/renderer/src/components/native-chat/structured-session-takeover-report.test.tsxsrc/renderer/src/components/native-chat/use-native-chat-composer-submit.test.tsxsrc/renderer/src/components/native-chat/use-native-chat-composer-submit.tssrc/renderer/src/components/native-chat/use-native-chat-rewind.test.tsxsrc/renderer/src/components/native-chat/use-native-chat-rewind.tssrc/renderer/src/components/native-chat/use-native-chat-structured-composer-send.test.tsxsrc/renderer/src/components/native-chat/use-native-chat-structured-composer-send.tssrc/renderer/src/components/native-chat/use-native-chat-structured-composer-transport.tssrc/renderer/src/components/native-chat/use-structured-agent-session-command-start-failure.test.tsxsrc/renderer/src/components/native-chat/use-structured-agent-session-mutate.test.tsxsrc/renderer/src/components/native-chat/use-structured-agent-session-mutate.tssrc/renderer/src/components/native-chat/use-structured-agent-session-queued-messages.resume.test.tsxsrc/renderer/src/components/native-chat/use-structured-agent-session-queued-messages.test.tsxsrc/renderer/src/components/native-chat/use-structured-agent-session-queued-messages.tssrc/renderer/src/components/native-chat/use-structured-agent-session-rewind.tssrc/renderer/src/components/native-chat/use-structured-agent-session.test.tsxsrc/renderer/src/components/native-chat/use-structured-agent-session.tssrc/renderer/src/components/native-chat/use-structured-native-chat-submit-reveal.test.tssrc/renderer/src/components/native-chat/use-structured-native-chat-submit-reveal.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 · Include the host and press fence in the Stop… · use-structured-agent-session-mutate.ts:172
src/renderer/src/components/native-chat/use-structured-agent-session-mutate.ts:172
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winInclude the host and press fence in the Stop deduplication key.
On an older host, a second Stop with the same fields can arrive after the runtime fence changes. This key joins the earlier request, so the second Stop is never sent with its captured fence and can report the earlier Stop’s success. Include
hostKeyandpressedFencein the key so only requests for the same host generation join.
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
f1ba8d08-703a-4294-8125-af8260633758
📒 Files selected for processing: 8
src/renderer/src/components/native-chat/NativeChatComposer.paste-remount.test.tsxsrc/renderer/src/components/native-chat/NativeChatComposer.test.tsxsrc/renderer/src/components/native-chat/NativeChatInterruptedContinue.test.tsxsrc/renderer/src/components/native-chat/native-chat-composer-prompt-recall.test.tsxsrc/renderer/src/components/native-chat/use-structured-agent-session-command-start-failure.test.tsxsrc/renderer/src/components/native-chat/use-structured-agent-session-mutate.test.tsxsrc/renderer/src/components/native-chat/use-structured-agent-session-mutate.tstests/e2e/cross-version-wire/agent-session-clear-old-client.unit.test.ts
🚧 Files skipped from review as they are similar to previous changes: 1
- src/renderer/src/components/native-chat/NativeChatInterruptedContinue.test.tsx
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 6 remain after this review.
brennanb2025
changed the title
|
Setting a goal in a fresh or resting Codex chat could start work but leave the objective and Goal chip in the composer. The renderer treated the startup generation change as authority to discard a successful command reply. This PR retains the actual result and settles captured content in the original chat, while late notices, focus and scrolling stay with the view that issued the action. Failed goals and newer writing remain available. Four review rounds complete, no open attributable findings. Reviewed head: Validation: current-head CI 40003934983 passed, including its final aggregate. The incorporated-main head passed 311 tests across 17 explicitly selected files, including Hangul/Japanese composition and acceptance-time draft settlement. The final test-only commit passed all 12 queue tests and independently verified current/obsolete already-sent Delete/Clear contracts. Main merge QA classification: Extra live evidence and limits: actual Codex Goal acceptance cleared the submitted composer/chip and produced its own reply; a real CDP IME composition retained 확인, and a bounded subsequent send received 확인했습니다. Actual Claude/Codex ordinary replies, Claude commands/options, question Answer→ALPHA31605, question Cancel→Interrupted, approval Allow and named task Stop were observed. The initial IME suffix was rejected before provider start and does not count as delivery. Full rewind/disposal, saved-card actions and the former complete provider matrix were not completed or claimed passed. Automatic mid-turn queue admission is dark; shipped saved-card actions remain supported and covered by renderer contracts. Observed in QA, not caused by this PR
Existing observations outside this cut remain separate: remembered options (tracked in #31208), pending-goal interaction, post-reply rewind catch-up, unrepresentable older-host answer feedback, Stop sharing across a full remount and failed-action queue refocus. No new capability/default/support choice or live performance measurement is claimed. For Brennan’s reviewThese are actual branch screenshots, not matched main/branch comparisons. No main live comparison completed.
Cleanup verified: owned apps, keepers, driver and terminals stopped; generated bundles restored and tracked checkout clean. QA ticket1889 released. Real config audit recorded a changed Codex config hash; Claude settings/hooks hashes unchanged. No real config contents were copied or restored. Three places for human judgment: |
Setting a goal in a fresh or resting Codex chat could start the goal while leaving its objective and Goal chip in the composer. Startup changes the runtime generation before the reply arrives, and the renderer discarded the successful reply as stale.
Each native chat command now keeps its actual result. An accepted goal clears unchanged submitted content; failed goals and newer writing remain available. The command finishes against its original chat while late notices, focus and scrolling apply only to the view that issued it.
Direct goals retain submitted content until acceptance because a refused goal removes its transcript row. Existing host admission, older-host requests and ordinary message sending stay supported. No wire format, saved-data schema, agent implementation, new control or screen layout changes are included.
This takes the full review path: asynchronous action ordering spans eleven renderer production files and follows repeated generation-based reply exceptions. Four rounds are complete, with no new finding in round four. All confirmed findings are fixed. Final whole-mechanism reference, readiness and performance source reviews are complete; differences from the common pattern are intentional and grounded in mixed-host compatibility and draft preservation. No live performance measurement is claimed.
Validation on reviewed head
f8269878a1af8395436ab49d22edfcd1183be0d0:qa=tests). No authored lifecycle/recovery, process ownership, Windows/WSL process or login flow requires a real QA round, and screen appearance is unchanged.Remembered option preferences (tracked in #31208), pending-goal interaction, post-reply rewind catch-up, older-host answer feedback, Stop sharing across a full remount and failed-action queue refocus remain separate existing observations. No new capability, default or support change is included.
Extra QA observations in unchanged code are tracked separately in STA-11942 (foreground Stop label) and STA-11943 (Claude rewind missing resume point). Their source traces and evidence limits are preserved in the review status comment.