Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .claude/commands/reviewer.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,9 @@ You are a single-shot PR reviewer worker. `task-reviewer` spawned this tab with
4. Run the `code-review` skill on the diff.
5. Post **one** substantive PR comment with the full analysis (spec compliance + code-review findings + verdict).
6. Radio PM back with `review-complete-clean` or `review-complete-with-findings` and a short summary.
<!-- region:reviewer-destruct-summary -->
7. **Auto-destruct.** Your analysis lives in the PR comment (durable) — the tab does not need to stay open. Immediately after the PR comment is posted AND PM is radioed, run `task-done --remove-worktree` as your final action to remove this review worktree and close the tab. Do NOT idle.
<!-- endregion:reviewer-destruct-summary -->

Refer to the project's `.claude/gh-workflow.md` for GitHub owner/repo conventions. Use the `gh` CLI for PR + issue I/O: `gh issue view`, `gh pr view`, `gh pr diff`, `gh pr comment`. Read-only `gh` patterns are pre-allowed in `.claude/settings.json`; mutations stay confirmation-gated.

Expand Down Expand Up @@ -47,6 +49,7 @@ $ARGUMENTS
- **Code-review findings** — correctness, security, edge cases, anything substantive from the skill's output.
- **Verdict** — `clean`, `clean-with-nits`, or `changes-requested`. Be explicit.

