feat(standards): cut github-workflow 1.9 with the ADR 0031 admission classifier - #232
Merged
Merged
Conversation
ADR 0031 D3: `gh-workflow admission --branch B [--since REF] [--offline]` classifies every commit in a range against the four admission classes, exits 1 listing the commits no class admits with the trailer or route each needs, and exits 0 only when every commit is admitted. The new internal/ghworkflow/admission package is split three ways so the rules are testable without a repository: the classification rules, the single `git log` invocation that feeds them, and the CLI surface. cmd/gh-workflow gains one blank import; no dispatch table is edited. `merge --pr N` now writes `Workflow-Admission: PR #N` into the merge or squash commit body, which is what turns pull-request provenance into a fact the classifier can read offline. GitHub applies `commit_message` to the merge and squash methods only, so a rebase-admitted PR carries no trailer; that limit is recorded where the trailer is built. The policy reader gains the four ADR 0031 D1 options, all optional scalars with defaults that reproduce 1.8 behavior, so a policy rendered by any earlier payload still loads. `handoff_admission` accepts only its two spellings — a typo that silently leaves the exemption on is the one outcome a consumer switching it off cannot detect. cli.DefaultVersion advances from "1.7", where it had been lagging two cuts, and scripts/build-gh-workflow.sh moves its output path and version stamp to 1.9. Refs #203, #218
…model Author 1.9 as a copy of 1.8 with the admission surfaces edited; 1.8 stays byte-immutable and moves to `retained` (ADR 0031; issues #203, #218). pr-standard.md replaces the two-class Admission section with the three branch classes, the four admission classes and their trailers, the fixed handoff path set, the mixed-commit rule, and a paragraph stating plainly that nothing runs the check for you. The orphaned `construction branch` sentence is deleted rather than defined: under D1 the integration branch is governed, which is the opposite of what an undefined construction branch implied. config.schema.json and resources/policy.toml add `integration_branch`, `release_subject_prefix`, `admission_floor`, and `handoff_admission` — four optional scalars, so the tool's bounded policy reader is unchanged and a consumer that configures none behaves exactly as under 1.8. The provider renders them and refuses a value carrying a quote or backslash rather than writing a file the tool would parse as something else. Both content budgets were paid for by displacement rather than raised, since NFR-006 and NFR-003 left 22 and 8 bytes of headroom. SKILL.md drops the lifecycle preconditions that pr-standard.md states and `ready`/`merge` enforce (70 lines / 11,997 B). The managed block drops the `Wait for CI` row — the one route that mutates nothing and costs at most one extra call when forgotten, still carried by SKILL.md's routing table — to make room for the four classes and the exempt paths that #218 criterion 3 requires it name (2,394 B at a 39-character login). The binary is rebuilt reproducibly for 1.9 and re-digested; projection, family landing pages, the standards index, and the top-level README versioned links move with the promotion. Refs #203, #218
tests/package_contract/test_github_workflow_1_9.py asserts the four classes on every surface that states them — the delivered reference, SKILL.md, and the rendered managed block, which is the one an always-injected delegated worker sees — plus both content budgets, the four defaulted options as rendered scalars, 1.8's byte immutability, and the projection and activation wiring. The classifier is proven by running the committed binary rather than by reading its bytes, because a check nobody runs is the gap #203 names. Two runs: over this repository's own history, where a clean report would disprove the control, and over a synthetic corpus where every commit is admitted, one trailer is removed, and the verdict flips. Both assert the shape from the classifier's own output — no ADR literal is pinned, since every one of those counts moves with the next commit. ADR 0031 moves to `active` on the owner's 2026-09-01 ratification, and two of its measurements are corrected in the same change: 33 of the 362 commits touch only handoff surfaces, not 66, and the derived unadmitted total moved with it; and "4 PR-admitted" confused merged pull requests with admitted commits, since none of those commits carries a trailer that only 1.9's `merge` began to write. The shipped classifier reports 353 of 362 unadmitted with this repository's release prefix configured, 362 of 362 with no policy at all. test_github_workflow_1_8.py becomes a retained-predecessor suite: the build-script pin inverts to the negative, the catalog roles flip, and the family-root navigation assertion is deleted rather than retargeted because test_catalog_roles.py derives it from the catalog for every family. The activation test advances `_BASELINE_REF` to v5.27.0 as its own comment requires of an activation commit; its `.standards/` assertions stay red until release-time reconcile, by design. Refs #203, #218
gosec G204 flagged the revision range reaching `git log` as a variable argv element. The range comes from an operator flag or the rendered policy, and without `--end-of-options` a value beginning with `-` is parsed by git as an option rather than as a revision. The separator closes that, and the annotated suppression records why the remaining construction is safe. The 1.9 binary is rebuilt reproducibly and re-digested for the changed source. Refs #203
…time The shipped binary's `ghapi.ValidateLogin` rejects a login containing `--`, and it is the function that reads the rendered `policy.toml`. The payload's render provider did not, so `organization = "a--b"` produced a file reconcile reported as a success and every subcommand then refused to load — the configuration failed where it was read rather than where it was written. 1.9's config.schema.json now carries the whole login grammar as a pattern, which is the single source the control plane validates against, and the provider keeps the same rule as a second layer so the failure names the grammar in prose. A contract-test case asserts both boundaries agree on the same accepted and refused spellings. Refs #203
A security read of the merge path found three ways an author could shape the commit the tool signs off on, now closed at both ends. The merge payload carries tool-owned text only. GitHub composes its default commit title and body from the pull request itself, so the previous change — which sent a body and left the title to GitHub — still let an author drive every terminal that prints `git log` through the subject. `merge` now sends both fields: a sanitized subject with the CR, LF, ESC, other C0/DEL, and Unicode bidi override and isolate characters removed, and a body that is a blank line and the trailer alone. The author's body is never echoed, so it cannot carry a forged `Workflow-Admission: PR #999` line into history. The classifier reads only the message's final paragraph, and only when that whole paragraph is trailer-shaped, which is git's own rule for a trailer block. A declaration written into the middle of a body admits nothing, and a body ending in prose has no trailer at all — so `git interpret-trailers` and this classifier cannot disagree about the same commit. Exactly one occurrence is required; a repeated value is now a conflict even when identical, because with two lines present which one wins is no longer decided here. Online, a `PR #N` trailer is authoritative only when pull request N is merged and its own merge commit IS the commit being classified. Without that binding, one genuinely merged pull request would admit every commit that named it. The 1.9 binary is rebuilt reproducibly and re-digested. Refs #203, #218
staticcheck ST1018 refuses a Go string literal containing raw Unicode format characters, and the sanitizer case needs U+202E and U+2069 by construction. The escape spellings are the same bytes and are legible in a review diff, which raw overrides are not. Refs #203
…ses it The schema pattern refuses `a--b` during option resolution, before any provider runs, so the render path raises PackageContractError rather than the provider's ValueError. The case now asserts both layers where each actually fires: the outer one through option resolution, and the inner one by handing the provider an effective config the schema would have rejected. Refs #203
…he 1.9 README markdownlint MD049 pins emphasis to underscores; the 1.9 "What changed" section used asterisks. Re-digested for the changed byte. Refs #203
…ditions markdownlint MD049 pins emphasis to underscores. Neither file is a payload, so nothing is re-digested. Refs #203
Three findings from the pre-release security read of the 1.9 classifier. F1: `git log --name-only` collapses a rename to its destination, so a commit declaring `Workflow-Admission: handoff` while `git mv`-ing any file into `docs/handoff/` presented only exempt paths and was admitted — an arbitrary change leaving governance through the one exemption the standard grants. The read is now `-z --name-status --no-renames`, which reports both sides of every rename; `-z` also suppresses git's C-quoting, which had been misreporting a genuine handoff document whose name carries a non-ASCII byte as mixed. Paths are compared as exact bytes, so a leading space no longer trims into an exempt path. F2: the revision argument now ends in a `--` pathspec terminator — `--branch docs` in a repository holding a `docs/` directory logged that directory's commits and exited 0 — and a range resolving to zero commits is refused with GHW-ADMISSION-EMPTY-RANGE rather than reported clean. F3: `admission_floor` is verified to be an ancestor of the classified branch (GHW-ADMISSION-FLOOR-UNRELATED otherwise), and the report states how many commits the floor excluded. Refs #203, #218
…release does not enforce F4: `integration_branch`, `release_subject_prefix`, and `admission_floor` rejected a quote and a backslash but accepted a newline, a carriage return, and unbounded length. A newline renders a `policy.toml` that the tool's bounded reader refuses on every Load — the same render/load asymmetry 1.9 closed for `organization`, reintroduced by the options 1.9 itself added. The schema pins maxLength on all three and a ref-name pattern on the two that name revisions; the provider accepts only bounded printable single-line text. The patterns end in `(?![\s\S])` rather than `$`, because `$` also matches before a trailing newline, which is exactly the value being refused. F5: the `release` class and `release_subject_prefix` admit on the author's word alone — no path, branch, or version check. That is a documented limitation of the model rather than enforcement, and pr-standard.md and ADR 0031's consequences now say so. The contract tests add the F1 rename-into-handoff case over the shipped binary and the F4 refusal at both boundaries. Payload digests move in the reseal commit that follows. Refs #203, #218
The classifier fix changes the Go source, so the committed `gh-workflow` bytes are rebuilt through `scripts/build-gh-workflow.sh` and every digest that moved is refreshed: the four changed payload resources and artifacts, the payload aggregate in `standards/github-workflow/standard.toml`, and the same aggregate in `catalogs/5.toml`. 1.9 is unreleased, so the bytes are edited in place. New binary digest: sha256:7b3951ab3e87b53225617bb0637092571ac1f32822d802667b69a330017e1cf2 New payload aggregate: sha256:2c9de8845e32bf93804b40867dc7f2bdb92ab17f596750e468befe663b40e5e3 Refs #203, #218
chrisdpurcell
marked this pull request as ready for review
September 1, 2026 13:31
This was referenced Sep 1, 2026
Closed
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
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.
Summary
github-workflow 1.9 (1.8 retained, byte-immutable) implements ADR 0031, ratified by the owner on 2026-09-01:
integration_branch, and topic branches; a commit on a governed branch is admitted by exactly one ofWorkflow-Admission: T0,PR #N,handoff, orrelease. Four newpolicy.tomloptions (integration_branch,release_subject_prefix,admission_floor,handoff_admission), all double-quoted scalars so the bounded parser is unchanged (github-workflow: the PR-admission rule ships with no enforcement mechanism #203).docs/handoff/**,docs/STATUS.md,docs/TODO.mdadmits directly with thehandofftrailer; a mixed commit is governed by the PR rule; the managed instruction block states both.gh-workflow admission --branch B [--since REF] [--offline](the eleventh subcommand) classifies every commit in a range and exits nonzero listing the unadmitted ones. Run over this repository's own history (9c47907f..3bda3cf4): 362 commits, 353 unadmitted withrelease_subject_prefixset — non-vacuous, and pinned by a contract test that asserts shape from the classifier's own output plus a synthetic negative case.mergenow writes thePR #Ntrailer into a tool-owned squash/merge message: title sanitized, PR body never echoed, author trailer keys stripped, trailer-only final paragraph; online, the trailer is authoritative only when the merged PR's merge SHA equals the commit.--end-of-optionsbounds the classifier's git range;organizationrefuses a doubled hyphen at render time so the Python renderer andghapi.ValidateLoginagree.activewith its corpus measurements corrected (33 handoff-only, not 66).git log -z --name-status --no-renameswith exact-byte path parsing (agit mvintodocs/handoff/previously admitted as handoff; a non-ASCII handoff path was refused), terminates the range with--and refuses an empty range (GHW-ADMISSION-EMPTY-RANGE), validatesadmission_floorancestry before reading history and reports the excluded count, and bounds the three string options (printable single-line, ≤ 255, ref-name pattern) at both the schema and the provider. The release class is documented as declared-but-not-enforced.Two judgment calls for review: the managed block dropped the "Wait for CI" routing row (the only row that mutates nothing; still in SKILL.md) to stay inside its byte cap rather than raise the NFR; and duplicate
Workflow-Admissiontrailers are a conflict even when identical.Not in this cut: the repository-side D4 migration of the hand-written carve-outs (after release-time reconcile), and a managed CI job (deferred to 1.10 per the ADR). A rebase-merged PR carries no trailer (GitHub applies
commit_messageto merge/squash only) — documented.Reconcile-lag reds expected until release prep: the three
test_current_catalog_activation.pyassertions that.standards/{catalog,lock}.tomlstill resolve 1.8.Governing work
Final: #203
#218 ships in the same cut and is closed separately with this PR as its evidence.
Acceptance coverage
commits > 300,unadmitted > 200); negative case (all four classes admitted → 0; one trailer removed → 1); targets the branch where work lands (--branchon a non-checked-out branch, Go case); pinned bytest_github_workflow_1_9.py.GHW-ADMISSION-HANDOFF-MIXED); AC3 block text names the exemption and the mixed rule (assertion on_block_body()); AC4 1.8 byte-immutable (git diff --stat origin/testing -- …/1.8empty), validators green.Verification
standards validate-packages,validate-graph,render-catalog --check,sync-payload-projection --check→ 0; ruff format/check → 0; basedpyright → 0; markdownlint + Prettier over changed docs → 0.rexec -- make go-check→ 0,go-verify-binary: the sealed 1.9 binary matches a rebuild (re-run by the orchestrator on the rebased tree: exit 0, 0 issues).check-release --baseline v5.27.0→ onlyPC-RELEASE-PROJECTION(mid-train by design).test_github_workflow_1_9.py(19),_1_8.py,test_repository_hygiene.py→ 34 passed; activation → 3 reconcile-lag reds quoted.56b028fb) with a rebuilt candidate wheel: validators → 0;rexec -- make go-check→ binary7b3951ab…matches a rebuild, 0 issues;go vet ./...→ 0; remote pytest over_1_9,_1_8, hygiene, catalog roles, release consistency, provider registry, composition, module-loading guard → 169 passed, 82 skipped.