Repository navigation
Native chat: one settlement transaction for everything an ended run left (4/5, rebuild of #26867) - #28111
Merged
Conversation
Each journal item row, subagent roster entry and background-task entry now records ownerFence: the lease fence of the agent run whose output produced it. Optional, persisted, no row version bump and no migration. The journal's itemFence() returns it, so the turn-verdict rules and per-run provider activity read the producing run rather than the writer's fence.
… record death first The host decides what work is current in one place (structured-agent-session-current-work.ts) from the lease, the exit this host saw, the runtimes it replaced, its live child and each item's provenance. Every host reader (queue, Stop, prompt answers, commands, status feed, idle sweep, teardown snapshot, history pages and frames, orchestration mailbox) reads it, so a dead agent's leftovers stop reading Working or holding the queue even when their cleanup write failed. The journal's activeTurnId()/liveTurnScope() become the raw runningTurn(). On exit the host records the child's end first and signals generationEnded, which republishes frames, the status feed and settlement waits and wakes the queue drain with no journal row. Pages and frames carry optional working and actionablePromptIds fields.
…wers to a stopped agent plainly Desktop and mobile prefer the host's working and actionablePromptIds fields when present (merged across batches like latestTurn) and fall back to today's row derivation against an older host. A prompt an ended agent raised stops looking active; answering it is refused before any provider sees it, with "<Agent> has stopped, so your answer was not sent. Send a message to continue." Cancelling it only dismisses it in the journal: it no longer triggers the chat's Stop, pauses the queue or withdraws queued cards.
… run left An exit now settles, in one lifecycle batch planned at its own turn in the write queue, everything its run left (turn, calls, prompts, reasoning, unanswered sends) plus leftovers of earlier ended runs, and only the subagent roster entries and background tasks those runs observed, by each entry's own provenance. Main's stale settle moves to stale-state-settlement.ts with fence scoping; dead-generation-settlement.ts becomes a planner; leftover-settlement.ts combines them. Resume eligibility widens in one case: this host saw the root exit at the lease's fence and the release write failed; the acquisition replaces the owner only once a probe proves it gone.
This was referenced Oct 13, 2026
…l until a caller needs it Nothing outside the projection calls it in this part; Part 5 exports it for the start-failure row.
…wner checks into the main case Drops the epoch-replacement case (Part 2 rewind test reads the same values through the same rebuild) and the corrupt-value case (no version writes one).
…nd the still-live rewind case The flow cases keep every outcome: dead leftovers read as nothing, Stop has nothing to stop, a waiting card drains, a live owner publishes as before, and a rewind never promotes dead work.
…plit-3 # Conflicts: # src/main/native-chat/agent-session-wire/structured-agent-session-current-work.test.ts
…ack each Keeps desktop and phone Working from the host (with the older-host fallback), the reducer merge contract, the ended question Cancel that only dismisses, the agent-named refusal and the resolution receipt. Drops per-helper prompt tests, the transcript-placement test, the stop-note client reduction, the Steer cards that match main, and per-reason refusal rows.
…ws and the per-entry scoping flow Drops the no-added-delay timing check, the frame-once check, two stamping unit cases and the Codex combined case (the roster flow test covers its outcome).
… the unstamped-entry case to Part 1
…s as its row provenance Moved here from Part 4 so it sits beside stampJournalEntryProvenance.
…plit-1 # Conflicts: # src/main/acp/acp-timeline-grok-subagent-retention.test.ts # src/main/acp/acp-timeline-grok-subagents.test.ts
…n removed its body helper Main #27947 writes one row per subagent and deleted journal-subagent-group-body; a roster row from an older build is still what this case reads.
…amping Main's #27947 writes one saved row per subagent, and each background task already has its own row, so an item's own ownerFence names the run of every entry a producer still writes. Per-entry stamps inside a row mattered only for the shared roster rows saved before #27947, and no run writes into those any more, so the row's own provenance settles them correctly. Removes the entry stamping, the optional ownerFence on roster entries and background tasks, and their tests.
…plit-2 Brings in main 6741f4f through part 1. Main's history page now reads with a HistoryReader, so the published current work moves to agent-session-published-work.ts and lays over each finished page. Main's new Stop path (chat-stop target, acceptance, unconfirmed note, targeted cancel) and the per-subagent row writer read what runs through the host's current-work projection, as the rest of this part does.
…plit-3 Brings in main 6741f4f through parts 1 and 2. Main now names the agent in "didn't accept this message"; the stopped-agent words this part adds keep their own module beside it.
…plit-4 Brings in main 6741f4f through parts 1 to 3, and drops the per-entry scoping of roster and background-task settlement. Main's #27947 saves one row per subagent and new runs no longer write the shared roster row, and every background task already had its own row, so each item's own provenance names the run that ran it: an ended run's rows are settled whole, as on main. Removed: the `lost` entry filter in journal-subagent-liveness.ts (now main's), `entriesBelow` and `journalEntryOwnerFence`, and the roster-provenance test that pinned them.
…isions keep it A newer run's provider can revise an item an older run created: Pi's start and an ACP session/load both report context usage onto the newest saved turn. Restamping that turn with the newer run let the newer run's exit settle the older run's unverifiable turn as interrupted. Only creation states an owner now, as main's itemFences rule did.
…plit-2 Carries part 1's fix: an item's provenance is set when it is created and revisions keep it.
…plit-2 Syncs main b83a6f7 through part 1.
…plit-3 Syncs main b83a6f7 and part 1's creation-only provenance through part 2. Main grew both chat-session hooks to their line limit, so this part's prompt reads take the transport state whole on desktop and one memo on the phone.
…plit-4 Syncs main b83a6f7 and part 1's creation-only provenance through part 3.
…pt, through dismissPrompt The ended-generation dismissal wrote the cancelled resolution without the Cancel's operation receipt and without the dismissal's failure handling, so a receipt write failing after it would answer "outcome unknown" over a card already closed. It now goes through dismissPrompt, which carries the uncommitted receipt in the same append and answers dismissalNotSaved on failure; a null `answer` skips the adapter, since no live provider holds an ended generation's card (Pi's adapter refuses a fence with no live session outright).
…plit-4 Carries part 3's ended-card Cancel through dismissPrompt.
Part 2 landed on main as 3bfb114, so files this part never changed take main's text. Where both sides changed a file, the result is this part's change laid onto main, checked against Part 2's final head 674dbed: - the client coalescer keeps this part's merged `actionablePromptIds` and `working` beside main's `hasNewer` (#28333, the "New" line above unseen messages); - the transcript row keeps this part's actionable-prompt import beside main's new-rows marker imports (#28333).
Part 3 now sits on main (Part 2's squash 3bfb114), so this brings Part 4 onto main's current state. No conflicts.
…the run executing it A subagent row that run 1 created and run 2 resumed kept run 1's creation fence, so settling run 1's death marked run 2's live subagent unverifiable. Claude background tasks reuse their rows the same way (a matching parent tool use, or a parentless row). Rows of reusable child execution now also name their execution owner: the fence of the child process whose report the write copies, captured when that report arrives, never the lease that is current when a queued write flushes. - Subagent rows: the host captures the live child's fence when its child-work report changes the records, and rewrites a running subagent another child now reports even when its text is unchanged. - Claude background-task rows: written through that child's own event sink, which stamps its bound fence. - Creation provenance (`ownerFence`) is unchanged. Writes that do not state an owner (settlement, Pi's start usage, ACP session/load replay, ACP tasks) keep it, and a row without one falls back to its creation fence. `journal.itemFence()` now answers whose execution an item is (`executionOwner ?? ownerFence`), so every settlement and liveness read, inside its write transaction, follows the run that resumed the work. Also drops the event sink's compatibility alias for the identity resolver (the Codex sink names the resolver itself), keeping the sink under its line limit.
… it copies Review of c0add67: the subagent row writer read its execution owner from the host's installed child when a report landed. A child that is still being acquired reports a resumed subagent before the host installs it, so its row was written at once under the old owner, then the attach's settlement of what the previous child left marked it unverifiable, and nothing rewrote it until the next report. Each adapter (Claude, Codex, ACP) now hands its child-work report to the host with that child's own event sink, the write path its rows already take. The host passes it through the status feed to the subagent row writer, which writes the rows that report changes through that sink with `executes`. They land in the child's own order, after the attach's settlement, at the fence the sink is bound to, and that fence is the row's execution owner. A running subagent a child has not written yet is written once even when its text is unchanged. Changes the host makes itself (a Stop's acknowledgement, an end) still write directly and keep the row's owner. The shared child-work evidence type is unchanged: the sink travels as a separate argument. Test: a Claude helper the next child reports again while it is still starting stays working after that start's settlement, through the shipping adapter and host. It fails on c0add67 (the row lands `unverifiable`).
CI's type-aware lint flagged the row writer handing `Promise.all` a value that could be undefined when a sink has no `written()` barrier. A sink without one now reads as landed.
…ugh a rewind Review of b161aa3 / 129f36d: - `admitChildWork`'s producer is optional again, so the host's own turn-started report compiles. - A subagent row written through the reporting child's sink uses the sink's checked admission. A write it refuses under backpressure is queued again once what the sink holds has landed; a refusal or a failed write frees that child's claim on the row, so a later report retries it. - A rewind carried `itemFence()` (now whose execution an item is) into the rebuilt row's creation `ownerFence`, and dropped `executionOwner`. Retained rows now carry both as stored (`journal.itemProvenance`), the rewind record keeps `executionOwner` beside `ownerFence`, and both rebuild paths write it back: epoch replacement and the context rewind, which builds its rows from the same replacement items. Test: the two-fence test now rebuilds the epoch from the resumed rows and settles the first run's death again; the resumed rows stay live. It fails if the epoch replacement drops `executionOwner`.
It opens a real journal database, so it belongs on the list the SQLite runtime boundary test checks.
brennanb2025
changed the base branch from
brennanb2025/settle-split-3
to
main
October 15, 2026 07:49
main moved journal-atomic-terminal-settlement.test.ts into a registered .spec.ts (#29163); this branch's edit (the observed-exit test settler) is carried there. Registrations take main's createAcpAdapter shape with this branch's producer argument.
brennanb2025
marked this pull request as ready for review
October 16, 2026 02:07
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Priority: ➖ Normal Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 48 functions across 50 files. (12 skipped… | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Title check The title clearly identifies the primary change: combining settlement into one transaction for ended runs. The part and issue context are concise and relevant. Description check The description is comprehensive and follows the required template. It explains the user impact, implementation, rationale, linked issue context, testing, compatibility considerations, and deferred sc… 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: Docstring Coverage
-
Explanation
Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 48 functions across 50 files. (12 skipped: 12 over the file limit.)
✨ 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.
This was referenced Oct 18, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part 4 of 5 of a stack that rebuilds #26867 as smaller PRs. Stack: Part 1 #28092 (merged) → Part 2 #28103 (merged) → Part 3 #28104 (merged) → Part 4 (this PR) → Part 5 #28122. Base:
main.ELI5
When an agent exits, Orca writes a tidy-up for what it left: the reply it was writing, its tools, an approval it was waiting on, its subagents and background tasks, and messages it never answered. Today that tidy-up is split across several writes and rules. This PR settles everything an ended run left in one write, touching only the rows that run (or an earlier ended run) created, so Part 5 can tidy up while a newer run is live.
What Changed
The problem
unverifiableuntil its next update. Claude background tasks reuse their rows the same way (a matching parent tool use, or a parentless row).Before and after, as you see it
After:
Writes still use the normal 5 s wait in this part, and settling at open and at start is unchanged. A failed exit settlement is picked up by those existing paths, as on main.
How it works
structured-agent-session-dead-generation-settlement.tsbecomes a planner (planStructuredAgentSessionDeadGeneration) for the exit's own rows;structured-agent-session-stale-state-settlement.tsholds main's stale settle, moved, plus scoping by each item's provenance (below).structured-agent-session-leftover-settlement.tscombines both into one lifecycle batch planned at its own turn in the write queue (appendPlannedLifecycleBatch), so an answer or Stop queued ahead of it is what it settles from.child-exit.tsbuilds the exit's account and settles through the leftover settlement before the release, with the host's sight of the exit (ended) so the run reads ended even if the release fails.structured-agent-session-resume-eligibility.tswidens in exactly one case: this host saw the root exit at the lease's own fence and the record is surface-releasable. The acquisition then probes the owner and replaces it only once proven dead.executionOwneron the item row, the fence of the child process whose report the write copies. Part 1's creationownerFenceis unchanged.structured-agent-session-subagent-rows.ts): each adapter (Claude, Codex, ACP) hands its child-work report to the host with that child's own event sink, the path its rows already take; the status feed passes it to the row writer, which writes the rows that report changes through that sink withexecutes. They land in the child's own order, after the settlement its attach writes first, at the fence the sink is bound to: a child still being acquired, not yet installed, cannot lose a resumed subagent to that settlement. A running subagent a child has not written yet is written once even when its text is unchanged. Writes use the sink's checked admission: one refused under backpressure is queued again once what the sink holds has landed, and a refusal or failure frees the child's claim so a later report retries it. Changes the host makes itself (a Stop's acknowledgement, an end) write directly and keep the row's owner. The shared child-work evidence type is unchanged; the sink travels as a separate argument.claude-background-task-row-journal.ts): written through that child's own event sink withexecutes, which stamps the sink's bound fence (structured-agent-session-journal-append-options.ts).journal.itemProvenance): the rewind record keepsexecutionOwnerbesideownerFence, and both rebuild paths (epoch replacement, and the context rewind built from the same replacement items) write it back.journal.itemFence()answersexecutionOwner ?? ownerFence(journal-item-provenance.ts,journal-store.ts), so every settlement and liveness read, made inside its write transaction, follows the run that resumed the work. A row without the field falls back to its creation fence.orca-stop-row.ts) is written at the lease's current fence, which the child's end may have moved past the stopped child's.Why
One plan per settlement removes the class of bugs where two cleanup writers disagree about scope. Scoping by each item's provenance (Part 1) is what makes it safe to settle while a newer run is live, which later parts need because settlement moves off the open path. Earlier rounds of this PR also scoped each entry inside the shared subagent roster row by its own stamp. Main's #27947 replaced that row with one row per subagent and no new run writes the old one, so that scoping was removed and liveness settlement is main's (
journal-subagent-liveness.tsunchanged).Keeping fail-fast writes out of this part is deliberate: writes that give up at once with no retry would leave work unsettled after brief locks, so fail-fast lands with its retry in Part 5.
Differences from the common pattern and from main
Linked Issue
Part of the rebuild of #26867 (root-cause follow-up to #26664).
Visual Proof
N/A for this part on its own: the visible effect is which rows an exit settles. The full stack's before/after screenshots and QA are in Part 5.
Testing
I manually tested these changes locally
Automated tests added/updated, or explained why not below
New:
structured-agent-session-exit-delivery.test.ts(a card behind an exit is sent over a proven-dead owner whose release keeps failing, with no Working frame and no answerable prompt before it; an alive or unproven owner is never replaced and the card comes back), plus thesettlement-doubleandobserved-exitfixtures. The roster-provenance test was removed with the per-entry scoping it pinned.Dropped as mirrors or review-round guards with no observed bug: a no-added-delay timing check, a frame-published-once check, a resume-eligibility predicate case, two stamping unit cases and a Codex case that called the planner directly.
Updated: dead-generation settlement, unexpected exit, startup-failure exit, atomic terminal settlement, reasoning sweep, recovered-turn clock, late-proof tool ends, stale-turn verdict, Claude permission request, ACP host crash (
settleObservedExitForTestreplaces the removed exit settler).New:
structured-agent-session-resumed-child-owner.test.ts: run 1 creates a subagent row and a Claude background-task row, run 2 resumes both under fence 2 through its own sink, then run 1's death is settled; both stay live, and stay live again after the epoch is rebuilt from those rows as a rewind does. It fails without the execution owner, and fails if the epoch replacement drops it (the existing restart test runs both runs at fence 1, so it never covered this).New case in
structured-agent-session-claude-subagent-row.test.ts, through the shipping Claude adapter and host: the next child reports a helper running again while it is still starting; the helper stays working past that start's settlement. It fails on the first version of this change (c0add67cca4), where the row landsunverifiable.Merged main
b83a6f74be9through Part 3, and removed the per-entry roster scoping (see Why).Merged Part 3
3dce3120fea(onmain), with no conflicts.Merged
maine2f2277d512as24c5e672c17. Two conflicts: main movedjournal-atomic-terminal-settlement.test.tsinto a registeredjournal-atomic-terminal-settlement.spec.ts(ci: reduce fixture startup and shutdown waits #29163, same cases, wrapped in a function), so this PR's edit to it (the observed-exit test settler in place of the removed exit settler) is carried into the.spec.tsunchanged; andstructured-agent-runtime-registrations.ts, where main split the ACP adapter intocreateAcpAdapter(adding its data directory) and this PR passes the producer topublishChildWorkEvidence, resolved as main's shape with the producer argument. The PR's diff againstmainis otherwise file for file the same. Part 4's test files pass locally (16 files, 183 tests, plus the SQLite boundary test).CI at
24c5e672c17: green, 14 pass and 18 skipped, including the five test shards, static analysis and typecheck, and the test vs non-test line count check.Merged
maincaa4155e61b(which has Part 3 asd9de1fad1da) asc48875bce11. One conflict, in the SQLite test list, where this PR's entry sits next to linesmainchanged; resolved by keeping this PR's entry andmain's lines. The PR's diff againstmainis file for file the same as its diff against Part 3 before the merge. Part 4's own 16 test files pass locally on the merged tree (132 tests).Local at
44f283eadb0: focused Node runs of the touched areas pass (rewind, retained rows, epoch replacement, status-feed child work, subagent rows, the Codex and ACP child-work producers, the Claude helper flows: 24 files, 193 tests). Earlier on this branch,src/main/native-chat,src/main/claude,src/main/codexandsrc/main/acppassed whole, and the real Claude CLI tests passed (ORCA_REAL_CLAUDE_CLI_TEST=1, 14 tests). Lint of the changed files passes. Typecheck and full lint are CI's: no local or remote typecheck runs.CI at
c48875bce11(onmain): green, 15 pass and 17 skipped.CI at
44f283eadb0: green, 15 pass and 17 skipped (this base runs the smaller workflow), including the five test shards, relay integration, static analysis and typecheck, and the test vs non-test line count check. Earlier heads failed on a test-only type error, a type-aware lint finding and the SQLite test list, each fixed in its own commit.Size against Part 3: production +693/-277, tests +558/-151.
What the reviewer should check
owns(fence)scopes the exit's own rows to its run and later;belowscopes the stale settle to rows ended runs created, so a later live run's rows stay untouched.staleStopRow,lastProvenTurnLiveAtandrecovered.itemFence()reads it first everywhere.AI Disclosure
Review
Stacked on Part 3. Part 5 builds on this branch.
Agent skill upstream boundary
Notes
executionOwner, beside Part 1'sownerFence, and the same optional field on a rewind record's retained rows. Older builds ignore it, rows without it fall back to the creation fence, and no client wire changes.Author: Brennan Benson (@BrennanKB5).
Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)