diff --git a/src/adapters/adapter.test.ts b/src/adapters/adapter.test.ts index 87ce481..04dbe31 100644 --- a/src/adapters/adapter.test.ts +++ b/src/adapters/adapter.test.ts @@ -1,4 +1,6 @@ import { EventEmitter } from 'node:events'; +import fs from 'node:fs'; +import path from 'node:path'; import { vi } from 'vitest'; import type { AgentAdapter, AgentInvocation, AgentResponse } from './base.js'; import { AdapterRegistry, parseAdapterExtraArgs } from './index.js'; @@ -207,3 +209,256 @@ describe('adapter extraArgs passthrough', () => { expect(argv[1]).toBe('--dangerously-bypass-approvals-and-sandbox'); }); }); + +describe('OpenCodeAdapter NDJSON output collapse', () => { + beforeEach(() => { + spawnMock.mockReset(); + }); + + const fixturePath = path.resolve( + __dirname, + '..', + '..', + 'tests', + 'fixtures', + 'fake-agents', + 'opencode-ndjson-text-parts.json', + ); + + it('collapses the event stream into text-part contents (prose + trailing verdict)', async () => { + const ndjson = fs.readFileSync(fixturePath, 'utf8'); + spawnMock.mockImplementation(() => makeFakeChild(0, ndjson)); + + const adapter = new OpenCodeAdapter(); + const response = await adapter.invoke({ prompt: 'review this' }); + + expect(response.success).toBe(true); + expect(response.output).toContain('All 12 tests pass.'); + expect(response.output).toContain('"passed": true'); + // Raw NDJSON event framing must not leak through. + expect(response.output).not.toContain('"type":"step_start"'); + expect(response.output).not.toContain('"type":"tool_use"'); + }); + + it('joins multiple text events in stream order (real streams emit many)', async () => { + const ndjson = fs.readFileSync(fixturePath, 'utf8'); + spawnMock.mockImplementation(() => makeFakeChild(0, ndjson)); + + const adapter = new OpenCodeAdapter(); + const response = await adapter.invoke({ prompt: 'review this' }); + + const zeroFindings = response.output.indexOf('zero-findings recheck'); + const verdict = response.output.indexOf('All 12 tests pass'); + expect(zeroFindings).toBeGreaterThanOrEqual(0); + expect(verdict).toBeGreaterThan(zeroFindings); + }); + + it('end-to-end: collapsed output yields a passed rubric through extractRubricJson', async () => { + const ndjson = fs.readFileSync(fixturePath, 'utf8'); + spawnMock.mockImplementation(() => makeFakeChild(0, ndjson)); + + const adapter = new OpenCodeAdapter(); + const response = await adapter.invoke({ prompt: 'review this' }); + + // The real production parser — not a mirror. Mirrors drift. + const { extractRubricJson } = await import('../core/rubric.js'); + const parsed = extractRubricJson(response.output); + expect(parsed).not.toBeNull(); + expect(parsed?.passed).toBe(true); + expect(parsed?.stage).toBe('code_critic'); + }); + + it('preserves the verdict even when the final line is truncated (context-limit tail)', async () => { + const ndjson = fs.readFileSync(fixturePath, 'utf8'); + // Cut mid-event: a context-limit kill can truncate the stream before the + // final step_finish. The text part (with the rubric) must still survive. + const truncated = ndjson.split('\n').slice(0, -1).join('\n') + '\n{"type":"step_finish","timestamp":1785417871'; + spawnMock.mockImplementation(() => makeFakeChild(0, truncated)); + + const adapter = new OpenCodeAdapter(); + const response = await adapter.invoke({ prompt: 'review this' }); + + expect(response.success).toBe(true); + expect(response.output).toContain('"passed": true'); + }); + + it('falls back to raw stdout when no text part parses', async () => { + const garbage = 'opencode: something went wrong\n{"type":"error","message":"boom"'; + spawnMock.mockImplementation(() => makeFakeChild(0, garbage)); + + const adapter = new OpenCodeAdapter(); + const response = await adapter.invoke({ prompt: 'hi' }); + + expect(response.output).toBe(garbage); + }); + + it('skips valid-JSON non-event lines (null, numbers) without crashing', async () => { + const mixed = 'null\n42\n{"type":"step_start"}\n{"type":"text","part":{"text":"verdict here"}}'; + spawnMock.mockImplementation(() => makeFakeChild(0, mixed)); + + const adapter = new OpenCodeAdapter(); + const response = await adapter.invoke({ prompt: 'hi' }); + + expect(response.success).toBe(true); + expect(response.output).toBe('verdict here'); + }); + + it('skips text events whose part.text is not a string (shape-change guard)', async () => { + // The typeof guard is the load-bearing line against a future opencode + // shape change (e.g. part.text becoming an object). A truthy check would + // push a non-string into parts and emit "[object Object]" into the output + // that feeds rubric extraction — this test pins the guard. + const mixed = [ + '{"type":"text","part":{"text":{"nested":true}}}', + '{"type":"text","part":{"text":42}}', + '{"type":"text","part":{"text":"real verdict here"}}', + ].join('\n'); + spawnMock.mockImplementation(() => makeFakeChild(0, mixed)); + + const adapter = new OpenCodeAdapter(); + const response = await adapter.invoke({ prompt: 'hi' }); + + expect(response.success).toBe(true); + expect(response.output).toBe('real verdict here'); + expect(response.output).not.toContain('[object Object]'); + }); + + it('verdict survives a later text event containing real braces (extractRubricJson scan)', async () => { + const stream = [ + '{"type":"step_start","part":{"id":"a","type":"step-start"}}', + '{"type":"text","part":{"text":"{\\"passed\\": true, \\"stage\\": \\"code_critic\\", \\"findings\\": []}"}}', + '{"type":"text","part":{"text":"Note: the fix touched { src/foo.ts } and { src/bar.ts } (see commit 4b8b12f)."}}', + '{"type":"step_finish","part":{"id":"b","reason":"stop","type":"step-finish"}}', + ].join('\n'); + spawnMock.mockImplementation(() => makeFakeChild(0, stream)); + + const adapter = new OpenCodeAdapter(); + const response = await adapter.invoke({ prompt: 'review this' }); + + const { extractRubricJson } = await import('../core/rubric.js'); + const parsed = extractRubricJson(response.output); + expect(parsed).not.toBeNull(); + expect(parsed?.passed).toBe(true); + }); + + it('extractRubricJson ignores a trailing object without a passed key', async () => { + const { extractRubricJson } = await import('../core/rubric.js'); + const output = [ + 'I reviewed the diff. Verdict:', + '{"passed": true, "stage": "code_critic", "findings": []}', + '{"note": "this is a trailing note, not a verdict"}', + ].join('\n'); + const parsed = extractRubricJson(output); + expect(parsed).not.toBeNull(); + expect(parsed?.passed).toBe(true); + expect(parsed?.stage).toBe('code_critic'); + }); + + it('extractRubricJson returns null when no object has a passed key', async () => { + const { extractRubricJson } = await import('../core/rubric.js'); + const output = 'I began my review but ran out of context before finishing.'; + expect(extractRubricJson(output)).toBeNull(); + }); + + it('extractRubricJson never picks a nested passed object over the top-level verdict', async () => { + const { extractRubricJson } = await import('../core/rubric.js'); + + // t1: failing verdict + trailing tool result with nested passed:true — + // must return the FAILING verdict, not false-green the gate. + const t1 = [ + 'V: {"passed": false, "stage": "code_critic", "findings": [{"category":"product_bug","detail":"x"}]}', + 'Tool output: {"results": [{"name": "syntax", "passed": true}]}', + ].join('\n'); + const r1 = extractRubricJson(t1); + expect(r1).not.toBeNull(); + expect(r1?.passed).toBe(false); + expect(r1?.stage).toBe('code_critic'); + + // t2: passing verdict + trailing object with nested passed:false — + // must return the PASSING verdict, not false-strand the parent. + const t2 = [ + 'V: {"passed": true, "stage": "code_critic", "findings": []}', + '{"checks": {"lint": {"passed": false}}}', + ].join('\n'); + const r2 = extractRubricJson(t2); + expect(r2).not.toBeNull(); + expect(r2?.passed).toBe(true); + expect(r2?.stage).toBe('code_critic'); + }); + + it('extractRubricJson handles the custom-critic shape (nested assertions in the top-level verdict)', async () => { + const { extractRubricJson } = await import('../core/rubric.js'); + const output = JSON.stringify({ + passed: true, + stage: 'credit_ledger_integrity', + assertions: [ + { name: 'append_only', passed: true, evidence: 'ledger is append-only' }, + { name: 'audit_fields', passed: true, evidence: 'created_at set' }, + ], + findings: [], + }); + const parsed = extractRubricJson(output); + expect(parsed).not.toBeNull(); + expect(parsed?.passed).toBe(true); + expect(parsed?.stage).toBe('credit_ledger_integrity'); + }); + + it('collapses NDJSON on a non-zero exit (failure path)', async () => { + const ndjson = fs.readFileSync(fixturePath, 'utf8'); + spawnMock.mockImplementation(() => makeFakeChild(1, ndjson)); + + const adapter = new OpenCodeAdapter(); + const response = await adapter.invoke({ prompt: 'review this' }); + + expect(response.success).toBe(false); + // The collapse applies on failure paths too (documented divergence from + // claude/codex, which return raw stdout). + expect(response.output).toContain('"passed": true'); + expect(response.output).not.toContain('"type":"step_start"'); + }); + + it('collapses NDJSON on a spawn error (failure path)', async () => { + const ndjson = fs.readFileSync(fixturePath, 'utf8'); + const child = new EventEmitter() as FakeChild; + child.stdin = { write: () => undefined, end: () => undefined }; + child.stdout = new EventEmitter(); + child.stderr = new EventEmitter(); + child.kill = () => undefined; + setImmediate(() => { + child.stdout.emit('data', Buffer.from(ndjson)); + child.emit('error', new Error('spawn ENOENT')); + }); + spawnMock.mockImplementation(() => child); + + const adapter = new OpenCodeAdapter(); + const response = await adapter.invoke({ prompt: 'review this' }); + + expect(response.success).toBe(false); + expect(response.output).toContain('"passed": true'); + }); + + it('collapses NDJSON on a timeout (failure path)', async () => { + const ndjson = fs.readFileSync(fixturePath, 'utf8'); + const child = new EventEmitter() as FakeChild; + child.stdin = { write: () => undefined, end: () => undefined }; + child.stdout = new EventEmitter(); + child.stderr = new EventEmitter(); + // The adapter's timeout handler kills the child; a real child then emits + // 'close'. Emit it so the promise resolves through the timedOut branch. + child.kill = () => { + setImmediate(() => child.emit('close', 1)); + }; + setImmediate(() => { + child.stdout.emit('data', Buffer.from(ndjson)); + // Never emit 'close' on our own — the adapter's timeout must fire. + }); + spawnMock.mockImplementation(() => child); + + const adapter = new OpenCodeAdapter(50); + const response = await adapter.invoke({ prompt: 'review this' }); + + expect(response.success).toBe(false); + expect(response.error).toContain('timed out'); + expect(response.output).toContain('"passed": true'); + }); +}); diff --git a/src/adapters/opencode-adapter.ts b/src/adapters/opencode-adapter.ts index 54c4acf..691466d 100644 --- a/src/adapters/opencode-adapter.ts +++ b/src/adapters/opencode-adapter.ts @@ -2,6 +2,55 @@ import spawn from 'cross-spawn'; import { DEFAULT_JOB_TIMEOUT_MS } from '../core/runtime-config.js'; import type { AgentAdapter, AgentInvocation, AgentResponse } from './base.js'; +/** + * opencode --format json emits a stream of NDJSON events (step_start, tool_use, + * text, step_finish...). The assistant's actual message content lives in the + * `text` events' `part.text`. Collapse the stream into those text parts joined + * by newlines so downstream consumers (rubric extraction, job_result, worker + * output) see plain text like the claude adapter produces — not raw event + * bytes. Returns null when no text part parsed (e.g. an error banner or empty + * stdout) so callers can fall back to the raw stream. + * + * NOTE: the collapse is applied on ALL resolve paths (success, timeout, + * non-zero exit, spawn error), unlike the claude/codex adapters which return + * raw stdout on failure. This is deliberate (a partial NDJSON stream is more + * useful collapsed), but it means a failed opencode job's stored output drops + * the raw event stream (including any error-type event) — observability only, + * since rubric extraction runs only on success. + * + * Schema pin: this matches the event shape opencode emits today (each line is + * `{"type":"text", ..., "part":{"type":"text","text":"..."}}`). If a future + * opencode bump changes the part shape (e.g. a `parts[]` array), this collapse + * silently degrades to the raw-stream fallback — the fixture + * `tests/fixtures/fake-agents/opencode-ndjson-text-parts.json` and the + * end-to-end test in `src/adapters/adapter.test.ts` will catch it. Re-verify + * against live output when bumping opencode. + */ +export const extractTextPartsFromNdjson = (stdout: string): string | null => { + const parts: string[] = []; + for (const line of stdout.split('\n')) { + if (!line.trim()) { + continue; + } + let event: unknown; + try { + event = JSON.parse(line); + } catch { + // Skip malformed lines — a context-limit kill can truncate the tail mid-event. + continue; + } + if (!event || typeof event !== 'object') { + // Valid JSON that isn't an event object (e.g. `null`, numbers) — skip. + continue; + } + const record = event as { type?: unknown; part?: { text?: unknown } }; + if (record.type === 'text' && typeof record.part?.text === 'string') { + parts.push(record.part.text); + } + } + return parts.length > 0 ? parts.join('\n') : null; +}; + export class OpenCodeAdapter implements AgentAdapter { public readonly name = 'opencode'; private readonly timeoutMs: number; @@ -54,7 +103,7 @@ export class OpenCodeAdapter implements AgentAdapter { if (timedOut) { resolve({ success: false, - output: stdout, + output: extractTextPartsFromNdjson(stdout) ?? stdout, error: `opencode invocation timed out after ${effectiveTimeout}ms`, durationMs, }); @@ -63,7 +112,7 @@ export class OpenCodeAdapter implements AgentAdapter { resolve({ success: code === 0, - output: stdout, + output: extractTextPartsFromNdjson(stdout) ?? stdout, error: code === 0 ? undefined : stderr || `opencode exited with code ${code ?? 'unknown'}`, durationMs, }); @@ -73,7 +122,7 @@ export class OpenCodeAdapter implements AgentAdapter { clearTimeout(timeout); resolve({ success: false, - output: stdout, + output: extractTextPartsFromNdjson(stdout) ?? stdout, error: error.message, durationMs: Date.now() - startedAt, }); diff --git a/src/core/integration.test.ts b/src/core/integration.test.ts index f97a970..7eff5e8 100644 --- a/src/core/integration.test.ts +++ b/src/core/integration.test.ts @@ -322,6 +322,1423 @@ describe('integration: blocking finding auto-resume', () => { expect(store.getJob(parent.id)?.status).toBe('blocked_on_finding'); }); + + it('C3: stale failed critic from an earlier eval round does not block a later all-green round', async () => { + const { store, manager, reportBlockingFinding } = createHarness([ + { match: matchers.devJob(), fixture: 'dev-with-changes.md' }, + // Round 1: code_critic FAILS (served once), then falls through to the passing fixture. + { match: matchers.evalStage('code_critic'), fixture: 'critic-failing-with-findings.json', maxUses: 1 }, + { match: matchers.evalStage('code_critic'), fixture: 'critic-passing-clean.json' }, + { match: matchers.evalStage('test_critic'), fixture: 'test-critic-passing.json' }, + { match: matchers.evalStage('interaction_e2e'), fixture: 'playwright-layer2-completed.json' }, + ]); + + const parent = store.createJob({ + type: 'dev', + status: 'running', + params: { name: 'final-report', prompt: 'verify', report_only: true }, + retryCount: 0, + maxRetries: 3, + }); + + const r = await reportBlockingFinding({ + job_id: parent.id, + finding: 'some bug', + why_it_blocks_report: 'report cannot pass', + recommended_followup: 'fix it', + }); + const parsed = parseResult(r); + const childId = parsed.child_job_id as string; + + await manager.waitForJob(childId, null, 5_000); + + const dispatchRound = async (): Promise => { + const code = manager.startJob('critic_eval', { + source_job_id: childId, + eval_stage: 'code_critic', + prompt: 'Code Critic review', + }); + const test = manager.startJob('eval', { + source_job_id: childId, + eval_stage: 'test_critic', + prompt: 'Test Critic review', + }); + const play = manager.startJob('interaction_e2e', { + source_job_id: childId, + eval_stage: 'interaction_e2e', + prompt: 'Interaction E2E eval', + }); + await manager.waitForJob(code.id, null, 5_000); + await manager.waitForJob(test.id, null, 5_000); + await manager.waitForJob(play.id, null, 5_000); + }; + + // Round 1: code_critic fails → parent stays blocked. + await dispatchRound(); + expect(store.getJob(parent.id)?.status).toBe('blocked_on_finding'); + + // Round 2 (re-fired eval after the child retry): all three pass. + await dispatchRound(); + + // The round-1 failed code_critic must not poison the gate — only the newest + // critic per stage counts, and round 2 is fully green. + expect(store.getJob(parent.id)?.status).toBe('pending'); + }); + + it('C4 (round-id): failing round 1 + all-green round 2 → parent resumes on round 2', async () => { + // NOTE: C4 is a REGRESSION test, not a discriminator — it passes under + // BOTH per-stage-newest and round-based logic (round 2 is the newest per + // stage AND the newest complete round). C5 is the true discriminator: it + // asserts the round gate does NOT mix a round-2 pass with a round-1 pass + // when round 2 is missing a stage. Keep C5 green when touching the gate. + const { store, manager, startEval, reportBlockingFinding } = createHarness([ + { match: matchers.devJob(), fixture: 'dev-with-changes.md' }, + { match: matchers.evalStage('code_critic'), fixture: 'critic-failing-with-findings.json', maxUses: 1 }, + { match: matchers.evalStage('code_critic'), fixture: 'critic-passing-clean.json' }, + { match: matchers.evalStage('test_critic'), fixture: 'test-critic-passing.json' }, + { match: matchers.evalStage('interaction_e2e'), fixture: 'playwright-layer2-completed.json' }, + ]); + + store.setConfig('eval_parallel_safe:credit-fixes', 'true'); + + const parent = store.createJob({ + type: 'dev', + status: 'running', + params: { name: 'final-report', prompt: 'verify', report_only: true }, + retryCount: 0, + maxRetries: 3, + }); + + const r = await reportBlockingFinding({ + job_id: parent.id, + finding: 'some bug', + why_it_blocks_report: 'report cannot pass', + recommended_followup: 'fix it', + }); + const childId = parseResult(r).child_job_id as string; + await manager.waitForJob(childId, null, 5_000); + + const runRound = async (): Promise => { + const result = await startEval({ job_id: childId, output: {}, feature: 'credit-fixes' }); + const parsed = parseResult(result); + for (const role of ['code_critic', 'test_critic', 'interaction_e2e']) { + const id = jobId(parsed, role); + await manager.waitForJob(id, null, 5_000); + } + }; + + // Round 1: code_critic fails → parent stays blocked. + await runRound(); + expect(store.getJob(parent.id)?.status).toBe('blocked_on_finding'); + + // Round 2: all pass → parent resumes. The gate keys on the newest round + // (newest eval_round id), not the newest critic per stage. startEval stamps + // eval_round on every critic, so this exercises the round-based path. + await runRound(); + expect(store.getJob(parent.id)?.status).toBe('pending'); + }); + + it('C6 (round-id, sequential): parent stays blocked until the LAST chained critic of the green round completes', async () => { + const { store, manager, startEval, reportBlockingFinding } = createHarness([ + { match: matchers.devJob(), fixture: 'dev-with-changes.md' }, + { match: matchers.evalStage('code_critic'), fixture: 'critic-failing-with-findings.json', maxUses: 1 }, + { match: matchers.evalStage('code_critic'), fixture: 'critic-passing-clean.json' }, + { match: matchers.evalStage('test_critic'), fixture: 'test-critic-passing.json' }, + { match: matchers.evalStage('interaction_e2e'), fixture: 'playwright-layer2-completed.json' }, + ]); + + // Sequential mode (default when no parallel verdict): critics run in roster + // order, each chained after its predecessor completes. + store.setConfig('eval_parallel_safe:credit-fixes', 'false'); + + const parent = store.createJob({ + type: 'dev', + status: 'running', + params: { name: 'final-report', prompt: 'verify', report_only: true }, + retryCount: 0, + maxRetries: 3, + }); + + const r = await reportBlockingFinding({ + job_id: parent.id, + finding: 'some bug', + why_it_blocks_report: 'report cannot pass', + recommended_followup: 'fix it', + }); + const childId = parseResult(r).child_job_id as string; + await manager.waitForJob(childId, null, 5_000); + + const ids = async (): Promise> => { + const result = await startEval({ job_id: childId, output: {}, feature: 'credit-fixes' }); + const parsed = parseResult(result); + const out: Record = {}; + for (const role of ['code_critic', 'test_critic', 'interaction_e2e']) { + out[role] = jobId(parsed, role); + } + return out; + }; + + // Round 1: all three chain, code_critic fails → parent stays blocked. + const round1 = await ids(); + await manager.waitForJob(round1.code_critic, null, 5_000); + await manager.waitForJob(round1.test_critic, null, 5_000); + await manager.waitForJob(round1.interaction_e2e, null, 5_000); + expect(store.getJob(parent.id)?.status).toBe('blocked_on_finding'); + + // Round 2: startEval in sequential mode chains test_critic and + // interaction_e2e as pending behind code_critic — assert the chaining + // (parentJobId set, eval_round stamped, not running yet) deterministically + // before any critic completes. + const round2 = await ids(); + const t2 = store.getJob(round2.test_critic); + const p2 = store.getJob(round2.interaction_e2e); + expect(t2?.status).toBe('pending'); + expect(t2?.parentJobId).toBe(round2.code_critic); + expect(typeof t2?.params.eval_round).toBe('string'); + expect(p2?.status).toBe('pending'); + expect(p2?.parentJobId).toBe(round2.test_critic); + // Mid-round: the chained critics are still pending, so round 2 is + // incomplete — the parent must NOT have resumed yet. (The fake adapter + // completes too fast to assert this after the round finishes, so the + // pending-chaining assert above is the deterministic mid-round signal.) + expect(store.getJob(parent.id)?.status).toBe('blocked_on_finding'); + + // Wait for the whole chained round; only after the LAST critic completes + // may the parent resume. + await manager.waitForJob(round2.code_critic, null, 5_000); + await manager.waitForJob(round2.test_critic, null, 5_000); + await manager.waitForJob(round2.interaction_e2e, null, 5_000); + expect(store.getJob(parent.id)?.status).toBe('pending'); + }); + + it('C5 (round-id): newest round missing a stage does NOT mix with older round (no cross-round unblock)', async () => { + // The exact scenario the user flagged: round 1's test_critic PASS must NOT + // combine with round 2's code_critic PASS when round 2 is missing + // test_critic. Per-stage-newest unblocks (mixing across code revisions); + // round-based stays blocked (round 2 is incomplete). + const { store, manager, reportBlockingFinding } = createHarness([ + { match: matchers.devJob(), fixture: 'dev-with-changes.md' }, + // code_critic: round 1 FAILS, round 2 PASSES (maxUses trick). + { match: matchers.evalStage('code_critic'), fixture: 'critic-failing-with-findings.json', maxUses: 1 }, + { match: matchers.evalStage('code_critic'), fixture: 'critic-passing-clean.json' }, + { match: matchers.evalStage('test_critic'), fixture: 'test-critic-passing.json' }, + { match: matchers.evalStage('interaction_e2e'), fixture: 'playwright-layer2-completed.json' }, + ]); + + const parent = store.createJob({ + type: 'dev', + status: 'running', + params: { name: 'final-report', prompt: 'verify', report_only: true }, + retryCount: 0, + maxRetries: 3, + }); + + const r = await reportBlockingFinding({ + job_id: parent.id, + finding: 'some bug', + why_it_blocks_report: 'report cannot pass', + recommended_followup: 'fix it', + }); + const childId = parseResult(r).child_job_id as string; + await manager.waitForJob(childId, null, 5_000); + + const stamp = (round: string, stage: string) => { + const promptLabel = stage === 'code_critic' ? 'Code Critic' + : stage === 'test_critic' ? 'Test Critic' + : 'Interaction E2E'; + return { + source_job_id: childId, + eval_stage: stage, + eval_round: round, + expected_eval_stages: ['code_critic', 'test_critic', 'interaction_e2e'], + prompt: `${promptLabel} review — stage: ${stage}`, + }; + }; + + // Round 1: code_critic FAIL (failing fixture), test_critic PASS, interaction_e2e PASS. + const r1c = manager.startJob('critic_eval', stamp('round-1', 'code_critic')); + const r1t = manager.startJob('eval', stamp('round-1', 'test_critic')); + const r1p = manager.startJob('interaction_e2e', stamp('round-1', 'interaction_e2e')); + await manager.waitForJob(r1c.id, null, 5_000); + await manager.waitForJob(r1t.id, null, 5_000); + await manager.waitForJob(r1p.id, null, 5_000); + // Round 1: code_critic failed → parent stays blocked. + expect(store.getJob(parent.id)?.status).toBe('blocked_on_finding'); + + // Round 2: code_critic PASS, interaction_e2e PASS, but NO test_critic + // (simulates a critic that wasn't dispatched or context-limited away). + const r2c = manager.startJob('critic_eval', stamp('round-2', 'code_critic')); + const r2p = manager.startJob('interaction_e2e', stamp('round-2', 'interaction_e2e')); + await manager.waitForJob(r2c.id, null, 5_000); + await manager.waitForJob(r2p.id, null, 5_000); + + // Per-stage-newest would pick: code_critic = round 2 PASS, test_critic = + // round 1 PASS → UNBLOCK (mixing). Round-based: round 2 is incomplete + // (missing test_critic) → stays blocked. This is the exact behavior change. + expect(store.getJob(parent.id)?.status).toBe('blocked_on_finding'); + }); + + it('C7 (round-id): a job-level failed critic in the newest round keeps the parent blocked', async () => { + const { store, manager, startEval, reportBlockingFinding } = createHarness([ + { match: matchers.devJob(), fixture: 'dev-with-changes.md' }, + // Round 1: code_critic FAILS at the rubric level (job completes). + { match: matchers.evalStage('code_critic'), fixture: 'critic-failing-with-findings.json', maxUses: 1 }, + // Round 2: code_critic FAILS at the JOB level (adapter success:false). + { match: matchers.evalStage('code_critic'), fixture: 'critic-passing-clean.json', success: false }, + { match: matchers.evalStage('test_critic'), fixture: 'test-critic-passing.json' }, + { match: matchers.evalStage('interaction_e2e'), fixture: 'playwright-layer2-completed.json' }, + ]); + + store.setConfig('eval_parallel_safe:credit-fixes', 'true'); + + const parent = store.createJob({ + type: 'dev', + status: 'running', + params: { name: 'final-report', prompt: 'verify', report_only: true }, + retryCount: 0, + maxRetries: 3, + }); + + const r = await reportBlockingFinding({ + job_id: parent.id, + finding: 'some bug', + why_it_blocks_report: 'report cannot pass', + recommended_followup: 'fix it', + }); + const childId = parseResult(r).child_job_id as string; + await manager.waitForJob(childId, null, 5_000); + + const runRound = async (): Promise => { + const result = await startEval({ job_id: childId, output: {}, feature: 'credit-fixes' }); + const parsed = parseResult(result); + for (const role of ['code_critic', 'test_critic', 'interaction_e2e']) { + const id = jobId(parsed, role); + await manager.waitForJob(id, null, 5_000); + } + }; + + // Round 1: code_critic rubric-fails → parent stays blocked. + await runRound(); + expect(store.getJob(parent.id)?.status).toBe('blocked_on_finding'); + + // Round 2: code_critic fails at the job level. The gate must NOT unblock — + // the status check is the only guard against a failed critic that carried + // stored passing output. + await runRound(); + expect(store.getJob(parent.id)?.status).toBe('blocked_on_finding'); + }); + + it('C8 (round-id): an ad-hoc unstamped critic cannot disable the round gate (no cross-round unblock)', async () => { + const { store, manager, reportBlockingFinding } = createHarness([ + { match: matchers.devJob(), fixture: 'dev-with-changes.md' }, + // Round 1 code_critic FAILS (served once), then the ad-hoc re-fire PASSES. + { match: matchers.evalStage('code_critic'), fixture: 'critic-failing-with-findings.json', maxUses: 1 }, + { match: matchers.evalStage('code_critic'), fixture: 'critic-passing-clean.json' }, + { match: matchers.evalStage('test_critic'), fixture: 'test-critic-passing.json' }, + { match: matchers.evalStage('interaction_e2e'), fixture: 'playwright-layer2-completed.json' }, + ]); + + const parent = store.createJob({ + type: 'dev', + status: 'running', + params: { name: 'final-report', prompt: 'verify', report_only: true }, + retryCount: 0, + maxRetries: 3, + }); + + const r = await reportBlockingFinding({ + job_id: parent.id, + finding: 'some bug', + why_it_blocks_report: 'report cannot pass', + recommended_followup: 'fix it', + }); + const childId = parseResult(r).child_job_id as string; + await manager.waitForJob(childId, null, 5_000); + + const stamp = (round: string, stage: string) => { + const promptLabel = stage === 'code_critic' ? 'Code Critic' + : stage === 'test_critic' ? 'Test Critic' + : 'Interaction E2E'; + return { + source_job_id: childId, + eval_stage: stage, + eval_round: round, + expected_eval_stages: ['code_critic', 'test_critic', 'interaction_e2e'], + prompt: `${promptLabel} review — stage: ${stage}`, + }; + }; + + // Round 1 (stamped): code_critic FAILS, test_critic + interaction_e2e PASS. + const r1c = manager.startJob('critic_eval', stamp('round-1', 'code_critic')); + const r1t = manager.startJob('eval', stamp('round-1', 'test_critic')); + const r1p = manager.startJob('interaction_e2e', stamp('round-1', 'interaction_e2e')); + await manager.waitForJob(r1c.id, null, 5_000); + await manager.waitForJob(r1t.id, null, 5_000); + await manager.waitForJob(r1p.id, null, 5_000); + expect(store.getJob(parent.id)?.status).toBe('blocked_on_finding'); + + // Ad-hoc re-fire of code_critic (tenet_start_job-style dispatch): NO + // eval_round. It PASSES. The round gate must ignore it — round 1's failed + // code_critic still decides, so the parent stays blocked. (Per-stage-newest + // would mix the ad-hoc pass with round 1's green critics and unblock.) + const adHoc = manager.startJob('critic_eval', { + source_job_id: childId, + eval_stage: 'code_critic', + prompt: 'Code Critic review — ad-hoc re-fire', + }); + await manager.waitForJob(adHoc.id, null, 5_000); + expect(store.getJob(parent.id)?.status).toBe('blocked_on_finding'); + }); + + it('C9 (round-id): a legacy unstamped critic newer than the newest round does not unblock', async () => { + const { store, manager, reportBlockingFinding } = createHarness([ + { match: matchers.devJob(), fixture: 'dev-with-changes.md' }, + // code_critic FAILS in round 1 and round 2 (served twice), then the + // legacy unstamped critic PASSES. + { match: matchers.evalStage('code_critic'), fixture: 'critic-failing-with-findings.json', maxUses: 2 }, + { match: matchers.evalStage('code_critic'), fixture: 'critic-passing-clean.json' }, + { match: matchers.evalStage('test_critic'), fixture: 'test-critic-passing.json' }, + { match: matchers.evalStage('interaction_e2e'), fixture: 'playwright-layer2-completed.json' }, + ]); + + const parent = store.createJob({ + type: 'dev', + status: 'running', + params: { name: 'final-report', prompt: 'verify', report_only: true }, + retryCount: 0, + maxRetries: 3, + }); + + const r = await reportBlockingFinding({ + job_id: parent.id, + finding: 'some bug', + why_it_blocks_report: 'report cannot pass', + recommended_followup: 'fix it', + }); + const childId = parseResult(r).child_job_id as string; + await manager.waitForJob(childId, null, 5_000); + + const stamp = (round: string, stage: string) => { + const promptLabel = stage === 'code_critic' ? 'Code Critic' + : stage === 'test_critic' ? 'Test Critic' + : 'Interaction E2E'; + return { + source_job_id: childId, + eval_stage: stage, + eval_round: round, + expected_eval_stages: ['code_critic', 'test_critic', 'interaction_e2e'], + prompt: `${promptLabel} review — stage: ${stage}`, + }; + }; + + // Round 1 (stamped): code_critic FAILS, test_critic + interaction_e2e PASS. + const r1c = manager.startJob('critic_eval', stamp('round-1', 'code_critic')); + const r1t = manager.startJob('eval', stamp('round-1', 'test_critic')); + const r1p = manager.startJob('interaction_e2e', stamp('round-1', 'interaction_e2e')); + await manager.waitForJob(r1c.id, null, 5_000); + await manager.waitForJob(r1t.id, null, 5_000); + await manager.waitForJob(r1p.id, null, 5_000); + expect(store.getJob(parent.id)?.status).toBe('blocked_on_finding'); + + // Round 2 (stamped): code_critic FAILS again, test_critic + interaction_e2e PASS. + const r2c = manager.startJob('critic_eval', stamp('round-2', 'code_critic')); + const r2t = manager.startJob('eval', stamp('round-2', 'test_critic')); + const r2p = manager.startJob('interaction_e2e', stamp('round-2', 'interaction_e2e')); + await manager.waitForJob(r2c.id, null, 5_000); + await manager.waitForJob(r2t.id, null, 5_000); + await manager.waitForJob(r2p.id, null, 5_000); + expect(store.getJob(parent.id)?.status).toBe('blocked_on_finding'); + + // Legacy unstamped code_critic PASSES, created after round 2. The round + // gate must ignore it — round 2 (newest) has a failed code_critic, so the + // parent stays blocked. (Per-stage-newest would pick the legacy pass + + // round 2's green critics and unblock.) + const legacy = manager.startJob('critic_eval', { + source_job_id: childId, + eval_stage: 'code_critic', + prompt: 'Code Critic review — legacy unstamped', + }); + await manager.waitForJob(legacy.id, null, 5_000); + expect(store.getJob(parent.id)?.status).toBe('blocked_on_finding'); + }); + + it('C10 (round-id): a newer unstamped RED critic cannot be ignored while the parent unblocks on an older green round', async () => { + // The fail-open direction of the unstamped blind spot: an ad-hoc re-fire + // (tenet_start_job, no eval_round) that comes back RED must force the gate + // to wait — not be invisible while the parent unblocks on an older round's + // stale green. Unstamped siblings become singleton rounds, which can never + // satisfy "every expected stage present", so the gate stays blocked. + const { store, manager, reportBlockingFinding } = createHarness([ + { match: matchers.devJob(), fixture: 'dev-with-changes.md' }, + // Round 1 code_critic PASSES (served once), then the ad-hoc re-fire FAILS. + { match: matchers.evalStage('code_critic'), fixture: 'critic-passing-clean.json', maxUses: 1 }, + { match: matchers.evalStage('code_critic'), fixture: 'critic-failing-with-findings.json' }, + { match: matchers.evalStage('test_critic'), fixture: 'test-critic-passing.json' }, + { match: matchers.evalStage('interaction_e2e'), fixture: 'playwright-layer2-completed.json' }, + ]); + + const parent = store.createJob({ + type: 'dev', + status: 'running', + params: { name: 'final-report', prompt: 'verify', report_only: true }, + retryCount: 0, + maxRetries: 3, + }); + + const r = await reportBlockingFinding({ + job_id: parent.id, + finding: 'some bug', + why_it_blocks_report: 'report cannot pass', + recommended_followup: 'fix it', + }); + const childId = parseResult(r).child_job_id as string; + await manager.waitForJob(childId, null, 5_000); + + const stamp = (round: string, stage: string) => { + const promptLabel = stage === 'code_critic' ? 'Code Critic' + : stage === 'test_critic' ? 'Test Critic' + : 'Interaction E2E'; + return { + source_job_id: childId, + eval_stage: stage, + eval_round: round, + expected_eval_stages: ['code_critic', 'test_critic', 'interaction_e2e'], + prompt: `${promptLabel} review — stage: ${stage}`, + }; + }; + + // Round 1 (stamped): code_critic + test_critic PASS. + const r1c = manager.startJob('critic_eval', stamp('round-1', 'code_critic')); + const r1t = manager.startJob('eval', stamp('round-1', 'test_critic')); + await manager.waitForJob(r1c.id, null, 5_000); + await manager.waitForJob(r1t.id, null, 5_000); + + // Round 1's last critic (interaction_e2e) is created, THEN an ad-hoc + // unstamped code_critic re-fire is created — so the ad-hoc is the NEWEST + // sibling. It FAILS (passed:false) and must not be invisible to the gate. + const r1p = manager.startJob('interaction_e2e', stamp('round-1', 'interaction_e2e')); + const adHoc = manager.startJob('critic_eval', { + source_job_id: childId, + eval_stage: 'code_critic', + prompt: 'Code Critic review — ad-hoc re-fire', + }); + await manager.waitForJob(adHoc.id, null, 5_000); + await manager.waitForJob(r1p.id, null, 5_000); + + // The gate must NOT unblock on round 1's stale green while a newer red + // evaluation exists. + expect(store.getJob(parent.id)?.status).toBe('blocked_on_finding'); + }); + + it('C11 (round-id): a malformed expected_eval_stages stamp cannot fail the gate open', async () => { + // A stamp that filters to an empty set ([123]) must fall back to + // DEFAULT_EVAL_STAGES — otherwise the stage-presence and completion loops + // pass trivially and the parent unblocks with no critic evaluated. + const { store, manager, reportBlockingFinding } = createHarness([ + { match: matchers.devJob(), fixture: 'dev-with-changes.md' }, + { match: matchers.evalStage('code_critic'), fixture: 'critic-passing-clean.json' }, + { match: matchers.evalStage('test_critic'), fixture: 'test-critic-passing.json' }, + { match: matchers.evalStage('interaction_e2e'), fixture: 'playwright-layer2-completed.json' }, + ]); + + const parent = store.createJob({ + type: 'dev', + status: 'running', + params: { name: 'final-report', prompt: 'verify', report_only: true }, + retryCount: 0, + maxRetries: 3, + }); + + const r = await reportBlockingFinding({ + job_id: parent.id, + finding: 'some bug', + why_it_blocks_report: 'report cannot pass', + recommended_followup: 'fix it', + }); + const childId = parseResult(r).child_job_id as string; + await manager.waitForJob(childId, null, 5_000); + + // A single-critic round with a malformed stamp: only code_critic exists, + // so even with the DEFAULT_EVAL_STAGES fallback the round is incomplete. + const malformed = manager.startJob('critic_eval', { + source_job_id: childId, + eval_stage: 'code_critic', + eval_round: 'round-1', + expected_eval_stages: [123], + prompt: 'Code Critic review — malformed stamp', + }); + await manager.waitForJob(malformed.id, null, 5_000); + + // With an empty expectedStages the gate would unblock (both loops pass + // trivially). With the DEFAULT_EVAL_STAGES fallback it stays blocked. + expect(store.getJob(parent.id)?.status).toBe('blocked_on_finding'); + }); + + it('C12 (round-id): a cancelled critic in the newest round keeps the parent blocked', async () => { + // The status check (s.status !== 'completed') is the only guard against a + // cancelled critic. A cancelled critic must keep the round incomplete — + // the parent stays blocked until a fresh round is dispatched. + const { store, manager, reportBlockingFinding } = createHarness([ + { match: matchers.devJob(), fixture: 'dev-with-changes.md' }, + { match: matchers.evalStage('code_critic'), fixture: 'critic-passing-clean.json' }, + { match: matchers.evalStage('test_critic'), fixture: 'test-critic-passing.json' }, + { match: matchers.evalStage('interaction_e2e'), fixture: 'playwright-layer2-completed.json' }, + ]); + + const parent = store.createJob({ + type: 'dev', + status: 'running', + params: { name: 'final-report', prompt: 'verify', report_only: true }, + retryCount: 0, + maxRetries: 3, + }); + + const r = await reportBlockingFinding({ + job_id: parent.id, + finding: 'some bug', + why_it_blocks_report: 'report cannot pass', + recommended_followup: 'fix it', + }); + const childId = parseResult(r).child_job_id as string; + await manager.waitForJob(childId, null, 5_000); + + const stamp = (round: string, stage: string) => { + const promptLabel = stage === 'code_critic' ? 'Code Critic' + : stage === 'test_critic' ? 'Test Critic' + : 'Interaction E2E'; + return { + source_job_id: childId, + eval_stage: stage, + eval_round: round, + expected_eval_stages: ['code_critic', 'test_critic', 'interaction_e2e'], + prompt: `${promptLabel} review — stage: ${stage}`, + }; + }; + + // Round 1 (stamped): cancel test_critic synchronously while it is still + // pending (before the dispatch loop picks it up). + const r1c = manager.startJob('critic_eval', stamp('round-1', 'code_critic')); + const r1t = manager.startJob('eval', stamp('round-1', 'test_critic')); + const r1p = manager.startJob('interaction_e2e', stamp('round-1', 'interaction_e2e')); + manager.cancelJob(r1t.id); + expect(store.getJob(r1t.id)?.status).toBe('cancelled'); + + // The other two critics complete green, but the round is incomplete + // (test_critic cancelled) — the parent must stay blocked. + await manager.waitForJob(r1c.id, null, 5_000); + await manager.waitForJob(r1p.id, null, 5_000); + expect(store.getJob(parent.id)?.status).toBe('blocked_on_finding'); + }); + + it('C13 (round-id): an unstamped singleton cannot self-stamp its way to a complete round', async () => { + // An ad-hoc re-fire (tenet_start_job) could pass expected_eval_stages: + // ['code_critic'] and satisfy the gate on one critic's verdict. Unstamped + // rounds must always require the full DEFAULT_EVAL_STAGES. + const { store, manager, reportBlockingFinding } = createHarness([ + { match: matchers.devJob(), fixture: 'dev-with-changes.md' }, + // Round 1 code_critic FAILS (served once), then the ad-hoc re-fire PASSES. + { match: matchers.evalStage('code_critic'), fixture: 'critic-failing-with-findings.json', maxUses: 1 }, + { match: matchers.evalStage('code_critic'), fixture: 'critic-passing-clean.json' }, + { match: matchers.evalStage('test_critic'), fixture: 'test-critic-passing.json' }, + { match: matchers.evalStage('interaction_e2e'), fixture: 'playwright-layer2-completed.json' }, + ]); + + const parent = store.createJob({ + type: 'dev', + status: 'running', + params: { name: 'final-report', prompt: 'verify', report_only: true }, + retryCount: 0, + maxRetries: 3, + }); + + const r = await reportBlockingFinding({ + job_id: parent.id, + finding: 'some bug', + why_it_blocks_report: 'report cannot pass', + recommended_followup: 'fix it', + }); + const childId = parseResult(r).child_job_id as string; + await manager.waitForJob(childId, null, 5_000); + + const stamp = (round: string, stage: string) => { + const promptLabel = stage === 'code_critic' ? 'Code Critic' + : stage === 'test_critic' ? 'Test Critic' + : 'Interaction E2E'; + return { + source_job_id: childId, + eval_stage: stage, + eval_round: round, + expected_eval_stages: ['code_critic', 'test_critic', 'interaction_e2e'], + prompt: `${promptLabel} review — stage: ${stage}`, + }; + }; + + // Round 1 (stamped): code_critic FAILS, test_critic + interaction_e2e PASS. + const r1c = manager.startJob('critic_eval', stamp('round-1', 'code_critic')); + const r1t = manager.startJob('eval', stamp('round-1', 'test_critic')); + const r1p = manager.startJob('interaction_e2e', stamp('round-1', 'interaction_e2e')); + await manager.waitForJob(r1c.id, null, 5_000); + await manager.waitForJob(r1t.id, null, 5_000); + await manager.waitForJob(r1p.id, null, 5_000); + expect(store.getJob(parent.id)?.status).toBe('blocked_on_finding'); + + // Ad-hoc unstamped code_critic re-fire carrying a self-serving single-stage + // stamp. It PASSES and is the newest sibling. It must NOT satisfy the gate. + const adHoc = manager.startJob('critic_eval', { + source_job_id: childId, + eval_stage: 'code_critic', + expected_eval_stages: ['code_critic'], + prompt: 'Code Critic review — self-stamped re-fire', + }); + await manager.waitForJob(adHoc.id, null, 5_000); + expect(store.getJob(parent.id)?.status).toBe('blocked_on_finding'); + }); + + it('C14 (per-stage): a green ad-hoc re-fire created long after the round cannot mask a red original', async () => { + // The 5.5s delay to cross COHORT_WINDOW_MS exceeds the default 5s timeout. + // All-legacy DB (no eval_round anywhere) → the per-stage fallback runs. + // A single ad-hoc re-fire created > COHORT_WINDOW_MS after the original + // round is a partial re-evaluation and must not mask the red original. + const { store, manager, reportBlockingFinding } = createHarness([ + { match: matchers.devJob(), fixture: 'dev-with-changes.md' }, + // Original code_critic FAILS (served once), then the ad-hoc re-fire PASSES. + { match: matchers.evalStage('code_critic'), fixture: 'critic-failing-with-findings.json', maxUses: 1 }, + { match: matchers.evalStage('code_critic'), fixture: 'critic-passing-clean.json' }, + { match: matchers.evalStage('test_critic'), fixture: 'test-critic-passing.json' }, + { match: matchers.evalStage('interaction_e2e'), fixture: 'playwright-layer2-completed.json' }, + ]); + + const parent = store.createJob({ + type: 'dev', + status: 'running', + params: { name: 'final-report', prompt: 'verify', report_only: true }, + retryCount: 0, + maxRetries: 3, + }); + + const r = await reportBlockingFinding({ + job_id: parent.id, + finding: 'some bug', + why_it_blocks_report: 'report cannot pass', + recommended_followup: 'fix it', + }); + const childId = parseResult(r).child_job_id as string; + await manager.waitForJob(childId, null, 5_000); + + // Original round (no eval_round): code_critic FAILS, test + interaction PASS. + const c = manager.startJob('critic_eval', { + source_job_id: childId, + eval_stage: 'code_critic', + prompt: 'Code Critic review', + }); + const t = manager.startJob('eval', { + source_job_id: childId, + eval_stage: 'test_critic', + prompt: 'Test Critic review', + }); + const p = manager.startJob('interaction_e2e', { + source_job_id: childId, + eval_stage: 'interaction_e2e', + prompt: 'Interaction E2E eval', + }); + await manager.waitForJob(c.id, null, 5_000); + await manager.waitForJob(t.id, null, 5_000); + await manager.waitForJob(p.id, null, 5_000); + expect(store.getJob(parent.id)?.status).toBe('blocked_on_finding'); + + // Ad-hoc code_critic re-fire created > COHORT_WINDOW_MS later. It PASSES. + // The per-stage gate must NOT pick it as the newest code_critic and unblock + // on a partial re-evaluation. + await new Promise((resolve) => setTimeout(resolve, 5_500)); + const adHoc = manager.startJob('critic_eval', { + source_job_id: childId, + eval_stage: 'code_critic', + prompt: 'Code Critic review — ad-hoc re-fire', + }); + await manager.waitForJob(adHoc.id, null, 5_000); + expect(store.getJob(parent.id)?.status).toBe('blocked_on_finding'); + }, 15_000); + + it('C15 (per-stage): a single-critic roster unblocks (consistent with the round gate)', async () => { + // All-legacy DB → per-stage fallback. A single code_critic carrying + // expected_eval_stages: ['code_critic'] is a legitimate 1-critic roster + // (or a self-serving stamp — the two are indistinguishable by shape, and + // the round gate has no minimum either). The gate unblocks on the one + // critic's verdict, matching the round gate's behavior for a stamped + // 1-critic round. + const { store, manager, reportBlockingFinding } = createHarness([ + { match: matchers.devJob(), fixture: 'dev-with-changes.md' }, + { match: matchers.evalStage('code_critic'), fixture: 'critic-passing-clean.json' }, + ]); + + const parent = store.createJob({ + type: 'dev', + status: 'running', + params: { name: 'final-report', prompt: 'verify', report_only: true }, + retryCount: 0, + maxRetries: 3, + }); + + const r = await reportBlockingFinding({ + job_id: parent.id, + finding: 'some bug', + why_it_blocks_report: 'report cannot pass', + recommended_followup: 'fix it', + }); + const childId = parseResult(r).child_job_id as string; + await manager.waitForJob(childId, null, 5_000); + + const single = manager.startJob('critic_eval', { + source_job_id: childId, + eval_stage: 'code_critic', + expected_eval_stages: ['code_critic'], + prompt: 'Code Critic review — self-stamped', + }); + await manager.waitForJob(single.id, null, 5_000); + expect(store.getJob(parent.id)?.status).toBe('pending'); + }); + + it('C16 (per-stage): a self-serving 2-stage stamp cannot exclude a red stage from the gate', async () => { + // All-legacy DB → per-stage fallback. The fallback always requires the + // full DEFAULT_EVAL_STAGES roster — a self-serving 2-stage stamp on an + // ad-hoc re-fire must not exclude a red stage and unblock. + const { store, manager, reportBlockingFinding } = createHarness([ + { match: matchers.devJob(), fixture: 'dev-with-changes.md' }, + // Original code_critic FAILS (served once), then the ad-hoc re-fire PASSES. + { match: matchers.evalStage('code_critic'), fixture: 'critic-failing-with-findings.json', maxUses: 1 }, + { match: matchers.evalStage('code_critic'), fixture: 'critic-passing-clean.json' }, + // test_critic FAILS (a red non-re-fired stage the 2-stage stamp excludes). + { match: matchers.evalStage('test_critic'), fixture: 'critic-failing-with-findings.json' }, + { match: matchers.evalStage('interaction_e2e'), fixture: 'playwright-layer2-completed.json' }, + ]); + + const parent = store.createJob({ + type: 'dev', + status: 'running', + params: { name: 'final-report', prompt: 'verify', report_only: true }, + retryCount: 0, + maxRetries: 3, + }); + + const r = await reportBlockingFinding({ + job_id: parent.id, + finding: 'some bug', + why_it_blocks_report: 'report cannot pass', + recommended_followup: 'fix it', + }); + const childId = parseResult(r).child_job_id as string; + await manager.waitForJob(childId, null, 5_000); + + // Original round (no eval_round): code FAILS, test FAILS, interaction + // PASSES. The originals carry the full roster stamp (as a real legacy + // dispatch would). + const fullRoster = ['code_critic', 'test_critic', 'interaction_e2e']; + const c = manager.startJob('critic_eval', { + source_job_id: childId, + eval_stage: 'code_critic', + expected_eval_stages: fullRoster, + prompt: 'Code Critic review', + }); + const t = manager.startJob('eval', { + source_job_id: childId, + eval_stage: 'test_critic', + expected_eval_stages: fullRoster, + prompt: 'Test Critic review', + }); + const p = manager.startJob('interaction_e2e', { + source_job_id: childId, + eval_stage: 'interaction_e2e', + expected_eval_stages: fullRoster, + prompt: 'Interaction E2E eval', + }); + await manager.waitForJob(c.id, null, 5_000); + await manager.waitForJob(t.id, null, 5_000); + await manager.waitForJob(p.id, null, 5_000); + expect(store.getJob(parent.id)?.status).toBe('blocked_on_finding'); + + // Ad-hoc code_critic re-fire with a 2-stage stamp excluding the red + // test_critic. It PASSES. The gate must still require test_critic (red). + const adHoc = manager.startJob('critic_eval', { + source_job_id: childId, + eval_stage: 'code_critic', + expected_eval_stages: ['code_critic', 'interaction_e2e'], + prompt: 'Code Critic review — 2-stage self-stamp', + }); + await manager.waitForJob(adHoc.id, null, 5_000); + expect(store.getJob(parent.id)?.status).toBe('blocked_on_finding'); + }); + + it('C17 (round-id): an unstamped critic older than the newest stamped round is history', async () => { + // The round gate keys on the newest stamped round. An unstamped critic + // created between two stamped rounds is an older singleton — it never + // decides the gate, and a newer full stamped round wins. + const { store, manager, reportBlockingFinding } = createHarness([ + { match: matchers.devJob(), fixture: 'dev-with-changes.md' }, + // R1 code_critic FAILS (served once), ad-hoc FAILS (served again), R2 PASSES. + { match: matchers.evalStage('code_critic'), fixture: 'critic-failing-with-findings.json', maxUses: 2 }, + { match: matchers.evalStage('code_critic'), fixture: 'critic-passing-clean.json' }, + { match: matchers.evalStage('test_critic'), fixture: 'test-critic-passing.json' }, + { match: matchers.evalStage('interaction_e2e'), fixture: 'playwright-layer2-completed.json' }, + ]); + + const parent = store.createJob({ + type: 'dev', + status: 'running', + params: { name: 'final-report', prompt: 'verify', report_only: true }, + retryCount: 0, + maxRetries: 3, + }); + + const r = await reportBlockingFinding({ + job_id: parent.id, + finding: 'some bug', + why_it_blocks_report: 'report cannot pass', + recommended_followup: 'fix it', + }); + const childId = parseResult(r).child_job_id as string; + await manager.waitForJob(childId, null, 5_000); + + const stamp = (round: string, stage: string) => { + const promptLabel = stage === 'code_critic' ? 'Code Critic' + : stage === 'test_critic' ? 'Test Critic' + : 'Interaction E2E'; + return { + source_job_id: childId, + eval_stage: stage, + eval_round: round, + expected_eval_stages: ['code_critic', 'test_critic', 'interaction_e2e'], + prompt: `${promptLabel} review — stage: ${stage}`, + }; + }; + + // R1 (stamped): code_critic FAILS, test + interaction PASS → blocked. + const r1c = manager.startJob('critic_eval', stamp('round-1', 'code_critic')); + const r1t = manager.startJob('eval', stamp('round-1', 'test_critic')); + const r1p = manager.startJob('interaction_e2e', stamp('round-1', 'interaction_e2e')); + await manager.waitForJob(r1c.id, null, 5_000); + await manager.waitForJob(r1t.id, null, 5_000); + await manager.waitForJob(r1p.id, null, 5_000); + expect(store.getJob(parent.id)?.status).toBe('blocked_on_finding'); + + // Ad-hoc unstamped code_critic re-fire (FAILS) created after R1. + const adHoc = manager.startJob('critic_eval', { + source_job_id: childId, + eval_stage: 'code_critic', + prompt: 'Code Critic review — ad-hoc re-fire', + }); + await manager.waitForJob(adHoc.id, null, 5_000); + expect(store.getJob(parent.id)?.status).toBe('blocked_on_finding'); + + // R2 (stamped): all green. The gate keys on R2 (newest stamped round) and + // unblocks — the older unstamped ad-hoc is history. + const r2c = manager.startJob('critic_eval', stamp('round-2', 'code_critic')); + const r2t = manager.startJob('eval', stamp('round-2', 'test_critic')); + const r2p = manager.startJob('interaction_e2e', stamp('round-2', 'interaction_e2e')); + await manager.waitForJob(r2c.id, null, 5_000); + await manager.waitForJob(r2t.id, null, 5_000); + await manager.waitForJob(r2p.id, null, 5_000); + expect(store.getJob(parent.id)?.status).toBe('pending'); + }); + + it('C18 (round-id): on a same-ms createdAt tie, the NEWER round wins (>= tie-break)', async () => { + // The >= tie-break keeps the later-seen round when two rounds share a max + // createdAt — with > the older round would win and unblock on stale green. + // Force a tie by rewriting created_at in the DB. + const { store, manager, reportBlockingFinding } = createHarness([ + { match: matchers.devJob(), fixture: 'dev-with-changes.md' }, + // R1 code_critic FAILS (served once), R2 code_critic PASSES. + { match: matchers.evalStage('code_critic'), fixture: 'critic-failing-with-findings.json', maxUses: 1 }, + { match: matchers.evalStage('code_critic'), fixture: 'critic-passing-clean.json' }, + { match: matchers.evalStage('test_critic'), fixture: 'test-critic-passing.json' }, + { match: matchers.evalStage('interaction_e2e'), fixture: 'playwright-layer2-completed.json' }, + ]); + + const parent = store.createJob({ + type: 'dev', + status: 'running', + params: { name: 'final-report', prompt: 'verify', report_only: true }, + retryCount: 0, + maxRetries: 3, + }); + + const r = await reportBlockingFinding({ + job_id: parent.id, + finding: 'some bug', + why_it_blocks_report: 'report cannot pass', + recommended_followup: 'fix it', + }); + const childId = parseResult(r).child_job_id as string; + await manager.waitForJob(childId, null, 5_000); + + const stamp = (round: string, stage: string) => { + const promptLabel = stage === 'code_critic' ? 'Code Critic' + : stage === 'test_critic' ? 'Test Critic' + : 'Interaction E2E'; + return { + source_job_id: childId, + eval_stage: stage, + eval_round: round, + expected_eval_stages: ['code_critic', 'test_critic', 'interaction_e2e'], + prompt: `${promptLabel} review — stage: ${stage}`, + }; + }; + + // R1 (stamped): code_critic FAILS, test + interaction PASS → blocked. + const r1c = manager.startJob('critic_eval', stamp('round-1', 'code_critic')); + const r1t = manager.startJob('eval', stamp('round-1', 'test_critic')); + const r1p = manager.startJob('interaction_e2e', stamp('round-1', 'interaction_e2e')); + await manager.waitForJob(r1c.id, null, 5_000); + await manager.waitForJob(r1t.id, null, 5_000); + await manager.waitForJob(r1p.id, null, 5_000); + expect(store.getJob(parent.id)?.status).toBe('blocked_on_finding'); + + // R2 (stamped): all green. Force a same-ms tie on created_at BEFORE the + // dispatch loop runs R2's critics, so both rounds share a max createdAt. + const r2c = manager.startJob('critic_eval', stamp('round-2', 'code_critic')); + const r2t = manager.startJob('eval', stamp('round-2', 'test_critic')); + const r2p = manager.startJob('interaction_e2e', stamp('round-2', 'interaction_e2e')); + const db = (store as unknown as { db: { prepare: (sql: string) => { run: (...a: unknown[]) => void } } }).db; + const tieAt = store.getJob(r1c.id)?.createdAt ?? Date.now(); + for (const id of [r1c.id, r1t.id, r1p.id, r2c.id, r2t.id, r2p.id]) { + db.prepare('UPDATE jobs SET created_at = ? WHERE id = ?').run(tieAt, id); + } + + await manager.waitForJob(r2c.id, null, 5_000); + await manager.waitForJob(r2t.id, null, 5_000); + await manager.waitForJob(r2p.id, null, 5_000); + + // With >= the later-seen round (R2, green) wins the tie → unblock. With > + // the older round (R1, red) would win → stay blocked. + expect(store.getJob(parent.id)?.status).toBe('pending'); + }); + + it('C19 (round-id): a stamped 1-critic round (legitimate roster) unblocks', async () => { + // A project with a 1-critic roster dispatches a stamped singleton via + // tenet_start_eval. The round gate must use the consensus roster stamp + // (['code_critic']), not the full DEFAULT_EVAL_STAGES, or the parent + // strands forever. + const { store, manager, reportBlockingFinding } = createHarness([ + { match: matchers.devJob(), fixture: 'dev-with-changes.md' }, + { match: matchers.evalStage('code_critic'), fixture: 'critic-passing-clean.json' }, + ]); + + const parent = store.createJob({ + type: 'dev', + status: 'running', + params: { name: 'final-report', prompt: 'verify', report_only: true }, + retryCount: 0, + maxRetries: 3, + }); + + const r = await reportBlockingFinding({ + job_id: parent.id, + finding: 'some bug', + why_it_blocks_report: 'report cannot pass', + recommended_followup: 'fix it', + }); + const childId = parseResult(r).child_job_id as string; + await manager.waitForJob(childId, null, 5_000); + + // A stamped singleton round: eval_round + a single-stage roster stamp. + const single = manager.startJob('critic_eval', { + source_job_id: childId, + eval_stage: 'code_critic', + eval_round: 'round-1', + expected_eval_stages: ['code_critic'], + prompt: 'Code Critic review — 1-critic roster', + }); + await manager.waitForJob(single.id, null, 5_000); + expect(store.getJob(parent.id)?.status).toBe('pending'); + }); + + it('C20 (per-stage): a partial-stamp ad-hoc re-fire cannot shrink the roster when originals are unstamped', async () => { + // All-legacy DB where the originals predate the expected_eval_stages stamp. + // A single ad-hoc re-fire carrying a partial stamp must NOT become the + // consensus roster and exclude a red stage. + const { store, manager, reportBlockingFinding } = createHarness([ + { match: matchers.devJob(), fixture: 'dev-with-changes.md' }, + // Original code_critic FAILS (served once), then the ad-hoc re-fire PASSES. + { match: matchers.evalStage('code_critic'), fixture: 'critic-failing-with-findings.json', maxUses: 1 }, + { match: matchers.evalStage('code_critic'), fixture: 'critic-passing-clean.json' }, + // test_critic FAILS (a red stage the partial stamp would exclude). + { match: matchers.evalStage('test_critic'), fixture: 'critic-failing-with-findings.json' }, + { match: matchers.evalStage('interaction_e2e'), fixture: 'playwright-layer2-completed.json' }, + ]); + + const parent = store.createJob({ + type: 'dev', + status: 'running', + params: { name: 'final-report', prompt: 'verify', report_only: true }, + retryCount: 0, + maxRetries: 3, + }); + + const r = await reportBlockingFinding({ + job_id: parent.id, + finding: 'some bug', + why_it_blocks_report: 'report cannot pass', + recommended_followup: 'fix it', + }); + const childId = parseResult(r).child_job_id as string; + await manager.waitForJob(childId, null, 5_000); + + // Original round (no eval_round, NO expected_eval_stages — legacy shape): + // code FAILS, test FAILS, interaction PASSES. + const c = manager.startJob('critic_eval', { + source_job_id: childId, + eval_stage: 'code_critic', + prompt: 'Code Critic review', + }); + const t = manager.startJob('eval', { + source_job_id: childId, + eval_stage: 'test_critic', + prompt: 'Test Critic review', + }); + const p = manager.startJob('interaction_e2e', { + source_job_id: childId, + eval_stage: 'interaction_e2e', + prompt: 'Interaction E2E eval', + }); + await manager.waitForJob(c.id, null, 5_000); + await manager.waitForJob(t.id, null, 5_000); + await manager.waitForJob(p.id, null, 5_000); + expect(store.getJob(parent.id)?.status).toBe('blocked_on_finding'); + + // Ad-hoc code_critic re-fire with a partial stamp. It PASSES. The gate + // must still require test_critic (red) — the partial stamp must not + // become the consensus when the originals are unstamped. + const adHoc = manager.startJob('critic_eval', { + source_job_id: childId, + eval_stage: 'code_critic', + expected_eval_stages: ['code_critic', 'interaction_e2e'], + prompt: 'Code Critic review — partial-stamp re-fire', + }); + await manager.waitForJob(adHoc.id, null, 5_000); + expect(store.getJob(parent.id)?.status).toBe('blocked_on_finding'); + }); + + it('C21 (round-id): a stamped 1-critic round after a larger round (roster shrink) unblocks', async () => { + // A roster shrink between rounds (a built-in disabled) must be honored: + // the newest stamped singleton's own roster stamp decides, not the older + // round's larger consensus. + const { store, manager, reportBlockingFinding } = createHarness([ + { match: matchers.devJob(), fixture: 'dev-with-changes.md' }, + // R1 code_critic FAILS (served once), R2 code_critic PASSES. + { match: matchers.evalStage('code_critic'), fixture: 'critic-failing-with-findings.json', maxUses: 1 }, + { match: matchers.evalStage('code_critic'), fixture: 'critic-passing-clean.json' }, + { match: matchers.evalStage('test_critic'), fixture: 'test-critic-passing.json' }, + { match: matchers.evalStage('interaction_e2e'), fixture: 'playwright-layer2-completed.json' }, + ]); + + const parent = store.createJob({ + type: 'dev', + status: 'running', + params: { name: 'final-report', prompt: 'verify', report_only: true }, + retryCount: 0, + maxRetries: 3, + }); + + const r = await reportBlockingFinding({ + job_id: parent.id, + finding: 'some bug', + why_it_blocks_report: 'report cannot pass', + recommended_followup: 'fix it', + }); + const childId = parseResult(r).child_job_id as string; + await manager.waitForJob(childId, null, 5_000); + + const stamp = (round: string, stage: string) => { + const promptLabel = stage === 'code_critic' ? 'Code Critic' + : stage === 'test_critic' ? 'Test Critic' + : 'Interaction E2E'; + return { + source_job_id: childId, + eval_stage: stage, + eval_round: round, + expected_eval_stages: ['code_critic', 'test_critic', 'interaction_e2e'], + prompt: `${promptLabel} review — stage: ${stage}`, + }; + }; + + // R1 (stamped, 3-critic roster): code_critic FAILS → blocked. + const r1c = manager.startJob('critic_eval', stamp('round-1', 'code_critic')); + const r1t = manager.startJob('eval', stamp('round-1', 'test_critic')); + const r1p = manager.startJob('interaction_e2e', stamp('round-1', 'interaction_e2e')); + await manager.waitForJob(r1c.id, null, 5_000); + await manager.waitForJob(r1t.id, null, 5_000); + await manager.waitForJob(r1p.id, null, 5_000); + expect(store.getJob(parent.id)?.status).toBe('blocked_on_finding'); + + // R2 (stamped, 1-critic roster after a shrink): code_critic PASSES. The + // newest singleton's own roster stamp must decide, not the older 3-stage + // consensus. + const r2c = manager.startJob('critic_eval', { + source_job_id: childId, + eval_stage: 'code_critic', + eval_round: 'round-2', + expected_eval_stages: ['code_critic'], + prompt: 'Code Critic review — 1-critic roster', + }); + await manager.waitForJob(r2c.id, null, 5_000); + expect(store.getJob(parent.id)?.status).toBe('pending'); + }); + + it('C22 (per-stage): a job-level failed critic with stored passing output cannot unblock', async () => { + // The per-stage fallback's status guard (s.status !== 'completed') is the + // only defense against a failed critic that carried stored passing output + // (setJobOutput runs before the success check). + const { store, manager, reportBlockingFinding } = createHarness([ + { match: matchers.devJob(), fixture: 'dev-with-changes.md' }, + // code_critic FAILS at the JOB level but serves passing output. + { match: matchers.evalStage('code_critic'), fixture: 'critic-passing-clean.json', success: false }, + { match: matchers.evalStage('test_critic'), fixture: 'test-critic-passing.json' }, + { match: matchers.evalStage('interaction_e2e'), fixture: 'playwright-layer2-completed.json' }, + ]); + + const parent = store.createJob({ + type: 'dev', + status: 'running', + params: { name: 'final-report', prompt: 'verify', report_only: true }, + retryCount: 0, + maxRetries: 3, + }); + + const r = await reportBlockingFinding({ + job_id: parent.id, + finding: 'some bug', + why_it_blocks_report: 'report cannot pass', + recommended_followup: 'fix it', + }); + const childId = parseResult(r).child_job_id as string; + await manager.waitForJob(childId, null, 5_000); + + // All-legacy round (no eval_round): code_critic FAILS at the job level + // (stored passing output), test + interaction PASS. + const c = manager.startJob('critic_eval', { + source_job_id: childId, + eval_stage: 'code_critic', + prompt: 'Code Critic review', + }); + const t = manager.startJob('eval', { + source_job_id: childId, + eval_stage: 'test_critic', + prompt: 'Test Critic review', + }); + const p = manager.startJob('interaction_e2e', { + source_job_id: childId, + eval_stage: 'interaction_e2e', + prompt: 'Interaction E2E eval', + }); + await manager.waitForJob(c.id, null, 5_000); + await manager.waitForJob(t.id, null, 5_000); + await manager.waitForJob(p.id, null, 5_000); + + // The failed code_critic's stored passing output must not count — the + // status guard keeps the parent blocked. + expect(store.getJob(parent.id)?.status).toBe('blocked_on_finding'); + }); + + it('C23 (per-stage): a cancelled critic in the fallback gate keeps the parent blocked', async () => { + // The per-stage fallback's status guard (s.status !== 'completed') is the + // only defense against a cancelled critic too — the missing cell in the + // status-guard matrix (C12 covers the round gate, C22 the failed case). + const { store, manager, reportBlockingFinding } = createHarness([ + { match: matchers.devJob(), fixture: 'dev-with-changes.md' }, + { match: matchers.evalStage('code_critic'), fixture: 'critic-passing-clean.json' }, + { match: matchers.evalStage('test_critic'), fixture: 'test-critic-passing.json' }, + { match: matchers.evalStage('interaction_e2e'), fixture: 'playwright-layer2-completed.json' }, + ]); + + const parent = store.createJob({ + type: 'dev', + status: 'running', + params: { name: 'final-report', prompt: 'verify', report_only: true }, + retryCount: 0, + maxRetries: 3, + }); + + const r = await reportBlockingFinding({ + job_id: parent.id, + finding: 'some bug', + why_it_blocks_report: 'report cannot pass', + recommended_followup: 'fix it', + }); + const childId = parseResult(r).child_job_id as string; + await manager.waitForJob(childId, null, 5_000); + + // All-legacy round (no eval_round): cancel test_critic synchronously + // while it is still pending. + const c = manager.startJob('critic_eval', { + source_job_id: childId, + eval_stage: 'code_critic', + prompt: 'Code Critic review', + }); + const t = manager.startJob('eval', { + source_job_id: childId, + eval_stage: 'test_critic', + prompt: 'Test Critic review', + }); + const p = manager.startJob('interaction_e2e', { + source_job_id: childId, + eval_stage: 'interaction_e2e', + prompt: 'Interaction E2E eval', + }); + manager.cancelJob(t.id); + expect(store.getJob(t.id)?.status).toBe('cancelled'); + + // code + interaction pass, but the cancelled test_critic keeps the round + // incomplete — the parent must stay blocked. + await manager.waitForJob(c.id, null, 5_000); + await manager.waitForJob(p.id, null, 5_000); + expect(store.getJob(parent.id)?.status).toBe('blocked_on_finding'); + }); + + it('C24 (per-stage): a partial stamp shared by 2+ ad-hoc re-fires cannot shrink the roster', async () => { + // All-legacy DB with unstamped originals. Two ad-hoc re-fires sharing a + // partial stamp must NOT become the consensus roster (a minority of the + // total siblings) and exclude a red stage. + const { store, manager, reportBlockingFinding } = createHarness([ + { match: matchers.devJob(), fixture: 'dev-with-changes.md' }, + // Original code_critic FAILS (served once), then the ad-hoc re-fires PASS. + { match: matchers.evalStage('code_critic'), fixture: 'critic-failing-with-findings.json', maxUses: 1 }, + { match: matchers.evalStage('code_critic'), fixture: 'critic-passing-clean.json' }, + // test_critic FAILS (a red stage the partial stamp would exclude). + { match: matchers.evalStage('test_critic'), fixture: 'critic-failing-with-findings.json' }, + { match: matchers.evalStage('interaction_e2e'), fixture: 'playwright-layer2-completed.json' }, + ]); + + const parent = store.createJob({ + type: 'dev', + status: 'running', + params: { name: 'final-report', prompt: 'verify', report_only: true }, + retryCount: 0, + maxRetries: 3, + }); + + const r = await reportBlockingFinding({ + job_id: parent.id, + finding: 'some bug', + why_it_blocks_report: 'report cannot pass', + recommended_followup: 'fix it', + }); + const childId = parseResult(r).child_job_id as string; + await manager.waitForJob(childId, null, 5_000); + + // Original round (no eval_round, NO expected_eval_stages — legacy shape): + // code FAILS, test FAILS, interaction PASSES. + const c = manager.startJob('critic_eval', { + source_job_id: childId, + eval_stage: 'code_critic', + prompt: 'Code Critic review', + }); + const t = manager.startJob('eval', { + source_job_id: childId, + eval_stage: 'test_critic', + prompt: 'Test Critic review', + }); + const p = manager.startJob('interaction_e2e', { + source_job_id: childId, + eval_stage: 'interaction_e2e', + prompt: 'Interaction E2E eval', + }); + await manager.waitForJob(c.id, null, 5_000); + await manager.waitForJob(t.id, null, 5_000); + await manager.waitForJob(p.id, null, 5_000); + expect(store.getJob(parent.id)?.status).toBe('blocked_on_finding'); + + // Two ad-hoc re-fires (code + interaction) sharing a partial stamp that + // excludes the red test_critic. Both PASS. The partial stamp is a minority + // of the 5 siblings — it must NOT become the consensus roster. + const partial = ['code_critic', 'interaction_e2e']; + const adHocC = manager.startJob('critic_eval', { + source_job_id: childId, + eval_stage: 'code_critic', + expected_eval_stages: partial, + prompt: 'Code Critic review — partial-stamp re-fire', + }); + const adHocP = manager.startJob('interaction_e2e', { + source_job_id: childId, + eval_stage: 'interaction_e2e', + expected_eval_stages: partial, + prompt: 'Interaction E2E review — partial-stamp re-fire', + }); + await manager.waitForJob(adHocC.id, null, 5_000); + await manager.waitForJob(adHocP.id, null, 5_000); + expect(store.getJob(parent.id)?.status).toBe('blocked_on_finding'); + }); + + it('C25 (round-id): an unstamped red critic created BETWEEN a round\'s critics forces the gate to wait', async () => { + // The round gate keys on the round's START (min createdAt). An unstamped + // ad-hoc created between a stamped round's critics (after the round + // started, before its last critic) is a newer evaluation and must not be + // invisible — otherwise the parent unblocks on the round's stale green. + const { store, manager, reportBlockingFinding } = createHarness([ + { match: matchers.devJob(), fixture: 'dev-with-changes.md' }, + // Round-1 code_critic PASSES (served once), then the ad-hoc re-fire FAILS. + { match: matchers.evalStage('code_critic'), fixture: 'critic-passing-clean.json', maxUses: 1 }, + { match: matchers.evalStage('code_critic'), fixture: 'critic-failing-with-findings.json' }, + { match: matchers.evalStage('test_critic'), fixture: 'test-critic-passing.json' }, + { match: matchers.evalStage('interaction_e2e'), fixture: 'playwright-layer2-completed.json' }, + ]); + + const parent = store.createJob({ + type: 'dev', + status: 'running', + params: { name: 'final-report', prompt: 'verify', report_only: true }, + retryCount: 0, + maxRetries: 3, + }); + + const r = await reportBlockingFinding({ + job_id: parent.id, + finding: 'some bug', + why_it_blocks_report: 'report cannot pass', + recommended_followup: 'fix it', + }); + const childId = parseResult(r).child_job_id as string; + await manager.waitForJob(childId, null, 5_000); + + const stamp = (round: string, stage: string) => { + const promptLabel = stage === 'code_critic' ? 'Code Critic' + : stage === 'test_critic' ? 'Test Critic' + : 'Interaction E2E'; + return { + source_job_id: childId, + eval_stage: stage, + eval_round: round, + expected_eval_stages: ['code_critic', 'test_critic', 'interaction_e2e'], + prompt: `${promptLabel} review — stage: ${stage}`, + }; + }; + + // Round-1 (stamped): all green. Ad-hoc unstamped code_critic re-fire + // (RED) created after the round started. Force distinct created_at so the + // ad-hoc lands BETWEEN the round's critics (round start t, ad-hoc t+1, + // round's last critic t+2). + const r1c = manager.startJob('critic_eval', stamp('round-1', 'code_critic')); + const r1t = manager.startJob('eval', stamp('round-1', 'test_critic')); + const r1p = manager.startJob('interaction_e2e', stamp('round-1', 'interaction_e2e')); + const adHoc = manager.startJob('critic_eval', { + source_job_id: childId, + eval_stage: 'code_critic', + prompt: 'Code Critic review — ad-hoc re-fire', + }); + const db = (store as unknown as { db: { prepare: (sql: string) => { run: (...a: unknown[]) => void } } }).db; + const t0 = store.getJob(r1c.id)?.createdAt ?? Date.now(); + db.prepare('UPDATE jobs SET created_at = ? WHERE id = ?').run(t0, r1c.id); + db.prepare('UPDATE jobs SET created_at = ? WHERE id = ?').run(t0, r1t.id); + db.prepare('UPDATE jobs SET created_at = ? WHERE id = ?').run(t0 + 1, adHoc.id); + db.prepare('UPDATE jobs SET created_at = ? WHERE id = ?').run(t0 + 2, r1p.id); + + await manager.waitForJob(r1c.id, null, 5_000); + await manager.waitForJob(r1t.id, null, 5_000); + await manager.waitForJob(r1p.id, null, 5_000); + await manager.waitForJob(adHoc.id, null, 5_000); + + // The ad-hoc (t+1) is newer than the round's start (t) — the gate must + // wait, not unblock on the round's stale green. + expect(store.getJob(parent.id)?.status).toBe('blocked_on_finding'); + }); }); // ─── D. Layer 2 status surfacing via tenet_get_status ─────────────────────── @@ -371,6 +1788,32 @@ describe('integration: latest_e2e_status surfacing', () => { const parsed = parseResult(r); expect(parsed.latest_e2e_status).toBe('failed'); }); + + it('D4: prose braces after the verdict still surface layer2_status (shared-parser regression)', async () => { + // The old first-{ to last-} slice in tenet_get_status spanned the verdict + // plus the prose braces, JSON.parse failed, and latest_e2e_status was + // silently dropped. The shared parser (rubric.ts) must surface it. + const h = createHarness([ + { match: matchers.evalStage('interaction_e2e'), fixture: 'playwright-layer2-completed-prose-braces.md' }, + ]); + await driveOneE2e(h, []); + const r = await h.getStatus({}); + const parsed = parseResult(r); + expect(parsed.latest_e2e_status).toBe('completed'); + }); + + it('D5: unbalanced { in prose before the verdict still surfaces layer2_status (recovery path)', async () => { + // D4 uses balanced prose braces, so the recovery branch never runs through + // the tool. This fixture has a stray { (truncated code snippet) before the + // verdict — the recovery path must surface layer2_status end-to-end. + const h = createHarness([ + { match: matchers.evalStage('interaction_e2e'), fixture: 'playwright-layer2-completed-unbalanced-brace.md' }, + ]); + await driveOneE2e(h, []); + const r = await h.getStatus({}); + const parsed = parseResult(r); + expect(parsed.latest_e2e_status).toBe('completed'); + }); }); // ─── E. Parser stress tests ───────────────────────────────────────────────── diff --git a/src/core/job-manager.ts b/src/core/job-manager.ts index c1c779e..2e86824 100644 --- a/src/core/job-manager.ts +++ b/src/core/job-manager.ts @@ -12,6 +12,7 @@ import { import { StateStore } from './state-store.js'; import { DEFAULT_EVAL_STAGES } from './critic-roster.js'; import { readArtifactFile, type ArtifactPaths } from './artifact-paths.js'; +import { extractRubricJson } from './rubric.js'; /** * Extract a typed {@link ArtifactPaths} from an untyped `job.params.artifact_paths` @@ -68,53 +69,24 @@ type JobManagerConfig = { const TERMINAL_STATUSES = new Set(['completed', 'failed', 'cancelled']); +/** + * Window for grouping legacy (unstamped) eval critics into "cohorts" in the + * per-stage fallback gate. A full re-evaluation dispatches all critics + * synchronously (ms apart); a single ad-hoc re-fire via tenet_start_job is a + * separate dispatch created later. The newest critic per stage must be created + * within this window of the completing critic, or the round is a partial + * re-evaluation and the gate stays closed. Kept small (1s) to narrow the + * blind spot for re-fires created shortly after the round — a heuristic, since + * the per-stage path has no round ids to distinguish a full re-evaluation from + * a partial re-fire. + */ +const COHORT_WINDOW_MS = 1_000; + const sleep = async (ms: number): Promise => new Promise((resolve) => { setTimeout(resolve, ms); }); -const extractRubricJson = (rawOutput: unknown): Record | null => { - if (rawOutput && typeof rawOutput === 'object') { - return rawOutput as Record; - } - - if (typeof rawOutput !== 'string') { - return null; - } - - const stripped = rawOutput.trim(); - const fenced = stripped.match(/```(?:json)?\s*([\s\S]*?)```/); - const candidates = fenced ? [fenced[1].trim(), stripped] : [stripped]; - - for (const candidate of candidates) { - try { - const parsed = JSON.parse(candidate); - if (parsed && typeof parsed === 'object') { - return parsed as Record; - } - } catch { - // Try next candidate - } - - // Fallback: locate the outermost JSON object substring - const start = candidate.indexOf('{'); - const end = candidate.lastIndexOf('}'); - if (start >= 0 && end > start) { - try { - const sliced = candidate.slice(start, end + 1); - const parsed = JSON.parse(sliced); - if (parsed && typeof parsed === 'object') { - return parsed as Record; - } - } catch { - // Give up - } - } - } - - return null; -}; - export class JobManager { private readonly stateStore: StateStore; private readonly adapterRegistry: AdapterRegistry; @@ -974,15 +946,57 @@ export class JobManager { * project's `.tenet/critics.json` roster — so disabling a built-in or adding a * custom critic shrinks/grows the set). Sibling jobs from before that stamping * existed fall back to the 3 built-ins. + * + * When a source job was evaluated multiple times (re-fired `tenet_start_eval` + * after retries), the stamp shared by the MOST siblings is authoritative — the + * roster at dispatch. A legitimate dispatch stamps every critic identically, + * so a self-serving partial stamp on a single ad-hoc re-fire cannot shrink + * the roster (a disabled built-in or a custom critic shrinks/grows the set + * consistently across all critics of a dispatch). */ private resolveExpectedEvalStages(sourceJobId: string): Set { const siblings = this.stateStore.getEvalsForSource(sourceJobId); + // Use the expected_eval_stages stamp shared by the MOST siblings — the + // roster at dispatch. A legitimate dispatch stamps every critic with the + // same roster (so a disabled built-in or a custom critic shrinks/grows the + // set consistently); a self-serving partial stamp on a single ad-hoc + // re-fire must not override it, or the gate would exclude a red stage and + // unblock on a partial re-evaluation. Falls back to DEFAULT_EVAL_STAGES + // when no stamp is shared. + const counts = new Map(); + let unstampedCount = 0; for (const s of siblings) { const stamped = s.params.expected_eval_stages; - if (Array.isArray(stamped) && stamped.length > 0) { - return new Set(stamped.filter((stage): stage is string => typeof stage === 'string')); + if (!Array.isArray(stamped) || stamped.length === 0) { + unstampedCount++; + continue; + } + const stages = stamped.filter((st): st is string => typeof st === 'string'); + if (stages.length === 0) { + unstampedCount++; + continue; + } + const key = stages.join(','); + const entry = counts.get(key); + if (entry) { + entry.count++; + } else { + counts.set(key, { stages, count: 1 }); } } + let best: { stages: string[]; count: number } | undefined; + for (const entry of counts.values()) { + if (!best || entry.count > best.count) { + best = entry; + } + } + // Adopt the stamp only when it is shared by a MAJORITY of the total + // siblings (or all siblings are stamped). A partial stamp shared by a + // minority of ad-hoc re-fires against unstamped legacy originals must not + // become the roster — fall back to DEFAULT. + if (best && (best.count > siblings.length / 2 || unstampedCount === 0)) { + return new Set(best.stages); + } return new Set(DEFAULT_EVAL_STAGES); } @@ -1015,6 +1029,142 @@ export class JobManager { return; } + const siblings = this.stateStore.getEvalsForSource(sourceJobId); + + // Round-based resume gate. `tenet_start_eval` stamps every critic in one + // dispatch with a shared `eval_round` id. A re-fire (after a child retry) + // gets a fresh id. Only the NEWEST round ever decides the gate — every + // critic in a round evaluated the same source state, so requiring the + // whole round to pass avoids mixing verdicts across code revisions (the + // "green gate, still wrong code" failure). The gate runs whenever at least + // one stamped round exists; unstamped siblings (ad-hoc re-fires via + // tenet_start_job, legacy pre-stamp evals) become singleton rounds inside + // the round gate, so a NEWER unstamped critic forces the gate to wait for + // a fresh stamped round (fail-closed) instead of being invisible — a red + // ad-hoc re-fire must not be ignored while the parent unblocks on an older + // round's stale green. Only when NO sibling is stamped (all-legacy DB) do + // we fall back to per-stage-newest so old stuck parents still recover. + const hasStampedRound = + siblings.length > 0 && + siblings.some((s) => typeof s.params.eval_round === 'string' && s.params.eval_round !== ''); + if (hasStampedRound) { + this.checkBlockingFindingResumeByRound(siblings, blockedParentId, sourceJobId); + return; + } + + // Fallback: per-stage-newest (pre-round-id behavior). + this.checkBlockingFindingResumeByStage(completedJob, siblings, sourceJobId, completedStage, rawOutput, blockedParentId); + } + + private checkBlockingFindingResumeByRound( + siblings: Job[], + blockedParentId: string, + sourceJobId: string, + ): void { + // Group by round id; pick the newest round by its START (min createdAt of + // its critics) — see the selection loop below. + // Unstamped siblings (ad-hoc re-fires via tenet_start_job, legacy pre-stamp + // evals) become singleton rounds keyed by job id. A singleton can never + // satisfy "every expected stage present", so a NEWER unstamped critic + // forces the gate to wait for a fresh stamped round (fail-closed) instead + // of being invisible — otherwise a red ad-hoc re-fire could be ignored + // while the parent unblocks on an older round's stale green. + const byRound = new Map(); + for (const s of siblings) { + const stamped = typeof s.params.eval_round === 'string' && s.params.eval_round !== ''; + const roundId = stamped ? (s.params.eval_round as string) : `__unstamped__${s.id}`; + const arr = byRound.get(roundId) ?? []; + arr.push(s); + byRound.set(roundId, arr); + } + let newestRoundId = ''; + let newestCreatedAt = -1; + for (const [roundId, jobs] of byRound) { + // Key on the round's START (min createdAt), not its max: a round's + // source state is its dispatch time, and an unstamped ad-hoc critic + // created BETWEEN a round's critics (after the round started but before + // its last critic) is a newer evaluation that must force the gate to + // wait — with max-based selection it would be invisible (the round's + // max beats it). + const minCreated = jobs.reduce((m, j) => (j.createdAt < m ? j.createdAt : m), Infinity); + // >= (not >): on a same-ms tie, keep the later-seen round (iteration + // order is createdAt ASC), never the stale one. + if (minCreated >= newestCreatedAt) { + newestCreatedAt = minCreated; + newestRoundId = roundId; + } + } + if (!newestRoundId) return; + const currentRound = byRound.get(newestRoundId) ?? []; + + // Read this round's own expected_eval_stages stamp (shared by all its + // critics). A STAMPED round — multi-critic or singleton — trusts its own + // stamp: a legitimate dispatch stamps every critic with the CURRENT + // roster, so a roster shrink between rounds (a built-in disabled) is + // honored and a 1-critic roster is not stranded against an older round's + // larger roster or the full DEFAULT_EVAL_STAGES. An UNSTAMPED round + // (ad-hoc re-fire) always requires the full DEFAULT_EVAL_STAGES. + // KNOWN LIMITATION: a FORGED stamped round (ad-hoc re-fires via + // tenet_start_job carrying a made-up eval_round + a self-serving partial + // stamp) is trusted outright — defense-in-depth, since a determined caller + // could instead create a full passing round and unblock legitimately. + const isStampedRound = currentRound.some( + (s) => typeof s.params.eval_round === 'string' && s.params.eval_round !== '', + ); + const stamp = isStampedRound + ? currentRound.find((s) => Array.isArray(s.params.expected_eval_stages))?.params.expected_eval_stages + : undefined; + const filteredStages = Array.isArray(stamp) && stamp.length > 0 + ? new Set(stamp.filter((st): st is string => typeof st === 'string')) + : undefined; + // A stamp that filters to an empty set (malformed non-string entries) must + // NOT produce an empty expectedStages — both the stage-presence loop and + // the completion loop would pass trivially and the gate would fail open. + const expectedStages = filteredStages && filteredStages.size > 0 + ? filteredStages + : new Set(DEFAULT_EVAL_STAGES); + + const presentStages = new Set( + currentRound + .map((s) => (typeof s.params.eval_stage === 'string' ? s.params.eval_stage : '')) + .filter((st) => expectedStages.has(st)), + ); + for (const expected of expectedStages) { + if (!presentStages.has(expected)) return; + } + + for (const s of currentRound) { + const stage = typeof s.params.eval_stage === 'string' ? s.params.eval_stage : ''; + if (!expectedStages.has(stage)) continue; + if (s.status !== 'completed') return; + const siblingOutput = this.stateStore.getJobOutput(s.id); + const rawSibling = this.extractAdapterRawOutput(siblingOutput); + const parsed = extractRubricJson(rawSibling); + if (!parsed || parsed.passed !== true) return; + } + + // Newest round fully passed — let the report-only parent run again. + this.stateStore.updateJob(blockedParentId, { + status: 'pending', + startedAt: undefined, + completedAt: undefined, + lastHeartbeat: undefined, + error: undefined, + }); + this.stateStore.appendEvent(blockedParentId, 'blocking_finding_resolved', { + child_job_id: sourceJobId, + eval_round: newestRoundId, + }); + } + + private checkBlockingFindingResumeByStage( + completedJob: Job, + siblings: Job[], + sourceJobId: string, + completedStage: string, + rawOutput: unknown, + blockedParentId: string, + ): void { // Parse this critic's output to confirm it passed const thisCritic = extractRubricJson(rawOutput); if (!thisCritic || thisCritic.passed !== true) { @@ -1026,22 +1176,52 @@ export class JobManager { return; } - const siblings = this.stateStore.getEvalsForSource(sourceJobId); const evalSiblings = siblings.filter((s) => { const stage = typeof s.params.eval_stage === 'string' ? s.params.eval_stage : ''; return expectedStages.has(stage); }); - // Wait until every expected stage has a sibling before deciding — a disabled - // built-in shrinks this set, a custom critic grows it. - const presentStages = new Set(evalSiblings.map((s) => s.params.eval_stage as string)); + // A source job may have been evaluated multiple times (re-fired + // `tenet_start_eval` after retries). Older rounds' critics — including ones + // that failed — are history: only the newest critic per stage decides the + // gate, otherwise a stale failure from an earlier round blocks the parent + // forever even after the current round is fully green. + const latestByStage = new Map(); + for (const s of evalSiblings) { + const stage = typeof s.params.eval_stage === 'string' ? s.params.eval_stage : ''; + const existing = latestByStage.get(stage); + // >= (not >): equal createdAt (same-ms dispatch in tests) keeps the + // later-seen job — never let the stale one win a tie. + if (!existing || s.createdAt >= existing.createdAt) { + latestByStage.set(stage, s); + } + } + const currentRound = [...latestByStage.values()]; + + const presentStages = new Set(currentRound.map((s) => s.params.eval_stage as string)); for (const expected of expectedStages) { if (!presentStages.has(expected)) { return; } } - for (const s of evalSiblings) { + // A single ad-hoc re-fire (tenet_start_job) created long after the other + // stages' critics is a partial re-evaluation, not a full round — it must + // not mask an older red critic for its stage. A full re-evaluation + // dispatches all critics synchronously, so require the newest critic per + // stage to be created within a window of the completing critic. + // KNOWN LIMITATION: a re-fire created >1s after the round dispatch but + // BEFORE the round completes strands the green round (every subsequent + // completion fails the window) — fail-closed, recoverable by a fresh full + // round, but requires manual intervention. + const completingCreatedAt = completedJob.createdAt; + for (const s of currentRound) { + if (Math.abs(s.createdAt - completingCreatedAt) > COHORT_WINDOW_MS) { + return; + } + } + + for (const s of currentRound) { if (s.status !== 'completed') { return; } @@ -1053,7 +1233,6 @@ export class JobManager { } } - // All expected critics passed — let the report-only parent run again with fresh context. this.stateStore.updateJob(blockedParentId, { status: 'pending', startedAt: undefined, diff --git a/src/core/rubric.production.test.ts b/src/core/rubric.production.test.ts new file mode 100644 index 0000000..1fd5aee --- /dev/null +++ b/src/core/rubric.production.test.ts @@ -0,0 +1,45 @@ +import fs from 'node:fs'; +import path from 'node:path'; +import { describe, expect, it } from 'vitest'; +import { extractRubricJson } from './rubric.js'; + +// ─── Production-shape golden test ─────────────────────────────────────────── +// Critic outputs shaped like real production data (prose + verdict, fenced, +// tool echoes, truncated, unmatched-quote-in-prose, raw NDJSON), with the +// ground-truth verdict for each. This is the regression baseline for the +// parser: it ensures any refactor behaves the same on realistic shapes, not +// just on synthetic fixtures. The expected verdicts were established with a +// simple rightmost-object walk (walk { from the end, parse to its matching }, +// keep the first object with a boolean `passed` key, preferring a `stage` key). +// +// case-07 pins the unmatched-quote shape: prose before the verdict contains +// an unmatched double quote that a string-state walk would misread as opening +// a string, false-rejecting a valid verdict. The rightmost-walk handles it. + +type GoldenCase = { + id: string; + type: string; + output: string; + expected: { passed: boolean; stage: string } | null; +}; + +const fixtures: GoldenCase[] = JSON.parse( + fs.readFileSync( + path.resolve(__dirname, '..', '..', 'tests', 'fixtures', 'fake-agents', 'production-critic-outputs.json'), + 'utf8', + ), +); + +describe('extractRubricJson on production critic outputs', () => { + it.each(fixtures.map((f) => [f.id, f] as const))('%s — matches the ground-truth verdict', (_id, f) => { + const got = extractRubricJson(f.output); + const expected = f.expected; + if (expected === null) { + expect(got).toBeNull(); + return; + } + expect(got).not.toBeNull(); + expect(got?.passed).toBe(expected.passed); + expect(got?.stage).toBe(expected.stage); + }); +}); diff --git a/src/core/rubric.test.ts b/src/core/rubric.test.ts new file mode 100644 index 0000000..34deef6 --- /dev/null +++ b/src/core/rubric.test.ts @@ -0,0 +1,464 @@ +import { describe, expect, it } from 'vitest'; +import { extractRubricJson } from './rubric.js'; + +describe('extractRubricJson', () => { + it('returns a bare verdict object as-is', () => { + const parsed = extractRubricJson({ passed: true, stage: 'code_critic', findings: [] }); + expect(parsed?.passed).toBe(true); + }); + + it('parses a bare JSON string verdict', () => { + const parsed = extractRubricJson('{"passed": true, "stage": "code_critic", "findings": []}'); + expect(parsed?.passed).toBe(true); + expect(parsed?.stage).toBe('code_critic'); + }); + + it('parses a fenced verdict block', () => { + const parsed = extractRubricJson('```json\n{"passed": true, "stage": "code_critic", "findings": []}\n```'); + expect(parsed?.passed).toBe(true); + }); + + it('returns null when no object has a passed key', () => { + expect(extractRubricJson('I began my review but ran out of context before finishing.')).toBeNull(); + }); + + it('ignores a trailing object without a passed key', () => { + const output = [ + 'I reviewed the diff. Verdict:', + '{"passed": true, "stage": "code_critic", "findings": []}', + '{"note": "this is a trailing note, not a verdict"}', + ].join('\n'); + const parsed = extractRubricJson(output); + expect(parsed?.passed).toBe(true); + expect(parsed?.stage).toBe('code_critic'); + }); + + it('never picks a nested passed object over the top-level verdict', () => { + // Failing verdict + trailing tool result with nested passed:true — must + // return the FAILING verdict, not false-green the gate. + const t1 = [ + 'V: {"passed": false, "stage": "code_critic", "findings": [{"category":"product_bug","detail":"x"}]}', + 'Tool output: {"results": [{"name": "syntax", "passed": true}]}', + ].join('\n'); + const r1 = extractRubricJson(t1); + expect(r1?.passed).toBe(false); + expect(r1?.stage).toBe('code_critic'); + + // Passing verdict + trailing object with nested passed:false — must + // return the PASSING verdict, not false-strand the parent. + const t2 = [ + 'V: {"passed": true, "stage": "code_critic", "findings": []}', + '{"checks": {"lint": {"passed": false}}}', + ].join('\n'); + const r2 = extractRubricJson(t2); + expect(r2?.passed).toBe(true); + expect(r2?.stage).toBe('code_critic'); + }); + + it('handles the custom-critic shape (nested assertions in the top-level verdict)', () => { + const output = JSON.stringify({ + passed: true, + stage: 'credit_ledger_integrity', + assertions: [ + { name: 'append_only', passed: true, evidence: 'ledger is append-only' }, + { name: 'audit_fields', passed: true, evidence: 'created_at set' }, + ], + findings: [], + }); + const parsed = extractRubricJson(output); + expect(parsed?.passed).toBe(true); + expect(parsed?.stage).toBe('credit_ledger_integrity'); + }); + + it('does NOT return the first fenced object when a later object is the verdict (fenced-echo regression)', () => { + // A critic quotes tool output in a fenced block carrying passed:true, then + // gives a FAILING verdict. The old fenced-first fast path returned the tool + // object and false-greened the gate. + const t1 = [ + 'The tool reported:', + '```json', + '{"tool": "pytest", "passed": true, "count": 12}', + '```', + 'Verdict: {"passed": false, "stage": "code_critic", "findings": [{"category":"product_bug","detail":"x"}]}', + ].join('\n'); + const r1 = extractRubricJson(t1); + expect(r1?.passed).toBe(false); + expect(r1?.stage).toBe('code_critic'); + + // Two fenced blocks, verdict in the second. + const t2 = [ + '```json', + '{"tool": "pytest", "passed": true}', + '```', + '```json', + '{"passed": false, "stage": "code_critic", "findings": []}', + '```', + ].join('\n'); + const r2 = extractRubricJson(t2); + expect(r2?.passed).toBe(false); + expect(r2?.stage).toBe('code_critic'); + }); + + it('prefers a staged verdict over a later stage-less echo (echoed-verdict regression)', () => { + // Failing staged verdict + trailing stage-less tool echo with passed:true. + const t1 = [ + 'V: {"passed": false, "stage": "code_critic", "findings": [{"category":"product_bug","detail":"x"}]}', + 'Tool output: {"passed": true, "tool": "syntax-check"}', + ].join('\n'); + const r1 = extractRubricJson(t1); + expect(r1?.passed).toBe(false); + expect(r1?.stage).toBe('code_critic'); + + // Passing staged verdict + trailing stage-less echo with passed:false. + const t2 = [ + 'V: {"passed": true, "stage": "code_critic", "findings": []}', + 'Tool output: {"passed": false, "tool": "syntax-check"}', + ].join('\n'); + const r2 = extractRubricJson(t2); + expect(r2?.passed).toBe(true); + expect(r2?.stage).toBe('code_critic'); + }); + + it('recovers a verdict after an unbalanced { in prose (unbalanced-brace regression)', () => { + // A stray { in prose before the verdict used to leave a permanent stack + // entry, so the verdict was never top-level and the parent stranded. + const t1 = 'The signature is foo({ and then the verdict: {"passed": true, "stage": "code_critic", "findings": []}'; + const r1 = extractRubricJson(t1); + expect(r1?.passed).toBe(true); + expect(r1?.stage).toBe('code_critic'); + + // Truncated code block before the verdict. + const t2 = 'I checked the diff:\n```ts\nfunction foo() {\n return 1;\n```\nVerdict: {"passed": false, "stage": "code_critic", "findings": ["x"]}'; + const r2 = extractRubricJson(t2); + expect(r2?.passed).toBe(false); + expect(r2?.stage).toBe('code_critic'); + }); + + it('keeps the top-level invariant for a stage-less verdict followed by a nested passed object', () => { + const output = [ + 'V: {"passed": true}', + '{"checks": {"lint": {"passed": false}}}', + ].join('\n'); + const parsed = extractRubricJson(output); + expect(parsed?.passed).toBe(true); + }); + + it('rejects an object wrapped in a top-level array (bracket-depth regression)', () => { + // The scanner tracks {} but must also track [] — an array of verdicts is + // not a verdict object, and the whole-string fast path already rejects it. + const t1 = '[{"passed": true, "stage": "code_critic"}]'; + expect(extractRubricJson(t1)).toBeNull(); + + // A verdict followed by an array of passed objects must keep the verdict. + const t2 = [ + 'V: {"passed": false, "stage": "code_critic", "findings": ["x"]}', + '[{"passed": true, "tool": "syntax-check"}]', + ].join('\n'); + const r2 = extractRubricJson(t2); + expect(r2?.passed).toBe(false); + expect(r2?.stage).toBe('code_critic'); + }); + + it('does not run the brace recovery on balanced output with no top-level verdict', () => { + // The recovery exists for unbalanced braces in prose. On balanced output + // whose only passed object is nested, it must NOT fire and pick up the + // nested object — that would false-strand (or false-green) the gate. + const t1 = '{"checks": {"lint": {"passed": false}}}'; + expect(extractRubricJson(t1)).toBeNull(); + + const t2 = '{"results": [{"name": "syntax", "passed": true}]}'; + expect(extractRubricJson(t2)).toBeNull(); + }); + + it('recovery: a passing tool echo after a failing verdict never wins (stray-brace + echo)', () => { + // A stray { in prose makes the strict scan fail; the recovery must still + // prefer the staged failing verdict over the stage-less passing echo. + const t1 = [ + 'The signature is foo({ and then the verdict:', + '{"passed": false, "stage": "code_critic", "findings": ["x"]}', + 'and the tool said {"results": [{"name": "syntax", "passed": true}]}', + ].join('\n'); + const r1 = extractRubricJson(t1); + expect(r1?.passed).toBe(false); + expect(r1?.stage).toBe('code_critic'); + + const t2 = [ + 'prose { then verdict {"passed": false, "stage": "code_critic", "findings": []}', + 'then tool {"passed": true, "tool": "syntax-check"}', + ].join('\n'); + const r2 = extractRubricJson(t2); + expect(r2?.passed).toBe(false); + expect(r2?.stage).toBe('code_critic'); + }); + + it('recovery: a verdict with nested objects in findings still parses (matching-close)', () => { + // The recovery must slice to the verdict's MATCHING } — the first } after + // the { would slice an unterminated findings array and strand the parent. + const t1 = 'The signature is foo({ and then the verdict: {"passed": false, "stage": "code_critic", "findings": [{"category":"product_bug","detail":"x"}]}'; + const r1 = extractRubricJson(t1); + expect(r1?.passed).toBe(false); + expect(r1?.stage).toBe('code_critic'); + + // A finding object carrying its own passed key must never be picked over + // the staged verdict. + const t2 = 'Note: { and then {"passed": false, "stage": "code_critic", "findings": [{"category":"product_bug","detail":"x","passed": true}]}'; + const r2 = extractRubricJson(t2); + expect(r2?.passed).toBe(false); + expect(r2?.stage).toBe('code_critic'); + }); + + it('recovery: a passing echo BEFORE a stray { cannot mask a failing verdict (merge regression)', () => { + // The strict scan accepts the stage-less echo as bestAny; the stray { + // hides the staged failing verdict. The recovery must still run and its + // staged verdict must win over the echo. + const t1 = [ + 'Tool output: {"passed": true, "tool": "pytest", "count": 12}', + 'The signature is foo({ and then the verdict: {"passed": false, "stage": "code_critic", "findings": ["x"]}', + ].join('\n'); + const r1 = extractRubricJson(t1); + expect(r1?.passed).toBe(false); + expect(r1?.stage).toBe('code_critic'); + + const t2 = '{"passed": true} some prose {unterminated {"passed": false, "stage": "code_critic"}'; + const r2 = extractRubricJson(t2); + expect(r2?.passed).toBe(false); + expect(r2?.stage).toBe('code_critic'); + }); + + it('recovery: a trailing stray { after the verdict does not strand it (continue regression)', () => { + // The recovery walks { from the end; a trailing { with no matching close + // must be skipped, not break the walk before reaching the verdict. + const t1 = 'The signature is foo({ and then the verdict: {"passed": true, "stage": "code_critic", "findings": []} and then more prose {'; + const r1 = extractRubricJson(t1); + expect(r1?.passed).toBe(true); + expect(r1?.stage).toBe('code_critic'); + + const t2 = 'stray { {"passed": true, "stage": "code_critic", "findings": []} more { no close'; + const r2 = extractRubricJson(t2); + expect(r2?.passed).toBe(true); + expect(r2?.stage).toBe('code_critic'); + }); + + it('recovery: two staged objects with a stray brace keep the RIGHTMOST verdict (rightmost-wins regression)', () => { + // The recovery walks { right-to-left but must keep the first accepted + // object per class (the rightmost), not overwrite with the leftmost. + const t1 = [ + '{"passed": true, "stage": "code_critic", "findings": []}', + 'The signature is foo({ and then the verdict:', + '{"passed": false, "stage": "code_critic", "findings": ["x"]}', + ].join('\n'); + const r1 = extractRubricJson(t1); + expect(r1?.passed).toBe(false); + expect(r1?.stage).toBe('code_critic'); + + // Mirrored: failing verdict before the stray brace, passing verdict after. + const t2 = [ + '{"passed": false, "stage": "code_critic", "findings": ["x"]}', + 'The signature is foo({ and then the verdict:', + '{"passed": true, "stage": "code_critic", "findings": []}', + ].join('\n'); + const r2 = extractRubricJson(t2); + expect(r2?.passed).toBe(true); + expect(r2?.stage).toBe('code_critic'); + }); + + it('recovery: escaped quotes and unterminated strings in the verdict still parse (string-state)', () => { + // findMatchingClose must treat \" inside a string as escaped, not a string + // terminator — a regression would close the string early and strand the + // verdict. + const t1 = 'The signature is foo({ and then the verdict: {"passed": false, "stage": "code_critic", "findings": [{"category":"product_bug","detail":"the fix broke \\"login\\""}]}'; + const r1 = extractRubricJson(t1); + expect(r1?.passed).toBe(false); + expect(r1?.stage).toBe('code_critic'); + + // A trailing unterminated string after a green verdict must be skipped. + const t2 = 'stray { {"passed": true, "stage": "code_critic", "findings": []} then {"note": "unterminated'; + const r2 = extractRubricJson(t2); + expect(r2?.passed).toBe(true); + expect(r2?.stage).toBe('code_critic'); + }); + + it('recovery: a nested passed+stage object inside the verdict never wins (top-level guard)', () => { + // The recovery must reject objects nested inside a valid JSON object, even + // when they echo the verdict shape (passed + stage). + const t1 = 'stray { {"passed": false, "detail": {"passed": true, "stage": "code_critic"}}'; + const r1 = extractRubricJson(t1); + expect(r1?.passed).toBe(false); + + const t2 = 'stray { {"passed": false, "stage": "code_critic", "findings": [{"category":"x","detail":"y","passed": true, "stage": "code_critic"}]}'; + const r2 = extractRubricJson(t2); + expect(r2?.passed).toBe(false); + expect(r2?.stage).toBe('code_critic'); + }); + + it('recovery: an array-wrapped staged echo never wins (bracket-depth guard)', () => { + const t1 = 'stray { {"passed": false, "stage": "code_critic", "findings": ["x"]} [{"passed": true, "stage": "code_critic"}]'; + const r1 = extractRubricJson(t1); + expect(r1?.passed).toBe(false); + expect(r1?.stage).toBe('code_critic'); + }); + + it('recovery: a stage-less verdict followed by a nested passed:true echo keeps the verdict', () => { + const t1 = [ + 'I checked the signature foo({ and the verdict:', + '{"passed": false}', + 'Tool output: {"checks": {"lint": {"passed": true}}}', + ].join('\n'); + const r1 = extractRubricJson(t1); + expect(r1?.passed).toBe(false); + }); + + it('recovery: a stray { balanced by a stray } after the verdict does not strand it', () => { + // The stack ends balanced (unbalanced=false) but the verdict is hidden + // behind the stray pair — the recovery must still run and find it. + const t1 = 'The signature is foo({ and the verdict: {"passed": true, "stage": "code_critic", "findings": []} and the closing brace }'; + const r1 = extractRubricJson(t1); + expect(r1?.passed).toBe(true); + expect(r1?.stage).toBe('code_critic'); + }); + + it('recovery: a verdict behind TWO+ stray braces still wins over a passing echo', () => { + // isTopLevelish must accept the verdict behind any number of stray prose + // braces (e.g. a truncated `if (x) { if (y) {` snippet), not just one. + const t1 = [ + 'Tool output: {"passed": true, "tool": "pytest", "count": 12}', + 'The code: if (x) { if (y) { and then the verdict:', + '{"passed": false, "stage": "code_critic", "findings": ["x"]}', + ].join('\n'); + const r1 = extractRubricJson(t1); + expect(r1?.passed).toBe(false); + expect(r1?.stage).toBe('code_critic'); + + // No echo: the verdict behind two stray braces must still be found. + const t2 = 'The code: if (x) { if (y) { and then the verdict: {"passed": true, "stage": "code_critic", "findings": []}'; + const r2 = extractRubricJson(t2); + expect(r2?.passed).toBe(true); + expect(r2?.stage).toBe('code_critic'); + }); + + it('recovery: a stray [ in prose does not strand the verdict (bracket-stray)', () => { + // isTopLevelish must distinguish a stray [ in prose from a genuine array. + const t1 = 'The list was [1, 2, 3 and then the verdict: {"passed": true, "stage": "code_critic", "findings": []}'; + const r1 = extractRubricJson(t1); + expect(r1?.passed).toBe(true); + expect(r1?.stage).toBe('code_critic'); + + // A balanced array before the verdict is fine too. + const t2 = '[1, 2, 3] and then the verdict: {"passed": true, "stage": "code_critic", "findings": []}'; + const r2 = extractRubricJson(t2); + expect(r2?.passed).toBe(true); + }); + + it('a later verdict inside a balanced stray pair IS found (rightmost-wins)', () => { + // The single rightmost-walk has no short-circuit, so a real later verdict + // written inside a stray balanced brace pair is found — the balanced-pair + // false-negative the old two-pass design accepted is gone. + const t1 = 'Verdict: {"passed": false, "stage": "code_critic", "findings": ["x"]} then prose { and the real final verdict {"passed": true, "stage": "code_critic", "findings": []} }'; + const r1 = extractRubricJson(t1); + expect(r1?.passed).toBe(true); + expect(r1?.stage).toBe('code_critic'); + }); + + it('KNOWN LIMITATION: truncated containers and quoted verdicts are accepted', () => { + // The simplified parser deliberately does NOT reject objects nested inside + // a truncated JSON container (a context-limit kill cut it mid-JSON) or a + // JSON object quoted inside a string. Neither shape has been observed in + // production output (the golden test covers the observed shapes), and the + // old inString/truncated guards caused a real regression (an unmatched + // quote in prose false-rejected a valid verdict). Documented as accepted + // trade-offs rather than defended. + const t1 = '{"passed": false, "stage": "code_critic", "findings": ["x"]} Tool output: [{"passed": true, "stage": "code_critic", "findings": []}'; + expect(extractRubricJson(t1)?.passed).toBe(true); + + const t2 = '{"passed": false, "stage": "code_critic", "findings": ["x"]} then tool: {"checks": {"lint": {"passed": true, "stage": "code_critic"}'; + expect(extractRubricJson(t2)?.passed).toBe(true); + + const t3 = 'The tool said "the result was {"passed": true, "stage": "code_critic"}'; + expect(extractRubricJson(t3)?.passed).toBe(true); + }); + + it('recovery: a stage-less echo before a stray brace cannot mask a stage-less verdict (merge order)', () => { + // When neither object is staged, the recovery's rightmost result must win + // over the strict scan's echo (the "verdict at the END" preamble). + const t1 = 'Tool output: {"passed": true, "tool": "syntax-check"} and then a stray { and then the verdict: {"passed": false}'; + const r1 = extractRubricJson(t1); + expect(r1?.passed).toBe(false); + + const t2 = 'Tool output: {"passed": false, "tool": "syntax-check"} and then a stray { and then the verdict: {"passed": true}'; + const r2 = extractRubricJson(t2); + expect(r2?.passed).toBe(true); + }); + + it('recovery: a stage-less echo + balanced stray pair cannot mask a failing verdict', () => { + // The strict scan accepts the echo (bestAny) and the stray pair balances + // (unbalanced=false), so the recovery must still run — a stage-less best + // must not short-circuit the recovery's staged verdict. + const t1 = [ + 'Tool output: {"passed": true, "tool": "pytest", "count": 12}', + 'and prose { and the verdict: {"passed": false, "stage": "code_critic", "findings": ["x"]} }', + ].join('\n'); + const r1 = extractRubricJson(t1); + expect(r1?.passed).toBe(false); + expect(r1?.stage).toBe('code_critic'); + }); + + it('KNOWN LIMITATION: a staged echo (quoted prior verdict) after a failing verdict wins', () => { + // The parser cannot distinguish a real verdict from a quoted earlier + // verdict by shape — both carry passed + stage. The preamble mandates the + // verdict at the END, so a critic that quotes a same-stage verdict after + // its own violates the preamble and the rightmost staged object wins. + // This is a documented limitation, not a regression to fix silently. + const t1 = [ + 'Final: {"passed": false, "stage": "code_critic", "findings": ["x"]}', + '(quoted from round 1: {"passed": true, "stage": "code_critic"})', + ].join('\n'); + const r1 = extractRubricJson(t1); + expect(r1?.passed).toBe(true); + }); +}); + +describe('tenet_get_status surface (extractRubricJson — the production parser)', () => { + it('extracts layer2_status from e2e output with prose braces after the verdict', () => { + const output = [ + 'I explored the UI and ran the scripted checks.', + '{"passed": true, "stage": "interaction_e2e", "layer2_status": "completed", "scripted_results": "all green"}', + 'Note: the fix touched { src/foo.ts } and { src/bar.ts }.', + ].join('\n'); + const parsed = extractRubricJson(output); + expect(parsed?.layer2_status).toBe('completed'); + }); + + it('recovers layer2_status after an unbalanced { in prose', () => { + const output = 'Note: { src/foo.ts and then {"passed": true, "stage": "interaction_e2e", "layer2_status": "completed"}'; + const parsed = extractRubricJson(output); + expect(parsed?.layer2_status).toBe('completed'); + }); + + it('a valid-JSON tool echo after the verdict does not override layer2_status (stage-preference)', () => { + // The old accept-any scan returned the echo object, falsifying or dropping + // layer2_status. The e2e verdict carries stage; the echo does not. + const t1 = [ + '{"passed": true, "stage": "interaction_e2e", "layer2_status": "completed"}', + 'Tool: {"layer2_status": "failed", "tool": "syntax-check"}', + ].join('\n'); + const r1 = extractRubricJson(t1); + expect(r1?.layer2_status).toBe('completed'); + + const t2 = [ + '```json', + '{"passed": true, "stage": "interaction_e2e", "layer2_status": "completed"}', + '```', + 'Then I checked: {"note": "all good"}', + ].join('\n'); + const r2 = extractRubricJson(t2); + expect(r2?.layer2_status).toBe('completed'); + }); + + it('a verdict without a passed key drops layer2_status (fail-closed, matches the gate)', () => { + // tenet_get_status uses extractRubricJson, which requires `passed` — the + // same requirement as the resume gate. A malformed verdict without passed + // drops layer2_status rather than surfacing a value the gate would reject. + const output = '{"layer2_status": "completed", "stage": "interaction_e2e"}'; + expect(extractRubricJson(output)).toBeNull(); + }); +}); diff --git a/src/core/rubric.ts b/src/core/rubric.ts new file mode 100644 index 0000000..3555cc4 --- /dev/null +++ b/src/core/rubric.ts @@ -0,0 +1,193 @@ +/** + * Shared rubric extraction for critic verdicts. + * + * Every critic preamble mandates the verdict shape "End with: {…passed…}" and + * the built-in critics additionally mandate a `stage` key. This module is the + * single parser for that shape, shared by the resume gate (job-manager.ts) and + * the status surface (tenet-get-status.ts) so the two consumers of the same + * stored critic output can never drift apart. + * + * The verdict is the LAST top-level object with a boolean `passed` key. The + * parser walks `{` positions from the end (the preamble mandates the verdict + * at the END), parses each to its matching `}`, and keeps the first accepted + * object per class — preferring one with a `stage` key over a stage-less tool + * echo. An object nested inside a VALID JSON object or array is never the + * verdict. + */ + +/** + * Find the index of the `}` that closes the object opened at `open`, tracking + * nested braces, brackets, and strings. Returns -1 when no matching close + * exists (a stray `{` in prose). + */ +const findMatchingClose = (text: string, open: number): number => { + let depth = 0; + let inString = false; + let escaped = false; + for (let i = open; i < text.length; i++) { + const ch = text[i]; + if (inString) { + if (escaped) { + escaped = false; + } else if (ch === '\\') { + escaped = true; + } else if (ch === '"') { + inString = false; + } + continue; + } + if (ch === '"') { + inString = true; + continue; + } + if (ch === '{' || ch === '[') { + depth++; + } else if (ch === '}' || ch === ']') { + depth--; + if (depth === 0) { + return i; + } + } + } + return -1; +}; + +/** + * True when the object opened at `open` is top-level: not nested inside a + * VALID JSON object or array. Prose braces/brackets around it (a stray `{` + * before the verdict, a truncated container) are fine — that is the recovery + * case the walk exists to handle. Deliberately does NOT track strings while + * scanning the prefix: an unmatched quote in prose (e.g. a substring check + * like `'uv tool install "mkdocs-material'`) must not false-reject a real + * verdict that follows. + * + * KNOWN LIMITATION: an object nested inside a truncated JSON container (a + * context-limit kill cut it mid-JSON) is accepted as top-level — and a JSON + * object quoted inside a string is too. Neither shape has been observed in + * production output (the golden test covers the observed shapes). + */ +const isTopLevelish = (text: string, open: number): boolean => { + const braceStack: number[] = []; + const bracketStack: number[] = []; + for (let i = 0; i < open; i++) { + const ch = text[i]; + if (ch === '{') { + braceStack.push(i); + } else if (ch === '}') { + braceStack.pop(); + } else if (ch === '[') { + bracketStack.push(i); + } else if (ch === ']') { + bracketStack.pop(); + } + } + if (bracketStack.length > 0) { + // Inside an array. Reject if the INNERMOST enclosing bracket forms a + // valid array (genuinely array-wrapped). + const enclosing = bracketStack[bracketStack.length - 1]; + const close = findMatchingClose(text, enclosing); + if (close >= 0) { + try { + JSON.parse(text.slice(enclosing, close + 1)); + return false; + } catch { + // Stray/truncated bracket — accept. + } + } + } + if (braceStack.length > 0) { + // Nested under one or more {. Reject if the INNERMOST enclosing brace + // forms a valid object (a finding, a tool echo); accept if it is a stray + // prose brace (the verdict behind it). + const enclosing = braceStack[braceStack.length - 1]; + const close = findMatchingClose(text, enclosing); + if (close >= 0) { + try { + JSON.parse(text.slice(enclosing, close + 1)); + return false; + } catch { + // Stray/truncated brace — accept. + } + } + } + return true; +}; + +/** + * Rightmost TOP-LEVEL object carrying a boolean `passed` key — the rubric shape + * every critic preamble mandates. Prefers an object that also carries a + * `stage` key (the built-in critics' verdict shape): a tool-result echo like + * `{"passed": true, "tool": "syntax-check"}` rarely carries `stage`, so a + * staged verdict wins over a later stage-less echo — a failing critic that + * pastes a passing tool result after its verdict must not false-green the gate. + */ +export const findRightmostPassedObject = (text: string): Record | null => { + let bestStaged: Record | null = null; + let bestAny: Record | null = null; + let i = text.lastIndexOf('{'); + while (i >= 0) { + const end = findMatchingClose(text, i); + if (end >= 0 && isTopLevelish(text, i)) { + try { + const parsed = JSON.parse(text.slice(i, end + 1)) as unknown; + if (parsed && typeof parsed === 'object' && !Array.isArray(parsed)) { + const record = parsed as Record; + if (typeof record.passed === 'boolean') { + // The walk goes right-to-left, so the FIRST accepted object per + // class is the RIGHTMOST — keep it (only set when null). + if (typeof record.stage === 'string') { + if (!bestStaged) { + bestStaged = record; + } + } else if (!bestAny) { + bestAny = record; + } + } + } + } catch { + // Not valid JSON — prose braces, skip. + } + } + // NOTE: lastIndexOf('{', -1) clamps to 0 and would re-find a `{` at + // position 0 forever, so break explicitly at i === 0. + if (i === 0) { + break; + } + i = text.lastIndexOf('{', i - 1); + } + return bestStaged ?? bestAny; +}; + +/** + * Extract the critic verdict from a worker's raw output. Accepts a bare object + * (already-parsed output), a bare JSON string, or prose containing the verdict + * JSON. Returns null when no passed-bearing object is found. + */ +export const extractRubricJson = (rawOutput: unknown): Record | null => { + if (rawOutput && typeof rawOutput === 'object') { + return rawOutput as Record; + } + + if (typeof rawOutput !== 'string') { + return null; + } + + const stripped = rawOutput.trim(); + + // Whole-output fast path: the output is exactly a verdict object. + try { + const parsed = JSON.parse(stripped) as unknown; + if ( + parsed && + typeof parsed === 'object' && + !Array.isArray(parsed) && + typeof (parsed as Record).passed === 'boolean' + ) { + return parsed as Record; + } + } catch { + // Not a bare JSON object — scan below. + } + + return findRightmostPassedObject(stripped); +}; diff --git a/src/mcp/tools/tenet-get-status.ts b/src/mcp/tools/tenet-get-status.ts index e918632..fef8505 100644 --- a/src/mcp/tools/tenet-get-status.ts +++ b/src/mcp/tools/tenet-get-status.ts @@ -1,5 +1,6 @@ import { z } from 'zod'; import { JobManager } from '../../core/job-manager.js'; +import { extractRubricJson } from '../../core/rubric.js'; import { StateStore } from '../../core/state-store.js'; import { checkForUpdate } from '../../core/update-checker.js'; import type { Job, JobStatus, ProjectStatus } from '../../types/index.js'; @@ -19,32 +20,16 @@ const extractRawOutput = (output: unknown): string | undefined => { const extractJsonObject = (raw: string | undefined): Record | undefined => { if (!raw) return undefined; - const stripped = raw.trim(); - const fenced = stripped.match(/```(?:json)?\s*([\s\S]*?)```/); - const candidates = fenced ? [fenced[1].trim(), stripped] : [stripped]; - for (const candidate of candidates) { - try { - const parsed = JSON.parse(candidate); - if (parsed && typeof parsed === 'object') { - return parsed as Record; - } - } catch { - /* continue */ - } - const start = candidate.indexOf('{'); - const end = candidate.lastIndexOf('}'); - if (start >= 0 && end > start) { - try { - const parsed = JSON.parse(candidate.slice(start, end + 1)); - if (parsed && typeof parsed === 'object') { - return parsed as Record; - } - } catch { - /* continue */ - } - } - } - return undefined; + // The SAME parser the resume gate uses (extractRubricJson) — the e2e verdict + // carries both `passed` and `layer2_status`, so the two consumers select the + // same object from the same stored output. (They still select different JOBS: + // this surface keys on the most recently completed interaction_e2e, while the + // gate keys on the newest round when a stamped round exists, or the newest + // critic per stage in the all-legacy fallback — a slow old-round e2e + // completing late can surface a stale layer2_status.) The old first-{ to + // last-} slice spanned multiple objects/prose and dropped layer2_status + // whenever prose contained braces. + return extractRubricJson(raw) ?? undefined; }; const findLatestE2eStatus = (stateStore: StateStore): string | undefined => { diff --git a/src/mcp/tools/tenet-start-eval.ts b/src/mcp/tools/tenet-start-eval.ts index a5667d6..d2b3f71 100644 --- a/src/mcp/tools/tenet-start-eval.ts +++ b/src/mcp/tools/tenet-start-eval.ts @@ -1,5 +1,6 @@ import fs from 'node:fs'; import path from 'node:path'; +import { randomUUID } from 'node:crypto'; import { z } from 'zod'; import { JobManager } from '../../core/job-manager.js'; import { loadCriticRoster, type ResolvedCritic } from '../../core/critic-roster.js'; @@ -424,9 +425,16 @@ export const registerTenetStartEvalTool = (registerTool: RegisterTool, jobManage // Stages the resume gate waits for = exactly the stages we dispatched. const expectedEvalStages = dispatchList.map((d) => d.evalStage); + // One round id per call so the resume gate can key on "newest complete + // round" rather than "newest critic per stage" — every critic in this + // dispatch evaluated the same source state, and a re-fire (after a child + // retry) gets a fresh id, preventing cross-round verdict mixing. + const evalRound = randomUUID(); + const buildParams = (d: CriticDispatch) => ({ source_job_id: job_id, eval_stage: d.evalStage, + eval_round: evalRound, name: `${d.evalStage} for ${job_id.slice(0, 8)}`, prompt: d.prompt, output: outputObj, diff --git a/tests/fixtures/fake-agents/opencode-ndjson-text-parts.json b/tests/fixtures/fake-agents/opencode-ndjson-text-parts.json new file mode 100644 index 0000000..bd9a2e7 --- /dev/null +++ b/tests/fixtures/fake-agents/opencode-ndjson-text-parts.json @@ -0,0 +1,7 @@ +{"type":"step_start","timestamp":1785417868273,"sessionID":"ses_04ccd6af3ffeN0sAaNmRq1iLOi","part":{"id":"prt_a","messageID":"msg_a","type":"step-start"}} +{"type":"tool_use","timestamp":1785417869000,"sessionID":"ses_04ccd6af3ffeN0sAaNmRq1iLOi","part":{"type":"tool","tool":"bash","callID":"call_1","state":{"status":"completed","input":{"command":"uv run pytest tests/unit -q"},"output":"12 passed","metadata":{"output":"12 passed","exit":0,"truncated":false}},"id":"prt_b","messageID":"msg_a"}} +{"type":"step_finish","timestamp":1785417869500,"sessionID":"ses_04ccd6af3ffeN0sAaNmRq1iLOi","part":{"id":"prt_c","reason":"tool-calls","messageID":"msg_a","type":"step-finish","tokens":{"total":1000,"input":900,"output":100,"cost":0}}} +{"type":"step_start","timestamp":1785417870000,"sessionID":"ses_04ccd6af3ffeN0sAaNmRq1iLOi","part":{"id":"prt_d","messageID":"msg_b","type":"step-start"}} +{"type":"text","timestamp":1785417870300,"sessionID":"ses_04ccd6af3ffeN0sAaNmRq1iLOi","part":{"id":"prt_e1","messageID":"msg_b","type":"text","text":"Running the zero-findings recheck from alternate angles."}} +{"type":"text","timestamp":1785417870500,"sessionID":"ses_04ccd6af3ffeN0sAaNmRq1iLOi","part":{"id":"prt_e2","messageID":"msg_b","type":"text","text":"All 12 tests pass. The fix was already applied in commit `4b8b12f`.\n\n**Verdict**: The finding is resolved. The tests now pass with the credit gate in place.\n\n{\"passed\": true, \"stage\": \"code_critic\", \"findings\": []}"}} +{"type":"step_finish","timestamp":1785417871000,"sessionID":"ses_04ccd6af3ffeN0sAaNmRq1iLOi","part":{"id":"prt_f","reason":"stop","messageID":"msg_b","type":"step-finish","tokens":{"total":1100,"input":990,"output":110,"cost":0}}} diff --git a/tests/fixtures/fake-agents/playwright-layer2-completed-prose-braces.md b/tests/fixtures/fake-agents/playwright-layer2-completed-prose-braces.md new file mode 100644 index 0000000..86f956d --- /dev/null +++ b/tests/fixtures/fake-agents/playwright-layer2-completed-prose-braces.md @@ -0,0 +1,5 @@ +I explored the UI via Playwright MCP. The login flow works end-to-end and the navigation bar links are all reachable. + +{"passed": true, "stage": "interaction_e2e", "layer2_status": "completed", "scripted_results": "42 passed, 0 failed", "exploratory_findings": ["Login flow verified end-to-end"], "screenshots": ["screenshot-login.png"]} + +Note: the fix touched { src/foo.ts } and { src/bar.ts } (see commit 4b8b12f). diff --git a/tests/fixtures/fake-agents/playwright-layer2-completed-unbalanced-brace.md b/tests/fixtures/fake-agents/playwright-layer2-completed-unbalanced-brace.md new file mode 100644 index 0000000..85bff3a --- /dev/null +++ b/tests/fixtures/fake-agents/playwright-layer2-completed-unbalanced-brace.md @@ -0,0 +1,5 @@ +I explored the UI via Playwright MCP. The login flow works end-to-end and the navigation bar links are all reachable. + +Note: the fix touched { src/foo.ts (unclosed brace from a truncated code snippet + +{"passed": true, "stage": "interaction_e2e", "layer2_status": "completed", "scripted_results": "42 passed, 0 failed", "exploratory_findings": ["Login flow verified end-to-end"], "screenshots": ["screenshot-login.png"]} diff --git a/tests/fixtures/fake-agents/production-critic-outputs.json b/tests/fixtures/fake-agents/production-critic-outputs.json new file mode 100644 index 0000000..e6b859c --- /dev/null +++ b/tests/fixtures/fake-agents/production-critic-outputs.json @@ -0,0 +1,74 @@ +[ + { + "id": "case-01", + "type": "critic_eval", + "output": "I reviewed the changes and found no in-scope blockers. The implementation matches the spec.\n\n## Verdict\n\n{\"passed\": true, \"stage\": \"code_critic\", \"findings\": []}", + "expected": { "passed": true, "stage": "code_critic" } + }, + { + "id": "case-02", + "type": "eval", + "output": "No findings. The strengthened regression test covers the previously-missed edge case.\n\n## Test Critic Verdict: PASS\n\n{\"passed\": true, \"stage\": \"test_critic\", \"findings\": [], \"missing_tests\": []}", + "expected": { "passed": true, "stage": "test_critic" } + }, + { + "id": "case-03", + "type": "critic_eval", + "output": "{\"passed\": false, \"stage\": \"code_critic\", \"findings\": [{\"category\": \"product_bug\", \"detail\": \"the retry path does not handle the timeout case\"}]}", + "expected": { "passed": false, "stage": "code_critic" } + }, + { + "id": "case-04", + "type": "critic_eval", + "output": "Findings:\n- `product_bug`: the config loader ignores unknown keys instead of failing\n- `product_bug`: the redaction hook runs after the audit log is written\n\n{\"passed\": false, \"stage\": \"code_critic\", \"findings\": [{\"category\": \"product_bug\", \"detail\": \"config loader ignores unknown keys\"}]}", + "expected": { "passed": false, "stage": "code_critic" } + }, + { + "id": "case-05", + "type": "eval", + "output": "Verdict: FAIL. The regression tests called out in the retry are still missing coverage for the empty-input path.\n\n{\"passed\": false, \"stage\": \"test_critic\", \"findings\": [{\"category\": \"test_bug\", \"detail\": \"empty-input path untested\"}], \"missing_tests\": [\"test_empty_input\"]}", + "expected": { "passed": false, "stage": "test_critic" } + }, + { + "id": "case-06", + "type": "interaction_e2e", + "output": "Surface: `cli` / runtime tests. Harness policy explicitly maps the CLI surface to scripted checks.\n\n{\"passed\": true, \"stage\": \"playwright_eval\", \"surface\": \"cli\", \"layer2_status\": \"completed\", \"scripted_results\": \"all green\", \"exploratory_findings\": [], \"screenshots\": []}", + "expected": { "passed": true, "stage": "playwright_eval" } + }, + { + "id": "case-07", + "type": "critic_eval", + "output": "I have everything I need. Let me record my independent verification before delivering the verdict.\n\n## Code Critic — Purpose Alignment Verdict\n\nI hand-traced all the requirements against the committed config. The install step guard checks `'uv tool install \"the-package' is not present in the run command — the substring check is the regression guard.\n\n### Second-pass (alternate angles, zero-findings rule)\n\n{\"passed\": true, \"stage\": \"code_critic\", \"findings\": []}", + "expected": { "passed": true, "stage": "code_critic" } + }, + { + "id": "case-08", + "type": "eval", + "output": "{\"passed\": true, \"stage\": \"test_critic\", \"findings\": [], \"missing_tests\": []}", + "expected": { "passed": true, "stage": "test_critic" } + }, + { + "id": "case-09", + "type": "interaction_e2e", + "output": "Surface: CLI/scripted with the pilot tests for the TUI. The scripted checks all pass.\n\n{\"passed\": true, \"stage\": \"playwright_eval\", \"surface\": \"cli\", \"layer2_status\": \"completed\", \"scripted_results\": \"all green\", \"exploratory_findings\": [], \"screenshots\": []}", + "expected": { "passed": true, "stage": "playwright_eval" } + }, + { + "id": "case-10", + "type": "interaction_e2e", + "output": "**Eval Result**\nSurface: `cli`. Harness policy: the CLI surface is scripted. The scripted checks fail on the error path.\n\n{\"passed\": false, \"stage\": \"playwright_eval\", \"surface\": \"cli\", \"layer2_status\": \"failed\", \"scripted_results\": \"2 failed\", \"exploratory_findings\": [\"error path not handled\"], \"screenshots\": []}", + "expected": { "passed": false, "stage": "playwright_eval" } + }, + { + "id": "case-11", + "type": "eval", + "output": "The targeted regression test is sufficient for this job scope. The fix is covered.\n\n{\"passed\": true, \"stage\": \"test_critic\", \"findings\": [], \"missing_tests\": []}", + "expected": { "passed": true, "stage": "test_critic" } + }, + { + "id": "case-12", + "type": "eval", + "output": "{\"type\":\"step_start\",\"timestamp\":1783582108303,\"sessionID\":\"ses_000000000000000000000000000000000000\",\"part\":{\"id\":\"prt_a\",\"messageID\":\"msg_a\",\"type\":\"step-start\"}}\n{\"type\":\"text\",\"timestamp\":1783582108400,\"sessionID\":\"ses_000000000000000000000000000000000000\",\"part\":{\"id\":\"prt_b\",\"messageID\":\"msg_b\",\"type\":\"text\",\"text\":\"I reviewed the code.\"}}\n{\"type\":\"step_finish\",\"timestamp\":1783582108500,\"sessionID\":\"ses_000000000000000000000000000000000000\",\"part\":{\"id\":\"prt_c\",\"reason\":\"stop\",\"messageID\":\"msg_b\",\"type\":\"step-finish\"}}", + "expected": null + } +]