Sign up
View stargazers13Sign up

fix(server): recover the login shell SSH agent#10312

open5
main
GitHub fork: fix/server-login-shell-ssh-agent
openedSep 6, 2026byYyashranaway
2 files changed
View on GitHubTwo-way sync enabledPR last imported 4d ago

Problem

The Windows desktop WSL backend can start without SSH_AUTH_SOCK even when its login shell exports a working agent. With SSH commit signing enabled, Commit, Commit & push, and Commit, push & PR then fail at the commit step.

Change

Recover a missing SSH_AUTH_SOCK alongside PATH from the accepted login-shell probe during unprepared Linux/macOS server startup. Preserve an inherited socket, the prepared-environment early return, and PATH/mise hydration.

Scope and approval

Fixes #5740 GitCafe: https://git.cafe/pingdotgg/t3code/issues/5740 Triaged bug via the automated issue flow. Existing contributor implementation and maintainer takeover retained. Limited to agent sockets exported by the login shell; it does not address #10179.

Verification

Rechecked during issue triage on October 22:

  • vp test run apps/server/src/os-jank.test.ts packages/shared/src/shell.test.ts: 63 passed, 1 skipped at the PR head.
  • Production fixPath plus real Git SSH signing, using an isolated agent, disposable key and a wrapper shell: current main 3b95e7831b leaves the socket absent and commit exits 128; PR head recovers the socket, commit exits 0, and the commit contains an SSH signature. This is a direct server-startup behavior check, not a Windows/WSL desktop reproduction.
  • No visible render paths changed; visual fixture matrix is empty. Windows/WSL desktop remains cannot-verify desktop-wsl because wsl.exe is unavailable here.

5740-runtime.mjs 5740-runtime-current-main.log 5740-runtime-pr.log

Previously recorded verification follows:

Verification on d189ceac58:

  • Replaced the six original hydrator-level cases with a five-row extension of main’s startup test: Linux/macOS recovery, forwarded socket precedence, accepted shell without an agent, and no accepted PATH. Existing HOME/prepared/Windows checks remain; mise also asserts its associated PATH. os-jank cases: 15→13.
  • Both recovery cases fail behaviorally against main; focused os-jank/shared-shell suites pass 63 tests with one existing skip. Scoped server typecheck, targeted lint, formatting, and whitespace checks pass.
  • Headless Linux proof ran production fixPath through a real login-shell wrapper and two temporary empty SSH agents:
text
1recovery: ssh-add -l exit 2 (no agent) → exit 1 (agent has no identities)
2forwarded: inherited second-agent socket preserved
3prepared: shell calls=0; existing PATH preserved
4PATH and __MISE_DIFF propagation: passed
5owned agents stopped; temporary HOME/sockets/wrapper removed

The wrapper avoids user profiles; mise’s record was synthetic. Empty-agent connectivity does not establish key signing, remote Git authentication, or a provider CLI flow. macOS, Windows, iOS, Android, Electron desktop, OrbStack, and WSL were not verified live. No visual UI change; screenshots do not apply.

One independent review round (Claude Opus 5.5 and GPT-6-Sol) found no code blockers. Historical GitHub findings were rechecked against this revision. Windows desktop/WSL remains unverified (cannot-verify desktop-wsl); this Linux VM has no wsl.exe.

Tests: GPT-6-Astra; implementation: GPT-6.1-Sol; adversarial reviews: Claude Opus 5.5 and GPT-6-Sol. T3 Code using the Codex and Claude harnesses.

Checks

23 successful · 8 skipped · 1 neutral
All checks passed
d189cea

Added 1 commit

  • fix(server): recover the login shell SSH agent39987ea
