Repository navigation
Guard governance lock pins and verify authoritative SonarCloud receipts - #66
Merged
Merged
Conversation
…eipt verifier Co-authored-by: arena-agent <297053741+arena-agent@users.noreply.github.com>
Contributor
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Lock-pin representation check scripts/check-lock-pin-contract.sh, tests/lock-pin-contract.sh, .github/workflows/secret-scanner-self-test.yml |
The checker validates supported lock schemas and dependency records. Contract tests cover valid and invalid representations, and the workflow runs those tests. |
SonarCloud result verification actions/sonarcloud-result-check/action.yml, actions/sonarcloud-result-check/check.sh |
The composite action passes its inputs and token to a script. The script polls check runs, selects the highest-ID matching official SonarCloud run, and reports its recognised conclusion. |
Verifier regression checks and governance guidance actions/sonarcloud-result-check/test.sh, .github/workflows/secret-scanner-self-test.yml, docs/GOVERNANCE-AND-SONAR-REPAIR.adoc |
Mocked API tests cover accepted and rejected results. The workflow checks script syntax, parses the action YAML and runs the tests. The documentation records the checks’ criteria and limits. |
Estimated code review effort: 3 (Moderate) | ~20 minutes
Change: Feature
Sequence Diagram(s)
sequenceDiagram
participant Workflow as GitHub Actions workflow
participant Action as SonarCloud composite action
participant Checker as check.sh
participant API as GitHub Checks API
Workflow->>Action: Run result verification
Action->>Checker: Pass repository, SHA, wait limit and token
Checker->>API: Poll paginated check runs
API-->>Checker: Return check-run data
Checker-->>Action: Return exit status and result
Action-->>Workflow: Expose run ID and conclusion
Possibly related issues
- Issue 63 requests a general readiness evidence checker. This pull request verifies one required check, the official SonarCloud result, but does not implement the general checker.
Possibly related PRs
- hyperpolymath/cicd-suite#57: This PR changes a theatre gate pin; this pull request adds a checker for the lock-record ref representation rule addressed by that pin change.
Merge Risk: 
7dcd0
-
This PR adds a lock-pin format check and a SonarCloud result verifier, with mocked tests and documentation. No concrete defect was established. The PR description says a hosted control run is still required before the yq-dependent lock fixtures can be treated as verified.
Pre-merge checks: 
-
Check name Status Explanation Resolution Docstring Coverage Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat… Write docstrings for the functions missing them to satisfy the coverage threshold. -
Check name Status Explanation Title check The title clearly and concisely identifies both main changes: governance lock-pin protection and authoritative SonarCloud receipt verification. Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Description check The description directly matches the changeset. It covers the lock-pin regression guard, the SonarCloud result verifier, rejection rules, test evidence, and rollout boundaries.
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 2 functions across 4 files. (3 skipped: 3 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
-
Finishing Touches
-
Generate docstrings
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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.
hyperpolymath
marked this pull request as ready for review
October 31, 2026 10:52
|
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
hyperpolymath
added a commit
to metadatastician/standards
that referenced
this pull request
Oct 31, 2026
… pins, floating helper checkouts (#1381) ## What this is A narrowly scoped repair of the **shared governance startup machinery** in `metadatastician/standards`, plus the controls that stop each repaired class from being recommitted. It does **not** repin any consumer, does **not** change any lock record, and does **not** adopt the newer policy set. Design note with evidence, migration and rollback: `docs/GOVERNANCE-STARTUP-REPAIR.adoc`. ## The measured failure (not a hypothesis) `metadatastician/svalinn`, pinned to standards `fad242d3`, run [40108154356](https://github.com/metadatastician/svalinn/actions/runs/40108154356): **12 governance jobs, one step each — `Set up job` — all failing**, no policy step executed. The annotation: ``` Unable to resolve action `actions/checkout@3d3c42e5…`: invalid lockfile pin. Run `gh actions-lock` to update the lockfile ``` The record at `fad242d3` had `ref: '<the pin's own SHA>'`. The lock consulted for a reusable workflow's dependencies is the **defining** repository's, so no caller could fix this from its own repository. **No duplicate fix here.** `main` already records `ref: v7.0.1` with the matching `sha1-` commit (#1271's nine, #1290's four), and its lock passes the new contract with **0 violations**. Nothing in `actions.lock` changes in this PR. Consumer census (contract violations at each consumer's pinned standards commit, plus the observed run): | consumer | pin | violations | observed | |---|---|---|---| | svalinn | `fad242d3` | 11 | 12/12 jobs dead at `Set up job` | | odraz | `da2c748a` | 13 | `startup_failure`; later runs `failure`, zero jobs | | cadastra | `e3b9929c` | 14 | `failure`, zero jobs | | lintel | `e16a2317` | 5 | zero jobs | | burble, marid | `01c9b0c9` | 0 | green, 21 jobs with 6–13 steps (policy executed) | | **main** | `1725bebd` | 0 | — | ## What was still unguarded, and what changes 1. **No lock-representation control** (#1290 AC3, which nothing satisfied). Adds `scripts/check-lock-pin-contract.sh`, adopted from `hyperpolymath/cicd-suite#66` — every rule of that guard kept — and extended as that PR's own guidance demands: **closure** (`close-lock.sh --check`) in the same gate, **commit↔key provenance conjugation**, tag-keyed records refused, and the caller-side exemption limited to exactly what #1290 states (this repository — either slug — plus a record carrying a `uses:` list, plus `ref` == the key's own SHA). Uninspectable states exit **2**, never 0. Wired into the required `uses ⊆ actions.lock` gate. 2. **A stale literal helper pin.** Three steps in `governance-reusable.yml` staged helpers from `317101e03b8f` (2026-09-14) while `main` had moved **194 files** under `scripts/` — including the orphan-pin ancestry probe (#782) and the KYAML normalisation that ended 14 phantom duplicate reports. Repaired to `ref: ${{ job.workflow_sha }}` (the reusable's own commit; the same expression the lock gate has used since #1119), with a 40-hex assertion per job because GHES resolves it empty and `ref: ''` silently follows the default branch. 3. **Floating helper checkouts.** `changelog-reusable.yml` followed `ref: main`; `readme-derive-reusable.yml` and `fork-governance-reusable.yml` defaulted their standards source inputs to `main`. All three now default to this reusable's commit; the readme-derive fetch became a `git fetch` (a SHA is not a branch) and stopped naming the pre-transfer slug. 4. **A guard for the class.** `scripts/check-standards-helper-pins.sh` admits only `job.workflow_sha` (optionally behind a documented `inputs.x ||`) or a 40-hex commit at every standards checkout, rejects `github.sha` / `github.workflow_sha` / branches / tags / bare inputs / a missing `ref`, and with `--compare` applies the path-scoped freshness predicate. Wired into Self Test against the PR base, so a helper edit is not deadlocked on its own unborn merge commit. ## Regression evidence | command | result | |---|---| | `scripts/tests/check-lock-pin-contract-test.sh` | **23/23** — historical raw-SHA rejected, corrected record accepted, exemption and its boundary, tag-keyed/missing-provenance/commit-mismatch/dangling-edge rejected, all uninspected states exit 2, **this repo's own lock passes** | | `scripts/tests/check-standards-helper-pins-test.sh` | **18/18** — branch/tag/HEAD/github.sha/github.workflow_sha/bare-input/missing-ref rejected; `job.workflow_sha` and `inputs.x \|\| job.workflow_sha` accepted; freshness path-scoped (stale path red, unchanged path green) | | `scripts/tests/governance-canary-test.sh` | **11/11** — STARTUP clean with both measured killers caught as negative controls; POLICY executes the shipped gates on a digest-pinned canary consumer, passing clean and failing when a defect is seeded | | `scripts/tests/governance-canary-test.sh fad242d` | **exit 1** — the canary refuses to certify the commit that broke svalinn | | `scripts/check-lock-pin-contract.sh` | 0 violations, 2 documented exemptions | | `scripts/close-lock.sh --check .github/workflows/actions.lock` | closed | | `scripts/check-standards-helper-pins.sh .github/workflows --compare HEAD` | 34 sites, 0 failures (was: 3 stale literals, 2 floating, 1 unscanned clone) | **No regression in the existing suite.** All 35 pre-existing tests that touch workflows/locks were run on this branch and on `base` in the same sandbox: the result lists are **identical**, differing only by the two new passing tests. (The three suites that report failures do so identically at base — a sandbox without mikefarah `yq`; they are green on the runner.) **Locks and closure**: no `uses:` ref changed, so no regeneration was required and `.github/workflows/actions.lock` is byte-identical to `main`'s. Coverage (`.githooks/validate-actions-lock.sh`: 28 SHA-pinned refs among 30 lock keys), the contract guard and `close-lock.sh --check` all run in the canary and in CI. ## Migration (ordered; not compressible) 1. Merge this PR — the merge commit is the first source carrying the guards. 2. Publish, then test the exact commit: `bash scripts/tests/governance-canary-test.sh <merge-sha>`. Red ⇒ do not repin. 3. Repin each consumer's `governance.yml` to that **40-hex commit** (a tag or branch re-creates the class). 4. Check the consumer's own lock and allowlist admission afterwards; neither is changed here. 5. Re-measure the canary consumers (svalinn/odraz/cadastra/lintel are startup-dead; burble/marid are the green controls) and record the run URL. One step per job means still-dead startup, not a policy result. 6. Only then consider `75bd5994` — it is **454 commits** ahead, 300 files, and adds four required contexts (UUID v7/v8, JS runtimes ×2): a reviewed policy upgrade, not a startup repair. Consumers passing `canon-ref:` / `standards-ref:` with a branch should change to a 40-hex commit or drop the input. ## Rollback Revert the PR. No lock record and no `uses:` ref changed, so a revert restores the previous behaviour exactly — including, deliberately, the stale literals and the floating checkouts. Per-site rollback is itemised in the design note; do not relax a guard's rules to keep it green. ## Coordination / known limits * **standards#1290** AC3 implemented (comment to follow); AC4 needs the consumer owners and is migration step 5. * **cicd-suite PR66** guard and guidance adopted and cited; the closure and provenance halves that PR explicitly did not claim are added here. A passing fixture is not proof of a working consumer job — hence the canary and the publish-before-repin ordering. * **Hypatia / gitbot-fleet / cyo**: asks recorded in the design note. The lock-sync automation must preserve reviewed tag refs and composite closure rather than rewriting refs to raw SHAs, and must not edit a failing guard. * Not covered, stated plainly: remote reachability, tag movement, allowlist admission, policy validity, and `git clone …standards.git` inside a `run:` body (a review question, not a silent pass). * **No signing key is available in this environment**, so these commits are unsigned — flagged for the merge/re-sign step rather than hidden. Fixes the startup half of #1290. Refs #1271, #1281, #946, #1119, #963.# --------- Signed-off-by: Jonathan D.A. Jewell <6759885+hyperpolymath@users.noreply.github.com> Co-authored-by: arena-agent <297053741+arena-agent@users.noreply.github.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: Arena Agent <agent@arena.ai>
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.
Scope
Evidence
Rollout boundaries
No consumer edits/deployments. No required checks disabled or bypassed. Canonical standards main already corrects the lock record; a blind 454-commit repin is not policy-neutral. Publish and test suite main before any consumer pins. Hosted controls and final external checks verified. Ready for owner merge; no merge performed by this session.