fix(marketplace): distinguish marketplace-installed modes from local custom modes by origin flags; key installed metadata by marketplaceItemId; restrict removal to marketplace-installed mode in selected scope; add scoped delete in CustomModesManager to avoid unintended deletions (fixes #4828)

This commit is contained in:
matt-rudolph 2025-09-19 11:51:28 -06:00
parent f934363beb
commit a76bc0b5ae
5 changed files with 114 additions and 23 deletions

View file

@ -70,6 +70,9 @@ export const modeConfigSchema = z.object({
customInstructions: z.string().optional(),
groups: groupEntryArraySchema,
source: z.enum(["global", "project"]).optional(),
// Marketplace origin metadata (optional)
installedFromMarketplace: z.boolean().optional(),
marketplaceItemId: z.string().optional(),
})
export type ModeConfig = z.infer<typeof modeConfigSchema>

View file

@ -550,6 +550,58 @@ export class CustomModesManager {
vscode.window.showErrorMessage(t("common:customModes.errors.deleteFailed", { error: errorMessage }))
}
}
/**
* Deletes a custom mode only from the specified source (project or global)
* without affecting the other scope. Also handles rules folder cleanup.
* @param slug - The mode slug
* @param source - "project" or "global"
* @param fromMarketplace - Whether this deletion was initiated by marketplace flows
*/
public async deleteCustomModeForSource(
slug: string,
source: "project" | "global",
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 () => {
// Delete only from the selected source file
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)
// Refresh state and clear caches
this.clearCache()
await this.refreshMergedState()
})
} catch (error) {
const errorMessage = error instanceof Error ? error.message : String(error)
vscode.window.showErrorMessage(t("common:customModes.errors.deleteFailed", { error: errorMessage }))
}
}
/**
* Deletes the rules folder for a specific mode

View file

@ -258,8 +258,11 @@ export class MarketplaceManager {
const data = yaml.parse(content)
if (data?.customModes && Array.isArray(data.customModes)) {
for (const mode of data.customModes) {
if (mode.slug) {
metadata[mode.slug] = {
// 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",
}
}
@ -303,8 +306,11 @@ export class MarketplaceManager {
const data = yaml.parse(content)
if (data?.customModes && Array.isArray(data.customModes)) {
for (const mode of data.customModes) {
if (mode.slug) {
metadata[mode.slug] = {
// 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",
}
}

View file

@ -7,9 +7,10 @@ import { GlobalFileNames } from "../../shared/globalFileNames"
import { ensureSettingsDirectoryExists } from "../../utils/globalContext"
import type { CustomModesManager } from "../../core/config/CustomModesManager"
export interface InstallOptions extends InstallMarketplaceItemOptions {
export interface InstallOptions {
target: "project" | "global"
selectedIndex?: number // Which installation method to use (for array content)
parameters?: Record<string, any>
}
export class SimpleInstaller {
@ -47,8 +48,14 @@ export class SimpleInstaller {
// If CustomModesManager is available, use importModeWithRules
if (this.customModesManager) {
// Transform marketplace content to import format (wrap in customModes array)
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
}
const importData = {
customModes: [yaml.parse(item.content)],
customModes: [parsedMode],
}
const importYaml = yaml.stringify(importData)
@ -72,7 +79,7 @@ export class SimpleInstaller {
// Find the line containing the slug of the added mode
if (modeData?.slug) {
const slugLineIndex = lines.findIndex(
(l) => l.includes(`slug: ${modeData.slug}`) || l.includes(`slug: "${modeData.slug}"`),
(l: string) => l.includes(`slug: ${modeData.slug}`) || l.includes(`slug: "${modeData.slug}"`),
)
if (slugLineIndex >= 0) {
line = slugLineIndex + 1 // Convert to 1-based line number
@ -88,6 +95,11 @@ export class SimpleInstaller {
// Fallback to original implementation if CustomModesManager is not available
const filePath = await this.getModeFilePath(target)
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
}
// Read existing file or create new structure
let existingData: any = { customModes: [] }
@ -143,7 +155,7 @@ export class SimpleInstaller {
const addedMode = existingData.customModes[addedModeIndex]
if (addedMode?.slug) {
const slugLineIndex = lines.findIndex(
(l) => l.includes(`slug: ${addedMode.slug}`) || l.includes(`slug: "${addedMode.slug}"`),
(l: string) => l.includes(`slug: ${addedMode.slug}`) || l.includes(`slug: "${addedMode.slug}"`),
)
if (slugLineIndex >= 0) {
line = slugLineIndex + 1 // Convert to 1-based line number
@ -319,13 +331,27 @@ export class SimpleInstaller {
throw new Error("Mode missing slug identifier")
}
// Get the current modes to determine the source
// Get the current modes and locate the exact marketplace-installed mode for the selected target
const modes = await this.customModesManager.getCustomModes()
const mode = modes.find((m) => m.slug === modeSlug)
const candidate = modes.find(
(m: any) =>
m.slug === modeSlug &&
m.installedFromMarketplace === true &&
m.marketplaceItemId === item.id &&
m.source === target,
)
// Use CustomModesManager to delete the mode configuration
// This also handles rules folder deletion
await this.customModesManager.deleteCustomMode(modeSlug, true)
if (!candidate) {
throw new Error("This mode was not installed from the marketplace for the selected target")
}
// 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)
}
}
private async removeMcp(item: MarketplaceItem, target: "project" | "global"): Promise<void> {

View file

@ -220,7 +220,7 @@ describe("SimpleInstaller", () => {
it("should use CustomModesManager to delete mode and clean up rules folder", async () => {
// Mock that the mode exists with project source
vi.mocked(mockCustomModesManager.getCustomModes).mockResolvedValueOnce([
{ slug: "test", name: "Test Mode", source: "project" } as any,
{ slug: "test", name: "Test Mode", source: "project", installedFromMarketplace: true, marketplaceItemId: "test-mode" } as any,
])
await installer.removeItem(mockModeItem, { target: "project" })
@ -235,7 +235,7 @@ describe("SimpleInstaller", () => {
it("should handle global mode removal with rules cleanup", async () => {
// Mock that the mode exists with global source
vi.mocked(mockCustomModesManager.getCustomModes).mockResolvedValueOnce([
{ slug: "test", name: "Test Mode", source: "global" } as any,
{ slug: "test", name: "Test Mode", source: "global", installedFromMarketplace: true, marketplaceItemId: "test-mode" } as any,
])
await installer.removeItem(mockModeItem, { target: "global" })
@ -250,7 +250,7 @@ describe("SimpleInstaller", () => {
it("should handle case when rules folder does not exist", async () => {
// Mock that the mode exists
vi.mocked(mockCustomModesManager.getCustomModes).mockResolvedValueOnce([
{ slug: "test", name: "Test Mode", source: "project" } as any,
{ slug: "test", name: "Test Mode", source: "project", installedFromMarketplace: true, marketplaceItemId: "test-mode" } as any,
])
await installer.removeItem(mockModeItem, { target: "project" })
@ -265,7 +265,7 @@ describe("SimpleInstaller", () => {
it("should throw error if deleteCustomMode fails", async () => {
// Mock that the mode exists
vi.mocked(mockCustomModesManager.getCustomModes).mockResolvedValueOnce([
{ slug: "test", name: "Test Mode", source: "project" } as any,
{ 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"))
@ -276,16 +276,15 @@ describe("SimpleInstaller", () => {
expect(mockCustomModesManager.deleteCustomMode).toHaveBeenCalledWith("test", true)
})
it("should handle mode not found in custom modes list", async () => {
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 installer.removeItem(mockModeItem, { target: "project" })
await expect(installer.removeItem(mockModeItem, { target: "project" })).rejects.toThrow(
"This mode was not installed from the marketplace for the selected target",
)
expect(mockCustomModesManager.deleteCustomMode).toHaveBeenCalledWith("test", true)
// Should not attempt to delete rules folder
expect(fileExistsAtPath).not.toHaveBeenCalled()
expect(mockFs.rm).not.toHaveBeenCalled()
expect(mockCustomModesManager.deleteCustomMode).not.toHaveBeenCalled()
})
it("should throw error when mode content is invalid YAML", async () => {
@ -333,6 +332,11 @@ describe("SimpleInstaller", () => {
] as any,
}
// Mock installed marketplace mode with matching marketplaceItemId and source
vi.mocked(mockCustomModesManager.getCustomModes).mockResolvedValueOnce([
{ slug: "test-array", name: "Test Array Mode", source: "project", installedFromMarketplace: true, marketplaceItemId: "test-mode" } as any,
])
await installer.removeItem(arrayContentItem, { target: "project" })
expect(mockCustomModesManager.deleteCustomMode).toHaveBeenCalledWith("test-array", true)