apps/server/src/os-jank.tslines 31–31Outdated · 39987eaOpen thread
……@@ -17,13 +18,21 @@ function logPathHydrationWarning(message: string, error?: unknown): void {
24- shellPath = readPathFromLoginShell(shell);
29+ const shellEnvironment = readShellEnvironment(shell, ["PATH", "SSH_AUTH_SOCK"]);
30+ shellPath = shellEnvironment.PATH;
31+ if (!env.SSH_AUTH_SOCK && shellEnvironment.SSH_AUTH_SOCK) {
MmacroscopeappBoton lines 31–31Sep 6, 2026

🟠 High src/os-jank.ts:31

A fallback shell's SSH_AUTH_SOCK is committed even when that shell provides no PATH, so a later shell's selected PATH can be paired with the earlier shell's stale agent socket. Only copy SSH_AUTH_SOCK when the candidate also provides the PATH that causes it to be accepted.

No longer relevant as of 863f077c23fdd9a838e6bf46b9145035e864af4c

YyashranawaySep 6, 2026

Fixed in 863f077c2: the agent socket now comes from the accepted shell, with coverage for a preceding candidate that has no PATH.

MmacroscopeappBotSep 6, 2026

Sorry, I'm unable to act on this request because you do not have permissions within this repository.

MmacroscopeappBotSep 6, 2026
Synced from GitHub

Approvability

Verdict: Needs Human Approval

Macroscope's review found this PR requires human approval — The production change modifies server startup environment hydration so downstream operations can use an SSH agent recovered from the login shell. Although the implementation is small and well-covered by tests, this authentication-sensitive runtime behavior requires human review.

You can add or adjust custom eligibility rules. Learn more.

Added 1 commit

  • fix(server): use the accepted shell agent socket863f077
JJulius Marminge25d ago

Note

This comment is posted by Julius' dot

The reporter in #10179 returned “absent” when the login shell started without SSH_AUTH_SOCK, and this patch reads that same login-shell environment. Can you remove the “Fixes #10179” claim and limit the stated coverage to agents available from the login shell, or provide evidence showing how this path recovers that reporter's socket? The verification requirement calls for evidence matching the claimed fix; #5740 remains a useful, distinct case.

Yyashranaway24d ago
Synced from GitHub

Removed the automatic closing claim for #10179 and narrowed the description to sockets exported by the login shell. The patch does not recover an agent socket absent from that environment; #5740 remains the covered case.

Added 3 commits

  • Merge remote-tracking branch 'origin/main' into batch/ssh-relay-remote-03/10312bc29425
  • test(server): verify login-shell SSH agent during startup5b2344e
  • fix(server): keep login-shell SSH recovery in private PATH hydrationd189cea
JJulius Marminge7d ago

Taking over this one. I’m retaining @yashranaway’s login-shell agent recovery, resolving main’s PATH/mise and prepared-environment conflicts, and replacing the test-only hydrator seam with startup behavior coverage. Headless Linux proof uses two isolated SSH agents. Leaving it unmerged for credential-propagation review.

CcoderabbitaiBot7d ago
Synced from GitHub

Warning

Review limit reached

Only developers with an assigned seat can use this organization's usage-based review budget, and seats here are assigned manually. Ask an admin to assign a seat, or change the review continuation mode in Billing.

Next included review available in 14 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 10 included reviews currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration
  • Configuration used: Path: .coderabbit.config.ts
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 031c6640-fc41-4666-8e8f-a4b9a771c670
📥 Commits

Reviewing files that changed from the base of the PR and between 1451e7cb23034d29774338608368a15d8845df36 and d189ceac5889557c541873352bbb2ef5a0ed5a4b.

📒 Files selected for processing: 2
  • apps/server/src/os-jank.test.ts
  • apps/server/src/os-jank.ts
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

JJulius Marminge7d ago

Takeover complete at d189ceac58. Current-head build, lint, and test jobs pass; skipped jobs remain skipped. No unresolved inline findings. One full independent review round with Claude Opus 5.5 and GPT-6-Sol found no code blockers. The PR body contains the current focused tests, headless proof, original-author credit, and platform limits.

Macroscope correctness passed. Its neutral approvability result explicitly requires human review of login-shell credential propagation. Leaving unmerged.