Skip to content

fix(server): opt-in fail-closed policy for codex thread resume - #2

Draft
nullStack65 wants to merge 1 commit into
mainfrom
a3s/v0042-strict-resume
Draft

nullStack65 wants to merge 1 commit into
mainfrom
a3s/v0042-strict-resume

Conversation

@nullStack65

Copy link
Copy Markdown
Owner

Problem

When a Codex provider thread is requested for resume and the app-server returns a recoverable error (thread missing/not found), the runtime silently starts a fresh provider thread and marks the session ready with the replacement thread id. A failed resume is therefore accepted as successful continuity, and the replacement thread masquerades as the resumed one.

Source (stock v0.0.42, apps/server/src/provider/Layers/CodexSessionRuntime.ts): openCodexThread catches isRecoverableThreadResumeError and calls thread/start; the caller accepts opened.thread.id into resumeCursor + status: "ready" unconditionally.

Fix

Add an opt-in, typed resumeFailurePolicy for the Codex provider:

  • fallback-to-new-thread (default, stock behavior) — unchanged.
  • fail-closed — a recoverable resume failure is logged and propagated; thread/start is not called, so a replacement provider thread can never be accepted as continuity.

The policy is exposed per provider instance via CodexSettings.resumeFailurePolicy (hidden settings field, defaults to fallback-to-new-thread), propagated through CodexAdapter into CodexSessionRuntimeOptions and openCodexThread.

Scope

  • packages/contracts/src/settings.tsCodexSettings + CodexSettingsPatch field
  • apps/server/src/provider/Layers/CodexAdapter.ts — pass policy into the runtime
  • apps/server/src/provider/Layers/CodexSessionRuntime.ts — typed option + fail-closed branch
  • apps/server/src/provider/Layers/CodexSessionRuntime.test.ts — focused tests
  • two settings fixtures updated for the new defaulted field

Tests

vp test run src/provider/Layers/CodexSessionRuntime.test.ts src/serverSettings.test.ts src/provider/Layers/ProviderInstanceRegistryLive.test.ts src/provider/Layers/CodexAdapter.test.ts → 151 passed. New tests cover: no-resume start, valid resume, recoverable fallback (default), fail-closed refuses thread/start, fail-closed valid resume, fail-closed no-resume, explicit default equivalence. vp --filter t3 typecheck, @t3tools/contracts typecheck (427 passed), targeted lint/fmt all green.

Provenance

  • Upstream base (tag v0.0.42): 719a76ca1dbf5490f1aa33ffb9966301e02be9a9
  • Fork base (nullStack65/t3code main): e6ae764f47d678b96f4a1b2c8c76eedf1a342e26
  • Head: a3s/v0042-strict-resume
  • Verified to apply cleanly to current upstream main 8dd02470b1e7603b8d36a21ae27c510480351f18 (git apply --check --3way)

This is a Closura review carrier derived from stock T3 Code, not stock T3 Code itself. Draft; not for merge.

Model/harness: openrouter/deepseek/deepseek-v4.1-flash via opencode.

A requested provider-thread resume currently falls back to a fresh thread on any recoverable error, so a failed resume is accepted as successful continuity. Add an opt-in 'fail-closed' Codex resumeFailurePolicy (per provider instance) that propagates the resume failure instead of starting a replacement thread. Default behavior is unchanged.
@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:M labels Sep 19, 2026
@nullStack65
nullStack65 force-pushed the a3s/v0042-strict-resume branch from 1ba1ee7 to f014905 Compare September 20, 2026 01:33

@nullStack65 nullStack65 left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A3V independent composed review — T3 fork #2 (COMMENT, not approval)

Reviewed exact head f014905ed1c73600310accc6d2bc378c9d5c7840 against fork main e6ae764f47d678b96f4a1b2c8c76eedf1a342e26 (1 ahead / 0 behind).

Independently reproduced:

  • Six-file scope exactly as required; all six blob SHAs identical to the pre-refresh A3 head 1ba1ee7… (patch-equivalent).
  • Focused Vitest: 151 passed / 4 files (CodexSessionRuntime, serverSettings, ProviderInstanceRegistryLive, CodexAdapter).
  • Contracts suites: 427 passed / 24 files.
  • @t3tools/contracts and apps/server tsc --noEmit → EXIT 0, no errors.
  • Targeted vp lint / vp fmt --check on changed files → PASS (2 pre-existing no-inline-schema-compile warnings in serverSettings.test.ts:1304-1305, not on A3-added lines).
  • Equivalent git diff --check: reverse-apply with --whitespace=error-all → clean; no conflict markers.
  • resumeFailurePolicy typed/bounded (Schema.Literals), defaulting fallback-to-new-thread; only path to thread/start after a resume request is the non-strict branch; verified no other thread/start producer in server source; strict failure exits makeCodexSessionRuntime.start before status:"ready", resumeCursor, and session/ready, closing the race.

Disposition for this repo: T3_STRICT_POLICY PASS, STOCK_DEFAULT_PRESERVED PASS, STRICT_NO_THREAD_START PASS, T3_SETTING_PROPAGATION PASS, T3_BASE_COMPOSITION PASS, CONTINUITY_ACCEPTANCE_ORDERING PASS.

Exact-head CI still QUEUED (Test, Test Server 1-3, Check, Rust, Release Smoke, Mobile Native Changes, Native fingerprint diff). No dispatch/rerun performed.

Cross-repo note (not a T3 defect): the agent-config successor contract currently cites the pre-refresh head 1ba1ee7…, not this head. See agent-config pingdotgg#227.

@github-actions

Copy link
Copy Markdown

Thread transfer impact

⚠️ The latest CI run did not produce a thread transfer result for f014905.

This comment will update automatically after the next completed run.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant