mirror of
https://github.com/RooVetGit/Roo-Code.git
synced 2026-09-07 08:26:51 +00:00
feat: improve package scanning
- Stop scanning at package boundaries - Remove items array handling (only for external refs) - Add comprehensive test coverage
This commit is contained in:
Smartsheet-JB-Brown
parent
172f8d747c
commit
6f1befb235
2 changed files with 266 additions and 5 deletions
|
|
@ -3,7 +3,7 @@ import * as fs from "fs/promises"
|
|||
import * as vscode from "vscode"
|
||||
import * as yaml from "js-yaml"
|
||||
import { validateAnyMetadata } from "./schemas"
|
||||
import { ComponentMetadata, ComponentType, LocalizedMetadata, PackageManagerItem } from "./types"
|
||||
import { ComponentMetadata, ComponentType, LocalizedMetadata, PackageManagerItem, PackageMetadata } from "./types"
|
||||
|
||||
/**
|
||||
* Handles component discovery and metadata loading
|
||||
|
|
@ -30,12 +30,20 @@ export class MetadataScanner {
|
|||
|
||||
if (metadata?.["en"]) {
|
||||
const item = await this.createPackageManagerItem(metadata["en"], componentDir, repoUrl, sourceName)
|
||||
if (item) items.push(item)
|
||||
if (item) {
|
||||
items.push(item)
|
||||
// Skip recursion if this is a package directory
|
||||
if (this.isPackageMetadata(metadata["en"])) {
|
||||
continue
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Recursively scan subdirectories
|
||||
const subItems = await this.scanDirectory(componentDir, repoUrl, sourceName)
|
||||
items.push(...subItems)
|
||||
// Recursively scan subdirectories only if not in a package
|
||||
if (!metadata?.["en"] || !this.isPackageMetadata(metadata["en"])) {
|
||||
const subItems = await this.scanDirectory(componentDir, repoUrl, sourceName)
|
||||
items.push(...subItems)
|
||||
}
|
||||
}
|
||||
} catch (error) {
|
||||
console.error(`Error scanning directory ${rootDir}:`, error)
|
||||
|
|
@ -151,4 +159,13 @@ export class MetadataScanner {
|
|||
private isValidComponentType(type: string): type is ComponentType {
|
||||
return ["role", "mcp server", "storage", "mode", "prompt", "package"].includes(type)
|
||||
}
|
||||
|
||||
/**
|
||||
* Type guard for package metadata
|
||||
* @param metadata The metadata to check
|
||||
* @returns Whether the metadata is for a package
|
||||
*/
|
||||
private isPackageMetadata(metadata: ComponentMetadata): metadata is PackageMetadata {
|
||||
return metadata.type === "package"
|
||||
}
|
||||
}
|
||||
|
|
|
|||
244
src/services/package-manager/__tests__/PackageScanning.test.ts
Normal file
244
src/services/package-manager/__tests__/PackageScanning.test.ts
Normal file
|
|
@ -0,0 +1,244 @@
|
|||
import * as path from "path"
|
||||
import * as fs from "fs/promises"
|
||||
import { Dirent } from "fs"
|
||||
import { MetadataScanner } from "../MetadataScanner"
|
||||
import { ComponentMetadata, PackageMetadata } from "../types"
|
||||
|
||||
jest.mock("fs/promises")
|
||||
|
||||
// Create mock Dirent objects
|
||||
const createMockDirent = (name: string, isDir: boolean): Dirent => {
|
||||
return {
|
||||
name,
|
||||
isDirectory: () => isDir,
|
||||
isFile: () => !isDir,
|
||||
isBlockDevice: () => false,
|
||||
isCharacterDevice: () => false,
|
||||
isFIFO: () => false,
|
||||
isSocket: () => false,
|
||||
isSymbolicLink: () => false,
|
||||
// These are readonly in the real Dirent
|
||||
path: "",
|
||||
parentPath: "",
|
||||
} as Dirent
|
||||
}
|
||||
|
||||
describe("Package Scanning Tests", () => {
|
||||
let metadataScanner: MetadataScanner
|
||||
const mockBasePath = "/test/repo"
|
||||
const mockRepoUrl = "https://example.com/repo"
|
||||
|
||||
beforeEach(() => {
|
||||
metadataScanner = new MetadataScanner()
|
||||
jest.resetAllMocks()
|
||||
})
|
||||
|
||||
it("should not scan inside package directories", async () => {
|
||||
// Mock directory structure:
|
||||
// /test/repo/
|
||||
// package1/
|
||||
// metadata.en.yml (package)
|
||||
// item1/
|
||||
// metadata.en.yml
|
||||
// item2/
|
||||
// metadata.en.yml
|
||||
// package2/
|
||||
// metadata.en.yml (package)
|
||||
// item3/
|
||||
// metadata.en.yml
|
||||
|
||||
// Mock root directory listing
|
||||
const mockRootEntries = [createMockDirent("package1", true), createMockDirent("package2", true)]
|
||||
|
||||
;(fs.readdir as jest.Mock).mockImplementation((dir: string) => {
|
||||
if (dir === mockBasePath) {
|
||||
return mockRootEntries
|
||||
}
|
||||
if (dir === path.join(mockBasePath, "package1")) {
|
||||
return [
|
||||
createMockDirent("metadata.en.yml", false),
|
||||
createMockDirent("item1", true),
|
||||
createMockDirent("item2", true),
|
||||
]
|
||||
}
|
||||
if (dir === path.join(mockBasePath, "package2")) {
|
||||
return [createMockDirent("metadata.en.yml", false), createMockDirent("item3", true)]
|
||||
}
|
||||
return []
|
||||
})
|
||||
|
||||
// Mock metadata file reads
|
||||
;(fs.readFile as jest.Mock).mockImplementation((filePath: string) => {
|
||||
if (filePath.includes("package1/metadata.en.yml")) {
|
||||
return JSON.stringify({
|
||||
name: "Package 1",
|
||||
description: "Test Package 1",
|
||||
version: "1.0.0",
|
||||
type: "package",
|
||||
items: [
|
||||
{ type: "mode", path: "item1" },
|
||||
{ type: "prompt", path: "item2" },
|
||||
],
|
||||
})
|
||||
}
|
||||
if (filePath.includes("package2/metadata.en.yml")) {
|
||||
return JSON.stringify({
|
||||
name: "Package 2",
|
||||
description: "Test Package 2",
|
||||
version: "1.0.0",
|
||||
type: "package",
|
||||
items: [{ type: "mode", path: "item3" }],
|
||||
})
|
||||
}
|
||||
return "{}"
|
||||
})
|
||||
|
||||
// Mock file stats
|
||||
;(fs.stat as jest.Mock).mockResolvedValue({
|
||||
mtime: new Date(),
|
||||
isFile: () => true,
|
||||
})
|
||||
|
||||
const items = await metadataScanner.scanDirectory(mockBasePath, mockRepoUrl)
|
||||
|
||||
// Should only return the two packages, not their nested items
|
||||
expect(items).toHaveLength(2)
|
||||
expect(items[0].name).toBe("Package 1")
|
||||
expect(items[1].name).toBe("Package 2")
|
||||
|
||||
// Verify we didn't try to read metadata from nested items
|
||||
const readFileCalls = (fs.readFile as jest.Mock).mock.calls.map((call) => call[0])
|
||||
expect(readFileCalls).not.toContain(expect.stringContaining("item1/metadata.en.yml"))
|
||||
expect(readFileCalls).not.toContain(expect.stringContaining("item2/metadata.en.yml"))
|
||||
expect(readFileCalls).not.toContain(expect.stringContaining("item3/metadata.en.yml"))
|
||||
})
|
||||
|
||||
it("should handle nested packages correctly", async () => {
|
||||
// Mock directory structure:
|
||||
// /test/repo/
|
||||
// outer-package/
|
||||
// metadata.en.yml (package)
|
||||
// inner-package/
|
||||
// metadata.en.yml (package)
|
||||
|
||||
// Mock directory listings
|
||||
const mockRootEntries = [createMockDirent("outer-package", true)]
|
||||
;(fs.readdir as jest.Mock).mockImplementation((dir: string) => {
|
||||
if (dir === mockBasePath) {
|
||||
return mockRootEntries
|
||||
}
|
||||
if (dir === path.join(mockBasePath, "outer-package")) {
|
||||
return [createMockDirent("metadata.en.yml", false), createMockDirent("inner-package", true)]
|
||||
}
|
||||
return []
|
||||
})
|
||||
|
||||
// Mock metadata file reads
|
||||
;(fs.readFile as jest.Mock).mockImplementation((filePath: string) => {
|
||||
if (filePath.includes("outer-package/metadata.en.yml")) {
|
||||
return JSON.stringify({
|
||||
name: "Outer Package",
|
||||
description: "Test Outer Package",
|
||||
version: "1.0.0",
|
||||
type: "package",
|
||||
items: [{ type: "package", path: "inner-package" }],
|
||||
})
|
||||
}
|
||||
return "{}"
|
||||
})
|
||||
|
||||
// Mock file stats
|
||||
;(fs.stat as jest.Mock).mockResolvedValue({
|
||||
mtime: new Date(),
|
||||
isFile: () => true,
|
||||
})
|
||||
|
||||
const items = await metadataScanner.scanDirectory(mockBasePath, mockRepoUrl)
|
||||
|
||||
// Should only return the outer package
|
||||
expect(items).toHaveLength(1)
|
||||
expect(items[0].name).toBe("Outer Package")
|
||||
|
||||
// Verify we didn't try to read inner package metadata
|
||||
const readFileCalls = (fs.readFile as jest.Mock).mock.calls.map((call) => call[0])
|
||||
expect(readFileCalls).not.toContain(expect.stringContaining("inner-package/metadata.en.yml"))
|
||||
})
|
||||
|
||||
it("should handle mixed package and non-package directories", async () => {
|
||||
// Mock directory structure:
|
||||
// /test/repo/
|
||||
// package1/
|
||||
// metadata.en.yml (package)
|
||||
// mode1/
|
||||
// metadata.en.yml (mode)
|
||||
// submode/
|
||||
// metadata.en.yml (mode)
|
||||
|
||||
// Mock directory listings
|
||||
const mockRootEntries = [createMockDirent("package1", true), createMockDirent("mode1", true)]
|
||||
;(fs.readdir as jest.Mock).mockImplementation((dir: string) => {
|
||||
if (dir === mockBasePath) {
|
||||
return mockRootEntries
|
||||
}
|
||||
if (dir === path.join(mockBasePath, "package1")) {
|
||||
return [createMockDirent("metadata.en.yml", false)]
|
||||
}
|
||||
if (dir === path.join(mockBasePath, "mode1")) {
|
||||
return [createMockDirent("metadata.en.yml", false), createMockDirent("submode", true)]
|
||||
}
|
||||
if (dir === path.join(mockBasePath, "mode1/submode")) {
|
||||
return [createMockDirent("metadata.en.yml", false)]
|
||||
}
|
||||
return []
|
||||
})
|
||||
|
||||
// Mock metadata file reads
|
||||
;(fs.readFile as jest.Mock).mockImplementation((filePath: string) => {
|
||||
if (filePath.includes("package1/metadata.en.yml")) {
|
||||
return JSON.stringify({
|
||||
name: "Package 1",
|
||||
description: "Test Package",
|
||||
version: "1.0.0",
|
||||
type: "package",
|
||||
})
|
||||
}
|
||||
if (filePath.includes("mode1/metadata.en.yml")) {
|
||||
return JSON.stringify({
|
||||
name: "Mode 1",
|
||||
description: "Test Mode",
|
||||
version: "1.0.0",
|
||||
type: "mode",
|
||||
})
|
||||
}
|
||||
if (filePath.includes("submode/metadata.en.yml")) {
|
||||
return JSON.stringify({
|
||||
name: "Submode",
|
||||
description: "Test Submode",
|
||||
version: "1.0.0",
|
||||
type: "mode",
|
||||
})
|
||||
}
|
||||
return "{}"
|
||||
})
|
||||
|
||||
// Mock file stats
|
||||
;(fs.stat as jest.Mock).mockResolvedValue({
|
||||
mtime: new Date(),
|
||||
isFile: () => true,
|
||||
})
|
||||
|
||||
const items = await metadataScanner.scanDirectory(mockBasePath, mockRepoUrl)
|
||||
|
||||
// Should return package and both modes
|
||||
expect(items).toHaveLength(3)
|
||||
|
||||
// Verify items are returned in correct order
|
||||
const types = items.map((item) => item.type)
|
||||
expect(types).toContain("package")
|
||||
expect(types).toContain("mode")
|
||||
|
||||
// Verify we recursed into mode directory but not package
|
||||
const readFileCalls = (fs.readFile as jest.Mock).mock.calls.map((call) => call[0])
|
||||
expect(readFileCalls).toContainEqual(expect.stringContaining("mode1/submode/metadata.en.yml"))
|
||||
})
|
||||
})
|
||||
Loading…
Add table
Reference in a new issue