Replies: 1 comment
|
Measurement check on defect 1 (two argv elements vs. one) I reproduced this on
That last row is most likely what looked like "Explorer opens the Desktop": an operand that parses but cannot be resolved produces an empty window, whereas a malformed operand (percent-encoded form, or quotes embedded in the element) makes Explorer fall back to What actually breaks your repro is the other two layers, both covered in #6629: (a) Explorer does not decode the percent-encoded file URL, so non-ASCII paths never resolve, and (b) One caveat on the fix shape so it does not regress: merging |
Uh oh!
There was an error while loading. Please reload this page.
Environment
0.1.5-rc.2@deepseek-ai/dsh-native-command0.1.5-rc.2@deepseek-ai/dsh-api-session-controller0.1.5-rc.2@deepseek-ai/dsh-client-ui-deliverables0.1.5-rc.2explorer.exe10.0.26100.8875v22.22.2Summary
Clicking "Reveal in File Explorer" on a presented-file card does nothing visible.
The request succeeds end-to-end — HTTP 200,
{ opened: true }, no error in the UI — but no Explorer window opens on the target directory. In some runs Explorer instead opens the Desktop folder.Root cause:
revealNativePath()on Windows has two independent defects that both make theexplorer.exe /select,invocation invalid, and the surroundingcatchdeliberately swallows Explorer's exit code 1, so the failure is indistinguishable from success.Steps to reproduce
presenttool call), so a file card appears in the conversation.Reproduced with a path containing non-ASCII characters, e.g.
E:\AI\AgentsWorkspace\DeepSeek_Harness\HalloWord\docs\browser-control-分享版.md.Expected vs observed
200, body accepts the revealRoot cause
packages/util/native-command/src/path-opener.ts(modulepath-opener), Windows branch ofrevealNativePath:Defect 1 —
/select,and the path are passed as two separate argv entriesrunNativeCommandusesexecFilewithoutwindowsVerbatimArguments:Node therefore reconstructs a command line with a space between the two array entries:
explorer.exerequires/select,<path>to be one single argument. With the split, Explorer treats/select,as an empty target and falls back to opening the Desktop.Defect 2 — the path is a percent-encoded
file://URL, not a native pathpathToFileURL(...).hrefescapes non-ASCII and spaces:Even when the argument is correctly merged into one, this escaped URL is not accepted by
explorer.exe /select,(see variant c below). The native Windows path is required.Evidence (measured on the reporting host)
Each variant was invoked with the exact same
execFilesemantics DSH uses (same argv array, same options), and the outcome was determined by enumerating Explorer windows through theShell.ApplicationCOM object (.Windows().LocationURL) before and after.["/select,", fileURL]explorer.exe /select, file:///...%E5%88%86...mdC:\Users\csyt\Desktop(wrong)[`/select,${windowsPath}`]explorer.exe /select,E:\...\browser-control-分享版.mdE:\...\HalloWord\docs(correct)[`/select,${fileURL}`]explorer.exe /select,file:///...%E5%88%86...mdexplorer.exereturned exit code 1 in all three variants, confirming that the exit code carries no signal here.Why the bug is silent
The
catchblock intentionally accepts exit code 1 — per the doc comment, "Explorer exit 1 is accepted as a delegated handoff, not proof of selection." That assumption is correct in general; Explorer frequently exits 1 even on success.The consequence, however, is that success and total failure are indistinguishable. The host route still returns success:
and the browser half only reacts to a non-
okresponse:So the UI reports success while nothing happens — the user has no signal to report or debug.
Proposed fix
if (manager === "explorer") { let windowsPath = path; if (platform === "linux") { const translated = await run("wslpath", ["-w", path], signal); signal.throwIfAborted(); windowsPath = translated.stdout.replace(/[\r\n]+$/, ""); if (windowsPath === "") throw new Error("wslpath returned no Windows path"); } - const target = pathToFileURL(windowsPath, { windows: true }).href.replaceAll(",", "%2C"); try { - await run("explorer.exe", ["/select,", target], signal); + await run("explorer.exe", [`/select,${windowsPath}`], signal); } catch (error) { signal.throwIfAborted(); if (!(error instanceof Error) || !("code" in error) || error.code !== 1) throw error; } return; }Three changes:
/select,and the path into one argv entry.pathToFileURL— pass the native Windows path. (ThepathToFileURLimport becomes unused and can be removed if nothing else needs it.)Open question: paths containing commas
The original
%2Cescaping suggests comma-containing paths were a known concern. With the single-argument form:execFilequotes arguments containing spaces)./select. Explicit quoting (/select,"C:\a,b.md") would be the safe form.Suggest covering this in the fix and adding a regression test.
Not affected
The sibling menu item "Open with default app" is fine — it uses a different code path:
Invoke-Item -LiteralPathhas neither the/select,argument-splitting problem nor the URL-escaping problem. So the two menu items disagree:Suggested regression tests
For
revealNativePathwith an injectedrunspy, onplatform: "win32":runis called as("explorer.exe", [/select,${nativePath}]): exactly one argument after the executable, and it starts with/select,.file://or percent escapes.C:\docs\报告.mdis passed through unchanged.C:\docs\a,b.md(quoted form).Reproduction script and a three-variant comparison are available on request.
All reactions