Read-only independent review, round 1 — FIX FIRST

Reviewed [#22775](https://git.cafe/pingdotgg/t3code/pulls/22775), local head [34a6a9635a](https://git.cafe/pingdotgg/t3code/commit/34a6a9635aa2d3e764a0650149e8a222ebd7fe9f), against merge-base [21da3061aa](https://git.cafe/pingdotgg/t3code/commit/21da3061aa006fc852ea4bd84c31ec084a3afa26). Complete five-file diff inspected in the automatic worktree. Forge still records the earlier test-only head; its CI is not evidence for this local implementation. No previous review findings were supplied or used.

1. Worth having / smallest shape / product decisions

Worth having: notification completion should not silently disarm an already-needed usage-limit recovery. Reusing the shared selector and mirroring it in the two SQL projections is a reasonable small implementation with no schema, dependency, or transport additions. However, this is not strictly server-only: the shared helper also changes client thread presentation and queue eligibility. Its rule intentionally treats EVERY completed notification-root run as irrelevant to usage-limit recovery, including a notification run that actually performed useful provider work. That broader product rule should be understood; no concrete duplicate-work failure was established here. Manual continuation must agree with automatic continuation before merging.

2. Confirmed concrete bug

P2, merge-blocking: preserved usage limit leaves the held-queue Resume action unable to continue.

Diff anchor: packages/shared/src/orchestrationV2ThreadError.ts:94–100 and apps/server/src/orchestration-v2/Orchestrator.ts:11430. Relevant unchanged callers: apps/web/src/components/ChatView.tsx:1024–1034, 4570–4589; server manual-continuation validation at Orchestrator.ts:4793–4801.

Failing supported sequence: a provider run fails with usage_limit; a user follow-up remains queueHeld; a later background notification wake completes; the user presses Resume after quota resets (including when automatic resume is disabled). The new usageLimitBlockedRun ignores the wake and retains the failed limit run. But ChatView derives resumableRunId using latestExecutedRun, which selects the completed wake and therefore returns null. Because a held queue exists, Resume remains available and sends queue.resume. The changed filtered read plus new selector makes usageLimitHoldsQueue return true, so the server rejects it with “Continue the limited thread before resuming its queue.” Repeating Resume repeats the rejection. Without a held queue, the manual resume action is absent. Manually typing a fresh message or enabling automatic recovery remains a workaround.

Affected users: web and desktop users with this supported background-wake/held-queue sequence, especially those who disable automatic resume or cancel it and later resume manually. Local versus remote transport does not change the failure. Do not claim native mobile has this button: no equivalent manualContinuationOfRunId caller was found there.

Evidence: actual head helpers on a failed-limit + held queue + completed notification projection produce {latestExecutedRun:"wake", usageLimitBlockedRun:"limited", manualGuardAllowsLimited:false, hasHeldQueue:true}. The call path above deterministically selects queue.resume and rejects it. No browser or integrated runtime reproduction was performed. This is a regression in queue.resume acceptance from preserving the limit; the earlier lack of a manual run action after the wake is pre-existing but now conflicts with the newly preserved failure presentation. Simply changing the client selector is insufficient: explicit manualContinuationOfRunId for the old failed run also fails the server’s unchanged latestExecutedRun identity check.

Smallest fix: for usage-limit manual resumption, align the client source selection and the server manual-continuation guard with usageLimitBlockedRun; preserve latestExecutedRun checks for ordinary interrupted-run continuation. Keep queue holding until the limited run is explicitly continued.

3. Sensitive surfaces

No changed auth, credentials, secrets, permission grants, approval policy, runtime modes, sandbox, trust, or MCP semantics identified. Existing pending-runtime-request checks still gate arming and automatic dispatch. No NEEDS_JULIUS finding. This is scheduling/run-selection behavior, not a permission bypass.

4. Regex / parser / input loops and complexity

No changed regex or text parser; no nested overlapping quantifiers. The new firstInputs loop is O(I) expected Map work; runs.filter and latestExecutedRun are O(R); existing latestRootProviderFailure adds O(I). Extra memory O(I + R). Text content is opaque.

Each changed SQL subquery adds a first-root-input lookup for each completed candidate run. Existing (run_id, ordinal) index supports the ordered probe; worst case scans all items in that run if no matching root input exists. Across disjoint runs this adds approximately O(R log I + I), in addition to existing run ordering and outer thread selection. JSON extraction examines existing run payloads; no new untrusted-text parsing loop. No SQL benchmark was run, so index reasoning is not a latency measurement.

Independently reran supplied complexity.mjs against the exact source head with 3,000 malformed “{ ” repetitions per string, 1,000 calls after warmup. One wake: median 0.000983 ms, p95 0.004238 ms, max 1.368591 ms. 3,000 wakes/items: median 0.243952 ms, p95 0.382237 ms, max 0.670459 ms. See review-1-codex-complexity.log. This is a nominal 3,000-token stress string, not tokenizer-certified token counting; the helper never parses it. No exponential input behavior found.

5. Evidence against the diff

Independently ran the two entire changed test files using the explicitly permitted source checkout after confirming its HEAD equals the reviewed SHA: 96/96 passed (85 runtime-layer, 11 client execution), review-1-codex-tests.log. The supplied baseline log shows both new completed-wake cases fail on baseline at candidate discovery/due recovery. Existing recovery coverage plus new before-arm/after-arm cases demonstrate automatic recovery and duplicate-dispatch protection, not manual recovery after a completed wake.

Read supplied baseline, implementation, complexity and validation evidence. Scoped lint/typecheck/format logs were supplied by implementation; not independently rerun. Forge metadata/comments/status were read only with cafe. There were no reviews, and the visible checks relate to the earlier test-only head (24 successful, 2 failing), not this local SHA. No issue or other PR was read or changed. No browser, installs, extra worktrees, edits, commits, pushes, comments, or merges. Only review evidence files were created. T3 PR registration was performed and verified as required by the harness.

The real provider/runtime transcript is scheduled for the next phase. Its absence is a verification limitation, not a code defect. These event-harness tests do not establish that a live Claude quota limit and provider wake emit precisely this event history. The shared helper also ships in clients, so “server-only” is accurate only for the absence of UI component edits, not behavioral scope.

6. Requested tests

Required regression: failed usage-limit run + held user queue + completed notification wake, autoResume=false; choose the same manual continuation source the client would choose, dispatch it, verify exactly one continuation starts while the user queue stays held, then allow queue resume. This catches the uncovered mismatch between usageLimitBlockedRun and latestExecutedRun in both client selection and server validation. Also retain rejection when a newer real user run or active wake supersedes the source. Extend the existing manual-resume usage-limit test rather than adding a new harness. No request for static markup or source-text assertions.

7. Compatibility / existing data / reconnect / cancellation / platform

No wire/schema migration or new required field. Existing persisted run and notification input records can be reclassified immediately; completed runs lacking a root notification input retain previous behavior. Shared SQL and JS selection both use earliest root input ordinal, preserve active wakes, and ignore child-only notifications. No supported equal-ordinal ambiguity was identified. Reconnect recomputes state from existing projections; older clients still have the old thread-presentation helper until updated. No cross-version transport incompatibility identified.

Existing cancellation/stop/stale-command guards remain in place and the focused tests pass, including stop/rearm and active-wake cases. Manual recovery compatibility is the blocker above. No provider-specific protocol change; the selector applies to all adapters. No OS-specific APIs were introduced. Performance caveat: root notification items must remain available in bounded client projections for client presentation to match the server; no independent history-window/reconnect test was run. No confirmed defect established from that caveat.

Automatic worktree retained for lead cleanup. No installations were created to remove. Disk stayed above 12 GB free.

VERDICT: FIX_FIRST