Repository navigation
ci: two check rows per pull request - the guards run as the first step of the required check - #3822
Conversation
…p of the required check Every pull request started twelve workflow runs: the one required check and eleven advisory rows, each with its own checkout and setup-python for a script that finishes in seconds. Measured on strands-labs#3816: 13 check rows, 12 runner slots and 7.1 runner-minutes beside the 29.8-minute suite, and one row (llm-input-safety) ended with a step named "Always pass". Now two rows. `ci.yml` (was pr-and-push.yml) keeps the required check's job id and callee, so the ruleset's one required context `call-test-lint / Test and Lint` is unchanged, and adds a `security` job: CodeQL init/analyze and the dependency review in one job, the weekly CodeQL schedule kept. Inside test-lint.yml a `Guards` step runs before the install - scripts/ci_guards.py calls the same scripts with the same arguments and folds their exit statuses: closing reference, changelog fragment, merge-base overlap, `uv lock --check`, and the LLM-input scan, which now fails on a finding (zero on main). A branch that breaks a convention hears about it in under a minute, not after the suite. griffe and the AgentTool contract diff run after the install and write to the step summary; the two bot comments are gone. The docs build runs only when docs/ or mkdocs.yml changed; docs.yml deploys from main. The last-push-approval report is deleted: the ruleset enforces it and a check that cannot fail is a row that says nothing. Deleted: agent-api-check, breaking-change-check, changelog-fragment, closing-reference, codeql, dependency-review, last-push-approval, llm-input-safety, lockfile-parity, merge-base-overlap. Workflow files 14 -> 4. The base-checkout shape (strands-labs#1791) is no longer needed: the merge commit the required check evaluates carries every script on the base tip. The commits each guard grades are still named with --head, because a merge-commit head defeats the overlap and fragment checks. The pins that read the deleted workflows now read ci.yml, test-lint.yml or ci_guards.py; the two exemption tables (closing-reference's `edited` trigger and its missing concurrency block) are empty because no workflow subscribes to a sha-invariant event any more. Three whole-tree graders read the new files and had to be answered rather than worked around: the list-display concatenation rule (the griffe footer is now a named constant), and the rule that a documented check invocation names its repository - the sweep AGENTS.md already spells with `--repo` further down, so the duplicate sentence goes. The same section still described the deleted `Report the last-push-approval state` row in the present tense, and the advisory-context list still named `dependency-review` and `Detect Breaking Changes`; both now read as they are. One test cell is added because none of the eight failed when the LLM-input guard returned its hits as a pass, which is the behaviour this removes: it drives `main` with the other four guards stubbed, so a return to advisory is a red cell.
yinsong1986
left a comment
There was a problem hiding this comment.
Summary
Folds ten advisory workflow rows into the one required check: pr-and-push.yml becomes ci.yml (keeping the required call-test-lint / Test and Lint context byte-identical), a new security job carries CodeQL + dependency review (including the weekly schedule), and scripts/ci_guards.py runs the five cheap guards as the first step of test-lint.yml before the install, folding their 0/1/2 exit statuses. The LLM-input scan is promoted from annotation to gate (zero findings on the tree, pinned by tests/test_ci_guards.py), the griffe/AgentTool drift reports move to the step summary, and docs strict-builds run inside the required check only when docs/ or mkdocs.yml changed. The deleted per-workflow pins are re-homed onto the fold rather than dropped, and the merge-tree-vs-head-sha reasoning (--head stays the PR head because the overlap/fragment checks are defeated by a merge commit) is carried over correctly.
What's good
- The required check context (
call-test-lint / Test and Lint) is preserved exactly, so branch protection needs no ruleset change. - Every folded guard keeps its script, arguments, and 0/1/2 exit-status contract;
_as_verdictmaps unexpected statuses to BROKEN rather than a finding, matching the repo's exit-2 conventions. - The regression tests were migrated, not deleted: 155 tests in the touched CI/gate suites pass on the head, including the new pin that fails if the LLM-input guard is reverted to advisory.
- Changelog fragment present and named for the PR; no emojis, no host paths, all action refs SHA-pinned.
Verification suggestions
After addressing the inline permissions comment, confirm the closing-reference guard actually reads the link set in CI: open a scratch PR whose title carries closes #<open-issue> with no body link, and check the Guards step reports title-only-claim (exit 1) rather than unknown-links (exit 0). The unknown-links outcome renders as a pass, so only a planted finding distinguishes a working guard from a silently defanged one.
…check Four module docstrings still described `pr-and-push.yml` in the present tense after the rename, so a reader following the reference reached a file that does not exist. The past-tense and dated-measurement mentions keep the name the file had when they were measured, which is what makes them readable.
|
Pushed
Two review findings left, both needing a workflow-file edit: 1. # Deploy to GitHub Pages only from main.
deploy:
+ if: github.ref == 'refs/heads/main'
needs: buildNothing pinned it; this fails on the ungated job and passes on the gated one: def test_only_main_can_publish_the_docs_site() -> None:
docs = (_WORKFLOW_DIR / "docs.yml").read_text(encoding="utf-8")
assert "workflow_dispatch:" in docs, "docs.yml no longer takes a dispatch; re-derive whether the gate is needed"
header = docs[docs.index(" deploy:") :].split("steps:")[0]
assert "if: github.ref == 'refs/heads/main'" in header, header2. Three rows, not two. |
… caller grant is only a ceiling A called workflow's job-level permissions block sets every scope it does not name to none, and the caller can only downgrade. So the grant on ci.yml's call-test-lint never reached the guards step: the closing- reference query ran without the scope, the script mapped the unreadable link set to unknown-links / exit 0, and the guard passed every pull request while rendering like a working one - with the workflow that had carried the working grant deleted in the same change. Declare the scope on the test-lint job and grade both halves: the callee's declaration and the caller's ceiling.
…he ref that dispatched it Folding the pull-request build into the required check dropped the deploy job's if: along with its pull_request half, but workflow_dispatch is still subscribed and runs on whatever ref dispatched it, so a dispatch from a topic branch would have published that branch to the live site under a comment reading 'only from main'. Restore the ref half and pin it on the job header.
yinsong1986
left a comment
There was a problem hiding this comment.
Summary
Folds ten advisory workflow rows into the one required check: pr-and-push.yml becomes ci.yml with the required call-test-lint / Test and Lint context unchanged (job id and callee job name preserved), a security job carries CodeQL + dependency review with their steps and the weekly schedule intact, and scripts/ci_guards.py runs the five cheap guards (closing-reference, LLM-input scan, changelog-fragment, merge-base-overlap, uv lock --check) as the first step of test-lint.yml before the install, folding their exit statuses with 0/1/2 semantics that distinguish a finding from a broken guard. The R1 permissions finding is resolved at this head: test-lint.yml's job declares pull-requests: read itself (a called workflow's job-level block zeroes unnamed scopes; the caller's grant is only a ceiling), and the pin test grades both halves. The docs-deploy regression from the fold (workflow_dispatch publishing a topic branch) is fixed with if: github.ref == 'refs/heads/main' on deploy and pinned by test_only_main_can_publish_the_docs_site. Verified locally at 6309d777: all 425 tests across the 14 CI-scope test files pass, ruff is clean on the new files, and ci_guards.py behaves as documented on non-PR events (exit 0) and a missing head sha (exit 2, broken-not-finding).
What's good
- Every deleted workflow's function is accounted for: guards fold into the required check, griffe/agent-api become the
api-driftsummary report, CodeQL/dependency-review move to thesecurityjob with the schedule and the event-keyed concurrency group preserved, and the docs PR build moves intotest-lint.yml. - The LLM-input scan becoming a gate ships with a zero-findings pin on the tree and an entry-point regression test (
test_a_planted_finding_is_the_step_exit_status) that fails if the guard reverts to advisory. - Exit-status discipline (0 clean / 1 finding / 2 broken) is carried through
_as_verdictand_report, so a guard that cannot compute an answer reads as a defect in the guard, never as a pass or a finding on the branch. - All
uses:references stay pinned to full commit SHAs; the guards add no new third-party action (pip install uvper lockfile-parity.yml's original reasoning). - Both R1 fixes carry regression tests that fail on the pre-fix shape, per AGENTS.md's pin-every-reviewed-fix convention.
…sk for A called workflow may not hold a scope its caller did not grant, and the mismatch is a startup failure with no job to read. strands-labs#3822 gave test-lint's job pull-requests: read for the closing-reference guard and granted it from ci.yml; pypi-publish-on-release.yml, the other caller, was not updated. The v0.5.2 release fired it at 07:05:33Z and it never ran: the tag reached GitHub and not PyPI (pypi.org still serves 0.5.1).
…lee declares test-lint.yml's job asks for pull-requests: read (strands-labs#3822). A caller's permissions block is a ceiling, so a call site that does not grant it makes GitHub refuse to start the run - startup_failure, zero jobs, nothing to read. ci.yml was updated with the callee; pypi-publish-on-release.yml, the other caller, was not, so the v0.5.2 tag reached GitHub and never reached PyPI. The existing pin named ci.yml as the caller, which is how the second one went unnoticed. The call sites are discovered instead, and each is graded against the scopes every job of its callee declares.
…sks for The publish caller granted contents: read while test-lint.yml asks for pull-requests: read since strands-labs#3822, so GitHub refused the v0.5.2 publish at startup with no job to read. Fails on the pre-fix tree naming that pair; actionlint reads each workflow alone and does not model the ceiling.
…sk for (#3831) * fix(ci): the release workflow grants test-lint the scope its guards ask for A called workflow may not hold a scope its caller did not grant, and the mismatch is a startup failure with no job to read. #3822 gave test-lint's job pull-requests: read for the closing-reference guard and granted it from ci.yml; pypi-publish-on-release.yml, the other caller, was not updated. The v0.5.2 release fired it at 07:05:33Z and it never ran: the tag reached GitHub and not PyPI (pypi.org still serves 0.5.1). * test(ci): every reusable-workflow call site grants the scopes its callee declares test-lint.yml's job asks for pull-requests: read (#3822). A caller's permissions block is a ceiling, so a call site that does not grant it makes GitHub refuse to start the run - startup_failure, zero jobs, nothing to read. ci.yml was updated with the callee; pypi-publish-on-release.yml, the other caller, was not, so the v0.5.2 tag reached GitHub and never reached PyPI. The existing pin named ci.yml as the caller, which is how the second one went unnoticed. The call sites are discovered instead, and each is graded against the scopes every job of its callee declares. --------- Co-authored-by: strands-agent <217235299+strands-agent@users.noreply.github.com>
…ish by tag (#3832) * ci(publish): grant the reusable test job the permission it asks for, and re-run a refused publish by tag Publishing v0.5.2 was refused at startup: the caller granted call-test-lint contents: read, and test-lint.yml has asked for pull-requests: read since the guards moved into it (#3822). A reusable job may hold nothing its caller withheld, so no job started and PyPI still serves 0.5.1. Grant it. Add workflow_dispatch with a tag input, checked out in the build job so hatch-vcs reads the version from that tag - a refused publish is re-run against the release that exists rather than re-published. * test(ci): a caller grants the scopes the reusable workflow it calls asks for The publish caller granted contents: read while test-lint.yml asks for pull-requests: read since #3822, so GitHub refused the v0.5.2 publish at startup with no job to read. Fails on the pre-fix tree naming that pair; actionlint reads each workflow alone and does not model the ceiling. * chore(changelog): name the fragment for its pull request number * test(ci): pin the dispatch path that re-runs a publish against its tag #3831 already grades the caller-vs-callee permission ceiling, so the duplicate grader here is dropped in favour of it. What remains unpinned is the half only this branch carries: a release event runs the workflow file the tag names, so the dispatch input is what re-publishes a release already out. --------- Co-authored-by: strands-agent <strands-agent@users.noreply.github.com>
What
Ten advisory workflows fold into the one required check.
pr-and-push.ymlbecomesci.yml, keeping job idcall-test-lintand its callee, so the required contextcall-test-lint / Test and Lintis unchanged; asecurityjob carries CodeQL and the dependency review.scripts/ci_guards.pycalls the same guard scripts with the same arguments before the install and folds their exit statuses (0 clean / 1 finding / 2 broken). Workflow files 14 -> 4.Why
Each advisory row paid its own checkout and
setup-pythonfor a script that finishes in tenths of a second, and 3 of the 13 rows measured on #3819 could not fail - one listed twice, one a step named "Always pass", which is now a gate. A broken convention is reported in under a minute instead of after the suite.Tests
call-test-lint / Test and Lint,Securitychangelog-fragment FINDING, exit 1, other four still reportedtests/test_ci_guards.pyAuto-merge could not be armed: enabling it on a pull request that edits
.github/workflows/**needsworkflowscope.§13 Review rounds
pull-requests: readon ci.yml'scall-test-lintnever reached the guards step: a called workflow's job-levelpermissionssets every unnamed scope to none and the caller can only downgrade, socheck_closing_reference.pyran without the scope, mapped the unreadable link set tounknown-links/ exit 0, and the guard passed every pull request while rendering like a working one.1ead4f1c- thetest-lintjob declares the scope itself; ci.yml keeps the ceiling.test_closing_reference_gate.py::test_the_gate_asks_for_the_scope_the_link_set_needsnow grades both halves (callee declaration AND caller ceiling); fails with the callee grant removed, 89/89 in file.deployjob'sif:with itspull_requesthalf, butworkflow_dispatchis still subscribed and runs on the dispatching ref, so a dispatch from a topic branch published that branch to the live site.6309d777-if: github.ref == 'refs/heads/main'restored ondeploy.test_only_main_can_publish_the_docs_site- fails on the ungated job. 522 pass across the 20 workflow-reading test files, ruff clean.Still true and not a defect in this fold: a third check row,
CodeQL, is posted bycodeql-action/analyzeitself regardless of the job it sits in, so the count per pull request is three rows, not two; the title's "two" refers to the two jobs this workflow declares.