fix(coding-agent): remove process-cwd tool singletons and use tool-name allowlists
- switch SDK/CLI tool selection to name-based allowlists - apply allowlists across built-in, extension, and SDK tools - remove ambient process.cwd defaults from core tooling and resource helpers - update tests, examples, and TUI callers for explicit cwd plumbing - add regression coverage for extension tool filtering closes #3452 closes #2835
This commit is contained in:
@@ -18,7 +18,7 @@ import { AuthStorage } from "../src/core/auth-storage.js";
|
||||
import { ModelRegistry } from "../src/core/model-registry.js";
|
||||
import { SessionManager } from "../src/core/session-manager.js";
|
||||
import { SettingsManager } from "../src/core/settings-manager.js";
|
||||
import { codingTools } from "../src/core/tools/index.js";
|
||||
import { createCodingTools } from "../src/index.js";
|
||||
import { API_KEY, createTestResourceLoader } from "./utilities.js";
|
||||
|
||||
describe.skipIf(!API_KEY)("AgentSession compaction e2e", () => {
|
||||
@@ -52,7 +52,7 @@ describe.skipIf(!API_KEY)("AgentSession compaction e2e", () => {
|
||||
initialState: {
|
||||
model,
|
||||
systemPrompt: "You are a helpful assistant. Be concise.",
|
||||
tools: codingTools,
|
||||
tools: createCodingTools(process.cwd()),
|
||||
},
|
||||
});
|
||||
|
||||
|
||||
@@ -3,8 +3,8 @@ import { existsSync, mkdirSync, readFileSync, rmSync } from "node:fs";
|
||||
import { tmpdir } from "node:os";
|
||||
import { join } from "node:path";
|
||||
import { afterEach, beforeEach, describe, expect, it } from "vitest";
|
||||
import { executeBash } from "../src/core/bash-executor.js";
|
||||
import { createBashTool } from "../src/core/tools/bash.js";
|
||||
import { executeBashWithOperations } from "../src/core/bash-executor.js";
|
||||
import { createBashTool, createLocalBashOperations } from "../src/core/tools/bash.js";
|
||||
|
||||
function toBashSingleQuotedArg(value: string): string {
|
||||
return `'${value.replace(/\\/g, "/").replace(/'/g, `'"'"'`)}'`;
|
||||
@@ -87,9 +87,15 @@ describe.skipIf(process.platform !== "win32")("Windows child-process close handl
|
||||
const controller = new AbortController();
|
||||
|
||||
try {
|
||||
const result = await withTimeout(executeBash(command, { signal: controller.signal }), 3000, () => {
|
||||
controller.abort();
|
||||
});
|
||||
const result = await withTimeout(
|
||||
executeBashWithOperations(command, process.cwd(), createLocalBashOperations(), {
|
||||
signal: controller.signal,
|
||||
}),
|
||||
3000,
|
||||
() => {
|
||||
controller.abort();
|
||||
},
|
||||
);
|
||||
|
||||
expect(result.output).toContain("child-exiting");
|
||||
expect(result.exitCode).toBe(0);
|
||||
|
||||
@@ -21,7 +21,7 @@ import { ModelRegistry } from "../src/core/model-registry.js";
|
||||
import { SessionManager } from "../src/core/session-manager.js";
|
||||
import { SettingsManager } from "../src/core/settings-manager.js";
|
||||
import { createSyntheticSourceInfo } from "../src/core/source-info.js";
|
||||
import { codingTools } from "../src/core/tools/index.js";
|
||||
import { createCodingTools } from "../src/index.js";
|
||||
import { createTestResourceLoader } from "./utilities.js";
|
||||
|
||||
const API_KEY = process.env.ANTHROPIC_OAUTH_TOKEN || process.env.ANTHROPIC_API_KEY;
|
||||
@@ -92,7 +92,7 @@ describe.skipIf(!API_KEY)("Compaction extensions", () => {
|
||||
initialState: {
|
||||
model,
|
||||
systemPrompt: "You are a helpful assistant. Be concise.",
|
||||
tools: codingTools,
|
||||
tools: createCodingTools(process.cwd()),
|
||||
},
|
||||
});
|
||||
|
||||
|
||||
@@ -112,7 +112,7 @@ describe("FooterDataProvider reftable branch detection", () => {
|
||||
mkdirSync(nestedDir, { recursive: true });
|
||||
process.chdir(nestedDir);
|
||||
|
||||
const provider = new FooterDataProvider();
|
||||
const provider = new FooterDataProvider(nestedDir);
|
||||
try {
|
||||
expect(provider.getGitBranch()).toBe("main");
|
||||
expect(vi.mocked(spawnSync)).not.toHaveBeenCalled();
|
||||
@@ -125,7 +125,7 @@ describe("FooterDataProvider reftable branch detection", () => {
|
||||
const repoDir = createPlainReftableRepo(tempDir);
|
||||
process.chdir(repoDir);
|
||||
|
||||
const provider = new FooterDataProvider();
|
||||
const provider = new FooterDataProvider(repoDir);
|
||||
try {
|
||||
expect(provider.getGitBranch()).toBe("main");
|
||||
expect(vi.mocked(spawnSync)).toHaveBeenCalledWith(
|
||||
@@ -146,7 +146,7 @@ describe("FooterDataProvider reftable branch detection", () => {
|
||||
const { worktreeDir } = createReftableWorktree(tempDir);
|
||||
process.chdir(worktreeDir);
|
||||
|
||||
const provider = new FooterDataProvider();
|
||||
const provider = new FooterDataProvider(worktreeDir);
|
||||
try {
|
||||
expect(provider.getGitBranch()).toBe("main");
|
||||
} finally {
|
||||
@@ -159,7 +159,7 @@ describe("FooterDataProvider reftable branch detection", () => {
|
||||
process.chdir(repoDir);
|
||||
resolvedBranch = "";
|
||||
|
||||
const provider = new FooterDataProvider();
|
||||
const provider = new FooterDataProvider(repoDir);
|
||||
try {
|
||||
expect(provider.getGitBranch()).toBe("detached");
|
||||
} finally {
|
||||
@@ -171,7 +171,7 @@ describe("FooterDataProvider reftable branch detection", () => {
|
||||
const { worktreeDir, reftableDir } = createReftableWorktree(tempDir);
|
||||
process.chdir(worktreeDir);
|
||||
|
||||
const provider = new FooterDataProvider();
|
||||
const provider = new FooterDataProvider(worktreeDir);
|
||||
try {
|
||||
expect(provider.getGitBranch()).toBe("main");
|
||||
vi.mocked(spawnSync).mockClear();
|
||||
@@ -194,7 +194,7 @@ describe("FooterDataProvider reftable branch detection", () => {
|
||||
const { worktreeDir, reftableDir } = createReftableWorktree(tempDir);
|
||||
process.chdir(worktreeDir);
|
||||
|
||||
const provider = new FooterDataProvider();
|
||||
const provider = new FooterDataProvider(worktreeDir);
|
||||
try {
|
||||
expect(provider.getGitBranch()).toBe("main");
|
||||
vi.mocked(execFile).mockClear();
|
||||
@@ -215,7 +215,7 @@ describe("FooterDataProvider reftable branch detection", () => {
|
||||
const { worktreeDir, reftableDir } = createReftableWorktree(tempDir);
|
||||
process.chdir(worktreeDir);
|
||||
|
||||
const provider = new FooterDataProvider();
|
||||
const provider = new FooterDataProvider(worktreeDir);
|
||||
try {
|
||||
expect(provider.getGitBranch()).toBe("main");
|
||||
resolvedBranch = "foo";
|
||||
|
||||
@@ -12,6 +12,7 @@ import { mkdirSync, rmSync, writeFileSync } from "fs";
|
||||
import { tmpdir } from "os";
|
||||
import { join } from "path";
|
||||
import { afterAll, describe, expect, test } from "vitest";
|
||||
import { getAgentDir } from "../src/config.js";
|
||||
import { loadPromptTemplates, parseCommandArgs, substituteArgs } from "../src/core/prompt-templates.js";
|
||||
|
||||
// ============================================================================
|
||||
@@ -406,6 +407,8 @@ You are given one or more GitHub PR URLs: $@`,
|
||||
);
|
||||
|
||||
const templates = loadPromptTemplates({
|
||||
cwd: process.cwd(),
|
||||
agentDir: getAgentDir(),
|
||||
promptPaths: [testDir],
|
||||
includeDefaults: false,
|
||||
});
|
||||
@@ -427,6 +430,8 @@ Wrap it. Additional instructions: $ARGUMENTS`,
|
||||
);
|
||||
|
||||
const templates = loadPromptTemplates({
|
||||
cwd: process.cwd(),
|
||||
agentDir: getAgentDir(),
|
||||
promptPaths: [testDir],
|
||||
includeDefaults: false,
|
||||
});
|
||||
@@ -447,6 +452,8 @@ Audit changelog entries for all commits since the last release.`,
|
||||
);
|
||||
|
||||
const templates = loadPromptTemplates({
|
||||
cwd: process.cwd(),
|
||||
agentDir: getAgentDir(),
|
||||
promptPaths: [testDir],
|
||||
includeDefaults: false,
|
||||
});
|
||||
@@ -467,6 +474,8 @@ Do something`,
|
||||
);
|
||||
|
||||
const templates = loadPromptTemplates({
|
||||
cwd: process.cwd(),
|
||||
agentDir: getAgentDir(),
|
||||
promptPaths: [testDir],
|
||||
includeDefaults: false,
|
||||
});
|
||||
@@ -487,6 +496,8 @@ Analyze GitHub issue(s): $ARGUMENTS`,
|
||||
);
|
||||
|
||||
const templates = loadPromptTemplates({
|
||||
cwd: process.cwd(),
|
||||
agentDir: getAgentDir(),
|
||||
promptPaths: [testDir],
|
||||
includeDefaults: false,
|
||||
});
|
||||
|
||||
@@ -354,6 +354,7 @@ describe("skills", () => {
|
||||
agentDir: emptyAgentDir,
|
||||
cwd: emptyCwd,
|
||||
skillPaths: [join(fixturesDir, "valid-skill")],
|
||||
includeDefaults: true,
|
||||
});
|
||||
expect(skills).toHaveLength(1);
|
||||
expect(skills[0].sourceInfo.scope).toBe("temporary");
|
||||
@@ -365,6 +366,7 @@ describe("skills", () => {
|
||||
agentDir: emptyAgentDir,
|
||||
cwd: emptyCwd,
|
||||
skillPaths: ["/non/existent/path"],
|
||||
includeDefaults: true,
|
||||
});
|
||||
expect(skills).toHaveLength(0);
|
||||
expect(diagnostics.some((d: ResourceDiagnostic) => d.message.includes("does not exist"))).toBe(true);
|
||||
@@ -376,11 +378,13 @@ describe("skills", () => {
|
||||
agentDir: emptyAgentDir,
|
||||
cwd: emptyCwd,
|
||||
skillPaths: ["~/.pi/agent/skills"],
|
||||
includeDefaults: true,
|
||||
});
|
||||
const { skills: withoutTilde } = loadSkills({
|
||||
agentDir: emptyAgentDir,
|
||||
cwd: emptyCwd,
|
||||
skillPaths: [homeSkillsDir],
|
||||
includeDefaults: true,
|
||||
});
|
||||
expect(withTilde.length).toBe(withoutTilde.length);
|
||||
});
|
||||
|
||||
@@ -8,6 +8,7 @@ describe("buildSystemPrompt", () => {
|
||||
selectedTools: [],
|
||||
contextFiles: [],
|
||||
skills: [],
|
||||
cwd: process.cwd(),
|
||||
});
|
||||
|
||||
expect(prompt).toContain("Available tools:\n(none)");
|
||||
@@ -18,6 +19,7 @@ describe("buildSystemPrompt", () => {
|
||||
selectedTools: [],
|
||||
contextFiles: [],
|
||||
skills: [],
|
||||
cwd: process.cwd(),
|
||||
});
|
||||
|
||||
expect(prompt).toContain("Show file paths clearly");
|
||||
@@ -35,6 +37,7 @@ describe("buildSystemPrompt", () => {
|
||||
},
|
||||
contextFiles: [],
|
||||
skills: [],
|
||||
cwd: process.cwd(),
|
||||
});
|
||||
|
||||
expect(prompt).toContain("- read:");
|
||||
@@ -53,6 +56,7 @@ describe("buildSystemPrompt", () => {
|
||||
},
|
||||
contextFiles: [],
|
||||
skills: [],
|
||||
cwd: process.cwd(),
|
||||
});
|
||||
|
||||
expect(prompt).toContain("- dynamic_tool: Run dynamic test behavior");
|
||||
@@ -63,6 +67,7 @@ describe("buildSystemPrompt", () => {
|
||||
selectedTools: ["read", "dynamic_tool"],
|
||||
contextFiles: [],
|
||||
skills: [],
|
||||
cwd: process.cwd(),
|
||||
});
|
||||
|
||||
expect(prompt).not.toContain("dynamic_tool");
|
||||
@@ -76,6 +81,7 @@ describe("buildSystemPrompt", () => {
|
||||
promptGuidelines: ["Use dynamic_tool for project summaries."],
|
||||
contextFiles: [],
|
||||
skills: [],
|
||||
cwd: process.cwd(),
|
||||
});
|
||||
|
||||
expect(prompt).toContain("- Use dynamic_tool for project summaries.");
|
||||
@@ -87,6 +93,7 @@ describe("buildSystemPrompt", () => {
|
||||
promptGuidelines: ["Use dynamic_tool for summaries.", " Use dynamic_tool for summaries. ", " "],
|
||||
contextFiles: [],
|
||||
skills: [],
|
||||
cwd: process.cwd(),
|
||||
});
|
||||
|
||||
expect(prompt.match(/- Use dynamic_tool for summaries\./g)).toHaveLength(1);
|
||||
|
||||
@@ -40,7 +40,15 @@ describe("ToolExecutionComponent parity", () => {
|
||||
renderResult: () => new Text("custom result", 0, 0),
|
||||
};
|
||||
|
||||
const component = new ToolExecutionComponent("custom_tool", "tool-1", {}, {}, toolDefinition, createFakeTui());
|
||||
const component = new ToolExecutionComponent(
|
||||
"custom_tool",
|
||||
"tool-1",
|
||||
{},
|
||||
{},
|
||||
toolDefinition,
|
||||
createFakeTui(),
|
||||
process.cwd(),
|
||||
);
|
||||
expect(stripAnsi(component.render(120).join("\n"))).toContain("custom call");
|
||||
|
||||
component.updateResult(
|
||||
@@ -69,6 +77,7 @@ describe("ToolExecutionComponent parity", () => {
|
||||
{},
|
||||
overrideDefinition,
|
||||
createFakeTui(),
|
||||
process.cwd(),
|
||||
);
|
||||
component.updateResult({ content: [], details: { diff: "+1 after", firstChangedLine: 1 }, isError: false });
|
||||
const rendered = stripAnsi(component.render(120).join("\n"));
|
||||
@@ -85,6 +94,7 @@ describe("ToolExecutionComponent parity", () => {
|
||||
{},
|
||||
undefined,
|
||||
createFakeTui(),
|
||||
process.cwd(),
|
||||
);
|
||||
const rendered = stripAnsi(component.render(120).join("\n"));
|
||||
expect(rendered).toContain("read");
|
||||
@@ -119,6 +129,7 @@ describe("ToolExecutionComponent parity", () => {
|
||||
{},
|
||||
createReadToolDefinition(process.cwd()),
|
||||
createFakeTui(),
|
||||
process.cwd(),
|
||||
);
|
||||
component.updateResult({ content: [{ type: "text", text: "hello" }], details: undefined, isError: false }, false);
|
||||
const rendered = stripAnsi(component.render(120).join("\n"));
|
||||
@@ -138,6 +149,7 @@ describe("ToolExecutionComponent parity", () => {
|
||||
{},
|
||||
overrideDefinition,
|
||||
createFakeTui(),
|
||||
process.cwd(),
|
||||
);
|
||||
component.updateResult({ content: [{ type: "text", text: "hello" }], details: undefined, isError: false }, false);
|
||||
const rendered = stripAnsi(component.render(120).join("\n"));
|
||||
@@ -158,6 +170,7 @@ describe("ToolExecutionComponent parity", () => {
|
||||
{},
|
||||
overrideDefinition,
|
||||
createFakeTui(),
|
||||
process.cwd(),
|
||||
);
|
||||
component.updateResult({ content: [{ type: "text", text: "hello" }], details: undefined, isError: false }, false);
|
||||
const rendered = stripAnsi(component.render(120).join("\n"));
|
||||
@@ -179,6 +192,7 @@ describe("ToolExecutionComponent parity", () => {
|
||||
renderResult: () => new Text("override result", 0, 0),
|
||||
},
|
||||
createFakeTui(),
|
||||
process.cwd(),
|
||||
);
|
||||
component.updateResult({ content: [{ type: "text", text: "hello" }], details: undefined, isError: false }, false);
|
||||
const rendered = stripAnsi(component.render(120).join("\n"));
|
||||
@@ -201,6 +215,7 @@ describe("ToolExecutionComponent parity", () => {
|
||||
renderResult: () => new Text("wrapped override result", 0, 0),
|
||||
},
|
||||
createFakeTui(),
|
||||
process.cwd(),
|
||||
);
|
||||
component.updateResult({ content: [{ type: "text", text: "hello" }], details: undefined, isError: false }, false);
|
||||
const rendered = stripAnsi(component.render(120).join("\n"));
|
||||
@@ -221,7 +236,15 @@ describe("ToolExecutionComponent parity", () => {
|
||||
},
|
||||
};
|
||||
|
||||
const component = new ToolExecutionComponent("custom_tool", "tool-5", {}, {}, toolDefinition, createFakeTui());
|
||||
const component = new ToolExecutionComponent(
|
||||
"custom_tool",
|
||||
"tool-5",
|
||||
{},
|
||||
{},
|
||||
toolDefinition,
|
||||
createFakeTui(),
|
||||
process.cwd(),
|
||||
);
|
||||
component.updateResult({ content: [{ type: "text", text: "done" }], details: {}, isError: false }, false);
|
||||
const rendered = stripAnsi(component.render(120).join("\n"));
|
||||
expect(rendered).toContain("custom call shared-token");
|
||||
@@ -243,6 +266,7 @@ describe("ToolExecutionComponent parity", () => {
|
||||
{},
|
||||
toolDefinition,
|
||||
createFakeTui(),
|
||||
process.cwd(),
|
||||
);
|
||||
component.updateResult({ content: [{ type: "text", text: "done" }], details: {}, isError: false }, false);
|
||||
const rendered = stripAnsi(component.render(120).join("\n"));
|
||||
@@ -261,6 +285,7 @@ describe("ToolExecutionComponent parity", () => {
|
||||
{},
|
||||
toolDefinition,
|
||||
createFakeTui(),
|
||||
process.cwd(),
|
||||
);
|
||||
component.updateResult({ content: [{ type: "text", text: "done" }], details: {}, isError: false }, false);
|
||||
const rendered = stripAnsi(component.render(120).join("\n"));
|
||||
@@ -276,6 +301,7 @@ describe("ToolExecutionComponent parity", () => {
|
||||
{},
|
||||
createWriteToolDefinition(process.cwd()),
|
||||
createFakeTui(),
|
||||
process.cwd(),
|
||||
);
|
||||
const rendered = stripAnsi(component.render(120).join("\n"));
|
||||
expect(rendered).toContain("one");
|
||||
@@ -291,6 +317,7 @@ describe("ToolExecutionComponent parity", () => {
|
||||
{},
|
||||
createReadToolDefinition(process.cwd()),
|
||||
createFakeTui(),
|
||||
process.cwd(),
|
||||
);
|
||||
component.updateResult(
|
||||
{ content: [{ type: "text", text: "one\ntwo\n" }], details: undefined, isError: false },
|
||||
|
||||
@@ -2,16 +2,26 @@ import { existsSync, mkdirSync, readFileSync, rmSync, writeFileSync } from "fs";
|
||||
import { tmpdir } from "os";
|
||||
import { join } from "path";
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import { executeBash } from "../src/core/bash-executor.js";
|
||||
import { bashTool, createBashTool, createLocalBashOperations } from "../src/core/tools/bash.js";
|
||||
import { editTool } from "../src/core/tools/edit.js";
|
||||
import { findTool } from "../src/core/tools/find.js";
|
||||
import { grepTool } from "../src/core/tools/grep.js";
|
||||
import { lsTool } from "../src/core/tools/ls.js";
|
||||
import { readTool } from "../src/core/tools/read.js";
|
||||
import { writeTool } from "../src/core/tools/write.js";
|
||||
import { executeBashWithOperations } from "../src/core/bash-executor.js";
|
||||
import { createBashTool, createLocalBashOperations } from "../src/core/tools/bash.js";
|
||||
import {
|
||||
createEditTool,
|
||||
createFindTool,
|
||||
createGrepTool,
|
||||
createLsTool,
|
||||
createReadTool,
|
||||
createWriteTool,
|
||||
} from "../src/index.js";
|
||||
import * as shellModule from "../src/utils/shell.js";
|
||||
|
||||
const readTool = createReadTool(process.cwd());
|
||||
const writeTool = createWriteTool(process.cwd());
|
||||
const editTool = createEditTool(process.cwd());
|
||||
const bashTool = createBashTool(process.cwd());
|
||||
const grepTool = createGrepTool(process.cwd());
|
||||
const findTool = createFindTool(process.cwd());
|
||||
const lsTool = createLsTool(process.cwd());
|
||||
|
||||
// Helper to extract text from content blocks
|
||||
function getTextOutput(result: any): string {
|
||||
return (
|
||||
@@ -438,7 +448,11 @@ describe("Coding Agent Tools", () => {
|
||||
});
|
||||
|
||||
it("should preserve executeBash sanitization when using local bash operations", async () => {
|
||||
const result = await executeBash("printf '\\033[31mred\\033[0m\\r\\n'");
|
||||
const result = await executeBashWithOperations(
|
||||
"printf '\\033[31mred\\033[0m\\r\\n'",
|
||||
process.cwd(),
|
||||
createLocalBashOperations(),
|
||||
);
|
||||
|
||||
expect(result.exitCode).toBe(0);
|
||||
expect(result.output).toBe("red\n");
|
||||
@@ -468,7 +482,7 @@ describe("Coding Agent Tools", () => {
|
||||
});
|
||||
|
||||
it("executeBash should persist full output when truncation happens by line count only", async () => {
|
||||
const result = await executeBash("seq 3000");
|
||||
const result = await executeBashWithOperations("seq 3000", process.cwd(), createLocalBashOperations());
|
||||
const fullOutputPath = result.fullOutputPath;
|
||||
|
||||
expect(result.truncated).toBe(true);
|
||||
|
||||
@@ -17,7 +17,7 @@ import { ModelRegistry } from "../src/core/model-registry.js";
|
||||
import type { ResourceLoader } from "../src/core/resource-loader.js";
|
||||
import { SessionManager } from "../src/core/session-manager.js";
|
||||
import { SettingsManager } from "../src/core/settings-manager.js";
|
||||
import { codingTools } from "../src/core/tools/index.js";
|
||||
import { createCodingTools } from "../src/index.js";
|
||||
|
||||
/**
|
||||
* API key for authenticated tests. Tests using this should be wrapped in
|
||||
@@ -242,7 +242,7 @@ export function createTestSession(options: TestSessionOptions = {}): TestSession
|
||||
initialState: {
|
||||
model,
|
||||
systemPrompt: options.systemPrompt ?? "You are a helpful assistant. Be extremely concise.",
|
||||
tools: codingTools,
|
||||
tools: createCodingTools(process.cwd()),
|
||||
},
|
||||
});
|
||||
|
||||
|
||||
Reference in New Issue
Block a user