From 7cb4b57caf5b5373671b70e95b05c802f5b812b0 Mon Sep 17 00:00:00 2001 From: Jean-Michel Bouchard Date: Sun, 19 Jul 2026 15:12:18 -0400 Subject: [PATCH 01/11] Add telemetry consent and identity core Persist default-off pseudonymous telemetry state outside generated projects, expose explicit user controls, and document the privacy contract. Checks: make check (656 tests) Refs ENG-197 Refs ENG-198 --- .gitignore | 1 + docs/README.md | 1 + docs/contracts/README.md | 1 + docs/contracts/telemetry.md | 117 +++++++++++++ src/cli/main.py | 2 + src/cli/telemetry.py | 94 ++++++++++ src/model/__init__.py | 1 + src/model/dto/__init__.py | 31 ++++ src/model/dto/telemetry.py | 174 +++++++++++++++++++ src/telemetry/__init__.py | 1 + src/telemetry/policy.py | 67 +++++++ src/telemetry/sink.py | 31 ++++ src/telemetry/state.py | 193 +++++++++++++++++++++ src/utils/errors.py | 5 + tests/unit/cli/test_telemetry_cli.py | 75 ++++++++ tests/unit/model/dto/test_telemetry_dto.py | 84 +++++++++ tests/unit/telemetry/test_policy.py | 79 +++++++++ tests/unit/telemetry/test_sink.py | 58 +++++++ tests/unit/telemetry/test_state.py | 131 ++++++++++++++ 19 files changed, 1146 insertions(+) create mode 100644 docs/contracts/telemetry.md create mode 100644 src/cli/telemetry.py create mode 100644 src/model/__init__.py create mode 100644 src/model/dto/__init__.py create mode 100644 src/model/dto/telemetry.py create mode 100644 src/telemetry/__init__.py create mode 100644 src/telemetry/policy.py create mode 100644 src/telemetry/sink.py create mode 100644 src/telemetry/state.py create mode 100644 tests/unit/cli/test_telemetry_cli.py create mode 100644 tests/unit/model/dto/test_telemetry_dto.py create mode 100644 tests/unit/telemetry/test_policy.py create mode 100644 tests/unit/telemetry/test_sink.py create mode 100644 tests/unit/telemetry/test_state.py diff --git a/.gitignore b/.gitignore index da7a919..5cf5fb8 100644 --- a/.gitignore +++ b/.gitignore @@ -2,6 +2,7 @@ __pycache__/ *.pyc *.log .venv/ +/.env .mypy_cache/ .claude/settings.local.json /kickstart diff --git a/docs/README.md b/docs/README.md index 8b3f1c0..24b15bd 100644 --- a/docs/README.md +++ b/docs/README.md @@ -5,6 +5,7 @@ - [Agent Map](../AGENTS.md): repo-local map for agents modifying kickstart itself. - [Architecture](architecture/README.md): canonical architecture entrypoint for kickstart itself. - [Contracts](contracts/README.md): public and generated-output contracts. +- [Pseudonymous CLI Telemetry](contracts/telemetry.md): default-off consent, identity, and data-minimization contract. - [Scaffold Contract](scaffold-contract.md): generated docs, metadata, and option vocabulary. - [Decisions](decisions/README.md): durable project decisions. - [CLI Framework Research](decisions/cli-framework-research.md): sourced recommendation for Rust, Python, and TypeScript CLI framework defaults. diff --git a/docs/contracts/README.md b/docs/contracts/README.md index 19afcc8..a36a451 100644 --- a/docs/contracts/README.md +++ b/docs/contracts/README.md @@ -8,6 +8,7 @@ Current contract references: - [Plan Command Drift Scope](plan-drift-scope.md): what `kickstart plan` covers, defers, and emits. - [Adoption Tiers](adoption-tiers.md): Level 1 conformant vs Level 2 managed, exit codes, and JSON shape. - [Backstage Export](backstage-export.md): derived/declared/passthrough field classes and re-export semantics. +- [Pseudonymous CLI Telemetry](telemetry.md): consent, identity, event allowlist, privacy, and provider boundaries. - [Install Binaries](../install-binaries.md): release asset names and install commands. - [Human Guide](../human-guide.md): supported project kinds, languages, runtimes, and CLI options. diff --git a/docs/contracts/telemetry.md b/docs/contracts/telemetry.md new file mode 100644 index 0000000..bf4ba15 --- /dev/null +++ b/docs/contracts/telemetry.md @@ -0,0 +1,117 @@ +# Pseudonymous CLI telemetry contract + +This contract governs the default-off product telemetry emitted by the kickstart CLI. It is a data-minimization boundary, not a promise that a stable identifier is anonymous: a durable random identifier correlates events over time and is therefore pseudonymous. + +## Product questions + +The first event exists only to answer these questions: + +1. Which project kinds, languages, runtimes, and optional scaffold capabilities are useful? +2. Which supported combinations are most common? +3. Which coarse failure categories prevent scaffold creation? +4. Which released CLI versions and packaged platforms are still active? +5. Is the event contract useful enough to justify maintaining telemetry or a first-party relay? + +Telemetry is deliberately lossy and spoofable. It is product feedback, not billing, audit, security, identity, unique-person, or authoritative usage evidence. + +## Consent and effective enablement + +- Telemetry is disabled by default. +- No identity or state file is created by ordinary commands before explicit opt-in. +- `kickstart telemetry enable` persists opt-in and lazily creates a random UUIDv4. +- `kickstart telemetry disable` persists opt-out and retains an existing ID. +- `kickstart telemetry status` reads state without creating it and displays the current ID when one exists. +- `kickstart telemetry reset-id` rotates an existing ID for future events and reports the previous ID. Rotation does not delete historical events. + +Hard process-level opt-outs override persisted opt-in in this order: + +1. `DO_NOT_TRACK=1` +2. `KICKSTART_TELEMETRY_DISABLED=1` +3. a truthy `CI` environment variable +4. pytest via `PYTEST_CURRENT_TEST` +5. `KICKSTART_EVAL=1` +6. execution from the kickstart source checkout + +A deliberate development-only verification may bypass the source-checkout guard with `KICKSTART_TELEMETRY_ALLOW_DEVELOPMENT=1`; it does not bypass consent or any other hard opt-out. + +## Identity and persistence + +State is stored separately from repository configuration and the replaceable application payload: + +```text +${XDG_CONFIG_HOME:-~/.config}/kickstart/telemetry.json +``` + +The versioned document contains only `schema_version`, `consent`, and an optional random UUIDv4 `anonymous_id`. The ID is never derived from a username, hostname, MAC address, Git configuration, path, email address, project, repository, or PostHog value. Atomic writes, serialized initialization, and user-only permissions are used where the platform supports them. + +Missing, malformed, unreadable, busy, or unwritable state fails closed. Telemetry failures never change the output, exit status, generated files, or deterministic behavior of another command. + +## Initial event allowlist + +The only initial event is `scaffold_create_completed`. One event may be attempted for an opted-in `kickstart create` invocation that reaches a terminal handled outcome. Parser failures that occur before the command function starts are outside this boundary. + +The provider-neutral envelope contains only: + +- `event_id`: random UUIDv4 for deduplication +- `anonymous_id`: the persisted random UUIDv4 +- `occurred_at`: UTC event time +- `name`: the allowlisted event name +- `properties`: the closed property object below + +The initial property allowlist is: + +- `cli_version` +- `project_type` +- `language` +- `runtime` +- `cloud` +- `framework` +- `database` +- `cache` +- `auth` +- `knowledge` +- `workspace_tooling` +- `helm` +- `github_requested` +- `interactive` +- `outcome` +- `error_category` +- `duration_bucket` +- `platform` +- `architecture` + +Canonical supported values, fixed booleans, fixed outcome/error categories, and duration buckets are permitted. Unknown or invalid inputs are omitted or represented by fixed categories; their raw values are never serialized. + +## Prohibited data + +No telemetry event, local telemetry file, diagnostic, or provider log may contain: + +- project or organization names +- roots, paths, current working directories, usernames, or home directories +- repository metadata, branches, commits, remotes, or URLs +- raw command arguments, prompts, configuration values, or environment values +- tokens, credentials, secrets, generated content, manifests, diffs, stdout, or stderr +- raw exception types, messages, stack traces, or arbitrary error text +- free-form user feedback + +Free-form feedback, if added later, must be a separate user-initiated surface with its own disclosure and storage contract. + +## Provider boundary and network behavior + +CLI event producers depend only on provider-neutral DTOs and a `TelemetrySink`. Phase one maps the approved envelope directly to PostHog through an isolated adapter. PostHog DTOs live under `src/model/dto/`; provider behavior lives under `src/telemetry/`. + +The initial destination is the dedicated `kickstart-cli` project on PostHog Cloud US. Direct capture uses the public `phc_` project token only. A `phx_` personal API key or any read-capable credential must never enter the application, repository, build, logs, tests, or issues. + +Every PostHog event sets `$process_person_profile` to `false` and `$geoip_disable` to `true`. The adapter never identifies, aliases, groups, or sets person properties. The PostHog project must discard captured IP data. Because phase one connects directly, PostHog still receives the HTTPS connection before applying its project-level discard policy. + +Direct public capture can be spoofed. Provider, destination, or token changes require upgraded direct clients, and there is no first-party server kill switch in phase one. A later first-party relay may replace the sink without changing event producers, consent, or identity. + +For local development, credentials may be exported from an ignored `.env` into the process. The CLI never auto-loads a current repository's `.env`, because kickstart runs inside arbitrary repositories and repository content must not influence telemetry routing or consent. The numeric PostHog project ID is not part of capture payloads. + +## Delivery and retention + +Delivery is best-effort: one request, a small timeout, no retries, and no local raw-event queue. State, mapping, serialization, and transport exceptions are contained within telemetry. + +The initial account uses PostHog Cloud US on the Free plan with its current default one-year event retention. Retention is an account setting and must be rechecked before release. + +Historical deletion is handled on request for a supplied pseudonymous ID. `status` exposes the current ID; `reset-id` reports the previous one. Rotation affects future events only. The deletion workflow must explicitly delete events, must be verified using a synthetic ID, and must never expose the required personal API key. The public deletion-request contact channel remains a release TODO. diff --git a/src/cli/main.py b/src/cli/main.py index b4d97e6..5ce2d9c 100644 --- a/src/cli/main.py +++ b/src/cli/main.py @@ -31,6 +31,7 @@ from src.generator.docs_plan import DocsPlanTargetError, inspect_docs from src.cli.options import CreateCommandOptions, CreateOptions, ResolvedCreateArgs from src.cli.prompts import ConfirmReader, PromptReader, prompt_for_missing_args +from src.cli.telemetry import telemetry_app from src.utils.errors import KickstartError from src.utils.config import load_config from src.utils.installer import ( @@ -56,6 +57,7 @@ logger = logging.getLogger(__name__) app: typer.Typer = typer.Typer(help="kickstart: Full-stack project scaffolding CLI") +app.add_typer(telemetry_app, name="telemetry") def _print_version_and_exit(value: bool) -> None: diff --git a/src/cli/telemetry.py b/src/cli/telemetry.py new file mode 100644 index 0000000..64a703f --- /dev/null +++ b/src/cli/telemetry.py @@ -0,0 +1,94 @@ +"""User controls for kickstart pseudonymous telemetry.""" + +import json + +import typer + +from src.model.dto.telemetry import EffectiveTelemetry, TelemetryState, TelemetrySuppressionReason +from src.telemetry.policy import resolve_telemetry +from src.telemetry.state import TelemetryStateStore +from src.utils.errors import TelemetryStateError + + +telemetry_app = typer.Typer(help="Inspect and control default-off pseudonymous telemetry.") + + +def _state_store() -> TelemetryStateStore: + return TelemetryStateStore.from_environment() + + +@telemetry_app.command("status") +def telemetry_status( + json_output: bool = typer.Option(False, "--json", help="Emit machine-readable telemetry status."), +) -> None: + """Show persisted consent, effective status, identity, and state location.""" + store = _state_store() + try: + state = store.read() + effective = resolve_telemetry(store, state=state) + consent = state.consent.value + except TelemetryStateError: + state = TelemetryState() + effective = EffectiveTelemetry( + enabled=False, + reason=TelemetrySuppressionReason.INVALID_STATE, + anonymous_id=None, + ) + consent = "invalid" + payload = { + "anonymous_id": str(state.anonymous_id) if state.anonymous_id is not None else None, + "consent": consent, + "effective": effective.enabled, + "reason": effective.reason.value, + "state_file": str(store.path), + } + if json_output: + typer.echo(json.dumps(payload, sort_keys=True)) + return + typer.echo(f"consent: {payload['consent']}") + typer.echo(f"effective: {'enabled' if effective.enabled else 'disabled'}") + typer.echo(f"reason: {payload['reason']}") + typer.echo(f"anonymous-id: {payload['anonymous_id'] or 'not-created'}") + typer.echo(f"state-file: {payload['state_file']}") + + +@telemetry_app.command("enable") +def telemetry_enable() -> None: + """Explicitly opt in and create an identity only when needed.""" + try: + state = _state_store().enable() + except TelemetryStateError as exc: + _exit_with_state_error(exc) + typer.echo("Telemetry enabled.") + typer.echo(f"anonymous-id: {state.anonymous_id}") + + +@telemetry_app.command("disable") +def telemetry_disable() -> None: + """Explicitly opt out while retaining any existing identity.""" + try: + state = _state_store().disable() + except TelemetryStateError as exc: + _exit_with_state_error(exc) + typer.echo("Telemetry disabled.") + typer.echo(f"anonymous-id: {state.anonymous_id or 'not-created'}") + + +@telemetry_app.command("reset-id") +def telemetry_reset_id() -> None: + """Rotate an existing ID for future events without deleting history.""" + try: + result = _state_store().reset_id() + except TelemetryStateError as exc: + _exit_with_state_error(exc) + if result.previous_id is None: + typer.echo("No telemetry ID exists; nothing was changed.") + return + typer.echo(f"previous-anonymous-id: {result.previous_id}") + typer.echo(f"anonymous-id: {result.current_id}") + typer.echo("Historical events are not deleted; request deletion separately using the previous ID.") + + +def _exit_with_state_error(error: TelemetryStateError) -> None: + typer.echo(f"Telemetry state error: {error}", err=True) + raise typer.Exit(code=1) from error diff --git a/src/model/__init__.py b/src/model/__init__.py new file mode 100644 index 0000000..cb92f9a --- /dev/null +++ b/src/model/__init__.py @@ -0,0 +1 @@ +"""Typed domain and provider models.""" diff --git a/src/model/dto/__init__.py b/src/model/dto/__init__.py new file mode 100644 index 0000000..9e6c704 --- /dev/null +++ b/src/model/dto/__init__.py @@ -0,0 +1,31 @@ +"""Data-transfer objects shared across kickstart boundaries.""" + +from src.model.dto.telemetry import ( + EffectiveTelemetry, + ScaffoldCreateErrorCategory, + ScaffoldCreateOutcome, + ScaffoldCreateProperties, + TelemetryDurationBucket, + TelemetryConsent, + TelemetryEnvelope, + TelemetryEvent, + TelemetryEventName, + TelemetryIdentityReset, + TelemetryState, + TelemetrySuppressionReason, +) + +__all__ = [ + "EffectiveTelemetry", + "ScaffoldCreateErrorCategory", + "ScaffoldCreateOutcome", + "ScaffoldCreateProperties", + "TelemetryConsent", + "TelemetryDurationBucket", + "TelemetryEnvelope", + "TelemetryEvent", + "TelemetryEventName", + "TelemetryIdentityReset", + "TelemetryState", + "TelemetrySuppressionReason", +] diff --git a/src/model/dto/telemetry.py b/src/model/dto/telemetry.py new file mode 100644 index 0000000..135427c --- /dev/null +++ b/src/model/dto/telemetry.py @@ -0,0 +1,174 @@ +"""Provider-neutral telemetry data-transfer objects.""" + +from dataclasses import dataclass +from datetime import datetime +from enum import StrEnum +from typing import TypeAlias, TypedDict +from uuid import UUID + + +TELEMETRY_STATE_SCHEMA_VERSION = 1 +TelemetryPropertyValue: TypeAlias = str | int | float | bool + + +class TelemetryConsent(StrEnum): + """The user's persisted telemetry preference.""" + + UNSET = "unset" + ENABLED = "enabled" + DISABLED = "disabled" + + +class TelemetryEventName(StrEnum): + """Closed event-name allowlist for the initial telemetry contract.""" + + SCAFFOLD_CREATE_COMPLETED = "scaffold_create_completed" + + +class ScaffoldCreateOutcome(StrEnum): + """Closed terminal outcomes for a create invocation.""" + + SUCCESS = "success" + CANCELLED = "cancelled" + INPUT_ENDED = "input_ended" + EXPECTED_ERROR = "expected_error" + INVALID_CONFIGURATION = "invalid_configuration" + UNEXPECTED_ERROR = "unexpected_error" + + +class ScaffoldCreateErrorCategory(StrEnum): + """Coarse error categories that never contain raw exception data.""" + + NONE = "none" + INTERRUPTED = "interrupted" + INPUT_ENDED = "input_ended" + UNSUPPORTED_PROJECT_TYPE = "unsupported_project_type" + UNSUPPORTED_OPTION = "unsupported_option" + PROJECT_CREATION_ERROR = "project_creation_error" + INVALID_CONFIGURATION = "invalid_configuration" + UNEXPECTED_ERROR = "unexpected_error" + + +class TelemetryDurationBucket(StrEnum): + """Fixed duration buckets rather than identifying precise timings.""" + + UNDER_ONE_SECOND = "under_1s" + ONE_TO_FIVE_SECONDS = "1_to_5s" + FIVE_TO_THIRTY_SECONDS = "5_to_30s" + THIRTY_SECONDS_OR_MORE = "30s_or_more" + + +class TelemetrySuppressionReason(StrEnum): + """Why telemetry is not effective for the current process.""" + + ENABLED = "enabled" + NOT_OPTED_IN = "not_opted_in" + USER_DISABLED = "user_disabled" + DO_NOT_TRACK = "do_not_track" + ENVIRONMENT_DISABLED = "environment_disabled" + CI = "ci" + TEST = "test" + DEVELOPMENT = "development" + EVALUATION = "evaluation" + INVALID_STATE = "invalid_state" + + +@dataclass(frozen=True) +class ScaffoldCreateProperties: + """Closed property DTO for the initial scaffold completion event.""" + + cli_version: str + project_type: str + language: str + runtime: str + cloud: str + framework: str + database: str + cache: str + auth: str + knowledge: str + workspace_tooling: str + helm: bool + github_requested: bool + interactive: bool + outcome: ScaffoldCreateOutcome + error_category: ScaffoldCreateErrorCategory + duration_bucket: TelemetryDurationBucket + platform: str + architecture: str + + def as_mapping(self) -> dict[str, TelemetryPropertyValue]: + """Return the exact provider-neutral property allowlist.""" + return { + "architecture": self.architecture, + "auth": self.auth, + "cache": self.cache, + "cli_version": self.cli_version, + "cloud": self.cloud, + "database": self.database, + "duration_bucket": self.duration_bucket.value, + "error_category": self.error_category.value, + "framework": self.framework, + "github_requested": self.github_requested, + "helm": self.helm, + "interactive": self.interactive, + "knowledge": self.knowledge, + "language": self.language, + "outcome": self.outcome.value, + "platform": self.platform, + "project_type": self.project_type, + "runtime": self.runtime, + "workspace_tooling": self.workspace_tooling, + } + + +@dataclass(frozen=True) +class TelemetryEvent: + """A typed product event before identity and delivery metadata are added.""" + + name: TelemetryEventName + properties: ScaffoldCreateProperties + + +@dataclass(frozen=True) +class TelemetryEnvelope: + """The vendor-neutral envelope delivered to a telemetry sink.""" + + event_id: UUID + anonymous_id: UUID + occurred_at: datetime + event: TelemetryEvent + + +@dataclass(frozen=True) +class TelemetryState: + """Versioned user consent and pseudonymous identity state.""" + + consent: TelemetryConsent = TelemetryConsent.UNSET + anonymous_id: UUID | None = None + schema_version: int = TELEMETRY_STATE_SCHEMA_VERSION + + +class TelemetryStateDocument(TypedDict, total=False): + """JSON representation persisted in the user configuration directory.""" + + schema_version: int + consent: str + anonymous_id: str + + +@dataclass(frozen=True) +class EffectiveTelemetry: + """The current process-level telemetry decision.""" + + enabled: bool + reason: TelemetrySuppressionReason + anonymous_id: UUID | None + + +@dataclass(frozen=True) +class TelemetryIdentityReset: + """The result of an explicit identity rotation request.""" + + previous_id: UUID | None + current_id: UUID | None diff --git a/src/telemetry/__init__.py b/src/telemetry/__init__.py new file mode 100644 index 0000000..90f8a79 --- /dev/null +++ b/src/telemetry/__init__.py @@ -0,0 +1 @@ +"""Provider-neutral telemetry policy, state, and delivery helpers.""" diff --git a/src/telemetry/policy.py b/src/telemetry/policy.py new file mode 100644 index 0000000..60f33ab --- /dev/null +++ b/src/telemetry/policy.py @@ -0,0 +1,67 @@ +"""Fail-closed telemetry enablement policy.""" + +import os +from collections.abc import Mapping +from pathlib import Path + +from src.model.dto.telemetry import EffectiveTelemetry, TelemetryConsent, TelemetryState, TelemetrySuppressionReason +from src.telemetry.state import TelemetryStateStore +from src.utils.errors import TelemetryStateError + + +_TRUTHY_VALUES = {"1", "true", "yes", "on"} + + +def resolve_telemetry( + store: TelemetryStateStore, + environ: Mapping[str, str] | None = None, + *, + development: bool | None = None, + state: TelemetryState | None = None, +) -> EffectiveTelemetry: + """Resolve hard opt-outs before consulting persisted consent.""" + process_environment = os.environ if environ is None else environ + if _truthy(process_environment.get("DO_NOT_TRACK")): + return _disabled(TelemetrySuppressionReason.DO_NOT_TRACK) + if _truthy(process_environment.get("KICKSTART_TELEMETRY_DISABLED")): + return _disabled(TelemetrySuppressionReason.ENVIRONMENT_DISABLED) + if _truthy(process_environment.get("CI")): + return _disabled(TelemetrySuppressionReason.CI) + if "PYTEST_CURRENT_TEST" in process_environment: + return _disabled(TelemetrySuppressionReason.TEST) + if _truthy(process_environment.get("KICKSTART_EVAL")): + return _disabled(TelemetrySuppressionReason.EVALUATION) + + development_run = _is_source_checkout() if development is None else development + if development_run and not _truthy(process_environment.get("KICKSTART_TELEMETRY_ALLOW_DEVELOPMENT")): + return _disabled(TelemetrySuppressionReason.DEVELOPMENT) + + try: + current_state = store.read() if state is None else state + except TelemetryStateError: + return _disabled(TelemetrySuppressionReason.INVALID_STATE) + if current_state.consent is TelemetryConsent.ENABLED and current_state.anonymous_id is not None: + return EffectiveTelemetry( + enabled=True, + reason=TelemetrySuppressionReason.ENABLED, + anonymous_id=current_state.anonymous_id, + ) + reason = ( + TelemetrySuppressionReason.USER_DISABLED + if current_state.consent is TelemetryConsent.DISABLED + else TelemetrySuppressionReason.NOT_OPTED_IN + ) + return EffectiveTelemetry(enabled=False, reason=reason, anonymous_id=current_state.anonymous_id) + + +def _disabled(reason: TelemetrySuppressionReason) -> EffectiveTelemetry: + return EffectiveTelemetry(enabled=False, reason=reason, anonymous_id=None) + + +def _truthy(value: str | None) -> bool: + return value is not None and value.strip().lower() in _TRUTHY_VALUES + + +def _is_source_checkout() -> bool: + repository_root = Path(__file__).resolve().parents[2] + return (repository_root / ".git").exists() and (repository_root / "pyproject.toml").exists() diff --git a/src/telemetry/sink.py b/src/telemetry/sink.py new file mode 100644 index 0000000..93700bb --- /dev/null +++ b/src/telemetry/sink.py @@ -0,0 +1,31 @@ +"""Provider-neutral telemetry sink interfaces.""" + +from dataclasses import dataclass, field +from typing import Protocol + +from src.model.dto.telemetry import TelemetryEnvelope + + +class TelemetrySink(Protocol): + """A replaceable destination for a fully formed telemetry envelope.""" + + def record(self, envelope: TelemetryEnvelope) -> None: + """Record an envelope or raise a transport-specific exception.""" + + +class NoOpTelemetrySink: + """Discard telemetry without side effects.""" + + def record(self, envelope: TelemetryEnvelope) -> None: + """Intentionally discard the envelope.""" + + +@dataclass +class InMemoryTelemetrySink: + """Record exact envelopes for tests without network access.""" + + envelopes: list[TelemetryEnvelope] = field(default_factory=list) + + def record(self, envelope: TelemetryEnvelope) -> None: + """Append one envelope to the in-memory record.""" + self.envelopes.append(envelope) diff --git a/src/telemetry/state.py b/src/telemetry/state.py new file mode 100644 index 0000000..e20040d --- /dev/null +++ b/src/telemetry/state.py @@ -0,0 +1,193 @@ +"""Durable user-scoped telemetry consent and identity state.""" + +import json +import os +import tempfile +import time +from collections.abc import Callable, Iterator, Mapping +from contextlib import contextmanager +from pathlib import Path +from typing import TypeAlias, cast +from uuid import UUID, uuid4 + +from src.model.dto.telemetry import ( + TELEMETRY_STATE_SCHEMA_VERSION, + TelemetryConsent, + TelemetryIdentityReset, + TelemetryState, + TelemetryStateDocument, +) +from src.utils.errors import TelemetryStateError + + +JsonScalar: TypeAlias = str | int | float | bool | None +JsonValue: TypeAlias = JsonScalar | list["JsonValue"] | dict[str, "JsonValue"] +_LOCK_TIMEOUT_SECONDS = 1.0 +_STALE_LOCK_SECONDS = 30.0 + + +def default_telemetry_state_path( + environ: Mapping[str, str] | None = None, + *, + home: Path | None = None, +) -> Path: + """Return a user-scoped path that survives application upgrades.""" + process_environment = os.environ if environ is None else environ + xdg_value = process_environment.get("XDG_CONFIG_HOME") + if xdg_value: + xdg_path = Path(xdg_value).expanduser() + if xdg_path.is_absolute(): + return xdg_path / "kickstart" / "telemetry.json" + home_path = Path.home() if home is None else home + return home_path.expanduser() / ".config" / "kickstart" / "telemetry.json" + + +class TelemetryStateStore: + """Read and atomically update versioned telemetry state.""" + + def __init__(self, path: Path, *, uuid_factory: Callable[[], UUID] = uuid4) -> None: + self.path = path + self._uuid_factory = uuid_factory + + @classmethod + def from_environment(cls, environ: Mapping[str, str] | None = None) -> "TelemetryStateStore": + """Construct the default store without consulting project configuration.""" + return cls(default_telemetry_state_path(environ)) + + def read(self) -> TelemetryState: + """Read state without creating a file; missing state means not opted in.""" + if not self.path.exists(): + return TelemetryState() + try: + payload = cast(JsonValue, json.loads(self.path.read_text(encoding="utf-8"))) + return _parse_state(payload) + except (OSError, TypeError, ValueError) as exc: + raise TelemetryStateError("Telemetry state is unreadable or malformed; telemetry is disabled.") from exc + + def enable(self) -> TelemetryState: + """Persist opt-in and lazily create a stable UUIDv4 identity.""" + with self._locked(): + current = self.read() + anonymous_id = current.anonymous_id or self._uuid_factory() + state = TelemetryState(consent=TelemetryConsent.ENABLED, anonymous_id=anonymous_id) + self._write(state) + return state + + def disable(self) -> TelemetryState: + """Persist opt-out while retaining any existing identity.""" + with self._locked(): + current = self.read() + state = TelemetryState(consent=TelemetryConsent.DISABLED, anonymous_id=current.anonymous_id) + self._write(state) + return state + + def reset_id(self) -> TelemetryIdentityReset: + """Rotate an existing identity without changing consent.""" + with self._locked(): + current = self.read() + if current.anonymous_id is None: + return TelemetryIdentityReset(previous_id=None, current_id=None) + new_id = self._uuid_factory() + self._write(TelemetryState(consent=current.consent, anonymous_id=new_id)) + return TelemetryIdentityReset(previous_id=current.anonymous_id, current_id=new_id) + + @contextmanager + def _locked(self) -> Iterator[None]: + """Serialize updates with a small, cross-platform directory lock.""" + self._ensure_parent() + lock_path = self.path.with_name(f".{self.path.name}.lock") + deadline = time.monotonic() + _LOCK_TIMEOUT_SECONDS + while True: + try: + lock_path.mkdir(mode=0o700) + break + except FileExistsError as exc: + if _remove_stale_lock(lock_path): + continue + if time.monotonic() >= deadline: + raise TelemetryStateError("Telemetry state is busy; no preference was changed.") from exc + time.sleep(0.01) + except OSError as exc: + raise TelemetryStateError("Telemetry state cannot be locked; no preference was changed.") from exc + try: + yield + finally: + try: + lock_path.rmdir() + except OSError: + pass + + def _ensure_parent(self) -> None: + try: + self.path.parent.mkdir(mode=0o700, parents=True, exist_ok=True) + self.path.parent.chmod(0o700) + except OSError as exc: + raise TelemetryStateError("Telemetry state directory cannot be created; telemetry is disabled.") from exc + + def _write(self, state: TelemetryState) -> None: + document: TelemetryStateDocument = { + "schema_version": state.schema_version, + "consent": state.consent.value, + } + if state.anonymous_id is not None: + document["anonymous_id"] = str(state.anonymous_id) + + temporary_path: Path | None = None + try: + descriptor, temporary_name = tempfile.mkstemp(prefix=".telemetry-", suffix=".tmp", dir=self.path.parent) + temporary_path = Path(temporary_name) + with os.fdopen(descriptor, "w", encoding="utf-8") as handle: + os.fchmod(handle.fileno(), 0o600) + handle.write(json.dumps(document, indent=2, sort_keys=True)) + handle.write("\n") + handle.flush() + os.fsync(handle.fileno()) + os.replace(temporary_path, self.path) + self.path.chmod(0o600) + except OSError as exc: + raise TelemetryStateError("Telemetry state cannot be written; telemetry is disabled.") from exc + finally: + if temporary_path is not None and temporary_path.exists(): + try: + temporary_path.unlink() + except OSError: + pass + + +def _parse_state(payload: JsonValue) -> TelemetryState: + if not isinstance(payload, dict): + raise ValueError("state must be an object") + allowed_keys = {"schema_version", "consent", "anonymous_id"} + if set(payload) - allowed_keys: + raise ValueError("state contains unknown fields") + if payload.get("schema_version") != TELEMETRY_STATE_SCHEMA_VERSION: + raise ValueError("unsupported state schema") + + consent_value = payload.get("consent") + if not isinstance(consent_value, str): + raise ValueError("consent must be a string") + consent = TelemetryConsent(consent_value) + if consent is TelemetryConsent.UNSET: + raise ValueError("unset consent is not persisted") + + identity_value = payload.get("anonymous_id") + anonymous_id: UUID | None = None + if identity_value is not None: + if not isinstance(identity_value, str): + raise ValueError("anonymous_id must be a string") + anonymous_id = UUID(identity_value) + if anonymous_id.version != 4: + raise ValueError("anonymous_id must be UUIDv4") + if consent is TelemetryConsent.ENABLED and anonymous_id is None: + raise ValueError("enabled telemetry requires an identity") + return TelemetryState(consent=consent, anonymous_id=anonymous_id) + + +def _remove_stale_lock(lock_path: Path) -> bool: + try: + if time.time() - lock_path.stat().st_mtime <= _STALE_LOCK_SECONDS: + return False + lock_path.rmdir() + return True + except OSError: + return False diff --git a/src/utils/errors.py b/src/utils/errors.py index b7b4f0f..8fd71e8 100644 --- a/src/utils/errors.py +++ b/src/utils/errors.py @@ -82,3 +82,8 @@ class MissingCreateArgumentsError(KickstartError): class ExtensionError(KickstartError): """Raised when extension setup fails.""" pass + + +class TelemetryStateError(KickstartError): + """Raised when explicit telemetry state management cannot complete safely.""" + pass diff --git a/tests/unit/cli/test_telemetry_cli.py b/tests/unit/cli/test_telemetry_cli.py new file mode 100644 index 0000000..9a29000 --- /dev/null +++ b/tests/unit/cli/test_telemetry_cli.py @@ -0,0 +1,75 @@ +import json +from pathlib import Path + +from typer.testing import CliRunner + +from src.cli.main import app + + +def _env(tmp_path: Path) -> dict[str, str]: + return {"XDG_CONFIG_HOME": str(tmp_path), "HOME": str(tmp_path)} + + +def test_status_is_read_only_before_opt_in(tmp_path: Path) -> None: + runner = CliRunner() + + result = runner.invoke(app, ["telemetry", "status", "--json"], env=_env(tmp_path)) + + assert result.exit_code == 0 + payload = json.loads(result.stdout) + assert payload["consent"] == "unset" + assert payload["effective"] is False + assert payload["anonymous_id"] is None + assert not (tmp_path / "kickstart" / "telemetry.json").exists() + + +def test_enable_disable_and_status_preserve_identity(tmp_path: Path) -> None: + runner = CliRunner() + environment = _env(tmp_path) + + enabled = runner.invoke(app, ["telemetry", "enable"], env=environment) + status = runner.invoke(app, ["telemetry", "status", "--json"], env=environment) + disabled = runner.invoke(app, ["telemetry", "disable"], env=environment) + + assert enabled.exit_code == 0 + assert status.exit_code == 0 + assert disabled.exit_code == 0 + identity = json.loads(status.stdout)["anonymous_id"] + assert identity is not None + assert identity in enabled.stdout + assert identity in disabled.stdout + + +def test_reset_reports_previous_identity_without_deleting_history(tmp_path: Path) -> None: + runner = CliRunner() + environment = _env(tmp_path) + enabled = runner.invoke(app, ["telemetry", "enable"], env=environment) + original_id = enabled.stdout.split("anonymous-id: ", maxsplit=1)[1].strip() + + reset = runner.invoke(app, ["telemetry", "reset-id"], env=environment) + + assert reset.exit_code == 0 + assert f"previous-anonymous-id: {original_id}" in reset.stdout + assert "Historical events are not deleted" in reset.stdout + + +def test_disable_before_enable_does_not_create_an_identity(tmp_path: Path) -> None: + result = CliRunner().invoke(app, ["telemetry", "disable"], env=_env(tmp_path)) + + assert result.exit_code == 0 + assert "anonymous-id: not-created" in result.stdout + + +def test_malformed_state_reports_fail_closed_json_status(tmp_path: Path) -> None: + state_path = tmp_path / "kickstart" / "telemetry.json" + state_path.parent.mkdir() + state_path.write_text("{bad json", encoding="utf-8") + + result = CliRunner().invoke(app, ["telemetry", "status", "--json"], env=_env(tmp_path)) + + assert result.exit_code == 0 + payload = json.loads(result.stdout) + assert payload["consent"] == "invalid" + assert payload["effective"] is False + assert payload["reason"] == "invalid_state" + assert payload["anonymous_id"] is None diff --git a/tests/unit/model/dto/test_telemetry_dto.py b/tests/unit/model/dto/test_telemetry_dto.py new file mode 100644 index 0000000..dacdf8c --- /dev/null +++ b/tests/unit/model/dto/test_telemetry_dto.py @@ -0,0 +1,84 @@ +from dataclasses import FrozenInstanceError +from datetime import UTC, datetime +from uuid import uuid4 + +import pytest + +from src.model.dto.telemetry import ( + TelemetryEnvelope, + ScaffoldCreateErrorCategory, + ScaffoldCreateOutcome, + ScaffoldCreateProperties, + TelemetryDurationBucket, + TelemetryEvent, + TelemetryEventName, +) + + +def _properties() -> ScaffoldCreateProperties: + return ScaffoldCreateProperties( + cli_version="0.4.3", + project_type="service", + language="python", + runtime="container", + cloud="none", + framework="fastapi", + database="none", + cache="none", + auth="none", + knowledge="none", + workspace_tooling="none", + helm=False, + github_requested=False, + interactive=False, + outcome=ScaffoldCreateOutcome.SUCCESS, + error_category=ScaffoldCreateErrorCategory.NONE, + duration_bucket=TelemetryDurationBucket.UNDER_ONE_SECOND, + platform="linux", + architecture="x86_64", + ) + + +def test_event_properties_use_a_frozen_closed_dto() -> None: + properties = _properties() + event = TelemetryEvent(TelemetryEventName.SCAFFOLD_CREATE_COMPLETED, properties) + + assert event.properties.project_type == "service" + with pytest.raises(FrozenInstanceError): + setattr(event.properties, "project_type", "frontend") + + +def test_property_mapping_contains_only_the_documented_allowlist() -> None: + assert set(_properties().as_mapping()) == { + "architecture", + "auth", + "cache", + "cli_version", + "cloud", + "database", + "duration_bucket", + "error_category", + "framework", + "github_requested", + "helm", + "interactive", + "knowledge", + "language", + "outcome", + "platform", + "project_type", + "runtime", + "workspace_tooling", + } + + +def test_envelope_uses_provider_neutral_identity_names() -> None: + envelope = TelemetryEnvelope( + event_id=uuid4(), + anonymous_id=uuid4(), + occurred_at=datetime.now(UTC), + event=TelemetryEvent(TelemetryEventName.SCAFFOLD_CREATE_COMPLETED, _properties()), + ) + + assert "posthog" not in repr(envelope).lower() + assert envelope.anonymous_id.version == 4 diff --git a/tests/unit/telemetry/test_policy.py b/tests/unit/telemetry/test_policy.py new file mode 100644 index 0000000..7a1f380 --- /dev/null +++ b/tests/unit/telemetry/test_policy.py @@ -0,0 +1,79 @@ +from pathlib import Path + +import pytest + +from src.model.dto.telemetry import TelemetrySuppressionReason +from src.telemetry.policy import resolve_telemetry +from src.telemetry.state import TelemetryStateStore + + +@pytest.mark.parametrize( + ("environ", "reason"), + [ + ({"DO_NOT_TRACK": "1"}, TelemetrySuppressionReason.DO_NOT_TRACK), + ({"KICKSTART_TELEMETRY_DISABLED": "true"}, TelemetrySuppressionReason.ENVIRONMENT_DISABLED), + ({"CI": "yes"}, TelemetrySuppressionReason.CI), + ({"PYTEST_CURRENT_TEST": "test"}, TelemetrySuppressionReason.TEST), + ({"KICKSTART_EVAL": "on"}, TelemetrySuppressionReason.EVALUATION), + ], +) +def test_hard_opt_outs_precede_persisted_consent( + tmp_path: Path, + environ: dict[str, str], + reason: TelemetrySuppressionReason, +) -> None: + store = TelemetryStateStore(tmp_path / "telemetry.json") + store.enable() + + resolved = resolve_telemetry(store, environ, development=False) + + assert resolved.enabled is False + assert resolved.reason is reason + assert resolved.anonymous_id is None + + +def test_missing_state_is_disabled_without_creating_an_identity(tmp_path: Path) -> None: + store = TelemetryStateStore(tmp_path / "telemetry.json") + + resolved = resolve_telemetry(store, {}, development=False) + + assert resolved.enabled is False + assert resolved.reason is TelemetrySuppressionReason.NOT_OPTED_IN + assert resolved.anonymous_id is None + assert not store.path.exists() + + +def test_enabled_state_is_effective_outside_suppressed_contexts(tmp_path: Path) -> None: + store = TelemetryStateStore(tmp_path / "telemetry.json") + state = store.enable() + + resolved = resolve_telemetry(store, {}, development=False) + + assert resolved.enabled is True + assert resolved.reason is TelemetrySuppressionReason.ENABLED + assert resolved.anonymous_id == state.anonymous_id + + +def test_source_checkout_is_disabled_without_explicit_development_override(tmp_path: Path) -> None: + store = TelemetryStateStore(tmp_path / "telemetry.json") + store.enable() + + disabled = resolve_telemetry(store, {}, development=True) + enabled = resolve_telemetry( + store, + {"KICKSTART_TELEMETRY_ALLOW_DEVELOPMENT": "1"}, + development=True, + ) + + assert disabled.reason is TelemetrySuppressionReason.DEVELOPMENT + assert enabled.enabled is True + + +def test_malformed_state_fails_closed(tmp_path: Path) -> None: + path = tmp_path / "telemetry.json" + path.write_text("{bad json", encoding="utf-8") + + resolved = resolve_telemetry(TelemetryStateStore(path), {}, development=False) + + assert resolved.enabled is False + assert resolved.reason is TelemetrySuppressionReason.INVALID_STATE diff --git a/tests/unit/telemetry/test_sink.py b/tests/unit/telemetry/test_sink.py new file mode 100644 index 0000000..8034aba --- /dev/null +++ b/tests/unit/telemetry/test_sink.py @@ -0,0 +1,58 @@ +from datetime import UTC, datetime +from uuid import uuid4 + +from src.model.dto.telemetry import ( + ScaffoldCreateErrorCategory, + ScaffoldCreateOutcome, + ScaffoldCreateProperties, + TelemetryDurationBucket, + TelemetryEnvelope, + TelemetryEvent, + TelemetryEventName, +) +from src.telemetry.sink import InMemoryTelemetrySink, NoOpTelemetrySink + + +def _envelope() -> TelemetryEnvelope: + return TelemetryEnvelope( + event_id=uuid4(), + anonymous_id=uuid4(), + occurred_at=datetime.now(UTC), + event=TelemetryEvent( + TelemetryEventName.SCAFFOLD_CREATE_COMPLETED, + ScaffoldCreateProperties( + cli_version="0.4.3", + project_type="service", + language="python", + runtime="container", + cloud="none", + framework="fastapi", + database="none", + cache="none", + auth="none", + knowledge="none", + workspace_tooling="none", + helm=False, + github_requested=False, + interactive=False, + outcome=ScaffoldCreateOutcome.SUCCESS, + error_category=ScaffoldCreateErrorCategory.NONE, + duration_bucket=TelemetryDurationBucket.UNDER_ONE_SECOND, + platform="linux", + architecture="x86_64", + ), + ), + ) + + +def test_noop_sink_discards_without_error() -> None: + NoOpTelemetrySink().record(_envelope()) + + +def test_in_memory_sink_records_exact_envelope() -> None: + sink = InMemoryTelemetrySink() + envelope = _envelope() + + sink.record(envelope) + + assert sink.envelopes == [envelope] diff --git a/tests/unit/telemetry/test_state.py b/tests/unit/telemetry/test_state.py new file mode 100644 index 0000000..c4fc8e8 --- /dev/null +++ b/tests/unit/telemetry/test_state.py @@ -0,0 +1,131 @@ +import json +import stat +from concurrent.futures import ThreadPoolExecutor +from pathlib import Path +from uuid import UUID, uuid4 + +import pytest + +from src.model.dto.telemetry import TelemetryConsent +from src.telemetry.state import TelemetryStateStore, default_telemetry_state_path +from src.utils.errors import TelemetryStateError + + +def test_default_path_honors_absolute_xdg_config_home(tmp_path: Path) -> None: + assert default_telemetry_state_path({"XDG_CONFIG_HOME": str(tmp_path)}) == ( + tmp_path / "kickstart" / "telemetry.json" + ) + + +def test_default_path_ignores_relative_xdg_config_home(tmp_path: Path) -> None: + assert default_telemetry_state_path({"XDG_CONFIG_HOME": "relative"}, home=tmp_path) == ( + tmp_path / ".config" / "kickstart" / "telemetry.json" + ) + + +def test_reading_missing_state_is_lazy(tmp_path: Path) -> None: + store = TelemetryStateStore(tmp_path / "telemetry.json") + + state = store.read() + + assert state.consent is TelemetryConsent.UNSET + assert state.anonymous_id is None + assert not store.path.exists() + + +def test_enable_creates_uuid4_and_reuses_it_across_store_instances(tmp_path: Path) -> None: + path = tmp_path / "telemetry.json" + + first = TelemetryStateStore(path).enable() + second = TelemetryStateStore(path).enable() + + assert first.anonymous_id is not None + assert first.anonymous_id.version == 4 + assert second.anonymous_id == first.anonymous_id + assert second.consent is TelemetryConsent.ENABLED + + +def test_disable_before_enable_does_not_create_identity(tmp_path: Path) -> None: + store = TelemetryStateStore(tmp_path / "telemetry.json") + + state = store.disable() + + assert state.consent is TelemetryConsent.DISABLED + assert state.anonymous_id is None + assert "anonymous_id" not in json.loads(store.path.read_text(encoding="utf-8")) + + +def test_disable_and_reenable_preserve_identity(tmp_path: Path) -> None: + store = TelemetryStateStore(tmp_path / "telemetry.json") + enabled = store.enable() + + disabled = store.disable() + reenabled = store.enable() + + assert disabled.anonymous_id == enabled.anonymous_id + assert reenabled.anonymous_id == enabled.anonymous_id + + +def test_reset_rotates_existing_identity_and_preserves_consent(tmp_path: Path) -> None: + store = TelemetryStateStore(tmp_path / "telemetry.json") + original = store.enable() + + reset = store.reset_id() + persisted = store.read() + + assert reset.previous_id == original.anonymous_id + assert reset.current_id is not None + assert reset.current_id != original.anonymous_id + assert persisted.anonymous_id == reset.current_id + assert persisted.consent is TelemetryConsent.ENABLED + + +def test_reset_without_identity_is_read_only(tmp_path: Path) -> None: + store = TelemetryStateStore(tmp_path / "telemetry.json") + + reset = store.reset_id() + + assert reset.previous_id is None + assert reset.current_id is None + assert not store.path.exists() + + +def test_concurrent_enablement_converges_on_one_identity(tmp_path: Path) -> None: + path = tmp_path / "telemetry.json" + candidates = iter([uuid4(), uuid4()]) + + def enable() -> UUID | None: + return TelemetryStateStore(path, uuid_factory=lambda: next(candidates)).enable().anonymous_id + + with ThreadPoolExecutor(max_workers=2) as executor: + identities = list(executor.map(lambda _: enable(), range(2))) + + assert identities[0] == identities[1] + + +@pytest.mark.skipif(not hasattr(stat, "S_IRUSR"), reason="permission bits unavailable") +def test_state_file_uses_user_only_permissions(tmp_path: Path) -> None: + path = tmp_path / "kickstart" / "telemetry.json" + + TelemetryStateStore(path).enable() + + assert stat.S_IMODE(path.stat().st_mode) == 0o600 + assert stat.S_IMODE(path.parent.stat().st_mode) == 0o700 + + +@pytest.mark.parametrize( + "document", + [ + {}, + {"schema_version": 2, "consent": "enabled", "anonymous_id": str(uuid4())}, + {"schema_version": 1, "consent": "enabled"}, + {"schema_version": 1, "consent": "enabled", "anonymous_id": "not-a-uuid"}, + {"schema_version": 1, "consent": "enabled", "anonymous_id": str(uuid4()), "extra": True}, + ], +) +def test_malformed_or_unknown_state_is_rejected(tmp_path: Path, document: dict[str, str | int | bool]) -> None: + path = tmp_path / "telemetry.json" + path.write_text(json.dumps(document), encoding="utf-8") + + with pytest.raises(TelemetryStateError): + TelemetryStateStore(path).read() From 6ed245f834566ee188dc01c57a9d76cb7c6c280b Mon Sep 17 00:00:00 2001 From: Jean-Michel Bouchard Date: Sun, 19 Jul 2026 15:23:46 -0400 Subject: [PATCH 02/11] Add direct PostHog telemetry sink Map provider-neutral telemetry envelopes through typed PostHog DTOs, read the public project token only from the process environment, and contain all delivery failures. Checks: make check (676 tests) Refs ENG-202 --- docs/contracts/telemetry.md | 4 +- src/model/dto/__init__.py | 3 + src/model/dto/posthog.py | 47 +++++++ src/telemetry/config.py | 34 ++++++ src/telemetry/posthog.py | 39 ++++++ src/telemetry/reporter.py | 68 +++++++++++ tests/unit/model/dto/test_posthog_dto.py | 115 ++++++++++++++++++ tests/unit/telemetry/test_posthog.py | 93 ++++++++++++++ tests/unit/telemetry/test_reporter.py | 93 ++++++++++++++ tests/unit/telemetry/test_telemetry_config.py | 43 +++++++ 10 files changed, 537 insertions(+), 2 deletions(-) create mode 100644 src/model/dto/posthog.py create mode 100644 src/telemetry/config.py create mode 100644 src/telemetry/posthog.py create mode 100644 src/telemetry/reporter.py create mode 100644 tests/unit/model/dto/test_posthog_dto.py create mode 100644 tests/unit/telemetry/test_posthog.py create mode 100644 tests/unit/telemetry/test_reporter.py create mode 100644 tests/unit/telemetry/test_telemetry_config.py diff --git a/docs/contracts/telemetry.md b/docs/contracts/telemetry.md index bf4ba15..de6b054 100644 --- a/docs/contracts/telemetry.md +++ b/docs/contracts/telemetry.md @@ -102,11 +102,11 @@ CLI event producers depend only on provider-neutral DTOs and a `TelemetrySink`. The initial destination is the dedicated `kickstart-cli` project on PostHog Cloud US. Direct capture uses the public `phc_` project token only. A `phx_` personal API key or any read-capable credential must never enter the application, repository, build, logs, tests, or issues. -Every PostHog event sets `$process_person_profile` to `false` and `$geoip_disable` to `true`. The adapter never identifies, aliases, groups, or sets person properties. The PostHog project must discard captured IP data. Because phase one connects directly, PostHog still receives the HTTPS connection before applying its project-level discard policy. +Every PostHog event sets `$process_person_profile` to `false`. The adapter never identifies, aliases, groups, or sets person properties. GeoIP enrichment is not disabled: because phase one connects directly, PostHog receives the HTTPS connection and may process its source IP and add provider-derived geographic properties. These connection and provider-derived fields are outside the CLI-authored property allowlist and are accepted for the initial deployment. Direct public capture can be spoofed. Provider, destination, or token changes require upgraded direct clients, and there is no first-party server kill switch in phase one. A later first-party relay may replace the sink without changing event producers, consent, or identity. -For local development, credentials may be exported from an ignored `.env` into the process. The CLI never auto-loads a current repository's `.env`, because kickstart runs inside arbitrary repositories and repository content must not influence telemetry routing or consent. The numeric PostHog project ID is not part of capture payloads. +For local development, `POSTHOG_PUBLIC_CUSTOMER_API_TOKEN` may be exported from an ignored `.env` into the process. The CLI reads only the already-exported process environment and never auto-loads a current repository's `.env`, because kickstart runs inside arbitrary repositories and repository content must not influence telemetry routing or consent. A missing or malformed token silently selects a no-op sink; token presence never enables telemetry. The numeric `POSTHOG_PROJECT_ID` is not read and is not part of capture payloads. ## Delivery and retention diff --git a/src/model/dto/__init__.py b/src/model/dto/__init__.py index 9e6c704..2f7f528 100644 --- a/src/model/dto/__init__.py +++ b/src/model/dto/__init__.py @@ -1,5 +1,6 @@ """Data-transfer objects shared across kickstart boundaries.""" +from src.model.dto.posthog import PostHogCapturePayload, PostHogCaptureRequest from src.model.dto.telemetry import ( EffectiveTelemetry, ScaffoldCreateErrorCategory, @@ -17,6 +18,8 @@ __all__ = [ "EffectiveTelemetry", + "PostHogCapturePayload", + "PostHogCaptureRequest", "ScaffoldCreateErrorCategory", "ScaffoldCreateOutcome", "ScaffoldCreateProperties", diff --git a/src/model/dto/posthog.py b/src/model/dto/posthog.py new file mode 100644 index 0000000..85f62ac --- /dev/null +++ b/src/model/dto/posthog.py @@ -0,0 +1,47 @@ +"""PostHog-specific wire DTOs for the replaceable telemetry adapter.""" + +from dataclasses import dataclass, field +from datetime import UTC +from typing import TypedDict + +from src.model.dto.telemetry import TelemetryEnvelope, TelemetryPropertyValue + + +class PostHogCapturePayload(TypedDict): + """The exact JSON object accepted by PostHog's single-event endpoint.""" + + api_key: str + event: str + distinct_id: str + timestamp: str + uuid: str + properties: dict[str, TelemetryPropertyValue] + + +@dataclass(frozen=True) +class PostHogCaptureRequest: + """Map one provider-neutral envelope to PostHog without expanding its data.""" + + project_token: str = field(repr=False) + envelope: TelemetryEnvelope + + def __post_init__(self) -> None: + if not self.project_token.startswith("phc_") or len(self.project_token) == len("phc_"): + raise ValueError("PostHog capture requires a public phc_ project token") + + def as_payload(self) -> PostHogCapturePayload: + """Return a new exact payload containing only approved and reserved fields.""" + occurred_at = self.envelope.occurred_at + if occurred_at.tzinfo is None or occurred_at.utcoffset() is None: + raise ValueError("Telemetry timestamps must be timezone-aware") + properties = self.envelope.event.properties.as_mapping() + properties["$process_person_profile"] = False + timestamp = occurred_at.astimezone(UTC).isoformat(timespec="milliseconds").replace("+00:00", "Z") + return { + "api_key": self.project_token, + "distinct_id": str(self.envelope.anonymous_id), + "event": self.envelope.event.name.value, + "timestamp": timestamp, + "uuid": str(self.envelope.event_id), + "properties": properties, + } diff --git a/src/telemetry/config.py b/src/telemetry/config.py new file mode 100644 index 0000000..adf8403 --- /dev/null +++ b/src/telemetry/config.py @@ -0,0 +1,34 @@ +"""Runtime-only configuration for telemetry delivery.""" + +import os +from collections.abc import Mapping +from dataclasses import dataclass, field + + +POSTHOG_PROJECT_TOKEN_ENV = "POSTHOG_PUBLIC_CUSTOMER_API_TOKEN" +POSTHOG_US_CAPTURE_ENDPOINT = "https://us.i.posthog.com/i/v0/e/" +POSTHOG_TIMEOUT_SECONDS = 2.0 + + +@dataclass(frozen=True) +class PostHogSettings: + """Validated PostHog delivery settings; the public token stays out of reprs.""" + + project_token: str = field(repr=False) + endpoint: str = POSTHOG_US_CAPTURE_ENDPOINT + timeout_seconds: float = POSTHOG_TIMEOUT_SECONDS + + def __post_init__(self) -> None: + if not self.project_token.startswith("phc_") or len(self.project_token) == len("phc_"): + raise ValueError("PostHog capture requires a public phc_ project token") + if self.timeout_seconds <= 0: + raise ValueError("PostHog timeout must be positive") + + +def posthog_settings_from_environment(environ: Mapping[str, str] | None = None) -> PostHogSettings | None: + """Read a public capture token from the process environment, never a project file.""" + process_environment = os.environ if environ is None else environ + project_token = process_environment.get(POSTHOG_PROJECT_TOKEN_ENV, "").strip() + if not project_token.startswith("phc_") or len(project_token) == len("phc_"): + return None + return PostHogSettings(project_token=project_token) diff --git a/src/telemetry/posthog.py b/src/telemetry/posthog.py new file mode 100644 index 0000000..57eda57 --- /dev/null +++ b/src/telemetry/posthog.py @@ -0,0 +1,39 @@ +"""PostHog transport adapter for provider-neutral telemetry envelopes.""" + +from dataclasses import dataclass, field +from typing import Protocol + +import requests + +from src.model.dto.posthog import PostHogCapturePayload, PostHogCaptureRequest +from src.model.dto.telemetry import TelemetryEnvelope +from src.telemetry.config import PostHogSettings + + +class PostHogTransport(Protocol): + """Minimal injectable HTTP boundary used by the PostHog sink.""" + + def send(self, endpoint: str, payload: PostHogCapturePayload, timeout_seconds: float) -> None: + """Send one payload or raise a transport-specific exception.""" + + +class RequestsPostHogTransport: + """Issue exactly one blocking HTTP request with no transport retries.""" + + def send(self, endpoint: str, payload: PostHogCapturePayload, timeout_seconds: float) -> None: + response = requests.post(endpoint, json=payload, timeout=timeout_seconds, allow_redirects=False) + if 300 <= response.status_code < 400: + raise requests.HTTPError("PostHog capture endpoint redirected", response=response) + response.raise_for_status() + + +@dataclass(frozen=True) +class PostHogSink: + """Map and deliver envelopes through the replaceable sink interface.""" + + settings: PostHogSettings + transport: PostHogTransport = field(default_factory=RequestsPostHogTransport) + + def record(self, envelope: TelemetryEnvelope) -> None: + request = PostHogCaptureRequest(project_token=self.settings.project_token, envelope=envelope) + self.transport.send(self.settings.endpoint, request.as_payload(), self.settings.timeout_seconds) diff --git a/src/telemetry/reporter.py b/src/telemetry/reporter.py new file mode 100644 index 0000000..14a65af --- /dev/null +++ b/src/telemetry/reporter.py @@ -0,0 +1,68 @@ +"""Best-effort telemetry orchestration independent of any event producer.""" + +import os +from collections.abc import Callable, Mapping +from dataclasses import dataclass +from datetime import UTC, datetime +from uuid import UUID, uuid4 + +from src.model.dto.telemetry import TelemetryEnvelope, TelemetryEvent +from src.telemetry.config import posthog_settings_from_environment +from src.telemetry.policy import resolve_telemetry +from src.telemetry.posthog import PostHogSink +from src.telemetry.sink import NoOpTelemetrySink, TelemetrySink +from src.telemetry.state import TelemetryStateStore + + +def _utc_now() -> datetime: + return datetime.now(UTC) + + +@dataclass(frozen=True) +class TelemetryReporter: + """Attach consented identity metadata and contain all telemetry failures.""" + + state_store: TelemetryStateStore + sink: TelemetrySink + environ: Mapping[str, str] + development: bool | None = None + clock: Callable[[], datetime] = _utc_now + uuid_factory: Callable[[], UUID] = uuid4 + + def record(self, event: TelemetryEvent) -> None: + """Attempt one event only when effective policy permits it.""" + try: + effective = resolve_telemetry( + self.state_store, + self.environ, + development=self.development, + ) + if not effective.enabled or effective.anonymous_id is None: + return + envelope = TelemetryEnvelope( + event_id=self.uuid_factory(), + anonymous_id=effective.anonymous_id, + occurred_at=self.clock(), + event=event, + ) + self.sink.record(envelope) + except Exception: + # Telemetry must never change another command's output or exit status. + return + + +def default_telemetry_reporter( + environ: Mapping[str, str] | None = None, + *, + development: bool | None = None, +) -> TelemetryReporter: + """Build the default reporter without reading cwd configuration or dotenv files.""" + process_environment = os.environ if environ is None else environ + settings = posthog_settings_from_environment(process_environment) + sink: TelemetrySink = NoOpTelemetrySink() if settings is None else PostHogSink(settings) + return TelemetryReporter( + state_store=TelemetryStateStore.from_environment(process_environment), + sink=sink, + environ=process_environment, + development=development, + ) diff --git a/tests/unit/model/dto/test_posthog_dto.py b/tests/unit/model/dto/test_posthog_dto.py new file mode 100644 index 0000000..b43c88a --- /dev/null +++ b/tests/unit/model/dto/test_posthog_dto.py @@ -0,0 +1,115 @@ +from datetime import UTC, datetime +from uuid import UUID + +import pytest + +from src.model.dto.posthog import PostHogCaptureRequest +from src.model.dto.telemetry import ( + ScaffoldCreateErrorCategory, + ScaffoldCreateOutcome, + ScaffoldCreateProperties, + TelemetryDurationBucket, + TelemetryEnvelope, + TelemetryEvent, + TelemetryEventName, +) + + +EVENT_ID = UUID("11111111-1111-4111-8111-111111111111") +ANONYMOUS_ID = UUID("22222222-2222-4222-8222-222222222222") +OCCURRED_AT = datetime(2026, 7, 19, 14, 30, 15, 123456, tzinfo=UTC) + + +def telemetry_envelope() -> TelemetryEnvelope: + return TelemetryEnvelope( + event_id=EVENT_ID, + anonymous_id=ANONYMOUS_ID, + occurred_at=OCCURRED_AT, + event=TelemetryEvent( + name=TelemetryEventName.SCAFFOLD_CREATE_COMPLETED, + properties=ScaffoldCreateProperties( + cli_version="0.4.3", + project_type="service", + language="python", + runtime="container", + cloud="none", + framework="fastapi", + database="postgres", + cache="redis", + auth="none", + knowledge="none", + workspace_tooling="none", + helm=False, + github_requested=True, + interactive=False, + outcome=ScaffoldCreateOutcome.SUCCESS, + error_category=ScaffoldCreateErrorCategory.NONE, + duration_bucket=TelemetryDurationBucket.ONE_TO_FIVE_SECONDS, + platform="linux", + architecture="x86_64", + ), + ), + ) + + +def test_request_maps_only_allowlisted_and_required_posthog_fields() -> None: + request = PostHogCaptureRequest(project_token="phc_public_test_token", envelope=telemetry_envelope()) + + assert request.as_payload() == { + "api_key": "phc_public_test_token", + "distinct_id": str(ANONYMOUS_ID), + "event": "scaffold_create_completed", + "timestamp": "2026-07-19T14:30:15.123Z", + "uuid": str(EVENT_ID), + "properties": { + "$process_person_profile": False, + "architecture": "x86_64", + "auth": "none", + "cache": "redis", + "cli_version": "0.4.3", + "cloud": "none", + "database": "postgres", + "duration_bucket": "1_to_5s", + "error_category": "none", + "framework": "fastapi", + "github_requested": True, + "helm": False, + "interactive": False, + "knowledge": "none", + "language": "python", + "outcome": "success", + "platform": "linux", + "project_type": "service", + "runtime": "container", + "workspace_tooling": "none", + }, + } + + +def test_request_repr_never_contains_project_token() -> None: + request = PostHogCaptureRequest(project_token="phc_do_not_print", envelope=telemetry_envelope()) + + assert "phc_do_not_print" not in repr(request) + + +def test_request_rejects_non_public_token() -> None: + with pytest.raises(ValueError, match="public phc_"): + PostHogCaptureRequest(project_token="phx_personal_key", envelope=telemetry_envelope()) + + +def test_request_rejects_empty_public_token_body() -> None: + with pytest.raises(ValueError, match="public phc_"): + PostHogCaptureRequest(project_token="phc_", envelope=telemetry_envelope()) + + +def test_request_rejects_naive_timestamp() -> None: + envelope = telemetry_envelope() + naive_envelope = TelemetryEnvelope( + event_id=envelope.event_id, + anonymous_id=envelope.anonymous_id, + occurred_at=datetime(2026, 7, 19, 14, 30), + event=envelope.event, + ) + + with pytest.raises(ValueError, match="timezone-aware"): + PostHogCaptureRequest("phc_public_test_token", naive_envelope).as_payload() diff --git a/tests/unit/telemetry/test_posthog.py b/tests/unit/telemetry/test_posthog.py new file mode 100644 index 0000000..4cf553f --- /dev/null +++ b/tests/unit/telemetry/test_posthog.py @@ -0,0 +1,93 @@ +from typing import cast +from unittest.mock import Mock, patch + +import pytest +import requests + +from src.model.dto.posthog import PostHogCapturePayload +from src.telemetry.config import PostHogSettings +from src.telemetry.posthog import PostHogSink, RequestsPostHogTransport +from tests.unit.model.dto.test_posthog_dto import telemetry_envelope + + +class RecordingTransport: + def __init__(self) -> None: + self.calls: list[tuple[str, PostHogCapturePayload, float]] = [] + + def send(self, endpoint: str, payload: PostHogCapturePayload, timeout_seconds: float) -> None: + self.calls.append((endpoint, payload, timeout_seconds)) + + +class FailingTransport: + def __init__(self) -> None: + self.call_count = 0 + + def send(self, endpoint: str, payload: PostHogCapturePayload, timeout_seconds: float) -> None: + self.call_count += 1 + raise requests.Timeout("synthetic timeout") + + +def test_sink_maps_and_sends_one_request() -> None: + transport = RecordingTransport() + settings = PostHogSettings("phc_public_test_token") + + PostHogSink(settings, transport).record(telemetry_envelope()) + + assert len(transport.calls) == 1 + endpoint, payload, timeout = transport.calls[0] + assert endpoint == settings.endpoint + assert payload["api_key"] == "phc_public_test_token" + assert timeout == 2.0 + + +def test_sink_does_not_retry_transport_failures() -> None: + transport = FailingTransport() + + with pytest.raises(requests.Timeout, match="synthetic"): + PostHogSink(PostHogSettings("phc_public_test_token"), transport).record(telemetry_envelope()) + + assert transport.call_count == 1 + + +def test_requests_transport_posts_json_and_checks_response() -> None: + response = Mock() + response.status_code = 202 + payload = cast( + PostHogCapturePayload, + { + "api_key": "phc_public_test_token", + "distinct_id": "anonymous-id", + "event": "event", + "timestamp": "now", + "uuid": "event-id", + "properties": {}, + }, + ) + + with patch("src.telemetry.posthog.requests.post", return_value=response) as post: + RequestsPostHogTransport().send("https://example.test/capture", payload, 1.5) + + post.assert_called_once_with("https://example.test/capture", json=payload, timeout=1.5, allow_redirects=False) + response.raise_for_status.assert_called_once_with() + + +def test_requests_transport_rejects_redirect_without_following_it() -> None: + response = Mock(status_code=307) + payload = cast( + PostHogCapturePayload, + { + "api_key": "phc_public_test_token", + "distinct_id": "anonymous-id", + "event": "event", + "timestamp": "now", + "uuid": "event-id", + "properties": {}, + }, + ) + + with patch("src.telemetry.posthog.requests.post", return_value=response) as post: + with pytest.raises(requests.HTTPError, match="redirected"): + RequestsPostHogTransport().send("https://example.test/capture", payload, 1.5) + + post.assert_called_once_with("https://example.test/capture", json=payload, timeout=1.5, allow_redirects=False) + response.raise_for_status.assert_not_called() diff --git a/tests/unit/telemetry/test_reporter.py b/tests/unit/telemetry/test_reporter.py new file mode 100644 index 0000000..10c8517 --- /dev/null +++ b/tests/unit/telemetry/test_reporter.py @@ -0,0 +1,93 @@ +from datetime import UTC, datetime +from pathlib import Path +from uuid import UUID + +from src.model.dto.telemetry import TelemetryEvent +from src.telemetry.posthog import PostHogSink +from src.telemetry.reporter import TelemetryReporter, default_telemetry_reporter +from src.telemetry.sink import InMemoryTelemetrySink, NoOpTelemetrySink +from src.telemetry.state import TelemetryStateStore +from tests.unit.model.dto.test_posthog_dto import telemetry_envelope + + +EVENT_ID = UUID("33333333-3333-4333-8333-333333333333") +NOW = datetime(2026, 7, 19, 15, 0, tzinfo=UTC) + + +class FailingSink: + def record(self, envelope: object) -> None: + raise RuntimeError("synthetic transport failure") + + +def _event() -> TelemetryEvent: + return telemetry_envelope().event + + +def test_reporter_adds_stable_identity_and_delivery_metadata(tmp_path: Path) -> None: + store = TelemetryStateStore(tmp_path / "telemetry.json") + state = store.enable() + sink = InMemoryTelemetrySink() + reporter = TelemetryReporter( + state_store=store, + sink=sink, + environ={}, + development=False, + clock=lambda: NOW, + uuid_factory=lambda: EVENT_ID, + ) + + event = _event() + reporter.record(event) + + assert len(sink.envelopes) == 1 + assert sink.envelopes[0].anonymous_id == state.anonymous_id + assert sink.envelopes[0].event_id == EVENT_ID + assert sink.envelopes[0].occurred_at == NOW + assert sink.envelopes[0].event is event + + +def test_reporter_does_nothing_before_opt_in(tmp_path: Path) -> None: + sink = InMemoryTelemetrySink() + reporter = TelemetryReporter( + state_store=TelemetryStateStore(tmp_path / "telemetry.json"), + sink=sink, + environ={}, + development=False, + ) + + reporter.record(_event()) + + assert sink.envelopes == [] + + +def test_reporter_contains_sink_failures(tmp_path: Path) -> None: + store = TelemetryStateStore(tmp_path / "telemetry.json") + store.enable() + reporter = TelemetryReporter(store, FailingSink(), {}, development=False) + + reporter.record(_event()) + + +def test_default_reporter_without_token_uses_noop_sink(tmp_path: Path) -> None: + reporter = default_telemetry_reporter({"XDG_CONFIG_HOME": str(tmp_path)}, development=False) + reporter.state_store.enable() + + reporter.record(_event()) + + assert isinstance(reporter.sink, NoOpTelemetrySink) + + +def test_environment_token_does_not_enable_telemetry(tmp_path: Path) -> None: + state_path = tmp_path / "kickstart" / "telemetry.json" + reporter = default_telemetry_reporter( + { + "XDG_CONFIG_HOME": str(tmp_path), + "POSTHOG_PUBLIC_CUSTOMER_API_TOKEN": "phc_public_test_token", + }, + development=False, + ) + + reporter.record(_event()) + + assert isinstance(reporter.sink, PostHogSink) + assert not state_path.exists() diff --git a/tests/unit/telemetry/test_telemetry_config.py b/tests/unit/telemetry/test_telemetry_config.py new file mode 100644 index 0000000..9a3001d --- /dev/null +++ b/tests/unit/telemetry/test_telemetry_config.py @@ -0,0 +1,43 @@ +from pathlib import Path + +import pytest + +from src.telemetry.config import POSTHOG_US_CAPTURE_ENDPOINT, PostHogSettings, posthog_settings_from_environment + + +def test_environment_config_accepts_only_public_capture_token() -> None: + settings = posthog_settings_from_environment( + { + "POSTHOG_PUBLIC_CUSTOMER_API_TOKEN": "phc_public_test_token", + "POSTHOG_PROJECT_ID": "123456", + } + ) + + assert settings is not None + assert settings.project_token == "phc_public_test_token" + assert settings.endpoint == POSTHOG_US_CAPTURE_ENDPOINT + + +def test_project_id_alone_does_not_configure_capture() -> None: + assert posthog_settings_from_environment({"POSTHOG_PROJECT_ID": "123456"}) is None + + +def test_personal_api_key_is_rejected() -> None: + assert posthog_settings_from_environment({"POSTHOG_PUBLIC_CUSTOMER_API_TOKEN": "phx_secret"}) is None + + +def test_empty_public_token_body_is_rejected() -> None: + assert posthog_settings_from_environment({"POSTHOG_PUBLIC_CUSTOMER_API_TOKEN": "phc_"}) is None + + +def test_settings_repr_never_contains_project_token() -> None: + settings = PostHogSettings("phc_do_not_print") + + assert "phc_do_not_print" not in repr(settings) + + +def test_configuration_never_auto_loads_cwd_dotenv(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + (tmp_path / ".env").write_text("POSTHOG_PUBLIC_CUSTOMER_API_TOKEN=phc_untrusted_project_value\n", encoding="utf-8") + monkeypatch.chdir(tmp_path) + + assert posthog_settings_from_environment({}) is None From cdbcca9a8795b4fff40c19067f92166f11f28ab0 Mon Sep 17 00:00:00 2001 From: Jean-Michel Bouchard Date: Sun, 19 Jul 2026 15:36:19 -0400 Subject: [PATCH 03/11] Instrument create with privacy-safe telemetry Attempt one allowlisted terminal event for create outcomes, normalize aliases and unknown values without names or paths, suppress telemetry in tests and evals, and smoke-test the packaged controls. Checks: make check (700 tests); run_evals.py --tier smoke; binary packaging smoke Refs ENG-200 --- scripts/bootstrap_eval.py | 5 +- scripts/ci/name-and-smoke-binary.sh | 8 + scripts/run_evals.py | 5 +- scripts/scaffold_matrix_eval.py | 1 + scripts/token_savings_eval.py | 4 + src/cli/main.py | 97 +++++-- src/model/dto/__init__.py | 2 + src/model/dto/telemetry.py | 23 ++ src/telemetry/events.py | 227 ++++++++++++++++ tests/integration/conftest.py | 1 + tests/unit/cli/test_create_telemetry.py | 129 ++++++++++ tests/unit/model/dto/test_telemetry_dto.py | 8 +- tests/unit/telemetry/test_events.py | 285 +++++++++++++++++++++ 13 files changed, 770 insertions(+), 25 deletions(-) create mode 100644 src/telemetry/events.py create mode 100644 tests/unit/cli/test_create_telemetry.py create mode 100644 tests/unit/telemetry/test_events.py diff --git a/scripts/bootstrap_eval.py b/scripts/bootstrap_eval.py index c060b0d..4aa745c 100644 --- a/scripts/bootstrap_eval.py +++ b/scripts/bootstrap_eval.py @@ -26,6 +26,7 @@ import argparse from dataclasses import dataclass import json +import os from pathlib import Path import re import shutil @@ -334,7 +335,9 @@ def audit_capability_tests(root: Path) -> tuple[Violation, ...]: def generate_case(case: BootstrapCase, root: Path, repo_root: Path) -> subprocess.CompletedProcess[str]: """Generate one scaffold via the CLI module, mirroring agent usage.""" command = (sys.executable, "-m", "src.cli.main", *case.args, "--root", str(root)) - return subprocess.run(command, cwd=repo_root, capture_output=True, text=True, check=False) + env = os.environ.copy() + env["KICKSTART_EVAL"] = "1" + return subprocess.run(command, cwd=repo_root, env=env, capture_output=True, text=True, check=False) def run_make_check(project: Path, env: dict[str, str], timeout_seconds: int) -> subprocess.CompletedProcess[str]: diff --git a/scripts/ci/name-and-smoke-binary.sh b/scripts/ci/name-and-smoke-binary.sh index 623027a..f6249a0 100755 --- a/scripts/ci/name-and-smoke-binary.sh +++ b/scripts/ci/name-and-smoke-binary.sh @@ -31,6 +31,14 @@ mv "${DIST_DIR}/kickstart" "${payload}" "${payload}/kickstart" version "${payload}/kickstart" --help +# Verify the telemetry controls are bundled and status remains read-only. +mkdir -p "${SMOKE_ROOT}" +telemetry_config="$(mktemp -d "${SMOKE_ROOT}/telemetry-smoke.XXXXXX")" +XDG_CONFIG_HOME="${telemetry_config}" KICKSTART_TELEMETRY_DISABLED=1 \ + "${payload}/kickstart" telemetry status --json +test ! -e "${telemetry_config}/kickstart/telemetry.json" +rm -rf "${telemetry_config}" + # Smoke-test the install subcommand end-to-end against an isolated target dir. here="$(cd "$(dirname "$0")" && pwd)" "${here}/smoke-install.sh" \ diff --git a/scripts/run_evals.py b/scripts/run_evals.py index 8e064db..a8eae05 100644 --- a/scripts/run_evals.py +++ b/scripts/run_evals.py @@ -23,6 +23,7 @@ import argparse from dataclasses import dataclass +import os from pathlib import Path import subprocess import sys @@ -115,7 +116,9 @@ def tier_steps(tier: str) -> tuple[EvalStep, ...]: def run_step(step: EvalStep, repo_root: Path) -> StepResult: """Run one step, capturing its tail for the summary.""" started = time.monotonic() - completed = subprocess.run(step.args, cwd=repo_root, capture_output=True, text=True, check=False) + env = os.environ.copy() + env["KICKSTART_EVAL"] = "1" + completed = subprocess.run(step.args, cwd=repo_root, env=env, capture_output=True, text=True, check=False) elapsed = time.monotonic() - started tail = "\n".join((completed.stdout + completed.stderr).strip().splitlines()[-8:]) return StepResult(step=step, returncode=completed.returncode, seconds=elapsed, tail=tail) diff --git a/scripts/scaffold_matrix_eval.py b/scripts/scaffold_matrix_eval.py index 5ed9b42..526551f 100644 --- a/scripts/scaffold_matrix_eval.py +++ b/scripts/scaffold_matrix_eval.py @@ -539,6 +539,7 @@ def _system_component_name(plan: ProjectPlan, component: ComponentPlan) -> str: def _run_command(command: tuple[str, ...], repo_root: Path) -> subprocess.CompletedProcess[str]: env = os.environ.copy() env["PYTHONPATH"] = str(repo_root) + env["KICKSTART_EVAL"] = "1" return subprocess.run( command, cwd=repo_root, diff --git a/scripts/token_savings_eval.py b/scripts/token_savings_eval.py index c852bcd..b2ed68b 100644 --- a/scripts/token_savings_eval.py +++ b/scripts/token_savings_eval.py @@ -22,6 +22,7 @@ import argparse from dataclasses import dataclass import json +import os from pathlib import Path import shutil import subprocess @@ -202,9 +203,12 @@ def run_case(case: ScaffoldCase, output_root: Path, repo_root: Path) -> CaseResu shutil.rmtree(case_root) case_root.mkdir(parents=True) + env = os.environ.copy() + env["KICKSTART_EVAL"] = "1" completed = subprocess.run( scaffold_command(case, case_root), cwd=repo_root, + env=env, capture_output=True, text=True, check=False, diff --git a/src/cli/main.py b/src/cli/main.py index 5ce2d9c..184f810 100644 --- a/src/cli/main.py +++ b/src/cli/main.py @@ -5,6 +5,7 @@ from contextlib import contextmanager from dataclasses import dataclass from pathlib import Path +from time import monotonic from typing import Optional, cast import typer @@ -32,8 +33,14 @@ from src.cli.options import CreateCommandOptions, CreateOptions, ResolvedCreateArgs from src.cli.prompts import ConfirmReader, PromptReader, prompt_for_missing_args from src.cli.telemetry import telemetry_app -from src.utils.errors import KickstartError +from src.model.dto.telemetry import ( + ScaffoldCreateContext, + ScaffoldCreateErrorCategory, + ScaffoldCreateOutcome, +) +from src.telemetry.events import capture_scaffold_create_terminal, classify_scaffold_create_exception from src.utils.config import load_config +from src.utils.errors import KickstartError from src.utils.installer import ( BINARY_NAME, DEFAULT_APP_ROOT, @@ -381,10 +388,7 @@ def _apply_path_update(ctx: _InstallContext) -> None: print(f"[green]✔ Updated {update.rc_file} with a managed PATH entry.[/]") else: print(f"[green]✔ {update.rc_file} already contains the managed PATH entry.[/]") - print( - "[cyan]Restart your shell or run " - f"[bold]source {update.rc_file}[/bold] to pick up the new PATH.[/cyan]" - ) + print(f"[cyan]Restart your shell or run [bold]source {update.rc_file}[/bold] to pick up the new PATH.[/cyan]") def _clean_managed_path_block(ctx: _InstallContext) -> None: @@ -519,6 +523,29 @@ def _dispatch_project_creation( ) +def _scaffold_create_context( + options: CreateCommandOptions | CreateOptions, + *, + interactive: bool, +) -> ScaffoldCreateContext: + """Copy only telemetry-approved create options; exclude name and root.""" + return ScaffoldCreateContext( + project_type=options.project_type, + language=options.lang, + runtime=options.runtime, + cloud=options.cloud, + framework=options.framework, + database=options.database, + cache=options.cache, + auth=options.auth, + knowledge=options.knowledge, + workspace_tooling=options.workspace_tooling, + helm=options.helm, + github_requested=options.gh, + interactive=interactive, + ) + + @app.command( epilog=( "Examples:\n\n" @@ -564,7 +591,9 @@ def create( help="HTTP framework (minimal for standard library, default is FastAPI)", ), cloud: str = typer.Option("multi", "--cloud", help="System provider target (aws, gcp, cloudflare, multi, none)"), - knowledge: str = typer.Option("none", "--knowledge", help="External knowledge adapter metadata (none, obsidian, backstage, both)"), + knowledge: str = typer.Option( + "none", "--knowledge", help="External knowledge adapter metadata (none, obsidian, backstage, both)" + ), runtime: Optional[str] = typer.Option( None, "--runtime", @@ -577,29 +606,36 @@ def create( ), ) -> None: """Create a new service, lib, CLI, frontend, or system.""" + command_options = CreateCommandOptions( + project_type=project_type, + name=name, + root=root, + lang=lang, + gh=gh, + helm=helm, + database=database, + cache=cache, + auth=auth, + framework=framework, + cloud=cloud, + knowledge=knowledge, + runtime=runtime, + workspace_tooling=workspace_tooling, + ) + interactive = not project_type or not name + telemetry_context = _scaffold_create_context(command_options, interactive=interactive) + started_at = monotonic() + outcome = ScaffoldCreateOutcome.UNEXPECTED_ERROR + error_category = ScaffoldCreateErrorCategory.UNEXPECTED_ERROR try: config: GeneratorConfig = load_config() options = prompt_for_missing_args( - CreateCommandOptions( - project_type=project_type, - name=name, - root=root, - lang=lang, - gh=gh, - helm=helm, - database=database, - cache=cache, - auth=auth, - framework=framework, - cloud=cloud, - knowledge=knowledge, - runtime=runtime, - workspace_tooling=workspace_tooling, - ), + command_options, config, prompt=cast(PromptReader, Prompt), confirm=cast(ConfirmReader, Confirm), ) + telemetry_context = _scaffold_create_context(options, interactive=interactive) dispatch_project_creation(options, config, _project_creators()) project_path = Path(options.root) / options.name if options.root else Path(options.name) print( @@ -607,29 +643,46 @@ def create( f" cd {escape(str(project_path))} && make check [dim]# install deps, lint, typecheck, test[/]\n" f" cat AGENTS.md [dim]# orientation map; scaffold metadata in .kickstart/scaffold.json[/]" ) + outcome = ScaffoldCreateOutcome.SUCCESS + error_category = ScaffoldCreateErrorCategory.NONE except KeyboardInterrupt: + outcome = ScaffoldCreateOutcome.CANCELLED + error_category = ScaffoldCreateErrorCategory.INTERRUPTED print("\n[yellow]Operation cancelled by user.[/]") # Conventional SIGINT exit status; cancel must not read as success. raise typer.Exit(code=130) from None except EOFError as exc: + outcome = ScaffoldCreateOutcome.INPUT_ENDED + error_category = ScaffoldCreateErrorCategory.INPUT_ENDED print( "[bold red]Interactive input ended before required arguments were provided. " "Pass them explicitly, for example: kickstart create service my-api --lang python[/]" ) raise typer.Exit(code=2) from exc except KickstartError as exc: + outcome, error_category = classify_scaffold_create_exception(exc) print(f"[bold red]Failed to create project: {exc}[/]") raise typer.Exit(code=1) from exc except ValueError as exc: + outcome, error_category = classify_scaffold_create_exception(exc) # Stack-registry validation errors (unknown runtime/cloud/...) are # user-input problems: report them cleanly without a traceback. print(f"[bold red]Failed to create project: {exc}[/]") raise typer.Exit(code=1) from exc except Exception as exc: + outcome, error_category = classify_scaffold_create_exception(exc) print(f"[bold red]Failed to create project: {exc}[/]") logger.exception("Project creation failed") raise typer.Exit(code=1) from exc + finally: + capture_scaffold_create_terminal( + telemetry_context, + outcome, + error_category, + monotonic() - started_at, + cli_version=__version__, + ) def main() -> None: diff --git a/src/model/dto/__init__.py b/src/model/dto/__init__.py index 2f7f528..640db71 100644 --- a/src/model/dto/__init__.py +++ b/src/model/dto/__init__.py @@ -3,6 +3,7 @@ from src.model.dto.posthog import PostHogCapturePayload, PostHogCaptureRequest from src.model.dto.telemetry import ( EffectiveTelemetry, + ScaffoldCreateContext, ScaffoldCreateErrorCategory, ScaffoldCreateOutcome, ScaffoldCreateProperties, @@ -20,6 +21,7 @@ "EffectiveTelemetry", "PostHogCapturePayload", "PostHogCaptureRequest", + "ScaffoldCreateContext", "ScaffoldCreateErrorCategory", "ScaffoldCreateOutcome", "ScaffoldCreateProperties", diff --git a/src/model/dto/telemetry.py b/src/model/dto/telemetry.py index 135427c..568da04 100644 --- a/src/model/dto/telemetry.py +++ b/src/model/dto/telemetry.py @@ -46,6 +46,7 @@ class ScaffoldCreateErrorCategory(StrEnum): UNSUPPORTED_OPTION = "unsupported_option" PROJECT_CREATION_ERROR = "project_creation_error" INVALID_CONFIGURATION = "invalid_configuration" + EXPECTED_ERROR = "expected_error" UNEXPECTED_ERROR = "unexpected_error" @@ -122,6 +123,28 @@ def as_mapping(self) -> dict[str, TelemetryPropertyValue]: } +@dataclass(frozen=True) +class ScaffoldCreateContext: + """Safe create inputs retained for terminal event normalization. + + Project names and roots are intentionally absent from this DTO. + """ + + project_type: str | None + language: str + runtime: str | None + cloud: str + framework: str | None + database: str | None + cache: str | None + auth: str | None + knowledge: str + workspace_tooling: str | None + helm: bool + github_requested: bool + interactive: bool + + @dataclass(frozen=True) class TelemetryEvent: """A typed product event before identity and delivery metadata are added.""" diff --git a/src/telemetry/events.py b/src/telemetry/events.py new file mode 100644 index 0000000..ea911de --- /dev/null +++ b/src/telemetry/events.py @@ -0,0 +1,227 @@ +"""Build and record closed, provider-neutral product events.""" + +import platform as runtime_platform + +from src.model.dto.telemetry import ( + ScaffoldCreateContext, + ScaffoldCreateErrorCategory, + ScaffoldCreateOutcome, + ScaffoldCreateProperties, + TelemetryDurationBucket, + TelemetryEvent, + TelemetryEventName, +) +from src.stack.profile import stack_registry +from src.telemetry.reporter import TelemetryReporter, default_telemetry_reporter +from src.utils.errors import ( + DirectoryCreationError, + ExtensionError, + FileOperationError, + InvalidProjectNameError, + KickstartError, + LanguageNotSupportedError, + ManifestShapeError, + MarkerError, + MissingCreateArgumentsError, + ProjectCreationError, + TemplateError, + UnsupportedOptionError, + UnsupportedProjectTypeError, +) + + +_PROJECT_TYPES = {"service", "frontend", "lib", "cli", "system"} +_LEGACY_SYSTEM_PROJECT_TYPES = {"mono", "monorepo"} +_FRAMEWORKS = {"fastapi", "minimal"} +_DATABASES = {"none", "postgres"} +_CACHES = {"none", "redis"} +_AUTH = {"none", "jwt"} +_CONFIGURATION_ERRORS = ( + ExtensionError, + InvalidProjectNameError, + LanguageNotSupportedError, + MissingCreateArgumentsError, +) +_PROJECT_CREATION_ERRORS = ( + DirectoryCreationError, + FileOperationError, + ManifestShapeError, + MarkerError, + ProjectCreationError, + TemplateError, +) + + +def build_scaffold_create_event( + context: ScaffoldCreateContext, + outcome: ScaffoldCreateOutcome, + error_category: ScaffoldCreateErrorCategory, + duration_seconds: float, + *, + cli_version: str, + platform_name: str | None = None, + architecture: str | None = None, +) -> TelemetryEvent: + """Normalize one terminal create result into the closed property DTO.""" + project_type = _project_type(context.project_type) + language = _language(context.language, project_type) + return TelemetryEvent( + name=TelemetryEventName.SCAFFOLD_CREATE_COMPLETED, + properties=ScaffoldCreateProperties( + cli_version=cli_version, + project_type=project_type, + language=language, + runtime=_runtime(context.runtime, project_type), + cloud=_cloud(context.cloud, project_type), + framework=_framework(context.framework, project_type, language), + database=_extension(context.database, project_type, _DATABASES), + cache=_extension(context.cache, project_type, _CACHES), + auth=_extension(context.auth, project_type, _AUTH), + knowledge=_knowledge(context.knowledge, project_type), + workspace_tooling=_workspace_tooling( + context.workspace_tooling, + context.project_type, + project_type, + ), + helm=context.helm, + github_requested=context.github_requested, + interactive=context.interactive, + outcome=outcome, + error_category=error_category, + duration_bucket=_duration_bucket(duration_seconds), + platform=_platform(platform_name or runtime_platform.system()), + architecture=_architecture(architecture or runtime_platform.machine()), + ), + ) + + +def capture_scaffold_create_terminal( + context: ScaffoldCreateContext, + outcome: ScaffoldCreateOutcome, + error_category: ScaffoldCreateErrorCategory, + duration_seconds: float, + *, + cli_version: str, + reporter: TelemetryReporter | None = None, +) -> None: + """Attempt one terminal capture while containing mapping and reporter failures.""" + try: + event = build_scaffold_create_event( + context, + outcome, + error_category, + duration_seconds, + cli_version=cli_version, + ) + target_reporter = default_telemetry_reporter() if reporter is None else reporter + target_reporter.record(event) + except Exception: + return + + +def classify_scaffold_create_exception( + error: Exception, +) -> tuple[ScaffoldCreateOutcome, ScaffoldCreateErrorCategory]: + """Reduce handled exceptions to fixed categories without serializing details.""" + if isinstance(error, UnsupportedProjectTypeError): + return ScaffoldCreateOutcome.EXPECTED_ERROR, ScaffoldCreateErrorCategory.UNSUPPORTED_PROJECT_TYPE + if isinstance(error, UnsupportedOptionError): + return ScaffoldCreateOutcome.EXPECTED_ERROR, ScaffoldCreateErrorCategory.UNSUPPORTED_OPTION + if isinstance(error, _CONFIGURATION_ERRORS): + return ScaffoldCreateOutcome.EXPECTED_ERROR, ScaffoldCreateErrorCategory.INVALID_CONFIGURATION + if isinstance(error, _PROJECT_CREATION_ERRORS): + return ScaffoldCreateOutcome.EXPECTED_ERROR, ScaffoldCreateErrorCategory.PROJECT_CREATION_ERROR + if isinstance(error, KickstartError): + return ScaffoldCreateOutcome.EXPECTED_ERROR, ScaffoldCreateErrorCategory.EXPECTED_ERROR + if isinstance(error, ValueError): + return ScaffoldCreateOutcome.INVALID_CONFIGURATION, ScaffoldCreateErrorCategory.INVALID_CONFIGURATION + return ScaffoldCreateOutcome.UNEXPECTED_ERROR, ScaffoldCreateErrorCategory.UNEXPECTED_ERROR + + +def _project_type(value: str | None) -> str: + candidate = "" if value is None else value.strip().lower() + if candidate in _LEGACY_SYSTEM_PROJECT_TYPES: + return "system" + return candidate if candidate in _PROJECT_TYPES else "unknown" + + +def _language(value: str, project_type: str) -> str: + if project_type in {"frontend", "system"}: + return "none" + normalized = stack_registry.normalize_language(value) + return normalized if normalized in stack_registry.languages else "unknown" + + +def _runtime(value: str | None, project_type: str) -> str: + if project_type == "service": + normalized = stack_registry.normalize_service_runtime(value or "container") + return normalized if normalized in stack_registry.service_runtimes else "unknown" + if project_type == "system": + normalized = stack_registry.normalize_system_runtime(value or "kubernetes") + return normalized if normalized in stack_registry.system_runtimes else "unknown" + return "none" + + +def _cloud(value: str, project_type: str) -> str: + if project_type != "system": + return "none" + normalized = stack_registry.normalize_cloud(value) + return normalized if normalized in stack_registry.clouds else "unknown" + + +def _framework(value: str | None, project_type: str, language: str) -> str: + if project_type != "service": + return "none" + if language == "python" and value is None: + return "fastapi" + if value is None or value == "none": + return "none" + candidate = value.strip().lower() + return candidate if candidate in _FRAMEWORKS else "unknown" + + +def _extension(value: str | None, project_type: str, known: set[str]) -> str: + if project_type != "service" or value is None: + return "none" + candidate = value.strip().lower() + return candidate if candidate in known else "unknown" + + +def _knowledge(value: str, project_type: str) -> str: + if project_type != "system": + return "none" + normalized = stack_registry.normalize_knowledge(value) + return normalized if normalized in stack_registry.knowledge else "unknown" + + +def _workspace_tooling(value: str | None, raw_project_type: str | None, project_type: str) -> str: + if project_type != "system": + return "none" + raw_type = "" if raw_project_type is None else raw_project_type.strip().lower() + default = "bun-turbo" if raw_type in _LEGACY_SYSTEM_PROJECT_TYPES else "none" + normalized = stack_registry.normalize_workspace_tooling(value or default) + return normalized if normalized in stack_registry.workspace_tooling else "unknown" + + +def _duration_bucket(duration_seconds: float) -> TelemetryDurationBucket: + if duration_seconds < 1: + return TelemetryDurationBucket.UNDER_ONE_SECOND + if duration_seconds < 5: + return TelemetryDurationBucket.ONE_TO_FIVE_SECONDS + if duration_seconds < 30: + return TelemetryDurationBucket.FIVE_TO_THIRTY_SECONDS + return TelemetryDurationBucket.THIRTY_SECONDS_OR_MORE + + +def _platform(value: str) -> str: + candidate = value.strip().lower() + return {"darwin": "macos", "linux": "linux", "windows": "windows"}.get(candidate, "other") + + +def _architecture(value: str) -> str: + candidate = value.strip().lower() + if candidate in {"arm64", "aarch64"}: + return "arm64" + if candidate in {"x86_64", "amd64"}: + return "x86_64" + return "other" diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index 56ef399..6c65c54 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -37,6 +37,7 @@ def kickstart_env(repo_root: Path) -> dict[str, str]: """`os.environ`-shaped dict that lets `python -m src.cli.main` resolve `src.*`.""" env = os.environ.copy() env["PYTHONPATH"] = str(repo_root) + env["KICKSTART_TELEMETRY_DISABLED"] = "1" return env diff --git a/tests/unit/cli/test_create_telemetry.py b/tests/unit/cli/test_create_telemetry.py new file mode 100644 index 0000000..2cccf98 --- /dev/null +++ b/tests/unit/cli/test_create_telemetry.py @@ -0,0 +1,129 @@ +from unittest.mock import patch + +from typer.testing import CliRunner + +from src.cli.main import app +from src.model.dto.telemetry import ( + ScaffoldCreateContext, + ScaffoldCreateErrorCategory, + ScaffoldCreateOutcome, +) +from src.utils.errors import ProjectCreationError + + +def test_success_attempts_one_terminal_event_after_creation() -> None: + order: list[str] = [] + runner = CliRunner() + with ( + patch("src.cli.main.load_config", return_value={}), + patch("src.cli.main.create_service", side_effect=lambda *args, **kwargs: order.append("create")), + patch( + "src.cli.main.capture_scaffold_create_terminal", side_effect=lambda *args, **kwargs: order.append("capture") + ) as capture, + patch("src.cli.main.monotonic", side_effect=[10.0, 12.0]), + ): + result = runner.invoke( + app, + ["create", "service", "private-project-name", "--root", "/private/project/root", "--lang", "ts"], + ) + + assert result.exit_code == 0 + assert order == ["create", "capture"] + capture.assert_called_once() + context, outcome, error_category, duration = capture.call_args.args + assert isinstance(context, ScaffoldCreateContext) + assert not hasattr(context, "name") + assert not hasattr(context, "root") + assert context.project_type == "service" + assert context.language == "ts" + assert outcome is ScaffoldCreateOutcome.SUCCESS + assert error_category is ScaffoldCreateErrorCategory.NONE + assert duration == 2.0 + + +def test_expected_creation_failure_preserves_exit_and_attempts_one_terminal_event() -> None: + runner = CliRunner() + private_error = "private failure at /private/project/root" + with ( + patch("src.cli.main.load_config", return_value={}), + patch("src.cli.main.create_service", side_effect=ProjectCreationError(private_error)), + patch("src.cli.main.capture_scaffold_create_terminal") as capture, + ): + result = runner.invoke(app, ["create", "service", "private-project-name", "--root", "/tmp"]) + + assert result.exit_code == 1 + assert private_error in result.stdout + capture.assert_called_once() + context, outcome, error_category, _duration = capture.call_args.args + assert isinstance(context, ScaffoldCreateContext) + assert outcome is ScaffoldCreateOutcome.EXPECTED_ERROR + assert error_category is ScaffoldCreateErrorCategory.PROJECT_CREATION_ERROR + assert private_error not in repr(context) + + +def test_keyboard_interrupt_preserves_exit_130_and_attempts_one_terminal_event() -> None: + runner = CliRunner() + with ( + patch("src.cli.main.load_config", side_effect=KeyboardInterrupt), + patch("src.cli.main.capture_scaffold_create_terminal") as capture, + ): + result = runner.invoke(app, ["create", "service", "private-project-name"]) + + assert result.exit_code == 130 + capture.assert_called_once() + assert capture.call_args.args[1:3] == ( + ScaffoldCreateOutcome.CANCELLED, + ScaffoldCreateErrorCategory.INTERRUPTED, + ) + + +def test_input_end_preserves_exit_2_and_attempts_one_terminal_event() -> None: + runner = CliRunner() + with ( + patch("src.cli.main.load_config", side_effect=EOFError), + patch("src.cli.main.capture_scaffold_create_terminal") as capture, + ): + result = runner.invoke(app, ["create", "service", "private-project-name"]) + + assert result.exit_code == 2 + capture.assert_called_once() + assert capture.call_args.args[1:3] == ( + ScaffoldCreateOutcome.INPUT_ENDED, + ScaffoldCreateErrorCategory.INPUT_ENDED, + ) + + +def test_missing_name_marks_partial_argument_prompt_flow_interactive() -> None: + runner = CliRunner() + with ( + patch("src.cli.main.load_config", return_value={}), + patch("src.cli.main.Prompt.ask", side_effect=["none", "none", "none", "fastapi"]), + patch("src.cli.main.capture_scaffold_create_terminal") as capture, + ): + result = runner.invoke(app, ["create", "service"]) + + assert result.exit_code == 1 + capture.assert_called_once() + context = capture.call_args.args[0] + assert isinstance(context, ScaffoldCreateContext) + assert context.interactive is True + + +def test_parser_failure_before_create_emits_no_event() -> None: + runner = CliRunner() + with patch("src.cli.main.capture_scaffold_create_terminal") as capture: + result = runner.invoke(app, ["create", "service", "private-project-name", "--not-a-real-option"]) + + assert result.exit_code == 2 + capture.assert_not_called() + + +def test_non_create_commands_emit_no_event() -> None: + runner = CliRunner() + with patch("src.cli.main.capture_scaffold_create_terminal") as capture: + help_result = runner.invoke(app, ["--help"]) + version_result = runner.invoke(app, ["version"]) + + assert help_result.exit_code == 0 + assert version_result.exit_code == 0 + capture.assert_not_called() diff --git a/tests/unit/model/dto/test_telemetry_dto.py b/tests/unit/model/dto/test_telemetry_dto.py index dacdf8c..a3615fa 100644 --- a/tests/unit/model/dto/test_telemetry_dto.py +++ b/tests/unit/model/dto/test_telemetry_dto.py @@ -5,13 +5,14 @@ import pytest from src.model.dto.telemetry import ( - TelemetryEnvelope, + ScaffoldCreateContext, ScaffoldCreateErrorCategory, ScaffoldCreateOutcome, ScaffoldCreateProperties, TelemetryDurationBucket, TelemetryEvent, TelemetryEventName, + TelemetryEnvelope, ) @@ -82,3 +83,8 @@ def test_envelope_uses_provider_neutral_identity_names() -> None: assert "posthog" not in repr(envelope).lower() assert envelope.anonymous_id.version == 4 + + +def test_create_context_cannot_carry_project_name_or_root() -> None: + assert "name" not in ScaffoldCreateContext.__dataclass_fields__ + assert "root" not in ScaffoldCreateContext.__dataclass_fields__ diff --git a/tests/unit/telemetry/test_events.py b/tests/unit/telemetry/test_events.py new file mode 100644 index 0000000..405f9d0 --- /dev/null +++ b/tests/unit/telemetry/test_events.py @@ -0,0 +1,285 @@ +from pathlib import Path + +import pytest + +from src.model.dto.telemetry import ( + ScaffoldCreateContext, + ScaffoldCreateErrorCategory, + ScaffoldCreateOutcome, + TelemetryDurationBucket, +) +from src.telemetry.events import ( + build_scaffold_create_event, + capture_scaffold_create_terminal, + classify_scaffold_create_exception, +) +from src.telemetry.reporter import TelemetryReporter +from src.telemetry.sink import InMemoryTelemetrySink +from src.telemetry.state import TelemetryStateStore +from src.utils.errors import ( + ExtensionError, + KickstartError, + ProjectCreationError, + UnsupportedOptionError, + UnsupportedProjectTypeError, +) + + +def _service_context() -> ScaffoldCreateContext: + return ScaffoldCreateContext( + project_type="service", + language="ts", + runtime="docker", + cloud="private-cloud-value", + framework=None, + database="POSTGRES", + cache="redis", + auth="jwt", + knowledge="private-knowledge-value", + workspace_tooling="private-workspace-value", + helm=True, + github_requested=True, + interactive=False, + ) + + +def test_service_event_canonicalizes_aliases_and_ignores_inapplicable_values() -> None: + event = build_scaffold_create_event( + _service_context(), + ScaffoldCreateOutcome.SUCCESS, + ScaffoldCreateErrorCategory.NONE, + 2.5, + cli_version="0.4.3", + platform_name="Darwin", + architecture="aarch64", + ) + + assert event.properties.as_mapping() == { + "architecture": "arm64", + "auth": "jwt", + "cache": "redis", + "cli_version": "0.4.3", + "cloud": "none", + "database": "postgres", + "duration_bucket": "1_to_5s", + "error_category": "none", + "framework": "none", + "github_requested": True, + "helm": True, + "interactive": False, + "knowledge": "none", + "language": "typescript", + "outcome": "success", + "platform": "macos", + "project_type": "service", + "runtime": "container", + "workspace_tooling": "none", + } + + +def test_legacy_monorepo_event_uses_system_defaults() -> None: + context = ScaffoldCreateContext( + project_type="monorepo", + language="private-language-value", + runtime="k8s", + cloud="google", + framework="private-framework-value", + database="private-database-value", + cache="private-cache-value", + auth="private-auth-value", + knowledge="both", + workspace_tooling=None, + helm=False, + github_requested=False, + interactive=True, + ) + + properties = build_scaffold_create_event( + context, + ScaffoldCreateOutcome.SUCCESS, + ScaffoldCreateErrorCategory.NONE, + 10, + cli_version="0.4.3", + platform_name="Linux", + architecture="amd64", + ).properties + + assert properties.project_type == "system" + assert properties.language == "none" + assert properties.runtime == "kubernetes" + assert properties.cloud == "gcp" + assert properties.knowledge == "both" + assert properties.workspace_tooling == "bun-turbo" + assert properties.framework == "none" + assert properties.database == "none" + assert properties.duration_bucket is TelemetryDurationBucket.FIVE_TO_THIRTY_SECONDS + + +def test_canonical_system_defaults_to_vendor_neutral_workspace() -> None: + context = ScaffoldCreateContext( + project_type="system", + language="python", + runtime=None, + cloud="multi", + framework=None, + database=None, + cache=None, + auth=None, + knowledge="none", + workspace_tooling=None, + helm=False, + github_requested=False, + interactive=False, + ) + + properties = build_scaffold_create_event( + context, + ScaffoldCreateOutcome.SUCCESS, + ScaffoldCreateErrorCategory.NONE, + 0.5, + cli_version="0.4.3", + platform_name="Windows", + architecture="unknown-cpu", + ).properties + + assert properties.workspace_tooling == "none" + assert properties.runtime == "kubernetes" + assert properties.platform == "windows" + assert properties.architecture == "other" + + +def test_unknown_inputs_are_reduced_to_fixed_values_without_raw_data() -> None: + raw_values = { + "private-project-type", + "private-language", + "private-runtime", + "private-cloud", + "private-framework", + "private-database", + "private-cache", + "private-auth", + "private-knowledge", + "private-workspace", + } + context = ScaffoldCreateContext( + project_type="private-project-type", + language="private-language", + runtime="private-runtime", + cloud="private-cloud", + framework="private-framework", + database="private-database", + cache="private-cache", + auth="private-auth", + knowledge="private-knowledge", + workspace_tooling="private-workspace", + helm=False, + github_requested=False, + interactive=False, + ) + + mapping = build_scaffold_create_event( + context, + ScaffoldCreateOutcome.INVALID_CONFIGURATION, + ScaffoldCreateErrorCategory.INVALID_CONFIGURATION, + 30, + cli_version="0.4.3", + platform_name="private-platform", + architecture="private-architecture", + ).properties.as_mapping() + + assert mapping["project_type"] == "unknown" + assert mapping["language"] == "unknown" + assert mapping["platform"] == "other" + assert mapping["architecture"] == "other" + assert raw_values.isdisjoint(set(str(value) for value in mapping.values())) + assert {"name", "root", "path", "error", "exception"}.isdisjoint(mapping) + + +@pytest.mark.parametrize( + ("error", "outcome", "category"), + [ + ( + UnsupportedProjectTypeError("private detail"), + ScaffoldCreateOutcome.EXPECTED_ERROR, + ScaffoldCreateErrorCategory.UNSUPPORTED_PROJECT_TYPE, + ), + ( + UnsupportedOptionError("private detail"), + ScaffoldCreateOutcome.EXPECTED_ERROR, + ScaffoldCreateErrorCategory.UNSUPPORTED_OPTION, + ), + ( + ExtensionError("private detail"), + ScaffoldCreateOutcome.EXPECTED_ERROR, + ScaffoldCreateErrorCategory.INVALID_CONFIGURATION, + ), + ( + ProjectCreationError("private detail"), + ScaffoldCreateOutcome.EXPECTED_ERROR, + ScaffoldCreateErrorCategory.PROJECT_CREATION_ERROR, + ), + ( + KickstartError("private detail"), + ScaffoldCreateOutcome.EXPECTED_ERROR, + ScaffoldCreateErrorCategory.EXPECTED_ERROR, + ), + ( + ValueError("private detail"), + ScaffoldCreateOutcome.INVALID_CONFIGURATION, + ScaffoldCreateErrorCategory.INVALID_CONFIGURATION, + ), + ( + RuntimeError("private detail"), + ScaffoldCreateOutcome.UNEXPECTED_ERROR, + ScaffoldCreateErrorCategory.UNEXPECTED_ERROR, + ), + ], +) +def test_exception_classification_never_uses_exception_text( + error: Exception, + outcome: ScaffoldCreateOutcome, + category: ScaffoldCreateErrorCategory, +) -> None: + assert classify_scaffold_create_exception(error) == (outcome, category) + + +@pytest.mark.parametrize( + ("duration", "bucket"), + [ + (0.999, TelemetryDurationBucket.UNDER_ONE_SECOND), + (1.0, TelemetryDurationBucket.ONE_TO_FIVE_SECONDS), + (5.0, TelemetryDurationBucket.FIVE_TO_THIRTY_SECONDS), + (30.0, TelemetryDurationBucket.THIRTY_SECONDS_OR_MORE), + ], +) +def test_duration_boundaries_use_coarse_buckets(duration: float, bucket: TelemetryDurationBucket) -> None: + event = build_scaffold_create_event( + _service_context(), + ScaffoldCreateOutcome.SUCCESS, + ScaffoldCreateErrorCategory.NONE, + duration, + cli_version="0.4.3", + platform_name="Linux", + architecture="x86_64", + ) + + assert event.properties.duration_bucket is bucket + + +def test_capture_helper_records_exactly_one_event_for_enabled_reporter(tmp_path: Path) -> None: + store = TelemetryStateStore(tmp_path / "telemetry.json") + store.enable() + sink = InMemoryTelemetrySink() + reporter = TelemetryReporter(store, sink, {}, development=False) + + capture_scaffold_create_terminal( + _service_context(), + ScaffoldCreateOutcome.SUCCESS, + ScaffoldCreateErrorCategory.NONE, + 0.25, + cli_version="0.4.3", + reporter=reporter, + ) + + assert len(sink.envelopes) == 1 + assert sink.envelopes[0].event.properties.outcome is ScaffoldCreateOutcome.SUCCESS From 098c1481dc72085554b1e8a3c20d363d21bd2d95 Mon Sep 17 00:00:00 2001 From: Jean-Michel Bouchard Date: Sun, 19 Jul 2026 15:45:18 -0400 Subject: [PATCH 04/11] audit-round-1: close telemetry suppression and portability gaps Suppress synthetic Cloudflare smoke events and skip descriptor chmod where the platform does not provide it. Findings: 1, 2 Checks: make check (701 tests); bash -n scripts/ci/smoke-cloudflare-worker.sh Refs ENG-197 Refs ENG-200 --- scripts/ci/smoke-cloudflare-worker.sh | 2 +- src/telemetry/state.py | 3 ++- tests/unit/telemetry/test_state.py | 12 ++++++++++++ 3 files changed, 15 insertions(+), 2 deletions(-) diff --git a/scripts/ci/smoke-cloudflare-worker.sh b/scripts/ci/smoke-cloudflare-worker.sh index c4b7e18..548dd03 100755 --- a/scripts/ci/smoke-cloudflare-worker.sh +++ b/scripts/ci/smoke-cloudflare-worker.sh @@ -51,7 +51,7 @@ log_and_run() { log_and_run "generation" \ "${KICKSTART_CMD} create service ${PROJECT_NAME} --root ${temp_root} --lang typescript --runtime cloudflare-workers" \ "${generation_log}" \ - "${KS[@]}" create service "${PROJECT_NAME}" \ + env KICKSTART_EVAL=1 "${KS[@]}" create service "${PROJECT_NAME}" \ --root "${temp_root}" \ --lang typescript \ --runtime cloudflare-workers diff --git a/src/telemetry/state.py b/src/telemetry/state.py index e20040d..0f14785 100644 --- a/src/telemetry/state.py +++ b/src/telemetry/state.py @@ -137,7 +137,8 @@ def _write(self, state: TelemetryState) -> None: descriptor, temporary_name = tempfile.mkstemp(prefix=".telemetry-", suffix=".tmp", dir=self.path.parent) temporary_path = Path(temporary_name) with os.fdopen(descriptor, "w", encoding="utf-8") as handle: - os.fchmod(handle.fileno(), 0o600) + if hasattr(os, "fchmod"): + os.fchmod(handle.fileno(), 0o600) handle.write(json.dumps(document, indent=2, sort_keys=True)) handle.write("\n") handle.flush() diff --git a/tests/unit/telemetry/test_state.py b/tests/unit/telemetry/test_state.py index c4fc8e8..33a3d0f 100644 --- a/tests/unit/telemetry/test_state.py +++ b/tests/unit/telemetry/test_state.py @@ -7,6 +7,7 @@ import pytest from src.model.dto.telemetry import TelemetryConsent +from src.telemetry import state as state_module from src.telemetry.state import TelemetryStateStore, default_telemetry_state_path from src.utils.errors import TelemetryStateError @@ -113,6 +114,17 @@ def test_state_file_uses_user_only_permissions(tmp_path: Path) -> None: assert stat.S_IMODE(path.parent.stat().st_mode) == 0o700 +def test_state_write_succeeds_when_descriptor_chmod_is_unavailable( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.delattr(state_module.os, "fchmod", raising=False) + + state = TelemetryStateStore(tmp_path / "telemetry.json").enable() + + assert state.consent is TelemetryConsent.ENABLED + + @pytest.mark.parametrize( "document", [ From 2e8b692162d10e59b1a282871bc5564d83d25eab Mon Sep 17 00:00:00 2001 From: Jean-Michel Bouchard Date: Sun, 19 Jul 2026 16:31:09 -0400 Subject: [PATCH 05/11] Embed PostHog token in release artifacts --- .github/workflows/ci.yml | 23 ++++- .github/workflows/release.yml | 34 +++++++ .gitignore | 2 + README.md | 2 + docs/contracts/telemetry.md | 4 +- docs/install-binaries.md | 5 + scripts/ci/name-and-smoke-binary.sh | 12 ++- scripts/embed_posthog_token.py | 85 +++++++++++++++++ scripts/verify_embedded_telemetry_artifact.py | 92 +++++++++++++++++++ src/cli/telemetry.py | 5 +- src/telemetry/_build_config.py | 6 ++ src/telemetry/config.py | 13 ++- src/telemetry/reporter.py | 4 +- tests/unit/cli/test_telemetry_cli.py | 19 +++- .../unit/scripts/test_embed_posthog_token.py | 61 ++++++++++++ ...test_verify_embedded_telemetry_artifact.py | 50 ++++++++++ tests/unit/telemetry/test_telemetry_config.py | 44 +++++++-- 17 files changed, 444 insertions(+), 17 deletions(-) create mode 100644 scripts/embed_posthog_token.py create mode 100644 scripts/verify_embedded_telemetry_artifact.py create mode 100644 src/telemetry/_build_config.py create mode 100644 tests/unit/scripts/test_embed_posthog_token.py create mode 100644 tests/unit/scripts/test_verify_embedded_telemetry_artifact.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 12dd172..fdd5133 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -219,7 +219,10 @@ jobs: build-test: name: Manual Package & Binary for ${{ matrix.platform }} - if: ${{ github.event_name == 'workflow_dispatch' }} + # This owner-triggered job intentionally uses the canonical public capture + # token so a pre-release artifact can be tested end-to-end. Fork and PR + # packaging behavior is covered by the tokenless and synthetic unit tests. + if: ${{ github.event_name == 'workflow_dispatch' && github.actor == github.repository_owner }} runs-on: ${{ matrix.runner }} needs: [lint, test] strategy: @@ -248,18 +251,36 @@ jobs: python-version: ${{ env.PYTHON_VERSION }} poetry-version: ${{ env.POETRY_VERSION }} + - name: Stage public telemetry build configuration + env: + POSTHOG_PUBLIC_CUSTOMER_API_TOKEN: ${{ secrets.POSTHOG_PUBLIC_CUSTOMER_API_TOKEN }} + run: python3 scripts/embed_posthog_token.py --write + - name: Build package run: make package + - name: Verify package telemetry configuration + env: + POSTHOG_PUBLIC_CUSTOMER_API_TOKEN: ${{ secrets.POSTHOG_PUBLIC_CUSTOMER_API_TOKEN }} + run: >- + poetry run python scripts/verify_embedded_telemetry_artifact.py + --expected-token-env POSTHOG_PUBLIC_CUSTOMER_API_TOKEN + dist/*.whl dist/*.tar.gz + - name: Build kickstart binary run: make binary - name: Name, smoke-test, and archive binary env: + EXPECT_EMBEDDED_TELEMETRY: "1" PLATFORM: ${{ matrix.platform }} PYTHON_MINOR: ${{ env.PYTHON_VERSION }} run: scripts/ci/name-and-smoke-binary.sh "${PLATFORM}" "${PYTHON_MINOR}" + - name: Clear staged public telemetry build configuration + if: ${{ always() }} + run: python3 scripts/embed_posthog_token.py --clear + - name: Upload build artifacts uses: actions/upload-artifact@v4 with: diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index fb8be07..abad857 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -38,6 +38,11 @@ jobs: - name: Verify release version run: make release-check TAG="$GITHUB_REF_NAME" + - name: Verify public telemetry build token + env: + POSTHOG_PUBLIC_CUSTOMER_API_TOKEN: ${{ secrets.POSTHOG_PUBLIC_CUSTOMER_API_TOKEN }} + run: python3 scripts/embed_posthog_token.py --check + - name: Set up Python + Poetry uses: ./.github/actions/setup-python-poetry with: @@ -82,9 +87,26 @@ jobs: python-version: ${{ env.PYTHON_VERSION }} poetry-version: ${{ env.POETRY_VERSION }} + - name: Stage public telemetry build configuration + env: + POSTHOG_PUBLIC_CUSTOMER_API_TOKEN: ${{ secrets.POSTHOG_PUBLIC_CUSTOMER_API_TOKEN }} + run: python3 scripts/embed_posthog_token.py --write + - name: Build package run: make package + - name: Verify package telemetry configuration + env: + POSTHOG_PUBLIC_CUSTOMER_API_TOKEN: ${{ secrets.POSTHOG_PUBLIC_CUSTOMER_API_TOKEN }} + run: >- + poetry run python scripts/verify_embedded_telemetry_artifact.py + --expected-token-env POSTHOG_PUBLIC_CUSTOMER_API_TOKEN + dist/*.whl dist/*.tar.gz + + - name: Clear staged public telemetry build configuration + if: ${{ always() }} + run: python3 scripts/embed_posthog_token.py --clear + - name: Upload package artifact uses: actions/upload-artifact@v4 with: @@ -122,15 +144,25 @@ jobs: python-version: ${{ matrix.python-version }} poetry-version: ${{ env.POETRY_VERSION }} + - name: Stage public telemetry build configuration + env: + POSTHOG_PUBLIC_CUSTOMER_API_TOKEN: ${{ secrets.POSTHOG_PUBLIC_CUSTOMER_API_TOKEN }} + run: python3 scripts/embed_posthog_token.py --write + - name: Build kickstart binary run: make binary - name: Name, smoke-test, and archive binary env: + EXPECT_EMBEDDED_TELEMETRY: "1" PLATFORM: ${{ matrix.platform }} PYTHON_MINOR: ${{ matrix.python-version }} run: scripts/ci/name-and-smoke-binary.sh "${PLATFORM}" "${PYTHON_MINOR}" + - name: Clear staged public telemetry build configuration + if: ${{ always() }} + run: python3 scripts/embed_posthog_token.py --clear + - name: Upload binary artifact uses: actions/upload-artifact@v4 with: @@ -206,6 +238,8 @@ jobs: pip install ./kickstart-*-py3-none-any.whl ``` + Release wheels and binary archives contain the public PostHog capture configuration. A direct Git source install does not; opted-in telemetry from that installation requires `POSTHOG_PUBLIC_CUSTOMER_API_TOKEN` in the process environment. + ### Manual binary install Binary archives are attached for `linux-x64`, `linux-arm64`, and `macos-arm64` (Python 3.14). Pick the asset for your platform, extract it, and run `kickstart install` to place the binary payload in a user-writable directory and configure `PATH`: diff --git a/.gitignore b/.gitignore index 5cf5fb8..7a5beb8 100644 --- a/.gitignore +++ b/.gitignore @@ -3,6 +3,8 @@ __pycache__/ *.log .venv/ /.env +/.env.* +!/.env.example .mypy_cache/ .claude/settings.local.json /kickstart diff --git a/README.md b/README.md index 12f68a7..635fbd3 100644 --- a/README.md +++ b/README.md @@ -63,6 +63,8 @@ pipx install git+https://github.com/woud420/kickstart # from source pip install ./kickstart--py3-none-any.whl # wheel from the Releases page ``` +Release wheels and binary archives contain the public PostHog capture configuration. A direct Git source install does not; opted-in telemetry from that installation requires `POSTHOG_PUBLIC_CUSTOMER_API_TOKEN` in the process environment. + ## Get Started ```bash diff --git a/docs/contracts/telemetry.md b/docs/contracts/telemetry.md index de6b054..d29e010 100644 --- a/docs/contracts/telemetry.md +++ b/docs/contracts/telemetry.md @@ -106,7 +106,9 @@ Every PostHog event sets `$process_person_profile` to `false`. The adapter never Direct public capture can be spoofed. Provider, destination, or token changes require upgraded direct clients, and there is no first-party server kill switch in phase one. A later first-party relay may replace the sink without changing event producers, consent, or identity. -For local development, `POSTHOG_PUBLIC_CUSTOMER_API_TOKEN` may be exported from an ignored `.env` into the process. The CLI reads only the already-exported process environment and never auto-loads a current repository's `.env`, because kickstart runs inside arbitrary repositories and repository content must not influence telemetry routing or consent. A missing or malformed token silently selects a no-op sink; token presence never enables telemetry. The numeric `POSTHOG_PROJECT_ID` is not read and is not part of capture payloads. +Official wheel, source-distribution, and standalone-binary artifacts embed the public capture token from the `POSTHOG_PUBLIC_CUSTOMER_API_TOKEN` GitHub Actions secret during trusted build jobs. The secret prevents accidental disclosure in source and build logs; it does not make the token confidential after publication. Anyone can recover a public token from a distributed client, and token rotation requires newly built clients. + +For local development and source installations, `POSTHOG_PUBLIC_CUSTOMER_API_TOKEN` may be exported from an ignored `.env` into the process. An explicit process value overrides artifact configuration, including failing closed when that override is malformed. The CLI reads only the already-exported process environment and never auto-loads a current repository's `.env`, because kickstart runs inside arbitrary repositories and repository content must not influence telemetry routing or consent. A missing artifact token and missing or malformed runtime token silently select a no-op sink; token presence never enables telemetry. The numeric `POSTHOG_PROJECT_ID` is not read, embedded, or included in capture payloads. ## Delivery and retention diff --git a/docs/install-binaries.md b/docs/install-binaries.md index 17b7787..17772d0 100644 --- a/docs/install-binaries.md +++ b/docs/install-binaries.md @@ -12,6 +12,11 @@ Each platform is built for Python `3.14`. Platforms outside this matrix wheel attached to each GitHub Release. Do not `pip install kickstart`: the PyPI name belongs to an unrelated, abandoned 2011 project. +Release wheels and binary archives contain the public PostHog capture +configuration. Direct Git source installs do not; opted-in telemetry from a +source installation requires `POSTHOG_PUBLIC_CUSTOMER_API_TOKEN` in the +process environment. + ## Asset Names ```text diff --git a/scripts/ci/name-and-smoke-binary.sh b/scripts/ci/name-and-smoke-binary.sh index f6249a0..77b4978 100755 --- a/scripts/ci/name-and-smoke-binary.sh +++ b/scripts/ci/name-and-smoke-binary.sh @@ -34,8 +34,16 @@ mv "${DIST_DIR}/kickstart" "${payload}" # Verify the telemetry controls are bundled and status remains read-only. mkdir -p "${SMOKE_ROOT}" telemetry_config="$(mktemp -d "${SMOKE_ROOT}/telemetry-smoke.XXXXXX")" -XDG_CONFIG_HOME="${telemetry_config}" KICKSTART_TELEMETRY_DISABLED=1 \ - "${payload}/kickstart" telemetry status --json +telemetry_status="$( + env -u POSTHOG_PUBLIC_CUSTOMER_API_TOKEN \ + XDG_CONFIG_HOME="${telemetry_config}" KICKSTART_TELEMETRY_DISABLED=1 \ + "${payload}/kickstart" telemetry status --json +)" +printf '%s\n' "${telemetry_status}" +if [[ "${EXPECT_EMBEDDED_TELEMETRY:-0}" == "1" ]]; then + printf '%s\n' "${telemetry_status}" | python3 -c \ + 'import json, sys; raise SystemExit(0 if json.load(sys.stdin)["delivery_configured"] is True else 1)' +fi test ! -e "${telemetry_config}/kickstart/telemetry.json" rm -rf "${telemetry_config}" diff --git a/scripts/embed_posthog_token.py b/scripts/embed_posthog_token.py new file mode 100644 index 0000000..30e609e --- /dev/null +++ b/scripts/embed_posthog_token.py @@ -0,0 +1,85 @@ +"""Validate, stage, or clear the public PostHog token used in release artifacts.""" + +from __future__ import annotations + +import argparse +import json +import os +from pathlib import Path +from typing import Final + + +POSTHOG_PROJECT_TOKEN_ENV: Final[str] = "POSTHOG_PUBLIC_CUSTOMER_API_TOKEN" +DEFAULT_TARGET: Final[Path] = Path(__file__).parents[1] / "src" / "telemetry" / "_build_config.py" + + +def is_public_project_token(value: str) -> bool: + """Return whether a value is a non-empty public PostHog capture token.""" + return value.startswith("phc_") and len(value) > len("phc_") + + +def render_build_config(project_token: str) -> str: + """Render a Python module without interpolating unescaped configuration.""" + if not is_public_project_token(project_token): + raise ValueError("expected a non-empty public PostHog phc_ project token") + return ( + '"""Public telemetry configuration populated only while building release artifacts."""\n\n' + "from typing import Final\n\n\n" + f"EMBEDDED_POSTHOG_PROJECT_TOKEN: Final[str] = {json.dumps(project_token)}\n" + ) + + +def write_build_config(target: Path, project_token: str) -> None: + """Write the validated public token into the module packaged by Poetry and PyInstaller.""" + target.write_text(render_build_config(project_token), encoding="utf-8") + + +def clear_build_config(target: Path) -> None: + """Restore the checked-in tokenless build configuration.""" + target.write_text( + '"""Public telemetry configuration populated only while building release artifacts."""\n\n' + "from typing import Final\n\n\n" + 'EMBEDDED_POSTHOG_PROJECT_TOKEN: Final[str] = ""\n', + encoding="utf-8", + ) + + +def _parser() -> argparse.ArgumentParser: + parser = argparse.ArgumentParser(description=__doc__) + mode = parser.add_mutually_exclusive_group(required=True) + mode.add_argument("--check", action="store_true", help="Validate the process environment without writing.") + mode.add_argument("--write", action="store_true", help="Stage the token in the packaged build module.") + mode.add_argument("--clear", action="store_true", help="Restore the tokenless build module.") + parser.add_argument("--target", type=Path, default=DEFAULT_TARGET) + parser.add_argument( + "--allow-local", + action="store_true", + help="Permit --write outside GitHub Actions for an explicit local artifact test.", + ) + return parser + + +def main() -> int: + """Run the selected build-configuration operation without printing the token.""" + args = _parser().parse_args() + if args.clear: + clear_build_config(args.target) + print("Cleared staged public PostHog build configuration.") + return 0 + + if args.write and os.environ.get("GITHUB_ACTIONS") != "true" and not args.allow_local: + raise SystemExit("--write is restricted to GitHub Actions; pass --allow-local for an explicit local build") + + project_token = os.environ.get(POSTHOG_PROJECT_TOKEN_ENV, "").strip() + if not is_public_project_token(project_token): + raise SystemExit(f"{POSTHOG_PROJECT_TOKEN_ENV} must contain a non-empty public phc_ project token") + if args.write: + write_build_config(args.target, project_token) + print("Staged public PostHog configuration for artifact builds.") + else: + print("Public PostHog artifact-build configuration is valid.") + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/scripts/verify_embedded_telemetry_artifact.py b/scripts/verify_embedded_telemetry_artifact.py new file mode 100644 index 0000000..aa85808 --- /dev/null +++ b/scripts/verify_embedded_telemetry_artifact.py @@ -0,0 +1,92 @@ +"""Verify package artifacts contain a configured public PostHog capture token.""" + +from __future__ import annotations + +import argparse +import ast +import os +import tarfile +import zipfile +from pathlib import Path +from typing import Final + +from scripts.embed_posthog_token import is_public_project_token + + +BUILD_CONFIG_SUFFIX: Final[str] = "src/telemetry/_build_config.py" +TOKEN_NAME: Final[str] = "EMBEDDED_POSTHOG_PROJECT_TOKEN" + + +def _source_from_wheel(path: Path) -> str: + with zipfile.ZipFile(path) as archive: + names = [name for name in archive.namelist() if name.endswith(BUILD_CONFIG_SUFFIX)] + if len(names) != 1: + raise ValueError("wheel must contain exactly one telemetry build configuration") + return archive.read(names[0]).decode("utf-8") + + +def _source_from_sdist(path: Path) -> str: + with tarfile.open(path, mode="r:gz") as archive: + members = [member for member in archive.getmembers() if member.name.endswith(BUILD_CONFIG_SUFFIX)] + if len(members) != 1: + raise ValueError("sdist must contain exactly one telemetry build configuration") + extracted = archive.extractfile(members[0]) + if extracted is None: + raise ValueError("could not read telemetry build configuration from sdist") + return extracted.read().decode("utf-8") + + +def embedded_token_from_artifact(path: Path) -> str: + """Read the embedded token from a wheel or gzipped source distribution.""" + if path.suffix == ".whl": + source = _source_from_wheel(path) + elif path.name.endswith(".tar.gz"): + source = _source_from_sdist(path) + else: + raise ValueError("expected a .whl or .tar.gz package artifact") + + module = ast.parse(source) + for node in module.body: + if ( + isinstance(node, ast.AnnAssign) + and isinstance(node.target, ast.Name) + and node.target.id == TOKEN_NAME + and node.value is not None + ): + value = ast.literal_eval(node.value) + if not isinstance(value, str): + break + return value + raise ValueError("telemetry build configuration does not define the embedded token") + + +def verify_artifact(path: Path, *, expected_project_token: str | None = None) -> None: + """Fail when an artifact lacks a non-empty public capture token.""" + embedded_project_token = embedded_token_from_artifact(path) + if not is_public_project_token(embedded_project_token): + raise ValueError("artifact does not contain a non-empty public phc_ project token") + if expected_project_token is not None and embedded_project_token != expected_project_token: + raise ValueError("artifact capture token does not match the configured PostHog project") + + +def main() -> int: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument( + "--expected-token-env", + help="Environment variable containing the expected token; values are compared but never printed.", + ) + parser.add_argument("artifacts", nargs="+", type=Path) + args = parser.parse_args() + expected_project_token = None + if args.expected_token_env is not None: + expected_project_token = os.environ.get(args.expected_token_env, "").strip() + if not is_public_project_token(expected_project_token): + raise SystemExit(f"{args.expected_token_env} must contain a non-empty public phc_ project token") + for artifact in args.artifacts: + verify_artifact(artifact, expected_project_token=expected_project_token) + print(f"{artifact.name}: embedded public PostHog configuration verified") + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/src/cli/telemetry.py b/src/cli/telemetry.py index 64a703f..ddbaea4 100644 --- a/src/cli/telemetry.py +++ b/src/cli/telemetry.py @@ -5,6 +5,7 @@ import typer from src.model.dto.telemetry import EffectiveTelemetry, TelemetryState, TelemetrySuppressionReason +from src.telemetry.config import posthog_settings_from_configuration from src.telemetry.policy import resolve_telemetry from src.telemetry.state import TelemetryStateStore from src.utils.errors import TelemetryStateError @@ -21,7 +22,7 @@ def _state_store() -> TelemetryStateStore: def telemetry_status( json_output: bool = typer.Option(False, "--json", help="Emit machine-readable telemetry status."), ) -> None: - """Show persisted consent, effective status, identity, and state location.""" + """Show persisted consent, delivery configuration, identity, and state location.""" store = _state_store() try: state = store.read() @@ -38,6 +39,7 @@ def telemetry_status( payload = { "anonymous_id": str(state.anonymous_id) if state.anonymous_id is not None else None, "consent": consent, + "delivery_configured": posthog_settings_from_configuration() is not None, "effective": effective.enabled, "reason": effective.reason.value, "state_file": str(store.path), @@ -46,6 +48,7 @@ def telemetry_status( typer.echo(json.dumps(payload, sort_keys=True)) return typer.echo(f"consent: {payload['consent']}") + typer.echo(f"delivery-configured: {'yes' if payload['delivery_configured'] else 'no'}") typer.echo(f"effective: {'enabled' if effective.enabled else 'disabled'}") typer.echo(f"reason: {payload['reason']}") typer.echo(f"anonymous-id: {payload['anonymous_id'] or 'not-created'}") diff --git a/src/telemetry/_build_config.py b/src/telemetry/_build_config.py new file mode 100644 index 0000000..b04771a --- /dev/null +++ b/src/telemetry/_build_config.py @@ -0,0 +1,6 @@ +"""Public telemetry configuration populated only while building release artifacts.""" + +from typing import Final + + +EMBEDDED_POSTHOG_PROJECT_TOKEN: Final[str] = "" diff --git a/src/telemetry/config.py b/src/telemetry/config.py index adf8403..a980fde 100644 --- a/src/telemetry/config.py +++ b/src/telemetry/config.py @@ -4,6 +4,8 @@ from collections.abc import Mapping from dataclasses import dataclass, field +from src.telemetry._build_config import EMBEDDED_POSTHOG_PROJECT_TOKEN + POSTHOG_PROJECT_TOKEN_ENV = "POSTHOG_PUBLIC_CUSTOMER_API_TOKEN" POSTHOG_US_CAPTURE_ENDPOINT = "https://us.i.posthog.com/i/v0/e/" @@ -25,10 +27,15 @@ def __post_init__(self) -> None: raise ValueError("PostHog timeout must be positive") -def posthog_settings_from_environment(environ: Mapping[str, str] | None = None) -> PostHogSettings | None: - """Read a public capture token from the process environment, never a project file.""" +def posthog_settings_from_configuration( + environ: Mapping[str, str] | None = None, + *, + embedded_project_token: str = EMBEDDED_POSTHOG_PROJECT_TOKEN, +) -> PostHogSettings | None: + """Resolve an explicit runtime override or the token embedded in release artifacts.""" process_environment = os.environ if environ is None else environ - project_token = process_environment.get(POSTHOG_PROJECT_TOKEN_ENV, "").strip() + runtime_project_token = process_environment.get(POSTHOG_PROJECT_TOKEN_ENV) + project_token = embedded_project_token.strip() if runtime_project_token is None else runtime_project_token.strip() if not project_token.startswith("phc_") or len(project_token) == len("phc_"): return None return PostHogSettings(project_token=project_token) diff --git a/src/telemetry/reporter.py b/src/telemetry/reporter.py index 14a65af..1425464 100644 --- a/src/telemetry/reporter.py +++ b/src/telemetry/reporter.py @@ -7,7 +7,7 @@ from uuid import UUID, uuid4 from src.model.dto.telemetry import TelemetryEnvelope, TelemetryEvent -from src.telemetry.config import posthog_settings_from_environment +from src.telemetry.config import posthog_settings_from_configuration from src.telemetry.policy import resolve_telemetry from src.telemetry.posthog import PostHogSink from src.telemetry.sink import NoOpTelemetrySink, TelemetrySink @@ -58,7 +58,7 @@ def default_telemetry_reporter( ) -> TelemetryReporter: """Build the default reporter without reading cwd configuration or dotenv files.""" process_environment = os.environ if environ is None else environ - settings = posthog_settings_from_environment(process_environment) + settings = posthog_settings_from_configuration(process_environment) sink: TelemetrySink = NoOpTelemetrySink() if settings is None else PostHogSink(settings) return TelemetryReporter( state_store=TelemetryStateStore.from_environment(process_environment), diff --git a/tests/unit/cli/test_telemetry_cli.py b/tests/unit/cli/test_telemetry_cli.py index 9a29000..6c8262a 100644 --- a/tests/unit/cli/test_telemetry_cli.py +++ b/tests/unit/cli/test_telemetry_cli.py @@ -7,7 +7,11 @@ def _env(tmp_path: Path) -> dict[str, str]: - return {"XDG_CONFIG_HOME": str(tmp_path), "HOME": str(tmp_path)} + return { + "HOME": str(tmp_path), + "POSTHOG_PUBLIC_CUSTOMER_API_TOKEN": "", + "XDG_CONFIG_HOME": str(tmp_path), + } def test_status_is_read_only_before_opt_in(tmp_path: Path) -> None: @@ -18,11 +22,24 @@ def test_status_is_read_only_before_opt_in(tmp_path: Path) -> None: assert result.exit_code == 0 payload = json.loads(result.stdout) assert payload["consent"] == "unset" + assert payload["delivery_configured"] is False assert payload["effective"] is False assert payload["anonymous_id"] is None assert not (tmp_path / "kickstart" / "telemetry.json").exists() +def test_status_reports_runtime_delivery_configuration_without_exposing_token(tmp_path: Path) -> None: + environment = _env(tmp_path) + environment["POSTHOG_PUBLIC_CUSTOMER_API_TOKEN"] = "phc_status_test_token" + + result = CliRunner().invoke(app, ["telemetry", "status", "--json"], env=environment) + + assert result.exit_code == 0 + payload = json.loads(result.stdout) + assert payload["delivery_configured"] is True + assert "phc_status_test_token" not in result.stdout + + def test_enable_disable_and_status_preserve_identity(tmp_path: Path) -> None: runner = CliRunner() environment = _env(tmp_path) diff --git a/tests/unit/scripts/test_embed_posthog_token.py b/tests/unit/scripts/test_embed_posthog_token.py new file mode 100644 index 0000000..79efc96 --- /dev/null +++ b/tests/unit/scripts/test_embed_posthog_token.py @@ -0,0 +1,61 @@ +import os +import subprocess +import sys +from pathlib import Path + +import pytest + +from scripts.embed_posthog_token import clear_build_config, render_build_config, write_build_config + + +def test_render_build_config_rejects_non_capture_credentials() -> None: + with pytest.raises(ValueError, match="public PostHog phc_"): + render_build_config("phx_personal_key") + + +def test_write_and_clear_build_config(tmp_path: Path) -> None: + target = tmp_path / "_build_config.py" + + write_build_config(target, 'phc_public_token_with_"quotes"') + + written = target.read_text(encoding="utf-8") + assert 'phc_public_token_with_\\"quotes\\"' in written + clear_build_config(target) + assert 'EMBEDDED_POSTHOG_PROJECT_TOKEN: Final[str] = ""' in target.read_text(encoding="utf-8") + + +def test_cli_does_not_print_the_token(tmp_path: Path) -> None: + token = "phc_do_not_print_this_value" + target = tmp_path / "_build_config.py" + environment = os.environ.copy() + environment["POSTHOG_PUBLIC_CUSTOMER_API_TOKEN"] = token + + result = subprocess.run( + [sys.executable, "scripts/embed_posthog_token.py", "--write", "--allow-local", "--target", str(target)], + check=False, + capture_output=True, + text=True, + env=environment, + ) + + assert result.returncode == 0 + assert token not in result.stdout + assert token not in result.stderr + assert token in target.read_text(encoding="utf-8") + + +def test_cli_refuses_to_stage_into_a_local_checkout_without_explicit_override(tmp_path: Path) -> None: + environment = os.environ.copy() + environment.pop("GITHUB_ACTIONS", None) + environment["POSTHOG_PUBLIC_CUSTOMER_API_TOKEN"] = "phc_local_test_token" + + result = subprocess.run( + [sys.executable, "scripts/embed_posthog_token.py", "--write", "--target", str(tmp_path / "config.py")], + check=False, + capture_output=True, + text=True, + env=environment, + ) + + assert result.returncode != 0 + assert "pass --allow-local" in result.stderr diff --git a/tests/unit/scripts/test_verify_embedded_telemetry_artifact.py b/tests/unit/scripts/test_verify_embedded_telemetry_artifact.py new file mode 100644 index 0000000..95ce034 --- /dev/null +++ b/tests/unit/scripts/test_verify_embedded_telemetry_artifact.py @@ -0,0 +1,50 @@ +import io +import tarfile +import zipfile +from pathlib import Path + +import pytest + +from scripts.embed_posthog_token import render_build_config +from scripts.verify_embedded_telemetry_artifact import embedded_token_from_artifact, verify_artifact + + +def test_verify_wheel_with_embedded_token(tmp_path: Path) -> None: + artifact = tmp_path / "kickstart-test.whl" + with zipfile.ZipFile(artifact, mode="w") as archive: + archive.writestr("src/telemetry/_build_config.py", render_build_config("phc_wheel_test_token")) + + verify_artifact(artifact) + + assert embedded_token_from_artifact(artifact) == "phc_wheel_test_token" + + +def test_verify_sdist_with_embedded_token(tmp_path: Path) -> None: + artifact = tmp_path / "kickstart-test.tar.gz" + content = render_build_config("phc_sdist_test_token").encode() + member = tarfile.TarInfo("kickstart-test/src/telemetry/_build_config.py") + member.size = len(content) + with tarfile.open(artifact, mode="w:gz") as archive: + archive.addfile(member, io.BytesIO(content)) + + verify_artifact(artifact) + + +def test_verify_rejects_empty_placeholder(tmp_path: Path) -> None: + artifact = tmp_path / "kickstart-test.whl" + with zipfile.ZipFile(artifact, mode="w") as archive: + archive.writestr( + "src/telemetry/_build_config.py", render_build_config("phc_fixture").replace("phc_fixture", "") + ) + + with pytest.raises(ValueError, match="does not contain"): + verify_artifact(artifact) + + +def test_verify_rejects_a_token_from_another_posthog_project(tmp_path: Path) -> None: + artifact = tmp_path / "kickstart-test.whl" + with zipfile.ZipFile(artifact, mode="w") as archive: + archive.writestr("src/telemetry/_build_config.py", render_build_config("phc_wrong_project_token")) + + with pytest.raises(ValueError, match="configured PostHog project"): + verify_artifact(artifact, expected_project_token="phc_expected_project_token") diff --git a/tests/unit/telemetry/test_telemetry_config.py b/tests/unit/telemetry/test_telemetry_config.py index 9a3001d..435879d 100644 --- a/tests/unit/telemetry/test_telemetry_config.py +++ b/tests/unit/telemetry/test_telemetry_config.py @@ -2,11 +2,15 @@ import pytest -from src.telemetry.config import POSTHOG_US_CAPTURE_ENDPOINT, PostHogSettings, posthog_settings_from_environment +from src.telemetry.config import ( + POSTHOG_US_CAPTURE_ENDPOINT, + PostHogSettings, + posthog_settings_from_configuration, +) def test_environment_config_accepts_only_public_capture_token() -> None: - settings = posthog_settings_from_environment( + settings = posthog_settings_from_configuration( { "POSTHOG_PUBLIC_CUSTOMER_API_TOKEN": "phc_public_test_token", "POSTHOG_PROJECT_ID": "123456", @@ -19,15 +23,15 @@ def test_environment_config_accepts_only_public_capture_token() -> None: def test_project_id_alone_does_not_configure_capture() -> None: - assert posthog_settings_from_environment({"POSTHOG_PROJECT_ID": "123456"}) is None + assert posthog_settings_from_configuration({"POSTHOG_PROJECT_ID": "123456"}) is None def test_personal_api_key_is_rejected() -> None: - assert posthog_settings_from_environment({"POSTHOG_PUBLIC_CUSTOMER_API_TOKEN": "phx_secret"}) is None + assert posthog_settings_from_configuration({"POSTHOG_PUBLIC_CUSTOMER_API_TOKEN": "phx_secret"}) is None def test_empty_public_token_body_is_rejected() -> None: - assert posthog_settings_from_environment({"POSTHOG_PUBLIC_CUSTOMER_API_TOKEN": "phc_"}) is None + assert posthog_settings_from_configuration({"POSTHOG_PUBLIC_CUSTOMER_API_TOKEN": "phc_"}) is None def test_settings_repr_never_contains_project_token() -> None: @@ -40,4 +44,32 @@ def test_configuration_never_auto_loads_cwd_dotenv(tmp_path: Path, monkeypatch: (tmp_path / ".env").write_text("POSTHOG_PUBLIC_CUSTOMER_API_TOKEN=phc_untrusted_project_value\n", encoding="utf-8") monkeypatch.chdir(tmp_path) - assert posthog_settings_from_environment({}) is None + assert posthog_settings_from_configuration({}) is None + + +def test_embedded_project_token_is_used_without_runtime_override() -> None: + settings = posthog_settings_from_configuration({}, embedded_project_token="phc_embedded_test_token") + + assert settings is not None + assert settings.project_token == "phc_embedded_test_token" + + +def test_runtime_project_token_overrides_embedded_configuration() -> None: + settings = posthog_settings_from_configuration( + {"POSTHOG_PUBLIC_CUSTOMER_API_TOKEN": "phc_runtime_test_token"}, + embedded_project_token="phc_embedded_test_token", + ) + + assert settings is not None + assert settings.project_token == "phc_runtime_test_token" + + +@pytest.mark.parametrize("runtime_project_token", ["", " ", "phc_", "phx_not_a_capture_token"]) +def test_invalid_explicit_runtime_override_fails_closed_instead_of_falling_back(runtime_project_token: str) -> None: + assert ( + posthog_settings_from_configuration( + {"POSTHOG_PUBLIC_CUSTOMER_API_TOKEN": runtime_project_token}, + embedded_project_token="phc_embedded_test_token", + ) + is None + ) From cca5bd7467fb5b459ec274f9712193e54bad6f24 Mon Sep 17 00:00:00 2001 From: Jean-Michel Bouchard Date: Sun, 19 Jul 2026 16:43:27 -0400 Subject: [PATCH 06/11] Cover telemetry failure paths --- tests/unit/cli/test_create_telemetry.py | 17 +++++++ tests/unit/cli/test_telemetry_cli.py | 44 ++++++++++++++++++ tests/unit/telemetry/test_events.py | 17 +++++++ tests/unit/telemetry/test_policy.py | 9 ++++ tests/unit/telemetry/test_state.py | 46 ++++++++++++++++++- tests/unit/telemetry/test_telemetry_config.py | 11 +++++ 6 files changed, 143 insertions(+), 1 deletion(-) diff --git a/tests/unit/cli/test_create_telemetry.py b/tests/unit/cli/test_create_telemetry.py index 2cccf98..6a10f8e 100644 --- a/tests/unit/cli/test_create_telemetry.py +++ b/tests/unit/cli/test_create_telemetry.py @@ -61,6 +61,23 @@ def test_expected_creation_failure_preserves_exit_and_attempts_one_terminal_even assert private_error not in repr(context) +def test_unexpected_creation_failure_attempts_one_terminal_event() -> None: + runner = CliRunner() + with ( + patch("src.cli.main.load_config", return_value={}), + patch("src.cli.main.create_service", side_effect=RuntimeError("synthetic unexpected failure")), + patch("src.cli.main.capture_scaffold_create_terminal") as capture, + ): + result = runner.invoke(app, ["create", "service", "private-project-name", "--root", "/tmp"]) + + assert result.exit_code == 1 + capture.assert_called_once() + assert capture.call_args.args[1:3] == ( + ScaffoldCreateOutcome.UNEXPECTED_ERROR, + ScaffoldCreateErrorCategory.UNEXPECTED_ERROR, + ) + + def test_keyboard_interrupt_preserves_exit_130_and_attempts_one_terminal_event() -> None: runner = CliRunner() with ( diff --git a/tests/unit/cli/test_telemetry_cli.py b/tests/unit/cli/test_telemetry_cli.py index 6c8262a..5ed0b1a 100644 --- a/tests/unit/cli/test_telemetry_cli.py +++ b/tests/unit/cli/test_telemetry_cli.py @@ -1,9 +1,12 @@ import json from pathlib import Path +import pytest from typer.testing import CliRunner from src.cli.main import app +from src.telemetry.state import TelemetryStateStore +from src.utils.errors import TelemetryStateError def _env(tmp_path: Path) -> dict[str, str]: @@ -40,6 +43,18 @@ def test_status_reports_runtime_delivery_configuration_without_exposing_token(tm assert "phc_status_test_token" not in result.stdout +def test_status_supports_human_readable_output(tmp_path: Path) -> None: + result = CliRunner().invoke(app, ["telemetry", "status"], env=_env(tmp_path)) + + assert result.exit_code == 0 + assert "consent: unset" in result.stdout + assert "delivery-configured: no" in result.stdout + assert "effective: disabled" in result.stdout + assert "reason:" in result.stdout + assert "anonymous-id: not-created" in result.stdout + assert f"state-file: {tmp_path / 'kickstart' / 'telemetry.json'}" in result.stdout + + def test_enable_disable_and_status_preserve_identity(tmp_path: Path) -> None: runner = CliRunner() environment = _env(tmp_path) @@ -77,6 +92,35 @@ def test_disable_before_enable_does_not_create_an_identity(tmp_path: Path) -> No assert "anonymous-id: not-created" in result.stdout +def test_reset_before_enable_is_a_read_only_noop(tmp_path: Path) -> None: + result = CliRunner().invoke(app, ["telemetry", "reset-id"], env=_env(tmp_path)) + + assert result.exit_code == 0 + assert "No telemetry ID exists; nothing was changed." in result.stdout + assert not (tmp_path / "kickstart" / "telemetry.json").exists() + + +@pytest.mark.parametrize( + ("command", "method_name"), + [("enable", "enable"), ("disable", "disable"), ("reset-id", "reset_id")], +) +def test_mutating_commands_report_state_errors( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, + command: str, + method_name: str, +) -> None: + def fail(_store: TelemetryStateStore) -> None: + raise TelemetryStateError("synthetic state failure") + + monkeypatch.setattr(TelemetryStateStore, method_name, fail) + + result = CliRunner().invoke(app, ["telemetry", command], env=_env(tmp_path)) + + assert result.exit_code == 1 + assert "Telemetry state error: synthetic state failure" in result.stderr + + def test_malformed_state_reports_fail_closed_json_status(tmp_path: Path) -> None: state_path = tmp_path / "kickstart" / "telemetry.json" state_path.parent.mkdir() diff --git a/tests/unit/telemetry/test_events.py b/tests/unit/telemetry/test_events.py index 405f9d0..0e2047f 100644 --- a/tests/unit/telemetry/test_events.py +++ b/tests/unit/telemetry/test_events.py @@ -1,4 +1,5 @@ from pathlib import Path +from unittest.mock import Mock import pytest @@ -283,3 +284,19 @@ def test_capture_helper_records_exactly_one_event_for_enabled_reporter(tmp_path: assert len(sink.envelopes) == 1 assert sink.envelopes[0].event.properties.outcome is ScaffoldCreateOutcome.SUCCESS + + +def test_capture_helper_contains_unexpected_reporter_failures() -> None: + reporter = Mock(spec=TelemetryReporter) + reporter.record.side_effect = RuntimeError("synthetic reporter failure") + + capture_scaffold_create_terminal( + _service_context(), + ScaffoldCreateOutcome.SUCCESS, + ScaffoldCreateErrorCategory.NONE, + 0.25, + cli_version="0.4.3", + reporter=reporter, + ) + + reporter.record.assert_called_once() diff --git a/tests/unit/telemetry/test_policy.py b/tests/unit/telemetry/test_policy.py index 7a1f380..addfa5d 100644 --- a/tests/unit/telemetry/test_policy.py +++ b/tests/unit/telemetry/test_policy.py @@ -69,6 +69,15 @@ def test_source_checkout_is_disabled_without_explicit_development_override(tmp_p assert enabled.enabled is True +def test_source_checkout_is_detected_by_default(tmp_path: Path) -> None: + store = TelemetryStateStore(tmp_path / "telemetry.json") + store.enable() + + resolved = resolve_telemetry(store, {}) + + assert resolved.reason is TelemetrySuppressionReason.DEVELOPMENT + + def test_malformed_state_fails_closed(tmp_path: Path) -> None: path = tmp_path / "telemetry.json" path.write_text("{bad json", encoding="utf-8") diff --git a/tests/unit/telemetry/test_state.py b/tests/unit/telemetry/test_state.py index 33a3d0f..6714862 100644 --- a/tests/unit/telemetry/test_state.py +++ b/tests/unit/telemetry/test_state.py @@ -1,4 +1,5 @@ import json +import os import stat from concurrent.futures import ThreadPoolExecutor from pathlib import Path @@ -125,17 +126,60 @@ def test_state_write_succeeds_when_descriptor_chmod_is_unavailable( assert state.consent is TelemetryConsent.ENABLED +def test_stale_lock_is_removed_before_updating_state(tmp_path: Path) -> None: + path = tmp_path / "telemetry.json" + lock_path = tmp_path / ".telemetry.json.lock" + lock_path.mkdir() + os.utime(lock_path, (0, 0)) + + state = TelemetryStateStore(path).disable() + + assert state.consent is TelemetryConsent.DISABLED + assert not lock_path.exists() + + +def test_busy_lock_fails_without_changing_state(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + path = tmp_path / "telemetry.json" + lock_path = tmp_path / ".telemetry.json.lock" + lock_path.mkdir() + monkeypatch.setattr(state_module, "_LOCK_TIMEOUT_SECONDS", 0.0) + + with pytest.raises(TelemetryStateError, match="Telemetry state is busy"): + TelemetryStateStore(path).enable() + + assert not path.exists() + + +def test_write_failure_removes_temporary_state_file(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + path = tmp_path / "telemetry.json" + + def fail_replace(_source: Path, _destination: Path) -> None: + raise OSError("synthetic replace failure") + + monkeypatch.setattr(state_module.os, "replace", fail_replace) + + with pytest.raises(TelemetryStateError, match="cannot be written"): + TelemetryStateStore(path).enable() + + assert list(tmp_path.glob(".telemetry-*.tmp")) == [] + + @pytest.mark.parametrize( "document", [ + [], {}, {"schema_version": 2, "consent": "enabled", "anonymous_id": str(uuid4())}, {"schema_version": 1, "consent": "enabled"}, + {"schema_version": 1, "consent": 1}, + {"schema_version": 1, "consent": "unset"}, {"schema_version": 1, "consent": "enabled", "anonymous_id": "not-a-uuid"}, + {"schema_version": 1, "consent": "disabled", "anonymous_id": 1}, + {"schema_version": 1, "consent": "disabled", "anonymous_id": str(UUID(int=0, version=1))}, {"schema_version": 1, "consent": "enabled", "anonymous_id": str(uuid4()), "extra": True}, ], ) -def test_malformed_or_unknown_state_is_rejected(tmp_path: Path, document: dict[str, str | int | bool]) -> None: +def test_malformed_or_unknown_state_is_rejected(tmp_path: Path, document: object) -> None: path = tmp_path / "telemetry.json" path.write_text(json.dumps(document), encoding="utf-8") diff --git a/tests/unit/telemetry/test_telemetry_config.py b/tests/unit/telemetry/test_telemetry_config.py index 435879d..c665ecc 100644 --- a/tests/unit/telemetry/test_telemetry_config.py +++ b/tests/unit/telemetry/test_telemetry_config.py @@ -40,6 +40,17 @@ def test_settings_repr_never_contains_project_token() -> None: assert "phc_do_not_print" not in repr(settings) +@pytest.mark.parametrize("project_token", ["phx_personal_key", "phc_"]) +def test_settings_reject_invalid_project_tokens(project_token: str) -> None: + with pytest.raises(ValueError, match="public phc_ project token"): + PostHogSettings(project_token) + + +def test_settings_reject_nonpositive_timeout() -> None: + with pytest.raises(ValueError, match="timeout must be positive"): + PostHogSettings("phc_public_test_token", timeout_seconds=0) + + def test_configuration_never_auto_loads_cwd_dotenv(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: (tmp_path / ".env").write_text("POSTHOG_PUBLIC_CUSTOMER_API_TOKEN=phc_untrusted_project_value\n", encoding="utf-8") monkeypatch.chdir(tmp_path) From 0ac4f2158ef6e3a8a9c42a37cd03b7de9aac907c Mon Sep 17 00:00:00 2001 From: Jean-Michel Bouchard Date: Sun, 19 Jul 2026 17:03:19 -0400 Subject: [PATCH 07/11] Cover telemetry framework normalization --- tests/unit/telemetry/test_events.py | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/tests/unit/telemetry/test_events.py b/tests/unit/telemetry/test_events.py index 0e2047f..dccf8f8 100644 --- a/tests/unit/telemetry/test_events.py +++ b/tests/unit/telemetry/test_events.py @@ -1,3 +1,4 @@ +from dataclasses import replace from pathlib import Path from unittest.mock import Mock @@ -78,6 +79,27 @@ def test_service_event_canonicalizes_aliases_and_ignores_inapplicable_values() - } +@pytest.mark.parametrize( + ("framework", "expected"), + [(" FASTAPI ", "fastapi"), ("private-framework", "unknown")], +) +def test_service_framework_is_normalized_without_exposing_raw_values(framework: str, expected: str) -> None: + context = replace(_service_context(), language="python", framework=framework) + + mapping = build_scaffold_create_event( + context, + ScaffoldCreateOutcome.SUCCESS, + ScaffoldCreateErrorCategory.NONE, + 0.25, + cli_version="0.4.3", + platform_name="Linux", + architecture="x86_64", + ).properties.as_mapping() + + assert mapping["framework"] == expected + assert framework not in mapping.values() + + def test_legacy_monorepo_event_uses_system_defaults() -> None: context = ScaffoldCreateContext( project_type="monorepo", From 5235f753dbbf88ebbdf07fb2a7e6a0a6dc5f24d0 Mon Sep 17 00:00:00 2001 From: Jean-Michel Bouchard Date: Sun, 19 Jul 2026 17:59:00 -0400 Subject: [PATCH 08/11] Enable default-on telemetry and track CLI lifecycle events Make configured telemetry default-on with lazy, stable identity creation while preserving persistent and process-level opt-outs. Add closed install and upgrade outcome events, typed upgrade results, fail-closed symlink handling, and move the binary runbook under docs/operations.\n\nRefs ENG-197\nRefs ENG-198\nRefs ENG-200\nRefs ENG-201\nRefs ENG-202\nRefs ENG-203\n\nTests: make check (781 passed)\nCoverage: make coverage (95.8%) --- .github/workflows/ci.yml | 2 +- .github/workflows/release.yml | 4 +- README.md | 8 +- docs/README.md | 4 +- docs/contracts/README.md | 4 +- docs/contracts/telemetry.md | 83 ++++++++--- docs/contributing.md | 16 ++- docs/operations/README.md | 1 + docs/{ => operations}/install-binaries.md | 32 ++++- src/cli/main.py | 135 ++++++++++++++---- src/cli/telemetry.py | 4 +- src/model/dto/__init__.py | 20 +++ src/model/dto/telemetry.py | 133 +++++++++++++++++- src/model/dto/upgrade.py | 32 +++++ src/telemetry/events.py | 124 +++++++++++++++++ src/telemetry/policy.py | 16 +-- src/telemetry/reporter.py | 9 +- src/telemetry/state.py | 29 +++- src/utils/updater.py | 119 ++++++++++++++-- tests/unit/cli/test_main.py | 109 ++++++++++++++- tests/unit/cli/test_telemetry_cli.py | 21 ++- tests/unit/model/dto/test_posthog_dto.py | 42 ++++++ tests/unit/model/dto/test_telemetry_dto.py | 73 ++++++++++ tests/unit/telemetry/test_events.py | 155 +++++++++++++++++++++ tests/unit/telemetry/test_policy.py | 17 ++- tests/unit/telemetry/test_reporter.py | 103 ++++++++++++-- tests/unit/telemetry/test_state.py | 68 +++++++-- tests/unit/utils/test_updater.py | 139 +++++++++++++++++- 28 files changed, 1380 insertions(+), 122 deletions(-) rename docs/{ => operations}/install-binaries.md (80%) create mode 100644 src/model/dto/upgrade.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index fdd5133..0117dad 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -237,7 +237,7 @@ jobs: # schedules on arm64 hardware in this org, so PyInstaller fails with # IncompatibleBinaryArchError. This matches the release matrix, which # dropped macos-x64 in v0.4.1 (PR #66); x64 macOS installs from - # source or the release wheel (docs/install-binaries.md). + # source or the release wheel (docs/operations/install-binaries.md). - runner: macos-15 platform: macos-arm64 diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index abad857..e3a1290 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -238,7 +238,9 @@ jobs: pip install ./kickstart-*-py3-none-any.whl ``` - Release wheels and binary archives contain the public PostHog capture configuration. A direct Git source install does not; opted-in telemetry from that installation requires `POSTHOG_PUBLIC_CUSTOMER_API_TOKEN` in the process environment. + Release wheels and binary archives enable pseudonymous telemetry by default for eligible CLI commands. The first eligible event creates a random UUIDv4 that remains stable across upgrades. Events use closed, data-minimized property sets. Run `kickstart telemetry disable` to persist an opt-out; `kickstart telemetry status` is read-only and does not create an ID or send an event. + + To prevent an installer-invoked event before a persisted preference exists, set `KICKSTART_TELEMETRY_DISABLED=1` or `DO_NOT_TRACK=1` for that process. A direct Git source install has no embedded capture configuration; delivery from one requires `POSTHOG_PUBLIC_CUSTOMER_API_TOKEN` in the process environment, and a token never overrides an opt-out. ### Manual binary install diff --git a/README.md b/README.md index 635fbd3..32836a8 100644 --- a/README.md +++ b/README.md @@ -63,7 +63,13 @@ pipx install git+https://github.com/woud420/kickstart # from source pip install ./kickstart--py3-none-any.whl # wheel from the Releases page ``` -Release wheels and binary archives contain the public PostHog capture configuration. A direct Git source install does not; opted-in telemetry from that installation requires `POSTHOG_PUBLIC_CUSTOMER_API_TOKEN` in the process environment. +### Telemetry + +Release wheels and binary archives enable pseudonymous telemetry by default for eligible CLI commands. The first eligible event creates a random UUIDv4 in the user configuration directory; it remains stable across CLI upgrades. Events use closed, data-minimized property sets and never include project names, paths, command arguments, generated content, or arbitrary error text. + +Run `kickstart telemetry disable` to persist an opt-out. `kickstart telemetry status` is read-only and does not create an ID or send an event. To prevent an installer-invoked event before that preference exists, set `KICKSTART_TELEMETRY_DISABLED=1` or `DO_NOT_TRACK=1` for the install process; the binary installation runbook includes exact examples. + +A direct Git source install has no embedded capture configuration; delivery from one requires `POSTHOG_PUBLIC_CUSTOMER_API_TOKEN` in the process environment. A token never overrides an opt-out. See the [telemetry contract](docs/contracts/telemetry.md) and [binary installation runbook](docs/operations/install-binaries.md) for the exact event allowlist and controls. ## Get Started diff --git a/docs/README.md b/docs/README.md index 24b15bd..88b301c 100644 --- a/docs/README.md +++ b/docs/README.md @@ -5,7 +5,7 @@ - [Agent Map](../AGENTS.md): repo-local map for agents modifying kickstart itself. - [Architecture](architecture/README.md): canonical architecture entrypoint for kickstart itself. - [Contracts](contracts/README.md): public and generated-output contracts. -- [Pseudonymous CLI Telemetry](contracts/telemetry.md): default-off consent, identity, and data-minimization contract. +- [Pseudonymous CLI Telemetry](contracts/telemetry.md): default-on policy, persistent opt-out, identity, lifecycle events, and data-minimization contract. - [Scaffold Contract](scaffold-contract.md): generated docs, metadata, and option vocabulary. - [Decisions](decisions/README.md): durable project decisions. - [CLI Framework Research](decisions/cli-framework-research.md): sourced recommendation for Rust, Python, and TypeScript CLI framework defaults. @@ -14,6 +14,6 @@ - [Contributing](contributing.md): human contribution workflow. - [Agent Contributing](agent-contributing.md): agent-safe development workflow. - [Release Policy](release-policy.md): version tags, release assets, and same-version updates. -- [Install Binaries](install-binaries.md): release asset names and install commands. +- [Install Binaries](operations/install-binaries.md): release asset names, install commands, and first-run telemetry controls. Supported Python range: `>=3.12,<3.15`. diff --git a/docs/contracts/README.md b/docs/contracts/README.md index a36a451..c6a3017 100644 --- a/docs/contracts/README.md +++ b/docs/contracts/README.md @@ -8,8 +8,8 @@ Current contract references: - [Plan Command Drift Scope](plan-drift-scope.md): what `kickstart plan` covers, defers, and emits. - [Adoption Tiers](adoption-tiers.md): Level 1 conformant vs Level 2 managed, exit codes, and JSON shape. - [Backstage Export](backstage-export.md): derived/declared/passthrough field classes and re-export semantics. -- [Pseudonymous CLI Telemetry](telemetry.md): consent, identity, event allowlist, privacy, and provider boundaries. -- [Install Binaries](../install-binaries.md): release asset names and install commands. +- [Pseudonymous CLI Telemetry](telemetry.md): default-on policy, persistent opt-out, identity, closed event allowlists, privacy, and provider boundaries. +- [Install Binaries](../operations/install-binaries.md): release asset names, install commands, and first-run telemetry controls. - [Human Guide](../human-guide.md): supported project kinds, languages, runtimes, and CLI options. Add command, file format, and generated-output compatibility contracts here when they need standalone treatment. diff --git a/docs/contracts/telemetry.md b/docs/contracts/telemetry.md index d29e010..6f269d6 100644 --- a/docs/contracts/telemetry.md +++ b/docs/contracts/telemetry.md @@ -1,29 +1,32 @@ # Pseudonymous CLI telemetry contract -This contract governs the default-off product telemetry emitted by the kickstart CLI. It is a data-minimization boundary, not a promise that a stable identifier is anonymous: a durable random identifier correlates events over time and is therefore pseudonymous. +This contract governs the default-on product telemetry emitted by the kickstart CLI. It is a data-minimization boundary, not a promise that a stable identifier is anonymous: a durable random identifier correlates events over time and is therefore pseudonymous. ## Product questions -The first event exists only to answer these questions: +The allowlisted events exist only to answer these questions: 1. Which project kinds, languages, runtimes, and optional scaffold capabilities are useful? 2. Which supported combinations are most common? 3. Which coarse failure categories prevent scaffold creation? -4. Which released CLI versions and packaged platforms are still active? -5. Is the event contract useful enough to justify maintaining telemetry or a first-party relay? +4. Which supported install artifact kinds are used, and how often is PATH integration requested or already present? +5. Do CLI-managed installs and upgrades succeed, and where do checksum or update flows fail? +6. Which released CLI versions and packaged platforms are still active? +7. Is the event contract useful enough to justify maintaining telemetry or a first-party relay? Telemetry is deliberately lossy and spoofable. It is product feedback, not billing, audit, security, identity, unique-person, or authoritative usage evidence. ## Consent and effective enablement -- Telemetry is disabled by default. -- No identity or state file is created by ordinary commands before explicit opt-in. -- `kickstart telemetry enable` persists opt-in and lazily creates a random UUIDv4. +- Telemetry is enabled by default for ordinary installed CLI runs when delivery is configured. +- Missing state represents the default-enabled policy, not explicit consent. Merely checking status does not create state or an identity. +- The first eligible event with a configured sink lazily creates and persists a random UUIDv4 unless explicit enablement already created one. The ID remains stable across CLI upgrades. +- `kickstart telemetry enable` reverses a persisted disable and creates an identity if one does not exist. - `kickstart telemetry disable` persists opt-out and retains an existing ID. -- `kickstart telemetry status` reads state without creating it and displays the current ID when one exists. +- `kickstart telemetry status` is strictly read-only: it does not create or rewrite state, initialize an ID, or send an event. It displays the current ID when one exists. - `kickstart telemetry reset-id` rotates an existing ID for future events and reports the previous ID. Rotation does not delete historical events. -Hard process-level opt-outs override persisted opt-in in this order: +Hard process-level suppressions override both the default and persisted enablement in this order: 1. `DO_NOT_TRACK=1` 2. `KICKSTART_TELEMETRY_DISABLED=1` @@ -32,7 +35,23 @@ Hard process-level opt-outs override persisted opt-in in this order: 5. `KICKSTART_EVAL=1` 6. execution from the kickstart source checkout -A deliberate development-only verification may bypass the source-checkout guard with `KICKSTART_TELEMETRY_ALLOW_DEVELOPMENT=1`; it does not bypass consent or any other hard opt-out. +A deliberate development-only verification may bypass the source-checkout guard with `KICKSTART_TELEMETRY_ALLOW_DEVELOPMENT=1`; it does not bypass a persisted disable or any other hard suppression. + +To prevent an installer-invoked event before a preference can exist, set either +hard suppression on that invocation: + +```bash +KICKSTART_TELEMETRY_DISABLED=1 ./kickstart-macos-arm64-py3.14/kickstart install --update-path +# Or use the standard environment-wide preference: +DO_NOT_TRACK=1 ./kickstart-macos-arm64-py3.14/kickstart install --update-path + +# The same controls are inherited by the downloaded binary in the quick installer: +curl -fsSL https://raw.githubusercontent.com/woud420/kickstart/master/scripts/install.sh \ + | KICKSTART_TELEMETRY_DISABLED=1 bash +``` + +These environment settings apply to the current process. Run +`kickstart telemetry disable` to persist the preference for later commands. ## Identity and persistence @@ -42,13 +61,19 @@ State is stored separately from repository configuration and the replaceable app ${XDG_CONFIG_HOME:-~/.config}/kickstart/telemetry.json ``` -The versioned document contains only `schema_version`, `consent`, and an optional random UUIDv4 `anonymous_id`. The ID is never derived from a username, hostname, MAC address, Git configuration, path, email address, project, repository, or PostHog value. Atomic writes, serialized initialization, and user-only permissions are used where the platform supports them. +The versioned document contains only `schema_version`, `consent`, and an optional random UUIDv4 `anonymous_id`. Missing state is a valid default-enabled state but has no identity. An explicit disable may be persisted without creating an ID. Explicit enablement creates the ID immediately; otherwise, when delivery is configured, the first eligible event initializes it before capture. If persistence fails, no event is sent. The ID is never derived from a username, hostname, MAC address, Git configuration, path, email address, project, repository, or PostHog value. Atomic writes, serialized initialization, and user-only permissions are used where the platform supports them. + +Malformed, unreadable, busy, symlinked, or unwritable state fails closed. A genuinely missing state file follows the default-enabled behavior above. Telemetry failures never change the output, exit status, generated files, or deterministic behavior of another command. -Missing, malformed, unreadable, busy, or unwritable state fails closed. Telemetry failures never change the output, exit status, generated files, or deterministic behavior of another command. +## Event allowlist -## Initial event allowlist +Exactly three event names are allowed: -The only initial event is `scaffold_create_completed`. One event may be attempted for an opted-in `kickstart create` invocation that reaches a terminal handled outcome. Parser failures that occur before the command function starts are outside this boundary. +- `scaffold_create_completed` +- `cli_install_completed` +- `cli_upgrade_completed` + +At most one terminal event may be attempted for an eligible invocation that reaches the corresponding command boundary. Parser failures that occur before the command function starts are outside this boundary. `cli_install_completed` means only an invocation of `kickstart install`, including the PATH-configuration invocation made by the shell installer when applicable; the shell download/copy itself, `pip`, `pipx`, package managers, and manual copies are not independently counted. `cli_upgrade_completed` means only `kickstart upgrade`. These events therefore do not measure universal installation or upgrade totals. The provider-neutral envelope contains only: @@ -58,7 +83,7 @@ The provider-neutral envelope contains only: - `name`: the allowlisted event name - `properties`: the closed property object below -The initial property allowlist is: +The exact closed property set for `scaffold_create_completed` is: - `cli_version` - `project_type` @@ -82,6 +107,32 @@ The initial property allowlist is: Canonical supported values, fixed booleans, fixed outcome/error categories, and duration buckets are permitted. Unknown or invalid inputs are omitted or represented by fixed categories; their raw values are never serialized. +The six common lifecycle properties for install and upgrade events are exactly: + +- `cli_version` +- `outcome` +- `error_category` +- `duration_bucket` +- `platform` +- `architecture` + +The exact closed property set for `cli_install_completed` is those six common properties plus: + +- `artifact_kind`: `single_file`, `onedir`, or `unknown` +- `path_update_requested`: boolean +- `already_on_path`: boolean + +Its `outcome` is one of `success`, `no_change`, `partial_success`, `failed`, or `cancelled`. Its `error_category` is one of `none`, `interrupted`, `source_missing`, `destination_conflict`, `permission_denied`, `path_update`, `filesystem_error`, `expected_error`, or `unexpected_error`. + +The exact closed property set for `cli_upgrade_completed` is those six common properties plus: + +- `target_version`: a stable semantic version or `unknown` +- `checksum_status`: `verified`, `not_published`, `failed`, or `not_reached` + +Its `outcome` is one of `updated`, `already_current`, `failed`, or `cancelled`. Its `error_category` is one of `none`, `interrupted`, `release_lookup`, `invalid_release_metadata`, `unsupported_platform`, `archive_missing`, `download`, `checksum_fetch`, `checksum_mismatch`, `archive_extraction`, `installation`, or `unexpected_error`. + +No other event or property is permitted. In particular, uninstall does not emit an event. Values that are not in these fixed sets are omitted or normalized to an allowed fallback; raw values are never serialized. + ## Prohibited data No telemetry event, local telemetry file, diagnostic, or provider log may contain: @@ -108,7 +159,7 @@ Direct public capture can be spoofed. Provider, destination, or token changes re Official wheel, source-distribution, and standalone-binary artifacts embed the public capture token from the `POSTHOG_PUBLIC_CUSTOMER_API_TOKEN` GitHub Actions secret during trusted build jobs. The secret prevents accidental disclosure in source and build logs; it does not make the token confidential after publication. Anyone can recover a public token from a distributed client, and token rotation requires newly built clients. -For local development and source installations, `POSTHOG_PUBLIC_CUSTOMER_API_TOKEN` may be exported from an ignored `.env` into the process. An explicit process value overrides artifact configuration, including failing closed when that override is malformed. The CLI reads only the already-exported process environment and never auto-loads a current repository's `.env`, because kickstart runs inside arbitrary repositories and repository content must not influence telemetry routing or consent. A missing artifact token and missing or malformed runtime token silently select a no-op sink; token presence never enables telemetry. The numeric `POSTHOG_PROJECT_ID` is not read, embedded, or included in capture payloads. +For local development and source installations, `POSTHOG_PUBLIC_CUSTOMER_API_TOKEN` may be exported from an ignored `.env` into the process. An explicit process value overrides artifact configuration, including failing closed when that override is malformed. The CLI reads only the already-exported process environment and never auto-loads a current repository's `.env`, because kickstart runs inside arbitrary repositories and repository content must not influence telemetry routing or consent. A missing artifact token and missing or malformed runtime token silently select a no-op sink. A valid token configures delivery but never overrides a persisted disable or process-level suppression. The numeric `POSTHOG_PROJECT_ID` is not read, embedded, or included in capture payloads. ## Delivery and retention diff --git a/docs/contributing.md b/docs/contributing.md index 7481e35..d48dd7a 100644 --- a/docs/contributing.md +++ b/docs/contributing.md @@ -50,7 +50,21 @@ make check ``` Use `make package` to build the wheel/source distribution and `make binary` to build the local kickstart binary (a PyInstaller `--onedir` payload under `dist/kickstart/`). -GitHub Actions tests Python 3.12, 3.13, and 3.14 on Linux and macOS, then release builds attach Python 3.14 binary archives (`kickstart--py3.14.tar.gz`) for `linux-x64`, `linux-arm64`, and `macos-arm64`. Other platforms install from source or from the wheel attached to each GitHub Release (see docs/install-binaries.md). +GitHub Actions tests Python 3.12, 3.13, and 3.14 on Linux and macOS, then release builds attach Python 3.14 binary archives (`kickstart--py3.14.tar.gz`) for `linux-x64`, `linux-arm64`, and `macos-arm64`. Other platforms install from source or from the wheel attached to each GitHub Release (see [Install Binaries](operations/install-binaries.md)). + +## Telemetry Changes + +The public policy and closed event schemas live in +[`docs/contracts/telemetry.md`](contracts/telemetry.md). Telemetry is default-on +for eligible configured release commands, with persistent and process-level +opt-outs. Source-checkout development, CI, pytest, and evaluation runs remain +suppressed; do not weaken those guards to make a test pass. + +When changing instrumentation, update the provider-neutral DTO, the exact +property allowlist, privacy tests, public contract, and release smoke coverage +together. Event fields must remain fixed and typed: never pass through names, +paths, raw arguments, environment values, exception text, or generated +content. ## Releases diff --git a/docs/operations/README.md b/docs/operations/README.md index 2e35742..570646e 100644 --- a/docs/operations/README.md +++ b/docs/operations/README.md @@ -8,5 +8,6 @@ Current operations references: - [Contributing](../contributing.md): human contribution workflow. - [Agent Contributing](../agent-contributing.md): agent-safe development workflow. - [Release Policy](../release-policy.md): version tags, release assets, and same-version updates. +- [Install Binaries](install-binaries.md): release assets, installation and upgrade commands, and first-run telemetry controls. Move or add runbooks here when they become operational procedures rather than general guides. diff --git a/docs/install-binaries.md b/docs/operations/install-binaries.md similarity index 80% rename from docs/install-binaries.md rename to docs/operations/install-binaries.md index 17772d0..835b7b2 100644 --- a/docs/install-binaries.md +++ b/docs/operations/install-binaries.md @@ -13,9 +13,35 @@ wheel attached to each GitHub Release. Do not `pip install kickstart`: the PyPI name belongs to an unrelated, abandoned 2011 project. Release wheels and binary archives contain the public PostHog capture -configuration. Direct Git source installs do not; opted-in telemetry from a -source installation requires `POSTHOG_PUBLIC_CUSTOMER_API_TOKEN` in the -process environment. +configuration. Telemetry is enabled by default for eligible commands, but an +explicit `kickstart telemetry disable`, `DO_NOT_TRACK=1`, or +`KICKSTART_TELEMETRY_DISABLED=1` always wins. Direct Git source installs do not +contain the capture configuration; delivery from one requires +`POSTHOG_PUBLIC_CUSTOMER_API_TOKEN` in the process environment as well as an +otherwise eligible event. + +The `cli_install_completed` event refers only to an invocation of the +`kickstart install` command, including the PATH-configuration invocation made +by the shell installer when applicable. The shell download/copy itself, `pip`, +`pipx`, package managers, and manual file copies are not independently counted. +Likewise, `cli_upgrade_completed` refers only to `kickstart upgrade`, so these +events are not universal installation or upgrade totals. + +To prevent even the first `kickstart install` event, set either process-level +opt-out on that invocation: + +```bash +KICKSTART_TELEMETRY_DISABLED=1 ./kickstart-macos-arm64-py3.14/kickstart install --update-path +# Or, when DO_NOT_TRACK is your standard environment-wide preference: +DO_NOT_TRACK=1 ./kickstart-macos-arm64-py3.14/kickstart install --update-path + +# Suppress an installer-invoked event from the quick-install process: +curl -fsSL https://raw.githubusercontent.com/woud420/kickstart/master/scripts/install.sh \ + | KICKSTART_TELEMETRY_DISABLED=1 bash +``` + +Those environment settings apply to the current process. Run +`kickstart telemetry disable` to persist the preference for later commands. ## Asset Names diff --git a/src/cli/main.py b/src/cli/main.py index 184f810..f4995a8 100644 --- a/src/cli/main.py +++ b/src/cli/main.py @@ -34,11 +34,24 @@ from src.cli.prompts import ConfirmReader, PromptReader, prompt_for_missing_args from src.cli.telemetry import telemetry_app from src.model.dto.telemetry import ( + CliArtifactKind, + CliInstallErrorCategory, + CliInstallOutcome, + CliUpgradeChecksumStatus, + CliUpgradeErrorCategory, + CliUpgradeOutcome, ScaffoldCreateContext, ScaffoldCreateErrorCategory, ScaffoldCreateOutcome, ) -from src.telemetry.events import capture_scaffold_create_terminal, classify_scaffold_create_exception +from src.model.dto.upgrade import UNKNOWN_TARGET_VERSION, UpgradeResult +from src.telemetry.events import ( + capture_cli_install_terminal, + capture_cli_upgrade_terminal, + capture_scaffold_create_terminal, + classify_cli_install_exception, + classify_scaffold_create_exception, +) from src.utils.config import load_config from src.utils.errors import KickstartError from src.utils.installer import ( @@ -95,7 +108,33 @@ def version() -> None: @app.command() def upgrade() -> None: """Upgrade to the latest version.""" - check_for_update() + started_at = monotonic() + result: UpgradeResult | None = None + try: + result = check_for_update() + except KeyboardInterrupt: + result = UpgradeResult( + target_version=UNKNOWN_TARGET_VERSION, + outcome=CliUpgradeOutcome.CANCELLED, + error_category=CliUpgradeErrorCategory.INTERRUPTED, + checksum_status=CliUpgradeChecksumStatus.NOT_REACHED, + ) + raise + except Exception: + result = UpgradeResult( + target_version=UNKNOWN_TARGET_VERSION, + outcome=CliUpgradeOutcome.FAILED, + error_category=CliUpgradeErrorCategory.UNEXPECTED_ERROR, + checksum_status=CliUpgradeChecksumStatus.NOT_REACHED, + ) + raise + finally: + if result is not None: + capture_cli_upgrade_terminal( + result, + monotonic() - started_at, + cli_version=__version__, + ) export_app: typer.Typer = typer.Typer(help="Deterministic exporters derived from scaffold state.") @@ -288,30 +327,76 @@ def install( check: bool = typer.Option(False, "--check", help="Only report install/PATH status; do not modify anything."), ) -> None: """Install the kickstart binary to a user-writable directory and help configure PATH.""" - with _report_install_failure("Install"): - ctx = _resolve_install_context(target, app_dir=app_dir, shell=shell, rc_file=rc_file) - source = current_binary_path() - - if check: - _print_install_status(ctx, source) - return - - result: InstallResult = install_binary(source, ctx.install_dir, overwrite=force, app_root=ctx.app_root) - if result.already_installed: - print(f"[green]✔ kickstart is already installed at {result.destination}[/]") - elif result.copied: - print(f"[green]✔ Installed kickstart to {result.destination}[/]") - - if ctx.already_on_path: - print(f"[green]✔ {ctx.install_dir} is already on your PATH.[/]") - print("[cyan]Run [bold]kickstart --help[/bold] to get started.[/cyan]") - return - - if update_path: - _apply_path_update(ctx) - return + started_at = monotonic() + outcome = CliInstallOutcome.FAILED + error_category = CliInstallErrorCategory.UNEXPECTED_ERROR + artifact_kind = CliArtifactKind.UNKNOWN + already_on_path = False + binary_changed = False + during_path_update = False - _print_manual_path_instructions(ctx) + try: + with _report_install_failure("Install"): + try: + ctx = _resolve_install_context(target, app_dir=app_dir, shell=shell, rc_file=rc_file) + already_on_path = ctx.already_on_path + source = current_binary_path() + + if check: + _print_install_status(ctx, source) + return + + result: InstallResult = install_binary( + source, + ctx.install_dir, + overwrite=force, + app_root=ctx.app_root, + ) + artifact_kind = ( + CliArtifactKind.ONEDIR if result.app_path is not None else CliArtifactKind.SINGLE_FILE + ) + binary_changed = result.copied + outcome = CliInstallOutcome.NO_CHANGE if result.already_installed else CliInstallOutcome.SUCCESS + error_category = CliInstallErrorCategory.NONE + if result.already_installed: + print(f"[green]✔ kickstart is already installed at {result.destination}[/]") + elif result.copied: + print(f"[green]✔ Installed kickstart to {result.destination}[/]") + + if ctx.already_on_path: + print(f"[green]✔ {ctx.install_dir} is already on your PATH.[/]") + print("[cyan]Run [bold]kickstart --help[/bold] to get started.[/cyan]") + return + + if update_path: + during_path_update = True + _apply_path_update(ctx) + during_path_update = False + return + + _print_manual_path_instructions(ctx) + except KeyboardInterrupt: + outcome = CliInstallOutcome.PARTIAL_SUCCESS if binary_changed else CliInstallOutcome.CANCELLED + error_category = CliInstallErrorCategory.INTERRUPTED + raise + except Exception as exc: + outcome = CliInstallOutcome.PARTIAL_SUCCESS if binary_changed else CliInstallOutcome.FAILED + error_category = classify_cli_install_exception( + exc, + during_path_update=during_path_update, + ) + raise + finally: + if not check: + capture_cli_install_terminal( + outcome, + error_category, + artifact_kind, + path_update_requested=update_path, + already_on_path=already_on_path, + duration_seconds=monotonic() - started_at, + cli_version=__version__, + ) @app.command() diff --git a/src/cli/telemetry.py b/src/cli/telemetry.py index ddbaea4..5e9e71e 100644 --- a/src/cli/telemetry.py +++ b/src/cli/telemetry.py @@ -11,7 +11,7 @@ from src.utils.errors import TelemetryStateError -telemetry_app = typer.Typer(help="Inspect and control default-off pseudonymous telemetry.") +telemetry_app = typer.Typer(help="Inspect and control default-on pseudonymous telemetry.") def _state_store() -> TelemetryStateStore: @@ -57,7 +57,7 @@ def telemetry_status( @telemetry_app.command("enable") def telemetry_enable() -> None: - """Explicitly opt in and create an identity only when needed.""" + """Persist enablement and create an identity only when needed.""" try: state = _state_store().enable() except TelemetryStateError as exc: diff --git a/src/model/dto/__init__.py b/src/model/dto/__init__.py index 640db71..344380a 100644 --- a/src/model/dto/__init__.py +++ b/src/model/dto/__init__.py @@ -2,6 +2,14 @@ from src.model.dto.posthog import PostHogCapturePayload, PostHogCaptureRequest from src.model.dto.telemetry import ( + CliArtifactKind, + CliInstallErrorCategory, + CliInstallOutcome, + CliInstallProperties, + CliUpgradeChecksumStatus, + CliUpgradeErrorCategory, + CliUpgradeOutcome, + CliUpgradeProperties, EffectiveTelemetry, ScaffoldCreateContext, ScaffoldCreateErrorCategory, @@ -16,8 +24,17 @@ TelemetryState, TelemetrySuppressionReason, ) +from src.model.dto.upgrade import UNKNOWN_TARGET_VERSION, UpgradeResult, normalize_target_version __all__ = [ + "CliArtifactKind", + "CliInstallErrorCategory", + "CliInstallOutcome", + "CliInstallProperties", + "CliUpgradeChecksumStatus", + "CliUpgradeErrorCategory", + "CliUpgradeOutcome", + "CliUpgradeProperties", "EffectiveTelemetry", "PostHogCapturePayload", "PostHogCaptureRequest", @@ -33,4 +50,7 @@ "TelemetryIdentityReset", "TelemetryState", "TelemetrySuppressionReason", + "UNKNOWN_TARGET_VERSION", + "UpgradeResult", + "normalize_target_version", ] diff --git a/src/model/dto/telemetry.py b/src/model/dto/telemetry.py index 568da04..48fa4d0 100644 --- a/src/model/dto/telemetry.py +++ b/src/model/dto/telemetry.py @@ -20,9 +20,78 @@ class TelemetryConsent(StrEnum): class TelemetryEventName(StrEnum): - """Closed event-name allowlist for the initial telemetry contract.""" + """Closed event-name allowlist for the telemetry contract.""" SCAFFOLD_CREATE_COMPLETED = "scaffold_create_completed" + CLI_INSTALL_COMPLETED = "cli_install_completed" + CLI_UPGRADE_COMPLETED = "cli_upgrade_completed" + + +class CliInstallOutcome(StrEnum): + """Closed terminal outcomes for a binary install invocation.""" + + SUCCESS = "success" + NO_CHANGE = "no_change" + PARTIAL_SUCCESS = "partial_success" + FAILED = "failed" + CANCELLED = "cancelled" + + +class CliInstallErrorCategory(StrEnum): + """Coarse install failures that never contain path or exception data.""" + + NONE = "none" + INTERRUPTED = "interrupted" + SOURCE_MISSING = "source_missing" + DESTINATION_CONFLICT = "destination_conflict" + PERMISSION_DENIED = "permission_denied" + PATH_UPDATE = "path_update" + FILESYSTEM_ERROR = "filesystem_error" + EXPECTED_ERROR = "expected_error" + UNEXPECTED_ERROR = "unexpected_error" + + +class CliArtifactKind(StrEnum): + """Closed executable packaging shapes relevant to installation.""" + + SINGLE_FILE = "single_file" + ONEDIR = "onedir" + UNKNOWN = "unknown" + + +class CliUpgradeOutcome(StrEnum): + """Closed terminal outcomes for a self-upgrade invocation.""" + + UPDATED = "updated" + ALREADY_CURRENT = "already_current" + FAILED = "failed" + CANCELLED = "cancelled" + + +class CliUpgradeErrorCategory(StrEnum): + """Coarse upgrade failures that never contain transport details.""" + + NONE = "none" + INTERRUPTED = "interrupted" + RELEASE_LOOKUP = "release_lookup" + INVALID_RELEASE_METADATA = "invalid_release_metadata" + UNSUPPORTED_PLATFORM = "unsupported_platform" + ARCHIVE_MISSING = "archive_missing" + DOWNLOAD = "download" + CHECKSUM_FETCH = "checksum_fetch" + CHECKSUM_MISMATCH = "checksum_mismatch" + ARCHIVE_EXTRACTION = "archive_extraction" + INSTALLATION = "installation" + UNEXPECTED_ERROR = "unexpected_error" + + +class CliUpgradeChecksumStatus(StrEnum): + """Closed checksum states for a self-upgrade attempt.""" + + VERIFIED = "verified" + NOT_PUBLISHED = "not_published" + FAILED = "failed" + NOT_REACHED = "not_reached" class ScaffoldCreateOutcome(StrEnum): @@ -63,7 +132,7 @@ class TelemetrySuppressionReason(StrEnum): """Why telemetry is not effective for the current process.""" ENABLED = "enabled" - NOT_OPTED_IN = "not_opted_in" + DEFAULT_ENABLED = "default_enabled" USER_DISABLED = "user_disabled" DO_NOT_TRACK = "do_not_track" ENVIRONMENT_DISABLED = "environment_disabled" @@ -76,7 +145,7 @@ class TelemetrySuppressionReason(StrEnum): @dataclass(frozen=True) class ScaffoldCreateProperties: - """Closed property DTO for the initial scaffold completion event.""" + """Closed property DTO for the scaffold completion event.""" cli_version: str project_type: str @@ -123,6 +192,62 @@ def as_mapping(self) -> dict[str, TelemetryPropertyValue]: } +@dataclass(frozen=True) +class CliInstallProperties: + """Exact property allowlist for a terminal binary install event.""" + + cli_version: str + outcome: CliInstallOutcome + error_category: CliInstallErrorCategory + artifact_kind: CliArtifactKind + path_update_requested: bool + already_on_path: bool + duration_bucket: TelemetryDurationBucket + platform: str + architecture: str + + def as_mapping(self) -> dict[str, TelemetryPropertyValue]: + """Return the exact provider-neutral property allowlist.""" + return { + "already_on_path": self.already_on_path, + "architecture": self.architecture, + "artifact_kind": self.artifact_kind.value, + "cli_version": self.cli_version, + "duration_bucket": self.duration_bucket.value, + "error_category": self.error_category.value, + "outcome": self.outcome.value, + "path_update_requested": self.path_update_requested, + "platform": self.platform, + } + + +@dataclass(frozen=True) +class CliUpgradeProperties: + """Exact property allowlist for a terminal self-upgrade event.""" + + cli_version: str + target_version: str + outcome: CliUpgradeOutcome + error_category: CliUpgradeErrorCategory + checksum_status: CliUpgradeChecksumStatus + duration_bucket: TelemetryDurationBucket + platform: str + architecture: str + + def as_mapping(self) -> dict[str, TelemetryPropertyValue]: + """Return the exact provider-neutral property allowlist.""" + return { + "architecture": self.architecture, + "checksum_status": self.checksum_status.value, + "cli_version": self.cli_version, + "duration_bucket": self.duration_bucket.value, + "error_category": self.error_category.value, + "outcome": self.outcome.value, + "platform": self.platform, + "target_version": self.target_version, + } + + @dataclass(frozen=True) class ScaffoldCreateContext: """Safe create inputs retained for terminal event normalization. @@ -150,7 +275,7 @@ class TelemetryEvent: """A typed product event before identity and delivery metadata are added.""" name: TelemetryEventName - properties: ScaffoldCreateProperties + properties: ScaffoldCreateProperties | CliInstallProperties | CliUpgradeProperties @dataclass(frozen=True) diff --git a/src/model/dto/upgrade.py b/src/model/dto/upgrade.py new file mode 100644 index 0000000..07c1037 --- /dev/null +++ b/src/model/dto/upgrade.py @@ -0,0 +1,32 @@ +"""Typed, provider-neutral results for the self-upgrade workflow.""" + +import re +from dataclasses import dataclass + +from src.model.dto.telemetry import ( + CliUpgradeChecksumStatus, + CliUpgradeErrorCategory, + CliUpgradeOutcome, +) + + +_STABLE_SEMVER = re.compile(r"^v?((?:0|[1-9]\d*)\.(?:0|[1-9]\d*)\.(?:0|[1-9]\d*))$") +UNKNOWN_TARGET_VERSION = "unknown" + + +def normalize_target_version(value: str | None) -> str: + """Return a stable semantic version without a leading ``v``, or ``unknown``.""" + if value is None: + return UNKNOWN_TARGET_VERSION + match = _STABLE_SEMVER.fullmatch(value) + return match.group(1) if match is not None else UNKNOWN_TARGET_VERSION + + +@dataclass(frozen=True) +class UpgradeResult: + """Safe terminal result returned by the updater to its CLI caller.""" + + target_version: str + outcome: CliUpgradeOutcome + error_category: CliUpgradeErrorCategory + checksum_status: CliUpgradeChecksumStatus diff --git a/src/telemetry/events.py b/src/telemetry/events.py index ea911de..2d9ee56 100644 --- a/src/telemetry/events.py +++ b/src/telemetry/events.py @@ -3,6 +3,11 @@ import platform as runtime_platform from src.model.dto.telemetry import ( + CliArtifactKind, + CliInstallErrorCategory, + CliInstallOutcome, + CliInstallProperties, + CliUpgradeProperties, ScaffoldCreateContext, ScaffoldCreateErrorCategory, ScaffoldCreateOutcome, @@ -11,6 +16,7 @@ TelemetryEvent, TelemetryEventName, ) +from src.model.dto.upgrade import UpgradeResult, normalize_target_version from src.stack.profile import stack_registry from src.telemetry.reporter import TelemetryReporter, default_telemetry_reporter from src.utils.errors import ( @@ -119,6 +125,124 @@ def capture_scaffold_create_terminal( return +def build_cli_install_event( + outcome: CliInstallOutcome, + error_category: CliInstallErrorCategory, + artifact_kind: CliArtifactKind, + *, + path_update_requested: bool, + already_on_path: bool, + duration_seconds: float, + cli_version: str, + platform_name: str | None = None, + architecture: str | None = None, +) -> TelemetryEvent: + """Build the closed terminal event for a binary install invocation.""" + return TelemetryEvent( + name=TelemetryEventName.CLI_INSTALL_COMPLETED, + properties=CliInstallProperties( + cli_version=cli_version, + outcome=outcome, + error_category=error_category, + artifact_kind=artifact_kind, + path_update_requested=path_update_requested, + already_on_path=already_on_path, + duration_bucket=_duration_bucket(duration_seconds), + platform=_platform(platform_name or runtime_platform.system()), + architecture=_architecture(architecture or runtime_platform.machine()), + ), + ) + + +def capture_cli_install_terminal( + outcome: CliInstallOutcome, + error_category: CliInstallErrorCategory, + artifact_kind: CliArtifactKind, + *, + path_update_requested: bool, + already_on_path: bool, + duration_seconds: float, + cli_version: str, + reporter: TelemetryReporter | None = None, +) -> None: + """Attempt one install capture while containing all telemetry failures.""" + try: + event = build_cli_install_event( + outcome, + error_category, + artifact_kind, + path_update_requested=path_update_requested, + already_on_path=already_on_path, + duration_seconds=duration_seconds, + cli_version=cli_version, + ) + target_reporter = default_telemetry_reporter() if reporter is None else reporter + target_reporter.record(event) + except Exception: + return + + +def classify_cli_install_exception( + error: Exception, + *, + during_path_update: bool, +) -> CliInstallErrorCategory: + """Reduce an install exception to a fixed, non-identifying category.""" + if during_path_update: + return CliInstallErrorCategory.PATH_UPDATE + if isinstance(error, FileNotFoundError): + return CliInstallErrorCategory.SOURCE_MISSING + if isinstance(error, FileExistsError): + return CliInstallErrorCategory.DESTINATION_CONFLICT + if isinstance(error, PermissionError): + return CliInstallErrorCategory.PERMISSION_DENIED + if isinstance(error, OSError): + return CliInstallErrorCategory.FILESYSTEM_ERROR + if isinstance(error, KickstartError): + return CliInstallErrorCategory.EXPECTED_ERROR + return CliInstallErrorCategory.UNEXPECTED_ERROR + + +def build_cli_upgrade_event( + result: UpgradeResult, + duration_seconds: float, + *, + cli_version: str, + platform_name: str | None = None, + architecture: str | None = None, +) -> TelemetryEvent: + """Build the closed terminal event for a self-upgrade invocation.""" + return TelemetryEvent( + name=TelemetryEventName.CLI_UPGRADE_COMPLETED, + properties=CliUpgradeProperties( + cli_version=cli_version, + target_version=normalize_target_version(result.target_version), + outcome=result.outcome, + error_category=result.error_category, + checksum_status=result.checksum_status, + duration_bucket=_duration_bucket(duration_seconds), + platform=_platform(platform_name or runtime_platform.system()), + architecture=_architecture(architecture or runtime_platform.machine()), + ), + ) + + +def capture_cli_upgrade_terminal( + result: UpgradeResult, + duration_seconds: float, + *, + cli_version: str, + reporter: TelemetryReporter | None = None, +) -> None: + """Attempt one upgrade capture while containing all telemetry failures.""" + try: + event = build_cli_upgrade_event(result, duration_seconds, cli_version=cli_version) + target_reporter = default_telemetry_reporter() if reporter is None else reporter + target_reporter.record(event) + except Exception: + return + + def classify_scaffold_create_exception( error: Exception, ) -> tuple[ScaffoldCreateOutcome, ScaffoldCreateErrorCategory]: diff --git a/src/telemetry/policy.py b/src/telemetry/policy.py index 60f33ab..39e716b 100644 --- a/src/telemetry/policy.py +++ b/src/telemetry/policy.py @@ -1,4 +1,4 @@ -"""Fail-closed telemetry enablement policy.""" +"""Resolve default-on telemetry with fail-closed suppression and state handling.""" import os from collections.abc import Mapping @@ -40,18 +40,18 @@ def resolve_telemetry( current_state = store.read() if state is None else state except TelemetryStateError: return _disabled(TelemetrySuppressionReason.INVALID_STATE) - if current_state.consent is TelemetryConsent.ENABLED and current_state.anonymous_id is not None: + if current_state.consent is TelemetryConsent.DISABLED: return EffectiveTelemetry( - enabled=True, - reason=TelemetrySuppressionReason.ENABLED, + enabled=False, + reason=TelemetrySuppressionReason.USER_DISABLED, anonymous_id=current_state.anonymous_id, ) reason = ( - TelemetrySuppressionReason.USER_DISABLED - if current_state.consent is TelemetryConsent.DISABLED - else TelemetrySuppressionReason.NOT_OPTED_IN + TelemetrySuppressionReason.DEFAULT_ENABLED + if current_state.consent is TelemetryConsent.UNSET + else TelemetrySuppressionReason.ENABLED ) - return EffectiveTelemetry(enabled=False, reason=reason, anonymous_id=current_state.anonymous_id) + return EffectiveTelemetry(enabled=True, reason=reason, anonymous_id=current_state.anonymous_id) def _disabled(reason: TelemetrySuppressionReason) -> EffectiveTelemetry: diff --git a/src/telemetry/reporter.py b/src/telemetry/reporter.py index 1425464..5c81374 100644 --- a/src/telemetry/reporter.py +++ b/src/telemetry/reporter.py @@ -32,16 +32,21 @@ class TelemetryReporter: def record(self, event: TelemetryEvent) -> None: """Attempt one event only when effective policy permits it.""" try: + if isinstance(self.sink, NoOpTelemetrySink): + return effective = resolve_telemetry( self.state_store, self.environ, development=self.development, ) - if not effective.enabled or effective.anonymous_id is None: + if not effective.enabled: + return + anonymous_id = self.state_store.identity_for_event() + if anonymous_id is None: return envelope = TelemetryEnvelope( event_id=self.uuid_factory(), - anonymous_id=effective.anonymous_id, + anonymous_id=anonymous_id, occurred_at=self.clock(), event=event, ) diff --git a/src/telemetry/state.py b/src/telemetry/state.py index 0f14785..fdd46b8 100644 --- a/src/telemetry/state.py +++ b/src/telemetry/state.py @@ -2,6 +2,7 @@ import json import os +import stat import tempfile import time from collections.abc import Callable, Iterator, Mapping @@ -55,9 +56,15 @@ def from_environment(cls, environ: Mapping[str, str] | None = None) -> "Telemetr return cls(default_telemetry_state_path(environ)) def read(self) -> TelemetryState: - """Read state without creating a file; missing state means not opted in.""" - if not self.path.exists(): + """Read state without creating a file; missing state uses the default policy.""" + try: + file_status = self.path.lstat() + except FileNotFoundError: return TelemetryState() + except OSError as exc: + raise TelemetryStateError("Telemetry state is unreadable or malformed; telemetry is disabled.") from exc + if stat.S_ISLNK(file_status.st_mode): + raise TelemetryStateError("Telemetry state is unreadable or malformed; telemetry is disabled.") try: payload = cast(JsonValue, json.loads(self.path.read_text(encoding="utf-8"))) return _parse_state(payload) @@ -65,7 +72,7 @@ def read(self) -> TelemetryState: raise TelemetryStateError("Telemetry state is unreadable or malformed; telemetry is disabled.") from exc def enable(self) -> TelemetryState: - """Persist opt-in and lazily create a stable UUIDv4 identity.""" + """Persist explicit enablement and create a stable UUIDv4 identity if needed.""" with self._locked(): current = self.read() anonymous_id = current.anonymous_id or self._uuid_factory() @@ -73,6 +80,22 @@ def enable(self) -> TelemetryState: self._write(state) return state + def identity_for_event(self) -> UUID | None: + """Return the stable identity for an eligible event, creating it atomically. + + The state is re-read while holding the update lock so an explicit opt-out + observed before identity acquisition always wins over default enablement. + """ + with self._locked(): + current = self.read() + if current.consent is TelemetryConsent.DISABLED: + return None + if current.anonymous_id is not None: + return current.anonymous_id + anonymous_id = self._uuid_factory() + self._write(TelemetryState(consent=TelemetryConsent.ENABLED, anonymous_id=anonymous_id)) + return anonymous_id + def disable(self) -> TelemetryState: """Persist opt-out while retaining any existing identity.""" with self._locked(): diff --git a/src/utils/updater.py b/src/utils/updater.py index 97691f8..199789a 100644 --- a/src/utils/updater.py +++ b/src/utils/updater.py @@ -21,6 +21,12 @@ import requests from src import __version__ +from src.model.dto.telemetry import ( + CliUpgradeChecksumStatus, + CliUpgradeErrorCategory, + CliUpgradeOutcome, +) +from src.model.dto.upgrade import UNKNOWN_TARGET_VERSION, UpgradeResult, normalize_target_version from src.utils.installer import ( APP_DIR_NAME, BINARY_NAME, @@ -145,36 +151,82 @@ def _extract_archive_launcher(archive_path: Path, dest_root: Path) -> Path: return launcher -def check_for_update() -> None: - """Check for updates and install the newest release in-place.""" +def check_for_update() -> UpgradeResult: + """Check for updates, install the newest release, and return a safe terminal result.""" print(f"[cyan]Checking for updates (current version: {__version__})...") try: info = fetch_release_info() except Exception as exc: print(f"[red]✖ Update failed: could not query the latest release: {exc}") - return + return UpgradeResult( + target_version=UNKNOWN_TARGET_VERSION, + outcome=CliUpgradeOutcome.FAILED, + error_category=CliUpgradeErrorCategory.RELEASE_LOOKUP, + checksum_status=CliUpgradeChecksumStatus.NOT_REACHED, + ) + + try: + tag_name = info["tag_name"] + assets = info["assets"] + metadata_is_valid = ( + isinstance(tag_name, str) + and bool(tag_name) + and isinstance(assets, list) + and all( + isinstance(asset, dict) + and isinstance(asset.get("name"), str) + and isinstance(asset.get("browser_download_url"), str) + for asset in assets + ) + ) + except (KeyError, TypeError): + metadata_is_valid = False + if not metadata_is_valid: + print("[red]✖ Update failed: latest release metadata is invalid.") + return UpgradeResult( + target_version=UNKNOWN_TARGET_VERSION, + outcome=CliUpgradeOutcome.FAILED, + error_category=CliUpgradeErrorCategory.INVALID_RELEASE_METADATA, + checksum_status=CliUpgradeChecksumStatus.NOT_REACHED, + ) - latest = info["tag_name"].lstrip("v") + latest = tag_name.lstrip("v") + target_version = normalize_target_version(tag_name) if latest == __version__: print("[green]✅ You're already up to date.") - return + return UpgradeResult( + target_version=target_version, + outcome=CliUpgradeOutcome.ALREADY_CURRENT, + error_category=CliUpgradeErrorCategory.NONE, + checksum_status=CliUpgradeChecksumStatus.NOT_REACHED, + ) try: archive_name = expected_archive_name() except RuntimeError as exc: print(f"[red]✖ Update failed: {exc}") - return + return UpgradeResult( + target_version=target_version, + outcome=CliUpgradeOutcome.FAILED, + error_category=CliUpgradeErrorCategory.UNSUPPORTED_PLATFORM, + checksum_status=CliUpgradeChecksumStatus.NOT_REACHED, + ) - archive_asset = find_asset(info["assets"], archive_name) + archive_asset = find_asset(assets, archive_name) if archive_asset is None: print( - f"[red]✖ Update failed: no asset named {archive_name} on release {info['tag_name']}. " + f"[red]✖ Update failed: no asset named {archive_name} on release {tag_name}. " "Wait for the release to publish the matching binary archive." ) - return + return UpgradeResult( + target_version=target_version, + outcome=CliUpgradeOutcome.FAILED, + error_category=CliUpgradeErrorCategory.ARCHIVE_MISSING, + checksum_status=CliUpgradeChecksumStatus.NOT_REACHED, + ) - hash_asset = find_asset(info["assets"], f"{archive_name}.sha256") + hash_asset = find_asset(assets, f"{archive_name}.sha256") launcher_dir, app_root = resolve_current_install_layout() print(f"[yellow]⬆ New version available: {latest} — downloading {archive_name}...") @@ -189,20 +241,41 @@ def check_for_update() -> None: download_to(archive_asset["browser_download_url"], archive_path) except Exception as exc: print(f"[red]✖ Update failed: could not download {archive_name}: {exc}") - return + return UpgradeResult( + target_version=target_version, + outcome=CliUpgradeOutcome.FAILED, + error_category=CliUpgradeErrorCategory.DOWNLOAD, + checksum_status=CliUpgradeChecksumStatus.NOT_REACHED, + ) if hash_asset is not None: try: hash_response = requests.get(hash_asset["browser_download_url"], timeout=10) hash_response.raise_for_status() expected_hash = hash_response.text.strip().split()[0] + except Exception as exc: + print(f"[red]✖ Update failed: could not verify checksum: {exc}") + return UpgradeResult( + target_version=target_version, + outcome=CliUpgradeOutcome.FAILED, + error_category=CliUpgradeErrorCategory.CHECKSUM_FETCH, + checksum_status=CliUpgradeChecksumStatus.FAILED, + ) + try: verify_sha256(archive_path, expected_hash) - print(f"[green]🔒 Checksum verified ({expected_hash}).") except Exception as exc: print(f"[red]✖ Update failed: could not verify checksum: {exc}") - return + return UpgradeResult( + target_version=target_version, + outcome=CliUpgradeOutcome.FAILED, + error_category=CliUpgradeErrorCategory.CHECKSUM_MISMATCH, + checksum_status=CliUpgradeChecksumStatus.FAILED, + ) + print(f"[green]🔒 Checksum verified ({expected_hash}).") + checksum_status = CliUpgradeChecksumStatus.VERIFIED else: print("[yellow]⚠ No matching .sha256 asset; skipping checksum verification.") + checksum_status = CliUpgradeChecksumStatus.NOT_PUBLISHED extract_root = tmp / "extracted" extract_root.mkdir() @@ -210,7 +283,12 @@ def check_for_update() -> None: launcher = _extract_archive_launcher(archive_path, extract_root) except Exception as exc: print(f"[red]✖ Update failed: could not extract archive: {exc}") - return + return UpgradeResult( + target_version=target_version, + outcome=CliUpgradeOutcome.FAILED, + error_category=CliUpgradeErrorCategory.ARCHIVE_EXTRACTION, + checksum_status=checksum_status, + ) try: result: InstallResult = install_binary( @@ -221,9 +299,20 @@ def check_for_update() -> None: ) except Exception as exc: print(f"[red]✖ Update failed: could not install the new app bundle: {exc}") - return + return UpgradeResult( + target_version=target_version, + outcome=CliUpgradeOutcome.FAILED, + error_category=CliUpgradeErrorCategory.INSTALLATION, + checksum_status=checksum_status, + ) print(f"[green]✔ Updated to {latest}.") print(f" launcher: {result.destination}") if result.app_path is not None: print(f" app: {result.app_path}") + return UpgradeResult( + target_version=target_version, + outcome=CliUpgradeOutcome.UPDATED, + error_category=CliUpgradeErrorCategory.NONE, + checksum_status=checksum_status, + ) diff --git a/tests/unit/cli/test_main.py b/tests/unit/cli/test_main.py index 98aff5e..d856dc0 100644 --- a/tests/unit/cli/test_main.py +++ b/tests/unit/cli/test_main.py @@ -10,7 +10,17 @@ _report_install_failure, _resolve_install_context, app, + upgrade, ) +from src.model.dto.telemetry import ( + CliArtifactKind, + CliInstallErrorCategory, + CliInstallOutcome, + CliUpgradeChecksumStatus, + CliUpgradeErrorCategory, + CliUpgradeOutcome, +) +from src.model.dto.upgrade import UpgradeResult from src.utils.errors import ExtensionError, KickstartError @@ -33,13 +43,43 @@ def test_version_command(runner): assert "kickstart v1.0.0" in result.stdout -@patch('src.cli.main.check_for_update') -def test_upgrade_command(mock_check_for_update, runner): +@patch("src.cli.main.capture_cli_upgrade_terminal") +@patch("src.cli.main.check_for_update") +def test_upgrade_command(mock_check_for_update, mock_capture, runner): """Test the upgrade command calls check_for_update.""" + update_result = UpgradeResult( + target_version="1.1.0", + outcome=CliUpgradeOutcome.UPDATED, + error_category=CliUpgradeErrorCategory.NONE, + checksum_status=CliUpgradeChecksumStatus.VERIFIED, + ) + mock_check_for_update.return_value = update_result result = runner.invoke(app, ["upgrade"]) - + assert result.exit_code == 0 mock_check_for_update.assert_called_once() + mock_capture.assert_called_once() + assert mock_capture.call_args.args[0] == update_result + + +@pytest.mark.parametrize( + ("error", "outcome", "category"), + [ + (KeyboardInterrupt(), CliUpgradeOutcome.CANCELLED, CliUpgradeErrorCategory.INTERRUPTED), + (RuntimeError("private detail"), CliUpgradeOutcome.FAILED, CliUpgradeErrorCategory.UNEXPECTED_ERROR), + ], +) +def test_upgrade_captures_unhandled_terminal_outcomes(error, outcome, category): + with ( + patch("src.cli.main.check_for_update", side_effect=error), + patch("src.cli.main.capture_cli_upgrade_terminal") as capture, + pytest.raises(type(error)), + ): + upgrade() + + captured = capture.call_args.args[0] + assert captured.outcome is outcome + assert captured.error_category is category def test_completion_command_bash(runner): @@ -610,7 +650,10 @@ def test_install_command_check_mode(runner, tmp_path): source = _make_single_file_binary(tmp_path) target_dir = tmp_path / "user-bin" - with patch("src.cli.main.current_binary_path", return_value=source): + with ( + patch("src.cli.main.current_binary_path", return_value=source), + patch("src.cli.main.capture_cli_install_terminal") as capture, + ): result = runner.invoke( app, ["install", "--target", str(target_dir), "--check", "--shell", "zsh"], @@ -621,6 +664,7 @@ def test_install_command_check_mode(runner, tmp_path): assert "Install status" in result.stdout assert "missing" in result.stdout assert not (target_dir / "kickstart").exists() + capture.assert_not_called() def test_install_command_copies_binary_and_prints_path_hint(runner, tmp_path): @@ -628,7 +672,10 @@ def test_install_command_copies_binary_and_prints_path_hint(runner, tmp_path): source = _make_single_file_binary(tmp_path) target_dir = tmp_path / "user-bin" - with patch("src.cli.main.current_binary_path", return_value=source): + with ( + patch("src.cli.main.current_binary_path", return_value=source), + patch("src.cli.main.capture_cli_install_terminal") as capture, + ): result = runner.invoke( app, ["install", "--target", str(target_dir), "--shell", "zsh"], @@ -645,6 +692,10 @@ def test_install_command_copies_binary_and_prints_path_hint(runner, tmp_path): assert (target_dir / "kickstart").exists() assert f'export PATH="{target_dir}:$PATH"' in result.stdout assert "not on your PATH" in result.stdout + capture.assert_called_once() + assert capture.call_args.args[0].value == "success" + assert capture.call_args.args[1].value == "none" + assert capture.call_args.args[2].value == "single_file" def test_install_command_updates_rc_file(runner, tmp_path): @@ -710,7 +761,10 @@ def test_install_command_refuses_overwrite_without_force(runner, tmp_path): target_dir.mkdir() (target_dir / "kickstart").write_text("placeholder") - with patch("src.cli.main.current_binary_path", return_value=source): + with ( + patch("src.cli.main.current_binary_path", return_value=source), + patch("src.cli.main.capture_cli_install_terminal") as capture, + ): result = runner.invoke( app, ["install", "--target", str(target_dir), "--shell", "zsh"], @@ -718,6 +772,49 @@ def test_install_command_refuses_overwrite_without_force(runner, tmp_path): ) assert result.exit_code != 0 + capture.assert_called_once() + assert capture.call_args.args[0].value == "failed" + assert capture.call_args.args[1].value == "destination_conflict" + + +def test_install_command_reports_partial_success_when_path_update_fails(runner, tmp_path): + source = _make_single_file_binary(tmp_path) + target_dir = tmp_path / "user-bin" + + with ( + patch("src.cli.main.current_binary_path", return_value=source), + patch("src.cli.main._apply_path_update", side_effect=OSError("private detail")), + patch("src.cli.main.capture_cli_install_terminal") as capture, + ): + result = runner.invoke( + app, + ["install", "--target", str(target_dir), "--shell", "zsh", "--update-path"], + env={"PATH": "/usr/bin:/bin", "HOME": str(tmp_path), "SHELL": "/bin/zsh"}, + ) + + assert result.exit_code == 1 + assert (target_dir / "kickstart").exists() + capture.assert_called_once() + assert capture.call_args.args[0] is CliInstallOutcome.PARTIAL_SUCCESS + assert capture.call_args.args[1] is CliInstallErrorCategory.PATH_UPDATE + assert capture.call_args.args[2] is CliArtifactKind.SINGLE_FILE + + +def test_install_command_captures_interrupt_before_binary_change(runner, tmp_path): + with ( + patch("src.cli.main.current_binary_path", side_effect=KeyboardInterrupt), + patch("src.cli.main.capture_cli_install_terminal") as capture, + ): + result = runner.invoke( + app, + ["install", "--target", str(tmp_path / "user-bin"), "--shell", "zsh"], + env={"PATH": "/usr/bin:/bin", "HOME": str(tmp_path), "SHELL": "/bin/zsh"}, + ) + + assert result.exit_code != 0 + capture.assert_called_once() + assert capture.call_args.args[0] is CliInstallOutcome.CANCELLED + assert capture.call_args.args[1] is CliInstallErrorCategory.INTERRUPTED def test_uninstall_command_removes_binary(runner, tmp_path): diff --git a/tests/unit/cli/test_telemetry_cli.py b/tests/unit/cli/test_telemetry_cli.py index 5ed0b1a..7e3c91b 100644 --- a/tests/unit/cli/test_telemetry_cli.py +++ b/tests/unit/cli/test_telemetry_cli.py @@ -12,12 +12,17 @@ def _env(tmp_path: Path) -> dict[str, str]: return { "HOME": str(tmp_path), + "KICKSTART_TELEMETRY_ALLOW_DEVELOPMENT": "1", "POSTHOG_PUBLIC_CUSTOMER_API_TOKEN": "", "XDG_CONFIG_HOME": str(tmp_path), } -def test_status_is_read_only_before_opt_in(tmp_path: Path) -> None: +def test_status_is_read_only_for_default_enabled_missing_state( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.delenv("PYTEST_CURRENT_TEST", raising=False) runner = CliRunner() result = runner.invoke(app, ["telemetry", "status", "--json"], env=_env(tmp_path)) @@ -26,7 +31,8 @@ def test_status_is_read_only_before_opt_in(tmp_path: Path) -> None: payload = json.loads(result.stdout) assert payload["consent"] == "unset" assert payload["delivery_configured"] is False - assert payload["effective"] is False + assert payload["effective"] is True + assert payload["reason"] == "default_enabled" assert payload["anonymous_id"] is None assert not (tmp_path / "kickstart" / "telemetry.json").exists() @@ -41,16 +47,21 @@ def test_status_reports_runtime_delivery_configuration_without_exposing_token(tm payload = json.loads(result.stdout) assert payload["delivery_configured"] is True assert "phc_status_test_token" not in result.stdout + assert not (tmp_path / "kickstart" / "telemetry.json").exists() -def test_status_supports_human_readable_output(tmp_path: Path) -> None: +def test_status_supports_human_readable_output( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.delenv("PYTEST_CURRENT_TEST", raising=False) result = CliRunner().invoke(app, ["telemetry", "status"], env=_env(tmp_path)) assert result.exit_code == 0 assert "consent: unset" in result.stdout assert "delivery-configured: no" in result.stdout - assert "effective: disabled" in result.stdout - assert "reason:" in result.stdout + assert "effective: enabled" in result.stdout + assert "reason: default_enabled" in result.stdout assert "anonymous-id: not-created" in result.stdout assert f"state-file: {tmp_path / 'kickstart' / 'telemetry.json'}" in result.stdout diff --git a/tests/unit/model/dto/test_posthog_dto.py b/tests/unit/model/dto/test_posthog_dto.py index b43c88a..4089a35 100644 --- a/tests/unit/model/dto/test_posthog_dto.py +++ b/tests/unit/model/dto/test_posthog_dto.py @@ -5,6 +5,10 @@ from src.model.dto.posthog import PostHogCaptureRequest from src.model.dto.telemetry import ( + CliArtifactKind, + CliInstallErrorCategory, + CliInstallOutcome, + CliInstallProperties, ScaffoldCreateErrorCategory, ScaffoldCreateOutcome, ScaffoldCreateProperties, @@ -86,6 +90,44 @@ def test_request_maps_only_allowlisted_and_required_posthog_fields() -> None: } +def test_request_maps_lifecycle_events_through_the_same_provider_boundary() -> None: + envelope = TelemetryEnvelope( + event_id=EVENT_ID, + anonymous_id=ANONYMOUS_ID, + occurred_at=OCCURRED_AT, + event=TelemetryEvent( + name=TelemetryEventName.CLI_INSTALL_COMPLETED, + properties=CliInstallProperties( + cli_version="1.2.3", + outcome=CliInstallOutcome.SUCCESS, + error_category=CliInstallErrorCategory.NONE, + artifact_kind=CliArtifactKind.ONEDIR, + path_update_requested=True, + already_on_path=False, + duration_bucket=TelemetryDurationBucket.ONE_TO_FIVE_SECONDS, + platform="macos", + architecture="arm64", + ), + ), + ) + + payload = PostHogCaptureRequest("phc_public_test_token", envelope).as_payload() + + assert payload["event"] == "cli_install_completed" + assert payload["properties"] == { + "$process_person_profile": False, + "already_on_path": False, + "architecture": "arm64", + "artifact_kind": "onedir", + "cli_version": "1.2.3", + "duration_bucket": "1_to_5s", + "error_category": "none", + "outcome": "success", + "path_update_requested": True, + "platform": "macos", + } + + def test_request_repr_never_contains_project_token() -> None: request = PostHogCaptureRequest(project_token="phc_do_not_print", envelope=telemetry_envelope()) diff --git a/tests/unit/model/dto/test_telemetry_dto.py b/tests/unit/model/dto/test_telemetry_dto.py index a3615fa..8d09c1d 100644 --- a/tests/unit/model/dto/test_telemetry_dto.py +++ b/tests/unit/model/dto/test_telemetry_dto.py @@ -5,6 +5,14 @@ import pytest from src.model.dto.telemetry import ( + CliArtifactKind, + CliInstallErrorCategory, + CliInstallOutcome, + CliInstallProperties, + CliUpgradeChecksumStatus, + CliUpgradeErrorCategory, + CliUpgradeOutcome, + CliUpgradeProperties, ScaffoldCreateContext, ScaffoldCreateErrorCategory, ScaffoldCreateOutcome, @@ -14,6 +22,7 @@ TelemetryEventName, TelemetryEnvelope, ) +from src.model.dto.upgrade import normalize_target_version def _properties() -> ScaffoldCreateProperties: @@ -88,3 +97,67 @@ def test_envelope_uses_provider_neutral_identity_names() -> None: def test_create_context_cannot_carry_project_name_or_root() -> None: assert "name" not in ScaffoldCreateContext.__dataclass_fields__ assert "root" not in ScaffoldCreateContext.__dataclass_fields__ + + +def test_install_property_mapping_is_an_exact_closed_allowlist() -> None: + properties = CliInstallProperties( + cli_version="1.2.3", + outcome=CliInstallOutcome.SUCCESS, + error_category=CliInstallErrorCategory.NONE, + artifact_kind=CliArtifactKind.ONEDIR, + path_update_requested=True, + already_on_path=False, + duration_bucket=TelemetryDurationBucket.ONE_TO_FIVE_SECONDS, + platform="macos", + architecture="arm64", + ) + + assert set(properties.as_mapping()) == { + "already_on_path", + "architecture", + "artifact_kind", + "cli_version", + "duration_bucket", + "error_category", + "outcome", + "path_update_requested", + "platform", + } + + +def test_upgrade_property_mapping_is_an_exact_closed_allowlist() -> None: + properties = CliUpgradeProperties( + cli_version="1.2.3", + target_version="1.3.0", + outcome=CliUpgradeOutcome.UPDATED, + error_category=CliUpgradeErrorCategory.NONE, + checksum_status=CliUpgradeChecksumStatus.VERIFIED, + duration_bucket=TelemetryDurationBucket.FIVE_TO_THIRTY_SECONDS, + platform="linux", + architecture="x86_64", + ) + + assert set(properties.as_mapping()) == { + "architecture", + "checksum_status", + "cli_version", + "duration_bucket", + "error_category", + "outcome", + "platform", + "target_version", + } + + +@pytest.mark.parametrize( + ("value", "expected"), + [ + (None, "unknown"), + ("v1.2.3", "1.2.3"), + ("1.2.3", "1.2.3"), + ("01.2.3", "unknown"), + ("1.2.3-private", "unknown"), + ], +) +def test_upgrade_target_version_allows_only_stable_semver(value: str | None, expected: str) -> None: + assert normalize_target_version(value) == expected diff --git a/tests/unit/telemetry/test_events.py b/tests/unit/telemetry/test_events.py index dccf8f8..81feb29 100644 --- a/tests/unit/telemetry/test_events.py +++ b/tests/unit/telemetry/test_events.py @@ -5,14 +5,26 @@ import pytest from src.model.dto.telemetry import ( + CliArtifactKind, + CliInstallErrorCategory, + CliInstallOutcome, + CliUpgradeChecksumStatus, + CliUpgradeErrorCategory, + CliUpgradeOutcome, ScaffoldCreateContext, ScaffoldCreateErrorCategory, ScaffoldCreateOutcome, TelemetryDurationBucket, ) +from src.model.dto.upgrade import UpgradeResult from src.telemetry.events import ( + build_cli_install_event, + build_cli_upgrade_event, build_scaffold_create_event, + capture_cli_install_terminal, + capture_cli_upgrade_terminal, capture_scaffold_create_terminal, + classify_cli_install_exception, classify_scaffold_create_exception, ) from src.telemetry.reporter import TelemetryReporter @@ -45,6 +57,149 @@ def _service_context() -> ScaffoldCreateContext: ) +def test_install_event_contains_only_normalized_closed_properties() -> None: + event = build_cli_install_event( + CliInstallOutcome.SUCCESS, + CliInstallErrorCategory.NONE, + CliArtifactKind.SINGLE_FILE, + path_update_requested=False, + already_on_path=True, + duration_seconds=0.5, + cli_version="1.2.3", + platform_name="Darwin", + architecture="aarch64", + ) + + assert event.name.value == "cli_install_completed" + assert event.properties.as_mapping() == { + "already_on_path": True, + "architecture": "arm64", + "artifact_kind": "single_file", + "cli_version": "1.2.3", + "duration_bucket": "under_1s", + "error_category": "none", + "outcome": "success", + "path_update_requested": False, + "platform": "macos", + } + + +def test_upgrade_event_replaces_non_stable_target_version_with_unknown() -> None: + result = UpgradeResult( + target_version="v1.2.3-private-value", + outcome=CliUpgradeOutcome.FAILED, + error_category=CliUpgradeErrorCategory.ARCHIVE_MISSING, + checksum_status=CliUpgradeChecksumStatus.NOT_REACHED, + ) + + event = build_cli_upgrade_event( + result, + 6, + cli_version="1.2.2", + platform_name="Linux", + architecture="amd64", + ) + + assert event.name.value == "cli_upgrade_completed" + assert event.properties.as_mapping() == { + "architecture": "x86_64", + "checksum_status": "not_reached", + "cli_version": "1.2.2", + "duration_bucket": "5_to_30s", + "error_category": "archive_missing", + "outcome": "failed", + "platform": "linux", + "target_version": "unknown", + } + + +@pytest.mark.parametrize( + ("error", "during_path_update", "expected"), + [ + (RuntimeError("private detail"), True, CliInstallErrorCategory.PATH_UPDATE), + (FileNotFoundError("private detail"), False, CliInstallErrorCategory.SOURCE_MISSING), + (FileExistsError("private detail"), False, CliInstallErrorCategory.DESTINATION_CONFLICT), + (PermissionError("private detail"), False, CliInstallErrorCategory.PERMISSION_DENIED), + (OSError("private detail"), False, CliInstallErrorCategory.FILESYSTEM_ERROR), + (KickstartError("private detail"), False, CliInstallErrorCategory.EXPECTED_ERROR), + (RuntimeError("private detail"), False, CliInstallErrorCategory.UNEXPECTED_ERROR), + ], +) +def test_install_exception_classification_never_uses_exception_text( + error: Exception, + during_path_update: bool, + expected: CliInstallErrorCategory, +) -> None: + assert classify_cli_install_exception(error, during_path_update=during_path_update) is expected + + +def test_lifecycle_capture_helpers_record_through_the_shared_reporter(tmp_path: Path) -> None: + store = TelemetryStateStore(tmp_path / "telemetry.json") + sink = InMemoryTelemetrySink() + reporter = TelemetryReporter(store, sink, {}, development=False) + upgrade = UpgradeResult( + target_version="1.2.3", + outcome=CliUpgradeOutcome.UPDATED, + error_category=CliUpgradeErrorCategory.NONE, + checksum_status=CliUpgradeChecksumStatus.VERIFIED, + ) + + capture_cli_install_terminal( + CliInstallOutcome.SUCCESS, + CliInstallErrorCategory.NONE, + CliArtifactKind.ONEDIR, + path_update_requested=True, + already_on_path=False, + duration_seconds=1, + cli_version="1.2.2", + reporter=reporter, + ) + capture_cli_upgrade_terminal( + upgrade, + 2, + cli_version="1.2.2", + reporter=reporter, + ) + + assert [envelope.event.name.value for envelope in sink.envelopes] == [ + "cli_install_completed", + "cli_upgrade_completed", + ] + assert sink.envelopes[0].anonymous_id == sink.envelopes[1].anonymous_id + + +@pytest.mark.parametrize("capture", ["install", "upgrade"]) +def test_lifecycle_capture_helpers_contain_reporter_failures(capture: str) -> None: + reporter = Mock(spec=TelemetryReporter) + reporter.record.side_effect = RuntimeError("synthetic reporter failure") + + if capture == "install": + capture_cli_install_terminal( + CliInstallOutcome.FAILED, + CliInstallErrorCategory.UNEXPECTED_ERROR, + CliArtifactKind.UNKNOWN, + path_update_requested=False, + already_on_path=False, + duration_seconds=0, + cli_version="1.2.3", + reporter=reporter, + ) + else: + capture_cli_upgrade_terminal( + UpgradeResult( + target_version="unknown", + outcome=CliUpgradeOutcome.FAILED, + error_category=CliUpgradeErrorCategory.UNEXPECTED_ERROR, + checksum_status=CliUpgradeChecksumStatus.NOT_REACHED, + ), + 0, + cli_version="1.2.3", + reporter=reporter, + ) + + reporter.record.assert_called_once() + + def test_service_event_canonicalizes_aliases_and_ignores_inapplicable_values() -> None: event = build_scaffold_create_event( _service_context(), diff --git a/tests/unit/telemetry/test_policy.py b/tests/unit/telemetry/test_policy.py index addfa5d..993b932 100644 --- a/tests/unit/telemetry/test_policy.py +++ b/tests/unit/telemetry/test_policy.py @@ -32,17 +32,28 @@ def test_hard_opt_outs_precede_persisted_consent( assert resolved.anonymous_id is None -def test_missing_state_is_disabled_without_creating_an_identity(tmp_path: Path) -> None: +def test_missing_state_is_default_enabled_without_creating_an_identity(tmp_path: Path) -> None: store = TelemetryStateStore(tmp_path / "telemetry.json") resolved = resolve_telemetry(store, {}, development=False) - assert resolved.enabled is False - assert resolved.reason is TelemetrySuppressionReason.NOT_OPTED_IN + assert resolved.enabled is True + assert resolved.reason is TelemetrySuppressionReason.DEFAULT_ENABLED assert resolved.anonymous_id is None assert not store.path.exists() +def test_explicit_disable_is_a_hard_opt_out(tmp_path: Path) -> None: + store = TelemetryStateStore(tmp_path / "telemetry.json") + store.disable() + + resolved = resolve_telemetry(store, {}, development=False) + + assert resolved.enabled is False + assert resolved.reason is TelemetrySuppressionReason.USER_DISABLED + assert resolved.anonymous_id is None + + def test_enabled_state_is_effective_outside_suppressed_contexts(tmp_path: Path) -> None: store = TelemetryStateStore(tmp_path / "telemetry.json") state = store.enable() diff --git a/tests/unit/telemetry/test_reporter.py b/tests/unit/telemetry/test_reporter.py index 10c8517..ca64238 100644 --- a/tests/unit/telemetry/test_reporter.py +++ b/tests/unit/telemetry/test_reporter.py @@ -1,8 +1,11 @@ from datetime import UTC, datetime from pathlib import Path +from unittest.mock import patch from uuid import UUID -from src.model.dto.telemetry import TelemetryEvent +import pytest + +from src.model.dto.telemetry import TelemetryConsent, TelemetryEvent from src.telemetry.posthog import PostHogSink from src.telemetry.reporter import TelemetryReporter, default_telemetry_reporter from src.telemetry.sink import InMemoryTelemetrySink, NoOpTelemetrySink @@ -46,10 +49,11 @@ def test_reporter_adds_stable_identity_and_delivery_metadata(tmp_path: Path) -> assert sink.envelopes[0].event is event -def test_reporter_does_nothing_before_opt_in(tmp_path: Path) -> None: +def test_reporter_creates_and_persists_identity_for_first_default_on_event(tmp_path: Path) -> None: + store = TelemetryStateStore(tmp_path / "telemetry.json") sink = InMemoryTelemetrySink() reporter = TelemetryReporter( - state_store=TelemetryStateStore(tmp_path / "telemetry.json"), + state_store=store, sink=sink, environ={}, development=False, @@ -57,27 +61,84 @@ def test_reporter_does_nothing_before_opt_in(tmp_path: Path) -> None: reporter.record(_event()) + persisted = store.read() + assert persisted.consent is TelemetryConsent.ENABLED + assert persisted.anonymous_id is not None + assert len(sink.envelopes) == 1 + assert sink.envelopes[0].anonymous_id == persisted.anonymous_id + + +@pytest.mark.parametrize( + "environ", + [ + {"DO_NOT_TRACK": "1"}, + {"KICKSTART_TELEMETRY_DISABLED": "1"}, + {"CI": "1"}, + {"PYTEST_CURRENT_TEST": "test"}, + {"KICKSTART_EVAL": "1"}, + ], +) +def test_hard_suppression_does_not_create_state_or_send( + tmp_path: Path, + environ: dict[str, str], +) -> None: + store = TelemetryStateStore(tmp_path / "telemetry.json") + sink = InMemoryTelemetrySink() + reporter = TelemetryReporter(store, sink, environ, development=False) + + reporter.record(_event()) + assert sink.envelopes == [] + assert not store.path.exists() + + +def test_explicit_disable_does_not_create_identity_or_send(tmp_path: Path) -> None: + store = TelemetryStateStore(tmp_path / "telemetry.json") + store.disable() + sink = InMemoryTelemetrySink() + + TelemetryReporter(store, sink, {}, development=False).record(_event()) + + assert sink.envelopes == [] + persisted = store.read() + assert persisted.consent is TelemetryConsent.DISABLED + assert persisted.anonymous_id is None + + +def test_disable_between_policy_and_identity_acquisition_wins(tmp_path: Path) -> None: + class DisableBeforeIdentityStore(TelemetryStateStore): + def identity_for_event(self) -> UUID | None: + self.disable() + return super().identity_for_event() + + store = DisableBeforeIdentityStore(tmp_path / "telemetry.json") + sink = InMemoryTelemetrySink() + + TelemetryReporter(store, sink, {}, development=False).record(_event()) + + assert sink.envelopes == [] + assert store.read().consent is TelemetryConsent.DISABLED def test_reporter_contains_sink_failures(tmp_path: Path) -> None: store = TelemetryStateStore(tmp_path / "telemetry.json") - store.enable() reporter = TelemetryReporter(store, FailingSink(), {}, development=False) reporter.record(_event()) + assert store.read().anonymous_id is not None + def test_default_reporter_without_token_uses_noop_sink(tmp_path: Path) -> None: reporter = default_telemetry_reporter({"XDG_CONFIG_HOME": str(tmp_path)}, development=False) - reporter.state_store.enable() reporter.record(_event()) assert isinstance(reporter.sink, NoOpTelemetrySink) + assert not reporter.state_store.path.exists() -def test_environment_token_does_not_enable_telemetry(tmp_path: Path) -> None: +def test_environment_token_routes_default_on_event_without_real_network(tmp_path: Path) -> None: state_path = tmp_path / "kickstart" / "telemetry.json" reporter = default_telemetry_reporter( { @@ -87,7 +148,33 @@ def test_environment_token_does_not_enable_telemetry(tmp_path: Path) -> None: development=False, ) - reporter.record(_event()) + with patch.object(PostHogSink, "record") as record: + reporter.record(_event()) assert isinstance(reporter.sink, PostHogSink) - assert not state_path.exists() + record.assert_called_once() + assert TelemetryStateStore(state_path).read().anonymous_id is not None + + +def test_invalid_state_fails_closed_without_sending(tmp_path: Path) -> None: + state_path = tmp_path / "telemetry.json" + state_path.write_text("{bad json", encoding="utf-8") + sink = InMemoryTelemetrySink() + + TelemetryReporter(TelemetryStateStore(state_path), sink, {}, development=False).record(_event()) + + assert sink.envelopes == [] + assert state_path.read_text(encoding="utf-8") == "{bad json" + + +def test_dangling_state_symlink_fails_closed_without_replacement(tmp_path: Path) -> None: + state_path = tmp_path / "telemetry.json" + missing_target = tmp_path / "missing-state.json" + state_path.symlink_to(missing_target) + sink = InMemoryTelemetrySink() + + TelemetryReporter(TelemetryStateStore(state_path), sink, {}, development=False).record(_event()) + + assert sink.envelopes == [] + assert state_path.is_symlink() + assert not missing_target.exists() diff --git a/tests/unit/telemetry/test_state.py b/tests/unit/telemetry/test_state.py index 6714862..34e1928 100644 --- a/tests/unit/telemetry/test_state.py +++ b/tests/unit/telemetry/test_state.py @@ -35,6 +35,30 @@ def test_reading_missing_state_is_lazy(tmp_path: Path) -> None: assert not store.path.exists() +def test_dangling_state_symlink_is_rejected_instead_of_treated_as_missing(tmp_path: Path) -> None: + path = tmp_path / "telemetry.json" + missing_target = tmp_path / "missing-state.json" + path.symlink_to(missing_target) + + with pytest.raises(TelemetryStateError, match="unreadable or malformed"): + TelemetryStateStore(path).read() + + assert path.is_symlink() + assert not missing_target.exists() + + +def test_state_metadata_error_fails_closed(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + path = tmp_path / "telemetry.json" + + def fail_lstat(_path: Path) -> os.stat_result: + raise OSError("synthetic metadata failure") + + monkeypatch.setattr(Path, "lstat", fail_lstat) + + with pytest.raises(TelemetryStateError, match="unreadable or malformed"): + TelemetryStateStore(path).read() + + def test_enable_creates_uuid4_and_reuses_it_across_store_instances(tmp_path: Path) -> None: path = tmp_path / "telemetry.json" @@ -47,6 +71,18 @@ def test_enable_creates_uuid4_and_reuses_it_across_store_instances(tmp_path: Pat assert second.consent is TelemetryConsent.ENABLED +def test_event_identity_is_created_once_and_reused_across_store_instances(tmp_path: Path) -> None: + path = tmp_path / "telemetry.json" + + first = TelemetryStateStore(path).identity_for_event() + second = TelemetryStateStore(path).identity_for_event() + + assert first is not None + assert first.version == 4 + assert second == first + assert TelemetryStateStore(path).read().anonymous_id == first + + def test_disable_before_enable_does_not_create_identity(tmp_path: Path) -> None: store = TelemetryStateStore(tmp_path / "telemetry.json") @@ -57,6 +93,17 @@ def test_disable_before_enable_does_not_create_identity(tmp_path: Path) -> None: assert "anonymous_id" not in json.loads(store.path.read_text(encoding="utf-8")) +def test_disabled_state_prevents_event_identity_creation(tmp_path: Path) -> None: + store = TelemetryStateStore(tmp_path / "telemetry.json") + store.disable() + + assert store.identity_for_event() is None + + persisted = store.read() + assert persisted.consent is TelemetryConsent.DISABLED + assert persisted.anonymous_id is None + + def test_disable_and_reenable_preserve_identity(tmp_path: Path) -> None: store = TelemetryStateStore(tmp_path / "telemetry.json") enabled = store.enable() @@ -92,16 +139,17 @@ def test_reset_without_identity_is_read_only(tmp_path: Path) -> None: assert not store.path.exists() -def test_concurrent_enablement_converges_on_one_identity(tmp_path: Path) -> None: +def test_concurrent_event_identity_creation_converges_on_one_identity(tmp_path: Path) -> None: path = tmp_path / "telemetry.json" candidates = iter([uuid4(), uuid4()]) - def enable() -> UUID | None: - return TelemetryStateStore(path, uuid_factory=lambda: next(candidates)).enable().anonymous_id + def identity_for_event() -> UUID | None: + return TelemetryStateStore(path, uuid_factory=lambda: next(candidates)).identity_for_event() with ThreadPoolExecutor(max_workers=2) as executor: - identities = list(executor.map(lambda _: enable(), range(2))) + identities = list(executor.map(lambda _: identity_for_event(), range(2))) + assert identities[0] is not None assert identities[0] == identities[1] @@ -109,7 +157,7 @@ def enable() -> UUID | None: def test_state_file_uses_user_only_permissions(tmp_path: Path) -> None: path = tmp_path / "kickstart" / "telemetry.json" - TelemetryStateStore(path).enable() + TelemetryStateStore(path).identity_for_event() assert stat.S_IMODE(path.stat().st_mode) == 0o600 assert stat.S_IMODE(path.parent.stat().st_mode) == 0o700 @@ -121,9 +169,11 @@ def test_state_write_succeeds_when_descriptor_chmod_is_unavailable( ) -> None: monkeypatch.delattr(state_module.os, "fchmod", raising=False) - state = TelemetryStateStore(tmp_path / "telemetry.json").enable() + store = TelemetryStateStore(tmp_path / "telemetry.json") + identity = store.identity_for_event() - assert state.consent is TelemetryConsent.ENABLED + assert identity is not None + assert store.read().consent is TelemetryConsent.ENABLED def test_stale_lock_is_removed_before_updating_state(tmp_path: Path) -> None: @@ -145,7 +195,7 @@ def test_busy_lock_fails_without_changing_state(tmp_path: Path, monkeypatch: pyt monkeypatch.setattr(state_module, "_LOCK_TIMEOUT_SECONDS", 0.0) with pytest.raises(TelemetryStateError, match="Telemetry state is busy"): - TelemetryStateStore(path).enable() + TelemetryStateStore(path).identity_for_event() assert not path.exists() @@ -159,7 +209,7 @@ def fail_replace(_source: Path, _destination: Path) -> None: monkeypatch.setattr(state_module.os, "replace", fail_replace) with pytest.raises(TelemetryStateError, match="cannot be written"): - TelemetryStateStore(path).enable() + TelemetryStateStore(path).identity_for_event() assert list(tmp_path.glob(".telemetry-*.tmp")) == [] diff --git a/tests/unit/utils/test_updater.py b/tests/unit/utils/test_updater.py index b9cbf30..bee90ab 100644 --- a/tests/unit/utils/test_updater.py +++ b/tests/unit/utils/test_updater.py @@ -274,7 +274,11 @@ def test_check_for_update_already_up_to_date(mock_print): response.raise_for_status.return_value = None response.json.return_value = payload with patch("src.utils.updater.requests.get", return_value=response): - updater.check_for_update() + result = updater.check_for_update() + assert result.target_version == "1.0.0" + assert result.outcome.value == "already_current" + assert result.error_category.value == "none" + assert result.checksum_status.value == "not_reached" messages = [c.args[0] for c in mock_print.call_args_list] assert any("You're already up to date" in m for m in messages) @@ -291,7 +295,10 @@ def test_check_for_update_missing_archive_for_host(mock_print, monkeypatch): response.raise_for_status.return_value = None response.json.return_value = payload with patch("src.utils.updater.requests.get", return_value=response): - updater.check_for_update() + result = updater.check_for_update() + assert result.target_version == "1.1.0" + assert result.outcome.value == "failed" + assert result.error_category.value == "archive_missing" messages = [c.args[0] for c in mock_print.call_args_list] assert any("no asset named kickstart-macos-arm64-py3.14.tar.gz" in m for m in messages) @@ -300,7 +307,10 @@ def test_check_for_update_missing_archive_for_host(mock_print, monkeypatch): @patch("builtins.print") def test_check_for_update_release_lookup_failure(mock_print): with patch("src.utils.updater.requests.get", side_effect=requests.ConnectionError("offline")): - updater.check_for_update() + result = updater.check_for_update() + assert result.target_version == "unknown" + assert result.outcome.value == "failed" + assert result.error_category.value == "release_lookup" messages = [c.args[0] for c in mock_print.call_args_list] assert any("Update failed: could not query the latest release" in m for m in messages) @@ -317,6 +327,115 @@ def test_check_for_update_invalid_json(mock_print): assert any("Update failed" in m for m in messages) +@patch("src.utils.updater.__version__", "1.0.0") +@patch("builtins.print") +def test_check_for_update_invalid_release_metadata_is_categorized(mock_print): + response = MagicMock() + response.raise_for_status.return_value = None + response.json.return_value = {} + + with patch("src.utils.updater.requests.get", return_value=response): + result = updater.check_for_update() + + assert result.target_version == "unknown" + assert result.outcome.value == "failed" + assert result.error_category.value == "invalid_release_metadata" + assert result.checksum_status.value == "not_reached" + messages = [call.args[0] for call in mock_print.call_args_list] + assert any("release metadata is invalid" in message for message in messages) + + +@patch("src.utils.updater.__version__", "1.0.0") +@patch("builtins.print") +def test_check_for_update_unsupported_platform_is_categorized(mock_print): + payload = _release_payload("v1.1.0", "kickstart-linux-x64-py3.14.tar.gz") + + with ( + patch("src.utils.updater.fetch_release_info", return_value=payload), + patch("src.utils.updater.expected_archive_name", side_effect=RuntimeError("private detail")), + ): + result = updater.check_for_update() + + assert result.error_category.value == "unsupported_platform" + assert result.target_version == "1.1.0" + + +@patch("src.utils.updater.__version__", "1.0.0") +@patch("builtins.print") +def test_check_for_update_download_failure_is_categorized(mock_print, tmp_path): + archive_name = "kickstart-linux-x64-py3.14.tar.gz" + payload = _release_payload("v1.1.0", archive_name, with_sha=False) + + with ( + patch("src.utils.updater.fetch_release_info", return_value=payload), + patch("src.utils.updater.expected_archive_name", return_value=archive_name), + patch("src.utils.updater.resolve_current_install_layout", return_value=(tmp_path, None)), + patch("src.utils.updater.download_to", side_effect=requests.ConnectionError("private detail")), + ): + result = updater.check_for_update() + + assert result.error_category.value == "download" + assert result.checksum_status.value == "not_reached" + + +@patch("src.utils.updater.__version__", "1.0.0") +@patch("builtins.print") +def test_check_for_update_checksum_fetch_failure_is_categorized(mock_print, tmp_path): + archive_name = "kickstart-linux-x64-py3.14.tar.gz" + payload = _release_payload("v1.1.0", archive_name) + + with ( + patch("src.utils.updater.fetch_release_info", return_value=payload), + patch("src.utils.updater.expected_archive_name", return_value=archive_name), + patch("src.utils.updater.resolve_current_install_layout", return_value=(tmp_path, None)), + patch("src.utils.updater.download_to"), + patch("src.utils.updater.requests.get", side_effect=requests.ConnectionError("private detail")), + ): + result = updater.check_for_update() + + assert result.error_category.value == "checksum_fetch" + assert result.checksum_status.value == "failed" + + +@patch("src.utils.updater.__version__", "1.0.0") +@patch("builtins.print") +def test_check_for_update_extraction_failure_preserves_missing_checksum_status(mock_print, tmp_path): + archive_name = "kickstart-linux-x64-py3.14.tar.gz" + payload = _release_payload("v1.1.0", archive_name, with_sha=False) + + with ( + patch("src.utils.updater.fetch_release_info", return_value=payload), + patch("src.utils.updater.expected_archive_name", return_value=archive_name), + patch("src.utils.updater.resolve_current_install_layout", return_value=(tmp_path, None)), + patch("src.utils.updater.download_to"), + patch("src.utils.updater._extract_archive_launcher", side_effect=RuntimeError("private detail")), + ): + result = updater.check_for_update() + + assert result.error_category.value == "archive_extraction" + assert result.checksum_status.value == "not_published" + + +@patch("src.utils.updater.__version__", "1.0.0") +@patch("builtins.print") +def test_check_for_update_install_failure_is_categorized(mock_print, tmp_path): + archive_name = "kickstart-linux-x64-py3.14.tar.gz" + payload = _release_payload("v1.1.0", archive_name, with_sha=False) + + with ( + patch("src.utils.updater.fetch_release_info", return_value=payload), + patch("src.utils.updater.expected_archive_name", return_value=archive_name), + patch("src.utils.updater.resolve_current_install_layout", return_value=(tmp_path, None)), + patch("src.utils.updater.download_to"), + patch("src.utils.updater._extract_archive_launcher", return_value=tmp_path / "kickstart"), + patch("src.utils.updater.install_binary", side_effect=OSError("private detail")), + ): + result = updater.check_for_update() + + assert result.error_category.value == "installation" + assert result.checksum_status.value == "not_published" + + @patch("src.utils.updater.__version__", "1.0.0") @patch("builtins.print") def test_check_for_update_end_to_end_installs_bundle(mock_print, tmp_path, monkeypatch): @@ -364,7 +483,12 @@ def fake_get(url, **kwargs): monkeypatch.setattr(updater, "current_binary_path", lambda: launcher) with patch("src.utils.updater.requests.get", side_effect=fake_get): - updater.check_for_update() + result = updater.check_for_update() + + assert result.target_version == "1.1.0" + assert result.outcome.value == "updated" + assert result.error_category.value == "none" + assert result.checksum_status.value == "verified" # Verify the launcher now points at the freshly-extracted bundle. assert launcher.is_symlink() @@ -407,7 +531,12 @@ def fake_get(url, **kwargs): monkeypatch.setattr(updater, "current_binary_path", lambda: tmp_path / "bin" / "kickstart") with patch("src.utils.updater.requests.get", side_effect=fake_get): - updater.check_for_update() + result = updater.check_for_update() + + assert result.target_version == "1.1.0" + assert result.outcome.value == "failed" + assert result.error_category.value == "checksum_mismatch" + assert result.checksum_status.value == "failed" messages = [c.args[0] for c in mock_print.call_args_list] assert any("could not verify checksum" in m for m in messages) From 04c639a01d8e9805b9be38b95ce5a0b40722d417 Mon Sep 17 00:00:00 2001 From: Jean-Michel Bouchard Date: Sun, 19 Jul 2026 18:02:11 -0400 Subject: [PATCH 09/11] Isolate telemetry status tests from CI suppression The default-enabled status tests deliberately remove pytest suppression, so also neutralize GitHub Actions' inherited CI=true value.\n\nRefs ENG-198\nRefs ENG-201\n\nTests: CI=true make check (781 passed) --- tests/unit/cli/test_telemetry_cli.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/unit/cli/test_telemetry_cli.py b/tests/unit/cli/test_telemetry_cli.py index 7e3c91b..8214b95 100644 --- a/tests/unit/cli/test_telemetry_cli.py +++ b/tests/unit/cli/test_telemetry_cli.py @@ -11,6 +11,7 @@ def _env(tmp_path: Path) -> dict[str, str]: return { + "CI": "0", "HOME": str(tmp_path), "KICKSTART_TELEMETRY_ALLOW_DEVELOPMENT": "1", "POSTHOG_PUBLIC_CUSTOMER_API_TOKEN": "", From 24d1076f31148d5cffc3595efe2233aeb07a305b Mon Sep 17 00:00:00 2001 From: Jean-Michel Bouchard Date: Sun, 19 Jul 2026 18:13:31 -0400 Subject: [PATCH 10/11] audit-round-1: align install PATH outcomes (finding 2) Classify PATH-only changes as success, unresolved requested PATH integration as partial success, and path-phase failures as partial whenever a usable binary is already present. Document and test each boundary.\n\nRefs ENG-203\n\nTests: CI=true make check (784 passed)\nCoverage: CI=true make coverage (95.8%) --- docs/contracts/telemetry.md | 2 + src/cli/main.py | 20 ++++++---- tests/unit/cli/test_main.py | 79 +++++++++++++++++++++++++++++++++++++ 3 files changed, 94 insertions(+), 7 deletions(-) diff --git a/docs/contracts/telemetry.md b/docs/contracts/telemetry.md index 6f269d6..729bef4 100644 --- a/docs/contracts/telemetry.md +++ b/docs/contracts/telemetry.md @@ -124,6 +124,8 @@ The exact closed property set for `cli_install_completed` is those six common pr Its `outcome` is one of `success`, `no_change`, `partial_success`, `failed`, or `cancelled`. Its `error_category` is one of `none`, `interrupted`, `source_missing`, `destination_conflict`, `permission_denied`, `path_update`, `filesystem_error`, `expected_error`, or `unexpected_error`. +A successful PATH-only change is `success` even when the binary was already installed. If PATH integration was requested but no shell rc file could be resolved, the command remains non-fatal but reports `partial_success` with `path_update`; a request that changes neither the binary nor the managed PATH block is `no_change`. + The exact closed property set for `cli_upgrade_completed` is those six common properties plus: - `target_version`: a stable semantic version or `unknown` diff --git a/src/cli/main.py b/src/cli/main.py index f4995a8..2109337 100644 --- a/src/cli/main.py +++ b/src/cli/main.py @@ -332,7 +332,7 @@ def install( error_category = CliInstallErrorCategory.UNEXPECTED_ERROR artifact_kind = CliArtifactKind.UNKNOWN already_on_path = False - binary_changed = False + binary_ready = False during_path_update = False try: @@ -355,7 +355,7 @@ def install( artifact_kind = ( CliArtifactKind.ONEDIR if result.app_path is not None else CliArtifactKind.SINGLE_FILE ) - binary_changed = result.copied + binary_ready = True outcome = CliInstallOutcome.NO_CHANGE if result.already_installed else CliInstallOutcome.SUCCESS error_category = CliInstallErrorCategory.NONE if result.already_installed: @@ -370,17 +370,22 @@ def install( if update_path: during_path_update = True - _apply_path_update(ctx) + path_update = _apply_path_update(ctx) during_path_update = False + if path_update is None: + outcome = CliInstallOutcome.PARTIAL_SUCCESS + error_category = CliInstallErrorCategory.PATH_UPDATE + elif path_update.changed: + outcome = CliInstallOutcome.SUCCESS return _print_manual_path_instructions(ctx) except KeyboardInterrupt: - outcome = CliInstallOutcome.PARTIAL_SUCCESS if binary_changed else CliInstallOutcome.CANCELLED + outcome = CliInstallOutcome.PARTIAL_SUCCESS if binary_ready else CliInstallOutcome.CANCELLED error_category = CliInstallErrorCategory.INTERRUPTED raise except Exception as exc: - outcome = CliInstallOutcome.PARTIAL_SUCCESS if binary_changed else CliInstallOutcome.FAILED + outcome = CliInstallOutcome.PARTIAL_SUCCESS if binary_ready else CliInstallOutcome.FAILED error_category = classify_cli_install_exception( exc, during_path_update=during_path_update, @@ -458,7 +463,7 @@ def _print_install_status(ctx: _InstallContext, source: Path) -> None: print(f" rc file: {ctx.rc_file}") -def _apply_path_update(ctx: _InstallContext) -> None: +def _apply_path_update(ctx: _InstallContext) -> PathUpdateResult | None: """Write (or refresh) the managed PATH block in the resolved rc file, or fall back to instructions.""" if ctx.rc_file is None: print( @@ -466,7 +471,7 @@ def _apply_path_update(ctx: _InstallContext) -> None: "or [bold]--shell bash|zsh|fish[/bold].[/yellow]" ) _print_manual_path_instructions(ctx, suppress_rc_hint=True) - return + return None changed = update_path_in_rc(ctx.rc_file, ctx.snippet) update = PathUpdateResult(rc_file=ctx.rc_file, snippet=ctx.snippet, changed=changed, on_path_now=False) if update.changed: @@ -474,6 +479,7 @@ def _apply_path_update(ctx: _InstallContext) -> None: else: print(f"[green]✔ {update.rc_file} already contains the managed PATH entry.[/]") print(f"[cyan]Restart your shell or run [bold]source {update.rc_file}[/bold] to pick up the new PATH.[/cyan]") + return update def _clean_managed_path_block(ctx: _InstallContext) -> None: diff --git a/tests/unit/cli/test_main.py b/tests/unit/cli/test_main.py index d856dc0..5411eac 100644 --- a/tests/unit/cli/test_main.py +++ b/tests/unit/cli/test_main.py @@ -800,6 +800,85 @@ def test_install_command_reports_partial_success_when_path_update_fails(runner, assert capture.call_args.args[2] is CliArtifactKind.SINGLE_FILE +def test_install_command_reports_partial_success_when_path_update_fails_after_binary_noop(runner, tmp_path): + target_dir = tmp_path / "user-bin" + target_dir.mkdir() + source = target_dir / "kickstart" + source.write_text("#!/bin/sh\n") + source.chmod(0o755) + + with ( + patch("src.cli.main.current_binary_path", return_value=source), + patch("src.cli.main._apply_path_update", side_effect=OSError("private detail")), + patch("src.cli.main.capture_cli_install_terminal") as capture, + ): + result = runner.invoke( + app, + ["install", "--target", str(target_dir), "--shell", "zsh", "--update-path"], + env={"PATH": "/usr/bin:/bin", "HOME": str(tmp_path), "SHELL": "/bin/zsh"}, + ) + + assert result.exit_code == 1 + capture.assert_called_once() + assert capture.call_args.args[0] is CliInstallOutcome.PARTIAL_SUCCESS + assert capture.call_args.args[1] is CliInstallErrorCategory.PATH_UPDATE + + +def test_install_command_reports_success_for_path_only_change(runner, tmp_path): + target_dir = tmp_path / "user-bin" + target_dir.mkdir() + source = target_dir / "kickstart" + source.write_text("#!/bin/sh\n") + source.chmod(0o755) + rc_file = tmp_path / ".zshrc" + + with ( + patch("src.cli.main.current_binary_path", return_value=source), + patch("src.cli.main.capture_cli_install_terminal") as capture, + ): + result = runner.invoke( + app, + [ + "install", + "--target", + str(target_dir), + "--rc-file", + str(rc_file), + "--shell", + "zsh", + "--update-path", + ], + env={"PATH": "/usr/bin:/bin", "HOME": str(tmp_path), "SHELL": "/bin/zsh"}, + ) + + assert result.exit_code == 0, result.stdout + assert rc_file.exists() + capture.assert_called_once() + assert capture.call_args.args[0] is CliInstallOutcome.SUCCESS + assert capture.call_args.args[1] is CliInstallErrorCategory.NONE + + +def test_install_command_reports_partial_success_when_path_update_cannot_be_resolved(runner, tmp_path): + source = _make_single_file_binary(tmp_path) + target_dir = tmp_path / "user-bin" + + with ( + patch("src.cli.main.current_binary_path", return_value=source), + patch("src.cli.main.capture_cli_install_terminal") as capture, + ): + result = runner.invoke( + app, + ["install", "--target", str(target_dir), "--update-path"], + env={"PATH": "/usr/bin:/bin", "HOME": str(tmp_path), "SHELL": "/bin/unknown"}, + ) + + assert result.exit_code == 0, result.stdout + assert "Could not infer a shell rc file" in result.stdout + capture.assert_called_once() + assert capture.call_args.args[0] is CliInstallOutcome.PARTIAL_SUCCESS + assert capture.call_args.args[1] is CliInstallErrorCategory.PATH_UPDATE + + def test_install_command_captures_interrupt_before_binary_change(runner, tmp_path): with ( patch("src.cli.main.current_binary_path", side_effect=KeyboardInterrupt), From 4de5c85a362b0b0a68a2151fec0b9df4773d665b Mon Sep 17 00:00:00 2001 From: Jean-Michel Bouchard Date: Sun, 19 Jul 2026 19:20:47 -0400 Subject: [PATCH 11/11] audit-round-2: make install path implementation-neutral (finding 1) Correct the inherited ENG-10 wording without changing updater behavior. The existing nesting defect is tracked separately as ENG-204. Tests: CI=true make check (784 passed) --- docs/operations/install-binaries.md | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/docs/operations/install-binaries.md b/docs/operations/install-binaries.md index 835b7b2..3b1050c 100644 --- a/docs/operations/install-binaries.md +++ b/docs/operations/install-binaries.md @@ -144,10 +144,13 @@ on `PATH` should resolve to, so a stale entrypoint can always be diagnosed and repaired. The canonical install is the one this document describes: a launcher at -`~/.local/bin/kickstart` (symlink or `#!/bin/sh` wrapper) pointing into -`~/.local/share/kickstart/current/kickstart`, owned by `kickstart install` and -refreshed by `kickstart upgrade`. Nothing else should shadow it. In particular, -ad-hoc shims in personal `bin` directories (for example +`~/.local/bin/kickstart` (symlink or `#!/bin/sh` wrapper), owned by +`kickstart install`, which leads to the active managed payload under +`~/.local/share/kickstart`. `kickstart upgrade` refreshes that managed +installation. Treat the payload's internal path as an implementation detail +and use `kickstart install --check` to inspect it. Nothing else should shadow +the launcher. In particular, ad-hoc shims in personal `bin` directories (for +example `~/workspace/bin/kickstart`) are not supported: they bypass `kickstart upgrade`, go stale silently, and hide which binary owns the command. Delete them, or reduce them to a one-line `exec "$HOME/.local/bin/kickstart" "$@"` @@ -181,5 +184,6 @@ kickstart upgrade `kickstart upgrade` discovers the asset matching the running host's platform and Python minor, verifies its SHA-256 against the published `.sha256` file, -and re-uses the same installer code path described above to replace the -launcher and refresh the binary payload directory. +and re-uses the same installer code path described above to refresh the active +managed installation. Do not script against the payload's internal directory; +use `kickstart install --check` to inspect it.