Skip to content

fix(desktop): use consistent env for persistent terminal sessions#783

Merged
Kitenite merged 5 commits intomainfrom
env-var-testing-feature
Jan 17, 2026
Merged

fix(desktop): use consistent env for persistent terminal sessions#783
Kitenite merged 5 commits intomainfrom
env-var-testing-feature

Conversation

@Kitenite
Copy link
Copy Markdown
Collaborator

@Kitenite Kitenite commented Jan 17, 2026

Summary

  • Fix environment variable handling for persistent terminal sessions in the desktop app
  • Ensure consistent env configuration across terminal sessions

Test plan

  • Verify terminal sessions receive correct environment variables
  • Test persistent terminal sessions maintain env consistency

Summary by CodeRabbit

  • Refactor
    • Streamlined environment variable handling for terminal sessions to make startup more reliable.
    • Removed verbose debug logging that exposed shell invocation details and environment paths, reducing console clutter while preserving prior child-process behavior.

✏️ Tip: You can customize this high-level summary in your review settings.

The daemon-based terminal manager was merging the daemon's process.env
with the filtered environment from buildTerminalEnv(). This caused
inconsistent behavior between persistent and non-persistent modes:

- Non-persistent: uses only the filtered env from buildTerminalEnv()
- Persistent: merged daemon's process.env + filtered env

The daemon's process.env could be stale (from when daemon was first
spawned) or contain different values than expected, leading to
unpredictable terminal environments.

Now both modes use only the filtered env from buildTerminalEnv(),
ensuring consistent and predictable behavior.
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Jan 17, 2026

📝 Walkthrough

Walkthrough

Session.spawn now builds the child process environment using buildSafeEnv(...) over the merged host/env map instead of locally filtering process.env; TERM is set to "xterm-256color" and ELECTRON_RUN_AS_NODE behavior for the Electron subprocess is preserved.

Changes

Cohort / File(s) Summary
PTY Environment Handling
apps/desktop/src/main/terminal-host/session.ts
Replaced manual filtering/merging of process.env with buildSafeEnv({ ...process.env, ...env }) to produce processEnv; removed debug spawn logging; still sets TERM="xterm-256color" and preserves ELECTRON_RUN_AS_NODE for the spawned Electron subprocess.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

Poem

🐰 I hop through vars both near and far,
Trusting the main to set the spar,
TERM stays bold, the rest made neat,
Less local fuss — a tidy seat! 🥕✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title 'fix(desktop): use consistent env for persistent terminal sessions' clearly and concisely describes the main change: standardizing environment variable handling for persistent terminal sessions in the desktop app.
Description check ✅ Passed The pull request description covers the main objectives and test plan, but lacks several template sections including 'Related Issues', 'Type of Change', and 'Additional Notes'.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

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.

❤️ Share

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

@Kitenite Kitenite force-pushed the env-var-testing-feature branch from 1ce6346 to bc840ed Compare January 17, 2026 01:31
…essions

Merge process.env with passed env (passed takes precedence), then filter
through buildSafeEnv allowlist. This ensures shells always have a working
base environment while filtering out secrets and Electron-specific vars.
@Kitenite Kitenite force-pushed the env-var-testing-feature branch from bc840ed to 3cb7471 Compare January 17, 2026 01:34
@Kitenite Kitenite merged commit becee54 into main Jan 17, 2026
4 of 5 checks passed
@Kitenite Kitenite deleted the env-var-testing-feature branch January 17, 2026 01:37
@github-actions
Copy link
Copy Markdown
Contributor

🧹 Preview Cleanup Complete

The following preview resources have been cleaned up:

  • ⚠️ Neon database branch
  • ⚠️ Electric Fly.io app

Thank you for your contribution! 🎉

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