refactor(marketplace): typed origin checks + helper; scoped deletion safety and i18n; atomic deleteCustomModeForSource; update tests and en locale

This commit is contained in:
matt-rudolph 2025-09-19 14:28:11 -06:00
parent a76bc0b5ae
commit c6b644a1b6
5 changed files with 92 additions and 78 deletions

View file

@ -563,35 +563,35 @@ export class CustomModesManager {
fromMarketplace = false,
): Promise<void> {
try {
const settingsPath = await this.getCustomModesFilePath()
const roomodesPath = await this.getWorkspaceRoomodes()
let targetPath: string | undefined
let modeToDelete: ModeConfig | undefined
if (source === "project") {
if (!roomodesPath) {
throw new Error(t("common:customModes.errors.noWorkspaceForProject"))
}
targetPath = roomodesPath
const roomodesModes = await this.loadModesFromFile(roomodesPath)
modeToDelete = roomodesModes.find((m) => m.slug === slug)
} else {
targetPath = settingsPath
const settingsModes = await this.loadModesFromFile(settingsPath)
modeToDelete = settingsModes.find((m) => m.slug === slug)
}
if (!modeToDelete) {
throw new Error(t("common:customModes.errors.modeNotFound"))
}
await this.queueWrite(async () => {
const settingsPath = await this.getCustomModesFilePath()
const roomodesPath = await this.getWorkspaceRoomodes()
let targetPath: string
let modeToDelete: ModeConfig | undefined
if (source === "project") {
if (!roomodesPath) {
throw new Error(t("common:customModes.errors.noWorkspaceForProject"))
}
targetPath = roomodesPath
const roomodesModes = await this.loadModesFromFile(roomodesPath)
modeToDelete = roomodesModes.find((m) => m.slug === slug)
} else {
targetPath = settingsPath
const settingsModes = await this.loadModesFromFile(settingsPath)
modeToDelete = settingsModes.find((m) => m.slug === slug)
}
if (!modeToDelete) {
throw new Error(t("common:customModes.errors.modeNotFound"))
}
// Delete only from the selected source file
await this.updateModesInFile(targetPath!, (modes) => modes.filter((m) => m.slug !== slug))
await this.updateModesInFile(targetPath, (modes) => modes.filter((m) => m.slug !== slug))
// Delete associated rules folder using the located mode (preserves correct scope)
await this.deleteRulesFolder(slug, modeToDelete!, fromMarketplace)
await this.deleteRulesFolder(slug, modeToDelete, fromMarketplace)
// Refresh state and clear caches
this.clearCache()

View file

@ -65,5 +65,8 @@
"removing": "Removing item: \"{{itemName}}\"",
"removeSuccess": "\"{{itemName}}\" removed successfully",
"removeError": "Failed to remove \"{{itemName}}\": {{errorMessage}}"
},
"errors": {
"scopedDeletionNotSupported": "Scoped deletion is not supported in this version. Please update Roo Code to the latest version and try again."
}
}

View file

@ -4,7 +4,8 @@ import * as path from "path"
import * as vscode from "vscode"
import * as yaml from "yaml"
import type { OrganizationSettings, MarketplaceItem, MarketplaceItemType, McpMarketplaceItem } from "@roo-code/types"
import type { OrganizationSettings, MarketplaceItem, MarketplaceItemType, McpMarketplaceItem, ModeConfig } from "@roo-code/types"
import { customModesSettingsSchema } from "@roo-code/types"
import { TelemetryService } from "@roo-code/telemetry"
import { CloudService } from "@roo-code/cloud"
@ -241,6 +242,38 @@ export class MarketplaceManager {
return metadata
}
// Helper: identify marketplace-installed modes
private isMarketplaceInstalledMode(
mode: ModeConfig,
): mode is ModeConfig & { installedFromMarketplace: true; marketplaceItemId: string } {
return (
mode.installedFromMarketplace === true &&
typeof mode.marketplaceItemId === "string" &&
mode.marketplaceItemId.length > 0
)
}
// Helper: parse YAML and collect installed mode metadata with proper typing
private collectInstalledModesFromYaml(
content: string,
out: Record<string, { type: string }>,
): void {
try {
const parsed = yaml.parse(content)
const result = customModesSettingsSchema.safeParse(parsed)
if (!result.success) {
return
}
for (const mode of result.data.customModes) {
if (this.isMarketplaceInstalledMode(mode)) {
out[mode.marketplaceItemId!] = { type: "mode" }
}
}
} catch {
// Ignore parse errors here; caller handles file existence/errors
}
}
/**
* Check for project-level installed items
*/
@ -255,19 +288,7 @@ export class MarketplaceManager {
const projectModesPath = path.join(workspaceFolder.uri.fsPath, ".roomodes")
try {
const content = await fs.readFile(projectModesPath, "utf-8")
const data = yaml.parse(content)
if (data?.customModes && Array.isArray(data.customModes)) {
for (const mode of data.customModes) {
// Only consider marketplace-installed modes and key by marketplaceItemId
const fromMarketplace = (mode as any)?.installedFromMarketplace === true
const marketplaceItemId = (mode as any)?.marketplaceItemId
if (fromMarketplace && typeof marketplaceItemId === "string" && marketplaceItemId.length > 0) {
metadata[marketplaceItemId] = {
type: "mode",
}
}
}
}
this.collectInstalledModesFromYaml(content, metadata)
} catch (error) {
// File doesn't exist or can't be read, skip
}
@ -303,19 +324,7 @@ export class MarketplaceManager {
const globalModesPath = path.join(globalSettingsPath, GlobalFileNames.customModes)
try {
const content = await fs.readFile(globalModesPath, "utf-8")
const data = yaml.parse(content)
if (data?.customModes && Array.isArray(data.customModes)) {
for (const mode of data.customModes) {
// Only consider marketplace-installed modes and key by marketplaceItemId
const fromMarketplace = (mode as any)?.installedFromMarketplace === true
const marketplaceItemId = (mode as any)?.marketplaceItemId
if (fromMarketplace && typeof marketplaceItemId === "string" && marketplaceItemId.length > 0) {
metadata[marketplaceItemId] = {
type: "mode",
}
}
}
}
this.collectInstalledModesFromYaml(content, metadata)
} catch (error) {
// File doesn't exist or can't be read, skip
}

View file

@ -2,10 +2,11 @@ import * as vscode from "vscode"
import * as path from "path"
import * as fs from "fs/promises"
import * as yaml from "yaml"
import type { MarketplaceItem, MarketplaceItemType, InstallMarketplaceItemOptions, McpParameter } from "@roo-code/types"
import type { MarketplaceItem, MarketplaceItemType, InstallMarketplaceItemOptions, McpParameter, ModeConfig } from "@roo-code/types"
import { GlobalFileNames } from "../../shared/globalFileNames"
import { ensureSettingsDirectoryExists } from "../../utils/globalContext"
import type { CustomModesManager } from "../../core/config/CustomModesManager"
import { t } from "../../i18n"
export interface InstallOptions {
target: "project" | "global"
@ -28,7 +29,7 @@ export class SimpleInstaller {
case "mcp":
return await this.installMcp(item, target, options)
default:
throw new Error(`Unsupported item type: ${(item as any).type}`)
throw new Error("Unsupported item type")
}
}
@ -51,8 +52,8 @@ export class SimpleInstaller {
const parsedMode = yaml.parse(item.content)
// Annotate marketplace origin to disambiguate from user-created modes
if (parsedMode && typeof parsedMode === "object") {
;(parsedMode as any).installedFromMarketplace = true
;(parsedMode as any).marketplaceItemId = item.id
;(parsedMode as ModeConfig).installedFromMarketplace = true
;(parsedMode as ModeConfig).marketplaceItemId = item.id
}
const importData = {
customModes: [parsedMode],
@ -97,8 +98,8 @@ export class SimpleInstaller {
const modeData = yaml.parse(item.content)
// Annotate marketplace origin fields for fallback path as well
if (modeData && typeof modeData === "object") {
;(modeData as any).installedFromMarketplace = true
;(modeData as any).marketplaceItemId = item.id
;(modeData as ModeConfig).installedFromMarketplace = true
;(modeData as ModeConfig).marketplaceItemId = item.id
}
// Read existing file or create new structure
@ -301,7 +302,7 @@ export class SimpleInstaller {
await this.removeMcp(item, target)
break
default:
throw new Error(`Unsupported item type: ${(item as any).type}`)
throw new Error("Unsupported item type")
}
}
@ -334,7 +335,7 @@ export class SimpleInstaller {
// Get the current modes and locate the exact marketplace-installed mode for the selected target
const modes = await this.customModesManager.getCustomModes()
const candidate = modes.find(
(m: any) =>
(m: ModeConfig) =>
m.slug === modeSlug &&
m.installedFromMarketplace === true &&
m.marketplaceItemId === item.id &&
@ -342,15 +343,15 @@ export class SimpleInstaller {
)
if (!candidate) {
throw new Error("This mode was not installed from the marketplace for the selected target")
throw new Error(t("common:customModes.errors.modeNotFound"))
}
// Delete only from the selected source to avoid unintended removals
if (typeof (this.customModesManager as any).deleteCustomModeForSource === "function") {
await (this.customModesManager as any).deleteCustomModeForSource(modeSlug, target, true)
} else {
// Fallback to legacy deletion if helper is not available
await this.customModesManager.deleteCustomMode(modeSlug, true)
// Scoped deletion not supported in this version
throw new Error(t("marketplace:errors.scopedDeletionNotSupported"))
}
}

View file

@ -42,6 +42,7 @@ describe("SimpleInstaller", () => {
mockContext = {} as vscode.ExtensionContext
mockCustomModesManager = {
deleteCustomMode: vi.fn().mockResolvedValue(undefined),
deleteCustomModeForSource: vi.fn().mockResolvedValue(undefined),
importModeWithRules: vi.fn().mockResolvedValue({ success: true }),
getCustomModes: vi.fn().mockResolvedValue([]),
} as any
@ -225,8 +226,8 @@ describe("SimpleInstaller", () => {
await installer.removeItem(mockModeItem, { target: "project" })
// Should call deleteCustomMode with fromMarketplace flag set to true
expect(mockCustomModesManager.deleteCustomMode).toHaveBeenCalledWith("test", true)
// Should call scoped deletion for the selected source with fromMarketplace flag set to true
expect((mockCustomModesManager as any).deleteCustomModeForSource).toHaveBeenCalledWith("test", "project", true)
// The rules folder deletion is now handled by CustomModesManager, not SimpleInstaller
expect(fileExistsAtPath).not.toHaveBeenCalled()
expect(mockFs.rm).not.toHaveBeenCalled()
@ -240,8 +241,8 @@ describe("SimpleInstaller", () => {
await installer.removeItem(mockModeItem, { target: "global" })
// Should call deleteCustomMode with fromMarketplace flag set to true
expect(mockCustomModesManager.deleteCustomMode).toHaveBeenCalledWith("test", true)
// Should call scoped deletion for the selected source with fromMarketplace flag set to true
expect((mockCustomModesManager as any).deleteCustomModeForSource).toHaveBeenCalledWith("test", "global", true)
// The rules folder deletion is now handled by CustomModesManager, not SimpleInstaller
expect(fileExistsAtPath).not.toHaveBeenCalled()
expect(mockFs.rm).not.toHaveBeenCalled()
@ -255,8 +256,8 @@ describe("SimpleInstaller", () => {
await installer.removeItem(mockModeItem, { target: "project" })
// Should call deleteCustomMode with fromMarketplace flag set to true
expect(mockCustomModesManager.deleteCustomMode).toHaveBeenCalledWith("test", true)
// Should call scoped deletion for the selected source with fromMarketplace flag set to true
expect((mockCustomModesManager as any).deleteCustomModeForSource).toHaveBeenCalledWith("test", "project", true)
// The rules folder deletion is now handled by CustomModesManager, not SimpleInstaller
expect(fileExistsAtPath).not.toHaveBeenCalled()
expect(mockFs.rm).not.toHaveBeenCalled()
@ -267,24 +268,24 @@ describe("SimpleInstaller", () => {
vi.mocked(mockCustomModesManager.getCustomModes).mockResolvedValueOnce([
{ slug: "test", name: "Test Mode", source: "project", installedFromMarketplace: true, marketplaceItemId: "test-mode" } as any,
])
// Mock that deleteCustomMode fails
mockCustomModesManager.deleteCustomMode = vi.fn().mockRejectedValueOnce(new Error("Permission denied"))
// Mock that scoped deletion fails
;(mockCustomModesManager as any).deleteCustomModeForSource = vi
.fn()
.mockRejectedValueOnce(new Error("Permission denied"))
// Should throw the error from deleteCustomMode
// Should throw the error from scoped deletion
await expect(installer.removeItem(mockModeItem, { target: "project" })).rejects.toThrow("Permission denied")
expect(mockCustomModesManager.deleteCustomMode).toHaveBeenCalledWith("test", true)
expect((mockCustomModesManager as any).deleteCustomModeForSource).toHaveBeenCalledWith("test", "project", true)
})
it("should throw when mode is not marketplace-installed for selected target", async () => {
// Mock that the mode doesn't exist in the list
vi.mocked(mockCustomModesManager.getCustomModes).mockResolvedValueOnce([])
await expect(installer.removeItem(mockModeItem, { target: "project" })).rejects.toThrow(
"This mode was not installed from the marketplace for the selected target",
)
await expect(installer.removeItem(mockModeItem, { target: "project" })).rejects.toThrow(/Mode not found/)
expect(mockCustomModesManager.deleteCustomMode).not.toHaveBeenCalled()
expect((mockCustomModesManager as any).deleteCustomModeForSource).not.toHaveBeenCalled()
})
it("should throw error when mode content is invalid YAML", async () => {
@ -339,7 +340,7 @@ describe("SimpleInstaller", () => {
await installer.removeItem(arrayContentItem, { target: "project" })
expect(mockCustomModesManager.deleteCustomMode).toHaveBeenCalledWith("test-array", true)
expect((mockCustomModesManager as any).deleteCustomModeForSource).toHaveBeenCalledWith("test-array", "project", true)
})
it("should throw error when CustomModesManager is not available", async () => {