claude-notion: reviewer self-destruct + tight-PR norm + PM reviewer perms

[martin-conur]

What & why

Three workflow improvements validated while dogfooding the claude-notion loadout on a downstream project, ported back to the framework.

Changes (claude-notion loadout)

• commands/reviewer.md — the reviewer is now single-shot + self-destructing: right after it posts the PR comment and radios PM, it runs task-done --remove-worktree instead of idling with the tab open. Its analysis lives durably in the PR comment, so there's no reason to keep the worktree/tab around. Also adds a tight-PR norm: frame every finding as fix-in-this-PR, never "defer to a follow-up" for in-scope work.
• commands/pm.md — reflects the self-cleaning reviewer (no manual cleanup, nothing to scroll back to); adds the tight-PR forwarding norm (forward all in-scope findings — blockers + nits — in one changes-requested round; only genuinely out-of-scope work gets deferred, and the PM grooms that into a ticket); notes that pre-allowing Bash(task-reviewer *) lets PM dispatch reviewers hands-off.
• steering/notion-workflow.example.md — new "Reviewer role (single-shot, self-cleaning)" and "Tight-PR norm" sections documenting the above.

Notes

• Scoped to the claude-notion loadout only. The same generic change applies to claude-gh / claude-local / claude-jira / the .claude dogfood copy — to follow in a separate pass.
• The Bash(task-reviewer *) permission is per-project (the loadout ships no settings.json), so this PR is prompt/doc-only; projects add the allow entry to their own .claude/settings.json.
• No project-specific identifiers in the diff (placeholders only).

🤖 Generated with Claude Code

[martin-conur]

PR Review — #160: claude-notion: reviewer self-destruct + tight-PR norm + PM reviewer perms

Diff-only review — no spec issue linked; reviewed against the PR description's stated goals.

---

Spec compliance

No linked spec issue. The PR description is clear and self-contained: three workflow improvements (auto-destruct, tight-PR norm, PM permission hint), scoped to the claude-notion loadout only. The diff matches the description. Nothing is missing relative to what was promised.

---

Code-review findings

Finding 1 — --no-auto description is misleading after auto-destruct (nit)

File: claude-notion/commands/pm.md

The line:

Pass --no-auto if you want to babysit a specific review.

…implies the reviewer tab stays open for post-review inspection. It no longer does — the reviewer now unconditionally self-destructs after posting the comment and radioing PM. In --no-auto mode, the only difference is that Claude pauses for permission prompts on each mutation (including the task-done call); the tab still closes once those are approved. Users expecting a persistent tab for inspection will be surprised. The description should be updated to reflect what --no-auto actually buys in the new model: approval of each mutation step, not a keep-alive tab.

---

Finding 2 — Auto-destruct is unconditional; a failed gh pr comment silently loses the review (changes-requested)

File: claude-notion/commands/reviewer.md

Step 7 says:

Immediately after the PR comment is posted AND PM is radioed, run task-done --remove-worktree

The AND reads as sequential steps, not a success condition. If gh pr comment fails (expired auth, rate limit, transient network error), the reviewer will still proceed to radio PM and run task-done --remove-worktree. PM receives a review-complete-* ping with no corresponding PR comment. The worktree is destroyed, taking the unreported analysis with it. The failure is silent — neither the PM nor the worker has any indication the review content was never posted.

Suggested fix: Make the guard explicit. Add a conditional: only radio PM and run task-done when the gh pr comment call exits successfully. If the comment fails, halt and leave the tab open (forfeiting the auto-destruct) so the analysis is not lost. Something like:

If the gh pr comment call fails, do NOT radio PM or run task-done — stop, report the error, and leave the tab open. The auto-destruct is contingent on a successful post.

---

Finding 3 — Tight-PR norm duplicated across three files; future edits must stay in sync (nit)

Files: reviewer.md, pm.md, notion-workflow.example.md

The tight-PR norm is now described independently in all three files. If the policy changes (e.g., adding a blanket exception for dependency bumps), all three need to be updated in the same commit or they'll silently diverge, giving the reviewer and PM conflicting guidance. The reference doc (notion-workflow.example.md) is the natural single source of truth; reviewer.md and pm.md could cross-reference it rather than restate the full rule. That said, keeping it close to the instructions for each role is a common trade-off — call this a low-priority nit.

---

Out-of-scope observation (not a blocker)

The PR body explicitly notes the same changes need to be applied to the claude-gh, claude-local, claude-jira, and .claude dogfood loadouts in a separate pass. That gap is acknowledged and tracked. The current review instance is itself running from the .claude dogfood copy, which still has the old "idle" behavior — no action needed from this PR, just confirming the follow-up is load-bearing.

---

Verdict

changes-requested — one substantive issue (Finding 2) and two nits.

• Finding 2 is the blocker: unconditional auto-destruct on a failed comment post silently drops the review with no recovery path. Worth one focused fix before merge.
• Finding 1 (misleading --no-auto doc) should be folded into the same commit.
• Finding 3 (three-file duplication) is low-priority — fine to address in the same pass that updates the other loadouts, or leave as-is with an explicit acknowledgment in notion-workflow.example.md that it's the authoritative definition.

---

🤖 Reviewed with Claude Code