-
Notifications
You must be signed in to change notification settings - Fork 1.1k
fix(kap-server): quote only the path in explorer /select, argument on Windows #2645
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| "@moonshot-ai/kimi-code": patch | ||
| --- | ||
|
|
||
| Fix the web UI opening the Documents folder instead of the requested file on Windows when the file path contains spaces. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,6 +6,7 @@ export interface LaunchCommand { | |
| readonly command: string; | ||
| readonly args: readonly string[]; | ||
| readonly shell?: boolean; | ||
| readonly windowsVerbatimArguments?: boolean; | ||
| } | ||
|
|
||
| export function openFileCommandFor( | ||
|
|
@@ -44,7 +45,17 @@ export function revealFileCommandFor( | |
| case 'darwin': | ||
| return { command: 'open', args: ['-R', absolutePath] }; | ||
| case 'win32': | ||
| return { command: 'explorer.exe', args: [`/select,${absolutePath}`] }; | ||
| // explorer.exe parses its RAW command line (not argv), so Node's | ||
| // default spawn quoting breaks `/select,` whenever the path contains | ||
| // spaces: the command line becomes `"/select,\"C:\some dir\f.txt\""`, | ||
| // which explorer's parser rejects, silently opening the Documents | ||
| // folder. `windowsVerbatimArguments: true` keeps the command line in | ||
| // the documented `/select,"C:\some dir\f.txt"` form. | ||
| return { | ||
| command: 'explorer.exe', | ||
| args: [explorerSelectArg(absolutePath)], | ||
| windowsVerbatimArguments: true, | ||
| }; | ||
| default: | ||
| return { command: 'xdg-open', args: [path.dirname(absolutePath)] }; | ||
| } | ||
|
|
@@ -163,7 +174,11 @@ function openInFinder( | |
| case 'win32': | ||
| return isDirectory | ||
| ? { command: 'explorer.exe', args: [absolutePath] } | ||
| : { command: 'explorer.exe', args: [`/select,${absolutePath}`] }; | ||
| : { | ||
| command: 'explorer.exe', | ||
| args: [explorerSelectArg(absolutePath)], | ||
| windowsVerbatimArguments: true, | ||
| }; | ||
| default: | ||
| return { | ||
| command: 'xdg-open', | ||
|
|
@@ -191,6 +206,7 @@ export async function launchDetached(cmd: LaunchCommand): Promise<void> { | |
| detached: true, | ||
| stdio: 'ignore', | ||
| shell: cmd.shell, | ||
| windowsVerbatimArguments: cmd.windowsVerbatimArguments, | ||
| }); | ||
| child.once('error', (err) => { | ||
| if (settled) return; | ||
|
|
@@ -219,6 +235,20 @@ function supportsLineTarget(command: string): boolean { | |
| return /(?:^|\/)(code|cursor|windsurf)(?:\.cmd|\.exe)?$/i.test(first); | ||
| } | ||
|
|
||
| /** | ||
| * Build the single `/select,` argument for explorer.exe, quoting only the | ||
| * path: `/select,"C:\some dir\f.txt"`. Must be launched with | ||
| * `windowsVerbatimArguments: true` — explorer parses its raw command line, | ||
| * and Node's default quoting would wrap the whole argument as | ||
| * `"/select,\"C:\...\""`, which explorer rejects (it then silently opens the | ||
| * Documents folder). A trailing backslash is dropped so it cannot escape the | ||
| * closing quote. | ||
| */ | ||
| function explorerSelectArg(absolutePath: string): string { | ||
| const trimmed = absolutePath.replace(/\\+$/, ''); | ||
| return `/select,"${trimmed}"`; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When a Windows path contains spaces, this helper returns the intended textual form, but Useful? React with 👍 / 👎. |
||
| } | ||
|
|
||
| function quoteShellArg(value: string, platform: NodeJS.Platform): string { | ||
| if (platform === 'win32') return `"${value.replaceAll('"', '\\"')}"`; | ||
| return `'${value.replaceAll("'", "'\\''")}'`; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,54 @@ | ||
| import { describe, expect, it } from 'vitest'; | ||
|
|
||
| import { openInAppCommandFor, revealFileCommandFor } from '../src/lib/fileLaunch'; | ||
|
|
||
| describe('fileLaunch', () => { | ||
| describe('win32 explorer /select, quoting', () => { | ||
| // explorer.exe parses its RAW command line (not argv). Node's default | ||
| // spawn quoting renders a `/select,` argument with spaces as | ||
| // `"/select,\"C:\...\""`, which explorer rejects — it then silently opens | ||
| // the Documents folder. The argument must quote only the path, and the | ||
| // launch must bypass Node's quoting via windowsVerbatimArguments. | ||
| it('revealFileCommandFor quotes only the path and uses verbatim arguments', () => { | ||
| const cmd = revealFileCommandFor('C:\\some dir\\sub\\file.txt', 'win32'); | ||
| expect(cmd.command).toBe('explorer.exe'); | ||
| expect(cmd.args).toEqual(['/select,"C:\\some dir\\sub\\file.txt"']); | ||
| expect(cmd.windowsVerbatimArguments).toBe(true); | ||
| }); | ||
|
|
||
| it('openInAppCommandFor (finder) quotes only the path and uses verbatim arguments', () => { | ||
| const cmd = openInAppCommandFor( | ||
| 'finder', | ||
| 'C:\\some dir\\sub\\file.txt', | ||
| { isDirectory: false }, | ||
| 'win32', | ||
| ); | ||
| expect(cmd.command).toBe('explorer.exe'); | ||
| expect(cmd.args).toEqual(['/select,"C:\\some dir\\sub\\file.txt"']); | ||
| expect(cmd.windowsVerbatimArguments).toBe(true); | ||
| }); | ||
|
|
||
| it('openInAppCommandFor (finder) opens directories without /select,', () => { | ||
| const cmd = openInAppCommandFor( | ||
| 'finder', | ||
| 'C:\\some dir\\sub', | ||
| { isDirectory: true }, | ||
| 'win32', | ||
| ); | ||
| expect(cmd.command).toBe('explorer.exe'); | ||
| expect(cmd.args).toEqual(['C:\\some dir\\sub']); | ||
| expect(cmd.windowsVerbatimArguments).toBeUndefined(); | ||
| }); | ||
|
|
||
| it('drops a trailing backslash so it cannot escape the closing quote', () => { | ||
| const cmd = revealFileCommandFor('C:\\some dir\\sub\\', 'win32'); | ||
| expect(cmd.args).toEqual(['/select,"C:\\some dir\\sub"']); | ||
| }); | ||
|
|
||
| it('paths without spaces keep the same quoting', () => { | ||
| const cmd = revealFileCommandFor('C:\\proj\\file.txt', 'win32'); | ||
| expect(cmd.args).toEqual(['/select,"C:\\proj\\file.txt"']); | ||
| expect(cmd.windowsVerbatimArguments).toBe(true); | ||
| }); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When a Windows path contains spaces, this helper returns the intended textual form, but
launchDetachedstill callsspawnwithoutwindowsVerbatimArguments; Node'schild_process.spawndocs say that option defaults tofalse, so Node quotes/escapes this argument as a whole beforeCreateProcess. That leavesexplorer.exein the same failure mode this change is trying to avoid, so reveal/open-in will still fall back to Documents for OneDrive/Program Files paths unless the explorer LaunchCommand carrieswindowsVerbatimArguments: trueor otherwise bypasses Node's argument quoting.Useful? React with 👍 / 👎.