Skip to content

fix(shell): let shell startup files detect environment probes - #13952

Open
jimeh wants to merge 1 commit into
pingdotgg:mainfrom
jimeh:t3code/mark-shell-environment-probes
Open

jimeh wants to merge 1 commit into
pingdotgg:mainfrom
jimeh:t3code/mark-shell-environment-probes

Conversation

@jimeh

@jimeh jimeh commented Sep 27, 2026 •

Copy link
Copy Markdown

What Changed

T3 Code now sets T3CODE_RESOLVING_ENVIRONMENT=1 in the environment of the shell processes it spawns to read the user's environment:

  • The login-shell probes: DesktopShellEnvironment on desktop, and readEnvironmentFromLoginShell in packages/shared, which the server's fixPath uses.
  • The PowerShell probes, with and without the profile, in the same two places.

Only the probe child gets the variable. It never lands in T3 Code's own process.env, and it can't be captured back because the probes copy only allowlisted variables such as PATH and SSH_AUTH_SOCK. The rest of the inherited environment, the shell flags, timeouts, captured variables, and marker-string protocol are unchanged. The launchctl getenv PATH fallback stays unmarked because it runs no user scripts.

The install guide now explains the probe and shows how startup files can use the marker:

[ -n "$T3CODE_RESOLVING_ENVIRONMENT" ] && return

Why

T3 Code resolves PATH by running $SHELL -ilc "<capture command>". The -i matters because many users set PATH in .zshrc or .bashrc, but it also makes the probe indistinguishable from a real terminal: [[ -o interactive ]] is true and there is no other reliable signal. Today the only way to detect it is to match T3 Code's internal __T3CODE_ENV_* strings in the command, which only works in zsh.

So the full interactive setup runs during the probe, and the PATH it produces is installed into the T3 Code process and inherited by every agent session and terminal. Some of that setup is meant for one live shell. For example, mise activate swaps mise's shims directory for the install directories of whatever tool versions are active at that moment. Agents then keep those pinned versions and miss newly installed tools until the app restarts. Slow startup files also eat into the probes' 5 second timeout.

VS Code has the same kind of probe and sets VSCODE_RESOLVING_ENVIRONMENT=1 for exactly this reason. This gives startup files the same signal under a T3 Code name. Users who don't check it see no change.

Notes for review:

  • VS Code only resolves the shell environment on macOS and Linux. Marking the PowerShell probes goes a step further, because the profile probe runs $PROFILE, where slow prompt setup is common.
  • Background processes that a startup file launches during the probe inherit the marker, as they do with VS Code.
  • This complements fix(shell): avoid interactive job control in environment probes #11275, which drops -i from the probes and so skips .zshrc entirely. The marker keeps the default behavior and lets each user opt out in their own startup files.

Testing

  • vp test run packages/shared/src/shell.test.ts apps/desktop/src/shell/DesktopShellEnvironment.test.ts: new and updated tests assert that the login-shell and PowerShell probes get the marker on top of the inherited environment, that launchctl does not, and that neither the process environment nor the captured environment contains it. Removing the marker makes the new tests fail.
  • vp run --filter <package> test passes for @t3tools/shared, @t3tools/desktop, and t3, except desktop's scripts/browser-secret-native.test.mjs, which needs libsecret-1 development headers that were not installed on the test machine.
  • vp run --filter <package> typecheck passes for all three packages, and vp lint passes.
  • A real probe against zsh on Linux, with a temporary ZDOTDIR, showed the startup file seeing the marker and returning early while the parent environment stayed clean. The snippet was also checked in dash, bash, and zsh. Not verified on macOS or Windows.

Checklist

  • This PR is small and focused
  • I explained what changed and why
  • I included before/after screenshots for any UI changes (no UI changes)
  • I included a video for animation/interaction changes (none)

Written on behalf of jimeh by claude-opus-5-5 using T3 Code.

Summary by CodeRabbit

  • New Features

    • Shell startup scripts can detect when T3 Code is reading the environment using T3CODE_RESOLVING_ENVIRONMENT=1. The marker is set only for the probe process, not added to the parent environment.
  • Documentation

    • Added guidance on environment detection during startup on macOS, Linux, and Windows, including how to use the marker to skip slow or session-specific setup.

