fix(desktop): stop simple-git custom-binary warning spam on Windows

[hernanda-git]

Fixes #79245

Symptom

On Windows, hermes desktop floods the launching terminal with hundreds of identical lines while the app sits idle:

Invalid value supplied for custom binary, restricted characters must be removed or supply the unsafe.allowUnsafeCustomBinary option

Real errors in the same stream (Error occurred in handler for 'hermes:api') get buried in the noise.

Root cause

apps/desktop/electron/git-review-ops.ts → gitFor() deliberately passes
unsafe: { allowUnsafeCustomBinary: true } when the resolved git binary path
contains a space — the default Windows install is C:\Program Files\Git\cmd\git.exe.
That opt-in is correct and is what keeps the Review pane working on Windows
(#54888, #64810), so it must be preserved.

But simple-git's custom-binary.plugin runs its validation on every
construction and simply trades the throw for a warn:

const isBad = input.some(isBadArgument);   // /^([a-z]:)?([a-z0-9/.\\_~-]+)$/i — a space fails
if (isBad) {
  if (allowUnsafe) { console.warn(WRONG_CHARS_ERR); }
  else { throw new GitPluginError(...); }
}

gitFor() is not memoized: it builds a fresh client per call, and it is called
from repoStatus() and the review ops the coding rail polls. One warning per
poll, forever.

The warning carries no information for us — gitBin is resolved inside the
Electron main process from known install locations / PATH, never from renderer or
user input, and we opted in knowingly.

The fix

Suppress only that one known warning, only around the construction call:

• match on the known prefix, so any other console.warn still passes through;
• restore console.warn in a finally, so the patch cannot leak past the call;
• only wrap at all when we're actually taking the unsafe-binary path — the common
  case is untouched.

The alternative (memoizing gitFor) would reduce but not eliminate the spam, and
would introduce client-lifetime/cwd-caching questions this bug doesn't warrant.

Tests

Added to apps/desktop/electron/git-review-ops.test.ts:

• constructing with a spaced binary path 5× emits zero warnings;
• console.warn is restored afterwards, so an unrelated warning still surfaces.

Both fail on main and pass with this change:

# main (patch reverted, tests kept)
 × gitFor does not log simple-git custom-binary warnings for a spaced binary path
 × gitFor restores console.warn so unrelated warnings still surface
   Tests  2 failed | 9 passed (11)

# with the fix
 ✓ electron/git-review-ops.test.ts (11 tests) 4449ms
   Tests  11 passed (11)

npx tsc -p tsconfig.electron.json --noEmit is clean.

Verified on Windows 10 with git at C:\Program Files\Git\cmd\git.exe.

[hernanda-git]

Companion fix for the other symptom in the same log excerpt: #79266 (the Error occurred in handler for 'hermes:api' stack traces). Independent branch off main, no dependency between the two.

[DavidMetcalfe]

Commenting in the context of #80184 (the composite startup-noise report, whose simple-git half duplicates #79245).

Reviewed this against current main (simple-git 3.36.0) — the fix matches the actual failure mode.

Verification: I reproduced the warning independently: constructing simpleGit({ binary: 'C:\\Program Files\\Git\\cmd\\git.exe', unsafe: { allowUnsafeCustomBinary: true } }) emits exactly one console.warn("Invalid value supplied for custom binary...") per construction, confirming the issue body's root cause (unmemoized gitFor() + per-construction validation in simple-git's custom-binary plugin).

The fix looks right:

• Scoping the suppression to the construction call (rather than a global console.warn override at startup) is the right layer — the warning is emitted synchronously by simple-git during simpleGit(...), and restoring console.warn in finally keeps unrelated warnings visible.
• The startsWith(SIMPLE_GIT_UNSAFE_BINARY_WARNING) match is appropriately narrow.
• The new tests cover both the suppression and the restore behavior, which is the exact regression surface (spaced binary path on Windows).

Sibling case worth confirming (whole-bug-class check): the unsafe flag on main is gated on /\s/.test(gitBin) — space only. simple-git's restricted-character regex (/^([a-z]:)?([a-z0-9/.\\_~-]+)$/i) rejects more than spaces. I verified against the pinned 3.36.0:

• C:\Git(x86)\cmd\git.exe (parens, no space) → throws GitPluginError without the unsafe flag
• C:\Users\José\Git\cmd\git.exe (non-ASCII, no space) → throws without the unsafe flag

The common Windows x86 install C:\Program Files (x86)\Git\cmd\git.exe contains a space, so it gets the unsafe flag on main and only warns (which this PR silences) — but a no-space restricted-char path (e.g. a non-ASCII home directory, cf. #60447) would still throw the original #54888/#64810 error even with this PR's suppression. #71944 addresses exactly that by making the flag unconditional, though it does not silence the warning. A combined fix (unconditional unsafe flag + this PR's construction-scoped suppression) would cover the whole class. Is the no-space restricted-char throw intentionally out of scope for this PR, or should it fold in?

Either way, this PR correctly fixes the reported spam. Just flagging the sibling case per the repo's "fix the whole bug class" guidance.

[andrexibiza]

Superseded by current-main delivery PR #91798.

The exact implementation was replayed through GitHub onto current upstream state while preserving @hernanda-git as the Git author:

• e90dc2a747d03e124f74b6e41f481d5f296f5119

Exact-head receipts:

• CI 32523118643 — success
• Docker 32523118040 — success
• Nix 32523117979 — success

GitHub reports #91798 clean, mergeable, and rebaseable. Closing this stale/conflicting transport PR so one credited delivery owner remains. This is a supersession receipt, not a rejection of the original implementation.