Repository navigation
Optimizing and documenting repo-hygiene processes: rename the local pre-push gate off the name ci, and guard it out of CI #2146
Description
Activity
- addedv2Issues and PRs for v2Issues and PRs for v2choreMaintenance: deps, build tooling, CI, cleanup — no user-facing behavior changeMaintenance: deps, build tooling, CI, cleanup — no user-facing behavior change
on Aug 26, 2026 Triage: Priority Medium (total 6)
- Severity 2 — nothing is broken; the cost is confusion and a latent hazard, and knowing
npm ci≠npm run ciis a workaround - Urgency 3 — wanted this milestone. The hazard is concrete rather than theoretical: the local-only steps are one plausible workflow edit away from running in GitHub CI, and one of them (WebKit) fails outright
- Bonuses: +1 milestone
Not scored higher despite the "must never run in CI" framing: the guard in scope item 2 is what makes that durable, and until someone actually edits the workflow, nothing is currently wrong.
- Severity 2 — nothing is broken; the cost is confusion and a latent hazard, and knowing
Correction — the scope of the guard was wrong as first written. Body updated.
@cliffhall: "I do want smoke tests to run in CI (they do now, right?) just with chromium only. It's the firefox and safari ones I'm not certain of."
Right on both counts, and I had conflated three different things into one list.
Yes — the smokes run in GitHub CI today. The workflow runs
npm run smoke, which issmoke:launcher && smoke:cli && smoke:tui && smoke:web && smoke:web:chromium. Chromium only. That is the intended state and nothing here should change it.The three categories, kept separate now:
In GitHub CI? Guard it? smoke:launcher,smoke:cli,smoke:web,smoke:web:chromium✅ yes, and should be No smoke:tui✅ invoked via smoke, self-skips there (needs a real TTY)No — it handles itself smoke:web:firefox,smoke:web:webkit❌ never Yes My first draft listed
smoke:tuialongside the engine passes. That was wrong: it is already in CI and no-ops there by design, so forbidding it would both misdescribe the status quo and block someone legitimately touching it later.The guard is about engines, not smokes. It now fails on: invoking
local:gate; invokingsmoke:web:firefox/smoke:web:webkit; invokingsmoke:web:engine(env-driven, so the engine cannot be read off the workflow at all); or settingSMOKE_BROWSERto a non-Chromium value — that last one being the back door that would redirect an otherwise innocentnpm run smoke. It explicitly must not forbidnpm run smoke,smoke:web:chromium, orsmoke:tui.Acceptance criteria updated to match.
- linked a pull request that will close this issuechore(scripts): rename the local pre-push gate off `ci`, and guard it out of CI (#2146) #2167
on Aug 27, 2026 - added 12 commits that reference this issue
on Aug 27, 2026 - added a commit that references this issue
on Aug 29, 2026
Problem
The root
ciscript is not what runs in CI, and the name actively works against us.The smokes themselves do run in GitHub CI, and should — the workflow runs
npm run smoke, which coverssmoke:launcher,smoke:cli,smoke:tui,smoke:webandsmoke:web:chromium. That is not in question here.What must never run in CI is narrower: the non-Chromium engine passes.
smoke:web:firefox(#2086) is deliberately absent, andsmoke:web:webkitfails two of the three smokes outright. Confidence in the cross-engine runs is not high enough to let them break CI.smoke:tuiis a third, separate case and is not in that category: it is invoked in CI viasmokeand self-skips there (process.env.CI), because the Ink TUI needs a real TTY. It handles itself and needs no guarding.GitHub CI runs
validate,coverage, the two verify gates,smokeandtest:storybook— never the rootciscript itself.Three distinct problems follow from the name:
1. It collides with a built-in npm command.
npm ciis clean-install-from-lockfile. It does not run theciscript — verified. Sonpm ciandnpm run ciare one keystroke apart, do entirely different things, and the wrong one fails by succeeding at something else: several minutes of reinstallingnode_modulesinstead of running the gate, with no error to tell you.2. It invites the cross-engine passes into CI by accident. This is the one that matters. A future contributor editing
.github/workflows/main.ymlwho sees a script calledcihas every reason to think it belongs there — or to "fix" a workflow by changingnpm citonpm run ci. Either would silently pullsmoke:web:firefoxinto GitHub CI. The maintainer position is that the cross-engine passes are always the local pre-push gate, period — confidence in them is not high enough to let them break CI, andsmoke:web:webkitfails two of the three outright.3. It muddles the concept every time it is documented. Six review rounds on #2133 kept turning up prose that blurred "GitHub CI" and "the local gate". A script literally named
cifor the local-only thing guarantees that keeps happening.Scope
Widened from a pure rename to repo-hygiene process work, since it touches ~40 references across 15 files and the concepts they describe.
1. Rename the gate, with no alias
ci→local:gate(suggestions welcome, but it should say local).No back-compat alias. Deliberate: an alias preserves exactly the association being scrubbed, and leaves something copy-pasteable for a workflow author. Removing it outright means
npm run cifails withMissing script: ciand npm prints the available scripts — a good failure that teaches the right name.prepush:gate. npm pre/post hooks match the exact script name, soprepush:gateis the pre-hook ofpush:gateand fires automatically if anyone ever adds one — verified. Plainprepushis safe (it does not hookpush:gate), butlocal:gateavoids the class entirely and reads as the counterpart to GitHub CI.Rename
ci:storybooktoo — it is also local-only (the workflow callsnpm run test:storybookfromclients/webdirectly, not this wrapper). Something likelocal:storybookor fold it into the gate.Commit history and older PR bodies will reference
npm run ci. That is fine; history is a record of what was true then.2. Guard that the gate can never drift into CI
The rename removes the invitation; a check removes the possibility. Add a
test:scriptsassertion over.github/workflows/**that fails on:local:gate);smoke:web:firefox,smoke:web:webkit);smoke:web:engine, whose engine comes from the environment and so cannot be read off the workflow at all;SMOKE_BROWSERto anything other thanchromium— the back door that would redirect an otherwise innocentnpm run smoke.It must NOT forbid
npm run smoke,smoke:web:chromium, orsmoke:tui. Those belong in CI and are there today;smoke:tuiself-skips underprocess.env.CIon its own. The guard is about engines, not about smokes.This is the durable half. Everything else here is prose that can rot; this cannot.
3. Document the two-tier model in one place
Right now the CI-vs-local split is described in
README.md,AGENTS.md,.github/copilot-instructions.md, three client READMEs, and several script headers — which is why it keeps drifting. One canonical table (what runs in GitHub CI, what runs only locally, and why each local-only step is local-only), with the others pointing at it.Per the mirror rule,
AGENTS.mdand.github/copilot-instructions.mdchange in the same PR.4. Sweep the stale references
~40 across
README.md,AGENTS.md,.github/copilot-instructions.md,clients/{web,cli}/README.md,docs/inspector-roadmap-2026-h2.md, and header comments inscripts/*.mjs+ one test.Acceptance criteria
ciorci:*;npm run cifails with npm's missing-script error.test:scriptscheck fails if any workflow file invokes the gate, a non-Chromium engine pass, orsmoke:web:engine, or setsSMOKE_BROWSERto a non-Chromium value — while continuing to allownpm run smokeandsmoke:web:chromium, which belong in CI.AGENTS.mdand.github/copilot-instructions.mdupdated together.Notes
Came out of #2133, where
smoke:web:firefoxwas deliberately placed in the local gate rather than GitHub CI after a trialled CI job was removed for never disagreeing with Chromium. That decision is only as durable as the thing stopping someone from undoing it by accident, which today is nothing but a comment.