<!-- region:reviewer-post-radio-destruct -->
**Tight-PR norm — prefer fix-in-this-PR over deferral.** Frame every finding as something the worker should fix *in this PR*, not as a follow-up ticket, whenever the fix is feasible within the PR's scope. Do NOT suggest "defer to a follow-up" / "track separately" for anything in-scope — that's a deferral the team is trying to avoid. Only flag a deferral when the work is genuinely out of this PR's scope (a separate subsystem, a large refactor), and say so explicitly — PM will decide whether to groom it into a ticket. Default: tight PR, everything fixed before merge.
7. Post the comment — **on stdin, never through a double-quoted shell string.** Inside `"…"` the shell command-substitutes every backticked token in your prose: `` `pwd -P` `` posts as your working directory, `` `radio orphans` `` posts as that command's output, and a backticked identifier that names no command — `` `$SOME_VAR` ``, `` `_some_helper()` `` — posts as **nothing at all**, deleting the finding's subject and leaving a sentence that reads as if a word were missing. Findings here name functions, variables and paths in inline code constantly, and this comment is the only thing that outlives your tab, so feed the body in with `--body-file -` and a **quote-delimited** heredoc:
```bash
Expand All @@ -71,6 +74,7 @@ $ARGUMENTS
9. **Auto-destruct.** Run `task-done --remove-worktree` as your final action — it removes this review worktree and closes the tab, and `--remove-worktree` skips the confirmation prompt, so it needs no input from you. PM reads your verdict from the PR comment + the radio ping (on its next `radio check`), never from this tab, so there is nothing to keep open. Self-cleaning is mandatory, not optional.

**The one thing that is not in the PR comment** is step 8's delivery outcome. If `radio send` reported anything other than a landed delivery — `WARNING — no session for pm-…`, `WARNING — pm-… looks dead …`, or `queued — pm is idle but wake failed …` — that warning exists nowhere but this tab, and destroying it would strand the verdict unread. In that case only: say so to the user, and hold the tab until they have seen it. Otherwise, destruct.
<!-- endregion:reviewer-post-radio-destruct -->

### Authority boundaries

Expand Down
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,12 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/),
## [Unreleased]

### Added
- **kiro-notion and kiro-local ship a reviewer agent, and every kiro reviewer now self-destructs like the claude one (#320).** Only kiro-gh had `agents/reviewer.json`, so `task-reviewer` refused on kiro-notion and kiro-local ("Reviewer support currently ships for …"), the one gap in the reviewer column; it was hit on a kiro-notion repo, where the way round was `--agent claude` on a dual install. Both loadouts now ship one, ported from kiro-gh's and adjusted the way each loadout's `worker.json` is: kiro-notion's carries the `@notion` tool and the `notion` MCP server and reads the spec page passed as the second arg through it; kiro-local's reads the `tasks/` file named by a slug or path from disk, with no tracker MCP. Neither auto-detects a spec from `Closes #N`, which is GitHub-only. `task-init` installs the new agent with its radio hooks merged in, as it does every file under `agents/`, so `task-config add assistant kiro` on a notion or local repo brings it along.
- **The kiro reviewer self-destructs.** kiro-gh's reviewer still ended "Idle — the tab stays open … Do NOT auto-cleanup": #162 made the claude reviewers single-shot and self-cleaning and left kiro out. All three kiro reviewers now end the way the claude ones do: post the comment on stdin, radio the verdict and read `radio send`'s outcome, then `task-done --remove-worktree`, holding the tab open only when the verdict never reached PM. They also gain the tight-PR norm. That prose is now one drift-checked block in all eight reviewer prompts (the four claude `commands/reviewer.md`, the dogfood `.claude/commands/reviewer.md` and the three kiro `agents/reviewer.json`), the new `reviewer-contract` group in `tools/check-drift.sh`, so the two assistants cannot part ways on it again.
- **`tools/check-drift.sh` reads prompts.** A kiro agent config (`*.json`) is now checked on its decoded `prompt`, whose lines exist only once the JSON string is decoded, and a region may be marked `<!-- region:NAME -->` / `<!-- endregion:NAME -->` where a `# region:` line would render as a heading in a markdown prompt.
- **`task-reviewer`'s refusal** no longer lists loadouts: it is still keyed on the shipped file, so it now names the missing `<impl>/agents/reviewer.json` and the `--agent claude` way round. No shipped loadout hits it.
New tests: the kiro-notion and kiro-local launches pass their spec through (stubbed zellij and kiro-cli), a `Closes #N` is not their spec, the refusal on a tree copy with the agent taken out, `task-init` installing `reviewer.json` with its register hook on both, `add assistant kiro` bringing it along, and drift-checker cases for JSON prompts, comment sentinels and a broken kiro copy of the reviewer contract. The prompt sweeps (`prose_body_file`, `worker_checklist`, `kiro_radio_pull`) now cover all eight reviewers. Removing either new `reviewer.json` turns its launch tests red. README, the kiro-gh workflow doc (its reviewer tab no longer "stays open") and the kiro-notion / kiro-local workflow docs, which had no reviewer paragraph at all, describe it.
**Upgrading: re-run `task-init kiro-notion` / `task-init kiro-local`** to install the reviewer agent, and `task-init kiro-gh` so its reviewer self-destructs. The claude `commands/reviewer.md` changed only by the two invisible region comments, so a claude re-run is optional.
- **PM handoff across assistants: a board digest at a fresh PM start, an optional handoff note, and no second live PM (#301).** With both assistants installed (#293) the PM role moves from claude to kiro and back. The inbox carried over, since the address is the same, but the working picture did not: a new PM started blind and spent tokens re-reading the tracker to rebuild what task-monitor already knew.
- **Board digest.** A fresh `pm-*` register now prints `task-monitor-snapshot --local --brief` under the offline-backlog summary: attention signals first, then in flight (ref, title, PR, review state), the queue with its notes, and the last 24h of merges with each PR summary's first sentence. `--brief` is a new output mode; the text comes from `lib/monitor-digest.jq`, and `tm_board_digest` in `lib/monitor-text.sh` caps it at 40 lines, the last one saying how many were cut. `--local` means no forge call, so start-up stays fast and works offline. It is printed under the same gate as the backlog summary (`fresh=1`, not an alias), so never on a `/compact` or `/clear` re-register, and never for a worker. Hook stdout reaches the model on claude and kiro alike. `TASK_FORCE_NO_PM_DIGEST=1` turns it off.
- **Handoff note.** `task-monitor-queue handoff write` saves 5–10 lines from stdin to `$TASK_FORCE_HOME/monitor/<repo>/handoff.md`, with `written_at`, the PM role and its agent in frontmatter; `handoff show` prints it and `handoff clear` removes it. It is a `task-monitor-queue` subcommand rather than a new command, so there is nothing new to install. The snapshot gains a `handoff` field (`{text, written_at, role, agent}` or null). The next fresh PM start prints the note under the digest with its age, capped at 20 lines, and it stays until overwritten. task-monitor shows it as the repo view's last row, collapsed to its first line with its agent, age and length, and `⏎` expands it.
Expand Down
14 changes: 7 additions & 7 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -1289,14 +1289,14 @@ task-reviewer <pr-url-or-number> [<spec-identifier>]
`task-reviewer` spawns a fresh zellij tab + git worktree on the PR's head ref, then runs the `/reviewer` slash command (or the kiro `reviewer` agent) inside it on Sonnet 5.5 — `ANTHROPIC_MODEL=claude-sonnet-5-5` by default on claude (pre-set the env var to override), `--model claude-sonnet-5.5` on kiro (`-m` to override). The kiro reviewer also reads `$TASK_WORK_MODEL` ahead of that default, so a worker override exported in your shell dispatches reviewers on the same model, at its price (`claude-opus-5.5` is 2.00x kiro credits, Sonnet 1.30x); unset it or pass `-m` to keep reviewers on Sonnet. Claude Code and kiro-cli spell the same model differently, hyphens versus dots. The PM's tab stays focused. Add `Bash(task-reviewer *)` to the project's `.claude/settings.json` `permissions.allow` so the PM can dispatch a reviewer without a permission prompt.

The reviewer:
1. Reads the spec (passed as the second arg — issue number / URL for `claude-gh` / `kiro-gh`; Jira key for `claude-jira`; Notion page URL for `claude-notion`; local task slug or path for `claude-local`). On `claude-gh` / `kiro-gh` only, the wrapper also auto-detects from the PR body's first `Closes #N` / `Fixes #N` / `Resolves #N` line (case-insensitive); non-gh loadouts require the spec identifier explicitly because PR bodies don't carry their tracker's linking convention.
1. Reads the spec (passed as the second arg — issue number / URL for `claude-gh` / `kiro-gh`; Jira key for `claude-jira`; Notion page URL for `claude-notion` / `kiro-notion`; local task slug or path for `claude-local` / `kiro-local`). On `claude-gh` / `kiro-gh` only, the wrapper also auto-detects from the PR body's first `Closes #N` / `Fixes #N` / `Resolves #N` line (case-insensitive); non-gh loadouts require the spec identifier explicitly because PR bodies don't carry their tracker's linking convention.
2. Reads the PR diff + comments.
3. Cross-checks the diff against the spec, then runs the `code-review` skill on top (claude variants — kiro stays prompt-driven).
3. Cross-checks the diff against the spec, then runs the `code-review` skill on top (claude variants — kiro stays prompt-driven). The kiro-notion reviewer reads its spec page through the Notion MCP, as its worker does; the kiro-local one reads the task file from disk.
4. Posts **one** thorough PR comment with spec-compliance findings, code-review findings, and a verdict (`clean`, `clean-with-nits`, or `changes-requested`) — via `gh pr comment --body-file -` off a quoted heredoc, so the backticked code its findings are made of survives the shell.
5. Radios PM back with `review-complete-clean` or `review-complete-with-findings`.
6. **Auto-destructs** — on the claude loadouts the reviewer's final action is `task-done --remove-worktree`, which removes the review worktree and closes its own tab. It is single-shot and self-cleaning: the analysis is durable in the PR comment, so nothing is lost when the tab goes. The kiro `reviewer` agents still idle with the tab open — clean those up by hand.
6. **Auto-destructs** — on every loadout, claude and kiro, the reviewer's final action is `task-done --remove-worktree`, which removes the review worktree and closes its own tab. It is single-shot and self-cleaning: the analysis is durable in the PR comment, so nothing is lost when the tab goes. Until #320 the kiro reviewer idled with its tab open instead. Posting on stdin, the radio verdict and the self-destruct are one block of prose in all eight reviewer prompts, drift-checked by `tools/check-drift.sh` (the `reviewer-contract` group), so the two assistants cannot part ways on it again.

PM still decides whether to merge or request changes — the reviewer never approves, merges, closes, or mutates Status. Re-read a review with `gh pr view <N> --comments` rather than hunting for a tab that cleaned itself up. The one case a claude reviewer keeps its tab is a failed radio delivery: the verdict never reached PM, and that outcome is the one thing its PR comment does not record, so it holds open to say so.
PM still decides whether to merge or request changes — the reviewer never approves, merges, closes, or mutates Status. Re-read a review with `gh pr view <N> --comments` rather than hunting for a tab that cleaned itself up. The one case a reviewer keeps its tab is a failed radio delivery: the verdict never reached PM, and that outcome is the one thing its PR comment does not record, so it holds open to say so.

**Tight-PR norm.** The reviewer frames every finding as fix-in-this-PR rather than "defer to a follow-up", and the PM forwards all in-scope findings — blockers *and* nits — in a single `changes-requested` round. Only work genuinely out of the PR's scope gets deferred, and the PM grooms that into a ticket during the session instead of leaving it as a loose "later."

Expand All @@ -1305,12 +1305,12 @@ task-reviewer 42 # claude-gh / kiro
task-reviewer https://github.com/owner/repo/pull/42 # PR by URL
task-reviewer 42 38 # claude-gh / kiro-gh: PR + GitHub issue explicit
task-reviewer 42 PROJ-123 # claude-jira: PR + Jira key explicit (no auto-detect)
task-reviewer 42 https://notion.so/page-id # claude-notion: PR + Notion page URL explicit
task-reviewer 42 042-add-login # claude-local: PR + local task slug explicit
task-reviewer 42 https://notion.so/page-id # claude-notion / kiro-notion: PR + Notion page URL explicit
task-reviewer 42 042-add-login # claude-local / kiro-local: PR + local task slug explicit
task-reviewer 42 --no-auto # opt out of auto-permission (interactive review)
```

`--auto` is the default — the reviewer's authority list rules out merge / push / approve / close / Status, so auto-permission is safe for the review flow, and matches the "dispatch and walk away" intent of the command. Pass `--no-auto` to drop into the interactive permission-prompt mode. Radio auto-submit is a separate setting, also on by default, and `--no-auto` no longer switches it off (#246): clicking through a reviewer's tool calls says nothing about whether PM's pings into it should need an Enter. `--no-auto-submit` is its own opt-out. Kiro reviewers default to `--trust-all-tools` for the same reason; opt out with `--no-trust-all`, which likewise leaves auto-submit on (#146). Both agents run the one canonical `bin/task-reviewer`, so a kiro-gh reviewer resolves its spec exactly as a claude-gh one does.
`--auto` is the default — the reviewer's authority list rules out merge / push / approve / close / Status, so auto-permission is safe for the review flow, and matches the "dispatch and walk away" intent of the command. Pass `--no-auto` to drop into the interactive permission-prompt mode. Radio auto-submit is a separate setting, also on by default, and `--no-auto` no longer switches it off (#246): clicking through a reviewer's tool calls says nothing about whether PM's pings into it should need an Enter. `--no-auto-submit` is its own opt-out. Kiro reviewers default to `--trust-all-tools` for the same reason; opt out with `--no-trust-all`, which likewise leaves auto-submit on (#146). Both agents run the one canonical `bin/task-reviewer`, so a kiro reviewer resolves its spec exactly as the claude one on the same tracker does. Every loadout but the missing kiro-jira (#92) ships a reviewer; kiro-notion and kiro-local gained theirs in #320, and before that `task-reviewer` refused there (on a dual repo, `--agent claude` was the way round).

---

Expand Down
24 changes: 13 additions & 11 deletions bin/task-reviewer
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,11 @@ set -eEuo pipefail
# task-reviewer — canonical entry point for all loadouts (#170).
#
# One body for every loadout that ships a reviewer: the four claude ones
# (formerly 4 byte-identical per-loadout copies) and kiro-gh, whose own copy
# was folded in here (#146). The two agents differ only in their flags, their
# preflight and the launch line, which branch on AGENT below; spec resolution
# (formerly 4 byte-identical per-loadout copies) and the three kiro ones —
# kiro-gh, whose own copy was folded in here (#146), and kiro-local /
# kiro-notion, which gained a reviewer agent in #320. The two agents differ
# only in their flags, their preflight and the launch line, which branch on
# AGENT below; spec resolution
# is the tracker module's. Impl resolution comes from lib/detect-impl.sh —
# override with --impl <name> or AW_IMPL=<name>.

Expand Down Expand Up @@ -141,15 +143,15 @@ fi

AGENT="${IMPL%%-*}"

# A kiro loadout has a reviewer exactly when it ships the agent config:
# kiro-gh does, kiro-local / kiro-notion do not yet. Keyed on the file rather
# than an impl list so the loadout that adds one needs no edit here. Surfaced
# explicitly rather than as aw_require_kiro_agent's "run task-init" remedy —
# the impl is configured, re-running task-init would install no reviewer.
# A kiro loadout has a reviewer exactly when it ships the agent config. All
# three do since #320; keyed on the file rather than an impl list so a new
# loadout (#92) is covered the day it ships one, and refuses until then.
# Surfaced explicitly rather than as aw_require_kiro_agent's "run task-init"
# remedy — the impl is configured, re-running task-init would install no reviewer.
if [[ "$AGENT" == kiro && ! -f "$AW_ROOT/$IMPL/agents/reviewer.json" ]]; then
echo "Error: task-reviewer is not available for impl '$IMPL'." >&2
echo " Reviewer support currently ships for claude-gh, claude-jira," >&2
echo " claude-notion, claude-local, and kiro-gh." >&2
echo "Error: task-reviewer is not available for impl '$IMPL':" >&2
echo " it ships no kiro reviewer agent ($IMPL/agents/reviewer.json)." >&2
echo " On a repo with both assistants installed, --agent claude reviews it." >&2
exit 1
fi

Expand Down
Loading
Loading