From e66d71f6b959f79ef5166479b7c2eea2bac5e332 Mon Sep 17 00:00:00 2001 From: can1357 Date: Mon, 2 Feb 2026 14:55:48 +0100 Subject: [PATCH] refactor(tools): consolidated directory listing functionality from ls tool into read tool - Consolidated directory listing functionality from dedicated `ls` tool into the `read` tool. - Removed the `ls` tool and all related exports, types, and configurations from the public API. - Updated `read` tool to return formatted directory listings with modification times instead of redirecting to `ls` tool. - Removed `bashInterceptor.simpleLs` setting and related interception logic from bash tool. - Removed `ls.enabled` setting and related tool registration logic. - Removed test cases for the `ls` tool and updated tool type definitions to remove `LsToolResultEvent`. --- packages/coding-agent/CHANGELOG.md | 6 +- .../src/commit/prompts/analysis-system.md | 1 - packages/coding-agent/src/cursor.ts | 3 +- .../src/extensibility/extensions/index.ts | 2 - .../src/extensibility/extensions/types.ts | 9 - .../src/extensibility/hooks/types.ts | 9 +- packages/coding-agent/src/index.ts | 3 - .../src/prompts/review-request.md | 1 - .../src/prompts/system/plan-mode-active.md | 1 - .../coding-agent/src/prompts/tools/read.md | 2 +- packages/coding-agent/src/sdk.ts | 2 - .../src/tools/bash-interceptor.ts | 24 -- packages/coding-agent/src/tools/bash.ts | 8 +- packages/coding-agent/src/tools/index.ts | 4 - packages/coding-agent/src/tools/ls.ts | 307 ------------------ packages/coding-agent/src/tools/read.ts | 91 +++++- packages/coding-agent/src/tools/renderers.ts | 2 - packages/coding-agent/test/tools.test.ts | 16 - 18 files changed, 84 insertions(+), 407 deletions(-) delete mode 100644 packages/coding-agent/src/tools/ls.ts diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 692b8666c..97705ba4a 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -1,7 +1,6 @@ # Changelog ## [Unreleased] - ### Added - Added `find.enabled`, `grep.enabled`, `ls.enabled`, `notebook.enabled`, `fetch.enabled`, `web_search.enabled`, `lsp.enabled`, and `calc.enabled` settings to control availability of individual tools @@ -11,12 +10,17 @@ ### Changed +- Updated `read` tool to handle directory paths by returning formatted listings with modification times instead of redirecting to `ls` +- Updated tool documentation to reflect that `read` now handles both files and directories - Updated system prompt tool precedence section to conditionally display only available specialized tools based on enabled settings - Renamed todo completion settings from `todoCompletion.*` to `todos.reminders.*` and `todos.enabled` for clearer organization - Updated todo reminder logic to check both `todos.reminders` and `todos.enabled` settings independently ### Removed +- Removed `ls` tool—directory listing is now handled by the `read` tool +- Removed `ls.enabled` setting and related configuration options +- Removed `bashInterceptor.simpleLs` setting that redirected simple `ls` commands to the dedicated tool - Removed project tree snapshot generation from system prompt (unused feature) ### Fixed diff --git a/packages/coding-agent/src/commit/prompts/analysis-system.md b/packages/coding-agent/src/commit/prompts/analysis-system.md index 4a6ebd5b4..9048bec9e 100644 --- a/packages/coding-agent/src/commit/prompts/analysis-system.md +++ b/packages/coding-agent/src/commit/prompts/analysis-system.md @@ -4,7 +4,6 @@ Senior release engineer writing precise, changelog-ready commit classifications. Classify git diff into conventional commit format. - ## 1. Determine Scope Apply scope when 60%+ line changes target single component: diff --git a/packages/coding-agent/src/cursor.ts b/packages/coding-agent/src/cursor.ts index 07cfcbe74..8240a8488 100644 --- a/packages/coding-agent/src/cursor.ts +++ b/packages/coding-agent/src/cursor.ts @@ -156,7 +156,8 @@ export class CursorExecHandlers implements ICursorExecHandlers { async ls(args: Parameters>[0]) { const toolCallId = decodeToolCallId(args.toolCallId); - const toolResultMessage = await executeTool(this.options, "ls", toolCallId, { path: args.path }); + // Redirect ls to read tool, which handles directories + const toolResultMessage = await executeTool(this.options, "read", toolCallId, { path: args.path }); return toolResultMessage; } diff --git a/packages/coding-agent/src/extensibility/extensions/index.ts b/packages/coding-agent/src/extensibility/extensions/index.ts index 242d1ee4e..81fbfe02e 100644 --- a/packages/coding-agent/src/extensibility/extensions/index.ts +++ b/packages/coding-agent/src/extensibility/extensions/index.ts @@ -56,7 +56,6 @@ export type { InputEventResult, KeybindingsManager, LoadExtensionsResult, - LsToolResultEvent, // Message Rendering MessageRenderer, MessageRenderOptions, @@ -110,7 +109,6 @@ export { isEditToolResult, isFindToolResult, isGrepToolResult, - isLsToolResult, isReadToolResult, isWriteToolResult, } from "./types"; diff --git a/packages/coding-agent/src/extensibility/extensions/types.ts b/packages/coding-agent/src/extensibility/extensions/types.ts index 00f028d35..e7b03e45c 100644 --- a/packages/coding-agent/src/extensibility/extensions/types.ts +++ b/packages/coding-agent/src/extensibility/extensions/types.ts @@ -495,11 +495,6 @@ export interface FindToolResultEvent extends ToolResultEventBase { details: FindToolDetails | undefined; } -export interface LsToolResultEvent extends ToolResultEventBase { - toolName: "ls"; - details: LsToolDetails | undefined; -} - export interface CustomToolResultEvent extends ToolResultEventBase { toolName: string; details: unknown; @@ -513,7 +508,6 @@ export type ToolResultEvent = | WriteToolResultEvent | GrepToolResultEvent | FindToolResultEvent - | LsToolResultEvent | CustomToolResultEvent; // Type guards @@ -535,9 +529,6 @@ export function isGrepToolResult(e: ToolResultEvent): e is GrepToolResultEvent { export function isFindToolResult(e: ToolResultEvent): e is FindToolResultEvent { return e.toolName === "find"; } -export function isLsToolResult(e: ToolResultEvent): e is LsToolResultEvent { - return e.toolName === "ls"; -} /** Union of all event types */ export type ExtensionEvent = diff --git a/packages/coding-agent/src/extensibility/hooks/types.ts b/packages/coding-agent/src/extensibility/hooks/types.ts index bb30c4275..276b3d7b5 100644 --- a/packages/coding-agent/src/extensibility/hooks/types.ts +++ b/packages/coding-agent/src/extensibility/hooks/types.ts @@ -20,7 +20,7 @@ import type { SessionEntry, SessionManager, } from "../../session/session-manager"; -import type { BashToolDetails, FindToolDetails, GrepToolDetails, LsToolDetails, ReadToolDetails } from "../../tools"; +import type { BashToolDetails, FindToolDetails, GrepToolDetails, ReadToolDetails } from "../../tools"; // Re-export for backward compatibility export type { ExecOptions, ExecResult } from "../../exec/exec"; @@ -454,12 +454,6 @@ export interface FindToolResultEvent extends ToolResultEventBase { details: FindToolDetails | undefined; } -/** Tool result event for ls tool */ -export interface LsToolResultEvent extends ToolResultEventBase { - toolName: "ls"; - details: LsToolDetails | undefined; -} - /** Tool result event for custom/unknown tools */ export interface CustomToolResultEvent extends ToolResultEventBase { toolName: string; @@ -478,7 +472,6 @@ export type ToolResultEvent = | WriteToolResultEvent | GrepToolResultEvent | FindToolResultEvent - | LsToolResultEvent | CustomToolResultEvent; // Type guards for narrowing ToolResultEvent to specific tool types diff --git a/packages/coding-agent/src/index.ts b/packages/coding-agent/src/index.ts index e8be29313..1888a67a8 100644 --- a/packages/coding-agent/src/index.ts +++ b/packages/coding-agent/src/index.ts @@ -256,9 +256,6 @@ export { type GrepOperations, type GrepToolDetails, type GrepToolOptions, - type LsOperations, - type LsToolDetails, - type LsToolOptions, type PythonToolDetails, type ReadToolDetails, type TruncationOptions, diff --git a/packages/coding-agent/src/prompts/review-request.md b/packages/coding-agent/src/prompts/review-request.md index 6d2c02459..5e4ab460f 100644 --- a/packages/coding-agent/src/prompts/review-request.md +++ b/packages/coding-agent/src/prompts/review-request.md @@ -24,7 +24,6 @@ _No files to review._ ### Distribution Guidelines {{#when agentCount "==" 1}}Use **1 reviewer agent**.{{else}}Spawn **{{agentCount}} reviewer agents** in parallel.{{/when}} - {{#if multiAgent}} Group files by locality, e.g.: - Same directory/module → same agent diff --git a/packages/coding-agent/src/prompts/system/plan-mode-active.md b/packages/coding-agent/src/prompts/system/plan-mode-active.md index 3159fdc2e..b3d31816a 100644 --- a/packages/coding-agent/src/prompts/system/plan-mode-active.md +++ b/packages/coding-agent/src/prompts/system/plan-mode-active.md @@ -34,7 +34,6 @@ Plan execution runs in fresh context (session cleared). Make plan file self-cont - **Same task, continuing** → Update and clean outdated sections 4. Call `exit_plan_mode` when complete - {{/if}} {{#if iterative}} diff --git a/packages/coding-agent/src/prompts/tools/read.md b/packages/coding-agent/src/prompts/tools/read.md index ff04b35d5..87d15b644 100644 --- a/packages/coding-agent/src/prompts/tools/read.md +++ b/packages/coding-agent/src/prompts/tools/read.md @@ -7,7 +7,7 @@ Reads files from local filesystem or internal URLs. - Use `offset` and `limit` for large files - Use `lines: true` include line numbers - Supports images (PNG, JPG) and PDFs -- For directories, use ls tool instead +- For directories, returns formatted listing with modification times - Parallelize reads when exploring related files - Supports internal URLs: - `skill://` - read SKILL.md for a skill diff --git a/packages/coding-agent/src/sdk.ts b/packages/coding-agent/src/sdk.ts index 425811ce8..ee7537917 100644 --- a/packages/coding-agent/src/sdk.ts +++ b/packages/coding-agent/src/sdk.ts @@ -96,7 +96,6 @@ import { FindTool, GrepTool, getWebSearchTools, - LsTool, loadSshTool, PythonTool, ReadTool, @@ -235,7 +234,6 @@ export { FindTool, GrepTool, loadSshTool, - LsTool, PythonTool, ReadTool, WriteTool, diff --git a/packages/coding-agent/src/tools/bash-interceptor.ts b/packages/coding-agent/src/tools/bash-interceptor.ts index 34b570c77..e78096ba8 100644 --- a/packages/coding-agent/src/tools/bash-interceptor.ts +++ b/packages/coding-agent/src/tools/bash-interceptor.ts @@ -103,27 +103,3 @@ export function checkBashInterception( return { block: false }; } - -/** - * Check if a command is a simple directory listing that should use `ls` tool. - * Only applies to bare `ls` without complex flags. - */ -export function checkSimpleLsInterception(command: string, availableTools: string[]): InterceptionResult { - if (!availableTools.includes("ls")) { - return { block: false }; - } - - // Match simple ls commands (ls, ls -la, ls /path, etc.) - // Don't intercept complex pipes or commands - const simpleLsPattern = /^\s*ls(\s+(-[a-zA-Z]+\s*)*)?(\s+[^|;&]+)?\s*$/; - - if (simpleLsPattern.test(command.trim())) { - return { - block: true, - message: `Use the \`ls\` tool instead of bash ls. It provides structured output.\n\nOriginal command: ${command}`, - suggestedTool: "ls", - }; - } - - return { block: false }; -} diff --git a/packages/coding-agent/src/tools/bash.ts b/packages/coding-agent/src/tools/bash.ts index 306f7c1e7..65982d0c2 100644 --- a/packages/coding-agent/src/tools/bash.ts +++ b/packages/coding-agent/src/tools/bash.ts @@ -11,7 +11,7 @@ import type { Theme } from "../modes/theme/theme"; import bashDescription from "../prompts/tools/bash.md" with { type: "text" }; import { renderOutputBlock, renderStatusLine } from "../tui"; import type { ToolSession } from "."; -import { checkBashInterception, checkSimpleLsInterception } from "./bash-interceptor"; +import { checkBashInterception } from "./bash-interceptor"; import { applyHeadTail, normalizeBashCommand } from "./bash-normalize"; import type { OutputMeta } from "./output-meta"; import { allocateOutputArtifact, createTailBuffer } from "./output-utils"; @@ -83,12 +83,6 @@ export class BashTool implements AgentTool { if (interception.block) { throw new ToolError(interception.message ?? "Command blocked"); } - if (this.session.settings.get("bashInterceptor.simpleLs")) { - const lsInterception = checkSimpleLsInterception(command, ctx?.toolNames ?? []); - if (lsInterception.block) { - throw new ToolError(lsInterception.message ?? "Command blocked"); - } - } } const commandCwd = cwd ? resolveToCwd(cwd, this.session.cwd) : this.session.cwd; diff --git a/packages/coding-agent/src/tools/index.ts b/packages/coding-agent/src/tools/index.ts index ba8ecb206..ebe6f89bc 100644 --- a/packages/coding-agent/src/tools/index.ts +++ b/packages/coding-agent/src/tools/index.ts @@ -22,7 +22,6 @@ import { ExitPlanModeTool } from "./exit-plan-mode"; import { FetchTool } from "./fetch"; import { FindTool } from "./find"; import { GrepTool } from "./grep"; -import { LsTool } from "./ls"; import { NotebookTool } from "./notebook"; import { wrapToolsWithMetaNotice } from "./output-meta"; import { PythonTool } from "./python"; @@ -76,7 +75,6 @@ export { FetchTool, type FetchToolDetails } from "./fetch"; export { type FindOperations, FindTool, type FindToolDetails, type FindToolOptions } from "./find"; export { setPreferredImageProvider } from "./gemini-image"; export { type GrepOperations, GrepTool, type GrepToolDetails, type GrepToolOptions } from "./grep"; -export { type LsOperations, LsTool, type LsToolDetails, type LsToolOptions } from "./ls"; export { NotebookTool, type NotebookToolDetails } from "./notebook"; export { PythonTool, type PythonToolDetails, type PythonToolOptions } from "./python"; export { ReadTool, type ReadToolDetails } from "./read"; @@ -170,7 +168,6 @@ export const BUILTIN_TOOLS: Record = { edit: s => new EditTool(s), find: s => new FindTool(s), grep: s => new GrepTool(s), - ls: s => new LsTool(s), lsp: LspTool.createIf, notebook: s => new NotebookTool(s), read: s => new ReadTool(s), @@ -279,7 +276,6 @@ export async function createTools(session: ToolSession, toolNames?: string[]): P if (name === "bash") return allowBash; if (name === "python") return allowPython; if (name === "todo_write") return !includeSubmitResult && session.settings.get("todo.enabled"); - if (name === "ls") return session.settings.get("ls.enabled"); if (name === "find") return session.settings.get("find.enabled"); if (name === "grep") return session.settings.get("grep.enabled"); if (name === "notebook") return session.settings.get("notebook.enabled"); diff --git a/packages/coding-agent/src/tools/ls.ts b/packages/coding-agent/src/tools/ls.ts deleted file mode 100644 index 2734daee7..000000000 --- a/packages/coding-agent/src/tools/ls.ts +++ /dev/null @@ -1,307 +0,0 @@ -import nodePath from "node:path"; -import type { AgentTool, AgentToolResult } from "@oh-my-pi/pi-agent-core"; -import { type Component, Text } from "@oh-my-pi/pi-tui"; -import { untilAborted } from "@oh-my-pi/pi-utils"; -import { Type } from "@sinclair/typebox"; -import type { RenderResultOptions } from "../extensibility/custom-tools/types"; -import type { Theme } from "../modes/theme/theme"; -import { renderFileList, renderStatusLine } from "../tui"; -import type { ToolSession } from "."; -import { applyListLimit } from "./list-limit"; -import type { OutputMeta } from "./output-meta"; -import { resolveToCwd } from "./path-utils"; -import { - formatAge, - formatBytes, - formatCount, - formatEmptyMessage, - formatErrorMessage, - PREVIEW_LIMITS, -} from "./render-utils"; -import { ToolError, throwIfAborted } from "./tool-errors"; -import { toolResult } from "./tool-result"; -import { type TruncationResult, truncateHead } from "./truncate"; - -const lsSchema = Type.Object({ - path: Type.Optional(Type.String({ description: "Directory to list (default: current directory)" })), - limit: Type.Optional(Type.Number({ description: "Maximum number of entries to return (default: 500)" })), -}); - -const DEFAULT_LIMIT = 500; - -/** - * Pluggable operations for the ls tool. - * Override these to delegate directory listing to remote systems (e.g., SSH). - */ -export interface LsOperations { - /** Check if path exists and return stats. Returns undefined if not found. */ - stat: (absolutePath: string) => Promise<{ isDirectory: () => boolean; mtimeMs: number } | undefined>; - /** Read directory entries (names only) */ - readdir: (absolutePath: string) => Promise; -} - -export interface LsToolOptions { - /** Custom operations for directory listing. Default: local filesystem via Bun */ - operations?: LsOperations; -} - -export interface LsToolDetails { - entries?: string[]; - /** Raw entry names (with / suffix for dirs) without age suffix, for icon detection */ - rawEntries?: string[]; - dirCount?: number; - fileCount?: number; - truncation?: TruncationResult; - truncationReasons?: Array<"entryLimit" | "byteLimit">; - entryLimitReached?: number; - meta?: OutputMeta; -} - -/** Default operations using Bun APIs */ -const defaultLsOperations: LsOperations = { - async stat(absolutePath: string) { - try { - const s = await Bun.file(absolutePath).stat(); - return { isDirectory: () => s.isDirectory(), mtimeMs: s.mtimeMs }; - } catch { - return undefined; - } - }, - async readdir(absolutePath: string) { - return Array.fromAsync(new Bun.Glob("*").scan({ cwd: absolutePath, dot: true, onlyFiles: false })); - }, -}; - -export class LsTool implements AgentTool { - public readonly name = "ls"; - public readonly label = "Ls"; - public readonly description = - 'List directory contents with modification times. Returns entries sorted alphabetically, with \'/\' suffix for directories and relative age (e.g., "2d ago", "just now"). Includes dotfiles. Output is truncated to 500 entries or 50KB (whichever is hit first).'; - public readonly parameters = lsSchema; - - private readonly session: ToolSession; - private readonly ops: LsOperations; - - constructor(session: ToolSession, options?: LsToolOptions) { - this.session = session; - this.ops = options?.operations ?? defaultLsOperations; - } - - public async execute( - _toolCallId: string, - { path, limit }: { path?: string; limit?: number }, - signal?: AbortSignal, - ): Promise> { - return untilAborted(signal, async () => { - const dirPath = resolveToCwd(path || ".", this.session.cwd); - const effectiveLimit = limit ?? DEFAULT_LIMIT; - - // Check if path exists and is a directory - const dirStat = await this.ops.stat(dirPath); - if (!dirStat) { - throw new ToolError(`Path not found: ${dirPath}`); - } - - if (!dirStat.isDirectory()) { - throw new ToolError(`Not a directory: ${dirPath}`); - } - - // Read directory entries - let entries: string[]; - try { - entries = await this.ops.readdir(dirPath); - } catch (error) { - const message = error instanceof Error ? error.message : String(error); - throw new ToolError(`Cannot read directory: ${message}`); - } - - // Sort alphabetically (case-insensitive) - entries.sort((a, b) => a.toLowerCase().localeCompare(b.toLowerCase())); - - const listLimit = applyListLimit(entries, { limit: effectiveLimit }); - const limitedEntries = listLimit.items; - const limitMeta = listLimit.meta; - - // Format entries with directory indicators - const results: string[] = []; - const rawResults: string[] = []; - let dirCount = 0; - let fileCount = 0; - - for (const entry of limitedEntries) { - throwIfAborted(signal); - const fullPath = nodePath.join(dirPath, entry); - let suffix = ""; - let age = ""; - - const entryStat = await this.ops.stat(fullPath); - if (!entryStat) { - // Skip entries we can't stat - continue; - } - - if (entryStat.isDirectory()) { - suffix = "/"; - dirCount += 1; - } else { - fileCount += 1; - } - // Calculate age from mtime - const ageSeconds = Math.floor((Date.now() - entryStat.mtimeMs) / 1000); - age = formatAge(ageSeconds); - - // Format: "name/ (2d ago)" or "name (just now)" - const line = age ? `${entry}${suffix} (${age})` : entry + suffix; - results.push(line); - rawResults.push(entry + suffix); - } - - if (results.length === 0) { - return { content: [{ type: "text", text: "(empty directory)" }], details: {} }; - } - - // Apply byte truncation (no line limit since we already have entry limit) - const rawOutput = results.join("\n"); - const truncation = truncateHead(rawOutput, { maxLines: Number.MAX_SAFE_INTEGER }); - - const output = truncation.content; - const details: LsToolDetails = { - entries: results, - rawEntries: rawResults, - dirCount, - fileCount, - }; - const truncationReasons: Array<"entryLimit" | "byteLimit"> = []; - - if (limitMeta.resultLimit) { - details.entryLimitReached = limitMeta.resultLimit.reached; - truncationReasons.push("entryLimit"); - } - - if (truncation.truncated) { - details.truncation = truncation; - truncationReasons.push("byteLimit"); - } - - if (truncationReasons.length > 0) { - details.truncationReasons = truncationReasons; - } - - const resultBuilder = toolResult(details).text(output).limits({ resultLimit: limitMeta.resultLimit?.reached }); - if (truncation.truncated) { - resultBuilder.truncation(truncation, { direction: "head" }); - } - - return resultBuilder.done(); - }); - } -} - -// ============================================================================= -// TUI Renderer -// ============================================================================= - -interface LsRenderArgs { - path?: string; - limit?: number; -} - -const COLLAPSED_LIST_LIMIT = PREVIEW_LIMITS.COLLAPSED_ITEMS; - -export const lsToolRenderer = { - inline: true, - renderCall(args: LsRenderArgs, uiTheme: Theme): Component { - const meta: string[] = []; - if (args.limit !== undefined) meta.push(`limit:${args.limit}`); - const text = renderStatusLine({ icon: "pending", title: "Ls", description: args.path || ".", meta }, uiTheme); - return new Text(text, 0, 0); - }, - - renderResult( - result: { content: Array<{ type: string; text?: string }>; details?: LsToolDetails; isError?: boolean }, - { expanded }: RenderResultOptions, - uiTheme: Theme, - args?: LsRenderArgs, - ): Component { - const details = result.details; - const textContent = result.content?.find(c => c.type === "text")?.text ?? ""; - - if (result.isError) { - const header = renderStatusLine({ icon: "error", title: "Ls", description: args?.path || "." }, uiTheme); - return new Text([header, formatErrorMessage(textContent, uiTheme)].join("\n"), 0, 0); - } - - if ( - (!textContent || textContent.trim() === "" || textContent.trim() === "(empty directory)") && - (!details?.entries || details.entries.length === 0) - ) { - const header = renderStatusLine({ icon: "warning", title: "Ls", description: args?.path || "." }, uiTheme); - return new Text([header, formatEmptyMessage("Empty directory", uiTheme)].join("\n"), 0, 0); - } - - let entries: string[] = details?.entries ? [...details.entries] : []; - let rawEntries: string[] | undefined = details?.rawEntries; - if (entries.length === 0) { - const rawLines = textContent.split("\n").filter((l: string) => l.trim()); - entries = rawLines.filter(line => !/^\[.*\]$/.test(line.trim())); - rawEntries = undefined; // Can't reliably extract raw paths from text - } - - if (entries.length === 0) { - const header = renderStatusLine({ icon: "warning", title: "Ls", description: args?.path || "." }, uiTheme); - return new Text([header, formatEmptyMessage("Empty directory", uiTheme)].join("\n"), 0, 0); - } - - let dirCount = details?.dirCount; - let fileCount = details?.fileCount; - if (dirCount === undefined || fileCount === undefined) { - dirCount = 0; - fileCount = 0; - for (let i = 0; i < entries.length; i++) { - const entry = entries[i]; - const raw = rawEntries?.[i] ?? entry; - const name = raw.split(" (")[0]; - if (name.endsWith("/")) { - dirCount += 1; - } else { - fileCount += 1; - } - } - } - - const truncation = details?.meta?.truncation; - const limits = details?.meta?.limits; - const truncated = Boolean(details?.entryLimitReached || truncation || limits?.resultLimit || limits?.headLimit); - const summaryText = [formatCount("dir", dirCount ?? 0), formatCount("file", fileCount ?? 0)].join( - uiTheme.sep.dot, - ); - const meta = truncated ? [summaryText, uiTheme.fg("warning", "truncated")] : [summaryText]; - const header = renderStatusLine( - { icon: truncated ? "warning" : "success", title: "Ls", description: args?.path || ".", meta }, - uiTheme, - ); - - const fileLines = renderFileList( - { - files: entries.map((entry, index) => ({ - path: entry, - isDirectory: (rawEntries?.[index] ?? entry).endsWith("/"), - })), - expanded, - maxCollapsed: COLLAPSED_LIST_LIMIT, - }, - uiTheme, - ); - - const truncationReasons: string[] = []; - if (limits?.resultLimit) truncationReasons.push(`entry limit ${limits.resultLimit.reached}`); - if (truncation) truncationReasons.push(`output cap ${formatBytes(truncation.outputBytes)}`); - if (truncation?.artifactId) truncationReasons.push(`full output: artifact://${truncation.artifactId}`); - - const extraLines = - truncationReasons.length > 0 ? [uiTheme.fg("warning", `truncated: ${truncationReasons.join(", ")}`)] : []; - - return new Text([header, ...fileLines, ...extraLines].join("\n"), 0, 0); - }, - mergeCallAndResult: true, -}; diff --git a/packages/coding-agent/src/tools/read.ts b/packages/coding-agent/src/tools/read.ts index 271a5d9ba..71db6ec5e 100644 --- a/packages/coding-agent/src/tools/read.ts +++ b/packages/coding-agent/src/tools/read.ts @@ -19,10 +19,9 @@ import { formatDimensionNote, resizeImage } from "../utils/image-resize"; import { detectSupportedImageMimeTypeFromFile } from "../utils/mime"; import { ensureTool } from "../utils/tools-manager"; import { applyListLimit } from "./list-limit"; -import { LsTool } from "./ls"; import type { OutputMeta } from "./output-meta"; import { resolveReadPath, resolveToCwd } from "./path-utils"; -import { shortenPath, wrapBrackets } from "./render-utils"; +import { formatAge, shortenPath, wrapBrackets } from "./render-utils"; import { ToolAbortError, ToolError, throwIfAborted } from "./tool-errors"; import { toolResult } from "./tool-result"; import { @@ -519,7 +518,7 @@ const readSchema = Type.Object({ export interface ReadToolDetails { truncation?: TruncationResult; - redirectedTo?: "ls"; + isDirectory?: boolean; resolvedPath?: string; meta?: OutputMeta; } @@ -530,7 +529,7 @@ type ReadParams = { path: string; offset?: number; limit?: number; lines?: boole * Read tool implementation. * * Reads files with support for images, documents (via markitdown), and text. - * Directories redirect to the ls tool. + * Directories return a formatted listing with modification times. */ export class ReadTool implements AgentTool { public readonly name = "read"; @@ -542,20 +541,18 @@ export class ReadTool implements AgentTool { private readonly session: ToolSession; private readonly autoResizeImages: boolean; private readonly defaultLineNumbers: boolean; - private readonly lsTool: LsTool; constructor(session: ToolSession) { this.session = session; this.autoResizeImages = session.settings.get("images.autoResize"); this.defaultLineNumbers = session.settings.get("readLineNumbers"); - this.lsTool = new LsTool(session); this.description = renderPromptTemplate(readDescription, { DEFAULT_MAX_LINES: String(DEFAULT_MAX_LINES), }); } public async execute( - toolCallId: string, + _toolCallId: string, params: ReadParams, signal?: AbortSignal, _onUpdate?: AgentToolUpdateCallback, @@ -606,13 +603,7 @@ export class ReadTool implements AgentTool { } if (isDirectory) { - const lsResult = await this.lsTool.execute(toolCallId, { path: readPath, limit }, signal); - const details: ReadToolDetails = { - redirectedTo: "ls", - truncation: lsResult.details?.truncation, - meta: lsResult.details?.meta, - }; - return toolResult(details).content(lsResult.content).done(); + return this.readDirectory(absolutePath, limit, signal); } const mimeType = await detectSupportedImageMimeTypeFromFile(absolutePath); @@ -982,6 +973,75 @@ export class ReadTool implements AgentTool { } return resultBuilder.done(); } + + /** Read directory contents as a formatted listing */ + private async readDirectory( + absolutePath: string, + limit: number | undefined, + signal?: AbortSignal, + ): Promise> { + const DEFAULT_LIMIT = 500; + const effectiveLimit = limit ?? DEFAULT_LIMIT; + + let entries: string[]; + try { + entries = await fs.readdir(absolutePath); + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + throw new ToolError(`Cannot read directory: ${message}`); + } + + // Sort alphabetically (case-insensitive) + entries.sort((a, b) => a.toLowerCase().localeCompare(b.toLowerCase())); + + const listLimit = applyListLimit(entries, { limit: effectiveLimit }); + const limitedEntries = listLimit.items; + const limitMeta = listLimit.meta; + + // Format entries with directory indicators and ages + const results: string[] = []; + + for (const entry of limitedEntries) { + throwIfAborted(signal); + const fullPath = path.join(absolutePath, entry); + let suffix = ""; + let age = ""; + + try { + const entryStat = await fs.stat(fullPath); + suffix = entryStat.isDirectory() ? "/" : ""; + const ageSeconds = Math.floor((Date.now() - entryStat.mtimeMs) / 1000); + age = formatAge(ageSeconds); + } catch { + // Skip entries we can't stat + continue; + } + + const line = age ? `${entry}${suffix} (${age})` : entry + suffix; + results.push(line); + } + + if (results.length === 0) { + return { content: [{ type: "text", text: "(empty directory)" }], details: {} }; + } + + const output = results.join("\n"); + const truncation = truncateHead(output, { maxLines: Number.MAX_SAFE_INTEGER }); + + const details: ReadToolDetails = { + isDirectory: true, + }; + + const resultBuilder = toolResult(details) + .text(truncation.content) + .limits({ resultLimit: limitMeta.resultLimit?.reached }); + if (truncation.truncated) { + resultBuilder.truncation(truncation, { direction: "head" }); + details.truncation = truncation; + } + + return resultBuilder.done(); + } } // ============================================================================= @@ -1029,9 +1089,6 @@ export const readToolRenderer = { const warningLines: string[] = []; const truncation = details?.meta?.truncation; const fallback = details?.truncation; - if (details?.redirectedTo) { - warningLines.push(uiTheme.fg("warning", wrapBrackets(`Redirected to ${details.redirectedTo}`, uiTheme))); - } if (details?.resolvedPath) { warningLines.push(uiTheme.fg("dim", wrapBrackets(`Resolved path: ${details.resolvedPath}`, uiTheme))); } diff --git a/packages/coding-agent/src/tools/renderers.ts b/packages/coding-agent/src/tools/renderers.ts index e63e8f304..0040d7b92 100644 --- a/packages/coding-agent/src/tools/renderers.ts +++ b/packages/coding-agent/src/tools/renderers.ts @@ -16,7 +16,6 @@ import { calculatorToolRenderer } from "./calculator"; import { fetchToolRenderer } from "./fetch"; import { findToolRenderer } from "./find"; import { grepToolRenderer } from "./grep"; -import { lsToolRenderer } from "./ls"; import { notebookToolRenderer } from "./notebook"; import { pythonToolRenderer } from "./python"; import { readToolRenderer } from "./read"; @@ -49,7 +48,6 @@ export const toolRenderers: Record = { edit: editToolRenderer as ToolRenderer, find: findToolRenderer as ToolRenderer, grep: grepToolRenderer as ToolRenderer, - ls: lsToolRenderer as ToolRenderer, lsp: lspToolRenderer as ToolRenderer, notebook: notebookToolRenderer as ToolRenderer, read: readToolRenderer as ToolRenderer, diff --git a/packages/coding-agent/test/tools.test.ts b/packages/coding-agent/test/tools.test.ts index 37d9e562a..29328e1c0 100644 --- a/packages/coding-agent/test/tools.test.ts +++ b/packages/coding-agent/test/tools.test.ts @@ -8,7 +8,6 @@ import type { ToolSession } from "@oh-my-pi/pi-coding-agent/tools"; import { BashTool } from "@oh-my-pi/pi-coding-agent/tools/bash"; import { FindTool } from "@oh-my-pi/pi-coding-agent/tools/find"; import { GrepTool } from "@oh-my-pi/pi-coding-agent/tools/grep"; -import { LsTool } from "@oh-my-pi/pi-coding-agent/tools/ls"; import { wrapToolWithMetaNotice } from "@oh-my-pi/pi-coding-agent/tools/output-meta"; import { ReadTool } from "@oh-my-pi/pi-coding-agent/tools/read"; import { WriteTool } from "@oh-my-pi/pi-coding-agent/tools/write"; @@ -43,7 +42,6 @@ describe("Coding Agent Tools", () => { let bashTool: BashTool; let grepTool: GrepTool; let findTool: FindTool; - let lsTool: LsTool; let originalEditVariant: string | undefined; beforeEach(() => { @@ -63,7 +61,6 @@ describe("Coding Agent Tools", () => { bashTool = wrapToolWithMetaNotice(new BashTool(session)); grepTool = wrapToolWithMetaNotice(new GrepTool(session)); findTool = wrapToolWithMetaNotice(new FindTool(session)); - lsTool = wrapToolWithMetaNotice(new LsTool(session)); }); afterEach(() => { @@ -566,19 +563,6 @@ function b() { expect(output).not.toContain("ignored.txt"); }); }); - - describe("ls tool", () => { - it("should list dotfiles and directories", async () => { - fs.writeFileSync(path.join(testDir, ".hidden-file"), "secret"); - fs.mkdirSync(path.join(testDir, ".hidden-dir")); - - const result = await lsTool.execute("test-call-15", { path: testDir }); - const output = getTextOutput(result); - - expect(output).toContain(".hidden-file"); - expect(output).toContain(".hidden-dir/"); - }); - }); }); describe("edit tool CRLF handling", () => {