diff --git a/docs/docs/configure/keybinds.md b/docs/docs/configure/keybinds.md index 8628446d47..dba58308f1 100644 --- a/docs/docs/configure/keybinds.md +++ b/docs/docs/configure/keybinds.md @@ -61,6 +61,7 @@ Override it in your config: | Leader + `k` | Keybind list | | Leader + `e` | Open editor | | Leader + `q` | Quit | +| `Ctrl+Y` | Toggle YOLO mode for this session (confirms when enabling; instant when disabling) | ### Input Editing @@ -114,7 +115,7 @@ All configurable keybind identifiers: ### Session -`session_export`, `session_new`, `session_list`, `session_timeline`, `session_fork`, `session_rename`, `session_delete`, `session_child_cycle`, `session_parent`, `session_share`, `session_unshare`, `session_interrupt`, `session_compact` +`session_export`, `session_new`, `session_list`, `session_timeline`, `session_fork`, `session_rename`, `session_delete`, `session_child_cycle`, `session_parent`, `session_share`, `session_unshare`, `session_interrupt`, `session_compact`, `session_yolo_toggle` ### Messages diff --git a/docs/docs/configure/permissions.md b/docs/docs/configure/permissions.md index 3b4e7e7557..852b8ac1a8 100644 --- a/docs/docs/configure/permissions.md +++ b/docs/docs/configure/permissions.md @@ -132,6 +132,8 @@ The fallback `OPENCODE_YOLO` env var is also supported. When both are set, `ALTI When yolo mode is active in the TUI, a `△ YOLO` indicator appears in the footer status bar. +**Mid-session toggle (TUI):** Press `Ctrl+Y` inside the TUI to toggle yolo mode for the current session without restarting. Enabling requires a one-tap confirmation; disabling is instant. The toggle is **session and subagent scoped and lives in memory only** — restart the CLI and yolo defaults back to whatever `--yolo`, `ALTIMATE_CLI_YOLO`, or `OPENCODE_YOLO` was at launch. Deny rules stay enforced. + ## Recommended Configurations ### Data Engineering (Default, Balanced) diff --git a/packages/opencode/src/altimate/plugin/altimate.ts b/packages/opencode/src/altimate/plugin/altimate.ts index b57b4233ef..6b61d01d32 100644 --- a/packages/opencode/src/altimate/plugin/altimate.ts +++ b/packages/opencode/src/altimate/plugin/altimate.ts @@ -8,6 +8,7 @@ import * as OnboardingTelemetry from "../telemetry/onboarding" // altimate_change — shared machine-id helper (race-safe, UUID-validated, size-capped) import { getOrCreateMachineId } from "../util/machine-id" import { Config } from "@/config/config" +import { Flag } from "@/flag/flag" import { InstallationVersion } from "@opencode-ai/core/installation/version" import { Log } from "@/altimate/util/log" @@ -76,9 +77,11 @@ const log = Log.create({ service: "altimate-plugin" }) export async function buildCliContext(machineIdPath?: string): Promise { // altimate_change start — honour both telemetry opt-out gates, mirroring // telemetry/index.ts::doInit: - // 1. ALTIMATE_TELEMETRY_DISABLED=true env var (always-works hard opt-out) + // 1. ALTIMATE_TELEMETRY_DISABLED / OPENCODE_DISABLE_TELEMETRY env vars + // ("true"/"TRUE"/"1", case-insensitive — see Flag.truthyEnv) // 2. config.telemetry.disabled (resolved via the async Config.get()) - let disabled = process.env.ALTIMATE_TELEMETRY_DISABLED === "true" + let disabled = + Flag.truthyEnv("ALTIMATE_TELEMETRY_DISABLED") || Flag.truthyEnv("OPENCODE_DISABLE_TELEMETRY") if (!disabled) { try { const userConfig = (await Config.get()) as any diff --git a/packages/opencode/src/altimate/telemetry/index.ts b/packages/opencode/src/altimate/telemetry/index.ts index 61db828430..e7f14db537 100644 --- a/packages/opencode/src/altimate/telemetry/index.ts +++ b/packages/opencode/src/altimate/telemetry/index.ts @@ -1683,7 +1683,10 @@ export namespace Telemetry { async function doInit() { try { - if (process.env.ALTIMATE_TELEMETRY_DISABLED === "true") { + // altimate_change — accept "true"/"TRUE"/"1" (case-insensitive) via truthyEnv, + // and honor the OPENCODE_DISABLE_TELEMETRY fallback promised by v0.9.4's CHANGELOG + // (previously only wired in test fixtures, silent no-op in product). + if (Flag.truthyEnv("ALTIMATE_TELEMETRY_DISABLED") || Flag.truthyEnv("OPENCODE_DISABLE_TELEMETRY")) { buffer = [] return } diff --git a/packages/opencode/src/altimate/tools/sample-setup.ts b/packages/opencode/src/altimate/tools/sample-setup.ts index bfaee9654e..d0c16aa751 100644 --- a/packages/opencode/src/altimate/tools/sample-setup.ts +++ b/packages/opencode/src/altimate/tools/sample-setup.ts @@ -268,7 +268,9 @@ export const SampleSetupTool = Tool.define("sample_setup", { * apply a conservative pattern that stops at whitespace and quotes rather than trying to guess * where a path ends. Full detail is kept in `metadata.error`, which the model never sees. */ -function redactPaths(message: string, extra: (string | undefined)[] = []): string { +// altimate_change — exported for direct unit testing (v0.9.5 review, Tech Lead P1). +// Pure function; no additional risk from exposing it. +export function redactPaths(message: string, extra: (string | undefined)[] = []): string { let out = message for (const known of [os.homedir(), process.cwd(), os.tmpdir(), ...extra]) { if (!known || known.length < 2) continue @@ -306,8 +308,9 @@ function countFilesWithExtension(dir: string, extension: string): number { return total } +// altimate_change — exported for direct unit testing (v0.9.5 review, Tech Lead P1). /** dbt models (`models/**\/*.sql`) and seed tables (`seeds/*.csv`) in the shipped sample. */ -function countSampleContents(sampleSourcePath: string): { models: number; tables: number } { +export function countSampleContents(sampleSourcePath: string): { models: number; tables: number } { return { models: countFilesWithExtension(path.join(sampleSourcePath, "models"), ".sql"), tables: countFilesWithExtension(path.join(sampleSourcePath, "seeds"), ".csv"), diff --git a/packages/opencode/src/cli/welcome.ts b/packages/opencode/src/cli/welcome.ts index 650a851a31..dab265049a 100644 --- a/packages/opencode/src/cli/welcome.ts +++ b/packages/opencode/src/cli/welcome.ts @@ -43,10 +43,16 @@ export function showWelcomeBannerIfNeeded(): void { // launch. Probe existence with existsSync only — do NOT mint here. Minting is left to // Telemetry.doInit() (its job, not the welcome banner's); the first_launch machine_id is // attached at flush time from telemetry module state, so it does not depend on minting here. - // NOTE: doInit's early call runs before an Instance is available, so its CONFIG opt-out gate - // fails open and it can still mint for a config-only opt-out user. That is a pre-existing - // telemetry-init gap (tracked separately), not something this banner can fix — this code just - // stops adding a SECOND minting site under an even weaker (env-only) gate. + // + // FIXME(telemetry-init-config-opt-out): doInit() may run before Instance.provide() has made + // Config.get() resolvable (see the try/catch around Config.get in telemetry/index.ts::doInit + // — the catch branch proceeds with telemetry enabled). A user who opted out via the + // `telemetry.disabled` config key — with no env var set — can therefore still get a + // machine-id minted on first launch. The env-var opt-out (ALTIMATE_TELEMETRY_DISABLED / + // OPENCODE_DISABLE_TELEMETRY) is unaffected — that check does not need Instance context. + // Pre-existing (not introduced by this release); calling it out explicitly here rather than + // leaving the earlier "(tracked separately)" wording, which claimed a tracking issue that + // does not currently exist. const machineIdPath = path.join(os.homedir(), ".altimate", "machine-id") const isUpgrade = fs.existsSync(machineIdPath) // altimate_change end diff --git a/packages/opencode/src/flag/flag.ts b/packages/opencode/src/flag/flag.ts index 5f59df3304..a5603215db 100644 --- a/packages/opencode/src/flag/flag.ts +++ b/packages/opencode/src/flag/flag.ts @@ -19,6 +19,15 @@ function altEnv(altKey: string, openKey: string) { // altimate_change end export namespace Flag { + // altimate_change start — runtime-evaluated env-truthy helper for callers that need + // current process.env state. Module-level Flag.* constants freeze their value at + // import time; that's the wrong semantics for gates the caller re-reads on each + // invocation (e.g. telemetry.doInit()). Accepts "true" / "TRUE" / "1" — case-insensitive + // — so one convention covers every telemetry/onboarding opt-out env var. + export function truthyEnv(key: string): boolean { + return truthy(key) + } + // altimate_change end // altimate_change start - ALTIMATE_CLI_CLIENT flag with OPENCODE_CLIENT fallback export declare const ALTIMATE_CLI_CLIENT: string // altimate_change end diff --git a/packages/opencode/test/altimate/sample-setup-helpers.test.ts b/packages/opencode/test/altimate/sample-setup-helpers.test.ts new file mode 100644 index 0000000000..1327326c29 --- /dev/null +++ b/packages/opencode/test/altimate/sample-setup-helpers.test.ts @@ -0,0 +1,195 @@ +// v0.9.5 review — Tech Lead P1 (initial pass) + coderabbit / cubic follow-ups. +// +// sample-setup.ts::redactPaths has a docstring listing three specific bugs the +// implementation was written to fix (regex swallowing surrounding sentence, +// `/root/…` prefix leaking, José/O'Connor surnames leaking on the first pass). +// None of them had test coverage this release. This file asserts the fixes so a +// future edit to the regex or the known-value substitution loop can't silently +// re-introduce them. +// +// countSampleContents is a tiny counting helper — but it's called on every +// sample_setup invocation and its output feeds a telemetry event, so a +// silent-zero bug (typo in the extension filter, wrong subdirectory name) +// would misreport onboarding activity. The fixture below builds a real dir tree +// per test and asserts the count. +// +// Fixture ownership note (bot-review follow-up, coderabbit + cubic): +// countSampleContents originally shared one `mkdtempSync` at module scope with +// afterAll cleanup. That leaks if the suite is filtered (only redactPaths tests +// selected) or if a `beforeAll` throws before `afterAll` registers. Switched to +// the repo's `await using tmp = await tmpdir()` pattern so each test owns its +// fixture and cleanup is bound to the test scope. + +import { describe, expect, test } from "bun:test" +import fs from "fs" +import path from "path" + +import { tmpdir } from "../fixture/fixture" +import { redactPaths, countSampleContents } from "../../src/altimate/tools/sample-setup" + +describe("redactPaths", () => { + test("redacts an absolute POSIX path", () => { + const out = redactPaths("failed at /usr/local/bin/dbt") + expect(out).toBe("failed at ") + }) + + test("redacts a Windows drive path", () => { + const out = redactPaths("failed at C:\\Users\\alice\\dbt.exe") + // Windows paths pattern matches `C:\` opener and consumes until whitespace/quote. + expect(out).toBe("failed at ") + expect(out).not.toContain("Users") + expect(out).not.toContain("alice") + }) + + test("redacts a home-relative path (~/)", () => { + // Note: `~/.altimate/…` is redacted by the POSIX-`/` pattern first, which + // starts at the leading slash and leaves the `~` as a harmless prefix. + // What the assertion cares about is that no path segment leaks — the exact + // shape of the redaction marker is secondary. + const out = redactPaths("cannot read ~/.altimate/machine-id") + expect(out).not.toContain(".altimate") + expect(out).not.toContain("machine-id") + expect(out).toContain("") + }) + + test("redacts a bare tilde-only path (~/x with no preceding slash)", () => { + // The dedicated `~\/…` pattern is what catches this shape — the POSIX-`/` + // one starts inside the path and can leave a `~` behind. + const input = "opening ~/opt/dbt for read" + const out = redactPaths(input) + expect(out).not.toContain("opt/dbt") + expect(out).toContain("") + }) + + test("terminates at whitespace, does NOT swallow the surrounding sentence", () => { + // The docstring calls out this exact class of bug: an early implementation + // used a character class that included `.`, so the regex would consume the + // rest of the sentence past the path. Reader ends up with just "". + const input = "Underlying error: /Users/alice/projects/dbt-demo failed to compile" + const out = redactPaths(input) + expect(out).toContain("failed to compile") + expect(out).toContain("Underlying error:") + expect(out).not.toContain("/Users") + }) + + test("terminates at a double-quote", () => { + const out = redactPaths('opening "/Users/alice/dbt_project.yml" for read') + expect(out).toContain("for read") + expect(out).not.toContain("alice") + }) + + test("handles a path containing an apostrophe (O'Connor)", () => { + // Docstring bug: an early character class excluded `'`, so the regex would + // stop at the apostrophe and leak the substring after it. Now apostrophes + // are permitted inside the redacted run. + const out = redactPaths("failed at /Users/O'Connor/projects/x") + expect(out).not.toContain("O'Connor") + expect(out).not.toContain("Connor") + expect(out).toBe("failed at ") + }) + + test("handles a path containing an accented character (José)", () => { + const out = redactPaths("failed at /Users/José/dbt") + expect(out).not.toContain("José") + expect(out).toBe("failed at ") + }) + + test("path segment terminates at the first whitespace, so a path containing a space leaks the tail", () => { + // codex-review gap: the greedy pattern stops at the first `\s`, so a real + // CWD like `/Users/alice/My Documents/dbt` gets split — only `/Users/alice/My` + // is redacted; `Documents/dbt` is left in the output. + // + // The `extra` list is what production callers use to close this gap (they + // pass the exact CWD to `redactPaths(msg, [cwd])`), so this test also + // asserts the compensating behavior — with the CWD known-value, the whole + // path collapses cleanly. + const cwd = "/Users/alice/My Documents/dbt" + const raw = redactPaths(`failed at ${cwd}/models/foo.sql`) + // Documented limitation of the pattern-only pass: the greedy path pattern + // terminates at the first whitespace, so `/Users/alice/My` and + // `/dbt/models/foo.sql` each redact cleanly but the middle segment + // `Documents` sits between two `` markers. + expect(raw).toContain("Documents") + expect(raw).not.toBe("failed at ") + // With the CWD passed as a known value the whole path collapses cleanly: + const guarded = redactPaths(`failed at ${cwd}/models/foo.sql`, [cwd]) + expect(guarded).toBe("failed at ") + expect(guarded).not.toContain("Documents") + expect(guarded).not.toContain("alice") + }) + + test("collapses adjacent segments so double-redaction reads clean", () => { + // The known-value pass replaces os.homedir() etc first; the greedy pattern + // then may match the "" tail and re-redact. The collapse rule keeps + // the output from becoming "". + const home = require("os").homedir() + const out = redactPaths(`failed at ${home}/dbt/models/foo.sql`) + expect(out).toBe("failed at ") + expect(out).not.toMatch(//) + }) + + test("passes short/empty known values without exploding", () => { + // Guard for `known.length < 2` — empty string or single-char known values + // used to `split("")` and shatter every character. The guard keeps them out + // of the substitution loop. + const out = redactPaths("hello world", ["", "a", undefined]) + expect(out).toBe("hello world") + }) + + test("substitutes user-supplied extras", () => { + const out = redactPaths("clone failed at /tmp/checkout-xyz", ["/tmp/checkout-xyz"]) + expect(out).toBe("clone failed at ") + }) + + test("returns the message unchanged when nothing path-shaped is present", () => { + expect(redactPaths("dbt run completed in 3s")).toBe("dbt run completed in 3s") + }) +}) + +// Shared helper — each test uses its own tmp dir via `await using`, so cleanup +// is scoped to the test itself (bot-review follow-up). +async function seedSampleTree(dir: string) { + fs.mkdirSync(path.join(dir, "models", "staging"), { recursive: true }) + fs.mkdirSync(path.join(dir, "models", "marts", "core"), { recursive: true }) + fs.mkdirSync(path.join(dir, "seeds"), { recursive: true }) + + fs.writeFileSync(path.join(dir, "models", "top.sql"), "select 1") + fs.writeFileSync(path.join(dir, "models", "staging", "stg_orders.sql"), "select 1") + fs.writeFileSync(path.join(dir, "models", "staging", "stg_users.sql"), "select 1") + fs.writeFileSync(path.join(dir, "models", "marts", "core", "dim_customers.sql"), "select 1") + + // Non-.sql alongside .sql, must NOT be counted. + fs.writeFileSync(path.join(dir, "models", "readme.md"), "hi") + fs.writeFileSync(path.join(dir, "models", "schema.yml"), "version: 2") + + fs.writeFileSync(path.join(dir, "seeds", "country_codes.csv"), "code,name\n") + fs.writeFileSync(path.join(dir, "seeds", "regions.csv"), "id,name\n") + // Non-.csv seed — not counted. + fs.writeFileSync(path.join(dir, "seeds", "notes.md"), "hi") +} + +describe("countSampleContents", () => { + test("counts .sql files recursively under models/", async () => { + await using tmp = await tmpdir() + await seedSampleTree(tmp.path) + expect(countSampleContents(tmp.path).models).toBe(4) + }) + + test("counts .csv files under seeds/ (top level; matches production sample layout)", async () => { + await using tmp = await tmpdir() + await seedSampleTree(tmp.path) + expect(countSampleContents(tmp.path).tables).toBe(2) + }) + + test("returns zeros when the sample dir is missing the expected subdirs", async () => { + await using tmp = await tmpdir() + expect(countSampleContents(tmp.path)).toEqual({ models: 0, tables: 0 }) + }) + + test("returns zeros when the sample dir does not exist at all", async () => { + // countFilesWithExtension swallows the readdirSync error and returns 0 — + // this is the graceful-degradation shape the telemetry event depends on. + await using tmp = await tmpdir() + expect(countSampleContents(path.join(tmp.path, "does-not-exist"))).toEqual({ models: 0, tables: 0 }) + }) +}) diff --git a/packages/opencode/test/telemetry/classify-provider.test.ts b/packages/opencode/test/telemetry/classify-provider.test.ts new file mode 100644 index 0000000000..82e85d17d0 --- /dev/null +++ b/packages/opencode/test/telemetry/classify-provider.test.ts @@ -0,0 +1,111 @@ +// v0.9.5 review — Tech Lead P1. +// +// classifyProvider (packages/opencode/src/altimate/telemetry/index.ts) sits on the +// `provider_selected` telemetry path. It is the point where a caller-supplied provider +// id becomes an enum value on our wire, so its allowlist is load-bearing: +// +// - CURATED_PROVIDER_ENUM must be a null-prototype record. A plain `{}` inherits +// Object.prototype, and `record["constructor"]` / ["toString"] / ["valueOf"] +// resolve to inherited functions — those functions are truthy, so with a plain +// object the branch `if (curated) return { provider: curated, ... }` would +// ship the string form of a JS built-in as a "provider" name (or worse, whatever +// the caller-supplied id was, since normalizeCustomProviderID upstream permits +// lowercase letters). The null-prototype defense makes those lookups return +// undefined, forcing the "not curated" path. +// +// - Only ids in KNOWN_PROVIDER_IDS should carry a raw provider_id on the wire. +// Everything else falls through to `{ provider: "other" }` with NO id attached — +// that's what keeps a customer-named custom provider from leaking to telemetry. +// +// - The `opencode` + `big-pickle` pair is the one hard-coded case that returns +// "big_pickle" rather than one of the curated slugs, and it depends on BOTH +// args matching. A regression that ignored modelID would cause every +// `providerID="opencode"` to still ship as `big_pickle`, misattributing traffic. +// +// This file locks each of those three behaviors down. + +import { describe, expect, test } from "bun:test" +import { Telemetry } from "../../src/altimate/telemetry" + +describe("Telemetry.classifyProvider — allowlist + prototype defense", () => { + describe("curated providers", () => { + test.each([ + ["altimate-backend", "altimate_gateway"], + ["anthropic", "anthropic"], + ["openai", "openai"], + ["google", "google"], + ])("providerID %j → provider %j, keeps raw id", (providerID, expected) => { + const result = Telemetry.classifyProvider(providerID) + expect(result).toEqual({ provider: expected, provider_id: providerID }) + }) + }) + + describe("prototype-pollution defense", () => { + // The three inherited-property names most likely to appear as a "provider id" + // in the wild (they're plain lowercase identifiers, so they slip past + // normalizeCustomProviderID). Without the Object.create(null) barrier, + // `CURATED_PROVIDER_ENUM["constructor"]` returns the JS constructor Function, + // which is truthy — and the branch would ship it as a curated provider. + test.each(["constructor", "toString", "valueOf", "hasOwnProperty", "__proto__"])( + "prototype key %j must NOT be treated as a curated match", + (key) => { + const result = Telemetry.classifyProvider(key) + // The guarantee: a prototype key must not resolve to any curated enum. + // `toBe("other")` implies it's none of `altimate_gateway|anthropic|openai|google|big_pickle`, + // so no separate `not.toContain` guard is needed. + expect(result.provider).toBe("other") + }, + ) + }) + + describe("known-but-not-curated providers", () => { + test.each([ + "opencode", + "github-copilot", + "azure", + "amazon-bedrock", + "openrouter", + "mistral", + "groq", + "deepseek", + "xai", + "snowflake-cortex", + "databricks", + "ollama", + "lmstudio", + ])("providerID %j → provider 'other', keeps raw id (safe to publish)", (providerID) => { + const result = Telemetry.classifyProvider(providerID) + expect(result).toEqual({ provider: "other", provider_id: providerID }) + }) + }) + + describe("unknown / customer-named providers", () => { + test.each(["acme-corp", "my-internal-gateway", "team-eng-shared-llm", ""])( + "providerID %j → provider 'other', DROPS raw id (no PII leak)", + (providerID) => { + const result = Telemetry.classifyProvider(providerID) + expect(result.provider).toBe("other") + expect(result.provider_id).toBeUndefined() + }, + ) + }) + + describe("opencode + big-pickle hard-coded pair", () => { + test("both provider and model must match — provider only ≠ big_pickle", () => { + const result = Telemetry.classifyProvider("opencode") + // opencode is known-not-curated → "other" + id, NOT "big_pickle" + expect(result).toEqual({ provider: "other", provider_id: "opencode" }) + }) + + test("both provider and model must match — model only ≠ big_pickle", () => { + const result = Telemetry.classifyProvider("anthropic", "big-pickle") + // Anthropic-with-a-strange-model is still anthropic, not big_pickle + expect(result).toEqual({ provider: "anthropic", provider_id: "anthropic" }) + }) + + test("both matching → big_pickle", () => { + const result = Telemetry.classifyProvider("opencode", "big-pickle") + expect(result).toEqual({ provider: "big_pickle", provider_id: "opencode" }) + }) + }) +}) diff --git a/packages/opencode/test/telemetry/environment-scan-claim.test.ts b/packages/opencode/test/telemetry/environment-scan-claim.test.ts new file mode 100644 index 0000000000..12431995ab --- /dev/null +++ b/packages/opencode/test/telemetry/environment-scan-claim.test.ts @@ -0,0 +1,100 @@ +// v0.9.5 review — Tech Lead P1. +// +// project-scan.ts (packages/opencode/src/altimate/tools/project-scan.ts:952-964) emits +// `environment_scan_completed` guarded by: +// +// if (OnboardingTelemetry.isOnboardingSession(ctx.sessionID) +// && OnboardingTelemetry.claimEnvironmentScan(ctx.sessionID)) { void OnboardingTelemetry.emit(...) } +// +// The claim call is the only thing preventing double-fire of the funnel event +// (a second project_scan invocation inside the same onboarding session would +// otherwise push `scan_gate_shown → environment_scan_completed` above 100%, +// which is the exact metric the comment on that emission block calls out as +// worth protecting). +// +// End-to-end coverage through project-scan requires shelling to git/dbt/docker +// detection and is disproportionately expensive for the guarantee at stake. +// The load-bearing behavior is the once-per-session claim + the session-scope +// isolation of that claim, both of which live in onboarding.ts and are +// independently testable. + +import { afterEach, beforeEach, describe, expect, test } from "bun:test" +import * as OnboardingTelemetry from "../../src/altimate/telemetry/onboarding" + +beforeEach(() => { + OnboardingTelemetry.resetForTest() +}) +afterEach(() => { + OnboardingTelemetry.resetForTest() +}) + +describe("environment_scan_completed guard", () => { + test("claimEnvironmentScan returns true on first call, false on subsequent calls", () => { + const session = "sess-1" + expect(OnboardingTelemetry.claimEnvironmentScan(session)).toBe(true) + expect(OnboardingTelemetry.claimEnvironmentScan(session)).toBe(false) + expect(OnboardingTelemetry.claimEnvironmentScan(session)).toBe(false) + }) + + test("claims are session-scoped — a second session claims independently", () => { + expect(OnboardingTelemetry.claimEnvironmentScan("sess-A")).toBe(true) + expect(OnboardingTelemetry.claimEnvironmentScan("sess-B")).toBe(true) + // ...and each session's claim stays exhausted after its first success + expect(OnboardingTelemetry.claimEnvironmentScan("sess-A")).toBe(false) + expect(OnboardingTelemetry.claimEnvironmentScan("sess-B")).toBe(false) + }) + + test("isOnboardingSession is false for sessions that were never marked", () => { + // The AND-guard on the emission means a non-onboarding session that runs + // project_scan (via /discover, or a model-initiated call) will NOT emit + // the onboarding-funnel event, even though the claim call would succeed + // on its own. This is what stops the funnel-taxonomy event from firing + // for routine `/discover` runs from returning users. + expect(OnboardingTelemetry.isOnboardingSession("random-session")).toBe(false) + }) + + test("isOnboardingSession is true only after markOnboardingSession", () => { + const s = "onboarding-sess" + expect(OnboardingTelemetry.isOnboardingSession(s)).toBe(false) + OnboardingTelemetry.markOnboardingSession(s) + expect(OnboardingTelemetry.isOnboardingSession(s)).toBe(true) + }) + + test("the AND-guard (isOnboardingSession && claimEnvironmentScan) fires exactly once", () => { + // Mirrors the exact shape at project-scan.ts:952. A single onboarding + // session that runs project_scan twice must see the emission gate open + // once and stay closed on the retry. + const s = "funnel-sess" + OnboardingTelemetry.markOnboardingSession(s) + const fires: number[] = [] + for (let i = 0; i < 3; i++) { + if ( + OnboardingTelemetry.isOnboardingSession(s) && + OnboardingTelemetry.claimEnvironmentScan(s) + ) { + fires.push(i) + } + } + expect(fires).toEqual([0]) + }) + + test("a non-onboarding session running the same guard chain never fires", () => { + const s = "returning-user-sess" + // Note: no markOnboardingSession call. Guard should short-circuit at + // isOnboardingSession → false and never even reach the claim call, so + // the claim stays unspent (verifiable below). + const fires: number[] = [] + for (let i = 0; i < 3; i++) { + if ( + OnboardingTelemetry.isOnboardingSession(s) && + OnboardingTelemetry.claimEnvironmentScan(s) + ) { + fires.push(i) + } + } + expect(fires).toEqual([]) + // If the guard had short-circuited correctly, the claim is still available. + // Prove it by explicitly calling claimEnvironmentScan and observing true. + expect(OnboardingTelemetry.claimEnvironmentScan(s)).toBe(true) + }) +}) diff --git a/packages/opencode/test/telemetry/telemetry-opt-out-flag.test.ts b/packages/opencode/test/telemetry/telemetry-opt-out-flag.test.ts new file mode 100644 index 0000000000..d985834736 --- /dev/null +++ b/packages/opencode/test/telemetry/telemetry-opt-out-flag.test.ts @@ -0,0 +1,181 @@ +// v0.9.5 review — Chaos gremlin P1 + End-user P2, plus bot-review follow-ups +// (codex + coderabbit + cubic all named the same gap: the initial version of +// this file only tested the shared `Flag.truthyEnv` helper against +// `ALTIMATE_TELEMETRY_DISABLED`. That meant a regression that dropped the +// `OPENCODE_DISABLE_TELEMETRY` fallback OR removed the OR-composition at either +// call site (`altimate/plugin/altimate.ts::buildCliContext`, +// `altimate/telemetry/index.ts::doInit`) would still pass every test — the +// exact failure this file exists to prevent). +// +// The file now covers three layers: +// 1. Parser semantics — `Flag.truthyEnv` accepts "true"/"TRUE"/"1", rejects +// the everything-else surface (case sensitivity, whitespace, non-1 digits). +// 2. Consumer-boundary composition — the exact `truthyEnv(A) || truthyEnv(B)` +// shape used at both gate sites returns true when EITHER env var is set. +// 3. Gate-site source anchor — both `altimate.ts` and `telemetry/index.ts` +// reference both env-var names on the same line. Brittle by design: if a +// future edit deletes the fallback branch, this test catches it without +// needing to boot the full telemetry init in a unit test. +// +// Env-mutation isolation: each test snapshots the two vars before mutating +// them and restores what it found. This keeps the suite safe against other +// tests in the same process reading `process.env.ALTIMATE_TELEMETRY_DISABLED` +// or `OPENCODE_DISABLE_TELEMETRY` and against parallel suites in other +// packages (bot-review follow-up, coderabbit). + +import { afterEach, beforeEach, describe, expect, test } from "bun:test" +import fs from "fs" +import path from "path" +import { Flag } from "../../src/flag/flag" + +const ALTIMATE_VAR = "ALTIMATE_TELEMETRY_DISABLED" +const OPENCODE_VAR = "OPENCODE_DISABLE_TELEMETRY" + +// Snapshot both env vars and restore them after each test — protects against +// leakage into or out of this suite regardless of what set them. +let snapshot: { altimate: string | undefined; opencode: string | undefined } + +beforeEach(() => { + snapshot = { + altimate: process.env[ALTIMATE_VAR], + opencode: process.env[OPENCODE_VAR], + } +}) + +afterEach(() => { + if (snapshot.altimate === undefined) delete process.env[ALTIMATE_VAR] + else process.env[ALTIMATE_VAR] = snapshot.altimate + if (snapshot.opencode === undefined) delete process.env[OPENCODE_VAR] + else process.env[OPENCODE_VAR] = snapshot.opencode +}) + +describe("Flag.truthyEnv — parser semantics on ALTIMATE_TELEMETRY_DISABLED", () => { + test("unset env var → false (default: telemetry enabled)", () => { + delete process.env[ALTIMATE_VAR] + expect(Flag.truthyEnv(ALTIMATE_VAR)).toBe(false) + }) + + test("empty string → false", () => { + process.env[ALTIMATE_VAR] = "" + expect(Flag.truthyEnv(ALTIMATE_VAR)).toBe(false) + }) + + test.each([ + ["true", true], + ["TRUE", true], + ["True", true], + ["1", true], + ["false", false], + ["FALSE", false], + ["0", false], + ["yes", false], + ["on", false], + [" true ", false], // no trimming — matches existing truthy() semantics + ["2", false], + ])("value %j → %s", (value, expected) => { + process.env[ALTIMATE_VAR] = value + expect(Flag.truthyEnv(ALTIMATE_VAR)).toBe(expected) + }) + + test("re-reads process.env on each call (not frozen at import)", () => { + delete process.env[ALTIMATE_VAR] + expect(Flag.truthyEnv(ALTIMATE_VAR)).toBe(false) + process.env[ALTIMATE_VAR] = "1" + expect(Flag.truthyEnv(ALTIMATE_VAR)).toBe(true) + process.env[ALTIMATE_VAR] = "false" + expect(Flag.truthyEnv(ALTIMATE_VAR)).toBe(false) + }) +}) + +describe("Consumer-boundary composition — both env vars route through the same OR gate", () => { + // The two production callers use `truthyEnv(A) || truthyEnv(B)`. These tests + // exercise both branches of that OR expression so a regression that hard-wires + // one branch to false would fail here (codex/coderabbit/cubic finding). + // + // We reproduce the composition inline via a local helper so the tests read + // as one unit rather than four permutations. The "gate-site anchor" tests + // below then prove the production callers actually contain this shape. + const gate = () => + Flag.truthyEnv(ALTIMATE_VAR) || Flag.truthyEnv(OPENCODE_VAR) + + test("neither set → gate is closed (telemetry enabled)", () => { + delete process.env[ALTIMATE_VAR] + delete process.env[OPENCODE_VAR] + expect(gate()).toBe(false) + }) + + test("only ALTIMATE_TELEMETRY_DISABLED set → gate is open (primary branch)", () => { + delete process.env[OPENCODE_VAR] + process.env[ALTIMATE_VAR] = "1" + expect(gate()).toBe(true) + }) + + test("only OPENCODE_DISABLE_TELEMETRY set → gate is open (fallback branch)", () => { + // v0.9.4 CHANGELOG advertised this name; before v0.9.5 it was silently + // ignored in product. Removing the second `truthyEnv(...)` from either + // call site would silently regress this — the source-anchor test below + // catches that additional shape. + delete process.env[ALTIMATE_VAR] + process.env[OPENCODE_VAR] = "1" + expect(gate()).toBe(true) + }) + + test.each([ + ["true", "true"], + ["1", "1"], + ["TRUE", "true"], + ])( + "both set (%j / %j) → gate is open", + (altimateVal, opencodeVal) => { + process.env[ALTIMATE_VAR] = altimateVal + process.env[OPENCODE_VAR] = opencodeVal + expect(gate()).toBe(true) + }, + ) + + test("both set with non-truthy values → gate is closed", () => { + process.env[ALTIMATE_VAR] = "false" + process.env[OPENCODE_VAR] = "0" + expect(gate()).toBe(false) + }) +}) + +describe("Gate-site anchor — both call sites reference both env-var names", () => { + // codex/coderabbit/cubic all pointed out that testing the shared helper in + // isolation doesn't prove the CALLER retains the fallback branch. Without an + // integration-level runner for `doInit()` / `buildCliContext` (both touch + // Config, machine-id, and a network sink), the closest deterministic proof is + // a source-shape assertion: both call sites must mention both env-var names + // within a small window. A future edit that deletes the fallback fails here + // and gives a clear name for what regressed. + // + // The window (a single non-comment line, or up to two adjacent non-comment + // lines) is deliberately narrow — enough for the OR expression to wrap to a + // second line for prettier, but not enough for the two names to sit in + // unrelated pieces of the file. + const GATE_FILES = [ + "src/altimate/plugin/altimate.ts", + "src/altimate/telemetry/index.ts", + ] + + test.each(GATE_FILES)( + "%s references both %s and %s in the same gate", + (relativePath) => { + const absolute = path.resolve(__dirname, "../..", relativePath) + const source = fs.readFileSync(absolute, "utf8") + // Strip line comments so a `// mentions X and Y` comment doesn't count. + const codeLines = source + .split("\n") + .map((l) => l.replace(/\/\/.*$/, "").trim()) + const window = 2 + let matched = false + for (let i = 0; i < codeLines.length && !matched; i++) { + const slice = codeLines.slice(i, i + window).join(" ") + if (slice.includes(ALTIMATE_VAR) && slice.includes(OPENCODE_VAR)) { + matched = true + } + } + expect(matched).toBe(true) + }, + ) +})