charles/claude-hooks

Fork 0

feat(agents): reviewer specialisation — area:security and type:chore instances (M17-3) #161

Merged

code-lead merged 2 commits from dev/154 into main

2026-04-20 16:50:16 +00:00

dev commented

2026-04-20 15:49:45 +00:00

Collaborator

Summary

Adds area:security to config/labels.json (the routing label used by reviewer-security)
Adds 4 new pool tests in pool.test.ts verifying that a 3-instance reviewer pool routes area:security-labeled PRs to reviewer-security even when the other instances are idle and appear first in round-robin order
Adds a webhook-handlers test verifying handleReviewRequested correctly extracts linked-issue labels (Closes #N → fetch issue #10 → area:security) and reaches the dispatch step
Documents the specialised-reviewer pattern in CLAUDE.md ("Adding a specialised reviewer instance")

The actual SQLite rows (reviewer-security, reviewer-fast) are runtime operations via the dashboard CRUD — no JSON edit or restart required. See the CLAUDE.md section for the exact POST payload.

Closes #154

Post-merge smoke

After merge, create an area:security issue on this repo and paste the live-service routing log line here to confirm reviewer-security is selected.

## Summary - Adds `area:security` to `config/labels.json` (the routing label used by `reviewer-security`) - Adds 4 new pool tests in `pool.test.ts` verifying that a 3-instance reviewer pool routes `area:security`-labeled PRs to `reviewer-security` even when the other instances are idle and appear first in round-robin order - Adds a webhook-handlers test verifying `handleReviewRequested` correctly extracts linked-issue labels (`Closes #N` → fetch issue #10 → `area:security`) and reaches the dispatch step - Documents the specialised-reviewer pattern in CLAUDE.md ("Adding a specialised reviewer instance") The actual SQLite rows (`reviewer-security`, `reviewer-fast`) are runtime operations via the dashboard CRUD — no JSON edit or restart required. See the CLAUDE.md section for the exact POST payload. Closes #154 ## Post-merge smoke After merge, create an `area:security` issue on this repo and paste the live-service routing log line here to confirm `reviewer-security` is selected.

dev added 1 commit

2026-04-20 15:49:46 +00:00

feat(agents): reviewer specialisation — area:security and type:chore instances (M17-3)

qa / qa (pull_request) Failing after 3m6s

Details

qa / dockerfile (pull_request) Successful in 9s

Details

a26971a3a0

Adds the groundwork for running reviewer-security (opus) and reviewer-fast
(haiku) pool members alongside reviewer-default (sonnet). The SQLite rows
are created via the dashboard CRUD after merge (no JSON edit needed).

- config/labels.json: add area:security routing label
- pool.test.ts: 4 tests for 3-instance reviewer pool; verifies area:security
  picks reviewer-security even when others are idle and alphabetically earlier
- webhook-handlers.test.ts: verifies handleReviewRequested fetches linked-issue
  labels from `Closes #N` and reaches dispatch (label extraction path works)
- CLAUDE.md: document how to add a specialised reviewer instance

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

dev requested review from reviewer

2026-04-20 15:52:49 +00:00

reviewer commented

2026-04-20 15:53:25 +00:00

Collaborator

CI still pending at review time (run #308, sha a26971a). Stepping off the review request — will be re-dispatched automatically when CI completes.

CI still pending at review time (run #308, sha `a26971a`). Stepping off the review request — will be re-dispatched automatically when CI completes.

dev force-pushed dev/154 from a26971a3a0

qa / qa (pull_request) Failing after 3m6s

Details

qa / dockerfile (pull_request) Successful in 9s

Details

to cef2f3b56a

qa / qa (pull_request) Failing after 3m13s

Details

qa / dockerfile (pull_request) Successful in 8s

Details

2026-04-20 15:58:39 +00:00

Compare

dev force-pushed dev/154 from cef2f3b56a

qa / qa (pull_request) Failing after 3m13s

Details

qa / dockerfile (pull_request) Successful in 8s

Details

to c81a97506e

qa / qa (pull_request) Successful in 3m8s

Details

qa / dockerfile (pull_request) Successful in 9s

Details

2026-04-20 16:09:39 +00:00

Compare

dev force-pushed dev/154 from c81a97506e

qa / qa (pull_request) Successful in 3m8s

Details

qa / dockerfile (pull_request) Successful in 9s

Details

to 3e833229fd

qa / qa (pull_request) Successful in 3m13s

Details

qa / dockerfile (pull_request) Successful in 8s

Details

2026-04-20 16:38:10 +00:00

Compare

reviewer approved these changes

2026-04-20 16:49:23 +00:00

reviewer left a comment

Review: APPROVED ✅

CI is green (run #1759, 3m22s). All acceptance criteria that can be met via code are satisfied.

Checklist

Criterion	Status
`area:security` added to `config/labels.json`	✅
`type:chore` present in `config/labels.json`	✅ (already there)
`pool.test.ts` — 3-instance reviewer pool, 4 M17-3 tests	✅
`webhook-handlers.test.ts` — `handleReviewRequested` label extraction + dispatch	✅
CLAUDE.md — "Adding a specialised reviewer instance" section	✅
SQLite rows (`reviewer-security`, `reviewer-fast`)	✅ deferred to runtime CRUD (correct per issue spec)
Smoke test	✅ deferred to post-merge (correct per issue spec)

Pool tests (M17-3 section)

Four tests correctly model the production setup (reviewer-default < reviewer-fast < reviewer-security alphabetically, so cursor=0 without label-awareness would land on reviewer-default). The label-aware path correctly overrides that in all four scenarios:

area:security → reviewer-security even when all three are idle and it is last alphabetically
type:chore → reviewer-fast while others are idle
Unmatched label → reviewer-default (catch-all)
area:security wins even when reviewer-security is busy and reviewer-default is idle — the match tier is non-empty so the catch-all is correctly ignored

Webhook-handlers test

Self-contained via the mock.module("./main") stub at the top of the file — fixes the mock-ordering flake that caused the first CI run to fail. The test correctly verifies two things: (1) handleReviewRequested reaches dispatch (non-null taskId), and (2) /issues/10 was fetched, confirming the Closes #10 body parsing and label extraction pipeline works end-to-end.

Labels

area:security entry is well-formed: color dc2626 (red, appropriate for security), description "Security — routes to reviewer-security (opus)", scope "area". Consistent with the rest of the area:* entries.

No issues found.

## Review: APPROVED ✅ CI is green (run #1759, 3m22s). All acceptance criteria that can be met via code are satisfied. ### Checklist | Criterion | Status | |---|---| | `area:security` added to `config/labels.json` | ✅ | | `type:chore` present in `config/labels.json` | ✅ (already there) | | `pool.test.ts` — 3-instance reviewer pool, 4 M17-3 tests | ✅ | | `webhook-handlers.test.ts` — `handleReviewRequested` label extraction + dispatch | ✅ | | CLAUDE.md — "Adding a specialised reviewer instance" section | ✅ | | SQLite rows (`reviewer-security`, `reviewer-fast`) | ✅ deferred to runtime CRUD (correct per issue spec) | | Smoke test | ✅ deferred to post-merge (correct per issue spec) | ### Pool tests (M17-3 section) Four tests correctly model the production setup (`reviewer-default < reviewer-fast < reviewer-security` alphabetically, so cursor=0 without label-awareness would land on `reviewer-default`). The label-aware path correctly overrides that in all four scenarios: - `area:security` → `reviewer-security` even when all three are idle and it is last alphabetically - `type:chore` → `reviewer-fast` while others are idle - Unmatched label → `reviewer-default` (catch-all) - `area:security` wins even when `reviewer-security` is busy and `reviewer-default` is idle — the match tier is non-empty so the catch-all is correctly ignored ### Webhook-handlers test Self-contained via the `mock.module("./main")` stub at the top of the file — fixes the mock-ordering flake that caused the first CI run to fail. The test correctly verifies two things: (1) `handleReviewRequested` reaches dispatch (non-null taskId), and (2) `/issues/10` was fetched, confirming the `Closes #10` body parsing and label extraction pipeline works end-to-end. ### Labels `area:security` entry is well-formed: color `dc2626` (red, appropriate for security), description "Security — routes to reviewer-security (opus)", scope "area". Consistent with the rest of the `area:*` entries. No issues found.