Repository navigation
ci: teach main's admission rules and classifier the arm PR leg - #702
luisleo526 wants to merge 3 commits into
Conversation
…as two exact variants The PR workflow's admit job runs main's admission rules on the pull request's own gke-pr.yml before any privileged leg runs. A PR that adds the arm64 leg (a build-arm job, an arm_relevant admit output, and matching legs and result checks) is refused by those rules, and the base checker's ci_docs_only.py does not publish arm_relevant, so the new result check would also fail with an unanswered classification. This change is the main-side half of that PR and does not touch gke-pr.yml: - ci_docs_only.py publishes arm_relevant (true for sources, build inputs and test scripts and for any path it cannot classify; false for established docs-only and .github-only changes), with its tests. - ci_preflight.py admits exactly two shapes of gke-pr.yml. The shape that has no build-arm job is checked by the unchanged rules. The shape that defines build-arm is checked by the full new rule set: the job inventory, the arm_relevant output, the pinned arm64 label, the path policy, the exact setup and Debug steps, and the complete legs and result gate bodies. The two sets are selected together by the job definition and never mixed, so a hybrid, a second arm job, an extra job in either shape, another label, or a container, services, credentials or timeout on the arm job match neither shape and are refused. - test_ci_preflight.py runs the real rules against main's current workflow and the same workflow with the arm leg applied, and asserts the exact findings of each refusal control. - docs/ci.md documents the classifier and both admission variants without changing the file's line count. Co-Authored-By: Claude Sonnet 5.5 <[email protected]>
…tted arm job The admission rules pinned the arm job's step names, run text and absence of conditions, but a later change could still weaken the job in place: continue-on-error on the Verify Debug step or on the job would turn a failed arm build into a success in the legs check, and env, shell or working-directory on a step would change what the pinned commands run against. The generic continue-on-error rule is not part of the admission entry, so the arm job needs its own check there. In the arm variant, admission now refuses continue-on-error anywhere in the arm job and refuses env, shell or working-directory on any of its steps. Step and job timeouts stay refused by the existing runner rule. Controls change one thing each and assert the exact findings, and a quoted job key is shown to keep the exact refusals it already gets from the YAML whitelist and the unparsed-lines check. Co-Authored-By: Claude Sonnet 5.5 <[email protected]>
There was a problem hiding this comment.
Graphify reviewed this change.
Worth a look — the grounded gate found no coupling regressions or blocking issues, but 3 advisory finding(s) below merit a look before merge.
Not checked: tests were not run; formal verification proved 0 of 6 changed function(s) (1 sampled, not proven, 4 not verified).
Formal verification. 1 change(s) alter behavior, breaking input(s) attached. PR-changed functions: 2/6 verified (0 proven, 1 may-equivalent, 1 distinguished) · 4 not verified (3 vacuous, 1 unsupported).
Behavior changes: classify\_paths changes behavior, here is the input that shows it.
The verifier found a concrete input on which classify\_paths behaves differently before and after the change. If that change is intended, ship it; if not, this is your bug.
Guarantee: This difference was REPRODUCED, the verifier actually ran both versions on that input and saw them disagree. It is real, not an artifact.
Evidence: On input \{"paths":"\['a', 'a'\]"\}, the old code produced \(False, False\) but the new code produces \(False, False, True\) (outputs first differ at character 14). Paste that input straight into a regression test.
Not verified on this run: classify (unsupported), decide (vacuous: never exercised), main (vacuous: never exercised), runner\_findings (vacuous: never exercised).
Graphify review — findings
Adds an arm_relevant output to the docs-only classifier: source and build paths, scripts/test_*, C/C++/CMake files, and any unrecognised path set it true, as do empty, unreadable or malformed change lists, while established docs-only and .github/-only PRs set it false. classify_paths, classify and main now return and write a third value, so every caller that unpacked the old pair has to change, and a doc sitting under a source directory in a mixed docs/.github PR can still be flagged. Teaches preflight that gke-pr.yml has two admission variants: the OLD shape keeps the current jobs, outputs and gate bodies, and defining build-arm selects the NEW checks together (arm classifier output, a fixed Debug job on gke-16cpu-32gb-arm64, an arm-aware legs check and an independent result check). Hybrids and extra jobs, outputs, needs or arm steps are refused, but the runner-pin and missing-job checks in the arm path need to cover the arm runner and the non-GKE jobs as tightly as the old path does. Updates docs/ci.md to describe the new output and the two-step rule that checker changes land before the workflow change that uses them, and adds tests for the variant, classifier and refusal paths.
Worth a look
- classify_paths return arity changed from 2 to 3 —
scripts/ci_docs_only.py:113· Escalate · medium- NOT verified (no proof, no reproducing execution) — needs human review
- ARM variant missing-job check ignores ARM runner pins —
scripts/ci_preflight.py:2715· Escalate · medium- NOT verified (no proof, no reproducing execution) — needs human review
- Arm variant drops runs-on pins for the non-GKE gke-pr jobs —
scripts/ci_preflight.py:1762· Escalate · medium- NOT verified (no proof, no reproducing execution) — needs human review
Review partial — this diff was larger than one review pass covers, so later files were not reviewed; some findings may be missing.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 882 functions depend on the 851 functions this change touches.
Health — this change adds coupling hotspots:
- worse:
gke_admit_workflow_findings()— 14 callers, 11 callees - worse:
admit_rule_findings()— 7 callers, 9 callees - worse:
runner_findings()— 2 callers, 22 callees
Verification — 882 functions in the blast radius were not formally verified this run (proofs are advisory here).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 879 function(s) in the blast radius were not formally verified this run
Docs that may be stale (advisory)
benchmarks/results/selection.md§ Manifest (lines 50-255): references changed symbolsclassify
Formal verification
Behavior changes: classify\_paths changes behavior, here is the input that shows it.
The verifier found a concrete input on which classify\_paths behaves differently before and after the change. If that change is intended, ship it; if not, this is your bug.
Guarantee: This difference was REPRODUCED, the verifier actually ran both versions on that input and saw them disagree. It is real, not an artifact.
Evidence: On input \{"paths":"\['a', 'a'\]"\}, the old code produced \(False, False\) but the new code produces \(False, False, True\) (outputs first differ at character 14). Paste that input straight into a regression test.
Could not verify: Could not verify classify.
The verifier did not have enough to check classify, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: synthesized inputs (no suite coverage): POSSIBLE DIFFERENCE on 4 constructed input(s) — both versions returned but disagreed on the return (e.g. old=["tuple", [["bool", false], ["bool", false], ["list", [["str", "not docs-only: an unnamed event runs every job"]]]]], new=["tuple", [["bool", false], ["bool", false], ["bool", true], ["list", [["str", "not docs-only: an unnamed event runs every job"]]]]]); ADVISORY, unverified — a synthesized input may violate an unstated precondition, so this is NOT a sound refutation and does NOT gate the merge; confirm with a real test, or let carve/property refute on real inputs; property tier: not verifiable here: def-time exec: NameError (name 'ROOT' is not defined) — the function cannot be defined in the harness because a name its module provides (decorator / default / annotation / import) did not resolve in the harness namespace; a harness artifact, not evidence about the edit
Could not verify: Could not verify decide.
The verifier did not have enough to check decide, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: not verifiable here: def-time exec: NameError (name 'ROOT' is not defined) — the function cannot be defined in the harness because a name its module provides (decorator / default / annotation / import) did not resolve in the harness namespace; a harness artifact, not evidence about the edit
Could not verify: Could not verify main.
The verifier did not have enough to check main, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: not verifiable: all 12 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous (mostly NameError — names the real obstacle, not a sampling gap)
No difference found (not proven): No behavior difference found in gke\_admit\_workflow\_findings (not a proof).
The verifier ran both versions of gke\_admit\_workflow\_findings on many inputs and saw identical behavior every time. Strong evidence the change is safe, but evidence, not a proof.
Guarantee: Empirical: differential testing (both versions run on many generated inputs). A divergence on an untested input remains possible, so this is 'no counterexample found', not 'proven equivalent'.
Note: An input the sampler did not try could still differ.
Could not verify: Could not verify runner\_findings.
The verifier did not have enough to check runner\_findings, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: non-vacuity: domain too small (only 2 distinct inputs exercised, need 3) — 'no divergence' would be near-vacuous
· 1 grounded finding(s) anchored inline below; 2 more finding(s) on lines outside this diff (see the check run).
| return name == 'gke-pr.yml' and 'build-arm' in jobs | ||
|
|
||
|
|
||
| def gke_admit_workflow_findings(workflow: str, name: str = 'gke-pr.yml') -> list[str]: |
There was a problem hiding this comment.
gke_admit_workflow_findings()
fans out to 11 callees (efferent coupling); 14 callers depend on it (afferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
…ites This commit changes no file. The documentation check for unnamed sentence deletions (introduced by f85ce18) reads the commit messages in the range, and the admission-rule change rewrote six sentences of docs/ci.md into shorter wording that keeps their meaning and the file's line count. The rewritten sentences are named here: - "First merge the checker or allow-table change while the existing workflows pass." and "Then merge the workflow change that uses the newly admitted rule or label." and "Rule and table changes land in two steps." and "This order prevents one pull request from changing both sides of admission." are now one sentence: rule and table changes land first while existing workflows pass, workflow changes follow, so one PR cannot change both sides of admission. - "The classifier uses exact `.github/` and `docs/` prefixes, so lookalike directories and the classifier itself cannot create a skip." and "Renames contribute both names; empty, missing, malformed, truncated or unreadable lists are neither class." are now folded into the shorter paragraph that describes the path file and the exact prefixes, with the new arm_relevant output described beside it. Co-Authored-By: Claude Sonnet 5.5 <[email protected]>
|
Superseded by #704: the same tree as one commit whose message names the six docs/ci.md sentences it removes. Branch kept. |
…as two exact variants (#704) Main-side half of the arm PR leg. The GKE PR workflow runs main's admission rules and main's docs-only classifier against a pull request's workflows (pull_request_target), so a workflow change that adds a job can only pass once main already knows that job's exact shape. This change teaches main both halves, ahead of the workflow change itself: - scripts/ci_docs_only.py: the classifier also answers whether a change is arm-relevant (an arm_relevant output). - scripts/ci_preflight.py: the admission rules accept exactly the two new shapes (the optional build-arm job with its pinned label and outputs, and the matching admit/legs/result checks); any other added job in the privileged workflow is still rejected. - Tests for both, written red-first, plus the docs/ci.md rows. Security: the new rules admit only those exact shapes; no wider pattern lets a later pull request add other jobs to the privileged workflow. Known limit (follow-up): the admission entry does not yet check env, shell, working-directory or continue-on-error on build and build-debug. Deferred to the workflow change that follows: the real-file arm proof tests, the actual-workflow allowlist and actionlint tests, the arm inventory and aggregate expansions, independent arm-read execution tests, and the docs rows for the build-arm leg itself. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Replaces #702 as one commit whose message names the six docs/ci.md sentences it removes (the docs-revert check judges each commit by its own message). Same tree as #702. ## Commits - 5a53e8a ci: teach main's GKE admission rules the arm leg of the PR workflow, as two exact variants The PR workflow's admit job runs main's admission rules on the pull request's own gke-pr.yml before any privileged leg runs. A PR that adds the arm64 leg (a build-arm job, an arm_relevant admit output, and matching legs and result checks) is refused by those rules, and the base checker's ci_docs_only.py does not publish arm_relevant, so the new result check would also fail with an unanswered classification. This change is the main-side half of that PR and does not touch gke-pr.yml: - ci_docs_only.py publishes arm_relevant (true for sources, build inputs and test scripts and for any path it cannot classify; false for established docs-only and .github-only changes), with its tests. - ci_preflight.py admits exactly two shapes of gke-pr.yml. The shape that has no build-arm job is checked by the unchanged rules. The shape that defines build-arm is checked by the full new rule set: the job inventory, the arm_relevant output, the pinned arm64 label, the path policy, the exact setup and Debug steps, and the complete legs and result gate bodies. The two sets are selected together by the job definition and never mixed, so a hybrid, a second arm job, an extra job in either shape, another label, or a container, services, credentials or timeout on the arm job match neither shape and are refused. In the arm shape, continue-on-error anywhere in the arm job and env, shell or working-directory on any arm step are refused too. - test_ci_preflight.py runs the real rules against main's current workflow and the same workflow with the arm leg applied, and asserts the exact findings of each refusal control. - docs/ci.md documents the classifier and both admission variants without changing the file's line count. Its rewritten sentences are named here, as the documentation check for unnamed sentence deletions (introduced by f85ce18) requires: "First merge the checker or allow-table change while the existing workflows pass." and "Then merge the workflow change that uses the newly admitted rule or label." and "Rule and table changes land in two steps." and "This order prevents one pull request from changing both sides of admission." are now one sentence (rule and table changes land first while existing workflows pass, workflow changes follow, so one PR cannot change both sides of admission); "The classifier uses exact `.github/` and `docs/` prefixes, so lookalike directories and the classifier itself cannot create a skip." and "Renames contribute both names; empty, missing, malformed, truncated or unreadable lists are neither class." are folded into the shorter paragraph that describes the path file and the exact prefixes, with the new arm_relevant output described beside it. Co-Authored-By: Claude Sonnet 5.5 <[email protected]> - 62fd39f Merge main (c3d3159) into the admission-rule branch The strict ruleset requires the branch to contain current main. The only change main gained since the branch base is README.md; no file of this branch is touched by the merge. Co-Authored-By: Claude Sonnet 5.5 <[email protected]> ## Merge note Merged on the engine merge gate for this exact head. pineforge/verify: CARRY (preflight and docs on the head tree 416b004, captured after the head commit; the other six steps carried from the full verify of tree 0ba6e76, all passed). pineforge/parity: CARRY of the full parity sweep of 0ba6e76 paired with codegen main; the head delta is one documentation file (README.md, from the main merge). Sweep result: comparable 7989, coverage lost 0, both bands +0/-0, hard regressions 0, hard moves 0, leavers 0. Co-Authored-By: Claude Opus 5.5 <[email protected]>
Main-side half of the arm PR leg. The GKE PR workflow runs main's admission rules and main's docs-only classifier against a pull request's workflows (pull_request_target), so a workflow change that adds a job can only pass once main already knows that job's exact shape. This change teaches main both halves, ahead of the workflow change itself:
Security: the new rules admit only those exact shapes; no wider pattern lets a later pull request add other jobs to the privileged workflow.
Known limit (follow-up): the admission entry does not yet check env, shell, working-directory or continue-on-error on build and build-debug.
Deferred to the workflow change that follows: the real-file arm proof tests, the actual-workflow allowlist and actionlint tests, the arm inventory and aggregate expansions, independent arm-read execution tests, and the docs rows for the build-arm leg itself.
🤖 Generated with Claude Code