docs: align CI contract with codeql required-check rename - #74
Conversation
The org ruleset 'CI: Python Tier' previously required 'Analyze (python)', the SARIF-upload check emitted by github/codeql-action/analyze. That check never fires on Dependabot pull requests because Dependabot's GITHUB_TOKEN is forced read-only on pull_request events regardless of the workflow's permissions: block — the SARIF upload silently no-ops and the named check is never registered, leaving every Dependabot PR permanently blocked on a missing required check. Switch the required check to 'codeql' (the GitHub Actions job key in codeql.yml), which fires on every PR including Dependabot's. Security posture is unchanged: the analyze step still runs in the same job, and SARIF still uploads on push events to main and the weekly scheduled run, populating the Security tab. Also drops 'test' from the documented required-checks list — it was never actually in the ruleset; the docs had drifted. Files: - .github/workflows/codeql.yml: rewrite the contract comment - CLAUDE.md: update the gotcha, workflow table, and ruleset contract - tests/README.md: align CI section with current ruleset
The intro sentence to the CI section claimed ci.yml 'holds all but one of the required checks'. Under the new 6-check contract (lint / typecheck / security / codeql / semgrep / quality-gate) ci.yml holds four — codeql.yml and semgrep.yml hold the other two. Update the sentence to match.
ci.yml's header comment claimed 'all org-required checks live here' and listed 'test' as one of the required CONTRACT items. After the Analyze (python) -> codeql swap, neither is true: the file holds four of six required checks (codeql.yml and semgrep.yml hold the others), and 'test' was never actually in the ruleset's required list — it's a fail-fast aggregator over the test-run matrix. - Header CONTRACT block now lists only what this file actually provides (lint, typecheck, security, quality-gate) and points to where codeql and semgrep come from. - The 'required checks' section header acknowledges the 'test' job is an aggregator, not a required check. - The per-job comment above 'test' replaces the false 'org ruleset requires this exact check name' claim with a NOTE explaining the job's actual role.
Review Summary by QodoAlign CI contract docs with codeql required-check rename
WalkthroughsDescription• Rename required CI check from Analyze (python) to codeql job key • Fix arithmetic: ci.yml holds 4 of 6 required checks, not all-but-one • Clarify test job is aggregator, not in ruleset's required list • Explain Dependabot token read-only issue motivating the rename Diagramflowchart LR
A["Analyze (python)<br/>SARIF-upload check"] -->|"Dependabot PR<br/>token read-only"| B["Never fires<br/>on Dependabot"]
C["codeql<br/>job key"] -->|"Fires on all PRs<br/>including Dependabot"| D["Unblocks<br/>Dependabot"]
E["ci.yml<br/>4 checks"] -->|"plus"| F["codeql.yml<br/>1 check"]
F -->|"plus"| G["semgrep.yml<br/>1 check"]
G -->|"equals"| H["6 required<br/>checks total"]
File Changes1. .github/workflows/ci.yml
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 830a33a416
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "Codex (@codex) review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "Codex (@codex) address that feedback".
| # step) because the SARIF-upload check that produces 'Analyze (python)' does | ||
| # not fire on Dependabot pull requests — Dependabot's GITHUB_TOKEN is forced | ||
| # read-only on pull_request events, so the upload silently no-ops and the | ||
| # named check is never registered, freezing every Dependabot PR. | ||
| codeql: |
There was a problem hiding this comment.
Correct the Dependabot CodeQL upload rationale
For Dependabot PRs targeting main, this workflow runs on pull_request, and GitHub’s code-scanning troubleshooting docs state SARIF uploads are still allowed for pull_request-triggered runs even when Dependabot’s token is read-only; the 403/read-only problem applies to branch uploads such as push on Dependabot branches. Documenting that the upload no-ops and Analyze (python) never registers on these PRs leaves the CI contract/runbook pointing maintainers at the wrong failure mode.
Useful? React with 👍 / 👎.
Summary
Brings repo-local CI contract docs and workflow comments into agreement with the org ruleset change made earlier today (
Analyze (python)→codeql). Three commits, each scoped:2d4e52brename —Analyze (python)→codeqlinCLAUDE.md,tests/README.md, and the contract comment in.github/workflows/codeql.yml. Also dropstestfrom the documented required-checks list (it was never actually in the ruleset; pre-existing drift).fbe4d0barithmetic —CLAUDE.mdclaimedci.yml"holds all but one of the required checks." Under the 6-check contract it holds four; the other two are incodeql.ymlandsemgrep.yml. Sentence rewritten to match.7bdb718ci.yml comments — header CONTRACT block previously said "all org-required checks live here" and listedtestas required. Both false. Header now lists what this file actually provides; the per-job comment abovetestreplaces the false "ruleset requires this exact check name" claim with a NOTE explaining it's a fail-fast aggregator.Why
The org
CI: Python Tierruleset previously requiredAnalyze (python)— the SARIF-upload check fromgithub/codeql-action/analyze. That check never fires on Dependabot PRs because Dependabot'sGITHUB_TOKENis forced read-only onpull_requestevents regardless of the workflow'spermissions:block; the SARIF upload silently no-ops and the named check is never registered. Result: every Dependabot PR was permanently blocked on a missing required check.Switching the required check to
codeql(the GitHub Actions job key) — which fires on every PR including Dependabot's — unsticks Dependabot without changing the security posture: the analyze step still runs in the same job, and SARIF still uploads onpushevents to main and the weekly scheduled run.What's not in this PR
Test plan
lint,typecheck,security,codeql,semgrep,quality-gateall pass on this PRcodeqlcheck fires (proves the new contract holds even on a non-Dependabot PR)grepforAnalyze (python)returns only intentional context references (explaining why the rename), not stale contract claims🤖 Generated with Claude Code