harivansh-afk/sandbox-agent
1
0
Fork 0
mirror of https://github.com/harivansh-afk/sandbox-agent.git synced 2026-04-15 07:04:48 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions

fix: route Claude AskUserQuestion answers via permission control response (#127)

Browse source
This commit is contained in:
NathanFlurry 2026-02-07 06:43:45 +00:00
parent c8fd3aa382
commit bdf9b7cadd
No known key found for this signature in database
GPG key ID: 6A5F43A4F3241BCA
8 changed files with 252 additions and 273 deletions
Download patch file Download diff file Expand all files Collapse all files

9
server/packages/sandbox-agent/src/opencode_compat.rs
View file

@ -23,7 +23,9 @@ use tokio::sync::{broadcast, Mutex};
use tokio::time::interval;
use utoipa::{IntoParams, OpenApi, ToSchema};
use crate::router::{AgentModelInfo, AppState, CreateSessionRequest, PermissionReply};
use crate::router::{
is_question_tool_action, AgentModelInfo, AppState, CreateSessionRequest, PermissionReply,
};
use sandbox_agent_agent_management::agents::AgentId;
use sandbox_agent_error::SandboxError;
use sandbox_agent_universal_agent_schema::{
@ -1646,6 +1648,11 @@ async fn apply_permission_event(
event: UniversalEvent,
permission: PermissionEventData,
) {
// Suppress question-tool permissions (AskUserQuestion/ExitPlanMode) — these are
// handled internally via reply_question/reject_question, not exposed as permissions.
if is_question_tool_action(&permission.action) {
return;
}
let session_id = event.session_id.clone();
match permission.status {
PermissionStatus::Requested => {

213
server/packages/sandbox-agent/src/router.rs
View file

@ -690,6 +690,21 @@ impl SessionState {
self.update_pending(&event);
self.update_item_tracking(&event);
// Suppress question-tool permissions (AskUserQuestion/ExitPlanMode) from frontends.
// The permission is still stored in pending_permissions (via update_pending above)
// so reply_question/reject_question can find and resolve it internally.
if matches!(
event.event_type,
UniversalEventType::PermissionRequested | UniversalEventType::PermissionResolved
) {
if let UniversalEventData::Permission(ref data) = event.data {
if is_question_tool_action(&data.action) {
return None;
}
}
}
self.events.push(event.clone());
let _ = self.broadcaster.send(event.clone());
if self.native_session_id.is_none() {
@ -767,6 +782,17 @@ impl SessionState {
self.pending_permissions.remove(permission_id)
}
/// Find and remove a pending permission whose action matches a question tool
/// (AskUserQuestion or ExitPlanMode variants). Returns (permission_id, PendingPermission).
fn take_question_tool_permission(&mut self) -> Option<(String, PendingPermission)> {
let key = self
.pending_permissions
.iter()
.find(|(_, p)| is_question_tool_action(&p.action))
.map(|(k, _)| k.clone());
key.and_then(|k| self.pending_permissions.remove(&k).map(|p| (k, p)))
}
fn mark_ended(
&mut self,
exit_code: Option<i32>,
@ -2117,7 +2143,7 @@ impl SessionManager {
question_id: &str,
answers: Vec<Vec<String>>,
) -> Result<(), SandboxError> {
let (agent, native_session_id, pending_question, claude_sender) = {
let (agent, native_session_id, pending_question, claude_sender, linked_permission) = {
let mut sessions = self.sessions.lock().await;
let session = Self::session_mut(&mut sessions, session_id).ok_or_else(|| {
SandboxError::SessionNotFound {
@ -2133,11 +2159,18 @@ impl SessionManager {
if let Some(err) = session.ended_error() {
return Err(err);
}
// For Claude, check if there's a linked AskUserQuestion/ExitPlanMode permission
let linked_perm = if session.agent == AgentId::Claude {
session.take_question_tool_permission()
} else {
None
};
(
session.agent,
session.native_session_id.clone(),
pending,
session.claude_sender(),
linked_perm,
)
};
@ -2150,28 +2183,67 @@ impl SessionManager {
.ok_or_else(|| SandboxError::InvalidRequest {
message: "missing OpenCode session id".to_string(),
})?;
self.opencode_question_reply(&agent_session_id, question_id, answers)
self.opencode_question_reply(&agent_session_id, question_id, answers.clone())
.await?;
} else if agent == AgentId::Claude {
let sender = claude_sender.ok_or_else(|| SandboxError::InvalidRequest {
message: "Claude session is not active".to_string(),
})?;
let session_id = native_session_id
.clone()
.unwrap_or_else(|| session_id.to_string());
let response_text = response.clone().unwrap_or_default();
let line = claude_tool_result_line(&session_id, question_id, &response_text, false);
sender
.send(line)
.map_err(|_| SandboxError::InvalidRequest {
message: "Claude session is not active".to_string(),
})?;
if let Some((perm_id, perm)) = &linked_permission {
// Use the permission control response to deliver the answer.
// Build updatedInput from the original input with the answers map added.
let original_input = perm
.metadata
.as_ref()
.and_then(|m| m.get("input"))
.cloned()
.unwrap_or(Value::Null);
let mut updated = match original_input {
Value::Object(map) => map,
_ => serde_json::Map::new(),
};
// Build answers map: { "0": "selected option", "1": "another option", ... }
let answers_map: serde_json::Map<String, Value> = answers
.iter()
.enumerate()
.filter_map(|(i, inner)| {
inner
.first()
.map(|v| (i.to_string(), Value::String(v.clone())))
})
.collect();
updated.insert("answers".to_string(), Value::Object(answers_map));
let mut response_map = serde_json::Map::new();
response_map.insert("updatedInput".to_string(), Value::Object(updated));
let line =
claude_control_response_line(perm_id, "allow", Value::Object(response_map));
sender
.send(line)
.map_err(|_| SandboxError::InvalidRequest {
message: "Claude session is not active".to_string(),
})?;
} else {
// No linked permission — fall back to tool_result
let native_sid = native_session_id
.clone()
.unwrap_or_else(|| session_id.to_string());
let response_text = response.clone().unwrap_or_default();
let line =
claude_tool_result_line(&native_sid, question_id, &response_text, false);
sender
.send(line)
.map_err(|_| SandboxError::InvalidRequest {
message: "Claude session is not active".to_string(),
})?;
}
} else {
// TODO: Forward question replies to subprocess agents.
}
// Emit QuestionResolved
if let Some(pending) = pending_question {
let resolved = EventConversion::new(
let mut conversions = vec![EventConversion::new(
UniversalEventType::QuestionResolved,
UniversalEventData::Question(QuestionEventData {
question_id: question_id.to_string(),
@ -2182,8 +2254,26 @@ impl SessionManager {
}),
)
.synthetic()
.with_native_session(native_session_id);
let _ = self.record_conversions(session_id, vec![resolved]).await;
.with_native_session(native_session_id.clone())];
// Also emit PermissionResolved for the linked permission
if let Some((perm_id, perm)) = linked_permission {
conversions.push(
EventConversion::new(
UniversalEventType::PermissionResolved,
UniversalEventData::Permission(PermissionEventData {
permission_id: perm_id,
action: perm.action,
status: PermissionStatus::Approved,
metadata: perm.metadata,
}),
)
.synthetic()
.with_native_session(native_session_id),
);
}
let _ = self.record_conversions(session_id, conversions).await;
}
Ok(())
@ -2194,7 +2284,7 @@ impl SessionManager {
session_id: &str,
question_id: &str,
) -> Result<(), SandboxError> {
let (agent, native_session_id, pending_question, claude_sender) = {
let (agent, native_session_id, pending_question, claude_sender, linked_permission) = {
let mut sessions = self.sessions.lock().await;
let session = Self::session_mut(&mut sessions, session_id).ok_or_else(|| {
SandboxError::SessionNotFound {
@ -2210,11 +2300,17 @@ impl SessionManager {
if let Some(err) = session.ended_error() {
return Err(err);
}
let linked_perm = if session.agent == AgentId::Claude {
session.take_question_tool_permission()
} else {
None
};
(
session.agent,
session.native_session_id.clone(),
pending,
session.claude_sender(),
linked_perm,
)
};
@ -2231,26 +2327,43 @@ impl SessionManager {
let sender = claude_sender.ok_or_else(|| SandboxError::InvalidRequest {
message: "Claude session is not active".to_string(),
})?;
let session_id = native_session_id
.clone()
.unwrap_or_else(|| session_id.to_string());
let line = claude_tool_result_line(
&session_id,
question_id,
"User rejected the question.",
true,
);
sender
.send(line)
.map_err(|_| SandboxError::InvalidRequest {
message: "Claude session is not active".to_string(),
})?;
if let Some((perm_id, _)) = &linked_permission {
// Deny via the permission control response
let mut response_map = serde_json::Map::new();
response_map.insert(
"message".to_string(),
Value::String("Permission denied.".to_string()),
);
let line =
claude_control_response_line(perm_id, "deny", Value::Object(response_map));
sender
.send(line)
.map_err(|_| SandboxError::InvalidRequest {
message: "Claude session is not active".to_string(),
})?;
} else {
let native_sid = native_session_id
.clone()
.unwrap_or_else(|| session_id.to_string());
let line = claude_tool_result_line(
&native_sid,
question_id,
"User rejected the question.",
true,
);
sender
.send(line)
.map_err(|_| SandboxError::InvalidRequest {
message: "Claude session is not active".to_string(),
})?;
}
} else {
// TODO: Forward question rejections to subprocess agents.
}
// Emit QuestionResolved
if let Some(pending) = pending_question {
let resolved = EventConversion::new(
let mut conversions = vec![EventConversion::new(
UniversalEventType::QuestionResolved,
UniversalEventData::Question(QuestionEventData {
question_id: question_id.to_string(),
@ -2261,8 +2374,26 @@ impl SessionManager {
}),
)
.synthetic()
.with_native_session(native_session_id);
let _ = self.record_conversions(session_id, vec![resolved]).await;
.with_native_session(native_session_id.clone())];
// Also emit PermissionResolved for the linked permission
if let Some((perm_id, perm)) = linked_permission {
conversions.push(
EventConversion::new(
UniversalEventType::PermissionResolved,
UniversalEventData::Permission(PermissionEventData {
permission_id: perm_id,
action: perm.action,
status: PermissionStatus::Denied,
metadata: perm.metadata,
}),
)
.synthetic()
.with_native_session(native_session_id),
);
}
let _ = self.record_conversions(session_id, conversions).await;
}
Ok(())
@ -5076,6 +5207,22 @@ fn claude_control_response_line(request_id: &str, behavior: &str, response: Valu
.to_string()
}
/// Returns true if the given action name corresponds to a question tool
/// (AskUserQuestion or ExitPlanMode in any casing convention).
pub(crate) fn is_question_tool_action(action: &str) -> bool {
matches!(
action,
"AskUserQuestion"
| "ask_user_question"
| "askUserQuestion"
| "ask-user-question"
| "ExitPlanMode"
| "exit_plan_mode"
| "exitPlanMode"
| "exit-plan-mode"
)
}
fn read_lines<R: std::io::Read>(reader: R, sender: mpsc::UnboundedSender<String>) {
let mut reader = BufReader::new(reader);
let mut line = String::new();

65
server/packages/sandbox-agent/tests/sessions/snapshots/sessions__sessions__multi_turn__assert_session_snapshot@multi_turn_mock.snap.new
View file

@ -38,36 +38,13 @@ first:
status: in_progress
seq: 5
type: item.started
- delta:
delta: "<redacted>"
item_id: "<redacted>"
native_item_id: "<redacted>"
- item:
content_types: []
kind: message
role: assistant
status: completed
seq: 6
type: item.delta
- delta:
delta: "<redacted>"
item_id: "<redacted>"
native_item_id: "<redacted>"
seq: 7
type: item.delta
- delta:
delta: "<redacted>"
item_id: "<redacted>"
native_item_id: "<redacted>"
seq: 8
type: item.delta
- delta:
delta: "<redacted>"
item_id: "<redacted>"
native_item_id: "<redacted>"
seq: 9
type: item.delta
- delta:
delta: "<redacted>"
item_id: "<redacted>"
native_item_id: "<redacted>"
seq: 10
type: item.delta
type: item.completed
second:
- item:
content_types:
@ -105,27 +82,11 @@ second:
native_item_id: "<redacted>"
seq: 5
type: item.delta
- delta:
delta: "<redacted>"
item_id: "<redacted>"
native_item_id: "<redacted>"
- item:
content_types:
- text
kind: message
role: assistant
status: completed
seq: 6
type: item.delta
- delta:
delta: "<redacted>"
item_id: "<redacted>"
native_item_id: "<redacted>"
seq: 7
type: item.delta
- delta:
delta: "<redacted>"
item_id: "<redacted>"
native_item_id: "<redacted>"
seq: 8
type: item.delta
- delta:
delta: "<redacted>"
item_id: "<redacted>"
native_item_id: "<redacted>"
seq: 9
type: item.delta
type: item.completed

156
server/packages/sandbox-agent/tests/sessions/snapshots/sessions__sessions__permissions__assert_session_snapshot@permission_events_mock.snap.new
View file

@ -79,159 +79,3 @@ expression: value
native_item_id: "<redacted>"
seq: 12
type: item.delta
- delta:
delta: "<redacted>"
item_id: "<redacted>"
native_item_id: "<redacted>"
seq: 13
type: item.delta
- delta:
delta: "<redacted>"
item_id: "<redacted>"
native_item_id: "<redacted>"
seq: 14
type: item.delta
- delta:
delta: "<redacted>"
item_id: "<redacted>"
native_item_id: "<redacted>"
seq: 15
type: item.delta
- delta:
delta: "<redacted>"
item_id: "<redacted>"
native_item_id: "<redacted>"
seq: 16
type: item.delta
- delta:
delta: "<redacted>"
item_id: "<redacted>"
native_item_id: "<redacted>"
seq: 17
type: item.delta
- delta:
delta: "<redacted>"
item_id: "<redacted>"
native_item_id: "<redacted>"
seq: 18
type: item.delta
- delta:
delta: "<redacted>"
item_id: "<redacted>"
native_item_id: "<redacted>"
seq: 19
type: item.delta
- delta:
delta: "<redacted>"
item_id: "<redacted>"
native_item_id: "<redacted>"
seq: 20
type: item.delta
- delta:
delta: "<redacted>"
item_id: "<redacted>"
native_item_id: "<redacted>"
seq: 21
type: item.delta
- delta:
delta: "<redacted>"
item_id: "<redacted>"
native_item_id: "<redacted>"
seq: 22
type: item.delta
- delta:
delta: "<redacted>"
item_id: "<redacted>"
native_item_id: "<redacted>"
seq: 23
type: item.delta
- delta:
delta: "<redacted>"
item_id: "<redacted>"
native_item_id: "<redacted>"
seq: 24
type: item.delta
- delta:
delta: "<redacted>"
item_id: "<redacted>"
native_item_id: "<redacted>"
seq: 25
type: item.delta
- delta:
delta: "<redacted>"
item_id: "<redacted>"
native_item_id: "<redacted>"
seq: 26
type: item.delta
- delta:
delta: "<redacted>"
item_id: "<redacted>"
native_item_id: "<redacted>"
seq: 27
type: item.delta
- delta:
delta: "<redacted>"
item_id: "<redacted>"
native_item_id: "<redacted>"
seq: 28
type: item.delta
- delta:
delta: "<redacted>"
item_id: "<redacted>"
native_item_id: "<redacted>"
seq: 29
type: item.delta
- delta:
delta: "<redacted>"
item_id: "<redacted>"
native_item_id: "<redacted>"
seq: 30
type: item.delta
- delta:
delta: "<redacted>"
item_id: "<redacted>"
native_item_id: "<redacted>"
seq: 31
type: item.delta
- delta:
delta: "<redacted>"
item_id: "<redacted>"
native_item_id: "<redacted>"
seq: 32
type: item.delta
- delta:
delta: "<redacted>"
item_id: "<redacted>"
native_item_id: "<redacted>"
seq: 33
type: item.delta
- delta:
delta: "<redacted>"
item_id: "<redacted>"
native_item_id: "<redacted>"
seq: 34
type: item.delta
- delta:
delta: "<redacted>"
item_id: "<redacted>"
native_item_id: "<redacted>"
seq: 35
type: item.delta
- delta:
delta: "<redacted>"
item_id: "<redacted>"
native_item_id: "<redacted>"
seq: 36
type: item.delta
- delta:
delta: "<redacted>"
item_id: "<redacted>"
native_item_id: "<redacted>"
seq: 37
type: item.delta
- delta:
delta: "<redacted>"
item_id: "<redacted>"
native_item_id: "<redacted>"
seq: 38
type: item.delta

8
server/packages/sandbox-agent/tests/sessions/snapshots/sessions__sessions__questions__assert_session_snapshot@question_reply_events_mock.snap.new
View file

@ -91,15 +91,9 @@ expression: value
native_item_id: "<redacted>"
seq: 14
type: item.delta
- delta:
delta: "<redacted>"
item_id: "<redacted>"
native_item_id: "<redacted>"
seq: 15
type: item.delta
- question:
id: "<redacted>"
options: 4
status: requested
seq: 16
seq: 15
type: question.requested

18
server/packages/sandbox-agent/tests/sessions/snapshots/sessions__sessions__session_lifecycle__assert_session_snapshot@concurrency_events_mock.snap.new
View file

@ -44,24 +44,6 @@ session_a:
native_item_id: "<redacted>"
seq: 6
type: item.delta
- delta:
delta: "<redacted>"
item_id: "<redacted>"
native_item_id: "<redacted>"
seq: 7
type: item.delta
- delta:
delta: "<redacted>"
item_id: "<redacted>"
native_item_id: "<redacted>"
seq: 8
type: item.delta
- delta:
delta: "<redacted>"
item_id: "<redacted>"
native_item_id: "<redacted>"
seq: 9
type: item.delta
session_b:
- metadata: true
seq: 1

11
server/packages/sandbox-agent/tests/sessions/snapshots/sessions__sessions__session_lifecycle__run_http_events_snapshot@http_events_mock.snap.new
View file

@ -37,10 +37,9 @@ expression: normalized
status: in_progress
seq: 5
type: item.started
- item:
content_types: []
kind: message
role: assistant
status: completed
- delta:
delta: "<redacted>"
item_id: "<redacted>"
native_item_id: "<redacted>"
seq: 6
type: item.completed
type: item.delta

45
server/packages/sandbox-agent/tests/sessions/snapshots/sessions__sessions__session_lifecycle__run_sse_events_snapshot@sse_events_mock.snap.new Normal file
View file

@ -0,0 +1,45 @@
---
source: server/packages/sandbox-agent/tests/sessions/../common/http.rs
assertion_line: 1039
expression: normalized
---
- metadata: true
seq: 1
session: started
type: session.started
- item:
content_types:
- text
kind: message
role: user
status: in_progress
seq: 2
type: item.started
- delta:
delta: "<redacted>"
item_id: "<redacted>"
native_item_id: "<redacted>"
seq: 3
type: item.delta
- item:
content_types:
- text
kind: message
role: user
status: completed
seq: 4
type: item.completed
- item:
content_types:
- text
kind: message
role: assistant
status: in_progress
seq: 5
type: item.started
- delta:
delta: "<redacted>"
item_id: "<redacted>"
native_item_id: "<redacted>"
seq: 6
type: item.delta