Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 56 additions & 0 deletions devlog/_plan/260911_ws_commit_boundary/000_plan.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
# WS commit boundary — 260911

Base: `origin/dev` `babb76449f` (fetched 2026-09-11 KST). Branch `codex/260911-ws-commit-boundary`,
worktree `/Users/jun/.codex/worktrees/260911-wsc/opencodex`.

## Why this unit exists

#4191 reports a long Codex thread that fails only while routed through OpenCodex, as either
`codex websocket closed before a Responses terminal event (close 1006 Connection ended)` or
`codex websocket response prelude timed out`, and works immediately when the proxy is bypassed.
#4083 raised the fixed prelude deadline from 30 s to 90 s for slow multi-image starts; #3976 asked
for the number to be configurable; #2471 fixed the 16 MiB create-frame ceiling.

The lane dispatch round (`260911_lane_dispatch_round`) added the #4191 failure-stage counters so
a user can tell an unanswered socket from one that carried only quota frames. That was
diagnosis. This unit is the fix to the boundary the diagnosis exposed, after an external
semantic review (`010_journey_evaluation.md`) overturned the first framing.

## Scope

- `src/server/responses/codex-ws-exchange.ts` — settle post-send, pre-response failures as an
honest HTTP status; replace the fixed prelude timer with silence-based liveness; cancel the
upstream turn on a pre-commit client abort.
- `src/server/responses/codex-ws-wire.ts` — liveness constants and the non-replayable body shape.
- `src/lib/upstream-retry.ts` — a non-replayable marker that `fetchWithTransientRetry` honours, and the
structured error codes the other resend paths stop on.
- `src/server/responses/core.ts` — two early returns on the marker (pool quota rotation, opaque-blob
recovery); `src/combos/failover.ts` — structured-code stop. See 025.
- `docs-site/src/content/docs/reference/configuration/server.md` — the prelude paragraph.
- `tests/responses/ws-upstream.test.ts`, `tests/lib/upstream-retry.test.ts` — oracle updates and
new cases.

Out of scope, recorded in `020_design_record.md`: resume-by-id after 1006 (Codex does not request
background responses, so the vendor resume surface does not apply), the opt-in provider path
without a metadata channel (it commits at send today and keeps doing so), the create-frame size
predicate, and any core.ts change beyond the two marker guards named in 025.

## Rules for this unit

- No local product suite: no `bun test`, `bun run test`, `test:changed`, `typecheck`,
`build:gui`, or `bun install` in this worktree. Every verification line reads NOT RUN until
remote CI on the final head says otherwise.
- Push with `--no-verify` and `core.hooksPath=/dev/null`.
- xai/grok-4.6 subagents are read-only verifiers of the diff; aside/web research is free.
- One work-phase is one PABCD cycle: wp1 this roadmap, wp2 honest status + marker, wp3 liveness
and abort propagation, wp4 PR, review, CI.

## Work phases

| wp | unit | doc | exit |
|---|---|---|---|
| wp1 | roadmap | 000, 010, 020 | docs committed on the branch |
| wp2 | honest post-send status | 030 | code + tests committed, NOT RUN |
| wp3 | liveness + abort | 040 | code + tests committed, NOT RUN |
| wp4 | PR + review + CI | 050 | final-head CI green, review dispositioned |

Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
# Journey evaluation — how the framing changed

## What was done before this unit

