modelcontextprotocol/inspector
Watch
1
0
Fork
You've already forked inspector
 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions 2

feat: queue concurrent local:gate runs on a machine-wide lease (#2339)

Browse source 
`npm run local:gate` now runs its stages under `scripts/gate-lease.mjs`, a
`proper-lockfile` lease under `os.tmpdir()` (or `$XDG_RUNTIME_DIR`), so a gate
started in a second worktree waits for the first instead of overlapping it.

Measured on v2/main @ aa56551b: a quiet gate is 257s green; two started
together had one fail at 279s (`smoke:web:chromium` found port 6298 held by
the other gate's smoke) and the survivor take 338s. Under the lease, three
gates started within seconds ran back to back (259s, then 262s, then the
third), each green with zero timeouts.

The lease grants rather than polls a threshold, so two waiters cannot
deadlock on each other; a holder that dies stops refreshing and is taken over
after 30s; a signal to the wrapper stops the whole process group, releases,
and exits 128+signal; the wait names the holder's pid and worktree and gives
up after 45 minutes. `INSPECTOR_SKIP_GATE_LEASE=1` bypasses it. A lock that
cannot be created runs the gate unleased with a warning.

`scriptChainRuns` looks through the wrapper (and only it) so the existing
vouches about what `local:gate` runs keep reading the real chain.

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-12 22:00:33 -04:00
commit f99402918d
10 changed files with 849 additions and 12 deletions
Download patch file Download diff file Expand all files Collapse all files

34
.claude/skills/pre-push-gate/SKILL.md
View file

@ -148,9 +148,37 @@ Vite's `fs.allow`. Do a real `npm install` in the worktree.
### Everything times out at once ### Everything times out at once
⚠️ Two concurrent `npm run local:gate` runs starve each other — ~326 tests time ⚠️ Two `npm run local:gate` runs on one machine starve each other — under four
out at 5s. Run one at a time. (A `pgrep -f "npm run local:gate"` wait loop worktrees ~326 tests timed out at 5s (#2323), and even two collide
matches _itself_ and never exits.) deterministically on the web smokes' fixed ports. Since #2339 the gate takes a
**machine-wide lease**, so a second run queues rather than overlapping; if
everything is still timing out at once, look for what is _not_ the gate: a
bypassed lease (`INSPECTOR_SKIP_GATE_LEASE` set in that shell), a bare
`npm run coverage` or `test:storybook` in another session, or Spotlight
indexing a fresh `node_modules` (a `mdworker` storm after `npm install` in a
new worktree pushed the load average to 20 for ten minutes). Do not write a
"wait until the machine is clear" loop — two of them deadlock on each other,
and a `pgrep -f "npm run local:gate"` loop matches _itself_ and never exits.
### Waiting on the lease
A gate that starts with
```
gate-lease: pid 12345 in /Users/you/Projects/mcp-inspector-wt-1, running for 2m10s holds the gate lease; waiting …
```
is queued behind another worktree's gate, and will start the moment it
releases (it re-checks every 2s and prints `still waiting` once a minute). The
holder's pid and worktree are in the line, so you can decide whether to wait
or to stop that gate. A holder that was **killed** — a closed terminal, an
OOM'd session — stops refreshing its lock and is taken over after 30s; nothing
needs cleaning up by hand. The wait gives up after 45 minutes, naming the
holder, which only ever happens against a live gate that has hung.
`INSPECTOR_SKIP_GATE_LEASE=1 npm run local:gate` runs without the lease. It is
for a measurement that needs contention; it does not get a result sooner,
because the queued run finishes before an overlapped one would.
## Local-only steps ## Local-only steps

1
AGENTS.md
View file

@ -476,6 +476,7 @@ free to run a full `local:gate`). Raising a budget nobody chose hides no race.
- **`npm run local:gate` is the mandatory pre-push command.** It runs **every check** `.github/workflows/main.yml` runs, plus two local-only steps, so a green run here is the strongest predictor of a green CI this repo has: every check CI applies has already passed on your machine. It is not a proof — CI runs on a different OS and, since #2341, runs each client's suite bare where the gate runs it only instrumented — so the one residual is a test that passes *only* when slowed down, which is a race (#1596) to fix, never headroom to keep. Expect several minutes. - **`npm run local:gate` is the mandatory pre-push command.** It runs **every check** `.github/workflows/main.yml` runs, plus two local-only steps, so a green run here is the strongest predictor of a green CI this repo has: every check CI applies has already passed on your machine. It is not a proof — CI runs on a different OS and, since #2341, runs each client's suite bare where the gate runs it only instrumented — so the one residual is a test that passes *only* when slowed down, which is a race (#1596) to fix, never headroom to keep. Expect several minutes.
- **The gate runs each client's test suite once, instrumented; CI runs it twice (#2341).** CI's `build` job runs the bare `test` inside `validate` and its parallel `coverage` job runs `test:coverage`, which costs CI no wall clock. Serially in one process the bare pass was ~80s of a ~370s gate, re-running exactly the files the coverage pass runs a few minutes later — so the gate calls **`local:validate`**, which is `validate` minus each client's `test` leg (every client's `validate` is `check && test`, and `local:validate` runs the `check` half). "Every check" is therefore a claim about checks, not invocations, and it holds because `@vitest/coverage-v8` collects coverage from V8's own profiler (`Profiler.takePreciseCoverage`) and rewrites no source: a test file sees identical code either way, and the instrumented run is only *slower*, which makes it the stricter of the two for the failure class this repo actually sees (a correct test cut off under load). A test that passed only *because* it ran slower would be a race — #1596's class, a defect wherever it surfaces — and CI's bare pass still runs it. **`npm run validate` is unchanged**, because CI runs it directly and it is the inner-loop check; `verify:*` guards that ask "is this reachable from `validate`" are unaffected. `local:validate` lives in the `local:` namespace so the workflow guard keeps it out of CI by construction. - **The gate runs each client's test suite once, instrumented; CI runs it twice (#2341).** CI's `build` job runs the bare `test` inside `validate` and its parallel `coverage` job runs `test:coverage`, which costs CI no wall clock. Serially in one process the bare pass was ~80s of a ~370s gate, re-running exactly the files the coverage pass runs a few minutes later — so the gate calls **`local:validate`**, which is `validate` minus each client's `test` leg (every client's `validate` is `check && test`, and `local:validate` runs the `check` half). "Every check" is therefore a claim about checks, not invocations, and it holds because `@vitest/coverage-v8` collects coverage from V8's own profiler (`Profiler.takePreciseCoverage`) and rewrites no source: a test file sees identical code either way, and the instrumented run is only *slower*, which makes it the stricter of the two for the failure class this repo actually sees (a correct test cut off under load). A test that passed only *because* it ran slower would be a race — #1596's class, a defect wherever it surfaces — and CI's bare pass still runs it. **`npm run validate` is unchanged**, because CI runs it directly and it is the inner-loop check; `verify:*` guards that ask "is this reachable from `validate`" are unaffected. `local:validate` lives in the `local:` namespace so the workflow guard keeps it out of CI by construction.
- **`npm run validate` is the fast inner-loop check and is NOT an acceptable substitute.** It runs `test`, not `test:coverage`, so it does **zero** coverage gating, no smokes, and no Storybook tests. Skipping the gate is how a push passes every fast local check and still fails CI. - **`npm run validate` is the fast inner-loop check and is NOT an acceptable substitute.** It runs `test`, not `test:coverage`, so it does **zero** coverage gating, no smokes, and no Storybook tests. Skipping the gate is how a push passes every fast local check and still fails CI.
- **Concurrent gates queue; they do not overlap.** `local:gate` takes a machine-wide lease (`scripts/gate-lease.mjs`, #2339) so a gate started in a second worktree waits for the first rather than running alongside it. Overlap is not merely slow — the web smokes bind fixed ports, so two gates reaching the same smoke together go red on a diff that cannot have caused it (measured: one of two concurrent gates failed at 279s on port 6298 while a quiet gate passed in 257s). The wait names the holder and its worktree; a holder that dies releases within 30s; `INSPECTOR_SKIP_GATE_LEASE=1` bypasses it, which is for a measurement that _needs_ contention, never for getting a result sooner — the queued run finishes sooner anyway.
- There is deliberately **no `npm run ci`** — that name collided with the `npm ci` built-in, which clean-installs from the lockfile and does not run this script. - There is deliberately **no `npm run ci`** — that name collided with the `npm ci` built-in, which clean-installs from the lockfile and does not run this script.
- What each stage covers, and why two of them are local-only, is [`docs/quality-gate.md`](./docs/quality-gate.md); how to diagnose a failing stage is the `pre-push-gate` skill. - What each stage covers, and why two of them are local-only, is [`docs/quality-gate.md`](./docs/quality-gate.md); how to diagnose a failing stage is the `pre-push-gate` skill.

2
docs/quality-gate.md
View file

@ -43,7 +43,7 @@ That is the readable half, and prose rots. The enforced half is `scripts/lib/wor
| `npm run verify:typecheck-coverage` | The typecheck-coverage analog of the above (#1791): for each Node client (auto-discovered from disk — enrolled via its `typecheck` script's projects, or for a `tsc -b` client like `clients/web` via its `tsconfig.json` `references`) it runs those projects with `tsc --listFilesOnly`, unions them, and **fails** listing any tracked `.ts`/`.tsx`/`.mts`/`.cts` under the client that lands in no project (so a new top-level config/helper can't silently go untypechecked). It also requires, deny-by-default, the first-party TS no client owns (`test-servers/src`, the root `vitest.shared.mts`, all of `core/`, and any new top-level location) to land in some client project's tsc pass — so a `core` `*.tsx` web's projects don't reach is caught too. Also asserts the gate is wired (each client's typecheck pass — its `typecheck` script, or web's `tsc -b` — is reachable from its `validate`, and the root chain runs each client's `validate`). Runs in `validate`. | | `npm run verify:typecheck-coverage` | The typecheck-coverage analog of the above (#1791): for each Node client (auto-discovered from disk — enrolled via its `typecheck` script's projects, or for a `tsc -b` client like `clients/web` via its `tsconfig.json` `references`) it runs those projects with `tsc --listFilesOnly`, unions them, and **fails** listing any tracked `.ts`/`.tsx`/`.mts`/`.cts` under the client that lands in no project (so a new top-level config/helper can't silently go untypechecked). It also requires, deny-by-default, the first-party TS no client owns (`test-servers/src`, the root `vitest.shared.mts`, all of `core/`, and any new top-level location) to land in some client project's tsc pass — so a `core` `*.tsx` web's projects don't reach is caught too. Also asserts the gate is wired (each client's typecheck pass — its `typecheck` script, or web's `tsc -b` — is reachable from its `validate`, and the root chain runs each client's `validate`). Runs in `validate`. |
| `npm run verify:dep-lockstep` | Guards the "one version per install-crossing dependency" invariant (#1896). v2 is not a workspace, so a client's test project compiles the shared first-party TypeScript — `core/`, `test-servers/src`, and the root-owned `vitest.shared.mts`, all of which resolve their dependencies from the **root** install — alongside the client's own sources, putting the same package in one `tsc` program twice. At the same version that's harmless; skewed, TypeScript must relate two structurally-distinct copies of every type, which for a recursive-generic surface is exponential (zod `4.3.6` vs `4.4.3` exhausted the 4GB tsc heap in `clients/web`). Derives its candidate set from **what actually enters each program** (#1965) — every client tsconfig project listed with `tsc --listFilesOnly` via the shared `scripts/lib/tsc-program.mjs`, each resolved `node_modules` file mapped to its owning install, keeping the packages that reach one program from two installs (a package whose declarations arrive only through another package's `.d.ts`, as `@modelcontextprotocol/sdk`'s do, is invisible to a scan of first-party imports). Prices each copy from the lockfile entry for the exact install path the program resolved, compares only the installs that met in one program, and **fails deny-by-default** on any disagreement not in the annotated `TOLERATED_SKEW` allowlist — empty today — with an allowlisted package tolerated only *within a major version*. A **second tier** (#2226) runs alongside it, asking the weaker but broader question the `AGENTS.md` rule actually states: does a package this repo *declares* anywhere resolve to two versions across our installs at all? Its candidate set is every name in any install's `dependencies`/`devDependencies`/`optionalDependencies` — unioned across the root and all four clients, so a copy declared by only one of them still counts — that **more than one install holds a top-level copy of** (17 packages today). Nested copies are excluded: one exists because some dependency asked for a different version, so it is that dependency's range to govern, not ours. Neither tier subsumes the other — the program tier sees a copy no manifest names (`@modelcontextprotocol/sdk`, arriving through another package's `.d.ts`), while the declared tier sees a **transitive** copy no program loads (cli's `@types/node`, hoisted via `@types/express` — the case that motivated it), two **clients** disagreeing with no root copy involved (`@types/react`, web against tui), and the peer shadows `eslint`/`typescript`/`vitest` that never enter a program. Same deny-by-default and same within-a-major rule, against its own `TOLERATED_DECLARED_SKEW` — also empty. Two limits: it reads **lockfiles**, so an uncommitted hand-installed copy is invisible, and it compares only declared names, so a purely transitive package no manifest names stays the first tier's business. Runs in `validate`. | `npm run verify:dep-lockstep` | Guards the "one version per install-crossing dependency" invariant (#1896). v2 is not a workspace, so a client's test project compiles the shared first-party TypeScript — `core/`, `test-servers/src`, and the root-owned `vitest.shared.mts`, all of which resolve their dependencies from the **root** install — alongside the client's own sources, putting the same package in one `tsc` program twice. At the same version that's harmless; skewed, TypeScript must relate two structurally-distinct copies of every type, which for a recursive-generic surface is exponential (zod `4.3.6` vs `4.4.3` exhausted the 4GB tsc heap in `clients/web`). Derives its candidate set from **what actually enters each program** (#1965) — every client tsconfig project listed with `tsc --listFilesOnly` via the shared `scripts/lib/tsc-program.mjs`, each resolved `node_modules` file mapped to its owning install, keeping the packages that reach one program from two installs (a package whose declarations arrive only through another package's `.d.ts`, as `@modelcontextprotocol/sdk`'s do, is invisible to a scan of first-party imports). Prices each copy from the lockfile entry for the exact install path the program resolved, compares only the installs that met in one program, and **fails deny-by-default** on any disagreement not in the annotated `TOLERATED_SKEW` allowlist — empty today — with an allowlisted package tolerated only *within a major version*. A **second tier** (#2226) runs alongside it, asking the weaker but broader question the `AGENTS.md` rule actually states: does a package this repo *declares* anywhere resolve to two versions across our installs at all? Its candidate set is every name in any install's `dependencies`/`devDependencies`/`optionalDependencies` — unioned across the root and all four clients, so a copy declared by only one of them still counts — that **more than one install holds a top-level copy of** (17 packages today). Nested copies are excluded: one exists because some dependency asked for a different version, so it is that dependency's range to govern, not ours. Neither tier subsumes the other — the program tier sees a copy no manifest names (`@modelcontextprotocol/sdk`, arriving through another package's `.d.ts`), while the declared tier sees a **transitive** copy no program loads (cli's `@types/node`, hoisted via `@types/express` — the case that motivated it), two **clients** disagreeing with no root copy involved (`@types/react`, web against tui), and the peer shadows `eslint`/`typescript`/`vitest` that never enter a program. Same deny-by-default and same within-a-major rule, against its own `TOLERATED_DECLARED_SKEW` — also empty. Two limits: it reads **lockfiles**, so an uncommitted hand-installed copy is invisible, and it compares only declared names, so a purely transitive package no manifest names stays the first tier's business. Runs in `validate`.
| `npm run verify:test-timeouts` | Guards the wall-clock budgets the test gates run under (#2323). The class it encodes against is a budget **nobody chose**: three of the six Vitest projects ran on Vitest's own `testTimeout: 5000` and five on its `hookTimeout`/`teardownTimeout: 10000`, sized for an idle machine rather than the one this team works on — three or four concurrent agent sessions in separate worktrees, each free to run a full `local:gate`, on eight logical cores. A correct, deterministic test cut off by such a budget fails a gate its diff did not break, which is the same "channel nobody trusts" failure [Lint has no warning tier](../AGENTS.md#lint-has-no-warning-tier) describes from the other direction; #2292, #1942 and #1742 were each an instance, found one site at a time. Deliberately **not** cited: #2278 (a missing condition wait around a geometry read) and #2250 (a real race in a test's own timing) were fixed by making the test wait for the right thing, and presenting a race fix as evidence for a larger ceiling would argue against #1596. It asks **Vitest itself** to resolve each of the four configs and reads the number a test actually gets, rather than checking that a key is absent from a config block — which would pass just as happily on a config that had stopped being loaded. A **seventh project** with no row, or a config file it does not discover (it knows all twelve filenames Vitest accepts, not just the two this repo uses), is an error rather than a silent skip. It also checks that every project loads `vitest.setup.shared.mts`. ⚠️ **Everything it reads comes from a resolved project, never from source text** — the two rules no config can report are asserted at runtime instead: `retry` by `vitest.setup.shared.mts` (which reads the value Vitest resolved, so a per-test option, a `describe` option, a project setting and a `--retry` flag are one check) and Testing Library's `asyncUtilTimeout` by `clients/web/src/test/asyncUtilTimeout.test.ts` (which reads what the project's own `waitFor`s use). Both began as source scanning in #2334 and the review found a new valid spelling missed in five consecutive rounds, so the boundary is deliberate: if a rule cannot be answered by asking the tool, assert it at runtime rather than reading the source for it. Observed **red** against the pre-#2323 config before it was trusted. Runs in `validate`; its decision logic has a sibling `verify-test-timeouts.test.mjs` under `test:scripts`. | `npm run verify:test-timeouts` | Guards the wall-clock budgets the test gates run under (#2323). The class it encodes against is a budget **nobody chose**: three of the six Vitest projects ran on Vitest's own `testTimeout: 5000` and five on its `hookTimeout`/`teardownTimeout: 10000`, sized for an idle machine rather than the one this team works on — three or four concurrent agent sessions in separate worktrees, each free to run a full `local:gate`, on eight logical cores. A correct, deterministic test cut off by such a budget fails a gate its diff did not break, which is the same "channel nobody trusts" failure [Lint has no warning tier](../AGENTS.md#lint-has-no-warning-tier) describes from the other direction; #2292, #1942 and #1742 were each an instance, found one site at a time. Deliberately **not** cited: #2278 (a missing condition wait around a geometry read) and #2250 (a real race in a test's own timing) were fixed by making the test wait for the right thing, and presenting a race fix as evidence for a larger ceiling would argue against #1596. It asks **Vitest itself** to resolve each of the four configs and reads the number a test actually gets, rather than checking that a key is absent from a config block — which would pass just as happily on a config that had stopped being loaded. A **seventh project** with no row, or a config file it does not discover (it knows all twelve filenames Vitest accepts, not just the two this repo uses), is an error rather than a silent skip. It also checks that every project loads `vitest.setup.shared.mts`. ⚠️ **Everything it reads comes from a resolved project, never from source text** — the two rules no config can report are asserted at runtime instead: `retry` by `vitest.setup.shared.mts` (which reads the value Vitest resolved, so a per-test option, a `describe` option, a project setting and a `--retry` flag are one check) and Testing Library's `asyncUtilTimeout` by `clients/web/src/test/asyncUtilTimeout.test.ts` (which reads what the project's own `waitFor`s use). Both began as source scanning in #2334 and the review found a new valid spelling missed in five consecutive rounds, so the boundary is deliberate: if a rule cannot be answered by asking the tool, assert it at runtime rather than reading the source for it. Observed **red** against the pre-#2323 config before it was trusted. Runs in `validate`; its decision logic has a sibling `verify-test-timeouts.test.mjs` under `test:scripts`.
| `npm run local:gate` | **Mandatory pre-push command.** `local:validate` → `verify:skills:cli` → `coverage` → `verify:build-gate` → `verify:bundle-externals` → `smoke` → `smoke:web:firefox` → `local:storybook`. Every GitHub CI check plus two local-only ones — see [Two tiers](#two-tiers-github-ci-and-the-local-gate). Named `local:` rather than `ci` on purpose (#2146); there is no `npm run ci` alias. | | `npm run local:gate` | **Mandatory pre-push command.** `local:validate` → `verify:skills:cli` → `coverage` → `verify:build-gate` → `verify:bundle-externals` → `smoke` → `smoke:web:firefox` → `local:storybook`. Every GitHub CI check plus two local-only ones — see [Two tiers](#two-tiers-github-ci-and-the-local-gate). Named `local:` rather than `ci` on purpose (#2146); there is no `npm run ci` alias. **Runs under a machine-wide lease** (`scripts/gate-lease.mjs`, [#2339](https://github.com/modelcontextprotocol/inspector/issues/2339)): a second `local:gate` started in another worktree waits for the first to finish instead of running alongside it. Measured on `aa56551b`, a quiet gate took 257s; two started together had one **fail** at 279s on a smoke port collision (`smoke:web:chromium` binds 6298, and so did the other gate's) and the survivor take 338s — so overlapping gates are red by construction, not merely slow, and back to back both finish green in ~2x257s. The wait prints the holder's pid and worktree; a holder that dies without releasing is taken over after 30s (`proper-lockfile` stale detection); `INSPECTOR_SKIP_GATE_LEASE=1` bypasses it. The stages themselves are `local:gate:stages`, which the wrapper runs verbatim. |
| `npm run local:validate` | The gate's first stage: `validate` with each client's `test` leg removed (#2341) — the same `validate:guards` and `validate:core`, then every client's `check` — `format:check` + `lint` + `typecheck`, plus `build` for web, tui and launcher, which name one explicitly (cli's only validate-time build was `test`'s `pretest` hook, so it goes with the `test` leg and `coverage:cli` builds the binary once, later) — each client's `validate` being `check && test`. The suites run once, under `coverage`, instead of bare here and instrumented there. In the `local:` namespace so the workflow guard keeps it out of CI, where the bare pass is free. | | `npm run local:validate` | The gate's first stage: `validate` with each client's `test` leg removed (#2341) — the same `validate:guards` and `validate:core`, then every client's `check` — `format:check` + `lint` + `typecheck`, plus `build` for web, tui and launcher, which name one explicitly (cli's only validate-time build was `test`'s `pretest` hook, so it goes with the `test` leg and `coverage:cli` builds the binary once, later) — each client's `validate` being `check && test`. The suites run once, under `coverage`, instead of bare here and instrumented there. In the `local:` namespace so the workflow guard keeps it out of CI, where the bare pass is free. |
| `npm run pack:verify` | Publish smoke — see [Publishing](./publishing.md). | | `npm run pack:verify` | Publish smoke — see [Publishing](./publishing.md). |

3
package.json
View file

@ -38,7 +38,8 @@
"build:tui": "cd clients/tui && npm run build", "build:tui": "cd clients/tui && npm run build",
"build:web": "cd clients/web && npm run build", "build:web": "cd clients/web && npm run build",
"build:launcher": "cd clients/launcher && npm run build", "build:launcher": "cd clients/launcher && npm run build",
"local:gate": "npm run local:validate && npm run verify:skills:cli && npm run coverage && npm run verify:build-gate && npm run verify:bundle-externals && npm run smoke && npm run smoke:web:firefox && npm run local:storybook", "local:gate": "node scripts/gate-lease.mjs npm run local:gate:stages",
"local:gate:stages": "npm run local:validate && npm run verify:skills:cli && npm run coverage && npm run verify:build-gate && npm run verify:bundle-externals && npm run smoke && npm run smoke:web:firefox && npm run local:storybook",
"local:validate": "npm run validate:guards && npm run validate:core && cd clients/web && npm run check && cd ../cli && npm run check && cd ../tui && npm run check && cd ../launcher && npm run check", "local:validate": "npm run validate:guards && npm run validate:core && cd clients/web && npm run check && cd ../cli && npm run check && cd ../tui && npm run check && cd ../launcher && npm run check",
"local:storybook": "cd clients/web && npx playwright install chromium && npm run test:storybook", "local:storybook": "cd clients/web && npx playwright install chromium && npm run test:storybook",
"verify:build-gate": "node scripts/verify-build-gate.mjs", "verify:build-gate": "node scripts/verify-build-gate.mjs",

401
scripts/gate-lease.mjs Normal file
View file

@ -0,0 +1,401 @@
#!/usr/bin/env node
/**
* Run a command under a machine-wide lease, so that concurrent `npm run
* local:gate` runs in different worktrees queue instead of contending (#2339).
*
* **Two things go wrong when gates overlap, and both were measured** on
* `v2/main` at `aa56551b` (M3, 8 logical cores). A quiet gate took 257s and
* passed. Two started together in separate worktrees: one **failed** at 279s
* — `smoke:web:chromium` found its fixed port (6298) held by the other gate's
* copy of the same smoke — and the survivor took 338s, 1.3x, with the machine
* half-free for its last minute. So the first problem is deterministic, not a
* load effect: the web smokes bind fixed ports (6296–6299, plus the sandbox
* and app-origin ports), so two gates reaching the same smoke together produce
* a red run whose diff cannot have caused it — exactly the failure #2338
* exists to stop. The second is load: with three or four sessions (#2323
* measured sustained load averages of 50–70) every test budget is the wrong
* number. Run back to back, the same two gates finish green in ~2x257s, and
* each is measured against the quiet baseline every other #2338 aspect is
* tuned to. So a second gate waits.
*
* **Why a lease and not a load threshold.** Two sessions each polling "is the
* machine quiet yet?" deadlock on each other — neither ever clears, since each
* is the reason the other is waiting (#2323). A lease *grants*: exactly one
* waiter wins the `mkdir`, runs, and releases; the rest keep asking. There is
* nothing to wait *for* except a release, and a release always comes — see the
* crash case below.
*
* **Why `proper-lockfile`.** `core/auth/node/file-lock.ts` (#2082) records why
* a hand-rolled election loses to it: `mkdir` is atomic, a live holder
* refreshes the lock's mtime at `stale / 2` for as long as it lives, and a
* holder that dies without releasing — a killed terminal, an OOM'd session —
* stops refreshing, goes stale, and is taken over by the next waiter. That is
* the whole "how does a crashed holder release" answer, and it is the
* library's, not ours. The file-lock module's caveats about stale takeover
* being non-single-winner apply here too and matter less: the worst case is
* two gates running at once, which is exactly today's behaviour.
*
* **Why the lease is machine-wide.** The lock lives under `os.tmpdir()`
* (`$XDG_RUNTIME_DIR` where a desktop session sets it), never inside a
* worktree — a lock in the repo would be one per worktree, which is one per
* session, which coordinates nothing.
*
* **Why default-on.** The sessions that need it are the ones that did not
* think to opt in. `INSPECTOR_SKIP_GATE_LEASE=1` bypasses it, and the first
* "waiting" line names the holder, its worktree, and that variable, so a wait
* is never a mystery. It is also never a gate: a lock that cannot be *created*
* (an unwritable tmpdir) runs the command unleased with a warning, because a
* coordination aid must not acquire a new way to fail the gate it wraps.
*
* **Why the whole gate and not its parallel stages.** The serial stages are
* not free — `lint` is type-aware and `build` is a full Vite bundle — and a
* per-stage lease would let two gates interleave into the same 2.5x. One
* lease around the whole run is the smallest thing that keeps the measured
* baseline meaningful.
*
* Usage: `node scripts/gate-lease.mjs <command> [args...]`. The command runs
* with inherited stdio in its own process group, so a signal to this process
* stops the whole tree — `npm run` nests four deep and `sh` forwards nothing —
* and the lease is released before this process exits with `128 + signal`.
*/
import { spawn } from "node:child_process";
import { mkdirSync, readFileSync, rmSync, writeFileSync } from "node:fs";
import { constants as osConstants, tmpdir } from "node:os";
import path from "node:path";
import { setTimeout as delay } from "node:timers/promises";
import { fileURLToPath } from "node:url";
// CJS-only package; a default import is the shape every loader agrees on.
import properLockfile from "proper-lockfile";
/** Set (to anything but `0` or empty) to run without taking the lease. */
export const SKIP_ENV = "INSPECTOR_SKIP_GATE_LEASE";
/** Overrides where the lease lives. Mostly a test seam; also a shared-box escape. */
export const DIR_ENV = "INSPECTOR_GATE_LEASE_DIR";
/**
* How long a lease may go unrefreshed before a waiter may take it over.
*
* `proper-lockfile` refreshes the lock's mtime at `stale / 2`, so this is not
* "how long a gate may run" — it is how long after a holder *dies* the queue
* stays blocked. 30s rather than the library's 10s default, on purpose: the
* refresh is a timer in an otherwise idle process, and the cost of a timer
* firing late (a laptop waking from sleep, an event loop starved by the very
* load this exists to manage) is a false takeover — two gates running at once,
* the thing being prevented — whereas the cost of the longer window is 20
* more seconds of waiting after a crash, against a gate that takes minutes.
*/
export const STALE_MS = 30_000;
/** How often a waiter re-asks. The acquire is one `mkdir`; this is not a hot loop. */
export const POLL_MS = 2_000;
/** How often a waiter says it is still waiting, so a long queue is visibly alive. */
export const PROGRESS_MS = 60_000;
/**
* How long a waiter waits before giving up.
*
* A dead holder releases within {@link STALE_MS}, so this only ever expires
* against a *live* holder that is still running after 45 minutes — four
* queued quiet gates are ~18 minutes, so a holder that long is a hung gate, and
* the right outcome is a loud failure naming it rather than a fifth process
* joining the pile. An unbounded wait would be a task that looks like progress
* and can never succeed.
*/
export const MAX_WAIT_MS = 45 * 60_000;
/** Where the lease lives. `$XDG_RUNTIME_DIR` is per-user and per-session where it exists. */
export function leaseDir(env = process.env) {
if (env[DIR_ENV]) return path.resolve(env[DIR_ENV]);
return path.join(env.XDG_RUNTIME_DIR || tmpdir(), "mcp-inspector-gate-lease");
}
/**
* The lease target: `proper-lockfile` locks `<target>.lock` beside it and
* never opens the target itself, so the target doubles as the holder record
* (pid, worktree, start time) a waiter prints. It need not exist to be locked
* (`realpath: false` below).
*/
export function leaseTarget(dir) {
return path.join(dir, "local-gate");
}
/** The lock directory `proper-lockfile` creates for {@link leaseTarget}. */
export function lockPathOf(dir) {
return `${leaseTarget(dir)}.lock`;
}
/** `INSPECTOR_SKIP_GATE_LEASE=0` and an empty value both mean "not skipped". */
export function isSkipped(env = process.env) {
const value = env[SKIP_ENV];
return value !== undefined && value !== "" && value !== "0";
}
/** `95s` → `1m35s`; sub-minute values stay in seconds. */
export function formatDuration(ms) {
const s = Math.max(0, Math.round(ms / 1000));
if (s < 60) return `${s}s`;
return `${Math.floor(s / 60)}m${String(s % 60).padStart(2, "0")}s`;
}
/**
* The exit code to report for a finished child: its own, or `128 + signal`
* (the shell convention) when a signal ended it. `code` and `signal` are
* mutually exclusive on a real ChildProcess exit.
*/
export function exitCodeFor({ code, signal }) {
if (typeof code === "number") return code;
return 128 + (osConstants.signals[signal] ?? 0);
}
/** The holder record the current holder wrote, or `null` if unreadable or malformed. */
export function readHolder(dir) {
try {
const record = JSON.parse(readFileSync(leaseTarget(dir), "utf8"));
if (
typeof record?.pid === "number" &&
typeof record?.cwd === "string" &&
typeof record?.startedAt === "number"
) {
return record;
}
} catch {
// Missing, unreadable, or not JSON — all mean "nothing to report".
}
return null;
}
/** One line a waiter can act on: who holds it, where, and for how long. */
export function describeHolder(holder, now = Date.now()) {
if (holder === null) return "another local:gate";
return `pid ${holder.pid} in ${holder.cwd}, running for ${formatDuration(now - holder.startedAt)}`;
}
/**
* Send `signal` to the child's whole process group (it was spawned as a group
* leader), falling back to the child alone where groups are unavailable or the
* group is already gone.
*/
function signalTree(child, signal) {
try {
if (process.platform !== "win32") {
process.kill(-child.pid, signal);
return;
}
} catch {
// ESRCH: the group is already gone, or the child never became a leader.
}
try {
child.kill(signal);
} catch {
// Already exited; nothing to signal.
}
}
/**
* Take the lease, waiting for a holder to release or go stale.
*
* Returns the release function, or `null` when a lock cannot be created here
* at all — the caller then runs unleased. Throws only when the wait budget is
* exhausted against a live holder.
*/
async function acquireLease({ dir, log, pollMs, progressMs, maxWaitMs }) {
const target = leaseTarget(dir);
const startedWaiting = Date.now();
let lastProgress = startedWaiting;
let announced = false;
for (;;) {
try {
return await properLockfile.lock(target, {
realpath: false,
stale: STALE_MS,
retries: 0,
// The library's default throws from a timer with no caller on the
// stack, which would take the *holder* down mid-gate. A compromised
// lease means another gate is now running alongside this one — worth
// saying, not worth killing a gate that is otherwise fine.
onCompromised: (err) =>
log(
`gate-lease: another process took the lease over while this gate was running (${err.message}); continuing without it.`,
),
});
} catch (err) {
if (err?.code !== "ELOCKED") {
log(
`gate-lease: could not take the lease at ${lockPathOf(dir)} (${err?.message ?? err}); running without it.`,
);
return null;
}
}
const waited = Date.now() - startedWaiting;
if (!announced) {
announced = true;
log(
`gate-lease: ${describeHolder(readHolder(dir))} holds the gate lease; waiting for it to finish so the two do not contend. ${SKIP_ENV}=1 runs anyway.`,
);
} else if (Date.now() - lastProgress >= progressMs) {
lastProgress = Date.now();
log(
`gate-lease: still waiting (${formatDuration(waited)}) on ${describeHolder(readHolder(dir))}.`,
);
}
if (waited >= maxWaitMs) {
throw new Error(
`gate-lease: gave up after ${formatDuration(waited)} — ${describeHolder(readHolder(dir))} still holds ${lockPathOf(dir)}. If that gate is hung, stop it; ${SKIP_ENV}=1 runs without the lease.`,
);
}
await delay(pollMs);
}
}
/**
* Run `command args` holding the lease, and resolve with the exit code to
* report. Signals to this process stop the child's whole tree first and are
* reported as `128 + signal` after the lease is released.
*
* Every knob is injectable so the tests can drive a real lock, a real child
* and a real signal without waiting real minutes.
*
* @param {object} opts
* @param {string} opts.command
* @param {string[]} opts.args
* @param {string} [opts.dir] lease directory; default {@link leaseDir}
* @param {NodeJS.ProcessEnv} [opts.env]
* @param {(line: string) => void} [opts.log]
* @param {number} [opts.pollMs]
* @param {number} [opts.progressMs]
* @param {number} [opts.maxWaitMs]
* @param {number} [opts.graceMs] SIGTERM → SIGKILL escalation on a signal
* @param {import("node:child_process").StdioOptions} [opts.stdio]
* @returns {Promise<number>}
*/
export async function runUnderLease({
command,
args,
dir = leaseDir(),
env = process.env,
log = (line) => console.error(line),
pollMs = POLL_MS,
progressMs = PROGRESS_MS,
maxWaitMs = MAX_WAIT_MS,
graceMs = 5_000,
stdio = "inherit",
}) {
let release = null;
let waited = 0;
if (isSkipped(env)) {
log(`gate-lease: ${SKIP_ENV} is set; running without the lease.`);
} else {
let usable = true;
try {
mkdirSync(dir, { recursive: true });
} catch (err) {
usable = false;
log(
`gate-lease: could not create ${dir} (${err?.message ?? err}); running without the lease.`,
);
}
if (usable) {
const startedWaiting = Date.now();
release = await acquireLease({ dir, log, pollMs, progressMs, maxWaitMs });
waited = Date.now() - startedWaiting;
}
if (release !== null) {
if (waited >= pollMs) {
log(`gate-lease: acquired after ${formatDuration(waited)}.`);
}
try {
writeFileSync(
leaseTarget(dir),
JSON.stringify({
pid: process.pid,
cwd: process.cwd(),
startedAt: Date.now(),
}),
);
} catch {
// Best effort: the record only improves a waiter's message.
}
}
}
const startedRunning = Date.now();
const child = spawn(command, args, {
stdio,
env,
// Its own group, so one signal reaches every descendant. See the header.
detached: process.platform !== "win32",
});
const signals = ["SIGINT", "SIGTERM", "SIGHUP"];
let stoppedBy = null;
let escalation = null;
const onSignal = (signal) => {
if (stoppedBy !== null) return;
stoppedBy = signal;
log(`gate-lease: received ${signal}; stopping the gate.`);
signalTree(child, signal);
escalation = setTimeout(() => {
log(
`gate-lease: the gate did not exit within ${graceMs}ms of ${signal}; sending SIGKILL.`,
);
signalTree(child, "SIGKILL");
}, graceMs);
};
const handlers = signals.map((signal) => {
const handler = () => onSignal(signal);
process.on(signal, handler);
return [signal, handler];
});
const outcome = await new Promise((resolve, reject) => {
child.once("error", reject);
child.once("exit", (code, signal) => resolve({ code, signal }));
}).finally(() => {
clearTimeout(escalation);
for (const [signal, handler] of handlers) process.off(signal, handler);
});
if (release !== null) {
// The record is only meaningful while the lock beside it is held; a
// waiter reading a fresh lock must not be told about the previous holder.
rmSync(leaseTarget(dir), { force: true });
try {
await release();
} catch (err) {
log(
`gate-lease: could not release the lease (${err?.message ?? err}); it goes stale on its own after ${formatDuration(STALE_MS)}.`,
);
}
log(
`gate-lease: released after ${formatDuration(Date.now() - startedRunning)}${waited >= pollMs ? ` (waited ${formatDuration(waited)} first)` : ""}.`,
);
}
return stoppedBy !== null
? exitCodeFor({ code: null, signal: stoppedBy })
: exitCodeFor(outcome);
}
/** `node scripts/gate-lease.mjs <command> [args...]` */
export async function main(argv = process.argv.slice(2)) {
const [command, ...args] = argv;
if (!command) {
console.error("usage: node scripts/gate-lease.mjs <command> [args...]");
return 2;
}
try {
return await runUnderLease({ command, args });
} catch (err) {
console.error(err?.message ?? err);
return 1;
}
}
if (
process.argv[1] &&
path.resolve(process.argv[1]) === fileURLToPath(import.meta.url)
) {
process.exitCode = await main();
}

327
scripts/gate-lease.test.mjs Normal file
View file

@ -0,0 +1,327 @@
// Tests for the local:gate lease (#2339). The pure helpers are table-driven;
// the lease itself is exercised against a REAL `proper-lockfile` lock, a real
// child process and — for the signal path — a real signal to a real wrapper
// process, each against a throwaway lease directory so no test can queue
// behind (or block) a developer's actual gate. Run via `npm run test:scripts`
// (node:test; the root has no vitest harness).
import { test } from "node:test";
import assert from "node:assert/strict";
import { spawn } from "node:child_process";
import {
existsSync,
mkdirSync,
mkdtempSync,
readFileSync,
utimesSync,
writeFileSync,
} from "node:fs";
import { tmpdir } from "node:os";
import { join } from "node:path";
import { setTimeout as delay } from "node:timers/promises";
import { fileURLToPath } from "node:url";
import {
DIR_ENV,
SKIP_ENV,
STALE_MS,
describeHolder,
exitCodeFor,
formatDuration,
isSkipped,
leaseDir,
leaseTarget,
lockPathOf,
main,
readHolder,
runUnderLease,
} from "./gate-lease.mjs";
const SCRIPT = fileURLToPath(new URL("./gate-lease.mjs", import.meta.url));
/** A fresh lease directory per test, so tests never contend with each other. */
function freshDir() {
return mkdtempSync(join(tmpdir(), "gate-lease-test-"));
}
function collectLog() {
const lines = [];
return { lines, log: (line) => lines.push(line) };
}
/** Run `node -e <source>` under the lease with quiet stdio and fast knobs. */
function runNode(source, opts = {}) {
return runUnderLease({
command: process.execPath,
args: ["-e", source],
stdio: "ignore",
pollMs: 25,
progressMs: 60_000,
...opts,
});
}
/** Poll until `predicate()` holds or `timeoutMs` passes. */
async function waitFor(predicate, timeoutMs = 5_000) {
const deadline = Date.now() + timeoutMs;
while (!predicate()) {
assert.ok(Date.now() < deadline, "condition not met in time");
await delay(25);
}
}
function isAlive(pid) {
try {
process.kill(pid, 0);
return true;
} catch {
return false;
}
}
test("leaseDir: explicit override, then XDG_RUNTIME_DIR, then tmpdir", () => {
assert.equal(leaseDir({ [DIR_ENV]: "/x/y" }), "/x/y");
assert.equal(
leaseDir({ XDG_RUNTIME_DIR: "/run/user/1" }),
join("/run/user/1", "mcp-inspector-gate-lease"),
);
assert.equal(leaseDir({}), join(tmpdir(), "mcp-inspector-gate-lease"));
// The lock is a sibling of the target, never inside it — a file inside the
// lock directory would make its removal (and so stale takeover) fail.
assert.equal(lockPathOf("/d"), `${leaseTarget("/d")}.lock`);
});
test("isSkipped: unset, empty and `0` mean the lease is on", () => {
for (const [env, expected] of [
[{}, false],
[{ [SKIP_ENV]: "" }, false],
[{ [SKIP_ENV]: "0" }, false],
[{ [SKIP_ENV]: "1" }, true],
[{ [SKIP_ENV]: "true" }, true],
]) {
assert.equal(isSkipped(env), expected, JSON.stringify(env));
}
});
test("formatDuration: seconds under a minute, m/ss above", () => {
for (const [ms, expected] of [
[0, "0s"],
[-500, "0s"],
[4_400, "4s"],
[59_499, "59s"],
[60_000, "1m00s"],
[95_000, "1m35s"],
[3_605_000, "60m05s"],
]) {
assert.equal(formatDuration(ms), expected, String(ms));
}
});
test("exitCodeFor: the child's code, else 128 + signal", () => {
assert.equal(exitCodeFor({ code: 0, signal: null }), 0);
assert.equal(exitCodeFor({ code: 7, signal: null }), 7);
assert.equal(exitCodeFor({ code: null, signal: "SIGTERM" }), 143);
assert.equal(exitCodeFor({ code: null, signal: "SIGINT" }), 130);
assert.equal(exitCodeFor({ code: null, signal: "SIGNOPE" }), 128);
});
test("readHolder/describeHolder: a missing or malformed record is not an error", () => {
const dir = freshDir();
assert.equal(readHolder(dir), null);
assert.equal(describeHolder(null), "another local:gate");
writeFileSync(leaseTarget(dir), "not json");
assert.equal(readHolder(dir), null);
writeFileSync(leaseTarget(dir), JSON.stringify({ pid: "12", cwd: "/w" }));
assert.equal(readHolder(dir), null);
const record = { pid: 12, cwd: "/w", startedAt: 1_000 };
writeFileSync(leaseTarget(dir), JSON.stringify(record));
assert.deepEqual(readHolder(dir), record);
assert.equal(
describeHolder(record, 96_000),
"pid 12 in /w, running for 1m35s",
);
});
test("two concurrent runs serialize: the second starts after the first ends", async () => {
const dir = freshDir();
const marks = join(dir, "marks.log");
const { lines, log } = collectLog();
const body = (label) =>
`const fs=require("fs");const f=${JSON.stringify(marks)};` +
`fs.appendFileSync(f,"${label} start "+Date.now()+"\\n");` +
`setTimeout(()=>fs.appendFileSync(f,"${label} end "+Date.now()+"\\n"),300)`;
const first = runNode(body("a"), { dir, log });
// Give the first run the lock — and its holder record, which is written
// just after — before the second asks, so which one waits, and what it is
// told, are not themselves races the assertions have to allow for.
await waitFor(() => existsSync(leaseTarget(dir)));
const second = runNode(body("b"), { dir, log });
assert.deepEqual(await Promise.all([first, second]), [0, 0]);
const marksByLabel = Object.fromEntries(
readFileSync(marks, "utf8")
.trim()
.split("\n")
.map((line) => line.split(" "))
.map(([label, phase, at]) => [`${label} ${phase}`, Number(at)]),
);
assert.ok(
marksByLabel["b start"] >= marksByLabel["a end"],
`b started (${marksByLabel["b start"]}) before a ended (${marksByLabel["a end"]})`,
);
// The waiter said who it was waiting on, named the bypass, and reported the
// wait when it finally acquired; the uncontended run said nothing on entry.
assert.equal(
lines.filter((l) => l.includes("holds the gate lease")).length,
1,
);
assert.match(
lines.find((l) => l.includes("holds the gate lease")),
/pid \d+ in .+, running for/,
);
assert.match(
lines.find((l) => l.includes("holds the gate lease")),
new RegExp(`${SKIP_ENV}=1`),
);
assert.equal(
lines.filter((l) => l.startsWith("gate-lease: acquired after")).length,
1,
);
assert.equal(
lines.filter((l) => l.startsWith("gate-lease: released after")).length,
2,
);
assert.ok(!existsSync(lockPathOf(dir)), "the lock is released at the end");
assert.ok(!existsSync(leaseTarget(dir)), "the holder record goes with it");
});
test("the child's exit code is the run's exit code", async () => {
const dir = freshDir();
assert.equal(await runNode("process.exit(7)", { dir, log: () => {} }), 7);
});
test("a stale lock (its holder died) is taken over, not waited on", async () => {
const dir = freshDir();
mkdirSync(lockPathOf(dir), { recursive: true });
const dead = new Date(Date.now() - STALE_MS - 1_000);
utimesSync(lockPathOf(dir), dead, dead);
const { lines, log } = collectLog();
assert.equal(await runNode("", { dir, log }), 0);
assert.equal(
lines.filter((l) => l.includes("holds the gate lease")).length,
0,
);
assert.ok(!existsSync(lockPathOf(dir)));
});
test("a live lock that never releases fails the wait loudly, naming the holder", async () => {
const dir = freshDir();
// A fresh lock directory nobody refreshes is indistinguishable from a live
// holder for STALE_MS, which is longer than this test is prepared to wait.
mkdirSync(lockPathOf(dir), { recursive: true });
writeFileSync(
leaseTarget(dir),
JSON.stringify({
pid: 424242,
cwd: "/some/worktree",
startedAt: Date.now(),
}),
);
const { lines, log } = collectLog();
await assert.rejects(runNode("", { dir, log, maxWaitMs: 0 }), (err) => {
assert.match(err.message, /gave up after/);
assert.match(err.message, /pid 424242 in \/some\/worktree/);
assert.match(err.message, new RegExp(`${SKIP_ENV}=1`));
return true;
});
assert.equal(
lines.filter((l) => l.includes("holds the gate lease")).length,
1,
);
// The lock was somebody else's; giving up must not remove it.
assert.ok(existsSync(lockPathOf(dir)));
});
test("a waiter reports progress while it waits", async () => {
const dir = freshDir();
mkdirSync(lockPathOf(dir), { recursive: true });
const { lines, log } = collectLog();
await assert.rejects(
runNode("", { dir, log, maxWaitMs: 120, pollMs: 30, progressMs: 0 }),
/gave up/,
);
assert.ok(
lines.some((l) => l.startsWith("gate-lease: still waiting")),
lines.join("\n"),
);
});
test(`${SKIP_ENV}=1 runs without touching the lease`, async () => {
const dir = freshDir();
const { lines, log } = collectLog();
const env = { ...process.env, [SKIP_ENV]: "1" };
assert.equal(await runNode("", { dir, log, env }), 0);
assert.match(lines.join("\n"), new RegExp(`${SKIP_ENV} is set`));
assert.ok(!existsSync(lockPathOf(dir)));
});
test("a lease directory that cannot exist degrades to an unleased run", async () => {
const parent = freshDir();
const file = join(parent, "a-file");
writeFileSync(file, "");
const { lines, log } = collectLog();
assert.equal(await runNode("", { dir: join(file, "nested"), log }), 0);
assert.match(
lines.join("\n"),
/could not create .*; running without the lease/,
);
});
test("main: no command is a usage error", async () => {
const { error } = console;
const lines = [];
console.error = (line) => lines.push(line);
try {
assert.equal(await main([]), 2);
} finally {
console.error = error;
}
assert.match(lines.join("\n"), /usage:/);
});
test("a signal to the wrapper stops the whole tree, releases the lease and exits 128+signal", async () => {
const dir = freshDir();
const pidFile = join(dir, "grandchild.pid");
// The gate stand-in: records its pid, then idles forever unless signalled.
const idle =
`require("fs").writeFileSync(${JSON.stringify(pidFile)}, String(process.pid));` +
`setInterval(() => {}, 1000)`;
const wrapper = spawn(
process.execPath,
[SCRIPT, process.execPath, "-e", idle],
{
env: { ...process.env, [DIR_ENV]: dir },
stdio: ["ignore", "ignore", "pipe"],
},
);
let stderr = "";
wrapper.stderr.on("data", (chunk) => (stderr += chunk));
const exited = new Promise((resolve) =>
wrapper.once("exit", (code, signal) => resolve({ code, signal })),
);
await waitFor(() => existsSync(pidFile));
const grandchild = Number(readFileSync(pidFile, "utf8"));
assert.ok(isAlive(grandchild));
assert.ok(existsSync(lockPathOf(dir)), "the wrapper held the lease");
wrapper.kill("SIGTERM");
const outcome = await exited;
assert.deepEqual(outcome, { code: 143, signal: null }, stderr);
await waitFor(() => !isAlive(grandchild));
assert.ok(
!existsSync(lockPathOf(dir)),
"the lease was released on the way out",
);
assert.match(stderr, /received SIGTERM; stopping the gate/);
});

20
scripts/lib/npm-scripts.mjs
View file

@ -50,6 +50,16 @@ export function reachableScripts(scripts, entry = "validate") {
return reached; return reached;
} }
/** The lease wrapper `local:gate` runs under (#2339); see `scripts/gate-lease.mjs`. */
export const GATE_LEASE_WRAPPER = "node scripts/gate-lease.mjs ";
/** `node scripts/gate-lease.mjs npm run X` → `npm run X`; anything else unchanged. */
function unwrapGateLease(segment) {
return segment.startsWith(GATE_LEASE_WRAPPER)
? segment.slice(GATE_LEASE_WRAPPER.length).trim()
: segment;
}
/** The command strings of every script reachable from the root `validate`. */ /** The command strings of every script reachable from the root `validate`. */
function rootReachedCommands(rootScripts) { function rootReachedCommands(rootScripts) {
return [...reachableScripts(rootScripts)] return [...reachableScripts(rootScripts)]
@ -108,6 +118,12 @@ export function rootReachesScript(rootScripts, scriptName) {
* flags, as are npm's implicit `pre`/`post` hooks. Only the target is matched * flags, as are npm's implicit `pre`/`post` hooks. Only the target is matched
* exactly, which suits the argument-less scripts a vouch asks about. * exactly, which suits the argument-less scripts a vouch asks about.
* *
* One wrapper is transparent: `node scripts/gate-lease.mjs npm run X` runs
* `npm run X` under the local:gate lease (#2339) and nothing else, so a vouch
* about what `local:gate` runs looks through it. It is matched by its exact
* leading tokens — a wrapper with flags, or any other wrapper, still hides the
* invocation, because this helper cannot know what an arbitrary wrapper does.
*
* @param {Record<string, string>} scripts * @param {Record<string, string>} scripts
* @param {string} entry * @param {string} entry
* @param {string} target * @param {string} target
@ -124,7 +140,9 @@ export function scriptChainRuns(scripts, entry, target) {
if (typeof scripts?.[hook] === "string") queue.push(hook); if (typeof scripts?.[hook] === "string") queue.push(hook);
const body = scripts?.[name]; const body = scripts?.[name];
if (typeof body !== "string") continue; if (typeof body !== "string") continue;
for (const segment of body.split(/\n|;|&&/).map((part) => part.trim())) { for (const segment of body
.split(/\n|;|&&/)
.map((part) => unwrapGateLease(part.trim()))) {
if (segment === `npm run ${target}`) return true; if (segment === `npm run ${target}`) return true;
const tokens = segment.split(/\s+/); const tokens = segment.split(/\s+/);
if (tokens[0] === "npm" && tokens[1] === "run" && tokens[2]) { if (tokens[0] === "npm" && tokens[1] === "run" && tokens[2]) {

30
scripts/lib/workflow-gate.test.mjs
View file

@ -23,7 +23,11 @@ import {
findWorkflowViolations, findWorkflowViolations,
formatWorkflowViolations, formatWorkflowViolations,
} from "./workflow-gate.mjs"; } from "./workflow-gate.mjs";
import { reachableScripts } from "./npm-scripts.mjs"; import {
GATE_LEASE_WRAPPER,
reachableScripts,
scriptChainRuns,
} from "./npm-scripts.mjs";
const repoRoot = join(import.meta.dirname, "..", ".."); const repoRoot = join(import.meta.dirname, "..", "..");
const workflowDir = join(repoRoot, ".github", "workflows"); const workflowDir = join(repoRoot, ".github", "workflows");
@ -607,7 +611,22 @@ describe("the gate's name", () => {
it("is `local:gate`, and runs the local-only Storybook step", () => { it("is `local:gate`, and runs the local-only Storybook step", () => {
assert.ok(scripts["local:gate"], "the pre-push gate must exist"); assert.ok(scripts["local:gate"], "the pre-push gate must exist");
assert.match(scripts["local:gate"], /local:storybook/); assert.match(scripts["local:gate:stages"], /local:storybook/);
});
it("runs its stages under the lease wrapper, and nothing else (#2339)", () => {
// The wrapper serializes gates across worktrees. It must be the ONLY thing
// `local:gate` does — a stage placed beside it would run outside the
// lease — and it must run exactly the stages script, so every assertion
// below about what the gate runs can read `local:gate:stages` directly.
assert.equal(
scripts["local:gate"],
`${GATE_LEASE_WRAPPER}npm run local:gate:stages`,
);
assert.ok(
scriptChainRuns(scripts, "local:gate", "local:storybook"),
"the wrapper is transparent to a chain vouch",
);
}); });
describe("runs each client's test suite once, not twice (#2341)", () => { describe("runs each client's test suite once, not twice (#2341)", () => {
@ -631,8 +650,11 @@ describe("the gate's name", () => {
); );
it("starts from `local:validate`, and never from `validate`", () => { it("starts from `local:validate`, and never from `validate`", () => {
assert.match(scripts["local:gate"], /^npm run local:validate && /); assert.match(scripts["local:gate:stages"], /^npm run local:validate && /);
assert.doesNotMatch(scripts["local:gate"], /\brun validate(?=$|[\s&;])/); assert.doesNotMatch(
scripts["local:gate:stages"],
/\brun validate(?=$|[\s&;])/,
);
// Nothing reachable from the WHOLE gate — not just its first stage — // Nothing reachable from the WHOLE gate — not just its first stage —
// may run `validate` or a client's `validate:*`: those are the doors // may run `validate` or a client's `validate:*`: those are the doors
// through which the bare `test` leg would come back, and a later stage // through which the bare `test` leg would come back, and a later stage

4
scripts/run-engine-smokes.test.mjs
View file

@ -17,6 +17,7 @@ import { join } from "node:path";
import { describe, it } from "node:test"; import { describe, it } from "node:test";
import { ENGINE_SMOKES } from "./run-engine-smokes.mjs"; import { ENGINE_SMOKES } from "./run-engine-smokes.mjs";
import { SUPPORTED_BROWSERS } from "./lib/headless-browser.mjs"; import { SUPPORTED_BROWSERS } from "./lib/headless-browser.mjs";
import { scriptChainRuns } from "./lib/npm-scripts.mjs";
const scriptDir = import.meta.dirname; const scriptDir = import.meta.dirname;
const scripts = JSON.parse( const scripts = JSON.parse(
@ -73,7 +74,8 @@ describe("every engine tier consumes ENGINE_SMOKES", () => {
it("the Firefox tier goes through the runner and is in the pre-push gate", () => { it("the Firefox tier goes through the runner and is in the pre-push gate", () => {
assert.match(scripts["smoke:web:firefox"], /run-engine-smokes\.mjs/); assert.match(scripts["smoke:web:firefox"], /run-engine-smokes\.mjs/);
assert.match(scripts["local:gate"], /smoke:web:firefox/); // Through the #2339 lease wrapper, which `scriptChainRuns` looks past.
assert.ok(scriptChainRuns(scripts, "local:gate", "smoke:web:firefox"));
}); });
it("every supported engine has a `smoke:web:<engine>` script", () => { it("every supported engine has a `smoke:web:<engine>` script", () => {

39
scripts/verify-skills.main.test.mjs
View file

@ -30,7 +30,7 @@ import {
ciRunsUnconditionally, ciRunsUnconditionally,
runsCommand, runsCommand,
} from "./verify-skills.mjs"; } from "./verify-skills.mjs";
import { scriptChainRuns } from "./lib/npm-scripts.mjs"; import { GATE_LEASE_WRAPPER, scriptChainRuns } from "./lib/npm-scripts.mjs";
const SCRIPT = path.join( const SCRIPT = path.join(
path.dirname(fileURLToPath(import.meta.url)), path.dirname(fileURLToPath(import.meta.url)),
@ -491,6 +491,43 @@ test("scriptChainRuns follows the chain to a real invocation", () => {
); );
}); });
test("scriptChainRuns looks through the local:gate lease wrapper, and only it", () => {
// `local:gate` runs its stages under `scripts/gate-lease.mjs` (#2339). The
// wrapper runs exactly the `npm run …` it is handed, so a vouch sees past
// it; a wrapper carrying flags, or any other wrapper, still hides the chain.
const T = "verify:skills:cli";
const stages = `npm run validate && npm run ${T}`;
assert.equal(
scriptChainRuns(
{
"local:gate": `${GATE_LEASE_WRAPPER}npm run local:gate:stages`,
"local:gate:stages": stages,
},
"local:gate",
T,
),
true,
);
assert.equal(
scriptChainRuns(
{ "local:gate": `${GATE_LEASE_WRAPPER}npm run ${T}` },
"local:gate",
T,
),
true,
);
for (const wrapped of [
`node scripts/gate-lease.mjs --quiet npm run ${T}`,
`node scripts/other-wrapper.mjs npm run ${T}`,
]) {
assert.equal(
scriptChainRuns({ "local:gate": wrapped }, "local:gate", T),
false,
wrapped,
);
}
});
test("scriptChainRuns is not satisfied by a mention or a longer name", () => { test("scriptChainRuns is not satisfied by a mention or a longer name", () => {
// `reachableScripts` extracts any `npm run …` substring, so both of these // `reachableScripts` extracts any `npm run …` substring, so both of these
// passed the old check while the validator never ran. // passed the old check while the validator never ran.