fix(core,skills): diagnose missing paths in read_file/edit_file instead of implying absolute paths are rejected (#155)
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
1 parent
aeffc8a02f
commit
835dfb5c5a
5 files changed
+138
-7
No files matched your search
@@ -28,6 +28,7 @@ import type { ToolDefinitionConfig } from "../../interfaces.js";
|
|||||||
import type { BuiltinTool, ToolExecutionContext, ToolResult } from "./types.js";
|
import type { BuiltinTool, ToolExecutionContext, ToolResult } from "./types.js";
|
||||||
import { atomicWriteFile } from "./file-utils.js";
|
import { atomicWriteFile } from "./file-utils.js";
|
||||||
import { buildReplacementHunks, renderHunk } from "./diff.js";
|
import { buildReplacementHunks, renderHunk } from "./diff.js";
|
||||||
|
import { missingPathHint } from "./path-hint.js";
|
||||||
|
|
||||||
/** Tool name constant (used only within this tool module, never exposed to Environment). */
|
/** Tool name constant (used only within this tool module, never exposed to Environment). */
|
||||||
export const EDIT_FILE_NAME = "edit_file";
|
export const EDIT_FILE_NAME = "edit_file";
|
||||||
@@ -113,9 +114,12 @@ export function createEditFileTool(definition: ToolDefinitionConfig): BuiltinToo
|
|||||||
} catch (err) {
|
} catch (err) {
|
||||||
if (signal?.aborted) return { stopReason: "aborted" };
|
if (signal?.aborted) return { stopReason: "aborted" };
|
||||||
const code = (err as NodeJS.ErrnoException).code;
|
const code = (err as NodeJS.ErrnoException).code;
|
||||||
if (code === "ENOENT") {
|
// ENOTDIR is the same mistake seen one segment later (a file used as a directory),
|
||||||
|
// so it gets the same diagnosis instead of a raw errno message.
|
||||||
|
if (code === "ENOENT" || code === "ENOTDIR") {
|
||||||
|
const hint = await missingPathHint(resolved);
|
||||||
yield delta(
|
yield delta(
|
||||||
`File not found: "${filePath}". edit_file only edits existing files — check the path, or use write_file to create it.`,
|
`File not found: "${filePath}". edit_file only edits existing files — check the path (absolute paths are supported), or use write_file to create it.${hint}`,
|
||||||
);
|
);
|
||||||
} else {
|
} else {
|
||||||
const message = err instanceof Error ? err.message : String(err);
|
const message = err instanceof Error ? err.message : String(err);
|
||||||
|
|||||||
@@ -0,0 +1,74 @@
|
|||||||
|
/**
|
||||||
|
* path-hint — shared missing-path diagnostic for the file tools (read_file / edit_file).
|
||||||
|
*
|
||||||
|
* A bare "File not found" regularly sends the model down the wrong road: it retries
|
||||||
|
* relative/absolute variants of the same wrong path instead of questioning the path itself
|
||||||
|
* (issue #138 — a missing `agent_state/` segment read as "absolute paths are rejected").
|
||||||
|
* The cure is to show where the path leaves reality: walk up to the deepest existing
|
||||||
|
* ancestor and list the entries closest in name to the missing segment, so the next call
|
||||||
|
* can be the right one. Failure-path only — the extra stats/readdir never run on success.
|
||||||
|
*/
|
||||||
|
import path from "node:path";
|
||||||
|
import { readdir, stat } from "node:fs/promises";
|
||||||
|
import type { Dirent } from "node:fs";
|
||||||
|
import { modelVisiblePath } from "../../internal/model-visible-path.js";
|
||||||
|
|
||||||
|
/** Max directory entries named in the hint; the rest collapse into a "+N more" count. */
|
||||||
|
const MAX_LISTED_ENTRIES = 8;
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Entry labels ranked by longest shared case-insensitive prefix with the missing name
|
||||||
|
* (near-misses like `agent_state/` vs `AGENTS.md` surface first), ties alphabetical.
|
||||||
|
* Directories get a trailing "/" so the model can tell what one more segment would hit.
|
||||||
|
*/
|
||||||
|
function rankEntries(entries: Dirent[], missing: string): string[] {
|
||||||
|
const target = missing.toLowerCase();
|
||||||
|
const scored = entries.map((entry) => {
|
||||||
|
const name = entry.name.toLowerCase();
|
||||||
|
const max = Math.min(name.length, target.length);
|
||||||
|
let prefix = 0;
|
||||||
|
while (prefix < max && name[prefix] === target[prefix]) prefix += 1;
|
||||||
|
return { entry, prefix };
|
||||||
|
});
|
||||||
|
scored.sort((a, b) => b.prefix - a.prefix || a.entry.name.localeCompare(b.entry.name));
|
||||||
|
return scored.map(({ entry }) => (entry.isDirectory() ? `${entry.name}/` : entry.name));
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Diagnostic sentence(s) for a path that failed to stat, prefixed with a space so callers
|
||||||
|
* can append it verbatim to their own message ("" when nothing useful can be said).
|
||||||
|
* Reports the deepest existing ancestor, the first missing segment, and the ancestor's
|
||||||
|
* nearest-named entries — or that the "ancestor" is a file (the ENOTDIR case).
|
||||||
|
*/
|
||||||
|
export async function missingPathHint(resolved: string): Promise<string> {
|
||||||
|
let ancestor = path.dirname(resolved);
|
||||||
|
let missing = path.basename(resolved);
|
||||||
|
for (;;) {
|
||||||
|
try {
|
||||||
|
const st = await stat(ancestor);
|
||||||
|
if (!st.isDirectory()) {
|
||||||
|
return ` Note: "${modelVisiblePath(ancestor)}" exists but is a file, not a directory.`;
|
||||||
|
}
|
||||||
|
break;
|
||||||
|
} catch {
|
||||||
|
// Any stat failure (missing, unreadable) keeps the walk going: report the deepest
|
||||||
|
// ancestor that is *visibly* there. dirname is a fixpoint at the filesystem root.
|
||||||
|
const parent = path.dirname(ancestor);
|
||||||
|
if (parent === ancestor) return "";
|
||||||
|
missing = path.basename(ancestor);
|
||||||
|
ancestor = parent;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
const head = ` The directory "${modelVisiblePath(ancestor)}" exists but has no entry "${missing}".`;
|
||||||
|
let entries: Dirent[];
|
||||||
|
try {
|
||||||
|
entries = await readdir(ancestor, { withFileTypes: true });
|
||||||
|
} catch {
|
||||||
|
return head;
|
||||||
|
}
|
||||||
|
if (entries.length === 0) return `${head} It is empty.`;
|
||||||
|
const labels = rankEntries(entries, missing);
|
||||||
|
const shown = labels.slice(0, MAX_LISTED_ENTRIES);
|
||||||
|
const hidden = labels.length - shown.length;
|
||||||
|
return `${head} It contains: ${shown.join(", ")}${hidden > 0 ? ` (+${hidden} more)` : ""}.`;
|
||||||
|
}
|
||||||
@@ -33,6 +33,7 @@ import { open, realpath, stat } from "node:fs/promises";
|
|||||||
import { partialToolCallOutput } from "../../omnimessage/index.js";
|
import { partialToolCallOutput } from "../../omnimessage/index.js";
|
||||||
import type { OmniMessage } from "../../omnimessage/index.js";
|
import type { OmniMessage } from "../../omnimessage/index.js";
|
||||||
import type { ToolDefinitionConfig } from "../../interfaces.js";
|
import type { ToolDefinitionConfig } from "../../interfaces.js";
|
||||||
|
import { missingPathHint } from "./path-hint.js";
|
||||||
import type { BuiltinTool, ToolExecutionContext, ToolResult } from "./types.js";
|
import type { BuiltinTool, ToolExecutionContext, ToolResult } from "./types.js";
|
||||||
|
|
||||||
/** Tool name constant (used only within this tool module, never exposed to Environment). */
|
/** Tool name constant (used only within this tool module, never exposed to Environment). */
|
||||||
@@ -299,9 +300,12 @@ export function createReadFileTool(definition: ToolDefinitionConfig): BuiltinToo
|
|||||||
} catch (err) {
|
} catch (err) {
|
||||||
if (signal?.aborted) return { stopReason: "aborted" };
|
if (signal?.aborted) return { stopReason: "aborted" };
|
||||||
const code = (err as NodeJS.ErrnoException).code;
|
const code = (err as NodeJS.ErrnoException).code;
|
||||||
if (code === "ENOENT") {
|
// ENOTDIR is the same mistake seen one segment later (a file used as a directory),
|
||||||
|
// so it gets the same diagnosis instead of a raw errno message.
|
||||||
|
if (code === "ENOENT" || code === "ENOTDIR") {
|
||||||
|
const hint = await missingPathHint(resolved);
|
||||||
yield delta(
|
yield delta(
|
||||||
`File not found: "${filePath}". Check the path — relative paths resolve against the workspace (${modelVisiblePath(ctx.workspaceDir)}).`,
|
`File not found: "${filePath}". Absolute paths are supported; relative paths resolve against the workspace (${modelVisiblePath(ctx.workspaceDir)}).${hint}`,
|
||||||
);
|
);
|
||||||
} else {
|
} else {
|
||||||
const message = err instanceof Error ? err.message : String(err);
|
const message = err instanceof Error ? err.message : String(err);
|
||||||
|
|||||||
@@ -31,6 +31,7 @@ import { WRITE_FILE_NAME, createWriteFileTool } from "../src/environment/tools/w
|
|||||||
import type { BuiltinTool, ToolResult } from "../src/environment/tools/types.js";
|
import type { BuiltinTool, ToolResult } from "../src/environment/tools/types.js";
|
||||||
import type { OmniMessage } from "../src/omnimessage/index.js";
|
import type { OmniMessage } from "../src/omnimessage/index.js";
|
||||||
import type { ToolDefinitionConfig } from "../src/interfaces.js";
|
import type { ToolDefinitionConfig } from "../src/interfaces.js";
|
||||||
|
import { modelVisiblePath } from "../src/internal/model-visible-path.js";
|
||||||
|
|
||||||
function def(name: string, permission: "r" | "rw"): ToolDefinitionConfig {
|
function def(name: string, permission: "r" | "rw"): ToolDefinitionConfig {
|
||||||
return { name, description: "test", permission };
|
return { name, description: "test", permission };
|
||||||
@@ -128,6 +129,51 @@ describe("read_file", () => {
|
|||||||
expect(text).toContain("File not found");
|
expect(text).toContain("File not found");
|
||||||
expect(text).toContain("missing.txt");
|
expect(text).toContain("missing.txt");
|
||||||
expect(text).toContain("workspace");
|
expect(text).toContain("workspace");
|
||||||
|
expect(text).toContain("Absolute paths are supported");
|
||||||
|
expect(text).toContain(
|
||||||
|
`The directory "${modelVisiblePath(tmp)}" exists but has no entry "missing.txt".`,
|
||||||
|
);
|
||||||
|
expect(text).toContain("It is empty.");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("points at the nearest existing directory and its closest-named entries", async () => {
|
||||||
|
await mkdir(path.join(tmp, "agent_state"));
|
||||||
|
await writeFile(path.join(tmp, "agent_state", "AGENTS.md"), "persona\n");
|
||||||
|
await writeFile(path.join(tmp, "notes.txt"), "n\n");
|
||||||
|
const { result, text } = await run(tool(), { file_path: path.join(tmp, "AGENTS.md") }, tmp);
|
||||||
|
expect(result?.stopReason).toBe("failed");
|
||||||
|
expect(text).toContain(
|
||||||
|
`The directory "${modelVisiblePath(tmp)}" exists but has no entry "AGENTS.md".`,
|
||||||
|
);
|
||||||
|
// Ranked by shared prefix with the missing name: agent_state/ first, dirs marked with "/".
|
||||||
|
expect(text).toContain("It contains: agent_state/, notes.txt");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("reports the first missing segment of a deeper path", async () => {
|
||||||
|
const missing = path.join(tmp, "nope", "deep", "file.txt");
|
||||||
|
const { result, text } = await run(tool(), { file_path: missing }, tmp);
|
||||||
|
expect(result?.stopReason).toBe("failed");
|
||||||
|
expect(text).toContain(
|
||||||
|
`The directory "${modelVisiblePath(tmp)}" exists but has no entry "nope".`,
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("says so when a path segment is a file, not a directory", async () => {
|
||||||
|
await writeFile(path.join(tmp, "a.txt"), "x\n");
|
||||||
|
const inner = path.join(tmp, "a.txt", "inner.txt");
|
||||||
|
const { result, text } = await run(tool(), { file_path: inner }, tmp);
|
||||||
|
expect(result?.stopReason).toBe("failed");
|
||||||
|
expect(text).toContain("File not found");
|
||||||
|
expect(text).toContain(
|
||||||
|
`Note: "${modelVisiblePath(path.join(tmp, "a.txt"))}" exists but is a file, not a directory.`,
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("caps the entry listing and counts the rest", async () => {
|
||||||
|
for (let i = 0; i < 10; i += 1) await writeFile(path.join(tmp, `e${i}.txt`), "x\n");
|
||||||
|
const { text } = await run(tool(), { file_path: "zz.txt" }, tmp);
|
||||||
|
expect(text).toContain("(+2 more)");
|
||||||
|
expect(text).not.toContain("e8.txt");
|
||||||
});
|
});
|
||||||
|
|
||||||
it("fails when the path is a directory", async () => {
|
it("fails when the path is a directory", async () => {
|
||||||
@@ -250,6 +296,9 @@ describe("edit_file", () => {
|
|||||||
expect(result?.stopReason).toBe("failed");
|
expect(result?.stopReason).toBe("failed");
|
||||||
expect(text).toContain("File not found");
|
expect(text).toContain("File not found");
|
||||||
expect(text).toContain("write_file");
|
expect(text).toContain("write_file");
|
||||||
|
expect(text).toContain(
|
||||||
|
`The directory "${modelVisiblePath(tmp)}" exists but has no entry "nope.txt".`,
|
||||||
|
);
|
||||||
});
|
});
|
||||||
|
|
||||||
it("fails when the path is a directory", async () => {
|
it("fails when the path is a directory", async () => {
|
||||||
|
|||||||
@@ -3,8 +3,8 @@ name: agent-creation
|
|||||||
description: Create or configure an Agent State from a user requirement by writing AGENTS.md, setting identity metadata, and installing only needed Skills.
|
description: Create or configure an Agent State from a user requirement by writing AGENTS.md, setting identity metadata, and installing only needed Skills.
|
||||||
short_description: Turn a requirement into a working agent.
|
short_description: Turn a requirement into a working agent.
|
||||||
short_description_zh: 把需求变成可用的 Agent。
|
short_description_zh: 把需求变成可用的 Agent。
|
||||||
version: 6
|
version: 7
|
||||||
updated: 2026-07-29T17:20:58Z
|
updated: 2026-08-03T04:03:59Z
|
||||||
---
|
---
|
||||||
|
|
||||||
# Agent Creation
|
# Agent Creation
|
||||||
@@ -89,7 +89,7 @@ mkdir -p "$TARGET/agent_state/skills" "$TARGET/agent_state/memory" "$TARGET/agen
|
|||||||
cp "$APP_DATA_DIR/agents/default_agent/agent_state/system_config.yaml" "$TARGET/agent_state/"
|
cp "$APP_DATA_DIR/agents/default_agent/agent_state/system_config.yaml" "$TARGET/agent_state/"
|
||||||
```
|
```
|
||||||
|
|
||||||
Then set the top-level `name`, `description`, and `version: 1`, set `model.thinking_level` to the resolved value, write `AGENTS.md`, and install only the Skills required by the user's requirement. Do not persist the resolved provider/model pair in the Agent State.
|
Then set the top-level `name`, `description`, and `version: 1`, set `model.thinking_level` to the resolved value, write `agent_state/AGENTS.md` (it lives under `agent_state/`, not at the agent directory root), and install only the Skills required by the user's requirement. Do not persist the resolved provider/model pair in the Agent State.
|
||||||
|
|
||||||
## Validate and report
|
## Validate and report
|
||||||
|
|
||||||
|
|||||||
Reference in new issue
Block a user