merge pr-356

Resolved test/proxy-held-port.test.mjs, both hunks to the incumbent side.

The first is a third wording of one type guard in classify(); all three are
behaviourally identical, and the one kept names the mechanism (freePort
releases a port before its caller binds it, so a neighbour's 200 arrives as a
Number) rather than the single 502 that surfaced it.

The second is not a wording difference: cnighswonger#356 predates the two cases pr-345
added here (the port-1 refusal probe and readyBody), so taking its side would
delete them. Kept.

Co-Authored-By: Claude <noreply@anthropic.com>

Author: codeslake

CHANGELOG.md

@@ -10,6 +10,16 @@
 
 ### Fixed
 
+- **The two drain budgets reject a value they used to read as zero, and the route tally no longer renders an authority.** `Number(x) || fallback` took the fallback for an explicit `0` and passed a NEGATIVE straight through, and a negative stall budget ends every owed connection on the first tick -- the guillotine this drain replaced. Tightening that to a finite non-negative number then left a second hole in the same place: `Number()` reads whitespace as 0, so `CACHE_FIX_DRAIN_STALL_MS=" "` in an env file was the same guillotine, measured severing a healthy 3-second-gap stream. Both knobs now take a plain non-negative decimal and fall back otherwise. Separately, `drainRoute` guarded on the request-target's SHAPE, which cannot enforce a content rule: `/http://user:pass@host/v1`, a backslash, a percent-encoded `//` and a `#` fragment are each one leading slash and each put an authority in a log line that outlives the process. It now judges the label it is about to write.
+
+- **A drain no longer severs a reply that had finished but not flushed, and no longer keeps the port in order to deliver it.** Node counts a response whose `end()` has been CALLED as idle, and `http.Server.close()` runs that idle sweep *before* it unbinds — so a reply that was complete but still queued was destroyed at drain start: measured 4,217,623 bytes delivered of a declared 16,777,216, with `drained clean` printed for it. Discriminated on plain Node across four arms — no close 100% delivered, `http.Server.close()` 24.9%, the same plus an explicit sweep 24.9%, `net.Server.prototype.close` 100%, all of them refusing new connections afterwards — the sweep is the agent and the unbind costs nothing. The listening socket is therefore released at once and only the sweep waits for the flush; waiting with the unbind instead held the address for the whole drain budget, after the release had already been announced and a holder had settled on it.
+
+- **A handover drain now ends stalled connections one at a time instead of judging the whole port.** The stall test kept a single timestamp and reset it whenever ANY owed connection moved a byte, so on a port with traffic one live stream held the clock open and a genuinely stalled reply was never aged out — measured, it polled for a full 1800 s answering "still moving" and then cut six replies on the backstop. Each owed connection now carries its own last-byte stamp, dated from when the request ARRIVED while nothing has left its socket since, and one quiet for `CACHE_FIX_DRAIN_STALL_MS` (default 90000) is ended alone while the rest keep delivering. The cut moved with the clock: ending the whole drain on the first quiet connection would take the live ones with it. A response is reset rather than closed unless bytes actually reached the client, because `res.end()` on a header-less reply emits a well-formed empty `200` a client cannot distinguish from a real one and will not retry. The forced-close line no longer counts a connection the stall test already ended, and a drain that ended replies no longer calls itself "clean". The 5 s ceiling now applies to the STANDALONE arm alone -- a proxy with nothing supervising it, where the process draining is the process the service manager is waiting on. A stop under a live holder no longer takes it; see the entry below for what that costs.
+
+- **A holder handover no longer freezes the config the outgoing holder booted with.** `SIGUSR2` passes the listening socket to a successor spawned with `{ ...process.env }` — faithful, and therefore stale: a `CACHE_FIX_*` switch added after a holder started could not reach it without releasing the address, and releasing it cuts whatever is streaming across the gap. The successor now re-reads `CACHE_FIX_*` settings from `${CLAUDE_CONFIG_DIR:-~/.claude}/cache-fix-handover.env` (relocate with `CACHE_FIX_HANDOVER_ENV`) and lets them win over what it inherited. Every failure is off — absent, unreadable or malformed leaves the inherited value standing, since a handover that refuses over a bad config file is worse than one carrying a stale switch. A line is honoured only once its newline has been written, so a handover that catches the file half-written cannot pick up a truncated value: `CACHE_FIX_PROXY_UPSTREAM=http://ho` is a well-formed assignment and nothing later corrects it. The port, the bind and the orphan guard stay pinned by the holder: the successor *adopts* the socket, so a bind written here could not move it but would mislabel `CACHE_FIX_HELD_HOST`, the proxy child and `/health`. Not covered: stdio. `inherit` passes file descriptors, not environment, so a lineage started with its stderr on `/dev/null` keeps it however this file is written.
+
+- **A stop under a holder no longer severs the reply it is delivering, and the holder no longer waits for it.** The holder settled on its child's EXIT, so every second the child spent draining was a second `systemctl stop` was blocked -- which is what forced a 5 s ceiling onto the child and cut 15 replies across four stops on one host (4, 3, 1, 7). The holder now settles on the proxy's RELEASE announcement, which the proxy makes *before* it drains and after the listening socket is already free, so the port is available to a successor in under a second while the outgoing proxy finishes what it owes. **The operational trade is a resident drainer:** a stop returns immediately and leaves the old proxy alive, holding no listener and no lock, until its replies finish or `CACHE_FIX_DRAIN_MS` expires -- up to 30 minutes by default, one process per stop. It exits 0, so a supervisor that restarts on failure does not restart it. The holder also signals that child `SIGUSR2` rather than `SIGHUP` on a handover: `forward()` rewrites every stop to `SIGHUP`, so one word could not mean both "a successor is already serving" and "the supervisor is stopping us", and those want opposite budgets.
+
 - **A refused fd-3 handover no longer makes the proxy claim it handed the socket on.** `inheritedSocket` was computed from "handover was attempted", not "handover succeeded", so a proxy that was refused fd 3 and fell back to binding its own port still advertised an inherited socket. On `SIGTERM` it then spawned a successor pointed at the same unservable descriptor and exited `75` — telling the supervisor a successor holds the socket — while the port it actually served was released with nobody on it. Exits `0` now, spawns nothing, and leaves no orphan.
 
 - **A dead log reader no longer kills the supervisor or the relay.** The proxy gained an EPIPE swallower after a measured 27-minute outage; the launcher's holder and the gap relay share the same pipe and never got one, so the same killed reader took down the process whose whole job is to put the proxy back. Both now install stream `'error'` listeners, as does the proxy in reverse mode, where its own guard had been attached only when forward mode was active. The interactive wrapper deliberately keeps the old behaviour — a foreground producer whose consumer dies should end.

README.md

@@ -450,6 +450,9 @@ All proxy settings are controlled via environment variables. Set them before sta
 | `CACHE_FIX_FORWARD_PROXY` | unset | Set to `on` for forward-proxy mode (HTTP CONNECT + selective MITM of the upstream host) so the client points `HTTPS_PROXY` at the proxy instead of `ANTHROPIC_BASE_URL`, keeping Remote Control enabled. See [Forward-proxy mode](#forward-proxy-mode-keeps-remote-control-working). |
 | `CACHE_FIX_CA_DIR` | `~/.claude/cache-fix-ca` | Directory for the forward-proxy CA/leaf cert (generated once on first start). The client trusts `ca.pem` via `NODE_EXTRA_CA_CERTS`. |
 | `CACHE_FIX_PROXY_TIMEOUT` | `600000` | Request timeout in milliseconds |
+| `CACHE_FIX_DRAIN_MS` | `1800000` | Backstop for a handover drain: how long the departing proxy keeps delivering replies it still owes after it has released the listening socket. It is a re-evaluation point rather than a deadline — a connection still writing bytes when it expires keeps the drain alive and is reported every 60 s, because the thing on the other side of that branch is a reply somebody is reading. Applies to the handover arm only; a stop with nothing supervising the proxy keeps a fixed 5 s ceiling, since there the process draining IS the process the service manager is waiting on. Parsed the same way as `CACHE_FIX_DRAIN_STALL_MS` above. |
+| `CACHE_FIX_DRAIN_STALL_MS` | 90000 | How long an in-flight reply may go with **no byte written** before a handover drain ends that one connection. Paired with `CACHE_FIX_DRAIN_MS` (the drain's backstop, default 1800000), which bounds the whole drain rather than any connection. The default is a judgement bounded by two observations, not a percentile: content-free waits reach ~186 s while byte-free waits reach ~23 s on a busy stream, so 90000 sits past any gap a live reply produces and short of the ten minutes that was cutting real work. **Do not tighten it toward an observed maximum** — that maximum is not stable, and anything under ~60000 is inside the range of a healthy stream on a slow link. Raise it if your upstream is slower than the samples behind the default. Accepted values are a plain non-negative decimal, optionally padded; anything else -- whitespace, a negative, `1e3`, `0x10` -- falls back to the default rather than being guessed at. `Number()` alone reads whitespace as 0, and 0 here is an immediate cut. |
+| `CACHE_FIX_HANDOVER_ENV` | `~/.claude/cache-fix-handover.env` | File the holder re-reads when it hands its listening socket to a successor on `SIGUSR2`, so a `CACHE_FIX_*` setting can reach a running fleet without dropping the port. One `KEY=value` per line, each terminated by a newline — no quoting, no `export`; only `CACHE_FIX_*` keys are read, and every failure (absent, unreadable, malformed) leaves the inherited value standing. An unterminated last line is ignored, so a handover that catches the file half-written cannot pick up a truncated value. Settings only — the holder pins the port, the bind and `CACHE_FIX_STANDBY` over anything written here, and sets the `CACHE_FIX_HELD_*` lineage keys itself at each spawn, so a bind change still needs a real restart. **Write access to this file is control of the proxy** — it can set `CACHE_FIX_PROXY_UPSTREAM` and `CACHE_FIX_REQUEST_CAPTURE`, so keep its permissions as tight as the config dir's. |
 | `CACHE_FIX_EXTENSIONS_DIR` | `proxy/extensions/` | Directory for extension `.mjs` files |
 | `CACHE_FIX_EXTENSIONS_CONFIG` | `proxy/extensions.json` | Extension configuration file |
 | `CACHE_FIX_DEBUG` | `0` | Enable debug logging |

bin/claude-via-proxy.mjs

@@ -11,6 +11,7 @@ import net from "node:net";
 import { EventEmitter } from "node:events";
 import { getSystemErrorName } from "node:util";
 import { bundleUsable, carriesOurCA, salvageBundle } from "./ca-trust.mjs";
+import { handoverEnv, handoverEnvPath } from "./handover-env.mjs";
 import { sourceFingerprintSync } from "../proxy/source-fingerprint.mjs";
 
 const __dirname = dirname(fileURLToPath(import.meta.url));
@@ -1072,8 +1073,21 @@ function holdPort(rest) {
           // 9901."
           //
           // We are bound to the number, so there is nothing to guess.
-          env: { ...process.env, CACHE_FIX_HOLDER_HANDOVER: "1", LISTEN_FDS: "1",
+          env: { ...handoverEnv(process.env), CACHE_FIX_HOLDER_HANDOVER: "1", LISTEN_FDS: "1",
                  CACHE_FIX_PROXY_PORT: String(holder._port || port),
+                 // AND THE BIND: the successor ADOPTS this socket, so a bind from
+                 // the file cannot move it but would still relabel _host, and every
+                 // downstream name (HELD_HOST, the proxy child, /health) with it.
+                 CACHE_FIX_PROXY_BIND: bindAddr(),
+                 // AND THE PATH TO THE FILE, or the file moves its own trust
+                 // anchor: a CACHE_FIX_HANDOVER_ENV written there points every
+                 // later handover somewhere else, and since absence means
+                 // inherit, reverting the original file cannot take it back.
+                 CACHE_FIX_HANDOVER_ENV: handoverEnvPath(),
+                 // AND A HOLDER IS NOT A STANDBY. openGap() sheds HOLDER_TREE and
+                 // HELD_BY the same way; this one only became reachable when the
+                 // env above stopped being ours alone.
+                 CACHE_FIX_STANDBY: undefined,
                  CACHE_FIX_EXIT_WITH_PARENT: "0" },
         });
         // WE LEAVE WHEN THE SUCCESSOR EXISTS, not when we have asked for one.
@@ -1103,11 +1117,24 @@ function holdPort(rest) {
         // mutually exclusive, so nothing reaches recovery after departure today;
         // it is kept so a future reordering cannot quietly re-arm a holder that
         // has already handed the address on.
+        // SIGUSR2 IS THE HANDOVER'S OWN WORD, and it has to be its own: this
+        // block rewrites every stop to SIGHUP, so SIGHUP cannot also mean "a
+        // successor is already serving". The proxy picks its drain budget on
+        // the difference — 5 s where a supervisor waits serially, half an hour
+        // where nothing waits on it at all.
+        //
+        // SAFE ONLY BECAUSE `child` IS ALWAYS OURS. A proxy with no SIGUSR2
+        // handler takes node's default and dies outright, cutting everything
+        // with no drain — the hazard deploy.sh guards with a holder_tree check
+        // before signalling a holder it did not start. Here the only assignment
+        // that creates a child spawns SERVER_PATH from beside this file, so it
+        // cannot predate the handler. Adopting a proxy we did not spawn would
+        // make this line fatal.
         successor.once("spawn", () => {
           // The child under us keeps serving until IT is replaced by the
           // successor's own child; nothing here interrupts the accept path.
           if (child && child.exitCode === null && !child.signalCode) {
-            try { child.kill("SIGHUP"); } catch { }
+            try { child.kill("SIGUSR2"); } catch { }
           }
           left = true;
           settle(0);
@@ -1431,6 +1458,24 @@ function holdPort(rest) {
         if (!retired && line.includes("releasing the listening socket")) {
           retired = true;
           if (child === me) child = null;
+          // A STOP ENDS HERE, NOT AT THE CHILD'S EXIT. The proxy has just closed
+          // its listening socket; from this line it owns nothing but the replies
+          // it still owes. Waiting for those is what forced a ceiling onto the
+          // child — it could not afford patience while a stop was blocked on it.
+          // Its budget keys on being held (server.mjs `unwaited`), so the two
+          // cannot be separated.
+          //
+          // Two cross-file invariants make the orphaned drainer safe, and
+          // nothing else records either:
+          //   - a released proxy must not resurrect the lineage
+          //     (server.mjs `if (releasingPort) return`), or the orphan returns
+          //     as a rival holder
+          //   - it must exit 0, not 75. A launchd agent with
+          //     `KeepAlive = { SuccessfulExit = false }` restarts a non-zero exit
+          //
+          // reclaim() and spawnWhenReady() below already return early while
+          // stopping, so this returns past nothing.
+          if (stopping) return settle(0);
           // "(handed off)" means the proxy already put its own successor on the
           // socket before announcing, and that successor is serving right now.
           // Reclaiming would take the port from a live proxy and spawning would

bin/handover-env.mjs

@@ -0,0 +1,48 @@
+import { join } from "node:path";
+import { readFileSync } from "node:fs";
+import { claudeHome } from "../proxy/claude-home.mjs";
+
+// A handover keeps the port alive and, until this existed, kept the CONFIG the
+// outgoing holder booted with too — so a switch added afterwards could not reach
+// the fleet without dropping the address, and dropping it cuts whatever streams
+// across the gap. Re-read CACHE_FIX_* from a file the holder does not own.
+//
+// EVERY FAILURE IS OFF. Absent, unreadable, malformed, unrecognised: the
+// inherited value stands. A handover that refuses because a config file is bad
+// is worse than one carrying a stale switch.
+//
+// Only CACHE_FIX_ keys are honoured, which keeps PATH and LD_PRELOAD out and is
+// the limit of what the prefix buys. It is NOT a lower trust level: within the
+// prefix this file can set CACHE_FIX_PROXY_UPSTREAM, CACHE_FIX_REQUEST_CAPTURE
+// and CACHE_FIX_PROXY_CA_FILE, so write access to it is control of the proxy.
+//
+// ABSENCE MEANS INHERIT, NEVER UNSET. There is no syntax here that removes a
+// key, so deleting a line does not turn a switch off — the value the outgoing
+// holder carries stands, and one set through this file is then sticky across
+// every later handover. Turn a switch off by its own off value
+// (`CACHE_FIX_REQUEST_CAPTURE=0`), not by removing the line.
+//
+// Own module: the launcher is an executable, so importing it runs the CLI.
+// Parsed by hand, not util.parseEnv: measured undefined on node 18, which
+// package.json declares as the minimum — there the call throws, outside the try.
+export const handoverEnvPath = () =>
+  process.env.CACHE_FIX_HANDOVER_ENV || join(claudeHome(), "cache-fix-handover.env");
+
+export function handoverEnv(base, path = handoverEnvPath()) {
+  let text;
+  try { text = readFileSync(path, "utf8"); } catch { return base; }
+  const out = { ...base };
+  // A LINE IS HONOURED ONCE ITS TERMINATOR IS THERE. A half-written file parses
+  // cleanly -- `..._UPSTREAM=http://ho` is a well-formed assignment with a broken
+  // value -- so dropping the unterminated tail is what makes a truncated write
+  // invisible here. It also means a file with no final newline loses its last
+  // line, which is the fail-safe direction: the inherited value stands.
+  for (const line of text.split("\n").slice(0, -1)) {
+    const eq = line.indexOf("=");
+    if (eq < 1) continue;
+    const k = line.slice(0, eq).trim();
+    if (!k.startsWith("CACHE_FIX_")) continue;
+    out[k] = line.slice(eq + 1).trim();
+  }
+  return out;
+}