docs: address Copilot review round 3 (#2354)

Narrow the lease's guarantees to what acquireLease and finishLease do; say
where no release ever comes; and record the concurrent-sibling arm of the
maxWorkers measurement beside the solo one.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PsmUAZ6aLUAFqenjJKSSRp
Signed-off-by: cliffhall <cliff@futurescale.com>

docs/quality-gate.md

@@ -91,15 +91,15 @@ The machine this repo is worked on runs three or four agent sessions at once, ea
 
 `npm run local:gate` is `node scripts/gate-lease.mjs npm run local:gate:stages`: the stage chain runs as the child of a wrapper that holds **one machine-wide lease for the whole run**. The lock lives under `os.tmpdir()` — or `$XDG_RUNTIME_DIR` where a desktop session sets it — and never inside a worktree, because a lock in the repo would be one per worktree, which is one per session, which coordinates nothing. A second gate started in another worktree while one holds the lease **waits**: it prints one line naming the holder's pid, worktree and elapsed time, re-asks every 2s, says `still waiting` once a minute, and starts the moment the holder releases.
 
-The lease is released on every way out — a normal exit, a spawn failure, or a signal. On POSIX the wrapper runs the chain in its own process group, so Ctrl-C, a plain `kill`, a closed terminal or Ctrl-\ stop the whole tree (`npm run` nests four deep and `sh` forwards nothing), release, and exit `128 + signal`; on Windows there is no process group (`spawnSpec` spawns through `cmd.exe`, not detached), the signal is delivered to the immediate child only, and the lease is released just the same — what is not guaranteed there is that every descendant of that child has stopped. A holder that dies without releasing — a killed terminal, an OOM'd session — stops refreshing its lock and is taken over after **30s** (`proper-lockfile`'s stale detection, at 30s rather than the library's 10s default: a refresh timer firing late under the very load this exists to manage would otherwise cause a false takeover, which is two gates overlapping — the thing being prevented — whereas the cost of the longer window is 20 more seconds after a crash, against a gate that takes minutes). A waiter gives up after **45 minutes**, naming the holder and the lock path. That budget is a **total queueing budget**: it counts from the waiter's first attempt and is not reset when the holder ahead releases and another queued gate takes the lease, so it expires against a live gate that has hung, a dead holder's lock directory that cannot be removed, or a queue deeper than the budget covers — about ten gates at ~4.5 minutes each (see [capacity](#what-it-means-for-capacity)). A give-up behind a queue of healthy gates is that third case, not a hung holder; the line it prints names whichever gate holds the lease at that moment. `INSPECTOR_SKIP_GATE_LEASE=1` bypasses the lease — for a measurement that *needs* contention, never for a sooner result, since the queued run finishes before an overlapped one would. And the lease can never fail the gate it wraps: a lock that cannot be *created* (an unwritable tmpdir) runs the gate unleased with a warning.
+The lease is released on every way out — a normal exit, a spawn failure, or a signal. On POSIX the wrapper runs the chain in its own process group, so Ctrl-C, a plain `kill`, a closed terminal or Ctrl-\ stop the whole tree (`npm run` nests four deep and `sh` forwards nothing), release, and exit `128 + signal`; on Windows there is no process group (`spawnSpec` spawns through `cmd.exe`, not detached), the signal is delivered to the immediate child only, and the lease is released just the same — what is not guaranteed there is that every descendant of that child has stopped. A holder that dies without releasing — a killed terminal, an OOM'd session — stops refreshing its lock and is taken over after **30s** (`proper-lockfile`'s stale detection, at 30s rather than the library's 10s default: a refresh timer firing late under the very load this exists to manage would otherwise cause a false takeover, which is two gates overlapping — the thing being prevented — whereas the cost of the longer window is 20 more seconds after a crash, against a gate that takes minutes). A waiter gives up after **45 minutes**, naming the holder and the lock path. That budget is a **total queueing budget**: it counts from the waiter's first attempt and is not reset when the holder ahead releases and another queued gate takes the lease, so it expires against a live gate that has hung, a dead holder's lock directory that cannot be removed, or a queue deeper than the budget covers — about ten gates at ~4.5 minutes each (see [capacity](#what-it-means-for-capacity)). A give-up behind a queue of healthy gates is that third case, not a hung holder; the line it prints names whichever gate holds the lease at that moment. `INSPECTOR_SKIP_GATE_LEASE=1` bypasses the lease — for a measurement that *needs* contention, never for a sooner result, since the queued run finishes before an overlapped one would. And the lease adds only one way for the wrapper itself to exit red — that 45-minute give-up, which happens before any stage is spawned. A lock that cannot be *created* (an unwritable tmpdir) degrades to an unleased run with a warning rather than failing, and every handled exit *attempts* the release; if the release itself fails, the wrapper says so and leaves the lock for the stale takeover, or for a hand removal if whatever blocked it persists.
 
 ### Why a queue, and not a load-based wait or a worker cap
 
 Each alternative was ruled out by a measurement rather than an argument:
 
 - **Overlapping gates are red by construction, so no budget or worker cap could make two of them green.** The web smokes bind fixed ports — 6295–6299 by default (`smoke:web:tabs` 6295, `smoke:web:elicit` 6296, `smoke:web:app` 6297, `smoke:web:browser` 6298, `smoke:web` 6299), plus the sandbox and app-origin ports. Measured for #2339 on `aa56551b`: a quiet gate took 257s and passed; two started together in separate worktrees had one **fail** at 279s — `smoke:web:chromium` found port 6298 held by the other gate's copy of the same smoke — while the survivor took 338s, ~1.3x, with zero test timeouts. That is a red run on a diff that cannot have caused it, the exact failure #2338 exists to stop, and only not overlapping fixes it. (The corollary corrected the effort's own framing: at two sessions the load inflation is ~1.3x; the 2.5x recorded on [#2323](https://github.com/modelcontextprotocol/inspector/issues/2323), with ~326 tests timing out at 5s, was the four-worktree number.)
-- **A load-threshold wait deadlocks.** Two sessions each polling "is the machine quiet yet?" never clear, because each is the reason the other is waiting — the trap #2339 recorded before any code existed. A lease *grants* instead: exactly one waiter wins the `mkdir`, runs, and releases; the rest keep asking until they acquire it or exhaust their own 45-minute budget. There is nothing to wait *for* except the holder's release, and that always comes — by exit, by signal, or by the stale takeover above — which is what a load threshold cannot promise. A `pgrep -f "npm run local:gate"` loop is the same trap from another side — it matches itself and never exits.
-- **A worker cap is a permanent solo-run cost bought against a failure that does not occur.** Measured for #2336 (PR [#2345](https://github.com/modelcontextprotocol/inspector/pull/2345)) at the leased baseline, arms interleaved over three rounds: `maxWorkers: 4` cost +14% wall on web `unit` and +39% on `coverage:web` — ~35s on every `coverage:web`, a tenth of the whole gate — for zero fewer failures (0 across all 18 runs). With gates serialized, the oversubscription a cap would mitigate comes only from non-gate work in a sibling session, and against a concurrent bare unit run the total throughput was the same with both sides capped as with both at the default, because eight cores are saturated either way. So `maxWorkers` stays unset, stated as a choice beside `TIMEOUTS` in `vitest.shared.mts`.
+- **A load-threshold wait deadlocks.** Two sessions each polling "is the machine quiet yet?" never clear, because each is the reason the other is waiting — the trap #2339 recorded before any code existed. A lease *grants* instead: exactly one waiter wins the `mkdir`, runs, and releases; the rest keep asking until they acquire it or exhaust their own 45-minute budget. There is nothing to wait *for* except the holder's release — by exit, by signal, or by the stale takeover above — and the wait for it is bounded, which is what a load threshold cannot promise. The one case where no release ever comes is the unremovable stale lock above, and there the waiter still exits at its budget naming the path rather than waiting forever. A `pgrep -f "npm run local:gate"` loop is the same trap from another side — it matches itself and never exits.
+- **A worker cap is a permanent solo-run cost bought against a failure that does not occur at the leased baseline.** Measured for #2336 (PR [#2345](https://github.com/modelcontextprotocol/inspector/pull/2345)), arms interleaved over three rounds, nothing else running: `maxWorkers: 4` cost +14% wall on web `unit` and +39% on `coverage:web` — ~35s on every `coverage:web`, a tenth of the whole gate — for zero fewer failures (0 across all 18 runs). With gates serialized, the oversubscription a cap would mitigate comes only from non-gate work in a sibling session, and that arm was measured separately against a concurrent bare unit run: total throughput was the same with both sides capped as with both at the default, because eight cores are saturated either way, and the one thing a cap moved was failures — 2 of 10 uncapped 7-vs-7 runs lost `AppRenderer.test.tsx`'s theme-flip case to its 5s inner `waitFor` at 14 workers, against 0 of 12 with a cap on either side. That is a single test's inner budget starving under two sessions' worth of workers, fixable at that site, not a case for taxing every solo run. So `maxWorkers` stays unset, stated as a choice beside `TIMEOUTS` in `vitest.shared.mts`.
 
 Two further calls, recorded on PR #2344: the lease covers the **whole gate**, not only its parallel stages, because the serial stages are not free (type-aware lint, a full Vite build) and per-stage leases would let two gates interleave into the same contention; and it is **on by default**, because the sessions that need it are the ones that did not think to opt in.