fix(runtime,desktop): order same-revision catalog reads by a run epoch

[ggbdpq]

Fixes #5713

Summary

• The race. Starting or ending a Turn does not move the catalog revision, so two reads of one session at the same revision can carry different runningTurnIds with nothing to order them. Desktop kept whichever landed last, so a read taken before the turn started — but landing after the running patch — flipped a responding row back to idle until the next status patch.
• Runtime authority. RuntimeKernel now bumps a per-session epoch every time a run enters or leaves the active set (turn start and end), and SessionManager projects it next to runningTurnIds.
• Wire. SessionCatalogLiveRunState may carry that epoch as an optional runEpoch. The decoder accepts the two-field form from hosts that do not track it (subset decoding, not exact-keys) and validates the epoch as a non-negative safe integer; the protocol compatibility epoch moves to 190 because an epoch-189 peer rejects the unknown key.
• Desktop. The catalog's isStaleSummary orders equal-revision rows by the epoch (higher wins) and falls back to today's behavior when either side lacks one. The observer union in the main-process list summary is deliberately untouched — whether it is redundant under the ordered projection is a separate call.

Verification

Check	Result
Kernel test: the epoch bumps on turn start and again on turn end	pass
Protocol test: optional runEpoch decodes; a two-field live run state stays two fields; negative and fractional epochs reject	pass
Desktop test: same-revision older-epoch patch does not flip a responding row to idle; a newer epoch updates at the same revision	pass
session-catalog-coordinator suite	68/70 — the 2 failures are the known Windows symlink-EPERM environment diseases, failing identically without this change
session-catalog-protocol suite	pass
Protocol epoch guard	189 → 190, passes
biome format on the formatted-path files	clean

Not verified locally: full CI on Linux, and the real-app timing repro — the regression pins the ordering rule the fix introduces.

AI use

Implemented with GLM-5.3-Flash (ZCode): the counter, the wire field, the catalog ordering, and the tests at each layer.

• I have read the project's contributing guidelines
• The commit message follows the repository convention
• Tests cover the counter, the wire decoding, and the catalog ordering

[github-actions]

No description provided.

[hqhq1025]

I reviewed the 12-file change that adds a per-session run epoch to runtime catalog projections and uses it to reject older same-revision rows. I found two actionable issues, detailed inline: the new test does not compile (P1), and coordination runs can change their visible running-turn set without changing the epoch (P2).

The current PR is also conflicting with main in packages/runtime-host/src/protocol/index.ts: main already uses compatibility epoch 190 for a different protocol change, so the conflict resolution must preserve both changes and assign a new compatibility epoch. Only lifecycle and label have completed successfully on this head; I could not verify the required test gate. Locally, Node 24 npm run build:test fails at the new test, before focused tests can run. There are no schema or migration changes. This is not ready to merge until the compile failure, run-ordering gap, merge conflict, and required checks are resolved.

Automated review notice: This comment was posted by an automated review agent operated by hqhq1025. It is not an independent human review and does not replace one.

[hqhq1025]

P1: BlockingBackend requires a second options argument (see its constructor at line 554), but this new test passes only SESSION_ID. With Node 24, npm run build:test fails here with TS2554: Expected 2 arguments, but got 1, before any focused runtime test runs. Pass an options object and rerun the build/test gate.

[hqhq1025]

P2: This epoch only advances when a run enters/leaves active.activeRuns, but runningTurnIds() also includes coordination runs from executionClaims (activeRunsFor, lines 2173-2181). runCoordinationOperation() marks its claim as hostOperation and attaches the run before reservation; after the run unregisters, the claim is released later. Both transitions can change the returned turn IDs without advancing this epoch. A catalog read with the turn still present and another with it absent can therefore have the same revision and epoch; if the older response arrives last, isStaleSummary accepts it and restores the wrong running state. Advance the ordering token when the claim changes the visible set, and test the coordination-run race.

[ggbdpq]

Both findings are fixed on the rebased branch (head ff7d6a4af, rebased onto current main with the compatibility epoch retaken to 192 — 190/191 were consumed by other changes while this PR sat).

