fix(codeql): serialize exact dispatch wakeups - #2056
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
There was a problem hiding this comment.
Pull request overview
OpenCode reviewed the current-head product diff. Coverage is a separate gate.
Changed files
.github/workflows/codeql-scan-dispatch.yml— GitHub Actions review jobAGENTS.md— repository behaviorCHANGELOG.md— repository behaviorCLAUDE.md— repository behaviordocs/doctoring/codeql-dispatch-multi-language-wake-race.md— operator or user guidancetests/test_codeql_scan_dispatch_workflow_contract.py— regression suite
Changed behavior
flowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Workflow: codeql-scan-dispatch.yml"]
S1 --> I1["GitHub Actions review job"]
I1 --> R1["Review risk: Workflow: codeql-scan-dispatch.yml"]
R1 --> V1["actionlint plus required checks"]
Evidence --> S2["Repository file: AGENTS.md"]
S2 --> I2["repository behavior"]
I2 --> R2["Review risk: Repository file: AGENTS.md"]
R2 --> V2["required checks"]
Evidence --> S3["Repository file: CHANGELOG.md"]
S3 --> I3["repository behavior"]
I3 --> R3["Review risk: Repository file: CHANGELOG.md"]
R3 --> V3["required checks"]
Evidence --> S4["Repository file: CLAUDE.md"]
S4 --> I4["repository behavior"]
I4 --> R4["Review risk: Repository file: CLAUDE.md"]
R4 --> V4["required checks"]
Evidence --> S5["Docs: codeql-dispatch-multi-language-wake-race.md"]
S5 --> I5["operator or user guidance"]
I5 --> R5["Review risk: Docs: codeql-dispatch-multi-language-wake-race.md"]
R5 --> V5["docs review"]
Evidence --> S6["Test: test_codeql_scan_dispatch_workflow_contract.py"]
S6 --> I6["regression suite"]
I6 --> R6["Review risk: Test: test_codeql_scan_dispatch_workflow_contract.py"]
R6 --> V6["targeted test run"]
Findings
No source-backed product finding is synthesized from the coverage gate. A coverage miss belongs in the status comment.
- Head SHA:
75ca862da09729361b52183b4345987fc0112911 - Workflow run: 34323731752
- Workflow attempt: 1
- Coverage gate:
failure
Review outcome
Coverage is a gate, not the review. This body reviews the changed product files.
Changed-File Evidence Map
flowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Workflow: codeql-scan-dispatch.yml"]
S1 --> I1["GitHub Actions review job"]
I1 --> R1["Review risk: Workflow: codeql-scan-dispatch.yml"]
R1 --> V1["actionlint plus required checks"]
Evidence --> S2["Repository file: AGENTS.md"]
S2 --> I2["repository behavior"]
I2 --> R2["Review risk: Repository file: AGENTS.md"]
R2 --> V2["required checks"]
Evidence --> S3["Repository file: CHANGELOG.md"]
S3 --> I3["repository behavior"]
I3 --> R3["Review risk: Repository file: CHANGELOG.md"]
R3 --> V3["required checks"]
Evidence --> S4["Repository file: CLAUDE.md"]
S4 --> I4["repository behavior"]
I4 --> R4["Review risk: Repository file: CLAUDE.md"]
R4 --> V4["required checks"]
Evidence --> S5["Docs: codeql-dispatch-multi-language-wake-race.md"]
S5 --> I5["operator or user guidance"]
I5 --> R5["Review risk: Docs: codeql-dispatch-multi-language-wake-race.md"]
R5 --> V5["docs review"]
Evidence --> S6["Test: test_codeql_scan_dispatch_workflow_contract.py"]
S6 --> I6["regression suite"]
I6 --> R6["Review risk: Test: test_codeql_scan_dispatch_workflow_contract.py"]
R6 --> V6["targeted test run"]
OpenCode Review Overview
|
…deql-atomic-wake-20260909 Signed-off-by: Seongho Bae <me@seonghobae.me> # Conflicts: # .github/workflows/codeql-scan-dispatch.yml # AGENTS.md # CLAUDE.md # tests/test_codeql_scan_dispatch_workflow_contract.py
…om/ContextualWisdomLab/.github into codex/codeql-atomic-wake-20260909 Signed-off-by: Seongho Bae <me@seonghobae.me> # Conflicts: # docs/adr/0025-codeql-required-workflow-dispatch-architecture.md
Exact-head visual inspection and review request
The existing OpenCode review is stale because it names head @cwl-noema-review @opencode-agent review exact head |
There was a problem hiding this comment.
Pull request overview
OpenCode reviewed the current-head mergeability evidence and changed-file flow before approval, then found merge conflicts on the affected path.
Findings
1. HIGH Merge Conflict Guidance - Resolve the PR branch against the latest base branch
- Problem: GitHub reports mergeStateStatus
DIRTYfor this pull request. - Root cause: Branch
codex/codeql-atomic-wake-20260909cannot be merged cleanly intofix/codeql-wake-sibling-rerun-race; the changed-file flow below shows which review/runtime path is blocked by the conflict. - Fix: Merge or rebase the latest
fix/codeql-wake-sibling-rerun-raceintocodex/codeql-atomic-wake-20260909, resolve conflict markers in the PR branch, rerun the focused checks, and push the same branch. - Repair commands:
gh pr checkout 2056 --repo ContextualWisdomLab/.github
git fetch origin fix/codeql-wake-sibling-rerun-race
git merge --no-ff origin/fix/codeql-wake-sibling-rerun-race # or: git rebase origin/fix/codeql-wake-sibling-rerun-race
git status --short
# resolve files, then git add <resolved-files>
# merge path: git commit
# rebase path: git rebase --continue
git push origin HEAD:codex/codeql-atomic-wake-20260909
# rebase path only: git push --force-with-lease origin HEAD:codex/codeql-atomic-wake-20260909- Regression test: Keep OpenCode approval gated on mergeability so model-output failures cannot approve a conflicted PR.
Merge Conflict Evidence Map
flowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Workflow: codeql-scan-dispatch.yml"]
S1 --> I1["GitHub Actions review job"]
I1 --> Conflict["Merge conflict blocks this path"]
Conflict --> V1["actionlint plus required checks"]
Evidence --> S2["Repository file: AGENTS.md"]
S2 --> I2["repository behavior"]
I2 --> Conflict["Merge conflict blocks this path"]
Conflict --> V2["required checks"]
Evidence --> S3["Repository file: CHANGELOG.md"]
S3 --> I3["repository behavior"]
I3 --> Conflict["Merge conflict blocks this path"]
Conflict --> V3["required checks"]
Evidence --> S4["Repository file: CLAUDE.md"]
S4 --> I4["repository behavior"]
I4 --> Conflict["Merge conflict blocks this path"]
Conflict --> V4["required checks"]
Evidence --> S5["Docs: 0025-codeql-required-workflow-dispatch-architecture.md (4 files)"]
S5 --> I5["operator or user guidance"]
I5 --> Conflict["Merge conflict blocks this path"]
Conflict --> V5["docs review"]
Evidence --> S6["Test: test_codeql_scan_dispatch_workflow_contract.py"]
S6 --> I6["regression suite"]
I6 --> Conflict["Merge conflict blocks this path"]
Conflict --> V6["targeted test run"]
- Result: REQUEST_CHANGES
- Reason: mergeStateStatus is
DIRTY; mergeable isCONFLICTING. - Head SHA:
78717d74e014039f4db9d6e0a805fb19387b826f - Workflow run: 34328413232
- Workflow attempt: 1
Changed-File Evidence Map
flowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Workflow: codeql-scan-dispatch.yml"]
S1 --> I1["GitHub Actions review job"]
I1 --> Conflict["Merge conflict blocks this path"]
Conflict --> V1["actionlint plus required checks"]
Evidence --> S2["Repository file: AGENTS.md"]
S2 --> I2["repository behavior"]
I2 --> Conflict["Merge conflict blocks this path"]
Conflict --> V2["required checks"]
Evidence --> S3["Repository file: CHANGELOG.md"]
S3 --> I3["repository behavior"]
I3 --> Conflict["Merge conflict blocks this path"]
Conflict --> V3["required checks"]
Evidence --> S4["Repository file: CLAUDE.md"]
S4 --> I4["repository behavior"]
I4 --> Conflict["Merge conflict blocks this path"]
Conflict --> V4["required checks"]
Evidence --> S5["Docs: 0025-codeql-required-workflow-dispatch-architecture.md (4 files)"]
S5 --> I5["operator or user guidance"]
I5 --> Conflict["Merge conflict blocks this path"]
Conflict --> V5["docs review"]
Evidence --> S6["Test: test_codeql_scan_dispatch_workflow_contract.py"]
S6 --> I6["regression suite"]
I6 --> Conflict["Merge conflict blocks this path"]
Conflict --> V6["targeted test run"]
There was a problem hiding this comment.
Noema LLM review
The PR changes the wake condition from needs.scan.result != 'cancelled' to needs.scan.result == 'success' at .github/workflows/codeql-scan-dispatch.yml:522. This narrows the wake trigger so that a skipped, failed, or otherwise non-successful scan matrix will never wake the required workflow, which can leave a failed CodeQL required run unrewoken and therefore block a PR without producing the expected evidence. The failed-job-set comparison also only matches jobs where the bound language passes a strict regex and integer filter, so a legitimately failed CodeQL job whose binding does not satisfy this filter will be rejected and the run will never be rerun. These are concrete behavioral regressions that need tests and narrowed conditions before approval.
Reviewed changed lines
.github/workflows/codeql-scan-dispatch.yml:522 (LEFT): Old conditionneeds.scan.result != 'cancelled'wakes the required workflow even when a scan shard has concluded with success, failure, or skipped states..github/workflows/codeql-scan-dispatch.yml:522 (RIGHT): New conditionneeds.scan.result == 'success'suppresses the exact wake when any shard is skipped or failed, which can leave a failed required CodeQL run without its authorized rerun..github/workflows/codeql-scan-dispatch.yml:599 (RIGHT):select((.language | type) == "string")filters language identity into the expected failed set, but if the authenticated CodeQL binding contains a failed job whose language is not a matching string, the exact-set comparison will reject the run without rerunning.
Adversarial validation
.github/workflows/codeql-scan-dispatch.yml:522 (RIGHT)confirmed: The exact wake runs only when the whole scan matrix concludes as success. — Hardcoded condition&& needs.scan.result == 'success'at changed line 522 directly gates the wake coordinator..github/workflows/codeql-scan-dispatch.yml:599 (RIGHT)confirmed: The failed-job-set comparison reruns every failed CodeQL job bound by the production payload. — The right-side jq filterselect((.language | type) == "string")andtest("^[a-z0-9-]+$")at changed lines 599-600 constrain the expected set.- Residual risk: A skipped or canceled scan shard after the matrix completes can now suppress the only authorized wake path, leaving a failed required CodeQL run uncorrected. The failed-job-set identity check can also reject a valid run when any bound job record omits a string language or when an earlier fixtureless GH API response changes pagination shape.
Findings
- [high] .github/workflows/codeql-scan-dispatch.yml:522 (RIGHT): Changing
needs.scan.result != 'cancelled'toneeds.scan.result == 'success'suppresses the wake whenever any scan shard completes as failed or skipped. Since the wake coordinator is the only authorization for rerunning a failed required CodeQL job, this can leave a failing PR unrewoken and break the exact-wake contract. - [medium] .github/workflows/codeql-scan-dispatch.yml:599 (RIGHT): The expected failed-job identifier set is assembled by selecting only string, lowercase-alphanumeric-hyphen languages and numeric job ids. A valid failed CodeQL binding that does not satisfy these filters will cause the observed/expected set comparison to fail closed without issuing the rerun.
- Result: REQUEST_CHANGES
- Head SHA:
78717d74e014039f4db9d6e0a805fb19387b826f - Reviewer credential:
noema-review-github-app-refresh - Actor:
cwl-noema-review[bot]
…ce' into HEAD Signed-off-by: Seongho Bae <me@seonghobae.me> # Conflicts: # docs/doctoring/codeql-partial-shard-wake-duplicate-dispatch.md # tests/test_codeql_scan_dispatch_workflow_contract.py
최신 owner base 승계#2056을 #2051 exact base
이전 head @cwl-noema-review @opencode-agent review exact head |
Signed-off-by: Seongho Bae <me@seonghobae.me>
exact-head 재검증대상 head는
기존 OpenCode/Noema 리뷰는 @cwl-noema-review @opencode-agent review exact head |
Root cause
Concurrent language shards independently woke failed jobs in one CodeQL required-workflow run. The first wake reactivated the run; a sibling wake then received GitHub HTTP 403 (
workflow run already running) and could leave that sibling's failed verdict unchanged.Change
This PR is stacked on #2051. That predecessor moves wake responsibility after the complete scan matrix and binds terminal evidence to the exact PR, head, base ref, base SHA, and required run. This successor tightens the atomic wake: it requires the supplied job ids to equal the run's complete failed-job set before one
rerun-failed-jobscall. A terminal scan failure also reaches the wake so the required check reflects the real failure; cancellation remains excluded because it has no complete evidence.Exact-head evidence
69ae472562c93cc17674af5e2085a58947d3fab8558693e0333e48012beea142f739bc634b0674a752 passed in 16.46s3000 passed, 1 skipped, 21 subtests passed in 170.92sactionlint .github/workflows/codeql-pr.yml .github/workflows/codeql-scan-dispatch.yml: passedgit diff --check: passedVerification boundary
repository_dispatchexecutes the protected default-branch workflow, not this open PR's branch version. Fixture-backed tests prove the branch contract; a fresh default-branch dispatch after protected integration remains required runtime evidence. No manual branch-selected workflow or security bypass was added. Earlier reviews target stale heads and are not current acceptance evidence.Scope
Nine files relative to #2051: owner workflow, regression contracts, ADR/doctoring/gap baseline, changelog, and reusable agent know-how. No consumer workflow or secret changed.