Repository navigation
Admit one provider event's journal writes as one queued operation, decided when it runs - #25141
Conversation
…al timeline folder Pure moves so a shared timeline assembler can use them: Codex's message ordinal counter becomes ProviderTurnMessageOrdinals and Claude's turn-row revision becomes the provider-neutral agent-journal turn-row revision. Only names and import paths change.
…found again after a restart - A sink transition is admitted whole or not at all; its steps run back to back at their turn in the journal's write queue, and each resolver reads the fold with every earlier write landed. A resolver may also say where the row belongs (turn scope, provider reference), and the writer always hears how the transition landed. A resolved lifecycle batch chooses its settlement mutations from the fold at execution. - New optional row field providerItemRef: the provider's own reference for the item a row is, written only where the row's identity cannot spell it (Codex keys messages by their place in the turn and renumbers its item ids on resume). Set by the creating write, kept by revisions, indexed by the journal fold, never read by clients. A downgrade test shows an older host and client render such rows unchanged. - Provider timeline identity schemes (shared legacy arm, Codex) and the join index that resolves a provider item to its row from memory or the fold: ordinals and request incarnations are read back from the rows, so a restart or an evicted entry finds the original row instead of placing a new one.
… joins read from a replaced epoch A fresh join index continued a turn's messages at the first free place, so a journal holding only a later ordinal (an imported or removed earlier row) had its sequence back-filled. The place is now one past the highest ordinal any row or echoed send holds there, read through a pure scheme reader. The join caches also drop what they read when the journal's epoch is replaced.
Keeps both multi-row writes: the transition's per-row write (each built after the one before it folds) and main's all-or-nothing queued-rejection write. Main's ledger receipt rides the shared single-row write. A resolved lifecycle batch cannot carry rejectsQueued, so it is never silently dropped.
…timeline-transitions
…m the transition PR Nothing in production reaches the state they defended (an assembler that lost its memory while its child keeps streaming the same turn), and the stored Codex id was positional. The legacy identity scheme moves to the assembler PR with its first caller; the Codex scheme and any persisted reference wait for Codex to move onto the assembler. The Codex ordinal counter goes back to codex/, since no neutral code imports it.
A settlement too large for one row now commits all its rows or none, through the journal's existing all-or-nothing write, instead of a row-by-row writer. Every row is built before any commits, so a settlement naming one item twice is refused before anything is written.
…d-forget Nothing reads which steps of a transition wrote. A failed step fails the sink, leaving the steps before it written; the header says so, and tests cover it plus a settlement whose second row fails inside the transaction. writeAgentJournalTurnRow returns nothing again, as on main.
…d options A failed step no longer lets the steps after it write: each step checks, at its own turn in the journal's queue, whether the write handed over just ahead of it completed, using the queue's count of completed write bodies (a promise would report the failure only after the next step ran). The sink fails only once every step has had its turn. The item step's `paced` size bypass and the resolver's replacement `options` are removed; nothing planned uses them.
The steps of one event now share one turn in the journal's write queue: a loop writes each in its own transaction through the row writer's synchronous writeRows (split out of enqueueRows) and stops at the first throw. Prefix semantics and "nothing lands between the steps" now hold by construction, so the completed-write counter on the queue, the step gate and the allSettled barrier are gone; the queue is back to main's bytes.
…timeline-transitions
Review summaryThe problem this PR addresses. Structured native chat is getting a shared "timeline assembler" (#25064, stacked on this PR) so that new agents (Grok first, over the Agent Client Protocol) don't each need their own hand-written translator into Orca's chat history (the journal). That assembler needs one building block from the journal layer: a way to accept all the journal writes caused by one agent event as a single unit, with each write decided at the moment it actually runs, against the journal as it stands then. User-facing change: none. Nothing calls the new code yet. The only code that runs differently today is Claude's turn-row writer, which moved to a shared module with identical behaviour. What changed during review. The review found that an earlier version of this PR (and #25064) built a lot of machinery to recover from a situation that cannot happen in production: the assembler losing its memory while the agent process keeps streaming the same turn. On main, an agent process and its translator always live and die together, and the old turn is closed out before a new process's events are written. That machinery included a new persisted row field (
Deferred (low impact, not blocking).
Verified. Four review rounds plus a readiness checklist (no P0/P1/P2). Main's guarantee suites for the touched journal code pass on this branch (57 files / 647 tests in one run), plus the PR's own tests, including tests that fail with the stop-at-first-failure loop removed. CI on the final head Not verified. No live agent, no Windows/Linux run, no headless or SSH runtime; nothing is wired. |
- journal-store.ts imports: main #24576 dropped journalStoreLoadedFields; this branch types appendResolvedItem off JournalItemAppender, so neither it nor JournalResolvedItem is imported. - event-sink-queue: keeps this branch's journalItems; journalStopDecidesTurn takes main #24864's two arguments (openedBy plumbing removed on main).
|
Merged current main into this branch (now
|
|
Earlier CI merges missed main's restored provider-handle import and the test-only parser dependency still needed by old release checkouts. Merged main |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (14)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 2 remain after this review. 📝 WalkthroughWalkthroughThe changes add ordered structured transitions to the agent-session journal. They include resolved lifecycle batching, transactional row writes, and a step writer used by the journal store. The structured sink exposes transition operations, which resolve and append steps before publishing once when applicable. The changes also generalize turn-row revision APIs, add coalescer methods for dirty streams, and add tests for transition ordering, failures, settlement batches, and coalescer behavior. Priority: ⬇️ Low Merge Risk: ⚪ Minimal · up to The transition API is not wired into an agent, and no actionable merge-blocking issue was established. The change is ready for normal merge checks. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to Existing behavior is preserved, and the new capability has no live caller. Risk is limited, but safe recovery from partially completed work still depends on future integrations. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
Hardening Proposals
🚥 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 |
Main squash-merged D1 #24990 (from 320f347) and C1a #25141 (383b0dc). Conflicts: - #25720 dropped the launch-command override gate from native-chat create support and renamed the route input to startsOutsideWorkspaceRoot: main's version, with D3's agent-generic typing and host structured-agents input. - the model catalog store's refresh: D3's agent: string plus main's new lister parameter. - #25654's tool-run header restyle lands in C5's NativeChatToolRunCallCounts, which holds that span.
ELI5
In a structured chat, Orca writes everything an agent does as rows in the chat's history, called the journal: the agent's reply, each tool call, each approval prompt, and one row per turn. Today Claude and Codex each have their own translator that writes those rows. The next PR, #25064 (stacked on this one), will add one shared timeline assembler that writes the rows for new agents, starting with Grok.
That assembler will need something the journal can't give it yet. One event from an agent (for example, "this turn ended") often needs several rows: the turn row, plus closing every tool call still open in it. Writes go through a bounded queue (the event sink) that can refuse work when it is full. If the queue took the first two writes and refused the third, the assembler's memory ("I ended that turn") and the journal (the turn still looks open) would disagree.
This PR adds the small pieces that let one event's writes be accepted or refused as a single unit, with each write decided against the journal at the moment it runs. Nothing uses them yet, so there is no user-visible change.
What Changed
User-facing change: none. Nothing new is wired into an agent, a client, or the runtime.
The mechanism:
A transition: one event's writes as one queued operation (
structured-agent-session-transition.ts, new). The event sink gainstryAppendTransition. All the rows one event writes enter the sink's queue as one operation, so the queue accepts the whole operation or refuses it whole. When it runs, its steps are written back to back, so no other write lands between them. Each step reads the journal as it stands at that moment, including the rows the steps before it just wrote. That way the journal decides what a step writes, not the assembler's memory, which can be stale (another writer, such as a person pressing Stop, may have changed the row since).journal-step-writer.ts, new; the journal'sappendSteps). The journal runs its queued writes one at a time. A transition is one of those writes, and its body is a loop over the steps. Each step does what main's multi-row write already does (check the journal is writable, choose the rows, build them, insert them in their own database transaction, apply them to the in-memory view of the journal); that part is split out ofenqueueRowsaswriteRows, andenqueueRowsnow just queues it, so its existing callers behave exactly as before. The loop stops at the first step that throws and passes that error on. So "the steps after a failure never run" and "no other write lands between the steps" hold because of how the code is shaped, not because a later step infers what happened before it. A write issued while the steps run, even from inside a step's own code, waits until the whole transition is done.journalItems), and the journal gainsitem(id): one row with the turn it belongs to.A settlement chosen when it runs (
journal-lifecycle-batch-appender.ts,journal-store.ts). A settlement is the batch of rows that closes out what a turn or session left open (running tool calls, pending prompts). A settlement step picks those rows from the journal at its turn in the queue, not when it was planned (planResolvedon the settlement writer). A settlement too large for one row is split into consecutive rows, and all of them commit in one database transaction throughwriteRows, the body of main's all-or-nothingenqueueRows(the multi-row write added on main in fix(native-chat): show a message Orca accepted and then failed to deliver as "Not sent" in the chat #24710). Every row is built before any commits, so a settlement that names the same item twice is refused before anything is written. Otherwise the second row would reuse the first row's revision number.Streamed text that a caller writes itself (
agent-session-delta-coalescer.ts). The helper that batches streamed text gainsdirty()(streams with text not written yet) andmarkFlushed(key)(the caller wrote it). The assembler will use them to put pending text inside a transition instead of letting the helper write it separately.The turn-row writer moves to shared code (
claude/claude-turn-row-revision.ts→native-chat/agent-session-timeline/agent-journal-turn-row-revision.ts). This is more than a rename, so here is all of it:agentJournalTurnRowReservedBytesandresolveAgentJournalTurnRowWrite, so a transition step can reuse it.onlyWhileRunning: write only while the turn row is absent or still running, so an ended turn is never reopened. Claude doesn't set it, so Claude's behavior is unchanged.Why
The shared assembler must never act on memory the journal can contradict. Deciding each write against the journal when it runs, and admitting one event's writes as a single unit, gives it exactly that. It needs no new durable state.
What an earlier version of this PR had, and why it is gone. The first version also added a new optional field on journal rows holding the agent's own id for the item (a "provider item reference"). It also added an index that re-found rows by that reference after a restart, per-agent id-spelling schemes, and a counter for message positions within a turn. These defended one situation: the assembler loses its memory while the same agent process keeps writing the same turn. Nothing in production reaches that situation. The assembler lives exactly as long as the agent process it translates. After a restart, a new process starts, and Orca's existing startup cleanup settles whatever the old one left open. The Codex id the field would have stored was also unstable: Codex renumbers its item ids when a conversation resumes. A field persisted in users' data forever, defending an unreachable state with an unstable value, is worse than none. So the field, the index, the schemes, the cross-version test for the field, and the move of Codex's message counter are all removed. If Codex later moves onto the assembler and proves it needs a stored reference, that work adds it with its own evidence. Also removed, because nothing planned uses them: the earlier version's row-by-row settlement writer, a callback reporting which steps wrote, a flag that let a step skip its size check, and a way for a step to replace the row placement it was planned with.
Alternatives considered:
Differences from the common pattern
Linked Issue
N/A (maintainer). Part of the work to give agents beyond Claude and Codex a structured native chat. #25064 (the assembler) is stacked on this PR.
Visual Proof
N/A: no UI or behavior change. Nothing is wired.
Testing
What I verified:
structured-agent-session-transition.test.ts(14 tests): steps land back to back, each reading the one before it; a refused transition writes nothing; a failed last step keeps the steps before it, fails the sink, publishes nothing, and the sink refuses later work; in a three-step transition whose middle step fails, the third step writes nothing, for each of three failures: the middle row is larger than it reserved (the third step's code is never called), the middle settlement names an item twice (the earlier tool call stays running and no next-turn row appears), and the database refuses the middle row; a settlement too large for one row lands as consecutive rows from the journal as it stands; a settlement whose second row's insert fails inside the transaction leaves neither row durable; a settlement naming one item twice is refused before writing; nothing written means nothing announced; a write a step's own code issues lands after the whole transition, not between its steps; a transition waiting behind a chat's not-yet-copied history (the copy runs before the chat's next write, so writes wait in line) still runs its steps back to back, ahead of a write issued after it; in that same waiting case, a failed middle step stops the third, and a write the sink accepted before the failure still lands; the text helper'sdirty/markFlushed.oxlintandoxfmtpass on every changed file. Nomax-linesexception.ff089865ceb(this branch with current main merged in). Its only errors are 5 missingstream-json/stream-chainmodule paths from this machine's stale shared install, in files this PR does not touch. CI is the authority.ff089865ceb: "static analysis and typecheck" passes, including lint, the full typecheck and the localization extraction, catalog and coverage checks. The localization-coverage and editor-test typecheck failures main had earlier on 10-05 are fixed on main and no longer affect this branch.What I did not verify:
AI Disclosure
Review
Agent skill upstream boundary
docs/reference/agent-skill-sharing-upstream-boundary.mdand copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.Notes
Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred). Locally:oxlinton the changed files, the node typecheck above, and the 23 suites above. CI covers the full runs.