Repository navigation
Native chat: one current-work projection for every host reader, death recorded first (2/5, rebuild of #26867) - #28103
Merged
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.
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.
…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.
…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.
brennanb2025
changed the base branch from
brennanb2025/settle-split-1
to
main
October 14, 2026 19:07
…plit-2 Part 1 landed on main as fd4a802. Main's #27623 (a stopped message reaches the agent with the next send) crossed this part's reads of what runs: its Stop withdrawal words are fixed on main, so the acceptance input keeps only the host's live turn; and the stopped history a handover carries is decided from the host's current-work turn scope, the same one the dispatch row is written with.
brennanb2025
marked this pull request as ready for review
October 15, 2026 00:53
Contributor
📝 Walkthrough
Priority: ➖ Normal Merge Risk: 🔵 Low · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 62.96% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 54 functions across 51 files. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation 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. Title check The title clearly identifies the main changes: a shared current-work projection and recording agent death before further writes. The part and issue context are appropriate. Description check The description is detailed and follows the repository template. It explains the problem, behavior changes, design, testing, issue context, compatibility considerations, and checklist status. The link…
- Fix all pre-merge checks with AI
✨ 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.
Main's #28333 (a "New" line above unseen messages) marks a catch-up page that has more pages behind it with an optional `hasNewer` on the batch event. It crossed this part's current-work fields on the same event: the batch type keeps both `hasNewer` and `AgentSessionCurrentWorkFields`, and the subscriber catch-up page sends `actionablePromptIds`, `working` and `hasNewer` together.
Main's #28394 (chat scratch folders expire after 30 days unused) named the idle sweep's two dispatch checks, `hasOpenDispatch` and `providerHoldsDispatch`, so its new `runWhileExecutionIdle` veto can reuse them. That crossed this part's `store` reads handed to the same sweep: the sweep now gets this part's `store` plus main's named checks. The veto keeps main's own reads.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
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 · Do not renew idle activity when restating a… · structured-agent-session-client-delivery.ts:141-147
src/main/native-chat/agent-session-wire/structured-agent-session-client-delivery.ts:141-147
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winDo not renew idle activity when restating a generation end.
An unexpected child exit with a successful owner release reaches
generationEnded(..., { restate: true }). That branch callssnapshot, whose publication hook callspublishJournalwith its defaultactivity = true. The queue then callssessions.touch(sessionId). This renews the idle clock even though generation end is not activity, and can delay idle-sweep stop and conversation close by another idle interval.Pass
activity: falsethrough the restate snapshot hook.Suggested fix
diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-subscribers.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-subscribers.ts @@ - onJournalPublished?: (sessionId: string, journal: AgentSessionJournal) => void + onJournalPublished?: ( + sessionId: string, + journal: AgentSessionJournal, + activity?: boolean + ) => void @@ - snapshot(sessionId: string, journal: AgentSessionJournal, fence: number): void { + snapshot( + sessionId: string, + journal: AgentSessionJournal, + fence: number, + activity?: boolean + ): void { @@ - this.hooks.onJournalPublished?.(sessionId, journal) + this.hooks.onJournalPublished?.(sessionId, journal, activity) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-client-delivery.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-client-delivery.ts @@ - onJournalPublished: (sessionId, journal) => this.publishJournal(sessionId, journal), + onJournalPublished: (sessionId, journal, activity) => + this.publishJournal(sessionId, journal, activity), @@ - this.subscribers.snapshot(sessionId, journal, fence) + this.subscribers.snapshot(sessionId, journal, fence, false)
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
592de49e-adfa-449c-8d4f-a773155cb286
📒 Files selected for processing: 3
config/scripts/vitest-sqlite-runtime-files.mjssrc/main/native-chat/agent-session-wire/structured-agent-session-conversation-lifetime.tssrc/main/native-chat/agent-session-wire/structured-agent-session-host.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.
This was referenced Oct 15, 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 2 of 5 of a stack that rebuilds #26867 as smaller PRs. Stack: Part 1 #28092 (merged) → Part 2 (this PR) → Part 3 #28104 → Part 4 #28111 → Part 5 #28122. Base:
main(Part 1 is merged asfd4a802b3ce).ELI5
When a native chat's agent dies or is replaced, the reply it was writing, its tool calls, an approval it was waiting on and the messages it hadn't answered are still saved as "running" until a cleanup write marks them finished. Today every part of Orca decides "is the agent working?" by reading those raw rows, so if the cleanup write fails or hasn't run yet, the chat keeps saying Working, Stop stays offered, and queued messages wait forever.
This PR gives the host one place that answers "what work is current right now", and makes every host reader ask it. A dead agent's leftovers stop counting the moment Orca knows the agent ended, whether or not the cleanup write landed.
What Changed
The problem
worktree ps, mobile list), the orchestration mailbox's idle check, and the frames sent to desktop and mobile.Before and after, as you see it
Before. After an agent died mid-reply and its cleanup write failed: the chat could keep reading Working with Stop, the session list could keep counting it as working, and a card queued behind the dead turn did not send.
After, on the host:
Opening a chat still settles leftovers exactly as on main; this PR only adds a derived reading on top. Desktop and mobile start using the new fields in Part 3; until then they keep deriving from rows as today.
How it works
structured-agent-session-current-work.tsdecides what is current from:lastEndedChild), which ends a run even if its release write failed;ownerFence).An owner that is unreconciled or recovering, with no child in this process, reads "unverifiable": nothing it left is current, and nothing is written.
heldStructuredAgentSessionCurrentWork, with this host's child as evidence): the queue gate and drain, Stop and its notes, prompt answers' turn scope, conversation commands, the status feed and session list, the idle sweep, teardown's restart snapshot, history pages and subscriber frames, and the orchestration mailbox pointer. The journal'sactiveTurnId()andliveTurnScope()are replaced byrunningTurn(), the raw read, so no reader can skip the projection by accident.recordProviderChildEndruns before the exit's tail rows, its settlement or its release, then agenerationEndedsignal fires. The child stays on record untilendChildso its sink can drain the tail.publishGenerationEndedre-derives the status feed, settlement waits and each subscriber's frame, and schedules the queue drain, without renewing the idle clock. A subscriber's caught-up frame restateslatestTurn,actionablePromptIdsandworkingwhenever the projection's view changed (viewKey, which includes "unverifiable").workingandactionablePromptIds, besidelatestTurn. No new stream opcode; old clients ignore them.Why
Main decided "working" in many places from rows that only a successful cleanup write would fix, so any failed or late write left the chat stuck. Deriving current work in one place from facts the host already holds (the lease, the exit it saw, provenance) makes the answer correct immediately and makes cleanup pure bookkeeping that never gates the person.
Rejected: holding the queue while an owner is unconfirmed (it held a person's own send as a card nothing would send; main never held there). The queue hold here is exactly main's ("working" or "prompt"), read through the projection, and never holds on the lease alone.
Differences from the common pattern and from main
generationEndedstraight topublishGenerationEndedand bumpsoperationalRevisioninstructured-agent-session-host.ts(about 8 lines). Part 5 replaces this with the retry coordinator'ssignal, which does the same plus the background settlement.onGenerationEndedexists. Main pushes nothing there either, and every fresh read already gets the right answer.Linked Issue
Part of the rebuild of #26867 (root-cause follow-up to #26664).
Visual Proof
N/A for this part on its own: clients do not read the new fields until Part 3, and the host-side change shows up in the session list only when a cleanup write fails. 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-current-work.test.ts(real host, store and SQLite: dead leftovers with failed settlement read as nothing to every host reader in a worktree and a folder workspace; Stop has nothing to stop; a close Orca asked for republishes with no row; a waiting card drains after the generation ends with no journal write in between; a live owner publishes exactly as before),structured-agent-session-rewind-provenance.test.ts(a rewind after a failed cleanup never promotes the dead run's work),agent-session-stop-note-refresh.test.ts(the refresh frame restates the host's current work).Reader changes: queue gate/edit/card-hold tests, send settlement, stopping and Stop-event tests, status feed, conversation-command admission and compaction, outgrown child (main renamed signed-out child), idle sweep, Codex exit convergence, Claude permission request;
activeTurnId()→runningTurn()churn; hand-made journal stubs gainitemFence,wroteBeforeOpenandrunningTurn;publishFence→generationEnded.Merged main
b83a6f74be9through Part 1. Main's history page now reads with a per-subscriber reader, so the published current work moved toagent-session-published-work.tsand is laid over each finished page and frame. Main's new Stop path (the Stop's target, its acceptance, the unconfirmed note, the targeted cancel) and its per-subagent row writer now read what runs through the host's current-work projection, as the rest of this part does; the Stop event's turn comes from that projection too. The current-work wire fields moved toagent-session-current-work-wire.ts, the status feed's deps to their own module, and the revision bump intopublishGenerationEnded, to stay under the line limits main reached.Merged main
fd4a802b3ce(Part 1's squash). Main's Native chat: a message stopped before the agent got it reaches the agent with your next send #27623 (a stopped message reaches the agent with the next send) crossed this part's reads of what runs: the Stop's withdrawal words are now fixed on main, so the Stop acceptance input keeps only the host's live turn; and the stopped messages a handover carries are decided from the host's current-work turn scope, the same one the dispatch row is written with.Merged main
6d519f7835a. Main's feat(native-chat): add a "New" line above unseen messages #28333 (a "New" line above unseen messages) marks a catch-up page that has more pages behind it with an optionalhasNeweron the batch; it sat beside this part's current-work fields on the same event, so the batch type and the subscriber catch-up page keep both.Merged main
1bdbc236480. Main's feat(native-chat): chat scratch folders expire after 30 days unused #28394 (chat scratch folders expire after 30 days unused) gave the idle sweep's two dispatch checks names so its new scratch-expiry veto can reuse them; the sweep keeps this part'sstorereads beside them, and the veto keeps main's own reads.Local at
674dbedd87e:tc:node,tc:web,tc:clipass (on openclaw);src/main/native-chatpasses under Node (404 files, 3737 tests); CI's static checks pass apart from the known local-only mobileno-cycle. The changed-code and type-aware lint passes are left to CI.CI at
674dbedd87e: green, 18 pass and 14 skipped, no reruns. That includes the five test shards, relay integration, static analysis and typecheck, and the test vs non-test line count check against main.Size against main: production +1250/-394, tests +827/-114.
What the reviewer should check
heldStructuredAgentSessionCurrentWorkwith this host's child as evidence. GreprunningTurn(): outside tests only the projection calls it. The readers that still read raw journal facts do so on purpose: whether this process accepted a send (wroteBeforeOpen), cache keys (the cursor), and the "worked for" timer.liveFence). Null when the lease is released; a newer server replaced the run and the claim isn't conflicted; the owner is unreconciled or recovering and this host holds no child for it; or this host saw its root exit at the lease's fence. With no record the fence is 0 and every item counts. Losing contact never ends a run (the SSH rule).recordProviderChildEndandendChildthe child is still on record so its sink can drain the tail. Each send, Stop or answer path runs under the chat's serialize, so none can reach the ended child in that window.publishGenerationEndedre-derives frames, the status feed and settlement waits with no row and without renewing the idle clock;viewKeyincludes "unverifiable".AI Disclosure
Review
Part 1 has merged, so this targets
main. Parts 3 to 5 build on this branch.Agent skill upstream boundary
Notes
Author: Brennan Benson (@BrennanKB5).
Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)