Fallback to available shell when run_terminal_command shell is missing#11131
Open
siewcapital wants to merge 1 commit intocontinuedev:mainfrom
Open
Fallback to available shell when run_terminal_command shell is missing#11131siewcapital wants to merge 1 commit intocontinuedev:mainfrom
siewcapital wants to merge 1 commit intocontinuedev:mainfrom
Conversation
Contributor
|
I have read the CLA Document and I hereby sign the CLA You can retrigger this bot by commenting recheck in this Pull Request. Posted by the CLA Assistant Lite bot. |
Contributor
There was a problem hiding this comment.
1 issue found across 2 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="core/tools/implementations/runTerminalCommand.vitest.ts">
<violation number="1" location="core/tools/implementations/runTerminalCommand.vitest.ts:162">
P1: Environment variable cleanup may leak SHELL state - assigning undefined to process.env creates string 'undefined'</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| expect(result[0].status).toBe("Command completed"); | ||
| expect(result[0].content).toContain("fallback shell works"); | ||
| } finally { | ||
| process.env.SHELL = originalShell; |
Contributor
There was a problem hiding this comment.
P1: Environment variable cleanup may leak SHELL state - assigning undefined to process.env creates string 'undefined'
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At core/tools/implementations/runTerminalCommand.vitest.ts, line 162:
<comment>Environment variable cleanup may leak SHELL state - assigning undefined to process.env creates string 'undefined'</comment>
<file context>
@@ -142,6 +142,27 @@ describe("runTerminalCommandImpl", () => {
+ expect(result[0].status).toBe("Command completed");
+ expect(result[0].content).toContain("fallback shell works");
+ } finally {
+ process.env.SHELL = originalShell;
+ }
+ });
</file context>
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.
Summary
SHELL,/bin/bash,/bin/sh, and/bin/ashSHELLpoints to a missing path or/bin/bashis not installedSHELLis invalidWhy
In remote/devcontainer setups, some environments do not expose the expected shell path. Falling back to an available shell prevents
spawn ... ENOENTfailures and keeps terminal tooling usable.Closes #11129
Summary by cubic
Gracefully fall back to an available Unix shell in runTerminalCommand when SHELL is invalid or /bin/bash is missing, preventing spawn ENOENT errors in remote/devcontainer environments. Fixes #11129.
Written for commit e93cef3. Summary will update on new commits.