Skip to content

fix(git): run local git children with the login shell's environment - #4

Merged
nplez1 merged 2 commits into
nplez1/mainfrom
fix/git-login-shell-env
Sep 19, 2026
Merged

nplez1 merged 2 commits into
nplez1/mainfrom
fix/git-login-shell-env

Conversation

@nplez1

@nplez1 nplez1 commented Sep 19, 2026 •

Copy link
Copy Markdown
Owner
Files Added Deleted Net
Test 2 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​258 0 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​258
Prod 8 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​350 $\color{#cf222e}{\Huge{\mathbf{−}}}$​19 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​331

ELI5

Orca already patches up PATH from your shell config, but nothing else. So if you export a value in ~/.zprofile and then press Commit, the git process — and therefore your pre-commit hook — never sees it, and the commit fails for a reason the Source Control pane cannot explain.

What Changed

Before: every git child the Source Control pane spawned ran with the environment Orca's own process happened to have. On a Dock/Launchpad launch that is launchd's minimal environment, so exports from ~/.zprofile, ~/.zshenv or ~/.profile were missing — except for the one variable Orca already hydrates on purpose (PATH, in startup/hydrate-shell-path.ts). pre-commit and pre-push run inside git's process, so a hook that reads such a variable failed; a push that needs the ssh-agent socket (SSH_AUTH_SOCK) could not authenticate.

After: those git children start from the same environment Orca's terminals start from, so a hook sees what the user's own shell would give it.

Mechanism — new src/main/git/command-runner/local-login-shell-git-environment.ts:

  • resolves the profile-loading shell's environment through resolveLoginShellEnvironment(), the resolver PTY panes and structured agent sessions already start from, so there is one source of truth instead of a second probe;
  • memoizes it once per process and layers it under the caller's environment, so nothing Orca pins deliberately is lost — the hydrated PATH, the untranslated-output locale pins, the credential-prompt guards;
  • exposes a sync snapshot for the spawn paths that cannot await, so a git status read racing app startup gets the previous environment rather than stalling;
  • degrades to the launch environment if the probe fails, because every later git child shares that one promise;
  • is enabled at startup (src/main/startup/main-process-ready-foundation.ts) on a packaged POSIX build — the launch shape that loses the exports — where it warms while the first window opens.

Wired into gitExecFileAsync, gitSpawnAfterWindowsEnvironmentReady, gitStreamStdout, gitSpawn and gitExecFileSync. WSL-routed and native Windows git keep exactly today's environment: the former already runs inside the distro's login shell, the latter already inherits the user's registry environment.

Six source files (five touched, one new) plus two test files.

Second commit — process guard. While landing this, an attempt was made to open the PR against stablyai/orca instead of this fork, and then to open it against nplez1/main from a branch cut from the mirror main — either would have been wrong, and the second would have shown every fork-only patch as a reversion. So the branch also carries local/open-pr.mjs, which defaults to this fork and refuses a head that is not a descendant of the base; see Notes. Say the word if you would rather split that into its own PR.

Why

The gap is in the launch environment, not in Source Control, so it cannot be fixed at the commit call site: the hook reads git's environment, and git's environment came from Orca's.

Alternatives considered:

  • Merge the whole shell environment into process.env at startup (roughly what VS Code does on macOS). Rejected: app-wide blast radius — agents, plugins, language servers and Electron internals would all inherit it — for a problem that lives in git children, and it would import shell-internal variables (SHLVL, _, OLDPWD) into the app process.
  • Parse the startup files for the variable names with the existing readShellStartupEnvVar() helper. Rejected: it resolves one named variable and cannot enumerate, and it deliberately does not evaluate conditionals, eval, or sourced files, so exactly the setups that need this would stay broken.
  • Run git through a login shell (-ilc). Rejected: one extra shell per git call, rc-file output landing on stdout for commands Orca parses, and changed argv/cwd semantics.

Linked Issue

N/A — this PR targets the fork's own line (nplez1/main), so there is no issue to close, and no issue has been filed upstream for this bug either. Left unlinked deliberately rather than overlooked; the body above is the written record of the reproduction.

Visual Proof

N/A — no visual or interaction change. No .tsx or renderer file is touched; the visible effect is a commit or push that used to fail now succeeding.

Testing

  • pnpm typecheck (all projects), pnpm lint, and pnpm run check:code-quality:changed (0 new findings across the 8 changed files) all pass on this branch.

  • pnpm test src/main/git src/main/startup: 294 files, 3050 passed, 0 failed, 17 skipped.

  • New tests cover merge precedence, one probe per process, aborting during the probe, the sync-spawn snapshot, failure degradation, and — at the spawn boundary rather than on the module — that the profile exports actually reach execFile/spawn for a commit.

  • Real-shell evidence, macOS with /bin/zsh and a real ~/.zprofile: run with a launchd-like 35-key environment, the probe returns the missing exports in ~25 ms, including the two variables the reporting setup's hooks need, while the hydrated PATH still wins unchanged. (Key names only; no values were read or recorded.)

  • local/open-pr.mjs: --dry-run emits the fork-targeted gh pr create; a probe branch cut from main is refused with the reversion explanation, which is the mistake this guard exists for.

  • Not tested here: the packaged build on the affected machine, which is the user-facing confirmation this PR is waiting on. No pnpm build was run locally. Linux takes the same code path but was not exercised manually; Windows and WSL are untouched by design.

  • I manually tested these changes locally (macOS, real login shell and real .zprofile)

  • Automated tests added/updated, or explained why not below

AI Disclosure

AI-assisted: Pi coding agent on DeepSeek Flash — implementation, tests, the real-shell evidence run, and the process guard.

Review

Code review summary, covering the six checks CONTRIBUTING lists:

  • Cross-platform: POSIX-only by construction, gated on isShellStartupEnvProbeSupported(). Native Windows keeps its registry environment and the Windows PATH-readiness helper is untouched (its win32 tests still pass). No path, shortcut, or shell-string construction is added.
  • SSH / remote / local: host-local only. A remote worktree's commit runs on the execution host through the relay, which assembles its own environment in src/relay/relay-command-env.ts; that is a separate fix and is called out in Notes rather than left silent.
  • Agents / integrations: no agent launch path changes. The probe reuses the resolver structured agent sessions already use, so their environment is unchanged.
  • Performance: one interactive login shell per process, memoized, and only on a packaged POSIX launch; ~25 ms measured here, behind the resolver's existing 5 s cap. The sync spawn paths never wait on it, and no git call gains a shell.
  • UI quality: no UI change.
  • Security: the environment is read, not written; no new input, file, or network access. The probe runs the user's own shell with the user's own rc files, which is what the existing startup PATH probe and structured agent sessions already do.

Agent skill upstream boundary

  • Not applicable, or this change follows docs/reference/agent-skill-sharing-upstream-boundary.md and copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.

Notes

  • Boundaries left deliberately, for review: (1) remote/SSH worktrees still get the execution host's relay environment rather than a profile probe on that host; (2) the binary blob read (gitExecFileAsyncBuffer, git show) keeps today's environment, since no hook runs on it. Both are follow-ups rather than silent gaps.
  • Precedence trade-off: the shell environment is layered under the existing one, so it only fills gaps. A value the launch environment already carries (a launchd LANG, say) is not replaced by the profile's value. That is the conservative reading; the alternative — profile values overriding inherited ones — would also override Orca's own pins.
  • The probe spawns the user's shell with -ilc, so rc-file side effects run once per packaged launch. That is the same thing the startup PATH probe already does on macOS, and it is why this is opt-in per process rather than always on.
  • Process guard: local/open-pr.mjs makes this fork the default PR target and requires --upstream for stablyai/orca, and it refuses a head branch that is not a descendant of the base it is opened against. BRANCHES.md now states the same rule in words, because the old text only said PR branches are cut from origin/main and never said which repository they are opened against — with the fork's local/branch-status.mjs looking for PRs on upstream, upstream read as the default.
  • If this fix is ever wanted upstream, the same commit applies cleanly to origin/main (verified by cherry-pick, with typecheck and tests green there); it would just be re-cut from main per the branch rules.
  • Visual proof is N/A (above), so there are no attachments.

Checklist

  • This PR is small and focused
  • I explained what changed and why (ELI5, the user-facing before/after, the mechanism, and why over the alternatives)
  • Before/after screenshots or videos attached for UI changes, or N/A with reason
  • Self-reviewed for correctness, security, and performance
  • Cross-platform, SSH/remote, and path/shortcut impact considered (or N/A)
  • pnpm lint, pnpm typecheck, pnpm test, and pnpm build pass (or CI will cover; local preferred) — lint, typecheck and test were run locally; pnpm build is left to CI

A Dock/Launchpad launch inherits launchd's minimal environment, so anything a
user exports from ~/.zprofile, ~/.zshenv or ~/.profile was missing from git's
process. Commit and push hooks run with git's environment, so a hook that needs
one of those variables failed with no visible cause, and a push needing the
agent socket could not authenticate. Only PATH was ever hydrated
(startup/hydrate-shell-path.ts); every other export stopped at the app process.

- Reuses the profile-loading shell resolver that structured agent sessions
  already start from, so a hook sees what the user's terminal sees, and layers
  it *under* the caller's environment so Orca's own values still win: the
  hydrated PATH, the untranslated-output locale, the credential guards.
- Opt-in per process and enabled at startup on a packaged build, which is the
  launch shape that loses the exports. Unpackaged runs, unit tests and the relay
  spawn no extra shell, and the probe is memoized once resolved.
- Sync spawn paths read the settled result rather than stalling, so a status
  read racing app startup gets the previous environment, never a wait.
- WSL and native Windows are untouched: their git already runs inside the
  distro's login shell or with the user's registry environment.
- A failed probe degrades to the launch environment instead of failing every
  later git child, since all of them share the one promise.
Nothing in the repository said which repository a PR belongs to: BRANCHES.md only
said which base a branch is *cut* from, and local/branch-status.mjs looks for PRs
on upstream, so upstream read as the default everywhere. A PR aimed at stablyai
is a decision, and getting it wrong is expensive in both directions — proposing
fork-only work upstream, or cutting a head from `main` and opening it against
`nplez1/main`, where the diff silently carries every fork patch as a reversion.

- local/open-pr.mjs opens against nplez1/orca with base nplez1/main by default,
  requires an explicit --upstream to leave the fork, and refuses a head that is
  not a descendant of the base it is opened against.
- BRANCHES.md states the same two rules in words and names the two `main`
  branches explicitly, since their difference is what the mistake turned on.
@nplez1
nplez1 merged commit 32babc1 into nplez1/main Sep 19, 2026
1 check passed
nplez1 pushed a commit that referenced this pull request Oct 19, 2026
…-page readers off the deprecated owner resolver (stablyai#30478)

Thirteen read-routing callers (#4, #5, #10-#20 of the round-3 caller list) read the owner through
runtimeEnvironmentIdWithLegacyFallbackUnlessTwin. A workspace two hosts publish is no longer
searched on either host. The focus-read ratchet now counts the legacy-fallback adapter, which
reads the focused runtime inside an object literal its discovery skipped; the five F1 entries
that blind spot pruned come back. Deprecated importers 53 -> 40.
nplez1 pushed a commit that referenced this pull request Oct 21, 2026
…executionHostId (stablyai#31054)

* fix(runtime): PR/MR worktree base refuses SSH projects named only by executionHostId

Codex review 6 #4: the guard read the legacy connectionId, so a row stamped
executionHostId 'ssh:<target>' without it ran git fetch on this machine.

* fix(runtime): one gate for local Git on a repo row; base resolution agrees with create

Review follow-up: a local row with a stale connectionId and runtime: rows now
refuse too (create already refused runtime: rows), through
requireRepoLocalGitRoute in repo-execution-host.ts.

* fix(runtime): keep base resolution for runtime-stamped rows this server owns

Codex follow-up: older servers keep a client-minted runtime:<id> stamp on
rows whose files are here, and base resolution worked for them before.
requireRepoLocalGitRoute reads the stored host (getStoredRepoExecutionHostId),
so they run locally again; SSH rows and any row with a connectionId still
refuse.
nplez1 pushed a commit that referenced this pull request Oct 21, 2026
…p ones (stablyai#31096)

* feat(ci): ratchet raw repo connectionId host reads; one drop-in resolver

Codex review 6 class fix (#4/#6/#7): host decisions read off a repo row's
legacy connectionId miss rows that name SSH only as executionHostId.

- getRepoRoutingConnectionId (shared/execution-host.ts): drop-in for a raw
  repo.connectionId routing check that also finds executionHostId-only SSH.
- check-repo-host-read-ratchet: per-file count of raw repo connectionId
  reads in src/main, src/shared, src/cli; counts may only shrink.
- Migrated 66 cheap reads: runtime GitHub/GitLab commands, PR/MR head ref
  fetches, preserved-branch cleanup, ref queries, base-ref IPC, worktree
  preparation, trash sweep, scan cache TTL, usage metadata.

* fix(ipc): PR base IPC routes executionHostId-only SSH rows; ratchet is a stated lower bound

Review follow-up (Opus on stablyai#31096):
- worktrees:resolvePrBase decided git/gh routing on the raw connectionId while
  its head-ref fetches already used the resolver; one resolved connectionId
  now drives all of it (test fails on main).
- getRepoRoutingConnectionId's contract is stated exactly: never local when the
  raw field is set; on a contradictory ssh row executionHostId wins.
- The ratchet regex no longer matches report.connectionId; header and docs
  say it is a lower bound.
- Regression tests for a runtime GitHub and GitLab command site.

* test(ci): pin the repo host-read ratchet step to the static phase

* fix(ci): repo host-read ratchet skips verbatim field copies into projected rows

A copy of connectionId into a projected row decides nothing; migrating it to
the resolver would change what the projection publishes. The sidebar feed
catalog's copy (added after the baseline) and two older projections no longer
count; baseline pruned to 91.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant