Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/skill-file-resolution-order.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"eve": patch
---

Resolve skill file reads in the documented order: `$HOME/.agents/skills/<skill>/` first, with `/workspace/skills/<skill>/` 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.
33 changes: 29 additions & 4 deletions packages/eve/src/execution/sandbox/read-file-tool.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";

Expand Down Expand Up @@ -58,8 +59,19 @@ export async function executeReadFileOnSandbox(
): Promise<ReadFileResult> {
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;
Expand All @@ -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(
Expand Down
89 changes: 89 additions & 0 deletions packages/eve/src/shared/skill-paths.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
resolveSandboxSeedFilePath,
resolveSandboxSkillReadPaths,
resolveSandboxSkillRoot,
resolveSkillFilePathCandidates,
} from "#shared/skill-paths.js";

const HOME_PROBE_COMMAND = `printf '%s\\n' "$HOME"`;
Expand Down Expand Up @@ -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",
]);
});
});
});
60 changes: 60 additions & 0 deletions packages/eve/src/shared/skill-paths.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<readonly string[]> {
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;
Expand All @@ -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;
Expand Down
106 changes: 103 additions & 3 deletions packages/eve/test/read-file-tool.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import { ReadFileStateKey } from "../src/runtime/framework-tools/file-state.js";
// Helpers
// ---------------------------------------------------------------------------

function createFakeAccess(files: Record<string, string | null>): SandboxAccess {
function createFakeAccess(files: Record<string, string | null>, home?: string): SandboxAccess {
return {
async captureState() {
return { initialized: false, session: null };
Expand All @@ -40,7 +40,7 @@ function createFakeAccess(files: Record<string, string | null>): 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();
Expand All @@ -56,8 +56,9 @@ function createFakeAccess(files: Record<string, string | null>): SandboxAccess {
async function runInContext(
files: Record<string, string | null>,
fn: (sandbox: SandboxSession) => Promise<unknown>,
home?: string,
): Promise<unknown> {
const access = createFakeAccess(files);
const access = createFakeAccess(files, home);
const ctx = new ContextContainer();
ctx.set(SandboxKey, access);
ctx.set(ReadFileStateKey, { byTarget: {} });
Expand Down Expand Up @@ -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();
});
});
});