fix(queue): preserve terminal pre-execution failures - #2213
Conversation
|
Warning Review limit reachedNext included review available in 14 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: Advanced Run ID: 📒 Files selected for processing (4)
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 |
|
@coderabbitai review Please review exact head |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
parent='42bb922f03bf75aed1bc1931d9fbaf04a5433e20'
head='b016348d1ed35304eb2504d30093922dbf2d42ae'
git cat-file -e "${parent}^{commit}"
git cat-file -e "${head}^{commit}"
git diff --check "$parent" "$head"
git diff --stat "$parent" "$head"
git diff --find-renames --find-copies --unified=80 "$parent" "$head" -- \
scripts/ci/actions_queue_health.py \
tests/test_actions_queue_health_terminal_preexecution.py
printf '\n--- collector outline ---\n'
ast-grep outline scripts/ci/actions_queue_health.py --items all --view expanded
printf '\n--- regression-test outline ---\n'
ast-grep outline tests/test_actions_queue_health_terminal_preexecution.py --items all --view expandedLength of output: 20956 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- terminal collection and report implementation ---'
sed -n '99,368p' scripts/ci/actions_queue_health.py
sed -n '1,360p' scripts/ci/actions_queue_health_core.py
printf '%s\n' '--- related test definitions and contexts ---'
rg -n -i -C 12 \
'startup_failure|cancelled_before_runner_assignment|terminal_pre_execution|runner_assigned|steps_count|conclusion.*failure|status.*completed' \
tests scripts/ci \
-g '*.py'
printf '%s\n' '--- all changed paths and commit metadata ---'
git diff --name-status 42bb922f03bf75aed1bc1931d9fbaf04a5433e20 b016348d1ed35304eb2504d30093922dbf2d42ae
git show -s --format='commit=%H%nparents=%P%nsubject=%s' b016348d1ed35304eb2504d30093922dbf2d42ae
git merge-base --is-ancestor 42bb922f03bf75aed1bc1931d9fbaf04a5433e20 b016348d1ed35304eb2504d30093922dbf2d42ae
printf 'parent_is_ancestor=%s\n' "$?"Length of output: 201418 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- entrypoint terminal collection ---'
sed -n '200,368p' scripts/ci/actions_queue_health.py
printf '%s\n' '--- core symbol map ---'
ast-grep outline scripts/ci/actions_queue_health_core.py --items all --view expanded
printf '%s\n' '--- core normalisation and report blocks ---'
rg -n -C 6 '^def (_normalise_run|build_report|_normalise_job|_run_is_pending|_job_is_pending)\b|execution_state|is_pending|runner_assigned|steps_count' \
scripts/ci/actions_queue_health_core.py
printf '%s\n' '--- focused queue-health test file inventory ---'
fd -t f -E '__pycache__' 'actions_queue_health' tests | sort
printf '%s\n' '--- focused queue-health test contract locations ---'
rg -n -C 8 'startup_failure|cancelled_before_runner_assignment|runner_assigned|steps_count|terminal.*failure|execution_state' \
tests/test_actions_queue_health*.pyLength of output: 53197 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- core job classification and report construction ---'
sed -n '267,740p' scripts/ci/actions_queue_health_core.py
printf '%s\n' '--- queue-health report contract tests ---'
sed -n '1,260p' tests/test_actions_queue_health_contract.py
sed -n '820,1010p' tests/test_actions_queue_health.py
printf '%s\n' '--- all queue-health failed-job fixture assertions ---'
rg -n -i -C 10 \
'conclusion["'"'"']?\s*:\s*["'"'"']failure|conclusion.*FAILURE|terminal_job_count|execution_state.*terminal|runner_id.*[1-9]|steps.*\[[^]]' \
tests/test_actions_queue_health*.pyLength of output: 62990
The collector otherwise has the required behavior:
I did not execute repository tests because the review environment permits source inspection only. You are interacting with an AI system. |
|
Review finding verification on successor I added that exact aggregate invariant as a regression assertion in |
|
@coderabbitai review Re-review current exact |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
base='42bb922f03bf75aed1bc1931d9fbaf04a5433e20'
fix='b016348d1ed35304eb2504d30093922dbf2d42ae'
head='ecbdc3507148e2ab922deba8432db87c129cf433'
git cat-file -e "${base}^{commit}"
git cat-file -e "${fix}^{commit}"
git cat-file -e "${head}^{commit}"
git merge-base --is-ancestor "$base" "$head"
printf '%s\n' '--- exact commits ---'
git show -s --format='commit=%H%nparents=%P%nsubject=%s' "$fix" "$head"
printf '%s\n' '--- paths base..head ---'
git diff --name-status "$base" "$head"
printf '%s\n' '--- successor diff fix..head ---'
git diff --check "$fix" "$head"
git diff --unified=80 "$fix" "$head" -- tests/test_actions_queue_health_terminal_preexecution.py scripts/ci/actions_queue_health.py
printf '%s\n' '--- full scoped diff base..head ---'
git diff --check "$base" "$head"
git diff --unified=50 "$base" "$head" -- scripts/ci/actions_queue_health.py tests/test_actions_queue_health_terminal_preexecution.py
printf '%s\n' '--- entrypoint outline ---'
ast-grep outline scripts/ci/actions_queue_health.py --items all --view expanded
printf '%s\n' '--- core outline ---'
ast-grep outline scripts/ci/actions_queue_health_core.py --items all --view expanded
printf '%s\n' '--- report construction and wrapper relabeling references ---'
rg -n -C 12 '_CORE_BUILD_REPORT|terminal_job_count|terminal_pre_execution_failure|TERMINAL_DIAGNOSTIC_STATUSES|startup_failure|cancelled_before_runner_assignment' \
scripts/ci/actions_queue_health.py scripts/ci/actions_queue_health_core.py tests/test_actions_queue_health_terminal_preexecution.py
printf '%s\n' '--- relevant implementation slices ---'
sed -n '180,520p' scripts/ci/actions_queue_health.py
sed -n '500,720p' scripts/ci/actions_queue_health_core.py
printf '%s\n' '--- regression fixture ---'
cat -n tests/test_actions_queue_health_terminal_preexecution.pyLength of output: 79380
The prior I found no different causal defect in the scoped changes. The collector preserves the current-head I performed source inspection only. I did not execute repository tests. You are interacting with an AI system. |
|
Fresh exact-head queue evidence on This is stronger evidence than generic runner starvation: the exact workflow allowed the compatibility shards to consume/reject a missing verdict before the coordinator that creates the current-head dispatch receipt was able to execute, and the coordinator then succeeded later. The overall CodeQL PR run remains FAILURE because those earlier shard failures were not converted into a terminal current-head verdict in this run. Treat this as central dispatch/verdict ordering/retry-control evidence, separate from #2213's |
There was a problem hiding this comment.
Pull request overview
OpenCode reviewed the current-head product diff. Coverage is a separate gate.
Changed files
scripts/ci/actions_queue_health.py— review and security gate shell pathtests/test_actions_queue_health_terminal_preexecution.py— regression suite
Changed behavior
flowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["CI script: actions_queue_health.py"]
S1 --> I1["review and security gate shell path"]
I1 --> R1["Review risk: CI script: actions_queue_health.py"]
R1 --> V1["bash -n plus Strix self-test"]
Evidence --> S2["Test: test_actions_queue_health_terminal_preexecution.py"]
S2 --> I2["regression suite"]
I2 --> R2["Review risk: Test: test_actions_queue_health_terminal_preexecution.py"]
R2 --> V2["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:
ecbdc3507148e2ab922deba8432db87c129cf433 - Workflow run: 35044119068
- 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["CI script: actions_queue_health.py"]
S1 --> I1["review and security gate shell path"]
I1 --> R1["Review risk: CI script: actions_queue_health.py"]
R1 --> V1["bash -n plus Strix self-test"]
Evidence --> S2["Test: test_actions_queue_health_terminal_preexecution.py"]
S2 --> I2["regression suite"]
I2 --> R2["Review risk: Test: test_actions_queue_health_terminal_preexecution.py"]
R2 --> V2["targeted test run"]
OpenCode Review Overview
Coverage evidence did not pass, so approval is blocked. The formal pull-request review is the source-backed diff review, not this status comment. |
Preserve the queued job timing contract after rebasing onto main so queue age uses job eligibility, not the parent run start. Co-authored-by: Cursor <cursoragent@cursor.com>
0616226 to
4b2c426
Compare
Parent owner: #1150. Rebased onto current
mainafter #2219 (eb993211e). Incident evidence: #712 and OriginWeave exact-head Actions specimens. Current exact head is4b2c426badc525f3412b3c669c1cebc5ec4a034a.Finding
The canonical read-only queue-health collector preserves
startup_failureand pre-runnercancelleddiagnostics, but parent #1150 drops a distinct terminal class: a current-head run can endcompleted/failurewith a materialized failed job that has no runner assignment andsteps=[]. Becausefailureis excluded fromTERMINAL_DIAGNOSTIC_STATUSES, that run is filtered before job evidence is fetched. If such a job is supplied offline, the core report otherwise labels the row generic terminal evidence.That state must remain fail-closed and distinct from a product/security test that actually obtained a runner and executed steps.
RED → minimal causal repair
Test-first adds one behavior-level regression fixture for terminal pre-execution failure. Production repair in
scripts/ci/actions_queue_health.py: completedfailurejoins terminal diagnostics; only a current-head failed job with no runner and zero executed steps is reclassified asterminal_pre_execution_failure. A normal failed job with a runner or executed steps remains the ordinary terminal product/security failure path.Successor commits preserve
terminal_job_countaggregate accounting and queued current-head job timing evidence (job_created_atover parent-run start). After the main rebase, queued current-head runs also fetch job evidence so that timing contract stays green.Effective paths:
scripts/ci/actions_queue_health.pytests/test_actions_queue_health.pytests/test_actions_queue_health_terminal_preexecution.pytests/test_actions_queue_health_queued_job_evidence.pyNo workflow trigger, runner selector, token/credential, cancellation/rerun behavior, leaf repository, ruleset, required check, or success conclusion is changed.
Acceptance
Ready status admits the exact head to hosted checks and independent review; it is not merge readiness. Do not merge until applicable exact-head checks are terminal and valid findings are resolved. The collector must preserve repository/PR/head/run/job identity and must never translate this pre-execution terminal failure into success, ordinary executed test failure, or pending runner capacity. If source changes are needed, return the PR to Draft while repairing. No self-approval, bypass, force update, destructive rebase, blind rerun, workflow/gate weakening, merge, tag, or release is authorized by this child.
Local verification