The runner builds nothing, so serialising it only added its wait to the critical path; the worktree-isolated fallback is what must follow the lenses, and the harness case now records lens count at the FALLBACK's start alone. setTimeout in the harness is globalThis.setTimeout (the .mjs lint config has ES builtins only). head_sha carries the same description in both scripts and every fixer/implementer prompt asks for the worktree HEAD, not a PR head. The mechanics page says why the cap stays at one after the serialisation and restores the 20%/10% RAM thresholds by key; the record says "several", not "three". Decisions-Edit: yes Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015QqCpYFsKgnAnx6jVwrKiV
193 lines
11 KiB
JavaScript
193 lines
11 KiB
JavaScript
// Pins the review-loop contract of the orchestration Workflow scripts (ersatztv#907, #911):
|
|
// a round carrying any blocking OR should-fix finding goes back to the fixer whatever the lens's
|
|
// verdict word; nits alone end the loop; a round in which every lens failed is an error, never a
|
|
// clean pass; the recorded history binds each fix to the round it answered and names only the
|
|
// commits that fix added; and the post-rebase round applies the same filter.
|
|
//
|
|
// The scripts run inside the Workflow tool, which supplies agent/parallel/phase/log and top-level
|
|
// await/return. This harness compiles each script body as an AsyncFunction with stubbed versions of
|
|
// those, so it executes the COMMITTED control flow rather than a re-description of it.
|
|
import { readFileSync } from 'node:fs';
|
|
import { fileURLToPath } from 'node:url';
|
|
import { dirname, resolve } from 'node:path';
|
|
import { describe, expect, it } from 'vitest';
|
|
|
|
const here = dirname(fileURLToPath(import.meta.url));
|
|
const scriptsDir = resolve(here, '../../.claude/workflows');
|
|
const AsyncFunction = Object.getPrototypeOf(async function () {}).constructor;
|
|
|
|
function compile(name) {
|
|
const src = readFileSync(resolve(scriptsDir, name), 'utf8');
|
|
const body = src.replace(/^export const meta = \{[\s\S]*?\n\}\n/, '');
|
|
return new AsyncFunction('args', 'agent', 'parallel', 'phase', 'log', body);
|
|
}
|
|
|
|
const TRAILER = 'Co-Authored-By: t <t@example>\nClaude-Session: https://example/session';
|
|
const buildArgs = {
|
|
issues: [1], slug: 's', title: 't', body_summary: 'b', done_condition: 'd', files_likely: [],
|
|
area: 'docs', size: 'small', risk: 'routine', needs_e2e: false, port: 8440, avoid: [], trailer: TRAILER,
|
|
};
|
|
const resumeArgs = {
|
|
issues: [1], branch: 'b', wt: '/tmp/wt', pr: 42, mode: 'fix', title: 't', risk: 'routine',
|
|
needs_e2e: false, port: 8440, trailer: TRAILER, brief: '/tmp/brief.json',
|
|
};
|
|
|
|
const IMPL_COMMITS = 'aaaaaaa feat(1): the implementer commit';
|
|
const finding = (severity) => ({ severity, file: 'f', summary: `${severity} finding`, evidence: 'e' });
|
|
|
|
// Every stub lens returns the same findings, so a finding counts once per lens (two lenses → 2).
|
|
// reviewRounds: array indexed by round-1 of {findings, lensesNull?}; landOverrides: extra fields on the finisher report;
|
|
// postRebase: findings returned by the review after a patch-changing rebase, or 'lensesNull'; fixerNull: the fixer agent dies.
|
|
function run(script, args, { reviewRounds, landOverrides = {}, postRebase = null, fixerNull = false, fixerDone = true, fixerNoCommit = false }) {
|
|
const calls = [];
|
|
let round = 0;
|
|
let fixCount = 0;
|
|
let lensesReturned = 0;
|
|
const lensesReturnedWhenFallbackStarted = [];
|
|
const agent = async (prompt, opts) => {
|
|
const label = (opts && opts.label) || '';
|
|
calls.push({ label, prompt, opts });
|
|
if (label.startsWith('impl:') || label.startsWith('fix:#')) return { done: true, summary: '', verified: '', left: '', commits: IMPL_COMMITS, head_sha: 'aaaaaaa' };
|
|
if (label.startsWith('review:codex:')) {
|
|
return { verdict: 'merge', findings: [], ran: false };
|
|
}
|
|
if (label.startsWith('review:fallback:')) {
|
|
lensesReturnedWhenFallbackStarted.push(lensesReturned);
|
|
const spec = reviewRounds[round - 1] || { findings: [] };
|
|
return { verdict: 'merge', findings: spec.findings };
|
|
}
|
|
if (label.startsWith('review:')) {
|
|
const spec = reviewRounds[round - 1] || { findings: [] };
|
|
await new Promise((resolve) => globalThis.setTimeout(resolve, 5));
|
|
lensesReturned += 1;
|
|
if (spec.lensesNull) return null;
|
|
return { verdict: 'merge', findings: spec.findings };
|
|
}
|
|
if (label.startsWith('fix:r')) {
|
|
fixCount += 1;
|
|
if (fixerNull) return null;
|
|
if (!fixerDone) return { done: false, summary: 'stopped', verified: '', left: 'blocked', commits: IMPL_COMMITS };
|
|
return { done: true, summary: '', verified: '', left: '', commits: fixerNoCommit ? IMPL_COMMITS : `f${fixCount}xxxxx fix round ${fixCount}\n${IMPL_COMMITS}`, head_sha: fixerNoCommit ? 'aaaaaaa' : `f${fixCount}xxxxx` };
|
|
}
|
|
if (label.startsWith('land:')) return { done: true, summary: '', verified: '', left: '', commits: '', pr_url: 'http://pr/1', head_sha: 'deadbeef', patch_changed: false, ...landOverrides };
|
|
throw new Error(`unexpected agent label ${label}`);
|
|
};
|
|
const parallel = async (thunks) => {
|
|
round += 1;
|
|
if (postRebase !== null && calls.some((c) => c.label.startsWith('land:'))) {
|
|
return Promise.all(thunks.map(() => Promise.resolve(postRebase === 'lensesNull' ? null : { verdict: 'merge', findings: postRebase })));
|
|
}
|
|
return Promise.all(thunks.map((t) => t().catch(() => null)));
|
|
};
|
|
return script(args, agent, parallel, () => {}, () => {}).then((result) => ({ result, calls, fixCount, lensesReturnedWhenFallbackStarted }));
|
|
}
|
|
|
|
const cases = [
|
|
['ersatztv-issue-build.js', buildArgs],
|
|
['ersatztv-resume-branch.js', resumeArgs],
|
|
];
|
|
|
|
describe.each(cases)('%s review loop', (name, args) => {
|
|
const script = compile(name);
|
|
const finisherPrompt = (calls) => calls.find((c) => c.label.startsWith('land:')).prompt;
|
|
|
|
it('a clean first round lands with zero fix rounds and says so', async () => {
|
|
const { result, calls, fixCount } = await run(script, args, { reviewRounds: [{ findings: [finding('nit')] }] });
|
|
expect(result.error).toBeUndefined();
|
|
expect(fixCount).toBe(0);
|
|
expect(finisherPrompt(calls)).toContain('round 1: 2 lens(es); 0 blocking, 0 should-fix, 2 nit; clean — loop ended');
|
|
expect(finisherPrompt(calls)).toContain('no fix commit exists');
|
|
});
|
|
|
|
it('a should-fix finding sends the round back even though the lens said merge', async () => {
|
|
const { result, calls, fixCount } = await run(script, args, {
|
|
reviewRounds: [{ findings: [finding('should-fix')] }, { findings: [] }],
|
|
});
|
|
expect(result.error).toBeUndefined();
|
|
expect(fixCount).toBe(1);
|
|
const hist = finisherPrompt(calls);
|
|
expect(hist).toContain('round 1: 2 lens(es); 0 blocking, 2 should-fix, 0 nit; answered by the fix commit(s) in git log --oneline aaaaaaa..f1xxxxx');
|
|
expect(hist).toContain('round 2: 2 lens(es); 0 blocking, 0 should-fix, 0 nit; clean — loop ended');
|
|
expect(hist).not.toContain(IMPL_COMMITS);
|
|
expect(hist).toContain('for exactly the commits git log --oneline lists in these ranges: aaaaaaa..f1xxxxx');
|
|
});
|
|
|
|
it('a blocking finding that survives two fix rounds stops before the push', async () => {
|
|
const { result, calls, fixCount } = await run(script, args, {
|
|
reviewRounds: [{ findings: [finding('blocking')] }, { findings: [finding('blocking')] }, { findings: [finding('blocking')] }],
|
|
});
|
|
expect(fixCount).toBe(2);
|
|
expect(result.error).toMatch(/^blocking findings after two fix rounds/);
|
|
expect(calls.some((c) => c.label.startsWith('land:'))).toBe(false);
|
|
});
|
|
|
|
it('a should-fix finding that survives two fix rounds stops before the push', async () => {
|
|
const { result, calls, fixCount } = await run(script, args, {
|
|
reviewRounds: [{ findings: [finding('should-fix')] }, { findings: [finding('should-fix')] }, { findings: [finding('should-fix')] }],
|
|
});
|
|
expect(fixCount).toBe(2);
|
|
expect(result.error).toMatch(/^should-fix findings still open after two fix rounds/);
|
|
expect(calls.some((c) => c.label.startsWith('land:'))).toBe(false);
|
|
});
|
|
|
|
it('a round in which every lens failed is an error, not a clean round', async () => {
|
|
const { result, calls, fixCount } = await run(script, args, { reviewRounds: [{ lensesNull: true }] });
|
|
expect(fixCount).toBe(0);
|
|
expect(result.error).toMatch(/^review round 1 produced no reviews/);
|
|
expect(calls.some((c) => c.label.startsWith('land:'))).toBe(false);
|
|
});
|
|
|
|
it('a round in which every lens failed AFTER a fix round is an error, not a clean round', async () => {
|
|
const { result, calls, fixCount } = await run(script, args, {
|
|
reviewRounds: [{ findings: [finding('should-fix')] }, { lensesNull: true }],
|
|
});
|
|
expect(fixCount).toBe(1);
|
|
expect(result.error).toMatch(/^review round 2 produced no reviews/);
|
|
expect(calls.some((c) => c.label.startsWith('land:'))).toBe(false);
|
|
});
|
|
|
|
it('a fixer that dies is an error, not a clean round', async () => {
|
|
const { result, calls } = await run(script, args, { reviewRounds: [{ findings: [finding('should-fix')] }], fixerNull: true });
|
|
expect(result.error).toMatch(/^fixer for round 1 returned nothing/);
|
|
expect(calls.some((c) => c.label.startsWith('land:'))).toBe(false);
|
|
});
|
|
|
|
it('a fixer that stops (done=false) is an error, not a clean round', async () => {
|
|
const { result, calls } = await run(script, args, { reviewRounds: [{ findings: [finding('should-fix')] }], fixerDone: false });
|
|
expect(result.error).toMatch(/^fixer for round 1 stopped/);
|
|
expect(calls.some((c) => c.label.startsWith('land:'))).toBe(false);
|
|
});
|
|
|
|
it('findings answered without a new commit are described as such, never as a clean round one', async () => {
|
|
const { calls } = await run(script, args, { reviewRounds: [{ findings: [finding('should-fix')] }, { findings: [] }], fixerNoCommit: true });
|
|
const p = finisherPrompt(calls);
|
|
expect(p).toContain('answered without a new commit');
|
|
expect(p).toContain('every finding was answered without a new commit');
|
|
expect(p).not.toContain('round one was clean');
|
|
});
|
|
|
|
it('a post-rebase round in which every lens failed is an error, never a verdict', async () => {
|
|
const { result } = await run(script, args, { reviewRounds: [{ findings: [] }], landOverrides: { patch_changed: true }, postRebase: 'lensesNull' });
|
|
expect(result.error).toMatch(/^the post-rebase review round produced no reviews/);
|
|
});
|
|
|
|
it('on a rubric-class change the worktree-isolated fallback reviewer starts only after both lenses have returned', async () => {
|
|
const rubricArgs = { ...args, risk: 'rubric' };
|
|
const { result, calls, lensesReturnedWhenFallbackStarted } = await run(script, rubricArgs, { reviewRounds: [{ findings: [] }] });
|
|
expect(result.error).toBeUndefined();
|
|
expect(calls.filter((c) => c.label.startsWith('review:codex:')).length).toBe(1);
|
|
expect(calls.filter((c) => c.label.startsWith('review:fallback:')).length).toBe(1);
|
|
expect(lensesReturnedWhenFallbackStarted).toEqual([2]);
|
|
expect(finisherPrompt(calls)).toContain('substituted a cold same-family review-only agent');
|
|
});
|
|
|
|
it('a should-fix on the pushed head after a patch-changing rebase is an error', async () => {
|
|
const { result } = await run(script, args, {
|
|
reviewRounds: [{ findings: [] }],
|
|
landOverrides: { patch_changed: true },
|
|
postRebase: [finding('should-fix')],
|
|
});
|
|
expect(result.error).toMatch(/^blocking or should-fix findings on the pushed head/);
|
|
});
|
|
});
|