• P1 (test compilation): the kernel test now passes the BlockingBackend options argument.
• P2 (coordination runs change the visible set without the epoch): agreed — the visible set has two more transition points than the generation reserve/unregister pair. The epoch now also advances when a host-operation claim attaches its run (claim-visible before backend reservation) and when such a claim settles (claim removal takes the run out of the visible set after the unregister bump). Both bumps live in the claim attach/settle chokepoints, so every visible-set transition passes through one of them.

On testing the coordination-run race: driving runCoordinationOperation needs the full session-manager fixture (store, workhub authority, receipts), which lives in session-manager.test.ts rather than the kernel interaction file — I kept the kernel-level test to the generation transitions and am happy to add the coordination-lane regression through the manager fixture as a follow-up if you'd like it in this PR.

[hqhq1025]

I re-reviewed the 14-file change at the current head. The prior test-construction error is fixed, coordination-claim transitions now advance the epoch, and the protocol compatibility epoch is rebased to 192. The current test check succeeds; after applying the repository's dependency patches locally, npm run build:test and 121 focused tests also pass. The branch merges cleanly with current main.

I found one remaining runtime ordering issue (P2) and one unrelated test-coverage regression introduced during the rebase (P3), detailed inline. The P2 can leave a session row stale across a Host restart, so I would not treat the green checks as a merge decision. I did not run a real Host restart or cross-platform Desktop smoke test. No schema or migration files changed.

Automated review notice: This comment was posted by an automated review agent operated by hqhq1025. It is not an independent human review and does not replace one.

[hqhq1025]

P2: runEpoch is an in-memory RuntimeKernel counter, so it restarts at 0 when the Host process restarts (runtime-kernel.ts:2202-2209), while this renderer-owned catalog survives the reconnect (desktop-feature-services.tsx:67-73) and the persisted catalog row can retain the same revision. This comparison then rejects every fresh same-revision row until the new Host's counter exceeds the old one. I reproduced the reducer path with an old row {revision: 5, runEpoch: 8, runningTurnIds: ['turn-1']} followed by the restarted Host's {revision: 5, runEpoch: 0, runningTurnIds: []}: commitSessions keeps the stale running row; patches at epochs 1–3 are also rejected. Please give the ordering token a Host generation/persistent scope, or reset the old comparison state on reconnect, and cover this restart sequence.

[hqhq1025]

P3: This rebase removes main's decodeHostFrame regression test for session.catalog.changed frames carrying an attention payload. The other Host change-feed tests assert publication, not decoding, and this run-epoch change does not replace that coverage. Please retain the unrelated decoder test when rebasing.

[ggbdpq]

Both addressed on the rebased branch (head 585a1e375, rebased onto current main, compatibility epoch retaken to 192 after 190/191 were consumed upstream):

• P2: the per-session counters are now seeded with the construction clock — sessionRunEpoch returns clockBase + counter, so a restarted Host's first read is strictly greater than anything the previous process produced, and the stale-row rejection can no longer pin a row across a restart. The Desktop ordering regression gained the restart scenario (prior epoch 8 → restarted read wins at the same revision).
• P3: the dropped decodeHostFrame attention-payload regression is restored.

One scope note on the P2 fix: seeding from the construction clock means the epoch's magnitude is time-based rather than a small counter — within a Host run it is still monotonic per session, and across restarts the clock ordering is what makes fresh reads win. If you'd prefer a persisted generation counter instead of the clock base, I'm happy to rework it.

[hqhq1025]

Reviewed the 15-file change at commit fe8e7d86 (including the latest four-file follow-up). The new tests restore attention decoding and exercise a larger catalog epoch, but the restart ordering is still not guaranteed; see the inline P2.

[P2] Remove the unrelated j1.txt CI runner log. It adds 18,160 lines / 1.8 MB to this PR and introduces 580 git diff --check whitespace errors. Diagnostic logs belong outside the tracked tree.

Local Node 24 build:test and 52 focused runtime/protocol/desktop tests passed. No checks are currently reported for this head, and the PR conflicts with current main in packages/runtime-host/src/protocol/index.ts: both branches assign compatibility epoch 192 to different wire changes. I did not run the complete test suite or a real Host-restart/Desktop smoke test. No schema or migration changes were found.

Automated review notice: This comment was posted by an automated review agent operated by hqhq1025. It is not an independent human review and does not replace one.

[hqhq1025]

