[AI-55] fix: Capture skills invoked by slash command - #180
Conversation
Skills invoked as /plugin:skill produced no telemetry at all. Claude Code expands a slash invocation into the prompt instead of dispatching the Skill tool, so PostToolUse never fires and tool_input.skill never exists. The UserPromptExpansion hook's command_name is the only signal that carries the name. Measured before this change: one user had 37 native skill activations, of which only 13 were Skill tool calls, leaving roughly two thirds of their skill usage invisible to telemetry. A slash expansion is recorded with bw.tool = "Skill" so it matches the same @bw.tool:Skill queries as the tool path, with bw.hook preserving the true origin. Plugin commands are captured alongside skills and deliberately not distinguished. Expansion types other than slash_command are suppressed, since they carry no skill identity and would otherwise write content-free bw.identity rows.
Plugin Validation — PR #180Plugin: Verdict: PASS with 1 warning to resolve before merge and 2 minor doc fixes. No critical or major errors. No secrets, no credential exposure, no permission or auto-approval risk. Errors (must fix)None. Warnings (should fix)
|
| Check | Result |
|---|---|
Committed settings.local.json |
✅ None anywhere in the repo |
| Hardcoded secrets / credentials | ✅ None. Only regex hits are benign: README prose about secrets, a code comment, and a deliberate file:///etc/passwd negative-test fixture at test_emit.py:82 |
| Permission scoping | ✅ N/A — plugin ships no settings.json and requests no permissions |
| Dangerous command auto-approvals | ✅ None. Hook commands are fixed python3 <quoted-path> invocations with no interpolation of untrusted data |
| Broad file access | ✅ None requested |
Notable positives in the changed code:
- No prompt text leaks through the new path. The
UserPromptExpansionpayload carriescommand_args(the user's actual prompt text — visible in the test fixture attest_emit_identity.py:80), and_build_identity_attrsdeliberately reads onlycommand_name. That upholds the README's "never collects prompt text" guarantee at the exact point where it would have been easiest to break. - Collector allowlist is correctly implemented (
emit.py:31-47): enforceshttpsand restricts the host tobitwarden.pw/*.bitwarden.pw, checking the parsedhostnamerather than a substring — which defeats the userinfo bypasshttps://ait.bitwarden.pw@evil.com/.urlsplit'sValueErroron malformed input is caught so a bad env value fails closed instead of escaping at import. emit()does not mutate the caller's dict (emit.py:65,attrs = dict(attrs)), and the guard at line 66 preserves a caller-supplied non-emptyevent.timestamp. Both behaviors are covered by tests.- Fail-open holds: every hook exits 0,
urlopenis bounded at 1s inside the 5s hook timeout, all exceptions swallowed. Telemetry cannot break a session. - New import is
datetimefrom the standard library — no third-party dependency added.
Test coverage added
15 new tests across the two suites, covering timestamp presence, ISO-8601/UTC shape, millisecond precision, caller-supplied and empty-string precedence, non-mutation, slash-expansion skill recovery, bw.tool query parity, origin preservation in bw.hook, tool-path precedence over expansion, and all four _should_emit branches. Genuinely thorough for the logic under test — the one gap is that the input fixture is hand-authored rather than captured (see W1).
Validation environment note
./scripts/validate-plugin-structure.sh, ./scripts/validate-marketplace.sh, pnpm run lint, and python3 -m unittest could not be executed — Bash commands were not permitted in this sandbox. Their checks were instead traced by hand against the scripts' source and the files. Both scripts are expected to pass: required files present, all required plugin.json fields present with valid version format, CHANGELOG's first version entry (1.1.0) matching the manifest, README containing the substrings the script greps for. CI remains the authority on lint.yml (prettier + cspell) and validate-plugins.yml.
🤖 Bitwarden Claude Code ReviewOverall Assessment: APPROVE Re-reviewed after the follow-up docs commit. The Code Review DetailsNo outstanding findings. Previously raised and now closed:
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| "UserPromptExpansion": [ | ||
| { | ||
| "hooks": [ | ||
| { | ||
| "type": "command", | ||
| "command": "python3 \"${CLAUDE_PLUGIN_ROOT}/hooks/emit_identity.py\"", | ||
| "timeout": 5 | ||
| } | ||
| ] | ||
| } | ||
| ], |
There was a problem hiding this comment.
♻️ DEBT: Plugin README still documents only PostToolUse and SubagentStop as registered hooks.
Details and fix
plugins/bitwarden-ai-telemetry/README.md is the plugin's collection-disclosure document, and it enumerates both the registered hooks and the trigger for each event family:
- Line 7: "The plugin registers Claude Code lifecycle hooks (
PostToolUseandSubagentStop) that fire after tool use and subagent completion." - Line 11:
| `bw.identity` | `Task` / `Agent` / `Skill` tool use; subagent stop |
Both are now incomplete — UserPromptExpansion is a third registered hook, and bw.identity also fires on slash-command expansion. Suggest updating line 7 to include UserPromptExpansion and the bw.identity row to mention slash invocation (/plugin:skill), so the disclosure matches what actually ships.
| if h.get("expansion_type") != SLASH_EXPANSION: | ||
| return "" | ||
| return h.get("command_name") or "" |
There was a problem hiding this comment.
❓ QUESTION: Should command_source gate this, so project/user-level custom commands aren't counted as skills?
Details
The PR notes that a plugin command vs. a skill is deliberately not distinguished — that reads reasonable, since both are plugin-authored and fleet-meaningful. But expansion_type == "slash_command" also matches personal (~/.claude/commands) and project (.claude/commands) commands, which the test fixture's "command_source": "plugin" implies are distinguishable here.
Those land in bw.skill with bw.tool = "Skill", so they join the same @bw.tool:Skill dashboards as names that correspond to no plugin skill at all — which slightly cuts against the per-skill count accuracy this PR is restoring. Was including non-plugin sources intended, or would gating on command_source == "plugin" better match the metric's purpose?
There was a problem hiding this comment.
No. Behavior is intended.
The README's hook list and bw.identity event description were stale after this PR added the UserPromptExpansion hook for slash-command invocation.
1 participant
🎟️ Tracking
AI-55
📔 Objective
Skills invoked by slash command (
/plugin:skill) produced no telemetry at all, so per-skill usage counts understated real usage.Claude Code treats a slash invocation as a prompt expansion, not a Skill tool dispatch — it injects the skill body as a user message rather than calling the Skill tool.
PostToolUsetherefore never fires andtool_input.skillnever exists, soemit_identity.py'sTask|Agent|Skillmatcher never matches. Native telemetry still emitsskill_activated, which is why the gap was visible at all.UserPromptExpansionis the only hook event carrying the skill name, incommand_name.Investigation detail — measured impact and how the mechanism was established
Measured on live Datadog before this change:
Roughly 25% of activations were invisible fleet-wide, and about two thirds for the heaviest slash user — whose
committing-changescount read 1 against roughly 20 real uses.The mechanism was established by tracing all ~30 subscribable hook events during a slash invocation. Ten fired; none carried
tool_name = "Skill". The only tool call in the session was an incidentalReadof the skill's own reference file.Changes
UserPromptExpansionand emitbw.identitywithbw.skilltaken fromcommand_name.event.timestamp(ISO-8601 UTC, millisecond precision) on every emitted record in the sharedemit(), so allbw.*events carry a client clock rather than consumers falling back to collector ingest time. Millisecond precision matters because downstream deduplication keys include the timestamp.Notes for reviewers
@bw.tool:Skill, so their counts correct themselves once this ships — no dashboard work is required alongside this PR.bw.skill,bw.tool,bw.hookandevent.timestampare all already on the middleware allowlist.bw.tool = "Skill"even though no tool ran, so it matches the same@bw.tool:Skillqueries as the tool path.bw.hookpreserves the true origin (UserPromptExpansionvsPostToolUse).command_namemay name a plugin command rather than a skill. The two are deliberately not distinguished.slash_commandare suppressed rather than emitted — they carry no skill identity and would write content-freebw.identityrows./clear,/plugin) fire no prompt hook at all, so they cannot pollute counts.event.timestampkeeps it, and the caller's dict is never mutated.