Merge remote-tracking branch 'origin/master' into agent/fix-dwc2-host-fifo-allocation

5 files changed, 172 insertions, 13 deletions

diff --git a/docs/superpowers/followup/pr3803-flasher-recover.md b/docs/superpowers/followup/pr3803-flasher-recover.md
index e9fff7480..1f71c990f 100644
--- a/docs/superpowers/followup/pr3803-flasher-recover.md
+++ b/docs/superpowers/followup/pr3803-flasher-recover.md
@@ -19,18 +19,18 @@ libjaylink, J-Link probes.
 - Roster JSON: `test/hil/tinyusb.json`. `flasher_recover` is OPTIONAL; absent means today's
   behaviour (`recover_flasher` returns the primary).
 - Never change the shape of `board['flasher']` — it is read as a dict in `hil_flash`,
-  `hil_test`, `usbtest`, `hil_pool_check`, `hil_select` and the roster lint, and is shipped
+  `hil_test`, `usbtest`, `hil_pool_check`, `ci_select` and the roster lint, and is shipped
   as JSON to a subprocess.
 - Flasher dispatch is by name: `getattr(hil_flash, f'flash_{name}')` / `reset_{name}`.
 - `RECOVER_FLASH_TIMEOUT = 90`, `RECOVER_RESET_TIMEOUT = 30` (`usbtest.py`). Any board whose
   flash cannot finish inside 90 s is not a candidate.
-- Tests run offline: `cd test/hil && python3 test/test_hil_select.py`.
+- Tests run offline: `cd test/hil && python3 test/test_ci_select.py`.
 
 ## What is already established
 
 **Landed on PR #3803 and inert without roster entries:** `hil_flash.recover_flasher()`,
 `convoy_safe()` accepting openocd-over-jlink, `hil_test` substituting the recovery flasher
-into `--recover-board`, and `test_hil_select.FlasherRecoverEntry` (4 tests).
+into `--recover-board`, and `test_ci_select.FlasherRecoverEntry` (4 tests).
 
 **Verified in source:**
 - openocd's jlink driver ignores `adapter usb vid_pid` — `jlink.c` never reads
@@ -77,7 +77,7 @@ is a different scope from containing a wedge; and it needs bench time on seven b
 - `test/hil/hil_flash.py` — add `flash_openocd_seq` / `reset_openocd_seq`; extend
   `convoy_safe` to accept the new name. This is the only file that learns the command form.
 - `test/hil/tinyusb.json` — seven `flasher_recover` entries.
-- `test/hil/test/test_hil_select.py` — extend `FlasherRecoverEntry`; add a roster lint.
+- `test/hil/test/test_ci_select.py` — extend `FlasherRecoverEntry`; add a roster lint.
 
 ---
 
@@ -85,7 +85,7 @@ is a different scope from containing a wedge; and it needs bench time on seven b
 
 **Files:**
 - Modify: `test/hil/hil_flash.py` (beside `flash_openocd`, ~line 100)
-- Test: `test/hil/test/test_hil_select.py`
+- Test: `test/hil/test/test_ci_select.py`
 
 **Interfaces:**
 - Consumes: `_openocd_cmd_base(flasher)`, `hil_util.run_cmd`.
@@ -120,7 +120,7 @@ is a different scope from containing a wedge; and it needs bench time on seven b
 
 - [ ] **Step 2: Run test to verify it fails**
 
-Run: `cd test/hil && python3 test/test_hil_select.py FlasherRecoverEntry -v`
+Run: `cd test/hil && python3 test/test_ci_select.py FlasherRecoverEntry -v`
 Expected: FAIL — `module 'hil_flash' has no attribute 'flash_openocd_seq'`
 
 - [ ] **Step 3: Write minimal implementation**
@@ -155,13 +155,13 @@ In `convoy_safe`, replace `if name != 'openocd':` with:
 
 - [ ] **Step 4: Run test to verify it passes**
 
