mirror of
https://github.com/harivansh-afk/sandbox-agent.git
synced 2026-04-18 19:03:48 +00:00
fix: fix missing permissions handler
This commit is contained in:
parent
267269db90
commit
7f9460bbbb
3 changed files with 240 additions and 19 deletions
|
|
@ -247,3 +247,13 @@ Update this file continuously during the migration.
|
||||||
- Owner: Unassigned.
|
- Owner: Unassigned.
|
||||||
- Status: in_progress
|
- Status: in_progress
|
||||||
- Links: `research/acp/simplify-server.md`, `docs/mcp-config.mdx`, `docs/skills-config.mdx`
|
- Links: `research/acp/simplify-server.md`, `docs/mcp-config.mdx`, `docs/skills-config.mdx`
|
||||||
|
|
||||||
|
- Date: 2026-02-12
|
||||||
|
- Area: OpenCode compat old-session prompt behavior
|
||||||
|
- Issue: Prompting historical sessions could return immediate `400` (`MODEL_CHANGE_ERROR`) when the client sent a newer default model (for example `gpt-5.3-codex`) than the session's persisted model (`gpt-5.2-codex`), which appeared as a no-op in Gigacode.
|
||||||
|
- Impact: Users could not continue old sessions after daemon/model-default changes, despite using the same provider/agent.
|
||||||
|
- Proposed direction: Keep strict model-change rejection for active sessions, but allow same-agent/same-provider model rebinding for stale sessions (where `last_connection_id` differs) during `POST /opencode/session/{id}/message`.
|
||||||
|
- Decision: Accepted and implemented.
|
||||||
|
- Owner: Unassigned.
|
||||||
|
- Status: resolved
|
||||||
|
- Links: `server/packages/opencode-adapter/src/lib.rs`
|
||||||
|
|
|
||||||
|
|
@ -91,6 +91,11 @@ pub struct OpenCodeAdapterConfig {
|
||||||
/// Optional pre-built provider payload for `/provider` and `/config/providers`.
|
/// Optional pre-built provider payload for `/provider` and `/config/providers`.
|
||||||
/// When `None`, falls back to the hardcoded mock/amp/claude/codex list.
|
/// When `None`, falls back to the hardcoded mock/amp/claude/codex list.
|
||||||
pub provider_payload: Option<Value>,
|
pub provider_payload: Option<Value>,
|
||||||
|
/// Optional display name for `/agent` metadata. Defaults to "Sandbox Agent".
|
||||||
|
pub agent_display_name: Option<String>,
|
||||||
|
/// Optional description for `/agent` metadata. Defaults to
|
||||||
|
/// "Sandbox Agent compatibility layer".
|
||||||
|
pub agent_description: Option<String>,
|
||||||
}
|
}
|
||||||
|
|
||||||
impl Default for OpenCodeAdapterConfig {
|
impl Default for OpenCodeAdapterConfig {
|
||||||
|
|
@ -104,6 +109,8 @@ impl Default for OpenCodeAdapterConfig {
|
||||||
native_proxy_manager: None,
|
native_proxy_manager: None,
|
||||||
acp_dispatch: None,
|
acp_dispatch: None,
|
||||||
provider_payload: None,
|
provider_payload: None,
|
||||||
|
agent_display_name: None,
|
||||||
|
agent_description: None,
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
@ -167,10 +174,16 @@ struct AcpPendingRequest {
|
||||||
|
|
||||||
#[derive(Debug, Clone)]
|
#[derive(Debug, Clone)]
|
||||||
enum AcpPendingKind {
|
enum AcpPendingKind {
|
||||||
Permission,
|
Permission { options: Vec<AcpPermissionOption> },
|
||||||
Question,
|
Question,
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[derive(Debug, Clone)]
|
||||||
|
struct AcpPermissionOption {
|
||||||
|
option_id: String,
|
||||||
|
kind: String,
|
||||||
|
}
|
||||||
|
|
||||||
struct AdapterState {
|
struct AdapterState {
|
||||||
config: OpenCodeAdapterConfig,
|
config: OpenCodeAdapterConfig,
|
||||||
sqlite_path: String,
|
sqlite_path: String,
|
||||||
|
|
@ -898,12 +911,22 @@ async fn oc_agent_list(State(state): State<Arc<AdapterState>>) -> Response {
|
||||||
if let Err(err) = state.ensure_initialized().await {
|
if let Err(err) = state.ensure_initialized().await {
|
||||||
return internal_error(err);
|
return internal_error(err);
|
||||||
}
|
}
|
||||||
|
let agent_name = state
|
||||||
|
.config
|
||||||
|
.agent_display_name
|
||||||
|
.clone()
|
||||||
|
.unwrap_or_else(|| "Sandbox Agent".to_string());
|
||||||
|
let agent_description = state
|
||||||
|
.config
|
||||||
|
.agent_description
|
||||||
|
.clone()
|
||||||
|
.unwrap_or_else(|| "Sandbox Agent compatibility layer".to_string());
|
||||||
(
|
(
|
||||||
StatusCode::OK,
|
StatusCode::OK,
|
||||||
Json(json!([
|
Json(json!([
|
||||||
{
|
{
|
||||||
"name": "Sandbox Agent",
|
"name": agent_name,
|
||||||
"description": "Sandbox Agent compatibility layer",
|
"description": agent_description,
|
||||||
"mode": "all",
|
"mode": "all",
|
||||||
"native": false,
|
"native": false,
|
||||||
"hidden": false,
|
"hidden": false,
|
||||||
|
|
@ -1929,13 +1952,33 @@ async fn oc_session_prompt(
|
||||||
.map(|session| !session.messages.is_empty())
|
.map(|session| !session.messages.is_empty())
|
||||||
.unwrap_or(false)
|
.unwrap_or(false)
|
||||||
};
|
};
|
||||||
|
let session_is_stale =
|
||||||
|
meta.last_connection_id != state.current_connection_for_agent(&meta.agent).await;
|
||||||
|
|
||||||
if let Some(selection) = requested_selection.as_ref() {
|
if let Some(selection) = requested_selection.as_ref() {
|
||||||
let selection_changed =
|
let selection_changed =
|
||||||
meta.provider_id != selection.provider_id || meta.model_id != selection.model_id;
|
meta.provider_id != selection.provider_id || meta.model_id != selection.model_id;
|
||||||
if has_messages && selection_changed {
|
let allow_stale_model_rebind = has_messages
|
||||||
|
&& selection_changed
|
||||||
|
&& session_is_stale
|
||||||
|
&& meta.agent == selection.agent
|
||||||
|
&& meta.provider_id == selection.provider_id;
|
||||||
|
|
||||||
|
if has_messages && selection_changed && !allow_stale_model_rebind {
|
||||||
return bad_request(MODEL_CHANGE_ERROR);
|
return bad_request(MODEL_CHANGE_ERROR);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if allow_stale_model_rebind {
|
||||||
|
tracing::info!(
|
||||||
|
session_id = %session_id,
|
||||||
|
agent = %meta.agent,
|
||||||
|
provider_id = %meta.provider_id,
|
||||||
|
from_model_id = %meta.model_id,
|
||||||
|
to_model_id = %selection.model_id,
|
||||||
|
"allowing stale session model rebind on prompt"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
meta.provider_id = selection.provider_id.clone();
|
meta.provider_id = selection.provider_id.clone();
|
||||||
meta.model_id = selection.model_id.clone();
|
meta.model_id = selection.model_id.clone();
|
||||||
meta.agent = selection.agent.clone();
|
meta.agent = selection.agent.clone();
|
||||||
|
|
@ -2815,8 +2858,11 @@ async fn oc_question_reply(
|
||||||
}
|
}
|
||||||
}));
|
}));
|
||||||
|
|
||||||
if let Err(err) = set_session_status(&state, &session_id, "idle").await {
|
// In ACP mode, the in-flight prompt turn emits idle when the turn completes.
|
||||||
return internal_error(err);
|
if pending.is_none() {
|
||||||
|
if let Err(err) = set_session_status(&state, &session_id, "idle").await {
|
||||||
|
return internal_error(err);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
(StatusCode::OK, Json(json!(true))).into_response()
|
(StatusCode::OK, Json(json!(true))).into_response()
|
||||||
|
|
@ -2888,8 +2934,11 @@ async fn oc_question_reject(
|
||||||
}
|
}
|
||||||
}));
|
}));
|
||||||
|
|
||||||
if let Err(err) = set_session_status(&state, &session_id, "idle").await {
|
// In ACP mode, the in-flight prompt turn emits idle when the turn completes.
|
||||||
return internal_error(err);
|
if pending.is_none() {
|
||||||
|
if let Err(err) = set_session_status(&state, &session_id, "idle").await {
|
||||||
|
return internal_error(err);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
(StatusCode::OK, Json(json!(true))).into_response()
|
(StatusCode::OK, Json(json!(true))).into_response()
|
||||||
|
|
@ -2925,6 +2974,102 @@ async fn oc_provider_oauth_callback() -> Response {
|
||||||
(StatusCode::OK, Json(json!(true))).into_response()
|
(StatusCode::OK, Json(json!(true))).into_response()
|
||||||
}
|
}
|
||||||
|
|
||||||
|
fn parse_acp_permission_options(params: &Value) -> Vec<AcpPermissionOption> {
|
||||||
|
params
|
||||||
|
.get("options")
|
||||||
|
.and_then(Value::as_array)
|
||||||
|
.map(|options| {
|
||||||
|
options
|
||||||
|
.iter()
|
||||||
|
.filter_map(|option| {
|
||||||
|
let option_id = option.get("optionId").and_then(Value::as_str)?;
|
||||||
|
let kind = option.get("kind").and_then(Value::as_str)?;
|
||||||
|
Some(AcpPermissionOption {
|
||||||
|
option_id: option_id.to_string(),
|
||||||
|
kind: kind.to_string(),
|
||||||
|
})
|
||||||
|
})
|
||||||
|
.collect::<Vec<_>>()
|
||||||
|
})
|
||||||
|
.unwrap_or_default()
|
||||||
|
}
|
||||||
|
|
||||||
|
fn preferred_permission_kinds_for_reply(reply: &str) -> &'static [&'static str] {
|
||||||
|
match reply {
|
||||||
|
"always" => &["allow_always", "allow_once"],
|
||||||
|
"reject" | "deny" => &["reject_once", "reject_always"],
|
||||||
|
_ => &["allow_once", "allow_always"],
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
fn select_permission_option_for_reply<'a>(
|
||||||
|
reply: &str,
|
||||||
|
options: &'a [AcpPermissionOption],
|
||||||
|
) -> Option<&'a AcpPermissionOption> {
|
||||||
|
let preferred_kinds = preferred_permission_kinds_for_reply(reply);
|
||||||
|
|
||||||
|
for kind in preferred_kinds {
|
||||||
|
if let Some(option) = options.iter().find(|option| option.kind == *kind) {
|
||||||
|
return Some(option);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
if matches!(reply, "reject" | "deny") {
|
||||||
|
if let Some(option) = options
|
||||||
|
.iter()
|
||||||
|
.find(|option| option.kind.starts_with("reject_"))
|
||||||
|
{
|
||||||
|
return Some(option);
|
||||||
|
}
|
||||||
|
} else if let Some(option) = options
|
||||||
|
.iter()
|
||||||
|
.find(|option| option.kind.starts_with("allow_"))
|
||||||
|
{
|
||||||
|
return Some(option);
|
||||||
|
}
|
||||||
|
|
||||||
|
options.first()
|
||||||
|
}
|
||||||
|
|
||||||
|
fn build_acp_permission_result(reply: &str, options: &[AcpPermissionOption]) -> Value {
|
||||||
|
let preferred_kind = preferred_permission_kinds_for_reply(reply)
|
||||||
|
.first()
|
||||||
|
.copied()
|
||||||
|
.unwrap_or("allow_once");
|
||||||
|
|
||||||
|
if let Some(option) = select_permission_option_for_reply(reply, options) {
|
||||||
|
let mut result = json!({
|
||||||
|
"outcome": {
|
||||||
|
"outcome": "selected",
|
||||||
|
"optionId": option.option_id,
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
|
if option.kind != preferred_kind {
|
||||||
|
if let Some(object) = result.as_object_mut() {
|
||||||
|
object.insert(
|
||||||
|
"_meta".to_string(),
|
||||||
|
json!({
|
||||||
|
"sandboxagent.dev": {
|
||||||
|
"requestedReply": reply,
|
||||||
|
"selectedOptionKind": option.kind,
|
||||||
|
"fallback": true,
|
||||||
|
}
|
||||||
|
}),
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
return result;
|
||||||
|
}
|
||||||
|
|
||||||
|
json!({
|
||||||
|
"outcome": {
|
||||||
|
"outcome": "cancelled",
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
||||||
async fn resolve_permission_inner(
|
async fn resolve_permission_inner(
|
||||||
state: &Arc<AdapterState>,
|
state: &Arc<AdapterState>,
|
||||||
session_id: &str,
|
session_id: &str,
|
||||||
|
|
@ -2945,20 +3090,28 @@ async fn resolve_permission_inner(
|
||||||
.map(|s| s.meta.agent_session_id.clone())
|
.map(|s| s.meta.agent_session_id.clone())
|
||||||
};
|
};
|
||||||
if let Some(server_id) = agent_session_id {
|
if let Some(server_id) = agent_session_id {
|
||||||
let option_kind = match reply {
|
let response_result = match &pending.kind {
|
||||||
"always" => "allow_always",
|
AcpPendingKind::Permission { options } => {
|
||||||
"reject" | "deny" => "reject_once",
|
build_acp_permission_result(reply, options)
|
||||||
_ => "allow_once",
|
}
|
||||||
|
AcpPendingKind::Question => {
|
||||||
|
warn!(
|
||||||
|
session_id = %session_id,
|
||||||
|
permission_id = %permission_id,
|
||||||
|
"permission response matched pending question request; sending cancelled"
|
||||||
|
);
|
||||||
|
json!({
|
||||||
|
"outcome": {
|
||||||
|
"outcome": "cancelled",
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
};
|
};
|
||||||
|
|
||||||
let response = json!({
|
let response = json!({
|
||||||
"jsonrpc": "2.0",
|
"jsonrpc": "2.0",
|
||||||
"id": pending.jsonrpc_id,
|
"id": pending.jsonrpc_id,
|
||||||
"result": {
|
"result": response_result,
|
||||||
"outcome": "selected",
|
|
||||||
"selectedOption": {
|
|
||||||
"kind": option_kind
|
|
||||||
}
|
|
||||||
}
|
|
||||||
});
|
});
|
||||||
if let Err(err) = dispatch.post(&server_id, None, response).await {
|
if let Err(err) = dispatch.post(&server_id, None, response).await {
|
||||||
warn!(?err, "failed to forward permission response to ACP agent");
|
warn!(?err, "failed to forward permission response to ACP agent");
|
||||||
|
|
@ -2993,6 +3146,12 @@ async fn resolve_permission_inner(
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// In ACP mode, the in-flight `session/prompt` turn owns session completion
|
||||||
|
// and emits `session.idle` when the turn really ends.
|
||||||
|
if pending.is_some() {
|
||||||
|
return Ok(());
|
||||||
|
}
|
||||||
|
|
||||||
set_session_status(state, session_id, "idle").await
|
set_session_status(state, session_id, "idle").await
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
@ -3731,6 +3890,7 @@ async fn acp_sse_translation_task(
|
||||||
Some("session/request_permission") => {
|
Some("session/request_permission") => {
|
||||||
let request_id = state.next_id("perm_");
|
let request_id = state.next_id("perm_");
|
||||||
let params = payload.get("params").cloned().unwrap_or(json!({}));
|
let params = payload.get("params").cloned().unwrap_or(json!({}));
|
||||||
|
let options = parse_acp_permission_options(¶ms);
|
||||||
let permission_request = json!({
|
let permission_request = json!({
|
||||||
"id": request_id,
|
"id": request_id,
|
||||||
"sessionID": session_id,
|
"sessionID": session_id,
|
||||||
|
|
@ -3747,7 +3907,7 @@ async fn acp_sse_translation_task(
|
||||||
AcpPendingRequest {
|
AcpPendingRequest {
|
||||||
opencode_session_id: session_id.clone(),
|
opencode_session_id: session_id.clone(),
|
||||||
jsonrpc_id: jrpc_id,
|
jsonrpc_id: jrpc_id,
|
||||||
kind: AcpPendingKind::Permission,
|
kind: AcpPendingKind::Permission { options },
|
||||||
},
|
},
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
@ -4361,3 +4521,52 @@ fn internal_error(message: String) -> Response {
|
||||||
)
|
)
|
||||||
.into_response()
|
.into_response()
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[cfg(test)]
|
||||||
|
mod tests {
|
||||||
|
use super::*;
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn select_permission_option_prefers_reply_semantics() {
|
||||||
|
let options = vec![
|
||||||
|
AcpPermissionOption {
|
||||||
|
option_id: "allow-once".to_string(),
|
||||||
|
kind: "allow_once".to_string(),
|
||||||
|
},
|
||||||
|
AcpPermissionOption {
|
||||||
|
option_id: "allow-always".to_string(),
|
||||||
|
kind: "allow_always".to_string(),
|
||||||
|
},
|
||||||
|
AcpPermissionOption {
|
||||||
|
option_id: "reject-once".to_string(),
|
||||||
|
kind: "reject_once".to_string(),
|
||||||
|
},
|
||||||
|
];
|
||||||
|
|
||||||
|
let once =
|
||||||
|
select_permission_option_for_reply("once", &options).expect("expected allow_once");
|
||||||
|
assert_eq!(once.option_id, "allow-once");
|
||||||
|
|
||||||
|
let always =
|
||||||
|
select_permission_option_for_reply("always", &options).expect("expected allow_always");
|
||||||
|
assert_eq!(always.option_id, "allow-always");
|
||||||
|
|
||||||
|
let reject =
|
||||||
|
select_permission_option_for_reply("reject", &options).expect("expected reject_once");
|
||||||
|
assert_eq!(reject.option_id, "reject-once");
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn build_permission_result_uses_schema_shape() {
|
||||||
|
let options = vec![AcpPermissionOption {
|
||||||
|
option_id: "allow-once".to_string(),
|
||||||
|
kind: "allow_once".to_string(),
|
||||||
|
}];
|
||||||
|
|
||||||
|
let result = build_acp_permission_result("once", &options);
|
||||||
|
|
||||||
|
assert_eq!(result["outcome"]["outcome"], json!("selected"));
|
||||||
|
assert_eq!(result["outcome"]["optionId"], json!("allow-once"));
|
||||||
|
assert!(result.get("selectedOption").is_none());
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
|
||||||
|
|
@ -199,6 +199,8 @@ pub fn build_router_with_state(shared: Arc<AppState>) -> (Router, Arc<AppState>)
|
||||||
native_proxy_manager: Some(shared.opencode_server_manager()),
|
native_proxy_manager: Some(shared.opencode_server_manager()),
|
||||||
acp_dispatch: Some(shared.acp_proxy() as Arc<dyn sandbox_agent_opencode_adapter::AcpDispatch>),
|
acp_dispatch: Some(shared.acp_proxy() as Arc<dyn sandbox_agent_opencode_adapter::AcpDispatch>),
|
||||||
provider_payload: Some(build_provider_payload_for_opencode(&shared)),
|
provider_payload: Some(build_provider_payload_for_opencode(&shared)),
|
||||||
|
agent_display_name: Some(shared.branding.product_name().to_string()),
|
||||||
|
agent_description: Some(format!("{} compatibility layer", shared.branding.product_name())),
|
||||||
..OpenCodeAdapterConfig::default()
|
..OpenCodeAdapterConfig::default()
|
||||||
})
|
})
|
||||||
.unwrap_or_else(|err| {
|
.unwrap_or_else(|err| {
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue