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 |
|
Fresh owner-path RCA from parent #2051 exact run Default-branch dispatch Please preserve #2056's complete-failed-job-set/one-wake semantics and add or adopt the parent repair as a prerequisite: under exactly one current required-run |
|
Follow-up reality RED on current |
|
Downstream exact-head canary from At leaf head The leaf has since advanced forward-only to |
|
Current-head recurrence for the same downstream canary: |
|
Fresh stacked-prerequisite update: predecessor #2051 remains exact The independent Preserve this #2056 exact |
|
Fresh fast-mlsirm canary on canonical report-focus #1768 reproduces the ordering failure on a new exact leaf head after all leaf-owned report/security evidence went GREEN. Required CodeQL PR run |
|
Fresh fast-mlsirm consumer canary for the exact-head wake ordering contract: |
seonghobae
left a comment
There was a problem hiding this comment.
P1 운영/비용 경계가 하나 더 남아 있습니다. #2051 required CodeQL run 34332431435가 동일 run id·동일 PR/head/base에서 github-actions[bot]에 의해 attempt 33까지 자동 재실행됐고, attempt 33도 actions/python reader가 Read current-head CodeQL dispatch verdict에서 실패한 뒤 dispatcher가 다시 성공했습니다. 이는 단일 attempt 안에서 failed-job set을 원자적으로 wake하는 #2056 수리와 별개로, 같은 required run을 attempt 간에 반복 wake/dispatch할 수 있음을 실제로 재현합니다.
현재 #2051 reader는 RUN_ATTEMPT != 1이고 exact successor-schema verdict가 없으면 fail-closed하지만, coordinator 쪽은 그 실패 attempt에서도 다시 dispatch할 수 있고 protected predecessor handler는 다시 rerun-failed-jobs를 호출할 수 있습니다. bootstrap schema mismatch가 지속되면 attempt N -> dispatch -> wake -> attempt N+1이 무기한 이어져 runner/job ceiling과 비용을 소모합니다.
최소 causal contract는 required run별 wake를 single-use로 만드는 것입니다. Attempt 1에서만 미결 language를 dispatch/wake하고, run_attempt > 1에서는 terminal dispatch evidence를 소비하거나 그대로 terminal failure로 끝내야 하며 새 dispatch나 rerun-failed-jobs를 다시 발행하면 안 됩니다. #2051의 bounded predecessor-title compatibility가 들어오면 attempt 2가 그 evidence를 소비해 정상 종료할 수 있습니다. Compatibility가 실패해도 자동 attempt 3+로 증폭되면 안 됩니다.
Reality RED로 동일 required run의 attempt=2, exact verdict 미인식 상태를 fixture에 넣어 dispatcher/wake 호출 수가 0이어야 함을 고정하고, attempt=1에서는 기존 complete-failed-job-set 검증 뒤 정확히 한 번만 wake됨을 보존해 주세요. SARIF failure/cancellation/base-ref/head/run/language 불일치는 계속 fail closed여야 하며, #2056의 complete failed-job set 검증은 그대로 유지해야 합니다. 이 finding은 ConceptWeave leaf 재실행으로 우회하지 않습니다.
seonghobae
left a comment
There was a problem hiding this comment.
Fresh exact-run RED가 기존 cross-attempt P1을 다시 재현했습니다. Required CodeQL run 34332431435는 PR #2051 / head 558693e0333e48012beea142f739bc634b0674a7 / base main@7fd571dbcdbae6acf29d8f4ee704d7ba6297e4db 결속을 유지한 채 github-actions[bot]으로 attempt 39까지 자동 재진입했습니다. Attempt 39에서도 actions/python compatibility reader가 모두 Read current-head CodeQL dispatch verdict에서 실패했고, 그 직후 Dispatch current-head CodeQL scan이 다시 실행 상태로 들어갔습니다.
새 finding을 추가하는 것이 아니라 review 5155230095의 reality RED를 더 강하게 확정하는 증거입니다. #2056의 complete-failed-job-set 검증과 attempt-1 atomic wake는 보존하되, run_attempt > 1에서는 새 dispatch 또는 rerun-failed-jobs를 발행하지 않아야 합니다. 이미 존재하는 terminal dispatch evidence를 exact {repository, PR, head, base_ref, base_sha, required_run_id, language}에 맞춰 소비하거나 그대로 fail closed로 종료해야 합니다. Fixture에는 attempt 2+ / exact successor verdict 미인식 상태에서 dispatch count=0, wake count=0을 고정하고 attempt 1의 기존 complete-set one-wake 계약은 유지해 주세요. SARIF failure, cancellation, stale head/base/ref/run/language는 계속 non-passing이어야 합니다.
ConceptWeave leaf를 no-op push/manual rerun해서 이 central-owner 결함을 우회하지 않습니다.
|
Fresh owner-path verification strengthens the existing cross-attempt P1 without adding a new finding. Required CodeQL run Current #2056 exact source |
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.