Skip to content

feat(telemetry): add GDPR-compliant telemetry and TypeScript Ralph scripts - #101

Merged
lavaman131 merged 39 commits into
mainfrom
flora131/feature/add-anon-telem
Jan 25, 2026
Merged

feat(telemetry): add GDPR-compliant telemetry and TypeScript Ralph scripts#101
lavaman131 merged 39 commits into
mainfrom
flora131/feature/add-anon-telem

Conversation

@flora131

@flora131 flora131 commented Jan 22, 2026

Copy link
Copy Markdown
Collaborator

Summary

Implements a GDPR-compliant, privacy-first anonymous telemetry system for Atomic CLI that tracks command usage patterns without collecting any personally identifiable information. Additionally, converts Ralph loop shell scripts to TypeScript for improved type safety, cross-platform compatibility, and elimination of the jq dependency.


Privacy & Compliance

✅ Privacy-First Design

  • Explicit Consent Required: Telemetry is disabled by default; users must explicitly opt-in during atomic init or via atomic config
  • GDPR Compliant: Requires explicit consent before any data collection
  • Anonymous UUID: Rotated monthly to prevent long-term correlation
  • No PII Collection: Zero personally identifiable information tracked
  • No Session Duration: Removed to eliminate potential correlation risks
  • Minimal Data: Command names only—no prompts, paths, errors, or code content
  • Multiple Opt-Outs: ATOMIC_TELEMETRY=0, DO_NOT_TRACK=1, config command, automatic CI disable

❌ What We DON'T Collect

  • User prompts or command arguments
  • File paths, repo names, working directories
  • IP addresses or network identifiers
  • Error messages, stack traces, or code content
  • Session duration or timing data
  • Any personally identifiable information

✅ What We DO Collect

  • Command names (init, /research-codebase, etc.)
  • Agent type (claude, opencode, copilot)
  • Anonymous UUID (rotated monthly)
  • Platform metadata (OS, Atomic version)
  • Success/failure status

Features

🔒 Telemetry System

Event Tracking (3 Types):

  1. CLI Commands: init, update, uninstall, run
  2. Slash Commands via CLI: Parsed from CLI arguments (atomic -a claude -- /research-codebase)
  3. Agent Sessions: Via platform hooks (Claude Code, Copilot CLI, OpenCode)

Multi-Platform Hook Integration:

  • Claude Code: .claude/hooks/telemetry-stop.ts parses transcripts on session end
  • Copilot CLI: Three-hook strategy (prompt accumulation + session end)
  • OpenCode: .opencode/plugin/telemetry.ts SDK integration
  • Cross-platform: TypeScript implementation works on macOS, Linux, and Windows

User Control:

# Enable during init (interactive prompt with full disclosure)
atomic init

# Manage via config command
atomic config set telemetry true   # Enable
atomic config set telemetry false  # Disable

# Environment-based opt-out
export ATOMIC_TELEMETRY=0  # Disable via Atomic-specific var
export DO_NOT_TRACK=1      # Disable via standard opt-out

Data Flow:

User Action → Event Generation → JSONL Buffer → Background Upload → Azure App Insights
                                 (local file)     (detached process)

🔄 TypeScript Conversion

Converted Shell Scripts (.github/scripts/):

  • cancel-ralph.shcancel-ralph.ts (222 lines)
  • setup-ralph-loop.sh + log-ralph-prompt.shralph-loop.ts (366 lines)
  • start-ralph-session.shstart-ralph-session.ts (206 lines)

Benefits:

  • No jq dependency: Native JSON/YAML parsing with Bun APIs
  • Cross-platform: Single TypeScript file replaces separate .sh/.ps1 files
  • Type safety: Compile-time type checking for state management
  • Consistent patterns: Follows existing TypeScript hooks in .claude/ and .github/hooks/

State File Migration:

  • Old: JSON state files (.local.json)
  • New: YAML frontmatter markdown (.local.md) matching .opencode/ and .claude/ conventions

Implementation Details

Event Schema

All events include:

  • anonymousId: UUID v4 (rotated monthly)
  • eventId: Unique UUID v4 per event
  • eventType: atomic_command | cli_command | agent_session
  • timestamp: ISO 8601
  • Platform metadata: OS, Atomic version

Backend Integration

  • Azure Application Insights: Industry-standard telemetry backend
  • OpenTelemetry: Standard protocol for telemetry data export
  • Batch Upload: Events buffered locally, uploaded in background process
  • 30-Day Retention: Stale events automatically filtered before upload
  • Write-Only Access: Connection string safely committed (ingestion only, no read access)

Dependencies Added

  • ci-info@^4.3.1: CI environment detection
  • @types/ci-info@^3.1.4: TypeScript definitions
  • @azure/monitor-opentelemetry@^1.15.0: Azure App Insights SDK
  • @opentelemetry/api@^1.9.0: OpenTelemetry core API
  • @opentelemetry/api-logs@^0.52.0: OpenTelemetry logs API

File Changes

  • 71 files changed: 17,087 insertions, 934 deletions
  • New telemetry modules: src/utils/telemetry/ (11 TypeScript modules + test suites)
  • Platform hooks: .claude/, .github/, .opencode/ (TypeScript implementations)
  • Converted scripts: .github/scripts/ (3 shell scripts → TypeScript)

Testing

  • 7 comprehensive telemetry test suites with full coverage
  • 5 Ralph loop TypeScript conversion test suites including E2E tests
  • 100% coverage of telemetry modules
  • CI detection properly mocked and tested
  • E2E tests for agent detection across platforms
  • All tests passing ✅

Documentation

  • Telemetry Spec: specs/anonymous-telemetry-implementation.md (794 lines)
  • Backend Spec: specs/phase-6-telemetry-upload-backend.md (655 lines)
  • TypeScript Conversion Spec: specs/bun-shell-script-conversion.md (520 lines)
  • Copilot Agent Detection: specs/copilot-agent-detection-refactoring.md (616 lines)
  • Research Docs: 10 research documents totaling 5,100+ lines
  • User Docs: Updated README.md with comprehensive telemetry section

Migration Notes

No breaking changes.

  • Telemetry: Opt-in and disabled by default. Users must explicitly consent during atomic init or via atomic config set telemetry true. Existing users will be prompted on their next atomic init run.
  • Ralph Scripts: TypeScript versions are drop-in replacements with identical behavior. Old shell scripts have been deleted.

Security & Privacy Guarantees

  • Fail-Safe: Telemetry failures never impact CLI functionality
  • Transparent: Clear documentation of exactly what's collected and what isn't
  • Controllable: Multiple opt-out mechanisms (env vars, CLI, config)
  • Privacy-First: No PII, anonymous UUID rotated monthly, no session duration tracking
  • GDPR Compliant: Explicit consent required before any data collection
  • Industry Standard: Backend architecture mirrors Google Analytics, Segment, Mixpanel
  • Open Source: All telemetry code is publicly auditable in this repository

Add core telemetry module with privacy-preserving anonymous usage tracking:

- TelemetryState interface with enabled, consentGiven, anonymousId,
  createdAt, and rotatedAt fields
- Anonymous ID generation using crypto.randomUUID() (UUID v4)
- State persistence to ~/.local/share/atomic/telemetry.json
- Monthly ID rotation for enhanced privacy
- Priority-based opt-out: CI detection (ci-info) > ATOMIC_TELEMETRY env
  > DO_NOT_TRACK env > config file
- ATOMIC_COMMANDS constant for slash command tracking
- Comprehensive test suite with 100% coverage of core functions

Dependencies: ci-info ^4.3.1, @types/ci-info ^3.1.4

Ref: specs/anonymous-telemetry-implementation.md (Phase 1)
Assistant-model: Claude Code
Add telemetry tracking for Atomic CLI commands (init, update, uninstall, run).

- Create telemetry-cli.ts module with trackAtomicCommand function
- Implement JSONL event buffering to ~/.local/share/atomic/telemetry-events.jsonl
- Add AtomicCommandEvent interface matching spec Section 5.3.1 schema
- Integrate tracking into init, update, uninstall, and run-agent commands
- Add comprehensive unit tests for telemetry-cli module
- Add integration tests for end-to-end command tracking
- Update feature-list.json with Phase 2 completion status

Events track command name, agent type, success status, platform, and version.
All tracking respects opt-out mechanisms (ATOMIC_TELEMETRY=0, DO_NOT_TRACK=1).

Assistant-model: Claude Code
Add tracking for slash commands passed via CLI invocation (e.g.,
`atomic -a claude -- /research-codebase src/`). This complements
Phase 2 atomic command tracking by capturing skill usage analytics.

Changes:
- Add CliCommandEvent type and TelemetryEvent union type
- Implement extractCommandsFromArgs() to parse slash commands from CLI args
- Implement trackCliInvocation() function for cli_command events
- Integrate tracking into run-agent.ts before Bun.spawn()
- Export new types and functions from telemetry/index.ts
- Add comprehensive unit tests (11 extraction, 13 tracking tests)
- Add integration tests (9 tests for full CLI flow)
- Update feature-list.json with Phase 3 tasks
- Add progress.txt documenting implementation status

All 439 tests pass, lint passes, TypeScript compilation passes.

Refs: specs/anonymous-telemetry-implementation.md Section 5.3.2
Assistant-model: Claude Code
Add AgentSessionEvent type and session tracking utilities for
hook-based telemetry collection across all agent platforms.

- Add AgentSessionEvent interface with sessionId, sessionStartedAt,
  commands tracking (preserves duplicates for usage frequency)
- Create telemetry-session.ts with extractCommandsFromTranscript,
  createSessionEvent, and trackAgentSession functions
- Export session tracking functions from telemetry/index.ts
- Update test files with safer optional chaining
- Add comprehensive unit and integration tests (776+ lines)

Assistant-model: Claude Code
Implement Phase 4 session tracking hooks for Claude Code, Copilot CLI,
and OpenCode agents with three-hook accumulation strategy.

Claude Code:
- .claude/hooks/telemetry-stop.sh parses transcript_path from stdin JSON
- .claude/hooks/hooks.json registers Stop hook

Copilot CLI (three-hook accumulation):
- .github/hooks/prompt-hook.sh accumulates commands via userPromptSubmitted
- .github/hooks/stop-hook.sh writes event at sessionEnd
- .github/scripts/start-ralph-session.sh initializes temp files

