From 946d60b274300f3be2c2f8c5ba28515c85d22994 Mon Sep 17 00:00:00 2001 From: Kaden Schutt <151092359+Kaden-Schutt@users.noreply.github.com> Date: Sat, 5 Sep 2026 00:03:09 +0000 Subject: [PATCH] fix(hw-gate): make a decision artifact attributable to the commit it judged #702 was blocked tonight by #682's verdict. Its own lanes were 8/8 pass (run 33929015500, head 87233eae1, evidence `verdict: pass`), yet the run published a decision carrying `hw_run_result=failure`, `evidence verdict='fail'`, and an announcement about "six refused loads ... master says 'no model loaded' for four of them" -- which is #682's source-aware admission work, not a DFlash kernel PR. Cause: the runner workspace is reused and `upload-artifact` runs `if: always()`. #702's decide phase failed before writing its own decision.json, so the file left behind by the previous run on that runner -- #682's re-gate -- was uploaded as `hw-gate-decision` for #702, and the status job read it and blocked the PR. #705 fixed the same hazard for `fable-evidence/` and `fable-home/`; decision.json was missed, and it is worse, because that file is the gate's verdict rather than an input to it. Two changes, because cleaning is necessary but not sufficient: 1. The decide step removes a stale `decision.json` alongside the evidence dirs, so the common case cannot arise. 2. review.py records `base` and `head` in decision.json, and the status job refuses a decision whose `head` is not this run's head: "decision artifact is for but this run is -- stale decision.json from a reused workspace; re-run the gate". An artifact from another commit is a gate malfunction, not a verdict, so it fails as one instead of being obeyed. Artifacts predating this field warn rather than fail, so an in-flight run does not break on merge. The lane evidence already carried base/head for exactly this reason (hw-gate.json records both); the decision did not. Test: `test_decision_records_the_commit_it_judged` asserts both fields match the commit under review. 107/107 hw-gate tests pass. --- .github/workflows/hw-gate.yml | 23 ++++++++++++++++++++++- scripts/hw-gate/review.py | 11 +++++++++++ scripts/hw-gate/tests/test_review.py | 23 +++++++++++++++++++++++ 3 files changed, 56 insertions(+), 1 deletion(-) diff --git a/.github/workflows/hw-gate.yml b/.github/workflows/hw-gate.yml index 32317af72..891dfde46 100644 --- a/.github/workflows/hw-gate.yml +++ b/.github/workflows/hw-gate.yml @@ -476,7 +476,13 @@ jobs: # previous PR's evidence files and home (run 33866758629 on #702 # carried #700's fable-summary.md and #686's route outputs) and can # cite them as its own. Every session starts empty. - rm -rf fable-evidence fable-home + # decision.json is the same hazard, one level worse: upload-artifact + # runs `if: always()`, so a decide phase that dies before writing its + # own file publishes the PREVIOUS run's verdict as this run's. #702's + # decide job failed and the run published #682's decision -- "six + # refused loads ... master says 'no model loaded'" -- blocking a PR + # whose own lanes were 8/8 pass. + rm -rf fable-evidence fable-home decision.json mkdir -p fable-evidence fable-home exec 9>/home/kaden/actions-runner/_cache/hw-gate-gpu.lock flock --exclusive --timeout 3600 9 @@ -575,6 +581,21 @@ jobs: # floor-applied verdict under `.decision_final`; `.decision` is a JSON # object, so the old jq yielded "[object]" and every run — including # a successful merge-staging (#689, run 33889229683) — went red. + # A decision is only this PR's if it names this PR's head. The runner + # workspace is reused and upload-artifact runs `if: always()`, so a + # decide phase that dies can publish the previous run's verdict: + # #702 was blocked on #682's decision while its own lanes were 8/8 + # pass. An artifact from another commit is a gate malfunction, not a + # verdict, so it fails as one rather than being obeyed. + want_head="${{ needs.select.outputs.head_sha }}" + got_head=$(jq -r '.head // ""' decision/decision.json) + if [ -n "$got_head" ] && [ "$got_head" != "$want_head" ]; then + echo "::error::decision artifact is for $got_head but this run is $want_head — stale decision.json from a reused workspace; re-run the gate" + exit 1 + fi + if [ -z "$got_head" ]; then + echo "::warning::decision artifact predates head attribution; cannot verify it belongs to $want_head" + fi decision=$(jq -r '.decision_final // .decision.decision // "hold"' decision/decision.json) hard=$(jq -r '(.floor.hard // []) | join(", ")' decision/decision.json) echo "decision=$decision hard_floor=[$hard] human_reviewed=${{ needs.select.outputs.human_reviewed }}" diff --git a/scripts/hw-gate/review.py b/scripts/hw-gate/review.py index d810d3754..3a9fda187 100755 --- a/scripts/hw-gate/review.py +++ b/scripts/hw-gate/review.py @@ -1690,6 +1690,17 @@ def _has_hold(r: str) -> bool: "version": 1, "seat": "fable", "model": model, + # The decision must be attributable to the commit it judged. The + # runner workspace is reused, so a decide phase that dies before + # writing its own decision.json leaves the PREVIOUS run's file in + # place, and `upload-artifact: if: always()` publishes it as this + # run's decision. That happened on 2026-09-04: #702's decide job + # failed and the run published #682's verdict -- "six refused loads + # ... master says 'no model loaded'" -- blocking a PR whose own lanes + # were 8/8 pass. The status job cross-checks these against the run's + # head, so a stale artifact is caught instead of obeyed. + "base": args.base, + "head": args.head, "decision": decision, "floor": {"hard": hard, "soft": soft}, "decision_final": decision_final, diff --git a/scripts/hw-gate/tests/test_review.py b/scripts/hw-gate/tests/test_review.py index b7626f615..d3a661dc8 100644 --- a/scripts/hw-gate/tests/test_review.py +++ b/scripts/hw-gate/tests/test_review.py @@ -874,3 +874,26 @@ class R: returncode, stdout, stderr = 0, "", "" assert "--thinking" in cmd and cmd[cmd.index("--thinking") + 1] == "xhigh" assert cmd[cmd.index("--max-time") + 1] == f"{minutes}m" assert "GH_TOKEN" not in captured["env"] and captured["env"]["HW_GATE_DEVICES"] == "0,1,2,3,4" + + +def test_decision_records_the_commit_it_judged(): + """A decision is only usable if it names the commit it judged. + + The runner workspace is reused and upload-artifact runs `if: always()`, so a + decide phase that dies publishes the previous run's decision.json as this + run's. On 2026-09-04 #702 was blocked by #682's verdict -- announcement and + all -- while #702's own lanes were 8/8 pass. The status job cross-checks + `.head`, which only works if review.py records it. + """ + tmp = Path(tempfile.mkdtemp()) + result, out_path, gh_log, gh_comments, omp_log, base, head, checkout = _run_decide( + tmp, sol_final="greenlight", + fable_response={"phase": "decide", "decision": "merge-staging", "agrees_with_sol": True, + "override": None, "regressions": [], "further_evidence_wanted": [], + "rationale": "r", "announcement": "a"}, + ) + data = json.loads(out_path.read_text()) + assert data["head"] == head, data.get("head") + assert data["base"] == base, data.get("base") + +