task-reviewer resolves the spec through lib/trackers (#239, step 2 of 2)

[martin-conur]

Closes #239 — the task-reviewer step. Step 1 (task-recreate-worker) was #252.

What changed

The claude body of bin/task-reviewer no longer branches on $TRACKER. Everything tracker-shaped moves into four new tracker hooks:

Old inline code	Hook
parse_issue_number, the gh Closes/Fixes/Resolves #N scan, the issues-URL synthesis	aw_tracker_review_spec <input> <pr_url> <pr_body> → sets AW_SPEC_ID / AW_SPEC_REF
the two no-spec warnings	aw_tracker_review_no_spec_warning <pr_number>
the four-loadout <spec-identifier> help table	aw_tracker_review_usage_spec + aw_tracker_review_spec_shape

Defaults vs overrides.

• _default.sh holds jira/notion/local's opaque pass-through, so those three modules each override only the one-line shape noun.
• gh.sh overrides the spec resolution, the help paragraph and the warning.

Why new hooks rather than aw_tracker_parse_ref. This is the API question the spec asked to record. A reviewer's spec is not task-work's ref:

• on gh it can be a bare issue number, which gh.sh's aw_tracker_is_ref rejects;
• on local it is a slug for a file that need not exist, while local.sh's predicate requires -f.

So these are the "neighbour" hooks the spec anticipated, not evidence that the task-work API was drawn in the wrong place.

Behaviour

Unchanged on every loadout: spec resolution, the .info contents, the warnings, the error text and the /reviewer launch line. All 82 pre-existing task_reviewer.bats cases pass unmodified.

Declared change:

• --help prints only this repo's tracker's spec shape, instead of all four loadouts' shapes.
• Outside a configured repo, --help says the shape depends on the tracker.
• The synopsis now calls the argument [<spec-identifier>], matching the README.

#146 overlap

This PR does not touch kiro-gh/bin/task-reviewer; kiro-* still execs into it. It does make #146's code half much smaller:

• kiro-gh's parse_issue_number / ISSUE_URL / PR-body scan is exactly what lib/trackers/gh.sh's aw_tracker_review_spec now does.
• So migrating it means sourcing _default.sh + gh.sh and calling that one hook, not porting the task-reviewer: tracker-aware spec-issue cross-check (claude-jira/notion/local) (#144) #145 SPEC_IDENTIFIER logic by hand.
• The kiro-jira / -notion / -local extension post-#144 reviewer cleanup: kiro-gh task-reviewer SPEC_IDENTIFIER migration + the stale signature in .claude/gh-workflow.md #146 lists as out of scope would get the opaque pass-through for free from _default.sh.

task-pm: skipped

bin/task-pm's two case "$AGENT" arms exec an argv (exec claude "/pm"). They do not build the bash -ic command string that aw_agent_launch_cmd returns, so the hook does not fit without a new PM-launch hook. The spec marks this step optional and lowest-payoff, so it is left alone.

Tests

bats --count: 1311 at step 1's fc4d87b → 1315. All 1315 pass under ./run_tests.sh (run before the rebase onto origin/main). After the rebase I re-ran task_reviewer, loadout_modules and task_recreate_worker, and all passed.

New tests:

• task_reviewer.bats:
  • --help names only its own tracker's shape, on all four claude loadouts;
  • --help works outside a configured repo;
  • ISSUE_NUMBER in .info is the tracker's own id, for each of the four trackers.
• loadout_modules.bats:
  • every impl's tracker module defines all four task-reviewer hooks.

Each new test was checked against a broken tree and went red:

• the old bin/task-reviewer fails both help tests;
• blanking AW_SPEC_ID in the default hook fails the .info test, and only that test. The non-gh ISSUE_NUMBER had no coverage before;
• renaming aw_tracker_review_no_spec_warning in _default.sh fails the hook-presence test.

Also green: tools/check-drift.sh (15 groups) and shellcheck -x on bin/task-reviewer and lib/trackers/*.sh.

No task-init re-run needed. The CHANGELOG entry is under [Unreleased] → Changed.

🤖 Generated with Claude Code

[martin-conur]

Spec compliance

Issue #239 called for migrating bin/task-reviewer to use lib/trackers/*.sh modules (step 2 of 2, step 1 being #252), fixing the live bug in bin/task-recreate-worker's tracker-specific sidecar read, and adding per-tracker tests. All deliverables are present:

Deliverable	Status
bin/task-reviewer — remove inline parse_issue_number, tracker if/elif chain, four-loadout help table	✓ Done
Four tracker hooks defined in _default.sh, overridden where needed	✓ Done: aw_tracker_review_spec, aw_tracker_review_spec_shape, aw_tracker_review_usage_spec, aw_tracker_review_no_spec_warning
gh.sh full overrides; jira/notion/local shape-only override	✓ Done
New tests — per-tracker --help, outside-repo --help, ISSUE_NUMBER per tracker	✓ Done, all mutation-checked
loadout_modules.bats — all four hooks listed for every impl	✓ Done
CHANGELOG under [Unreleased] → Changed	✓ Done
task-pm skip justified	✓ Explained: exec argv shape doesn't fit aw_agent_launch_cmd's contract
#146 overlap addressed	✓ Documented: migrating kiro-gh gets aw_tracker_review_spec for free, making that work smaller

The API design question the spec asked to record is answered clearly: aw_tracker_review_spec is intentionally separate from aw_tracker_parse_ref because a reviewer's gh spec accepts a bare issue number (which aw_tracker_is_ref rejects) and a local one accepts a non-existent slug (which local.sh's predicate blocks). Good answer, well-documented.

Code-review findings

gh.sh aw_tracker_review_spec — all input paths correct. Bare number → AW_SPEC_ID set, AW_SPEC_REF empty → URL synthesis fires at the bottom. GitHub URL → both vars set before synthesis, -z "$AW_SPEC_REF" guard blocks overwrite. PR body scan → same URL synthesis path. Empty input with non-empty body → body scan only. All three cases verified against the diff. ✓

_default.sh unquoted heredoc in aw_tracker_review_usage_spec is intentional. The <<TXT (not <<'TXT') allows $(aw_tracker_review_spec_shape) to expand at call time, after gh.sh / jira.sh / etc. have overridden the shape function. This is correct and the only way to compose the dynamic noun into a shared body. ✓

gh.sh does not define aw_tracker_review_spec_shape. Acceptable: gh.sh's aw_tracker_review_usage_spec override never calls aw_tracker_review_spec_shape — it has its own inline text. The loadout_modules.bats test finds the default version via _default.sh composition and is satisfied. ✓

SPEC_IDENTIFIER is printf %q'd before the slash-command line (bin/task-reviewer:320). Injection defense carried over correctly. ✓

Mutation checking documented and thorough. Old binary fails both help tests; blanking AW_SPEC_ID fails only the .info test; renaming aw_tracker_review_no_spec_warning fails the hook-presence test. Non-gh ISSUE_NUMBER had no coverage before — that gap is now closed. ✓

CI: 1 run exists for the head commit (16fed08). Not a suppressed run.

No correctness bugs, no security concerns, no edge-case gaps.

Verdict

clean — all spec deliverables met, code is correct on all input paths, tests mutation-verified, CI ran. Ready to merge.