diff --git a/openspec/changes/restructure-cli-telemetry/.openspec.yaml b/openspec/changes/archive/2026-06-13-restructure-cli-telemetry/.openspec.yaml similarity index 100% rename from openspec/changes/restructure-cli-telemetry/.openspec.yaml rename to openspec/changes/archive/2026-06-13-restructure-cli-telemetry/.openspec.yaml diff --git a/openspec/changes/restructure-cli-telemetry/design.md b/openspec/changes/archive/2026-06-13-restructure-cli-telemetry/design.md similarity index 99% rename from openspec/changes/restructure-cli-telemetry/design.md rename to openspec/changes/archive/2026-06-13-restructure-cli-telemetry/design.md index 2c616578..395760e4 100644 --- a/openspec/changes/restructure-cli-telemetry/design.md +++ b/openspec/changes/archive/2026-06-13-restructure-cli-telemetry/design.md @@ -55,7 +55,7 @@ auth login success → cli_authenticated { } auth logout success → cli_logged_out { } init/install success → cli_installed { targets? } onboard complete → cli_onboarded { } -check finishes → cli_check_completed{ errorCount, warningCount, filesScanned } +check finishes → cli_check_completed{ errorCount, warningCount, findings } any command fails → cli_error { command, code } help served → cli_help { topic } (topic = "(index)" for no-arg, the attempted topic otherwise) diff --git a/openspec/changes/restructure-cli-telemetry/proposal.md b/openspec/changes/archive/2026-06-13-restructure-cli-telemetry/proposal.md similarity index 100% rename from openspec/changes/restructure-cli-telemetry/proposal.md rename to openspec/changes/archive/2026-06-13-restructure-cli-telemetry/proposal.md diff --git a/openspec/changes/restructure-cli-telemetry/specs/analytics/spec.md b/openspec/changes/archive/2026-06-13-restructure-cli-telemetry/specs/analytics/spec.md similarity index 100% rename from openspec/changes/restructure-cli-telemetry/specs/analytics/spec.md rename to openspec/changes/archive/2026-06-13-restructure-cli-telemetry/specs/analytics/spec.md diff --git a/openspec/changes/archive/2026-06-13-restructure-cli-telemetry/tasks.md b/openspec/changes/archive/2026-06-13-restructure-cli-telemetry/tasks.md new file mode 100644 index 00000000..aace17da --- /dev/null +++ b/openspec/changes/archive/2026-06-13-restructure-cli-telemetry/tasks.md @@ -0,0 +1,62 @@ +# Tasks + +## Phasing — stacked PRs + +This change is cut into committable phases, each of which leaves the build and +tests green and maps to one stacked PR (Git Town). Tests travel with the phase +that introduces the behavior — there is no trailing "tests" phase. Phases are +ordered so the stack reads bottom → top: + +``` +main +└── docs openspec change contract (proposal/design/specs/tasks) + └── phase 1 cli_run denominator + cli_error (runner) + └── phase 2 rule events (created/improved/deleted) + └── phase 3 auth + lifecycle events (auth/install/onboard/check) + └── phase 4 cli_help { topic } + drop info/detect bespoke events + └── phase 5 finalize: sweep, gate, archive ← tip +``` + +Transitional note: while the stack is mid-flight, a command may briefly emit +both `cli_run` and a soon-to-be-removed legacy event (e.g. after phase 1 but +before phase 2). That dual signal exists only within the unmerged stack; the +hard cut (no dual-emit) holds for the released, fully-merged state. Each phase +keeps the suite green on its own. + +## 1. Phase 1 — cli_run denominator + cli_error (PR 1) + +- [x] 1.1 In `packages/cli/src/index.ts`, wrap command execution so exactly one `cli_run` is emitted per invocation from a `finally`-equivalent path, with `{ command, cli_version, success, durationMs, anonymous, loggedIn }` +- [x] 1.2 Resolve `command` from the matched citty subcommand (e.g. `"rule create"`, `"help"`); derive `success` from a thrown error / non-zero `process.exitCode`; measure `durationMs` from a start timestamp — extracted to a testable `telemetry-run.ts` (resolveCommandName/resolveCwd/emitRunEvents) so the entry module's side-effecting top level stays untested +- [x] 1.3 Emit `cli_error { command, code }` from the runner's catch path when the failure carries a stable `CLIErrorCode` — added an optional `code` to `CLIError`; falls back to `INTERNAL_ERROR` +- [x] 1.4 Tests: one `cli_run` per invocation (success and failure), and `cli_error` on a known-code failure — `test/cli-run.test.ts` +- [x] 1.5 typecheck + lint + suite green; commit; open PR 1 + +## 2. Phase 2 — rule concrete-state events (PR 2, on PR 1) + +- [x] 2.1 `commands/rules.ts`: remove `cli_rule_create(_completed)`, `cli_rule_improve(_completed)`, `cli_rule_delete(_completed)`, `cli_rule_meta(_completed)`, `cli_rule_verify(_completed)` +- [x] 2.2 Emit `cli_rule_created`, `cli_rule_improved`, `cli_rule_deleted` at the point each state changes (counts/ids/booleans only); `verify`/`meta` are covered by `cli_run` alone (their command-level telemetry was removed entirely) +- [x] 2.3 Update rule command tests to the new events; assert no `cli_rule_*_completed` — only `telemetry.test.ts` referenced an old rule name (a sample), updated to `cli_rule_created`; rule-from/verify tests assert behavior, not events +- [x] 2.4 typecheck + lint + suite green; commit; open PR 2 + +## 3. Phase 3 — auth + lifecycle events (PR 3, on PR 2) + +- [x] 3.1 `commands/auth.ts`: remove `cli_auth_login(_completed)`, `cli_auth_logout(_completed)`, `cli_auth_status(_completed)`; emit `cli_authenticated` (fresh login only) and `cli_logged_out` (token actually removed); status → `cli_run` only +- [x] 3.2 `commands/init.ts` + `wizard/index.ts`: remove `cli_init(_completed)`, `cli_init_cancelled`, `cli_update(_completed)`; emit `cli_installed` on a successful install (interactive + non-interactive + update) +- [x] 3.3 `commands/onboard.ts`: remove `cli_onboard_recipe` / `cli_onboard_already_done`; emit `cli_onboarded` when onboarding is marked complete +- [x] 3.4 `commands/check.ts`: remove `cli_check(_completed)`; emit `cli_check_completed { errorCount, warningCount, findings }` only when a scan actually runs (counts only — no matched code; `findings` replaces the unavailable `filesScanned`) +- [x] 3.5 Update auth/init/onboard/check tests to the new events — wizard-integration assertions updated to `cli_installed` / no-event-on-cancel; telemetry.test sample names → `cli_run` +- [x] 3.6 typecheck + lint + suite green; commit; open PR 3 + +## 4. Phase 4 — cli_help { topic } + drop bespoke info/detect events (PR 4, on PR 3) + +- [x] 4.1 `commands/help.ts`: replace `help_index`, `help_`, `help_unknown` with one `cli_help { topic }` (served topic, `"(index)"` marker for no-arg, the attempted topic for unknown) +- [x] 4.2 `commands/info.ts`: remove bespoke `cli_info(_completed)` (covered by `cli_run`); also drop its now-unused `getTelemetry` import. NOTE: `detect.ts`/`cli_detect` is NOT on this branch's lineage (it lives in the unmerged local-rule-routing stack) — no change needed here; it will be reconciled when that stack and this one both land +- [x] 4.3 Assert `cli_help` carries `topic` and no `help_*` event — added `test/help-telemetry.test.ts` (served topic, index marker, unknown topic, and no legacy `help_*`) +- [x] 4.4 typecheck + lint + suite green; commit; open PR 4 + +## 5. Phase 5 — finalize (PR 5, tip) + +- [x] 5.1 Grep the CLI for any remaining old event names (`_completed`, `help_index`, `help_`, `help_unknown`, legacy `cli_` starts); remove any stragglers — clean; the only `_completed` is the intentional concrete event `cli_check_completed` +- [x] 5.2 Run `pnpm openspec validate restructure-cli-telemetry`; `pnpm typecheck`; `pnpm lint`; full suite green (259) +- [x] 5.3 Manual smoke: `info`, `help check`, `help` (index) run end-to-end after the refactor; concrete events + cli_run/cli_help/cli_error verified by the in-process tests +- [x] 5.4 Archive the change (`openspec archive restructure-cli-telemetry`) so the tip carries the spec sync + dated archive; commit; open PR 5 diff --git a/openspec/specs/analytics/spec.md b/openspec/specs/analytics/spec.md index 6f1173c1..02dc1f4e 100644 --- a/openspec/specs/analytics/spec.md +++ b/openspec/specs/analytics/spec.md @@ -111,13 +111,13 @@ Every `capture()` call SHALL include the `cli` property (anonymous UUID), the `c #### Scenario: Anonymous capture includes standard properties -- **WHEN** `capture("cli_check")` is called without authentication +- **WHEN** `capture("cli_run")` is called without authentication - **THEN** the event SHALL include `{ cli: anonymousUuid, cliVersion: , scaffoldVersion: }` - **AND** the event SHALL NOT include a `groups` parameter #### Scenario: Authenticated capture includes standard properties and group -- **WHEN** `capture("cli_rule_create")` is called with authentication +- **WHEN** `capture("cli_rule_created")` is called with authentication - **THEN** the event SHALL include `{ cli: anonymousUuid, cliVersion: , scaffoldVersion: }` - **AND** the `groups` parameter SHALL include `{ organization: String(orgId) }` @@ -134,56 +134,87 @@ Every `capture()` call SHALL include the `cli` property (anonymous UUID), the `c ### Requirement: CLI events use cli\_ prefix -CLI action events SHALL continue to use the `cli_` prefix, but the event taxonomy SHALL be reorganized as follows: - -- `cli_` — fired when an action command begins execution (e.g. `cli_rule_create`, `cli_rule_improve`, `cli_rule_delete`, `cli_check`, `cli_info`, `cli_init`, `cli_auth_login`, `cli_auth_logout`) -- `cli__completed` — fired when an action command finishes execution; event properties SHALL include `success: boolean`, `durationMs: number`, and `errorCode?: string` (when failure) -- `help_` — fired when the help command serves a specific topic (e.g. `help_rule_create`, `help_check`, `help_auth`); replaces previous `cli_help_` events -- `help_index` — fired when the help command is invoked with no arguments (probable agent confusion / routing failure) -- `help_unknown` — fired when the help command receives an unknown topic; event properties SHALL include `topic: string` (the attempted topic) - -The previous event names `cli_help`, `cli_help_auth`, `cli_help_check`, `cli_help_info`, `cli_help_init`, `cli_help_rule` SHALL be removed in this release. There is no dual-emit window — the rename is a hard cut. - -#### Scenario: Action command emits start and completion events - -- **WHEN** a user runs `taskless rule create --from req.json` -- **THEN** PostHog SHALL receive a `cli_rule_create` event when execution begins -- **AND** SHALL receive a `cli_rule_create_completed` event when execution finishes, with properties including `success`, `durationMs`, and (on failure) `errorCode` - -#### Scenario: Help fetch emits topic intent +CLI events SHALL use the `cli_` prefix, with the taxonomy organized as a +`cli_run` denominator plus concrete state-transition events: + +- `cli_run` — exactly one per invocation (see the dedicated requirement). This + replaces every previous `cli_` start event and `cli__completed` + event; the `success`/`durationMs`/`command` signal lives here. +- Concrete state-transition events, each fired at the point the state actually + changes, carrying counts/ids/booleans only (never rule content, prompts, or + matched source): + - `cli_rule_created`, `cli_rule_improved`, `cli_rule_deleted` + - `cli_authenticated`, `cli_logged_out` + - `cli_installed`, `cli_onboarded` + - `cli_check_completed` — error/warning counts only (e.g. `errorCount`, + `warningCount`, `findings`) + - `cli_error` — a single failure event with `command` and `code` (a stable + `CLIErrorCode`) +- `cli_help` — fired when the help command serves a request, with a `topic` + property (the served topic; the exact literal `"(index)"` when invoked with no + topic; the attempted topic for an unknown request). This replaces the previous + `help_index`, `help_`, and `help_unknown` events. + +Commands that carry no concrete state beyond the invocation (e.g. `info`, +`detect`, `update`, `auth status`, `rule verify`, `rule meta`) SHALL rely on +`cli_run` alone and SHALL NOT emit a bespoke event. The previous taxonomy +(`cli_`, `cli__completed`, `help_index`, `help_`, +`help_unknown`) SHALL be removed in this release; there is no dual-emit window. + +#### Scenario: Rule creation emits a concrete state event plus cli_run + +- **WHEN** a user runs `taskless rule create --from req.json` and a rule is written +- **THEN** PostHog SHALL receive one `cli_run` event with `command: "rule create"` +- **AND** SHALL receive a `cli_rule_created` event +- **AND** SHALL NOT receive `cli_rule_create` or `cli_rule_create_completed` + +#### Scenario: Help fetch emits cli_help with a topic - **WHEN** an agent runs `taskless help rule create` -- **THEN** PostHog SHALL receive a `help_rule_create` event +- **THEN** PostHog SHALL receive a `cli_help` event with `topic: "rule create"` +- **AND** SHALL NOT receive a `help_rule_create` event -#### Scenario: Help no-args emits index event +#### Scenario: Help with no topic emits cli_help with the index marker - **WHEN** an agent runs `taskless help` -- **THEN** PostHog SHALL receive a `help_index` event +- **THEN** PostHog SHALL receive a `cli_help` event with `topic: "(index)"` +- **AND** SHALL NOT receive a `help_index` event -#### Scenario: Help unknown topic emits help_unknown +#### Scenario: A command failure emits cli_error -- **WHEN** an agent runs `taskless help nonexistent` -- **THEN** PostHog SHALL receive a `help_unknown` event with property `topic: "nonexistent"` +- **WHEN** a command fails with a known `CLIErrorCode` +- **THEN** PostHog SHALL receive a `cli_error` event with `command` and `code` #### Scenario: Old event names are not emitted -- **WHEN** any CLI command runs in v0.7.0 -- **THEN** PostHog SHALL NOT receive any event named `cli_help`, `cli_help_`, or any other event under the previous taxonomy +- **WHEN** any CLI command runs in this release +- **THEN** PostHog SHALL NOT receive any event named `cli__completed`, + `help_index`, `help_`, or `help_unknown` ### Requirement: Wrong-topic re-routing is observable as a derivable funnel -The new event taxonomy is structured so that wrong-topic re-routing is a derivable funnel signal: +The taxonomy SHALL keep wrong-topic re-routing derivable as a funnel signal from +the new events: -- A `help_` event followed by no `cli_` event AND a subsequent `help_` event indicates the agent fetched the recipe for topic A, did not act on it, and re-routed to topic B -- A `help_index` event followed by a `help_` event indicates the agent consulted the index before picking a topic (expected behavior; baseline) -- A `help_` event with no subsequent `cli_` event AND no further `help_*` event indicates the agent abandoned the action +- A `cli_help { topic: A }` event not followed by the concrete event for topic A + (or by `cli_run` with the corresponding `command`), and then a subsequent + `cli_help { topic: B }`, indicates the agent fetched recipe A, did not act on + it, and re-routed to topic B. +- A `cli_help` index-marker event followed by a `cli_help { topic }` event + indicates the agent consulted the index before picking a topic (baseline). +- A `cli_help { topic }` event with no subsequent acting `cli_run` and no further + `cli_help` event indicates the agent abandoned the action. -No additional events SHALL be added to capture this signal directly — the funnel is derivable from the event sequence in PostHog. Dashboards SHOULD be created to surface re-routing rates per topic so wrong-topic confusion can be measured. +No additional events SHALL be added to capture this signal directly — it is +derivable from the `cli_help` / `cli_run` sequence. Dashboards SHOULD surface +re-routing rates per topic. #### Scenario: Funnel data supports wrong-topic detection - **WHEN** dashboards are constructed in PostHog -- **THEN** the events SHALL be sufficient to compute "rate of `help_` events not followed by a corresponding `cli_` event within N minutes" +- **THEN** the `cli_help` (with `topic`) and `cli_run` (with `command`) events + SHALL be sufficient to compute "rate of `cli_help { topic }` not followed by a + corresponding acting `cli_run` within N minutes" ### Requirement: Telemetry failures are silent @@ -218,3 +249,26 @@ Each command handler SHALL call `getTelemetry(cwd)` to lazily initialize the sin - **WHEN** the CLI exits without running a command (e.g. showing top-level help) - **THEN** `shutdownTelemetry()` SHALL be a no-op and no PostHog client SHALL be created + +### Requirement: Every invocation emits exactly one cli_run event + +The CLI SHALL emit exactly one `cli_run` event per invocation, from the top-level +runner rather than from individual commands. The event SHALL carry the properties +`command` (the resolved subcommand name, e.g. `"rule create"` or `"help"`), +`cli_version`, `success` (boolean), `durationMs` (number), `anonymous` (boolean), +and `loggedIn` (boolean). The event SHALL be emitted on both success and failure +(from a `finally`-equivalent path), and no command SHALL emit its own +"started" or "ran" event. + +#### Scenario: A successful command emits one cli_run + +- **WHEN** a user runs `taskless info` +- **THEN** PostHog SHALL receive exactly one `cli_run` event with + `command: "info"`, `success: true`, a numeric `durationMs`, and the + `cli_version`, `anonymous`, and `loggedIn` properties +- **AND** SHALL NOT receive a separate `cli_info` or `cli_info_completed` event + +#### Scenario: A failing command still emits cli_run + +- **WHEN** a command exits with an error +- **THEN** PostHog SHALL receive one `cli_run` event with `success: false` diff --git a/packages/cli/src/api/config.ts b/packages/cli/src/api/config.ts index 55e1dea4..6c6ab607 100644 --- a/packages/cli/src/api/config.ts +++ b/packages/cli/src/api/config.ts @@ -6,15 +6,15 @@ import { getConfigDirectory } from "../auth/token"; const DEFAULT_BASE_URL = "https://app.taskless.io/cli"; const CONFIG_FILE = "config.json"; -interface CliConfig { +interface CLIConfig { apiUrl?: string; } -function readConfigFile(): CliConfig | undefined { +function readConfigFile(): CLIConfig | undefined { try { const filePath = join(getConfigDirectory(), CONFIG_FILE); const raw = readFileSync(filePath, "utf8"); - return JSON.parse(raw) as CliConfig; + return JSON.parse(raw) as CLIConfig; } catch { return undefined; } diff --git a/packages/cli/src/commands/auth.ts b/packages/cli/src/commands/auth.ts index a8d96321..0c45ee75 100644 --- a/packages/cli/src/commands/auth.ts +++ b/packages/cli/src/commands/auth.ts @@ -5,7 +5,7 @@ import { loginInteractive } from "../auth/login-interactive"; import { getToken, removeToken } from "../auth/token"; import { fetchWhoami } from "../auth/whoami"; import { getTelemetry } from "../telemetry"; -import { type CliErrorCode, writeJsonError } from "../types/errors"; +import { type CLIErrorCode, writeJsonError } from "../types/errors"; const loginCommand = defineCommand({ meta: { @@ -33,15 +33,9 @@ const loginCommand = defineCommand({ async run({ args }) { const cwd = resolve(args.dir ?? process.cwd()); const telemetry = await getTelemetry(cwd); - const startedAt = Date.now(); - telemetry.capture("cli_auth_login"); - - /** Tracks the last emitted error code so the completion event can include it. */ - let lastErrorCode: CliErrorCode | undefined; /** Emit an error in the right channel and set exit code. */ - const fail = (code: CliErrorCode, message: string): void => { - lastErrorCode = code; + const fail = (code: CLIErrorCode, message: string): void => { if (args.json) { writeJsonError(code, message); } else { @@ -52,15 +46,12 @@ const loginCommand = defineCommand({ if (args.anonymous) { fail("INVALID_INPUT", "auth commands cannot be anonymous."); - telemetry.capture("cli_auth_login_completed", { - success: false, - durationMs: Date.now() - startedAt, - errorCode: lastErrorCode, - }); return; } - let success = false; + // Set true only when a fresh authentication completes; drives the + // cli_authenticated event in the finally. + let authenticated = false; try { // In --json mode the user is an agent / pipe; suppress the device-flow // chatter and only emit a single structured line on error. @@ -71,7 +62,7 @@ const loginCommand = defineCommand({ switch (result.status) { case "ok": { - success = true; + authenticated = true; return; } case "already_logged_in": { @@ -79,11 +70,10 @@ const loginCommand = defineCommand({ console.log("You are already logged in."); console.log("Run `taskless auth logout` first to re-authenticate."); } - success = true; return; } case "cancelled": { - const code: CliErrorCode = + const code: CLIErrorCode = result.reason === "denied" ? "AUTH_REQUIRED" : "NETWORK_ERROR"; const message = result.message ?? @@ -97,11 +87,10 @@ const loginCommand = defineCommand({ } } } finally { - telemetry.capture("cli_auth_login_completed", { - success, - durationMs: Date.now() - startedAt, - ...(success ? {} : { errorCode: lastErrorCode }), - }); + // Concrete state event: a fresh authentication succeeded. + if (authenticated) { + telemetry.capture("cli_authenticated"); + } } }, }); @@ -132,21 +121,18 @@ const logoutCommand = defineCommand({ async run({ args }) { const cwd = resolve(args.dir ?? process.cwd()); const telemetry = await getTelemetry(cwd); - const startedAt = Date.now(); - telemetry.capture("cli_auth_logout"); - let success = false; + let removed = false; try { - const removed = await removeToken(cwd); + removed = await removeToken(cwd); if (!args.json) { console.log(removed ? "Logged out." : "Not logged in."); } - success = true; } finally { - telemetry.capture("cli_auth_logout_completed", { - success, - durationMs: Date.now() - startedAt, - }); + // Concrete state event: a saved token was actually removed. + if (removed) { + telemetry.capture("cli_logged_out"); + } } }, }); @@ -181,39 +167,25 @@ export const authCommand = defineCommand({ } const cwd = resolve(args.dir ?? process.cwd()); - const telemetry = await getTelemetry(cwd); - const startedAt = Date.now(); - telemetry.capture("cli_auth_status"); - - let success = false; - try { - const token = await getToken(cwd); - if (!token) { - console.log("Not logged in."); - console.log("Run `taskless auth login` to authenticate."); - success = true; - return; - } - const whoami = await fetchWhoami(token); - if (!whoami) { - console.log("Logged in, but unable to verify identity."); - console.log( - "Your token may be invalid or expired. Run `taskless auth login` to re-authenticate." - ); - success = true; - return; - } + const token = await getToken(cwd); + if (!token) { + console.log("Not logged in."); + console.log("Run `taskless auth login` to authenticate."); + return; + } - const orgs = whoami.orgs.map((o) => o.name); - const orgSuffix = orgs.length > 0 ? ` (${orgs.join(", ")})` : ""; - console.log(`Logged in as ${whoami.user}${orgSuffix}.`); - success = true; - } finally { - telemetry.capture("cli_auth_status_completed", { - success, - durationMs: Date.now() - startedAt, - }); + const whoami = await fetchWhoami(token); + if (!whoami) { + console.log("Logged in, but unable to verify identity."); + console.log( + "Your token may be invalid or expired. Run `taskless auth login` to re-authenticate." + ); + return; } + + const orgs = whoami.orgs.map((o) => o.name); + const orgSuffix = orgs.length > 0 ? ` (${orgs.join(", ")})` : ""; + console.log(`Logged in as ${whoami.user}${orgSuffix}.`); }, }); diff --git a/packages/cli/src/commands/check.ts b/packages/cli/src/commands/check.ts index bab9e7cb..f1e82b49 100644 --- a/packages/cli/src/commands/check.ts +++ b/packages/cli/src/commands/check.ts @@ -108,10 +108,12 @@ export const checkCommand = defineCommand({ async run({ args, rawArgs }) { const cwd = resolve(args.dir ?? process.cwd()); const telemetry = await getTelemetry(cwd); - const startedAt = Date.now(); - telemetry.capture("cli_check"); - let success = false; + // Set when a scan actually runs; drives cli_check_completed with counts + // only (never matched code). + let scanCounts: + | { errorCount: number; warningCount: number; findings: number } + | undefined; try { const positionalPaths = extractPositionalPaths(rawArgs); const hadExplicitPaths = positionalPaths.length > 0; @@ -129,7 +131,6 @@ export const checkCommand = defineCommand({ ) ); } - success = true; return; } @@ -155,7 +156,6 @@ export const checkCommand = defineCommand({ "No rules configured. Create one with `taskless rule create`." ); } - success = true; return; } @@ -163,7 +163,14 @@ export const checkCommand = defineCommand({ try { await generateSgConfig(cwd); const { results } = await runAstGrepScan(cwd, existingPaths); - const hasErrors = results.some((r) => r.severity === "error"); + let errorCount = 0; + let warningCount = 0; + for (const result of results) { + if (result.severity === "error") errorCount++; + else if (result.severity === "warning") warningCount++; + } + const hasErrors = errorCount > 0; + scanCounts = { errorCount, warningCount, findings: results.length }; // Format output if (args.json) { @@ -180,7 +187,6 @@ export const checkCommand = defineCommand({ if (hasErrors) { process.exitCode = 1; } - success = !hasErrors; } catch (error) { const message = `Error: ${error instanceof Error ? error.message : String(error)}`; if (args.json) { @@ -193,10 +199,10 @@ export const checkCommand = defineCommand({ process.exitCode = 1; } } finally { - telemetry.capture("cli_check_completed", { - success, - durationMs: Date.now() - startedAt, - }); + // Concrete state event: a scan completed; counts only, no matched code. + if (scanCounts) { + telemetry.capture("cli_check_completed", scanCounts); + } } }, }); diff --git a/packages/cli/src/commands/help.ts b/packages/cli/src/commands/help.ts index 71793854..f6e12938 100644 --- a/packages/cli/src/commands/help.ts +++ b/packages/cli/src/commands/help.ts @@ -153,8 +153,8 @@ export function createHelpCommand(subCommands: SubCommandsDef) { const telemetry = await getTelemetry(cwd); if (positionals.length === 0) { - // help_index: agent fetched the topic list - telemetry.capture("help_index"); + // cli_help with the index marker: agent fetched the topic list + telemetry.capture("cli_help", { topic: "(index)" }); console.log("Taskless CLI\n"); console.log( @@ -198,16 +198,13 @@ export function createHelpCommand(subCommands: SubCommandsDef) { : helpMap.get(key); if (content) { - // help_: agent fetched a specific recipe (intent signal) - const topicEvent = `help_${key.replaceAll("-", "_")}`; - telemetry.capture(topicEvent, { - topic: positionals.join(" "), - anonymous: args.anonymous, - }); + // cli_help: agent fetched a specific recipe (intent signal). The topic + // is the served topic; filtering on it replaces the old per-topic events. + telemetry.capture("cli_help", { topic: positionals.join(" ") }); console.log(renderRecipe(content, key).trimEnd()); } else { - // help_unknown: agent asked for a topic that does not exist - telemetry.capture("help_unknown", { topic: positionals.join(" ") }); + // cli_help for an unknown topic — still the attempted topic string. + telemetry.capture("cli_help", { topic: positionals.join(" ") }); console.error(`Unknown command: ${positionals.join(" ")}`); console.error("Run `taskless help` for available commands."); process.exitCode = 1; diff --git a/packages/cli/src/commands/info.ts b/packages/cli/src/commands/info.ts index 5ebfafb7..1da8a835 100644 --- a/packages/cli/src/commands/info.ts +++ b/packages/cli/src/commands/info.ts @@ -5,7 +5,6 @@ import { checkStaleness } from "../install/install"; import { getToken } from "../auth/token"; import { fetchWhoami } from "../auth/whoami"; import { outputSchema as infoOutputSchema } from "../schemas/info"; -import { getTelemetry } from "../telemetry"; import { makeErrorEnvelope } from "../types/errors"; export const infoCommand = defineCommand({ @@ -32,100 +31,87 @@ export const infoCommand = defineCommand({ }, async run({ args }) { const cwd = resolve(args.dir ?? process.cwd()); - const telemetry = await getTelemetry(cwd); - const startedAt = Date.now(); - telemetry.capture("cli_info"); - let success = false; - try { - const [tools, token] = await Promise.all([ - checkStaleness(cwd), - args.anonymous ? Promise.resolve() : getToken(cwd), - ]); + const [tools, token] = await Promise.all([ + checkStaleness(cwd), + args.anonymous ? Promise.resolve() : getToken(cwd), + ]); - let auth: { user: string; email: string; orgs: string[] } | undefined; - if (!args.anonymous && token) { - const whoami = await fetchWhoami(token); - if (whoami) { - auth = { - user: whoami.user, - email: whoami.email, - orgs: whoami.orgs.map((o) => o.name), - }; - } + let auth: { user: string; email: string; orgs: string[] } | undefined; + if (!args.anonymous && token) { + const whoami = await fetchWhoami(token); + if (whoami) { + auth = { + user: whoami.user, + email: whoami.email, + orgs: whoami.orgs.map((o) => o.name), + }; } + } - const result = { - success: true as const, - version: __VERSION__, - tools, - loggedIn: token !== undefined, - auth, - }; + const result = { + success: true as const, + version: __VERSION__, + tools, + loggedIn: token !== undefined, + auth, + }; - if (args.json) { - const parsed = infoOutputSchema.safeParse(result); - if (!parsed.success) { - console.log( - JSON.stringify( - makeErrorEnvelope( - "INTERNAL_ERROR", - "Internal schema validation failed" - ) + if (args.json) { + const parsed = infoOutputSchema.safeParse(result); + if (!parsed.success) { + console.log( + JSON.stringify( + makeErrorEnvelope( + "INTERNAL_ERROR", + "Internal schema validation failed" ) - ); - process.exitCode = 1; - return; - } - console.log(JSON.stringify(parsed.data)); - success = true; + ) + ); + process.exitCode = 1; return; } + console.log(JSON.stringify(parsed.data)); + return; + } - // Human-readable output - console.log(`Taskless CLI v${__VERSION__}\n`); + // Human-readable output + console.log(`Taskless CLI v${__VERSION__}\n`); - if (tools.length === 0) { - console.log("Tools: none detected"); - } else { - console.log("Tools:"); - for (const tool of tools) { - const total = tool.skills.length; - const upToDate = tool.skills.filter((s) => s.current).length; - const stale = total - upToDate; + if (tools.length === 0) { + console.log("Tools: none detected"); + } else { + console.log("Tools:"); + for (const tool of tools) { + const total = tool.skills.length; + const upToDate = tool.skills.filter((s) => s.current).length; + const stale = total - upToDate; - if (stale === 0) { - console.log( - ` ${tool.name}: ${String(total)} skills (all up to date)` - ); - } else { - console.log( - ` ${tool.name}: ${String(total)} skills (${String(stale)} outdated)` - ); - for (const skill of tool.skills) { - if (!skill.current) { - console.log( - ` - ${skill.name}: ${skill.installedVersion ?? "missing"} → ${skill.currentVersion}` - ); - } + if (stale === 0) { + console.log( + ` ${tool.name}: ${String(total)} skills (all up to date)` + ); + } else { + console.log( + ` ${tool.name}: ${String(total)} skills (${String(stale)} outdated)` + ); + for (const skill of tool.skills) { + if (!skill.current) { + console.log( + ` - ${skill.name}: ${skill.installedVersion ?? "missing"} → ${skill.currentVersion}` + ); } } } } + } - console.log(""); - if (auth) { - const orgs = auth.orgs.length > 0 ? ` (${auth.orgs.join(", ")})` : ""; - console.log(`Auth: logged in as ${auth.user}${orgs}`); - } else { - console.log("Auth: not logged in"); - } - success = true; - } finally { - telemetry.capture("cli_info_completed", { - success, - durationMs: Date.now() - startedAt, - }); + console.log(""); + if (auth) { + const orgs = auth.orgs.length > 0 ? ` (${auth.orgs.join(", ")})` : ""; + console.log(`Auth: logged in as ${auth.user}${orgs}`); + } else { + console.log("Auth: not logged in"); } }, }); diff --git a/packages/cli/src/commands/init.ts b/packages/cli/src/commands/init.ts index 7f134e3c..b3f0b7dc 100644 --- a/packages/cli/src/commands/init.ts +++ b/packages/cli/src/commands/init.ts @@ -53,7 +53,6 @@ export const initCommand = defineCommand({ async run({ args }) { const cwd = resolve(args.dir ?? process.cwd()); const telemetry = await getTelemetry(cwd); - telemetry.capture("cli_init"); const interactive = shouldRunInteractively(args["no-interactive"]); @@ -71,19 +70,12 @@ export const initCommand = defineCommand({ ); } - const start = Date.now(); const result = await runNonInteractive(cwd); console.log( getOnboardTrailer({ commandsInstalled: result.commandsInstalled }) ); - telemetry.capture("cli_init_completed", { - locations: await detectedLocationDirectories(cwd), - optionalSkills: [], - authPromptShown: false, - authCompleted: false, - nonInteractive: true, - durationMs: Date.now() - start, - }); + // Concrete state event: skills/commands were installed (non-interactive). + telemetry.capture("cli_installed"); }, }); @@ -108,19 +100,16 @@ export const updateCommand = defineCommand({ async run({ args }) { const cwd = resolve(args.dir ?? process.cwd()); const telemetry = await getTelemetry(cwd); - const startedAt = Date.now(); - telemetry.capture("cli_update"); let success = false; try { await runNonInteractive(cwd); success = true; } finally { - telemetry.capture("cli_update_completed", { - locations: await detectedLocationDirectories(cwd), - success, - durationMs: Date.now() - startedAt, - }); + // Concrete state event: skills/commands were installed/updated. + if (success) { + telemetry.capture("cli_installed"); + } } }, }); @@ -233,7 +222,3 @@ function groupValuesByTarget( } return map; } - -async function detectedLocationDirectories(cwd: string): Promise { - return detectSelectedDirectories(cwd); -} diff --git a/packages/cli/src/commands/onboard.ts b/packages/cli/src/commands/onboard.ts index 19a078f9..99d97d2b 100644 --- a/packages/cli/src/commands/onboard.ts +++ b/packages/cli/src/commands/onboard.ts @@ -5,7 +5,7 @@ import { defineCommand } from "citty"; import { ensureTasklessDirectory } from "../filesystem/directory"; import { readManifest, writeManifest } from "../filesystem/migrate"; import { getTelemetry } from "../telemetry"; -import { CliError } from "../util/cli-error"; +import { CLIError } from "../util/cli-error"; import { getRecipe } from "./help"; @@ -67,7 +67,7 @@ export const onboardCommand = defineCommand({ " --force re-runs the discovery recipe; --mark-complete records completion." ); process.exitCode = 1; - throw new CliError("conflicting flags"); + throw new CLIError("conflicting flags"); } await ensureTasklessDirectory(cwd); @@ -80,7 +80,8 @@ export const onboardCommand = defineCommand({ manifest.install = install; await writeManifest(tasklessDirectory, manifest, raw); console.log("Marked Taskless onboarding as complete."); - telemetry.capture("cli_onboard_marked_complete"); + // Concrete state event: onboarding reached completion. + telemetry.capture("cli_onboarded"); return; } @@ -92,7 +93,6 @@ export const onboardCommand = defineCommand({ console.log( "Run `taskless onboard --force` to re-run the discovery recipe." ); - telemetry.capture("cli_onboard_already_done"); return; } @@ -101,9 +101,8 @@ export const onboardCommand = defineCommand({ // Should not happen — onboard.txt is embedded at build time. console.error("Internal error: onboard recipe is not available."); process.exitCode = 1; - throw new CliError("recipe missing"); + throw new CLIError("recipe missing"); } console.log(recipe.trimEnd()); - telemetry.capture("cli_onboard_recipe", { forced: args.force }); }, }); diff --git a/packages/cli/src/commands/rules.ts b/packages/cli/src/commands/rules.ts index a9d65065..64e884fe 100644 --- a/packages/cli/src/commands/rules.ts +++ b/packages/cli/src/commands/rules.ts @@ -25,8 +25,8 @@ import { import { outputSchema as metaOutputSchema } from "../schemas/rules-meta"; import { verifyOutputSchema } from "../schemas/rules-verify"; import { getTelemetry } from "../telemetry"; -import { CliError } from "../util/cli-error"; -import { type CliErrorCode, makeErrorEnvelope } from "../types/errors"; +import { CLIError } from "../util/cli-error"; +import { type CLIErrorCode, makeErrorEnvelope } from "../types/errors"; /** Format today's date as YYYYMMDD */ function getTimestamp(): string { @@ -71,13 +71,11 @@ const createCommand = defineCommand({ async run({ args }) { const cwd = resolve(args.dir ?? process.cwd()); const telemetry = await getTelemetry(cwd); - const startedAt = Date.now(); - telemetry.capture("cli_rule_create"); /** Emit an error and exit, respecting --json mode */ function fail( message: string, - code: CliErrorCode = "INTERNAL_ERROR" + code: CLIErrorCode = "INTERNAL_ERROR" ): never { if (args.json) { console.log(JSON.stringify(makeErrorEnvelope(code, message))); @@ -85,7 +83,7 @@ const createCommand = defineCommand({ console.error(`Error: ${message}`); } process.exitCode = 1; - throw new CliError(message); + throw new CLIError(message); } if (args.anonymous) { @@ -101,14 +99,12 @@ const createCommand = defineCommand({ console.error(message); } process.exitCode = 1; - telemetry.capture("cli_rule_create_completed", { - success: false, - durationMs: Date.now() - startedAt, - }); return; } - let success = false; + // Set to the number of rules written when generation succeeds; drives the + // cli_rule_created event in the finally. + let createdRuleCount: number | undefined; try { // 1. Read and validate --from file if (!args.from) { @@ -157,7 +153,7 @@ const createCommand = defineCommand({ // resolveIdentity throws on missing auth or missing git remote; // surface the original message but pick a best-guess code. const message = error instanceof Error ? error.message : String(error); - const code: CliErrorCode = /git remote|origin/i.test(message) + const code: CLIErrorCode = /git remote|origin/i.test(message) ? "NO_GITHUB_REMOTE" : "AUTH_REQUIRED"; fail(message, code); @@ -249,7 +245,7 @@ const createCommand = defineCommand({ console.log(` ${filePath}`); } } - success = true; + if (rules.length > 0) createdRuleCount = rules.length; return; } case "pr": @@ -267,16 +263,15 @@ const createCommand = defineCommand({ } else { console.log(`Rule ${ruleId} is in state "${status.status}".`); } - success = true; return; } } } } finally { - telemetry.capture("cli_rule_create_completed", { - success, - durationMs: Date.now() - startedAt, - }); + // Concrete state event: a rule was actually generated and written. + if (createdRuleCount !== undefined) { + telemetry.capture("cli_rule_created", { ruleCount: createdRuleCount }); + } } }, }); @@ -313,13 +308,11 @@ const improveCommand = defineCommand({ async run({ args }) { const cwd = resolve(args.dir ?? process.cwd()); const telemetry = await getTelemetry(cwd); - const startedAt = Date.now(); - telemetry.capture("cli_rule_improve"); /** Emit an error and exit, respecting --json mode */ function fail( message: string, - code: CliErrorCode = "INTERNAL_ERROR" + code: CLIErrorCode = "INTERNAL_ERROR" ): never { if (args.json) { console.log(JSON.stringify(makeErrorEnvelope(code, message))); @@ -327,7 +320,7 @@ const improveCommand = defineCommand({ console.error(`Error: ${message}`); } process.exitCode = 1; - throw new CliError(message); + throw new CLIError(message); } if (args.anonymous) { @@ -341,14 +334,12 @@ const improveCommand = defineCommand({ console.error(message); } process.exitCode = 1; - telemetry.capture("cli_rule_improve_completed", { - success: false, - durationMs: Date.now() - startedAt, - }); return; } - let success = false; + // Set to the number of rules written when iteration succeeds; drives the + // cli_rule_improved event in the finally. + let improvedRuleCount: number | undefined; try { // 1. Read and validate --from file if (!args.from) { @@ -395,7 +386,7 @@ const improveCommand = defineCommand({ identity = await resolveIdentity(cwd); } catch (error) { const message = error instanceof Error ? error.message : String(error); - const code: CliErrorCode = /git remote|origin/i.test(message) + const code: CLIErrorCode = /git remote|origin/i.test(message) ? "NO_GITHUB_REMOTE" : "AUTH_REQUIRED"; fail(message, code); @@ -487,7 +478,7 @@ const improveCommand = defineCommand({ console.log(` ${filePath}`); } } - success = true; + if (rules.length > 0) improvedRuleCount = rules.length; return; } case "pr": @@ -506,16 +497,17 @@ const improveCommand = defineCommand({ `Request ${requestId} is in state "${status.status}".` ); } - success = true; return; } } } } finally { - telemetry.capture("cli_rule_improve_completed", { - success, - durationMs: Date.now() - startedAt, - }); + // Concrete state event: a rule was actually iterated and rewritten. + if (improvedRuleCount !== undefined) { + telemetry.capture("cli_rule_improved", { + ruleCount: improvedRuleCount, + }); + } } }, }); @@ -549,13 +541,10 @@ const metaCommand = defineCommand({ }, async run({ args }) { const cwd = resolve(args.dir ?? process.cwd()); - const telemetry = await getTelemetry(cwd); - const startedAt = Date.now(); - telemetry.capture("cli_rule_meta"); function fail( message: string, - code: CliErrorCode = "INTERNAL_ERROR" + code: CLIErrorCode = "INTERNAL_ERROR" ): never { if (args.json) { console.log(JSON.stringify(makeErrorEnvelope(code, message))); @@ -563,45 +552,36 @@ const metaCommand = defineCommand({ console.error(`Error: ${message}`); } process.exitCode = 1; - throw new CliError(message); + throw new CLIError(message); } - let success = false; - try { - const meta = await readRuleMetaFile(cwd, args.id); - if (!meta) { - fail( - `No metadata found for rule "${args.id}". Expected .taskless/rule-metadata/${args.id}.yml`, - "RULE_NOT_FOUND" - ); - } + const meta = await readRuleMetaFile(cwd, args.id); + if (!meta) { + fail( + `No metadata found for rule "${args.id}". Expected .taskless/rule-metadata/${args.id}.yml`, + "RULE_NOT_FOUND" + ); + } - if (args.json) { - let output; - try { - output = metaOutputSchema.parse({ id: args.id, ...meta }); - } catch (error) { - if (error instanceof ZodError) { - fail( - `Invalid metadata for rule "${args.id}": ${error.issues.map((issue) => issue.message).join(", ")}`, - "INVALID_INPUT" - ); - } - fail(error instanceof Error ? error.message : String(error)); - } - console.log(JSON.stringify(output)); - } else { - console.log(`Metadata for rule "${args.id}":\n`); - for (const [key, value] of Object.entries(meta)) { - console.log(` ${key}: ${String(value)}`); + if (args.json) { + let output; + try { + output = metaOutputSchema.parse({ id: args.id, ...meta }); + } catch (error) { + if (error instanceof ZodError) { + fail( + `Invalid metadata for rule "${args.id}": ${error.issues.map((issue) => issue.message).join(", ")}`, + "INVALID_INPUT" + ); } + fail(error instanceof Error ? error.message : String(error)); + } + console.log(JSON.stringify(output)); + } else { + console.log(`Metadata for rule "${args.id}":\n`); + for (const [key, value] of Object.entries(meta)) { + console.log(` ${key}: ${String(value)}`); } - success = true; - } finally { - telemetry.capture("cli_rule_meta_completed", { - success, - durationMs: Date.now() - startedAt, - }); } }, }); @@ -637,8 +617,6 @@ const deleteCommand = defineCommand({ async run({ args }) { const cwd = resolve(args.dir ?? process.cwd()); const telemetry = await getTelemetry(cwd); - const startedAt = Date.now(); - telemetry.capture("cli_rule_delete"); const id = args.id; let success = false; @@ -661,10 +639,10 @@ const deleteCommand = defineCommand({ process.exitCode = 1; } } finally { - telemetry.capture("cli_rule_delete_completed", { - success, - durationMs: Date.now() - startedAt, - }); + // Concrete state event: a rule and its tests were actually removed. + if (success) { + telemetry.capture("cli_rule_deleted"); + } } }, }); @@ -698,73 +676,61 @@ const verifyCommand = defineCommand({ }, async run({ args }) { const cwd = resolve(args.dir ?? process.cwd()); - const telemetry = await getTelemetry(cwd); - const startedAt = Date.now(); - telemetry.capture("cli_rule_verify"); - - let success = false; - try { - if (!args.id) { - if (args.json) { - console.log( - JSON.stringify( - makeErrorEnvelope("INVALID_INPUT", "Rule ID is required.") - ) - ); - } else { - console.error( - "Error: Rule ID is required.\n Usage: taskless rule verify " - ); - } - process.exitCode = 1; - return; - } - - const result = await verifyRule(cwd, args.id); + if (!args.id) { if (args.json) { - console.log(JSON.stringify(verifyOutputSchema.parse(result))); - } else { - console.log(`Verifying rule: ${result.ruleId}\n`); - - // Layer 1 console.log( - `Schema: ${result.schema.valid ? "✓ valid" : "✗ invalid"}` + JSON.stringify( + makeErrorEnvelope("INVALID_INPUT", "Rule ID is required.") + ) ); - for (const error of result.schema.errors) { - console.log(` - ${error}`); - } - - // Layer 2 - console.log( - `Requirements: ${result.requirements.valid ? "✓ valid" : "✗ invalid"}` + } else { + console.error( + "Error: Rule ID is required.\n Usage: taskless rule verify " ); - for (const error of result.requirements.errors) { - console.log(` - ${error}`); - } + } + process.exitCode = 1; + return; + } - // Layer 3 - console.log( - `Tests: ${result.tests.valid ? "✓ passed" : "✗ failed"} (${String(result.tests.passed)} passed, ${String(result.tests.failed)} failed)` - ); - for (const error of result.tests.errors) { - console.log(` - ${error}`); - } + const result = await verifyRule(cwd, args.id); - console.log( - `\nResult: ${result.success ? "✓ All checks passed" : "✗ Verification failed"}` - ); + if (args.json) { + console.log(JSON.stringify(verifyOutputSchema.parse(result))); + } else { + console.log(`Verifying rule: ${result.ruleId}\n`); + + // Layer 1 + console.log( + `Schema: ${result.schema.valid ? "✓ valid" : "✗ invalid"}` + ); + for (const error of result.schema.errors) { + console.log(` - ${error}`); } - if (!result.success) { - process.exitCode = 1; + // Layer 2 + console.log( + `Requirements: ${result.requirements.valid ? "✓ valid" : "✗ invalid"}` + ); + for (const error of result.requirements.errors) { + console.log(` - ${error}`); } - success = result.success; - } finally { - telemetry.capture("cli_rule_verify_completed", { - success, - durationMs: Date.now() - startedAt, - }); + + // Layer 3 + console.log( + `Tests: ${result.tests.valid ? "✓ passed" : "✗ failed"} (${String(result.tests.passed)} passed, ${String(result.tests.failed)} failed)` + ); + for (const error of result.tests.errors) { + console.log(` - ${error}`); + } + + console.log( + `\nResult: ${result.success ? "✓ All checks passed" : "✗ Verification failed"}` + ); + } + + if (!result.success) { + process.exitCode = 1; } }, }); diff --git a/packages/cli/src/index.ts b/packages/cli/src/index.ts index 2b9060cd..ae895704 100644 --- a/packages/cli/src/index.ts +++ b/packages/cli/src/index.ts @@ -7,8 +7,13 @@ import { infoCommand } from "./commands/info"; import { createHelpCommand } from "./commands/help"; import { onboardCommand } from "./commands/onboard"; import { ruleCommand } from "./commands/rules"; -import { shutdownTelemetry } from "./telemetry"; -import { CliError } from "./util/cli-error"; +import { + getTelemetry, + resolveRunIdentity, + shutdownTelemetry, +} from "./telemetry"; +import { emitRunEvents, resolveCommandName, resolveCwd } from "./telemetry-run"; +import { CLIError } from "./util/cli-error"; const subCommands = { init: initCommand, @@ -97,15 +102,43 @@ const main = defineCommand({ }); // main loop to run cli and make every attempt to shut down gracefully +const rawArguments = process.argv.slice(2); +const runCwd = resolveCwd(rawArguments); +const startedAt = Date.now(); +// Resolve identity at invocation START so cli_run reports who *initiated* the +// run, not the post-command state — e.g. `auth login` run by a logged-out user +// reports loggedIn:false (the login was performed as a logged-out user). +const startIdentity = await resolveRunIdentity(runCwd); +let thrown: unknown; try { - await runCommand(main, { rawArgs: process.argv.slice(2) }); + await runCommand(main, { rawArgs: rawArguments }); } catch (error) { - // CliError = expected failure (already printed output, exitCode already set) - if (!(error instanceof CliError)) { + // CLIError = expected failure (already printed output, exitCode already set) + thrown = error; + if (!(error instanceof CLIError)) { process.exitCode = 1; console.error(error instanceof Error ? error.message : String(error)); } } finally { + // cli_run is the per-invocation denominator: emitted exactly once here, on + // both success and failure, so no command has to remember to. Telemetry is + // best-effort and never affects the exit. + try { + const telemetry = await getTelemetry(runCwd); + const success = + thrown === undefined && + (process.exitCode === undefined || process.exitCode === 0); + emitRunEvents(telemetry, { + command: resolveCommandName(rawArguments), + success, + durationMs: Date.now() - startedAt, + anonymous: startIdentity.anonymous, + loggedIn: startIdentity.loggedIn, + error: thrown, + }); + } catch { + // Telemetry failures are silent + } try { await shutdownTelemetry(); } catch { diff --git a/packages/cli/src/telemetry-run.ts b/packages/cli/src/telemetry-run.ts new file mode 100644 index 00000000..3421b929 --- /dev/null +++ b/packages/cli/src/telemetry-run.ts @@ -0,0 +1,89 @@ +import { resolve } from "node:path"; + +import type { TelemetryClient } from "./telemetry"; +import { CLIError } from "./util/cli-error"; + +/** + * Derive the cli_run `command` property from the raw argv. Flags (and the + * value after `-d`/`--dir`) are skipped; the first positional is the command, + * and `rule` keeps its subcommand (e.g. `rule create`) since that distinction + * is meaningful. `help`'s topic is recorded separately on cli_help, so the + * command for a help invocation is just `help`. + */ +export function resolveCommandName(rawArguments: string[]): string { + const valueFlags = new Set(["-d", "--dir"]); + const positionals: string[] = []; + for (let index = 0; index < rawArguments.length; index++) { + const argument = rawArguments[index]!; + if (argument.startsWith("-")) { + if (!argument.includes("=") && valueFlags.has(argument)) index++; + continue; + } + positionals.push(argument); + } + + if (positionals.length === 0) return "(default)"; + const top = positionals[0]!; + if (top === "rule" && positionals[1]) return `rule ${positionals[1]}`; + return top; +} + +/** Resolve the working directory from `-d`/`--dir`, defaulting to cwd. */ +export function resolveCwd(rawArguments: string[]): string { + for (let index = 0; index < rawArguments.length; index++) { + const argument = rawArguments[index]!; + if ( + (argument === "-d" || argument === "--dir") && + rawArguments[index + 1] + ) { + return resolve(rawArguments[index + 1]!); + } + if (argument.startsWith("--dir=")) { + return resolve(argument.slice("--dir=".length)); + } + } + return process.cwd(); +} + +export interface RunContext { + command: string; + success: boolean; + durationMs: number; + /** Resolved fresh at emission time (see resolveRunIdentity) so auth-changing + * commands report post-invocation state. */ + anonymous: boolean; + loggedIn: boolean; + /** The thrown error, if the command threw. */ + error?: unknown; +} + +/** + * Emit the per-invocation telemetry: a single `cli_run` denominator event + * (always), preceded by `cli_error` only when the command threw. The CLI + * version is NOT added here — it rides on the standard `cliVersion` property + * the telemetry client attaches to every event. + */ +export function emitRunEvents( + telemetry: Pick, + context: RunContext +): void { + // cli_error fires only for a thrown failure (with a known CLIErrorCode when + // available). A failure signalled purely via process.exitCode (the command + // printed its own error) is captured by cli_run's success:false — emitting + // cli_error there would mislabel it INTERNAL_ERROR. + if (context.error !== undefined) { + const code = + context.error instanceof CLIError && context.error.code + ? context.error.code + : "INTERNAL_ERROR"; + telemetry.capture("cli_error", { command: context.command, code }); + } + + telemetry.capture("cli_run", { + command: context.command, + success: context.success, + durationMs: context.durationMs, + anonymous: context.anonymous, + loggedIn: context.loggedIn, + }); +} diff --git a/packages/cli/src/telemetry.ts b/packages/cli/src/telemetry.ts index 2ca14a2e..4a46355f 100644 --- a/packages/cli/src/telemetry.ts +++ b/packages/cli/src/telemetry.ts @@ -23,6 +23,26 @@ export interface TelemetryClient { shutdown(): Promise; } +/** + * Resolve the current auth identity by reading the token fresh. Unlike the + * telemetry client's cached identity (fixed at init), this reflects the state + * AT CALL TIME — so the runner can stamp cli_run with the post-invocation + * identity even for commands that change auth state mid-run (auth login/logout). + */ +export async function resolveRunIdentity( + cwd?: string +): Promise<{ anonymous: boolean; loggedIn: boolean }> { + try { + const token = await getToken(cwd, { silent: true }); + if (!token) return { anonymous: true, loggedIn: false }; + // A token is present → logged in. anonymous tracks whether a subject + // (authenticated identity) decoded from it. + return { anonymous: decodeSubject(token) === undefined, loggedIn: true }; + } catch { + return { anonymous: true, loggedIn: false }; + } +} + function isTelemetryDisabled(): boolean { return ( process.env.TASKLESS_TELEMETRY_DISABLED === "1" || diff --git a/packages/cli/src/types/errors.ts b/packages/cli/src/types/errors.ts index ba357b5b..ab5504f9 100644 --- a/packages/cli/src/types/errors.ts +++ b/packages/cli/src/types/errors.ts @@ -6,7 +6,7 @@ * Add new codes by extending the union; do not rename existing codes * without a major version bump. */ -export type CliErrorCode = +export type CLIErrorCode = | "AUTH_REQUIRED" | "NO_GITHUB_REMOTE" | "RULE_GENERATION_FAILED" @@ -20,19 +20,19 @@ export type CliErrorCode = * Standardized JSON error envelope written to stdout when an action * command exits with an error AND `--json` was set. */ -export interface CliErrorEnvelope { +export interface CLIErrorEnvelope { ok: false; - code: CliErrorCode; + code: CLIErrorCode; message: string; } export function makeErrorEnvelope( - code: CliErrorCode, + code: CLIErrorCode, message: string -): CliErrorEnvelope { +): CLIErrorEnvelope { return { ok: false, code, message }; } -export function writeJsonError(code: CliErrorCode, message: string): void { +export function writeJsonError(code: CLIErrorCode, message: string): void { console.log(JSON.stringify(makeErrorEnvelope(code, message))); } diff --git a/packages/cli/src/util/cli-error.ts b/packages/cli/src/util/cli-error.ts index 43ab7b90..e2fe76f8 100644 --- a/packages/cli/src/util/cli-error.ts +++ b/packages/cli/src/util/cli-error.ts @@ -1,8 +1,20 @@ +import type { CLIErrorCode } from "../types/errors"; + /** * Sentinel error for expected CLI failures (e.g. validation errors). * The top-level catch in index.ts uses this to distinguish expected exits * (already printed their own output) from unexpected crashes. + * + * An optional `code` (a stable `CLIErrorCode`) lets the runner attribute a + * `cli_error` telemetry event to a known failure mode. Omitting it is fine; + * the runner falls back to `INTERNAL_ERROR`. */ -export class CliError extends Error { - override name = "CliError"; +export class CLIError extends Error { + override name = "CLIError"; + readonly code?: CLIErrorCode; + + constructor(message?: string, code?: CLIErrorCode) { + super(message); + this.code = code; + } } diff --git a/packages/cli/src/wizard/index.ts b/packages/cli/src/wizard/index.ts index 157b56ef..22a7565c 100644 --- a/packages/cli/src/wizard/index.ts +++ b/packages/cli/src/wizard/index.ts @@ -102,19 +102,8 @@ export async function runWizard( function finish(args: { status: "completed" | "cancelled" }): WizardResult { const durationMs = Date.now() - start; if (args.status === "completed") { - telemetry.capture("cli_init_completed", { - locations, - optionalSkills, - authPromptShown, - authCompleted, - nonInteractive: false, - durationMs, - }); - } else { - telemetry.capture("cli_init_cancelled", { - atStep: cancelledStep ?? "unknown", - durationMs, - }); + // Concrete state event: skills/commands were installed (interactive). + telemetry.capture("cli_installed"); } return { status: args.status, diff --git a/packages/cli/test/cli-run.test.ts b/packages/cli/test/cli-run.test.ts new file mode 100644 index 00000000..40aa46bb --- /dev/null +++ b/packages/cli/test/cli-run.test.ts @@ -0,0 +1,130 @@ +import { describe, expect, it, vi } from "vitest"; + +import { emitRunEvents, resolveCommandName } from "../src/telemetry-run"; +import { CLIError } from "../src/util/cli-error"; + +describe("resolveCommandName", () => { + it.each([ + [["info"], "info"], + [["check", "--json"], "check"], + [["rule", "create"], "rule create"], + [["rule"], "rule"], + [["help", "route"], "help"], + [["-d", "/tmp", "check"], "check"], + [["--dir", "/tmp", "info"], "info"], + [[], "(default)"], + ])("resolves %j to %s", (argv, expected) => { + expect(resolveCommandName(argv)).toBe(expected); + }); +}); + +function fakeTelemetry() { + return { capture: vi.fn() }; +} + +const anon = { anonymous: true, loggedIn: false }; + +describe("emitRunEvents", () => { + it("emits exactly one cli_run on success, with no cli_error and no cli_version", () => { + const telemetry = fakeTelemetry(); + emitRunEvents(telemetry, { + command: "info", + success: true, + durationMs: 5, + ...anon, + }); + + expect(telemetry.capture).toHaveBeenCalledTimes(1); + expect(telemetry.capture).toHaveBeenCalledWith( + "cli_run", + expect.objectContaining({ + command: "info", + success: true, + durationMs: 5, + anonymous: true, + loggedIn: false, + }) + ); + // Version rides on the standard cliVersion property, not a cli_version field. + const properties = telemetry.capture.mock.calls[0]![1] as Record< + string, + unknown + >; + expect(properties).not.toHaveProperty("cli_version"); + }); + + it("emits cli_error then cli_run, in that order and exactly twice", () => { + const telemetry = fakeTelemetry(); + emitRunEvents(telemetry, { + command: "rule create", + success: false, + durationMs: 9, + ...anon, + error: new CLIError("nope", "AUTH_REQUIRED"), + }); + + expect(telemetry.capture).toHaveBeenCalledTimes(2); + expect(telemetry.capture).toHaveBeenNthCalledWith(1, "cli_error", { + command: "rule create", + code: "AUTH_REQUIRED", + }); + expect(telemetry.capture).toHaveBeenNthCalledWith( + 2, + "cli_run", + expect.objectContaining({ command: "rule create", success: false }) + ); + }); + + it("falls back to INTERNAL_ERROR for a thrown non-CLIError", () => { + const telemetry = fakeTelemetry(); + emitRunEvents(telemetry, { + command: "info", + success: false, + durationMs: 1, + ...anon, + error: new Error("boom"), + }); + + expect(telemetry.capture).toHaveBeenCalledWith("cli_error", { + command: "info", + code: "INTERNAL_ERROR", + }); + }); + + it("does NOT emit cli_error for an exitCode-only failure (no thrown error)", () => { + const telemetry = fakeTelemetry(); + emitRunEvents(telemetry, { + command: "check", + success: false, + durationMs: 3, + ...anon, + // no error — failure signalled via process.exitCode + }); + + expect(telemetry.capture).toHaveBeenCalledTimes(1); + expect(telemetry.capture).toHaveBeenCalledWith( + "cli_run", + expect.objectContaining({ success: false }) + ); + const events = telemetry.capture.mock.calls.map( + (call) => call[0] as string + ); + expect(events).not.toContain("cli_error"); + }); + + it("reflects an authenticated identity as loggedIn", () => { + const telemetry = fakeTelemetry(); + emitRunEvents(telemetry, { + command: "info", + success: true, + durationMs: 2, + anonymous: false, + loggedIn: true, + }); + + expect(telemetry.capture).toHaveBeenCalledWith( + "cli_run", + expect.objectContaining({ anonymous: false, loggedIn: true }) + ); + }); +}); diff --git a/packages/cli/test/help-telemetry.test.ts b/packages/cli/test/help-telemetry.test.ts new file mode 100644 index 00000000..5e11937e --- /dev/null +++ b/packages/cli/test/help-telemetry.test.ts @@ -0,0 +1,88 @@ +import { readdirSync, readFileSync } from "node:fs"; +import { join, resolve } from "node:path"; + +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +// Spy on telemetry by mocking the module the help command imports. The factory +// is invoked lazily at import time (same pattern as telemetry.test.ts). +const capture = vi.fn(); +vi.mock("../src/telemetry", () => ({ + getTelemetry: vi.fn(() => + Promise.resolve({ capture, shutdown: () => Promise.resolve() }) + ), + shutdownTelemetry: () => Promise.resolve(), +})); + +const { createHelpCommand } = await import("../src/commands/help"); + +interface RunnableCommand { + run: (context: { + args: { dir: string; anonymous: boolean }; + rawArgs: string[]; + }) => Promise; +} + +async function runHelp(rawArguments: string[]): Promise { + const command = createHelpCommand({}) as unknown as RunnableCommand; + await command.run({ + args: { dir: process.cwd(), anonymous: false }, + rawArgs: rawArguments, + }); +} + +describe("help emits cli_help { topic }", () => { + let logSpy: ReturnType; + let errorSpy: ReturnType; + + beforeEach(() => { + capture.mockClear(); + logSpy = vi.spyOn(console, "log").mockImplementation(() => {}); + errorSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + }); + + afterEach(() => { + logSpy.mockRestore(); + errorSpy.mockRestore(); + }); + + it("captures the served topic", async () => { + await runHelp(["help", "rule", "create"]); + expect(capture).toHaveBeenCalledWith("cli_help", { topic: "rule create" }); + }); + + it("captures the index marker for no topic", async () => { + await runHelp(["help"]); + expect(capture).toHaveBeenCalledWith("cli_help", { topic: "(index)" }); + }); + + it("captures the attempted topic for an unknown topic", async () => { + await runHelp(["help", "nope"]); + expect(capture).toHaveBeenCalledWith("cli_help", { topic: "nope" }); + }); +}); + +// Rather than asserting "no help_* event" inside every behavioral test above, +// prove it once at the source: after this change lands, no legacy help_* event +// name is emitted anywhere in the CLI. +function collectSourceFiles(directory: string): string[] { + const files: string[] = []; + for (const entry of readdirSync(directory, { withFileTypes: true })) { + const full = join(directory, entry.name); + if (entry.isDirectory()) files.push(...collectSourceFiles(full)); + else if (entry.name.endsWith(".ts")) files.push(full); + } + return files; +} + +describe("no legacy help_* event remains in the CLI source", () => { + it("emits no help_* event-name literal under src/", () => { + const sourceDirectory = resolve(import.meta.dirname, "../src"); + // Match a string/template literal that begins with help_ (e.g. "help_index", + // "help_unknown", or a `help_${...}` topic event). + const legacyHelpEvent = /["`]help_/; + const offenders = collectSourceFiles(sourceDirectory).filter((file) => + legacyHelpEvent.test(readFileSync(file, "utf8")) + ); + expect(offenders).toEqual([]); + }); +}); diff --git a/packages/cli/test/telemetry.test.ts b/packages/cli/test/telemetry.test.ts index 671bef6b..76fe4462 100644 --- a/packages/cli/test/telemetry.test.ts +++ b/packages/cli/test/telemetry.test.ts @@ -81,7 +81,7 @@ describe("telemetry disabled", () => { vi.stubEnv("TASKLESS_TELEMETRY_DISABLED", "1"); const telemetry = await getTelemetry(); - telemetry.capture("cli_check"); + telemetry.capture("cli_run"); await telemetry.shutdown(); expect(mockCapture).not.toHaveBeenCalled(); @@ -93,7 +93,7 @@ describe("telemetry disabled", () => { vi.stubEnv("DO_NOT_TRACK", "1"); const telemetry = await getTelemetry(); - telemetry.capture("cli_check"); + telemetry.capture("cli_run"); await telemetry.shutdown(); expect(mockCapture).not.toHaveBeenCalled(); @@ -166,7 +166,7 @@ describe("authenticated identity", () => { await writeTokenFile(cwd, jwt); const telemetry = await getTelemetry(cwd); - telemetry.capture("cli_check"); + telemetry.capture("cli_run"); expect(mockIdentify).toHaveBeenCalledWith( expect.objectContaining({ @@ -203,7 +203,7 @@ describe("authenticated identity", () => { it("falls back to anonymous UUID when no JWT is available", async () => { const telemetry = await getTelemetry(); - telemetry.capture("cli_check"); + telemetry.capture("cli_run"); // distinctId should be the anonymous UUID, not a JWT sub const captureArgument = mockCapture.mock.calls[0]![0] as { @@ -219,7 +219,7 @@ describe("authenticated identity", () => { describe("capture", () => { it("includes cli property on every event", async () => { const telemetry = await getTelemetry(); - telemetry.capture("cli_check"); + telemetry.capture("cli_run"); expect(mockCapture).toHaveBeenCalledWith( expect.objectContaining({ @@ -234,11 +234,11 @@ describe("capture", () => { it("merges custom properties with standard properties", async () => { const telemetry = await getTelemetry(); - telemetry.capture("cli_check", { foo: "bar" }); + telemetry.capture("cli_run", { foo: "bar" }); expect(mockCapture).toHaveBeenCalledWith( expect.objectContaining({ - event: "cli_check", + event: "cli_run", properties: expect.objectContaining({ cli: expect.any(String) as string, foo: "bar", @@ -249,7 +249,7 @@ describe("capture", () => { it("does not include groups when unauthenticated", async () => { const telemetry = await getTelemetry(); - telemetry.capture("cli_check"); + telemetry.capture("cli_run"); const captureArgument = mockCapture.mock.calls[0]![0] as Record< string, @@ -260,7 +260,7 @@ describe("capture", () => { it("includes cliVersion and scaffoldVersion on every anonymous capture", async () => { const telemetry = await getTelemetry(); - telemetry.capture("cli_check"); + telemetry.capture("cli_run"); expect(mockCapture).toHaveBeenCalledWith( expect.objectContaining({ @@ -287,7 +287,7 @@ describe("capture", () => { await writeTokenFile(cwd, jwt); const telemetry = await getTelemetry(cwd); - telemetry.capture("cli_rule_create"); + telemetry.capture("cli_rule_created"); expect(mockCapture).toHaveBeenCalledWith( expect.objectContaining({ @@ -307,7 +307,7 @@ describe("capture", () => { const cwd = await mkdtemp(join(tmpdir(), "taskless-no-manifest-")); try { const telemetry = await getTelemetry(cwd); - telemetry.capture("cli_check"); + telemetry.capture("cli_run"); expect(mockCapture).toHaveBeenCalledWith( expect.objectContaining({ diff --git a/packages/cli/test/wizard-integration.test.ts b/packages/cli/test/wizard-integration.test.ts index e74d2607..d399a6b6 100644 --- a/packages/cli/test/wizard-integration.test.ts +++ b/packages/cli/test/wizard-integration.test.ts @@ -96,14 +96,7 @@ describe("runWizard end-to-end", () => { }; expect(manifest.install.targets[".claude"]?.skills).toContain("taskless"); - expect(captureSpy).toHaveBeenCalledWith( - "cli_init_completed", - expect.objectContaining({ - locations: [".claude"], - optionalSkills: [], - nonInteractive: false, - }) - ); + expect(captureSpy).toHaveBeenCalledWith("cli_installed"); }); it("re-running with the same location is idempotent", async () => { @@ -123,7 +116,7 @@ describe("runWizard end-to-end", () => { ).toBe(true); }); - it("cancelling at locations step writes nothing and emits cli_init_cancelled", async () => { + it("cancelling at locations step writes nothing and emits no install event", async () => { clackResponses.locations = fakeCancelSymbol; const { runWizard } = await import("../src/wizard"); @@ -137,10 +130,11 @@ describe("runWizard end-to-end", () => { ); expect(await exists(join(cwd, ".taskless", "taskless.json"))).toBe(false); - expect(captureSpy).toHaveBeenCalledWith( - "cli_init_cancelled", - expect.objectContaining({ atStep: "locations" }) - ); + // A cancelled wizard installs nothing, so it emits no cli_installed event; + // the invocation itself is captured by cli_run at the runner level. Assert + // on the event name across all calls so extra properties can't slip past. + const events = captureSpy.mock.calls.map((call) => call[0] as string); + expect(events).not.toContain("cli_installed"); }); it("cancelling the summary confirm writes nothing", async () => {