Run the instruction drift check as a step - #994
Conversation
The check was a job calling `check-drift.yml` as a reusable workflow. Actions is free on a public repository, so nothing was billed for it; what it cost was a job -- a scheduling slot and a row in every run, and this is the busiest consumer on the list -- for nine seconds of byte comparison. As a step it runs inside a job that already exists, and the fleet's other consumers now carry it the same way. `check` hosts it because the step must not inherit the `docs_only` gate. `changes` counts `**/*.md` as documentation and AGENTS.md is a `.md` file, so a pull request that edits only a generated region is docs-only: `test-core` and the Docker E2E matrix would skip the check on exactly the changes that cause drift, and the redteam job, gated the other way, would skip it on every change that touches code. `check` carries no `if:` and already runs the Markdown lint. There is no `Markdown` job here to use instead, as there is in every other consumer. The step goes last and carries a guard. A step's default `success()` would skip it whenever any of the twenty steps above failed -- clippy, ruff, biome, five validation scripts and cargo audit among them -- and this job has more ways to fail before reaching it than any other in the fleet. As a job it reported regardless, and it should. The header of update-instructions.yml named `.agents/instructions.toml`, which exists in no consumer; the pin is `.agent-instructions.toml`. Closes #993
Temporary: proves the drift check still fails from its new home. Reverted in the next commit.
Restores the marked block. The failing run it produced is linked from the pull request.
Test plan resultsA drifted block fails. Run 34420384401 ( One thing in that run is worth reading carefully, because it looks like evidence and is not. All three downstream jobs report That is not the change filter. Those jobs are The docs-only case rests on The revert goes green. Run 34421788966 ( |
1 participant
Closes #993.
The drift check was a job calling
aicers/agent-instructions/.github/workflows/check-drift.yml@mainas a reusable workflow. Actions is free on a public repository, so there is no bill to cut here — what the arrangement cost was a job: a scheduling slot and a row in every run, and this is the busiest consumer on the list, with at least 300 CI runs in the last thirty days. As a step it runs inside a job that already exists. aicers/agent-instructions#60 shipped the check as a composite action, and every other consumer already carries it this way.What changed
ci.yml: theinstructionsjob is gone;checkgainsid: checkouton its checkout and- uses: aicers/agent-instructions/check-drift@mainas its last step, aftercargo audit.update-instructions.yml: the header comment said the apply moves the pin in.agents/instructions.toml. No such file exists in any consumer — the pin is.agent-instructions.toml. The other copy of that wrong path was in the job comment this change deletes.Nothing else in
checkis touched —Swatinem/rust-cache,actions/setup-pythonand the marketplace lint actions in that job are separate questions, and folding any of them in would bury this change..agent-instructions.tomland the marked blocks inAGENTS.mdare untouched,checkstays onubuntu-latest, and the check does the same work with the same messages.Why
check, and not theMarkdownjob the siblings useThere is no
Markdownjob here.Run Markdown Lintis a step insidecheck, beside the Rust, Python, Biome and validation steps.What decides it is the
docs_onlygate.changescountsdocs/**,**/*.md,mkdocs.ymland.markdownlint*as documentation, andAGENTS.mdmatches**/*.md— so a pull request confined to a generated region is docs-only:test-coreandtest-docker-e2e-matrixare gated ondocs_only != 'true', so a step in either would be skipped on precisely the pull requests that can cause drift.test-registrar-redteam-docsis gated the other way, ondocs_only == 'true', so a step there would be skipped on every pull request that touches any code.checkis the only job with noif:at all.Why the step is last, and guarded
This matters more here than in any other consumer:
checkhas roughly twenty steps ahead of the new one —cargo clippy,cargo doc,ruff,biome ci, five validation scripts,cargo audit— and a step's defaultsuccess()would skip the drift check whenever any of them failed. As a separate job it reported regardless, and it should: a drifted block is drift whether or not clippy is also unhappy.!cancelled()rather thanalways()so a cancelled run stops; thecheckoutclause keeps a failed checkout from being reported as drift, since the action fails outright when it cannot read the pin file and that message names the pin for a problem that is not the pin's.Branch protection
Nothing to edit, and this is the part CI cannot demonstrate.
GET /repos/aicers/bootroot/branches/main/protectionanswers 404"Branch not protected", so no status check is required onmainandInstructions / checknever was. Organization-level rulesets needadmin:orgto read and were not inspected — worth a maintainer's glance that no organization rule names the check this removes.Verification
actionlintreports nothing new on the edited file. It does report three shellcheck findings —SC2129,SC2155,SC2034— but all three are pre-existing: the same three appear whenactionlintis run againstmain's copy of this file, at lines 188, 296 and 381 there and at 223, 331 and 416 here only because the drift step shifted the line numbers. They are left alone rather than folded into an unrelated change.A drift run and its revert are posted below.