Skip to content

fix(hw-gate): a stale decision.json blocked #702 with #682's verdict - #721

Merged
Kaden-Schutt merged 1 commit into
masterfrom
fix/hw-gate-decision-attribution
Sep 5, 2026
Merged

fix(hw-gate): a stale decision.json blocked #702 with #682's verdict#721
Kaden-Schutt merged 1 commit into
masterfrom
fix/hw-gate-decision-attribution

Conversation

@Kaden-Schutt

Copy link
Copy Markdown
Collaborator

#702 was blocked tonight by #682's verdict.

#702's 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 this same hazard for fable-evidence/ and fable-home/. decision.json was missed, and it is the worse case, because that file is the gate's verdict rather than an input to it.

Fix

Cleaning is necessary but not sufficient, so both:

  1. The decide step removes a stale decision.json alongside the evidence dirs, so the common case cannot arise.
  2. review.py records base/head in decision.json, and the status job refuses a decision whose head is not this run's head:
::error::decision artifact is for <sha> but this run is <sha> — 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 the field warn rather than fail, so an in-flight run does not break on merge.

Worth noting 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.

Policy-floor by construction, so it cannot self-merge.

@hipfire-sol

hipfire-sol Bot commented Sep 5, 2026

Copy link
Copy Markdown

hw-gate sol prelim

summary: The change hardens the hardware-gate decision artifact against reuse across commits: the Fable investigation step deletes any existing decision.json, review.py records the base and head SHAs in each new decision, and the status job rejects an attributed decision whose head differs from the current PR head while warning for legacy unattributed artifacts. The added unit test verifies that review.py writes both commit fields; it does not exercise the workflow’s cleanup or rejection branches.

run_hardware: true
run_hardware_reasons: The changed workflow and review script are readable and contain no obfuscated behavior or new dependencies, network operations, credential access, or process execution beyond the gate’s existing GitHub and reviewer machinery.; The filesystem change is bounded to deleting per-run files and directories in the reused self-hosted runner workspace: fable-evidence, fable-home, and decision.json.; The status validation only reads the downloaded decision artifact with jq and compares its recorded head to the head SHA already resolved by the select job.; This is a policy-floor change, so it requires execution and human review even though no model fixture route is relevant.

routes:

mode tag source why
no routes

unavailable_routes:

(none)

claim_assessment: The author claims that generated decisions now identify the base and head commits, stale attributed decisions are rejected by the status job, and the reused workspace is cleaned before an investigating Fable run. The new unit test can prove only the first claim. The cleanup and status behavior require workflow-level or shell-equivalent tests covering a mismatched head, a matching head, a legacy missing head, and a decide failure before writing a replacement artifact; model generation fixtures do not prove these policy behaviors.

questions_for_author:

  • What automated test exercises the status shell’s mismatched-head rejection and confirms that a failed decide phase cannot publish an existing decision.json?
  • Why should an unattributed legacy artifact remain authoritative after merely warning, given that such an artifact is exactly the class that cannot be distinguished from a stale reused-workspace verdict?

@hipfire-sol

hipfire-sol Bot commented Sep 5, 2026

Copy link
Copy Markdown

hw-gate evidence — 2 lane(s) — verdict pass

lane hiptrx (gfx1201)

hw-gate evidence

field value
base 4d6103bc11a4914b79b19e24b193fcb9d27a0d61
head 946d60b274300f3be2c2f8c5ba28515c85d22994
buckets
host gfx gfx1201
host rocm 7.15.26333-0000000
device 1
runner hiptrx
daemon_md5 17ba7dfab49aa1c1110d3f412f072648
hipfire_md5 21dce4bf43980ab638fae2f11ef782e5
build_seconds 40.59495449066162
verdict pass
logs_dir hw-gate-logs

fixtures

no fixtures

kernel

not run

lane hipx (gfx1100)

hw-gate evidence

field value
base 4d6103bc11a4914b79b19e24b193fcb9d27a0d61
head 946d60b274300f3be2c2f8c5ba28515c85d22994
buckets
host gfx gfx1100
host rocm 7.15.26333-0000000
device 0
runner hipx
daemon_md5 17ba7dfab49aa1c1110d3f412f072648
hipfire_md5 21dce4bf43980ab638fae2f11ef782e5
build_seconds 5.945122003555298
verdict pass
logs_dir hw-gate-logs

fixtures

no fixtures

kernel

not run

@hipfire-sol

hipfire-sol Bot commented Sep 5, 2026

Copy link
Copy Markdown

hw-gate sol verdict

{
  "claim_verdict": "not-exercised",
  "confidence": 0.94,
  "coverage": {
    "gaps": [
      "No test exercises the status shell with matching, mismatched, or missing `.head` values.",
      "No evidence demonstrates stale decision.json cleanup when seat preflight fails; that branch bypasses the cleanup at .github/workflows/hw-gate.yml:485.",
      "Legacy decisions without `.head` remain authoritative after only a warning, so their provenance cannot be verified."
    ],
    "surfaces_evidenced": [
      "decision attribution serialization"
    ],
    "surfaces_touched": [
      "policy",
      "ci",
      "filesystem"
    ]
  },
  "decision": "needs-human",
  "eyeball": [
    ".github/workflows/hw-gate.yml:450-506: the unavailable-decider branch runs before the cleanup embedded in the investigating branch.",
    ".github/workflows/hw-gate.yml:590-599: confirm whether accepting an unattributed legacy artifact is intended policy."
  ],
  "phase": "verdict",
  "rationale": "Both hardware lanes passed, but they had no fixture routes and do not exercise this policy change. The diff proves review.py records base/head and the investigating path removes decision.json, but the cleanup is not shared by the unavailable-decider path and no test covers the workflow rejection logic. Because this changes required-status policy and leaves material workflow coverage gaps, it requires human review.",
  "regressions": []
}

Floor: hard=['policy_paths: .github/workflows/hw-gate.yml,scripts/hw-gate/review.py,scripts/hw-gate/tests/test_review.py'] soft=["coverage_gaps: ['No test exercises the status shell with matching, mismatched, or missing .head values.', 'No evidence demonstrates stale decision.json cleanup when seat preflight fails; that branch bypasses the cleanup at .github/workflows/hw-gate.yml:485.', 'Legacy decisions without .head remain authoritative after only a warning, so their provenance cannot be verified.']", 'model needs-human'] model_decision=needs-human final=needs-human

@hipfire-sol hipfire-sol Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

hw-gate sol verdict needs-human: The diff adds the intended defenses: decision.json cleanup, base/head attribution in scripts/hw-gate/review.py, and rejection of a nonempty mismatched head in the Status step. Both hardware lanes passed, but no route exercised this policy-only artifact lifecycle. Policy changes and the untested stale-artifact CI scenario require human review.

…judged

#702 was blocked tonight by #682's verdict.

Its own lanes were 8/8 pass (run 33929015500, head 87233ea, 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 <sha> but this run is <sha> -- 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.
@Kaden-Schutt
Kaden-Schutt force-pushed the fix/hw-gate-decision-attribution branch from e84a645 to 946d60b Compare September 5, 2026 00:56
@Kaden-Schutt
Kaden-Schutt merged commit d1da3ea into master Sep 5, 2026
9 checks passed

@hipfire-sol hipfire-sol Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

hw-gate sol verdict needs-human: Both hardware lanes passed, but they had no fixture routes and do not exercise this policy change. The diff proves review.py records base/head and the investigating path removes decision.json, but the cleanup is not shared by the unavailable-decider path and no test covers the workflow rejection logic. Because this changes required-status policy and leaves material workflow coverage gaps, it requires human review.

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