Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 45 additions & 0 deletions src/pi-tools.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
import assert from "node:assert/strict";
import { mkdtemp, rm, writeFile } from "node:fs/promises";
import { tmpdir } from "node:os";
import { join } from "node:path";
import test from "node:test";
import { setTimeout as delay } from "node:timers/promises";
import { grepFilesTool, listDirectoryTool } from "./pi-tools.js";

function text(response: { content: Array<{ type: string; text?: string }> }): string {
return response.content.flatMap((part) => part.type === "text" ? [part.text ?? ""] : []).join("\n");
}

test("native discovery propagates cancellation with an attributable error and no mutation", async () => {
const root = await mkdtemp(join(tmpdir(), "devspace-pi-cancel-"));
try {
await writeFile(join(root, "fixture.txt"), "alpha|beta\n");
const controller = new AbortController();
controller.abort(new Error("fixture cancellation"));
const response = await listDirectoryTool(
{ path: "." },
{ cwd: root, root, signal: controller.signal },
);
assert.equal(response.isError, true);
assert.match(text(response), /DISCOVERY_CANCELLED/);
} finally {
await rm(root, { recursive: true, force: true });
}
});

test("native discovery classifies an already-expired AbortSignal as a timeout", async () => {
const root = await mkdtemp(join(tmpdir(), "devspace-pi-timeout-"));
try {
await writeFile(join(root, "fixture.txt"), "alpha|beta\n");
const signal = AbortSignal.timeout(1);
await delay(5);
const response = await grepFilesTool(
{ pattern: "alpha|beta", path: ".", limit: 20 },
{ cwd: root, root, signal },
);
assert.equal(response.isError, true);
assert.match(text(response), /DISCOVERY_TIMEOUT/);
} finally {
await rm(root, { recursive: true, force: true });
}
});
39 changes: 36 additions & 3 deletions src/pi-tools.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,11 @@ interface ToolContext {
cwd: string;
root: string;
readRoots?: string[];
signal?: AbortSignal;
}

const DISCOVERY_TIMEOUT_MS = 10_000;

