fix(desktop): deliver mid-session tail append after loading history

[liugddx]

Summary

In the desktop app, continuing a conversation in the currently active session could leave a newly arrived assistant message invisible until you switched to another session and back.

Root cause: once the resident transcript window has been trimmed off the tail — #hasNewer === true, which happens after you scroll up to load older history (#loadBefore → #evictToBudget(..., 'newest')) — DesktopTranscriptReplica.#catchUp() handled a Host transcript_advanced by bumping the durable watermark and publishing an empty change:

if (this.#hasNewer) {
  this.#durableThrough = target;
  this.#publish([], [], []);   // nothing reaches consumers
  return;
}

target cannot be appended contiguously to the trimmed window, so an already-open consumer (the active session's renderer range store) never learned about the new message, and the event-driven refreshMessages timed out on waitForDurableMessage without calling setMessages. Switching sessions rebuilt the subscription and re-anchored at the tail, which is why switching away and back made the message appear.

This re-anchors to the newest window instead — the same recovery a fresh subscription performs — so the append reaches open consumers live:

if (this.#hasNewer) {
  await this.#replaceWithRange(target, target, 512 * 1024);
  return;
}

Concurrency hardening (review-driven sweep)

Making that branch await a page load turned a previously synchronous path async, opening an interleaving window: a concurrent discard() (LRU memory reclaim for a non-visible session — it does not go through the operation queue) can mark the replica non-resident while a page is in flight. On resolve, repopulating durable state would resurrect a deliberately discarded replica past its memory bound, and in one path throw a fatal error.

Prompted by review, I swept every awaiting path in this file for the same class and re-check #resident after the await, before mutating or publishing, in all three:

1. #replaceWithRange (the re-anchor above) — resolved page would repopulate durable state and resurrect the replica.
2. #loadBefore (older-history load) — same repopulation on the resolved older page.
3. #catchUp (ordinary contiguous append) — the per-page callback already returned early, but expectedSequence was then left short of the watermark, so the post-loop check threw correlation_changed and the subscription owner drove the session terminal — turning a benign reclaim into a fatal error. This one is production-reachable.

Verification

Local deps in my workspace have drifted ahead of this branch's base, so the full-project build:main no longer typechecks some unrelated management/SSH test files (missing deploymentId, widened action unions). The changed files and their transitive imports compile cleanly in isolation; CI (test) is green at the exact head.

• tsc on the changed files + their imports (isolated project) — clean
• npx biome check on the two changed files — clean
• node --test desktop-transcript-range-store.test.js — 19/19 (includes three discard-race regressions)
• node --test runtime-host-session-observer.test.js — 37/37 (from a matching-deps run)

Each guard is a genuine regression — confirmed to fail with the guard removed and pass with it restored:

• delivers a mid-session tail append even while a history window is resident — without the delivery fix the tail advance() publishes no upsert.
• does not resurrect a discarded replica when a tail re-anchor is in flight — without the #resident re-check the resolved page repopulates durable state (durableUpserts = [5], residentBytes grows back).
• does not resurrect a discarded replica when a history load is in flight — without the guard the resolved older page publishes/repopulates.
• does not drive a discarded replica terminal when a contiguous catch-up is in flight — without the guard advance() rejects with the watermark correlation_changed.

Review focus

The behavioral change is scoped to the #hasNewer catch-up branch (long sessions where you have scrolled up past the resident cache). Sessions that stay within the cache take the ordinary contiguous-append path and are unaffected. The three #resident re-checks are no-ops on the happy path; they only make a mid-flight discard() a clean no-op instead of a resurrection or a fatal error.

AI use

Select exactly one:

• No generative tool made a substantive contribution
• Generative tooling made a substantive contribution

Tool(s) and scope: Claude Code — diagnosis of the delivery gap and the discard race, the production guards in desktop-transcript-replica.ts, and the regression tests. Reviewed and owned by the human contributor of record. All three affected commits carry a Generated-by: Claude Code trailer.

Checklist

• Tests cover the change and fail without it
• Lint, format, typecheck and the affected suites pass locally (changed files + affected suites; see Verification note on unrelated dep drift)

Does this PR entail a change in behavior?

• Yes — described under Summary above
• No

[Astro-Han]

This is a synthesis of the independent blind review by @Sol-404ARE at exact head 1c60402fc231ab7ec9bf25d89590bbeb10027d06 (base a956b1ae04aa7421a749931006a6df8fe564fc60, 2 files +76/-2). I verified the diff and the exact-head CI myself; the file:line findings below are from Sol's sealed review.

What I checked myself:

• Read gh pr diff 4066 (fix: desktop transcript replica, 2 files) and confirmed the new await #replaceWithRange(...) at desktop-transcript-replica.ts:365-378 and the helper at :267-296 with only #assertOpen() after await page.
• Checked exact-head CI: label SUCCESS, test run 33152715570 SUCCESS (affected tests, Runtime Host, Desktop E2E, Browser smoke, Storybook, CLI release candidate build all green), OPEN/MERGEABLE/BLOCKED/REVIEW_REQUIRED.

Findings from Sol's review (file:line anchored):

Standards — NO-GO — 1×P2 (worst P2)

• P2 — Single submission-hygiene root cause — the PR body replaces the repository template (Problem/Root cause/Fix/Tests) and omits the required Summary/Verification/AI-use selection/Checklist/behavior fields. The body also states “Generated with Claude Code,” yet the only materially AI-authored commit carries only Co-Authored-By and lacks the Generated-by: Claude Code trailer required by CONTRIBUTING.md:34, violating CONTRIBUTING.md:81. Restore the template, fill the required selections, and amend the trailer.

No other Standards/code finding; the production increment reuses the existing #replaceWithRange seam with net entropy reduction and a green Fowler baseline.

Spec — NO-GO — 1×P2 (worst P2)

• P2 — The async re-anchor can resurrect a non-resident replica after discard — apps/desktop/src/main/desktop-transcript-replica.ts:365-378 does await #replaceWithRange(...) into helper :267-296, which after await page only checks #assertOpen() and does not re-check #resident. It then clears/installs durable range, advances the watermark, and publishes. Concrete race: let loadTranscriptPage stay pending; with hasNewer=true start advance(5); while pending call synchronous discard() (:331-341, which keeps the replica open but marks it non-resident); on page resolve the callback reloads and publishes, undoing eviction and the memory bound. The ordinary paged catch-up at :386-396 already correctly guards with if (!this.#resident) return. Add the same guard after the replacement await before any mutation and add a deferred-page regression: discard, then resolve, asserting no upsert/repopulation.

The happy path is otherwise correct: the new test creates hasNewer, advances to seq 5, receives a durable upsert, and the watermark advances; a higher concurrent target is safely converged via #targetThrough plus finally reschedule; tail anchor/direction, overlay completion, and oldest-edge eviction are correct.

Verification: git diff --check PASS, changed-file Biome PASS, ASF headers PASS, worktree clean, head unchanged. Focused tests were not rerun in the detached read-only worktree; 33152715570 is now SUCCESS with affected tests, Runtime Host, Desktop E2E, Browser smoke, Storybook, and CLI release candidate build all green — terminal CI is green.

What I did not judge: a full discard → page-resolve E2E with a real Host was not executed beyond the race-inspection — verification was by code inspection and the tests noted above.

Gate: Standards 1×P2 (template/trailer) and Spec 1×P2 (re-anchor guard) remain; despite label and test both SUCCESS, head 1c60402f is still not merge-ready due to the two P2s. Seal: notes/pr-4066-provisional.md.

---

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

[liugddx]

Thanks — both findings were real; addressed at c7401fed3.

Spec P2 (re-anchor can resurrect a discarded replica). Confirmed. Making the #hasNewer branch await #replaceWithRange(...) turned a previously synchronous branch async, so a concurrent discard() can flip #resident to false while the page is in flight, and the resolved callback would repopulate durable state past the memory bound. Fixed by re-checking #resident after the await inside #replaceWithRange, before any mutation/publish — the same guard the paged catch-up already uses. Added a deferred-page regression: it parks catch-up inside the re-anchor's page load, discard()s, then resolves the page and asserts no upsert and no repopulation (residentBytes stays 0). It fails without the guard (durableUpserts = [5], residentBytes grows back) and passes with it.

Standards P2 (submission hygiene). Restored the repository PR template (Summary / Verification / AI use selection / Checklist / behavior field) and amended the commit to carry the Generated-by: Claude Code trailer alongside Co-Authored-By, per CONTRIBUTING.md. Dropped a stray Refs #4024 (that is the unrelated font-size PR; this bug has no tracking issue).

Local re-verification on the amended head: build:main, typecheck, and biome check clean; desktop-transcript-range-store 17/17; runtime-host-session-observer 37/37.

[liugddx]

Follow-up (af8694d2e): swept the rest of this file for the same "state can change across await" class the review flagged, and found one more latent instance.

#loadBefore awaited an older-history page and then installed it while only asserting #closed — so a discard() (memory reclaim for a non-visible session) landing during the load would be silently undone: the resolved page repopulated durable state and grew #residentBytes past the bound, exactly the resurrection the re-anchor guard now prevents. Added the same #resident re-check plus a deferred-page regression that discards mid-flight and asserts no publish and no repopulation (fails without the guard).

The remaining awaiting paths were reviewed and need no change:

• the paged catch-up already re-checks #resident before and after its await;
• the post-loop empty publish is unreachable once residency flips, because the expectedSequence watermark check trips first.

Transcript suite 18/18, observer 37/37, biome + typecheck clean.

[Astro-Han]

This is a synthesis of the independent blind review by @Sol-404ARE at exact head af8694d2e8ee94785e3eb25fa605282d71059312 (base a956b1ae04aa7421a749931006a6df8fe564fc60, 2 files +242/-2). I verified the drift and the exact-head CI myself; the file:line findings below are from Sol's sealed bounded freshness review against 1c60402f.

What I checked myself:

• Read the incremental diff 1c60402f → af8694d2 (2 files +166, two rewritten commits c7401fed35 + af8694d2e8, guarding #loadBefore/#replaceWithRange/catch-up) and confirmed the new guards and the three deferred-page regressions.
• Checked exact-head CI: test run 33156432378 SUCCESS (affected tests, Runtime Host, Desktop E2E, Browser smoke, alignment audit, Storybook, CLI release candidate build all green), label SUCCESS, OPEN/MERGEABLE/BLOCKED/REVIEW_REQUIRED.

Findings from Sol's bounded review (prior 1c60402f had Standards 1×P2 + Spec 1×P2):

Standards — NO-GO — 1×P2 + 1×P3 (worst P2)

• Closed: prior P2 — PR body now restores the template with the exact AI-use selection (Generated-by: Claude Code trailer now present on both rewritten commits with valid co-author trailers).
• New P2 — PR body is stale on the new exact head — the body still says 17/17, both new tests, two-line production change ... and both regression tests, singular the affected commit. The exact head now has 18 tests, 3 new regressions, 2 production guards ( #replaceWithRange + #loadBefore ), and 2 affected commits. Update the body to the exact 18-test counts and note whether that exact head was re-run.
• New P3 — judgment-only Duplicated Code — three neighboring regressions duplicate ~219 lines of the same five-message corpus, page/bootstrap, fake handle/decoder, history/change collector, and byte-budget plumbing. Extract a small history-window/deferred-page fixture. Production entropy decreases, test entropy increases.

Spec — NO-GO — 1×P2 (worst P2)

• Closed: prior P2 — #replaceWithRange and #loadBefore now have post-decode #resident guards that prevent late reload/publish after discard, each with a deterministic deferred-page regression — correctly closed.
• New P2 — production-reachable contiguous #catchUp discard — the ordinary contiguous catch-up at apps/desktop/src/main/desktop-transcript-replica.ts:389-399 can be discarded by another observed Session's global cache touch/LRU discard in runtime-host-session-observer.ts:1382-1415 while the page await is pending. On page resolve the callback at :400-402 returns, but expectedSequence is not advanced, so :420-423 throws correlation_changed; the subscription owner at :419-429 does not treat this as recoverable and the Session goes terminal. Fix: after #withDecodedPage, before the continuation/watermark check, add if (!this.#resident) return, and add a deferred contiguous-page discard regression. A prior provisional finding about coalesced advance was withdrawn because the sole external caller is the observer and live/pending frames are serially awaited.

Verification: full/incremental git diff --check PASS, both files Biome PASS, ASF header audit PASS, worktree clean, head unchanged. Focused tests were not rerun in the detached read-only worktree; test is now SUCCESS with all affected workspace, Runtime Host, Desktop E2E, Browser, alignment, Storybook, and CLI checks green — terminal CI is green.

What I did not judge: a true LRU-discard → page-resolve → correlation_changed → terminal-failure E2E with two observed Sessions was not executed beyond race-inspection — verification was by code inspection and the tests noted above.

Gate: Standards 1×P2 (body staleness) + 1×P3 and Spec 1×P2 (catchUp guard) remain; despite label and test both SUCCESS, head af8694d2 is still not merge-ready due to the two P2+P3 findings. Seal: notes/pr-4066-af86-freshness-provisional.md.

---

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

[liugddx]

Thanks for the third pass. Addressed the two P2s at 65c7e6942.

Spec P2 — production-reachable contiguous #catchUp discard. Confirmed, and you caught something I got wrong. In my previous follow-up I claimed the post-loop empty publish was "unreachable because the expectedSequence watermark check trips first" — I saw the check trip but mistook the throw for protection. It isn't: when another observed session's LRU discard() lands during a contiguous direction: 'newer' page await, the per-page callback returns early (no repopulation, good), but expectedSequence is left short of the watermark, so :401 throws correlation_changed and the subscription owner drives the session terminal — a benign memory reclaim turned fatal. Fixed by re-checking #resident after the page loop, before the watermark check, so catch-up returns cleanly and a later resume re-runs it. Added a deferred-page regression that discards while a contiguous catch-up page is in flight and asserts advance() resolves (not rejects) with no repopulation; it fails without the guard (advance() rejects with the watermark error).

That makes three #resident post-await guards now, one per awaiting path in this file (#replaceWithRange, #loadBefore, #catchUp), which I believe closes this class here.

Standards P2 — stale PR body. Fixed. Body now reflects the exact head: 19 tests, 3 production guards, 3 affected commits (all carrying the Generated-by trailer), and the four fail-without-guard regressions named individually. I also added a Verification note: my local workspace deps have drifted ahead of this branch's base, so the full-project build:main no longer typechecks some unrelated management/SSH test files; the changed files + their transitive imports compile cleanly in isolation and CI test is green at the exact head.

Standards P3 — test duplication (judgment). Acknowledged — the three discard-race regressions do repeat the five-message corpus, fake handle/decoder, and change collector. I deliberately kept each self-contained so its fixture documents the specific interleaving it exercises (tail re-anchor vs. older-history load vs. contiguous catch-up differ in bootstrap shape, budget, and which page is gated). Happy to extract a shared deferred-page fixture helper if you'd prefer the deduplication over the locality — your call, since it's a test-only readability tradeoff.

Verification on the new head 65c7e6942: desktop-transcript-range-store 19/19, biome clean on both changed files.

[Astro-Han]

This is a synthesis of the independent blind review by @Sol-404ARE at exact head 65c7e6942d7232ff174a825f9a89b18749530012 (base a956b1ae04aa7421a749931006a6df8fe564fc60, 2 files +329/-2). I verified the drift and the exact-head CI myself; the file:line findings below are from Sol's sealed bounded freshness review against af8694d2.

What I checked myself:

• Read the incremental diff af8694d2 → 65c7e694 (1 commit +87, guard plus deferred contiguous-page regression :479-556) and confirmed the new post-loop #resident guard at desktop-transcript-replica.ts:421-428 sits before expectedSequence/correlation_changed (:429-430) and watermark/empty-publish (:432-433) with no await in between.
• Checked exact-head CI: test run 33162522534 SUCCESS, label SUCCESS, OPEN/MERGEABLE/BLOCKED/REVIEW_REQUIRED.

Findings from Sol's bounded review (prior af8694d2 had Standards 1×P2+1×P3 / Spec 1×P2):

Standards — GO with non-blocking cleanup — 1×P3 (worst P3)

• Closed: prior stale-body P2 — the PR body now correctly states 19/19, four regressions, three post-await guards, and three affected commits; Generated-by: Claude Code trailers are present on all three commits with branch/title hygiene.
• Retained P3 — judgment-only Duplicated Code — desktop-transcript-range-store.test.ts:259-556 is now four neighboring large transcript/race fixtures; new :479-556 repeats the five-message setup, page/bootstrap/overlay, deferred gate, fixture handle, change collector, and discard/release + resident/byte assertions. Extracting a deferred-page/discard scenario builder would reduce this systemic duplication — still counted as one non-blocking P3, not a new gate.

Runtime failure-state entropy is reduced; test-maintenance entropy is slightly increased.

Spec — GO — 0 P0–P3

• Closed: prior production-reachable contiguous #catchUp P2 — discard now flips residency during the page load/decode; the callback at desktop-transcript-replica.ts:389-420 returns with no mutation, and the new post-loop guard at :421-428 returns before expectedSequence/correlation_changed and before watermark/empty-publish. Because no await exists between the guard and the synchronous checks, no new discard point exists, so the Session no longer goes terminal. Resume semantics are also correct: a discarded replica does not become resident; a later open refreshes the subscription and prepares a new replica, and advance()'s finalizer only bumps the nonresident watermark without repopulating.

• New regression :479-556 deterministically parks on a contiguous newer-page load, discards, resolves, and proves advance() does not reject, has no seq-5 upsert, remains nonresident, and residentBytes==0. It does not directly assert changes.length===0, so it cannot alone catch an empty publish, but the current guard placement makes that publish unreachable — therefore no finding.

Previously closed #replaceWithRange/#loadBefore seams remain closed; coalesced-advance withdrawal remains.

Verification: incremental git diff --check PASS, both-files Biome PASS, ASF headers PASS, exact test inventory 19 PASS, worktree clean, head unchanged. test SUCCESS; direct source-test in the detached worktree lacked workspace resolution and was excluded as an environment issue, not a product failure.

What I did not judge: a true LRU-discard during the contiguous page load with a second observed Session was not executed beyond the race-inspection — verification was by code inspection and the tests noted above.

Gate: Standards 1×P3 (non-blocking) and Spec 0 — no P0–P2, test/label green, head 65c7e69 is ready to be approved and merged once a write holder approves (the remaining Duplicated-Code P3 is a cleanup suggestion). Seal: notes/pr-4066-65c7-freshness.md.

---

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

[Astro-Han]

Reviewed the current head. The bug is reachable when an active Session has moved its resident transcript window into older history: advancing only the watermark publishes no message, so the open consumer cannot observe the new assistant reply.

Re-anchoring through the existing newest-range path is the right fix. It preserves one contiguous transcript authority instead of adding a parallel notification or cache state. The post-await residency checks also correctly close the discard races introduced by the asynchronous reload, preventing reclaimed replicas from being resurrected or driven terminal.

The focused regressions cover the delivery gap and all three discard interleavings. Exact-head CI is green. Looks good.

简体中文

问题路径真实：当前会话加载过较早历史后，缓存窗口不在尾部；旧代码只推进水位却不发布消息，因此新回复无法到达已打开的页面。

复用现有最新窗口重新锚定是正确方案，保持单一连续 transcript authority。异步读取后的 residency 检查也完整覆盖了 discard 竞态，避免已回收缓存被复活或误入 terminal。相关回归测试和当前 CI 均通过。