docs: address Copilot review round 2 (#2354)

The 45-minute wait is a total queueing budget in the skill too; the lease
promises the holder's release, not every waiter's acquisition; and what it
removes is gate-on-gate contention, for as many starts as fit the budget.

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>
This commit is contained in:
cliffhall 2026-09-13 18:09:18 -04:00
commit a57f2654a9
2 changed files with 7 additions and 4 deletions

View file

@ -176,8 +176,11 @@ OOM'd session — stops refreshing its lock and is taken over after 30s; nothing
needs cleaning up by hand. The one exception is a dead holder's lock directory
that cannot be removed (a stray file inside it, or permissions): the takeover
fails, the waiter keeps waiting, and the wait runs to its 45-minute cap naming
the path — remove that directory by hand. So the give-up happens against a
live gate that has hung, or a stale lock that would not go away; never on its
the path — remove that directory by hand. The cap is a total wait budget, counted
from the waiter's first attempt and not reset as the queue ahead of it drains,
so a queue of healthy gates deeper than it covers — about ten, at ~4.5 minutes
each — reaches it too. So the give-up happens against a live gate that has
hung, a stale lock that would not go away, or a queue that deep; never on its
own.
`INSPECTOR_SKIP_GATE_LEASE=1 npm run local:gate` runs without the lease. It is

View file

@ -98,14 +98,14 @@ The lease is released on every way out — a normal exit, a spawn failure, or a
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, and a release always comes (see the stale takeover above). A `pgrep -f "npm run local:gate"` loop is the same trap from another side — it matches itself and never exits.
- **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`.
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.
### What it means for capacity
**One gate runs at a time — before and after.** What changed is what happens when a second one is started. Before, a second start was a wasted run: one of the two went red on the port collision regardless of budgets, so the honest capacity was one green gate at a time. After, any number can be **started** from separate worktrees and all complete green, back to back — three started within seconds of each other all passed with zero timeouts. Each gate is also shorter (a quiet run went ~358s → ~260s with #2341), so a queue drains sooner: three queued gates finish in roughly 13 minutes in total, against one green, one red and a re-run before. The practical ceiling on queue depth is the 45-minute wait cap — at ~4.5 minutes per gate, about ten can be queued before the last gives up and names the holder. Whether truly parallel gates could ever be green was not measured: the port collision makes that a different project (per-gate port allocation), not a tuning problem, and nothing in #2338's goal needs it. The measurements are collected in [#2338's epilogue](https://github.com/modelcontextprotocol/inspector/issues/2338).
**One gate runs at a time — before and after.** What changed is what happens when a second one is started. Before, a second start was a wasted run: one of the two went red on the port collision regardless of budgets, so the honest capacity was one green gate at a time. After, as many as fit inside the wait budget — about ten, see below — can be **started** from separate worktrees, and each runs back to back with no other gate contending for the machine or its ports. What the lease removes is gate-on-gate contention, not any other reason a check can fail; measured, three started within seconds of each other all completed green with zero timeouts. Each gate is also shorter (a quiet run went ~358s → ~260s with #2341), so a queue drains sooner: three queued gates finish in roughly 13 minutes in total, against one green, one red and a re-run before. The ceiling on queue depth is the 45-minute wait budget — at ~4.5 minutes per gate, about ten can be queued before the last gives up and names the holder, so a deeper queue is one where the last starts give up rather than wait. Whether truly parallel gates could ever be green was not measured: the port collision makes that a different project (per-gate port allocation), not a tuning problem, and nothing in #2338's goal needs it. The measurements are collected in [#2338's epilogue](https://github.com/modelcontextprotocol/inspector/issues/2338).
### What it does not do