Skip to content

fix(hw-gate): never let a dead decide seat hold the GPU lock, never re-gate a merged PR - #712

Merged
Kaden-Schutt merged 2 commits into
masterfrom
fix/hw-gate-lock-and-merged-guard
Sep 4, 2026
Merged

fix(hw-gate): never let a dead decide seat hold the GPU lock, never re-gate a merged PR#712
Kaden-Schutt merged 2 commits into
masterfrom
fix/hw-gate-lock-and-merged-guard

Conversation

@Kaden-Schutt

Copy link
Copy Markdown
Collaborator

Two defects found while driving the ladder tonight. Both cost live rungs their hardware lane, and both are gate plumbing rather than product code.

1 · A decide seat that cannot answer holds all five GPUs for 45 minutes

fable-decide takes flock --exclusive on hw-gate-gpu.lock around the whole review.py --phase decide process (workflow line 411), for its full HW_GATE_MAX_MINUTES budget. Tonight #711's decide phase had no provider credits and could never produce a verdict — yet it held that lock for ~20 minutes while #687 and #688 sat queued with hardware (gfx1201) unable to start. I only found it by fuser-ing the lock file and seeing a --pr 711 process on a PR that merged at 19:16.

A seat that cannot answer must not own the hardware.

Fix: one 90 s toolless probe of the decide model before the lock is taken.

  • probe replies → the locked phase runs exactly as before, unchanged
  • probe fails → the locked step is skipped entirely, and a new unlocked step records the hold via --decider-unavailable REASON, which short-circuits the model call in review.py and lets the floors, PR comment, and labels run as usual

The GPUs are never claimed, and the PR still gets a comment explaining why it held. This is not a fallback model or a retry — an out-of-credit seat still fails, it just fails without taking the hardware down with it.

2 · A merged PR can re-run the entire gate

pull_request_target fires on labeled even for a closed PR, and the gate itself applies labels. Run 33915818350 re-ran the whole gate on #711 five minutes after it merged, taking the runner and the exclusive lock from live rungs; the same thing happened again on #691, #686, #687 and #688 after each staging merge — I cancelled six of these by hand tonight.

Fix: select refuses any event whose PR is already merged. workflow_dispatch is explicitly unaffected, so a manual re-gate of a merged PR still works when you want one.

Test

test_decide_unavailable_seat_holds_without_calling_the_model asserts the hold, the recorded reason, no merge — and no model invocation at all (the fake never opens its log, so the assertion is not omp_log.exists()). 105/105 hw-gate tests pass.