@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:M 30-99 changed lines (additions + deletions). labels Sep 27, 2026
T3 Code resolves PATH by running the user's shell with -ilc, so rc files
cannot tell the probe apart from a real terminal. Heavy interactive setup
such as `mise activate` then runs during the probe and its session-only
PATH leaks into every agent and terminal the app starts.

Set T3CODE_RESOLVING_ENVIRONMENT=1 in the child environment of the
desktop and server login-shell and PowerShell probes, mirroring VS Code's
VSCODE_RESOLVING_ENVIRONMENT. The marker never reaches the app's own
environment because only allowlisted variables are copied back. The
install guide shows a POSIX check for rc files.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@jimeh
jimeh force-pushed the t3code/mark-shell-environment-probes branch from 9691fff to 0e4adab Compare September 27, 2026 13:37
@macroscopeapp

macroscopeapp Bot commented Sep 27, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — This PR adds a new environment signal to production shell probes in both shared/server and desktop startup paths, allowing user startup files to skip session-only setup. The implementation is small and tested, but it introduces cross-platform runtime behavior across files historically maintained by other contributors.

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

@coderabbitai

coderabbitai Bot commented Sep 27, 2026

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: pingdotgg/t3code/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: 18fda126-4d94-4bf1-a2ae-b6566a8bc6cc

📥 Commits

Reviewing files that changed from the base of the PR and between de251fc and 0e4adab.

📒 Files selected for processing (5)
  • apps/desktop/src/shell/DesktopShellEnvironment.test.ts
  • apps/desktop/src/shell/DesktopShellEnvironment.ts
  • docs/user/install.md
  • packages/shared/src/shell.test.ts
  • packages/shared/src/shell.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.


📝 Walkthrough

Walkthrough

Shell environment probes now pass T3CODE_RESOLVING_ENVIRONMENT=1 to child processes on macOS, Linux, and Windows. Tests check the child environments and confirm that the marker is not added to the parent environment. Setup documentation describes the probes and shows how shell profiles can check the marker.

Changes

Shell environment probes

Layer / File(s) Summary
Shared shell probe environment
packages/shared/src/shell.ts, packages/shared/src/shell.test.ts
The shared shell module defines the marker and passes it to login-shell and PowerShell probes. Tests check the marker in child environments and retain assertions for probe options.
Desktop probe integration
apps/desktop/src/shell/DesktopShellEnvironment.ts, apps/desktop/src/shell/DesktopShellEnvironment.test.ts, docs/user/install.md
Desktop probes pass the marker through runCommandOutput. Tests check probe options and verify the parent environment remains unchanged. Setup documentation describes the marker and provides profile examples.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Feature

Suggested reviewers: juliusmarminge, t3dotgg

Merge Risk: ⚪ Minimal · up to 0e4ad

The marker remains limited to shell probes; no issue identified here prevents merging after normal checks.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 0e4ad

The marker appears limited to environment-probe subprocesses, with no demonstrated path for it to enter the application’s environment. The remaining risk is limited but not fully resolved because the shared readers also accept caller-selected variable names.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The marker is visible to probe shells and processes they spawn. Captured environment values can subsequently affect the desktop process and its children; evidence does not establish a broader tenant or service boundary change.

Trust Boundaries and Controls

  • observed — The shared probe builds a marked copy of process.env rather than assigning the marker to the parent. The desktop probe supplies a child override, and desktop extraction returns only requested names.
  • observed — The generic shared readers accept caller-selected capture names. They could return the marker if it were explicitly requested; whether untrusted input can select those names remains unestablished.

Resilience and Maintainability Implications

  • observed — Failed or timed-out desktop probes return empty output. The marker is supplied at child creation, not installed through the returned environment patch.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 4 files. (1 skipped: 1 … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: shell startup files can detect environment probes.
Description check ✅ Passed The description includes complete What Changed, Why, Testing, and Checklist sections. It explains the implementation, scope, motivation, test coverage, and known platform limitations.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 4 files. (1 skipped: 1 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR

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

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M 30-99 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant