Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion crates/code_assistant/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ base64 = "0.22"
image = "0.25"

# Agent Client Protocol
agent-client-protocol = { version = "0.7", features = ["unstable"] }
agent-client-protocol = { version = "0.9", features = ["unstable"] }
tokio-util = { version = "0.7", features = ["compat"] }

[dev-dependencies]
Expand Down
136 changes: 57 additions & 79 deletions crates/code_assistant/src/acp/agent.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,11 +77,8 @@ impl ACPAgentImpl {
}

fn agent_info() -> acp::Implementation {
acp::Implementation {
name: "code-assistant".to_string(),
title: Some("Code Assistant".to_string()),
version: env!("CARGO_PKG_VERSION").to_string(),
}
acp::Implementation::new("code-assistant", env!("CARGO_PKG_VERSION"))
.title("Code Assistant")
}

fn compute_model_state(
Expand Down Expand Up @@ -117,26 +114,28 @@ impl ACPAgentImpl {
Some(provider_config.label.clone())
};

let model_meta = json!({
"provider": {
let mut model_meta = JsonMap::new();
model_meta.insert(
"provider".into(),
json!({
"id": model_config.provider,
"label": provider_config.label,
"type": provider_config.provider,
},
"model": {
}),
);
model_meta.insert(
"model".into(),
json!({
"id": model_config.id,
},
"display_name": display_name,
});
}),
);
model_meta.insert("display_name".into(), json!(display_name));

entries.push((
provider_config.label.clone(),
acp::ModelInfo {
model_id: acp::ModelId(display_name.clone().into()),
name: display_name.clone(),
description,
meta: Some(model_meta),
},
acp::ModelInfo::new(display_name.clone(), display_name.clone())
.description(description)
.meta(model_meta),
));
}

Expand Down Expand Up @@ -213,11 +212,8 @@ impl ACPAgentImpl {
state_meta.insert("providers".into(), JsonValue::Object(providers_meta));

Some(ModelStateInfo {
state: acp::SessionModelState {
current_model_id: acp::ModelId(selected_model_id.clone().into()),
available_models,
meta: Some(JsonValue::Object(state_meta)),
},
state: acp::SessionModelState::new(selected_model_id.clone(), available_models)
.meta(state_meta),
selected_model_name,
selection_changed,
})
Expand Down Expand Up @@ -258,32 +254,27 @@ impl acp::Agent for ACPAgentImpl {
*caps = Some(arguments.client_capabilities.clone());
}

Ok(acp::InitializeResponse {
protocol_version: acp::V1,
agent_capabilities: acp::AgentCapabilities {
load_session: true,
mcp_capabilities: acp::McpCapabilities {
http: false,
sse: false,
meta: None,
},
prompt_capabilities: acp::PromptCapabilities {
image: true,
audio: false,
embedded_context: true,
meta: None,
},
meta: Some(json!({
"models": {
"supportsModelSelector": true,
"idFormat": "display_name",
},
})),
},
auth_methods: Vec::new(),
agent_info: Some(Self::agent_info()),
meta: None,
})
let mut capabilities_meta = JsonMap::new();
capabilities_meta.insert(
"models".into(),
json!({
"supportsModelSelector": true,
"idFormat": "display_name",
}),
);

Ok(acp::InitializeResponse::new(acp::ProtocolVersion::V1)
.agent_capabilities(
acp::AgentCapabilities::new()
.load_session(true)
.prompt_capabilities(
acp::PromptCapabilities::new()
.image(true)
.embedded_context(true),
)
.meta(capabilities_meta),
)
.agent_info(Self::agent_info()))
})
}

