From 14a92287d2a8bc1f7f86f67c07d330cc85a7063d Mon Sep 17 00:00:00 2001 From: "Zoo (VP)" Date: Mon, 21 Sep 2026 06:44:20 +0900 Subject: [PATCH 1/5] fix(commands): list global commands over colliding built-in commands --- .../__tests__/frontmatter-commands.spec.ts | 62 +++++++++++++++++++ src/services/command/commands.ts | 21 +++---- 2 files changed, 71 insertions(+), 12 deletions(-) diff --git a/src/services/command/__tests__/frontmatter-commands.spec.ts b/src/services/command/__tests__/frontmatter-commands.spec.ts index 3f93b55f94..44cf7b94e1 100644 --- a/src/services/command/__tests__/frontmatter-commands.spec.ts +++ b/src/services/command/__tests__/frontmatter-commands.spec.ts @@ -1,6 +1,7 @@ import fs from "fs/promises" import * as path from "path" +import { getBuiltInCommands } from "../built-in-commands" import { getCommand, getCommands } from "../commands" // Mock fs and path modules @@ -471,4 +472,65 @@ Deploy the app.` ) }) }) + + describe("getCommands source precedence", () => { + const mockGetBuiltInCommands = vi.mocked(getBuiltInCommands) + + it("should list a global command over a colliding built-in command", async () => { + const globalInitContent = `# Global Init + +Global init instructions.` + + mockGetBuiltInCommands.mockResolvedValueOnce([ + { + name: "init", + content: "Built-in init instructions.", + source: "built-in", + filePath: "built-in://init", + }, + ]) + mockFs.stat = vi.fn().mockResolvedValue({ isDirectory: () => true }) + mockFs.readdir = vi + .fn() + .mockResolvedValueOnce([{ name: "init.md", isFile: () => true }]) + .mockResolvedValueOnce([]) + mockFs.readFile = vi.fn().mockResolvedValue(globalInitContent) + + const result = await getCommands("/test/cwd") + const init = result.find((command) => command.name === "init") + + expect(init).toEqual( + expect.objectContaining({ + source: "global", + content: "# Global Init\n\nGlobal init instructions.", + }), + ) + }) + + it("should list a project command over a colliding global command", async () => { + const globalInitContent = `# Global Init + +Global init instructions.` + const projectInitContent = `# Project Init + +Project init instructions.` + + mockFs.stat = vi.fn().mockResolvedValue({ isDirectory: () => true }) + mockFs.readdir = vi + .fn() + .mockResolvedValueOnce([{ name: "init.md", isFile: () => true }]) + .mockResolvedValueOnce([{ name: "init.md", isFile: () => true }]) + mockFs.readFile = vi.fn().mockResolvedValueOnce(globalInitContent).mockResolvedValueOnce(projectInitContent) + + const result = await getCommands("/test/cwd") + const init = result.find((command) => command.name === "init") + + expect(init).toEqual( + expect.objectContaining({ + source: "project", + content: "# Project Init\n\nProject init instructions.", + }), + ) + }) + }) }) diff --git a/src/services/command/commands.ts b/src/services/command/commands.ts index 4e69558dc1..43ca00f13e 100644 --- a/src/services/command/commands.ts +++ b/src/services/command/commands.ts @@ -328,18 +328,15 @@ async function scanCommandDirectory( commandContent = content.trim() } - // Project commands override global ones - if (source === "project" || !commands.has(commandName)) { - commands.set(commandName, { - name: commandName, - content: commandContent, - source, - filePath: resolvedPath, - description, - argumentHint, - mode, - }) - } + commands.set(commandName, { + name: commandName, + content: commandContent, + source, + filePath: resolvedPath, + description, + argumentHint, + mode, + }) } catch (error) { console.warn(`Failed to read command file ${resolvedPath}:`, error) } From ac9aacede7348818dd911c908007d19d5226aa7c Mon Sep 17 00:00:00 2001 From: "Zoo (VP)" Date: Mon, 21 Sep 2026 19:38:41 +0900 Subject: [PATCH 2/5] fix(commands): resolve duplicate command names across symlinked directories deterministically Symlinked command directories are resolved concurrently, so collected file info lands in completion order and commands.set (last write wins) picked the winner for duplicate command names within a source nondeterministically. Sort the collected file info by original path before inserting so the lowest path always wins; the documented project > global > built-in priority across sources is unchanged. --- .../__tests__/symlink-commands.spec.ts | 86 +++++++++++++++++++ src/services/command/commands.ts | 7 ++ 2 files changed, 93 insertions(+) diff --git a/src/services/command/__tests__/symlink-commands.spec.ts b/src/services/command/__tests__/symlink-commands.spec.ts index 0d77967484..92f10ed37a 100644 --- a/src/services/command/__tests__/symlink-commands.spec.ts +++ b/src/services/command/__tests__/symlink-commands.spec.ts @@ -240,6 +240,92 @@ description: Symlinked command // Note: Nested symlinks (symlink -> symlink -> file) are automatically followed by fs.stat, // so they work transparently. The MAX_DEPTH protection prevents infinite loops. + it("should pick a deterministic winner for duplicate command names from symlinked directories", async () => { + const alphaContent = `# Alpha Command` + const betaContent = `# Beta Command` + + mockFs.stat = vi.fn().mockResolvedValue({ isDirectory: () => true }) + + // The commands directory contains two directory symlinks whose targets + // both define a command with the same name. + mockFs.readdir = vi.fn().mockImplementation((dirPath: string) => { + const normalizedPath = dirPath.replace(/\\/g, "/") + if (normalizedPath.includes("shared-alpha")) { + return Promise.resolve([ + { + name: "dup.md", + isFile: () => true, + isSymbolicLink: () => false, + parentPath: "/mock/shared-alpha", + }, + ]) + } + if (normalizedPath.includes("shared-beta")) { + return Promise.resolve([ + { + name: "dup.md", + isFile: () => true, + isSymbolicLink: () => false, + parentPath: "/mock/shared-beta", + }, + ]) + } + if (normalizedPath.includes("/global/")) { + return Promise.resolve([]) + } + return Promise.resolve([ + { + name: "alpha-link", + isFile: () => false, + isSymbolicLink: () => true, + parentPath: "/mock/project/.roo/commands", + }, + { + name: "beta-link", + isFile: () => false, + isSymbolicLink: () => true, + parentPath: "/mock/project/.roo/commands", + }, + ]) + }) + + mockFs.readlink = vi.fn().mockImplementation((linkPath: string) => { + const normalizedPath = linkPath.replace(/\\/g, "/") + return Promise.resolve(normalizedPath.includes("alpha-link") ? "/mock/shared-alpha" : "/mock/shared-beta") + }) + + mockFs.lstat = vi.fn().mockResolvedValue({ + isDirectory: () => true, + isFile: () => false, + isSymbolicLink: () => false, + }) + + mockFs.readFile = vi.fn().mockImplementation((filePath: string) => { + const normalizedPath = filePath.toString().replace(/\\/g, "/") + if (normalizedPath.includes("shared-alpha")) { + return Promise.resolve(alphaContent) + } + if (normalizedPath.includes("shared-beta")) { + return Promise.resolve(betaContent) + } + return Promise.reject(new Error("File not found")) + }) + + const result = await getCommands("/test/cwd") + + expect(result).toHaveLength(1) + // The candidate with the lowest path wins, regardless of the order in + // which the symlinked directories were resolved. + expect(result[0]).toEqual( + expect.objectContaining({ + name: "dup", + content: "# Alpha Command", + source: "project", + filePath: path.resolve("/mock/shared-alpha", "dup.md"), + }), + ) + }) + it("should handle cyclic symlinks gracefully (MAX_DEPTH protection)", async () => { // Create a cyclic symlink scenario // Mock lstat to return symlink for all targets (creating infinite loop) diff --git a/src/services/command/commands.ts b/src/services/command/commands.ts index 43ca00f13e..683e8d46be 100644 --- a/src/services/command/commands.ts +++ b/src/services/command/commands.ts @@ -290,6 +290,13 @@ async function scanCommandDirectory( // Wait for all files to be resolved await Promise.all(initialPromises) + // Symlinked directories are resolved concurrently above, so fileInfo order + // reflects completion order rather than a stable order. Sort it so duplicate + // command names within this source resolve deterministically: iterate from + // highest to lowest original path so commands.set (last write wins) keeps + // the lowest path. + fileInfo.sort((a, b) => (a.originalPath < b.originalPath ? 1 : a.originalPath > b.originalPath ? -1 : 0)) + // Process each collected file for (const { originalPath, resolvedPath } of fileInfo) { // Command name comes from the original path (symlink name if symlinked) From 0fdb37c8999e76781bdc9f0d37ddb28f50a1c3e8 Mon Sep 17 00:00:00 2001 From: "Zoo (VP)" Date: Mon, 21 Sep 2026 20:20:03 +0900 Subject: [PATCH 3/5] test(commands): cover duplicate entries from symlinks sharing one target --- .../__tests__/symlink-commands.spec.ts | 73 +++++++++++++++++++ 1 file changed, 73 insertions(+) diff --git a/src/services/command/__tests__/symlink-commands.spec.ts b/src/services/command/__tests__/symlink-commands.spec.ts index 92f10ed37a..a9808da441 100644 --- a/src/services/command/__tests__/symlink-commands.spec.ts +++ b/src/services/command/__tests__/symlink-commands.spec.ts @@ -326,6 +326,79 @@ description: Symlinked command ) }) + it("should handle duplicate entries when two symlinks resolve to the same target", async () => { + const sharedContent = `# Shared Command` + + mockFs.stat = vi.fn().mockResolvedValue({ isDirectory: () => true }) + + // Two directory symlinks pointing at the SAME target directory both + // contribute an entry for the same file, so the deterministic sort sees + // identical originalPaths (the comparator's equal branch). + mockFs.readdir = vi.fn().mockImplementation((dirPath: string) => { + const normalizedPath = dirPath.replace(/\\/g, "/") + if (normalizedPath.includes("shared-same")) { + return Promise.resolve([ + { + name: "dup.md", + isFile: () => true, + isDirectory: () => false, + isSymbolicLink: () => false, + parentPath: "/mock/shared-same", + }, + ]) + } + if (normalizedPath.includes("/global/")) { + return Promise.resolve([]) + } + return Promise.resolve([ + { + name: "alpha-link", + isFile: () => false, + isDirectory: () => false, + isSymbolicLink: () => true, + parentPath: "/mock/project/.roo/commands", + }, + { + name: "twin-link", + isFile: () => false, + isDirectory: () => false, + isSymbolicLink: () => true, + parentPath: "/mock/project/.roo/commands", + }, + ]) + }) + + mockFs.readlink = vi.fn().mockResolvedValue("/mock/shared-same") + + mockFs.lstat = vi.fn().mockResolvedValue({ + isDirectory: () => true, + isFile: () => false, + isSymbolicLink: () => false, + }) + + mockFs.readFile = vi.fn().mockImplementation((filePath: string) => { + const normalizedPath = filePath.toString().replace(/\\/g, "/") + if (normalizedPath.includes("shared-same")) { + return Promise.resolve(sharedContent) + } + return Promise.reject(new Error("File not found")) + }) + + const result = await getCommands("/test/cwd") + + // Both symlinks contribute the same file; the command appears once with + // the shared target's content regardless of resolution order. + expect(result).toHaveLength(1) + expect(result[0]).toEqual( + expect.objectContaining({ + name: "dup", + content: "# Shared Command", + source: "project", + filePath: path.resolve("/mock/shared-same", "dup.md"), + }), + ) + }) + it("should handle cyclic symlinks gracefully (MAX_DEPTH protection)", async () => { // Create a cyclic symlink scenario // Mock lstat to return symlink for all targets (creating infinite loop) From fbb7340a2ea42035ce646518db0348375150fc9f Mon Sep 17 00:00:00 2001 From: myk1yt Date: Tue, 6 Oct 2026 01:52:04 +0900 Subject: [PATCH 4/5] fix(commands): align command listing with execution resolution getCommands() scanned directory symlink targets and let those entries override built-in commands in the listing, while getCommand() only probes direct command files and file symlinks, so /init could list a global command yet execute the built-in. The listing now skips directory symlink targets, so both paths resolve the same commands. Impact: no order-dependent duplicate-name resolution remains, so the fileInfo sort and its two tests are removed; precedence tests gain filePath and exactly-one-row assertions plus project-over-built-in and three-way cases; real-filesystem tests with a directory symlink assert listing and execution agree. --- .../__tests__/frontmatter-commands.spec.ts | 82 ++++++++- .../__tests__/symlink-commands.spec.ts | 172 +----------------- .../symlink-directory-consistency.spec.ts | 86 +++++++++ src/services/command/commands.ts | 18 +- 4 files changed, 174 insertions(+), 184 deletions(-) create mode 100644 src/services/command/__tests__/symlink-directory-consistency.spec.ts diff --git a/src/services/command/__tests__/frontmatter-commands.spec.ts b/src/services/command/__tests__/frontmatter-commands.spec.ts index 44cf7b94e1..4d249c5ccc 100644 --- a/src/services/command/__tests__/frontmatter-commands.spec.ts +++ b/src/services/command/__tests__/frontmatter-commands.spec.ts @@ -497,12 +497,15 @@ Global init instructions.` mockFs.readFile = vi.fn().mockResolvedValue(globalInitContent) const result = await getCommands("/test/cwd") - const init = result.find((command) => command.name === "init") + const init = result.filter((command) => command.name === "init") - expect(init).toEqual( + // A duplicate init row would mean the built-in was listed twice. + expect(init).toHaveLength(1) + expect(init[0]).toEqual( expect.objectContaining({ source: "global", content: "# Global Init\n\nGlobal init instructions.", + filePath: expect.stringContaining(path.join(".roo", "commands", "init.md")), }), ) }) @@ -523,12 +526,83 @@ Project init instructions.` mockFs.readFile = vi.fn().mockResolvedValueOnce(globalInitContent).mockResolvedValueOnce(projectInitContent) const result = await getCommands("/test/cwd") - const init = result.find((command) => command.name === "init") + const init = result.filter((command) => command.name === "init") - expect(init).toEqual( + expect(init).toHaveLength(1) + expect(init[0]).toEqual( expect.objectContaining({ source: "project", content: "# Project Init\n\nProject init instructions.", + filePath: path.resolve(path.join("/test/cwd", ".roo", "commands", "init.md")), + }), + ) + }) + + it("should list a project command over a colliding built-in command when no global command exists", async () => { + const projectInitContent = `# Project Init + +Project init instructions.` + + mockGetBuiltInCommands.mockResolvedValueOnce([ + { + name: "init", + content: "Built-in init instructions.", + source: "built-in", + filePath: "built-in://init", + }, + ]) + mockFs.stat = vi.fn().mockResolvedValue({ isDirectory: () => true }) + mockFs.readdir = vi + .fn() + .mockResolvedValueOnce([]) // global commands directory is empty + .mockResolvedValueOnce([{ name: "init.md", isFile: () => true }]) + mockFs.readFile = vi.fn().mockResolvedValue(projectInitContent) + + const result = await getCommands("/test/cwd") + const init = result.filter((command) => command.name === "init") + + expect(init).toHaveLength(1) + expect(init[0]).toEqual( + expect.objectContaining({ + source: "project", + content: "# Project Init\n\nProject init instructions.", + filePath: path.resolve(path.join("/test/cwd", ".roo", "commands", "init.md")), + }), + ) + }) + + it("should list exactly one command for a three-way built-in/global/project collision", async () => { + const globalInitContent = `# Global Init + +Global init instructions.` + const projectInitContent = `# Project Init + +Project init instructions.` + + mockGetBuiltInCommands.mockResolvedValueOnce([ + { + name: "init", + content: "Built-in init instructions.", + source: "built-in", + filePath: "built-in://init", + }, + ]) + mockFs.stat = vi.fn().mockResolvedValue({ isDirectory: () => true }) + mockFs.readdir = vi + .fn() + .mockResolvedValueOnce([{ name: "init.md", isFile: () => true }]) + .mockResolvedValueOnce([{ name: "init.md", isFile: () => true }]) + mockFs.readFile = vi.fn().mockResolvedValueOnce(globalInitContent).mockResolvedValueOnce(projectInitContent) + + const result = await getCommands("/test/cwd") + const init = result.filter((command) => command.name === "init") + + expect(init).toHaveLength(1) + expect(init[0]).toEqual( + expect.objectContaining({ + source: "project", + content: "# Project Init\n\nProject init instructions.", + filePath: path.resolve(path.join("/test/cwd", ".roo", "commands", "init.md")), }), ) }) diff --git a/src/services/command/__tests__/symlink-commands.spec.ts b/src/services/command/__tests__/symlink-commands.spec.ts index a9808da441..6f2db7d696 100644 --- a/src/services/command/__tests__/symlink-commands.spec.ts +++ b/src/services/command/__tests__/symlink-commands.spec.ts @@ -1,5 +1,4 @@ import fs from "fs/promises" -import * as path from "path" import { getCommand, getCommands } from "../commands" @@ -156,7 +155,7 @@ description: Symlinked command expect(symlinkCmd?.content).toContain("Symlinked Command") }) - it.skipIf(process.platform === "win32")("should discover commands from symlinked directories", async () => { + it.skipIf(process.platform === "win32")("should not discover commands from symlinked directories", async () => { const nestedContent = `# Nested Command from Symlinked Dir` // Mock lstat for symlink target type checking (lstat doesn't follow symlinks) @@ -229,176 +228,19 @@ description: Symlinked command return Promise.reject(new Error("File not found")) }) + // The listing must not surface commands that are only reachable through a + // directory symlink: getCommand() only probes direct command files and + // file symlinks, so getCommands() applies the same limits. const result = await getCommands("/test/cwd") + expect(result.find((c) => c.name === "nested")).toBeUndefined() - // Find a command that was discovered from the symlinked directory - const nestedCmd = result.find((c) => c.name === "nested") - expect(nestedCmd).toBeDefined() - expect(nestedCmd?.content).toContain("Nested Command from Symlinked Dir") + // Execution agrees: the command cannot be resolved either. + expect(await getCommand("/test/cwd", "nested")).toBeUndefined() }) // Note: Nested symlinks (symlink -> symlink -> file) are automatically followed by fs.stat, // so they work transparently. The MAX_DEPTH protection prevents infinite loops. - it("should pick a deterministic winner for duplicate command names from symlinked directories", async () => { - const alphaContent = `# Alpha Command` - const betaContent = `# Beta Command` - - mockFs.stat = vi.fn().mockResolvedValue({ isDirectory: () => true }) - - // The commands directory contains two directory symlinks whose targets - // both define a command with the same name. - mockFs.readdir = vi.fn().mockImplementation((dirPath: string) => { - const normalizedPath = dirPath.replace(/\\/g, "/") - if (normalizedPath.includes("shared-alpha")) { - return Promise.resolve([ - { - name: "dup.md", - isFile: () => true, - isSymbolicLink: () => false, - parentPath: "/mock/shared-alpha", - }, - ]) - } - if (normalizedPath.includes("shared-beta")) { - return Promise.resolve([ - { - name: "dup.md", - isFile: () => true, - isSymbolicLink: () => false, - parentPath: "/mock/shared-beta", - }, - ]) - } - if (normalizedPath.includes("/global/")) { - return Promise.resolve([]) - } - return Promise.resolve([ - { - name: "alpha-link", - isFile: () => false, - isSymbolicLink: () => true, - parentPath: "/mock/project/.roo/commands", - }, - { - name: "beta-link", - isFile: () => false, - isSymbolicLink: () => true, - parentPath: "/mock/project/.roo/commands", - }, - ]) - }) - - mockFs.readlink = vi.fn().mockImplementation((linkPath: string) => { - const normalizedPath = linkPath.replace(/\\/g, "/") - return Promise.resolve(normalizedPath.includes("alpha-link") ? "/mock/shared-alpha" : "/mock/shared-beta") - }) - - mockFs.lstat = vi.fn().mockResolvedValue({ - isDirectory: () => true, - isFile: () => false, - isSymbolicLink: () => false, - }) - - mockFs.readFile = vi.fn().mockImplementation((filePath: string) => { - const normalizedPath = filePath.toString().replace(/\\/g, "/") - if (normalizedPath.includes("shared-alpha")) { - return Promise.resolve(alphaContent) - } - if (normalizedPath.includes("shared-beta")) { - return Promise.resolve(betaContent) - } - return Promise.reject(new Error("File not found")) - }) - - const result = await getCommands("/test/cwd") - - expect(result).toHaveLength(1) - // The candidate with the lowest path wins, regardless of the order in - // which the symlinked directories were resolved. - expect(result[0]).toEqual( - expect.objectContaining({ - name: "dup", - content: "# Alpha Command", - source: "project", - filePath: path.resolve("/mock/shared-alpha", "dup.md"), - }), - ) - }) - - it("should handle duplicate entries when two symlinks resolve to the same target", async () => { - const sharedContent = `# Shared Command` - - mockFs.stat = vi.fn().mockResolvedValue({ isDirectory: () => true }) - - // Two directory symlinks pointing at the SAME target directory both - // contribute an entry for the same file, so the deterministic sort sees - // identical originalPaths (the comparator's equal branch). - mockFs.readdir = vi.fn().mockImplementation((dirPath: string) => { - const normalizedPath = dirPath.replace(/\\/g, "/") - if (normalizedPath.includes("shared-same")) { - return Promise.resolve([ - { - name: "dup.md", - isFile: () => true, - isDirectory: () => false, - isSymbolicLink: () => false, - parentPath: "/mock/shared-same", - }, - ]) - } - if (normalizedPath.includes("/global/")) { - return Promise.resolve([]) - } - return Promise.resolve([ - { - name: "alpha-link", - isFile: () => false, - isDirectory: () => false, - isSymbolicLink: () => true, - parentPath: "/mock/project/.roo/commands", - }, - { - name: "twin-link", - isFile: () => false, - isDirectory: () => false, - isSymbolicLink: () => true, - parentPath: "/mock/project/.roo/commands", - }, - ]) - }) - - mockFs.readlink = vi.fn().mockResolvedValue("/mock/shared-same") - - mockFs.lstat = vi.fn().mockResolvedValue({ - isDirectory: () => true, - isFile: () => false, - isSymbolicLink: () => false, - }) - - mockFs.readFile = vi.fn().mockImplementation((filePath: string) => { - const normalizedPath = filePath.toString().replace(/\\/g, "/") - if (normalizedPath.includes("shared-same")) { - return Promise.resolve(sharedContent) - } - return Promise.reject(new Error("File not found")) - }) - - const result = await getCommands("/test/cwd") - - // Both symlinks contribute the same file; the command appears once with - // the shared target's content regardless of resolution order. - expect(result).toHaveLength(1) - expect(result[0]).toEqual( - expect.objectContaining({ - name: "dup", - content: "# Shared Command", - source: "project", - filePath: path.resolve("/mock/shared-same", "dup.md"), - }), - ) - }) - it("should handle cyclic symlinks gracefully (MAX_DEPTH protection)", async () => { // Create a cyclic symlink scenario // Mock lstat to return symlink for all targets (creating infinite loop) diff --git a/src/services/command/__tests__/symlink-directory-consistency.spec.ts b/src/services/command/__tests__/symlink-directory-consistency.spec.ts new file mode 100644 index 0000000000..2e4aa00ad1 --- /dev/null +++ b/src/services/command/__tests__/symlink-directory-consistency.spec.ts @@ -0,0 +1,86 @@ +import fs from "fs/promises" +import * as path from "path" +import { tmpdir } from "node:os" + +const mockHome = vi.hoisted(() => ({ path: "" })) + +vi.mock("os", async (importOriginal) => { + const actual = await importOriginal() + return { + ...actual, + homedir: () => mockHome.path, + } +}) + +import { getCommand, getCommands } from "../commands" + +/** + * Real-filesystem tests for listing/execution consistency. getCommands() (the + * listing) must surface exactly what getCommand() (execution) can resolve: + * direct .md files and file symlinks in the commands directory. Directory + * symlinks are not followed by either path. + */ +describe("command listing and execution consistency for symlinks", () => { + let tempDir: string + let cwd: string + let globalCommandsDir: string + + const dirLinkType = process.platform === "win32" ? "junction" : "dir" + + beforeEach(async () => { + tempDir = await fs.mkdtemp(path.join(tmpdir(), "zoo-command-symlink-")) + mockHome.path = path.join(tempDir, "home") + cwd = path.join(tempDir, "workspace") + globalCommandsDir = path.join(mockHome.path, ".roo", "commands") + await fs.mkdir(globalCommandsDir, { recursive: true }) + await fs.mkdir(cwd, { recursive: true }) + }) + + afterEach(async () => { + await fs.rm(tempDir, { recursive: true, force: true }) + }) + + it("selects the built-in command in both listing and execution when a global directory symlink collides with it", async () => { + const sharedDir = path.join(tempDir, "shared") + await fs.mkdir(sharedDir, { recursive: true }) + await fs.writeFile(path.join(sharedDir, "init.md"), "# Global Init via directory symlink") + await fs.symlink(sharedDir, path.join(globalCommandsDir, "shared-link"), dirLinkType) + + const listed = (await getCommands(cwd)).filter((command) => command.name === "init") + expect(listed).toHaveLength(1) + expect(listed[0].source).toBe("built-in") + + const executed = await getCommand(cwd, "init") + expect(executed?.source).toBe("built-in") + + // Listing and execution must select the same command. + expect(listed[0].content).toBe(executed?.content) + expect(listed[0].filePath).toBe(executed?.filePath) + }) + + it("does not list or execute a command that is only reachable through a directory symlink", async () => { + const sharedDir = path.join(tempDir, "shared") + await fs.mkdir(sharedDir, { recursive: true }) + await fs.writeFile(path.join(sharedDir, "nested.md"), "# Nested Command") + await fs.symlink(sharedDir, path.join(globalCommandsDir, "shared-link"), dirLinkType) + + const listed = await getCommands(cwd) + expect(listed.find((command) => command.name === "nested")).toBeUndefined() + + expect(await getCommand(cwd, "nested")).toBeUndefined() + }) + + it.skipIf(process.platform === "win32")("lists and executes file symlinks at the commands directory root", async () => { + const sharedDir = path.join(mockHome.path, ".roo", "shared") + await fs.mkdir(sharedDir, { recursive: true }) + await fs.writeFile(path.join(sharedDir, "aliased-target.md"), "# Aliased Command") + await fs.symlink(path.join(sharedDir, "aliased-target.md"), path.join(globalCommandsDir, "aliased.md"), "file") + + const listed = (await getCommands(cwd)).filter((command) => command.name === "aliased") + expect(listed).toHaveLength(1) + expect(listed[0].content).toContain("Aliased Command") + + const executed = await getCommand(cwd, "aliased") + expect(executed?.content).toContain("Aliased Command") + }) +}) diff --git a/src/services/command/commands.ts b/src/services/command/commands.ts index 683e8d46be..8bf569f16a 100644 --- a/src/services/command/commands.ts +++ b/src/services/command/commands.ts @@ -52,18 +52,13 @@ async function resolveCommandSymLink(symlinkPath: string, fileInfo: CommandFileI // For symlinks to files, store the symlink path as original and target as resolved fileInfo.push({ originalPath: symlinkPath, resolvedPath: resolvedTarget }) } - } else if (stats.isDirectory()) { - // Read the target directory and process its entries - const entries = await fs.readdir(resolvedTarget, { withFileTypes: true }) - const directoryPromises: Promise[] = [] - for (const entry of entries) { - directoryPromises.push(resolveCommandDirectoryEntry(entry, resolvedTarget, fileInfo, depth + 1)) - } - await Promise.all(directoryPromises) } else if (stats.isSymbolicLink()) { // Handle nested symlinks await resolveCommandSymLink(resolvedTarget, fileInfo, depth + 1) } + // Directory symlink targets are intentionally not descended into, so the + // listing surfaces exactly what getCommand() can execute: direct command + // files and file symlinks at the commands directory root. } catch { // Skip invalid symlinks } @@ -290,13 +285,6 @@ async function scanCommandDirectory( // Wait for all files to be resolved await Promise.all(initialPromises) - // Symlinked directories are resolved concurrently above, so fileInfo order - // reflects completion order rather than a stable order. Sort it so duplicate - // command names within this source resolve deterministically: iterate from - // highest to lowest original path so commands.set (last write wins) keeps - // the lowest path. - fileInfo.sort((a, b) => (a.originalPath < b.originalPath ? 1 : a.originalPath > b.originalPath ? -1 : 0)) - // Process each collected file for (const { originalPath, resolvedPath } of fileInfo) { // Command name comes from the original path (symlink name if symlinked) From 9f764fc42707fab6333fae52eeaa449741cad4b7 Mon Sep 17 00:00:00 2001 From: myk1yt Date: Tue, 6 Oct 2026 02:38:50 +0900 Subject: [PATCH 5/5] test(commands): make directory-symlink mock consistent for execution probe The readFile mock answered for any nested.md path, so getCommand() found a direct project file that the mocked readdir never listed and the execution-parity assertion failed on ubuntu. nested.md now only exists inside the mocked symlink target, matching real fs behavior. --- src/services/command/__tests__/symlink-commands.spec.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/services/command/__tests__/symlink-commands.spec.ts b/src/services/command/__tests__/symlink-commands.spec.ts index 6f2db7d696..e0d0762a80 100644 --- a/src/services/command/__tests__/symlink-commands.spec.ts +++ b/src/services/command/__tests__/symlink-commands.spec.ts @@ -219,10 +219,11 @@ description: Symlinked command // Mock readlink for symlink to directory mockFs.readlink = vi.fn().mockResolvedValue("/mock/shared-commands") - // Mock readFile for content + // Mock readFile for content: nested.md only exists inside the symlink + // target, never as a direct file in a commands directory. mockFs.readFile = vi.fn().mockImplementation((filePath: string) => { const normalizedPath = filePath.toString().replace(/\\/g, "/") - if (normalizedPath.includes("nested.md")) { + if (normalizedPath.includes("shared-commands") && normalizedPath.includes("nested.md")) { return Promise.resolve(nestedContent) } return Promise.reject(new Error("File not found"))