From d93b4321a550acf7151949d1fee1c436945e2ee0 Mon Sep 17 00:00:00 2001 From: Dallin Romney Date: Mon, 22 Dec 2025 10:47:28 -0800 Subject: [PATCH 1/3] fix: don't fallback to relative path if not markdown file --- extensions/cli/src/services/AgentFileService.ts | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/extensions/cli/src/services/AgentFileService.ts b/extensions/cli/src/services/AgentFileService.ts index d7874a4f022..0d1c1d9c22f 100644 --- a/extensions/cli/src/services/AgentFileService.ts +++ b/extensions/cli/src/services/AgentFileService.ts @@ -59,12 +59,19 @@ export class AgentFileService try { return await loadPackageFromHub(agentPath, agentFileProcessor); } catch (e) { - logger.info( - `Failed to load agent file from slug-like path ${agentPath}: ${getErrorString(e)}`, - ); // slug COULD be path, fall back to relative path } } + + const isMarkdownPath = + agentPath.endsWith(".md") || agentPath.endsWith(".markdown"); + // Only fall back to relative path for markdown files + if (!isMarkdownPath) { + throw new Error( + `Failed to load agent from ${agentPath}. Not a markdown file`, + ); + } + const resolvedPath = agentPath.startsWith("file:/") ? fileURLToPath(agentPath) : path.resolve(agentPath); From 4c0c53257baf7edb943c76155fa6b0bde79b554f Mon Sep 17 00:00:00 2001 From: Nate Date: Mon, 22 Dec 2025 15:29:23 -0500 Subject: [PATCH 2/3] fix: fix lint error and update tests for markdown-only fallback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Remove unused variable 'e' in catch block - Update tests to use .md extensions for file path loading tests - Add test for non-markdown paths throwing errors 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 --- .../cli/src/services/AgentFileService.test.ts | 67 +++++++++---------- .../cli/src/services/AgentFileService.ts | 2 +- 2 files changed, 31 insertions(+), 38 deletions(-) diff --git a/extensions/cli/src/services/AgentFileService.test.ts b/extensions/cli/src/services/AgentFileService.test.ts index 6aab1f10885..8d237a3447c 100644 --- a/extensions/cli/src/services/AgentFileService.test.ts +++ b/extensions/cli/src/services/AgentFileService.test.ts @@ -525,17 +525,17 @@ You are a helpful agent`; expect(result).toEqual(mockAgentFile); }); - it("should fallback to file path when hub loading fails for slug-like path", async () => { + it("should fallback to file path when hub loading fails for slug-like path with .md extension", async () => { mockLoadPackageFromHub.mockRejectedValue(new Error("Hub error")); mockPathResolve.mockImplementation((p: string) => `/resolved/${p}`); mockReadFileSync.mockReturnValue(mockFileContent); - const result = await agentFileService.getAgentFile("owner/agent"); + const result = await agentFileService.getAgentFile("owner/agent.md"); - expect(mockLoadPackageFromHub).toHaveBeenCalled(); - expect(mockPathResolve).toHaveBeenCalledWith("owner/agent"); + expect(mockLoadPackageFromHub).toHaveBeenCalled(); // Two-part paths try hub first + expect(mockPathResolve).toHaveBeenCalledWith("owner/agent.md"); expect(mockReadFileSync).toHaveBeenCalledWith( - "/resolved/owner/agent", + "/resolved/owner/agent.md", "utf-8", ); expect(result).toBeDefined(); @@ -641,13 +641,9 @@ You are a helpful agent`; describe("path format edge cases", () => { it("should treat single-part path as file path, not hub slug", async () => { - const singlePath = "agent"; - mockPathResolve.mockReturnValue("/resolved/agent"); + const singlePath = "agent.md"; + mockPathResolve.mockReturnValue("/resolved/agent.md"); mockReadFileSync.mockReturnValue(mockFileContent); - // Make hub loading fail so it falls back to file system - mockLoadPackageFromHub.mockRejectedValueOnce( - new Error("Not a hub slug"), - ); const result = await agentFileService.getAgentFile(singlePath); @@ -658,13 +654,9 @@ You are a helpful agent`; }); it("should treat three-part path as file path, not hub slug", async () => { - const threePath = "path/to/agent"; - mockPathResolve.mockReturnValue("/resolved/path/to/agent"); + const threePath = "path/to/agent.md"; + mockPathResolve.mockReturnValue("/resolved/path/to/agent.md"); mockReadFileSync.mockReturnValue(mockFileContent); - // Make hub loading fail so it falls back to file system - mockLoadPackageFromHub.mockRejectedValueOnce( - new Error("Not a hub slug"), - ); const result = await agentFileService.getAgentFile(threePath); @@ -675,29 +667,20 @@ You are a helpful agent`; }); it("should not treat two-part path with empty part as hub slug", async () => { - const emptyPartPath = "owner/"; - mockPathResolve.mockReturnValue("/resolved/owner/"); - mockReadFileSync.mockReturnValue(mockFileContent); - // Make hub loading fail so it falls back to file system - mockLoadPackageFromHub.mockRejectedValueOnce( - new Error("Not a hub slug"), - ); + const emptyPartPath = "owner/"; // Empty second part - const result = await agentFileService.getAgentFile(emptyPartPath); + // With the new behavior, non-markdown paths throw errors + await expect( + agentFileService.getAgentFile(emptyPartPath), + ).rejects.toThrow("Not a markdown file"); - expect(mockLoadPackageFromHub).not.toHaveBeenCalled(); - expect(mockPathResolve).toHaveBeenCalledWith(emptyPartPath); - expect(result).toBeDefined(); + expect(mockLoadPackageFromHub).not.toHaveBeenCalled(); // Empty part means no hub attempt }); it("should not treat path starting with slash as hub slug", async () => { - const slashPath = "/owner/agent"; + const slashPath = "/owner/agent.md"; mockPathResolve.mockReturnValue(slashPath); mockReadFileSync.mockReturnValue(mockFileContent); - // Make hub loading fail so it falls back to file system - mockLoadPackageFromHub.mockRejectedValueOnce( - new Error("Not a hub slug"), - ); const result = await agentFileService.getAgentFile(slashPath); @@ -741,19 +724,29 @@ You are a helpful agent`; ).rejects.toThrow("Failed to load agent from ./invalid.md"); }); - it("should throw error when hub loading fails and file fallback also fails", async () => { - mockLoadPackageFromHub.mockRejectedValue(new Error("Hub error")); - mockPathResolve.mockReturnValue("/resolved/owner/agent"); + it("should throw error when file reading fails for markdown path", async () => { + mockPathResolve.mockReturnValue("/resolved/owner/agent.md"); mockReadFileSync.mockImplementation(() => { throw new Error("File not found"); }); + await expect( + agentFileService.getAgentFile("owner/agent.md"), + ).rejects.toThrow("Failed to load agent from owner/agent.md"); + await expect( + agentFileService.getAgentFile("owner/agent.md"), + ).rejects.toThrow("File not found"); + }); + + it("should throw error for non-markdown paths without trying file fallback", async () => { + mockLoadPackageFromHub.mockRejectedValue(new Error("Hub error")); + await expect( agentFileService.getAgentFile("owner/agent"), ).rejects.toThrow("Failed to load agent from owner/agent"); await expect( agentFileService.getAgentFile("owner/agent"), - ).rejects.toThrow("File not found"); + ).rejects.toThrow("Not a markdown file"); }); it("should handle permission errors when reading files", async () => { diff --git a/extensions/cli/src/services/AgentFileService.ts b/extensions/cli/src/services/AgentFileService.ts index 0d1c1d9c22f..e03e8faf9af 100644 --- a/extensions/cli/src/services/AgentFileService.ts +++ b/extensions/cli/src/services/AgentFileService.ts @@ -58,7 +58,7 @@ export class AgentFileService if (parts.length === 2 && parts[0] && parts[1] && !parts.includes(".")) { try { return await loadPackageFromHub(agentPath, agentFileProcessor); - } catch (e) { + } catch { // slug COULD be path, fall back to relative path } } From 53a9fa4cb34d97a9f3e2af1f26a02b10bbd0a9a1 Mon Sep 17 00:00:00 2001 From: Nate Date: Mon, 22 Dec 2025 15:34:38 -0500 Subject: [PATCH 3/3] fix: add missing hub mock rejection in test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add mockLoadPackageFromHub.mockRejectedValue() to test that verifies file reading failure for markdown paths. Without this, the hub would succeed and the file reading code wouldn't be exercised. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 --- extensions/cli/src/services/AgentFileService.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/extensions/cli/src/services/AgentFileService.test.ts b/extensions/cli/src/services/AgentFileService.test.ts index 8d237a3447c..4914387e535 100644 --- a/extensions/cli/src/services/AgentFileService.test.ts +++ b/extensions/cli/src/services/AgentFileService.test.ts @@ -725,6 +725,7 @@ You are a helpful agent`; }); it("should throw error when file reading fails for markdown path", async () => { + mockLoadPackageFromHub.mockRejectedValue(new Error("Hub error")); mockPathResolve.mockReturnValue("/resolved/owner/agent.md"); mockReadFileSync.mockImplementation(() => { throw new Error("File not found");