Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
381 changes: 274 additions & 107 deletions packages/opencode/src/installation/index.ts

Large diffs are not rendered by default.

19 changes: 16 additions & 3 deletions packages/opencode/test/branding/upstream-merge-guard.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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"),

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: The claimed brand guard does not actually scan the detection implementation. The segment window (line 59 to export interface ResolvedInstall, line 78) covers only the regex-constant header, and the methodBlock window only covers the method() wrapper that calls resolveInstall(). Detection logic now lives in resolveInstall()'s body (lines 88-108), which neither not.toContain("opencode-ai") assertion covers. Extend the slice end marker so the resolvere body is included, so a stale upstream package-name path reintroduced inside the resolver is caught as the comment promises.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/opencode/test/branding/upstream-merge-guard.test.ts, line 60:

<comment>The claimed brand guard does not actually scan the detection implementation. The `segment` window (line 59 to `export interface ResolvedInstall`, line 78) covers only the regex-constant header, and the `methodBlock` window only covers the `method()` wrapper that calls `resolveInstall()`. Detection logic now lives in `resolveInstall()`'s body (lines 88-108), which neither `not.toContain("opencode-ai")` assertion covers. Extend the slice end marker so the resolvere body is included, so a stale upstream package-name path reintroduced inside the resolver is caught as the comment promises.</comment>

<file context>
@@ -51,13 +51,26 @@ describe("Installation script branding", () => {
+    // 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")
</file context>

)
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", () => {
Expand Down
26 changes: 19 additions & 7 deletions packages/opencode/test/install/upgrade-method.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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
})
})

Expand Down
16 changes: 14 additions & 2 deletions packages/opencode/test/installation/installation.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
}),
)

Expand All @@ -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
}),
)

Expand Down
82 changes: 82 additions & 0 deletions packages/opencode/test/installation/resolve-install.test.ts
Original file line number Diff line number Diff line change
@@ -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 -<os>-<arch> 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")
})
})
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,7 @@ function setPlatform(value: string) {
Object.defineProperty(process, "platform", { value, configurable: true })
}

type HttpHandler = (
request: HttpClientRequest.HttpClientRequest,
) => Response | Effect.Effect<Response, unknown>
type HttpHandler = (request: HttpClientRequest.HttpClientRequest) => Response | Effect.Effect<Response, unknown>

type SpawnResult = string | { code: number; stdout?: string; stderr?: string }
type SpawnCall = { cmd: string; args: readonly string[]; env?: Record<string, string>; stdin?: unknown }
Expand Down Expand Up @@ -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)))
Expand Down Expand Up @@ -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
})
})

Expand Down Expand Up @@ -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", () => {
Expand Down
Loading