fix: solve #4599 — strip EDITOR/GIT_EDITOR so git ≥ 2.50 stops blocking workspace spawns

[github-actions]

Root cause

Git 2.50 added a security check that refuses to honor inherited EDITOR / GIT_EDITOR env vars on non-interactive callers, emitting:

Use of "EDITOR" is not permitted without enabling allowUnsafeEditor

simple-git surfaces this as GitPluginError, which is what fires when users try to spawn a new workspace (reported in #4599 against 1.9.5).

The same class of error already broke us once for PAGER/GIT_PAGER, and apps/desktop/src/lib/trpc/routers/workspaces/utils/shell-env.ts already strips those in getProcessEnvWithShellPath. The fix just extends that allowlist to EDITOR / GIT_EDITOR.

The fix

apps/desktop/src/lib/trpc/routers/workspaces/utils/shell-env.ts:206-211 — delete EDITOR and GIT_EDITOR alongside PAGER and GIT_PAGER before returning the env that gets piped into simple-git / execGit. Programmatic git children never prompt for commit messages or hunk edits, so no replacement value is needed. User-facing terminals continue to inherit EDITOR from the interactive shell (this code path doesn't run for them).

Why this is safe

• getProcessEnvWithShellPath is only used to build env for the host-service / desktop main process's own git children (getSimpleGitWithShellPath, execGitWithShellPath).
• Those children all run with piped stdio and never invoke git commit / git rebase -i / git add -p, so an editor never needs to launch.
• This mirrors the existing PAGER / GIT_PAGER strip and the rationale documented inline.

Tests

apps/desktop/src/lib/trpc/routers/workspaces/utils/shell-env.test.ts — added two cases under getProcessEnvWithShellPath strips unsafe git env vars:

• strips PAGER and GIT_PAGER from the returned env (pins existing behavior so it can't regress)
• strips EDITOR and GIT_EDITOR from the returned env (the bug — fails on main, passes with the fix)

Full suite at apps/desktop/src/lib/trpc/routers/workspaces/utils/: 203 pass / 0 fail.

Closes #4599

---

Summary by cubic

Prevents workspace creation from failing on Git ≥ 2.50 by stripping EDITOR/GIT_EDITOR from env for non-interactive git calls, matching existing PAGER/GIT_PAGER logic. Fixes #4599.

• Bug Fixes
  • Remove EDITOR and GIT_EDITOR in getProcessEnvWithShellPath before calling simple-git/execGit.
  • Scope: only programmatic git children; user terminals keep their EDITOR.
  • Tests added for both PAGER/GIT_PAGER and EDITOR/GIT_EDITOR stripping.

^{Written for commit 6a811a3. Summary will update on new commits.}

[capy-ai]

Capy auto-review is paused for this organization because the monthly auto-review limit has been reached. Increase the limit or turn it off in billing settings to resume automatic reviews.

[cubic-dev-ai]

No issues found across 2 files

[greptile-apps]

Greptile Summary

This PR fixes a workspace-spawn breakage caused by git ≥ 2.50's new security check that refuses inherited EDITOR/GIT_EDITOR env vars on non-interactive callers, extending the existing PAGER/GIT_PAGER strip in getProcessEnvWithShellPath to cover the editor variables as well.

• shell-env.ts: Adds delete env.EDITOR and delete env.GIT_EDITOR immediately after the existing PAGER/GIT_PAGER deletes, following the identical pattern and rationale already documented inline.
• shell-env.test.ts: Adds a new describe block with two tests — one pinning the existing PAGER behavior and one covering the new EDITOR fix — both using a controlled baseEnv and clean absence assertions.

Confidence Score: 4/5

Safe to merge — the fix is a minimal, targeted deletion that follows an already-proven pattern, and the new tests correctly cover the changed behaviour.

The change is straightforward and well-tested. The one open question is whether VISUAL (git's third-priority editor fallback, checked before EDITOR) should also be stripped; users who have VISUAL set in their environment but not EDITOR could still trigger the same git 2.50 block.

apps/desktop/src/lib/trpc/routers/workspaces/utils/shell-env.ts — specifically the list of deleted env vars around line 208.

Important Files Changed

Filename	Overview
apps/desktop/src/lib/trpc/routers/workspaces/utils/shell-env.ts	Extends the PAGER/GIT_PAGER stripping pattern to also delete EDITOR and GIT_EDITOR before returning the env used by programmatic git children; VISUAL (checked by git before EDITOR) is not yet stripped.
apps/desktop/src/lib/trpc/routers/workspaces/utils/shell-env.test.ts	Adds two regression tests for PAGER/GIT_PAGER and EDITOR/GIT_EDITOR stripping; tests are well-isolated using a controlled baseEnv and make clean absence assertions.

Prompt To Fix All With AI

Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
apps/desktop/src/lib/trpc/routers/workspaces/utils/shell-env.ts:208-213
Git resolves its editor via `$GIT_EDITOR` → `core.editor` → `$VISUAL` → `$EDITOR` (per `git var GIT_EDITOR` docs). If a user has `VISUAL` set but not `EDITOR`, git 2.50's `allowUnsafeEditor` check would still fire on that variable, reproducing the same `GitPluginError` this PR is fixing. `GIT_SEQUENCE_EDITOR` is similarly consulted for interactive-rebase todo files and could trigger the same class of error. Stripping them here alongside the others would be consistent and pre-emptive.

```suggestion
	delete env.PAGER;
	delete env.GIT_PAGER;
	delete env.EDITOR;
	delete env.GIT_EDITOR;
	delete env.VISUAL;
	delete env.GIT_SEQUENCE_EDITOR;

	return env;
```

_{Reviews (1): Last reviewed commit: "fix: solve #4599 — strip EDITOR/GIT_EDIT..." | Re-trigger Greptile}