A round in which every lens returned null read as a clean pass and would now have been quoted verbatim into the PR body; it is an error before the push, in the review loop and in the post-rebase round, which also gains the same blocking-or-should-fix filter; a fixer that dies or stops (no done) is an error too, the same test the implementer already gets. The history entry now carries the fix that answered that round and only the commits that fix added (a line-set difference against the previous branch log — a fixer that reformats or rebases mid-loop defeats it, which is why the finisher is told to read git show, not the list). An empty fix-commit set is described as "answered without a new commit" when a fix round ran, and as "round one was clean" only when none did. web/scripts/orchestration-workflow-loop.test.mjs compiles the committed script bodies with stubbed agent/parallel and pins eleven paths per script (22 tests). Measured: reverting the loop condition to blocking-only reddens six cases per script (every case that needs a should-fix round to reach the fixer); deleting any of the three zero-lens guards, the fixer guard or its done half, or the empty-fix sentence branch reddens its own case, in both scripts. web/vite.config.ts is untouched: it is pinned whole by test_image_build_delegates_the_spa_suite.py, comments included. Decisions-Edit: yes Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015QqCpYFsKgnAnx6jVwrKiV
171 lines
9.2 KiB
JavaScript
171 lines
9.2 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, fixerCommits = null }) {
|
|
const calls = [];
|
|
let round = 0;
|
|
let fixCount = 0;
|
|
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 };
|
|
if (label.startsWith('review:')) {
|
|
const spec = reviewRounds[round - 1] || { findings: [] };
|
|
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: fixerCommits !== null ? fixerCommits : `f${fixCount}xxxxx fix round ${fixCount}\n${IMPL_COMMITS}` };
|
|
}
|
|
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 }));
|
|
}
|
|
|
|
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 fix commit(s): f1xxxxx fix round 1');
|
|
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('what each of these fix commits changed');
|
|
});
|
|
|
|
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: [] }], fixerCommits: IMPL_COMMITS });
|
|
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('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/);
|
|
});
|
|
});
|