wave: passthrough hardening — timeout semantics, byte-budget admission, synthesized-response marker, token clobber guard - #33
Open
dean0x wants to merge 60 commits into
Open
wave: passthrough hardening — timeout semantics, byte-budget admission, synthesized-response marker, token clobber guard#33dean0x wants to merge 60 commits into
dean0x wants to merge 60 commits into
Conversation
…en identity check The concurrent-refresh guard in `persistTokens` used lexicographic `>` on `last_refresh` strings, which is wrong: ISO-8601 format divergence (e.g., fractional-second precision) or a same-second write makes the comparison unreliable. If the guard missed, subswitch would clobber the Codex CLI's rotated refresh_token with its own already-consumed token, causing an `invalid_grant` failure on the next cycle with no downstream rescue. Fix 1: replace the timestamp comparison with a refresh-token identity check (`fileNow.tokens.refresh_token !== refreshToken`). This is format-independent and is the established idiom in the `invalid_grant` retry path at line 291. A secondary `Date.parse` numeric comparison is kept for the same-second case. Fix 2: add an early return when `fileIsNewer` is true but `materialFrom` fails on the newer file. Previously, control fell through into the merge and wrote anyway, defeating the guard even when it had correctly fired. Fixes #26 Co-Authored-By: Claude <noreply@anthropic.com>
…ix duplicate error log Fixes #27 Defect A: upstream.setTimeout(connectTimeoutMs) was armed immediately after http.request() and never re-armed on connect, so it measured time-to-first-byte rather than connection establishment time. On a keep-alive pooled socket there is no connect phase at all, making the 10 s budget a hard cap on upstream think-time and causing spurious 504s for long-running requests. Fix: fold a rearm into the existing 'socket' listener. When socket.connecting is true (fresh connection), re-arm to streamIdleTimeoutMs on the 'connect' event. When socket.connecting is false (pooled socket), re-arm immediately — no 'connect' event fires on a reused socket. The existing re-arm inside the response callback (for mid-stream idle semantics) is left in place. Defect B: the timeout handler called upstream.destroy() before writing the 504. destroy() on an in-flight ClientRequest emits 'error' (ECONNRESET) on the next tick. The error handler's `responded` guard was inert on the pre-header path (responded was still false), so both anthropic_upstream_timeout and anthropic_upstream_error were logged and res.end() was called after res.destroy(). Fix: replace the `responded` guard in the error handler with a new `settled` flag that is set by whichever handler responds first (timeout 504 writer or the response callback). The 504 is also written before upstream.destroy() to narrow the race window. Adds three regression tests: think-time > connectTimeoutMs succeeds; exactly one warn event on genuine timeout; pooled-socket path arms streamIdleTimeoutMs immediately. Co-Authored-By: Claude <noreply@anthropic.com>
…dget Adds a three-budget design to the Anthropic passthrough: - connectTimeoutMs (10 s): TCP establishment only — unchanged. - headerTimeoutMs (600 s): connect→response-headers, re-armed on socket connect (or immediately for pooled sockets). Defaults to Anthropic's own server-side ceiling so the relay never fires before the origin does. - streamIdleTimeoutMs (300 s): headers→stream-end, reset per chunk — unchanged. Previously the socket re-armed to streamIdleTimeoutMs on connect; a non-streaming Opus completion with large max_tokens that buffers server-side for several minutes could be cut off at 300 s where a direct connection would have succeeded. With headerTimeoutMs defaulting to 600 s the relay is aligned with the origin's own timeout. Also: - CHANGELOG: version [Unreleased] → [0.2.1] - 2026-08-19; corrects the res.end/res.destroy ordering claim (it was res.destroy after res.end, not the reverse); documents the TTFB budget widening. - Tests: updated three existing timeout tests to exercise headerTimeoutMs explicitly; renamed to reflect the new knob. - Fixes missing headerTimeoutMs in server-wiring and doctor test fixtures. Co-Authored-By: Claude <noreply@anthropic.com>
…terialFrom fallback
- CHANGELOG: version header from [Unreleased] to [0.2.1] - 2026-08-19 so the
release-notes extractor can locate the entry (RELEASE-FLOW.md:137)
- Comments: reword 'same-second case' to 'cross-format ordering' in both the
CHANGELOG entry and the codex-auth.ts inline comment; the Date.parse secondary
signal fires on format mismatches (e.g. '.500Z' vs 'Z'), not same-second writes
where identical strings produce equality. Add NaN note (Change 5).
- Regression test: fix the format-diverged fixture so lexicographic order is
genuinely wrong while chronological order is right. Baseline '2027-01-15T08:00:00.000Z',
file writes epoch-millis '1800000005000' ('1' < '2' lexicographically → string guard
misses; identity check fires). Empirically confirmed: FAILS on main, PASSES on branch.
Add Falsifier: comment per convention.
- materialFrom fallback: when fileIsNewer is true but materialFrom fails on the newer
file, serve the just-refreshed tokens from memory rather than returning an error.
No write occurs (clobber protection intact); the request now succeeds. Update the
corresponding test to assert success + no write.
Co-Authored-By: Claude <noreply@anthropic.com>
…e headerTimeoutMs default to 660 s Three changes from verification follow-up on #27: 1. Add mid-stream idle test to passthrough.test.ts. The new test sends 6 SSE chunks ~50 ms apart (~300 ms of active streaming) then stalls. With streamIdleTimeoutMs=100ms the idle timer fires after the stall and res.destroy() truncates the body. A 2 s timing bound makes the test non-vacuous: with streamIdleTimeoutMs=10_000 the elapsed time reaches ~10 261 ms, failing the bound and proving the knob does the work. The active-chunk phase also pins the "reset by every received chunk" invariant. 2. Raise headerTimeoutMs default from 600_000 to 660_000 ms. The relay's clock starts at TCP connect; the origin's starts at full-request-received. Equal budgets with an earlier start lets the relay pre-empt the origin by the request-upload time plus RTT. The 60 s of headroom corrects that. Updated in src/config.ts (schema + JSDoc), subswitch.config.example.json, README.md table, and CHANGELOG.md. 3. Add assert.equal(result.value.config.anthropic.headerTimeoutMs, 660_000) to test/unit/config.test.ts alongside its connectTimeoutMs and streamIdleTimeoutMs siblings so the default is not unasserted. Co-Authored-By: Claude <noreply@anthropic.com>
…rorType variants - Map body_too_large → request_too_large (was invalid_request_error) in proxyErrorToAnthropic; status code remains 413 - Add not_found_error and request_too_large to AnthropicErrorType union so callers can type-check against all Anthropic-spec error types - Add unit test asserting the correct mapping (L4 regression test)
…thropic When a model name contains a colon prefix that is not a registered provider id (e.g. "claude-sonnet-9:preview"), the relay was returning a relay-invented 400 error. This violates the passthrough principle — api.anthropic.com would have accepted or rejected the request on its own terms. Change: in the unknown_provider switch arm of dispatch(), forward to the Anthropic upstream and emit a warn-level log instead of returning 400. Real codex: prefix routing is unaffected. Adds routing-behavior tests: L1 (fail-open for unknown qualifier) and the existing F6g updated to assert the request reaches Anthropic, not 400.
Replace the 32-request concurrent-count gate (which could 503 under normal load) with a byte-budget admission gate that queues rather than rejects: - New config knobs: limits.maxInFlightBytes (default 2 GiB), limits.maxQueueDepth (1000), limits.maxQueueWaitMs (60 000 ms) - limits.maxConcurrentRequests deprecated (kept for backward compat) - anthropic.maxUpstreamSockets default raised from 32 → 256 - Admission uses content-length header; POST /v1/messages without content-length falls back to maxBodyBytes (conservative, no bypass) - Single-request progress: an oversized request is always admitted when the server is idle (inFlightBytes === 0) - Budget released on res.finish (keep-alive safe) AND res.close with a once-guard to prevent double-decrement - Client disconnect while queued removes the entry and drains the queue without leaking the reservation - Queue overflow returns HTTP 529 overloaded_error (not 503) - /__subswitch/* exempt from the gate Integration tests cover: admitted under budget, queueing-not-rejection, single-request progress, disconnect-while-queued, counter-returns-to-zero, queue-exhaustion 529, health never gated.
…on, test non-vacuity
Required fixes from adversarial review:
1. FIFO / anti-starvation (BLOCKING): add queue.length === 0 guard to acquireSlot
immediate-admission condition. New arrivals must join the queue when waiters
are already present — prevents small requests from barging past a large queued
one and causing starvation-to-529.
2. Rewrite vacuous b1 test (BLOCKING): previous "queueing" test used budget=100
with 10-byte bodies (nothing ever queued) and asserted status < 529 (passes on
504). New test: budget=10, asserts parkedRes===null before release (req2 must
be queued), then asserts res2.status===200 and body.ok===true.
3. Add queued-state assertion in b2 test (BLOCKING): assert parked.parkedRes===null
before controller1.abort() so the test fails when queueing is disabled (req2
would be immediately admitted and parkedRes would be non-null).
4. Assert inFlightBytes invariant (BLOCKING): replace Math.max silent clamp with
delta-check + logger.log("error", "inFlightBytes_underflow", ...) so a
double-release is visible in operator logs. Still clamps to prevent counter
going negative; justified in comment.
5. Reconcile chunked reservation (BLOCKING): for POST /v1/messages without
content-length, getReservationBytes reserves maxBodyBytes (32 MiB) as an
anti-bypass estimate. After bufferBody() returns, shrink inFlightBytes and
the slot to the actual body size. reservationBytes is now let so the
releaseSlot closure sees the updated amount. Documented in JSDoc, README,
CHANGELOG.
Recommended fixes:
6. Request identity assertions: add distinct ?id= query params and assert
parkedUrl?.includes(...) in tests b1, b2, d, e so tests verify WHICH
request reached upstream, not just that something arrived.
7. Positive control in test (g): before the health check, fire a second POST
and assert it queues (parkedRes===null after 30ms), proving the gate is
engaged before verifying health bypasses it.
8. Real-registry L1 test: add "claude-sonnet-9:preview" through the real
registry (no synthetic resolver), asserting Anthropic routing (200) and
byte-identical response body forwarding.
9. acquireSlot returns Result<void, SlotError>: converts from throw/reject to
discriminated Result type per codebase convention. DisconnectedWhileQueued
sentinel class removed; kind "disconnected" now handled via Result.error.kind.
Route logged as "disconnected_while_queued" to prevent spurious
route:anthropic/status:200 log entries for unserved requests.
10. Dead code removed: QueueEntry.reject field (never called by drainQueue),
not_found_error from AnthropicErrorType union (appeared only in type
declaration, unused in src/ or test/).
11. Bad log line: DisconnectedWhileQueued early-return now sets
route = "disconnected_while_queued" before returning so request_complete
does not emit anthropic/200 for a request that was never served.
Non-vacuity verified: b1 queued-assertion fails with huge budget, b2 queued-
assertion fails with huge budget, h (FIFO) only-A-assertion fails without the
queue.length guard.
Co-Authored-By: Claude <noreply@anthropic.com>
ClientRequest.setTimeout() defers via an internal 'connect' listener so it
fires only after TCP connect — in the same tick as the headerTimeoutMs rearm —
meaning connectTimeoutMs was never in force for any measurable interval.
Measured before fix: blackholed IP (192.0.2.1) with connectTimeoutMs=700 failed
after 75 019 ms (macOS kernel TCP timeout). Neither budget fired.
Fix: arm the timer directly on the socket inside the 'socket' event handler,
before 'connect' fires. Node v22's internal socket-timeout handler (onTimeout)
skips req.emit('timeout') when socket.connecting is true, so we explicitly
forward socket 'timeout' → upstream.emit('timeout') to trigger the 504 handler.
On connect we cancel the connect timer and re-arm to headerTimeoutMs via the
normal upstream.setTimeout() path (which propagates correctly for connected
sockets). Measured after fix: same scenario fails at ~700 ms. ✓
On HTTPS, 'connect' fires after TCP but before the TLS handshake, so TLS
negotiation falls under headerTimeoutMs, not connectTimeoutMs. Documented in
JSDoc for both PassthroughOptions and config.ts AnthropicSchema.
New test: connectTimeoutMs fires during TCP connect to a non-routable upstream
(192.0.2.1, TEST-NET-1). Without fix the test was cancelled at the 30 s
--test-timeout limit; with fix it passes in ~261 ms. Non-vacuity confirmed
empirically. Exactly one anthropic_upstream_timeout warn is emitted — settled
de-dup holds on the connect-timeout path.
Co-Authored-By: Claude <noreply@anthropic.com>
…; silence client-abort warn
CHANGE 1 (L2): When a connection-level error (ECONNRESET / EPIPE / socket hang
up) arrives before any response byte and the request body is buffered, retry
exactly once on a fresh socket (agent: false, bypassing the stale pool entry).
If both attempts fail, res.destroy() presents a transport failure rather than
a relay-synthesised HTTP 502 the origin cannot produce.
CHANGE 2 (L3): Add x-subswitch-synthesized: 1 to every response the relay
generates — 504 (timeout), 502 (connection failure), 503 (concurrency gate),
413 (body too large), 500 (internal proxy error), 400 (ambiguous / unknown
provider), 404 (unknown /__subswitch path), 200 (health). Proxied origin
responses, including upstream error statuses such as 429, do not receive the
header.
CHANGE 3: Set settled = true in res.on("close") before calling
upstream.destroy(), so the error event emitted by destroy() on the next tick
does not produce a spurious anthropic_upstream_error warn or attempt to write
a 502 into an already-closed response. Also set settled = true in the error
handler before writing the 502 for defence against theoretically possible
double-emission.
Tests: 8 new integration tests (L2 retry bounded/success/both-fail/no-retry-
after-headers; L3 header present/absent; Change 3 client-abort silence).
Co-Authored-By: Claude <noreply@anthropic.com>
…horitativeness
Remove the upstream retry block (L2): the ECONNRESET predicate cannot
distinguish a stale pooled socket from an origin that received the
request and began processing it, creating a billing-duplicate window
that spans request-write to first byte. Node's keep-alive agent already
evicts destroyed sockets and the Anthropic SDK retries connection errors
above the relay, so the relay retry is both unsafe and redundant.
- Remove the retry request, the anthropic_upstream_retry log event, the
isConnectionError predicate, and the applyRequestHeaders helper (now
single-use; inlined into the main path)
- Remove the four L2 tests
Fix the x-subswitch-synthesized marker header (L3):
- Strip the header from proxied upstream responses (added to HOP_BY_HOP)
so an origin that sets it cannot impersonate the relay; adds strip test
- Add the marker to respondJson by default so all 12+ codex-leg
respondJson/respondProxyError call sites are covered in one line
- Add the marker to the codex-leg SSE writeHead(200) (the one remaining
synthesized site not going through respondJson)
- Replace per-call-site { "x-subswitch-synthesized": "1" } spreads in
server.ts with a synthesizedHeaders() helper for correct-by-default
enforcement on future synthesized response sites
- Add two codex-leg marker tests (streaming and non-streaming)
- Update CHANGELOG and add operator-facing README section documenting
the wire contract
Keep Change 3 (client-abort warn fix) unchanged; add comment explaining
the timeout handler asymmetry.
Co-Authored-By: Claude <noreply@anthropic.com>
…test Commit 12d05af changed the synthesized 413 body to use `request_too_large` (matching Anthropic's own API taxonomy) and updated the unit test, but the matching assertion in test/integration/passthrough.test.ts was missed. Update `body.error.type` assertion from `invalid_request_error` to `request_too_large` to match the current source. Co-Authored-By: Claude <noreply@anthropic.com>
…through-hardening
…h-hardening # Conflicts: # CHANGELOG.md
…passthrough-hardening
…-hardening # Conflicts: # CHANGELOG.md # README.md # src/server.ts
Remove the relay-invented admission gate, arm no timer after TCP connect, add a structural terminal-outcome latch, harden the logger field set, add soft-deprecation warnings for retired config keys, and clean up fixtures. Item 1 — Shared deprecation mechanism (config.ts, cli.ts) - DEPRECATED_KEYS table (6 entries: anthropic.headerTimeoutMs, anthropic.streamIdleTimeoutMs, limits.maxConcurrentRequests, limits.maxInFlightBytes, limits.maxQueueDepth, limits.maxQueueWaitMs) - detectDeprecatedConfigKeys() — pure, prototype-safe - LoadConfigResult.deprecatedKeys field - serve() warns per entry after config_loaded - Fix LEGACY_KEY_MOVES: limits.streamIdleTimeoutMs → providers.codex only Item 2 — Delete admission gate (server.ts) - Remove SlotError, QueueEntry, inFlightBytes, queue, drainQueue, getReservationBytes, acquireSlot and all call sites (gate 519-542, release slot 553-574, chunked reconciliation 594-617) - Fix bufferBody memory defect: chunks.length = 0 after Buffer.concat Item 3 — Remove headerTimeoutMs / streamIdleTimeoutMs (anthropic-passthrough.ts) - Remove fields from PassthroughOptions - Remove upstream.setTimeout(streamIdleTimeoutMs) in response callback - Socket handler: arm connectTimeoutMs only while connecting, disarm on connect, arm NOTHING afterwards (ADR-010) Item 4 — Structural terminal-outcome latch (anthropic-passthrough.ts) - Replace bare `settled` flag with settle() one-way latch - All four terminal handlers (response, timeout, error, client-close) use it Item 5 — Server knobs + clientError handler (server.ts) - SERVER_TUNING const (requestTimeout, headersTimeout, keepAliveTimeout, maxRequestsPerSocket, maxHeaderSize) - attachClientErrorHandler — Anthropic-shaped 400/431 + x-subswitch-synthesized: 1 - createProxyServer applies all tuning values Item 11 — Logger completeness guard (logger.ts) - Remove inFlightBytes and reservationBytes from LogFields - Add satisfies readonly (keyof LogFields)[] to FIELD_KEYS - Add _FieldKeysComplete compile-time exhaustiveness check Item 12 — Fixture drift fix (test/unit/doctor.test.ts, test/integration/server-wiring.test.ts) - Remove deprecated fields from makeTestConfig / makeMinimalConfig Example config cleanup - Remove headerTimeoutMs, streamIdleTimeoutMs, maxInFlightBytes, maxQueueDepth, maxQueueWaitMs from subswitch.config.example.json Test rewrites - concurrency.test.ts: delete gate suite (P0-2b…P0-2h), rewrite P0-2a as "N concurrent POSTs all reach upstream" (630 tests, was 639) - passthrough.test.ts: rewrite 4 timeout tests for ADR-010, fix L3 504 test to use 192.0.2.1 + connectTimeoutMs, fix client-abort test - config.test.ts: update defaults assertions, replace gate tests with soft-deprecation tests, add DEPRECATED_KEYS invariant test
…nomy, ambiguous fail-open
Item 6 — 413 delivery race (drainRejectedUpload):
Replace req.destroy() + connection:close with drainRejectedUpload(req, res) on the
body_too_large path. Calling destroy() with unread inbound bytes sends RST; the client
may see ECONNRESET before reading the 413. drainRejectedUpload() calls req.resume() to
drain remaining upload data so the TCP teardown uses FIN instead, giving the client time
to read the response. A 2-second unref'd safety timer bounds resource use.
Item 7 — Header fidelity (directional strip):
Split the single HOP_BY_HOP set into HOP_BY_HOP (RFC 7230 §6.1, both directions) and
RESPONSE_STRIP (adds x-subswitch-synthesized, response direction only). filterRawHeaders
now takes a required `strip` parameter so call sites must declare direction. x-subswitch-
synthesized is no longer stripped from client requests — stripping it was relay-invented
behaviour prohibited by ADR-010.
Item 8 — Error taxonomy:
8a: Route /__subswitch/* 404 through toAnthropicErrorBody("not_found_error", ...) with a
fixed message (no path reflection). Add not_found_error to AnthropicErrorType.
8b: Remove body_too_large and client_disconnected from ProxyError into a local
BufferBodyError type in server.ts. This is a compile-time enforcement: the compiler
now prevents calling proxyErrorToAnthropic with either removed variant.
8c: Fix codex-leg test title — "aggregation !ok maps to 502 (no message_start in stream)"
→ "unrepresentable upstream stream format (no message_start) maps to 502".
8d: Set route = "internal_error" in dispatch().catch() and update the error message to
identify the fault as a proxy fault, not an upstream failure.
Item 9 — Ambiguous family fails open:
Change the "ambiguous" route case from returning a relay-invented 400 to forwarding to
Anthropic (same policy as unknown_provider). Preserves the ambiguous_model_name warn log
with both provider names in the model field. Rationale: a relay-invented 400 naming our
own provider registry is an ADR-010 violation; PROVIDER_IDS only has "codex" today so
this branch is currently unreachable, but the fix applies the correct policy before a
second provider ships.
Mutation-proven tests:
- filterRawHeaders request-direction strip: RED when x-subswitch-synthesized is in HOP_BY_HOP
- 404 Anthropic shape: RED when body reverts to JSON.stringify({error:"not found"})
- route=internal_error: RED when assignment is removed from dispatch().catch()
- F7 fail-open: RED (2 failures) when ambiguous reverts to 400
Applying: ADR-010 (relay transparency), ADR-008 (credential redaction at render site),
PF-011 (mutation-proof tests), PF-012 (non-vacuity argument documented in comments).
…NG pins, coverage gaps
Item A (12-REDONE): replace hand-coded makeTestConfig()/makeMinimalConfig() with
defaultConfig() derived from loadConfig empty-config pattern, eliminating the
32-vs-256 maxUpstreamSockets drift. Non-vacuity guards in both files prove the
fixtures cannot silently diverge from real schema defaults.
Item B — five new behaviour-pinning tests (mutation-proven RED/GREEN):
B1: SERVER_TUNING properties actually wired onto the http.Server
B2: Keep-Alive: timeout=300 header present on responses
B3: 6.5 s idle socket reused (keepAliveTimeout=300_000 > Node default 5 s)
B4: raw TCP request with 65 KiB header receives Anthropic-shaped 431 JSON
with x-subswitch-synthesized: 1 (not Node's bare status line)
B5: upstream ECONNRESET produces exactly 1 anthropic_upstream_error event
Item C — coverage gaps:
C6: x-subswitch-synthesized: 1 asserted on 502 (passthrough) and health 200
C7: request_complete log event fields verified from a live request
C8: F6g test now captures logger; unknown_provider_qualifier warn asserted
C9: example-config test gains deprecatedKeys:[] + maxUpstreamSockets spot-check
C10: stale 529/concurrency comment at passthrough.test.ts ~670 replaced with
accurate synthesized-response coverage map
Item D (verified): 413 drain race — mutation-proven: restoring req.destroy() makes
the "413 race: large upload" test fail reliably; drainRejectedUpload() is correct.
Mutation table: B1 RED, B2 RED, B3 RED, B4 RED, D RED — all restored GREEN.
B5 settle() latch not deterministically provable via single ECONNRESET (noted).
640/640 tests pass.
…precation test (items 10,13,14,15,16)
Item 10 — unknown_provider severity: "fail" → "info" (ADR-010)
An unknown provider qualifier does not block the request — subswitch
forwards it to Anthropic unchanged and it works. Calling it a failure
contradicts ADR-010. ambiguous stays "fail" because that conflict is
subswitch-derived and will 404 at the origin.
- src/agent-scan.ts: severity literal + JSDoc updated
- src/router.ts: comment updated to reflect informational status
- test/unit/agent-scan.test.ts: add severity assertion to unknown_provider
test; replace hand-written failingKinds set with severity-derived check
- test/unit/doctor.test.ts: +2 tests — unknown_provider exit-0 and
info-tag rendering, with positive control (unresolvable → exit 1)
Item 13 — stale prose removed, end-state documented
- src/codex-auth.ts: remove "32 concurrency slots / 503s" references
- src/provider-transport.ts: remove "concurrency slot leaks" reference
- test/unit/codex-auth.test.ts: same fixes in RELI-04/05 doc comments
- test/unit/provider-transport.test.ts: same fix
- README.md: remove deprecated headerTimeoutMs/streamIdleTimeoutMs rows;
remove deleted admission-gate rows (maxInFlightBytes, maxQueueDepth,
maxQueueWaitMs, maxConcurrentRequests); add dedicated "Deprecated config
keys" table; document SERVER_TUNING values and keepAliveTimeout 300s
rationale (PF-018); document drainRejectedUpload and 64 KiB maxHeaderSize;
document count_tokens codex estimate is deliberate; fix x-subswitch-
synthesized list (remove "529 concurrency gate", add 431); fix example
config (remove stale limits.maxInFlightBytes)
- CHANGELOG.md: fix "529 concurrency gate" reference in PR #32 entry;
replace stale maxConcurrentRequests deprecation entry with accurate
description of all six deprecated keys
Item 14 — memory bench committed
- test/tools/memory-concurrent-uploads.bench.ts: fires N concurrent bodies
at the proxy, prints RSS/heap deltas, documents the reference measurement
(100 concurrent × 3.01 MiB → +1002 MiB RSS, +4.9 MiB heap). Excluded
from npm test globs (flat non-recursive, .bench.ts not .test.ts).
Item 15 — CLI deprecation warning test
- test/unit/cli.test.ts: runServeUntilReady helper spawns the real CLI,
waits for the ready banner, SIGTERMs, captures stderr. Two tests:
deprecated key present → warning on stderr; clean config → no warning.
Non-vacuous: mutation (suppress errOut loop) turns positive test RED.
Item 16 — 404 synthesized marker already covered
- Contradiction in phase 3 handoff resolved: the x-subswitch-synthesized: 1
assertion on 404 was added in phase 2 and confirmed present. No change
needed. Full coverage: 413, 502, 504, 500, 404, 431, health 200.
Test count: 640 → 644 (+4: 2 doctor unknown_provider + 2 CLI deprecation)
Mutation table:
impl severity "info"→"fail" | 3 tests RED (agent-scan + 2 doctor)
CLI errOut loop removed | 1 test RED (cli deprecation positive case)
Co-Authored-By: Claude <noreply@anthropic.com>
Four items cleaned up after the four-author wave: - server-wiring.test.ts: duplicate section header block (first shorter version before the extended PF-010 block) — genuine duplication from parallel work, second block is the authoritative version. - server-wiring.test.ts: "were removed in phase 1 (commit 0a00a42)" replaced with a description of the current end-state reason. - passthrough.test.ts: "as of phase 3 — C10 correction" parenthetical dropped from the synthesized-response coverage inventory comment. - passthrough.test.ts: "Change 3:" prefix removed from section comment and test name for the client-abort test. Functionality unchanged, 644/644 pass.
…, four disarmed controls
P0 — upstream socket leak on mid-stream client abort (anthropic-passthrough.ts)
res.on("close") gated upstream.destroy() on the settle() latch, but the response
callback claims that latch as soon as headers are relayed, so a client aborting
mid-stream skipped the teardown entirely. pipe() does not propagate destination
teardown to the source, leaving the upstream response half-read and its socket
permanently outside the agent's free pool. Measured: 5 aborts -> 5 held sockets.
At maxUpstreamSockets (256) the pool exhausts and every later request queues in
http.Agent forever — an unbounded hang no origin produces, and precisely the leak
ADR-010 warns the removed streamIdleTimeoutMs no longer covers. Introduced by this
PR's item 4. settle() now arbitrates only the warn log and client-visible outcome.
P0 — inbound timeouts reported as 400 instead of 408 (server.ts)
Attaching a clientError listener suppresses Node's canned reply, so the handler
added for 400/431 shaping also swallowed the 408 that requestTimeout and
headersTimeout produce (both arrive as ERR_HTTP_REQUEST_TIMEOUT on Node 22) and
re-emitted it as 400 "malformed request" — a status the origin never sends for a
slow client, plus a wrong diagnosis. Statuses now come from a table keyed on the
clientError code. PF-021 records this response as un-interceptable; measured, that
holds for Node's DEFAULT reply only.
P1 — drainRejectedUpload's 2 s bound never fired (server.ts)
Disarmed on res "close", which fires a tick after the 413 is written, cancelling
the timer ~2 ms after arming. A client that ignored the 413 and kept uploading was
never cut off: the bound existed only in its comment (PF-019). Disarm now comes
only from req "end"/"close"/"error".
P1 — four disarmed test controls (PF-011/PF-012)
- SERVER_TUNING values were compared only to SERVER_TUNING; requestTimeout 600s ->
2s and keepAliveTimeout 300s -> 5s (the PF-018 regression) both shipped green.
Added independent literal pins alongside the wiring assertions.
- The clientError 400 arm had no test at all — status, type, message and the
client_error warn were all freely mutable. Added B4c.
- agent-scan severity test named retired/preview_only/provider_unconfigured but
asserted only the last; promoting the others to "fail" (a PF-006 CI break)
passed green. All four kinds now asserted.
- CLI deprecation assertion OR'd the structured record with the human notice, so
dropping config_key_deprecated to debug stayed green. Split into two.
- DEPRECATED_KEYS "completeness" test derives both sides from the table; its
comment claimed it catches removals. Scope corrected, real guard named.
P2 — stale prose that asserted bounds the code does not have
connectTimeoutMs JSDoc still described a headerTimeoutMs re-arm (PF-019's exact
failure mode); maxUpstreamSockets JSDoc justified itself by the deleted byte gate;
README described requestTimeout as bounding the request lifetime when it bounds
receipt only; the 404 arm carried a duplicated comment block.
Also: bufferBody no longer holds (and re-concatenates) up to maxBodyBytes after a 413.
New tests: B4b (408 preserved), B4c (400 fallback), B6 (abort reclaims socket),
B7 (drain bound fires). Each proven RED against the specific defect it catches.
Suite: 648/648 (644 baseline + 4). typecheck clean.
…dictions, tautological guards, concurrency N=20, duplicate F7 test
FIX 1 — CHANGELOG [0.2.1]: rewrote the section to describe net effect only.
Removed the "Byte-based admission with queueing" Changed entry and the
limits.maxInFlightBytes / maxQueueDepth / maxQueueWaitMs Added entries —
features added and removed within the same unreleased cycle should not appear
as either additions or deprecations. Fixed the connectTimeoutMs entry title
(removed "new headerTimeoutMs knob") and body (removed the three-budget design
description that referenced deprecated knobs as current behavior). The
Deprecated section now lists all six deprecated-and-ignored keys together with
a clear "remove them" instruction. Added missing items: 404 error-shape fix,
route=internal_error labeling, SERVER_TUNING / keepAliveTimeout, and ambiguous
model family fail-open. Fixed the maxUpstreamSockets rationale to not mention
admission gates. Nothing anywhere in the file describes a synthesized 529 or
an admission/queueing gate as current behavior.
FIX 2 — Tautological non-vacuity guards (avoids PF-011, PF-012):
• test/unit/doctor.test.ts: deleted the "defaultConfig — non-vacuity guard"
describe block. The guard compared defaultConfig() against an identical
loadConfig() call — both sides are the same function with the same args, so
the assertion cannot fail. Protection lives in config.test.ts literals.
• test/integration/server-wiring.test.ts: replaced spread-vs-base comparisons
with literal value assertions (port=4141, connectTimeoutMs=10_000, etc.).
MUTATION PROOF: changing DEFAULT_PORT to 4142 turns the guard RED; the old
spread-vs-base test would have remained green (both sides change together).
FIX 3 — test/integration/concurrency.test.ts residue:
• Raised N from 2 to 20 (one-fifth of the ~100-sub-agent product thesis).
All 20 requests fire simultaneously; test fails if any are blocked.
• ParkingUpstream redesigned to multi-slot (arrivedCount / releaseAll) —
removes the dead parkedUrl scaffolding entirely.
• Both tests now have explicit positive controls: the N-concurrent test
releases all 20 and asserts 200; the health test releases the parked POST
and asserts it also returns 200, confirming it was genuinely in-flight.
MUTATION PROOF: adding a gate that rejects requests > 1 turns the test RED.
FIX 4 — test/integration/routing-behavior.test.ts duplicate F7:
Removed the second F7 test ("forwarded ambiguous request reaches the Anthropic
upstream"). It only re-asserted status 200 and upstream reach, both already
covered (with the stronger ambiguous_model_name warn-log check) by the first
F7 test. The first test remains; F7 / ambiguous-fail-open coverage is intact.
MUTATION PROOF: reverting the ambiguous branch to return 400 turns the
remaining F7 test RED.
Final: 646/646 green (648 - 1 doctor guard - 1 F7 duplicate = 646).
Co-Authored-By: Claude <noreply@anthropic.com>
FIX 1 — routing-behavior.test.ts: replace vacuous notEqual(400) with equal(200) at the F6g test; update the stale non-vacuity comment in the L1-real block that still referenced notEqual. FIX 2 — agent-scan.test.ts: add severity assertions to the unresolvable and ambiguous tests that previously checked kind only — no test anywhere was asserting severity "fail" for ambiguous. Add a doctor-level test in doctor.test.ts that proves ambiguous → severity "fail" → failures++ → exit 1. Mutation-proved: flipping ambiguous to "info" turns both tests RED. FIX 3 — anthropic-passthrough.ts: correct the JSDoc for maxUpstreamSockets from the hard-error path limits.maxUpstreamSockets to the correct key anthropic.maxUpstreamSockets. FIX 4 — passthrough.test.ts: update the x-subswitch-synthesized strip comment from HOP_BY_HOP to RESPONSE_STRIP, reflecting the item-7 split that separated response-only stripping from the bidirectional set. FIX 5 — untrack .devflow/docs/handoff-passthrough-hardening.md which was force-added past .gitignore in commit 6d29d4f. File remains on disk. FIX 6 — README: add a factual note under the deprecated-keys table explaining that with headerTimeoutMs and streamIdleTimeoutMs removed, a connected-but- silent upstream produces no response until the client's own timeout (deliberate per ADR-010 — matches direct-to-origin behavior). Co-Authored-By: Claude <noreply@anthropic.com>
This was referenced Aug 19, 2026
Closed
…leg status assertions I-042: add maxUpstreamSockets: 1 to the health-under-in-flight-POST test so the parked POST holds the only upstream connection. Health answering under that condition proves the endpoint does not need an upstream socket — a falsifiable property (if health queued behind the parked POST the test would hang to the 30 s deadline). Retitle to state the property actually asserted. I-075: tighten the B7 drain-bound assertion from < 6_000 ms to > 1_500 && < 3_000 ms. The documented bound is 2 s (drainRejectedUpload); 6_000 ms is 3× slack and would pass even if the timer were set to 5 s. The tighter bound tolerates CI jitter (±1 s) while catching a bound regression; the lower side catches a spurious instant close. I-077: add response.status === 200 assertion to the "codex: prefix still resolves" routing test. Previously a 500 or 502 on the Codex leg would pass silently. The assertion bites: the Codex leg currently returns 502 (pre-existing src/ behavior, outside this batch's scope); the test is now RED and correctly visible, whereas before the vacuous pass hid the failure. Co-Authored-By: Claude <noreply@anthropic.com>
Resolves I-004, I-005, I-014, I-016. I-004 — the clientError guard was per-SOCKET, not per-REQUEST. `socket.bytesWritten` is cumulative over the socket's lifetime, so after the first response on a keep-alive connection it is permanently non-zero and `bytesWritten === 0` is permanently false. Every later client error was silently destroyed: no 408, no 431, no Anthropic body, no synthesized marker, not even the `client_error` warn. Because registering the listener has already suppressed Node's canned reply, the intercept converted a reply into NO reply — strictly worse than the bare status line it replaced. This branch makes reuse the steady state (keepAliveTimeout 300 s, maxRequestsPerSocket 0), so it was the common case, not an edge. The fix keeps a WeakMap baseline of bytesWritten stamped when each response FINISHES, and replies iff bytesWritten equals that baseline — the public equivalent of Node's own private `_httpMessage._header` check. Only 'finish' stamps it: a response aborted mid-write must keep the old baseline so the guard still sees unflushed bytes and destroys. I-005 — the handler annotated its socket parameter as net.Socket where @types/node declares stream.Duplex. Listener parameters are bivariant, so that was an `as` in all but name, invisible to a grep for assertions; a non-net.Socket Duplex would make bytesWritten undefined, fail `=== 0`, and take the whole 400/408/431 taxonomy dark with no compile error. It is now annotated Duplex with an `instanceof net.Socket` narrowing, so that case is an explicit deliberate destroy. Proven load-bearing: removing the narrowing yields `TS2339: Property 'bytesWritten' does not exist on type 'Duplex'`. `err` is NodeJS.ErrnoException (optional members only, so satisfied by any Error), which drops the cast without reintroducing an unsound narrowing. I-014/I-016 — inbound HTTP transport policy moves out of server.ts (665 lines, four concerns) into src/inbound-policy.ts with ONE entry point, applyInboundPolicy(server, logger), owning SERVER_TUNING, CLIENT_ERROR_RESPONSES and the clientError handler. The separate attachClientErrorHandler export is deleted (it had zero importers across src/, test/ and e2e/). PF-021's lesson is that the timeouts and the response taxonomy for their expiry are ONE policy: exporting the halves separately let a caller apply tuning without the handler — PF-021's original defect verbatim — or the handler without the tuning, shaping 408s around Node's 60 s default. One call makes that unrepresentable. maxHeaderSize stays in server.ts's createServer options: it is constructor-only. server.ts is now 557 lines; the raw HTTP/1.1 wire serialization no longer lives there. Applies ADR-008 and ADR-010; avoids PF-021 and PF-011. Controls (avoids PF-011 — each proven RED by RUNNING it, not predicting): B4r/B4br repeat the 431 and 408 cases on a socket that has already completed one successful request/response, and additionally assert the `client_error` warn fires. Against the per-socket predicate both failed with "connection closed before a second, shaped response on the reused connection; received 0 byte(s)" — reproducing the measured symptom exactly. A first RED attempt failed on the FIXTURE instead (the helper did not understand the health endpoint's chunked framing); that was corrected so the controls fail for the reason they claim. Both GREEN after the fix; 41/41 in scope. SERVER_TUNING's literal-value pins are preserved unchanged, with the import path updated. Its JSDoc prose is left as-is for the batch that owns I-015/I-026/I-062. Co-Authored-By: Claude <noreply@anthropic.com>
…upstream so its 200 assertion is satisfiable I-077: the fake Codex upstream was returning Anthropic-shaped SSE events (message_start, content_block_stop, message_delta, message_stop). The codex leg translates OpenAI Responses-API SSE, so those events were all silently ignored, the aggregator found no message_start, and returned 502. Replace the body with a minimal valid Responses-API stream (response.created → response.completed) that the translator converts to message_start + message_delta + message_stop, allowing aggregateFrames to assemble a 200 response. Co-Authored-By: Claude <noreply@anthropic.com>
…nfig; fix maxBodyBytes doc; dedupe defaults test I-039: tighten reasoningCache from z.object to z.strictObject — a typo'd leaf key (e.g. maxEntires) now hard-errors instead of silently reverting to defaults (avoids PF-010; non-breaking per PF-020). Regression test added under TS-02 describe block: RED against z.object, GREEN after fix. I-053: delete export type ProviderFileConfig — zero consumers in src/ or test/ (noUnusedLocals cannot see exported types; grep confirmed before and after). I-067: fix JSDoc on maxBodyBytes — drop "Codex" from "before the Codex routing decision"; server.ts calls bufferBody for every POST /v1/messages* and forwards the buffered body on the Anthropic leg too, so "before the routing decision" is the correct text and config.ts was the drifted copy. I-045: delete duplicate anthropic.maxUpstreamSockets default assertion that sat under the "Removed keys" banner (line ~333) — the assertion at line ~30 already covers it. Co-Authored-By: Claude <noreply@anthropic.com>
…error; status-order the union I-034 (ADR-010): add 404 → not_found_error and 413 → request_too_large arms to upstreamStatusToAnthropicError so the same HTTP status carries the same error.type whether the relay or the origin produced it. I-018: overloaded_error had zero producers (verified: upstream 529 falls to api_error via the status >= 500 arm; Anthropic-leg 529 passes through raw headers, never touches the mapper). git grep -n overloaded_error confirmed only docs and the definition itself. Delete the member and its retention comment; add a doc note that 5xx including 529 maps to api_error. I-029: not_found_error was appended last in the union but belongs between permission_error (403) and request_too_large (413) in status order. I-064: not_found_error's comment named its one consumer (/__subswitch/* 404), making it stale the moment a second 404 site appears. State the meaning instead: resource or path not found; the origin's 404 error type. I-013: CHANGELOG [0.3.0] — add maxHeaderSize: 64 * 1024 to the SERVER_TUNING Changed entry (fifth constant, determines 431 threshold); add not_found_error alongside request_too_large in the Added bullet. Co-Authored-By: Claude <noreply@anthropic.com>
…ion.test.ts, end-state BREAKING notes Owner decision: the BREAKING config-key removal ships as a minor bump, matching the project's own precedent for a config-format break (0.1.0 -> 0.2.0). - version.test.ts now pins all five release literals, not two: src/version.ts, package.json, both package-lock.json version fields, and the newest `## [X.Y.Z]` heading in CHANGELOG.md. Proven RED against the pre-bump tree (CHANGELOG 0.2.1 vs literals 0.2.0) and re-proven non-vacuous on the lockfile arm by mutation. Closes the sync gap PF-014 left open, where the only assertion compared two files that are always co-edited and so could not fail. - package.json, both package-lock.json fields, and src/version.ts -> 0.3.0. - README BREAKING note states the end state: two keys removed (anthropic.streamIdleTimeoutMs, limits.maxConcurrentRequests), not six. The other four were born and died inside this branch and never shipped in 0.2.0, so no config that has ever existed can contain them. The changelog link was a bare `#fragment` resolving inside README against a wrong slug; it now points at CHANGELOG.md#030---2026-08-19. The matching CHANGELOG edits (heading retitle + BREAKING list trimmed to the two shipped keys) were made in the working tree and swept into 3ab02ce by a concurrent `git commit --only -- CHANGELOG.md`; content is intact in HEAD. The `removed in 0.3.0` strings in src/config.ts stay hardcoded rather than being derived from SUBSWITCH_VERSION: a frozen historical fact must not follow the current version (avoids PF-023). Addresses I-002, I-012, I-021, I-049, and the user-facing half of I-009.
…ototype-safe error lookup
I-015 (avoids PF-018): keepAliveTimeout JSDoc replaced with the real rationale.
Node's 5 s default closes an idle inbound keep-alive socket at the exact moment a
client starts its next POST, producing an ECONNRESET the relay cannot retry for a
non-idempotent request; 300 s makes that window negligible. The previous claim
("matches the Anthropic keep-alive pool") was false: the outbound agent has no
idle-socket timeout and governs a disjoint socket set.
I-026: The applyInboundPolicy inline comment now states the 4+1 split explicitly.
Four timer/counter knobs are set after construction; maxHeaderSize is
constructor-only (no http.Server property) and is passed in the createServer
options object in server.ts. The SERVER_TUNING JSDoc already stated this correctly;
the inline comment was the inconsistency.
I-062: SERVER_TUNING JSDoc now notes that the member names mirror http.Server's
own property names verbatim and are unrelated to providers.*.requestTimeoutMs.
I-074 (avoids PF-021): applyInboundPolicy is now idempotent via a module-level
WeakSet<http.Server> guard. A second call on the same server returns early; without
the guard, double-calling would register two clientError listeners and double-reply
into the same socket. Tested: server.listenerCount("clientError") === 1 after two calls.
I-055: Extract responseForClientError(code) using Object.hasOwn so prototype
properties ("constructor", "__proto__") cannot masquerade as response descriptors.
The bare bracket lookup reaches Object.prototype for those names; ?? does not fire
(truthy function value); the socket would receive "HTTP/1.1 undefined undefined".
Matches Object.hasOwn usage at config.ts and models.ts. RED/GREEN: 4 failures
proved before fix, 7/7 pass after.
I-078: socket.end(head + body, () => socket.destroy()) ensures destroy() runs only
after the kernel accepts the write. The bare sequential pattern races the buffer
and can truncate the synthesized body before the client reads it.
Co-Authored-By: Claude <noreply@anthropic.com>
…roy; name the drain bound
I-017: synthesizedHeaders() extra param deleted. All four call sites pass zero args;
the docblock demonstrated { connection: "close" } which is exactly the header this
branch removed from the 413 response with an 11-line comment explaining why it was
wrong. A future contributor following the docblock would repeat the mistake.
I-056 (ADR-010): respondJson extraHeaders spread order corrected so the synthesized
marker is always last and cannot be overridden by a caller. A call site in
codex-handler.ts passes retry-after via extraHeaders, so the param itself is kept;
the ordering fix makes the marker non-overridable by construction. The param
deletion is listed in Observed, not fixed.
I-058: respondJson docblock now scopes the "always synthesized" premise to the
translating (Codex) leg. The Anthropic passthrough leg's upstream responses are not
marked by this helper; only relay-generated errors on that leg are synthesized. The
module name is generic so the scoping note is load-bearing for future providers.
I-060: REJECTED_UPLOAD_DRAIN_MS = 2_000 extracted as a named module-level const
colocated with drainRejectedUpload. The 2_000 literal was the only inline numeric
bound on a branch where every other bound is named. The constant is colocated (not
in SERVER_TUNING) because a later batch will move drainRejectedUpload to
provider-transport.ts and the constant must travel with it. The B7 test window
(1.5-3 s) remains valid.
Co-Authored-By: Claude <noreply@anthropic.com>
… table, removal not deprecation, new chokepoints
I-003 (CRITICAL doc contradiction): replace the two-mechanism
deprecation story with the single LEGACY_KEY_ENTRIES table where
both moved and removed kinds hard-error via detectLegacyConfigKeys;
purge five dead identifiers (DEPRECATED_KEYS, detectDeprecatedConfigKeys,
DeprecatedConfigKey, LoadConfigResult.deprecatedKeys, config_key_deprecated,
attachClientErrorHandler, LEGACY_KEY_MOVES) from frontmatter, Key Files,
Anti-Patterns, and Gotchas.
I-065: fix four overloaded_error references — it was removed from
AnthropicErrorType; upstream 5xx incl. 529 maps to api_error via
upstreamStatusToAnthropicError.
Riders:
- anthropic.headerTimeoutMs and anthropic.streamIdleTimeoutMs: not
deprecated/silently ignored — removed (0.3.0, ADR-010); loadConfig
returns err; applies ADR-010, ADR-006, PF-023.
- I-011 (keepalive false positive): add sentence that pooled outbound
sockets carry OS TCP keepalive via Agent { keepAlive: true }; no
explicit setKeepAlive call needed.
Additional corrections per code audit:
- renderToken strips all C0/DEL/C1 control chars, not just backslash-r-backslash-n
- applyInboundPolicy replaces attachClientErrorHandler (unified entry point)
- SYNTHESIZED_HEADER/SYNTHESIZED_MARKER from src/errors.ts as single chokepoint
- src/inbound-policy.ts added to directories and Key Files
- index.md keyword cache synced to corrected frontmatter
- reasoningCache is z.strictObject; ProviderFileConfig type gone
…0.1.0->0.3.0 migration note; end-state Fixed bullets I-020: all four doc sites (config.ts JSDoc, README table, README note, CHANGELOG 0.3.0 Fixed bullet) updated from "TCP connection establishment" to "DNS resolution and TCP establishment". Upgrade note added: operators with a slow resolver may now see connectTimeoutMs 504s that 0.2.0 would have passed through. I-046: two operator caveats added to README after the Anthropic-leg-only note: no effect on pooled sockets (steady-state is warm), and TLS negotiation is not covered by the budget (connectTimeoutMs ends at the TCP connect event). I-023: drain bullet (~:92) deleted by rider; socket-leak bullet (~:68) deleted by rider. I-023 PF-019 removal and PF-022 addition are no-ops on deleted bullets. I-032: bracketed note appended after both instruction sites in the frozen 0.2.0 section (code block and migration table). Quoted error message at ~:144 verified against v0.2.0:src/config.ts and matches exactly (same prefix, same LEGACY_KEY_MOVES entries, same codex.models phrasing). Left untouched as a frozen historical description. Rider (PF-023 / end-state): four intra-branch Fixed bullets deleted after checking v0.2.0 code; one 0.2.0-symptom bullet rewritten as user-facing: - "client abort spurious warn" (narrates settled=true): DELETED. v0.2.0 correctly suppressed via if (responded) return in error handler. - "client abort leaks socket" (narrates settle() latch): DELETED. v0.2.0 correctly called upstream.destroy() via if (!res.writableFinished). - "408 reported as 400" (narrates clientError defect): DELETED. v0.2.0 had no clientError listener; 408 behavior change folded into SERVER_TUNING Changed bullet. - "drainRejectedUpload 2s bound fires" (v0.2.0 had no drain): DELETED. Detail folded into 413 Changed bullet. - "pre-header timeout spurious warn": KEPT and rewritten. v0.2.0 DID exhibit this (responded=false on pre-header path; destroy() caused unconditional error log). Applies PF-023; cites PF-019 (connect-phase, in config.ts JSDoc). Co-Authored-By: Claude <noreply@anthropic.com>
…lared content-length; bounded drain
I-035: bufferBody's outcomes are now a switch with a `const _exhaustive: never`
default, matching the discipline the Route union already gets 40 lines below in
the same function. The `client_disconnected` arm is explicit: no response can
reach a client that is gone, so nothing is written to `res`. Proven by adding a
third BufferBodyError variant temporarily — tsc reports TS2322 at the default
arm, "Type '{ readonly kind: "probe_variant"; ... }' is not assignable to type
'never'"; before this change it compiled clean and hung the request.
Such a request is also no longer recorded as a success. res.statusCode at close
time is Node's 200 initialiser, not a status the relay sent, so
`request_complete status=200` rendered an abandoned upload identically to a
served one. The close handler now emits `client_disconnected` (path, route,
model, latencyMs — no status) when no response ever reached the client.
Measured on Node 22.22, `res` "close" fires before `req` "error", so the
decision belongs in the close handler, not in the dispatch arm. Control B8.
I-037: a declared Content-Length over limits.maxBodyBytes is answered 413
before a body byte is read, rather than after buffering a full maxBodyBytes —
a 40 MiB upload paid 32 MiB of buffering to reach a 413 that was already
decided. The client-visible outcome is byte-identical (413, request_too_large,
synthesized marker) and the origin rejects on the declared length too, so the
relay's shape is unchanged (applies ADR-010). Chunked and undeclared bodies
keep the streaming cap. Controls B9: the declared case, plus a chunked case
that pins the streaming cap now that every other 413 test takes the early path.
I-038: (a) bufferBody's data listener is detached on rejection; it re-entered
the rejected branch for every chunk the drain still had to read. (b)
drainRejectedUpload gains REJECTED_UPLOAD_DRAIN_BYTES (32 MiB) beside
REJECTED_UPLOAD_DRAIN_MS. Time did not bound volume: measured on this loopback,
a client at line rate pushes ~1 GB through the drain in ~215 ms, so the 2 s
window admitted gigabytes per rejected request. The bound is a constant rather
than the configured maxBodyBytes — an operator who lowered the cap would
otherwise have the relay destroy every socket with more than the cap still in
flight, which is the RST the drain exists to prevent. Control B10.
I-019: the 11-line changelog above the rejection handling ("was incorrect",
"was removed because…") is replaced by end-state prose, as is the
drainRejectedUpload contract and the createProxyServer wiring note. Reasoning
that lives in the ledger is cited by ID instead of retold.
Every new control was proven RED before the fix (avoids PF-011):
B8 request_complete status=200 present, no client_disconnected record
B9 "no reply within 3000 ms — the relay is still reading a body whose
outcome is already decided"
B10 "the client got 1003487345 bytes onto the socket (ceiling 209715200,
declared 1000000000)"
B7's 1.5-3 s time bound and the 8 MiB 413-delivery race stay green.
… full model on unknown qualifier; document route values + client_disconnected
I-022, I-059 (applies ADR-010, avoids PF-023): `ambiguous` and `unknown_provider`
dispatch arms now assign `route = "anthropic:ambiguous"` and `route = "anthropic:fallback"`
respectively, so request_complete is distinguishable from intended Anthropic passthrough
in post-hoc log analysis. Both retain the `anthropic` prefix for leg-level filtering.
I-051: `unknown_provider_qualifier` warn now logs the full as-requested model name
(e.g. "kimee:k2") via `model ?? decision.qualifier`, not just the qualifier ("kimee").
I-043 + rider: README Logging section gains a `route` value reference table and a
log-events table (including `client_disconnected`). CHANGELOG [0.3.0] gains:
- `### Added` bullets for `client_disconnected` event and the two new route values
- `### Changed` entry listing the three retired route values (`rate_limited`,
`ambiguous`, `unknown_provider`) with upgrade guidance
Test: routing-behavior.test.ts RED→GREEN controls confirm the new route values on
request_complete for both fail-open arms; model assertion updated to "kimee:k2".
Co-Authored-By: Claude <noreply@anthropic.com>
…scan and router comments I-044 (avoids PF-006): replaced the "will 404 at the origin" rationale in both agent-scan.ts and router.ts with the accurate discriminator: - unknown_provider / unknown_qualifier → "info" because an unknown colon prefix may be a legitimate future model name; the relay forwards unchanged and the request works at runtime, so a doctor `fail` would produce false CI failures. - ambiguous → "fail" because two providers claiming the same family name is a subswitch-derived config defect that requires operator action to resolve, not a passing runtime situation. Routing behaviour and severities are unchanged (avoids PF-006). Co-Authored-By: Claude <noreply@anthropic.com>
… status assertion
I-006: nothing constructed an upstream failure AFTER response headers were
relayed — every other upstream-failure control fails before the response
callback runs, and the nearest neighbour destroys an IDLE pooled socket. B11
kills the origin socket with the body half-relayed and pins that the client
stream terminates as an error rather than hanging or reporting a clean EOF.
Proven RED against the three named mutations applied together (delete
upstreamRes.on("error", () => res.destroy()) plus both else { res.destroy(); }
arms neutered): B11 fails on "still open 3000 ms later" while the other 30
tests in the file stay green — the hang ADR-010 says a relay must not produce.
I-007: B6's status assertion sat inside a bare catch whose comment claimed it
was there for AbortError, but controller.abort() was the last statement in the
try, so the only thing it swallowed was the AssertionError. All three leak
assertions compare against 0, which a relay that never contacted the origin
also satisfies. The status check is hoisted out and upstreamConnections counts
connections at the origin, asserted === ABORTS (avoids PF-011, PF-012).
Proven RED, both halves independently:
- relay stubbed to answer 502 without contacting the origin: HEAD's B6 stays
green, the hoisted assertion goes red on 502 !== 200
- relay stubbed to answer 200 locally: status and all three zero-comparisons
pass, only upstreamConnections fires (0/5)
B6 still turns red on the PF-022 defect it was built for (5 of 5 held).
…estroy on the unbuffered 504
I-033: req.on("error") destroyed the upstream without claiming the latch, unlike
the res "close" sibling five lines above. On the unbuffered path that left the
settlement open for the ECONNRESET destroy() raises a tick later, so the upstream
error handler could win, log anthropic_upstream_error, and write a 502 for a
request the origin never failed — a client fault reported as an origin fault
(avoids PF-022, applies ADR-010). Now settles first, exactly like the sibling.
B12 is the control. It emits the client error directly rather than killing the
client socket: killing the socket also fires res "close", which on Node 22 lands
first, claims the latch, and masks the defect on most runs. The control removes
the race rather than betting on it. Proven RED against the unfixed handler —
one anthropic_upstream_error warn plus a 502 written to a live client.
I-079: drainRejectedUpload, REJECTED_UPLOAD_DRAIN_MS and REJECTED_UPLOAD_DRAIN_BYTES
move from server.ts to provider-transport.ts and are exported; server.ts imports
them for the 413 path and the passthrough leg calls the helper on the unbuffered
504 path, unpiping first because req.pipe() already set _consuming and _dump()
cannot rescue it. The FLAGGED — NOT FIXED marker is gone with the defect.
The name stays drainRejectedUpload: both callers are requests the relay rejected
with a synthesized error while the upload was still in flight, and renaming would
churn four prose sites in server.ts and three in the test suite for no gain.
Measured before the fix: the 504 is delivered whole, and the connection is then
held with the client still uploading into a body nothing will read — Node cannot
parse a next request out of an unconsumed body, so the socket sat open past the
8 s give-up, reclaimable only by server.requestTimeout (600 s). After: closed at
2267 ms by the drain's time bound, response intact. B13 pins both halves and is
proven RED with the drain call removed (still open at 8 s).
B7 and B10, the two opposite controls on the helper's bounds, stay green across
the relocation.
I-028: the state machine had outcomes labelled 2, 3 and 4 and no 1 — the response callback claimed the latch unlabelled, so grep returned three hits for four states. Outcome 1 is labelled and the four-item inventory now sits next to the settle() declaration, where it says what the latch answers and what it does not (teardown has its own predicate). The two sites that intentionally discard settle()'s return read `void settle()`, so a discard is no longer indistinguishable from an omission. I-019 (passthrough half): the 17-line PF-022 restatement above the 5-line close handler is rewritten in end-state voice — what the handler guarantees and why — citing PF-022 by ID for the history rather than retelling it. The prose comment on the response callback said "strips x-subswitch-synthesized"; it names SYNTHESIZED_HEADER instead, so the ADR-008 chokepoint holds in the comment too. Swept the rest of the file for transition narration; the only remaining hit describes what the code does, not what it used to do.
…502 path too
I-079 sibling: the upstream.on("error") handler had the identical unbuffered
no-drain defect the 504 path had. On the unbuffered path (body === undefined)
the client's upload is piped into the upstream; when the upstream connection
fails the relay wrote its 502 but never unpiped or drained the request.
Node cannot parse the next request out of a body it never consumed, so the
socket was held until server.requestTimeout (600 s) with the client still
pushing into it — identical to the 504 wedge measured in commit 9e75b2b.
Fix: add `req.unpipe(upstream); drainRejectedUpload(req)` guarded by
`body === undefined`, mirroring the timeout handler exactly.
B14 pins it: a non-/v1/messages POST with a large body against a loopback
origin that accepts TCP then destroys immediately (deterministic ECONNRESET →
502, no TEST-NET-1 topology dependence). The connection was still open at 8 s
(RED), and now closes at ~2 s (GREEN — drain time bound). B13/B7/B10 stay green.
Applies ADR-010 (drained intact error delivery over RST/wedge). Avoids PF-011
(control proven RED before GREEN).
…in would never receive I-047. The relay binds 127.0.0.1, requires no authentication, and holds the operator's Codex OAuth material, so reachability was its only access control — and DNS rebinding defeats reachability. A page served from http://evil.test:4141 that rebinds to 127.0.0.1 is same-origin with the relay as far as the browser is concerned: the response is fully READABLE, and gpt-* names route to the Codex leg with the victim's ~/.codex/auth.json attached. dispatch() now consults hostGateVerdict() (inbound-policy.ts) before routing and before body buffering, for every path and method including /__subswitch/*. The Host must name a loopback address; an Origin, when present, must be a loopback origin (browsers always send one cross-origin; Claude Code and curl send none). - 403 / permission_error, not 421: the origin never emits 421, while Anthropic does emit 403/permission_error (applies ADR-010 — a Host naming a domain this relay does not serve is a request the origin would never have received, so refusing it is not a transparency violation, but the refusal must wear a shape the origin could produce). The body renders through toAnthropicErrorBody (applies ADR-008) and never reflects the rejected value; that value reaches the operator's log only, lower-cased, charset-restricted and capped at 64 chars. - The loopback predicate is deliberately NOT config.ts's isLoopbackHost: its startsWith("127.") arm accepts 127.0.0.1.evil.test — the nip.io/sslip.io rebinding-domain shape — so reusing it would have shipped a gate that admits precisely the attack it exists to stop. The wire-side predicate accepts localhost, ::1, and 127.0.0.0/8 in dotted-quad form only, and is pinned by a dedicated control in both test files. - The authority parser refuses what it cannot read rather than guessing: a naive /:\d+$/ strip turns the bare IPv6 literal ::1 into ":". [::ffff:127.0.0.1] unwraps to 127.0.0.1, being the same address. - A rejected upload goes through drainRejectedUpload, exactly like the 413 path, so a client still mid-POST reads the whole 403 instead of taking an RST. Every control was proven RED against HEAD first (avoids PF-011): the foreign-Host requests were forwarded and answered 200, the foreign-Host health probe answered 200, and the Codex-routed one reached the auth layer (401 != 403). 696/696 green afterwards; tsc --noEmit clean.
….0.1.evil.test no longer passes ADR-009 vetting
The old predicate used hostname.startsWith("127."), which admitted
127.0.0.1.evil.test — an attacker-registrable domain. A config pointing
anthropic.baseUrl or oauthTokenUrl at http://127.0.0.1.evil.test/ started
cleanly under ADR-009 startup vetting and forwarded OAuth credentials in
cleartext to that domain.
Fix: tighten isLoopbackHost to exact loopback forms only:
- localhost (case-insensitive; new URL() normalises to lowercase)
- ::1 / [::1] (IPv6 loopback; WHATWG URL .hostname preserves brackets)
- 127.x.y.z in strict dotted-quad: all four decimal octets, each 0-255,
no trailing labels (the ^…$ anchors are load-bearing)
Refused: 127.0.0.1.evil.test, 127.1, 2130706433, foo.localhost,
localhost.evil.test, 127.256.0.0, ::ffff:127.x.y.z.
Also fixes the pre-existing [::1] URL regression: new URL("http://[::1]:3000")
.hostname returns "[::1]" (brackets preserved by WHATWG URL); the old
hostname === "::1" check never matched this form. Bracket-stripping before
comparison fixes both the unit-test path (bare "::1") and the URL path ("[::1]").
Adds CONFIG_IPV4_DOTTED regex (parallel to inbound-policy.ts's own copy —
kept separate per ADR-009/PF-011: different trust boundaries, independent
rigor, neither inherits relaxations from the other).
RED controls run against unfixed code confirmed wrong-clean results for
127.0.0.1.evil.test on both anthropic.baseUrl and oauthTokenUrl paths.
19 new tests added (unit + ADR-009 integration); full suite 75 → 94, 0 fail.
Applies ADR-009, avoids PF-011.
This was referenced Aug 19, 2026
… client_disconnected
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Integration branch combining PRs #29, #30, #32, #31 (merged in that order with
--no-ffso PR provenance stays visible in history); it exists so the whole set can be reviewed together rather than four times in isolation.Do not merge the four constituent PRs. They remain open and should be closed, not merged, once this PR lands — their commits are already contained here.
Constituent PRs
#29 — fix(codex-auth): replace lexicographic timestamp compare with refresh-token identity check
>compare onlast_refreshinpersistTokenswith a refresh-token identity check, so the concurrent-refresh guard no longer depends on the Codex CLI emitting byte-identical ISO-8601 formatting.Date.parsenumeric compare as a secondary ordering signal for divergent timestamp formats.fileIsNeweris true butmaterialFromfails on the newer file, the request is now served from the just-refreshed in-memory tokens instead of falling through and clobbering the CLI's rotated refresh token.#30 — fix(passthrough): bound connectTimeoutMs to TCP establishment; fix duplicate error log
'socket'handler, soconnectTimeoutMsactually bounds the TCP handshake (previously deferred to after connect, so it never fired — a blackholed IP took ~75 s instead of the configured budget).connectTimeoutMs(TCP establishment),headerTimeoutMs(connect → response headers, 660 s default),streamIdleTimeoutMs(headers → stream end).settledflag so exactly one warn event is logged per timeout, eliminating the duplicateanthropic_upstream_error.#32 — fix(passthrough): drop retry; complete synthesized-header coverage
synthesizedHeaders(...)helper so every relay-generated response carriesx-subswitch-synthesized: 1by default and new synthesized sites are correct without the author remembering.x-subswitch-synthesizedfrom proxied upstream responses so an origin cannot impersonate the relay marker.anthropic_upstream_errorwarn on normal client aborts.#31 — feat: native passthrough behavior — byte-budget admission, fail-open routing, correct error types
maxInFlightBytes,maxQueueDepth,maxQueueWaitMs); queue exhaustion returns HTTP 529 overloaded_error, the documented Anthropic status, instead of the undocumented 503.anthropic.maxUpstreamSocketsdefault 32 → 256 so the socket pool is not a hidden concurrency cap once admission is byte-based.request_too_large, matching Anthropic's own taxonomy.Merge conflict resolutions
Seven conflict hunks across three files.
src/config.tsauto-merged cleanly, as expected.src/server.ts— hunk 1:synthesizedHeadershelper vsSlotError/QueueEntrytypesPurely additive collision — both sides inserted new declarations at the same location. Resolution: union. Kept #32's
synthesizedHeadershelper and #31'sSlotErrortype andQueueEntryinterface. Nothing dropped.src/server.ts— hunk 2: the concurrency-gate response (composed, not "take one side")This was pre-identified as "take #32's version". Reality differed — see the discrepancy note below. The two sides were:
res.writeHead(503, synthesizedHeaders())— has the marker, wrong status.res.writeHead(529, { "content-type": "application/json" })— right status, no marker.Resolution: composed to
res.writeHead(529, synthesizedHeaders())with #31's reworded message. Taking either side wholesale would have lost one half: HEAD's version would have discarded #31's deliberate 503 → 529 taxonomy fix (and would have contradicted the surviving comment one line above, which already reads "529 overloaded_error (not 503, per Anthropic taxonomy)"); #31's version would have dropped the synthesized marker from a relay-generated response.src/server.ts— hunk 3: the 413 response (composed)Exactly as pre-identified. The two sides were:
synthesizedHeaders({ connection: "close" })+invalid_request_error.{ "content-type": "application/json", connection: "close" }+request_too_large.Resolution: composed —
res.writeHead(413, synthesizedHeaders({ connection: "close" }))with error typerequest_too_large, plus #31's explanatory comment. The marker exists only on #32 and the correct error type only on #31, so a naive ours/theirs loses one half either way. Both halves are load-bearing: the marker tells operators the 413 came from the relay, andrequest_too_largeis what clients keying on Anthropic's taxonomy expect.src/server.ts— hunk 4:unknown_providerroute armNot pre-identified. #32 kept a relay-invented 400 (wrapped in
synthesizedHeaders()); #31 replaced it with fail-open forwarding to Anthropic. Resolution: took #31. Fail-open is a headline feature of #31, the surviving comment directly above the hunk describes fail-open behavior (so keeping the 400 would leave the code contradicting its own comment), andtest/integration/routing-behavior.test.tsassertsstatus !== 400for this path. No marker is lost, because the synthesized response itself no longer exists at this site.CHANGELOG.mdTwo conflicts (one when merging #30, one when merging #31), both purely additive. Resolution: union — #29's and #30's bullets concatenated under
### Fixed, then #31's### Changed/### Added/### Deprecatedsections appended, all under the existing## [0.2.1]heading. Nothing dropped from any of the four PRs.README.mdOne conflict in the config table, resolved field by field rather than by side:
anthropic.connectTimeoutMs— kept fix(passthrough): bound connectTimeoutMs to TCP establishment; fix duplicate error log #30's more precise "TCP connection establishment timeout" wording.anthropic.headerTimeoutMs— kept; exists only on fix(passthrough): bound connectTimeoutMs to TCP establishment; fix duplicate error log #30 and would have been silently dropped by taking feat: native passthrough behavior — byte-budget admission, fail-open routing, correct error types #31's side.anthropic.streamIdleTimeoutMs— kept fix(passthrough): bound connectTimeoutMs to TCP establishment; fix duplicate error log #30's fuller "(headers→stream-end, reset by every chunk)" description.anthropic.maxUpstreamSockets— took feat: native passthrough behavior — byte-budget admission, fail-open routing, correct error types #31's256default, verified against the mergedsrc/config.ts(.default(256)).The byte-budget admission fields (
maxInFlightBytes,maxQueueDepth,maxQueueWaitMs) and thex-subswitch-synthesizedsection were outside the conflict region and are present and intact.Fixes beyond conflict resolution
Three stale references that the merge itself created — the 503 → 529 status change from #31 invalidated text written by #32 that git merged without conflict, since the lines were not adjacent:
CHANGELOG.md— fix(passthrough): drop retry; complete synthesized-header coverage #32's synthesized-header bullet enumerated "503 concurrency gate"; corrected to 529.README.md— thex-subswitch-synthesizedsection listed "503 (concurrency gate)"; corrected to 529.test/integration/passthrough.test.ts— the L3 section header comment listed 503; corrected to 529. Comment only, no assertion changed.The historical 503 references in the
## [0.2.0]CHANGELOG section and #31's own "the previous 503 was not a documented Anthropic status code" note were intentionally left alone — both are accurate as history.No test assertions were modified, and no test was weakened or skipped.
Validation
Run on the merge commit, sequentially, with nothing else running:
npm run typechecknpm testlintscript exists in this repo;npm run checkistypecheck && test, both of which are covered abovePer-branch counts before the merge were #29 612, #30 614, #32 621, #31 624. The merged branch is at 639 — above the 624 floor, consistent with the union of each branch's added tests.