-Run: `cd test/hil && python3 test/test_hil_select.py FlasherRecoverEntry -v`
+Run: `cd test/hil && python3 test/test_ci_select.py FlasherRecoverEntry -v`
 Expected: PASS
 
 - [ ] **Step 5: Commit**
 
 ```bash
-git add test/hil/hil_flash.py test/hil/test/test_hil_select.py
+git add test/hil/hil_flash.py test/hil/test/test_ci_select.py
 git commit -m "hil: add openocd_seq flasher for convoy-safe recovery delivery"
 ```
 
@@ -171,7 +171,7 @@ git commit -m "hil: add openocd_seq flasher for convoy-safe recovery delivery"
 
 **Files:**
 - Modify: `test/hil/tinyusb.json`
-- Test: `test/hil/test/test_hil_select.py`
+- Test: `test/hil/test/test_ci_select.py`
 
 **Interfaces:**
 - Consumes: `flash_openocd_seq` / `reset_openocd_seq` from Task 1.
@@ -196,7 +196,7 @@ git commit -m "hil: add openocd_seq flasher for convoy-safe recovery delivery"
 
 - [ ] **Step 2: Run test to verify it fails**
 
-Run: `cd test/hil && python3 test/test_hil_select.py FlasherRecoverEntry -v`
+Run: `cd test/hil && python3 test/test_ci_select.py FlasherRecoverEntry -v`
 Expected: FAIL — `0 >= 7`
 
 - [ ] **Step 3: Add the entries**
@@ -224,13 +224,13 @@ Add to each board below, using the SAME `uid` as its primary jlink entry:
 
 - [ ] **Step 4: Run test to verify it passes**
 
-Run: `cd test/hil && python3 test/test_hil_select.py -v`
+Run: `cd test/hil && python3 test/test_ci_select.py -v`
 Expected: PASS, and no other selector test regresses.
 
 - [ ] **Step 5: Commit**
 
 ```bash
-git add test/hil/tinyusb.json test/hil/test/test_hil_select.py
+git add test/hil/tinyusb.json test/hil/test/test_ci_select.py
 git commit -m "hil: give seven J-Link boards a convoy-safe recovery flasher"
 ```
 
diff --git a/docs/superpowers/followup/pr3803-hil-blindness-reporting.md b/docs/superpowers/followup/pr3803-hil-blindness-reporting.md
index 69ff939b0..374ee62c7 100644
--- a/docs/superpowers/followup/pr3803-hil-blindness-reporting.md
+++ b/docs/superpowers/followup/pr3803-hil-blindness-reporting.md
@@ -158,7 +158,8 @@ def _board_result_on_error(name, exc):
     """A row for a board that died by exception. err_count 1, no per-test detail, but the
     blindness and stray fields are still accurate -- they explain the failure more often
     than the exception text does."""
