mirror of
https://github.com/RooVetGit/Roo-Code.git
synced 2026-09-07 08:26:51 +00:00
fix: replace CLAUDE_CODE_SYSTEM_PROMPT env var with temporary file approach
- Remove undocumented CLAUDE_CODE_SYSTEM_PROMPT environment variable usage - Implement temporary file solution for long system prompts (>7000 chars) - Use --system-prompt @filepath pattern for file-based input - Add proper cleanup for temporary files using vscode.workspace.fs - Fix parameter naming conflict between path module and claudeCodePath parameter - Update tests to verify temporary file usage and proper cleanup - Maintain backward compatibility for short system prompts using command line args Resolves Windows ENAMETOOLONG error without relying on undocumented features.
This commit is contained in:
Roo Code
parent
8dc50dc767
commit
d0f36c0f78
2 changed files with 111 additions and 34 deletions
|
|
@ -1,4 +1,4 @@
|
|||
import { describe, test, expect, vi, beforeEach } from "vitest"
|
||||
import { describe, test, expect, vi, beforeEach, afterEach } from "vitest"
|
||||
|
||||
// Mock vscode workspace
|
||||
vi.mock("vscode", () => ({
|
||||
|
|
@ -10,6 +10,13 @@ vi.mock("vscode", () => ({
|
|||
},
|
||||
},
|
||||
],
|
||||
fs: {
|
||||
writeFile: vi.fn().mockResolvedValue(undefined),
|
||||
delete: vi.fn().mockResolvedValue(undefined),
|
||||
},
|
||||
},
|
||||
Uri: {
|
||||
file: vi.fn((path: string) => ({ fsPath: path })),
|
||||
},
|
||||
}))
|
||||
|
||||
|
|
@ -86,6 +93,15 @@ vi.mock("readline", () => ({
|
|||
},
|
||||
}))
|
||||
|
||||
// Mock path and os modules
|
||||
vi.mock("path", () => ({
|
||||
join: vi.fn((...args: string[]) => args.join("/")),
|
||||
}))
|
||||
|
||||
vi.mock("os", () => ({
|
||||
tmpdir: vi.fn(() => "/tmp"),
|
||||
}))
|
||||
|
||||
describe("runClaudeCode", () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks()
|
||||
|
|
@ -313,7 +329,7 @@ describe("runClaudeCode", () => {
|
|||
expect(execaOptions.env?.CLAUDE_CODE_SYSTEM_PROMPT).toBeUndefined()
|
||||
})
|
||||
|
||||
test("should use environment variable for long system prompts to avoid Windows ENAMETOOLONG error", async () => {
|
||||
test("should use temporary file for long system prompts to avoid Windows ENAMETOOLONG error", async () => {
|
||||
const { runClaudeCode } = await import("../run")
|
||||
// Create a system prompt longer than MAX_COMMAND_LINE_LENGTH (7000 chars)
|
||||
const longSystemPrompt = "You are a helpful assistant. " + "A".repeat(7000)
|
||||
|
|
@ -330,13 +346,23 @@ describe("runClaudeCode", () => {
|
|||
// Clean up the generator
|
||||
await generator.return(undefined)
|
||||
|
||||
// Verify execa was called without --system-prompt in command line arguments
|
||||
// Verify execa was called with --system-prompt @filepath pattern
|
||||
const [, args, execaOptions] = mockExeca.mock.calls[0]
|
||||
expect(args).not.toContain("--system-prompt")
|
||||
expect(args).toContain("--system-prompt")
|
||||
|
||||
// Find the system prompt argument
|
||||
const systemPromptIndex = args.indexOf("--system-prompt")
|
||||
expect(systemPromptIndex).toBeGreaterThan(-1)
|
||||
const systemPromptArg = args[systemPromptIndex + 1]
|
||||
|
||||
// Verify it uses the @filepath pattern for temp files
|
||||
expect(systemPromptArg).toMatch(/^@.*claude-system-prompt-.*\.txt$/)
|
||||
|
||||
// Verify the long system prompt is not directly in the arguments
|
||||
expect(args).not.toContain(longSystemPrompt)
|
||||
|
||||
// Verify environment variable was set with the long system prompt
|
||||
expect(execaOptions.env?.CLAUDE_CODE_SYSTEM_PROMPT).toBe(longSystemPrompt)
|
||||
// Verify no environment variable was set for system prompt
|
||||
expect(execaOptions.env?.CLAUDE_CODE_SYSTEM_PROMPT).toBeUndefined()
|
||||
})
|
||||
|
||||
test("should handle exactly MAX_COMMAND_LINE_LENGTH system prompt using command line", async () => {
|
||||
|
|
@ -361,11 +387,13 @@ describe("runClaudeCode", () => {
|
|||
expect(args).toContain("--system-prompt")
|
||||
expect(args).toContain(exactLengthPrompt)
|
||||
|
||||
// Verify no environment variable was set
|
||||
expect(execaOptions.env?.CLAUDE_CODE_SYSTEM_PROMPT).toBeUndefined()
|
||||
// Verify no temporary file was used (no @ prefix)
|
||||
const systemPromptIndex = args.indexOf("--system-prompt")
|
||||
const systemPromptArg = args[systemPromptIndex + 1]
|
||||
expect(systemPromptArg).not.toMatch(/^@/)
|
||||
})
|
||||
|
||||
test("should handle system prompt one character over threshold using environment variable", async () => {
|
||||
test("should handle system prompt one character over threshold using temporary file", async () => {
|
||||
const { runClaudeCode } = await import("../run")
|
||||
// Create a system prompt one character over the threshold (7001 chars)
|
||||
const overThresholdPrompt = "A".repeat(7001)
|
||||
|
|
@ -382,16 +410,25 @@ describe("runClaudeCode", () => {
|
|||
// Clean up the generator
|
||||
await generator.return(undefined)
|
||||
|
||||
// Verify execa was called without --system-prompt in command line arguments
|
||||
// Verify execa was called with --system-prompt @filepath pattern
|
||||
const [, args, execaOptions] = mockExeca.mock.calls[0]
|
||||
expect(args).not.toContain("--system-prompt")
|
||||
expect(args).toContain("--system-prompt")
|
||||
|
||||
// Find the system prompt argument
|
||||
const systemPromptIndex = args.indexOf("--system-prompt")
|
||||
const systemPromptArg = args[systemPromptIndex + 1]
|
||||
|
||||
// Verify it uses the @filepath pattern for temp files
|
||||
expect(systemPromptArg).toMatch(/^@.*claude-system-prompt-.*\.txt$/)
|
||||
|
||||
// Verify the long system prompt is not directly in the arguments
|
||||
expect(args).not.toContain(overThresholdPrompt)
|
||||
|
||||
// Verify environment variable was set
|
||||
expect(execaOptions.env?.CLAUDE_CODE_SYSTEM_PROMPT).toBe(overThresholdPrompt)
|
||||
// Verify no environment variable was set
|
||||
expect(execaOptions.env?.CLAUDE_CODE_SYSTEM_PROMPT).toBeUndefined()
|
||||
})
|
||||
|
||||
test("should preserve existing environment variables when using CLAUDE_CODE_SYSTEM_PROMPT", async () => {
|
||||
test("should preserve existing environment variables when using temporary files", async () => {
|
||||
const { runClaudeCode } = await import("../run")
|
||||
|
||||
// Mock process.env to have some existing variables
|
||||
|
|
@ -416,14 +453,16 @@ describe("runClaudeCode", () => {
|
|||
// Clean up the generator
|
||||
await generator.return(undefined)
|
||||
|
||||
// Verify environment variables include both existing and new ones
|
||||
// Verify environment variables include existing ones but no CLAUDE_CODE_SYSTEM_PROMPT
|
||||
const [, , execaOptions] = mockExeca.mock.calls[0]
|
||||
expect(execaOptions.env).toEqual({
|
||||
...process.env,
|
||||
CLAUDE_CODE_MAX_OUTPUT_TOKENS: expect.any(String), // Always set by the implementation
|
||||
CLAUDE_CODE_SYSTEM_PROMPT: longSystemPrompt,
|
||||
})
|
||||
|
||||
// Verify no system prompt environment variable was set
|
||||
expect(execaOptions.env?.CLAUDE_CODE_SYSTEM_PROMPT).toBeUndefined()
|
||||
|
||||
// Restore original environment
|
||||
process.env = originalEnv
|
||||
})
|
||||
|
|
@ -448,7 +487,9 @@ describe("runClaudeCode", () => {
|
|||
expect(args).toContain("--system-prompt")
|
||||
expect(args).toContain("")
|
||||
|
||||
// Verify no environment variable was set
|
||||
expect(execaOptions.env?.CLAUDE_CODE_SYSTEM_PROMPT).toBeUndefined()
|
||||
// Verify no temporary file was used (no @ prefix)
|
||||
const systemPromptIndex = args.indexOf("--system-prompt")
|
||||
const systemPromptArg = args[systemPromptIndex + 1]
|
||||
expect(systemPromptArg).not.toMatch(/^@/)
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -4,6 +4,8 @@ import { execa } from "execa"
|
|||
import { ClaudeCodeMessage } from "./types"
|
||||
import readline from "readline"
|
||||
import { CLAUDE_CODE_DEFAULT_MAX_OUTPUT_TOKENS } from "@roo-code/types"
|
||||
import * as path from "path"
|
||||
import * as os from "os"
|
||||
|
||||
const cwd = vscode.workspace.workspaceFolders?.map((folder) => folder.uri.fsPath).at(0)
|
||||
|
||||
|
|
@ -21,10 +23,15 @@ type ProcessState = {
|
|||
exitCode: number | null
|
||||
}
|
||||
|
||||
type TempFileCleanup = {
|
||||
filePath: string
|
||||
cleanup: () => Promise<void>
|
||||
}
|
||||
|
||||
export async function* runClaudeCode(
|
||||
options: ClaudeCodeOptions & { maxOutputTokens?: number },
|
||||
): AsyncGenerator<ClaudeCodeMessage | string> {
|
||||
const process = runProcess(options)
|
||||
const { process, tempFileCleanup } = await runProcess(options)
|
||||
|
||||
const rl = readline.createInterface({
|
||||
input: process.stdout,
|
||||
|
|
@ -84,6 +91,10 @@ export async function* runClaudeCode(
|
|||
if (!process.killed) {
|
||||
process.kill()
|
||||
}
|
||||
// Clean up temporary file if it was created
|
||||
if (tempFileCleanup) {
|
||||
await tempFileCleanup.cleanup()
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -111,25 +122,55 @@ const claudeCodeTools = [
|
|||
const CLAUDE_CODE_TIMEOUT = 600000 // 10 minutes
|
||||
|
||||
// Windows has a command line length limit of ~8191 characters
|
||||
// If the system prompt is too long, we'll use an environment variable instead
|
||||
// If the system prompt is too long, we'll write it to a temporary file instead
|
||||
const MAX_COMMAND_LINE_LENGTH = 7000 // Conservative limit to account for other arguments
|
||||
|
||||
function runProcess({
|
||||
async function runProcess({
|
||||
systemPrompt,
|
||||
messages,
|
||||
path,
|
||||
path: claudeCodePath,
|
||||
modelId,
|
||||
maxOutputTokens,
|
||||
}: ClaudeCodeOptions & { maxOutputTokens?: number }) {
|
||||
const claudePath = path || "claude"
|
||||
}: ClaudeCodeOptions & { maxOutputTokens?: number }): Promise<{
|
||||
process: ReturnType<typeof execa>
|
||||
tempFileCleanup: TempFileCleanup | null
|
||||
}> {
|
||||
const claudePath = claudeCodePath || "claude"
|
||||
|
||||
// Check if system prompt is too long for command line
|
||||
const useEnvForSystemPrompt = systemPrompt.length > MAX_COMMAND_LINE_LENGTH
|
||||
const useTempFileForSystemPrompt = systemPrompt.length > MAX_COMMAND_LINE_LENGTH
|
||||
|
||||
const args = ["-p"]
|
||||
let tempFileCleanup: TempFileCleanup | null = null
|
||||
|
||||
// Only add --system-prompt to command line if it's short enough
|
||||
if (!useEnvForSystemPrompt) {
|
||||
// Handle system prompt - use temp file for long prompts, command line for short ones
|
||||
if (useTempFileForSystemPrompt) {
|
||||
// Create temporary file for system prompt
|
||||
const tempFilePath = path.join(
|
||||
os.tmpdir(),
|
||||
`claude-system-prompt-${Date.now()}-${Math.random().toString(36).substring(2)}.txt`,
|
||||
)
|
||||
|
||||
try {
|
||||
await vscode.workspace.fs.writeFile(vscode.Uri.file(tempFilePath), Buffer.from(systemPrompt, "utf8"))
|
||||
args.push("--system-prompt", `@${tempFilePath}`)
|
||||
|
||||
tempFileCleanup = {
|
||||
filePath: tempFilePath,
|
||||
cleanup: async () => {
|
||||
try {
|
||||
await vscode.workspace.fs.delete(vscode.Uri.file(tempFilePath))
|
||||
} catch (error) {
|
||||
// Ignore cleanup errors - temp files will be cleaned up by OS eventually
|
||||
console.warn(`Failed to clean up temporary system prompt file ${tempFilePath}:`, error)
|
||||
}
|
||||
},
|
||||
}
|
||||
} catch (error) {
|
||||
throw new Error(`Failed to create temporary file for system prompt: ${error}`)
|
||||
}
|
||||
} else {
|
||||
// Use command line argument for short system prompts
|
||||
args.push("--system-prompt", systemPrompt)
|
||||
}
|
||||
|
||||
|
|
@ -157,11 +198,6 @@ function runProcess({
|
|||
CLAUDE_CODE_DEFAULT_MAX_OUTPUT_TOKENS.toString(),
|
||||
}
|
||||
|
||||
// If system prompt is too long, pass it via environment variable
|
||||
if (useEnvForSystemPrompt) {
|
||||
env.CLAUDE_CODE_SYSTEM_PROMPT = systemPrompt
|
||||
}
|
||||
|
||||
const child = execa(claudePath, args, {
|
||||
stdin: "pipe",
|
||||
stdout: "pipe",
|
||||
|
|
@ -176,7 +212,7 @@ function runProcess({
|
|||
// This avoids the E2BIG error on Linux and ENAMETOOLONG error on Windows when passing large data as command line arguments
|
||||
// Linux has a per-argument limit of ~128KiB for execve() system calls
|
||||
// Windows has a total command line length limit of ~8191 characters
|
||||
// For system prompts, we use environment variables when they exceed the safe limit
|
||||
// For system prompts, we use temporary files when they exceed the safe limit
|
||||
const messagesJson = JSON.stringify(messages)
|
||||
|
||||
// Use setImmediate to ensure the process has been spawned before writing to stdin
|
||||
|
|
@ -196,7 +232,7 @@ function runProcess({
|
|||
}
|
||||
})
|
||||
|
||||
return child
|
||||
return { process: child, tempFileCleanup }
|
||||
}
|
||||
|
||||
function parseChunk(data: string, processState: ProcessState) {
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue