diff --git a/package-manager-template/packages/test-source-url/metadata.en.yml b/package-manager-template/packages/test-source-url/metadata.en.yml new file mode 100644 index 0000000000..4c76ee784b --- /dev/null +++ b/package-manager-template/packages/test-source-url/metadata.en.yml @@ -0,0 +1,8 @@ +name: Test Source URL +description: A test package with source URL +type: package +version: 1.0.0 +sourceUrl: https://example.com/test-package +tags: + - test + - source-url \ No newline at end of file diff --git a/src/services/package-manager/MetadataScanner.ts b/src/services/package-manager/MetadataScanner.ts index beb58b50bb..b5b16d2e9e 100644 --- a/src/services/package-manager/MetadataScanner.ts +++ b/src/services/package-manager/MetadataScanner.ts @@ -249,6 +249,7 @@ export class MetadataScanner { items: [], // Initialize empty items array for all components author: metadata.author, authorUrl: metadata.authorUrl, + sourceUrl: metadata.sourceUrl, } } diff --git a/src/services/package-manager/__tests__/MetadataScanner.test.ts b/src/services/package-manager/__tests__/MetadataScanner.test.ts index f20664dd1b..6b887db669 100644 --- a/src/services/package-manager/__tests__/MetadataScanner.test.ts +++ b/src/services/package-manager/__tests__/MetadataScanner.test.ts @@ -44,7 +44,7 @@ describe("MetadataScanner", () => { }) describe("Basic Metadata Scanning", () => { - it("should discover components with English metadata", async () => { + it("should discover components with English metadata and sourceUrl", async () => { // Mock directory structure const mockDirents = [ { @@ -86,6 +86,7 @@ name: Test Component description: A test component type: mcp server version: 1.0.0 +sourceUrl: https://example.com/component1 `), ) @@ -96,6 +97,54 @@ version: 1.0.0 expect(items[0].type).toBe("mcp server") expect(items[0].url).toBe("https://example.com/repo/tree/main/component1") expect(items[0].path).toBe("component1") + expect(items[0].sourceUrl).toBe("https://example.com/component1") + }) + it("should handle missing sourceUrl in metadata", async () => { + const mockDirents = [ + { + name: "component2", + isDirectory: () => true, + isFile: () => false, + }, + { + name: "metadata.en.yml", + isDirectory: () => false, + isFile: () => true, + }, + ] as Dirent[] + + const mockEmptyDirents = [] as Dirent[] + const mockStats = { + isDirectory: () => true, + isFile: () => true, + mtime: new Date(), + } as Stats + + const mockedFs = jest.mocked(fs) + mockedFs.stat.mockResolvedValue(mockStats) + ;(mockedFs.readdir as any).mockImplementation(async (path: any, options?: any) => { + if (path.toString().includes("/component2/")) { + return options?.withFileTypes ? mockEmptyDirents : [] + } + return options?.withFileTypes ? mockDirents : mockDirents.map((d) => d.name) + }) + mockedFs.readFile.mockResolvedValue( + Buffer.from(` +name: Test Component 2 +description: A test component without sourceUrl +type: mcp server +version: 1.0.0 +`), + ) + + const items = await metadataScanner.scanDirectory(mockBasePath, mockRepoUrl) + + expect(items).toHaveLength(1) + expect(items[0].name).toBe("Test Component 2") + expect(items[0].type).toBe("mcp server") + expect(items[0].url).toBe("https://example.com/repo/tree/main/component2") + expect(items[0].path).toBe("component2") + expect(items[0].sourceUrl).toBeUndefined() }) }) }) diff --git a/src/services/package-manager/schemas.ts b/src/services/package-manager/schemas.ts index b41b545c2d..86763cbfe3 100644 --- a/src/services/package-manager/schemas.ts +++ b/src/services/package-manager/schemas.ts @@ -11,6 +11,7 @@ export const baseMetadataSchema = z.object({ tags: z.array(z.string()).optional(), author: z.string().optional(), authorUrl: z.string().url("Author URL must be a valid URL").optional(), + sourceUrl: z.string().url("Source URL must be a valid URL").optional(), }) /** diff --git a/src/services/package-manager/types.ts b/src/services/package-manager/types.ts index bea06ebe35..feb28153f1 100644 --- a/src/services/package-manager/types.ts +++ b/src/services/package-manager/types.ts @@ -27,6 +27,7 @@ export interface BaseMetadata { tags?: string[] author?: string authorUrl?: string + sourceUrl?: string } /** diff --git a/webview-ui/src/components/package-manager/components/PackageManagerItemCard.tsx b/webview-ui/src/components/package-manager/components/PackageManagerItemCard.tsx index fa773f39ce..d07b940839 100644 --- a/webview-ui/src/components/package-manager/components/PackageManagerItemCard.tsx +++ b/webview-ui/src/components/package-manager/components/PackageManagerItemCard.tsx @@ -64,14 +64,19 @@ export const PackageManagerItemCard: React.FC = ({ } const handleOpenUrl = () => { - let urlToOpen = item.sourceUrl && isValidUrl(item.sourceUrl) ? item.sourceUrl : item.repoUrl + // If sourceUrl is present and valid, use it directly without modifications + if (item.sourceUrl && isValidUrl(item.sourceUrl)) { + return vscode.postMessage({ + type: "openExternal", + url: item.sourceUrl, + }) + } - // If we have a defaultBranch, append it to the URL + // Otherwise use repoUrl with git path information + let urlToOpen = item.repoUrl if (item.defaultBranch) { urlToOpen = `${urlToOpen}/tree/${item.defaultBranch}` - // If we also have a path, append it if (item.path) { - // Ensure path uses forward slashes and doesn't start with one const normalizedPath = item.path.replace(/\\/g, "/").replace(/^\/+/, "") urlToOpen = `${urlToOpen}/${normalizedPath}` } @@ -192,11 +197,17 @@ export const PackageManagerItemCard: React.FC = ({ )} - diff --git a/webview-ui/src/components/package-manager/components/__tests__/PackageManagerItemCard.test.tsx b/webview-ui/src/components/package-manager/components/__tests__/PackageManagerItemCard.test.tsx index 842065b1c1..af842b334b 100644 --- a/webview-ui/src/components/package-manager/components/__tests__/PackageManagerItemCard.test.tsx +++ b/webview-ui/src/components/package-manager/components/__tests__/PackageManagerItemCard.test.tsx @@ -95,18 +95,66 @@ describe("PackageManagerItemCard", () => { expect(screen.getByText(/Apr \d{1,2}, 2025/)).toBeInTheDocument() }) - it("should handle source URL click", () => { - renderWithProviders() + describe("URL handling", () => { + it("should use sourceUrl directly when present and valid", () => { + const itemWithSourceUrl = { + ...mockItem, + sourceUrl: "https://example.com/direct-link", + defaultBranch: "main", + path: "some/path", + } + renderWithProviders() - // Find the source button by its text content - const sourceButton = screen.getByRole("button", { - name: /Source/i, + const button = screen.getByRole("button", { name: /^$/ }) // Button with no text, only icon + fireEvent.click(button) + + expect(mockPostMessage).toHaveBeenCalledWith({ + type: "openExternal", + url: "https://example.com/direct-link", + }) }) - fireEvent.click(sourceButton) - expect(mockPostMessage).toHaveBeenCalledWith({ - type: "openExternal", - url: "test-url", + it("should use repoUrl with git path when sourceUrl is not present", () => { + const itemWithGitPath = { + ...mockItem, + defaultBranch: "main", + path: "some/path", + } + renderWithProviders() + + const button = screen.getByRole("button", { name: /Source/i }) + fireEvent.click(button) + + expect(mockPostMessage).toHaveBeenCalledWith({ + type: "openExternal", + url: "test-url/tree/main/some/path", + }) + }) + + it("should show only icon when sourceUrl is present and valid", () => { + const itemWithSourceUrl = { + ...mockItem, + sourceUrl: "https://example.com/direct-link", + } + renderWithProviders() + + // Find the source button by its empty aria-label + const button = screen.getByRole("button", { + name: "", // Empty aria-label when sourceUrl is present + }) + expect(button.querySelector(".codicon-link-external")).toBeInTheDocument() + expect(button.textContent).toBe("") // Verify no text content + }) + + it("should show text label when sourceUrl is not present", () => { + renderWithProviders() + + // Find the source button by its aria-label + const button = screen.getByRole("button", { + name: "Source", + }) + expect(button.querySelector(".codicon-link-external")).toBeInTheDocument() + expect(button).toHaveTextContent(/Source/i) }) })