fix(coding-agent): use tool-name allowlists and remove cwd-bound singletons
- treat tools as a global allowlist across built-in, extension, and SDK tools - remove process-cwd singleton tool usage from SDK and CLI paths - add regression coverage for extension tool filtering closes #3452 closes #2835
This commit is contained in:
@@ -22,7 +22,6 @@ import {
|
||||
} from "../src/core/agent-session-runtime.js";
|
||||
import { AuthStorage } from "../src/core/auth-storage.js";
|
||||
import { SessionManager } from "../src/core/session-manager.js";
|
||||
import { codingTools } from "../src/core/tools/index.js";
|
||||
import { API_KEY } from "./utilities.js";
|
||||
|
||||
describe.skipIf(!API_KEY)("AgentSession forking", () => {
|
||||
@@ -40,7 +39,6 @@ describe.skipIf(!API_KEY)("AgentSession forking", () => {
|
||||
if (runtimeHost) {
|
||||
await runtimeHost.dispose();
|
||||
}
|
||||
process.chdir(tmpdir());
|
||||
if (tempDir && existsSync(tempDir)) {
|
||||
rmSync(tempDir, { recursive: true });
|
||||
}
|
||||
@@ -73,7 +71,7 @@ describe.skipIf(!API_KEY)("AgentSession forking", () => {
|
||||
sessionManager,
|
||||
sessionStartEvent,
|
||||
model,
|
||||
tools: codingTools,
|
||||
tools: ["read", "bash", "edit", "write"],
|
||||
})),
|
||||
services,
|
||||
diagnostics: services.diagnostics,
|
||||
|
||||
@@ -76,7 +76,6 @@ describe("AgentSessionRuntime session lifecycle events", () => {
|
||||
cleanups.push(async () => {
|
||||
await runtimeHost.dispose();
|
||||
faux.unregister();
|
||||
process.chdir(tmpdir());
|
||||
if (existsSync(tempDir)) {
|
||||
rmSync(tempDir, { recursive: true, force: true });
|
||||
}
|
||||
|
||||
@@ -18,7 +18,7 @@ import { AgentSession } from "../src/core/agent-session.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/core/tools/index.js";
|
||||
import {
|
||||
API_KEY,
|
||||
createTestResourceLoader,
|
||||
@@ -70,7 +70,7 @@ describe.skipIf(!HAS_ANTIGRAVITY_AUTH)("Compaction with thinking models (Antigra
|
||||
initialState: {
|
||||
model,
|
||||
systemPrompt: "You are a helpful assistant. Be concise.",
|
||||
tools: codingTools,
|
||||
tools: createCodingTools(process.cwd()),
|
||||
thinkingLevel,
|
||||
},
|
||||
});
|
||||
@@ -168,7 +168,7 @@ describe.skipIf(!HAS_ANTHROPIC_AUTH)("Compaction with thinking models (Anthropic
|
||||
initialState: {
|
||||
model,
|
||||
systemPrompt: "You are a helpful assistant. Be concise.",
|
||||
tools: codingTools,
|
||||
tools: createCodingTools(process.cwd()),
|
||||
thinkingLevel,
|
||||
},
|
||||
});
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { existsSync, mkdirSync, rmSync } from "node:fs";
|
||||
import { existsSync, mkdirSync, realpathSync, rmSync } from "node:fs";
|
||||
import { tmpdir } from "node:os";
|
||||
import { join } from "node:path";
|
||||
import { getModel } from "@mariozechner/pi-ai";
|
||||
@@ -63,4 +63,33 @@ describe("createAgentSession session manager defaults", () => {
|
||||
|
||||
session.dispose();
|
||||
});
|
||||
|
||||
it("derives cwd from an explicit sessionManager when cwd is omitted", async () => {
|
||||
const model = getModel("anthropic", "claude-sonnet-4-5");
|
||||
expect(model).toBeTruthy();
|
||||
|
||||
const sessionCwd = join(tempDir, "session-project");
|
||||
mkdirSync(sessionCwd, { recursive: true });
|
||||
const sessionManager = SessionManager.inMemory(sessionCwd);
|
||||
const { session } = await createAgentSession({
|
||||
agentDir,
|
||||
model: model!,
|
||||
sessionManager,
|
||||
});
|
||||
|
||||
expect(session.sessionManager).toBe(sessionManager);
|
||||
expect(session.systemPrompt).toContain(`Current working directory: ${sessionCwd}`);
|
||||
|
||||
const bashTool = session.agent.state.tools.find((tool) => tool.name === "bash");
|
||||
expect(bashTool).toBeTruthy();
|
||||
const result = await bashTool!.execute("test", { command: "pwd" });
|
||||
const output = result.content
|
||||
.filter((item): item is { type: "text"; text: string } => item.type === "text")
|
||||
.map((item) => item.text)
|
||||
.join("");
|
||||
|
||||
expect(realpathSync(output.trim())).toBe(realpathSync(sessionCwd));
|
||||
|
||||
session.dispose();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -28,7 +28,6 @@ describe("AgentSessionRuntime characterization", () => {
|
||||
while (cleanups.length > 0) {
|
||||
await cleanups.pop()?.();
|
||||
}
|
||||
process.chdir(tmpdir());
|
||||
});
|
||||
|
||||
async function createRuntimeForTest(
|
||||
@@ -391,7 +390,7 @@ describe("AgentSessionRuntime characterization", () => {
|
||||
await expect(runtime.fork("missing-entry")).rejects.toThrow("Invalid entry ID for forking");
|
||||
});
|
||||
|
||||
it("updates process.cwd() on cross-cwd session replacement", async () => {
|
||||
it("updates the runtime session cwd on cross-cwd session replacement", async () => {
|
||||
const firstDir = join(tmpdir(), `pi-runtime-cwd-a-${Date.now()}-${Math.random().toString(36).slice(2)}`);
|
||||
const secondDir = join(tmpdir(), `pi-runtime-cwd-b-${Date.now()}-${Math.random().toString(36).slice(2)}`);
|
||||
mkdirSync(firstDir, { recursive: true });
|
||||
@@ -459,8 +458,8 @@ describe("AgentSessionRuntime characterization", () => {
|
||||
|
||||
await runtime.switchSession(otherSessionFile);
|
||||
|
||||
expect(realpathSync(process.cwd())).toBe(realpathSync(secondDir));
|
||||
expect(realpathSync(runtime.session.sessionManager.getCwd())).toBe(realpathSync(secondDir));
|
||||
expect(realpathSync(runtime.cwd)).toBe(realpathSync(secondDir));
|
||||
});
|
||||
|
||||
it("restores model and thinking state from the destination session", async () => {
|
||||
|
||||
@@ -0,0 +1,94 @@
|
||||
import { existsSync, mkdirSync, rmSync } from "node:fs";
|
||||
import { tmpdir } from "node:os";
|
||||
import { join } from "node:path";
|
||||
import { getModel } from "@mariozechner/pi-ai";
|
||||
import { Type } from "@sinclair/typebox";
|
||||
import { afterEach, beforeEach, describe, expect, it } from "vitest";
|
||||
import { DefaultResourceLoader } from "../../../src/core/resource-loader.js";
|
||||
import { createAgentSession } from "../../../src/core/sdk.js";
|
||||
import { SessionManager } from "../../../src/core/session-manager.js";
|
||||
import { SettingsManager } from "../../../src/core/settings-manager.js";
|
||||
|
||||
describe("regression #2835: tool allowlists filter extension tools", () => {
|
||||
let tempDir: string;
|
||||
let agentDir: string;
|
||||
|
||||
beforeEach(() => {
|
||||
tempDir = join(tmpdir(), `pi-tools-filter-${Date.now()}-${Math.random().toString(36).slice(2)}`);
|
||||
agentDir = join(tempDir, "agent");
|
||||
mkdirSync(agentDir, { recursive: true });
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
if (tempDir && existsSync(tempDir)) {
|
||||
rmSync(tempDir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
async function createSession(allowedToolNames?: string[]) {
|
||||
const settingsManager = SettingsManager.create(tempDir, agentDir);
|
||||
const sessionManager = SessionManager.inMemory(tempDir);
|
||||
const resourceLoader = new DefaultResourceLoader({
|
||||
cwd: tempDir,
|
||||
agentDir,
|
||||
settingsManager,
|
||||
extensionFactories: [
|
||||
(pi) => {
|
||||
pi.on("session_start", () => {
|
||||
pi.registerTool({
|
||||
name: "dynamic_tool",
|
||||
label: "Dynamic Tool",
|
||||
description: "Tool registered from session_start",
|
||||
promptSnippet: "Run dynamic test behavior",
|
||||
parameters: Type.Object({}),
|
||||
execute: async () => ({
|
||||
content: [{ type: "text", text: "ok" }],
|
||||
details: {},
|
||||
}),
|
||||
});
|
||||
});
|
||||
},
|
||||
],
|
||||
});
|
||||
await resourceLoader.reload();
|
||||
|
||||
const { session } = await createAgentSession({
|
||||
cwd: tempDir,
|
||||
agentDir,
|
||||
model: getModel("anthropic", "claude-sonnet-4-5")!,
|
||||
settingsManager,
|
||||
sessionManager,
|
||||
resourceLoader,
|
||||
tools: allowedToolNames,
|
||||
});
|
||||
await session.bindExtensions({});
|
||||
return session;
|
||||
}
|
||||
|
||||
it("allows only explicitly listed built-in and extension tools", async () => {
|
||||
const session = await createSession(["read", "dynamic_tool"]);
|
||||
|
||||
expect(
|
||||
session
|
||||
.getAllTools()
|
||||
.map((tool) => tool.name)
|
||||
.sort(),
|
||||
).toEqual(["dynamic_tool", "read"]);
|
||||
expect(session.getActiveToolNames().sort()).toEqual(["dynamic_tool", "read"]);
|
||||
expect(session.systemPrompt).toContain("- read: Read file contents");
|
||||
expect(session.systemPrompt).toContain("- dynamic_tool: Run dynamic test behavior");
|
||||
expect(session.systemPrompt).not.toContain("- bash:");
|
||||
expect(session.systemPrompt).not.toContain("- edit:");
|
||||
session.dispose();
|
||||
});
|
||||
|
||||
it("disables all tools when the allowlist is empty", async () => {
|
||||
const session = await createSession([]);
|
||||
|
||||
expect(session.getAllTools()).toEqual([]);
|
||||
expect(session.getActiveToolNames()).toEqual([]);
|
||||
expect(session.systemPrompt).toContain("Available tools:\n(none)");
|
||||
expect(session.systemPrompt).not.toContain("dynamic_tool");
|
||||
session.dispose();
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user