ADR-255: Update reviewer/developer skill files to use thread schema

[jodavis-claude]

Work item

ADR-255: Update the three agent skill markdown files (reviewer-review.md, reviewer-sign-off.md, developer-fix.md) to use the new thread schema so that all GitHub interactions are centralized in the scrum master rather than in sub-agents.

Changes

• plugins/dev-team/commands/reviewer-review.md — Removed $PR_URL input and Step 3 (PR gate); replaced gh pr diff with git diff origin/$BASE_BRANCH; removed Step 6 (gh api post); updated output schema to {"body", "status", "threads[]} with no id field, filePath camelCase, 1-based lineNumber, and comments array; added explicit instruction that lineNumber is source-file line not diff position
• plugins/dev-team/commands/reviewer-sign-off.md — Replaced $PR_URL header with $BASE_BRANCH and $REVIEW_THREADS; removed Step 3 (GraphQL fetch), Step 4 (GitHub MCP resolve calls and mutation), Step 6 (gh api post), and Step 6a (pr ready/Jira assign); restructured step to evaluate threads from $REVIEW_THREADS with "return every thread" instruction and comments: [] rule; output is {"body", "threads[]} with no status field; carried-over threads preserve id, new threads omit it
• plugins/dev-team/commands/developer-fix.md — Removed Step 3 (GitHub MCP fetch) and Step 4 (triage with direct GitHub reply); added ### Build/Test Issues and ### Review Threads input sections; added mode-detection block; in review-fix mode Developer iterates threads and returns full threads[] fenced JSON block; in build/test-fix mode no JSON output is produced; steps renumbered 1–5

Design decisions

• Sign-off id handling: Carried-over threads in reviewer-sign-off.md output include the original id so SignoffStep can match them; new threads introduced during sign-off omit id (pipeline assigns). This is required for matching without an additional lookup step.
• Gate removal: No explicit guard added for empty $BASE_BRANCH in reviewer-review.md — if $BASE_BRANCH is empty, the git diff command fails naturally with a clear error.
• Sign-off diff scope: Uses git diff origin/$BASE_BRANCH over all changed files rather than filtering to last-commit files, to avoid missing issues introduced across multiple fix commits.

[jodavis-claude]

Review: ADR-255 — Update reviewer/developer skill files to use thread schema

All three exit criteria are met for reviewer-review.md, reviewer-sign-off.md, and developer-fix.md. Two correctness issues require fixes before approval.

P1-A — developer-fix.md:122: "may leave comments empty" erases Reviewer comment history

FixPrStep uses wholesale comment replacement: matched.comments = [ReviewComment(...) for c in thread_dict.get("comments", [])]. When the Developer returns "comments": [] for a thread it fixed silently (following the current instruction), this assignment erases the original Reviewer comment from the in-memory thread. At sign-off, the Reviewer receives the thread with "comments": [] and only knows the file/line location — no description of the original issue — making it harder to verify the fix.

The spec (task brief) is clear: "Each returned thread object must include all prior comments plus the new Developer reply (if any)."

Fix: Change line 122 from:

"you may leave comments empty (the Reviewer will evaluate during sign-off)"

To:

"include all prior comments from the thread's comments array as-is, and append your own {"author": "Developer", "comment": "..."} entry only if you are disagreeing or want to explain the fix. Do not clear prior comments — the sign-off Reviewer needs the original issue description."

Also update the example JSON block at lines 128–146 so the first thread (where the Developer agreed and fixed silently) shows the prior Reviewer comment preserved, not "comments": [].

P1-B — dev_team.py:885: ctx.save() called before ctx.review_fix_iteration += 1

ctx.save(self._context_path)       # line 885 — saves stale iteration count
ctx.review_fix_iteration += 1      # line 886

If the pipeline crashes between the save and the state transition, the next run re-enters FixPrStep with the same iteration count. MAX_REVIEW_FIX_ITERATIONS prevents an infinite loop, but the counter being under-counted allows one extra fix iteration than intended.

Fix: Swap the two lines so ctx.review_fix_iteration += 1 runs before ctx.save(self._context_path).

[jodavis-claude]

Sign-off review for ADR-255: all exit criteria verified.

All three skill files updated correctly:

• reviewer-review.md: uses threads[] (no id field), lineNumber is source-file line, no GitHub posting step, $BASE_BRANCH input present
• reviewer-sign-off.md: inputs use $BASE_BRANCH and $REVIEW_THREADS, output has no status field, returns every thread with comments:[] for unchanged threads, no GitHub posting steps
• developer-fix.md: old Steps 3-4 (GitHub fetch/reply) removed, $ISSUES and $REVIEW_THREADS input sections present, mode detection correct, threads[] output in review-fix mode only, prior comments preserved

dev_team.py: FixPrStep.review_fix_iteration incremented before ctx.save(); developer-fix thread merge correct.

No prior review threads to evaluate. No new Priority 1-4 issues found in modified files. Approved.