Conversation
ApprovabilityVerdict: Not approved Macroscope's review found this PR not approvable — This changes the default environment of integrated terminals by stripping multiplexer context from inherited server variables, affecting all relevant terminal sessions rather than an opt-in path. The implementation is small and tested, but the default behavior change warrants human review. You can add or adjust custom eligibility rules. Learn more. |
|
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 configurationConfiguration used: Repository: pingdotgg/t3code/.coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📥 CommitsReviewing files that changed from the base of the PR and between 54fce3db31f62feb8c8965142d197e021e9599c2 and 7866600. 📒 Files selected for processing (1)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughThe terminal spawn environment now excludes multiplexer variables inherited from the launching pane. A test checks that these variables are filtered and that an explicitly supplied ChangesTerminal environment filtering
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~8 minutes Change: Bug fix Merge Risk: ⚪ Minimal · up to Terminal spawns no longer inherit the launching multiplexer’s variables, while explicitly supplied terminal values remain available. No material merge-blocking risk is evident. Architecture SummaryArchitecture risk: 🔵 Low · up to The change affects 1 system. Changed systems: Architecture concerns Review detailsSystems and components
Before / after behavior
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
When the server runs inside tmux, screen, or Zellij, integrated terminals inherited TMUX, TMUX_PANE, STY, WINDOW, and ZELLIJ* from it. Tools then acted on the pane that launched the server, e.g. clipboard helpers wrote to that tmux buffer instead of the T3 Code terminal. Drop these from the inherited env; explicit terminal env can still set them. Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
54fce3d to
7866600
Compare
What Changed
Integrated terminals no longer inherit
TMUX,TMUX_PANE,STY,WINDOW,ZELLIJ,ZELLIJ_SESSION_NAME, orZELLIJ_PANE_IDfrom the server process. The change adds these names toTERMINAL_ENV_BLOCKLISTinapps/server/src/terminal/Manager.ts, the same list that already dropsELECTRON_RUN_AS_NODE(#1162).The blocklist only filters the inherited server env. A per-terminal or provider
envcan still set any of these variables. One test inManager.test.tscovers both cases.Why
If you start the server from a tmux pane with
t3 serve,npx t3, orvp run dev, every integrated terminal gets that pane'sTMUXandTMUX_PANE. Shells in the terminal then act as if they run inside tmux, even though their pty belongs to T3 Code:$TMUXruntmux load-buffer -w -. The copy goes to the tmux session that launched the server, not to the T3 Code terminal.tmux split-window,tmux display, and anything that readsTMUX_PANEact on that unrelated pane.GNU screen (
STY,WINDOW) and Zellij (ZELLIJ*) have the same problem. I only drop the variables Zellij sets for a session, so user settings likeZELLIJ_CONFIG_DIRstill pass through.The background service (
t3 service install) is not affected. systemd and launchd start it with a clean env.I checked it by hand. I ran
vp run devfrom a tmux pane, opened an integrated terminal, and ranecho "TMUX=$TMUX PANE=$TMUX_PANE TP=$TERM_PROGRAM". It printedTMUX= PANE= TP=tmux.TERM_PROGRAM=tmuxstill comes through because this PR leaves it alone. #9954 setsTERM_PROGRAMfor T3 Code terminals, which would fix that part.This does not make copying from the terminal work. That needs OSC 52 support (#8361).
Tests:
vp test run src/terminal/Manager.test.ts(83 passed), server typecheck, and lint on the changed files.Checklist
Model: Claude Opus 5.5. Harness: Claude Code in T3 Code.
Summary by CodeRabbit
TMUX_PANEvalue.