Expand All @@ -303,7 +294,7 @@ impl acp::Agent for ACPAgentImpl {
{
Box::pin(async move {
tracing::info!("ACP: Received authenticate request");
Ok(acp::AuthenticateResponse { meta: None })
Ok(acp::AuthenticateResponse::new())
})
}

Expand Down Expand Up @@ -369,12 +360,7 @@ impl acp::Agent for ACPAgentImpl {
models_state = Some(model_info.state);
}

Ok(acp::NewSessionResponse {
session_id: acp::SessionId(session_id.into()),
modes: None, // TODO: Support modes like "Plan", "Architect" and "Code".
models: models_state,
meta: None,
})
Ok(acp::NewSessionResponse::new(session_id).models(models_state))
})
}

Expand Down Expand Up @@ -473,11 +459,7 @@ impl acp::Agent for ACPAgentImpl {

tracing::info!("ACP: Loaded session: {}", arguments.session_id.0);

Ok(acp::LoadSessionResponse {
modes: None,
models: models_state,
meta: None,
})
Ok(acp::LoadSessionResponse::new().models(models_state))
})
}

Expand Down Expand Up @@ -532,7 +514,7 @@ impl acp::Agent for ACPAgentImpl {
let acp_ui = Arc::new(ACPUserUI::new(
arguments.session_id.clone(),
session_update_tx.clone(),
base_path,
base_path.clone(),
));

// Store it so cancel() can reach it
Expand All @@ -544,7 +526,8 @@ impl acp::Agent for ACPAgentImpl {
let ui: Arc<dyn crate::ui::UserInterface> = acp_ui.clone();

// Convert prompt content blocks
let content_blocks = convert_prompt_to_content_blocks(arguments.prompt);
let content_blocks =
convert_prompt_to_content_blocks(arguments.prompt, base_path.as_deref());

let config_result = {
let manager = session_manager.lock().await;
Expand Down Expand Up @@ -764,10 +747,7 @@ impl acp::Agent for ACPAgentImpl {
uis.remove(arguments.session_id.0.as_ref());
}

return Ok(acp::PromptResponse {
stop_reason: acp::StopReason::Cancelled,
meta: None,
});
return Ok(acp::PromptResponse::new(acp::StopReason::Cancelled));
}
}

Expand Down Expand Up @@ -799,10 +779,7 @@ impl acp::Agent for ACPAgentImpl {
return Err(to_acp_error(&anyhow::anyhow!(message)));
}

Ok(acp::PromptResponse {
stop_reason: acp::StopReason::EndTurn,
meta: None,
})
Ok(acp::PromptResponse::new(acp::StopReason::EndTurn))
})
}

Expand Down Expand Up @@ -879,14 +856,15 @@ impl acp::Agent for ACPAgentImpl {
display_name,
);

Ok(acp::SetSessionModelResponse {
meta: Some(json!({
"model": {
"id": display_name,
"display_name": display_name,
}
})),
})
let mut response_meta = JsonMap::new();
response_meta.insert(
"model".into(),
json!({
"id": display_name,
"display_name": display_name,
}),
);
Ok(acp::SetSessionModelResponse::new().meta(response_meta))
})
}

