diff --git a/.changeset/telemetry-command-outcomes.md b/.changeset/telemetry-command-outcomes.md new file mode 100644 index 0000000000..a1e8986bf6 --- /dev/null +++ b/.changeset/telemetry-command-outcomes.md @@ -0,0 +1,15 @@ +--- +"@fission-ai/openspec": minor +--- + +Record how commands end in usage telemetry, so failures surface without someone filing an issue. + +A new `command_completed` event carries the outcome, a failure class from a fixed list, a bucketed exit code, and a bucketed duration. Runs that previously produced no telemetry at all now do: unknown commands, unknown flags, and a group invoked with no subcommand all exited before the tracking hook ran. Activation milestones, retry visibility, and a per-run correlation id are included. + +`OPENSPEC_TELEMETRY_DEBUG=1` prints every event to stderr and sends nothing, so the collected list can be verified locally rather than taken on trust. `openspec config get telemetry` now reports the enabled state, the anonymous id, and the file holding it. + +Command behavior, output, and exit codes are unchanged. Telemetry remains opt-out via `openspec config set telemetry.enabled false`, `OPENSPEC_TELEMETRY=0`, or `DO_NOT_TRACK=1`, and stays off in CI. + +**Privacy:** this collects more than earlier releases did. `SECURITY.md` previously stated that no environment was collected, and the README stated that only command names and version were collected. Both commitments end here: platform, Node major, install kind, and the invoking coding agent are now included. They are replaced by a narrower and checkable commitment — every event name, property key, and value must be a member of a fixed list, enforced by dropping anything else before the payload is built, so no field exists that could carry a name, path, or message. Tool identities are sent as standalone events with no other property attached, and durations and exit codes are bucketed, so no single event describes a machine precisely enough to single out its owner. The full property list is in the README, and a test fails if it drifts from the code. Data is described as pseudonymous rather than anonymous, and a deletion route is published — though deleting the local id severs your history without asking anyone. + +Every event sets `$ip: null` and `$geoip_disable: true`, so no address or derived location is recorded. The disclosure states that and the in-transit caveat, rather than asserting anything about proxy logging that this repository cannot enforce. No retention period is published until one is configured. diff --git a/README.md b/README.md index ddf36a476e..8731aec249 100644 --- a/README.md +++ b/README.md @@ -233,9 +233,52 @@ Open a discussion (for core design changes) or an issue before you open a PR, an
Telemetry -OpenSpec collects anonymous usage stats. +OpenSpec collects pseudonymous usage stats: a random id generated on your machine, plus the properties listed below. Automatically disabled in CI. -We collect only command names and version to understand usage patterns. No arguments, paths, content, or PII. Automatically disabled in CI. +**See exactly what would be sent, on your own machine:** + +```bash +OPENSPEC_TELEMETRY_DEBUG=1 openspec list +``` + +That prints every event to stderr and sends nothing. It works even if you have opted out, and it does not create the id it shows you. + +**Everything collected:** + +| Property | Values | +| --- | --- | +| `command` | The command you ran, e.g. `archive`, `change:validate`. Never its arguments | +| `version`, `version_code` | The OpenSpec version, and the same version as a sortable integer | +| `outcome` | `success`, `user_error`, `internal_error`, `cancelled` | +| `error_class` | The kind of failure, from a fixed list, e.g. `no_root`, `validation_failed`. Never the message | +| `exit_code` | `0`, `1`, `130`, `other` | +| `duration` | `<100`, `100-500`, `500-2000`, `2000-10000`, `10000+` milliseconds | +| `previous_outcome`, `previous_command_same` | Whether your last run failed, and whether it was the same command | +| `platform`, `node_major` | `darwin`/`linux`/`win32`; the Node major version | +| `install_kind` | `global`, `npx`, `source`, `other` | +| `invoker` | Which coding agent is running the command, from a fixed list, or `terminal`/`unknown` | +| `stdout_tty`, `json_mode`, `prompted`, `first_run` | Booleans | +| `profile`, `delivery` | Your install profile and delivery mode | +| `tools_count` | How many AI tools are configured: `0`, `1`, `2-3`, `4+` | +| `schema_source` | `package`, `project`, or `user` | +| `store_in_use` | Whether this run resolved through a store rather than a local root. Never which one | +| `changes` | How many active changes: `00`, `01-03`, `04-10`, `11-30`, `31+` | +| `milestone`, `time_to_reach` | The first time you reach each of `install` (your first run), `init`, `propose` (`openspec new change`), `apply` (`openspec validate`), and `archive`, and how long it took | +| `tool` | Each AI tool you have configured, reported once, as its own event carrying no run context and no run id | +| `run_id`, `work_session_id` | Random ids correlating one run, and runs less than 30 minutes apart | +| `surface` | Always `cli` | + +Every one of those has a fixed set of possible values. Anything else is dropped before the payload is built, so there is no field that could carry a name, path, or message. + +**Never collected:** command arguments, file paths, project names, change/spec/schema/artifact names, store ids or remotes, file contents, error messages, environment variable names or values, hostnames, usernames, git remotes, or IP addresses. + +**Stored on your machine** in the config file (`openspec config get telemetry` prints its path): the random id, the notice version, the time of your first run, the work-session id and last-activity time, which milestones and tools have been reported, and your previous run's outcome. Nothing is written at all if you have opted out. + +**No IP, no location.** Every event sets `$ip: null` and `$geoip_disable: true`, so the analytics backend records neither your address nor anything derived from it. Requests do reach a first-party endpoint that terminates TLS, which necessarily observes the connecting address in transit — those two flags are what the shipped code guarantees, and you can see them yourself with `OPENSPEC_TELEMETRY_DEBUG=1`. + +**Deletion:** open a [GitHub issue](https://github.com/Fission-AI/OpenSpec/issues/new) with the id from `openspec config get telemetry`, or send it privately through [GitHub Security Advisories](https://github.com/Fission-AI/OpenSpec/security/advisories/new) if you would rather not post it publicly. You do not have to ask us for anything, though: deleting the id from your config severs all future events from everything before it, immediately and on your own. + +The id identifies a configuration directory, not a person — a shared home directory means one id covers several people, so it is not a user count. **Opt-out (any one is enough):** - `openspec config set telemetry.enabled false` (global config; unset means on) diff --git a/SECURITY.md b/SECURITY.md index e0dc5e08da..e65a259f3d 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -12,7 +12,7 @@ Fixes ship in the latest published version on npm. Older versions are not patche ## Threat model -OpenSpec is a local command-line tool. It has no server, no network listener, and no privileged daemon. It reads and writes markdown under the directory you run it in, using paths you supply, with your own user permissions. It can offer to upgrade itself during `openspec update`, and only with your say-so. It sends anonymous usage telemetry, which you can disable with `OPENSPEC_TELEMETRY=0`. +OpenSpec is a local command-line tool. It has no server, no network listener, and no privileged daemon. It reads and writes markdown under the directory you run it in, using paths you supply, with your own user permissions. It can offer to upgrade itself during `openspec update`, and only with your say-so. It sends pseudonymous usage telemetry, which you can inspect with `OPENSPEC_TELEMETRY_DEBUG=1` and disable with `OPENSPEC_TELEMETRY=0`. That shapes what is and isn't a vulnerability here: @@ -45,7 +45,7 @@ ls node_modules | grep -E '^(vite|rollup|vitest|eslint|js-yaml|minimatch)$' # | Install scripts | The package ships no `preinstall`, `install`, or `postinstall` script, so installing it from the npm registry runs no code from OpenSpec. (`prepare` is still declared; npm runs it only for git and local-directory installs, where it builds from source.) Shell completions are opt-in via `openspec completion install`; the CLI prints a one-line tip about them on its first run. | | Running other programs | Every call that goes through a shell uses a fixed literal (`which gh`, `gh auth status`). Anything carrying your input — issue text, editor paths, workset commands, the path passed to `openspec update` — uses an argument array, never string interpolation into a shell. On Windows, `.cmd` shims are launched through `cross-spawn`, which escapes arguments rather than concatenating them. | | Installing software | `openspec update` can run `npm install -g @fission-ai/openspec@latest` and then re-run `openspec update` with the upgraded CLI. It does this only after you answer yes to a prompt, only for the OpenSpec package itself, only when npm owns the install, and never in CI or a non-interactive shell. A global install lives outside your project, so it runs with your permissions there and executes whatever lifecycle scripts the published package ships. It then reads the installed binary's version back rather than assuming the upgrade took. Decline and it prints the command for you to run yourself. | -| Telemetry | Command name, OpenSpec version, and a locally generated random UUID. No file paths, no file contents, no environment, no hostname, and IP capture is explicitly disabled. Opt out with `OPENSPEC_TELEMETRY=0` or `DO_NOT_TRACK=1`; it is off in CI automatically. | +| Telemetry | The command name, how it ended (outcome, a failure class from a fixed list, a bucketed exit code and duration), and bounded run context: platform, Node major, install kind, which coding agent invoked it, a count of configured tools, and bucketed counts of changes. Plus a locally generated random UUID. **This is more than earlier releases collected — platform and Node major are environment facts, which previous versions of this document said were not collected.** No file paths, no file contents, no environment variable names or values, no hostname, no usernames. Every event sets `$ip: null` and `$geoip_disable: true`, so neither your address nor a location derived from it is recorded; the ingest endpoint terminates TLS and so observes the connecting address in transit, which those flags do not change. Every property has a fixed set of possible values and anything else is dropped before the payload is built; see the full list in the README. Verify it yourself with `OPENSPEC_TELEMETRY_DEBUG=1`, which prints the events and sends nothing. Opt out with `OPENSPEC_TELEMETRY=0` or `DO_NOT_TRACK=1`; it is off in CI automatically. | | Network | Telemetry when enabled, and one npm registry request during `openspec update` to check whether a newer CLI has been published. That request sends no data about you beyond what any HTTP request reveals, runs once per `openspec update` with nothing cached, and is skipped when `CI` is set to anything but an explicit off-value, under `NODE_ENV=test`, or when `OPENSPEC_NO_UPDATE_CHECK`, `DO_NOT_TRACK=1`, or `OPENSPEC_TELEMETRY=0` is set. Reading, writing, and validating specs is entirely local. | ## Automated checks diff --git a/docs-lab/reference/configuration/environment-variables.md b/docs-lab/reference/configuration/environment-variables.md index dc1c608ef8..c68b4cfc8a 100644 --- a/docs-lab/reference/configuration/environment-variables.md +++ b/docs-lab/reference/configuration/environment-variables.md @@ -9,6 +9,15 @@ its network-permission flag. XDG vars move the config/data directories. --> ## OPENSPEC_TELEMETRY +Set to `0` to disable usage telemetry. Telemetry is on by default (opt-out) and +off automatically when `CI` is set to anything but an explicit off-value. + +## OPENSPEC_TELEMETRY_DEBUG + +Set to `1` to print every telemetry event to stderr and send nothing. Works +while opted out, and does not create the anonymous id it shows you. This is the +way to verify what is collected without taking the documentation on trust. + ## DO_NOT_TRACK ## XDG_CONFIG_HOME and XDG_DATA_HOME diff --git a/docs/cli.md b/docs/cli.md index e74bc60f2d..200d8b1e44 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -1183,7 +1183,10 @@ openspec config profile core ``` **Telemetry opt-out:** `telemetry.enabled` defaults to on when unset (opt-out model). -Set it to `false` to disable anonymous usage stats and the `openspec update` version check. +Set it to `false` to disable pseudonymous usage stats and the `openspec update` version check. +`OPENSPEC_TELEMETRY_DEBUG=1` prints every event that would be sent to stderr and sends nothing, so you can +see exactly what is collected. `openspec config get telemetry` reports the current state, the anonymous id, +and the file holding it. Environment variables take precedence over config: `OPENSPEC_TELEMETRY=0`, `DO_NOT_TRACK=1`, and a truthy `CI` value (e.g. `true`/`1`/`yes`) always disable telemetry regardless of the config value. diff --git a/docs/faq.md b/docs/faq.md index 770479aa3e..65a0733106 100644 --- a/docs/faq.md +++ b/docs/faq.md @@ -132,7 +132,7 @@ OpenSpec works best with high-reasoning models. The README recommends models lik ### Does OpenSpec collect data? -It collects anonymous usage stats: command names and version only. No arguments, paths, content, or personal data, and it's off automatically in CI. Opt out with `export OPENSPEC_TELEMETRY=0` or `export DO_NOT_TRACK=1`. +It collects pseudonymous usage stats: the command you ran, how it ended, and bounded run context such as OS and Node major. No arguments, paths, content, item names, or personal data, and it's off automatically in CI. Every property has a fixed set of possible values — see the full list in the README. Print exactly what would be sent with `OPENSPEC_TELEMETRY_DEBUG=1`. Opt out with `export OPENSPEC_TELEMETRY=0` or `export DO_NOT_TRACK=1`. ### How do I upgrade? diff --git a/openspec/changes/add-command-outcome-telemetry/proposal.md b/openspec/changes/add-command-outcome-telemetry/proposal.md new file mode 100644 index 0000000000..0043b3808d --- /dev/null +++ b/openspec/changes/add-command-outcome-telemetry/proposal.md @@ -0,0 +1,122 @@ +# Record command outcomes in anonymous telemetry + +## Why + +Telemetry today fires once, in the `preAction` hook, carrying `command`, +`version`, and `surface` (`src/telemetry/index.ts`, `trackCommand`). It can +answer "how often is `archive` run" and nothing else. + +It cannot answer any question we act on: + +- Did the command **succeed**? Nothing is recorded after the action runs. +- If it failed, **how**? Every command catches its own error, prints + `Error: `, and sets `process.exitCode = 1`. The class of failure + never leaves the process. +- Was the caller a **person or an agent**? Both look identical. +- Do users get from `init` to a first archived change? Unknown. + +Worse, failures are the *least* visible runs. Seventeen call sites in +`src/cli/index.ts` end with `process.exit(1)`, which skips commander's +`postAction` hook — the trap the code already documents at +`src/cli/index.ts:317` and `:468`. Commander's own usage errors — unknown +command, unknown flag, a group run with no subcommand — exit before `preAction` +runs, so they produce no event at all. And commander chains hooks without a +`catch`, so an error escaping a command's own handling skips the hook too. + +So the runs we most need to see are the ones most likely to vanish: the user +who typed the wrong command, and our own bugs. + +The feedback loop is closed only in the good case. Users hitting a confusing +failure do not run `openspec feedback` and do not open an issue; they stop using +the tool. We ship fixes for the problems that get reported, not the problems +that happen. + +This change closes that loop without collecting anything about *what* a user is +working on. + +## What Changes + +**One new event, `command_completed`**, emitted on every exit path that reaches +our own error handling, carrying the outcome, a bounded error class, a bucketed +exit code, and a bucketed duration. + +**A hard property contract.** Every event name, property key, and property value +must be a member of a compile-time list, a boolean, or a bucket label — and the +allowlist is enforced at send time, not just asserted in a test. This is the +structural reason the new data cannot describe what someone is working on: there +is no field it could travel in, and an unrecognized field is dropped before the +payload is serialized. + +**Bounded run context**: platform, Node major, install kind, invoker, TTY and +JSON flags, profile, delivery, a *count* of configured tools, where the schema +came from, and a bucketed change count. + +Deliberately excluded, each for a stated reason: schema, artifact, change, spec, +and store names, because they are user-authored text; store remotes and paths, +because they identify an organization; and **raw millisecond durations**, because +they profile the machine and, at an interactive prompt, record human response +times. + +**Which assistant people use, without the fingerprint.** Tool identity ships as a +separate `tool_configured` event — once per tool per user, carrying no run +context at all. That answers how much of the userbase runs Cursor or Claude Code +while never assembling the configured *set* alongside platform, install kind, and +counts in one row, which is the combination that would single out an unusual +user. The `invoker` enum complements it by recording which agent is actually +driving a given run. + +**Telemetry never interrupts.** No prompts, ever. It does not block the command, +does not delay exit beyond the existing 1-second timeout, and never writes to +stdout. The one-line first-run disclosure is a notice on stderr, not a question. + +**Outcome coverage** for all three families of exit that skip the hooks today, +including commander's own usage errors — which are invisible now and are exactly +the "user typed the wrong thing" signal. + +**Retry visibility.** Whether a user recovers from a failure is the most +actionable signal we can have, and it is not otherwise computable. Two bounded +properties carry it: the previous run's outcome, and whether it was the same +command. + +**Correlation at two scales:** a per-invocation `run_id` whose real job is to +reveal exit paths this spec failed to cover, and a `work_session_id` with a +30-minute window, because a CLI work session is many invocations and command +sequences are not computable without it. + +**Five milestone events** — `install`, `init`, `propose`, `apply`, `archive` — +giving the activation funnel a real denominator. + +**`OPENSPEC_TELEMETRY_DEBUG=1`** prints every event that would be sent and sends +nothing. It works when telemetry is *disabled*, since the person most likely to +want it is someone who opted out and is deciding whether to opt back in, and it +never creates the anonymous id it is being used to inspect. + +**Data subject controls**: `openspec config get telemetry` shows the state, the +id, and the file holding it; deleting the id severs all future events from all +prior ones; the disclosure carries a retention period and a deletion contact. + +**Honest disclosure.** `SECURITY.md` promises "no environment" and `README.md` +promises "only command names and version." This change ends both. The spec +requires the changelog to say so under a `Privacy` heading rather than quietly +editing the promise, requires a test that fails when an allowlisted property is +undocumented, and requires the docs to stop calling the data "anonymous" +unqualified — a persistent id plus device characteristics is pseudonymous, and +overstating it is what would undermine every other claim on the page. + +Two smaller corrections the review surfaced: cancellation must never delay exit +to flush telemetry (Ctrl-C should stop the process, not phone home), and the +ingest proxy must not log client IPs — it terminates TLS, so `$ip: null` +governs what the backend records, not what our own infrastructure sees. + +Telemetry stays opt-out and unchanged otherwise: same `OPENSPEC_TELEMETRY=0`, +`DO_NOT_TRACK=1`, `openspec config set telemetry.enabled false`, same automatic +off-in-CI, same silent failure, same 1-second timeout. Users who have seen the +old notice get a one-line notice naming what changed, once. + +## Impact + +- Affected specs: `telemetry` (ADDED: 15 requirements; MODIFIED: 3) +- Affected code: `src/telemetry/`, `src/cli/index.ts`, `src/commands/shared-output.ts`, `src/commands/config.ts` +- Affected docs: `README.md`, `SECURITY.md`, `CHANGELOG.md`, `docs-lab/reference/configuration/environment-variables.md` +- Affected infrastructure: the `edge.openspec.dev` ingest proxy (IP logging, GeoIP) +- Command behavior, output, and exit codes are unchanged for every user. diff --git a/openspec/changes/add-command-outcome-telemetry/specs/telemetry/spec.md b/openspec/changes/add-command-outcome-telemetry/specs/telemetry/spec.md new file mode 100644 index 0000000000..5f6917bcd7 --- /dev/null +++ b/openspec/changes/add-command-outcome-telemetry/specs/telemetry/spec.md @@ -0,0 +1,599 @@ +## ADDED Requirements + +### Requirement: Bounded property contract +Every event name, property key, and property value SHALL be a member of a compile-time constant list declared in source. A property value SHALL be one of: a member of such a list, a boolean, or a bucket label from a fixed bucket list. + +An event name or property key SHALL NOT be constructed by concatenation, interpolation, or any other transformation of a runtime value. Binding only values would leave the guarantee open: `{"schema:acme-internal": true}` carries a boolean value and still ships the user's schema name. + +A property value SHALL NOT be derived from user-authored text. This includes, and is not limited to: change ids, spec ids, schema ids, artifact ids, store ids, store remotes, store branches, store local paths, `defaultStore`, `featureFlags` keys, `openers` content, file paths, project names, error messages, and command arguments. + +Where a value comes from a set the user can extend, the system SHALL check membership against the compile-time list and SHALL substitute a fixed fallback label (or omit the property) when the value is not a member. The system SHALL NOT pass such a value through unchecked. + +The allowlist SHALL be authoritative at send time, not only at build time. Immediately before serialization the system SHALL drop any property whose key is not on the allowlist, and any value that is not a member of that property's declared value set. A dropped property SHALL NOT prevent the event from being sent. Enforcement in a test alone is insufficient: a test passes vacuously for any code path it does not construct. + +The lists SHALL be literal declarations in source. They SHALL NOT be computed from a schema, catalog, or any other file the user can author. + +A property SHALL NOT be added where the joint distribution of an event's properties would make a substantial fraction of runs unique. A set-valued property drawn from a registry of more than eight members SHALL be sent as a count bucket rather than as a set. + +#### Scenario: Value from a closed constant list +- **WHEN** the system records the active install profile +- **THEN** the property value is one of the values declared by the `Profile` type (`core`, `custom`) + +#### Scenario: Value from a user-extensible set +- **WHEN** a user has forked a schema into `openspec/schemas/acme-internal/` +- **AND** a command runs against that schema +- **THEN** no property key or value carries the string `acme-internal` +- **AND** the schema is described only by where it was loaded from (`package`, `project`, or `user`) + +#### Scenario: Property key derived from a runtime value +- **WHEN** code attempts to send a property whose key embeds a schema, change, or store name +- **THEN** the key is not on the allowlist +- **AND** the property is dropped before the event is serialized + +#### Scenario: Unknown property dropped at send time +- **WHEN** a code path adds a property that is not on the allowlist +- **THEN** the property is dropped immediately before serialization +- **AND** the event is still sent with its remaining properties + +#### Scenario: Counts are bucketed +- **WHEN** the system records how many active changes a project has +- **THEN** the property value is a bucket label from a fixed list, not the exact count + +#### Scenario: New property without a closed value set +- **WHEN** a proposed property's value set cannot be enumerated at build time +- **THEN** the property SHALL NOT be added to any event + +### Requirement: Command outcome tracking +The system SHALL send a `command_completed` event after every command finishes, whether it succeeded or failed, carrying `command`, `version`, `surface`, `run_id`, `work_session_id`, `outcome`, `error_class`, `exit_code`, and `duration`. + +`outcome` SHALL be one of: `success`, `user_error`, `internal_error`, `cancelled`. + +`command` SHALL be the command path commander resolved, checked for membership in the registered command list, and sent as `unknown` when it is not a member. It SHALL NOT be derived from what the user typed. + +`exit_code` SHALL be a bucket label from the fixed list `0`, `1`, `130`, `other`. It SHALL NOT be the raw process exit code, because several commands pass a child process's code through unchanged — `workset open` returns the launched editor's code (including `128 + signal`), `feedback` returns `gh`'s status, and `update` returns the re-spawned CLI's code. A raw code would be an unbounded value. + +`duration` SHALL be a bucket label from the fixed list `1_under_100ms`, `2_100-500ms`, `3_500ms-2s`, `4_2-10s`, `5_over_10s`, measured in milliseconds from the start of the `preAction` hook, excluding any time the process spent blocked on an interactive prompt. + +Raw millisecond durations SHALL NOT be sent. Full-resolution timings profile the machine's performance, leak repo scale past the count buckets, and — on interactive commands — record human response times, which are a behavioral biometric. Excluding prompt-blocked time is also what makes the measurement mean anything: `init`, `archive`, and `config` all prompt, so an unexcluded duration measures how long someone read a menu. + +A command that fails a check it was asked to perform — a failing `validate`, an `archive` blocked by incomplete tasks — SHALL be recorded as `user_error`, never `internal_error`. Failing a check is a routine outcome of the command working correctly. + +#### Scenario: Successful command +- **WHEN** a command completes with exit code 0 +- **THEN** the system sends `command_completed` with `outcome: "success"`, `error_class: "none"`, and `exit_code: "0"` + +#### Scenario: Failed command +- **WHEN** a command fails and sets a non-zero exit code +- **THEN** the system sends `command_completed` with a non-`success` outcome and a classified `error_class` + +#### Scenario: Exit code passed through from a child process +- **WHEN** `openspec workset open` exits with the launched editor's code of 137 +- **THEN** the event carries `exit_code: "other"` +- **AND** no property carries the value 137 + +#### Scenario: Failing validation is not an internal error +- **WHEN** `openspec validate` runs correctly and reports the change is invalid +- **THEN** the event carries `outcome: "user_error"` and `error_class: "validation_failed"` + +#### Scenario: Time spent at a prompt is excluded +- **WHEN** a command waits four minutes for a user to answer a confirmation prompt and then finishes in 300ms of work +- **THEN** the event carries `duration: "2_100-500ms"` + +#### Scenario: Unregistered command name +- **WHEN** the resolved command path is not a member of the registered command list +- **THEN** the event carries `command: "unknown"` + +#### Scenario: No message content +- **WHEN** a command fails with the message `Change "acme-billing-rewrite" not found` +- **THEN** the event carries `error_class: "item_not_found"` and no part of the message + +### Requirement: Bounded error classification +The system SHALL classify a failure into an `error_class` drawn from a compile-time allowlist declared as a literal string union in source. A failure that reaches this classifier and cannot be identified SHALL be recorded as `error_class: "other"` with `outcome: "internal_error"`. A failure that never reaches a classifier at all — a non-zero exit set without a throw, of which the CLI has many — SHALL be recorded as `error_class: "unclassified"` with `outcome: "user_error"`. + +The distinction is what keeps `internal_error` meaning "our bug". An error object we failed to recognize is ours to explain; a command that simply set an exit code is not evidence of anything, and counting it as a bug would drown the metric that exists to find real ones. + +Where a failure carries a diagnostic `code` (as `StoreError` and `RootSelectionError` do), the system SHALL map that code through the allowlist and SHALL NOT send the code through unchecked, because a diagnostic code is not guaranteed to be free of user-authored text. + +The allowlist SHALL cover at minimum these classes, which correspond to the failure families the CLI actually has: + +| Class | Covers | +| --- | --- | +| `none` | Success | +| `cancelled` | Ctrl-C at a prompt, a declined confirmation | +| `not_interactive` | A prompt was needed but stdin/stdout is not a terminal, or `--json` was passed. Distinct from `cancelled`: the user was never asked | +| `no_root` | No OpenSpec root resolved, no registered store, unhealthy or mismatched store root | +| `item_not_found` | A named change, spec, workset, or store does not exist | +| `ambiguous_item` | A name matched more than one item | +| `schema_not_found` | A schema, artifact, or template could not be resolved | +| `schema_invalid` | A schema failed its own validation | +| `bad_usage` | Bad flags or arguments, including commander's own usage errors | +| `unknown_subcommand` | A group was given an operand it does not recognize | +| `validation_failed` | Content failed validation — a routine outcome, not an exception | +| `archive_blocked` | Archive refused a precondition: incomplete tasks, existing target, failed spec validation | +| `concurrent_modification` | The working tree changed underneath a command mid-operation | +| `store_error` | Store registration, metadata, identity, or path failures | +| `git_error` | Store git init, identity, commit, or remote failures | +| `fs_error` | Permission denied, path outside the allowed directory, not writable, not a directory | +| `parse_error` | A markdown or YAML document could not be parsed | +| `metadata_invalid` | Change metadata was missing or malformed | +| `external_tool_failed` | A launched editor, agent, or the `gh` CLI failed | +| `network_error` | An outbound request failed | +| `already_exists` | A create operation found its target already present | +| `unclassified` | A non-zero exit that reached no classifier, or a diagnostic code the map does not know. Recorded as a user error: the CLI has many paths that set an exit code without throwing, and presuming a bug there would drown the `internal_error` rate | +| `internal_error` | An error that escaped a command's own handling | +| `other` | Anything unmapped | + +#### Scenario: Known diagnostic code +- **WHEN** a command fails with diagnostic code `unknown_item` +- **THEN** the event carries the mapped `error_class: "item_not_found"` + +#### Scenario: Unrecognized diagnostic code +- **WHEN** a command fails with a diagnostic code the map does not know +- **THEN** the event carries `error_class: "unclassified"` +- **AND** the raw code is not sent + +#### Scenario: Exit code set without a throw +- **WHEN** a command sets a non-zero exit code without throwing and without classifying +- **THEN** the event carries `error_class: "unclassified"` and `outcome: "user_error"` + +#### Scenario: Unclassified error +- **WHEN** a command fails with a plain `Error` carrying no diagnostic +- **THEN** the event carries `error_class: "other"` +- **AND** carries `outcome: "internal_error"` + +### Requirement: Outcome coverage across exit paths +Every exit path that reaches the CLI's own error handling SHALL produce exactly one `command_completed` event before the process exits. Paths outside that handling — an OOM kill, `SIGKILL`, a crash in the runtime itself — cannot emit and are out of scope. + +Three families of exit currently bypass commander's `postAction` hook, and all three SHALL be covered: + +1. **Action handlers that call `process.exit()`.** These SHALL set `process.exitCode` and return instead, so the hook runs. This covers the seventeen `process.exit(1)` sites in `src/cli/index.ts`, the `process.exit(1)` in `src/core/view.ts` and the `config` group guard, and the `process.exit(0)` success paths in `src/core/init.ts`, `src/ui/welcome-screen.ts`, and `src/commands/feedback.ts`. +2. **Commander's own usage errors.** Unknown option, unknown command, missing argument, excess arguments, and a group invoked with no subcommand all exit before the `preAction` hook runs, so today they produce no event at all. The system SHALL intercept these and emit `command_completed` with `error_class: "bad_usage"` before exiting with the code commander chose. +3. **A rejected action promise.** Commander chains hooks without a `catch`, so a throw that escapes a command's own handler skips `postAction` and becomes an unhandled rejection. The system SHALL install a handler that emits `command_completed` with `outcome: "internal_error"` and then preserves the existing exit behavior. + +Telemetry flushing is asynchronous, so the system SHALL NOT rely on a `process.on('exit')` handler, which cannot await. + +`--help` and `--version` SHALL NOT emit a `command_completed` event. + +#### Scenario: Failing command reaches the completion hook +- **WHEN** a command fails +- **THEN** the process does not call `process.exit()` before the `postAction` hook has run +- **AND** exactly one `command_completed` event is sent + +#### Scenario: Exit code preserved +- **WHEN** a failing command sets `process.exitCode` instead of calling `process.exit()` +- **THEN** the process still exits with the same code it exited with before this change + +#### Scenario: Unknown command +- **WHEN** a user runs `openspec proposal` and commander rejects it as an unknown command +- **THEN** the system sends `command_completed` with `error_class: "bad_usage"` +- **AND** the process still exits with the code commander chose + +#### Scenario: Group invoked with no subcommand +- **WHEN** a user runs `openspec spec` with no subcommand and commander prints help and exits 1 +- **THEN** the system sends `command_completed` with `error_class: "bad_usage"` + +#### Scenario: Help and version are not commands +- **WHEN** a user runs `openspec --help` or `openspec --version` +- **THEN** no `command_completed` event is sent + +#### Scenario: Error escaping a command handler +- **WHEN** an action handler rejects with an error its own catch does not cover +- **THEN** the system sends `command_completed` with `outcome: "internal_error"` +- **AND** the process exits as it did before this change + +#### Scenario: No duplicate events +- **WHEN** a command both sets an exit code and returns normally +- **THEN** exactly one `command_completed` event is sent for that invocation + +### Requirement: Cancellation never delays exit +A cancelled run SHALL NOT delay process exit in order to send or flush telemetry. Where the event cannot be dispatched without delaying exit, it SHALL be dropped. + +Ctrl-C is the user asking the process to stop. A request that holds the process open for up to the telemetry timeout while the user presses Ctrl-C again is a worse outcome than a lossy cancellation metric, and the ratio is all the metric is used for. + +#### Scenario: Ctrl-C during a command +- **WHEN** a user presses Ctrl-C +- **THEN** the process exits without waiting on a telemetry request +- **AND** the cancellation event is dropped if it cannot be sent without waiting + +#### Scenario: Cancellation recorded when it is free +- **WHEN** a command exits 130 through the normal completion hook +- **THEN** the system sends `command_completed` with `outcome: "cancelled"` and `error_class: "cancelled"` + +### Requirement: Run and work session correlation +The system SHALL generate a random UUID per CLI invocation and include it as `run_id` on every event from that invocation. The `run_id` SHALL NOT be persisted to disk and SHALL NOT be derived from the anonymous id, the process id, the working directory, or the clock. + +`run_id` pairs `command_executed` with `command_completed`. Its job is to reveal when that pair is broken — an invocation that started and never completed is the signature of an exit path this spec failed to cover. + +The system SHALL additionally maintain a `work_session_id`: a random UUID persisted in the telemetry config section alongside the time of last activity. It SHALL be reused when the last activity was less than 30 minutes ago and regenerated otherwise. The value SHALL be random; only the reuse window consults the clock. + +A CLI work session is many invocations, not one. Without a correlation unit spanning them, command sequences and within-session drop-off are not computable at all. + +#### Scenario: Same run id across one invocation's events +- **WHEN** one invocation sends multiple events +- **THEN** every event carries the same `run_id` + +#### Scenario: Different run id across invocations +- **WHEN** the same user runs two commands in sequence +- **THEN** the two invocations carry different `run_id` values +- **AND** both carry the same `anonymousId` as `distinct_id` + +#### Scenario: Work session continues across invocations +- **WHEN** a user runs a second command ten minutes after the first +- **THEN** both invocations carry the same `work_session_id` + +#### Scenario: Work session expires +- **WHEN** a user runs a command more than 30 minutes after their last one +- **THEN** the invocation carries a newly generated `work_session_id` + +#### Scenario: Run id not persisted +- **WHEN** an invocation ends +- **THEN** no `run_id` is written to the global config file + +### Requirement: Retry visibility +The system SHALL persist the outcome and command of the previous invocation in the telemetry config section, and SHALL include `previous_outcome` (an `outcome` value or `none`) and `previous_command_same` (boolean) on `command_completed`. + +Whether a user recovers from a failure is the most actionable maintainer signal available, and it is not otherwise computable: a funnel cannot express "same command, previously failed, now succeeded" without raw queries. + +Only the outcome label and the previous command name SHALL be stored, and the name SHALL be compared locally and never sent — the event carries a boolean, not the name. + +#### Scenario: Successful retry +- **WHEN** a user runs a command that fails, then runs the same command again and it succeeds +- **THEN** the second event carries `previous_outcome: "user_error"` and `previous_command_same: true` + +#### Scenario: First invocation ever +- **WHEN** no previous invocation is recorded +- **THEN** the event carries `previous_outcome: "none"` + +#### Scenario: Different command +- **WHEN** the previous invocation was a different command +- **THEN** the event carries `previous_command_same: false` + +### Requirement: Bounded run context +The system SHALL attach run context to `command_completed`. Every context property SHALL satisfy the bounded property contract. + +The context SHALL be limited to: `platform` (`darwin`, `linux`, `win32`, `other`), `node_major` (a label from a fixed list of supported majors, `other` otherwise), `install_kind` (`global`, `npx`, `source`, `other`), `invoker`, `stdout_tty` (boolean), `json_mode` (boolean), `prompted` (boolean), `profile`, `delivery`, `tools_count` (bucket), `schema_source` (`package`, `project`, `user`), `store_in_use` (boolean), `changes` (bucket), and `first_run` (boolean). + +Context SHALL be kept to what a decision actually turns on. Each property is a bit of entropy in a row that already carries a persistent id, and bits accumulate into a fingerprint whether or not any single one looks harmful. A property nobody would act on is not neutral — it is cost with no return, and it SHALL be removed rather than kept for completeness. + +`tools_count` SHALL be a bucket label from `0`, `1`, `2-3`, `4+`. The identities of the configured tools SHALL NOT appear on a per-run event. The registry holds tens of tools, so a set drawn from it carries more than enough entropy to make an off-the-mode user unique when joined with the rest of the context — which is the whole risk, since it would attach a real-world identity to the anonymous id rather than merely linking sessions. + +`invoker` SHALL be a label from a fixed list of known coding-agent environments, `terminal` when none matches and stdout is a terminal, and `unknown` otherwise. It SHALL be derived by testing for the presence of a compile-time list of environment markers. No environment variable name or value SHALL be sent, and an unrecognized marker SHALL collapse to `unknown`. The markers probed SHALL be named in the public disclosure. + +`prompted` SHALL be true when the invocation opened any interactive prompt, or could have — both streams a terminal and interactivity not disabled. The measured half excludes think time from the duration; the capability half is what latency analysis filters on, since a run that could have prompted is not comparable to an agent's. + +Prompts SHALL be loaded through a single seam so the timing is applied once rather than at each call site, which is how it would rot. + +`first_run` SHALL be true only on the invocation during which the anonymous id is generated. It is not per-project. + +Count buckets SHALL use the fixed labels `00`, `01-03`, `04-10`, `11-30`, `31+`. + +Bucket labels SHALL be written so they sort in their natural order under a lexicographic sort, because that is how they are ordered wherever they are charted. A scrambled histogram is worse than no histogram. + +Collecting run context SHALL NOT add filesystem traversal beyond a single non-recursive directory read per counted collection. Any context value that cannot be read cheaply or throws SHALL be omitted, and the event SHALL still be sent. + +#### Scenario: Context collection failure +- **WHEN** reading the changes directory throws +- **THEN** the `changes` property is omitted +- **AND** the `command_completed` event is still sent with its remaining properties + +#### Scenario: Store in use +- **WHEN** a command resolves its root through a registered store +- **THEN** the event carries `store_in_use: true` +- **AND** carries no store id, remote, branch, or path + +#### Scenario: Configured tools are counted, not named +- **WHEN** a user has three AI tools configured +- **THEN** the event carries `tools_count: "2-3"` +- **AND** carries no tool identity + +#### Scenario: Agent-driven run +- **WHEN** a command is run inside a recognized coding agent +- **THEN** the event carries that agent's `invoker` label +- **AND** carries no environment variable name or value + +#### Scenario: Unrecognized environment +- **WHEN** no known agent marker is present and stdout is not a terminal +- **THEN** the event carries `invoker: "unknown"` + +#### Scenario: Interactive run is marked +- **WHEN** a command opens a confirmation prompt +- **THEN** the event carries `prompted: true` + +#### Scenario: No traversal for counts +- **WHEN** the system counts active changes +- **THEN** it performs a single non-recursive directory read and discards the entry names, keeping only the bucketed count + +### Requirement: Activation milestone events +The system SHALL send a `milestone_reached` event the first time a user reaches each of `install`, `init`, `propose`, `apply`, and `archive`, carrying `milestone`, `version`, `run_id`, and `time_to_reach`. + +`propose` and `apply` are agent workflows, not CLI commands, so the CLI SHALL observe them through the commands run on their behalf: `new change` for `propose`, and `validate` for `apply`. A milestone that no command can reach is not a funnel step. + +`install` SHALL be recorded on the invocation that generates the anonymous id. Without it the activation funnel has no denominator. The remaining milestones SHALL be recorded on first successful completion of the corresponding command. + +A milestone SHALL be recorded at most once per anonymous id. The set of milestones already reached SHALL be persisted in the global config under the telemetry section. + +`time_to_reach` SHALL use the fixed labels `1_under_1h`, `2_1-24h`, `3_1-7d`, `4_8-30d`, `5_over_30d`, computed from a date recorded when the anonymous id is first generated. + +The sub-day buckets are deliberate. Whether a user reaches their first archived change in one sitting or on the fourth day is the difference between a tool that lands and one that needs a second attempt, and it is the activation question an investor asks by name. A coarser first bucket makes the two indistinguishable. + +The residual risk is stated rather than hidden: a `<1h` milestone, combined with the server's own receipt time, dates that user's first run to within the hour. This is accepted because the run context no longer carries a fingerprint to join it against — tool identities are decoupled, durations and exit codes are bucketed — so the value dates a cohort rather than identifying a person. The recorded date SHALL NOT be sent directly, only the bucket. + +The recorded date is the first run with telemetry enabled, not the install. It SHALL be named accordingly and SHALL NOT be described as an install date. + +Where an anonymous id predates the recorded date, `time_to_reach` SHALL be omitted rather than sent as the lowest bucket, which would fabricate a wave of instant activations across the existing userbase. + +#### Scenario: First successful archive +- **WHEN** a user archives a change successfully for the first time +- **THEN** the system sends `milestone_reached` with `milestone: "archive"` and the current `version` + +#### Scenario: Subsequent archive +- **WHEN** the same user archives another change later +- **THEN** no further `archive` milestone event is sent + +#### Scenario: Failed command reaches no milestone +- **WHEN** a command fails +- **THEN** no milestone is recorded for it + +#### Scenario: Existing user with no recorded date +- **WHEN** a user whose anonymous id predates this change reaches a milestone +- **THEN** the event omits `time_to_reach` + +#### Scenario: Milestones respect opt-out +- **WHEN** telemetry is disabled +- **THEN** no milestone is sent and no milestone state is written to config + +### Requirement: Bounded persisted telemetry state +State the system persists for telemetry SHALL be limited to enum labels, counters, booleans, timestamps, and randomly generated identifiers. A persisted timestamp SHALL NOT be sent; only a bucket derived from it may be. It SHALL NOT include command arguments, item names, paths, hashes of paths, or any other user-authored value. + +No telemetry state SHALL be written to disk when telemetry is disabled. This covers the anonymous id, the work session id and its activity time, the milestone set, the first-seen time, the reported tool set, and the previous-outcome record. + +The public disclosure SHALL enumerate every field persisted for telemetry and SHALL state where the file lives. + +#### Scenario: Opted-out user leaves no trace +- **WHEN** a user has opted out and runs any command +- **THEN** no telemetry field is created or updated in the global config + +#### Scenario: Persisted state is enumerable +- **WHEN** a user opens the global config file +- **THEN** every telemetry field it holds is one the disclosure names + +### Requirement: Event volume cap +The system SHALL cap the events one CLI invocation may send. Where more would be produced, the excess SHALL be dropped rather than queued. + +A one-shot event — a milestone, a tool report — SHALL check the remaining budget *before* claiming. A claim is persisted permanently, so claiming and then hitting the cap would lose that milestone for the life of the install; leaving it unclaimed sends it one run later instead. + +An agent harness can invoke the CLI dozens of times inside one task. An uncapped per-invocation event count turns that into a burst of outbound requests the user never asked for. + +#### Scenario: Invocation producing many events +- **WHEN** an invocation would produce more events than the cap allows +- **THEN** the excess are dropped +- **AND** the command completes normally + +#### Scenario: One-shot event that does not fit +- **WHEN** a milestone or tool report cannot be sent because the cap is reached +- **THEN** it is not marked as reported +- **AND** it is sent on a later invocation + +### Requirement: Telemetry never interrupts the user +Telemetry SHALL be silent and non-blocking. It SHALL NOT prompt the user, SHALL NOT ask for input, SHALL NOT block or delay command execution, and SHALL NOT write to stdout. + +No telemetry decision SHALL ever be put to the user interactively. Consent is expressed through the documented opt-out mechanisms, which work offline and without a prompt. A CLI that stops to ask about analytics is a CLI that interrupts an agent mid-task. + +The one-line first-run disclosure is a notice on stderr, not a prompt: it asks nothing, blocks nothing, and the command proceeds regardless. + +Requests SHALL remain fire-and-forget and time-bounded, and a failure SHALL remain silent. + +#### Scenario: Telemetry never asks +- **WHEN** any telemetry code path runs +- **THEN** no prompt is displayed and no input is read + +#### Scenario: Command is not delayed +- **WHEN** the telemetry endpoint is slow or unreachable +- **THEN** the command runs and exits without waiting beyond the request timeout + +#### Scenario: stdout stays clean +- **WHEN** telemetry emits anything at all +- **THEN** it is written to stderr, never stdout + +### Requirement: Assistant adoption tracking +The system SHALL send a `tool_configured` event once per configured tool id per anonymous id, carrying only `tool` (an id checked for membership in the `AI_TOOLS` registry), `version`, and `run_id`. + +The event SHALL carry no run context and no `run_id`. Sending tool identities on every `command_completed` would put the full configured *set* in one row alongside platform, install kind, and counts, which is enough to make an unusual user unique; keeping the run id here would let a single join rebuild that same row. + +The limit of this split SHALL be stated rather than overclaimed: these events still share `distinct_id` with every other event, so a determined query can associate them. What it buys is that the set is never assembled in one row, and that the tools of a user who never completes a command are never learned. + +The set of tools already reported SHALL be persisted in the telemetry config section, on the same terms as milestones. + +#### Scenario: Tool configured +- **WHEN** a user has Cursor configured and no `tool_configured` event has been sent for it +- **THEN** the system sends `tool_configured` with `tool: "cursor"` +- **AND** the event carries no platform, install kind, count, or other run context + +#### Scenario: Reported once +- **WHEN** the same user runs another command +- **THEN** no further `tool_configured` event is sent for that tool + +#### Scenario: Tool added later +- **WHEN** a user configures an additional tool +- **THEN** a `tool_configured` event is sent for the new tool only + +#### Scenario: Unregistered tool id +- **WHEN** a configured tool id is not a member of the `AI_TOOLS` registry +- **THEN** no event is sent for it + +### Requirement: Local telemetry inspection +The system SHALL print every event it would send to stderr and send nothing when `OPENSPEC_TELEMETRY_DEBUG` is set to `1`. The printed form SHALL be the exact payload, so a user can verify what is collected without trusting the documentation. + +Debug mode SHALL work when telemetry is disabled, printing the payloads that would be sent, prefixed with a line stating telemetry is off and nothing was sent. The person most likely to want to inspect the payloads is the person who has already opted out and is deciding whether to opt back in; hiding the verification path from them defeats its purpose. + +Debug mode SHALL NOT generate or persist an anonymous id, or any other telemetry state. Where a payload would carry an id that does not yet exist, the printed form SHALL show a placeholder. Inspecting the telemetry must not create the identifier being inspected. + +#### Scenario: Debug mode prints and does not send +- **WHEN** `OPENSPEC_TELEMETRY_DEBUG=1` is set +- **THEN** each event payload is printed to stderr +- **AND** no network request is made + +#### Scenario: Debug mode does not pollute stdout +- **WHEN** `OPENSPEC_TELEMETRY_DEBUG=1` is set and a command runs with `--json` +- **THEN** stdout still contains exactly one valid JSON document + +#### Scenario: Debug mode for an opted-out user +- **WHEN** `OPENSPEC_TELEMETRY_DEBUG=1` is set and telemetry is disabled +- **THEN** the payloads are printed with a line stating telemetry is off and nothing was sent +- **AND** no network request is made + +#### Scenario: Debug mode creates no identity +- **WHEN** `OPENSPEC_TELEMETRY_DEBUG=1` is set on a machine with no anonymous id +- **THEN** the printed payload shows a placeholder id +- **AND** no anonymous id is written to the global config + +### Requirement: Data subject controls +The system SHALL expose the telemetry state through `openspec config get telemetry`, showing whether telemetry is enabled, the anonymous id if one exists, and the path to the file holding it. + +Deleting the anonymous id from the global config SHALL be sufficient to sever all future events from all prior ones, and the disclosure SHALL say so. + +The public disclosure SHALL name a route for a deletion request that the project actually operates, stating that a request is made by sending the anonymous id, and SHALL lead with the fact that deleting the id locally severs all future events from all prior ones without asking anyone. + +The disclosure SHALL NOT publish a retention period until one is configured in the analytics backend. An unset retention published as fact is the same defect as an unmonitored contact address: a promise the system cannot keep, which costs more trust than saying nothing would. + +The disclosure SHALL describe the data as pseudonymous rather than anonymous. A persistent random identifier combined with device characteristics is pseudonymous personal data; describing it as anonymous overstates the guarantee, and the overstatement is what a reader would hold against every other claim on the page. + +The disclosure SHALL state that the anonymous id identifies a configuration directory rather than a person — a shared home directory means one id spans several people, and a fresh container per run means a new id each time — and SHALL NOT present a count of ids as a count of users. + +#### Scenario: User inspects telemetry state +- **WHEN** a user runs `openspec config get telemetry` +- **THEN** the output shows the enabled state, the anonymous id, and the config file path + +#### Scenario: User severs their history +- **WHEN** a user deletes the anonymous id from the global config +- **THEN** the next event uses a newly generated id unrelated to the previous one + +### Requirement: Ingest handling of network-level identifiers +Every event SHALL set `$ip: null` and `$geoip_disable: true`, so neither the connecting address nor anything derived from it is recorded. + +The disclosure SHALL describe only what the shipped code guarantees. Events reach a first-party endpoint that terminates TLS, so it necessarily observes the connecting address in transit, and no payload flag changes that. Claiming the proxy does not log addresses would be asserting an infrastructure fact a reader cannot verify and this repository cannot enforce — so the disclosure SHALL state the two flags and the transit caveat instead. + +The ingest proxy SHOULD additionally be configured not to log client addresses. That is an operational commitment, not a property of this code, and the disclosure SHALL NOT present it as one. + +Event timestamps SHALL be UTC and SHALL carry no local UTC offset, which combined with the rest of the context would locate the user. + +#### Scenario: Every event suppresses address and location +- **WHEN** the system sends any telemetry event +- **THEN** the payload carries `$ip: null` and `$geoip_disable: true` + +#### Scenario: The disclosure does not claim an infrastructure fact +- **WHEN** the disclosure describes IP handling +- **THEN** it states the two payload flags and that the endpoint sees the address in transit +- **AND** does not assert that the proxy keeps no logs + +#### Scenario: No derived location +- **WHEN** an event is stored +- **THEN** no property derived from the connecting address is attached to it + +### Requirement: Public disclosure parity +The public disclosure SHALL enumerate every event, every property, and every persisted field the system uses. A change that adds, removes, or renames any of these SHALL update the disclosure in the same change. + +The disclosure lives in `README.md`, `SECURITY.md`, and the environment-variable reference. Each SHALL state the full property list, the opt-out mechanisms, the deletion route, and `OPENSPEC_TELEMETRY_DEBUG=1` as the way to verify the list locally. + +The property allowlist constant SHALL be the source the disclosure is checked against, and a test SHALL fail when a property exists in the allowlist that is absent from the disclosure documents. An unenforced documentation requirement decays within two releases. + +Where a change narrows or removes an existing published privacy commitment, it SHALL record the removal in the changelog under a `Privacy` heading, naming the previous commitment and what replaces it. Editing the commitment without that record SHALL NOT satisfy this requirement. + +This change is itself an instance: `SECURITY.md` currently promises "no environment," and `README.md` promises "only command names and version." Adding platform, Node major, and install kind ends both commitments, and that has to be stated rather than quietly edited. + +#### Scenario: Disclosure matches the code +- **WHEN** the event property allowlist changes +- **THEN** the disclosure documents are updated in the same change +- **AND** a test fails if any allowlisted property is undocumented + +#### Scenario: An existing commitment is narrowed +- **WHEN** a change makes a previously published privacy commitment untrue +- **THEN** the changelog records the previous commitment and what replaces it under a `Privacy` heading + +#### Scenario: Disclosure names the verification path +- **WHEN** a user reads the telemetry disclosure +- **THEN** it tells them how to print the events locally rather than asking them to take the list on trust + +## MODIFIED Requirements + +### Requirement: Privacy-preserving event design +The property allowlist is authoritative; the exclusions below are illustrative of what it already forbids. A blocklist fails on the item nobody thought of, which is why the allowlist exists. + +The system SHALL NOT include command arguments, file paths, project names, spec content, error messages, or IP addresses in telemetry events. + +The system SHALL additionally exclude: change ids, spec ids, schema ids, artifact ids, store ids, store remotes, store branches, store local paths, `defaultStore`, `featureFlags` keys, `openers` content, environment variable names and values, hostnames, usernames, and git remotes. + +Every event name, property key, and property value SHALL satisfy the bounded property contract. + +#### Scenario: Command with arguments +- **WHEN** a user runs `openspec init my-project --force` +- **THEN** the telemetry event contains only allowlisted properties and no argument values + +#### Scenario: IP address exclusion +- **WHEN** the system sends a telemetry event +- **THEN** the event explicitly sets `$ip: null` to prevent IP tracking +- **AND** sets `$geoip_disable: true` so no location is derived from the connecting address + +#### Scenario: Named item in a command +- **WHEN** a user runs `openspec archive acme-billing-rewrite` +- **THEN** no event property contains `acme-billing-rewrite` + +#### Scenario: Store-backed run +- **WHEN** a command runs against a store whose remote is a private git URL +- **THEN** no event property contains the store id, the remote, or any path + +### Requirement: Command execution tracking +The system SHALL send a `command_executed` event to PostHog when any CLI command executes, including the command name, OpenSpec version, surface, `run_id`, and `work_session_id` as properties. + +This event is retained despite `command_completed` covering every reachable exit path, because it is the only detector of a run that died so hard the completion hook never ran. If the completion coverage has a gap, only the unmatched pair reveals it — and a systematic blind spot in failure reporting is the exact defect this change exists to remove. + +#### Scenario: Standard command execution +- **WHEN** a user runs any openspec command +- **THEN** the system sends a `command_executed` event with `command`, `version`, `surface`, `run_id`, and `work_session_id` properties + +#### Scenario: Subcommand execution +- **WHEN** a user runs a nested command like `openspec change apply` +- **THEN** the system sends a `command_executed` event with the full command path (e.g., `change:apply`) + +#### Scenario: Unmatched start event +- **WHEN** a `command_executed` event has no matching `command_completed` with the same `run_id` +- **THEN** the gap is attributable to an exit path the completion coverage does not reach + +### 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 the notice state unset, deferring the disclosure to the first later non-JSON run. + +The telemetry config section SHALL record which version of the notice a user has seen. When the disclosed collection scope expands, the system SHALL show a notice naming what changed, once, and record the new notice version. + +The system SHALL NOT reset the seen state to re-notify. Resetting discards the knowledge that the user was told, and shows a generic sentence to someone who already read it, which teaches them to ignore it. A versioned notice distinguishes "never told" from "told about an earlier scope," and lets the new message say what actually changed. + +The notice SHALL NOT describe the data as anonymous without qualification. + +#### Scenario: First command execution +- **WHEN** a user runs their first openspec command without `--json` +- **AND** telemetry is enabled +- **THEN** the system displays the disclosure notice, naming the opt-out + +#### Scenario: Subsequent command execution +- **WHEN** a user has already seen the current notice version +- **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** the notice state 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 + +#### Scenario: Collection scope expands +- **WHEN** the disclosed property list expands in a release +- **AND** a user has seen an earlier notice version +- **THEN** the system displays a notice naming what changed, once +- **AND** records the new notice version diff --git a/openspec/changes/add-command-outcome-telemetry/tasks.md b/openspec/changes/add-command-outcome-telemetry/tasks.md new file mode 100644 index 0000000000..5f8f90ca6e --- /dev/null +++ b/openspec/changes/add-command-outcome-telemetry/tasks.md @@ -0,0 +1,55 @@ +# Tasks + +## 1. Property contract +- [x] 1.1 Add `src/telemetry/properties.ts`: the event-name, property-key, and value allowlists, the error-class union, the diagnostic-code map, and the bucketers — all literal declarations, never computed from a schema +- [x] 1.2 Enforce the allowlist immediately before serialization: drop unknown keys and out-of-set values, send the event regardless +- [x] 1.3 Test: a property key built from a schema, change, or store name is dropped and the event still sends +- [x] 1.4 Test: an unrecognized diagnostic code maps to `other` and never appears in the payload + +## 2. Correlation and outcome +- [x] 2.1 Generate a per-invocation `run_id`; add `work_session_id` with a 30-minute reuse window +- [x] 2.2 Emit `command_completed` from `postAction` with outcome, error class, bucketed exit code, and bucketed duration excluding prompt-blocked time +- [x] 2.3 Classify in `failWithError`/`emitFailure` so `postAction` reads a class, not an error object; unclassified means `internal_error` +- [x] 2.4 Persist and attach `previous_outcome` and `previous_command_same` +- [x] 2.5 Test: success, user error, internal error, and Ctrl-C each produce the expected outcome and class + +## 3. Outcome coverage +- [x] 3.1 Convert the `process.exit()` call sites in `src/cli/index.ts`, `src/core/view.ts`, `src/core/init.ts`, `src/commands/feedback.ts`, and the `config` group guard to set `process.exitCode` and return, or to report and flush where they cannot +- [x] 3.2 Intercept commander's usage errors so unknown commands and bare groups emit `bad_usage`, preserving commander's exit code +- [x] 3.3 Handle an escaped rejection as `internal_error` while preserving existing exit behavior +- [x] 3.4 Ensure a cancelled run never waits on a telemetry request +- [x] 3.5 Assert no telemetry path prompts, blocks, or writes to stdout +- [x] 3.6 Test end to end against the built binary: one `command_completed` per invocation, `--help`/`--version` emit none, exit codes unchanged + +## 4. Run context +- [x] 4.1 Collect the bounded context; count tools rather than naming them; derive `invoker` from a compile-time marker list without sending any env name or value +- [x] 4.2 Bucket the change count from a single non-recursive directory read, discarding names +- [x] 4.3 Cap the invocation at four events +- [x] 4.4 Test: a user-named schema, store, change, and tool set never appear in any payload + +## 5. Milestones and persisted state +- [x] 5.1 Persist the milestone set, first-seen time, reported tool set, work session, and previous outcome; write none of it when telemetry is disabled +- [x] 5.2 Emit `milestone_reached` once per milestone with `version` and `time_to_reach`, omitting the bucket for ids that predate the recorded time +- [x] 5.3 Emit `tool_configured` once per registry tool id, carrying no run context +- [x] 5.4 Test: the milestone fires once, never on failure, and an opted-out run leaves the config untouched +- [x] 5.5 Test: `tool_configured` fires once per tool and carries no context property + +## 6. Inspection and controls +- [x] 6.1 Add `OPENSPEC_TELEMETRY_DEBUG=1` — print payloads to stderr, send nothing, work when opted out, never create an anonymous id +- [x] 6.2 Surface state through `openspec config get telemetry`: enabled, id, file path +- [x] 6.3 Test: debug mode prints, sends nothing, leaves `--json` stdout valid, and writes no config + +## 7. Disclosure +- [x] 7.1 Update `README.md`, `SECURITY.md`, and the environment-variable reference with every event, property, and persisted field, the retention period, the deletion contact, and the debug flag +- [x] 7.2 Replace unqualified "anonymous" with "pseudonymous" in the docs and the notice; state that the id identifies a config directory, not a person +- [x] 7.3 Record the narrowed "no environment" and "only command names and version" commitments in `CHANGELOG.md` under a `Privacy` heading +- [x] 7.4 Add `noticeVersion` and a one-line notice naming what changed for users who saw the earlier scope +- [x] 7.5 Test: an allowlisted property absent from the disclosure documents fails the build + +## 8. Ingest +- [ ] 8.1 Confirm the `edge.openspec.dev` proxy does not log or forward client IPs; disable GeoIP enrichment on the telemetry project +- [ ] 8.2 Publish the retention period and configure it in PostHog + +## 9. Known gaps +- [ ] 9.1 `src/ui/welcome-screen.ts` exits 0 from inside a keypress handler on Ctrl-C, so that cancellation is not reported. Undercounts `cancelled` on the welcome screen only. +- [ ] 9.2 `store_in_use` and `install_kind` are heuristics: a project with both a local root and a store reports `store_in_use: false`, and a checkout outside a recognizable path reports `install_kind: other`. Directionally right, not exact. diff --git a/src/cli/index.ts b/src/cli/index.ts index 75324cfa38..9f649cab0d 100644 --- a/src/cli/index.ts +++ b/src/cli/index.ts @@ -1,5 +1,5 @@ import { asStatus } from '../commands/shared-output.js'; -import { Command, Option } from 'commander'; +import { Command, CommanderError, Option } from 'commander'; import { createRequire } from 'module'; import ora from 'ora'; import path from 'path'; @@ -7,6 +7,7 @@ import { fileURLToPath } from 'url'; import { existsSync, promises as fs } from 'fs'; import { AI_TOOLS, TOOL_ID_ALIASES } from '../core/config.js'; import { UpdateCommand } from '../core/update.js'; +import { InitCancelledError } from '../core/init.js'; import { getAvailableCliUpdate, displayCliUpdateNote, @@ -49,7 +50,26 @@ import { type SchemasOptions, type NewChangeOptions, } from '../commands/workflow/index.js'; -import { maybeShowTelemetryNotice, trackCommand, shutdown } from '../telemetry/index.js'; +import { + isDebugMode, + isTelemetryEnabled, + maybeShowTelemetryNotice, + trackCommand, + shutdown, +} from '../telemetry/index.js'; +import { findLocalRoot, detectSchemaSource } from '../telemetry/context.js'; +import { getConfiguredTools } from '../core/shared/tool-detection.js'; +import { getGlobalConfig } from '../core/global-config.js'; +import { + beginRun, + finishAndFlush, + finishRun, + markInteractiveCapable, + markFailure, + markMilestone, + markOutcome, + registerAllowlists, +} from '../telemetry/cli-runtime.js'; import { maybeShowCompletionTip } from '../core/completion-tip.js'; import { COMMON_FLAGS } from '../core/completions/shared-flags.js'; import { isInteractive } from '../utils/interactive.js'; @@ -71,6 +91,9 @@ function failWithError( error: unknown, json?: { enabled: boolean | undefined; payload?: Record; fallbackCode?: string } ): void { + // Every command's catch funnels through here, so classifying once covers a + // command that grows a new error path without touching that command. + markOutcome(error); // The agent contract: every --json failure leaves exactly one JSON // document on stdout (the command's null-shape plus a status array). if (json?.enabled) { @@ -114,7 +137,10 @@ export function getCommandPath(command: Command): string { current = current.parent; } - return names.join(':') || 'openspec'; + // 'unknown', not 'openspec': the root has no action handler, and the + // allowlist is built from its children, so 'openspec' would fail closed and + // silently drop the property instead of reporting an unattributed run. + return names.join(':') || 'unknown'; } /** @@ -179,6 +205,8 @@ program.hook('preAction', async (thisCommand, actionCommand) => { process.env.NO_COLOR = '1'; } + beginRun(version); + // Show first-run telemetry notice (if not seen). It's written to stderr, so it // never pollutes stdout — but --json runs still defer it (see isJsonRun) so the // very first invocation stays free of any incidental output on either stream. @@ -186,12 +214,47 @@ program.hook('preAction', async (thisCommand, actionCommand) => { // Track command execution (use actionCommand to get the actual subcommand) const commandPath = getCommandPath(actionCommand); + markMilestone(commandPath); await trackCommand(commandPath, version); }); // Shutdown telemetry after command completes program.hook('postAction', async (_thisCommand, actionCommand) => { + // Before the completions tip: the tip writes to the screen and can throw, + // and the outcome must be recorded either way. + try { + // Resolved here rather than inside telemetry so the collector stays a pure + // function of its input, and so a failure resolving context cannot reach + // the command that already did its work. + // Resolving context touches the filesystem, so an opted-out user must not + // pay for it. Checked here rather than inside the collector so the cost is + // skipped, not just the send. + if (!isTelemetryEnabled() && !isDebugMode()) { + return; + } + + const localRoot = findLocalRoot(); + markInteractiveCapable(isInteractive() && Boolean(process.stdout.isTTY)); + + await finishRun({ + command: getCommandPath(actionCommand), + version, + exitCode: process.exitCode === undefined ? 0 : Number(process.exitCode), + jsonMode: isJsonRun(actionCommand), + projectRoot: localRoot, + installDir: getInstallDir(), + // No local root but a store configured means this run resolved through + // one. The store's id, remote, and path are never read, let alone sent. + storeInUse: + localRoot === null && Boolean(getGlobalConfig().defaultStore), + schemaSource: detectSchemaSource(localRoot), + toolIds: localRoot ? safeConfiguredTools(path.dirname(localRoot)) : undefined, + }); + } catch { + // Telemetry never breaks a command that already did its work. + } + // Show the first-run shell-completions tip (on stderr, so piped stdout stays // clean). postAction, not preAction: the tip trails the command's own output // instead of pushing an error message or `init`'s setup summary down the @@ -259,8 +322,18 @@ program }); await initCommand.execute(targetPath); } catch (error) { + // Declining the legacy cleanup ends the command without an error banner: + // it was the user's answer, and it already printed its own message. + if (error instanceof InitCancelledError) { + markFailure('cancelled', 'cancelled'); + process.exitCode = 0; + return; + } failWithError(error); - process.exit(1); + // failWithError already set exitCode 1. Returning instead of exiting + // lets commander run postAction, which reports the failure and flushes; + // process.exit() here would drop both. + return; } }); @@ -281,7 +354,10 @@ program await initCommand.execute('.'); } catch (error) { failWithError(error); - process.exit(1); + // failWithError already set exitCode 1. Returning instead of exiting + // lets commander run postAction, which reports the failure and flushes; + // process.exit() here would drop both. + return; } }); @@ -346,7 +422,10 @@ program } } catch (error) { failWithError(error); - process.exit(1); + // failWithError already set exitCode 1. Returning instead of exiting + // lets commander run postAction, which reports the failure and flushes; + // process.exit() here would drop both. + return; } }); @@ -385,7 +464,10 @@ program payload: options?.specs ? { specs: [], root: null } : { changes: [], root: null }, fallbackCode: 'list_error', }); - process.exit(1); + // failWithError already set exitCode 1. Returning instead of exiting + // lets commander run postAction, which reports the failure and flushes; + // process.exit() here would drop both. + return; } }); @@ -407,7 +489,10 @@ program await viewCommand.execute(root.path); } catch (error) { failWithError(error); - process.exit(1); + // failWithError already set exitCode 1. Returning instead of exiting + // lets commander run postAction, which reports the failure and flushes; + // process.exit() here would drop both. + return; } }); @@ -491,7 +576,10 @@ program await archiveCommand.execute(changeName, options); } catch (error) { failWithError(error); - process.exit(1); + // failWithError already set exitCode 1. Returning instead of exiting + // lets commander run postAction, which reports the failure and flushes; + // process.exit() here would drop both. + return; } }); @@ -525,7 +613,10 @@ program await validateCommand.execute(itemName, options); } catch (error) { failWithError(error, { enabled: options?.json, fallbackCode: 'validate_error' }); - process.exit(1); + // failWithError already set exitCode 1. Returning instead of exiting + // lets commander run postAction, which reports the failure and flushes; + // process.exit() here would drop both. + return; } }); @@ -556,7 +647,10 @@ program await showCommand.execute(itemName, options ?? {}); } catch (error) { failWithError(error, { enabled: options?.json, fallbackCode: 'show_error' }); - process.exit(1); + // failWithError already set exitCode 1. Returning instead of exiting + // lets commander run postAction, which reports the failure and flushes; + // process.exit() here would drop both. + return; } }); @@ -571,7 +665,10 @@ program await feedbackCommand.execute(message, options); } catch (error) { failWithError(error); - process.exit(1); + // failWithError already set exitCode 1. Returning instead of exiting + // lets commander run postAction, which reports the failure and flushes; + // process.exit() here would drop both. + return; } }); @@ -589,7 +686,10 @@ completionCmd await completionCommand.generate({ shell }); } catch (error) { failWithError(error); - process.exit(1); + // failWithError already set exitCode 1. Returning instead of exiting + // lets commander run postAction, which reports the failure and flushes; + // process.exit() here would drop both. + return; } }); @@ -603,7 +703,10 @@ completionCmd await completionCommand.install({ shell, verbose: options?.verbose }); } catch (error) { failWithError(error); - process.exit(1); + // failWithError already set exitCode 1. Returning instead of exiting + // lets commander run postAction, which reports the failure and flushes; + // process.exit() here would drop both. + return; } }); @@ -617,7 +720,10 @@ completionCmd await completionCommand.uninstall({ shell, yes: options?.yes }); } catch (error) { failWithError(error); - process.exit(1); + // failWithError already set exitCode 1. Returning instead of exiting + // lets commander run postAction, which reports the failure and flushes; + // process.exit() here would drop both. + return; } }); @@ -660,7 +766,10 @@ program payload: options.all ? BATCH_STATUS_FAILURE_PAYLOAD : undefined, fallbackCode: 'change_error', }); - process.exit(1); + // failWithError already set exitCode 1. Returning instead of exiting + // lets commander run postAction, which reports the failure and flushes; + // process.exit() here would drop both. + return; } }); @@ -685,7 +794,10 @@ program } } catch (error) { failWithError(error, { enabled: options.json, fallbackCode: 'change_error' }); - process.exit(1); + // failWithError already set exitCode 1. Returning instead of exiting + // lets commander run postAction, which reports the failure and flushes; + // process.exit() here would drop both. + return; } }); @@ -700,7 +812,10 @@ program await templatesCommand(options); } catch (error) { failWithError(error); - process.exit(1); + // failWithError already set exitCode 1. Returning instead of exiting + // lets commander run postAction, which reports the failure and flushes; + // process.exit() here would drop both. + return; } }); @@ -720,7 +835,10 @@ program payload: { schemas: [], root: null }, fallbackCode: 'schemas_error', }); - process.exit(1); + // failWithError already set exitCode 1. Returning instead of exiting + // lets commander run postAction, which reports the failure and flushes; + // process.exit() here would drop both. + return; } }); @@ -745,14 +863,120 @@ newCmd await newChangeCommand(name, options); } catch (error) { failWithError(error); - process.exit(1); + // failWithError already set exitCode 1. Returning instead of exiting + // lets commander run postAction, which reports the failure and flushes; + // process.exit() here would drop both. + return; } }); export { program }; +/** + * Configured tool ids, or undefined if detection is unavailable. + * + * Detection touches the filesystem, so it must never be the reason a command + * that already succeeded reports nothing. + */ +function safeConfiguredTools(projectPath: string): string[] | undefined { + try { + return getConfiguredTools(projectPath); + } catch { + return undefined; + } +} + +/** + * Report a run that exits outside the normal hook path, then flush. + * + * `minimal` skips context collection: these paths are already exiting, and the + * outcome is the part that matters. + */ +function reportOutOfBandExit(command: string, exitCode: number): Promise { + return finishAndFlush({ command, version, exitCode, jsonMode: false, minimal: true }); +} + export function runCli(argv = process.argv): void { - program.parse(argv); + // Teach the allowlist which command paths and tool ids are real, so the + // property contract can reject anything else without importing the registry. + registerAllowlists(program, AI_TOOLS.map((tool) => tool.value)); + + // Commander's own usage errors — unknown command, unknown option, missing + // argument, and a group invoked with no subcommand — call process.exit() + // before the preAction hook has run, so today they produce no telemetry at + // all. They are also the clearest signal that someone could not find the + // command they wanted, which is exactly what we want to see. exitOverride + // turns them into a throw we can report on before exiting ourselves. + // Installed here, not at module scope: importing this module (the tests and + // any library consumer do) must not add a process-wide handler that exits. + installRejectionHandler(); + + program.exitOverride(); + for (const command of collectCommands(program)) { + command.exitOverride(); + } + + try { + program.parse(argv); + } catch (error) { + // Only commander's own errors are usage errors. Anything else is a real + // crash during parsing: rethrowing keeps Node's message and stack, which + // swallowing would have hidden while also filing our bug as a user error. + if (!(error instanceof CommanderError)) { + throw error; + } + + const code = error.exitCode ?? 1; + + // Help and version are not commands and are not failures. `commander.help` + // covers `openspec help [cmd]`; the same code is raised with exit 1 when a + // group is invoked with no subcommand, which *is* a usage error. + const isHelpOrVersion = + error.code === 'commander.helpDisplayed' || + error.code === 'commander.version' || + (error.code === 'commander.help' && code === 0); + if (isHelpOrVersion) { + process.exitCode = code === 0 ? undefined : code; + return; + } + + markFailure('bad_usage'); + process.exitCode = code; + void reportOutOfBandExit('unknown', code).catch(() => { + // A telemetry failure must not change the exit code commander chose. + }); + } +} + +/** Every registered command, depth-first. */ +function collectCommands(root: Command): Command[] { + const found: Command[] = []; + const walk = (command: Command): void => { + for (const child of command.commands) { + found.push(child); + walk(child); + } + }; + walk(root); + return found; +} + +/** + * An error escaping a command's own handling. Commander chains its hooks + * without a catch, so the rejection skips postAction and would otherwise land + * nowhere — making our own bugs the one failure class we never see. + */ +function installRejectionHandler(): void { + process.on('unhandledRejection', (reason) => { + markOutcome(reason); + process.exitCode = 1; + // Print first, then flush. Node printed this immediately, and delaying a + // crash message behind a network flush is a regression the user would feel. + console.error(reason); + void reportOutOfBandExit('unknown', 1).finally(() => { + process.exit(1); + }); + }); } if (process.argv[1] && path.resolve(process.argv[1]) === fileURLToPath(import.meta.url)) { diff --git a/src/commands/change.ts b/src/commands/change.ts index 8cd23b3030..1472294283 100644 --- a/src/commands/change.ts +++ b/src/commands/change.ts @@ -21,6 +21,7 @@ import { diffRequirementBlock, buildRenameMap, } from '../utils/requirement-diff.js'; +import { loadPrompts } from '../utils/prompt-module.js'; /** * True only when `target` is definitively absent. An EACCES or I/O failure @@ -94,7 +95,7 @@ export class ChangeCommand { // Offer exactly the changes `show ` can resolve. const changes = await getActiveChangeIds(this.rootPath ?? process.cwd()); if (canPrompt && changes.length > 0) { - const { select } = await import('@inquirer/prompts'); + const { select } = await loadPrompts(); const selected = await select({ message: 'Select a change to show', choices: changes.map(id => ({ name: id, value: id })), @@ -502,7 +503,7 @@ export class ChangeCommand { const canPrompt = isInteractive(options); const changes = await getActiveChangeIds(); if (canPrompt && changes.length > 0) { - const { select } = await import('@inquirer/prompts'); + const { select } = await loadPrompts(); const selected = await select({ message: 'Select a change to validate', choices: changes.map(id => ({ name: id, value: id })), diff --git a/src/commands/completion.ts b/src/commands/completion.ts index a0487e5740..c7dde865e3 100644 --- a/src/commands/completion.ts +++ b/src/commands/completion.ts @@ -4,6 +4,7 @@ import { COMMAND_REGISTRY } from '../core/completions/command-registry.js'; import { detectShell, SupportedShell } from '../utils/shell-detection.js'; import { CompletionProvider } from '../core/completions/completion-provider.js'; import { getArchivedChangeIds } from '../utils/item-discovery.js'; +import { loadPrompts } from '../utils/prompt-module.js'; interface GenerateOptions { shell?: string; @@ -212,7 +213,7 @@ export class CompletionCommand { // Prompt for confirmation unless --yes flag is provided if (!skipConfirmation) { - const { confirm } = await import('@inquirer/prompts'); + const { confirm } = await loadPrompts(); // Get shell-specific config file path const configPaths: Record = { diff --git a/src/commands/config.ts b/src/commands/config.ts index 2c93a1a56a..0de6150c7c 100644 --- a/src/commands/config.ts +++ b/src/commands/config.ts @@ -25,6 +25,10 @@ import { OPENSPEC_DIR_NAME } from '../core/config.js'; import { hasProjectConfigDrift } from '../core/profile-sync-drift.js'; import { UpdateCommand } from '../core/update.js'; import { asErrorMessage, isPromptCancellationError } from './shared-output.js'; +import { isTelemetryEnabled } from '../telemetry/index.js'; +import { finishAndFlush, getRunVersion, markFailure } from '../telemetry/cli-runtime.js'; +import { getConfigPath } from '../telemetry/config.js'; +import { loadPrompts } from '../utils/prompt-module.js'; type ProfileAction = 'both' | 'delivery' | 'workflows' | 'keep'; @@ -216,10 +220,21 @@ export function registerConfigCommand(program: Command): void { .command('config') .description('View and modify global OpenSpec configuration') .option('--scope ', 'Config scope (only "global" supported currently)') - .hook('preAction', (thisCommand) => { + .hook('preAction', async (thisCommand) => { const opts = thisCommand.opts(); if (opts.scope && opts.scope !== 'global') { console.error('Error: Project-local config is not yet implemented'); + // This guard must stop the command, so it exits rather than returning. + // The root preAction has already run, so the outcome is reported and + // flushed first — otherwise the run starts and never finishes. + markFailure('bad_usage'); + await finishAndFlush({ + command: 'config', + version: getRunVersion(), + exitCode: 1, + jsonMode: false, + minimal: true, + }); process.exit(1); } }); @@ -278,6 +293,21 @@ export function registerConfigCommand(program: Command): void { .description('Get a specific value (raw, scriptable)') .action((key: string) => { const config = getGlobalConfig(); + + // `telemetry` on its own is the data-subject view: what is collected + // about this machine, and where it lives. Still one JSON document on + // stdout, so it stays scriptable; `telemetry.enabled` is unaffected. + if (key === 'telemetry') { + console.log( + JSON.stringify({ + ...(config.telemetry ?? {}), + enabled: isTelemetryEnabled(), + configPath: getConfigPath(), + }) + ); + return; + } + const value = getNestedValue(config as Record, key); if (value === undefined) { @@ -369,7 +399,7 @@ export function registerConfigCommand(program: Command): void { } if (!options.yes) { - const { confirm } = await import('@inquirer/prompts'); + const { confirm } = await loadPrompts(); let confirmed: boolean; try { confirmed = await confirm({ @@ -488,7 +518,7 @@ export function registerConfigCommand(program: Command): void { } // Interactive picker - const { select, checkbox, confirm } = await import('@inquirer/prompts'); + const { select, checkbox, confirm } = await loadPrompts(); const chalk = (await import('chalk')).default; try { diff --git a/src/commands/feedback.ts b/src/commands/feedback.ts index ada7119154..54c74b0a20 100644 --- a/src/commands/feedback.ts +++ b/src/commands/feedback.ts @@ -1,6 +1,7 @@ import { execSync, execFileSync } from 'child_process'; import { createRequire } from 'module'; import os from 'os'; +import { markFailure } from '../telemetry/cli-runtime.js'; const require = createRequire(import.meta.url); const MAX_TITLE_LENGTH = 72; @@ -178,8 +179,11 @@ function reportGhFailure(error: any, title: string, body: string): void { console.log('Please submit your feedback manually:'); console.log(manualUrl); - // Exit with the same code as gh CLI - process.exit(error.status ?? 1); + // exitCode, not exit(): exiting skips commander's postAction hook, so this + // failure would never be reported or flushed. The code is preserved exactly, + // including gh's own non-standard statuses. + markFailure('external_tool_failed'); + process.exitCode = error.status ?? 1; } /** @@ -263,8 +267,9 @@ function handleFallback(title: string, body: string, reason: 'missing' | 'unauth console.log('\nTo auto-submit in the future: gh auth login'); } - // Exit with success code (fallback is successful) - process.exit(0); + // The manual fallback is a success. Left to exit naturally so the completion + // hook still runs. + process.exitCode = 0; } /** diff --git a/src/commands/schema.ts b/src/commands/schema.ts index c05d2956fe..a96ea4dc61 100644 --- a/src/commands/schema.ts +++ b/src/commands/schema.ts @@ -16,6 +16,7 @@ import { parseSchema, SchemaValidationError } from '../core/artifact-graph/schem import type { SchemaYaml, Artifact } from '../core/artifact-graph/types.js'; import { resolveConfigFilePath } from '../core/project-config.js'; import { FileSystemUtils } from '../utils/file-system.js'; +import { loadPrompts } from '../utils/prompt-module.js'; /** * Schema source location type @@ -1084,7 +1085,7 @@ export function registerSchemaCommand(program: Command): void { if (isInteractive) { // Interactive mode - const { input, checkbox, confirm } = await import('@inquirer/prompts'); + const { input, checkbox, confirm } = await loadPrompts(); description = await input({ message: 'Schema description:', diff --git a/src/commands/shared-output.ts b/src/commands/shared-output.ts index 56fbb1ea20..15ad90f45a 100644 --- a/src/commands/shared-output.ts +++ b/src/commands/shared-output.ts @@ -5,6 +5,7 @@ * array in JSON mode. */ import { StoreError, type StoreDiagnostic } from '../core/store/errors.js'; +import { markOutcome } from '../telemetry/cli-runtime.js'; export function printJson(payload: unknown): void { console.log(JSON.stringify(payload, null, 2)); @@ -50,6 +51,11 @@ export function emitFailure( error: unknown, fallbackCode: string ): void { + // The other shared failure seam. Most commands set process.exitCode here + // rather than throwing to the CLI's catch, so without this the whole + // command layer's failures arrive unclassified and get filed as our bugs. + markOutcome(error); + // Ctrl-C in a prompt is the user's choice, not an error: every // command group gets the Cancelled./130 convention through here. if (!json && isPromptCancellationError(error)) { diff --git a/src/commands/show.ts b/src/commands/show.ts index c452229f4c..e15c3ca703 100644 --- a/src/commands/show.ts +++ b/src/commands/show.ts @@ -11,6 +11,7 @@ import { import { ChangeCommand } from './change.js'; import { SpecCommand } from './spec.js'; import { nearestMatches } from '../utils/match.js'; +import { loadPrompts } from '../utils/prompt-module.js'; type ItemType = 'change' | 'spec'; @@ -38,7 +39,7 @@ export class ShowCommand { if (!itemName) { if (interactive) { - const { select } = await import('@inquirer/prompts'); + const { select } = await loadPrompts(); const type = await select({ message: 'What would you like to show?', choices: [ @@ -76,7 +77,7 @@ export class ShowCommand { options: ShowExecuteOptions, root: ResolvedOpenSpecRoot ): Promise { - const { select } = await import('@inquirer/prompts'); + const { select } = await loadPrompts(); if (type === 'change') { const changes = await getActiveChangeIds(root.path); if (changes.length === 0) { diff --git a/src/commands/spec.ts b/src/commands/spec.ts index e459342db5..a2e13de505 100644 --- a/src/commands/spec.ts +++ b/src/commands/spec.ts @@ -9,6 +9,8 @@ import { isInteractive } from '../utils/interactive.js'; import { getSpecIds } from '../utils/item-discovery.js'; import { discoverSpecFiles } from '../utils/spec-discovery.js'; import { FileSystemUtils } from '../utils/file-system.js'; +import { loadPrompts } from '../utils/prompt-module.js'; +import { markCheckFailed } from '../telemetry/cli-runtime.js'; const SPECS_DIR = 'openspec/specs'; @@ -106,7 +108,7 @@ export class SpecCommand { const canPrompt = isInteractive(options); const specIds = await getSpecIds(this.rootPath ?? process.cwd()); if (canPrompt && specIds.length > 0) { - const { select } = await import('@inquirer/prompts'); + const { select } = await loadPrompts(); specId = await select({ message: 'Select a spec to show', choices: specIds.map(id => ({ name: id, value: id })), @@ -242,7 +244,7 @@ export function registerSpecCommand(rootProgram: typeof program) { const canPrompt = isInteractive(options); const specIds = await getSpecIds(); if (canPrompt && specIds.length > 0) { - const { select } = await import('@inquirer/prompts'); + const { select } = await loadPrompts(); specId = await select({ message: 'Select a spec to validate', choices: specIds.map(id => ({ name: id, value: id })), @@ -277,6 +279,7 @@ export function registerSpecCommand(rootProgram: typeof program) { }); } } + if (!report.valid) markCheckFailed(); process.exitCode = report.valid ? 0 : 1; } catch (error) { console.error(`Error: ${error instanceof Error ? error.message : 'Unknown error'}`); diff --git a/src/commands/store.ts b/src/commands/store.ts index 1a91d89984..0692bb2673 100644 --- a/src/commands/store.ts +++ b/src/commands/store.ts @@ -27,6 +27,7 @@ import { type SetupStoreInput, } from '../core/store/index.js'; import { isInteractive } from '../utils/interactive.js'; +import { loadPrompts } from '../utils/prompt-module.js'; interface StoreSetupOptions { path?: string; @@ -223,7 +224,7 @@ function formatPathForHuman(targetPath: string): string { } async function promptStoreId(): Promise { - const { input } = await import('@inquirer/prompts'); + const { input } = await loadPrompts(); return input({ message: 'Store name', @@ -240,7 +241,7 @@ async function promptStoreId(): Promise { } async function promptStorePath(id: string): Promise { - const { input } = await import('@inquirer/prompts'); + const { input } = await loadPrompts(); // Suggest a visible, user-owned location — never the managed XDG data dir. const defaultPath = ['~', 'openspec', id].join('/'); @@ -303,7 +304,7 @@ async function confirmSetup( prepared: Awaited>, initGit: boolean ): Promise { - const { confirm } = await import('@inquirer/prompts'); + const { confirm } = await loadPrompts(); console.log(''); console.log('OpenSpec will create:'); @@ -344,7 +345,7 @@ async function confirmRemove(id: string, root: string, options: StoreRemoveOptio ); } - const { confirm } = await import('@inquirer/prompts'); + const { confirm } = await loadPrompts(); const confirmed = await confirm({ message: `Delete local store folder ${formatPathForHuman(root)}?`, default: false, @@ -370,7 +371,7 @@ function isRegisterIdentityConfirmationError(error: unknown): boolean { } async function confirmRegisterConversion(error: unknown): Promise { - const { confirm } = await import('@inquirer/prompts'); + const { confirm } = await loadPrompts(); const confirmed = await confirm({ message: asErrorMessage(error), default: false, diff --git a/src/commands/validate.ts b/src/commands/validate.ts index 70b3d135e7..987af1d2a0 100644 --- a/src/commands/validate.ts +++ b/src/commands/validate.ts @@ -16,6 +16,8 @@ import { nearestMatches } from '../utils/match.js'; import { promises as fs } from 'fs'; import { getTaskProgressDetailForChange, type SchemaGlobCache } from '../utils/task-progress.js'; import { FileSystemUtils } from '../utils/file-system.js'; +import { loadPrompts } from '../utils/prompt-module.js'; +import { markCheckFailed } from '../telemetry/cli-runtime.js'; type ItemType = 'change' | 'spec'; @@ -170,7 +172,7 @@ export class ValidateCommand { } private async runInteractiveSelector(root: ResolvedOpenSpecRoot, opts: { strict: boolean; json: boolean; concurrency?: string }): Promise { - const { select } = await import('@inquirer/prompts'); + const { select } = await loadPrompts(); const choice = await select({ message: 'What would you like to validate?', choices: [ @@ -282,6 +284,7 @@ export class ValidateCommand { const durationMs = Date.now() - start; this.printReport('change', id, report, durationMs, opts.json, root); // Non-zero exit if invalid (keeps enriched output test semantics) + if (!report.valid) markCheckFailed(); process.exitCode = report.valid ? 0 : 1; return; } @@ -290,6 +293,7 @@ export class ValidateCommand { const report = await validator.validateSpec(file); const durationMs = Date.now() - start; this.printReport('spec', id, report, durationMs, opts.json, root); + if (!report.valid) markCheckFailed(); process.exitCode = report.valid ? 0 : 1; } @@ -497,6 +501,8 @@ export class ValidateCommand { this.printBulkDetails(results, root); } + if (failed > 0) markCheckFailed(); + process.exitCode = failed > 0 ? 1 : 0; } @@ -596,6 +602,7 @@ export class ValidateCommand { if (opts.findingsScope) { this.printFindingsReport({ items: results, summary, root: toRootOutput(root) }, opts.findingsScope, opts.json, root); + if (failed > 0) markCheckFailed(); process.exitCode = failed > 0 ? 1 : 0; return; } @@ -603,6 +610,7 @@ export class ValidateCommand { if (opts.json) { const out = { items: results, summary, version: '1.0', root: toRootOutput(root) }; console.log(JSON.stringify(out, null, 2)); + if (failed > 0) markCheckFailed(); process.exitCode = failed > 0 ? 1 : 0; return; } @@ -627,6 +635,7 @@ export class ValidateCommand { } } console.log(`Totals: ${summary.totals.passed} passed, ${summary.totals.failed} failed (${summary.totals.items} items)`); + if (failed > 0) markCheckFailed(); process.exitCode = failed > 0 ? 1 : 0; } } diff --git a/src/commands/workset-prompts.ts b/src/commands/workset-prompts.ts index 95c9247b99..55ee40324f 100644 --- a/src/commands/workset-prompts.ts +++ b/src/commands/workset-prompts.ts @@ -25,6 +25,7 @@ import { formatMemberRows, resolveMemberFlags, } from './workset-input.js'; +import { loadPrompts } from '../utils/prompt-module.js'; export interface ComposeInput { memberFlags: string[]; @@ -36,7 +37,7 @@ export async function composeInteractively( input: ComposeInput, table: OpenerDefinition[] ): Promise { - const prompts = await import('@inquirer/prompts'); + const prompts = await loadPrompts(); console.log('[1/3] Name the workset'); let name: string; @@ -152,7 +153,7 @@ export async function composeInteractively( export async function promptToolFromChoices( available: OpenerChoice[] ): Promise { - const { select } = await import('@inquirer/prompts'); + const { select } = await loadPrompts(); return select({ message: 'Open with:', choices: available.map((choice) => ({ @@ -163,7 +164,7 @@ export async function promptToolFromChoices( } export async function promptOpenNow(label: string): Promise { - const { confirm } = await import('@inquirer/prompts'); + const { confirm } = await loadPrompts(); return confirm({ message: `Open it now in ${label}?`, default: true, @@ -174,7 +175,7 @@ export async function promptOpenNow(label: string): Promise { export async function confirmRemoveInteractively( workset: Workset ): Promise { - const { confirm } = await import('@inquirer/prompts'); + const { confirm } = await loadPrompts(); console.log(`Workset '${workset.name}':`); for (const row of formatMemberRows(workset.members)) { diff --git a/src/core/archive.ts b/src/core/archive.ts index 888a6135a6..8415d1bd2d 100644 --- a/src/core/archive.ts +++ b/src/core/archive.ts @@ -28,6 +28,7 @@ import { METADATA_FILENAME, readRetireCapabilitiesMarker, readSkipSpecsMarker } import { confirmPrompt, isNonInteractivePromptError } from '../utils/interactive.js'; import { FileSystemUtils } from '../utils/file-system.js'; import { folderStyleNameProblem } from './id.js'; +import { loadPrompts } from '../utils/prompt-module.js'; function isMissingPathError(error: unknown): boolean { return ( @@ -2062,7 +2063,7 @@ export class ArchiveCommand { root: ResolvedOpenSpecRoot, options: ArchiveOptions ): Promise { - const { select } = await import('@inquirer/prompts'); + const { select } = await loadPrompts(); const changeDirs = await listActiveChangeNames(changesDir); if (changeDirs.length === 0) { diff --git a/src/core/global-config.ts b/src/core/global-config.ts index fe1ec797e2..6f03f584e8 100644 --- a/src/core/global-config.ts +++ b/src/core/global-config.ts @@ -19,6 +19,26 @@ export interface TelemetryConfig { anonymousId?: string; /** Whether the first-run telemetry notice has been shown. */ noticeSeen?: boolean; + /** + * Which disclosure version the user has seen. An expansion of what is + * collected shows a notice naming what changed rather than resetting + * noticeSeen, which would discard the fact that they were told at all. + */ + noticeVersion?: number; + /** ISO time of the first run with telemetry enabled. Never sent; only a bucket derived from it is. */ + firstSeenAt?: string; + /** Random id shared by invocations less than 30 minutes apart. */ + workSessionId?: string; + /** ISO time of the last invocation, for the work-session window. */ + lastActivityAt?: string; + /** Milestones already reported, so each is sent at most once. */ + milestones?: string[]; + /** Registry tool ids already reported via tool_configured. */ + reportedTools?: string[]; + /** Outcome of the previous invocation, for retry visibility. */ + previousOutcome?: string; + /** Command path of the previous invocation. Compared locally; never sent. */ + previousCommand?: string; } // TypeScript interfaces diff --git a/src/core/init.ts b/src/core/init.ts index f8fa0773b5..d1fefe4923 100644 --- a/src/core/init.ts +++ b/src/core/init.ts @@ -85,6 +85,20 @@ import { findUnmanagedCloudFiles, listManagedCloudFiles, } from './github-copilot/cloud-agent.js'; +import { loadPrompts } from '../utils/prompt-module.js'; + +/** + * The user declined the legacy cleanup. A cancellation, not an error: it + * carries the diagnostic code the telemetry classifier maps to `cancelled`, + * and the command's own handler prints nothing extra for it. + */ +export class InitCancelledError extends Error { + readonly diagnostic = { severity: 'error' as const, code: 'init_cancelled', message: 'Initialization cancelled.' }; + constructor() { + super('Initialization cancelled.'); + this.name = 'InitCancelledError'; + } +} const require = createRequire(import.meta.url); const { version: OPENSPEC_VERSION } = require('../../package.json'); @@ -423,7 +437,7 @@ export class InitCommand { } if (this.canPromptInteractively()) { - const { confirm } = await import('@inquirer/prompts'); + const { confirm } = await loadPrompts(); const answer = await confirm({ message: 'Set up GitHub Copilot cloud coding-agent files? This is for the GitHub-hosted ' + @@ -506,7 +520,7 @@ export class InitCommand { } // Interactive mode: prompt for confirmation - const { confirm } = await import('@inquirer/prompts'); + const { confirm } = await loadPrompts(); const shouldCleanup = await confirm({ message: 'Upgrade and clean up legacy files?', default: true, @@ -515,7 +529,10 @@ export class InitCommand { if (!shouldCleanup) { console.log(chalk.dim('Initialization cancelled.')); console.log(chalk.dim('Run with --force to skip this prompt, or manually remove legacy files.')); - process.exit(0); + // Declining is the user's choice, not a failure. Throwing a cancellation + // rather than exiting stops the command the same way while letting + // commander's postAction hook run. + throw new InitCancelledError(); } await this.performImmediateLegacyCleanup(projectPath, detection); diff --git a/src/core/update.ts b/src/core/update.ts index 5e44b571c3..52d47a7d22 100644 --- a/src/core/update.ts +++ b/src/core/update.ts @@ -77,6 +77,7 @@ import { writeSharedSkillTarget, } from './shared-skill-target.js'; import { includesGitHubCopilot, writeCopilotCloudFiles, removeCopilotCloudFiles, isCopilotCloudEnabled, readCopilotCloudOptIn, findUnmanagedCloudFiles } from './github-copilot/cloud-agent.js'; +import { loadPrompts } from '../utils/prompt-module.js'; const require = createRequire(import.meta.url); const { version: OPENSPEC_VERSION } = require('../../package.json'); @@ -902,7 +903,7 @@ export class UpdateCommand { console.log(chalk.yellow(legacyMigrationNotice(migration))); if (!this.force && isInteractive()) { - const { confirm } = await import('@inquirer/prompts'); + const { confirm } = await loadPrompts(); let shouldMigrate: boolean; try { shouldMigrate = await confirm({ @@ -1007,7 +1008,7 @@ export class UpdateCommand { } // Interactive mode: prompt for confirmation - const { confirm } = await import('@inquirer/prompts'); + const { confirm } = await loadPrompts(); const shouldCleanup = await confirm({ message: 'Upgrade and clean up legacy files?', default: true, diff --git a/src/core/version-check.ts b/src/core/version-check.ts index fd5b05c564..f245b2d139 100644 --- a/src/core/version-check.ts +++ b/src/core/version-check.ts @@ -6,6 +6,7 @@ import { createRequire } from 'module'; import chalk from 'chalk'; import { isCiEnvironment } from '../utils/ci.js'; import { getGlobalConfig } from './global-config.js'; +import { loadPrompts } from '../utils/prompt-module.js'; const require = createRequire(import.meta.url); const { name: PACKAGE_NAME, version: OPENSPEC_VERSION } = require('../../package.json'); @@ -681,7 +682,7 @@ function isPromptCancellation(error: unknown): boolean { * of silently doing nothing. */ export async function offerCliUpgrade(latestVersion: string): Promise { - const { confirm } = await import('@inquirer/prompts'); + const { confirm } = await loadPrompts(); let accepted = false; try { diff --git a/src/core/view.ts b/src/core/view.ts index e79c1905a7..a7bd0e3cb1 100644 --- a/src/core/view.ts +++ b/src/core/view.ts @@ -11,7 +11,10 @@ export class ViewCommand { if (!fs.existsSync(openspecDir)) { console.error(chalk.red('No openspec directory found')); - process.exit(1); + // exitCode, not exit(): exiting here skips commander's postAction hook, + // so the failure would never be reported or flushed. + process.exitCode = 1; + return; } console.log(chalk.bold('\nOpenSpec Dashboard\n')); diff --git a/src/telemetry/classify.ts b/src/telemetry/classify.ts new file mode 100644 index 0000000000..bb06c19591 --- /dev/null +++ b/src/telemetry/classify.ts @@ -0,0 +1,198 @@ +/** + * Map a failure onto the bounded `error_class` allowlist. + * + * Diagnostic codes are mapped, never passed through. A code is developer- + * authored today, but nothing in the type system guarantees a future one is + * free of user text, and a passthrough would be exactly the hole the property + * contract exists to close. + * + * An unrecognized failure is `other` with outcome `internal_error`, not + * `user_error`. A failure we did not anticipate is our problem until shown + * otherwise; biasing the other way would make the `internal_error` rate + * under-report the thing it exists to surface. + */ +import type { ErrorClass, Outcome } from './properties.js'; + +/** + * Exact diagnostic codes that do not follow their family's prefix rule, or + * whose family maps to more than one class. + */ +const CODE_MAP: Readonly> = { + no_openspec_root: 'no_root', + no_registered_stores: 'no_root', + no_root_with_registered_stores: 'no_root', + unhealthy_root: 'no_root', + unhealthy_store_root: 'no_root', + store_identity_mismatch: 'no_root', + store_path_not_supported: 'no_root', + invalid_store_pointer: 'no_root', + + unknown_item: 'item_not_found', + ambiguous_item: 'ambiguous_item', + store_not_found: 'item_not_found', + unknown_store: 'item_not_found', + workset_not_found: 'item_not_found', + archive_change_not_found: 'item_not_found', + + unknown_subcommand: 'unknown_subcommand', + unknown_store_subcommand: 'unknown_subcommand', + unknown_workset_subcommand: 'unknown_subcommand', + + schema_not_found: 'schema_not_found', + schema_invalid: 'schema_invalid', + + archive_validation_failed: 'validation_failed', + archive_spec_validation_failed: 'validation_failed', + invalid_validation_report_request: 'bad_usage', + + archive_change_name_required: 'not_interactive', + archive_confirmation_required: 'not_interactive', + store_register_identity_confirmation_required: 'not_interactive', + store_remove_confirmation_required: 'not_interactive', + workset_open_json_unsupported: 'not_interactive', + + init_cancelled: 'cancelled', + store_setup_cancelled: 'cancelled', + store_register_cancelled: 'cancelled', + store_remove_cancelled: 'cancelled', + workset_remove_cancelled: 'cancelled', + + store_id_conflict: 'already_exists', + store_already_registered: 'already_exists', + store_path_conflict: 'already_exists', + archive_target_exists: 'already_exists', + workset_exists: 'already_exists', + + store_registry_changed: 'concurrent_modification', + store_registry_busy: 'concurrent_modification', + workset_file_busy: 'concurrent_modification', + store_checkout_drift: 'concurrent_modification', + + store_path_missing: 'fs_error', + store_path_not_directory: 'fs_error', + store_root_missing: 'fs_error', + store_root_not_directory: 'fs_error', + archive_path_outside_root: 'fs_error', + archive_change_symlink: 'fs_error', + + store_metadata_missing: 'metadata_invalid', + store_metadata_invalid: 'metadata_invalid', + store_metadata_id_mismatch: 'metadata_invalid', + invalid_store_metadata: 'metadata_invalid', + invalid_store_registry: 'metadata_invalid', + invalid_workset_file: 'metadata_invalid', + invalid_opener_config: 'metadata_invalid', + + workset_launch_failed: 'external_tool_failed', + workset_tool_unavailable: 'external_tool_failed', + workset_tool_unknown: 'external_tool_failed', + workset_cli_opener_disabled: 'external_tool_failed', +}; + +/** Prefix rules for the families that map uniformly. Order matters. */ +const PREFIX_RULES: ReadonlyArray = [ + ['store_git_', 'git_error'], + ['store_remote_', 'git_error'], + ['store_clone_', 'git_error'], + ['archive_tasks_', 'archive_blocked'], + ['archive_spec_update_', 'archive_blocked'], + ['archive_', 'archive_blocked'], + ['invalid_store_', 'store_error'], + ['invalid_workset_', 'bad_usage'], + ['store_setup_', 'store_error'], + ['store_', 'store_error'], + ['workset_', 'bad_usage'], +]; + +/** Node's own filesystem error codes. */ +const FS_ERRNO = new Set(['EACCES', 'EPERM', 'EISDIR', 'ENOTDIR', 'EROFS', 'EMFILE', 'ENOSPC']); +const NETWORK_ERRNO = new Set([ + 'ENOTFOUND', + 'ECONNREFUSED', + 'ECONNRESET', + 'ETIMEDOUT', + 'EAI_AGAIN', + 'ENETUNREACH', +]); + +function classifyCode(code: string): ErrorClass | undefined { + const mapped = CODE_MAP[code]; + if (mapped) { + return mapped; + } + for (const [prefix, errorClass] of PREFIX_RULES) { + if (code.startsWith(prefix)) { + return errorClass; + } + } + return undefined; +} + +function readDiagnosticCode(error: unknown): string | undefined { + const code = (error as { diagnostic?: { code?: unknown } })?.diagnostic?.code; + return typeof code === 'string' ? code : undefined; +} + +function readErrno(error: unknown): string | undefined { + const code = (error as { code?: unknown })?.code; + return typeof code === 'string' ? code : undefined; +} + +export interface Classification { + outcome: Outcome; + errorClass: ErrorClass; +} + +export function classifyError(error: unknown): Classification { + // Ctrl-C at a prompt is the user's choice, not a failure. Matched the same + // way `isPromptCancellationError` does, without importing it: this module + // stays free of the command layer's import graph. + const name = (error as { name?: unknown })?.name; + const message = error instanceof Error ? error.message : ''; + if (name === 'ExitPromptError' || name === 'AbortPromptError' || message.includes('force closed the prompt')) { + return { outcome: 'cancelled', errorClass: 'cancelled' }; + } + + const diagnosticCode = readDiagnosticCode(error); + if (diagnosticCode) { + const mapped = classifyCode(diagnosticCode); + if (mapped) { + return { + outcome: mapped === 'cancelled' ? 'cancelled' : 'user_error', + errorClass: mapped, + }; + } + // A code we do not recognize is a stale map, not a crash. Separating the + // two keeps `internal_error` meaning "our bug" rather than "our classifier + // fell behind", which is the distinction the metric exists for. The raw + // code never leaves this function either way. + return { outcome: 'user_error', errorClass: 'unclassified' }; + } + + const errno = readErrno(error); + if (errno) { + if (FS_ERRNO.has(errno)) return { outcome: 'user_error', errorClass: 'fs_error' }; + if (NETWORK_ERRNO.has(errno)) return { outcome: 'user_error', errorClass: 'network_error' }; + if (errno === 'ENOENT') return { outcome: 'user_error', errorClass: 'item_not_found' }; + } + + const constructorName = (error as { constructor?: { name?: string } })?.constructor?.name; + switch (constructorName) { + case 'SchemaValidationError': + return { outcome: 'user_error', errorClass: 'schema_invalid' }; + case 'SchemaLoadError': + case 'TemplateLoadError': + return { outcome: 'user_error', errorClass: 'schema_not_found' }; + case 'ChangeMetadataError': + return { outcome: 'user_error', errorClass: 'metadata_invalid' }; + default: + break; + } + + return { outcome: 'internal_error', errorClass: 'other' }; +} + +/** A command that failed a check it ran correctly — never an internal error. */ +export function classifyCheckFailure(errorClass: ErrorClass): Classification { + return { outcome: 'user_error', errorClass }; +} diff --git a/src/telemetry/cli-runtime.ts b/src/telemetry/cli-runtime.ts new file mode 100644 index 0000000000..a7f3673ec1 --- /dev/null +++ b/src/telemetry/cli-runtime.ts @@ -0,0 +1,269 @@ +/** + * Wiring between the CLI and telemetry. + * + * The hard part this module exists for is coverage. Three families of exit + * skip commander's `postAction` hook, and each one hides the runs we most need + * to see: + * + * 1. An action handler calling `process.exit()`. Those sites now set + * `process.exitCode` and return; `markOutcome` records the class on the way + * past so the hook can report it. + * 2. Commander's own usage errors — unknown command, unknown flag, a group + * invoked with no subcommand. These exit before `preAction` ever runs, so + * they produce no event at all today, and they are precisely the "user + * typed the wrong thing" signal. + * 3. An error escaping a command's own handling. Commander chains hooks + * without a `catch`, so the rejection skips the hook and lands nowhere. + * + * Nothing here prompts, blocks, or writes to stdout. + */ +import type { Command } from 'commander'; +import { classifyError } from './classify.js'; +import { collectRunContext } from './context.js'; +import { + isFirstRun, + isTelemetryEnabled, + shutdown, + trackCompletion, + trackConfiguredTools, + trackMilestone, +} from './index.js'; +import { setRegistryChecks, type ErrorClass, type Milestone, type Outcome } from './properties.js'; + +/** Milliseconds spent blocked on an interactive prompt, excluded from duration. */ +let promptedMs = 0; +let promptOpenedAt: number | null = null; +let startedAt: number | null = null; +let completionSent = false; + +let pending: { outcome: Outcome; errorClass: ErrorClass } | null = null; +let earnedMilestone: Milestone | null = null; + +/** Commands whose success is an activation milestone. */ +/** + * Commands whose success is an activation milestone. + * + * `propose` and `apply` are agent workflows, not CLI commands, so the CLI + * observes them through the commands an agent runs on their behalf: + * `new:change` creates the proposal, and `validate` is what an agent runs as + * it works the change. `install` is not a command at all — it is the run that + * mints the anonymous id, so it is emitted from the completion path. + */ +const MILESTONE_COMMANDS: Readonly> = { + init: 'init', + 'new:change': 'propose', + validate: 'apply', + archive: 'archive', +}; + +/** + * The running CLI version, captured once so out-of-band reporters can send it + * without importing package.json — which would form a cycle back through the + * command modules that need to report. + */ +let runVersion = '0.0.0'; + +export function getRunVersion(): string { + return runVersion; +} + +export function beginRun(version = runVersion): void { + runVersion = version; + startedAt = Date.now(); + completionSent = false; + pending = null; + earnedMilestone = null; + promptedMs = 0; + promptOpenedAt = null; + interactiveCapable = false; +} + +/** Called around an interactive prompt so its wait never enters the duration. */ +export function markPromptOpen(): void { + promptOpenedAt = Date.now(); +} + +export function markPromptClosed(): void { + if (promptOpenedAt !== null) { + promptedMs += Date.now() - promptOpenedAt; + promptOpenedAt = null; + } +} + +/** + * True when this run either opened a prompt or could have. + * + * Both halves matter. The measured half excludes think time from the + * duration; the capability half is what latency analysis filters on, because + * a run that *could* have prompted is one whose timing is not comparable to an + * agent's — and agent runs, which never prompt, are the population whose + * latency we actually want to read. + */ +export function wasPrompted(): boolean { + return promptedMs > 0 || promptOpenedAt !== null || interactiveCapable; +} + +let interactiveCapable = false; + +/** Record whether this run could prompt at all (both streams a terminal). */ +export function markInteractiveCapable(capable: boolean): void { + interactiveCapable = capable; +} + +/** + * Record how this run ended, for the completion hook to report. + * + * Called from the shared failure paths rather than from each command, so a + * command that gains a new error path is covered without touching it. + */ +export function markOutcome(error: unknown): void { + if (pending) { + return; // First classification wins; a rethrow must not reclassify. + } + pending = classifyError(error); +} + +/** Record a failure whose class is already known, without an error object. */ +export function markFailure(errorClass: ErrorClass, outcome: Outcome = 'user_error'): void { + if (!pending) { + pending = { outcome, errorClass }; + } +} + +/** A command that ran correctly and reported the content invalid. */ +export function markCheckFailed(): void { + markFailure('validation_failed'); +} + +export function markMilestone(commandPath: string): void { + const milestone = MILESTONE_COMMANDS[commandPath]; + if (milestone) { + earnedMilestone = milestone; + } +} + +/** Teach the property allowlist which commands and tools are registered. */ +export function registerAllowlists(program: Command, toolIds: readonly string[]): void { + const commands = new Set(); + const walk = (command: Command, prefix: string[]): void => { + for (const child of command.commands) { + const name = child.name(); + const path = [...prefix, name]; + commands.add(path.join(':')); + walk(child, path); + } + }; + walk(program, []); + + const tools = new Set(toolIds); + setRegistryChecks({ + isCommand: (value) => commands.has(value), + isTool: (value) => tools.has(value), + }); +} + +export interface CompletionInput { + command: string; + version: string; + exitCode: number | undefined; + jsonMode: boolean; + projectRoot?: string | null; + installDir?: string | null; + storeInUse?: boolean; + schemaSource?: 'package' | 'project' | 'user'; + toolIds?: string[]; + /** Skip the context collection when the exit path cannot afford it. */ + minimal?: boolean; +} + +/** + * Send `command_completed` exactly once for this invocation. + * + * Idempotent by design: several exit paths may reach it, and a duplicate would + * double-count every failure rate built on this event. + */ +export async function finishRun(input: CompletionInput): Promise { + if (completionSent) { + return; + } + completionSent = true; + + const exitCode = input.exitCode; + const failed = exitCode !== undefined && exitCode !== 0; + + let outcome: Outcome; + let errorClass: ErrorClass; + if (pending) { + ({ outcome, errorClass } = pending); + } else if (exitCode === 130) { + outcome = 'cancelled'; + errorClass = 'cancelled'; + } else if (failed) { + // A non-zero exit that reached no classifier. Recorded as `unclassified` + // rather than `internal_error`: the CLI has many paths that set an exit + // code without throwing, so presuming a bug here would drown the + // internal_error rate in ordinary user errors and make it useless for the + // one thing it exists to measure. + outcome = 'user_error'; + errorClass = 'unclassified'; + } else { + outcome = 'success'; + errorClass = 'none'; + } + + const durationMs = Math.max(0, (startedAt ? Date.now() - startedAt : 0) - promptedMs); + + let context: Record | undefined; + if (!input.minimal) { + try { + context = await collectRunContext({ + projectRoot: input.projectRoot, + installDir: input.installDir, + stdoutIsTty: Boolean(process.stdout.isTTY), + jsonMode: input.jsonMode, + prompted: wasPrompted(), + firstRun: isFirstRun(), + storeInUse: Boolean(input.storeInUse), + schemaSource: input.schemaSource, + toolCount: input.toolIds?.length, + }); + } catch { + // Context is decoration on the outcome; losing it never loses the event. + } + } + + await trackCompletion({ + command: input.command, + version: input.version, + outcome, + errorClass, + exitCode, + durationMs, + context, + }); + + // The run that mints the anonymous id is the install: without it the + // activation funnel has no denominator. + if (isFirstRun()) { + await trackMilestone('install', input.version); + } + + if (outcome === 'success' && earnedMilestone) { + await trackMilestone(earnedMilestone, input.version); + } + + if (outcome === 'success' && input.toolIds?.length && isTelemetryEnabled()) { + await trackConfiguredTools(input.toolIds, input.version); + } +} + +/** + * Report a failure that exits before the normal hook can run, then flush. + * + * Used for commander's own usage errors and for an escaped rejection: both + * exit immediately, so the flush has to be explicit. + */ +export async function finishAndFlush(input: CompletionInput): Promise { + await finishRun(input); + await shutdown(); +} diff --git a/src/telemetry/config.ts b/src/telemetry/config.ts index f994cfacd0..a66e09cef0 100644 --- a/src/telemetry/config.ts +++ b/src/telemetry/config.ts @@ -136,11 +136,15 @@ export function getConfigPath(): string { * Read the global config file. * Returns an empty object if the file doesn't exist. */ -export async function readConfig(): Promise { +export async function readConfig(options: { persist?: boolean } = {}): Promise { const configPath = getConfigPath(); const read = await readConfigFile(configPath); const config = read.status === 'ok' ? read.config : {}; - return migrateLegacyTelemetryConfig(configPath, config, read.status !== 'invalid'); + // The one-time legacy migration writes. An inspection-only run must not, + // or reading what telemetry would send materializes the very id being + // inspected at a new path. + const persist = options.persist !== false && read.status !== 'invalid'; + return migrateLegacyTelemetryConfig(configPath, config, persist); } /** @@ -165,8 +169,10 @@ export async function writeConfig(updates: Partial): Promise /** * Get the telemetry config section. */ -export async function getTelemetryConfig(): Promise { - const config = await readConfig(); +export async function getTelemetryConfig( + options: { persist?: boolean } = {} +): Promise { + const config = await readConfig(options); return config.telemetry ?? {}; } diff --git a/src/telemetry/context.ts b/src/telemetry/context.ts new file mode 100644 index 0000000000..4f548eb31e --- /dev/null +++ b/src/telemetry/context.ts @@ -0,0 +1,186 @@ +/** + * Bounded run context for `command_completed`. + * + * Everything here reduces to a member of a list in `properties.ts`. Where a + * value comes from a set the user can extend — a schema they forked, a store + * they named — only the shape of it survives, never the name. + * + * Collection is best-effort by design: a context value that cannot be read + * cheaply is omitted and the event is still sent. The outcome signal is the + * point of the event; the context is decoration on it. + */ +import { promises as fs, statSync } from 'fs'; +import path from 'path'; +import { getGlobalConfig } from '../core/global-config.js'; +import { + bucketCount, + bucketNodeMajor, + bucketPlatform, + bucketToolCount, + type Invoker, +} from './properties.js'; + +/** + * Markers that identify the agent driving this run. Presence only — no + * variable name and no value is ever sent, and anything unlisted collapses to + * `unknown`, so a new agent's marker cannot leak as free text. + */ +const INVOKER_MARKERS: ReadonlyArray = [ + ['claude_code', ['CLAUDECODE', 'CLAUDE_CODE']], + ['cursor', ['CURSOR_TRACE_ID', 'CURSOR_AGENT']], + ['github_copilot', ['COPILOT_AGENT_ID', 'GITHUB_COPILOT_AGENT']], + ['codex', ['CODEX_SANDBOX', 'CODEX_THREAD_ID']], + ['gemini', ['GEMINI_CLI']], + ['opencode', ['OPENCODE_BIN_PATH', 'OPENCODE']], + ['zed', ['ZED_TERM']], + ['devin', ['DEVIN_SESSION_ID', 'WINDSURF_SESSION_ID']], +]; + +export function detectInvoker( + env: NodeJS.ProcessEnv = process.env, + stdoutIsTty: boolean = Boolean(process.stdout.isTTY) +): Invoker { + for (const [invoker, markers] of INVOKER_MARKERS) { + if (markers.some((marker) => env[marker] !== undefined && env[marker] !== '')) { + return invoker; + } + } + return stdoutIsTty ? 'terminal' : 'unknown'; +} + +/** + * Count entries in one directory without recursing and without keeping the + * names. The names are change and spec ids — user-authored text that must not + * survive past this function. + */ +async function countEntries(dir: string): Promise { + try { + const entries = await fs.readdir(dir, { withFileTypes: true }); + return entries.filter( + (entry) => entry.isDirectory() && !entry.name.startsWith('.') && entry.name !== 'archive' + ).length; + } catch { + return undefined; + } +} + +/** + * Nearest `openspec/` directory walking up from cwd. + * + * Deliberately not the CLI's real root resolution: that consults stores, can + * throw, and can prompt. Telemetry gets a bounded stat walk instead, and null + * when there is nothing local. + */ +export function findLocalRoot(from: string = process.cwd(), maxDepth = 24): string | null { + let dir = from; + for (let depth = 0; depth < maxDepth; depth += 1) { + const candidate = path.join(dir, 'openspec'); + try { + if (statSync(candidate).isDirectory()) { + return candidate; + } + } catch { + // Not here; keep walking. + } + const parent = path.dirname(dir); + if (parent === dir) { + break; + } + dir = parent; + } + return null; +} + +/** + * Where the active schema was loaded from. + * + * A schema the user forked lives in the project; the bundled one ships with + * the package. The schema's *name* is a directory the user named, so only the + * source is reported. + */ +export function detectSchemaSource( + localRoot: string | null +): 'package' | 'project' | 'user' | undefined { + if (!localRoot) { + return undefined; + } + try { + const projectSchemas = path.join(localRoot, 'schemas'); + if (statSync(projectSchemas).isDirectory()) { + return 'project'; + } + } catch { + // No project schemas directory: the bundled schema is in use. + } + return 'package'; +} + +/** + * How this copy of the CLI was installed. `npx` runs out of a cache directory, + * a clone runs out of a checkout, and everything else is a global install. + * Answers whether upgrade advice is reachable, and how much of the userbase + * is trying the tool through `npx` rather than installing it. + */ +export function detectInstallKind( + installDir: string | null, + env: NodeJS.ProcessEnv = process.env +): 'global' | 'npx' | 'source' | 'other' { + if (env.npm_command === 'exec' || env.npm_lifecycle_event === 'npx') return 'npx'; + if (!installDir) return 'other'; + const normalized = installDir.replace(/\\/g, '/'); + if (normalized.includes('/_npx/')) return 'npx'; + if (normalized.endsWith('/src') || normalized.includes('/OpenSpec/')) return 'source'; + return 'global'; +} + +export interface RunContextInput { + projectRoot?: string | null; + installDir?: string | null; + stdoutIsTty: boolean; + jsonMode: boolean; + prompted: boolean; + firstRun: boolean; + storeInUse: boolean; + schemaSource?: 'package' | 'project' | 'user'; + toolCount?: number; + env?: NodeJS.ProcessEnv; +} + +export async function collectRunContext( + input: RunContextInput +): Promise> { + const env = input.env ?? process.env; + const context: Record = { + platform: bucketPlatform(process.platform), + node_major: bucketNodeMajor(process.versions.node), + install_kind: detectInstallKind(input.installDir ?? null, env), + invoker: detectInvoker(env, input.stdoutIsTty), + stdout_tty: input.stdoutIsTty, + json_mode: input.jsonMode, + prompted: input.prompted, + first_run: input.firstRun, + store_in_use: input.storeInUse, + schema_source: input.schemaSource, + }; + + try { + const config = getGlobalConfig(); + context.profile = config.profile; + context.delivery = config.delivery; + } catch { + // A config that cannot be read costs two properties, not the event. + } + + if (input.toolCount !== undefined) { + context.tools_count = bucketToolCount(input.toolCount); + } + + if (input.projectRoot) { + const changes = await countEntries(path.join(input.projectRoot, 'changes')); + if (changes !== undefined) { + context.changes = bucketCount(changes); + } + } + + return context; +} diff --git a/src/telemetry/index.ts b/src/telemetry/index.ts index 0e4d6290eb..5bd551d373 100644 --- a/src/telemetry/index.ts +++ b/src/telemetry/index.ts @@ -23,6 +23,27 @@ import { randomUUID } from 'crypto'; import { getGlobalConfig } from '../core/global-config.js'; import { isCiEnvironment } from '../utils/ci.js'; import { getTelemetryConfig, updateTelemetryConfig } from './config.js'; +import { + bucketDuration, + bucketExitCode, + isEventName, + isRegistryTool, + sanitizeProperties, + versionCode, + type ErrorClass, + type EventName, + type Milestone, + type Outcome, +} from './properties.js'; +import { + NOTICE_VERSION, + claimMilestone, + claimUnreportedTools, + getRunId, + loadSessionState, + recordOutcome, + type SessionState, +} from './state.js'; // PostHog API key - public key for client-side analytics // This is safe to embed as it only allows sending events, not reading data @@ -33,6 +54,53 @@ const TELEMETRY_REQUEST_TIMEOUT_MS = 1000; let anonymousId: string | null = null; +/** + * Events already sent this invocation, against the per-invocation cap. An + * agent harness can invoke this CLI dozens of times inside one task; an + * uncapped per-invocation count turns that into a burst of outbound requests + * nobody asked for. + */ +const MAX_EVENTS_PER_INVOCATION = 12; +let eventsSent = 0; + +/** + * Whether another event would fit under the cap. + * + * One-shot events must consult this *before* claiming. A claim is persisted + * forever, so claiming and then hitting the cap would lose that milestone or + * tool permanently — the metric would silently under-report for the life of + * the install, which is worse than reporting it one run later. + */ +export function hasEventCapacity(): boolean { + return eventsSent < MAX_EVENTS_PER_INVOCATION; +} + +/** + * True when the user asked to see the payloads instead of sending them. + * Deliberately independent of whether telemetry is enabled: the person most + * likely to want this is someone who already opted out and is deciding whether + * to opt back in. + */ +export function isDebugMode(): boolean { + return process.env.OPENSPEC_TELEMETRY_DEBUG === '1'; +} + +/** Test seam: forget the per-invocation event count. */ +export function resetEventCount(): void { + eventsSent = 0; + exitWasCancelled = false; +} + +/** + * True when this run ended in cancellation, so the flush must not wait. + */ +let exitWasCancelled = false; + +/** Mark this run as cancelled, so shutdown() returns without awaiting. */ +export function markCancelledExit(): void { + exitWasCancelled = true; +} + /** * Requests started by trackCommand and not yet settled, so shutdown can * flush them before the process exits. Each request is individually @@ -120,11 +188,86 @@ export async function getOrCreateAnonymousId(): Promise { return anonymousId; } +/** + * Placeholder shown in debug mode for an id that does not exist yet. Shaped + * like a UUID so the printed payload is the real thing structurally. + */ +const PLACEHOLDER_ID = '00000000-0000-0000-0000-000000000000'; + +let cachedState: SessionState | null = null; + +/** + * Load the persisted state, or synthesize a read-only one in debug mode. + * + * Debug mode must not generate or persist an anonymous id: inspecting what + * telemetry would send cannot be the act that creates the identifier being + * inspected. + */ +async function loadState(): Promise { + if (cachedState) { + return cachedState; + } + + if (isDebugMode()) { + const existing = await getTelemetryConfig({ persist: false }); + cachedState = { + anonymousId: existing.anonymousId ?? PLACEHOLDER_ID, + workSessionId: existing.workSessionId ?? PLACEHOLDER_ID, + firstRun: existing.anonymousId === undefined, + firstSeenAt: existing.firstSeenAt, + previousOutcome: (existing.previousOutcome as Outcome | undefined) ?? 'none', + previousCommand: existing.previousCommand, + milestones: existing.milestones ?? [], + reportedTools: existing.reportedTools ?? [], + }; + return cachedState; + } + + if (!isTelemetryEnabled()) { + return null; + } + + cachedState = await loadSessionState(); + return cachedState; +} + +/** Test seam: forget the cached state. */ +export function resetState(): void { + cachedState = null; +} + +/** Whether this invocation is the user's first ever. */ +export function isFirstRun(): boolean { + return cachedState?.firstRun ?? false; +} + /** * Send one capture event to PostHog's batch endpoint. Fire-and-forget: * bounded by the request timeout, never throws, never retries. */ -function sendEvent(distinctId: string, event: string, properties: Record): void { +function sendEvent(distinctId: string, event: EventName, properties: Record): void { + // The allowlist is the authority here, not at the call sites: a builder the + // tests never construct still cannot ship a key or value off the list. + const clean = sanitizeProperties(properties); + + if (!isEventName(event)) { + return; + } + + if (eventsSent >= MAX_EVENTS_PER_INVOCATION) { + return; + } + eventsSent += 1; + + if (isDebugMode()) { + // stderr, never stdout: stdout carries command output and must stay + // parser-safe even while someone is inspecting telemetry. + console.error( + `[openspec telemetry] ${JSON.stringify({ event, distinct_id: distinctId, properties: clean })}` + ); + return; + } + const body = JSON.stringify({ api_key: POSTHOG_API_KEY, batch: [ @@ -132,7 +275,9 @@ function sendEvent(distinctId: string, event: string, properties: Record { - if (!isTelemetryEnabled()) { + if (!isTelemetryEnabled() && !isDebugMode()) { return; } try { - const userId = await getOrCreateAnonymousId(); + const state = await loadState(); + if (!state) { + return; + } - sendEvent(userId, 'command_executed', { + sendEvent(state.anonymousId, 'command_executed', { command: commandName, - version: version, + version, + version_code: versionCode(version), surface: 'cli', + run_id: getRunId(), + work_session_id: state.workSessionId, $ip: null, // Explicitly disable IP tracking + // Location is never derived from the connecting address either. The + // payload asks for this so the guarantee lives in code we ship, not + // only in a proxy configuration a reader cannot inspect. + $geoip_disable: true, }); } catch { // Silent failure - telemetry should never break CLI } } +/** + * Record how a command ended. + * + * Sent from the postAction hook, from the interception of commander's own + * usage errors, and from the unhandled-rejection handler — the three families + * of exit that would otherwise leave a failure invisible. + */ +export async function trackCompletion(input: { + command: string; + version: string; + outcome: Outcome; + errorClass: ErrorClass; + exitCode: number | undefined; + durationMs: number; + context?: Record; +}): Promise { + if (!isTelemetryEnabled() && !isDebugMode()) { + return; + } + + // Ctrl-C is the user asking the process to stop: the event goes out, but the + // flush must never hold the exit for it. Set here rather than in the CLI + // wiring so every caller of this function gets the rule. + if (input.outcome === 'cancelled') { + markCancelledExit(); + } + + try { + const state = await loadState(); + if (!state) { + return; + } + + sendEvent(state.anonymousId, 'command_completed', { + // Context first: a future context key sharing a name with an outcome + // field must never overwrite the reason this event exists. + ...(input.context ?? {}), + command: input.command, + version: input.version, + version_code: versionCode(input.version), + surface: 'cli', + run_id: getRunId(), + work_session_id: state.workSessionId, + outcome: input.outcome, + error_class: input.errorClass, + exit_code: bucketExitCode(input.exitCode), + duration: bucketDuration(input.durationMs), + previous_outcome: state.previousOutcome, + previous_command_same: state.previousCommand === input.command, + $ip: null, + $geoip_disable: true, + }); + + // Debug mode inspects; it never writes. That includes the retry record, + // not just the anonymous id. + if (isTelemetryEnabled() && !isDebugMode()) { + await recordOutcome(input.command, input.outcome); + } + } catch { + // Silent failure - telemetry should never break CLI + } +} + +/** Send a milestone the first time it is reached. */ +export async function trackMilestone(milestone: Milestone, version: string): Promise { + if (!isTelemetryEnabled() && !isDebugMode()) { + return; + } + + try { + const state = await loadState(); + if (!state) { + return; + } + if (!hasEventCapacity()) { + return; // Claim nothing; the milestone is still unreported next run. + } + const claim = await claimMilestone(milestone, state, new Date(), !isDebugMode()); + if (!claim) { + return; + } + + sendEvent(state.anonymousId, 'milestone_reached', { + milestone, + version, + version_code: versionCode(version), + run_id: getRunId(), + time_to_reach: claim.timeToReach, + $ip: null, + $geoip_disable: true, + }); + } catch { + // Silent failure - telemetry should never break CLI + } +} + +/** + * Report configured tools, once each. + * + * Carries no run context and no `run_id`. Keeping the run id here would have + * defeated the point: one join on it reassembles the full configured set + * beside platform, install kind, and counts — the row this split exists to + * avoid. + * + * The honest limit of the split: these events still share `distinct_id` with + * every other event, so a determined query can associate them. What it buys is + * that the set is never assembled *in one row*, and that the tools of a user + * who never triggers a completion event are never learned at all. + */ +export async function trackConfiguredTools(toolIds: string[], version: string): Promise { + if (!isTelemetryEnabled() && !isDebugMode()) { + return; + } + + try { + const state = await loadState(); + if (!state) { + return; + } + // Filter before claiming: an unlisted id would otherwise be marked + // reported and produce an event whose only real property was stripped. + const known = toolIds.filter((id) => isRegistryTool(id)); + // Claim only as many as can actually be sent this run; the rest stay + // unreported and go out on a later invocation. + const room = MAX_EVENTS_PER_INVOCATION - eventsSent; + if (room <= 0) { + return; + } + for (const tool of await claimUnreportedTools(known.slice(0, room), state, !isDebugMode())) { + sendEvent(state.anonymousId, 'tool_configured', { + tool, + version, + version_code: versionCode(version), + $ip: null, + $geoip_disable: true, + }); + } + } catch { + // Silent failure - telemetry should never break CLI + } +} + /** * Show first-run telemetry notice if not already seen. */ @@ -184,9 +481,23 @@ export async function maybeShowTelemetryNotice( return; } + // Inspecting telemetry must not consume the first-run disclosure. Without + // this, a user whose very first command is a debug run never sees the notice + // on their first real one — the only run it exists for. + if (isDebugMode()) { + return; + } + try { const config = await getTelemetryConfig(); - if (config.noticeSeen) { + + // Versioned rather than reset: resetting noticeSeen would discard the fact + // that the user was told at all, and show the same generic sentence to + // someone who already read it, which teaches them to ignore it. A version + // distinguishes "never told" from "told about an earlier scope", and lets + // the message say what actually changed. + const seenVersion = config.noticeVersion ?? (config.noticeSeen ? 1 : 0); + if (seenVersion >= NOTICE_VERSION) { return; } @@ -199,12 +510,14 @@ export async function maybeShowTelemetryNotice( // Display notice on stderr, not stdout: stdout is reserved for command // output (raw passthrough text, JSON, etc.) and must stay parser/pipe-safe. + // It is a notice, never a prompt: nothing is asked and nothing blocks. console.error( - 'Note: OpenSpec collects anonymous usage stats. Opt out: OPENSPEC_TELEMETRY=0 or openspec config set telemetry.enabled false' + seenVersion === 0 + ? 'Note: OpenSpec collects pseudonymous usage stats (command, outcome, and basic run context). See them with OPENSPEC_TELEMETRY_DEBUG=1. Opt out: OPENSPEC_TELEMETRY=0 or openspec config set telemetry.enabled false' + : 'Note: OpenSpec usage stats now also record whether a command succeeded, plus basic run context (OS, Node major). See them with OPENSPEC_TELEMETRY_DEBUG=1. Opt out: OPENSPEC_TELEMETRY=0 or openspec config set telemetry.enabled false' ); - // Mark as seen - await updateTelemetryConfig({ noticeSeen: true }); + await updateTelemetryConfig({ noticeSeen: true, noticeVersion: NOTICE_VERSION }); } catch { // Silent failure - telemetry should never break CLI } @@ -215,6 +528,15 @@ export async function maybeShowTelemetryNotice( * Call this before CLI exit. */ export async function shutdown(): Promise { + // Ctrl-C is the user asking the process to stop. Holding it open for up to + // the request timeout while they press Ctrl-C again is a worse outcome than + // a lossy cancellation metric — and the ratio is all that metric is used + // for. The request is already in flight; it lands or it does not. + if (exitWasCancelled) { + pendingEvents.clear(); + return; + } + if (pendingEvents.size === 0) { return; } diff --git a/src/telemetry/properties.ts b/src/telemetry/properties.ts new file mode 100644 index 0000000000..6780907e9c --- /dev/null +++ b/src/telemetry/properties.ts @@ -0,0 +1,340 @@ +/** + * The telemetry property contract. + * + * Every event name and property key is a member of a literal list declared in + * this file. Every property value is a member of such a list, a boolean, a + * bounded number, or a randomly generated id matched against a fixed pattern. + * Nothing here is computed from a schema, a catalog, or any other file a user + * can author — that is what makes the guarantee checkable by reading one + * module. + * + * The lists are enforced at send time by `sanitizeEvent`, not only asserted in + * a test: a test passes vacuously for any code path it does not construct, so + * a convention would decay the first time someone adds an event builder the + * tests do not cover. + * + * Binding values alone would not be enough. `{ "schema:acme-internal": true }` + * carries a boolean value and still ships the user's schema name, so keys are + * bound on the same terms as values. + */ + +/** Every event this CLI may send. */ +export const EVENT_NAMES = [ + 'command_executed', + 'command_completed', + 'milestone_reached', + 'tool_configured', +] as const; +export type EventName = (typeof EVENT_NAMES)[number]; + +/** How a command ended. */ +export const OUTCOMES = ['success', 'user_error', 'internal_error', 'cancelled'] as const; +export type Outcome = (typeof OUTCOMES)[number]; + +/** + * The failure families this CLI actually has. Declared as a literal union so a + * future contributor cannot generate classes from a user-authored schema and + * still be compliant. + */ +export const ERROR_CLASSES = [ + 'none', + 'cancelled', + 'not_interactive', + 'no_root', + 'item_not_found', + 'ambiguous_item', + 'schema_not_found', + 'schema_invalid', + 'bad_usage', + 'unknown_subcommand', + 'validation_failed', + 'archive_blocked', + 'concurrent_modification', + 'store_error', + 'git_error', + 'fs_error', + 'parse_error', + 'metadata_invalid', + 'external_tool_failed', + 'network_error', + 'already_exists', + 'internal_error', + 'unclassified', + 'other', +] as const; +export type ErrorClass = (typeof ERROR_CLASSES)[number]; + +export const PLATFORMS = ['darwin', 'linux', 'win32', 'other'] as const; +export const INSTALL_KINDS = ['global', 'npx', 'source', 'other'] as const; +export const SCHEMA_SOURCES = ['package', 'project', 'user'] as const; +export const EXIT_CODES = ['0', '1', '130', 'other'] as const; +/** + * Bucket labels are ordered lexicographically wherever they are charted, so + * they are written to sort that way. Without this, a duration breakdown reads + * `100-500`, `10000+`, `2000-10000`, `500-2000`, `<100` — a histogram in + * scrambled order, which is worse than no histogram. Counts zero-pad; the + * ranges that mix units carry an ordinal prefix, since no padding rescues + * `<1h` against `31d+`. + */ +export const COUNT_BUCKETS = ['00', '01-03', '04-10', '11-30', '31+'] as const; +export const TOOL_COUNT_BUCKETS = ['0', '1', '2-3', '4+'] as const; +export const DURATION_BUCKETS = [ + '1_under_100ms', + '2_100-500ms', + '3_500ms-2s', + '4_2-10s', + '5_over_10s', +] as const; +export const TIME_TO_REACH_BUCKETS = [ + '1_under_1h', + '2_1-24h', + '3_1-7d', + '4_8-30d', + '5_over_30d', +] as const; +export const MILESTONES = ['install', 'init', 'propose', 'apply', 'archive'] as const; +export type Milestone = (typeof MILESTONES)[number]; + +/** + * Node majors we report by name. Anything else is `other` — a nightly or an + * odd-numbered major is a small enough population to be identifying, and the + * support-window decision only needs the supported majors distinguished. + */ +export const NODE_MAJORS = ['20', '22', '24', '26', 'other'] as const; + +/** + * Which agent is driving this run. Derived by testing for the presence of a + * marker (see `invoker.ts`); no environment variable name or value is ever + * sent, and an unrecognized marker collapses to `unknown`. + */ +export const INVOKERS = [ + 'claude_code', + 'cursor', + 'github_copilot', + 'codex', + 'gemini', + 'opencode', + 'zed', + 'devin', + 'terminal', + 'unknown', +] as const; +export type Invoker = (typeof INVOKERS)[number]; + +/** Values a property may hold, keyed by property. `true` means any boolean. */ +const PROPERTY_VALUES = { + // Identity and correlation + command: 'command-list', + version: 'free-version', + version_code: 'version-code', + surface: ['cli'], + run_id: 'uuid', + work_session_id: 'uuid', + $ip: 'null-only', + $geoip_disable: 'true-only', + + // Outcome + outcome: OUTCOMES, + error_class: ERROR_CLASSES, + exit_code: EXIT_CODES, + duration: DURATION_BUCKETS, + previous_outcome: [...OUTCOMES, 'none'], + previous_command_same: 'boolean', + + // Run context + platform: PLATFORMS, + node_major: NODE_MAJORS, + install_kind: INSTALL_KINDS, + invoker: INVOKERS, + stdout_tty: 'boolean', + json_mode: 'boolean', + prompted: 'boolean', + first_run: 'boolean', + profile: ['core', 'custom'], + delivery: ['both', 'skills', 'commands'], + tools_count: TOOL_COUNT_BUCKETS, + schema_source: SCHEMA_SOURCES, + store_in_use: 'boolean', + changes: COUNT_BUCKETS, + + // Milestones and adoption + milestone: MILESTONES, + time_to_reach: TIME_TO_REACH_BUCKETS, + tool: 'tool-registry', +} as const satisfies Record; + +export type PropertyKey = keyof typeof PROPERTY_VALUES; + +/** Every property key that may leave this process. */ +export const PROPERTY_KEYS = Object.keys(PROPERTY_VALUES) as PropertyKey[]; + +const UUID_PATTERN = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i; +/** Semver as this package emits it; never user input, but bounded anyway. */ +const VERSION_PATTERN = /^[0-9]+\.[0-9]+\.[0-9]+(?:-[0-9A-Za-z.-]+)?$/; + +/** + * Membership checks for the property kinds whose value set is a registry + * elsewhere in the codebase. Injected rather than imported so this module + * stays free of the CLI's import graph — `src/cli/index.ts` already documents + * that a telemetry helper reaching into command registration creates a cycle. + */ +export interface RegistryChecks { + isCommand(value: string): boolean; + isTool(value: string): boolean; +} + +let registry: RegistryChecks = { + isCommand: () => false, + isTool: () => false, +}; + +export function setRegistryChecks(checks: RegistryChecks): void { + registry = checks; +} + +function isAllowedValue(key: PropertyKey, value: unknown): boolean { + const rule = PROPERTY_VALUES[key] as readonly string[] | string; + + if (Array.isArray(rule)) { + return typeof value === 'string' && rule.includes(value); + } + + switch (rule) { + case 'boolean': + return typeof value === 'boolean'; + case 'uuid': + return typeof value === 'string' && UUID_PATTERN.test(value); + case 'free-version': + return typeof value === 'string' && VERSION_PATTERN.test(value); + case 'version-code': + // Bounded, not merely numeric: an unbounded integer would be a 53-bit + // channel through a contract that exists to have none. + return ( + typeof value === 'number' && + Number.isInteger(value) && + value >= 0 && + value < 1_000_000_000 + ); + case 'null-only': + return value === null; + case 'true-only': + return value === true; + case 'command-list': + return typeof value === 'string' && (value === 'unknown' || registry.isCommand(value)); + case 'tool-registry': + return typeof value === 'string' && registry.isTool(value); + default: + return false; + } +} + +/** Whether a tool id is in the registry, for callers that must skip rather than strip. */ +export function isRegistryTool(value: string): boolean { + return registry.isTool(value); +} + +export function isEventName(value: string): value is EventName { + return (EVENT_NAMES as readonly string[]).includes(value); +} + +/** + * Drop anything not on the allowlist, immediately before serialization. + * + * A dropped property never prevents the event from being sent: a malformed + * value is a reason to lose one field, not to lose the outcome signal the + * event exists to carry. + */ +export function sanitizeProperties( + properties: Record +): Record { + const clean: Record = {}; + for (const [key, value] of Object.entries(properties)) { + if (value === undefined) { + continue; + } + if (!(PROPERTY_KEYS as string[]).includes(key)) { + continue; + } + if (!isAllowedValue(key as PropertyKey, value)) { + continue; + } + clean[key] = value; + } + return clean; +} + +/** Bucket a count into the fixed labels. */ +/** + * Bucketed, never exact: the count separates a trial from real use, which is + * the question it exists for, and the exact number would say more about the + * project than any decision needs. + */ +export function bucketCount(count: number): (typeof COUNT_BUCKETS)[number] { + if (count <= 0) return '00'; + if (count <= 3) return '01-03'; + if (count <= 10) return '04-10'; + if (count <= 30) return '11-30'; + return '31+'; +} + +export function bucketToolCount(count: number): (typeof TOOL_COUNT_BUCKETS)[number] { + if (count <= 0) return '0'; + if (count === 1) return '1'; + if (count <= 3) return '2-3'; + return '4+'; +} + +export function bucketDuration(ms: number): (typeof DURATION_BUCKETS)[number] { + if (ms < 100) return '1_under_100ms'; + if (ms < 500) return '2_100-500ms'; + if (ms < 2000) return '3_500ms-2s'; + if (ms < 10000) return '4_2-10s'; + return '5_over_10s'; +} + +export function bucketExitCode(code: number | undefined): (typeof EXIT_CODES)[number] { + // Not a raw code: `workset open` returns the launched editor's status + // (including 128 + signal), `feedback` returns gh's, and `update` returns the + // re-spawned CLI's, so the raw value is unbounded. + if (code === undefined || code === 0) return '0'; + if (code === 1) return '1'; + if (code === 130) return '130'; + return 'other'; +} + +export function bucketTimeToReach(msSinceFirstSeen: number): (typeof TIME_TO_REACH_BUCKETS)[number] { + const hours = msSinceFirstSeen / 3_600_000; + if (hours < 1) return '1_under_1h'; + if (hours < 24) return '2_1-24h'; + const days = hours / 24; + if (days < 8) return '3_1-7d'; + if (days <= 30) return '4_8-30d'; + return '5_over_30d'; +} + +/** + * A sortable integer for a semver string, so version-over-version charts order + * numerically. Lexicographically `1.10.0` sorts before `1.9.0`, which silently + * misreads every regression comparison. + */ +export function versionCode(version: string): number | undefined { + const match = /^(\d+)\.(\d+)\.(\d+)/.exec(version); + if (!match) { + return undefined; + } + const [, major, minor, patch] = match; + return Number(major) * 1_000_000 + Number(minor) * 1_000 + Number(patch); +} + +export function bucketNodeMajor(version: string): (typeof NODE_MAJORS)[number] { + const major = version.replace(/^v/, '').split('.')[0]; + return (NODE_MAJORS as readonly string[]).includes(major) + ? (major as (typeof NODE_MAJORS)[number]) + : 'other'; +} + +export function bucketPlatform(platform: string): (typeof PLATFORMS)[number] { + return (PLATFORMS as readonly string[]).includes(platform) + ? (platform as (typeof PLATFORMS)[number]) + : 'other'; +} diff --git a/src/telemetry/state.ts b/src/telemetry/state.ts new file mode 100644 index 0000000000..0fbba03747 --- /dev/null +++ b/src/telemetry/state.ts @@ -0,0 +1,159 @@ +/** + * Persisted telemetry state: identity, work session, milestones, and the + * previous run's outcome. + * + * Two invariants hold across everything here: + * + * 1. Nothing is written when telemetry is disabled. An opted-out user leaves + * no trace, including the anonymous id itself. + * 2. Only enum labels, counters, booleans, timestamps, and random ids are + * stored. A timestamp is never sent — only a bucket derived from it. + */ +import { randomUUID } from 'crypto'; +import { getTelemetryConfig, updateTelemetryConfig } from './config.js'; +import { + bucketTimeToReach, + type Milestone, + type Outcome, + type TIME_TO_REACH_BUCKETS, +} from './properties.js'; + +/** How long a gap may be before a new work session starts. */ +const WORK_SESSION_WINDOW_MS = 30 * 60 * 1000; + +/** Bumped whenever the disclosed property list expands. */ +export const NOTICE_VERSION = 2; + +let cachedRunId: string | null = null; + +/** + * The id for this invocation. Random, never persisted, and never derived from + * the anonymous id, the pid, the working directory, or the clock. + * + * Its job is to reveal a `command_executed` with no matching + * `command_completed` — the signature of an exit path the coverage missed. + */ +export function getRunId(): string { + if (!cachedRunId) { + cachedRunId = randomUUID(); + } + return cachedRunId; +} + +/** Test seam: forget the cached run id. */ +export function resetRunId(): void { + cachedRunId = null; +} + +export interface SessionState { + anonymousId: string; + workSessionId: string; + firstRun: boolean; + firstSeenAt: string | undefined; + previousOutcome: Outcome | 'none'; + previousCommand: string | undefined; + milestones: string[]; + reportedTools: string[]; +} + +/** + * Read the persisted state, minting identity and session ids as needed. + * + * Callers must confirm telemetry is enabled first: this writes. + */ +export async function loadSessionState(now: Date = new Date()): Promise { + const config = await getTelemetryConfig(); + const updates: Record = {}; + + let anonymousId = config.anonymousId; + let firstRun = false; + if (!anonymousId) { + anonymousId = randomUUID(); + updates.anonymousId = anonymousId; + updates.firstSeenAt = now.toISOString(); + firstRun = true; + } + + const lastActivity = config.lastActivityAt ? Date.parse(config.lastActivityAt) : NaN; + const withinWindow = + Number.isFinite(lastActivity) && now.getTime() - lastActivity < WORK_SESSION_WINDOW_MS; + + let workSessionId = config.workSessionId; + if (!workSessionId || !withinWindow) { + workSessionId = randomUUID(); + updates.workSessionId = workSessionId; + } + updates.lastActivityAt = now.toISOString(); + + if (Object.keys(updates).length > 0) { + await updateTelemetryConfig(updates); + } + + return { + anonymousId, + workSessionId, + firstRun, + firstSeenAt: firstRun ? now.toISOString() : config.firstSeenAt, + previousOutcome: (config.previousOutcome as Outcome | undefined) ?? 'none', + previousCommand: config.previousCommand, + milestones: config.milestones ?? [], + reportedTools: config.reportedTools ?? [], + }; +} + +/** Record this run's outcome so the next one can report whether it recovered. */ +export async function recordOutcome(command: string, outcome: Outcome): Promise { + await updateTelemetryConfig({ previousOutcome: outcome, previousCommand: command }); +} + +/** + * Claim a milestone, returning its time bucket when this is the first time. + * + * Returns null when the milestone is already recorded, so the caller sends + * nothing. + */ +export async function claimMilestone( + milestone: Milestone, + state: SessionState, + now: Date = new Date(), + persist = true +): Promise<{ timeToReach: (typeof TIME_TO_REACH_BUCKETS)[number] | undefined } | null> { + if (state.milestones.includes(milestone)) { + return null; + } + + const milestones = [...state.milestones, milestone]; + state.milestones = milestones; + // Inspecting what would be sent must not spend the one-shot claim, or the + // real event would never fire on a later run. + if (persist) { + await updateTelemetryConfig({ milestones }); + } + + // An id minted before this change has no recorded first-seen time. Omitting + // the bucket is honest; sending the lowest one would fabricate a wave of + // instant activations across the existing userbase. + const firstSeen = state.firstSeenAt ? Date.parse(state.firstSeenAt) : NaN; + if (!Number.isFinite(firstSeen)) { + return { timeToReach: undefined }; + } + return { timeToReach: bucketTimeToReach(Math.max(0, now.getTime() - firstSeen)) }; +} + +/** Registry tool ids not yet reported. Claims them so each is sent once. */ +export async function claimUnreportedTools( + toolIds: string[], + state: SessionState, + persist = true +): Promise { + const unreported = toolIds.filter((id) => !state.reportedTools.includes(id)); + if (unreported.length === 0) { + return []; + } + const reportedTools = [...state.reportedTools, ...unreported]; + state.reportedTools = reportedTools; + if (persist) { + await updateTelemetryConfig({ reportedTools }); + } + return unreported; +} diff --git a/src/utils/interactive.ts b/src/utils/interactive.ts index 400249e867..5f34ad0863 100644 --- a/src/utils/interactive.ts +++ b/src/utils/interactive.ts @@ -1,5 +1,6 @@ import { createInterface } from 'node:readline'; import type { Readable, Writable } from 'node:stream'; +import { loadPrompts } from './prompt-module.js'; export type InteractiveOptions = { /** @@ -99,7 +100,7 @@ export async function confirmPrompt( Boolean((input as { isTTY?: boolean }).isTTY) && Boolean((output as { isTTY?: boolean }).isTTY); if (isTerminal) { - const { confirm } = await import('@inquirer/prompts'); + const { confirm } = await loadPrompts(); return confirm(prompt); } return readYesNo(prompt, input, output); diff --git a/src/utils/prompt-module.ts b/src/utils/prompt-module.ts new file mode 100644 index 0000000000..983a59aa94 --- /dev/null +++ b/src/utils/prompt-module.ts @@ -0,0 +1,56 @@ +/** + * Single loader for the interactive prompt module. + * + * Every prompt in the CLI goes through here so the time a person spends + * thinking at a menu can be measured once, in one place, instead of being + * counted as time the command took to run. Without this, `init`, `archive`, + * and `config profile` would report a p95 latency that is really a p95 + * reading speed. + * + * The wrapper is timing only: it forwards arguments and results untouched, + * and a rejection (Ctrl-C) still closes the window before it propagates. + */ +import { markPromptClosed, markPromptOpen } from '../telemetry/cli-runtime.js'; + +type PromptModule = typeof import('@inquirer/prompts'); +type PromptName = 'confirm' | 'input' | 'select' | 'checkbox'; + +function timed Promise>(fn: T): T { + return (async (...args: Parameters) => { + markPromptOpen(); + try { + return await fn(...args); + } finally { + markPromptClosed(); + } + }) as T; +} + +/** + * Load @inquirer/prompts with every prompt wrapped in timing. + * + * Kept as a dynamic import, matching what each call site did before: the + * module is heavy and most runs never prompt at all. + */ +export async function loadPrompts(): Promise { + const prompts = await import('@inquirer/prompts'); + // A plain copy, not Object.create(prompts): module namespace properties are + // non-writable, so assigning a wrapper over one through the prototype throws + // in strict mode. + const wrapped = { ...prompts } as Record; + for (const name of ['confirm', 'input', 'select', 'checkbox'] satisfies PromptName[]) { + // Read defensively: a test double for this module exposes only the prompts + // that test needs, and reading an absent export off a mocked namespace + // throws rather than returning undefined. + let fn: unknown; + try { + fn = prompts[name]; + } catch { + continue; + } + if (typeof fn === 'function') { + wrapped[name] = timed(fn as (...args: never[]) => Promise); + } + } + return wrapped as PromptModule; +} diff --git a/test/commands/feedback.test.ts b/test/commands/feedback.test.ts index 9503610610..57a1d3076c 100644 --- a/test/commands/feedback.test.ts +++ b/test/commands/feedback.test.ts @@ -413,9 +413,10 @@ describe('FeedbackCommand', () => { throw error; }); - await expect(feedbackCommand.execute('Test')).rejects.toThrow( - 'process.exit(1)' - ); + // Exits by setting the code, not by exiting: exiting here would skip + // commander's postAction hook and drop the failure report. + await feedbackCommand.execute('Test'); + expect(process.exitCode).toBe(1); // Should display the error from gh CLI expect(consoleErrorSpy).toHaveBeenCalledWith( @@ -459,9 +460,8 @@ describe('FeedbackCommand', () => { throw error; }); - await expect( - feedbackCommand.execute('gh could not add label bug report') - ).rejects.toThrow('process.exit(1)'); + await feedbackCommand.execute('gh could not add label bug report'); + expect(process.exitCode).toBe(1); expect(mockExecFileSync).toHaveBeenCalledTimes(1); expect(consoleLogSpy).not.toHaveBeenCalledWith( @@ -556,9 +556,10 @@ describe('FeedbackCommand', () => { throw error; }); - await expect(feedbackCommand.execute('Test')).rejects.toThrow( - 'process.exit(4)' - ); + // Exits by setting the code, not by exiting: exiting here would skip + // commander's postAction hook and drop the failure report. + await feedbackCommand.execute('Test'); + expect(process.exitCode).toBe(4); expect(mockExecFileSync).toHaveBeenCalledTimes(2); expect(consoleErrorSpy).toHaveBeenCalledWith( diff --git a/test/telemetry/classify.test.ts b/test/telemetry/classify.test.ts new file mode 100644 index 0000000000..17a1a35de2 --- /dev/null +++ b/test/telemetry/classify.test.ts @@ -0,0 +1,97 @@ +import { describe, it, expect } from 'vitest'; +import { classifyError } from '../../src/telemetry/classify.js'; +import { ERROR_CLASSES, sanitizeProperties, setRegistryChecks } from '../../src/telemetry/properties.js'; + +setRegistryChecks({ isCommand: () => true, isTool: () => true }); + +function withDiagnostic(code: string, message = 'boom') { + const error = new Error(message) as Error & { diagnostic: { code: string; message: string } }; + error.diagnostic = { code, message }; + return error; +} + +describe('classifyError', () => { + it('maps known diagnostic codes onto the allowlist', () => { + expect(classifyError(withDiagnostic('unknown_item')).errorClass).toBe('item_not_found'); + expect(classifyError(withDiagnostic('no_openspec_root')).errorClass).toBe('no_root'); + expect(classifyError(withDiagnostic('archive_tasks_incomplete')).errorClass).toBe('archive_blocked'); + expect(classifyError(withDiagnostic('store_git_commit_failed')).errorClass).toBe('git_error'); + expect(classifyError(withDiagnostic('store_id_conflict')).errorClass).toBe('already_exists'); + expect(classifyError(withDiagnostic('store_registry_changed')).errorClass).toBe('concurrent_modification'); + expect(classifyError(withDiagnostic('workset_launch_failed')).errorClass).toBe('external_tool_failed'); + expect(classifyError(withDiagnostic('unknown_store_subcommand')).errorClass).toBe('unknown_subcommand'); + }); + + it('treats a blocked precondition as user error, not our bug', () => { + const result = classifyError(withDiagnostic('archive_validation_failed')); + expect(result.outcome).toBe('user_error'); + expect(result.errorClass).toBe('validation_failed'); + }); + + it('treats a store cancellation as cancelled', () => { + expect(classifyError(withDiagnostic('store_setup_cancelled'))).toEqual({ + outcome: 'cancelled', + errorClass: 'cancelled', + }); + }); + + it('never passes an unrecognized diagnostic code through', () => { + // A code carrying user text still maps to a constant, so nothing leaks + // even when a future code is shaped in a way this map did not anticipate. + const familiar = classifyError(withDiagnostic('store_unreachable_at_/Users/jane/acme')); + expect(familiar.errorClass).toBe('store_error'); + expect(JSON.stringify(sanitizeProperties({ error_class: familiar.errorClass }))).not.toContain('jane'); + + // A code matching no map entry and no family prefix means a stale + // classifier, not a crash — kept out of internal_error so that metric + // still means "our bug". + const foreign = classifyError(withDiagnostic('quux_failed_for_/Users/jane/acme')); + expect(foreign.errorClass).toBe('unclassified'); + expect(foreign.outcome).toBe('user_error'); + expect(JSON.stringify(sanitizeProperties({ error_class: foreign.errorClass }))).not.toContain('jane'); + }); + + it('classifies prompt cancellation', () => { + const error = new Error('User force closed the prompt with SIGINT'); + error.name = 'ExitPromptError'; + expect(classifyError(error)).toEqual({ outcome: 'cancelled', errorClass: 'cancelled' }); + }); + + it('classifies filesystem and network errnos', () => { + const eacces = Object.assign(new Error('denied'), { code: 'EACCES' }); + expect(classifyError(eacces).errorClass).toBe('fs_error'); + const dns = Object.assign(new Error('dns'), { code: 'ENOTFOUND' }); + expect(classifyError(dns).errorClass).toBe('network_error'); + const enoent = Object.assign(new Error('missing'), { code: 'ENOENT' }); + expect(classifyError(enoent).errorClass).toBe('item_not_found'); + }); + + it('defaults an unclassifiable error to our problem', () => { + expect(classifyError(new Error('Change "acme-billing" not found'))).toEqual({ + outcome: 'internal_error', + errorClass: 'other', + }); + expect(classifyError('a string')).toEqual({ outcome: 'internal_error', errorClass: 'other' }); + expect(classifyError(undefined)).toEqual({ outcome: 'internal_error', errorClass: 'other' }); + }); + + it('never carries the error message', () => { + const result = classifyError(new Error('Change "acme-billing-rewrite" not found at /Users/jane')); + expect(JSON.stringify(result)).not.toContain('acme-billing-rewrite'); + expect(JSON.stringify(result)).not.toContain('jane'); + }); + + it('only ever returns an allowlisted class', () => { + const samples: unknown[] = [ + new Error('x'), + withDiagnostic('store_path_missing'), + withDiagnostic('totally_made_up'), + Object.assign(new Error('y'), { code: 'EPERM' }), + null, + 42, + ]; + for (const sample of samples) { + expect(ERROR_CLASSES).toContain(classifyError(sample).errorClass); + } + }); +}); diff --git a/test/telemetry/cli-runtime.test.ts b/test/telemetry/cli-runtime.test.ts new file mode 100644 index 0000000000..a318babf73 --- /dev/null +++ b/test/telemetry/cli-runtime.test.ts @@ -0,0 +1,172 @@ +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import * as fs from 'node:fs'; +import * as path from 'node:path'; +import * as os from 'node:os'; +import { + beginRun, + finishRun, + markFailure, + markCheckFailed, + markMilestone, + markOutcome, + markPromptClosed, + markPromptOpen, + registerAllowlists, + wasPrompted, +} from '../../src/telemetry/cli-runtime.js'; +import { resetEventCount, resetState, shutdown } from '../../src/telemetry/index.js'; +import { resetRunId } from '../../src/telemetry/state.js'; +import { sanitizeProperties, setRegistryChecks } from '../../src/telemetry/properties.js'; +import { Command } from 'commander'; + +describe('telemetry/cli-runtime', () => { + let tempDir: string; + let originalEnv: NodeJS.ProcessEnv; + let fetchSpy: ReturnType>; + + beforeEach(() => { + originalEnv = { ...process.env }; + tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'openspec-runtime-')); + process.env.XDG_CONFIG_HOME = tempDir; + process.env.HOME = tempDir; + process.env.USERPROFILE = tempDir; + delete process.env.OPENSPEC_TELEMETRY; + delete process.env.DO_NOT_TRACK; + delete process.env.CI; + delete process.env.OPENSPEC_TELEMETRY_DEBUG; + + resetEventCount(); + resetState(); + resetRunId(); + setRegistryChecks({ isCommand: () => true, isTool: () => true }); + fetchSpy = vi.spyOn(globalThis, 'fetch').mockResolvedValue(new Response(null, { status: 200 })); + beginRun('1.2.3'); + }); + + afterEach(async () => { + await shutdown(); + process.env = originalEnv; + fs.rmSync(tempDir, { recursive: true, force: true }); + vi.restoreAllMocks(); + }); + + function completions() { + return fetchSpy.mock.calls + .map(([, o]) => JSON.parse(String((o as RequestInit).body)).batch[0]) + .filter((e) => e.event === 'command_completed'); + } + + async function complete(exitCode: number | undefined) { + await finishRun({ command: 'archive', version: '1.2.3', exitCode, jsonMode: false, minimal: true }); + await shutdown(); + } + + it('reports a clean run as success', async () => { + await complete(0); + expect(completions()[0].properties).toMatchObject({ + outcome: 'success', + error_class: 'none', + exit_code: '0', + }); + }); + + it('sends exactly one completion however many times it is asked', async () => { + await finishRun({ command: 'archive', version: '1.2.3', exitCode: 1, jsonMode: false, minimal: true }); + await finishRun({ command: 'archive', version: '1.2.3', exitCode: 1, jsonMode: false, minimal: true }); + await shutdown(); + // A duplicate would double-count every failure rate built on this event. + expect(completions()).toHaveLength(1); + }); + + it('keeps the first classification when an error is reclassified', async () => { + markOutcome(Object.assign(new Error('x'), { diagnostic: { code: 'no_openspec_root' } })); + markOutcome(new Error('a later, less specific error')); + await complete(1); + expect(completions()[0].properties.error_class).toBe('no_root'); + }); + + it('treats a failed check as a user error, never our bug', async () => { + markCheckFailed(); + await complete(1); + expect(completions()[0].properties).toMatchObject({ + outcome: 'user_error', + error_class: 'validation_failed', + }); + }); + + it('reports an unclassified failure without blaming the user or us falsely', async () => { + await complete(1); + // Neither `none` nor `internal_error`: the CLI has many paths that set an + // exit code without throwing, and calling those our bugs would drown the + // metric that exists to find real ones. + expect(completions()[0].properties).toMatchObject({ + outcome: 'user_error', + error_class: 'unclassified', + }); + }); + + it('reads a Ctrl-C exit as cancelled', async () => { + await complete(130); + expect(completions()[0].properties).toMatchObject({ + outcome: 'cancelled', + error_class: 'cancelled', + exit_code: '130', + }); + }); + + it('excludes time spent at a prompt from the duration', async () => { + markPromptOpen(); + expect(wasPrompted()).toBe(true); + // Simulate a long think by rewinding nothing: the window is what counts. + markPromptClosed(); + await complete(0); + expect(completions()[0].properties.duration).toBe('1_under_100ms'); + }); + + it('resets per-run state so a classification cannot leak into the next run', async () => { + markFailure('archive_blocked'); + await complete(1); + expect(completions()[0].properties.error_class).toBe('archive_blocked'); + + resetEventCount(); + resetState(); + beginRun('1.2.3'); + fetchSpy.mockClear(); + await complete(0); + expect(completions()[0].properties).toMatchObject({ + outcome: 'success', + error_class: 'none', + }); + }); + + it('marks a milestone only for the commands that earn one', async () => { + markMilestone('list'); + await complete(0); + const milestones = fetchSpy.mock.calls + .map(([, o]) => JSON.parse(String((o as RequestInit).body)).batch[0]) + .filter((e) => e.event === 'milestone_reached') + .map((e) => e.properties.milestone); + // `install` is earned by the run that mints the id, and is the funnel's + // denominator. `list` earns nothing. + expect(milestones).toEqual(['install']); + }); + + describe('registerAllowlists', () => { + it('accepts every registered command path and nothing else', () => { + const program = new Command('openspec'); + const group = program.command('store'); + group.command('doctor'); + program.command('archive'); + + const accepted: string[] = []; + registerAllowlists(program, ['claude']); + // Round-trip through the sanitizer, which is what actually gates sends. + for (const candidate of ['archive', 'store', 'store:doctor', 'rm -rf /', 'openspec']) { + if (sanitizeProperties({ command: candidate }).command === candidate) { + accepted.push(candidate); + } + } + expect(accepted.sort()).toEqual(['archive', 'store', 'store:doctor']); + }); + }); +}); diff --git a/test/telemetry/context.test.ts b/test/telemetry/context.test.ts new file mode 100644 index 0000000000..d1cc2e8b41 --- /dev/null +++ b/test/telemetry/context.test.ts @@ -0,0 +1,101 @@ +import { describe, it, expect } from 'vitest'; +import { promises as fs } from 'fs'; +import os from 'os'; +import path from 'path'; +import { detectInvoker, detectInstallKind, collectRunContext } from '../../src/telemetry/context.js'; +import { sanitizeProperties, setRegistryChecks } from '../../src/telemetry/properties.js'; + +setRegistryChecks({ isCommand: () => true, isTool: () => true }); + +describe('detectInvoker', () => { + it('names a recognized agent', () => { + expect(detectInvoker({ CLAUDECODE: '1' }, false)).toBe('claude_code'); + expect(detectInvoker({ CURSOR_TRACE_ID: 'abc' }, false)).toBe('cursor'); + }); + + it('falls back to terminal when stdout is a tty', () => { + expect(detectInvoker({}, true)).toBe('terminal'); + }); + + it('collapses an unrecognized environment to unknown', () => { + expect(detectInvoker({ ACME_INTERNAL_AGENT: '1' }, false)).toBe('unknown'); + }); + + it('ignores an empty marker', () => { + expect(detectInvoker({ CLAUDECODE: '' }, true)).toBe('terminal'); + }); +}); + +describe('detectInstallKind', () => { + it('recognizes npx, a checkout, and a global install', () => { + expect(detectInstallKind('/Users/j/.npm/_npx/abc/node_modules/openspec', {})).toBe('npx'); + expect(detectInstallKind(null, { npm_command: 'exec' })).toBe('npx'); + expect(detectInstallKind('/usr/local/lib/node_modules/openspec', {})).toBe('global'); + expect(detectInstallKind(null, {})).toBe('other'); + }); +}); + +describe('collectRunContext', () => { + it('buckets the change count and keeps no names', async () => { + const root = await fs.mkdtemp(path.join(os.tmpdir(), 'openspec-ctx-')); + const changes = path.join(root, 'changes'); + await fs.mkdir(path.join(changes, 'acme-billing-rewrite'), { recursive: true }); + await fs.mkdir(path.join(changes, 'secret-project-x'), { recursive: true }); + await fs.mkdir(path.join(changes, 'archive'), { recursive: true }); + + const context = await collectRunContext({ + projectRoot: root, + stdoutIsTty: false, + jsonMode: true, + prompted: false, + firstRun: false, + storeInUse: true, + toolCount: 3, + env: {}, + }); + + expect(context.changes).toBe('01-03'); + expect(context.tools_count).toBe('2-3'); + expect(context.store_in_use).toBe(true); + + const serialized = JSON.stringify(sanitizeProperties(context)); + expect(serialized).not.toContain('acme-billing-rewrite'); + expect(serialized).not.toContain('secret-project-x'); + expect(serialized).not.toContain(root); + + await fs.rm(root, { recursive: true, force: true }); + }); + + it('omits the count when the directory cannot be read', async () => { + const context = await collectRunContext({ + projectRoot: '/nonexistent-openspec-root', + stdoutIsTty: true, + jsonMode: false, + prompted: false, + firstRun: true, + storeInUse: false, + env: {}, + }); + expect(context.changes).toBeUndefined(); + // The event still carries everything else. + expect(context.platform).toBeDefined(); + expect(context.first_run).toBe(true); + }); + + it('produces only allowlisted properties', async () => { + const context = await collectRunContext({ + stdoutIsTty: false, + jsonMode: false, + prompted: true, + firstRun: false, + storeInUse: false, + schemaSource: 'user', + toolCount: 0, + env: { CLAUDECODE: '1' }, + }); + // Nothing is dropped: every key collectRunContext emits is on the allowlist. + expect(sanitizeProperties(context)).toEqual(context); + expect(context.invoker).toBe('claude_code'); + expect(context.schema_source).toBe('user'); + }); +}); diff --git a/test/telemetry/disclosure.test.ts b/test/telemetry/disclosure.test.ts new file mode 100644 index 0000000000..de271896ba --- /dev/null +++ b/test/telemetry/disclosure.test.ts @@ -0,0 +1,91 @@ +import { describe, it, expect } from 'vitest'; +import * as fs from 'node:fs'; +import * as path from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { PROPERTY_KEYS, EVENT_NAMES } from '../../src/telemetry/properties.js'; + +const repoRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '../..'); + +/** + * The disclosure is only meaningful if it matches the code. An unenforced + * documentation requirement decays within two releases, so the allowlist is + * the source and the docs are checked against it. + */ +describe('telemetry disclosure parity', () => { + const readme = fs.readFileSync(path.join(repoRoot, 'README.md'), 'utf-8'); + + it('documents every property that can be sent', () => { + // The `$`-prefixed keys are transport switches that turn collection OFF + // ($ip: null, $geoip_disable: true), not data about the user; they are + // described in prose rather than as table rows. Everything else must be + // named. + const undocumented = PROPERTY_KEYS.filter((key) => !key.startsWith('$')).filter( + (key) => !readme.includes(`\`${key}\``) + ); + expect(undocumented).toEqual([]); + }); + + it('names the local verification path and the opt-out', () => { + expect(readme).toContain('OPENSPEC_TELEMETRY_DEBUG=1'); + expect(readme).toContain('openspec config set telemetry.enabled false'); + expect(readme).toContain('OPENSPEC_TELEMETRY=0'); + }); + + it('states a deletion path that exists, and claims no retention it has not set', () => { + expect(readme.toLowerCase()).toContain('deleting the id'); + // No retention promise until one is actually configured in the backend. + // A published period nobody set is a claim the code cannot keep. + expect(readme, 'README promises a retention period — confirm it is configured').not.toMatch( + /retained for \d+ months/ + ); + // A deletion route has to be one the project actually operates. An + // invented address is a worse privacy posture than none, and it would + // bounce silently. + const contacts = readme.match(/[\w.+-]+@[\w.-]+\.\w+/g) ?? []; + expect(contacts, 'README offers an email contact — confirm the mailbox exists').toEqual([]); + expect(readme).toContain('github.com/Fission-AI/OpenSpec/issues/new'); + }); + + it('does not describe the data as anonymous', () => { + // A persistent random id plus device characteristics is pseudonymous. + // Overstating it is what would undermine every other claim on the page. + const telemetrySection = readme.slice(readme.indexOf('Telemetry')); + const section = telemetrySection.slice(0, telemetrySection.indexOf('
')); + expect(section).not.toMatch(/anonymous usage/i); + expect(section).toContain('pseudonymous'); + }); + + it('keeps SECURITY.md honest about what changed', () => { + const security = fs.readFileSync(path.join(repoRoot, 'SECURITY.md'), 'utf-8'); + // The previous commitment was "no environment". It has to be named, not + // quietly edited away. + expect(security).toContain('more than earlier releases collected'); + expect(security).toContain('OPENSPEC_TELEMETRY_DEBUG=1'); + }); + + it('keeps the FAQ and CLI reference from contradicting the disclosure', () => { + // Three documents describe telemetry. A parity test that reads two of them + // lets the third go stale, which is how the FAQ kept claiming "command + // names and version only" after that stopped being true. + for (const file of ['docs/faq.md', 'docs/cli.md']) { + const text = fs.readFileSync(path.join(repoRoot, file), 'utf-8'); + expect(text, `${file} still calls the data anonymous`).not.toMatch( + /anonymous usage stats/i + ); + expect(text, `${file} still claims only names and version`).not.toMatch( + /command names and version only/i + ); + } + }); + + it('names every event it can send', () => { + // Not a tautology: the count is pinned so a new event forces a decision + // about disclosing it. + expect([...EVENT_NAMES].sort()).toEqual([ + 'command_completed', + 'command_executed', + 'milestone_reached', + 'tool_configured', + ]); + }); +}); diff --git a/test/telemetry/e2e.test.ts b/test/telemetry/e2e.test.ts new file mode 100644 index 0000000000..3cde2b9a2e --- /dev/null +++ b/test/telemetry/e2e.test.ts @@ -0,0 +1,137 @@ +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { spawnSync } from 'node:child_process'; +import * as fs from 'node:fs'; +import * as path from 'node:path'; +import * as os from 'node:os'; +import { fileURLToPath } from 'node:url'; + +const repoRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '../..'); +const cli = path.join(repoRoot, 'bin', 'openspec.js'); + +/** + * These run the real binary. The behaviors here — which exit paths report, what + * reaches stdout, what lands on disk — are properties of the wiring, and a + * unit test of the telemetry module cannot see any of them. + */ +describe('telemetry end to end', () => { + let home: string; + + function run(args: string[], env: Record = {}) { + const cwd = path.join(home, 'proj'); + // spawnSync, not execFileSync: a successful run's stderr is where every + // telemetry payload lands, and execFileSync only hands back stdout. + const result = spawnSync(process.execPath, [cli, ...args], { + cwd, + encoding: 'utf-8', + env: { + ...process.env, + HOME: home, + USERPROFILE: home, + XDG_CONFIG_HOME: path.join(home, '.config'), + XDG_DATA_HOME: path.join(home, '.local'), + CI: '', + NO_COLOR: '1', + // vitest.config.ts force-disables telemetry for every worker so the + // suite never writes a developer's real config or posts an event. + // These runs are the telemetry test, and use a throwaway HOME. + OPENSPEC_TELEMETRY: '', + DO_NOT_TRACK: '', + ...env, + }, + }); + return { code: result.status ?? 0, stdout: result.stdout ?? '', stderr: result.stderr ?? '' }; + } + + function debugRun(args: string[]) { + const result = run(args, { OPENSPEC_TELEMETRY_DEBUG: '1' }); + const events = result.stderr + .split('\n') + .filter((line) => line.startsWith('[openspec telemetry] ')) + .map((line) => JSON.parse(line.replace('[openspec telemetry] ', ''))); + return { ...result, events }; + } + + beforeAll(() => { + home = fs.mkdtempSync(path.join(os.tmpdir(), 'openspec-e2e-tel-')); + fs.mkdirSync(path.join(home, 'proj'), { recursive: true }); + // No build here: the suite already runs against a built dist/, and building + // inside a hook blows the hook timeout on CI. + }); + + afterAll(() => fs.rmSync(home, { recursive: true, force: true })); + + it('reports an unknown command, which used to emit nothing at all', () => { + const { events, code } = debugRun(['proposal']); + const completed = events.filter((e) => e.event === 'command_completed'); + expect(completed).toHaveLength(1); + expect(completed[0].properties.error_class).toBe('bad_usage'); + expect(code).toBe(1); + }); + + it('reports an unknown flag and a group with no subcommand', () => { + for (const args of [['list', '--bogus'], ['spec']]) { + const { events } = debugRun(args); + expect(events.find((e) => e.event === 'command_completed')?.properties.error_class).toBe( + 'bad_usage' + ); + } + }); + + it('emits nothing for --help and --version', () => { + expect(debugRun(['--help']).events).toHaveLength(0); + expect(debugRun(['--version']).events).toHaveLength(0); + expect(debugRun(['help']).events).toHaveLength(0); + }); + + it('sends exactly one completion per invocation', () => { + for (const args of [['schemas'], ['list'], ['proposal'], ['spec']]) { + const { events } = debugRun(args); + expect(events.filter((e) => e.event === 'command_completed'), args.join(' ')).toHaveLength(1); + } + }); + + it('keeps stdout a single valid JSON document while inspecting telemetry', () => { + const { stdout } = debugRun(['list', '--json']); + expect(() => JSON.parse(stdout)).not.toThrow(); + }); + + it('never lets a project name reach a payload', () => { + const named = path.join(home, 'proj', 'openspec', 'changes', 'acme-billing-rewrite'); + fs.mkdirSync(named, { recursive: true }); + const { events } = debugRun(['list']); + const serialized = JSON.stringify(events); + expect(serialized).not.toContain('acme-billing-rewrite'); + expect(serialized).not.toContain(home); + // The count is still reported, as a bucket. + expect(events.find((e) => e.event === 'command_completed')?.properties.changes).toBe('01-03'); + fs.rmSync(named, { recursive: true, force: true }); + }); + + it('writes nothing at all while inspecting or opted out', () => { + const configDir = path.join(home, '.config-probe'); + for (const env of [{ OPENSPEC_TELEMETRY_DEBUG: '1' }, { OPENSPEC_TELEMETRY: '0' }]) { + fs.rmSync(configDir, { recursive: true, force: true }); + run(['schemas'], { ...env, XDG_CONFIG_HOME: configDir }); + expect(fs.existsSync(path.join(configDir, 'openspec', 'config.json')), JSON.stringify(env)).toBe( + false + ); + } + }); + + it('shows the user their own telemetry state', () => { + run(['schemas']); // mint an id + const { stdout } = run(['config', 'get', 'telemetry']); + const state = JSON.parse(stdout); + expect(state.enabled).toBe(true); + expect(state.anonymousId).toMatch(/^[0-9a-f-]{36}$/); + expect(fs.existsSync(state.configPath)).toBe(true); + }); + + it('preserves exit codes on every reported path', () => { + expect(run(['--version']).code).toBe(0); + expect(run(['--help']).code).toBe(0); + expect(run(['proposal']).code).toBe(1); + expect(run(['list', '--bogus']).code).toBe(1); + expect(run(['spec']).code).toBe(1); + }); +}); diff --git a/test/telemetry/events.test.ts b/test/telemetry/events.test.ts new file mode 100644 index 0000000000..6b6c4a0cdb --- /dev/null +++ b/test/telemetry/events.test.ts @@ -0,0 +1,317 @@ +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import * as fs from 'node:fs'; +import * as path from 'node:path'; +import * as os from 'node:os'; +import { + resetEventCount, + resetState, + shutdown, + trackCommand, + trackCompletion, + trackMilestone, + trackConfiguredTools, +} from '../../src/telemetry/index.js'; +import { getTelemetryConfig } from '../../src/telemetry/config.js'; +import { setRegistryChecks } from '../../src/telemetry/properties.js'; +import { resetRunId } from '../../src/telemetry/state.js'; + +describe('telemetry events', () => { + let tempDir: string; + let originalEnv: NodeJS.ProcessEnv; + let fetchSpy: ReturnType>; + let errorSpy: ReturnType; + let logSpy: ReturnType; + + beforeEach(() => { + originalEnv = { ...process.env }; + tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'openspec-events-')); + process.env.XDG_CONFIG_HOME = tempDir; + process.env.HOME = tempDir; + process.env.USERPROFILE = tempDir; + process.env.APPDATA = path.join(tempDir, 'appdata'); + delete process.env.OPENSPEC_TELEMETRY; + delete process.env.DO_NOT_TRACK; + delete process.env.CI; + delete process.env.OPENSPEC_TELEMETRY_DEBUG; + + resetEventCount(); + resetState(); + resetRunId(); + setRegistryChecks({ isCommand: () => true, isTool: (v) => ['claude', 'cursor'].includes(v) }); + + fetchSpy = vi.spyOn(globalThis, 'fetch').mockResolvedValue(new Response(null, { status: 200 })); + errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); + logSpy = vi.spyOn(console, 'log').mockImplementation(() => {}); + }); + + afterEach(async () => { + await shutdown(); + process.env = originalEnv; + fs.rmSync(tempDir, { recursive: true, force: true }); + vi.restoreAllMocks(); + }); + + function sentEvents() { + return fetchSpy.mock.calls.map( + ([, options]) => JSON.parse(String((options as RequestInit).body)).batch[0] + ); + } + + it('sends a completion event with a bucketed exit code and duration', async () => { + await trackCompletion({ + command: 'archive', + version: '1.2.3', + outcome: 'user_error', + errorClass: 'archive_blocked', + exitCode: 1, + durationMs: 1500, + context: { platform: 'darwin', changes: '04-10' }, + }); + await shutdown(); + + const [event] = sentEvents(); + expect(event.event).toBe('command_completed'); + expect(event.properties).toMatchObject({ + command: 'archive', + outcome: 'user_error', + error_class: 'archive_blocked', + exit_code: '1', + duration: '3_500ms-2s', + previous_outcome: 'none', + previous_command_same: false, + platform: 'darwin', + changes: '04-10', + }); + }); + + it('buckets a child process exit code rather than sending it', async () => { + await trackCompletion({ + command: 'workset:open', + version: '1.2.3', + outcome: 'internal_error', + errorClass: 'external_tool_failed', + exitCode: 137, + durationMs: 50, + }); + await shutdown(); + + const body = String((fetchSpy.mock.calls[0][1] as RequestInit).body); + expect(JSON.parse(body).batch[0].properties.exit_code).toBe('other'); + expect(body).not.toContain('137'); + }); + + it('reports a retry against the previous run', async () => { + await trackCompletion({ + command: 'archive', + version: '1.2.3', + outcome: 'user_error', + errorClass: 'archive_blocked', + exitCode: 1, + durationMs: 10, + }); + resetState(); + resetEventCount(); + + await trackCompletion({ + command: 'archive', + version: '1.2.3', + outcome: 'success', + errorClass: 'none', + exitCode: 0, + durationMs: 10, + }); + await shutdown(); + + const last = sentEvents().at(-1); + expect(last.properties.previous_outcome).toBe('user_error'); + expect(last.properties.previous_command_same).toBe(true); + }); + + it('sends a milestone once and carries no run context on tool events', async () => { + await trackMilestone('archive', '1.2.3'); + await trackConfiguredTools(['claude', 'cursor'], '1.2.3'); + await shutdown(); + + const events = sentEvents(); + const milestone = events.find((e) => e.event === 'milestone_reached'); + expect(milestone.properties.milestone).toBe('archive'); + expect(milestone.properties.time_to_reach).toBe('1_under_1h'); + + const tools = events.filter((e) => e.event === 'tool_configured'); + expect(tools.map((e) => e.properties.tool).sort()).toEqual(['claude', 'cursor']); + for (const tool of tools) { + expect(tool.properties.platform).toBeUndefined(); + expect(tool.properties.changes).toBeUndefined(); + expect(tool.properties.install_kind).toBeUndefined(); + } + + resetState(); + resetEventCount(); + fetchSpy.mockClear(); + await trackMilestone('archive', '1.2.3'); + await trackConfiguredTools(['claude', 'cursor'], '1.2.3'); + await shutdown(); + expect(fetchSpy).not.toHaveBeenCalled(); + }); + + it('drops a tool id that is not in the registry', async () => { + await trackConfiguredTools(['acme-internal-agent'], '1.2.3'); + await shutdown(); + expect(fetchSpy).not.toHaveBeenCalled(); + }); + + it('caps the events one invocation may send', async () => { + for (let i = 0; i < 40; i += 1) { + await trackCommand('list', '1.2.3'); + } + await shutdown(); + expect(fetchSpy.mock.calls.length).toBeLessThanOrEqual(12); + }); + + it('never spends a one-shot claim it cannot send', async () => { + // Fill the cap with ordinary events, then try to claim a milestone. + for (let i = 0; i < 20; i += 1) { + await trackCommand('list', '1.2.3'); + } + fetchSpy.mockClear(); + + await trackMilestone('archive', '1.2.3'); + await trackConfiguredTools(['claude'], '1.2.3'); + await shutdown(); + + expect(fetchSpy).not.toHaveBeenCalled(); + // Nothing was marked reported, so a later run still sends them. A claim + // persisted without its event would under-report for the life of the + // install. + const telemetry = await getTelemetryConfig(); + expect(telemetry.milestones ?? []).not.toContain('archive'); + expect(telemetry.reportedTools ?? []).not.toContain('claude'); + }); + + describe('debug mode', () => { + beforeEach(() => { + process.env.OPENSPEC_TELEMETRY_DEBUG = '1'; + }); + + it('prints to stderr and sends nothing', async () => { + await trackCompletion({ + command: 'init', + version: '1.2.3', + outcome: 'success', + errorClass: 'none', + exitCode: 0, + durationMs: 5, + }); + await shutdown(); + + expect(fetchSpy).not.toHaveBeenCalled(); + expect(logSpy).not.toHaveBeenCalled(); + const printed = errorSpy.mock.calls.map((c) => String(c[0])).join('\n'); + expect(printed).toContain('command_completed'); + expect(printed).toContain('"outcome":"success"'); + }); + + it('works when telemetry is disabled', async () => { + process.env.OPENSPEC_TELEMETRY = '0'; + await trackCommand('list', '1.2.3'); + await shutdown(); + + expect(fetchSpy).not.toHaveBeenCalled(); + expect(errorSpy.mock.calls.map((c) => String(c[0])).join('\n')).toContain('command_executed'); + }); + + it('creates no anonymous id and spends no milestone claim', async () => { + await trackCommand('list', '1.2.3'); + await trackMilestone('init', '1.2.3'); + await shutdown(); + + await trackCompletion({ + command: 'list', + version: '1.2.3', + outcome: 'success', + errorClass: 'none', + exitCode: 0, + durationMs: 5, + }); + + // Inspecting writes nothing at all — not the id, not the milestone + // claim, not the retry record. + const telemetry = await getTelemetryConfig(); + expect(telemetry).toEqual({}); + + const printed = errorSpy.mock.calls.map((c) => String(c[0])).join('\n'); + expect(printed).toContain('00000000-0000-0000-0000-000000000000'); + }); + }); + + describe('opt-out', () => { + it('writes nothing to config and sends nothing', async () => { + process.env.OPENSPEC_TELEMETRY = '0'; + await trackCommand('list', '1.2.3'); + await trackCompletion({ + command: 'list', + version: '1.2.3', + outcome: 'success', + errorClass: 'none', + exitCode: 0, + durationMs: 5, + }); + await trackMilestone('init', '1.2.3'); + await trackConfiguredTools(['claude'], '1.2.3'); + await shutdown(); + + expect(fetchSpy).not.toHaveBeenCalled(); + expect(await getTelemetryConfig()).toEqual({}); + }); + }); +}); + +describe('cancellation', () => { + let tempDir: string; + let originalEnv: NodeJS.ProcessEnv; + let fetchSpy: ReturnType>; + + beforeEach(() => { + originalEnv = { ...process.env }; + tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'openspec-cancel-')); + process.env.XDG_CONFIG_HOME = tempDir; + process.env.HOME = tempDir; + process.env.USERPROFILE = tempDir; + delete process.env.OPENSPEC_TELEMETRY; + delete process.env.DO_NOT_TRACK; + delete process.env.CI; + delete process.env.OPENSPEC_TELEMETRY_DEBUG; + resetEventCount(); + resetState(); + resetRunId(); + setRegistryChecks({ isCommand: () => true, isTool: () => true }); + }); + + afterEach(() => { + process.env = originalEnv; + fs.rmSync(tempDir, { recursive: true, force: true }); + vi.restoreAllMocks(); + }); + + it('does not wait on the flush when the user pressed Ctrl-C', async () => { + // A request that never settles: if shutdown awaited it, this test would + // hang rather than fail, so the assertion is the timing itself. + fetchSpy = vi.spyOn(globalThis, 'fetch').mockImplementation( + () => new Promise(() => {}) + ); + + await trackCompletion({ + command: 'archive', + version: '1.2.3', + outcome: 'cancelled', + errorClass: 'cancelled', + exitCode: 130, + durationMs: 20, + }); + + const startedAt = Date.now(); + await shutdown(); + expect(Date.now() - startedAt).toBeLessThan(100); + expect(fetchSpy).toHaveBeenCalledTimes(1); + }); +}); diff --git a/test/telemetry/index.test.ts b/test/telemetry/index.test.ts index 7db56ddeed..d10af0c69e 100644 --- a/test/telemetry/index.test.ts +++ b/test/telemetry/index.test.ts @@ -3,8 +3,17 @@ import * as fs from 'node:fs'; 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'; +import { + isTelemetryEnabled, + maybeShowTelemetryNotice, + resetEventCount, + resetState, + shutdown, + trackCommand, +} from '../../src/telemetry/index.js'; +import { getTelemetryConfig, updateTelemetryConfig } from '../../src/telemetry/config.js'; +import { setRegistryChecks } from '../../src/telemetry/properties.js'; +import { resetRunId } from '../../src/telemetry/state.js'; describe('telemetry/index', () => { let tempDir: string; @@ -28,6 +37,13 @@ describe('telemetry/index', () => { // Clear all mocks vi.clearAllMocks(); + // Module-level per-invocation state: a real CLI run starts fresh, so each + // test must too, or the event cap leaks across tests. + resetEventCount(); + resetState(); + resetRunId(); + setRegistryChecks({ isCommand: () => true, isTool: () => true }); + // Notice is written to stderr so it never pollutes stdout (raw/JSON output) consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); // Telemetry must never reach the real network in tests @@ -185,10 +201,25 @@ describe('telemetry/index', () => { await maybeShowTelemetryNotice(); expect(consoleErrorSpy).toHaveBeenCalledTimes(1); expect(consoleErrorSpy).toHaveBeenCalledWith( - expect.stringContaining('OpenSpec collects anonymous usage stats') + expect.stringContaining('OpenSpec collects pseudonymous usage stats') + ); + + // The notice version is now persisted: a second run stays quiet. + await maybeShowTelemetryNotice(); + expect(consoleErrorSpy).toHaveBeenCalledTimes(1); + }); + + it('tells a user who saw the earlier scope what changed, once', async () => { + enableTelemetry(); + // Someone who accepted the old disclosure: told, but about less. + await updateTelemetryConfig({ noticeSeen: true }); + + await maybeShowTelemetryNotice(); + expect(consoleErrorSpy).toHaveBeenCalledTimes(1); + expect(consoleErrorSpy).toHaveBeenCalledWith( + expect.stringContaining('now also record whether a command succeeded') ); - // noticeSeen is now persisted: a second run stays quiet. await maybeShowTelemetryNotice(); expect(consoleErrorSpy).toHaveBeenCalledTimes(1); }); @@ -207,7 +238,7 @@ describe('telemetry/index', () => { await maybeShowTelemetryNotice(); expect(consoleErrorSpy).toHaveBeenCalledTimes(1); expect(consoleErrorSpy).toHaveBeenCalledWith( - expect.stringContaining('OpenSpec collects anonymous usage stats') + expect.stringContaining('OpenSpec collects pseudonymous usage stats') ); }); }); @@ -254,9 +285,15 @@ describe('telemetry/index', () => { expect(event.properties).toEqual({ command: 'test', version: '1.0.0', + version_code: 1000000, surface: 'cli', + $geoip_disable: true, + run_id: expect.stringMatching(/^[0-9a-f-]{36}$/), + work_session_id: expect.stringMatching(/^[0-9a-f-]{36}$/), $ip: null, }); + // The start event carries no run context: that lives on command_completed. + expect(event.properties.platform).toBeUndefined(); }); it('should bound the request with a timeout signal', async () => { diff --git a/test/telemetry/properties.test.ts b/test/telemetry/properties.test.ts new file mode 100644 index 0000000000..a1aa038efc --- /dev/null +++ b/test/telemetry/properties.test.ts @@ -0,0 +1,175 @@ +import { describe, it, expect, beforeEach } from 'vitest'; +import { + sanitizeProperties, + setRegistryChecks, + bucketCount, + bucketToolCount, + bucketDuration, + bucketExitCode, + bucketTimeToReach, + bucketNodeMajor, + bucketPlatform, + isEventName, + PROPERTY_KEYS, + ERROR_CLASSES, +} from '../../src/telemetry/properties.js'; + +const RUN_ID = '3f2504e0-4f89-11d3-9a0c-0305e82c3301'; + +beforeEach(() => { + setRegistryChecks({ + isCommand: (value) => ['archive', 'change:validate', 'init'].includes(value), + isTool: (value) => ['claude', 'cursor'].includes(value), + }); +}); + +describe('sanitizeProperties', () => { + it('keeps allowlisted properties', () => { + expect( + sanitizeProperties({ + command: 'archive', + outcome: 'success', + error_class: 'none', + exit_code: '0', + duration: '2_100-500ms', + run_id: RUN_ID, + $ip: null, + }) + ).toEqual({ + command: 'archive', + outcome: 'success', + error_class: 'none', + exit_code: '0', + duration: '2_100-500ms', + run_id: RUN_ID, + $ip: null, + }); + }); + + it('drops a key built from a user-authored name', () => { + // The hole a value-only contract would leave: the value is a boolean, and + // the user's schema name still ships in the key. + const clean = sanitizeProperties({ + outcome: 'success', + 'schema:acme-internal': true, + 'change:billing-rewrite': 1, + }); + expect(clean).toEqual({ outcome: 'success' }); + expect(JSON.stringify(clean)).not.toContain('acme-internal'); + expect(JSON.stringify(clean)).not.toContain('billing-rewrite'); + }); + + it('drops an allowlisted key holding an out-of-set value', () => { + expect( + sanitizeProperties({ + outcome: 'exploded', + error_class: 'store_unreachable_at_/Users/jane/work', + platform: 'sunos', + command: 'archive', + }) + ).toEqual({ command: 'archive' }); + }); + + it('never lets a raw exit code or duration through', () => { + expect(sanitizeProperties({ exit_code: 137, duration: 41822 })).toEqual({}); + }); + + it('keeps the event sendable when a property is dropped', () => { + const clean = sanitizeProperties({ outcome: 'success', bogus: 'x' }); + expect(clean.outcome).toBe('success'); + }); + + it('drops undefined values rather than sending null', () => { + expect(sanitizeProperties({ outcome: 'success', changes: undefined })).toEqual({ + outcome: 'success', + }); + }); + + it('checks command and tool ids for registry membership', () => { + expect(sanitizeProperties({ command: 'archive' })).toEqual({ command: 'archive' }); + expect(sanitizeProperties({ command: 'unknown' })).toEqual({ command: 'unknown' }); + expect(sanitizeProperties({ command: 'rm -rf /' })).toEqual({}); + expect(sanitizeProperties({ tool: 'cursor' })).toEqual({ tool: 'cursor' }); + expect(sanitizeProperties({ tool: 'acme-internal-agent' })).toEqual({}); + }); + + it('rejects a run id that is not a uuid', () => { + expect(sanitizeProperties({ run_id: '/Users/jane/project' })).toEqual({}); + }); + + it('accepts only null for $ip, and only true for the geoip switch', () => { + expect(sanitizeProperties({ $ip: '203.0.113.4' })).toEqual({}); + expect(sanitizeProperties({ $geoip_disable: true })).toEqual({ $geoip_disable: true }); + // Never false: the switch exists to be on. + expect(sanitizeProperties({ $geoip_disable: false })).toEqual({}); + }); +}); + +describe('event names', () => { + it('rejects a name built from a runtime value', () => { + expect(isEventName('milestone_reached')).toBe(true); + expect(isEventName('milestone_reached:acme-billing')).toBe(false); + }); +}); + +describe('buckets', () => { + it('buckets counts', () => { + expect(bucketCount(0)).toBe('00'); + expect(bucketCount(3)).toBe('01-03'); + expect(bucketCount(11)).toBe('11-30'); + expect(bucketCount(3500)).toBe('31+'); + }); + + it('buckets tool counts', () => { + expect(bucketToolCount(0)).toBe('0'); + expect(bucketToolCount(1)).toBe('1'); + expect(bucketToolCount(3)).toBe('2-3'); + expect(bucketToolCount(9)).toBe('4+'); + }); + + it('buckets durations', () => { + expect(bucketDuration(0)).toBe('1_under_100ms'); + expect(bucketDuration(499)).toBe('2_100-500ms'); + expect(bucketDuration(1999)).toBe('3_500ms-2s'); + expect(bucketDuration(60_000)).toBe('5_over_10s'); + }); + + it('buckets exit codes, including passed-through child codes', () => { + expect(bucketExitCode(undefined)).toBe('0'); + expect(bucketExitCode(0)).toBe('0'); + expect(bucketExitCode(1)).toBe('1'); + expect(bucketExitCode(130)).toBe('130'); + // workset open returns the editor's status; feedback returns gh's. + expect(bucketExitCode(137)).toBe('other'); + expect(bucketExitCode(2)).toBe('other'); + }); + + it('buckets time to reach a milestone', () => { + const h = 3_600_000; + expect(bucketTimeToReach(0)).toBe('1_under_1h'); + expect(bucketTimeToReach(2 * h)).toBe('2_1-24h'); + expect(bucketTimeToReach(72 * h)).toBe('3_1-7d'); + expect(bucketTimeToReach(24 * h * 20)).toBe('4_8-30d'); + expect(bucketTimeToReach(24 * h * 400)).toBe('5_over_30d'); + }); + + it('buckets node majors and platforms', () => { + expect(bucketNodeMajor('v22.11.0')).toBe('22'); + expect(bucketNodeMajor('v23.0.0-nightly')).toBe('other'); + expect(bucketPlatform('darwin')).toBe('darwin'); + expect(bucketPlatform('sunos')).toBe('other'); + }); +}); + +describe('allowlist shape', () => { + it('declares no property key that looks user-authored', () => { + for (const key of PROPERTY_KEYS) { + expect(key).toMatch(/^[$a-z_]+$/); + } + }); + + it('classifies an unmapped failure as other', () => { + expect(ERROR_CLASSES).toContain('other'); + expect(ERROR_CLASSES).toContain('none'); + }); +}); diff --git a/test/telemetry/state.test.ts b/test/telemetry/state.test.ts new file mode 100644 index 0000000000..6ac62e5111 --- /dev/null +++ b/test/telemetry/state.test.ts @@ -0,0 +1,137 @@ +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import * as fs from 'node:fs'; +import * as path from 'node:path'; +import * as os from 'node:os'; +import { + loadSessionState, + claimMilestone, + claimUnreportedTools, + recordOutcome, + getRunId, + resetRunId, +} from '../../src/telemetry/state.js'; +import { getConfigPath, getTelemetryConfig } from '../../src/telemetry/config.js'; + +describe('telemetry/state', () => { + let tempDir: string; + let originalEnv: NodeJS.ProcessEnv; + + beforeEach(() => { + originalEnv = { ...process.env }; + tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'openspec-state-test-')); + process.env.XDG_CONFIG_HOME = path.join(tempDir, 'config'); + process.env.HOME = tempDir; + process.env.USERPROFILE = tempDir; + resetRunId(); + }); + + afterEach(() => { + for (const key of Object.keys(process.env)) delete process.env[key]; + Object.assign(process.env, originalEnv); + fs.rmSync(tempDir, { recursive: true, force: true }); + }); + + it('mints identity on the first run and reuses it after', async () => { + const first = await loadSessionState(); + expect(first.firstRun).toBe(true); + expect(first.anonymousId).toMatch(/^[0-9a-f-]{36}$/); + + const second = await loadSessionState(); + expect(second.firstRun).toBe(false); + expect(second.anonymousId).toBe(first.anonymousId); + }); + + it('reuses the work session inside the window and rotates outside it', async () => { + const start = new Date('2026-09-10T10:00:00Z'); + const first = await loadSessionState(start); + + const soon = await loadSessionState(new Date(start.getTime() + 10 * 60 * 1000)); + expect(soon.workSessionId).toBe(first.workSessionId); + + // The window slides from the last activity, not from the session start: + // 31 minutes after `start` is only 21 minutes after the `soon` call. + const stillSame = await loadSessionState(new Date(start.getTime() + 31 * 60 * 1000)); + expect(stillSame.workSessionId).toBe(first.workSessionId); + + const later = await loadSessionState(new Date(start.getTime() + 90 * 60 * 1000)); + expect(later.workSessionId).not.toBe(first.workSessionId); + }); + + it('gives each invocation its own run id, unrelated to identity', async () => { + const state = await loadSessionState(); + const runId = getRunId(); + expect(getRunId()).toBe(runId); + expect(runId).not.toBe(state.anonymousId); + expect(runId).not.toBe(state.workSessionId); + + resetRunId(); + expect(getRunId()).not.toBe(runId); + }); + + it('never writes the run id to disk', async () => { + await loadSessionState(); + getRunId(); + const onDisk = fs.readFileSync(getConfigPath(), 'utf-8'); + expect(onDisk).not.toContain(getRunId()); + }); + + it('claims a milestone once and buckets the time to reach it', async () => { + const start = new Date('2026-09-10T10:00:00Z'); + const state = await loadSessionState(start); + + const first = await claimMilestone('archive', state, new Date(start.getTime() + 30 * 60 * 1000)); + expect(first).toEqual({ timeToReach: '1_under_1h' }); + + const again = await claimMilestone('archive', state, new Date()); + expect(again).toBeNull(); + }); + + it('omits the bucket for an id that predates the recorded time', async () => { + await loadSessionState(); + // Simulate a user whose anonymousId was minted before firstSeenAt existed. + const configPath = getConfigPath(); + const raw = JSON.parse(fs.readFileSync(configPath, 'utf-8')); + delete raw.telemetry.firstSeenAt; + fs.writeFileSync(configPath, JSON.stringify(raw)); + + const state = await loadSessionState(); + expect(await claimMilestone('propose', state)).toEqual({ timeToReach: undefined }); + }); + + it('reports each tool once', async () => { + const state = await loadSessionState(); + expect(await claimUnreportedTools(['claude', 'cursor'], state)).toEqual(['claude', 'cursor']); + expect(await claimUnreportedTools(['claude', 'cursor'], state)).toEqual([]); + expect(await claimUnreportedTools(['claude', 'zed'], state)).toEqual(['zed']); + }); + + it('records the previous outcome for retry visibility', async () => { + await loadSessionState(); + await recordOutcome('archive', 'user_error'); + + const next = await loadSessionState(); + expect(next.previousOutcome).toBe('user_error'); + expect(next.previousCommand).toBe('archive'); + }); + + it('persists only bounded fields', async () => { + const state = await loadSessionState(); + await claimMilestone('init', state); + await claimUnreportedTools(['claude'], state); + await recordOutcome('init', 'success'); + + const telemetry = await getTelemetryConfig(); + expect(Object.keys(telemetry).sort()).toEqual( + [ + 'anonymousId', + 'firstSeenAt', + 'lastActivityAt', + 'milestones', + 'previousCommand', + 'previousOutcome', + 'reportedTools', + 'workSessionId', + ].sort() + ); + }); +});