fix(desktop): force GIT_PAGER=cat to dodge Apple Git 2.50+ block#4568
Conversation
|
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. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughgetProcessEnvWithShellPath now applies a shell-derived PATH only when available (no early return) and removes both env.PAGER and env.GIT_PAGER from the returned environment. ChangesGit Pager and Shell PATH Configuration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add 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 |
Greptile SummaryThis PR fixes a workspace-creation breakage on macOS with Apple Git 2.50+ (Sequoia 15.5 / current Homebrew), which refuses to honour an inherited
Confidence Score: 4/5Safe to merge — the change is a small, well-contained one-liner that fixes a real workspace-creation failure on Apple Git 2.50+. The fix is correct and clearly motivated. The only gap is that
|
| Filename | Overview |
|---|---|
| apps/desktop/src/lib/trpc/routers/workspaces/utils/shell-env.ts | Adds env.GIT_PAGER = "cat" to getProcessEnvWithShellPath and refactors the early-return on missing PATH to a positive conditional, ensuring the pager override is always applied. execWithShellEnv still uses getProcessEnvWithShellEnv directly and won't inherit this fix. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["getProcessEnvWithShellPath(baseEnv)"] --> B["getShellEnvironment()"]
B --> C["getProcessEnvWithShellEnv(baseEnv, shellEnvResult)\n(existing values win)"]
C --> D{shellPath found?}
D -- Yes --> E["env.PATH = shellPath\n(+ env.Path on win32)"]
D -- No --> F[skip PATH assignment]
E --> G["env.GIT_PAGER = 'cat' ← NEW"]
F --> G
G --> H["return env"]
H --> I["getSimpleGitWithShellPath\n(simple-git wrapper)"]
H --> J["host-service-coordinator.ts:515\n(child spawn)"]
K["execWithShellEnv(cmd, args)"] --> L["getProcessEnvWithShellEnv only\n⚠️ GIT_PAGER NOT set here"]
Comments Outside Diff (1)
-
apps/desktop/src/lib/trpc/routers/workspaces/utils/shell-env.ts, line 213-226 (link)execWithShellEnvmisses theGIT_PAGERfixexecWithShellEnv(line 213) callsgetProcessEnvWithShellEnvdirectly — notgetProcessEnvWithShellPath— so any git command routed through it will not carryGIT_PAGER=catand remains exposed to the same Apple Git 2.50+ block. If any current or future caller passes a git binary throughexecWithShellEnv, the fix won't apply. Moving theGIT_PAGERassignment intogetProcessEnvWithShellEnv(which is called by both paths) would close this gap universally.Prompt To Fix With AI
This is a comment left during a code review. Path: apps/desktop/src/lib/trpc/routers/workspaces/utils/shell-env.ts Line: 213-226 Comment: **`execWithShellEnv` misses the `GIT_PAGER` fix** `execWithShellEnv` (line 213) calls `getProcessEnvWithShellEnv` directly — not `getProcessEnvWithShellPath` — so any git command routed through it will not carry `GIT_PAGER=cat` and remains exposed to the same Apple Git 2.50+ block. If any current or future caller passes a git binary through `execWithShellEnv`, the fix won't apply. Moving the `GIT_PAGER` assignment into `getProcessEnvWithShellEnv` (which is called by both paths) would close this gap universally. How can I resolve this? If you propose a fix, please make it concise.
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:213-226
**`execWithShellEnv` misses the `GIT_PAGER` fix**
`execWithShellEnv` (line 213) calls `getProcessEnvWithShellEnv` directly — not `getProcessEnvWithShellPath` — so any git command routed through it will not carry `GIT_PAGER=cat` and remains exposed to the same Apple Git 2.50+ block. If any current or future caller passes a git binary through `execWithShellEnv`, the fix won't apply. Moving the `GIT_PAGER` assignment into `getProcessEnvWithShellEnv` (which is called by both paths) would close this gap universally.
Reviews (1): Last reviewed commit: "fix(desktop): force GIT_PAGER=cat to dod..." | Re-trigger Greptile
Git ≥ 2.50 refuses to honor an *inherited* PAGER or GIT_PAGER env on
non-interactive callers, erroring with:
Use of "PAGER" is not permitted without enabling allowUnsafePager
(or "GIT_PAGER", if that's the one set)
simple-git wraps that stderr as GitPluginError and breaks every git
read path used by workspace creation — git status, git worktree list,
git worktree prune, git for-each-ref — surfacing in the renderer as
"Couldn't create workspace".
The first version of this PR set GIT_PAGER=cat to override PAGER, but
git also rejects an inherited GIT_PAGER under the same check, so CI
turned up the same error with the variable name swapped (visible in
the hasUnpushedCommits test fallbacks).
Right fix: delete both. simple-git's git children have piped stdout
(not a TTY), so git skips paging entirely — no replacement value is
needed. Interactive user terminals get their own PAGER/GIT_PAGER from
the login shell snapshot, so this only affects the host-service /
desktop-main process's own git children.
`getProcessEnvWithShellPath` is the single env builder for both
desktop's simple-git wrapper (`getSimpleGitWithShellPath`) and the
host-service child spawn (`host-service-coordinator.ts:515`). Stripping
the pager vars here cascades to:
- all simple-git calls in desktop main
- the host-service's `process.env` → inherited by every simpleGit()
call inside host-service (13 callsites)
saddlepaddle
force-pushed
the
saddlepaddle/git-pager-fix
branch
from
e690941 to
2570c92
Compare
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
simple-git's @simple-git/argv-parser (since v3.36) rejects inherited EDITOR/GIT_EDITOR env vars unless allowUnsafeEditor is set, surfacing as "Use of \"EDITOR\" is not permitted without enabling allowUnsafeEditor" and breaking workspace creation, status reads, worktree prune, etc. for any user with EDITOR exported in their shell. #4568 already fixed the PAGER half by stripping PAGER/GIT_PAGER from getProcessEnvWithShellPath, which builds the env for both the desktop simple-git wrapper and the host-service spawn. Extend the same strip to EDITOR/GIT_EDITOR — none of our scripted git plumbing opens an editor. Adds a reproduction test plus a sibling PAGER test to lock in the existing #4568 behavior.
…ns (superset-sh#4568) Git ≥ 2.50 refuses to honor an *inherited* PAGER or GIT_PAGER env on non-interactive callers, erroring with: Use of "PAGER" is not permitted without enabling allowUnsafePager (or "GIT_PAGER", if that's the one set) simple-git wraps that stderr as GitPluginError and breaks every git read path used by workspace creation — git status, git worktree list, git worktree prune, git for-each-ref — surfacing in the renderer as "Couldn't create workspace". The first version of this PR set GIT_PAGER=cat to override PAGER, but git also rejects an inherited GIT_PAGER under the same check, so CI turned up the same error with the variable name swapped (visible in the hasUnpushedCommits test fallbacks). Right fix: delete both. simple-git's git children have piped stdout (not a TTY), so git skips paging entirely — no replacement value is needed. Interactive user terminals get their own PAGER/GIT_PAGER from the login shell snapshot, so this only affects the host-service / desktop-main process's own git children. `getProcessEnvWithShellPath` is the single env builder for both desktop's simple-git wrapper (`getSimpleGitWithShellPath`) and the host-service child spawn (`host-service-coordinator.ts:515`). Stripping the pager vars here cascades to: - all simple-git calls in desktop main - the host-service's `process.env` → inherited by every simpleGit() call inside host-service (13 callsites)
1 participant
Summary
Apple Git 2.50 (shipped in macOS Sequoia 15.5+ and current Homebrew) refuses to honor an inherited
PAGERenv var on non-interactive callers, erroring with:```
Use of "PAGER" is not permitted without enabling allowUnsafePager
```
simple-gitwraps that stderr asGitPluginError, which breaks every git read path used by workspace creation —git status,git worktree list,git worktree prune,git for-each-ref— and bubbles up to the renderer as "Couldn't create workspace".Fix
One-liner in
getProcessEnvWithShellPath(apps/desktop/src/lib/trpc/routers/workspaces/utils/shell-env.ts), which is the single env builder used by both:getSimpleGitWithShellPath)host-service-coordinator.ts:515)Setting
GIT_PAGER=catcascades to all simple-git calls in desktop main + the host-service'sprocess.env→ inherited by everysimpleGit()call inside host-service (13 callsites, mostly without explicit.env()overrides).GIT_PAGERtakes precedence overPAGERfor git, so users' interactive terminals still see their preferred pager (delta, less, etc.). Only git inside non-interactive helpers is silenced — pagers are useless in programmatic git invocations anyway.Test plan
PAGERset in login shell: open Canary → New Workspace → workspace creates without error~/.superset/host/<org>/host-service.logno longer showsGitPluginError: Use of "PAGER" is not permitted...linesPAGERfor non-git toolsBackground log evidence (Satya's local Canary)
```
[git/getBranchSyncStatus] git.status() failed; treating working tree as clean for this read GitPluginError: Use of "PAGER" is not permitted without enabling allowUnsafePager
[workspace-creation] git worktree list failed: GitPluginError: Use of "PAGER" is not permitted without enabling allowUnsafePager
[workspaceCreation.searchBranches] git for-each-ref failed: GitPluginError: Use of "PAGER" is not permitted without enabling allowUnsafePager
[workspaces.create] worktree prune failed: GitPluginError: Use of "PAGER" is not permitted without enabling allowUnsafePager
```
Summary by cubic
Strip
PAGERandGIT_PAGERfrom the env for all desktop and host-servicegitcalls to avoid Apple Git 2.50+ blocking inherited pager vars in non-interactive runs. This restores workspace creation and other git reads while leaving interactive terminals unchanged.PAGERandGIT_PAGERingetProcessEnvWithShellPath, cascading to allsimple-gitcalls and host-service spawns (fixesgit status, worktree ops, and ref listing).PATH/PathwhenshellPathis present.Written for commit 2570c92. Summary will update on new commits.
Summary by CodeRabbit