Skip to content

Commit c6b644a

Browse files
matt-rudolphmatt-rudolph
committed
refactor(marketplace): typed origin checks + helper; scoped deletion safety and i18n; atomic deleteCustomModeForSource; update tests and en locale
1 parent a76bc0b commit c6b644a

File tree

5 files changed

+88
-74
lines changed

5 files changed

+88
-74
lines changed

‎src/core/config/CustomModesManager.ts‎

Lines changed: 21 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -563,35 +563,35 @@ export class CustomModesManager {
563563
fromMarketplace = false,
564564
): Promise<void> {
565565
try {
566-
const settingsPath = await this.getCustomModesFilePath()
567-
const roomodesPath = await this.getWorkspaceRoomodes()
566+
await this.queueWrite(async () => {
567+
const settingsPath = await this.getCustomModesFilePath()
568+
const roomodesPath = await this.getWorkspaceRoomodes()
568569

569-
let targetPath: string | undefined
570-
let modeToDelete: ModeConfig | undefined
570+
let targetPath: string
571+
let modeToDelete: ModeConfig | undefined
571572

572-
if (source === "project") {
573-
if (!roomodesPath) {
574-
throw new Error(t("common:customModes.errors.noWorkspaceForProject"))
573+
if (source === "project") {
574+
if (!roomodesPath) {
575+
throw new Error(t("common:customModes.errors.noWorkspaceForProject"))
576+
}
577+
targetPath = roomodesPath
578+
const roomodesModes = await this.loadModesFromFile(roomodesPath)
579+
modeToDelete = roomodesModes.find((m) => m.slug === slug)
580+
} else {
581+
targetPath = settingsPath
582+
const settingsModes = await this.loadModesFromFile(settingsPath)
583+
modeToDelete = settingsModes.find((m) => m.slug === slug)
575584
}
576-
targetPath = roomodesPath
577-
const roomodesModes = await this.loadModesFromFile(roomodesPath)
578-
modeToDelete = roomodesModes.find((m) => m.slug === slug)
579-
} else {
580-
targetPath = settingsPath
581-
const settingsModes = await this.loadModesFromFile(settingsPath)
582-
modeToDelete = settingsModes.find((m) => m.slug === slug)
583-
}
584585

585-
if (!modeToDelete) {
586-
throw new Error(t("common:customModes.errors.modeNotFound"))
587-
}
586+
if (!modeToDelete) {
587+
throw new Error(t("common:customModes.errors.modeNotFound"))
588+
}
588589

589-
await this.queueWrite(async () => {
590590
// Delete only from the selected source file
591-
await this.updateModesInFile(targetPath!, (modes) => modes.filter((m) => m.slug !== slug))
591+
await this.updateModesInFile(targetPath, (modes) => modes.filter((m) => m.slug !== slug))
592592

593593
// Delete associated rules folder using the located mode (preserves correct scope)
594-
await this.deleteRulesFolder(slug, modeToDelete!, fromMarketplace)
594+
await this.deleteRulesFolder(slug, modeToDelete, fromMarketplace)
595595

596596
// Refresh state and clear caches
597597
this.clearCache()

‎src/i18n/locales/en/marketplace.json‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,5 +65,8 @@
6565
"removing": "Removing item: \"{{itemName}}\"",
6666
"removeSuccess": "\"{{itemName}}\" removed successfully",
6767
"removeError": "Failed to remove \"{{itemName}}\": {{errorMessage}}"
68+
},
69+
"errors": {
70+
"scopedDeletionNotSupported": "Scoped deletion is not supported in this version. Please update Roo Code to the latest version and try again."
6871
}
6972
}

‎src/services/marketplace/MarketplaceManager.ts‎

Lines changed: 36 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,8 @@ import * as path from "path"
44
import * as vscode from "vscode"
55
import * as yaml from "yaml"
66

7-
import type { OrganizationSettings, MarketplaceItem, MarketplaceItemType, McpMarketplaceItem } from "@roo-code/types"
7+
import type { OrganizationSettings, MarketplaceItem, MarketplaceItemType, McpMarketplaceItem, ModeConfig } from "@roo-code/types"
8+
import { customModesSettingsSchema } from "@roo-code/types"
89
import { TelemetryService } from "@roo-code/telemetry"
910
import { CloudService } from "@roo-code/cloud"
1011

@@ -241,6 +242,38 @@ export class MarketplaceManager {
241242
return metadata
242243
}
243244

245+
// Helper: identify marketplace-installed modes
246+
private isMarketplaceInstalledMode(
247+
mode: ModeConfig,
248+
): mode is ModeConfig & { installedFromMarketplace: true; marketplaceItemId: string } {
249+
return (
250+
mode.installedFromMarketplace === true &&
251+
typeof mode.marketplaceItemId === "string" &&
252+
mode.marketplaceItemId.length > 0
253+
)
254+
}
255+
256+
// Helper: parse YAML and collect installed mode metadata with proper typing
257+
private collectInstalledModesFromYaml(
258+
content: string,
259+
out: Record<string, { type: string }>,
260+
): void {
261+
try {
262+
const parsed = yaml.parse(content)
263+
const result = customModesSettingsSchema.safeParse(parsed)
264+
if (!result.success) {
265+
return
266+
}
267+
for (const mode of result.data.customModes) {
268+
if (this.isMarketplaceInstalledMode(mode)) {
269+
out[mode.marketplaceItemId!] = { type: "mode" }
270+
}
271+
}
272+
} catch {
273+
// Ignore parse errors here; caller handles file existence/errors
274+
}
275+
}
276+
244277
/**
245278
* Check for project-level installed items
246279
*/
@@ -255,19 +288,7 @@ export class MarketplaceManager {
255288
const projectModesPath = path.join(workspaceFolder.uri.fsPath, ".roomodes")
256289
try {
257290
const content = await fs.readFile(projectModesPath, "utf-8")
258-
const data = yaml.parse(content)
259-
if (data?.customModes && Array.isArray(data.customModes)) {
260-
for (const mode of data.customModes) {
261-
// Only consider marketplace-installed modes and key by marketplaceItemId
262-
const fromMarketplace = (mode as any)?.installedFromMarketplace === true
263-
const marketplaceItemId = (mode as any)?.marketplaceItemId
264-
if (fromMarketplace && typeof marketplaceItemId === "string" && marketplaceItemId.length > 0) {
265-
metadata[marketplaceItemId] = {
266-
type: "mode",
267-
}
268-
}
269-
}
270-
}
291+
this.collectInstalledModesFromYaml(content, metadata)
271292
} catch (error) {
272293
// File doesn't exist or can't be read, skip
273294
}
@@ -303,19 +324,7 @@ export class MarketplaceManager {
303324
const globalModesPath = path.join(globalSettingsPath, GlobalFileNames.customModes)
304325
try {
305326
const content = await fs.readFile(globalModesPath, "utf-8")
306-
const data = yaml.parse(content)
307-
if (data?.customModes && Array.isArray(data.customModes)) {
308-
for (const mode of data.customModes) {
309-
// Only consider marketplace-installed modes and key by marketplaceItemId
310-
const fromMarketplace = (mode as any)?.installedFromMarketplace === true
311-
const marketplaceItemId = (mode as any)?.marketplaceItemId
312-
if (fromMarketplace && typeof marketplaceItemId === "string" && marketplaceItemId.length > 0) {
313-
metadata[marketplaceItemId] = {
314-
type: "mode",
315-
}
316-
}
317-
}
318-
}
327+
this.collectInstalledModesFromYaml(content, metadata)
319328
} catch (error) {
320329
// File doesn't exist or can't be read, skip
321330
}

‎src/services/marketplace/SimpleInstaller.ts‎

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,11 @@ import * as vscode from "vscode"
22
import * as path from "path"
33
import * as fs from "fs/promises"
44
import * as yaml from "yaml"
5-
import type { MarketplaceItem, MarketplaceItemType, InstallMarketplaceItemOptions, McpParameter } from "@roo-code/types"
5+
import type { MarketplaceItem, MarketplaceItemType, InstallMarketplaceItemOptions, McpParameter, ModeConfig } from "@roo-code/types"
66
import { GlobalFileNames } from "../../shared/globalFileNames"
77
import { ensureSettingsDirectoryExists } from "../../utils/globalContext"
88
import type { CustomModesManager } from "../../core/config/CustomModesManager"
9+
import { t } from "../../i18n"
910

1011
export interface InstallOptions {
1112
target: "project" | "global"
@@ -28,7 +29,7 @@ export class SimpleInstaller {
2829
case "mcp":
2930
return await this.installMcp(item, target, options)
3031
default:
31-
throw new Error(`Unsupported item type: ${(item as any).type}`)
32+
throw new Error("Unsupported item type")
3233
}
3334
}
3435

@@ -51,8 +52,8 @@ export class SimpleInstaller {
5152
const parsedMode = yaml.parse(item.content)
5253
// Annotate marketplace origin to disambiguate from user-created modes
5354
if (parsedMode && typeof parsedMode === "object") {
54-
;(parsedMode as any).installedFromMarketplace = true
55-
;(parsedMode as any).marketplaceItemId = item.id
55+
;(parsedMode as ModeConfig).installedFromMarketplace = true
56+
;(parsedMode as ModeConfig).marketplaceItemId = item.id
5657
}
5758
const importData = {
5859
customModes: [parsedMode],
@@ -97,8 +98,8 @@ export class SimpleInstaller {
9798
const modeData = yaml.parse(item.content)
9899
// Annotate marketplace origin fields for fallback path as well
99100
if (modeData && typeof modeData === "object") {
100-
;(modeData as any).installedFromMarketplace = true
101-
;(modeData as any).marketplaceItemId = item.id
101+
;(modeData as ModeConfig).installedFromMarketplace = true
102+
;(modeData as ModeConfig).marketplaceItemId = item.id
102103
}
103104

104105
// Read existing file or create new structure
@@ -301,7 +302,7 @@ export class SimpleInstaller {
301302
await this.removeMcp(item, target)
302303
break
303304
default:
304-
throw new Error(`Unsupported item type: ${(item as any).type}`)
305+
throw new Error("Unsupported item type")
305306
}
306307
}
307308

@@ -334,23 +335,23 @@ export class SimpleInstaller {
334335
// Get the current modes and locate the exact marketplace-installed mode for the selected target
335336
const modes = await this.customModesManager.getCustomModes()
336337
const candidate = modes.find(
337-
(m: any) =>
338+
(m: ModeConfig) =>
338339
m.slug === modeSlug &&
339340
m.installedFromMarketplace === true &&
340341
m.marketplaceItemId === item.id &&
341342
m.source === target,
342343
)
343344

344345
if (!candidate) {
345-
throw new Error("This mode was not installed from the marketplace for the selected target")
346+
throw new Error(t("common:customModes.errors.modeNotFound"))
346347
}
347348

348349
// Delete only from the selected source to avoid unintended removals
349350
if (typeof (this.customModesManager as any).deleteCustomModeForSource === "function") {
350351
await (this.customModesManager as any).deleteCustomModeForSource(modeSlug, target, true)
351352
} else {
352-
// Fallback to legacy deletion if helper is not available
353-
await this.customModesManager.deleteCustomMode(modeSlug, true)
353+
// Scoped deletion not supported in this version
354+
throw new Error(t("marketplace:errors.scopedDeletionNotSupported"))
354355
}
355356
}
356357

‎src/services/marketplace/__tests__/SimpleInstaller.spec.ts‎

Lines changed: 16 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ describe("SimpleInstaller", () => {
4242
mockContext = {} as vscode.ExtensionContext
4343
mockCustomModesManager = {
4444
deleteCustomMode: vi.fn().mockResolvedValue(undefined),
45+
deleteCustomModeForSource: vi.fn().mockResolvedValue(undefined),
4546
importModeWithRules: vi.fn().mockResolvedValue({ success: true }),
4647
getCustomModes: vi.fn().mockResolvedValue([]),
4748
} as any
@@ -225,8 +226,8 @@ describe("SimpleInstaller", () => {
225226

226227
await installer.removeItem(mockModeItem, { target: "project" })
227228

228-
// Should call deleteCustomMode with fromMarketplace flag set to true
229-
expect(mockCustomModesManager.deleteCustomMode).toHaveBeenCalledWith("test", true)
229+
// Should call scoped deletion for the selected source with fromMarketplace flag set to true
230+
expect((mockCustomModesManager as any).deleteCustomModeForSource).toHaveBeenCalledWith("test", "project", true)
230231
// The rules folder deletion is now handled by CustomModesManager, not SimpleInstaller
231232
expect(fileExistsAtPath).not.toHaveBeenCalled()
232233
expect(mockFs.rm).not.toHaveBeenCalled()
@@ -240,8 +241,8 @@ describe("SimpleInstaller", () => {
240241

241242
await installer.removeItem(mockModeItem, { target: "global" })
242243

243-
// Should call deleteCustomMode with fromMarketplace flag set to true
244-
expect(mockCustomModesManager.deleteCustomMode).toHaveBeenCalledWith("test", true)
244+
// Should call scoped deletion for the selected source with fromMarketplace flag set to true
245+
expect((mockCustomModesManager as any).deleteCustomModeForSource).toHaveBeenCalledWith("test", "global", true)
245246
// The rules folder deletion is now handled by CustomModesManager, not SimpleInstaller
246247
expect(fileExistsAtPath).not.toHaveBeenCalled()
247248
expect(mockFs.rm).not.toHaveBeenCalled()
@@ -255,8 +256,8 @@ describe("SimpleInstaller", () => {
255256

256257
await installer.removeItem(mockModeItem, { target: "project" })
257258

258-
// Should call deleteCustomMode with fromMarketplace flag set to true
259-
expect(mockCustomModesManager.deleteCustomMode).toHaveBeenCalledWith("test", true)
259+
// Should call scoped deletion for the selected source with fromMarketplace flag set to true
260+
expect((mockCustomModesManager as any).deleteCustomModeForSource).toHaveBeenCalledWith("test", "project", true)
260261
// The rules folder deletion is now handled by CustomModesManager, not SimpleInstaller
261262
expect(fileExistsAtPath).not.toHaveBeenCalled()
262263
expect(mockFs.rm).not.toHaveBeenCalled()
@@ -267,24 +268,24 @@ describe("SimpleInstaller", () => {
267268
vi.mocked(mockCustomModesManager.getCustomModes).mockResolvedValueOnce([
268269
{ slug: "test", name: "Test Mode", source: "project", installedFromMarketplace: true, marketplaceItemId: "test-mode" } as any,
269270
])
270-
// Mock that deleteCustomMode fails
271-
mockCustomModesManager.deleteCustomMode = vi.fn().mockRejectedValueOnce(new Error("Permission denied"))
271+
// Mock that scoped deletion fails
272+
;(mockCustomModesManager as any).deleteCustomModeForSource = vi
273+
.fn()
274+
.mockRejectedValueOnce(new Error("Permission denied"))
272275

273-
// Should throw the error from deleteCustomMode
276+
// Should throw the error from scoped deletion
274277
await expect(installer.removeItem(mockModeItem, { target: "project" })).rejects.toThrow("Permission denied")
275278

276-
expect(mockCustomModesManager.deleteCustomMode).toHaveBeenCalledWith("test", true)
279+
expect((mockCustomModesManager as any).deleteCustomModeForSource).toHaveBeenCalledWith("test", "project", true)
277280
})
278281

279282
it("should throw when mode is not marketplace-installed for selected target", async () => {
280283
// Mock that the mode doesn't exist in the list
281284
vi.mocked(mockCustomModesManager.getCustomModes).mockResolvedValueOnce([])
282285

283-
await expect(installer.removeItem(mockModeItem, { target: "project" })).rejects.toThrow(
284-
"This mode was not installed from the marketplace for the selected target",
285-
)
286+
await expect(installer.removeItem(mockModeItem, { target: "project" })).rejects.toThrow(/Mode not found/)
286287

287-
expect(mockCustomModesManager.deleteCustomMode).not.toHaveBeenCalled()
288+
expect((mockCustomModesManager as any).deleteCustomModeForSource).not.toHaveBeenCalled()
288289
})
289290

290291
it("should throw error when mode content is invalid YAML", async () => {
@@ -339,7 +340,7 @@ describe("SimpleInstaller", () => {
339340

340341
await installer.removeItem(arrayContentItem, { target: "project" })
341342

342-
expect(mockCustomModesManager.deleteCustomMode).toHaveBeenCalledWith("test-array", true)
343+
expect((mockCustomModesManager as any).deleteCustomModeForSource).toHaveBeenCalledWith("test-array", "project", true)
343344
})
344345

345346
it("should throw error when CustomModesManager is not available", async () => {

0 commit comments

Comments
 (0)