Skip to content

fix(queue): preserve terminal pre-execution failures - #2213

Merged
seonghobae merged 5 commits into
mainfrom
codex/queue-health-terminal-preexecution-failure
Sep 17, 2026
Merged

seonghobae merged 5 commits into
mainfrom
codex/queue-health-terminal-preexecution-failure

Conversation

@seonghobae

@seonghobae seonghobae commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

Parent owner: #1150. Rebased onto current main after #2219 (eb993211e). Incident evidence: #712 and OriginWeave exact-head Actions specimens. Current exact head is 4b2c426badc525f3412b3c669c1cebc5ec4a034a.

Finding

The canonical read-only queue-health collector preserves startup_failure and pre-runner cancelled diagnostics, but parent #1150 drops a distinct terminal class: a current-head run can end completed/failure with a materialized failed job that has no runner assignment and steps=[]. Because failure is excluded from TERMINAL_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: completed failure joins terminal diagnostics; only a current-head failed job with no runner and zero executed steps is reclassified as terminal_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_count aggregate accounting and queued current-head job timing evidence (job_created_at over 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.py
  • tests/test_actions_queue_health.py
  • tests/test_actions_queue_health_terminal_preexecution.py
  • tests/test_actions_queue_health_queued_job_evidence.py

No 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

python3 -m pytest tests/test_actions_queue_health*.py -q
# 74 passed

@coderabbitai

coderabbitai Bot commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

Next included review available in 14 minutes.

Check out review usage here.

View limit details

Limit 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.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 3763e13d-1f6f-4e5c-822a-3fbba417ada7

📥 Commits

Reviewing files that changed from the base of the PR and between eb99321 and 4b2c426.

📒 Files selected for processing (4)
  • scripts/ci/actions_queue_health.py
  • tests/test_actions_queue_health.py
  • tests/test_actions_queue_health_queued_job_evidence.py
  • tests/test_actions_queue_health_terminal_preexecution.py

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

Please review exact head b016348d1ed35304eb2504d30093922dbf2d42ae against parent #1150 exact 42bb922f03bf75aed1bc1931d9fbaf04a5433e20. Focus on whether completed/failure + materialized zero-step/no-runner jobs are preserved as fail-closed terminal pre-execution evidence without reclassifying ordinary runner-backed failures or weakening existing startup/cancelled semantics.

@coderabbitai

coderabbitai Bot commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

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 expanded

Length 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*.py

Length 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*.py

Length of output: 62990


@seonghobae I found one summary-accounting defect.

  • scripts/ci/actions_queue_health.py, Line 469: The code changes execution_state from the core value "terminal" to "terminal_pre_execution_failure". The core summary computes terminal_job_count from execution_state == "terminal" before this override. Therefore, a report that contains only this terminal failure reports terminal_job_count: 0. This loses the terminal classification in aggregate data and can make terminal-failure monitoring undercount. Keep the distinct execution state, but update terminal_job_count to include "terminal_pre_execution_failure", or compute the count before the override. Add an assertion for terminal_job_count == 1 to tests/test_actions_queue_health_terminal_preexecution.py.

The collector otherwise has the required behavior:

  • It includes failure in the completed current-head terminal diagnostic query.
  • It fetches job evidence before report classification.
  • It requires current-head identity, failed run, failed job, no runner assignment, and exactly zero steps.
  • It leaves runner-backed or stepped failures as the normal "terminal" path.
  • It does not change the startup failure or cancelled predicates.
  • The target state remains non-pending and non-successful.

I did not execute repository tests because the review environment permits source inspection only.

You are interacting with an AI system.

@seonghobae
seonghobae marked this pull request as draft September 15, 2026 14:09

Copy link
Copy Markdown
Contributor Author

Review finding verification on successor ecbdc3507148e2ab922deba8432db87c129cf433: the aggregate undercount claim is not reproduced by the current call order. _CORE_BUILD_REPORT(...) constructs rows with the core _job_state, where this failed job is still execution_state="terminal", and computes summary["terminal_job_count"] from those core rows before the wrapper re-labels the returned row as terminal_pre_execution_failure. The wrapper does not recompute or decrement that existing summary field, so a single specimen retains terminal_job_count == 1.

I added that exact aggregate invariant as a regression assertion in ecbdc350...; no production change is warranted for this review comment. The original owner fix remains b016348d.... Please re-review the current head and flag any different causal defect; do not infer a RED that the actual report construction order does not produce.

@seonghobae
seonghobae marked this pull request as ready for review September 15, 2026 14:11

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

Re-review current exact ecbdc3507148e2ab922deba8432db87c129cf433. The only successor after your prior review is the aggregate invariant assertion summary["terminal_job_count"] == 1; source inspection of _CORE_BUILD_REPORT shows it computes that count while the row is still core terminal, before the wrapper relabels the returned row. Please verify the current head rather than transferring the previous undercount finding.

@coderabbitai

coderabbitai Bot commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

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.py

Length of output: 79380


@seonghobae Re-review of exact head ecbdc3507148e2ab922deba8432db87c129cf433 is complete.

The prior terminal_job_count finding is withdrawn. scripts/ci/actions_queue_health.py calls _CORE_BUILD_REPORT(...) before Line 469 relabels the row. The core report computes summary["terminal_job_count"] while the row has execution_state == "terminal". The wrapper does not recompute or decrement that value. The added assertion correctly verifies this behavior.

I found no different causal defect in the scoped changes. The collector preserves the current-head completed/failure specimen, fetches its job evidence, and classifies only the zero-step, no-runner failed job as terminal_pre_execution_failure. The runner-backed or stepped failure path remains terminal. The existing startup-failure and cancelled predicates remain unchanged.

I performed source inspection only. I did not execute repository tests.

You are interacting with an AI system.

seonghobae commented Sep 15, 2026

Copy link
Copy Markdown
Contributor Author

Fresh exact-head queue evidence on ecbdc3507148e2ab922deba8432db87c129cf433, updated after the dispatch job finally ran: Semgrep and Security Scan are SUCCESS, and CodeQL language detection ran successfully on hosted runner 1001987520. Compatibility jobs python 104501557063 (runner 1001989212) and actions 104501557079 (runner 1001989220) both executed earlier and failed specifically at Release runner or enforce current-head CodeQL verdict after their verdict-read step. The same run's coordinator Dispatch current-head CodeQL scan job 104577757456, which was still pre-runner queued when this note was first posted, was later admitted on runner 1001991159 at 2026-09-16 01:12:55Z and completed SUCCESS at 01:13:04Z.

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 completed/failure + runner_id=0 + steps=[] classifier repair. Preserve the exact head/run/job identities; no blind rerun or OriginWeave consumer workaround is warranted.

@opencode-agent opencode-agent Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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 path
  • tests/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"]
Loading

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"]
Loading

@opencode-agent

opencode-agent Bot commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

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.

@seonghobae
seonghobae marked this pull request as draft September 17, 2026 08:05
Base automatically changed from codex/pr1142-current-main-successor to main September 17, 2026 15:58
@seonghobae
seonghobae force-pushed the codex/queue-health-terminal-preexecution-failure branch from 0616226 to 4b2c426 Compare September 17, 2026 17:51
@seonghobae
seonghobae marked this pull request as ready for review September 17, 2026 17:51
@seonghobae
seonghobae merged commit 64aa08d into main Sep 17, 2026
4 of 28 checks passed
@seonghobae
seonghobae deleted the codex/queue-health-terminal-preexecution-failure branch September 17, 2026 17:59
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.

1 participant