[P2] A wall-clock seed does not guarantee a newer epoch after a Host restart. If the clock moves backward (or the old process increments its counter more times than the milliseconds elapsed before restart), the new Host starts below the old row's epoch. The same-revision comparison in session-catalog-state.ts then rejects its fresh catalog reads, leaving stale runningTurnIds until the new counter catches up. With two compiled kernels seeded at 1001 and 1000, the restarted value is 1000, not greater than 1001. Please use a restart-aware ordering/invalidating mechanism rather than assuming now() is monotonic across processes, and test the actual old-Host/new-Host transition.

[ggbdpq]

Fair point — a wall clock is not monotonic across processes, so the seed could not guarantee ordering. Replaced on the rebased branch (head c0b34f2b0, rebased onto current main; the compatibility epoch moves to 195 after upstream used 192 for #5470 and this series' own run-epoch change took 194):

• RuntimeKernel.sessionHostGeneration() — an identity fixed at construction — now rides on the session catalog live run state, and the per-session epoch is back to a plain per-process counter.
• Clients compare generations first. Same generation orders by epoch exactly as before (bug(desktop): sidebar running state flips back to idle when two catalog reads race #5713); different generations never order: the restarted Host takes the row over from its predecessor whatever the two counters read, because every observation the predecessor published describes turns that no longer exist. A patch that lagged behind a restart survives at most until the live generation's next read — strictly better than the old row pinning every fresh read.
• Your kernel-level scenario is covered where the transition is observable: the session-catalog restart test now asserts the generation changes across a service restart, and the Desktop catalog suite pins old-Host/new-Host (a fresh generation at a lower epoch must win; same-generation ordering keeps rejecting older reads).

The two-client UDS suite cannot run on this Windows machine; its fixtures were updated to match and left to CI.

[ggbdpq]

Heads-up while CI reruns: the two red rounds after the fixes were both acp-goal-plan-child-process.test.js — the Goal/Plan real-Host suite that landed with #5734/#5740 — timing out on a different case each round (30s on the Plan paging case, then 15s on the Plan execution case), with the file stretching to ~900s in the job. That suite forks real Host child processes and is orthogonal to this PR's catalog live-run-state changes; it smells like runner load rather than a regression. Retriggered; if it keeps failing I'd suggest looking at the suite's timeout budget on CI rather than at this diff.

[hqhq1025]

Reviewed current head b76062d736ee73cb483d4845b97072bcabb4a772. I found one P2 correctness issue in the renderer's cross-Host ordering; see the inline comment.

The change adds a per-session run epoch and a random Host-generation identity, projects both through the catalog protocol, and rejects older same-generation observations at equal Session revision. The former clock-based ordering concern is removed, the stray CI log is gone, and the attention decoder test is restored. The current-head test check passed; the diff check and merge-tree against current main are clean. There is no storage schema migration.

I did not run the suite locally (this checkout has Node 18 and no dependencies) or reproduce the restart race in a packaged Desktop. The regression test covers new-Host takeover and older reads within that Host, but not an old-Host response arriving after takeover. This comment is not merge approval.

Automated review notice: This comment was posted by an automated review agent operated by hqhq1025. It is not an independent human review and does not replace one.

[hqhq1025]

P2: A different random generation is treated as newer in both directions. If a read from Host A is delayed, Host B restarts and its same-revision row is committed, then A's already-issued response arrives, this branch returns false and commitPatch/commitSessions replaces B's row with A's stale runningTurnIds. The old row can remain until another catalog read/event. The existing test checks A→B and an older B epoch, but not B→late A. Generation identity alone cannot establish chronology; fence outstanding reads when the Host changes or carry an authoritative ordered incarnation, and add the reverse-arrival regression.

[ggbdpq]

Reviewed the mechanism and I believe this exact arrival order cannot be produced by the transport. Closing a connection rejects every in-flight request with connection_lost (client/connection.ts walks #pendingRequests and rejects them all on close), so the predecessor's read — the only way a cross-generation response with stale runningTurnIds could arrive — never resolves successfully after the Host died. And within the old connection's lifetime the generation never changes, so the branch in question is never reached. The reverse-arrival regression would have to fake a successful response across a dead connection, which would test the mock rather than the transport. The comment on the comparison now records this closure guarantee; happy to add a connection-generation fence if you still want belt-and-braces defense beyond it.

[hqhq1025]

I re-reviewed the exact head 2efb1580b12ee74aabef99493e748b6314f63eb1 (14 PR files, +538/-60). The only delta from the previously reviewed b76062d7 is the explanatory comment in session-catalog-state.ts:198-205; runtime behavior is unchanged. I revisited my earlier P2 about a predecessor Host row arriving after a successor row. The pure isStaleSummary function would accept such an ordering, but I could not establish that ordering through the production read path: connection failure rejects pending Host requests (packages/runtime-host/src/client/connection.ts:1021-1044), the Main IPC router retries a reconnectable read when its handler was replaced (apps/desktop/src/main/runtime-host-reconnecting-ipc-main.ts:309-324), and the renderer serializes full-list refreshes and same-session patch reads (session-read-state.ts:40-59, session-catalog-sync.ts:39-55). I therefore do not carry the earlier P2 forward as a substantiated current-head defect. I found no other substantiated P0-P3 issue in this re-review.

The exact-head test check succeeded. The PR diff passes git diff --check and a synthetic merge against fresh main (18827d99) is clean; GitHub currently reports MERGEABLE/BLOCKED, not a merge approval. I did not run the suite locally (Node 18/no installed dependencies), a packaged Desktop Host restart, or an Electron IPC-ordering probe for a response already settled before disconnect. Those remain validation gaps. No storage schema migration is involved.

Automated review notice: This comment was posted by an automated review agent operated by hqhq1025. It is not an independent human review and does not replace one.

[Astro-Han]

Second independent review lineage at 2efb1580 (supplements the earlier review on this head).

No P0–P2. I traced every path by which an old-Host row could reach the renderer catalog after a new-Host row at the same revision, and none can deliver an authoritative predecessor row that still carries its running state:

• sessions:get goes through the pending-request rejection on disconnect (connection.ts) and the handler re-check in runtime-host-reconnecting-ipc-main.ts, so a predecessor reply is always sent before any successor reply.
• A predecessor list result still pending in preload is discarded, because any successor row admitted via get marks the refresher dirty (runtime-host-session-catalog.ts, preload.ts).
• sessions:changed carries only an id, so it can only trigger a read that routes to the new Host.

The comparator, the kernel epoch increments (reserve/unregister/claim attach/settle), the protocol decoding, and the Desktop field pass-through all look correct. The new renderer and kernel tests would fail on the old code.

Non-blocking P3s:

• P3 — the comment overstates the guarantee (inline). One cross-generation row can arrive late: the main-process cache (session-local-service.ts catalog()) returns predecessor rows as localState: 'cached', with runningTurnIds stripped but runEpoch/runHostGeneration kept, until the new Host's catalog refresh completes. Because different generations are never stale, such a row can briefly overwrite the successor's running row. This matches main (same-revision last-writer-wins), so it is not a regression.
• P3 — same-generation reconnect no longer degrades to cached. If the Host process survives and only the connection drops, a cached row carries the epoch from the last catalog refresh. If turns started or stopped since then, the renderer's newer epoch makes the cached row stale, so the row keeps showing the pre-disconnect running state instead of degrading as before. It recovers on reconnect. This is arguably more accurate, but worth confirming it's intended given what localState: 'cached' is meant to signal.
• P3 — minor. A third per-process identity is added alongside the existing hostEpoch. Shared/guest session projections don't carry the new fields, so the #5713 flip remains for shared sessions. There's no regression test for the host-operation claim attach/settle increments.

Not verified: tests were not run locally (static read).

This review was produced with automated assistance (AI review agents) and checked by a maintainer-side reviewer before posting.

[Astro-Han]

P3 — This holds for authoritative Host reads, but not for every row that reaches the catalog. After a restart, session-local-service.ts catalog() can still return the predecessor's rows as localState: 'cached' (runningTurnIds stripped, but runEpoch/runHostGeneration kept) until the new Host's catalog refresh lands. Since cross-generation rows are never stale, such a row can briefly overwrite the successor's row. That matches the old last-writer-wins behaviour, so it's not a regression. Suggest narrowing the wording to "an authoritative (non-cached) predecessor read never delivers late", and optionally not emitting runEpoch/runHostGeneration on cached rows.

[Astro-Han]

Approved at @Astro-Han's explicit request: independent automated reviews of this head from two model families found no blocking (P0–P2) issues, and CI is green. The remaining P3 notes (comment wording, cached-row behaviour) are non-blocking follow-ups.