diff --git a/.changeset/suppress-telemetry-notice-json.md b/.changeset/suppress-telemetry-notice-json.md new file mode 100644 index 0000000000..02760341b1 --- /dev/null +++ b/.changeset/suppress-telemetry-notice-json.md @@ -0,0 +1,8 @@ +--- +"@fission-ai/openspec": patch +--- + +Suppress the first-run telemetry disclosure notice when `--json` is used. On a +first-ever run the notice was written to stdout and could break `--json` +consumers; it is now deferred to the first later non-JSON run, keeping `--json` +output valid while still guaranteeing the disclosure. diff --git a/openspec/changes/suppress-telemetry-notice-in-json/.openspec.yaml b/openspec/changes/suppress-telemetry-notice-in-json/.openspec.yaml new file mode 100644 index 0000000000..878dc3156e --- /dev/null +++ b/openspec/changes/suppress-telemetry-notice-in-json/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-08-07 diff --git a/openspec/changes/suppress-telemetry-notice-in-json/proposal.md b/openspec/changes/suppress-telemetry-notice-in-json/proposal.md new file mode 100644 index 0000000000..579026b05c --- /dev/null +++ b/openspec/changes/suppress-telemetry-notice-in-json/proposal.md @@ -0,0 +1,44 @@ +# Suppress the first-run telemetry notice in --json mode + +## Why + +`openspec --json` is meant to emit exactly one machine-readable JSON +document on stdout so agents and automation can parse it. Spinner suppression +and structured JSON errors already ship on main, but one stdout writer remains: +the first-run telemetry disclosure notice. + +On a user's first-ever command, `maybeShowTelemetryNotice()` runs from the +global `preAction` hook and `console.log`s the disclosure to **stdout** — before +the command's JSON payload. A `--json` consumer parsing that first run gets +invalid JSON. It is first-run-only (the notice sets `noticeSeen`), but that is +exactly the run an automation is most likely to hit on a fresh machine or CI +image. + +## What Changes + +- `maybeShowTelemetryNotice()` accepts a `silent` option. When silent, it prints + nothing **and** leaves `noticeSeen` unset, so the disclosure is deferred rather + than skipped. +- The `preAction` hook passes `silent: true` when the executing command asked + for JSON, decided by `isJsonRun(command)`. `--json` reaches commands three + ways, so a single parsed option (`opts().json`) is not enough: on the leaf + (`status --json`), on a parent group read via `optsWithGlobals` + (`workset --json list`), and as a residual arg on permissive groups that never + declare the option (`openspec store --json`). `isJsonRun` checks + `optsWithGlobals().json` and `command.args`, covering all three. + +Net effect: any `--json` invocation never emits the notice on stdout; the user +still sees the disclosure on their first later non-JSON run. Suppressing is +always safe — worst case the disclosure defers one run. Telemetry remains opt-out +and otherwise unchanged. + +Out of scope: a few commands write scriptable output to stdout without a `--json` +flag (`completion generate`, `config get`, `config path`, the hidden `__complete`). +Their first-run notice pollution is a separate, pre-existing issue not addressed +here. + +## Impact + +- Affected specs: `telemetry` (MODIFIED: First-run telemetry notice) +- Affected code: `src/telemetry/index.ts`, `src/cli/index.ts` +- No change to non-JSON behavior; no new events or data collected. diff --git a/openspec/changes/suppress-telemetry-notice-in-json/specs/telemetry/spec.md b/openspec/changes/suppress-telemetry-notice-in-json/specs/telemetry/spec.md new file mode 100644 index 0000000000..8e1316eee2 --- /dev/null +++ b/openspec/changes/suppress-telemetry-notice-in-json/specs/telemetry/spec.md @@ -0,0 +1,28 @@ +## MODIFIED Requirements + +### Requirement: First-run telemetry notice +The system SHALL display a one-line telemetry disclosure notice on the first command execution, before any telemetry is sent. In `--json` mode the system SHALL NOT display the notice on that run and SHALL leave `noticeSeen` unset, deferring the disclosure to the first later non-JSON run. + +#### Scenario: First command execution +- **WHEN** a user runs their first openspec command without `--json` +- **AND** telemetry is enabled +- **THEN** the system displays: "Note: OpenSpec collects anonymous usage stats. Opt out: OPENSPEC_TELEMETRY=0" + +#### Scenario: Subsequent command execution +- **WHEN** a user has already seen the notice (noticeSeen: true in config) +- **THEN** the system does not display the notice + +#### Scenario: Notice before telemetry +- **WHEN** displaying the first-run notice +- **THEN** the notice appears before any telemetry event is sent + +#### Scenario: First command execution in JSON mode +- **WHEN** a user's first openspec command passes `--json` +- **AND** telemetry is enabled +- **THEN** the system displays no notice on stdout +- **AND** `noticeSeen` remains unset + +#### Scenario: Disclosure deferred, not skipped +- **WHEN** a user's first run was in `--json` mode and displayed no notice +- **AND** the user later runs a command without `--json` +- **THEN** the system displays the disclosure notice on that later run diff --git a/openspec/changes/suppress-telemetry-notice-in-json/tasks.md b/openspec/changes/suppress-telemetry-notice-in-json/tasks.md new file mode 100644 index 0000000000..b1af654c0d --- /dev/null +++ b/openspec/changes/suppress-telemetry-notice-in-json/tasks.md @@ -0,0 +1,9 @@ +# Tasks + +## 1. Suppress notice in JSON mode +- [x] 1.1 Add a `silent` option to `maybeShowTelemetryNotice()` that skips the notice and leaves `noticeSeen` unset +- [x] 1.2 Read `actionCommand.opts().json` in the `preAction` hook and pass `silent` accordingly + +## 2. Tests +- [x] 2.1 Assert a first-run `--json` (silent) call prints nothing and does not mark the notice seen +- [x] 2.2 Assert the disclosure still appears on the first later non-silent run diff --git a/src/cli/index.ts b/src/cli/index.ts index 6772643c68..f932f53dba 100644 --- a/src/cli/index.ts +++ b/src/cli/index.ts @@ -115,6 +115,27 @@ export function getCommandPath(command: Command): string { return names.join(':') || 'openspec'; } +/** + * True when the executing command asked for JSON output — used to suppress the + * first-run telemetry notice so stdout stays a single valid JSON document. + * + * `--json` reaches commands three ways, so a single parsed option is not enough: + * - declared on the leaf (`openspec status --json`) → `opts().json` + * - declared on a parent group and read via globals (`openspec workset --json list`) + * → `optsWithGlobals().json` + * - a residual arg on a permissive group that never declares the option + * (`openspec store --json`, which detects it from `command.args`) → `args` + * + * Suppressing is always safe: the disclosure is only deferred to the next + * non-JSON run, never lost, whereas printing it on a JSON run corrupts stdout. + */ +export function isJsonRun(command: Command): boolean { + return ( + command.optsWithGlobals().json === true || + command.args.includes('--json') + ); +} + program .name('openspec') .description('AI-native system for spec-driven development') @@ -133,8 +154,9 @@ program.hook('preAction', async (thisCommand, actionCommand) => { process.env.NO_COLOR = '1'; } - // Show first-run telemetry notice (if not seen) - await maybeShowTelemetryNotice(); + // Show first-run telemetry notice (if not seen). Suppress it whenever the run + // asked for JSON so stdout stays a single valid JSON document (see isJsonRun). + await maybeShowTelemetryNotice({ silent: isJsonRun(actionCommand) }); // Track command execution (use actionCommand to get the actual subcommand) const commandPath = getCommandPath(actionCommand); diff --git a/src/telemetry/index.ts b/src/telemetry/index.ts index fce6a67534..3eb1978e3f 100644 --- a/src/telemetry/index.ts +++ b/src/telemetry/index.ts @@ -177,7 +177,9 @@ export async function trackCommand(commandName: string, version: string): Promis /** * Show first-run telemetry notice if not already seen. */ -export async function maybeShowTelemetryNotice(): Promise { +export async function maybeShowTelemetryNotice( + options: { silent?: boolean } = {} +): Promise { if (!isTelemetryEnabled()) { return; } @@ -188,6 +190,13 @@ export async function maybeShowTelemetryNotice(): Promise { return; } + // In --json mode the notice would pollute stdout and break parsers, so + // defer it: skip the notice AND leave noticeSeen unset so the disclosure + // still appears on the user's first later non-JSON run. + if (options.silent) { + return; + } + // Display notice console.log( 'Note: OpenSpec collects anonymous usage stats. Opt out: OPENSPEC_TELEMETRY=0 or openspec config set telemetry.enabled false' diff --git a/test/core/cli-is-json-run.test.ts b/test/core/cli-is-json-run.test.ts new file mode 100644 index 0000000000..fa4110a0be --- /dev/null +++ b/test/core/cli-is-json-run.test.ts @@ -0,0 +1,79 @@ +import { describe, it, expect } from 'vitest'; +import { Command, Option } from 'commander'; + +import { isJsonRun } from '../../src/cli/index.js'; + +/** + * Reproduce the three ways `--json` reaches a command in the real CLI, so a + * future refactor of the telemetry-notice guard can't silently reintroduce + * first-run stdout pollution for `store --json` / `workset --json `. + */ +function buildProgram(capture: (command: Command) => void): Command { + const program = new Command(); + program.name('openspec').exitOverride(); + program.configureOutput({ writeOut: () => {}, writeErr: () => {} }); + program.option('--no-color', 'Disable color output'); + program.hook('preAction', (_thisCommand, actionCommand) => { + capture(actionCommand); + }); + + // 1. Leaf declares --json (e.g. `openspec status --json`). + program + .command('status') + .option('--json', 'Output as JSON') + .action(() => {}); + + // 2. Permissive bare group that never declares --json and detects it from + // residual args (e.g. `openspec store --json`). + const store = program.command('store'); + store.allowExcessArguments(true); + store.allowUnknownOption(true); + store.action(() => {}); + + // 3. Parent group declares --json (read via optsWithGlobals) with its own + // subcommands (e.g. `openspec workset --json list`). + const workset = program.command('workset'); + workset.addOption(new Option('--json', 'Output as JSON').hideHelp()); + workset + .command('list') + .option('--json', 'Output as JSON') + .action(() => {}); + + return program; +} + +describe('isJsonRun', () => { + async function actionCommandFor(argv: string[]): Promise { + let captured: Command | undefined; + const program = buildProgram((command) => { + captured = command; + }); + await program.parseAsync(['node', 'openspec', ...argv]); + if (!captured) throw new Error(`no action command captured for: ${argv.join(' ')}`); + return captured; + } + + it('detects --json declared on the leaf command', async () => { + expect(isJsonRun(await actionCommandFor(['status', '--json']))).toBe(true); + }); + + it('detects --json as a residual arg on a permissive bare group', async () => { + expect(isJsonRun(await actionCommandFor(['store', '--json']))).toBe(true); + }); + + it('detects --json on a parent group placed before the subcommand', async () => { + expect(isJsonRun(await actionCommandFor(['workset', '--json', 'list']))).toBe(true); + }); + + it('detects --json declared on the subcommand leaf', async () => { + expect(isJsonRun(await actionCommandFor(['workset', 'list', '--json']))).toBe(true); + }); + + it('is false when no --json is present', async () => { + expect(isJsonRun(await actionCommandFor(['status']))).toBe(false); + }); + + it('is false for a bare group with unrelated residual args', async () => { + expect(isJsonRun(await actionCommandFor(['store', 'bogus']))).toBe(false); + }); +}); diff --git a/test/telemetry/index.test.ts b/test/telemetry/index.test.ts index b3b21f7aa9..3d2afe9151 100644 --- a/test/telemetry/index.test.ts +++ b/test/telemetry/index.test.ts @@ -4,6 +4,7 @@ import * as path from 'node:path'; import * as os from 'node:os'; import { isTelemetryEnabled, maybeShowTelemetryNotice, shutdown, trackCommand } from '../../src/telemetry/index.js'; +import { getTelemetryConfig } from '../../src/telemetry/config.js'; describe('telemetry/index', () => { let tempDir: string; @@ -177,6 +178,38 @@ describe('telemetry/index', () => { expect(consoleLogSpy).not.toHaveBeenCalled(); }); + + it('should show notice on the first non-silent run, then never repeat it', async () => { + enableTelemetry(); + + await maybeShowTelemetryNotice(); + expect(consoleLogSpy).toHaveBeenCalledTimes(1); + expect(consoleLogSpy).toHaveBeenCalledWith( + expect.stringContaining('OpenSpec collects anonymous usage stats') + ); + + // noticeSeen is now persisted: a second run stays quiet. + await maybeShowTelemetryNotice(); + expect(consoleLogSpy).toHaveBeenCalledTimes(1); + }); + + it('should suppress the notice in silent (--json) mode and defer the disclosure', async () => { + enableTelemetry(); + + // A first-ever run in --json mode must not pollute stdout. + await maybeShowTelemetryNotice({ silent: true }); + expect(consoleLogSpy).not.toHaveBeenCalled(); + + // The disclosure must be deferred, not consumed: noticeSeen stays unset. + expect((await getTelemetryConfig()).noticeSeen).toBeFalsy(); + + // Disclosure is only deferred, not skipped: the next non-JSON run shows it. + await maybeShowTelemetryNotice(); + expect(consoleLogSpy).toHaveBeenCalledTimes(1); + expect(consoleLogSpy).toHaveBeenCalledWith( + expect.stringContaining('OpenSpec collects anonymous usage stats') + ); + }); }); describe('trackCommand', () => {