Skip to content

docs: align CI contract with codeql required-check rename - #74

Merged
Navi Bot (project-navi-bot) merged 3 commits into
mainfrom
chore/codeql-required-check-rename
May 7, 2026
Merged

docs: align CI contract with codeql required-check rename#74
Navi Bot (project-navi-bot) merged 3 commits into
mainfrom
chore/codeql-required-check-rename

Conversation

@Fieldnote-Echo

Copy link
Copy Markdown
Member

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:

  1. 2d4e52b renameAnalyze (python)codeql in CLAUDE.md, tests/README.md, and the contract comment in .github/workflows/codeql.yml. Also drops test from the documented required-checks list (it was never actually in the ruleset; pre-existing drift).
  2. fbe4d0b arithmeticCLAUDE.md claimed ci.yml "holds all but one of the required checks." Under the 6-check contract it holds four; the other two are in codeql.yml and semgrep.yml. Sentence rewritten to match.
  3. 7bdb718 ci.yml comments — header CONTRACT block previously said "all org-required checks live here" and listed test as required. Both false. Header now lists what this file actually provides; the per-job comment above test replaces the false "ruleset requires this exact check name" claim with a NOTE explaining it's a fail-fast aggregator.

Why

The org CI: Python Tier ruleset previously required Analyze (python) — the SARIF-upload check from github/codeql-action/analyze. That check never fires on Dependabot PRs 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. 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 on push events to main and the weekly scheduled run.

What's not in this PR

  • The ruleset PUT itself — already applied (org-level, affects all 5 repos in the ruleset). This PR only updates the in-repo docs/comments to match.
  • Dependabot bypass entry cleanup — the bypass was added earlier as a misdiagnosis; user removed it manually after the rename made it unnecessary.

Test plan

  • lint, typecheck, security, codeql, semgrep, quality-gate all pass on this PR
  • codeql check fires (proves the new contract holds even on a non-Dependabot PR)
  • grep for Analyze (python) returns only intentional context references (explaining why the rename), not stale contract claims

🤖 Generated with Claude Code

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.
@qodo-code-review

Copy link
Copy Markdown

Review Summary by Qodo

Align CI contract docs with codeql required-check rename

📝 Documentation

Grey Divider

Walkthroughs

Description
• 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
Diagram
flowchart 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"]
Loading

Grey Divider

File Changes

1. .github/workflows/ci.yml 📝 Documentation +10/-4

Clarify contract scope and test job role

• Updated header comment to clarify file holds 4 of 6 required checks, not all
• Added note that codeql and semgrep checks live in separate workflow files
• Replaced false "ruleset requires this exact check name 'test'" claim with NOTE explaining test
 is a fail-fast aggregator not in ruleset
• Updated section header to list actual required checks in this file

.github/workflows/ci.yml


2. .github/workflows/codeql.yml 📝 Documentation +8/-6

Explain codeql job key requirement and Dependabot issue

• Rewrote CONTRACT comment to explain why job key codeql is required instead of Analyze (python)
• Added explanation that Dependabot's read-only token prevents SARIF upload and Analyze (python)
 check from firing on Dependabot PRs
• Clarified that Analyze (python) still fires on push and scheduled runs but not on Dependabot
 PRs
• Emphasized not to rename job or add name: override

.github/workflows/codeql.yml


3. CLAUDE.md 📝 Documentation +5/-5

Update CI contract documentation and Dependabot explanation

• Updated CodeQL gotcha to clarify required check is codeql job key, not Analyze (python)
 SARIF-upload check
• Explained Dependabot token read-only issue that motivated the rename
• Fixed CI section intro: changed "all but one" to accurate "four of six" required checks in
 ci.yml
• Updated workflow table to reflect new 6-check contract and clarify test is aggregator not in
 ruleset
• Updated org ruleset contract line to list codeql instead of Analyze (python) and clarify
 test is not required

CLAUDE.md


View more (1)
4. tests/README.md 📝 Documentation +2/-2

Update required checks list and test job clarification

• Removed test and Analyze (python) from required checks list
• Added codeql to required checks list
• Clarified test aggregator runs on every PR but is not in ruleset's required list

tests/README.md


Grey Divider

Qodo Logo

@qodo-code-review

qodo-code-review Bot commented May 7, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

Grey Divider

Qodo Logo

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 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".

Comment on lines +23 to 27
# 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:

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge 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 👍 / 👎.

@project-navi-bot
Navi Bot (project-navi-bot) merged commit 09d056d into main May 7, 2026
17 checks passed
@project-navi-bot
Navi Bot (project-navi-bot) deleted the chore/codeql-required-check-rename branch May 7, 2026 17:45
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.

2 participants