ROCm · demonsan · Sep 11, 2026
diff --git a/.agent_loop/.gitignore b/.agent_loop/.gitignore
@@ -0,0 +1,7 @@
+# installation-specific and never committed
+config/loop.json
+*.key
+id_*
+*.pem
+__pycache__/
+*.pyc
diff --git a/.agent_loop/README.md b/.agent_loop/README.md
@@ -0,0 +1,83 @@
+# .agent_loop
+
+An orchestration layer that drives the two review skills in this repo — `.claude/skills/review-pr`
+and `.claude/skills/validate-kernel-pr` — as one repeatable loop, so that a PR goes from a number
+to a gated verdict card plus deterministic on-GPU evidence without a human holding it by the hand.
+
+It is **not** a third skill. The skills decide *what* a review means; this decides *who runs what,
+in which order, and what may not be believed*. It calls their CLI surface and nothing else.
+
+```
+[once per host] environment probe → human confirms → env record
+[per PR]  phase P (prefetch): verify env + skill Step 1 fetch        ← cheap, 2 agents
+             ↓  ★ the single human break: targets, models, budget — asked once
+          phase R: analysis fan-out → independent refutation → card → seven gates
+                   → on-GPU validation → refute the executor → refold → cleanup   ← no stops
+```
+
+## Why a loop rather than one review pass
+
+Three things only a loop can do, each of which happened during the runs this was built from:
+
+- **It re-reviews the same PR as it moves.** When the base advances, a file the previous head added
+  and this head removed is invisible in the diff — it reads as if it never existed. Phase P reports
+  `git log <prev-head>..<head>` for exactly this reason.
+- **It puts the tools on trial too.** A validator finding is a claim, not a fact. Stage S6b refutes
+  the executor's own findings; on the runs behind this package that withdrew four false ones,
+  including a "comparison tolerance widened" verdict that was an artifact of positional matching.
+- **It makes the review record falsifiable.** Every finding on the card was attacked by a fresh
+  agent that never saw the reasoning behind it, and that agent — not the card's author — wrote the
+  ledger line recording the outcome.
+
+## Quick start
+
+```bash
+cp config/loop.example.json config/loop.json && $EDITOR config/loop.json
+
+node tools/dryrun.mjs            # zero-token: proves the guards fire. 10/10 or do not proceed.
+```
+
+Then, from an agent harness that provides a `workflow` tool (the scripts are workflow script
+bodies, not standalone programs):
+
+1. run `workflows/prefetch.workflow.js` with `args` built from your config plus `{ pr, sinceHead }`;
+2. **stop and ask a human** the four questions phase P's report makes answerable — which validation
+   targets (the skill refuses to pick when several candidates exist), which model per role, the
+   refutation budget, and whether the environment record still holds;
+3. run `workflows/run.workflow.js` with those answers. It does not stop again.
+
+## What it guarantees, and what it does not
+
+**Guarantees.** Every finding on the card was attacked by an independent agent that did not see the
+analysis; the ledger recording that was written by the refuters themselves and is checked for
+count and format before a card can be certified; the seven gates are judged by the skill's own exit
+codes, never by a model's say-so; deterministic claims come only from a `validation_report.json`
+whose `repo.head` matches the PR head; a run that cannot establish any of this aborts rather than
+producing a green result.
+
+**Does not guarantee.** That the findings are *correct* — that is what a human reads the card for.
+That the review is *complete*: the set of files given to the per-file assessment is still chosen by
+the caller. That the executor never produces false signals — hence S6b. See
+[docs/known-limitations.md](docs/known-limitations.md), which is deliberately specific.
+
+## Layout
+
+| path | what |
+|---|---|
+| `loop.md` | the contract: roles, artifact layout, eight stages, gates, termination, environment traps, and the lessons behind each guard |
+| `workflows/prefetch.workflow.js` | phase P |
+| `workflows/run.workflow.js` | phase R |
+| `tools/dryrun.mjs` | local harness that stubs the workflow hooks and asserts every guard fires — no model calls |
+| `tools/gh_shim.py` | drop-in `gh` for hosts without it, or for a diff over GitHub's 20000-line API cap |
+| `tools/agent_cost.py`, `dump_subagent_log.py`, `list_run_agents.py` | after-the-fact inspection: token accounting, readable transcripts, and which agents a run actually started |
+| `config/loop.example.json` | every installation-specific value, with the reasoning inline |
+| `docs/known-limitations.md` | what is still broken or unverified |
+| `docs/upstream-findings.md` | four reproducible defects this loop found **in the skills it drives** |
+| `VALIDATED-AGAINST.md` | the skill commit, PR heads, hardware and measurements this was validated on |
+
+## The one number worth knowing
+
+The refuter must not be the same model as the worker. Measured over three rounds of the same PR:
+a same-model refuter killed **18%** of findings; a cross-model refuter killed **40%**, including
+four of six `RED`s in one round. The false positive that a same-model round shipped to the card —
+and that had to be withdrawn by hand — was caught automatically once the refuter changed family.
diff --git a/.agent_loop/VALIDATED-AGAINST.md b/.agent_loop/VALIDATED-AGAINST.md
@@ -0,0 +1,74 @@
+# Validated against
+
+This loop consumes the **CLI surface** of two skills in this repository. That surface is still
+evolving, so a release pins what it was exercised against. If a pinned command or flag has moved,
+the loop can still appear to work while drawing the wrong conclusion — check before trusting a run.
+
+## Loop
+
+    .agent_loop 0.1.0
+
+## Skills
+
+    ROCm/aiter .claude/skills @ the commit this directory was added on
+
+**`review-pr` — the seven gates, all of which must exist and keep their argument order:**
+
+    triage.py answers      <answers.txt>
+    triage.py diagnostic   <ai_diagnostic.txt>
+    triage.py corefiles    <core_files.txt> <pr.diff> <project-root>
+    triage.py ledger       <rules.txt> <verdicts.txt> <pr.diff>
+    triage.py card         <card.md> <verdicts.txt> <ai_diagnostic.txt> <answers.txt> <pr.diff> <late_findings.txt>
+    triage.py refutations  <refutations.txt> <pr.diff> <card.md>
+    triage.py independent  <independent.txt> <card.md>
+
+    fetch.sh <PR> <owner/repo>            # honours REVIEW_AUTO_VALIDATE=0
+
+Artifacts the loop reads by name from `fetch.sh`'s scratch dir: `pr.diff`, `pr_meta.json`,
+`rules.txt`, `rules_expanded.txt`, `applies.txt`, `merge_target.txt`, `guards.txt`, `siblings.txt`,
+`symbols.txt`, `twins.txt`, `test_quality.txt`, `kernel_tests.txt`, `ci_coverage.txt`,
+`perf_claims.txt`, `struct_abi.txt`, `comment_only.txt`, `evidence.txt`,
+`validation_requirement.json`, and the `head/` and `merge-target/` worktrees.
+
+**`validate-kernel-pr` — the flags the loop passes:**
+
+    validate_pr.sh --repo --patch --head-sha --target --label --out
+                   [--expected-route] [--shape-argnames] [--shape-env] [--no-perf]
+
+Report fields the loop reads: `verdict`, `findings[]`, `stages.*.status` and their notes,
+`test_selection`, `execution_receipt`, `stages.correctness_repo_tests.stats`, `stages.perf`
+(`median_ratio`, `worst_column`, `matched_rows`, `threshold`, `baseline_method`), `arch_coverage`.
+
+## Pull request
+
+    ROCm/aiter#4961, reviewed at three successive heads:
+      round 1  head a65d5ffee51752b9051a76328631994dd4231248  base 24a62b1c122f23645a19b9d8b0abd4750c59359b
+      round 2  head a05fe49e1c28d36b21419b543e62a89357ca2aaf  base f0321c0e8927d1d90a29385433f71e592b1c51f5
+      round 3  head 1f24221fe2dfd3b36e742a628eee4ebc8abe1aea  base 226ee790953feedcc9d5e17323db0a019a177d23
+
+    diff size 21.7k-22.0k lines, 68-74 changed files, 30-32 of 53 rules derived
+
+## Hardware
+
+    gfx950  (MI350X, 8 GPUs)      — correctness, policy, interface and JIT-cache targets
+    gfx1250 (MI450X ES, 1 GPU)    — the CO-integration and split-K targets that skip on gfx950
+
+## Measurements
+
+    subagents over the whole exercise      211
+    tokens                                  in 6.82M / out 5.86M
+    one full phase R on a 21.7k-line diff   ~45 agents, in ~1.5M / out ~1.1M
+    refutation cost                         3.7k-38.8k input per finding, median ~10k
+
+    refutation kill rate, same-model refuter        18%   (2 of 11)
+    refutation kill rate, cross-model refuter       40%   (7 of 17, 6 of 15)
+    dry-run guard scenarios                         10/10
+
+## What a real run produced
+
+Round 2's validation on gfx1250 found a reproducible failure in the PR's own added test
+(`correctness_repo_tests: fail`, all other stages passing), which a dedicated debugging agent traced
+to a kernel id registered in Python but absent from the generated C++ dispatch table on any fresh
+build — and whose one-line fix it verified on hardware. Round 3, on a head where that test had been
+removed, produced no correctness failure. Both rounds independently withdrew validator artifacts as
+non-defects. That is the behaviour this package exists to reproduce.
diff --git a/.agent_loop/config/loop.example.json b/.agent_loop/config/loop.example.json
@@ -0,0 +1,61 @@
+{
+  "$schema": "./loop.schema.json",
+  "_comment": "Copy to loop.json and fill in. Nothing here may contain a private key, a real hostname or a token; loop.json itself is gitignored.",
+
+  "workspace": {
+    "tmpRoot": "<absolute path to a scratch dir outside the repo>",
+    "_layout": "tmpRoot/pr-review/<pr>/<round>/{work,reports} — every intermediate lives here and the whole tree is deletable after a round"
+  },
+
+  "local": {
+    "ssh": "ssh",
+    "scp": "scp",
+    "identityFile": "<path to the ssh key, or null to use the agent's default>",
+    "python": "python3",
+    "toolsRoot": "<absolute path to .agent_loop/tools>",
+    "_note": "On Windows use the native OpenSSH binaries (C:\\Windows\\System32\\OpenSSH\\ssh.exe); the ones shipped with Git may not start under a confined harness."
+  },
+
+  "skill": {
+    "projectRoot": "<absolute path to the aiter checkout the gates read>",
+    "reviewPrRoot": "<projectRoot>/.claude/skills/review-pr",
+    "validateSkillRoot": "<projectRoot>/.claude/skills/validate-kernel-pr"
+  },
+
+  "providers": {
+    "_note": "Never hardcode these in the workflow scripts. The refuter MUST be a different model family from the worker: a same-model refuter killed 18% of findings, a cross-model one killed 40%.",
+    "worker":    { "provider": "<local-model>",    "model": "<local-model-id>" },
+    "refuter":   { "provider": "<frontier-model>", "model": "<frontier-model-id>" },
+    "formatter": { "provider": "<local-model>",    "model": "<local-model-id>" },
+    "remote":    { "provider": "<local-model>",    "model": "<local-model-id>" }
+  },
+
+  "hosts": [
+    {
+      "name": "gfx950-box",
+      "sshHost": "<fqdn>",
+      "user": "<user>",
+      "uidGid": "<uid>:<gid>",
+      "container": "<container name>",
+      "image": "<image the container was made from>",
+      "hostRoot": "/home/<user>/pr-review",
+      "containerMount": "/work",
+      "scratch": "/work/_tmp/{runId}",
+      "gpuGate": "/opt/rocm/bin/rocm-smi --showpids",
+      "_rules": "All PR testing runs inside the container. Scratch never goes to the host /tmp. The GPU gate must report no KFD processes before any run."
+    }
+  ],
+
+  "policy": {
+    "maxGateRounds": 3,
+    "refuteGranularity": "per-finding",
+    "severityBar": "in-tree-trigger-required",
+    "_severityBar": "A 🔴 requires a triggering point verifiable in this tree. A protocol documented outside the tree (a README telling downstream integrations to call something) caps the finding at ⚠️. Fixing this bar is what makes rounds comparable.",
+    "resume": true,
+    "budget": {
+      "maxAgents": 120,
+      "maxInputTokens": 4000000,
+      "_measured": "One full round on a 21.7k-line diff: ~45 agents, ~1.5M input / ~1.1M output tokens. Refutation is ~15k input per finding."
+    }
+  }
+}
diff --git a/.agent_loop/docs/known-limitations.md b/.agent_loop/docs/known-limitations.md
@@ -0,0 +1,78 @@
+# Known limitations
+
+Written so that nobody has to discover these by being misled. Each entry says what is missing, what
+it costs, and what would fix it.
+
+## Coverage
+
+**The per-file assessment set is chosen by the caller, not derived.** `args.files` and
+`args.ruleGroups` are filled in by whoever starts phase R, from the fetch report. On the runs behind
+this package, 9 of 68 changed files were assessed. The `corefiles` gate derives the *backbone* set
+from the diff and will refuse a card that ignores one of those, so a Tier-1/Tier-2 omission is
+caught — but a Tier-3 omission is not. **Fix:** derive the file set from the diff with the same
+tiering questions the gate uses, and let the caller only add to it.
+
+**Rule partitioning is manual.** `ruleGroups` splits the derived rule ids into batches for parallel
+adjudication. A rule that lands in no group is caught by the `ledger` gate, but the split itself is
+hand-made. **Fix:** partition programmatically from `rules.txt`.
+
+## Provenance
+
+**The ledger check counts, it does not authenticate.** Phase R verifies that the number of
+refuter-written files equals the number of ledger lines equals the number of findings, and that
+every line is well formed. It cannot detect a party overwriting the *first line* of an existing
+`F<nn>.md`. The separation-of-duties instruction forbids it and nothing observed has attempted it,
+but the guarantee is procedural, not cryptographic. **Fix:** have each refuter record a hash of its
+own file, and verify hashes at assembly.
+
+**Cross-round isolation is not enforced.** Artifacts from a previous round sit in a sibling
+directory and agents can and do find them: in one round a refuter cited the previous round's card in
+its evidence, despite the run being configured as "no prior". Either isolate the rounds or state on
+the card that priors were reachable — claiming independence while leaving them readable is the one
+unacceptable option. **Fix:** per-round workspace roots, and a prompt-level prohibition.
+
+## Operation
+
+**Human answers are not persisted.** Targets, model roles and budget are re-asked every round;
+the environment record is written but never read back by phase P (`envKnown` is passed in by the
+caller). **Fix:** a `state.json` per PR that the break writes and phase P reads.
+
+**No budget enforcement.** `policy.budget` is documented in the config and honoured by nothing. A
+round is roughly 45 agents and 2.6M tokens; nothing stops a pathological run from doing far more.
+**Fix:** count agents in the script and abort past the cap.
+
+**Resume is partial.** `resume: true` skips refuters whose file already exists. Analysis outputs,
+validation reports and the card are always redone. **Fix:** the same existence check for the S2
+artifacts, which are already all on disk.
+
+**Cleanup failure is a warning, not an error.** If a cleanup agent cannot reach a host, the run logs
+it and still returns. The remote side is then left dirty for the next round, which discovers it as a
+permission failure when removing a worktree. **Fix:** treat a failed cleanup as a failed round.
+
+## Environment
+
+**Windows hosts hit a long list of avoidable traps** — no usable bash under a confined harness,
+`schannel` TLS failures for git over https, CRLF breaking the skill's own rule parsing, and
+PowerShell quoting mangling remote commands. All are worked around in `loop.md` §7, and none of them
+exist on a Linux host. **Running the loop from Linux removes most of that section.**
+
+**One GPU per host is common and matters.** A host may expose a single GPU shared with other users'
+long-running containers. The GPU gate (`rocm-smi --showpids`) is checked immediately before launch,
+but nothing holds the GPU for the duration beyond the validator's own `flock`, and a co-tenant can
+claim it mid-run. A run that loses the GPU should be reported as an environment gap, never as a PR
+defect.
+
+**A host can reboot mid-run.** It happened. The validation agent returns no report, and the card
+records `NO REPORT` with the reason rather than inferring a verdict from another target. That is the
+correct behaviour, but the round is then incomplete and must be finished by hand.
+
+## Scope
+
+**Validated against one repository and one PR.** Three rounds of the same PR, on two GPU
+architectures. The loop contract is repo-agnostic, but nothing here has been run against a second
+repository, and the skill CLI surface it depends on is pinned in `../VALIDATED-AGAINST.md` precisely
+because that surface is still evolving.
+
+**The card's five-finding cap is a readability limit, not a recall claim.** It comes from the
+`review-pr` skill, which says so itself. Everything dropped is accounted for on the card and kept in
+full in `findings.md`.