1. Lane dispatch round: seven file-disjoint lanes from `6d3ad12e3`, each a worktree and a
Codex thread, merged serially on final-head green CI (#4217 … #4248). One of those lanes landed
the #4191 failure-stage counters in `codex-ws-wire.ts`: request bytes, sent, frames, control
frames, relayed events, first-frame and elapsed durations. The counters are content-free by
construction and only classify; they were never a fallback signal.
2. Structure question from the owner: `codex -> http -> opencodex -> ws -> openai` — is the
asymmetry itself the bug? Source reading said no: WS is chosen only for streaming POSTs on a
bounded-relay Bun, the create frame is measured before dialling, and the one reversible point is
the send. First framing: the reversible window is too narrow and judged by size alone; widen the
HTTP path below the ceiling and scale the prelude budget by frame size.
3. Semantic review by anthropic/claude-fable-5-1. Three corrections were accepted after source
confirmation:
- The no-resend-after-send rule is not a defect. RFC 9110 §9.2.2 forbids an intermediary from
automatically repeating a non-idempotent request; the user agent owns that decision. Offering
"allow fallback after send" as an option was the wrong question.
- The broken contract is the status code. `commitResponse` builds `new Response(stream,
{ status: 200 })` before any upstream frame, and `failStream` commits that 200 on the failure
path (`if (sent) commitResponse()`) precisely so the pre-stream wrapper cannot resend. The proxy
therefore converts "no response" into "a response that failed", removes the status the client
would use for its own retry policy, and neuters the client's first-byte timeout with chunked
headers. Direct-to-vendor Codex survives the same at-most-once lane through its own retry; the
proxy is stricter than the party whose money is at stake and pays for it with a hard failure.
- The 90 s prelude is the wrong kind of quantity: it folds "dead" and "slow" into one number.
Dead is a liveness question with a native answer (ping/pong); slow already has an owner (the
client deadline). A fixed proxy deadline in series always inherits the tighter bound.

## What the evaluation keeps and drops

Kept: every existing oracle (no HTTP fallback after send, one `response.create` per exchange,
refused-create 4xx projection, correlation before conversion, bounded queue). Kept: the 90 s
number, but demoted from "time to first response event" to "unanswered silence with no pong",
which is unreachable on a socket whose peer answers pings.

Dropped: post-send HTTP fallback (never acceptable), size-scaled prelude budgets (treats the
symptom), resume-by-id after 1006 (Codex sends `stream: true` without `background: true`; the
vendor resume endpoint requires a background response, so there is nothing to resume for this
client; recorded as a follow-up for callers that do opt in).

## What this unit does not claim

It does not claim the Codex backend answers WebSocket pings; the exchange feature-detects
`ws.ping` and degrades to the previous 90 s behaviour when no pong ever arrives. It does not run
any local suite. Whether the honest 504 improves the #4191 user's experience is a live question
that only a field report can answer; what this unit guarantees is that the proxy stops hiding the
signal that user's client needs.

95 changes: 95 additions & 0 deletions devlog/_plan/260911_ws_commit_boundary/020_design_record.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
# Design record — commit boundary, liveness, abort

## Invariants that stay

- I1 No HTTP SSE fallback once `ws.send()` has returned (`sent === true`).
- I2 One `response.create` frame per exchange; no proxy-internal resend after send.
- I3 A refused create (`type: error`, no `stream_id`, 4xx status) before any response event is
projected as that 4xx with the metadata snapshot (#3740); correlation runs first.
- I4 After the first `response.*` or `error` event has been relayed, every later failure is a
body error on the already-committed 200 (the relay synthesizes `response.failed`).

## New invariant

- I5 (exchanges with a metadata channel, i.e. the canonical Codex backend) The client commit never
precedes the upstream acknowledgment. Before the first
`response.*`/`error` event the exchange holds no client Response. A failure in that window
settles as a JSON error with an honest gateway status, marked non-replayable.

## Diff-level plan

### `src/lib/upstream-retry.ts`

Add a `WeakSet<Response>` with `markResponseNonReplayable(res)` and
`isNonReplayableResponse(res)`. In `fetchWithTransientRetry` the loop guard becomes
`if (res.ok || !isTransientUpstreamStatus(res.status) || isNonReplayableResponse(res)) return res;`.
Rationale in the doc comment: the origin may already be executing the request (RFC 9110 §9.2.2),
so a gateway status from a post-send transport is returned to the caller for its own policy.

### `src/server/responses/codex-ws-wire.ts`

- `CODEX_WS_LIVENESS_PING_INTERVAL_MS = 15_000`.
- `CODEX_WS_RESPONSE_PRELUDE_TIMEOUT_MS` keeps its value (90 000) and gains a new meaning in its
comment: the longest inbound silence (no message frame, no pong) tolerated before the first
response event.
- `codexWsPreResponseFailure(status, message, prelude: Headers): Response` — builds
`{ error: { type: "upstream_error", code, message } }` with `content-type: application/json`,
`cache-control: no-store`, the metadata snapshot headers, and calls
`markResponseNonReplayable`. `code` is `upstream_timeout` for 504 and
`upstream_closed_before_response` for 502.
- `CodexWsFailureStage` gains `pings` and `pongs`; `codexWsFailureDetail` appends
` pings=N pongs=N` inside the bracket, after `elapsed`. `tests/responses/ws-failure-stage.test.ts`
is updated in the same commit.

### `src/server/responses/codex-ws-exchange.ts`

- `failStream(error, status: 502 | 504 = 502)`: when `sent && !responseCommitted`, resolve
`codexWsPreResponseFailure(status, message, metadata.snapshot())` instead of committing a 200,
close the controller, dispose the session. When committed, unchanged.
- `cancelExchange(reason)` when `sent && !responseCommitted`: mark terminal, cleanup, dispose the
session (this closes the socket, which is the upstream cancel), `reject(reason)`. The caller's
own abort is never retried by the wrappers (`isConnectionResetError` excludes AbortError and the
retry loops check `abortSignal.aborted`).
- Liveness replaces the single `preludeTimer`:
- `armSilence()` (re)starts a `CODEX_WS_RESPONSE_PRELUDE_TIMEOUT_MS` timer whose expiry calls
`failStream("codex websocket response prelude timed out" + detail, 504)`.
- `onMessage` and `onPong` call `armSilence()` while `!responseCommitted`.
- After send, when `typeof ws.ping === "function"`, a repeating
`CODEX_WS_LIVENESS_PING_INTERVAL_MS` timer calls `ws.ping()` until commit or terminal; a
throwing `ping()` stops the pinger only.
- `cleanup()` clears both timers and removes the `pong` listener; `commitResponse()` clears
them too.
- The non-metadata path (`if (!metadata) commitResponse()`) is unchanged.

### Tests (`tests/responses/ws-upstream.test.ts`)

Updated oracles: prelude overflow → 502 JSON, not a WS-marked stream; first-response deadline
through `fetchWithTransientRetry` → 504, one send, zero HTTP; foreign-stream identity mismatch →
502; close 1006 / 1009 before any response event → 502 carrying the same messages; abort after send
before commit → the pending fetch rejects with the caller reason and the socket is closed.

New cases: a pong resets the silence clock past 90 s and the response still completes with one
send; a socket exposing `ping` is pinged every 15 s of prelude and stops after commit; a socket
without `ping` is never pinged and keeps the 90 s bound; `fetchWithTransientRetry` returns a
non-replayable 504 without a second call (`tests/lib/upstream-retry.test.ts`).

## Audit amendments

See `025_audit_round1.md`; its deltas override this file where they differ.

## Risks and their answers

- Client behaviour on 504: Codex retries stream requests on 5xx with backoff, which is the same
policy it applies on the direct path; the proxy no longer substitutes its own.
- Pool recovery on 5xx: `shouldRetryCodexPoolAccountQuota` rotates only on body-confirmed quota
evidence; the new body carries none. Opaque-blob recovery excludes 5xx other than 502 with an
encrypted-output body, which this is not.
- Backend pong support unknown: feature-detected and degrades to the current bound.
- Request log: the failure is now a 504/502 row instead of a 200 with `streamAborted`; this is
the intended diagnostic change.

## Verification

NOT RUN locally by rule. Remote CI on the final head is the only executable proof; the read-only
grok-4.6 review of the diff is the second pair of eyes.

29 changes: 29 additions & 0 deletions devlog/_plan/260911_ws_commit_boundary/025_audit_round1.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
# Audit round 1 — xai/grok-4.6 (read-only), dispositions

Verdict received: FAIL as written. Every finding below is dispositioned; the design record is
amended in place and the scope in 000 is widened to match.

| # | severity | finding | disposition |
|---|---|---|---|
| 1 | blocker | A marker honoured only by `fetchWithTransientRetry` leaves the Codex pool quota rotation (`shouldRetryCodexPoolAccountQuota`, core.ts:1120) and the combo 5xx hop (core.ts:3004 → `comboFailureDecision`) free to send again after `ws.send()`. | ACCEPTED. core.ts and src/combos/failover.ts enter scope minimally: (a) `shouldRetryCodexPoolAccountQuota` and `opaqueBlobRejectionBodyForRecovery` return early on `isNonReplayableResponse`; (b) the JSON body carries a structured `error.code` (`upstream_no_response`, `upstream_closed_before_response`) and `comboFailureDecision` returns `stop` for those codes, the same mechanism `origin_rejected` already uses. The code set lives in `src/lib/upstream-retry.ts` so combos need no server import. |
| 2 | major | Resetting the 90 s clock on quota/control frames removes the cap for a quota-only socket; it then runs to `connectTimeoutMs` (default 200 s) and settles as a `TimeoutError` 502 from `transportFailureResponse`, not the 504 the record promises. | ACCEPTED as a named behaviour change, with the status fixed. A socket that keeps sending frames or pongs is alive; the record now says so and names the quota-only case explicitly: it waits up to the operator's `connectTimeoutMs`, then the composite signal aborts with `TimeoutError`, and `cancelExchange` maps a pre-commit `TimeoutError` to the same non-replayable 504 instead of rejecting. Only a caller abort (AbortError) rejects. |
| 3 | major | Oracle list is short: metadata budget overflow rows (794), cumulative prelude bound (807), pre-response oversized frame (1064), and the `failureMessage()` helper cases in ws-failure-stage (171, 182, 208) all leave the 200 body-error shape. Foreign-stream 502 conflicts with the in-source note that a reused socket's foreign error must not become an HTTP refusal. | ACCEPTED. All listed tests are updated in wp2. The foreign-stream note was about a 4xx conversion that could authorize account replay; a non-replayable 502 authorizes nothing, and the test now asserts status 502, one send, zero fallback. The source comment is reworded to say that. |
| 4 | major | `failStream` rewrite could skip `cleanup()` and leak the pinger, silence timer, pong listener, or double-settle via `onClose`. | ACCEPTED. Order fixed in the record: `terminal = true; cleanup();` then settle, then `session.dispose()`. `cleanup()` and `commitResponse()` both clear the liveness timers and detach `pong`. |
| 5 | minor | I5 is stated globally while the no-metadata path commits at send. | ACCEPTED. I5 is scoped to exchanges with a metadata channel (the canonical Codex backend). |
| 6 | minor | `connectTimeoutMs` < 90 s makes a post-send abort a 502 connect timeout, not a 504. | ACCEPTED via finding 2: any pre-commit `TimeoutError` becomes the non-replayable 504. |
| 7 | minor | `docs-site/` paragraph on the fixed 90-second prelude deadline (reference/configuration/server.md:38-47) becomes wrong. | ACCEPTED. The paragraph is rewritten in wp3 to describe silence-based liveness and the honest status. |
| 8 | nit | Exact `codexWsFailureDetail` pin, `stage()` fixture defaults, fake-timer stepping for pong tests, feature-detect `ping` not pong. | ACCEPTED. `stage()` defaults `pings: 0, pongs: 0`; pinned strings updated; pong tests step the clock. |

Not accepted: none.

## Amended plan deltas (authoritative over 020 where they differ)

- Scope adds `src/server/responses/core.ts` (two early returns), `src/combos/failover.ts` (one
structured-code stop), `docs-site/src/content/docs/reference/configuration/server.md` (one
paragraph), `tests/responses/ws-failure-stage.test.ts`, `tests/combos/*` only if an existing
decision table needs the new row.
- `cancelExchange(reason)` pre-commit: `reason?.name === "TimeoutError"` → non-replayable 504
with `upstream_no_response`; anything else → `reject(reason)`.
- Liveness semantics: silence = no inbound message frame and no pong. Any inbound frame resets.
Quota-only sockets are alive and wait for the client or `connectTimeoutMs`.

36 changes: 36 additions & 0 deletions devlog/_plan/260911_ws_commit_boundary/030_wp2_plan.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
# wp2 — honest post-send status and the non-replayable marker

Previous D (wp1): roadmap locked at c3c1ea6731; direction unchanged — the fix is the commit
boundary, not the transport choice. 025 deltas are authoritative over 020.

## Files and exact changes

### src/lib/upstream-retry.ts
- Add a WeakSet<Response> with markResponseNonReplayable(res) and isNonReplayableResponse(res).
- Add NON_REPLAYABLE_UPSTREAM_CODES = {"upstream_no_response", "upstream_closed_before_response"} and isNonReplayableUpstreamCode(code).
- fetchWithTransientRetry loop guard: return res when isNonReplayableResponse(res).

### src/combos/failover.ts
- comboFailureDecision: after the 499/origin_rejected checks, return "stop" when isNonReplayableUpstreamCode(options?.code).

### src/server/responses/core.ts
- shouldRetryCodexPoolAccountQuota: first line returns false on isNonReplayableResponse(response).
- opaqueBlobRejectionBodyForRecovery: same early return undefined.

### src/server/responses/codex-ws-wire.ts
- codexWsPreResponseFailure(status: 502 | 504, message, prelude: Headers): Response — JSON body { error: { type: "upstream_error", code, message } }, code by status (504 upstream_no_response, 502 upstream_closed_before_response), headers = prelude snapshot + content-type application/json + cache-control no-store, marked non-replayable.

### src/server/responses/codex-ws-exchange.ts
- failStream(error, status = 502): when sent && !responseCommitted && metadata: terminal = true; cleanup(); resolve(codexWsPreResponseFailure(status, message, metadata.snapshot())); close the unused controller; session.dispose(). Otherwise the existing body-error path. (The non-metadata path commits at send.)
- cancelExchange(reason) when sent && !responseCommitted && metadata: TimeoutError -> failStream(reason, 504); otherwise terminal = true; cleanup(); session.dispose(); reject(reason).
- The prelude timer expiry calls failStream(..., 504); liveness itself is wp3.
- Reword the foreign-stream comment: a pre-response failure settles as a non-replayable 502; the 4xx projection stays reserved for a genuine refused create.

### Tests
- ws-upstream.test.ts: update 794/807 (metadata overflow -> 502 JSON, isCodexWsUpstreamResponse false), 873 foreign -> 502 + one send, 1064 oversized pre-response -> 502, 1180 abort after open -> the fetch rejects with the caller reason and the socket is closed, 1218 -> 502, 1262 -> 504 with sends === 1, 1533/1547 -> 502 with the same messages in error.message.
- ws-failure-stage.test.ts: failureMessage() returns error.message from a 5xx JSON body, else the thrown body error.
- New: upstream-transient-retry.test.ts — a marked 504 returns after one send; an unmarked 504 still retries. combos test — comboFailureDecision(504, "Provider error 504", { code: "upstream_no_response" }) is stop.

## Verification
NOT RUN locally (owner rule). Remote CI on the final head in wp4.

Loading
Loading