diff --git a/packages/core/src/flag/flag.ts b/packages/core/src/flag/flag.ts index 92aae8f55..c9f3b6503 100644 --- a/packages/core/src/flag/flag.ts +++ b/packages/core/src/flag/flag.ts @@ -41,8 +41,7 @@ export namespace Flag { OPENCODE_DISABLE_CLAUDE_CODE || truthy("OPENCODE_DISABLE_CLAUDE_CODE_PROMPT") export const OPENCODE_DISABLE_CLAUDE_CODE_SKILLS = OPENCODE_DISABLE_CLAUDE_CODE || truthy("OPENCODE_DISABLE_CLAUDE_CODE_SKILLS") - export const OPENCODE_DISABLE_EXTERNAL_SKILLS = - OPENCODE_DISABLE_CLAUDE_CODE_SKILLS || truthy("OPENCODE_DISABLE_EXTERNAL_SKILLS") + export const OPENCODE_DISABLE_EXTERNAL_SKILLS = truthy("OPENCODE_DISABLE_EXTERNAL_SKILLS") export declare const OPENCODE_DISABLE_PROJECT_CONFIG: boolean export const OPENCODE_FAKE_VCS = process.env["OPENCODE_FAKE_VCS"] export declare const OPENCODE_CLIENT: string diff --git a/packages/core/src/global.ts b/packages/core/src/global.ts index dbb5ccc98..e06a89516 100644 --- a/packages/core/src/global.ts +++ b/packages/core/src/global.ts @@ -11,6 +11,7 @@ const data = path.join(xdgData!, app) const cache = path.join(xdgCache!, app) const config = path.join(xdgConfig!, app) const state = path.join(xdgState!, app) +const tmp = path.join(state, "tmp") const paths = { get home() { @@ -22,12 +23,32 @@ const paths = { cache, config, state, + tmp, } export const Path = paths Flock.setGlobal({ state }) +async function ensurePrivateDirectory(dir: string) { + try { + const stat = await fs.lstat(dir) + if (stat.isSymbolicLink() || !stat.isDirectory()) throw new Error(`${dir} is not a directory`) + } catch (error: any) { + if (error?.code !== "ENOENT") throw error + await fs.mkdir(dir, { recursive: true, mode: 0o700 }) + } + + const stat = await fs.lstat(dir) + if (stat.isSymbolicLink() || !stat.isDirectory()) throw new Error(`${dir} is not a private directory`) + + if (process.platform !== "win32") { + const uid = process.getuid?.() + if (uid !== undefined && stat.uid !== uid) throw new Error(`${dir} is not owned by the current user`) + if ((stat.mode & 0o077) !== 0) await fs.chmod(dir, 0o700) + } +} + await Promise.all([ fs.mkdir(Path.data, { recursive: true }), fs.mkdir(Path.config, { recursive: true }), @@ -35,6 +56,7 @@ await Promise.all([ fs.mkdir(Path.log, { recursive: true }), fs.mkdir(Path.bin, { recursive: true }), ]) +await ensurePrivateDirectory(Path.tmp) export class Service extends Context.Service()("@opencode/Global") {} @@ -44,6 +66,7 @@ export interface Interface { readonly cache: string readonly config: string readonly state: string + readonly tmp: string readonly bin: string readonly log: string } @@ -57,6 +80,7 @@ export const layer = Layer.effect( cache: Path.cache, config: Path.config, state: Path.state, + tmp: Path.tmp, bin: Path.bin, log: Path.log, }) diff --git a/packages/core/src/npm.ts b/packages/core/src/npm.ts index 5cb5209af..672315964 100644 --- a/packages/core/src/npm.ts +++ b/packages/core/src/npm.ts @@ -57,6 +57,12 @@ const resolveEntryPoint = (name: string, dir: string): EntryPoint => { } } +const packageName = (pkg: string) => { + const scoped = pkg.match(/^(@[^/]+\/[^@]+)/)?.[1] + if (scoped) return scoped + return pkg.match(/^([^@]+)/)?.[1] ?? pkg +} + interface ArboristNode { name: string path: string @@ -121,6 +127,7 @@ export const layer = Layer.effect( const add = Effect.fn("Npm.add")(function* (pkg: string) { const dir = directory(pkg) + const name = packageName(pkg) // Only validate cached installs when the lockfile is present — bare cache // dirs without package-lock.json mean reify never finished, so skip @@ -153,7 +160,11 @@ export const layer = Layer.effect( const tree = yield* reify({ dir, add: [pkg] }) const first = tree.edgesOut.values().next().value?.to - if (!first) return yield* new InstallFailedError({ add: [pkg], dir }) + if (!first) { + const result = resolveEntryPoint(name, path.join(dir, "node_modules", name)) + if (Option.isSome(result.entrypoint)) return result + return yield* new InstallFailedError({ add: [pkg], dir }) + } return resolveEntryPoint(first.name, first.path) }, Effect.scoped) diff --git a/packages/core/test/fixture/effect-flock-worker.ts b/packages/core/test/fixture/effect-flock-worker.ts index 3dc3ee2c8..2b45664e7 100644 --- a/packages/core/test/fixture/effect-flock-worker.ts +++ b/packages/core/test/fixture/effect-flock-worker.ts @@ -28,6 +28,7 @@ const testGlobal = Layer.succeed( cache: os.tmpdir(), config: os.tmpdir(), state: os.tmpdir(), + tmp: os.tmpdir(), bin: os.tmpdir(), log: os.tmpdir(), }), diff --git a/packages/core/test/global.test.ts b/packages/core/test/global.test.ts index 82f8e5a4b..a9e4ee588 100644 --- a/packages/core/test/global.test.ts +++ b/packages/core/test/global.test.ts @@ -51,6 +51,7 @@ describe("shared Global runtime namespace", () => { expect(paths.cache).toBe(path.join(root.cache, "pawwork")) expect(paths.config).toBe(path.join(root.config, "pawwork")) expect(paths.state).toBe(path.join(root.state, "pawwork")) + expect(paths.tmp).toBe(path.join(root.state, "pawwork", "tmp")) }) test("accepts PawWork variant namespaces", () => { diff --git a/packages/core/test/npm.test.ts b/packages/core/test/npm.test.ts index 3e94a0869..6103fc0f3 100644 --- a/packages/core/test/npm.test.ts +++ b/packages/core/test/npm.test.ts @@ -1,7 +1,12 @@ import fs from "fs/promises" import path from "path" import { describe, expect, test } from "bun:test" +import { NodeFileSystem } from "@effect/platform-node" +import { Effect, Layer, Option } from "effect" +import { AppFileSystem } from "@opencode-ai/core/filesystem" +import { Global } from "@opencode-ai/core/global" import { Npm } from "@opencode-ai/core/npm" +import { EffectFlock } from "@opencode-ai/core/util/effect-flock" import { tmpdir } from "./fixture/tmpdir" const win = process.platform === "win32" @@ -15,6 +20,28 @@ const writePackage = (dir: string, pkg: Record) => }), ) +const npmLayer = (cache: string) => + Npm.layer.pipe( + Layer.provide(EffectFlock.layer), + Layer.provide(AppFileSystem.layer), + Layer.provide( + Layer.succeed( + Global.Service, + Global.Service.of({ + home: cache, + data: path.join(cache, "data"), + cache, + config: path.join(cache, "config"), + state: path.join(cache, "state"), + tmp: path.join(cache, "tmp"), + bin: path.join(cache, "bin"), + log: path.join(cache, "log"), + }), + ), + ), + Layer.provide(NodeFileSystem.layer), + ) + describe("Npm.sanitize", () => { test("keeps normal scoped package specs unchanged", () => { expect(Npm.sanitize("@opencode/acme")).toBe("@opencode/acme") @@ -29,6 +56,30 @@ describe("Npm.sanitize", () => { }) }) +describe("Npm.add", () => { + test("reifies when package cache directory exists without the package installed", async () => { + await using tmp = await tmpdir() + await fs.mkdir(path.join(tmp.path, "fixture-provider")) + await writePackage(path.join(tmp.path, "fixture-provider"), { + name: "fixture-provider", + main: "index.js", + }) + await Bun.write(path.join(tmp.path, "fixture-provider", "index.js"), "export const fixture = true\n") + + const spec = `fixture-provider@file:${path.join(tmp.path, "fixture-provider")}` + await fs.mkdir(path.join(tmp.path, "cache", "packages", Npm.sanitize(spec)), { recursive: true }) + + const effect = Effect.gen(function* () { + const npm = yield* Npm.Service + return yield* npm.add(spec) + }).pipe(Effect.scoped, Effect.provide(npmLayer(path.join(tmp.path, "cache")))) + const entry = await Effect.runPromise(effect) + + expect(Option.isSome(entry.entrypoint)).toBe(true) + expect(Option.getOrThrow(entry.entrypoint)).toContain(path.join("fixture-provider", "index.js")) + }) +}) + describe("Npm.install", () => { test("respects omit from project .npmrc", async () => { await using tmp = await tmpdir() diff --git a/packages/core/test/util/effect-flock.test.ts b/packages/core/test/util/effect-flock.test.ts index 01413e7d6..33b647ce2 100644 --- a/packages/core/test/util/effect-flock.test.ts +++ b/packages/core/test/util/effect-flock.test.ts @@ -86,6 +86,7 @@ const testGlobal = Layer.succeed( cache: os.tmpdir(), config: os.tmpdir(), state: os.tmpdir(), + tmp: os.tmpdir(), bin: os.tmpdir(), log: os.tmpdir(), }), diff --git a/packages/opencode/src/agent/agent.ts b/packages/opencode/src/agent/agent.ts index 2da490309..1eca7ecad 100644 --- a/packages/opencode/src/agent/agent.ts +++ b/packages/opencode/src/agent/agent.ts @@ -19,6 +19,8 @@ import { Plugin } from "@/plugin" import { Effect, Context, Layer } from "effect" import { InstanceState } from "@/effect/instance-state" import { makeRuntime } from "@/effect/run-service" +import { Global } from "@opencode-ai/core/global" +import path from "path" export namespace Agent { export const Info = z @@ -244,20 +246,24 @@ export namespace Agent { item.permission = Permission.merge(item.permission, Permission.fromConfig(value.permission ?? {})) } - // Ensure Truncate.GLOB is allowed unless explicitly configured + // Ensure runtime-owned helper dirs are allowed unless explicitly configured. for (const name in agents) { const agent = agents[name] - const explicit = agent.permission.some((r) => { - if (r.permission !== "external_directory") return false - if (r.action !== "deny") return false - return r.pattern === Truncate.GLOB - }) - if (explicit) continue - - agents[name].permission = Permission.merge( - agents[name].permission, - Permission.fromConfig({ external_directory: { [Truncate.GLOB]: "allow" } }), - ) + const allow: Record = {} + for (const pattern of [Truncate.GLOB, path.join(Global.Path.tmp, "*")]) { + const explicit = agent.permission.some((r) => { + if (r.permission !== "external_directory") return false + if (r.action !== "deny") return false + return r.pattern === pattern + }) + if (!explicit) allow[pattern] = "allow" + } + if (Object.keys(allow).length > 0) { + agents[name].permission = Permission.merge( + agents[name].permission, + Permission.fromConfig({ external_directory: allow }), + ) + } } const get = Effect.fnUntraced(function* (agent: string) { diff --git a/packages/opencode/src/format/index.ts b/packages/opencode/src/format/index.ts index eb6499cea..bc887d645 100644 --- a/packages/opencode/src/format/index.ts +++ b/packages/opencode/src/format/index.ts @@ -126,11 +126,14 @@ export namespace Format { const dir = yield* InstanceState.directory const code = yield* spawner .spawn( - ChildProcess.make(replaced[0]!, replaced.slice(1), { - cwd: dir, - env: item.environment, - extendEnv: true, - }), + ChildProcess.make(replaced[0]!, replaced.slice(1), { + cwd: dir, + env: item.environment, + extendEnv: true, + stdin: "ignore", + stdout: "ignore", + stderr: "ignore", + }), ) .pipe( Effect.flatMap((handle) => handle.exitCode), diff --git a/packages/opencode/src/git/index.ts b/packages/opencode/src/git/index.ts index 677085765..76d5dcf49 100644 --- a/packages/opencode/src/git/index.ts +++ b/packages/opencode/src/git/index.ts @@ -25,6 +25,9 @@ export namespace Git { text: () => "", stdout: Buffer.alloc(0), stderr: Buffer.from(err instanceof Error ? err.message : String(err)), + truncated: false, + stdoutTruncated: false, + stderrTruncated: false, }) satisfies Result export type Kind = "added" | "deleted" | "modified" @@ -46,16 +49,30 @@ export namespace Git { readonly deletions: number } + export type Patch = { + readonly text: string + readonly truncated: boolean + } + + export interface PatchOptions { + readonly context?: number + readonly maxOutputBytes?: number + } + export interface Result { readonly exitCode: number readonly text: () => string readonly stdout: Buffer readonly stderr: Buffer + readonly truncated: boolean + readonly stdoutTruncated: boolean + readonly stderrTruncated: boolean } export interface Options { readonly cwd: string readonly env?: Record + readonly maxOutputBytes?: number } export interface Interface { @@ -77,6 +94,12 @@ export namespace Git { readonly statsUnstaged: (cwd: string) => Effect.Effect readonly statsStaged: (cwd: string) => Effect.Effect readonly statsHead: (cwd: string, ref: string) => Effect.Effect + readonly patch: (cwd: string, ref: string, file: string, options?: PatchOptions) => Effect.Effect + readonly patchUnstaged: (cwd: string, file: string, options?: PatchOptions) => Effect.Effect + readonly patchStaged: (cwd: string, file: string, options?: PatchOptions) => Effect.Effect + readonly patchHead: (cwd: string, ref: string, file: string, options?: PatchOptions) => Effect.Effect + readonly patchUntracked: (cwd: string, file: string, options?: PatchOptions) => Effect.Effect + readonly statUntracked: (cwd: string, file: string) => Effect.Effect } const kind = (code: string): Kind => { @@ -105,15 +128,34 @@ export namespace Git { stderr: "pipe", }) const handle = yield* spawner.spawn(proc) - const [stdout, stderr] = yield* Effect.all( - [Stream.mkString(Stream.decodeText(handle.stdout)), Stream.mkString(Stream.decodeText(handle.stderr))], - { concurrency: 2 }, - ) + const collect = (stream: typeof handle.stdout) => + Stream.runFold( + stream, + () => ({ chunks: [] as Uint8Array[], bytes: 0, truncated: false }), + (acc, chunk) => { + if (opts.maxOutputBytes === undefined) { + acc.chunks.push(chunk) + acc.bytes += chunk.length + return acc + } + const remaining = opts.maxOutputBytes - acc.bytes + if (remaining > 0) acc.chunks.push(remaining >= chunk.length ? chunk : chunk.slice(0, remaining)) + acc.bytes += chunk.length + acc.truncated = acc.truncated || acc.bytes > opts.maxOutputBytes + return acc + }, + ).pipe(Effect.map((x) => ({ buffer: Buffer.concat(x.chunks), truncated: x.truncated }))) + const [stdout, stderr] = yield* Effect.all([collect(handle.stdout), collect(handle.stderr)], { + concurrency: 2, + }) return { exitCode: yield* handle.exitCode, - text: () => stdout, - stdout: Buffer.from(stdout), - stderr: Buffer.from(stderr), + text: () => stdout.buffer.toString("utf8"), + stdout: stdout.buffer, + stderr: stderr.buffer, + truncated: stdout.truncated, + stdoutTruncated: stdout.truncated, + stderrTruncated: stderr.truncated, } satisfies Result }, Effect.scoped, @@ -382,6 +424,80 @@ export namespace Git { }) }) + const patchResult = Effect.fnUntraced(function* (args: string[], cwd: string, options?: PatchOptions) { + const result = yield* run(args, { cwd, maxOutputBytes: options?.maxOutputBytes }) + return { text: result.truncated ? "" : result.text(), truncated: result.truncated } satisfies Patch + }) + + const patch = Effect.fn("Git.patch")(function* (cwd: string, ref: string, file: string, options?: PatchOptions) { + return yield* patchResult( + ["diff", "--patch", "--no-ext-diff", "--no-renames", `--unified=${options?.context ?? 3}`, ref, "--", file], + cwd, + options, + ) + }) + + const patchUnstaged = Effect.fn("Git.patchUnstaged")(function* (cwd: string, file: string, options?: PatchOptions) { + return yield* patchResult( + ["diff", "--patch", "--no-ext-diff", "--no-renames", `--unified=${options?.context ?? 3}`, "--", file], + cwd, + options, + ) + }) + + const patchStaged = Effect.fn("Git.patchStaged")(function* (cwd: string, file: string, options?: PatchOptions) { + return yield* patchResult( + [ + "diff", + "--cached", + "--patch", + "--no-ext-diff", + "--no-renames", + `--unified=${options?.context ?? 3}`, + "--", + file, + ], + cwd, + options, + ) + }) + + const patchHead = Effect.fn("Git.patchHead")(function* (cwd: string, ref: string, file: string, options?: PatchOptions) { + return yield* patchResult( + ["diff", "--patch", "--no-ext-diff", "--no-renames", `--unified=${options?.context ?? 3}`, ref, "HEAD", "--", file], + cwd, + options, + ) + }) + + const patchUntracked = Effect.fn("Git.patchUntracked")(function* (cwd: string, file: string, options?: PatchOptions) { + return yield* patchResult( + ["diff", "--no-index", "--patch", "--no-ext-diff", "--no-renames", `--unified=${options?.context ?? 3}`, "--", "/dev/null", file], + cwd, + options, + ) + }) + + const statUntracked = Effect.fn("Git.statUntracked")(function* (cwd: string, file: string) { + const result = yield* run(["diff", "--no-index", "--numstat", "--", "/dev/null", file], { + cwd, + maxOutputBytes: 4096, + }) + if (result.truncated) return + const match = result + .text() + .trim() + .match(/^(\d+|-)\t(\d+|-)\t/) + if (!match) return + const additions = match[1] === "-" ? 0 : Number.parseInt(match[1] || "0", 10) + const deletions = match[2] === "-" ? 0 : Number.parseInt(match[2] || "0", 10) + return { + file, + additions: Number.isFinite(additions) ? additions : 0, + deletions: Number.isFinite(deletions) ? deletions : 0, + } satisfies Stat + }) + return Service.of({ run, branch, @@ -401,6 +517,12 @@ export namespace Git { statsUnstaged, statsStaged, statsHead, + patch, + patchUnstaged, + patchStaged, + patchHead, + patchUntracked, + statUntracked, }) }), ) diff --git a/packages/opencode/src/mcp/index.ts b/packages/opencode/src/mcp/index.ts index aabd3c42b..71595d808 100644 --- a/packages/opencode/src/mcp/index.ts +++ b/packages/opencode/src/mcp/index.ts @@ -129,6 +129,11 @@ export namespace MCP { const sanitize = (s: string) => s.replace(/[^a-zA-Z0-9_-]/g, "_") + function remoteURL(key: string, value: string) { + if (URL.canParse(value)) return new URL(value) + log.warn("invalid remote mcp url", { key }) + } + // Convert MCP tool definition to AI SDK Tool type function convertMcpTool(mcpTool: MCPToolDef, client: MCPClient, timeout?: number): Tool { const inputSchema = mcpTool.inputSchema @@ -276,6 +281,13 @@ export namespace MCP { ) { const oauthDisabled = mcp.oauth === false const oauthConfig = typeof mcp.oauth === "object" ? mcp.oauth : undefined + const url = remoteURL(key, mcp.url) + if (!url) { + return { + client: undefined as MCPClient | undefined, + status: { status: "failed" as const, error: `Invalid MCP URL for "${key}"` }, + } + } let authProvider: McpOAuthProvider | undefined if (!oauthDisabled) { @@ -299,14 +311,14 @@ export namespace MCP { const transports: Array<{ name: string; transport: TransportWithAuth }> = [ { name: "StreamableHTTP", - transport: new StreamableHTTPClientTransport(new URL(mcp.url), { + transport: new StreamableHTTPClientTransport(url, { authProvider, requestInit: mcp.headers ? { headers: mcp.headers } : undefined, }), }, { name: "SSE", - transport: new SSEClientTransport(new URL(mcp.url), { + transport: new SSEClientTransport(url, { authProvider, requestInit: mcp.headers ? { headers: mcp.headers } : undefined, }), @@ -704,6 +716,8 @@ export namespace MCP { if (!mcpConfig) throw new Error(`MCP server ${mcpName} not found or disabled`) if (mcpConfig.type !== "remote") throw new Error(`MCP server ${mcpName} is not a remote server`) if (mcpConfig.oauth === false) throw new Error(`MCP server ${mcpName} has OAuth explicitly disabled`) + const url = remoteURL(mcpName, mcpConfig.url) + if (!url) throw new Error(`Invalid MCP URL for "${mcpName}"`) // OAuth config is optional - if not provided, we'll use auto-discovery const oauthConfig = typeof mcpConfig.oauth === "object" ? mcpConfig.oauth : undefined @@ -732,7 +746,7 @@ export namespace MCP { }, ) - const transport = new StreamableHTTPClientTransport(new URL(mcpConfig.url), { authProvider }) + const transport = new StreamableHTTPClientTransport(url, { authProvider }) return yield* Effect.tryPromise({ try: () => { diff --git a/packages/opencode/src/project/vcs.ts b/packages/opencode/src/project/vcs.ts index 5b58efb7a..300f5c86b 100644 --- a/packages/opencode/src/project/vcs.ts +++ b/packages/opencode/src/project/vcs.ts @@ -1,11 +1,9 @@ import { Effect, Layer, Context, Stream } from "effect" import { formatPatch, structuredPatch } from "diff" -import path from "path" import { Bus } from "@/bus" import { BusEvent } from "@/bus/bus-event" import { InstanceState } from "@/effect/instance-state" import { makeRuntime } from "@/effect/run-service" -import { AppFileSystem } from "@opencode-ai/core/filesystem" import { FileWatcher } from "@/file/watcher" import { Git } from "@/git" import { Log } from "@opencode-ai/core/util/log" @@ -14,20 +12,12 @@ import z from "zod" export namespace Vcs { const log = Log.create({ service: "vcs" }) + const PATCH_CONTEXT_LINES = 2_147_483_647 + // A single useful patch may consume the whole budget; later files then fall back to empty patches. + const MAX_PATCH_BYTES = 10_000_000 + const MAX_TOTAL_PATCH_BYTES = 10_000_000 - const count = (text: string) => { - if (!text) return 0 - if (!text.endsWith("\n")) return text.split("\n").length - return text.slice(0, -1).split("\n").length - } - - const work = Effect.fnUntraced(function* (fs: AppFileSystem.Interface, cwd: string, file: string) { - const full = path.join(cwd, file) - if (!(yield* fs.exists(full).pipe(Effect.orDie))) return "" - const buf = yield* fs.readFile(full).pipe(Effect.catch(() => Effect.succeed(new Uint8Array()))) - if (Buffer.from(buf).includes(0)) return "" - return Buffer.from(buf).toString("utf8") - }) + const emptyPatch = (file: string) => formatPatch(structuredPatch(file, file, "", "", "", "", { context: 0 })) const nums = (list: Git.Stat[]) => new Map(list.map((item) => [item.file, { additions: item.additions, deletions: item.deletions }] as const)) @@ -40,37 +30,54 @@ export namespace Vcs { return [...out.values()] } - const staged = Effect.fnUntraced(function* (fs: AppFileSystem.Interface, git: Git.Interface, cwd: string) { + type PatchBatch = { total: number; capped: boolean } + + const boundedPatch = Effect.fnUntraced(function* ( + batch: PatchBatch, + item: Git.Item, + patch: Effect.Effect, + ) { + if (batch.capped) return emptyPatch(item.file) + const result = yield* patch + if (result.truncated) { + batch.capped = true + return emptyPatch(item.file) + } + const size = Buffer.byteLength(result.text) + if (batch.total + size > MAX_TOTAL_PATCH_BYTES) { + batch.capped = true + return emptyPatch(item.file) + } + batch.total += size + return result.text || emptyPatch(item.file) + }) + + const staged = Effect.fnUntraced(function* (git: Git.Interface, cwd: string) { const [list, stats] = yield* Effect.all([git.diffStaged(cwd), git.statsStaged(cwd)], { concurrency: 2 }) const statMap = nums(stats) - const base = yield* git.prefix(cwd) - const patch = (file: string, before: string, after: string) => - formatPatch(structuredPatch(file, file, before, after, "", "", { context: Number.MAX_SAFE_INTEGER })) + const batch: PatchBatch = { total: 0, capped: false } const next = yield* Effect.forEach( - list, + list.toSorted((a, b) => a.file.localeCompare(b.file)), (item) => Effect.gen(function* () { - const before = item.status === "added" ? "" : yield* git.show(cwd, "HEAD", item.file, base) - const after = item.status === "deleted" ? "" : yield* git.showIndex(cwd, item.file, base) const stat = statMap.get(item.file) return { file: item.file, - patch: patch(item.file, before, after), - additions: stat?.additions ?? (item.status === "added" ? count(after) : 0), - deletions: stat?.deletions ?? (item.status === "deleted" ? count(before) : 0), + patch: yield* boundedPatch(batch, item, git.patchStaged(cwd, item.file, { + context: PATCH_CONTEXT_LINES, + maxOutputBytes: MAX_PATCH_BYTES, + })), + additions: stat?.additions ?? 0, + deletions: stat?.deletions ?? 0, status: item.status, } satisfies FileDiff }), - { concurrency: 8 }, + { concurrency: 1 }, ) - return next.toSorted((a, b) => a.file.localeCompare(b.file)) + return next }) - const unstaged = Effect.fnUntraced(function* ( - fs: AppFileSystem.Interface, - git: Git.Interface, - cwd: string, - ) { + const unstaged = Effect.fnUntraced(function* (git: Git.Interface, cwd: string) { const [tracked, extra, stats] = yield* Effect.all( [git.diffUnstaged(cwd), git.statusUnstaged(cwd), git.statsUnstaged(cwd)], { concurrency: 3 }, @@ -80,58 +87,52 @@ export namespace Vcs { extra.filter((item) => item.code === "??"), ) const statMap = nums(stats) - const base = yield* git.prefix(cwd) - const patch = (file: string, before: string, after: string) => - formatPatch(structuredPatch(file, file, before, after, "", "", { context: Number.MAX_SAFE_INTEGER })) + const batch: PatchBatch = { total: 0, capped: false } const next = yield* Effect.forEach( - list, + list.toSorted((a, b) => a.file.localeCompare(b.file)), (item) => Effect.gen(function* () { - const before = item.code === "??" ? "" : yield* git.showIndex(cwd, item.file, base) - const after = item.status === "deleted" ? "" : yield* work(fs, cwd, item.file) - const stat = statMap.get(item.file) + const stat = statMap.get(item.file) ?? (item.status === "added" ? yield* git.statUntracked(cwd, item.file) : undefined) + const patch = + item.code === "??" + ? git.patchUntracked(cwd, item.file, { context: PATCH_CONTEXT_LINES, maxOutputBytes: MAX_PATCH_BYTES }) + : git.patchUnstaged(cwd, item.file, { context: PATCH_CONTEXT_LINES, maxOutputBytes: MAX_PATCH_BYTES }) return { file: item.file, - patch: patch(item.file, before, after), - additions: stat?.additions ?? (item.status === "added" ? count(after) : 0), - deletions: stat?.deletions ?? (item.status === "deleted" ? count(before) : 0), + patch: yield* boundedPatch(batch, item, patch), + additions: stat?.additions ?? 0, + deletions: stat?.deletions ?? 0, status: item.status, } satisfies FileDiff }), - { concurrency: 8 }, + { concurrency: 1 }, ) - return next.toSorted((a, b) => a.file.localeCompare(b.file)) + return next }) - const branchHead = Effect.fnUntraced(function* ( - fs: AppFileSystem.Interface, - git: Git.Interface, - cwd: string, - ref: string, - ) { + const branchHead = Effect.fnUntraced(function* (git: Git.Interface, cwd: string, ref: string) { const [list, stats] = yield* Effect.all([git.diffHead(cwd, ref), git.statsHead(cwd, ref)], { concurrency: 2 }) const statMap = nums(stats) - const base = yield* git.prefix(cwd) - const patch = (file: string, before: string, after: string) => - formatPatch(structuredPatch(file, file, before, after, "", "", { context: Number.MAX_SAFE_INTEGER })) + const batch: PatchBatch = { total: 0, capped: false } const next = yield* Effect.forEach( - list, + list.toSorted((a, b) => a.file.localeCompare(b.file)), (item) => Effect.gen(function* () { - const before = item.status === "added" ? "" : yield* git.show(cwd, ref, item.file, base) - const after = item.status === "deleted" ? "" : yield* git.show(cwd, "HEAD", item.file, base) const stat = statMap.get(item.file) return { file: item.file, - patch: patch(item.file, before, after), - additions: stat?.additions ?? (item.status === "added" ? count(after) : 0), - deletions: stat?.deletions ?? (item.status === "deleted" ? count(before) : 0), + patch: yield* boundedPatch(batch, item, git.patchHead(cwd, ref, item.file, { + context: PATCH_CONTEXT_LINES, + maxOutputBytes: MAX_PATCH_BYTES, + })), + additions: stat?.additions ?? 0, + deletions: stat?.deletions ?? 0, status: item.status, } satisfies FileDiff }), - { concurrency: 8 }, + { concurrency: 1 }, ) - return next.toSorted((a, b) => a.file.localeCompare(b.file)) + return next }) export const Mode = z.enum(["unstaged", "staged", "branch"]) @@ -183,10 +184,9 @@ export namespace Vcs { export class Service extends Context.Service()("@opencode/Vcs") {} - export const layer: Layer.Layer = Layer.effect( + export const layer: Layer.Layer = Layer.effect( Service, Effect.gen(function* () { - const fs = yield* AppFileSystem.Service const git = yield* Git.Service const bus = yield* Bus.Service @@ -238,28 +238,24 @@ export namespace Vcs { const value = yield* InstanceState.get(state) if (Instance.project.vcs !== "git") return [] if (mode === "unstaged") { - return yield* unstaged(fs, git, Instance.directory) + return yield* unstaged(git, Instance.directory) } if (mode === "staged") { - return yield* staged(fs, git, Instance.directory) + return yield* staged(git, Instance.directory) } if (!value.root) return [] if (value.current && value.current === value.root.name) return [] const ref = yield* git.mergeBase(Instance.directory, value.root.ref) if (!ref) return [] - return yield* branchHead(fs, git, Instance.directory, ref) + return yield* branchHead(git, Instance.directory, ref) }), }) }), ) - export const defaultLayer = layer.pipe( - Layer.provide(Git.defaultLayer), - Layer.provide(AppFileSystem.defaultLayer), - Layer.provide(Bus.layer), - ) + export const defaultLayer = layer.pipe(Layer.provide(Git.defaultLayer), Layer.provide(Bus.layer)) const { runPromise } = makeRuntime(Service, defaultLayer) diff --git a/packages/opencode/src/provider/error.ts b/packages/opencode/src/provider/error.ts index 68f9513fe..7fb674053 100644 --- a/packages/opencode/src/provider/error.ts +++ b/packages/opencode/src/provider/error.ts @@ -160,6 +160,7 @@ export function parseStreamError(input: unknown): ParsedStreamError | undefined isRetryable: false, responseBody, } + case "server_is_overloaded": case "server_error": return { type: "api_error", diff --git a/packages/opencode/src/provider/transform.ts b/packages/opencode/src/provider/transform.ts index 189d7e610..9401e2b01 100644 --- a/packages/opencode/src/provider/transform.ts +++ b/packages/opencode/src/provider/transform.ts @@ -1,4 +1,4 @@ -import type { ModelMessage } from "ai" +import type { ModelMessage, ToolResultPart } from "ai" import { mergeDeep, unique } from "remeda" import type { JSONSchema7 } from "@ai-sdk/provider" import type { JSONSchema } from "zod/v4/core" @@ -17,6 +17,19 @@ function mimeToModality(mime: string): Modality | undefined { return undefined } +export function sanitizeSurrogates(content: string) { + return content.replace(/[\uD800-\uDBFF](?![\uDC00-\uDFFF])|(? }) { + return Boolean( + part.providerOptions?.anthropic?.signature ?? + part.providerOptions?.anthropic?.redactedData ?? + part.providerOptions?.bedrock?.signature ?? + part.providerOptions?.bedrock?.redactedData, + ) +} + function filterEmptyMessageContent(msg: ModelMessage): ModelMessage | undefined { if (typeof msg.content === "string") { if (msg.content === "") return undefined @@ -24,9 +37,12 @@ function filterEmptyMessageContent(msg: ModelMessage): ModelMessage | undefined } if (!Array.isArray(msg.content)) return msg const filtered = msg.content.filter((part) => { - if (part.type === "text" || part.type === "reasoning") { + if (part.type === "text") { return part.text !== "" } + if (part.type === "reasoning") { + return part.text.trim().length > 0 || hasReasoningMetadata(part) + } return true }) if (filtered.length === 0) return undefined @@ -144,6 +160,66 @@ function normalizeMessages( model: Provider.Model, _options: Record, ): ModelMessage[] { + const sanitizeToolResultOutput = (content: ToolResultPart) => { + if (content.output.type === "text" || content.output.type === "error-text") { + content.output.value = sanitizeSurrogates(content.output.value) + } + if (content.output.type === "content") { + content.output.value = content.output.value.map((item) => { + if (item.type === "text") { + item.text = sanitizeSurrogates(item.text) + } + return item + }) + } + return content + } + + msgs = msgs.map((msg) => { + switch (msg.role) { + case "tool": + if (!Array.isArray(msg.content)) return msg + msg.content = msg.content.map((content) => { + if (content.type === "tool-result") return sanitizeToolResultOutput(content) + return content + }) + return msg + case "system": + if (typeof msg.content === "string") { + msg.content = sanitizeSurrogates(msg.content) + } else if (Array.isArray(msg.content)) { + msg.content = (msg.content as any[]).map((content) => { + if (content.type === "text") content.text = sanitizeSurrogates(content.text) + return content + }) as never + } + return msg + case "user": + if (typeof msg.content === "string") { + msg.content = sanitizeSurrogates(msg.content) + } else { + msg.content = msg.content.map((content) => { + if (content.type === "text") content.text = sanitizeSurrogates(content.text) + return content + }) + } + return msg + case "assistant": + if (typeof msg.content === "string") { + msg.content = sanitizeSurrogates(msg.content) + } else { + msg.content = msg.content.map((content) => { + if (content.type === "text" || content.type === "reasoning") { + content.text = sanitizeSurrogates(content.text) + } + if (content.type === "tool-result") return sanitizeToolResultOutput(content) + return content + }) + } + return msg + } + }) + // Anthropic rejects messages with empty content - filter out empty string messages // and remove empty text/reasoning parts from array content if (model.api.npm === "@ai-sdk/anthropic") { diff --git a/packages/opencode/src/session/message-v2.ts b/packages/opencode/src/session/message-v2.ts index 0100c2357..5a3ab0254 100644 --- a/packages/opencode/src/session/message-v2.ts +++ b/packages/opencode/src/session/message-v2.ts @@ -50,7 +50,7 @@ interface FetchDecompressionError extends Error { path: string } -export const SYNTHETIC_ATTACHMENT_PROMPT = "Attached image(s) from tool result:" +export const SYNTHETIC_ATTACHMENT_PROMPT = "Attached media from tool result:" export const OutputLengthError = NamedError.create("MessageOutputLengthError", z.object({})) export const AbortedError = NamedError.create("MessageAbortedError", z.object({ message: z.string() })) @@ -659,25 +659,25 @@ export const toModelMessagesEffect = Effect.fnUntraced(function* ( const result: UIMessage[] = [] const toolNames = new Set() // Track media from tool results that need to be injected as user messages - // for providers that don't support media in tool results. + // for providers that don't support that media type in tool results. // // OpenAI-compatible APIs only support string content in tool results, so we need - // to extract media and inject as user messages. Other SDKs (anthropic, google, - // bedrock) handle type: "content" with media parts natively. + // to extract media and inject as user messages. Some SDKs only support a subset + // of media in tool results; e.g. Bedrock supports images but not PDFs there. // - // Only apply this workaround if the model actually supports image input - - // otherwise there's no point extracting images. - const supportsMediaInToolResults = (() => { + // Only apply this workaround if the model actually supports that media input - + // otherwise unsupportedParts() will turn it into a user-visible error. + const supportsMediaInToolResult = (attachment: { mime: string }) => { if (model.api.npm === "@ai-sdk/anthropic") return true if (model.api.npm === "@ai-sdk/openai") return true - if (model.api.npm === "@ai-sdk/amazon-bedrock") return true + if (model.api.npm === "@ai-sdk/amazon-bedrock") return attachment.mime.startsWith("image/") if (model.api.npm === "@ai-sdk/google-vertex/anthropic") return true if (model.api.npm === "@ai-sdk/google") { const id = model.api.id.toLowerCase() return id.includes("gemini-3") && !id.includes("gemini-2") } return false - })() + } const toModelOutput = (options: { toolCallId: string; input: unknown; output: unknown }) => { const output = options.output @@ -697,7 +697,7 @@ export const toModelMessagesEffect = Effect.fnUntraced(function* ( return { type: "content", value: [ - { type: "text", text: outputObject.text }, + ...(outputObject.text ? [{ type: "text", text: outputObject.text }] : []), ...attachments.map((attachment) => ({ type: "media", mediaType: attachment.mime, @@ -722,9 +722,8 @@ export const toModelMessagesEffect = Effect.fnUntraced(function* ( role: "user", parts: [], } - result.push(userMessage) for (const part of msg.parts) { - if (part.type === "text" && !part.ignored) + if (part.type === "text" && !part.ignored && part.text !== "") userMessage.parts.push({ type: "text", text: part.text, @@ -759,11 +758,12 @@ export const toModelMessagesEffect = Effect.fnUntraced(function* ( }) } } + if (userMessage.parts.length > 0) result.push(userMessage) } if (msg.info.role === "assistant") { const differentModel = `${model.providerID}/${model.id}` !== `${msg.info.providerID}/${msg.info.modelID}` - const media: Array<{ mime: string; url: string }> = [] + const media: Array<{ mime: string; url: string; filename?: string }> = [] if ( msg.info.error && @@ -779,13 +779,19 @@ export const toModelMessagesEffect = Effect.fnUntraced(function* ( role: "assistant", parts: [], } + const hasAnthropicSignedReasoning = msg.parts.some((part) => { + if (part.type !== "reasoning") return false + return part.metadata?.anthropic?.signature != null + }) for (const part of msg.parts) { - if (part.type === "text") + if (part.type === "text") { + const text = part.text === "" && hasAnthropicSignedReasoning ? " " : part.text assistantMessage.parts.push({ type: "text", - text: part.text, + text, ...(differentModel ? {} : { providerMetadata: part.metadata }), }) + } if (part.type === "step-start") assistantMessage.parts.push({ type: "step-start", @@ -801,11 +807,11 @@ export const toModelMessagesEffect = Effect.fnUntraced(function* ( // For providers that don't support media in tool results, extract media files // (images, PDFs) to be sent as a separate user message const mediaAttachments = attachments.filter((a) => isMedia(a.mime)) - const nonMediaAttachments = attachments.filter((a) => !isMedia(a.mime)) - if (!supportsMediaInToolResults && mediaAttachments.length > 0) { - media.push(...mediaAttachments) + const extractedMedia = mediaAttachments.filter((a) => !supportsMediaInToolResult(a)) + if (extractedMedia.length > 0) { + media.push(...extractedMedia) } - const finalAttachments = supportsMediaInToolResults ? attachments : nonMediaAttachments + const finalAttachments = attachments.filter((a) => !isMedia(a.mime) || supportsMediaInToolResult(a)) const output = finalAttachments.length > 0 @@ -907,6 +913,7 @@ export const toModelMessagesEffect = Effect.fnUntraced(function* ( type: "file" as const, url: attachment.url, mediaType: attachment.mime, + filename: attachment.filename, })), ], }) @@ -1035,12 +1042,39 @@ export function filterCompacted(msgs: Iterable) { completed.add(msg.info.parentID) } result.reverse() + let filtered = result if (tailStartID) { // After reversing to chronological order, drop everything before the retained tail. const idx = result.findIndex((msg) => msg.info.id === tailStartID) - if (idx >= 0) return result.slice(idx) + if (idx >= 0) filtered = result.slice(idx) + } + const compactionIndex = filtered.findLastIndex( + (msg) => + msg.info.role === "user" && + msg.parts.some((item): item is CompactionPart => item.type === "compaction" && item.tail_start_id !== undefined), + ) + const compaction = filtered[compactionIndex] + const part = compaction?.parts.find( + (item): item is CompactionPart => item.type === "compaction" && item.tail_start_id !== undefined, + ) + const summaryIndex = compaction + ? filtered.findIndex( + (msg, index) => + index > compactionIndex && + msg.info.role === "assistant" && + msg.info.summary && + msg.info.parentID === compaction.info.id, + ) + : -1 + const tailIndex = part?.tail_start_id ? filtered.findIndex((msg) => msg.info.id === part.tail_start_id) : -1 + if (tailIndex >= 0 && tailIndex < compactionIndex && summaryIndex > compactionIndex) { + return [ + ...filtered.slice(compactionIndex, summaryIndex + 1), + ...filtered.slice(tailIndex, compactionIndex), + ...filtered.slice(summaryIndex + 1), + ] } - return result + return filtered } export const filterCompactedEffect = Effect.fnUntraced(function* (sessionID: SessionID) { diff --git a/packages/opencode/src/session/prompt.ts b/packages/opencode/src/session/prompt.ts index ab4988c71..b908f06bc 100644 --- a/packages/opencode/src/session/prompt.ts +++ b/packages/opencode/src/session/prompt.ts @@ -268,9 +268,8 @@ export const layer = Layer.effect( // "child runner aborted" from "model returned naturally" without racing ctx.abort.aborted. const interruptedSessions = new Set() const ops = Effect.fn("SessionPrompt.ops")(function* () { - const run = yield* runner() return { - cancel: (sessionID: SessionID) => run.fork(cancel(sessionID)), + cancel: (sessionID: SessionID) => cancel(sessionID), resolvePromptParts: (template: string) => resolvePromptParts(template), prompt: (input: PromptInput) => prompt(input), // Self-cleaning read: each subagent dispatch reads `wasInterrupted` exactly once after @@ -685,9 +684,18 @@ NOTE: At any point in time through this workflow you should feel free to ask the { tool: key, sessionID: ctx.sessionID, callID: opts.toolCallId }, { args }, ) - yield* ctx.ask({ permission: key, metadata: {}, patterns: ["*"], always: ["*"] }) - const result: Awaited>> = yield* Effect.promise(() => - execute(args, opts), + const result: Awaited>> = yield* Effect.gen(function* () { + yield* ctx.ask({ permission: key, metadata: {}, patterns: ["*"], always: ["*"] }) + return yield* Effect.promise(() => execute(args, opts)) + }).pipe( + Effect.withSpan("Tool.execute", { + attributes: { + "tool.name": key, + "tool.call_id": opts.toolCallId, + "session.id": ctx.sessionID, + "message.id": input.processor.message.id, + }, + }), ) yield* plugin.trigger( "tool.execute.after", diff --git a/packages/opencode/src/session/session.ts b/packages/opencode/src/session/session.ts index e3b6e91af..0efb92c1c 100644 --- a/packages/opencode/src/session/session.ts +++ b/packages/opencode/src/session/session.ts @@ -789,12 +789,16 @@ export const layer: Layer.Layer = }) for (const part of msg.parts) { - yield* updatePart({ + const p: MessageV2.Part = { ...part, id: PartID.ascending(), messageID: cloned.id, sessionID: session.id, - }) + } + if (p.type === "compaction" && p.tail_start_id) { + p.tail_start_id = idMap.get(p.tail_start_id) ?? p.tail_start_id + } + yield* updatePart(p) } } return session diff --git a/packages/opencode/src/skill/index.ts b/packages/opencode/src/skill/index.ts index cc8c5c3ff..6e5832e2c 100644 --- a/packages/opencode/src/skill/index.ts +++ b/packages/opencode/src/skill/index.ts @@ -21,7 +21,7 @@ import { Discovery } from "./discovery" export namespace Skill { const log = Log.create({ service: "skill" }) const processWithResourcesPath = process as NodeJS.Process & { resourcesPath?: string } - const EXTERNAL_DIRS = [".agents"] + const AGENTS_EXTERNAL_DIR = ".agents" const EXTERNAL_SKILL_PATTERN = "skills/**/SKILL.md" const BUILTIN_SKILL_PATTERN = "*/SKILL.md" const OPENCODE_SKILL_PATTERN = "{skill,skills}/**/SKILL.md" @@ -176,14 +176,15 @@ export namespace Skill { worktree: string, ) { if (!Flag.OPENCODE_DISABLE_EXTERNAL_SKILLS) { - for (const dir of EXTERNAL_DIRS) { + const externalDirs = [AGENTS_EXTERNAL_DIR] + for (const dir of externalDirs) { const root = path.join(Global.Path.home, dir) if (!(yield* fsys.isDir(root))) continue yield* scan(state, bus, root, EXTERNAL_SKILL_PATTERN, { dot: true, scope: "global" }) } const upDirs = yield* fsys - .up({ targets: EXTERNAL_DIRS, start: directory, stop: worktree }) + .up({ targets: externalDirs, start: directory, stop: worktree }) .pipe(Effect.catch(() => Effect.succeed([] as string[]))) for (const root of upDirs) { diff --git a/packages/opencode/src/tool/agent.ts b/packages/opencode/src/tool/agent.ts index 648b53430..14ef11dd7 100644 --- a/packages/opencode/src/tool/agent.ts +++ b/packages/opencode/src/tool/agent.ts @@ -7,11 +7,12 @@ import { Agent } from "../agent/agent" import type { SessionPrompt } from "../session/prompt" import { Config } from "../config" import { SubagentRun } from "../session/subagent-run" -import { Cause, Effect, Schema } from "effect" +import { Cause, Effect, Exit, Schema } from "effect" import { NotFoundError } from "../storage/db" +import { EffectBridge } from "@/effect" export interface AgentPromptOps { - cancel(sessionID: SessionID): void + cancel(sessionID: SessionID): Effect.Effect resolvePromptParts(template: string): Effect.Effect prompt(input: SessionPrompt.PromptInput): Effect.Effect /** @@ -302,6 +303,9 @@ export const AgentTool = Tool.define( createdByAgentTool: true, subagentType: params.subagent_type, permission: [ + ...(parent.permission ?? []).filter( + (rule) => rule.permission === "external_directory" || rule.action === "deny", + ), // v1 nested-deny: agent is denied unconditionally so a subagent cannot // recursively dispatch its own subagents (#283 non-goal: nested subagents). { @@ -353,11 +357,15 @@ export const AgentTool = Tool.define( }, }) - const onParentAbort = () => ops.cancel(nextSession.id) - ctx.abort.addEventListener("abort", onParentAbort) - yield* Effect.addFinalizer(() => - Effect.sync(() => ctx.abort.removeEventListener("abort", onParentAbort)), - ) + const runCancel = yield* EffectBridge.make() + const cancelChild = ops.cancel(nextSession.id) + let cancelledChild = false + const cancelChildOnce = Effect.suspend(() => { + if (cancelledChild) return Effect.void + cancelledChild = true + return cancelChild + }) + const onParentAbort = () => runCancel.fork(cancelChildOnce) // Pre-aborted short-circuit. If parent already aborted before we got here, skip // ops.prompt entirely and finalize as canceled_by_user with no partial_result. @@ -416,36 +424,54 @@ export const AgentTool = Tool.define( // row that synthesizeOutput would render as a defective success. // - failure: err-finalize runs (best-effort row cleanup), then the original cause // re-raises via failCause so stack/annotations survive. - const parts = yield* ops.resolvePromptParts(params.prompt) - yield* ops - .prompt({ - messageID: MessageID.ascending(), - sessionID: nextSession.id, - model: { modelID: model.modelID, providerID: model.providerID }, - agent: next.name, - tools: { - agent: false, - "enter-worktree": false, - "exit-worktree": false, - ...(canTodo ? {} : { todowrite: false }), - ...Object.fromEntries( - (cfg.experimental?.primary_tools ?? []).map((item) => [item, false]), - ), - }, - parts, - }) - .pipe( - Effect.matchCauseEffect({ - onSuccess: (r) => finalizeAfter({ kind: "ok", r }), - onFailure: (cause) => - Effect.gen(function* () { - yield* finalizeAfter({ kind: "err", error: Cause.squash(cause) }).pipe( - Effect.catchCause(() => Effect.void), - ) - return yield* Effect.failCause(cause) - }), + yield* Effect.acquireUseRelease( + Effect.sync(() => { + ctx.abort.addEventListener("abort", onParentAbort) + }), + () => + Effect.gen(function* () { + const parts = yield* ops.resolvePromptParts(params.prompt) + yield* ops + .prompt({ + messageID: MessageID.ascending(), + sessionID: nextSession.id, + model: { modelID: model.modelID, providerID: model.providerID }, + agent: next.name, + tools: { + agent: false, + "enter-worktree": false, + "exit-worktree": false, + ...(canTodo ? {} : { todowrite: false }), + ...Object.fromEntries( + (cfg.experimental?.primary_tools ?? []).map((item) => [item, false]), + ), + }, + parts, + }) + .pipe( + Effect.matchCauseEffect({ + onSuccess: (r) => finalizeAfter({ kind: "ok", r }), + onFailure: (cause) => + Effect.gen(function* () { + yield* finalizeAfter({ kind: "err", error: Cause.squash(cause) }).pipe( + Effect.catchCause(() => Effect.void), + ) + return yield* Effect.failCause(cause) + }), + }), + ) }), - ) + (_, exit) => + Effect.gen(function* () { + if (Exit.hasInterrupts(exit)) yield* cancelChildOnce + }).pipe( + Effect.ensuring( + Effect.sync(() => { + ctx.abort.removeEventListener("abort", onParentAbort) + }), + ), + ), + ) const finalPart = yield* subagentRun.read(ctx.callID!) return synthesizeOutput(finalPart, nextSession.id) diff --git a/packages/opencode/src/tool/bash.ts b/packages/opencode/src/tool/bash.ts index 6c579cbc7..749ce58cc 100644 --- a/packages/opencode/src/tool/bash.ts +++ b/packages/opencode/src/tool/bash.ts @@ -7,7 +7,7 @@ import DESCRIPTION from "./bash.txt" import { Log } from "../util" import { Instance } from "../project/instance" import { lazy } from "@/util/lazy" -import { Language, type Node } from "web-tree-sitter" +import { Language, type Node, type Tree } from "web-tree-sitter" import { AppFileSystem } from "@opencode-ai/core/filesystem" import { fileURLToPath } from "url" @@ -22,6 +22,8 @@ import { Effect, Stream } from "effect" import { ChildProcess } from "effect/unstable/process" import { ChildProcessSpawner } from "effect/unstable/process/ChildProcessSpawner" import { withoutInternalServerAuthEnv } from "@/util/env" +import { Global } from "@opencode-ai/core/global" +import { resolveExternalPathForPermission } from "./external-directory" const MAX_METADATA_LENGTH = 30_000 const DEFAULT_TIMEOUT = Flag.OPENCODE_EXPERIMENTAL_BASH_DEFAULT_TIMEOUT_MS || 2 * 60 * 1000 @@ -253,7 +255,7 @@ function tail(text: string, maxLines: number, maxBytes: number) { const parse = Effect.fn("BashTool.parse")(function* (command: string, ps: boolean) { const tree = yield* Effect.promise(() => parser().then((p) => (ps ? p.ps : p.bash).parse(command))) if (!tree) throw new Error("Failed to parse command") - return tree.rootNode + return tree }) const ask = Effect.fn("BashTool.ask")(function* (ctx: Tool.Context, scan: Scan) { @@ -343,7 +345,11 @@ export const BashTool = Tool.define( return AppFileSystem.normalizePath(file) }) - const resolvePath = Effect.fn("BashTool.resolvePath")(function* (text: string, root: string, shell: string) { + const resolveExecutionPath = Effect.fn("BashTool.resolveExecutionPath")(function* ( + text: string, + root: string, + shell: string, + ) { if (process.platform === "win32") { if (Shell.posix(shell) && text.startsWith("/") && AppFileSystem.windowsPath(text) === text) { const file = yield* cygpath(shell, text) @@ -351,7 +357,22 @@ export const BashTool = Tool.define( } return AppFileSystem.normalizePath(path.resolve(root, AppFileSystem.windowsPath(text))) } - return path.resolve(root, text) + return path.isAbsolute(text) ? text : `${root.replace(/\/+$/, "")}/${text}` + }) + + const resolvePermissionTarget = Effect.fn("BashTool.resolvePermissionTarget")(function* ( + text: string, + root: string, + shell: string, + ) { + if (process.platform === "win32") { + if (Shell.posix(shell) && text.startsWith("/") && AppFileSystem.windowsPath(text) === text) { + const file = yield* cygpath(shell, text) + if (file) return file + } + return AppFileSystem.windowsPath(text) + } + return path.isAbsolute(text) ? text : `${root.replace(/\/+$/, "")}/${text}` }) const argPath = Effect.fn("BashTool.argPath")(function* (arg: string, cwd: string, ps: boolean, shell: string) { @@ -360,7 +381,7 @@ export const BashTool = Tool.define( if (!file || dynamic(file, ps)) return const next = ps ? provider(file) : file if (!next) return - return yield* resolvePath(next, cwd, shell) + return yield* resolvePermissionTarget(next, cwd, shell) }) const collect = Effect.fn("BashTool.collect")(function* (root: Node, cwd: string, ps: boolean, shell: string) { @@ -379,8 +400,10 @@ export const BashTool = Tool.define( for (const arg of pathArgs(command, ps)) { const resolved = yield* argPath(arg, cwd, ps, shell) log.info("resolved path", { arg, resolved }) - if (!resolved || Instance.containsPath(resolved)) continue - const dir = (yield* fs.isDir(resolved)) ? resolved : path.dirname(resolved) + if (!resolved) continue + const permissionPath = resolveExternalPathForPermission(resolved, cwd) + if (Instance.containsPath(permissionPath)) continue + const dir = (yield* fs.isDir(permissionPath)) ? permissionPath : path.dirname(permissionPath) scan.dirs.add(dir) } } @@ -589,6 +612,7 @@ export const BashTool = Tool.define( return { description: DESCRIPTION.replaceAll("${directory}", Instance.directory) + .replaceAll("${tmp}", Global.Path.tmp) .replaceAll("${os}", process.platform) .replaceAll("${shell}", name) .replaceAll("${chaining}", chain) @@ -597,18 +621,27 @@ export const BashTool = Tool.define( parameters: Parameters, execute: (params: Schema.Schema.Type, ctx: Tool.Context) => Effect.gen(function* () { - const cwd = params.workdir - ? yield* resolvePath(params.workdir, Instance.directory, shell) + const cwd = params.workdir ? yield* resolveExecutionPath(params.workdir, Instance.directory, shell) : Instance.directory + const permissionCwdTarget = params.workdir + ? yield* resolvePermissionTarget(params.workdir, Instance.directory, shell) : Instance.directory if (params.timeout !== undefined && params.timeout <= 0) { throw new Error(`Invalid timeout value: ${params.timeout}. Timeout must be a positive number.`) } const timeout = params.timeout ?? DEFAULT_TIMEOUT const ps = PS.has(name) - const root = yield* parse(params.command, ps) - const scan = yield* collect(root, cwd, ps, shell) - if (!Instance.containsPath(cwd)) scan.dirs.add(cwd) - yield* ask(ctx, scan) + yield* Effect.scoped( + Effect.gen(function* () { + const tree = yield* Effect.acquireRelease( + parse(params.command, ps), + (tree: Tree) => Effect.sync(() => tree.delete()), + ) + const scan = yield* collect(tree.rootNode, cwd, ps, shell) + const permissionCwd = resolveExternalPathForPermission(permissionCwdTarget, Instance.directory) + if (!Instance.containsPath(permissionCwd)) scan.dirs.add(permissionCwd) + yield* ask(ctx, scan) + }), + ) return yield* run( { diff --git a/packages/opencode/src/tool/bash.txt b/packages/opencode/src/tool/bash.txt index 1208230b7..41efd929e 100644 --- a/packages/opencode/src/tool/bash.txt +++ b/packages/opencode/src/tool/bash.txt @@ -4,6 +4,8 @@ Be aware: OS: ${os}, Shell: ${shell} All commands run in the current working directory by default. Use the `workdir` parameter if you need to run a command in a different directory. AVOID using `cd && ` patterns - use `workdir` instead. +Use `${tmp}` for temporary work outside the workspace. This directory is pre-approved for external directory access. + IMPORTANT: This tool is for terminal operations like git, npm, docker, etc. DO NOT use it for file operations (reading, writing, editing, searching, finding files) - use the specialized tools for this instead. Before executing the command, please follow these steps: diff --git a/packages/opencode/src/tool/external-directory.ts b/packages/opencode/src/tool/external-directory.ts index e0cb20706..755fee815 100644 --- a/packages/opencode/src/tool/external-directory.ts +++ b/packages/opencode/src/tool/external-directory.ts @@ -1,4 +1,5 @@ import path from "path" +import { lstatSync, realpathSync } from "fs" import { Effect } from "effect" import * as EffectLogger from "@opencode-ai/core/effect/logger" import { InstanceState } from "@/effect/instance-state" @@ -13,6 +14,114 @@ type Options = { kind?: Kind } +type PermissionPathFs = { + lstat: (path: string) => ReturnType + realpath: (path: string) => string +} + +const defaultPermissionPathFs: PermissionPathFs = { + lstat: lstatSync, + realpath: realpathSync.native, +} + +export function resolveExternalPathForPermission( + target: string, + base: string, + fs: PermissionPathFs = defaultPermissionPathFs, +): string { + if (process.platform !== "win32") return resolvePosixForPermission(target, base, fs) + return resolveWindowsForPermission(target, base, fs) +} + +function resolveWindowsForPermission(target: string, base: string, fs: PermissionPathFs): string { + const raw = windowsPermissionPath(target, base) + const parsed = path.win32.parse(raw) + if (!parsed.root) return AppFileSystem.normalizePath(raw, { base }) + + const parts = raw.slice(parsed.root.length).split(/[\\/]+/).filter(Boolean) + let current = parsed.root + + for (let i = 0; i < parts.length; i++) { + const part = parts[i]! + if (part === ".") continue + if (part === "..") { + current = path.win32.dirname(current) + continue + } + + const candidate = path.win32.join(current, part) + try { + const stat = fs.lstat(candidate) + if (!stat) throw Object.assign(new Error("missing"), { code: "ENOENT" }) + current = stat.isSymbolicLink() ? fs.realpath(candidate) : candidate + } catch (error: any) { + if (error?.code !== "ENOENT" && error?.code !== "ENOTDIR") throw error + return AppFileSystem.normalizePath(path.win32.join(current, part, ...parts.slice(i + 1)), { base }) + } + } + + try { + return AppFileSystem.normalizePath(fs.realpath(current), { base }) + } catch { + return AppFileSystem.normalizePath(current, { base }) + } +} + +function windowsPermissionPath(target: string, base: string): string { + const input = stripWindowsExtendedPrefix(AppFileSystem.windowsPath(target)).replaceAll("/", "\\") + if (/^\\\\/.test(input) || /^[A-Za-z]:[\\/]/.test(input)) return uppercaseDriveRoot(input) + const driveRelative = input.match(/^([A-Za-z]):(?![\\/])(.*)$/) + if (driveRelative) { + const drive = `${driveRelative[1].toUpperCase()}:` + const baseRoot = path.win32.parse(base).root + const baseDrive = baseRoot.slice(0, 2).toUpperCase() + const root = drive === baseDrive ? base : `${drive}\\` + return uppercaseDriveRoot(`${root.replace(/[\\/]+$/, "")}\\${driveRelative[2]}`) + } + if (/^[\\/](?![\\/])/.test(input)) { + const root = path.win32.parse(AppFileSystem.normalizePath(base, { base })).root + if (root) return uppercaseDriveRoot(root.replace(/[\\/]+$/, "") + input) + } + return uppercaseDriveRoot(`${base.replace(/[\\/]+$/, "")}\\${input}`) +} + +function stripWindowsExtendedPrefix(target: string): string { + if (/^\\\\\?\\UNC\\/i.test(target)) return target.replace(/^\\\\\?\\UNC\\/i, "\\\\") + if (/^\\\\\?\\[A-Za-z]:\\/i.test(target)) return target.slice(4) + return target +} + +function uppercaseDriveRoot(target: string): string { + return target.replace(/^([a-z]):/, (_, drive) => `${drive.toUpperCase()}:`) +} + +function resolvePosixForPermission(target: string, base: string, fs: PermissionPathFs): string { + const raw = path.isAbsolute(target) ? target : `${base.replace(/\/+$/, "")}/${target}` + const parts = raw.split("/").filter(Boolean) + let current = "/" + + for (let i = 0; i < parts.length; i++) { + const part = parts[i]! + if (part === ".") continue + if (part === "..") { + current = path.dirname(current) + continue + } + + const candidate = path.join(current, part) + try { + const stat = fs.lstat(candidate) + if (!stat) throw Object.assign(new Error("missing"), { code: "ENOENT" }) + current = stat.isSymbolicLink() ? fs.realpath(candidate) : candidate + } catch (error: any) { + if (error?.code !== "ENOENT" && error?.code !== "ENOTDIR") throw error + return path.resolve(current, part, ...parts.slice(i + 1)) + } + } + + return fs.realpath(current) +} + export const assertExternalDirectoryEffect = Effect.fn("Tool.assertExternalDirectory")(function* ( ctx: Tool.Context, target?: string, @@ -21,12 +130,23 @@ export const assertExternalDirectoryEffect = Effect.fn("Tool.assertExternalDirec if (!target) return const ins = yield* InstanceState.context - const full = process.platform === "win32" ? AppFileSystem.normalizePath(target, { base: ins.directory }) : target + const full = + process.platform === "win32" + ? windowsPermissionPath(target, ins.directory) + : path.isAbsolute(target) + ? target + : `${ins.directory.replace(/\/+$/, "")}/${target}` if (options?.bypass) return full - if (Instance.containsPath(full, ins)) return full + const resolved = resolveExternalPathForPermission(full, ins.directory) + const scope = { + ...ins, + directory: resolveExternalPathForPermission(ins.directory, ins.directory), + worktree: ins.worktree === "/" ? ins.worktree : resolveExternalPathForPermission(ins.worktree, ins.directory), + } + if (Instance.containsPath(resolved, scope)) return full const kind = options?.kind ?? "file" - const dir = kind === "directory" ? full : path.dirname(full) + const dir = kind === "directory" ? resolved : path.dirname(resolved) const glob = process.platform === "win32" ? AppFileSystem.normalizePathPattern(path.join(dir, "*"), { base: ins.directory }) @@ -38,6 +158,7 @@ export const assertExternalDirectoryEffect = Effect.fn("Tool.assertExternalDirec always: [glob], metadata: { filepath: full, + realpath: resolved, parentDir: dir, }, }) diff --git a/packages/opencode/src/tool/read.ts b/packages/opencode/src/tool/read.ts index 95e55c8cc..b68af2bdf 100644 --- a/packages/opencode/src/tool/read.ts +++ b/packages/opencode/src/tool/read.ts @@ -8,7 +8,7 @@ import DESCRIPTION from "./read.txt" import { Instance } from "../project/instance" import { assertExternalDirectoryEffect } from "./external-directory" import { Instruction } from "../session/instruction" -import { isImageAttachment, isPdfAttachment, sniffAttachmentMime } from "@/util/media" +import { isPdfAttachment, sniffAttachmentMime } from "@/util/media" const DEFAULT_READ_LIMIT = 2000 const MAX_LINE_LENGTH = 2000 @@ -18,6 +18,7 @@ const MAX_BYTES_LABEL = `${MAX_BYTES / 1024} KB` const MAX_ATTACHMENT_BYTES = 5 * 1024 * 1024 const MAX_ATTACHMENT_BYTES_LABEL = `${MAX_ATTACHMENT_BYTES / 1024 / 1024} MB` const SAMPLE_BYTES = 4096 +const SUPPORTED_IMAGE_MIMES = new Set(["image/jpeg", "image/png", "image/gif", "image/webp"]) // `offset` and `limit` were originally `z.coerce.number()` — the runtime // coercion was useful when the tool was called from a shell but serves no @@ -27,7 +28,7 @@ const SAMPLE_BYTES = 4096 export const Parameters = Schema.Struct({ filePath: Schema.String.annotate({ description: "The absolute path to the file or directory to read" }), offset: Schema.optional(Schema.Number).annotate({ - description: "The line number to start reading from (1-indexed)", + description: "The line number to start reading from (1-indexed; 0 is accepted as the first line/entry)", }), limit: Schema.optional(Schema.Number).annotate({ description: "The maximum number of lines to read (defaults to 2000)", @@ -155,8 +156,8 @@ export const ReadTool = Tool.define( params: Schema.Schema.Type, ctx: Tool.Context, ) { - if (params.offset !== undefined && (!Number.isInteger(params.offset) || params.offset < 1)) { - return yield* Effect.fail(new Error("offset must be a positive integer (>= 1)")) + if (params.offset !== undefined && (!Number.isInteger(params.offset) || params.offset < 0)) { + return yield* Effect.fail(new Error("offset must be a non-negative integer")) } if (params.limit !== undefined && (!Number.isInteger(params.limit) || params.limit < 1)) { return yield* Effect.fail(new Error("limit must be a positive integer (>= 1)")) @@ -196,7 +197,7 @@ export const ReadTool = Tool.define( if (stat.type === "Directory") { const items = yield* list(filepath) const limit = params.limit ?? DEFAULT_READ_LIMIT - const offset = params.offset ?? 1 + const offset = params.offset || 1 const start = offset - 1 const sliced = items.slice(start, start + limit) const truncated = start + sliced.length < items.length @@ -234,7 +235,8 @@ export const ReadTool = Tool.define( const sample = yield* readSample(filepath, Number(stat.size), SAMPLE_BYTES) const mime = sniffAttachmentMime(sample, AppFileSystem.mimeType(filepath)) - if (isImageAttachment(mime) || isPdfAttachment(mime)) { + const isImage = SUPPORTED_IMAGE_MIMES.has(mime) + if (isImage || isPdfAttachment(mime)) { if (Number(stat.size) > MAX_ATTACHMENT_BYTES) { return yield* Effect.fail( new Error(`Cannot read attachment larger than ${MAX_ATTACHMENT_BYTES_LABEL}: ${filepath}`), @@ -265,7 +267,7 @@ export const ReadTool = Tool.define( } const file = yield* Effect.promise(() => - lines(filepath, { limit: params.limit ?? DEFAULT_READ_LIMIT, offset: params.offset ?? 1 }), + lines(filepath, { limit: params.limit ?? DEFAULT_READ_LIMIT, offset: params.offset || 1 }), ) if (file.count < file.offset && !(file.count === 0 && file.offset === 1)) { return yield* Effect.fail( diff --git a/packages/opencode/src/tool/registry.ts b/packages/opencode/src/tool/registry.ts index cb7f48d39..9fc5c4f6c 100644 --- a/packages/opencode/src/tool/registry.ts +++ b/packages/opencode/src/tool/registry.ts @@ -183,7 +183,16 @@ export namespace ToolRegistry { outputPath: out.truncated ? out.outputPath : undefined, }, } - }), + }).pipe( + Effect.withSpan("Tool.execute", { + attributes: { + "tool.name": id, + "session.id": toolCtx.sessionID, + "message.id": toolCtx.messageID, + ...(toolCtx.callID ? { "tool.call_id": toolCtx.callID } : {}), + }, + }), + ), } } diff --git a/packages/opencode/src/util/timeout.ts b/packages/opencode/src/util/timeout.ts index 877996552..31ac48146 100644 --- a/packages/opencode/src/util/timeout.ts +++ b/packages/opencode/src/util/timeout.ts @@ -1,9 +1,8 @@ export function withTimeout(promise: Promise, ms: number): Promise { let timeout: NodeJS.Timeout return Promise.race([ - promise.then((result) => { + promise.finally(() => { clearTimeout(timeout) - return result }), new Promise((_, reject) => { timeout = setTimeout(() => { diff --git a/packages/opencode/src/worktree/index.ts b/packages/opencode/src/worktree/index.ts index ffb4816ef..3f4803390 100644 --- a/packages/opencode/src/worktree/index.ts +++ b/packages/opencode/src/worktree/index.ts @@ -449,16 +449,15 @@ export namespace Worktree { const createFromInfo = Effect.fn("Worktree.createFromInfo")(function* (info: Info, startCommand?: string) { yield* setup(info) - yield* boot(info, startCommand) + yield* boot(info, startCommand).pipe( + Effect.catchCause((cause) => Effect.sync(() => log.error("worktree bootstrap failed", { cause }))), + Effect.forkIn(scope), + ) }) const create = Effect.fn("Worktree.create")(function* (input?: CreateInput) { const info = yield* makeWorktreeInfo(input?.name) - yield* setup(info) - yield* boot(info, input?.startCommand).pipe( - Effect.catchCause((cause) => Effect.sync(() => log.error("worktree bootstrap failed", { cause }))), - Effect.forkIn(scope), - ) + yield* createFromInfo(info, input?.startCommand) return info }) diff --git a/packages/opencode/test/agent/agent.test.ts b/packages/opencode/test/agent/agent.test.ts index c33dd6a5a..d8fa0fa13 100644 --- a/packages/opencode/test/agent/agent.test.ts +++ b/packages/opencode/test/agent/agent.test.ts @@ -4,6 +4,7 @@ import { tmpdir } from "../fixture/fixture" import { Instance } from "../../src/project/instance" import { Agent } from "../../src/agent/agent" import { Permission } from "../../src/permission" +import { Global } from "@opencode-ai/core/global" // Helper to evaluate permission for a tool with wildcard pattern function evalPerm(agent: Agent.Info | undefined, permission: string): Permission.Action | undefined { @@ -593,6 +594,29 @@ description: Permission skill. } }) +test("opencode tmp directory is allowed for external_directory", async () => { + await using tmp = await tmpdir({ + config: { + permission: { + external_directory: { + "*": "deny", + }, + }, + }, + }) + await Instance.provide({ + directory: tmp.path, + fn: async () => { + const build = await Agent.get("build") + const target = path.join(Global.Path.tmp, "agent-work", "scratch.txt") + expect(Permission.evaluate("external_directory", target, build!.permission).action).toBe("allow") + expect( + Permission.evaluate("external_directory", path.join(path.dirname(tmp.path), "random", "scratch.txt"), build!.permission).action, + ).toBe("deny") + }, + }) +}) + test("defaultAgent returns build when no default_agent config", async () => { await using tmp = await tmpdir() await Instance.provide({ diff --git a/packages/opencode/test/git/git.test.ts b/packages/opencode/test/git/git.test.ts index e0fed7d82..553ff5c99 100644 --- a/packages/opencode/test/git/git.test.ts +++ b/packages/opencode/test/git/git.test.ts @@ -221,6 +221,71 @@ describe("Git", () => { }), ) + test("patch helpers return capped native patch output", async () => { + await using tmp = await tmpdir({ git: true }) + await fs.writeFile(path.join(tmp.path, weird), "before\n", "utf-8") + await $`git add .`.cwd(tmp.path).quiet() + await $`git commit --no-gpg-sign -m "add file"`.cwd(tmp.path).quiet() + await fs.writeFile(path.join(tmp.path, weird), "after\n", "utf-8") + + await withGit(async (rt) => { + const [patch, capped] = await Promise.all([ + rt.runPromise(Git.Service.use((git) => git.patch(tmp.path, "HEAD", weird, { context: 2_147_483_647 }))), + rt.runPromise(Git.Service.use((git) => git.patch(tmp.path, "HEAD", weird, { maxOutputBytes: 1 }))), + ]) + + expect(patch.truncated).toBe(false) + expect(patch.text).toContain("diff --git") + expect(patch.text).toContain("-before") + expect(patch.text).toContain("+after") + expect(capped.truncated).toBe(true) + expect(capped.text).toBe("") + }) + }) + + test("run keeps stderr truncation separate from stdout truncation", async () => { + if (process.platform === "win32") return + await using tmp = await tmpdir({ git: true }) + + await withGit(async (rt) => { + const result = await rt.runPromise( + Git.Service.use((git) => + git.run( + [ + "-c", + "alias.noisy=!f() { printf ok; i=0; while [ $i -lt 2048 ]; do printf x >&2; i=$((i+1)); done; }; f", + "noisy", + ], + { cwd: tmp.path, maxOutputBytes: 16 }, + ), + ), + ) + + expect(result.text()).toBe("ok") + expect(result.truncated).toBe(false) + expect(result.stdoutTruncated).toBe(false) + expect(result.stderrTruncated).toBe(true) + }) + }) + + test("patchUntracked() and statUntracked() handle added files", async () => { + await using tmp = await tmpdir({ git: true }) + await fs.writeFile(path.join(tmp.path, weird), "one\ntwo\n", "utf-8") + + await withGit(async (rt) => { + const [patch, stat] = await Promise.all([ + rt.runPromise(Git.Service.use((git) => git.patchUntracked(tmp.path, weird, { context: 2_147_483_647 }))), + rt.runPromise(Git.Service.use((git) => git.statUntracked(tmp.path, weird))), + ]) + + expect(patch.truncated).toBe(false) + expect(patch.text).toContain("diff --git") + expect(patch.text).toContain("+one") + expect(patch.text).toContain("+two") + expect(stat).toEqual(expect.objectContaining({ file: weird, additions: 2, deletions: 0 })) + }) + }) + test("show() returns empty text for binary blobs", async () => { await using tmp = await tmpdir({ git: true }) await fs.writeFile(path.join(tmp.path, "bin.dat"), new Uint8Array([0, 1, 2, 3])) diff --git a/packages/opencode/test/mcp/headers.test.ts b/packages/opencode/test/mcp/headers.test.ts index 69998aaaa..78a3a2068 100644 --- a/packages/opencode/test/mcp/headers.test.ts +++ b/packages/opencode/test/mcp/headers.test.ts @@ -151,3 +151,22 @@ test("no requestInit when headers are not provided", async () => { }, }) }) + +test("invalid remote url returns failed status without constructing transports", async () => { + await using tmp = await tmpdir() + + await Instance.provide({ + directory: tmp.path, + fn: async () => { + transportCalls.length = 0 + + const result = await MCP.add("bad-url", { + type: "remote", + url: "not a valid url", + }) + + expect(result.status).toEqual({ "bad-url": { status: "failed", error: 'Invalid MCP URL for "bad-url"' } }) + expect(transportCalls.length).toBe(0) + }, + }) +}) diff --git a/packages/opencode/test/project/vcs.test.ts b/packages/opencode/test/project/vcs.test.ts index 78ae49136..165ab05e2 100644 --- a/packages/opencode/test/project/vcs.test.ts +++ b/packages/opencode/test/project/vcs.test.ts @@ -187,6 +187,8 @@ describe("Vcs diff", () => { expect.objectContaining({ file: "untracked.txt", status: "added" }), ]), ) + expect(diff.find((item) => item.file === "tracked.txt")?.patch).toContain("diff --git") + expect(diff.find((item) => item.file === "untracked.txt")?.patch).toContain("+untracked") expect(diff).not.toEqual(expect.arrayContaining([expect.objectContaining({ file: "staged.txt" })])) }) }) @@ -221,6 +223,7 @@ describe("Vcs diff", () => { await withVcsOnly(tmp.path, async () => { const diff = await Vcs.diff("staged") expect(diff).toEqual(expect.arrayContaining([expect.objectContaining({ file: "staged.txt", status: "added" })])) + expect(diff.find((item) => item.file === "staged.txt")?.patch).toContain("+staged") expect(diff).not.toEqual(expect.arrayContaining([expect.objectContaining({ file: "unstaged.txt" })])) }) }) @@ -253,6 +256,7 @@ describe("Vcs diff", () => { await withVcsOnly(tmp.path, async () => { const diff = await Vcs.diff("branch") expect(diff).toEqual(expect.arrayContaining([expect.objectContaining({ file: "branch.txt", status: "added" })])) + expect(diff.find((item) => item.file === "branch.txt")?.patch).toContain("+branch") expect(diff).not.toEqual(expect.arrayContaining([expect.objectContaining({ file: "staged.txt" })])) expect(diff).not.toEqual(expect.arrayContaining([expect.objectContaining({ file: "unstaged.txt" })])) expect(diff).not.toEqual(expect.arrayContaining([expect.objectContaining({ file: "untracked.txt" })])) diff --git a/packages/opencode/test/project/worktree.test.ts b/packages/opencode/test/project/worktree.test.ts index 275c97e5a..3db18fc5d 100644 --- a/packages/opencode/test/project/worktree.test.ts +++ b/packages/opencode/test/project/worktree.test.ts @@ -177,10 +177,11 @@ describe("Worktree", () => { }) describe("createFromInfo", () => { - wintest("creates and bootstraps git worktree", async () => { + wintest("creates git worktree and boots asynchronously", async () => { await using tmp = await tmpdir({ git: true }) const info = await withInstance(tmp.path, () => Worktree.makeWorktreeInfo("from-info-test")) + const ready = waitReady(path.join(tmp.path, ".worktrees", "pawwork")) await withInstance(tmp.path, () => Worktree.createFromInfo(info)) // Worktree should exist in git (normalize slashes for Windows) @@ -189,7 +190,7 @@ describe("Worktree", () => { const normalizedDir = info.directory.replace(/\\/g, "/") expect(normalizedList).toContain(normalizedDir) - // Cleanup + await ready await withInstance(tmp.path, () => Worktree.remove({ directory: info.directory })) }) }) diff --git a/packages/opencode/test/provider/transform.test.ts b/packages/opencode/test/provider/transform.test.ts index 79b70418b..e0acf87c3 100644 --- a/packages/opencode/test/provider/transform.test.ts +++ b/packages/opencode/test/provider/transform.test.ts @@ -1620,6 +1620,109 @@ describe("ProviderTransform.message - DeepSeek reasoning content", () => { }) }) +describe("ProviderTransform.message - surrogate sanitization", () => { + const model = { + id: "test/test-model", + providerID: "test", + api: { + id: "test-model", + url: "https://api.test.com", + npm: "@ai-sdk/openai-compatible", + }, + name: "Test Model", + capabilities: { + temperature: true, + reasoning: true, + attachment: true, + toolcall: true, + input: { text: true, audio: false, image: true, video: false, pdf: false }, + output: { text: true, audio: false, image: false, video: false, pdf: false }, + interleaved: false, + }, + cost: { input: 0.001, output: 0.002, cache: { read: 0.0001, write: 0.0002 } }, + limit: { context: 128000, output: 8192 }, + status: "active", + options: {}, + headers: {}, + } as any + + test("replaces lone surrogates in model-visible text", () => { + const lone = "\uD83D" + const valid = "\uD83D\uDE80" + const text = (label: string) => `${label} ${lone} and ${valid}` + const expected = (label: string) => `${label} \uFFFD and ${valid}` + const msgs = [ + { role: "system", content: text("system") }, + { role: "user", content: text("user string") }, + { + role: "user", + content: [ + { type: "text", text: text("user text") }, + { type: "image", image: "data:image/png;base64,abcd" }, + ], + }, + { role: "assistant", content: text("assistant string") }, + { + role: "assistant", + content: [ + { type: "text", text: text("assistant text") }, + { type: "reasoning", text: text("assistant reasoning") }, + { + type: "tool-result", + toolCallId: "call-1", + toolName: "Read", + output: { type: "text", value: text("assistant tool text") }, + }, + { + type: "tool-result", + toolCallId: "call-2", + toolName: "Read", + output: { type: "error-text", value: text("assistant tool error") }, + }, + { + type: "tool-result", + toolCallId: "call-3", + toolName: "Read", + output: { type: "content", value: [{ type: "text", text: text("assistant tool content") }] }, + }, + ], + }, + { + role: "tool", + content: [ + { + type: "tool-result", + toolCallId: "call-4", + toolName: "Read", + output: { type: "text", value: text("tool text") }, + }, + { + type: "tool-result", + toolCallId: "call-5", + toolName: "Read", + output: { type: "content", value: [{ type: "text", text: text("tool content") }] }, + }, + ], + }, + ] as any[] + + const result = ProviderTransform.message(msgs, model, {}) as any[] + + expect(result[0].content).toBe(expected("system")) + expect(result[1].content).toBe(expected("user string")) + expect(result[2].content[0].text).toBe(expected("user text")) + expect(result[2].content[1]).toEqual({ type: "image", image: "data:image/png;base64,abcd" }) + expect(result[3].content).toBe(expected("assistant string")) + expect(result[4].content[0].text).toBe(expected("assistant text")) + expect(result[4].content[1].text).toBe(expected("assistant reasoning")) + expect(result[4].content[2].output.value).toBe(expected("assistant tool text")) + expect(result[4].content[3].output.value).toBe(expected("assistant tool error")) + expect(result[4].content[4].output.value[0].text).toBe(expected("assistant tool content")) + expect(result[5].content[0].output.value).toBe(expected("tool text")) + expect(result[5].content[1].output.value[0].text).toBe(expected("tool content")) + }) +}) + describe("ProviderTransform.message - empty image handling", () => { const mockModel = { id: "anthropic/claude-3-5-sonnet", @@ -1808,6 +1911,83 @@ describe("ProviderTransform.message - anthropic empty content filtering", () => expect(result[0].content[0]).toEqual({ type: "text", text: "Answer" }) }) + test("keeps empty Anthropic reasoning parts with signature metadata", () => { + const msgs = [ + { + role: "assistant", + content: [ + { + type: "reasoning", + text: "", + providerOptions: { anthropic: { signature: "sig" } }, + }, + { type: "text", text: "Answer" }, + ], + }, + ] as any[] + + const result = ProviderTransform.message(msgs, anthropicModel, {}) + + expect(result).toHaveLength(1) + expect(result[0].content).toEqual([ + { + type: "reasoning", + text: "", + providerOptions: { anthropic: { signature: "sig" } }, + }, + { type: "text", text: "Answer" }, + ]) + }) + + test("keeps empty Bedrock reasoning parts with signature or redacted data metadata", () => { + const bedrockModel = { + ...anthropicModel, + id: "amazon-bedrock/anthropic.claude-opus-4-6", + providerID: "amazon-bedrock", + api: { + id: "anthropic.claude-opus-4-6", + url: "https://bedrock-runtime.us-east-1.amazonaws.com", + npm: "@ai-sdk/amazon-bedrock", + }, + } + const msgs = [ + { + role: "assistant", + content: [ + { + type: "reasoning", + text: "", + providerOptions: { bedrock: { signature: "sig" } }, + }, + { + type: "reasoning", + text: "", + providerOptions: { bedrock: { redactedData: "redacted" } }, + }, + { type: "reasoning", text: "" }, + { type: "text", text: "Answer" }, + ], + }, + ] as any[] + + const result = ProviderTransform.message(msgs, bedrockModel, {}) + + expect(result).toHaveLength(1) + expect(result[0].content).toEqual([ + { + type: "reasoning", + text: "", + providerOptions: { bedrock: { signature: "sig" } }, + }, + { + type: "reasoning", + text: "", + providerOptions: { bedrock: { redactedData: "redacted" } }, + }, + { type: "text", text: "Answer" }, + ]) + }) + test("removes entire message when all parts are empty", () => { const msgs = [ { role: "user", content: "Hello" }, diff --git a/packages/opencode/test/session/message-v2.test.ts b/packages/opencode/test/session/message-v2.test.ts index 3c88c16d0..4a3978164 100644 --- a/packages/opencode/test/session/message-v2.test.ts +++ b/packages/opencode/test/session/message-v2.test.ts @@ -158,6 +158,54 @@ describe("session.message-v2.toModelMessage", () => { expect(await MessageV2.toModelMessages(input, model)).toStrictEqual([]) }) + test("filters out user messages with only empty text parts", async () => { + const messageID = "m-user" + + const input: MessageV2.WithParts[] = [ + { + info: userInfo(messageID), + parts: [ + { + ...basePart(messageID, "p1"), + type: "text", + text: "", + }, + ] as MessageV2.Part[], + }, + ] + + expect(await MessageV2.toModelMessages(input, model)).toStrictEqual([]) + }) + + test("filters empty user text parts while keeping non-empty parts", async () => { + const messageID = "m-user" + + const input: MessageV2.WithParts[] = [ + { + info: userInfo(messageID), + parts: [ + { + ...basePart(messageID, "p1"), + type: "text", + text: "", + }, + { + ...basePart(messageID, "p2"), + type: "text", + text: "hello", + }, + ] as MessageV2.Part[], + }, + ] + + expect(await MessageV2.toModelMessages(input, model)).toStrictEqual([ + { + role: "user", + content: [{ type: "text", text: "hello" }], + }, + ]) + }) + test("includes synthetic text parts", async () => { const messageID = "m-user" @@ -425,6 +473,167 @@ describe("session.message-v2.toModelMessage", () => { }) }) + test("omits empty text when tool output only has media attachments", async () => { + const userID = "m-user" + const assistantID = "m-assistant" + + const input: MessageV2.WithParts[] = [ + { + info: userInfo(userID), + parts: [ + { + ...basePart(userID, "u1"), + type: "text", + text: "inspect image", + }, + ] as MessageV2.Part[], + }, + { + info: assistantInfo(assistantID, userID), + parts: [ + { + ...basePart(assistantID, "a1"), + type: "tool", + callID: "call-1", + tool: "read", + state: { + status: "completed", + input: { filePath: "image.png" }, + output: "", + title: "Read", + metadata: {}, + time: { start: 0, end: 1 }, + attachments: [ + { + ...basePart(assistantID, "file-1"), + type: "file", + mime: "image/png", + filename: "image.png", + url: "data:image/png;base64,aW1hZ2U=", + }, + ], + }, + }, + ] as MessageV2.Part[], + }, + ] + + const [, , tool] = await MessageV2.toModelMessages(input, model) + expect(tool).toMatchObject({ + role: "tool", + content: [ + { + output: { + type: "content", + value: [{ type: "media", mediaType: "image/png", data: "aW1hZ2U=" }], + }, + }, + ], + }) + }) + + test("moves bedrock pdf tool-result media into a separate user message", async () => { + const bedrockModel: Provider.Model = { + ...model, + id: ModelID.make("amazon-bedrock/anthropic.claude-sonnet-4-6"), + providerID: ProviderID.make("amazon-bedrock"), + api: { + id: "anthropic.claude-sonnet-4-6", + url: "https://bedrock-runtime.us-east-1.amazonaws.com", + npm: "@ai-sdk/amazon-bedrock", + }, + capabilities: { + ...model.capabilities, + attachment: true, + input: { + ...model.capabilities.input, + image: true, + pdf: true, + }, + }, + } + const pdf = Buffer.from("%PDF-1.4\n").toString("base64") + const userID = "m-user-bedrock-pdf" + const assistantID = "m-assistant-bedrock-pdf" + const input: MessageV2.WithParts[] = [ + { + info: userInfo(userID), + parts: [ + { + ...basePart(userID, "u1-bedrock-pdf"), + type: "text", + text: "run tool", + }, + ] as MessageV2.Part[], + }, + { + info: assistantInfo(assistantID, userID), + parts: [ + { + ...basePart(assistantID, "a1-bedrock-pdf"), + type: "tool", + callID: "call-bedrock-pdf-1", + tool: "read", + state: { + status: "completed", + input: { filePath: "/tmp/example.pdf" }, + output: "PDF read successfully", + title: "Read", + metadata: {}, + time: { start: 0, end: 1 }, + attachments: [ + { + ...basePart(assistantID, "file-bedrock-pdf-1"), + type: "file", + mime: "application/pdf", + filename: "example.pdf", + url: `data:application/pdf;base64,${pdf}`, + }, + ], + }, + }, + ] as MessageV2.Part[], + }, + ] + + expect(await MessageV2.toModelMessages(input, bedrockModel)).toStrictEqual([ + { + role: "user", + content: [{ type: "text", text: "run tool" }], + }, + { + role: "assistant", + content: [ + { + type: "tool-call", + toolCallId: "call-bedrock-pdf-1", + toolName: "read", + input: { filePath: "/tmp/example.pdf" }, + providerExecuted: undefined, + }, + ], + }, + { + role: "tool", + content: [ + { + type: "tool-result", + toolCallId: "call-bedrock-pdf-1", + toolName: "read", + output: { type: "text", value: "PDF read successfully" }, + }, + ], + }, + { + role: "user", + content: [ + { type: "text", text: "Attached media from tool result:" }, + { type: "file", mediaType: "application/pdf", filename: "example.pdf", data: `data:application/pdf;base64,${pdf}` }, + ], + }, + ]) + }) + test("omits provider metadata when assistant model differs", async () => { const userID = "m-user" const assistantID = "m-assistant" @@ -1086,6 +1295,66 @@ describe("session.message-v2.toModelMessage", () => { }, ]) }) + + test("substitutes space for empty text between Anthropic signed reasoning blocks", async () => { + const assistantID = "m-assistant" + const input: MessageV2.WithParts[] = [ + { + info: assistantInfo(assistantID, "m-parent"), + parts: [ + { ...basePart(assistantID, "p1"), type: "step-start" }, + { + ...basePart(assistantID, "p2"), + type: "reasoning", + text: "thinking-one", + metadata: { anthropic: { signature: "sig1" } }, + }, + { ...basePart(assistantID, "p3"), type: "text", text: "" }, + { ...basePart(assistantID, "p4"), type: "step-start" }, + { + ...basePart(assistantID, "p5"), + type: "reasoning", + text: "thinking-two", + metadata: { anthropic: { signature: "sig2" } }, + }, + { ...basePart(assistantID, "p6"), type: "text", text: "the answer" }, + ] as MessageV2.Part[], + }, + ] + + const result = await MessageV2.toModelMessages(input, model) + + expect(result).toHaveLength(2) + expect((result[0].content as any[]).find((p) => p.type === "text").text).toBe(" ") + expect((result[1].content as any[]).find((p) => p.type === "text").text).toBe("the answer") + }) + + test("leaves empty text alone when reasoning signature is under bedrock namespace", async () => { + const assistantID = "m-assistant-bedrock" + const input: MessageV2.WithParts[] = [ + { + info: assistantInfo(assistantID, "m-parent"), + parts: [ + { + ...basePart(assistantID, "p1"), + type: "reasoning", + text: "thinking-bedrock", + metadata: { bedrock: { signature: "bedrock-sig" } }, + }, + { ...basePart(assistantID, "p2"), type: "text", text: "" }, + { ...basePart(assistantID, "p3"), type: "text", text: "answer" }, + ] as MessageV2.Part[], + }, + ] + + const result = await MessageV2.toModelMessages(input, model) + + expect(result).toHaveLength(1) + expect((result[0].content as any[]).filter((p) => p.type === "text").map((p) => p.text)).toStrictEqual([ + "", + "answer", + ]) + }) }) describe("session.message-v2.fromError", () => { diff --git a/packages/opencode/test/session/messages-pagination.test.ts b/packages/opencode/test/session/messages-pagination.test.ts index e74d5e038..90df84e71 100644 --- a/packages/opencode/test/session/messages-pagination.test.ts +++ b/packages/opencode/test/session/messages-pagination.test.ts @@ -33,6 +33,9 @@ const svc = { updatePart(part: T) { return run(SessionNs.Service.use((svc) => svc.updatePart(part))) }, + fork(input: { sessionID: SessionID; messageID?: MessageID }) { + return run(SessionNs.Service.use((svc) => svc.fork(input))) + }, } async function fill(sessionID: SessionID, count: number, time = (i: number) => Date.now() + i) { @@ -111,13 +114,14 @@ async function addAssistant( return id } -async function addCompactionPart(sessionID: SessionID, messageID: MessageID) { +async function addCompactionPart(sessionID: SessionID, messageID: MessageID, tailStartID?: MessageID) { await svc.updatePart({ id: PartID.ascending(), sessionID, messageID, type: "compaction", auto: true, + tail_start_id: tailStartID, } as any) } @@ -810,6 +814,90 @@ describe("MessageV2.filterCompacted", () => { ), ) + test("fork remaps compaction tail_start_id for filterCompacted", async () => { + await Instance.provide({ + directory: root, + fn: async () => { + const session = await svc.create({}) + + const old = await addUser(session.id, "old") + const oldAssistant = await addAssistant(session.id, old) + await svc.updatePart({ + id: PartID.ascending(), + sessionID: session.id, + messageID: oldAssistant, + type: "text", + text: "old reply", + }) + + const retained = await addUser(session.id, "retained") + const retainedAssistant = await addAssistant(session.id, retained) + await svc.updatePart({ + id: PartID.ascending(), + sessionID: session.id, + messageID: retainedAssistant, + type: "text", + text: "retained reply", + }) + + const boundary = await addUser(session.id, "compact") + await addCompactionPart(session.id, boundary, retained) + const summary = await addAssistant(session.id, boundary, { summary: true, finish: "end_turn" }) + await svc.updatePart({ + id: PartID.ascending(), + sessionID: session.id, + messageID: summary, + type: "text", + text: "summary", + }) + + const parentFiltered = MessageV2.filterCompacted(MessageV2.stream(session.id)) + expect(parentFiltered.map((message) => message.info.id)).toEqual([ + boundary, + summary, + retained, + retainedAssistant, + ]) + + const forked = await svc.fork({ sessionID: session.id }) + const childFiltered = MessageV2.filterCompacted(MessageV2.stream(forked.id)) + expect(childFiltered).toHaveLength(parentFiltered.length) + + const tailPart = childFiltered.flatMap((message) => message.parts).find((part) => part.type === "compaction") + expect(tailPart?.type).toBe("compaction") + if (!tailPart || tailPart.type !== "compaction") throw new Error("Expected forked compaction part") + expect(tailPart.tail_start_id).toBeDefined() + expect(childFiltered.some((message) => message.info.id === tailPart.tail_start_id)).toBe(true) + + await svc.remove(forked.id) + await svc.remove(session.id) + }, + }) + }) + + test("fork preserves compaction tail_start_id when the tail is not cloned", async () => { + await Instance.provide({ + directory: root, + fn: async () => { + const session = await svc.create({}) + + const boundary = await addUser(session.id, "compact") + const tail = await addUser(session.id, "retained") + await addCompactionPart(session.id, boundary, tail) + + const forked = await svc.fork({ sessionID: session.id, messageID: tail }) + const childMessages = MessageV2.filterCompacted(MessageV2.stream(forked.id)) + const tailPart = childMessages.flatMap((message) => message.parts).find((part) => part.type === "compaction") + expect(tailPart?.type).toBe("compaction") + if (!tailPart || tailPart.type !== "compaction") throw new Error("Expected forked compaction part") + expect(tailPart.tail_start_id).toBe(tail) + + await svc.remove(forked.id) + await svc.remove(session.id) + }, + }) + }) + it.live( "keeps retained tail messages after successful compaction summary", provideTmpdirInstance(() => diff --git a/packages/opencode/test/session/retry.test.ts b/packages/opencode/test/session/retry.test.ts index 93b0c4153..313172705 100644 --- a/packages/opencode/test/session/retry.test.ts +++ b/packages/opencode/test/session/retry.test.ts @@ -344,6 +344,25 @@ describe("session.message-v2.fromError", () => { expect(SessionRetry.retryable(result)).toBe("An error occurred while processing your request.") }) + test("converts OpenAI server_is_overloaded stream chunks to retryable APIError", () => { + const result = MessageV2.fromError( + { + message: JSON.stringify({ + type: "error", + error: { + code: "server_is_overloaded", + message: "The server is overloaded. Please try again later.", + }, + }), + }, + { providerID: ProviderID.make("openai") }, + ) + + expect(MessageV2.APIError.isInstance(result)).toBe(true) + expect((result as MessageV2.APIError).data.isRetryable).toBe(true) + expect(SessionRetry.retryable(result)).toBe("The server is overloaded. Please try again later.") + }) + test("uses fallback message for OpenAI server_error stream chunks without message", () => { const result = MessageV2.fromError( { diff --git a/packages/opencode/test/skill/skill.test.ts b/packages/opencode/test/skill/skill.test.ts index 9c1def0ab..2dd0ae07c 100644 --- a/packages/opencode/test/skill/skill.test.ts +++ b/packages/opencode/test/skill/skill.test.ts @@ -289,6 +289,53 @@ description: A skill in the .claude/skills directory. }) }) +test("Claude Code skills flag does not disable .agents/skills discovery", async () => { + await using tmp = await tmpdir({ + git: true, + init: async (dir) => { + const agentDir = path.join(dir, ".agents", "skills", "agent-skill") + const claudeDir = path.join(dir, ".claude", "skills", "claude-skill") + await Bun.write( + path.join(agentDir, "SKILL.md"), + `--- +name: agent-skill +description: A skill in the .agents/skills directory. +--- + +# Agent Skill +`, + ) + await Bun.write( + path.join(claudeDir, "SKILL.md"), + `--- +name: claude-skill +description: A skill in the .claude/skills directory. +--- + +# Claude Skill +`, + ) + }, + }) + + const original = process.env.OPENCODE_DISABLE_CLAUDE_CODE_SKILLS + process.env.OPENCODE_DISABLE_CLAUDE_CODE_SKILLS = "true" + + try { + await Instance.provide({ + directory: tmp.path, + fn: async () => { + const skills = await Skill.all() + expect(skills.find((s) => s.name === "agent-skill")).toBeDefined() + expect(skills.find((s) => s.name === "claude-skill")).toBeUndefined() + }, + }) + } finally { + if (original === undefined) delete process.env.OPENCODE_DISABLE_CLAUDE_CODE_SKILLS + else process.env.OPENCODE_DISABLE_CLAUDE_CODE_SKILLS = original + } +}) + test("properly resolves directories that skills live in", async () => { await using tmp = await tmpdir({ git: true, diff --git a/packages/opencode/test/tool/agent.test.ts b/packages/opencode/test/tool/agent.test.ts index c0b21816e..c8c338117 100644 --- a/packages/opencode/test/tool/agent.test.ts +++ b/packages/opencode/test/tool/agent.test.ts @@ -1,5 +1,5 @@ import { afterEach, describe, expect, test } from "bun:test" -import { Effect, Layer } from "effect" +import { Effect, Exit, Fiber, Layer } from "effect" import { Agent } from "../../src/agent/agent" import { Config } from "../../src/config/config" import * as CrossSpawnSpawner from "@opencode-ai/core/cross-spawn-spawner" @@ -7,7 +7,7 @@ import { Instance } from "../../src/project/instance" import { Session } from "../../src/session" import { MessageV2 } from "../../src/session/message-v2" import type { SessionPrompt } from "../../src/session/prompt" -import { MessageID, PartID } from "../../src/session/schema" +import { MessageID, PartID, type SessionID } from "../../src/session/schema" import { ModelID, ProviderID } from "../../src/provider/schema" import { AgentTool, sanitizeErrorMessage, type AgentPromptOps } from "../../src/tool/agent" import { SubagentRun } from "../../src/session/subagent-run" @@ -37,13 +37,12 @@ const it = testEffect( ), ) -const seed = Effect.fn("AgentToolTest.seed")(function* (title = "Pinned") { +const seedAssistant = Effect.fn("AgentToolTest.seedAssistant")(function* (sessionID: SessionID) { const session = yield* Session.Service - const chat = yield* session.create({ title }) const user = yield* session.updateMessage({ id: MessageID.ascending(), role: "user", - sessionID: chat.id, + sessionID, agent: "build", model: ref, time: { created: Date.now() }, @@ -52,7 +51,7 @@ const seed = Effect.fn("AgentToolTest.seed")(function* (title = "Pinned") { id: MessageID.ascending(), role: "assistant", parentID: user.id, - sessionID: chat.id, + sessionID, mode: "build", agent: "build", cost: 0, @@ -63,6 +62,13 @@ const seed = Effect.fn("AgentToolTest.seed")(function* (title = "Pinned") { time: { created: Date.now() }, } yield* session.updateMessage(assistant) + return assistant +}) + +const seed = Effect.fn("AgentToolTest.seed")(function* (title = "Pinned") { + const session = yield* Session.Service + const chat = yield* session.create({ title }) + const assistant = yield* seedAssistant(chat.id) return { chat, assistant } }) @@ -72,7 +78,7 @@ function stubOps(opts?: { interruptedSessions?: ReadonlySet }): AgentPromptOps { return { - cancel() {}, + cancel: () => Effect.void, resolvePromptParts: (template) => Effect.succeed([{ type: "text" as const, text: template }]), prompt: (input) => Effect.sync(() => { @@ -574,4 +580,116 @@ describe("tool.agent", () => { }, ), ) + + it.live("execute preserves parent external-directory and deny permissions in child sessions", () => + provideTmpdirInstance(() => + Effect.gen(function* () { + const sessions = yield* Session.Service + const chat = yield* sessions.create({ + title: "Pinned", + permission: [ + { permission: "external_directory", pattern: "/tmp/project/*", action: "allow" }, + { permission: "bash", pattern: "rm *", action: "deny" }, + { permission: "read", pattern: "*", action: "allow" }, + ], + }) + const assistant = yield* seedAssistant(chat.id) + const tool = yield* AgentTool + const def = yield* tool.init() + + const result = yield* def.execute( + { + description: "inspect bug", + prompt: "look into the cache key path", + subagent_type: "general", + }, + { + sessionID: chat.id, + messageID: assistant.id, + agent: "build", + abort: new AbortController().signal, + callID: "call_test_" + Math.random().toString(36).slice(2), + extra: { promptOps: stubOps(), bypassAgentCheck: true }, + messages: [], + metadata: () => Effect.void, + ask: () => Effect.void, + }, + ) + + const child = yield* sessions.get(result.metadata.sessionId!) + expect(child.permission).toContainEqual({ + permission: "external_directory", + pattern: "/tmp/project/*", + action: "allow", + }) + expect(child.permission).toContainEqual({ + permission: "bash", + pattern: "rm *", + action: "deny", + }) + expect(child.permission).not.toContainEqual({ + permission: "read", + pattern: "*", + action: "allow", + }) + }), + ), + ) + + it.live("execute cancels child session when abort signal fires", () => + provideTmpdirInstance(() => + Effect.gen(function* () { + const { chat, assistant } = yield* seed() + const tool = yield* AgentTool + const def = yield* tool.init() + const abort = new AbortController() + let resolveReady!: (input: SessionPrompt.PromptInput) => void + let resolveCancelled!: (sessionID: string) => void + const ready = new Promise((resolve) => { + resolveReady = resolve + }) + const cancelled = new Promise((resolve) => { + resolveCancelled = resolve + }) + const promptOps: AgentPromptOps = { + cancel: (sessionID) => Effect.sync(() => resolveCancelled(sessionID)), + resolvePromptParts: (template) => Effect.succeed([{ type: "text" as const, text: template }]), + prompt: (input) => + Effect.promise(() => { + resolveReady(input) + return cancelled + }).pipe(Effect.as(reply(input, "cancelled"))), + wasInterrupted: () => false, + } + + const fiber = yield* def + .execute( + { + description: "inspect bug", + prompt: "look into the cache key path", + subagent_type: "general", + }, + { + sessionID: chat.id, + messageID: assistant.id, + agent: "build", + abort: abort.signal, + callID: "call_test_" + Math.random().toString(36).slice(2), + extra: { promptOps, bypassAgentCheck: true }, + messages: [], + metadata: () => Effect.void, + ask: () => Effect.void, + }, + ) + .pipe(Effect.forkChild) + + const input = yield* Effect.promise(() => ready) + abort.abort() + expect(yield* Effect.promise(() => cancelled)).toBe(input.sessionID) + + const exit = yield* Fiber.await(fiber) + expect(Exit.isSuccess(exit)).toBe(true) + }), + ), + ) }) diff --git a/packages/opencode/test/tool/bash.test.ts b/packages/opencode/test/tool/bash.test.ts index 2277332ee..ba0503188 100644 --- a/packages/opencode/test/tool/bash.test.ts +++ b/packages/opencode/test/tool/bash.test.ts @@ -17,6 +17,7 @@ import { SessionID, MessageID } from "../../src/session/schema" import * as CrossSpawnSpawner from "@opencode-ai/core/cross-spawn-spawner" import { AppFileSystem } from "@opencode-ai/core/filesystem" import { Plugin } from "../../src/plugin" +import { Global } from "@opencode-ai/core/global" const runtime = ManagedRuntime.make( Layer.mergeAll( @@ -323,7 +324,8 @@ describe("tool.bash permissions", () => { const err = new Error("stop after permission") const requests: Array> = [] const file = process.platform === "win32" ? `${process.env.WINDIR!.replaceAll("\\", "/")}/*` : "/etc/*" - const want = process.platform === "win32" ? glob(path.join(process.env.WINDIR!, "*")) : "/etc/*" + const want = + process.platform === "win32" ? glob(path.join(process.env.WINDIR!, "*")) : path.join(await fs.promises.realpath("/etc"), "*") await expect( Effect.runPromise( bash.execute( @@ -411,6 +413,167 @@ describe("tool.bash permissions", () => { ) } + for (const item of ps) { + test( + `asks for real external_directory when PowerShell reads through tmp junction [${item.label}]`, + withShell(item, async () => { + await using outside = await tmpdir({ + init: async (dir) => { + await Bun.write(path.join(dir, "secret.txt"), "secret") + }, + }) + await using tmp = await tmpdir({ git: true }) + + const link = path.join(Global.Path.tmp, `bash-win-existing-${process.pid}-${Date.now()}`) + await fs.promises.rm(link, { recursive: true, force: true }) + await fs.promises.symlink(outside.path, link, "junction") + try { + await Instance.provide({ + directory: tmp.path, + fn: async () => { + const bash = await initBash() + const err = new Error("stop after permission") + const requests: Array> = [] + await expect( + Effect.runPromise( + bash.execute( + { + command: `Get-Content "${path.join(link, "secret.txt")}"`, + description: "Read tmp junction file", + }, + capture(requests, err), + ), + ), + ).rejects.toThrow(err.message) + const extDirReq = requests.find((r) => r.permission === "external_directory") + const expected = glob(path.join(await fs.promises.realpath(outside.path), "*")) + expect(extDirReq).toBeDefined() + expect(extDirReq!.patterns).toContain(expected) + }, + }) + } finally { + await fs.promises.rm(link, { recursive: true, force: true }) + } + }), + ) + + test( + `asks for real external_directory when PowerShell creates through tmp junction [${item.label}]`, + withShell(item, async () => { + await using outside = await tmpdir() + await using tmp = await tmpdir({ git: true }) + + const link = path.join(Global.Path.tmp, `bash-win-new-${process.pid}-${Date.now()}`) + await fs.promises.rm(link, { recursive: true, force: true }) + await fs.promises.symlink(outside.path, link, "junction") + try { + await Instance.provide({ + directory: tmp.path, + fn: async () => { + const bash = await initBash() + const err = new Error("stop after permission") + const requests: Array> = [] + await expect( + Effect.runPromise( + bash.execute( + { + command: `New-Item -ItemType File "${path.join(link, "new.txt")}"`, + description: "Create tmp junction file", + }, + capture(requests, err), + ), + ), + ).rejects.toThrow(err.message) + const extDirReq = requests.find((r) => r.permission === "external_directory") + const expected = glob(path.join(await fs.promises.realpath(outside.path), "*")) + expect(extDirReq).toBeDefined() + expect(extDirReq!.patterns).toContain(expected) + }, + }) + } finally { + await fs.promises.rm(link, { recursive: true, force: true }) + } + }), + ) + + test( + `asks for real external_directory when PowerShell path traverses after tmp junction [${item.label}]`, + withShell(item, async () => { + await using outside = await tmpdir() + await using tmp = await tmpdir({ git: true }) + + const link = path.join(Global.Path.tmp, `bash-win-dotdot-${process.pid}-${Date.now()}`) + await fs.promises.rm(link, { recursive: true, force: true }) + await fs.promises.symlink(outside.path, link, "junction") + try { + await Instance.provide({ + directory: tmp.path, + fn: async () => { + const bash = await initBash() + const err = new Error("stop after permission") + const requests: Array> = [] + const target = `${link}\\..\\new.txt` + await expect( + Effect.runPromise( + bash.execute( + { + command: `Set-Content "${target}" "x"`, + description: "Write parent junction file", + }, + capture(requests, err), + ), + ), + ).rejects.toThrow(err.message) + const extDirReq = requests.find((r) => r.permission === "external_directory") + const expected = glob(path.join(path.dirname(await fs.promises.realpath(outside.path)), "*")) + expect(extDirReq).toBeDefined() + expect(extDirReq!.patterns).toContain(expected) + }, + }) + } finally { + await fs.promises.rm(link, { recursive: true, force: true }) + } + }), + ) + + test( + `asks for real external_directory when PowerShell workdir traverses after junction [${item.label}]`, + withShell(item, async () => { + await using outside = await tmpdir() + await using tmp = await tmpdir({ git: true }) + + const link = path.join(tmp.path, "link") + await fs.promises.symlink(outside.path, link, "junction") + + await Instance.provide({ + directory: tmp.path, + fn: async () => { + const bash = await initBash() + const err = new Error("stop after permission") + const requests: Array> = [] + const workdir = `${link}\\..` + await expect( + Effect.runPromise( + bash.execute( + { + command: "Write-Output ok", + workdir, + description: "Echo from junction parent", + }, + capture(requests, err), + ), + ), + ).rejects.toThrow(err.message) + const extDirReq = requests.find((r) => r.permission === "external_directory") + const expected = glob(path.join(path.dirname(await fs.promises.realpath(outside.path)), "*")) + expect(extDirReq).toBeDefined() + expect(extDirReq!.patterns).toContain(expected) + }, + }) + }), + ) + } + for (const item of ps) { test( `asks for nested PowerShell command permissions [${item.label}]`, @@ -803,7 +966,7 @@ describe("tool.bash permissions", () => { ).rejects.toThrow(err.message) const extDirReq = requests.find((r) => r.permission === "external_directory") expect(extDirReq).toBeDefined() - expect(extDirReq!.patterns).toContain(glob(path.join(os.tmpdir(), "*"))) + expect(extDirReq!.patterns).toContain(glob(path.join(await fs.promises.realpath(os.tmpdir()), "*"))) }, }) }) @@ -945,6 +1108,155 @@ describe("tool.bash permissions", () => { }) }) + if (process.platform !== "win32") { + test("asks for real external_directory when bash reads through tmp symlink", async () => { + await using outside = await tmpdir({ + init: async (dir) => { + await Bun.write(path.join(dir, "secret.txt"), "secret") + }, + }) + await using tmp = await tmpdir({ git: true }) + + const link = path.join(Global.Path.tmp, `bash-existing-${process.pid}-${Date.now()}`) + await fs.promises.rm(link, { recursive: true, force: true }) + await fs.promises.symlink(outside.path, link, "dir") + try { + await Instance.provide({ + directory: tmp.path, + fn: async () => { + const bash = await initBash() + const err = new Error("stop after permission") + const requests: Array> = [] + await expect( + Effect.runPromise( + bash.execute( + { + command: `cat ${link}/secret.txt`, + description: "Read tmp symlink file", + }, + capture(requests, err), + ), + ), + ).rejects.toThrow(err.message) + const extDirReq = requests.find((r) => r.permission === "external_directory") + const expected = glob(path.join(await fs.promises.realpath(outside.path), "*")) + expect(extDirReq).toBeDefined() + expect(extDirReq!.patterns).toContain(expected) + }, + }) + } finally { + await fs.promises.rm(link, { recursive: true, force: true }) + } + }) + + test("asks for real external_directory when bash creates through tmp symlink", async () => { + await using outside = await tmpdir() + await using tmp = await tmpdir({ git: true }) + + const link = path.join(Global.Path.tmp, `bash-new-${process.pid}-${Date.now()}`) + await fs.promises.rm(link, { recursive: true, force: true }) + await fs.promises.symlink(outside.path, link, "dir") + try { + await Instance.provide({ + directory: tmp.path, + fn: async () => { + const bash = await initBash() + const err = new Error("stop after permission") + const requests: Array> = [] + await expect( + Effect.runPromise( + bash.execute( + { + command: `touch ${link}/new.txt`, + description: "Create tmp symlink file", + }, + capture(requests, err), + ), + ), + ).rejects.toThrow(err.message) + const extDirReq = requests.find((r) => r.permission === "external_directory") + const expected = glob(path.join(await fs.promises.realpath(outside.path), "*")) + expect(extDirReq).toBeDefined() + expect(extDirReq!.patterns).toContain(expected) + }, + }) + } finally { + await fs.promises.rm(link, { recursive: true, force: true }) + } + }) + + test("asks for real external_directory when bash path traverses after tmp symlink", async () => { + await using outside = await tmpdir() + await using tmp = await tmpdir({ git: true }) + + const link = path.join(Global.Path.tmp, `bash-dotdot-${process.pid}-${Date.now()}`) + await fs.promises.rm(link, { recursive: true, force: true }) + await fs.promises.symlink(outside.path, link, "dir") + try { + await Instance.provide({ + directory: tmp.path, + fn: async () => { + const bash = await initBash() + const err = new Error("stop after permission") + const requests: Array> = [] + const target = `${link}/../new.txt` + await expect( + Effect.runPromise( + bash.execute( + { + command: `touch ${target}`, + description: "Create parent symlink file", + }, + capture(requests, err), + ), + ), + ).rejects.toThrow(err.message) + const extDirReq = requests.find((r) => r.permission === "external_directory") + const expected = glob(path.join(path.dirname(await fs.promises.realpath(outside.path)), "*")) + expect(extDirReq).toBeDefined() + expect(extDirReq!.patterns).toContain(expected) + }, + }) + } finally { + await fs.promises.rm(link, { recursive: true, force: true }) + } + }) + + test("asks for real external_directory when bash workdir traverses after symlink", async () => { + await using outside = await tmpdir() + await using tmp = await tmpdir({ git: true }) + + const link = path.join(tmp.path, "link") + await fs.promises.symlink(outside.path, link, "dir") + + await Instance.provide({ + directory: tmp.path, + fn: async () => { + const bash = await initBash() + const err = new Error("stop after permission") + const requests: Array> = [] + const workdir = `${link}/..` + await expect( + Effect.runPromise( + bash.execute( + { + command: "echo ok", + workdir, + description: "Echo from symlink parent", + }, + capture(requests, err), + ), + ), + ).rejects.toThrow(err.message) + const extDirReq = requests.find((r) => r.permission === "external_directory") + const expected = glob(path.join(path.dirname(await fs.promises.realpath(outside.path)), "*")) + expect(extDirReq).toBeDefined() + expect(extDirReq!.patterns).toContain(expected) + }, + }) + }) + } + each("does not ask for external_directory permission when rm inside project", async () => { await using tmp = await tmpdir({ init: async (dir) => { diff --git a/packages/opencode/test/tool/external-directory.test.ts b/packages/opencode/test/tool/external-directory.test.ts index 9381d1981..d9ca9138e 100644 --- a/packages/opencode/test/tool/external-directory.test.ts +++ b/packages/opencode/test/tool/external-directory.test.ts @@ -1,9 +1,11 @@ import { describe, expect, test } from "bun:test" import path from "path" +import fs from "fs/promises" import { Effect } from "effect" +import { Global } from "@opencode-ai/core/global" import type * as Tool from "../../src/tool/tool" import { Instance } from "../../src/project/instance" -import { assertExternalDirectory } from "../../src/tool/external-directory" +import { assertExternalDirectory, resolveExternalPathForPermission } from "../../src/tool/external-directory" import { Filesystem } from "../../src/util/filesystem" import { tmpdir } from "../fixture/fixture" import type { Permission } from "../../src/permission" @@ -74,7 +76,8 @@ describe("tool.assertExternalDirectory", () => { const directory = "/tmp/project" const target = "/tmp/outside/file.txt" - const expected = glob(path.join(path.dirname(target), "*")) + const realTmp = process.platform === "win32" ? "/tmp" : await fs.realpath("/tmp") + const expected = glob(path.join(realTmp, "outside", "*")) await Instance.provide({ directory, @@ -89,12 +92,111 @@ describe("tool.assertExternalDirectory", () => { expect(req!.always).toEqual([expected]) }) + if (process.platform !== "win32") { + test("asks for the real target when a tmp child is a symlink", async () => { + const { requests, ctx } = makeCtx() + + await using outside = await tmpdir({ + init: async (dir) => { + await Bun.write(path.join(dir, "secret.txt"), "secret") + }, + }) + await using tmp = await tmpdir({ git: true }) + + const link = path.join(Global.Path.tmp, `external-directory-${process.pid}-${Date.now()}`) + await fs.rm(link, { recursive: true, force: true }) + await fs.symlink(outside.path, link, "dir") + try { + const target = path.join(link, "secret.txt") + const realTarget = path.join(await fs.realpath(outside.path), "secret.txt") + const expected = glob(path.join(path.dirname(realTarget), "*")) + + await Instance.provide({ + directory: tmp.path, + fn: async () => { + await assertExternalDirectory(ctx, target) + }, + }) + + const req = requests.find((r) => r.permission === "external_directory") + expect(req).toBeDefined() + expect(req!.patterns).toEqual([expected]) + expect(req!.always).toEqual([expected]) + expect(req!.metadata.filepath).toBe(target) + expect(req!.metadata.realpath).toBe(realTarget) + } finally { + await fs.rm(link, { recursive: true, force: true }) + } + }) + + test("asks for the real parent when a new tmp symlink child does not exist yet", async () => { + const { requests, ctx } = makeCtx() + + await using outside = await tmpdir() + await using tmp = await tmpdir({ git: true }) + + const link = path.join(Global.Path.tmp, `external-directory-new-${process.pid}-${Date.now()}`) + await fs.rm(link, { recursive: true, force: true }) + await fs.symlink(outside.path, link, "dir") + try { + const target = path.join(link, "new.txt") + const realTarget = path.join(await fs.realpath(outside.path), "new.txt") + const expected = glob(path.join(path.dirname(realTarget), "*")) + + await Instance.provide({ + directory: tmp.path, + fn: async () => { + await assertExternalDirectory(ctx, target) + }, + }) + + const req = requests.find((r) => r.permission === "external_directory") + expect(req).toBeDefined() + expect(req!.patterns).toEqual([expected]) + expect(req!.always).toEqual([expected]) + expect(req!.metadata.filepath).toBe(target) + expect(req!.metadata.realpath).toBe(realTarget) + } finally { + await fs.rm(link, { recursive: true, force: true }) + } + }) + + test("preserves symlink traversal before dot-dot normalization", async () => { + const { requests, ctx } = makeCtx() + + await using outside = await tmpdir() + await using tmp = await tmpdir({ git: true }) + + const link = path.join(tmp.path, "link") + await fs.symlink(outside.path, link, "dir") + + const target = `${link}/../external-directory-${process.pid}-${Date.now()}.txt` + const realTarget = path.join(path.dirname(await fs.realpath(outside.path)), path.basename(target)) + const expected = glob(path.join(path.dirname(realTarget), "*")) + + await Instance.provide({ + directory: tmp.path, + fn: async () => { + await assertExternalDirectory(ctx, target) + }, + }) + + const req = requests.find((r) => r.permission === "external_directory") + expect(req).toBeDefined() + expect(req!.patterns).toEqual([expected]) + expect(req!.always).toEqual([expected]) + expect(req!.metadata.filepath).toBe(target) + expect(req!.metadata.realpath).toBe(realTarget) + }) + } + test("uses target directory when kind=directory", async () => { const { requests, ctx } = makeCtx() const directory = "/tmp/project" const target = "/tmp/outside" - const expected = glob(path.join(target, "*")) + const realTmp = process.platform === "win32" ? "/tmp" : await fs.realpath("/tmp") + const expected = glob(path.join(realTmp, "outside", "*")) await Instance.provide({ directory, @@ -140,6 +242,44 @@ describe("tool.assertExternalDirectory", () => { }) }) + test("resolves Windows junction traversal before dot-dot normalization", async () => { + await withWin32Platform(async () => { + const junction = "C:\\project\\tmp\\link" + const outside = "D:\\outside" + const missing = "D:\\secret.txt" + const missingError = Object.assign(new Error("missing"), { code: "ENOENT" }) + const resolved = resolveExternalPathForPermission("C:\\project\\tmp\\link\\..\\secret.txt", "C:\\project", { + lstat: (candidate) => { + if (candidate.toLowerCase() === missing.toLowerCase()) throw missingError + return { + isSymbolicLink: () => candidate.toLowerCase() === junction.toLowerCase(), + } as ReturnType + }, + realpath: (candidate) => (candidate.toLowerCase() === junction.toLowerCase() ? outside : candidate), + }) + + expect(resolved).toBe(missing) + }) + }) + + test("resolves Windows drive-relative paths against the base path", async () => { + await withWin32Platform(async () => { + const expected = "C:\\project\\outside.txt" + const missingError = Object.assign(new Error("missing"), { code: "ENOENT" }) + const resolved = resolveExternalPathForPermission("C:..\\outside.txt", "C:\\project\\child", { + lstat: (candidate) => { + if (candidate.toLowerCase() === expected.toLowerCase()) throw missingError + return { + isSymbolicLink: () => false, + } as ReturnType + }, + realpath: (candidate) => candidate, + }) + + expect(resolved).toBe(expected) + }) + }) + if (process.platform === "win32") { test("normalizes Windows path variants to one glob", async () => { const { requests, ctx } = makeCtx() diff --git a/packages/opencode/test/tool/read.test.ts b/packages/opencode/test/tool/read.test.ts index 92d968b3b..41f67dd8d 100644 --- a/packages/opencode/test/tool/read.test.ts +++ b/packages/opencode/test/tool/read.test.ts @@ -335,6 +335,18 @@ describe("tool.read truncation", () => { }), ) + it.live("treats offset 0 as the first line", () => + Effect.gen(function* () { + const dir = yield* tmpdirScoped() + yield* put(path.join(dir, "offset-zero.txt"), "first\nsecond") + + const result = yield* exec(dir, { filePath: path.join(dir, "offset-zero.txt"), offset: 0, limit: 1 }) + + expect(result.output).toContain("1: first") + expect(result.output).not.toContain("2: second") + }), + ) + it.live("throws when offset is beyond end of file", () => Effect.gen(function* () { const dir = yield* tmpdirScoped() @@ -454,20 +466,20 @@ describe("tool.read truncation", () => { }), ) - it.live("detects modern image attachment media from file contents", () => + it.live("does not attach unsupported modern image formats", () => Effect.gen(function* () { const dir = yield* tmpdirScoped() yield* put(path.join(dir, "photo.bin"), Buffer.from([0x49, 0x49, 0x2a, 0x00, 0x00, 0x08, 0x00, 0x00])) yield* put(path.join(dir, "phone.dat"), ftyp("heic", "\0\0\0\0")) yield* put(path.join(dir, "screen.bin"), ftyp("avif", "\0\0\0\0")) - const tiff = yield* exec(dir, { filePath: path.join(dir, "photo.bin") }) - const heic = yield* exec(dir, { filePath: path.join(dir, "phone.dat") }) - const avif = yield* exec(dir, { filePath: path.join(dir, "screen.bin") }) + const tiff = yield* fail(dir, { filePath: path.join(dir, "photo.bin") }) + const heic = yield* fail(dir, { filePath: path.join(dir, "phone.dat") }) + const avif = yield* fail(dir, { filePath: path.join(dir, "screen.bin") }) - expect(tiff.attachments?.[0]?.mime).toBe("image/tiff") - expect(heic.attachments?.[0]?.mime).toBe("image/heic") - expect(avif.attachments?.[0]?.mime).toBe("image/avif") + expect(tiff.message).toContain("Cannot read binary file") + expect(heic.message).toContain("Cannot read binary file") + expect(avif.message).toContain("Cannot read binary file") }), ) @@ -483,20 +495,20 @@ describe("tool.read truncation", () => { }), ) - it.live("detects ftyp compatible-brand image media from file contents", () => + it.live("does not attach unsupported ftyp compatible-brand image media", () => Effect.gen(function* () { const dir = yield* tmpdirScoped() yield* put(path.join(dir, "phone.bin"), ftyp("isom", "\0\0\0\0", "heix")) yield* put(path.join(dir, "motion.bin"), ftyp("isom", "\0\0\0\0", "hevx")) yield* put(path.join(dir, "screen.dat"), ftyp("isom", "\0\0\0\0", "avis")) - const heix = yield* exec(dir, { filePath: path.join(dir, "phone.bin") }) - const hevx = yield* exec(dir, { filePath: path.join(dir, "motion.bin") }) - const avis = yield* exec(dir, { filePath: path.join(dir, "screen.dat") }) + const heix = yield* fail(dir, { filePath: path.join(dir, "phone.bin") }) + const hevx = yield* fail(dir, { filePath: path.join(dir, "motion.bin") }) + const avis = yield* fail(dir, { filePath: path.join(dir, "screen.dat") }) - expect(heix.attachments?.[0]?.mime).toBe("image/heic") - expect(hevx.attachments?.[0]?.mime).toBe("image/heic") - expect(avis.attachments?.[0]?.mime).toBe("image/avif") + expect(heix.message).toContain("Cannot read binary file") + expect(hevx.message).toContain("Cannot read binary file") + expect(avis.message).toContain("Cannot read binary file") }), ) diff --git a/packages/opencode/test/util/timeout.test.ts b/packages/opencode/test/util/timeout.test.ts index db1b03472..14601b081 100644 --- a/packages/opencode/test/util/timeout.test.ts +++ b/packages/opencode/test/util/timeout.test.ts @@ -18,4 +18,20 @@ describe("util.timeout", () => { await expect(withTimeout(slowPromise, 50)).rejects.toThrow("Operation timed out after 50ms") }) + + test("should clear timeout after promise rejection", async () => { + const original = globalThis.clearTimeout + let cleared = 0 + globalThis.clearTimeout = ((timer: Parameters[0]) => { + cleared++ + return original(timer) + }) as typeof clearTimeout + + try { + await expect(withTimeout(Promise.reject(new Error("boom")), 100)).rejects.toThrow("boom") + expect(cleared).toBe(1) + } finally { + globalThis.clearTimeout = original + } + }) }) diff --git a/packages/ui/src/components/session-diff.test.ts b/packages/ui/src/components/session-diff.test.ts index 463a72977..edaa15b84 100644 --- a/packages/ui/src/components/session-diff.test.ts +++ b/packages/ui/src/components/session-diff.test.ts @@ -34,4 +34,20 @@ describe("session diff", () => { expect(text(view, "deletions")).toBe("one\n") expect(text(view, "additions")).toBe("two\n") }) + + test("ignores malformed persisted patches", () => { + const diff = { + file: "a.ts", + patch: + "diff --git a/a.ts b/a.ts\nindex ff4ceb2..65a1de0 100644\n--- a/a.ts\n+++ b/a.ts\n@@ -1,3 +1,3 @@\n keep\n+add\n same\r", + additions: 1, + deletions: 1, + status: "modified" as const, + } + const view = normalize(diff) + + expect(view.patch).toBe(diff.patch) + expect(text(view, "deletions")).toBe("") + expect(text(view, "additions")).toBe("") + }) }) diff --git a/packages/ui/src/components/session-diff.ts b/packages/ui/src/components/session-diff.ts index a5fbdbc5c..324cfb221 100644 --- a/packages/ui/src/components/session-diff.ts +++ b/packages/ui/src/components/session-diff.ts @@ -27,26 +27,32 @@ const cache = new Map() function patch(diff: ReviewDiff) { if (typeof diff.patch === "string") { - const [patch] = parsePatch(diff.patch) + try { + const [patch] = parsePatch(diff.patch) + if (!patch) return { before: "", after: "", patch: diff.patch } - const beforeLines = [] - const afterLines = [] + const beforeLines = [] + const afterLines = [] - for (const hunk of patch.hunks) { - for (const line of hunk.lines) { - if (line.startsWith("-")) { - beforeLines.push(line.slice(1)) - } else if (line.startsWith("+")) { - afterLines.push(line.slice(1)) - } else { - // context line (starts with ' ') - beforeLines.push(line.slice(1)) - afterLines.push(line.slice(1)) + for (const hunk of patch.hunks) { + for (const line of hunk.lines) { + if (line.startsWith("-")) { + beforeLines.push(line.slice(1)) + } else if (line.startsWith("+")) { + afterLines.push(line.slice(1)) + } else { + // context line (starts with ' ') + beforeLines.push(line.slice(1)) + afterLines.push(line.slice(1)) + } } } - } - return { before: beforeLines.join("\n"), after: afterLines.join("\n"), patch: diff.patch } + const join = (lines: string[]) => (lines.length === 0 ? "" : lines.join("\n") + "\n") + return { before: join(beforeLines), after: join(afterLines), patch: diff.patch } + } catch { + return { before: "", after: "", patch: diff.patch } + } } return { before: "before" in diff && typeof diff.before === "string" ? diff.before : "",