From 83961acb83abec1011506e418477e62bf9f9695d Mon Sep 17 00:00:00 2001 From: anandgupta42 Date: Wed, 5 Aug 2026 09:58:15 +0530 Subject: [PATCH] fix: Windows grep outage and test-run telemetry pollution MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two production defects found in Azure telemetry over 2026-07-22 → 2026-08-05. **Windows `grep` broken for 99 machines** `core_failure` showed 328 events across 99 distinct Windows machines (of 617 total) carrying `? is not recognized as an internal or external command, operable program or batch file.` and its German, French, Spanish and Portuguese translations. Present on released 0.9.2, 0.9.3 and 0.9.4. Root cause: ripgrep's Windows release is a zip, and `RipgrepBinary` extracted it via `powershell.exe -Command Expand-Archive`, falling back to the literal string `"powershell.exe"` when neither `powershell.exe` nor `pwsh.exe` resolved. `cross-spawn`'s `parseNonShell()` sets `needsShell = true` when `resolveCommand()` returns undefined and re-spawns through `cmd.exe /d /s /c`, so cmd.exe produced that message. `throw new Error(result.stderr.trim())` made it the error verbatim, and since `RipgrepBinary.filepath` is `Effect.cached`, one failed extraction broke grep for the whole session. Upstream carries the same fragility: anomalyco/opencode#24291 is open, reporting `Expand-Archive` unusable when spawned from the Bun-compiled binary, affecting `grep`, `glob` and `skill`. Their #23457 fix only corrected how paths were passed to PowerShell (the `$args` → inlined-and-escaped form we already carry); it did not remove the dependency on PowerShell being resolvable. Extract the zip in-process with `@zip.js/zip.js`, converging on the approach the `packages/opencode/src/file/ripgrep.ts` shim already uses in production. `unzipExecutable` is exported so archive handling is tested directly, and decodes with `checkSignature: true` — zip.js defaults it off, and a CRC-corrupt download would otherwise be written to the cache and trusted by every later session. Install the binary atomically (stage to `rg.exe.tmp`, then rename). `filepath` trusts the cached binary on existence alone, so an interrupted write previously left a truncated `rg.exe` that every later session reused — the same permanent breakage `checkSignature` guards against, which CRC cannot catch because it is verified before the write. The tar path installs the same way. Attribute resolution failures. Child stderr was reported verbatim, so a shell-level failure was indistinguishable from a tool bug; and because a resolve failure is memoized, it is re-reported on every later grep in the session. The tar branch now names ripgrep, and any typed filesystem or HTTP failure is wrapped as `ripgrep binary resolve failed: …`. Note the blast radius is wider than the `grep` tool: `@opencode-ai/core/ripgrep` also backs the HTTP-API file handlers and `cli/cmd/debug/ripgrep.ts`. **Test runs shipped telemetry to the production resource** 1,020 of 3,135 machine ids in the same window emitted `provider_id="test"` / `cli_version="local"` — test processes that regenerate their machine id every run, inflating install and active-machine counts by roughly a third. `doInit()` gated only on `ALTIMATE_TELEMETRY_DISABLED`. Refuse the baked-in connection string when `NODE_ENV=test`, `BUN_TEST`, `VITEST` or `JEST_WORKER_ID` is set. Keyed on test runners, deliberately not on CI: `altimate-code-actions` wraps this CLI, so every run of that shipped product sets `CI`/`GITHUB_ACTIONS`, and gating on those would blind a real product surface. `bun test` sets `NODE_ENV=test`, which covers CI and developer machines alike. An explicit `APPLICATIONINSIGHTS_CONNECTION_STRING` is always honoured, so suites with their own sink are unaffected; `ALTIMATE_TELEMETRY_FORCE=true` overrides the default-sink refusal, and the existing opt-outs still win over both. Test plan: 8 ripgrep tests — including a layer-level test that drives `filepath` through the Windows zip path with a spawner that fails if invoked, asserting the staged-then-renamed install, and a CRC case that fails without `checkSignature` — plus 15 telemetry-gate tests covering CI-alone-still-reports and one that relies on the real runner's `NODE_ENV`. `turbo typecheck` green, marker guard green, `packages/core` and `packages/opencode` suites show no failures beyond the pre-existing set on `main`. Co-Authored-By: Claude Opus 5 --- .github/workflows/ci.yml | 33 +++ bun.lock | 1 + docs/docs/reference/telemetry.md | 18 ++ packages/core/package.json | 1 + packages/core/script/windows-ripgrep-e2e.ts | 150 ++++++++++++ packages/core/src/ripgrep.ts | 6 +- packages/core/src/ripgrep/binary.ts | 150 +++++++++--- packages/core/test/ripgrep-windows.test.ts | 227 ++++++++++++++++++ .../opencode/src/altimate/telemetry/index.ts | 32 ++- .../test/telemetry/automated-run.test.ts | 202 ++++++++++++++++ 10 files changed, 788 insertions(+), 32 deletions(-) create mode 100644 packages/core/script/windows-ripgrep-e2e.ts create mode 100644 packages/core/test/ripgrep-windows.test.ts create mode 100644 packages/opencode/test/telemetry/automated-run.test.ts diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7bb116a6eb..8826a5579e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -379,6 +379,39 @@ jobs: $config.Output.Verbosity = "Detailed" Invoke-Pester -Configuration $config + # altimate_change start — Windows ripgrep E2E (issue #1072) + # --------------------------------------------------------------------------- + # Real Windows check for ripgrep binary resolution. Downloads and extracts the + # actual archive with PowerShell stripped from PATH, then executes the binary. + # This is the condition that broke grep for 99 Windows machines; no amount of + # unit testing on Linux/macOS covers it. + # --------------------------------------------------------------------------- + windows-ripgrep-e2e: + name: Windows ripgrep E2E + needs: changes + if: needs.changes.outputs.typescript == 'true' || github.event_name == 'push' + runs-on: windows-latest + timeout-minutes: 15 + steps: + - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 + + - uses: oven-sh/setup-bun@ecf28ddc73e819eb6fa29df6b34ef8921c743461 # v2 + with: + bun-version: "1.3.14" + + # --ignore-scripts: `tree-sitter-powershell` has no Windows prebuild and its fallback + # compile needs Visual Studio Build Tools, which this runner does not have (see + # anomalyco/opencode#25563). This check only needs the pure-JS dependency graph + # (effect, @zip.js/zip.js, which, xdg-basedir), so skipping lifecycle scripts is enough. + - name: Install dependencies + run: bun install --ignore-scripts + + # Run from packages/core so `effect` and the other deps resolve — they are not root deps. + - name: Resolve ripgrep with PowerShell unavailable + working-directory: packages/core + run: bun run script/windows-ripgrep-e2e.ts + # altimate_change end + # --------------------------------------------------------------------------- # dbt-tools E2E — slow (~3 min), only on push to main. # Tests dbt CLI fallbacks against real dbt versions (1.8, 1.10, 1.11) and diff --git a/bun.lock b/bun.lock index c7dfd8ffd3..b1091b850f 100644 --- a/bun.lock +++ b/bun.lock @@ -95,6 +95,7 @@ "@opentelemetry/sdk-trace-base": "2.6.1", "@parcel/watcher": "2.5.1", "@silvia-odwyer/photon-node": "0.3.4", + "@zip.js/zip.js": "2.7.62", "ai-gateway-provider": "3.1.2", "bun-pty": "0.4.8", "cross-spawn": "catalog:", diff --git a/docs/docs/reference/telemetry.md b/docs/docs/reference/telemetry.md index 0ef5926aba..f521e8f806 100644 --- a/docs/docs/reference/telemetry.md +++ b/docs/docs/reference/telemetry.md @@ -126,6 +126,24 @@ export ALTIMATE_TELEMETRY_DISABLED=true When telemetry is disabled, no events are sent and no network requests are made to the telemetry endpoint. +### Test runs are excluded + +Test runners never reach the default telemetry endpoint. Telemetry is suppressed when `NODE_ENV=test`, +`BUN_TEST`, `VITEST`, or `JEST_WORKER_ID` is present. This exists because test processes regenerate +their machine ID on every run, so without the exclusion they dominate install and active-machine counts. + +Running in CI is **not** excluded — that is ordinary product usage (for example +[altimate-code-actions](https://github.com/AltimateAI/altimate-code-actions) wraps this CLI), so +`CI` and `GITHUB_ACTIONS` on their own do not suppress anything. + +Two escape hatches exist for reporting from a test run deliberately: + +- Set `APPLICATIONINSIGHTS_CONNECTION_STRING` to your own endpoint — an explicitly-configured sink + is always honoured, which is how the project's own telemetry tests work. +- Set `ALTIMATE_TELEMETRY_FORCE=true` to use the default endpoint anyway. + +`ALTIMATE_TELEMETRY_DISABLED` and the config opt-out take precedence over both. + ## Privacy We take your privacy seriously. Altimate Code telemetry **never** collects: diff --git a/packages/core/package.json b/packages/core/package.json index 7e12e5bac1..19e76e778b 100644 --- a/packages/core/package.json +++ b/packages/core/package.json @@ -97,6 +97,7 @@ "@opentelemetry/sdk-trace-base": "2.6.1", "@parcel/watcher": "2.5.1", "@silvia-odwyer/photon-node": "0.3.4", + "@zip.js/zip.js": "2.7.62", "@openrouter/ai-sdk-provider": "2.9.0", "ai-gateway-provider": "3.1.2", "bun-pty": "0.4.8", diff --git a/packages/core/script/windows-ripgrep-e2e.ts b/packages/core/script/windows-ripgrep-e2e.ts new file mode 100644 index 0000000000..4225068764 --- /dev/null +++ b/packages/core/script/windows-ripgrep-e2e.ts @@ -0,0 +1,150 @@ +/** + * Windows end-to-end check for ripgrep binary resolution. + * + * Reproduces as much as a GitHub runner allows of the condition behind the outage in + * https://github.com/AltimateAI/altimate-code/issues/1072: a cold cache on a Windows machine + * where PowerShell cannot be resolved from PATH. The old implementation extracted ripgrep's zip + * by shelling out to `powershell.exe -Command Expand-Archive`, so cross-spawn fell back to + * `cmd.exe /d /s /c` and the extraction died with "is not recognized as an internal or external + * command" — which `Effect.cached` then replayed for the rest of the session. + * + * This performs a real download and a real extraction, then executes the resulting binary. It is + * deliberately not a unit test: the point is to exercise the actual filesystem, the actual archive + * and the actual process launch on a real Windows host. + * + * Scope note: stripping PATH does NOT make PowerShell unspawnable on Windows (see the control + * probe at the end), so this does not reproduce the affected machines. The guarantee that + * extraction spawns nothing at all is established by test/ripgrep-windows.test.ts. + * + * Run: bun run script/windows-ripgrep-e2e.ts (from packages/core — `effect` resolves there) + */ +import { execFileSync } from "node:child_process" +import { mkdtempSync, existsSync, readdirSync, statSync, rmSync } from "node:fs" +import { tmpdir } from "node:os" +import path from "node:path" + +function fail(message: string): never { + console.error(`FAIL: ${message}`) + process.exit(1) +} + +function ok(message: string) { + console.log(`ok ${message}`) +} + +if (process.platform !== "win32") fail(`this check only means anything on Windows (got ${process.platform})`) + +// Cold cache: point the XDG cache root at a throwaway directory *before* importing anything that +// reads it, since Global computes its paths at module load. +const cacheRoot = mkdtempSync(path.join(tmpdir(), "rg-e2e-")) +process.env.XDG_CACHE_HOME = cacheRoot +process.env.LOCALAPPDATA = cacheRoot + +// Remove every PATH entry that could provide PowerShell. System32 itself is kept so cmd.exe and +// the rest of Windows still work — this simulates the locked-down/sanitized-PATH machines in the +// telemetry, not a broken OS. +const originalPath = process.env.PATH ?? process.env.Path ?? "" +const stripped = originalPath + .split(path.delimiter) + .filter((entry) => entry && !/powershell/i.test(entry)) + .join(path.delimiter) +process.env.PATH = stripped +process.env.Path = stripped + +const { which } = await import("../src/util/which") + +// Guard against a vacuous run: if PowerShell is still resolvable, this proves nothing. +const ps = which("powershell.exe") +const pwsh = which("pwsh.exe") +if (ps || pwsh) fail(`PowerShell is still resolvable (${ps ?? pwsh}) — the scenario was not reproduced`) +ok("PowerShell is not resolvable on PATH (failure condition reproduced)") + +// Also assert the binary really is absent, so we exercise download + extract rather than a cache hit. +const { Global } = await import("../src/global") +const target = path.join(Global.Path.bin, "rg.exe") +if (existsSync(target)) fail(`expected a cold cache but ${target} already exists`) +ok(`cold cache at ${Global.Path.bin}`) + +const { Effect } = await import("effect") +const { RipgrepBinary } = await import("../src/ripgrep/binary") + +/** Resolve through the real layer: real HTTP, real filesystem, real process launch. */ +async function resolveBinary(): Promise { + const program = Effect.gen(function* () { + const binary = yield* RipgrepBinary.Service + return yield* binary.filepath + }).pipe(Effect.provide(RipgrepBinary.defaultLayer)) + return (await Effect.runPromise(program as never)) as string +} + +let resolved: string +try { + resolved = await resolveBinary() +} catch (err: unknown) { + fail(`binary.filepath failed: ${err instanceof Error ? err.message : String(err)}`) +} + +ok(`resolved ${resolved}`) + +if (!existsSync(resolved)) fail(`resolved path does not exist: ${resolved}`) +const size = statSync(resolved).size +if (size < 100_000) fail(`resolved binary is implausibly small (${size} bytes) — likely a partial write`) +ok(`binary present, ${size} bytes`) + +// No staging files should survive a successful install. +const leftovers = readdirSync(Global.Path.bin).filter((f) => f.endsWith(".tmp")) +if (leftovers.length > 0) fail(`staging files left behind: ${leftovers.join(", ")}`) +ok("no staging files left behind") + +// The real proof: the extracted binary actually executes. +const version = execFileSync(resolved, ["--version"], { encoding: "utf8" }) +if (!/ripgrep\s+\d/.test(version)) fail(`unexpected --version output: ${version.trim()}`) +ok(`executes: ${version.split("\n")[0]!.trim()}`) + +// And it can actually search. +const hit = execFileSync(resolved, ["--no-config", "NEEDLE_MARKER", "--", import.meta.filename], { + encoding: "utf8", +}) +if (!hit.includes("NEEDLE_MARKER")) fail("ripgrep did not return the expected match") +ok("search returns matches") // NEEDLE_MARKER + +// A second resolve must hit the cache and stay valid. +const again = await resolveBinary() +if (again !== resolved) fail(`second resolve returned a different path: ${again}`) +ok("second resolve hits the cache") + +// --------------------------------------------------------------------------- +// Control probe — informational, deliberately not a hard failure. +// +// It would be neater to also show that the OLD implementation fails here, making this a true +// counterfactual. It does not, and that is worth recording: emptying PATH is not enough to make +// PowerShell unspawnable on Windows. cross-spawn falls back to `cmd.exe /d /s /c`, and Windows +// process creation searches beyond PATH (the caller's directory, the system directories, and the +// App Paths registry key), so `powershell.exe` still starts on a stock GitHub runner even though +// `which()` cannot see it. +// +// So this job does NOT reproduce the affected machines. What it does prove is the part that +// matters: on real Windows the new path downloads, extracts in-process, installs and produces a +// working rg.exe. That PowerShell's availability is irrelevant to it is established separately and +// structurally by `test/ripgrep-windows.test.ts`, which drives the same code with a spawner that +// fails the test if anything is launched at all. +// --------------------------------------------------------------------------- +const launch = (await import("cross-spawn")).default +const control = launch.sync("powershell.exe", ["-NoProfile", "-NonInteractive", "-Command", "exit 0"], { + encoding: "utf8", +}) + +if (control.status === 0) { + console.log( + "note this runner still spawns powershell.exe despite the stripped PATH (Windows resolves\n" + + " executables beyond PATH), so the affected environment is not reproduced here — the\n" + + " no-spawn guarantee comes from the unit layer test, not from this job", + ) +} else { + const output = `${control.stderr ?? ""}${control.stdout ?? ""}${control.error?.message ?? ""}`.trim() + ok(`bonus: the old PowerShell path also fails here (${output.split("\n")[0]?.slice(0, 100) || `status=${control.status}`})`) +} + +rmSync(cacheRoot, { recursive: true, force: true }) +console.log("\nPASS — on real Windows, ripgrep downloads, extracts in-process, installs atomically") +console.log(" and runs. See the note above for what this job does and does not establish.") diff --git a/packages/core/src/ripgrep.ts b/packages/core/src/ripgrep.ts index 34d88b29db..99c851ed1b 100644 --- a/packages/core/src/ripgrep.ts +++ b/packages/core/src/ripgrep.ts @@ -140,7 +140,11 @@ export const layer = Layer.effect( return yield* new InvalidPatternError({ pattern: input.pattern, message: stderr.trim() }) } if (code !== 0 && code !== 1 && code !== 2) { - return yield* failure(stderr.trim() || `ripgrep failed with code ${code}`) + // altimate_change start — upstream_fix: keep child stderr attributable to ripgrep. + // Reporting stderr verbatim made shell-level failures (e.g. a Windows "not recognized" + // message) look like they came from the tool itself, with no hint of the real source. + return yield* failure(`ripgrep failed with code ${code}: ${stderr.trim() || "no output"}`) + // altimate_change end } return { items: code === 1 ? [] : rows, truncated: false, partial: code === 2 } }), diff --git a/packages/core/src/ripgrep/binary.ts b/packages/core/src/ripgrep/binary.ts index 99fa8a2fd0..3f482ccb87 100644 --- a/packages/core/src/ripgrep/binary.ts +++ b/packages/core/src/ripgrep/binary.ts @@ -9,6 +9,10 @@ import { httpClient } from "../effect/layer-node-platform" import { FSUtil } from "../fs-util" import { Global } from "../global" import { which } from "../util/which" +// altimate_change start — upstream_fix: unzip in-process instead of shelling out to PowerShell. +import { randomUUID } from "node:crypto" +import { BlobReader, BlobWriter, ZipReader } from "@zip.js/zip.js" +// altimate_change end export namespace RipgrepBinary { const VERSION = "15.1.0" @@ -26,6 +30,48 @@ export namespace RipgrepBinary { readonly filepath: Effect.Effect } + // altimate_change start — upstream_fix: unzip in-process instead of shelling out to PowerShell. + // Windows is the only platform that ships ripgrep as a zip, and the previous `Expand-Archive` + // implementation needed a resolvable powershell.exe/pwsh.exe. When neither resolved (locked-down + // or non-English corporate images), cross-spawn silently re-spawned through `cmd.exe /d /s /c`, + // which answers "'powershell.exe' is not recognized as an internal or external command". That + // string became the thrown Error verbatim; `filepath` is Effect.cached, so one failed extraction + // broke grep for the rest of the session. Telemetry showed 99 Windows machines stuck on this. + // Decoding in-process removes the external dependency entirely. Upstream has the same fragility + // open as anomalyco/opencode#24291 (Expand-Archive unusable from a Bun-spawned process) — their + // #23457 fix only corrected how the paths were passed to PowerShell, not the dependency on it. + /** Decode the `rg` executable out of a ripgrep release zip. Exported for tests. */ + export const unzipExecutable = Effect.fnUntraced(function* (bytes: ArrayBuffer) { + const reader = new ZipReader(new BlobReader(new Blob([bytes]))) + + // The reader stays open across both getEntries() and getData() — closing after the first would + // release it while entry reads are still outstanding. + return yield* Effect.gen(function* () { + const entries = yield* Effect.tryPromise({ + try: () => reader.getEntries(), + catch: (cause) => new Error(`ripgrep archive could not be read: ${cause}`), + }) + + // Release zips nest the binary under `ripgrep--/`, but match a bare + // `rg.exe` too so a flattened or repackaged archive still works. + const entry = entries.find((x) => !x.directory && /(^|[\\/])rg\.exe$/i.test(x.filename)) + if (!entry?.getData) return yield* Effect.fail(new Error("ripgrep archive did not contain rg.exe")) + + // checkSignature defaults to false in zip.js, which would let a CRC-corrupt download decode + // "successfully". The bytes are then written to Global.Path.bin and trusted by every later + // session purely because the file exists — a corrupt download would break grep permanently, + // which is the failure class this change exists to remove. + const blob = yield* Effect.tryPromise({ + try: () => entry.getData!(new BlobWriter(), { checkSignature: true }), + catch: (cause) => new Error(`ripgrep archive entry could not be decoded: ${cause}`), + }) + const decoded = yield* Effect.promise(() => blob.arrayBuffer()) + if (decoded.byteLength === 0) return yield* Effect.fail(new Error("ripgrep archive contained an empty rg.exe")) + return new Uint8Array(decoded) + }).pipe(Effect.ensuring(Effect.promise(() => reader.close()).pipe(Effect.ignore))) + }) + // altimate_change end + export class Service extends Context.Service()("@opencode/RipgrepBinary") {} export const layer = Layer.effect( @@ -48,34 +94,60 @@ export namespace RipgrepBinary { return { stdout, stderr, code } }, Effect.scoped) - const extract = Effect.fnUntraced(function* ( + // altimate_change start — upstream_fix: install the binary atomically. + // `target` is the cache path every later session trusts on existence alone + // (`fs.isFile(target)` below — no size or integrity check). A write interrupted partway + // therefore leaves a truncated `rg.exe` that is reused forever, which is the same + // "permanently broken until the cache is deleted by hand" failure this change exists to + // remove — and `checkSignature` cannot help, since CRC is verified before the write. + // Staging next to the target keeps the rename within one filesystem, so it is atomic. + const install = Effect.fnUntraced( + function* (target: string, write: (staged: string) => Effect.Effect) { + // Staging name is unique per attempt. A shared `${target}.tmp` lets two cold-cache + // processes clobber each other: one renames while the other is still writing, so the + // loser publishes a partial binary or renames a file that no longer exists. + const staged = `${target}.${process.pid}.${randomUUID().slice(0, 8)}.tmp` + yield* Effect.gen(function* () { + yield* write(staged) + if (process.platform !== "win32") yield* fs.chmod(staged, 0o755) + // POSIX rename replaces atomically. Windows fails when the destination exists, so + // retry once after removing it — but only after the first attempt has failed, so a + // rename that fails for any other reason leaves the existing binary untouched. + yield* fs.rename(staged, target).pipe( + Effect.catch(() => + Effect.gen(function* () { + yield* fs.remove(target, { force: true }).pipe(Effect.ignore) + yield* fs.rename(staged, target) + }), + ), + ) + }).pipe(Effect.onError(() => fs.remove(staged, { force: true }).pipe(Effect.ignore))) + }, + // Name the failure. A resolve failure is memoized by Effect.cached, so it is re-reported on + // every later grep of the session — an unattributed filesystem message there is what made + // the Windows outage read as a tool bug rather than a binary problem. + Effect.mapError((cause) => { + const message = cause instanceof Error ? cause.message : String(cause) + return /ripgrep/i.test(message) ? cause : new Error(`ripgrep binary install failed: ${message}`) + }), + ) + // altimate_change end + + // altimate_change start — upstream_fix: tar.gz path only; zip is handled by unzipExecutable. + const extractTar = Effect.fnUntraced(function* ( archive: string, config: (typeof PLATFORM)[keyof typeof PLATFORM], target: string, ) { const dir = yield* fs.makeTempDirectoryScoped({ directory: Global.Path.bin, prefix: "ripgrep-" }) - if (config.extension === "zip") { - const shell = (yield* Effect.sync(() => which("powershell.exe") ?? which("pwsh.exe"))) ?? "powershell.exe" - const result = yield* run(shell, [ - "-NoProfile", - "-NonInteractive", - "-Command", - `$global:ProgressPreference = 'SilentlyContinue'; Expand-Archive -LiteralPath '${archive.replaceAll("'", "''")}' -DestinationPath '${dir.replaceAll("'", "''")}' -Force`, - ]) - if (result.code !== 0) - throw new Error( - result.stderr.trim() || result.stdout.trim() || `ripgrep extraction failed with code ${result.code}`, - ) - } - - if (config.extension === "tar.gz") { - const result = yield* run("tar", ["-xzf", archive, "-C", dir]) - if (result.code !== 0) - throw new Error( - result.stderr.trim() || result.stdout.trim() || `ripgrep extraction failed with code ${result.code}`, - ) - } + const result = yield* run("tar", ["-xzf", archive, "-C", dir]) + // Attribute the failure to ripgrep extraction rather than reporting child stderr verbatim — + // an unattributed shell string is what made the Windows outage undiagnosable. + if (result.code !== 0) + throw new Error( + `ripgrep extraction failed with code ${result.code}: ${result.stderr.trim() || result.stdout.trim() || "no output"}`, + ) const extracted = path.join( dir, @@ -84,9 +156,9 @@ export namespace RipgrepBinary { ) if (!(yield* fs.isFile(extracted))) throw new Error(`ripgrep archive did not contain executable: ${extracted}`) - yield* fs.copyFile(extracted, target) - if (process.platform !== "win32") yield* fs.chmod(target, 0o755) + yield* install(target, (staged) => fs.copyFile(extracted, staged)) }, Effect.scoped) + // altimate_change end return Service.of({ filepath: yield* Effect.cached( @@ -103,20 +175,40 @@ export namespace RipgrepBinary { const filename = `ripgrep-${VERSION}-${config.platform}.${config.extension}` const url = `https://github.com/BurntSushi/ripgrep/releases/download/${VERSION}/${filename}` - const archive = path.join(Global.Path.bin, filename) yield* Effect.logInfo("downloading ripgrep", { url }) yield* fs.ensureDir(Global.Path.bin).pipe(Effect.orDie) const bytes = yield* HttpClientRequest.get(url).pipe( http.execute, Effect.flatMap((response) => response.arrayBuffer), - Effect.mapError((cause) => (cause instanceof Error ? cause : new Error(String(cause)))), + // altimate_change start — upstream_fix: name the download failure. + // A bare HttpClientError/ResponseError says nothing about ripgrep, and because + // `filepath` is Effect.cached it is then re-reported on every later grep of the + // session — a network block or proxy 403 would read as a grep bug. + Effect.mapError((cause) => { + const message = cause instanceof Error ? cause.message : String(cause) + return /ripgrep/i.test(message) + ? cause instanceof Error + ? cause + : new Error(message) + : new Error(`ripgrep download failed from ${url}: ${message}`) + }), + // altimate_change end ) if (bytes.byteLength === 0) throw new Error(`failed to download ripgrep from ${url}`) - yield* fs.writeWithDirs(archive, new Uint8Array(bytes)) - yield* extract(archive, config, target) - yield* fs.remove(archive, { force: true }).pipe(Effect.ignore) + // altimate_change start — upstream_fix: zip extracts in-process, no PowerShell. + // The staging archive only exists on the tar path, so its cleanup lives there too. + if (config.extension === "zip") { + const decoded = yield* unzipExecutable(bytes) + yield* install(target, (staged) => fs.writeWithDirs(staged, decoded)) + } else { + const archive = path.join(Global.Path.bin, filename) + yield* fs.writeWithDirs(archive, new Uint8Array(bytes)) + yield* extractTar(archive, config, target) + yield* fs.remove(archive, { force: true }).pipe(Effect.ignore) + } + // altimate_change end return target }), ), diff --git a/packages/core/test/ripgrep-windows.test.ts b/packages/core/test/ripgrep-windows.test.ts new file mode 100644 index 0000000000..869ea58d4f --- /dev/null +++ b/packages/core/test/ripgrep-windows.test.ts @@ -0,0 +1,227 @@ +import { describe, expect, test } from "bun:test" +import { BlobWriter, TextReader, Uint8ArrayReader, ZipWriter } from "@zip.js/zip.js" +import { Effect, Layer } from "effect" +import { HttpClient, HttpClientResponse } from "effect/unstable/http" +import { ChildProcessSpawner } from "effect/unstable/process/ChildProcessSpawner" +import { FSUtil } from "../src/fs-util" +import { RipgrepBinary } from "../src/ripgrep/binary" + +// Regression coverage for the Windows `grep` outage: ripgrep's Windows zip was extracted by +// shelling out to `powershell.exe -Command Expand-Archive`. When PowerShell did not resolve, +// cross-spawn re-spawned through cmd.exe, whose "is not recognized as an internal or external +// command" reply became the thrown error verbatim — and because RipgrepBinary.filepath is +// Effect.cached, one failed extraction broke grep for the rest of the session. +// Extraction must stay in-process: no spawn, no external tool. + +async function zipWith( + entries: Array<{ name: string; body: Uint8Array | string }>, + options?: { level?: number }, +): Promise { + const writer = new ZipWriter(new BlobWriter("application/zip")) + for (const entry of entries) { + await writer.add( + entry.name, + typeof entry.body === "string" ? new TextReader(entry.body) : new Uint8ArrayReader(entry.body), + options, + ) + } + const blob = await writer.close() + return blob.arrayBuffer() +} + +/** Index of `needle` within `haystack`, or -1. */ +function indexOfBytes(haystack: Uint8Array, needle: Uint8Array): number { + outer: for (let i = 0; i <= haystack.length - needle.length; i++) { + for (let j = 0; j < needle.length; j++) if (haystack[i + j] !== needle[j]) continue outer + return i + } + return -1 +} + +const run = (effect: Effect.Effect) => Effect.runPromise(effect as Effect.Effect) + +describe("RipgrepBinary.unzipExecutable", () => { + test("extracts rg.exe from the nested release layout", async () => { + const payload = new Uint8Array([0x4d, 0x5a, 0x90, 0x00, 0x03]) + const zip = await zipWith([ + { name: "ripgrep-15.1.0-x86_64-pc-windows-msvc/doc/README.md", body: "docs" }, + { name: "ripgrep-15.1.0-x86_64-pc-windows-msvc/rg.exe", body: payload }, + ]) + + const result = await run(RipgrepBinary.unzipExecutable(zip)) + + expect(Array.from(result)).toEqual(Array.from(payload)) + }) + + test("accepts a flattened archive with rg.exe at the root", async () => { + const payload = new Uint8Array([1, 2, 3, 4]) + const zip = await zipWith([{ name: "rg.exe", body: payload }]) + + const result = await run(RipgrepBinary.unzipExecutable(zip)) + + expect(Array.from(result)).toEqual(Array.from(payload)) + }) + + test("does not mistake a similarly-named file for the executable", async () => { + const zip = await zipWith([ + { name: "ripgrep-15.1.0-x86_64-pc-windows-msvc/rg.exe.sig", body: "signature" }, + { name: "ripgrep-15.1.0-x86_64-pc-windows-msvc/notrg.exe", body: new Uint8Array([9, 9]) }, + { name: "ripgrep-15.1.0-x86_64-pc-windows-msvc/rg.exe", body: new Uint8Array([7, 7]) }, + ]) + + const result = await run(RipgrepBinary.unzipExecutable(zip)) + + expect(Array.from(result)).toEqual([7, 7]) + }) + + test("fails with a named error when the archive has no rg.exe", async () => { + const zip = await zipWith([{ name: "ripgrep-15.1.0-x86_64-pc-windows-msvc/README.md", body: "nope" }]) + + const exit = await run(Effect.result(RipgrepBinary.unzipExecutable(zip))) + + expect(exit._tag).toBe("Failure") + expect(String((exit as { failure: Error }).failure.message)).toContain("did not contain rg.exe") + }) + + test("rejects an empty rg.exe rather than writing a zero-byte binary", async () => { + const zip = await zipWith([{ name: "rg.exe", body: new Uint8Array(0) }]) + + const exit = await run(Effect.result(RipgrepBinary.unzipExecutable(zip))) + + expect(exit._tag).toBe("Failure") + expect(String((exit as { failure: Error }).failure.message)).toContain("empty rg.exe") + }) + + test("rejects a CRC-corrupt entry instead of persisting a broken binary", async () => { + // Without checkSignature, zip.js decodes corrupt data "successfully"; those bytes get written + // to Global.Path.bin and trusted by every later session, breaking grep until the cache is cleared. + const payload = new Uint8Array([0xca, 0xfe, 0xba, 0xbe, 0xde, 0xad, 0xbe, 0xef, 0x11, 0x22, 0x33, 0x44]) + // level 0 stores the payload verbatim, so it can be located and corrupted precisely. + const zip = await zipWith([{ name: "rg.exe", body: payload }], { level: 0 }) + const bytes = new Uint8Array(zip) + const at = indexOfBytes(bytes, payload) + expect(at).toBeGreaterThan(-1) + bytes[at + 3] ^= 0xff + + const exit = await run(Effect.result(RipgrepBinary.unzipExecutable(bytes.buffer))) + + expect(exit._tag).toBe("Failure") + }) + + test("surfaces a decode failure as a typed error rather than throwing raw", async () => { + const garbage = new Uint8Array([0, 1, 2, 3, 4, 5, 6, 7]).buffer + + const exit = await run(Effect.result(RipgrepBinary.unzipExecutable(garbage))) + + expect(exit._tag).toBe("Failure") + }) +}) + +// Serial: `asWindows` redefines `process.platform` and `process.arch` process-wide, so a +// concurrently-running test would observe the wrong platform. +describe.serial("RipgrepBinary.filepath — Windows install path", () => { + /** Pretend to be 64-bit Windows so `filepath` selects the zip platform entry. */ + const asWindows = async (fn: () => Promise): Promise => { + const platform = Object.getOwnPropertyDescriptor(process, "platform")! + const arch = Object.getOwnPropertyDescriptor(process, "arch")! + Object.defineProperty(process, "platform", { value: "win32", configurable: true }) + Object.defineProperty(process, "arch", { value: "x64", configurable: true }) + try { + return await fn() + } finally { + Object.defineProperty(process, "platform", platform) + Object.defineProperty(process, "arch", arch) + } + } + + /** Minimal FSUtil recording what the install path does. Only the methods `filepath` uses. */ + function fsStub() { + const writes = new Map() + const renames: Array<[string, string]> = [] + const removed: string[] = [] + const service = { + isFile: () => Effect.succeed(false), + ensureDir: () => Effect.void, + writeWithDirs: (p: string, content: Uint8Array) => + Effect.sync(() => { + writes.set(p, content) + }), + rename: (from: string, to: string) => + Effect.sync(() => { + renames.push([from, to]) + const body = writes.get(from) + if (body) { + writes.delete(from) + writes.set(to, body) + } + }), + remove: (p: string) => + Effect.sync(() => { + removed.push(p) + }), + chmod: () => Effect.void, + copyFile: () => Effect.void, + makeTempDirectoryScoped: () => Effect.succeed("/tmp/unused"), + } + return { writes, renames, removed, layer: Layer.succeed(FSUtil.Service)(service as never) } + } + + /** A spawner that fails the test if anything tries to launch a process. */ + function spawnerStub(onSpawn: (cmd: string) => void) { + return Layer.succeed(ChildProcessSpawner)({ + spawn: (command: any) => + Effect.sync(() => { + onSpawn(String(command?.command ?? command)) + throw new Error("spawn must not be called") + }), + } as never) + } + + function httpStub(body: ArrayBuffer) { + return Layer.succeed(HttpClient.HttpClient)( + HttpClient.make((request) => + Effect.succeed(HttpClientResponse.fromWeb(request, new Response(body, { status: 200 }))), + ), + ) + } + + test("extracts and installs rg.exe with no process spawn, via a temp file it renames", async () => { + const payload = new Uint8Array([0x4d, 0x5a, 0x42, 0x43]) + const zip = await zipWith([{ name: "ripgrep-15.1.0-x86_64-pc-windows-msvc/rg.exe", body: payload }]) + + const spawned: string[] = [] + const fs = fsStub() + + const resolved = await asWindows(() => + Effect.runPromise( + Effect.gen(function* () { + const binary = yield* RipgrepBinary.Service + return yield* binary.filepath + }).pipe( + Effect.provide( + RipgrepBinary.layer.pipe( + Layer.provide(fs.layer), + Layer.provide(httpStub(zip)), + Layer.provide(spawnerStub((c) => spawned.push(c))), + ), + ), + ) as Effect.Effect, + ), + ) + + // The whole point of the fix: the Windows zip path must not shell out. + expect(spawned).toEqual([]) + expect(resolved.endsWith("rg.exe")).toBe(true) + // Installed atomically: written to a staging path, then renamed onto the target. + expect(fs.renames.length).toBe(1) + const [staged, published] = fs.renames[0]! + expect(published).toBe(resolved) + expect(staged.startsWith(`${resolved}.`)).toBe(true) + expect(staged.endsWith(".tmp")).toBe(true) + // Unique per attempt — a shared `${target}.tmp` lets concurrent cold-cache processes + // publish each other's partial downloads. + expect(staged).not.toBe(`${resolved}.tmp`) + expect(staged).toContain(`.${process.pid}.`) + expect(Array.from(fs.writes.get(resolved) ?? [])).toEqual(Array.from(payload)) + }) +}) diff --git a/packages/opencode/src/altimate/telemetry/index.ts b/packages/opencode/src/altimate/telemetry/index.ts index bb4c9ee467..ed2da7e1de 100644 --- a/packages/opencode/src/altimate/telemetry/index.ts +++ b/packages/opencode/src/altimate/telemetry/index.ts @@ -45,6 +45,23 @@ const log = Log.create({ service: "telemetry" }) */ // altimate_change end +/** True when a test runner is driving the process rather than a real user session. + * + * Deliberately keyed on test runners, NOT on CI. Running in CI is legitimate product usage — + * `altimate-code-actions` wraps this CLI and every invocation sets `CI`/`GITHUB_ACTIONS` — so + * gating on those would make a shipped product surface invisible. The polluting population was + * our own suites (`provider_id="test"`, `cli_version="local"`), all of which run under a test + * runner. `bun test` sets NODE_ENV=test, which covers both CI and developer machines. + * + * Set `ALTIMATE_TELEMETRY_FORCE=true` to opt back in, or point + * APPLICATIONINSIGHTS_CONNECTION_STRING at your own sink — an explicit sink is always honoured. + */ +function isAutomatedRun(): boolean { + if (process.env.ALTIMATE_TELEMETRY_FORCE === "true") return false + if (process.env.NODE_ENV === "test") return true + return Boolean(process.env.BUN_TEST || process.env.VITEST || process.env.JEST_WORKER_ID) +} + export namespace Telemetry { const FLUSH_INTERVAL_MS = 5_000 const MAX_BUFFER_SIZE = 200 @@ -1680,8 +1697,19 @@ export namespace Telemetry { } catch { // Config unavailable — proceed with telemetry enabled } - // App Insights: env var overrides default (for dev/testing), otherwise use the baked-in key - const connectionString = process.env.APPLICATIONINSIGHTS_CONNECTION_STRING ?? DEFAULT_CONNECTION_STRING + // App Insights: env var overrides default (for dev/testing), otherwise use the baked-in key. + // The baked-in key is refused under a test runner so suites never ship to the production + // resource. Note this deliberately does NOT key on CI — see isAutomatedRun. + // Telemetry's own tests set APPLICATIONINSIGHTS_CONNECTION_STRING explicitly and are unaffected — + // only the implicit production sink is withheld. 1,020 of 3,135 machine ids in a 14-day window + // were test processes, which regenerate their machine id every run — inflating every install + // and active-machine metric by ~33%. + const explicit = process.env.APPLICATIONINSIGHTS_CONNECTION_STRING + if (!explicit && isAutomatedRun()) { + buffer = [] + return + } + const connectionString = explicit ?? DEFAULT_CONNECTION_STRING const cfg = parseConnectionString(connectionString) if (!cfg) { buffer = [] diff --git a/packages/opencode/test/telemetry/automated-run.test.ts b/packages/opencode/test/telemetry/automated-run.test.ts new file mode 100644 index 0000000000..1820ba7002 --- /dev/null +++ b/packages/opencode/test/telemetry/automated-run.test.ts @@ -0,0 +1,202 @@ +import { afterEach, describe, expect, mock, spyOn, test } from "bun:test" +import { mkdtemp, rm } from "node:fs/promises" +import { tmpdir } from "node:os" +import path from "node:path" +import { Telemetry } from "../../src/altimate/telemetry" + +// A 14-day production window carried 1,020 test-process machine ids out of 3,135 total — roughly a +// third of every install and active-machine number, because a test process regenerates its machine +// id on every run. Test runners must never reach the baked-in production App Insights resource. +// CI on its own must still report: altimate-code-actions wraps this CLI, so gating on CI would +// blind a shipped product surface. + +const ENV_KEYS = [ + "ALTIMATE_TELEMETRY_DISABLED", + "ALTIMATE_TELEMETRY_FORCE", + "APPLICATIONINSIGHTS_CONNECTION_STRING", + "CI", + "NODE_ENV", + "BUN_TEST", + "VITEST", + "JEST_WORKER_ID", + "GITHUB_ACTIONS", + "BUILDKITE", + "GITLAB_CI", +] as const + +/** Run `fn` with exactly `env` set across all telemetry-relevant keys, then restore. + * + * HOME/USERPROFILE are redirected to a throwaway directory as well: the non-suppressed cases + * reach `doInit`'s machine-id block, which reads and writes `~/.altimate/machine-id` via + * `os.homedir()`. Without this the suite would mint a real machine id on the developer's + * machine, and assertions would depend on whatever was already there. + */ +async function withEnv(env: Partial>, fn: () => Promise) { + const saved = new Map() + for (const key of ENV_KEYS) { + saved.set(key, process.env[key]) + delete process.env[key] + } + const homeKeys = ["HOME", "USERPROFILE"] as const + for (const key of homeKeys) saved.set(key, process.env[key]) + const home = await mkdtemp(path.join(tmpdir(), "telemetry-home-")) + for (const key of homeKeys) process.env[key] = home + + Object.assign(process.env, env) + try { + await fn() + } finally { + for (const key of [...ENV_KEYS, ...homeKeys]) { + const value = saved.get(key) + if (value === undefined) delete process.env[key] + else process.env[key] = value + } + await rm(home, { recursive: true, force: true }).catch(() => {}) + } +} + +/** Init telemetry, emit one event, flush, and report every outbound request. */ +async function shippedRequests(): Promise> { + const urls: string[] = [] + const fetchMock = spyOn(global, "fetch").mockImplementation((async (input: any) => { + urls.push(String(input)) + return new Response("", { status: 200 }) + }) as unknown as typeof fetch) + try { + await Telemetry.init() + Telemetry.track({ + type: "session_start", + timestamp: 1000, + session_id: "s1", + model_id: "m1", + provider_id: "test", + agent: "builder", + project_id: "proj1", + os: "linux", + arch: "x64", + node_version: "v22.0.0", + }) + await Telemetry.flush() + return urls + } finally { + fetchMock.mockRestore() + } +} + +// Serial: every case mutates process-wide state — `process.env`, the global `fetch`, and the +// module-level Telemetry singleton (which `afterEach` shuts down). Bun runs a file's tests +// sequentially by default, so this is currently belt-and-braces; it stops `--concurrent` from +// silently turning these into flakes later. +describe.serial("telemetry: automated runs never reach the production sink", () => { + afterEach(async () => { + await Telemetry.shutdown() + mock.restore() + }) + + for (const marker of ["BUN_TEST", "VITEST", "JEST_WORKER_ID"]) { + test(`${marker} suppresses the baked-in connection string`, async () => { + await withEnv({ [marker]: "true" } as any, async () => { + expect(await shippedRequests()).toEqual([]) + }) + }) + } + + test("NODE_ENV=test suppresses the baked-in connection string", async () => { + // This is the one that matters: `bun test` sets it, on CI and on developer machines alike. + await withEnv({ NODE_ENV: "test" }, async () => { + expect(await shippedRequests()).toEqual([]) + }) + }) + + // CI alone must NOT suppress. altimate-code-actions wraps this CLI, so every run of that + // shipped product sets CI/GITHUB_ACTIONS — gating on those would blind a real product surface. + for (const marker of ["CI", "GITHUB_ACTIONS", "BUILDKITE", "GITLAB_CI"]) { + test(`${marker} alone still reports — running in CI is legitimate product usage`, async () => { + await withEnv({ [marker]: "true" } as any, async () => { + expect(await shippedRequests()).not.toEqual([]) + }) + }) + } + + test("a test runner inside CI is still suppressed", async () => { + await withEnv({ CI: "true", GITHUB_ACTIONS: "true", NODE_ENV: "test" }, async () => { + expect(await shippedRequests()).toEqual([]) + }) + }) + + test("an explicit connection string still ships — suites with their own sink keep working", async () => { + await withEnv( + { + NODE_ENV: "test", + APPLICATIONINSIGHTS_CONNECTION_STRING: "InstrumentationKey=e2e;IngestionEndpoint=https://sink.example.com", + }, + async () => { + expect(await shippedRequests()).toEqual(["https://sink.example.com/v2/track"]) + }, + ) + }) + + test("ALTIMATE_TELEMETRY_FORCE opts an automated run back in", async () => { + await withEnv({ NODE_ENV: "test", ALTIMATE_TELEMETRY_FORCE: "true" }, async () => { + expect(await shippedRequests()).not.toEqual([]) + }) + }) + + test("ALTIMATE_TELEMETRY_DISABLED still wins over the force flag", async () => { + await withEnv({ ALTIMATE_TELEMETRY_DISABLED: "true", ALTIMATE_TELEMETRY_FORCE: "true" }, async () => { + expect(await shippedRequests()).toEqual([]) + }) + }) + + test("ALTIMATE_TELEMETRY_DISABLED still wins over an explicit sink", async () => { + await withEnv( + { + ALTIMATE_TELEMETRY_DISABLED: "true", + APPLICATIONINSIGHTS_CONNECTION_STRING: "InstrumentationKey=e2e;IngestionEndpoint=https://sink.example.com", + }, + async () => { + expect(await shippedRequests()).toEqual([]) + }, + ) + }) + + test("the real test runner is detected without any env stubbing", async () => { + // Every other case sets its env explicitly, so nothing would notice if `bun test` stopped + // setting NODE_ENV=test — the assumption the whole gate rests on. This one deliberately + // touches no env at all and relies on the runner's own. + expect(process.env.NODE_ENV).toBe("test") + + const urls: string[] = [] + const fetchMock = spyOn(global, "fetch").mockImplementation((async (input: any) => { + urls.push(String(input)) + return new Response("", { status: 200 }) + }) as unknown as typeof fetch) + try { + await Telemetry.init() + Telemetry.track({ + type: "session_start", + timestamp: 1000, + session_id: "s1", + model_id: "m1", + provider_id: "test", + agent: "builder", + project_id: "proj1", + os: "linux", + arch: "x64", + node_version: "v22.0.0", + }) + await Telemetry.flush() + expect(urls).toEqual([]) + } finally { + fetchMock.mockRestore() + } + }) + + test("an ordinary interactive run is unaffected", async () => { + await withEnv({}, async () => { + const urls = await shippedRequests() + expect(urls.length).toBe(1) + expect(urls[0]).toContain("applicationinsights.azure.com") + }) + }) +})