Repository navigation
fix(native-chat): settle a dead agent's leftover work in one write that always fits - #26664
Conversation
…at always fits A dead generation's cleanup was several separately committed writes, and a single leftover item larger than one batch row was rejected on every try, so the chat stayed stuck. Plan every row first and commit them in one transaction: bounded batch rows, an oversize item as an ordinary row with its full body, and the settlement's own send verdicts. Readers see it only after the commit. Settlement rows are rebuilt only from schema-known fields, address saved items by their stored key, never invent an end time, and new writes require safe revisions, so cleanup never builds a row it cannot save.
…s a newer build saved - Codex's own exit cleanup hands the whole settlement to the journal's one-transaction planner instead of committing chunk by chunk. - One shared planner rejects an item named twice, on both the direct and resolved paths. - Rewriting a turn keeps fields this build does not know, as the journal schema promises; the legacy carrier's kind still cannot override it. - Drop the write-only revision admission and the planner skip that worked around it: insert admission is the reader's again, and a saved revision that cannot advance is skipped by the reducer while the rest settles, as before. - Remove two dead-generation journal members settlement no longer calls.
…nd keep nested turn fields Repeated Codex question ids in one write fold in order again instead of failing the write, and a saved turn's context usage keeps fields a newer build stored inside it.
…-fix # Conflicts: # src/main/native-chat/agent-session-journal/journal-row-writer.ts
Review summaryProblem. When a chat's agent died mid-response, Orca's tidy-up of what it left behind (the running response, tool calls, an open approval, messages waiting to be sent) was saved in several separate pieces, each in a row capped at 1.5 MB. If one leftover item was bigger than the cap, such as a tool call holding a ~1.6 MB file, the tidy-up failed on every attempt. The chat stayed "Working" forever, an old approval could take over the composer, and new messages failed with "Your message was not sent." Restarting didn't help. What changes for the user. After a crash, reopening the chat shows the old response as interrupted and the old approval as Cancelled, and the next message is delivered. No new UI or wording. Fixed during review
Deferred (pre-existing on main, not introduced here)
Verified
Not verified: Windows, Linux, a real SSH host, real Codex/Claude/OpenCode CLIs, mobile. Seen during QA, not caused by this PR (it doesn't touch these surfaces): Agent Session History listed the same chat twice; the chat's status read "Failed" after the crash was tidied up, then "Done" after the next reply. Not changed here (TEMPORARY): when the tidy-up runs. It still happens on exit and when a chat is opened or an agent is attached. Moving it to the moment a new owner takes over is the follow-up stack, starting with #26667. CI at 4a307ac: every check finished: 15 passed (all five unit test shards, relay integration, static analysis and typecheck, cross-version wire compatibility, package on macOS and Windows), and 17 were skipped by their path filters. No failures. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (41)
💤 Files with no reviewable changes (1)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review. 📝 WalkthroughWalkthroughThe change adds shared lifecycle mutation and item-revision helpers, updates journal batch planning and commits, and combines dispatch resolutions with lifecycle settlement. It also changes interrupted-turn handling to omit invalid completion times and filters invalid optional turn fields. New tests cover transaction rollback, retries, settlement idempotence, Codex exit convergence, prompt revisions, and persistence after reopening. Priority: ➖ Normal Merge Risk: ⚪ Minimal · up to Dead-agent cleanup now writes its settlement in one transaction, and oversized leftover items no longer block the chat. No concrete outstanding defects were identified, so the change appears ready to merge after normal checks. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
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 |
ELI5
When a chat's agent died mid-response (Orca quit or crashed, or the agent exited), Orca has to tidy up what the agent left behind: mark the response as interrupted, stop its running tool calls, cancel its open approval, and decide what happens to messages that were waiting to be sent. On main that tidy-up was split into several separately saved pieces, each in a database row capped at 1.5 MB. A single leftover item bigger than the cap (for example a tool call whose ~1.6 MB file content had been saved) made the tidy-up fail every time it ran. The chat then stayed "Working" forever, an old approval could take over the composer, and every new message failed with "Your message was not sent." / "OMP couldn't restart." Restarting Orca didn't help.
Now the whole tidy-up is saved as one all-or-nothing database transaction, and an oversized item is saved as its own full-size row instead of being forced into a capped one. After a crash, reopening the chat shows the old response as interrupted, the old approval as Cancelled, and the next message is delivered normally.
Before / after (real app, hidden test window, fake model, chat seeded with a 1.6 MB running tool call)
Before (main): the old response is still "Working" five minutes later and the new message is refused.
After, on reopening: the old response shows as ended, the tool call as interrupted, the approval as Cancelled, and the composer is free.
After, sending: the message is delivered and answered.
After quitting and relaunching Orca: still settled, with no duplicate stop rows, and a third message is answered.
What Changed
interruptedwith nocompletedAt, which the schema already allows. Items are addressed by their exact saved key, so old or unusual keys are rewritten in place rather than duplicated.Why
On main, the separately saved pieces could leave a partial result, and the size cap made some chats fail every retry with no way out. Truncating the big item would lose content, and catching the error would leave the chat stuck anyway. Building only rows the table accepts and saving them all together removes that whole class of failure for every caller, using the writer the journal already has. That's the common pattern: unfinished work is turned into terminal records in one transaction.
Differences from the common pattern:
unverifiable, because agents can outlive the app and run over SSH, and losing contact is not proof of death.Linked Issue
Supersedes #26576: this PR includes its size calculation and full-body test cases, but not its loop that commits chunks separately. #26576 will be closed once this lands.
Testing
I manually tested these changes locally (hidden isolated app rig on macOS with a fake model, screenshots above; no real agent login)
Automated tests added/updated, or explained why not below
Real-SQLite tests: the whole settlement commits once or not at all (rollback on a later row leaves disk, memory, cursor and subscribers unchanged), full-size rows for oversized items, idempotent replay and reopen, repeated items within and across rows, all-oversized settlements, unusual saved keys, untimed exits, preserved unknown turn fields, the Codex exit path with a refused queue, and repeated Codex question ids.
44 targeted test files, 552 tests, pass on the merged head. node / web / cli typechecks pass.
orca-ci-checksreports 22/22.QA (laptop, hidden window, before = main at the PR's base, after = this branch). Before: the chat stayed "Working" and both sends failed. After: settled on reopen, three messages delivered across a quit and relaunch, and the journal shows the 1.6 MB tool call rewritten in full with no partial or duplicate rows. Not exercised: Windows, Linux, a real SSH host, real Codex/Claude CLIs.
AI Disclosure
Built and reviewed with AI agents.
Review
Two review rounds plus a narrow check of the review fixes. Deferred, all pre-existing on main: a Codex turn-end settlement over 16 MiB is retried while the chat stays Working until the agent exits; the live end-of-turn cleanup rebuilds very old item keys from scratch; an oversized item body is serialized a few times while it's being sized. Merge order for the stack: this PR, #26555 and #26577 in any order, then #26667 and the move-the-settlement PR on top of it.
Agent skill upstream boundary
Notes
The execution host still owns the journal and its tidy-up; nothing here probes a remote host locally. Persisted rows and wire messages keep their shapes, so older clients read the same rows they read today. Folder workspaces are covered by the fixtures.
Author: Brennan Benson (@BrennanKB5).
Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)