Multi-agent dev/test harness: worker agents, workflows, /pre-pr gate, per-board HIL locks#3762
Multi-agent dev/test harness: worker agents, workflows, /pre-pr gate, per-board HIL locks#3762hathach wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces a Claude Code multi-agent development/test harness for TinyUSB (custom worker agents + deterministic workflow scripts + /pre-pr entry skill) and replaces the “stop the runner” HIL rig discipline with per-board kernel flock locking so CI and dev sessions can safely share hardware concurrently.
Changes:
- Add
.claude/agents/worker definitions and.claude/workflows/scripts to orchestrate build/verify/review/HIL/PR-triage tasks. - Add per-board HIL locking via
test/hil/board_lock.pyand ahil_test.pyguard that self-locks each board for flash+test. - Add design/spec + implementation plan + smoke records under
docs/superpowers/, plus updates to the HIL skill docs to reflect the new lock protocol.
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
test/hil/hil_test.py |
Adds per-board lock acquisition around each board’s flash+test run. |
test/hil/board_lock.py |
New CLI utility to hold/release/status per-board advisory locks using kernel flock. |
docs/superpowers/specs/2026-07-09-claude-agents-workflows-design.md |
Design spec for the multi-agent harness and lock protocol. |
docs/superpowers/plans/2026-07-09-smoke-results.md |
Smoke-test notes/results for the harness and HIL lock behavior. |
docs/superpowers/plans/2026-07-09-claude-agents-workflows.md |
Detailed implementation plan for agents/workflows/skill + lock protocol. |
.claude/workflows/check.sh |
Node-based syntax checker for workflow JS (wraps to allow top-level return). |
.claude/workflows/validate.js |
Parallel software gate: unit + per-board builds + size compare + PVS. |
.claude/workflows/fanout-dev.js |
Fan-out development workflow (port-dev per item + optional verify/review). |
.claude/workflows/driver-review.js |
Driver audit workflow with adversarial verification pass. |
.claude/workflows/hil-validate.js |
Serialized HIL runner workflow (supports lock-aware retry and optional force). |
.claude/workflows/full-check.js |
Composes validate then hil-validate (when requested) into one gate. |
.claude/workflows/pr-babysit.js |
Iterative PR triage/fix/verify/push workflow with optional auto-push authorization. |
.claude/skills/pre-pr/SKILL.md |
/pre-pr skill instructions for deriving boards and launching full-check. |
.claude/skills/hil/SKILL.md |
Updates HIL procedure docs to use per-board locks instead of stopping runner. |
.claude/agents/builder.md |
Builder agent contract for example build sweeps (structured JSON output). |
.claude/agents/port-dev.md |
Port developer agent contract (scoped edits + clang-format + build verify). |
.claude/agents/driver-reviewer.md |
Read-only driver reviewer/verifier contract (structured findings/verdicts). |
.claude/agents/hil-operator.md |
Rig operator agent contract (lock protocol, no runner stops, HIL runs). |
.claude/agents/pr-monitor.md |
PR monitor agent contract (CI triage + bot finding validation + drafted replies). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| `Compare TinyUSB code size against ${base}: python3 tools/metrics_compare_base.py -b ${args.boards[0]} -e device/cdc_msc . ` + | ||
| '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 }, |
| `Post these threaded replies to review comments on PR #${args.pr}. For each reply: try ` + | ||
| `gh api repos/{owner}/{repo}/pulls/${args.pr}/comments/{commentId}/replies -f body=<body> ` + | ||
| '(valid for inline review comments); if that 404s, the id is an issue comment — post a regular PR comment instead ' + | ||
| `(gh pr comment ${args.pr} --body <quote the original point, then the reply>). ` + | ||
| `Replies: ${JSON.stringify(t.replies)}. pass=true only if every reply was posted; detail = what went where.`, |
| try: | ||
| fcntl.flock(fh, fcntl.LOCK_EX | fcntl.LOCK_NB) | ||
| except OSError: | ||
| try: | ||
| info = fh.read(500).strip() | ||
| except (OSError, UnicodeDecodeError): | ||
| info = '' | ||
| fh.close() | ||
| raise RuntimeError(f'board locked: {info or "unknown holder"}') |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 186a6876e1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| if (!skip.includes('size')) thunks.push(() => | ||
| agent( | ||
| `Compare TinyUSB code size against ${base}: python3 tools/metrics_compare_base.py -b ${args.boards[0]} -e device/cdc_msc . ` + |
There was a problem hiding this comment.
When the validate workflow reaches the size stage, this prompt tells the worker to run metrics_compare_base.py with a trailing .. That script's argparse definition has no positional arguments, so the command exits with error: unrecognized arguments: .; because the workflow says the stage fails when the tool errors, the default /pre-pr software gate will fail even when tests and builds are clean.
Useful? React with 👍 / 👎.
| - `.claude/skills/usb-recover/SKILL.md` — only when a device/fixture is wedged or processes hang in D state. | ||
| - `.claude/skills/usb-debug/SKILL.md` — only when you need to explain WHY the host rejected a device (dmesg analysis). |
There was a problem hiding this comment.
In wedged or USB-debug HIL scenarios, this agent is instructed to read .claude/skills/usb-recover/SKILL.md and .claude/skills/usb-debug/SKILL.md, but I searched the repo (rg "usb-recover|usb-debug" . and the .claude/skills tree) and those skill files are not present. As soon as hil-validate hits the documented unresponsive-board path, the operator has no recovery/debug procedure to follow, so either the files need to be added or these references should point at the existing skill(s).
Useful? React with 👍 / 👎.
| // Post drafted replies as soon as triage produces them — decoupled from | ||
| // fixing/pushing so done/unactionable cycles still post. Outward-facing, | ||
| // so gated on autoPush like every other side effect. | ||
| if (t.replies.length > 0 && args.autoPush !== false) { |
There was a problem hiding this comment.
When pr-babysit is invoked with the documented minimal args ({pr: N}), autoPush is undefined and this condition treats it as enabled, so the workflow can post public replies immediately and later commit/push fixes unless the caller remembered to pass autoPush: false. Since the workflow description says pushes are authorized only with autoPush enabled, this should require autoPush === true for outward side effects rather than making write access the default.
Useful? React with 👍 / 👎.
| fd = os.open(os.path.join(BOARD_LOCK_DIR, f'{board_name}.lock'), | ||
| os.O_RDWR | os.O_CREAT, 0o666) | ||
| fh = os.fdopen(fd, 'r+') | ||
| except OSError: | ||
| return None # odd lock dir (perms, path collision): proceed unlocked |
There was a problem hiding this comment.
On rigs where CI and dev sessions run as different Unix users, the first created lock file is still filtered by the process umask (commonly becoming 0644 despite the 0o666 mode), so a later user cannot reopen it with O_RDWR; this except OSError path then returns None and runs HIL completely unlocked. That makes the per-board arbitration fail exactly in cross-session use unless the lock file is chmodded after creation or opened in a way that does not fail open on ordinary permission mismatches.
Useful? React with 👍 / 👎.
| ? 'THE USER HAS EXPLICITLY AUTHORIZED FORCING: run hil_test.py with HIL_NO_BOARD_LOCK=1 in the environment (bypasses the board lock check; do NOT release or kill the existing holder). ' | ||
| : 'If the run fails because the board lock is held (a dev session or concurrent CI job), report pass=false and set detail to start EXACTLY with "board locked:" followed by the holder JSON verbatim — never force the lock. ') + | ||
| 'Reserve the phrase "board locked" strictly for lock contention; describe a frozen or non-enumerating board as "unresponsive" instead. ' + | ||
| `Firmware is in examples/cmake-build-${b}. Use the config for this host (hostname first), single-board flag -b ${b}, Bash timeout >= 20 min, never cancel early. ` + |
There was a problem hiding this comment.
When /pre-pr chooses boards from test/hil/tinyusb.json while running on htpc, this prompt only says to use the host config; the HIL skill defaults htpc to local test/hil/local.json and uses remote ci.lan only when explicitly requested. In that context a rig-only -b board can be absent from the local config, causing hil_test.py to select no boards and return a passing run without exercising the requested hardware, so the workflow needs to explicitly request the remote HIL path for htpc rig-roster boards.
Useful? React with 👍 / 👎.
| 'detail = GA:1/GA:2 counts plus any diagnostics in changed files.', | ||
| { label: 'pvs', phase: 'Validate', model: 'sonnet', effort: 'low', schema: STAGE }, |
There was a problem hiding this comment.
For the default /pre-pr gate, this instruction tells the PVS worker to set pass=false only for GA:1 diagnostics in changed files, so a missing pvs-studio-analyzer, failed CMake build, or analyzer error can be reported as passing because it produces no GA:1 diagnostics. Unlike the size stage above, tool failures need to fail the stage; otherwise the software gate can go green without actually running PVS.
Useful? React with 👍 / 👎.
Size Difference ReportBecause TinyUSB code size varies by port and configuration, the metrics below represent the averaged totals across all example builds. Note: If there is no change, only one value is shown. Changes >1% in sizeNo entries. Changes <1% in sizeNo entries. No changes
|
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| // Post drafted replies as soon as triage produces them — decoupled from | ||
| // fixing/pushing so done/unactionable cycles still post. Outward-facing, | ||
| // so gated on autoPush like every other side effect. | ||
| if (t.replies.length > 0 && args.autoPush !== false) { |
There was a problem hiding this comment.
pr-babysit performs irreversible, outward-facing actions (push + public PR comments) by default when autoPush is omitted — the opposite of the documented opt-in.
meta.whenToUse says "Invoking with autoPush enabled authorizes pushes to that branch", and the comment right above this line says outward actions are "gated on autoPush like every other side effect" — both read as opt-in (act only when autoPush is enabled).
But autoPush is never defaulted (only maxCycles is, at L12-L14), so an omitted flag stays undefined, and every gate treats undefined as enabled:
- Post public replies —
args.autoPush !== falseistruewhenundefined, so this agent posts to the PR (L98). - Commit + push — the dry-run early-return only fires on an explicit
=== false(L158-L162); otherwise execution falls through to "commit ALL working-tree changes … then push to the PR's remote branch" (L172-L178).
Net effect: pr-babysit({ pr }) with no autoPush — which the docs say should not authorize pushes — commits, pushes to the remote branch, and posts public review replies. That is a fail-unsafe default for hard-to-reverse, outward-facing actions.
To match the documented opt-in contract, gate on autoPush === true instead of !== false (push/post only when explicitly enabled): use args.autoPush === true at L98, and invert the L159 guard so it dry-runs whenever autoPush !== true. Since the fix spans both gates I've described it rather than attaching a committable suggestion.
hathach
force-pushed
the
claude/agents-workflows
branch
from
186a687 to
2de933e
Compare
Add worker agents (builder, port-dev, driver-reviewer, hil-operator, pr-monitor), deterministic workflows (validate, fanout-dev, driver-review, hil-validate, full-check, pr-babysit) and a /pre-pr gate skill, so sessions can fan build/test/review/PR-triage work out to tiered subagents. pr-babysit drives a PR to green: triage CI + bot reviews, fix validated findings, verify, push, and reply-to + resolve each inline review thread (fixed or refuted). Replace the stop-the-runner HIL discipline with per-board flock locks: test/hil/board_lock.py plus a fail-open guard in hil_test.py let CI and dev sessions share the rig per board (locked boards fail fast and re-run; HIL_NO_BOARD_LOCK=1 is a user-authorized bypass). The actions-runner is never stopped. Design spec, implementation plan, and real-rig smoke evidence under docs/superpowers/. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Rn1AN5DsTdFhRwhugfgKZi
hathach
force-pushed
the
claude/agents-workflows
branch
from
2de933e to
e3dd924
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e3dd9245ef
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| if (!skip.includes('size')) thunks.push(() => | ||
| agent( | ||
| `Compare TinyUSB code size against ${base}: python3 tools/metrics_compare_base.py -b ${args.boards[0]} -e device/cdc_msc . ` + |
There was a problem hiding this comment.
When /pre-pr or full-check is invoked with a user-named base other than master, this stage still tells the worker to run metrics_compare_base.py without the script's --base-branch option (confirmed via python3 tools/metrics_compare_base.py --help: --base-branch BASE_BRANCH), so the size verdict is produced against the tool default master even though the prompt says ${base}. That can hide or invent size regressions for release/backport PRs; pass ${base} through to the command.
Useful? React with 👍 / 👎.
| '`gh api graphql -f query=\'query($o:String!,$r:String!,$p:Int!){repository(owner:$o,name:$r){pullRequest(number:$p){reviewThreads(first:100){nodes{id isResolved comments(first:50){nodes{databaseId}}}}}}}\' -F o=OWNER -F r=REPO -F p=' + args.pr + '` ' + | ||
| '(paginate with the endCursor if there are more than 100 threads), pick the thread whose comments contain that databaseId, then resolve it with ' + |
There was a problem hiding this comment.
For PRs with more than 100 review threads, this recipe cannot actually follow the stated pagination path: the query caps reviewThreads(first:100) but does not request pageInfo { hasNextPage endCursor } or accept an after cursor. GitHub's GraphQL pagination docs describe endCursor and hasNextPage as coming from pageInfo (https://docs.github.com/en/graphql/guides/using-pagination-in-the-graphql-api), so comments beyond the first page will not be found or resolved.
Useful? React with 👍 / 👎.
|
|
||
| ## Finish checklist (in order) | ||
|
|
||
| 1. Format only the lines you changed: `git clang-format` (no args — formats working-tree changes vs HEAD using the repo `.clang-format`). If it reformats anything, re-check your diff still builds. |
There was a problem hiding this comment.
In the default fanout-dev path, multiple port-dev workers edit the same checkout in parallel, but git clang-format with no paths operates on all working-tree lines that differ from HEAD (confirmed via git clang-format -h). When two scopes are active, one worker can format or race over another worker's in-progress changes despite the touch nothing outside it contract, producing cross-scope edits or nondeterministic diffs; pass the assigned scope to git clang-format or force worktree isolation.
Useful? React with 👍 / 👎.
| const push = await agent( | ||
| `On the current PR branch: commit ALL working-tree changes as ONE commit (imperative message summarizing the cycle-${cycle} fixes for PR #${args.pr}, repo commit conventions), ` + | ||
| "then push to the PR's remote branch. pass=true only if commit AND push succeeded; detail = pushed SHA.", |
There was a problem hiding this comment.
When pr-babysit runs from a PR checkout that already has uncommitted local edits (for example after a previous dry-run or failed-verification cycle left changes behind), this push step tells the agent to commit ALL working-tree changes. The workflow never checks for a clean tree before triage, so autoPush can publish unrelated files together with the current verified fixes; abort on a dirty pre-state or commit only the files tied to verified fixes.
Useful? React with 👍 / 👎.
|
|
||
| - `BASE` = `master` unless the user names another base. | ||
| - `git diff --name-only $(git merge-base HEAD $BASE)..HEAD` | ||
| - If NO C sources changed (only docs / `.claude/` / tools): say so, and run a minimal software-only gate — `boards = [stm32f407disco]`, no HIL — unless the user asks for more. |
There was a problem hiding this comment.
When a branch changes only non-C HIL code such as test/hil/hil_test.py or test/hil/board_lock.py, this rule matches because no C sources changed and it explicitly selects a minimal software-only gate with no HIL. That skips the hardware run that would exercise the changed harness/locking behavior, and the later catch-all only covers C/CMake source changes, so these Python HIL changes never reach hilBoards.
Useful? React with 👍 / 👎.
| 2. Verify with a targeted build of `device/cdc_msc` for the board named in your task (or pick one from `hw/bsp/<family>/boards/` whose family uses your scope). Use a unique build dir to survive parallel siblings: | ||
| ```bash | ||
| BUILD=$(mktemp -d /tmp/portdev-<BOARD>-XXXX) | ||
| cmake -S examples/device/cdc_msc -B "$BUILD" -DBOARD=<BOARD> -G Ninja -DCMAKE_BUILD_TYPE=MinSizeRel && cmake --build "$BUILD" |
There was a problem hiding this comment.
When the assigned scope is an HCD or other host-only path, this checklist still verifies only device/cdc_msc, which can leave the changed host source uncompiled (for example RP2040 CMake puts hcd_rp2040.c in the host library while device examples link the device side). A broken host-controller edit can therefore report buildOk: true; choose a host example for host scopes or require both device and host builds when the scope is ambiguous.
Useful? React with 👍 / 👎.
|
|
||
| ## Bot review harvest | ||
|
|
||
| - 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 pr view <N> --comments`. |
There was a problem hiding this comment.
In multi-cycle pr-babysit runs, resolved inline threads are still returned by the REST PR review-comments listing used here; that endpoint lists comments rather than unresolved review threads, and the resolution state lives on GraphQL reviewThreads.isResolved. After a prior cycle replies to and resolves a bot finding, the next triage can treat the same resolved comment as unresolved again, causing repeated fix/reply work unless the harvest starts from unresolved GraphQL threads or filters resolved ones.
Useful? React with 👍 / 👎.
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. |
Hardware-in-the-loop (HIL) Test Reporthfp-iar✅ 52 passed · ❌ 0 failed · ⚪ 0 skipped · blank not run
hfp.json✅ 52 passed · ❌ 0 failed · ⚪ 0 skipped · blank not run
tinyusb.json✅ 315 passed · ❌ 24 failed · ⚪ 11 skipped · blank not run
|
Review-fix batch (owner-confirmed) on the multi-agent harness: - board_lock: detach holder stdio so a captured `hold` cannot hang on the daemon's inherited pipe; probe locks by holder-pid liveness instead of a momentary flock, which could spuriously fail a concurrent acquirer (storm-tested: 1 winner in 10, 0/15 acquire failures under probe storm) - hil_test: locked board renders a visible board-locked fail row so the report matches the exit code; stale marker cleared on a real re-run - pr-babysit: autoPush now opt-in (default dry run); resolve recipe paginates reviewThreads; post-push resolve gets issue-comment fallback - validate: size stage honors non-default base via --base-branch; pvs stage delegated to the new agent - new static-analyzer agent (sonnet): PVS-Studio SAST+MISRA for one board, structured findings gated on files changed vs base Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Rn1AN5DsTdFhRwhugfgKZi
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0557655afb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if not isinstance(pid, int) or pid <= 0: | ||
| return False | ||
| try: | ||
| os.kill(pid, 0) |
There was a problem hiding this comment.
When board_lock.py release SIGTERMs the detached holder, that process can remain as a zombie if PID 1 does not reap it promptly (common in containers). os.kill(pid, 0) still succeeds for zombies, so is_locked() keeps reporting the stale lock file as held even though the kernel flock was released; subsequent release reports ERROR: still locked and future hold calls refuse the board. Check the process state or the flock itself instead of treating any live PID as an active holder.
Useful? React with 👍 / 👎.
| pids.add(info['pid']) | ||
| for holder in sorted(pids): | ||
| try: | ||
| os.kill(holder, signal.SIGTERM) |
There was a problem hiding this comment.
Because lock files are intentionally left behind after a holder exits, a later release can read an old JSON pid after the OS has reused it for an unrelated process. In that case is_locked() treats the board as held and this line sends SIGTERM to whatever now owns that pid, even though the flock was already released; this can terminate an unrelated job on the rig. Revalidate the flock/current holder before killing, or remove/ignore stale records rather than trusting the stored pid alone.
Useful? React with 👍 / 👎.
Summary
.claude/agents/: builder, port-dev, driver-reviewer, hil-operator, pr-monitor), six deterministic workflow scripts (.claude/workflows/: validate, fanout-dev, driver-review, hil-validate, full-check, pr-babysit) plus a syntax checker, and a/pre-prskill that derives affected boards from the branch diff and runs the composed software+hardware gate.test/hil/board_lock.py(background holder process, kernel flocks — auto-release on holder death, holder-signaled acquire, storm-tested) and a fail-open guard intest/hil/hil_test.pythat self-locks each board for its flash+test. A locked board fails fast withboard locked: <holder JSON>and is re-runnable;HIL_NO_BOARD_LOCK=1is a user-authorized bypass. The GitHub Actions runner keeps running; CI and dev sessions share the rig per board. CI starts enforcing the guard as soon as this merges (PR CI runshil_test.pyfrom the merge ref).docs/superpowers/.Test evidence
stm32f407disco+raspberry_pi_pico(43 examples each); code size +0.0% vs master; PVS gate green vsorigin/master.py_compileclean.activethroughout,svc.shnever touched): lock-conflict fail-fast withlocked[]surfaced → forced run with the holder surviving → normal self-locking run; board flashed/booted 18×/run. The 5 host-mode test failures observed are a rig fixture issue (pico PIO-USB host-port peers not enumerating after the 2026-07-09 rig rework) — unrelated to this PR; all 13 device-mode tests pass.git clang-formatclean, independently build-verified, reverted.🤖 Generated with Claude Code
https://claude.ai/code/session_01Rn1AN5DsTdFhRwhugfgKZi