From e98ba6daaf21909f1b4653bef600b9dd8edee0a3 Mon Sep 17 00:00:00 2001 From: Sarav Date: Fri, 11 Sep 2026 15:48:11 +0530 Subject: [PATCH] fix: resolve the install from the running binary; make upgrade failures diagnosable (#1305) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `Installation.method()` never established where the running executable came from. It guessed two ways, and both were unsound: - A substring test on `process.execPath`. `~/.local/bin` is a generic user bin dir, so an npm install with `npm config set prefix ~/.local` was classified `curl`, and `altimate upgrade` ran `curl | bash` — silently converting an npm install into a standalone one and leaving the npm copy orphaned on PATH. - A probe loop (`npm list -g`, `brew list`, ...) returning the first manager whose output mentioned the package. That answers "is this installed anywhere?", not "did THIS binary come from you", so it picked arbitrarily whenever several installs existed. Replaced with `resolveInstall()`, which resolves `realpath(process.execPath)` and matches the package segment. The npm `bin/altimate` shim `spawnSync()`s the per-platform package, so execPath always lands under `node_modules` for package-manager installs; the optional `--` suffix is matched explicitly. Removes up to seven subprocess spawns from the startup update-check path. Added a writability preflight so an upgrade that cannot succeed is refused before shelling out, with a message naming the directory and the exact remedy. Uses `npm root -g` rather than `/lib/node_modules`, which is Unix-only, and derives the bin dir from `npm prefix -g` because `npm bin -g` was removed in npm 9. Also fixed an asymmetry in the failure branch: the success path logged the real stdout/stderr while the failure path discarded them, so every non-permission failure (network, `E404`, `ENOSPC`, a failing lifecycle script) collapsed into an identical `Upgrade failed for npm (exit code N).` with nothing written anywhere. The real output is now logged locally, the message carries a classified hint plus a pointer to the log, and telemetry records a stable classification code instead of the generic string — previously every failed upgrade looked identical on a dashboard. The user-facing message and the telemetry payload stay redacted. Four existing tests asserted on the source text or the exact error string and were updated to track the new contract while preserving their intent (brand guard, redaction guards). Co-Authored-By: Claude Opus 5 (1M context) --- packages/opencode/src/installation/index.ts | 381 +++++++++++++----- .../branding/upstream-merge-guard.test.ts | 19 +- .../test/install/upgrade-method.test.ts | 26 +- .../test/installation/installation.test.ts | 16 +- .../test/installation/resolve-install.test.ts | 82 ++++ .../windows-installer-930.test.ts | 26 +- 6 files changed, 422 insertions(+), 128 deletions(-) create mode 100644 packages/opencode/test/installation/resolve-install.test.ts diff --git a/packages/opencode/src/installation/index.ts b/packages/opencode/src/installation/index.ts index 0fb8ab2751..bb0a82d6bc 100644 --- a/packages/opencode/src/installation/index.ts +++ b/packages/opencode/src/installation/index.ts @@ -8,6 +8,8 @@ import { errorMessage } from "@/util/error" import { ChildProcess } from "effect/unstable/process" import { AppProcess } from "@opencode-ai/core/process" import path from "path" +import fs from "fs" +import { Global } from "@opencode-ai/core/global" import { EventV2 } from "@opencode-ai/core/event" import { makeRuntime } from "@opencode-ai/core/effect/runtime" import semver from "semver" @@ -37,6 +39,112 @@ const UPGRADE_INSTALL_PS_URL = "https://www.altimate.sh/install.ps1" const UPGRADE_FETCH_TIMEOUT_MS = 15_000 // altimate_change end +// altimate_change start — deterministic install resolution (#1305) +// Detection used to guess two ways, and both were unsound: a substring test on +// process.execPath (`.local/bin` is a generic user bin dir, so an npm install with +// `npm config set prefix ~/.local` was classified "curl" and upgraded via +// `curl | bash`, orphaning the npm copy), and a probe loop asking each package +// manager "do you have this package?" — which answers a different question than +// "did THIS running binary come from you", so it picked arbitrarily whenever more +// than one install existed. +// +// The running binary's own path is the ground truth. The npm `bin/altimate` shim is +// a Node script that spawnSync()s the PLATFORM package's binary, so inside the CLI +// process.execPath is: +// /lib/node_modules/@altimateai/altimate-code/node_modules/ +// @altimateai/altimate-code-darwin-arm64/bin/altimate-code +// i.e. it always lands under node_modules for every package-manager install. Match +// the optional `--` suffix explicitly rather than relying on the +// wrapper name happening to be a prefix of the platform package name. +const PKG_SEGMENT_RE = + /[\\/]node_modules[\\/]@altimateai[\\/]altimate-code(?:-[a-z0-9]+-[a-z0-9]+(?:-[a-z0-9]+)?)?(?:[\\/]|$)/i +// pnpm global installs may expose the package via the `.pnpm` virtual store OR via a +// plain `pnpm/global/` link path (no `.pnpm` segment), so match both spellings — +// otherwise the plain layout falls through to the npm default and routes upgrades at +// the wrong manager. +const PNPM_SEGMENT_RE = /[\\/](?:\.pnpm|pnpm)[\\/]/i +const BUN_SEGMENT_RE = /[\\/]\.bun[\\/]/i +const YARN_SEGMENT_RE = /[\\/](?:\.yarn|yarn[\\/]global)[\\/]/i +// Homebrew bin entries are symlinks into Cellar, so realpath lands there. Match the +// Cellar segment rather than the prefix: /usr/local is also a common npm prefix. +const BREW_SEGMENT_RE = /[\\/]Cellar[\\/]altimate-code[\\/]/i +const SCOOP_SEGMENT_RE = /[\\/]scoop[\\/]apps[\\/]/i +const CHOCO_SEGMENT_RE = /[\\/]chocolatey[\\/]/i +// The standalone (curl / install.ps1 / `install --binary`) layout. `.opencode/bin` is +// the pre-v0.7.1 directory name, kept for users who have not re-installed since. +// NOTE: `.local/bin` is deliberately NOT here — see the comment above. +const STANDALONE_SEGMENT_RE = /[\\/]\.(?:altimate|opencode)[\\/]bin[\\/]/i + +export interface ResolvedInstall { + readonly method: Method + /** Directory the upgrade would mutate. Only set where we can name it without a subprocess. */ + readonly root?: string +} + +/** Resolve the install that produced THIS process. + * + * Pure in (execPath, env) so it can be unit-tested against fabricated layouts + * without spawning real installs. */ +export function resolveInstall( + execPath: string = realExecPath(), + env: NodeJS.ProcessEnv = process.env, +): ResolvedInstall { + // The shim honours ALTIMATE_CODE_BIN_PATH ahead of everything else, so the running + // binary is whatever the user pointed at — not something an installer manages. + // Never auto-upgrade a pinned path. + if (env["ALTIMATE_CODE_BIN_PATH"]) return { method: "unknown" } + + if (PKG_SEGMENT_RE.test(execPath)) { + if (PNPM_SEGMENT_RE.test(execPath)) return { method: "pnpm" } + if (BUN_SEGMENT_RE.test(execPath)) return { method: "bun" } + if (YARN_SEGMENT_RE.test(execPath)) return { method: "yarn" } + return { method: "npm" } + } + if (BREW_SEGMENT_RE.test(execPath)) return { method: "brew" } + if (SCOOP_SEGMENT_RE.test(execPath)) return { method: "scoop" } + if (CHOCO_SEGMENT_RE.test(execPath)) return { method: "choco" } + if (STANDALONE_SEGMENT_RE.test(execPath)) return { method: "curl", root: path.dirname(execPath) } + return { method: "unknown" } +} + +/** realpath so a symlinked bin entry (npm, brew) resolves to the file it points at. + * Falls back to the raw path when the file is gone or unreadable. */ +function realExecPath(): string { + try { + return fs.realpathSync(process.execPath) + } catch { + return process.execPath + } +} + +function isWritable(dir: string): boolean { + try { + fs.accessSync(dir, fs.constants.W_OK) + return true + } catch { + return false + } +} + +/** Classify a failed upgrade into a stable code plus a message safe to show. + * + * Deliberately does NOT echo the package manager's stderr — it can carry tokens and + * environment. The classification is derived from it, the raw text is only logged + * locally (see the logWarning in upgrade()). */ +function classifyFailure(stderr: string, stdout: string): { code: string; hint?: string } { + const t = `${stderr}\n${stdout}` + if (/EACCES|EPERM|permission denied/i.test(t)) + return { code: "permission", hint: "the install directory is not writable" } + if (/ENOTFOUND|ECONNREFUSED|ETIMEDOUT|EAI_AGAIN|network|ENETUNREACH/i.test(t)) + return { code: "network", hint: "the registry could not be reached" } + if (/E404|404 Not Found/i.test(t)) return { code: "not-found", hint: "that version does not exist in the registry" } + if (/ENOSPC|no space left/i.test(t)) return { code: "disk-full", hint: "the disk is full" } + if (/ETARGET|No matching version/i.test(t)) + return { code: "no-matching-version", hint: "no published version satisfies that range" } + return { code: "unknown" } +} +// altimate_change end + export type Method = "curl" | "npm" | "yarn" | "pnpm" | "bun" | "brew" | "scoop" | "choco" | "unknown" export type ReleaseType = "patch" | "minor" | "major" @@ -177,51 +285,123 @@ export const layer: Layer.Layer/node_modules and the shims at itself, so the Unix + // /lib/node_modules is wrong there. `npm bin -g` was REMOVED in npm 9 + // ("Unknown command: bin"), so derive the bin dir from the prefix instead. + const root = (yield* text(["npm", "root", "-g"])).trim() + const prefix = (yield* text(["npm", "prefix", "-g"])).trim() + const bin = prefix ? (process.platform === "win32" ? prefix : path.join(prefix, "bin")) : "" + return [root, bin].filter(Boolean) + } + case "pnpm": { + // Both: a global install writes the store root AND the shim dir; checking only + // one lets the other fail with EACCES after we have already shelled out. + const root = (yield* text(["pnpm", "root", "-g"])).trim() + const bin = (yield* text(["pnpm", "bin", "-g"])).trim() + return [root, bin].filter(Boolean) + } + case "bun": { + const bin = (yield* text(["bun", "pm", "bin", "-g"])).trim() + return [bin].filter(Boolean) + } + case "yarn": { + const dir = (yield* text(["yarn", "global", "dir"])).trim() + const bin = (yield* text(["yarn", "global", "bin"])).trim() + return [dir, bin].filter(Boolean) + } + case "curl": { + const resolved = resolveInstall() + return resolved.root ? [resolved.root] : [] + } + // brew / scoop / choco own their own elevation and policy — do not second-guess them. + default: + return [] as string[] + } + }) + + const remediation = (m: Method, dir: string, target: string) => { + const pkg = `@altimateai/altimate-code@${target}` + switch (m) { + case "npm": + return `Cannot write to the npm global prefix (${dir}). Run \`sudo npm install -g ${pkg}\`, or switch to a user-owned prefix with \`npm config set prefix ~/.npm-global\`.` + case "pnpm": + return `Cannot write to the pnpm global directory (${dir}). Run \`pnpm setup\` to use a user-owned location, or re-run the install with elevated permissions.` + case "bun": + return `Cannot write to the bun global bin directory (${dir}). Set BUN_INSTALL to a user-owned location, or re-run the install with elevated permissions.` + case "yarn": + return `Cannot write to the yarn global directory (${dir}). Set a user-owned prefix with \`yarn config set prefix ~/.yarn\`, or re-run with elevated permissions.` + case "curl": + return `Cannot write to the install directory (${dir}). Fix its permissions, or re-run the installer.` + default: + return `Cannot write to the install directory (${dir}).` + } + } + + /** Returns an error message when the upgrade cannot possibly succeed, else undefined. + * + * Checking first means we never shell out to a command that is going to fail on + * permissions — which is what produced the old, undiagnosable + * "Upgrade failed for npm (exit code 243)." */ + const preflight = Effect.fnUntraced(function* (m: Method, target: string) { + const dirs = yield* globalDirs(m) + for (const dir of dirs) { + if (!dir) continue + // A directory that does not exist yet is not a permission problem: the package + // manager creates it. Only an EXISTING, unwritable directory is a hard stop. + if (!fs.existsSync(dir)) continue + if (!isWritable(dir)) return remediation(m, dir, target) + } + return undefined + }) + // altimate_change end + const upgradeScriptShell = Effect.fnUntraced(function* () { const bashVersion = yield* text(["bash", "--version"]) if (bashVersion) return "bash" return "sh" }) - const upgradeCurl = Effect.fnUntraced( - function* (target: string) { - // altimate_change start — friendly fetch error + manual-recovery hint, branded install URL, bounded timeout - const response = yield* httpOk - .execute(HttpClientRequest.get(UPGRADE_INSTALL_URL)) - .pipe( - Effect.timeout(UPGRADE_FETCH_TIMEOUT_MS), - Effect.mapError( - (err) => - new UpgradeFailedError({ - stderr: - `Could not download install script from ${UPGRADE_INSTALL_URL}: ${errorMessage(err)}. ` + - `Re-run the install manually: curl -fsSL ${UPGRADE_INSTALL_URL} | bash — ` + - `or download a release binary directly from https://github.com/AltimateAI/altimate-code/releases/latest`, - }), - ), - ) - const body = yield* response.text.pipe( - Effect.mapError(() => new UpgradeFailedError({ stderr: upgradeFailure("curl") })), - ) - // altimate_change end - const bodyBytes = new TextEncoder().encode(body) - const shell = yield* upgradeScriptShell() - const result = yield* appProcess - .run( - ChildProcess.make(shell, [], { - stdin: Stream.make(bodyBytes), - env: { VERSION: target }, - extendEnv: true, + const upgradeCurl = Effect.fnUntraced(function* (target: string) { + // altimate_change start — friendly fetch error + manual-recovery hint, branded install URL, bounded timeout + const response = yield* httpOk.execute(HttpClientRequest.get(UPGRADE_INSTALL_URL)).pipe( + Effect.timeout(UPGRADE_FETCH_TIMEOUT_MS), + Effect.mapError( + (err) => + new UpgradeFailedError({ + stderr: + `Could not download install script from ${UPGRADE_INSTALL_URL}: ${errorMessage(err)}. ` + + `Re-run the install manually: curl -fsSL ${UPGRADE_INSTALL_URL} | bash — ` + + `or download a release binary directly from https://github.com/AltimateAI/altimate-code/releases/latest`, }), - ) - .pipe(Effect.mapError(() => new UpgradeFailedError({ stderr: upgradeFailure("curl") }))) - return { - code: result.exitCode, - stdout: result.stdout.toString("utf8"), - stderr: result.stderr.toString("utf8"), - } - }, - ) + ), + ) + const body = yield* response.text.pipe( + Effect.mapError(() => new UpgradeFailedError({ stderr: upgradeFailure("curl") })), + ) + // altimate_change end + const bodyBytes = new TextEncoder().encode(body) + const shell = yield* upgradeScriptShell() + const result = yield* appProcess + .run( + ChildProcess.make(shell, [], { + stdin: Stream.make(bodyBytes), + env: { VERSION: target }, + extendEnv: true, + }), + ) + .pipe(Effect.mapError(() => new UpgradeFailedError({ stderr: upgradeFailure("curl") }))) + return { + code: result.exitCode, + stdout: result.stdout.toString("utf8"), + stderr: result.stderr.toString("utf8"), + } + }) // altimate_change start — Windows curl-install upgrade via PowerShell // The curl/standalone install on native Windows lives in %USERPROFILE%\.altimate\bin @@ -231,20 +411,18 @@ export const layer: Layer.Layer - new UpgradeFailedError({ - stderr: - `Could not download install script from ${UPGRADE_INSTALL_PS_URL}: ${errorMessage(err)}. ` + - `Re-run the install manually: powershell -c "irm ${UPGRADE_INSTALL_PS_URL} | iex" — ` + - `or download a release binary directly from https://github.com/AltimateAI/altimate-code/releases/latest`, - }), - ), - ) + yield* httpOk.execute(HttpClientRequest.head(UPGRADE_INSTALL_PS_URL)).pipe( + Effect.timeout(UPGRADE_FETCH_TIMEOUT_MS), + Effect.mapError( + (err) => + new UpgradeFailedError({ + stderr: + `Could not download install script from ${UPGRADE_INSTALL_PS_URL}: ${errorMessage(err)}. ` + + `Re-run the install manually: powershell -c "irm ${UPGRADE_INSTALL_PS_URL} | iex" — ` + + `or download a release binary directly from https://github.com/AltimateAI/altimate-code/releases/latest`, + }), + ), + ) return yield* run( ["powershell", "-NoProfile", "-ExecutionPolicy", "Bypass", "-Command", `irm ${UPGRADE_INSTALL_PS_URL} | iex`], { env: { VERSION: target } }, @@ -260,52 +438,13 @@ export const layer: Layer.Layer Effect.Effect }> = [ - { name: "npm", command: () => text(["npm", "list", "-g", "--depth=0"]) }, - { name: "yarn", command: () => text(["yarn", "global", "list"]) }, - { name: "pnpm", command: () => text(["pnpm", "list", "-g", "--depth=0"]) }, - { name: "bun", command: () => text(["bun", "pm", "ls", "-g"]) }, - // altimate_change start — brew formula name - { name: "brew", command: () => text(["brew", "list", "--formula", "altimate-code"]) }, - // altimate_change end - { name: "scoop", command: () => text(["scoop", "list", "opencode"]) }, - { name: "choco", command: () => text(["choco", "list", "--limit-output", "opencode"]) }, - ] - - checks.sort((a, b) => { - const aMatches = exec.includes(a.name) - const bMatches = exec.includes(b.name) - if (aMatches && !bMatches) return -1 - if (!aMatches && bMatches) return 1 - return 0 - }) - - for (const check of checks) { - const output = yield* check.command() - // altimate_change start — package names for detection - const installedName = - check.name === "brew" - ? "altimate-code" - : check.name === "choco" || check.name === "scoop" - ? "opencode" - : "@altimateai/altimate-code" - // altimate_change end - if (output.includes(installedName)) { - return check.name - } - } - - return "unknown" as Method }), latest: Effect.fn("Installation.latest")(function* (installMethod?: Method) { const detectedMethod = installMethod || (yield* result.method()) @@ -376,12 +515,15 @@ export const layer: Layer.Layer (exit code N)." with nothing written anywhere. + // The log file is local and already carries this content on success, so logging + // it here is consistency, not new exposure — the user-facing message and the + // telemetry payload both stay redacted. + const classified = classifyFailure(upgradeResult?.stderr ?? "", upgradeResult?.stdout ?? "") + yield* Effect.logWarning("upgrade failed", { + method: m, + target, + code: upgradeResult?.code, + reason: classified.code, + stdout: upgradeResult?.stdout, + stderr: upgradeResult?.stderr, + }) + const base = upgradeFailure(m, upgradeResult) + const stderr = [ + base, + classified.hint ? `Likely cause: ${classified.hint}.` : undefined, + `Details were written to ${Global.Path.log}.`, + ] + .filter(Boolean) + .join(" ") const T = yield* Effect.promise(() => getTelemetry()) T.track({ type: "upgrade_attempted", @@ -449,9 +611,12 @@ export const layer: Layer.Layer layer.pipe(Layer.provide(FetchHttpClient.layer), Layer.provide(AppProcess.defaultLayer))) +export const defaultLayer = Layer.suspend(() => + layer.pipe(Layer.provide(FetchHttpClient.layer), Layer.provide(AppProcess.defaultLayer)), +) // altimate_change end const { runPromise } = makeRuntime(Service, defaultLayer) diff --git a/packages/opencode/test/branding/upstream-merge-guard.test.ts b/packages/opencode/test/branding/upstream-merge-guard.test.ts index 28d441cfa4..8d4714f8c8 100644 --- a/packages/opencode/test/branding/upstream-merge-guard.test.ts +++ b/packages/opencode/test/branding/upstream-merge-guard.test.ts @@ -51,13 +51,26 @@ describe("Installation script branding", () => { }) test("method() detects npm-installed @altimateai/altimate-code, not opencode-ai", () => { - // The installedName for npm/bun/pnpm must be our scoped package, not upstream + // altimate_change start — #1305: detection moved out of the `method:` block into + // resolveInstall()/PKG_SEGMENT_RE, so slicing between the `method:` and `latest:` + // markers no longer covers it. Assert on the package segment that detection actually + // matches; the brand intent (our scope, never upstream's) is unchanged. + const segment = installSrc.slice( + installSrc.indexOf("const PKG_SEGMENT_RE"), + installSrc.indexOf("export interface ResolvedInstall"), + ) + expect(segment).toContain("@altimateai") + expect(segment).toContain("altimate-code") + expect(segment).not.toContain("opencode-ai") + // The resolver must be what method() returns, so the guard cannot be bypassed by + // leaving a stale detection path behind. const methodBlock = installSrc.slice( installSrc.indexOf('method: Effect.fn("Installation.method")'), installSrc.indexOf('latest: Effect.fn("Installation.latest")'), ) - expect(methodBlock).toContain("@altimateai/altimate-code") - expect(methodBlock).not.toMatch(/installedName[^@]*opencode-ai/) + expect(methodBlock).toContain("resolveInstall()") + expect(methodBlock).not.toMatch(/opencode-ai/) + // altimate_change end }) test("method() detects brew formula as altimate-code, not opencode", () => { diff --git a/packages/opencode/test/install/upgrade-method.test.ts b/packages/opencode/test/install/upgrade-method.test.ts index 653ee77e4b..a4823a9cb0 100644 --- a/packages/opencode/test/install/upgrade-method.test.ts +++ b/packages/opencode/test/install/upgrade-method.test.ts @@ -8,10 +8,7 @@ import { describe, test, expect } from "bun:test" import fs from "fs" import path from "path" -const INSTALLATION_SRC = fs.readFileSync( - path.resolve(import.meta.dir, "../../src/installation/index.ts"), - "utf-8", -) +const INSTALLATION_SRC = fs.readFileSync(path.resolve(import.meta.dir, "../../src/installation/index.ts"), "utf-8") const CORE_VERSION_SRC = fs.readFileSync( path.resolve(import.meta.dir, "../../../../packages/core/src/installation/version.ts"), "utf-8", @@ -31,9 +28,24 @@ describe("installation method detection", () => { expect(INSTALLATION_SRC).toContain('"brew", "list", "--formula"') }) - test("method detection prioritizes matching exec path", () => { - // checks.sort puts the manager matching process.execPath first - expect(INSTALLATION_SRC).toContain("exec.includes(a.name)") + test("method detection resolves the running binary, not a package-manager listing", () => { + // altimate_change start — #1305: detection no longer sorts a probe list by execPath + // substring. It resolves realpath(process.execPath) and matches the package segment, + // so the assertion tracks the new contract rather than the deleted `checks` array. + expect(INSTALLATION_SRC).toContain("resolveInstall(") + expect(INSTALLATION_SRC).toContain("fs.realpathSync(process.execPath)") + // The probe loop must stay gone: it answered "is this package installed anywhere?", + // which picks arbitrarily when more than one install exists. + expect(INSTALLATION_SRC).not.toContain("exec.includes(a.name)") + // altimate_change end + }) + + test("`.local/bin` is not treated as a standalone install", () => { + // altimate_change start — #1305: `.local/bin` is a generic user bin dir. Treating it + // as curl misrouted `npm config set prefix ~/.local` installs into `curl | bash`, + // which orphaned the npm copy and left two binaries fighting over PATH. + expect(INSTALLATION_SRC).not.toMatch(/path\.join\("\.local", "bin"\)/) + // altimate_change end }) }) diff --git a/packages/opencode/test/installation/installation.test.ts b/packages/opencode/test/installation/installation.test.ts index bb57f836c1..40f625fabc 100644 --- a/packages/opencode/test/installation/installation.test.ts +++ b/packages/opencode/test/installation/installation.test.ts @@ -186,10 +186,16 @@ describe("installation", () => { Effect.gen(function* () { const error = yield* Effect.flip(Installation.use.upgrade("npm", "9.9.9")) expect(error).toBeInstanceOf(Installation.UpgradeFailedError) - expect(error.stderr).toBe("Upgrade failed for npm (exit code 1).") + // altimate_change start — #1305: the message now also points at the local log, + // where the REAL stderr is written. Redaction is what this test guards, so the + // not.toContain assertions below are the contract; the prefix is matched rather + // than compared exactly so the pointer can be appended. + expect(error.stderr).toContain("Upgrade failed for npm (exit code 1).") + expect(error.stderr).toContain("Details were written to") expect(error.message).toBe(error.stderr) expect(error.stderr).not.toContain("secret") expect(error.stderr).not.toContain("command output") + // altimate_change end }), ) @@ -206,10 +212,16 @@ describe("installation", () => { Effect.gen(function* () { const error = yield* Effect.flip(Installation.use.upgrade("curl", "9.9.9")) expect(error).toBeInstanceOf(Installation.UpgradeFailedError) - expect(error.stderr).toBe("Upgrade failed for curl (exit code 1).") + // altimate_change start — #1305: the message now also points at the local log, + // where the REAL stderr is written. Redaction is what this test guards, so the + // not.toContain assertions below are the contract; the prefix is matched rather + // than compared exactly so the pointer can be appended. + expect(error.stderr).toContain("Upgrade failed for curl (exit code 1).") + expect(error.stderr).toContain("Details were written to") expect(error.message).toBe(error.stderr) expect(error.stderr).not.toContain("secret") expect(error.stderr).not.toContain("script output") + // altimate_change end }), ) diff --git a/packages/opencode/test/installation/resolve-install.test.ts b/packages/opencode/test/installation/resolve-install.test.ts new file mode 100644 index 0000000000..a89c808871 --- /dev/null +++ b/packages/opencode/test/installation/resolve-install.test.ts @@ -0,0 +1,82 @@ +/** + * Install resolution (#1305). + * + * `resolveInstall()` answers "which install produced THIS process", replacing a + * substring test on execPath plus a probe loop that asked each package manager + * whether it had the package at all. The second question picks arbitrarily when more + * than one install exists, which is the common case once a user has tried both the + * curl installer and npm. + * + * These cases are table-driven over fabricated paths because the real layouts cannot + * be created on a test machine. + */ +import { describe, test, expect } from "bun:test" +import { resolveInstall, type Method } from "../../src/installation" + +const NPM_PREFIXED = "/usr/local/lib/node_modules/@altimateai/altimate-code" +const PLATFORM = "node_modules/@altimateai/altimate-code-darwin-arm64/bin/altimate-code" + +describe("resolveInstall", () => { + const cases: Array<[string, string, Method]> = [ + // The npm bin/altimate shim spawns the PLATFORM package, so execPath is the nested + // platform binary rather than the wrapper — detection must match the -- suffix. + ["npm, default prefix", `${NPM_PREFIXED}/${PLATFORM}`, "npm"], + // Regression: this is the layout the old `.local/bin` rule misread as "curl", which + // made `altimate upgrade` run `curl | bash` and orphan the npm install. + [ + "npm, prefix set to ~/.local", + "/home/u/.local/lib/node_modules/@altimateai/altimate-code/node_modules/@altimateai/altimate-code-linux-x64/bin/altimate-code", + "npm", + ], + [ + "pnpm, virtual store layout", + "/home/u/.local/share/pnpm/global/5/.pnpm/@altimateai+altimate-code@0.11.2/node_modules/@altimateai/altimate-code-linux-x64/bin/altimate-code", + "pnpm", + ], + [ + "pnpm, plain global link layout", + "/home/u/.local/share/pnpm/global/5/node_modules/@altimateai/altimate-code-linux-x64/bin/altimate-code", + "pnpm", + ], + [ + "bun global", + "/home/u/.bun/install/global/node_modules/@altimateai/altimate-code-linux-x64/bin/altimate-code", + "bun", + ], + ["yarn global", "/home/u/.yarn/global/node_modules/@altimateai/altimate-code-linux-x64/bin/altimate-code", "yarn"], + // Homebrew bin entries are symlinks into Cellar; realpath lands there. Matching the + // Cellar segment (not the prefix) keeps /usr/local from colliding with npm. + ["brew, apple silicon", "/opt/homebrew/Cellar/altimate-code/0.11.2/bin/altimate", "brew"], + ["brew, intel prefix", "/usr/local/Cellar/altimate-code/0.11.2/bin/altimate", "brew"], + ["standalone install", "/home/u/.altimate/bin/altimate", "curl"], + ["standalone, pre-v0.7.1 dir", "/home/u/.opencode/bin/altimate", "curl"], + ["scoop", "C:\\Users\\u\\scoop\\apps\\altimate-code\\current\\altimate.exe", "scoop"], + ["choco", "C:\\ProgramData\\chocolatey\\lib\\altimate-code\\tools\\altimate.exe", "choco"], + // A dev build or an unrecognised location must not be attributed to a package + // manager — "unknown" degrades to notify-only rather than running someone else's + // installer over it. + ["dev build", "/tmp/build/dist/altimate", "unknown"], + ] + + for (const [name, execPath, expected] of cases) { + test(`${name} -> ${expected}`, () => { + expect(resolveInstall(execPath, {}).method).toBe(expected) + }) + } + + test("a pinned ALTIMATE_CODE_BIN_PATH is never attributed to an installer", () => { + // The shim honours this ahead of everything else, so the running binary is whatever + // the user pointed at. Auto-upgrading it would overwrite a deliberate choice. + const env = { ALTIMATE_CODE_BIN_PATH: "/somewhere/custom/altimate" } + expect(resolveInstall(`${NPM_PREFIXED}/${PLATFORM}`, env).method).toBe("unknown") + }) + + test("standalone resolution reports the directory the upgrade would write", () => { + expect(resolveInstall("/home/u/.altimate/bin/altimate", {}).root).toBe("/home/u/.altimate/bin") + }) + + test("a plain user bin directory is not a standalone install", () => { + // `.local/bin` on its own carries no information about who installed the binary. + expect(resolveInstall("/home/u/.local/bin/altimate", {}).method).toBe("unknown") + }) +}) diff --git a/packages/opencode/test/release-validation/windows-installer-930.test.ts b/packages/opencode/test/release-validation/windows-installer-930.test.ts index 1a31bda539..b5e253ed10 100644 --- a/packages/opencode/test/release-validation/windows-installer-930.test.ts +++ b/packages/opencode/test/release-validation/windows-installer-930.test.ts @@ -60,9 +60,7 @@ function setPlatform(value: string) { Object.defineProperty(process, "platform", { value, configurable: true }) } -type HttpHandler = ( - request: HttpClientRequest.HttpClientRequest, -) => Response | Effect.Effect +type HttpHandler = (request: HttpClientRequest.HttpClientRequest) => Response | Effect.Effect type SpawnResult = string | { code: number; stdout?: string; stderr?: string } type SpawnCall = { cmd: string; args: readonly string[]; env?: Record; stdin?: unknown } @@ -115,9 +113,7 @@ function upgradeWith(input: { setPlatform(input.platform) const appProcess = AppProcess.layer.pipe(Layer.provide(mockSpawner(input.spawn))) const layer = Installation.layer.pipe( - Layer.provide( - mockHttpClient(input.http ?? (() => new Response("", { status: 200, statusText: "OK" }))), - ), + Layer.provide(mockHttpClient(input.http ?? (() => new Response("", { status: 200, statusText: "OK" })))), Layer.provide(appProcess), ) return Effect.runPromise(Installation.use.upgrade("curl", input.target ?? "1.2.3").pipe(Effect.provide(layer))) @@ -341,15 +337,25 @@ describe("upgradePowershell result shape is consumed by upgrade()", () => { // detect with instanceof (matches src/cli/cmd/upgrade.ts) rather than the removed .isInstance() static. expect(err instanceof Installation.UpgradeFailedError).toBe(true) // altimate_change end - expect((err as any).stderr).toBe("Upgrade failed for curl (exit code 1).") + // altimate_change start — #1305: message keeps the sanitized prefix and now also points + // at the local log, where the real installer stderr is written. + expect((err as any).stderr).toContain("Upgrade failed for curl (exit code 1).") + expect((err as any).stderr).toContain("Details were written to") + expect((err as any).stderr).not.toContain("powershell not found") + // altimate_change end // An error telemetry event was emitted carrying the sanitized stderr. expect(tracked).toHaveLength(1) expect(tracked[0].type).toBe("upgrade_attempted") expect(tracked[0].status).toBe("error") expect(tracked[0].to_version).toBe("1.2.3") - expect(tracked[0].error).toBe("Upgrade failed for curl (exit code 1).") + // altimate_change start — #1305: telemetry now carries a stable classification code + // plus the exit status instead of the generic message. The old value was identical for + // every failure, so causes could not be told apart on a dashboard. Redaction is + // unchanged — the installer's stderr still never reaches the event. + expect(tracked[0].error).toBe("unknown: exit 1") expect(tracked[0].error).not.toContain("powershell not found") + // altimate_change end }) }) @@ -446,7 +452,9 @@ describe("install.ps1 — GITHUB_PATH emission gated on GitHub Actions (static)" describe("install.ps1 — missing altimate.exe in archive fails + cleans up (static)", () => { test("throws 'Archive did not contain' when the extracted binary is absent", () => { // if (-not (Test-Path $extracted)) { throw "Archive did not contain $BinaryName" } - expect(PS1).toMatch(/if\s*\(-not\s*\(Test-Path\s+\$extracted\)\)\s*\{\s*throw\s+"Archive did not contain \$BinaryName"/) + expect(PS1).toMatch( + /if\s*\(-not\s*\(Test-Path\s+\$extracted\)\)\s*\{\s*throw\s+"Archive did not contain \$BinaryName"/, + ) }) test("the temp dir (altimate_install_$PID) is removed in a finally block", () => {