Merge pull request #9266 from continuedev/dallin/relative-path-fallback-cli

fix: don't fallback to relative path if not markdown file

Author: sestinj

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,30 @@ 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 () => {
+      it("should throw error when file reading fails for markdown path", async () => {
         mockLoadPackageFromHub.mockRejectedValue(new Error("Hub error"));
-        mockPathResolve.mockReturnValue("/resolved/owner/agent");
+        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 () => {

extensions/cli/src/services/AgentFileService.ts

@@ -58,13 +58,20 @@ export class AgentFileService
       if (parts.length === 2 && parts[0] && parts[1] && !parts.includes(".")) {
         try {
           return await loadPackageFromHub(agentPath, agentFileProcessor);
-        } catch (e) {
-          logger.info(
-            `Failed to load agent file from slug-like path ${agentPath}: ${getErrorString(e)}`,
-          );
+        } catch {
           // 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);