Read-only review round 2 — MERGE (reviewed code)

Reviewed [#22775](https://git.cafe/pingdotgg/t3code/pulls/22775), local head [80d27e96db](https://git.cafe/pingdotgg/t3code/commit/80d27e96db888d3ab0dbf12314f136f4e8b3e9b4), against merge-base [21da3061aa](https://git.cafe/pingdotgg/t3code/commit/21da3061aa006fc852ea4bd84c31ec084a3afa26). Inspected the complete six-file diff and surrounding selectors, command reads, manual/automatic dispatch guards, Stop handling, queue behavior, recovery worker and client presentation. Read both prior reports and implement-followup.md in full.

Confirmed findings

No confirmed merge-blocking defect in this revision. Both prior correctness blockers are resolved:

- Manual Resume: apps/web/src/components/ChatView.tsx:1028 selects usageLimitBlockedRun before latestExecutedRun. apps/server/src/orchestration-v2/Orchestrator.ts:4802 validates the same blocked source for a failed usage-limit run; interrupted continuation retains its previous latest-executed guard. The new runtime case selects the source with the client expression, rejects it during an active wake, accepts it once after the pre-reset wake completes and reset elapses, then rejects duplicate continuation. It also exercises the subsequent queue.resume call. This closes the prior client/server mismatch for web and desktop without browser-only proof.
- Stop before arming: Orchestrator.ts:9131–9164 reads the relevant inputs/errors, selects the preserved failed source, marks it stopped and records/disarms recovery for that source. The completed-wake-stop-before-arm case verifies candidate suppression, rejects automatic rearming through autoArmLimitRecovery, and permits explicit rearming. The old Stop invariant is restored on the newly supported history.
- Successful post-reset wake: the shared helper (packages/shared/src/orchestrationV2ThreadError.ts:83–106) and both SQL selection paths skip only completed notification-first runs whose completion is at or before reset. The postreset regression checks no recovery candidate, completed shell status, no limit error, stale automatic delivery starting no run, and rejection of stale manual continuation. This resolves the prior stale-recovery objection.

No failing sequence/user group remains to report as a confirmed bug. Queue holds being released by an explicit manual continuation is existing intentional behavior. The prior request to keep the queue held throughout manual continuation was not a necessary fix; the current regression verifies continuation priority while the user message remains queued.

Worth having / smallest shape / product decisions

Worth having: pre-reset background notifications must not erase a usage-limit stop or disarm recovery, whether the sweep already armed it or has yet to see it. This preserves the interrupted task and the user's explicit Stop decision.

The shape is reasonable: one shared rule, its two existing SQL mirrors, the necessary projection read filters, and aligned existing controls. No new abstraction, migration, service, dependency, wire field or provider protocol. Resolving the latest failure/error outside each SQL candidate probe avoids nesting that search under every run. I would not request a broader refactor in this fix.

The explicit product rule is based on completion time and first root input, not whether output is empty or useful. Exactly-at-reset completion is preserved; strictly-after-reset completion supersedes the limit. It can therefore retain a productive pre-reset notification run, or accept an empty post-reset completion. The implementation response expressly chooses this tradeoff, and it matches the incident's before-reset wakes. No additional product decision is needed for this review.

Minor notes

- Before publishing, update the draft description/title from the earlier test-only/“empty wake” account to the implemented reset-boundary behavior. This is PR bookkeeping, not a code blocker.
- The SQL mirrors are inherently a maintenance obligation. An eventual compact parity table in the existing projection tests would protect that obligation better than adding more scenario branches to the already broad recovery table. No production abstraction is needed just to share SQL with JavaScript.

Sensitive surfaces

No changed authentication, credentials, permissions, approvals, modes, sandbox, trust or MCP semantics. Pending runtime requests still block automatic recovery and manual continuation. Marking the preserved source stopped enforces the existing Stop boundary for late work; it does not broaden agent authority. No NEEDS_JULIUS finding.

Complexity / malformed inputs

No regex added or changed, no nested quantifiers, and no message-text parser. Shared selection performs a constant number of scans over runs/items plus expected-linear Map operations: O(R + I) time and O(R + I) extra space. Date.parse examines resetAt once; notification/error text remains opaque.

SQL adds a latest-failed-run ordered lookup and latest-root-error ordered lookup per thread, plus an indexed first-input lookup for each completed run considered by the existing latest-run scan. Worst-case work includes ordering failed runs/errors and reading candidate input ranges; it is not constant cost. Stop also now reads all relevant historical input/error rows to make the shared selection. No measured latency regression established; I did not benchmark the full shell query or Stop on very large histories.

Independently reran the supplied followup-complexity.mjs against the exact reviewed checkout, after confirming its SHA. Strings contain 3,000 repetitions of `{ ` (nominal malformed tokens, not tokenizer-certified). 1,000 timed calls:

- One wake: median 0.001293 ms, p95 0.005804 ms, max 0.650654 ms.
- 3,000 wakes/items: median 0.230477 ms, p95 0.272648 ms, max 0.502824 ms.
- Malformed resetAt: median 0.107917 ms, p95 0.116656 ms, max 0.148802 ms.

Evidence: review-2-codex-complexity.log. No pathological input growth observed.

Evidence and SQL/shared parity

- Independently reran both complete affected test files in the authorized dependency-installed checkout, confirmed at the exact head: 99/99 pass (88 runtime-layer, 11 client execution). Evidence: review-2-codex-tests.log. vp was absent from PATH; the existing checkout-local node_modules/.bin/vp succeeded. No installation was performed.
- Independently extracted both actual latest-failure/blocked-run SQL join blocks from the reviewed ProjectionStore.ts and executed them against in-memory SQLite, comparing the selected source to the actual shared helper. All 28 histories agree: just before/at/after reset; equivalent timezone offsets; starting/running/waiting/interrupted/failed/cancelled/queued wakes; missing/invalid reset; root versus child notification; user-first versus notification-first inputs; ordinary success; cancellation before start; newer non-limit failure or missing failure item. Evidence: review-2-codex-parity.log. SQLite 3.53.4, separate from the server driver's complete queries; runtime tests supply coverage through the actual server service.
- Read supplied baseline/follow-up evidence. The four follow-up cases fail against the preceding implementation for the reported reasons; supplied 22-case recovery run passes. Scoped typecheck/lint/format logs were read, not rerun. Independent full-diff whitespace check passes; worktree remains clean.
- GitCafe metadata/comments/status still describe the earlier test-only head [8f9294071b](https://git.cafe/pingdotgg/t3code/commit/8f9294071ba6d16c0e00224f622e130246cb727f). Its draft status and 24 successful/2 failing checks are not validation of the reviewed local head. No forge review comments besides the transfer-impact bot report were present. The PR link was registered and verified with T3 as required.

Gaps / needed tests

No additional test is required to close a confirmed code defect. Optional durable parity coverage should name these failures: exact-reset completion accidentally superseding the failure; a latest non-limit failure reviving an older limit; missing/invalid reset hiding a successful wake; user-first or child-only notification being misclassified. The independent parity diagnostic covered these selection cases today; the client test permanently covers the inclusive boundary and input classification, while the runtime suite covers the main command histories.

Live provider/dev-server transcript remains the planned lead evidence stage. These tests use the real orchestration/SQL harness with synthetic wake completion events, not a real Claude quota failure or adapter-driven completion. That is a stated verification limit, not a code defect. No screenshot-only proof is needed for this change, and no browser was authorized or used.

Compatibility / surfaces

No schema migration or new required wire field. Persisted input/error history is reinterpreted on read; null/missing invalid resets do not cause completed wakes to be skipped. Reconnect rebuilds from projections. Mixed-version clients can retain old local presentation/Resume selection until updated, but server guards remain authoritative and no new RPC incompatibility is introduced. No independent rolling-version or history-window test was run.

Web and desktop share the corrected Resume entry point. Shared runtime presentation applies to mobile; no native mobile manualContinuationOfRunId caller exists to align. Generic message commands, queue.resume, Stop, metadata arming and scheduled recovery all reach server service paths rather than transport-only fixes. All providers and local/remote/tunnel connections use these common paths; no adapter-specific change is required. Active wakes, ordinary completions, cancelled-before-start runs, explicit rearm and stale delivery remain covered. No documentation change is needed for corrected existing behavior.

Read-only scope honored: no source edits, fetch, install, new worktree, browser, push, review comment, merge or watcher. Temporary parity script removed; evidence logs/report retained. Automatic detached-head worktree retained for lead cleanup. Root disk remains approximately 12 GB free.

VERDICT: MERGE