feat(telemetry): emit skill invocations as cli_skill_invoked events - #2023
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds detached telemetry for “skill invocation” events surfaced by agent hooks, and wires lifecycle session handling to forward newly recorded SkillEvents as privacy-preserving analytics events (skill/agent/signal/event_type only).
Changes:
- Introduces
SkillInvocation,BuildSkillEventPayload, andTrackSkillInvocationsDetachedto emitcli_skill_invokedevents from a detached subprocess. - Updates lifecycle session mutation flow to dedupe/append skill events and forward newly appended events to telemetry (gated by the user’s telemetry opt-in setting).
- Adds unit tests validating skill payload shape and that
appendEventSkillEventsToStatereturns only newly appended events.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| cmd/entire/cli/telemetry/detached.go | Adds skill-invocation telemetry payload + detached tracking loop. |
| cmd/entire/cli/telemetry/detached_test.go | Adds tests for building skill telemetry payloads and privacy constraints. |
| cmd/entire/cli/lifecycle.go | Threads newly appended skill events through session state + forwards them to telemetry behind opt-in gating. |
| cmd/entire/cli/lifecycle_test.go | Adds coverage for “return only newly appended skill events” behavior. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
Addressed all three review findings in 9c05a09:
Full suite re-run: 9,170 unit / 492 integration / canary green. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.
Suppressed comments (2)
cmd/entire/cli/lifecycle.go:1595
- This only forwards hook-provided events that passed through
appendEventSkillEventsToState. Claude Code'sSkillEventExtractorruns later during condensation (strategy/manual_commit_condensation.go:1116,1162) and its results are merged directly into checkpoint metadata at line 396, soskill_tool_use/tool_invocationevents never reach this loop or PostHog. Wire newly merged extractor events into the telemetry path (while deduping against state), otherwise the new event undercounts the main tool-based detection path.
for _, ev := range appended {
invocations = append(invocations, telemetry.SkillInvocation{
Skill: ev.Skill.Name,
Agent: ev.Source.Agent,
Signal: ev.Source.Signal,
cmd/entire/cli/telemetry/detached.go:205
- Raw skill names are not content-free. The prompt extractor accepts any leading token matching
/<token>(agent/skill_events_prompt.go:12,48-61), and Claude's extractor copies the arbitrarySkillinput verbatim (agent/claudecode/transcript.go:328-341). A custom command such as/customer-acme-incident-123would therefore be sent to PostHog. This differs from plugin telemetry, which explicitly allowlists official names before sending (plugin.go:71-85). Restrict raw names to a known allowlist or map unknown/custom names to a fixed category before adding this property.
"skill": inv.Skill,
9c05a09 to
5d52152
Compare
Address Copilot review on #2023: - persistEventMetadataToState now returns the newly-appended skill events and all four MutateSessionState call sites (plus the TurnStart closure) forward them to trackSkillInvocations only after the mutation returns — the settings load and detached spawn no longer run while the session lock is held, and never fire for events that failed to persist. - BuildSkillEventPayload defaults an empty agent to "auto", matching BuildEventPayload. - TrackSkillInvocationsDetached caps detached senders at 10 per call to guard against a pathological transcript producing a process storm. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Entire-Checkpoint: 01M08QAE03CF9G1R5KE77EGR72
5d52152 to
4015a35
Compare
|
Addressed the two remaining review findings in 4015a35, and rebased the stack on main:
Full suite re-run: 9,316 unit / 514 integration / canary green. |
Address Copilot review on #2023: - persistEventMetadataToState now returns the newly-appended skill events and all four MutateSessionState call sites (plus the TurnStart closure) forward them to trackSkillInvocations only after the mutation returns — the settings load and detached spawn no longer run while the session lock is held, and never fire for events that failed to persist. - BuildSkillEventPayload defaults an empty agent to "auto", matching BuildEventPayload. - TrackSkillInvocationsDetached caps detached senders at 10 per call to guard against a pathological transcript producing a process storm. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Entire-Checkpoint: 01M08QAE03CF9G1R5KE77EGR72
4015a35 to
9f5e2e2
Compare
|
Removed a stray |
Skill events are already extracted per agent and recorded in session state (SkillEventExtractor, appendEventSkillEventsToState), so internal usage is measurable from checkpoint refs — but nothing reached PostHog, leaving external skill adoption invisible while agent-help usage is tracked. That asymmetry biases any skills-vs-agent-help comparison toward the instrumented side. Forward exactly the newly-appended (deduped) skill events to a detached cli_skill_invoked event: skill name, agent, detection signal, event type. Content-free by construction — no prompt text, arguments, or transcript content — mirroring the plugin-name-only rule. Gated on the same opt-in telemetry setting as command tracking, plus the ENTIRE_TELEMETRY_OPTOUT env opt-out; hook lifecycle paths never produce cli_command_executed (the hooks tree is hidden), so this is the only signal that a skill fired. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Address Copilot review on #2023: - persistEventMetadataToState now returns the newly-appended skill events and all four MutateSessionState call sites (plus the TurnStart closure) forward them to trackSkillInvocations only after the mutation returns — the settings load and detached spawn no longer run while the session lock is held, and never fire for events that failed to persist. - BuildSkillEventPayload defaults an empty agent to "auto", matching BuildEventPayload. - TrackSkillInvocationsDetached caps detached senders at 10 per call to guard against a pathological transcript producing a process storm. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Entire-Checkpoint: 01M08QAE03CF9G1R5KE77EGR72
Two review findings: Raw skill names are user content. The prompt extractor accepts any leading /<token> and Claude's extractor copies the Skill input verbatim, so a custom command like /customer-acme-incident-123 would have reached PostHog. Names now pass through only when on the official allowlist (seeded with "entire", the agent-help skill); everything else is reported as the fixed "custom" category, keeping adoption volume measurable without the identifier — the skills equivalent of IsOfficialPlugin. Claude Code's skill_tool_use events never reached telemetry. They only surface via ExtractSkillEvents on the condensation and turn-end finalize paths, which merged them straight into checkpoint metadata — so the main tool-based detection path was invisible to the new event. Those paths now persist newly extracted events into SessionState.SkillEvents (the durable dedup that keeps reporting exactly-once across repeated full-transcript extraction) and forward exactly the new ones: CondenseSession returns them via CondenseResult.NewSkillEvents, its three callers emit after their MutateSessionState saves, and the finalize path is picked up by a state-growth snapshot around HandleTurnEnd in the lifecycle turn-end handler. The emitter moved to strategy (EmitSkillInvocationTelemetry); lifecycle's trackSkillInvocations delegates to it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ffolded one
The allowlist held only "entire" (the scaffolded agent-help skill), so every
skill Entire itself ships reported as "custom", indistinguishable from a
third-party one — undercutting the official-adoption-vs-agent-help comparison
this event exists to serve. entireio/skills publishes its 12 skills as a
Claude Code plugin named "entire", and Claude Code invokes a plugin skill as
"<plugin>:<skill>", so they arrive namespaced ("entire:search").
Matched by leaf name after stripping the namespace, rather than by an
"entire:" prefix: the namespace comes from whatever plugin the user installed
or authored locally, so everything after the colon is arbitrary user text —
prefix-matching would reopen the leak the allowlist exists to close. The
emitted vocabulary stays closed, with a test asserting no input escapes it.
Two judgment calls, both taking the safe direction:
- A skill released upstream before this cross-repo list is updated reports as
"entire:unlisted" rather than "custom", so a lagging list undercounts the
per-skill breakdown but never adoption volume. Without this the drift
silently recreates the bias against skills that motivated the event.
- Bare leaf names are deliberately NOT matched, so cross-agent installs
without a namespace (Pi normalizes "/skill:<name>" to a bare name; a
hand-copied SKILL.md has none) report as "custom". Matching them would fold
Claude Code's built-in /review and any local /search into Entire's numbers
— an overcount invisible in the data, where this undercount has a known
cause.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…tracts
Addresses trail 1070's findings.
Batching (high): the 10-event cap silently dropped invocations. Its rationale
("real turns carry at most a few skill events") stopped holding at 9f5e2e2,
when condensation began extracting from transcript offset 0 — the first
condensation of a session drains its whole backlog in one call, and a dropped
event is never re-reported because session state has already recorded it.
TrackSkillInvocationsDetached now builds every payload and hands them to one
detached child; the cap is gone. Oversized batches split across children
rather than truncate, so nothing is lost. __send_analytics (SendEvents)
accepts an array or a single object — the child re-executes os.Executable(),
so a self-update between spawn and exec can hand a payload to a different
build — and resolves the git version once per send instead of once per event.
This also answers the open batching thread on the PR.
Dedupe (medium): skillEventExists and skillEventKey both guarded
state.SkillEvents with different notions of duplicate, and exactly-once
telemetry now rests on that answer. The weaker one is gone (it compared IDs
only, and its field-wise fallback was unreachable for two ID-bearing events);
strategy.AppendNewSkillEvents is now the single dedupe for hook-provided and
transcript-extracted events alike.
Save ordering (medium): mutErr == nil was never proof of a save —
ErrMutationSkip returns nil and skips SaveSessionState. Every site was correct
only by ordering. MutateSessionStateSaved now reports whether the state was
persisted, and all eight emit sites gate on that instead.
Tests (medium): the previous rounds' contracts had no coverage. Added the
opt-in gate (including that an absent key does not send), the batching and
no-drop behavior, payload decoding both shapes, and the two contracts that
could regress silently — emission outside the session gate, and skip reported
as unsaved. The gate test discriminates on durability rather than visibility,
since a nested call shares the outer's state pointer; verified it fails when
the emit is moved back inside the closure.
Minors: len() guard on the turn-end snapshot slice, inlined the one-line
trackSkillInvocations pass-through at its call sites, and moved the
thrice-copied telemetry opt-in check into settings.IsTelemetryEnabled.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
c6d2c44 to
d5c69a3
Compare
https://entire.io/gh/entireio/cli/trails/1070
Skill events are already extracted per agent and recorded in session state (
SkillEventExtractor,appendEventSkillEventsToState), so internal skill usage is measurable from checkpoint refs — but nothing reached PostHog, leaving external skill adoption invisible while agent-help usage is tracked. That asymmetry biases any skills-vs-agent-help comparison toward the instrumented side.This PR forwards exactly the newly-appended (deduped) skill events to a detached
cli_skill_invokedevent: skill name, agent, detection signal, event type.BuildPluginEventPayload.ENTIRE_TELEMETRY_OPTOUTenv opt-out.cli_command_executed(the hooks tree is hidden), so this is the only telemetry signal that a skill fired.appendEventSkillEventsToStatenow returns the appended events instead of a bool so both call sites (TurnStart slash-command path andpersistEventMetadataToState) forward exactly what was new.Tests: payload shape + content-free assertions in
telemetry, append/dedup semantics inlifecycle_test. Full suite: 9,170 unit / 492 integration / canary green.🤖 Generated with Claude Code
Which skill names are sent
Skill names are arbitrary user tokens, so the emitted
skillproperty comesfrom a closed vocabulary: a recognized Entire skill name, or one of two fixed
categories (
custom,entire:unlisted). Recognition matches the leaf nameafter stripping Entire's plugin namespace — not the
entire:prefix itself,which comes from whatever plugin the user installed or authored and so vouches
for nothing.
Two scoping choices, both taking the direction whose error is visible:
while the skills ship from
entireio/skills, so a skill released upstreambefore the list is updated reports as
entire:unlisted— a lagging listundercounts the per-skill breakdown but never adoption volume.
cross-agent and can arrive without a namespace (Pi normalizes
/skill:<name>to a bare name; a hand-copiedSKILL.mdhas none), andthose report as
custom. Matching bare leaves would fold Claude Code'sbuilt-in
/reviewand any local/searchinto Entire's numbers — anovercount invisible in the data, where this undercount has a known cause.
Worth revisiting if cross-agent installs become a meaningful share.
entireio/pr-review-toolkitskills are deliberately out of scope: the CLI'sinstall hints for it are still placeholders (
skilldiscovery/registry.go), sonothing ships under that namespace yet.
Counting caveats
Which paths actually emit. The condense path (commit and session-end) is
where transcript-extracted events reach telemetry in practice. The turn-end
snapshot around
HandleTurnEndonly yields events when the turn had mid-turncommits —
finalizeAllTurnCheckpointsearly-returns on an emptyTurnCheckpointIDs— so in an ordinary turn it is a no-op and the condensepath provides the coverage. Two gaps stay open by design: sessions the
exited-owner sweep marks ENDED without condensing (when
sweepCondenseBudgetis exhausted) never emit their tool-based events, and
claudeSkillEventIDfalls back to a transcript-relative id when
tool_use_idis absent, whichshadow-vs-live extraction could double-count — pre-existing, and now costing a
telemetry event rather than a duplicate metadata row.
One action could emit twice. A typed
/entire:reviewcan produce aprompt_slash_commandevent from the hook path and askill_tool_useeventfrom the transcript extractor; the ids differ, so both are legitimately new and
both emit.
event_typedisambiguates, so count byevent_type, or countdistinct actions rather than raw
cli_skill_invokedvolume — a raw countwould inflate. Deliberately not deduped across signals: the two events are
distinct records the session UI relies on for its collapse behavior, so
collapsing them in telemetry would mean either dropping a recorded event or
adding a correlation identifier to the payload.
Measured rather than assumed: across 72 real Claude Code transcripts on this
machine, 8 contain
Skilltool invocations and none pairs one with a literal/<name>user message, so in practice the tool signal is what appears. Theinflation is a real possibility to guard the insight against, not something
observed.