fix(proxy): close the activity entry a request opened, exactly once

[ak2k]

Builds on #180, which added the outer catch this uses. Review that one first; the diff here is the last commit only.

Cancel a request in Claude Code with Ctrl+C and the proxy never learns it ended: the TUI's activity row spins forever, and a headless consumer's in-flight count grows by one, permanently, per cancelled upload.

$ node repro-leak.mjs . 5
5 cancelled uploads -> 5 activity rows still open      (0 after this change)

repro-leak.mjs

// Count the activity rows left open after N cancelled uploads.
// usage: node repro-leak.mjs <repo-dir> <n>
import http from 'node:http';
const [, , repo, nArg] = process.argv;
const { AccountManager } = await import(`${repo}/src/account-manager.js`);
const { createProxyServer } = await import(`${repo}/src/server.js`);

const active = new Map();                       // the TUI's `active`, in miniature
const am = new AccountManager([{ name: 'alice', type: 'apikey', apiKey: 'k1' }], 0.98);
const server = createProxyServer(am, { proxy: {}, upstream: 'http://127.0.0.1:1' }, {
  onRequestStart: (id, info) => active.set(id, info),
  onRequestEnd: (id) => active.delete(id),
});
const realErr = console.error;
console.error = () => {};

const port = await new Promise(r => server.listen(0, '127.0.0.1', () => r(server.address().port)));
const body = JSON.stringify({ model: 'claude-opus-5', messages: [] });

for (let i = 0; i < Number(nArg); i++) {
  await new Promise((resolve) => {
    const req = http.request({
      host: '127.0.0.1', port, method: 'POST', path: '/v1/messages',
      headers: { 'content-type': 'application/json', 'content-length': String(body.length + 64) },
    });
    req.on('error', () => {});
    req.write(body.slice(0, 24));               // partial upload, then Ctrl+C
    setTimeout(() => { req.destroy(); resolve(); }, 30);
  });
}
await new Promise(r => setTimeout(r, 300));
console.error = realErr;
console.log(`${nArg} cancelled uploads -> ${active.size} activity rows still open`);
server.close();

Why

The entry is opened by onRequestStart but only closed by the finally around forwardRequest, so anything that throws above the inner try leaves it open forever. Two ordinary things do: a client cancelling mid-upload (for await (const chunk of req) rejects above the inner try), and a start hook that registers its row and then throws, which is the shipped TUI hook's shape (active.set(id, ...), then a render that can rethrow).

The fix

The listener tracks the open entry in one marker: set before the start hook, cleared before the end hook at every closing site, and the outer catch closes whatever is left (499 if the client is gone or the response is past answering, 502 when that is what it is about to write). Each ordering trades a spurious close, which every shipped consumer tolerates, against a permanent leak. The outer catch's own hook call is guarded, because the throw that sent it there may be that hook; unguarded, it escapes as an unhandled rejection, which crash-log.js turns into exit(1).

The recovery has to survive its own logging

Under the TUI the console is the TUI: console.error renders, so a broken render makes the first line of every recovery throw. That half exists on plain master today. onRequestRouted is called inside forwardRequest, whose catch answers 502; run the same request twice and vary only the console:

working console  -> { clientGot: "HTTP 502", escaped: [] }
throwing console -> { clientGot: "HUNG (no response in 3s)", escaped: ["TUI render failed"] }

probe-master-logging.mjs

// Isolate the logging defect on plain upstream master, away from the ledger
// leak and the unanswered-socket defect.
//
// `onRequestRouted` is called INSIDE forwardRequest, so a throwing one lands in
// the inner catch, which is the one recovery master already has: it writes a
// 502. Run it twice, once with a working console and once with a TUI-shaped
// console that throws. The only difference between the runs is the console.
//
// usage: node probe-master-logging.mjs <repo-dir>
import http from 'node:http';
const [, , repo] = process.argv;
const { AccountManager } = await import(`${repo}/src/account-manager.js`);
const { createProxyServer } = await import(`${repo}/src/server.js`);

async function run(consoleThrows) {
  const am = new AccountManager([{ name: 'alice', type: 'apikey', apiKey: 'k1' }], 0.98);
  const escaped = [];
  const onRej = (e) => escaped.push(e?.message);
  process.on('unhandledRejection', onRej);

  const server = createProxyServer(am, { proxy: {}, upstream: 'http://127.0.0.1:1' }, {
    onRequestRouted: () => { throw new Error('routed hook failed'); },
  });
  const port = await new Promise(r => server.listen(0, '127.0.0.1', () => r(server.address().port)));

  const realErr = console.error;
  console.error = consoleThrows
    ? () => { throw new Error('TUI render failed'); }
    : () => {};

  const answer = await new Promise((resolve) => {
    const req = http.request({
      host: '127.0.0.1', port, method: 'POST', path: '/v1/messages',
      headers: { 'content-type': 'application/json' },
    }, (res) => { res.resume(); res.on('end', () => resolve(`HTTP ${res.statusCode}`)); });
    req.on('error', (e) => resolve(`no answer (${e.code})`));
    setTimeout(() => resolve('HUNG (no response in 3s)'), 3000);
    req.end(JSON.stringify({ model: 'claude-opus-5', messages: [] }));
  });

  await new Promise(r => setTimeout(r, 200));
  console.error = realErr;
  process.off('unhandledRejection', onRej);
  server.close();
  return { consoleThrows, clientGot: answer, escaped };
}

console.log(JSON.stringify(await run(false), null, 2));
console.log(JSON.stringify(await run(true), null, 2));
process.exit(0);

Inside this change, one broken render causes both throws at once. Measured before this part was added: { rowsStillOpen: 1, clientGot: "HUNG", escaped: ["TUI render failed"] }. After: { rowsStillOpen: 0, clientGot: "HTTP 502", escaped: [] }.

So every recovery-path report goes through one reportFailure helper that falls back to stderr with the full stack, as tui.js already does when its own activity stream fails. The fallback writes with writeSync, not process.stderr.write: a closed stderr surfaces EPIPE asynchronously where no try can catch it, and this daemon treats an uncaught EPIPE as fatal.

process.stderr.write -> { code: 9, out: ["after=none", "UNCAUGHT:EPIPE"] }
writeSync(2, ...)    -> { code: 0, out: ["after=CAUGHT:EPIPE", "STILL-ALIVE"] }

Three of the four converted sites are each held by a test; the fourth, the inner catch around forwardRequest, is converted for consistency and carries no claim.

Tests

Eight in test/activity-entries.test.js. Six fail on this PR's base:

✖ a request aborted mid-body closes its activity entry
✖ a start hook that registers its row and then throws does not orphan it
✖ a hook that throws on every call cannot bring the process down
✖ a console that throws does not defeat the recovery
✖ a console that throws does not defeat the control-plane recovery
✖ a report to a closed stderr does not kill the process

The three console tests run the proxy in a child with a throwing (not muted) console.error, since a raw fd-2 write bypasses any in-process stub. The other two pass on the base and pin the clear-before-call orderings; moving either clear turns its test red.

538/538 (530 on the base), #180's five tests unmodified, npx eslint . clean.

Compatibility

Accounting and hooks only; no routing, selection, retry or upstream changes. Clients see a difference in two places, neither a loss: an abandoned request's entry now closes as 499 where the client previously got nothing, and under a throwing TUI render, requests that used to hang now get their 502. onRequestEnd consumers can now see account: null, model: null with a 499 or 502 for a request that never reached selection, the same shape an early failure already produced.