Browse Source

fix(telemetry): honor opt-out across running processes (#1880)

Refresh consent across processes, reset identity on opt-out, discard obsolete buffers and stop subsequent requests/requeues. Preserve environment override precedence and document in-flight semantics.

Fixes #1869.
Colby Mchenry 6 days ago
parent
commit
51116a26cb
5 changed files with 191 additions and 46 deletions
  1. 2 0
      CHANGELOG.md
  2. 10 3
      TELEMETRY.md
  3. 92 0
      __tests__/telemetry-optout.test.ts
  4. 11 5
      docs/design/telemetry.md
  5. 76 38
      src/telemetry/index.ts

+ 2 - 0
CHANGELOG.md

@@ -145,6 +145,8 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 ### Fixes
 
+- Turning telemetry off now resets its identity and stops running processes from recording, sending, or restoring unsent data. (#1869)
+
 - Calls between JavaScript, JSX and TypeScript files keep their callers and callback flows.
 - Zustand actions keep their callers when read through typed stores, destructured from store state, or selected by a hook.
 - Steps diagrams retain database operations made through external client chains without inventing internal dependencies.

+ 10 - 3
TELEMETRY.md

@@ -26,7 +26,14 @@ toggle and never re-asks. If you never saw the installer (e.g. `npx` straight in
 a one-line notice is printed to stderr before the first time anything is sent.
 
 Off means off: when disabled, CodeGraph records nothing, opens no connection to the
-telemetry endpoint, and sends no "opted out" ping.
+telemetry endpoint, and sends no "opted out" ping. Running processes recheck the stored
+choice before recording, persisting, and each send. Turning it off removes the local
+identity and unsent queues (including claimed queues); turning it back on creates a new
+identity. An HTTP request already started cannot be recalled, but opt-out prevents later
+request chunks and prevents its unsent data from being requeued.
+
+Environment overrides still apply: `CODEGRAPH_TELEMETRY=1` explicitly forces telemetry
+on for that process even when the stored choice is off; `DO_NOT_TRACK=1` takes precedence.
 
 Separately from telemetry, the MCP server checks GitHub for a newer release in the
 background (at most once a day) so it can tell you an update exists — it fetches a
@@ -96,8 +103,8 @@ source lives in [`telemetry-worker/`](telemetry-worker/) in this repository. It
 every event and property against the allowlist above (anything else is dropped), never
 reads the client IP, and rate-limits per machine ID. Sends are fire-and-forget with a
 short timeout: offline or air-gapped machines buffer a bounded local file (256 KB cap)
-and never retry-loop, log errors, or slow a command down. Telemetry never adds latency to
-MCP tool calls — recording is an in-memory counter.
+and never retry-loop, log errors, or slow a command down. Recording refreshes the small local consent file, then increments an in-memory counter;
+MCP tool calls never wait for telemetry network requests or queue writes.
 
 ## Where it is stored
 

+ 92 - 0
__tests__/telemetry-optout.test.ts

@@ -0,0 +1,92 @@
+import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
+import * as fs from 'fs';
+import * as os from 'os';
+import * as path from 'path';
+import { Telemetry } from '../src/telemetry';
+
+describe('telemetry opt-out across running instances (#1869)', () => {
+  let dir: string;
+  let now: Date;
+  let sends: any[];
+  const make = (env = {}, fetchImpl: typeof fetch = async (_url, init) => {
+    sends.push(JSON.parse(String(init?.body)));
+    return new Response(null, { status: 204 });
+  }) => new Telemetry({ dir, env, fetchImpl, now: () => now, stderr: () => {}, installExitHook: false });
+  const queued = () => fs.readdirSync(dir).filter(n => n.startsWith('telemetry-queue'));
+  beforeEach(() => { dir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-telemetry-off-')); now = new Date('2026-06-12T08:00:00Z'); sends = []; });
+  afterEach(() => { vi.useRealTimers(); fs.rmSync(dir, { recursive: true, force: true }); });
+
+  it('removes the identity on off and assigns a new one on on', () => {
+    const a = make(); a.setEnabled(true, 'cli'); const old = a.getStatus().machineId;
+    make().setEnabled(false, 'cli');
+    expect(a.getStatus()).toMatchObject({ enabled: false, machineId: null });
+    expect(fs.readFileSync(a.configPath, 'utf8')).not.toContain(old!);
+    make().setEnabled(true, 'cli');
+    expect(a.getStatus().machineId).toBeTruthy(); expect(a.getStatus().machineId).not.toBe(old);
+  });
+
+  it.each(['persist', 'flush'] as const)('drops memory and new recording after another instance opts out: %s', async action => {
+    const a = make(); a.setEnabled(true, 'cli'); a.recordLifecycle('install', {});
+    make().setEnabled(false, 'cli'); a.recordLifecycle('index', {}); a.recordUsage('mcp_tool', 'codegraph_explore', true);
+    if (action === 'persist') a.persistSync(); else await a.flushNow();
+    expect(sends).toEqual([]); expect(queued()).toEqual([]);
+    make().setEnabled(true, 'cli'); await a.flushNow();
+    expect(sends).toEqual([]); expect(queued()).toEqual([]);
+  });
+
+  it('observes external off at the completed-day interval', async () => {
+    vi.useFakeTimers(); const a = make(); a.setEnabled(true, 'cli'); a.recordUsage('cli_command', 'query', true);
+    a.startInterval(); await vi.advanceTimersByTimeAsync(0);
+    make().setEnabled(false, 'cli'); now = new Date('2026-06-13T08:00:00Z');
+    try { await vi.advanceTimersByTimeAsync(6 * 60 * 60_000); expect(sends).toEqual([]); expect(queued()).toEqual([]); }
+    finally { a.stopInterval(); }
+  });
+
+  it('does not reuse pre-opt-out memory after an off/on cycle the process missed', async () => {
+    const a = make(); a.setEnabled(true, 'cli'); a.recordLifecycle('install', {});
+    const b = make(); b.setEnabled(false, 'cli'); b.setEnabled(true, 'cli');
+    a.recordLifecycle('index', {}); await a.flushNow();
+    expect(sends).toHaveLength(1); expect(sends[0].events.map((e: any) => e.event)).toEqual(['index']);
+    expect(sends[0].machine_id).toBe(b.getStatus().machineId);
+  });
+
+  it.each([false, true])('in-flight failure cannot recreate old data after off (re-enable=%s)', async reEnable => {
+    let reject!: (error: Error) => void;
+    const a = make({}, async () => { sends.push('started'); return new Promise((_resolve, fail) => { reject = fail; }); });
+    a.setEnabled(true, 'cli'); a.recordLifecycle('install', {}); a.recordUsage('cli_command', 'query', true);
+    const flushing = a.flushNow(); expect(sends).toEqual(['started']);
+    const b = make(); b.setEnabled(false, 'cli'); if (reEnable) b.setEnabled(true, 'cli');
+    reject(new Error('network failure')); await flushing;
+    expect(queued()).toEqual([]); await b.flushNow(); expect(sends).toEqual(['started']);
+  });
+
+  it('checks consent before each request chunk after an in-flight request returns', async () => {
+    let finish!: (r: Response) => void;
+    const a = make({}, async () => { sends.push('started'); if (sends.length > 1) return new Response(null, { status: 204 }); return new Promise(resolve => { finish = resolve; }); });
+    a.setEnabled(true, 'cli'); for (let i = 0; i < 105; i++) a.recordLifecycle('index', {});
+    const flushing = a.flushNow(); expect(sends).toHaveLength(1);
+    make().setEnabled(false, 'cli'); finish(new Response(null, { status: 204 })); await flushing;
+    expect(sends).toHaveLength(1); expect(queued()).toEqual([]);
+  });
+
+  it('off removes stale claims as well as the queue so on cannot revive them', async () => {
+    const a = make(); a.setEnabled(true, 'cli');
+    const claim = path.join(dir, 'telemetry-queue.sending.98765.jsonl');
+    fs.writeFileSync(claim, JSON.stringify({ v: 2, ev: 'install', ts: now.toISOString(), props: {} }) + '\n');
+    const old = new Date(now.getTime() - 2 * 60 * 60_000); fs.utimesSync(claim, old, old);
+    a.setEnabled(false, 'cli'); expect(queued()).toEqual([]); a.setEnabled(true, 'cli'); await a.flushNow(); expect(sends).toEqual([]);
+  });
+
+  it.each(['DO_NOT_TRACK', 'CODEGRAPH_TELEMETRY'])('environment off drops pending memory: %s', async key => {
+    const env: NodeJS.ProcessEnv = {}; const a = make(env); a.setEnabled(true, 'cli'); a.recordLifecycle('install', {});
+    env[key] = key === 'DO_NOT_TRACK' ? '1' : '0'; await a.flushNow(); a.persistSync(); delete env[key]; await a.flushNow();
+    expect(sends).toEqual([]); expect(queued()).toEqual([]);
+  });
+
+  it('retains the documented explicit environment-on override without resurrecting the old identity', async () => {
+    const a = make(); a.setEnabled(true, 'cli'); const old = a.getStatus().machineId; a.setEnabled(false, 'cli');
+    const forced = make({ CODEGRAPH_TELEMETRY: '1' }); forced.recordLifecycle('index', {}); await forced.flushNow();
+    expect(sends).toHaveLength(1); expect(sends[0].machine_id).toMatch(/^[0-9a-f-]{36}$/); expect(sends[0].machine_id).not.toBe(old);
+    expect(make().isEnabled()).toBe(false);
+  });
+});

+ 11 - 5
docs/design/telemetry.md

@@ -41,8 +41,8 @@ Answer, in aggregate and anonymously:
 1. **The schema is the allowlist.** Client sends only the events below; the ingest Worker
    validates against the same allowlist and drops anything else. Adding a field = PR that
    edits this doc + `TELEMETRY.md` + the Worker allowlist together.
-2. **Telemetry may never cost the user anything**: zero added latency on the MCP tool-call
-   hot path (the repo's core invariant), zero new npm dependencies (global `fetch`, Node ≥18),
+2. **Telemetry may never cost the user anything**: no network requests or queue writes on the MCP tool-call
+   hot path (only a small local consent-file read), zero new npm dependencies (global `fetch`, Node ≥18),
    zero bytes on stdout (stdio is the MCP protocol channel), zero retries, zero error noise.
    Every failure mode is silence.
 3. **Off is off.** When disabled, no process opens a socket to the telemetry endpoint — not
@@ -53,7 +53,7 @@ Answer, in aggregate and anonymously:
 
 ## Events
 
-Common envelope on every batch (computed once per process):
+Common envelope on every batch (identity revalidated before each request):
 
 | field | example | notes |
 |---|---|---|
@@ -131,7 +131,12 @@ Surfaces:
   `codegraph collects anonymous usage stats (no code or paths) — "codegraph telemetry off" or CODEGRAPH_TELEMETRY=0 disables. Details: TELEMETRY.md`
 - **CLI:** `codegraph telemetry status|on|off` (status prints the machine ID, current
   state, and what decided it). Deleting `~/.codegraph/telemetry.json` resets everything,
-  including the machine ID.
+  including the machine ID. Turning telemetry off stores a null `machine_id` and removes
+  both queued and claimed unsent data. Turning it back on mints a new ID; processes
+  discard memory from the previous identity even if they missed the off/on transition.
+  Requests already in flight cannot be recalled, but every later request chunk and
+  requeue checks current consent and identity again. Config writes use atomic replacement
+  so concurrent readers never see a half-written choice.
 
 `~/.codegraph/telemetry.json`:
 
@@ -154,7 +159,8 @@ other filenames.)
 New module `src/telemetry/` (single small module, no deps):
 
 - **Counters in memory** — recording a tool call/CLI command is an in-memory increment.
-  Nothing on the hot path touches disk or network. MCP tool handlers call
+  The small consent file is refreshed before recording so another process's opt-out is
+  observed. No queue writes or network requests run on this path. MCP tool handlers call
   `telemetry.count('mcp_tool', name, ok)` and move on.
 - **Buffer** — counters persist (debounced, async) to `~/.codegraph/telemetry-queue.jsonl`.
   Hard cap ~256 KB; on overflow drop oldest lines. Corrupt buffer → truncate, never throw.

+ 76 - 38
src/telemetry/index.ts

@@ -5,7 +5,7 @@
  * (and user-facing TELEMETRY.md); the ingest endpoint that enforces it is
  * public at telemetry-worker/. This module honors four invariants:
  *
- * 1. Zero hot-path cost: recording is an in-memory increment. Disk writes are
+ * 1. Recording rechecks the small consent file, then increments in memory. Disk writes are
  *    a tiny synchronous append at process exit (works under `process.exit()`,
  *    where `beforeExit` never fires); network sends happen opportunistically
  *    (startup of long-running commands, daemon interval, bounded await at the
@@ -88,7 +88,7 @@ export interface ClientInfo {
 
 interface ConfigFile {
   enabled: boolean;
-  machine_id: string;
+  machine_id: string | null;
   consent_source: 'installer' | 'default-notice' | 'cli';
   first_run_notice_shown?: boolean;
   updated_at: string;
@@ -159,7 +159,7 @@ export class Telemetry {
   private events: EventLine[] = [];
   private readonly installExitHook: boolean;
   private exitHookInstalled = false;
-  private configCache: ConfigFile | null | undefined; // undefined = not read yet
+  private configCache: ConfigFile | null | undefined; // last observed identity, not a lifetime cache
   private intervalHandle: NodeJS.Timeout | null = null;
 
   constructor(opts: TelemetryOptions = {}) {
@@ -189,14 +189,17 @@ export class Telemetry {
     const machineId = config?.machine_id ?? null;
     const dnt = this.env.DO_NOT_TRACK;
     if (dnt !== undefined && dnt !== '' && dnt !== '0' && dnt.toLowerCase() !== 'false') {
+      this.clearPending();
       return { enabled: false, decidedBy: 'DO_NOT_TRACK', machineId, configPath: this.configPath };
     }
     const forced = this.env.CODEGRAPH_TELEMETRY;
     if (forced !== undefined && forced !== '') {
       const on = forced !== '0' && forced.toLowerCase() !== 'false';
+      if (!on) this.clearPending();
       return { enabled: on, decidedBy: 'CODEGRAPH_TELEMETRY', machineId, configPath: this.configPath };
     }
     if (config) {
+      if (!config.enabled) this.clearPending();
       return { enabled: config.enabled, decidedBy: 'config', machineId, configPath: this.configPath };
     }
     return { enabled: true, decidedBy: 'default', machineId, configPath: this.configPath };
@@ -215,13 +218,21 @@ export class Telemetry {
     const existing = this.readConfig();
     this.writeConfig({
       enabled,
-      machine_id: existing?.machine_id ?? randomUUID(),
+      machine_id: enabled ? (existing?.enabled && existing.machine_id ? existing.machine_id : randomUUID()) : null,
       consent_source: source,
       first_run_notice_shown: true,
       updated_at: this.now().toISOString(),
     });
     if (!enabled) {
-      try { fs.rmSync(this.queuePath, { force: true }); } catch { /* fail silent */ }
+      this.clearPending();
+      try {
+        // Claimed data must not reappear on a later opt-in/crash recovery.
+        for (const name of fs.readdirSync(this.dir)) {
+          if (name === 'telemetry-queue.jsonl' || /^telemetry-queue\.sending\.\d+\.jsonl$/.test(name)) {
+            try { fs.rmSync(path.join(this.dir, name), { force: true }); } catch { /* fail silent */ }
+          }
+        }
+      } catch { /* fail silent */ }
     }
   }
 
@@ -232,7 +243,7 @@ export class Telemetry {
 
   // -------------------------------------------------------------- recording
 
-  /** In-memory increment — safe on the MCP tool-call hot path. */
+  /** Recheck shared consent, then increment in memory; no network or writes. */
   recordUsage(kind: UsageKind, name: string, ok: boolean, client?: ClientInfo): void {
     if (!this.isEnabled()) return;
     const day = this.utcDay();
@@ -279,6 +290,7 @@ export class Telemetry {
     try {
       this.persistSync();
       this.recoverStaleClaims();
+      let identity = this.getStatus().machineId;
       const claim = this.claimQueue();
       if (!claim) return;
       const { claimPath, lines } = claim;
@@ -298,13 +310,18 @@ export class Telemetry {
         // its explicit consent toggle before any notice can fire, instead of
         // the preAction usage count pre-empting it. An explicit installer/CLI
         // choice sets first_run_notice_shown and suppresses this permanently.
+        if (!this.canUseIdentity(identity)) {
+          try { fs.rmSync(claimPath, { force: true }); } catch { /* fail silent */ }
+          return;
+        }
         this.firstRunNotice();
-        failed = await this.send(sendable, timeoutMs);
+        identity = this.getStatus().machineId;
+        failed = await this.send(sendable, timeoutMs, identity);
       }
       // Whatever didn't go out returns to the queue (append — writers may
       // have created a fresh queue file while we held the claim).
       const back = [...failed, ...keep];
-      if (back.length > 0) this.appendLines(back);
+      if (back.length > 0) this.appendLines(back, identity);
       try { fs.rmSync(claimPath, { force: true }); } catch { /* fail silent */ }
     } catch {
       /* fail silent */
@@ -335,24 +352,44 @@ export class Telemetry {
     return this.now().toISOString().slice(0, 10);
   }
 
+  private clearPending(): void {
+    this.counts.clear();
+    this.events = [];
+  }
+
+  private canUseIdentity(identity: string | null): boolean {
+    const status = this.getStatus();
+    return status.enabled && status.machineId === identity;
+  }
+
   private readConfig(): ConfigFile | null {
-    if (this.configCache !== undefined) return this.configCache;
+    // A process-lifetime cache misses another CLI's opt-out. Read the tiny file
+    // before recording, persistence and sending, including between HTTP chunks.
+    let config: ConfigFile | null = null;
     try {
       const raw = JSON.parse(fs.readFileSync(this.configPath, 'utf8')) as ConfigFile;
-      this.configCache = typeof raw.machine_id === 'string' && typeof raw.enabled === 'boolean' ? raw : null;
-    } catch {
-      this.configCache = null;
-    }
-    return this.configCache;
+      if (typeof raw.enabled === 'boolean' && (typeof raw.machine_id === 'string' ||
+        (!raw.enabled && raw.machine_id === null))) config = raw;
+    } catch { /* absent config retains the documented default */ }
+    if (this.configCache !== undefined &&
+      (this.configCache?.machine_id !== config?.machine_id ||
+        (this.configCache?.enabled && !config?.enabled))) this.clearPending();
+    this.configCache = config;
+    return config;
   }
 
   private writeConfig(config: ConfigFile): void {
+    const temp = `${this.configPath}.${process.pid}.${randomUUID()}.tmp`;
     try {
       fs.mkdirSync(this.dir, { recursive: true, mode: 0o700 });
-      fs.writeFileSync(this.configPath, JSON.stringify(config, null, 2) + '\n');
+      // Readers must see either complete choice, never a truncated JSON file.
+      fs.writeFileSync(temp, JSON.stringify(config, null, 2) + '\n', { mode: 0o600 });
+      fs.renameSync(temp, this.configPath);
       this.configCache = config;
     } catch {
       /* fail silent */
+    } finally {
+      try { fs.rmSync(temp, { force: true }); } catch { /* fail silent */ }
     }
   }
 
@@ -361,19 +398,19 @@ export class Telemetry {
    * installs record their choice explicitly and never reach this).
    */
   private firstRunNotice(): void {
+    if (!this.isEnabled()) return;
     const config = this.readConfig();
+    if (config?.first_run_notice_shown && config.machine_id) return;
+    this.writeConfig({
+      enabled: config?.enabled ?? true,
+      machine_id: config?.machine_id ?? randomUUID(),
+      consent_source: config?.consent_source ?? 'default-notice',
+      first_run_notice_shown: true,
+      updated_at: this.now().toISOString(),
+    });
+    // An explicit env-on override can mint a new identity while the stored
+    // choice stays off for every other process. It does not need a new notice.
     if (config?.first_run_notice_shown) return;
-    if (!config) {
-      this.writeConfig({
-        enabled: true,
-        machine_id: randomUUID(),
-        consent_source: 'default-notice',
-        first_run_notice_shown: true,
-        updated_at: this.now().toISOString(),
-      });
-    } else {
-      this.writeConfig({ ...config, first_run_notice_shown: true, updated_at: this.now().toISOString() });
-    }
     this.writeStderr(
       `codegraph collects anonymous usage stats (no code, paths, or names) — ` +
       `"codegraph telemetry off" or CODEGRAPH_TELEMETRY=0 disables. Details: ${TELEMETRY_DOCS}\n`,
@@ -385,17 +422,16 @@ export class Telemetry {
    * Runs on `process.on('exit')`, so it must never be async or slow.
    */
   persistSync(): void {
+    const status = this.getStatus();
+    if (!status.enabled) return;
     if (this.counts.size === 0 && this.events.length === 0) return;
     const lines: BufferLine[] = [...this.counts.values(), ...this.events];
-    this.counts.clear();
-    this.events = [];
-    // Re-check at persist time: `codegraph telemetry off` mid-process must not
-    // have its own invocation resurrect the queue file at exit.
-    if (!this.isEnabled()) return;
-    this.appendLines(lines);
+    this.clearPending();
+    this.appendLines(lines, status.machineId);
   }
 
-  private appendLines(lines: BufferLine[]): void {
+  private appendLines(lines: BufferLine[], identity: string | null): void {
+    if (!this.canUseIdentity(identity)) return;
     try {
       fs.mkdirSync(this.dir, { recursive: true, mode: 0o700 });
       const payload = lines.map((l) => JSON.stringify(l)).join('\n') + '\n';
@@ -445,6 +481,8 @@ export class Telemetry {
 
   private recoverStaleClaims(): void {
     try {
+      const identity = this.getStatus().machineId;
+      if (!this.canUseIdentity(identity)) return;
       const cutoff = this.now().getTime() - STALE_CLAIM_MS;
       for (const name of fs.readdirSync(this.dir)) {
         if (!name.startsWith('telemetry-queue.sending.')) continue;
@@ -453,7 +491,7 @@ export class Telemetry {
           if (fs.statSync(full).mtimeMs < cutoff) {
             const content = fs.readFileSync(full, 'utf8');
             fs.rmSync(full, { force: true });
-            if (content.trim()) fs.appendFileSync(this.queuePath, content.endsWith('\n') ? content : content + '\n');
+            if (content.trim() && this.canUseIdentity(identity)) fs.appendFileSync(this.queuePath, content.endsWith('\n') ? content : content + '\n');
           }
         } catch {
           /* fail silent */
@@ -465,9 +503,8 @@ export class Telemetry {
   }
 
   /** Returns the lines that did NOT make it out (to be re-queued). */
-  private async send(lines: BufferLine[], timeoutMs: number): Promise<BufferLine[]> {
-    const config = this.readConfig();
-    if (!config) return [];
+  private async send(lines: BufferLine[], timeoutMs: number, identity: string | null): Promise<BufferLine[]> {
+    if (!identity || !this.canUseIdentity(identity)) return [];
     const events = lines.map((line) =>
       'ev' in line
         ? { event: line.ev, ts: line.ts, props: line.props }
@@ -485,7 +522,7 @@ export class Telemetry {
           },
     );
     const envelope = {
-      machine_id: config.machine_id,
+      machine_id: identity,
       codegraph_version: this.packageVersion(),
       os: process.platform,
       arch: process.arch,
@@ -495,6 +532,7 @@ export class Telemetry {
     };
     const endpoint = this.env.CODEGRAPH_TELEMETRY_ENDPOINT || TELEMETRY_ENDPOINT;
     for (let i = 0; i < events.length; i += MAX_EVENTS_PER_REQUEST) {
+      if (!this.canUseIdentity(identity)) return [];
       const chunk = events.slice(i, i + MAX_EVENTS_PER_REQUEST);
       const body = JSON.stringify({ ...envelope, events: chunk });
       this.debug(`POST ${endpoint} (${chunk.length} events)`);