Skip to content

perf(native-chat): chat tabs list from saved state, and history opens on first use - #23131

Closed
brennanb2025 wants to merge 115 commits into
mainfrom
brennanb2025/native-chat-lazy-history
Closed

brennanb2025 wants to merge 115 commits into
mainfrom
brennanb2025/native-chat-lazy-history

Conversation

@brennanb2025

@brennanb2025 brennanb2025 commented Sep 26, 2026 •

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 42 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​2096 $\color{#cf222e}{\Huge{\mathbf{−}}}$​568 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​1528
Prod 39 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​1635 $\color{#cf222e}{\Huge{\mathbf{−}}}$​390 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​1245

ELI5

When Orca restarts with many native chats, it used to load every chat's full history before it could even list the chat tabs. With about 250 chats, the window waited seconds for that. Now the tab list comes straight from what Orca already has saved on disk, and a chat's history is loaded the first time something actually needs it. Each chat's status in the sidebar (working, done, the last reply) still appears after a restart. Orca saves a small copy of each idle chat's status, and it only trusts that copy when the chat's history hasn't moved since the copy was saved.

This PR is stacked on #22835 and must merge together with it. It fixes the startup cost of #22835's startup restore: opening every listed chat's history before the tab list answers.

What Changed

Before: session.tabs.listAll, and so the renderer's startup and first paint, waited for the host to run startup reconcile and then open and replay the journal of every listed chat. Measured on a 24-core Windows machine with 247 restored chats (a 1,301-record store), main spends about 10 s in listAll. Roughly 3.6 s of that is opening and replaying journals, about 1.2 s is opening each journal database, and about 3 s is record-store work; that last part is fixed separately by #23115. A chat whose journal could not be read quietly disappeared as a tab.

After:

  • The tab list comes from durable rows. The host publishes chat tabs once, from the store's tab table and the records, as soon as it is installed. listAll no longer waits on reconcile or on any journal. A chat whose journal cannot be read keeps its tab, and its pane says the history couldn't be loaded, without retrying forever.
  • History opens on first use. Reads, sends and reveals open a chat's journal through the existing single open path. A first read never waits on the global startup reconcile.
  • One startup pass after paint derives status, in two phases, one chat at a time with a yield between chats:
    • Phase 1: for each listed chat, a read-only check compares the chat's saved status copy with the journal's current position. If they match, the saved copy is published through the normal replay path. If they don't, the chat is marked for phase 2.
    • Phase 2 runs after reconcile. It resolves records in recovery, settles open conversations that still owe a settlement, and opens the chats phase 1 marked, then settles them.
      • It opens the active worktree's chats first, then other visible worktrees, then the rest.
      • A chat opened only to derive its status is closed again afterwards.
      • A user's read or send of any chat opens it immediately, ahead of the pass.
      • If the startup reconcile fails, the pass retries it after 1, 4 and 15 s. The first later reconcile that succeeds runs phase 2 again for every listed chat that still owes a settlement, so a crashed chat nobody opened can't stay "working". After the last retry, that chat settles on its next attach or send.
    • Both phases take the active worktree's chats first.
    • A crashed chat that the user opens before reconcile finishes shows "working" until reconcile ends and the pass settles it.
  • Saved status per chat. A small SQLite store in the host's state folder keeps each chat's last status while it is idle, or before it has a first turn. Each entry holds a version, the journal position it was derived from, the status projection and the last activity time.
    • Writes are grouped and flushed on a timer and at quit.
    • A corrupt store file is recreated. A store from a newer build is ignored.
    • An entry is never used unless its journal position matches exactly. So a chat that crashed mid-turn always has its history opened and settled at startup, and never shows its old status from before the crash.
    • A failed flush backs off (up to 16 s) instead of retrying every 250 ms, and quit still closes the store if recovery teardown throws.
  • Paint is no longer gated on chat bookkeeping. The renderer's startup no longer waits for chat restore, and the prepare step no longer waits for reconcile.
  • Legacy stores are seeded once, when the store opens, instead of by whichever write happened to run first. The seed is re-applied if another build rewrites the file before this one's first write, so no legacy chat drops out of the tab list.
  • The renderer's chat tab sync still subscribes when the startup tab restore fails, so tabs created later (from a phone, or by an orchestration worker) still appear.

Why

The chat list and each chat's status are small facts. Before this change they could only be read by replaying every chat's full history. Opening histories lazily removes that cost from startup. But listed chats must keep their status rows after a restart, so the status has to be available without opening the history.

The approach here: keep a saved copy of each chat's status, and check it against the history's position at read time. Orca keeps one history file per chat and has no single database transaction spanning chats, so the copy can't be written in the same transaction as the history. Checking the position when reading catches any mismatch, and a mismatch just falls back to opening the history. The saved copy is always re-derivable and never the only source of truth.

This is a deliberate deviation from the usual design, where a list status is written in the same transaction as the log it summarizes. Here the list status is a cursor-validated cache outside the log's transaction, because each chat's journal is its own file. A single app-wide chat database would remove the cache and the startup probe; that is a larger storage change outside this PR.

Alternatives considered:

  • Reading only the tail of each journal at startup. Rejected. A chat's status depends on its whole history (pending prompts, the crash boundary, settlement), and the tail of a large chat costs about as much to read as the whole file.
  • Saving the status with no validation. Rejected, because it would keep showing an old "done" after a crash.
  • Adding a status table inside each journal. Rejected. Startup would still open every journal to read it, so it saves no boot cost. It would still need the position check for writes made by older builds. And it adds a status write inside the journal's fully-synced transaction on every append.

Linked Issue

No ticket. Found by profiling native-chat startup with about 250 restored chats.

Visual Proof

Live QA on a macOS machine with 14 native Claude chats across two projects, restarting the app between steps.

After a restart, every chat tab comes back and every chat shows its status in the sidebar before any chat is opened:

1-tabs-restored-sidebar-14-done.jpg

Second boot: all 14 statuses come from saved copies (14 from saved copies, 0 to open), and no chat history is open when the window paints:

2-second-boot-first-paint-no-chat-opened.jpg

A chat whose history file is unreadable keeps its tab and says it couldn't load, with one open attempt and no retry loop. Plain wording for the message is coming separately (see Notes):

4-unreadable-pane.jpg

Before, on #22835, the same chat's history error also failed the whole session restore:

4-BASE-unreadable-restore-failed-toast.jpg

A chat killed mid-turn is settled at startup with no user action and never shows its pre-crash status. The settled turn still displays as "done" instead of interrupted, the same as on #22835; #23456 fixes that:

3-crash-pane-after-open.jpg

Testing

  • Tests: new or changed tests, each paired with a deletion ablation that turns it red. They cover:

    • the tab list: from the table and records, never waiting on reconcile or journals, and an unloadable journal stays listed;
    • the startup pass: phase order, tiers, user reads jumping the queue, closing after status-only opens, settlement of flagged chats with no user action;
    • saved status: an exact match seeds with no open; a chat that crashed mid-turn is a miss, is opened once and settled, and never republishes its pre-crash status; a version bump is a miss; a model switched while idle shows through; seeds never rename, notify or withdraw a restart offer; the store recovers from corruption; writes are grouped;
    • the read-only position check: it never uses immutable or no-lock reads, it sees an uncheckpointed WAL, and the journal bytes stay identical;
    • paint gating and renderer saves: a save between paint and listAll loses no chat tab layout; each host answer waits for the tab publication;
    • a failed startup reconcile: the retry recovers with no attach, the first later clean reconcile settles a crashed chat nobody opened, and retries stop at quit;
    • the cross-version wire test.
  • Full run on the final head: 1,273 files and 12,175 tests pass. One test fails: release-checkout.unit.test.ts times out in this environment. It covers release tooling this PR doesn't touch.

  • Live measurement: a 24-core Windows machine; a seed profile with 247 restored chats in a 1,301-record store. Compared against fix(native-chat): the conversation outlives its agent #22835 at this PR's merge base. Every run restored all 247 chats.

    • Cold first boot after upgrade (no saved statuses yet, the worst case), one run per arm with the status count recorded in the page on every change:

      fix(native-chat): the conversation outlives its agent #22835 this PR
      Chat tab list (listAll) 9.63 s 0.46 s
      Window paints 29.49 s 21.71 s (7.8 s sooner)
      Every listed chat has its sidebar status 25.71 s 24.81 s
      Statuses already shown at paint 247 94

      With this PR the rows fill in chat by chat after paint (25/50/75/100% at +0, +0.5, +1.6 and +3.1 s), at about 45-60 chats per second. Both arms publish each chat's status as its journal opens; journal open to renderer row is 26/56 ms p50/p90 here and 29/70 ms on fix(native-chat): the conversation outlives its agent #22835.

    • First boot, three runs per arm (medians): listAll 10.62 s → 0.46 s; paint 26.45 s → 19.98 s; worst main-process stall 800-1104 ms → 420-638 ms. The terminal input-echo stall was no worse: max 396-541 ms vs 392 ms, p99 44-61 ms vs 81 ms.

    • Second boot of the same profile (saved statuses reused): phase 1 took 353 ms, with 246 chats served from saved copies and 0 left to open (the 247th is the active chat, which its own first read opens). The whole startup pass took 0.9 s and listAll 0.46 s, and all 247 rows were present at paint. One journal was opened, against 247 on the first boot.

    • Crash (a forced kill of the app mid-turn), one run per arm: listAll 11.69 s → 0.43 s and paint 29.16 s → 23.53 s. Every chat had its status at paint in both. The crashed chat was settled during the startup pass with no user action, and a chat closed before the crash stayed closed.

    • The fill-in opens one chat at a time on purpose. Opening several at once would finish slightly sooner but block the main process for longer.

  • I manually tested these changes locally

  • Automated tests added/updated, or explained why not below

AI Disclosure

Review

Agent skill upstream boundary

  • Not applicable, or this change follows docs/reference/agent-skill-sharing-upstream-boundary.md and copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.

Notes

Checklist

  • This PR is small and focused
  • I explained what changed and why (ELI5, the user-facing before/after, the mechanism, and why over the alternatives)
  • Before/after screenshots or videos attached for UI changes, or N/A with reason
  • Self-reviewed for correctness, security, and performance
  • Cross-platform, SSH/remote, and path/shortcut impact considered (or N/A)
  • pnpm lint, pnpm typecheck, pnpm test, and pnpm build pass (or CI will cover; local preferred)

brennanb2025 and others added 30 commits September 24, 2026 23:36
No client ever called agentSession.requestHandoff or mounted the handoff
chrome. Delete the handoff coordinator, the terminal-owner runtime, the
proof write path and the unmounted UI. Keep agentSession.handoffStatus,
which released desktop clients read for worktree activation, and let
records an older build left mid handoff reconcile through the ordinary
restart and recovery paths.
Eviction now drains delivered events before quit's resume-offer snapshot. An
unbounded wait there sits ahead of the provider stop, so a sink whose journal
write stalls kept the child running until the step deadline aborted the
eviction. The offer is advisory: bound the drain and stop the child regardless.

Co-Authored-By: Claude <noreply@anthropic.com>
`claudeAuthEnvCarriedForward`, `isPathWithinDirectory` and
`queryWindowsProcessRowsFresh` lost their last caller with the handoff. The
fresh-scan tests now go through `queryWindowsProcessDescendants({ fresh: true })`,
the teardown path that still depends on that contract.

Co-Authored-By: Claude <noreply@anthropic.com>
Six comments still named the handoff coordinator, a handoff suspend, or a
terminal-owned session as live participants in the flows they describe.

Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
…ement

The removed restart handoff test pinned this branch; nothing else did.

Co-Authored-By: Claude <noreply@anthropic.com>
The handoff removal dropped the per-session queue from `handoffStatus`, so a
read landing mid-start reported the reservation (no owner) instead of the
settled chat owner, and shipped desktop clients blocked worktree activation on
it. The read is queued again, as it was before the removal.

Co-Authored-By: Claude <noreply@anthropic.com>
The gate only refused a write when a PTY had been bound to a chat session, and the
only code that ever bound one was the terminal handoff this branch removes. With it
gone, every admit/readmit returned "admitted" unconditionally, so the checks on the
renderer write path, the runtime controller backstop, terminal.send, agent prompts,
preview input and orchestration pointers, the refusal fields on terminal.send and
worker-start receipts, the plugin and CLI refusal copy, and the adopted-pane
orchestration routing could no longer run. Ordinary writes take the same path in
the same order as before.

Co-Authored-By: Claude <noreply@anthropic.com>
…alled

appendLegacyTranscriptMessages fed the terminal transcript catch-up and
proveClaudeTranscriptBranch backed the terminal owner's exit proof. Both lost
their last caller with the handoff. Their tests now go through the live entry
points instead: the roster bounds through the legacy import, the pinned-read and
growth tests through the ancestry replay the history window uses, and the marker
rules through the string proof in their own file rather than the session-file
resolver's.

Co-Authored-By: Claude <noreply@anthropic.com>
A send refused because the chat's owner is not settled showed "The session is
mid-handoff (<stage>)." in the composer. With the handoff gone, the stages that
reach it are a chat that is still starting, or one whose previous agent process
has not yet been confirmed stopped. The message now says which of the two it is.
The refusal code is unchanged.

Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
With the terminal handoff gone, the module named codex-tui-rollout-proof holds
only the pinned rollout lookup that structured Codex launches use to resume a
thread, so the name described code that no longer exists. Rename the module and
its options type. Also drop a mobile allowlist assertion that pinned the
removed agentSession.requestHandoff method, which no longer exists to allow.
The handoffStatus reply type still listed the terminal handoff's fields and
states (terminal placement, host label, proof retry, queued and waiting phases,
the to-terminal direction). No host writes them any more and the only client
reader parses the reply as unknown, so they described nothing. The reply on the
wire is unchanged.
…t decode

Nothing in this build writes a terminal owner (`runtimeKind: 'tui'`) or the
handoff's `preparing` / `old-owner-stopped` stages, but the in-memory types
still admitted them, so readers across the host kept branches for values no
path produces and the compiler could not point at them.

The store now validates the on-disk shape, which still accepts those values so
an older record is not quarantined, and maps them once while parsing:

- `preparing` and `old-owner-stopped` become `recovering`
- a `tui` lease becomes `native`; when it records a process it also becomes
  `conflicted`, the claim every build probes but never stops. A plain native
  owner would be stopped by restart recovery, here and in older builds.

Revisions are taken over the normalized state on both sides of every compare,
and the mapped record reaches disk with the store's first transaction, the
same way the tab-id backfill does.

The in-memory types narrow to what this build writes, and the branches that
existed only for the removed values go. Structured-worker identity keeps its
verdict for a former terminal owner by refusing a conflicted claim rather
than a non-native kind.
…ation

A reservation only ever names a native owner now, so the request no longer
carries a kind and the reserved lease records `native` directly. The attach
params keep `runtimeKind`: agentSession.ensure and create accept it, and the
operation fingerprint stored in the ledger covers it.
…at changes nothing else

Hiding a tab also committed the visibility index, so the no-op transaction
wrote the file even when its open-time revision was wrong. Committing the index
first leaves the pending rewrite as the only reason to write.
…ration

A write carried the fence of the last frame the pane read, and the host refused it
unless that fence was still current. An idle release and the restart after it each
move the fence, and the release publishes nothing, so a send after a release was
refused "Expected runtime fence 1; the session is at 3", and a Stop queued behind a
cold start was refused as stale.

Every write already names what it acts on: a send its conversation, a cancel its
turn, a prompt answer its item revision, a rewind its epoch; an option is
last-writer-wins. So admission stops comparing the client's fence, and the rebase
that papered over one restart (admitAtResumedFence, resumedFromFence) goes with it.
The writer-lease check stays, and so does the attach's compare-and-swap.

Frames now stamp the fence read when each frame is sent instead of a copy each
subscriber kept, which went stale on the same release.
A journal write and its delivery to open readers were two calls, and some
writers made only the first. A failed start whose lease could not be handed
back, a provider revision with no frame behind it, and eviction's settlement
were all journaled without reaching an open chat.

A journal handle now reports every durable change, and the host's session map
binds that report to the session's readers when the handle is set. Writers no
longer publish what they append; the per-writer publish calls are deleted.
…ackfill cannot supply its rewrite

The seeded record had no surface tab id, so the next open backfilled one and
that rewrite alone made the no-op transaction write. The test passed with the
legacy-lease rewrite signal removed.
The handoff removal deleted it alongside the terminal-owner tests, but it
covers the surfaced-PTY block that still guards resume, including an agent
whose ownership is unknown.
Each commit now delivers itself, so the publish a provider frame still sends
afterwards found every reader caught up but still read rows and rebuilt the
timeline for each one. A caught-up reader now skips the read.
# Conflicts:
#	src/main/native-chat/agent-session-wire/structured-agent-session-subscribers.ts
A send to a chat with no running agent restarted the agent inside the send
call, before the message was recorded, so the client waited for the whole
start and a failed restart refused the message. Claude held prompts sent
during startup, and those could settle as "unconfirmed".

A send is now accepted inside the session's serialized queue: one ledger row
and one submission row marked handoverRecorded, published, answered pending.
A per-session delivery loop exists while a message is queued. It starts the
agent through the same serialized attach a hold uses, waits outside the queue
for a Claude child to prove its start, and hands the oldest queued message
over as its own serialized step, writing dispatch{pending} before the adapter
call. A start it needed and did not get writes one error-tone row and rejects
every queued message with the same words; a start Stop cancelled writes none.

Settlement follows from the rows. A queued message is provably unwritten, so a
close, an eviction or an exit rejects it. A handed-over message stays in doubt.
A queued row at or below the sequence a handle found when it opened was left
by an earlier process and is rejected at open, with no latch. Stop withdraws
queued messages with no writer lease and no fence. An attach failure keeps the
conversation open, and the attach adopts its journal. Owed work counts the
loop and queued rows.

A compaction or rewind found prepared when a conversation opens was started
under a child this process no longer has, so the open settles it rather than
leaving it to refuse every send until a view attaches. The open cursor is
scoped to its epoch, because sequences restart when an epoch is replaced.

Deleted: restart-before-admission, recordFailedRestart, the fence rebase,
Claude's startup gate, the attach's forget on failure and its own crash
boundary. Clients without agent-session.accepted-send.v1 get their reply held
until the handover; the desktop and paired desktop lists advertise it.
A child that proved its start and then exited before its message was handed
over left the message queued: the exit settlement returned early when nothing
else was in flight. Delivery then started another child for it, and a child
that died the same way started another, without end and without a row.

A retried settlement for an earlier generation, run by the attach that
delivery started, did the opposite: with that generation's turn unfinished it
rejected the message queued for the child being attached.

The settlement now takes the rejection for queued messages from its caller.
The unexpected exit and the eviction pass one, and it applies even with no
other work in flight; the retry for an earlier generation passes none.
Routing, group addressing, worker-show and the idle sweep each composed their own reading of
whether orchestration still holds a structured worker, so each new obligation or retirement state
had to be added to every reader. structured-worker-custody now derives both answers from the
worker-terminal list state coordinators see in worker-list: addressable is owned and not released,
and owed work is an active custody or an unsettled task dispatched to the same incarnation. The
owner's state is read through the remote dispatch attachment too, as the terminal transfer lookup
already does. Behaviour is unchanged; a settled worker awaiting its coordinator still rests.
…utlives-agent' into brennanb2025/chat-conversation-outlives-agent

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ️ No critical issues — one edge-case note inline.

Reviewed changes — read the complete diff at 30dabd9 (77 files).

  • Durable tab listing: chat tabs are built from the record store and tab table (listPersistedStructuredAgentSessionTabs) rather than any journal open; a chat whose journal cannot be read keeps its tab and explains itself there. restoreStructuredAgentSessionTabsOnce awaits host install, kicks the startup pass without awaiting it, then publishes every listed tab synchronously in one store write per workspace via the extracted appendStructuredAgentSessionTabs.
  • One startup pass (structured-agent-session-restart-restore.ts): phase 1 seeds each listed chat's status from a saved copy proven current; then reconcile, recovery exits, settle open conversations, then phase 2 opens and settles every miss and every listed record owing settlement (active workspace first), closing the opens that only the pass wanted. Each step runs under its session lock with a yield between chats, and lifetime.startupStep closes a status-only open unless a reader reached it.
  • Saved status store (agent-session-saved-status.db): idle/no-turn projections keyed by the journal cursor they were computed at, coalesced writes, corrupt-file recreation, newer-build file ignored, version pinned by a snapshot test.
  • Journal cursor probe: read-only, WAL-aware max(seq) read with a repair-marker guard, used to prove a saved copy current without opening the journal.
  • Paint ungated: IPC bootstrap and renderer startup hydration no longer await chat bookkeeping; session.tabs.* answers still wait for the one-shot tab publication, and the tab table is seeded from the saved workspace session at store open.
  • New refusal and worker-release ordering: agent_session_journal_unreadable is final for the renderer read transport, and a structured worker release installs the host and reconciles restart leases before observation.

ℹ️ First boot after upgrade still pays the pass, just off the critical path

With no saved copies yet, every listed chat is a phase-2 miss, so the pass opens and settles each one, one at a time, after paint. That is the intended trade, but on the 247-chat profile the PR profiled this is still hundreds of journal opens — worth confirming the one-time cost is acceptable there. Separately, structured-worker-release-host.ts records a deliberate follow-up: a restarted runtime is not rebound to a structured worker's redrive subscription, so a worker that survives a restart has its parked mail wait for the next arrival.

Pullfrog  | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

if (!epoch || pendingJournalRepairSequence(db, sessionId, epoch) !== null) {
return null
}
const newest = db.prepare(SELECT_NEWEST_SEQUENCE).get(sessionId, epoch)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The probe returns the raw max(seq) for the live epoch, but the open's cursor is the reducer's applied lastSequence, which stops at the first unparseable row or sequence gap and then deletes the suffix and re-anchors the epoch (journal-open.ts:72-108). A row corrupted in place at or below the saved tip (same seq) can therefore match the saved cursor and skip the open — the probe's own C15 test already asserts it counts rows a real replay rejects. The consequence is bounded (the chat shows its pre-corruption settled status until first read, and that open repairs then), but the "position matches exactly" invariant holds for the raw tip, not the replay-applied position.

Technical details
# `probeJournalCursor` is not replay-equivalent

## Affected sites
- `src/main/native-chat/agent-session-journal/journal-cursor-probe.ts:42` — returns `max(seq)` over `journal_rows` for the epoch as the cursor.
- `src/main/native-chat/agent-session-journal/journal-open.ts:72-108` — replay breaks at the first unparseable row (`truncateFrom = entry.seq`) or gap, so `state.lastSequence` can sit below `max(seq)`.
- `src/main/native-chat/agent-session-wire/structured-agent-session-restart-restore.ts:156-162` — a matching cursor seeds and adds the chat to neither `misses` nor `owed`, so the open that would repair is deferred.

## Required outcome
- A saved copy is used only when the journal's replay-applied position equals it, or the difference is provably harmless.

## Suggested approach (optional)
- Cheapest partial guard: require `count(*) === max(seq)` (catches gaps and deleted rows) and that the tip row parses. That still misses a malformed middle row with an intact `seq`, which needs a bounded replay; if that cost is unacceptable, a one-line note on the probe saying it trusts the raw tip and defers repair to first read would make the contract honest.

## Open questions for the human (optional)
- Is an in-place-corrupted journal row (same `seq`) considered reachable enough to guard, given SQLite WAL checksums? The repair subsystem implies the authors treat corruption as reachable.

…utlives-agent' into brennanb2025/native-chat-lazy-history

# Conflicts:
#	src/main/runtime/structured-worker-authority.ts
…writer rewrites the file before the first write
…ed chat, and the pass closes what it opened by the idle sweep's rule

The first clean reconcile after a failed startup one, from a bounded retry or from any attach,
reruns the startup pass's post-reconcile half once, so a crashed chat nobody opened leaves
"working". The per-step reader flag is gone: the pass closes a conversation it opened when the
idle sweep would.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ️ No new issues in the latest commits — the two follow-up fixes and the updated restart test are correct.

Reviewed changes — read the complete diff at ef47c7d and the delta since the prior pullfrog review at 21e02a4.

  • Legacy tab table re-seeded on reload. AgentSessionRecordStore.open now hands a normalizeLoaded callback to AgentSessionStoreTransactionQueue, which re-runs seedAgentSessionTabTable whenever another writer replaces a file that still carries no tab index — so an outgoing build's mid-restart write can no longer drop the seeded tabs and leave only the one the next write touches.
  • Corrupt saved-status file recreated on flush. AgentSessionSavedStatusStore.flush only issues ROLLBACK when SQLite still reports an open transaction, so a corrupt page that SQLite already rolled back no longer masks the corruption, and the flush path deletes and recreates the file instead of failing every flush. Contention (SQLITE_BUSY/LOCKED) is still treated as a skip, not corruption, so a held lock never triggers that delete.
  • Restart status test follows the new pass. The owner-status activation test now drives restoreStartupSessions (after listing the tab it wants reached) and asserts the status-only open is closed and the lease released, replacing the removed restoreReadableSessions path.

Pullfrog  | Fix it ➔ | View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

…s incarnation

A supervised worker's own dispatch context stays open exactly while the worker is active, so the
separate active-custody branch only repeated it. Owed work is now one fact, which also states the
policy that a worker awaiting its coordinator's decision may rest, and both custody decisions are
written once at the top of the module.
… their id

The offer recorded each continuation id in a list on its capsule entry, capped at 16, and a running
action's id in memory. Both could disagree with the journal: past the cap an old rejected
continuation read as the chat moving on, and a crash during a retry restored the failure's older
entry, which lacked the retry's id. Each continuation id now carries a tag derived from the offer
(its teardown and chat), then the action's own part, so any continuation of this offer, queued or
rejected, is recognised from the journal row and the marker alone. The persisted list, its cap and
the in-memory action map are deleted; the agent-start withdrawal keeps an offer whose own
continuation the start was for, read against the stored marker.
…utlives-agent' into brennanb2025/native-chat-lazy-history

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ️ No new issues in the incremental delta — the reconcile-retry, flush-backoff, and teardown fixes are correct.

Reviewed changes — the six commits since the prior pullfrog review (ef47c7d → 31c0297), re-checked against the full diff at 31c0297.

  • Retried a failed startup reconcile. The pass factors its post-reconcile half into settleAfterReconcile(reconciled, misses): a failed reconcile still opens and publishes the misses, then retries reconcile (1s/4s/15s, tests override), and the first clean one runs recovery exits and settles anything newly owed (settleAfterFailedStartup, at most once). onReconciled covers a clean reconcile landing during or after that half; disposed stops retries after quit begins.
  • Closed a status-only open by the idle sweep's rule. The per-step reader flag is gone; startupStep now receives close() and drops a conversation only when it has no child, no owed wind-down and nothing queued or pending — the same predicate the sweep uses.
  • Backed off a saved-status flush that keeps failing. Retries wait 250 ms × 2^failures (cap 16 s) and the counter resets on success; a busy/locked database is still a skip, not a recreate.
  • Closed the saved-status store when the first recovery drain fails. Teardown flush-closes it before rethrowing, so statuses already settled during teardown reach disk.
  • Tightened the crashed-turn tests. They wait for the settled status and the cleared settlementRetryRequired together, and assert the settlement the code produces (turnOutcome undefined, turn states ['unverifiable']) through the host's typed test seam. The commit records the dead-generation crash verdict as a follow-up.

Verified with pnpm vitest run over the four touched suites (startup-reconcile-retry, startup-settlement, saved-status-store, runtime-teardown): 4 files, 21 tests pass.

Pullfrog  | Fix it ➔ | View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

…tab restore failed

Startup no longer waits on the tab restore, so a failed one no longer stops the app; it reached
only the sync, which then never subscribed and missed every host tab change for the run. The
subscription's own retry now re-asks the host instead.
The startup pass ordered only its opens by workspace; the saved-copy check that publishes most rows
ran in index order, so with hundreds of chats the workspace on screen could be the last to fill in.
…on a loaded runner

L19 and the startup-reconcile retry tests waited 1 s for a settlement a clean reconcile starts but
does not await; L19 missed that window twice under load.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ️ No new issues in the incremental delta — the two behavioral fixes and the test-only timeout bump are correct.

Reviewed changes — the three commits since the prior pullfrog review (d09d941 → efefb80), re-checked against the full diff at efefb80.

  • Ordered the saved-copy check by workspace priority. runOnce now walks the listed chats through a shared inOrder(...) (active workspace first, then the other visible ones, then the rest), so phase 1's seeded status rows publish for the on-screen workspace first instead of only the phase-2 opens being ordered. New test pins the seed order.
  • Kept the chat tab sync subscribing after a failed startup restore. startLocalStructuredSessionTabsSync catches and logs restoreLocalStructuredSessionTabsOnce's rejection instead of aborting, so subscribeAll still runs; the stream's initial snapshots inventory and the subscribe retry recover the tab list. New happy-dom test covers it.
  • Gave the unawaited post-reconcile settlement room. Test-only: the reconcile-retry and settlement suites wait up to 10 s for the settlement a clean reconcile starts but does not await.

Pullfrog  | Fix it ➔ | View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

@brennanb2025

Copy link
Copy Markdown
Contributor Author

Review status: paused, not ready to merge

State: head efefb803ed. Two review loops have run, and every fix they found is pushed. The review is now paused while the storage layout for chat histories is decided.

Why paused: #22835's latest head brings in #22820, which settles a crashed turn when a chat is opened and removes the settlement flag this PR's startup pass relies on. That merge means porting the pass's settle step, and the pending storage decision (one history file per chat, or one app-wide database) could make most of that step unnecessary. The merge and port will follow that decision.

Fixed in this review:

  • A store written before the tab table existed could lose every chat tab but one if another build rewrote the file before this build's first write. The seed is now re-applied on reload.
  • If the startup reconcile failed, a crashed chat nobody opened could stay "working" until its next send. The reconcile is now retried after 1, 4 and 15 s, and the first later reconcile that succeeds settles every listed chat that still owes it.
  • A saved-status file that SQLite had already found corrupt was never recreated, so every flush failed for the rest of the run. It is now recreated, and a flush that keeps failing backs off.
  • If the startup tab restore failed, the renderer's chat tab sync never subscribed and missed later tab changes. It now subscribes anyway.
  • Saved statuses now publish for the active workspace first.
  • The startup pass now closes a chat it opened only to read status through the same closability rule as the idle sweep, which replaced a separate tracking path.
  • A test that called a removed method (outside the typecheck's reach) now runs the startup pass.
  • Each fix has a test that fails without it.

Live QA (macOS, 14 chats, screenshots in the description):

  • Every tab comes back after a restart.
  • Status shows before any chat is opened.
  • The second boot opens no history.
  • A chat closed before the restart stays closed.
  • A chat opened while the startup pass is running is not closed under the user.
  • An unreadable history keeps its tab and does not retry.
  • Terminal typing shows no delay during the pass.

Live measurement (Windows, 247 chats):

Known issues, not caused by this PR:

Still open before merge:

@brennanb2025

Copy link
Copy Markdown
Contributor Author

Closing in favour of #24233, which solves the same problem on current main.

This PR made native-chat startup skip replaying every chat's history by keeping a separate saved-status file, checked against each chat's history file when read. That design only made sense while every chat had its own history file. Main has since moved every chat into one database per host (#23613, #24006), so #24233 stores each chat's status in that database, in the same transaction as the history it describes. No second file and no read-time check are needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant