From 2cca74ee25a87aa41ea8df791ee20e3f56db0dcd Mon Sep 17 00:00:00 2001 From: Yuhan Lei Date: Tue, 12 May 2026 10:33:33 +0800 Subject: [PATCH 1/4] feat(opencode): add expected_outputs protocol to bash tool --- packages/opencode/src/tool/bash.ts | 134 ++++++++++++++++++++++- packages/opencode/test/tool/bash.test.ts | 77 +++++++++++++ 2 files changed, 207 insertions(+), 4 deletions(-) diff --git a/packages/opencode/src/tool/bash.ts b/packages/opencode/src/tool/bash.ts index 0e6d266b9..03ca72c98 100644 --- a/packages/opencode/src/tool/bash.ts +++ b/packages/opencode/src/tool/bash.ts @@ -1,8 +1,10 @@ import { Schema } from "effect" import os from "os" import { createWriteStream } from "node:fs" +import nodefs from "node:fs/promises" import * as Tool from "./tool" import path from "path" +import crypto from "crypto" import DESCRIPTION from "./bash.txt" import { Log } from "../util" import { Instance, type InstanceContext } from "../project/instance" @@ -23,11 +25,14 @@ 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" +import { assertExternalDirectoryEffect, resolveExternalPathForPermission } from "./external-directory" import { InstanceState } from "@/effect/instance-state" +import * as Bom from "@/util/bom" +import { TurnChange, type FileState } from "@/session/turn-change" const MAX_METADATA_LENGTH = 30_000 const DEFAULT_TIMEOUT = Flag.OPENCODE_EXPERIMENTAL_BASH_DEFAULT_TIMEOUT_MS || 2 * 60 * 1000 +const TRACKED_OUTPUT_LIMIT = 20 * 1024 * 1024 const PS = new Set(["powershell", "pwsh"]) const CWD = new Set(["cd", "push-location", "set-location"]) const FILES = new Set([ @@ -61,6 +66,12 @@ export const Parameters = Schema.Struct({ workdir: Schema.optional(Schema.String).annotate({ description: `The working directory to run the command in. Defaults to the current directory. Use this instead of 'cd' commands.`, }), + expected_outputs: Schema.optional( + Schema.Array(Schema.String).annotate({ + description: + "Optional absolute or workdir-relative file paths that this command is expected to create or modify. The runtime will verify these paths after execution and register any real file changes in turn-change.", + }), + ), description: Schema.String.annotate({ description: "Clear, concise description of what this command does in 5-10 words. Examples:\nInput: ls\nOutput: Lists files in current directory\n\nInput: git status\nOutput: Shows working tree status\n\nInput: npm install\nOutput: Installs package dependencies\n\nInput: mkdir foo\nOutput: Creates directory 'foo'", @@ -253,6 +264,25 @@ function tail(text: string, maxLines: number, maxBytes: number) { } } +function textHash(content: string, bom?: boolean) { + return "sha256:" + crypto.createHash("sha256").update(`${bom ? "bom:1" : "bom:0"}\0${content}`).digest("hex") +} + +function binaryHash(buffer: Buffer) { + return "sha256-bin:" + crypto.createHash("sha256").update(buffer).digest("hex") +} + +function sameState(before: FileState, after: FileState) { + if (!before.exists && !after.exists) return true + return ( + before.exists === after.exists && + before.hash === after.hash && + before.bom === after.bom && + before.large === after.large && + before.binary === after.binary + ) +} + 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") @@ -333,9 +363,10 @@ export const BashTool = Tool.define( "bash", Effect.gen(function* () { const spawner = yield* ChildProcessSpawner - const fs = yield* AppFileSystem.Service + const afs = yield* AppFileSystem.Service const trunc = yield* Truncate.Service const plugin = yield* Plugin.Service + const turnChange = yield* TurnChange.Service const cygpath = Effect.fn("BashTool.cygpath")(function* (shell: string, text: string) { const lines = yield* spawner @@ -410,7 +441,7 @@ export const BashTool = Tool.define( if (!resolved) continue const permissionPath = resolveExternalPathForPermission(resolved, cwd) if (Instance.containsPath(permissionPath, instance)) continue - const dir = (yield* fs.isDir(permissionPath)) ? permissionPath : path.dirname(permissionPath) + const dir = (yield* afs.isDir(permissionPath)) ? permissionPath : path.dirname(permissionPath) scan.dirs.add(dir) } } @@ -443,6 +474,46 @@ export const BashTool = Tool.define( }) }) + const readTrackedState = Effect.fn("BashTool.readTrackedState")((file: string) => + Effect.promise(async () => { + try { + const stat = await nodefs.stat(file) + if (stat.isDirectory()) return { exists: true, restorable: false, hash: "directory", binary: true } satisfies FileState + if (stat.size > TRACKED_OUTPUT_LIMIT) { + return { + exists: true, + restorable: false, + hash: `large:${stat.size}:${stat.mtimeMs}`, + large: true, + } satisfies FileState + } + const buffer = await nodefs.readFile(file) + if (buffer.includes(0)) { + return { + exists: true, + restorable: false, + hash: binaryHash(buffer), + binary: true, + } satisfies FileState + } + const current = Bom.split(buffer.toString("utf-8")) + return { + exists: true, + content: current.text, + bom: current.bom, + hash: textHash(current.text, current.bom), + } satisfies FileState + } catch (err) { + if ((err as NodeJS.ErrnoException).code === "ENOENT") return { exists: false } satisfies FileState + return { + exists: true, + restorable: false, + hash: `error:${(err as NodeJS.ErrnoException).code ?? "unknown"}`, + } satisfies FileState + } + }).pipe(Effect.orDie), + ) + const run = Effect.fn("BashTool.run")(function* ( input: { shell: string @@ -653,7 +724,29 @@ export const BashTool = Tool.define( }), ) - return yield* run( + const trackedOutputs = yield* Effect.forEach(params.expected_outputs ?? [], (rawPath) => + Effect.gen(function* () { + const resolved = yield* resolveExecutionPath(rawPath, cwd, shell) + const normalized = AppFileSystem.normalizePath(resolved) + const filepath = (yield* assertExternalDirectoryEffect(ctx, normalized, { kind: "file" })) ?? normalized + return { + normalized: AppFileSystem.normalizePath(filepath), + path: filepath, + before: yield* readTrackedState(filepath), + } + }), + ).pipe( + Effect.map((items) => { + const deduped = new Map() + for (const item of items) { + if (deduped.has(item.normalized)) continue + deduped.set(item.normalized, { path: item.path, before: item.before }) + } + return Array.from(deduped.values()) + }), + ) + + const result = yield* run( { shell, name, @@ -665,6 +758,39 @@ export const BashTool = Tool.define( }, ctx, ) + + if (!trackedOutputs.length) return result + + const artifacts = yield* Effect.forEach(trackedOutputs, (tracked) => + Effect.gen(function* () { + const after = yield* readTrackedState(tracked.path) + const changed = !sameState(tracked.before, after) + if (changed) { + yield* turnChange.recordWrite({ + sessionID: ctx.sessionID, + messageID: ctx.messageID, + path: tracked.path, + before: tracked.before, + after, + }) + } + return { + path: tracked.path, + exists: after.exists, + changed, + ...(after.binary ? { binary: true } : {}), + ...(after.large ? { large: true } : {}), + } + }), + ) + + return { + ...result, + metadata: { + ...result.metadata, + artifacts, + }, + } }), } }) diff --git a/packages/opencode/test/tool/bash.test.ts b/packages/opencode/test/tool/bash.test.ts index ba0503188..0a455551c 100644 --- a/packages/opencode/test/tool/bash.test.ts +++ b/packages/opencode/test/tool/bash.test.ts @@ -18,6 +18,11 @@ 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" +import { TurnChange } from "../../src/session/turn-change" +import { Session as SessionNs } from "../../src/session" +import { MessageV2 } from "../../src/session/message-v2" +import { ModelID, ProviderID } from "../../src/provider/schema" +import { resetDatabase } from "../fixture/db" const runtime = ManagedRuntime.make( Layer.mergeAll( @@ -26,6 +31,7 @@ const runtime = ManagedRuntime.make( Plugin.defaultLayer, Truncate.defaultLayer, Agent.defaultLayer, + TurnChange.defaultLayer, ), ) @@ -44,6 +50,26 @@ const ctx = { ask: () => Effect.void, } +async function createTurn() { + const session = await SessionNs.create({ title: "bash external artifact" }) + const messageID = MessageID.make(`msg_bash_${Date.now()}_${Math.random().toString(36).slice(2, 8)}`) + await SessionNs.updateMessage({ + id: messageID, + sessionID: session.id, + role: "assistant", + parentID: MessageID.make("msg_user"), + time: { created: Date.now() }, + modelID: ModelID.make("test"), + providerID: ProviderID.make("test"), + mode: "", + agent: "build", + path: { cwd: "/", root: "/" }, + cost: 0, + tokens: { input: 0, output: 0, reasoning: 0, cache: { read: 0, write: 0 } }, + } as unknown as MessageV2.Info) + return { sessionID: session.id, messageID } +} + Shell.acceptable.reset() const quote = (text: string) => `"${text}"` const squote = (text: string) => `'${text}'` @@ -239,6 +265,57 @@ describe("tool.bash", () => { }) }) +describe("tool.bash expected_outputs", () => { + test("records a declared binary artifact in turn-change", async () => { + await resetDatabase() + await using tmp = await tmpdir() + await Instance.provide({ + directory: tmp.path, + fn: async () => { + const bash = await initBash() + const turn = await createTurn() + const target = path.join(tmp.path, "report.docx") + const script = path.join(tmp.path, "write-docx.cjs") + await fs.promises.writeFile( + script, + "require('node:fs').writeFileSync(process.argv[2], Buffer.from([80,75,3,4,0,1,2,3]))\n", + "utf-8", + ) + const command = `${PS.has(sh()) ? "& " : ""}${bin} ${quote(script.replaceAll("\\", "/"))} ${quote(target.replaceAll("\\", "/"))}` + const result = await Effect.runPromise( + bash.execute( + { + command, + expected_outputs: [target], + description: "Create binary report", + }, + { ...ctx, ...turn }, + ), + ) + + expect(result.metadata.exit).toBe(0) + expect( + (result.metadata as { artifacts?: Array<{ path: string; exists: boolean; changed: boolean; binary?: boolean }> }) + .artifacts, + ).toEqual([ + { path: target, exists: true, changed: true, binary: true }, + ]) + + const display = TurnChange.finalize(turn) + expect(display?.files).toEqual([ + { + path: "report.docx", + status: "added", + binary: true, + restoreAvailable: false, + expandable: false, + }, + ]) + }, + }) + }) +}) + describe("tool.bash permissions", () => { each("asks for bash permission with correct pattern", async () => { await using tmp = await tmpdir() From 643c57466fe91cb8d0daaa55db49d30776f14dd5 Mon Sep 17 00:00:00 2001 From: Yuhan Lei Date: Tue, 12 May 2026 10:33:46 +0800 Subject: [PATCH 2/4] test(opencode): cover bash external artifact edge cases --- packages/opencode/test/tool/bash.test.ts | 97 ++++++++++++++++++++++++ 1 file changed, 97 insertions(+) diff --git a/packages/opencode/test/tool/bash.test.ts b/packages/opencode/test/tool/bash.test.ts index 0a455551c..e06fbb5ca 100644 --- a/packages/opencode/test/tool/bash.test.ts +++ b/packages/opencode/test/tool/bash.test.ts @@ -314,6 +314,103 @@ describe("tool.bash expected_outputs", () => { }, }) }) + + test("records declared outputs even when command exits non-zero after writing", async () => { + await resetDatabase() + await using tmp = await tmpdir() + await Instance.provide({ + directory: tmp.path, + fn: async () => { + const bash = await initBash() + const turn = await createTurn() + const target = path.join(tmp.path, "partial.docx") + const script = path.join(tmp.path, "write-then-fail.cjs") + await fs.promises.writeFile( + script, + "require('node:fs').writeFileSync(process.argv[2], Buffer.from([80,75,3,4,0,9]))\nprocess.exit(1)\n", + "utf-8", + ) + const command = `${PS.has(sh()) ? "& " : ""}${bin} ${quote(script.replaceAll("\\", "/"))} ${quote(target.replaceAll("\\", "/"))}` + const result = await Effect.runPromise( + bash.execute( + { + command, + expected_outputs: [target], + description: "Write then fail", + }, + { ...ctx, ...turn }, + ), + ) + + expect(result.metadata.exit).toBe(1) + const display = TurnChange.finalize(turn) + expect(display?.files[0]).toMatchObject({ + path: "partial.docx", + status: "added", + binary: true, + expandable: false, + }) + }, + }) + }) + + test("does not record unchanged declared paths", async () => { + await resetDatabase() + await using tmp = await tmpdir({ + init: async (dir) => { + await fs.promises.writeFile(path.join(dir, "stable.txt"), "stable\n", "utf-8") + }, + }) + await Instance.provide({ + directory: tmp.path, + fn: async () => { + const bash = await initBash() + const turn = await createTurn() + const target = path.join(tmp.path, "stable.txt") + await Effect.runPromise( + bash.execute( + { + command: "echo noop", + expected_outputs: [target], + description: "Leave file unchanged", + }, + { ...ctx, ...turn }, + ), + ) + + expect(TurnChange.finalize(turn)).toBeUndefined() + }, + }) + }) + + test("preserves legacy behavior when expected_outputs is omitted", async () => { + await resetDatabase() + await using tmp = await tmpdir() + await Instance.provide({ + directory: tmp.path, + fn: async () => { + const bash = await initBash() + const turn = await createTurn() + const target = path.join(tmp.path, "undeclared.txt") + const script = path.join(tmp.path, "write-plain.cjs") + await fs.promises.writeFile(script, "require('node:fs').writeFileSync(process.argv[2], 'hello\\n')\n", "utf-8") + const command = `${PS.has(sh()) ? "& " : ""}${bin} ${quote(script.replaceAll("\\", "/"))} ${quote(target.replaceAll("\\", "/"))}` + const result = await Effect.runPromise( + bash.execute( + { + command, + description: "Write without declaration", + }, + { ...ctx, ...turn }, + ), + ) + + expect(result.metadata.exit).toBe(0) + expect((result.metadata as { artifacts?: unknown[] }).artifacts).toBeUndefined() + expect(TurnChange.finalize(turn)).toBeUndefined() + }, + }) + }) }) describe("tool.bash permissions", () => { From aa1b54d34209fdefae91b7bd7a20773be4e15355 Mon Sep 17 00:00:00 2001 From: Yuhan Lei Date: Tue, 12 May 2026 10:33:50 +0800 Subject: [PATCH 3/4] docs(skills): teach document-processing to declare expected outputs --- skills/document-processing/SKILL.md | 1 + 1 file changed, 1 insertion(+) diff --git a/skills/document-processing/SKILL.md b/skills/document-processing/SKILL.md index 33a7b86c3..bf1bf54da 100644 --- a/skills/document-processing/SKILL.md +++ b/skills/document-processing/SKILL.md @@ -48,6 +48,7 @@ Execution rules: - Prefer edits that preserve the original structure over destructive conversions. - If a conversion risks losing formulas, layout, comments, or branding, explain the tradeoff before finalizing. - Save the output in the current workspace unless the user gave a different path. +- When a third-party CLI writes a known output file through the `bash` tool, declare that absolute file path in `expected_outputs` so PawWork can register it in turn-change. ## Step 3: Verify From e467089d44ba79cb56166b454a44e4d990dc3c75 Mon Sep 17 00:00:00 2001 From: Yuhan Lei Date: Tue, 12 May 2026 10:43:29 +0800 Subject: [PATCH 4/4] fix(opencode): avoid false positives for indeterminate outputs --- packages/opencode/src/tool/bash.ts | 96 +++++++++++++++++------- packages/opencode/test/tool/bash.test.ts | 53 +++++++++++++ skills/document-processing/SKILL.md | 2 +- 3 files changed, 122 insertions(+), 29 deletions(-) diff --git a/packages/opencode/src/tool/bash.ts b/packages/opencode/src/tool/bash.ts index 03ca72c98..fd54433de 100644 --- a/packages/opencode/src/tool/bash.ts +++ b/packages/opencode/src/tool/bash.ts @@ -283,6 +283,13 @@ function sameState(before: FileState, after: FileState) { ) } +type TrackedOutputState = { + state: FileState + comparable: boolean + kind: "missing" | "file" | "directory" | "error" + errorCode?: string +} + 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") @@ -478,38 +485,62 @@ export const BashTool = Tool.define( Effect.promise(async () => { try { const stat = await nodefs.stat(file) - if (stat.isDirectory()) return { exists: true, restorable: false, hash: "directory", binary: true } satisfies FileState + if (stat.isDirectory()) { + return { + state: { exists: true, restorable: false, hash: "directory", binary: true } satisfies FileState, + comparable: true, + kind: "directory", + } satisfies TrackedOutputState + } if (stat.size > TRACKED_OUTPUT_LIMIT) { return { - exists: true, - restorable: false, - hash: `large:${stat.size}:${stat.mtimeMs}`, - large: true, - } satisfies FileState + state: { + exists: true, + restorable: false, + hash: `large:${stat.size}:${stat.mtimeMs}`, + large: true, + } satisfies FileState, + comparable: true, + kind: "file", + } satisfies TrackedOutputState } const buffer = await nodefs.readFile(file) if (buffer.includes(0)) { return { - exists: true, - restorable: false, - hash: binaryHash(buffer), - binary: true, - } satisfies FileState + state: { + exists: true, + restorable: false, + hash: binaryHash(buffer), + binary: true, + } satisfies FileState, + comparable: true, + kind: "file", + } satisfies TrackedOutputState } const current = Bom.split(buffer.toString("utf-8")) return { - exists: true, - content: current.text, - bom: current.bom, - hash: textHash(current.text, current.bom), - } satisfies FileState + state: { + exists: true, + content: current.text, + bom: current.bom, + hash: textHash(current.text, current.bom), + } satisfies FileState, + comparable: true, + kind: "file", + } satisfies TrackedOutputState } catch (err) { - if ((err as NodeJS.ErrnoException).code === "ENOENT") return { exists: false } satisfies FileState + const code = (err as NodeJS.ErrnoException).code + if (code === "ENOENT") return { state: { exists: false } satisfies FileState, comparable: true, kind: "missing" } satisfies TrackedOutputState return { - exists: true, - restorable: false, - hash: `error:${(err as NodeJS.ErrnoException).code ?? "unknown"}`, - } satisfies FileState + state: { + exists: true, + restorable: false, + hash: `error:${code ?? "unknown"}`, + } satisfies FileState, + comparable: false, + kind: "error", + ...(code ? { errorCode: code } : {}), + } satisfies TrackedOutputState } }).pipe(Effect.orDie), ) @@ -737,7 +768,7 @@ export const BashTool = Tool.define( }), ).pipe( Effect.map((items) => { - const deduped = new Map() + const deduped = new Map() for (const item of items) { if (deduped.has(item.normalized)) continue deduped.set(item.normalized, { path: item.path, before: item.before }) @@ -764,22 +795,31 @@ export const BashTool = Tool.define( const artifacts = yield* Effect.forEach(trackedOutputs, (tracked) => Effect.gen(function* () { const after = yield* readTrackedState(tracked.path) - const changed = !sameState(tracked.before, after) + const changed = tracked.before.comparable && after.comparable && !sameState(tracked.before.state, after.state) if (changed) { yield* turnChange.recordWrite({ sessionID: ctx.sessionID, messageID: ctx.messageID, path: tracked.path, - before: tracked.before, - after, + before: tracked.before.state, + after: after.state, }) } return { path: tracked.path, - exists: after.exists, + exists: after.state.exists, changed, - ...(after.binary ? { binary: true } : {}), - ...(after.large ? { large: true } : {}), + ...(after.kind === "directory" ? { directory: true } : {}), + ...(after.state.binary && after.kind !== "directory" ? { binary: true } : {}), + ...(after.state.large ? { large: true } : {}), + ...(!tracked.before.comparable || !after.comparable + ? { + comparable: false, + errorCode: + ("errorCode" in tracked.before ? tracked.before.errorCode : undefined) ?? + ("errorCode" in after ? after.errorCode : undefined), + } + : {}), } }), ) diff --git a/packages/opencode/test/tool/bash.test.ts b/packages/opencode/test/tool/bash.test.ts index e06fbb5ca..ef35777d8 100644 --- a/packages/opencode/test/tool/bash.test.ts +++ b/packages/opencode/test/tool/bash.test.ts @@ -1,6 +1,8 @@ import { describe, expect, test } from "bun:test" +import { spyOn } from "bun:test" import { Effect, Layer, ManagedRuntime } from "effect" import fs from "node:fs" +import nodefs from "node:fs/promises" import os from "os" import path from "path" import { existsSync } from "node:fs" @@ -411,6 +413,57 @@ describe("tool.bash expected_outputs", () => { }, }) }) + + test("does not record false positives when the before state is indeterminate", async () => { + await resetDatabase() + await using tmp = await tmpdir({ + init: async (dir) => { + const target = path.join(dir, "restricted.txt") + await fs.promises.writeFile(target, "secret\n", "utf-8") + }, + }) + await Instance.provide({ + directory: tmp.path, + fn: async () => { + const bash = await initBash() + const turn = await createTurn() + const target = path.join(tmp.path, "restricted.txt") + const originalReadFile = nodefs.readFile + const readSpy = spyOn(nodefs as any, "readFile").mockImplementationOnce(async (...args: any[]) => { + const filepath = typeof args[0] === "string" ? args[0] : String(args[0]) + if (filepath === target) { + const err = Object.assign(new Error("denied"), { code: "EACCES" }) + throw err + } + return await (originalReadFile as any)(...args) + }) + try { + const result = await Effect.runPromise( + bash.execute( + { + command: `rm ${quote(target.replaceAll("\\", "/"))}`, + expected_outputs: [target], + description: "Delete unreadable file", + }, + { ...ctx, ...turn }, + ), + ) + + expect(result.metadata.exit).toBe(0) + expect( + ( + result.metadata as { + artifacts?: Array<{ path: string; exists: boolean; changed: boolean; comparable?: boolean; errorCode?: string }> + } + ).artifacts, + ).toEqual([{ path: target, exists: false, changed: false, comparable: false, errorCode: "EACCES" }]) + expect(TurnChange.finalize(turn)).toBeUndefined() + } finally { + readSpy.mockRestore() + } + }, + }) + }) }) describe("tool.bash permissions", () => { diff --git a/skills/document-processing/SKILL.md b/skills/document-processing/SKILL.md index bf1bf54da..14b1cfeed 100644 --- a/skills/document-processing/SKILL.md +++ b/skills/document-processing/SKILL.md @@ -48,7 +48,7 @@ Execution rules: - Prefer edits that preserve the original structure over destructive conversions. - If a conversion risks losing formulas, layout, comments, or branding, explain the tradeoff before finalizing. - Save the output in the current workspace unless the user gave a different path. -- When a third-party CLI writes a known output file through the `bash` tool, declare that absolute file path in `expected_outputs` so PawWork can register it in turn-change. +- When a third-party CLI writes a known output file through the `bash` tool, declare that file path (absolute or relative to `workdir`) in `expected_outputs` so PawWork can register it in turn-change. ## Step 3: Verify