Uh oh!
There was an error while loading. Please reload this page.
refactor: replace Bun shell execution with portable Process utilities - #18318
Conversation
Use Process.lines() in MCP child process discovery and Process.text() in session prompt shell execution instead of Bun.spawn and Bun.$, making these codepaths work under Node.js.
Uh oh!
There was an error while loading. Please reload this page.
Greptile SummaryThis PR removes two direct Bun API dependencies (
Confidence Score: 3/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant Template as Template Engine<br/>(prompt.ts)
participant CM as ConfigMarkdown
participant Shell as Shell.preferred()
participant Process as Process.text()
participant SH as System Shell<br/>(sh/bash/fish/nu)
Template->>CM: shell(template)
CM-->>Template: shellMatches (array of [match, cmd])
Template->>Shell: preferred()
Shell-->>Template: sh (e.g. /bin/bash or fish)
loop For each cmd
Template->>Process: text([cmd], { shell: sh, nothrow: true })
Process->>SH: spawn(sh -c cmd)
SH-->>Process: stdout
Process-->>Template: TextResult { text }
end
Template->>Template: replace !`cmd` placeholders with results
Note over Template,SH: ⚠️ If sh is fish/nu, POSIX syntax in cmd will fail silently
Last reviewed commit: "refactor: replace Bu..." |
| const sh = Shell.preferred() | ||
| const results = await Promise.all( | ||
| shell.map(async ([, cmd]) => { | ||
| try { | ||
| return await $`${{ raw: cmd }}`.quiet().nothrow().text() | ||
| } catch (error) { | ||
| return `Error executing command: ${error instanceof Error ? error.message : String(error)}` | ||
| } | ||
| shellMatches.map(async ([, cmd]) => { | ||
| const out = await Process.text([cmd], { shell: sh, nothrow: true }) |
There was a problem hiding this comment.
Shell.preferred() may return incompatible shell for POSIX commands
The template shell commands (matched by !cmd`` syntax) are expected to be POSIX shell code. Using Shell.preferred() can return `fish` or `nu` — both of which are explicitly blacklisted in `Shell.acceptable()` because they are incompatible with POSIX syntax. When a user's `$SHELL` is set to fish or nu, the spawned process will receive POSIX shell syntax it cannot parse, and the failure will be silently swallowed by `nothrow: true`, leaving the template substitution as an empty string with no feedback.
bash.ts uses Shell.acceptable() for exactly this reason. This call should too.
| constsh=Shell.preferred() | |
| constresults=awaitPromise.all( | |
| shell.map(async([,cmd])=>{ | |
| try{ | |
| returnawait$`${{raw: cmd}}`.quiet().nothrow().text() | |
| }catch(error){ | |
| return`Error executing command: ${errorinstanceofError ? error.message : String(error)}` | |
| } | |
| shellMatches.map(async([,cmd])=>{ | |
| constout=awaitProcess.text([cmd],{shell: sh,nothrow: true}) | |
| constsh=Shell.acceptable() |
| shellMatches.map(async ([, cmd]) => { | ||
| const out = await Process.text([cmd], { shell: sh, nothrow: true }) | ||
| return out.text |
There was a problem hiding this comment.
Silent error swallowing on spawn failure
The old code had an explicit catch that returned "Error executing command: ${message}", making it visible in the template output when a command could not even be spawned. With nothrow: true, Process.run absorbs spawn errors and returns an empty stdout buffer, so out.text is "". The error is now completely silent — no mention of it appears in the rendered template.
Consider returning a fallback string on failure to preserve debuggability:
shellMatches.map(async([,cmd])=>{constout=awaitProcess.text([cmd],{shell: sh,nothrow: true})returnout.text||(out.code!==0 ? `Error executing command (exit ${out.code})` : "")}),
Summary
Bun.spawnwithProcess.lines()in MCP child process tree discoveryBun.$template literal shell execution withProcess.text()in session prompt shell commandsProcessutilities, removing direct Bun API dependencies