diff --git a/.claude/workflows/ersatztv-issue-build.js b/.claude/workflows/ersatztv-issue-build.js index d11b49b42..7f4c6b379 100644 --- a/.claude/workflows/ersatztv-issue-build.js +++ b/.claude/workflows/ersatztv-issue-build.js @@ -60,7 +60,7 @@ const GATE = gateFor(args.port, WT) const REPORT_SCHEMA = { type: 'object', - required: ['done', 'summary', 'verified', 'left', 'commits'], + required: ['done', 'summary', 'verified', 'left', 'commits', 'head_sha'], properties: { done: { type: 'boolean' }, summary: { type: 'string', description: 'what was built, file by file' }, @@ -68,7 +68,7 @@ const REPORT_SCHEMA = { left: { type: 'string', description: 'what is not done and why; what the next agent must know' }, commits: { type: 'string', description: 'git log --oneline origin/main..HEAD' }, pr_url: { type: 'string' }, - head_sha: { type: 'string' }, + head_sha: { type: 'string', description: 'git rev-parse HEAD after your last commit — the finisher derives fix commits from these' }, patch_changed: { type: 'boolean', description: 'finisher only: true if the pre-push rebase changed the patch-id (a conflict resolved or an artifact regenerated)' }, }, } @@ -148,25 +148,28 @@ You are a COLD, review-only substitute for a cross-family reviewer that could no { label: `review:fallback:r${round}`, phase: 'Review', model: 'opus', effort: 'high', isolation: 'worktree', schema: FINDINGS_SCHEMA }) } async function review(round) { - const runs = LENSES.map(l => () => agent(`${reviewCommon(Number(args.port) + 1)} + // The two lenses run together; the cross-family step (the Codex runner, then its worktree-isolated + // fallback) runs AFTER they return. Two worktree-isolated reviewers building .NET at once from one + // slot saturated swap on the 16 GB host (#916), and the fallback is one of them. + const lenses = (await parallel(LENSES.map(l => () => agent(`${reviewCommon(Number(args.port) + 1)} Review round ${round} of the branch for ${REF}. Lens: ${l.prompt} Be adversarial; report only what you verified, with evidence. blocking = done condition or a repo rule violated, or a test that passes for the wrong reason; should-fix = real defect; nit = style. Verdict send-back if any blocking.`, - { label: `review:${l.key}:r${round}`, phase: 'Review', model: l.model, effort: 'high', isolation: l.isolation, schema: FINDINGS_SCHEMA })) - if (rubric) runs.push(() => codexReview(round)) - return parallel(runs) + { label: `review:${l.key}:r${round}`, phase: 'Review', model: l.model, effort: 'high', isolation: l.isolation, schema: FINDINGS_SCHEMA })))).filter(Boolean) + if (!rubric) return lenses + const xf = await codexReview(round) + return xf ? lenses.concat([xf]) : lenses } 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 knownHead = impl.head_sha 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')) let sendBack = actionable(reviews) -const history = [{ round, reviews, fix: null, fix_commits: [] }] +const history = [{ round, reviews, fix: null, fix_range: null }] 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} @@ -180,19 +183,19 @@ 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 + history[history.length - 1].fix_range = fix.head_sha && fix.head_sha !== knownHead ? `${knownHead}..${fix.head_sha}` : null + knownHead = fix.head_sha || knownHead 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')) sendBack = actionable(reviews) - history.push({ round, reviews, fix: null, fix_commits: [] }) + history.push({ round, reviews, fix: null, fix_range: null }) } 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') +const FIX_RANGES = history.map(h => h.fix_range).filter(Boolean) +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_range ? `; answered by the fix commit(s) in git log --oneline ${h.fix_range}` : '; 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} @@ -200,7 +203,7 @@ Then ONE push: git push -u origin ${BRANCH}. Open the PR with the Gitea API (POS <> -${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: +${FIX_RANGES.length ? `followed by what each fix commit changed, read from git show and not from memory, for exactly the commits git log --oneline lists in these ranges: ${FIX_RANGES.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} diff --git a/.claude/workflows/ersatztv-resume-branch.js b/.claude/workflows/ersatztv-resume-branch.js index 946418533..4f205e55a 100644 --- a/.claude/workflows/ersatztv-resume-branch.js +++ b/.claude/workflows/ersatztv-resume-branch.js @@ -46,7 +46,7 @@ const GATE = gateFor(args.port, WT) const REBASE = `Rebase onto origin/main FIRST: git fetch origin; git rebase origin/main; resolve conflicts faithfully, keeping both sides' intent; regenerate generated artifacts rather than hand-resolving them. A commit titled "WIP: orchestrator checkpoint" holds uncommitted work from the paused session and must be folded into the commit it belongs to, never left in history — if it sits directly on that commit: git reset --soft HEAD~1 && git commit --amend --no-edit; otherwise: git commit --fixup= is already its shape, so GIT_SEQUENCE_EDITOR=true git rebase --autosquash ~1 folds it non-interactively.` const REPORT_SCHEMA = { - type: 'object', required: ['done', 'summary', 'verified', 'left', 'commits'], + type: 'object', required: ['done', 'summary', 'verified', 'left', 'commits', 'head_sha'], properties: { done: { type: 'boolean' }, summary: { type: 'string' }, verified: { type: 'string', description: 'exact gate commands run and their real output summary' }, @@ -116,25 +116,28 @@ You are a COLD, review-only substitute for a cross-family reviewer that could no { label: `review:fallback:r${round}`, phase: 'Review', model: 'opus', effort: 'high', isolation: 'worktree', schema: FINDINGS_SCHEMA }) } async function review(round) { - const runs = LENSES.map(l => () => agent(`${reviewCommon(Number(args.port) + 1)} + // The two lenses run together; the cross-family step (the Codex runner, then its worktree-isolated + // fallback) runs AFTER they return. Two worktree-isolated reviewers building .NET at once from one + // slot saturated swap on the 16 GB host (#916), and the fallback is one of them. + const lenses = (await parallel(LENSES.map(l => () => agent(`${reviewCommon(Number(args.port) + 1)} Review round ${round} of the branch for ${REF}. Lens: ${l.prompt} Be adversarial; report only what you verified, with evidence. blocking = done condition or a repo rule violated, or a test that passes for the wrong reason; should-fix = real defect; nit = style. Verdict send-back if any blocking.`, - { label: `review:${l.key}:r${round}`, phase: 'Review', model: l.model, effort: 'high', isolation: l.isolation, schema: FINDINGS_SCHEMA })) - if (rubric) runs.push(() => codexReview(round)) - return parallel(runs) + { label: `review:${l.key}:r${round}`, phase: 'Review', model: l.model, effort: 'high', isolation: l.isolation, schema: FINDINGS_SCHEMA })))).filter(Boolean) + if (!rubric) return lenses + const xf = await codexReview(round) + return xf ? lenses.concat([xf]) : lenses } 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 knownHead = work.head_sha 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')) let sendBack = actionable(reviews) -const history = [{ round, reviews, fix: null, fix_commits: [] }] +const history = [{ round, reviews, fix: null, fix_range: null }] 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} @@ -145,19 +148,19 @@ 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 + history[history.length - 1].fix_range = fix.head_sha && fix.head_sha !== knownHead ? `${knownHead}..${fix.head_sha}` : null + knownHead = fix.head_sha || knownHead 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')) sendBack = actionable(reviews) - history.push({ round, reviews, fix: null, fix_commits: [] }) + history.push({ round, reviews, fix: null, fix_range: null }) } 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') +const FIX_RANGES = history.map(h => h.fix_range).filter(Boolean) +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_range ? `; answered by the fix commit(s) in git log --oneline ${h.fix_range}` : '; 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} @@ -165,7 +168,7 @@ Then git push --force-with-lease origin ${BRANCH}. ${args.pr ? `Update PR #${arg <> -${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: +${FIX_RANGES.length ? `followed by what each fix commit changed, read from git show and not from memory, for exactly the commits git log --oneline lists in these ranges: ${FIX_RANGES.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} diff --git a/docs/handoffs/orchestration.md b/docs/handoffs/orchestration.md index b985a82e2..4282084c8 100644 --- a/docs/handoffs/orchestration.md +++ b/docs/handoffs/orchestration.md @@ -51,10 +51,15 @@ script and returns a path for later runs. - `web/node_modules` is per worktree: clone it from the shared checkout with `cp -Rc` when the lockfiles match, otherwise `npm ci`. The shared copy is kept current by `scripts/refresh-shared-checkout.sh` at session end. -- **Three issues in flight**, not five: every implementer and every worktree-isolated reviewer runs - a `dotnet` build and a web build, and the cap is 3–4 concurrent builds gated on free RAM - (`process.build-concurrency-limits`). The Agent hook enforces the RAM gate for agents launched by - the Agent tool, not for a workflow's agents, so the orchestrator holds the count itself. +- **One .NET-building slot at a time on this host; docs and Python slots may run beside it.** A + slot's correctness reviewer builds .NET in its own worktree, and on a rubric-class change the + cross-family fallback is a second such reviewer, so the review round of ONE .NET slot is two + build-and-test pipelines on top of the implementer's lingering MSBuild node servers. Measured + 2026-09-05 on the 16 GB Mac: three slots in review reached load 82 and swap exhaustion; one slot's + round alone took swap from 6.8 GB to 10.8 GB in three minutes. The scripts therefore run the + cross-family step after the lenses return, never beside them (`process.build-concurrency-limits` + is the standing rule; the Agent hook's RAM gate does not see a workflow's agents, so the + orchestrator holds the count itself). - **Live-E2E runs per worktree on the slot's own port.** `scripts/e2e-local.sh` refuses concurrent runs *within one repo root* because each run re-copies that root's `wwwroot`; across worktrees there is nothing shared but the ports, so every slot gets a distinct `port` argument, one run at a diff --git a/docs/handoffs/orchestrator-prompt.md b/docs/handoffs/orchestrator-prompt.md index 0af2d2d13..66475a6ce 100644 --- a/docs/handoffs/orchestrator-prompt.md +++ b/docs/handoffs/orchestrator-prompt.md @@ -7,10 +7,10 @@ Way of working: - Size every subagent to its task; this is a rule, not a preference, because value per token is what the session is judged on. Picking and refuting: sonnet, medium. Recon on a large issue, implementation, fixing and correctness review: Opus at high (xhigh for a lock, threading or migration fix). Fable is for orchestrating and for the frontier escalations the kickoff lists, never for implementing. Small, well-specified fixes and mechanical finishing (rebase, push, PR body, label clearing): sonnet, medium. Conformance review: sonnet, high. Cross-family review through `codex exec` for the rubric's risk classes (locks, auth, API write paths, migrations, more than ~150 changed C# lines); when Codex cannot run, the workflow substitutes a cold same-family review-only agent and states the substitution in the PR body and its return, and your verdict note repeats it — a substitution is never silent. State the model and effort in every launch and revisit them when an incident degrades a model. - You never pick, claim, code or push yourself. A picker (sonnet, medium; `.claude/workflows/ersatztv-pick-next.js` with the taken list as args) runs `scripts/select-queue.sh`, applies the kickoff's claim and bundle rules to live Gitea state, and two refuters try to overturn it; you accept, or take a refuter's better pick when its rule is right. Then one issue-build workflow per pick, from `.claude/workflows/ersatztv-issue-build.js`, passing the session's commit trailer as `trailer`. - Before picking anything, finish what is already open: every open non-Renovate PR and every `in-progress` issue with a branch is a paused branch, resumed through `.claude/workflows/ersatztv-resume-branch.js` from a JSON brief. -- Keep three issues in flight, each in its own worktree under `~/orca/workspaces/ersatztv/wt-` on branch `-`, cut from a fresh `origin/main` by absolute path, never under `/tmp`, and each with its own E2E port passed as `port`. Refill a slot as soon as one merges. Pass every running issue's files as the avoid list so two agents never edit one file; a pick that needs a file another slot holds waits for that slot. +- Keep at most one .NET-building issue in flight (a docs or Python issue may run beside it), each in its own worktree under `~/orca/workspaces/ersatztv/wt-` on branch `-`, cut from a fresh `origin/main` by absolute path, never under `/tmp`, and each with its own E2E port passed as `port`. Refill a slot as soon as one merges. Pass every running issue's files as the avoid list so two agents never edit one file; a pick that needs a file another slot holds waits for that slot. - Each implementer claims its own issue after the four-way check (open PRs, remote branches, comments predating the label, a fresh fetch), with the `in-progress` label and a claiming comment; rebases onto a fresh `origin/main`, runs the local gate and sits inside the two-lens review loop BEFORE its single push; its finisher opens a PR whose body says `fixes #N` for every issue in the bundle and states the cross-family review status, and posts the `## Closing record` on each issue with the evidence per `## Done-when` box, ticking nothing — a box ticked by the agent it certifies is self-consent. - You referee: read the verdicts and the evidence, send a PR back for anything that lets a route or test pass having done nothing, tick each Done-when box whose evidence holds, re-read the head sha, then post the verdict with `scripts/post-review-verdict.sh MERGEABLE ` and tick the review box. Merge only through the Gitea merge tool with the full head sha, so the consent hook derives consent from the ticked boxes, the sha-bound verdict and green CI. A rebase voids the verdict: the finisher rebases once before the push and reports whether the patch changed; a changed patch gets one more review round before you post. After the merge clear the `in-progress` label and remove the worktree. -- Builds are capped at three to four concurrent and gated on free RAM; live-E2E runs one at a time per worktree on the slot's own port, and a busy port is reported, never taken over. Kill only PIDs you started, gently. A `dotnet test` or E2E run silent for ten minutes is a hang. +- One .NET slot means two build pipelines in its review round (the correctness lens and, on a rubric change, the fallback reviewer, run one after the other); measure `memory_pressure -Q` and swap before every launch; live-E2E runs one at a time per worktree on the slot's own port, and a busy port is reported, never taken over. Kill only PIDs you started, gently. A `dotnet test` or E2E run silent for ten minutes is a hang. - Reviewers never delete anything outside a scratch directory they created under `/private/tmp` and never build a path with `..` segments; the auto-mode classifier is the last line, not the first. - Under an API incident (watch `https://status.claude.com/api/v2/status.json`), move mechanical finishers and probe-driven reviews to whichever model is healthy and park large implementation until it clears; back off before retrying. - Report to the user only what changes what they would do next: merged and closed issues, a send-back and why, a hang, an incident. Before any stop: the H12 audit (`process.issue-qualification-audit`), `scripts/refresh-shared-checkout.sh`, and a handoff memory naming every open branch, its worktree, its PR and its next step, so a resumed session finishes those before picking anything new. diff --git a/web/scripts/orchestration-workflow-loop.test.mjs b/web/scripts/orchestration-workflow-loop.test.mjs index 35bd5936d..606875375 100644 --- a/web/scripts/orchestration-workflow-loop.test.mjs +++ b/web/scripts/orchestration-workflow-loop.test.mjs @@ -38,16 +38,30 @@ const finding = (severity) => ({ severity, file: 'f', summary: `${severity} find // 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 }) { +function run(script, args, { reviewRounds, landOverrides = {}, postRebase = null, fixerNull = false, fixerDone = true, fixerNoCommit = false }) { const calls = []; let round = 0; let fixCount = 0; + let lensesInFlight = 0; + const xfamilyStartedWithLensesInFlight = []; 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('impl:') || label.startsWith('fix:#')) return { done: true, summary: '', verified: '', left: '', commits: IMPL_COMMITS, head_sha: 'aaaaaaa' }; + if (label.startsWith('review:codex:')) { + xfamilyStartedWithLensesInFlight.push(lensesInFlight); + return { verdict: 'merge', findings: [], ran: false }; + } + if (label.startsWith('review:fallback:')) { + xfamilyStartedWithLensesInFlight.push(lensesInFlight); + const spec = reviewRounds[round - 1] || { findings: [] }; + return { verdict: 'merge', findings: spec.findings }; + } if (label.startsWith('review:')) { const spec = reviewRounds[round - 1] || { findings: [] }; + lensesInFlight += 1; + await new Promise((resolve) => setTimeout(resolve, 5)); + lensesInFlight -= 1; if (spec.lensesNull) return null; return { verdict: 'merge', findings: spec.findings }; } @@ -55,7 +69,7 @@ function run(script, args, { reviewRounds, landOverrides = {}, postRebase = null 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}` }; + 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}`); @@ -67,7 +81,7 @@ function run(script, args, { reviewRounds, landOverrides = {}, postRebase = null } return Promise.all(thunks.map((t) => t().catch(() => null))); }; - return script(args, agent, parallel, () => {}, () => {}).then((result) => ({ result, calls, fixCount })); + return script(args, agent, parallel, () => {}, () => {}).then((result) => ({ result, calls, fixCount, xfamilyStartedWithLensesInFlight })); } const cases = [ @@ -94,10 +108,10 @@ describe.each(cases)('%s review loop', (name, args) => { 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 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('what each of these fix commits changed'); + 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 () => { @@ -147,7 +161,7 @@ describe.each(cases)('%s review loop', (name, args) => { }); 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 { 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'); @@ -159,6 +173,16 @@ describe.each(cases)('%s review loop', (name, args) => { expect(result.error).toMatch(/^the post-rebase review round produced no reviews/); }); + it('on a rubric-class change the cross-family step starts only after both lenses have returned', async () => { + const rubricArgs = { ...args, risk: 'rubric' }; + const { result, calls, xfamilyStartedWithLensesInFlight } = 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(xfamilyStartedWithLensesInFlight).toEqual([0, 0]); + 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: [] }],