ci(k9): make a failing K9 gate legible (follow-up to #1143) - #1144
Merged
Merged
Conversation
… suite (#1058, D173) Ruling D173: "K9 needs a Nickel contract, not an ABNF." Implements it. Standards - 1-formats/k9/spec/K9-CONTRACT-SPEC.adoc v1.0.0 — normative. File envelope, dialect rules, versioned contract, closed leash set, default-deny capability model, the five Hunt preconditions, signature semantics, four conformance layers. 23 rule ids, indexed in Appendix A. - spec/contract/k9_contract.ncl — the machine-readable contract. - SPEC.adoc — points at the contract as normative; names the component/repo pedigree collision instead of leaving two shapes called "pedigree". Deliberately no k9.abnf: a component body IS a Nickel term, so a whole-file grammar would be a drifting restatement of a language we do not own. The envelope is specified as three octets plus a first-significant-line table. Two distinctions made load-bearing rather than prose - Presence is not verification (10.4): the Hunt `signature` precondition is satisfiable by 'Verified only. 'Present_Unverified is false. No input turns "no verifier ran" into "verified". - A flag is a request that must be paid for (8.4): allow_network/fs_write/ subprocess now REQUIRE net.fetch/fs.write/process.spawn in the grant. A component asking for the network while granting itself nothing is invalid. Hunt is otherwise unchanged: all five preconditions, always, no subset. Validators aligned - tools/k9-validate.sh — canonical. Layered L0 envelope / L1 structural / L2 Nickel / L3 crypto, so a lexical check cannot report a higher layer's authority. A check that could not run is SKIPPED, never a pass; --strict fails the run rather than reporting green over nothing. - .githooks/validate-k9.sh — was its own format: it grepped for a line beginning `contract`, which 0 of 30 tracked K9 files have, so it exited 1 with 30 errors on a clean tree. Now delegates and owns only commit policy. - .githooks/validate-lint-format.sh — excludes *.k9.ncl from the bare nickel typecheck, matching ci-pipeline.yml (2 staged .ncl in, 1 out). - k9-contractile.yml — installs Nickel pinned+sha256 (same pin as ci-pipeline.yml) and runs --self-test, the fixtures --strict, and the corpus. Fixtures: 5 positive, 21 negative. Each negative names its rule and layer and the runner asserts it was rejected BY that rule AT that layer, so a fixture cannot pass for the wrong reason. 20 of 23 rules have a control. Migration: spec/MIGRATION-1058.adoc. Baseline measured — 30 tracked K9 files, 5 conforming, 25 not. .machine_readable/k9-contract-debt.txt grandfathers them shrink-only: fixing a file forces its entry out, and editing a listed file removes its protection. Not yet run: L2. No nickel binary is obtainable in the preparation sandbox (release-asset host TLS-refused, no cargo to build the codeload tarball), so this commit's L2 result comes from the workflow_dispatch run of the job added here. Co-authored-by: arena-agent <297053741+arena-agent@users.noreply.github.com>
The three K9 steps now tee into $GITHUB_STEP_SUMMARY. Step conclusions were already readable through the check-run API; the step bodies were not, and the log blob host is not reachable from every machine that needs the result. The PR now carries the verdict itself. No behavioural change to the gates: each step still exits with the validator's own status via PIPESTATUS[0], and the run scripts drop -e so a failing validator reaches the tee instead of aborting before it. Co-authored-by: arena-agent <297053741+arena-agent@users.noreply.github.com>
The step summaries written last commit turned out not to be readable: the check-run API returns an empty output.summary for Actions jobs, and the log blob hosts are unreachable from the sandbox that needs the result. So the verdict is now posted to the PR itself, edited in place on re-runs. Adds pull-requests:write at the job level (the workflow-level grant stays contents:read) and runs with always(), because a failing gate is exactly when the detail is needed. Co-authored-by: arena-agent <297053741+arena-agent@users.noreply.github.com>
…uld not fail
The first CI run of this branch produced the first Nickel verdict this corpus
has ever had, and it found two defects — both in this PR's own code.
1. `Record` is not a Nickel type. Component's three open fields named it, so
k9_contract.ncl failed with `unbound identifier` and every L2 verdict
downstream was void. All 5 positive controls failed. Now `{ _ : Any }`.
2. Negative-control attribution matched the rule id in the FILENAME. A control
is named `L2-K9-N001-…`, and the human finding line echoes the path, so
grepping that output for `K9-N001` succeeded no matter which rule fired.
With the contract broken, both L2 controls were rejected by K9-N002 and
both still reported `ok`. The suite exists to catch gates that cannot fire;
this was one. Attribution now reads the structured findings and requires an
`error`-severity finding whose rule AND layer both match the filename.
A broken contract is also called out by name rather than reported as "wrong
rule": when K9-N002 fires, no L2 control proved anything.
self-test gains a block asserting the predicate itself — a rule id present only
in a path is not attributed, and a skipped finding cannot satisfy a control.
24 assertions become 29. The first draft of that block asserted E001 fires
once; it fires twice (bad magic also leaves the body unclaimed), so the
well-formedness count is compared rather than fixed at 1.
Still unverified locally: L2. No nickel binary is obtainable in this sandbox,
so the fixtures' L2 half is asserted by the workflow run of this commit.
Co-authored-by: arena-agent <297053741+arena-agent@users.noreply.github.com>
The publish step failed on the last run, so the PR comment stayed pinned to the
run before it. `gh pr view --json comments` returns API urls, not web urls, so
stripping a github.com prefix left a string that was not a resource path; the
id is now taken off the end and PATCHed as issues/comments/{id}. `(.body //
"")` guards a null body on a deleted comment.
Failing fixtures and corpus steps now also emit `::error`. Check-run
annotations are readable from the check-run API, which unlike the log blob host
is reachable from the sandbox diagnosing the run.
Co-authored-by: arena-agent <297053741+arena-agent@users.noreply.github.com>
Two runs were spent on an update path that failed and could not be read back. Creating the comment is the one branch observed to complete, so it is now the only branch; stacked reports are cheaper than no result. Also statically audited the contract for the identifier class that broke it: every type-position identifier is now either defined in the file or a Nickel builtin, and every std.* function it calls is one the estate's CI-passing .ncl already uses with the same argument order. Co-authored-by: arena-agent <297053741+arena-agent@users.noreply.github.com>
Two follow-ups to what landed in #1143, both about being able to READ a failure rather than about the gate: - A whole-report ::error never reached the check-run API. Annotations are now emitted per FAIL/ERROR line, capped at 20, each short enough to survive. - The publish step tried to PATCH its previous comment and failed, pinning the PR to a stale report for two runs. It now always posts; stacked reports are cheaper than no result. The K9 fixtures step is red on main as of b3075e0 and this is the change that makes the reason legible. Co-authored-by: arena-agent <297053741+arena-agent@users.noreply.github.com>
Contributor
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 30 minutes. View limit detailsLimit details: You’ve used the included review currently available. Review configuration: ⚙️ Run configuration
📒 Files selected for processing (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
hyperpolymath
marked this pull request as ready for review
October 4, 2026 01:38
Signed-off-by: Jonathan D.A. Jewell <6759885+hyperpolymath@users.noreply.github.com>
hyperpolymath
enabled auto-merge (squash)
October 4, 2026 01:39
Contributor
K9 contract conformancerun https://github.com/hyperpolymath/standards/actions/runs/37168693062 K9 contract self-testK9 conformance fixtures |
hyperpolymath
disabled auto-merge
October 4, 2026 01:40
|
hyperpolymath
pushed a commit
that referenced
this pull request
Oct 4, 2026
main is red on the K9 gate. #1144 merged at 9c971da, one commit before this fix, so `{ _ : Any }` is what shipped — and `Any` is not a Nickel type. The annotations #1144 added name it: `unbound identifier 'Any'` at k9_contract.ncl:422:20. The dynamic type is `Dyn`. `Record` was wrong before it; both names were asserted from memory in a sandbox with no nickel binary, and both voided every L2 verdict downstream, because a contract that does not typecheck cannot judge anything. The static audit meant to catch the first one did not: it scanned `| T` positions only, and `{ _ : Any }` puts its type after a colon. It now also scans `_ : T` and `Array T`, and its builtin whitelist is narrowed to the four types this repo's CI-passing .ncl actually uses. It reports exactly one identifier it cannot evidence from the repo: `Dyn`, lines 428-430. k9-contractile.yml gains a `K9 normative contract typecheck` step ahead of the fixtures. `nickel typecheck` stops at the first error, and a broken contract presents as five non-conforming positive controls rather than one broken contract — that misdirection cost two runs to see through. Unverified here: whether `Dyn` typechecks. Nothing else in this repo uses it, so the workflow run of this commit is the evidence. Co-authored-by: arena-agent <297053741+arena-agent@users.noreply.github.com>
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
2 participants
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.
Follow-up to #1143. The K9 fixtures step is red on
mainas ofb3075e0and nothing about that is currently readable: the log blob host is unreachable from the sandbox, the step summary is not exposed by the check-run API, and the report comment was pinned to a stale run because the publish step failed trying to PATCH it.This PR changes no gate. It changes only whether a failure can be read.
::errornever reachedcheck-runs/{id}/annotations. Annotations are now emitted perFAIL/ERRORline (capped at 20), each short enough to survive the parser.Why this matters beyond convenience
The first CI run of #1143 was the first time any tool in this estate ran Nickel over a K9 file, and it immediately found two defects in that PR's own code:
k9_contract.nclnamed aRecordtype that does not exist in Nickel. The contract failed withunbound identifier, so every L2 verdict downstream was void and all 5 positive controls failed.L2-K9-N001-…. So the assertion "rejected by K9-N001" succeeded no matter which rule fired. With the contract broken, both L2 controls were rejected byK9-N002and both still printedok. A gate that cannot fire is the defect class rsr-antipattern.yml: BUILTIN_GLOBS bash block stranded outside Python heredoc (exit 127) #49 and Hypatia dogfooding job red estate-wide — unresolvable setup-beam pins (companion to hypatia-side fix) #64 established; this suite was built to prevent it and contained one.Both are fixed in #1143. What is not yet established is whether the fixtures pass at L2 with the contract repaired — the step is still red, and this PR is how that gets read.
self-testnow asserts the attribution predicate itself (29 assertions): a rule id present only in a filename is not attributed, and a skipped finding cannot satisfy a control.