Merge pull request #3863 from hathach/claude/validate-loop
validate workflow: loop validate -> fix cycles until green
Turn the single-pass validate gate into a loop: run unit + builds +
size + PVS + claude/codex reviews in parallel; on a red verdict one
fix agent repairs the gate-failing evidence (CONFIRMED findings,
codex P0/P1, failed stages - PLAUSIBLE/quality stay report-only),
commits, and the affected stages re-run, up to maxCycles (default 5).
Hardened per review: fix-commit paths verified from git rather than
self-report, restartRequired when a fix edits the workflow itself,
per-stage evidence budgeting so the fixer prompt JSON never truncates
mid-document, dirty-tree and moving-base-ref guards, dead stage
agents retried instead of ending the loop, and only pure-docs fixes
skip a full stage re-run.
diff --git a/.claude/workflows/validate.js b/.claude/workflows/validate.js
index dadf8a7..773dd47 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 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' },
+ },
+}
-if (!skip.includes('unit')) thunks.push(() =>
- agent(
+// 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 (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 @@
'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)
-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 }
+ 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 failures = [...latest.values()].filter(r => !r.pass)
+return { pass: false, cycles: history, stages: [...latest.values()], failures }