-    rows = [(name, {BOUNDARY_CELL: f'{REPORT_CELL["fail"]} {type(exc).__name__}'}, None)]
+    rows = [(name, {hil_report.BOUNDARY_CELL:
+                    f'{hil_report.REPORT_CELL["fail"]} {type(exc).__name__}'}, None)]
     return _board_result(name, 1, [], rows, 0.0, True)
 ```
 
diff --git a/docs/superpowers/followup/pr3840-mret-board-result.md b/docs/superpowers/followup/pr3840-mret-board-result.md
new file mode 100644
index 000000000..7b8da7b9c
--- /dev/null
+++ b/docs/superpowers/followup/pr3840-mret-board-result.md
@@ -0,0 +1,90 @@
+# Give the HIL worker result a name
+
+**Origin:** split out of PR #3840 (making `hil_report.md` a rendering of `hil_report.json`).
+Delete this file when its own PR lands.
+
+## What is established
+
+`test_board()` returns a bare tuple that three producers build and fourteen call sites read
+positionally. It has grown 5 → 6 → 7 fields, and the code already works around its own
+shape:
+
+```python
+hil_test.py:1992   dirty = [(r[0], r[6]) for r in mret if len(r) > 6 and r[6]]
+hil_test.py:2014   blind = [r[0] for r in mret if len(r) > 5 and r[5]]
+hil_test.py:2386   for name, _, _, _, dur, *_ in mret:
+hil_report.py:306  for name, _, _, rows, *_ in mret:
+```
+
+Two facts make this worth closing rather than tolerating:
+
+- **The declared type is already wrong.** `hil_test.py:1711` says
+  `tuple[str, int, list[str], list, float]` — five fields — while the main return at `:1872`
+  yields seven (`+ sysfs_blind(), stray`).
+- **A wrong slot is a wrong verdict, not a crash.** Field 5 is `blind`, which decides whether
+  a board's red cells are reported as broken hardware or as "could not tell". Inserting a
+  field mid-tuple makes `r[5]` read the wrong slot and keep running.
+
+It has bitten once already: `test_hil_bounded.py`'s
+`test_both_row_widths_survive_the_report_writers` exists because the blindness flag widened
+the tuple to 6 while the pool-timeout path still synthesised 5-field rows, and *"a
+fixed-width unpack in either one raises INSIDE the containment path, which is where a raise
+costs every board's results."* That is why the unpacks end in `*_`.
+
+## What remains
+
+A `NamedTuple` with defaults. Verified to pickle across the pool boundary and to stay
+fully tuple-compatible — existing `r[0]`, `e[1]`, `for name, _, _, rows, *_` and `len(r)`
+all keep working, so it lands without touching the fourteen consumers:
+
+```python
+class BoardResult(NamedTuple):
+    """What one worker returns. Field ORDER is load-bearing: it is unpacked positionally
+    in a dozen places, and the pool-timeout path synthesises one by hand."""
+    name: str
+    err_count: int
+    failed_tests: list[str]
+    rows: list | None          # None from the pool-timeout synthesis, never []
+    duration: float
+    blind: bool = False        # defaults, so a synthesised result is full-width
+    stray: int = 0
+```
+
+Then a second, smaller step removes the coupling itself: `accumulate_report` takes
+`[(name, rows)]` pairs instead of `mret`, and `hil_test` does the extraction because it owns
+the shape. One line at each end; the subtle merge logic — stale lock clearing,
+`BOUNDARY_CELL`, `duration=None` preservation — is untouched.
+
+## Sizing
+
+| | Sites |
+|---|---|
+| Producers to convert | 4 (`hil_test.py:1724`, `:1872`, `:2283`, `:2327`) |
+| Arity guards deleted | 2 (`:1992`, `:2014`) |
+| Wrong annotation fixed | 1 (`:1711`) |
+| `hil_report`'s coupled line | 1 (`:306`) |
+| Positional consumers (optional migration) | 14 |
+| **Test fixtures building tuples by hand** | **34** |
+
+Production code is roughly ten changed lines. **The work is dominated by the test
+fixtures**, which is also the risk.
+
+## Do this first, or the refactor is unverifiable
+
+`test_hil_report.py` (27 sites) and `test_hil_bounded.py` (7) construct plain tuples by
+hand — `('boardA', 0, [], [], 1.0, True)`. A producer that forgot to switch to
+`BoardResult`, or a pickling regression, **passes the entire 310-test suite** and surfaces
+only on the rig. Convert the fixtures to build `BoardResult` as task 1, before touching any
+producer. This ordering is not optional.
+
+Second trap: `rows` is `None` on the pool-timeout path (`hil_test.py:2283`), never `[]`, and
+`accumulate_report` guards with `if rows and ...`. A well-meaning `rows: list = []` default
+silently changes that path. Pin it with a test before the conversion.
+
+## Why it was split out
+
+PR #3840 touches the report document. This touches `test_board`'s return and the containment
+paths, where a raise costs every board's results rather than one board's — a different blast
+radius, needing its own review and its own rig run. #3840 is twice-reviewed and dogfooded
+ten times on hardware; folding this in would reset that surface for a latent-trap cleanup
+that is not causing bugs today.
diff --git a/docs/superpowers/followup/pr3840-skill-md-no-boards-drift.md b/docs/superpowers/followup/pr3840-skill-md-no-boards-drift.md
new file mode 100644
index 000000000..a039a8c12
--- /dev/null
+++ b/docs/superpowers/followup/pr3840-skill-md-no-boards-drift.md
@@ -0,0 +1,38 @@
+# `SKILL.md` contradicts the code on no-boards tables
+
+**Origin:** split out of PR #3840, surfaced by its second review round. Delete this file
+when its own PR lands.
+
+`.claude/skills/hil/SKILL.md:150-151` tells the reading agent:
+
+> `**HIL run selected no boards.**` — the filters intersected to nothing, so there is **no
+> table at all**. Report that (and the filter shown), never `"pass": true`.
+
+That was true when the no-boards exit wrote a bare notice. It no longer is. An
+`--accumulate` no-boards run keeps the accumulated rows — deliberately, because wiping them
+destroyed real results — so the artifact now reads:
+
+```
+**HIL run selected no boards.** filters emptied
+
+**✅ 1 passed · ❌ 0 failed · ⚪ 0 skipped · blank not run**
+
+| Board | t | duration |
+...
+```
+
+The behaviour is correct; the documentation is wrong, and wrong in the direction that
+matters. An agent is told to expect no table, sees one, and has no rule for whether those
+rows are reportable. **They are not this run's** — they are a previous attempt's, carried
+forward.
+
+**What remains:** update that bullet to describe both cases — a fresh run has no table, an
+`--accumulate` run shows the previous attempt's rows under the notice and they must not be
+reported as this run's. Add a test asserting the fresh case renders no matrix, so the two
+halves cannot drift again.
+
+## Why it was split out
+
+PR #3840 fixed the findings that changed a verdict. This is a documentation drift: the
+behaviour is correct and the doc describing it is not, so it is better reviewed on its own
+than appended to a branch already carrying a module consolidation.
diff --git a/docs/superpowers/followup/pr3840-write-report-atomicity.md b/docs/superpowers/followup/pr3840-write-report-atomicity.md
new file mode 100644
index 000000000..2094207bb
--- /dev/null
+++ b/docs/superpowers/followup/pr3840-write-report-atomicity.md
@@ -0,0 +1,30 @@
+# `write_report` commits the two artifacts non-atomically
+
+**Origin:** split out of PR #3840, surfaced by its second review round. Delete this file
+when its own PR lands.
+
+```python
+md = render_report(doc) + '\n'
+report_dir.mkdir(parents=True, exist_ok=True)
+(report_dir / REPORT_JSON).write_text(json.dumps(doc, indent=2) + '\n')
+(report_dir / REPORT_MD).write_text(md, encoding='utf-8')
+```
+
+Rendering before writing closed the *render-failure* case: a raise can no longer commit a
+sidecar the markdown contradicts. It does not close the *interrupted-between-writes* case. A
+kill between those two lines leaves the pair disagreeing — and this runs on the containment
+path, on the way to `os._exit`, on a rig whose jobs get cancelled by the GitHub job ceiling.
+
+**What remains:** write both to temp files, then `os.replace` both. The window shrinks from
+two full writes to two renames, and neither file is ever observed half-written. `os.replace`
+is atomic per file on POSIX; the pair is still not transactional, which is acceptable and
+should be said in the docstring rather than implied away.
+
+Worth pairing with a test that kills between the writes — or, more practically, one that
+asserts no partial file is ever visible by checking the temp-then-rename shape directly.
+
+## Why it was split out
+
+A durability edge, not a wrong verdict. PR #3840 closed the render-failure half of this
+(nothing is written until the markdown renders); the interrupted-between-writes half needs
+a temp-then-rename and is better reviewed on its own.