Skip to content

Commit 4abf91e

Browse files
fix: isolate claude-code sessions via stream-json session_id (#7108)
Signed-off-by: Adrian Cole <adrian@tetrate.io>
1 parent 3a304c6 commit 4abf91e

3 files changed

Lines changed: 255 additions & 192 deletions

File tree

crates/goose/src/providers/claude_code.rs

Lines changed: 76 additions & 97 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@ struct CliProcess {
3131
reader: BufReader<tokio::process::ChildStdout>,
3232
#[allow(dead_code)]
3333
stderr_handle: tokio::task::JoinHandle<String>,
34-
messages_sent: usize,
3534
}
3635

3736
impl Drop for CliProcess {
@@ -56,23 +55,15 @@ pub struct ClaudeCodeProvider {
5655
}
5756

5857
impl ClaudeCodeProvider {
59-
pub async fn from_env(model: ModelConfig) -> Result<Self> {
60-
let config = crate::config::Config::global();
61-
let command: String = config.get_claude_code_command().unwrap_or_default().into();
62-
let resolved_command = SearchPaths::builder().with_npm().resolve(&command)?;
63-
64-
Ok(Self {
65-
command: resolved_command,
66-
model,
67-
name: CLAUDE_CODE_PROVIDER_NAME.to_string(),
68-
cli_process: tokio::sync::OnceCell::new(),
69-
})
70-
}
71-
72-
/// Build Anthropic content blocks from goose messages, supporting text and images.
73-
fn messages_to_content_blocks(&self, messages: &[Message]) -> Vec<Value> {
58+
/// Build content blocks from the last user message only — the CLI maintains
59+
/// conversation context internally per session_id.
60+
fn last_user_content_blocks(&self, messages: &[Message]) -> Vec<Value> {
61+
let msgs = match messages.iter().rev().find(|m| m.role == Role::User) {
62+
Some(msg) => std::slice::from_ref(msg),
63+
None => messages,
64+
};
7465
let mut blocks: Vec<Value> = Vec::new();
75-
for message in messages.iter().filter(|m| m.is_agent_visible()) {
66+
for message in msgs.iter().filter(|m| m.is_agent_visible()) {
7667
let prefix = match message.role {
7768
Role::User => "Human: ",
7869
Role::Assistant => "Assistant: ",
@@ -235,6 +226,9 @@ impl ClaudeCodeProvider {
235226

236227
// Combine all text content into a single message
237228
let combined_text = all_text_content.join("\n\n");
229+
if combined_text.contains("Prompt is too long") {
230+
return Err(ProviderError::ContextLengthExceeded(combined_text));
231+
}
238232
if combined_text.is_empty() {
239233
return Err(ProviderError::RequestFailed(
240234
"No text content found in response".to_string(),
@@ -257,6 +251,7 @@ impl ClaudeCodeProvider {
257251
system: &str,
258252
messages: &[Message],
259253
_tools: &[Tool],
254+
session_id: &str,
260255
) -> Result<Vec<String>, ProviderError> {
261256
let filtered_system = filter_extensions_from_system_prompt(system);
262257

@@ -331,25 +326,16 @@ impl ClaudeCodeProvider {
331326
stdin,
332327
reader: BufReader::new(stdout),
333328
stderr_handle,
334-
messages_sent: 0,
335329
}))
336330
})
337331
.await?;
338332

339333
let mut process = process_mutex.lock().await;
340334

341-
// Build content from new messages only (skip already-sent ones).
342-
// If messages is shorter than messages_sent, the caller started a fresh
343-
// conversation on the same provider instance — send everything.
344-
let new_messages = if process.messages_sent > 0 && process.messages_sent < messages.len() {
345-
&messages[process.messages_sent..]
346-
} else {
347-
messages
348-
};
349-
let new_blocks = self.messages_to_content_blocks(new_messages);
335+
let blocks = self.last_user_content_blocks(messages);
350336

351337
// Write NDJSON line to stdin
352-
let ndjson_line = build_stream_json_input(&new_blocks);
338+
let ndjson_line = build_stream_json_input(&blocks, session_id);
353339
process
354340
.stdin
355341
.write_all(ndjson_line.as_bytes())
@@ -399,9 +385,6 @@ impl ClaudeCodeProvider {
399385
}
400386
}
401387

402-
// Update messages_sent for next turn
403-
process.messages_sent = messages.len();
404-
405388
tracing::debug!("Command executed successfully, got {} lines", lines.len());
406389
for (i, line) in lines.iter().enumerate() {
407390
tracing::debug!("Line {}: {}", i, line);
@@ -456,8 +439,8 @@ impl ClaudeCodeProvider {
456439
}
457440
}
458441

459-
fn build_stream_json_input(content_blocks: &[Value]) -> String {
460-
let msg = json!({"type":"user","message":{"role":"user","content":content_blocks}});
442+
fn build_stream_json_input(content_blocks: &[Value], session_id: &str) -> String {
443+
let msg = json!({"type":"user","session_id":session_id,"message":{"role":"user","content":content_blocks}});
461444
serde_json::to_string(&msg).expect("serializing JSON content blocks cannot fail")
462445
}
463446

@@ -479,7 +462,18 @@ impl ProviderDef for ClaudeCodeProvider {
479462
}
480463

481464
fn from_env(model: ModelConfig) -> BoxFuture<'static, Result<Self::Provider>> {
482-
Box::pin(Self::from_env(model))
465+
Box::pin(async move {
466+
let config = crate::config::Config::global();
467+
let command: String = config.get_claude_code_command().unwrap_or_default().into();
468+
let resolved_command = SearchPaths::builder().with_npm().resolve(command)?;
469+
470+
Ok(Self {
471+
command: resolved_command,
472+
model,
473+
name: CLAUDE_CODE_PROVIDER_NAME.to_string(),
474+
cli_process: tokio::sync::OnceCell::new(),
475+
})
476+
})
483477
}
484478
}
485479

@@ -507,7 +501,7 @@ impl Provider for ClaudeCodeProvider {
507501
)]
508502
async fn complete_with_model(
509503
&self,
510-
_session_id: Option<&str>, // create_session == YYYYMMDD_N, but --session-id requires a UUID
504+
session_id: Option<&str>,
511505
model_config: &ModelConfig,
512506
system: &str,
513507
messages: &[Message],
@@ -518,7 +512,9 @@ impl Provider for ClaudeCodeProvider {
518512
return self.generate_simple_session_description(messages);
519513
}
520514

521-
let json_lines = self.execute_command(system, messages, tools).await?;
515+
// session_id is None before a session is created (e.g. model listing).
516+
let sid = session_id.unwrap_or("default");
517+
let json_lines = self.execute_command(system, messages, tools, sid).await?;
522518

523519
let (message, usage) = self.parse_claude_response(&json_lines)?;
524520

@@ -548,6 +544,7 @@ impl Provider for ClaudeCodeProvider {
548544
#[cfg(test)]
549545
mod tests {
550546
use super::*;
547+
use goose_test_support::session::TEST_SESSION_ID;
551548
use serde_json::json;
552549
use test_case::test_case;
553550

@@ -576,98 +573,75 @@ mod tests {
576573
}
577574

578575
#[test_case(
579-
&[],
576+
build_messages(&[]),
580577
&[]
581578
; "empty"
582579
)]
583580
#[test_case(
584-
&[("user", "Hello", None)],
581+
build_messages(&[("user", "Hello", None)]),
585582
&[json!({"type":"text","text":"Human: Hello"})]
586583
; "single_user"
587584
)]
588585
#[test_case(
589-
&[("user", "Hello", None), ("assistant", "Hi there!", None)],
590-
&[json!({"type":"text","text":"Human: Hello"}), json!({"type":"text","text":"Assistant: Hi there!"})]
591-
; "user_and_assistant"
586+
build_messages(&[("user", "Hello", None), ("assistant", "Hi there!", None)]),
587+
&[json!({"type":"text","text":"Human: Hello"})]
588+
; "picks_last_user_ignores_assistant"
592589
)]
593590
#[test_case(
594-
&[("user", "Describe this", Some(("base64data", "image/png")))],
591+
build_messages(&[("user", "First", None), ("assistant", "Reply", None), ("user", "Second", None)]),
592+
&[json!({"type":"text","text":"Human: Second"})]
593+
; "multi_turn_picks_last_user"
594+
)]
595+
#[test_case(
596+
build_messages(&[("user", "Describe this", Some(("base64data", "image/png")))]),
595597
&[json!({"type":"text","text":"Human: Describe this"}),
596598
json!({"type":"image","source":{"type":"base64","media_type":"image/png","data":"base64data"}})]
597599
; "user_with_image"
598600
)]
599601
#[test_case(
600-
&[("user", "", Some(("iVBORw0KGgo", "image/png")))],
602+
build_messages(&[("user", "", Some(("iVBORw0KGgo", "image/png")))]),
601603
&[json!({"type":"image","source":{"type":"base64","media_type":"image/png","data":"iVBORw0KGgo"}})]
602604
; "image_only"
603605
)]
604-
fn test_messages_to_content_blocks(pairs: &[MsgSpec], expected: &[Value]) {
606+
#[test_case(
607+
vec![Message::new(Role::Assistant, 0, vec![
608+
MessageContent::tool_request("call_123", Ok(rmcp::model::CallToolRequestParams {
609+
name: "developer__shell".into(),
610+
arguments: Some(serde_json::from_value(json!({"cmd": "ls"})).unwrap()),
611+
meta: None, task: None,
612+
}))
613+
])],
614+
&[json!({"type":"text","text":"Assistant: [tool_use: developer__shell id=call_123]"})]
615+
; "tool_request_no_user_fallback"
616+
)]
617+
#[test_case(
618+
vec![Message::new(Role::User, 0, vec![
619+
MessageContent::tool_response("call_123", Ok(rmcp::model::CallToolResult {
620+
content: vec![rmcp::model::Content::text("file1.txt\nfile2.txt")],
621+
is_error: None, structured_content: None, meta: None,
622+
}))
623+
])],
624+
&[json!({"type":"text","text":"Human: [tool_result id=call_123] file1.txt\nfile2.txt"})]
625+
; "tool_response"
626+
)]
627+
fn test_last_user_content_blocks(messages: Vec<Message>, expected: &[Value]) {
605628
let provider = make_provider();
606-
let messages = build_messages(pairs);
607-
let blocks = provider.messages_to_content_blocks(&messages);
629+
let blocks = provider.last_user_content_blocks(&messages);
608630
assert_eq!(blocks, expected);
609631
}
610632

