diff --git a/.changeset/telemetry-adoption-dimensions.md b/.changeset/telemetry-adoption-dimensions.md new file mode 100644 index 00000000..802d08b3 --- /dev/null +++ b/.changeset/telemetry-adoption-dimensions.md @@ -0,0 +1,17 @@ +--- +"@taskless/cli": patch +--- + +Telemetry now records six adoption dimensions on every event: `workspaceId` and +`repositoryId` (both hashed), `envOS`, `ci`, `ciProvider`, and `languageStack`. +`cli_check_completed` also reports `ruleCount`, so a scan that loaded no rules +is distinguishable from one that loaded rules and found nothing. + +Nothing to react to. No command changes behaviour, no output changes shape, and +every dimension falls back to a sentinel rather than failing — telemetry is not +a precondition for any command. `TASKLESS_TELEMETRY_DISABLED=1` and +`DO_NOT_TRACK=1` continue to short-circuit before any of it is resolved, so the +opt-out remains an opt-out of the work rather than only of the send. + +`patch` rather than `minor` because the package is pre-1.0, where added surface +does not earn a `minor`, and because none of this is API a consumer can call. diff --git a/openspec/changes/archive/2026-09-09-telemetry-adoption-dimensions/.openspec.yaml b/openspec/changes/archive/2026-09-09-telemetry-adoption-dimensions/.openspec.yaml new file mode 100644 index 00000000..7a8e2be6 --- /dev/null +++ b/openspec/changes/archive/2026-09-09-telemetry-adoption-dimensions/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-09-08 diff --git a/openspec/changes/archive/2026-09-09-telemetry-adoption-dimensions/proposal.md b/openspec/changes/archive/2026-09-09-telemetry-adoption-dimensions/proposal.md new file mode 100644 index 00000000..b6e036e9 --- /dev/null +++ b/openspec/changes/archive/2026-09-09-telemetry-adoption-dimensions/proposal.md @@ -0,0 +1,119 @@ +## Why + +`cli_run` counts invocations. It cannot count _workspaces_, and almost every +adoption metric worth reporting is per-workspace rather than per-invocation. + +The gap is structural, not a matter of tuning dashboards. Today the finest +identity we carry is `cli` — the anonymous UUID in +`$XDG_CONFIG_HOME/taskless/anonymous_id`. It is per config directory, so: + +- one developer working across five repositories is one `cli`, and their five + projects are indistinguishable from five runs in one; +- a fresh CI container regenerates the file on every job, so each CI run looks + like a brand-new install. + +Both distortions push the same direction — they inflate breadth and deflate +depth — and neither is measurable after the fact. There is no property stored on +past events that separates a CI run from a human one, which means the existing +series cannot be cleaned retroactively, only replaced going forward. + +**Retention cannot be backfilled.** A four-week retention number needs four +weeks of a property that already exists. Every week the dimensions are absent is +a week the cohort clock is not running, which is why this change is scoped to +the dimensions and defers the analysis built on top of them. + +## What Changes + +Five new super-properties on every captured event, and one count added to an +existing event. + +**`workspaceId`** — a SHA-256 hash of the workspace root's absolute path, where +the root is the git top-level when there is one and the resolved working +directory otherwise. Anchoring on the top-level is what makes it a workspace +identifier rather than a directory identifier: `check` run from `packages/cli` +and from the repository root must report the same workspace, and they only do if +the path is resolved upward first. + +**`repositoryId`** — a SHA-256 hash of a canonical `{host}/{owner}/{repo}` +derived from the `origin` remote, and **not GitHub-specific**. `ghOwner` is +GitHub-only by construction, because it exists to answer a GitHub question. A +repository identifier answers "how many distinct codebases", which a GitLab or +self-hosted repository participates in exactly as much as a GitHub one, so +excluding them would understate deployment breadth and do it silently. Where no +remote resolves, the sentinel `[unknown]` is sent, matching `ghOwner`'s existing +treatment. + +**`envOS`** — `process.platform`. + +**`ci`** — a boolean, true when `process.env.CI` holds a positive value. + +**`ciProvider`** — the detected provider, `[unknown]` when `ci` is true but no +provider is recognized, `[none]` when `ci` is false. + +**`languageStack`** — the languages evidenced by manifest files at the workspace +root, so the rule corpus can be prioritized against the stacks that actually run +it. + +**`ruleCount` on `cli_check_completed`** — the number of rules the scan had +loaded. The event reports findings but not how many rules were live, so a scan +with zero findings and a scan with zero rules are the same event today. That +also blocks the two ratios that ask whether authored rules become recurring +infrastructure, which is the product's central claim. + +### Two decisions worth stating rather than assuming + +**`ghOwner` stays unhashed.** A GitHub owner is public identity, and the value +is load-bearing precisely because it is legible: excluding `taskless` from +external-owner counts, and eyeballing a cohort for plausibility, both need the +name. Hashing it would buy no privacy that matters and cost the metric its +usefulness. + +**`repositoryId` is hashed, and the reason is not the same reason.** A repository +_name_ can be an unannounced product; an owner name generally cannot. That is +the line: owner legible, repository not. + +Neither hash is a secret, and the spec says so, because a hash of a public +repository URL is reversible by anyone who can enumerate candidate URLs. It is a +stable pseudonym. `workspaceId` is different in kind — local absolute paths +contain usernames and are not enumerable — so hashing there is genuinely +protective. Recording which is which prevents a later reader from assuming a +guarantee that was never made. + +## Capabilities + +### Modified Capabilities + +- `analytics`: the standard-properties requirement covers five new dimensions; + the taxonomy requirement adds `ruleCount` to `cli_check_completed`. + +### Added Capabilities + +- `analytics`: workspace and repository identity, execution environment + dimensions, and the language stack dimension, each as its own requirement, so + the hashing boundary and the sentinel treatment are stated where they apply. + +## Impact + +- `openspec/specs/analytics/spec.md` — two requirements amended, three added. +- `packages/cli/src/telemetry.ts` — resolves and attaches the new super-properties. +- `packages/cli/src/util/git-remote.ts` — gains a host-agnostic repository + canonicalization beside the existing GitHub-only one. +- `packages/cli/src/detect/scan.ts` — `LANGUAGE_MARKERS` is exported for reuse. +- `packages/cli/src/commands/check.ts` — `scanCounts` gains `ruleCount`. +- No command behavior changes, and no failure path is added: every resolution + falls back to a sentinel, consistent with the existing rule that telemetry is + never a precondition. + +## Deferred, deliberately + +- **`cli_rule_verified`.** Fixture-verification counts would need a new event on + `rule verify`, which reverses the standing decision that verify rides on + `cli_run` alone. It serves one lower-tier metric and is not worth reopening + that decision inside a change whose value is time-sensitive. +- **Rule-resolution rate.** Deriving "a finding was fixed" needs stable finding + identity across runs — rule, file, line — which is a materially larger privacy + question than anything here. It should be proposed on its own. +- **`cli_installed` is misnamed.** It fires on `init` and the wizard, so it + measures initialization, not installation. Renaming it is a separate change; + until then, install counts should be derived as the first `cli_run` per `cli` + id rather than from the event that sounds like it. diff --git a/openspec/changes/archive/2026-09-09-telemetry-adoption-dimensions/specs/analytics/spec.md b/openspec/changes/archive/2026-09-09-telemetry-adoption-dimensions/specs/analytics/spec.md new file mode 100644 index 00000000..e8737a8f --- /dev/null +++ b/openspec/changes/archive/2026-09-09-telemetry-adoption-dimensions/specs/analytics/spec.md @@ -0,0 +1,288 @@ +## ADDED Requirements + +### Requirement: Workspace and repository identity + +Telemetry SHALL attach a `workspaceId` and a `repositoryId` property to every +identify and every captured event. + +`workspaceId` SHALL be the SHA-256 hash, hex-encoded, of the absolute path of +the workspace root. The workspace root SHALL be the git top-level directory when +the working directory sits inside a git working tree, and the resolved working +directory otherwise. Resolving upward to the top-level is what makes the value a +workspace identifier rather than a directory identifier: an invocation from a +subdirectory SHALL report the same `workspaceId` as one from the root. + +`repositoryId` SHALL be the SHA-256 hash, hex-encoded, of a canonical +`{host}/{owner}/…/{repo}` string derived from the `origin` remote — the +WHOLE remote path, so two repositories sharing an owner and a leaf name under +different nested groups stay distinct — lowercased, +with any `.git` suffix and trailing slash removed and any userinfo, port, query, +and fragment discarded. It SHALL NOT be restricted to GitHub remotes: a GitLab, +Bitbucket, or self-hosted repository SHALL receive a `repositoryId` on the same +terms. Where no `origin` remote resolves — for any reason, including a directory +that is not a repository and a host where `git` is unavailable — `repositoryId` +SHALL be the literal sentinel `[unknown]`, present rather than omitted, so those +runs stay countable. + +Neither hash SHALL be described or relied upon as a secret. `repositoryId` is a +stable pseudonym: a hash of a remote URL is reversible by anyone who can +enumerate candidate URLs. It is hashed because a repository NAME can be an +unannounced product, which is a different question from whether the value is +confidential. `workspaceId` hashes a local absolute path, which commonly contains +a username and is not enumerable, so hashing there is protective in a way that +hashing `repositoryId` is not. + +`ghOwner` SHALL remain unhashed. A GitHub owner is public identity, and the +value is load-bearing precisely because it is legible: excluding a known owner +from external-adoption counts, and judging a cohort for plausibility, both +require the name. The boundary is therefore owner legible, repository not. + +Resolution SHALL NOT fail a command. Every unresolvable case SHALL produce a +value, never an exception. + +#### Scenario: Invocation from a subdirectory reports the workspace root + +- **WHEN** a command runs in a subdirectory of a git working tree +- **THEN** `workspaceId` SHALL be the hash of the git top-level path +- **AND** SHALL equal the `workspaceId` reported by the same command run at the root + +#### Scenario: Working directory is not a git working tree + +- **WHEN** a command runs outside any git working tree +- **THEN** `workspaceId` SHALL be the hash of the resolved working directory + +#### Scenario: Non-GitHub remote still yields a repository identity + +- **WHEN** a command runs in a repository whose `origin` is hosted somewhere + other than GitHub +- **THEN** `repositoryId` SHALL be a hash of that repository's canonical + `{host}/{owner}/…/{repo}` +- **AND** `ghOwner` SHALL be `[unknown]`, since the GitHub-owner question has no + answer for that remote + +#### Scenario: No origin remote + +- **WHEN** a command runs in a directory with no resolvable `origin` remote +- **THEN** `repositoryId` SHALL be `[unknown]` +- **AND** the property SHALL be present rather than omitted + +#### Scenario: The same repository cloned twice + +- **WHEN** the same repository is cloned to two paths on one machine +- **THEN** the two clones SHALL report the same `repositoryId` +- **AND** SHALL report different `workspaceId` values + +#### Scenario: The GitHub owner is not hashed + +- **WHEN** `ghOwner` and `repositoryId` are recorded for the same GitHub repository +- **THEN** `ghOwner` SHALL be the owner segment verbatim +- **AND** `repositoryId` SHALL be a hash + +### Requirement: Execution environment dimensions + +Telemetry SHALL attach `envOS`, `ci`, and `ciProvider` properties to every +identify and every captured event. + +`envOS` SHALL be the value of `process.platform`. + +`ci` SHALL be a boolean, true when the `CI` environment variable is set to a +positive value. Unset, empty, `"0"`, and `"false"` SHALL each be treated as +false; any other non-empty value SHALL be treated as true. + +`ciProvider` SHALL name the detected continuous-integration provider. When `ci` +is true and no provider is recognized, it SHALL be the literal sentinel +`[unknown]`. When `ci` is false, it SHALL be the literal sentinel `[none]`. Both +SHALL be present rather than omitted, so unrecognized and non-CI runs remain +countable and remain distinguishable from each other. + +These dimensions exist because the anonymous identity cannot separate automated +runs from human ones. `$XDG_CONFIG_HOME/taskless/anonymous_id` is regenerated in +a fresh container, so without `ci` every CI job is indistinguishable from a new +install, and no property stored on past events can repair that after the fact. + +#### Scenario: Local run + +- **WHEN** a command runs with `CI` unset +- **THEN** `ci` SHALL be false +- **AND** `ciProvider` SHALL be `[none]` + +#### Scenario: CI run on a recognized provider + +- **WHEN** a command runs under a recognized CI provider +- **THEN** `ci` SHALL be true +- **AND** `ciProvider` SHALL name that provider + +#### Scenario: CI run on an unrecognized provider + +- **WHEN** a command runs with `CI` set to a positive value and no known + provider environment variable present +- **THEN** `ci` SHALL be true +- **AND** `ciProvider` SHALL be `[unknown]` + +#### Scenario: CI is set to a negative value + +- **WHEN** a command runs with `CI` set to `"0"`, `"false"`, or the empty string +- **THEN** `ci` SHALL be false + +### Requirement: Language stack dimension + +Telemetry SHALL attach a `languageStack` property to every identify and every +captured event, listing the languages evidenced by manifest files present at the +workspace root. + +The property exists to prioritize the rule corpus against the stacks that +actually run it, so its value is a coarse dimension rather than a detection +result. + +It SHALL be resolved from a bounded, root-only probe and SHALL NOT invoke +`detectRepository`. The detection scan performs a recursive walk with manifest +parsing, which is acceptable for a command the user asked for and is not +acceptable on every invocation, including the `agent` fetches an agent makes +repeatedly. + +The probe SHALL read its language-to-manifest mapping from the same +`LANGUAGE_MARKERS` constant the detection scan uses, extended with the Node +manifest that the scan derives JavaScript and TypeScript from. The two SHALL +differ only in search scope. This is stated so a future reader does not +reconcile them by making the telemetry probe recursive: a root-only probe misses +a language confined to a sub-package of a monorepo, and that is the accepted +cost of the property being free. + +Where no manifest is found, `languageStack` SHALL be an empty array rather than +omitted. + +#### Scenario: Root manifests are reported + +- **WHEN** a command runs in a workspace whose root contains a `package.json` + and a `go.mod` +- **THEN** `languageStack` SHALL include the languages both manifests evidence + +#### Scenario: No manifests present + +- **WHEN** a command runs in a workspace root with no recognized manifest +- **THEN** `languageStack` SHALL be an empty array +- **AND** the property SHALL be present rather than omitted + +#### Scenario: The probe does not run the detection scan + +- **WHEN** telemetry resolves `languageStack` +- **THEN** it SHALL NOT call `detectRepository` + +#### Scenario: A language confined to a sub-package + +- **WHEN** a monorepo's root carries only a `package.json` and a Python service + lives in a sub-directory +- **THEN** `languageStack` SHALL report the root evidence only +- **AND** this SHALL NOT be treated as a defect in the property + +## MODIFIED Requirements + +### Requirement: All capture calls include standard properties + +Every `capture()` call SHALL include the `cli` property (anonymous UUID), the `cliVersion` property (the version the build reports as its own, baked in at build time), and the `scaffoldVersion` property (the `version` field from `.taskless/taskless.json`, or `0` if the manifest is absent or unreadable). It SHALL also include the adoption dimensions `workspaceId`, `repositoryId`, `envOS`, `ci`, `ciProvider`, and `languageStack`, each defined by its own requirement. When authenticated, the `groups` parameter SHALL include `{ organization: String(orgId) }`. The `cliVersion` and `scaffoldVersion` values SHALL be resolved once at telemetry initialization and attached to every subsequent `capture()` call without re-reading the source files, and the adoption dimensions SHALL be resolved once on the same terms. + +Resolving the adoption dimensions SHALL NOT be a precondition for any command. Each one has a defined value for every failure of resolution, so a capture never has to choose between omitting a property and failing. + +#### Scenario: Anonymous capture includes standard properties + +- **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_created")` is called with authentication +- **THEN** the event SHALL include `{ cli: anonymousUuid, cliVersion: , scaffoldVersion: }` +- **AND** the `groups` parameter SHALL include `{ organization: String(orgId) }` + +#### Scenario: Scaffold version falls back to 0 when manifest missing + +- **WHEN** `getTelemetry(cwd)` is initialized in a directory with no `.taskless/taskless.json` +- **THEN** every `capture()` call from the returned client SHALL include `scaffoldVersion: 0` + +#### Scenario: CLI version is the version the build reports as its own + +- **WHEN** `getTelemetry()` is initialized +- **THEN** `cliVersion` SHALL be the version the build reports as its own, bundled at build time +- **AND** for every build target but `nightly` that SHALL be the version in `packages/cli/package.json` +- **AND** for a `nightly` it SHALL be the version the nightly is published under, so events are attributed to the build that emitted them rather than to the release it anticipates +- **AND** SHALL be attached to every event emitted through the returned client + +#### Scenario: Every event carries the adoption dimensions + +- **WHEN** any event is captured, authenticated or not +- **THEN** it SHALL include `workspaceId`, `repositoryId`, `envOS`, `ci`, `ciProvider`, and `languageStack` + +#### Scenario: The dimensions are resolved once + +- **WHEN** several events are captured within one invocation +- **THEN** each adoption dimension SHALL be resolved once at initialization +- **AND** SHALL NOT be re-read per event + +### Requirement: CLI events use cli\_ prefix + +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 and the number of rules the + scan had loaded: `errorCount`, `warningCount`, `findings`, `ruleCount`. + Counts only, never rule content, rule names, or matched source + - `cli_error` — a single failure event with `command` and `code` (a stable + `CLIErrorCode`) +- `cli_agent` — fired when the `agent` 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: Recipe fetch emits cli_agent with a topic + +- **WHEN** an agent runs `taskless agent create-sg-rule` +- **THEN** PostHog SHALL receive a `cli_agent` event with `topic: "create-sg-rule"` +- **AND** SHALL NOT receive a `help_create_sg_rule` event + +#### Scenario: Fetch with no topic emits cli_agent with the index marker + +- **WHEN** an agent runs `taskless agent` +- **THEN** PostHog SHALL receive a `cli_agent` event with `topic: "(index)"` +- **AND** SHALL NOT receive a `help_index` event + +#### Scenario: A command failure emits cli_error + +- **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 this release +- **THEN** PostHog SHALL NOT receive any event named `cli__completed`, + `help_index`, `help_`, or `help_unknown` + +#### Scenario: A completed scan reports how many rules were loaded + +- **WHEN** a scan completes +- **THEN** the `cli_check_completed` event SHALL include `ruleCount`, the number + of rules the scan had loaded +- **AND** a scan that loaded no rules SHALL be distinguishable from a scan that + loaded rules and found nothing diff --git a/openspec/changes/archive/2026-09-09-telemetry-adoption-dimensions/tasks.md b/openspec/changes/archive/2026-09-09-telemetry-adoption-dimensions/tasks.md new file mode 100644 index 00000000..abc43356 --- /dev/null +++ b/openspec/changes/archive/2026-09-09-telemetry-adoption-dimensions/tasks.md @@ -0,0 +1,55 @@ +# Tasks + +**Delivery shape: single PR.** The dimensions are only useful together — a +`workspaceId` without `ci` produces workspace counts that CI still inflates, and +shipping them in sequence would start each cohort clock on a different date, so +the first comparable window would be the last property's. One reviewable diff, +no stack. + +## 1. Resolve workspace and repository identity + +- [x] 1.1 Add a workspace-root resolver: `git rev-parse --show-toplevel`, falling back to the resolved working directory when the command fails for any reason, including git being absent. Never throws — the existing `resolveRepositoryContext` is the shape to follow +- [x] 1.2 Hash the root path with SHA-256, hex-encoded, as `workspaceId`. Assert in a test that a subdirectory and the root produce the same value, since that equality is the whole reason the resolver looks upward +- [x] 1.3 Add a host-agnostic repository canonicalization beside `canonicalizeGitHubUrl`. **Do not extend the GitHub one**: it throws `UNSUPPORTED_REMOTE_HOST` deliberately, and that refusal is a capability boundary on remote rule generation that this change must not soften +- [x] 1.4 Cover the remote forms `canonicalOwnerUrl` already handles — scp-like SSH, `ssh://`, `git://`, `https://`, bare owner — at repository granularity rather than owner granularity +- [x] 1.5 Hash it as `repositoryId`; emit `[unknown]` when no remote resolves, present rather than omitted +- [x] 1.6 Test that a GitLab or self-hosted remote yields a real `repositoryId` while `ghOwner` is `[unknown]`. That combination is the point of the property and is the case a GitHub-shaped implementation silently gets wrong + +## 2. Resolve the execution environment + +- [x] 2.1 Add `envOS` from `process.platform` +- [x] 2.2 Add `ci`, treating unset, empty, `"0"`, and `"false"` as false and any other non-empty value as true. `init.ts` already reads `process.env.CI` for interactivity with a narrower test (`"true"`/`"1"`); leave it alone and note the difference — one decides whether to prompt, the other classifies a run, and they are allowed to disagree +- [x] 2.3 Add `ciProvider` with a provider table, `[unknown]` when `ci` is true and nothing matches, `[none]` when `ci` is false +- [x] 2.4 Test all three branches of `ciProvider`. The `[unknown]`/`[none]` distinction is the one that matters: collapsing them loses the ability to tell an unrecognized provider from a local run + +## 3. Resolve the language stack + +- [x] 3.1 Export `LANGUAGE_MARKERS` from `detect/scan.ts` and read the probe's mapping from it, so the two cannot disagree about which manifest means which language +- [x] 3.2 Add the Node manifest to the probe's inputs. `LANGUAGE_MARKERS` has no entry for JavaScript or TypeScript — the scan derives those from `package.json` separately — so a probe built from the constant alone reports nothing for the stack the CLI is most used on +- [x] 3.3 Probe the workspace root only, with `existsSync`, and assert in a test that `detectRepository` is not called. The scan is a recursive walk with manifest parsing, and this runs on every invocation including `agent` +- [x] 3.4 Emit an empty array, not an omitted property, when nothing matches + +## 4. Attach the dimensions + +- [x] 4.1 Resolve all six once in `getTelemetry` and attach them to `identify` and to every `capture`, alongside `cli`/`cliVersion`/`scaffoldVersion`/`ghOwner` +- [x] 4.2 Keep every resolution inside the existing failure-tolerant path: a dimension that cannot be resolved yields its sentinel, and telemetry that fails entirely still falls back to the no-op client +- [x] 4.3 Confirm `TASKLESS_TELEMETRY_DISABLED=1` and `DO_NOT_TRACK=1` still short-circuit before any resolution runs — no git spawn, no filesystem probe, no `anonymous_id` read. The opt-out has to stay an opt-out of the work, not only of the send + +## 5. Report the rule count + +- [x] 5.1 Add `ruleCount` to `scanCounts` in `check.ts`, set from the rules the scan loaded +- [x] 5.2 Set it on the path that assigns `scanCounts` today, so a scan that throws after loading rules keeps reporting nothing rather than reporting a partial count + +## 6. Say so in the spec + +- [x] 6.1 Add the three new requirements: workspace and repository identity, execution environment dimensions, language stack dimension +- [x] 6.2 Amend the standard-properties requirement to list the six dimensions +- [x] 6.3 Amend the taxonomy requirement's `cli_check_completed` bullet to include `ruleCount` +- [x] 6.4 Carry every existing scenario into both MODIFIED deltas — 4 in standard-properties, 5 in the taxonomy requirement — and keep both TITLES byte-identical, including the escaped underscore in `CLI events use cli\_ prefix`. A delta replaces a requirement rather than patching it, and a renamed title applies nothing at all +- [x] 6.5 Verify by archiving on a scratch commit and grepping the standing spec for every prior scenario, then resetting to the recorded SHA + +## 7. Close out + +- [x] 7.1 Run `pnpm typecheck`, `pnpm lint`, and `pnpm test` +- [x] 7.2 Add a changeset. Pre-1.0, added telemetry surface is a `patch`: no consumer must react to it +- [x] 7.3 Archive the change diff --git a/openspec/specs/analytics/spec.md b/openspec/specs/analytics/spec.md index 778ff629..07ec1004 100644 --- a/openspec/specs/analytics/spec.md +++ b/openspec/specs/analytics/spec.md @@ -107,7 +107,9 @@ The PostHog client SHALL be created with `flushAt: 1` and `flushInterval: 0` bec ### Requirement: All capture calls include standard properties -Every `capture()` call SHALL include the `cli` property (anonymous UUID), the `cliVersion` property (the version the build reports as its own, baked in at build time), and the `scaffoldVersion` property (the `version` field from `.taskless/taskless.json`, or `0` if the manifest is absent or unreadable). When authenticated, the `groups` parameter SHALL include `{ organization: String(orgId) }`. The `cliVersion` and `scaffoldVersion` values SHALL be resolved once at telemetry initialization and attached to every subsequent `capture()` call without re-reading the source files. +Every `capture()` call SHALL include the `cli` property (anonymous UUID), the `cliVersion` property (the version the build reports as its own, baked in at build time), and the `scaffoldVersion` property (the `version` field from `.taskless/taskless.json`, or `0` if the manifest is absent or unreadable). It SHALL also include the adoption dimensions `workspaceId`, `repositoryId`, `envOS`, `ci`, `ciProvider`, and `languageStack`, each defined by its own requirement. When authenticated, the `groups` parameter SHALL include `{ organization: String(orgId) }`. The `cliVersion` and `scaffoldVersion` values SHALL be resolved once at telemetry initialization and attached to every subsequent `capture()` call without re-reading the source files, and the adoption dimensions SHALL be resolved once on the same terms. + +Resolving the adoption dimensions SHALL NOT be a precondition for any command. Each one has a defined value for every failure of resolution, so a capture never has to choose between omitting a property and failing. #### Scenario: Anonymous capture includes standard properties @@ -134,6 +136,17 @@ Every `capture()` call SHALL include the `cli` property (anonymous UUID), the `c - **AND** for a `nightly` it SHALL be the version the nightly is published under, so events are attributed to the build that emitted them rather than to the release it anticipates - **AND** SHALL be attached to every event emitted through the returned client +#### Scenario: Every event carries the adoption dimensions + +- **WHEN** any event is captured, authenticated or not +- **THEN** it SHALL include `workspaceId`, `repositoryId`, `envOS`, `ci`, `ciProvider`, and `languageStack` + +#### Scenario: The dimensions are resolved once + +- **WHEN** several events are captured within one invocation +- **THEN** each adoption dimension SHALL be resolved once at initialization +- **AND** SHALL NOT be re-read per event + ### Requirement: CLI events use cli\_ prefix CLI events SHALL use the `cli_` prefix, with the taxonomy organized as a @@ -148,8 +161,9 @@ CLI events SHALL use the `cli_` prefix, with the taxonomy organized as a - `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_check_completed` — error/warning counts and the number of rules the + scan had loaded: `errorCount`, `warningCount`, `findings`, `ruleCount`. + Counts only, never rule content, rule names, or matched source - `cli_error` — a single failure event with `command` and `code` (a stable `CLIErrorCode`) - `cli_agent` — fired when the `agent` command serves a request, with a `topic` @@ -193,6 +207,14 @@ Commands that carry no concrete state beyond the invocation (e.g. `info`, - **THEN** PostHog SHALL NOT receive any event named `cli__completed`, `help_index`, `help_`, or `help_unknown` +#### Scenario: A completed scan reports how many rules were loaded + +- **WHEN** a scan completes +- **THEN** the `cli_check_completed` event SHALL include `ruleCount`, the number + of rules the scan had loaded +- **AND** a scan that loaded no rules SHALL be distinguishable from a scan that + loaded rules and found nothing + ### Requirement: Wrong-topic re-routing is observable as a derivable funnel The taxonomy SHALL keep wrong-topic re-routing derivable as a funnel signal from @@ -343,3 +365,178 @@ An unresolvable owner SHALL NOT affect the command: it is a telemetry value, not - **WHEN** `ghOwner` is recorded - **THEN** the CLI SHALL NOT infer or record whether the owner is an organization or a user account + +### Requirement: Workspace and repository identity + +Telemetry SHALL attach a `workspaceId` and a `repositoryId` property to every +identify and every captured event. + +`workspaceId` SHALL be the SHA-256 hash, hex-encoded, of the absolute path of +the workspace root. The workspace root SHALL be the git top-level directory when +the working directory sits inside a git working tree, and the resolved working +directory otherwise. Resolving upward to the top-level is what makes the value a +workspace identifier rather than a directory identifier: an invocation from a +subdirectory SHALL report the same `workspaceId` as one from the root. + +`repositoryId` SHALL be the SHA-256 hash, hex-encoded, of a canonical +`{host}/{owner}/…/{repo}` string derived from the `origin` remote — the +WHOLE remote path, so two repositories sharing an owner and a leaf name under +different nested groups stay distinct — lowercased, +with any `.git` suffix and trailing slash removed and any userinfo, port, query, +and fragment discarded. It SHALL NOT be restricted to GitHub remotes: a GitLab, +Bitbucket, or self-hosted repository SHALL receive a `repositoryId` on the same +terms. Where no `origin` remote resolves — for any reason, including a directory +that is not a repository and a host where `git` is unavailable — `repositoryId` +SHALL be the literal sentinel `[unknown]`, present rather than omitted, so those +runs stay countable. + +Neither hash SHALL be described or relied upon as a secret. `repositoryId` is a +stable pseudonym: a hash of a remote URL is reversible by anyone who can +enumerate candidate URLs. It is hashed because a repository NAME can be an +unannounced product, which is a different question from whether the value is +confidential. `workspaceId` hashes a local absolute path, which commonly contains +a username and is not enumerable, so hashing there is protective in a way that +hashing `repositoryId` is not. + +`ghOwner` SHALL remain unhashed. A GitHub owner is public identity, and the +value is load-bearing precisely because it is legible: excluding a known owner +from external-adoption counts, and judging a cohort for plausibility, both +require the name. The boundary is therefore owner legible, repository not. + +Resolution SHALL NOT fail a command. Every unresolvable case SHALL produce a +value, never an exception. + +#### Scenario: Invocation from a subdirectory reports the workspace root + +- **WHEN** a command runs in a subdirectory of a git working tree +- **THEN** `workspaceId` SHALL be the hash of the git top-level path +- **AND** SHALL equal the `workspaceId` reported by the same command run at the root + +#### Scenario: Working directory is not a git working tree + +- **WHEN** a command runs outside any git working tree +- **THEN** `workspaceId` SHALL be the hash of the resolved working directory + +#### Scenario: Non-GitHub remote still yields a repository identity + +- **WHEN** a command runs in a repository whose `origin` is hosted somewhere + other than GitHub +- **THEN** `repositoryId` SHALL be a hash of that repository's canonical + `{host}/{owner}/…/{repo}` +- **AND** `ghOwner` SHALL be `[unknown]`, since the GitHub-owner question has no + answer for that remote + +#### Scenario: No origin remote + +- **WHEN** a command runs in a directory with no resolvable `origin` remote +- **THEN** `repositoryId` SHALL be `[unknown]` +- **AND** the property SHALL be present rather than omitted + +#### Scenario: The same repository cloned twice + +- **WHEN** the same repository is cloned to two paths on one machine +- **THEN** the two clones SHALL report the same `repositoryId` +- **AND** SHALL report different `workspaceId` values + +#### Scenario: The GitHub owner is not hashed + +- **WHEN** `ghOwner` and `repositoryId` are recorded for the same GitHub repository +- **THEN** `ghOwner` SHALL be the owner segment verbatim +- **AND** `repositoryId` SHALL be a hash + +### Requirement: Execution environment dimensions + +Telemetry SHALL attach `envOS`, `ci`, and `ciProvider` properties to every +identify and every captured event. + +`envOS` SHALL be the value of `process.platform`. + +`ci` SHALL be a boolean, true when the `CI` environment variable is set to a +positive value. Unset, empty, `"0"`, and `"false"` SHALL each be treated as +false; any other non-empty value SHALL be treated as true. + +`ciProvider` SHALL name the detected continuous-integration provider. When `ci` +is true and no provider is recognized, it SHALL be the literal sentinel +`[unknown]`. When `ci` is false, it SHALL be the literal sentinel `[none]`. Both +SHALL be present rather than omitted, so unrecognized and non-CI runs remain +countable and remain distinguishable from each other. + +These dimensions exist because the anonymous identity cannot separate automated +runs from human ones. `$XDG_CONFIG_HOME/taskless/anonymous_id` is regenerated in +a fresh container, so without `ci` every CI job is indistinguishable from a new +install, and no property stored on past events can repair that after the fact. + +#### Scenario: Local run + +- **WHEN** a command runs with `CI` unset +- **THEN** `ci` SHALL be false +- **AND** `ciProvider` SHALL be `[none]` + +#### Scenario: CI run on a recognized provider + +- **WHEN** a command runs under a recognized CI provider +- **THEN** `ci` SHALL be true +- **AND** `ciProvider` SHALL name that provider + +#### Scenario: CI run on an unrecognized provider + +- **WHEN** a command runs with `CI` set to a positive value and no known + provider environment variable present +- **THEN** `ci` SHALL be true +- **AND** `ciProvider` SHALL be `[unknown]` + +#### Scenario: CI is set to a negative value + +- **WHEN** a command runs with `CI` set to `"0"`, `"false"`, or the empty string +- **THEN** `ci` SHALL be false + +### Requirement: Language stack dimension + +Telemetry SHALL attach a `languageStack` property to every identify and every +captured event, listing the languages evidenced by manifest files present at the +workspace root. + +The property exists to prioritize the rule corpus against the stacks that +actually run it, so its value is a coarse dimension rather than a detection +result. + +It SHALL be resolved from a bounded, root-only probe and SHALL NOT invoke +`detectRepository`. The detection scan performs a recursive walk with manifest +parsing, which is acceptable for a command the user asked for and is not +acceptable on every invocation, including the `agent` fetches an agent makes +repeatedly. + +The probe SHALL read its language-to-manifest mapping from the same +`LANGUAGE_MARKERS` constant the detection scan uses, extended with the Node +manifest that the scan derives JavaScript and TypeScript from. The two SHALL +differ only in search scope. This is stated so a future reader does not +reconcile them by making the telemetry probe recursive: a root-only probe misses +a language confined to a sub-package of a monorepo, and that is the accepted +cost of the property being free. + +Where no manifest is found, `languageStack` SHALL be an empty array rather than +omitted. + +#### Scenario: Root manifests are reported + +- **WHEN** a command runs in a workspace whose root contains a `package.json` + and a `go.mod` +- **THEN** `languageStack` SHALL include the languages both manifests evidence + +#### Scenario: No manifests present + +- **WHEN** a command runs in a workspace root with no recognized manifest +- **THEN** `languageStack` SHALL be an empty array +- **AND** the property SHALL be present rather than omitted + +#### Scenario: The probe does not run the detection scan + +- **WHEN** telemetry resolves `languageStack` +- **THEN** it SHALL NOT call `detectRepository` + +#### Scenario: A language confined to a sub-package + +- **WHEN** a monorepo's root carries only a `package.json` and a Python service + lives in a sub-directory +- **THEN** `languageStack` SHALL report the root evidence only +- **AND** this SHALL NOT be treated as a defect in the property diff --git a/packages/cli/src/commands/check.ts b/packages/cli/src/commands/check.ts index 618738f9..4f237aa7 100644 --- a/packages/cli/src/commands/check.ts +++ b/packages/cli/src/commands/check.ts @@ -124,7 +124,12 @@ export const checkCommand = defineCommand({ // Set when a scan actually runs; drives cli_check_completed with counts // only (never matched code). let scanCounts: - | { errorCount: number; warningCount: number; findings: number } + | { + errorCount: number; + warningCount: number; + findings: number; + ruleCount: number; + } | undefined; try { const positionalPaths = extractPositionalPaths(rawArgs); @@ -269,7 +274,24 @@ export const checkCommand = defineCommand({ if (result.severity === "error") errorCount++; else if (result.severity === "warning") warningCount++; } - scanCounts = { errorCount, warningCount, findings: results.length }; + // `ruleCount` is how many rules the scan LOADED, across all three + // engines. Without it a scan with no findings and a scan with no rules + // are the same event, which is exactly the pair the metrics need to + // tell apart. Runtime rules the plan skipped still count: the question + // is how many rules this workspace has configured, not how many + // executed on this run. + // + // `listRuleIds` swallows its own read errors and returns `[]`, so this + // cannot turn an unreadable directory into a failed scan. A telemetry + // count must never be the thing that fails a command. + const valeRuleIds = await listRuleIds(cwd, "vale"); + scanCounts = { + errorCount, + warningCount, + findings: results.length, + ruleCount: + astGrepRuleIds.length + valeRuleIds.length + runtimeRules.length, + }; // Computed by `runEngines`, not here: the exit code is a fact about a // completed dispatch, and an engine failure has to fail the check even diff --git a/packages/cli/src/detect/scan.ts b/packages/cli/src/detect/scan.ts index 6c019a05..1de74565 100644 --- a/packages/cli/src/detect/scan.ts +++ b/packages/cli/src/detect/scan.ts @@ -231,8 +231,19 @@ const LINTER_SIGNALS: readonly LinterSignal[] = [ * Languages inferred from the presence of a manifest or marker file anywhere in * the tree. JavaScript and TypeScript are resolved separately (they share * `package.json`). + * + * Exported so telemetry's root-only `languageStack` probe reads the same + * language-to-manifest mapping this scan does. The two differ only in search + * scope: this walks the tree, the probe checks the workspace root. Sharing the + * mapping keeps them from disagreeing about which manifest means which + * language; the scope difference is deliberate and documented there. Note the + * probe adds its own Node entry, since the JavaScript/TypeScript resolution + * mentioned above happens outside this constant. */ -const LANGUAGE_MARKERS: ReadonlyArray<{ language: string; files: string[] }> = [ +export const LANGUAGE_MARKERS: ReadonlyArray<{ + language: string; + files: string[]; +}> = [ { language: "Python", files: [ diff --git a/packages/cli/src/telemetry.ts b/packages/cli/src/telemetry.ts index ee7dacc9..1a164562 100644 --- a/packages/cli/src/telemetry.ts +++ b/packages/cli/src/telemetry.ts @@ -6,6 +6,7 @@ import { PostHog } from "posthog-node"; import { decodeJwt } from "jose"; import { decodeOrgId, NIL_ORG_ID } from "./auth/jwt"; +import { resolveAdoptionDimensions } from "./util/adoption-dimensions"; import { resolveRepositoryContext, UNKNOWN_GH_OWNER } from "./util/git-remote"; import { getConfigDirectory, getToken } from "./auth/token"; import { CLI_VERSION } from "./version"; @@ -162,7 +163,27 @@ export async function getTelemetry(cwd?: string): Promise { orgSubject = decodeOrgId(token) ?? NIL_ORG_ID; } - const scaffoldVersion = await resolveScaffoldVersion(cwd); + // These three are independent of each other and each spawns a subprocess + // or reads the filesystem, so they run concurrently rather than in a + // chain. `getTelemetry` runs on essentially every invocation, and this was + // three sequential git/fs round trips in front of every command. + // + // Everything here runs only when telemetry is enabled: the opt-out returns + // the no-op client before `getTelemetry` reaches this point, so + // `DO_NOT_TRACK=1` costs no git spawn and no filesystem probe. The opt-out + // has to be an opt-out of the WORK, not only of the send. + const [scaffoldVersion, repository, dimensions] = await Promise.all([ + resolveScaffoldVersion(cwd), + // Which GitHub owner is using the CLI, including anonymously — see the + // note below on why this is resolved from the git remote. + cwd ? resolveRepositoryContext(cwd) : undefined, + // The adoption dimensions: workspace and repository identity, execution + // environment, and language stack. Resolved ONCE, like `cliVersion` and + // `scaffoldVersion`, and attached to identify and to every capture. It + // handles an absent `cwd` itself, sentinelling only the three dimensions + // that depend on one. + resolveAdoptionDimensions(cwd), + ]); // Which GitHub owner is using the CLI, including anonymously — that is // the question this property exists to answer, so it is resolved from the @@ -184,7 +205,6 @@ export async function getTelemetry(cwd?: string): Promise { // never throws, so a host with no git installed lands here like any other // unresolvable case. No `cwd` is treated the same way, matching // `resolveScaffoldVersion` above; every real call site passes one. - const repository = cwd ? await resolveRepositoryContext(cwd) : undefined; const ghOwner = repository ? repository.ghOwner : UNKNOWN_GH_OWNER; posthog = new PostHog(POSTHOG_PROJECT_TOKEN, { @@ -201,6 +221,7 @@ export async function getTelemetry(cwd?: string): Promise { cliVersion: CLI_VERSION, scaffoldVersion, ghOwner, + ...dimensions, }, }); @@ -225,6 +246,7 @@ export async function getTelemetry(cwd?: string): Promise { cliVersion: CLI_VERSION, scaffoldVersion, ghOwner, + ...dimensions, }, ...(!anonymous && orgSubject !== undefined ? { groups: { organization: String(orgSubject) } } diff --git a/packages/cli/src/util/adoption-dimensions.ts b/packages/cli/src/util/adoption-dimensions.ts new file mode 100644 index 00000000..ea9c5989 --- /dev/null +++ b/packages/cli/src/util/adoption-dimensions.ts @@ -0,0 +1,252 @@ +import { execFile } from "node:child_process"; +import { createHash } from "node:crypto"; +import { existsSync } from "node:fs"; +import { resolve } from "node:path"; + +import { LANGUAGE_MARKERS } from "../detect/scan"; +import { resolveRepositoryPath } from "./git-remote"; + +/** + * The value a dimension carries when it cannot be resolved. + * + * A sentinel rather than an omitted property, matching `UNKNOWN_GH_OWNER`: + * runs that cannot resolve a dimension stay countable instead of vanishing + * from aggregates. Hashes are hex, so no real value can collide with it. + */ +export const UNKNOWN_DIMENSION = "[unknown]"; + +/** `ciProvider` when the run is not CI at all, as distinct from unrecognized. */ +export const NO_CI_PROVIDER = "[none]"; + +/** Hex SHA-256. Shared so `workspaceId` and `repositoryId` cannot diverge. */ +function hashIdentity(value: string): string { + return createHash("sha256").update(value).digest("hex"); +} + +/** + * The absolute path of the workspace root: the git top-level when `cwd` is + * inside a working tree, otherwise `cwd` resolved. + * + * Resolving upward is the entire reason this exists. `check` run from + * `packages/cli` and `check` run from the repository root are the same + * workspace, and they only report the same `workspaceId` if the path is taken + * to the top level first. Anchoring on `cwd` would make every subdirectory its + * own workspace and inflate the count without bound. + * + * Asks git rather than looking for a `.git` directory, for the reason + * `isGitWorkTree` gives: a worktree, a submodule and a `GIT_DIR` override are + * all real working trees with no `.git` directory at `cwd`. + * + * Never rejects. + */ +export function resolveWorkspaceRoot(cwd: string): Promise { + return new Promise((resolvePromise) => { + execFile( + "git", + ["rev-parse", "--show-toplevel"], + { cwd }, + (error, stdout) => { + const top = stdout.trim(); + resolvePromise(error || !top ? resolve(cwd) : resolve(top)); + } + ); + }); +} + +/** + * A stable identifier for the workspace, hashed. + * + * Hashing here is genuinely protective, unlike in `resolveRepositoryId` below: + * a local absolute path commonly contains a username, and the space of local + * paths is not enumerable, so the hash cannot be walked back to its input. + * + * Two clones of one repository report DIFFERENT values, which is correct — + * they are two workspaces — while `repositoryId` reports the same for both. + * That split is what lets "how many checkouts" and "how many codebases" be + * counted separately. + */ +export async function resolveWorkspaceId(cwd: string): Promise { + return hashIdentity(await resolveWorkspaceRoot(cwd)); +} + +/** + * A stable identifier for the repository, hashed, or `[unknown]`. + * + * The hash is NOT a secret and must not be described as one: a remote URL is + * reversible by anyone who can enumerate candidate URLs. It is hashed because + * a repository NAME can be an unannounced product, which is a different + * question from confidentiality. `ghOwner` stays unhashed alongside it, + * because an owner is public identity and the value is load-bearing precisely + * when legible — excluding a known owner from external-adoption counts needs + * the name. Owner legible, repository not. + */ +export async function resolveRepositoryId(cwd: string): Promise { + const path = await resolveRepositoryPath(cwd); + return path ? hashIdentity(path) : UNKNOWN_DIMENSION; +} + +/** + * Whether `CI` holds a positive value. + * + * Unset, empty, `"0"` and `"false"` are false; any other non-empty value is + * true. This is DELIBERATELY wider than `init.ts`'s interactivity check, which + * accepts only `"true"`/`"1"`. The two answer different questions — one + * decides whether to prompt a human, this one classifies a run for analytics — + * and a provider exporting `CI=yes` should count as CI even where erring + * toward prompting would be wrong. They are allowed to disagree. + */ +export function isContinuousIntegration( + environment: NodeJS.ProcessEnv = process.env +): boolean { + const value = environment.CI?.trim().toLowerCase(); + if (!value) return false; + return value !== "0" && value !== "false"; +} + +/** + * Environment variables that name a CI provider, most specific first. + * + * Ordered because providers nest: Codespaces and many self-hosted setups also + * export `CI`, and GitHub Actions sets both `GITHUB_ACTIONS` and `CI`. The + * first match wins, so a more specific marker is never shadowed by a generic + * one. + */ +const CI_PROVIDERS: ReadonlyArray<{ variable: string; name: string }> = [ + { variable: "GITHUB_ACTIONS", name: "github_actions" }, + { variable: "GITLAB_CI", name: "gitlab_ci" }, + { variable: "CIRCLECI", name: "circleci" }, + { variable: "BUILDKITE", name: "buildkite" }, + { variable: "TRAVIS", name: "travis" }, + { variable: "APPVEYOR", name: "appveyor" }, + { variable: "DRONE", name: "drone" }, + { variable: "TEAMCITY_VERSION", name: "teamcity" }, + { variable: "BITBUCKET_BUILD_NUMBER", name: "bitbucket_pipelines" }, + { variable: "TF_BUILD", name: "azure_pipelines" }, + { variable: "CODEBUILD_BUILD_ID", name: "aws_codebuild" }, + { variable: "JENKINS_URL", name: "jenkins" }, + { variable: "WOODPECKER", name: "woodpecker" }, + { variable: "VERCEL", name: "vercel" }, + { variable: "NETLIFY", name: "netlify" }, +]; + +/** + * The detected CI provider, `[unknown]` on CI with no recognized marker, or + * `[none]` off CI. + * + * Two sentinels rather than one, because collapsing them loses the ability to + * tell "a provider we have not taught this list about" from "a laptop". The + * first is a gap in the table worth closing; the second is the ordinary case + * and needs no action. + */ +export function resolveCiProvider( + environment: NodeJS.ProcessEnv = process.env +): string { + if (!isContinuousIntegration(environment)) return NO_CI_PROVIDER; + const matched = CI_PROVIDERS.find( + ({ variable }) => (environment[variable] ?? "").trim() !== "" + ); + return matched ? matched.name : UNKNOWN_DIMENSION; +} + +/** + * Node's manifest, which `LANGUAGE_MARKERS` has no entry for. + * + * The detection scan derives JavaScript and TypeScript from `package.json` + * dependencies rather than from a marker, so a probe built from the shared + * constant alone reports NOTHING for the stack this CLI is most used on. Added + * here rather than to `LANGUAGE_MARKERS` itself, since the scan already has a + * richer answer for Node and would then have two. + */ +const NODE_MARKER = { + language: "JavaScript/TypeScript", + files: ["package.json"], +}; + +/** + * The languages evidenced by manifest files AT THE WORKSPACE ROOT. + * + * Deliberately not `detectRepository`. That performs a recursive glob with + * manifest parsing, which is fine for a command the user asked for and is not + * fine on every invocation — including the `agent` fetches an agent makes + * repeatedly. This is a bounded `existsSync` per marker. + * + * The mapping comes from the scan's own `LANGUAGE_MARKERS` so the two cannot + * disagree about which manifest means which language; they differ only in + * search scope. That difference has a real cost: a language confined to a + * sub-package of a monorepo does not appear here. It is accepted, because this + * is a coarse telemetry dimension rather than a detection result, and it is + * the price of the property being free. Do not "fix" it by making this + * recursive. + */ +export function resolveLanguageStack(workspaceRoot: string): string[] { + const languages: string[] = []; + for (const marker of [NODE_MARKER, ...LANGUAGE_MARKERS]) { + if (marker.files.some((file) => existsSync(resolve(workspaceRoot, file)))) { + languages.push(marker.language); + } + } + return languages; +} + +/** Every adoption dimension, resolved once per process by `getTelemetry`. */ +export interface AdoptionDimensions { + workspaceId: string; + repositoryId: string; + envOS: string; + ci: boolean; + ciProvider: string; + languageStack: string[]; +} + +/** + * Resolve all six dimensions. + * + * Never rejects: each dimension has a defined value for every failure of + * resolution, so a capture never has to choose between omitting a property and + * failing. Telemetry is not a precondition for any command. + * + * With no `cwd`, only the three dimensions that DEPEND on one fall back to + * their sentinels, matching how `resolveScaffoldVersion` and `ghOwner` treat + * the same case. `envOS`, `ci` and `ciProvider` are properties of the process + * rather than of a directory, so they stay real — sentinelling them would + * discard a known answer to look consistent. + * + * The earlier version defaulted to `process.cwd()` here, which contradicted + * the comment at the call site claiming it behaved like its neighbours: it + * resolved live git state instead. Unreachable today, since every call site + * passes a `cwd`, and a trap for the next one that does not. + */ +export async function resolveAdoptionDimensions( + cwd: string | undefined +): Promise { + const environment = { + envOS: process.platform, + ci: isContinuousIntegration(), + ciProvider: resolveCiProvider(), + }; + + if (!cwd) { + return { + workspaceId: UNKNOWN_DIMENSION, + repositoryId: UNKNOWN_DIMENSION, + languageStack: [], + ...environment, + }; + } + + // Independent lookups, so they run concurrently: each spawns its own git + // process and neither reads the other's answer. `languageStack` is the + // exception and stays sequential — it probes the workspace root, so it + // cannot start until that root is known. + const [workspaceRoot, repositoryId] = await Promise.all([ + resolveWorkspaceRoot(cwd), + resolveRepositoryId(cwd), + ]); + + return { + workspaceId: hashIdentity(workspaceRoot), + repositoryId, + languageStack: resolveLanguageStack(workspaceRoot), + ...environment, + }; +} diff --git a/packages/cli/src/util/git-remote.ts b/packages/cli/src/util/git-remote.ts index 16d2946e..9388860c 100644 --- a/packages/cli/src/util/git-remote.ts +++ b/packages/cli/src/util/git-remote.ts @@ -257,3 +257,92 @@ export async function listRemoteOwnerUrls(cwd: string): Promise { } return owners; } + +/** + * Reduce a git remote reference to a canonical REPOSITORY path, + * `{host}/{owner}/{repo}`, or `null` when it carries no owner/repo pair. + * + * Host-agnostic, unlike `canonicalizeGitHubUrl` above. That function throws + * `UNSUPPORTED_REMOTE_HOST` for a non-GitHub remote, and the refusal is + * load-bearing: it is the capability boundary on REMOTE rule generation. This + * answers a different question, "which codebase is this", which a GitLab, + * Bitbucket or self-hosted repository participates in exactly as much as a + * GitHub one. Teaching the GitHub parser to accept other hosts would have + * softened a refusal doing real work elsewhere, so the two sit side by side. + * + * Parsing follows `canonicalOwnerUrl` — scp-like SSH, `ssh://`, `git://`, + * `https://`, scheme-relative and bare paths — differing only in taking two + * path segments rather than one. Host, owner and repository are lowercased: + * GitHub treats all three case-insensitively, so `Foo/Bar` and `foo/bar` are + * one repository and must not become two identities. + * + * Never throws. A remote it cannot parse yields `null`, which the caller turns + * into the `[unknown]` sentinel. + */ +export function canonicalRepositoryPath(remote: string): string | null { + const raw = remote.trim(); + if (!raw) return null; + + let host = "github.com"; + let path = raw; + + const sshRemote = /^[^@/]+@([^:/]+):(.+)$/.exec(raw); + if (sshRemote) { + host = sshRemote[1] ?? host; + path = sshRemote[2] ?? path; + } else if (/^[a-z][a-z0-9+.-]*:\/\//i.test(raw) || raw.startsWith("//")) { + try { + const url = new URL(raw.startsWith("//") ? `https:${raw}` : raw); + host = url.hostname; + path = url.pathname; + } catch { + // Not parseable as a URL — treat the input as a path, as + // `canonicalOwnerUrl` does for the same inputs. + } + } + + host = host.toLowerCase().replace(/^www\./, ""); + const segments = path + .replace(/^\/+/, "") + .split("/") + .filter((segment) => segment.length > 0); + if (segments.length < 2) return null; + + // The WHOLE path is kept, not just the first and last segments. A + // self-hosted GitLab serves repositories under nested subgroups, and + // collapsing them looked attractive because a renamed group would then not + // change the identity. It trades the wrong way: dropping the middle makes + // `acme/team1/api` and `acme/team2/api` — two unrelated repositories that + // happen to share an owner and a leaf name — one identity, silently + // undercounting in exactly the nested-group case this parser exists to + // serve. A rename is the milder failure and the visible one: it starts a + // new id on a date someone can find, where a collision is never visible at + // all. It is also the same class of event as a GitHub transfer, which + // already changes the identity here and is accepted. + const cleaned = segments.map((segment) => segment.toLowerCase()); + cleaned[cleaned.length - 1] = cleaned.at(-1)!.replace(/\.git$/i, ""); + if (cleaned.some((segment) => segment.length === 0)) return null; + + return `${host}/${cleaned.join("/")}`; +} + +/** + * The canonical repository path for `cwd`'s `origin` remote, or `null`. + * + * Reads `origin` alone rather than walking the `REMOTE_PRECEDENCE` fallback + * `listRemoteOwnerUrls` uses. A repository identity has to be the SAME value + * for every clone of one codebase, and precedence makes it depend on which + * remotes a given clone happens to have configured: a fork with `origin` on + * the fork and `upstream` on the source would report whichever the ordering + * picked, so two clones of the same fork could disagree. + * + * Never throws. Not a repository, no `origin`, an unparseable remote, and git + * missing from the host all yield `null`. + */ +export async function resolveRepositoryPath( + cwd: string +): Promise { + const remotes = await listRemoteConfig(cwd); + const origin = remotes.find((remote) => remote.name === "origin"); + return origin ? canonicalRepositoryPath(origin.url) : null; +} diff --git a/packages/cli/test/adoption-dimensions.test.ts b/packages/cli/test/adoption-dimensions.test.ts new file mode 100644 index 00000000..c70e120b --- /dev/null +++ b/packages/cli/test/adoption-dimensions.test.ts @@ -0,0 +1,218 @@ +import { execFileSync } from "node:child_process"; +import { mkdtempSync, mkdirSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join, resolve } from "node:path"; + +import { describe, expect, it } from "vitest"; + +import { + NO_CI_PROVIDER, + UNKNOWN_DIMENSION, + isContinuousIntegration, + resolveCiProvider, + resolveLanguageStack, + resolveRepositoryId, + resolveWorkspaceId, + resolveWorkspaceRoot, +} from "../src/util/adoption-dimensions"; +import { canonicalRepositoryPath } from "../src/util/git-remote"; + +/** + * Real git repositories rather than a mocked spawn. + * + * The behaviour under test IS git's answer — what `--show-toplevel` reports + * from a subdirectory, and what a remote URL looks like once configured — so a + * stub would only assert that the stub was written to match. The unresolvable + * cases (git absent, spawn failure) are covered by the existing mocked suite + * in `git-remote-context.test.ts`, which is the right shape for those. + */ +function makeRepository(remote?: string): string { + const root = mkdtempSync(join(tmpdir(), "taskless-dimensions-")); + execFileSync("git", ["init", "-q"], { cwd: root }); + if (remote) { + execFileSync("git", ["remote", "add", "origin", remote], { cwd: root }); + } + return root; +} + +describe("workspaceId identifies the workspace, not the directory", () => { + it("reports the git top level from a subdirectory", async () => { + const root = makeRepository(); + const nested = join(root, "packages", "cli"); + mkdirSync(nested, { recursive: true }); + + // The equality IS the requirement. `check` from a package directory and + // `check` from the repository root are one workspace, and anchoring on + // cwd instead of the top level would make every subdirectory its own. + expect(await resolveWorkspaceId(nested)).toBe( + await resolveWorkspaceId(root) + ); + }); + + it("falls back to the working directory outside a git work tree", async () => { + const plain = mkdtempSync(join(tmpdir(), "taskless-plain-")); + expect(await resolveWorkspaceRoot(plain)).toBe(resolve(plain)); + expect(await resolveWorkspaceId(plain)).toMatch(/^[\da-f]{64}$/); + }); + + it("distinguishes two clones of one repository", async () => { + const remote = "git@github.com:taskless/cli.git"; + const first = makeRepository(remote); + const second = makeRepository(remote); + + // Different workspaces, same codebase. That split is the whole point of + // carrying both properties. + expect(await resolveWorkspaceId(first)).not.toBe( + await resolveWorkspaceId(second) + ); + expect(await resolveRepositoryId(first)).toBe( + await resolveRepositoryId(second) + ); + }); +}); + +describe("repositoryId is host-agnostic", () => { + it("resolves a non-GitHub remote", async () => { + const root = makeRepository("git@gitlab.com:acme/widgets.git"); + + // The case a GitHub-shaped implementation silently gets wrong: a GitLab + // repository is a codebase like any other and must be counted. + expect(await resolveRepositoryId(root)).toMatch(/^[\da-f]{64}$/); + expect(await resolveRepositoryId(root)).not.toBe(UNKNOWN_DIMENSION); + }); + + it("reports the sentinel with no origin remote", async () => { + const root = makeRepository(); + expect(await resolveRepositoryId(root)).toBe(UNKNOWN_DIMENSION); + }); + + it("reports the sentinel outside a repository", async () => { + const plain = mkdtempSync(join(tmpdir(), "taskless-plain-")); + expect(await resolveRepositoryId(plain)).toBe(UNKNOWN_DIMENSION); + }); +}); + +describe("canonicalRepositoryPath", () => { + it("reduces every remote form to one identity", () => { + const expected = "github.com/taskless/cli"; + for (const remote of [ + "git@github.com:taskless/cli.git", + "git@github.com:taskless/cli", + "https://github.com/taskless/cli.git", + "https://github.com/taskless/cli", + "ssh://git@github.com/taskless/cli.git", + "git://github.com/taskless/cli.git", + "https://user:token@github.com:443/taskless/cli.git?ref=main#top", + "//github.com/taskless/cli", + ]) { + expect(canonicalRepositoryPath(remote), remote).toBe(expected); + } + }); + + it("lowercases, because GitHub treats the path case-insensitively", () => { + expect(canonicalRepositoryPath("https://GitHub.com/Taskless/CLI")).toBe( + "github.com/taskless/cli" + ); + }); + + it("keeps non-GitHub hosts distinct", () => { + expect(canonicalRepositoryPath("git@gitlab.com:acme/widgets.git")).toBe( + "gitlab.com/acme/widgets" + ); + expect(canonicalRepositoryPath("https://git.internal/acme/widgets")).toBe( + "git.internal/acme/widgets" + ); + }); + + it("keeps the whole nested group path", () => { + expect( + canonicalRepositoryPath("git@gitlab.com:acme/team/sub/widgets.git") + ).toBe("gitlab.com/acme/team/sub/widgets"); + }); + + it("does not collapse two repositories that share an owner and a leaf name", () => { + // The reason the whole path is kept (taskless/cli#326 review). Dropping + // the middle segments merged these two into one identity, silently + // undercounting distinct codebases in exactly the nested-group case this + // parser exists to serve. + expect( + canonicalRepositoryPath("git@gitlab.com:acme/team1/api.git") + ).not.toBe(canonicalRepositoryPath("git@gitlab.com:acme/team2/api.git")); + }); + + it("returns null when there is no owner/repo pair", () => { + expect(canonicalRepositoryPath("")).toBeNull(); + expect(canonicalRepositoryPath("taskless")).toBeNull(); + expect(canonicalRepositoryPath("https://github.com/taskless")).toBeNull(); + }); +}); + +describe("the CI dimensions", () => { + it("treats any non-negative value as CI", () => { + for (const value of ["1", "true", "yes", "woodpecker"]) { + expect(isContinuousIntegration({ CI: value }), value).toBe(true); + } + }); + + it("treats unset, empty, 0 and false as not CI", () => { + expect(isContinuousIntegration({})).toBe(false); + for (const value of ["", " ", "0", "false", "FALSE"]) { + expect(isContinuousIntegration({ CI: value }), value).toBe(false); + } + }); + + it("names a recognized provider", () => { + expect(resolveCiProvider({ CI: "true", GITHUB_ACTIONS: "true" })).toBe( + "github_actions" + ); + expect(resolveCiProvider({ CI: "true", BUILDKITE: "true" })).toBe( + "buildkite" + ); + }); + + it("distinguishes an unrecognized provider from a local run", () => { + // The two sentinels carry different meanings and must not collapse: one + // is a gap in the provider table, the other is an ordinary laptop. + expect(resolveCiProvider({ CI: "true" })).toBe(UNKNOWN_DIMENSION); + expect(resolveCiProvider({})).toBe(NO_CI_PROVIDER); + }); + + it("prefers the specific marker over the generic one", () => { + // Providers nest — GitHub Actions sets CI as well — so ordering decides. + expect( + resolveCiProvider({ + CI: "true", + GITHUB_ACTIONS: "true", + JENKINS_URL: "x", + }) + ).toBe("github_actions"); + }); +}); + +describe("languageStack", () => { + it("reports the languages its root manifests evidence", () => { + const root = mkdtempSync(join(tmpdir(), "taskless-stack-")); + writeFileSync(join(root, "package.json"), "{}"); + writeFileSync(join(root, "go.mod"), "module example\n"); + + const stack = resolveLanguageStack(root); + expect(stack).toContain("JavaScript/TypeScript"); + expect(stack).toContain("Go"); + }); + + it("reports an empty array rather than nothing when no manifest is present", () => { + const root = mkdtempSync(join(tmpdir(), "taskless-empty-")); + expect(resolveLanguageStack(root)).toEqual([]); + }); + + it("does not see a language confined to a sub-package", () => { + // Documented and accepted: the probe is root-only so it can run on every + // invocation. Pinned so nobody "fixes" it into a recursive walk. + const root = mkdtempSync(join(tmpdir(), "taskless-mono-")); + writeFileSync(join(root, "package.json"), "{}"); + mkdirSync(join(root, "services", "api"), { recursive: true }); + writeFileSync(join(root, "services", "api", "pyproject.toml"), ""); + + expect(resolveLanguageStack(root)).not.toContain("Python"); + }); +}); diff --git a/packages/cli/test/telemetry.test.ts b/packages/cli/test/telemetry.test.ts index 9ebf84cd..f3087429 100644 --- a/packages/cli/test/telemetry.test.ts +++ b/packages/cli/test/telemetry.test.ts @@ -468,3 +468,110 @@ describe("ghOwner", () => { } }); }); + +/** The properties on the most recent call recorded by a posthog mock. */ +function lastProperties(mock: { + mock: { calls: unknown[][] }; +}): Record { + const call = mock.mock.calls.at(-1)?.[0] as { + properties?: Record; + }; + return call.properties ?? {}; +} + +/** Every adoption dimension, by name, as attached to identify and capture. */ +const DIMENSION_KEYS = [ + "workspaceId", + "repositoryId", + "envOS", + "ci", + "ciProvider", + "languageStack", +] as const; + +/** + * The dimensions reach `identify` and `capture`. + * + * `adoption-dimensions.test.ts` covers how each value is RESOLVED. This covers + * that the resolved values are actually attached, which is a separate claim + * and the one the whole change rests on: the wiring is two `...dimensions` + * spreads, and dropping either would leave every resolver test passing while + * no event carried a single new property. + */ +describe("adoption dimensions are attached to events", () => { + it("carries every dimension on identify", async () => { + const cwd = await mkdtemp(join(tmpdir(), "taskless-dimensions-")); + try { + await execFileAsync("git", ["init"], { cwd }); + + await getTelemetry(cwd); + + const properties = lastProperties(mockIdentify); + for (const key of DIMENSION_KEYS) { + expect(properties, key).toHaveProperty(key); + } + } finally { + await rm(cwd, { recursive: true, force: true }); + } + }); + + it("carries every dimension on capture", async () => { + const cwd = await mkdtemp(join(tmpdir(), "taskless-dimensions-capture-")); + try { + await execFileAsync("git", ["init"], { cwd }); + + const telemetry = await getTelemetry(cwd); + telemetry.capture("cli_run", { command: "check" }); + + const properties = lastProperties(mockCapture); + for (const key of DIMENSION_KEYS) { + expect(properties, key).toHaveProperty(key); + } + // The event's own properties survive the spread rather than being + // overwritten by it. + expect(properties.command).toBe("check"); + } finally { + await rm(cwd, { recursive: true, force: true }); + } + }); + + it("resolves real values, not just present keys", async () => { + const cwd = await mkdtemp(join(tmpdir(), "taskless-dimensions-values-")); + try { + await execFileAsync("git", ["init"], { cwd }); + await execFileAsync( + "git", + ["remote", "add", "origin", "git@github.com:acme/widgets.git"], + { cwd } + ); + await writeFile(join(cwd, "package.json"), "{}"); + + const telemetry = await getTelemetry(cwd); + telemetry.capture("cli_run"); + + const properties = lastProperties(mockCapture); + // Hashes, so asserted by shape rather than by value — the values + // themselves are covered in `adoption-dimensions.test.ts`. + expect(properties.workspaceId).toMatch(/^[\da-f]{64}$/); + expect(properties.repositoryId).toMatch(/^[\da-f]{64}$/); + expect(properties.envOS).toBe(process.platform); + expect(properties.languageStack).toContain("JavaScript/TypeScript"); + } finally { + await rm(cwd, { recursive: true, force: true }); + } + }); + + it("sentinels only the cwd-dependent dimensions when no cwd is given", async () => { + // `envOS`, `ci` and `ciProvider` are properties of the process rather than + // of a directory, so they stay real. Sentinelling them would discard a + // known answer for the sake of looking consistent. + await getTelemetry(); + + const properties = lastProperties(mockIdentify); + expect(properties.workspaceId).toBe("[unknown]"); + expect(properties.repositoryId).toBe("[unknown]"); + expect(properties.languageStack).toEqual([]); + expect(properties.envOS).toBe(process.platform); + expect(properties.ciProvider).toEqual(expect.any(String)); + }); +});