fix(dev): make pnpm dev:app:win work on default Git for Windows installs - #5342
fix(dev): make pnpm dev:app:win work on default Git for Windows installs#5342Mustaqeem66 wants to merge 10 commits into
pnpm dev:app:win work on default Git for Windows installs#5342Conversation
pnpm runs package.json scripts through `cmd.exe /d /s /c`, which strips
the first and last quote characters from the command string. The
`dev:app:win` body quoted an absolute bash.exe path containing a space,
so cmd parsed `C:/Program` as the program name and the script failed on
every default Git for Windows install:
'C:/Program' is not recognized as an internal or external command
Quoting works normally inside a .cmd file, so route the entry point
through a new scripts/run-dev-win.cmd. package.json now references a
relative, space-free path that needs no quoting, and the wrapper quotes
the bash.exe path itself. This mirrors the .bat shim run-dev-win.sh
already generates internally for cargo-tauri's beforeDevCommand.
The wrapper probes the standard Git for Windows locations plus the
parent of git.exe on PATH, and honours OPENHUMAN_BASH_EXE for portable
installs. It deliberately avoids a bare `bash` lookup, which resolves to
the WSL launcher in System32 when WSL is enabled.
Closes tinyhumansai#5270
An unescaped `)` inside `%ProgramFiles(x86)%` in the diagnostic echo would have closed the enclosing `if (...)` block early, so the guidance never printed correctly. Jump to a label instead, where parentheses are literal. Refs tinyhumansai#5270
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe Windows development command now invokes a ChangesWindows launcher flow
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Poem
Sequence Diagram(s)sequenceDiagram
participant pnpm
participant run-dev-win.cmd
participant GitBash
participant run-dev-win.sh
pnpm->>run-dev-win.cmd: Execute dev:app:win
run-dev-win.cmd->>run-dev-win.cmd: Discover BASH_EXE
run-dev-win.cmd->>GitBash: Invoke run-dev-win.sh with arguments
GitBash->>run-dev-win.sh: Execute shell launcher
run-dev-win.sh-->>run-dev-win.cmd: Return exit status
run-dev-win.cmd-->>pnpm: Preserve exit status
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/run-dev-win.cmd`:
- Line 1: Convert scripts/run-dev-win.cmd to CRLF line endings so cmd.exe
correctly handles CALL :use_if_exists and labels, and add an eol=crlf repository
rule if no existing rule enforces it.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: d8baaff7-85f9-47e8-9f62-85863ef23db1
📒 Files selected for processing (3)
app/package.jsonapp/test/dev-app-win-launcher.test.tsscripts/run-dev-win.cmd
|
| Filename | Overview |
|---|---|
| .gitattributes | Adds *.cmd and *.bat CRLF checkout rules, consistent with the existing *.ps1 exception. Correct and uncontroversial. |
| app/package.json | Replaces the hard-coded, space-containing bash.exe path (broken by cmd.exe /S /C quote-stripping) with a relative, space-free .cmd wrapper path. Single-line change; correct. |
| scripts/run-dev-win.cmd | New batch wrapper that locates bash.exe via four ordered probes (fixed paths, scoop, git-on-PATH), quotes it correctly, and propagates exit codes. Written without labels or jumps to be line-ending agnostic. Well-structured and safe. |
| app/test/dev-app-win-launcher.test.ts | Five-case test suite covering the cmd.exe /S quoting regression, the new script parse-safety, file existence, no-bare-bash, and no-label-flow invariants. Minor gap: launcherCodeLines filter misses @rem comment lines. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["pnpm dev:app:win"] -->|"cmd.exe /d /S /c ..\\scripts\\run-dev-win.cmd"| B["run-dev-win.cmd"]
B --> C{"OPENHUMAN_BASH_EXE set and exists?"}
C -->|Yes| G["Run bash.exe with run-dev-win.sh and forward args"]
C -->|No| D{"ProgramFiles Git bin or x86 or LOCALAPPDATA?"}
D -->|Exists| G
D -->|No| E{"Scoop apps/git/current/bin in user, global, or relocated root?"}
E -->|Exists| G
E -->|No| F{"git.exe on PATH - derive parent bin/bash.exe?"}
F -->|Exists| G
F -->|No| H["Print diagnostic to stderr and exit /b 1"]
G --> I["exit /b ERRORLEVEL"]
Reviews (5): Last reviewed commit: "test: detect @-prefixed and colon-joined..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Collapse call expressions that fit within the repo's printWidth of 100 and replace the multi-line concatenation with straight-line statements, so `pnpm --filter openhuman-app format:check` is stable. Refs tinyhumansai#5270
The repository normalises everything to LF via `* text=auto eol=lf`, and `*.ps1` is the only Windows exception. cmd.exe's parser is line-ending sensitive -- LF-only batch files can mis-parse `goto`/`call :label` because of the 512-byte read-boundary bug -- so batch scripts need the same treatment PowerShell already gets. Refs tinyhumansai#5270
Deriving the Git root from `where git.exe` does not work under scoop: PATH only ever contains the shim directory, so `<shim dir>\..\bin\bash.exe` resolves to `scoop\bin\bash.exe`, which does not exist. The real binary lives under `scoop\apps\git\current\bin`, so probe that directly for the user-scope, global and relocated (`%SCOOP%`) install roots. Refs tinyhumansai#5270
|
Both review findings were valid and are now addressed. (Replying here rather than in-thread — threaded replies are blocked for me by the org's OAuth app access restrictions.) 1. CRLF line endings on
|
|
Tip For best results, initiate chat on the files or code changes.
🐇 ✅ ✏️ Learnings added
If you found this review helpful, would you consider giving us a shout-out on X? Thank you for using CodeRabbit! You are interacting with an AI system. |
|
Both fixes are well-executed. The current diff confirms the changes are in place: CRLF fix — Adding Scoop fix — The trace is correct and the fix is sound. Using One minor thing to verify: when The PR looks good to merge. Both original concerns are fully addressed, the explanation is thorough, and the fix remains narrowly scoped to the Windows dev entry point only. Tip: You can customize Greptile's behavior for this repo with |
|
Note on the one review thread still showing as open — the CodeRabbit CRLF finding on The repo root sets The durable fix is the checkout rule, added alongside the
The Greptile scoop finding was also valid and is fixed in f141144 (probing The one thing I still can't verify myself is an end-to-end |
CodeRabbit/Blinter E018 flagged the launcher for LF-only line endings: cmd.exe seeks labels by byte offset and re-reads the file in 512-byte chunks, so `goto`/`call :label` is the one batch construct whose behaviour depends on line endings. Rather than depend on a checkout rule being right on every machine, remove the dependency: the script is now straight-line batch with no labels, no `goto`/`call`, and no multi-line parenthesised blocks, so it behaves identically under LF and CRLF. The `.gitattributes` eol=crlf rule stays for editor/tooling consistency, but correctness no longer rests on it. Behaviour is unchanged: same probe order, same quoting of %BASH_EXE%, same exit-code propagation, still no bare `bash` PATH lookup.
…l flow Locks in the property that makes scripts/run-dev-win.cmd correct under both LF and CRLF: no labels, no goto/call. Without this a future edit could reintroduce the line-ending dependency that Blinter E018 flags.
coderabbitai
Bot
added
the
feature
|
Round-2 review feedback addressed in @coderabbitai —
|
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- changed files ---'
git diff --name-only HEAD^ HEAD || true
printf '%s\n' '--- .gitattributes ---'
cat -n .gitattributes
printf '%s\n' '--- scripts/run-dev-win.cmd ---'
cat -n scripts/run-dev-win.cmd
printf '%s\n' '--- app/package.json dev:app:win ---'
rg -n -C2 '"dev:app:win"\s*:' app/package.json
printf '%s\n' '--- app/test/dev-app-win-launcher.test.ts outline ---'
ast-grep outline app/test/dev-app-win-launcher.test.ts --items all || true
printf '%s\n' '--- launcher test source ---'
cat -n app/test/dev-app-win-launcher.test.ts
printf '%s\n' '--- executable batch control-flow tokens ---'
rg -n -i '^[[:space:]]*:|(^|[^[:alnum:]_])(goto|call)[[:space:]]+' scripts/run-dev-win.cmd || trueLength of output: 13628
The launcher keeps quoted
The original LF line-ending finding is no longer applicable. A real Windows execution remains useful final validation, but it is separate from this resolved control-flow concern. ✏️ Learnings added
You are interacting with an AI system. |
|
The label-free rewrite is the right call — much cleaner than arguing about checkout bytes forever. The implementation looks solid:
One minor nit: the The LF/CRLF concern is resolved. |
The launcher test added in the previous commit wrote its single-parameter arrow functions as `(line) => ...`. app/.prettierrc sets `"arrowParens": "avoid"`, so `pnpm --filter openhuman-app format:check` rejected them. Drop the parentheses; no behaviour change.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/test/dev-app-win-launcher.test.ts`:
- Around line 91-101: Move the Vitest suite containing the launcher label and
control-flow assertions from the current test location into app/src/**,
preserving its tests and imports. Alternatively, narrow the Vitest include
pattern so it only applies under app/test, but ensure the suite is located
within the required co-location path.
- Around line 91-101: Update the control-flow assertion in the test “uses no
label-based control flow, so line endings cannot change behaviour” to also
detect commands prefixed with `@`, including `@goto` and `@call :label`, while
preserving detection of the existing unprefixed forms.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 307eb427-3629-408f-a5c5-bdc8c34cb86d
📒 Files selected for processing (2)
app/test/dev-app-win-launcher.test.tsscripts/run-dev-win.cmd
🚧 Files skipped from review as they are similar to previous changes (1)
- scripts/run-dev-win.cmd
| it('uses no label-based control flow, so line endings cannot change behaviour', () => { | ||
| const code = launcherCodeLines(readFileSync(LAUNCHER_PATH, 'utf8')); | ||
|
|
||
| // cmd.exe finds a label by byte offset and re-reads the script in 512-byte | ||
| // chunks, so `goto`/`call :label` is the one construct whose behaviour | ||
| // depends on whether the file is LF or CRLF. Keeping the launcher free of | ||
| // labels makes it correct either way instead of relying on a checkout rule. | ||
| expect(code.filter((line) => line.startsWith(':'))).toEqual([]); | ||
| expect(code.filter((line) => /(^|\s)(goto|call)\s/i.test(line))).toEqual([]); | ||
| expect(code.length).toBeGreaterThan(0); | ||
| }); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== file exists =="
git ls-files | grep -F 'app/test/dev-app-win-launcher.test.ts' || true
echo "== matching files under app/test and app/src =="
git ls-files | grep -E '^app/.*\.test\.(ts|tsx)$' | sed -n '1,120p'
echo "== launcher source candidates =="
git ls-files | grep -E 'dev-app-win-launcher' | sed -n '1,120p'
echo "== package / config hints =="
git ls-files | grep -E '(^|/)(package\.json|vitest\.config\.[^/]*|\.gitignore)$' | while read -r f; do
echo "--- $f"
sed -n '1,220p' "$f"
doneRepository: tinyhumansai/openhuman
Length of output: 28228
Move the Vitest suite to app/src/**.
app/test/dev-app-win-launcher.test.ts is outside the required test co-location path. Apply the existing Vitest include pattern only under app/test, or move the spec under app/src/**.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/test/dev-app-win-launcher.test.ts` around lines 91 - 101, Move the Vitest
suite containing the launcher label and control-flow assertions from the current
test location into app/src/**, preserving its tests and imports. Alternatively,
narrow the Vitest include pattern so it only applies under app/test, but ensure
the suite is located within the required co-location path.
Source: Coding guidelines
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Target test excerpt:"
sed -n '80,105p' app/test/dev-app-win-launcher.test.ts || true
echo
echo "Launcher path definition:"
rg -n "LAUNCHER_PATH|launcherCodeLines|dev-app-win-launcher" app/test/dev-app-win-launcher.test.ts app -g '*.ts' -g '*.tsx'
echo
echo "Focused registry of launcher/test files:"
git ls-files | rg '(^|/)dev-app-win-launcher|launcher|win|batch' || trueRepository: tinyhumansai/openhuman
Length of output: 4743
🏁 Script executed:
#!/bin/bash
set -euo pipefail
node - <<'JS'
const current = /(^|\s)(goto|call)\s/i;
const proposed = /(^|[\s&|()])@?(goto|call)\s/i;
const lines = [
'goto :label',
'call :label',
'`@goto` :label',
'`@call` :label',
'echo go to label',
'call "path with spaces\\label"',
'`@call` "label"',
'goto\t:label',
'& goto :label',
'& `@goto` :label',
'( goto :label)',
'( `@goto` :label)',
];
console.log(JSON.stringify(lines.map(line => ({
input: line,
current: current.test(line),
proposed: proposed.test(line)
})), null, 2));
JSRepository: tinyhumansai/openhuman
Length of output: 1163
Detect @goto and @call :label forms.
CMD accepts @goto :label and @call :label; the current regex only fires when goto/call follows start-of-line or whitespace, so the regression test can still pass if those forms are added.
Proposed fix
- expect(code.filter(line => /(^|\s)(goto|call)\s/i.test(line))).toEqual([]);
+ expect(code.filter(line => /(^|[\s&|()])@?(goto|call)\s/i.test(line))).toEqual([]);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| it('uses no label-based control flow, so line endings cannot change behaviour', () => { | |
| const code = launcherCodeLines(readFileSync(LAUNCHER_PATH, 'utf8')); | |
| // cmd.exe finds a label by byte offset and re-reads the script in 512-byte | |
| // chunks, so `goto`/`call :label` is the one construct whose behaviour | |
| // depends on whether the file is LF or CRLF. Keeping the launcher free of | |
| // labels makes it correct either way instead of relying on a checkout rule. | |
| expect(code.filter((line) => line.startsWith(':'))).toEqual([]); | |
| expect(code.filter((line) => /(^|\s)(goto|call)\s/i.test(line))).toEqual([]); | |
| expect(code.length).toBeGreaterThan(0); | |
| }); | |
| it('uses no label-based control flow, so line endings cannot change behaviour', () => { | |
| const code = launcherCodeLines(readFileSync(LAUNCHER_PATH, 'utf8')); | |
| // cmd.exe finds a label by byte offset and re-reads the script in 512-byte | |
| // chunks, so `goto`/`call :label` is the one construct whose behaviour | |
| // depends on whether the file is LF or CRLF. Keeping the launcher free of | |
| // labels makes it correct either way instead of relying on a checkout rule. | |
| expect(code.filter((line) => line.startsWith(':'))).toEqual([]); | |
| expect(code.filter(line => /(^|[\s&|()])@?(goto|call)\s/i.test(line))).toEqual([]); | |
| expect(code.length).toBeGreaterThan(0); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/test/dev-app-win-launcher.test.ts` around lines 91 - 101, Update the
control-flow assertion in the test “uses no label-based control flow, so line
endings cannot change behaviour” to also detect commands prefixed with `@`,
including `@goto` and `@call :label`, while preserving detection of the existing
unprefixed forms.
The no-label guard only matched `goto`/`call` at start-of-line or after whitespace, so `@goto :label` and `& call :label` could be added without tripping it (CodeRabbit review on tinyhumansai#5342). Widen the jump pattern to allow the `@` echo-suppression prefix and the command separators cmd accepts before a command (whitespace, `&`, `|`, parentheses), and to treat `goto:eof` — the colon-joined form, which the suggested pattern still missed — as a jump. The label pattern likewise tolerates a leading `@`. Verified against the current launcher (21 executable lines, zero matches) under both LF and CRLF, with 14 positive and 6 negative control lines.
|
Round-3 review response — one finding accepted, one respectfully declined. (Posting top-level: threaded replies from my account return ✅ Accepted —
|
|
Both points are well-handled. Regex improvements — looks great. The widened
Wrapping the patterns themselves in positive/negative controls is exactly the right move. Without those, a future reader tightening the regex could hollow out the protection without any test failure. The 9/4 control split gives real confidence. File location — point taken. I missed that No remaining concerns from my side. The outstanding item is the live Windows smoke test you've already flagged; that's outside review scope. LGTM. |
1 participant
Summary
pnpm dev:app:winfails on every default Git for Windows install with'C:/Program' is not recognized as an internal or external command.scripts/run-dev-win.cmdwrapper so thebash.exepath is quoted in a context where quoting actually survives.app/package.jsonnow points at a relative, space-free path (..\scripts\run-dev-win.cmd) that needs no quoting of its own.goto/call, so its behaviour does not depend on whether it is checked out with LF or CRLF..gitattributesnow checks out*.cmd/*.batwith CRLF, matching the exception*.ps1already had.app/test/dev-app-win-launcher.test.ts, which modelscmd.exe /d /s /cargument parsing and locks in both the quoting regression and the no-label property.scripts/run-dev-win.shis unchanged — this only fixes how it is invoked.Problem
app/package.jsondeclared:pnpm runs package.json scripts through
cmd.exe /d /s /c <string>. The/Sflag strips the first and last quote characters of<string>before parsing it. So cmd receives:and takes
C:/Programas the program name, producing exactly the error reported in #5270. The quotes cannot protect the path at this layer — they are the first and last characters, so they are precisely what/Sremoves.Because
C:\Program Files\Gitis the default install location, this breaks the documented Windows dev path for essentially every contributor who did not install Git somewhere without a space.Solution
Quoting behaves normally inside a
.cmdfile, so the fix is to make the package.json body something cmd cannot mis-split, and do the real quoting one level down.app/package.json(single line changed):The new value has no spaces and no quotes, so
/Sstripping is a no-op on it. Backslashes are used rather than forward slashes because cmd treats a leading/as a switch prefix. pnpm runs lifecycle scripts with the package directory as cwd, so the relative path resolves fromapp/.scripts/run-dev-win.cmdthen locatesbash.exeand invokes it with the path properly quoted:Lookup order:
%OPENHUMAN_BASH_EXE%— escape hatch for portable/exotic Git installs.%ProgramFiles%\Git\bin,%ProgramFiles(x86)%\Git\bin,%LOCALAPPDATA%\Programs\Git\bin— the machine-scope and user-scope Git for Windows installers.%SCOOP%,%USERPROFILE%\scoop,%ProgramData%\scoop→apps\git\current\bin\bash.exe— scoop only exposes shims onPATH, so it must be probed directly (see below).git.exeonPATH— covers winget and chocolatey, wheregit.exelives in\cmdand bash in\bin.If none match it exits
1with actionable guidance instead of failing obscurely.No labels, no
goto/call— deliberateEach candidate is applied by a single-line
if not defined BASH_EXE if exist "…" set "BASH_EXE=…", and the failure path is a run of single-lineif not defined BASH_EXE echo …>&2lines terminated byif not defined BASH_EXE exit /b 1. There is nocall :use_if_existshelper and nogoto :no_bashjump.That is not stylistic.
cmd.exelocates a label by byte offset and re-reads the script in 512-byte chunks, which makes label-based control flow the one batch construct whose behaviour depends on the file's line endings — the basis of the well-known LF-only batch corruption bug. Writing the launcher without labels removes the dependency at the source, instead of relying on every checkout on every machine having applied the right.gitattributesrule. A test pins the property so it cannot silently regress..gitattributesThe repo root normalises everything with
* text=auto eol=lfand carves out exactly one Windows exception,*.ps1 text eol=crlf. Batch files had no rule at all, so*.cmd/*.bat text eol=crlfwas added alongside it. With the label-free launcher this is defence in depth and consistency for editors and future batch files, rather than the thing correctness rests on.Design notes / rejected alternatives
The issue suggested three options. This PR takes option 1 deliberately:
bash ../scripts/run-dev-win.shwas rejected as actively harmful.Git\binis not onPATHby default (onlyGit\cmd, which has nobash.exe), and on machines with WSL enabledbashresolves toC:\Windows\System32\bash.exe— the WSL launcher. That would run the script inside a Linux distro with none of the Windows toolchain it configures (MSVC, CEF, cargo-tauri), producing a far more confusing failure than the current one. The wrapper therefore never falls back to a barebashlookup, and there is a test asserting it does not..npmrcscript-shellwas rejected because it changes shell semantics repo-wide for every platform to fix one Windows script.scripts/run-dev-win.shgenerates.batshims internally (vcvars_launcher,run-vite.bat) for cargo-tauri'sbeforeDevCommandto dodge this exact quoting problem. This change applies the same trick one level up, at the pnpm boundary.Impact
dev:app,dev:web,dev:wryand all CI/build/release scripts are untouched; nothing outside Windows executes a.cmd.OPENHUMAN_BASH_EXEcovers anything exotic.where git.exe.Related
Review feedback addressed
text=autonormalises the blob to LF on commit regardless, so a checkout rule is the only durable form of that fix.where git.exeunder scoop returns<scoop>\shims\git.exe, so the derived<shim>\..\bin\bash.exeresolved to<scoop>\bin\bash.exe, which never exists. Now probingapps\git\current\bin\bash.exe— the real binary behind the shim, via thecurrentjunction scoop maintains across upgrades — across the user, global and%SCOOP%-relocated roots. The failure-path diagnostic and the comment above thegit.exeprobe were corrected to match.E018) after the.gitattributesfix, and it has a fair point that the earlier fix left the repository object LF-only — which is what anyone reading the file through the API, a raw link, or a source archive sees. Rather than argue the checkout rule again, the dependency itself is now gone: the launcher was rewritten with no labels and nogoto/call, which is the only batch construct whose parsing is line-ending sensitive. The file is now correct under LF and CRLF, the.gitattributesrule stays for consistency, andapp/test/dev-app-win-launcher.test.tsgained a fifth case asserting no label-based control flow so this cannot regress. Behaviour is otherwise unchanged: identical probe order, identical quoting, identical exit-code propagation.Submission Checklist
app/test/dev-app-win-launcher.test.tshas 5 cases. The failure-path case reconstructs the old script body verbatim and assertscmd /Sparsing yieldsC:/Program, so the regression is pinned rather than merely described. The others assert the current body survives/Sas one token, that it points at a file that exists, that the wrapper quotes%BASH_EXE%and contains no bare-bashlookup, and that the wrapper uses no label-based control flow.N/A, and verifiably so rather than by assertion:app/test/vitest.config.tssetscoverage.include: ["src/**/*.{ts,tsx}"]. All four changed files (app/package.json,app/test/dev-app-win-launcher.test.ts,scripts/run-dev-win.cmd,.gitattributes) fall outsideapp/src/**, so no changed line appears in the lcov report anddiff-cover --fail-under=80has no lines to judge. No Rust changed, socargo-llvm-covis unaffected. Please flag if you would rather I approach this differently.N/A: developer tooling change.docs/TEST-COVERAGE-MATRIX.mdtracks product feature rows with IDs validated against a catalog; a build-script entry point has no feature ID, and inventing one would add noise. Calling this out explicitly rather than silently skipping — happy to add a row if you disagree.## Related—N/A, none (see above).N/A: does not touch release-cut surfaces. This affects the dev entry point only; release builds go throughmacos:build:*/build:app.Closes #NNNin## Related—Closes #5270.AI Authored PR Metadata (required for Codex/Linear PRs)
Disclosing this fully, since the template asks for it: the investigation and patch were drafted with AI assistance, reviewed line-by-line by me before pushing. It is not a Codex/Linear-tracked PR, so those fields are
N/A.Linear Issue
N/A— external contribution, tracked by GitHub issue dev:app:win always fails on standard Git for Windows install (cmd.exe strips nested quotes) #5270.Commit & Branch
Mustaqeem66:fix/5270-dev-app-win-cmd-quotingb353a745ad541696126481825deb6bb5a040cd7fValidation Run
pnpm --filter openhuman-app format:checkpnpm typecheckpnpm --filter openhuman-app test -- dev-app-win-launcherN/A— no Rust changed.N/A— no Tauri config or Rust changed.Validation Blocked
I would rather state this plainly than tick boxes I did not earn:
command:pnpm --filter openhuman-app format:check,pnpm typecheck,pnpm test,pnpm test:coverageerror:pnpm: command not found— the environment I prepared this in has no Node package manager, no Rust toolchain and no network access, so dependencies cannot be installed.impact:The test file has not been executed locally under vitest. To de-risk that, the pure-logic helpers were extracted and run standalone under plainnode: the legacy body yieldsC:/Program, the new body survives/Sunchanged, and the no-label assertions were run against both an LF and a CRLF copy of the launcher (both pass, which is the point of the rewrite). The remaining assertions areexistsSyncand substring checks against files added in this PR. CI is the real verification for typecheck/format/lint — the first run flagged aformat:checkfailure (hand-wrapped call expressions that fit inside the repo'sprintWidthof 100), fixed in bb9e76a with no behavioural change to the tests.impact (Windows):I have no Windows machine attached to this environment, sopnpm dev:app:winhas not been run end-to-end against a real Git install. The.cmdwas desk-checked for batch parsing hazards — one was found and fixed in9df25a7(an unescaped)in%ProgramFiles(x86)%inside anif (...)block would have closed the block early), and the later rewrite removed every multi-line parenthesised block and every label, so that class of hazard is now structurally absent. A maintainer sanity-check on a real Windows box would be genuinely valuable before merge, and I will act on any failure immediately.Behavior Changes
pnpm dev:app:winresolvesbash.exeat runtime instead of assuming one hardcoded absolute path, and is invoked so thatcmd.exe /Scannot split it.pnpm dev:app:winstarts working on default Git for Windows installs instead of erroring immediately. No end-user or product-facing change.Parity Contract
<bash.exe> <repo>/scripts/run-dev-win.sh, withrun-dev-win.shbyte-identical. On the machines where the old form happened to work (Git atC:\Program Files\Git), the wrapper's first filesystem probe resolves to that samebash.exe, so behavior is identical. Arguments are forwarded via%*.exit /b %ERRORLEVEL%so failures still surface to pnpm. The fallback chain only ever adds candidates that the old hardcoded path did not cover; it removes none, and the label-free rewrite preserved the probe order exactly. The one intentional non-fallback is a barebashPATH lookup, excluded because of the WSL hazard described above and asserted absent by test.Duplicate / Superseded PR Handling
N/ASummary by CodeRabbit
Bug Fixes
Chores
Tests