From 8e205fb8f4a45a09cf6d2c13987d7fd63f1aa7d1 Mon Sep 17 00:00:00 2001 From: Mohith Gajjela <109003762+Mohith26@users.noreply.github.com> Date: Tue, 28 Jul 2026 11:44:38 -0500 Subject: [PATCH 1/2] fix(skills): resolve skill file reads home-root first as documented The docs and system prompt state skill files resolve from $HOME/.agents/skills// first, with /workspace/skills// as the fallback, and skill packages are seeded into the home root (RUNTIME_SANDBOX_CONTRACT_VERSION 7, #554). But the model-facing read_file tool rejected the advertised $HOME/... form (absolute-path validation) and did no cross-root fallback, so in practice the only usable literal path was the workspace fallback: reads of /workspace/skills/... hard-failed with 'File not found' even though the file existed under the home root, forcing a slow model-driven retry loop (~2s per miss). read_file now resolves skill-rooted paths (symbolic $HOME form, concrete home-rooted form, or /workspace/skills form) to ordered candidates via resolveSkillFilePathCandidates: the probed home root first, then /workspace/skills, matching the documented order. Non-skill paths are unchanged. The result path and read stamp use the path that actually resolved so follow-up writes target the real file, and the not-found error lists every checked location. Fixes #1234 --- .changeset/skill-file-resolution-order.md | 5 ++ .../src/execution/sandbox/read-file-tool.ts | 33 ++++++++-- packages/eve/src/shared/skill-paths.ts | 60 +++++++++++++++++++ 3 files changed, 94 insertions(+), 4 deletions(-) create mode 100644 .changeset/skill-file-resolution-order.md diff --git a/.changeset/skill-file-resolution-order.md b/.changeset/skill-file-resolution-order.md new file mode 100644 index 000000000..840fe7525 --- /dev/null +++ b/.changeset/skill-file-resolution-order.md @@ -0,0 +1,5 @@ +--- +"eve": patch +--- + +Resolve skill file reads in the documented order: `$HOME/.agents/skills//` first, with `/workspace/skills//` as the fallback. `read_file` now accepts the symbolic `$HOME/.agents/skills/...` form and falls back across skill roots instead of failing when the model names the workspace path. diff --git a/packages/eve/src/execution/sandbox/read-file-tool.ts b/packages/eve/src/execution/sandbox/read-file-tool.ts index 3485c5585..536aceda1 100644 --- a/packages/eve/src/execution/sandbox/read-file-tool.ts +++ b/packages/eve/src/execution/sandbox/read-file-tool.ts @@ -6,6 +6,7 @@ import { setReadFileStamp, } from "#runtime/framework-tools/file-state.js"; import { validateAbsoluteFilePath } from "#execution/sandbox/require-sandbox.js"; +import { resolveSkillFilePathCandidates } from "#shared/skill-paths.js"; import type { SandboxSession } from "#shared/sandbox-session.js"; import { capLineLength, MAX_OUTPUT_BYTES } from "#execution/sandbox/truncate-output.js"; @@ -58,8 +59,19 @@ export async function executeReadFileOnSandbox( ): Promise { const { filePath, offset, limit } = args; - validateAbsoluteFilePath(filePath); - const normalizedPath = normalizeModelPath(filePath); + // ── Resolve skill-root candidates (documented order: $HOME first) ─── + // Skill files are documented to resolve from `$HOME/.agents/skills/` + // first with `/workspace/skills/` as the fallback. The model may name + // either root (or the symbolic `$HOME` form); reads honor the + // documented order regardless. Non-skill paths resolve to themselves. + const candidatePaths = await resolveSkillFilePathCandidates({ + path: filePath, + sandbox, + }); + + for (const candidatePath of candidatePaths) { + validateAbsoluteFilePath(candidatePath); + } // ── Validate offset / limit ───────────────────────────────────────── const effectiveOffset = offset ?? DEFAULT_OFFSET; @@ -70,14 +82,27 @@ export async function executeReadFileOnSandbox( } // ── Read full file for fingerprinting ─────────────────────────────── - const rawContent = await sandbox.readTextFile({ path: filePath }); + let rawContent: string | null = null; + let resolvedReadPath = candidatePaths[0] ?? filePath; + + for (const candidatePath of candidatePaths) { + rawContent = await sandbox.readTextFile({ path: candidatePath }); + if (rawContent !== null) { + resolvedReadPath = candidatePath; + break; + } + } if (rawContent === null) { + const checkedLocations = + candidatePaths.length > 1 ? ` Checked: ${candidatePaths.join(", ")}.` : ""; throw new Error( - `File not found: ${filePath}. Verify the path exists and is accessible in the sandbox.`, + `File not found: ${filePath}.${checkedLocations} Verify the path exists and is accessible in the sandbox.`, ); } + const normalizedPath = normalizeModelPath(resolvedReadPath); + // ── Reject non-text (NUL bytes) ───────────────────────────────────── if (rawContent.includes("\0")) { throw new Error( diff --git a/packages/eve/src/shared/skill-paths.ts b/packages/eve/src/shared/skill-paths.ts index fd5b91fc1..b16ef04d2 100644 --- a/packages/eve/src/shared/skill-paths.ts +++ b/packages/eve/src/shared/skill-paths.ts @@ -68,6 +68,42 @@ export async function resolveSandboxSkillWritePath(input: { }); } +/** + * Resolves the ordered candidate read paths for one model-supplied file + * path that targets a skill file. + * + * Implements the documented resolution order: the `$HOME/.agents/skills` + * root is checked first, with `/workspace/skills` as the fallback when the + * file is absent there or `$HOME` is unavailable. Accepts the symbolic + * `$HOME/.agents/skills/...` form the system prompt advertises, a concrete + * home-rooted `.agents/skills` path, or a `/workspace/skills/...` path. + * Paths that do not target a skill root pass through unchanged as the + * single candidate. + */ +export async function resolveSkillFilePathCandidates(input: { + readonly path: string; + readonly sandbox: SandboxSession; +}): Promise { + const relativePath = extractSkillFileRelativePath(input.path); + if (relativePath === null) { + return [input.path]; + } + + const root = await resolveSandboxSkillRoot({ sandbox: input.sandbox }); + const candidates = [`${root}/${relativePath}`]; + + if (input.path !== candidates[0] && !input.path.startsWith("$")) { + candidates.push(input.path); + } + + const fallbackPath = `${FALLBACK_SKILL_ROOT}/${relativePath}`; + if (!candidates.includes(fallbackPath)) { + candidates.push(fallbackPath); + } + + return candidates; +} + export async function resolveSandboxSeedFilePath(input: { readonly path: string; readonly sandbox: SandboxSession; @@ -80,6 +116,30 @@ export async function resolveSandboxSeedFilePath(input: { return `${root}${input.path.slice(MODEL_SKILL_ROOT.length)}`; } +function extractSkillFileRelativePath(path: string): string | null { + const modelPrefix = `${MODEL_SKILL_ROOT}/`; + if (path.startsWith(modelPrefix)) { + return emptyToNull(path.slice(modelPrefix.length)); + } + + const fallbackPrefix = `${FALLBACK_SKILL_ROOT}/`; + if (path.startsWith(fallbackPrefix)) { + return emptyToNull(path.slice(fallbackPrefix.length)); + } + + const homeMarker = `/${HOME_SKILL_SUFFIX}/`; + const markerIndex = path.indexOf(homeMarker); + if (markerIndex !== -1) { + return emptyToNull(path.slice(markerIndex + homeMarker.length)); + } + + return null; +} + +function emptyToNull(value: string): string | null { + return value.length > 0 ? value : null; +} + function formatSkillPath(input: { readonly name: string; readonly relativePath: string; From 30a5de6f1afddffb9d93eb59269a4c5d850a7627 Mon Sep 17 00:00:00 2001 From: Mohith Gajjela <109003762+Mohith26@users.noreply.github.com> Date: Tue, 28 Jul 2026 11:44:39 -0500 Subject: [PATCH 2/2] test(skills): cover documented skill file resolution order Regression tests for #1234: - resolveSkillFilePathCandidates orders the probed home root before the /workspace/skills fallback, resolves the symbolic $HOME form, collapses to the fallback when $HOME is unusable, passes non-skill paths through without probing, and keeps a distinct concrete home path as a candidate. - executeReadFileOnSandbox prefers the home copy when a skill file exists in both roots, falls back to /workspace/skills on a home miss, accepts the symbolic $HOME path the prompt advertises, lists both checked locations in the not-found error, and stamps the resolved path for read-before-write tracking. --- packages/eve/src/shared/skill-paths.test.ts | 89 ++++++++++++++++ packages/eve/test/read-file-tool.test.ts | 106 +++++++++++++++++++- 2 files changed, 192 insertions(+), 3 deletions(-) diff --git a/packages/eve/src/shared/skill-paths.test.ts b/packages/eve/src/shared/skill-paths.test.ts index cea8261c7..4d1da451d 100644 --- a/packages/eve/src/shared/skill-paths.test.ts +++ b/packages/eve/src/shared/skill-paths.test.ts @@ -9,6 +9,7 @@ import { resolveSandboxSeedFilePath, resolveSandboxSkillReadPaths, resolveSandboxSkillRoot, + resolveSkillFilePathCandidates, } from "#shared/skill-paths.js"; const HOME_PROBE_COMMAND = `printf '%s\\n' "$HOME"`; @@ -97,4 +98,92 @@ describe("skill path helpers", () => { }), ).resolves.toBe("/home/agent/.agents/skills/research/references/catalog.md"); }); + + describe("resolveSkillFilePathCandidates", () => { + it("orders the HOME root before the /workspace fallback for a workspace skill path", async () => { + const sandbox = mockSandbox({ + commands: { + [HOME_PROBE_COMMAND]: { exitCode: 0, stderr: "", stdout: "/home/agent\n" }, + }, + }); + + await expect( + resolveSkillFilePathCandidates({ + path: `${FALLBACK_SKILL_ROOT}/research/references/notes.md`, + sandbox: sandbox.session, + }), + ).resolves.toEqual([ + "/home/agent/.agents/skills/research/references/notes.md", + "/workspace/skills/research/references/notes.md", + ]); + }); + + it("resolves the symbolic $HOME skill path to concrete candidates", async () => { + const sandbox = mockSandbox({ + commands: { + [HOME_PROBE_COMMAND]: { exitCode: 0, stderr: "", stdout: "/home/agent\n" }, + }, + }); + + await expect( + resolveSkillFilePathCandidates({ + path: `${MODEL_SKILL_ROOT}/research/SKILL.md`, + sandbox: sandbox.session, + }), + ).resolves.toEqual([ + "/home/agent/.agents/skills/research/SKILL.md", + "/workspace/skills/research/SKILL.md", + ]); + }); + + it("collapses to the /workspace fallback when HOME is unusable", async () => { + const sandbox = mockSandbox({ + commands: { + [HOME_PROBE_COMMAND]: { exitCode: 0, stderr: "", stdout: "\n" }, + }, + }); + + await expect( + resolveSkillFilePathCandidates({ + path: `${MODEL_SKILL_ROOT}/research/SKILL.md`, + sandbox: sandbox.session, + }), + ).resolves.toEqual(["/workspace/skills/research/SKILL.md"]); + }); + + it("passes non-skill paths through unchanged without probing HOME", async () => { + const sandbox = mockSandbox({ + commands: { + [HOME_PROBE_COMMAND]: { exitCode: 0, stderr: "", stdout: "/home/agent\n" }, + }, + }); + + await expect( + resolveSkillFilePathCandidates({ + path: "/workspace/src/index.ts", + sandbox: sandbox.session, + }), + ).resolves.toEqual(["/workspace/src/index.ts"]); + expect(sandbox.commandLog).toEqual([]); + }); + + it("keeps a distinct concrete home path as a candidate after the resolved root", async () => { + const sandbox = mockSandbox({ + commands: { + [HOME_PROBE_COMMAND]: { exitCode: 0, stderr: "", stdout: "/home/agent\n" }, + }, + }); + + await expect( + resolveSkillFilePathCandidates({ + path: "/root/.agents/skills/research/SKILL.md", + sandbox: sandbox.session, + }), + ).resolves.toEqual([ + "/home/agent/.agents/skills/research/SKILL.md", + "/root/.agents/skills/research/SKILL.md", + "/workspace/skills/research/SKILL.md", + ]); + }); + }); }); diff --git a/packages/eve/test/read-file-tool.test.ts b/packages/eve/test/read-file-tool.test.ts index f5bb52f0b..42a101868 100644 --- a/packages/eve/test/read-file-tool.test.ts +++ b/packages/eve/test/read-file-tool.test.ts @@ -14,7 +14,7 @@ import { ReadFileStateKey } from "../src/runtime/framework-tools/file-state.js"; // Helpers // --------------------------------------------------------------------------- -function createFakeAccess(files: Record): SandboxAccess { +function createFakeAccess(files: Record, home?: string): SandboxAccess { return { async captureState() { return { initialized: false, session: null }; @@ -40,7 +40,7 @@ function createFakeAccess(files: Record): SandboxAccess { return path; }, async run(_options: { command: string }) { - return { exitCode: 0, stderr: "", stdout: "" }; + return { exitCode: 0, stderr: "", stdout: home === undefined ? "" : `${home}\n` }; }, async spawn() { return stubSpawnProcess(); @@ -56,8 +56,9 @@ function createFakeAccess(files: Record): SandboxAccess { async function runInContext( files: Record, fn: (sandbox: SandboxSession) => Promise, + home?: string, ): Promise { - const access = createFakeAccess(files); + const access = createFakeAccess(files, home); const ctx = new ContextContainer(); ctx.set(SandboxKey, access); ctx.set(ReadFileStateKey, { byTarget: {} }); @@ -300,4 +301,103 @@ describe("executeReadFileOnSandbox", () => { // The stamp should be under the normalized path expect(state.byTarget["/workspace/foo.ts"]).toBeDefined(); }); + + // --------------------------------------------------------------------------- + // Skill file resolution order (docs: $HOME/.agents/skills first, + // /workspace/skills as the fallback — see issue #1234) + // --------------------------------------------------------------------------- + + describe("skill file resolution order", () => { + const HOME = "/home/agent"; + const HOME_PATH = `${HOME}/.agents/skills/research/references/notes.md`; + const WORKSPACE_PATH = "/workspace/skills/research/references/notes.md"; + + it("prefers the $HOME skill root when the file exists in both locations", async () => { + const result = (await runInContext( + { + [HOME_PATH]: "home copy\n", + [WORKSPACE_PATH]: "workspace copy\n", + }, + (sandbox) => executeReadFileOnSandbox(sandbox, { filePath: WORKSPACE_PATH }), + HOME, + )) as ReadFileResult; + + expect(result.content).toContain("1: home copy"); + expect(result.path).toBe(HOME_PATH); + }); + + it("falls back to /workspace/skills when the $HOME root misses", async () => { + const result = (await runInContext( + { [WORKSPACE_PATH]: "workspace copy\n" }, + (sandbox) => executeReadFileOnSandbox(sandbox, { filePath: WORKSPACE_PATH }), + HOME, + )) as ReadFileResult; + + expect(result.content).toContain("1: workspace copy"); + expect(result.path).toBe(WORKSPACE_PATH); + }); + + it("resolves the symbolic $HOME skill path advertised in the prompt", async () => { + const result = (await runInContext( + { [HOME_PATH]: "home copy\n" }, + (sandbox) => + executeReadFileOnSandbox(sandbox, { + filePath: "$HOME/.agents/skills/research/references/notes.md", + }), + HOME, + )) as ReadFileResult; + + expect(result.content).toContain("1: home copy"); + expect(result.path).toBe(HOME_PATH); + }); + + it("reads a concrete home-rooted skill path directly", async () => { + const result = (await runInContext( + { [HOME_PATH]: "home copy\n" }, + (sandbox) => executeReadFileOnSandbox(sandbox, { filePath: HOME_PATH }), + HOME, + )) as ReadFileResult; + + expect(result.content).toContain("1: home copy"); + expect(result.path).toBe(HOME_PATH); + }); + + it("resolves skill paths to /workspace/skills when $HOME is unusable", async () => { + const result = (await runInContext({ [WORKSPACE_PATH]: "workspace copy\n" }, (sandbox) => + executeReadFileOnSandbox(sandbox, { + filePath: "$HOME/.agents/skills/research/references/notes.md", + }), + )) as ReadFileResult; + + expect(result.content).toContain("1: workspace copy"); + expect(result.path).toBe(WORKSPACE_PATH); + }); + + it("lists both checked locations when a skill file is missing everywhere", async () => { + await expect( + runInContext( + {}, + (sandbox) => executeReadFileOnSandbox(sandbox, { filePath: WORKSPACE_PATH }), + HOME, + ), + ).rejects.toThrow(`Checked: ${HOME_PATH}, ${WORKSPACE_PATH}.`); + }); + + it("stamps the resolved path so follow-up writes target the real file", async () => { + const access = createFakeAccess({ [HOME_PATH]: "home copy\n" }, HOME); + const sandbox = (await access.get())!; + + const ctx = new ContextContainer(); + ctx.set(SandboxKey, access); + ctx.set(ReadFileStateKey, { byTarget: {} }); + + await contextStorage.run(ctx, () => + executeReadFileOnSandbox(sandbox, { filePath: WORKSPACE_PATH }), + ); + + const state = ctx.require(ReadFileStateKey); + expect(state.byTarget[HOME_PATH]).toBeDefined(); + expect(state.byTarget[WORKSPACE_PATH]).toBeUndefined(); + }); + }); });