Skip to content

Fix/replace all spawn with cross spawn#638

Open
blackmammoth wants to merge 2 commits intomainfrom
fix/replace-all-spawn-with-cross-spawn
Open

Fix/replace all spawn with cross spawn#638
blackmammoth wants to merge 2 commits intomainfrom
fix/replace-all-spawn-with-cross-spawn

Conversation

@blackmammoth
Copy link
Copy Markdown
Collaborator

@blackmammoth blackmammoth commented Apr 10, 2026

Summary by CodeRabbit

Release Notes

  • Chores
    • Standardized subprocess spawning implementation across multiple server modules to use a consistent subprocess handling approach for improved stability and maintainability.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Apr 10, 2026

📝 Walkthrough

Walkthrough

This pull request systematically replaces the Node.js built-in child_process module with the cross-spawn package across the server codebase. In addition, two CLI files (cursor-cli.js and gemini-cli.js) remove platform-specific conditional logic for process spawning, standardizing on cross-spawn uniformly.

Changes

Cohort / File(s) Summary
CLI Implementation Simplification
server/cursor-cli.js, server/gemini-cli.js
Removed platform-dependent process spawning logic (Windows vs. non-Windows conditionals) and now consistently use cross-spawn for launching child processes.
Server Core & Route Handlers
server/index.js, server/routes/agent.js, server/routes/cli-auth.js, server/routes/codex.js, server/routes/cursor.js, server/routes/git.js, server/routes/mcp.js, server/routes/projects.js, server/routes/taskmaster.js, server/routes/user.js
Updated spawn imports from Node's child_process to cross-spawn, replacing the underlying subprocess implementation provider while preserving all existing control flow and error handling.
Utility Modules
server/utils/gitConfig.js, server/utils/plugin-loader.js, server/utils/plugin-process-manager.js
Switched spawn import source from child_process to cross-spawn without altering helper function logic, process lifecycle management, or API signatures.

Poem

🐰 Cross-platform hops now all the same,
No Windows checks—cross-spawn's the game!
From cursor to codex, git to task,
One spawn to rule them all, no mask.
Simpler code, compatibility's flame! 🌟

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely summarizes the main change: replacing Node's built-in spawn with the cross-spawn package across multiple files.

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

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/replace-all-spawn-with-cross-spawn

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.

import path from 'path';
import os from 'os';
import { spawn } from 'child_process';
import { spawn } from 'cross-spawn';
import { fileURLToPath } from 'url';
import { dirname } from 'path';
import { spawn } from 'child_process';
import { spawn } from 'cross-spawn';
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@server/routes/mcp.js`:
- Line 7: The module-level import "import { spawn } from 'cross-spawn';" is
unused while each route handler still does "await import('child_process')";
update each route handler that currently uses "await import('child_process')" to
instead call the top-level "spawn" exported from 'cross-spawn' (replace child's
spawn/exec usages with the imported spawn), remove the dynamic import
statements, and ensure any options/argument arrays passed to the dynamic spawn
calls are adapted to cross-spawn's API; keep the single module-level "spawn"
import as the shared spawn implementation for all five handlers.

In `@server/routes/taskmaster.js`:
- Line 15: The route still uses child_process.spawn in one path, shadowing the
cross-spawn import; remove or replace the second import/require of spawn (the
shadowing instance) and ensure the /next/:projectName handler uses the top-level
imported spawn from 'cross-spawn' (the symbol imported at the file head) instead
of child_process.spawn—update any local variable names if needed and delete the
redundant child_process import so all spawn calls consistently use the
cross-spawn spawn function.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: b889c7e1-5652-46b3-a8e8-f276951145bf

📥 Commits

Reviewing files that changed from the base of the PR and between 9552577 and 3bbd56e.

📒 Files selected for processing (15)
  • server/cursor-cli.js
  • server/gemini-cli.js
  • server/index.js
  • server/routes/agent.js
  • server/routes/cli-auth.js
  • server/routes/codex.js
  • server/routes/cursor.js
  • server/routes/git.js
  • server/routes/mcp.js
  • server/routes/projects.js
  • server/routes/taskmaster.js
  • server/routes/user.js
  • server/utils/gitConfig.js
  • server/utils/plugin-loader.js
  • server/utils/plugin-process-manager.js

import { fileURLToPath } from 'url';
import { dirname } from 'path';
import { spawn } from 'child_process';
import { spawn } from 'cross-spawn';
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify this file no longer re-imports child_process spawn.
rg -n -C2 "await import\\('child_process'\\)|from 'child_process'|from \"child_process\"" server/routes/mcp.js

Repository: siteboon/claudecodeui

Length of output: 1090


Module-level cross-spawn import is unused; route handlers still dynamically import child_process.

The top-level cross-spawn import at line 7 is not referenced anywhere. All five route handlers at lines 20, 65, 181, 253, and 311 still import and use child_process directly via await import('child_process'), which defeats the standardization goal.

Proposed fix

Replace all local child_process imports with the module-level cross-spawn:

     const { spawn } = await import('child_process');
+    // use module-level cross-spawn import instead

(Apply same change at lines 20, 65, 181, 253, 311)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@server/routes/mcp.js` at line 7, The module-level import "import { spawn }
from 'cross-spawn';" is unused while each route handler still does "await
import('child_process')"; update each route handler that currently uses "await
import('child_process')" to instead call the top-level "spawn" exported from
'cross-spawn' (replace child's spawn/exec usages with the imported spawn),
remove the dynamic import statements, and ensure any options/argument arrays
passed to the dynamic spawn calls are adapted to cross-spawn's API; keep the
single module-level "spawn" import as the shared spawn implementation for all
five handlers.

import path from 'path';
import { promises as fsPromises } from 'fs';
import { spawn } from 'child_process';
import { spawn } from 'cross-spawn';
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify remaining child_process spawn imports in touched files.
rg -n -C2 "await import\\('child_process'\\)|from 'child_process'|from \"child_process\"" \
  server/routes/taskmaster.js \
  server/routes/mcp.js \
  server/index.js \
  server/cursor-cli.js \
  server/gemini-cli.js \
  server/utils/plugin-process-manager.js \
  server/routes/codex.js

Repository: siteboon/claudecodeui

Length of output: 1986


🏁 Script executed:

head -20 server/routes/taskmaster.js

Repository: siteboon/claudecodeui

Length of output: 722


🏁 Script executed:

sed -n '470,485p' server/routes/taskmaster.js

Repository: siteboon/claudecodeui

Length of output: 567


Incomplete migration: this route still bypasses cross-spawn in one code path.

Line 477 re-imports spawn from child_process, shadowing the cross-spawn import at line 15. The /next/:projectName endpoint therefore does not use the standardized spawn path.

🔧 Proposed fix
-            const { spawn } = await import('child_process');
-            
             const nextTaskCommand = spawn('task-master', ['next'], {
                 cwd: projectPath,
                 stdio: ['pipe', 'pipe', 'pipe']
             });
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@server/routes/taskmaster.js` at line 15, The route still uses
child_process.spawn in one path, shadowing the cross-spawn import; remove or
replace the second import/require of spawn (the shadowing instance) and ensure
the /next/:projectName handler uses the top-level imported spawn from
'cross-spawn' (the symbol imported at the file head) instead of
child_process.spawn—update any local variable names if needed and delete the
redundant child_process import so all spawn calls consistently use the
cross-spawn spawn function.

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