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

chore(scripts): rename the local pre-push gate off ci, and guard it out of CI (#2146)

Browse source 
The root `ci` script was never what runs in GitHub CI, and the name worked
against us three ways: it collides with the built-in `npm ci` (which does
something else entirely and is one keystroke away), it invites the
cross-engine passes into a workflow by looking like the thing a workflow
should call, and it blurs "GitHub CI" and "the local gate" in every doc
that names it.

- Rename `ci` → `local:gate` and `ci:storybook` → `local:storybook`, with
  no back-compat alias: `npm run ci` now fails with npm's missing-script
  error, which prints the available scripts.

- Add `scripts/lib/workflow-gate.mjs` + its `test:scripts` suite, which
  runs over the repo's real `.github/workflows/**` and fails on an
  invocation of a `local:*` script, of a non-Chromium engine pass (derived
  from SUPPORTED_BROWSERS), or of `smoke:web:engine`, or on `SMOKE_BROWSER`
  set to anything but a literal `chromium`. It does not forbid
  `npm run smoke`, `smoke:web:chromium` or `smoke:tui` — those belong in CI
  and are there today. Comments are not scanned, so the workflow can still
  explain itself. This is the durable half; the rest is prose.

- Document the two-tier model once, in the root README ("Two tiers: GitHub
  CI and the local gate"), and point the other docs at it rather than
  restating the split.

- Sweep the ~40 stale references across README, AGENTS.md,
  .github/copilot-instructions.md, the client READMEs, the roadmap doc and
  the script headers. AGENTS.md and copilot-instructions.md change
  together, per the mirror rule.

No gate changes what it actually runs.

Signed-off-by: Cliff Hall <cliff@futurescale.com>
Signed-off-by: cliffhall <cliff@futurescale.com>
This commit is contained in:
cliffhall 2026-08-26 23:56:34 -04:00
commit 0b64460097
4 changed files with 423 additions and 48 deletions
Download patch file Download diff file Expand all files Collapse all files

2
scripts/lib/announced-child.mjs
View file

@ -3,7 +3,7 @@
*
* Extracted from `scripts/smoke-web-app.mjs` so the failure path can be tested
* (#2000). A smoke script only ever exercises its own happy path: the real MCP
* test server always announces, so nothing in `npm run ci` proved that a child
* test server always announces, so nothing in `npm run local:gate` proved that a child
* which stays alive *through* the readiness timeout is still reachable by the
* caller's teardown. That is precisely the case that used to orphan a live
* server holding its port, and it is invisible to the smoke itself.

2
scripts/lib/announced-child.test.mjs
View file

@ -4,7 +4,7 @@
* The invariant under test is the one a smoke script can never check itself:
* a child that stays alive *through* the readiness timeout must still be
* reachable by the caller's teardown. `smoke:web:app`'s happy path always gets
* the announcement, so a regression here would be silent in `npm run ci` and
* the announcement, so a regression here would be silent in `npm run local:gate` and
* would surface only as a stray process holding a port on a later run.
*
* Every case drives a real `node -e` child rather than a spy, so the assertion

166
scripts/lib/workflow-gate.mjs Normal file
View file

@ -0,0 +1,166 @@
/**
* The guard that keeps the LOCAL pre-push gate out of GitHub CI (#2146).
*
* Two tiers run the same checks and are deliberately not the same set:
*
* - **GitHub CI** (`.github/workflows/**`) runs `validate`, `coverage`, the
* two verify gates, `smoke` and the Storybook tests. `npm run smoke` covers
* `smoke:launcher`, `smoke:cli`, `smoke:tui`, `smoke:web` and
* `smoke:web:chromium` — all of which BELONG there.
* - **`npm run local:gate`**, the pre-push gate, is a strict superset: it adds
* the non-Chromium engine passes, and `smoke:tui` really runs there rather
* than self-skipping.
*
* The narrow thing that must never reach CI is the **non-Chromium engine
* passes**. `smoke:web:firefox` was placed in the local gate on purpose (#2086,
* #2133) after a CI job was trialled and removed for never once disagreeing
* with Chromium, and `smoke:web:webkit` fails two of the three smokes outright
* for reasons nobody has identified. Confidence in the cross-engine runs is not
* high enough to let them break CI.
*
* Renaming the gate off `ci` removed the *invitation* to add it to a workflow.
* This removes the *possibility*: it is the only part of #2146 that cannot rot,
* since everything else there is prose.
*
* WHAT IT FORBIDS, and nothing more:
*
* 1. Invoking a `local:*` script. That namespace means local-only by
* construction, so the rule covers `local:storybook` and any future
* sibling without being edited.
* 2. Invoking a non-Chromium engine pass (`smoke:web:firefox`,
* `smoke:web:webkit`). Derived from `SUPPORTED_BROWSERS`, so adding an
* engine extends the guard rather than opening a hole in it.
* 3. Invoking `smoke:web:engine`, whose engine comes from the environment and
* therefore cannot be read off the workflow at all.
* 4. Setting `SMOKE_BROWSER` to anything but a literal `chromium` — the back
* door that would redirect an otherwise innocent `npm run smoke`. An
* expression (`${{ matrix.browser }}`) is flagged for the same reason as
* `smoke:web:engine`: the guard cannot tell what it resolves to.
*
* WHAT IT MUST NOT FORBID: `npm run smoke`, `smoke:web:chromium`, `smoke:tui`.
* Those belong in CI and are there today — `smoke:tui` self-skips under
* `process.env.CI` on its own, so it needs no guarding.
*
* COMMENTS ARE NOT SCANNED. A line whose first non-whitespace character is `#`
* is skipped, whether it is a YAML comment or a shell comment inside a `run:`
* block. A comment cannot invoke anything, and the workflow's value is largely
* in its prose — a guard that made the two tiers undocumentable in the file
* that implements one of them would be traded for the wrong thing.
*/
import { SUPPORTED_BROWSERS, DEFAULT_BROWSER } from "./headless-browser.mjs";
/** Scripts whose `local:` prefix declares them local-only. */
export const LOCAL_SCRIPT_PREFIX = "local:";
/**
* Engine passes that must never run in CI: every supported engine except the
* default, plus the env-driven runner whose engine is unknowable statically.
*/
export const FORBIDDEN_ENGINE_SCRIPTS = [
...SUPPORTED_BROWSERS.filter((b) => b !== DEFAULT_BROWSER).map(
(b) => `smoke:web:${b}`,
),
"smoke:web:engine",
];
/** Reasons a finding can carry, kept as constants so tests pin them. */
export const VIOLATION = {
LOCAL_SCRIPT: "local-only-script",
ENGINE_SCRIPT: "non-chromium-engine-pass",
BROWSER_ENV: "smoke-browser-override",
};
const LOCAL_SCRIPT_RE = /\blocal:[a-z0-9][a-z0-9:-]*/gi;
// The value may be quoted (`SMOKE_BROWSER: "firefox"`), assigned
// (`SMOKE_BROWSER=firefox`), or an expression — capture whatever follows.
const BROWSER_ENV_RE = /\bSMOKE_BROWSER\s*[:=]\s*(\S+)/gi;
function isComment(line) {
return line.trimStart().startsWith("#");
}
function stripQuotes(value) {
const trimmed = value.trim().replace(/[,;]+$/, "");
const quoted = /^(["'])(.*)\1$/.exec(trimmed);
return quoted ? quoted[2] : trimmed;
}
/**
* Scan one workflow file's text for invocations that must stay local-only.
*
* Pure: takes text, returns findings. `file` is carried through only so a
* caller can report where a finding came from.
*
* @param {string} text
* @param {string} [file]
* @returns {Array<{file: string, line: number, rule: string, match: string, message: string}>}
*/
export function findWorkflowViolations(text, file = "<workflow>") {
const findings = [];
const lines = text.split(/\r?\n/);
lines.forEach((line, index) => {
if (isComment(line)) return;
const lineNumber = index + 1;
for (const match of line.matchAll(LOCAL_SCRIPT_RE)) {
findings.push({
file,
line: lineNumber,
rule: VIOLATION.LOCAL_SCRIPT,
match: match[0],
message:
`\`${match[0]}\` is a local-only script — the \`${LOCAL_SCRIPT_PREFIX}\` prefix means ` +
`it runs in the pre-push gate, never in GitHub CI.`,
});
}
for (const script of FORBIDDEN_ENGINE_SCRIPTS) {
// Word-boundary on the right so `smoke:web:chromium` can never be read as
// a prefix of a forbidden name (and vice versa).
// `:` is not a regex metacharacter, so the name needs no escaping.
const re = new RegExp(`\\b${script}\\b`, "g");
for (const match of line.matchAll(re)) {
findings.push({
file,
line: lineNumber,
rule: VIOLATION.ENGINE_SCRIPT,
match: match[0],
message:
`\`${script}\` is a non-Chromium engine pass and belongs to the local gate only ` +
`(#2086). GitHub CI runs \`npm run smoke\`, which covers \`smoke:web:chromium\`.`,
});
}
}
for (const match of line.matchAll(BROWSER_ENV_RE)) {
const value = stripQuotes(match[1]);
if (value === DEFAULT_BROWSER) continue;
findings.push({
file,
line: lineNumber,
rule: VIOLATION.BROWSER_ENV,
match: match[0].trim(),
message:
`SMOKE_BROWSER must not be set to \`${value}\` in a workflow — only ` +
`\`${DEFAULT_BROWSER}\` runs in GitHub CI, and an expression cannot be ` +
`checked statically. Leave it unset; the default is \`${DEFAULT_BROWSER}\`.`,
});
}
});
return findings;
}
/**
* Render findings as a single message, for an assertion failure or a CLI.
*
* @param {ReturnType<typeof findWorkflowViolations>} findings
* @returns {string}
*/
export function formatWorkflowViolations(findings) {
return findings
.map((f) => ` ${f.file}:${f.line} [${f.rule}] ${f.message}`)
.join("\n");
}

178
scripts/lib/workflow-gate.test.mjs Normal file
View file

@ -0,0 +1,178 @@
/**
* The local-gate-out-of-CI guard (#2146), in two halves.
*
* The table-driven half pins one case per rule the parser encodes — including
* the negative cases, which are the ones that matter most here: a guard that
* also rejected `npm run smoke` or `smoke:web:chromium` would be "fixed" by
* deleting it, taking the real protection with it.
*
* The second half runs the parser over the repository's ACTUAL workflow files.
* That is the assertion the issue asks for: it fails the moment a workflow
* invokes the pre-push gate, a non-Chromium engine pass, or `smoke:web:engine`,
* or points `SMOKE_BROWSER` at something else.
*/
import assert from "node:assert/strict";
import { readdirSync, readFileSync } from "node:fs";
import { join } from "node:path";
import { describe, it } from "node:test";
import {
FORBIDDEN_ENGINE_SCRIPTS,
VIOLATION,
findWorkflowViolations,
formatWorkflowViolations,
} from "./workflow-gate.mjs";
const repoRoot = join(import.meta.dirname, "..", "..");
const workflowDir = join(repoRoot, ".github", "workflows");
describe("findWorkflowViolations", () => {
const cases = [
{
name: "flags the pre-push gate itself",
text: " - run: npm run local:gate\n",
rules: [VIOLATION.LOCAL_SCRIPT],
},
{
name: "flags any other local: script, without being edited for it",
text: " - run: npm run local:storybook\n",
rules: [VIOLATION.LOCAL_SCRIPT],
},
{
name: "flags the Firefox engine pass",
text: " - run: npm run smoke:web:firefox\n",
rules: [VIOLATION.ENGINE_SCRIPT],
},
{
name: "flags the WebKit engine pass",
text: " - run: npm run smoke:web:webkit\n",
rules: [VIOLATION.ENGINE_SCRIPT],
},
{
name: "flags the env-driven runner, whose engine cannot be read statically",
text: " - run: npm run smoke:web:engine\n",
rules: [VIOLATION.ENGINE_SCRIPT],
},
{
name: "flags SMOKE_BROWSER pointed at another engine",
text: " env:\n SMOKE_BROWSER: firefox\n",
rules: [VIOLATION.BROWSER_ENV],
},
{
name: "flags a quoted SMOKE_BROWSER value",
text: ' SMOKE_BROWSER: "webkit"\n',
rules: [VIOLATION.BROWSER_ENV],
},
{
name: "flags a shell-assigned SMOKE_BROWSER",
text: " - run: SMOKE_BROWSER=firefox npm run smoke\n",
rules: [VIOLATION.BROWSER_ENV],
},
{
name: "flags an expression, which cannot be checked statically",
text: " SMOKE_BROWSER: ${{ matrix.browser }}\n",
rules: [VIOLATION.BROWSER_ENV],
},
{
name: "allows SMOKE_BROWSER set explicitly to chromium",
text: " SMOKE_BROWSER: chromium\n",
rules: [],
},
{
name: "allows the smoke suite, which belongs in CI",
text: " - run: npm run smoke\n",
rules: [],
},
{
name: "allows the Chromium engine pass, which belongs in CI",
text: " - run: npm run smoke:web:chromium\n",
rules: [],
},
{
name: "allows smoke:tui, which self-skips under CI on its own",
text: " - run: npm run smoke:tui\n",
rules: [],
},
{
name: "allows the rest of the CI chain",
text: [
" - run: npm run validate",
" - run: npm run coverage",
" - run: npm run verify:build-gate",
" - run: npm run verify:bundle-externals",
" - run: npm run test:storybook",
"",
].join("\n"),
rules: [],
},
{
name: "does not read a comment as an invocation",
text: " # smoke:web:firefox runs in npm run local:gate, never here\n",
rules: [],
},
{
name: "does not read a shell comment inside a run block as one either",
text: " # SMOKE_BROWSER=webkit is local-only\n",
rules: [],
},
];
for (const { name, text, rules } of cases) {
it(name, () => {
const found = findWorkflowViolations(text, "case.yml").map((f) => f.rule);
assert.deepEqual(
found,
rules,
formatWorkflowViolations(findWorkflowViolations(text, "case.yml")),
);
});
}
it("reports the line a finding came from", () => {
const [finding] = findWorkflowViolations(
"jobs:\n build:\n steps:\n - run: npm run local:gate\n",
"main.yml",
);
assert.equal(finding.line, 4);
assert.equal(finding.file, "main.yml");
assert.equal(finding.match, "local:gate");
});
it("derives the forbidden engines rather than listing them by hand", () => {
// Adding a fourth engine to SUPPORTED_BROWSERS must extend the guard, not
// open a hole in it — which is why this reads the derived constant.
assert.deepEqual(FORBIDDEN_ENGINE_SCRIPTS, [
"smoke:web:firefox",
"smoke:web:webkit",
"smoke:web:engine",
]);
});
});
describe(".github/workflows", () => {
const files = readdirSync(workflowDir).filter((f) => /\.ya?ml$/.test(f));
it("has workflow files to check", () => {
// Deny-by-default: an empty glob would otherwise make the check below pass
// vacuously after a rename or a move of the workflow directory.
assert.ok(
files.length > 0,
`no workflow files found in ${workflowDir} — the guard below would pass vacuously`,
);
});
it("never invokes the local pre-push gate or a non-Chromium engine pass", () => {
const findings = files.flatMap((file) =>
findWorkflowViolations(
readFileSync(join(workflowDir, file), "utf8"),
file,
),
);
assert.deepEqual(
findings,
[],
"GitHub CI must not run the local-only gate or a non-Chromium engine " +
`pass (#2146):\n${formatWorkflowViolations(findings)}`,
);
});
});