Repository navigation
review: the two reviewers share one context and one model, so the isolation SKILL.md requires is nominal #71
Description
Activity
Loop triage —
feature/ready-for-agent(confidence: high)Confirmed the defect against the tree, not just the report:
skills/review/SKILL.md:9-10requires two reviewer agents "with isolation (separate worktree or equivalent)".scripts/loop/shared/agent.mjsrunPi()builds one argv and performs onespawn, with no--toolsflag.scripts/loop/entry.mjsdoRun()callsrunPi()once, with a singlesessionDir(LOOP_SESSION_DIR/<rid>) and a singlemodel(LOOP_MODEL).- The most recent
pr-reviewreport (state/reports/r-20260921-164320-7u4k.md) is one run whose "Two-lens review" is two sections of the same report, not two independent verdicts. - pi here is
0.85.1: there is no sub-agent mechanism, so the skill sentence cannot be satisfied as written.
Why this is loop work. Option A (two
runPi()calls in one job) touches onlyscripts/loop/**and the norms-layerskills/review/SKILL.md— no.github/workflows/**change, so the job token can push the branch.docs/norms/ops.mdexplicitly allows host changes as loop PRs (owner's review is the checker) and norms-layer changes as proposal PRs. Option B (strategy.matrixinloop.yml) and option C (pi-subagents) both need workflow/config files the loop cannot push, so they are human work.Acceptance maps to A: separate
--session-dirper reviewer → distinct session files/logs; a second--modelfrom the available catalog (deepseek-v4-flash/-vision-exp/-pro) → a genuinely different lens;--tools read,grep,find,ls,bash→ theedit/writetools are absent; and theSKILL.mdreword removes the sub-agent instruction.One caveat to carry into the implementation:
bashremains in that allowlist (the reviewer needsgit/gh), so "read-only" must be stated as "noedit/writetools", not filesystem read-onlyness. Also give each reviewer its own result/report path, since the current contract assumes one<rid>.md/<rid>.result.jsonper run and the second pass would otherwise overwrite the first.Labelling
ready-for-agent.Loop feature — PR opened for this.
pr-reviewnow runs two genuinely isolated reviewers instead of two passes of one context: twopiprocesses with separate--session-dir, a different--modelfor the second lens, and--tools read,grep,find,ls,bash, merged by the host. Both must approve (accepted); a missing verdict is never counted as approval (transientretry, otherwisefailed).skills/review/SKILL.mdwas reworded to match the mechanism (norms proposal) and no longer tells the agent to spawn sub-agents.Verified with a fake-engine harness: exactly two invocations, distinct session dirs and models, both read-only, plus the accept / request-changes / inconclusive merge paths (32/32 checks). All repo gates green (
pnpm lint,pnpm fmt:check,pnpm build,pnpm check:types, Azure + TOS oracles, headless browser suite 24 files / 255 passed / 26 skipped).
What
skills/review/SKILL.mdrequires two independent reviewers:The current loop does not do this.
pr-reviewis a single stage(
docs/agents/ops.md, "Trigger → loop map"), executed as one GitHub Actions job byone
piprocess (scripts/loop/shared/agent.mjs:207runPi, onespawn), in onesession. Reviewer A and Reviewer B are two passes of the same context and the same
model, not two independent readers.
pi's core has no sub-agent mechanism at all —
@earendil-works/pi-coding-agent@0.85.1,docs/usage.md, Design Principles:So the sentence in the skill cannot be satisfied by the current engine as written.
Why it matters
Independence is the entire point of the split. In Addy Osmani's four-reviewer
experiment (146 PRs, 679 findings), of 617 distinct flagged locations 93.4% were
caught by exactly one tool and 0% by all four — heterogeneity is what produces
coverage. Two passes of one model over one context is "a single reviewer with a larger
invoice", and it silently breaks a norms-layer promise.
Options
runPi()twice per review stage with separate--session-dir, a different--modelfor the second lens, and a read-only toolallowlist (
--tools read,grep,find,ls,bash). Smallest host change:runPialreadyaccepts
sessionDirandmodel; only--toolsneeds exposing.strategy.matrix.reviewer: [A, B]). Adds filesystem andenvironment isolation on top of context isolation. Higher CI cost.
pi-subagents(npm:pi-subagents@0.70.1, third-party) and define.pi/agents/*.md. Most capability, but pulls an unaudited 10 MB dependency into CI,and
.github/workflows/**is human-only (seeops.md), so C is not loop work.Whichever is chosen,
skills/review/SKILL.mdmust be reworded so the requirementmatches the mechanism (norms layer → proposal PR).
Acceptance
separate session files/logs.
skills/review/SKILL.mdno longer instructs the agent to spawn sub-agents.References
packages/coding-agent/docs/usage.md(Design Principles); CLI flags--tools,--model,--skill