# Independent full final-diff review A, round 3

**FIX_FIRST for the complete committed diff: one confirmed capture-fixture lifetime bug. Product code: MERGE on inspection. No visual acceptance.**

Reviewed [#23519](https://git.cafe/pingdotgg/t3code/pulls/23519) / [#23519](https://github.com/pingdotgg/t3code/pull/23519), branch `fix/issue-16534` → `main`, pinned [a9dcdf37fd](https://git.cafe/pingdotgg/t3code/commit/a9dcdf37fd17842bb87a068fa4c94f535cbfc140), against merge-base [43879d9702](https://git.cafe/pingdotgg/t3code/commit/43879d970204eb28092d098deb4c842a22a9ab75). Current metadata matched that pin. Full diff: 12 paths, 1119 additions/17 deletions. Code reads used immutable objects in `/home/ubuntu/wt/pr-23519`; no implementation, tests, checkout files or capture inputs changed.

## Confirmed blocker: pending release targets a retired shell subscription

**P2, fixture-only, FIX_FIRST.** Locations: [replay.js:177](https://git.cafe/pingdotgg/t3code/blob/a9dcdf37fd17842bb87a068fa4c94f535cbfc140/scripts/fixtures/issue-16534/replay.js#L177), HTTP release at [line 15](https://git.cafe/pingdotgg/t3code/blob/a9dcdf37fd17842bb87a068fa4c94f535cbfc140/scripts/fixtures/issue-16534/replay.js#L15), and captured WebSocket release closure at [line 277](https://git.cafe/pingdotgg/t3code/blob/a9dcdf37fd17842bb87a068fa4c94f535cbfc140/scripts/fixtures/issue-16534/replay.js#L277).

This differs from the fixed release → resume cursor bug: **resume occurs while A1 is still withheld**.

1. Enable `delay:true`; authoritative shell sequence is 40 and A1 is withheld. Subscribe as request `old`.
2. Foreground resubscribe before `release()`: retire `old`, create request `current` with cursor 40.
3. The valid resume produces metadata-only snapshots with empty `threads`; it does not provide another full A1 snapshot to hold.
4. Call `__sidebarReplay.release()`. HTTP holding still references `delayedStream=old`, because the new request only assigns it when it is null. The WebSocket holding closure permanently captured the old stream/emitter.
5. Both paths emit A1 at sequence 41 addressed to request `old`. Neither emits A1 to `current`. The retired RPC stream no longer feeds the shell, so the row stays absent. The current stream also never acquires the inserted-event offset.

Fresh diagnostic loaded the exact pinned replay bytes in Node VM, using EventTarget/MessageEvent and stub native transport. It independently reproduced both HTTP-seeded and WebSocket-snapshot-seeded paths:

```json
{"seed":"http","currentSubscription":"current","released":[{"requestId":"old","sequence":41,"id":"triage-A-1"}],"currentReceivesA1":false}
{"seed":"ws","currentSubscription":"current","released":[{"requestId":"old","sequence":41,"id":"triage-A-1"}],"currentReceivesA1":false}
```

This is a supported client transition: [shell.ts:313](https://git.cafe/pingdotgg/t3code/blob/a9dcdf37fd17842bb87a068fa4c94f535cbfc140/packages/client-runtime/src/state/shell.ts#L313) supplies foreground resubscriptions, and [client.ts:280](https://git.cafe/pingdotgg/t3code/blob/a9dcdf37fd17842bb87a068fa4c94f535cbfc140/packages/client-runtime/src/rpc/client.ts#L280) uses `Stream.switchMap` to replace the previous subscription. The server's valid resume supplies metadata then persisted replay/live events, rather than a replacement full thread snapshot. With no real A1 mutation, there is no later event that repairs its absence.

Smallest fix: keep the pending synthetic row independent of a retired subscription and transfer its release target to the current shell stream on resubscribe, including the offset that subsequent events need. Retire/drop obsolete held callbacks so release occurs once. A tiny pending-row/current-stream model is preferable to adding another fallback replay layer. Preserve socket isolation and the already-correct cursor translation.

Regression proof should cover hold → same-socket foreground resume → metadata-only refresh → release, for HTTP and WS seeds: the **current request** receives A1 once, the shared reducer accepts it, a subsequent genuine source event is accepted, and the next resume translates the cursor correctly. The matrix P6 is already asking for the current-subscription witness; this is a concrete code defect, not merely missing visual evidence. No real-client reproduction was performed, and no production defect is alleged. It blocks complete committed-fixture approval under the explicitly requested reconstruction/replay scope.

## Prior cursor finding is fixed

The former A blocker is resolved at the pin. `send()` translates client cursor 41 back to server cursor 40 and carries offset 1 into the resumed stream. Fresh scratch verification exercised object, array and NDJSON envelopes, with both HTTP and WS seeds: **84 assertions passed**.

The sequence checked was initial sequence40 → synthetic release41 → outgoing resume40 → metadata-only high-water41 → real source event41 delivered42 → late real event42 delivered43 → next outgoing resume42 → source43 delivered44. The actual immutable shared reducer's `mergeShellSnapshotProjects` and `applyShellStreamEvent` functions were evaluated after Node stripped TypeScript; external schema-equivalence functions were stubbed with structural equality for synthetic rows. Metadata repository identity updated without replacing the retained row or advancing the client cursor; real and late titles were accepted. This is not a real RPC decoder or browser test.

The settled-source guard also remains correct: empty metadata snapshots do not invent incomplete synthetic rows. My pending-release reproduction does not reopen either settled-empty or post-release cursor finding.

## Fresh product assessment

The feature is worth having: history, palette and rendered links change the route without necessarily revealing the sidebar row. A layout-owned two-field reveal record, plus per-mount scroll debt in each sidebar, is a small adequate model. It avoids patching individual navigation callers or introducing a new service/contract.

- The shared helper ignores transient null router parameters, retains incomplete work and recognizes resolved row changes. Mutations occur through effects; the stable callback updates its closure in layout effect. Settings/drawer remount gets scroll debt while preserving the observed route and manually chosen scope.
- The new sidebar defers while search replaces the list, retries when normal rendering returns, requires an actual active DOM row before completion, and widens scope only for a newly observed route hidden by scope but otherwise eligible. The existing setter persists All and clears selection. Archived/ineligible rows do not force widening. Existing shelf/tail logic retains the active row and ancestor-row resolution is consistent with the renderer.
- Legacy retains the active row beyond the preview cut as a sorted union, without duplication. Its panel and keyboard-key derivation have different overflow predicates but yield the same rendered key order: the sole active row past the cut makes all rows visible with no empty More control; another hidden row preserves More. Collapsed projects retain only the active row. No mandatory refactor follows from the textual duplication.
- Once reveal debt is settled, thread updates cause an inexpensive early return. Pending work scans threads and queries the DOM once per relevant effect; no animation loop, added wire traffic or dependency is introduced.

No concrete product bug found. Draft-only rows absent from quiet shells do not receive this reveal until persisted, as B noted; no new regression was established. Existing focused helper tests catch meaningful transient-null behavior. A shared behavioral table for legacy preview/shortcut boundaries would guard future drift, but is not a blocker. Actual DOM timing/search-clear/scope behavior belongs to the integrated capture, not static-prop tests.

Surface accounting: chat, palette, links and history converge on route observation; Settings and drawer remounts are handled separately. Web and desktop share the renderer. Native mobile has separate navigation and no changed code. No provider adapter, RPC contract/permission, backend service, scheduler/MCP entry point, auth or persistence-schema behavior changes. Environment-scoped row identities remain intact; remote/relay/tunnel rendering uses the same code, but multi-environment and Electron-shell runtime behavior were not independently exercised. Reverse navigation does not rely on guessed history direction.

## Fixtures, complexity and sensitive semantics

The user's explicit requirement to commit seed/replay reconstruction is settled and overrides the earlier fixture-policy objection. Do not remove these files. Knip's narrow entries describe manually injected browser files; they do not import fixtures into the product. The generators emit deterministic synthetic projections/transcripts without DB access or provider starts; their SQL remains intended for stopped isolated homes, not coherent event-history proof.

Bootstrap preserves callable and constructible Date while leaving timers/performance native; it explicitly writes supplied storage before modules. Replay's sole new regex is anchored fixed alternation, without nested repetition. JSON parse followed by line splitting and per-line JSON parse has bounded work in frame size. Fresh bounded parser diagnostic: 6001 malformed bytes, 1000 iterations, 1000 caught failures, **146.86ms total (~0.147ms/input)** on this host. This is fixture-parser evidence, not product throughput. No parser complexity blocker found.

No production sensitive surface changes. `__sidebarWire` still stores whole parsed native frames even when replay is disabled, and filtering visible synthetic rows does not sanitize those buffers. No raw buffers, storage, auth, DB, logs or runtime frames were uploaded. Only this synthetic review report is intended for upload. No browser or sensitive runtime surface was opened, so no observed leak is claimed.

## Verification and evidence boundaries

- Read the complete final diff/context, both full prior reports, proof matrix, PR description and available review/comment context. Product code is unchanged from the prior product-tested revisions.
- Fresh: immutable diff check; 84 scratch assertions for the corrected sequence/metadata/late-event path; independent pending-release reproduction for both seed transports; bounded parser diagnostic. Scratch files live only under `/tmp/pr23519-review-a-r3`.
- Attributed, not rerun: prior 99 scoped tests and B's 57 logic tests. No product tests, lint or typecheck rerun against the capturer's mutable checkout.
- One-shot GitCafe status: pinned head matched; **30 successful, 0 failing, 0 pending checks**. Conflict observation was stale, so current-base mergeability was not established. No watching or polling.
- M6/M13/P1–P6 runtime captures remain separately owned evidence debt. No screenshots/videos inspected, no visual sign-off, no complete-set acceptance claim. Preserve reusable evidence with its original attribution. The new confirmed lifetime failure concerns P6's pre-release resubscribe case; it does not invalidate unrelated product capture chapters.

## FRICTION

- Capture worktree has temporary product/HTML edits owned by another task. I used immutable objects and never touched its processes, files or browser.
- Initial scratch diagnostic could not load the repository's `typescript` package because it is a tsc-rs replacement. Switched to Node's built-in TypeScript stripping; no dependency/install changes.
- B round2's MERGE for the post-release cursor delta is corroborated, but its review did not exercise resume while the row is still held. The complete-diff verdict differs solely for that independently reproduced fixture lifetime sequence.
- Green checks are current; conflict freshness and visual evidence remain independent limits. There was no head drift observed during this round and no capture-team interruption.

Model/harness: GPT-6.1-sol / Codex in T3 Code. Evidence-only review; no fixes, commits, pushes, comments, body edits, watches, server or browser.