From fc7abd13f050a63ebfd9828a26000bd2b433be5a Mon Sep 17 00:00:00 2001 From: Nathan Flurry Date: Sun, 8 Feb 2026 11:59:43 -0800 Subject: [PATCH] fix: simplify opencode turn lifecycle handling --- .../sandbox-agent/src/opencode_compat.rs | 96 ++++++------------- server/packages/sandbox-agent/src/router.rs | 2 +- .../tests/opencode-compat/events.test.ts | 53 ++++++++++ .../tests/opencode-compat/session.test.ts | 37 +++---- 4 files changed, 102 insertions(+), 86 deletions(-) diff --git a/server/packages/sandbox-agent/src/opencode_compat.rs b/server/packages/sandbox-agent/src/opencode_compat.rs index e668fed..1852a20 100644 --- a/server/packages/sandbox-agent/src/opencode_compat.rs +++ b/server/packages/sandbox-agent/src/opencode_compat.rs @@ -671,12 +671,16 @@ struct OpenCodeUpdateSessionRequest { title: Option, #[schema(value_type = String)] model: Option, - #[serde(rename = "providerID", alias = "provider_id")] + #[serde(rename = "providerID", alias = "provider_id", alias = "providerId")] provider_id: Option, - #[serde(rename = "modelID", alias = "model_id")] + #[serde(rename = "modelID", alias = "model_id", alias = "modelId")] model_id: Option, } +fn update_requests_model_change(update: &OpenCodeUpdateSessionRequest) -> bool { + update.model.is_some() || update.provider_id.is_some() || update.model_id.is_some() +} + #[derive(Debug, Deserialize, IntoParams)] struct DirectoryQuery { directory: Option, @@ -2068,23 +2072,16 @@ async fn apply_universal_event(state: Arc, event: UniversalEve _ => None, }; let mut should_emit_idle = false; - let runtime = state + state .opencode .update_runtime(&event.session_id, |runtime| { let was_turn_in_progress = runtime.turn_in_progress; - if runtime.open_tool_calls.is_empty() { - runtime.active_assistant_message_id = None; - runtime.turn_in_progress = false; - should_emit_idle = was_turn_in_progress; - } else { - runtime.turn_in_progress = true; - should_emit_idle = false; - } + runtime.active_assistant_message_id = None; + runtime.turn_in_progress = false; + runtime.open_tool_calls.clear(); + should_emit_idle = was_turn_in_progress; }) .await; - if !runtime.open_tool_calls.is_empty() { - return; - } if let Some(turn_data) = turn_data { if let Some((message, details)) = turn_error_from_metadata(&turn_data.metadata) { emit_session_error(&state.opencode, &event.session_id, &message, None, details); @@ -2093,15 +2090,7 @@ async fn apply_universal_event(state: Arc, event: UniversalEve if !should_emit_idle { return; } - let session_id = event.session_id.clone(); - state.opencode.emit_event(json!({ - "type": "session.status", - "properties": {"sessionID": session_id, "status": {"type": "idle"}} - })); - state.opencode.emit_event(json!({ - "type": "session.idle", - "properties": {"sessionID": session_id} - })); + emit_session_idle(&state.opencode, &event.session_id); } UniversalEventType::ItemDelta => { if let UniversalEventData::ItemDelta(ItemDeltaData { @@ -2121,22 +2110,19 @@ async fn apply_universal_event(state: Arc, event: UniversalEve } } UniversalEventType::SessionEnded => { + let mut should_emit_idle = false; state .opencode .update_runtime(&event.session_id, |runtime| { + should_emit_idle = runtime.turn_in_progress; runtime.turn_in_progress = false; runtime.active_assistant_message_id = None; + runtime.open_tool_calls.clear(); }) .await; - let session_id = event.session_id.clone(); - state.opencode.emit_event(json!({ - "type": "session.status", - "properties": {"sessionID": session_id, "status": {"type": "idle"}} - })); - state.opencode.emit_event(json!({ - "type": "session.idle", - "properties": {"sessionID": event.session_id} - })); + if should_emit_idle { + emit_session_idle(&state.opencode, &event.session_id); + } } UniversalEventType::PermissionRequested | UniversalEventType::PermissionResolved => { if let UniversalEventData::Permission(permission) = &event.data { @@ -3857,30 +3843,14 @@ async fn oc_session_get( async fn oc_session_update( State(state): State>, Path(session_id): Path, - Json(body): Json, + Json(body): Json, ) -> impl IntoResponse { let mut sessions = state.opencode.sessions.lock().await; if let Some(session) = sessions.get_mut(&session_id) { - let requests_model_change = body - .as_object() - .map(|obj| { - obj.contains_key("model") - || obj.contains_key("providerID") - || obj.contains_key("modelID") - || obj.contains_key("provider_id") - || obj.contains_key("model_id") - || obj.contains_key("providerId") - || obj.contains_key("modelId") - }) - .unwrap_or(false); - if requests_model_change { + if update_requests_model_change(&body) { return bad_request(OPENCODE_MODEL_CHANGE_AFTER_SESSION_CREATE_ERROR).into_response(); } - if let Some(title) = body - .get("title") - .and_then(|value| value.as_str()) - .map(|value| value.to_string()) - { + if let Some(title) = body.title { if let Err(err) = state .inner .session_manager() @@ -4199,14 +4169,6 @@ async fn oc_session_message_create( .clone() .unwrap_or_else(|| next_id("msg_", &MESSAGE_COUNTER)); - state.opencode.emit_event(json!({ - "type": "session.status", - "properties": { - "sessionID": session_id, - "status": {"type": "busy"} - } - })); - let mut user_message = build_user_message( &session_id, &user_message_id, @@ -4243,7 +4205,6 @@ async fn oc_session_message_create( let _ = state .opencode .update_runtime(&session_id, |runtime| { - runtime.turn_in_progress = true; runtime.last_user_message_id = Some(user_message_id.clone()); runtime.active_assistant_message_id = None; runtime.last_agent = Some(agent_mode.clone()); @@ -4269,20 +4230,12 @@ async fn oc_session_message_create( ) .await { - let _ = state - .opencode - .update_runtime(&session_id, |runtime| { - runtime.turn_in_progress = false; - runtime.active_assistant_message_id = None; - }) - .await; tracing::warn!( target = "sandbox_agent::opencode", ?err, "failed to ensure backing session" ); emit_session_error(&state.opencode, &session_id, &err.to_string(), None, None); - emit_session_idle(&state.opencode, &session_id); return sandbox_error_response(err).into_response(); } else { ensure_session_stream(state.clone(), session_id.clone()).await; @@ -4300,11 +4253,14 @@ async fn oc_session_message_create( .send_message(session_id.clone(), prompt_text) .await { + let mut should_emit_idle = false; let _ = state .opencode .update_runtime(&session_id, |runtime| { + should_emit_idle = runtime.turn_in_progress; runtime.turn_in_progress = false; runtime.active_assistant_message_id = None; + runtime.open_tool_calls.clear(); }) .await; tracing::warn!( @@ -4313,7 +4269,9 @@ async fn oc_session_message_create( "failed to send message to backing agent" ); emit_session_error(&state.opencode, &session_id, &err.to_string(), None, None); - emit_session_idle(&state.opencode, &session_id); + if should_emit_idle { + emit_session_idle(&state.opencode, &session_id); + } return sandbox_error_response(err).into_response(); } } diff --git a/server/packages/sandbox-agent/src/router.rs b/server/packages/sandbox-agent/src/router.rs index ae9c014..ccf20b7 100644 --- a/server/packages/sandbox-agent/src/router.rs +++ b/server/packages/sandbox-agent/src/router.rs @@ -5321,7 +5321,7 @@ fn agent_capabilities_for(agent: AgentId) -> AgentCapabilities { session_lifecycle: true, error_events: true, reasoning: false, - status: true, + status: false, command_execution: false, file_changes: false, mcp_tools: false, diff --git a/server/packages/sandbox-agent/tests/opencode-compat/events.test.ts b/server/packages/sandbox-agent/tests/opencode-compat/events.test.ts index 44a577f..367e6e8 100644 --- a/server/packages/sandbox-agent/tests/opencode-compat/events.test.ts +++ b/server/packages/sandbox-agent/tests/opencode-compat/events.test.ts @@ -210,10 +210,63 @@ describe("OpenCode-compatible Event Streaming", () => { await collectIdle; expect(statuses).toContain("busy"); + expect(statuses.filter((status) => status === "busy")).toHaveLength(1); const finalStatus = await client.session.status(); expect(finalStatus.data?.[sessionId]?.type).toBe("idle"); }); + it("should report busy via /session/status while turn is in flight", async () => { + const sessionId = uniqueSessionId("status-busy-inflight"); + await initSessionViaHttp(sessionId, { providerID: "mock", modelID: "mock" }); + + const eventStream = await client.event.subscribe(); + let busySnapshot: string | undefined; + + const waitForIdle = new Promise((resolve, reject) => { + const timeout = setTimeout( + () => reject(new Error("Timed out waiting for busy status snapshot + session.idle")), + 15_000 + ); + (async () => { + try { + for await (const event of (eventStream as any).stream) { + if (event?.properties?.sessionID !== sessionId) continue; + + if (event.type === "session.status" && event?.properties?.status?.type === "busy" && !busySnapshot) { + for (let attempt = 0; attempt < 5; attempt += 1) { + const status = await client.session.status(); + busySnapshot = status.data?.[sessionId]?.type; + if (busySnapshot === "busy") { + break; + } + await new Promise((resolveAttempt) => setTimeout(resolveAttempt, 20)); + } + } + + if (event.type === "session.idle") { + clearTimeout(timeout); + resolve(); + break; + } + } + } catch { + // Stream ended + } + })(); + }); + + await client.session.prompt({ + path: { id: sessionId }, + body: { + model: { providerID: "mock", modelID: "mock" }, + parts: [{ type: "text", text: "tool" }], + }, + }); + + await waitForIdle; + expect(busySnapshot).toBe("busy"); + }); + it("should emit session.error and return idle for failed turns", async () => { const sessionId = uniqueSessionId("status-error"); await initSessionViaHttp(sessionId, { providerID: "mock", modelID: "mock" }); diff --git a/server/packages/sandbox-agent/tests/opencode-compat/session.test.ts b/server/packages/sandbox-agent/tests/opencode-compat/session.test.ts index 1aafa19..bdb1052 100644 --- a/server/packages/sandbox-agent/tests/opencode-compat/session.test.ts +++ b/server/packages/sandbox-agent/tests/opencode-compat/session.test.ts @@ -318,23 +318,28 @@ describe("OpenCode-compatible Session API", () => { const created = await client.session.create({ body: { title: "Original" } }); const sessionId = created.data?.id!; - const response = await fetch(`${handle.baseUrl}/opencode/session/${sessionId}`, { - method: "PATCH", - headers: { - Authorization: `Bearer ${handle.token}`, - "Content-Type": "application/json", - }, - body: JSON.stringify({ - providerID: "codex", - modelID: "gpt-5", - }), - }); - const data = await response.json(); + const payloads = [ + { providerID: "codex", modelID: "gpt-5" }, + { provider_id: "codex", model_id: "gpt-5" }, + { providerId: "codex", modelId: "gpt-5" }, + ]; - expect(response.status).toBe(400); - expect(data?.errors?.[0]?.message).toBe( - "OpenCode compatibility currently does not support changing the model after creating a session. Export with /export and load in to a new session." - ); + for (const payload of payloads) { + const response = await fetch(`${handle.baseUrl}/opencode/session/${sessionId}`, { + method: "PATCH", + headers: { + Authorization: `Bearer ${handle.token}`, + "Content-Type": "application/json", + }, + body: JSON.stringify(payload), + }); + const data = await response.json(); + + expect(response.status).toBe(400); + expect(data?.errors?.[0]?.message).toBe( + "OpenCode compatibility currently does not support changing the model after creating a session. Export with /export and load in to a new session." + ); + } }); });