611-
#[test]
612-
fn test_messages_to_content_blocks_tool_request() {
613-
use rmcp::model::CallToolRequestParams;
614-
let provider = make_provider();
615-
let tool_call = Ok(CallToolRequestParams {
616-
name: "developer__shell".into(),
617-
arguments: Some(serde_json::from_value(json!({"cmd": "ls"})).unwrap()),
618-
meta: None,
619-
task: None,
620-
});
621-
let msg = Message::new(
622-
Role::Assistant,
623-
0,
624-
vec![MessageContent::tool_request("call_123", tool_call)],
625-
);
626-
let blocks = provider.messages_to_content_blocks(&[msg]);
627-
assert_eq!(
628-
blocks,
629-
vec![
630-
json!({"type":"text","text":"Assistant: [tool_use: developer__shell id=call_123]"})
631-
]
632-
);
633-
}
634-
635-
#[test]
636-
fn test_messages_to_content_blocks_tool_response() {
637-
use rmcp::model::{CallToolResult, Content};
638-
let provider = make_provider();
639-
let result = CallToolResult {
640-
content: vec![Content::text("file1.txt\nfile2.txt")],
641-
is_error: None,
642-
structured_content: None,
643-
meta: None,
644-
};
645-
let msg = Message::new(
646-
Role::User,
647-
0,
648-
vec![MessageContent::tool_response("call_123", Ok(result))],
649-
);
650-
let blocks = provider.messages_to_content_blocks(&[msg]);
651-
assert_eq!(
652-
blocks,
653-
vec![
654-
json!({"type":"text","text":"Human: [tool_result id=call_123] file1.txt\nfile2.txt"})
655-
]
656-
);
657-
}
658-
659633
#[test_case(
660634
&[json!({"type":"text","text":"Hello"})],
661-
json!({"type":"user","message":{"role":"user","content":[{"type":"text","text":"Hello"}]}})
635+
json!({"type":"user","session_id":TEST_SESSION_ID,"message":{"role":"user","content":[{"type":"text","text":"Hello"}]}})
662636
; "text_block"
663637
)]
664638
#[test_case(
665639
&[json!({"type":"text","text":"Look"}), json!({"type":"image","source":{"type":"base64","media_type":"image/png","data":"abc"}})],
666-
json!({"type":"user","message":{"role":"user","content":[{"type":"text","text":"Look"},{"type":"image","source":{"type":"base64","media_type":"image/png","data":"abc"}}]}})
640+
json!({"type":"user","session_id":TEST_SESSION_ID,"message":{"role":"user","content":[{"type":"text","text":"Look"},{"type":"image","source":{"type":"base64","media_type":"image/png","data":"abc"}}]}})
667641
; "text_and_image_blocks"
668642
)]
669643
fn test_build_stream_json_input(blocks: &[Value], expected: Value) {
670-
let line = build_stream_json_input(blocks);
644+
let line = build_stream_json_input(blocks, TEST_SESSION_ID);
671645
let parsed: Value = serde_json::from_str(&line).unwrap();
672646
assert_eq!(parsed, expected);
673647
}
@@ -733,6 +707,11 @@ mod tests {
733707
ProviderError::RequestFailed("Claude CLI error: Model not supported".into())
734708
; "generic_error"
735709
)]
710+
#[test_case(
711+
&[r#"{"type":"assistant","message":{"role":"assistant","content":[{"type":"text","text":"Prompt is too long"}]}}"#],
712+
ProviderError::ContextLengthExceeded("Prompt is too long".into())
713+
; "prompt_too_long_exact"
714+
)]
736715
fn test_parse_claude_response_err(lines: &[&str], expected: ProviderError) {
737716
let provider = make_provider();
738717
let lines: Vec<String> = lines.iter().map(|s| s.to_string()).collect();

crates/goose/src/providers/codex.rs

Lines changed: 38 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -53,45 +53,6 @@ pub struct CodexProvider {
5353
}
5454

5555
impl CodexProvider {
56-
pub async fn from_env(model: ModelConfig) -> Result<Self> {
57-
let config = Config::global();
58-
let command: String = config.get_codex_command().unwrap_or_default().into();
59-
let resolved_command = SearchPaths::builder().with_npm().resolve(&command)?;
60-
61-
// Get reasoning effort from config, default to "high"
62-
let reasoning_effort = config
63-
.get_codex_reasoning_effort()
64-
.map(String::from)
65-
.unwrap_or_else(|_| "high".to_string());
66-
67-
// Validate reasoning effort
68-
let reasoning_effort =
69-
if Self::supports_reasoning_effort(&model.model_name, &reasoning_effort) {
70-
reasoning_effort
71-
} else {
72-
tracing::warn!(
73-
"Invalid CODEX_REASONING_EFFORT '{}' for model '{}', using 'high'",
74-
reasoning_effort,
75-
model.model_name
76-
);
77-
"high".to_string()
78-
};
79-
80-
// Get skip_git_check from config, default to false
81-
let skip_git_check = config
82-
.get_codex_skip_git_check()
83-
.map(|s| s.to_lowercase() == "true")
84-
.unwrap_or(false);
85-
86-
Ok(Self {
87-
command: resolved_command,
88-
model,
89-
name: CODEX_PROVIDER_NAME.to_string(),
90-
reasoning_effort,
91-
skip_git_check,
92-
})
93-
}
94-
9556
fn supports_reasoning_effort(model_name: &str, reasoning_effort: &str) -> bool {
9657
if !CODEX_REASONING_LEVELS.contains(&reasoning_effort) {
9758
return false;
@@ -600,7 +561,44 @@ impl ProviderDef for CodexProvider {
600561
}
601562

602563
fn from_env(model: ModelConfig) -> BoxFuture<'static, Result<Self::Provider>> {
603-
Box::pin(Self::from_env(model))
564+
Box::pin(async move {
565+
let config = Config::global();
566+
let command: String = config.get_codex_command().unwrap_or_default().into();
567+
let resolved_command = SearchPaths::builder().with_npm().resolve(command)?;
568+
569+
// Get reasoning effort from config, default to "high"
570+
let reasoning_effort = config
571+
.get_codex_reasoning_effort()
572+
.map(String::from)
573+
.unwrap_or_else(|_| "high".to_string());
574+
575+
// Validate reasoning effort
576+
let reasoning_effort =
577+
if Self::supports_reasoning_effort(&model.model_name, &reasoning_effort) {
578+
reasoning_effort
579+
} else {
580+
tracing::warn!(
581+
"Invalid CODEX_REASONING_EFFORT '{}' for model '{}', using 'high'",
582+
reasoning_effort,
583+
model.model_name
584+
);
585+
"high".to_string()
586+
};
587+
588+
// Get skip_git_check from config, default to false
589+
let skip_git_check = config
590+
.get_codex_skip_git_check()
591+
.map(|s| s.to_lowercase() == "true")
592+
.unwrap_or(false);
593+
594+
Ok(Self {
595+
command: resolved_command,
596+
model,
597+
name: CODEX_PROVIDER_NAME.to_string(),
598+
reasoning_effort,
599+
skip_git_check,
600+
})
601+
})
604602
}
605603
}
606604

0 commit comments

Comments
 (0)