mirror of
https://github.com/tinyhumansai/openhuman.git
synced 2026-07-27 21:08:00 +00:00
fix(inference): keep Claude CLI prompts out of Windows argv (#5103)
Co-authored-by: Sami Rusani <14844597+samrusani@users.noreply.github.com>
This commit is contained in:
Sam
@@ -2,7 +2,7 @@
|
||||
|
||||
use anyhow::Context;
|
||||
use async_trait::async_trait;
|
||||
use tokio::io::{AsyncBufReadExt, BufReader};
|
||||
use tokio::io::{AsyncBufReadExt, AsyncWriteExt, BufReader};
|
||||
use tokio::process::Command;
|
||||
use tokio::time::{timeout, Duration};
|
||||
|
||||
@@ -15,6 +15,45 @@ pub struct ClaudeAgentSdkProvider {
|
||||
pub(super) config: ClaudeAgentSdkConfig,
|
||||
}
|
||||
|
||||
struct ClaudeInvocation {
|
||||
args: Vec<String>,
|
||||
stdin: String,
|
||||
}
|
||||
|
||||
fn build_invocation(
|
||||
system_prompt: Option<&str>,
|
||||
message: &str,
|
||||
model: &str,
|
||||
max_budget_usd: Option<f64>,
|
||||
) -> ClaudeInvocation {
|
||||
let stdin = match system_prompt {
|
||||
Some(system) if !system.trim().is_empty() => {
|
||||
format!("[SYSTEM]\n{system}\n[/SYSTEM]\n\n{message}")
|
||||
}
|
||||
_ => message.to_string(),
|
||||
};
|
||||
let mut args = vec![
|
||||
"-p".to_string(),
|
||||
"--model".to_string(),
|
||||
model.to_string(),
|
||||
"--output-format".to_string(),
|
||||
"stream-json".to_string(),
|
||||
"--no-color".to_string(),
|
||||
];
|
||||
if let Some(budget) = max_budget_usd {
|
||||
args.push("--max-turns".to_string());
|
||||
args.push("10".to_string());
|
||||
args.push("--budget".to_string());
|
||||
args.push(format!("{budget:.4}"));
|
||||
}
|
||||
ClaudeInvocation { args, stdin }
|
||||
}
|
||||
|
||||
fn spawn_error(binary: &str, source: std::io::Error) -> anyhow::Error {
|
||||
let message = format!("failed to spawn claude binary '{binary}': {source}");
|
||||
anyhow::Error::new(source).context(message)
|
||||
}
|
||||
|
||||
impl ClaudeAgentSdkProvider {
|
||||
pub fn new(config: ClaudeAgentSdkConfig) -> Self {
|
||||
Self { config }
|
||||
@@ -36,42 +75,47 @@ impl Provider for ClaudeAgentSdkProvider {
|
||||
model
|
||||
};
|
||||
|
||||
// Prepend system prompt inline — claude -p has no separate system flag.
|
||||
let full_message = match system_prompt {
|
||||
Some(s) if !s.trim().is_empty() => {
|
||||
format!("[SYSTEM]\n{s}\n[/SYSTEM]\n\n{message}")
|
||||
}
|
||||
_ => message.to_string(),
|
||||
};
|
||||
// `claude -p` reads stdin in non-interactive mode. Keep the full
|
||||
// request out of argv so large harness prompts can spawn on Windows.
|
||||
let invocation =
|
||||
build_invocation(system_prompt, message, model, self.config.max_budget_usd);
|
||||
|
||||
let mut cmd = Command::new(&self.config.binary);
|
||||
cmd.arg("-p")
|
||||
.arg(&full_message)
|
||||
.arg("--model")
|
||||
.arg(model)
|
||||
.arg("--output-format")
|
||||
.arg("stream-json")
|
||||
.arg("--no-color")
|
||||
cmd.args(&invocation.args)
|
||||
.stdout(std::process::Stdio::piped())
|
||||
.stderr(std::process::Stdio::piped())
|
||||
.stdin(std::process::Stdio::null());
|
||||
|
||||
if let Some(budget) = self.config.max_budget_usd {
|
||||
cmd.arg("--max-turns").arg("10");
|
||||
// Note: --budget flag controls the spend cap in the Claude CLI
|
||||
cmd.arg("--budget").arg(format!("{budget:.4}"));
|
||||
}
|
||||
.stdin(std::process::Stdio::piped())
|
||||
.kill_on_drop(true);
|
||||
|
||||
tracing::debug!(
|
||||
"[claude_agent_sdk] spawning claude binary={} model={} message_len={}",
|
||||
self.config.binary,
|
||||
model,
|
||||
full_message.len()
|
||||
invocation.stdin.len()
|
||||
);
|
||||
|
||||
let mut child = cmd
|
||||
.spawn()
|
||||
.with_context(|| format!("failed to spawn claude binary '{}'", self.config.binary))?;
|
||||
let mut child = cmd.spawn().map_err(|source| {
|
||||
tracing::error!(
|
||||
error = %source,
|
||||
binary = %self.config.binary,
|
||||
"[claude_agent_sdk] failed to spawn claude binary"
|
||||
);
|
||||
spawn_error(&self.config.binary, source)
|
||||
})?;
|
||||
|
||||
let mut stdin = child
|
||||
.stdin
|
||||
.take()
|
||||
.context("claude subprocess has no stdin")?;
|
||||
stdin
|
||||
.write_all(invocation.stdin.as_bytes())
|
||||
.await
|
||||
.context("failed to write claude request to stdin")?;
|
||||
stdin
|
||||
.shutdown()
|
||||
.await
|
||||
.context("failed to close claude subprocess stdin")?;
|
||||
drop(stdin);
|
||||
|
||||
let stdout = child
|
||||
.stdout
|
||||
@@ -213,4 +257,112 @@ mod tests {
|
||||
assert!(!config.enabled);
|
||||
assert!(config.max_budget_usd.is_none());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn large_request_is_delivered_over_stdin_instead_of_argv() {
|
||||
let system_prompt = "system instruction\n".repeat(2_500);
|
||||
assert!(system_prompt.len() > 32_767);
|
||||
|
||||
let invocation = build_invocation(Some(&system_prompt), "hello", "claude-sonnet-4-6", None);
|
||||
|
||||
assert_eq!(
|
||||
invocation.args,
|
||||
[
|
||||
"-p",
|
||||
"--model",
|
||||
"claude-sonnet-4-6",
|
||||
"--output-format",
|
||||
"stream-json",
|
||||
"--no-color"
|
||||
]
|
||||
);
|
||||
assert!(!invocation
|
||||
.args
|
||||
.iter()
|
||||
.any(|arg| arg.contains(&system_prompt)));
|
||||
assert_eq!(
|
||||
invocation.stdin,
|
||||
format!("[SYSTEM]\n{system_prompt}\n[/SYSTEM]\n\nhello")
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn invocation_preserves_plain_message_and_budget_flags() {
|
||||
let invocation = build_invocation(None, "hello", "claude-opus-4-6", Some(1.25));
|
||||
|
||||
assert_eq!(invocation.stdin, "hello");
|
||||
assert_eq!(
|
||||
&invocation.args[6..],
|
||||
["--max-turns", "10", "--budget", "1.2500"]
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn spawn_error_message_includes_the_os_source() {
|
||||
let source = std::io::Error::from_raw_os_error(206);
|
||||
let error = spawn_error(r"C:\Users\test\.local\bin\claude.exe", source);
|
||||
|
||||
assert!(error.to_string().contains("os error 206"));
|
||||
assert_eq!(
|
||||
error.chain().count(),
|
||||
2,
|
||||
"io::Error source must be preserved"
|
||||
);
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
#[tokio::test]
|
||||
async fn provider_pipes_large_request_to_cli_stdin() {
|
||||
use std::os::unix::fs::PermissionsExt;
|
||||
|
||||
let dir = tempfile::tempdir().expect("tempdir");
|
||||
let script = dir.path().join("claude");
|
||||
std::fs::write(
|
||||
&script,
|
||||
r#"#!/bin/sh
|
||||
cat > "$0.stdin"
|
||||
printf '%s\n' '{"type":"result","result":"captured","is_error":false}'
|
||||
"#,
|
||||
)
|
||||
.expect("write fake claude");
|
||||
std::fs::set_permissions(&script, std::fs::Permissions::from_mode(0o700))
|
||||
.expect("make fake claude executable");
|
||||
|
||||
let mut config = ClaudeAgentSdkConfig::default();
|
||||
config.binary = script.display().to_string();
|
||||
let provider = ClaudeAgentSdkProvider::new(config);
|
||||
let system_prompt = "system instruction\n".repeat(2_500);
|
||||
|
||||
let output = provider
|
||||
.chat_with_system(Some(&system_prompt), "hello", "claude-sonnet-4-6", 0.0)
|
||||
.await
|
||||
.expect("fake claude response");
|
||||
|
||||
assert_eq!(output, "captured");
|
||||
assert_eq!(
|
||||
std::fs::read_to_string(format!("{}.stdin", script.display())).expect("captured stdin"),
|
||||
format!("[SYSTEM]\n{system_prompt}\n[/SYSTEM]\n\nhello")
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn provider_spawn_error_includes_the_os_source() {
|
||||
let dir = tempfile::tempdir().expect("tempdir");
|
||||
let mut config = ClaudeAgentSdkConfig::default();
|
||||
config.binary = dir.path().join("missing-claude").display().to_string();
|
||||
let provider = ClaudeAgentSdkProvider::new(config);
|
||||
|
||||
let error = provider
|
||||
.chat_with_system(None, "hello", "claude-sonnet-4-6", 0.0)
|
||||
.await
|
||||
.expect_err("missing binary must fail");
|
||||
|
||||
assert!(error.to_string().contains("failed to spawn claude binary"));
|
||||
assert!(error.to_string().contains("os error"));
|
||||
assert_eq!(
|
||||
error.chain().count(),
|
||||
2,
|
||||
"io::Error source must be preserved"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -186,6 +186,42 @@ fn write_mcp_http_config(
|
||||
Ok(path)
|
||||
}
|
||||
|
||||
/// Keep the potentially large harness prompt out of argv. Windows flattens
|
||||
/// argv into a command line capped at 32,767 UTF-16 code units, while Claude's
|
||||
/// file flag has no such limit. The per-turn scratch directory owns cleanup.
|
||||
fn append_system_prompt_args(
|
||||
dir: &std::path::Path,
|
||||
prompt: Option<&str>,
|
||||
) -> std::io::Result<Vec<String>> {
|
||||
let Some(prompt) = prompt.filter(|value| !value.trim().is_empty()) else {
|
||||
return Ok(Vec::new());
|
||||
};
|
||||
|
||||
let path = dir.join("append-system-prompt.txt");
|
||||
log::debug!(
|
||||
"[claude-code][driver] append-system-prompt file write start path={} bytes={}",
|
||||
path.display(),
|
||||
prompt.len()
|
||||
);
|
||||
if let Err(error) = std::fs::write(&path, prompt) {
|
||||
log::warn!(
|
||||
"[claude-code][driver] append-system-prompt file write failed path={} error={}",
|
||||
path.display(),
|
||||
error
|
||||
);
|
||||
return Err(error);
|
||||
}
|
||||
log::debug!(
|
||||
"[claude-code][driver] append-system-prompt file write complete path={} bytes={}",
|
||||
path.display(),
|
||||
prompt.len()
|
||||
);
|
||||
Ok(vec![
|
||||
"--append-system-prompt-file".to_string(),
|
||||
path.display().to_string(),
|
||||
])
|
||||
}
|
||||
|
||||
/// Run one turn against the `claude` CLI. Awaits process exit. Forwards
|
||||
/// `ProviderDelta`s through `ctx.stream` as they arrive and returns the
|
||||
/// aggregated `ChatResponse` when done.
|
||||
@@ -281,14 +317,10 @@ pub async fn run_turn(ctx: TurnContext<'_>) -> anyhow::Result<ChatResponse> {
|
||||
"--model".into(),
|
||||
ctx.model.clone(),
|
||||
];
|
||||
if let Some(sp) = ctx
|
||||
.append_system_prompt
|
||||
.as_ref()
|
||||
.filter(|s| !s.trim().is_empty())
|
||||
{
|
||||
args.push("--append-system-prompt".into());
|
||||
args.push(sp.clone());
|
||||
}
|
||||
args.extend(
|
||||
append_system_prompt_args(scratch.path(), ctx.append_system_prompt.as_deref())
|
||||
.map_err(|e| anyhow::anyhow!("write Claude Code system prompt file: {e}"))?,
|
||||
);
|
||||
if let Some(p) = mcp_config_path.as_ref() {
|
||||
args.push("--mcp-config".into());
|
||||
args.push(p.display().to_string());
|
||||
@@ -486,6 +518,44 @@ mod tests {
|
||||
assert!(server.get("command").is_none());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn large_system_prompt_is_written_to_file_instead_of_argv() {
|
||||
let dir = tempfile::tempdir().expect("tempdir");
|
||||
let prompt = "system instruction\n".repeat(2_500);
|
||||
assert!(prompt.len() > 32_767);
|
||||
|
||||
let args = append_system_prompt_args(dir.path(), Some(&prompt)).expect("prompt args");
|
||||
|
||||
assert_eq!(args[0], "--append-system-prompt-file");
|
||||
assert_eq!(args.len(), 2);
|
||||
assert!(!args.iter().any(|arg| arg.contains(&prompt)));
|
||||
assert_eq!(
|
||||
std::fs::read_to_string(&args[1]).expect("read prompt file"),
|
||||
prompt
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn empty_system_prompt_does_not_add_an_argument() {
|
||||
let dir = tempfile::tempdir().expect("tempdir");
|
||||
let args = append_system_prompt_args(dir.path(), Some(" \n ")).expect("prompt args");
|
||||
|
||||
assert!(args.is_empty());
|
||||
assert!(!dir.path().join("append-system-prompt.txt").exists());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn system_prompt_write_error_is_propagated() {
|
||||
let dir = tempfile::tempdir().expect("tempdir");
|
||||
let not_a_directory = dir.path().join("file");
|
||||
std::fs::write(¬_a_directory, "occupied").expect("write blocking file");
|
||||
|
||||
let error = append_system_prompt_args(¬_a_directory, Some("system prompt"))
|
||||
.expect_err("non-directory parent must fail");
|
||||
|
||||
assert!(!error.to_string().is_empty());
|
||||
}
|
||||
|
||||
#[cfg(target_os = "macos")]
|
||||
#[test]
|
||||
fn seatbelt_profile_denies_whole_openhuman_root_not_just_subdir() {
|
||||
|
||||
Reference in New Issue
Block a user