diff --git a/.changeset/telemetry-without-posthog-node.md b/.changeset/telemetry-without-posthog-node.md new file mode 100644 index 0000000000..b27975a767 --- /dev/null +++ b/.changeset/telemetry-without-posthog-node.md @@ -0,0 +1,5 @@ +--- +"@fission-ai/openspec": patch +--- + +Telemetry no longer depends on `posthog-node`: the single usage event is sent with a plain fetch to the same endpoint. Installing OpenSpec no longer pulls the fast-publishing `posthog-node`/`@posthog/core`/`@posthog/types` tree, which broke downstream installs under supply-chain age policies like pnpm's `minimumReleaseAge` (#1390). diff --git a/flake.nix b/flake.nix index dc02ee814a..35fa2803e4 100644 --- a/flake.nix +++ b/flake.nix @@ -51,7 +51,7 @@ inherit (finalAttrs) pname version src; pnpm = pkgs.pnpm_9; fetcherVersion = 3; - hash = "sha256-z9NIWAY1KODgALBML1bBFpM2K9N7Z4L9jFBJC/t+Mww="; + hash = "sha256-AHPKWjhrk4aTJvp9uqTJk15vASEZyRUoSw0W9oV2650="; }; nativeBuildInputs = with pkgs; [ diff --git a/package.json b/package.json index fc89329e13..7e19dec560 100644 --- a/package.json +++ b/package.json @@ -81,7 +81,6 @@ "cross-spawn": "7.0.6", "fast-glob": "^3.3.3", "ora": "^9.4.1", - "posthog-node": "^5.46.0", "yaml": "^2.8.3", "zod": "^4.4.3" }, diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 83f71fa159..49f86e2c3f 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -32,9 +32,6 @@ importers: ora: specifier: ^9.4.1 version: 9.4.1 - posthog-node: - specifier: ^5.46.0 - version: 5.46.1 yaml: specifier: ^2.8.3 version: 2.9.0 @@ -503,12 +500,6 @@ packages: '@polka/url@1.0.0-next.29': resolution: {integrity: sha512-wwQAWhWSuHaag8c4q/KN/vCoeOJYshAIvMQwD4GpSb3OiZklFfvAgmj0VCBBImRpuF/aFgIRzllXlVX93Jevww==} - '@posthog/core@1.45.1': - resolution: {integrity: sha512-tLtvzomavb2PPWdGYKsusyIzIeL2Px47v348Smibkay7sMy/83TyPk+Ptsp2NdeOgJsbuwSxWkR2+XA0aSCAaA==} - - '@posthog/types@1.398.0': - resolution: {integrity: sha512-sJMkl4k+u8yS/0fjHsKqE9xTdsAh30a2WvgChiptellnVoE0e8QJKFgqOMD2sk8FaEArPdeFklAhXvmENAt3Sg==} - '@rollup/rollup-android-arm-eabi@4.62.2': resolution: {integrity: sha512-6o7ZLZK+BeenkZCFNDXqpbjw9bD6nuWonvS/lwQJp7NoVVxm6p3qE7qQ5jGuBjiFsgvqjD8mZAU5oWxTmbOeOg==} cpu: [arm] @@ -1297,15 +1288,6 @@ packages: resolution: {integrity: sha512-KBDEIpLrvpv16pp3K0Fw+UCoZfopFjjgeB+0tA/aaThfEE74kKDLrgg603YvOWJyg3+WYtyq3xYsQWsIyZlPqQ==} engines: {node: ^10 || ^12 || >=14} - posthog-node@5.46.1: - resolution: {integrity: sha512-WjCqExq44pBdyg9MSsH6UAE0tNZ88p4aIuVFicgqhjf2Fbws6IhS4ioYUa4aBrbUPS9EDRXtBTtF5DpP1ml8Pw==} - engines: {node: ^20.20.0 || >=22.22.0} - peerDependencies: - rxjs: ^7.0.0 - peerDependenciesMeta: - rxjs: - optional: true - prelude-ls@1.2.1: resolution: {integrity: sha512-vkcDPrRZo1QZLbn5RLGPpg/WmIQ65qoWWhcGKf/b5eplkkarX0m9z8ppCat4mlOqUsWpyNuYgO3VRyrYHSzX5g==} engines: {node: '>= 0.8.0'} @@ -2065,12 +2047,6 @@ snapshots: '@polka/url@1.0.0-next.29': {} - '@posthog/core@1.45.1': - dependencies: - '@posthog/types': 1.398.0 - - '@posthog/types@1.398.0': {} - '@rollup/rollup-android-arm-eabi@4.62.2': optional: true @@ -2814,10 +2790,6 @@ snapshots: picocolors: 1.1.1 source-map-js: 1.2.1 - posthog-node@5.46.1: - dependencies: - '@posthog/core': 1.45.1 - prelude-ls@1.2.1: {} prettier@2.8.8: {} diff --git a/src/telemetry/index.ts b/src/telemetry/index.ts index e7a92dcdc4..d496dee8c6 100644 --- a/src/telemetry/index.ts +++ b/src/telemetry/index.ts @@ -7,8 +7,17 @@ * - Opt-out via OPENSPEC_TELEMETRY=0 or DO_NOT_TRACK=1 * - Auto-disabled in CI environments * - Anonymous ID is a random UUID with no relation to the user + * + * Events are sent with a plain fetch to PostHog's stable public `/batch/` + * endpoint — the same one posthog-node used — instead of through the SDK. + * The SDK's only remaining job here was the wire format: every reliability + * knob was already forced to "send one event immediately, time-bounded, + * never retry, never throw". Carrying `posthog-node` for that shipped its + * fast-moving transitive tree (`@posthog/core`, `@posthog/types`, multiple + * releases per day) to every downstream consumer, where supply-chain age + * policies such as pnpm's `minimumReleaseAge` rejected the freshly published + * versions and broke installs (#1390). */ -import { PostHog } from 'posthog-node'; import { randomUUID } from 'crypto'; import { getTelemetryConfig, updateTelemetryConfig } from './config.js'; @@ -19,12 +28,25 @@ const POSTHOG_API_KEY = 'phc_Hthu8YvaIJ9QaFKyTG4TbVwkbd5ktcAFzVTKeMmoW2g'; const POSTHOG_HOST = 'https://edge.openspec.dev'; const TELEMETRY_REQUEST_TIMEOUT_MS = 1000; -let posthogClient: PostHog | null = null; let anonymousId: string | null = null; +/** + * Requests started by trackCommand and not yet settled, so shutdown can + * flush them before the process exits. Each request is individually + * time-bounded, so awaiting them cannot stall exit for more than the + * request timeout. + */ +const pendingEvents = new Set>(); + async function safeTelemetryFetch(url: string, options: RequestInit): Promise { try { const response = await fetch(url, options); + // Telemetry never reads the body, but undici keeps the connection + // occupied until the body is consumed or canceled — dispose of it on + // every path so no socket outlives shutdown(). + if (response.body) { + await response.body.cancel(); + } if (response.ok) { return response; } @@ -86,24 +108,32 @@ export async function getOrCreateAnonymousId(): Promise { } /** - * Get the PostHog client instance. - * Creates it on first call with CLI-optimized settings. + * Send one capture event to PostHog's batch endpoint. Fire-and-forget: + * bounded by the request timeout, never throws, never retries. */ -function getClient(): PostHog { - if (!posthogClient) { - posthogClient = new PostHog(POSTHOG_API_KEY, { - host: POSTHOG_HOST, - flushAt: 1, // Send immediately, don't batch - flushInterval: 0, // No timer-based flushing - fetchRetryCount: 0, - requestTimeout: TELEMETRY_REQUEST_TIMEOUT_MS, - preloadFeatureFlags: false, - disableRemoteConfig: true, - disableSurveys: true, - fetch: safeTelemetryFetch, - }); - } - return posthogClient; +function sendEvent(distinctId: string, event: string, properties: Record): void { + const body = JSON.stringify({ + api_key: POSTHOG_API_KEY, + batch: [ + { + type: 'capture', + event, + distinct_id: distinctId, + properties, + timestamp: new Date().toISOString(), + }, + ], + }); + + const request = safeTelemetryFetch(`${POSTHOG_HOST}/batch/`, { + method: 'POST', + headers: { 'content-type': 'application/json' }, + body, + signal: AbortSignal.timeout(TELEMETRY_REQUEST_TIMEOUT_MS), + }).then(() => undefined); + + pendingEvents.add(request); + void request.finally(() => pendingEvents.delete(request)); } /** @@ -119,17 +149,12 @@ export async function trackCommand(commandName: string, version: string): Promis try { const userId = await getOrCreateAnonymousId(); - const client = getClient(); - - client.capture({ - distinctId: userId, - event: 'command_executed', - properties: { - command: commandName, - version: version, - surface: 'cli', - $ip: null, // Explicitly disable IP tracking - }, + + sendEvent(userId, 'command_executed', { + command: commandName, + version: version, + surface: 'cli', + $ip: null, // Explicitly disable IP tracking }); } catch { // Silent failure - telemetry should never break CLI @@ -163,19 +188,19 @@ export async function maybeShowTelemetryNotice(): Promise { } /** - * Shutdown the PostHog client and flush pending events. + * Flush pending telemetry events. * Call this before CLI exit. */ export async function shutdown(): Promise { - if (!posthogClient) { + if (pendingEvents.size === 0) { return; } try { - await posthogClient.shutdown(); + await Promise.allSettled([...pendingEvents]); } catch { // Silent failure - telemetry should never break CLI exit } finally { - posthogClient = null; + pendingEvents.clear(); } } diff --git a/test/telemetry/index.test.ts b/test/telemetry/index.test.ts index e4c6da6c21..6ff9b13c86 100644 --- a/test/telemetry/index.test.ts +++ b/test/telemetry/index.test.ts @@ -3,19 +3,7 @@ import * as fs from 'node:fs'; import * as path from 'node:path'; import * as os from 'node:os'; -// Mock posthog-node before importing the module -vi.mock('posthog-node', () => { - return { - PostHog: vi.fn().mockImplementation(() => ({ - capture: vi.fn(), - shutdown: vi.fn().mockResolvedValue(undefined), - })), - }; -}); - -// Import after mocking import { isTelemetryEnabled, maybeShowTelemetryNotice, shutdown, trackCommand } from '../../src/telemetry/index.js'; -import { PostHog } from 'posthog-node'; describe('telemetry/index', () => { let tempDir: string; @@ -38,7 +26,10 @@ describe('telemetry/index', () => { // Spy on console.log for notice tests consoleLogSpy = vi.spyOn(console, 'log').mockImplementation(() => {}); - fetchSpy = vi.spyOn(globalThis, 'fetch'); + // Telemetry must never reach the real network in tests + fetchSpy = vi + .spyOn(globalThis, 'fetch') + .mockResolvedValue(new Response(null, { status: 200 })); }); afterEach(async () => { @@ -58,6 +49,12 @@ describe('telemetry/index', () => { vi.restoreAllMocks(); }); + function enableTelemetry() { + delete process.env.OPENSPEC_TELEMETRY; + delete process.env.DO_NOT_TRACK; + delete process.env.CI; + } + describe('isTelemetryEnabled', () => { it('should return false when OPENSPEC_TELEMETRY=0', () => { process.env.OPENSPEC_TELEMETRY = '0'; @@ -75,9 +72,7 @@ describe('telemetry/index', () => { }); it('should return true when no opt-out is set', () => { - delete process.env.OPENSPEC_TELEMETRY; - delete process.env.DO_NOT_TRACK; - delete process.env.CI; + enableTelemetry(); expect(isTelemetryEnabled()).toBe(true); }); @@ -100,118 +95,174 @@ describe('telemetry/index', () => { }); describe('trackCommand', () => { - it('should not track when telemetry is disabled', async () => { + it('should send nothing when telemetry is disabled', async () => { process.env.OPENSPEC_TELEMETRY = '0'; await trackCommand('test', '1.0.0'); + await shutdown(); - expect(PostHog).not.toHaveBeenCalled(); + expect(fetchSpy).not.toHaveBeenCalled(); }); - it('should track when telemetry is enabled', async () => { - delete process.env.OPENSPEC_TELEMETRY; - delete process.env.DO_NOT_TRACK; - delete process.env.CI; + it('should post one capture event to the batch endpoint when enabled', async () => { + enableTelemetry(); await trackCommand('test', '1.0.0'); - - expect(PostHog).toHaveBeenCalled(); + await shutdown(); + + expect(fetchSpy).toHaveBeenCalledTimes(1); + const [url, options] = fetchSpy.mock.calls[0] as [string, RequestInit]; + expect(url).toBe('https://edge.openspec.dev/batch/'); + expect(options.method).toBe('POST'); + + const payload = JSON.parse(String(options.body)); + expect(payload.api_key).toEqual(expect.any(String)); + expect(payload.batch).toHaveLength(1); + const event = payload.batch[0]; + expect(event.type).toBe('capture'); + expect(event.event).toBe('command_executed'); + expect(event.distinct_id).toMatch(/^[0-9a-f-]{36}$/); + expect(event.timestamp).toEqual(expect.any(String)); + expect(event.properties).toEqual({ + command: 'test', + version: '1.0.0', + surface: 'cli', + $ip: null, + }); }); - it('should construct PostHog with bounded silent-failure settings', async () => { - delete process.env.OPENSPEC_TELEMETRY; - delete process.env.DO_NOT_TRACK; - delete process.env.CI; + it('should bound the request with a timeout signal', async () => { + enableTelemetry(); await trackCommand('test', '1.0.0'); + await shutdown(); - expect(PostHog).toHaveBeenCalledWith( - expect.any(String), - expect.objectContaining({ - host: 'https://edge.openspec.dev', - flushAt: 1, - flushInterval: 0, - fetchRetryCount: 0, - requestTimeout: 1000, - preloadFeatureFlags: false, - disableRemoteConfig: true, - disableSurveys: true, - fetch: expect.any(Function), - }) - ); + const [, options] = fetchSpy.mock.calls[0] as [string, RequestInit]; + expect(options.signal).toBeInstanceOf(AbortSignal); }); - it('should return a synthetic success response when fetch throws a network error', async () => { - delete process.env.OPENSPEC_TELEMETRY; - delete process.env.DO_NOT_TRACK; - delete process.env.CI; - await trackCommand('test', '1.0.0'); - - const fetchFn = (PostHog as any).mock.calls[0][1].fetch as typeof fetch; + it('should swallow a network error silently', async () => { + enableTelemetry(); fetchSpy.mockRejectedValueOnce(new Error('network down')); - const response = await fetchFn('https://edge.openspec.dev/batch/', { method: 'POST' }); + await trackCommand('test', '1.0.0'); + await expect(shutdown()).resolves.not.toThrow(); + }); + + it('should swallow an abort silently', async () => { + enableTelemetry(); + fetchSpy.mockRejectedValueOnce(new DOMException('This operation was aborted', 'AbortError')); - expect(response.status).toBe(204); + await trackCommand('test', '1.0.0'); + await expect(shutdown()).resolves.not.toThrow(); }); - it('should return a synthetic success response when fetch aborts', async () => { - delete process.env.OPENSPEC_TELEMETRY; - delete process.env.DO_NOT_TRACK; - delete process.env.CI; + it('should swallow a non-2xx response silently', async () => { + enableTelemetry(); + fetchSpy.mockResolvedValueOnce(new Response('forbidden', { status: 403 })); + await trackCommand('test', '1.0.0'); + await expect(shutdown()).resolves.not.toThrow(); + }); - const fetchFn = (PostHog as any).mock.calls[0][1].fetch as typeof fetch; - fetchSpy.mockRejectedValueOnce(new DOMException('This operation was aborted', 'AbortError')); + it('should dispose the response body of a successful response before the event settles', async () => { + // Undici holds the connection until the body is consumed or canceled; + // an undisposed body would let the socket outlive shutdown(). + enableTelemetry(); + const response = new Response('{"status": 1}', { status: 200 }); + fetchSpy.mockResolvedValueOnce(response); - const response = await fetchFn('https://edge.openspec.dev/batch/', { method: 'POST' }); + await trackCommand('test', '1.0.0'); + await shutdown(); - expect(response.status).toBe(204); + expect(response.bodyUsed).toBe(true); }); - it('should return a synthetic success response for non-2xx responses', async () => { - delete process.env.OPENSPEC_TELEMETRY; - delete process.env.DO_NOT_TRACK; - delete process.env.CI; - await trackCommand('test', '1.0.0'); + it('should dispose the response body of a non-2xx response before the event settles', async () => { + enableTelemetry(); + const response = new Response('rate limited', { status: 429 }); + fetchSpy.mockResolvedValueOnce(response); - const fetchFn = (PostHog as any).mock.calls[0][1].fetch as typeof fetch; - fetchSpy.mockResolvedValueOnce(new Response('forbidden', { status: 403 })); + await trackCommand('test', '1.0.0'); + await shutdown(); - const response = await fetchFn('https://edge.openspec.dev/batch/', { method: 'POST' }); + expect(response.bodyUsed).toBe(true); + }); + }); - expect(response.status).toBe(204); + describe('shutdown', () => { + it('should not throw when nothing is pending', async () => { + await expect(shutdown()).resolves.not.toThrow(); }); - it('should pass through successful responses from fetch', async () => { - delete process.env.OPENSPEC_TELEMETRY; - delete process.env.DO_NOT_TRACK; - delete process.env.CI; + it('should flush an in-flight event before returning', async () => { + enableTelemetry(); + + let settle!: (response: Response) => void; + fetchSpy.mockImplementationOnce( + () => new Promise((resolve) => (settle = resolve)) + ); + await trackCommand('test', '1.0.0'); - const fetchFn = (PostHog as any).mock.calls[0][1].fetch as typeof fetch; - const expectedResponse = new Response(null, { status: 200 }); - fetchSpy.mockResolvedValueOnce(expectedResponse); + let flushed = false; + const flushing = shutdown().then(() => { + flushed = true; + }); - const response = await fetchFn('https://edge.openspec.dev/batch/', { method: 'POST' }); + // The event is still in flight, so shutdown must still be waiting. + await Promise.resolve(); + expect(flushed).toBe(false); - expect(response).toBe(expectedResponse); + settle(new Response(null, { status: 200 })); + await flushing; + expect(flushed).toBe(true); }); }); - describe('shutdown', () => { - it('should not throw when no client exists', async () => { - await expect(shutdown()).resolves.not.toThrow(); - }); + describe('published dependency tree (#1390)', () => { + it('ships no posthog packages to consumers', () => { + // Downstream supply-chain age policies (pnpm minimumReleaseAge) broke + // installs whenever the posthog subtree had a release younger than the + // policy window — which, at posthog's publish cadence, was most days. + // Telemetry now speaks the wire format directly; nothing in the + // published manifest may reintroduce that tree. + const manifest = JSON.parse( + fs.readFileSync(path.join(process.cwd(), 'package.json'), 'utf-8') + ) as { + dependencies?: Record; + optionalDependencies?: Record; + peerDependencies?: Record; + }; - it('should handle shutdown errors silently', async () => { - const mockPostHog = { - capture: vi.fn(), - shutdown: vi.fn().mockRejectedValue(new Error('Network error')), + const shipped = { + ...manifest.dependencies, + ...manifest.optionalDependencies, + ...manifest.peerDependencies, }; - (PostHog as any).mockImplementation(() => mockPostHog); + const posthogDeps = Object.keys(shipped).filter((name) => + name.toLowerCase().includes('posthog') + ); + expect(posthogDeps).toEqual([]); + }); - await expect(shutdown()).resolves.not.toThrow(); + it('imports no posthog module anywhere in src', () => { + const hits: string[] = []; + const walk = (dir: string): void => { + for (const entry of fs.readdirSync(dir, { withFileTypes: true })) { + const full = path.join(dir, entry.name); + if (entry.isDirectory()) { + walk(full); + } else if (entry.name.endsWith('.ts')) { + const content = fs.readFileSync(full, 'utf-8'); + if (/from\s+['"](posthog|@posthog)/.test(content)) { + hits.push(full); + } + } + } + }; + walk(path.join(process.cwd(), 'src')); + expect(hits).toEqual([]); }); }); });