Skip to content

Refactor telemetry to use detached worker process - #65

Merged
knoxgraeme merged 4 commits into
mainfrom
claude/fix-telemetry-skill-key-n76do
May 3, 2026
Merged

knoxgraeme merged 4 commits into
mainfrom
claude/fix-telemetry-skill-key-n76do

Conversation

@knoxgraeme

Copy link
Copy Markdown
Owner

Summary

Refactors the telemetry system to dispatch events to a detached child process (telemetry-worker.js) instead of making HTTP requests directly from the CLI. This ensures the CLI returns to the user immediately without being blocked by network I/O, even if process.exit() is called in command code.

Key improvements:

  • Non-blocking: CLI no longer waits for telemetry HTTP requests to complete
  • Robust: process.exit() in commands no longer aborts in-flight telemetry
  • Flexible: Enhanced trackInstall() to capture platform/agent names where skills were installed
  • Better documentation: Expanded README with detailed telemetry opt-out and implementation details

Changes

Core Telemetry Refactor

  • src/telemetry.ts: Refactored to spawn a detached worker process instead of making direct HTTP requests

    • Moved HTTP logic to new telemetry-worker.ts
    • Changed trackCommand() and trackInstall() to return void instead of Promise<void>
    • Added isTelemetryDisabled() helper with improved env var handling (supports any truthy value, not just "1"/"true")
    • Added detection to skip telemetry when loaded from TypeScript source (dev/test via tsx)
    • Payloads sent via stdin to worker; parent process detaches immediately via child.unref()
  • src/telemetry-worker.ts (new): Standalone worker that reads JSON from stdin and POSTs to telemetry endpoint

    • Runs in detached child process spawned by parent CLI
    • Handles timeouts and network errors silently
    • Exits cleanly after request completes or times out

Enhanced Install Tracking

  • src/telemetry.ts: trackInstall() now accepts optional platform parameter (agent names)
  • src/commands/install.ts: Passes result.installed.map((i) => i.agent) to trackInstall()
  • src/commands/add.ts: Passes result.installed.map((i) => i.agent) to trackInstall()

Call Site Updates

  • Removed void prefix from all trackCommand() and trackInstall() calls (now return void, not Promise)
  • Updated in: add.ts, bundle.ts, init.ts, install.ts, list.ts, remove.ts, search.ts, submit.ts, update.ts

Documentation

  • README.md: Expanded telemetry section with:
    • Detailed explanation of what data is collected (command names, skill repos, platform names)
    • Clear opt-out instructions for DO_NOT_TRACK and CI env vars
    • Note that telemetry is auto-disabled when running from source
    • Explanation of detached worker implementation

Type of Change

  • New feature (non-blocking telemetry via detached worker)
  • Documentation update

Testing

The refactoring maintains backward compatibility in telemetry behavior:

  • Events are still sent to the same endpoint with the same payload structure
  • Opt-out via DO_NOT_TRACK=1 and CI=true still works
  • Error handling is preserved (failures swallowed silently)
  • Existing tests should pass; the change is internal implementation detail

Build verification: npm run build will compile both telemetry.ts and new telemetry-worker.ts to dist/.

https://claude.ai/code/session_01NNWdGxePTFy2h67N4bFjNy

claude added 4 commits May 3, 2026 20:52
trackInstall() POSTs the skill_key payload, but the call was
fire-and-forget (`void`) and process.exit() ran immediately after,
aborting the in-flight request. Only the trackCommand event (fired
at command start, with no skill_key) reliably reached the backend,
making it look like skill_key was missing from add/install events.

Collect the trackInstall promises and await Promise.allSettled before
exiting. sendTelemetry already enforces a 5s timeout, so the flush
cannot hang.
The previous fix awaited trackInstall before process.exit so the POST
carrying skill_key would actually flush. That worked, but it put the
telemetry endpoint on the user's critical path — a slow or down server
could add up to 5s of wall-clock latency to every successful install.

Move telemetry to the standard CLI pattern used by Next.js, Gatsby, etc:
spawn a detached child process that owns the HTTP request and survives
the parent's exit.

- New src/telemetry-worker.ts: reads JSON payload from stdin, POSTs it
  with a 5s timeout, exits silently on success or failure.
- telemetry.ts spawns the worker with detached:true and unref(), then
  writes the payload to its stdin and returns immediately. Small JSON
  fits in the kernel pipe buffer so the write completes synchronously
  and the worker reads it after the parent exits.
- trackCommand/trackInstall now return void (truly fire-and-forget); the
  `void` prefix at call sites was misleading and is removed.
- Telemetry is auto-skipped when loaded from .ts source (tsx in
  dev/test) since the compiled worker only ships in dist/.

Smoke-tested against a local fake endpoint: parent spawn+stdin-write
took ~4ms and the server received the full payload including skill_key.
- trackInstall now accepts the list of agent names the skill was
  installed to (e.g. ['Claude Code', 'Cursor']). De-duplicated before
  send. Lets us answer "which agents are people actually installing
  to?" without collecting any PII — agent names are product names, not
  user identifiers.
- Loosen the DO_NOT_TRACK / CI env check: any non-empty value other
  than "0" / "false" disables. Previously only literal "1" worked, so
  DO_NOT_TRACK=true silently did nothing.
- Expand README disclosure: list every field we send, document the
  detached-worker dispatch, link the DNT convention.

Smoke-tested against a local fake endpoint:
  agents: install event arrived with agents:["Claude Code","Cursor"]
  DNT=true: no request fired
  DNT=0:    request fired (per DNT spec)
The telemetry_events table column is named 'platform', not 'agents'.
Renaming the install payload field so the backend reads the value
without a translation layer.

Smoke-tested: wire payload now arrives as
  "platform":["Claude Code","Cursor"]
@knoxgraeme
knoxgraeme merged commit 23a2ca4 into main May 3, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants