Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions .changeset/suppress-telemetry-notice-json.md
Original file line number Diff line number Diff line change
@@ -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.
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
schema: spec-driven
created: 2026-08-07
44 changes: 44 additions & 0 deletions openspec/changes/suppress-telemetry-notice-in-json/proposal.md
Original file line number Diff line number Diff line change
@@ -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.
Original file line number Diff line number Diff line change
@@ -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"
Comment thread
coderabbitai[bot] marked this conversation as resolved.

#### 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
9 changes: 9 additions & 0 deletions openspec/changes/suppress-telemetry-notice-in-json/tasks.md
Original file line number Diff line number Diff line change
@@ -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
26 changes: 24 additions & 2 deletions src/cli/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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')
);
Comment on lines +132 to +136

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

rg -n -C 4 'allowUnknownOption|allowExcessArguments|--json|isJsonRun' src test

Repository: Fission-AI/OpenSpec

Length of output: 50376


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the CLI helper and surrounding tests, and list dependency versions/config snippets.
sed -n '120,170p' src/cli/index.ts
printf '\n--- relevant telemetry helper/tests ---\n'
sed -n '1,230p' test/telemetry/index.test.ts
printf '\n--- package commander version refs ---\n'
rg -n '"commander"|commander' package.json pnpm-lock.yaml package-lock.json yarn.lock 2>/dev/null | head -80

Repository: Fission-AI/OpenSpec

Length of output: 9706


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Locate definitions and usages that make subcommands accept residual --json.
rg -n "allowUnknownOption|allowExcessArguments|command\.args|function createProgram|program|subcommand|hooks" src test/telemetry/index.test.ts | head -200
printf '\n--- candidate source files ---\n'
git ls-files src | rg 'cli|command|index' | head -100

Repository: Fission-AI/OpenSpec

Length of output: 15953


🌐 Web query:

Commander.js terminal option terminator -- options after stop option processing args

💡 Result:

In Commander.js, the double dash (--) serves as a terminal option terminator [1][2][3]. When encountered, it instructs the parser to immediately stop processing further command-line arguments as options [2][4][3]. Any remaining arguments provided after the -- are treated strictly as positional operands (or command-arguments) and are not interpreted or parsed as flags, even if they begin with a dash (-) [1][5][6]. This behavior aligns with the standard POSIX guideline (Guideline 10), which uses -- as a delimiter to signify the end of options [6]. Common use cases include: 1. Passing arguments that start with a dash to a subcommand or script, ensuring they are not mistakenly parsed as options for your program [6]. 2. Separating your program's own flags from external arguments that might conflict with them [1]. If you want to avoid requiring your users to use --, you have a few alternatives: - Put your options before any positional arguments [1]. - Use.passThroughOptions to stop processing options automatically when a non-option argument is encountered, allowing subsequent arguments (including options meant for another program) to be passed through unprocessed [2][7][3]. - Incorporate -- into your documented usage syntax if you want to make it an explicit part of your command interface [1].

Citations:


🌐 Web query:

Commander.js v14 allowExcessArguments parse behavior command.args after --

💡 Result:

In Commander.js v14, the behavior of command-line arguments is defined by the default setting of.allowExcessArguments(false) [1][2]. 1. Excess Arguments Behavior Since version 13.0.0, Commander throws an error by default if a user provides more arguments than the command expects [2][3]. If your application receives unexpected arguments, the parser will stop and display an error message (e.g., "error: too many arguments") [1][4]. To revert to legacy behavior where excess arguments are silently ignored or collected in command.args, you must explicitly call.allowExcessArguments on your program or command instance [1][3]. 2. Parsing with -- The -- separator is a standard feature in Commander.js that signals the end of option processing [5][6]. Any arguments appearing after -- are treated as literal arguments, not as options [5][7]. Crucially, when -- is used: - Arguments following -- are still subject to the.allowExcessArguments check [4]. If you have a command that expects specific arguments and you provide more than that total (including those after --), Commander will still throw a "too many arguments" error unless.allowExcessArguments(true) is configured [4]. - These arguments are typically captured in command.args if they do not violate the excess argument limit [5][6]. - If you intend to pass arbitrary arguments through to a subprocess or another utility, using -- effectively stops the current command's option parser from misinterpreting flags that follow [8][6]. If you find yourself needing to manually parse or trim these arguments, consider using.passThroughOptions, which can sometimes obviate the need for -- by passing all subsequent arguments through to the action handler or a subcommand [9][6].

Citations:


Do not treat a post-terminator operand as JSON mode.

command.args includes arguments after --, and Commander stops option parsing at --. An invocation such as openspec store -- --json will match command.args.includes('--json') and suppress the telemetry notice as JSON output, even though --json is an operand, not an option. Preserve whether --json was parsed as an option and add a regression for ['store', '--', '--json'] that expects false.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/cli/index.ts` around lines 132 - 136, Update isJsonRun so --json is
recognized only when parsed as an option, not when present in command.args after
the -- terminator. Preserve the parsed option state and add a regression
covering ['store', '--', '--json'] that expects false.

}

program
.name('openspec')
.description('AI-native system for spec-driven development')
Expand All @@ -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);
Expand Down
11 changes: 10 additions & 1 deletion src/telemetry/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand All @@ -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'
Expand Down
79 changes: 79 additions & 0 deletions test/core/cli-is-json-run.test.ts
Original file line number Diff line number Diff line change
@@ -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);
});
});
33 changes: 33 additions & 0 deletions test/telemetry/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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', () => {
Expand Down
Loading