feat(#347): class-aware role-trigger banner — HYBRID spawn vs in-thread (Wave 2 PR 5)#362
Conversation
…ad (Wave 2 PR 5) Per AgDR-0050 § Axis 6, role triggers now emit one of two banner shapes depending on the matched role's **Class** value in its `## Activation mode` section: - isolated-work-class (12 roles: Heads-of-X, Tech Lead, QA Engineer, SRE, Security Auditor, Pen Tester, Product Analyst, Data Analyst) → banner instructs the agent to SPAWN the sub-agent via the Agent tool with `subagent_type: <slug>`. Isolated work benefits from isolated context + tool restriction. - in-flow-class (7 roles: Backend / Frontend / Platform Engineer, Product Manager, UI / UX Designer, Data Engineer) → banner instructs the agent to ADOPT the persona IN-THREAD by reading the role file. In-flow work loses too much shared context if spawned out-of-thread. Implementation: - New `lookup_role_class()` helper reads `**Class**:` from the role file at trigger time. Falls back to in-flow-class on any failure (missing file, missing section, unparseable value) — conservative fail-open. - New `agent_slug_for()` helper maps role-file path → agent subagent_type slug. 1:1 by default (`backend-engineer.md` → `backend-engineer`); the one exception is `security-auditor.md` → `security-reviewer` (the Hatim→Hakim consolidation from PR #360 kept the filename so `/security-review` + the auto-fire trigger keep working). - `emit_banner()` is restructured to look up class + slug from the file arg and emit the appropriate shape. Caller signatures unchanged — every call site still passes (role, file, reason); the helper does the class-aware fan-out. Tests (7 new cases, all PASS, 26/26 overall): - 4a: Security Auditor → isolated-work-class banner with the consolidation slug `security-reviewer` (NOT `security-auditor`). - 4b: Platform Engineer → in-flow-class banner. - 4c: Tech Lead → isolated-work-class. - 4d: QA Engineer → isolated-work-class (label-based trigger). - 4e: Backend Engineer prompted → in-flow-class. - 4f: UX Designer prompted → in-flow-class. - 4g: Pen Tester prompted → isolated-work-class. Existing 19 tests unchanged + still PASS — their regexes are broad enough to match both old and new banner shapes, so this is a strict extension (no behaviour regressions on the in-thread path). Refs #347 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`.claude/rules/role-triggers.md` mechanical-backstop section now describes both class-aware banner shapes (SPAWN for isolated-work-class, IN-THREAD adoption for in-flow-class), the 12-vs-7 role split per AgDR-0050 § Axis 6, and the one filename-vs-slug exception (security-auditor.md → security-reviewer agent slug, from PR #360's Hatim→Hakim consolidation). Refs #347 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
atlas-apex
left a comment
There was a problem hiding this comment.
Code Review: PR #362
Commit: b84c9bab1c98f7798605e1c38c0b8a79ef596f9c
Summary
Wires detect-role-trigger.sh to emit class-aware banners per AgDR-0050 § Axis 6's HYBRID design: isolated-work-class roles get a "SPAWN the sub-agent via the Agent tool" banner; in-flow-class roles get an "adopt the persona IN-THREAD" banner. Adds two helpers (lookup_role_class reads **Class**: from the role file's ## Activation mode section; agent_slug_for maps role-file path → agent slug with one explicit exception for security-auditor.md → security-reviewer). 91/4 lines in the hook, 63 new test lines (7 new cases), 11 doc lines. With PR 5 merged, the framework's auto-triggers no longer just remind — they drive sub-agent spawn for the 12 isolated-work-class roles.
Checklist Results
- ✅ Architecture & Design: Pass
- ✅ Code Quality: Pass
- ✅ Testing: Pass (26/26 PASS)
- ✅ Security: Pass
- ✅ Performance: Pass
- ✅ PR Description & Glossary: Pass
- ✅ Summary Bullet Narrative: Pass — all 5 bullets answer what + why
- ✅ Technical Decisions (AgDR): N/A — implements AgDR-0050 § Axis 6 (already decided); no new decisions introduced
- ✅ Adopter Handbooks: N/A — only architecture/general handbooks always-load; none apply to shell + markdown changes
Verification of focus areas
1. lookup_role_class() correctness. Ran bash .claude/agents/tests/test_agent_wrap_shape.sh — all 19 role files parse correctly (12 isolated, 7 in-flow). The awk regex /^## Activation mode/ matches the exact section header on every role file (grep-confirmed: ## Activation mode is the literal heading in all 19 files, no longer variants). The **Class**: line is unique to that section across all 19 files (grep-confirmed: exactly one match per file). The fallback to in-flow-class on missing-file / missing-section / unparseable value is conservative — under-triggers SPAWN rather than over-triggers, matching the docstring claim. ✓
2. agent_slug_for() exception coverage. Compared .claude/agents/*.md basenames against roles/*/*.md basenames. The set of agent-only names is {code-reviewer, dependency-auditor, pr-manager, security-reviewer, ticket-manager} — four utility agents (no role wraps them) plus the security-reviewer consolidation exception. The set of role-only names is {security-auditor} — the one role-side counterpart to the exception. No other consolidation-class mismatches lurking. ✓
3. Banner regex test coverage. The 4a test regex Isolated-work-class.*subagent_type: security-reviewer correctly anchors on both the class label AND the slug. A regression where agent_slug_for returns security-auditor would emit subagent_type: security-auditor, which would not match subagent_type: security-reviewer. Verified empirically by capturing the actual banner output from the hook. ✓
5. Strict-extension claim. Ran bash .claude/hooks/tests/test_detect_role_trigger.sh — 26/26 PASS (19 prior unchanged + 7 new). Spot-checked: prior test 1a's regex "ROLE TRIGGER: QA Engineer.*roles/engineering/qa-engineer\\.md" matches the new banner because BOTH the role display name AND the role file path are preserved verbatim in the isolated-work-class banner, separated by class-specific content that .* matches over. Same shape for the in-flow-class banner. ✓
6. .claude/rules/role-triggers.md doc accuracy. Counted classes from the live role files: 12 isolated + 7 in-flow = 19 total — matches the doc claim exactly. Enumeration check: doc lists Heads-of-X (5) + Tech Lead + QA Engineer + SRE + Security Auditor + Pen Tester + Product Analyst + Data Analyst = 12 isolated ✓; Backend / Frontend / Platform Engineer + Product Manager + UI / UX Designer + Data Engineer = 7 in-flow ✓. Security-auditor exception description matches the code (agent_slug_for() returns security-reviewer for the security-auditor.md stem). ✓
7. Scope discipline. git diff --stat dev..b84c9ba shows exactly 3 files changed: .claude/hooks/detect-role-trigger.sh (+91/−4), .claude/hooks/tests/test_detect_role_trigger.sh (+63/−0), .claude/rules/role-triggers.md (+11/−0). No CLAUDE.md changes (no count changes needed), no site / AGENTS.md changes, no drive-by edits. ✓
8. Closes #347 claim. Grepped AgDR-0050 for #347 PR N references: lists PR 1-5 only (lines 126, 147, 162, 171-173, 179). PR 5 is the last entry, described as "Role-trigger integration (detect-role-trigger.sh switches from in-thread injection to sub-agent spawn for the isolated-work-class roles)" — exactly what this PR does. ✓
Issues Found
None.
Suggestions (non-blocking)
4. In-flow-class banner wording — minor tightening opportunity. The current banner reads:
In-flow-class — adopt the persona IN-THREAD. Read roles/engineering/platform-engineer.md before continuing. Per AgDR-0050 § Axis 6.
The contrast with the isolated-work-class banner ("SPAWN the sub-agent via the Agent tool with subagent_type: ...") is fairly explicit — the in-flow case doesn't mention the Agent tool at all, and IN-THREAD is in all-caps as a load-bearing disambiguator. However, a particularly literal reader could parse "adopt the persona IN-THREAD" as "spawn a sub-agent whose persona is adopted in-thread". One word of explicit negation would close that gap entirely:
In-flow-class — adopt the persona IN-THREAD (do NOT spawn a sub-agent). Read roles/engineering/platform-engineer.md before continuing. Per AgDR-0050 § Axis 6.
nit: — not blocking. The current wording is adequate given the contrast structure; this is a defence-in-depth tightening. Worth a follow-up PR if the team sees an agent misinterpret it; not worth holding this PR for. The 7 new tests don't test against this specific failure mode (and don't need to — they assert on the banner shape, which is correct as-is).
Verdict
APPROVED
The PR cleanly implements AgDR-0050 § Axis 6's HYBRID design. The helper structure is conservative (fail-open lookup, narrow slug exception with explicit rationale), the test additions cover the exact regression surfaces (class-aware banner shape + slug exception), the strict-extension property holds (19 prior tests still PASS unchanged), the doc accurately enumerates the 12-vs-7 split, and scope is disciplined (3 files, no drive-bys). With PR 5 merged, the 5-PR wave plan for #347 is complete — auto-triggers now drive sub-agent spawn for isolated-work-class roles per the AgDR's design.
🤖 Reviewed by Rex (Code Reviewer Agent)
📌 Reviewed commit: b84c9bab1c98f7798605e1c38c0b8a79ef596f9c
…ad (Wave 2 PR 5) (#362) * feat(#347): class-aware role-trigger banner — HYBRID spawn vs in-thread (Wave 2 PR 5) Per AgDR-0050 § Axis 6, role triggers now emit one of two banner shapes depending on the matched role's **Class** value in its `## Activation mode` section: - isolated-work-class (12 roles: Heads-of-X, Tech Lead, QA Engineer, SRE, Security Auditor, Pen Tester, Product Analyst, Data Analyst) → banner instructs the agent to SPAWN the sub-agent via the Agent tool with `subagent_type: <slug>`. Isolated work benefits from isolated context + tool restriction. - in-flow-class (7 roles: Backend / Frontend / Platform Engineer, Product Manager, UI / UX Designer, Data Engineer) → banner instructs the agent to ADOPT the persona IN-THREAD by reading the role file. In-flow work loses too much shared context if spawned out-of-thread. Implementation: - New `lookup_role_class()` helper reads `**Class**:` from the role file at trigger time. Falls back to in-flow-class on any failure (missing file, missing section, unparseable value) — conservative fail-open. - New `agent_slug_for()` helper maps role-file path → agent subagent_type slug. 1:1 by default (`backend-engineer.md` → `backend-engineer`); the one exception is `security-auditor.md` → `security-reviewer` (the Hatim→Hakim consolidation from PR #360 kept the filename so `/security-review` + the auto-fire trigger keep working). - `emit_banner()` is restructured to look up class + slug from the file arg and emit the appropriate shape. Caller signatures unchanged — every call site still passes (role, file, reason); the helper does the class-aware fan-out. Tests (7 new cases, all PASS, 26/26 overall): - 4a: Security Auditor → isolated-work-class banner with the consolidation slug `security-reviewer` (NOT `security-auditor`). - 4b: Platform Engineer → in-flow-class banner. - 4c: Tech Lead → isolated-work-class. - 4d: QA Engineer → isolated-work-class (label-based trigger). - 4e: Backend Engineer prompted → in-flow-class. - 4f: UX Designer prompted → in-flow-class. - 4g: Pen Tester prompted → isolated-work-class. Existing 19 tests unchanged + still PASS — their regexes are broad enough to match both old and new banner shapes, so this is a strict extension (no behaviour regressions on the in-thread path). Refs #347 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs(#347): document live HYBRID role-trigger banner shape (Wave 2 PR 5) `.claude/rules/role-triggers.md` mechanical-backstop section now describes both class-aware banner shapes (SPAWN for isolated-work-class, IN-THREAD adoption for in-flow-class), the 12-vs-7 role split per AgDR-0050 § Axis 6, and the one filename-vs-slug exception (security-auditor.md → security-reviewer agent slug, from PR #360's Hatim→Hakim consolidation). Refs #347 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: me2resh <ahmed.abdelaliem@gmail.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
detect-role-trigger.shnow emits class-aware banners per AgDR-0050 § Axis 6's HYBRID design. When a trigger fires, the hook reads the matched role's**Class**:value from the## Activation modesection of its role file and emits one of two banner shapes: isolated-work-class → "SPAWN the sub-agent via the Agent tool withsubagent_type: <slug>"; in-flow-class → "adopt the persona IN-THREAD". This closes [Feature] Promote all 19 role definitions to Claude Code sub-agents (per-role model + tool restriction + isolated context) #347 — the 5-PR wave plan is now complete (PRs 1-4 promoted the 18 role-derived agents + 4 utility agents to sub-agents with explicit models; PR 5 wires the auto-triggers to actually spawn them for isolated-work-class roles).lookup_role_class()+agent_slug_for()helpers.lookup_role_classparses**Class**:from a role file's## Activation modesection; falls back toin-flow-classon any failure (missing file, missing section, unparseable value) — conservative fail-open so a malformed role file doesn't break trigger detection.agent_slug_formaps role-file path → agentsubagent_typeslug; 1:1 by default but with one explicit exception:security-auditor.md→security-reviewer(the Hatim→Hakim consolidation from PR feat(#347)!: Hatim→Hakim consolidation + security + data sub-agents (Wave 2 PR 3) #360 kept the agent filename so/security-review+auto-code-review.shkeep working).emit_banner()restructured to look up class + slug from the file argument and emit the appropriate shape. Caller signatures unchanged — every existing call site still passes(role, file, reason); the helper does the class-aware fan-out internally. This means the existing 19 tests' broad regexes (ROLE TRIGGER:.*Security Auditor) all still PASS as a strict-extension check.test_detect_role_trigger.shcover both class shapes across all three trigger families (path, label, prompted): Security Auditor → isolated-work-class withsecurity-reviewerslug (NOTsecurity-auditor— the consolidation exception); Platform Engineer → in-flow-class; Tech Lead → isolated-work-class; QA Engineer → isolated-work-class via label; Backend Engineer prompted → in-flow-class; UX Designer prompted → in-flow-class; Pen Tester prompted → isolated-work-class. 26/26 total PASS..claude/rules/role-triggers.mdmechanical-backstop section documents the live HYBRID design: both banner shapes, the 12-vs-7 role split (12 isolated-work-class, 7 in-flow-class), and the security-auditor → security-reviewer slug exception.Testing
bash .claude/hooks/tests/test_detect_role_trigger.sh— PASS at 26/26 (19 prior + 7 new class-aware).bash .claude/agents/tests/test_agent_wrap_shape.sh— PASS at 13 ROLE_AGENTS + 5 UTILITY_AGENTS + 19 ROLE_CLASSES (all 19 role files have correct**Class**:values that the new lookup reads).auth/, observe Rex emits the new "SPAWN with subagent_type: security-reviewer" banner shape (not the old advisory "Read the role file" shape). Not blocking.Glossary
## Activation modesection of the role file. The trigger hook reads the class and emits the appropriate banner shape.lookup_role_class()reads**Class**:from a role file's## Activation modesection. Returnsisolated-work-class/in-flow-class/in-flow-class(fail-open fallback). Conservative: better to under-trigger sub-agent spawn than to incorrectly suggest one for an unclassified role.agent_slug_for(roles/security/security-auditor.md)returnssecurity-reviewer, notsecurity-auditor. This is the only non-1:1 mapping in the framework. Rationale: the Hatim→Hakim consolidation from PR #360 kept the agent filename so/security-review+auto-code-review.sh+.claude/rules/role-triggers.mdreferences stayed working.test_detect_role_trigger.shuse broad regexes likeROLE TRIGGER:.*Security Auditorthat match BOTH the old and new banner shapes. No existing test regex needed updating — the new banner is a strict extension. The 7 new tests assert the class-aware extension specifically.Closes #347
This is the 5th and final PR of the #347 wave plan from AgDR-0050. With PR 5 merged, the framework's auto-triggers no longer just remind operators that a role activated — they actually drive sub-agent spawn for the 12 isolated-work-class roles, which is what the AgDR-0050 design called for.
Refs #347
Refs #353 / #355 / #356 / #357 / #360 / #361 (prior PRs in the wave)