Touches .github/workflows/hw-gate.yml and scripts/hw-gate/**, so it is policy-floor by construction and cannot self-merge.

…e-gate a merged PR

Two defects observed on 2026-09-04 while driving the ladder, both of which
cost live rungs their hardware lane:

1. The decide phase takes `flock --exclusive` on all five GPUs around the
   whole `review.py --phase decide` process, for its full 45-minute budget.
   #711's seat had no provider credits and could never produce a verdict, yet
   it held that lock for ~20 minutes while #687 and #688 sat queued with
   'hardware (gfx1201)' unable to start. A seat that cannot answer must not
   own the hardware.

   Fixed with a preflight: one 90 s toolless probe of the decide model before
   the lock is taken. If it replies, the locked phase runs exactly as before.
   If it does not, the locked step is skipped entirely and a new unlocked step
   records the hold via `--decider-unavailable REASON`, which short-circuits
   the model call in review.py and lets the floors, comment, and labels run as
   usual. The GPUs are never claimed.

2. `pull_request_target` fires on `labeled` even for a merged PR, so label
   churn re-ran the entire gate on #711 five minutes after it merged (run
   33915818350) — taking the runner and the exclusive lock from live rungs.
   `select` now refuses any event whose PR is already merged; `workflow_dispatch`
   is unaffected, so a manual re-gate still works.

Test: `test_decide_unavailable_seat_holds_without_calling_the_model` asserts
hold, the reason recorded, no merge, and no model invocation at all (the fake
never opens its log). 105/105 hw-gate tests pass.
@hipfire-sol

hipfire-sol Bot commented Sep 4, 2026

Copy link
Copy Markdown

hw-gate sol prelim

summary: The change skips automatic gate runs for already-merged PRs, probes the Fable decide provider before acquiring the exclusive GPU lock, records an unavailable-seat hold without invoking the model, and changes serve/Redline validation to use harnesses from the gate checkout rather than the PR checkout. The preflight failure path currently interpolates provider-controlled stderr into generated shell source, creating a command-injection path on the self-hosted runner.

run_hardware: false
run_hardware_reasons: The workflow introduces a new networked omp process and carries its externally produced stderr into shell source on a self-hosted runner. This is an unaccounted process-execution boundary and must not run until the reason is transported as data rather than executable shell text.; GPU fixtures cannot validate these policy and orchestration changes; the appropriate evidence is focused hw-gate unit tests plus a workflow-level failure-path test covering hostile stderr.; The gate-root harness selection itself is bounded and improves oracle integrity: PR binaries remain under test while serve_harness.py and redline_daemon_harness.py come from the trusted gate checkout.

routes:

mode tag source why
no routes

unavailable_routes:

(none)

claim_assessment: The merged-PR guard and unavailable-decider short circuit are visible in the diff, and the review.py unit test exercises that no model invocation or merge occurs after --decider-unavailable. The claimed safe unlocked workflow path is not proven because no test exercises GitHub Actions output transport and shell rendering, and that path is presently unsafe for shell metacharacters in probe stderr. The separate gate-checkout harness correction is supported by a focused serve-harness unit test, but the analogous Redline path lacks a focused test.

questions_for_author:

  • Can the unavailable reason be passed through a quoted environment variable or a file instead of ${{ steps.seat.outputs.reason }} interpolation, with a regression test containing command-substitution syntax in probe stderr?
  • Can the workflow-level test also prove the failed probe path never reaches the flock command, rather than only proving review.py skips its own model call?

Third defect from tonight's ladder, and the one with real teeth: the hardware
lanes execute `pr/scripts/serve_harness.py` — the branch's own copy of the
instrument that judges it.

#682 is a fork PR based on master from before #703, so its harness has no
`ATTRACTOR_MIN_WINDOW` guard. Run 33921475093 ran that stale copy and flagged
the 3-token answer `Answer: 43` as a token attractor on BOTH lanes, exiting 1
on a fixture whose own rows record finish=stop, gen=9, recall=1/1, attractor
false, empty false, runaway false. Two green lanes turned red and the rung
hard-floored on evidence that was correct.

Two consequences, one worse than the other:
- any PR older than a harness fix inherits that harness's bugs, which is a
  false-failure generator that gets worse as the harness improves
- a branch can weaken its own oracle, which is the part that must never be
  possible in a gate

Fixtures, registry, prompts and review.py already resolve against the gate
checkout for exactly this reason (see the `_gate_root` comment above the
battery-prompt resolution). serve_harness.py and redline_daemon_harness.py now
do too, including their existence preconditions. The PR still supplies the
binaries under test — only the instrument moved.

Test: `test_serve_harness_runs_from_gate_root_not_pr` writes a different
harness into each tree and asserts the gate's is the one invoked. 106/106
hw-gate tests pass.
@Kaden-Schutt

Copy link
Copy Markdown
Collaborator Author

Added a third defect, found ten minutes after opening this: the hardware lanes run the PR's own copy of the harness.

run.py resolved serve_harness.py from --repo pr, so #682 — a fork PR based on master from before #703 — ran the pre-guard harness and false-flagged the 3-token answer Answer: 43 as a token attractor on both lanes (run 33921475093), hard-flooring a rung whose own evidence rows all read attractor:false, recall_ok:true.

That is two problems in one: every PR older than a harness fix inherits that harness's bugs (a false-failure generator that worsens as the harness improves), and a branch can weaken the oracle that judges it — which must never be possible in a gate. Fixtures, registry, prompts and review.py already resolve against the gate checkout for exactly this reason; the two harnesses now do too, including their existence preconditions. The PR still supplies the binaries under test.

31ef8cb88, with test_serve_harness_runs_from_gate_root_not_pr writing a different harness into each tree and asserting the gate's is invoked. 106/106 hw-gate tests pass.

All three defects cost live rungs tonight: #687/#688 lost ~20 min to a credit-less seat holding the exclusive GPU lock, I hand-cancelled six re-gates of merged PRs, and #682 failed on a correct answer.

@Kaden-Schutt
Kaden-Schutt merged commit 0652500 into master Sep 4, 2026
12 of 13 checks passed
ghazni101 pushed a commit to ghazni101/hipfire that referenced this pull request Sep 5, 2026
A rung that merges to `beta` stays OPEN by design -- promoting beta -> master
is the maintainer's call -- so `pull_request.merged` is false and the
merged-PR guard from warpfront#712 does not apply. Every later touch of that branch then
re-runs the full gate on work that is already staged: warpfront#692 and warpfront#723 both
re-ran within minutes of their staging merges, taking the runner from live
rungs, and the same pattern accounted for several of the runs cancelled by hand
tonight.

`select` now asks whether the head is an ancestor of the staging branch. If it
is, the evidence exists and the hardware has nothing to add, so `run_hw` is
false: the lanes, Sol's verdict and the decide phase all skip, and the recorded
decision still governs the status. The PR is not touched and no label changes.

Deliberately an ancestor test rather than a SHA equality test: a rung merges as
a staging commit whose parent is the head, so equality would never match, and
an ancestor test also covers a rung whose branch was merged and then pushed
again without new work.

`workflow_dispatch` is unaffected, so a manual re-gate of a staged rung still
runs -- that is the escape hatch for re-measuring after a gate fix, which is
exactly what warpfront#702 needed tonight.

132/132 hw-gate tests pass; the workflow parses and the select job's step list
and `run_hw` expression were checked.
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