From 9974a78394a05cd9cf90b64fb2bf95206fba4ca9 Mon Sep 17 00:00:00 2001 From: sebseb7 Date: Thu, 14 Aug 2025 10:22:27 +0000 Subject: [PATCH] Refactor path handling in list_files.js and patch_files.js for improved clarity and consistency. Update error messages to reflect 'root' terminology instead of 'chroot' and enhance path resolution logic for better handling of absolute and relative paths. --- tools/list_files.js | 27 +++++++++++++++++---------- tools/patch_files.js | 17 ++++++++++------- 2 files changed, 27 insertions(+), 17 deletions(-) diff --git a/tools/list_files.js b/tools/list_files.js index cdaac44..e8fd75b 100644 --- a/tools/list_files.js +++ b/tools/list_files.js @@ -1,7 +1,7 @@ import { promises as fs } from "node:fs"; import path from "node:path"; -// Utility to normalize and validate paths within chroot +// Utility to normalize and validate paths within a contained root directory const normalizePath = (inputPath, chrootDir) => { // Resolve chroot directory const chrootResolved = path.resolve(chrootDir); @@ -17,12 +17,19 @@ const normalizePath = (inputPath, chrootDir) => { // Ensure the path is within chrootDir if (!normalized.startsWith(chrootResolved)) { - throw new Error(`Path escapes chroot boundary: ${inputPath}`); + throw new Error(`Path escapes root boundary: ${inputPath}`); } return normalized; }; +// Convert an absolute path under the contained root to a user-display path (root-relative) +const toDisplayPath = (absPath, chrootDir) => { + const rel = path.relative(path.resolve(chrootDir), absPath); + if (!rel || rel === "") return "/"; + return `/${rel}`; +}; + // Main recursive directory listing function async function listEntriesRecursive(startDir, chrootDir, maxDepth, includeHidden = false) { const results = []; @@ -35,7 +42,7 @@ async function listEntriesRecursive(startDir, chrootDir, maxDepth, includeHidden try { dirents = await fs.readdir(currentDir, { withFileTypes: true }); } catch (err) { - throw new Error(`Failed to read directory: ${currentDir} (${err?.message || String(err)})`); + throw new Error(`Failed to read directory: ${toDisplayPath(currentDir, chrootDir)}`); } for (const dirent of dirents) { @@ -91,13 +98,13 @@ async function listEntriesRecursive(startDir, chrootDir, maxDepth, includeHidden export default { type: "function", name: "list_files", - description: "List files and directories recursively within a chroot directory with customizable options", + description: "List files and directories recursively from the root with customizable options", parameters: { type: "object", properties: { path: { type: "string", - description: "Directory or file path to list relative to chroot. Use '/' for the chroot root. Defaults to chroot root if not specified.", + description: "Directory or file path relative to the root. Use '/' for the root. Defaults to root if not specified.", }, depth: { type: "integer", @@ -123,7 +130,7 @@ export async function run(args) { const chrootPath = '/workspaces/aiTools/root'; if (!chrootPath) { - return { err: "Chroot path is required" }; + return { err: "Root path is required" }; } if (depth < -1) { return { err: `Depth must be >= -1, received ${args?.depth}` }; @@ -135,7 +142,7 @@ export async function run(args) { // Auto-create the chroot base directory if it does not exist await fs.mkdir(chrootResolved, { recursive: true }); } catch (err) { - return { err: `Failed to prepare chroot path: ${chrootPath} (${err?.message || String(err)})` }; + return { err: "Failed to initialize root directory" }; } let resolvedBase; @@ -149,7 +156,7 @@ export async function run(args) { try { stat = await fs.lstat(resolvedBase); } catch (err) { - return { err: `Path does not exist: ${resolvedBase} (${err?.message || String(err)})` }; + return { err: `Path does not exist${inputPath ? `: ${inputPath}` : ""}` }; } const cwd = path.relative(chrootResolved, stat.isFile() ? path.dirname(resolvedBase) : resolvedBase) || "."; @@ -166,7 +173,7 @@ export async function run(args) { // Handle non-directory case if (!stat.isDirectory()) { - return { err: `Not a file or directory: ${resolvedBase}` }; + return { err: `Not a file or directory${inputPath ? `: ${inputPath}` : ""}` }; } // Handle directory case @@ -181,6 +188,6 @@ export async function run(args) { files: mapped, }; } catch (err) { - return { err: `Failed to list files: ${err?.message || String(err)}` }; + return { err: "Failed to list files" }; } } \ No newline at end of file diff --git a/tools/patch_files.js b/tools/patch_files.js index 1a8ac2d..e125d65 100644 --- a/tools/patch_files.js +++ b/tools/patch_files.js @@ -126,10 +126,10 @@ function resolvePath(chroot, filepath) { const root = normalizePath(chroot); - // If file is absolute, use it as-is (after normalization). - // We assume the caller ensures it is inside the chroot. + // If file is absolute, resolve it under the chroot rather than using host FS root if (file.startsWith('/')) { - return file; + const resolvedAbs = joinPaths(root, file); + return resolvedAbs.startsWith('/') ? resolvedAbs : '/' + resolvedAbs; } // If file is relative, join with chroot @@ -730,20 +730,23 @@ function load_files(paths, open_fn) { function apply_commit(commit, write_fn, remove_fn, chroot = null) { for (const [path, change] of Object.entries(commit.changes)) { if (change.type === ActionType.DELETE) { - remove_fn(path); + const target = resolvePath(chroot, path); + remove_fn(target); } else if (change.type === ActionType.ADD) { if (change.new_content === null) { throw new DiffError(`ADD change for ${path} has no content`); } - write_fn(path, change.new_content); + const target = resolvePath(chroot, path); + write_fn(target, change.new_content); } else if (change.type === ActionType.UPDATE) { if (change.new_content === null) { throw new DiffError(`UPDATE change for ${path} has no new content`); } - const target = change.move_path ? resolvePath(chroot, change.move_path) : path; + const target = change.move_path ? resolvePath(chroot, change.move_path) : resolvePath(chroot, path); write_fn(target, change.new_content); if (change.move_path) { - remove_fn(path); + const source = resolvePath(chroot, path); + remove_fn(source); } } }