## Problem CC's Skill tool input and subagent_type fields can carry a plugin prefix (`tmb:` or `tmb-rc:`) depending on context (project-local override vs global plugin agent vs slash-command vs direct invocation). Five plugin hooks compare these fields against the bare role name only — when CC supplies the prefixed form, the comparison fails and the hook silently skips. Four of the five are SAFETY GATES; the fifth is the analytics hook that caused L6 step 14 to fail. ## Evidence (verified 2026-05-17/18) **L6 step-14 reproduction (Phase B branch, post-prompt-rewrite):** bro invoked the Skill tool TWICE per `tool-uses.jsonl`: ``` {"name":"Skill","input":{"skill":"tmb:tmb_planning"}} {"name":"Skill","input":{"skill":"tmb:tmb_recovery"}} ``` But `skill_invocations` rows remain 0 — `skill-invocation-record.sh:61` does `WHERE name = '$SKILL_NAME'` with `SKILL_NAME='tmb:tmb_planning'`, but the catalog stores `name='tmb_planning'` (no prefix). No row matches → silent exit → no junction row. **Affected hooks:** | # | Hook | Line | Comparison | Severity | |---|---|---|---|---| | 1 | `scripts/hooks/skill-invocation-record.sh` | 61 | `WHERE name = '$SKILL_NAME'` | analytics; silent — step 14 fails | | 2 | `scripts/hooks/pr-reviewer-no-worktree.sh` | 35 | `[ "$SUBAGENT" = "pr-reviewer" ]` | **safety gate silently disabled** — pr-reviewer can spawn with worktree | | 3 | `scripts/hooks/require-task-spec.sh` | 15 | `[ "$AGENT_TYPE" != "swe" ]` | **safety gate silently disabled** — SWE can spawn without task_id in prompt | | 4 | `scripts/hooks/require-feature-branch-active.sh` | 21 | `[ "$AGENT_TYPE" = "swe" ]` | **safety gate silently disabled** — SWE can spawn off wrong branch | | 5 | `scripts/hooks/pr-reviewer-spawn-prompt-shape.sh` (Phase B NEW) | 26 | `[ "$SUBAGENT" = "pr-reviewer" ]` | **safety gate silently disabled** — malformed pr-reviewer spawn prompts pass | **Working hooks for pattern comparison** (handle both forms explicitly): - `scripts/hooks/swe-atomic-close.sh:75` — `[ "$AGENT_TYPE" != "swe" ] && [ "$AGENT_TYPE" != "tmb:swe" ]` - `scripts/hooks/git-push-guard.sh:93` — `[ "$AGENT_TYPE" = "tmb:swe" ] || [ "$AGENT_TYPE" = "swe" ]` **Why this wasn't caught:** - Existing lint `tests/lint/no-hardcoded-plugin-name.sh` catches `="tmb"` style hardcodes in path-construction code. It does NOT catch role-comparison patterns that omit the `tmb:` prefix variant. - Two hooks (`swe-atomic-close`, `git-push-guard`) added the `|| "tmb:swe"` workaround ad-hoc when their authors hit the bug. The pattern wasn't extracted into a helper; new hooks (including Phase B's `pr-reviewer-spawn-prompt-shape.sh`) inherited the wrong shape. ## Plan (structural, prevents future regression) ### 1. New helper — `scripts/hooks/lib/normalize-role.sh` ```bash # Strip plugin prefix from a role identifier. # CC passes role names with or without a `<plugin>:` prefix depending on # context (project-local override vs global agent vs slash-command). Strip # the prefix to get the canonical role name for comparison. # # Usage: role=$(normalize_role "$RAW") normalize_role() { printf '%s' "${1#*:}" } ``` `${1#*:}` strips the shortest match up to and including the first colon. If no colon present, returns the input unchanged. Works for `tmb:swe` → `swe`, `tmb-rc:pr-reviewer` → `pr-reviewer`, raw `swe` → `swe`. ### 2. Hook fixes (5 files) Each hook sources the helper and normalizes before comparing: ```bash . "$(dirname "${BASH_SOURCE[0]}")/lib/normalize-role.sh" # was: [ "$SUBAGENT" = "pr-reviewer" ] || exit 0 SUBAGENT=$(normalize_role "$SUBAGENT") [ "$SUBAGENT" = "pr-reviewer" ] || exit 0 ``` Equivalent for `AGENT_TYPE` and `SKILL_NAME` in the affected hooks. For `skill-invocation-record.sh`, normalize before the catalog SQL. The two already-correct hooks (`swe-atomic-close.sh`, `git-push-guard.sh`) should ALSO migrate to the helper — collapse the `|| "tmb:swe"` pattern into the normalized form. Same semantics, less drift surface. ### 3. New lint — `tests/lint/no-bare-role-compare.sh` Scans `scripts/hooks/*.sh` for `subagent_type`/`agent_type`/`skill` comparisons against bare role names without a normalize_role call earlier in the same script. Allowlist: the helper itself, and hooks that explicitly handle both forms (transitional during migration). Failure mode: clear error "hook `<file>:<line>` compares `<field>` against bare `<role>` without normalize_role. Either source `scripts/hooks/lib/normalize-role.sh` and normalize, or add the `|| tmb:<role>` form." ### 4. New L3 hook-script tests For each fixed hook, add an L3 test (`tests/hooks/*.test.sh`) that sends both the raw form (`"swe"`) and the prefixed form (`"tmb:swe"`) through the hook and asserts the gate fires identically. Covers regression of the same bug. ## Acceptance criteria - Helper `scripts/hooks/lib/normalize-role.sh` exists with one function. - All 5 affected hooks source the helper + normalize before compare. - The 2 already-correct hooks migrated to the helper (semantic-equivalent). - New lint `no-bare-role-compare.sh` catches the regression pattern; passes on dev. - L3 tests for each fixed hook verify both raw + prefixed forms fire the gate. - L6 step 14 passes naturally — `skill_invocations` row lands when bro fires `Skill(skill='tmb:tmb_planning')`. - L0+L1+L2+L3+L4 all green; no regressions. ## Out of scope - Changing CC's Skill/Agent tool input format (not under plugin control). - Migrating the role-handling logic into MCP server (helper is hook-local). - Refactoring agent_type values stored in the DB (catalog stays canonical without prefix). ## Coordination - L6 step 14 (`14-skill-invocation-recorded`) has been failing since at least 2026-05-15 due to bug #1 (analytics). Closing this issue + re-running L6 step 14 should turn it green. - The 4 safety-gate bugs are CRITICAL — they have been silently passing tests because the test suite invokes hooks with raw role names (`swe`), matching the buggy comparison. Real-world CC use with prefixed forms bypasses the gates. - This issue blocks shipping #2904 Phase A+B (the slim-skills MR) cleanly — Phase B's NEW `pr-reviewer-spawn-prompt-shape.sh` has the same bug and would land it into v0.7.0 dev. ## Note on source Discovered during post-Phase-B L6 step 14 re-test on 2026-05-17. The fix to step 14's prompt (commit `ed01f2a`) made bro invoke `Skill` correctly — exposing the underlying analytics hook bug. Codebase grep then surfaced the 4 safety-gate siblings.