mirror of
https://github.com/Fission-AI/OpenSpec.git
synced 2026-10-02 05:24:34 +08:00
fix(telemetry): suppress first-run notice in --json mode (#1609)
* fix(telemetry): suppress first-run notice in --json mode The first-run telemetry disclosure notice was written to stdout from the global preAction hook. On a user's first-ever command with --json this polluted stdout and could break JSON parsers. Read the executing command's --json flag (actionCommand.opts().json) and, when set, skip the notice and leave noticeSeen unset so the disclosure is deferred to the first later non-JSON run rather than lost. Spinner suppression, new-change --json output, and structured JSON errors already landed on main (#960, #1190); this closes the one remaining stdout writer in --json mode. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * harden: detect --json from argv to cover all invocation forms The preAction guard read actionCommand.opts().json, which only sees a declared leaf option. That missed two supported --json forms that emit a single JSON document to stdout: - openspec store --json (permissive group reads --json from residual args; never declares the option, so opts().json is undefined) - openspec workset --json <sub> (--json on the parent group, consumed before the leaf; leaf opts().json is undefined) Both would still print the first-run telemetry notice ahead of their JSON. Detect --json from process.argv instead: it covers leaf, parent, and residual-arg forms uniformly. Suppressing is always safe (the disclosure defers to the next non-JSON run, never lost), so a broad argv check is the correct, conservative signal. Also add a direct assertion that noticeSeen stays unset after a silent run, and note the pre-existing raw-stdout commands (completion generate, config get/path, __complete) as out of scope. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * refactor: derive --json from parsed command state + regression test Replace the process.argv check with isJsonRun(command), an exported pure helper that reads Commander's parsed state: optsWithGlobals().json (leaf and parent-group forms) OR command.args (residual --json on permissive bare groups like store). This is tied to the actually-parsed command rather than raw args, and — unlike process.argv — is unit-testable in-process. Add test/core/cli-is-json-run.test.ts: a synthetic program reproducing all three registration patterns proves isJsonRun returns true for status --json, store --json, workset --json list, and workset list --json, and false otherwise. This locks in the store/workset coverage against future regressions (an e2e test can't: telemetry is disabled under CI, so the notice never fires there). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(spec): qualify first-command notice scenario as non-JSON The generic 'First command execution' scenario asserted the notice displays on every first command, contradicting the JSON scenario that says it does not. Qualify it as 'without --json' so the required behavior is unambiguous. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
17581c11ed
commit
804427b6ff
@@ -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.
|
||||
@@ -0,0 +1,2 @@
|
||||
schema: spec-driven
|
||||
created: 2026-08-07
|
||||
@@ -0,0 +1,44 @@
|
||||
# Suppress the first-run telemetry notice in --json mode
|
||||
|
||||
## Why
|
||||
|
||||
`openspec <cmd> --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.
|
||||
@@ -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
|
||||
@@ -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
|
||||
+24
-2
@@ -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);
|
||||
|
||||
+10
-1
@@ -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<void> {
|
||||
export async function maybeShowTelemetryNotice(
|
||||
options: { silent?: boolean } = {}
|
||||
): Promise<void> {
|
||||
if (!isTelemetryEnabled()) {
|
||||
return;
|
||||
}
|
||||
@@ -188,6 +190,13 @@ export async function maybeShowTelemetryNotice(): Promise<void> {
|
||||
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'
|
||||
|
||||
@@ -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 <sub>`.
|
||||
*/
|
||||
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<Command> {
|
||||
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);
|
||||
});
|
||||
});
|
||||
@@ -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', () => {
|
||||
|
||||
Reference in New Issue
Block a user