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.

This commit is contained in:
sebseb7
2025-08-14 10:22:27 +00:00
parent 421b47355b
commit 9974a78394
2 changed files with 27 additions and 17 deletions

View File

@@ -1,7 +1,7 @@
import { promises as fs } from "node:fs"; import { promises as fs } from "node:fs";
import path from "node:path"; 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) => { const normalizePath = (inputPath, chrootDir) => {
// Resolve chroot directory // Resolve chroot directory
const chrootResolved = path.resolve(chrootDir); const chrootResolved = path.resolve(chrootDir);
@@ -17,12 +17,19 @@ const normalizePath = (inputPath, chrootDir) => {
// Ensure the path is within chrootDir // Ensure the path is within chrootDir
if (!normalized.startsWith(chrootResolved)) { if (!normalized.startsWith(chrootResolved)) {
throw new Error(`Path escapes chroot boundary: ${inputPath}`); throw new Error(`Path escapes root boundary: ${inputPath}`);
} }
return normalized; 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 // Main recursive directory listing function
async function listEntriesRecursive(startDir, chrootDir, maxDepth, includeHidden = false) { async function listEntriesRecursive(startDir, chrootDir, maxDepth, includeHidden = false) {
const results = []; const results = [];
@@ -35,7 +42,7 @@ async function listEntriesRecursive(startDir, chrootDir, maxDepth, includeHidden
try { try {
dirents = await fs.readdir(currentDir, { withFileTypes: true }); dirents = await fs.readdir(currentDir, { withFileTypes: true });
} catch (err) { } 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) { for (const dirent of dirents) {
@@ -91,13 +98,13 @@ async function listEntriesRecursive(startDir, chrootDir, maxDepth, includeHidden
export default { export default {
type: "function", type: "function",
name: "list_files", 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: { parameters: {
type: "object", type: "object",
properties: { properties: {
path: { path: {
type: "string", 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: { depth: {
type: "integer", type: "integer",
@@ -123,7 +130,7 @@ export async function run(args) {
const chrootPath = '/workspaces/aiTools/root'; const chrootPath = '/workspaces/aiTools/root';
if (!chrootPath) { if (!chrootPath) {
return { err: "Chroot path is required" }; return { err: "Root path is required" };
} }
if (depth < -1) { if (depth < -1) {
return { err: `Depth must be >= -1, received ${args?.depth}` }; 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 // Auto-create the chroot base directory if it does not exist
await fs.mkdir(chrootResolved, { recursive: true }); await fs.mkdir(chrootResolved, { recursive: true });
} catch (err) { } catch (err) {
return { err: `Failed to prepare chroot path: ${chrootPath} (${err?.message || String(err)})` }; return { err: "Failed to initialize root directory" };
} }
let resolvedBase; let resolvedBase;
@@ -149,7 +156,7 @@ export async function run(args) {
try { try {
stat = await fs.lstat(resolvedBase); stat = await fs.lstat(resolvedBase);
} catch (err) { } 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) || "."; const cwd = path.relative(chrootResolved, stat.isFile() ? path.dirname(resolvedBase) : resolvedBase) || ".";
@@ -166,7 +173,7 @@ export async function run(args) {
// Handle non-directory case // Handle non-directory case
if (!stat.isDirectory()) { if (!stat.isDirectory()) {
return { err: `Not a file or directory: ${resolvedBase}` }; return { err: `Not a file or directory${inputPath ? `: ${inputPath}` : ""}` };
} }
// Handle directory case // Handle directory case
@@ -181,6 +188,6 @@ export async function run(args) {
files: mapped, files: mapped,
}; };
} catch (err) { } catch (err) {
return { err: `Failed to list files: ${err?.message || String(err)}` }; return { err: "Failed to list files" };
} }
} }

View File

@@ -126,10 +126,10 @@ function resolvePath(chroot, filepath) {
const root = normalizePath(chroot); const root = normalizePath(chroot);
// If file is absolute, use it as-is (after normalization). // If file is absolute, resolve it under the chroot rather than using host FS root
// We assume the caller ensures it is inside the chroot.
if (file.startsWith('/')) { if (file.startsWith('/')) {
return file; const resolvedAbs = joinPaths(root, file);
return resolvedAbs.startsWith('/') ? resolvedAbs : '/' + resolvedAbs;
} }
// If file is relative, join with chroot // 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) { function apply_commit(commit, write_fn, remove_fn, chroot = null) {
for (const [path, change] of Object.entries(commit.changes)) { for (const [path, change] of Object.entries(commit.changes)) {
if (change.type === ActionType.DELETE) { if (change.type === ActionType.DELETE) {
remove_fn(path); const target = resolvePath(chroot, path);
remove_fn(target);
} else if (change.type === ActionType.ADD) { } else if (change.type === ActionType.ADD) {
if (change.new_content === null) { if (change.new_content === null) {
throw new DiffError(`ADD change for ${path} has no content`); 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) { } else if (change.type === ActionType.UPDATE) {
if (change.new_content === null) { if (change.new_content === null) {
throw new DiffError(`UPDATE change for ${path} has no new content`); 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); write_fn(target, change.new_content);
if (change.move_path) { if (change.move_path) {
remove_fn(path); const source = resolvePath(chroot, path);
remove_fn(source);
} }
} }
} }