fix: a Windows-path-aware commandArgv0 - #516
Conversation
`tokenizeCommand` treats `\` as a POSIX escape, so a Windows absolute path loses its separators before `commandArgv0` can take a basename: WATCHING rules never match a command invoked by full path, pane headers render `C:Program ...`, and `dor tool`'s take-over gate (PR #514) fails closed on `C:\bin\dor.cmd tool x`. Off main rather than the Dor Tools stack — the bug predates it, and the fix serves three consumers. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0166PG9g7V3kZ6Uo9EHrpoTD
…kenizer `tokenizeCommand` treated every `\` outside single quotes as a POSIX escape, so a command line reported by PowerShell or cmd lost its path separators before `commandArgv0`'s basename split could see them: `C:\tools\dor.cmd` keyed as `C:toolsdor.cmd`. WATCHING rules stored under the bare program name never matched, and pane headers rendered the mangled text. A backslash now escapes only what a shell actually escapes — whitespace, a quote, another backslash, a glob/metacharacter — and is a literal separator otherwise, so `foo\ bar` and `\*.ts` keep their meaning. cmd.exe's unquoted program path with spaces (`C:\Program Files\nodejs\npm.cmd run dev`) is re-joined afterwards, but only when the join lands on an executable suffix. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KzMCg7c37NRvdLEhpwmPpz
…ll's `&` /simplify follow-ups to the tokenizer fix: - The tokenizer's escapable set was a second copy of `shell-escape.ts`'s `POSIX_UNSAFE`, missing `~` — so a path Dormouse itself escaped for paste (`\~/my\ app`) no longer read back as itself. Export the class as `POSIX_ESCAPABLE` and unescape exactly what it escapes; a table test pins the round trip character by character. - `& "C:\Program Files\nodejs\npm.cmd" run dev` — the only way PowerShell runs a quoted program path — yielded `null` from `commandArgv0`, because a bare `&` at index 0 read as a boundary that left no command. Drop a leading `&`; it is never a POSIX background suffix. - Dedupe: one `commandBasename`, one `WINDOWS_DRIVE_PREFIX`, one `WINDOWS_EXECUTABLE_SUFFIX` in place of three, two, and two copies. - Tighten the tests that pinned nothing, and cut the spec addition to the one cross-file rule (the shared escape set) it needs to state. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KzMCg7c37NRvdLEhpwmPpz
…her suffixes /code-review found `joinWindowsProgramPath` returning the *wrong* argv0, not just a miss: `"C:\Program Files\Git\bin\bash" scripts\bootstrap.cmd` reduced to `bootstrap.cmd`, because the tokenizer discards quoting and the join could not tell a complete program path from one continued by an argument. Unquoted is no better — `C:\tools\node .\scripts\build.cmd` gave `build.cmd`. `A\B C\D.cmd` is genuinely `A\B` plus an argument as often as it is one path, and only the filesystem can say which; cmd.exe resolves it by probing, we cannot. It also missed every shape it was supposed to catch (`Program Files (x86)\Microsoft VS Code`, extension-less `Program Files\nodejs\node`, forward-slash drive paths). An unquoted Windows path with spaces now stays split — a miss, never a wrong program name. The quoted form PowerShell requires works from the escape fix alone. Also from the review: - `.cmd`/`.exe` is how the same program spells itself on Windows, so strip it before matching in `commandTitleTokens`. This PR is what makes those the normal Windows basename, and every per-program case is keyed on a bare name, so `vim.exe f` was rendering `vim.exe f` and `npm.cmd run dev` was missing the npm case. - WATCHING keys written by the old mangling (`C:toolsclaude.exe`) can never match again. `normalize` now drops any key holding a separator or `:` — no basename ever does — and both the stored copy and the host snapshot go through it. - `\s` in the escape test was redundant with `POSIX_ESCAPABLE`'s own space and tab, and made the shared-set claim untrue. - `commandBasename`'s `?? command` was unreachable; use a regex with no dead branch. Note the POSIX fidelity cost (`grep \-v` keeps its backslash) beside the Windows one. - The round-trip test asserted a tautology. It now pins the half that matters: no path character is in the escapable set. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KzMCg7c37NRvdLEhpwmPpz
Deploying mouseterm with
|
| Latest commit: |
af7a7b2
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://44018c8e.mouseterm.pages.dev |
| Branch Preview URL: | https://windows-argv0.mouseterm.pages.dev |
CI caught it: `terminal-state.ts` is bundled into the VS Code extension host and the Tauri sidecar, neither of which resolves the `dor/*` path that `shell-escape.ts` imports. Importing `POSIX_ESCAPABLE` from there dragged the `dor` CLI package into that graph and broke `message-router.test.ts` with `Cannot find package 'dor/commands/shell-quote'`. Give the set its own dependency-free module. It was always a contract between two modules rather than a detail of either — the escaper writes the characters, the tokenizer reads them back — so the shared home also states that plainly. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KzMCg7c37NRvdLEhpwmPpz
dormouse-bot
left a comment
There was a problem hiding this comment.
The tokenizer change reads correctly and I reproduced every row of the dialect table by executing the extracted tokenizeCommand / takePrimaryCommandTokens / commandBasename under node — all twelve pinned cases hold, including the two undecidable-path misses. 714da04d is the right fix for the Build & Test red on ea36545 (Cannot find package 'dor/commands/shell-quote' … ❯ terminal-state.ts:1:1, five vscode-ext/test/message-router.test.ts cases); POSIX_ESCAPABLE now has exactly one definition and three importers, all repointed.
One thing beyond the two inline notes:
The suffix strip lands on the display name but not on the key, and this PR is what splits them. commandTitleTokens strips .exe/.cmd, so C:\tools\claude.exe --print summarizes as claude --print, while commandArgv0 returns claude.exe. Before this PR the two agreed (both mangled to C:toolsclaude.exe); now the pane header reads claude, WatchedCommandList renders the rule row as claude.exe, and the bell tooltip says [a] Alert on all "claude.exe" — three names for one program on one screen. A rule created from a .exe/.cmd invocation is also a second row that never rings for the same program typed bare, which is the miss the PR opens on, one dialect over. The argument in the body — ".cmd/.exe is how the same program spells itself on Windows" — applies unchanged to the key.
Keeping them distinct is defensible (foo.bat and foo.exe really can be two files in one directory, and the rule set is the one place conflating them is irreversible from the UI). But it is currently pinned by a test and stated nowhere, so if it is the intended trade it is worth a line in alert.md or on commandArgv0 — that doc comment already says "This is the key WATCHING rules are stored under", which is exactly where a reader would look for it.
Smaller notes, not worth blocking on
setCommandWatcheddoesn't run its argument throughisKeyableName, so a key thatnormalizewill later drop can still be stored:commandArgv0('C:foo.exe')(a legal drive-relative invocation) returnsC:foo.exe, which matches in-session and then vanishes on the next reload with no feedback. Narrow enough to leave.- In VS Code the extension host keeps its own stale key and rebroadcasts it; the renderer filters it every time, so the state is stable but never actually cleaned up on the authoritative side.
- The character list in the round-trip
it.eachis a hand-maintained copy ofPOSIX_ESCAPABLE's contents. It pins that each listed character is in the set, but a character added to the regex later wouldn't fail anything — the companion path-character test only covers a fixed allowlist. posix-escape.tssaysterminal-state.tsis bundled into the VS Code extension host and the Tauri sidecar. What the red run actually showed is thevscode-extvitest process resolving the specifier without the tsconfigdor/*mapping; the sidecar half isn't demonstrated by anything I can see. The rule the comment states is right either way.
…d tooltip dormouse-bot on #516: stripping `.cmd`/`.exe` for display while `commandArgv0` keeps it split one program into three names on one screen — header `claude`, WATCHING rule row `claude.exe`, bell tooltip `Alert on all "claude.exe"`. This PR is what split them; before it, both sides mangled identically. Strip the suffix for *matching* the per-program cases only — that is what makes them fire on Windows at all — and render the basename as invoked. The key keeps the suffix, which `commandArgv0`'s doc comment now says, since that is where a reader looks: `foo.bat` and `foo.exe` really can be two files in one directory, and the rule set is the one place conflating them cannot be undone from the UI. Also from that review: - The escape-set comment claimed both costs were display-only. The first is not: when the metacharacter-initial segment is the *last* one, the eaten separator is the one the basename split needed (`C:\tools\$claude.exe` -> `tools$claude.exe`). Verified and corrected. - `posix-escape.ts` claimed the Tauri sidecar as a consumer; the sidecar mirrors constants by hand and imports nothing from lib. Narrowed to what is true. - The spec pointer still named the file `POSIX_ESCAPABLE` moved out of. - `setCommandWatched` now applies the same `isKeyableName` gate `normalize` does, so a key that would vanish on the next reload is never stored. - The round-trip test only proved each listed character was in the set. It now pins the set both ways, so adding a member fails here too. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KzMCg7c37NRvdLEhpwmPpz
|
Thanks — all five landed in 4a47693, plus the main point. One name per program. You're right that this PR is what split them, and the argument for stripping applies to the display, not to the key. So: strip for matching the per-program cases (which is what makes them fire on Windows at all), render the basename as invoked. The key keeps the suffix for the reason you gave — The escape-set cost. I had it backwards and you're right. Against this branch: The last one is display-only only because the damage sits upstream of the final separator, exactly as you said. Comment corrected. Smaller notes. I left the VS Code host-side stale key alone: the host is seeded from the renderer's already-normalized list on each host start, so the authoritative side gets the filtered set rather than keeping a stale one. Root suite green — lib 1996, vscode-ext 113, standalone 62, website 65, and all five lints. |
dormouse-bot on #516: `normalize` dropping an unkeyable key was covered, but deleting the `isKeyableName` guard from `setCommandWatched` left the file green — and that guard is the one added because the bug was reachable (`commandArgv0('C:foo.exe --print')` returns `C:foo.exe`, which looks keyable until the next reload). Confirmed the added line is load-bearing: removing the guard turns it red. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KzMCg7c37NRvdLEhpwmPpz
`npm.cmd`, `C:\tools\claude.exe` and `build.ps1` now reduce to `npm`, `claude` and `build` — for the WATCHING key, the pane header, and the bell tooltip alike. Keeping the suffix left `npm` and `npm.cmd` as two rules for one program, which is the miss this branch exists to close: rules are only ever created from the running command's own argv0, so a rule made from a PATH invocation never matched the same program launched by full path. PATHEXT means the user thinks of it as one program either way. Since `commandTitleTokens` already matched on the stripped form and rendered the unstripped one, folding both onto one `commandProgramName` deletes that split rather than adding a rule. `isGenericProcessTitle` keeps the plain basename — there the suffix is the evidence it tests for. Accepted: `foo.bat` and `foo.exe` in one directory cannot be watched separately. No migration cost — no shipped build ever stored a suffixed key, since POSIX stored bare names and Windows stored the mangled form this branch already drops. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0166PG9g7V3kZ6Uo9EHrpoTD
dormouse-bot
left a comment
There was a problem hiding this comment.
Re-ran the extracted tokenizeCommand / takePrimaryCommandTokens / commandProgramName under node against every row of the dialect table plus the new suffix cases — all hold, including & C:\tools\dor.cmd → dor and the two undecidable-path misses. Folding commandTitleTokens' match-vs-render split onto one commandProgramName is the right shape: the header, the rule row and the tooltip now provably read the same name, and the it.each table pins commandArgv0 and summarizeCommandLine on the same row, so they can't drift apart silently. pnpm lint:specs is green on the merged tree.
Two things on the reversal.
The stale-key sweep only reaches drive-absolute invocations — inline on isKeyableName. Relative-path invocations (tools\dor.cmd, .\build.ps1) mangled to keys with no : and no separator, so they pass the gate and are now permanently dead rows; and npm.cmd typed without a path never went through the mangling at all, which is the case 0698588's "no shipped build ever stored a suffixed key" doesn't cover.
alert.md still specifies the old key. The WATCHING rules list — "The key is commandArgv0(rawCommandLine) … then reduce argv[0] to its basename" — gives only POSIX examples, so a reader computing the key for npm.cmd from that line gets npm.cmd. terminal-state.md states the new rule, but alert.md is where the key itself is specified, and it now contradicts the code. It's outside the diff so I haven't touched it; happy to push the one-line amendment if you want it.
Minor: commandTitleTokens names its local basename while it now holds commandProgramName(command), three lines below a commandBasename that means the other thing. program would keep the distinction this PR is about.
`isKeyableName` only caught a separator or a `:`, which the old mangling left behind for a drive-absolute path alone. A relative invocation had no separator to eat (`tools\dor.cmd` -> `toolsdor.cmd`, `.\build.ps1` -> `.build.ps1`), and a bare launcher stored cleanly as `npm.cmd` — all three match nothing now that `commandProgramName` strips the suffix, and all three sat in the rule list looking real. One tell covers the class: `commandArgv0` can no longer return a name ending in a launcher suffix, so a stored key that does is dead. Residual, now stated at the gate: a mangled relative path with no suffix (`bin\claude` -> `binclaude`) reads exactly like a program named that and survives for the user to delete. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0166PG9g7V3kZ6Uo9EHrpoTD
alert.md owns the key spec and still said "reduce argv[0] to its basename" with POSIX-only examples, so a reader computing the key for `npm.cmd` from the line that defines it got `npm.cmd` — the exact shape `isKeyableName` now deletes. A pointer, not a restatement: terminal-state.md holds the rule and its examples. `commandTitleTokens`'s local is `program`, not `basename` — it holds `commandProgramName`, three lines under a `commandBasename` that means the other thing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0166PG9g7V3kZ6Uo9EHrpoTD
dormouse-bot
left a comment
There was a problem hiding this comment.
Both fixes from the last round land. alert.md's key line now points at terminal-state.md rather than restating the rule, and the local is program. The stale-key gate I confirmed on 72f01c1e is unchanged here, and pnpm lint:specs is green on the merged tree — alert.md at 7598 of its 7600-word budget, which is presumably why the Windows example went to the pointer rather than the line.
One new thing, from the gate the increment added — inline on isKeyableName.
| * | ||
| * Residual: a mangled *relative* path with no suffix (`bin\claude` -> | ||
| * `binclaude`) is indistinguishable from a program actually named that, and | ||
| * survives. The user deletes it from the rule list. |
There was a problem hiding this comment.
commandProgramName strips one suffix; this gate rejects any name ending in one, so the two are not inverses and the sentence above ("it never ends in a launcher suffix") is false for a doubled extension:
commandProgramName('a.exe.exe') -> 'a.exe'
isKeyableName('a.exe') -> false
(run against the two functions as written on this branch). That is a live argv0, not a stale key, and the failure mode is the one the write-path gate was added to prevent, one step worse: TodoAlertDialog renders Alert on all "a.exe", setCommandWatched early-returns, and on={watched.includes(argv0)} never flips — the switch is a permanent silent no-op rather than a rule that vanishes on reload.
It isn't fixable, which is why I'd state it rather than patch it: a stored npm.cmd is genuinely ambiguous between the stale key this gate exists to drop and the argv0 of npm.cmd.cmd, so any predicate that drops the first refuses the second. Same shape as the binclaude residual already documented here, so it reads naturally as the second one:
| * | |
| * Residual: a mangled *relative* path with no suffix (`bin\claude` -> | |
| * `binclaude`) is indistinguishable from a program actually named that, and | |
| * survives. The user deletes it from the rule list. | |
| * | |
| * Two residuals, both undecidable rather than fixable. A mangled *relative* | |
| * path with no suffix (`bin\claude` -> `binclaude`) reads exactly like a | |
| * program named that, so it survives for the user to delete. And a program | |
| * whose name genuinely ends in a suffix (`a.exe.exe`, argv0 `a.exe` — the | |
| * strip takes one) is refused rather than stored, because its key cannot be | |
| * told apart from the stale `npm.cmd` this gate drops. |
Rare either way — but the doc comment currently claims the case can't arise, and it's the claim a later reader would rely on.
dormouse-bot
left a comment
There was a problem hiding this comment.
Re-ran the extracted tokenizer / commandProgramName / isKeyableName under node against the increment. The drive-filter narrowing is right, and it recovers live keys the broad : test was deleting — ./foo:bar -> foo:bar and C:\Users\me\a.b:c -> a.b:c are keyable now, while every row of the stale-key table still drops on both the read and write paths (C:toolsclaude.exe, toolsdor.cmd, .build.ps1, ..binnode.exe, npm.cmd). The new foo\-bar -> -bar pin holds, and afterEach's clearRules still empties the store after the added foo:bar rows, since both are keyable and therefore removable. pnpm lint:specs green on the merged tree.
One inline finding, on the tokenizer comment the increment rewrote.
Two more for the still-open isKeyableName thread, so its edit lands complete instead of needing a third round:
WINDOWS_EXECUTABLE_SUFFIX's own doc comment carries the same claim — "which drops a stored key ending in one:commandArgv0cannot produce one" — so the suggestion onisKeyableNameleaves it false interminal-state.ts. (The PR body's stale-keys bullet says it a third time: "none of whichcommandArgv0can return".)- The drive clause has the same shape as the suffix clause.
commandArgv0('C:foo.exe --print')returnsC:foo, which the gate refuses, so the bell dialog readsAlert on all "C:foo"and the switch is a permanent no-op rather than a rule that vanishes on reload.watched-commands.test.tsalready names that shape ("a drive-relative invocation is the one shapecommandArgv0can still return with a:in it"), so the behavior is the intended trade — it is "a stored key is onecommandArgv0can still produce" that overclaims, in a second dimension.
| * `-bar`), while an eaten Windows separator leaves `C:\tools\$claude.exe` | ||
| * keyed as `tools$claude.exe`. |
There was a problem hiding this comment.
Verified against this branch, the key is tools$claude — commandProgramName strips the .exe after the mangled basename split, so the example predates the suffix strip landing in this PR:
C:\tools\$claude.exe --print -> tools$claude
The corrected value also flips the consequence. tools$claude.exe would be dropped by isKeyableName (launcher suffix), so the comment as written reads like the cost is self-cleaning; tools$claude passes all three clauses, so it persists as a second rule row for the program a bare claude keys as claude.
| * `-bar`), while an eaten Windows separator leaves `C:\tools\$claude.exe` | |
| * keyed as `tools$claude.exe`. | |
| * `-bar`), while an eaten Windows separator leaves `C:\tools\$claude.exe` | |
| * keyed as `tools$claude` — a name `isKeyableName` accepts, so it persists as | |
| * a second rule row for the program a bare invocation keys as `claude`. |
tokenizeCommandtreated every\outside single quotes as a POSIX escape, soa command line reported by PowerShell or cmd lost its path separators before
commandArgv0's basename split could see them. Everything keyed on the programname silently missed when the user (or a launcher, or a shim) invoked by
absolute path:
C:\Users\me\.claude\local\claudeC:Usersme.claudelocalclaudeclaudeC:\tools\dor.cmd tool storybookC:toolsdor.cmddor& "C:\Program Files\nodejs\npm.cmd" run devnullnpmRules are only ever created from the running command's own argv0 (the bell
dialog toggle — nobody types a name), so a mangled key was self-consistent and
did match the same line again. What missed is the same program reached two ways:
a rule made while running
claudefrom PATH never rang for a Session launchedas
C:\Users\me\.claude\local\claude, and the&form had no argv0 at all,so there was no name to key a rule on. Pane headers and the TODO dialog rendered
the mangled text throughout. The
dor tooltake-over gate in #514 has the same miss and picksthis up for free — one tokenizer, so the gate and the header can never disagree
about a command line.
The fix
A
\escapes only what a shell actually escapes; before anything else it is aliteral separator. The escapable set is
shellEscapePosix's ownPOSIX_ESCAPABLE, shared rather than re-derived, so POSIX escapes keep theirmeaning (
foo\ baris one token,\*.tspasses a literal glob) and a pathDormouse itself escaped for a drag-and-drop paste reads back as itself — the
two sets had silently disagreed about
~.Three consequences worth calling out:
&call operator.& "C:\...\npm.cmd" run dev— the onlyway that shell runs a quoted program path — returned
null, because a bare&at index 0 read as a boundary leaving no command. A leading&is nevera POSIX background suffix, so it is dropped.
.cmd/.exeis how the same program spells itself on Windows, so thesuffix is not part of its name:
npm.cmd,C:\tools\claude.exeandbuild.ps1reduce tonpm,claudeandbuildfor the WATCHING key, theheader, and the bell tooltip alike (
commandProgramName). Keeping it wouldleave
npmandnpm.cmdas two rules for one program, which is the missthis PR exists to close. Accepted:
foo.batandfoo.exein one directorycannot be watched separately.
isGenericProcessTitlekeeps the plainbasename — there the suffix is the evidence it tests for.
never match again and cannot be migrated: a key the old mangling wrote
(
C:\tools\claude.exe->C:toolsclaude.exe,tools\dor.cmd->toolsdor.cmd), and any key ending in a launcher suffix, including one thatstored cleanly (
npm.cmd), now thatcommandProgramNamestrips it.isKeyableNamedrops both — a separator, a leadingX:drive prefix, or alauncher suffix, none of which
commandArgv0can return. The drive prefix isnarrow on purpose: a bare
:is legal in a POSIX basename, and rejectingevery colon-bearing name would silently no-op the bell toggle for one. A Windows user whose rule was working
loses it on upgrade and re-enables it once from the bell dialog, now keyed on
a real program name. Residual: a mangled relative path with no suffix
(
bin\claude->binclaude) reads exactly like a program named that, so itsurvives as a dead row to delete by hand.
Not delivered: the unquoted Windows path with spaces
HANDOFF.mdwantedC:\Program Files\nodejs\npm.cmd run devto reduce tonpm.cmd. I built that (re-join the leading tokens, commit on an executablesuffix) and then removed it, because review showed it returning the wrong
argv0 rather than a miss:
The first is the normal PowerShell idiom, and it broke because the tokenizer
discards quoting, so the join cannot tell a complete program path from one
continued by an argument. Teaching it to track quotes would not save the second:
A\B C\D.cmdis genuinelyA\Bplus an argument as often as it is one path,and nothing short of the filesystem can say which. It also missed most of what
it was meant to catch (
Program Files (x86)\Microsoft VS Code, extension-lessProgram Files\nodejs\node, forward-slash drive paths).Worth saying plainly: the brief's wanted value for that row looks wrong. The
unquoted form is a parse error in PowerShell — which is the only Windows shell
with real OSC 633 integration — so it could only ever arrive from cmd.exe, which
has no per-command hook and reaches us through the keystroke fallback. Whether
cmd's own parser accepts it I have not verified;
CreateProcessdocumentsprefix-probing when
lpApplicationNameis NULL, but that is the API, not theshell. Either way the shape stays split: a miss, never a wrong program name —
the same trade #514 accepted. The quoted form PowerShell requires works from the
escape fix alone.
Verification
pnpm lint:specsand the fulllibsuite (1996 tests) pass;tsc --noEmitisclean. The new
command tokenizer dialectstable pins both dialects, the POSIXescapes as regression pins, and — character by character — that
shellEscapePosixoutput round-trips and that no path character is in theescapable set. jsdom covers the parsing; the payoff is on Windows, where a real
check is a PowerShell pane running an absolute-path command and confirming the
header shows the program name and a WATCHING rule on that name rings.
🤖 Generated with Claude Code
https://claude.ai/code/session_01KzMCg7c37NRvdLEhpwmPpz