Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 8 additions & 7 deletions src/integrations/claude-code/__tests__/run.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ describe("runClaudeCode", () => {
expect(typeof result[Symbol.asyncIterator]).toBe("function")
})

test("should handle platform-specific stdin behavior", async () => {
test("should always pass system prompt via flag and messages via stdin", async () => {
Copy link
Author

Choose a reason for hiding this comment

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

Good update to the test name! This now accurately reflects that we're testing consistent behavior across all platforms rather than platform-specific behavior.

const { runClaudeCode } = await import("../run")
const messages = [{ role: "user" as const, content: "Hello world!" }]
const systemPrompt = "You are a helpful assistant"
Expand All @@ -158,12 +158,13 @@ describe("runClaudeCode", () => {
results.push(chunk)
}

// On Windows, should NOT have --system-prompt in args
// On Windows, should now ALWAYS have --system-prompt in args
const [, args] = mockExeca.mock.calls[0]
expect(args).not.toContain("--system-prompt")
expect(args).toContain("--system-prompt")
expect(args).toContain(systemPrompt)

// Should pass both system prompt and messages via stdin
const expectedStdinData = JSON.stringify({ systemPrompt, messages })
// Should only pass messages via stdin (no longer includes system prompt)
const expectedStdinData = JSON.stringify(messages)
expect(mockStdin.write).toHaveBeenCalledWith(expectedStdinData, "utf8", expect.any(Function))

// Reset mocks for non-Windows test
Expand All @@ -179,12 +180,12 @@ describe("runClaudeCode", () => {
results2.push(chunk)
}

// On non-Windows, should have --system-prompt in args
// On non-Windows, should also have --system-prompt in args
const [, args2] = mockExeca.mock.calls[0]
expect(args2).toContain("--system-prompt")
expect(args2).toContain(systemPrompt)

// Should only pass messages via stdin
// Should only pass messages via stdin (consistent across platforms)
expect(mockStdin.write).toHaveBeenCalledWith(JSON.stringify(messages), "utf8", expect.any(Function))
})

Expand Down
24 changes: 5 additions & 19 deletions src/integrations/claude-code/run.ts
Original file line number Diff line number Diff line change
Expand Up @@ -154,15 +154,9 @@ function runProcess({
maxOutputTokens,
}: ClaudeCodeOptions & { maxOutputTokens?: number }) {
const claudePath = path || "claude"
const isWindows = os.platform() === "win32"

// Build args based on platform
const args = ["-p"]

// Pass system prompt as flag on non-Windows, via stdin on Windows (avoids cmd length limits)
if (!isWindows) {
args.push("--system-prompt", systemPrompt)
}
// Build args - always pass system prompt as flag to avoid mixing with Claude Code's default
const args = ["-p", "--system-prompt", systemPrompt]
Copy link
Author

Choose a reason for hiding this comment

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

Could we expand this comment to be more explicit about the dual purpose? Something like:

Suggested change
const args = ["-p", "--system-prompt", systemPrompt]
// Build args - always pass system prompt as flag to ensure Claude Code replaces its default prompt entirely
// (avoids mixing prompts) while keeping argv small by sending messages via stdin (avoids E2BIG)


args.push(
"--verbose",
Expand Down Expand Up @@ -196,17 +190,9 @@ function runProcess({
timeout: CLAUDE_CODE_TIMEOUT,
})

// Prepare stdin data: Windows gets both system prompt & messages (avoids 8191 char limit),
// other platforms get messages only (avoids Linux E2BIG error from ~128KiB execve limit)
let stdinData: string
if (isWindows) {
stdinData = JSON.stringify({
systemPrompt,
messages,
})
} else {
stdinData = JSON.stringify(messages)
}
// Prepare stdin data: only send messages to avoid E2BIG errors
// System prompt is now always passed via --system-prompt flag
Copy link
Author

Choose a reason for hiding this comment

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

Great simplification! These comments clearly explain the rationale. Is the os import on line 7 still needed now that we've removed the platform-specific logic?

const stdinData = JSON.stringify(messages)

// Use setImmediate to ensure process is spawned before writing (prevents stdin race conditions)
setImmediate(() => {
Expand Down
Loading