Fix launch shell startup behavior and inherit environment variables#1231
Open
Jelloeater wants to merge 4 commits into
Open
Fix launch shell startup behavior and inherit environment variables#1231Jelloeater wants to merge 4 commits into
Jelloeater wants to merge 4 commits into
Conversation
…iables Agent-Logs-Url: https://github.com/Jelloeater/agent-deck/sessions/dab152fe-e915-4f0b-8d0b-99312f757c82 Co-authored-by: Jelloeater <7862369+Jelloeater@users.noreply.github.com>
feat(shell): add launch_shell option to inherit shell environment variables
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThis PR adds a ChangesLaunch Shell Feature
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 6 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (6 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds an opt-in [shell].launch_shell config (issue #1218) that wraps agent spawn commands with an interactive shell invocation ($SHELL -il -c '<cmd>') so environment variables defined in ~/.zshrc/~/.bashrc are inherited by the agent process. Supports per-session override, excludes sandboxed/SSH/shell sessions, and integrates with the existing prepareCommand wrapping chain.
Changes:
- New
LaunchShell *boolfield inShellSettingsand per-Instance, withGetLaunchShell()/launchShellEnabled()accessors defaulting to off. - New
wrapLaunchShellstep inserted intoprepareCommand, with bash-specific~/.bashrcsourcing and single-quote escaping. - Documentation update in
config-reference.mdand a new test file covering happy paths, exclusions, config fallback, and interaction withexit_to_shell.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| skills/agent-deck/references/config-reference.md | Documents the new launch_shell config key. |
| internal/session/userconfig.go | Adds LaunchShell field and GetLaunchShell() accessor. |
| internal/session/instance.go | Adds Instance.LaunchShell, launchShellEnabled, wrapLaunchShell; wires into prepareCommand. |
| internal/session/env.go | Adds explanatory comment on layering with launch_shell. |
| internal/session/issue1218_launch_shell_test.go | New tests covering wrap behavior, exclusions, config fallback, and exit_to_shell interaction. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // | ||
| // The transform is: | ||
| // | ||
| // $SHELL -il -c '<command>' |
Comment on lines
+6893
to
+6895
| if filepath.Base(shell) == "bash" { | ||
| return fmt.Sprintf("%s -il -c 'if [ -f ~/.bashrc ]; then source ~/.bashrc; fi; %s'", shell, escaped) | ||
| } |
|
|
||
| // --- Empty SHELL env uses default --- | ||
|
|
||
| func TestLaunchShell_DefaultsToBashdWhenSHELLUnset(t *testing.T) { |
Comment on lines
+224
to
+227
| if !strings.Contains(wrapped, `opencode --query '"'"'hello world'"'"'`) && | ||
| !strings.Contains(wrapped, `opencode --query '\"'\"'hello world'\"'\"'`) { | ||
| t.Fatalf("single quotes must be escaped, got:\n%s", wrapped) | ||
| } |
|
|
||
| // --- Global config fallback --- | ||
|
|
||
| func TestLaunchShell_GlobalConfigFallback(t *testing.T) { |
|
|
||
| // --- Per-session override takes precedence --- | ||
|
|
||
| func TestLaunchShell_PerSessionOverride(t *testing.T) { |
Comment on lines
+6878
to
+6881
| func (i *Instance) wrapLaunchShell(command string) string { | ||
| if command == "" || i.IsSandboxed() || !i.launchShellEnabled() { | ||
| return command | ||
| } |
Comment on lines
+30
to
+35
| // launchShellTestEnv isolates HOME and SHELL for deterministic testing. | ||
| func launchShellTestEnv(t *testing.T) { | ||
| t.Helper() | ||
| origHome := os.Getenv("HOME") | ||
| origShell := os.Getenv("SHELL") | ||
| os.Setenv("HOME", t.TempDir()) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request introduces a new
[shell].launch_shellfeature to the agent session system, allowing agent commands to be wrapped in an interactive shell so that environment variables from shell startup files (like.zshrcor.bashrc) are available to the agent process. This addresses cases where agents launched from the TUI did not inherit the user's interactive shell environment, causing failures in MCP configs that reference environment variables. The implementation includes per-session and global config support, proper exclusions (sandbox, SSH, shell tools), robust quoting, and comprehensive tests.The most important changes are:
Core feature: Interactive shell wrapping for agent commands
LaunchShellfield to theInstancestruct and[shell]config section, with logic to wrap agent commands in an interactive shell invocation ($SHELL -il -c '<command>') when enabled. Bash sessions also explicitly source.bashrcto ensure environment variables are loaded. [1] [2] [3] [4]Command preparation logic:
prepareCommandmethod to apply the launch-shell wrapper in the correct order, ensuring shell startup files are loaded before the agent command and any user command wrappers.Documentation:
launch_shelloption in the configuration reference, including its default value, usage, and caveats. [1] [2]Testing:
issue1218_launch_shell_test.gocovering all major behaviors: flag on/off, per-session and global config, exclusions (sandbox, SSH, shell tool), quote escaping, and integration withprepareCommand.Code comments and clarity:
env.goto clarify the order of environment sourcing and where shell wrapping occurs in the startup sequence.Summary by CodeRabbit
launch_shellconfiguration option to wrap session commands in an interactive shell, enabling shell startup files to execute before launching the agent.