Skip to content

fix(desktop): support spaced Windows Git paths in review - #67612

Merged
teknium1 merged 1 commit into
mainfrom
hermes/hermes-61a56d51
Jul 19, 2026
Merged

fix(desktop): support spaced Windows Git paths in review#67612
teknium1 merged 1 commit into
mainfrom
hermes/hermes-61a56d51

Conversation

@teknium1

Copy link
Copy Markdown
Contributor

Summary

The Desktop Review pane now works on Windows when Git lives at a path containing spaces (the default C:\Program Files\Git\cmd\git.exe). simple-git's custom-binary validation rejects spaced paths, so every review IPC call threw and the pane silently showed "No diffs" (#54888).

Minimal fix: in gitFor() — the single simple-git constructor all desktop git ops use — when the internally resolved binary contains whitespace, opt into simple-git's supported unsafe.allowUnsafeCustomBinary escape hatch. The binary is resolved in the Electron main process from known install locations or PATH, never renderer/user input, so keeping the trusted absolute path beats falling back to PATH (often absent in GUI-launched apps; PATH lookup could also resolve a repo-local git.exe).

Simplified from #64713 by @unsupportedpastels (authorship preserved): drops the cmd.exe 8.3 short-path conversion layer — the escape hatch alone covers all volumes, including those with 8.3 names disabled. Supersedes #55337 / #60156.

Changes

  • apps/desktop/electron/git-review-ops.ts: gitFor() sets unsafe.allowUnsafeCustomBinary only when the resolved binary contains whitespace; gitFor exported for tests
  • apps/desktop/electron/git-review-ops.test.ts: constructor accepts a spaced Windows path (all OS); live status() through the real spaced Git for Windows binary (win32-only, skips when absent)

Validation

Before After
gitFor(cwd, 'C:\Program Files\Git\cmd\git.exe') throws GitPluginError constructs, runs
Non-spaced binaries validated by simple-git unchanged (no unsafe flag)
desktop vitest (git-review-ops.test.ts) 7/7 pass
tsc -p tsconfig.electron.json --noEmit clean

Fixes #54888

Infographic

review-pane-spaced-git-paths

simple-git's custom-binary validation rejects paths containing spaces, so
the default Windows Git install (C:\Program Files\Git\cmd\git.exe) made
every Review pane git call throw and the pane silently showed 'No diffs'.

The binary is resolved inside the Electron main process from known install
locations or PATH — never renderer/user input — so for spaced paths we opt
into simple-git's supported unsafe.allowUnsafeCustomBinary escape hatch
rather than falling back to PATH (often absent in GUI-launched apps).

Simplified from PR #64713 by @unsupportedpastels; supersedes the 8.3
short-path approaches in #55337/#60156.

Fixes #54888
@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/desktop Electron desktop app (apps/desktop/*) platform/windows Native Windows-specific behavior or breakage needs-decision Awaiting maintainer decision before any implementation sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows labels Jul 19, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Related: this uses simple-git's trusted custom-binary escape hatch, while #64713 also normalizes a Windows short path and #64812 falls back to PATH; maintainer choice is needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/desktop Electron desktop app (apps/desktop/*) needs-decision Awaiting maintainer decision before any implementation P3 Low — cosmetic, nice to have platform/windows Native Windows-specific behavior or breakage sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Hermes Desktop git-review errors on Windows — GitPluginError: restricted characters

3 participants