Skip to content

Fix CLI launch: remove shell: true from spawn#197

Merged
Alan-Jowett merged 1 commit intomicrosoft:mainfrom
Alan-Jowett:fix-cli-shell-spawn
Apr 7, 2026
Merged

Fix CLI launch: remove shell: true from spawn#197
Alan-Jowett merged 1 commit intomicrosoft:mainfrom
Alan-Jowett:fix-cli-shell-spawn

Conversation

@Alan-Jowett
Copy link
Copy Markdown
Member

Problem


px @alan-jowett/promptkit\ fails with:

\
error: too many arguments. Expected 0 arguments but got 3.
\\

Plus a Node deprecation warning:
\
(node:8212) [DEP0190] DeprecationWarning: Passing args to a child process
with shell option true can lead to security vulnerabilities
\\

Root Cause

\spawn(cmd, args, { shell: true })\ in \launch.js\ causes the shell to re-parse the args array. The bootstrap prompt string (\Read and execute /path/to/bootstrap.md) contains spaces, so the shell splits it into multiple arguments. \copilot\ then receives \Read, \�nd, \�xecute\ as 3 separate args instead of the single -i\ value.

Fix

Remove \shell: true. \spawn\ with an args array handles argument passing correctly without a shell intermediary — each element of the array becomes exactly one argument.

Testing

Verified
ode bin/cli.js --version\ still works after the change.

The spawn call used shell: true which caused two issues:
1. Node DEP0190 deprecation warning about unescaped args
2. The bootstrap prompt string containing spaces was split by
   the shell into multiple arguments, causing copilot to receive
   'Read', 'and', 'execute' as 3 separate args instead of one
   -i value. This produced: 'too many arguments. Expected 0
   arguments but got 3.'

shell: true is not needed — spawn with an args array handles
argument quoting correctly without a shell intermediary.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings April 7, 2026 01:35
@Alan-Jowett Alan-Jowett merged commit f570a7b into microsoft:main Apr 7, 2026
3 checks passed
@Alan-Jowett Alan-Jowett deleted the fix-cli-shell-spawn branch April 7, 2026 01:36
Alan-Jowett pushed a commit to Alan-Jowett/PromptKit that referenced this pull request Apr 7, 2026
Hotfix release: fixes CLI launch failure caused by shell: true
in spawn (PR microsoft#197).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@Alan-Jowett Alan-Jowett mentioned this pull request Apr 7, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes PromptKit CLI interactive launch failures caused by spawning the target LLM CLI with shell: true, which re-parses and splits the bootstrap prompt (containing spaces) into multiple arguments.

Changes:

  • Remove shell: true from the spawn(cmd, args, …) call in launchInteractive() to preserve argument boundaries.
  • Eliminate the associated Node deprecation warning about passing args with shell: true.

Comment on lines 114 to 119
// All CLIs are spawned from the user's original directory so the LLM
// session reflects the directory the user was working in.
const child = spawn(cmd, args, {
cwd: originalCwd,
stdio: "inherit",
shell: true,
});
Copy link

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The regression fixed here (bootstrap prompt containing spaces being split into multiple argv entries) isn’t currently asserted by tests. Consider adding/adjusting launch tests to verify that the "-i" value (or final prompt argument) is passed as a single argument exactly equal to Read and execute <absolute path> so shell: true/splitting can’t reappear unnoticed.

Copilot uses AI. Check for mistakes.
Alan-Jowett added a commit that referenced this pull request Apr 7, 2026
Hotfix release: fixes CLI launch failure caused by shell: true
in spawn (PR #197).

Co-authored-by: Alan Jowett <alan.jowett@microsoft.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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.

2 participants