OpenCode:
- .opencode/plugin/telemetry.ts tracks sessions via SDK

Shared:
- bin/telemetry-helper.sh provides common shell functions for hooks

Assistant-model: Claude Code
Document completed Phase 4 agent session tracking implementation:
- Update architecture diagram with Copilot three-hook strategy
- Add Copilot CLI userPromptSubmitted hook details
- Document command preservation (no deduplication) for usage frequency
- Mark Phase 4 checklist items as complete
- Add code references for new files
- Resolve open questions about Copilot transcript access and deduplication

Assistant-model: Claude Code
Add opt-in consent prompt during first-run and config command for
managing telemetry preferences at any time.

- Add telemetry-consent.ts module with consent prompt using @clack/prompts
- Add 'atomic config set telemetry <true|false>' command
- Integrate consent prompt into init command (skipped in --yes mode)
- Update README.md with telemetry documentation section
- Add unit tests for consent flow and config command

Assistant-model: Claude Code
@flora131 flora131 assigned lavaman131 and unassigned lavaman131 Jan 22, 2026
@claude claude Bot changed the title Flora131/feature/add anon telem feat(telemetry): implement privacy-preserving anonymous usage tracking Jan 22, 2026
Mock ci-info module in existing telemetry tests to prevent CI
detection from disabling telemetry during test execution. Add
dedicated telemetry-ci-detection.test.ts to verify telemetry
is correctly disabled when running in CI environments.

Assistant-model: Claude Code
@claude

claude Bot commented Jan 22, 2026

Copy link
Copy Markdown

PR Review: feat(telemetry): implement privacy-preserving anonymous usage tracking

Thanks for this comprehensive telemetry implementation! I've reviewed the code and have detailed feedback organized by category.


✅ Strengths

  1. Privacy-First Design: The implementation correctly follows privacy-preserving patterns:

    • Only collects command names, never prompts or arguments
    • Multiple opt-out mechanisms (env vars, config, CI auto-disable)
    • Monthly ID rotation for additional anonymity
    • GDPR-compliant opt-in consent flow
  2. Comprehensive Test Coverage: 776+ test lines with good coverage across:

    • Unit tests for all telemetry modules
    • Integration tests for hook functionality
    • CI detection tests in separate file (to avoid caching issues)
    • Edge cases like concurrent writes and error handling
  3. Fail-Safe Design: Telemetry failures never break CLI operation:

    • All tracking functions use try/catch with silent failures
    • Write errors are caught and ignored (telemetry-cli.ts:133-138)
  4. Good Documentation: Detailed spec and research docs explain design decisions.


⚠️ Potential Issues & Suggestions

1. Shell Script Portability (bin/telemetry-helper.sh)

# Line 209 uses macOS-specific date syntax
date -u -r "$START_SECS" +"%Y-%m-%dT%H:%M:%SZ"

Issue: The -r flag for date is macOS-specific. On Linux, this would be date -u -d @$START_SECS.

Suggestion: Add platform detection:

if [[ "$OSTYPE" == "darwin"* ]]; then
  date -u -r "$START_SECS" +"%Y-%m-%dT%H:%M:%SZ"
else
  date -u -d "@$START_SECS" +"%Y-%m-%dT%H:%M:%SZ"
fi

2. Missing jq Dependency Check (Shell Hooks)

The shell hooks use jq extensively but don't verify it's installed:

  • .claude/hooks/telemetry-stop.sh
  • .github/hooks/prompt-hook.sh
  • bin/telemetry-helper.sh

Suggestion: Add a check at the start of each hook:

if ! command -v jq &>/dev/null; then
  exit 0  # Fail silently without jq
fi

3. Race Condition Risk in JSONL Writes (telemetry-cli.ts:133)

appendFileSync(eventsPath, line, "utf-8");

While appendFileSync is atomic per-call, concurrent invocations (e.g., rapid CLI usage) could interleave lines if the kernel doesn't guarantee atomic appends for lines > 4KB.

Suggestion: For extra safety, consider file locking or using a write-ahead pattern for larger events (though the current implementation is probably fine for the expected event sizes ~200 bytes).

4. Temp File Location (.github/hooks/prompt-hook.sh:122)

COMMANDS_TEMP_FILE=".github/telemetry-session-commands.tmp"

Issue: This writes to the project directory, which could:

  • Pollute the working tree
  • Be accidentally committed
  • Fail if the directory is read-only

Suggestion: Use $TMPDIR or ~/.local/share/atomic/ instead:

COMMANDS_TEMP_FILE="${XDG_DATA_HOME:-$HOME/.local/share}/atomic/telemetry-session-commands.tmp"

Also add .github/telemetry-*.tmp to .gitignore as a safety measure.

5. OpenCode Plugin Event Types (.opencode/plugin/telemetry.ts:511-546)

if (event.type === "session.start" || event.type === "session.created") {

Question: Are these event types documented in OpenCode's plugin SDK? If not, they could break with SDK updates.

Suggestion: Add a comment referencing the OpenCode plugin documentation or version this was tested against.

6. Version String Handling (.opencode/plugin/telemetry.ts:402-404)

function getAtomicVersion(): string {
  return "unknown" // Plugin doesn't have easy access to atomic version
}

Suggestion: Consider reading from package.json if the plugin runs in the project context:

function getAtomicVersion(): string {
  try {
    const pkg = JSON.parse(readFileSync(join(process.cwd(), 'package.json'), 'utf-8'));
    return pkg.dependencies?.atomic || pkg.devDependencies?.atomic || "unknown";
  } catch {
    return "unknown";
  }
}

🔒 Security Review

No security concerns found. The implementation:

  • Never logs user prompts or file paths
  • Uses crypto.randomUUID() for IDs
  • Validates inputs before processing
  • Uses HTTPS for uploads (once implemented)
  • Has no command injection vectors in shell scripts (uses jq for JSON parsing)

🧪 Test Suggestions

  1. Add Test for Invalid JSON in State File: What happens if telemetry.json contains malformed JSON? The current readTelemetryState returns null, but a test would verify this.

  2. Add Cross-Platform Shell Hook Tests: Consider GitHub Actions matrix tests for the shell scripts on both Linux and macOS.

  3. Test Large Event Batches: Add a test that creates 1000+ events to verify JSONL integrity under load.


📝 Minor Style Notes

  1. Consistent Error Handling: Some places use catch {} while others use catch { /* comment */ }. Consider adding brief comments explaining why errors are ignored.

  2. Type Assertions: agentKey as AgentType in init.ts:271 and run-agent.ts:128 could use type guards for runtime safety.


Summary

This is a well-designed telemetry implementation that prioritizes user privacy. The code quality is high, tests are comprehensive, and the documentation is thorough.

Recommended actions before merge:

  1. Fix the macOS-specific date command in shell scripts
  2. Add jq dependency check to shell hooks
  3. Move temp files out of the project directory (or add to .gitignore)
  4. Consider the minor suggestions above

Overall: Approve with minor changes 👍

@claude

claude Bot commented Jan 22, 2026

Copy link
Copy Markdown

PR Review: Privacy-Preserving Anonymous Telemetry Implementation

This is a comprehensive review of the telemetry system implementation. Overall, this is a well-architected feature with strong privacy guarantees and extensive test coverage.


✅ What's Done Well

1. Privacy-First Design

  • Anonymous UUID v4 with monthly rotation
  • Explicit opt-in consent (GDPR compliant)
  • Multiple opt-out mechanisms (env vars, CLI command, config file)
  • CI environment auto-disable via ci-info package
  • Only collects command names, never prompts/paths/content

2. Architecture

  • Clean separation of concerns with modular telemetry modules
  • Factory pattern for event creation reduces duplication
  • Fail-safe design - telemetry never breaks CLI operation
  • Triple collection strategy covers all usage patterns (CLI commands, slash commands, session hooks)

3. Test Coverage

  • Comprehensive test suites (2,800+ lines)
  • CI detection tests in separate file to avoid module caching issues
  • Integration tests for end-to-end flows
  • Hook validation tests (bash syntax, JSON config)

4. Documentation

  • Detailed spec document (797 lines)
  • Research document explaining design decisions
  • Clear README.md section on telemetry

⚠️ Issues to Address

1. Security: Shell Script Command Injection Risk
In bin/telemetry-helper.sh:132-133:

if echo "$transcript" | grep -qE ...

The $transcript variable is passed directly to echo/grep without proper escaping. If a transcript contains malicious patterns, this could cause issues. Consider:

if printf '%s' "$transcript" | grep -qE ...

2. Race Condition in Temp File Usage
In .github/hooks/prompt-hook.sh:122 and stop-hook.sh:184:

COMMANDS_TEMP_FILE=".github/telemetry-session-commands.tmp"

Using a hardcoded temp file path could cause issues with concurrent sessions. Consider using a unique file per session (e.g., include session ID or PID in filename).

3. Missing Error Handling for jq Dependency
The bash hooks assume jq is installed but don't check for it. If jq is missing, the hooks will fail silently. Consider adding a dependency check:

if ! command -v jq &>/dev/null; then
  exit 0  # or handle gracefully
fi

4. Potential Infinite Loop in Upload Logic
In bin/telemetry-helper.sh:261:

spawn_upload_process() {
  if command -v atomic &>/dev/null; then
    nohup atomic --upload-telemetry > /dev/null 2>&1 &
  fi
}

The --upload-telemetry flag isn't documented in the spec's implementation checklist as completed. If it spawns another upload process, there's no recursion guard beyond the env var check.

5. OpenCode Plugin: Missing Session ID Tracking
In .opencode/plugin/telemetry.ts:449:

sessionId: eventId,

The sessionId is the same as eventId. For proper session correlation, consider using a separate sessionId that persists across the session.


🔍 Minor Suggestions

1. Constants Duplication
The ATOMIC_COMMANDS list is defined in three places:

  • src/utils/telemetry/constants.ts
  • bin/telemetry-helper.sh
  • .opencode/plugin/telemetry.ts

Consider generating the bash array from the TypeScript source during build, or at minimum add a comment noting these must stay in sync.

2. Date Formatting on macOS
In .github/hooks/stop-hook.sh:209:

SESSION_STARTED_AT=$(date -u -r "$START_SECS" +"%Y-%m-%dT%H:%M:%SZ" 2>/dev/null || echo "")

The -r flag for date works differently on macOS vs Linux. On macOS, -r takes a file reference, not seconds. Consider using:

date -u -d "@$START_SECS" +"%Y-%m-%dT%H:%M:%SZ" 2>/dev/null || 
date -u -r "$START_SECS" +"%Y-%m-%dT%H:%M:%SZ" 2>/dev/null || echo ""

3. Type Export Organization
In src/utils/telemetry/index.ts, consider grouping exports more clearly:

// Types (from types.ts)
export type { ... } from "./types";

// Core state management
export { ... } from "./telemetry";

// Event tracking
export { ... } from "./telemetry-cli";
export { ... } from "./telemetry-session";

// User consent
export { ... } from "./telemetry-consent";

4. Version Fallback in Plugin
In .opencode/plugin/telemetry.ts:402-404:

function getAtomicVersion(): string {
  return "unknown" // Plugin doesn't have easy access to atomic version
}

Consider reading from a version file or package.json if available.


📋 Performance Considerations

1. Synchronous File Operations
appendFileSync is used for writing events. For high-frequency operations, consider:

  • Using appendFile with callbacks/promises
  • Batching writes within a session

2. State File Reads
isTelemetryEnabledSync() reads the state file on every call. Consider caching the result for the lifetime of the process.


🔒 Security Checklist

Check Status
No PII collection
Anonymous ID rotation
Opt-out mechanisms
HTTPS for upload ✅ (assumed, endpoint TBD)
No secrets in hooks
Input validation ⚠️ See shell injection note

Summary

Recommendation: Approve with minor changes

This is a well-designed telemetry implementation that prioritizes user privacy. The test coverage is excellent and the documentation is thorough. The issues noted above are minor and can be addressed in a follow-up PR if needed.

Key strengths:

  • Privacy-first design with explicit consent
  • Comprehensive test coverage
  • Clean modular architecture
  • Fail-safe operation

Please address the shell script command injection risk before merging, as that's the most significant security concern.

Add consentGiven field to telemetry state and require both enabled
and consentGiven to be true before telemetry is sent. Also add
createdAt and rotatedAt fields to track anonymous ID lifecycle.

Assistant-model: Claude Code
@claude

claude Bot commented Jan 22, 2026

Copy link
Copy Markdown

PR Review: Privacy-Preserving Anonymous Telemetry System

Summary

This is a well-designed, comprehensive implementation of privacy-preserving telemetry for the Atomic CLI. The PR adds ~7,000 lines across 36 files, implementing a triple-collection strategy (CLI commands, slash command tracking, agent session hooks) with strong privacy guarantees.


✅ Strengths

Architecture & Design

  • Excellent documentation: The spec (specs/anonymous-telemetry-implementation.md) and research doc are thorough and follow industry best practices (Homebrew, VS Code, Yarn patterns)
  • Privacy-first design: Monthly UUID rotation, no PII collection, explicit consent required
  • Clean module structure: Telemetry code is well-organized in src/utils/telemetry/ with clear separation of concerns
  • Comprehensive opt-out mechanisms: Environment variables (ATOMIC_TELEMETRY=0, DO_NOT_TRACK=1), CLI command, and config file
  • CI auto-detection: Using ci-info package to automatically disable telemetry in CI environments

Code Quality

  • Strong test coverage: 2,800+ test lines across 7 test suites with good edge case coverage
  • Fail-safe behavior: All telemetry functions fail silently to never break CLI operation
  • TypeScript types: Well-defined interfaces for all event types
  • Factory patterns: Clean event creation via createBaseEvent() and createSessionEvent()

Security

  • No PII collected: Only command names, agent type, success/failure status
  • Local-first buffering: JSONL format allows user inspection and easy deletion
  • HTTPS transport: Specified for batch uploads

🔍 Areas for Improvement

1. Potential Race Condition in JSONL Writes (Low Risk)

telemetry-cli.ts:138-142 and telemetry-session.ts:141-145 use appendFileSync, which is atomic at the filesystem level but could lead to issues if multiple processes write simultaneously.

Recommendation: Consider using file locking for concurrent-safe writes, or document that this is acceptable given the low collision probability.

2. Shell Script Security Hardening

In bin/telemetry-helper.sh:198-214, the commands_str variable is passed to jq without strict validation. While this is internal telemetry data, consider:

# Current:
commands_json=$(echo "$commands_str" | tr ',' '\n' | jq -R . | jq -s .)

# Consider adding null check:
if [[ -z "$commands_str" ]]; then
  commands_json="[]"
else
  commands_json=$(echo "$commands_str" | tr ',' '\n' | jq -R . | jq -s .)
fi

3. Missing --upload-telemetry Implementation

The spec mentions a hidden --upload-telemetry flag and spawnTelemetryUpload() function (Spec Section 5.5), but I don't see the actual upload implementation in this PR. The local buffering works, but the batch upload to OTEL collector is referenced but not implemented.

Question: Is the upload functionality planned for a follow-up PR, or should this PR include it?

4. OpenCode Plugin Type Safety

In .opencode/plugin/telemetry.ts:282-283, the event property access lacks type guards:

const content = event.properties?.content
if (typeof content === "string") {

This is good defensive coding, but consider extracting a typed interface for the event structure to improve maintainability.

5. Copilot Hook Temp File Cleanup

In .github/hooks/stop-hook.sh, temp files are cleaned up on session end. However, if the hook fails mid-execution or the machine crashes, temp files could persist. Consider:

  • Adding temp file cleanup on session start (already done ✓)
  • Adding a max age check for stale temp files

6. Test Isolation Concern

Several test files create temp directories with Date.now() suffix, which is good. However, ensure parallel test runs don't collide:

// Current pattern (good):
const TEST_DATA_DIR = join(tmpdir(), "atomic-telemetry-test-" + Date.now());

Consider adding process.pid for extra uniqueness in CI environments.


📝 Minor Suggestions

Documentation

  1. The README telemetry section is clear and user-friendly ✓
  2. Consider adding a link to the full privacy policy/telemetry spec in the README

Code Style

  1. In telemetry-consent.ts:58-60, the multi-line string could use template literals for better readability
  2. The VERSION import from ../../version should be verified to exist and export correctly

Naming Consistency

  • isTelemetryEnabled (async) vs isTelemetryEnabledSync (sync) - good pattern
  • Consider documenting why both are needed (async for CI detection, sync for hot paths)

🧪 Test Coverage Notes

The tests are comprehensive. A few additional edge cases to consider:

  1. ✅ Corrupt telemetry.json handling
  2. ✅ Missing anonymousId in state
  3. ⚠️ Unicode commands in transcript (edge case)
  4. ⚠️ Very large transcript files (memory consideration)

🔐 Security Checklist

  • No secrets or credentials in code
  • No hardcoded endpoints (uses env vars)
  • GDPR-compliant consent flow
  • Monthly ID rotation for enhanced privacy
  • CI auto-disable prevents unintended collection
  • User-controlled opt-out mechanisms

Final Assessment

Overall: This is a well-thought-out implementation that balances product analytics needs with user privacy. The codebase follows modern best practices and the documentation is excellent.

Recommendation: Approve with minor suggestions

The main clarification needed is whether the batch upload functionality (--upload-telemetry handler and OTEL collector integration) is intentionally deferred or missing from this PR.


Review conducted on 2026-01-22

Simplify telemetry implementation by removing sessionStartedAt field
and session duration tracking. This reduces complexity without losing
critical command usage data.

Changes:
- Remove sessionStartedAt from AgentSessionEvent interface
- Remove session start timestamp storage in hooks
- Remove session duration metric from observability strategy
- Update spec to reflect simplified event schema
- Clean up temp file handling (removed SESSION_START_FILE)

Session end timestamp is still tracked via the timestamp field.

Assistant-model: Claude Code
@claude

claude Bot commented Jan 23, 2026

Copy link
Copy Markdown

PR Review: Anonymous Telemetry Implementation

Thank you for this comprehensive telemetry implementation. This is a well-architected feature with strong privacy considerations. Here's my detailed review:


✅ Strengths

Privacy-First Design

  • Anonymous UUID with monthly rotation - excellent for preventing long-term user tracking
  • Clear separation of what IS and IS NOT collected (documented in README and spec)
  • Multiple opt-out mechanisms: ATOMIC_TELEMETRY=0, DO_NOT_TRACK=1, atomic config set telemetry false
  • Automatic CI environment detection via ci-info package - prevents unintended collection in automation
  • GDPR-compliant explicit consent prompt during atomic init

Architecture

  • Clean separation of concerns with dedicated modules:
    • telemetry.ts - Core state management
    • telemetry-cli.ts - CLI command tracking
    • telemetry-session.ts - Agent session tracking
    • telemetry-consent.ts - User consent flow
  • JSONL buffering pattern enables offline resilience and batch uploads
  • Fail-safe design - telemetry failures never block CLI operation

Code Quality

  • Comprehensive test coverage (~2,800+ test lines across 7 test suites)
  • Good use of TypeScript interfaces for event schemas
  • Consistent error handling patterns

Documentation

  • Thorough spec document with Mermaid architecture diagrams
  • Clear README section explaining telemetry to users
  • Detailed research document capturing design decisions

🔍 Suggestions for Improvement

1. Security: Hardcoded Commands List Duplication

The ATOMIC_COMMANDS array is duplicated in multiple places:

  • src/utils/telemetry/constants.ts
  • bin/telemetry-helper.sh
  • .opencode/plugin/telemetry.ts

Consider generating the bash/TypeScript versions from a single source of truth to prevent drift.

2. Potential Race Condition in JSONL Writes

In telemetry-cli.ts:appendEvent() and telemetry-session.ts:appendEvent():

appendFileSync(eventsPath, line, "utf-8");

While appendFileSync is atomic at the OS level, concurrent processes (CLI + hooks) could interleave partial writes in rare cases. Consider using file locking or per-process temp files that get merged.

3. OpenCode Plugin: Missing Dependency Check

.opencode/plugin/telemetry.ts:462-465:

const atomicPath = ...
if (existsSync(atomicPath)) {

The Windows path construction may be incorrect:

process.platform === "win32"
  ? join(process.env.USERPROFILE || "", ".local", "bin", "atomic.exe")

On Windows, the standard install path is %USERPROFILE%\.local\bin\atomic.exe but this uses forward slashes via join. This should work, but verify on actual Windows.

4. Copilot CLI Temp File Location

.github/hooks/prompt-hook.sh:122:

COMMANDS_TEMP_FILE=".github/telemetry-session-commands.tmp"

This creates a temp file in the project directory. Consider using $TMPDIR or ~/.local/share/atomic/ to:

  • Avoid cluttering the project
  • Avoid accidental git commits of temp files

5. Missing .gitignore Entry

Add the temp file to .gitignore:

.github/telemetry-session-commands.tmp

6. Consent Prompt Timing

In src/commands/init.ts, the consent prompt happens BEFORE directory confirmation but AFTER agent selection. This means users see the telemetry prompt even if they cancel the init. Consider moving it after directory confirmation.

7. Test: Environment Variable Restoration

Several tests save/restore process.env:

const originalEnv = { ...process.env };
// ...
afterEach(() => {
  process.env = { ...originalEnv };
});

This shallow copy may not fully restore if tests delete keys. Consider:

afterEach(() => {
  Object.keys(process.env).forEach(key => delete process.env[key]);
  Object.assign(process.env, originalEnv);
});

8. Version Fallback in OpenCode Plugin

.opencode/plugin/telemetry.ts:387-389:

function getAtomicVersion(): string {
  return "unknown" // Plugin doesn't have easy access to atomic version
}

Consider reading from package.json or providing version via environment variable during install.


⚠️ Minor Issues

  1. Unused sessionStartedAt field: The AgentSessionEvent interface includes sessionStartedAt in the spec but it's not consistently populated in all code paths.

  2. Type assertion: In init.ts:266:

    trackAtomicCommand("init", agentKey as AgentType, true);

    The agentKey is already validated earlier - consider narrowing the type earlier to avoid the cast.

  3. Shell script compatibility: The bash scripts use set -euo pipefail which is good, but jq is a runtime dependency. Consider documenting this or providing a fallback.


🧪 Testing Notes

The test suite appears comprehensive. A few observations:

  • CI detection tests are properly isolated in telemetry-ci-detection.test.ts
  • Shell script syntax validation is included
  • Integration tests cover the full flow from command to JSONL file

Summary

This is a solid implementation of privacy-preserving telemetry. The architecture follows industry best practices (similar to Homebrew, Yarn, VS Code). The main areas for improvement are:

  1. Consolidate the commands list to a single source
  2. Consider file locking for concurrent writes
  3. Move temp files out of project directory
  4. Add .gitignore entry for temp files

Overall: Approve with suggestions - The privacy design is well thought out, code quality is high, and test coverage is thorough.

Add a guard at the top of all telemetry hook scripts to check for jq
availability before attempting any JSON parsing operations. This ensures
the hooks fail silently on systems without jq installed rather than
producing errors that could disrupt the user's workflow.

Assistant-model: Claude Code
@claude

claude Bot commented Jan 23, 2026

Copy link
Copy Markdown

PR Review: Privacy-Preserving Anonymous Telemetry Implementation

Overview

This is a well-structured and comprehensive telemetry implementation that demonstrates strong privacy-first design principles. The implementation follows GDPR compliance requirements with explicit opt-in consent and provides multiple opt-out mechanisms.


✅ Strengths

1. Privacy-First Architecture

  • Anonymous UUIDs rotated monthly prevent long-term tracking
  • Multiple opt-out mechanisms (environment variables, CLI command, config file)
  • CI environment auto-detection via ci-info package disables telemetry automatically
  • Explicit consent required before any data collection
  • Clear documentation of what IS and IS NOT collected

2. Robust Design Patterns

  • Fail-safe operation: telemetry failures never block CLI functionality (all try/catch blocks fail silently)
  • Local-first buffering with JSONL format for easy inspection and deletion
  • Triple collection strategy provides complete coverage (CLI commands, slash commands, session hooks)
  • Proper TypeScript types with comprehensive interface definitions

3. Excellent Test Coverage

  • ~2,800+ lines of tests across 7 test suites
  • CI detection properly mocked to allow testing in CI environments
  • Integration tests verify end-to-end flows
  • Hook scripts verified with bash syntax checks

4. Multi-Platform Support

  • Platform-specific hooks for Claude Code, Copilot CLI, and OpenCode
  • Shared bin/telemetry-helper.sh reduces code duplication
  • Cross-platform path handling (Windows, macOS, Linux)

5. Good Documentation

  • Comprehensive spec document (794 lines)
  • Clear research document (1,622 lines) explaining design decisions
  • README updated with user-facing telemetry documentation

⚠️ Suggestions for Improvement

1. Shell Script Robustness

In bin/telemetry-helper.sh and .claude/hooks/telemetry-stop.sh, the early exit for missing jq is good, but consider:

# Current (line 617 in bin/telemetry-helper.sh)
if ! command -v jq &>/dev/null; then
  exit 0  # Fail silently without jq
fi

This works, but when sourced as a library (not executed directly), exit 0 will exit the calling script. Consider using return 0 for sourced functions or wrapping in a guard:

# Suggestion: For sourced scripts
if ! command -v jq &>/dev/null; then
  # Define no-op functions when jq unavailable
  is_telemetry_enabled() { return 1; }
  extract_commands() { echo ""; }
  write_session_event() { return 0; }
  return 0 2>/dev/null || exit 0
fi

2. Potential Race Condition in JSONL Appending

The current implementation uses appendFileSync which is atomic on most systems, but consider adding file locking for high-concurrency scenarios:

// src/utils/telemetry/telemetry-cli.ts:117
function appendEvent(event: TelemetryEvent): void {
  // Current: appendFileSync(eventsPath, line, "utf-8");
  // Consider using Bun.file().writer() with proper flushing
}

This is a minor concern since telemetry is non-critical, but worth noting for robustness.

3. OpenCode Plugin Atomic Version

In .opencode/plugin/telemetry.ts:398:

function getAtomicVersion(): string {
  return "unknown" // Plugin doesn't have easy access to atomic version
}

Consider reading from package.json or a version file:

function getAtomicVersion(): string {
  try {
    const pkg = JSON.parse(readFileSync(join(__dirname, '../../../package.json'), 'utf-8'));
    return pkg.version || "unknown";
  } catch {
    return "unknown";
  }
}

4. Command Extraction Regex Edge Cases

In src/utils/telemetry/telemetry-session.ts:72:

const regex = new RegExp(`(?:^|\\s|[^\\w/])${escapedCmd}(?:\\s|$|[^\\w-:])`, "g");

This regex could have edge cases with special characters in command names. Consider adding unit tests for:

  • Commands at exact start/end of string
  • Commands followed by punctuation (e.g., /commit. or /commit,)
  • Commands in markdown code blocks (which might be false positives)

🔒 Security Considerations

1. No Sensitive Data Collection

The implementation correctly excludes:

  • User prompts and arguments
  • File paths and code content
  • IP addresses
  • Error messages with stack traces

2. State File Permissions

Consider ensuring the telemetry.json file has restrictive permissions (0600) to prevent other users from reading the anonymous ID:

// After writeFileSync
chmodSync(filePath, 0o600);

3. Upload Endpoint Not Hardcoded

Good: The OTEL endpoint is not hardcoded in the current implementation (Phase 6 upload not yet implemented).


🐛 Potential Issues

1. Consent Prompt Placement

In src/commands/init.ts, the consent prompt happens before directory confirmation:

// Line 142-149
if (!autoConfirm) {
  try {
    await handleTelemetryConsent();
  } catch {
    // Fail-safe
  }
}
// Line 152 - confirmDir prompt

This means users see the telemetry prompt even if they're going to cancel the init. Consider moving it after confirmDir or after agent selection.

2. Missing Telemetry State Initialization in Config Command

When running atomic config set telemetry true on a fresh install (no telemetry.json exists), setTelemetryEnabled creates the state correctly, but the anonymous ID generation happens lazily. This is fine, but document that the ID is generated on first enable.


📋 Minor Suggestions

  1. Type Safety: Consider using as const assertions for event type literals to catch typos at compile time

  2. Constants Duplication: The ATOMIC_COMMANDS array is duplicated in:

    • src/utils/telemetry/constants.ts
    • bin/telemetry-helper.sh
    • .opencode/plugin/telemetry.ts

    Consider generating the shell version from the TypeScript source during build.

  3. Test Cleanup: Some test files leave temp directories on failure. Consider using finally blocks or test framework hooks for cleanup.


✅ Conclusion

This is a well-implemented, privacy-respecting telemetry system that follows industry best practices (similar to Homebrew, Yarn, and Next.js telemetry). The code is well-organized, thoroughly tested, and properly documented.

Recommendation: Approve with minor suggestions

The suggestions above are quality improvements but not blockers. The implementation is production-ready and demonstrates excellent attention to privacy and user control.

Reorder the init flow so users confirm their target directory before
being prompted for telemetry consent. This provides a better user
experience by validating the primary action first.

Assistant-model: Claude Code
@claude

claude Bot commented Jan 23, 2026

Copy link
Copy Markdown

Pull Request Review: Anonymous Telemetry Implementation

Summary

This PR implements a comprehensive, privacy-first anonymous telemetry system for the Atomic CLI. The implementation is well-designed, thoroughly documented, and follows industry best practices.


✅ Strengths

Privacy & Security

  • Excellent privacy-preserving design with opt-in by default (GDPR compliant)
  • Monthly UUID rotation prevents long-term user tracking
  • Clear separation between what IS and what is NOT collected
  • Multiple opt-out mechanisms (ATOMIC_TELEMETRY=0, DO_NOT_TRACK=1, config command, CI auto-disable)
  • Uses ci-info package for automatic CI environment detection

Code Quality

  • Well-structured module organization under src/utils/telemetry/
  • Clear separation of concerns: telemetry.ts (state), telemetry-cli.ts (CLI tracking), telemetry-session.ts (hook tracking), telemetry-consent.ts (consent flow)
  • Comprehensive TypeScript types in types.ts
  • Factory pattern for creating base event fields reduces duplication
  • Fail-safe design - telemetry errors never break CLI functionality

Testing

  • Extensive test coverage (~2,800+ lines of tests across 7 test files)
  • Unit tests properly mock ci-info and other dependencies
  • Separate CI detection tests to handle module caching
  • Integration tests for hook functionality

Documentation

  • Detailed spec document (specs/anonymous-telemetry-implementation.md)
  • Comprehensive research document
  • README.md updated with clear telemetry disclosure

⚠️ Suggestions & Considerations

1. Telemetry Helper Shell Script - Error Handling
In bin/telemetry-helper.sh, the early exit on missing jq at the top level could be problematic - the exit 0 will exit any script that sources it. Consider changing this to a function-based check that returns early from individual functions instead of exiting the entire sourcing script.

2. Temp File Location for Copilot CLI
In .github/hooks/prompt-hook.sh:

COMMANDS_TEMP_FILE=".github/telemetry-session-commands.tmp"

This writes a temp file into the repository directory. Consider using a user-specific temp directory to avoid accidentally committing the temp file, potential conflicts with multiple concurrent sessions, and permission issues in read-only repos.

3. Potential Race Condition in Event Appending
In telemetry-cli.ts, appendFileSync is used for concurrent safety. While atomic at the OS level for small writes, consider adding a file lock mechanism for high-frequency concurrent writes, or document this as an acceptable trade-off for simplicity.

4. OpenCode Plugin - Version Hardcoding
In .opencode/plugin/telemetry.ts, getAtomicVersion() returns "unknown". Consider reading the version from package.json or the telemetry state file for meaningful version tracking.

5. UUID Fallback in Shell Script
The UUID fallback using /dev/urandom with od and awk does not produce a valid UUID v4 format. Consider using a more robust fallback or documenting this limitation.


🔍 Minor Issues

  1. Error Handling in config.ts - Uses process.exit(1) which requires mocking in tests. Consider using a custom error class or return pattern for better testability.

  2. Test File Organization - telemetry-ci-detection.test.ts exists separately due to ci-info module caching. Consider adding a comment explaining this in the main test files.

  3. README Documentation - The session hooks preserve duplicate commands for usage frequency tracking. Consider documenting this in the README so users understand what "commandCount" represents.


📋 Test Coverage Verification

The tests comprehensively cover:

  • ✅ Opt-out mechanisms (env vars, config file, CI detection)
  • ✅ Event schema validation (all required fields)
  • ✅ Command extraction from args and transcripts
  • ✅ Consent flow (first-run detection, user decisions)
  • ✅ Config command validation
  • ✅ Concurrent write handling
  • ✅ Fail-safe error handling

🔒 Security Review

  • ✅ No PII collection by design
  • ✅ Anonymous ID rotation
  • ✅ Local buffering before upload
  • ✅ HTTPS-only for uploads (when implemented)
  • ✅ No command arguments or file paths collected
  • ✅ Transcript parsing extracts only command names
  • ⚠️ Temp files in repo directory (see suggestion updates to readme and instructions #2)

Overall Assessment

This is a well-implemented privacy-first telemetry system. The architecture is clean, the tests are comprehensive, and the documentation is thorough. The suggestions above are minor improvements rather than blocking issues.

Recommendation: Approve with minor changes - Address the temp file location issue and consider the other suggestions for future iterations.


Review generated by Claude Code

The telemetry hook was writing pretty-printed JSON (multi-line) to the
events file, which broke JSONL format parsing in the upload handler.

This caused all events to be skipped during upload, preventing any
telemetry data from reaching Azure Application Insights.

Changed jq command from 'jq -n' to 'jq -nc' to output compact
single-line JSON, which is the correct format for JSONL files.

Reference: bin/telemetry-helper.sh:209
Add Copilot agent detection by parsing events.jsonl from session state:
- Detect agents via task tool calls, tool execution telemetry, and
  instruction headers in transformedContent
- Move telemetry tracking to run before Ralph loop check
- Remove userPromptSubmitted hook (replaced by sessionEnd detection)

Add background telemetry upload infrastructure:
- Spawn detached upload process on CLI exit via --upload-telemetry flag
- Add telemetry-upload.ts module with Azure Application Insights client
- Add telemetry-errors.ts and telemetry-file-io.ts for modular design
- Track nested --agent flags in copilot invocations

Refactor telemetry helpers:
- Add telemetry-helper.ps1 for Windows PowerShell support
- Enhance detect_copilot_agents function with three detection methods
- Add spawn_upload_process for non-blocking upload triggering
- Update events file path to include agent type suffix

Remove outdated test files:
- Tests moved to tests/ directory structure
- Will be refactored in separate commit

Assistant-model: Claude Code
Assistant-model: Claude Code
Add E2E bash tests for Copilot agent detection:
- copilot-agent-detection.test.sh: comprehensive agent detection tests
- test-agent-detection-e2e.sh: end-to-end validation
- test-copilot-agent-detection.sh: detection method verification

Reorganize test structure:
- Move config.test.ts from src/commands/ to tests/commands/
- Add refactored telemetry tests in tests/telemetry/
- Add test-utils.ts for shared telemetry test utilities
- Add atomic-commands-sync.test.ts for command list validation

Assistant-model: Claude Code
Remove dead code (getAtomicVersion stub) from telemetry.ts and normalize
version output in helper scripts by stripping "atomic v" prefix to match
the TypeScript VERSION constant format.

Assistant-model: Claude Code
Update feature-list.json with new detection implementation tasks and
progress.txt with implementation status (22/25 passing). Documents
that code is working but hooks execution needs investigation.

Assistant-model: Claude Code
Add leading slash to agent names when detected to match the format
used when agents are invoked (e.g., /code-review instead of code-review).

Assistant-model: Claude Code
Move jq availability checks from top-level early exits to individual
functions that require jq. This allows scripts that source the helper
to continue executing even when jq is unavailable, improving graceful
degradation.

Also removes unused telemetry initialization from start-ralph-session.sh
and adds documentation explaining appendFileSync atomicity guarantees.

Assistant-model: Claude Code
…le format

- Changed state file path from .local.json to .local.md
- Added parseRalphState() function using regex pattern for YAML frontmatter
- Added writeRalphState() function to write YAML frontmatter format
- Updated archive file format to use YAML frontmatter with completion metadata
- Removed unused checkCompletionPromise() function (handled by OpenCode plugin)
- Removed unused RalphState interface (replaced by ParsedRalphState)

This completes the migration from JSON to YAML frontmatter format for Ralph
loop state files, enabling consistent state file format across all scripts.

Assistant-model: Claude Code
- Changed sessionStart bash command to use bun run with start-ralph-session.ts
- Changed sessionStart powershell command to use bun run with start-ralph-session.ts
- Both sessionStart and sessionEnd hooks now use TypeScript consistently

This completes the migration of session hooks from shell scripts to TypeScript,
ensuring consistent behavior across platforms via Bun runtime.

Assistant-model: Claude Code
Delete shell scripts that have been replaced by TypeScript equivalents:
- .github/scripts/cancel-ralph.sh -> cancel-ralph.ts
- .github/scripts/setup-ralph-loop.sh -> ralph-loop.ts
- .github/scripts/start-ralph-session.sh -> start-ralph-session.ts
- .github/scripts/log-ralph-prompt.sh (no longer needed)

Update agent files to use the new TypeScript scripts:
- cancel-ralph.md: use bun run cancel-ralph.ts
- ralph-loop.md: use bun run ralph-loop.ts, update monitoring commands

This completes the migration from shell scripts to TypeScript for
cross-platform compatibility using Bun runtime.

Assistant-model: Claude Code
Add 46 unit tests for YAML frontmatter parsing and writing in
tests/ralph/yaml-frontmatter.test.ts covering:

- Parsing valid YAML frontmatter with all field combinations
- Handling missing optional fields with defaults
- Empty/malformed frontmatter error handling
- Completion promise variations (null, quoted, spaces)
- Cross-platform line ending normalization (CRLF/LF)
- Special characters (unicode, markdown, YAML-like content)
- Round-trip consistency across multiple cycles
- Edge cases (empty prompts, large values, --- delimiters)

All 534 tests pass.

Assistant-model: Claude Code
Add 41 unit tests for CLI argument parsing in
tests/ralph/ralph-loop-cli.test.ts covering:

- Help flags (-h, --help) and precedence
- Default values (prompt, iterations, completion promise)
- --max-iterations validation (positive integers, errors)
- --completion-promise handling (strings, spaces, warnings)
- --feature-list custom paths and validation
- Positional prompt arguments (single, multi-word, interspersed)
- State file creation (.local.md, .flag files)
- Output messages and orchestrator instructions
- Combined options in different orders

All 575 tests pass.

Assistant-model: Claude Code
Add 31 integration tests in tests/ralph/ralph-loop-integration.test.ts
covering the complete Ralph loop workflow:

- Loop setup: state file creation, continue flag, config options
- Session start hook: iteration increment on resume/startup
- Stop hook: session end logging, graceful missing state handling
- Cancel operation: archive creation, cleanup, no-op when inactive
- Max iterations: tracking, increment per session, unlimited mode
- Completion promise: storage, null handling, special characters
- Cross-platform: CRLF handling, Bun shebang, UTF-8 encoding
- Full lifecycle: setup -> cycles -> cancel, log accumulation
- Error handling: malformed JSON, empty input, missing directories

All 606 tests pass.

Assistant-model: Claude Code
- Update cancel-ralph.md with YAML frontmatter state file format
- Remove jq dependency documentation (uses grep-based parsing)
- Update slash-commands.md with .local.md state file paths
- Add migration note for legacy .local.json state files
- Mark Feature 10 as complete in feature-list.json
- Document completion of all 10 TypeScript conversion features

Assistant-model: Claude Code
…ependencies

Replace shell-based telemetry hooks with TypeScript implementations that
inline all dependencies. This eliminates cross-file imports that don't
work when plugins/hooks run in isolation from the main CLI binary.

Changes:
- Add .claude/hooks/telemetry-stop.ts (replaces telemetry-stop.sh)
- Update .opencode/plugin/telemetry.ts to inline all dependencies
- Update settings.json to use bun for TypeScript hook
- Update sync tests for new TypeScript-only architecture
- Remove obsolete shell scripts and documentation:
  - bin/telemetry-helper.sh, bin/telemetry-helper.ps1
  - .github/hooks/stop-hook.sh, .github/hooks/stop-hook.ps1
  - .claude/hooks/telemetry-stop.sh
  - docs/windows-telemetry.md
  - test/copilot-agent-detection.test.sh and related tests

Assistant-model: Claude Code
Add proper null checks and non-null assertions to satisfy TypeScript's
strict mode. Includes adding ParsedState interface, optional chaining
for regex matches, and null coalescing for potentially undefined values.

Assistant-model: Claude Code
@claude

claude Bot commented Jan 25, 2026

Copy link
Copy Markdown

PR Review: Privacy-Preserving Anonymous Telemetry

Overview

This is a large PR (17,000+ additions) that implements an opt-in telemetry system for Atomic CLI and converts several shell scripts to TypeScript. The PR demonstrates significant effort in privacy design, documentation, and testing.


Strengths

Privacy Design

  • Opt-in by default with explicit consent required - GDPR compliant approach
  • Monthly UUID rotation prevents long-term correlation
  • Multiple opt-out mechanisms (ATOMIC_TELEMETRY=0, DO_NOT_TRACK=1, config command, CI auto-disable)
  • Minimal data collection - only command names, no prompts, paths, or code content
  • Clear documentation of what is and is not collected

Code Quality

  • Comprehensive test coverage (2,800+ lines of telemetry tests, 1,900+ lines for Ralph TypeScript conversion)
  • Well-documented code with JSDoc comments and file headers
  • Good separation of concerns in src/utils/telemetry/ modules
  • TypeScript hooks follow existing patterns from .opencode/plugin/ralph.ts

Documentation

  • Detailed specs in specs/ directory
  • Research documents capturing decision rationale
  • Updated README with telemetry section

Issues to Address

1. Hardcoded Azure Connection String (Medium Priority)

In src/utils/telemetry/telemetry-upload.ts:52-53, the Azure connection string is hardcoded. While the comments explain this is write-only and safe to commit, consider using an environment variable override for flexibility. If this key ever needs rotation, all users would need to update.

Recommendation: Use process.env.ATOMIC_TELEMETRY_ENDPOINT as a fallback/override option.

2. Duplicated Code Across Hooks (Medium Priority)

The telemetry helper functions are duplicated across multiple files:

  • .claude/hooks/telemetry-stop.ts
  • .github/hooks/stop-hook.ts
  • .opencode/plugin/telemetry.ts

Each file has identical getTelemetryDataDir(), isTelemetryEnabled() functions, and ATOMIC_COMMANDS arrays with synchronization comments.

Recommendation: Create a shared bin/telemetry-helpers.ts module that can be imported, or generate these files from a single source of truth.

3. Error Handling in Background Process Spawn (Low Priority)

The background telemetry upload silently discards all errors (redirects to /dev/null). Consider logging to a debug file for troubleshooting.

4. Missing Cleanup for Legacy Files (Low Priority)

The PR adds per-agent telemetry files but findAllEventFiles() also looks for legacy telemetry-events.jsonl. Consider adding migration documentation.


Security Considerations

  1. Connection String Exposure: As noted, App Insights connection strings are write-only, but exposing the ApplicationId and IngestionEndpoint reveals your Azure infrastructure. This is generally acceptable for OSS projects but worth noting.

  2. Process Spawn Injection: The prompt escaping in stop-hook.ts uses simple quote escaping. Verify it handles all edge cases (newlines, backticks, etc.).

  3. File Permission: The telemetry files in ~/.local/share/atomic/ do not explicitly set restrictive permissions. Consider mkdirSync(dataDir, { recursive: true, mode: 0o700 }).


Suggestions

  1. Add telemetry to .gitignore template: Ensure users who clone projects do not accidentally commit their telemetry files.

  2. Consider rate limiting: The background upload spawns on every session end. Consider checking for an in-flight upload or adding a minimum interval between uploads.

  3. Add validation for command list sync: The ATOMIC_COMMANDS array in constants.ts is the source of truth but is duplicated in hooks. Consider a build-time check to verify they stay in sync.


Test Coverage

The test coverage looks comprehensive:

  • tests/telemetry/*.test.ts - Unit tests for all telemetry modules
  • tests/ralph/*.test.ts - Tests for TypeScript shell script conversions
  • Mocking of CI environment, file system, and external dependencies

Summary

This is a well-designed telemetry implementation with strong privacy guarantees. The opt-in approach with explicit consent is the right choice. The main concerns are:

  1. Code duplication across hooks (maintainability)
  2. Consider environment variable override for connection string (operational flexibility)
  3. Minor security hardening suggestions

Recommendation: Address the code duplication concern before merge to prevent synchronization issues as the codebase evolves. The other items can be addressed in follow-up PRs.

…ndencies

Remove backwards-compatible handling for legacy telemetry-events.jsonl
files since the 30-day grace period (Feb 22, 2025) has passed. Now only
agent-specific files (telemetry-events-{agent}.jsonl) are supported.

Also updates typescript, @azure/monitor-opentelemetry, and
@opentelemetry/api-logs to latest patch versions.

Assistant-model: Claude Code
@claude claude Bot changed the title feat(telemetry): implement privacy-preserving anonymous usage tracking feat(telemetry): implement privacy-preserving telemetry and TypeScript Ralph conversion Jan 25, 2026
@claude

claude Bot commented Jan 25, 2026

Copy link
Copy Markdown

PR Review: feat(telemetry): implement privacy-preserving anonymous usage tracking

Overall Assessment

This is a well-structured, comprehensive implementation of an opt-in anonymous telemetry system. The code demonstrates thoughtful privacy-by-design principles, thorough documentation, and extensive test coverage. The TypeScript conversion of shell scripts is also well-executed.


✅ Strengths

Privacy & Security

  • Explicit opt-in: Telemetry is disabled by default and requires explicit user consent during atomic init
  • Multiple opt-out mechanisms: ATOMIC_TELEMETRY=0, DO_NOT_TRACK=1, config command, automatic CI detection
  • Monthly UUID rotation: Anonymous IDs rotate monthly to prevent long-term correlation
  • Minimal data collection: Only command names, agent type, platform, and version—no PII, paths, or prompts

Code Quality

  • Well-organized module structure: Clear separation between core telemetry, CLI tracking, session tracking, upload, and consent
  • Comprehensive type definitions: Strong TypeScript types in types.ts with clear documentation
  • Fail-safe design: Telemetry errors are silently handled to never break CLI functionality
  • Factory pattern usage: createBaseEvent() and createSessionEvent() reduce code duplication

Testing

  • Extensive test coverage: 2,800+ lines of telemetry tests covering all major flows
  • Proper mocking: Tests mock ci-info and file paths to avoid polluting real config
  • Edge cases covered: Corrupted state, missing files, invalid JSON lines in JSONL

Documentation

  • Thorough PR description: Clear explanation of what is/is not collected
  • Inline documentation: JSDoc comments reference spec sections for traceability
  • Research documentation: Comprehensive research docs explaining design decisions

⚠️ Areas for Improvement

1. Code Duplication Across Hooks

Location: .claude/hooks/telemetry-stop.ts, .opencode/plugin/telemetry.ts

Issue: ATOMIC_COMMANDS is duplicated in 3 places:

  • src/utils/telemetry/constants.ts (source of truth)
  • .claude/hooks/telemetry-stop.ts:21-35
  • .opencode/plugin/telemetry.ts:77-91

While the code comments mention this and reference a sync test, this creates maintenance burden. If a new command is added, three files need updating.

Suggestion: Consider:

  1. Having hooks dynamically read the commands list from a shared location, OR
  2. Add a pre-commit hook or CI check that verifies synchronization automatically

2. Potential Race Condition in Upload Process

Location: src/utils/telemetry/telemetry-upload.ts:316-342

Issue: The atomic rename strategy is good, but there is a small window where events could be lost if the process crashes after claiming files but before upload completes.

Suggestion: Consider adding a cleanup mechanism that scans for stale .uploading.* files on startup and either restores them or deletes them if too old.

3. Hardcoded Azure Connection String

Location: src/utils/telemetry/telemetry-upload.ts:52-53

Issue: While the comment correctly explains this is safe (write-only access), hardcoding connection strings makes it difficult to:

  • Test against different environments
  • Rotate keys if compromised

Suggestion: Consider using an environment variable with a fallback to the hardcoded value for flexibility.

4. Timestamp Truncation Inconsistency

Location: .claude/hooks/telemetry-stop.ts:188-189 vs src/utils/telemetry/telemetry.ts

Issue: Timestamp formatting differs:

  • Hook: new Date().toISOString().replace(/\.\d{3}Z$/, "Z") (truncates milliseconds)
  • Core module: new Date().toISOString() (includes milliseconds)

This inconsistency could make event correlation slightly harder.

Suggestion: Standardize on one format across all modules.

5. Missing Type Guard for Event Validation

Location: src/utils/telemetry/telemetry-upload.ts:89-98

Issue: Event validation uses loose type assertions.

Suggestion: Consider adding a proper type guard function for better type safety.


🐛 Potential Bugs

1. Silent Failure When ci-info Not Loaded

Location: src/utils/telemetry/telemetry.ts:228-232

Issue: isTelemetryEnabledSync() only checks CI if ci-info is already loaded. If it has not been dynamically imported yet, CI environments will not be detected by the sync version.

Impact: Low—most paths use the async version, and CI typically also has DO_NOT_TRACK or CI env vars set.

2. Command Extraction Regex Edge Cases

Location: src/utils/telemetry/telemetry-session.ts:69

Issue: The regex for command detection might have edge cases with commands at the very start or end of messages.

Suggestion: Add test cases for edge cases like command at start, end, or with newlines.


📊 Performance Considerations

Synchronous File Operations in Hot Path

Location: src/utils/telemetry/telemetry-file-io.ts:30-52

Issue: appendEvent() uses synchronous file operations which blocks the event loop briefly.

Impact: Low—events are infrequent and writes are small.


🔒 Security Considerations

✅ Good Practices Observed

  1. No secrets or sensitive data in telemetry events
  2. Connection string is write-only (confirmed by Azure App Insights design)
  3. No command arguments/prompts are collected
  4. Proper input validation in consent flow

⚠️ Minor Notes

  1. The nohup command in .claude/hooks/telemetry-stop.ts:280 is appropriate for background upload
  2. The spawned upload process inherits env vars—ensure no sensitive env vars are accidentally logged

📝 Test Coverage Assessment

✅ Well Covered

  • Core telemetry state management
  • ID generation and rotation
  • Opt-out priority logic
  • Event creation and validation
  • CLI command extraction
  • Session event creation

⚠️ Could Use More Coverage

  • Concurrent upload race conditions
  • Network failure scenarios during upload
  • Malformed JSONL file recovery
  • Edge cases in command regex matching

🎯 Summary

Recommendation: ✅ Approve with minor suggestions

This PR demonstrates excellent engineering practices for telemetry implementation:

  • Privacy-first, opt-in design
  • Comprehensive documentation
  • Extensive test coverage
  • Fail-safe error handling

The issues identified are minor and do not block merging. Consider addressing the code duplication and timestamp inconsistency in a follow-up PR.


Reviewed files: 20+ source files across telemetry modules, hooks, tests, and TypeScript conversions

…nection string flexibility

- Standardize timestamp format in OpenCode plugin to match other hooks
  (truncate milliseconds for consistency across all telemetry sources)
- Add APPLICATIONINSIGHTS_CONNECTION_STRING env var override for flexibility
  (allows testing against different environments and key rotation)
@claude claude Bot changed the title feat(telemetry): implement privacy-preserving telemetry and TypeScript Ralph conversion feat(telemetry): add privacy-preserving telemetry and TypeScript Ralph conversion Jan 25, 2026
@claude

claude Bot commented Jan 25, 2026

Copy link
Copy Markdown

PR Review: feat(telemetry): implement privacy-preserving telemetry and TypeScript Ralph conversion

Overall Assessment

This is a well-structured, comprehensive PR that implements anonymous telemetry with strong privacy guarantees. The architecture follows good practices and the code is generally high quality. The TypeScript conversion of shell scripts improves maintainability and eliminates the jq dependency.


✅ Strengths

Privacy & Security

  • Excellent GDPR-compliant design with explicit opt-in consent required
  • Multiple opt-out mechanisms (ATOMIC_TELEMETRY=0, DO_NOT_TRACK=1, CI detection, config command)
  • Monthly UUID rotation prevents long-term user correlation
  • Minimal data collection - only command names, no PII, no prompts, no file paths
  • Azure App Insights connection string is write-only (safe to commit per industry standard)

Architecture

  • Clean module separation in src/utils/telemetry/ with clear responsibilities
  • Good use of TypeScript types for event schemas
  • Fail-safe error handling - telemetry never breaks CLI functionality
  • Atomic file operations with rename for upload concurrency safety

Testing

  • Comprehensive test coverage with ~2,800 test lines for telemetry
  • Synchronization tests for ATOMIC_COMMANDS across all hook files
  • Good use of mocking for CI detection and data directories

TypeScript Conversion

  • YAML frontmatter format for state files is a good choice
  • Cross-platform compatibility (single TS file vs separate .sh/.ps1)

⚠️ Potential Issues & Suggestions

1. Code Duplication

The telemetry helper logic is duplicated across multiple files:

  • .claude/hooks/telemetry-stop.ts
  • .github/hooks/stop-hook.ts
  • .opencode/plugin/telemetry.ts

Concern: Comments indicate this is intentional (TypeScript hooks cannot import at runtime), but this creates maintenance burden. Consider if there is a way to share code via a build step or extracting to a shared module that gets bundled.

2. Shell Command Injection Risk in Hooks

In .github/hooks/stop-hook.ts, the spawned shell command escapes the prompt but not the currentDir. If process.cwd() returned a path with single quotes, this could be exploited. Consider escaping both values consistently.

3. Async/Sync Mismatch in CI Detection

In telemetry.ts, isTelemetryEnabledSync() cannot check ci-info if not already loaded, which could lead to inconsistent behavior between sync and async versions. Consider preloading ci-info at module init time.

4. Stale Event Cleanup

filterStaleEvents() removes events >30 days old, but this only happens during upload. If a user disables telemetry, stale events accumulate indefinitely. Consider periodic cleanup or cleaning on disable.

5. Race Condition in Upload

If the process crashes after claiming files but before cleanup, .uploading.* files will be orphaned. Consider cleanup logic for stale .uploading.* files.


📝 Minor Suggestions

  1. Type annotation improvement - In several places, as any is used. Consider creating proper type guards instead.

  2. Consider using @opentelemetry/sdk-logs instead of the deprecated @opentelemetry/api-logs if available.

  3. README update could include a link to the detailed telemetry documentation in specs/.


🔒 Security Checklist

  • No PII collection
  • Explicit consent required
  • Multiple opt-out mechanisms
  • CI auto-disable
  • Connection string is write-only
  • No sensitive data in events
  • Events buffered locally before upload
  • Consider: Path escaping in shell spawn (minor risk)

📊 Test Coverage Assessment

The tests are comprehensive covering core telemetry state management, CLI tracking, session tracking, upload logic with batching and stale event filtering, command synchronization across files, and Ralph loop TypeScript conversion.

Suggested additions:

  • Test for orphaned .uploading.* file cleanup
  • Integration test for actual Azure upload (could use mock server)
  • Test for concurrent upload processes

Verdict

Approve with minor suggestions. This is a well-designed telemetry system with strong privacy guarantees. The code quality is high, error handling is appropriate, and test coverage is good. The suggestions above are improvements rather than blockers.

Great work on the privacy-first design!

Update bun.lock to reflect minor version updates for @azure/monitor-opentelemetry,
@opentelemetry/api-logs, and typescript.

Assistant-model: Claude Code
@claude claude Bot changed the title feat(telemetry): add privacy-preserving telemetry and TypeScript Ralph conversion feat(telemetry): add GDPR-compliant telemetry and TypeScript Ralph conversion Jan 25, 2026
@claude

claude Bot commented Jan 25, 2026

Copy link
Copy Markdown

Code Review: PR 101 - Anonymous Telemetry and TypeScript Ralph Conversion

I have reviewed this PR thoroughly. Here is my feedback organized by category:


Strengths

Privacy-First Design

  • Telemetry is correctly disabled by default - users must explicitly opt-in during atomic init
  • GDPR-compliant with explicit consent flow via promptTelemetryConsent()
  • Monthly UUID rotation provides good anonymization
  • Multiple opt-out mechanisms (env vars ATOMIC_TELEMETRY=0, DO_NOT_TRACK=1, CLI config, CI auto-disable)
  • Clear documentation of what is and is not collected

Code Quality

  • Good separation of concerns in telemetry modules (telemetry.ts, telemetry-upload.ts, telemetry-consent.ts, etc.)
  • Consistent error handling with handleTelemetryError() that fails silently to avoid breaking CLI
  • TypeScript conversion of shell scripts eliminates jq dependency and improves cross-platform compatibility
  • Comprehensive test coverage (2800+ lines for telemetry, 1900+ for Ralph scripts)

Architecture

  • Sensible use of JSONL buffering with background upload (fire-and-forget pattern)
  • OpenTelemetry plus Azure App Insights is a standard, proven stack
  • 30-day event staleness filtering is a good practice

Concerns and Suggestions

1. Code Duplication in Hooks
The telemetry helper functions are duplicated across multiple files: .claude/hooks/telemetry-stop.ts, .github/hooks/stop-hook.ts, and .opencode/plugin/telemetry.ts. Consider creating a shared build step that bundles shared utilities, or add tests that verify these duplicated functions behave identically.

2. YAML Frontmatter Parsing Edge Cases
The frontmatter parsing in parseRalphState() uses a simple regex. Consider what happens if the prompt body contains triple-dash separators, and note the getValue() function does not handle escaped quotes within strings.

3. Shell Command Injection in Copilot Hook
In .github/hooks/stop-hook.ts, the currentDir is used directly in Bun.spawn without escaping. If someone has a directory with special characters, this could break. Consider using proper argument passing rather than string interpolation.

4. Process Killing in cancel-ralph.ts
pkill -f copilot could kill unrelated copilot processes. Consider making the pattern more specific.

5. Missing Atomic File Operations
The comment mentions atomic file operations prevent duplicate uploads, but I do not see actual lockfile usage. Consider adding a lockfile for the upload process.


Security Assessment

  • No PII collection: Verified only command names, agent types, and anonymous UUIDs are collected
  • Safe connection string: Write-only access as documented
  • CI detection: Good use of ci-info to auto-disable in CI environments

Summary

This is a well-designed privacy-preserving telemetry system with good test coverage. The TypeScript conversion of shell scripts is a solid improvement. Address the code duplication concern and the shell injection edge case, and this should be good to merge.

Recommendation: Approve with minor changes (address shell escaping concern in stop-hook.ts)

Split the monolithic stop-hook.ts into three focused modules:
- telemetry-session.ts: incremental command logging on userPromptSubmitted
- telemetry-stop.ts: session end telemetry and upload spawning
- ralph-stop.ts: Ralph loop state tracking and session restart logic

Also removed unused .github/scripts/run.cmd and updated hooks.json
to wire up the new hooks with proper event triggers.

Assistant-model: Claude Code
Copilot AI review requested due to automatic review settings January 25, 2026 06:48
@claude claude Bot changed the title feat(telemetry): add GDPR-compliant telemetry and TypeScript Ralph conversion feat(telemetry): add GDPR-compliant telemetry and convert Ralph scripts to TypeScript Jan 25, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This pull request implements GDPR-compliant anonymous telemetry and converts Ralph loop shell scripts to TypeScript, representing a significant architectural improvement to the Atomic CLI.

Changes:

  • Adds privacy-first telemetry system with explicit consent, monthly UUID rotation, and multiple opt-out mechanisms
  • Converts Ralph loop shell scripts to TypeScript for cross-platform compatibility and type safety
  • Integrates telemetry hooks across Claude Code, OpenCode, and GitHub Copilot platforms

Reviewed changes

Copilot reviewed 69 out of 71 changed files in this pull request and generated 20 comments.

Show a summary per file
File Description
tests/telemetry/*.ts Comprehensive test suites with 2,800+ lines covering telemetry functionality
tests/ralph/*.ts Integration tests for TypeScript Ralph loop conversion (1,900+ lines)
tests/commands/config.test.ts Tests for new telemetry config command
src/utils/telemetry/*.ts Core telemetry implementation (11 modules)
src/commands/config.ts New config command for telemetry management
src/commands/*.ts Integration of telemetry tracking in init, update, uninstall, run-agent
src/index.ts Telemetry upload spawning and config command integration
.opencode/plugin/telemetry.ts Self-contained OpenCode telemetry plugin (418 lines)
.claude/hooks/telemetry-stop.ts Claude Code session end hook for telemetry
.github/hooks/*.ts GitHub Copilot telemetry hooks (session and stop)
.github/scripts/*.ts TypeScript Ralph loop scripts replacing shell scripts
package.json New dependencies for Azure App Insights and CI detection
README.md Telemetry documentation section
research/*.md Extensive research documentation (5,100+ lines)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

agentType,
success,
platform: process.platform,
atomicVersion: "0.1.0",

Copilot AI Jan 25, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The hardcoded version "0.1.0" in test fixtures should match the actual VERSION constant from src/version.ts to avoid inconsistencies. Consider importing and using the actual VERSION constant in tests.

Copilot uses AI. Check for mistakes.
* Read events from JSONL file
*/
export function readEvents(agentType?: string | null): TelemetryEvent[] {
const eventsPath = getEventsFilePath(agentType as any);

Copilot AI Jan 25, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The type assertion 'as any' bypasses TypeScript's type safety. Consider defining a proper type for the agentType parameter or using a more specific assertion like 'as AgentType | null' to maintain type safety.

Copilot uses AI. Check for mistakes.
} from "./types";
import { VERSION } from "../../version";
import { ATOMIC_COMMANDS } from "./constants";
import { appendEvent, getEventsFilePath } from "./telemetry-file-io";

Copilot AI Jan 25, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unused import getEventsFilePath.

Copilot uses AI. Check for mistakes.
import { VERSION } from "../../version";
import { ATOMIC_COMMANDS } from "./constants";
import { appendEvent, getEventsFilePath } from "./telemetry-file-io";
import { handleTelemetryError } from "./telemetry-errors";

Copilot AI Jan 25, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unused import handleTelemetryError.

Copilot uses AI. Check for mistakes.
* Reference: Spec Section 5.3.3
*/

import { readFileSync } from "fs";

Copilot AI Jan 25, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unused import readFileSync.

Copilot uses AI. Check for mistakes.
extractCommandsFromArgs,
getEventsFilePath,
} from "../../src/utils/telemetry/telemetry-cli";
import { writeTelemetryState, getTelemetryFilePath } from "../../src/utils/telemetry/telemetry";

Copilot AI Jan 25, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unused import getTelemetryFilePath.

Copilot uses AI. Check for mistakes.
}

// Helper to read all CliCommandEvents from all files
function readAllCliEvents(): CliCommandEvent[] {

Copilot AI Jan 25, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unused function readAllCliEvents.

Copilot uses AI. Check for mistakes.
createSessionEvent,
trackAgentSession,
} from "../../src/utils/telemetry/telemetry-session";
import { writeTelemetryState, getTelemetryFilePath } from "../../src/utils/telemetry/telemetry";

Copilot AI Jan 25, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unused import getTelemetryFilePath.

Copilot uses AI. Check for mistakes.
* - State initialization and lazy creation
*/

import { describe, test, expect, beforeEach, afterEach, mock, spyOn } from "bun:test";

Copilot AI Jan 25, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unused import spyOn.

Copilot uses AI. Check for mistakes.
Comment on lines +17 to +29
import {
generateAnonymousId,
getTelemetryFilePath,
readTelemetryState,
writeTelemetryState,
shouldRotateId,
rotateAnonymousId,
initializeTelemetryState,
getOrCreateTelemetryState,
isTelemetryEnabled,
isTelemetryEnabledSync,
setTelemetryEnabled,
} from "../../src/utils/telemetry/telemetry";

Copilot AI Jan 25, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unused import isTelemetryEnabledSync.

Copilot uses AI. Check for mistakes.
Update test file references to use the new modular hook filenames
introduced in the stop-hook refactoring.

Assistant-model: Claude Code
@claude claude Bot changed the title feat(telemetry): add GDPR-compliant telemetry and convert Ralph scripts to TypeScript feat(telemetry): add GDPR-compliant telemetry and TypeScript Ralph scripts Jan 25, 2026
@claude

claude Bot commented Jan 25, 2026

Copy link
Copy Markdown

PR Review: GDPR-Compliant Telemetry + TypeScript Shell Script Conversion

This is a substantial PR (17,087 additions) implementing anonymous telemetry with GDPR compliance and converting Ralph loop shell scripts to TypeScript. Overall, the implementation demonstrates strong privacy engineering practices and thorough documentation. Here is my detailed review:


Strengths

Privacy & GDPR Compliance

  • Opt-in by default - Telemetry is disabled until user explicitly consents during atomic init or via atomic config set telemetry true. This is the gold standard for GDPR compliance.
  • Multiple opt-out mechanisms - ATOMIC_TELEMETRY=0, DO_NOT_TRACK=1, config command, and automatic CI environment detection via ci-info.
  • Monthly ID rotation - Anonymous UUIDs rotate monthly (shouldRotateId() in telemetry.ts:149-157) preventing long-term user correlation.
  • Minimal data collection - Only command names, agent type, platform metadata. No PII, prompts, file paths, or error content.
  • Fail-safe design - Telemetry errors never impact CLI functionality (graceful degradation throughout).

Code Quality

  • Well-structured module system - Clean separation of concerns (telemetry.ts, telemetry-cli.ts, telemetry-upload.ts, telemetry-consent.ts, telemetry-session.ts).
  • Strong typing - TypeScript interfaces for all event types (TelemetryEvent, AtomicCommandEvent, CliCommandEvent, AgentSessionEvent).
  • Atomic file operations - Uses renameSync for claiming files during upload to prevent race conditions (telemetry-upload.ts:308-318).
  • Comprehensive test coverage - 2,800+ lines of telemetry tests, 1,900+ lines of Ralph loop tests.
  • Cross-platform - TypeScript scripts eliminate platform-specific shell/PowerShell duplication.

Documentation

  • Excellent spec documents with architecture diagrams, decision rationale, and explicit "what we DON'T collect" sections.

⚠️ Concerns & Suggestions

1. Duplicated ATOMIC_COMMANDS List (Medium Priority)

The list of trackable commands is duplicated in multiple locations:

  • src/utils/telemetry/constants.ts
  • .claude/hooks/telemetry-stop.ts:27-41
  • .github/hooks/telemetry-session.ts:13-26
  • .opencode/plugin/telemetry.ts

Comments reference "keep synchronized" but this is error-prone. Recommendation: Consider a single source of truth that hooks can import, or add a test that validates all lists are identical.

2. Connection String in Source Code (Low Priority - Acceptable)

The documentation correctly explains the App Insights connection string is safe (write-only ingestion). Worth noting:

  • Consider documenting key rotation procedures for when you need to change this
  • The ApplicationId in the connection string could be removed (it's optional and exposes internal resource ID)

3. YAML Frontmatter Parsing Without Library (Low Priority)

The YAML frontmatter parsing in Ralph scripts uses regex rather than a YAML parser. This works for the current simple structure but could break with multi-line values, escaped characters, or YAML comments. Since this is internal state file parsing (not user input), this is acceptable but worth a comment noting the limitations.

4. Potential Race Condition in Hook Command Extraction (Low Priority)

In .github/hooks/telemetry-session.ts, commands are appended to a temp file. If two userPromptSubmitted hooks fire rapidly, there's a small window for a race condition. Consider using file locking or append mode. The impact is minimal (possible missed command tracking).

5. Background Process Spawning (Informational)

The upload spawn patterns use different approaches (Bun shell syntax vs Node.js spawn). Ensure CI tests verify this doesn't leave zombie processes.


🔒 Security Review

  • ✅ No command injection vulnerabilities - inputs are properly handled through typed interfaces
  • ✅ File paths use path.join() throughout - no path traversal risks
  • ✅ JSON parsing has try/catch guards - no prototype pollution via JSON.parse
  • ✅ Connection string is write-only (ingestion endpoint) - no data exfiltration risk
  • ✅ No secrets beyond the App Insights connection string which is intentionally public
  • ✅ Hooks properly exit with code 0 to avoid blocking agent sessions

📊 Performance Considerations

  • ✅ Telemetry operations are designed to be non-blocking (background uploads)
  • ✅ Sync versions of telemetry checks available for performance-critical paths (isTelemetryEnabledSync)
  • ✅ JSONL format enables efficient append operations
  • ✅ Batch size limit (100 events) prevents memory issues during upload
  • ⚠️ ci-info dynamic import adds latency on first use - consider pre-loading during init

🧪 Test Coverage

The test suite is comprehensive with unit tests for all telemetry modules, integration tests for consent flow, and YAML frontmatter parsing edge cases.

Minor suggestion: Add a test case for the monthly ID rotation boundary (mocking dates around month transitions).


📝 Summary

Recommendation: Approve with minor suggestions

This is a well-engineered telemetry implementation that prioritizes user privacy. The GDPR compliance approach (explicit opt-in, clear disclosure, multiple opt-outs) is exemplary. The TypeScript conversion of shell scripts improves maintainability and cross-platform support.

The suggestions above are improvements rather than blockers. The code is production-ready.

Key metrics:

  • 71 files changed
  • Privacy-first design ✅
  • Test coverage ✅
  • Documentation ✅
  • Cross-platform ✅

@lavaman131
lavaman131 merged commit 484dccf into main Jan 25, 2026
4 checks passed
@lavaman131
lavaman131 deleted the flora131/feature/add-anon-telem branch January 25, 2026 07:05
lavaman131 added a commit that referenced this pull request Mar 26, 2026
feat(telemetry): add GDPR-compliant telemetry and TypeScript Ralph scripts
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.

3 participants