fix(drain): whitespace was still the guillotine, and a shape filter cannot hide an authority

Both defects are in the commit before this one, found by reviewing it rather
than the code it replaced.

`Number()` READS WHITESPACE AS 0. Rejecting a negative left `" "`, `"\t"` and
`"\n"` -- each trivially written into an env file or a unit -- coercing to an
explicit budget of zero, which for the stall knob is the guillotine the drain
exists to remove. Measured end to end: `CACHE_FIX_DRAIN_STALL_MS=" "` severs a
healthy 3-second-gap stream that survives at any real value. `null` and `-0`
went the same way. Both knobs now take a plain non-negative decimal and fall
back otherwise; exotic spellings that used to be guessed at (`1e3`, `0x10`,
`Infinity`) fall back rather than being interpreted.

A SHAPE FILTER CANNOT ENFORCE A CONTENT RULE. "A single leading slash" admits
`/http://user:pass@host/v1`, `/\user:pass@host/x`, a percent-encoded `//`, a
`;`-prefixed authority, and `#` survives the `?` split entirely -- five ways to
put an authority or a fragment into a line that outlives the process, in the
function whose own header says that is what it prevents. It now judges the
label it is about to write: neither `@` nor `#` belongs in a route.

README already documented both knobs; what it did not say is which values they
accept, which is exactly what changed. Said now, with a CHANGELOG entry.

RED: whitespace " " was read as an explicit budget
     an authority-first target rendered into the log: /http:/user:hunter2@example.test
GREEN: 33/33
mutation: dropping the decimal test kills the whitespace case; dropping the
  [@#] filter kills the authority case; restored 33/33

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

Author: codeslake

CHANGELOG.md

@@ -8,6 +8,8 @@
 
 ### 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.

README.md

@@ -450,8 +450,8 @@ 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. |
-| `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. |
+| `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 |

proxy/server.mjs

@@ -629,9 +629,18 @@ async function handlePassthrough(clientReq, clientRes) {
 // first tick, ending every owed connection at once -- the guillotine this drain
 // replaced, one typo away.
 export function drainBudgetMs(raw, fallback) {
-  if (raw === undefined || raw === "") return fallback;
-  const n = Number(raw);
-  return Number.isFinite(n) && n >= 0 ? n : fallback;
+  if (typeof raw !== "string") return fallback;
+  // A PLAIN NON-NEGATIVE DECIMAL, matched before `Number()` sees it. `Number()`
+  // reads whitespace as 0 -- `" "`, `"\t"`, `"\n"` all coerce -- so a value
+  // that is only whitespace in an env file or a unit becomes an explicit
+  // budget of zero, which is the guillotine this drain exists to remove. It
+  // also lets `-0` through, and `now - at < -0` is false on every tick just as
+  // `< -1` is. Exotic spellings (`1e3`, `0x10`, `Infinity`) fall back rather
+  // than being guessed at; none is a documented form.
+  const s = raw.trim();
+  if (!/^\d+(\.\d+)?$/.test(s)) return fallback;
+  const n = Number(s);
+  return Number.isFinite(n) ? n : fallback;
 }
 
 export function drainRoute(url) {
@@ -644,7 +653,13 @@ export function drainRoute(url) {
   // which is what separates `/v1/messages` from `//host/v1/messages`.
   if (!abs && !(url.startsWith("/") && !url.startsWith("//"))) return "?";
   const path = abs?.pathname ?? url;
-  return "/" + path.split("?")[0].split("/").filter(Boolean).slice(0, 2).join("/");
+  const route = "/" + path.split("?")[0].split("/").filter(Boolean).slice(0, 2).join("/");
+  // THE SHAPE TEST ABOVE CANNOT ENFORCE THIS. `/http://user:pass@host/v1`,
+  // `/\user:pass@host/x` and a percent-encoded `//` are all a single leading
+  // slash, and `#` survives the `?` split. Judge what is about to be WRITTEN:
+  // neither character belongs in a route label, and either one means an
+  // authority or a fragment came through.
+  return /[@#]/.test(route) ? "?" : route;
 }
 
 export function forcedCloseLine(ended, destroyed, held, budgetMs = 5_000, why = "", routes = "", quiet = "", owedAtStart = null) {

test/shutdown-exit-code.test.mjs

@@ -1240,10 +1240,18 @@ describe("SIGTERM exit code", () => {
     // reaches here unparsed and renders the authority verbatim, which is the
     // credential this function exists to keep out of the log. Origin-form is a
     // SINGLE leading slash, so `//host` is not one of them.
+    // A SHAPE FILTER CANNOT ENFORCE A CONTENT RULE. Each of these is a single
+    // leading slash, so a prefix test admits it, and each renders an authority
+    // into a log line that outlives the process.
     for (const target of [
       "//user:hunter2@example.test/v1/messages",
       "ftp://user:hunter2@example.test/v1/messages",
       "example.test:443",
+      "/http://user:hunter2@example.test/v1",
+      "/\\user:hunter2@example.test/x",
+      "/%2F%2Fuser:hunter2@example.test/x",
+      "/;user:hunter2@example.test/x",
+      "/v1/messages#tok=hunter2",
     ]) {
       assert.equal(drainRoute(target), "?",
         `an authority-first target rendered into the log: ${drainRoute(target)}`);
@@ -1265,6 +1273,17 @@ describe("SIGTERM exit code", () => {
     assert.equal(drainBudgetMs("0", 90_000), 0,
       "an explicit zero asks to cut now; it is not an absent setting");
     assert.equal(drainBudgetMs("30000", 90_000), 30_000);
+    // `Number()` reads whitespace as 0, so a value that is only whitespace --
+    // trivially written into an env file or a systemd unit -- becomes the
+    // guillotine, not the default. Same for a non-string caller.
+    for (const blank of [" ", "\t", "\n", "  \t "]) {
+      assert.equal(drainBudgetMs(blank, 90_000), 90_000,
+        `whitespace ${JSON.stringify(blank)} was read as an explicit budget`);
+    }
+    assert.equal(drainBudgetMs(null, 90_000), 90_000);
+    assert.equal(drainBudgetMs("-0", 90_000), 90_000,
+      "negative zero makes `now - at < budget` false on every tick, like -1");
+    assert.equal(drainBudgetMs(" 5 ", 90_000), 5, "a padded number still parses");
     assert.equal(drainRoute(undefined), "?");
   });