fix(916): the worktree-isolated fallback reviewer follows the lenses, fix commits are a sha range, and the cap is one .NET slot #918

Merged
timothy merged 5 commits from 916-serialise-review-fallback into main 2026-09-05 14:58:13 +02:00
6 changed files with 155 additions and 63 deletions
+34 -19
View File
@@ -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 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.`,
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,13 +133,16 @@ 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 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 <prompt>)" < /dev/null > <out> 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,25 +151,36 @@ 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)}
// 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)
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 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}`) }
if (!fb) { xfamily += ` — the substitute ALSO failed in round ${round}; no cross-family-equivalent review ran`; xfamilyFailedRound = round }
return fb ? lenses.concat([fb]) : 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}
@@ -176,23 +190,24 @@ ${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
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')
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')
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 +215,7 @@ Then ONE push: git push -u origin ${BRANCH}. Open the PR with the Gitea API (POS
<<REVIEW HISTORY
${REVIEW_HISTORY}
REVIEW HISTORY>>
${FIX_COMMITS.length ? `followed by what each of these fix commits changed, read from git show and not from memory: ${FIX_COMMITS.join(' | ')}` : (history.some(h => h.fix) ? 'and a sentence saying every finding was answered without a new commit, as the history block records' : 'and a sentence saying no fix commit exists because round one was clean')}, then the cross-family review status verbatim — "${xfamily}" — and every deliberately-left item with an issue number (file follow-up issues where needed). End the body with:
${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}
+35 -20
View File
@@ -46,11 +46,11 @@ 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=<target> is already its shape, so GIT_SEQUENCE_EDITOR=true git rebase --autosquash <target>~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' },
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' },
},
}
@@ -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' }
@@ -101,13 +101,16 @@ 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 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 <prompt>)" < /dev/null > <out> 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,48 +119,60 @@ 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)}
// 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)
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 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}`) }
if (!fb) { xfamily += ` — the substitute ALSO failed in round ${round}; no cross-family-equivalent review ran`; xfamilyFailedRound = round }
return fb ? lenses.concat([fb]) : 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}
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
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')
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')
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 +180,7 @@ Then git push --force-with-lease origin ${BRANCH}. ${args.pr ? `Update PR #${arg
<<REVIEW HISTORY
${REVIEW_HISTORY}
REVIEW HISTORY>>
${FIX_COMMITS.length ? `followed by what each of these fix commits changed, read from git show and not from memory: ${FIX_COMMITS.join(' | ')}` : (history.some(h => h.fix) ? 'and a sentence saying every finding was answered without a new commit, as the history block records' : 'and a sentence saying no fix commit exists because round one was clean')}, then the cross-family review status verbatim — "${xfamily}" — every deliberately-left item with an issue number, and end with:
${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}
@@ -10,9 +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
three 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
+18 -7
View File
@@ -51,10 +51,19 @@ 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 34 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 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 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
@@ -98,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 <pr> MERGEABLE
<note>` naming the reviewers, the rounds and the workflow's returned `cross_family` status (a
Codex failure in a post-rebase round is in that return, not in the PR body); tick the review
box. Wait for CI — a `cancelled` job
<note>` 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
+2 -2
View File
@@ -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-<n>` on branch `<n>-<slug>`, 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-<n>` on branch `<n>-<slug>`, 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 <pr> MERGEABLE <note>` 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 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.
@@ -36,18 +36,40 @@ const IMPL_COMMITS = 'aaaaaaa feat(1): the implementer commit';
const finding = (severity) => ({ severity, file: 'f', summary: `${severity} finding`, evidence: 'e' });
// Every stub lens returns the same findings, so a finding counts once per lens (two lenses → 2).
// reviewRounds: array indexed by round-1 of {findings, lensesNull?}; landOverrides: extra fields on the finisher report;
// postRebase: findings returned by the review after a patch-changing rebase, or 'lensesNull'; fixerNull: the fixer agent dies.
function run(script, args, { reviewRounds, landOverrides = {}, postRebase = null, fixerNull = false, fixerDone = true, fixerCommits = null }) {
// reviewRounds: array indexed by round-1 of {findings, lensesNull?, runnerRan?} — every review-stage stub reads
// its round from its own label (review:<lens>: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;
// Lens completions are counted PER ROUND, keyed by the round in each agent's label (review:<lens>: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) => {
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:')) {
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(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));
lensesReturnedByRound.set(roundOf(label), returnedIn(label) + 1);
if (spec.lensesNull) return null;
return { verdict: 'merge', findings: spec.findings };
}
@@ -55,19 +77,18 @@ 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}`);
};
const parallel = async (thunks) => {
round += 1;
if (postRebase !== null && calls.some((c) => c.label.startsWith('land:'))) {
return Promise.all(thunks.map(() => Promise.resolve(postRebase === 'lensesNull' ? null : { verdict: 'merge', findings: postRebase })));
}
return Promise.all(thunks.map((t) => t().catch(() => null)));
};
return script(args, agent, parallel, () => {}, () => {}).then((result) => ({ result, calls, fixCount }));
return script(args, agent, parallel, () => {}, () => {}).then((result) => ({ result, calls, fixCount, lensesReturnedWhenFallbackStarted, lensesReturnedWhenRunnerStarted }));
}
const cases = [
@@ -94,10 +115,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 +168,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 +180,36 @@ 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 });
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');
});
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: [] }],