diff --git a/.claude/skills/bughunt-run/SKILL.md b/.claude/skills/bughunt-run/SKILL.md index 82ef3958..5f1e8c2c 100644 --- a/.claude/skills/bughunt-run/SKILL.md +++ b/.claude/skills/bughunt-run/SKILL.md @@ -51,6 +51,16 @@ revertProof}` once something is actually fixed. Then: node .superpowers/render-ledger.mjs ``` +Each confirmed record carries `finder` — which model in the dual-model panel +produced it. The run also logs one `panel:` line with the split. The two finders +are unioned, not voted, so the second model's entire value is what it finds +alone; because duplicates collapse to the opus-slot record, a `sonnet` tag means +opus missed it. Watch that count across a few hunts. Consistently zero is the +evidence for dropping to a single finder — but note that would also weaken +convergence, since a round is only allowed to count as dry when the full panel +reported, so a lone finder having a bad day would read as "clean" instead of +"we didn't fully look". + ## 2. Gate (human) Read `.superpowers/FINDINGS.md`. Mark anything you do not want fixed as @@ -67,6 +77,7 @@ Workflow({ branch: "fix/bughunt-YYYY-MM-DD", only: ["OC-0042"], // optional maxSeverity: "medium", // optional + circuitBreaker: { threshold: 0.5, minAttempts: 3 }, // optional; false to disable }, }) ``` @@ -74,6 +85,25 @@ Workflow({ Create and check out the branch first — the workflow commits to whatever branch is current and does not create one. +## When a run trips the breaker + +The run stops early if more than `threshold` of attempted findings fail, once at +least `minAttempts` have been tried. `declined` never counts as a failure — a run +where several findings are correctly declined is a good run. There are two trip +points: the fix stage (before any prove agent runs) and inside the prove loop. + +**A tripped run means stop and investigate, do not re-run.** The usual causes are +being on the wrong branch, a broken test runner, or ledger coordinates gone stale +after a rebase. Re-running without fixing the cause just spends the budget again. + +Findings from clusters the run never reached come back `blocked` with a rationale +naming the breaker. Set those back to `open` once the underlying problem is fixed +— they were never attempted. Their edits are sitting uncommitted in the working +tree, so the debris warning above applies to them too. + +Whatever committed before the trip still goes through the gate, so `result.gate` +tells you whether those commits are green. + When it returns, for each entry in `result.results`: - `fixed` → status `fixed`, `fix: {commit, test, revertProof: "self-reported"}` diff --git a/.claude/workflows/bughunt-fix.harness.mjs b/.claude/workflows/bughunt-fix.harness.mjs index 40946dc8..e299a474 100644 --- a/.claude/workflows/bughunt-fix.harness.mjs +++ b/.claude/workflows/bughunt-fix.harness.mjs @@ -646,6 +646,161 @@ scenarios.f15_prove_prompt_names_every_touched_path = async () => { assert.match(provePromptText, /fix\/test-touched/, 'branch guard must name the expected branch') } +// ---------- circuit breaker ---------- +// Four findings, one per file, so each becomes its own cluster. +const FOUR_FILES = [ + rec('OC-0001'), + rec('OC-0002', { file: 'Server/ws/hub_sweep.go' }), + rec('OC-0003', { file: 'Client/tauri-client/src/lib/livekitSession.ts' }), + rec('OC-0004', { file: 'Client/tauri-client/src/components/VoiceWidget.ts' }), +] +// A fix stub that reports every id in its prompt as fixed. Ids go through a Set because rec() +// puts each id in both `id` and `title`, so the raw matchAll yields every id twice. +const fixAll = (prompt) => { + const ids = [...new Set([...prompt.matchAll(/OC-\d{4}/g)].map((m) => m[0]))] + return { results: ids.map((id) => ({ id, outcome: 'fixed', testPath: `t/${id}.ts`, rationale: '' })), touchedPaths: [] } +} +const PROVE_FAIL = { + committed: false, + sha: '', + redObserved: false, + greenObserved: true, + redOutput: '$ npx vitest run t.ts\nPASS t.ts (still passing with the fix reverted)', + greenOutput: '$ npx vitest run t.ts\nPASS t.ts', + note: 'test did not exercise the bug', +} +const proveOk = (sha) => ({ + committed: true, + sha, + redObserved: true, + greenObserved: true, + redOutput: '$ npx vitest run t.ts\nFAIL t.ts (reverted)', + greenOutput: '$ npx vitest run t.ts\nPASS t.ts (fixed)', + note: '', +}) + +// F16: three failed revert-proofs out of three attempts trips the breaker, and the fourth cluster +// is never handed to an agent. Load-bearing: this is the one that proves the loop actually stops. +scenarios.f16_breaker_trips_after_majority_prove_failures = async () => { + const { result, calls, logs } = await run({ + args: { findings: FOUR_FILES }, + agentStub: (prompt, opts) => { + if (String(opts.label).startsWith('fix:')) return fixAll(prompt) + if (String(opts.label).startsWith('prove:')) return PROVE_FAIL + return { passed: true, stacks: [], output: '' } + }, + }) + const proveCalls = calls.filter((c) => String(c.opts.label).startsWith('prove:')) + assert.equal(proveCalls.length, 3, 'breaker must stop the loop after minAttempts, not run all four') + assert.ok(result.breaker, 'breaker report must be present on the result') + assert.equal(result.breaker.trippedAt, 'prove') + assert.equal(result.breaker.attempted, 3) + assert.equal(result.breaker.failed, 3) + assert.equal(result.commits.length, 0) + // Every finding ends blocked, but the unreached one must say WHY it was never tried. + assert.ok(result.results.every((r) => r.outcome === 'blocked'), 'nothing may be left reported as fixed') + const unreached = result.results.filter((r) => /circuit breaker/i.test(r.rationale)) + assert.equal(unreached.length, 1, 'exactly the one unreached finding carries the breaker rationale') + assert.match(unreached[0].rationale, /uncommitted/i, 'operator must be told the edits are still in the tree') + assert.ok(logs.some((l) => /CIRCUIT BREAKER/.test(l)), 'a trip must be announced in the log') +} + +// F17: two failures out of two is 100%, but below minAttempts it is noise, not a signal. +scenarios.f17_breaker_holds_below_min_attempts = async () => { + const { result, calls } = await run({ + args: { findings: FOUR_FILES.slice(0, 2) }, + agentStub: (prompt, opts) => { + if (String(opts.label).startsWith('fix:')) return fixAll(prompt) + if (String(opts.label).startsWith('prove:')) return PROVE_FAIL + return { passed: true, stacks: [], output: '' } + }, + }) + assert.equal(calls.filter((c) => String(c.opts.label).startsWith('prove:')).length, 2, 'both clusters must be attempted') + assert.equal(result.breaker, null) + for (const r of result.results) { + assert.equal(r.outcome, 'blocked') + assert.match(r.rationale, /revert-proof failed/, 'rationale must be the real reason, not the breaker') + } +} + +// F18: one failure in four is a bad cluster, not a bad run. +scenarios.f18_breaker_holds_under_threshold = async () => { + let n = 0 + const { result } = await run({ + args: { findings: FOUR_FILES }, + agentStub: (prompt, opts) => { + if (String(opts.label).startsWith('fix:')) return fixAll(prompt) + if (String(opts.label).startsWith('prove:')) return ++n === 1 ? PROVE_FAIL : proveOk(`sha${n}`) + return { passed: true, stacks: [], output: '' } + }, + }) + assert.equal(result.breaker, null) + assert.equal(result.commits.length, 3, 'the three good clusters must still land') +} + +// F19: the operator can turn the guard off entirely. +scenarios.f19_breaker_disabled_by_args = async () => { + const { result, calls } = await run({ + args: { findings: FOUR_FILES, circuitBreaker: false }, + agentStub: (prompt, opts) => { + if (String(opts.label).startsWith('fix:')) return fixAll(prompt) + if (String(opts.label).startsWith('prove:')) return PROVE_FAIL + return { passed: true, stacks: [], output: '' } + }, + }) + assert.equal(calls.filter((c) => String(c.opts.label).startsWith('prove:')).length, 4, 'all four must be attempted when disabled') + assert.equal(result.breaker, null) +} + +// F20: when the FIX stage is what is failing, proving each of those costs a serial agent per +// cluster and cannot succeed. Load-bearing: this is the cheap early exit. +scenarios.f20_fix_stage_trip_skips_prove_entirely = async () => { + const { result, calls } = await run({ + args: { findings: FOUR_FILES }, + agentStub: (prompt, opts) => { + if (String(opts.label).startsWith('fix:')) { + const ids = [...new Set([...prompt.matchAll(/OC-\d{4}/g)].map((m) => m[0]))] + return { + results: ids.map((id) => + id === 'OC-0001' + ? { id, outcome: 'fixed', testPath: `t/${id}.ts`, rationale: '' } + : { id, outcome: 'blocked', testPath: '', rationale: 'could not run the test suite' }, + ), + touchedPaths: [], + } + } + return { passed: true, stacks: [], output: '' } + }, + }) + assert.ok(result.breaker, 'breaker report must be present') + assert.equal(result.breaker.trippedAt, 'fix') + assert.equal( + calls.filter((c) => String(c.opts.label).startsWith('prove:')).length, + 0, + 'a fix-stage trip must spend nothing on prove agents', + ) + assert.equal(result.commits.length, 0) + assert.equal(result.gate, null, 'no commits means no gate') +} + +// F21: a trip does not orphan whatever already landed - the operator needs to know if it is green. +scenarios.f21_trip_still_gates_existing_commits = async () => { + let n = 0 + const { result, calls } = await run({ + args: { findings: FOUR_FILES }, + agentStub: (prompt, opts) => { + if (String(opts.label).startsWith('fix:')) return fixAll(prompt) + if (String(opts.label).startsWith('prove:')) return ++n === 1 ? proveOk('aaa1111') : PROVE_FAIL + return { passed: true, stacks: ['client'], output: 'all green' } + }, + }) + assert.ok(result.breaker, 'breaker report must be present') + assert.equal(result.breaker.trippedAt, 'prove') + assert.equal(result.commits.length, 1, 'the one proven cluster must survive the trip') + assert.equal(calls.filter((c) => c.opts.label === 'gate').length, 1, 'the gate must still run over what landed') + assert.equal(result.gate.passed, true) +} + // ---------- runner ---------- const only = process.argv[2] for (const [name, fn] of Object.entries(scenarios)) { diff --git a/.claude/workflows/bughunt-fix.js b/.claude/workflows/bughunt-fix.js index 9fadd141..5af0606f 100644 --- a/.claude/workflows/bughunt-fix.js +++ b/.claude/workflows/bughunt-fix.js @@ -27,6 +27,18 @@ const BRANCH = ARGS.branch || 'fix/bughunt' const ONLY = Array.isArray(ARGS.only) && ARGS.only.length ? new Set(ARGS.only) : null const MAX_SEVERITY = ARGS.maxSeverity || 'low' const ALL = Array.isArray(ARGS.findings) ? ARGS.findings : [] +// Circuit breaker: stop a run that is going systematically wrong instead of spending a +// high-effort agent on every remaining cluster. `declined` is not a failure - it is a +// judgement the fix prompt explicitly invites - so only `blocked` counts. +// ?? not || so an explicit threshold of 0 is honoured. +const BREAKER = + ARGS.circuitBreaker === false + ? null + : { + threshold: ARGS.circuitBreaker?.threshold ?? 0.5, + minAttempts: ARGS.circuitBreaker?.minAttempts ?? 3, + } +let breaker = null // set to a report object if it trips // ---------- phase 1: plan ---------- phase('Plan') @@ -61,7 +73,10 @@ const clusters = [...byFile.entries()].map(([file, findings]) => ({ findings, })) -log(`plan: ${selected.length} finding(s) in ${clusters.length} file cluster(s) on ${BRANCH}`) +log(`plan: ${selected.length} finding(s) in ${clusters.length} file cluster(s) on ${BRANCH}` + + (BREAKER + ? ` (breaker: stop above ${Math.round(BREAKER.threshold * 100)}% failures after ${BREAKER.minAttempts} attempts)` + : ' (breaker disabled)')) for (const c of clusters) log(` ${c.file}: ${c.ids.join(', ')}`) // Announce every exclusion by id. Silent truncation reads as "covered everything" when it did not. for (const e of excluded) log(` excluded ${e.id}: ${e.reason}`) @@ -219,6 +234,25 @@ log(`fix: ${allResults.filter((r) => r.outcome === 'fixed').length} fixed, ` + `${allResults.filter((r) => r.outcome === 'declined').length} declined, ` + `${allResults.filter((r) => r.outcome === 'blocked').length} blocked`) +// ---------- phase 2.6: circuit breaker (fix stage) ---------- +// A high blocked rate here means the fixing itself is failing - bad ledger coordinates, a +// broken test runner, agents that cannot run the suite. Proving each of those costs a +// serial agent per cluster and cannot succeed, so stop before spending it. +if (BREAKER) { + const attempted = allResults.filter((r) => r.outcome !== 'declined').length + const failed = allResults.filter((r) => r.outcome === 'blocked').length + if (attempted >= BREAKER.minAttempts && failed / attempted > BREAKER.threshold) { + breaker = { + trippedAt: 'fix', + attempted, + failed, + threshold: BREAKER.threshold, + reason: `${failed}/${attempted} finding(s) could not be fixed - skipping prove and commit entirely`, + } + log(`CIRCUIT BREAKER: ${breaker.reason}`) + } +} + // ---------- phase 3: prove + commit ---------- phase('Prove') @@ -293,8 +327,22 @@ function provePrompt(cluster, fixedIds, testPaths, sourcePaths) { } const commits = [] +let proveAttempts = 0 +let proveFailures = 0 // Serial on purpose: parallel git commands collide on .git/index.lock. for (const { cluster, results, union } of fixed) { + if (breaker) { + // Tripped either before the loop (fix stage) or on an earlier iteration. Everything + // from here on was never attempted; say so rather than leaving it reported as fixed, + // which would put a `fixed` status in the ledger with no commit behind it. + for (const r of results) { + if (r.outcome === 'fixed') { + r.outcome = 'blocked' + r.rationale = `circuit breaker tripped before this cluster was attempted (${breaker.reason}); edits are uncommitted in the working tree` + } + } + continue + } const fixedHere = results.filter((r) => r.outcome === 'fixed') if (!fixedHere.length) { log(`prove ${cluster.file}: no fixes to prove - skipped`) @@ -313,6 +361,10 @@ for (const { cluster, results, union } of fixed) { schema: PROVE_RESULT, }).catch(() => null) + // Counted before the ok check on purpose: successes belong in the denominator. Increment + // this inside the failure branch instead and the ratio is failures-over-failures, which is + // always 1.0 - the breaker would trip on the first failed cluster at any threshold. + proveAttempts++ const ok = p && p.committed && p.redObserved && p.greenObserved && p.sha if (!ok) { const why = !p @@ -329,6 +381,17 @@ for (const { cluster, results, union } of fixed) { r.rationale = why } } + proveFailures++ + if (BREAKER && proveAttempts >= BREAKER.minAttempts && proveFailures / proveAttempts > BREAKER.threshold) { + breaker = { + trippedAt: 'prove', + attempted: proveAttempts, + failed: proveFailures, + threshold: BREAKER.threshold, + reason: `${proveFailures}/${proveAttempts} cluster(s) failed their revert-proof - stopping before the rest`, + } + log(`CIRCUIT BREAKER: ${breaker.reason}`) + } continue } commits.push({ sha: p.sha, file: cluster.file, ids }) @@ -407,4 +470,4 @@ if (commits.length) { log('gate: nothing committed - skipped') } -return { branch: BRANCH, clusters: publicClusters, excluded, commits, results: allResults, gate } +return { branch: BRANCH, clusters: publicClusters, excluded, commits, results: allResults, gate, breaker } diff --git a/.claude/workflows/bughunt.harness.mjs b/.claude/workflows/bughunt.harness.mjs index 244d4cfe..3e90a89f 100644 --- a/.claude/workflows/bughunt.harness.mjs +++ b/.claude/workflows/bughunt.harness.mjs @@ -551,6 +551,96 @@ scenarios.s_confirmed_carries_finder_detail = async () => { assert.equal(r.round, 1) } +// S_FINDER_ATTRIBUTION: confirmed records name which model found them. The panel unions rather +// than votes, so this is the only way to tell whether the second finder earns its cost. Because +// dedupe keeps the first occurrence and opus is index 0, `finder: 'sonnet'` means opus did NOT +// report it - i.e. a sonnet-unique find. `finder: 'opus'` says nothing about sonnet either way. +scenarios.s_finder_attribution = async () => { + const shared = finding(1) + // Different file, not just a different line: isDup treats same-file findings within + // TITLE_MATCH_WINDOW as duplicates on title-word overlap, and the finding() fixtures share + // enough words to collapse into one another. + const sonnetOnly = finding(2, { file: 'Server/api/user.go' }) + const { result } = await run({ + args: { maxRounds: 1, dryThreshold: 9 }, + agentStub: makeStub({ + hunt: (round, key, model) => { + if (round !== 1 || key !== 'ws-hub') return none + return model === 'opus' ? { findings: [shared] } : { findings: [shared, sonnetOnly] } + }, + verify: (round, key, cands) => confirmAll(cands), + }), + }) + const byTitle = Object.fromEntries(result.confirmed.map((r) => [r.title, r.finder])) + assert.equal(byTitle[shared.title], 'opus', 'a find both models made keeps the opus-first record') + assert.equal(byTitle[sonnetOnly.title], 'sonnet', 'a find only sonnet made must be attributed to sonnet') +} + +// S_FINDER_ATTRIBUTION_SURVIVES_DEAD_OPUS: the tag must be taken from the panel slot, not from the +// position in the surviving list. Filtering the nulls out BEFORE reading the index shifts sonnet +// into slot 0 and mislabels every one of its finds as opus - exactly when attribution matters most. +scenarios.s_finder_attribution_survives_dead_opus = async () => { + const only = finding(3) + const { result } = await run({ + args: { maxRounds: 1, dryThreshold: 9 }, + agentStub: makeStub({ + hunt: (round, key, model) => { + if (round !== 1 || key !== 'ws-hub') return none + return model === 'opus' ? null : { findings: [only] } + }, + verify: (round, key, cands) => confirmAll(cands), + }), + }) + assert.equal(result.confirmed.length, 1) + assert.equal(result.confirmed[0].finder, 'sonnet', 'a dead opus must not relabel sonnet finds as opus') +} + +// S_VERIFIER_IS_NOT_TOLD_THE_FINDER: the verifier prompt deliberately says "another model" and +// never names it. Leaking the attribution tag would tell a refute-by-default verifier that opus +// found something, which is exactly the kind of authority cue that erodes refute-by-default. +scenarios.s_verifier_is_not_told_the_finder = async () => { + let verifyPromptText = '' + await run({ + args: { maxRounds: 1, dryThreshold: 9 }, + agentStub: makeStub({ + hunt: (round, key, model) => + round === 1 && key === 'ws-hub' && model === 'opus' ? { findings: [finding(1)] } : none, + verify: (round, key, cands, retry, prompt) => { + verifyPromptText = prompt + return confirmAll(cands) + }, + }), + }) + assert.ok(verifyPromptText, 'the verifier must have been called') + assert.doesNotMatch(verifyPromptText, /finder/i, 'the verifier must not be told which model found the candidate') + // Guard against over-stripping: the fields the verifier actually needs must survive. + for (const field of ['title', 'file', 'line', 'why', 'repro', 'evidence']) { + assert.match(verifyPromptText, new RegExp(`"${field}"`), `candidates must still carry ${field}`) + } +} + +// S_PANEL_SPLIT_IS_REPORTED: a tag nobody reads is not a measurement. The run must say out loud +// how many confirmed findings only the second finder produced, which is the number that decides +// whether the second model is worth its cost. +scenarios.s_panel_split_is_reported = async () => { + const { logs } = await run({ + args: { maxRounds: 1, dryThreshold: 9 }, + agentStub: makeStub({ + hunt: (round, key, model) => { + if (round !== 1 || key !== 'ws-hub') return none + return model === 'opus' + ? { findings: [finding(1)] } + : { findings: [finding(1), finding(2, { file: 'Server/api/user.go' })] } + }, + verify: (round, key, cands) => confirmAll(cands), + }), + }) + const line = logs.find((l) => /panel:/.test(l)) + assert.ok(line, 'the run must report the panel split') + assert.match(line, /sonnet-only/, 'the split must name the sonnet-only count explicitly') + assert.match(line, /\b1\b/, 'exactly one confirmed finding here was sonnet-only') +} + // ---------- runner ---------- const only = process.argv[2] for (const [name, fn] of Object.entries(scenarios)) { diff --git a/.claude/workflows/bughunt.js b/.claude/workflows/bughunt.js index 9d2f6cfb..0b5a03c0 100644 --- a/.claude/workflows/bughunt.js +++ b/.claude/workflows/bughunt.js @@ -449,7 +449,9 @@ function verifyPrompt(lensKey, candidates) { `Re-rate severity yourself; do not inherit the hunter's rating. For each survivor, give the smallest ` + `correct fix - one guard in the shared function beats a guard in every caller.\n\n` + `Return one verdict per candidate, keeping title/file/line so they can be matched up.\n\n` + - `--- CANDIDATES ---\n${JSON.stringify(candidates, null, 2)}` + // Strip the panel attribution here rather than at the call sites: the prompt above says + // "another model" on purpose, and naming it is an authority cue that erodes refute-by-default. + `--- CANDIDATES ---\n${JSON.stringify(candidates.map(({ finder, ...c }) => c), null, 2)}` ) } @@ -481,7 +483,14 @@ while (dry < DRY_THRESHOLD && round < MAX_ROUNDS) { async (r) => { const { lens, pair } = r const finderFailed = pair.some((p) => p === null) - const union = pair.filter(Boolean).flatMap((p) => p.findings || []) + // Tag by panel slot, not by position in the surviving list: filtering the nulls out first + // would shift sonnet into slot 0 whenever opus dies and mislabel its finds. The panel unions + // rather than votes, so this is the only signal for whether the second finder earns its cost + // - and since dedupe keeps the first occurrence and opus is slot 0, `finder: 'sonnet'` means + // opus did not report it. `finder: 'opus'` says nothing about sonnet either way. + const union = pair.flatMap((p, i) => + p ? (p.findings || []).map((f) => ({ ...f, finder: i ? 'sonnet' : 'opus' })) : [], + ) const fresh = dedupe(union, seenAtStart) if (!fresh.length) return { lens, finderFailed, unionCount: union.length, fresh: [], verdicts: [] } log(`r${rnd} ${lens.key}: ${fresh.length} fresh candidate(s) -> verification`) @@ -544,6 +553,13 @@ while (dry < DRY_THRESHOLD && round < MAX_ROUNDS) { const converged = dry >= DRY_THRESHOLD +// The panel unions rather than votes, so the second finder's whole value is what it finds alone. +// dedupe keeps the opus-slot record when both report the same bug, so a confirmed finding tagged +// sonnet is one opus missed. A run where that count is 0 is the evidence for dropping the second +// model; anything above 0 is what it bought. +const sonnetOnly = confirmedAll.filter((f) => f.finder === 'sonnet').length +log(`panel: ${confirmedAll.length} confirmed, ${sonnetOnly} sonnet-only (opus missed), ${confirmedAll.length - sonnetOnly} found by opus`) + // ---------- report ---------- phase('Report') const RANK = { critical: 0, high: 1, medium: 2, low: 3 }