diff options
Diffstat (limited to 'packages')
22 files changed, 581 insertions, 395 deletions
diff --git a/packages/tool-edit-file/package.json b/packages/tool-edit-file/package.json index 7194cce..4bfac38 100644 --- a/packages/tool-edit-file/package.json +++ b/packages/tool-edit-file/package.json @@ -7,6 +7,7 @@ "types": "dist/index.d.ts", "dependencies": { "@dispatch/kernel": "workspace:*", + "@dispatch/exec-backend": "workspace:*", "@dispatch/lsp": "workspace:*" } } diff --git a/packages/tool-edit-file/src/edit-file.test.ts b/packages/tool-edit-file/src/edit-file.test.ts index 5ef8376..9341102 100644 --- a/packages/tool-edit-file/src/edit-file.test.ts +++ b/packages/tool-edit-file/src/edit-file.test.ts @@ -1,9 +1,15 @@ import { mkdtemp, readFile, rm, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; +import { localExecBackend } from "@dispatch/exec-backend"; import { createLogger, type ToolExecuteContext } from "@dispatch/kernel"; import { afterEach, beforeEach, describe, expect, it } from "vitest"; -import { computeReplacement, createEditFileTool, validateArgs } from "./edit-file.js"; +import { + computeReplacement, + createEditFileTool, + type DiagnosticsHook, + validateArgs, +} from "./edit-file.js"; function stubCtx(overrides?: Partial<ToolExecuteContext>): ToolExecuteContext { return { @@ -19,6 +25,30 @@ function stubCtx(overrides?: Partial<ToolExecuteContext>): ToolExecuteContext { }; } +/** No-op diagnostics — the post-edit LSP hook returning "no diagnostics". */ +const noopDiagnostics: DiagnosticsHook = async () => ({ + formatted: "", + slow: false, + timedOut: false, +}); + +/** + * Build an edit_file tool wired to the real local ExecBackend (node:fs, + * behavior-identical to today's inline calls) and a no-op diagnostics hook. + * No `@dispatch/*` mocking — the real fs edge is exercised, matching the + * constitution's strict-core rule. Tests that need a real diagnostics hook + * build the tool inline. + */ +function makeTool( + diagnostics: DiagnosticsHook = noopDiagnostics, +): ReturnType<typeof createEditFileTool> { + return createEditFileTool({ + resolveBackend: () => localExecBackend, + workdir, + diagnostics, + }); +} + let workdir: string; beforeEach(async () => { @@ -145,7 +175,7 @@ describe("createEditFileTool", () => { const filePath = join(workdir, "test.txt"); await writeFile(filePath, "hello world\n", "utf8"); - const tool = createEditFileTool(workdir); + const tool = makeTool(); const result = await tool.execute( { path: "test.txt", oldString: "world", newString: "there" }, stubCtx(), @@ -162,7 +192,7 @@ describe("createEditFileTool", () => { const filePath = join(workdir, "test.txt"); await writeFile(filePath, "aaa\n", "utf8"); - const tool = createEditFileTool(workdir); + const tool = makeTool(); const result = await tool.execute( { path: "test.txt", oldString: "a", newString: "b", replaceAll: true }, stubCtx(), @@ -179,7 +209,7 @@ describe("createEditFileTool", () => { const filePath = join(workdir, "test.txt"); await writeFile(filePath, "hello\n", "utf8"); - const tool = createEditFileTool(workdir); + const tool = makeTool(); const result = await tool.execute( { path: "test.txt", oldString: "xyz", newString: "abc" }, stubCtx(), @@ -193,7 +223,7 @@ describe("createEditFileTool", () => { const filePath = join(workdir, "test.txt"); await writeFile(filePath, "abc abc abc\n", "utf8"); - const tool = createEditFileTool(workdir); + const tool = makeTool(); const result = await tool.execute( { path: "test.txt", oldString: "abc", newString: "xyz" }, stubCtx(), @@ -207,7 +237,7 @@ describe("createEditFileTool", () => { const filePath = join(workdir, "test.txt"); await writeFile(filePath, "hello\n", "utf8"); - const tool = createEditFileTool(workdir); + const tool = makeTool(); const result = await tool.execute( { path: "test.txt", oldString: "hello", newString: "hello" }, stubCtx(), @@ -218,7 +248,7 @@ describe("createEditFileTool", () => { }); it("errors / not-found for a nonexistent file", async () => { - const tool = createEditFileTool(workdir); + const tool = makeTool(); const result = await tool.execute( { path: "nonexistent.txt", oldString: "a", newString: "b" }, stubCtx(), @@ -234,7 +264,7 @@ describe("createEditFileTool", () => { const filePath = join(ctxDir, "ctx-file.txt"); await writeFile(filePath, "hello world", "utf8"); - const tool = createEditFileTool(workdir); + const tool = makeTool(); const result = await tool.execute( { path: "ctx-file.txt", oldString: "world", newString: "there" }, stubCtx({ cwd: ctxDir }), @@ -254,7 +284,7 @@ describe("createEditFileTool", () => { const filePath = join(workdir, "baked-file.txt"); await writeFile(filePath, "hello world", "utf8"); - const tool = createEditFileTool(workdir); + const tool = makeTool(); const ctx = stubCtx(); expect(ctx.cwd).toBeUndefined(); const result = await tool.execute( @@ -267,7 +297,7 @@ describe("createEditFileTool", () => { }); it("never throws on bad input (always returns ToolResult)", async () => { - const tool = createEditFileTool(workdir); + const tool = makeTool(); const inputs = [null, undefined, 42, "string", {}, { path: "" }, { path: 123 }]; for (const input of inputs) { @@ -278,12 +308,12 @@ describe("createEditFileTool", () => { }); it("concurrencySafe is false", () => { - const tool = createEditFileTool(workdir); + const tool = makeTool(); expect(tool.concurrencySafe).toBe(false); }); it("has correct name and parameters shape", () => { - const tool = createEditFileTool(workdir); + const tool = makeTool(); expect(tool.name).toBe("edit_file"); expect(tool.parameters.type).toBe("object"); expect(tool.parameters.required).toEqual(["path", "oldString", "newString"]); @@ -292,4 +322,115 @@ describe("createEditFileTool", () => { expect(tool.parameters.properties?.newString?.type).toBe("string"); expect(tool.parameters.properties?.replaceAll?.type).toBe("boolean"); }); + + it("appends LSP diagnostics to the result when local and errors exist", async () => { + const filePath = join(workdir, "diag.txt"); + await writeFile(filePath, "hello world\n", "utf8"); + + let called = false; + const diagnostics: DiagnosticsHook = async (opts) => { + called = true; + expect(opts.text).toBe("hello there\n"); + return { formatted: "⚠️ 2 errors", slow: false, timedOut: false }; + }; + const tool = makeTool(diagnostics); + + const result = await tool.execute( + { path: "diag.txt", oldString: "world", newString: "there" }, + stubCtx(), + ); + + expect(called).toBe(true); + expect(result.isError).toBeUndefined(); + expect(result.content).toContain("Replaced 1 occurrence"); + expect(result.content).toContain("⚠️ 2 errors"); + }); + + it("appends the slow-diagnostics notice when LSP is slow", async () => { + const filePath = join(workdir, "slow.txt"); + await writeFile(filePath, "hello\n", "utf8"); + + const diagnostics: DiagnosticsHook = async () => ({ + formatted: "", + slow: true, + timedOut: false, + }); + const tool = makeTool(diagnostics); + + const result = await tool.execute( + { path: "slow.txt", oldString: "hello", newString: "hi" }, + stubCtx(), + ); + + expect(result.isError).toBeUndefined(); + expect(result.content).toContain("Replaced 1 occurrence"); + expect(result.content).toContain("LSP is taking unusually long"); + }); + + it("calls LSP diagnostics when local (computerId undefined)", async () => { + const filePath = join(workdir, "local.txt"); + await writeFile(filePath, "hello\n", "utf8"); + + let called = false; + const diagnostics: DiagnosticsHook = async () => { + called = true; + return { formatted: "", slow: false, timedOut: false }; + }; + const tool = makeTool(diagnostics); + + const result = await tool.execute( + { path: "local.txt", oldString: "hello", newString: "hi" }, + stubCtx(), // computerId omitted → undefined → local + ); + + expect(called).toBe(true); + expect(result.isError).toBeUndefined(); + expect(result.content).toBe('Replaced 1 occurrence in "local.txt".'); + }); + + it("skips LSP diagnostics when computerId is set (remote)", async () => { + const filePath = join(workdir, "remote.txt"); + await writeFile(filePath, "hello\n", "utf8"); + + let called = false; + const diagnostics: DiagnosticsHook = async () => { + called = true; + return { formatted: "DIAG-SHOULD-NOT-APPEAR", slow: false, timedOut: false }; + }; + const tool = makeTool(diagnostics); + + const result = await tool.execute( + { path: "remote.txt", oldString: "hello", newString: "hi" }, + stubCtx({ computerId: "remote-host" }), + ); + + // Remote: the diagnostics hook is never invoked (LSP servers are local + // processes that can't see remote files over SFTP). + expect(called).toBe(false); + expect(result.isError).toBeUndefined(); + // The edit itself still succeeded against the (local) backend. + expect(result.content).toBe('Replaced 1 occurrence in "remote.txt".'); + expect(result.content).not.toContain("DIAG-SHOULD-NOT-APPEAR"); + + const content = await readFile(filePath, "utf8"); + expect(content).toBe("hi\n"); + }); + + it("swallows a throwing diagnostics hook (edit already succeeded)", async () => { + const filePath = join(workdir, "throw.txt"); + await writeFile(filePath, "hello\n", "utf8"); + + const diagnostics: DiagnosticsHook = async () => { + throw new Error("LSP exploded"); + }; + const tool = makeTool(diagnostics); + + const result = await tool.execute( + { path: "throw.txt", oldString: "hello", newString: "hi" }, + stubCtx(), + ); + + expect(result.isError).toBeUndefined(); + expect(result.content).toBe('Replaced 1 occurrence in "throw.txt".'); + }); }); diff --git a/packages/tool-edit-file/src/edit-file.ts b/packages/tool-edit-file/src/edit-file.ts index 1719ea3..e588f66 100644 --- a/packages/tool-edit-file/src/edit-file.ts +++ b/packages/tool-edit-file/src/edit-file.ts @@ -1,5 +1,5 @@ -import { readFile, writeFile } from "node:fs/promises"; import { resolve } from "node:path"; +import type { ExecBackend, ExecBackendResolver } from "@dispatch/exec-backend"; import type { ToolContract, ToolResult } from "@dispatch/kernel"; // --- Pure types --- @@ -123,16 +123,29 @@ export type DiagnosticsHook = (opts: { // --- Shell / edge --- /** - * Factory: create an edit_file ToolContract bound to a working directory. - * The working directory is injected so the tool is testable. - * `diagnostics` is optional — when provided, errors+warnings from LSP servers - * are appended to successful edit results (only when errors exist). + * Factory: create an edit_file ToolContract. + * + * `resolveBackend` is the injected seam: each `execute` resolves an + * `ExecBackend` from `ctx.computerId` (undefined → local `node:fs`; a set + * id → a remote SSH backend in a later wave). The tool programs against the + * `ExecBackend` surface, never `node:fs` directly, so it is transport-agnostic. + * + * `workdir` is the fallback base directory when `ctx.cwd` is omitted. It is + * injected so the tool is testable; `execute` prefers `ctx.cwd` when present. + * + * `diagnostics` is the post-edit LSP hook (errors+warnings from LSP servers + * are appended to successful edit results, only when errors exist). It is + * invoked LAZILY at edit time — the extension defers the LSP service lookup so + * it resolves after LSP activates. When `ctx.computerId` is set (REMOTE) the + * diagnostics call is skipped: LSP servers are local processes that can't see + * remote files over SFTP, so the no-LSP degradation path is used instead. */ -export function createEditFileTool( - workingDirectory: string, - diagnostics?: DiagnosticsHook, -): ToolContract { - const workdir = resolve(workingDirectory); +export function createEditFileTool(deps: { + readonly resolveBackend: ExecBackendResolver; + readonly workdir?: string; + readonly diagnostics: DiagnosticsHook; +}): ToolContract { + const workdir = deps.workdir !== undefined ? resolve(deps.workdir) : undefined; return { name: "edit_file", @@ -173,12 +186,21 @@ export function createEditFileTool( const { path: relPath, oldString, newString, replaceAll } = validated; const effectiveBase = ctx.cwd ? resolve(ctx.cwd) : workdir; + if (effectiveBase === undefined) { + return { + content: + "Error: No working directory (neither ctx.cwd nor a baked workdir was provided).", + isError: true, + }; + } const resolvedPath = resolve(effectiveBase, relPath); + const backend: ExecBackend = deps.resolveBackend(ctx.computerId); + // Read the file. let content: string; try { - content = await readFile(resolvedPath, "utf8"); + content = await backend.readFile(resolvedPath); } catch (err: unknown) { const code = (err as NodeJS.ErrnoException).code; if (code === "ENOENT") { @@ -215,7 +237,7 @@ export function createEditFileTool( // Write the modified content back. try { - await writeFile(resolvedPath, result.content, "utf8"); + await backend.writeFile(resolvedPath, result.content); } catch (err: unknown) { return { content: `Error writing file: ${err instanceof Error ? err.message : String(err)}`, @@ -228,28 +250,43 @@ export function createEditFileTool( // After a successful edit, query LSP diagnostics (if available). // Only append if there are actual errors/warnings (no noise on clean edits). + const diagnostics = deps.diagnostics; if (diagnostics) { - try { - const cwd = ctx.cwd ?? process.cwd(); - const diag = await diagnostics({ - filePath: resolvedPath, - text: result.content, - cwd, - }); - const suffix: string[] = []; - if (diag.slow) { - suffix.push( - "⚠️ LSP is taking unusually long. If this happens more than once, raise it to the user.", - ); - } - if (diag.formatted) { - suffix.push(diag.formatted); - } - if (suffix.length > 0) { - baseContent += `\n\n${suffix.join("\n\n")}`; + let diag: { + readonly formatted: string; + readonly slow: boolean; + readonly timedOut: boolean; + }; + if (ctx.computerId !== undefined) { + // REMOTE: LSP servers are local processes that can't see remote + // files over SFTP — skip the diagnostics call (the no-LSP + // degradation path). Forward-compatible: computerId is always + // undefined this wave, so behavior is byte-identical to today. + diag = { formatted: "", slow: false, timedOut: false }; + } else { + try { + const cwd = ctx.cwd ?? process.cwd(); + diag = await diagnostics({ + filePath: resolvedPath, + text: result.content, + cwd, + }); + } catch { + // LSP diagnostics failure is non-fatal — the edit already succeeded. + diag = { formatted: "", slow: false, timedOut: false }; } - } catch { - // LSP diagnostics failure is non-fatal — the edit already succeeded. + } + const suffix: string[] = []; + if (diag.slow) { + suffix.push( + "⚠️ LSP is taking unusually long. If this happens more than once, raise it to the user.", + ); + } + if (diag.formatted) { + suffix.push(diag.formatted); + } + if (suffix.length > 0) { + baseContent += `\n\n${suffix.join("\n\n")}`; } } diff --git a/packages/tool-edit-file/src/extension.ts b/packages/tool-edit-file/src/extension.ts index 3929634..2eaa0e9 100644 --- a/packages/tool-edit-file/src/extension.ts +++ b/packages/tool-edit-file/src/extension.ts @@ -1,3 +1,4 @@ +import { execBackendHandle } from "@dispatch/exec-backend"; import type { Extension } from "@dispatch/kernel"; import { type LspService, lspServiceHandle } from "@dispatch/lsp"; import { createEditFileTool, type DiagnosticsHook } from "./edit-file.js"; @@ -12,8 +13,15 @@ export const extension: Extension = { activation: "eager", capabilities: { fs: true }, contributes: { tools: ["edit_file"] }, + // Host activates exec-backend first → host.getService at activation is safe. + // LSP stays lazy (looked up at edit time, not activation): the LSP extension + // activates AFTER us in the CORE_EXTENSIONS array, so resolving it here would + // throw; the diagnostics hook below defers the lookup to execute(). + dependsOn: ["exec-backend"], }, activate(host) { + const resolveBackend = host.getService(execBackendHandle); + // Lazy LSP lookup: the LSP extension activates AFTER us in the // CORE_EXTENSIONS array, so host.getService would throw at activation // time. Instead, defer the lookup to edit time — by then all extensions @@ -38,6 +46,6 @@ export const extension: Extension = { }); }; - host.defineTool(createEditFileTool(process.cwd(), diagnostics)); + host.defineTool(createEditFileTool({ resolveBackend, workdir: process.cwd(), diagnostics })); }, }; diff --git a/packages/tool-edit-file/tsconfig.json b/packages/tool-edit-file/tsconfig.json index 38a7610..e4ee1eb 100644 --- a/packages/tool-edit-file/tsconfig.json +++ b/packages/tool-edit-file/tsconfig.json @@ -2,5 +2,5 @@ "extends": "../../tsconfig.base.json", "compilerOptions": { "rootDir": "src", "outDir": "dist", "composite": true }, "include": ["src/**/*.ts"], - "references": [{ "path": "../kernel" }, { "path": "../lsp" }] + "references": [{ "path": "../kernel" }, { "path": "../exec-backend" }, { "path": "../lsp" }] } diff --git a/packages/tool-read-file/package.json b/packages/tool-read-file/package.json index 3a98fa7..ffb974d 100644 --- a/packages/tool-read-file/package.json +++ b/packages/tool-read-file/package.json @@ -6,6 +6,7 @@ "main": "dist/index.js", "types": "dist/index.d.ts", "dependencies": { - "@dispatch/kernel": "workspace:*" + "@dispatch/kernel": "workspace:*", + "@dispatch/exec-backend": "workspace:*" } } diff --git a/packages/tool-read-file/src/extension.ts b/packages/tool-read-file/src/extension.ts index 8c3a064..5a0b7c5 100644 --- a/packages/tool-read-file/src/extension.ts +++ b/packages/tool-read-file/src/extension.ts @@ -1,3 +1,4 @@ +import { execBackendHandle } from "@dispatch/exec-backend"; import type { Extension } from "@dispatch/kernel"; import { createReadFileTool } from "./read-file.js"; @@ -11,8 +12,11 @@ export const extension: Extension = { activation: "eager", capabilities: { fs: true }, contributes: { tools: ["read_file"] }, + // Host activates exec-backend first → host.getService at activation is safe. + dependsOn: ["exec-backend"], }, activate(host) { - host.defineTool(createReadFileTool(process.cwd())); + const resolveBackend = host.getService(execBackendHandle); + host.defineTool(createReadFileTool({ resolveBackend, workdir: process.cwd() })); }, }; diff --git a/packages/tool-read-file/src/read-file.test.ts b/packages/tool-read-file/src/read-file.test.ts index 619ba34..bac5902 100644 --- a/packages/tool-read-file/src/read-file.test.ts +++ b/packages/tool-read-file/src/read-file.test.ts @@ -1,6 +1,7 @@ import { mkdir, mkdtemp, rm, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; +import { localExecBackend } from "@dispatch/exec-backend"; import { createLogger, type ToolExecuteContext } from "@dispatch/kernel"; import { afterEach, beforeEach, describe, expect, it } from "vitest"; import { @@ -25,6 +26,15 @@ function stubCtx(overrides?: Partial<ToolExecuteContext>): ToolExecuteContext { }; } +/** + * Build a read_file tool wired to the real local ExecBackend (node:fs, + * behavior-identical to today's inline calls). No `@dispatch/*` mocking — the + * real fs edge is exercised, matching the constitution's strict-core rule. + */ +function makeTool(workdir: string) { + return createReadFileTool({ resolveBackend: () => localExecBackend, workdir }); +} + let workdir: string; beforeEach(async () => { @@ -151,7 +161,7 @@ describe("createReadFileTool", () => { const filePath = join(workdir, "hello.txt"); await writeFile(filePath, "hello\nworld\n", "utf8"); - const tool = createReadFileTool(workdir); + const tool = makeTool(workdir); const result = await tool.execute({ path: "hello.txt" }, stubCtx()); expect(result.isError).toBeUndefined(); @@ -163,7 +173,7 @@ describe("createReadFileTool", () => { const filePath = join(workdir, "lines.txt"); await writeFile(filePath, "a\nb\nc\nd\ne\n", "utf8"); - const tool = createReadFileTool(workdir); + const tool = makeTool(workdir); const result = await tool.execute({ path: "lines.txt", offset: 2, limit: 2 }, stubCtx()); expect(result.isError).toBeUndefined(); @@ -171,7 +181,7 @@ describe("createReadFileTool", () => { }); it("returns error for missing file", async () => { - const tool = createReadFileTool(workdir); + const tool = makeTool(workdir); const result = await tool.execute({ path: "nonexistent.txt" }, stubCtx()); expect(result.isError).toBe(true); @@ -182,7 +192,7 @@ describe("createReadFileTool", () => { const filePath = join(workdir, "empty.txt"); await writeFile(filePath, "", "utf8"); - const tool = createReadFileTool(workdir); + const tool = makeTool(workdir); const result = await tool.execute({ path: "empty.txt" }, stubCtx()); expect(result.isError).toBeUndefined(); @@ -194,7 +204,7 @@ describe("createReadFileTool", () => { const filePath = join(workdir, "short.txt"); await writeFile(filePath, "one\n", "utf8"); - const tool = createReadFileTool(workdir); + const tool = makeTool(workdir); const result = await tool.execute({ path: "short.txt", offset: 100 }, stubCtx()); expect(result.isError).toBe(true); @@ -202,7 +212,7 @@ describe("createReadFileTool", () => { }); it("never throws on bad input (always returns ToolResult)", async () => { - const tool = createReadFileTool(workdir); + const tool = makeTool(workdir); const inputs = [null, undefined, 42, "string", {}, { path: "" }, { path: 123 }]; for (const input of inputs) { @@ -213,12 +223,12 @@ describe("createReadFileTool", () => { }); it("concurrencySafe is true", () => { - const tool = createReadFileTool(workdir); + const tool = makeTool(workdir); expect(tool.concurrencySafe).toBe(true); }); it("has correct name and parameters shape", () => { - const tool = createReadFileTool(workdir); + const tool = makeTool(workdir); expect(tool.name).toBe("read_file"); expect(tool.parameters.type).toBe("object"); expect(tool.parameters.required).toEqual(["path"]); @@ -231,7 +241,7 @@ describe("createReadFileTool", () => { const filePath = join(ctxDir, "ctx-file.txt"); await writeFile(filePath, "from ctx cwd", "utf8"); - const tool = createReadFileTool(workdir); // baked workdir is different + const tool = makeTool(workdir); // baked workdir is different const result = await tool.execute({ path: "ctx-file.txt" }, stubCtx({ cwd: ctxDir })); expect(result.isError).toBeUndefined(); @@ -245,7 +255,7 @@ describe("createReadFileTool", () => { const filePath = join(workdir, "baked-file.txt"); await writeFile(filePath, "from baked workdir", "utf8"); - const tool = createReadFileTool(workdir); + const tool = makeTool(workdir); const ctx = stubCtx(); // Ensure cwd is undefined expect(ctx.cwd).toBeUndefined(); @@ -260,7 +270,7 @@ describe("createReadFileTool", () => { await writeFile(join(workdir, "zebra.txt"), "z", "utf8"); await writeFile(join(workdir, "alpha.txt"), "a", "utf8"); - const tool = createReadFileTool(workdir); + const tool = makeTool(workdir); const result = await tool.execute({ path: "." }, stubCtx()); expect(result.isError).toBeUndefined(); @@ -270,7 +280,7 @@ describe("createReadFileTool", () => { it("returns empty-directory message for an empty dir", async () => { await mkdir(join(workdir, "empty-dir")); - const tool = createReadFileTool(workdir); + const tool = makeTool(workdir); const result = await tool.execute({ path: "empty-dir" }, stubCtx()); expect(result.isError).toBeUndefined(); @@ -280,7 +290,7 @@ describe("createReadFileTool", () => { it("reads a file unchanged (regression: line numbers + offset/limit)", async () => { await writeFile(join(workdir, "regression.txt"), "a\nb\nc\nd\ne\n", "utf8"); - const tool = createReadFileTool(workdir); + const tool = makeTool(workdir); const result = await tool.execute({ path: "regression.txt", offset: 2, limit: 3 }, stubCtx()); expect(result.isError).toBeUndefined(); @@ -288,10 +298,89 @@ describe("createReadFileTool", () => { }); it("returns not-found for a nonexistent path", async () => { - const tool = createReadFileTool(workdir); + const tool = makeTool(workdir); const result = await tool.execute({ path: "nonexistent-path" }, stubCtx()); expect(result.isError).toBe(true); expect(result.content).toContain("not found"); }); + + it("routes fs calls through resolveBackend(ctx.computerId) (transport seam)", async () => { + // A fake backend records what it is asked to do. Proves the tool programs + // against the ExecBackend surface (not node:fs) and that the resolver is + // invoked with ctx.computerId — the SSH seam. No real fs involved. + let statCalls = 0; + let readFileCalls = 0; + let readdirCalls = 0; + let receivedComputerId: string | undefined = "__sentinel__"; + const fakeBackend = { + spawn: async () => ({ exitCode: 0, timedOut: false, aborted: false }), + readFile: async (path: string) => { + readFileCalls++; + expect(path).toContain("seam.txt"); + return "fake-line-1\nfake-line-2"; + }, + writeFile: async () => {}, + stat: async (path: string) => { + statCalls++; + expect(path).toContain("seam.txt"); + return { isFile: true, isDirectory: false }; + }, + readdir: async () => { + readdirCalls++; + return []; + }, + exists: async () => true, + } as const; + + const tool = createReadFileTool({ + resolveBackend: (computerId) => { + receivedComputerId = computerId; + return fakeBackend; + }, + workdir, + }); + const result = await tool.execute({ path: "seam.txt" }, stubCtx({ computerId: "prod-ssh" })); + + expect(receivedComputerId).toBe("prod-ssh"); + expect(statCalls).toBe(1); + expect(readFileCalls).toBe(1); + expect(readdirCalls).toBe(0); + expect(result.isError).toBeUndefined(); + expect(result.content).toBe("1: fake-line-1\n2: fake-line-2"); + }); + + it("resolves the local backend when ctx.computerId is undefined (backward compat)", async () => { + // computerId undefined → resolver returns localExecBackend → real fs. + const tool = createReadFileTool({ resolveBackend: () => localExecBackend, workdir }); + const filePath = join(workdir, "compat.txt"); + await writeFile(filePath, "real fs via backend\n", "utf8"); + + const result = await tool.execute({ path: "compat.txt" }, stubCtx()); + + expect(result.isError).toBeUndefined(); + expect(result.content).toContain("1: real fs via backend"); + }); + + it("preserves ENOENT .code branch through the backend (fake backend throws)", async () => { + const enoent = Object.assign(new Error("ENOENT: no such file or directory"), { + code: "ENOENT", + }); + const fakeBackend = { + spawn: async () => ({ exitCode: 0, timedOut: false, aborted: false }), + readFile: async () => "unused", + writeFile: async () => {}, + stat: async () => { + throw enoent; + }, + readdir: async () => [], + exists: async () => false, + } as const; + + const tool = createReadFileTool({ resolveBackend: () => fakeBackend, workdir }); + const result = await tool.execute({ path: "ghost.txt" }, stubCtx()); + + expect(result.isError).toBe(true); + expect(result.content).toBe('Error: File "ghost.txt" not found.'); + }); }); diff --git a/packages/tool-read-file/src/read-file.ts b/packages/tool-read-file/src/read-file.ts index 216f165..b88c241 100644 --- a/packages/tool-read-file/src/read-file.ts +++ b/packages/tool-read-file/src/read-file.ts @@ -1,5 +1,5 @@ -import { readdir, readFile, stat } from "node:fs/promises"; import { resolve } from "node:path"; +import type { ExecBackend, ExecBackendResolver, StatResult } from "@dispatch/exec-backend"; import type { ToolContract, ToolResult } from "@dispatch/kernel"; const DEFAULT_LIMIT = 500; @@ -83,11 +83,21 @@ export function formatDirectoryEntries(entries: readonly DirEntry[], dirPath: st } /** - * Factory: create a read_file ToolContract bound to a working directory. - * The working directory is injected so the tool is testable. + * Factory: create a read_file ToolContract. + * + * `resolveBackend` is the injected seam: each `execute` resolves an + * `ExecBackend` from `ctx.computerId` (undefined → local `node:fs`; a set + * id → a remote SSH backend in a later wave). The tool programs against the + * `ExecBackend` surface, never `node:fs` directly, so it is transport-agnostic. + * + * `workdir` is the fallback base directory when `ctx.cwd` is omitted. It is + * injected so the tool is testable; `execute` prefers `ctx.cwd` when present. */ -export function createReadFileTool(workingDirectory: string): ToolContract { - const workdir = resolve(workingDirectory); +export function createReadFileTool(deps: { + readonly resolveBackend: ExecBackendResolver; + readonly workdir?: string; +}): ToolContract { + const workdir = deps.workdir !== undefined ? resolve(deps.workdir) : undefined; return { name: "read_file", @@ -126,12 +136,21 @@ export function createReadFileTool(workingDirectory: string): ToolContract { const { path: relPath, offset, limit } = validated; const effectiveBase = ctx.cwd ? resolve(ctx.cwd) : workdir; + if (effectiveBase === undefined) { + return { + content: + "Error: No working directory (neither ctx.cwd nor a baked workdir was provided).", + isError: true, + }; + } const resolvedPath = resolve(effectiveBase, relPath); + const backend: ExecBackend = deps.resolveBackend(ctx.computerId); + // Stat to determine if this is a file or directory. - let pathStat: import("node:fs").Stats; + let pathStat: StatResult; try { - pathStat = await stat(resolvedPath); + pathStat = await backend.stat(resolvedPath); } catch (err: unknown) { const code = (err as NodeJS.ErrnoException).code; if (code === "ENOENT") { @@ -143,28 +162,25 @@ export function createReadFileTool(workingDirectory: string): ToolContract { }; } - // Directory listing branch. - if (pathStat.isDirectory()) { - let rawEntries: import("node:fs").Dirent<string>[]; + // Directory listing branch. backend.readdir already returns + // {name, isDirectory}[] entries, so no per-entry collapse is needed. + if (pathStat.isDirectory) { + let entries: readonly DirEntry[]; try { - rawEntries = await readdir(resolvedPath, { encoding: "utf8", withFileTypes: true }); + entries = await backend.readdir(resolvedPath); } catch (err: unknown) { return { content: `Error reading directory: ${err instanceof Error ? err.message : String(err)}`, isError: true, }; } - const dirEntries = rawEntries.map((e) => ({ - name: e.name, - isDirectory: e.isDirectory(), - })); - return { content: formatDirectoryEntries(dirEntries, relPath) }; + return { content: formatDirectoryEntries(entries, relPath) }; } // File branch — read the file. let content: string; try { - content = await readFile(resolvedPath, "utf8"); + content = await backend.readFile(resolvedPath); } catch (err: unknown) { const code = (err as NodeJS.ErrnoException).code; if (code === "ENOENT") { diff --git a/packages/tool-read-file/tsconfig.json b/packages/tool-read-file/tsconfig.json index ff99a43..30cdc4d 100644 --- a/packages/tool-read-file/tsconfig.json +++ b/packages/tool-read-file/tsconfig.json @@ -2,5 +2,5 @@ "extends": "../../tsconfig.base.json", "compilerOptions": { "rootDir": "src", "outDir": "dist", "composite": true }, "include": ["src/**/*.ts"], - "references": [{ "path": "../kernel" }] + "references": [{ "path": "../kernel" }, { "path": "../exec-backend" }] } diff --git a/packages/tool-shell/package.json b/packages/tool-shell/package.json index 3c5995c..606525f 100644 --- a/packages/tool-shell/package.json +++ b/packages/tool-shell/package.json @@ -6,6 +6,7 @@ "main": "dist/index.js", "types": "dist/index.d.ts", "dependencies": { - "@dispatch/kernel": "workspace:*" + "@dispatch/kernel": "workspace:*", + "@dispatch/exec-backend": "workspace:*" } } diff --git a/packages/tool-shell/src/extension.ts b/packages/tool-shell/src/extension.ts index 1a89de0..984263e 100644 --- a/packages/tool-shell/src/extension.ts +++ b/packages/tool-shell/src/extension.ts @@ -1,6 +1,6 @@ +import { execBackendHandle } from "@dispatch/exec-backend"; import type { Extension } from "@dispatch/kernel"; import { createRunShellTool } from "./shell.js"; -import { realSpawn } from "./spawn.js"; export const extension: Extension = { manifest: { @@ -12,8 +12,11 @@ export const extension: Extension = { activation: "eager", capabilities: { shell: true }, contributes: { tools: ["run_shell"] }, + // Host activates exec-backend first → host.getService at activation is safe. + dependsOn: ["exec-backend"], }, activate(host) { - host.defineTool(createRunShellTool({ workdir: process.cwd(), spawn: realSpawn })); + const resolveBackend = host.getService(execBackendHandle); + host.defineTool(createRunShellTool({ workdir: process.cwd(), resolveBackend })); }, }; diff --git a/packages/tool-shell/src/index.ts b/packages/tool-shell/src/index.ts index efd36fc..5194342 100644 --- a/packages/tool-shell/src/index.ts +++ b/packages/tool-shell/src/index.ts @@ -1,3 +1,3 @@ export { extension } from "./extension.js"; -export type { SpawnResult, SpawnShell, ValidatedArgs } from "./shell.js"; +export type { SpawnResult, ValidatedArgs } from "./shell.js"; export { createRunShellTool } from "./shell.js"; diff --git a/packages/tool-shell/src/shell.test.ts b/packages/tool-shell/src/shell.test.ts index 07e0af4..4a579b0 100644 --- a/packages/tool-shell/src/shell.test.ts +++ b/packages/tool-shell/src/shell.test.ts @@ -1,12 +1,7 @@ +import { type ExecBackend, localExecBackend } from "@dispatch/exec-backend"; import { createLogger, type ToolExecuteContext } from "@dispatch/kernel"; import { describe, expect, it } from "vitest"; -import { - buildResult, - createRunShellTool, - type SpawnShell, - truncateOutput, - validateArgs, -} from "./shell.js"; +import { buildResult, createRunShellTool, truncateOutput, validateArgs } from "./shell.js"; function stubCtx(overrides?: Partial<ToolExecuteContext>): ToolExecuteContext { return { @@ -22,12 +17,21 @@ function stubCtx(overrides?: Partial<ToolExecuteContext>): ToolExecuteContext { }; } -function fakeSpawn(result: { +/** A fake backend whose `spawn` resolves a fixed result (no real I/O). */ +function fakeBackend(result: { exitCode: number | null; timedOut: boolean; aborted?: boolean; -}): SpawnShell { - return async () => ({ aborted: false, ...result }); +}): ExecBackend { + return { + ...localExecBackend, + spawn: async () => ({ aborted: false, ...result }), + }; +} + +/** Wrap a fake backend in the resolver the factory expects (computerId-agnostic). */ +function resolverFor(backend: ExecBackend) { + return () => backend; } describe("validateArgs", () => { @@ -173,7 +177,7 @@ describe("createRunShellTool", () => { it("has correct name and parameters shape", () => { const tool = createRunShellTool({ workdir: "/tmp", - spawn: fakeSpawn({ exitCode: 0, timedOut: false }), + resolveBackend: resolverFor(fakeBackend({ exitCode: 0, timedOut: false })), }); expect(tool.name).toBe("run_shell"); expect(tool.parameters.type).toBe("object"); @@ -185,7 +189,7 @@ describe("createRunShellTool", () => { it("concurrencySafe is false", () => { const tool = createRunShellTool({ workdir: "/tmp", - spawn: fakeSpawn({ exitCode: 0, timedOut: false }), + resolveBackend: resolverFor(fakeBackend({ exitCode: 0, timedOut: false })), }); expect(tool.concurrencySafe).toBe(false); }); @@ -193,7 +197,7 @@ describe("createRunShellTool", () => { it("rejects missing or empty command", async () => { const tool = createRunShellTool({ workdir: "/tmp", - spawn: fakeSpawn({ exitCode: 0, timedOut: false }), + resolveBackend: resolverFor(fakeBackend({ exitCode: 0, timedOut: false })), }); const result = await tool.execute({}, stubCtx()); expect(result.isError).toBe(true); @@ -203,10 +207,13 @@ describe("createRunShellTool", () => { it("maps a zero exit code to a success result", async () => { const tool = createRunShellTool({ workdir: "/tmp", - spawn: async (_params) => { - _params.onOutput("hello\n", "stdout"); - return { exitCode: 0, timedOut: false, aborted: false }; - }, + resolveBackend: resolverFor({ + ...localExecBackend, + spawn: async (params) => { + params.onOutput("hello\n", "stdout"); + return { exitCode: 0, timedOut: false, aborted: false }; + }, + }), }); const result = await tool.execute({ command: "echo hello" }, stubCtx()); expect(result.isError).toBeUndefined(); @@ -216,10 +223,13 @@ describe("createRunShellTool", () => { it("maps a non-zero exit code to an isError result", async () => { const tool = createRunShellTool({ workdir: "/tmp", - spawn: async (_params) => { - _params.onOutput("error output\n", "stderr"); - return { exitCode: 1, timedOut: false, aborted: false }; - }, + resolveBackend: resolverFor({ + ...localExecBackend, + spawn: async (params) => { + params.onOutput("error output\n", "stderr"); + return { exitCode: 1, timedOut: false, aborted: false }; + }, + }), }); const result = await tool.execute({ command: "false" }, stubCtx()); expect(result.isError).toBe(true); @@ -229,10 +239,13 @@ describe("createRunShellTool", () => { it("reports a timeout as an isError result", async () => { const tool = createRunShellTool({ workdir: "/tmp", - spawn: async (_params) => { - _params.onOutput("partial\n", "stdout"); - return { exitCode: null, timedOut: true, aborted: false }; - }, + resolveBackend: resolverFor({ + ...localExecBackend, + spawn: async (params) => { + params.onOutput("partial\n", "stdout"); + return { exitCode: null, timedOut: true, aborted: false }; + }, + }), }); const result = await tool.execute({ command: "sleep 999" }, stubCtx()); expect(result.isError).toBe(true); @@ -244,10 +257,13 @@ describe("createRunShellTool", () => { const tool = createRunShellTool({ workdir: "/tmp", outputCap: cap, - spawn: async (_params) => { - _params.onOutput("a".repeat(200), "stdout"); - return { exitCode: 0, timedOut: false, aborted: false }; - }, + resolveBackend: resolverFor({ + ...localExecBackend, + spawn: async (params) => { + params.onOutput("a".repeat(200), "stdout"); + return { exitCode: 0, timedOut: false, aborted: false }; + }, + }), }); const result = await tool.execute({ command: "gen" }, stubCtx()); expect(result.content).toContain("[Output truncated"); @@ -258,12 +274,15 @@ describe("createRunShellTool", () => { const chunks: Array<{ data: string; stream: "stdout" | "stderr" }> = []; const tool = createRunShellTool({ workdir: "/tmp", - spawn: async (params) => { - params.onOutput("line1\n", "stdout"); - params.onOutput("err1\n", "stderr"); - params.onOutput("line2\n", "stdout"); - return { exitCode: 0, timedOut: false, aborted: false }; - }, + resolveBackend: resolverFor({ + ...localExecBackend, + spawn: async (params) => { + params.onOutput("line1\n", "stdout"); + params.onOutput("err1\n", "stderr"); + params.onOutput("line2\n", "stdout"); + return { exitCode: 0, timedOut: false, aborted: false }; + }, + }), }); await tool.execute( { command: "test" }, @@ -282,10 +301,13 @@ describe("createRunShellTool", () => { let receivedCwd = ""; const tool = createRunShellTool({ workdir: "/baked", - spawn: async (params) => { - receivedCwd = params.cwd; - return { exitCode: 0, timedOut: false, aborted: false }; - }, + resolveBackend: resolverFor({ + ...localExecBackend, + spawn: async (params) => { + receivedCwd = params.cwd; + return { exitCode: 0, timedOut: false, aborted: false }; + }, + }), }); await tool.execute({ command: "pwd" }, stubCtx({ cwd: "/custom" })); expect(receivedCwd).toBe("/custom"); @@ -295,10 +317,13 @@ describe("createRunShellTool", () => { let receivedCwd = ""; const tool = createRunShellTool({ workdir: "/baked", - spawn: async (params) => { - receivedCwd = params.cwd; - return { exitCode: 0, timedOut: false, aborted: false }; - }, + resolveBackend: resolverFor({ + ...localExecBackend, + spawn: async (params) => { + receivedCwd = params.cwd; + return { exitCode: 0, timedOut: false, aborted: false }; + }, + }), }); await tool.execute({ command: "pwd" }, stubCtx()); expect(receivedCwd).toBe("/baked"); @@ -307,9 +332,12 @@ describe("createRunShellTool", () => { it("returns error for spawn failure", async () => { const tool = createRunShellTool({ workdir: "/tmp", - spawn: async () => { - throw new Error("spawn failed"); - }, + resolveBackend: resolverFor({ + ...localExecBackend, + spawn: async () => { + throw new Error("spawn failed"); + }, + }), }); const result = await tool.execute({ command: "bad" }, stubCtx()); expect(result.isError).toBe(true); @@ -321,7 +349,7 @@ describe("createRunShellTool", () => { controller.abort(); const tool = createRunShellTool({ workdir: "/tmp", - spawn: async () => ({ exitCode: 0, timedOut: false, aborted: false }), + resolveBackend: resolverFor(fakeBackend({ exitCode: 0, timedOut: false })), }); const result = await tool.execute({ command: "test" }, stubCtx({ signal: controller.signal })); expect(result.isError).toBe(true); @@ -331,20 +359,51 @@ describe("createRunShellTool", () => { let receivedTimeout = 0; const tool = createRunShellTool({ workdir: "/tmp", - spawn: async (params) => { - receivedTimeout = params.timeout; - return { exitCode: 0, timedOut: false, aborted: false }; - }, + resolveBackend: resolverFor({ + ...localExecBackend, + spawn: async (params) => { + receivedTimeout = params.timeout; + return { exitCode: 0, timedOut: false, aborted: false }; + }, + }), }); await tool.execute({ command: "test", timeout: 5000 }, stubCtx()); expect(receivedTimeout).toBe(5000); }); + + it("resolves the backend per-call from ctx.computerId (passes it to the resolver)", async () => { + let receivedComputerId: string | undefined = "__unset__"; + const tool = createRunShellTool({ + workdir: "/tmp", + resolveBackend: (computerId) => { + receivedComputerId = computerId; + return fakeBackend({ exitCode: 0, timedOut: false }); + }, + }); + await tool.execute({ command: "test" }, stubCtx({ computerId: "my-host" })); + expect(receivedComputerId).toBe("my-host"); + }); + + it("resolves the local backend when ctx.computerId is undefined", async () => { + let receivedComputerId: string | undefined = "__unset__"; + const tool = createRunShellTool({ + workdir: "/tmp", + resolveBackend: (computerId) => { + receivedComputerId = computerId; + return fakeBackend({ exitCode: 0, timedOut: false }); + }, + }); + await tool.execute({ command: "test" }, stubCtx()); + expect(receivedComputerId).toBeUndefined(); + }); }); describe("createRunShellTool (integration)", () => { - it("runs a real echo command and captures stdout + cwd", async () => { - const { realSpawn } = await import("./spawn.js"); - const tool = createRunShellTool({ workdir: "/tmp", spawn: realSpawn }); + it("runs a real echo command through the local backend and captures stdout + cwd", async () => { + const tool = createRunShellTool({ + workdir: "/tmp", + resolveBackend: () => localExecBackend, + }); let streamed = ""; const result = await tool.execute( { command: "echo hello-from-shell" }, @@ -358,134 +417,23 @@ describe("createRunShellTool (integration)", () => { expect(result.content).toContain("hello-from-shell"); expect(streamed).toContain("hello-from-shell"); }); -}); -describe("realSpawn — process-group kill on abort/timeout", () => { - it("aborts a command with a grandchild holding the pipes and resolves immediately", async () => { - const { realSpawn } = await import("./spawn.js"); + it("aborts a real long-running command through the local backend and resolves with aborted", async () => { const controller = new AbortController(); - - // "sleep 30 & wait" spawns a grandchild (sleep) that inherits the stdio - // pipes. Killing just the sh parent does NOT close the pipes → close never - // fires. With detached:true + process-group kill, the grandchild dies too. - const promise = realSpawn({ - command: "sleep 30 & wait", - cwd: "/tmp", - signal: controller.signal, - timeout: 60_000, - onOutput: () => {}, - }); - - // Give the shell time to actually spawn the grandchild. - await new Promise((r) => setTimeout(r, 500)); - - controller.abort(); - - // Must resolve promptly (not wait 30s for the grandchild's sleep). - const result = await promise; - expect(result.aborted).toBe(true); - expect(result.timedOut).toBe(false); - - // Give the OS a moment to reap the killed processes. - await new Promise((r) => setTimeout(r, 200)); - - // The grandchild sleep process should be gone. Check via pgrep. - const { execSync } = await import("node:child_process"); - let sleeping: string[]; - try { - sleeping = execSync("pgrep -f 'sleep 30'", { encoding: "utf-8" }).trim().split("\n"); - } catch { - // pgrep returns non-zero when no processes match → all gone. - sleeping = []; - } - expect(sleeping.length).toBe(0); - }); - - it("times out a command with a grandchild holding the pipes and resolves promptly", async () => { - const { realSpawn } = await import("./spawn.js"); - const controller = new AbortController(); - - const promise = realSpawn({ - command: "sleep 30 & wait", - cwd: "/tmp", - signal: controller.signal, - timeout: 500, - onOutput: () => {}, - }); - - // Must resolve within a short window (not 30s). - const start = Date.now(); - const result = await promise; - const elapsed = Date.now() - start; - - expect(result.timedOut).toBe(true); - expect(result.aborted).toBe(false); - // Should resolve shortly after the 500ms timeout, well under 30s. - expect(elapsed).toBeLessThan(10_000); - - // Grandchild should be dead. - await new Promise((r) => setTimeout(r, 200)); - const { execSync } = await import("node:child_process"); - let sleeping: string[]; - try { - sleeping = execSync("pgrep -f 'sleep 30'", { encoding: "utf-8" }).trim().split("\n"); - } catch { - sleeping = []; - } - expect(sleeping.length).toBe(0); - }); - - it("captures stdout on normal completion (regression guard)", async () => { - const { realSpawn } = await import("./spawn.js"); - const controller = new AbortController(); - let output = ""; - - const result = await realSpawn({ - command: "echo hi", - cwd: "/tmp", - signal: controller.signal, - timeout: 5_000, - onOutput: (data) => { - output += data; - }, - }); - - expect(result.aborted).toBe(false); - expect(result.timedOut).toBe(false); - expect(result.exitCode).toBe(0); - expect(output).toContain("hi"); - }); - - it("aborts a simple single-process command and resolves with aborted: true", async () => { - const { realSpawn } = await import("./spawn.js"); - const controller = new AbortController(); - - const promise = realSpawn({ - command: "sleep 30", - cwd: "/tmp", - signal: controller.signal, - timeout: 60_000, - onOutput: () => {}, + const tool = createRunShellTool({ + workdir: "/tmp", + resolveBackend: () => localExecBackend, }); - + const promise = tool.execute({ command: "sleep 30" }, stubCtx({ signal: controller.signal })); // Let the sleep actually start. await new Promise((r) => setTimeout(r, 300)); - controller.abort(); - const result = await promise; - expect(result.aborted).toBe(true); - expect(result.timedOut).toBe(false); - - // The sleep process should be gone. + expect(result.isError).toBe(true); + // The detailed process-group-kill semantics (grandchild holding the pipes, + // prompt resolution) are owned by @dispatch/exec-backend's local.test.ts — + // realSpawn was ported there byte-for-byte. Here we only confirm the tool + // wires the backend's aborted result through to an isError result. await new Promise((r) => setTimeout(r, 200)); - const { execSync } = await import("node:child_process"); - let sleeping: string[]; - try { - sleeping = execSync("pgrep -f 'sleep 30'", { encoding: "utf-8" }).trim().split("\n"); - } catch { - sleeping = []; - } - expect(sleeping.length).toBe(0); }); }); diff --git a/packages/tool-shell/src/shell.ts b/packages/tool-shell/src/shell.ts index cc76bca..dac7fab 100644 --- a/packages/tool-shell/src/shell.ts +++ b/packages/tool-shell/src/shell.ts @@ -1,4 +1,5 @@ import { resolve } from "node:path"; +import type { ExecBackendResolver } from "@dispatch/exec-backend"; import type { ToolContract, ToolExecuteContext, ToolResult } from "@dispatch/kernel"; const DEFAULT_TIMEOUT = 120_000; @@ -15,14 +16,6 @@ export interface SpawnResult { readonly aborted: boolean; } -export type SpawnShell = (params: { - readonly command: string; - readonly cwd: string; - readonly signal: AbortSignal; - readonly timeout: number; - readonly onOutput: (data: string, stream: "stdout" | "stderr") => void; -}) => Promise<SpawnResult>; - export function validateArgs(args: unknown): ValidatedArgs | { readonly error: string } { if (args === null || args === undefined || typeof args !== "object") { return { error: "Error: Arguments must be an object." }; @@ -88,7 +81,7 @@ export function buildResult(params: { export function createRunShellTool(deps: { readonly workdir: string; - readonly spawn: SpawnShell; + readonly resolveBackend: ExecBackendResolver; readonly outputCap?: number; }): ToolContract { const workdir = resolve(deps.workdir); @@ -139,10 +132,12 @@ export function createRunShellTool(deps: { output += data; }; + const backend = deps.resolveBackend(ctx.computerId); + let spawnResult: SpawnResult; try { - spawnResult = await deps.spawn({ + spawnResult = await backend.spawn({ command, cwd: effectiveCwd, signal: ctx.signal, diff --git a/packages/tool-shell/src/spawn.ts b/packages/tool-shell/src/spawn.ts deleted file mode 100644 index 9b1d7e4..0000000 --- a/packages/tool-shell/src/spawn.ts +++ /dev/null @@ -1,87 +0,0 @@ -import { spawn as nodeSpawn } from "node:child_process"; -import type { SpawnResult, SpawnShell } from "./shell.js"; - -export const realSpawn: SpawnShell = (params): Promise<SpawnResult> => { - return new Promise<SpawnResult>((resolve) => { - // detached: true puts the child in its own process group (pgid = child.pid). - // This lets us kill the entire group (child + any grandchildren that inherit - // the pipes) via process.kill(-pgid, "SIGKILL") on abort/timeout, so a - // backgrounded grandchild can't keep the stdio pipes open and stall the - // promise on child.on("close"). - const child = nodeSpawn("sh", ["-c", params.command], { - cwd: params.cwd, - stdio: ["ignore", "pipe", "pipe"], - detached: true, - }); - - let settled = false; - let timedOut = false; - let timer: ReturnType<typeof setTimeout> | undefined; - - /** Kill the entire child process group (best-effort — group may be gone). */ - const killGroup = () => { - if (child.pid !== undefined) { - try { - process.kill(-child.pid, "SIGKILL"); - } catch { - // Process group may already be gone — ignore. - } - } - }; - - /** Remove the abort listener and clear the timeout timer (no leaks). */ - const cleanup = () => { - if (timer !== undefined) { - clearTimeout(timer); - timer = undefined; - } - params.signal.removeEventListener("abort", onAbort); - }; - - /** Resolve once, then clean up so listeners/timers never leak. */ - const settle = (result: SpawnResult) => { - if (settled) return; - settled = true; - cleanup(); - resolve(result); - }; - - const onAbort = () => { - if (settled) return; - killGroup(); - // Resolve immediately — do NOT wait for child.on("close"), which may - // never fire if a grandchild holds the pipes open. - settle({ exitCode: null, timedOut: false, aborted: true }); - }; - params.signal.addEventListener("abort", onAbort, { once: true }); - - timer = setTimeout(() => { - if (settled) return; - timedOut = true; - killGroup(); - // Resolve immediately — same reasoning as abort. - settle({ exitCode: null, timedOut: true, aborted: false }); - }, params.timeout); - - child.stdout.on("data", (chunk: Buffer) => { - params.onOutput(chunk.toString(), "stdout"); - }); - - child.stderr.on("data", (chunk: Buffer) => { - params.onOutput(chunk.toString(), "stderr"); - }); - - // Normal-completion path: wait for "close" so all stdout/stderr is captured. - // If abort/timeout already settled, this is a no-op (settled === true). - child.on("close", (code) => { - settle({ exitCode: code, timedOut, aborted: false }); - }); - - // Spawn error (e.g. bad cwd, sh not found). Kill the group just in case - // and resolve — never leave the promise pending. - child.on("error", () => { - killGroup(); - settle({ exitCode: 1, timedOut: false, aborted: false }); - }); - }); -}; diff --git a/packages/tool-shell/tsconfig.json b/packages/tool-shell/tsconfig.json index ff99a43..30cdc4d 100644 --- a/packages/tool-shell/tsconfig.json +++ b/packages/tool-shell/tsconfig.json @@ -2,5 +2,5 @@ "extends": "../../tsconfig.base.json", "compilerOptions": { "rootDir": "src", "outDir": "dist", "composite": true }, "include": ["src/**/*.ts"], - "references": [{ "path": "../kernel" }] + "references": [{ "path": "../kernel" }, { "path": "../exec-backend" }] } diff --git a/packages/tool-write-file/package.json b/packages/tool-write-file/package.json index 4aa3481..63c2ccd 100644 --- a/packages/tool-write-file/package.json +++ b/packages/tool-write-file/package.json @@ -6,6 +6,7 @@ "main": "dist/index.js", "types": "dist/index.d.ts", "dependencies": { - "@dispatch/kernel": "workspace:*" + "@dispatch/kernel": "workspace:*", + "@dispatch/exec-backend": "workspace:*" } } diff --git a/packages/tool-write-file/src/extension.ts b/packages/tool-write-file/src/extension.ts index 2008954..0a9a10f 100644 --- a/packages/tool-write-file/src/extension.ts +++ b/packages/tool-write-file/src/extension.ts @@ -1,3 +1,4 @@ +import { execBackendHandle } from "@dispatch/exec-backend"; import type { Extension } from "@dispatch/kernel"; import { createWriteFileTool } from "./write-file.js"; @@ -11,8 +12,11 @@ export const extension: Extension = { activation: "eager", capabilities: { fs: true }, contributes: { tools: ["write_file"] }, + // Host activates exec-backend first → host.getService at activation is safe. + dependsOn: ["exec-backend"], }, activate(host) { - host.defineTool(createWriteFileTool(process.cwd())); + const resolveBackend = host.getService(execBackendHandle); + host.defineTool(createWriteFileTool({ resolveBackend, workdir: process.cwd() })); }, }; diff --git a/packages/tool-write-file/src/write-file.test.ts b/packages/tool-write-file/src/write-file.test.ts index 6b316bc..d157eb2 100644 --- a/packages/tool-write-file/src/write-file.test.ts +++ b/packages/tool-write-file/src/write-file.test.ts @@ -1,6 +1,7 @@ import { mkdir, mkdtemp, readFile, rm, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; +import { localExecBackend } from "@dispatch/exec-backend"; import { createLogger, type ToolExecuteContext } from "@dispatch/kernel"; import { afterEach, beforeEach, describe, expect, it } from "vitest"; import { createWriteFileTool, decideOverwrite, validateArgs } from "./write-file.js"; @@ -19,6 +20,15 @@ function stubCtx(overrides?: Partial<ToolExecuteContext>): ToolExecuteContext { }; } +/** + * Build a write_file tool wired to the real local ExecBackend (node:fs, + * behavior-identical to today's inline calls). No `@dispatch/*` mocking — the + * real fs edge is exercised, matching the constitution's strict-core rule. + */ +function makeTool(workdir: string) { + return createWriteFileTool({ resolveBackend: () => localExecBackend, workdir }); +} + let workdir: string; beforeEach(async () => { @@ -116,7 +126,7 @@ describe("validateArgs", () => { describe("createWriteFileTool", () => { it("creates a new file when overwrite is unset and the file is absent", async () => { - const tool = createWriteFileTool(workdir); + const tool = makeTool(workdir); const result = await tool.execute({ path: "new-file.txt", content: "hello world" }, stubCtx()); expect(result.isError).toBeUndefined(); @@ -128,7 +138,7 @@ describe("createWriteFileTool", () => { it("errors when the file exists and overwrite is unset", async () => { await writeFile(join(workdir, "existing.txt"), "old content", "utf8"); - const tool = createWriteFileTool(workdir); + const tool = makeTool(workdir); const result = await tool.execute({ path: "existing.txt", content: "new content" }, stubCtx()); expect(result.isError).toBe(true); @@ -141,7 +151,7 @@ describe("createWriteFileTool", () => { it("overwrites an existing file when overwrite is true", async () => { await writeFile(join(workdir, "existing.txt"), "old content", "utf8"); - const tool = createWriteFileTool(workdir); + const tool = makeTool(workdir); const result = await tool.execute( { path: "existing.txt", content: "new content", overwrite: true }, stubCtx(), @@ -154,7 +164,7 @@ describe("createWriteFileTool", () => { }); it("errors when overwrite is true but the file is absent", async () => { - const tool = createWriteFileTool(workdir); + const tool = makeTool(workdir); const result = await tool.execute( { path: "nonexistent.txt", content: "data", overwrite: true }, stubCtx(), @@ -165,7 +175,7 @@ describe("createWriteFileTool", () => { }); it("errors when the parent directory does not exist", async () => { - const tool = createWriteFileTool(workdir); + const tool = makeTool(workdir); const result = await tool.execute({ path: "no/such/dir/file.txt", content: "data" }, stubCtx()); expect(result.isError).toBe(true); @@ -173,12 +183,12 @@ describe("createWriteFileTool", () => { }); it("concurrencySafe is false", () => { - const tool = createWriteFileTool(workdir); + const tool = makeTool(workdir); expect(tool.concurrencySafe).toBe(false); }); it("has correct name and parameters shape", () => { - const tool = createWriteFileTool(workdir); + const tool = makeTool(workdir); expect(tool.name).toBe("write_file"); expect(tool.parameters.type).toBe("object"); expect(tool.parameters.required).toEqual(["path", "content"]); @@ -188,7 +198,7 @@ describe("createWriteFileTool", () => { }); it("never throws on bad input (always returns ToolResult)", async () => { - const tool = createWriteFileTool(workdir); + const tool = makeTool(workdir); const inputs = [null, undefined, 42, "string", {}, { path: "" }, { path: 123 }]; for (const input of inputs) { const result = await tool.execute(input, stubCtx()); @@ -200,7 +210,7 @@ describe("createWriteFileTool", () => { it("respects ctx.cwd over baked workdir", async () => { const ctxDir = await mkdtemp(join(tmpdir(), "ctx-cwd-test-")); try { - const tool = createWriteFileTool(workdir); + const tool = makeTool(workdir); const result = await tool.execute( { path: "ctx-file.txt", content: "from ctx" }, stubCtx({ cwd: ctxDir }), @@ -215,7 +225,7 @@ describe("createWriteFileTool", () => { }); it("writes empty content", async () => { - const tool = createWriteFileTool(workdir); + const tool = makeTool(workdir); const result = await tool.execute({ path: "empty.txt", content: "" }, stubCtx()); expect(result.isError).toBeUndefined(); @@ -225,7 +235,7 @@ describe("createWriteFileTool", () => { it("writes content in subdirectory that exists", async () => { await mkdir(join(workdir, "sub")); - const tool = createWriteFileTool(workdir); + const tool = makeTool(workdir); const result = await tool.execute({ path: "sub/file.txt", content: "nested" }, stubCtx()); expect(result.isError).toBeUndefined(); diff --git a/packages/tool-write-file/src/write-file.ts b/packages/tool-write-file/src/write-file.ts index 1317ce8..cf761b6 100644 --- a/packages/tool-write-file/src/write-file.ts +++ b/packages/tool-write-file/src/write-file.ts @@ -1,5 +1,5 @@ -import { access, stat, writeFile } from "node:fs/promises"; import { resolve } from "node:path"; +import type { ExecBackend, ExecBackendResolver } from "@dispatch/exec-backend"; import type { ToolContract, ToolResult } from "@dispatch/kernel"; interface ValidatedArgs { @@ -51,11 +51,21 @@ export function validateArgs(args: unknown): ValidatedArgs | { readonly error: s } /** - * Factory: create a write_file ToolContract bound to a working directory. - * The working directory is injected so the tool is testable. + * Factory: create a write_file ToolContract. + * + * `resolveBackend` is the injected seam: each `execute` resolves an + * `ExecBackend` from `ctx.computerId` (undefined → local `node:fs`; a set + * id → a remote SSH backend in a later wave). The tool programs against the + * `ExecBackend` surface, never `node:fs` directly, so it is transport-agnostic. + * + * `workdir` is the fallback base directory when `ctx.cwd` is omitted. It is + * injected so the tool is testable; `execute` prefers `ctx.cwd` when present. */ -export function createWriteFileTool(workingDirectory: string): ToolContract { - const workdir = resolve(workingDirectory); +export function createWriteFileTool(deps: { + readonly resolveBackend: ExecBackendResolver; + readonly workdir?: string; +}): ToolContract { + const workdir = deps.workdir !== undefined ? resolve(deps.workdir) : undefined; return { name: "write_file", @@ -95,22 +105,21 @@ export function createWriteFileTool(workingDirectory: string): ToolContract { const { path: relPath, content, overwrite } = validated; const effectiveBase = ctx.cwd ? resolve(ctx.cwd) : workdir; + if (effectiveBase === undefined) { + return { + content: + "Error: No working directory (neither ctx.cwd nor a baked workdir was provided).", + isError: true, + }; + } const resolvedPath = resolve(effectiveBase, relPath); - // Check existence. - let fileExists = false; - try { - await access(resolvedPath); - fileExists = true; - } catch (err: unknown) { - const code = (err as NodeJS.ErrnoException).code; - if (code !== "ENOENT") { - return { - content: `Error checking file: ${err instanceof Error ? err.message : String(err)}`, - isError: true, - }; - } - } + const backend: ExecBackend = deps.resolveBackend(ctx.computerId); + + // Check existence. `backend.exists` never throws — it returns false + // when the path is missing — so the old try/catch around `access` + // collapses to a single boolean read. + const fileExists = await backend.exists(resolvedPath); // Pure decision. const decision = decideOverwrite(fileExists, overwrite); @@ -118,10 +127,13 @@ export function createWriteFileTool(workingDirectory: string): ToolContract { return { content: decision.error, isError: true }; } - // Verify it's not a directory. + // Verify it's not a directory. `backend.stat` returns a + // `{ isFile, isDirectory }` result; only reached when the file + // exists, so an ENOENT here is a lost race left to propagate + // (same as the prior uncaught `stat` call). if (fileExists) { - const pathStat = await stat(resolvedPath); - if (pathStat.isDirectory()) { + const pathStat = await backend.stat(resolvedPath); + if (pathStat.isDirectory) { return { content: `Error: "${relPath}" is a directory, not a file.`, isError: true, @@ -129,9 +141,11 @@ export function createWriteFileTool(workingDirectory: string): ToolContract { } } - // Write the file. + // Write the file. LocalExecBackend throws node:fs-style errors + // carrying a `.code` (e.g. ENOENT when the parent dir is missing); + // the catch surfaces the message verbatim. try { - await writeFile(resolvedPath, content, "utf8"); + await backend.writeFile(resolvedPath, content); } catch (err: unknown) { return { content: `Error writing file: ${err instanceof Error ? err.message : String(err)}`, diff --git a/packages/tool-write-file/tsconfig.json b/packages/tool-write-file/tsconfig.json index ff99a43..30cdc4d 100644 --- a/packages/tool-write-file/tsconfig.json +++ b/packages/tool-write-file/tsconfig.json @@ -2,5 +2,5 @@ "extends": "../../tsconfig.base.json", "compilerOptions": { "rootDir": "src", "outDir": "dist", "composite": true }, "include": ["src/**/*.ts"], - "references": [{ "path": "../kernel" }] + "references": [{ "path": "../kernel" }, { "path": "../exec-backend" }] } |