Skip to content

fix: respect customStoragePath setting for custom modes #7155

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

fix: respect customStoragePath setting for custom modes #7155

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/core/config/CustomModesManager.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import { getWorkspacePath } from "../../utils/path"
import { getGlobalRooDirectory } from "../../services/roo-config"
import { logger } from "../../utils/logging"
import { GlobalFileNames } from "../../shared/globalFileNames"
import { ensureSettingsDirectoryExists } from "../../utils/globalContext"
import { getSettingsDirectoryPath } from "../../utils/storage"
import { t } from "../../i18n"

const ROOMODES_FILENAME = ".roomodes"
Expand Down Expand Up @@ -246,7 +246,7 @@ export class CustomModesManager {
}

public async getCustomModesFilePath(): Promise<string> {
const settingsDir = await ensureSettingsDirectoryExists(this.context)
const settingsDir = await getSettingsDirectoryPath(this.context.globalStorageUri.fsPath)
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this intentional? You're passing context.globalStorageUri.fsPath to getSettingsDirectoryPath(), but that function expects the default storage path and internally calls getStorageBasePath() to apply custom path logic. This could cause the custom path resolution to be applied twice, potentially breaking the path resolution.

Should this instead just pass the context or ensure the function signature matches the expected input?

const filePath = path.join(settingsDir, GlobalFileNames.customModes)
const fileExists = await fileExistsAtPath(filePath)

Expand Down
9 changes: 3 additions & 6 deletions src/utils/globalContext.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,10 @@
import { mkdir } from "fs/promises"
import { join } from "path"
import { ExtensionContext } from "vscode"
import { getStorageBasePath, getSettingsDirectoryPath } from "./storage"

export async function getGlobalFsPath(context: ExtensionContext): Promise<string> {
return context.globalStorageUri.fsPath
return getStorageBasePath(context.globalStorageUri.fsPath)
}

export async function ensureSettingsDirectoryExists(context: ExtensionContext): Promise<string> {
const settingsDir = join(context.globalStorageUri.fsPath, "settings")
await mkdir(settingsDir, { recursive: true })
return settingsDir
return getSettingsDirectoryPath(context.globalStorageUri.fsPath)
}
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change looks correct in delegating to the storage utilities, but I'm wondering if there might be a subtle issue here. The getStorageBasePath() function expects the default path and applies custom path logic on top of it. Since you're passing context.globalStorageUri.fsPath directly, this should work as intended.

However, have we verified that all 17 call sites of ensureSettingsDirectoryExists() are expecting this new behavior where custom paths are respected? This could be a breaking change for code that specifically needs the default storage location.

Loading