From de084690171bc6db30d6dc23029184374054183b Mon Sep 17 00:00:00 2001 From: Ned Date: Thu, 17 Sep 2026 15:38:51 -0700 Subject: [PATCH 1/5] Keep the GUI-subsystem node out of every pane's PATH On Windows the bundled node.exe is patched to the GUI subsystem so spawning the sidecar never flashes a console window. A child of that node gets no console at all: stdin is already at EOF, `setRawMode` is missing, and everything it prints is dropped. `cargo run` puts the directory holding it on the dev app's PATH so Windows resolves the app's DLLs, and panes inherit the app's env -- so a bare `node` in a dev pane found that node.exe ahead of the developer's own and failed silently in both directions. The installed app never has that directory on PATH, so this only ever bit dev builds, and dropping the entry makes a dev pane match an installed one. `start_sidecar` passes the directory as DORMOUSE_GUI_NODE_DIR and the sidecar strips it from the PATH each pane inherits. The host names the directory because `process.execPath` is the wrong answer twice over: the agent-browser harness runs the sidecar under the developer's own node, and the same module runs under the VS Code pty host, where the running node's directory must stay. Co-Authored-By: Claude Opus 5 (1M context) --- docs/specs/standalone.md | 11 ++++ scripts/spec-word-budgets.json | 2 +- standalone/sidecar/pty-core.js | 44 +++++++++++++++- standalone/sidecar/pty-core.test.js | 79 +++++++++++++++++++++++++++++ standalone/src-tauri/src/lib.rs | 10 ++++ 5 files changed, 144 insertions(+), 2 deletions(-) diff --git a/docs/specs/standalone.md b/docs/specs/standalone.md index 99e7ffa10..f8280f1eb 100644 --- a/docs/specs/standalone.md +++ b/docs/specs/standalone.md @@ -237,10 +237,21 @@ because the sidecar and the `dor` CLI have opposite console requirements: `DORMOUSE_NODE` at it. `dor` always runs inside an existing pseudo-console, so that copy can never cause a stray window. +- **Never leave the GUI node's directory on a pane's PATH.** A child of that + node.exe gets no console — stdin already at EOF, no `setRawMode`, output + dropped — so a bare `node` in a dev pane, where `cargo run` adds that + directory for DLL resolution, fails silently. `start_sidecar` passes it as + `DORMOUSE_GUI_NODE_DIR`; the sidecar drops it from the PATH each pane + inherits, matching the installed app, which never has it on PATH. + The byte-flip lives in `standalone/src-tauri/src/pe_subsystem.rs`, shared with `build.rs`, so the load-bearing PE offsets are in one place; the mechanism is in the comments at `force_windows_gui_subsystem` and `resolve_dor_node_path`. +Source of truth: `withoutGuiNodeDir` in `standalone/sidecar/pty-core.js`, +pinned by `resolveSpawnConfig drops the GUI node directory from a pane PATH on +win32` in `standalone/sidecar/pty-core.test.js`. + ## Sidecar lifecycle Source of truth: `standalone/sidecar/main.js`. diff --git a/scripts/spec-word-budgets.json b/scripts/spec-word-budgets.json index aa6c18163..3b7ac434b 100644 --- a/scripts/spec-word-budgets.json +++ b/scripts/spec-word-budgets.json @@ -26,7 +26,7 @@ "docs/specs/security-supply-chain.md": 1250, "docs/specs/security.md": 1900, "docs/specs/shortcuts.md": 1150, - "docs/specs/standalone.md": 10600, + "docs/specs/standalone.md": 10700, "docs/specs/terminal-context.md": 1050, "docs/specs/terminal-escapes.md": 3850, "docs/specs/terminal-state.md": 2400, diff --git a/standalone/sidecar/pty-core.js b/standalone/sidecar/pty-core.js index 94106ff76..707bebb2f 100644 --- a/standalone/sidecar/pty-core.js +++ b/standalone/sidecar/pty-core.js @@ -138,10 +138,50 @@ function withPrependedPath(env, dir, platform = process.platform) { function withoutInternalDormouseEnv(env) { const next = { ...env }; delete next.DORMOUSE_CLI_BIN; + delete next.DORMOUSE_GUI_NODE_DIR; delete next.DORMOUSE_SHELL_INTEGRATION_DIR; return next; } +// Compare Windows PATH entries: case-insensitive, `/` and `\` equivalent, and +// ignoring trailing separators and the quotes an entry may carry. +function normalizeWindowsPathEntry(entry) { + return String(entry || '') + .trim() + .replace(/^"(.*)"$/, '$1') + .replace(/\//g, '\\') + .replace(/\\+$/, '') + .toLowerCase(); +} + +// Win32 only. The bundled node.exe is patched to the GUI subsystem so spawning +// the sidecar never flashes a console window (docs/specs/standalone.md -> +// "Windows node subsystem"; build.rs `force_windows_gui_subsystem`). A +// GUI-subsystem process gets no console, so a *child* of it sees stdin already +// at EOF, has no +// `setRawMode`, and has every byte it writes dropped. `cargo run` puts the +// directory holding it — the dev app's `target/debug` — on the app's PATH so +// Windows resolves the app's DLLs, and panes inherit the app's env, leaving +// that node.exe to shadow the developer's own on a bare `node`. The failure is +// silent in both directions, so drop the directory here: the installed app +// never has it on PATH, and a pane needs nothing from it. +// +// Only the host knows which directory it patched, so it passes +// `DORMOUSE_GUI_NODE_DIR`. This module also runs under the VS Code pty host, +// where `process.execPath` is an unrelated node whose directory (`/usr/bin`, +// say) must stay on PATH — hence an explicit variable and not execPath. +function withoutGuiNodeDir(env, platform = process.platform) { + const dir = env.DORMOUSE_GUI_NODE_DIR; + if (platform !== 'win32' || !dir) return env; + const key = pathEnvKey(env); + const existing = env[key]; + if (!existing) return env; + const target = normalizeWindowsPathEntry(dir); + if (!target) return env; + const kept = existing.split(';').filter((entry) => normalizeWindowsPathEntry(entry) !== target); + return { ...env, [key]: kept.join(';') }; +} + // Win32 only. Git Bash / MSYS `/etc/profile` reconstructs PATH from an // exported `ORIGINAL_PATH` whenever that variable is already set, and only // captures the live PATH into it when it is unset. `ORIGINAL_PATH` leaks into @@ -295,7 +335,9 @@ function resolveSpawnConfig(options, runtime = {}) { // DORMOUSE_* vars are stripped below. const integrationDir = resolveShellIntegrationDir(env, runtime); const envWithCliPath = withoutInheritedMsysOriginalPath( - withoutInternalDormouseEnv(withPrependedPath(env, env.DORMOUSE_CLI_BIN, platform)), + withoutInternalDormouseEnv( + withPrependedPath(withoutGuiNodeDir(env, platform), env.DORMOUSE_CLI_BIN, platform), + ), platform, ); const childEnv = { diff --git a/standalone/sidecar/pty-core.test.js b/standalone/sidecar/pty-core.test.js index c10a7c538..9f11263e5 100644 --- a/standalone/sidecar/pty-core.test.js +++ b/standalone/sidecar/pty-core.test.js @@ -151,6 +151,85 @@ test('resolveSpawnConfig keeps ORIGINAL_PATH on non-win32', () => { assert.equal(config.env.ORIGINAL_PATH, '/whatever'); }); +test('resolveSpawnConfig drops the GUI node directory from a pane PATH on win32', () => { + // `cargo run` puts the dev app's target dir on PATH, and the node.exe there + // is patched to the GUI subsystem: a child of it gets no console at all. A + // pane that kept the directory would run that node on a bare `node`. + const config = resolveSpawnConfig( + { surfaceId: 'pane-1' }, + { + platform: 'win32', + env: { + Path: [ + 'C:\\repo\\standalone\\src-tauri\\target\\debug', + 'C:\\repo\\standalone\\src-tauri\\target\\debug\\deps', + 'C:\\Users\\tester\\nodejs', + 'C:\\Windows\\System32', + ].join(';'), + DORMOUSE_GUI_NODE_DIR: 'C:\\repo\\standalone\\src-tauri\\target\\debug', + DORMOUSE_CLI_BIN: 'C:\\Dormouse\\dor-cli\\bin', + }, + osModule: { + homedir: () => 'C:\\Users\\tester', + tmpdir: () => 'C:\\Temp', + }, + }, + ); + + assert.equal( + config.env.Path, + [ + 'C:\\Dormouse\\dor-cli\\bin', + // `deps` is a sibling, not the GUI node's directory — it stays. + 'C:\\repo\\standalone\\src-tauri\\target\\debug\\deps', + 'C:\\Users\\tester\\nodejs', + 'C:\\Windows\\System32', + ].join(';'), + ); + assert.equal(config.env.DORMOUSE_GUI_NODE_DIR, undefined); +}); + +test('resolveSpawnConfig matches the GUI node directory regardless of spelling', () => { + const config = resolveSpawnConfig( + { surfaceId: 'pane-1' }, + { + platform: 'win32', + env: { + Path: 'C:/Repo/Target/Debug\\;C:\\Windows\\System32', + DORMOUSE_GUI_NODE_DIR: 'C:\\repo\\target\\debug', + }, + osModule: { + homedir: () => 'C:\\Users\\tester', + tmpdir: () => 'C:\\Temp', + }, + }, + ); + + assert.equal(config.env.Path, 'C:\\Windows\\System32'); +}); + +test('resolveSpawnConfig keeps the GUI node directory on non-win32', () => { + // On macOS and Linux the app's own directory is never on PATH, and + // `DORMOUSE_GUI_NODE_DIR` could name a directory a pane needs (`/usr/bin`). + const config = resolveSpawnConfig( + { surfaceId: 'pane-1' }, + { + platform: 'linux', + env: { + PATH: '/usr/bin:/bin', + DORMOUSE_GUI_NODE_DIR: '/usr/bin', + }, + osModule: { + homedir: () => '/home/tester', + tmpdir: () => '/tmp/fallback', + }, + }, + ); + + assert.equal(config.env.PATH, '/usr/bin:/bin'); + assert.equal(config.env.DORMOUSE_GUI_NODE_DIR, undefined); +}); + test('withPrependedPath preserves Windows Path casing', () => { const env = withPrependedPath( { Path: 'C:\\Windows\\System32' }, diff --git a/standalone/src-tauri/src/lib.rs b/standalone/src-tauri/src/lib.rs index 5b7513656..5de848486 100644 --- a/standalone/src-tauri/src/lib.rs +++ b/standalone/src-tauri/src/lib.rs @@ -3974,6 +3974,15 @@ fn start_sidecar(app: &AppHandle) -> Result { let node_path = resolve_node_binary_path()?; let dor_cli_paths = resolve_dor_cli_paths(&sidecar_path, manifest_dir); let dor_node_path = resolve_dor_node_path(&node_path, app); + // The directory the GUI-subsystem node sits in, for the sidecar to drop + // from every pane's PATH — `cargo run` puts it there for DLL resolution + // and a bare `node` in a dev pane would otherwise find a node with no + // console. Only this side knows which binary was patched; the sidecar + // also runs under the VS Code pty host, where nothing is. + let gui_node_dir = node_path + .parent() + .map(|dir| dir.to_string_lossy().into_owned()) + .unwrap_or_default(); let dor_control_token = dor_control_token(); let state_dir = burrow_state_dir(app); let recovery_dir = recovery_state_dir(app); @@ -4004,6 +4013,7 @@ fn start_sidecar(app: &AppHandle) -> Result { c.arg(&sidecar_path) .env("DORMOUSE_HOST", "standalone") .env("DORMOUSE_NODE", &dor_node_path) + .env("DORMOUSE_GUI_NODE_DIR", &gui_node_dir) .env("DORMOUSE_CLI_BIN", &dor_cli_paths.bin_dir) .env("DORMOUSE_CLI_JS", &dor_cli_paths.entrypoint) .env("DORMOUSE_CONTROL_TOKEN", &dor_control_token) From 00ae4cb2a6589c36c713fdf8674e7977ff9099b2 Mon Sep 17 00:00:00 2001 From: Ned Date: Thu, 17 Sep 2026 15:50:40 -0700 Subject: [PATCH 2/5] Move the PATH evidence to the rationale, and pass the dir as a Path Per review: AGENTS.md routes measurements and dead approaches to `.rationale.md`, keyed by the spec heading and marked `(rationale)` at the rule. The PATH-position/subsystem table and the ruling-out of the installed app lived only in the PR body, where the next person to touch the strip would not find them; they now sit under `## Windows node subsystem` in `docs/specs/standalone.rationale.md`, along with why `process.execPath` is the wrong way to name the directory. `node_path` comes back from `resolve_node_binary_path` as `dir.join(...)`, so `parent()` is always `Some` and the `unwrap_or_default()` arm was dead. Passing the `&Path` straight to `Command::env` also drops a `to_string_lossy` round-trip that could have handed the sidecar a spelling no longer matching the real directory, silently turning the strip into a no-op. Co-Authored-By: Claude Opus 5 (1M context) --- docs/specs/standalone.md | 10 +++++----- docs/specs/standalone.rationale.md | 4 ++++ standalone/src-tauri/src/lib.rs | 5 +---- 3 files changed, 10 insertions(+), 9 deletions(-) diff --git a/docs/specs/standalone.md b/docs/specs/standalone.md index f8280f1eb..45402bbb4 100644 --- a/docs/specs/standalone.md +++ b/docs/specs/standalone.md @@ -238,11 +238,11 @@ because the sidecar and the `dor` CLI have opposite console requirements: that copy can never cause a stray window. - **Never leave the GUI node's directory on a pane's PATH.** A child of that - node.exe gets no console — stdin already at EOF, no `setRawMode`, output - dropped — so a bare `node` in a dev pane, where `cargo run` adds that - directory for DLL resolution, fails silently. `start_sidecar` passes it as - `DORMOUSE_GUI_NODE_DIR`; the sidecar drops it from the PATH each pane - inherits, matching the installed app, which never has it on PATH. + node.exe gets no console, so a bare `node` in a dev pane — where `cargo run` + puts that directory for DLL resolution — fails silently in both directions + (rationale). `start_sidecar` passes it as `DORMOUSE_GUI_NODE_DIR`; the sidecar + drops it from the PATH each pane inherits, matching the installed app, which + never has it on PATH. The byte-flip lives in `standalone/src-tauri/src/pe_subsystem.rs`, shared with `build.rs`, so the load-bearing PE offsets are in one place; the mechanism is in diff --git a/docs/specs/standalone.rationale.md b/docs/specs/standalone.rationale.md index 3142972ef..8df57e831 100644 --- a/docs/specs/standalone.rationale.md +++ b/docs/specs/standalone.rationale.md @@ -10,6 +10,10 @@ **The two Windows Node variants.** `CREATE_NO_WINDOW`, `DETACHED_PROCESS`, and `STARTUPINFO` hiding all failed to suppress Windows 11's DefTerm handoff from a GUI parent (verified 2026-08): Windows launches Windows Terminal to host the console-subsystem child, flashing a stray WT window behind Dormouse. Should a current spawn-time option suppress it, both variants collapse back to the stock console-subsystem Node under `DORMOUSE_NODE`. `dor`'s opposite requirement comes from running inside a shell's ConPTY, where its stdout/stderr are console handles rather than pipes. +**What the GUI node's directory on PATH cost, and why only dev builds.** Measured in a dev pane (Windows 11 25H2, 2026-09): `node` resolved to `standalone/src-tauri/target/debug/node.exe` at PATH position 4 — `isTTY` undefined, no `setRawMode`, PE subsystem 2 — ahead of `node_modules/.bin/node.EXE` at 8 and the developer's own `.version-fox/sdks/nodejs/node.exe` at 35, both console-subsystem with `isTTY` true. "Silent in both directions" is literal: stdin is at EOF so a reader dies at startup, and stdout is unattached so it cannot report why — writing the values to a file was the only way to observe them. The installed app was ruled out rather than assumed: Dormouse Terminal 0.11.0 ships the same GUI-subsystem `node.exe`, but neither its install directory nor any `target/debug` is in the persistent user or machine PATH a shortcut-launched app inherits, and panes add only `DORMOUSE_CLI_BIN` — so `node` there was already the developer's own, and the strip makes a dev pane match an installed one. + +**Why the host names the directory instead of the sidecar deriving it.** `path.dirname(process.execPath)` was the first attempt and is wrong twice over: `standalone/scripts/dev-agent-browser.mjs` runs the sidecar under the developer's own node, so the strip would have removed the node it exists to protect, and the same module is the VS Code pty host (`vscode-ext/src/pty-host.js`), where that directory can be `/usr/bin` — dropping it from a pane's PATH would be destructive. Hence an explicit `DORMOUSE_GUI_NODE_DIR` from the side that patched the binary, and a win32-only strip. + ## Boot sequence **Why the peer-surface responder must follow `init()`.** Installed first, its seeding `status` command sits unanswered with no retry — nothing carries the answer back until the adapter has registered its listeners. diff --git a/standalone/src-tauri/src/lib.rs b/standalone/src-tauri/src/lib.rs index 5de848486..52894c331 100644 --- a/standalone/src-tauri/src/lib.rs +++ b/standalone/src-tauri/src/lib.rs @@ -3979,10 +3979,7 @@ fn start_sidecar(app: &AppHandle) -> Result { // and a bare `node` in a dev pane would otherwise find a node with no // console. Only this side knows which binary was patched; the sidecar // also runs under the VS Code pty host, where nothing is. - let gui_node_dir = node_path - .parent() - .map(|dir| dir.to_string_lossy().into_owned()) - .unwrap_or_default(); + let gui_node_dir = node_path.parent().unwrap_or_else(|| Path::new("")); let dor_control_token = dor_control_token(); let state_dir = burrow_state_dir(app); let recovery_dir = recovery_state_dir(app); From 299d87a6c766fe10cefc1f5c24fc5fccc7ec18d3 Mon Sep 17 00:00:00 2001 From: Ned Date: Thu, 17 Sep 2026 16:00:52 -0700 Subject: [PATCH 3/5] Name the patched node without a build-artifact path spec-lint check 4 resolves any backticked repo-relative path, and the dev app's target dir is not in SKIP_PATH_PREFIXES -- so the rationale's `standalone/src-tauri/target/debug/node.exe` passed here, where the artifact exists, and failed CI on a clean checkout. Describe the binary by where it sits instead of spelling a path that only exists after a build; the skip list is for references the specs need, not for prose that can be reworded. Co-Authored-By: Claude Opus 5 (1M context) --- docs/specs/standalone.rationale.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/specs/standalone.rationale.md b/docs/specs/standalone.rationale.md index 8df57e831..dee5dcf84 100644 --- a/docs/specs/standalone.rationale.md +++ b/docs/specs/standalone.rationale.md @@ -10,7 +10,7 @@ **The two Windows Node variants.** `CREATE_NO_WINDOW`, `DETACHED_PROCESS`, and `STARTUPINFO` hiding all failed to suppress Windows 11's DefTerm handoff from a GUI parent (verified 2026-08): Windows launches Windows Terminal to host the console-subsystem child, flashing a stray WT window behind Dormouse. Should a current spawn-time option suppress it, both variants collapse back to the stock console-subsystem Node under `DORMOUSE_NODE`. `dor`'s opposite requirement comes from running inside a shell's ConPTY, where its stdout/stderr are console handles rather than pipes. -**What the GUI node's directory on PATH cost, and why only dev builds.** Measured in a dev pane (Windows 11 25H2, 2026-09): `node` resolved to `standalone/src-tauri/target/debug/node.exe` at PATH position 4 — `isTTY` undefined, no `setRawMode`, PE subsystem 2 — ahead of `node_modules/.bin/node.EXE` at 8 and the developer's own `.version-fox/sdks/nodejs/node.exe` at 35, both console-subsystem with `isTTY` true. "Silent in both directions" is literal: stdin is at EOF so a reader dies at startup, and stdout is unattached so it cannot report why — writing the values to a file was the only way to observe them. The installed app was ruled out rather than assumed: Dormouse Terminal 0.11.0 ships the same GUI-subsystem `node.exe`, but neither its install directory nor any `target/debug` is in the persistent user or machine PATH a shortcut-launched app inherits, and panes add only `DORMOUSE_CLI_BIN` — so `node` there was already the developer's own, and the strip makes a dev pane match an installed one. +**What the GUI node's directory on PATH cost, and why only dev builds.** Measured in a dev pane (Windows 11 25H2, 2026-09): `node` resolved to the patched `node.exe` in the dev app's `target/debug` at PATH position 4 — `isTTY` undefined, no `setRawMode`, PE subsystem 2 — ahead of pnpm's console-subsystem `node.EXE` at position 8 and the developer's own at 35, both reporting `isTTY` true. "Silent in both directions" is literal: stdin is at EOF so a reader dies at startup, and stdout is unattached so it cannot report why — writing the values to a file was the only way to observe them. The installed app was ruled out rather than assumed: Dormouse Terminal 0.11.0 ships the same GUI-subsystem `node.exe`, but neither its install directory nor any `target/debug` is in the persistent user or machine PATH a shortcut-launched app inherits, and panes add only `DORMOUSE_CLI_BIN` — so `node` there was already the developer's own, and the strip makes a dev pane match an installed one. **Why the host names the directory instead of the sidecar deriving it.** `path.dirname(process.execPath)` was the first attempt and is wrong twice over: `standalone/scripts/dev-agent-browser.mjs` runs the sidecar under the developer's own node, so the strip would have removed the node it exists to protect, and the same module is the VS Code pty host (`vscode-ext/src/pty-host.js`), where that directory can be `/usr/bin` — dropping it from a pane's PATH would be destructive. Hence an explicit `DORMOUSE_GUI_NODE_DIR` from the side that patched the binary, and a win32-only strip. From 66ef3909230933b73c2011a8455e736fc71fe89b Mon Sep 17 00:00:00 2001 From: Ned Date: Thu, 17 Sep 2026 16:28:25 -0700 Subject: [PATCH 4/5] Cut the PATH strip down to what it needs 145 added lines for dropping one PATH entry was too much. No behaviour change, and every case the tests pinned is still pinned. - Fold `normalizeWindowsPathEntry` into a local. The trim and quote-strip were dead: the only entry that has to match is the one `cargo run` appended, which is neither padded nor quoted. Case folding stays and is load-bearing, not just test-pinned -- cargo builds its entry from the workspace path as typed, while the host derives the directory from `current_exe()`, so the two can differ in case. - Collapse four guards to one. `!target` was unreachable given `dir` is truthy, and `!existing` folds into the first condition. - Cut the comment from 16 lines to 9. The measurements and the installed-app ruling-out are in the rationale now; what stays is the mechanism an editor of this module needs, and the one sentence that stops a future simplification from reaching for `process.execPath`. - Merge the two win32 tests. They were one rule plus two facts about matching: spelling the target entry with `/`, a trailing separator and a different case pins all three at once, next to the `debug\deps` sibling that pins no prefix matching. - Hoist `paneEnv` so the env chain keeps its original shape. Also corrects a reason I had wrong: the comment and the non-win32 test claimed `DORMOUSE_GUI_NODE_DIR` could name a directory a pane needs, like `/usr/bin`. Only `start_sidecar` ever sets it, always to the bundled node's directory -- that hazard belongs to the rejected `process.execPath` approach. The gate is still right, for the real reason: on Unix the app's directory is never on PATH. Co-Authored-By: Claude Opus 5 (1M context) --- docs/specs/standalone.md | 11 +++--- standalone/sidecar/pty-core.js | 53 +++++++++-------------------- standalone/sidecar/pty-core.test.js | 45 ++++++------------------ standalone/src-tauri/src/lib.rs | 9 ++--- 4 files changed, 35 insertions(+), 83 deletions(-) diff --git a/docs/specs/standalone.md b/docs/specs/standalone.md index 45402bbb4..c9a5a39cb 100644 --- a/docs/specs/standalone.md +++ b/docs/specs/standalone.md @@ -237,12 +237,11 @@ because the sidecar and the `dor` CLI have opposite console requirements: `DORMOUSE_NODE` at it. `dor` always runs inside an existing pseudo-console, so that copy can never cause a stray window. -- **Never leave the GUI node's directory on a pane's PATH.** A child of that - node.exe gets no console, so a bare `node` in a dev pane — where `cargo run` - puts that directory for DLL resolution — fails silently in both directions - (rationale). `start_sidecar` passes it as `DORMOUSE_GUI_NODE_DIR`; the sidecar - drops it from the PATH each pane inherits, matching the installed app, which - never has it on PATH. +- **Never leave the GUI node's directory on a pane's PATH.** `cargo run` puts it + there for DLL resolution, so a bare `node` in a dev pane would get a + console-less one and fail silently in both directions (rationale). + `start_sidecar` passes it as `DORMOUSE_GUI_NODE_DIR`; the sidecar strips it + from each pane's PATH. The byte-flip lives in `standalone/src-tauri/src/pe_subsystem.rs`, shared with `build.rs`, so the load-bearing PE offsets are in one place; the mechanism is in diff --git a/standalone/sidecar/pty-core.js b/standalone/sidecar/pty-core.js index 707bebb2f..23e9c2171 100644 --- a/standalone/sidecar/pty-core.js +++ b/standalone/sidecar/pty-core.js @@ -143,42 +143,22 @@ function withoutInternalDormouseEnv(env) { return next; } -// Compare Windows PATH entries: case-insensitive, `/` and `\` equivalent, and -// ignoring trailing separators and the quotes an entry may carry. -function normalizeWindowsPathEntry(entry) { - return String(entry || '') - .trim() - .replace(/^"(.*)"$/, '$1') - .replace(/\//g, '\\') - .replace(/\\+$/, '') - .toLowerCase(); -} - -// Win32 only. The bundled node.exe is patched to the GUI subsystem so spawning -// the sidecar never flashes a console window (docs/specs/standalone.md -> -// "Windows node subsystem"; build.rs `force_windows_gui_subsystem`). A -// GUI-subsystem process gets no console, so a *child* of it sees stdin already -// at EOF, has no -// `setRawMode`, and has every byte it writes dropped. `cargo run` puts the -// directory holding it — the dev app's `target/debug` — on the app's PATH so -// Windows resolves the app's DLLs, and panes inherit the app's env, leaving -// that node.exe to shadow the developer's own on a bare `node`. The failure is -// silent in both directions, so drop the directory here: the installed app -// never has it on PATH, and a pane needs nothing from it. -// -// Only the host knows which directory it patched, so it passes -// `DORMOUSE_GUI_NODE_DIR`. This module also runs under the VS Code pty host, -// where `process.execPath` is an unrelated node whose directory (`/usr/bin`, -// say) must stay on PATH — hence an explicit variable and not execPath. +// Win32 only. The bundled node.exe is patched to the GUI subsystem +// (docs/specs/standalone.md -> "Windows node subsystem"), and a child of a +// GUI-subsystem process gets no console at all: stdin at EOF, no `setRawMode`, +// output dropped. `cargo run` leaves that node.exe's directory on the dev +// app's PATH for DLL resolution and panes inherit the app's env, so a bare +// `node` in a pane would find it. Never derive the directory from +// `process.execPath` — this module is also the VS Code pty host, and the +// agent-browser harness runs the sidecar under the developer's node +// (rationale). function withoutGuiNodeDir(env, platform = process.platform) { - const dir = env.DORMOUSE_GUI_NODE_DIR; - if (platform !== 'win32' || !dir) return env; const key = pathEnvKey(env); - const existing = env[key]; - if (!existing) return env; - const target = normalizeWindowsPathEntry(dir); - if (!target) return env; - const kept = existing.split(';').filter((entry) => normalizeWindowsPathEntry(entry) !== target); + if (platform !== 'win32' || !env.DORMOUSE_GUI_NODE_DIR || !env[key]) return env; + // Entries compare case-insensitively, `/` as `\`, trailing separators ignored. + const norm = (entry) => String(entry).replace(/\//g, '\\').replace(/\\+$/, '').toLowerCase(); + const target = norm(env.DORMOUSE_GUI_NODE_DIR); + const kept = env[key].split(';').filter((entry) => norm(entry) !== target); return { ...env, [key]: kept.join(';') }; } @@ -334,10 +314,9 @@ function resolveSpawnConfig(options, runtime = {}) { // Resolve the integration dir from the original env before the internal // DORMOUSE_* vars are stripped below. const integrationDir = resolveShellIntegrationDir(env, runtime); + const paneEnv = withoutGuiNodeDir(env, platform); const envWithCliPath = withoutInheritedMsysOriginalPath( - withoutInternalDormouseEnv( - withPrependedPath(withoutGuiNodeDir(env, platform), env.DORMOUSE_CLI_BIN, platform), - ), + withoutInternalDormouseEnv(withPrependedPath(paneEnv, env.DORMOUSE_CLI_BIN, platform)), platform, ); const childEnv = { diff --git a/standalone/sidecar/pty-core.test.js b/standalone/sidecar/pty-core.test.js index 9f11263e5..fa7eedd01 100644 --- a/standalone/sidecar/pty-core.test.js +++ b/standalone/sidecar/pty-core.test.js @@ -153,20 +153,21 @@ test('resolveSpawnConfig keeps ORIGINAL_PATH on non-win32', () => { test('resolveSpawnConfig drops the GUI node directory from a pane PATH on win32', () => { // `cargo run` puts the dev app's target dir on PATH, and the node.exe there - // is patched to the GUI subsystem: a child of it gets no console at all. A - // pane that kept the directory would run that node on a bare `node`. + // is patched to the GUI subsystem: a child of it gets no console at all. The + // target entry is spelled with `/`, a trailing separator and a different + // case, pinning that matching ignores all three; `debug\deps` is a sibling, + // pinning that this is not prefix matching. const config = resolveSpawnConfig( { surfaceId: 'pane-1' }, { platform: 'win32', env: { Path: [ - 'C:\\repo\\standalone\\src-tauri\\target\\debug', - 'C:\\repo\\standalone\\src-tauri\\target\\debug\\deps', - 'C:\\Users\\tester\\nodejs', + 'C:/Repo/Target/Debug\\', + 'C:\\repo\\target\\debug\\deps', 'C:\\Windows\\System32', ].join(';'), - DORMOUSE_GUI_NODE_DIR: 'C:\\repo\\standalone\\src-tauri\\target\\debug', + DORMOUSE_GUI_NODE_DIR: 'C:\\repo\\target\\debug', DORMOUSE_CLI_BIN: 'C:\\Dormouse\\dor-cli\\bin', }, osModule: { @@ -178,39 +179,15 @@ test('resolveSpawnConfig drops the GUI node directory from a pane PATH on win32' assert.equal( config.env.Path, - [ - 'C:\\Dormouse\\dor-cli\\bin', - // `deps` is a sibling, not the GUI node's directory — it stays. - 'C:\\repo\\standalone\\src-tauri\\target\\debug\\deps', - 'C:\\Users\\tester\\nodejs', - 'C:\\Windows\\System32', - ].join(';'), + ['C:\\Dormouse\\dor-cli\\bin', 'C:\\repo\\target\\debug\\deps', 'C:\\Windows\\System32'].join(';'), ); assert.equal(config.env.DORMOUSE_GUI_NODE_DIR, undefined); }); -test('resolveSpawnConfig matches the GUI node directory regardless of spelling', () => { - const config = resolveSpawnConfig( - { surfaceId: 'pane-1' }, - { - platform: 'win32', - env: { - Path: 'C:/Repo/Target/Debug\\;C:\\Windows\\System32', - DORMOUSE_GUI_NODE_DIR: 'C:\\repo\\target\\debug', - }, - osModule: { - homedir: () => 'C:\\Users\\tester', - tmpdir: () => 'C:\\Temp', - }, - }, - ); - - assert.equal(config.env.Path, 'C:\\Windows\\System32'); -}); - test('resolveSpawnConfig keeps the GUI node directory on non-win32', () => { - // On macOS and Linux the app's own directory is never on PATH, and - // `DORMOUSE_GUI_NODE_DIR` could name a directory a pane needs (`/usr/bin`). + // Only the standalone host sets the variable, always to the bundled node's + // directory — but on macOS and Linux the app's directory is never on PATH, + // so the strip would be a no-op at best and is gated off. const config = resolveSpawnConfig( { surfaceId: 'pane-1' }, { diff --git a/standalone/src-tauri/src/lib.rs b/standalone/src-tauri/src/lib.rs index 52894c331..3f49aa08c 100644 --- a/standalone/src-tauri/src/lib.rs +++ b/standalone/src-tauri/src/lib.rs @@ -3974,12 +3974,9 @@ fn start_sidecar(app: &AppHandle) -> Result { let node_path = resolve_node_binary_path()?; let dor_cli_paths = resolve_dor_cli_paths(&sidecar_path, manifest_dir); let dor_node_path = resolve_dor_node_path(&node_path, app); - // The directory the GUI-subsystem node sits in, for the sidecar to drop - // from every pane's PATH — `cargo run` puts it there for DLL resolution - // and a bare `node` in a dev pane would otherwise find a node with no - // console. Only this side knows which binary was patched; the sidecar - // also runs under the VS Code pty host, where nothing is. - let gui_node_dir = node_path.parent().unwrap_or_else(|| Path::new("")); + // Our own directory, which holds the GUI-subsystem node. The sidecar drops + // it from every pane's PATH (standalone.md -> "Windows node subsystem"). + let gui_node_dir = node_path.parent().unwrap_or(Path::new("")); let dor_control_token = dor_control_token(); let state_dir = burrow_state_dir(app); let recovery_dir = recovery_state_dir(app); From 5142a7268df81288bbed303a94a86815c2637218 Mon Sep 17 00:00:00 2001 From: Ned Date: Thu, 17 Sep 2026 16:35:30 -0700 Subject: [PATCH 5/5] Name the right mechanism: inherited console, not parentage Per review, the rewritten comment said a *child* of a GUI-subsystem process gets no console. That is wrong, and this branch's own measurements disprove it: the pane's shell is exactly such a child -- the sidecar runs under the GUI node and node-pty hands the shell a ConPTY -- and it works, as does the console-subsystem node.exe from `node_modules/.bin`, which reported `isTTY` true from inside the same pane. What fails is the patched binary itself: a GUI-subsystem binary does not attach to an *inherited* console, which is what `resolve_dor_node_path` and this section's `dor` bullet already say. Read the old wording literally and the strip looks removable by spawning panes from a console-subsystem process, which would not help at all. The spec bullet and both rationale paragraphs already stated it correctly; only the code comment and the win32 test's comment needed the edit. Co-Authored-By: Claude Opus 5 (1M context) --- standalone/sidecar/pty-core.js | 16 ++++++++-------- standalone/sidecar/pty-core.test.js | 8 ++++---- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/standalone/sidecar/pty-core.js b/standalone/sidecar/pty-core.js index 23e9c2171..b859a9b42 100644 --- a/standalone/sidecar/pty-core.js +++ b/standalone/sidecar/pty-core.js @@ -144,14 +144,14 @@ function withoutInternalDormouseEnv(env) { } // Win32 only. The bundled node.exe is patched to the GUI subsystem -// (docs/specs/standalone.md -> "Windows node subsystem"), and a child of a -// GUI-subsystem process gets no console at all: stdin at EOF, no `setRawMode`, -// output dropped. `cargo run` leaves that node.exe's directory on the dev -// app's PATH for DLL resolution and panes inherit the app's env, so a bare -// `node` in a pane would find it. Never derive the directory from -// `process.execPath` — this module is also the VS Code pty host, and the -// agent-browser harness runs the sidecar under the developer's node -// (rationale). +// (docs/specs/standalone.md -> "Windows node subsystem"), and a GUI-subsystem +// binary does not attach to an inherited console: run it inside a pane's +// ConPTY and stdin is already at EOF, there is no `setRawMode`, and output is +// dropped. `cargo run` leaves that node.exe's directory on the dev app's PATH +// for DLL resolution and panes inherit the app's env, so a bare `node` in a +// pane would find it. Never derive the directory from `process.execPath` — +// this module is also the VS Code pty host, and the agent-browser harness runs +// the sidecar under the developer's node (rationale). function withoutGuiNodeDir(env, platform = process.platform) { const key = pathEnvKey(env); if (platform !== 'win32' || !env.DORMOUSE_GUI_NODE_DIR || !env[key]) return env; diff --git a/standalone/sidecar/pty-core.test.js b/standalone/sidecar/pty-core.test.js index fa7eedd01..2857e6f48 100644 --- a/standalone/sidecar/pty-core.test.js +++ b/standalone/sidecar/pty-core.test.js @@ -153,10 +153,10 @@ test('resolveSpawnConfig keeps ORIGINAL_PATH on non-win32', () => { test('resolveSpawnConfig drops the GUI node directory from a pane PATH on win32', () => { // `cargo run` puts the dev app's target dir on PATH, and the node.exe there - // is patched to the GUI subsystem: a child of it gets no console at all. The - // target entry is spelled with `/`, a trailing separator and a different - // case, pinning that matching ignores all three; `debug\deps` is a sibling, - // pinning that this is not prefix matching. + // is patched to the GUI subsystem, so it never attaches to the pane's + // inherited console. The target entry is spelled with `/`, a trailing + // separator and a different case, pinning that matching ignores all three; + // `debug\deps` is a sibling, pinning that this is not prefix matching. const config = resolveSpawnConfig( { surfaceId: 'pane-1' }, {