Skip to content

fix: constrain assessment evidence references - #24

Merged
loadinglucian merged 7 commits into
mainfrom
fix/structured-assessment-references
Jul 28, 2026
Merged

fix: constrain assessment evidence references#24
loadinglucian merged 7 commits into
mainfrom
fix/structured-assessment-references

Conversation

@loadinglucian

@loadinglucian loadinglucian commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Summary

  • require completion-criterion evidence to use machine-resolvable plan references
  • keep model schemas synchronized with deterministic admission invariants
  • validate the deterministic evidence-state shape before any automated protected-path exception
  • require a GitHub OIDC/Sigstore attestation from the protected watcher, bound to the exact source commit, workflow run, action key, manifest, and file digest
  • wait only for the required no-change PR safety checks, preventing the bounded coordinator from timing out on optional native builds
  • suppress only the watcher’s own evidence-state commit from future wakeups while preserving wakeups for code and external-evidence changes
  • make no-change recording safe to retry without duplicate PRs or non-fast-forward branch failures
  • align the implementation merge timeout with the bounded native build and notify the owner on coordination failures
  • document the organization Actions PR setting and preserved owner-only control boundary

Root causes

  1. The model returned explanatory prose where admission requires evidence references such as evidence[0].
  2. The watcher no-change path writes a protected evidence-state file, but the generic owner-review gate had no securely authenticated automation exception.
  3. A shared github-actions[bot] identity cannot prove which workflow produced a PR.

Verification

  • ./scripts/test.sh (15 tests)
  • exact-head A00-A20 parity passed against php-bin 5cae3543e1f11450ddcf4c2ade2d6351efd19d27 and mise-php b0e3c11ca08ea82c47b87be57fdee56842d31586
  • report digest: sha256:e95586e45786af918788b8cb658f4cbeff62e7cef64af0b7f0bcc938839f5d62
  • live watcher rerun required after merge

Safety

Admission remains fail closed. The evidence-state exception requires a same-repository bot PR, an exact protected watcher run ID, an in-progress run on the exact main base SHA, a direct-parent commit changing only the deterministic state file, the reviewed seven healthy captures, and a verified GitHub OIDC/Sigstore attestation from the protected watcher workflow. All other protected changes still require exact-head owner approval.

@coderabbitai

coderabbitai Bot commented Jul 28, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The PR tightens maintenance-plan schemas and evidence validation, adds deterministic evidence-state checks, and introduces a guarded protected-controls workflow exception for trusted watcher updates. Administrative evidence snapshots, repository settings, investigation guidance, and tests are updated accordingly.

Changes

Maintenance evidence controls

Layer / File(s) Summary
Evidence contracts and validation
.github/codex/maintenance/investigation.md, maintenance/control.py, schemas/maintenance-plan.schema.json, scripts/validate-structured-output-schemas, tests/test_maintenance.py
Evidence references, required checks, repository counts, criterion IDs, and evidence-state records now follow strict schemas and validation rules, with corresponding tests.
Protected evidence approval path
.github/workflows/protected-controls.yml, docs/repository-settings.md
Protected-controls approval accepts only narrowly validated same-repository watcher evidence updates matching the required commit, workflow, branch, author, and record conditions.
Administrative evidence snapshots
docs/admin-state/php-bin-after.json, docs/maintenance-admin-evidence.json
Snapshot timestamps and digests are refreshed, and organization workflow permission verification data is recorded.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant PullRequest
  participant ProtectedControls
  participant GitHubAPI
  participant EvidenceState
  PullRequest->>ProtectedControls: Submit evidence-state change
  ProtectedControls->>GitHubAPI: Read commit, workflow run, and last-evidence.json
  GitHubAPI-->>ProtectedControls: Return watcher metadata and evidence record
  ProtectedControls->>EvidenceState: Validate deterministic record
  EvidenceState-->>ProtectedControls: Return healthy validation result
  ProtectedControls-->>PullRequest: Allow protected-controls approval
Loading

Possibly related PRs

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title accurately summarizes the primary change: constraining assessment evidence references.
Description check ✅ Passed The description covers summary, verification, root causes, and safety, but it omits the template's security/licensing section and checkbox details.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/structured-assessment-references

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/workflows/protected-controls.yml:
- Around line 85-112: The protected-controls evidence exception currently trusts
the shared github-actions[bot] identity rather than proving the watcher workflow
produced the record. In the protected-controls validation around evidence_run,
add watcher-exclusive GitHub App credential verification and a signed, run-bound
attestation tied to the expected workflow run before accepting the evidence PR;
retain schema and run checks. In docs/repository-settings.md lines 15-19, update
the exception description so it does not claim watcher-only provenance until
this verification is enforced.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 117101b5-4a45-4be0-b38d-61034b89d4bc

📥 Commits

Reviewing files that changed from the base of the PR and between 67f5e2e and 79ca957.

📒 Files selected for processing (9)
  • .github/codex/maintenance/investigation.md
  • .github/workflows/protected-controls.yml
  • docs/admin-state/php-bin-after.json
  • docs/maintenance-admin-evidence.json
  • docs/repository-settings.md
  • maintenance/control.py
  • schemas/maintenance-plan.schema.json
  • scripts/validate-structured-output-schemas
  • tests/test_maintenance.py

Comment thread .github/workflows/protected-controls.yml
@loadinglucian
loadinglucian merged commit 64eb4b7 into main Jul 28, 2026
3 of 4 checks passed
@loadinglucian
loadinglucian deleted the fix/structured-assessment-references branch July 28, 2026 14:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant