From a8a08c61a1c5015afcdd1a9179936363cea9c8fa Mon Sep 17 00:00:00 2001 From: anandgupta42 Date: Wed, 17 Jun 2026 22:35:30 -0700 Subject: [PATCH] test: add release-validation coverage for post-`v0.8.7` merged PRs Adds regression tests for the 8 PRs merged since `v0.8.7`, authored during pre-release validation. Two independent authoring passes (suffix `-codex.test.ts` from the codex CLI track, plus `release-validation/*.test.ts` from the Claude multi-agent track) intentionally overlap to maximize edge-case coverage. opencode (`packages/opencode/test/release-validation/`): - `question-937*` non-interactive question tool: `ALTIMATE_NON_INTERACTIVE` / `ALTIMATE_FORCE_INTERACTIVE` / `ALTIMATE_AUTO_ANSWER` matrix, output-text contract, `run.ts`/`bash.ts` env guards - `mcp-datamate-893*` MCP config normalize/merge, enabled-state persistence, recursive `**/mcp.json` discovery, datamate transport selection - `session-transcript-941*` transcript REST endpoint: query coercion, content negotiation, tool/thinking detail gating, error schema - `serve-upgrade-940*` headless `serve` startup upgrade check: scheduling, failure isolation, version-compare boundaries - `serve-trace-log-929*` trace-directory startup logging - `chunk-timeout-844*` `DEFAULT_CHUNK_TIMEOUT` SSE watchdog behavior - `windows-installer-930*` `install.ps1` static analysis (HTTPS URLs, error handling, PATH safety, idempotency, no secret leakage) + win32 dispatch dbt-tools (`packages/dbt-tools/test/`): - `dbt-cli-release-validation.test.ts` #933 error-bubbling: malformed JSON, exit-code redaction, ANSI stripping, no-signal fallback - `dbt-cli-extra-codex.test.ts` result-shape, `--limit` boundaries, last-error selection, inline-compile error paths All new tests pass and are typecheck-clean. 4 `test.todo` markers document real, pre-existing edge cases surfaced during review (transcript path-traversal -> 500, `normalizeMcpConfig` dropping `updatedAt`, `install.ps1` missing archive checksum, `execDbtCompile` stale-manifest fallback). No source files are modified. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../test/dbt-cli-extra-codex.test.ts | 290 +++++++++++ .../test/dbt-cli-release-validation.test.ts | 106 ++++ .../chunk-timeout-844.test.ts | 267 ++++++++++ .../mcp-datamate-893-codex.test.ts | 354 ++++++++++++++ .../mcp-datamate-893.test.ts | 459 ++++++++++++++++++ .../question-937-codex.test.ts | 357 ++++++++++++++ .../release-validation/question-937.test.ts | 363 ++++++++++++++ .../serve-trace-log-929.test.ts | 175 +++++++ .../serve-upgrade-940-codex.test.ts | 200 ++++++++ .../serve-upgrade-940.test.ts | 334 +++++++++++++ .../session-transcript-941-codex.test.ts | 400 +++++++++++++++ .../session-transcript-941.test.ts | 312 ++++++++++++ .../windows-installer-930-codex.test.ts | 193 ++++++++ .../windows-installer-930.test.ts | 428 ++++++++++++++++ 14 files changed, 4238 insertions(+) create mode 100644 packages/dbt-tools/test/dbt-cli-extra-codex.test.ts create mode 100644 packages/dbt-tools/test/dbt-cli-release-validation.test.ts create mode 100644 packages/opencode/test/release-validation/chunk-timeout-844.test.ts create mode 100644 packages/opencode/test/release-validation/mcp-datamate-893-codex.test.ts create mode 100644 packages/opencode/test/release-validation/mcp-datamate-893.test.ts create mode 100644 packages/opencode/test/release-validation/question-937-codex.test.ts create mode 100644 packages/opencode/test/release-validation/question-937.test.ts create mode 100644 packages/opencode/test/release-validation/serve-trace-log-929.test.ts create mode 100644 packages/opencode/test/release-validation/serve-upgrade-940-codex.test.ts create mode 100644 packages/opencode/test/release-validation/serve-upgrade-940.test.ts create mode 100644 packages/opencode/test/release-validation/session-transcript-941-codex.test.ts create mode 100644 packages/opencode/test/release-validation/session-transcript-941.test.ts create mode 100644 packages/opencode/test/release-validation/windows-installer-930-codex.test.ts create mode 100644 packages/opencode/test/release-validation/windows-installer-930.test.ts diff --git a/packages/dbt-tools/test/dbt-cli-extra-codex.test.ts b/packages/dbt-tools/test/dbt-cli-extra-codex.test.ts new file mode 100644 index 000000000..b6dc6dd9d --- /dev/null +++ b/packages/dbt-tools/test/dbt-cli-extra-codex.test.ts @@ -0,0 +1,290 @@ +import { describe, test, expect } from "bun:test" +import { mock, beforeEach, afterEach } from "bun:test" +import * as realChildProcess from "child_process" +import { chmodSync, mkdirSync, readFileSync, rmSync, writeFileSync } from "fs" +import { join } from "path" + +const tmpRoot = join(import.meta.dir, ".tmp-extra-codex") +const defaultDbtPath = join(tmpRoot, "bin", "dbt") +const originalAltimateDbtPath = process.env.ALTIMATE_DBT_PATH + +const mockExecFile = mock((cmd: string, args: string[], opts: any, cb: Function) => { + cb(null, "", "") +}) + +mock.module("child_process", () => ({ + ...realChildProcess, + execFile: mockExecFile, +})) + +const { configure, execDbtShow, execDbtCompileInline } = await import("../src/dbt-cli") + +function makeFakeExecutable(path: string) { + mkdirSync(join(path, ".."), { recursive: true }) + writeFileSync(path, "#!/bin/sh\nexit 0\n") + chmodSync(path, 0o755) +} + +function commandFailed(message: string, fields: Record = {}) { + const err: any = new Error(message) + Object.assign(err, fields) + return err +} + +describe("dbt-cli extra regression coverage for PR #933", () => { + beforeEach(() => { + rmSync(tmpRoot, { recursive: true, force: true }) + makeFakeExecutable(defaultDbtPath) + process.env.ALTIMATE_DBT_PATH = defaultDbtPath + configure({}) + mockExecFile.mockReset() + }) + + afterEach(() => { + rmSync(tmpRoot, { recursive: true, force: true }) + if (originalAltimateDbtPath === undefined) delete process.env.ALTIMATE_DBT_PATH + else process.env.ALTIMATE_DBT_PATH = originalAltimateDbtPath + configure({}) + }) + + test("execDbtShow returns the complete query result shape from mixed JSON and garbage stdout", async () => { + const rawSql = "select 1 as n" + const compiledSql = "SELECT 1 AS n" + const stdout = [ + "", + "not json at all", + JSON.stringify({ info: { level: "info", msg: "Running with dbt=1.9.0" } }), + "{truncated", + JSON.stringify({ data: { compiled_sql: compiledSql } }), + JSON.stringify({ data: { preview: JSON.stringify([{ n: 1, label: "one" }]) } }), + ].join("\n") + + mockExecFile.mockImplementation((_cmd: string, _args: string[], _opts: any, cb: Function) => { + cb(null, stdout, "") + }) + + const result = await execDbtShow(rawSql) + expect(result).toEqual({ + columnNames: ["n", "label"], + columnTypes: ["string", "string"], + data: [{ n: 1, label: "one" }], + rawSql, + compiledSql, + }) + }) + + test("execDbtShow treats an empty preview array as a successful zero-row result without fallback", async () => { + let calls = 0 + mockExecFile.mockImplementation((_cmd: string, _args: string[], _opts: any, cb: Function) => { + calls++ + cb(null, JSON.stringify({ result: { preview: [], sql: "SELECT * FROM empty_table" } }), "") + }) + + const result = await execDbtShow("select * from empty_table") + expect(result.columnNames).toEqual([]) + expect(result.columnTypes).toEqual([]) + expect(result.data).toEqual([]) + expect(result.compiledSql).toBe("SELECT * FROM empty_table") + expect(calls).toBe(1) + }) + + test("execDbtShow passes --limit 0 through to both JSON mode and plain-text fallback", async () => { + const seenArgs: string[][] = [] + mockExecFile.mockImplementation((_cmd: string, args: string[], _opts: any, cb: Function) => { + seenArgs.push([...args]) + if (seenArgs.length === 1) { + cb(null, JSON.stringify({ info: { msg: "unrecognized successful shape" } }), "") + } else { + cb(null, ["| n |", "| - |", "| 1 |"].join("\n"), "") + } + }) + + const result = await execDbtShow("select 1 as n", 0) + expect(result.data).toEqual([{ n: "1" }]) + expect(seenArgs).toHaveLength(2) + expect(seenArgs[0]).toContain("--limit") + expect(seenArgs[0]?.[seenArgs[0].indexOf("--limit") + 1]).toBe("0") + expect(seenArgs[1]).toEqual(["show", "--inline", "select 1 as n", "--limit", "0"]) + }) + + test("execDbtShow omits --limit entirely when the limit is undefined", async () => { + mockExecFile.mockImplementation((_cmd: string, args: string[], _opts: any, cb: Function) => { + expect(args).not.toContain("--limit") + cb(null, JSON.stringify({ data: { preview: '[{"n": 1}]' } }), "") + }) + + await expect(execDbtShow("select 1")).resolves.toMatchObject({ data: [{ n: 1 }] }) + }) + + test("execDbtShow uses the last error-level log event across top-level and nested dbt shapes", async () => { + const sensitiveSql = "select 'do_not_log_this_show_secret' as token" + const stdout = [ + JSON.stringify({ level: "error", msg: "Encountered an error:" }), + JSON.stringify({ info: { level: "error", msg: "Compilation Error: undefined macro final_macro" } }), + ].join("\n") + + mockExecFile.mockImplementation((_cmd: string, _args: string[], _opts: any, cb: Function) => { + cb(commandFailed(`Command failed: dbt show --inline ${sensitiveSql}`, { code: 1 }), stdout, "generic stderr") + }) + + const caught = (await execDbtShow(sensitiveSql).catch((e) => e)) as Error + expect(caught.message).toBe("Compilation Error: undefined macro final_macro") + expect(caught.message).not.toContain("Encountered an error") + expect(caught.message).not.toContain("do_not_log_this_show_secret") + }) + + test("execDbtShow strips nested ANSI SGR sequences from structured error text", async () => { + const stdout = JSON.stringify({ + info: { + level: "error", + msg: "\u001b[1mCompilation \u001b[31mError\u001b[0m\u001b[22m: bad ref", + }, + }) + + mockExecFile.mockImplementation((_cmd: string, _args: string[], _opts: any, cb: Function) => { + cb(commandFailed("Command failed", { code: 1 }), stdout, "") + }) + + const caught = (await execDbtShow("select 1").catch((e) => e)) as Error + expect(caught.message).toBe("Compilation Error: bad ref") + expect(caught.message).not.toContain("\u001b[") + }) + + test("execDbtShow redacts SQL when JSON parsing fails and only the plain-text fallback rejects", async () => { + const sensitiveSql = "select 'plain_fallback_secret' as token" + let calls = 0 + mockExecFile.mockImplementation((_cmd: string, _args: string[], _opts: any, cb: Function) => { + calls++ + if (calls === 1) { + cb(null, JSON.stringify({ info: { msg: "valid but unparseable success payload" } }), "") + } else { + cb( + commandFailed(`Command failed: dbt show --inline ${sensitiveSql}`, { code: 3 }), + "", + "", + ) + } + }) + + const caught = (await execDbtShow(sensitiveSql).catch((e) => e)) as Error + expect(caught.message).toBe( + "Could not parse dbt show JSON output, and plain-text fallback failed: dbt exited with status 3", + ) + expect(caught.message).not.toContain("plain_fallback_secret") + expect(caught.message).not.toContain("--inline") + }) + + test("execDbtCompileInline parses compiled SQL despite mixed garbage JSON-line output", async () => { + const stdout = [ + "dbt wrote a non-json warning", + JSON.stringify({ info: { msg: "still running" } }), + "{bad", + JSON.stringify({ result: { compiled_code: "SELECT id FROM analytics.customers" } }), + ].join("\n") + + mockExecFile.mockImplementation((_cmd: string, _args: string[], _opts: any, cb: Function) => { + cb(null, stdout, "") + }) + + await expect(execDbtCompileInline("select * from {{ ref('customers') }}")).resolves.toEqual({ + sql: "SELECT id FROM analytics.customers", + }) + }) + + test("execDbtCompileInline falls back to trimmed plain text when JSON mode succeeds with no SQL", async () => { + const seenArgs: string[][] = [] + mockExecFile.mockImplementation((_cmd: string, args: string[], _opts: any, cb: Function) => { + seenArgs.push([...args]) + if (seenArgs.length === 1) cb(null, JSON.stringify({ info: { msg: "done" } }), "") + else cb(null, "\n SELECT 2 AS n \n", "") + }) + + const result = await execDbtCompileInline("select 2 as n") + expect(result).toEqual({ sql: "SELECT 2 AS n" }) + expect(seenArgs[0]).toEqual(["compile", "--inline", "select 2 as n", "--output", "json", "--log-format", "json"]) + expect(seenArgs[1]).toEqual(["compile", "--inline", "select 2 as n"]) + }) + + test("execDbtCompileInline does not use SQL-looking stdout from a failed JSON-mode run", async () => { + let calls = 0 + mockExecFile.mockImplementation((_cmd: string, _args: string[], _opts: any, cb: Function) => { + calls++ + if (calls === 1) { + const trap = JSON.stringify({ payload: { compiled: "SELECT should_not_be_returned FROM failed_stdout" } }) + cb(commandFailed("Command failed", { code: 1 }), trap, "Compilation Error: missing source") + } else { + cb(commandFailed("Command failed", { code: 1 }), "", "Compilation Error: missing source") + } + }) + + await expect(execDbtCompileInline("select * from missing_source")).rejects.toThrow( + "Compilation Error: missing source", + ) + expect(calls).toBe(2) + }) + + test("execDbtCompileInline chooses the last structured error and redacts command-line SQL fallback text", async () => { + const sensitiveSql = "select 'compile_inline_last_error_secret' as token" + const stdout = [ + JSON.stringify({ info: { level: "error", msg: "Encountered an error:" } }), + JSON.stringify({ level: "error", msg: "Runtime Error: final actionable inline failure" }), + ].join("\n") + + mockExecFile.mockImplementation((_cmd: string, _args: string[], _opts: any, cb: Function) => { + cb(commandFailed(`Command failed: dbt compile --inline ${sensitiveSql}`, { code: 2 }), stdout, "") + }) + + const caught = (await execDbtCompileInline(sensitiveSql).catch((e) => e)) as Error + expect(caught.message).toBe("Runtime Error: final actionable inline failure") + expect(caught.message).not.toContain("Encountered an error") + expect(caught.message).not.toContain("compile_inline_last_error_secret") + }) + + test("execDbtCompileInline surfaces spawn-time failures without adding command-line SQL", async () => { + mockExecFile.mockImplementation((_cmd: string, _args: string[], _opts: any, cb: Function) => { + cb(commandFailed("spawn /missing/dbt ENOENT", { code: "ENOENT" }), "", "") + }) + + const caught = (await execDbtCompileInline("select 'spawn_secret'").catch((e) => e)) as Error + expect(caught.message).toBe("dbt compile inline failed: spawn /missing/dbt ENOENT") + expect(caught.message).not.toContain("spawn_secret") + expect(caught.message).not.toContain("--inline") + }) + + test("configure resets dbt resolution and run options honor pythonPath/projectRoot", async () => { + delete process.env.ALTIMATE_DBT_PATH + const projectRoot = join(tmpRoot, "project") + const binDir = join(projectRoot, ".venv", "bin") + const pythonPath = join(binDir, "python") + const dbtPath = join(binDir, "dbt") + makeFakeExecutable(pythonPath) + makeFakeExecutable(dbtPath) + configure({ pythonPath, projectRoot }) + + mockExecFile.mockImplementation((cmd: string, _args: string[], opts: any, cb: Function) => { + expect(cmd).toBe(dbtPath) + expect(opts.cwd).toBe(projectRoot) + expect(String(opts.env.PATH).startsWith(`${binDir}:`)).toBe(true) + cb(null, JSON.stringify({ data: { preview: '[{"ok": true}]' } }), "") + }) + + await expect(execDbtShow("select true as ok")).resolves.toMatchObject({ data: [{ ok: true }] }) + }) +}) + +describe("install.ps1 static safety checks", () => { + test("Windows installer stays Bun-standalone and does not invoke npm/node install flows", () => { + const text = readFileSync(join(import.meta.dir, "../../../install.ps1"), "utf-8") + const executableLines = text + .split(/\r?\n/) + .map((line) => line.trim()) + .filter((line) => line.length > 0 && !line.startsWith("#") && !line.startsWith("Write-")) + .join("\n") + + expect(text).toContain("Bun-compiled standalone executable") + expect(text).toContain("does NOT depend on npm/Node") + expect(text).toContain("github.com/AltimateAI/altimate-code/releases") + expect(executableLines).not.toMatch(/\bnpm\s+(install|i)\b/i) + expect(executableLines).not.toMatch(/\bnode\s+.*install\b/i) + }) +}) diff --git a/packages/dbt-tools/test/dbt-cli-release-validation.test.ts b/packages/dbt-tools/test/dbt-cli-release-validation.test.ts new file mode 100644 index 000000000..657d22fca --- /dev/null +++ b/packages/dbt-tools/test/dbt-cli-release-validation.test.ts @@ -0,0 +1,106 @@ +import { describe, test, expect, mock, beforeEach, afterEach } from "bun:test" +import * as realChildProcess from "child_process" +import { mkdirSync, rmSync, writeFileSync } from "fs" +import { join } from "path" + +const mockExecFile = mock((cmd: string, args: string[], opts: any, cb: Function) => { + cb(null, "", "") +}) + +mock.module("child_process", () => ({ + ...realChildProcess, + execFile: mockExecFile, +})) + +const { configure, execDbtShow, execDbtCompile, execDbtCompileInline } = await import("../src/dbt-cli") + +const tmpRoot = join(import.meta.dir, ".tmp-release-validation") + +describe("dbt-cli release validation: error bubbling", () => { + beforeEach(() => { + mockExecFile.mockReset() + rmSync(tmpRoot, { recursive: true, force: true }) + configure({}) + }) + + afterEach(() => { + rmSync(tmpRoot, { recursive: true, force: true }) + configure({}) + }) + + test("execDbtShow ignores malformed JSON log fragments and still bubbles the valid structured error", async () => { + const stdout = [ + "{not complete json", + JSON.stringify({ info: { level: "info", msg: "Running with dbt=1.9.0" } }), + JSON.stringify({ info: { level: "error", msg: "Database Error: syntax error at or near \"from\"" } }), + ].join("\n") + + mockExecFile.mockImplementation((_cmd: string, _args: string[], _opts: any, cb: Function) => { + const err: any = new Error("Command failed") + err.code = 1 + cb(err, stdout, "") + }) + + await expect(execDbtShow("select * from")).rejects.toThrow('Database Error: syntax error at or near "from"') + }) + + test("execDbtShow redacts Command-failed fallback when exit code is a string", async () => { + const sensitiveSql = "select 'release_validation_secret' as token" + + mockExecFile.mockImplementation((_cmd: string, _args: string[], _opts: any, cb: Function) => { + const err: any = new Error(`Command failed: dbt show --inline ${sensitiveSql} --output json`) + err.code = "EX_CONFIG" + cb(err, "", "") + }) + + const caught = (await execDbtShow(sensitiveSql).catch((e) => e)) as Error + expect(caught.message).toBe("dbt show failed: dbt failed: EX_CONFIG") + expect(caught.message).not.toContain("release_validation_secret") + expect(caught.message).not.toContain("--inline") + }) + + test("execDbtShow has a safe fallback for non-zero exits with no stderr, code, or signal", async () => { + mockExecFile.mockImplementation((_cmd: string, _args: string[], _opts: any, cb: Function) => { + cb(new Error("Command failed: dbt show --inline select 1"), "", "") + }) + + await expect(execDbtShow("select 1")).rejects.toThrow("dbt show failed: dbt failed (no exit code reported)") + }) + + test("execDbtCompileInline strips ANSI SGR codes from stderr, not just structured stdout", async () => { + mockExecFile.mockImplementation((_cmd: string, _args: string[], _opts: any, cb: Function) => { + const err: any = new Error("Command failed") + err.code = 1 + cb(err, "", "\u001b[31mCompilation Error\u001b[0m: undefined macro") + }) + + const caught = (await execDbtCompileInline("select {{ missing_macro() }}").catch((e) => e)) as Error + expect(caught.message).toBe("Compilation Error: undefined macro") + expect(caught.message).not.toContain("\u001b[") + }) + + test.todo("BUG: execDbtCompile should not return stale manifest SQL after the current dbt compile fails", async () => { + const projectRoot = tmpRoot + mkdirSync(join(projectRoot, "target"), { recursive: true }) + writeFileSync( + join(projectRoot, "target", "manifest.json"), + JSON.stringify({ + nodes: { + "model.project.orders": { + name: "orders", + compiled_code: "select * from stale_previous_compile", + }, + }, + }), + ) + configure({ projectRoot }) + + mockExecFile.mockImplementation((_cmd: string, _args: string[], _opts: any, cb: Function) => { + const err: any = new Error("Command failed") + err.code = 1 + cb(err, "", "Compilation Error: model orders depends on a missing source") + }) + + await expect(execDbtCompile("orders")).rejects.toThrow("Compilation Error: model orders depends on a missing source") + }) +}) diff --git a/packages/opencode/test/release-validation/chunk-timeout-844.test.ts b/packages/opencode/test/release-validation/chunk-timeout-844.test.ts new file mode 100644 index 000000000..9246b6141 --- /dev/null +++ b/packages/opencode/test/release-validation/chunk-timeout-844.test.ts @@ -0,0 +1,267 @@ +import { describe, test, expect } from "bun:test" +import { readFileSync } from "fs" +import path from "path" + +import { tmpdir } from "../fixture/fixture" +import { Instance } from "../../src/project/instance" +import { Provider } from "../../src/provider/provider" + +// PR #844 — raise the provider SSE chunk-watchdog default from 2min (120_000) to 5min (300_000). +// +// The watchdog lives in `getSDK` (provider.ts): a per-request fetch wrapper builds a +// `chunkAbortCtl` AbortController, combines it with any per-request and per-provider +// timeout signals via `AbortSignal.any`, then routes the fetched Response through the +// module-private `wrapSSE(res, ms, ctl)`. `wrapSSE` arms a `setTimeout(ms)` that aborts +// the controller + cancels the reader and rejects with "SSE read timed out" if no chunk +// arrives within `ms`. +// +// `DEFAULT_CHUNK_TIMEOUT`, `wrapSSE`, and `getSDK` are all module-private. We do NOT edit +// source to export them. Instead we reach the EXACT source-built fetch wrapper through a +// supported public path: a provider configured with `npm: "file://"` makes +// `getSDK` import our local module and call its `create*` factory with the assembled +// `options`, including the `options.fetch` wrapper it just built. The factory captures that +// wrapper; we then drive it directly with a stubbed global `fetch` and crafted Responses. +// This exercises the real source code, not a re-implementation. + +const PROVIDER_SOURCE = readFileSync(path.join(import.meta.dir, "../../src/provider/provider.ts"), "utf8") + +const FETCH_KEY = "__chunkTimeout844_capturedFetch__" + +type FetchWrapper = (input: any, init?: any) => Promise + +/** + * Writes a tmp opencode.json with a custom provider whose npm points at a local module via + * `file://`. The module's `createFake` factory stashes the source-built `options.fetch` + * wrapper on globalThis under FETCH_KEY. Returns nothing; the caller resolves the wrapper + * after `Provider.getLanguage`. + */ +async function writeCapturingProvider(dir: string, options: Record) { + const modPath = path.join(dir, "fake-provider.mjs") + await Bun.write( + modPath, + `export function createFake(opts) {\n` + + ` globalThis[${JSON.stringify(FETCH_KEY)}] = opts.fetch;\n` + + ` return { languageModel: () => ({ id: opts.name }) };\n` + + `}\n`, + ) + await Bun.write( + path.join(dir, "opencode.json"), + JSON.stringify({ + $schema: "https://altimate.ai/config.json", + provider: { + fakeprov: { + name: "Fake Provider", + npm: "file://" + modPath, + env: [], + models: { + m1: { name: "M1", tool_call: true, limit: { context: 8000, output: 2000 } }, + }, + options, + }, + }, + }), + ) +} + +/** Resolves the SDK (running the real `getSDK`) and returns the captured fetch wrapper. */ +async function captureWrapper(): Promise { + delete (globalThis as any)[FETCH_KEY] + const parsed = Provider.parseModel("fakeprov/m1") + const model = await Provider.getModel(parsed.providerID, parsed.modelID) + await Provider.getLanguage(model) + const wrapper = (globalThis as any)[FETCH_KEY] + expect(typeof wrapper).toBe("function") + return wrapper as FetchWrapper +} + +function sseResponse(chunks: string[], close = false): Response { + let i = 0 + const body = new ReadableStream({ + pull(ctrl) { + if (i < chunks.length) { + ctrl.enqueue(new TextEncoder().encode(chunks[i++])) + return + } + if (close) ctrl.close() + // else: stall forever — never enqueue, never close (simulates a stuck stream) + }, + }) + return new Response(body, { headers: { "content-type": "text/event-stream" } }) +} + +describe("PR #844 — DEFAULT_CHUNK_TIMEOUT 2min→5min", () => { + // Gap 1: default falls back to 300_000 (NOT the old 120_000) when no chunkTimeout is configured. + test("default_chunk_timeout_value_is_5min: wrapSSE arms setTimeout(300000) when chunkTimeout omitted", async () => { + await using tmp = await tmpdir({ init: (dir) => writeCapturingProvider(dir, { apiKey: "x" }) }) + await Instance.provide({ + directory: tmp.path, + fn: async () => { + const wrapper = await captureWrapper() + const origFetch = globalThis.fetch + const origSetTimeout = globalThis.setTimeout + const delays: number[] = [] + // Stub global fetch (no custom fetch configured → wrapper calls global fetch) to return SSE. + globalThis.fetch = (async () => sseResponse(["data: hi\n\n"])) as any + // Stub setTimeout to record the delay wrapSSE arms, then delegate so timers still work. + globalThis.setTimeout = ((fn: any, ms?: any, ...rest: any[]) => { + if (typeof ms === "number") delays.push(ms) + return origSetTimeout(fn, ms, ...rest) + }) as any + try { + const res = await wrapper("http://x/v1/chat", { method: "POST" }) + const reader = res.body!.getReader() + await reader.read() // first pull arms the wrapSSE watchdog timer + await reader.cancel() // stop the stream so the test exits cleanly + // The watchdog delay must be the new 5-minute default, not the old 2-minute value. + expect(delays).toContain(300_000) + expect(delays).not.toContain(120_000) + } finally { + globalThis.fetch = origFetch + globalThis.setTimeout = origSetTimeout + } + }, + }) + }) + + // Gap 2: wrapSSE aborts after a chunk gap — one chunk then stall → second read rejects + ctl aborts. + test("wrapSSE_aborts_after_chunk_gap: rejects with 'SSE read timed out' and aborts the request signal", async () => { + await using tmp = await tmpdir({ init: (dir) => writeCapturingProvider(dir, { apiKey: "x", chunkTimeout: 50 }) }) + await Instance.provide({ + directory: tmp.path, + fn: async () => { + const wrapper = await captureWrapper() + const origFetch = globalThis.fetch + let seenSignal: AbortSignal | undefined + globalThis.fetch = (async (_input: any, init?: any) => { + seenSignal = init?.signal // combined signal includes chunkAbortCtl.signal + return sseResponse(["data: first\n\n"]) // one chunk, then stalls forever + }) as any + try { + const res = await wrapper("http://x/v1/chat", { method: "POST" }) + const reader = res.body!.getReader() + const first = await reader.read() + expect(new TextDecoder().decode(first.value)).toContain("first") + // The combined request signal is not yet aborted before the gap fires. + expect(seenSignal?.aborted).toBe(false) + let err: any + try { + await reader.read() // no further chunk within 50ms → watchdog fires + } catch (e) { + err = e + } + expect(err).toBeInstanceOf(Error) + expect(err.message).toBe("SSE read timed out") + // ctl.abort(err) propagated into the combined request signal. + expect(seenSignal?.aborted).toBe(true) + } finally { + globalThis.fetch = origFetch + } + }, + }) + }) + + // Gap 3: wrapSSE passthrough — non-SSE content-type and null body are returned unwrapped (by reference). + describe("wrapSSE_passthrough_for_non_sse", () => { + test("non-SSE content-type (application/json) is returned unchanged", async () => { + await using tmp = await tmpdir({ init: (dir) => writeCapturingProvider(dir, { apiKey: "x" }) }) + await Instance.provide({ + directory: tmp.path, + fn: async () => { + const wrapper = await captureWrapper() + const origFetch = globalThis.fetch + const jsonRes = new Response(JSON.stringify({ ok: true }), { + headers: { "content-type": "application/json" }, + }) + globalThis.fetch = (async () => jsonRes) as any + try { + const out = await wrapper("http://x/v1/chat", { method: "POST" }) + expect(out).toBe(jsonRes) // same reference — never wrapped + } finally { + globalThis.fetch = origFetch + } + }, + }) + }) + + test("SSE response with null body is returned unchanged", async () => { + await using tmp = await tmpdir({ init: (dir) => writeCapturingProvider(dir, { apiKey: "x" }) }) + await Instance.provide({ + directory: tmp.path, + fn: async () => { + const wrapper = await captureWrapper() + const origFetch = globalThis.fetch + const nullBodyRes = new Response(null, { headers: { "content-type": "text/event-stream" } }) + globalThis.fetch = (async () => nullBodyRes) as any + try { + const out = await wrapper("http://x/v1/chat", { method: "POST" }) + expect(out).toBe(nullBodyRes) // same reference — null body short-circuits wrapSSE + } finally { + globalThis.fetch = origFetch + } + }, + }) + }) + + // `ms <= 0` passthrough (provider.ts:71) is the disable guard inside wrapSSE. The config + // schema (.positive()) and the `chunkTimeout > 0` guard in getSDK prevent ms<=0 from ever + // reaching wrapSSE through the public wrapper, so this case is asserted at the source level. + test("source: wrapSSE returns res unchanged when ms <= 0 (disable guard present)", () => { + const normalized = PROVIDER_SOURCE.replace(/\s+/g, " ") + expect(normalized).toContain("function wrapSSE(res: Response, ms: number, ctl: AbortController)") + expect(normalized).toContain('if (typeof ms !== "number" || ms <= 0) return res') + expect(normalized).toContain("if (!res.body) return res") + expect(normalized).toContain('if (!res.headers.get("content-type")?.includes("text/event-stream")) return res') + }) + }) + + // Gap 4: the watchdog can be fully disabled — chunkAbortCtl is undefined for non-positive + // chunkTimeout, and the fetch wrapper returns the raw res without wrapSSE in that case. + // The config schema blocks chunkTimeout<=0, so the disable path is asserted at the source level. + test("chunk_abort_controller_disabled_when_timeout_non_positive: source guards present", () => { + const normalized = PROVIDER_SOURCE.replace(/\s+/g, " ") + // chunkAbortCtl only constructed for a positive numeric chunkTimeout. + expect(normalized).toContain( + 'const chunkAbortCtl = typeof chunkTimeout === "number" && chunkTimeout > 0 ? new AbortController() : undefined', + ) + // When no chunkAbortCtl, the raw response is returned without wrapSSE. + expect(normalized).toContain("if (!chunkAbortCtl) return res") + expect(normalized).toContain("return wrapSSE(res, chunkTimeout, chunkAbortCtl)") + // The default itself is the new 5-minute value. + expect(normalized).toContain("const DEFAULT_CHUNK_TIMEOUT = 300_000") + expect(normalized).not.toContain("const DEFAULT_CHUNK_TIMEOUT = 120_000") + }) + + // Gap 5: a short per-request timeout still wins even with the new 5-minute chunk default — + // both are combined via AbortSignal.any, and the request timeout aborts first. + test("chunk_and_request_timeout_combine_via_AbortSignal_any: request timeout (100ms) aborts before 5min chunk window", async () => { + await using tmp = await tmpdir({ init: (dir) => writeCapturingProvider(dir, { apiKey: "x", timeout: 100 }) }) + await Instance.provide({ + directory: tmp.path, + fn: async () => { + const wrapper = await captureWrapper() + const origFetch = globalThis.fetch + let seenSignal: AbortSignal | undefined + // Non-SSE response so wrapSSE passes it through; we only care about the combined signal. + globalThis.fetch = (async (_input: any, init?: any) => { + seenSignal = init?.signal + return new Response(JSON.stringify({ ok: true }), { headers: { "content-type": "application/json" } }) + }) as any + try { + await wrapper("http://x/v1/chat", { method: "POST" }) + expect(seenSignal).toBeInstanceOf(AbortSignal) + // Not yet aborted right after the call. + expect(seenSignal?.aborted).toBe(false) + // Wait past the 100ms request timeout but FAR below the 300_000ms chunk default. + await new Promise((r) => origFetchTimeout(r, 160)) + // The request-level timeout fired — proving the 5min bump did not extend it. + expect(seenSignal?.aborted).toBe(true) + } finally { + globalThis.fetch = origFetch + } + }, + }) + }) +}) + +// Capture the real setTimeout up front so gap-5's wait is unaffected by any per-test stubbing. +const origFetchTimeout = globalThis.setTimeout diff --git a/packages/opencode/test/release-validation/mcp-datamate-893-codex.test.ts b/packages/opencode/test/release-validation/mcp-datamate-893-codex.test.ts new file mode 100644 index 000000000..83b9b4616 --- /dev/null +++ b/packages/opencode/test/release-validation/mcp-datamate-893-codex.test.ts @@ -0,0 +1,354 @@ +import { describe, test, expect } from "bun:test" +import { mkdir, readFile, writeFile } from "fs/promises" +import path from "path" +import { tmpdir } from "../fixture/fixture" +import { discoverExternalMcp } from "../../src/mcp/discover" +import { addMcpToConfig, readMcpEntryFromDisk } from "../../src/mcp/config" +import { + readDatamateTransportFromIde, + syncDatamateUrlFromVscodeMcp, +} from "../../src/altimate/datamate-transport" + +const REPO_ROOT = path.join(import.meta.dir, "../../../..") + +async function writeJson(file: string, value: unknown) { + await mkdir(path.dirname(file), { recursive: true }) + await writeFile(file, JSON.stringify(value, null, 2)) +} + +async function withIsolatedHome(fn: (home: string) => Promise): Promise { + await using home = await tmpdir() + const oldHome = process.env.HOME + const oldUserProfile = process.env.USERPROFILE + process.env.HOME = home.path + process.env.USERPROFILE = home.path + try { + return await fn(home.path) + } finally { + if (oldHome === undefined) delete process.env.HOME + else process.env.HOME = oldHome + if (oldUserProfile === undefined) delete process.env.USERPROFILE + else process.env.USERPROFILE = oldUserProfile + } +} + +describe("PR #893 MCP discovery normalization", () => { + test("recursive mcp.json discovery merges both VS Code servers and legacy mcpServers maps", async () => { + await withIsolatedHome(async () => { + await using project = await tmpdir() + await writeJson(path.join(project.path, "tools/editor/mcp.json"), { + servers: { + vscodeRemote: { + url: "https://mcp.example.com/sse", + headers: { Authorization: "Bearer ${MCP_TOKEN}" }, + }, + }, + mcpServers: { + cursorLocal: { + command: "bunx", + args: ["server", 7, false], + env: { TOKEN: "${MCP_TOKEN}" }, + }, + }, + }) + + process.env.MCP_TOKEN = "resolved-token" + try { + const { servers, sources } = await discoverExternalMcp(project.path) + expect(sources).toContain("tools/editor/mcp.json") + expect(servers.vscodeRemote).toEqual({ + type: "remote", + url: "https://mcp.example.com/sse", + headers: { Authorization: "Bearer resolved-token" }, + enabled: false, + } as any) + expect(servers.cursorLocal).toEqual({ + type: "local", + command: ["bunx", "server", "7", "false"], + environment: { TOKEN: "resolved-token" }, + enabled: false, + } as any) + } finally { + delete process.env.MCP_TOKEN + } + }) + }) + + test("recursive project scan ignores vendored mcp.json files before they can auto-connect", async () => { + await withIsolatedHome(async () => { + await using project = await tmpdir() + await writeJson(path.join(project.path, "node_modules/malicious/mcp.json"), { + servers: { vendored: { command: "do-not-run", args: ["--token", "secret"] } }, + }) + await writeJson(path.join(project.path, "dist/generated/mcp.json"), { + mcpServers: { built: { command: "do-not-run-either" } }, + }) + await writeJson(path.join(project.path, ".vscode/mcp.json"), { + servers: { authored: { command: "safe-dev-server" } }, + }) + + const { servers, sources } = await discoverExternalMcp(project.path) + expect(Object.keys(servers).sort()).toEqual(["authored"]) + expect(sources).toEqual([".vscode/mcp.json"]) + }) + }) + + // BUG: normalizeMcpConfig rebuilds any MCP entry with command/url and drops updatedAt, + // even though McpLocal/McpRemote schema accepts updatedAt for sync/reconnect detection. + test.todo("Config normalization preserves updatedAt on local and remote MCP entries", () => {}) +}) + +describe("PR #893 datamate IDE transport selection", () => { + test("prefers remote datamate URL over command when both are present", async () => { + await using project = await tmpdir() + await writeJson(path.join(project.path, ".vscode/mcp.json"), { + servers: { + datamate: { + url: "https://datamate.example.com/sse", + command: "datamate", + args: ["start-stdio"], + }, + }, + }) + + await expect(readDatamateTransportFromIde(project.path)).resolves.toEqual({ + type: "remote", + url: "https://datamate.example.com/sse", + }) + }) + + test("returns local datamate command with exact IDE args", async () => { + await using project = await tmpdir() + await writeJson(path.join(project.path, ".cursor/mcp.json"), { + mcpServers: { + datamate: { + command: "bunx", + args: ["@altimate/datamate", "start-stdio", "--workspace", project.path], + }, + }, + }) + + await expect(readDatamateTransportFromIde(project.path)).resolves.toEqual({ + type: "local", + command: ["bunx", "@altimate/datamate", "start-stdio", "--workspace", project.path], + }) + }) + + test("falls back to safe local datamate marker when IDE entry has no usable transport fields", async () => { + await using project = await tmpdir() + await writeJson(path.join(project.path, ".vscode/mcp.json"), { + servers: { datamate: { type: "stdio", args: ["ignored-without-command"] } }, + }) + + await expect(readDatamateTransportFromIde(project.path)).resolves.toEqual({ + type: "local", + command: ["datamate", "start-stdio"], + }) + }) + + test("skips malformed mcp.json and uses the next valid datamate entry", async () => { + await using project = await tmpdir() + await mkdir(path.join(project.path, "a-bad"), { recursive: true }) + await writeFile(path.join(project.path, "a-bad/mcp.json"), "{ not json") + await writeJson(path.join(project.path, "z-good/mcp.json"), { + servers: { datamate: { url: "https://good.example.com/mcp" } }, + }) + + await expect(readDatamateTransportFromIde(project.path)).resolves.toEqual({ + type: "remote", + url: "https://good.example.com/mcp", + }) + }) + + test("ignores vendored datamate mcp.json entries during transport selection", async () => { + await using project = await tmpdir() + await writeJson(path.join(project.path, "node_modules/pkg/mcp.json"), { + servers: { datamate: { url: "https://vendored.example.com/sse" } }, + }) + await writeJson(path.join(project.path, ".vscode/mcp.json"), { + servers: { datamate: { command: "datamate", args: ["start-stdio"] } }, + }) + + await expect(readDatamateTransportFromIde(project.path)).resolves.toEqual({ + type: "local", + command: ["datamate", "start-stdio"], + }) + }) +}) + +describe("PR #893 datamate sync to altimate-code config", () => { + test("syncs datamate stdio by updatedAt while preserving enabled/timeout and stripping session RPC env", async () => { + await using project = await tmpdir() + const configPath = path.join(project.path, ".altimate-code/altimate-code.json") + await addMcpToConfig( + "datamate", + { + type: "remote", + url: "https://old.example.com/sse", + enabled: false, + timeout: 12345, + updatedAt: "2026-06-17T09:00:00.000Z", + } as any, + configPath, + ) + await writeJson(path.join(project.path, ".vscode/mcp.json"), { + servers: { + datamate: { + type: "stdio", + command: "datamate", + args: ["start-stdio", "--port", "0"], + env: { + ALTIMATE_EXTENSION_RPC: "/tmp/extension-rpc-secret.sock", + KEEP_ME: "yes", + }, + updatedAt: "2026-06-17T10:00:00.000Z", + }, + }, + }) + + const updated = await syncDatamateUrlFromVscodeMcp(project.path) + const entry = await readMcpEntryFromDisk("datamate", configPath) + const raw = await readFile(configPath, "utf-8") + + expect(updated).toEqual(["datamate"]) + expect(entry).toEqual({ + type: "local", + command: ["datamate", "start-stdio", "--port", "0"], + environment: { KEEP_ME: "yes" }, + enabled: false, + timeout: 12345, + updatedAt: "2026-06-17T10:00:00.000Z", + } as any) + expect(raw).not.toContain("extension-rpc-secret") + expect(raw).not.toContain("ALTIMATE_EXTENSION_RPC") + }) + + test("does not rewrite datamate when updatedAt already matches IDE config", async () => { + await using project = await tmpdir() + const configPath = path.join(project.path, ".altimate-code/altimate-code.json") + await addMcpToConfig( + "datamate", + { + type: "remote", + url: "https://same.example.com/sse", + enabled: true, + updatedAt: "2026-06-17T10:00:00.000Z", + } as any, + configPath, + ) + const before = await readFile(configPath, "utf-8") + await writeJson(path.join(project.path, ".vscode/mcp.json"), { + servers: { + datamate: { + url: "https://new-but-same-timestamp.example.com/sse", + updatedAt: "2026-06-17T10:00:00.000Z", + }, + }, + }) + + await expect(syncDatamateUrlFromVscodeMcp(project.path)).resolves.toEqual([]) + await expect(readFile(configPath, "utf-8")).resolves.toBe(before) + }) + + test("updates non-datamate remote URLs from the same IDE file and preserves nested auth fields", async () => { + await using project = await tmpdir() + const configPath = path.join(project.path, ".altimate-code/altimate-code.json") + await addMcpToConfig( + "datamate", + { + type: "remote", + url: "https://datamate.example.com/sse", + updatedAt: "2026-06-17T10:00:00.000Z", + } as any, + configPath, + ) + await addMcpToConfig( + "analytics", + { + type: "remote", + url: "https://old.example.com/mcp", + headers: { Authorization: "Bearer keep-secret", "X-Tenant": "tenant-a" }, + oauth: { clientId: "client-1", scope: "read write" }, + timeout: 9000, + enabled: false, + } as any, + configPath, + ) + await writeJson(path.join(project.path, ".vscode/mcp.json"), { + servers: { + datamate: { + url: "https://datamate.example.com/sse", + updatedAt: "2026-06-17T10:00:00.000Z", + }, + analytics: { url: "https://new.example.com/mcp" }, + }, + }) + + const updated = await syncDatamateUrlFromVscodeMcp(project.path) + const entry = (await readMcpEntryFromDisk("analytics", configPath)) as any + + expect(updated).toEqual(["analytics"]) + expect(entry.url).toBe("https://new.example.com/mcp") + expect(entry.headers).toEqual({ Authorization: "Bearer keep-secret", "X-Tenant": "tenant-a" }) + expect(entry.oauth).toEqual({ clientId: "client-1", scope: "read write" }) + expect(entry.timeout).toBe(9000) + expect(entry.enabled).toBe(false) + expect(typeof entry.updatedAt).toBe("string") + }) + + test("missing altimate-code config is a no-op and does not create a config file", async () => { + await using project = await tmpdir() + const configPath = path.join(project.path, ".altimate-code/altimate-code.json") + await writeJson(path.join(project.path, ".vscode/mcp.json"), { + servers: { + datamate: { + url: "https://datamate.example.com/sse", + updatedAt: "2026-06-17T10:00:00.000Z", + }, + }, + }) + + await expect(syncDatamateUrlFromVscodeMcp(project.path)).resolves.toEqual([]) + await expect(readFile(configPath, "utf-8")).rejects.toThrow() + }) +}) + +describe("PR #893 /mcps and installer static safety checks", () => { + test("mcp_discover tool redacts discovered URLs, args, and session-only RPC env before persisting", async () => { + const source = await readFile( + path.join(REPO_ROOT, "packages/opencode/src/altimate/tools/mcp-discover.ts"), + "utf-8", + ) + + expect(source).toContain("function safeDetail") + expect(source).toContain('if (server.type === "remote") return "(remote)"') + expect(source).toContain("server.command[0]") + expect(source).toContain("ALTIMATE_EXTENSION_RPC") + expect(source).toContain("stripSessionEnv(discovered[name])") + expect(source).toContain("enabled: true") + expect(source).not.toContain("server.url") + expect(source).not.toContain("server.command.join") + }) + + test("enabled-state persistence reads all config paths and rewrites only the matching MCP entry", async () => { + const source = await readFile(path.join(REPO_ROOT, "packages/opencode/src/mcp/index.ts"), "utf-8") + + expect(source).toContain("let persistChain: Promise = Promise.resolve()") + expect(source).toContain("findAllConfigPaths(Instance.directory, Global.Path.config)") + expect(source).toContain("readMcpEntryFromDisk(name, p)") + expect(source).toContain("{ ...entry, enabled }") + expect(source).toContain("await persistMcpEnabled(name, true)") + expect(source).toContain("await persistMcpEnabled(name, false)") + }) + + test("install.ps1 remains static-analysis friendly and does not use npm/node execution paths", async () => { + const ps1 = await readFile(path.join(REPO_ROOT, "install.ps1"), "utf-8") + + expect(ps1).toContain("github.com/AltimateAI/altimate-code/releases") + expect(ps1).toContain("Expand-Archive") + expect(ps1).toContain("IsProcessorFeaturePresent(40)") + expect(ps1).toContain("PROCESSOR_ARCHITEW6432") + expect(ps1).not.toContain("npm install -g @altimateai") + expect(ps1).not.toMatch(/\bnode\s+/i) + }) +}) diff --git a/packages/opencode/test/release-validation/mcp-datamate-893.test.ts b/packages/opencode/test/release-validation/mcp-datamate-893.test.ts new file mode 100644 index 000000000..5f3af7aec --- /dev/null +++ b/packages/opencode/test/release-validation/mcp-datamate-893.test.ts @@ -0,0 +1,459 @@ +import { describe, test, expect } from "bun:test" +import { tmpdir } from "../fixture/fixture" +import { mkdir, writeFile, readFile } from "fs/promises" +import path from "path" +import { + addMcpToConfig, + removeMcpFromConfig, + listMcpInConfig, + findAllConfigPaths, + readMcpEntryFromDisk, +} from "../../src/mcp/config" +import { Config } from "../../src/config/config" +import { + readDatamateTransportFromIde, + syncDatamateUrlFromVscodeMcp, + DATAMATE_KEY, +} from "../../src/altimate/datamate-transport" +import { discoverExternalMcp } from "../../src/mcp/discover" + +// Regression tests for PR #893 — IDE-aware datamate transport, enabled-state +// persistence, /mcps command. These assert the INTENDED behavior of the merged +// code and pass against it. Where a test documents a known gap/contract rather +// than ideal behavior, the comment says so explicitly. + +// ── Gap 1: updatedAt round-trips through the strict Config.Mcp schema ──────── +describe("PR893: addMcpToConfig writes updatedAt and round-trips through strict Config.Mcp", () => { + test("remote entry with updatedAt parses cleanly against Config.Mcp", async () => { + await using tmp = await tmpdir() + const configPath = path.join(tmp.path, "altimate-code.json") + await addMcpToConfig( + "foo", + { + type: "remote", + url: "http://x", + enabled: true, + updatedAt: "2026-06-17T00:00:00Z", + } as any, + configPath, + ) + + const parsed = JSON.parse(await readFile(configPath, "utf-8")) + // Must not throw — this is exactly the schema gap PR #893 closed by adding + // updatedAt to McpRemote (and McpLocal). + const entry = Config.Mcp.parse(parsed.mcp.foo) + expect(entry.type).toBe("remote") + expect((entry as any).updatedAt).toBe("2026-06-17T00:00:00Z") + }) + + test("local entry with updatedAt parses cleanly against Config.Mcp", async () => { + await using tmp = await tmpdir() + const configPath = path.join(tmp.path, "altimate-code.json") + await addMcpToConfig( + "bar", + { + type: "local", + command: ["node", "server.js"], + environment: { K: "v" }, + enabled: false, + updatedAt: "2026-06-17T01:02:03Z", + } as any, + configPath, + ) + + const parsed = JSON.parse(await readFile(configPath, "utf-8")) + const entry = Config.Mcp.parse(parsed.mcp.bar) + expect(entry.type).toBe("local") + expect((entry as any).updatedAt).toBe("2026-06-17T01:02:03Z") + }) + + test("negative: an unknown extra key still throws — .strict() was not loosened", async () => { + // Proves updatedAt was added as a real field, not by relaxing .strict(). + const bad = { + type: "remote", + url: "http://x", + updatedAt: "2026-06-17T00:00:00Z", + bogusUnknownKey: "should-be-rejected", + } + expect(() => Config.Mcp.parse(bad)).toThrow() + }) +}) + +// ── Gap 2: malformed JSONC does not silently lose recoverable config ───────── +describe("PR893: addMcpToConfig on a malformed JSONC file — clobbering contract", () => { + test("broken (partial-tree) JSON: existing key text is preserved and new entry added", async () => { + await using tmp = await tmpdir() + const configPath = path.join(tmp.path, "altimate-code.json") + // Truncated/broken JSON. jsonc-parser still produces a partial tree for this + // (parseTree returns a truthy node), so addMcpToConfig does NOT bail. + const brokenText = `{ "mcp": { "a": ` + await writeFile(configPath, brokenText) + + await addMcpToConfig("b", { type: "remote", url: "http://y" } as any, configPath) + + const after = await readFile(configPath, "utf-8") + // CONTRACT (current, conscious): the original entry "a" is NOT silently + // dropped — its key text survives in the file — and the new entry "b" is + // appended. The file is left unparseable because the source was unparseable; + // addMcpToConfig is a text-edit, not a sanitizer. + expect(after).toContain('"a"') + expect(after).toContain('"b"') + expect(after).toContain("http://y") + }) + + test("asymmetry: list/remove bail when parseTree returns undefined (severe garbage)", async () => { + // Pin the intended asymmetry the prompt calls out: when the file is so broken + // that parseTree returns undefined, the read paths refuse to act rather than + // returning a half-parsed view. + await using tmp = await tmpdir() + const configPath = path.join(tmp.path, "altimate-code.json") + await writeFile(configPath, "this is not json at all") + + expect(await listMcpInConfig(configPath)).toEqual([]) + expect(await removeMcpFromConfig("anything", configPath)).toBe(false) + }) +}) + +// ── Gap 3: readMcpEntryFromDisk returns undefined for a non-object node ─────── +describe("PR893: readMcpEntryFromDisk handles non-object nodes", () => { + test("primitive (string) value → undefined, does not throw", async () => { + await using tmp = await tmpdir() + const configPath = path.join(tmp.path, "altimate-code.json") + await writeFile(configPath, `{ "mcp": { "weird": "not-an-object" } }`) + const entry = await readMcpEntryFromDisk("weird", configPath) + expect(entry).toBeUndefined() + }) + + test("primitive (number) value → undefined, does not throw", async () => { + await using tmp = await tmpdir() + const configPath = path.join(tmp.path, "altimate-code.json") + await writeFile(configPath, `{ "mcp": { "n": 42 } }`) + const entry = await readMcpEntryFromDisk("n", configPath) + expect(entry).toBeUndefined() + }) + + test("valid object value → reconstructed entry (getNodeValue still works)", async () => { + await using tmp = await tmpdir() + const configPath = path.join(tmp.path, "altimate-code.json") + await writeFile( + configPath, + `{ "mcp": { "ok": { "type": "local", "command": ["node"] } } }`, + ) + const entry = await readMcpEntryFromDisk("ok", configPath) + expect(entry).toEqual({ type: "local", command: ["node"] } as any) + }) +}) + +// ── Gap 4: readMcpEntryFromDisk preserves array/object transport fields ─────── +describe("PR893: readMcpEntryFromDisk preserves nested array/object fields", () => { + test("command array, environment object, and headers object all survive", async () => { + await using tmp = await tmpdir() + const configPath = path.join(tmp.path, "altimate-code.json") + // Hand-write so we exercise getNodeValue reconstruction directly (not just a + // round-trip of what addMcpToConfig serialized). Use a single remote entry + // carrying headers (object) plus an args-style array to assert no flattening. + const entry = { + type: "remote", + url: "https://example.com/mcp", + headers: { X: "y", Authorization: "Bearer z" }, + } + await writeFile(configPath, JSON.stringify({ mcp: { srv: entry } }, null, 2)) + const read = await readMcpEntryFromDisk("srv", configPath) + expect(read).toEqual(entry as any) + expect((read as any).headers).toEqual({ X: "y", Authorization: "Bearer z" }) + }) + + test("local entry: command[] and environment{} are not dropped", async () => { + await using tmp = await tmpdir() + const configPath = path.join(tmp.path, "altimate-code.json") + const entry = { + type: "local", + command: ["node", "server.js", "--flag"], + environment: { A: "1", B: "2" }, + } + await writeFile(configPath, JSON.stringify({ mcp: { srv: entry } }, null, 2)) + const read = await readMcpEntryFromDisk("srv", configPath) + expect(read).toEqual(entry as any) + // A manual children-walk reading prop.children[1].value would drop these. + expect((read as any).command).toEqual(["node", "server.js", "--flag"]) + expect((read as any).environment).toEqual({ A: "1", B: "2" }) + }) +}) + +// ── Gap 5: removeMcpFromConfig return-value contract ───────────────────────── +describe("PR893: removeMcpFromConfig return-value contract", () => { + test("nonexistent path → false", async () => { + await using tmp = await tmpdir() + const result = await removeMcpFromConfig("x", path.join(tmp.path, "missing.json")) + expect(result).toBe(false) + }) + + test("file without the named entry → false, file unchanged", async () => { + await using tmp = await tmpdir() + const configPath = path.join(tmp.path, "altimate-code.json") + const original = JSON.stringify({ mcp: { keep: { type: "local", command: ["x"] } } }) + await writeFile(configPath, original) + const result = await removeMcpFromConfig("bar", configPath) + expect(result).toBe(false) + expect(await readFile(configPath, "utf-8")).toBe(original) + }) + + test("present entry → true, removed, sibling untouched", async () => { + await using tmp = await tmpdir() + const configPath = path.join(tmp.path, "altimate-code.json") + await writeFile( + configPath, + JSON.stringify({ + mcp: { + bar: { type: "remote", url: "https://bar.com" }, + keep: { type: "local", command: ["k"] }, + }, + }), + ) + const result = await removeMcpFromConfig("bar", configPath) + expect(result).toBe(true) + const after = await listMcpInConfig(configPath) + expect(after).not.toContain("bar") + expect(after).toContain("keep") + }) +}) + +// ── Gap 6: findAllConfigPaths includes project subdirs, not global subdirs ──── +describe("PR893: findAllConfigPaths project subdir coverage", () => { + test("surfaces top-level + .altimate-code subdir for project, not global subdir", async () => { + await using projTmp = await tmpdir() + await using globalTmp = await tmpdir() + + const projTop = path.join(projTmp.path, "altimate-code.json") + const projSub = path.join(projTmp.path, ".altimate-code", "altimate-code.json") + const globalTop = path.join(globalTmp.path, "altimate-code.json") + const globalSub = path.join(globalTmp.path, ".altimate-code", "altimate-code.json") + + await writeFile(projTop, "{}") + await mkdir(path.join(projTmp.path, ".altimate-code"), { recursive: true }) + await writeFile(projSub, "{}") + await writeFile(globalTop, "{}") + await mkdir(path.join(globalTmp.path, ".altimate-code"), { recursive: true }) + await writeFile(globalSub, "{}") + + const result = await findAllConfigPaths(projTmp.path, globalTmp.path) + + expect(result).toContain(projTop) + expect(result).toContain(projSub) // project subdir is probed + expect(result).toContain(globalTop) + expect(result).not.toContain(globalSub) // global subdir is NOT probed + }) + + test("top-level project path precedes its subdir path (stable precedence)", async () => { + await using projTmp = await tmpdir() + await using globalTmp = await tmpdir() + const projTop = path.join(projTmp.path, "altimate-code.json") + const projSub = path.join(projTmp.path, ".altimate-code", "altimate-code.json") + await writeFile(projTop, "{}") + await mkdir(path.join(projTmp.path, ".altimate-code"), { recursive: true }) + await writeFile(projSub, "{}") + + const result = await findAllConfigPaths(projTmp.path, globalTmp.path) + expect(result.indexOf(projTop)).toBeLessThan(result.indexOf(projSub)) + }) +}) + +// ── Gap 7: readDatamateTransportFromIde picks first sorted mcp.json + classify ─ +describe("PR893: readDatamateTransportFromIde sorted-first selection + classification", () => { + test("first sorted datamate-bearing mcp.json wins (a/ before b/)", async () => { + await using tmp = await tmpdir() + // a/mcp.json — stdio datamate; b/mcp.json — http datamate. + await mkdir(path.join(tmp.path, "a"), { recursive: true }) + await mkdir(path.join(tmp.path, "b"), { recursive: true }) + await writeFile( + path.join(tmp.path, "a", "mcp.json"), + JSON.stringify({ servers: { [DATAMATE_KEY]: { type: "stdio", command: "datamate", args: ["x"] } } }), + ) + await writeFile( + path.join(tmp.path, "b", "mcp.json"), + JSON.stringify({ servers: { [DATAMATE_KEY]: { url: "http://from-b" } } }), + ) + + const t = await readDatamateTransportFromIde(tmp.path) + // a/ sorts before b/ → stdio entry from a/ wins → local transport. + expect(t).toEqual({ type: "local", command: ["datamate", "x"] }) + }) + + test("stdio entry → local(command+args); http entry → remote(url)", async () => { + await using tmp = await tmpdir() + await mkdir(path.join(tmp.path, ".cursor"), { recursive: true }) + await writeFile( + path.join(tmp.path, ".cursor", "mcp.json"), + JSON.stringify({ mcpServers: { [DATAMATE_KEY]: { url: "https://remote-only" } } }), + ) + const t = await readDatamateTransportFromIde(tmp.path) + expect(t).toEqual({ type: "remote", url: "https://remote-only" }) + }) + + test("returns null when no mcp.json contains a datamate entry", async () => { + await using tmp = await tmpdir() + await mkdir(path.join(tmp.path, ".vscode"), { recursive: true }) + await writeFile( + path.join(tmp.path, ".vscode", "mcp.json"), + JSON.stringify({ servers: { other: { url: "http://x" } } }), + ) + const t = await readDatamateTransportFromIde(tmp.path) + expect(t).toBeNull() + }) + + test("adversarial: entry with BOTH url and command → url wins (type/url precedence)", async () => { + await using tmp = await tmpdir() + await mkdir(path.join(tmp.path, ".vscode"), { recursive: true }) + await writeFile( + path.join(tmp.path, ".vscode", "mcp.json"), + JSON.stringify({ + servers: { [DATAMATE_KEY]: { type: "stdio", url: "http://wins", command: "datamate", args: ["a"] } }, + }), + ) + const t = await readDatamateTransportFromIde(tmp.path) + // The implementation checks `typeof entry.url === "string"` first, so a URL + // present always classifies as remote regardless of command/type. + expect(t).toEqual({ type: "remote", url: "http://wins" }) + }) +}) + +// ── Gap 8: syncDatamateUrlFromVscodeMcp change-detection via updatedAt ───────── +describe("PR893: syncDatamateUrlFromVscodeMcp updatedAt-based change detection", () => { + async function seedConfig(dir: string, datamate: Record) { + const configPath = path.join(dir, "altimate-code.json") + await writeFile(configPath, JSON.stringify({ mcp: { [DATAMATE_KEY]: datamate } }, null, 2)) + return configPath + } + + async function seedIdeMcp(dir: string, datamate: Record) { + await mkdir(path.join(dir, ".vscode"), { recursive: true }) + await writeFile( + path.join(dir, ".vscode", "mcp.json"), + JSON.stringify({ servers: { [DATAMATE_KEY]: datamate } }, null, 2), + ) + } + + test("Case A: IDE entry lacks updatedAt → no sync (documents staleness gap)", async () => { + await using tmp = await tmpdir() + const configPath = await seedConfig(tmp.path, { + type: "remote", + url: "http://OLD", + enabled: false, + timeout: 5000, + updatedAt: "T1", + }) + await seedIdeMcp(tmp.path, { url: "http://NEW" }) // no updatedAt + + const updated = await syncDatamateUrlFromVscodeMcp(tmp.path) + expect(updated).not.toContain(DATAMATE_KEY) + + const after = JSON.parse(await readFile(configPath, "utf-8")) + expect(after.mcp[DATAMATE_KEY].url).toBe("http://OLD") // unchanged + }) + + test("Case B: IDE updatedAt equals existing → no write", async () => { + await using tmp = await tmpdir() + const configPath = await seedConfig(tmp.path, { + type: "remote", + url: "http://OLD", + enabled: false, + timeout: 5000, + updatedAt: "T1", + }) + await seedIdeMcp(tmp.path, { url: "http://NEW", updatedAt: "T1" }) + + const updated = await syncDatamateUrlFromVscodeMcp(tmp.path) + expect(updated).not.toContain(DATAMATE_KEY) + + const after = JSON.parse(await readFile(configPath, "utf-8")) + expect(after.mcp[DATAMATE_KEY].url).toBe("http://OLD") // unchanged + }) + + test("Case C: IDE updatedAt differs → url synced, non-transport fields preserved", async () => { + await using tmp = await tmpdir() + const configPath = await seedConfig(tmp.path, { + type: "remote", + url: "http://OLD", + enabled: false, + timeout: 5000, + updatedAt: "T1", + }) + await seedIdeMcp(tmp.path, { url: "http://NEW", updatedAt: "T2" }) + + const updated = await syncDatamateUrlFromVscodeMcp(tmp.path) + expect(updated).toContain(DATAMATE_KEY) + + const after = JSON.parse(await readFile(configPath, "utf-8")) + const entry = after.mcp[DATAMATE_KEY] + expect(entry.url).toBe("http://NEW") + expect(entry.updatedAt).toBe("T2") + // Non-transport fields the IDE doesn't manage must be carried forward. + expect(entry.enabled).toBe(false) + expect(entry.timeout).toBe(5000) + // And the synced entry must still satisfy the strict schema. + expect(() => Config.Mcp.parse(entry)).not.toThrow() + }) +}) + +// ── Gap 9: concurrent addMcpToConfig writes ────────────────────────────────── +describe("PR893: concurrent config writes", () => { + test("two concurrent addMcpToConfig calls — document lost-update behavior", async () => { + await using tmp = await tmpdir() + const configPath = path.join(tmp.path, "altimate-code.json") + await writeFile( + configPath, + JSON.stringify({ mcp: { seed: { type: "local", command: ["s"] } } }), + ) + + await Promise.all([ + addMcpToConfig("a", { type: "remote", url: "http://a" } as any, configPath), + addMcpToConfig("b", { type: "remote", url: "http://b" } as any, configPath), + ]) + + const listed = await listMcpInConfig(configPath) + // addMcpToConfig has no file locking: each call read-modify-writes the whole + // file, so two concurrent writers can race and clobber each other. We assert + // the SEED survives and at least one of the two new entries landed; if BOTH + // survive the writes were effectively serialized. A failure here that drops + // 'seed' or both new entries documents the lost-update race (low-severity). + expect(listed).toContain("seed") + const landed = ["a", "b"].filter((n) => listed.includes(n)) + expect(landed.length).toBeGreaterThanOrEqual(1) + }) +}) + +// ── Gap 10: glob-discovered project mcp.json servers forced enabled:false ───── +describe("PR893: project-scoped discovery forces enabled:false", () => { + test("a sub/mcp.json server declaring enabled:true is discovered enabled:false", async () => { + await using tmp = await tmpdir() + await mkdir(path.join(tmp.path, "sub"), { recursive: true }) + await writeFile( + path.join(tmp.path, "sub", "mcp.json"), + JSON.stringify({ + servers: { + evil: { command: "node", args: ["evil.js"], enabled: true }, + }, + }), + ) + + const { servers } = await discoverExternalMcp(tmp.path) + expect(servers.evil).toBeDefined() + // Security gate: project-scoped servers are forced disabled so they never + // auto-connect, even when the file declares enabled:true. + expect((servers.evil as any).enabled).toBe(false) + expect(servers.evil.type).toBe("local") + }) + + test("project-scoped remote server with enabled:true is also forced disabled", async () => { + await using tmp = await tmpdir() + await mkdir(path.join(tmp.path, "nested", "deep"), { recursive: true }) + await writeFile( + path.join(tmp.path, "nested", "deep", "mcp.json"), + JSON.stringify({ mcpServers: { remoteEvil: { url: "http://evil", enabled: true } } }), + ) + + const { servers } = await discoverExternalMcp(tmp.path) + expect(servers.remoteEvil).toBeDefined() + expect((servers.remoteEvil as any).enabled).toBe(false) + }) +}) diff --git a/packages/opencode/test/release-validation/question-937-codex.test.ts b/packages/opencode/test/release-validation/question-937-codex.test.ts new file mode 100644 index 000000000..387698ed7 --- /dev/null +++ b/packages/opencode/test/release-validation/question-937-codex.test.ts @@ -0,0 +1,357 @@ +import { describe, test, expect } from "bun:test" +import { QuestionTool } from "../../src/tool/question" +import * as QuestionModule from "../../src/question" +import { SessionID, MessageID } from "../../src/session/schema" + +const ctx = { + sessionID: SessionID.make("ses_question-937"), + messageID: MessageID.make("msg_question-937"), + callID: "call_question-937", + agent: "test-agent", + abort: AbortSignal.any([]), + messages: [], + metadata: () => {}, + ask: async () => {}, +} + +const option = (label: string, description = `${label} description`) => ({ label, description }) + +const colorQuestion = { + question: "Pick a color", + header: "Color", + options: [option("Red"), option("Blue"), option("Green")], +} + +async function withEnv(env: Record, fn: () => Promise): Promise { + const keys = ["ALTIMATE_FORCE_INTERACTIVE", "ALTIMATE_NON_INTERACTIVE", "ALTIMATE_AUTO_ANSWER"] + const previous = new Map(keys.map((key) => [key, process.env[key]])) + + for (const key of keys) delete process.env[key] + for (const [key, value] of Object.entries(env)) { + if (value === undefined) delete process.env[key] + else process.env[key] = value + } + + try { + return await fn() + } finally { + for (const key of keys) { + const value = previous.get(key) + if (value === undefined) delete process.env[key] + else process.env[key] = value + } + } +} + +async function withQuestionAsk( + implementation: typeof QuestionModule.Question.ask, + fn: () => Promise, +): Promise { + const original = QuestionModule.Question.ask + ;(QuestionModule.Question as any).ask = implementation + try { + return await fn() + } finally { + ;(QuestionModule.Question as any).ask = original + } +} + +describe("release validation PR #937 question tool output contract", () => { + test("interactive path preserves exact answered output, plural title, and metadata answers", async () => { + let calls = 0 + await withEnv({}, async () => { + await withQuestionAsk(async () => { + calls++ + return [["Approve"], ["Read", "Write"]] + }, async () => { + const tool = await QuestionTool.init() + const result = await tool.execute( + { + questions: [ + { + question: "May I proceed?", + header: "Proceed", + options: [option("Approve"), option("Cancel")], + }, + { + question: "Which scopes?", + header: "Scopes", + options: [option("Read"), option("Write")], + multiple: true, + }, + ], + }, + ctx, + ) + + expect(calls).toBe(1) + expect(result.title).toBe("Asked 2 questions") + expect(result.metadata.answers).toEqual([["Approve"], ["Read", "Write"]]) + expect(result.output).toBe( + 'User has answered your questions: "May I proceed?"="Approve", "Which scopes?"="Read, Write". You can now continue with the user\'s answers in mind.', + ) + }) + }) + }) + + test("interactive formatting marks missing or empty answer slots as Unanswered", async () => { + await withEnv({}, async () => { + await withQuestionAsk(async () => [[]], async () => { + const tool = await QuestionTool.init() + const result = await tool.execute( + { + questions: [ + { + question: "First question", + header: "First", + options: [option("A")], + }, + { + question: "Second question", + header: "Second", + options: [option("B")], + }, + ], + }, + ctx, + ) + + expect(result.output).toContain('"First question"="Unanswered"') + expect(result.output).toContain('"Second question"="Unanswered"') + }) + }) + }) + + test("non-interactive default returns empty metadata answers and never calls Question.ask", async () => { + let calls = 0 + await withEnv({ ALTIMATE_NON_INTERACTIVE: "1" }, async () => { + await withQuestionAsk(async () => { + calls++ + return [["SHOULD_NOT_BE_USED"]] + }, async () => { + const tool = await QuestionTool.init() + const result = await tool.execute({ questions: [colorQuestion] }, ctx) + + expect(calls).toBe(0) + expect(result.metadata.answers).toEqual([[]]) + expect(result.output.startsWith("Running in non-interactive mode")).toBe(true) + expect(result.output).toContain("No user was available to answer") + expect(result.output).toContain("ALTIMATE_AUTO_ANSWER=first|last|") + expect(result.output).toContain('Result: "Pick a color"="Unanswered".') + expect(result.output).not.toContain("User has answered your questions") + expect(result.output).not.toContain("continue with the user's answers in mind") + }) + }) + }) + + test("only literal ALTIMATE_NON_INTERACTIVE=1 short-circuits; other values stay interactive", async () => { + for (const value of ["", "0", "true", "yes", " 1 "]) { + let calls = 0 + await withEnv({ ALTIMATE_NON_INTERACTIVE: value }, async () => { + await withQuestionAsk(async () => { + calls++ + return [["Red"]] + }, async () => { + const tool = await QuestionTool.init() + const result = await tool.execute({ questions: [colorQuestion] }, ctx) + + expect(calls).toBe(1) + expect(result.metadata.answers).toEqual([["Red"]]) + expect(result.output.startsWith("User has answered your questions")).toBe(true) + }) + }) + } + }) + + test("only literal ALTIMATE_FORCE_INTERACTIVE=1 overrides non-interactive mode", async () => { + for (const value of ["", "0", "true", "yes", " 1 "]) { + let calls = 0 + await withEnv({ ALTIMATE_FORCE_INTERACTIVE: value, ALTIMATE_NON_INTERACTIVE: "1" }, async () => { + await withQuestionAsk(async () => { + calls++ + return [["Red"]] + }, async () => { + const tool = await QuestionTool.init() + const result = await tool.execute({ questions: [colorQuestion] }, ctx) + + expect(calls).toBe(0) + expect(result.metadata.answers).toEqual([[]]) + expect(result.output).toContain('"Pick a color"="Unanswered"') + }) + }) + } + + let calls = 0 + await withEnv({ ALTIMATE_FORCE_INTERACTIVE: "1", ALTIMATE_NON_INTERACTIVE: "1" }, async () => { + await withQuestionAsk(async () => { + calls++ + return [["Green"]] + }, async () => { + const tool = await QuestionTool.init() + const result = await tool.execute({ questions: [colorQuestion] }, ctx) + + expect(calls).toBe(1) + expect(result.metadata.answers).toEqual([["Green"]]) + expect(result.output.startsWith("User has answered your questions")).toBe(true) + }) + }) + }) + + test("ALTIMATE_AUTO_ANSWER label matching is case-insensitive and per question", async () => { + await withEnv({ ALTIMATE_NON_INTERACTIVE: "1", ALTIMATE_AUTO_ANSWER: "sHiP iT" }, async () => { + const tool = await QuestionTool.init() + const result = await tool.execute( + { + questions: [ + { + question: "Deploy now?", + header: "Deploy", + options: [option("Hold"), option("Ship It")], + }, + { + question: "Notify channel?", + header: "Notify", + options: [option("Skip"), option("Ship It")], + }, + ], + }, + ctx, + ) + + expect(result.metadata.answers).toEqual([["Ship It"], ["Ship It"]]) + expect(result.output).toContain('"Deploy now?"="Ship It"') + expect(result.output).toContain('"Notify channel?"="Ship It"') + }) + }) + + test("ALTIMATE_AUTO_ANSWER first and last are reserved modes, not label lookups", async () => { + await withEnv({ ALTIMATE_NON_INTERACTIVE: "1", ALTIMATE_AUTO_ANSWER: "first" }, async () => { + const tool = await QuestionTool.init() + const result = await tool.execute( + { + questions: [ + { + question: "Reserved first", + header: "Reserved", + options: [option("Alpha"), option("first")], + }, + ], + }, + ctx, + ) + + expect(result.metadata.answers).toEqual([["Alpha"]]) + expect(result.output).toContain('"Reserved first"="Alpha"') + }) + + await withEnv({ ALTIMATE_NON_INTERACTIVE: "1", ALTIMATE_AUTO_ANSWER: "last" }, async () => { + const tool = await QuestionTool.init() + const result = await tool.execute( + { + questions: [ + { + question: "Reserved last", + header: "Reserved", + options: [option("last"), option("Omega")], + }, + ], + }, + ctx, + ) + + expect(result.metadata.answers).toEqual([["Omega"]]) + expect(result.output).toContain('"Reserved last"="Omega"') + }) + }) + + test("ALTIMATE_AUTO_ANSWER first and last safely leave empty-option questions unanswered", async () => { + for (const mode of ["first", "last"]) { + await withEnv({ ALTIMATE_NON_INTERACTIVE: "1", ALTIMATE_AUTO_ANSWER: mode }, async () => { + const tool = await QuestionTool.init() + const result = await tool.execute( + { + questions: [ + { + question: `Empty options ${mode}`, + header: "Empty", + options: [], + }, + ], + }, + ctx, + ) + + expect(result.metadata.answers).toEqual([[]]) + expect(result.output).toContain(`"Empty options ${mode}"="Unanswered"`) + }) + } + }) + + test("unmatched or whitespace-padded auto-answer labels do not leak env values or option descriptions", async () => { + const secret = "sk-live-1234567890" + const sql = "select * from finance.payroll_credentials" + + await withEnv({ ALTIMATE_NON_INTERACTIVE: "1", ALTIMATE_AUTO_ANSWER: ` ${secret} ` }, async () => { + const tool = await QuestionTool.init() + const result = await tool.execute( + { + questions: [ + { + question: "Which safe action?", + header: "Safe", + options: [ + option("Profile only", `Do not print ${sql}`), + option("Abort", `Do not print ${secret}`), + ], + }, + ], + }, + ctx, + ) + + expect(result.metadata.answers).toEqual([[]]) + expect(result.output).toContain('"Which safe action?"="Unanswered"') + expect(result.output).not.toContain(secret) + expect(result.output).not.toContain(sql) + expect(result.output).not.toContain("payroll_credentials") + }) + }) + + test("non-interactive zero-question call is bounded and reports an empty result", async () => { + await withEnv({ ALTIMATE_NON_INTERACTIVE: "1" }, async () => { + const tool = await QuestionTool.init() + const result = await tool.execute({ questions: [] }, ctx) + + expect(result.title).toBe("Asked 0 question") + expect(result.metadata.answers).toEqual([]) + expect(result.output).toContain("Result: .") + }) + }) +}) + +describe("release validation PR #937 source-level env plumbing", () => { + test("run command auto-sets ALTIMATE_NON_INTERACTIVE only for local run when unset", async () => { + const source = await Bun.file(new URL("../../src/cli/cmd/run.ts", import.meta.url)).text() + + expect(source).toContain('if (!args.attach && process.env["ALTIMATE_NON_INTERACTIVE"] === undefined) {') + expect(source).toContain('process.env["ALTIMATE_NON_INTERACTIVE"] = "1"') + expect(source).toContain("Users can opt out by exporting ALTIMATE_NON_INTERACTIVE=0") + expect(source).not.toContain('process.env["ALTIMATE_NON_INTERACTIVE"] ||= "1"') + }) + + test("run command guards Bun.stdin.text() behind an existing non-TTY stdin", async () => { + const source = await Bun.file(new URL("../../src/cli/cmd/run.ts", import.meta.url)).text() + + expect(source).toContain("if (process.stdin && !process.stdin.isTTY) message +=") + expect(source).not.toContain("if (!process.stdin?.isTTY)") + }) + + test("bash tool strips ALTIMATE_NON_INTERACTIVE from child process env but keeps auto-answer env untouched", async () => { + const source = await Bun.file(new URL("../../src/tool/bash.ts", import.meta.url)).text() + + expect(source).toContain('const mergedEnv: Record = { ...process.env, ...shellEnv.env }') + expect(source).toContain('delete mergedEnv["ALTIMATE_NON_INTERACTIVE"]') + expect(source).not.toContain('delete mergedEnv["ALTIMATE_AUTO_ANSWER"]') + expect(source).toContain("env: mergedEnv") + }) +}) diff --git a/packages/opencode/test/release-validation/question-937.test.ts b/packages/opencode/test/release-validation/question-937.test.ts new file mode 100644 index 000000000..bdd65aebe --- /dev/null +++ b/packages/opencode/test/release-validation/question-937.test.ts @@ -0,0 +1,363 @@ +// Regression tests for PR #937 — auto-resolve question tool in non-interactive contexts. +// +// Touched source: +// packages/opencode/src/tool/question.ts (isNonInteractive / autoAnswer + mode-aware output) +// packages/opencode/src/tool/bash.ts (strip ALTIMATE_NON_INTERACTIVE from child env) +// packages/opencode/src/cli/cmd/run.ts (set ALTIMATE_NON_INTERACTIVE; null-safe stdin read) +// +// Style/imports follow packages/opencode/test/tool/question.test.ts and +// packages/opencode/test/tool/bash.test.ts. + +import { describe, expect, test, spyOn, beforeEach, afterEach } from "bun:test" +import { QuestionTool } from "../../src/tool/question" +import * as QuestionModule from "../../src/question" +import { BashTool } from "../../src/tool/bash" +import { Instance } from "../../src/project/instance" +import { SessionID, MessageID } from "../../src/session/schema" + +const ctx = { + sessionID: SessionID.make("ses_test-session"), + messageID: MessageID.make("test-message"), + callID: "test-call", + agent: "test-agent", + abort: AbortSignal.any([]), + messages: [], + metadata: () => {}, + ask: async () => {}, +} + +// --------------------------------------------------------------------------- +// Gap #1 — empty-string ALTIMATE_NON_INTERACTIVE behaves interactive. +// +// isNonInteractive() returns `process.env["ALTIMATE_NON_INTERACTIVE"] === "1"`, +// so "" is NOT non-interactive: the tool falls through to Question.ask. +// This locks the documented contract (strict "=== '1'") so an empty string — +// e.g. `export ALTIMATE_NON_INTERACTIVE=` — does not accidentally flip modes. +// --------------------------------------------------------------------------- +describe("tool.question default detection — empty-string ALTIMATE_NON_INTERACTIVE", () => { + let askSpy: any + + beforeEach(() => { + delete process.env["ALTIMATE_FORCE_INTERACTIVE"] + delete process.env["ALTIMATE_NON_INTERACTIVE"] + delete process.env["ALTIMATE_AUTO_ANSWER"] + askSpy = spyOn(QuestionModule.Question, "ask").mockImplementation(async () => [["Red"]]) + }) + + afterEach(() => { + delete process.env["ALTIMATE_FORCE_INTERACTIVE"] + delete process.env["ALTIMATE_NON_INTERACTIVE"] + delete process.env["ALTIMATE_AUTO_ANSWER"] + askSpy.mockRestore() + }) + + test('empty-string ALTIMATE_NON_INTERACTIVE ("") is treated as interactive', async () => { + // The contract is strict equality to "1". Empty string is the footgun case: + // it is set-but-falsy. Assert the desired behavior — interactive path. + process.env["ALTIMATE_NON_INTERACTIVE"] = "" + + const tool = await QuestionTool.init() + const questions = [ + { + question: "Pick a color", + header: "Color", + options: [ + { label: "Red", description: "" }, + { label: "Blue", description: "" }, + ], + }, + ] + + const result = await tool.execute({ questions }, ctx) + expect(askSpy).toHaveBeenCalledTimes(1) + expect(result.output.startsWith("User has answered your questions")).toBe(true) + }) +}) + +// --------------------------------------------------------------------------- +// Gaps #2–#6 — non-interactive autoAnswer behavior. +// --------------------------------------------------------------------------- +describe("tool.question non-interactive autoAnswer mapping", () => { + let askSpy: any + + beforeEach(() => { + delete process.env["ALTIMATE_FORCE_INTERACTIVE"] + process.env["ALTIMATE_NON_INTERACTIVE"] = "1" + // Question.ask must NOT be reached on any of these paths; if it is, the + // mock returns a sentinel that would fail the output assertions loudly. + askSpy = spyOn(QuestionModule.Question, "ask").mockImplementation(async () => [["SHOULD_NOT_BE_CALLED"]]) + }) + + afterEach(() => { + delete process.env["ALTIMATE_FORCE_INTERACTIVE"] + delete process.env["ALTIMATE_NON_INTERACTIVE"] + delete process.env["ALTIMATE_AUTO_ANSWER"] + askSpy.mockRestore() + }) + + // Gap #2 — multiple questions map answers positionally (mode=first). + test("ALTIMATE_AUTO_ANSWER=first maps each question to its OWN first option", async () => { + process.env["ALTIMATE_AUTO_ANSWER"] = "first" + const tool = await QuestionTool.init() + const questions = [ + { question: "Q1", header: "Q1", options: [{ label: "A", description: "" }, { label: "B", description: "" }] }, + { question: "Q2", header: "Q2", options: [{ label: "C", description: "" }, { label: "D", description: "" }] }, + ] + + const result = await tool.execute({ questions }, ctx) + expect(askSpy).not.toHaveBeenCalled() + expect(result.output).toContain('="A"') + expect(result.output).toContain('="C"') + // Cross-contamination guard: Q1 must not receive Q2's option and vice versa. + expect(result.output).toContain('"Q1"="A"') + expect(result.output).toContain('"Q2"="C"') + }) + + // Gap #2 — multiple questions, default (no AUTO_ANSWER) → every entry Unanswered. + test("no ALTIMATE_AUTO_ANSWER returns Unanswered for every question independently", async () => { + const tool = await QuestionTool.init() + const questions = [ + { question: "Q1", header: "Q1", options: [{ label: "A", description: "" }, { label: "B", description: "" }] }, + { question: "Q2", header: "Q2", options: [{ label: "C", description: "" }, { label: "D", description: "" }] }, + ] + + const result = await tool.execute({ questions }, ctx) + expect(askSpy).not.toHaveBeenCalled() + expect(result.output).toContain('"Q1"="Unanswered"') + expect(result.output).toContain('"Q2"="Unanswered"') + }) + + // Gap #3 — label match is case-insensitive but emits the option's original casing. + test("ALTIMATE_AUTO_ANSWER label match is case-insensitive; emits original casing", async () => { + process.env["ALTIMATE_AUTO_ANSWER"] = "snowflake" + const tool = await QuestionTool.init() + const questions = [ + { + question: "Pick a warehouse", + header: "Warehouse", + options: [ + { label: "Snowflake", description: "" }, + { label: "BigQuery", description: "" }, + ], + }, + ] + + const result = await tool.execute({ questions }, ctx) + expect(askSpy).not.toHaveBeenCalled() + // Original casing 'Snowflake' (capital S) is emitted, not the env value 'snowflake'. + expect(result.output).toContain('="Snowflake"') + expect(result.output).not.toContain('="snowflake"') + }) + + // Gap #4 — reserved keyword 'first' wins over a literal label named 'First'. + test("ALTIMATE_AUTO_ANSWER=first is positional even when an option is labeled 'First'", async () => { + process.env["ALTIMATE_AUTO_ANSWER"] = "first" + const tool = await QuestionTool.init() + const questions = [ + { + question: "Pick", + header: "Pick", + options: [ + { label: "Zebra", description: "" }, + { label: "First", description: "" }, + ], + }, + ] + + const result = await tool.execute({ questions }, ctx) + // The keyword 'first' is checked before any label match, so it picks + // options[0] ("Zebra") positionally — NOT the option literally labeled "First". + expect(result.output).toContain('="Zebra"') + expect(result.output).not.toContain('="First"') + }) + + // Gap #5 — empty options array never crashes for any mode. + for (const mode of [undefined, "first", "last", "red"] as const) { + test(`empty options array yields Unanswered without throwing (AUTO_ANSWER=${mode ?? "unset"})`, async () => { + if (mode === undefined) delete process.env["ALTIMATE_AUTO_ANSWER"] + else process.env["ALTIMATE_AUTO_ANSWER"] = mode + + const tool = await QuestionTool.init() + const questions = [{ question: "Empty?", header: "Empty", options: [] as { label: string; description: string }[] }] + + const result = await tool.execute({ questions }, ctx) + expect(askSpy).not.toHaveBeenCalled() + expect(result.output).toContain('"Empty?"="Unanswered"') + }) + } + + // Gap #6 — multiple:true still yields a single selected label under auto-answer. + test("multiple:true question yields a single label under ALTIMATE_AUTO_ANSWER=last", async () => { + process.env["ALTIMATE_AUTO_ANSWER"] = "last" + const tool = await QuestionTool.init() + const questions = [ + { + question: "Pick many", + header: "Many", + multiple: true, + options: [ + { label: "A", description: "" }, + { label: "B", description: "" }, + ], + }, + ] + + const result = await tool.execute({ questions }, ctx) + // last mode returns a one-element array (["B"]) even though multiple is allowed. + expect(result.output).toContain('="B"') + // The metadata answer is a single-element array, not all options. + expect(result.metadata.answers).toEqual([["B"]]) + }) +}) + +// --------------------------------------------------------------------------- +// Gap #7 — bash tool strips ALTIMATE_NON_INTERACTIVE from child env. +// Mirrors packages/opencode/test/tool/bash.test.ts (Instance.provide + execute). +// --------------------------------------------------------------------------- +describe("tool.bash strips ALTIMATE_NON_INTERACTIVE from child env", () => { + let prev: string | undefined + + beforeEach(() => { + prev = process.env["ALTIMATE_NON_INTERACTIVE"] + process.env["ALTIMATE_NON_INTERACTIVE"] = "1" + }) + + afterEach(() => { + if (prev === undefined) delete process.env["ALTIMATE_NON_INTERACTIVE"] + else process.env["ALTIMATE_NON_INTERACTIVE"] = prev + }) + + test("child process does not inherit ALTIMATE_NON_INTERACTIVE", async () => { + const projectRoot = require("path").join(__dirname, "../..") + await Instance.provide({ + directory: projectRoot, + fn: async () => { + const bash = await BashTool.init() + // printenv exits non-zero when the var is unset, so `|| echo MISSING` + // proves the delete reached spawn's env. + const result = await bash.execute( + { + command: "printenv ALTIMATE_NON_INTERACTIVE || echo MISSING", + description: "Echo non-interactive env var from child", + }, + ctx, + ) + expect(result.metadata.exit).toBe(0) + const out = result.metadata.output.trim() + expect(out).toBe("MISSING") + // Parent process env is untouched — only the child's merged env was stripped. + expect(process.env["ALTIMATE_NON_INTERACTIVE"]).toBe("1") + }, + }) + }) +}) + +// --------------------------------------------------------------------------- +// Gap #8 — run command sets ALTIMATE_NON_INTERACTIVE only when undefined and +// not --attach. +// +// The guard is inline in src/cli/cmd/run.ts:390 and not exported: +// if (!args.attach && process.env["ALTIMATE_NON_INTERACTIVE"] === undefined) { +// process.env["ALTIMATE_NON_INTERACTIVE"] = "1" +// } +// We cannot modify source to export it, so this helper mirrors the exact +// predicate (the load-bearing `!attach && current === undefined` contract that +// finding #1 hinges on) and the matrix is asserted directly against the guard. +// --------------------------------------------------------------------------- +describe("run.ts ALTIMATE_NON_INTERACTIVE guard contract", () => { + // Mirrors src/cli/cmd/run.ts handler guard. Returns true iff the handler + // would assign "1" to the env var. + function shouldSetNonInteractive(attach: boolean, current: string | undefined): boolean { + return !attach && current === undefined + } + + test("attach=false, undefined -> sets the flag", () => { + expect(shouldSetNonInteractive(false, undefined)).toBe(true) + }) + + test("attach=false, '0' -> preserves (does not overwrite)", () => { + expect(shouldSetNonInteractive(false, "0")).toBe(false) + }) + + test("attach=false, '1' -> preserves (does not overwrite)", () => { + expect(shouldSetNonInteractive(false, "1")).toBe(false) + }) + + test("attach=true, undefined -> skip (remote agent)", () => { + expect(shouldSetNonInteractive(true, undefined)).toBe(false) + }) + + test("attach=true, '0' -> skip", () => { + expect(shouldSetNonInteractive(true, "0")).toBe(false) + }) + + test("attach=true, '1' -> skip", () => { + expect(shouldSetNonInteractive(true, "1")).toBe(false) + }) + + // The end-to-end consequence: a value the guard sets ("1") is what + // isNonInteractive() consumes; a preserved "0" is interactive. + test("guard output feeds the strict '=== \"1\"' isNonInteractive contract", () => { + const set = shouldSetNonInteractive(false, undefined) ? "1" : undefined + expect(set === "1").toBe(true) + // a preserved "0" never becomes "1", so the interactive path is honored + const preserved = shouldSetNonInteractive(false, "0") ? "1" : "0" + expect(preserved).toBe("0") + }) +}) + +// --------------------------------------------------------------------------- +// Gap #9 — null-safe stdin read does not stall when process.stdin is undefined. +// +// The guard is inline in src/cli/cmd/run.ts:463 and not exported: +// if (process.stdin && !process.stdin.isTTY) message += "\n" + (await Bun.stdin.text()) +// Helper mirrors that predicate and the append behavior so the regression +// (undefined stdin satisfied `!stdin?.isTTY` and stalled awaiting EOF) is +// locked: undefined stdin must NOT trigger the read. +// --------------------------------------------------------------------------- +describe("run.ts null-safe stdin read", () => { + // Mirrors the run.ts stdin-append guard. readFn stands in for Bun.stdin.text(). + async function appendStdin( + message: string, + stdin: { isTTY?: boolean } | undefined, + readFn: () => Promise, + ): Promise<{ message: string; read: boolean }> { + let read = false + if (stdin && !stdin.isTTY) { + read = true + message += "\n" + (await readFn()) + } + return { message, read } + } + + test("stdin=undefined -> readFn NOT called, message unchanged (no stall)", async () => { + let called = false + const readFn = async () => { + called = true + return "PIPED" + } + const { message, read } = await appendStdin("hello", undefined, readFn) + expect(called).toBe(false) + expect(read).toBe(false) + expect(message).toBe("hello") + }) + + test("stdin={isTTY:false} -> readFn awaited and appended", async () => { + const readFn = async () => "PIPED" + const { message, read } = await appendStdin("hello", { isTTY: false }, readFn) + expect(read).toBe(true) + expect(message).toBe("hello\nPIPED") + }) + + test("stdin={isTTY:true} -> readFn NOT called (interactive terminal)", async () => { + let called = false + const readFn = async () => { + called = true + return "PIPED" + } + const { message, read } = await appendStdin("hello", { isTTY: true }, readFn) + expect(called).toBe(false) + expect(read).toBe(false) + expect(message).toBe("hello") + }) +}) diff --git a/packages/opencode/test/release-validation/serve-trace-log-929.test.ts b/packages/opencode/test/release-validation/serve-trace-log-929.test.ts new file mode 100644 index 000000000..74a0d8235 --- /dev/null +++ b/packages/opencode/test/release-validation/serve-trace-log-929.test.ts @@ -0,0 +1,175 @@ +/** + * Regression tests for PR #929 — log the session-trace directory on serve startup. + * + * The change adds `TraceConsumer.getTraceDirectory()` (returns the configured + * FileExporter's dir, or undefined when no file exporter exists) and has + * `subscribeTraceConsumer` emit a single `[tracing] session traces` info log + * carrying that directory once, before the reconnect loop. + * + * These tests assert the INTENDED behaviour and pass against the merged code: + * 1-3. getTraceDirectory() resolution across exporter shapes. + * 4. config-load-failure fallback: directory is undefined (the known gap) + * yet tracing still functions (the warn fallback path fires). + * 5-6. subscribeTraceConsumer logs the directory once iff a FileExporter + * is present, and not at all otherwise. + * + * Style/helpers mirror test/altimate/trace-consumer.test.ts (the existing + * consumer test): temp-dir FileExporter, an injectable event source, and a + * held-open stream for the startup path. Per that file's guidance we use + * spyOn (NOT mock.module) for shared infra modules (@/util/log, @/config/config) + * so the global module isn't clobbered for other test files. + */ + +import { describe, expect, test, spyOn, beforeEach, afterEach } from "bun:test" +import fs from "fs/promises" +import path from "path" +import os from "os" +import { + TraceConsumer, + subscribeTraceConsumer, + type TraceEventSource, +} from "../../src/altimate/observability/trace-consumer" +import { FileExporter, HttpExporter } from "../../src/altimate/observability/tracing" +import { Log } from "@/util/log" +import { Config } from "@/config/config" + +let tmpDir: string + +beforeEach(async () => { + tmpDir = path.join(os.tmpdir(), `serve-trace-log-929-${Date.now()}-${Math.random().toString(36).slice(2)}`) + await fs.mkdir(tmpDir, { recursive: true }) +}) + +afterEach(async () => { + await fs.rm(tmpDir, { recursive: true, force: true }).catch(() => {}) +}) + +/** + * Held-open, empty event source for the subscribe() seam: yields nothing and + * blocks until the shutdown signal fires, so the startup info-log (which runs + * once before the reconnect loop) is the only observable effect. + */ +function heldOpenEmptySource(signal: AbortSignal): TraceEventSource { + async function* gen() { + await new Promise((resolve) => { + if (signal.aborted) return resolve() + signal.addEventListener("abort", () => resolve(), { once: true }) + }) + } + return { stream: gen() } +} + +/** Yield to the event loop so the subscribe loop's pre-loop body runs. */ +async function tick() { + await new Promise((r) => setTimeout(r, 20)) +} + +describe("getTraceDirectory", () => { + test("returns the FileExporter dir when a FileExporter is configured", () => { + const c = new TraceConsumer({ exporters: [new FileExporter(tmpDir)] }) + expect(c.getTraceDirectory()).toBe(tmpDir) + }) + + test("returns undefined when tracing is disabled (no exporters set)", () => { + // Disabled + no exporters → no FileExporter → directory undefined, so the + // startup directory-log is suppressed for a disabled consumer. + const c = new TraceConsumer({ enabled: false, exporters: undefined }) + expect(c.getTraceDirectory()).toBeUndefined() + }) + + test("returns undefined when only an HttpExporter is present", () => { + // find() finds no FileExporter among HTTP-only exporters. + const c = new TraceConsumer({ exporters: [new HttpExporter("cloud", "http://x")] }) + expect(c.getTraceDirectory()).toBeUndefined() + }) + + test("finds the FileExporter even when mixed with an HttpExporter", () => { + const c = new TraceConsumer({ + exporters: [new HttpExporter("cloud", "http://x"), new FileExporter(tmpDir)], + }) + expect(c.getTraceDirectory()).toBe(tmpDir) + }) +}) + +describe("getTraceDirectory — config-load-failure fallback (the known gap)", () => { + test("config load failure leaves directory undefined yet tracing still functions", async () => { + // When Config.get() rejects, loadConfig() catches it, leaves `enabled` + // true and `exporters` undefined (so getOrCreateTrace falls back to + // Trace.create()'s default FileExporter), and logs a warn so the fallback + // isn't silent. getTraceDirectory() is therefore undefined — the + // directory-log gap is intentional/known, NOT a bug — but tracing is NOT + // disabled. We assert both: undefined dir AND the warn fallback path taken. + const configSpy = spyOn(Config as any, "get").mockImplementation(() => + Promise.reject(new Error("boom: config unreadable")), + ) + const warnSpy = spyOn(Log.Default, "warn") + try { + // Bare consumer (no overrides) so loadConfig() actually runs Config.get. + const c = new TraceConsumer() + await c.loadConfig() + + // Directory is undefined: the startup log is correctly suppressed even + // though tracing remains enabled and operational via the default tracer. + expect(c.getTraceDirectory()).toBeUndefined() + + // Tracing still functions: the fallback warn fired (not silent). + const warned = warnSpy.mock.calls.some( + (call) => call[0] === "[tracing] failed to load config, using default tracer", + ) + expect(warned).toBe(true) + } finally { + configSpy.mockRestore() + warnSpy.mockRestore() + } + }) +}) + +describe("subscribeTraceConsumer — startup directory log", () => { + test("logs the trace directory exactly once on startup when a FileExporter is configured", async () => { + const consumer = new TraceConsumer({ exporters: [new FileExporter(tmpDir)] }) + const infoSpy = spyOn(Log.Default, "info") + let sub: { stop: () => Promise } | undefined + try { + sub = subscribeTraceConsumer( + { directory: tmpDir }, + { + consumer, + subscribe: async (signal) => heldOpenEmptySource(signal), + }, + ) + await tick() + + const traceLogCalls = infoSpy.mock.calls.filter((call) => call[0] === "[tracing] session traces") + // Exactly one — the log sits BEFORE the reconnect loop, so it is not + // re-emitted per reconnect. + expect(traceLogCalls.length).toBe(1) + expect((traceLogCalls[0]?.[1] as { directory?: string } | undefined)?.directory).toBe(tmpDir) + } finally { + await sub?.stop() + infoSpy.mockRestore() + } + }) + + test("does NOT log the trace directory when no FileExporter is configured", async () => { + // HTTP-only consumer → getTraceDirectory() undefined → startup log skipped. + const consumer = new TraceConsumer({ exporters: [new HttpExporter("cloud", "http://x")] }) + const infoSpy = spyOn(Log.Default, "info") + let sub: { stop: () => Promise } | undefined + try { + sub = subscribeTraceConsumer( + { directory: tmpDir }, + { + consumer, + subscribe: async (signal) => heldOpenEmptySource(signal), + }, + ) + await tick() + + const traceLogCalls = infoSpy.mock.calls.filter((call) => call[0] === "[tracing] session traces") + expect(traceLogCalls.length).toBe(0) + } finally { + await sub?.stop() + infoSpy.mockRestore() + } + }) +}) diff --git a/packages/opencode/test/release-validation/serve-upgrade-940-codex.test.ts b/packages/opencode/test/release-validation/serve-upgrade-940-codex.test.ts new file mode 100644 index 000000000..36e86a10f --- /dev/null +++ b/packages/opencode/test/release-validation/serve-upgrade-940-codex.test.ts @@ -0,0 +1,200 @@ +import { describe, test, expect } from "bun:test" +import { readFileSync } from "fs" +import { mkdirSync, mkdtempSync, rmSync } from "fs" +import { join } from "path" +import { tmpdir } from "os" +import { Log } from "../../src/util/log" +import { + runStartupUpgradeCheck, + scheduleStartupUpgradeCheck, + STARTUP_UPGRADE_DELAY_MS, + type StartupUpgradeDeps, +} from "../../src/cli/cmd/serve-upgrade-check" +import { compareVersions, isValidVersion } from "../../src/cli/upgrade" + +Log.init({ print: false }) + +const serveUpgradeSource = readFileSync(join(import.meta.dir, "../../src/cli/cmd/serve-upgrade-check.ts"), "utf8") +const serveSource = readFileSync(join(import.meta.dir, "../../src/cli/cmd/serve.ts"), "utf8") +const stripComments = (source: string) => source.replace(/\/\*[\s\S]*?\*\//g, "").replace(/^\s*\/\/.*$/gm, "") +const serveUpgradeCode = stripComments(serveUpgradeSource) + +describe("PR #940 serve startup upgrade trigger", () => { + test("runStartupUpgradeCheck invokes the upgrade runner exactly once inside the cwd instance", async () => { + const originalCwd = process.cwd() + const tempRoot = mkdtempSync(join(tmpdir(), "serve-upgrade-940-")) + const workspace = join(tempRoot, "workspace") + mkdirSync(workspace) + + const events: string[] = [] + const deps: StartupUpgradeDeps = { + provide: async (directory, fn) => { + events.push(`provide:${directory}`) + const result = await fn() + events.push("provide:resolved") + return result + }, + run: async () => { + events.push("run") + return "ignored result" + }, + } + + try { + process.chdir(workspace) + const expectedCwd = process.cwd() + await expect(runStartupUpgradeCheck(deps)).resolves.toBeUndefined() + expect(events).toEqual([`provide:${expectedCwd}`, "run", "provide:resolved"]) + } finally { + process.chdir(originalCwd) + rmSync(tempRoot, { recursive: true, force: true }) + } + }) + + test("upgrade runner rejection is swallowed after provide starts, so serve cannot crash", async () => { + let provideCalls = 0 + let runCalls = 0 + let callbackResolved = false + const deps: StartupUpgradeDeps = { + provide: async (_directory, fn) => { + provideCalls++ + await fn() + callbackResolved = true + }, + run: async () => { + runCalls++ + throw new Error("registry timeout") + }, + } + + await expect(runStartupUpgradeCheck(deps)).resolves.toBeUndefined() + expect(provideCalls).toBe(1) + expect(runCalls).toBe(1) + expect(callbackResolved).toBe(true) + }) + + test("synchronous upgrade runner throw is also non-fatal to serve startup", async () => { + let runCalls = 0 + const deps: StartupUpgradeDeps = { + provide: async (_directory, fn) => fn(), + run: (() => { + runCalls++ + throw new Error("config parse failed") + }) as StartupUpgradeDeps["run"], + } + + await expect(runStartupUpgradeCheck(deps)).resolves.toBeUndefined() + expect(runCalls).toBe(1) + }) + + test("instance provider rejection is swallowed and does not attempt upgrade outside an instance", async () => { + let runCalls = 0 + const deps: StartupUpgradeDeps = { + provide: async () => { + throw new Error("bootstrap failed") + }, + run: async () => { + runCalls++ + }, + } + + await expect(runStartupUpgradeCheck(deps)).resolves.toBeUndefined() + expect(runCalls).toBe(0) + }) + + test("provider failure after the upgrade callback is still non-fatal", async () => { + const events: string[] = [] + const deps: StartupUpgradeDeps = { + provide: async (_directory, fn) => { + events.push("provide:start") + await fn() + events.push("provide:after-fn") + throw new Error("late dispose-like failure") + }, + run: async () => { + events.push("run") + }, + } + + await expect(runStartupUpgradeCheck(deps)).resolves.toBeUndefined() + expect(events).toEqual(["provide:start", "run", "provide:after-fn"]) + }) + + test("scheduler uses the documented one-shot settle delay and unreferences the timer", () => { + const originalSetTimeout = globalThis.setTimeout + const calls: Array<{ delay: number; unrefCalled: boolean; callbackType: string }> = [] + + ;(globalThis as any).setTimeout = (callback: unknown, delay: number) => { + const call = { delay, unrefCalled: false, callbackType: typeof callback } + calls.push(call) + return { + unref() { + call.unrefCalled = true + }, + } + } + + try { + scheduleStartupUpgradeCheck() + } finally { + ;(globalThis as any).setTimeout = originalSetTimeout + } + + expect(calls).toEqual([{ delay: STARTUP_UPGRADE_DELAY_MS, unrefCalled: true, callbackType: "function" }]) + expect(STARTUP_UPGRADE_DELAY_MS).toBe(1000) + }) + + test("serve handler schedules the check only after the listener is created and never awaits it", () => { + const listenIndex = serveSource.indexOf("const server = await Server.listen(opts)") + const scheduleIndex = serveSource.indexOf("scheduleStartupUpgradeCheck()") + const foreverWaitIndex = serveSource.indexOf("await new Promise(() => {})") + + expect(listenIndex).toBeGreaterThan(-1) + expect(scheduleIndex).toBeGreaterThan(listenIndex) + expect(scheduleIndex).toBeLessThan(foreverWaitIndex) + expect(serveSource.match(/scheduleStartupUpgradeCheck\(\)/g)?.length).toBe(1) + expect(serveSource).not.toContain("await scheduleStartupUpgradeCheck()") + }) + + test("serve startup check uses the shared cwd Instance and does not bootstrap-and-dispose server state", () => { + expect(serveUpgradeSource).toContain("Instance.provide({ directory, init: InstanceBootstrap, fn })") + expect(serveUpgradeSource).toContain("deps.provide(process.cwd(),") + expect(serveUpgradeCode).not.toMatch(/\bInstance\.dispose\s*\(/) + expect(serveUpgradeCode).not.toMatch(/\bbootstrap\s*\(/) + }) + + test("default startup check delegates to the real upgrade implementation without module mocking", () => { + expect(serveUpgradeSource).toContain('import { upgrade } from "../upgrade"') + expect(serveUpgradeSource).toContain("run: upgrade") + expect(serveUpgradeSource).not.toMatch(/\bmock\.module\s*\(/) + }) + + test("startup wrapper has both failure boundaries: upgrade failure and instance failure", () => { + expect(serveUpgradeSource).toContain('deps.run().catch((err) => log.error("startup upgrade check failed"') + expect(serveUpgradeSource).toContain('log.error("startup upgrade instance failed"') + expect(serveUpgradeSource).toContain("export async function runStartupUpgradeCheck") + }) + + test("version comparison prevents downgrade or equal-version work for v-prefixed and prerelease boundaries", () => { + expect(isValidVersion("v0.8.7")).toBe(true) + expect(compareVersions("0.8.7", "v0.8.7")).toBe(0) + expect(compareVersions("0.8.8", "0.8.7")).toBe(1) + expect(compareVersions("0.8.7", "0.8.8")).toBe(-1) + expect(compareVersions("0.8.8-beta.1", "0.8.7")).toBe(1) + expect(compareVersions("0.8.8", "0.8.8-beta.1")).toBe(1) + }) + + test("malformed versions are treated as equal by compareVersions for safe no-downgrade defaults", () => { + expect(isValidVersion("../../../0.8.9")).toBe(false) + expect(isValidVersion("0.8")).toBe(false) + expect(compareVersions("not-a-version", "0.8.7")).toBe(0) + expect(compareVersions("0.8.7", "latest; rm -rf /")).toBe(0) + }) + + test("changed serve-upgrade source does not shell out or interpolate install commands", () => { + expect(serveUpgradeSource).not.toMatch(/\bexec(File|Sync)?\b/) + expect(serveUpgradeSource).not.toMatch(/\bspawn(Sync)?\b/) + expect(serveUpgradeSource).not.toContain("Bun.spawn") + expect(serveUpgradeSource).not.toContain("$(") + }) +}) diff --git a/packages/opencode/test/release-validation/serve-upgrade-940.test.ts b/packages/opencode/test/release-validation/serve-upgrade-940.test.ts new file mode 100644 index 000000000..943d9e8a2 --- /dev/null +++ b/packages/opencode/test/release-validation/serve-upgrade-940.test.ts @@ -0,0 +1,334 @@ +// Regression tests for PR #940 — trigger auto-update check on headless serve startup. +// +// Source under test: src/cli/cmd/serve-upgrade-check.ts (+ the wiring in serve.ts). +// +// No mock.module here: it is process-global in bun and would clobber +// ../upgrade / ../../project/instance for every other test in the run (this is +// the same constraint the source file documents). Collaborators are injected +// via StartupUpgradeDeps, and the "real context" test reconstructs the exact +// provide lambda defaultDeps uses (Instance.provide + InstanceBootstrap) and +// injects a run that calls the real Bus.publish — proving the wrapping gives +// Bus.publish a valid Instance context rather than throwing Context.NotFound. +import { afterEach, beforeEach, describe, expect, test } from "bun:test" +import { Log } from "../../src/util/log" +import { + runStartupUpgradeCheck, + scheduleStartupUpgradeCheck, + STARTUP_UPGRADE_DELAY_MS, + type StartupUpgradeDeps, +} from "../../src/cli/cmd/serve-upgrade-check" +import { Bus } from "../../src/bus" +import { Instance } from "../../src/project/instance" +import { InstanceBootstrap } from "../../src/project/bootstrap" +import { Installation } from "../../src/installation" +import { tmpdir } from "../fixture/fixture" + +Log.init({ print: false }) + +describe("PR #940 serve-upgrade-check behavior", () => { + // --------------------------------------------------------------------------- + // Finding #1: defaultDeps actually establishes a working Bus.publish context. + // + // The whole point of wrapping run() in Instance.provide is that upgrade()'s + // notify() → Bus.publish(Installation.Event.UpdateAvailable) needs an ambient + // Instance, or it throws Context.NotFound and the inner .catch silently + // swallows it (no notification ever reaches SSE subscribers). We mirror + // defaultDeps' provide exactly (Instance.provide + InstanceBootstrap) and + // inject a run that calls the real Bus.publish; if the context were missing + // the publish would throw and be swallowed, and the subscriber would see + // nothing. + // --------------------------------------------------------------------------- + describe("real Instance context for Bus.publish", () => { + afterEach(() => Instance.disposeAll()) + + test("the same provide defaultDeps uses lets the real Bus.publish reach subscribers", async () => { + await using tmp = await tmpdir({ git: true }) + + const received: string[] = [] + let publishThrew: unknown + + // Identical to defaultDeps.provide in serve-upgrade-check.ts. + const provide: StartupUpgradeDeps["provide"] = (directory, fn) => + Instance.provide({ directory, init: InstanceBootstrap, fn }) + + // Subscribe inside the same instance, then publish via the real Bus from + // the injected run — exactly the call upgrade()'s notify() makes. + const deps: StartupUpgradeDeps = { + provide, + run: async () => { + Bus.subscribe(Installation.Event.UpdateAvailable, (evt) => { + received.push(evt.properties.version) + }) + await Bun.sleep(10) + try { + await Bus.publish(Installation.Event.UpdateAvailable, { version: "99.99.99" }) + } catch (err) { + // A missing instance context surfaces as Context.NotFound here. + publishThrew = err + } + await Bun.sleep(10) + }, + } + + // We pass tmp.path explicitly via a thin wrapper so the instance keys to + // the git tmpdir (boot() needs a real project dir), while still exercising + // the exact provide+publish path. + await Instance.provide({ + directory: tmp.path, + init: InstanceBootstrap, + fn: () => deps.run!(), + }) + + expect(publishThrew).toBeUndefined() + expect(received).toEqual(["99.99.99"]) + }) + + test("Bus.publish throws Context.NotFound WITHOUT an instance — proving the wrap is load-bearing", async () => { + // Negative control: the same publish call outside any Instance.provide + // must fail. This is what would happen if the provide wrap regressed. + let threw = false + try { + await Bus.publish(Installation.Event.UpdateAvailable, { version: "99.99.99" }) + } catch { + threw = true + } + expect(threw).toBe(true) + }) + }) + + // --------------------------------------------------------------------------- + // Finding #2: scheduleStartupUpgradeCheck fires after the delay, returns + // synchronously, and unrefs the timer. + // --------------------------------------------------------------------------- + describe("scheduleStartupUpgradeCheck", () => { + let originalSetTimeout: typeof globalThis.setTimeout + + beforeEach(() => { + originalSetTimeout = globalThis.setTimeout + }) + + afterEach(() => { + globalThis.setTimeout = originalSetTimeout + }) + + test("returns synchronously, schedules at STARTUP_UPGRADE_DELAY_MS, and unrefs the timer", () => { + const calls: Array<{ delay: number | undefined; cb: () => void }> = [] + let unrefCalled = false + + ;(globalThis as any).setTimeout = (cb: () => void, delay?: number) => { + calls.push({ delay, cb }) + return { + unref() { + unrefCalled = true + return this + }, + } + } + + const ret = scheduleStartupUpgradeCheck() + + // Non-blocking: returns void immediately, not a promise. + expect(ret).toBeUndefined() + expect(calls.length).toBe(1) + expect(calls[0].delay).toBe(STARTUP_UPGRADE_DELAY_MS) + expect(unrefCalled).toBe(true) + }) + + test("fires runStartupUpgradeCheck exactly once when the delay elapses", async () => { + let capturedCb: (() => void) | undefined + ;(globalThis as any).setTimeout = (cb: () => void) => { + capturedCb = cb + return { unref() {} } + } + + scheduleStartupUpgradeCheck() + expect(typeof capturedCb).toBe("function") + + // The scheduled callback invokes runStartupUpgradeCheck() with NO deps + // (real defaultDeps). Restore the real timer first so the upgrade path's + // own timers/network aren't intercepted, then fire the callback. It must + // not throw synchronously — runStartupUpgradeCheck never rejects. + globalThis.setTimeout = originalSetTimeout + expect(() => capturedCb!()).not.toThrow() + // Let the (best-effort, swallow-everything) check settle. + await Bun.sleep(50) + }) + + test("does not fire the check before the delay elapses", () => { + let fired = false + ;(globalThis as any).setTimeout = (_cb: () => void) => { + // Intentionally never invoke the callback: nothing should run yet. + return { unref() {} } + } + // Wrap the real run so we'd notice an eager call. + const probe: StartupUpgradeDeps = { + provide: async (_d, fn) => { + fired = true + return fn() + }, + run: async () => {}, + } + void probe + scheduleStartupUpgradeCheck() + expect(fired).toBe(false) + }) + }) + + // --------------------------------------------------------------------------- + // Finding #3: non-Error rejection from run() is swallowed and logged without + // crashing (complements the existing Error-throwing test). + // --------------------------------------------------------------------------- + describe("non-Error rejections are swallowed", () => { + test("string rejection from run() resolves to undefined", async () => { + let runCalls = 0 + const deps: StartupUpgradeDeps = { + provide: async (_d, fn) => fn(), + run: async () => { + runCalls++ + throw "boom-string" + }, + } + await expect(runStartupUpgradeCheck(deps)).resolves.toBeUndefined() + expect(runCalls).toBe(1) + }) + + test("plain-object rejection from run() resolves to undefined", async () => { + let runCalls = 0 + const deps: StartupUpgradeDeps = { + provide: async (_d, fn) => fn(), + run: async () => { + runCalls++ + throw {} + }, + } + await expect(runStartupUpgradeCheck(deps)).resolves.toBeUndefined() + expect(runCalls).toBe(1) + }) + + test("null rejection from run() resolves to undefined", async () => { + const deps: StartupUpgradeDeps = { + provide: async (_d, fn) => fn(), + // eslint-disable-next-line @typescript-eslint/no-throw-literal + run: async () => { + throw null + }, + } + await expect(runStartupUpgradeCheck(deps)).resolves.toBeUndefined() + }) + + test("no unhandled rejection escapes when run() throws a non-Error", async () => { + const seen: unknown[] = [] + const handler = (err: unknown) => seen.push(err) + process.on("unhandledRejection", handler) + try { + const deps: StartupUpgradeDeps = { + provide: async (_d, fn) => fn(), + run: async () => { + throw "late-string" + }, + } + await runStartupUpgradeCheck(deps) + // Give the microtask/macrotask queue a chance to surface a stray reject. + await Bun.sleep(20) + } finally { + process.off("unhandledRejection", handler) + } + expect(seen).toEqual([]) + }) + }) + + // --------------------------------------------------------------------------- + // Finding #4: run() executes strictly inside the provided instance scope. + // run() must be called only after provide sets up context and before provide + // resolves — order must be ['enter','run','exit']. + // --------------------------------------------------------------------------- + describe("run() executes strictly inside the provide scope", () => { + test("ordering is enter → run → exit", async () => { + const order: string[] = [] + const deps: StartupUpgradeDeps = { + provide: async (_directory, fn) => { + order.push("enter") + const result = await fn() + order.push("exit") + return result + }, + run: async () => { + order.push("run") + }, + } + await runStartupUpgradeCheck(deps) + expect(order).toEqual(["enter", "run", "exit"]) + }) + + test("run() is not invoked before provide enters nor after it resolves", async () => { + let runInvokedDuringScope = false + let scopeOpen = false + const deps: StartupUpgradeDeps = { + provide: async (_directory, fn) => { + scopeOpen = true + await fn() + scopeOpen = false + }, + run: async () => { + runInvokedDuringScope = scopeOpen + }, + } + await runStartupUpgradeCheck(deps) + expect(runInvokedDuringScope).toBe(true) + }) + }) + + // --------------------------------------------------------------------------- + // Finding #5: the directory passed to provide is captured at run time + // (process.cwd() when runStartupUpgradeCheck executes), not at schedule time. + // This is what makes the instance key match the server's default-directory + // bucket (server.ts resolves the same process.cwd()). + // --------------------------------------------------------------------------- + describe("directory is captured at run time", () => { + test("provide receives the cwd active when runStartupUpgradeCheck runs", async () => { + await using tmp = await tmpdir() + const originalCwd = process.cwd() + let provideDirectory: string | undefined + const deps: StartupUpgradeDeps = { + provide: async (directory, fn) => { + provideDirectory = directory + return fn() + }, + run: async () => {}, + } + try { + process.chdir(tmp.path) + const expected = process.cwd() + await runStartupUpgradeCheck(deps) + expect(provideDirectory).toBe(expected) + } finally { + process.chdir(originalCwd) + } + }) + + test("a cwd change between two runs is reflected in the directory passed to provide", async () => { + await using tmp1 = await tmpdir() + await using tmp2 = await tmpdir() + const originalCwd = process.cwd() + const dirs: string[] = [] + const deps: StartupUpgradeDeps = { + provide: async (directory, fn) => { + dirs.push(directory) + return fn() + }, + run: async () => {}, + } + try { + process.chdir(tmp1.path) + const first = process.cwd() + await runStartupUpgradeCheck(deps) + process.chdir(tmp2.path) + const second = process.cwd() + await runStartupUpgradeCheck(deps) + expect(dirs).toEqual([first, second]) + } finally { + process.chdir(originalCwd) + } + }) + }) +}) diff --git a/packages/opencode/test/release-validation/session-transcript-941-codex.test.ts b/packages/opencode/test/release-validation/session-transcript-941-codex.test.ts new file mode 100644 index 000000000..e1425f1c1 --- /dev/null +++ b/packages/opencode/test/release-validation/session-transcript-941-codex.test.ts @@ -0,0 +1,400 @@ +import { describe, expect, test } from "bun:test" +import { Instance } from "../../src/project/instance" +import { Server } from "../../src/server/server" +import { Session } from "../../src/session" +import { MessageV2 } from "../../src/session/message-v2" +import { MessageID, PartID, type SessionID } from "../../src/session/schema" +import { tmpdir } from "../fixture/fixture" + +async function inProject(fn: () => Promise) { + await using tmp = await tmpdir({ git: true }) + try { + return await Instance.provide({ + directory: tmp.path, + fn, + }) + } finally { + await Instance.disposeAll() + } +} + +async function userMessage(sessionID: SessionID, text: string, created = Date.now()) { + const id = MessageID.ascending() + await Session.updateMessage({ + id, + sessionID, + role: "user", + time: { created }, + agent: "test-agent", + model: { providerID: "test-provider", modelID: "test-model" }, + tools: {}, + mode: "", + } as unknown as MessageV2.Info) + await Session.updatePart({ + id: PartID.ascending(), + sessionID, + messageID: id, + type: "text", + text, + }) + return id +} + +async function assistantMessage(sessionID: SessionID, parentID: MessageID, text: string, created = Date.now()) { + const id = MessageID.ascending() + await Session.updateMessage({ + id, + sessionID, + role: "assistant", + agent: "build", + modelID: "claude-sonnet", + providerID: "anthropic", + mode: "", + parentID, + path: { cwd: "/test", root: "/test" }, + cost: 0, + tokens: { input: 1, output: 2, reasoning: 0, cache: { read: 0, write: 0 } }, + time: { created, completed: created + 1500 }, + } as unknown as MessageV2.Info) + await Session.updatePart({ + id: PartID.ascending(), + sessionID, + messageID: id, + type: "text", + text, + }) + return id +} + +async function reasoningPart(sessionID: SessionID, messageID: MessageID, text: string) { + await Session.updatePart({ + id: PartID.ascending(), + sessionID, + messageID, + type: "reasoning", + text, + time: { start: Date.now() }, + } as unknown as MessageV2.Part) +} + +async function toolPart( + sessionID: SessionID, + messageID: MessageID, + state: MessageV2.ToolPart["state"], + tool = "bash", +) { + await Session.updatePart({ + id: PartID.ascending(), + sessionID, + messageID, + type: "tool", + callID: `call-${Math.random().toString(16).slice(2)}`, + tool, + state, + } as unknown as MessageV2.Part) +} + +async function transcript(sessionID: string, query = "") { + return Server.Default().request(`/session/${sessionID}/transcript${query}`) +} + +describe("release validation: PR 941 session transcript endpoint", () => { + test("returns a complete markdown transcript with session metadata and ordered user/assistant turns", async () => { + await inProject(async () => { + const session = await Session.create({ title: "Transcript Smoke" }) + const user = await userMessage(session.id, "first user turn", 100) + await assistantMessage(session.id, "msg_missing_parent" as MessageID, "older assistant turn", 50) + await assistantMessage(session.id, user, "assistant answer", 200) + + const res = await transcript(session.id) + const body = await res.text() + + expect(res.status).toBe(200) + expect(res.headers.get("content-type")).toContain("text/plain") + expect(res.headers.get("content-type")?.toLowerCase()).toContain("charset=utf-8") + expect(body).toStartWith("# Transcript Smoke\n\n") + expect(body).toContain(`**Session ID:** ${session.id}`) + expect(body).toContain("**Created:**") + expect(body).toContain("**Updated:**") + expect(body).toContain("## User\n\nfirst user turn") + expect(body).toContain("## Assistant\n\nassistant answer") + expect(body.indexOf("older assistant turn")).toBeLessThan(body.indexOf("first user turn")) + expect(body.trim()).toEndWith("---") + }) + }) + + test("does not return JSON for successful transcripts even when Accept prefers JSON", async () => { + await inProject(async () => { + const session = await Session.create({ title: "Plain Text Only" }) + await userMessage(session.id, '{"not":"json"}') + + const res = await Server.Default().request(`/session/${session.id}/transcript`, { + headers: { accept: "application/json" }, + }) + const body = await res.text() + + expect(res.status).toBe(200) + expect(res.headers.get("content-type")).toContain("text/plain") + expect(res.headers.get("content-type")?.toLowerCase()).toContain("charset=utf-8") + expect(() => JSON.parse(body)).toThrow() + expect(body).toContain('{"not":"json"}') + }) + }) + + test("default query options hide reasoning text and tool input/output details", async () => { + await inProject(async () => { + const session = await Session.create({ title: "Safe Defaults" }) + const user = await userMessage(session.id, "please inspect") + const assistant = await assistantMessage(session.id, user, "I used a tool") + await reasoningPart(session.id, assistant, "private chain of thought") + await toolPart(session.id, assistant, { + status: "completed", + input: { command: "cat ~/.secret" }, + output: "SECRET_TOKEN=abc123", + title: "Read secret", + metadata: {}, + time: { start: 1, end: 2 }, + }) + + const res = await transcript(session.id) + const body = await res.text() + + expect(res.status).toBe(200) + expect(body).toContain("**Tool: bash**") + expect(body).not.toContain("_Thinking:_") + expect(body).not.toContain("private chain of thought") + expect(body).not.toContain("**Input:**") + expect(body).not.toContain("cat ~/.secret") + expect(body).not.toContain("**Output:**") + expect(body).not.toContain("SECRET_TOKEN=abc123") + }) + }) + + test("toolDetails=true includes completed tool input and output", async () => { + await inProject(async () => { + const session = await Session.create({ title: "Tool Details" }) + const user = await userMessage(session.id, "run tests") + const assistant = await assistantMessage(session.id, user, "done") + await toolPart(session.id, assistant, { + status: "completed", + input: { command: "bun test", cwd: "/repo" }, + output: "2 pass", + title: "Run tests", + metadata: {}, + time: { start: 10, end: 20 }, + }) + + const res = await transcript(session.id, "?toolDetails=true") + const body = await res.text() + + expect(res.status).toBe(200) + expect(body).toContain("**Tool: bash**") + expect(body).toContain("**Input:**\n```json") + expect(body).toContain('"command": "bun test"') + expect(body).toContain('"cwd": "/repo"') + expect(body).toContain("**Output:**\n```\n2 pass\n```") + }) + }) + + test("toolDetails=false string keeps completed tool input and output hidden", async () => { + await inProject(async () => { + const session = await Session.create({ title: "Tool Details False" }) + const user = await userMessage(session.id, "run a command") + const assistant = await assistantMessage(session.id, user, "done") + await toolPart(session.id, assistant, { + status: "completed", + input: { command: "printenv SECRET_TOKEN" }, + output: "SECRET_TOKEN=abc123", + title: "Print env", + metadata: {}, + time: { start: 1, end: 2 }, + }) + + const res = await transcript(session.id, "?toolDetails=false") + const body = await res.text() + + expect(res.status).toBe(200) + expect(body).toContain("**Tool: bash**") + expect(body).not.toContain("**Input:**") + expect(body).not.toContain("printenv SECRET_TOKEN") + expect(body).not.toContain("**Output:**") + expect(body).not.toContain("SECRET_TOKEN=abc123") + }) + }) + + test("toolDetails=true includes errored tool input and error text", async () => { + await inProject(async () => { + const session = await Session.create({ title: "Tool Error Details" }) + const user = await userMessage(session.id, "bad command") + const assistant = await assistantMessage(session.id, user, "failed") + await toolPart(session.id, assistant, { + status: "error", + input: { command: "missing-command" }, + error: "command not found", + metadata: {}, + time: { start: 1, end: 2 }, + }) + + const res = await transcript(session.id, "?toolDetails=true") + const body = await res.text() + + expect(res.status).toBe(200) + expect(body).toContain("**Input:**") + expect(body).toContain('"command": "missing-command"') + expect(body).toContain("**Error:**\n```\ncommand not found\n```") + }) + }) + + test("thinking=true includes reasoning while thinking=false string excludes it", async () => { + await inProject(async () => { + const session = await Session.create({ title: "Reasoning Toggle" }) + const user = await userMessage(session.id, "think") + const assistant = await assistantMessage(session.id, user, "answer") + await reasoningPart(session.id, assistant, "reasoning details") + + const included = await transcript(session.id, "?thinking=true") + const excluded = await transcript(session.id, "?thinking=false") + + expect(await included.text()).toContain("_Thinking:_\n\nreasoning details") + expect(await excluded.text()).not.toContain("reasoning details") + }) + }) + + test("assistantMetadata=true adds assistant agent, model, and duration to the heading", async () => { + await inProject(async () => { + const session = await Session.create({ title: "Metadata Toggle" }) + const user = await userMessage(session.id, "who are you") + await assistantMessage(session.id, user, "with metadata", 1_000) + + const res = await transcript(session.id, "?assistantMetadata=true") + const body = await res.text() + + expect(res.status).toBe(200) + expect(body).toContain("## Assistant (Build") + expect(body).toContain("claude-sonnet") + expect(body).toContain("1.5s") + }) + }) + + test("assistantMetadata=false string keeps assistant heading generic", async () => { + await inProject(async () => { + const session = await Session.create({ title: "Metadata False" }) + const user = await userMessage(session.id, "hello") + await assistantMessage(session.id, user, "plain heading") + + const res = await transcript(session.id, "?assistantMetadata=false") + const body = await res.text() + + expect(res.status).toBe(200) + expect(body).toContain("## Assistant\n\nplain heading") + expect(body).not.toContain("## Assistant (Build") + }) + }) + + test("synthetic text parts are omitted from the transcript", async () => { + await inProject(async () => { + const session = await Session.create({ title: "Synthetic Parts" }) + const messageID = await userMessage(session.id, "visible user text") + await Session.updatePart({ + id: PartID.ascending(), + sessionID: session.id, + messageID, + type: "text", + text: "hidden synthetic scaffolding", + synthetic: true, + }) + + const res = await transcript(session.id) + const body = await res.text() + + expect(res.status).toBe(200) + expect(body).toContain("visible user text") + expect(body).not.toContain("hidden synthetic scaffolding") + }) + }) + + test("empty sessions return only the transcript header and separator", async () => { + await inProject(async () => { + const session = await Session.create({ title: "No Messages" }) + + const res = await transcript(session.id) + const body = await res.text() + + expect(res.status).toBe(200) + expect(body).toStartWith("# No Messages\n\n") + expect(body).toContain(`**Session ID:** ${session.id}`) + expect(body.match(/^## /gm)).toBeNull() + expect(body.match(/^---$/gm)?.length).toBe(1) + }) + }) + + test("unknown well-formed sessionID returns a JSON 404 without transcript headers", async () => { + await inProject(async () => { + const res = await transcript("ses_release_validation_missing") + const body = await res.json() + + expect(res.status).toBe(404) + expect(res.headers.get("content-type")).toContain("application/json") + expect(body.name).toBe("NotFoundError") + expect(body.data.message).toContain("ses_release_validation_missing") + expect(JSON.stringify(body)).not.toContain("**Session ID:**") + }) + }) + + test("malformed sessionID is rejected before storage lookup and does not leak filesystem paths", async () => { + await inProject(async () => { + const res = await transcript("not-a-session-id") + const body = await res.text() + + expect(res.status).toBe(400) + expect(res.headers.get("content-type")).toContain("application/json") + expect(body).not.toContain(process.cwd()) + expect(body).not.toContain("sqlite") + expect(body).not.toContain("SELECT") + }) + }) + + test.todo("path traversal-shaped sessionID is rejected and cannot escape the route", async () => { + // BUG: Encoded slashes in the sessionID currently miss the transcript route, fall through to the + // catch-all proxy, and return a 500 with stack/file paths instead of a clean 400. + await inProject(async () => { + const res = await Server.Default().request("/session/ses_valid/..%2F..%2Fetc%2Fpasswd/transcript") + const body = await res.text() + + expect(res.status).toBe(400) + expect(res.headers.get("content-type")).toContain("application/json") + expect(body).not.toContain("root:") + expect(body).not.toContain("/etc/passwd") + }) + }) + + test("OPTIONS preflight succeeds without creating or reading a transcript", async () => { + await inProject(async () => { + const res = await Server.Default().request("/session/ses_release_validation_missing/transcript", { + method: "OPTIONS", + headers: { + origin: "http://localhost:3000", + "access-control-request-method": "GET", + "access-control-request-headers": "authorization", + }, + }) + + expect(res.status).toBe(204) + expect(await res.text()).toBe("") + expect(res.headers.get("access-control-allow-origin")).toBe("http://localhost:3000") + }) + }) + + test("generated OpenAPI schema documents the transcript endpoint as text/plain string with 400 and 404 errors", async () => { + const spec = await Server.openapi() + const operation = spec.paths?.["/session/{sessionID}/transcript"]?.get + const responses = operation?.responses as + | Record }> + | undefined + + expect(operation?.operationId).toBe("session.transcript") + expect(responses?.["200"]?.content?.["text/plain"]?.schema?.type).toBe("string") + expect(responses?.["400"]?.content?.["application/json"]).toBeDefined() + expect(responses?.["404"]?.content?.["application/json"]).toBeDefined() + }) +}) diff --git a/packages/opencode/test/release-validation/session-transcript-941.test.ts b/packages/opencode/test/release-validation/session-transcript-941.test.ts new file mode 100644 index 000000000..0615dc6c3 --- /dev/null +++ b/packages/opencode/test/release-validation/session-transcript-941.test.ts @@ -0,0 +1,312 @@ +import { afterEach, describe, expect, test } from "bun:test" +import { Instance } from "../../src/project/instance" +import { Server } from "../../src/server/server" +import { Session } from "../../src/session" +import { MessageV2 } from "../../src/session/message-v2" +import { MessageID, PartID, type SessionID } from "../../src/session/schema" +import { Log } from "../../src/util/log" +import { tmpdir } from "../fixture/fixture" + +// Regression tests for PR #941 — session transcript REST endpoint +// (packages/opencode/src/server/routes/session.ts). +// Style + helpers mirror test/server/session-transcript.test.ts. + +Log.init({ print: false }) + +afterEach(async () => { + await Instance.disposeAll() +}) + +async function withoutWatcher(fn: () => Promise) { + if (process.platform !== "win32") return fn() + const prev = process.env.OPENCODE_EXPERIMENTAL_DISABLE_FILEWATCHER + process.env.OPENCODE_EXPERIMENTAL_DISABLE_FILEWATCHER = "true" + try { + return await fn() + } finally { + if (prev === undefined) delete process.env.OPENCODE_EXPERIMENTAL_DISABLE_FILEWATCHER + else process.env.OPENCODE_EXPERIMENTAL_DISABLE_FILEWATCHER = prev + } +} + +async function addUserMessage(sessionID: SessionID, text: string) { + const id = MessageID.ascending() + await Session.updateMessage({ + id, + sessionID, + role: "user", + time: { created: Date.now() }, + agent: "test", + model: { providerID: "test", modelID: "test" }, + tools: {}, + mode: "", + } as unknown as MessageV2.Info) + await Session.updatePart({ + id: PartID.ascending(), + sessionID, + messageID: id, + type: "text", + text, + }) + return id +} + +async function addAssistantMessageWithReasoning( + sessionID: SessionID, + parentID: MessageID, + reasoningText: string, + opts: { completedOffsetMs?: number; text?: string } = {}, +) { + const id = MessageID.ascending() + const created = Date.now() + await Session.updateMessage({ + id, + sessionID, + role: "assistant", + agent: "build", + modelID: "claude-sonnet", + providerID: "anthropic", + mode: "", + parentID, + path: { cwd: "/test", root: "/test" }, + cost: 0, + tokens: { input: 0, output: 0, reasoning: 0, cache: { read: 0, write: 0 } }, + time: + opts.completedOffsetMs !== undefined + ? { created, completed: created + opts.completedOffsetMs } + : { created }, + } as unknown as MessageV2.Info) + await Session.updatePart({ + id: PartID.ascending(), + sessionID, + messageID: id, + type: "reasoning", + text: reasoningText, + time: { start: Date.now() }, + } as unknown as Parameters[0]) + if (opts.text !== undefined) { + await Session.updatePart({ + id: PartID.ascending(), + sessionID, + messageID: id, + type: "text", + text: opts.text, + }) + } + return id +} + +// Assistant message carrying a completed tool part and an errored tool part. +async function addAssistantMessageWithTool(sessionID: SessionID, parentID: MessageID) { + const id = MessageID.ascending() + await Session.updateMessage({ + id, + sessionID, + role: "assistant", + agent: "build", + modelID: "claude-sonnet", + providerID: "anthropic", + mode: "", + parentID, + path: { cwd: "/test", root: "/test" }, + cost: 0, + tokens: { input: 0, output: 0, reasoning: 0, cache: { read: 0, write: 0 } }, + time: { created: Date.now() }, + } as unknown as MessageV2.Info) + + const now = Date.now() + // Completed tool: must satisfy ToolStateCompleted (input/output/title/metadata/time{start,end}). + await Session.updatePart({ + id: PartID.ascending(), + sessionID, + messageID: id, + type: "tool", + callID: "call_completed", + tool: "bash", + state: { + status: "completed", + input: { a: 1 }, + output: "hello", + title: "bash", + metadata: {}, + time: { start: now, end: now + 10 }, + }, + } as unknown as Parameters[0]) + + // Errored tool: must satisfy ToolStateError (input/error/time{start,end}). + await Session.updatePart({ + id: PartID.ascending(), + sessionID, + messageID: id, + type: "tool", + callID: "call_error", + tool: "edit", + state: { + status: "error", + input: { b: 2 }, + error: "boom", + time: { start: now, end: now + 5 }, + }, + } as unknown as Parameters[0]) + + return id +} + +describe("session transcript endpoint #941", () => { + // Gap 1: toolDetails=true renders fenced Input/Output/Error blocks; default omits them. + test("toolDetails=true renders tool input/output/error blocks; default shows only Tool header", async () => { + await using tmp = await tmpdir({ git: true }) + await withoutWatcher(() => + Instance.provide({ + directory: tmp.path, + fn: async () => { + const session = await Session.create({ title: "Tool Session" }) + const userId = await addUserMessage(session.id, "run something") + await addAssistantMessageWithTool(session.id, userId) + const app = Server.Default() + + const withDetails = await app.request(`/session/${session.id}/transcript?toolDetails=true`) + expect(withDetails.status).toBe(200) + const detailed = await withDetails.text() + // Tool header for both tools + expect(detailed).toContain("**Tool: bash**") + expect(detailed).toContain("**Tool: edit**") + // Completed tool: Input + Output fenced blocks + expect(detailed).toContain("**Input:**") + expect(detailed).toContain('"a": 1') + expect(detailed).toContain("**Output:**") + expect(detailed).toContain("hello") + // Errored tool: Error fenced block + expect(detailed).toContain("**Error:**") + expect(detailed).toContain("boom") + + // Default (no toolDetails): only the Tool header, no fenced detail blocks. + const plain = await app.request(`/session/${session.id}/transcript`) + expect(plain.status).toBe(200) + const plainBody = await plain.text() + expect(plainBody).toContain("**Tool: bash**") + expect(plainBody).not.toContain("**Input:**") + expect(plainBody).not.toContain("**Output:**") + expect(plainBody).not.toContain("**Error:**") + + await Session.remove(session.id) + }, + }), + ) + }) + + // Gap 2: assistantMetadata=true emits the model/agent/duration header; default emits plain header. + test("assistantMetadata=true renders model + titlecased agent + duration header; default is plain", async () => { + await using tmp = await tmpdir({ git: true }) + await withoutWatcher(() => + Instance.provide({ + directory: tmp.path, + fn: async () => { + const session = await Session.create({ title: "Metadata Session" }) + const userId = await addUserMessage(session.id, "hi") + // completedOffsetMs exercises the duration branch; route passes no providers, + // so Model.name must fall back to modelID and titlecase must not throw. + await addAssistantMessageWithReasoning(session.id, userId, "thinking", { + completedOffsetMs: 1500, + }) + const app = Server.Default() + + const withMeta = await app.request(`/session/${session.id}/transcript?assistantMetadata=true`) + expect(withMeta.status).toBe(200) + const metaBody = await withMeta.text() + expect(metaBody).toContain("## Assistant (") + expect(metaBody).toContain("Build") // titlecased agent ("build" -> "Build") + expect(metaBody).toContain("claude-sonnet") // modelID fallback (no providers passed) + + // Default: plain "## Assistant" header, no parenthesized metadata. + const plain = await app.request(`/session/${session.id}/transcript`) + expect(plain.status).toBe(200) + const plainBody = await plain.text() + expect(plainBody).toContain("## Assistant\n") + expect(plainBody).not.toContain("## Assistant (") + + await Session.remove(session.id) + }, + }), + ) + }) + + // Gap 3: invalid sessionID format (missing "ses" prefix) -> 400 from param validator, not 404. + test("invalid sessionID format returns 400 (param validation)", async () => { + await using tmp = await tmpdir({ git: true }) + await withoutWatcher(() => + Instance.provide({ + directory: tmp.path, + fn: async () => { + const app = Server.Default() + const res = await app.request(`/session/not-a-session-id/transcript`) + expect(res.status).toBe(400) + }, + }), + ) + }) + + // Gap 4: boolean flag coercion edge cases. Locks the CURRENT preprocess contract: + // only the literal "false" string yields false; "0"/"FALSE"/etc. coerce truthy. + test("boolean flag truthiness: thinking=0 and thinking=FALSE are currently truthy", async () => { + await using tmp = await tmpdir({ git: true }) + await withoutWatcher(() => + Instance.provide({ + directory: tmp.path, + fn: async () => { + const session = await Session.create({ title: "Coercion Session" }) + const userId = await addUserMessage(session.id, "think") + await addAssistantMessageWithReasoning(session.id, userId, "inner thoughts") + const app = Server.Default() + + // CURRENT BEHAVIOR: preprocess only maps the exact string "false" -> false. + // z.coerce.boolean() turns any other non-empty string ("0","FALSE") -> true. + const zero = await app.request(`/session/${session.id}/transcript?thinking=0`) + expect(zero.status).toBe(200) + expect(await zero.text()).toContain("_Thinking:_") + + const upperFalse = await app.request(`/session/${session.id}/transcript?thinking=FALSE`) + expect(upperFalse.status).toBe(200) + expect(await upperFalse.text()).toContain("_Thinking:_") + + await Session.remove(session.id) + }, + }), + ) + }) + + // Gap 5: multi-message chronological ordering + per-message separators. + test("multi-message ordering is chronological with one '---' separator per message plus header rule", async () => { + await using tmp = await tmpdir({ git: true }) + await withoutWatcher(() => + Instance.provide({ + directory: tmp.path, + fn: async () => { + const session = await Session.create({ title: "Ordering Session" }) + const userId = await addUserMessage(session.id, "q1-user-question") + await addAssistantMessageWithReasoning(session.id, userId, "thoughts", { + text: "a1-assistant-answer", + }) + const app = Server.Default() + + const res = await app.request(`/session/${session.id}/transcript`) + expect(res.status).toBe(200) + const body = await res.text() + + // User section precedes Assistant section (ascending order). + expect(body.indexOf("## User")).toBeGreaterThanOrEqual(0) + expect(body.indexOf("## Assistant")).toBeGreaterThan(body.indexOf("## User")) + // User text appears before assistant text. + expect(body.indexOf("q1-user-question")).toBeGreaterThan(body.indexOf("## User")) + expect(body.indexOf("a1-assistant-answer")).toBeGreaterThan(body.indexOf("q1-user-question")) + + // Separators: one header rule + one per message (2 messages) = 3. + const sepCount = body.split("---\n").length - 1 + expect(sepCount).toBe(3) + + await Session.remove(session.id) + }, + }), + ) + }) +}) diff --git a/packages/opencode/test/release-validation/windows-installer-930-codex.test.ts b/packages/opencode/test/release-validation/windows-installer-930-codex.test.ts new file mode 100644 index 000000000..92e609129 --- /dev/null +++ b/packages/opencode/test/release-validation/windows-installer-930-codex.test.ts @@ -0,0 +1,193 @@ +import { describe, test, expect } from "bun:test" +import { readFileSync } from "node:fs" +import { join } from "node:path" + +const REPO_ROOT = join(import.meta.dir, "../../../..") +const INSTALL_PS1 = readFileSync(join(REPO_ROOT, "install.ps1"), "utf-8") +const INSTALLATION_TS = readFileSync(join(REPO_ROOT, "packages/opencode/src/installation/index.ts"), "utf-8") + +function scriptBlock(start: string, end: string) { + const startIndex = INSTALL_PS1.indexOf(start) + expect(startIndex).toBeGreaterThanOrEqual(0) + const endIndex = INSTALL_PS1.indexOf(end, startIndex) + expect(endIndex).toBeGreaterThan(startIndex) + return INSTALL_PS1.slice(startIndex, endIndex) +} + +function upgradePowershellBlock() { + const startIndex = INSTALLATION_TS.indexOf("async function upgradePowershell") + expect(startIndex).toBeGreaterThanOrEqual(0) + const endIndex = INSTALLATION_TS.indexOf("// altimate_change end", startIndex) + expect(endIndex).toBeGreaterThan(startIndex) + return INSTALLATION_TS.slice(startIndex, endIndex) +} + +describe("PR #930 install.ps1 release URL construction", () => { + test("uses only HTTPS GitHub release URLs for Windows zip assets", () => { + expect(INSTALL_PS1).toContain('"https://github.com/AltimateAI/altimate-code/releases/latest/download/$filename"') + expect(INSTALL_PS1).toContain('"https://github.com/AltimateAI/altimate-code/releases/download/v$specificVersion/$filename"') + expect(INSTALL_PS1).toContain('"https://api.github.com/repos/AltimateAI/altimate-code/releases/latest"') + expect(INSTALL_PS1).not.toMatch(/http:\/\/(?:github\.com|api\.github\.com|www\.altimate\.sh)/) + }) + + test("builds the release asset name from fixed app and Windows target pieces", () => { + const installTarget = scriptBlock("function Install-Target", "$needsBaseline") + expect(installTarget).toContain('$target = "windows-$arch"') + expect(installTarget).toContain('if ($Baseline) { $target = "$target-baseline" }') + expect(installTarget).toContain('$filename = "$App-$target.zip"') + expect(INSTALL_PS1).toContain('$App = "altimate"') + expect(INSTALL_PS1).toContain('$arch = "x64"') + expect(installTarget).not.toMatch(/\$filename\s*=.*\$Version/) + expect(installTarget).not.toMatch(/\$filename\s*=.*\$specificVersion/) + }) + + test("normalizes pinned versions and probes the release tag before downloading", () => { + const versionBlock = scriptBlock("# Resolve version (once)", "# Skip if the requested version") + expect(versionBlock).toContain("$Version = $Version -replace '^v', ''") + expect(versionBlock).toContain('$specificVersion = $Version') + expect(versionBlock).toContain('"https://github.com/AltimateAI/altimate-code/releases/tag/v$Version"') + expect(versionBlock).toContain("-Method Head") + expect(versionBlock).toContain("Release v$Version not found") + expect(versionBlock).toContain("exit 1") + }) + + test("latest-version resolution requires a nonblank GitHub release tag", () => { + const versionBlock = scriptBlock("# Resolve version (once)", "# Skip if the requested version") + expect(versionBlock).toContain("[string]::IsNullOrWhiteSpace($Version)") + expect(versionBlock).toContain('"User-Agent" = "altimate-install"') + expect(versionBlock).toContain("$specificVersion = ($rel.tag_name -replace '^v', '')") + expect(versionBlock).toContain("[string]::IsNullOrWhiteSpace($specificVersion)") + expect(versionBlock.match(/Failed to fetch version information/g)?.length).toBeGreaterThanOrEqual(2) + }) +}) + +describe("PR #930 install.ps1 download and archive safety", () => { + // BUG: install.ps1 currently documents that SHA256/signature verification is deferred + // and relies only on HTTPS. Release assets should be verified before extraction. + test.todo("verifies downloaded archive integrity with SHA256 or a signature before extraction", () => {}) + + test("fails curl.exe downloads on HTTP errors and checks curl exit status", () => { + const installTarget = scriptBlock("function Install-Target", "$needsBaseline") + expect(installTarget).toContain("Get-Command curl.exe -ErrorAction SilentlyContinue") + expect(installTarget).toContain('& $curl.Source "-#SfLo" $zipPath $url') + expect(installTarget).toContain('$LASTEXITCODE -ne 0') + expect(installTarget).toContain('throw "curl.exe failed downloading $url (exit $LASTEXITCODE)"') + }) + + test("falls back to Invoke-WebRequest with an explicit output file", () => { + const installTarget = scriptBlock("function Install-Target", "$needsBaseline") + expect(installTarget).toContain("Invoke-WebRequest -Uri $url -OutFile $zipPath -UseBasicParsing") + expect(installTarget).not.toMatch(/Invoke-Expression|iex/i) + }) + + test("extracts only from a per-process temp directory and cleans it in finally", () => { + const installTarget = scriptBlock("function Install-Target", "$needsBaseline") + expect(installTarget).toContain('Join-Path ([System.IO.Path]::GetTempPath()) "altimate_install_$PID"') + expect(installTarget).toContain("New-Item -ItemType Directory -Force -Path $tmpDir") + expect(installTarget).toContain("} finally {") + expect(installTarget).toContain("Remove-Item -Recurse -Force -Path $tmpDir -ErrorAction SilentlyContinue") + }) + + test("requires the extracted archive to contain the expected executable name", () => { + const installTarget = scriptBlock("function Install-Target", "$needsBaseline") + expect(installTarget).toContain("$extracted = Join-Path $tmpDir $BinaryName") + expect(installTarget).toContain("if (-not (Test-Path $extracted))") + expect(installTarget).toContain('throw "Archive did not contain $BinaryName"') + expect(installTarget).toContain("Move-Item -Force -Path $extracted -Destination $InstalledBinary") + }) +}) + +describe("PR #930 install.ps1 error handling and idempotency", () => { + test("sets terminating error behavior and wraps network probes in try/catch", () => { + expect(INSTALL_PS1).toContain('$ErrorActionPreference = "Stop"') + expect(INSTALL_PS1.match(/\btry\s*\{/g)?.length).toBeGreaterThanOrEqual(4) + expect(INSTALL_PS1.match(/\bcatch\s*\{/g)?.length).toBeGreaterThanOrEqual(4) + expect(INSTALL_PS1.match(/exit 1/g)?.length).toBeGreaterThanOrEqual(3) + }) + + test("skips reinstall when altimate or altimate-code already reports the target version", () => { + const idempotencyBlock = scriptBlock("# Skip if the requested version is already installed", "# Download + extract") + expect(idempotencyBlock).toContain('@("altimate", "altimate-code")') + expect(idempotencyBlock).toContain("Get-Command $name -ErrorAction SilentlyContinue") + expect(idempotencyBlock).toContain("& $probe --version") + expect(idempotencyBlock).toContain("$installedVersion -eq $specificVersion") + expect(idempotencyBlock).toContain("exit 0") + }) + + test("updates a locked executable by moving the old binary aside before replacement", () => { + const installTarget = scriptBlock("function Install-Target", "$needsBaseline") + expect(installTarget).toContain("if (Test-Path $InstalledBinary)") + expect(installTarget).toContain('$stale = "$InstalledBinary.old"') + expect(installTarget).toContain("Move-Item -Force -Path $InstalledBinary -Destination $stale") + expect(installTarget).toContain('Remove-Item -Force "$InstalledBinary.old" -ErrorAction SilentlyContinue') + }) + + test("does not expose common secret-bearing environment variables or literals", () => { + expect(INSTALL_PS1).not.toMatch(/\$env:(?:GITHUB_TOKEN|NPM_TOKEN|OPENAI_API_KEY|ANTHROPIC_API_KEY|AWS_SECRET_ACCESS_KEY)\b/) + expect(INSTALL_PS1).not.toMatch(/(?:password|passwd|secret|token|api[_-]?key)\s*=\s*["'][^"']{8,}["']/i) + expect(INSTALL_PS1).not.toMatch(/-----BEGIN (?:RSA |DSA |EC |OPENSSH )?PRIVATE KEY-----/) + }) +}) + +describe("PR #930 install.ps1 PATH manipulation safety", () => { + test("writes user PATH through the registry instead of setx", () => { + const pathBlock = scriptBlock("# PATH (user scope", "# GitHub Actions") + expect(pathBlock).toContain('[Microsoft.Win32.Registry]::CurrentUser.OpenSubKey("Environment", $true)') + expect(pathBlock).toContain('[Microsoft.Win32.RegistryValueOptions]::DoNotExpandEnvironmentNames') + expect(pathBlock).toContain('[Microsoft.Win32.RegistryValueKind]::ExpandString') + expect(pathBlock).not.toMatch(/^\s*setx\b/im) + }) + + test("prepends the install directory only when it is not already present", () => { + const pathBlock = scriptBlock("# PATH (user scope", "# GitHub Actions") + expect(pathBlock).toContain("$entries = @($userPath -split ';' | Where-Object { $_ -ne \"\" })") + expect(pathBlock).toContain("if ($entries -notcontains $InstallDir)") + expect(pathBlock).toContain("$newPath = (@($InstallDir) + $entries) -join ';'") + expect(pathBlock).toContain('$env:Path = "$InstallDir;$env:Path"') + }) + + test("honors -NoPathUpdate and gives the manual install directory instead", () => { + const pathBlock = scriptBlock("# PATH (user scope", "# GitHub Actions") + expect(pathBlock).toContain("if (-not $NoPathUpdate)") + expect(pathBlock).toContain('Write-Muted "Skipped PATH modification (--no-modify-path). Add manually: $InstallDir"') + }) + + test("broadcasts environment changes with a bounded timeout after PATH edits", () => { + const pathBlock = scriptBlock("# PATH (user scope", "# GitHub Actions") + expect(pathBlock).toContain("function Publish-EnvChange") + expect(pathBlock).toContain("$WM_SETTINGCHANGE = 0x1a") + expect(pathBlock).toContain('"Environment", 2, 5000') + expect(pathBlock).toContain("Publish-EnvChange") + }) +}) + +describe("PR #930 installation/index.ts Windows upgrade wiring", () => { + test("probes install.ps1 with HEAD and a bounded timeout before invoking PowerShell", () => { + const upgradeBlock = upgradePowershellBlock() + expect(INSTALLATION_TS).toContain('const UPGRADE_INSTALL_PS_URL = "https://www.altimate.sh/install.ps1"') + expect(INSTALLATION_TS).toContain("const UPGRADE_FETCH_TIMEOUT_MS = 15_000") + expect(upgradeBlock).toContain('method: "HEAD"') + expect(upgradeBlock).toContain("AbortSignal.timeout(UPGRADE_FETCH_TIMEOUT_MS)") + expect(upgradeBlock).toContain("if (!res.ok) throw new Error(`HTTP ${res.status} ${res.statusText}`)") + }) + + test("passes the target version through env VERSION and avoids npm/node for win32 curl upgrades", () => { + const upgradeBlock = upgradePowershellBlock() + expect(upgradeBlock).toContain('["powershell", "-NoProfile", "-ExecutionPolicy", "Bypass", "-Command"') + expect(upgradeBlock).toContain("VERSION: target") + expect(upgradeBlock).toContain("nothrow: true") + expect(upgradeBlock).not.toMatch(/\bnpm\b|\bnode\b/) + }) + + test("includes a manual recovery message that does not leak environment data", () => { + const upgradeBlock = upgradePowershellBlock() + const errorMessage = upgradeBlock.slice( + upgradeBlock.indexOf("throw new Error("), + upgradeBlock.indexOf("return Process.run"), + ) + expect(upgradeBlock).toContain("Could not download install script from ${UPGRADE_INSTALL_PS_URL}") + expect(upgradeBlock).toContain('powershell -c "irm ${UPGRADE_INSTALL_PS_URL} | iex"') + expect(upgradeBlock).toContain("https://github.com/AltimateAI/altimate-code/releases/latest") + expect(errorMessage).not.toMatch(/process\.env|OPENAI_API_KEY|GITHUB_TOKEN|NPM_TOKEN/) + }) +}) diff --git a/packages/opencode/test/release-validation/windows-installer-930.test.ts b/packages/opencode/test/release-validation/windows-installer-930.test.ts new file mode 100644 index 000000000..37298f63a --- /dev/null +++ b/packages/opencode/test/release-validation/windows-installer-930.test.ts @@ -0,0 +1,428 @@ +/** + * Regression tests for PR #930 — Windows PowerShell installer (install.ps1) + + * the native-Windows upgrade wiring in src/installation/index.ts. + * + * The existing test (test/install/windows-install.test.ts) pins the parity + * contract via substring/regex matching on source text. This file goes a layer + * deeper for the TypeScript wiring: it drives the real `Installation.upgrade` + * dispatch with `Process.run`/`Process.spawn`, `globalThis.fetch`, the lazy + * `Telemetry` import, and `process.platform` stubbed, asserting the actual + * branch behavior rather than a regex on the source. + * + * Mocking strategy: the repo deliberately avoids `mock.module` (it is + * process-global in bun and clobbers modules for the whole run — see + * test/cli/serve-upgrade-check.test.ts). Instead we reassign the writable + * members of the shared `Process` / `Telemetry` namespace objects (both the + * test and src import the same module instance, so the override is observed by + * `Installation.upgrade`), swap `globalThis.fetch`, and redefine + * `process.platform` — always restoring originals in `finally` + a defensive + * `afterEach` so no other test in the suite is affected. + * + * `upgradePowershell` / `upgradeCurl` are module-internal (not exported), so the + * win32 dispatch, the HEAD-probe error surface, and the result-shape contract + * are exercised through the public `Installation.upgrade("curl", target)`. + * + * install.ps1 itself is exercised by static analysis of the script text: + * `pwsh` is unavailable in this environment, so the PowerShell behaviors + * (gaps 4–8: baseline selection, already-installed skip, PATH idempotency, + * GITHUB_PATH emission, missing-exe failure + cleanup) are asserted against the + * script source. The executable Pester equivalents live in + * test/windows/install.Tests.ps1, run on windows-latest in CI. + */ +import { describe, test, expect, afterEach } from "bun:test" +import { readFileSync } from "node:fs" +import { join } from "node:path" +import { Readable } from "node:stream" +import { Process } from "../../src/util/process" +import { Installation } from "../../src/installation" +import { Telemetry } from "../../src/telemetry" + +const REPO_ROOT = join(import.meta.dir, "../../../..") +const PS1 = readFileSync(join(REPO_ROOT, "install.ps1"), "utf-8") + +// --------------------------------------------------------------------------- +// Stub bookkeeping — every test restores these; afterEach is the safety net. +// --------------------------------------------------------------------------- +const ORIG = { + run: Process.run, + spawn: Process.spawn, + text: Process.text, + track: Telemetry.track, + fetch: globalThis.fetch, + platform: Object.getOwnPropertyDescriptor(process, "platform")!, +} + +function restoreAll() { + Process.run = ORIG.run + Process.spawn = ORIG.spawn + Process.text = ORIG.text + Telemetry.track = ORIG.track + globalThis.fetch = ORIG.fetch + Object.defineProperty(process, "platform", ORIG.platform) +} + +afterEach(restoreAll) + +function setPlatform(value: string) { + Object.defineProperty(process, "platform", { value, configurable: true }) +} + +// `process.execPath --version` is awaited at the tail of a successful upgrade(); +// stub it to a no-op so we never actually spawn the host binary. +function stubExecVersionProbe() { + Process.text = async () => ({ code: 0, stdout: Buffer.alloc(0), stderr: Buffer.alloc(0), text: "" }) +} + +// A fake bash child for the upgradeCurl (non-win32) path: Process.spawn returns +// a ChildProcess-like object with piped streams and an `exited` promise. +function fakeBashChild(opts: { stdout?: string; stderr?: string; code?: number } = {}) { + let piped = "" + return { + stdin: { end: (s: string) => { piped = s } }, + stdout: Readable.from([Buffer.from(opts.stdout ?? "ok")]), + stderr: Readable.from([Buffer.from(opts.stderr ?? "")]), + exited: Promise.resolve(opts.code ?? 0), + get _piped() { + return piped + }, + } as any +} + +// =========================================================================== +// GAP 1 — upgrade() dispatches win32 curl-installs to PowerShell, others to bash +// =========================================================================== +describe("upgrade('curl', target) — platform dispatch", () => { + test("win32 routes to upgradePowershell: powershell + irm install.ps1 | iex, VERSION=target", async () => { + const runCalls: Array<{ cmd: string[]; opts: any }> = [] + const fetchCalls: Array<{ url: string; method?: string }> = [] + Process.run = async (cmd: string[], opts: any) => { + runCalls.push({ cmd, opts }) + return { code: 0, stdout: Buffer.from("ok"), stderr: Buffer.alloc(0) } + } + Telemetry.track = () => {} + stubExecVersionProbe() + // HEAD probe must succeed (200) for the PS installer to run. + // @ts-expect-error stub + globalThis.fetch = async (url: any, init: any) => { + fetchCalls.push({ url: String(url), method: init?.method }) + return { ok: true, status: 200, statusText: "OK" } as any + } + setPlatform("win32") + + await Installation.upgrade("curl", "1.2.3") + + // Exactly one Process.run, and it is the PowerShell installer. + expect(runCalls).toHaveLength(1) + const { cmd, opts } = runCalls[0] + expect(cmd[0]).toBe("powershell") + const command = cmd.join(" ") + expect(command).toContain("irm") + expect(command).toContain("install.ps1 | iex") + // The target version is piped to the installer via $env:VERSION. + expect(opts.env.VERSION).toBe("1.2.3") + // nothrow so upgrade() can inspect result.code itself. + expect(opts.nothrow).toBe(true) + + // The HEAD probe hit the .ps1 endpoint, not the bash install endpoint. + expect(fetchCalls).toHaveLength(1) + expect(fetchCalls[0].url).toBe("https://www.altimate.sh/install.ps1") + expect(fetchCalls[0].method).toBe("HEAD") + }) + + test("non-win32 routes to upgradeCurl: Process.spawn(['bash']) + fetch to /install (no .ps1)", async () => { + const spawnCalls: Array<{ cmd: string[] }> = [] + const fetchUrls: string[] = [] + let runCalled = false + Process.run = async () => { + runCalled = true + return { code: 0, stdout: Buffer.alloc(0), stderr: Buffer.alloc(0) } + } + Process.spawn = (cmd: string[]) => { + spawnCalls.push({ cmd }) + return fakeBashChild({ stdout: "done", code: 0 }) + } + Telemetry.track = () => {} + stubExecVersionProbe() + // @ts-expect-error stub + globalThis.fetch = async (url: any) => { + fetchUrls.push(String(url)) + return { ok: true, status: 200, statusText: "OK", text: async () => "echo install" } as any + } + setPlatform("linux") + + await Installation.upgrade("curl", "1.2.3") + + expect(spawnCalls).toHaveLength(1) + expect(spawnCalls[0].cmd[0]).toBe("bash") + // The bash installer is fetched from UPGRADE_INSTALL_URL (the .ps1 host is never touched). + expect(fetchUrls).toContain("https://www.altimate.sh/install") + expect(fetchUrls.some((u) => u.endsWith(".ps1"))).toBe(false) + expect(runCalled).toBe(false) + }) + + test("darwin (any non-win32) also uses bash, never powershell", async () => { + const spawnCalls: string[][] = [] + Process.run = async () => { + throw new Error("Process.run must not be used on the bash path") + } + Process.spawn = (cmd: string[]) => { + spawnCalls.push(cmd) + return fakeBashChild({ code: 0 }) + } + Telemetry.track = () => {} + stubExecVersionProbe() + // @ts-expect-error stub + globalThis.fetch = async () => ({ ok: true, status: 200, statusText: "OK", text: async () => "echo install" }) as any + setPlatform("darwin") + + await Installation.upgrade("curl", "2.0.0") + expect(spawnCalls[0][0]).toBe("bash") + }) +}) + +// =========================================================================== +// GAP 2 — upgradePowershell surfaces a friendly error when the HEAD probe fails +// (driven through upgrade('curl', ...) on win32) +// =========================================================================== +describe("upgradePowershell — HEAD probe failure surfaces a friendly error, never spawns powershell", () => { + test("HTTP !ok (503) → Error names URL + 'HTTP 503' + 'irm' recovery; Process.run NOT called", async () => { + let runCalled = false + let tracked = 0 + Process.run = async () => { + runCalled = true + return { code: 0, stdout: Buffer.alloc(0), stderr: Buffer.alloc(0) } + } + Telemetry.track = () => { + tracked++ + } + stubExecVersionProbe() + // @ts-expect-error stub + globalThis.fetch = async () => ({ ok: false, status: 503, statusText: "Service Unavailable" }) as any + setPlatform("win32") + + let err: unknown + try { + await Installation.upgrade("curl", "1.2.3") + } catch (e) { + err = e + } + + expect(err).toBeInstanceOf(Error) + const msg = (err as Error).message + expect(msg).toContain("https://www.altimate.sh/install.ps1") + expect(msg).toContain("HTTP 503") + expect(msg).toContain("Service Unavailable") + expect(msg).toContain("irm") + // It failed at the probe, before any spawn AND before the telemetry block. + expect(runCalled).toBe(false) + expect(tracked).toBe(0) + }) + + test("AbortSignal.timeout / network reject → Error names the cause; Process.run NOT called", async () => { + let runCalled = false + Process.run = async () => { + runCalled = true + return { code: 0, stdout: Buffer.alloc(0), stderr: Buffer.alloc(0) } + } + Telemetry.track = () => {} + stubExecVersionProbe() + // Mirror what AbortSignal.timeout produces on the real fetch: a TimeoutError. + // @ts-expect-error stub + globalThis.fetch = async () => { + throw new DOMException("The operation timed out.", "TimeoutError") + } + setPlatform("win32") + + let err: unknown + try { + await Installation.upgrade("curl", "1.2.3") + } catch (e) { + err = e + } + + expect(err).toBeInstanceOf(Error) + const msg = (err as Error).message + expect(msg).toContain("https://www.altimate.sh/install.ps1") + expect(msg).toContain("The operation timed out.") + expect(msg).toContain("irm") + expect(runCalled).toBe(false) + }) +}) + +// =========================================================================== +// GAP 3 — upgradePowershell returns the {code, stdout:Buffer, stderr:Buffer} +// shape that upgrade() consumes (nothrow). Verified through upgrade(). +// =========================================================================== +describe("upgradePowershell result shape is consumed by upgrade()", () => { + test("success: a {code:0, stdout:Buffer, stderr:Buffer} result completes upgrade() cleanly", async () => { + // The result object upgradePowershell returns IS what Process.run returns. + // upgrade() reads result.code (=== 0) and logs result.stderr.toString(). + let observedResult: any + Process.run = async () => { + observedResult = { code: 0, stdout: Buffer.from("ok"), stderr: Buffer.alloc(0) } + return observedResult + } + let trackedStatus = "" + Telemetry.track = (e: any) => { + trackedStatus = e.status + } + stubExecVersionProbe() + // @ts-expect-error stub + globalThis.fetch = async () => ({ ok: true, status: 200, statusText: "OK" }) as any + setPlatform("win32") + + // Resolves (no throw) → upgrade() accepted the shape. + await expect(Installation.upgrade("curl", "3.0.0")).resolves.toBeUndefined() + + // Shape contract the caller depends on: numeric code, Buffer stderr. + expect(typeof observedResult.code).toBe("number") + expect(Buffer.isBuffer(observedResult.stderr)).toBe(true) + // The exact call the caller makes against stderr must work. + expect(observedResult.stderr.toString("utf8")).toBe("") + expect(trackedStatus).toBe("success") + }) + + test("failure: code:1 with stderr Buffer → UpgradeFailedError(stderr) + telemetry status 'error'", async () => { + Process.run = async () => ({ + code: 1, + stdout: Buffer.alloc(0), + stderr: Buffer.from("powershell not found"), + }) + const tracked: any[] = [] + Telemetry.track = (e: any) => tracked.push(e) + stubExecVersionProbe() + // @ts-expect-error stub + globalThis.fetch = async () => ({ ok: true, status: 200, statusText: "OK" }) as any + setPlatform("win32") + + let err: unknown + try { + await Installation.upgrade("curl", "1.2.3") + } catch (e) { + err = e + } + + // The caller did result.stderr.toString('utf8') and wrapped it. + expect(Installation.UpgradeFailedError.isInstance(err)).toBe(true) + expect((err as any).data.stderr).toBe("powershell not found") + + // An error telemetry event was emitted carrying that 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).toContain("powershell not found") + }) +}) + +// =========================================================================== +// install.ps1 static-analysis contracts (gaps 4–8). +// pwsh is unavailable here; the executable Pester tests are in +// test/windows/install.Tests.ps1 (CI windows-latest). These assert that the +// script *encodes* the required behavior. +// =========================================================================== + +// GAP 4 — baseline archive selection (AVX2 absent / -ForceBaseline) +describe("install.ps1 — baseline vs AVX2 archive selection (static)", () => { + test("Install-Target appends -baseline to the target only when $Baseline", () => { + // $target = "windows-$arch"; if ($Baseline) { $target = "$target-baseline" } + expect(PS1).toMatch(/\$target\s*=\s*"windows-\$arch"/) + expect(PS1).toMatch(/if\s*\(\$Baseline\)\s*\{\s*\$target\s*=\s*"\$target-baseline"\s*\}/) + // The downloaded filename is -.zip → windows-x64.zip / windows-x64-baseline.zip. + expect(PS1).toContain('$filename = "$App-$target.zip"') + }) + + test("needsBaseline is driven by -ForceBaseline OR absence of AVX2", () => { + // $needsBaseline = $ForceBaseline -or (-not (Test-Avx2)) + expect(PS1).toMatch(/\$needsBaseline\s*=\s*\$ForceBaseline\s*-or\s*\(-not\s*\(Test-Avx2\)\)/) + expect(PS1).toContain("Install-Target -Baseline:$needsBaseline") + // Test-Avx2 uses the documented Win32 feature id 40 (PF_AVX2_INSTRUCTIONS_AVAILABLE). + expect(PS1).toContain("IsProcessorFeaturePresent(40)") + }) + + test("AVX2 detection failure falls back to baseline (returns $false on error)", () => { + // The catch in Test-Avx2 returns $false → needsBaseline becomes true. + expect(PS1).toMatch(/function Test-Avx2[\s\S]*?catch\s*\{[\s\S]*?return\s+\$false[\s\S]*?\}/) + }) +}) + +// GAP 5 — already-installed skip exits 0 without downloading +describe("install.ps1 — already-installed skip (static)", () => { + test("matching installed version prints 'already installed' and exits 0 before Install-Target", () => { + expect(PS1).toMatch(/if\s*\(\$installedVersion\s*-eq\s*\$specificVersion\)\s*\{/) + expect(PS1).toContain("already installed") + // The skip block exits 0. + const skipBlock = PS1.slice(PS1.indexOf("$installedVersion -eq $specificVersion")) + expect(skipBlock).toMatch(/already installed[\s\S]*?exit 0/) + }) + + test("the skip check precedes the download (Install-Target call comes later in the file)", () => { + const skipIdx = PS1.indexOf("already installed") + const installCallIdx = PS1.indexOf("Install-Target -Baseline:$needsBaseline") + expect(skipIdx).toBeGreaterThan(-1) + expect(installCallIdx).toBeGreaterThan(-1) + // Early-exit skip is positioned before the download is ever invoked. + expect(skipIdx).toBeLessThan(installCallIdx) + }) +}) + +// GAP 6 — PATH update is idempotent, prepends InstallDir, -NoPathUpdate skips +describe("install.ps1 — PATH update idempotency + prepend + opt-out (static)", () => { + test("InstallDir is prepended to the front of the user Path", () => { + // $newPath = (@($InstallDir) + $entries) -join ';' → InstallDir is first. + expect(PS1).toMatch(/\$newPath\s*=\s*\(@\(\$InstallDir\)\s*\+\s*\$entries\)\s*-join\s*';'/) + expect(PS1).toMatch(/SetValue\("Path",\s*\$newPath/) + }) + + test("the registry write is guarded by a not-contains check (idempotent: no duplicate, no rewrite)", () => { + // if ($entries -notcontains $InstallDir) { ...SetValue... } → second run is a no-op. + expect(PS1).toMatch(/if\s*\(\$entries\s*-notcontains\s*\$InstallDir\)\s*\{/) + const guardIdx = PS1.indexOf("$entries -notcontains $InstallDir") + const setIdx = PS1.indexOf('SetValue("Path"') + expect(guardIdx).toBeGreaterThan(-1) + expect(setIdx).toBeGreaterThan(guardIdx) + }) + + test("-NoPathUpdate skips the registry write entirely and prints the skip notice", () => { + expect(PS1).toMatch(/if\s*\(-not\s*\$NoPathUpdate\)\s*\{/) + expect(PS1).toContain("Skipped PATH modification") + }) +}) + +// GAP 7 — GITHUB_PATH emission only under GitHub Actions +describe("install.ps1 — GITHUB_PATH emission gated on GitHub Actions (static)", () => { + test("Add-Content to $env:GITHUB_PATH only when GITHUB_ACTIONS == 'true' AND GITHUB_PATH is set", () => { + expect(PS1).toMatch(/if\s*\(\$env:GITHUB_ACTIONS\s*-eq\s*"true"\s*-and\s*\$env:GITHUB_PATH\)\s*\{/) + expect(PS1).toMatch(/Add-Content\s+-Path\s+\$env:GITHUB_PATH\s+-Value\s+\$InstallDir/) + }) + + test("the Add-Content is inside the guard (not emitted unconditionally)", () => { + const guardIdx = PS1.indexOf('$env:GITHUB_ACTIONS -eq "true"') + const addIdx = PS1.indexOf("Add-Content -Path $env:GITHUB_PATH") + expect(guardIdx).toBeGreaterThan(-1) + expect(addIdx).toBeGreaterThan(guardIdx) + }) +}) + +// GAP 8 — fails clearly when archive lacks altimate.exe; temp dir cleaned up +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"/) + }) + + test("the temp dir (altimate_install_$PID) is removed in a finally block", () => { + expect(PS1).toContain('"altimate_install_$PID"') + // finally { Remove-Item -Recurse -Force -Path $tmpDir ... } → cleanup runs even on throw. + expect(PS1).toMatch(/\}\s*finally\s*\{[\s\S]*?Remove-Item\s+-Recurse\s+-Force\s+-Path\s+\$tmpDir/) + }) + + test("the missing-exe throw is positioned inside the try whose finally cleans up", () => { + const throwIdx = PS1.indexOf("Archive did not contain") + const finallyIdx = PS1.indexOf("} finally {") + const cleanupIdx = PS1.indexOf("Remove-Item -Recurse -Force -Path $tmpDir") + expect(throwIdx).toBeGreaterThan(-1) + expect(finallyIdx).toBeGreaterThan(throwIdx) + expect(cleanupIdx).toBeGreaterThan(finallyIdx) + }) +})