Land gate-placement slice 0: ten push, round and CI gate repairs (#5437) - #41
Merged
Merged
Conversation
* Add the gate-placement slice 0 spec The spec folder for the ten gate repairs this branch carries, copied byte for byte from the lead's worktree, where it is untracked. It lives under Open Knowledge's specs/, which the public export strips. * [US-001] Provision actionlint for script-tests, map two allowlists, refuse unreadable manifests The round's native actionlint case skipped in CI because the script-tests job never installed actionlint. The version pin moves to one workflow-level ACTIONLINT_VERSION read by both format-and-actionlint and script-tests, and script-tests gains the same cache, install-on-miss and PATH steps, passing the version to the download script as its argument. Under a truthy CI the case now fails naming the missing binary instead of skipping; locally it still skips. The root round maps an allowlist-only edit of config/drift-allowlist.json to check:dep-drift and of config/nul-byte-allowlist.json to check:nul-bytes, in the shape of the guard-pairing allowlist mapping. The round's fixture plants a real public/private drift and scans its own root, so both directions are observed. The push helper used to read an unreadable affected subtree manifest as "script not defined" and keep going. It now fails selection with exit 2, naming the path and the parse cause, before anything is dispatched. A subtree directory absent from the checkout still reads as declaring no script. * [US-002] Run changed and paired root tests at push, and plugin tests only on plugin inputs The push ran the whole root script corpus for any change under scripts/, config/, .github/, .husky/ or a root config file, and the plugin corpus on every push. It now runs what the diff calls for, the same selection the root round makes, while both whole corpora stay required in CI's script-tests job. The push helper answers a new internal question, --push-script-tests. The answer opens with the marker push-script-tests/1 and names each selected root test with its reason (a changed test, the paired test of a changed script, or the surviving paired test of a deleted one), plus whether the plugin corpus is needed. The plugin trigger is derived from the marketplace manifest the plugin runner already reads: a change under a registered plugin source, to the manifest, or to the plugin runner. The change list is built without rename detection, as the round's is, so a rename selects alike at both boundaries. An unknown scope answers "root all" and "plugin run". --root-script-tests-needed keeps its exact contract for older hooks, and --print-base is unchanged. The selection moves out of check-agent.mjs into a leaf module, scripts/root-test-selection.mjs, which the round and the helper both import, so the two boundaries cannot drift and no import cycle forms. The root runner gains a selected-files mode (--selected-file=scripts/<name>), which it consumes and never forwards to node --test. It keeps the serial and parallel partition, skips an empty partition, refuses any file outside its own corpus with its remedy and no bypass text, and still refuses free positionals. The test:scripts command text stays as two tests pin it. The hook asks a question only of a helper whose source carries the question's flag, through #5318's helper_predates guard, which now also covers --push-script-tests. A helper older than the question is never executed for it, so a helper from before #4529 never runs its scoped check a second time. Any answer the hook did not design, including exit 0 without the marker, counts as no answer: the hook runs both corpora with a line naming what clears it, either updating the worktree's scripts from origin/main or, for a helper that failed, its own output printed above the line. Verification substrate: the real-hook fixture now takes a hook source and a helper file set, drives the real root runner with a planted-red paired test, and records every helper execution. Frozen copies of the hooks from before before #4529 sit under scripts/__fixtures__/pre-push-vintages/, each pinned byte-identical to its source blob, because script-tests checks out shallow. The older-hook cells run each older hook with the new helper and show the same behaviour as with its own helper. A parity fixture with a renamed paired script gives the same selection at the round and the push. new question, keeping each row's intent. The hook's comments, the helper's usage text, root AGENTS.md steps 7 and 8, CI.md, and the architecture, quality-gates and runbook documents describe the selection. * [US-003] Budget a pull request's changesets against its fetched base branch The Open Knowledge changelog budget step gave the guard the event's pull_request.base.sha. On a pull request the checkout is the merge ref, whose first parent is a newer base, so the merge-base was the stale snapshot and the diff swept in every changeset the base had gained since. A PR behind main could fail the budget on main's own changesets. On pull_request the step now passes origin/<the PR's base branch>, which the full-history checkout already fetches, and the guard takes the merge-base itself. The expression is conditioned on the event, so a merge group still gets its own base_sha and dispatch still gets origin/main, and no event can receive a bare origin/. PRs into research/ok-cloud get their own base branch, not a hardcoded main. The guard's suite gains a merge-ref fixture with both parents asserted: the snapshot base fails on main's changeset and the fetched base stays green, while both are green at the branch head. A PR's own over-budget changeset fails under both bases, and an absent base ref exits 2 with stderr naming it. The workflow pin now holds the event-conditioned expression and still asserts full-history checkout. * [US-004] Stop advising the hook bypass in refusals and gate documents The push helper ended its step-failure and scope-discovery refusals with "To skip in an emergency: git push --no-verify", the hook's header and its dependencies preflight named the flag, and four gate documents plus Open Knowledge's perf test guide offered the bypass as an action. Agents execute printed remedies literally, so a refusal should name its cause and a fix, never a way around the check. The guidance lines are deleted: the hook header line, both helper prints, the runbook's emergency-bypass bullet, the bypass clause in CI.md, the CI architecture document and the quality-gates layer-3 row, and the perf guide's push-with-the-flag bullet. Lines that named the flag while saying something else keep their meaning without it: the preflight now says to install rather than skip the hook, its comment says the errors invite skipping the hook, and the janitor test's message speaks of any push that skips the hook. Explanations, prohibitions and guard assertions stay. Tests: the failing-step and both scope-discovery fixtures assert their refusal names no bypass, and a new case drives the real hook's missing-dependencies preflight, asserting the cause, the install remedy, no bypass text, and that pnpm is never reached. no-bypass-advice.test.mjs makes the residual search a standing root test. It reads the tracked files of the hook, the root scripts and their tests, CI.md, AGENTS.md, the .github gate documents and the perf guide, allows the flag only inside the excerpts it lists (each an explanation, a prohibition or a no-bypass assertion), skips the frozen hook and helper copies under scripts/__fixtures__/, and fails naming any other line. It also reports an allowed excerpt that no longer appears, so the list cannot rot. Its self-tests show a planted hint, and a hint appended to an allowed line, are reported while allowed, frozen and unsearched lines are not. * [US-005] Print the derived safe Biome fix, only after a Biome failure After a lint failure the Open Knowledge executor printed `pnpm run format`, which writes safe and unsafe fixes over packages, docs and the root files, so following it rewrote files the change never touched. At the push it printed that for any whole-lint failure, including an Oxlint or no-comments one. The executor now derives the remedy from the same Biome leg it builds from CI's `lint`: Biome's safe write (--write, never --unsafe) over the changed files inside that leg's CI scope, printed as one runnable command with its working directory and every file operand single-quoted. At the round the failing leg identifies Biome. At the push, where `lint` runs as one step, a whole-lint failure is attributed by re-running only the derived Biome leg, check-only, over the push's changed in-scope files, and the remedy is printed only if that re-run fails; otherwise the executor says no formatting fix applies. With no changed file inside Biome's scope, or a leg whose changed-file form cannot be derived, it prints no command and says why. The push's success path is unchanged. The executor suite gains a real-Biome fixture: a pinBiome shim running Open Knowledge's Biome 2.5.11, a copy of its biome.jsonc, a formatting defect in changed files whose paths carry brackets, spaces and parentheses, and an unsafe-only useLiteralKeys finding in a changed and an unchanged file. The printed command, executed under sh and zsh from another directory, clears the defect, leaves the unchanged file byte-identical and leaves the finding in the changed file. Two push fixtures show an Oxlint and a no-comments failure print no fix, and a third shows the out-of-scope case names the scope. The existing remedy-text cases move to the new output. * [US-006] Compare an explicit stale base from the branch's departure point at the root Given an explicit pin the branch has since passed by merging a newer origin/main, the root round and the push helper compared from merge-base(pin, HEAD), so they checked main's changes as the branch's: paired tests of main's scripts, actionlint on main's workflows, subtree rounds and widening for main's files, and the changelog leg judged main's changesets. working directory as a parameter, from the Open Knowledge executor into departure-point.private.mjs, which the executor imports along with the shared failureOf. One shared resolution in check-pre-push.mjs applies it to an explicit base: the root round's --base= or GATE_BASE_SHA, and the push helper's --base= on its main path, both step-7 questions and --print-base. It keys commit-id-or-ref on the caller's original string, so a ref is used as given while an abbreviated or full commit id can move, and derived bases never reach it. Each entry point prints the helper's `base check:` line, on stderr wherever stdout is an answer the hook parses. The push helper's changelog leg and its remedy text now use the comparison. The module is loaded only on that path and only when present, so a checkout without the Open Knowledge scripts keeps the pin and says so, and #4529's absent-subtree rule still holds. On a base branch with no usable upstream the push helper now prints one `base:` line naming how the base was chosen, and the runbook notes that a main-bound branch which merged research/ok-cloud resolves to the cloud line, so its push does not check the cloud code it brings in; the PR's CI does. Both suites gain trusted-main fixtures with origin/main written by a real fetch: the move at both entry points and through GATE_BASE_SHA, the changelog leg's base, ref and abbreviated-id provenance, no line for a derived base, the stdout of both questions and --print-base, the missing helper, and the three keep-cases that had no fixture (a value that differs from the newest fetch, both merge-bases failing, git reflog show failing). The executor's 32 departure cases keep their assertions; every fixture that copies the executor or the push helper now copies the module too. * [US-007] Give Visimer and OK Marketing rounds that do real work Neither subtree declared check:agent, so the root round printed "no round gate declared" for a change in either and passed a defect there. A bare alias cannot be the round: the root delegation and the loops' skill text pass --base, which Visimer's Vitest and OK Marketing's Biome reject. Each subtree now declares `check:agent` as a small dependency-free entry plus the script it runs: Visimer's runs `test`, OK Marketing's runs `lint`. The entry takes the arguments Open Knowledge's round takes (--base=, --all, --all-because=, a bare --, and GATE_BASE_SHA when no --base= is given), runs the whole subtree whichever it gets and says the comparison does not narrow it, refuses anything else with exit 2, fails naming the install command when the subtree is not installed, and exits with its script's status. Before running a declared round, the root delegation reads its chain from check:agent, through the entry by the script its manifest names, through pnpm runs and recursive runs into the workspace's members, and refuses a chain that could pass having done no work: --passWithNoTests, --if-present, a recursive run no member answers, or a shape it does not model. Tokens match exactly and shapes off the chain are neither followed nor matched. A root-corpus test runs that rule over every round the repository declares, so a later zero-work chain reds the required script-tests job whichever path runs the round. Tests: a table-driven contract test over both entries, a routing fixture that logs each delegated round's working directory, GATE_BASE_SHA and argv, one fixture per refused shape plus the off-chain shapes, and the real-tree assertion. The required-checks roadmap and the Visimer workflow header now say Visimer Validation is required, as the ruleset has had it since 2026-09-29. * [US-008] Run the parse-health gate and its unit suite uncached in the required lint job test:health and test:health:unit ran only at push, through cached Turbo tasks, so a push that skipped the hook shipped them unobserved and no required context ever executed them. The required open-knowledge / lint job now runs both as explicit steps in packages/core, outside Turbo, beside the perf-comparator step and with the same implicit success condition. The push set is unchanged: the executor's list of the two moves from "kept because it has no CI home" to "homed in CI, push removal pending", its tripwire now requires both to have a CI home, and a new assertion shows a push whose closure holds core still runs both. The uncached tier's placement pin becomes table-driven over the tier and the two health steps, each judged by the same rules: exactly one lint step runs exactly its command, with no continue-on-error or if, the lint job carries neither, it runs on should-run and stays in the gate's needs and result loop, and the package script is unchanged. The same mutation cases, plus a step moved into another job, red each row, and adjacent changes do not. Open Knowledge's gate document, the executor's printed text, the health test guide and the quality-gates document now describe the CI home and the pending push removal. * [US-009] Keep the Windows packaged-app echo running after a real-PTY failure In the required Windows package job, the CDP client install and the packaged echo had no if:, so a real-PTY proof failure above them skipped the only step that runs the packaged Windows app, while packaging itself survived. The client install gets an id and runs when the run is not cancelled and packaging succeeded; the echo also needs the install to have succeeded. Each condition is one line, and no step moves. Tests: the masking model now shows a real-PTY failure masks neither step, and an exact-condition pin asserts both conditions, including the install-outcome one the model cannot see, with a mutation case. The refuted placement mutation becomes two-part, since the echo now protects itself, and the ConPTY placement pin gets a constructed unconditional witness so it can still fail. The workflow and test comments describe the new conditions. * [US-011] Refuse an uninstalled or stale standalone root before the first push slot The push helper checked a standalone root's install only at that root's turn in its per-slot loop, and Discord's slot is last. A widened push for #5317 ran about 49 minutes and then refused on Discord's missing install. The helper now checks every selected standalone root before the first slot runs and refuses once, naming each root with its install command. A root absent from the checkout keeps today's skip with its warning, and the refusal keeps its kind: a selected root that is not installed is never reported as a pass. The same check refuses an install that is stale against its lockfile. pnpm writes node_modules/.pnpm/lock.yaml from the same object as pnpm-lock.yaml and decides an install is current by comparing the two (checkDepsStatus and assertLockfilesEqual in pnpm 10.33.0; pnpm 12.8.1 refuses on the same difference, observed). The check reads two files, spawns nothing, and takes under a millisecond on Open Knowledge's real install. The stale witness is #5288's Turbo bump, after which stale Open Knowledge installs failed mid-run with "unknown key agentGuidance". The changelog budget still runs first. It needs no install, and the changeset length suite pins that a long entry is reported even when subtree dependencies are not installed. The install command is built in one function, which keeps the corepack form for a root whose pnpm pin differs from the root's. The runbook gains an entry for the refusal. * Add failing entry rows for a signal-killed and an unstartable pnpm A round entry whose pnpm dies on a signal exits 1 without naming the signal or the script. The signal row fails on the current entries; the unstartable row covers the existing refusal. * Name the signal when a subtree round entry's pnpm is killed A pnpm that dies on a signal now makes the Visimer and OK Marketing entries refuse with the script and the signal named, instead of exiting 1 with no cause. * Add failing push rows for a killed or unstartable Biome attribution re-run When the re-run that attributes a whole-lint push failure is killed by a signal or cannot start, the executor still prints that the failure includes Biome and the --write remedy. Both rows fail on the current executor. * Attribute a lint failure to Biome only on a real Biome exit A spawn failure or signal death of the attributing re-run now says the attribution could not be established, names the outcome, and prints no formatting fix. * Drop the in-loop missing-install refusal the up-front check replaced Every standalone root the slot loop dispatches has already passed the up-front install check, so runScript's own refusal could no longer fire. The absent-from-checkout skip stays. * Point apt at HTTPS sources before the lint job installs zsh The required Open Knowledge lint job installs zsh from apt on the Blacksmith pool, which lost port-80 egress on 2026-09-11. It now runs the apt-https-sources action first, like the other Blacksmith apt installs, and the documents that list the action's callers name it. * Remove the prose comments this change added outside its spec-named sites The gate scripts, workflows and tests lose the explanatory comments this change added or rewrote, including one that misstated which commands the zero-work reader refuses. A hook-grammar sync note stays as one WARN line, and the lockfile comparison and the actionlint pin keep their external constraints as single UPSTREAM lines. The parse-health step names drop a 'see comment' pointer that no longer resolves. * Scope the trigger-prefix and lockfile comments to what they govern The trigger-prefix comment now names the legacy --root-script-tests-needed predicate as the only reader of that set, since current hooks ask --push-script-tests, and drops the outdated corpus size. The UPSTREAM line at the lockfile comparison states the env-document condition in one reading, matching pnpm's lockfile docs: configDependencies, or a pnpm 12+ pin without pmOnFail: ignore. * Name devEngines.packageManager in the lockfile marker's env-document condition pnpm's lockfile page lists three ways the leading env document gets written: config dependencies, a declared devEngines.packageManager (any version), or a legacy packageManager pin of pnpm 12 or newer, with pmOnFail: ignore turning off the latter two. The marker named only the first and third. GitOrigin-RevId: 09fb1befd5ae99272df21a7d21964c66f6109ccc
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Copybara-translated 1 Inkeep OSS change. Rebase-merge this PR so the prepared commit lands directly on public main.