diff options
Diffstat (limited to '.claude')
| -rw-r--r-- | .claude/agents/pr-review-validator.md | 12 | ||||
| -rw-r--r-- | .claude/skills/etm-trace/boards.md | 100 | ||||
| -rw-r--r-- | .claude/skills/hil/SKILL.md | 13 | ||||
| -rw-r--r-- | .claude/skills/rtt/SKILL.md | 201 | ||||
| -rw-r--r-- | .claude/skills/rtt/boards.md | 78 | ||||
| -rw-r--r-- | .claude/skills/target-debug/SKILL.md | 46 | ||||
| -rw-r--r-- | .claude/skills/usbtest/SKILL.md | 2 | ||||
| -rw-r--r-- | .claude/workflows/pr-babysit.js | 35 | ||||
| -rw-r--r-- | .claude/workflows/validate.js | 282 |
9 files changed, 645 insertions, 124 deletions
diff --git a/.claude/agents/pr-review-validator.md b/.claude/agents/pr-review-validator.md index 324e8efcb..4069a856f 100644 --- a/.claude/agents/pr-review-validator.md +++ b/.claude/agents/pr-review-validator.md @@ -6,12 +6,12 @@ model: opus effort: xhigh --- -You validate the bot review findings on exactly one PR (number given in your prompt) using `gh`. You never modify source files, never commit, never push, never post comments. Do not read or classify CI. +You validate the bot review findings on exactly one PR (number given in your prompt) using `gh`. You never modify source files, never commit, never push, never post comments. Do not triage or classify CI failures or logs — pr-ci-watcher owns that; you may read the review bots' own check runs to see whether they concluded. ## Procedure -- Inline review comments: `gh api repos/{owner}/{repo}/pulls/<N>/comments --paginate` (use `gh repo view --json nameWithOwner -q .nameWithOwner` for owner/repo). Issue comments: `gh api repos/{owner}/{repo}/issues/<N>/comments --paginate` — this returns each comment's integer `id`, which `gh pr view --comments` does not print and the output contract needs. -- Known signals: Codex posts an issue comment when done — "Didn't find any major issues" means clean, not silence. Copilot is finished when it no longer appears in `requested_reviewers`. Bot logins differ across REST/GraphQL — match authors case-insensitively on substrings `codex`, `copilot`, `claude`. +- Inline review comments: `gh api repos/{owner}/{repo}/pulls/<N>/comments --paginate` (use `gh repo view --json nameWithOwner -q .nameWithOwner` for owner/repo). Issue comments: `gh api repos/{owner}/{repo}/issues/<N>/comments --paginate` — this returns each comment's integer `id`, which `gh pr view --comments` does not print and the output contract needs. PR reviews (the Copilot/Claude verdict bodies): `gh api repos/{owner}/{repo}/pulls/<N>/reviews --paginate` — compare each review's `commit_id` to the head SHA from `gh pr view <N> --json headRefOid -q .headRefOid` to tell a review of the current push from an older one. +- Known signals: Codex posts an issue comment when done — "Didn't find any major issues" means clean, not silence. It can also signal a clean pass with no comment at all: a 👍 (`+1`) reaction on the PR description (`gh api "repos/{owner}/{repo}/issues/<N>/reactions?content=%2B1&per_page=100" --paginate`, author matching `codex`; without `--paginate` a fresh reaction can fall off the first page and Codex looks pending forever) — settled when the reaction's `created_at` postdates the head push time defined below. Its body carries a `**Reviewed commit:** <short sha>` line: Codex is settled only when that short SHA prefix-matches the head SHA, otherwise the comment is a verdict for an older push and Codex is still pending. Its "Something went wrong" comment has no Reviewed-commit line, so correlate that one by time instead — against the moment the SHA *became* the head, `gh api repos/{owner}/{repo}/commits/<headSha>/check-suites --jq '[.check_suites[].created_at] | min'` (the suites are created when the push lands; fall back to `gh api repos/{owner}/{repo}/commits/<headSha> --jq .commit.committer.date` only if the SHA has no check suites). The committer date alone is when the commit was written, which can precede the push by hours and make a leftover error comment look fresh. An error/quota comment settles Codex only when its `created_at` postdates that push time, or when it arrives as a PR review whose `commit_id` is the head SHA. An older one is a leftover from an earlier push — Codex is still pending. Copilot submits a PR review whose body opens with a verdict header (`### 🟢 Approval recommended` / `### 🟡 Changes recommended`) and leaves `requested_reviewers` once submitted. The Claude bot posts a PR review, or its `claude-review` check run for the head SHA reaches `status: completed` — ask for that check by name, `gh api "repos/{owner}/{repo}/commits/<headSha>/check-runs?check_name=claude-review"`, since the unfiltered listing is paginated and drops it on a PR with more than a page of checks. A bot reporting a usage/quota limit counts as settled once that report postdates the head push time above (the check-suite timestamp, not the committer date) — do not wait on it. Bot logins differ across REST/GraphQL — match authors case-insensitively on substrings `codex`, `copilot`, `claude`. - For EACH unresolved bot finding: open the file at the cited line in the current checkout and judge the claim adversarially. `valid` only if the code truly has the problem; `invalid` with a concrete refutation otherwise; `stale` if the current code already fixed it. - Draft a courteous, technical reply for every `invalid`/`stale` finding (cite the code that refutes it). Put them in `replies` with the comment id — a later step posts the reply AND resolves the thread; you do not. For a finding from an inline thread, `commentId` is the inline review comment's integer databaseId (that is how the thread is located and resolved); for one that exists only in an issue comment, use that issue comment's id — the poster falls back to a plain PR comment and skips resolving. @@ -23,4 +23,8 @@ Your final message is parsed by a program. Return ONLY this JSON — no prose, n "replies": [{"commentId": 123, "body": "..."}], "done": false} -done = true only when no unresolved `valid` findings remain. +done = true only when no unresolved `valid` findings remain AND every auto-reviewer +has settled for the current head SHA: its verdict is posted (Copilot review header, +Codex verdict comment, Claude review or concluded check) or it reported hitting a +usage/quota limit. A reviewer that has not reported since the last push is pending — +return done = false so the caller re-checks next cycle. diff --git a/.claude/skills/etm-trace/boards.md b/.claude/skills/etm-trace/boards.md index 044d4e0ee..f54e1d7d6 100644 --- a/.claude/skills/etm-trace/boards.md +++ b/.claude/skills/etm-trace/boards.md @@ -26,7 +26,7 @@ reference. | mimxrt1170_evkb | 996 MHz | 50 MHz (root/2) | 1 | 0 | weld 0 Ω R1881-R1886; JP4 shorted; J58 (populated) | re-weld R1884 (D3 open; D1/D2 meter-verified good) → width 4 | | ra6m5_ek (M33) | 200 MHz | 25 MHz (TRCLK/4 /2) | 4 | 0 (unset) | J9 closed; native J20 trace | — | | ra8m1_ek (M85) | 480 MHz | 60 MHz (TRCLK/4 /2) | 4 | 0 (unset) | J9 closed + Table 7 jumpers | — | -| raspberry_pi_pico2 (RP2350 M33) | 48 MHz | 24 MHz (clk_sys/2) | 4 | 0 (unset) | fly-wire GPIO1-5 → MIPI20 (map in jdebug) | 72-80 MHz per seating (re-qualify); >80 needs V3 probe + trace board | +| pico2_etm_trace (RP2350 M33) | 150 MHz | 75 MHz (clk_sys/2) | 4 | +1 ns | Pico 2 on the trace-carrier PCB (MIPI-20) | — | | same54_xplained (E54 M4F) | 120 MHz | 60 MHz (CPU/2) | 4 | 0 (unset) | none — populated 20-pin ETM header | — | | same70_xplained (E70 M7) | 300 MHz | 37.5 MHz (PCK3/2) | 1 | 0 (unset) | solder 20-pin header on J403 (bottom) | width 4 blocked: D1 (J403.16) dead at speed — probe-channel crosscheck pending | | SEGGER H7/F407 ref | demo defaults | demo | 4 | demo | probe-powered: add `--power` | — | @@ -105,42 +105,68 @@ Board caveats (beyond the table): the decoder at t≈0.05 s every run. Runs both chip maxima (120 MHz TRCLK, 60 MHz pin) clean. `ReadIntoTraceCache 0x0 0x10000` in the download hook covers runtime chip-ROM execution. ISR entry: `tusb_int_handler`. -- **raspberry_pi_pico2** (RP2350): TRACECLK is a fixed clk_sys/2, no divider - (DDR data, like every ARM TPIU pin port). **Measured cliff on this rig:** - 80 MHz core (40 MHz TRACECLK) traces idle code but dies under dense data; - 88 MHz+ dies instantly at any width/global-timing/TIF/pad setting. Cause - not pinned down: the same V2 probe samples 66 MHz TRACECLK (132 Msample/s) - on metro_m7_1011, so it is NOT a plain probe sample-rate ceiling. The - cliff at >40 MHz TRACECLK (84+ MHz core) survived a full sweep - global - AND per-pin `--trace-timing`, pad drive 2/4/8/12 mA + slew, width 4/2/1, - TIF 1-25 MHz, newer J-Link library - all flat, so it is V3-probe / real- - trace-board territory (SEGGER's Pico 2 KB requires J-Trace PRO **V3.0+** - and recommends a proper trace board; community reports fly-wires fail at - 75 MHz for everyone, PCBs work). Separately, fly-wire seating quality - sets the width-4 DENSE-data ceiling (48-72 MHz observed across seatings): - after ANY rewiring re-qualify with idle blinky at the target clock, then - cdc_msc x3. Random unknown-packet deaths KB into a clean stream = one - marginal wire; `--trace-width` 1 vs 2 vs 4 bisects which (width 1 = - CLK+D0 only; D1 = GPIO3->MIPI20 pin 16 has gone marginal twice on this - rig). Width-1 is a full-quality fallback: complete cdc_msc profiles at up - to 80 MHz core even when width 4 is broken. - **Never set a custom JLinkScript** — it - replaces J-Link's built-in RP2350 device script, which both declares the - trace component map (funnel/TPIU/ETM are not in the ROM table → "Required - trace components for pin trace not found", 0 fetches) and re-arms the whole - chip-side path via `OnTraceStart` at every resume. Firmware therefore does - no trace setup; TRACE_ETM builds only (a) pin clk_sys to 48 MHz from crt0 - (board.cmake) — the fly-wire ceiling: 96/150 MHz kill the stream in the - startup burst at any sample timing (and at 150 MHz the saturated probe - stops answering halts, "CPU could not be halted"); any post-arm clock - change steps TRACECLK mid-stream and kills the decoder — and (b) - clear TIMER0/1 DBGPAUSE (family.c): debug sessions leave cores - halted-at-reset and the default DBGPAUSE freezes the µs timer, so every - `sleep_ms()` spins forever (looks like a dead board; watchdog-scratch - breadcrumbs survive warm resets but not POR when diagnosing). UART console - is TX-only (GPIO1 = TRACECLK). Empty reset/download hooks: the bootrom - must run the IMAGE_DEF. If the chip ends up wedged/un-attachable: - J-Link `erase` + reset drops it into BOOTSEL (2e8a:000f) for picotool. +- **pico2_etm_trace** (Pico 2 / RP2350 on the carrier; board `raspberry_pi_pico2` + is the bare module and has no trace wiring): rig = **pico2 trace motherboard PCB** + (~/code/pcb/pico2_trace_motherboard: MIPI-20, 27 Ohm source-terminated, + GND-guarded). TRACECLK is a fixed clk_sys/2 (DDR), so the board traces at + the rp2350 pico-sdk default 150 MHz -> 75 MHz TRACECLK width 4, validated + 2026-08-26: cdc_msc enumeration burst 3/3, zero overflow, **data sampling + +1 ns** (committed in the reference; idle eye -1000..+2000 ps, +3000 dead; + TD aliases modulo the 6.67 ns UI); soak: cdc_msc_throughput under a live + host CDC+MSC bulk pump, 3/3 x 15 s, zero overflow, 53.7M fetches (DCD hot + path at 9% load). `TRACE_ETM` is set by the board's own board.cmake - no + build flag needed. + **Other rates need a hand-built clock**: pass `SYS_CLK_KHZ` *together with* + `PLL_SYS_VCO_FREQ_HZ`/`POSTDIV1`/`POSTDIV2` from the SDK's + `scripts/vcocalc.py` as compile definitions (a bare `-DSYS_CLK_KHZ=` only + sets a CMake cache var and is silently ignored - the BSP no longer carries + a PLL table). Measured: 180000 = 90 MHz TRACECLK, loaded eye + +4000..+5000 ps (3/3); 240000 = **the J-Trace PRO V2 ceiling** (120 MHz + TRACECLK, TD +3500) — **⚠ 240 MHz was measured with the core regulator + raised to 1.15 V, which nothing does automatically any more: add + `SYS_CLK_VREG_VOLTAGE_AUTO_ADJUST=1` and + `SYS_CLK_VREG_VOLTAGE_MIN=VREG_VOLTAGE_1_15` yourself, or the chip runs + 60% over its 150 MHz rating at stock 1.10 V.** >=125 MHz + TRACECLK is a hard probe wall at every sample delay/width (the V2 AT its + documented limit: Arm spec 100 MHz in-spec, SEGGER's tuned-V2 best is + 120; the 150 MHz on current product pages is V3/V4). **Firmware needs + almost no trace code**: J-Link's built-in RP2350 device script declares + the off-ROM-table trace components (funnel/TPIU/ETM) and re-arms the + whole chip-side path via OnTraceStart at every resume — **never set a + custom JLinkScript** (it replaces the built-in script: "Required trace + components for pin trace not found", 0 fetches). What TRACE_ETM (set by + this board's board.cmake) does in firmware: (a) clears TIMER0/1 DBGPAUSE + — J-Link does NOT clear it, and with the reset default the us-timer + freezes while a core is debug-halted, so sleep_ms() spins forever after + any debugger session (measured: DBGPAUSE reads 0x7 and TIMERAWL stands + still until the clear); (b) compile-time pin-conflict checks — #error if + the UART console lands on a trace pin GP1-5, #pragma message if the + default I2C does. The console itself is full-duplex on GP12/13 (the + carrier routes it off GP0/1; the old TX-only fallback is gone with the + fly-wire rig). PCB A/B validation did remove the 12 mA fast-slew trace + pads (default pads pass 3/3 with a wider idle eye) — do not re-add + without fresh PCB evidence. A runtime clk_sys switch **silently + truncates the capture at the switch** (no decoder error — profile just + ends; verified 3/3 with a board_init-time 120->156 step), so nothing may + re-switch the clock at runtime. + **This is the only trace-capable board in the rp2040 family** - it owns the + sole ozone reference, so `--board <any other rp2040/rp2350 board>` exits + with "cannot resolve J-Link device" (the script's board.cmake fallback + cannot help: this family sets `JLINK_DEVICE` in family.cmake). Capture with + `--board pico2_etm_trace`. + **Arm-phase flake**: an occasional instant unknown-packet death at + offset ~0x10-0x6C right at trace start — just re-run; only mid-stream + deaths indicate a real problem. **Loose MIPI-20 cable symptom ladder**: + flash "Failed to perform RAMCode-sided Prepare()" / "Download failed" + first, then "Target voltage too low" (VTref lost) — reseat the cable at + both ends before debugging software. Empty reset/download hooks in the + reference: the bootrom must run the IMAGE_DEF (setting SP/PC from the + vector table bypasses it and the pico-sdk runtime never comes up). If + the chip ends up wedged/un-attachable: J-Link `erase` + reset drops it + into BOOTSEL (2e8a:000f) for picotool. *Historical*: bring-up used a + fly-wire rig (same GPIO1-5 -> MIPI20 map) whose wire SI capped TRACECLK + at 24-40 MHz and motivated the removed workarounds; it is retired — + details in git history (the 48/80 MHz PLL rows left with it). - **same54_xplained**: the CM4 trace unit is clocked from **GCLK channel 47 (GCLK_CM4_TRACE)** — with it disabled the pins mux fine, TPIU/ETM arm fine, and the port stays perfectly silent (zero fetches, no errors); diff --git a/.claude/skills/hil/SKILL.md b/.claude/skills/hil/SKILL.md index d1e4bdcd5..03a462ce6 100644 --- a/.claude/skills/hil/SKILL.md +++ b/.claude/skills/hil/SKILL.md @@ -65,8 +65,8 @@ hooks: `test_ci_select.py` covers only selection, `test_ci_metrics.py` only the plumbing. The containment work --- bounded reads, the kill ladders, the build and pool guards --- lives in `test_hil_bounded.py`, `test_hil_health.py` and `test_hil_util.py`, so run all five when changing `test/hil`: -`for f in test/hil/test/test_*.py; do python3 "$f"; done` (~84s, of which -`test_hil_bounded.py` is ~76s of deliberate hang/timeout simulation; the two `test_ci_*` +`for f in test/hil/test/test_*.py; do python3 "$f"; done` (~48s, of which +`test_hil_bounded.py` is ~30s of deliberate hang/timeout simulation; the two `test_ci_*` suites are ~4s together). ## Pre-flight rig health check @@ -82,6 +82,8 @@ See the `usb-kernel-recover` skill for what a real wedge looks like and how to c Examples must be built for the target board(s) — see CLAUDE.md "Build" → "All examples for a board" (produces `examples/cmake-build-<board>/`). `-B examples` points `hil_test.py` at that parent folder. (This applies to `hil_test.py`; `hil_pool_check.py` builds its own missing firmware.) +A board whose flasher probe has no VCOM (or whose BSP has no UART) uses RTT as its console — "No serial device found for /dev/serial/by-id/…" on every host test is the symptom. Config: `"logger": "rtt"` (jlink flashers only) plus a self-named variant carrying the define — `"variant": [{"name": "<board>", "defines": ["LOGGER=rtt"]}]` — and prebuilt example sets must carry the same `-DLOGGER=rtt`. Caveat: the cdc/msc-fixture host tests don't speak RTT yet, so such a board cannot carry `is_cdc`/`is_msc` fixtures (the config loader rejects it; see the rtt follow-up doc). Details: the `rtt` skill. + ## Arguments - **Board:** `-b BOARD_NAME`, repeatable for a subset (`-b a -b b`); omit to run all boards in the config. Give a whole set to ONE run rather than one run per board: it schedules the boards across host controllers and budgets concurrent flashes and usbtest batteries per controller (`hil_lock.py` `FLASH_PARALLEL`/`USBTEST_PARALLEL`). Those permits are in-process semaphores — a second `hil_test.py` running alongside does not share them, it multiplies the load on the same xHCI cards. @@ -135,8 +137,8 @@ The user-facing answer to a HIL run IS the tool's summary table: paste the compl table (and footer counts) verbatim — never truncate rows or reduce it to a prose digest; at most one line of commentary below it. -**First check what sits above the table.** Seven banners can appear there; match on a -PREFIX, since each carries trailing detail and one is a blockquote: +**First check what sits above the table.** Six banners can appear there; match on a +PREFIX, since each carries trailing detail and two are blockquotes: - `**HIL run abandoned: worker pool timed out after …s.**` — no results were collected this attempt, so any table below is a PREVIOUS attempt's. Report the abandonment, never those @@ -157,9 +159,6 @@ PREFIX, since each carries trailing detail and one is a blockquote: - `> **Rig dirty.**` — a process survived SIGKILL and still holds a probe or usbfs node into the NEXT job. The table below is this run's and can be reported, but say the rig is dirty: the next job starts degraded and nothing in the harness can clear it. -- `> **Not all verdicts are evidence.**` — one or more workers went blind on sysfs, so - "device not found" from the named boards means "could not tell". Do NOT report their red - cells as broken boards. On failure, retry once with `-v` — from the `<config>.failed` spec the run just wrote, which already begins with `--accumulate` and restricts each board to its failed tests. A hand-scoped diff --git a/.claude/skills/rtt/SKILL.md b/.claude/skills/rtt/SKILL.md new file mode 100644 index 000000000..5e14ae84c --- /dev/null +++ b/.claude/skills/rtt/SKILL.md @@ -0,0 +1,201 @@ +--- +name: rtt +description: Use when you need console or printf I/O, TU_LOG capture, or a raw byte channel over a debug probe on real hardware — the board has no UART wired or its probe no VCOM, a LOGGER=rtt build needs reading or writing, "RTT Control Block not found", an RTT server won't come up or drops output, JLinkRTTLogger/JLinkRTTClient/JLinkGDBServer/openocd rtt misbehave, or another workflow (HIL console, SystemView capture) needs RTT stood up on a J-Link, ST-Link, CMSIS-DAP or WCH-Link probe. +--- + +# rtt — SEGGER RTT transport and console + +RTT is nothing but RAM: a control block `_SEGGER_RTT` (starts with the magic +string `"SEGGER RTT"`) plus per-channel ring buffers +`{sName, pBuffer, SizeOfBuffer, WrOff, RdOff, Flags}`. The target advances +`WrOff`; the host must **write `RdOff` back** to free space — a reader that +only reads never drains the ring. Channel 0 is the "Terminal" console; +SystemView claims its own `"SysView"` up-buffer on the same control block — +they coexist. The debug probe reads/writes this RAM while the core runs, so +everything here is zero-wiring: no UART, no VCOM. + +Scope: byte transport and console. Timing/profiling → `etm-trace`/`sysview`; +debugging decision flows and the wedged-target drain model → `target-debug`; +Espressif consoles → `esp-target-debug` (USB-Serial-JTAG, no SEGGER RTT). + +## Quick start — console on a J-Link probe + +Use the skill's tool `tools/rtt.py` for every route; do not hand-roll +JLinkExe/JLinkGDBServer/openocd/telnet pipelines (`--help` for all modes): + +```bash +# firmware: TU_LOG + stdio → RTT channel 0 (hw/bsp/board.c routes sys_read too) +cmake -DBOARD=<board> -DLOG=2 -DLOGGER=rtt ... # Make: LOG=2 LOGGER=rtt + +# flash + reset FIRST (the console owns the probe once open), then: +python3 tools/rtt.py --backend jlink --probe <serial> --device <JLINK_DEVICE> --seconds 20 +# -i forwards stdin to the target; --seconds 0 streams until Ctrl-C/EOF +``` + +`JLINK_DEVICE` comes from `hw/bsp/<family>/boards/<board>/board.cmake` (or +`family.cmake`). Always pass the probe serial — rigs and benches run several +probes, and the `ninja <example>-jlink` flash target grabs whichever J-Link +enumerates first: pin it (`-DJLINK_OPTION="-USB <serial>"`) or flash with +`JLinkExe -SelectEmuBySN`. The HIL harness uses the same implementation +(`hil_util.JlinkRtt`) via a board's `"logger": "rtt"` (jlink flashers +only) plus a single self-named variant carrying the define — +`"variant": [{"name": "<board>", "defines": ["LOGGER=rtt"]}]`, the roster's +one shape for always-on defines — variant defines feed `hil_test.py +--build` and the CI matrix; a prebuilt `cmake-build-<board>` set must be +configured with the same `-DLOGGER=rtt` itself. Keep harness console builds +quiet (`LOGGER=rtt` WITHOUT `LOG=2`): reset-then-attach only preserves what +fits the up-buffer (stock 1 KB, NO_BLOCK_SKIP), and a chatty boot burst +truncates at the ring boundary before the drain attaches — measured +1022-1023 B captures on ea4088 with `LOG=2`, enumeration lines falling off +the end. `BUFFER_SIZE_UP` is the knob when verbose logs are really needed. +Rig boards need `hil_lock.py` held first — see the `hil` skill. + +To validate bidirectionality end-to-end you need firmware that both polls +the console AND replies via printf. `board_test` polls `board_getchar()` +(RTT-aware via `sys_read`) but echoes through `board_putchar` → +`board_uart_write`, which is NOT LOGGER-aware — on a UART-less board the +echo hits the `-1` stub and vanishes (measured on ea4088). For a validation +run, patch its echo to `printf` locally, or drive a host example's menu +(`msc_file_explorer`, `cdc_msc_hid` — they reply via printf). Sending +keystrokes to `cdc_msc` and expecting an echo proves nothing: it never polls +the console. + +## Transport matrix + +| Transport / tool | Live read | Write | Notes | +| ----------------------------------------------- | ---------- | -------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| ARM memory-AP (any J-Link/ST-Link/CMSIS-DAP) | yes | yes | zero intrusion; core keeps running | +| RISC-V SBA (where implemented) | yes | yes | autonomous like memory-AP | +| WCH QingKe SDI | **NO** | no | DM abstract-command reads perturb the running core: A/B-proven firmware kill ~1.9 s into USB traffic. Halt→read→resume or post-mortem dump ONLY | +| OpenOCD/jaylink on a genuine SEGGER J-Link | yes | untested | routine in the sysview campaigns (metro_m4_express, dozens of attaches, zero wedges); prefer SEGGER tools where both exist (drain rate) | +| OpenOCD/jaylink on the LPC-Link2 (J-Link OB fw) | forbidden | — | measured on ea4088's LPC-Link2 (2023 OB image): transport fails (`jaylink_swd_io`) and knocks the probe off USB; physical replug to recover — SEGGER tools only THERE. Verdict is for that probe only: other J-Link-OB firmware probes are untested — hardware-test before assuming either way | +| `JLinkRTTLogger` | unreliable | — | searches for the control block once at attach and gives up — on some parts it never finds it ("RTT Control Block not found" even with `-RTTAddress`; measured 0/6 on LPC4088). May work elsewhere, but don't build automation on a single-search tool | + +Validated boards, directions and per-board caveats: [boards.md](boards.md). + +## Capture: J-Link route + +`rtt.py` above is this route packaged. Raw form (what it runs): + +```bash +JLinkExe -USB <serial> -device <dev> -if swd -speed 4000 -NoGui 1 -AutoConnect 1 \ + -RTTTelnetPort <port> # keep stdin open; 'exit' tears it down +nc localhost <port> # JLinkRTTClient minus the banner; carries input too +``` + +Commander keeps hunting for the control block and delivers the buffered boot +burst once the target's first printf creates it. `JLinkGDBServer +-RTTTelnetPort` also serves the port but on some parts (measured: LPC4088) +never locates the control block **unless a GDB client attaches** — fine +inside a GDB session, a silent failure headless — and it briefly halts the +core on connect (measured), which matters for timing-sensitive repros; +Commander does not. One telnet client per port at a time. + +## Capture: OpenOCD route (native probes: ST-Link, CMSIS-DAP) + +This is the LIVE route — WCH-Link targets are SDI and get only the halt→dump +route (transport matrix). Same script, openocd backend (`--elf` = the +FLASHED elf; the script takes the exact control-block address from `nm` — +a full-RAM scan is slower and can match stale RAM after a soft reset): + +```bash +python3 tools/rtt.py --backend openocd --probe <serial> \ + --cfg "-f interface/stlink.cfg -f target/stm32h7x.cfg" --elf <flashed.elf> --seconds 20 +# --channel: up-buffer index (0 = "Terminal" console, 1 = SystemView's "SysView" +# buffer in TinyUSB builds); -i forwards stdin → down-buffer 0 +# --vid-pid "0x2e8a 0x000c": pin the probe by USB IDs (with or instead of --probe; +# also keeps openocd discovery off foreign usbfs nodes) +# --addr 0x2000xxxx: explicit control-block address when the flashed elf is not at hand +# --reset-before-attach: reset the target INSIDE the session (2 s settle, then +# attach — the control block must exist before `rtt start` can find it; the ring's +# NO_BLOCK_SKIP head-retention is what preserves byte 0 across the settle) — +# required for streams that only decode from byte 0 +# (SystemView emits its Init record, carrying the timestamp frequency, once at boot; +# a mid-flight attach yields a stream no decoder can lock onto). Verified on +# stm32h743nucleo: after the ring is drained, a plain attach misses the boot preamble +# entirely and this flag captures it. NOT for SAMD5x (an in-session reset via the DSU +# leaves the core held) or WCH SDI. +``` + +What it runs: `openocd <cfg> -c "adapter serial <sn>" -c init -c "rtt setup +<nm-addr> 0x800 \"SEGGER RTT\"" -c "rtt polling_interval 1" -c "rtt start" +-c "rtt server start <port> <ch>"`, then a socket on that port. + +Attach WITHOUT reset when the flash step already reset the board (on SAMD5x, +an in-session `reset run` goes through the DSU CPU Reset Extension and leaves +the core held). After any reset the target's offsets restart at zero while +the server holds stale ones, and the tool exposes no console to type into (it +launches openocd with tcl/gdb/telnet ports disabled): stop the capture and +run it again to resync — do not reset mid-capture if you can avoid it. `rtt start` +fails while the block doesn't exist yet: it appears at the firmware's first +RTT write, so reset, settle ~500 ms, then start. Read AND write validated on +the ci rig's 8 native-probe boards (ST-Link + CMSIS-DAP, incl. RP2350), +end-to-end through this script's backend on all 8 — per-board rows in +boards.md. OpenOCD polls, and host-side loss is invisible +to the target's overflow counter: at the default 100 ms interval a busy +stream loses most samples (measured 2066 of 5064 events/s delivered on +stm32f407disco) — `rtt polling_interval 1` is mandatory for quantitative +capture, not a tuning nicety. Prefer SEGGER tools where a J-Link exists. + +## Post-mortem: reading the ring without a live server + +Default log mode is `NO_BLOCK_SKIP`: with no reader draining, the ring holds +the **first KB after boot, not the tail** — interpretation rules in +`target-debug`. To keep the last N bytes instead, the firmware must log via +`SEGGER_RTT_WriteWithOverwriteNoLock` (target drags `RdOff` itself; no host +needed) — but SEGGER's own restriction comes with it: *"Do not use +SEGGER_RTT_WriteWithOverwriteNoLock if a J-Link connection reads RTT data"* +(`lib/SEGGER_RTT/RTT/SEGGER_RTT.c`), because the target moving `RdOff` races +the host reader. So it is for firmware you dump post-mortem, never for a +board that also runs a live console (every HIL rtt board does). Reading a wedged target's ring — debug-AP RAM reads don't halt the +core: + +```bash +python3 tools/rtt.py --backend jlink --dump ring.bin \ + --probe <serial> --device <JLINK_DEVICE> --elf <flashed.elf> # or --addr 0x... +# prints pBuffer/Size/WrOff/RdOff; WrOff/RdOff delimit the valid bytes +``` + +(What it runs, for hand-driving JLinkExe: `nm` the ELF for `_SEGGER_RTT`, +`mem32 <addr+0x18>, 6` = aUp[0] {sName,pBuffer,Size,WrOff,RdOff,Flags}, +then `savebin <file> <pBuffer> <SizeOfBuffer>`.) + +## Buffer modes and locking (target side) + +- Modes: `NO_BLOCK_SKIP` (default for logs — drops whole writes when full), + `NO_BLOCK_TRIM`, `BLOCK_IF_FIFO_FULL` (target spins — dangerous in ISRs). +- Throughput is drain-limited: measured 24.6 KiB/s over a J-Link console + against a saturating printf loop, with the drops happening at the target. + RTT console output is NOT lossless under load; for high-bandwidth streams + size the buffer up (SystemView needs 2048–8192) and watch for overflow. +- Non-ARM ports must supply `SEGGER_RTT_LOCK/UNLOCK`: the vendored generic + RISC-V lock uses `mstatus` CSRs that trap (mcause=2) on WCH QingKe. Worked + port on branch `claude/add-systemview-debug`: `hw/bsp/ch583/ + sysview_rtt_lock_wch.h` (brace-scoped save/restore of CSR 0x800), and the + shared `hw/bsp/sysview_rtt_conf_wch.h` that ch32v20x/ch32v30x family.cmake + force-include to win the include-guard race against the vendored conf. + +## Common mistakes + +- **Attaching before the first printf** — the control block is zeroed `.bss` + until the firmware's first RTT write; early readers see nothing (and + RTTLogger gives up for good). Commander/`rtt.py` keep hunting. +- **Sending input before the server finds the control block** — the J-Link + telnet route silently DROPS client bytes until then (measured on the rig: + an instant `ping` vanished, a delayed one echoed). `rtt.py -i` + holds stdin until target output flows (or 5 s); when driving the raw + socket yourself, wait for output before writing. +- **Resetting while a console is attached** — flash and reset first; the + console owns the probe until closed. +- **Killing servers with `pkill -f`** — the pattern matches your own shell's + cmdline (and unrelated sessions): a compound command that pkills its + wrapper then re-reads a stale log misdiagnosed a healthy probe for an + hour. Close `rtt.py` with Ctrl-C/`--seconds` (its teardown reaps + the whole process group); if you must pattern-kill, bracket a char: + `pkill -f '[J]LinkExe -USB <serial>'`. +- **Unpinned flash with several probes attached** — pin by serial, always. +- **Two probes wired to one SWD header** — wedges the target; rewire. +- **Expecting an echo from firmware that never reads the console** — only + code polling `board_getchar()` consumes down-buffer 0 (`board_test` does). +- **Full-RAM `rtt setup` scans** — can lock onto a stale pre-reset block; + use the `nm` address. diff --git a/.claude/skills/rtt/boards.md b/.claude/skills/rtt/boards.md new file mode 100644 index 000000000..5ddd072d2 --- /dev/null +++ b/.claude/skills/rtt/boards.md @@ -0,0 +1,78 @@ +# rtt — per-board validation matrix + +A row appears here only after the board was exercised on real hardware; a new +validation adds the row AND any caveat it surfaced. "Read" = console/log +capture reached the host; "Write" = the target demonstrably consumed console +input (a printf-echo `board_test` returned the sent bytes — stock +`board_test` cannot, see SKILL.md's echo-validation note). Routes match +SKILL.md's capture sections; `Device/cfg` is the J-Link `--device` string or +the openocd target cfg. Rig rows (ci.lan) were validated 2026-08-24 by a +flash→capture→`ping`-echo sweep under per-board `hil_lock` flocks, and +re-validated 2026-08-25 end-to-end through the skill's own CLI +(`tools/rtt.py`, jlink + openocd backends): 20/20 read+write — +including CONCURRENTLY at 8 parallel consoles (20 boards in 39 s, mixed +routes, no port collisions or cross-board output bleed: one server per +probe on its own ephemeral port). htpc rows on the local bench. The openocd backend's `--reset-before-attach` +is decode-validated: a channel-1 SystemView capture on stm32h743nucleo +(byte-identical boot preamble to the sysview campaign's golden reference, +49765 events decoded, ISR/task timings matching to 0.1 µs, overflow 0). + +| Board | Rig | Probe | Route | Read | Write | Device/cfg | +| ------------------------ | ---- | ---------------------- | ------- | ---- | ----- | --------------------- | +| ea4088_quickstart | htpc | LPC-Link2 J-Link fw | J-Link | yes | yes | `LPC4088` | +| raspberry_pi_pico2 | htpc | J-Trace PRO | J-Link | yes | — | `rp2350_m33_0` | +| frdm_k64f | ci | J-Link | J-Link | yes | yes | `MK64FN1M0xxx12` | +| feather_nrf52840_express | ci | J-Link | J-Link | yes | yes | `nrf52840_xxaa` | +| metro_m4_express | ci | J-Link | J-Link | yes | yes | `ATSAMD51J19` | +| lpcxpresso11u37 | ci | J-Link | J-Link | yes | yes | `LPC11U37/401` | +| lpcxpresso55s28 | ci | J-Link | J-Link | yes | yes | `LPC55S28` | +| ra4m1_ek | ci | J-Link | J-Link | yes | yes | `R7FA4M1AB` | +| stm32f072disco | ci | J-Link | J-Link | yes | yes | `stm32f072rb` | +| stm32f407disco | ci | J-Link | J-Link | yes | yes | `stm32f407vg` | +| stm32f723disco | ci | J-Link | J-Link | yes | yes | `stm32f723ie` | +| stm32l476disco | ci | J-Link | J-Link | yes | yes | `STM32L476VG` | +| mimxrt1064_evk | ci | J-Link | J-Link | yes | yes | `MIMXRT1064xxx6A` | +| nrf54lm20dk | ci | J-Link | J-Link | yes | yes | `NRF54LM20A_M33` | +| max32666fthr | ci | CMSIS-DAP | OpenOCD | yes | yes | `target/max32665.cfg` | +| raspberry_pi_pico | ci | debugprobe (CMSIS-DAP) | OpenOCD | yes | yes | `target/rp2040.cfg` | +| raspberry_pi_pico_w | ci | debugprobe (CMSIS-DAP) | OpenOCD | yes | yes | `target/rp2040.cfg` | +| raspberry_pi_pico2 | ci | debugprobe (CMSIS-DAP) | OpenOCD | yes | yes | `target/rp2350.cfg` | +| adafruit_fruit_jam | ci | debugprobe (CMSIS-DAP) | OpenOCD | yes | yes | `target/rp2350.cfg` | +| stm32h743nucleo | ci | ST-Link | OpenOCD | yes | yes | `target/stm32h7x.cfg` | +| stm32g0b1nucleo | ci | ST-Link | OpenOCD | yes | yes | `target/stm32g0x.cfg` | +| stm32u083nucleo | ci | ST-Link | OpenOCD | yes | yes | `target/stm32u0x.cfg` | + +Probe serials live in the rig configs (`test/hil/tinyusb.json`, bench +`local.json`) — always pass them (`--probe` / `adapter serial`). + +## Caveats + +- **ea4088_quickstart**: probe has no VCOM and the BSP has no UART — RTT is + the ONLY console; measured there: 6/6 JLinkExe attaches, boot burst + delivered, 24.6 KiB/s drain; the HIL suite runs over the RTT console + (device_info-class tests — the cdc/msc-fixture host tests don't speak RTT + yet, see the follow-up doc). + NEVER point OpenOCD at this J-Link-firmware probe (jaylink knocks it off + USB; physical replug). JLinkGDBServer never finds the CB headless on this + part; JLinkRTTLogger 0/6. +- **raspberry_pi_pico2 (htpc, J-Trace)**: pin the probe by serial — that + bench runs two J-Links (`-DJLINK_OPTION="-USB <sn>"` for the flash + target). Never set a custom JLinkScript for RP2350 over J-Link. Write path + untested there only because the flashed example doesn't poll the console + (the ci row's debugprobe sweep validated RP2350 writes). +- **ST-Link rows**: flashed by `STM32_Programmer_CLI`; RTT capture is a + separate openocd session (`interface/stlink.cfg` + the target cfg above), + attach without reset. + +## Excluded (recorded so absence is never read as "works") + +- `espressif_s3_devkitm`, `espressif_p4_function_ev` — no SEGGER RTT path in + our builds (console is the chip's USB-Serial-JTAG; see `esp-target-debug`). +- `ek_tm4c123gxl` — flashed by `lm4flash`; no debug-probe path configured on + the rig. +- `nanoch32v203`, `ch32v103r_r1_1v0`, `ch32v307v_r1_1v0`, `ch582m_evt` — a + `LOGGER=rtt` build traps on WCH QingKe (the vendored generic RISC-V + `SEGGER_RTT_LOCK` reads `mstatus` CSRs → mcause=2; the working lock port + `sysview_rtt_lock_wch.h` lives only on branch `claude/add-systemview-debug`), + and SDI permits no live streaming anyway (transport matrix). Revisit after + that branch merges. diff --git a/.claude/skills/target-debug/SKILL.md b/.claude/skills/target-debug/SKILL.md index 050a697b9..7ee96f48e 100644 --- a/.claude/skills/target-debug/SKILL.md +++ b/.claude/skills/target-debug/SKILL.md @@ -217,40 +217,36 @@ dump binary memory /tmp/ring.bin &dbg_ring[0] &dbg_ring[512] ## TU_LOG capture Build with `LOG=2` (`LOG=3` adds per-transfer noise and much more timing skew). -`LOGGER=rtt` routes it over the debug probe — no UART wiring. SEGGER's host -tools need a J-Link, but OpenOCD serves the same RTT buffer on ST-Link / -CMSIS-DAP / WCH-Link boards: +`LOGGER=rtt` routes it over the debug probe — no UART wiring. Stand the +channel up per the **rtt** skill (servers per probe, transport matrix, +control-block gotchas live there): ```bash -# RTT: JLinkGDBServer from CLAUDE.md "GDB Debugging" + -RTTTelnetPort, then: -timeout 20s JLinkRTTClient > /tmp/rtt.log # non-interactive capture +# RTT (J-Link probe; flash + reset first — the console owns the probe): +timeout 20s python3 tools/rtt.py --backend jlink --probe <sn> --device <JLINK_DEVICE> > /tmp/rtt.log # UART (board's debug serial, if wired): stty -F /dev/ttyACM<N> 115200 raw && timeout 20s cat /dev/ttyACM<N> | tee /tmp/uart.log ``` -```bash -# OpenOCD RTT (any probe OpenOCD drives) — in telnet :4444 (or -c equivalents): -rtt setup 0x20000000 0x8000 "SEGGER RTT" # RAM ORIGIN + LENGTH (from the .ld/map) -rtt start # after firmware booted; rerun after each reflash -rtt server start 19021 0 -# then: timeout 20s nc localhost 19021 > /tmp/rtt.log -``` - -OpenOCD polls — bursty logs can drop lines; prefer J-Link where both -exist. The drain-model warning below applies unchanged. +OpenOCD RTT (native probes: ST-Link/CMSIS-DAP): rtt skill §OpenOCD — exact +CB address from `nm`, attach-only. OpenOCD polls — bursty logs can drop +lines; prefer J-Link where both exist. The drain-model warning below +applies unchanged. An RTT-built firmware that has since wedged still holds a log tail in RAM — but ONLY what fits the drain model: the default SEGGER mode (NO_BLOCK_SKIP) **drops** writes once the ring fills with no reader, so an undrained target -holds the first KB after boot, not the wedge tail. There is no overwrite mode -in stock SEGGER RTT (only SKIP/TRIM/BLOCK): post-mortem RTT is evidence only -if a live drain was running — otherwise instrument with the RAM ring above. -Use `JLinkGDBServer -RTTTelnetPort 19021` + `JLinkRTTClient` for the drain -(proven; note the server briefly halts the core on connect). `JLinkRTTLogger` -fails to find the control block on some parts (LPC4088) even when it exists -and even given `-RTTAddress`; don't fight it — `nm` the ELF for `_SEGGER_RTT`, -read the aUp[0] descriptor (`mem32`), `savebin` the buffer — debug-AP RAM -reads don't halt the target. +holds the first KB after boot, not the wedge tail. The buffer flags have no +overwrite mode (only SKIP/TRIM/BLOCK); keeping the tail instead requires the +firmware-side overwrite write call (rtt skill §post-mortem). So post-mortem +RTT from a default-mode build is evidence only if a live drain was running — +otherwise instrument with the RAM ring above. +Stand up the drain per the **rtt** skill: JLinkExe's `-RTTTelnetPort` (what +`rtt.py` wraps) is the headless-proven route; JLinkGDBServer's needs +a GDB client attached on some parts (LPC4088), and JLinkRTTLogger fails to +find the control block on some parts (measured LPC4088, 0/6). The manual +ring read for a wedged target (`nm`/`mem32`/`savebin` — debug-AP reads don't +halt the core) lives there too. ## GDB — state autopsy and watchpoints @@ -331,7 +327,7 @@ Linux gadget peer): ```bash .claude/skills/usbmon/scripts/usbcap.sh cafe: 30 /tmp/host.pcapng & # host URBs (usbmon skill) -timeout 30s JLinkRTTClient > /tmp/target.rtt & # target (or ring dump after) +timeout 30s python3 tools/rtt.py --backend jlink --probe <sn> --device <dev> > /tmp/target.rtt & # target (rtt skill; or ring dump after) wait ``` diff --git a/.claude/skills/usbtest/SKILL.md b/.claude/skills/usbtest/SKILL.md index 4d7c6e5ab..606b379b5 100644 --- a/.claude/skills/usbtest/SKILL.md +++ b/.claude/skills/usbtest/SKILL.md @@ -30,7 +30,7 @@ python3 test/hil/usbtest.py --serial <uid> --keep-binding --tests 29 # one case ``` - **Always `--keep-binding`**: the cleanup unbind path has wedged host xHCIs (`usb_hcd_alloc_bandwidth`). -- CI (`hil_test.py`) additionally passes `--budget`, `--outer-timeout` and +- CI (`hil_test.py`) additionally passes `--budget` and `--recover-board`/`--recover-fw`: on a HUNG case the battery aborts, RESETS the DUT through its roster probe (non-destructive, ~130 ms) and reflashes only if that does not clear the wedge (see usb-kernel-recover). Manual runs without those flags leave a HUNG diff --git a/.claude/workflows/pr-babysit.js b/.claude/workflows/pr-babysit.js index 2731ed254..8438d4d04 100644 --- a/.claude/workflows/pr-babysit.js +++ b/.claude/workflows/pr-babysit.js @@ -129,6 +129,10 @@ const postReplyRecipe = (noun) => const history = [] const repliedIds = new Set() // issue comments can't be thread-resolved, so they re-harvest every cycle — never reply twice +// Backoff between cycles that have nothing to do but wait. Degrades to a no-op +// rather than throwing if the workflow host has no timer. +const nap = (ms) => new Promise(res => { if (typeof setTimeout === 'function') setTimeout(res, ms); else res() }) + // Canonicalize a repo-relative path for set/collision comparison: resolve ./.. // segments, unify separators; '' for anything that escapes the repo or uses // characters no repo path does (also makes the path shell-safe to interpolate). @@ -200,11 +204,28 @@ const fixAndVerify = async (workIn) => { } work.push(g) } + // HIL rig rosters (test/hil/*.json) describe physical hardware the user owns: + // never edit them autonomously — skipping/reshaping tests there papers over a + // failing fixture. A failure that needs hardware swapped or re-cabled stays RED + // for the user; roster edits happen only with the user's explicit approval. + const withheld = [] + for (const w of work) { + for (const f of [...w.files]) if (/^test\/hil\/[^/]+\.json$/.test(f)) { + w.files.delete(f) + log(`fix for ${w.key}: ${f} is a HIL rig config — edits need user approval, dropped from scope`) + } + if (w.files.size === 0) { + withheld.push(w) + log(`fix for ${w.key}: only a HIL rig config edit would address it — leaving red for the user`) + } + } + for (const w of withheld) work.splice(work.indexOf(w), 1) const scopeOf = (w) => [...w.files].join(', ') const fixes = await pipeline( work, w => agent( `Fix the following issues on the PR branch. ${IN_CHECKOUT}\n` + + 'Constraint: never modify test/hil/*.json (HIL rig hardware config) — a failure that needs hardware swapped/changed stays red for the user.\n' + `Scope: ${scopeOf(w)}\nIssues:\n- ${w.notes.join('\n- ')}`, { label: `fix:${w.key}`, phase: 'Fix', agentType: 'code-writer', schema: DEV }, ), @@ -218,7 +239,7 @@ const fixAndVerify = async (workIn) => { if (alive.length < work.length) log(`${work.length - alive.length} fix group(s) lost to dead workers`) const unverified = alive.filter(f => f.addresses !== true) for (const f of unverified) log(`fix for ${f.item}: failed verification — ${f.checkReason}`) - return { ok: unscoped.length === 0 && alive.length === work.length && unverified.length === 0, fixes: alive } + return { ok: unscoped.length === 0 && withheld.length === 0 && alive.length === work.length && unverified.length === 0, fixes: alive } } // Verification gates every push: never push unverified or partial edits. @@ -370,6 +391,18 @@ for (let cycle = 1; cycle <= maxCycles; cycle++) { log(`cycle ${cycle}: CI still settling (${c.infraRerun.length} infra re-run(s)) — re-arming`) continue } + if (!r.done) { + // A bot has not reported for this head SHA yet. With CI already green there is + // nothing else to wait on, so back off before re-arming or the cycle budget + // burns on back-to-back re-harvests of the same unchanged PR. + if (cycle < maxCycles) { + log(`cycle ${cycle}: auto-review still pending — re-arming after a wait`) + await nap(60000 * cycle) // no wait on the last cycle: nothing would re-check after it + } else { + log(`cycle ${cycle}: auto-review still pending — cycle budget exhausted`) + } + continue + } log(`cycle ${cycle}: nothing actionable`) return { pass: false, cycles: cycle, history, reason: 'unactionable' } } diff --git a/.claude/workflows/validate.js b/.claude/workflows/validate.js index dadf8a77f..773dd47ad 100644 --- a/.claude/workflows/validate.js +++ b/.claude/workflows/validate.js @@ -1,18 +1,33 @@ export const meta = { name: 'validate', - description: 'Pre-PR software validation: unit tests + per-board build sweeps + code-size compare + PVS + diff reviews (claude + codex), in parallel, joined into one verdict', + description: 'Pre-PR software validation loop: unit tests + per-board build sweeps + code-size compare + PVS + diff reviews (claude + codex) in parallel; a red verdict dispatches a fix agent for the confirmed findings, then the affected stages re-run — up to maxCycles (default 5) validation passes; a fix that edits a workflow file stops with restartRequired so the caller re-invokes it', whenToUse: 'Before opening or updating a PR, after any non-trivial change', - phases: [{ title: 'Validate', detail: 'unit + builds + size + pvs + reviews in parallel' }], + phases: [ + { title: 'Validate', detail: 'unit + builds + size + pvs + reviews in parallel' }, + { title: 'Fix', detail: 'one fix agent per red cycle; commits, then affected stages re-run' }, + ], } -// args: { boards: string[], examples?: string, base?: string, skip?: ('unit'|'size'|'pvs'|'review'|'codex')[] } +// args: { boards: string[], examples?: string, base?: string, +// skip?: ('unit'|'size'|'pvs'|'review'|'codex')[], maxCycles?: number } if (typeof args === 'string') { try { args = JSON.parse(args) } catch { /* not JSON: shape check below reports it */ } } if (!args || !Array.isArray(args.boards) || args.boards.length === 0) { - throw new Error('args must be { boards: string[], examples?, base?, skip? }') + throw new Error('args must be { boards: string[], examples?, base?, skip?, maxCycles? }') } +if (args.maxCycles !== undefined && (!Number.isInteger(args.maxCycles) || args.maxCycles < 1)) { + throw new Error('maxCycles must be an integer >= 1') +} +const maxCycles = args.maxCycles ?? 5 const skip = args.skip || [] for (const s of skip) log(`stage skipped by request: ${s}`) const base = args.base || 'master' +// Every stage agent re-resolves `base` in every cycle, and the fixer commits +// between cycles: a moving expression (HEAD~1, @{u}, a ^/~ walk) would advance +// with each fix commit, so cycle 2 would review only the fix and silently drop +// the original branch changes. Accept stationary refs only. +if (/(^|[^\w/-])HEAD/.test(base) || /[~^]/.test(base) || base.includes('@{')) { + throw new Error(`base must be a fixed ref (sha or branch name), not the moving expression "${base}" — resolve it with git rev-parse first`) +} const clip = (s, n = 800) => s.length > n ? s.slice(0, n) + ` …[truncated ${s.length - n} chars]` : s @@ -76,44 +91,83 @@ const REVIEW = { }, } -const thunks = [] +const FIX = { + type: 'object', additionalProperties: false, + required: ['changed', 'commit', 'files', 'summary'], + properties: { + changed: { type: 'boolean' }, commit: { type: 'string' }, + files: { type: 'array', items: { type: 'string' } }, + summary: { type: 'string' }, + }, +} + +// read back out of git what the fix commit actually touched +const PATHS = { + type: 'object', additionalProperties: false, + required: ['paths', 'isHead'], + properties: { + paths: { type: 'array', items: { type: 'string' } }, + isHead: { type: 'boolean' }, + }, +} + +// gate helpers — enforced here, never trusted from the agents +const confirmedReview = f => + /^confirmed/i.test(f.severity) && !/quality|simplification|style/i.test(f.severity) +const codexBlocking = f => /\bP[01]\b/i.test(f.severity) + +// --------------------------------------------------------------------------- +// Stage builders, parameterized so later cycles can re-run a subset. Stage +// names: 'unit', 'build:<board>', 'size', 'pvs', 'review', 'codex'. +// --------------------------------------------------------------------------- +const stageNames = [] +if (!skip.includes('unit')) stageNames.push('unit') +for (const b of args.boards) stageNames.push(`build:${b}`) +if (!skip.includes('size')) stageNames.push('size') +if (!skip.includes('pvs')) stageNames.push('pvs') +if (!skip.includes('review')) stageNames.push('review') +if (!skip.includes('codex')) stageNames.push('codex') + +function stageThunk(name, cycle) { + const label = (cycle > 1 ? `c${cycle}:` : '') + name + // findings: [] so a dead review/codex stage flows through fixerEvidence() + // instead of throwing on f.findings inside the fix dispatch's catch + const died = { stage: name, pass: false, findings: [], detail: 'stage agent died' } -if (!skip.includes('unit')) thunks.push(() => - agent( + if (name === 'unit') return () => agent( 'Run the TinyUSB unit tests: cd test/unit-test && ceedling test:all. ' + 'pass=true only if every test passes. detail = the ceedling summary line, or the first failing test output.', - { label: 'unit', phase: 'Validate', model: 'haiku', schema: STAGE }, - ).then(r => r && { stage: 'unit', ...r })) + { label, phase: 'Validate', model: 'haiku', schema: STAGE }, + ).then(r => r ? { stage: name, ...r } : died).catch(() => died) -for (const b of args.boards) thunks.push(() => - agent( - `Build TinyUSB examples for board ${b}` + (args.examples ? ` (only: ${args.examples})` : ' (full example set)') + '.', - { label: `build:${b}`, phase: 'Validate', agentType: 'builder', schema: BUILD }, - ).then(r => r && { - stage: `build:${b}`, pass: r.pass, - detail: r.pass ? `${r.builtCount} examples built` : clip(JSON.stringify(r.failures)), - })) + if (name.startsWith('build:')) { + const b = name.slice('build:'.length) + return () => agent( + `Build TinyUSB examples for board ${b}` + (args.examples ? ` (only: ${args.examples})` : ' (full example set)') + '.', + { label, phase: 'Validate', agentType: 'builder', schema: BUILD }, + ).then(r => r ? { + stage: name, pass: r.pass, + detail: r.pass ? `${r.builtCount} examples built` : clip(JSON.stringify(r.failures)), + } : died).catch(() => died) + } -if (!skip.includes('size')) thunks.push(() => - agent( + if (name === 'size') return () => agent( `Compare TinyUSB code size against ${base}: python3 tools/metrics_compare_base.py --base-branch ${base} -b ${args.boards[0]} -e device/cdc_msc (exactly this command — no extra positional args). ` + 'The report lands in cmake-metrics/<board>/metrics_compare.md. pass=false only if the tool itself errors; ' + 'detail = the flash/RAM delta summary from the report (mention any example that grew).', - { label: 'size', phase: 'Validate', model: 'haiku', schema: STAGE }, - ).then(r => r && { stage: 'size', ...r })) + { label, phase: 'Validate', model: 'haiku', schema: STAGE }, + ).then(r => r ? { stage: name, ...r } : died).catch(() => died) -if (!skip.includes('pvs')) thunks.push(() => - agent( + if (name === 'pvs') return () => agent( `Run PVS-Studio static analysis for board ${args.boards[0]}, gating on files changed vs ${base}. ` + 'Parallel build agents are running — use your dedicated build dir, never cmake-build-<board>.', - { label: 'pvs', phase: 'Validate', agentType: 'static-analyzer', effort: 'low', schema: PVS }, - ).then(r => r && { - stage: 'pvs', pass: r.pass, + { label, phase: 'Validate', agentType: 'static-analyzer', effort: 'low', schema: PVS }, + ).then(r => r ? { + stage: name, pass: r.pass, detail: r.pass ? r.detail : clip(`${r.detail} ${JSON.stringify(r.changedFindings)}`), - })) + } : died).catch(() => died) -if (!skip.includes('review')) thunks.push(() => - agent( + if (name === 'review') return () => agent( `Code-review this branch's diff vs ${base} (git diff ${base}...HEAD), coverage-first: walk every hunk, no spot checks. ` + 'Find pass — candidate defects across all dimensions: correctness/logic, ISR & concurrency safety, ' + 'memory/resource handling (bounds, leaks, no dynamic alloc), API contract & spec conformance, ' + @@ -123,32 +177,162 @@ if (!skip.includes('review')) thunks.push(() => 'Read-only: never apply fixes. severity = verdict plus category (e.g. "CONFIRMED correctness"). ' + 'pass=false if any CONFIRMED correctness/safety/security bug survives; PLAUSIBLE and quality findings keep pass=true. ' + 'detail = one-line review summary.', - { label: 'review', phase: 'Validate', model: 'opus', effort: 'high', schema: REVIEW }, - ).then(r => r && { - stage: 'review', - // gate enforced here, not trusted from the agent: any CONFIRMED non-quality finding fails - pass: r.pass && !r.findings.some(f => - /^confirmed/i.test(f.severity) && !/quality|simplification|style/i.test(f.severity)), + { label, phase: 'Validate', model: 'opus', effort: 'high', schema: REVIEW }, + ).then(r => r ? { + stage: name, + pass: r.pass && !r.findings.some(confirmedReview), findings: r.findings, detail: r.detail, - })) + } : died).catch(() => died) -if (!skip.includes('codex')) thunks.push(() => - agent( + if (name === 'codex') return () => agent( `Run a Codex review of this branch's diff vs ${base}: ` + `codex review --base ${base} -c model="gpt-5.6-sol" -c model_reasoning_effort="high" ` + '(Bash timeout 600000; run from the repo root). Parse its output into findings; severity = Codex\'s priority label. ' + 'pass=false only if Codex reports a correctness bug (P0/P1); style-level items keep pass=true. ' + 'detail = Codex\'s overall verdict line. If the codex CLI is missing or the run errors, pass=false with the error in detail.', - { label: 'codex', phase: 'Validate', model: 'haiku', schema: REVIEW }, - ).then(r => r && { - stage: 'codex', - pass: r.pass && !r.findings.some(f => /\bP[01]\b/i.test(f.severity)), + { label, phase: 'Validate', model: 'haiku', schema: REVIEW }, + ).then(r => r ? { + stage: name, + pass: r.pass && !r.findings.some(codexBlocking), findings: r.findings, detail: r.detail, - })) + } : died).catch(() => died) + + throw new Error(`unknown stage ${name}`) +} + +// Only the material that FAILED the gate reaches the fixer: confirmed review +// findings, codex P0/P1, and failed unit/build/size/pvs stage evidence. +// PLAUSIBLE and quality findings stay report-only — fixing them here would +// churn style on an otherwise green branch. Bounded at the leaves (per-stage +// finding cap, clipped summaries/details) so the serialized JSON stays valid +// and every failed stage is represented — a document-level clip could cut +// mid-JSON and silently drop trailing stages. +function fixerEvidence(failures) { + return failures.map(f => { + const findings = (f.findings || []) + .filter(f.stage === 'review' ? confirmedReview : codexBlocking) + .slice(0, 10) + .map(x => ({ ...x, summary: clip(x.summary, 300) })) + if (f.stage === 'review' || f.stage === 'codex') + return { stage: f.stage, detail: clip(f.detail, 300), findings } + return { stage: f.stage, detail: clip(f.detail) } + }) +} + +function fixThunkPrompt(cycle, failures) { + return 'You are the fix agent of the validate loop, cycle ' + cycle + ', in this TinyUSB repo (work from the repo root). ' + + 'Failed stages: ' + failures.map(f => f.stage).join(', ') + '. ' + + 'The JSON below carries their evidence (review findings are pre-verified CONFIRMED, codex ones are P0/P1):\n' + + JSON.stringify(fixerEvidence(failures), null, 1) + '\n\n' + + 'For each item: verify it against the actual code first; fix the real ones with the smallest correct change, matching surrounding style. ' + + 'Skip anything that is an infrastructure failure rather than a code defect (missing CLI, tool crash, dead stage agent) and anything you can refute with evidence — say which and why in summary. ' + + 'Run the tests/suites covering what you changed. ' + + 'BEFORE editing anything, run git status --porcelain and record every path already dirty in either column ' + + '(staged or unstaged — those are someone else\'s in-flight edits, and git add <path> would sweep them into your commit). ' + + 'If any file you need to modify is in that set, edit nothing at all: return changed=false, naming the file in summary. ' + + 'Commit as ONE commit, staging ONLY the files you changed (git add <paths> — never git add -A or git commit -a). ' + + 'Message: imperative mood, subject like "validate: fix cycle ' + cycle + ' findings", ' + + 'NO trailers of any kind (no Co-Authored-By, no Claude-Session). Do NOT push. Never spawn subagents. ' + + 'Return: changed=true only if you committed; commit = the new sha (empty string if none); ' + + 'files = the commit\'s own paths, verbatim from git show --name-only --format= HEAD (empty if you did not commit); ' + + 'summary = one paragraph of what was fixed/skipped and why.' +} + +// --------------------------------------------------------------------------- +// The loop: validate → (red) fix → re-run affected stages, up to maxCycles +// validation passes. Reviews always re-run after a fix (their input is the +// diff, which just changed); unit/builds/size/pvs re-run only when the fix +// touched code they consume, or when they failed themselves. +// --------------------------------------------------------------------------- +const latest = new Map() // stage name -> most recent result +const history = [] +let toRun = new Set(stageNames) + +for (let cycle = 1; cycle <= maxCycles; cycle++) { + log(`cycle ${cycle}/${maxCycles}: running ${toRun.size}/${stageNames.length} stage(s)`) + const results = await parallel([...toRun].map(n => stageThunk(n, cycle))) + for (const r of results.filter(Boolean)) latest.set(r.stage, r) + const failures = [...latest.values()].filter(r => !r.pass) + const entry = { cycle, ran: [...toRun], failed: failures.map(f => f.stage), fix: null } + history.push(entry) + + if (failures.length === 0) { + log(`cycle ${cycle}: all stages green`) + return { pass: true, cycles: history, stages: [...latest.values()], failures: [] } + } + log(`cycle ${cycle}: ${failures.length} stage(s) failing: ${entry.failed.join(', ')}`) + if (cycle === maxCycles) break + + const fix = await (async () => { + try { + return await agent(fixThunkPrompt(cycle, failures), + { label: `c${cycle}:fix`, phase: 'Fix', model: 'sonnet', schema: FIX }) + } catch { return null } + })() + if (!fix) { entry.fix = 'fix agent died'; break } + entry.fix = { changed: fix.changed, commit: fix.commit, files: fix.files, summary: clip(fix.summary) } + if (!fix.changed) { + // Nothing fixable in code. A dead stage agent is still worth retrying — + // that failure is transient infrastructure, and a retry is the only + // useful action for it. Everything else (refuted findings, missing CLI) + // would just spin, so stop with the report. + const deadStages = failures.filter(f => f.detail === 'stage agent died').map(f => f.stage) + if (deadStages.length > 0) { + log(`cycle ${cycle}: fix agent changed nothing — retrying dead stage(s): ${deadStages.join(', ')}`) + toRun = new Set(deadStages) + continue + } + log(`cycle ${cycle}: fix agent changed nothing — stopping`) + break + } + + // What re-runs is gated on what the commit actually contains, never on the + // fixer's self-report (the FIX schema lets `files` be empty or wrong, and a + // code fix reported as a doc path would keep stale-green results). Read the + // paths back out of git; if that read fails, fall back to the self-report and + // grant no exemption below. + const verified = await (async () => { + if (!fix.commit) return null + try { + return await agent( + `In this repo run: git show --name-only --format= ${fix.commit} and git rev-parse HEAD. ` + + 'paths = the repo-relative paths that commit touched, verbatim, one per array entry; ' + + `isHead = true only if git rev-parse HEAD is exactly ${fix.commit}. ` + + 'Read-only: edit nothing, commit nothing, never spawn subagents.', + { label: `c${cycle}:fix-paths`, phase: 'Fix', model: 'haiku', schema: PATHS }) + } catch { return null } + })() + const trusted = !!(verified && verified.isHead && verified.paths.length > 0) + const files = trusted ? verified.paths : (fix.files || []) + entry.fix.files = files + entry.fix.verified = trusted + if (!trusted) log(`cycle ${cycle}: could not confirm the fix commit's paths — treating the fix as touching everything`) + + // The fixer rewrote this workflow, but the stage thunks, the gates and this + // loop are the old file — already loaded in memory. Re-running here would + // validate the corrected workflow with superseded orchestration and could + // report green off it, so hand the restart back to the caller instead. + const workflowFiles = files.filter(f => /^\.claude\/workflows\//.test(f)) + if (workflowFiles.length > 0) { + log(`cycle ${cycle}: the fix commit edits ${workflowFiles.join(', ')} — stopping. ` + + 'Re-invoke validate so the committed workflow is loaded fresh; this run\'s verdict is not final.') + return { pass: false, restartRequired: true, cycles: history, stages: [...latest.values()], failures } + } + + // A fix can invalidate any stage: builds/unit consume src|hw|examples|test, + // and size/pvs run tools the fixer may have edited. Only a pure-docs fix + // is safe to exempt — everything else re-runs the full stage set. Agent and + // skill instructions are Markdown but drive the stage agents themselves, so + // they are operational, not documentation: editing them must re-run + // everything, or the loop reports green on results the old instructions produced. + const docsOnly = trusted && files.length > 0 && files.every(f => + !f.startsWith('.claude/') && f !== 'CLAUDE.md' && f !== 'AGENTS.md' && + (/^docs\//.test(f) || f.endsWith('.md') || f.endsWith('.rst'))) + toRun = new Set(failures.map(f => f.stage)) + if (!skip.includes('review')) toRun.add('review') + if (!skip.includes('codex')) toRun.add('codex') + if (!docsOnly) for (const n of stageNames) toRun.add(n) +} -const results = (await parallel(thunks)).filter(Boolean) -const dead = thunks.length - results.length -if (dead > 0) log(`${dead} stage agent(s) died — counted as failures`) -const failures = results.filter(r => !r.pass) -log(`${results.length}/${thunks.length} stages completed, ${failures.length} failing`) -return { pass: failures.length === 0 && dead === 0, stages: results, failures } +const failures = [...latest.values()].filter(r => !r.pass) +return { pass: false, cycles: history, stages: [...latest.values()], failures } |