Skip to content

fix: improve command registration for slow environments like PROOT-distro/Termux #8817

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: improve command registration for slow environments like PROOT-distro/Termux #8817

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
31 changes: 29 additions & 2 deletions src/activate/registerCommands.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,11 +64,38 @@ export type RegisterCommandOptions = {
}

export const registerCommands = (options: RegisterCommandOptions) => {
const { context } = options
const { context, outputChannel } = options
const failedCommands: string[] = []

for (const [id, callback] of Object.entries(getCommandsMap(options))) {
const command = getCommand(id as CommandId)
context.subscriptions.push(vscode.commands.registerCommand(command, callback))
try {
// Check if command already exists (might have been registered as placeholder)
const existingIndex = context.subscriptions.findIndex((sub) => (sub as any).command === command)
Copy link
Author

Choose a reason for hiding this comment

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

The command re-registration logic is broken because VSCode's Disposable objects don't have a command property. This findIndex will never find existing commands, meaning the disposal code on lines 77-80 will never execute. As a result, when commands are re-registered at line 303 in extension.ts, you'll get duplicate command registrations rather than replacements. VSCode command disposables are opaque objects - there's no way to introspect which command they represent. Consider maintaining a separate Map<string, Disposable> to track command registrations if you need to replace them.


// Remove existing placeholder if found
if (existingIndex !== -1) {
const existing = context.subscriptions[existingIndex]
context.subscriptions.splice(existingIndex, 1)
existing.dispose()
}

// Register the command with proper handler
const disposable = vscode.commands.registerCommand(command, callback)
context.subscriptions.push(disposable)
} catch (error) {
failedCommands.push(id)
outputChannel.appendLine(
`[Commands] Failed to register command '${id}': ${error instanceof Error ? error.message : String(error)}`,
)
}
}

if (failedCommands.length > 0) {
outputChannel.appendLine(
`[Commands] Warning: Failed to register ${failedCommands.length} command(s): ${failedCommands.join(", ")}`,
)
// Don't throw - allow extension to continue with partial functionality
}
}

Expand Down
88 changes: 84 additions & 4 deletions src/extension.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,24 @@ export async function activate(context: vscode.ExtensionContext) {
// Create logger for cloud services.
const cloudLogger = createDualLogger(createOutputChannelLogger(outputChannel))

// Initialize MDM service
const mdmService = await MdmService.createInstance(cloudLogger)
// Initialize MDM service (with timeout for slow environments)
let mdmService: MdmService | undefined
try {
mdmService = await Promise.race([
MdmService.createInstance(cloudLogger),
new Promise<MdmService | undefined>((resolve) =>
setTimeout(() => {
outputChannel.appendLine("[MDM] Initialization timeout - continuing without MDM")
resolve(undefined)
}, 5000),
),
])
} catch (error) {
outputChannel.appendLine(
`[MDM] Failed to initialize: ${error instanceof Error ? error.message : String(error)}`,
)
mdmService = undefined
}

// Initialize i18n for internationalization support.
initializeI18n(context.globalState.get("language") ?? formatLanguage(vscode.env.language))
Expand All @@ -97,7 +113,24 @@ export async function activate(context: vscode.ExtensionContext) {
context.globalState.update("allowedCommands", defaultCommands)
}

const contextProxy = await ContextProxy.getInstance(context)
// Get context proxy with timeout for slow environments
let contextProxy: ContextProxy
try {
contextProxy = await Promise.race([
ContextProxy.getInstance(context),
new Promise<ContextProxy>((resolve, reject) =>
setTimeout(() => {
reject(new Error("ContextProxy initialization timeout"))
}, 10000),
),
])
} catch (error) {
outputChannel.appendLine(
`[ContextProxy] Failed to initialize, using fallback: ${error instanceof Error ? error.message : String(error)}`,
)
// Force creation even if slow
contextProxy = await ContextProxy.getInstance(context)
}
Comment on lines +127 to +133
Copy link
Author

Choose a reason for hiding this comment

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

The ContextProxy fallback after timeout could still hang indefinitely. If ContextProxy.getInstance() times out on line 124, the catch block on line 132 forces creation without any timeout protection. In slow environments like PROOT-distro/Termux (the issue this PR addresses), this could cause the same hang that the timeout was trying to prevent. Consider either accepting undefined for contextProxy when timeout occurs, or implementing a retry with exponential backoff rather than forcing immediate re-creation.


// Initialize code index managers for all workspace folders.
const codeIndexManagers: CodeIndexManager[] = []
Expand Down Expand Up @@ -125,6 +158,45 @@ export async function activate(context: vscode.ExtensionContext) {
// Initialize the provider *before* the Roo Code Cloud service.
const provider = new ClineProvider(context, outputChannel, "sidebar", contextProxy, mdmService)

// CRITICAL: Register commands early to ensure they're available on slow environments
// This must happen before any long-running async operations
try {
registerCommands({ context, outputChannel, provider })
outputChannel.appendLine("[Commands] Successfully registered all commands")
} catch (error) {
outputChannel.appendLine(
`[Commands] CRITICAL: Failed to register commands: ${error instanceof Error ? error.message : String(error)}`,
)
// Attempt individual command registration as fallback
try {
const criticalCommands = [
"settingsButtonClicked",
"plusButtonClicked",
"mcpButtonClicked",
"historyButtonClicked",
]
for (const cmdId of criticalCommands) {
try {
const command = `${Package.name}.${cmdId}`
if (!context.subscriptions.find((sub) => (sub as any).command === command)) {
Copy link
Author

Choose a reason for hiding this comment

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

The fallback command detection has the same issue as the main registration - checking (sub as any).command === command won't work because Disposables don't expose which command they registered. This means the if condition on line 181 will always be false, and you'll register duplicate placeholder commands every time this fallback executes. This could lead to multiple warning messages appearing when a user clicks a button. The safer approach would be to maintain a Set of registered command IDs to prevent duplicates.

context.subscriptions.push(
vscode.commands.registerCommand(command, () => {
outputChannel.appendLine(`[Command] ${cmdId} invoked but handler not fully initialized`)
vscode.window.showWarningMessage(
`Extension is still initializing. Please try again in a moment.`,
)
}),
)
}
} catch (cmdError) {
outputChannel.appendLine(`[Commands] Failed to register fallback for ${cmdId}: ${cmdError}`)
}
}
} catch (fallbackError) {
outputChannel.appendLine(`[Commands] Failed to register fallback commands: ${fallbackError}`)
}
}

// Initialize Roo Code Cloud service.
const postStateListener = () => ClineProvider.getVisibleInstance()?.postStateToWebview()

Expand Down Expand Up @@ -225,7 +297,15 @@ export async function activate(context: vscode.ExtensionContext) {
)
}

registerCommands({ context, outputChannel, provider })
// Commands already registered earlier, but re-register to ensure proper handlers
// This overwrites the temporary handlers with the actual ones
try {
registerCommands({ context, outputChannel, provider })
} catch (error) {
outputChannel.appendLine(
`[Commands] Failed to re-register commands with proper handlers: ${error instanceof Error ? error.message : String(error)}`,
)
}

/**
* We use the text document content provider API to show the left side for diff
Expand Down