mosaicstack/stack
1
0
Fork 1
Code Issues 109 Pull Requests 1 Actions Packages Projects Releases 15 Wiki Activity

ci-queue-wait.sh: an unparseable status is dispositioned as PROCEED — the guard cannot distinguish "checked, clear" from "could not check" #950

New Issue
Open
opened 2026-07-30 14:25:07 +00:00 by Mos · 0 comments
Mos commented 2026-07-30 14:25:07 +00:00
Contributor
Copy Link
Copy Source

Two defects, one line apart. The second is the serious one and is fixable without touching the first.

Defect 1 — the classifier is a total no-op

get_state_from_status_json() reads the API response via python3 - <<'PY' … PY. The heredoc is stdin, so json.load(sys.stdin) reads the script body, never the JSON. The function can only ever return unknown — for a green status, a failing status, a pending status, or a malformed one alike.

Verified by execution: valid / failure / pending JSON all → unknown.

Defect 2 — unknown maps to PROCEED

The disposition table treats unknown as a pass. So the guard emits state=unknown and exits 0, and every caller reads that as "the queue is clear".

An instrument that cannot distinguish "I checked and it is fine" from "I could not check" must fail closed.

Why this is not cosmetic — a measured cost

This guard exists to prevent a merge landing on top of in-flight CI. It was run before a merge, printed state=unknown, and exited 0. A pipeline was running on the target branch. The merge proceeded, a new push superseded that pipeline, and the CI provider killed it in flight — destroying a pre-registered observation that a reviewer was actively watching.

Had the disposition been HOLD, the guard would have blocked the merge with an honest "I could not determine the queue state", and the observation would have survived. The run that died was the only one scheduled to answer the question it was watching.

This was a documented defect with no demonstrated consequence for some time. It now has one.

Fix — do part 2 first

Part 2 (one line, correct regardless): map unknown → HOLD, not PROCEED.

This is correct even if the parser is never repaired, because a guard that cannot read the queue should refuse rather than wave through. It removes the entire fail-open class in a single edit and does not depend on diagnosing the heredoc.

Part 1: fix the parser — feed the JSON on stdin (printf '%s' "$json" | python3 - <<'PY' is still wrong; pass via argv, a file, or a heredoc-free python3 -c).

Acceptance

  • Valid green status → PROCEED, and a negative control proving the guard can also emit HOLD.
  • Valid busy/pending status → HOLD.
  • Unparseable / empty / API-error response → HOLD, with a message distinguishing "could not determine" from "determined busy".
  • A test asserting the guard exits non-zero on an unparseable response — otherwise nobody has ever seen the hold fire.

Related

Same failure direction as the census tool that silently omits a step from its step list: both render an incomplete or unavailable result as a complete, satisfactory one. They are not the same incident — that one's measured cost is still zero — but the class is shared: a tool must not present the absence of a check as the success of one.

## Two defects, one line apart. The second is the serious one and is fixable without touching the first. ### Defect 1 — the classifier is a total no-op `get_state_from_status_json()` reads the API response via `python3 - <<'PY' … PY`. **The heredoc *is* stdin**, so `json.load(sys.stdin)` reads the script body, never the JSON. The function can only ever return `unknown` — for a green status, a failing status, a pending status, or a malformed one alike. Verified by execution: valid / failure / pending JSON all → `unknown`. ### Defect 2 — `unknown` maps to PROCEED The disposition table treats `unknown` as a pass. So the guard emits `state=unknown` and exits 0, and every caller reads that as "the queue is clear". > **An instrument that cannot distinguish "I checked and it is fine" from "I could not check" must fail closed.** ## Why this is not cosmetic — a measured cost This guard exists to prevent a merge landing on top of in-flight CI. It was run before a merge, printed `state=unknown`, and exited 0. A pipeline **was** running on the target branch. The merge proceeded, a new push superseded that pipeline, and the CI provider **killed it in flight** — destroying a pre-registered observation that a reviewer was actively watching. Had the disposition been HOLD, the guard would have blocked the merge with an honest "I could not determine the queue state", and the observation would have survived. The run that died was the *only* one scheduled to answer the question it was watching. This was a documented defect with no demonstrated consequence for some time. It now has one. ## Fix — do part 2 first **Part 2 (one line, correct regardless):** map `unknown` → **HOLD**, not PROCEED. This is correct *even if the parser is never repaired*, because a guard that cannot read the queue should refuse rather than wave through. It removes the entire fail-open class in a single edit and does not depend on diagnosing the heredoc. **Part 1:** fix the parser — feed the JSON on stdin (`printf '%s' "$json" | python3 - <<'PY'` is still wrong; pass via argv, a file, or a heredoc-free `python3 -c`). ## Acceptance - Valid green status → PROCEED, and a **negative control** proving the guard can also emit HOLD. - Valid busy/pending status → HOLD. - Unparseable / empty / API-error response → **HOLD**, with a message distinguishing "could not determine" from "determined busy". - A test asserting the guard exits non-zero on an unparseable response — otherwise nobody has ever seen the hold fire. ## Related Same failure *direction* as the census tool that silently omits a step from its step list: both render an incomplete or unavailable result as a complete, satisfactory one. They are **not** the same incident — that one's measured cost is still zero — but the class is shared: **a tool must not present the absence of a check as the success of one.**
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
fleet-enhancement
Fleet-wide reliability/speed/cost improvements surfaced by the nightly enhance pass
No labels
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
jason.woltje
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: mosaicstack/stack#950