ci(strict21): add canonical policy + preflight gate#96
Closed
Prekzursil wants to merge 1 commit intomainfrom
Closed
ci(strict21): add canonical policy + preflight gate#96Prekzursil wants to merge 1 commit intomainfrom
Prekzursil wants to merge 1 commit intomainfrom
Conversation
Co-authored-by: Codex <noreply@openai.com>
|
Unable to trigger custom agent "Code Reviewer". You have run out of credits 😔 |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (3)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Review Summary by QodoAdd strict-21 preflight validation with canonical policy
WalkthroughsDescription• Add strict-21 preflight validation script comparing canonical contexts • Define canonical branch protection policy with 24 required status checks • Implement GitHub Actions workflow for automated preflight checks on PRs • Validate branch protection and emitted check-runs against canonical contexts Diagramflowchart LR
A["Canonical Contexts List"] -->|"Compare"| B["Branch Protection Policy"]
A -->|"Compare"| C["Emitted Check-Runs"]
B -->|"Evaluate"| D["Preflight Script"]
C -->|"Evaluate"| D
D -->|"Generate"| E["JSON Report"]
D -->|"Generate"| F["Markdown Report"]
G["GitHub Actions Workflow"] -->|"Trigger"| D
File Changes1. scripts/strict21_preflight.py
|
Code Review by Qodo
1. strict21_preflight.py lacks tests
|
|
|
This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation. |
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
+20
to
+39
| - name: Run strict-21 preflight | ||
| env: | ||
| GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||
| STAMP: ${{ github.event.pull_request.number || github.run_id }} | ||
| run: | | ||
| set -euo pipefail | ||
| mkdir -p .tmp/strict21-preflight | ||
| python3 scripts/strict21_preflight.py \ | ||
| --repo "${GITHUB_REPOSITORY}" \ | ||
| --branch main \ | ||
| --ref "${GITHUB_SHA}" \ | ||
| --out-json ".tmp/strict21-preflight/preflight.json" \ | ||
| --out-md ".tmp/strict21-preflight/preflight.md" | ||
| cat .tmp/strict21-preflight/preflight.md | ||
|
|
||
| - name: Upload strict-21 artifact | ||
| uses: actions/upload-artifact@v4 | ||
| with: | ||
| name: strict21-preflight | ||
| path: .tmp/strict21-preflight |
There was a problem hiding this comment.
1. strict21_preflight.py lacks tests 📘 Rule violation ⛯ Reliability
This PR introduces new CI/tooling behavior (strict-21 preflight gate) but does not add deterministic automated tests or commit deterministic evidence artifacts to verify the change. Reviewers cannot reproducibly validate that the new preflight logic behaves as intended from repository artifacts alone.
Agent Prompt
## Issue description
New strict-21 CI/tooling behavior is added, but the PR does not include deterministic automated test coverage or deterministic, committed evidence artifacts verifying the change.
## Issue Context
The compliance requirement expects tooling/test-impacting changes to be reproducibly verifiable from repository artifacts (tests or committed evidence). The workflow currently uploads only a transient Actions artifact from `.tmp/strict21-preflight`.
## Fix Focus Areas
- .github/workflows/strict21-preflight.yml[20-39]
- scripts/strict21_preflight.py[99-129]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+262
to
+297
| token = (os.environ.get("GITHUB_TOKEN") or "").strip() or (os.environ.get("GH_TOKEN") or "").strip() | ||
| now = datetime.now(timezone.utc).isoformat() | ||
| if not token: | ||
| result, branch_required_checks, emitted_contexts = _missing_token_result() | ||
| else: | ||
| result, branch_required_checks, emitted_contexts = _run_preflight( | ||
| args=args, | ||
| canonical=canonical, | ||
| token=token, | ||
| ) | ||
|
|
||
| payload = { | ||
| "status": result.status, | ||
| "repo": args.repo, | ||
| "branch": args.branch, | ||
| "ref": args.ref, | ||
| "ref_sha": result.ref_sha, | ||
| "timestamp_utc": now, | ||
| "canonical_contexts": canonical, | ||
| "branch_protection_required_checks": branch_required_checks, | ||
| "emitted_contexts": emitted_contexts, | ||
| "missing_in_branch_protection": result.missing_in_branch_protection, | ||
| "missing_in_check_runs": result.missing_in_check_runs, | ||
| "findings": result.findings, | ||
| "http_status": result.http_status, | ||
| "http_error": result.http_error, | ||
| } | ||
|
|
||
| out_json.parent.mkdir(parents=True, exist_ok=True) | ||
| out_md.parent.mkdir(parents=True, exist_ok=True) | ||
| out_json.write_text(json.dumps(payload, indent=2, sort_keys=True) + "\n", encoding="utf-8") | ||
| out_md.write_text(_render_markdown(payload), encoding="utf-8") | ||
|
|
||
| if result.status in {"non_compliant", "api_error"}: | ||
| return 1 | ||
| return 0 |
There was a problem hiding this comment.
2. Preflight passes without token 📘 Rule violation ⛯ Reliability
When GITHUB_TOKEN/GH_TOKEN is missing or the API returns permission-related errors, the preflight sets status to inconclusive_permissions but still exits with code 0. This can produce a green required check even though required inputs/queries were unavailable, masking missing required artifacts/diagnostics.
Agent Prompt
## Issue description
The preflight returns exit code 0 for `inconclusive_permissions` (e.g., missing token or 401/403/404), which can silently pass a required check even though required inputs/queries were unavailable.
## Issue Context
Compliance requires failing loudly or explicitly indicating missing required artifacts/inputs instead of reporting success. The script already sets `status="inconclusive_permissions"`, but `main()` does not treat it as a failing status.
## Fix Focus Areas
- scripts/strict21_preflight.py[189-197]
- scripts/strict21_preflight.py[262-297]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+27
to
+32
| python3 scripts/strict21_preflight.py \ | ||
| --repo "${GITHUB_REPOSITORY}" \ | ||
| --branch main \ | ||
| --ref "${GITHUB_SHA}" \ | ||
| --out-json ".tmp/strict21-preflight/preflight.json" \ | ||
| --out-md ".tmp/strict21-preflight/preflight.md" |
There was a problem hiding this comment.
3. Wrong ref for checks 🐞 Bug ⛯ Reliability
The workflow inventories emitted checks using GITHUB_SHA, which on PR events can differ from the head SHA and can be queried before other check-runs exist, causing flaky false non_compliant failures. Use the PR head SHA (and/or wait/retry for expected contexts) before concluding missing emitted checks.
Agent Prompt
### Issue description
The preflight workflow uses `GITHUB_SHA` as the `--ref` for emitted-check inventory. On PR runs this can point at a merge SHA and/or be queried before other checks are created, producing flaky false failures.
### Issue Context
The checker currently queries check-runs/status once for the resolved SHA and immediately compares against canonical contexts.
### Fix Focus Areas
- .github/workflows/strict21-preflight.yml[20-32]
- scripts/strict21_preflight.py[226-249]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+233
to
+242
| protection = _api_get(args.api_base, args.repo, f"branches/{args.branch}/protection", token) | ||
| ref_payload = _api_get(args.api_base, args.repo, f"commits/{args.ref}", token) | ||
| ref_sha = str(ref_payload.get("sha") or "").strip() or None | ||
| if ref_sha is None: | ||
| raise RuntimeError(f"Unable to resolve SHA for ref {args.ref!r}") | ||
| check_runs = _api_get(args.api_base, args.repo, f"commits/{ref_sha}/check-runs?per_page=100", token) | ||
| status_payload = _api_get(args.api_base, args.repo, f"commits/{ref_sha}/status", token) | ||
|
|
||
| branch_required_checks = sorted((protection.get("required_status_checks") or {}).get("contexts") or []) | ||
| emitted_contexts = _collect_emitted_contexts(check_runs, status_payload) |
There was a problem hiding this comment.
4. Branch checks field ignored 🐞 Bug ✓ Correctness
Branch protection required checks are read only from required_status_checks.contexts; if the repo uses the newer required_status_checks.checks field, the script may treat the required list as empty and misreport compliance. Parse both contexts and checks to avoid false results.
Agent Prompt
### Issue description
The preflight only reads `required_status_checks.contexts` from the branch protection API response, which can miss required checks if the branch protection config is represented differently.
### Issue Context
A missing/empty `branch_required_checks` list will cause false non-compliance for every canonical context.
### Fix Focus Areas
- scripts/strict21_preflight.py[233-243]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+4
to
+29
| "required_status_checks": [ | ||
| "applitools-core", | ||
| "pr-agent", | ||
| "deep-agent", | ||
| "audit-pr-evidence", | ||
| "backend", | ||
| "backend-postgres", | ||
| "coverage", | ||
| "CodeQL", | ||
| "codecov-analytics", | ||
| "Analyze (actions)", | ||
| "Analyze (javascript-typescript)", | ||
| "Analyze (python)", | ||
| "CodeRabbit", | ||
| "dependency-review", | ||
| "compose-smoke", | ||
| "frontend", | ||
| "label", | ||
| "codacy-equivalent-zero", | ||
| "sonar-branch-zero", | ||
| "Seer Code Review", | ||
| "SonarCloud Code Analysis" | ||
| ], | ||
| "strict": true, | ||
| "require_linear_history": true, | ||
| "require_conversation_resolution": false |
There was a problem hiding this comment.
5. Policy conflicts repo baseline 🐞 Bug ✓ Correctness
The new canonical required_status_checks list omits repo-documented required checks (build-test, validate-policy-contracts) and sets require_conversation_resolution=false, conflicting with the existing branch protection baseline documented in the repo. This will either cause unexpected strict21 preflight failures or weaken protections unless docs/branch protection are updated in lockstep.
Agent Prompt
### Issue description
The committed canonical policy conflicts with repo documentation and existing workflow check names, which risks breaking preflight or weakening protections.
### Issue Context
`docs/KPI_BASELINE.md` defines the currently expected required checks for `main` and requires conversation resolution.
### Fix Focus Areas
- .github/branch-protection-policy.json[1-30]
- scripts/strict21_preflight.py[15-38]
- docs/KPI_BASELINE.md[54-76]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
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
1 participant
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.
Problem / Outcome
Start strict-21 rollout (Vercel-free) with canonical context policy and preflight gate.
Scope and Success Criteria
Risk Classification
risk:medium
Rollback Plan
Revert this branch if strict preflight blocks valid PRs unexpectedly.
Evidence Paths