docs(doctoring): record codeql-pr.yml required-workflow platform restriction - #1767
Merged
Merged
Conversation
…riction github/codeql-action/init and analyze are categorically disallowed inside a GitHub "required workflow" (rulesets' workflows rule type included) -- a documented platform limitation, not a config bug. Confirmed 0/35+ success rate for codeql-pr.yml's ruleset-injected runs across every sampled repo (wardnet, naruon, contextual-orchestrator, keyverse, html4tree), all startup_failure with zero jobs created, root cause only visible in the web UI annotations (REST API exposes nothing). Since do_not_enforce_on_create only affects branch/PR creation and not ongoing merge eligibility, this was a required, always-failing check blocking every ordinary merge org-wide, independent of the already-documented plan-ceiling and Strix-starvation causes. Removed codeql-pr.yml from ruleset 18156473's required workflows via admin:org PUT (diffed before/after: only that one entry changed). Some repos already carry GitHub's native code-scanning default setup independently and successfully, so this is not a coverage-to-zero change, just removal of an always-broken gate. Recommended follow-up (rebuild central coverage via the same repository_dispatch pattern strix.yml/ opencode-review.yml already use) is tracked in the doc, not attempted here. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Warning Review limit reachedNext included review available in 34 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (1)
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 |
seonghobae
deleted the
claude/fix-codeql-required-workflow-restriction
branch
September 3, 2026 03:29
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
Comment on lines
+43
to
+44
| `18156473`'s `pull_request` rule requires 1 approving review and its | ||
| `workflows` rule required `codeql-pr.yml` among nine others, with no |
Contributor
There was a problem hiding this comment.
🔍 Ruleset review count is stale
The record says required_approving_review_count is one, while the enforced audit contract requires two. Later incident reconstruction can rely on the wrong gate.
Was this helpful? React with 👍 or 👎 to provide feedback.
Comment on lines
+53
to
+54
| ([[project-actions-plan-concurrency-ceiling]]) and per-repo Strix starvation | ||
| ([[project-strix-concurrency-starvation-unfixed]]). Every merge that landed |
Contributor
There was a problem hiding this comment.
🔍 Related incident references do not resolve
Both [[project-*]] references lack repository targets. Readers cannot reach the concurrency and Strix evidence this record depends on.
Was this helpful? React with 👍 or 👎 to provide feedback.
This was referenced Sep 3, 2026
seonghobae
added a commit
that referenced
this pull request
Sep 3, 2026
Two real conflicts, both resolved by preserving intent from both sides: - scorecard-analysis.yml: main (#1768, peer 1) independently fixed the same missing-concurrency-block gap this branch fixed earlier (bdacd1b) but with a better-reasoned design -- cancel-in-progress: false so an older main commit's SARIF scan is never killed mid-flight, matching strix.yml's existing precedent, instead of this branch's earlier cancel-in-progress: true (which treated it like a PR-scoped check where only the latest head matters -- not true for permanent main history). Took main's version. - docs/product-technical-gap-baseline.md: both branches independently appended different new sections after the same shared paragraph (main reworded it slightly), so git's merge lost the boundary between the two branches' additions. Union-merged: kept this branch's item 39/ #1743-regression/item 13 sections, main's slightly-reworded shared paragraph, and main's "codeql-pr.yml required-workflow hard limit closed org-wide" section (peer 1's #1767 writeup) -- verified it wasn't already present elsewhere in this branch before keeping it. Full suite (2704 tests) passes after both resolutions. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
seonghobae
added a commit
that referenced
this pull request
Sep 3, 2026
…solidate duplicate CodeQL sections Devin Review caught two real issues: - Item 4's Status line still called the gateway attribution gap "still-open" while the Update paragraph below it already said an open PR fixes it -- a plain contradiction from an earlier edit that updated one paragraph but not the other. Fixed the Status line to match. - The new "Item 41" section and the pre-existing "codeql-pr.yml required-workflow hard limit closed org-wide" section documented the same closure with different scope and counts (23 vs. 23+16 repos), a real duplication risk for future operational drift. Consolidated by adding an explicit superseded/extended note to the older, narrower entry pointing to Item 41 as the current complete record, rather than deleting either -- each still has content (the #1767 citation, the 23-repo list) the other lacks. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
seonghobae
added a commit
that referenced
this pull request
Sep 3, 2026
…erences Devin Review caught it: my consolidation note (previous commit) used the shorthand .github#1767 twice, and the adjacent pre-existing sentence used it once more -- the repo's own binding convention (CLAUDE.md section 7) requires owner/repo#num or a full URL for cross-repository references. Fixed all three to #1767. Left an unrelated .github#1463 reference elsewhere in the file untouched -- pre-existing content outside this PR's diff, out of scope here. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
seonghobae
added a commit
that referenced
this pull request
Sep 3, 2026
…#1774) * refactor(codeql): extract the Medium+ SARIF gate into a shared, tested script codeql-pr.yml duplicated the same ~70-line inline Python severity gate in both analyze-head and analyze-merge. Extract it to scripts/ci/codeql_sarif_gate.py (100% coverage/docstrings, its own unit tests) so both jobs call one script, and so the dispatch handler designed in docs/adr/0025-codeql-required-workflow-dispatch-architecture.md can reuse it as a third caller without a third copy of the logic. tests/test_codeql_pr_workflow_contract.py pins exact workflow prose; updated its assertions to match the delegation and to exercise the real script file via subprocess instead of re-executing extracted inline script text. This is step 1 of ADR 0025's implementation follow-up. No codeql-action reference is touched — codeql-pr.yml is not currently in the required-workflow ruleset (removed in #1767), so this carries none of that admission-check risk. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(codeql): stop citing a file that doesn't exist on this branch yet Devin review on #1774 flagged the module docstring's reference to docs/adr/0025-codeql-required-workflow-dispatch-architecture.md -- that file only exists on the separate, still-unmerged .github#1772 branch, not here or on main, so the citation was dangling regardless of which PR merges first. Point at the PR instead of a file path that may or may not exist yet. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
seonghobae
added a commit
that referenced
this pull request
Sep 3, 2026
* feat(codeql): rewrite codeql-pr.yml as dispatch+poll (not wired to the ruleset) Step 3 of ADR 0025's implementation follow-up (docs/adr/0025-codeql-required-workflow-dispatch-architecture.md, .github#1772, still open): the last and highest-risk piece, since this is the org's central required-workflow file. Removes every github/codeql-action reference (the platform restriction root-caused in docs/doctoring/codeql-pr-required-workflow-always-fails.md and fixed by removing this file from ruleset 18156473 in #1767) -- this PR does NOT re-admit it to the ruleset, so merging carries zero required-workflow admission risk; it can only self-trigger on .github's own PRs until someone explicitly does that re-admission as a separate, later step. - detect-languages: unchanged. - analyze-head: two sequential steps in ONE job (mirroring opencode-review.yml's opencode-review-target job exactly) -- "Request current-head CodeQL scan dispatch" then "Fail closed without a current-head CodeQL dispatch verdict". Dispatch+poll live in the same job, not two jobs linked by `needs:`, specifically so a dispatch failure fails the job directly with no needs-based skip to reason about. - analyze-merge: deleted. Required nowhere per PR #1766; migrating it doubles this change's risk for a check that gates nothing today (ADR's explicit scope decision). Two bugs caught and fixed during implementation, before either was pushed: 1. A job-level `if:` on analyze-head would have reintroduced the exact unexpanded-matrix-name bug live evidence (run 33708209086) already proved real -- caught by the existing test_codeql_pr_gates_analyze_head_at_step_level_not_job_level contract test. Fixed by keeping analyze-head's admission unconditional (matching the original's proven-safe `needs: detect-languages` with no job-level `if:`) and gating only at step level. 2. An initial two-job (dispatch-analysis + analyze-head) split would have let analyze-head's matrix duplicate the dispatch N times (once per language), each carrying the full language matrix -- triggering N redundant full-matrix scans on the .github side. Fixed by merging dispatch+poll into one job and restricting the dispatch step to fire from only the first matrix shard via `matrix.language == fromJSON(needs.detect-languages.outputs.matrix).include[0].language`. tests/test_codeql_pr_workflow_contract.py rewritten for the new structure (previously pinned the old codeql-action/inline-SARIF-gate shape byte-for-byte). tests/test_docs_only_pr_runner_admission.py's job-vs-step-level gating test updated to match the new two-step shape; its core assertion (no job-level `if:` on analyze-head) is unchanged and still enforced. Depends on #1776 (the native dispatch handler) existing before this dispatch step can ever succeed against a real PR -- opened as draft for that reason, and because this is genuinely untestable live before merge. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(codeql): dispatch per-shard so a dispatch failure fails closed, not silently Peer review on #1778 found a real gap: only the first matrix shard dispatched (carrying the full language matrix), so if THAT dispatch failed, every other shard had no way to know -- each would poll the full 3-hour deadline before self-timing-out for a scan that was never actually requested. A repo with 3 CodeQL languages could turn one dispatch failure into ~9 wasted runner-hours, working directly against the org's active 60-job-ceiling capacity fight. Fixed by having every shard dispatch, but only its own single language (not the full matrix): N single-language dispatches cost the same total .github-side work as one N-language dispatch, while letting each shard read its own steps.dispatch.outcome and fail closed immediately instead of only detecting the failure 3 hours later. The peer's second finding (scope the concurrency group by exact head SHA, mirroring opencode-review.yml) does NOT apply here as a drop-in fix: tests/test_required_workflow_queue_contract.py::test_pull_request_close_events_cancel_superseded_runs_without_heavy_jobs explicitly requires codeql-pr.yml's group to omit head SHA, because this file has no dedicated cancel-on-close cleanup job. Adding head SHA without one would let a stale in-flight run for a superseded head survive a close event indefinitely (it and the closing run would land in different groups and never cancel each other) -- opencode-review.yml can safely add head SHA only because it also runs a separate cancel-superseded-opencode-review-runs job that sweeps stale runs via direct API calls regardless of head SHA. Documented the real, narrower residual risk in a code comment and left it as a tracked follow-up requiring a dedicated cleanup job, not a one-line group change that would regress an existing, deliberately-tested invariant. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(codeql): verify dispatch-status creator identity, not just context codeql-pr.yml's poll step matched a commit status by context alone ("codeql-dispatch/<language>"), which ADR 0025's own Security considerations section already flagged as unresolved: anyone with statuses:write on the target repository can publish an arbitrary context, so a malicious PR could forge its own passing status and skip being scanned entirely. codeql-scan-dispatch.yml mints its publishing token via the same OIDC audience (opencode-github-action) opencode-review-dispatch.yml uses, so the legitimate status always carries that app's bot identity. Mirror opencode-review.yml's existing opencode-agent/opencode-agent[bot] creator check in the poll's jq filter instead of trusting the context name alone. Adds two real-shell-exec regression tests against a faked `gh`: one proving a forged success status from another creator is ignored in favor of the legitimate (here, failing) verdict, one proving the legitimate creator's status is accepted normally. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
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.
Summary
github/codeql-action/init/analyzeare categorically disallowed inside a GitHub required workflow -- a documented platform restriction, not a config bug in this repo.18156473already hadcodeql-pr.ymlremoved from its required-workflow list via a directadmin:orgPUT (this PR is documentation of that already-applied fix, not the fix mechanism itself -- ruleset changes aren'''t file-based).Evidence
codeql-pr.yml's ruleset-injected runs across every sampled repo (wardnet, naruon, contextual-orchestrator, keyverse, html4tree) -- alwaysstartup_failure, zero jobs created.do_not_enforce_on_createonly exempts branch/PR creation, not ongoing merge eligibility -- confirmed this was a required, always-failing check blocking every ordinary (non-admin-bypass) merge org-wide.Test plan
codeql-pr.ymlentry was removed from theworkflowsarray;bypass_actors, the other 9 required workflows, and thepull_request/deletion/non_fast_forwardrules are byte-identical.🤖 Generated with Claude Code