Expand Down
51 changes: 24 additions & 27 deletions crates/code_assistant/src/acp/error_handling.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,29 +49,26 @@ pub fn to_acp_error(error: &anyhow::Error) -> acp::Error {
{
// Configuration errors - these are client-side issues
let formatted = format_config_error(error);
acp::Error::new((acp::ErrorCode::INVALID_PARAMS.code, error_str)).with_data(formatted)
acp::Error::invalid_params().data(formatted)
} else if error_lower.contains("401") || error_lower.contains("unauthorized") {
// Authentication errors
acp::Error::new((acp::ErrorCode::AUTH_REQUIRED.code, error_str.clone()))
.with_data(error_str)
acp::Error::auth_required().data(error_str)
} else if error_lower.contains("404") || error_lower.contains("not found") {
// Resource not found errors
acp::Error::new((acp::ErrorCode::RESOURCE_NOT_FOUND.code, error_str.clone()))
.with_data(error_str)
acp::Error::resource_not_found(None).data(error_str)
} else if error_lower.contains("400")
|| error_lower.contains("bad request")
|| error_lower.contains("invalid request")
{
// Client provided invalid parameters
acp::Error::new((acp::ErrorCode::INVALID_PARAMS.code, error_str.clone())).with_data(
acp::Error::invalid_params().data(
json!({
"hint": "The model configuration may include custom fields under `config` that this provider does not accept. Remove unexpected keys or move them into `meta`."
}),
)
} else {
// All other errors are internal
acp::Error::new((acp::ErrorCode::INTERNAL_ERROR.code, error_str.clone()))
.with_data(error_str)
acp::Error::internal_error().data(error_str)
}
}

Expand Down Expand Up @@ -112,11 +109,11 @@ mod tests {
let acp_error = to_acp_error(&error);

// Should be invalid_params for configuration errors
assert_eq!(acp_error.code, acp::ErrorCode::INVALID_PARAMS.code);
assert!(acp_error
.message
.contains("Providers configuration file not found"));
assert!(acp_error.data.is_some());
assert_eq!(acp_error.code, acp::ErrorCode::InvalidParams);
// The data should contain the formatted error with helpful context
let data = acp_error.data.expect("expected data");
let data_str = data.to_string();
assert!(data_str.contains("configuration file not found"));
}

#[test]
Expand All @@ -125,8 +122,10 @@ mod tests {
let acp_error = to_acp_error(&error);

// Should be auth_required for 401 errors - but using -32000 range
assert_eq!(acp_error.code, acp::ErrorCode::AUTH_REQUIRED.code);
assert!(acp_error.message.contains("HTTP 401"));
assert_eq!(acp_error.code, acp::ErrorCode::AuthRequired);
// Data should contain the original error message
let data = acp_error.data.expect("expected data");
assert!(data.to_string().contains("HTTP 401"));
}

#[test]
Expand All @@ -135,8 +134,10 @@ mod tests {
let acp_error = to_acp_error(&error);

// Should be resource_not_found for 404 errors
assert_eq!(acp_error.code, acp::ErrorCode::RESOURCE_NOT_FOUND.code);
assert!(acp_error.message.contains("HTTP 404"));
assert_eq!(acp_error.code, acp::ErrorCode::ResourceNotFound);
// Data should contain the original error message
let data = acp_error.data.expect("expected data");
assert!(data.to_string().contains("HTTP 404"));
}

#[test]
Expand All @@ -145,19 +146,18 @@ mod tests {
let acp_error = to_acp_error(&error);

// Should be internal_error for other errors
assert_eq!(acp_error.code, acp::ErrorCode::INTERNAL_ERROR.code);
assert!(acp_error.message.contains("Some unexpected error"));
assert_eq!(acp_error.code, acp::ErrorCode::InternalError);
// Data should contain the original error message
let data = acp_error.data.expect("expected data");
assert!(data.to_string().contains("Some unexpected error"));
}

#[test]
fn test_to_acp_error_bad_request() {
let error = anyhow!("HTTP 400: Bad request due to invalid payload");
let acp_error = to_acp_error(&error);

assert_eq!(acp_error.code, acp::ErrorCode::INVALID_PARAMS.code);
assert!(acp_error
.message
.contains("HTTP 400: Bad request due to invalid payload"));
assert_eq!(acp_error.code, acp::ErrorCode::InvalidParams);
let data = acp_error.data.expect("expected hint data");
let hint = data
.get("hint")
Expand All @@ -173,10 +173,7 @@ mod tests {
);
let acp_error = to_acp_error(&error);

assert_eq!(acp_error.code, acp::ErrorCode::INVALID_PARAMS.code);
assert!(acp_error
.message
.contains("thinking_budget: Extra inputs are not permitted"));
assert_eq!(acp_error.code, acp::ErrorCode::InvalidParams);
let data = acp_error.data.expect("expected hint data");
let hint = data
.get("hint")
Expand Down
Loading