-
Notifications
You must be signed in to change notification settings - Fork 2.2k
fix: pass system prompt via stdin for all platforms to fix Claude CLI compatibility #7146
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
… compatibility - Modified runProcess() to pass both system prompt and messages via stdin for all platforms - This fixes compatibility issues with Claude CLI versions newer than 1.0.58 - Avoids shell escaping issues and command-line length limits - Updated tests to reflect the new unified behavior Fixes #7145
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewing my own code is like debugging in a mirror - everything looks backwards but somehow still broken.
} | ||
// Pass both system prompt and messages via stdin for all platforms | ||
// This avoids shell escaping issues and command-line length limits | ||
const stdinData = JSON.stringify({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider adding a comment here explaining why we're using stdin for all platforms now. Something like:
This would help future maintainers (including future me) understand the design decision.
// Should only pass messages via stdin | ||
expect(mockStdin.write).toHaveBeenCalledWith(JSON.stringify(messages), "utf8", expect.any(Function)) | ||
// Should pass both system prompt and messages via stdin (same as Windows) | ||
expect(mockStdin.write).toHaveBeenCalledWith(expectedStdinData, "utf8", expect.any(Function)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good job updating the test to reflect the unified behavior. The test now correctly verifies that is not used on any platform and that both system prompt and messages are passed via stdin. Even I couldn't mess this up... wait, I wrote this.
This doesn't make any sense because that means that every macOS user would have been reporting this issue. I verify this on a clean install, actually I verify this on two different installs brand new MacBooks out of the box and when it's installing the latest version. Claude CLI it does not work. However when forcing the downgrade to v 1.0.58 it does work. |
Thank you for commenting on this @KnowhereFern, closing for now |
This PR fixes the compatibility issue with Claude CLI versions newer than 1.0.58.
Problem
Roo Code was only working with Claude CLI v1.0.58 and below. Newer versions were failing with exit code 1 due to how the system prompt was being passed as a command-line argument on non-Windows platforms.
Root Cause
--system-prompt
flagSolution
Modified the
runProcess()
function insrc/integrations/claude-code/run.ts
to pass both the system prompt and messages via stdin for all platforms, not just Windows. This approach:Changes
runProcess()
to use stdin for system prompt on all platformsTesting
Fixes #7145
Important
Pass system prompt via stdin for all platforms in
runProcess()
to fix Claude CLI compatibility issues.runProcess()
inrun.ts
now passes system prompt and messages via stdin for all platforms, fixing compatibility with Claude CLI versions newer than 1.0.58.run.spec.ts
to check that--system-prompt
is not in args for any platform.This description was created by
for 81fd53c. You can customize this summary. It will automatically update as commits are pushed.