From eadc3e2c9f8c6513123d6ae05e270e77b3b41705 Mon Sep 17 00:00:00 2001 From: Yuhan Lei Date: Wed, 3 Jun 2026 19:26:26 +0800 Subject: [PATCH] fix(config): anchor agent/command name matching at the relative path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit configEntryNameFromPath matched a prefix (agent/, agents/, command/, commands/, mode/, modes/) anywhere in the absolute file path. A user or parent directory whose name coincidentally contained one of those segments (e.g. a home dir /Users/agent/) won the substring match before the real entry directory, leaking the intervening path into the key — an agent at /Users/agent/proj/agent/build.md keyed as "proj/agent/build" instead of "build". Anchor the match at the start of the path and have the call sites pass the path relative to the directory they scanned (path.relative(dir, item)), so the relative path is always rooted at the prefix. Glob already scans {agent,agents}/**, {command,commands}/**, and {mode,modes}/* with cwd=dir, so the .opencode/ prefix variants are subsumed and the prefix list simplifies to the bare directory names. Case-insensitive matching and nested-subdirectory keys are preserved. Add test/config/entry-name.test.ts covering prefix stripping, nested keys, backslash normalization, case-insensitive matching, basename fallback, and the #28359 regression (a misleading parent /agent/ segment is no longer stripped). The existing "tolerate unnormalized roots" assertion still holds. Reimplemented for PawWork from upstream anomalyco/opencode e94d46af86 (PR #28359, regression #25713, thanks Kit Langton); the fork has no common ancestor so this is an adapted port that keeps PawWork's case-insensitive normalization, not a cherry-pick. --- packages/opencode/src/config/agent.ts | 6 +- packages/opencode/src/config/command.ts | 4 +- packages/opencode/src/config/entry-name.ts | 29 ++++---- .../opencode/test/config/entry-name.test.ts | 66 +++++++++++++++++++ 4 files changed, 89 insertions(+), 16 deletions(-) create mode 100644 packages/opencode/test/config/entry-name.test.ts diff --git a/packages/opencode/src/config/agent.ts b/packages/opencode/src/config/agent.ts index c53d6c13e..3d7f9305c 100644 --- a/packages/opencode/src/config/agent.ts +++ b/packages/opencode/src/config/agent.ts @@ -1,5 +1,6 @@ export * as ConfigAgent from "./agent" +import path from "path" import { Schema } from "effect" import z from "zod" import { Bus } from "@/bus" @@ -142,8 +143,7 @@ export async function load(dir: string) { }) if (!md) continue - const patterns = ["/.opencode/agent/", "/.opencode/agents/", "/agent/", "/agents/"] - const name = configEntryNameFromPath(item, patterns) + const name = configEntryNameFromPath(path.relative(dir, item), ["agent/", "agents/"]) const config = { ...md.data, @@ -182,7 +182,7 @@ export async function loadMode(dir: string) { const config = { ...md.data, - name: configEntryNameFromPath(item, []), + name: configEntryNameFromPath(path.relative(dir, item), ["mode/", "modes/"]), prompt: md.content.trim(), } const parsed = Info.safeParse(config) diff --git a/packages/opencode/src/config/command.ts b/packages/opencode/src/config/command.ts index 89f732bb9..aef5306dd 100644 --- a/packages/opencode/src/config/command.ts +++ b/packages/opencode/src/config/command.ts @@ -1,5 +1,6 @@ export * as ConfigCommand from "./command" +import path from "path" import { Log } from "../util" import { Schema } from "effect" import { NamedError } from "@opencode-ai/util/error" @@ -56,8 +57,7 @@ export async function load(dir: string) { }) if (!md) continue - const patterns = ["/.opencode/command/", "/.opencode/commands/", "/command/", "/commands/"] - const name = configEntryNameFromPath(item, patterns) + const name = configEntryNameFromPath(path.relative(dir, item), ["command/", "commands/"]) const config = { ...md.data, diff --git a/packages/opencode/src/config/entry-name.ts b/packages/opencode/src/config/entry-name.ts index e9ba74951..fa2320171 100644 --- a/packages/opencode/src/config/entry-name.ts +++ b/packages/opencode/src/config/entry-name.ts @@ -1,23 +1,30 @@ import path from "path" -function sliceAfterMatch(filePath: string, searchRoots: string[]) { - const normalizedPath = filePath.replaceAll("\\", "/") +// Strips a known prefix anchored at the START of an already-relative path. +// Callers pass the path relative to the directory they scanned (e.g. +// `path.relative(dir, item)`), so the prefix match is anchored. Matching a +// prefix anywhere in an absolute path used to mis-key entries whose parent or +// home segments coincidentally contained a prefix name — e.g. a user under +// `/Users/agent/` leaked the intervening path into the agent key (see #28359 / +// upstream #25713). Matching stays case-insensitive and roots are normalized to +// preserve PawWork's prior behavior. +function stripPrefix(relativePath: string, prefixes: string[]) { + const normalizedPath = relativePath.replaceAll("\\", "/") const comparablePath = normalizedPath.toLowerCase() - const normalizedRoots = searchRoots - .map((root) => root.replaceAll("\\", "/").replace(/\/+$/, "")) + const normalizedPrefixes = prefixes + .map((prefix) => prefix.replaceAll("\\", "/").replace(/\/+$/, "")) .filter(Boolean) .sort((a, b) => b.length - a.length) - for (const searchRoot of normalizedRoots) { - const needle = `${searchRoot}/` - const index = comparablePath.indexOf(needle.toLowerCase()) - if (index === -1) continue - return normalizedPath.slice(index + needle.length).replace(/^\/+/, "") + for (const prefix of normalizedPrefixes) { + const needle = `${prefix}/` + if (!comparablePath.startsWith(needle.toLowerCase())) continue + return normalizedPath.slice(needle.length).replace(/^\/+/, "") } } -export function configEntryNameFromPath(filePath: string, searchRoots: string[]) { - const candidate = sliceAfterMatch(filePath, searchRoots) ?? path.basename(filePath) +export function configEntryNameFromPath(relativePath: string, prefixes: string[]) { + const candidate = stripPrefix(relativePath, prefixes) ?? path.basename(relativePath) const ext = path.extname(candidate) return ext.length ? candidate.slice(0, -ext.length) : candidate } diff --git a/packages/opencode/test/config/entry-name.test.ts b/packages/opencode/test/config/entry-name.test.ts new file mode 100644 index 000000000..9254a33ce --- /dev/null +++ b/packages/opencode/test/config/entry-name.test.ts @@ -0,0 +1,66 @@ +import { describe, expect, test } from "bun:test" +import { posix } from "path" +import { configEntryNameFromPath } from "@/config/entry-name" + +// The prefixes shipped by config/agent.ts after the relative-path refactor. +const AGENT_PREFIXES = ["agent/", "agents/"] + +describe("configEntryNameFromPath", () => { + test("strips an `agents/` prefix and returns the bare name", () => { + expect(configEntryNameFromPath("agents/build.md", AGENT_PREFIXES)).toBe("build") + }) + + test("strips an `agent/` (singular) prefix", () => { + expect(configEntryNameFromPath("agent/build.md", AGENT_PREFIXES)).toBe("build") + }) + + test("preserves nested subdirectories in the key", () => { + expect(configEntryNameFromPath("agents/team/build.md", AGENT_PREFIXES)).toBe("team/build") + }) + + test("normalizes Windows-style backslashes", () => { + expect(configEntryNameFromPath("agents\\team\\build.md", AGENT_PREFIXES)).toBe("team/build") + }) + + // PawWork-specific: prefix matching is case-insensitive, but the returned key + // preserves the original casing of the entry name. + test("matches the prefix case-insensitively and preserves entry casing", () => { + expect(configEntryNameFromPath("Agents/Build.md", AGENT_PREFIXES)).toBe("Build") + expect(configEntryNameFromPath("AGENT/Build.md", AGENT_PREFIXES)).toBe("Build") + expect(configEntryNameFromPath("agents/Team/Build.md", ["AGENTS/"])).toBe("Team/Build") + }) + + test("falls back to basename when no prefix matches", () => { + expect(configEntryNameFromPath("orphaned.md", AGENT_PREFIXES)).toBe("orphaned") + expect(configEntryNameFromPath("anywhere/orphaned.md", [])).toBe("orphaned") + }) + + // Regression for #28359 (upstream #25713): a parent/home segment containing + // `agent` or `agents` used to win the substring match before the real + // `agents/` directory, leaking the intervening path into the key (e.g. + // `proj/agent/build`). Anchoring at the caller via `path.relative(dir, item)` + // makes this impossible — the relative path is always rooted at the prefix. + test("regression #28359: caller passes relative path; parent /agent/ segment is irrelevant", () => { + const dir = "/Users/agent/proj" + const item = "/Users/agent/proj/agent/build.md" + const relative = posix.relative(dir, item) + expect(relative).toBe("agent/build.md") + expect(configEntryNameFromPath(relative, AGENT_PREFIXES)).toBe("build") + }) + + // Anchoring is what makes the relative-path contract safe: a prefix that + // appears only in a deeper/parent segment of an absolute path is NOT stripped + // (the helper falls back to the basename). Before #28359 the unanchored + // substring match returned "proj/agent/build" here. + test("regression #28359: does not strip from a misleading parent segment", () => { + expect(configEntryNameFromPath("/Users/agent/proj/agent/build.md", AGENT_PREFIXES)).toBe("build") + }) + + test("regression #28359: parent /agents/ segment is irrelevant for nested entries", () => { + const dir = "/srv/agents/team/proj" + const item = "/srv/agents/team/proj/agents/team/build.md" + const relative = posix.relative(dir, item) + expect(relative).toBe("agents/team/build.md") + expect(configEntryNameFromPath(relative, AGENT_PREFIXES)).toBe("team/build") + }) +})