function toMcpContent(result: AgentToolResult<unknown>): McpContent[] {
return result.content.map((content) => {
if (content.type === "text") {
Expand All @@ -50,6 +53,36 @@ function formatToolError(error: unknown): McpContent[] {
return [{ type: "text", text: message }];
}

function discoveryAbortContent(signal: AbortSignal): McpContent[] | undefined {
if (!signal.aborted) return undefined;
const reason = signal.reason;
const name = reason instanceof Error ? reason.name : "";
const message = reason instanceof Error ? reason.message : String(reason ?? "aborted");
const code = name === "TimeoutError" ? "DISCOVERY_TIMEOUT" : "DISCOVERY_CANCELLED";
return [{ type: "text", text: `[${code}] Read-only discovery stopped: ${message}` }];
}

async function runDiscoveryTool<TInput, TDetails = unknown>(
execute: (input: TInput, signal: AbortSignal) => Promise<AgentToolResult<TDetails>>,
input: TInput,
context: ToolContext,
): Promise<ToolResponse<TDetails>> {
const timeoutSignal = AbortSignal.timeout(DISCOVERY_TIMEOUT_MS);
const signal = context.signal ? AbortSignal.any([context.signal, timeoutSignal]) : timeoutSignal;
const alreadyAborted = discoveryAbortContent(signal);
if (alreadyAborted) return { content: alreadyAborted, isError: true };
try {
const result = await execute(input, signal);
return {
content: toMcpContent(result),
details: result.details,
};
} catch (error) {
const aborted = discoveryAbortContent(signal);
return { content: aborted ?? formatToolError(error), isError: true };
}
}

async function runTool<TInput, TDetails = unknown>(
execute: (input: TInput) => Promise<AgentToolResult<TDetails>>,
input: TInput,
Expand Down Expand Up @@ -101,21 +134,21 @@ export async function grepFilesTool(input: GrepToolInput, context: ToolContext):
if (input.path) resolveAllowedPath(input.path, context.cwd, [context.root]);
const tool = createGrepTool(context.cwd);

return runTool((params) => tool.execute("grep_files", params), input, context);
return runDiscoveryTool((params, signal) => tool.execute("grep_files", params, signal), input, context);
}

export async function findFilesTool(input: FindToolInput, context: ToolContext): Promise<ToolResponse> {
if (input.path) resolveAllowedPath(input.path, context.cwd, [context.root]);
const tool = createFindTool(context.cwd);

return runTool((params) => tool.execute("find_files", params), input, context);
return runDiscoveryTool((params, signal) => tool.execute("find_files", params, signal), input, context);
}

export async function listDirectoryTool(input: LsToolInput, context: ToolContext): Promise<ToolResponse> {
if (input.path) resolveAllowedPath(input.path, context.cwd, [context.root]);
const tool = createLsTool(context.cwd);

return runTool((params) => tool.execute("list_directory", params), input, context);
return runDiscoveryTool((params, signal) => tool.execute("list_directory", params, signal), input, context);
}

export async function runShellTool(input: BashToolInput, context: ToolContext): Promise<ToolResponse> {
Expand Down
30 changes: 22 additions & 8 deletions src/server.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1985,6 +1985,7 @@ test("repository write and shell sink families reject an unbound caller before e
workspaceId,
profile: "mutator",
prompt: "must not launch without Core binding",
attemptKey: "core-unbound-agent-start",
executionContract: { writePaths: ["AGENTS.md"] },
},
_meta: conversation,
Expand Down Expand Up @@ -2353,6 +2354,10 @@ test("subagents enabled: agent tools are present and functional", async (t) => {
assert.ok(startProps.effort, "agent_start must advertise direct effort selection");
assert.ok(!(startTool.inputSchema.required as string[] | undefined)?.includes("profile"), "profile must be optional for direct dispatch");
assert.ok(startProps.attemptKey);
assert.ok(
(startTool.inputSchema.required as string[] | undefined)?.includes("attemptKey"),
"agent_start must require a physical replay identity before any provider side effect",
);

const continueProps = continueTool.inputSchema.properties as Record<string, any>;
assert.equal(continueProps.workspaceRoot, undefined);
Expand Down Expand Up @@ -2398,16 +2403,17 @@ test("subagents enabled: agent tools are present and functional", async (t) => {
provider: "codex",
model: "gpt-test",
prompt: "invalid selector",
attemptKey: "invalid-selector-attempt",
},
});
assert.equal(invalidSelector.isError, true);
assert.match(responseText(invalidSelector), /either profile|both provider and model/i);

for (const arguments_ of [
{ workspaceId, prompt: "missing selector" },
{ workspaceId, provider: "codex", prompt: "missing model" },
{ workspaceId, profile: "reviewer", effort: "high", prompt: "profile effort mismatch" },
{ workspaceId, provider: "codex", model: "", prompt: "empty model" },
{ workspaceId, prompt: "missing selector", attemptKey: "missing-selector-attempt" },
{ workspaceId, provider: "codex", prompt: "missing model", attemptKey: "missing-model-attempt" },
{ workspaceId, profile: "reviewer", effort: "high", prompt: "profile effort mismatch", attemptKey: "profile-effort-attempt" },
{ workspaceId, provider: "codex", model: "", prompt: "empty model", attemptKey: "empty-model-attempt" },
]) {
const rejected = await context.client.callTool({ name: "agent_start", arguments: arguments_ });
assert.equal(rejected.isError, true);
Expand Down Expand Up @@ -2594,7 +2600,7 @@ test("subagents: legacy durable session without execution generation does not si
const workspaceId = structuredContent(opened).workspaceId as string;
const start = await context.client.callTool({
name: "agent_start",
arguments: { workspaceId, profile: "reviewer", prompt: "legacy simulation" },
arguments: { workspaceId, profile: "reviewer", prompt: "legacy simulation", attemptKey: "legacy-generation-start" },
});
const agentId = structuredContent(start).agentId as string;

Expand Down Expand Up @@ -2642,6 +2648,7 @@ test("subagents: unknown/invalid workspaceId fails closed before durable-agent a
workspaceId: invalidWorkspaceId,
profile: "reviewer",
prompt: "fail prompt",
attemptKey: "invalid-workspace-start",
},
});
assert.equal(startRes.isError, true);
Expand Down Expand Up @@ -2928,6 +2935,7 @@ test("subagents: agent_start executionContract expectedHead mismatch fails close
workspaceId,
profile: "reviewer",
prompt: "work",
attemptKey: "expected-head-stale",
executionContract: { expectedHead: "a".repeat(40), writePaths: ["src"] },
},
});
Expand All @@ -2941,6 +2949,7 @@ test("subagents: agent_start executionContract expectedHead mismatch fails close
workspaceId,
profile: "reviewer",
prompt: "work",
attemptKey: "expected-head-current",
executionContract: { expectedHead: head.stdout.trim(), writePaths: ["src"] },
},
});
Expand All @@ -2955,7 +2964,7 @@ test("subagents: agent_reconcile reports physical diff as candidate evidence", a

const startResult = await context.client.callTool({
name: "agent_start",
arguments: { workspaceId, profile: "reviewer", prompt: "do work" },
arguments: { workspaceId, profile: "reviewer", prompt: "do work", attemptKey: "reconcile-physical-diff" },
});
const agentId = (structuredContent(startResult) as Record<string, unknown>).agentId as string;

Expand Down Expand Up @@ -3840,6 +3849,7 @@ test("agent_start MCP transports durable tool authority and projection into the
workspaceId,
profile: "reviewer",
prompt: "transport the bounded tool projection",
attemptKey: "tool-projection-start",
executionContract: {
authorityMode: "OWNER_DIRECT",
authorizedToolCeiling: ["workspace.search_text", "workspace.read"],
Expand Down Expand Up @@ -3884,6 +3894,7 @@ test("agent_start MCP transports durable tool authority and projection into the
workspaceId,
profile: "reviewer",
prompt: "reject provider-native tool id",
attemptKey: "provider-native-tool-reject",
executionContract: {
authorizedToolCeiling: ["codex.shell"],
},
Expand All @@ -3905,7 +3916,7 @@ test("direct agent selectors reject disabled providers before preflight", async
assert.match(responseText(result), /provider 'claude' is disabled/i);
const start = await context.client.callTool({
name: "agent_start",
arguments: { workspaceId, provider: "claude", model: "claude-test", prompt: "must be rejected" },
arguments: { workspaceId, provider: "claude", model: "claude-test", prompt: "must be rejected", attemptKey: "disabled-provider-start" },
});
assert.equal(start.isError, true);
assert.match(responseText(start), /provider 'claude' is disabled/i);
Expand Down Expand Up @@ -4318,8 +4329,11 @@ test("command_status metadata annotations and minimal mode visibility", async (t
const toolsList = await context.client.listTools();
const toolNames = toolsList.tools.map((t) => t.name);

// command_status is visible in minimal mode for read-only reconciliation
// command_status and native read-only discovery are visible in minimal mode.
assert.ok(toolNames.includes("command_status"), "command_status should be visible in minimal mode");
assert.ok(toolNames.includes("grep"), "grep should be visible in minimal mode");
assert.ok(toolNames.includes("glob"), "glob should be visible in minimal mode");
assert.ok(toolNames.includes("ls"), "ls should be visible in minimal mode");

// exec_command and write_stdin remain hidden in minimal mode
assert.ok(!toolNames.includes("exec_command"), "exec_command must stay hidden in minimal mode");
Expand Down
19 changes: 10 additions & 9 deletions src/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -456,9 +456,7 @@ function serverInstructions(config: ServerConfig): string {
return `Use DevSpace for coding work. Call ${toolNames.openWorkspace} once for each project folder or isolated worktree, then keep using its workspaceId. During continued work in the same project or worktree, do not call ${toolNames.openWorkspace} again. Open another workspace only when changing projects, switching checkout/worktree mode, creating another isolated worktree, or when the current workspaceId is rejected. Use ${toolNames.read} for direct file reads, apply_patch for all file modifications, exec_command for inspection, tests, builds, and other commands, and write_stdin to poll or interact with running processes. Follow instructions returned by ${toolNames.openWorkspace}; read applicable instruction and skill files before working in their scope.${artifactInstruction}${showChangesInstruction}${agentToolsInstruction}${gitCandidatesInstruction}${codexGoalsInstruction}${repositoryIntelligenceInstruction}`;
}

const inspection = config.toolMode !== "full"
? `In minimal tool mode, ${toolNames.grep}, ${toolNames.glob}, and ${toolNames.ls} are disabled; use ${toolNames.shell} with command-line tools such as grep, rg, find, ls, and tree for search and directory inspection. `
: `Prefer ${toolNames.read}, ${toolNames.grep}, ${toolNames.glob}, and ${toolNames.ls} for file inspection. `;
const inspection = `Prefer ${toolNames.read}, ${toolNames.grep}, ${toolNames.glob}, and ${toolNames.ls} for bounded read-only file inspection. Use ${toolNames.shell} only when shell semantics are actually needed. `;

const skills = config.skillsEnabled
? `When ${toolNames.openWorkspace} returns available skills and a task matches a skill, use ${toolNames.read} to read that skill's path before proceeding. Skill paths may be outside the workspace, but ${toolNames.read} only permits advertised SKILL.md files and files under already-loaded skill directories. `
Expand Down Expand Up @@ -2803,8 +2801,8 @@ function createAgentStartInputSchema() {
workspaceId: z.string().describe("Workspace identifier returned by open_workspace."),
...selectorShape.shape,
prompt: z.string().describe("Task prompt for the agent."),
attemptKey: z.string().regex(/^[A-Za-z0-9][A-Za-z0-9._:/-]{0,127}$/).optional().describe(
"Optional physical-workspace-scoped replay identity. Exact request replays reuse one durable agent; conflicting reuse fails closed.",
attemptKey: z.string().regex(/^[A-Za-z0-9][A-Za-z0-9._:/-]{0,127}$/).describe(
"Required physical-workspace-scoped replay identity. Exact request replays reuse one durable agent; conflicting reuse fails closed.",
),
executionContract,
};
Expand Down Expand Up @@ -4619,7 +4617,7 @@ export function createMcpServer(
);
}

if (config.toolMode === "full") {
if (config.toolMode !== "codex") {
registerAppTool(
server,
toolNames.grep,
Expand All @@ -4644,13 +4642,14 @@ export function createMcpServer(
...toolWidgetDescriptorMeta(config, "search"),
annotations: { readOnlyHint: true },
},
async ({ workspaceId, ...input }) => {
async ({ workspaceId, ...input }, extra) => {
const startedAt = performance.now();
const workspace = workspaces.getWorkspace(workspaceId);
if (input.path) workspaces.resolvePath(workspace, input.path);
const response = await grepFilesTool(input, {
cwd: workspace.root,
root: workspace.root,
signal: extra.signal,
});

if (response.isError) {
Expand Down Expand Up @@ -4714,13 +4713,14 @@ export function createMcpServer(
...toolWidgetDescriptorMeta(config, "search"),
annotations: { readOnlyHint: true },
},
async ({ workspaceId, ...input }) => {
async ({ workspaceId, ...input }, extra) => {
const startedAt = performance.now();
const workspace = workspaces.getWorkspace(workspaceId);
if (input.path) workspaces.resolvePath(workspace, input.path);
const response = await findFilesTool(input, {
cwd: workspace.root,
root: workspace.root,
signal: extra.signal,
});

if (response.isError) {
Expand Down Expand Up @@ -4784,13 +4784,14 @@ export function createMcpServer(
...toolWidgetDescriptorMeta(config, "directory"),
annotations: { readOnlyHint: true },
},
async ({ workspaceId, ...input }) => {
async ({ workspaceId, ...input }, extra) => {
const startedAt = performance.now();
const workspace = workspaces.getWorkspace(workspaceId);
workspaces.resolvePath(workspace, input.path);
const response = await listDirectoryTool(input, {
cwd: workspace.root,
root: workspace.root,
signal: extra.signal,
});

if (response.isError) {
Expand Down
Loading