| name | issue-pr-cycle |
|---|---|
| description | Use when managing the code/review cycle for a coherent unit of work (1+ GitHub issues, usually resolved by 1+ PRs) — spawns three persistent agents (manager/opus, implementer/sonnet, reviewer/sonnet) that hand off control cooperatively via SendMessage, preserving their conversation history across rounds instead of spawning fresh panels per round |
Manage the full lifecycle of a work unit — one or more GitHub issues, typically resolved by one PR but sometimes by several — from interrogation through approved PR(s), using three long-lived agents per work unit plus an orchestrator that mediates between them:
- Orchestrator (the caller of this skill — typically a higher-level execution skill, or you the user directly). The orchestrator is the ONLY entity that calls
Agent(to spawn) andSendMessage(to route). Subagents in this harness cannot call those tools themselves. The orchestrator spawns each role with aname, relays messages between manager ↔ reviewer and manager ↔ implementer, and holds no domain opinions — it executes what the manager directs. - Manager (opus) — owns the work unit's intent, scope, and architectural direction. Plans, directs the other two (produces their spawn prompts and their per-round instructions, hands them to the orchestrator), arbitrates their outputs, does the final cross-criterion review, decides when each PR is approved. Authors the consolidated per-round PR comments itself (via Bash /
gh pr comment). - Implementer (sonnet) — all code, test, and doc changes. Executes the plan, applies every revision the reviewer flags as blocking.
- Reviewer (sonnet) — walks six criteria in order per round (tracked as a
TaskCreatetask list). Round 1 STRICT; subsequent rounds LENIENT. Scope is always STRICT.
Each agent keeps its own conversation history across turns. Priming (reading the issue(s), CLAUDE.md, related source) happens once per agent, not per round — the orchestrator resumes them via SendMessage by the name they were spawned with. This is substantially cheaper in tokens than respawning, and the implementer / reviewer naturally remember their prior decisions.
Model split: opus for design / judgment (manager); sonnet for plan-following (implementer, reviewer). The manager's plan lets the sonnet implementer execute without re-litigating design; the reviewer's persistent context surfaces its own stuck positions without external bookkeeping.
A work unit is a coherent chunk of work the manager owns end-to-end. It's defined at spawn time by the caller:
- 1 issue → 1 PR (most common) — work unit = that issue/PR pair.
- 1 issue → N PRs (decomposed for size or stacking) — work unit = the issue plus all PRs the manager produces to resolve it. One manager coordinates across the PRs.
- N issues → 1 PR (one fix genuinely addresses several related issues) — work unit = the issue group plus the single PR. Manager sees all issues' context.
- N issues → M PRs (rare) — a tightly coupled group the manager is responsible for as a set.
Each manager is given the list of issues at spawn time. It owns the whole set; if the plan produces more than one PR, the same manager oversees every round for each.
- A work unit (1+ issues) needs implementation with thorough review before merge.
- Invoked standalone for a single issue, a group of related issues, or an existing PR.
- Invoked by a higher-level wave/batch orchestrator for each work unit in a batch (one manager per unit — usually one per issue, but the planner may group related issues).
- Issues (required): One or more issue numbers the manager owns. The lowest number is the "primary" — used for naming (
mgr-<primary>,impl-<primary>,rev-<primary>). - PR number (alternative entry): An existing open PR. Manager skips planning, reads PR + issue context, hands to reviewer directly.
- Worktree path (optional): Pre-created worktree for this work unit.
- Base branch (optional, default
main): Branch PRs should target. - Max rounds (optional, default 8): Hard ceiling on review rounds before escalation.
- Scope description (optional): When issues are large and PR(s) cover only part of them.
Many-to-many, owned by one manager:
- Manager may produce multiple PRs for the same issue (stacking, decomposition). Use "Part of #N" on intermediate PRs, "Closes #N" only when the set fully resolves the issue.
- A single PR may close multiple issues ("Closes #N, Closes #M") when one fix naturally addresses them.
- One manager owns the whole set. Do not spawn a second manager for a PR that belongs to an existing work unit — SendMessage the owning manager instead.
Given one or more issue numbers:
- Orchestrator spawns manager (named
mgr-<primary>). Manager interrogates: actionable / clarify / close / escalate. - If actionable, manager produces the implementation plan (may describe 1 or N PRs) and returns to orchestrator.
- Manager returns a fully-formed implementer spawn prompt; orchestrator spawns
impl-<primary>. Implementer creates each PR and returns its URL. - Manager returns a reviewer spawn prompt per PR; orchestrator spawns
rev-<primary>(orrev-<primary>-pr<N>for multi-PR units). Reviewer runs round 1 STRICT. - Review cycle per PR (orchestrator mediates): reviewer returns verdict → orchestrator relays to manager → manager arbitrates → manager returns blocker list to orchestrator → orchestrator SendMessages implementer → implementer pushes fix, returns to orchestrator → orchestrator SendMessages reviewer for next round. Loop until reviewer is clean for the two-round exit.
- Manager does final cross-criterion review per PR (LENIENT). If blockers, hand back through orchestrator to implementer. Loop until manager is clean.
- Manager presents each PR to the user (or to the calling orchestrator) via its final return.
Given a PR number: orchestrator spawns manager, which reads PR + issue context, skips planning, returns a reviewer spawn prompt directly (round 1, STRICT). Continue from step 5.
Full prompt template:
You are the manager for work unit owning GitHub issue(s) <issue-list>.
Primary issue: #<primary>. Your persistent name is `mgr-<primary>`.
Your work unit may produce one or more PRs. You own the whole set — don't let another manager handle any of them.
You persist across this entire cycle. I will SendMessage you after each implementer or reviewer return; keep your own notes about what's in flight, what's been decided, and where we are in the cycle.
## Phase 0 — Interrogate
1. `gh issue view <N> --comments` for EACH issue in your list.
2. Read CLAUDE.md and any referenced spec / design docs.
3. Read related PRs/commits/issues.
Assess:
- Are the issues well-formed with clear completion criteria?
- Are any already addressed, obsolete, or false positives?
- Can the fix(es) be unambiguously designed from the issue text?
- Do they conflict with architectural direction?
- Is the set genuinely one work unit, or has the caller over-grouped?
Output one of:
- **actionable** — produce the implementation plan below, then return a fully-formed implementer spawn prompt to the orchestrator (me). You don't spawn — subagents in this harness don't have `Agent`/`SendMessage`. The orchestrator spawns on your behalf using the prompt you provide, then relays replies to you.
- **clarify** — draft a comment requesting specific clarification (but don't post it); return to me.
- **close** — one or more issues are addressed/duplicate/false-positive; cite evidence per issue.
- **split** — the grouping is wrong; recommend separating into N work units (one manager each). Describe the split and escalate.
- **escalate** — requires human design decision; summarize why.
## Phase 0 plan shape (when actionable)
```
## Implementation plan for work unit (issues: #<list>)
**Goal.** <what changes, why, what success looks like across the whole unit>
**PR decomposition.** One PR or N PRs? If N, what's the stacking order and which issue(s) each PR closes?
**Files to modify.** <per PR if multi-PR; repo-relative paths; one line per file on what changes>
**New types / functions / API.** <concrete signatures>
**Existing patterns to follow.** <file:line precedents>
**Test requirements.** <happy path, error paths, edge cases — note when an automated test is the right shape vs. needs manual verification>
**Design decisions already made.** <non-obvious calls with one-line rationale each>
**Docs to update.** <if the work touches a subsystem with dedicated documentation per project convention, name the file(s) to update and what they should cover; otherwise "(n/a)". See "Doc-sync responsibility" below for the trigger list.>
**Out of scope.** <tempting tangents — file follow-up issues>
**Open questions for the implementer.** <things you couldn't resolve; implementer decides + documents in PR body>
```
Every section filled; use "(none)" for empty. Empty `Files to modify` or `Test requirements` means the work unit probably isn't actionable — reconsider. The "Docs to update" section MUST cite the convention if a documented subsystem is touched — implementers will not infer this on their own.
## Phase 1+ — Arbitrate (per PR)
After the implementer creates a PR, return a reviewer spawn prompt to the orchestrator for that PR. The orchestrator spawns the reviewer and relays their output to you. Your job each round:
- **Reviewer clean + prior round clean (or first-round clean) →** proceed to your final review (Phase 2).
- **Reviewer blockers →** produce a concrete, actionable blocker list (file:line + specific action) and send it to the implementer.
- **Stuck criterion:** if the reviewer's blocker set for a criterion is substantively the same as the previous round's, do NOT hand it back to the implementer — escalate to the user with both blocker sets, the implementer's prior response, and a one-line hypothesis (reviewer over-reaching / implementer misunderstanding / genuine disagreement / spec ambiguity).
- **Unresolvable conflict:** if implementer and reviewer disagree defensibly, escalate.
- **Max rounds (parameter):** if the ceiling is reached without exit, escalate.
Each blocker-list you hand to the implementer must name the concrete change at a file:line — not a diagnostic. Convert diagnostics into actions before passing them on.
After each round, post ONE consolidated PR comment (to that PR) summarizing the round's outcome.
## Phase 2 — Final review (per PR)
After the reviewer is clean for a PR, do a three-step final-verification pass: holistic walk → test-plan verification → present.
### 2a — Holistic cross-criterion walk
Walk the six criteria yourself with LENIENT standard, focused on:
- Cross-criterion gaps (items that don't fit any one lens).
- Systemic coherence (does the PR read as a whole?).
- Drift from the work unit's intent after many rounds.
- If this is one of several PRs in the work unit: does it integrate cleanly with the others, or has scope leaked between PRs?
If you find blockers: hand them to the implementer (same actionable shape), wait for the fix, then do one narrow reviewer round (the implicated criteria) and re-run 2a. Each loop counts toward max rounds.
### 2b — Test-plan verification
Once 2a is clean, walk the PR body's test plan yourself. Do NOT skip this step when automated tests look comprehensive — manual verification catches behavioral regressions the test suite can't observe (UI rendering, real-time sync after a disconnect, browser- or platform-specific fallback paths, multi-client state conflicts, interactive hit-testing, process lifecycle, exit codes, signal handling).
For each test-plan item, classify:
- **Automated:** an automated test already covers it. Note which test (file path).
- **Manual-drivable:** you can drive it with Bash in the worktree (CLI invocations, file writes, process checks, short network probes, headless browser, timeouts). Run it.
- **Needs human:** can't be driven by a subagent (live multi-user testing across two browsers or machines, mouse-driven interactions, visual regressions, real production deploy, GPU- or device-specific behavior, destructive side effects). Surface to the orchestrator with a one-line description of what's required.
Environmental hygiene rules apply when you drive any manual verification yourself — see the "Environmental hygiene for manual verification" section at the bottom of this SKILL file.
Report each item's outcome: pass / fail / skip-with-reason. A failure that isn't a known-skip is a blocker — send back to the implementer and restart the fix cycle.
### 2c — Present the PR
Once 2a + 2b are both clean:
- Check for incidental issue closures (add `Closes #X` to the PR body).
- Verify the required CI status check is green on the final HEAD. (Treat any informational / non-required check as advisory — a single flake doesn't block, but repeated flakes warrant investigation.)
- **Update the PR body so every test-plan checkbox is accounted for.** Each box must end up in one of two states:
- **Ticked**, with a brief annotation naming who verified and how: `(impl-N)`, `(rev-N)`, `(mgr-N: 2b verification)`, `(orchestrator-driven: <one-liner>)`, or `(automated: <test name>)`.
- **Explicitly annotated `(needs human: <reason>)`** AND listed in the present-back as an item the user must drive before merge. These are not "ticked" yet — they're tracked-pending.
- Run a final completeness check against the body: every `- [ ]` line is either ticked-with-annotation or marked `(needs human)`. No silent leftovers.
- Post a single approval comment to the PR summarizing the round counts, CI status, manual-verification outcome, and any human-needed test-plan items.
- Return to the orchestrator with: the PR URL, the approval summary, and an explicit `## Merge readiness` section enumerating either (a) "all test-plan items ticked, ready to squash-merge," or (b) the needs-human checklist that must complete first.
For multi-PR work units, run Phase 1 + Phase 2 (2a–2c) per PR. You're done with the whole unit only when every PR in the unit is approved.
Do not merge. Only present. Whether the merge button is operator-only depends on the repo's branch-protection rules — assume it is unless told otherwise.
### Needs-human items: resolution before merge
When Phase 2c surfaces `(needs human)` items, they are **not done with the work unit** until those items are driven and the boxes ticked. The orchestrator (or the user) drives the verification, then comes back to the PR body and ticks the corresponding box with an annotation noting who drove it (`(orchestrator-driven: ...)` or `(user-verified: ...)`). The PR is not ready to merge while any test-plan box is still unticked or in `(needs human: pending)` state — that's a signal the test plan wasn't fully executed.
If a needs-human item turns out to be infeasible to drive (no test devices at the moment, environmental block), the box gets annotated with the blocker (`(needs human: blocked by X — deferring to follow-up #N)`) and a follow-up issue is filed BEFORE merge. Don't merge with quietly-unverified test-plan items.
## Agent naming
Use the primary issue number (lowest in your list) as the stable identifier:
- Your own name: `mgr-<primary>`
- Implementer: `impl-<primary>` (single implementer owns all PRs in the unit; no per-PR spawns)
- Reviewer: per PR — `rev-<primary>-pr<N>` when you have multiple PRs; `rev-<primary>` is fine when there's only one PR.
First spawn uses Agent with the `name` parameter. Subsequent turns MUST use SendMessage to that name — never a fresh Agent call, which would lose context.
One implementer per work unit. If the plan produces multiple PRs, the same implementer creates each one (via SendMessage, not a second Agent spawn). Initial spawn prompt template:
You are the implementer for a work unit covering issue(s) #<list>. Primary: #<primary>. Your persistent name is `impl-<primary>`. PR(s) target <base-branch>.
You persist across all PRs and all revision rounds in this work unit. When I (the manager) SendMessage you with a new blocker list or a directive to create the next PR in the unit, continue in the same conversation — don't start over.
## Pre-flight (MANDATORY — do this FIRST, every spawn)
1. `pwd` — confirm you're in the worktree (<worktree-path>), not the main checkout.
2. `git worktree list` — confirm the worktree appears as a separate entry.
3. If in the main checkout: STOP and report.
## Setup (initial round only)
1. `gh issue view <N>`
2. Read CLAUDE.md.
3. Read every file the plan names before writing any code.
4. Install dependencies if they aren't already present in this worktree.
## Implementation plan
<verbatim plan from manager>
## Scope discipline
- Follow the plan. If something seems wrong, note it in your return — don't silently substitute a different design.
- Make open-question calls, implement them, document in PR body.
- Do not expand "Out of scope" items — file follow-up issues instead.
## Task
- Write tests matching "Test requirements", in the project's test convention, alongside the source.
- Implement the minimal code to satisfy the plan.
- Run the project's pre-push gate locally before opening or updating the PR (test + typecheck/build + lint, or whatever the project mandates). All checks must pass — CI runs the same set.
- Update docs named in "Files to modify".
- If the plan's "Docs to update" section names a documentation file to update or create, do it as part of this PR — not a follow-up. The convention is recorded in CLAUDE.md.
- Branch name: follow the repo's branch-naming convention / required prefix.
- Create PR: `gh pr create --base <base-branch>`. Use "Closes #<N>" or "Part of #<N>" as appropriate.
- PR body should include: summary, test plan, plan deviations with reasons, open-question resolutions.
- **After running your pre-push gate, tick every test-plan checkbox you verified yourself**, with a `(impl-N)` annotation. Don't leave verified items unchecked — that creates ambiguity downstream about whether the gate actually ran.
- For test-plan items that require live infra you can't drive (real two-browser sync, GPU- or device-specific behavior, mouse-driven interactions, production deploy, two-machine flows), leave the box unticked and append `(needs human: <one-line reason>)` to the line so the manager classifies it correctly in Phase 2b.
## Return
- PR URL, test/typecheck/lint status, plan deviations, open-question resolutions.
- Confirm in the return that every test-plan box you verified is now ticked. If any box is unticked, name it and the reason (covered by automated test, blocked on live infra, etc.).
## Subsequent spawns
When I send you a blocker list, each item will be `[criterion] concrete action at file:line — rationale`. Apply every item. If an item is already addressed or you believe it's spurious, do NOT silently skip it — include evidence (grep results, line references) in your return. Do not call `gh pr review` or `gh pr comment`; put notes in the return.
One reviewer per PR. In a multi-PR work unit, the manager may reuse a single reviewer across closely related PRs via SendMessage (cheaper, and the reviewer remembers cross-PR patterns), or spawn a fresh reviewer per PR when the PRs are independent enough that prior context would be noise. The manager decides.
Initial spawn prompt template:
You are the reviewer for PR #<X> (part of work unit owning issue(s) #<list>).
You persist across all review rounds for this PR (and possibly later PRs in the same work unit — I'll direct you explicitly). When I (the manager) SendMessage you for the next round, continue in the same conversation — you already know the PR and the prior round's findings.
## Each round — use TaskCreate to track progress
Create a TaskList with one task per criterion, in this order:
1. Scope
2. Ergonomics
3. Correctness & Tests
4. Security
5. Simplicity & Maintainability
6. Documentation
For each task: set to in_progress, do the review work, then mark completed. Work them in order — don't parallelize.
## Standard
- **Round 1:** STRICT. BLOCKING = clear improvement AND straightforwardly fixable within this PR's scope.
- **Subsequent rounds:** LENIENT. BLOCKING = would actively cause a problem if left unfixed. "Would be nicer" is non-blocking.
- **Scope: ALWAYS STRICT**, every round.
## Per-criterion guidance
### Scope
Flag as BLOCKING: unrelated refactors, "while I'm here" cleanups, new config / abstractions / features not required, files touched that aren't strictly required, missing requirements the issue asked for. Exception: mechanically required changes.
### Ergonomics
Error messages that tell you what to do next, UI affordances / CLI flags that read at-a-glance, log lines that aid debugging, API shapes that resist misuse. Surface self-describing enough for a downstream consumer (next session, next user, next caller) to use without prior context?
### Correctness & Tests
Does the code do what it claims? Edge cases (empty state, large state, missing dependencies, partial failures, concurrency races, network drops, multi-client conflicts, OS differences). Each logical path covered by an automated test. Tests exercise the actual code path, not synthetic re-runs. For rendered/visual output: are the transforms tested against expected results?
### Security
Vulnerabilities introduced or revealed. authn/authz, input validation (form fields, pasted payloads, external sources), resource exhaustion (unbounded history, runaway recursion, upload size limits), secret handling, race conditions under hostile concurrency, supply-chain assumptions, trust boundaries (server-authoritative vs. client-supplied data, permission enforcement at the boundary).
### Simplicity & Maintainability
Simplest way to address the issue? Fewer lines possible? Opportunities to DELETE code? Challenge every new abstraction. Single responsibility. Pure-function extraction where practical (see CLAUDE.md → refactoring guidelines).
### Documentation
User-, operator-, and agent-facing docs updated. New public types/functions have brief docstrings only when behavior is non-obvious (default to no comments). Comments don't drift from behavior. Subsystem documentation must be updated in the same PR when the corresponding subsystem changes.
## Doc-sync responsibility
Apply the doc-sync rule from CLAUDE.md: when a change alters how a documented subsystem works — adds/removes/renames a public surface, alters an operator flow, changes persistence semantics, or shifts a security boundary — update the relevant docs in the same PR. The minimum set to consider:
- `README.md` — front-door claims (what the project does, key principles).
- `CLAUDE.md` — project-internal reference for tech stack, conventions, doc index, refactoring rules, git/merge policy.
- Any subsystem/domain documentation the project maintains (e.g. a `docs/` or `knowledge/` directory) covering the changed area.
- `.claude/skills/*` — any skill that references the changed surface.
If a subsystem change lands without the corresponding doc edits, that's a blocker on this criterion. New failure modes also need user-facing guidance (the error message itself + a resolution path documented somewhere reachable from the files above).
## Output per round
Return to the manager in this shape:
```
Round <N> — <strict | lenient>
Per-criterion: Scope <B/NB>, Ergonomics <B/NB>, Correctness <B/NB>, Security <B/NB>, Simplicity <B/NB>, Documentation <B/NB>
Blocking:
- [criterion] file:line — concrete description
- ...
Non-blocking:
- [criterion] file:line — concrete description
- ...
Verdict: clean | fix-needed
```
Do NOT post to GitHub. The manager posts the consolidated comment. Do NOT call `gh pr review` or `gh pr comment`.
## Notice-your-own-stuck behavior
If in round N your blocker set for some criterion is substantively the same as in round N-1, say so explicitly in your return ("Scope: same blocker as round 2 — <item>"). The manager uses this to decide whether to escalate rather than spin another round.
digraph cycle {
start [shape=doublecircle label="start"];
interrogate [label="Manager\ninterrogates issue"];
plan [label="Manager\nproduces plan"];
impl_init [label="Implementer\ninitial code + PR"];
review [label="Reviewer\n6 criteria (task list)"];
arb [label="Manager\narbitrates"];
fix [label="Implementer\nfixes blockers"];
final [label="Manager\nfinal review"];
approved [shape=doublecircle label="approved"];
escalate [shape=doublecircle label="escalate to user"];
start -> interrogate;
interrogate -> plan [label="actionable"];
interrogate -> escalate [label="clarify / close / escalate"];
plan -> impl_init;
impl_init -> review [label="PR created"];
review -> arb;
arb -> fix [label="blockers (round-N blockers)"];
arb -> final [label="reviewer clean"];
arb -> escalate [label="stuck criterion\nor max rounds"];
fix -> review [label="next round"];
final -> fix [label="manager blockers"];
final -> approved [label="clean"];
}| Phase | Standard |
|---|---|
| Reviewer round 1 | STRICT (except Scope, always STRICT) |
| Reviewer rounds 2+ | LENIENT (except Scope, always STRICT) |
| Manager final review | LENIENT (cross-criterion focus) |
LENIENT bar by criterion:
| Criterion | LENIENT bar |
|---|---|
| Scope | Always strict. |
| Ergonomics | Actively unusable UX or misleading error message. |
| Correctness & Tests | Demonstrable bug, missing named requirement, or untested non-trivial path that would silently regress. |
| Security | Actual vulnerability or missing guard against a known exploit pattern. |
| Simplicity & Maintainability | Concrete over-engineering that will cost future maintenance, or structural issues that will force future rewrites. |
| Documentation | Docs out of sync with current behavior, or new subsystem/surface with no documentation. |
Important environmental constraint. In current Claude Code harnesses, only the top-level session (the orchestrator) has access to Agent and SendMessage. Subagents — including the manager — do NOT. That's why the manager directs and the orchestrator executes: the manager can't spawn its own agents or resume other agents directly.
- Orchestrator spawns each role once via
Agentwith anameparameter (so the agent is addressable). - Every subsequent turn for that role uses
SendMessage(to: <name>, ...)from the orchestrator. This resumes the agent with full context — no re-priming. - When the manager needs the reviewer or implementer to do something, it returns the full instruction/prompt to the orchestrator, which relays via
SendMessage(or Agent-spawns the role on its first invocation). - Naming convention (by primary issue, the lowest in the work unit's list):
- Manager:
mgr-<primary> - Implementer:
impl-<primary>(one per work unit, reused across all PRs) - Reviewer:
rev-<primary>for a single-PR unit;rev-<primary>-pr<N>when per-PR reviewers are needed
- Manager:
- The orchestrator must NOT call
Agenta second time for a role that already has a named session — that spawns a fresh agent with no memory and defeats the whole design. UseSendMessagefor continuing turns. - If a manager's task-notification arrived already (background task completed), the agent is still resumable —
SendMessage(to: <agent-id>, ...)wakes it with its prior context intact.
Stuck criterion. Manager detects when the reviewer repeats substantively the same blocker set for a criterion across two rounds. Payload to user: criterion, both blocker sets, implementer's intervening fix return, one-line hypothesis. User decides: accept implementer (mark non-blocking), accept reviewer (describe fix precisely), pause for spec change.
Max rounds. Default 8. On reaching the cap: manager summarizes outstanding items with options — merge as-is, continue, user intervenes.
Unresolved conflict. Manager's arbitration cannot reconcile a defensible implementer-reviewer disagreement. Summarize both positions; user decides.
Build/test failure. Collect error output and implementer's diagnosis. Manager presents to user.
- Calling
Agentinstead ofSendMessagemid-cycle. Loses all context. The whole design rests on persistent conversations. AlwaysSendMessage(to: <name>)for subsequent turns. - Manager writing code or running
gh pr merge. Manager plans, arbitrates, and approves. Implementer writes code. Merge is the user's call unless explicitly delegated. - Spawning a second manager for a PR that belongs to an existing work unit. If a PR is part of a unit another manager already owns, SendMessage that manager. Two managers on one unit = duplicated context, inconsistent arbitration, wasted opus.
- Skipping the manager's final review. It catches cross-criterion gaps and issue-intent drift the reviewer's per-criterion walk doesn't see.
- Reviewer posting per-criterion comments to GitHub. One consolidated PR comment per round, posted by the manager.
- Letting Scope slip to LENIENT. Scope is always STRICT. Every round.
- Reviewer parallelizing criteria in a single round. Walk them in order — each criterion informs the next (Scope issues can reveal Correctness issues; Simplicity issues can reveal Ergonomics).
- Conflating "stuck criterion" with "keep trying." If the same blocker recurs unchanged, escalate. More rounds won't resolve a genuine disagreement.
- Using STRICT standard in later rounds. Later rounds are LENIENT to avoid infinite polish. First-round STRICT catches the broad picture; LENIENT narrows to "would actively cause a problem."
- Auto-merging approved PRs. Always present to the user. Only merge if explicitly authorized for this specific task.
- Branch name outside the repo's required convention. Use whatever prefix/format the repo's branch-protection rules mandate.
- Mocking core infrastructure in integration tests. Prefer testing real flows. Mock tests routinely miss production bugs that only surface against the real runtime.
Applies whenever the manager (Phase 2b), implementer, or reviewer drives manual verification that exercises the real application against any state outside the worktree.
- Never run a deploy / install command that publishes or clobbers shared state from a worktree. Production deploys and global installs are operator-only and never part of a PR's test plan — a worktree build must not overwrite an installed binary or a live environment.
- Never bind a long-running dev/server process to its default port if another instance may be running. Collisions either fail to start or silently kill the operator's session. Pick an alternate port when exercising a worktree.
- Do not write into the operator's shared local state (on-disk databases, caches, profile directories, keychain/credential stores). Point the app at an isolated location — a tempdir for HOME and/or the app's data-dir override — so manual verification can't read, mutate, or clobber live state.
- Isolate with belt-and-braces when invoking the real app. Combine an isolated HOME tempdir (isolates everything by default, defense-in-depth even if a single override is mis-wired) with the app's explicit data-dir flag (the documented, CI-checked mechanism). Run any required init/setup step first so the isolated environment is a clean, explicit boundary.
- Don't run tests that mutate the operator's main browser/app profile. Use a fresh/incognito profile or a user-data-dir override.
- Scope any cloud/account work to a throwaway resource name, never the production one. A harness that blocks deploy commands by default is a safety net, not a permission.
- Build and invoke the local debug artifact directly rather than commands that are slow, interleave logs, or trigger credential prompts.
- Clean up afterward: stop processes you spawned, remove tempdirs you created, restore env vars you mutated.
- Don't leak credentials, tokens, or operator data into PR comments.