mirror of
https://github.com/harivansh-afk/sandbox-agent.git
synced 2026-04-20 00:02:19 +00:00
fix: simplify opencode turn lifecycle handling
This commit is contained in:
parent
3ba4c54c0c
commit
fc7abd13f0
4 changed files with 102 additions and 86 deletions
|
|
@ -671,12 +671,16 @@ struct OpenCodeUpdateSessionRequest {
|
||||||
title: Option<String>,
|
title: Option<String>,
|
||||||
#[schema(value_type = String)]
|
#[schema(value_type = String)]
|
||||||
model: Option<Value>,
|
model: Option<Value>,
|
||||||
#[serde(rename = "providerID", alias = "provider_id")]
|
#[serde(rename = "providerID", alias = "provider_id", alias = "providerId")]
|
||||||
provider_id: Option<String>,
|
provider_id: Option<String>,
|
||||||
#[serde(rename = "modelID", alias = "model_id")]
|
#[serde(rename = "modelID", alias = "model_id", alias = "modelId")]
|
||||||
model_id: Option<String>,
|
model_id: Option<String>,
|
||||||
}
|
}
|
||||||
|
|
||||||
|
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)]
|
#[derive(Debug, Deserialize, IntoParams)]
|
||||||
struct DirectoryQuery {
|
struct DirectoryQuery {
|
||||||
directory: Option<String>,
|
directory: Option<String>,
|
||||||
|
|
@ -2068,23 +2072,16 @@ async fn apply_universal_event(state: Arc<OpenCodeAppState>, event: UniversalEve
|
||||||
_ => None,
|
_ => None,
|
||||||
};
|
};
|
||||||
let mut should_emit_idle = false;
|
let mut should_emit_idle = false;
|
||||||
let runtime = state
|
state
|
||||||
.opencode
|
.opencode
|
||||||
.update_runtime(&event.session_id, |runtime| {
|
.update_runtime(&event.session_id, |runtime| {
|
||||||
let was_turn_in_progress = runtime.turn_in_progress;
|
let was_turn_in_progress = runtime.turn_in_progress;
|
||||||
if runtime.open_tool_calls.is_empty() {
|
runtime.active_assistant_message_id = None;
|
||||||
runtime.active_assistant_message_id = None;
|
runtime.turn_in_progress = false;
|
||||||
runtime.turn_in_progress = false;
|
runtime.open_tool_calls.clear();
|
||||||
should_emit_idle = was_turn_in_progress;
|
should_emit_idle = was_turn_in_progress;
|
||||||
} else {
|
|
||||||
runtime.turn_in_progress = true;
|
|
||||||
should_emit_idle = false;
|
|
||||||
}
|
|
||||||
})
|
})
|
||||||
.await;
|
.await;
|
||||||
if !runtime.open_tool_calls.is_empty() {
|
|
||||||
return;
|
|
||||||
}
|
|
||||||
if let Some(turn_data) = turn_data {
|
if let Some(turn_data) = turn_data {
|
||||||
if let Some((message, details)) = turn_error_from_metadata(&turn_data.metadata) {
|
if let Some((message, details)) = turn_error_from_metadata(&turn_data.metadata) {
|
||||||
emit_session_error(&state.opencode, &event.session_id, &message, None, details);
|
emit_session_error(&state.opencode, &event.session_id, &message, None, details);
|
||||||
|
|
@ -2093,15 +2090,7 @@ async fn apply_universal_event(state: Arc<OpenCodeAppState>, event: UniversalEve
|
||||||
if !should_emit_idle {
|
if !should_emit_idle {
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
let session_id = event.session_id.clone();
|
emit_session_idle(&state.opencode, &event.session_id);
|
||||||
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}
|
|
||||||
}));
|
|
||||||
}
|
}
|
||||||
UniversalEventType::ItemDelta => {
|
UniversalEventType::ItemDelta => {
|
||||||
if let UniversalEventData::ItemDelta(ItemDeltaData {
|
if let UniversalEventData::ItemDelta(ItemDeltaData {
|
||||||
|
|
@ -2121,22 +2110,19 @@ async fn apply_universal_event(state: Arc<OpenCodeAppState>, event: UniversalEve
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
UniversalEventType::SessionEnded => {
|
UniversalEventType::SessionEnded => {
|
||||||
|
let mut should_emit_idle = false;
|
||||||
state
|
state
|
||||||
.opencode
|
.opencode
|
||||||
.update_runtime(&event.session_id, |runtime| {
|
.update_runtime(&event.session_id, |runtime| {
|
||||||
|
should_emit_idle = runtime.turn_in_progress;
|
||||||
runtime.turn_in_progress = false;
|
runtime.turn_in_progress = false;
|
||||||
runtime.active_assistant_message_id = None;
|
runtime.active_assistant_message_id = None;
|
||||||
|
runtime.open_tool_calls.clear();
|
||||||
})
|
})
|
||||||
.await;
|
.await;
|
||||||
let session_id = event.session_id.clone();
|
if should_emit_idle {
|
||||||
state.opencode.emit_event(json!({
|
emit_session_idle(&state.opencode, &event.session_id);
|
||||||
"type": "session.status",
|
}
|
||||||
"properties": {"sessionID": session_id, "status": {"type": "idle"}}
|
|
||||||
}));
|
|
||||||
state.opencode.emit_event(json!({
|
|
||||||
"type": "session.idle",
|
|
||||||
"properties": {"sessionID": event.session_id}
|
|
||||||
}));
|
|
||||||
}
|
}
|
||||||
UniversalEventType::PermissionRequested | UniversalEventType::PermissionResolved => {
|
UniversalEventType::PermissionRequested | UniversalEventType::PermissionResolved => {
|
||||||
if let UniversalEventData::Permission(permission) = &event.data {
|
if let UniversalEventData::Permission(permission) = &event.data {
|
||||||
|
|
@ -3857,30 +3843,14 @@ async fn oc_session_get(
|
||||||
async fn oc_session_update(
|
async fn oc_session_update(
|
||||||
State(state): State<Arc<OpenCodeAppState>>,
|
State(state): State<Arc<OpenCodeAppState>>,
|
||||||
Path(session_id): Path<String>,
|
Path(session_id): Path<String>,
|
||||||
Json(body): Json<Value>,
|
Json(body): Json<OpenCodeUpdateSessionRequest>,
|
||||||
) -> impl IntoResponse {
|
) -> impl IntoResponse {
|
||||||
let mut sessions = state.opencode.sessions.lock().await;
|
let mut sessions = state.opencode.sessions.lock().await;
|
||||||
if let Some(session) = sessions.get_mut(&session_id) {
|
if let Some(session) = sessions.get_mut(&session_id) {
|
||||||
let requests_model_change = body
|
if update_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 {
|
|
||||||
return bad_request(OPENCODE_MODEL_CHANGE_AFTER_SESSION_CREATE_ERROR).into_response();
|
return bad_request(OPENCODE_MODEL_CHANGE_AFTER_SESSION_CREATE_ERROR).into_response();
|
||||||
}
|
}
|
||||||
if let Some(title) = body
|
if let Some(title) = body.title {
|
||||||
.get("title")
|
|
||||||
.and_then(|value| value.as_str())
|
|
||||||
.map(|value| value.to_string())
|
|
||||||
{
|
|
||||||
if let Err(err) = state
|
if let Err(err) = state
|
||||||
.inner
|
.inner
|
||||||
.session_manager()
|
.session_manager()
|
||||||
|
|
@ -4199,14 +4169,6 @@ async fn oc_session_message_create(
|
||||||
.clone()
|
.clone()
|
||||||
.unwrap_or_else(|| next_id("msg_", &MESSAGE_COUNTER));
|
.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(
|
let mut user_message = build_user_message(
|
||||||
&session_id,
|
&session_id,
|
||||||
&user_message_id,
|
&user_message_id,
|
||||||
|
|
@ -4243,7 +4205,6 @@ async fn oc_session_message_create(
|
||||||
let _ = state
|
let _ = state
|
||||||
.opencode
|
.opencode
|
||||||
.update_runtime(&session_id, |runtime| {
|
.update_runtime(&session_id, |runtime| {
|
||||||
runtime.turn_in_progress = true;
|
|
||||||
runtime.last_user_message_id = Some(user_message_id.clone());
|
runtime.last_user_message_id = Some(user_message_id.clone());
|
||||||
runtime.active_assistant_message_id = None;
|
runtime.active_assistant_message_id = None;
|
||||||
runtime.last_agent = Some(agent_mode.clone());
|
runtime.last_agent = Some(agent_mode.clone());
|
||||||
|
|
@ -4269,20 +4230,12 @@ async fn oc_session_message_create(
|
||||||
)
|
)
|
||||||
.await
|
.await
|
||||||
{
|
{
|
||||||
let _ = state
|
|
||||||
.opencode
|
|
||||||
.update_runtime(&session_id, |runtime| {
|
|
||||||
runtime.turn_in_progress = false;
|
|
||||||
runtime.active_assistant_message_id = None;
|
|
||||||
})
|
|
||||||
.await;
|
|
||||||
tracing::warn!(
|
tracing::warn!(
|
||||||
target = "sandbox_agent::opencode",
|
target = "sandbox_agent::opencode",
|
||||||
?err,
|
?err,
|
||||||
"failed to ensure backing session"
|
"failed to ensure backing session"
|
||||||
);
|
);
|
||||||
emit_session_error(&state.opencode, &session_id, &err.to_string(), None, None);
|
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();
|
return sandbox_error_response(err).into_response();
|
||||||
} else {
|
} else {
|
||||||
ensure_session_stream(state.clone(), session_id.clone()).await;
|
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)
|
.send_message(session_id.clone(), prompt_text)
|
||||||
.await
|
.await
|
||||||
{
|
{
|
||||||
|
let mut should_emit_idle = false;
|
||||||
let _ = state
|
let _ = state
|
||||||
.opencode
|
.opencode
|
||||||
.update_runtime(&session_id, |runtime| {
|
.update_runtime(&session_id, |runtime| {
|
||||||
|
should_emit_idle = runtime.turn_in_progress;
|
||||||
runtime.turn_in_progress = false;
|
runtime.turn_in_progress = false;
|
||||||
runtime.active_assistant_message_id = None;
|
runtime.active_assistant_message_id = None;
|
||||||
|
runtime.open_tool_calls.clear();
|
||||||
})
|
})
|
||||||
.await;
|
.await;
|
||||||
tracing::warn!(
|
tracing::warn!(
|
||||||
|
|
@ -4313,7 +4269,9 @@ async fn oc_session_message_create(
|
||||||
"failed to send message to backing agent"
|
"failed to send message to backing agent"
|
||||||
);
|
);
|
||||||
emit_session_error(&state.opencode, &session_id, &err.to_string(), None, None);
|
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();
|
return sandbox_error_response(err).into_response();
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -5321,7 +5321,7 @@ fn agent_capabilities_for(agent: AgentId) -> AgentCapabilities {
|
||||||
session_lifecycle: true,
|
session_lifecycle: true,
|
||||||
error_events: true,
|
error_events: true,
|
||||||
reasoning: false,
|
reasoning: false,
|
||||||
status: true,
|
status: false,
|
||||||
command_execution: false,
|
command_execution: false,
|
||||||
file_changes: false,
|
file_changes: false,
|
||||||
mcp_tools: false,
|
mcp_tools: false,
|
||||||
|
|
|
||||||
|
|
@ -210,10 +210,63 @@ describe("OpenCode-compatible Event Streaming", () => {
|
||||||
await collectIdle;
|
await collectIdle;
|
||||||
|
|
||||||
expect(statuses).toContain("busy");
|
expect(statuses).toContain("busy");
|
||||||
|
expect(statuses.filter((status) => status === "busy")).toHaveLength(1);
|
||||||
const finalStatus = await client.session.status();
|
const finalStatus = await client.session.status();
|
||||||
expect(finalStatus.data?.[sessionId]?.type).toBe("idle");
|
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<void>((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 () => {
|
it("should emit session.error and return idle for failed turns", async () => {
|
||||||
const sessionId = uniqueSessionId("status-error");
|
const sessionId = uniqueSessionId("status-error");
|
||||||
await initSessionViaHttp(sessionId, { providerID: "mock", modelID: "mock" });
|
await initSessionViaHttp(sessionId, { providerID: "mock", modelID: "mock" });
|
||||||
|
|
|
||||||
|
|
@ -318,23 +318,28 @@ describe("OpenCode-compatible Session API", () => {
|
||||||
const created = await client.session.create({ body: { title: "Original" } });
|
const created = await client.session.create({ body: { title: "Original" } });
|
||||||
const sessionId = created.data?.id!;
|
const sessionId = created.data?.id!;
|
||||||
|
|
||||||
const response = await fetch(`${handle.baseUrl}/opencode/session/${sessionId}`, {
|
const payloads = [
|
||||||
method: "PATCH",
|
{ providerID: "codex", modelID: "gpt-5" },
|
||||||
headers: {
|
{ provider_id: "codex", model_id: "gpt-5" },
|
||||||
Authorization: `Bearer ${handle.token}`,
|
{ providerId: "codex", modelId: "gpt-5" },
|
||||||
"Content-Type": "application/json",
|
];
|
||||||
},
|
|
||||||
body: JSON.stringify({
|
|
||||||
providerID: "codex",
|
|
||||||
modelID: "gpt-5",
|
|
||||||
}),
|
|
||||||
});
|
|
||||||
const data = await response.json();
|
|
||||||
|
|
||||||
expect(response.status).toBe(400);
|
for (const payload of payloads) {
|
||||||
expect(data?.errors?.[0]?.message).toBe(
|
const response = await fetch(`${handle.baseUrl}/opencode/session/${sessionId}`, {
|
||||||
"OpenCode compatibility currently does not support changing the model after creating a session. Export with /export and load in to a new session."
|
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."
|
||||||
|
);
|
||||||
|
}
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue