Replies: 5 comments
|
Verified against the checkout at commit Confirmed in the source
Mechanism — confirmed, one small precision fixNode's Independent reproduction (standalone Node script, no DSH involved)Enumerating
The hidden window had the correct title (the target's folder), an on-screen rect FixYour minimal fix is correct and safe:
Not verified / caveats
|
|
Thanks for the detailed verification -- and for the mechanism correction. You are right: libuv sets Filling in the items you marked as not verified. Reporter's environment -- and a correction to the "two builds" argument
One correction: the Windows build here is also 26200 -- the same as yours. So this does not Post-fix end-to-end clickVerified two ways on this machine.
The accumulated invisible windows can be cleaned up without touching ExplorerYou noted they "go away on Explorer restart / sign-out (not verified)". Related observation from They are closable programmatically, with no Explorer restart: $shell = New-Object -ComObject Shell.Application
foreach ($w in @($shell.Windows())) {
if (-not [WinApi]::IsWindowVisible([IntPtr][int64]$w.HWND)) { $w.Quit() }
}That closed all seven and left the visible one untouched. Not worth shipping as a feature, but a Your two suggestions
Foreground behaviour -- confirming your red herring, from the other sideYou reproduced |
|
All three accepted — thanks for filling in the items I left open:
Agreed on both code suggestions (mirror the path-opener.spec.ts:158 windowsHide assertion for the explorer call; one parameterized runner body). Nothing further needed from my side. |
|
Implementing the fix for anyone who lands here later.
The change adds a second runner that omits Measured with a bare Node script that loads no DSH plugin:
After the change One note for whoever picks this up: counting windows is not enough. The count is Branch on my fork (not a PR; I read that external PRs are not being taken right Leaving it here as reference for whenever you get to it. |
|
One more detail worth adding, since it changes what the fix implies elsewhere. A child that reads its own STARTUPINFO reports both effects of
So That second half matters for anyone reaching for The rule the two halves add up to, and the one
So ask two questions: does the child open a window itself (Explorer, an Electron app)? Two other projects hit the same thing in the same month, in both directions: |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Summary
On Windows, the Reveal in File Explorer action (the file-card menu item that hands the
path to
session-controller'srevealrequest ->revealNativePath) creates an Explorerwindow that is permanently invisible. No error is surfaced anywhere -- the UI simply
appears to do nothing, so it reads as a dead button.
Affected
@deepseek-ai/dsh-native-commandpackages/util/native-command/src/path-opener.ts(revealNativePath),src/runner.ts(runNativeCommand)win32) onlyc291e7961aRoot cause
revealNativePathspawns Explorer through the shared runner, which hard-codeswindowsHide: truebecause it was built for console tools:On Windows
windowsHide: truemakes libuv passSTARTF_USESHOWWINDOWwithSW_HIDE, so thechild's first window is created hidden. For a console tool that is exactly right. For a GUI
launcher whose entire purpose is to show a window, it is fatal.
Evidence
After triggering the reveal, enumerating shell windows via
Shell.Application.Windows()and querying the window manager:The window exists, with the right class, the right title and on-screen coordinates -- it is
simply not visible.
Controlled A/B on the same machine, isolated in a standalone Node script that does not
involve DSH at all (so this cannot be a plugin or composition issue):
{ windowsHide: true }{ windowsHide: false }Both cases return exit code 1, which the current code already tolerates as "Explorer delegated
to the existing desktop process" -- so the failure is silent by construction.
Why it is easy to misdiagnose
window, so
windowsHidelooks innocent unless you checkIsWindowVisible. This cost me afull wrong conclusion before I re-tested visibility.
SetForegroundWindowalso returnsFalse, which makes it look like the well-known"background process cannot steal foreground" limitation. That is a symptom, not the cause:
a hidden window cannot be foregrounded. Chasing foreground-lock workarounds goes nowhere.
Suggested fix
Give Explorer a runner that does not hide windows, while keeping the hiding runner for the
console tools (the PowerShell opener, registry probes,
wslpath):Keeping the
internals.run ??indirection preserves the existing test seam(
PathOpenerInternals.run), so adapter tests that inject a runner keep working unchanged.Verified after the change
The same host-side call, from inside the running host process:
whereas the pre-patch windows in the same session were all
visible=False. The user thenconfirmed that clicking the menu item now brings the Explorer window up on screen.
Environment
C:\Program Files\nodejs\node.exec291e7961a), profilewebacross all installed profile plugins found no reference to
openPath/revealPath/nativeOpen/session-controller.All reactions