fix(911): a should-fix finding sends the round back, a failed-lens round is an error, and a harness executes the loop #912
@@ -158,11 +158,17 @@ Be adversarial; report only what you verified, with evidence. blocking = done co
|
||||
}
|
||||
|
||||
let round = 1
|
||||
const actionable = rs => rs.flatMap(r => r.findings.filter(f => f.severity === 'blocking' || f.severity === 'should-fix'))
|
||||
const countBy = (rs, sev) => rs.flatMap(r => r.findings).filter(f => f.severity === sev).length
|
||||
const newLines = (before, after) => { const seen = new Set(String(before || '').split('\n')); return String(after || '').split('\n').filter(l => l && !seen.has(l)) }
|
||||
let knownCommits = impl.commits
|
||||
let reviews = (await review(round)).filter(Boolean)
|
||||
if (!reviews.length) return { issues, error: `review round ${round} produced no reviews (every lens failed); not pushed`, history: [] }
|
||||
let blocking = reviews.flatMap(r => r.findings.filter(f => f.severity === 'blocking'))
|
||||
const history = [{ round, reviews }]
|
||||
while (blocking.length && round < 3) {
|
||||
log(`${REF} round ${round}: ${blocking.length} blocking, sending back`)
|
||||
let sendBack = actionable(reviews)
|
||||
const history = [{ round, reviews, fix: null, fix_commits: [] }]
|
||||
while (sendBack.length && round < 3) {
|
||||
log(`${REF} round ${round}: ${blocking.length} blocking, ${sendBack.length - blocking.length} should-fix — sending back`)
|
||||
const fix = await agent(`${COMMON}
|
||||
|
||||
${WORKTREE}
|
||||
@@ -172,16 +178,29 @@ ${JSON.stringify(reviews.flatMap(r => r.findings.filter(f => f.severity !== 'nit
|
||||
Then re-run the LOCAL GATE and STOP without pushing; the reviewers read the worktree again. ${GATE}
|
||||
Report.`,
|
||||
{ label: `fix:r${round}`, phase: 'Fix', model: implModel, effort: implEffort, schema: REPORT_SCHEMA })
|
||||
if (!fix || !fix.done) return { issues, error: `fixer for round ${round} ${fix ? 'stopped' : 'returned nothing'}; not pushed`, fix, history }
|
||||
history[history.length - 1].fix = fix
|
||||
history[history.length - 1].fix_commits = newLines(knownCommits, fix.commits)
|
||||
knownCommits = fix.commits || knownCommits
|
||||
round++
|
||||
reviews = (await review(round)).filter(Boolean)
|
||||
if (!reviews.length) return { issues, error: `review round ${round} produced no reviews (every lens failed); not pushed`, history }
|
||||
blocking = reviews.flatMap(r => r.findings.filter(f => f.severity === 'blocking'))
|
||||
history.push({ round, fix, reviews })
|
||||
sendBack = actionable(reviews)
|
||||
history.push({ round, reviews, fix: null, fix_commits: [] })
|
||||
}
|
||||
if (blocking.length) return { issues, error: 'blocking findings after two fix rounds; not pushed', blocking_remaining: blocking, history }
|
||||
if (sendBack.length) return { issues, error: 'should-fix findings still open after two fix rounds; not pushed — the orchestrator decides', should_fix_remaining: sendBack, history }
|
||||
const FIX_COMMITS = history.flatMap(h => h.fix_commits)
|
||||
const REVIEW_HISTORY = history.map(h => `round ${h.round}: ${h.reviews.length} lens(es); ${countBy(h.reviews, 'blocking')} blocking, ${countBy(h.reviews, 'should-fix')} should-fix, ${countBy(h.reviews, 'nit')} nit` + (h.fix ? (h.fix_commits.length ? `; answered by fix commit(s): ${h.fix_commits.join(' | ')}` : '; answered without a new commit (findings refuted with evidence in the fixer report)') : '; clean — loop ended')).join('\n')
|
||||
|
||||
phase('Land')
|
||||
const FINISH = `FINISH, in this order. Record the patch-id first: git diff $(git merge-base origin/main HEAD)..HEAD | git patch-id --stable. Then git fetch origin; if origin/main moved, rebase onto it (never merge main in; regenerate, never hand-resolve, generated artifacts — the decisions catalog by its generator), re-run the LOCAL GATE, and recompute the patch-id: report patch_changed=true if it differs. ${GATE}
|
||||
Then ONE push: git push -u origin ${BRANCH}. Open the PR with the Gitea API (POST ${API}/pulls; head=${BRANCH}, base=main, title, body). The body must contain "fixes #N" for every issue in the bundle so the merge closes them, the root cause for a bug fix, the measured numbers, what the reviewers found across ${round} round(s) and how it was answered, the cross-family review status verbatim — "${xfamily}" — and every deliberately-left item with an issue number (file follow-up issues where needed). End the body with:
|
||||
Then ONE push: git push -u origin ${BRANCH}. Open the PR with the Gitea API (POST ${API}/pulls; head=${BRANCH}, base=main, title, body). The body must contain "fixes #N" for every issue in the bundle so the merge closes them, the root cause for a bug fix, the measured numbers, the review history VERBATIM as recorded by the workflow, one line per round, between the markers <<REVIEW HISTORY and REVIEW HISTORY>>:
|
||||
<<REVIEW HISTORY
|
||||
${REVIEW_HISTORY}
|
||||
REVIEW HISTORY>>
|
||||
${FIX_COMMITS.length ? `followed by what each of these fix commits changed, read from git show and not from memory: ${FIX_COMMITS.join(' | ')}` : (history.some(h => h.fix) ? 'and a sentence saying every finding was answered without a new commit, as the history block records' : 'and a sentence saying no fix commit exists because round one was clean')}, then the cross-family review status verbatim — "${xfamily}" — and every deliberately-left item with an issue number (file follow-up issues where needed). End the body with:
|
||||
🤖 Generated with [Claude Code](https://claude.com/claude-code)
|
||||
|
||||
${SESSION_URL}
|
||||
@@ -200,7 +219,8 @@ if (land.patch_changed) {
|
||||
log(`${REF}: patch changed on rebase — one more review round on the pushed head before any verdict`)
|
||||
round++
|
||||
post_rebase_reviews = (await review(round)).filter(Boolean)
|
||||
const late = post_rebase_reviews.flatMap(r => r.findings.filter(f => f.severity === 'blocking'))
|
||||
if (late.length) return { issues, error: 'blocking findings on the pushed head after the pre-push rebase; no verdict may be posted', pr_url: land.pr_url, head_sha: land.head_sha, blocking_remaining: late, cross_family: xfamily, history, post_rebase_reviews }
|
||||
if (!post_rebase_reviews.length) return { issues, error: 'the post-rebase review round produced no reviews (every lens failed); pushed, no verdict may be posted', pr_url: land.pr_url, head_sha: land.head_sha, cross_family: xfamily, history }
|
||||
const late = actionable(post_rebase_reviews)
|
||||
if (late.length) return { issues, error: 'blocking or should-fix findings on the pushed head after the pre-push rebase; no verdict may be posted', pr_url: land.pr_url, head_sha: land.head_sha, findings_remaining: late, cross_family: xfamily, history, post_rebase_reviews }
|
||||
}
|
||||
return { issues, pr_url: land.pr_url, head_sha: land.head_sha, patch_changed: !!land.patch_changed, cross_family: xfamily, impl, land, history, post_rebase_reviews }
|
||||
|
||||
@@ -95,7 +95,7 @@ const reviewCommon = (e2ePort) => `${COMMON}
|
||||
|
||||
${gateFor(e2ePort, 'your own isolated worktree (never ' + WT + ')')}
|
||||
|
||||
Diff: git -C ${WT} diff origin/main...HEAD (rebased, not yet pushed). Read-only except scratch you create under /private/tmp; do not commit or push. NEVER run rm -rf, git worktree remove, git branch -D or any delete outside a directory you created under /private/tmp this session, and never build a path with .. segments. If you must build or test, do it in your own isolated worktree, never in ${WT}: git fetch ${WT} ${BRANCH} && git checkout --detach FETCH_HEAD puts the branch there; gates sequentially; E2E there on port ${e2ePort} (the GATE above is written for your worktree and that port).`
|
||||
Diff: git -C ${WT} diff origin/main...HEAD (rebased, not yet pushed). ${args.pr ? `PR #${args.pr} exists: its pushed head, its body and any earlier closing record are INTENTIONALLY behind this worktree until the finisher pushes after this review loop and resyncs them — a stale PR head or body is not a finding, and neither is "not pushed".` : 'No PR exists yet; the finisher opens it after this loop.'} Read-only except scratch you create under /private/tmp; do not commit or push. NEVER run rm -rf, git worktree remove, git branch -D or any delete outside a directory you created under /private/tmp this session, and never build a path with .. segments. If you must build or test, do it in your own isolated worktree, never in ${WT}: git fetch ${WT} ${BRANCH} && git checkout --detach FETCH_HEAD puts the branch there; gates sequentially; E2E there on port ${e2ePort} (the GATE above is written for your worktree and that port).`
|
||||
|
||||
const LENSES = [
|
||||
{ key: 'correctness', model: 'opus', isolation: 'worktree', prompt: 'correctness against the done condition: run the gate and, for a write path or screen, the live-E2E route yourself, and read the output; try to break the change with the edge cases the issue and the docs name; check the pinning test reddens when the fix alone is reverted.' },
|
||||
@@ -126,27 +126,46 @@ Be adversarial; report only what you verified, with evidence. blocking = done co
|
||||
}
|
||||
|
||||
let round = 1
|
||||
const actionable = rs => rs.flatMap(r => r.findings.filter(f => f.severity === 'blocking' || f.severity === 'should-fix'))
|
||||
const countBy = (rs, sev) => rs.flatMap(r => r.findings).filter(f => f.severity === sev).length
|
||||
const newLines = (before, after) => { const seen = new Set(String(before || '').split('\n')); return String(after || '').split('\n').filter(l => l && !seen.has(l)) }
|
||||
let knownCommits = work.commits
|
||||
let reviews = (await review(round)).filter(Boolean)
|
||||
if (!reviews.length) return { issues, error: `review round ${round} produced no reviews (every lens failed); not pushed`, history: [] }
|
||||
let blocking = reviews.flatMap(r => r.findings.filter(f => f.severity === 'blocking'))
|
||||
const history = [{ round, reviews }]
|
||||
while (blocking.length && round < 3) {
|
||||
log(`${REF} round ${round}: ${blocking.length} blocking, sending back`)
|
||||
let sendBack = actionable(reviews)
|
||||
const history = [{ round, reviews, fix: null, fix_commits: [] }]
|
||||
while (sendBack.length && round < 3) {
|
||||
log(`${REF} round ${round}: ${blocking.length} blocking, ${sendBack.length - blocking.length} should-fix — sending back`)
|
||||
const fix = await agent(`${COMMON}
|
||||
|
||||
You are the fixer. Reviewers found these problems in the unpushed, rebased branch; fix every blocking and should-fix one as new commits, or show with evidence why a finding is wrong:
|
||||
${JSON.stringify(reviews.flatMap(r => r.findings.filter(f => f.severity !== 'nit')), null, 1)}
|
||||
Re-run the LOCAL GATE and STOP without pushing. ${GATE}`,
|
||||
{ label: `fix:r${round}`, phase: 'Fix', model: 'opus', effort: 'high', schema: REPORT_SCHEMA })
|
||||
if (!fix || !fix.done) return { issues, error: `fixer for round ${round} ${fix ? 'stopped' : 'returned nothing'}; not pushed`, fix, history }
|
||||
history[history.length - 1].fix = fix
|
||||
history[history.length - 1].fix_commits = newLines(knownCommits, fix.commits)
|
||||
knownCommits = fix.commits || knownCommits
|
||||
round++
|
||||
reviews = (await review(round)).filter(Boolean)
|
||||
if (!reviews.length) return { issues, error: `review round ${round} produced no reviews (every lens failed); not pushed`, history }
|
||||
blocking = reviews.flatMap(r => r.findings.filter(f => f.severity === 'blocking'))
|
||||
history.push({ round, fix, reviews })
|
||||
sendBack = actionable(reviews)
|
||||
history.push({ round, reviews, fix: null, fix_commits: [] })
|
||||
}
|
||||
if (blocking.length) return { issues, error: 'blocking findings after two fix rounds; not pushed', blocking_remaining: blocking, history }
|
||||
if (sendBack.length) return { issues, error: 'should-fix findings still open after two fix rounds; not pushed — the orchestrator decides', should_fix_remaining: sendBack, history }
|
||||
const FIX_COMMITS = history.flatMap(h => h.fix_commits)
|
||||
const REVIEW_HISTORY = history.map(h => `round ${h.round}: ${h.reviews.length} lens(es); ${countBy(h.reviews, 'blocking')} blocking, ${countBy(h.reviews, 'should-fix')} should-fix, ${countBy(h.reviews, 'nit')} nit` + (h.fix ? (h.fix_commits.length ? `; answered by fix commit(s): ${h.fix_commits.join(' | ')}` : '; answered without a new commit (findings refuted with evidence in the fixer report)') : '; clean — loop ended')).join('\n')
|
||||
|
||||
phase('Land')
|
||||
const FINISH = `FINISH: record the patch-id (git diff $(git merge-base origin/main HEAD)..HEAD | git patch-id --stable); git fetch origin; if origin/main moved again, rebase onto it, re-run the LOCAL GATE, and recompute the patch-id — report patch_changed=true if it differs. ${GATE}
|
||||
Then git push --force-with-lease origin ${BRANCH}. ${args.pr ? `Update PR #${args.pr}'s body (PATCH ${API}/pulls/${args.pr}) so it describes the branch as it now is` : `Open a PR (POST ${API}/pulls; head=${BRANCH}, base=main)`}: the body must contain "fixes #N" for every issue in the bundle, the root cause for a bug fix, the measured numbers, what the reviewers found across ${round} round(s) and how it was answered, the cross-family review status verbatim — "${xfamily}" — every deliberately-left item with an issue number, and end with:
|
||||
Then git push --force-with-lease origin ${BRANCH}. ${args.pr ? `Update PR #${args.pr}'s body (PATCH ${API}/pulls/${args.pr}) so it describes the branch as it now is` : `Open a PR (POST ${API}/pulls; head=${BRANCH}, base=main)`}: the body must contain "fixes #N" for every issue in the bundle, the root cause for a bug fix, the measured numbers, the review history VERBATIM as recorded by the workflow, one line per round, between the markers <<REVIEW HISTORY and REVIEW HISTORY>>:
|
||||
<<REVIEW HISTORY
|
||||
${REVIEW_HISTORY}
|
||||
REVIEW HISTORY>>
|
||||
${FIX_COMMITS.length ? `followed by what each of these fix commits changed, read from git show and not from memory: ${FIX_COMMITS.join(' | ')}` : (history.some(h => h.fix) ? 'and a sentence saying every finding was answered without a new commit, as the history block records' : 'and a sentence saying no fix commit exists because round one was clean')}, then the cross-family review status verbatim — "${xfamily}" — every deliberately-left item with an issue number, and end with:
|
||||
🤖 Generated with [Claude Code](https://claude.com/claude-code)
|
||||
|
||||
${SESSION_URL}
|
||||
@@ -163,7 +182,8 @@ if (land.patch_changed) {
|
||||
log(`${REF}: patch changed on rebase — one more review round on the pushed head before any verdict`)
|
||||
round++
|
||||
post_rebase_reviews = (await review(round)).filter(Boolean)
|
||||
const late = post_rebase_reviews.flatMap(r => r.findings.filter(f => f.severity === 'blocking'))
|
||||
if (late.length) return { issues, error: 'blocking findings on the pushed head after the pre-push rebase; no verdict may be posted', pr_url: land.pr_url || args.pr, head_sha: land.head_sha, blocking_remaining: late, cross_family: xfamily, history, post_rebase_reviews }
|
||||
if (!post_rebase_reviews.length) return { issues, error: 'the post-rebase review round produced no reviews (every lens failed); pushed, no verdict may be posted', pr_url: land.pr_url || args.pr, head_sha: land.head_sha, cross_family: xfamily, history }
|
||||
const late = actionable(post_rebase_reviews)
|
||||
if (late.length) return { issues, error: 'blocking or should-fix findings on the pushed head after the pre-push rebase; no verdict may be posted', pr_url: land.pr_url || args.pr, head_sha: land.head_sha, findings_remaining: late, cross_family: xfamily, history, post_rebase_reviews }
|
||||
}
|
||||
return { issues, pr_url: land.pr_url || args.pr, head_sha: land.head_sha, patch_changed: !!land.patch_changed, cross_family: xfamily, work, land, history, post_rebase_reviews }
|
||||
|
||||
@@ -41,7 +41,7 @@ context, which gives the login specs a signed-out browser without a logout dance
|
||||
CI — a retry lets a flaky flow merge looking green. Coupling worth knowing: vitest's default `include`
|
||||
glob would run `web/e2e/*.spec.ts` under jsdom, so `vite.config.ts` excludes `e2e/**` by spreading
|
||||
`configDefaults.exclude` — not by narrowing `include` to `src/**`, which would silently stop collecting
|
||||
the real vitest test under `web/scripts/`.
|
||||
the real vitest tests under `web/scripts/`.
|
||||
|
||||
**Lifecycle correctness in `scripts/e2e-ui.sh`** — all four found by adversarial review, **none by a
|
||||
passing run**; that is the transferable lesson (green runs never exercise the failure/interrupt paths).
|
||||
|
||||
@@ -79,20 +79,28 @@ the review loop *inside* the worktree, before the single push:
|
||||
touched `.cs` (`process.bom-format-detection-recipe`), and live-E2E on the slot's port for a
|
||||
write path or UI change (`testing.live-e2e-prepush-timing`).
|
||||
2. **Reviewers** read the worktree diff (`git diff origin/main...HEAD`) and run the gate in their
|
||||
own worktrees; the fixer answers `blocking` and `should-fix` findings; the loop ends on a clean
|
||||
round, never on "round one's findings are fixed".
|
||||
own worktrees. A round carrying any `blocking` or `should-fix` finding goes back to the fixer
|
||||
regardless of the lens's own verdict word (a `should-fix` is a real defect by definition); nits
|
||||
alone end the loop. A round in which every lens failed is an error, never a clean round. After
|
||||
two fix rounds an open finding stops the workflow before the push and the orchestrator decides.
|
||||
The same filter applies to the post-rebase round on the pushed head. In a resume, the existing
|
||||
PR's stale head and body are the expected state until the finisher pushes, never a finding. The
|
||||
loop ends on a clean round, never on "round one's findings are fixed".
|
||||
3. **Finisher**: fetch; if `origin/main` moved, rebase, re-run the gate, and compare the patch-id
|
||||
before and after — a changed patch (a conflict resolved, an artifact regenerated) sends the
|
||||
branch through one more review round before the verdict, because the reviewed tree must be the
|
||||
pushed tree. Then **one push**, `git push -u origin <branch>`; open the PR with `fixes #N` for
|
||||
every issue in the bundle, the cross-family review status, and the session trailers; arm the CI
|
||||
every issue in the bundle, the review history the workflow recorded (one line per round, quoted
|
||||
verbatim — the finisher never describes a fix it did not see), the cross-family review status,
|
||||
and the session trailers; arm the CI
|
||||
monitor on the head sha; post the `## Closing record` on each issue with the evidence for every
|
||||
`## Done-when` box, ticking none.
|
||||
4. **Orchestrator**: read the review evidence, not the summaries. Send the PR back for anything
|
||||
that lets a route or test pass having done nothing. Tick each box whose evidence holds, then
|
||||
re-read the head sha immediately before posting `scripts/post-review-verdict.sh <pr> MERGEABLE
|
||||
<note>` naming the reviewers, the rounds and the workflow's returned `cross_family` status (a
|
||||
Codex failure in a post-rebase round is in that return, not in the PR body); tick the review box. Wait for CI — a `cancelled` job
|
||||
Codex failure in a post-rebase round is in that return, not in the PR body); tick the review
|
||||
box. Wait for CI — a `cancelled` job
|
||||
reads as `failure` at the combined status endpoint, so resolve it via the run's jobs
|
||||
(`ci.cancelled-is-not-a-verdict`).
|
||||
5. **Merge through the Gitea merge tool with the full head sha**; the consent hook derives consent
|
||||
|
||||
@@ -0,0 +1,170 @@
|
||||
// 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/);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user