fix(server): add atomic t3_thread_recover_quota for usage-limit recovery

Closed
CR
8d ago
+1045-2

Tracker: CRGDan/subscription-gateway#71

Independent Claude and Codex Standards and Spec reviews are complete at ee797213b. Confirmed findings are fixed; remaining structural suggestions and test limitations are recorded below.

Summary

The subscription gateway resumes threads stalled on a usage limit that native reset recovery cannot own. Until now it could only use t3_thread_send. That tool has no failed-run precondition, and each webhook session namespaces its own command IDs. Concurrent or delayed sends could therefore start several continuations, steer newer work, or undo Stop, snooze or settle. This PR adds a dedicated tool and an atomic guard on the normal dispatch path.

t3_thread_recover_quota({ threadId, failedRunId })        MCP, writesThreads access
  caller scope, live caller, mode ceiling                  unchanged checks
  settings.withSettingsSnapshot                            lock: settings -> thread
    same-session committed replay -> original runId
    autoResumeLimitedThreads off  -> skipped
    ThreadManagementService.sendToThread                   fixed server-owned text
      message.dispatch { quotaContinuationOfRunId }        normal receipts and outbox
        under the thread dispatch lock:
          quotaContinuationRefusal(projection) ?? Stop     -> skipped(reason), nothing emitted
          new run.quotaContinuationOfRunId = failedRunId   persisted lifetime fence
  -> committed { runId } | skipped { reason } | unavailable { fixed detail }

The guard refuses unless all of the following hold:

  • No run already names the source. A continuation that was cancelled before it started still counts.
  • The source is still the latest executed run, and it is a root usage_limit failure that the current session does not supersede.
  • Native recovery does not own it. A readable reset later than completion, due or not, belongs to native recovery.
  • The thread's provider instance, selection instance and model ID match the source's exactly. Options-only changes remain eligible.
  • None of these block it: Stop, opt-out, settle (including settledAt), snooze, a pending request, a held queue, archive or delete, or subagent lineage.

Evidence

  • Before (974eaea6f, test only). Three concurrent sessions created three continuations (expected 3 to equal 1). A cancel-before-start left no fence. Each of the ten hold cases committed a continuation, and future or due native resets were taken.
  • After (8a4e3e1df, plus comment cleanup f417cb0d0 review fixes d024d5ab0 repair round 2dc70b692 and ee797213b). All of these pass:
    • runtimeLayer.test.ts › gateway quota continuation: 26 tests against the real orchestrator and SQLite. They cover one continuation across concurrent sessions, consumption after cancel-before-start, six reset-ownership cases and eleven holds with the thread left unchanged. Through the MCP service they cover a same-session replay (even after auto-resume is turned off or the thread is archived), a fresh archived request decided under the lock, a deletion seam reported as thread_not_found, refusal in a new session, a transient refusal followed by a fresh session, the settings lock and the mode ceiling.
    • core.test.ts: the tool is published with only threadId and failedRunId.
    • Touched-scope server suites: 216 tests in 10 files. tsc is clean for server, contracts and shared, and lint is clean on the changed files.
    • Consumers of the contracts change, each run on its own: web (6551 tests), mobile (2166), client-runtime (1957), scripts (341) and the oxlint plugin (181) all pass. Desktop fails only browser-secret-native, the same as unmodified base, because this machine lacks pkg-config and libsecret.
  • Independent review. Codex Standards and Spec found no findings at ee797213b. Claude Spec has no blocker; its remaining wording/test nits and Standards service-placement/duplication suggestions were assessed and retained as advisories. The coordinator independently reran all 26 quota tests at this head. No CI workflow is configured on this fork. Hosted Cubic is a separate review check.
  • Mutation checks. Without withSettingsSnapshot, the settings write lands during dispatch and the lock test fails. Without the settledAt clause, the auto-settled test fails.
  • Performance. On an unchanged dispatch path, the 21 existing usage-limit tests ran in alternating base and head runs, 7 runs per side. Base median 251.4 ms (range 236.9 to 296.1) and head median 248.9 ms (range 229.6 to 293.0). That is no measurable difference. Normal sends pay one undefined check, and only quota continuations run the guard. This was measured on a local test harness, not in production.

Merge Danger

Door: two-way. The run field and the command field are optional and additive. Reverting removes the tool, and old run payloads still decode.

Blast Radius: narrow. OrchestratorMcpService now depends on ServerSettingsService. The production MCP layer already provides it, and test layers add ServerSettings.layerTest(). Normal sends pay one undefined check. Auto-resume defaults to off, so recovery is skipped until the user enables it.

The cross-model repair uses explicit error tags with fixed redacted unavailable details. Initially absent/deleted targets retain the ordinary thread_not_found tool failure; a quota request that reaches the locked guard can return closed. Provider-session-error supersession reuses native classification and has no dedicated new integration fixture. No live recovery or deployment was exercised.

Install this T3 prerequisite before activating gateway PR #70. Enablement of global auto-resume remains an operator decision.

Model and harness: Claude Opus 5.5 in Claude Code, running inside T3 Code.

🤖 Generated with Claude Code