fix(proxy): survive a reload instead of cutting the response - #304
fix(proxy): survive a reload instead of cutting the response#304codeslake wants to merge 99 commits into
Conversation
8afb859 to
6cb7295
Compare
808cc80 to
41e2d96
Compare
8f7db16 to
cf1d7fe
Compare
|
Heads up: your branch has an accumulated 15 workflow runs in Practical impact on the queue: on the free-tier concurrency budget those runs are holding runner slots, and everything behind them is backing up. Two asks, both about future pushes rather than the accumulated ones (which need a maintainer with
Not blocking on your PR review — this is drafting-time hygiene, not a review finding. Ping when the hang is either reproduced locally or ruled out, and we'll figure out the next step from there. — Proxy Builder |
5406c3a to
a2363e1
Compare
…e wrong hop
Twelve findings from the review of this PR, plus one raised by cswap's pin
against the fix for the last of them. Each has a test that dies when its fix is
reverted.
THE DEPLOY ONES, which is why this is not a tidy-up:
otherHolderOn() compared process AGE only. Every incumbent outlives a process
that just started, so on every deploy the NEW code judged itself surplus, exited
0, and the OLD holder kept serving with nothing saying so. Now a holder is a
duplicate only when it is running the same code.
holderPidOn() answered "holder" on the mere presence of a run-service, which
made runningOurCode() unreachable: the holder always keeps a descriptor to the
listening socket, so the loop always returned before the fingerprint branch.
bindFailed() read no error code, so a bind that can NEVER work — an address not
on this host, a privileged port — took the "someone else has it" path, found no
incumbent to ask, and exited 0. A deploy that started nothing reported success.
Bind errors also carried libuv's errno through two hardcoded literals that named
EADDRINUSE and called everything else EACCES; util.getSystemErrorName is right
on both platforms and for every code.
The holder matched its child's release announcement against a RAW CHUNK while
the port line beside it was line-buffered. A chunk boundary inside "(handed
off)" reads a handover as a plain release, so the holder reclaims the port from
the successor already serving on it and spawns a second — the failure the
(handed off) marker exists to prevent, re-entered through the marker itself.
Ownership probes asked lsof about 127.0.0.1 while the bind honoured
CACHE_FIX_PROXY_BIND. Under any other address the probe matched nothing.
CACHE_FIX_PROXY_PORT=0 was rewritten to the legacy 9801 by `Number(env) || 9801`
("0" is a truthy string), while proxy/config.mjs read the same variable with
envInt and yielded 0.
THE SHUTDOWN ONES:
shutdown() had no re-entry guard although it is bound to SIGTERM, SIGINT and
SIGHUP, and a control-group stop delivers more than one. Each entry can put
another successor on fd 3. The window only exists while something is draining,
which a live session always is.
handle.close() always rejected on that path, because shutdown() closes the
server one line earlier and the second close reports ERR_SERVER_NOT_RUNNING.
Only the process.exit() inside .finally() beat the unhandled-rejection report.
THE HOP ONES:
/health.https_proxy published a configured candidate. resolveHop() falls
THROUGH the chain, so it named ":8118" while CONNECTs left via the second
fallback or via nothing at all. It now publishes the hop a resolve actually
used, and null when the chain was checked and found dead.
cswap's pin raised that the fix left one field carrying two meanings — a URL is
either measured or merely configured and a reader cannot tell. Split into
https_proxy_measured, and direct_last: a sticky ISO instant of the last direct
fall-through, under the name and for the reason the pin uses. A chain flaps back
within ~1s, so a point-in-time field cannot report the outage that happened.
hopAlive() and parseProxy() defaulted an https:// hop with no explicit port to
80, so a live TLS hop read as dead and the chain fell through past it.
CONNECT fell open to a direct dial with no way to refuse. Fail-open stays the
default on both ends of the chain — a hop restarting is back in ~1s and refusing
strands a session whose HTTPS_PROXY was baked at exec — but CACHE_FIX_REQUIRE_HOP=1
now exists for a deployment where the hop is a policy boundary rather than a cache.
Ref cnighswonger#304
Co-Authored-By: Claude <noreply@anthropic.com>
…open Self-review of the previous commit. The opt-in lived in forward-proxy.mjs and covered the two CONNECT paths only, while forwardRequest() — the relayed /v1/messages path, which is what this proxy exists for — kept dialling direct with the variable set. A door that closes the tunnel and leaves the main path open reads as closed and is not, which is worse than leaving it open honestly. THE OBVIOUS FIX IS WORSE, AND THAT IS WHY THIS COMMIT DOES NOT SHIP IT. Throwing from forwardRequest() is caught by handleMessages, but its catch opens with `if (abortController.signal.aborted) return`, and that signal is wired to clientReq's own "close" — which Node emits when the request BODY completes, not only when the client goes away. Measured with the guard in place: hop="" requireHop=true env="1" caught: no chain hop reachable | aborted=true | writableEnded=false POST /v1/messages -> TIMEOUT (10016ms) The client is still there and gets nothing. A leak that is honest beats a hang that reads as a refusal, so the guard stays off that path. The abort listener is a pre-existing defect, not one this introduced, and there is no evidence it has ever worked: instrumenting the same catch and running the existing "POST /v1/messages routes to upstream" case printed nothing at all — that test gets a real 401 and never enters the catch. Fixing it means changing streaming-abort semantics for every request, which is the highest-risk edit in this file and does not belong in a follow-up to a deploy that has already shipped. So the scope is recorded rather than hidden: requireHop moves to upstream.mjs beside resolveHop with the measurement in its comment, and the test asserts the relayed path is NOT refused, with a message telling whoever fixes the abort listener to come back and update both. No behaviour change from the previous commit. Ref cnighswonger#304 Co-Authored-By: Claude <noreply@anthropic.com>
handleMessages installs an abort so a client that gives up mid-SSE frees the upstream. It was keyed on clientReq's "close" — which Node emits when the request BODY has been consumed, i.e. on every request, immediately — so it aborted while the client was still sitting there, and the forwardRequest catch opens with `if (aborted) return`. Nothing was written back. Measured in reverse mode on the real /v1/messages path, against an upstream that refuses instantly, which is what a dead local hop does: before body at once -> HANG (6s client timeout) body delayed -> HANG after body at once -> 502 in 9ms body delayed -> 502 in 52ms Keyed on clientRes's close instead: it fires when the response finishes OR the connection is destroyed, so pairing it with writableEnded separates "we answered" from "the client hung up". Same change in handlePassthrough, which carried the identical line. This was found by measuring an exposure I had already dismissed as too risky to touch. The previous commit recorded it as a latent defect blocking a different fix; it is not latent, it is on the most ordinary upstream failure there is. WHAT THE TESTS DO AND DO NOT GUARD, because the difference matters: the 502 case dies when the listener is reverted to clientReq — mutation-checked the no-leak case does NOT die when the listener is deleted outright Two attempts at the second: client takes a frame then leaves (the pipe tears the upstream down by itself), and an upstream that accepts and never answers so no pipe exists (still freed). Both passed with the listener removed. So the listener may be doing nothing that socket teardown does not already do. It stays — "I could not demonstrate it matters" is not "it does not matter" — and the case is labelled as pinning the PROPERTY, not guarding the listener, so nobody reads it as coverage it is not. ALSO: the relayed probe added in 70ff998 dialled the real api.anthropic.com, because that test never set CACHE_FIX_PROXY_UPSTREAM and the default is the live host. That is the trap integrated.conf line 20 already warns about, and it took CI red on node 22 while bafabae with identical proxy code was green. It now runs against a local 418, on its own instance — pointing config.upstream at loopback for the whole case makes the CONNECT half read the tunnel target as the upstream and stop blind-tunnelling it, which failed the fail-open assertion for an unrelated reason. Ref cnighswonger#304 Co-Authored-By: Claude <noreply@anthropic.com>
The two branches answer the same question — is another process listening on the port we advertise — and they disagreed about what counts. /proc matched on the PORT alone (f[1] ends with :hexport, any local address); lsof pinned the 127.0.0.1 literal. The lsof branch is the only one a mac reaches, and two of three machines here are macs. So a proxy bound anywhere other than loopback read as "no successor" forever, and the outgoing proxy waited out its whole 30s ceiling on every handover instead of leaving as soon as its successor served. Found by sweeping for siblings of a fix already shipped: the launcher's two ownership probes were taught to honour CACHE_FIX_PROXY_BIND, and this third one in server.mjs was missed. That is twice in one day that a fix landed on the call sites in the diff and not on the ones a grep would have found, which is the class of mistake this sweep exists to catch. Matched to /proc rather than teaching /proc the address: a wildcard listener (0.0.0.0) serves loopback traffic but does NOT match an `-iTCP@127.0.0.1` query, so an address filter has a blind spot of its own — and it is the blind spot that errs toward "a successor exists", which would let a proxy leave an unowned port behind. The test spawns the wildcard listener in ANOTHER process. A self-owned one answers false either way, because the function excludes its own pid, so the first version of this case passed against the literal it was written to catch. Mutation-checked: restoring 127.0.0.1 fails it. NOT CHANGED, checked and deliberately left: forward-proxy.mjs connectUpstreamTLS defaults the upstream port to 443 regardless of scheme. It tls.connect()s unconditionally, so 443 is the right default for what that function does; defaulting by scheme would send an http upstream to port 80 over TLS, which is worse. The real oddity there is TLS to a plain-http upstream, and that is not this PR's to change. Ref cnighswonger#304 Co-Authored-By: Claude <noreply@anthropic.com>
… that did not
The previous commit shipped the abort fix with an honest note that the no-leak
half was unguarded: two attempts at a case for it both passed with the listener
DELETED outright, so it could not be claimed as coverage. It is guarded now, and
both earlier attempts were wrong for reasons worth keeping.
WHAT IT ACTUALLY DOES. forwardRequest wires the signal to upstreamReq.destroy().
When the upstream has not yet ANSWERED there is no pipe for socket teardown to
travel along, so the abort is the only thing that can free the connection:
with the listener dialled 1, live 1 at walk-away -> 0 after 2s
listener deleted dialled 1, live 1 at walk-away -> 1 after 2s
WHY THE FIRST TWO FAILED, both my own defects:
1. No premise. The case asserted only "0 connections at the end", which a
proxy that never dialled satisfies just as well. It carried an
`assert.ok(x || true, "")` placeholder I had left in — an assertion that
cannot fail. It now asserts that the proxy dialled AND that the connection
was live at the moment the client left.
2. Process-global contamination. With the premise added it STILL passed inside
proxy-server.test.mjs while the identical logic in a process of its own
separated cleanly — cached keep-alive agents, a forward-mode instance's
self-heal, another startProxy winding down. So it moves to its own file,
the same reason proxy-holder-handover.test.mjs is one case alone.
AND THE MUTATION EXPOSED A THIRD DEFECT IN THE TEST. With the listener deleted
it first died at the runner's 120s timeout reporting `pass 0 fail 0`: the leaked
connection kept h.close() draining, cleanup hung, and the assertion message that
had already fired was lost. Cleanup destroys the upstream sockets first now, so
the mutation fails in 2.5s with something readable. A case that discriminates
only by timing out is one nobody can act on.
No production change. Ref cnighswonger#304
Co-Authored-By: Claude <noreply@anthropic.com>
The review agent I had reported dead came back after 7.7h. Of its 15 findings
two were already closed by later commits; these are the live ones that are mine.
THE PORT-0 FIX CREATED THIS. Removing `Number(env) || 9801` let
CACHE_FIX_PROXY_PORT=0 through, and two sites below the bind still passed the
REQUESTED port where the BOUND one is required — while the gap and standby a
couple of hundred lines up already used `this._port`.
publishFingerprint(port) wrote cache-fix-proxy-0.sha256, so
runningOurCode(<bound>) from any other
launcher finds nothing and every port-0
install on the box collides on one file
CACHE_FIX_HELD_PORT: String(port) told the child "0", so its self-heal would
respawn on a DIFFERENT ephemeral port and
strand every session on the served one,
and successorServing("0") can never answer
Measured with CACHE_FIX_PROXY_PORT=0: bound 43557, record named …-0, child told
0. After: record named for the bound port, child told the bound port. Both
halves mutation-checked separately.
TWO DEFECTS IN MY OWN NEW TESTS, both the process-global class:
The first cut asserted on /tmp/cache-fix-proxy-0.sha256 — a GLOBAL path. It
passed alone and failed in the full suite, because something else on the box
had created it. An assertion on a shared path measures the machine's history,
not the code. The holder now gets a private TMPDIR.
`announces its release exactly once` was starving its own file. node runs a
describe's subtests concurrently, and that case adds a full run-service holder,
a proxy child, an in-flight connection, a 3s settle and a cleanup loop that
SIGHUPs every pid on its port. The agent measured it: 7 full runs, 3 failures,
all in that file and varying between cases, against 0 in 3 with the case
excised. It moves to its own file — the remedy proxy-holder-handover.test.mjs
already applies to itself, for the same reason, in its own header. Still
mutation-checked in its new home.
Full suite now 3 consecutive runs, 0 failures, 1854/1853.
ALSO, from cswap's pin: a tripwire on the CONNECT case, because an assertion
that fires on `[]` has already discarded the evidence that would narrow it.
Every endpoint records now, so a failure says which was touched — measured
`["UPSTREAM"]` when the tunnel is aimed there, `[]` when it reached none. The
comment says plainly that `[]` still does not name the third case (the proxy
MITM'ing the target itself), because narrowing is not naming.
STILL OPEN, recorded not fixed: CACHE_FIX_REQUIRE_HOP closes two of four
fall-open egress paths. bin/gap-relay.mjs direct() does not consult it at all,
and that is the tunnel that carries traffic precisely when the proxy is down.
The pin's own _blind_tunnel walks its chain per hop, treats a non-200 as
"refused BY this hop", and reaches direct only when none will carry — with the
refusal traced. Their advice, which this does not yet implement: closing on
no-hop trades an invisible fall-open for an invisible outage.
Ref cnighswonger#304
Co-Authored-By: Claude <noreply@anthropic.com>
It read the FIRST usable candidate at startup and fell straight to a direct dial
when that one would not carry, so a configured second hop was never tried.
proxy/upstream.mjs resolveHop() walks the whole list — one chain carrying two
definitions of itself, and the relay's copy is the one that runs precisely when
the proxy is down.
Measured, three recording endpoints and a relay per case:
hop1 dead, hop2 alive before: ORIGIN (dialled past a hop that would carry)
after: HOP2, 1 refusal traced
both hops dead after: ORIGIN, 2 refusals traced
NOT A HARD CLOSE when none will carry, and that is cswap's pin's call rather
than mine. I had proposed consulting CACHE_FIX_REQUIRE_HOP here; they measured
that closing on no-hop trades an invisible fall-open for an invisible outage,
and this is the tunnel that carries traffic when the proxy is down — the most
expensive place to take one. Their _blind_tunnel does the same walk, treats a
non-200 as "refused BY this hop", and reaches direct only when none will carry.
Direct stays the last resort; the refusals are traced so it is not a silent one.
The trace uses the string proxy/upstream.mjs already emits — `hop <addr>
unusable` — so one grep reads both ends of the chain.
Also retracted: I had argued this hole mattered because a direct route's leaf
carries no Authority Key Identifier. The pin corrected it — that applies to a
MITM leaf, not a blind tunnel carrying the client's own TLS to the origin. The
hole is real for a different reason: a bypass nobody can see in the log.
THE FIRST MEASUREMENT OF THIS SAID THE FIX DID NOTHING. Zero endpoints touched,
zero traces, both scenarios. The relay listens on `srv.listen({ fd: 3 })`
because the holder hands it an already-bound socket, and the fixture spawned it
without one — so it never listened, and a broken instrument read exactly like a
broken fix. The test now asserts `gap-relay carrying` as a premise before
measuring anything, so the next person gets "nothing was measured" instead of a
false negative.
Both halves mutation-checked separately: reverting the walk fails both cases,
and keeping the walk while dropping the trace fails both too.
Suite 1856/1855/0. Ref cnighswonger#304
Co-Authored-By: Claude <noreply@anthropic.com>
Three of the review agent's minor findings, verified rather than relayed. All three were mine. AN EMPTIED CHAIN LEFT A HOP BEHIND. Both getters read the env per call, so the list can go away under a running proxy, and resolveHop's `if (!chain.length) return ""` skipped _lastHop entirely. Measured: resolved :40559, chain emptied, resolveHop returned "" and lastHop() still said :40559 — so /health went on naming an address no request could take. That is the exact lie the field was fixed to stop telling, re-entering through an early return the fix did not touch. _directLast is deliberately NOT stamped there. "No chain was ever configured" is not a fall-through, because there was no chain to fall through; stamping it would fire on every reverse-mode proxy that never had one and empty the field of the meaning it exists for. Asserted, so the distinction survives a refactor. A COMMENT OF MINE WAS A LIE. Three tests set CACHE_FIX_CHAIN_GRACE_MS after importing upstream.mjs and called the retry loop "not what is under test". CHAIN_GRACE_MS is a module-level const captured at import, so the assignment does nothing — measured, the case runs 2,616 ms, one full 2,500 ms default window. Set BEFORE the module loads it works: 28 ms. So the knob is fine for an operator, who sets it before the proxy starts, and the production code is unchanged; the comment is what was wrong, and a comment telling the next reader the wait is gone is worse than the 2.5 s. THE SCHEME-PORT INVARIANT COVERED TWO COPIES OF THREE. bin/gap-relay.mjs carries its own portOf() because it imports node:net and nothing else — it runs when the proxy is DOWN, so depending on proxy/ modules would let a broken one take the relay with it. The duplication is deliberate; leaving it unchecked was not, and it was already correct there, which is why the other two read as a regression against it. Mutation-checked: breaking gap-relay's copy now fails the case. Suite 1856/1855/0. Ref cnighswonger#304 Co-Authored-By: Claude <noreply@anthropic.com>
…e wrong hop
Twelve findings from the review of this PR, plus one raised by cswap's pin
against the fix for the last of them. Each has a test that dies when its fix is
reverted.
THE DEPLOY ONES, which is why this is not a tidy-up:
otherHolderOn() compared process AGE only. Every incumbent outlives a process
that just started, so on every deploy the NEW code judged itself surplus, exited
0, and the OLD holder kept serving with nothing saying so. Now a holder is a
duplicate only when it is running the same code.
holderPidOn() answered "holder" on the mere presence of a run-service, which
made runningOurCode() unreachable: the holder always keeps a descriptor to the
listening socket, so the loop always returned before the fingerprint branch.
bindFailed() read no error code, so a bind that can NEVER work — an address not
on this host, a privileged port — took the "someone else has it" path, found no
incumbent to ask, and exited 0. A deploy that started nothing reported success.
Bind errors also carried libuv's errno through two hardcoded literals that named
EADDRINUSE and called everything else EACCES; util.getSystemErrorName is right
on both platforms and for every code.
The holder matched its child's release announcement against a RAW CHUNK while
the port line beside it was line-buffered. A chunk boundary inside "(handed
off)" reads a handover as a plain release, so the holder reclaims the port from
the successor already serving on it and spawns a second — the failure the
(handed off) marker exists to prevent, re-entered through the marker itself.
Ownership probes asked lsof about 127.0.0.1 while the bind honoured
CACHE_FIX_PROXY_BIND. Under any other address the probe matched nothing.
CACHE_FIX_PROXY_PORT=0 was rewritten to the legacy 9801 by `Number(env) || 9801`
("0" is a truthy string), while proxy/config.mjs read the same variable with
envInt and yielded 0.
THE SHUTDOWN ONES:
shutdown() had no re-entry guard although it is bound to SIGTERM, SIGINT and
SIGHUP, and a control-group stop delivers more than one. Each entry can put
another successor on fd 3. The window only exists while something is draining,
which a live session always is.
handle.close() always rejected on that path, because shutdown() closes the
server one line earlier and the second close reports ERR_SERVER_NOT_RUNNING.
Only the process.exit() inside .finally() beat the unhandled-rejection report.
THE HOP ONES:
/health.https_proxy published a configured candidate. resolveHop() falls
THROUGH the chain, so it named ":8118" while CONNECTs left via the second
fallback or via nothing at all. It now publishes the hop a resolve actually
used, and null when the chain was checked and found dead.
cswap's pin raised that the fix left one field carrying two meanings — a URL is
either measured or merely configured and a reader cannot tell. Split into
https_proxy_measured, and direct_last: a sticky ISO instant of the last direct
fall-through, under the name and for the reason the pin uses. A chain flaps back
within ~1s, so a point-in-time field cannot report the outage that happened.
hopAlive() and parseProxy() defaulted an https:// hop with no explicit port to
80, so a live TLS hop read as dead and the chain fell through past it.
CONNECT fell open to a direct dial with no way to refuse. Fail-open stays the
default on both ends of the chain — a hop restarting is back in ~1s and refusing
strands a session whose HTTPS_PROXY was baked at exec — but CACHE_FIX_REQUIRE_HOP=1
now exists for a deployment where the hop is a policy boundary rather than a cache.
Ref cnighswonger#304
Co-Authored-By: Claude <noreply@anthropic.com>
…open Self-review of the previous commit. The opt-in lived in forward-proxy.mjs and covered the two CONNECT paths only, while forwardRequest() — the relayed /v1/messages path, which is what this proxy exists for — kept dialling direct with the variable set. A door that closes the tunnel and leaves the main path open reads as closed and is not, which is worse than leaving it open honestly. THE OBVIOUS FIX IS WORSE, AND THAT IS WHY THIS COMMIT DOES NOT SHIP IT. Throwing from forwardRequest() is caught by handleMessages, but its catch opens with `if (abortController.signal.aborted) return`, and that signal is wired to clientReq's own "close" — which Node emits when the request BODY completes, not only when the client goes away. Measured with the guard in place: hop="" requireHop=true env="1" caught: no chain hop reachable | aborted=true | writableEnded=false POST /v1/messages -> TIMEOUT (10016ms) The client is still there and gets nothing. A leak that is honest beats a hang that reads as a refusal, so the guard stays off that path. The abort listener is a pre-existing defect, not one this introduced, and there is no evidence it has ever worked: instrumenting the same catch and running the existing "POST /v1/messages routes to upstream" case printed nothing at all — that test gets a real 401 and never enters the catch. Fixing it means changing streaming-abort semantics for every request, which is the highest-risk edit in this file and does not belong in a follow-up to a deploy that has already shipped. So the scope is recorded rather than hidden: requireHop moves to upstream.mjs beside resolveHop with the measurement in its comment, and the test asserts the relayed path is NOT refused, with a message telling whoever fixes the abort listener to come back and update both. No behaviour change from the previous commit. Ref cnighswonger#304 Co-Authored-By: Claude <noreply@anthropic.com>
4d55a3b to
1b38317
Compare
handleMessages installs an abort so a client that gives up mid-SSE frees the upstream. It was keyed on clientReq's "close" — which Node emits when the request BODY has been consumed, i.e. on every request, immediately — so it aborted while the client was still sitting there, and the forwardRequest catch opens with `if (aborted) return`. Nothing was written back. Measured in reverse mode on the real /v1/messages path, against an upstream that refuses instantly, which is what a dead local hop does: before body at once -> HANG (6s client timeout) body delayed -> HANG after body at once -> 502 in 9ms body delayed -> 502 in 52ms Keyed on clientRes's close instead: it fires when the response finishes OR the connection is destroyed, so pairing it with writableEnded separates "we answered" from "the client hung up". Same change in handlePassthrough, which carried the identical line. This was found by measuring an exposure I had already dismissed as too risky to touch. The previous commit recorded it as a latent defect blocking a different fix; it is not latent, it is on the most ordinary upstream failure there is. WHAT THE TESTS DO AND DO NOT GUARD, because the difference matters: the 502 case dies when the listener is reverted to clientReq — mutation-checked the no-leak case does NOT die when the listener is deleted outright Two attempts at the second: client takes a frame then leaves (the pipe tears the upstream down by itself), and an upstream that accepts and never answers so no pipe exists (still freed). Both passed with the listener removed. So the listener may be doing nothing that socket teardown does not already do. It stays — "I could not demonstrate it matters" is not "it does not matter" — and the case is labelled as pinning the PROPERTY, not guarding the listener, so nobody reads it as coverage it is not. ALSO: the relayed probe added in 70ff998 dialled the real api.anthropic.com, because that test never set CACHE_FIX_PROXY_UPSTREAM and the default is the live host. That is the trap integrated.conf line 20 already warns about, and it took CI red on node 22 while bafabae with identical proxy code was green. It now runs against a local 418, on its own instance — pointing config.upstream at loopback for the whole case makes the CONNECT half read the tunnel target as the upstream and stop blind-tunnelling it, which failed the fail-open assertion for an unrelated reason. Ref cnighswonger#304 Co-Authored-By: Claude <noreply@anthropic.com>
The two branches answer the same question — is another process listening on the port we advertise — and they disagreed about what counts. /proc matched on the PORT alone (f[1] ends with :hexport, any local address); lsof pinned the 127.0.0.1 literal. The lsof branch is the only one a mac reaches, and two of three machines here are macs. So a proxy bound anywhere other than loopback read as "no successor" forever, and the outgoing proxy waited out its whole 30s ceiling on every handover instead of leaving as soon as its successor served. Found by sweeping for siblings of a fix already shipped: the launcher's two ownership probes were taught to honour CACHE_FIX_PROXY_BIND, and this third one in server.mjs was missed. That is twice in one day that a fix landed on the call sites in the diff and not on the ones a grep would have found, which is the class of mistake this sweep exists to catch. Matched to /proc rather than teaching /proc the address: a wildcard listener (0.0.0.0) serves loopback traffic but does NOT match an `-iTCP@127.0.0.1` query, so an address filter has a blind spot of its own — and it is the blind spot that errs toward "a successor exists", which would let a proxy leave an unowned port behind. The test spawns the wildcard listener in ANOTHER process. A self-owned one answers false either way, because the function excludes its own pid, so the first version of this case passed against the literal it was written to catch. Mutation-checked: restoring 127.0.0.1 fails it. NOT CHANGED, checked and deliberately left: forward-proxy.mjs connectUpstreamTLS defaults the upstream port to 443 regardless of scheme. It tls.connect()s unconditionally, so 443 is the right default for what that function does; defaulting by scheme would send an http upstream to port 80 over TLS, which is worse. The real oddity there is TLS to a plain-http upstream, and that is not this PR's to change. Ref cnighswonger#304 Co-Authored-By: Claude <noreply@anthropic.com>
… that did not
The previous commit shipped the abort fix with an honest note that the no-leak
half was unguarded: two attempts at a case for it both passed with the listener
DELETED outright, so it could not be claimed as coverage. It is guarded now, and
both earlier attempts were wrong for reasons worth keeping.
WHAT IT ACTUALLY DOES. forwardRequest wires the signal to upstreamReq.destroy().
When the upstream has not yet ANSWERED there is no pipe for socket teardown to
travel along, so the abort is the only thing that can free the connection:
with the listener dialled 1, live 1 at walk-away -> 0 after 2s
listener deleted dialled 1, live 1 at walk-away -> 1 after 2s
WHY THE FIRST TWO FAILED, both my own defects:
1. No premise. The case asserted only "0 connections at the end", which a
proxy that never dialled satisfies just as well. It carried an
`assert.ok(x || true, "")` placeholder I had left in — an assertion that
cannot fail. It now asserts that the proxy dialled AND that the connection
was live at the moment the client left.
2. Process-global contamination. With the premise added it STILL passed inside
proxy-server.test.mjs while the identical logic in a process of its own
separated cleanly — cached keep-alive agents, a forward-mode instance's
self-heal, another startProxy winding down. So it moves to its own file,
the same reason proxy-holder-handover.test.mjs is one case alone.
AND THE MUTATION EXPOSED A THIRD DEFECT IN THE TEST. With the listener deleted
it first died at the runner's 120s timeout reporting `pass 0 fail 0`: the leaked
connection kept h.close() draining, cleanup hung, and the assertion message that
had already fired was lost. Cleanup destroys the upstream sockets first now, so
the mutation fails in 2.5s with something readable. A case that discriminates
only by timing out is one nobody can act on.
No production change. Ref cnighswonger#304
Co-Authored-By: Claude <noreply@anthropic.com>
The review agent I had reported dead came back after 7.7h. Of its 15 findings
two were already closed by later commits; these are the live ones that are mine.
THE PORT-0 FIX CREATED THIS. Removing `Number(env) || 9801` let
CACHE_FIX_PROXY_PORT=0 through, and two sites below the bind still passed the
REQUESTED port where the BOUND one is required — while the gap and standby a
couple of hundred lines up already used `this._port`.
publishFingerprint(port) wrote cache-fix-proxy-0.sha256, so
runningOurCode(<bound>) from any other
launcher finds nothing and every port-0
install on the box collides on one file
CACHE_FIX_HELD_PORT: String(port) told the child "0", so its self-heal would
respawn on a DIFFERENT ephemeral port and
strand every session on the served one,
and successorServing("0") can never answer
Measured with CACHE_FIX_PROXY_PORT=0: bound 43557, record named …-0, child told
0. After: record named for the bound port, child told the bound port. Both
halves mutation-checked separately.
TWO DEFECTS IN MY OWN NEW TESTS, both the process-global class:
The first cut asserted on /tmp/cache-fix-proxy-0.sha256 — a GLOBAL path. It
passed alone and failed in the full suite, because something else on the box
had created it. An assertion on a shared path measures the machine's history,
not the code. The holder now gets a private TMPDIR.
`announces its release exactly once` was starving its own file. node runs a
describe's subtests concurrently, and that case adds a full run-service holder,
a proxy child, an in-flight connection, a 3s settle and a cleanup loop that
SIGHUPs every pid on its port. The agent measured it: 7 full runs, 3 failures,
all in that file and varying between cases, against 0 in 3 with the case
excised. It moves to its own file — the remedy proxy-holder-handover.test.mjs
already applies to itself, for the same reason, in its own header. Still
mutation-checked in its new home.
Full suite now 3 consecutive runs, 0 failures, 1854/1853.
ALSO, from cswap's pin: a tripwire on the CONNECT case, because an assertion
that fires on `[]` has already discarded the evidence that would narrow it.
Every endpoint records now, so a failure says which was touched — measured
`["UPSTREAM"]` when the tunnel is aimed there, `[]` when it reached none. The
comment says plainly that `[]` still does not name the third case (the proxy
MITM'ing the target itself), because narrowing is not naming.
STILL OPEN, recorded not fixed: CACHE_FIX_REQUIRE_HOP closes two of four
fall-open egress paths. bin/gap-relay.mjs direct() does not consult it at all,
and that is the tunnel that carries traffic precisely when the proxy is down.
The pin's own _blind_tunnel walks its chain per hop, treats a non-200 as
"refused BY this hop", and reaches direct only when none will carry — with the
refusal traced. Their advice, which this does not yet implement: closing on
no-hop trades an invisible fall-open for an invisible outage.
Ref cnighswonger#304
Co-Authored-By: Claude <noreply@anthropic.com>
It read the FIRST usable candidate at startup and fell straight to a direct dial
when that one would not carry, so a configured second hop was never tried.
proxy/upstream.mjs resolveHop() walks the whole list — one chain carrying two
definitions of itself, and the relay's copy is the one that runs precisely when
the proxy is down.
Measured, three recording endpoints and a relay per case:
hop1 dead, hop2 alive before: ORIGIN (dialled past a hop that would carry)
after: HOP2, 1 refusal traced
both hops dead after: ORIGIN, 2 refusals traced
NOT A HARD CLOSE when none will carry, and that is cswap's pin's call rather
than mine. I had proposed consulting CACHE_FIX_REQUIRE_HOP here; they measured
that closing on no-hop trades an invisible fall-open for an invisible outage,
and this is the tunnel that carries traffic when the proxy is down — the most
expensive place to take one. Their _blind_tunnel does the same walk, treats a
non-200 as "refused BY this hop", and reaches direct only when none will carry.
Direct stays the last resort; the refusals are traced so it is not a silent one.
The trace uses the string proxy/upstream.mjs already emits — `hop <addr>
unusable` — so one grep reads both ends of the chain.
Also retracted: I had argued this hole mattered because a direct route's leaf
carries no Authority Key Identifier. The pin corrected it — that applies to a
MITM leaf, not a blind tunnel carrying the client's own TLS to the origin. The
hole is real for a different reason: a bypass nobody can see in the log.
THE FIRST MEASUREMENT OF THIS SAID THE FIX DID NOTHING. Zero endpoints touched,
zero traces, both scenarios. The relay listens on `srv.listen({ fd: 3 })`
because the holder hands it an already-bound socket, and the fixture spawned it
without one — so it never listened, and a broken instrument read exactly like a
broken fix. The test now asserts `gap-relay carrying` as a premise before
measuring anything, so the next person gets "nothing was measured" instead of a
false negative.
Both halves mutation-checked separately: reverting the walk fails both cases,
and keeping the walk while dropping the trace fails both too.
Suite 1856/1855/0. Ref cnighswonger#304
Co-Authored-By: Claude <noreply@anthropic.com>
… save Two ways the standby destroyed what it was added to protect, both found by review and both measured. It took the first fallback proxy RAW. `fallbackProxyUrls()` drops our own address from that list for a reason the fallback suite pins — a request routed there comes straight back — and the shipped list can legitimately begin with self. So an armed relay forwarded to itself: on one CONNECT its descriptor count went 22 -> 8,195 -> 22,733 -> 29,814 and kept climbing. The address is destroyed rather than degraded, in the one state where nothing else is left to serve it. It now excludes its own address and reads the same candidates in the same order the proxy does, so a host wired with only CACHE_FIX_UPSTREAM_PROXY no longer has a hop the relay cannot see and would have dialled around. And the recovery path killed it. holderPidOn() nominated the first listener, and lsof returns ascending pid order, so the standby — spawned at bind, before the first proxy child — was always first. A holder killed while its child lived sent the child's self-heal to take the port over, which SIGHUP'd the standby and then bind-failed for its whole 20s against the child that actually held the address. The one process whose job is to survive a dead holder was destroyed by the recovery path, and the log named the wrong pid. Gap relays are now filtered out of that nomination, and stay eligible only when nothing else is on the port, so a lone armed standby is still releasable. Smaller ones from the same pass: the exit handler asked whether A standby existed rather than whether it was the one it was given, so a killed predecessor's late exit could null out a live successor and leave a standby the holder could no longer close; getsockname() was being read from the handle we had already closed, which handed the standby a held port of 0 under CACHE_FIX_PROXY_PORT=0 and would have armed it beside a live proxy; the /health reply half-closed with the mute timer already cleared, holding one descriptor per probe forever; and the standby carried holder identity markers in its environment for its whole detached life. Co-Authored-By: Claude <noreply@anthropic.com>
…ying about it A release nominated one incumbent and never asked again. That cannot free a port two of our own processes hold: signalling the proxy child releases its descriptor, and a socket stays LISTENING — and refuses a second bind — for as long as ANY descriptor to it remains, which is the standby's whole reason for existing. Measured: the child went, the bind then failed for its full 20s against a standby nobody had asked to leave, and the deploy exited 1 having left the address with no proxy on it at all. It now re-asks every 500ms and signals whoever is still there, so the port converges instead of deadlocking. The standby was reading its own parent too late. `process.ppid` is available tens of milliseconds after spawn, and a holder that dies inside that window has already been replaced by init — so the comparison was 1 against 1 forever, the standby never armed, and it went on holding a listening socket. That is an address that ACCEPTS AND HANGS, which is worse than the refusal this mechanism replaced. The holder now hands down its own pid. A hop that DROPS cost the kernel's connect timeout. The measured fall-through case was a hop that refused, which is instant; a firewalled one or a VPN that went down leaves every request waiting ~130s on linux and ~75s on darwin before the direct dial it would have fallen through to. Two seconds, the same deadline the standby's own probe uses, and cleared once the tunnel is established so an idle CONNECT is never touched. Self-exclusion now covers the address we are BOUND to, not just loopback, and the bare host for 80/443 where URL drops the default port. And the proxy child's own list keys on the HELD port: the holder tells it CACHE_FIX_PROXY_PORT=0, so the existing guard was excluding 127.0.0.1:0 while the address it actually serves stayed in the list. Two give-up lines said "releasing the port" while the standby kept it bound and carrying. An operator reading that expects ECONNREFUSED and a free port, and would misdiagnose exactly when the port refuses to be taken. They now say what is true, and the case that pins the behaviour is anchored on the fact rather than on the old wording. Co-Authored-By: Claude <noreply@anthropic.com>
…rd the tunnel edges A failed handover left the holder alive and unable to ever start a proxy again. clearTimeout() stops a pending restart but leaves the handle non-null, and spawnWhenReady() returns early on a non-null one — so the recovery path that puts the standby back also froze the ladder, silently, on a holder that goes on looking healthy. An orphaned standby whose address still answers is not a transient, it is where every machine sits once a holder has died and the lineage self-healed. Polling it four times a second forever cost the live proxy a connection every 250ms and bought nothing; it now backs off to 2s and returns to 250ms the moment the answer stops. Both tunnel edges now close their opposite: pipe() does not end a destination whose source was destroyed rather than ended, and between a CONNECT 200 and the first TLS byte neither side writes, so nothing errors either. A review reported 20 leaked descriptors from 20 aborted tunnels; I could not reproduce it here, with or without these lines — the relay's fd table stayed at its single listener either way — and the comment says so rather than claiming a measurement I do not have. They stay because this process exits(1) on an accept error, so at an EMFILE ceiling the standby would drop the last descriptor and take the address with it. The case that proves the address survives its own lineage now keeps the holder's stderr and asserts a standby is on the port before the kill. It flaked once and could not say which half had failed, because the launcher's "standby relay gone" line was being discarded by the fixture that most needed it. Co-Authored-By: Claude <noreply@anthropic.com>
A request arriving after the holder and its proxy were both killed waited about four seconds to be served. It was never refused — it queued in the backlog of a socket the standby keeps alive — but four seconds of waiting is still an interruption to the session paying it. The wait was the DECISION, not the work. Arming is a one-way door, so it needs evidence that nothing is serving; the evidence is silence on /health, and a live proxy answers that in about a millisecond. Two seconds was slack for a stalled event loop rather than a measurement, and two of them ran serially. Three windows at 250ms cost more independent observations and far less waiting: measured, the first request after a holder+child kill went 3,899ms -> 694ms, steady 2ms after, with the same one-standby-per-socket and SIGHUP-retires-it properties intact. The accept queue would have been better still — a socket whose acceptor is gone piles connections up, which is the symptom rather than a proxy for it — but darwin reports Recv-Q as 0 for a listening socket and two of three machines are macs. Measured before choosing: linux exposes it in /proc/net/tcp, `netstat -an` on the mac does not. Co-Authored-By: Claude <noreply@anthropic.com>
…ce it is proven free The proof was the outage. Every window the standby spent establishing that nothing was serving was a window in which nothing was serving, and the request that arrived in it paid the whole of it: 3,899ms with two 2s silence windows, 694ms with three at 250ms. It can only approach zero from above while the decision comes before the accept. So the decision goes away. The standby arms the moment its holder is gone and checks nothing first. Measured on the same shape, holder and proxy both SIGKILLed: 3ms, which is also the steady state — there is no gap left to measure. Everything off, including the fallback hop: 3ms. Being wrong is cheap here and being slow is not. The only way to arm wrongly is for a proxy to have outlived the holder, and then both accept — ours carrying to the same hop the proxy would have used, so those connections are served uncached rather than lost or delayed. That state also clears itself: the surviving proxy self-heals into a new holder, and a new holder takes the port by asking everything still on it to let go. One standby before that sequence, one after. An intermediate draft stood down on its own when a probe returned 200, and that was a regression, not a refinement: closing the server closes the descriptor, so the proxy that answered can die a moment later with nothing left to re-arm with. Measured on that draft, in the exact sequence this exists to survive: ECONNREFUSED. Yielding is the claimant's decision, made with SIGHUP, and it stays that way. Co-Authored-By: Claude <noreply@anthropic.com>
…s visible gap-relay.mjs sits in bin/, which proxy_tree does not cover, and holder_tree hashed a single file. So the relay was covered by nothing: a change to it landed on all three machines while the deploy printed "live proxy already on this code" and verify agreed. Three boxes ran the old relay and every instrument said OK. This is the same blind spot that made proxy_tree alone insufficient, one layer further down, and it has now been found twice by the same route — a deploy that claimed there was nothing to do. holder_tree hashes both files of its layer, and deploy.sh and verify.sh compute it the same way. Co-Authored-By: Claude <noreply@anthropic.com>
Adding gap-relay.mjs to the hash fixed one instance and left the class open: the launcher also imports ca-trust.mjs, which lives in the same directory and was covered by nothing — proxy_tree walks proxy/, and holder_tree named two files. A change confined to it moves no fingerprint at all, so every check would call a stale machine current, which is the exact symptom that exposed the relay. The list was the defect. holder_tree now walks bin/ — every .mjs, sorted, name and bytes — the way proxy_tree already walks proxy/. deploy.sh, verify.sh and the case that pins the field recompute it the same way, so a future divergence fails a test rather than hiding a deploy. cswap's pin hit the identical shape the same day: its daemon fingerprint hashed proxy.py while the daemon also imports _host.py. Two systems, one habit. Co-Authored-By: Claude <noreply@anthropic.com>
…dress Binding does not establish ownership — binding a port another process merely BOUND succeeds, which is what makes the gap listener possible — and the listen() test that does establish it only settles the question while somebody is LISTENING. With no child serving, every arriving run-service passes the same test and believes it is the holder. Measured on lmd42: 27 alive, ZERO listening, one per shell launched in that window, none able to take the port and none willing to leave. The existing holderPidOn() cannot see this, and that is the point: it answers "is the incumbent ours", which is true of every copy of us. A new question is asked once at startup — is one of us already doing this job — and the surplus one exits. Measured: six run-service against a proxy that never listens leaves one alive and five exited 0. Asked ONCE, before the first bind, and never for a holder that was handed its socket. Both restrictions are measured, not defensive: on every `listening` it also fired for handover successors, whose predecessor is legitimately older, and broke three cases that depend on a successor taking over; on every rebind it made a sole holder exit as "surplus" and 200 of 200 concurrent requests hung. The caller-side guard in dotfiles' wire.zsh (do not spawn when the port already answers) is the other half. Neither is sufficient alone: the wrapper is not the only thing that starts a holder, and a holder that is already running cannot stop one that has not looked. Co-Authored-By: Claude <noreply@anthropic.com>
The directory walk counted every .mjs in bin/, and the suite writes .test-launcher-<tag>.mjs and .test-fake-server-<tag>.mjs into that same directory while it runs. So the hash depended on WHEN it was taken: CI on node 18 had the holder publish ba5cbf0b4567 at startup and the case recompute a7a72ba4c005 a moment later, both correct for their instant, and the disagreement read as a stale holder. Dot-prefixed files are excluded now, in the launcher, in the case that pins the field, and in deploy.sh and verify.sh, which must all compute it the same way. A hidden file is not part of the shipped layer, and the fixture is the reason there are any. Local runs never showed it because concurrency decides whether a stand-in exists at the moment of the read; node 18's scheduling made it reliable there and invisible on 20 and 22. Co-Authored-By: Claude <noreply@anthropic.com>
…wo answered /health has two 503 authors — the gap relay carrying an address with no proxy behind it, and a proxy reporting failed extensions — and the status line alone tells them apart in neither direction. CI run 31137828018 failed node 18 with "the held port cut 6 connection(s) during the restart: ERR:503" and named neither author. The case does not reproduce locally: 9 runs green, 8 of them under saturating load, and the upstream repo refuses a rerun without admin rights, so that log was the only witness and it had already discarded the evidence. Co-Authored-By: Claude <noreply@anthropic.com>
…o a red run A failure message is published output. The probes now append the 503 body, and the gap relay's 503 body carries the hop it would forward to — so a variable that gives a child a hop and is not scrubbed puts an internal proxy's host:port into a run someone pastes into a public issue. Six fixtures scrubbed HTTPS_PROXY and its three siblings; none scrubbed CACHE_FIX_UPSTREAM_PROXY or CACHE_FIX_FALLBACK_PROXIES, which bin/gap-relay.mjs reads FIRST. Six hand-written copies is how one gets missed, so they now share HOP_ENV, and suite-collection pins the relay's own reads against it: a hop variable the relay learns and the list does not is red. It matches PROX, not PROXY — CACHE_FIX_FALLBACK_PROXIES contains no "PROXY", and a PROXY-shaped grep missed exactly that one while this was being written. Neither 503 body carries a credential (URL.host drops userinfo, measured) or an origin IP; the hop's address is the whole exposure. GitHub-hosted runners set no proxy, so this reached local runs and any future self-hosted one. The three remaining probes that returned a bare ERR:<code> now carry the body too, and the takeover assertion prints the codes it collected instead of only counting them — both were the same blindness in places a failure can land. Copying is how it returns, so that is pinned statically as well. Co-Authored-By: Claude <noreply@anthropic.com>
The node-18 CI red was our own gap relay. The holder re-opens it on the
child-death path and closes it as the successor spawns, and through that window
the address is OWNED and carrying real traffic while answering /health — and
only /health — with 503, because a 200 there would announce a proxy that does
not exist. The case counted those as cut connections.
Measured with the restart window forced to 400ms: 20 errors, 19 of them
{"carrying":"gap-relay"}. On this box the window is ~0 because the first death
after serving respawns with zero delay (firstAfterServing ? 0), which is why it
never reproduced in 13 local runs and only ever on a loaded node-18 runner,
where it stretched to ~120ms and let 6 probes in at 20ms spacing.
Why it kept coming back: this file held FOUR hand-rolled definitions of "is
this an outage" — two counting every error, one counting only ECONNREFUSED and
ETIMEDOUT (narrowed 2026-08-05 in 01a9b98 after the same class hit a sibling
case), and one written while chasing this. Fixing a case taught one of the four.
They are now one classify(): served, carrying, reset, refused, degraded. A
degraded proxy's 503 carries the same status line and the opposite meaning, so
it stays red.
Mutation-checked, and the check found a bug in the fix first: classify() returns
null for both a 200 and a carrying 503, and using it as "is this a 200" sent a
carrying body into JSON.parse. Three-way split now. With that fixed — window
forced open and the gap removed and the successor stalled: pass 0, fail 1. Two
earlier "the mutation does not kill it" reports were the instrument, not the
guard: only the `?` branch of firstAfterServing was patched and the run took
the `:` branch.
Also measured, and it is why refusals are not the thing to assert here: with the
gap removed entirely and 700 concurrent requests against a 3s window, all 700
were served. The holder's fd keeps the socket listening across the child's
death, so arrivals queue in the backlog. The gap is what ANSWERS during the
window, not what prevents refusal.
Co-Authored-By: Claude <noreply@anthropic.com>
`bytes >= BODY` is monotonic, so the hop's data handler called end() again on every read that landed after the threshold — and the threshold is crossed with body still in flight, because it counts the request line and headers too. Whether another read follows is pure timing. CI node 22 hit it (run 31146142838): "write after end", ERR_STREAM_WRITE_AFTER_END, uncaughtException thrown from that handler, while 18 and 20 passed. This box coalesces the writes and never split it in any local run of the case. Reproduced away from the suite with a 1KiB drip writer, both directions: unguarded -> UNCAUGHT ERR_STREAM_WRITE_AFTER_END, guarded -> end() once, no throw. The stream raises it asynchronously, which is why a try/catch around the end() call does not see it and the run dies as an uncaught exception — the same failureType CI reported. Co-Authored-By: Claude <noreply@anthropic.com>
Fixing the case that broke CI left the SHAPE alive. A socket may be ended once
and a data handler runs per read, so calling end() on the socket from inside its
own data handler bets that no second read arrives — and whether one does is how
the peer's writes were coalesced, how loaded the box is, which libuv version.
CI node 22 collected on that bet while 18 and 20 passed.
The tree held exactly two of these. One had just cost a red run; the other,
`s.on("data", () => s.end("pong"))` at proxy-holder-handover:445, was still live
and had simply not been unlucky yet. Both answer once now, and
suite-collection refuses a third.
Matched on the RECEIVER, not the word: `r.on("end", …)` a line below a data
handler registers a listener and calls nothing, and a looser pattern counted 18
sites with 17 of them that false shape. A guard that cries wolf 17 times out of
18 is a guard the next person deletes. The check also skips its own file, which
it flagged on the first run because it contains the pattern as data — the shape
of every static check that reads the directory it lives in.
Mutation-checked at both sites: drop either answer-once guard and the invariant
names that file and line; restore and it is green.
Co-Authored-By: Claude <noreply@anthropic.com>
…e wrong hop
Twelve findings from the review of this PR, plus one raised by cswap's pin
against the fix for the last of them. Each has a test that dies when its fix is
reverted.
THE DEPLOY ONES, which is why this is not a tidy-up:
otherHolderOn() compared process AGE only. Every incumbent outlives a process
that just started, so on every deploy the NEW code judged itself surplus, exited
0, and the OLD holder kept serving with nothing saying so. Now a holder is a
duplicate only when it is running the same code.
holderPidOn() answered "holder" on the mere presence of a run-service, which
made runningOurCode() unreachable: the holder always keeps a descriptor to the
listening socket, so the loop always returned before the fingerprint branch.
bindFailed() read no error code, so a bind that can NEVER work — an address not
on this host, a privileged port — took the "someone else has it" path, found no
incumbent to ask, and exited 0. A deploy that started nothing reported success.
Bind errors also carried libuv's errno through two hardcoded literals that named
EADDRINUSE and called everything else EACCES; util.getSystemErrorName is right
on both platforms and for every code.
The holder matched its child's release announcement against a RAW CHUNK while
the port line beside it was line-buffered. A chunk boundary inside "(handed
off)" reads a handover as a plain release, so the holder reclaims the port from
the successor already serving on it and spawns a second — the failure the
(handed off) marker exists to prevent, re-entered through the marker itself.
Ownership probes asked lsof about 127.0.0.1 while the bind honoured
CACHE_FIX_PROXY_BIND. Under any other address the probe matched nothing.
CACHE_FIX_PROXY_PORT=0 was rewritten to the legacy 9801 by `Number(env) || 9801`
("0" is a truthy string), while proxy/config.mjs read the same variable with
envInt and yielded 0.
THE SHUTDOWN ONES:
shutdown() had no re-entry guard although it is bound to SIGTERM, SIGINT and
SIGHUP, and a control-group stop delivers more than one. Each entry can put
another successor on fd 3. The window only exists while something is draining,
which a live session always is.
handle.close() always rejected on that path, because shutdown() closes the
server one line earlier and the second close reports ERR_SERVER_NOT_RUNNING.
Only the process.exit() inside .finally() beat the unhandled-rejection report.
THE HOP ONES:
/health.https_proxy published a configured candidate. resolveHop() falls
THROUGH the chain, so it named ":8118" while CONNECTs left via the second
fallback or via nothing at all. It now publishes the hop a resolve actually
used, and null when the chain was checked and found dead.
cswap's pin raised that the fix left one field carrying two meanings — a URL is
either measured or merely configured and a reader cannot tell. Split into
https_proxy_measured, and direct_last: a sticky ISO instant of the last direct
fall-through, under the name and for the reason the pin uses. A chain flaps back
within ~1s, so a point-in-time field cannot report the outage that happened.
hopAlive() and parseProxy() defaulted an https:// hop with no explicit port to
80, so a live TLS hop read as dead and the chain fell through past it.
CONNECT fell open to a direct dial with no way to refuse. Fail-open stays the
default on both ends of the chain — a hop restarting is back in ~1s and refusing
strands a session whose HTTPS_PROXY was baked at exec — but CACHE_FIX_REQUIRE_HOP=1
now exists for a deployment where the hop is a policy boundary rather than a cache.
Ref cnighswonger#304
Co-Authored-By: Claude <noreply@anthropic.com>
…open Self-review of the previous commit. The opt-in lived in forward-proxy.mjs and covered the two CONNECT paths only, while forwardRequest() — the relayed /v1/messages path, which is what this proxy exists for — kept dialling direct with the variable set. A door that closes the tunnel and leaves the main path open reads as closed and is not, which is worse than leaving it open honestly. THE OBVIOUS FIX IS WORSE, AND THAT IS WHY THIS COMMIT DOES NOT SHIP IT. Throwing from forwardRequest() is caught by handleMessages, but its catch opens with `if (abortController.signal.aborted) return`, and that signal is wired to clientReq's own "close" — which Node emits when the request BODY completes, not only when the client goes away. Measured with the guard in place: hop="" requireHop=true env="1" caught: no chain hop reachable | aborted=true | writableEnded=false POST /v1/messages -> TIMEOUT (10016ms) The client is still there and gets nothing. A leak that is honest beats a hang that reads as a refusal, so the guard stays off that path. The abort listener is a pre-existing defect, not one this introduced, and there is no evidence it has ever worked: instrumenting the same catch and running the existing "POST /v1/messages routes to upstream" case printed nothing at all — that test gets a real 401 and never enters the catch. Fixing it means changing streaming-abort semantics for every request, which is the highest-risk edit in this file and does not belong in a follow-up to a deploy that has already shipped. So the scope is recorded rather than hidden: requireHop moves to upstream.mjs beside resolveHop with the measurement in its comment, and the test asserts the relayed path is NOT refused, with a message telling whoever fixes the abort listener to come back and update both. No behaviour change from the previous commit. Ref cnighswonger#304 Co-Authored-By: Claude <noreply@anthropic.com>
handleMessages installs an abort so a client that gives up mid-SSE frees the upstream. It was keyed on clientReq's "close" — which Node emits when the request BODY has been consumed, i.e. on every request, immediately — so it aborted while the client was still sitting there, and the forwardRequest catch opens with `if (aborted) return`. Nothing was written back. Measured in reverse mode on the real /v1/messages path, against an upstream that refuses instantly, which is what a dead local hop does: before body at once -> HANG (6s client timeout) body delayed -> HANG after body at once -> 502 in 9ms body delayed -> 502 in 52ms Keyed on clientRes's close instead: it fires when the response finishes OR the connection is destroyed, so pairing it with writableEnded separates "we answered" from "the client hung up". Same change in handlePassthrough, which carried the identical line. This was found by measuring an exposure I had already dismissed as too risky to touch. The previous commit recorded it as a latent defect blocking a different fix; it is not latent, it is on the most ordinary upstream failure there is. WHAT THE TESTS DO AND DO NOT GUARD, because the difference matters: the 502 case dies when the listener is reverted to clientReq — mutation-checked the no-leak case does NOT die when the listener is deleted outright Two attempts at the second: client takes a frame then leaves (the pipe tears the upstream down by itself), and an upstream that accepts and never answers so no pipe exists (still freed). Both passed with the listener removed. So the listener may be doing nothing that socket teardown does not already do. It stays — "I could not demonstrate it matters" is not "it does not matter" — and the case is labelled as pinning the PROPERTY, not guarding the listener, so nobody reads it as coverage it is not. ALSO: the relayed probe added in 70ff998 dialled the real api.anthropic.com, because that test never set CACHE_FIX_PROXY_UPSTREAM and the default is the live host. That is the trap integrated.conf line 20 already warns about, and it took CI red on node 22 while bafabae with identical proxy code was green. It now runs against a local 418, on its own instance — pointing config.upstream at loopback for the whole case makes the CONNECT half read the tunnel target as the upstream and stop blind-tunnelling it, which failed the fail-open assertion for an unrelated reason. Ref cnighswonger#304 Co-Authored-By: Claude <noreply@anthropic.com>
The two branches answer the same question — is another process listening on the port we advertise — and they disagreed about what counts. /proc matched on the PORT alone (f[1] ends with :hexport, any local address); lsof pinned the 127.0.0.1 literal. The lsof branch is the only one a mac reaches, and two of three machines here are macs. So a proxy bound anywhere other than loopback read as "no successor" forever, and the outgoing proxy waited out its whole 30s ceiling on every handover instead of leaving as soon as its successor served. Found by sweeping for siblings of a fix already shipped: the launcher's two ownership probes were taught to honour CACHE_FIX_PROXY_BIND, and this third one in server.mjs was missed. That is twice in one day that a fix landed on the call sites in the diff and not on the ones a grep would have found, which is the class of mistake this sweep exists to catch. Matched to /proc rather than teaching /proc the address: a wildcard listener (0.0.0.0) serves loopback traffic but does NOT match an `-iTCP@127.0.0.1` query, so an address filter has a blind spot of its own — and it is the blind spot that errs toward "a successor exists", which would let a proxy leave an unowned port behind. The test spawns the wildcard listener in ANOTHER process. A self-owned one answers false either way, because the function excludes its own pid, so the first version of this case passed against the literal it was written to catch. Mutation-checked: restoring 127.0.0.1 fails it. NOT CHANGED, checked and deliberately left: forward-proxy.mjs connectUpstreamTLS defaults the upstream port to 443 regardless of scheme. It tls.connect()s unconditionally, so 443 is the right default for what that function does; defaulting by scheme would send an http upstream to port 80 over TLS, which is worse. The real oddity there is TLS to a plain-http upstream, and that is not this PR's to change. Ref cnighswonger#304 Co-Authored-By: Claude <noreply@anthropic.com>
… that did not
The previous commit shipped the abort fix with an honest note that the no-leak
half was unguarded: two attempts at a case for it both passed with the listener
DELETED outright, so it could not be claimed as coverage. It is guarded now, and
both earlier attempts were wrong for reasons worth keeping.
WHAT IT ACTUALLY DOES. forwardRequest wires the signal to upstreamReq.destroy().
When the upstream has not yet ANSWERED there is no pipe for socket teardown to
travel along, so the abort is the only thing that can free the connection:
with the listener dialled 1, live 1 at walk-away -> 0 after 2s
listener deleted dialled 1, live 1 at walk-away -> 1 after 2s
WHY THE FIRST TWO FAILED, both my own defects:
1. No premise. The case asserted only "0 connections at the end", which a
proxy that never dialled satisfies just as well. It carried an
`assert.ok(x || true, "")` placeholder I had left in — an assertion that
cannot fail. It now asserts that the proxy dialled AND that the connection
was live at the moment the client left.
2. Process-global contamination. With the premise added it STILL passed inside
proxy-server.test.mjs while the identical logic in a process of its own
separated cleanly — cached keep-alive agents, a forward-mode instance's
self-heal, another startProxy winding down. So it moves to its own file,
the same reason proxy-holder-handover.test.mjs is one case alone.
AND THE MUTATION EXPOSED A THIRD DEFECT IN THE TEST. With the listener deleted
it first died at the runner's 120s timeout reporting `pass 0 fail 0`: the leaked
connection kept h.close() draining, cleanup hung, and the assertion message that
had already fired was lost. Cleanup destroys the upstream sockets first now, so
the mutation fails in 2.5s with something readable. A case that discriminates
only by timing out is one nobody can act on.
No production change. Ref cnighswonger#304
Co-Authored-By: Claude <noreply@anthropic.com>
The review agent I had reported dead came back after 7.7h. Of its 15 findings
two were already closed by later commits; these are the live ones that are mine.
THE PORT-0 FIX CREATED THIS. Removing `Number(env) || 9801` let
CACHE_FIX_PROXY_PORT=0 through, and two sites below the bind still passed the
REQUESTED port where the BOUND one is required — while the gap and standby a
couple of hundred lines up already used `this._port`.
publishFingerprint(port) wrote cache-fix-proxy-0.sha256, so
runningOurCode(<bound>) from any other
launcher finds nothing and every port-0
install on the box collides on one file
CACHE_FIX_HELD_PORT: String(port) told the child "0", so its self-heal would
respawn on a DIFFERENT ephemeral port and
strand every session on the served one,
and successorServing("0") can never answer
Measured with CACHE_FIX_PROXY_PORT=0: bound 43557, record named …-0, child told
0. After: record named for the bound port, child told the bound port. Both
halves mutation-checked separately.
TWO DEFECTS IN MY OWN NEW TESTS, both the process-global class:
The first cut asserted on /tmp/cache-fix-proxy-0.sha256 — a GLOBAL path. It
passed alone and failed in the full suite, because something else on the box
had created it. An assertion on a shared path measures the machine's history,
not the code. The holder now gets a private TMPDIR.
`announces its release exactly once` was starving its own file. node runs a
describe's subtests concurrently, and that case adds a full run-service holder,
a proxy child, an in-flight connection, a 3s settle and a cleanup loop that
SIGHUPs every pid on its port. The agent measured it: 7 full runs, 3 failures,
all in that file and varying between cases, against 0 in 3 with the case
excised. It moves to its own file — the remedy proxy-holder-handover.test.mjs
already applies to itself, for the same reason, in its own header. Still
mutation-checked in its new home.
Full suite now 3 consecutive runs, 0 failures, 1854/1853.
ALSO, from cswap's pin: a tripwire on the CONNECT case, because an assertion
that fires on `[]` has already discarded the evidence that would narrow it.
Every endpoint records now, so a failure says which was touched — measured
`["UPSTREAM"]` when the tunnel is aimed there, `[]` when it reached none. The
comment says plainly that `[]` still does not name the third case (the proxy
MITM'ing the target itself), because narrowing is not naming.
STILL OPEN, recorded not fixed: CACHE_FIX_REQUIRE_HOP closes two of four
fall-open egress paths. bin/gap-relay.mjs direct() does not consult it at all,
and that is the tunnel that carries traffic precisely when the proxy is down.
The pin's own _blind_tunnel walks its chain per hop, treats a non-200 as
"refused BY this hop", and reaches direct only when none will carry — with the
refusal traced. Their advice, which this does not yet implement: closing on
no-hop trades an invisible fall-open for an invisible outage.
Ref cnighswonger#304
Co-Authored-By: Claude <noreply@anthropic.com>
It read the FIRST usable candidate at startup and fell straight to a direct dial
when that one would not carry, so a configured second hop was never tried.
proxy/upstream.mjs resolveHop() walks the whole list — one chain carrying two
definitions of itself, and the relay's copy is the one that runs precisely when
the proxy is down.
Measured, three recording endpoints and a relay per case:
hop1 dead, hop2 alive before: ORIGIN (dialled past a hop that would carry)
after: HOP2, 1 refusal traced
both hops dead after: ORIGIN, 2 refusals traced
NOT A HARD CLOSE when none will carry, and that is cswap's pin's call rather
than mine. I had proposed consulting CACHE_FIX_REQUIRE_HOP here; they measured
that closing on no-hop trades an invisible fall-open for an invisible outage,
and this is the tunnel that carries traffic when the proxy is down — the most
expensive place to take one. Their _blind_tunnel does the same walk, treats a
non-200 as "refused BY this hop", and reaches direct only when none will carry.
Direct stays the last resort; the refusals are traced so it is not a silent one.
The trace uses the string proxy/upstream.mjs already emits — `hop <addr>
unusable` — so one grep reads both ends of the chain.
Also retracted: I had argued this hole mattered because a direct route's leaf
carries no Authority Key Identifier. The pin corrected it — that applies to a
MITM leaf, not a blind tunnel carrying the client's own TLS to the origin. The
hole is real for a different reason: a bypass nobody can see in the log.
THE FIRST MEASUREMENT OF THIS SAID THE FIX DID NOTHING. Zero endpoints touched,
zero traces, both scenarios. The relay listens on `srv.listen({ fd: 3 })`
because the holder hands it an already-bound socket, and the fixture spawned it
without one — so it never listened, and a broken instrument read exactly like a
broken fix. The test now asserts `gap-relay carrying` as a premise before
measuring anything, so the next person gets "nothing was measured" instead of a
false negative.
Both halves mutation-checked separately: reverting the walk fails both cases,
and keeping the walk while dropping the trace fails both too.
Suite 1856/1855/0. Ref cnighswonger#304
Co-Authored-By: Claude <noreply@anthropic.com>
Three of the review agent's minor findings, verified rather than relayed. All three were mine. AN EMPTIED CHAIN LEFT A HOP BEHIND. Both getters read the env per call, so the list can go away under a running proxy, and resolveHop's `if (!chain.length) return ""` skipped _lastHop entirely. Measured: resolved :40559, chain emptied, resolveHop returned "" and lastHop() still said :40559 — so /health went on naming an address no request could take. That is the exact lie the field was fixed to stop telling, re-entering through an early return the fix did not touch. _directLast is deliberately NOT stamped there. "No chain was ever configured" is not a fall-through, because there was no chain to fall through; stamping it would fire on every reverse-mode proxy that never had one and empty the field of the meaning it exists for. Asserted, so the distinction survives a refactor. A COMMENT OF MINE WAS A LIE. Three tests set CACHE_FIX_CHAIN_GRACE_MS after importing upstream.mjs and called the retry loop "not what is under test". CHAIN_GRACE_MS is a module-level const captured at import, so the assignment does nothing — measured, the case runs 2,616 ms, one full 2,500 ms default window. Set BEFORE the module loads it works: 28 ms. So the knob is fine for an operator, who sets it before the proxy starts, and the production code is unchanged; the comment is what was wrong, and a comment telling the next reader the wait is gone is worse than the 2.5 s. THE SCHEME-PORT INVARIANT COVERED TWO COPIES OF THREE. bin/gap-relay.mjs carries its own portOf() because it imports node:net and nothing else — it runs when the proxy is DOWN, so depending on proxy/ modules would let a broken one take the relay with it. The duplication is deliberate; leaving it unchecked was not, and it was already correct there, which is why the other two read as a regression against it. Mutation-checked: breaking gap-relay's copy now fails the case. Suite 1856/1855/0. Ref cnighswonger#304 Co-Authored-By: Claude <noreply@anthropic.com>
Every fix here is one shape: something that could not be determined was reported as a definite answer, and the definite answer opened a path that takes the proxy's address down under sessions that cannot re-read HTTPS_PROXY. Launcher ownership decisions: - runningOurCode() is three-state. Its two callers take "cannot tell" in opposite directions — holderPidOn must not signal an unidentified pid, otherHolderOn must not silently abandon a deploy — so one boolean default could not be safe for both. Unknown keeps its old exit and gains the operator's only message, naming BOTH causes (no usable record, or our own server.mjs unreadable); blaming the record sent an operator to /tmp for a broken install. - otherHolderOn's lsof probe piped stderr and split read-failure from absence. lsof exits 1 for both, with identical empty stdout — only stderr differs — and the call discarded stderr, so the instrument could not answer even in principle. On a box without a usable lsof every launcher read "no other holder" and the pileup this rule prevents came back, in silence. Proxy shutdown and liveness: - The 5s watchdog res.end()'d responses that had never sent headers, which emits an implicit 200 with Content-Length: 0. A stop during a slow upstream call turned a retryable reset into a well-formed empty success the client cannot distinguish and will not retry. Gated on headersSent. - "(handed off)" was announced on the INTENT to spawn a successor, not on the spawn succeeding. The holder reads that string as "a successor is serving", skipping both reclaim and respawn, so a failed spawn ended with nobody on the socket and a supervisor that believed it was covered. - The watchdog exited 0 while the graceful close exits 75, and the watchdog is the normal stop under systemd — so a supervisor keyed on 75 read "nothing to succeed to" for a lineage that had a successor already serving. - successorServing() answered false on a /proc/net/tcp miss. That file is IPv4-only, so an IPv6 bind made every handover burn its full 30s ceiling. A miss now falls through to lsof. - The self-upstream loop guard compared the REQUESTED port, which is 0 for every holder-spawned child, so it could never fire on the deployment shape it was written for. It now checks the advertised port too. - openGap never passed CACHE_FIX_HELD_HOST while openStandby always did, so a non-loopback bind let the gap relay forward to itself. - A malformed CACHE_FIX_UPDATE_CHANNEL_URL threw inside an async timer. In reverse mode nothing catches that, and Node terminates the process ~25s after boot — one typo taking down the address every session dials. Test-side, where the instrument was the defect: - The suite's scratch copies are named scratch-* rather than test-*. `node --test` with no path globs **/test-*.?(c|m)js repo-wide, so a killed run left bin/test-launcher-<tag>.mjs that the next run DISCOVERED AND EXECUTED as a test file. HOLDER_TREE excludes them by name, and dropping the leading dot also makes leftovers visible to `ls bin/`. - The chunk-boundary case asserted an empty action list with no positive control; empty is also what a dead dispatch produces, and it passed against one. It now drives a plain release and requires reclaim+spawn at every boundary. - The holder-tree case lifted the launcher's filter but matched raw source, so a comment quoting the rule shadowed broken code. Comments are stripped now. - askForSuccessor had no coverage at all: hard-coding it false left 66 of 66 green. Co-Authored-By: Claude <noreply@anthropic.com>
…, which hung CI CI has not completed on this branch since the rebase. Three jobs sat in `Run tests` for hours with no timeout-minutes on the workflow, so each was on course for GitHub's 6-hour ceiling: node reported `Promise resolution is still pending but the event loop has already resolved`. `withRelay` binds a socket, hands its fd to the relay child, and then keeps listening on it in the parent. Both processes are listeners, so the kernel gives each connection to whichever accepts first -- and this parent has no `connection` handler, so a connection it wins is held open with nothing to end it. `close(cb)` waits for every open connection, so its callback never comes and the promise never settles. The parent now stops accepting as soon as the child has the fd; `stdio` handed the child a dup, so the socket outlives the parent's copy and there is no window where nobody is listening. Measured, because it hides on a fast box: 48 cores here always let the child win the accept race and the file passes in 6.4s. Under `taskset -c 0-3`, matching the runner's core count, it times out. Reverting just the close makes it hang again -- EXIT=124 mutated, EXIT=0 restored. Two wrong answers were tried and rejected on evidence first. `--test-concurrency=8` was not the cause: the two runs that went green on this branch already carried it. Neither was the orphan-holds-stdio failure fixed in ac35800: the orphans found here had /dev/null on fd 1, so they were holding nothing. Suite: 1862 tests, 1861 pass, 0 fail, 1 skipped, unconstrained. Co-Authored-By: Claude <noreply@anthropic.com>
Four comments named cswap's pin as a consumer of /health.https_proxy and direct_last, and one called a chain flap "unfindable" once /health reads green. Both are measured false. The pin's chain check dials pin's own :36301 and reads chain / egress / direct_last, all produced by pin. Compared field-set to field-set, `chain` and `egress` do not exist on :9901 at all, and the only overlapping NAME is direct_last — which is pin's own. Two projects believed in the dependency for an hour because one field name appeared on both endpoints. The flap is unfindable THROUGH /health, not unfindable: resolveHop writes `hop <addr> unusable — ...` to stderr, and that log is how the same event was reconstructed on the peer side. The precision runs in the reader's direction — told nothing survives, nobody opens the log, which is the one place the trace is. Comments only: no non-comment line changes, suite 1862 tests / 1861 pass / 0 fail / 1 skipped, unchanged. Co-Authored-By: Claude <noreply@anthropic.com>
The shipped wiring configures CACHE_FIX_FALLBACK_PROXIES and nothing else, so
`primary` is "" — and "" is filtered out of the chain, which made `hop !==
primary` true of every fallback. Every proxy generation therefore logged
[upstream] hop direct unusable — routing via 127.0.0.1:8118
on its first resolve. Nothing was unusable; there was no primary to lose.
Measured on lambda-docker: 8 such lines in one 24,026-line log, one per
generation, and a peer session read them as eight real degradations of ours.
It costs more than noise. A next-hop degrade is unpublished and non-sticky, so
this stderr line is the ONLY surface where a genuine one can be found — the
same surface the peer's own 06:18 event was reconstructed from. A fault cried
on every healthy start poisons the one instrument that can answer the question.
Guarded on `primary` rather than the report deleted, and the test asserts both
directions: a fallback-only start says nothing, a configured-but-dead primary
still announces. Mutations: guard removed kills the first assert, report
removed kills the second.
Suite 1863 tests / 1862 pass / 0 fail / 1 skipped.
Co-Authored-By: Claude <noreply@anthropic.com>
When the last reader of the proxy's stdout/stderr pipe goes away — a closed terminal, a rotated log, a killed `tee` — the next write raises EPIPE. That arrives as an asynchronous 'error' event, not a synchronous throw, so Node promotes it to uncaughtException. The self-heal handler then formats err.stack and writes it to the same dead stderr, raising the next EPIPE: the mechanism that exists to keep the proxy up is what takes it down. Measured on a live proxy: 100% CPU, 22 minutes of CPU time burned, 18 connections accepted and none answered — while /health kept returning 200 in 0.35s with every self-reported field green, so no health-based check saw it. Install 'error' listeners on process.stdout/stderr that swallow EPIPE, and route the self-heal handlers' own logging through a guarded write so a log line can never become the next uncaught exception. Both halves are needed: the try/catch covers a synchronous throw on a destroyed stream, the listeners cover the asynchronous event that caused the outage. Removed again in removeSelfHeal, so an embedded host process regains Node's default behaviour. The test reproduces the outage rather than simulating it: it removes the only reader of the child's stdio, raises an uncaught exception from a check-phase callback, then asserts the proxy still serves and does not spin. Mutation-checked — deleting the two listeners fails it with "burned 2.00s of CPU in 1.5s wall". Co-Authored-By: Claude <noreply@anthropic.com>
…he proxy The launcher decides who owns the port by shelling out to `lsof` and `ps`. Both cost what the process table costs, and the moment that table is in trouble is exactly when this code runs hardest: holderPidOn() is called from the takeover retry loop every 500 ms and from the deploy watcher on every tick. None of the nine call sites carried a timeout, so each one blocked the process for as long as the machine stayed sick — a proxy that answers nothing while adding load to the machine it is waiting on. Measured on a user's laptop 2026-08-14: a crash reporter forked ~500 processes per second for fifteen minutes (kernel: "Too many corpses being created", pid 48888 -> 54010 in ten seconds). Every process-enumerating call on that box became unbounded. The freeze itself was not ours, but our tooling had no ceiling of its own, and that is the part we own: this is a refusal in code, not a promise to be careful. One `probe()` helper with timeout + SIGKILL + maxBuffer, used by all eight launcher call sites; the proxy's single lsof gets the same options inline rather than a second copy of the helper. Safe because every caller already treats "cannot tell" as an answer — holderPidOn returns null, and null means LEAVE IT ALONE. Abandoning a probe is never worse than blocking on it. CACHE_FIX_PROBE_TIMEOUT_MS overrides the 2s default. The new test replaces lsof AND ps with commands that never return and asserts run-service still finishes; unbounded, it runs for ever. Mutation-checked — deleting the timeout line alone fails it with the same assertion. proxy-held-port's two fingerprint cases lift the rule's source and evaluate it against an injected execFileSync, so they now lift probe() the same way they already lift bindAddr and runningOurCode. That is deliberate: a rule that stopped going through probe() would otherwise keep passing there while being unbounded in production. Co-Authored-By: Claude <noreply@anthropic.com>
Two upstream reviews plus five local review rounds. The blocker and the
egress hole are the load-bearing half; the rest is that most of the guards
written along the way could not fail, and finding that took the other four
rounds.
BLOCKER — a refused fd-3 handover claimed it had handed the socket on.
`inheritedSocket` was computed from `listenFd`, which records that handover
was ATTEMPTED. When the fd-3 listen fails and the fallback binds a real
port, `listenFd` stayed set, so shutdown spawned a successor pointed at the
same unservable descriptor and exited 75 — telling the supervisor a
successor holds the socket — while the port actually served was released
with nobody on it. Reproduced on the PR head: "socket handover refused
(EINVAL); binding 127.0.0.1:0 instead" alongside {"inheritedSocket":true}.
Clearing listenFd in the catch is the whole fix; the regression test drives
SIGTERM through it and asserts exit 0 and no successor.
SECURITY — CACHE_FIX_REQUIRE_HOP was honoured on the two CONNECT paths and
not on the relayed /v1/messages route, which dialled api.anthropic.com
directly with the caller's key. The reason recorded for leaving it open —
that throwing would hang the client — died when the abort listener moved to
clientRes' "close" gated by writableEnded; the throw now becomes the 502
that catch already writes. Refusal asks the same question getAgent asks, so
a configured hop is never refused, and NO_PROXY hosts stay exempt.
A DEAD LOG READER MUST NOT TAKE THE PROCESS THAT SERVES THE PORT. The proxy
got that guard after a measured 27-minute outage; the holder and the gap
relay share the same pipe and never did, and the proxy's own guard sat
inside installSelfHeal(), which runs only in opt-in forward mode. Three
processes, three guards, and the holder's belongs at the top of holdPort()
because `server` + CACHE_FIX_HOLD_PORT=on is a second door into it — via
that door the holder died, via run-service it survived.
Also: gap-relay keeps the socket on an error that left it listening and
refuses to arm without CACHE_FIX_STANDBY_PARENT rather than silently
comparing 1 against 1 forever; openGap gets the identity check openStandby
already had, so a retired gap's late exit cannot clear a live successor;
SIGUSR2 recovery is reachable from both the sync throw and the async
'error', and departs only once the successor has actually started, through
the restart ladder rather than past it; openssl is bounded; holderPidOn
warns on an unknown fingerprint the way otherHolderOn does and asks ps once
per pid instead of twice.
THE TESTS ARE MOST OF THIS DIFF, because most of them could not fail.
Measured, each: an assertion reading the wrong stream; a probe-bounded case
whose `ps` fake was never invoked; a static walker for "is this install
unconditionally reached" defeated six times — by a forward-mode-only copy,
one brace too shallow, a feature-flag `if`, a wrapped signature, an `else
if`, a comment carrying a stray `}`. Reachability is not a property of one
line of text, so that walker is deleted and the question is now asked at
runtime: kill the reader, force a write, see who is alive. openGap has no
observable window at all — start() closes the gap before spawning the child
because two handles may bind one port but only one may listen — so its rule
is driven directly instead.
Every guard here was mutation-checked: remove it, watch a named test die,
restore. Where a check could not certify itself — the comment stripper — it
says so rather than asserting an invariant blind to its own failure.
Co-Authored-By: Claude <noreply@anthropic.com>
0430f8e to
84e0472
Compare
What
A reload of the proxy cuts every session on the port. This makes the listening socket outlive the process that serves it, and makes the incumbent's forced shutdown end responses cleanly instead of destroying them.
Why it is user-visible
A reload today is kill-then-respawn, so the port is unbound for the gap. Measured: a stream through this proxy died
ECONNRESETafter 18 chunks; with the socket held across the handover the same stream ran to completion.The forced-shutdown path had a second cut.
closeAllConnections()destroys the socket and the kernel answers RST — measured, a client that had already received every byte still surfacedECONNRESETand threw the delivered data away.res.end()sends FIN, which the same client reads as a clean EOF.What is in it
A supervisor holds the listening socket and hands it down on fd 3 (
LISTEN_FDS). The proxy serves a socket it neither binds nor closes, so a reload replaces the serving process while the port stays bound. When fd 3 is not servable — in an IPC-forked child it is the IPC channel andlistenfailsEEXIST— it falls back to binding its own port, because a degraded proxy beats no proxy.This replaces an earlier
SO_REUSEPORTco-bind, which worked on one of the three runtimes we run:reusePortEADDRINUSEENOTSUPon the first listenAn inherited socket needs no platform support and has exactly one listener, which also retired the mode-conflict guard the co-bind design required.
bin/gap-relay.mjs(new, 293 lines): a standby armed by the holder. It is spawned holding a descriptor to the same listening socket and carries connections to the next hop the instant its holder is gone —process.ppid !== bornOf, no probe and no decision to wait for. Measured across holder-and-proxy-both-killed: two 2 s proving windows cost 3,899 ms, three 250 ms windows cost 694 ms, arming immediately costs 3 ms. It does not stand down on its own; yielding is the claimant's decision, made with SIGHUP.FIN, not RST, on the watchdog path. Open responses are tracked so the forced shutdown can
end()them, then force whatever did not take the FIN.The 5 s shutdown grace is unchanged. A supervised stop is serial, so a longer grace only extends the outage: measured at 120 s against
DefaultTimeoutStopSec=90s, the stop was SIGKILLed at the cap and restart downtime went 5.0 s → 53.9 s.CACHE_FIX_REQUIRE_HOP=1, new, off by default: makes an unreachable chain a 502 instead of a direct dial. It guards the CONNECT paths only —forwardRequeststill dials direct with it set, asserted by a test rather than left undocumented./healthgainshttps_proxy_measuredanddirect_last, andhttps_proxynow publishes the hop a resolve actually used rather than a configured candidate.Non-Functional Requirements
proxy/server.mjs. Actual production change: 2,848 insertions / 167 deletions across six files (bin/claude-via-proxy.mjs+1,724,proxy/server.mjs+744,bin/gap-relay.mjs+293 new,proxy/upstream.mjs,proxy/forward-proxy.mjs,proxy/config.mjs), 7,785 insertions across 21 files including tests. The growth is the supervisor/standby lifecycle plus defect fixes from review, not added feature scope — but it is an order of magnitude over the budget and a reviewer should weigh it as such rather than take the estimate on trust./healthfields are booleans/timestamps derived from our own state; the hop address is published without credentials as before. The inherited socket is passed by the supervisor that bound it; nothing reads an fd number from the environment without attemptinglistenon it and falling back when that fails.proxy/.listenOnceis local, one call site, and exists only because the fd path needs a re-callable listen.bin/gap-relay.mjsis a new file rather than an abstraction: it is a separate process by requirement, since its whole purpose is to outlive the one that spawned it.otherHolderOn/holderPidOn), the client-abandon abort, and two published/healthfields. Wants human review, not just Lead + Codex.Testing
Full suite: 1,862 tests, 1,861 pass, 0 fail, 1 skipped (46 s), re-run at this head. Branch is cut from
upstream/main, behind 0, no other work on it.Every fix from review has a test that fails when the fix is reverted.
Defects fixed during review
Four are worth naming because they are outside the feature this PR is titled for:
otherHolderOncompared process age only, so on every deploy the new code judged itself surplus and exited 0 while the old holder kept serving.holderPidOnreturned"holder"on the mere presence of a run-service, makingrunningOurCode()unreachable.bindFailedread no error code, so a bind that can never work exited 0.handleMessageskeyed its abort onclientReq's"close", which Node emits when the request body is consumed — it aborted on every request while the client was still waiting. Against an upstream that refuses instantly:HANG (full client timeout)→ 502 in 9 ms.successorServingasked its/procbranch about the port and itslsofbranch about127.0.0.1.lsofis the only branch a mac reaches, so a proxy bound off loopback read as "no successor" and every handover waited out its 30 s ceiling.Measurements for the rest are in the commit messages.
🤖 Generated with Claude Code