Unverified 提交 59b721b1 authored 作者: Will Chen's avatar Will Chen 提交者: GitHub

List files: recursive (#2169)

<!-- CURSOR_SUMMARY --> > [!NOTE] > **Enhancements** > - `list_files` now accepts `recursive` (default: false); non-recursive uses `/*`, recursive uses `/**`; consent preview/XML reflect recursion and UI title shows "(recursive)". > - UI: `dyad-list-files` renders as a collapsible panel with chevron toggle and abbreviated file list with total count; wired through `DyadMarkdownParser`. > > **Safety/Infra** > - Validates directory input via `resolveDirectoryWithinAppPath` to prevent traversal (Windows/POSIX cases covered) with unit tests. > - Fake LLM server resets fixture turn counting per prompt (counts tool results after last user message) for stable multi-prompt E2E. > - E2E: adds fixtures and snapshots for recursive and non-recursive modes; shorter toast duration in test mode to speed tests. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 3f366934b050a583ae66168e4c924a97a5529f51. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY --> <!-- This is an auto-generated description by cubic. --> --- ## Summary by cubic Add recursive support to the local agent list_files tool and make results collapsible in the chat UI. Includes E2E coverage and a fix to fixture turn counting for stable multi-prompt runs. - **New Features** - list_files accepts recursive (default: false); uses "/*" for non-recursive and "/**" for recursive. - Consent preview/XML and UI reflect recursion; collapsible panel shows abbreviated list with total count. - Recursive attribute wired through the markdown parser; added E2E tests and snapshots for both modes. - **Bug Fixes** - Validate directory paths to prevent path traversal outside the app directory. - Fake LLM server counts tool results after the last user message to reset turns per prompt. - Shorter toast duration in test mode to speed up E2E runs. <sup>Written for commit 3f366934b050a583ae66168e4c924a97a5529f51. Summary will update on new commits.</sup> <!-- End of auto-generated description by cubic. -->
上级 759c60f2
import type { LocalAgentFixture } from "../../../../testing/fake-llm-server/localAgentTypes";
export const fixture: LocalAgentFixture = {
description: "List files in directory without recursion",
turns: [
{
text: "I'll list the files in the src directory for you.",
toolCalls: [
{
name: "list_files",
args: {
directory: "src",
recursive: false,
},
},
],
},
{
text: "Here are the files in the src directory.",
},
],
};
import type { LocalAgentFixture } from "../../../../testing/fake-llm-server/localAgentTypes";
export const fixture: LocalAgentFixture = {
description: "List files in directory recursively",
turns: [
{
text: "I'll list all files in the src directory recursively for you.",
toolCalls: [
{
name: "list_files",
args: {
directory: "src",
recursive: true,
},
},
],
},
{
text: "Here are all the files in the src directory and its subdirectories.",
},
],
};
import { expect } from "@playwright/test";
import { testSkipIfWindows } from "./helpers/test_helper";
/**
* E2E tests for list_files tool
*/
testSkipIfWindows("local-agent - list_files", async ({ po }) => {
await po.setUpDyadPro({ localAgent: true });
await po.importApp("minimal");
await po.selectLocalAgentMode();
await po.sendPrompt("tc=local-agent/list-files-non-recursive");
await po.sendPrompt("tc=local-agent/list-files-recursive");
const listFiles1 = po.page.getByTestId("dyad-list-files").first();
await listFiles1.click();
await expect(listFiles1).toMatchAriaSnapshot();
const listFiles2 = po.page.getByTestId("dyad-list-files").nth(1);
await listFiles2.click();
await expect(listFiles2).toMatchAriaSnapshot();
});
...@@ -178,13 +178,17 @@ ...@@ -178,13 +178,17 @@
"type": "function", "type": "function",
"function": { "function": {
"name": "list_files", "name": "list_files",
"description": "List all files in the application directory recursively. If you are not sure, list all files by omitting the directory parameter.", "description": "List files in the application directory. By default, lists only the immediate directory contents. Use recursive=true to list all files recursively. If you are not sure, list all files by omitting the directory parameter.",
"parameters": { "parameters": {
"type": "object", "type": "object",
"properties": { "properties": {
"directory": { "directory": {
"type": "string", "type": "string",
"description": "Optional subdirectory to list" "description": "Optional subdirectory to list"
},
"recursive": {
"type": "boolean",
"description": "Whether to list files recursively (default: false)"
} }
}, },
"additionalProperties": false, "additionalProperties": false,
......
- 'button "List Files: src"':
- img
- img
- text: "- src/App.tsx - src/main.tsx - src/vite-env.d.ts (3 files total)"
\ No newline at end of file
- 'button "List Files: src (recursive)"':
- img
- img
- text: "- src/App.tsx - src/main.tsx - src/vite-env.d.ts (3 files total)"
\ No newline at end of file
import { describe, it, expect } from "vitest";
import { resolveDirectoryWithinAppPath } from "@/pro/main/ipc/handlers/local_agent/tools/path_safety";
describe("resolveDirectoryWithinAppPath", () => {
it("allows valid subdirectories even if appPath uses forward slashes (Windows)", () => {
const relativePathFromApp = resolveDirectoryWithinAppPath({
appPath: "C:/Users/project",
directory: "src",
});
expect(relativePathFromApp).toBe("src");
});
it("rejects any '..' segment (Windows)", () => {
expect(() =>
resolveDirectoryWithinAppPath({
appPath: "C:/Users/project",
directory: "src\\..\\src",
}),
).toThrow(/contains "\.\." path traversal segment/);
});
it("rejects traversal outside appPath (Windows)", () => {
expect(() =>
resolveDirectoryWithinAppPath({
appPath: "C:/Users/project",
directory: "..\\..\\Windows",
}),
).toThrow(/contains "\.\." path traversal segment/);
});
it("rejects absolute paths outside appPath (Windows)", () => {
expect(() =>
resolveDirectoryWithinAppPath({
appPath: "C:/Users/project",
directory: "C:\\Windows",
}),
).toThrow(/escapes the project directory/);
});
it("allows valid subdirectories on POSIX paths", () => {
const relativePathFromApp = resolveDirectoryWithinAppPath({
appPath: "/Users/project",
directory: "src",
});
expect(relativePathFromApp).toBe("src");
});
it("rejects any '..' segment (POSIX)", () => {
expect(() =>
resolveDirectoryWithinAppPath({
appPath: "/Users/project",
directory: "src/../src",
}),
).toThrow(/contains "\.\." path traversal segment/);
});
it("rejects traversal outside appPath on POSIX paths", () => {
expect(() =>
resolveDirectoryWithinAppPath({
appPath: "/Users/project",
directory: "../../etc",
}),
).toThrow(/contains "\.\." path traversal segment/);
});
it("rejects absolute paths outside appPath on POSIX paths", () => {
expect(() =>
resolveDirectoryWithinAppPath({
appPath: "/Users/project",
directory: "/etc",
}),
).toThrow(/escapes the project directory/);
});
it("allows absolute paths inside appPath on POSIX paths", () => {
const relativePathFromApp = resolveDirectoryWithinAppPath({
appPath: "/Users/project",
directory: "/Users/project/src",
});
expect(relativePathFromApp).toBe("src");
});
});
...@@ -66,7 +66,8 @@ export function ChatModeSelector() { ...@@ -66,7 +66,8 @@ export function ChatModeSelector() {
}} }}
/> />
), ),
{ duration: 8000 }, // Make the toast shorter in test mode for faster tests.
{ duration: settings?.isTestMode ? 50 : 8000 },
); );
} }
}; };
......
import React from "react"; import React, { useState } from "react";
import { CustomTagState } from "./stateTypes"; import { CustomTagState } from "./stateTypes";
import { FolderOpen, Loader2 } from "lucide-react"; import { ChevronRight, FolderOpen, Loader2 } from "lucide-react";
interface DyadListFilesProps { interface DyadListFilesProps {
node: { node: {
properties: { properties: {
directory?: string; directory?: string;
recursive?: string;
state?: CustomTagState; state?: CustomTagState;
}; };
}; };
...@@ -13,24 +14,45 @@ interface DyadListFilesProps { ...@@ -13,24 +14,45 @@ interface DyadListFilesProps {
} }
export function DyadListFiles({ node, children }: DyadListFilesProps) { export function DyadListFiles({ node, children }: DyadListFilesProps) {
const { directory, state } = node.properties; const { directory, recursive, state } = node.properties;
const isLoading = state === "pending"; const isLoading = state === "pending";
const isRecursive = recursive === "true";
const content = typeof children === "string" ? children : ""; const content = typeof children === "string" ? children : "";
const [isExpanded, setIsExpanded] = useState(false);
const getTitle = () => {
const parts: string[] = ["List Files"];
if (directory) {
parts[0] = `List Files: ${directory}`;
}
if (isRecursive) {
parts.push("(recursive)");
}
return parts.join(" ");
};
return ( return (
<div className="my-2 border rounded-md overflow-hidden"> <div
<div className="flex items-center gap-2 px-3 py-2 bg-muted/50 border-b"> data-testid="dyad-list-files"
className="my-2 border rounded-md overflow-hidden"
>
<button
type="button"
onClick={() => setIsExpanded(!isExpanded)}
className="flex items-center gap-2 px-3 py-2 bg-muted/50 w-full text-left hover:bg-muted/70 transition-colors"
>
<ChevronRight
className={`size-4 text-muted-foreground transition-transform ${isExpanded ? "rotate-90" : ""}`}
/>
{isLoading ? ( {isLoading ? (
<Loader2 className="size-4 animate-spin text-muted-foreground" /> <Loader2 className="size-4 animate-spin text-muted-foreground" />
) : ( ) : (
<FolderOpen className="size-4 text-muted-foreground" /> <FolderOpen className="size-4 text-muted-foreground" />
)} )}
<span className="font-medium text-sm"> <span className="font-medium text-sm">{getTitle()}</span>
{directory ? `List Files: ${directory}` : "List Files"} </button>
</span> {isExpanded && content && (
</div> <div className="p-3 text-xs font-mono whitespace-pre-wrap max-h-60 overflow-y-auto bg-muted/20 border-t">
{content && (
<div className="p-3 text-xs font-mono whitespace-pre-wrap max-h-60 overflow-y-auto bg-muted/20">
{content} {content}
</div> </div>
)} )}
......
...@@ -642,6 +642,7 @@ function renderCustomTag( ...@@ -642,6 +642,7 @@ function renderCustomTag(
node={{ node={{
properties: { properties: {
directory: attributes.directory || "", directory: attributes.directory || "",
recursive: attributes.recursive || "",
state: getState({ isStreaming, inProgress }), state: getState({ isStreaming, inProgress }),
}, },
}} }}
......
import path from "node:path";
import { z } from "zod"; import { z } from "zod";
import { ToolDefinition, AgentContext, escapeXmlAttr } from "./types"; import {
ToolDefinition,
AgentContext,
escapeXmlAttr,
escapeXmlContent,
} from "./types";
import { extractCodebase } from "../../../../../../utils/codebase"; import { extractCodebase } from "../../../../../../utils/codebase";
import { resolveDirectoryWithinAppPath } from "./path_safety";
const listFilesSchema = z.object({ const listFilesSchema = z.object({
directory: z.string().optional().describe("Optional subdirectory to list"), directory: z.string().optional().describe("Optional subdirectory to list"),
recursive: z
.boolean()
.optional()
.describe("Whether to list files recursively (default: false)"),
}); });
export const listFilesTool: ToolDefinition<z.infer<typeof listFilesSchema>> = { type ListFilesArgs = z.infer<typeof listFilesSchema>;
function getXmlAttributes(args: ListFilesArgs) {
const dirAttr = args.directory
? ` directory="${escapeXmlAttr(args.directory)}"`
: "";
const recursiveAttr =
args.recursive !== undefined ? ` recursive="${args.recursive}"` : "";
return `${dirAttr}${recursiveAttr}`;
}
export const listFilesTool: ToolDefinition<ListFilesArgs> = {
name: "list_files", name: "list_files",
description: description:
"List all files in the application directory recursively. If you are not sure, list all files by omitting the directory parameter.", "List files in the application directory. By default, lists only the immediate directory contents. Use recursive=true to list all files recursively. If you are not sure, list all files by omitting the directory parameter.",
inputSchema: listFilesSchema, inputSchema: listFilesSchema,
defaultConsent: "always", defaultConsent: "always",
getConsentPreview: (args) => getConsentPreview: (args) => {
args.directory ? `List ${args.directory}` : "List all files", const recursiveText = args.recursive ? " (recursive)" : "";
return args.directory
? `List ${args.directory}${recursiveText}`
: `List all files${recursiveText}`;
},
buildXml: (args, _isComplete) => { buildXml: (args, isComplete) => {
const dirAttr = args.directory if (isComplete) {
? ` directory="${escapeXmlAttr(args.directory)}"` return undefined;
: ""; }
return `<dyad-list-files${dirAttr}></dyad-list-files>`; return `<dyad-list-files${getXmlAttributes(args)}></dyad-list-files>`;
}, },
execute: async (args, ctx: AgentContext) => { execute: async (args, ctx: AgentContext) => {
// Validate directory path to prevent path traversal attacks
let sanitizedDirectory: string | undefined;
if (args.directory) {
const relativePathFromApp = resolveDirectoryWithinAppPath({
appPath: ctx.appPath,
directory: args.directory,
});
// Normalize for glob usage (glob treats "\" as an escape on Windows)
const normalizedRelativePath = relativePathFromApp
.split(path.sep)
.join("/")
.replace(/\\/g, "/");
// Empty means "root"
sanitizedDirectory = normalizedRelativePath || undefined;
}
// Use "**" for recursive, "*" for non-recursive (immediate children only)
const globSuffix = args.recursive ? "/**" : "/*";
const globPath = sanitizedDirectory
? sanitizedDirectory + globSuffix
: globSuffix.slice(1); // Remove leading "/" for root directory
const { files } = await extractCodebase({ const { files } = await extractCodebase({
appPath: ctx.appPath, appPath: ctx.appPath,
// TODO
chatContext: { chatContext: {
contextPaths: args.directory contextPaths: [{ globPath }],
? [{ globPath: args.directory + "/**" }]
: [],
smartContextAutoIncludes: [], smartContextAutoIncludes: [],
excludePaths: [], excludePaths: [],
}, },
}); });
return files.map((file) => " - " + file.path).join("\n") || ""; // Build full file list for LLM
const allFilesList =
files.map((file) => " - " + file.path).join("\n") || "";
// Build abbreviated list for UI display
const MAX_FILES_TO_SHOW = 20;
const totalCount = files.length;
const displayedFiles = files.slice(0, MAX_FILES_TO_SHOW);
const abbreviatedList =
displayedFiles.map((file) => " - " + file.path).join("\n") || "";
const countInfo =
totalCount > MAX_FILES_TO_SHOW
? `\n... and ${totalCount - MAX_FILES_TO_SHOW} more files (${totalCount} total)`
: `\n(${totalCount} files total)`;
// Write abbreviated list to UI
ctx.onXmlComplete(
`<dyad-list-files${getXmlAttributes(args)}>${escapeXmlContent(abbreviatedList + countInfo)}</dyad-list-files>`,
);
// Return full file list for LLM
return allFilesList;
}, },
}; };
import path from "node:path";
/**
* Resolve and validate that `directory` stays within `appPath`.
*
* Why not `startsWith`?
* - On Windows, `path.resolve` normalizes to backslashes, while stored `appPath`
* values may contain forward slashes. A string `startsWith` check can then
* falsely reject valid subdirectories.
*
* This uses `path.relative` instead, and treats Windows paths as case-insensitive.
*/
export function resolveDirectoryWithinAppPath(params: {
appPath: string;
directory: string;
}): string {
// Disallow any ".." path segment (even if the resolved path would remain within root).
// This makes path traversal attempts explicit and avoids surprising "a/../b" style inputs.
if (/(^|[\\/])\.\.([\\/]|$)/.test(params.directory)) {
throw new Error(
`Invalid directory path: "${params.directory}" contains ".." path traversal segment`,
);
}
// We sometimes persist Windows paths with forward slashes (e.g. "C:/..."),
// so detect win32-style roots and use win32 semantics for the safety check.
const looksLikeWin32Path =
/^[a-zA-Z]:[\\/]/.test(params.appPath) ||
params.appPath.startsWith("\\\\") ||
params.appPath.includes("\\");
const pathImpl = looksLikeWin32Path ? path.win32 : path.posix;
const caseInsensitive = looksLikeWin32Path;
const resolvedAppPath = pathImpl.resolve(params.appPath);
const resolvedPath = pathImpl.resolve(resolvedAppPath, params.directory);
const appForCheck = caseInsensitive
? resolvedAppPath.toLowerCase()
: resolvedAppPath;
const targetForCheck = caseInsensitive
? resolvedPath.toLowerCase()
: resolvedPath;
const relForCheck = pathImpl.relative(appForCheck, targetForCheck);
const isWithinRoot =
relForCheck === "" ||
(!relForCheck.startsWith(`..${pathImpl.sep}`) &&
relForCheck !== ".." &&
!pathImpl.isAbsolute(relForCheck));
if (!isWithinRoot) {
throw new Error(
`Invalid directory path: "${params.directory}" escapes the project directory`,
);
}
return pathImpl.relative(resolvedAppPath, resolvedPath);
}
...@@ -38,11 +38,24 @@ function getSessionId(messages: any[]): string { ...@@ -38,11 +38,24 @@ function getSessionId(messages: any[]): string {
} }
/** /**
* Count the number of tool result messages to determine which turn we're on * Count the number of tool result messages AFTER the last user message
* to determine which turn we're on for the current fixture.
* This ensures each new user prompt (fixture trigger) starts fresh at turn 0.
*/ */
function countToolResultRounds(messages: any[]): number { function countToolResultRounds(messages: any[]): number {
// Find the index of the last user message
let lastUserIndex = -1;
for (let i = messages.length - 1; i >= 0; i--) {
if (messages[i]?.role === "user") {
lastUserIndex = i;
break;
}
}
// Count tool results only after the last user message
let rounds = 0; let rounds = 0;
for (const msg of messages) { for (let i = lastUserIndex + 1; i < messages.length; i++) {
const msg = messages[i];
if (msg?.role === "tool") { if (msg?.role === "tool") {
rounds++; rounds++;
} else if (Array.isArray(msg?.content)) { } else if (Array.isArray(msg?.content)) {
...@@ -255,8 +268,8 @@ export async function handleLocalAgentFixture( ...@@ -255,8 +268,8 @@ export async function handleLocalAgentFixture(
const toolResultRounds = countToolResultRounds(messages); const toolResultRounds = countToolResultRounds(messages);
const turnIndex = toolResultRounds; const turnIndex = toolResultRounds;
console.log( console.error(
`[local-agent] Session: ${sessionId}, Turn: ${turnIndex}, Tool rounds: ${toolResultRounds}`, `[local-agent] Loaded fixture: ${fixtureName}, Session: ${sessionId}, Turn: ${turnIndex}, Tool rounds: ${toolResultRounds}`,
); );
if (turnIndex >= fixture.turns.length) { if (turnIndex >= fixture.turns.length) {
......
Markdown 格式
0%
您添加了 0 到此讨论。请谨慎行事。
请先完成此评论的编辑!
注册 或者 后发表评论