From 07cda2529312f1c0ae6132b79a52cf8e453be9e5 Mon Sep 17 00:00:00 2001 From: Clay Good Date: Tue, 28 Jul 2026 19:40:12 -0500 Subject: [PATCH 1/3] fix(telemetry): send the usage event directly instead of via posthog-node MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Installing OpenSpec shipped posthog-node's transitive tree (@posthog/core, @posthog/types) to every consumer. Those packages release several times a day, so any freshly resolved install tripped supply-chain age policies — pnpm's minimumReleaseAge failed with ERR_PNPM_MINIMUM_RELEASE_AGE_VIOLATION on entries younger than the policy window (#1390). No pinning fixes this: exact-pinning posthog-node leaves its own ranges floating, npm overrides only apply at a consumer's root, pnpm ignores a dependency's npm-shrinkwrap, and bundledDependencies under a pnpm-managed node_modules packs the virtual-store layout and breaks module resolution (verified: the bundled CLI crashes on import). The SDK's only remaining job here was the wire format: the client was already configured to send one event immediately, time-bounded, with no retries, through an injected fetch that never throws. Post the same capture payload to the same /batch/ endpoint with that fetch directly. Same event name, properties, distinct id, and opt-out guards; shutdown still flushes in-flight events, each bounded by the request timeout. Verified end to end: the packed tarball contains zero posthog files, a pnpm consumer with minimumReleaseAge: 1440 installs cleanly with zero posthog lockfile entries, and the live endpoint answers 200 OK to the new payload. Regression tests pin the manifest and src free of posthog. Fixes #1390 Co-Authored-By: Claude Opus 5 (1M context) --- .changeset/telemetry-without-posthog-node.md | 5 + package.json | 1 - pnpm-lock.yaml | 28 --- src/telemetry/index.ts | 87 +++++--- test/telemetry/index.test.ts | 207 +++++++++++-------- 5 files changed, 175 insertions(+), 153 deletions(-) create mode 100644 .changeset/telemetry-without-posthog-node.md 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/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..217286f7a9 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,9 +28,16 @@ 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); @@ -86,24 +102,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 +143,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 +182,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..244963ea1d 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,150 @@ 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' }); - - expect(response.status).toBe(204); - }); - - 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; 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; + it('should swallow an abort silently', async () => { + enableTelemetry(); fetchSpy.mockRejectedValueOnce(new DOMException('This operation was aborted', 'AbortError')); - const response = await fetchFn('https://edge.openspec.dev/batch/', { method: 'POST' }); - - expect(response.status).toBe(204); - }); - - 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'); + await expect(shutdown()).resolves.not.toThrow(); + }); - const fetchFn = (PostHog as any).mock.calls[0][1].fetch as typeof fetch; + it('should swallow a non-2xx response silently', async () => { + enableTelemetry(); fetchSpy.mockResolvedValueOnce(new Response('forbidden', { status: 403 })); - const response = await fetchFn('https://edge.openspec.dev/batch/', { method: 'POST' }); + await trackCommand('test', '1.0.0'); + await expect(shutdown()).resolves.not.toThrow(); + }); + }); - 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([]); }); }); }); From 4a1c6fd8b3b2c1a957630e1f7ad73d9ffa4b999a Mon Sep 17 00:00:00 2001 From: Clay Good Date: Tue, 28 Jul 2026 19:43:33 -0500 Subject: [PATCH 2/3] build(nix): update the pnpm deps hash for the posthog-node removal Value taken from the CI mismatch report. Co-Authored-By: Claude Opus 5 (1M context) --- flake.nix | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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; [ From c1b83b3b0a10c0af3cfec9d7a5837d92ee955c99 Mon Sep 17 00:00:00 2001 From: Clay Good Date: Tue, 28 Jul 2026 19:57:03 -0500 Subject: [PATCH 3/3] fix(telemetry): dispose the response body so no socket outlives shutdown MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit undici keeps the connection occupied until the response body is consumed or canceled, and telemetry never reads it — on both the success and non-2xx paths the socket could linger after shutdown() returned. Cancel the body before the tracked promise resolves, with coverage for both paths (bodyUsed asserted after shutdown), and the live endpoint re-verified with disposal in place. Co-Authored-By: Claude Opus 5 (1M context) --- src/telemetry/index.ts | 6 ++++++ test/telemetry/index.test.ts | 24 ++++++++++++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/src/telemetry/index.ts b/src/telemetry/index.ts index 217286f7a9..d496dee8c6 100644 --- a/src/telemetry/index.ts +++ b/src/telemetry/index.ts @@ -41,6 +41,12 @@ 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; } diff --git a/test/telemetry/index.test.ts b/test/telemetry/index.test.ts index 244963ea1d..6ff9b13c86 100644 --- a/test/telemetry/index.test.ts +++ b/test/telemetry/index.test.ts @@ -164,6 +164,30 @@ describe('telemetry/index', () => { await trackCommand('test', '1.0.0'); await expect(shutdown()).resolves.not.toThrow(); }); + + 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); + + await trackCommand('test', '1.0.0'); + await shutdown(); + + expect(response.bodyUsed).toBe(true); + }); + + 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); + + await trackCommand('test', '1.0.0'); + await shutdown(); + + expect(response.bodyUsed).toBe(true); + }); }); describe('shutdown', () => {