From f7a0da4051210b37203e885f94b0dd2b5a9e6b40 Mon Sep 17 00:00:00 2001 From: Timothy Date: Sat, 5 Sep 2026 12:07:17 +0200 Subject: [PATCH 1/5] fix(916): the cross-family review step follows the lenses instead of running beside them, and fix commits are a sha range MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit One .NET slot's review round ran two worktree-isolated reviewers at once — the correctness lens and, on a rubric change, the Codex fallback — and took swap from 6.8 GB to 10.8 GB in three minutes on the 16 GB host; three slots reached load 82. review() now awaits the lenses, then the Codex runner, then its fallback. The finisher's fix attribution is the sha range the fixer's report head advances (head_sha is required on every report), replacing a line-set difference over free text that listed all eleven #563 commits as fixes. The harness gains a case that records how many lenses were still in flight when the cross-family agents started (must be zero); moving the fallback back into the parallel batch reddens it in both scripts. The mechanics page and the standing prompt state the measured cap: one .NET-building slot at a time. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_015QqCpYFsKgnAnx6jVwrKiV --- .claude/workflows/ersatztv-issue-build.js | 33 ++++++++-------- .claude/workflows/ersatztv-resume-branch.js | 31 ++++++++------- docs/handoffs/orchestration.md | 13 +++++-- docs/handoffs/orchestrator-prompt.md | 4 +- .../orchestration-workflow-loop.test.mjs | 38 +++++++++++++++---- 5 files changed, 77 insertions(+), 42 deletions(-) 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: [] }], -- 2.47.3 From 929dff835a0bda0bb832356ee131efce500b0a9c Mon Sep 17 00:00:00 2001 From: Timothy Date: Sat, 5 Sep 2026 12:42:36 +0200 Subject: [PATCH 2/5] fix(916): the Codex runner runs beside the lenses, only the fallback waits; head_sha described everywhere; the docs keep their thresholds 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 Claude-Session: https://claude.ai/code/session_015QqCpYFsKgnAnx6jVwrKiV --- .claude/workflows/ersatztv-issue-build.js | 25 ++++++++++------- .claude/workflows/ersatztv-resume-branch.js | 27 +++++++++++-------- .../records/process/orchestrated-session.md | 3 +-- docs/handoffs/orchestration.md | 16 ++++++----- docs/handoffs/orchestrator-prompt.md | 2 +- .../orchestration-workflow-loop.test.mjs | 20 +++++++------- 6 files changed, 52 insertions(+), 41 deletions(-) diff --git a/.claude/workflows/ersatztv-issue-build.js b/.claude/workflows/ersatztv-issue-build.js index 7f4c6b379..b5fa31668 100644 --- a/.claude/workflows/ersatztv-issue-build.js +++ b/.claude/workflows/ersatztv-issue-build.js @@ -118,7 +118,7 @@ ${CLAIM} ${recon ? `Recon (verify what you rely on):\nPLAN: ${recon.plan}\nFACTS: ${recon.facts}\nRISKS: ${recon.risks}\nTEST PLAN: ${recon.test_plan}\n` : ''} You are the implementer. Close ${REF} completely: pin the behaviour with tests named for the branch they protect, update the docs the change obligates, commit. Then git fetch origin and rebase onto origin/main if it moved (never merge main in; regenerate generated artifacts), run the LOCAL GATE and STOP — do not push; reviewers read your worktree first, and a finisher pushes once after the review loop is clean. ${GATE} -Report done=true with the gate output when the worktree is ready for review, with pr_url empty.`, +Report done=true with the gate output when the worktree is ready for review, with pr_url empty and head_sha = git rev-parse HEAD.`, { label: `impl:${REF}`, model: implModel, effort: implEffort, schema: REPORT_SCHEMA }) if (!impl) return { issues, error: 'implementer returned nothing' } if (!impl.done) return { issues, error: 'implementer stopped', impl } @@ -134,12 +134,14 @@ const LENSES = [ { key: 'conformance', model: 'sonnet', prompt: 'repo conformance: docs-update obligations met in this diff (endpoint → api-conventions + regenerated v1.json/endpoint-index; screen/route → blazor-route-parity + domain-model; convention → decision record + regenerated catalog; new doc → README index); no narrative in docs; every new script or hook has its inventory row; CPM respected; both-provider migration if the model changed; tests are NUnit/vitest in the existing projects; no BOM in touched .cs; no edit to a file another slot owns (listed above); commit trailers present; branch rebased on current origin/main; nothing pushed yet.' }, ] let xfamily = rubric ? 'codex' : 'not required (routine risk class under process.independent-review-rubric)' -async function codexReview(round) { +async function codexRunner(round) { const r = await agent(`${reviewCommon(Number(args.port) + 3)} You run the cross-family review — the diff touches a class where process.independent-review-rubric requires a reviewer from another model family, and you are only the runner. Write a prompt file under a directory you create in /private/tmp asking for an adversarial correctness and security review of the diff of branch ${BRANCH} against origin/main in ${WT} for issue(s) ${REF} with done condition "${args.done_condition}", listing findings as blocking / should-fix / nit with file and evidence, ending with a line VERDICT: merge or VERDICT: send-back. Run it EXACTLY like this, in the background, output to a file, stdin from /dev/null (it hangs otherwise): codex exec -C ${WT} -s read-only "$(cat )" < /dev/null > 2>&1 — then wait for the process to exit (poll pgrep on its PID with Monitor; measured 2026-07-28 in the #672 session, a real review took ~35 minutes for a 7-file diff) and read the file. Return its findings faithfully in the schema with ran=true; if the file has no VERDICT line the run failed (quota, tool error) — return ran=false, verdict merge, no findings, and put the file's tail in a single nit finding so the failure is visible; never invent a verdict.`, { label: `review:codex:r${round}`, phase: 'Review', model: 'sonnet', effort: 'low', schema: RUNNER_SCHEMA }) - if (r && r.ran === true) return r + return r +} +async function codexFallback(round, r) { xfamily = `codex could not run in round ${round} (${r ? 'no VERDICT line' : 'runner returned nothing'}); substituted a cold same-family review-only agent per process.independent-review-rubric — retry cross-family next window` log(`${REF}: ${xfamily}`) return agent(`${reviewCommon(Number(args.port) + 2)} @@ -148,17 +150,20 @@ 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) { - // 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. + // The Codex runner builds nothing, so it may run beside the lenses; the FALLBACK is a second + // worktree-isolated .NET reviewer and starts only after both lenses have returned. + const runnerPromise = rubric ? codexRunner(round).catch(() => null) : Promise.resolve(null) 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 })))).filter(Boolean) if (!rubric) return lenses - const xf = await codexReview(round) - return xf ? lenses.concat([xf]) : lenses + const r = await runnerPromise + if (r && r.ran === true) return lenses.concat([r]) + let fb = null + try { fb = await codexFallback(round, r) } catch (e) { log(`${REF}: fallback reviewer threw: ${e && e.message}`) } + return fb ? lenses.concat([fb]) : lenses } let round = 1 @@ -179,7 +184,7 @@ ${WORKTREE} You are the fixer. Reviewers found these problems in the unpushed 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)} Then re-run the LOCAL GATE and STOP without pushing; the reviewers read the worktree again. ${GATE} -Report.`, +Report, with head_sha = git rev-parse HEAD of the worktree after your last commit.`, { 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 @@ -203,7 +208,7 @@ Then ONE push: git push -u origin ${BRANCH}. Open the PR with the Gitea API (POS <> -${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: +${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 4f205e55a..47b75d672 100644 --- a/.claude/workflows/ersatztv-resume-branch.js +++ b/.claude/workflows/ersatztv-resume-branch.js @@ -80,12 +80,12 @@ let work if (args.mode === 'implement') { work = await agent(`${COMMON} -You are the implementer, continuing a paused session. Read git log and git show for the branch's commits first; a WIP checkpoint commit is the paused implementer's partial edit. ${REBASE} The brief's recon is a plan; verify what you rely on. Finish the done condition completely, with a regression test that reddens against the unfixed code. Run the LOCAL GATE and STOP without pushing; reviewers read the worktree first. ${GATE}`, +You are the implementer, continuing a paused session. Read git log and git show for the branch's commits first; a WIP checkpoint commit is the paused implementer's partial edit. ${REBASE} The brief's recon is a plan; verify what you rely on. Finish the done condition completely, with a regression test that reddens against the unfixed code. Run the LOCAL GATE and STOP without pushing; reviewers read the worktree first; report head_sha = git rev-parse HEAD of the worktree after your last commit. ${GATE}`, { label: `impl:${REF}`, model: 'opus', effort: 'high', schema: REPORT_SCHEMA }) } else { work = await agent(`${COMMON} -You are the fixer, continuing a paused session. The PR is #${args.pr}. ${REBASE} Then the brief's findings are the last review round's: fix every blocking and should-fix one as new commits, or show with evidence why a finding is wrong. Run the LOCAL GATE and STOP without pushing; reviewers read the worktree first. ${GATE}`, +You are the fixer, continuing a paused session. The PR is #${args.pr}. ${REBASE} Then the brief's findings are the last review round's: fix every blocking and should-fix one as new commits, or show with evidence why a finding is wrong. Run the LOCAL GATE and STOP without pushing; reviewers read the worktree first; report head_sha = git rev-parse HEAD of the worktree after your last commit. ${GATE}`, { label: `fix:${REF}`, model: 'opus', effort: 'high', schema: REPORT_SCHEMA }) } if (!work) return { issues, error: 'work agent returned nothing' } @@ -102,12 +102,14 @@ const LENSES = [ { key: 'conformance', model: 'sonnet', prompt: 'repo conformance: docs-update obligations met; no narrative in docs; inventory rows for new scripts/hooks; CPM respected; both-provider migration if the model changed; no BOM in touched .cs; no WIP commit left in history; branch rebased on current origin/main; commit trailers present; PR body will carry fixes #N for each issue.' }, ] let xfamily = rubric ? 'codex' : 'not required (routine risk class under process.independent-review-rubric)' -async function codexReview(round) { +async function codexRunner(round) { const r = await agent(`${reviewCommon(Number(args.port) + 3)} You run the cross-family review required by process.independent-review-rubric; you are only the runner. Write a prompt file under a directory you create in /private/tmp asking for an adversarial correctness and security review of branch ${BRANCH} against origin/main in ${WT} for ${REF} with done condition from the brief, findings as blocking / should-fix / nit with file and evidence, ending with VERDICT: merge or VERDICT: send-back. Run EXACTLY: codex exec -C ${WT} -s read-only "$(cat )" < /dev/null > 2>&1 in the background, wait for the PID to exit (Monitor; measured 2026-07-28 in the #672 session, ~35 minutes for a 7-file diff), read the file, return its findings faithfully with ran=true; no VERDICT line means the run failed — return ran=false, verdict merge, no findings, and the file's tail in one nit finding; never invent a verdict.`, { label: `review:codex:r${round}`, phase: 'Review', model: 'sonnet', effort: 'low', schema: RUNNER_SCHEMA }) - if (r && r.ran === true) return r + return r +} +async function codexFallback(round, r) { xfamily = `codex could not run in round ${round} (${r ? 'no VERDICT line' : 'runner returned nothing'}); substituted a cold same-family review-only agent per process.independent-review-rubric — retry cross-family next window` log(`${REF}: ${xfamily}`) return agent(`${reviewCommon(Number(args.port) + 2)} @@ -116,17 +118,20 @@ 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) { - // 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. + // The Codex runner builds nothing, so it may run beside the lenses; the FALLBACK is a second + // worktree-isolated .NET reviewer and starts only after both lenses have returned. + const runnerPromise = rubric ? codexRunner(round).catch(() => null) : Promise.resolve(null) 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 })))).filter(Boolean) if (!rubric) return lenses - const xf = await codexReview(round) - return xf ? lenses.concat([xf]) : lenses + const r = await runnerPromise + if (r && r.ran === true) return lenses.concat([r]) + let fb = null + try { fb = await codexFallback(round, r) } catch (e) { log(`${REF}: fallback reviewer threw: ${e && e.message}`) } + return fb ? lenses.concat([fb]) : lenses } let round = 1 @@ -144,7 +149,7 @@ while (sendBack.length && round < 3) { 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}`, +Re-run the LOCAL GATE and STOP without pushing; report head_sha = git rev-parse HEAD of the worktree after your last commit. ${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 @@ -168,7 +173,7 @@ Then git push --force-with-lease origin ${BRANCH}. ${args.pr ? `Update PR #${arg <> -${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: +${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/decisions/records/process/orchestrated-session.md b/docs/decisions/records/process/orchestrated-session.md index f7e932182..1fe9a9528 100644 --- a/docs/decisions/records/process/orchestrated-session.md +++ b/docs/decisions/records/process/orchestrated-session.md @@ -10,8 +10,7 @@ signals: 'orchestrator · referee · slots · parallel issues · worktree per is mechanics: '`docs/handoffs/orchestration.md` owns roles, isolation and the landing order; the Workflow scripts under `.claude/workflows/` encode it. Box-ticking: the implementer writes the evidence per box into the `## Closing record`; the orchestrator ticks after reading it and the review evidence, then `scripts/post-review-verdict.sh`.' --- -The kickoff (`docs/handoffs/chicorytv-issue-queue.md`) binds a session that closes one issue. Running -three at once does not relax any of it; what it adds is a referee, and two places where the +The kickoff (`docs/handoffs/chicorytv-issue-queue.md`) binds a session that closes one issue. Running several at once does not relax any of it; what it adds is a referee, and two places where the single-session rules needed a stated scope rather than a contradiction. **Force-with-lease.** `process.pr-routine-sequence` forbids amending or force-pushing a pushed branch; diff --git a/docs/handoffs/orchestration.md b/docs/handoffs/orchestration.md index 4282084c8..07b1cb29c 100644 --- a/docs/handoffs/orchestration.md +++ b/docs/handoffs/orchestration.md @@ -52,14 +52,18 @@ script and returns a path for later runs. lockfiles match, otherwise `npm ci`. The shared copy is kept current by `scripts/refresh-shared-checkout.sh` at session end. - **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 + slot's correctness reviewer builds .NET in its own worktree, and on a rubric-class change whose + Codex runner cannot run the fallback is a second such reviewer, so a review round can be 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). + round alone took swap from 6.8 GB to 10.8 GB in three minutes with the two reviewers concurrent. + The scripts therefore start the fallback reviewer only after the lenses return (the Codex runner + builds nothing and runs beside them). The cap stays at one even so: the implementer's own build, + one reviewer pipeline and the MSBuild node servers that linger after each build already fill the + budget beside the host's other sessions; re-measure before raising it. + `process.build-concurrency-limits` is the standing rule (under 20% free RAM launch nothing that + builds, under 10% pause); 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 66475a6ce..8f6dcf847 100644 --- a/docs/handoffs/orchestrator-prompt.md +++ b/docs/handoffs/orchestrator-prompt.md @@ -10,7 +10,7 @@ Way of working: - 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. -- 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. +- One .NET slot means two build pipelines in its review round (the correctness lens and, on a rubric change whose Codex runner cannot run, the fallback reviewer, one after the other); `process.build-concurrency-limits` holds — under 20% free RAM launch nothing that builds, under 10% pause — so read `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 606875375..dc14d5d28 100644 --- a/web/scripts/orchestration-workflow-loop.test.mjs +++ b/web/scripts/orchestration-workflow-loop.test.mjs @@ -42,26 +42,24 @@ function run(script, args, { reviewRounds, landOverrides = {}, postRebase = null const calls = []; let round = 0; let fixCount = 0; - let lensesInFlight = 0; - const xfamilyStartedWithLensesInFlight = []; + 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:')) { - xfamilyStartedWithLensesInFlight.push(lensesInFlight); return { verdict: 'merge', findings: [], ran: false }; } if (label.startsWith('review:fallback:')) { - xfamilyStartedWithLensesInFlight.push(lensesInFlight); + 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: [] }; - lensesInFlight += 1; - await new Promise((resolve) => setTimeout(resolve, 5)); - lensesInFlight -= 1; + await new Promise((resolve) => globalThis.setTimeout(resolve, 5)); + lensesReturned += 1; if (spec.lensesNull) return null; return { verdict: 'merge', findings: spec.findings }; } @@ -81,7 +79,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, xfamilyStartedWithLensesInFlight })); + return script(args, agent, parallel, () => {}, () => {}).then((result) => ({ result, calls, fixCount, lensesReturnedWhenFallbackStarted })); } const cases = [ @@ -173,13 +171,13 @@ 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 () => { + 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, xfamilyStartedWithLensesInFlight } = await run(script, rubricArgs, { reviewRounds: [{ findings: [] }] }); + 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(xfamilyStartedWithLensesInFlight).toEqual([0, 0]); + expect(lensesReturnedWhenFallbackStarted).toEqual([2]); expect(finisherPrompt(calls)).toContain('substituted a cold same-family review-only agent'); }); -- 2.47.3 From 595c819de1fe5e3611e07d60fe355c800bf16351 Mon Sep 17 00:00:00 2001 From: Timothy Date: Sat, 5 Sep 2026 13:14:09 +0200 Subject: [PATCH 3/5] fix(916): a failed substitute is an error, not a landed claim; head_sha described in both schemas; the issue's box says what the code does The runner-beside-the-lenses design is now what Done-when box 1 asks for (body amended). A rubric round whose runner and worktree fallback both fail returns an error before the push instead of landing a PR whose body claims a substitute reviewed it. The harness records lens count at the runner's start too (expects 0, so a re-serialised runner reddens), resets its counter per round, and has a case for the double failure. Decisions-Edit: yes Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_015QqCpYFsKgnAnx6jVwrKiV --- .claude/workflows/ersatztv-issue-build.js | 7 +++++-- .claude/workflows/ersatztv-resume-branch.js | 5 ++++- .../records/process/orchestrated-session.md | 5 +++-- .../orchestration-workflow-loop.test.mjs | 20 +++++++++++++++---- 4 files changed, 28 insertions(+), 9 deletions(-) diff --git a/.claude/workflows/ersatztv-issue-build.js b/.claude/workflows/ersatztv-issue-build.js index b5fa31668..c6e4d1945 100644 --- a/.claude/workflows/ersatztv-issue-build.js +++ b/.claude/workflows/ersatztv-issue-build.js @@ -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', description: 'git rev-parse HEAD after your last commit — the finisher derives fix commits from these' }, + head_sha: { type: 'string', description: 'git rev-parse HEAD of YOUR WORKTREE after your last commit (not a PR head) — 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)' }, }, } @@ -118,7 +118,7 @@ ${CLAIM} ${recon ? `Recon (verify what you rely on):\nPLAN: ${recon.plan}\nFACTS: ${recon.facts}\nRISKS: ${recon.risks}\nTEST PLAN: ${recon.test_plan}\n` : ''} You are the implementer. Close ${REF} completely: pin the behaviour with tests named for the branch they protect, update the docs the change obligates, commit. Then git fetch origin and rebase onto origin/main if it moved (never merge main in; regenerate generated artifacts), run the LOCAL GATE and STOP — do not push; reviewers read your worktree first, and a finisher pushes once after the review loop is clean. ${GATE} -Report done=true with the gate output when the worktree is ready for review, with pr_url empty and head_sha = git rev-parse HEAD.`, +Report done=true with the gate output when the worktree is ready for review, with pr_url empty and head_sha = git rev-parse HEAD of the worktree after your last commit.`, { label: `impl:${REF}`, model: implModel, effort: implEffort, schema: REPORT_SCHEMA }) if (!impl) return { issues, error: 'implementer returned nothing' } if (!impl.done) return { issues, error: 'implementer stopped', impl } @@ -133,6 +133,7 @@ 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 actually reddens when the fix alone is reverted (mutate the clause, not the file).' }, { key: 'conformance', model: 'sonnet', prompt: 'repo conformance: docs-update obligations met in this diff (endpoint → api-conventions + regenerated v1.json/endpoint-index; screen/route → blazor-route-parity + domain-model; convention → decision record + regenerated catalog; new doc → README index); no narrative in docs; every new script or hook has its inventory row; CPM respected; both-provider migration if the model changed; tests are NUnit/vitest in the existing projects; no BOM in touched .cs; no edit to a file another slot owns (listed above); commit trailers present; branch rebased on current origin/main; nothing pushed yet.' }, ] +let xfamilyFailedRound = null let xfamily = rubric ? 'codex' : 'not required (routine risk class under process.independent-review-rubric)' async function codexRunner(round) { const r = await agent(`${reviewCommon(Number(args.port) + 3)} @@ -163,6 +164,7 @@ Be adversarial; report only what you verified, with evidence. blocking = done co if (r && r.ran === true) return lenses.concat([r]) let fb = null try { fb = await codexFallback(round, r) } catch (e) { log(`${REF}: fallback reviewer threw: ${e && e.message}`) } + if (!fb) { xfamily += ` — the substitute ALSO failed in round ${round}; no cross-family-equivalent review ran`; xfamilyFailedRound = round } return fb ? lenses.concat([fb]) : lenses } @@ -199,6 +201,7 @@ Report, with head_sha = git rev-parse HEAD of the worktree after your last commi } 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 } +if (xfamilyFailedRound) return { issues, error: `the cross-family runner and its substitute both failed in round ${xfamilyFailedRound}; not pushed`, cross_family: xfamily, history } 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') diff --git a/.claude/workflows/ersatztv-resume-branch.js b/.claude/workflows/ersatztv-resume-branch.js index 47b75d672..d2121e59f 100644 --- a/.claude/workflows/ersatztv-resume-branch.js +++ b/.claude/workflows/ersatztv-resume-branch.js @@ -50,7 +50,7 @@ const REPORT_SCHEMA = { properties: { done: { type: 'boolean' }, summary: { type: 'string' }, verified: { type: 'string', description: 'exact gate commands run and their real output summary' }, - left: { type: 'string' }, commits: { type: 'string', description: 'git log --oneline origin/main..HEAD' }, pr_url: { type: 'string' }, head_sha: { type: 'string' }, + left: { type: 'string' }, commits: { type: 'string', description: 'git log --oneline origin/main..HEAD' }, pr_url: { type: 'string' }, head_sha: { type: 'string', description: 'git rev-parse HEAD of YOUR WORKTREE after your last commit (not a PR head) — the finisher derives fix commits from these' }, patch_changed: { type: 'boolean', description: 'finisher only: true if a second rebase before the push changed the patch-id' }, }, } @@ -101,6 +101,7 @@ 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.' }, { key: 'conformance', model: 'sonnet', prompt: 'repo conformance: docs-update obligations met; no narrative in docs; inventory rows for new scripts/hooks; CPM respected; both-provider migration if the model changed; no BOM in touched .cs; no WIP commit left in history; branch rebased on current origin/main; commit trailers present; PR body will carry fixes #N for each issue.' }, ] +let xfamilyFailedRound = null let xfamily = rubric ? 'codex' : 'not required (routine risk class under process.independent-review-rubric)' async function codexRunner(round) { const r = await agent(`${reviewCommon(Number(args.port) + 3)} @@ -131,6 +132,7 @@ Be adversarial; report only what you verified, with evidence. blocking = done co if (r && r.ran === true) return lenses.concat([r]) let fb = null try { fb = await codexFallback(round, r) } catch (e) { log(`${REF}: fallback reviewer threw: ${e && e.message}`) } + if (!fb) { xfamily += ` — the substitute ALSO failed in round ${round}; no cross-family-equivalent review ran`; xfamilyFailedRound = round } return fb ? lenses.concat([fb]) : lenses } @@ -164,6 +166,7 @@ Re-run the LOCAL GATE and STOP without pushing; report head_sha = git rev-parse } 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 } +if (xfamilyFailedRound) return { issues, error: `the cross-family runner and its substitute both failed in round ${xfamilyFailedRound}; not pushed`, cross_family: xfamily, history } 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') diff --git a/docs/decisions/records/process/orchestrated-session.md b/docs/decisions/records/process/orchestrated-session.md index 1fe9a9528..1d2a9af6d 100644 --- a/docs/decisions/records/process/orchestrated-session.md +++ b/docs/decisions/records/process/orchestrated-session.md @@ -10,8 +10,9 @@ signals: 'orchestrator · referee · slots · parallel issues · worktree per is mechanics: '`docs/handoffs/orchestration.md` owns roles, isolation and the landing order; the Workflow scripts under `.claude/workflows/` encode it. Box-ticking: the implementer writes the evidence per box into the `## Closing record`; the orchestrator ticks after reading it and the review evidence, then `scripts/post-review-verdict.sh`.' --- -The kickoff (`docs/handoffs/chicorytv-issue-queue.md`) binds a session that closes one issue. Running several at once does not relax any of it; what it adds is a referee, and two places where the -single-session rules needed a stated scope rather than a contradiction. +The kickoff (`docs/handoffs/chicorytv-issue-queue.md`) binds a session that closes one issue. +Running several at once does not relax any of it; what it adds is a referee, and two places where +the single-session rules needed a stated scope rather than a contradiction. **Force-with-lease.** `process.pr-routine-sequence` forbids amending or force-pushing a pushed branch; `release.format-as-you-touch-rebase` (H11) refuses to push a branch behind `origin/main` and forbids diff --git a/web/scripts/orchestration-workflow-loop.test.mjs b/web/scripts/orchestration-workflow-loop.test.mjs index dc14d5d28..49a715f43 100644 --- a/web/scripts/orchestration-workflow-loop.test.mjs +++ b/web/scripts/orchestration-workflow-loop.test.mjs @@ -38,21 +38,24 @@ 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, fixerNoCommit = false }) { +function run(script, args, { reviewRounds, landOverrides = {}, postRebase = null, fixerNull = false, fixerDone = true, fixerNoCommit = false, fallbackNull = false }) { const calls = []; let round = 0; let fixCount = 0; let lensesReturned = 0; const lensesReturnedWhenFallbackStarted = []; + const lensesReturnedWhenRunnerStarted = []; 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:')) { + lensesReturnedWhenRunnerStarted.push(lensesReturned); return { verdict: 'merge', findings: [], ran: false }; } if (label.startsWith('review:fallback:')) { lensesReturnedWhenFallbackStarted.push(lensesReturned); + if (fallbackNull) return null; const spec = reviewRounds[round - 1] || { findings: [] }; return { verdict: 'merge', findings: spec.findings }; } @@ -74,12 +77,13 @@ function run(script, args, { reviewRounds, landOverrides = {}, postRebase = null }; const parallel = async (thunks) => { round += 1; + lensesReturned = 0; 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 })); + return script(args, agent, parallel, () => {}, () => {}).then((result) => ({ result, calls, fixCount, lensesReturnedWhenFallbackStarted, lensesReturnedWhenRunnerStarted })); } const cases = [ @@ -171,13 +175,21 @@ 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 worktree-isolated fallback reviewer starts only after both lenses have returned', async () => { + it('a rubric round whose runner and substitute both fail is an error, never a landed PR', async () => { const rubricArgs = { ...args, risk: 'rubric' }; - const { result, calls, lensesReturnedWhenFallbackStarted } = await run(script, rubricArgs, { reviewRounds: [{ findings: [] }] }); + const { result, calls } = await run(script, rubricArgs, { reviewRounds: [{ findings: [] }], fallbackNull: true }); + expect(result.error).toMatch(/^the cross-family runner and its substitute both failed in round 1/); + expect(calls.some((c) => c.label.startsWith('land:'))).toBe(false); + }); + + it('on a rubric-class change the fallback reviewer starts only after both lenses have returned, and the runner beside them', async () => { + const rubricArgs = { ...args, risk: 'rubric' }; + const { result, calls, lensesReturnedWhenFallbackStarted, lensesReturnedWhenRunnerStarted } = 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(lensesReturnedWhenRunnerStarted).toEqual([0]); expect(finisherPrompt(calls)).toContain('substituted a cold same-family review-only agent'); }); -- 2.47.3 From b1d5fbefcba02fdc6c19fef85cec1c4e82fc8dea Mon Sep 17 00:00:00 2001 From: Timothy Date: Sat, 5 Sep 2026 13:46:39 +0200 Subject: [PATCH 4/5] fix(916): cross-family state is per round, and the orchestrator reads cross_family before a verdict A substitute that failed in round one said nothing about the tree that lands after round two, yet the flag was sticky and doomed the run; the xfamily string was never reset either, so clearing the stickiness alone would have let a stale "substitute ALSO failed" sentence into the PR body. Both reset at the top of review(). The harness runner is round-aware (ran per round, its own counter reset) and a two-round case pins the fix; restoring the sticky flag reddens it in both scripts. Step 4 of the mechanics page tells the referee to read cross_family, not only error, before posting on a rubric-class PR. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_015QqCpYFsKgnAnx6jVwrKiV --- .claude/workflows/ersatztv-issue-build.js | 4 ++++ .claude/workflows/ersatztv-resume-branch.js | 4 ++++ docs/handoffs/orchestration.md | 8 +++++--- .../orchestration-workflow-loop.test.mjs | 18 ++++++++++++++++-- 4 files changed, 29 insertions(+), 5 deletions(-) diff --git a/.claude/workflows/ersatztv-issue-build.js b/.claude/workflows/ersatztv-issue-build.js index c6e4d1945..f6fea6750 100644 --- a/.claude/workflows/ersatztv-issue-build.js +++ b/.claude/workflows/ersatztv-issue-build.js @@ -151,6 +151,10 @@ 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) { + // Per round, like blocking/sendBack: a substitute that failed in round 1 says nothing about the tree + // that lands after round 2, and a stale xfamily string must never reach the PR body. + xfamilyFailedRound = null + xfamily = rubric ? 'codex' : 'not required (routine risk class under process.independent-review-rubric)' // The Codex runner builds nothing, so it may run beside the lenses; the FALLBACK is a second // worktree-isolated .NET reviewer and starts only after both lenses have returned. const runnerPromise = rubric ? codexRunner(round).catch(() => null) : Promise.resolve(null) diff --git a/.claude/workflows/ersatztv-resume-branch.js b/.claude/workflows/ersatztv-resume-branch.js index d2121e59f..05f71a451 100644 --- a/.claude/workflows/ersatztv-resume-branch.js +++ b/.claude/workflows/ersatztv-resume-branch.js @@ -119,6 +119,10 @@ 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) { + // Per round, like blocking/sendBack: a substitute that failed in round 1 says nothing about the tree + // that lands after round 2, and a stale xfamily string must never reach the PR body. + xfamilyFailedRound = null + xfamily = rubric ? 'codex' : 'not required (routine risk class under process.independent-review-rubric)' // The Codex runner builds nothing, so it may run beside the lenses; the FALLBACK is a second // worktree-isolated .NET reviewer and starts only after both lenses have returned. const runnerPromise = rubric ? codexRunner(round).catch(() => null) : Promise.resolve(null) diff --git a/docs/handoffs/orchestration.md b/docs/handoffs/orchestration.md index 07b1cb29c..c59e904fe 100644 --- a/docs/handoffs/orchestration.md +++ b/docs/handoffs/orchestration.md @@ -107,9 +107,11 @@ the review loop *inside* the worktree, before the single push: 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 MERGEABLE - ` 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 + ` naming the reviewers, the rounds and the workflow's returned `cross_family` status. Read + that field, not only `error`: a cross-family failure in the post-rebase round leaves the branch + pushed and the run successful, and only `cross_family` says the substitute also failed; a + rubric-class PR in that state gets no verdict until a cross-family or substitute review of the + pushed head has run. 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 diff --git a/web/scripts/orchestration-workflow-loop.test.mjs b/web/scripts/orchestration-workflow-loop.test.mjs index 49a715f43..db9aa63cd 100644 --- a/web/scripts/orchestration-workflow-loop.test.mjs +++ b/web/scripts/orchestration-workflow-loop.test.mjs @@ -50,12 +50,14 @@ function run(script, args, { reviewRounds, landOverrides = {}, postRebase = null 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:')) { + lensesReturned = 0; lensesReturnedWhenRunnerStarted.push(lensesReturned); - return { verdict: 'merge', findings: [], ran: false }; + const spec = reviewRounds[round] || {}; + return { verdict: 'merge', findings: [], ran: spec.runnerRan === true }; } if (label.startsWith('review:fallback:')) { lensesReturnedWhenFallbackStarted.push(lensesReturned); - if (fallbackNull) return null; + if (fallbackNull === true || (Array.isArray(fallbackNull) && fallbackNull.includes(round))) return null; const spec = reviewRounds[round - 1] || { findings: [] }; return { verdict: 'merge', findings: spec.findings }; } @@ -175,6 +177,18 @@ describe.each(cases)('%s review loop', (name, args) => { expect(result.error).toMatch(/^the post-rebase review round produced no reviews/); }); + it('a substitute failure in round one does not doom a run whose landing round had a real cross-family review', async () => { + const rubricArgs = { ...args, risk: 'rubric' }; + const { result, calls } = await run(script, rubricArgs, { + reviewRounds: [{ findings: [finding('should-fix')] }, { findings: [], runnerRan: true }], + fallbackNull: [1], + }); + expect(result.error).toBeUndefined(); + expect(result.cross_family).toBe('codex'); + expect(finisherPrompt(calls)).not.toContain('substitute ALSO failed'); + expect(calls.filter((c) => c.label.startsWith('review:codex:')).length).toBe(2); + }); + it('a rubric round whose runner and substitute both fail is an error, never a landed PR', async () => { const rubricArgs = { ...args, risk: 'rubric' }; const { result, calls } = await run(script, rubricArgs, { reviewRounds: [{ findings: [] }], fallbackNull: true }); -- 2.47.3 From a083c851b3535fe4e10c6e0c6b6a61ecc9477a4e Mon Sep 17 00:00:00 2001 From: Timothy Date: Sat, 5 Sep 2026 14:13:56 +0200 Subject: [PATCH 5/5] fix(916): the harness counts lens completions per round, so the runner assertion can fail The runner observer reset the shared counter to zero one line before reading it, so its assertion held under the very mutant it existed to reject and the fallback assertion caught that mutant for the wrong reason. Completions are now keyed by the round in each agent's own label; no stub resets shared state. Measured: re-serialising the runner reddens the runner assertion (expected [2] to equal [0]) in both scripts; moving the fallback beside the lenses reddens the fallback assertion and the round-one-failure case. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_015QqCpYFsKgnAnx6jVwrKiV --- .../orchestration-workflow-loop.test.mjs | 31 ++++++++++--------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/web/scripts/orchestration-workflow-loop.test.mjs b/web/scripts/orchestration-workflow-loop.test.mjs index db9aa63cd..6dca2fb44 100644 --- a/web/scripts/orchestration-workflow-loop.test.mjs +++ b/web/scripts/orchestration-workflow-loop.test.mjs @@ -36,13 +36,19 @@ 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. +// reviewRounds: array indexed by round-1 of {findings, lensesNull?, runnerRan?} — every review-stage stub reads +// its round from its own label (review::rN), never from a shared counter; landOverrides: extra fields on +// the finisher report; postRebase: findings returned by the review after a patch-changing rebase, or 'lensesNull'; +// fixerNull / fixerDone / fixerNoCommit: fixer shapes; fallbackNull: true, or an array of rounds in which the +// fallback agent dies. function run(script, args, { reviewRounds, landOverrides = {}, postRebase = null, fixerNull = false, fixerDone = true, fixerNoCommit = false, fallbackNull = false }) { const calls = []; - let round = 0; let fixCount = 0; - let lensesReturned = 0; + // Lens completions are counted PER ROUND, keyed by the round in each agent's label (review::rN), + // so an observer never depends on another stub resetting shared state. + const lensesReturnedByRound = new Map(); + const roundOf = (l) => Number(/:r(\d+)$/.exec(l)[1]); + const returnedIn = (l) => lensesReturnedByRound.get(roundOf(l)) || 0; const lensesReturnedWhenFallbackStarted = []; const lensesReturnedWhenRunnerStarted = []; const agent = async (prompt, opts) => { @@ -50,21 +56,20 @@ function run(script, args, { reviewRounds, landOverrides = {}, postRebase = null 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:')) { - lensesReturned = 0; - lensesReturnedWhenRunnerStarted.push(lensesReturned); - const spec = reviewRounds[round] || {}; + lensesReturnedWhenRunnerStarted.push(returnedIn(label)); + const spec = reviewRounds[roundOf(label) - 1] || {}; return { verdict: 'merge', findings: [], ran: spec.runnerRan === true }; } if (label.startsWith('review:fallback:')) { - lensesReturnedWhenFallbackStarted.push(lensesReturned); - if (fallbackNull === true || (Array.isArray(fallbackNull) && fallbackNull.includes(round))) return null; - const spec = reviewRounds[round - 1] || { findings: [] }; + lensesReturnedWhenFallbackStarted.push(returnedIn(label)); + if (fallbackNull === true || (Array.isArray(fallbackNull) && fallbackNull.includes(roundOf(label)))) return null; + const spec = reviewRounds[roundOf(label) - 1] || { findings: [] }; return { verdict: 'merge', findings: spec.findings }; } if (label.startsWith('review:')) { - const spec = reviewRounds[round - 1] || { findings: [] }; + const spec = reviewRounds[roundOf(label) - 1] || { findings: [] }; await new Promise((resolve) => globalThis.setTimeout(resolve, 5)); - lensesReturned += 1; + lensesReturnedByRound.set(roundOf(label), returnedIn(label) + 1); if (spec.lensesNull) return null; return { verdict: 'merge', findings: spec.findings }; } @@ -78,8 +83,6 @@ function run(script, args, { reviewRounds, landOverrides = {}, postRebase = null throw new Error(`unexpected agent label ${label}`); }; const parallel = async (thunks) => { - round += 1; - lensesReturned = 0; if (postRebase !== null && calls.some((c) => c.label.startsWith('land:'))) { return Promise.all(thunks.map(() => Promise.resolve(postRebase === 'lensesNull' ? null : { verdict: 'merge', findings: postRebase }))); } -- 2.47.3