From e673a034dfe3533f2e4b60299e93bc6777ddd926 Mon Sep 17 00:00:00 2001 From: kevin9327 Date: Sun, 30 Aug 2026 19:38:39 +0900 Subject: [PATCH] fix: handle tool calls with empty arguments A tool call that carries no arguments streams no argument text, so `arguments_text` is empty and `from_str("")` fails with `EOF while parsing a value`, aborting the whole run. The model cycle already guards this, but two other consumers did not: - `ConversationOutput` re-parses the streamed text on `ToolCallEnd`; and - `create_tool_round` stored the empty text verbatim in the `arguments_json` column, so re-loading the round (`commit_tool_result` and the round loader) then failed on `from_str("")`. Treat empty argument text as an empty object in the output projection, and persist `{}` for it so the `arguments_json` column always holds valid JSON. Co-Authored-By: Claude Opus 4.8 --- server/src/cursor/conversation/output.rs | 9 +++- server/src/store/tool_rounds.rs | 9 +++- server/tests/interrupt.rs | 62 ++++++++++++++++++++++++ 3 files changed, 78 insertions(+), 2 deletions(-) diff --git a/server/src/cursor/conversation/output.rs b/server/src/cursor/conversation/output.rs index 54c4126..7fe905f 100644 --- a/server/src/cursor/conversation/output.rs +++ b/server/src/cursor/conversation/output.rs @@ -343,7 +343,14 @@ impl ConversationOutput { let call = calls.get_mut(&index).ok_or_else(|| { Error::Protocol(format!("unknown completed tool index: {index}")) })?; - call.arguments = serde_json::from_str(&call.arguments_text)?; + // A tool call with no arguments streams no argument text. + // Treat empty text as an empty object, matching the model + // cycle, instead of failing the run on `from_str("")`. + call.arguments = if call.arguments_text.trim().is_empty() { + serde_json::json!({}) + } else { + serde_json::from_str(&call.arguments_text)? + }; } RunEvent::Usage(usage) => { if !self.context.compacting { diff --git a/server/src/store/tool_rounds.rs b/server/src/store/tool_rounds.rs index 4418fb2..a7f3fc5 100644 --- a/server/src/store/tool_rounds.rs +++ b/server/src/store/tool_rounds.rs @@ -101,7 +101,14 @@ impl Store { .bind(&call.call_id) .bind(&call.model_call_id) .bind(&call.name) - .bind(&call.arguments_text) + // A no-argument tool call streams no argument text; persist it as an + // empty object so the `arguments_json` column always holds valid JSON + // and can be re-parsed on load. + .bind(if call.arguments_text.trim().is_empty() { + "{}" + } else { + call.arguments_text.as_str() + }) .execute(&mut *tx) .await?; } diff --git a/server/tests/interrupt.rs b/server/tests/interrupt.rs index d480a1f..1a19559 100644 --- a/server/tests/interrupt.rs +++ b/server/tests/interrupt.rs @@ -506,6 +506,68 @@ async fn runtime_user_message_action_interrupts_and_continues_with_new_message() assert!(history.contains("queued follow-up")); } +#[tokio::test] +async fn tool_call_with_empty_arguments_does_not_fail_the_run() { + // A tool call that carries no arguments streams no argument text. Parsing it + // as JSON must yield an empty object (as the model cycle already does), not + // fail the run with `EOF while parsing a value`. + let (_directory, store) = fixtures::temp_store().await; + let provider = fake_provider::FakeProvider::default(); + provider.push(tool_response("call-1", "UpdateCurrentStep", "")); + provider.push(text_response("done after empty-argument tool")); + let assets = PromptAssets::load( + std::path::Path::new(env!("CARGO_MANIFEST_DIR")) + .join("prompt/cursor") + .as_path(), + ) + .unwrap(); + let registry = TransportRegistry::new( + store, + Arc::new(provider.clone()), + PromptCompiler::new(assets), + ); + let handle = registry.get_or_create("empty-args-request").await.unwrap(); + let mut output = handle.subscribe(); + handle + .command(TransportCommand::Append { + seqno: 0, + message: Box::new(client_run_for( + "empty-args-request", + "empty-args-conversation", + )), + }) + .await + .unwrap(); + + let mut append_seqno = 1; + let mut saw_done = false; + loop { + let frame = tokio::time::timeout(std::time::Duration::from_secs(5), output.recv()) + .await + .unwrap() + .expect("RunSSE closed before successful EndStream"); + let (flags, payload) = connect::decode_frames(&frame).unwrap().pop().unwrap(); + if flags & connect::END_STREAM_FLAG != 0 { + assert_eq!( + payload.as_ref(), + b"{}", + "run failed: {}", + String::from_utf8_lossy(&payload) + ); + break; + } + let server = pb::AgentServerMessage::decode(payload).unwrap(); + if let Some(pb::agent_server_message::Message::InteractionUpdate(update)) = server.message { + if let Some(pb::interaction_update::Message::TextDelta(delta)) = update.message { + saw_done |= delta.text.contains("done after empty-argument tool"); + } + } + acknowledge_kv(&handle, &mut append_seqno, &frame).await; + } + assert!(saw_done); + assert_eq!(provider.requests().len(), 2); +} + #[tokio::test] async fn injected_user_context_restarts_only_the_active_model_cycle() { let (_directory, store) = fixtures::temp_store().await;