task-reviewer: tracker-aware spec-issue cross-check (claude-jira/notion/local) (#144)

CHANGELOG.md

@@ -6,6 +6,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/),
 
 ## [Unreleased]
 
+### Fixed
+
+- **`task-reviewer`'s spec-issue cross-check now works on `claude-jira` / `claude-notion` / `claude-local`.** The dispatcher and the `/reviewer` prompt were both GitHub-only: `parse_issue_number` rejected anything that wasn't a bare integer or `github.com/.../issues/N`; PR-body auto-detect only matched `Closes #N`; the URL-synthesis step always emitted a GitHub issues URL; and the prompt hardcoded `gh issue view <M>` for spec lookup. Net result: in any non-gh loadout, `task-reviewer <pr> <spec-id>` silently degraded to a diff-only review. The dispatcher is now loadout-aware (detects from its own file path) — `claude-gh` keeps numeric / URL parsing and PR-body auto-detect; `claude-jira` / `claude-notion` / `claude-local` accept any non-empty 2nd positional arg as an opaque spec identifier (Jira key, Notion URL, local task slug) and pass it through to `/reviewer` unchanged. The 4 dispatcher bodies stay byte-identical (drift-checked). Each loadout's `commands/reviewer.md` now points the spec-lookup step at the right tool: GH stays on `gh issue view`; `claude-jira` uses `mcp__atlassian__getJiraIssue`; `claude-notion` uses `mcp__notion__notion-fetch`; `claude-local` reads `tasks/<id>-<slug>.md` via the `Read` tool. With no 2nd arg in a non-gh loadout, the wrapper emits a clear "PR-body auto-detect is GitHub-only — pass the spec id explicitly" advisory and proceeds diff-only. **Upgrading**: re-run `task-init <loadout>` after pulling this change so the updated `/reviewer` prompt lands in `.claude/commands/`. (#144)
+
 ### Changed
 
 - **`task-reviewer` redesigned from a long-lived listener-tab model into a dispatch-style worker.** New surface: `task-reviewer <pr-url-or-number> [<issue-url-or-number>] [--no-auto] [--no-launch]` (claude variants) / `[--no-trust-all] [--no-launch]` (kiro). Each invocation spawns a fresh zellij tab + git worktree on the PR's head ref, runs a thorough review (spec-compliance cross-check + `code-review` skill on the diff), posts a single PR comment with findings, and radios PM back with `review-complete-clean` or `review-complete-with-findings`. **Auto-permission is the default** — the `/reviewer` prompt's authority boundaries rule out merge / push / approve / close / Status edits, so hands-off dispatch is safe; pass `--no-auto` (or `--no-trust-all` for kiro) to drop into the interactive permission path. PM no longer needs a pre-spawned reviewer tab — the previous no-arg, in-place tab takeover (shipped in #135, rolled out in #137) is gone. Affects `claude-gh`, `claude-jira`, `claude-notion`, `claude-local`, and `kiro-gh`. The per-loadout `bin/task-reviewer`, the `/reviewer` slash command + kiro agent, PM's reviewer-delegation guidance, and the README / workflow docs were all rewritten. (#138)

README.md

@@ -504,13 +504,13 @@ On its next turn the worker sees the ping, sets the project Status field to `Don
 To shift PR review off the PM's (Opus) tab and onto a cheaper Sonnet model, dispatch a one-shot reviewer worker per PR:
 
 ```bash
-task-reviewer <pr-url-or-number> [<issue-url-or-number>]
+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 (`ANTHROPIC_MODEL=claude-sonnet-4-6` by default — pre-set the env var to override). The PM's tab stays focused.
 
 The reviewer:
-1. Reads the spec issue (passed as the second arg, or auto-detected from the PR body's first `Closes #N` / `Fixes #N` / `Resolves #N` line, case-insensitive).
+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.
 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).
 4. Posts **one** thorough PR comment with spec-compliance findings, code-review findings, and a verdict (`clean`, `clean-with-nits`, or `changes-requested`).
@@ -519,9 +519,12 @@ The reviewer:
 PM still decides whether to merge or request changes — the reviewer never approves, merges, closes, or mutates Status. The tab stays open showing the analysis; clean up the worktree later with `task-done --remove-worktree`.
 
 ```bash
-task-reviewer 42                                              # PR by number, auto-detect issue (auto-permission default)
+task-reviewer 42                                              # claude-gh / kiro-gh: PR by number, auto-detect issue
 task-reviewer https://github.com/owner/repo/pull/42           # PR by URL
-task-reviewer 42 38                                           # PR + spec issue explicit
+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 --no-auto                                    # opt out of auto-permission (interactive review)
 ```
 

claude-gh/bin/task-reviewer

@@ -35,11 +35,17 @@ PR comment, and radios PM back with a verdict — clean or with findings.
 Positional arguments:
   <pr-url-or-number>      The PR to review. Accepts a GitHub PR URL or a bare
                           number (resolved in the current repo's `origin`).
-  <issue-url-or-number>   Optional — the spec issue this PR claims to close.
-                          If omitted, auto-detected from the PR body's
-                          `Closes #N` / `Fixes #N` / `Resolves #N` line.
-                          With no detectable issue, the reviewer proceeds with
-                          a diff-only review.
+  <spec-identifier>       Optional — the spec this PR claims to close.
+                          Shape depends on the loadout:
+                            claude-gh     — issue number or GitHub issue URL.
+                                            If omitted, auto-detected from the
+                                            PR body's Closes/Fixes/Resolves #N.
+                            claude-jira   — Jira issue key (e.g. PROJ-123).
+                            claude-notion — Notion page URL.
+                            claude-local  — local task slug or filename.
+                          PR-body auto-detect is GitHub-only — non-gh loadouts
+                          must pass the 2nd arg explicitly. With no spec id,
+                          the reviewer proceeds with a diff-only review.
 
 Options:
       --no-auto           Opt out of --permission-mode auto. Default is auto
@@ -88,6 +94,15 @@ done
 
 [[ ${#POSITIONAL[@]} -lt 1 ]] && { echo "Error: PR url or number is required" >&2; usage 1; }
 
+# Detect which loadout this dispatcher is running as (claude-gh / claude-jira
+# / claude-notion / claude-local). The 4 dispatcher bodies are kept
+# byte-identical (sentinel-checked by tools/check-drift.sh) and branch on
+# this for tracker-aware behavior — only claude-gh resolves spec issues via
+# the GitHub `Closes #N` convention, the others accept any opaque 2nd
+# positional arg (Jira key, Notion URL, local slug) and pass it through to
+# /reviewer untouched (#144).
+AW_LOADOUT="$(basename "$(dirname "$(dirname "${BASH_SOURCE[0]}")")")"
+
 parse_pr_number() {
   local input="$1"
   if [[ "$input" =~ ^[0-9]+$ ]]; then
@@ -100,16 +115,24 @@ parse_pr_number() {
   return 1
 }
 
+# Spec identifier parsing is tracker-aware. claude-gh keeps numeric / GitHub
+# issue URL parsing. claude-jira / claude-notion / claude-local accept any
+# non-empty string and pass it through opaquely — the /reviewer prompt knows
+# how to resolve it via the loadout-appropriate MCP / file read.
 parse_issue_number() {
   local input="$1"
-  if [[ "$input" =~ ^[0-9]+$ ]]; then
-    echo "$input"; return 0
-  fi
-  if [[ "$input" == *"github.com"*"/issues/"* ]]; then
-    echo "$input" | sed 's|.*/issues/||' | sed 's|[^0-9].*||'
-    return 0
+  if [[ "$AW_LOADOUT" == "claude-gh" ]]; then
+    if [[ "$input" =~ ^[0-9]+$ ]]; then
+      echo "$input"; return 0
+    fi
+    if [[ "$input" == *"github.com"*"/issues/"* ]]; then
+      echo "$input" | sed 's|.*/issues/||' | sed 's|[^0-9].*||'
+      return 0
+    fi
+    return 1
   fi
-  return 1
+  [[ -n "$input" ]] || return 1
+  printf '%s\n' "$input"
 }
 
 PR_INPUT="${POSITIONAL[0]}"
@@ -136,31 +159,55 @@ PR_URL=$(gh pr view "$PR_NUMBER" --json url --jq .url 2>/dev/null) || {
 PR_BASE_REF=$(gh pr view "$PR_NUMBER" --json baseRefName --jq .baseRefName 2>/dev/null || echo "main")
 PR_BODY=$(gh pr view "$PR_NUMBER" --json body --jq .body 2>/dev/null || echo "")
 
-# Issue resolution: explicit arg → parse; else scan PR body for first
-# Closes/Fixes/Resolves #N line.
+# Spec identifier resolution:
+#   - explicit 2nd arg always wins (parsed by tracker-aware parse_issue_number)
+#   - claude-gh additionally falls back to scanning the PR body for
+#     Closes/Fixes/Resolves #N (the GitHub linking convention)
+#   - non-gh loadouts have no PR-body convention to scan — Jira keys / Notion
+#     URLs / local slugs don't appear in PR bodies in a standard form — so
+#     the 2nd positional arg is the only path (#144).
+#
+# ISSUE_NUMBER is the raw identifier (numeric for gh, opaque string elsewhere)
+# persisted into the .info file for task-done. SPEC_IDENTIFIER is what
+# /reviewer receives as $ARGUMENTS[1] (a GitHub issues URL for gh, the opaque
+# identifier as-is for others).
 ISSUE_NUMBER=""
-ISSUE_URL=""
+SPEC_IDENTIFIER=""
 if [[ -n "$ISSUE_INPUT" ]]; then
+  # Only claude-gh's parse_issue_number can fail with a non-empty input
+  # (numeric / URL parse rejection). Non-gh accepts any non-empty string,
+  # and the [[ -n "$ISSUE_INPUT" ]] guard above means parse_issue_number
+  # is never called with an empty input — so the failure path here is
+  # claude-gh-only (#144 round-5).
   ISSUE_NUMBER=$(parse_issue_number "$ISSUE_INPUT") || {
     echo "Error: could not parse issue number from '$ISSUE_INPUT'" >&2
     exit 1
   }
-  if [[ "$ISSUE_INPUT" == *"github.com"* ]]; then
-    ISSUE_URL="$ISSUE_INPUT"
+  if [[ "$AW_LOADOUT" == "claude-gh" ]]; then
+    [[ "$ISSUE_INPUT" == *"github.com"* ]] && SPEC_IDENTIFIER="$ISSUE_INPUT"
+  else
+    SPEC_IDENTIFIER="$ISSUE_INPUT"
   fi
-elif [[ -n "$PR_BODY" ]]; then
+elif [[ "$AW_LOADOUT" == "claude-gh" && -n "$PR_BODY" ]]; then
   ISSUE_NUMBER=$(printf '%s\n' "$PR_BODY" \
     | grep -iEo '(close[sd]?|fix(e[sd])?|resolve[sd]?)[[:space:]]+#[0-9]+' \
     | head -1 | grep -Eo '[0-9]+' || true)
 fi
 
-# Build a fallback ISSUE_URL from PR_URL when we only have a number.
-if [[ -n "$ISSUE_NUMBER" && -z "$ISSUE_URL" && -n "$PR_URL" ]]; then
-  ISSUE_URL=$(printf '%s\n' "$PR_URL" | sed "s|/pull/[0-9]*|/issues/$ISSUE_NUMBER|")
+# claude-gh: synthesize the GitHub issues URL from PR_URL + ISSUE_NUMBER when
+# the caller passed only a number (or we auto-detected from the body).
+if [[ "$AW_LOADOUT" == "claude-gh" && -n "$ISSUE_NUMBER" && -z "$SPEC_IDENTIFIER" && -n "$PR_URL" ]]; then
+  SPEC_IDENTIFIER=$(printf '%s\n' "$PR_URL" | sed "s|/pull/[0-9]*|/issues/$ISSUE_NUMBER|")
 fi
 
 if [[ -z "$ISSUE_NUMBER" ]]; then
-  echo "⚠ No spec issue associated with PR #$PR_NUMBER (no second arg, no Closes/Fixes in body)." >&2
+  if [[ "$AW_LOADOUT" == "claude-gh" ]]; then
+    echo "⚠ No spec issue associated with PR #$PR_NUMBER (no second arg, no Closes/Fixes in body)." >&2
+  else
+    echo "⚠ No spec identifier passed for PR #$PR_NUMBER." >&2
+    echo "  PR-body auto-detect is GitHub-only — pass the spec id (Jira key, Notion URL," >&2
+    echo "  local task slug) as the 2nd positional arg to enable spec-compliance review." >&2
+  fi
   echo "  Reviewer will proceed with a diff-only review." >&2
 fi
 
@@ -242,7 +289,14 @@ build_claude_cmd() {
   local mode_prefix=""
   [[ -n "$AUTO_MODE" ]] && mode_prefix="--permission-mode auto "
   local slash_args="$PR_URL"
-  [[ -n "$ISSUE_URL" ]] && slash_args+=" $ISSUE_URL"
+  # printf %q the user-supplied SPEC_IDENTIFIER so `$var`, `$(...)`, and
+  # other metacharacters in non-gh tracker identifiers (Jira keys, Notion
+  # URLs, local slugs) survive the `bash -ic` re-parse without being
+  # expanded by the child shell. Mirrors how RADIO_ENV_PREFIX values are
+  # already protected. `set +H` in the launch-site prefix handles `!`
+  # history expansion; printf %q closes the rest of the metachar class.
+  # (#144 round-5 review.)
+  [[ -n "$SPEC_IDENTIFIER" ]] && slash_args+=" $(printf %q "$SPEC_IDENTIFIER")"
   echo "claude ${mode_prefix}\"/reviewer $slash_args\""
 }
 
@@ -263,12 +317,22 @@ RADIO_ENV_PREFIX+="ANTHROPIC_MODEL=$(printf %q "$RESOLVED_MODEL") "
 # caller's tab (PM or wherever this was dispatched from) instead of snapping
 # into the new reviewer tab. Same convention task-work --auto adopted in
 # #133; reviewer always opts in since it's always a dispatched worker.
+#
+# `set +H; ` prefix disables bash history expansion in the spawned `bash -ic`
+# subshell — defends against SPEC_IDENTIFIER values containing `!` (Jira /
+# Notion / local slugs with `auth-login!v2` shape, etc.) from being silently
+# history-substituted before /reviewer ever sees the arg. Critically, this
+# prefix must come BEFORE $RADIO_ENV_PREFIX, not inside build_claude_cmd's
+# output, because `VAR=val cmd; cmd2` scopes VAR=val to `cmd` only — putting
+# `set +H` between RADIO_ENV_PREFIX and `claude` would attach the env
+# assignments to the `set` builtin and they'd never reach `claude` after the
+# `;` (radio routing, model selection, AUTO_SUBMIT all dead). (#144 round-3.)
 echo "Opening zellij tab '$SLUG'..."
 if [[ -n "$NO_LAUNCH" ]]; then
   aw_launch_tab "$SLUG" "$WORKTREE_DIR" "" "1"
   echo "Tab opened at $WORKTREE_DIR — claude NOT launched (--no-launch)."
 else
-  aw_launch_tab "$SLUG" "$WORKTREE_DIR" "${RADIO_ENV_PREFIX}$(build_claude_cmd)" "1"
+  aw_launch_tab "$SLUG" "$WORKTREE_DIR" "set +H; ${RADIO_ENV_PREFIX}$(build_claude_cmd)" "1"
   echo "Started reviewer in $WORKTREE_DIR for PR #$PR_NUMBER (issue: ${ISSUE_NUMBER:-none}, model: $RESOLVED_MODEL)"
 fi
 

claude-gh/bin/task-work

@@ -150,9 +150,17 @@ WORKTREE_DIR="${WORKTREE_BASE}/${SLUG}"
 
 # ---------------- command builder ----------------
 build_claude_cmd() {
+  # printf %q the user-supplied GH_URL so `$var`, `$(...)`, backticks, and
+  # other shell metacharacters survive the `bash -ic` re-parse without
+  # being expanded by the child shell. Mirrors RADIO_ENV_PREFIX which
+  # already uses %q for its values. `set +H` in the launch-site prefix
+  # handles `!` history expansion; printf %q closes the rest of the
+  # metachar class. (#144 round-5 review.)
+  local q_gh_url=""
+  [[ -n "$GH_URL" ]] && q_gh_url=$(printf %q "$GH_URL")
   if [[ -n "$PLAN_MODE" ]]; then
     if [[ -n "$GH_URL" ]]; then
-      echo "claude --permission-mode plan \"/planner $GH_URL\""
+      echo "claude --permission-mode plan \"/planner $q_gh_url\""
     else
       echo "claude --permission-mode plan \"/planner\""
     fi
@@ -161,7 +169,7 @@ build_claude_cmd() {
   local mode_prefix=""
   [[ -n "$AUTO_MODE" ]] && mode_prefix="--permission-mode auto "
   if [[ -n "$GH_URL" ]]; then
-    echo "claude ${mode_prefix}\"/worker Implement task: $GH_URL\""
+    echo "claude ${mode_prefix}\"/worker Implement task: $q_gh_url\""
   else
     echo "claude ${mode_prefix}\"/worker\""
   fi
@@ -223,10 +231,20 @@ RADIO_ENV_PREFIX="TASK_FORCE_ROLE=$(printf %q "$TASK_FORCE_ROLE_VALUE") ZELLIJ_T
 
 # region:worktree-creation-post-info
 echo "Opening zellij tab '$SLUG'..."
+# `set +H; ` prefix disables bash history expansion in the spawned `bash -ic`
+# subshell — defends against `!` in user-supplied $GH_URL (or future
+# user-input passthroughs that grow on this code path) from being silently
+# history-substituted before claude / the worker prompt ever sees the arg.
+# Critically, this prefix must come BEFORE $RADIO_ENV_PREFIX, not between
+# it and `claude`: `VAR=val cmd1; cmd2` scopes VAR=val to `cmd1` only, so
+# putting `set +H` between the env prefix and `claude` would attach the
+# env assignments to the `set` builtin and they'd never reach `claude`
+# after the `;` (radio routing / model / AUTO_SUBMIT all dead). Mirrors
+# the task-reviewer fix. (#144 round-4 review.)
 if [[ -n "$NO_LAUNCH" ]]; then
   aw_launch_tab "$SLUG" "$WORKTREE_DIR" "" "${AUTO_MODE:-}"
 else
-  aw_launch_tab "$SLUG" "$WORKTREE_DIR" "${RADIO_ENV_PREFIX}$(build_claude_cmd)" "${AUTO_MODE:-}"
+  aw_launch_tab "$SLUG" "$WORKTREE_DIR" "set +H; ${RADIO_ENV_PREFIX}$(build_claude_cmd)" "${AUTO_MODE:-}"
 fi
 
 if [[ -n "$NO_LAUNCH" ]]; then