Send session headers on all OpenCode requests (#63702)
Fixes https://github.com/zed-industries/zed/issues/63672 OpenCode Go now requires the `x-opencode-session` header on every model request. Zed previously sent the header only when a request already had a thread ID, so inline assistance, terminal assistance, and other standalone requests could omit it and risk rejection. This change makes the OpenCode provider add the header to every request. Existing thread and assist session IDs are reused when available. Standalone requests receive a generated non-empty ID. Thread title and summary requests now carry the thread ID so related requests keep the same session scope. The change also adds tests for missing, empty, and invalid IDs, plus a wire-level test that verifies the header reaches the HTTP client. The provider test file stays separate so the implementation remains under 1,000 lines. Testing: - `cargo fmt --all -- --check` - Focused OpenCode, thread summary, and thread title tests - `cargo check -p agent_ui -p sidebar` - `GITHUB_ACTIONS=1 ./script/clippy -p language_models` - `git diff --check` Release Notes: - Fixed OpenCode Go requests that could fail when Zed omitted the session header. --------- Co-authored-by: Kirill Bulatov <kirill@zed.dev> Co-authored-by: Christopher Biscardi <chris@christopherbiscardi.com>
This commit is contained in:
1 parent
c41a5fa028
commit
342bafafee
5 files changed
+171
-18
No files matched your search
@@ -3848,6 +3848,7 @@ impl Thread {
|
||||
return Task::ready(None).shared();
|
||||
};
|
||||
let mut request = LanguageModelRequest {
|
||||
thread_id: Some(self.id.to_string()),
|
||||
intent: Some(CompletionIntent::ThreadContextSummarization),
|
||||
temperature: AgentSettings::temperature_for_model(&model, cx),
|
||||
..Default::default()
|
||||
@@ -3936,7 +3937,7 @@ impl Thread {
|
||||
log::debug!("Generating title with model: {:?}", model.name());
|
||||
|
||||
let temperature = AgentSettings::temperature_for_model(&model, cx);
|
||||
let request = build_thread_title_request(&self.messages, temperature);
|
||||
let request = build_thread_title_request(&self.id, &self.messages, temperature);
|
||||
|
||||
let title_generation = cx.spawn(async move |_this, cx| {
|
||||
stream_thread_title(model, request, cx)
|
||||
@@ -4855,10 +4856,12 @@ fn retained_user_request_messages_before(
|
||||
}
|
||||
|
||||
pub fn build_thread_title_request(
|
||||
thread_id: &acp::SessionId,
|
||||
messages: &[Arc<Message>],
|
||||
temperature: Option<f32>,
|
||||
) -> LanguageModelRequest {
|
||||
let mut request = LanguageModelRequest {
|
||||
thread_id: Some(thread_id.to_string()),
|
||||
intent: Some(CompletionIntent::ThreadSummarization),
|
||||
temperature,
|
||||
..Default::default()
|
||||
@@ -6939,6 +6942,7 @@ mod tests {
|
||||
#[gpui::test]
|
||||
async fn test_thread_summary_request_uses_compacted_history(cx: &mut TestAppContext) {
|
||||
let (thread, _event_stream) = setup_thread_for_test(cx).await;
|
||||
let thread_id = thread.read_with(cx, |thread, _| thread.id().to_string());
|
||||
let summary_model = Arc::new(FakeLanguageModel::default());
|
||||
|
||||
let summary_task = cx.update(|cx| {
|
||||
@@ -6968,6 +6972,10 @@ mod tests {
|
||||
cx.run_until_parked();
|
||||
|
||||
let summary_request = summary_model.pending_completions().pop().unwrap();
|
||||
assert_eq!(
|
||||
summary_request.thread_id.as_deref(),
|
||||
Some(thread_id.as_str())
|
||||
);
|
||||
assert_eq!(
|
||||
summary_request.intent,
|
||||
Some(CompletionIntent::ThreadContextSummarization)
|
||||
@@ -7002,8 +7010,10 @@ mod tests {
|
||||
agent_text_message("after assistant"),
|
||||
];
|
||||
|
||||
let request = build_thread_title_request(&messages, Some(0.2));
|
||||
let request =
|
||||
build_thread_title_request(&acp::SessionId::new("thread-id"), &messages, Some(0.2));
|
||||
|
||||
assert_eq!(request.thread_id.as_deref(), Some("thread-id"));
|
||||
assert_eq!(request.intent, Some(CompletionIntent::ThreadSummarization));
|
||||
assert_eq!(request.temperature, Some(0.2));
|
||||
assert_eq!(
|
||||
|
||||
@@ -497,6 +497,7 @@ impl CodegenAlternative {
|
||||
.context("generating content prompt")?;
|
||||
|
||||
let temperature = AgentSettings::temperature_for_model(model, cx);
|
||||
let session_id = self.session_id.to_string();
|
||||
|
||||
let tool_choice = model
|
||||
.supports_tool_choice(LanguageModelToolChoice::Any)
|
||||
@@ -544,7 +545,7 @@ impl CodegenAlternative {
|
||||
];
|
||||
|
||||
LanguageModelRequest {
|
||||
thread_id: None,
|
||||
thread_id: Some(session_id),
|
||||
prompt_id: None,
|
||||
intent: Some(CompletionIntent::InlineAssist),
|
||||
tools,
|
||||
@@ -609,6 +610,7 @@ impl CodegenAlternative {
|
||||
.context("generating content prompt")?;
|
||||
|
||||
let temperature = AgentSettings::temperature_for_model(model, cx);
|
||||
let session_id = self.session_id.to_string();
|
||||
|
||||
Ok(cx.spawn(async move |_cx| {
|
||||
let mut request_message = LanguageModelRequestMessage {
|
||||
@@ -625,7 +627,7 @@ impl CodegenAlternative {
|
||||
request_message.content.push(prompt.into());
|
||||
|
||||
LanguageModelRequest {
|
||||
thread_id: None,
|
||||
thread_id: Some(session_id),
|
||||
prompt_id: None,
|
||||
intent: Some(CompletionIntent::InlineAssist),
|
||||
tools: Vec::new(),
|
||||
|
||||
@@ -244,6 +244,7 @@ impl TerminalInlineAssistant {
|
||||
)?;
|
||||
|
||||
let temperature = AgentSettings::temperature_for_model(&model, cx);
|
||||
let session_id = assist.codegen.read(cx).session_id().to_string();
|
||||
|
||||
let mention_set = prompt_editor.read(cx).mention_set().clone();
|
||||
let load_context_task = load_context(&mention_set, cx);
|
||||
@@ -263,7 +264,7 @@ impl TerminalInlineAssistant {
|
||||
request_message.content.push(prompt.into());
|
||||
|
||||
LanguageModelRequest {
|
||||
thread_id: None,
|
||||
thread_id: Some(session_id),
|
||||
prompt_id: None,
|
||||
intent: Some(CompletionIntent::TerminalInlineAssist),
|
||||
messages: vec![request_message],
|
||||
|
||||
@@ -64,7 +64,8 @@ const PROVIDER_NAME: LanguageModelProviderName = LanguageModelProviderName::new(
|
||||
|
||||
const API_KEY_ENV_VAR_NAME: &str = "OPENCODE_API_KEY";
|
||||
static API_KEY_ENV_VAR: LazyLock<EnvVar> = env_var!(API_KEY_ENV_VAR_NAME);
|
||||
pub(crate) const RESERVED_HEADER_NAMES: &[&str] = &["x-opencode-session"];
|
||||
const OPENCODE_SESSION_HEADER_NAME: &str = "x-opencode-session";
|
||||
pub(crate) const RESERVED_HEADER_NAMES: &[&str] = &[OPENCODE_SESSION_HEADER_NAME];
|
||||
|
||||
#[derive(Default, Clone, Debug, PartialEq)]
|
||||
pub struct OpenCodeSettings {
|
||||
@@ -326,9 +327,11 @@ impl HttpClient for InjectHeaderClient {
|
||||
fn user_agent(&self) -> Option<&http::HeaderValue> {
|
||||
self.inner.user_agent()
|
||||
}
|
||||
|
||||
fn proxy(&self) -> Option<&http_client::Url> {
|
||||
self.inner.proxy()
|
||||
}
|
||||
|
||||
fn send(
|
||||
&self,
|
||||
mut req: http::Request<AsyncBody>,
|
||||
@@ -339,6 +342,15 @@ impl HttpClient for InjectHeaderClient {
|
||||
}
|
||||
}
|
||||
|
||||
// Standalone requests do not have a conversation ID, but OpenCode requires a
|
||||
// non-empty session header.
|
||||
fn opencode_session_header_value(thread_id: Option<&str>) -> http::HeaderValue {
|
||||
thread_id
|
||||
.filter(|thread_id| !thread_id.is_empty())
|
||||
.and_then(|thread_id| http::HeaderValue::from_str(thread_id).ok())
|
||||
.unwrap_or_else(|| http::HeaderValue::from(rand::random::<u64>()))
|
||||
}
|
||||
|
||||
impl OpenCodeLanguageModel {
|
||||
fn base_api_url(&self, cx: &AsyncApp) -> SharedString {
|
||||
// Custom models can override the API URL
|
||||
@@ -633,17 +645,11 @@ impl LanguageModel for OpenCodeLanguageModel {
|
||||
LanguageModelCompletionError,
|
||||
>,
|
||||
> {
|
||||
let http_client = if let Some(ref thread_id) = request.thread_id
|
||||
&& let Ok(value) = http::HeaderValue::from_str(thread_id)
|
||||
{
|
||||
Arc::new(InjectHeaderClient {
|
||||
inner: self.http_client.clone(),
|
||||
name: http::HeaderName::from_static("x-opencode-session"),
|
||||
value,
|
||||
})
|
||||
} else {
|
||||
self.http_client.clone()
|
||||
};
|
||||
let http_client: Arc<dyn HttpClient> = Arc::new(InjectHeaderClient {
|
||||
inner: self.http_client.clone(),
|
||||
name: http::HeaderName::from_static(OPENCODE_SESSION_HEADER_NAME),
|
||||
value: opencode_session_header_value(request.thread_id.as_deref()),
|
||||
});
|
||||
let extra_headers = self.custom_headers(cx);
|
||||
|
||||
match self.model.protocol(self.subscription) {
|
||||
@@ -984,3 +990,133 @@ impl Render for ConfigurationView {
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use http_client::{FakeHttpClient, Response};
|
||||
use language_model::{LanguageModelRequestMessage, MessageContent, Role};
|
||||
use parking_lot::Mutex;
|
||||
|
||||
#[test]
|
||||
fn test_opencode_session_header_uses_thread_id() {
|
||||
let value = opencode_session_header_value(Some("thread-123"));
|
||||
|
||||
assert_eq!(value, "thread-123");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_opencode_session_header_without_thread_id() {
|
||||
let value = opencode_session_header_value(None);
|
||||
|
||||
assert_generated_session_id(&value);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_opencode_session_header_with_empty_thread_id() {
|
||||
let value = opencode_session_header_value(Some(""));
|
||||
|
||||
assert_generated_session_id(&value);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_opencode_session_header_with_invalid_thread_id() {
|
||||
let value = opencode_session_header_value(Some("thread\n123"));
|
||||
|
||||
assert_generated_session_id(&value);
|
||||
}
|
||||
|
||||
#[gpui::test]
|
||||
async fn test_stream_completion_sends_session_header_without_thread_id(
|
||||
cx: &mut gpui::TestAppContext,
|
||||
) {
|
||||
let captured_header = Arc::new(Mutex::new(None));
|
||||
let http_client = FakeHttpClient::create({
|
||||
let captured_header = captured_header.clone();
|
||||
move |request| {
|
||||
let captured_header = captured_header.clone();
|
||||
async move {
|
||||
*captured_header.lock() =
|
||||
Some(request.headers().get(OPENCODE_SESSION_HEADER_NAME).cloned());
|
||||
Ok(Response::builder().status(200).body(AsyncBody::from(
|
||||
"event: message_stop\ndata: {\"type\":\"message_stop\"}\n\n",
|
||||
))?)
|
||||
}
|
||||
}
|
||||
});
|
||||
let provider = cx.update(|cx| {
|
||||
let settings_store = SettingsStore::test(cx);
|
||||
cx.set_global(settings_store);
|
||||
OpenCodeLanguageModelProvider::new(http_client, Arc::new(TestCredentialsProvider), cx)
|
||||
});
|
||||
let store_key = provider.state.update(cx, |state, cx| {
|
||||
state.set_api_key(Some("test-key".to_string()), cx)
|
||||
});
|
||||
store_key.await.unwrap();
|
||||
let model =
|
||||
provider.create_language_model(opencode::Model::default(), OpenCodeSubscription::Go);
|
||||
let request = LanguageModelRequest {
|
||||
thread_id: None,
|
||||
messages: vec![LanguageModelRequestMessage {
|
||||
role: Role::User,
|
||||
content: vec![MessageContent::Text("Hello".to_string())],
|
||||
cache: false,
|
||||
reasoning_details: None,
|
||||
}],
|
||||
..Default::default()
|
||||
};
|
||||
|
||||
let stream = model
|
||||
.stream_completion(request, &cx.to_async())
|
||||
.await
|
||||
.unwrap();
|
||||
drop(stream);
|
||||
|
||||
let captured_header = captured_header
|
||||
.lock()
|
||||
.take()
|
||||
.expect("request should reach the http client")
|
||||
.expect("request should carry the session header");
|
||||
assert_generated_session_id(&captured_header);
|
||||
}
|
||||
|
||||
fn assert_generated_session_id(value: &http::HeaderValue) {
|
||||
value
|
||||
.to_str()
|
||||
.unwrap()
|
||||
.parse::<u64>()
|
||||
.expect("generated session id should be a u64");
|
||||
}
|
||||
|
||||
struct TestCredentialsProvider;
|
||||
|
||||
impl CredentialsProvider for TestCredentialsProvider {
|
||||
fn read_credentials<'a>(
|
||||
&'a self,
|
||||
_url: &'a str,
|
||||
_cx: &'a AsyncApp,
|
||||
) -> std::pin::Pin<
|
||||
Box<dyn std::future::Future<Output = Result<Option<(String, Vec<u8>)>>> + 'a>,
|
||||
> {
|
||||
Box::pin(async { Ok(None) })
|
||||
}
|
||||
|
||||
fn write_credentials<'a>(
|
||||
&'a self,
|
||||
_url: &'a str,
|
||||
_username: &'a str,
|
||||
_password: &'a [u8],
|
||||
_cx: &'a AsyncApp,
|
||||
) -> std::pin::Pin<Box<dyn std::future::Future<Output = Result<()>> + 'a>> {
|
||||
Box::pin(async { Ok(()) })
|
||||
}
|
||||
|
||||
fn delete_credentials<'a>(
|
||||
&'a self,
|
||||
_url: &'a str,
|
||||
_cx: &'a AsyncApp,
|
||||
) -> std::pin::Pin<Box<dyn std::future::Future<Output = Result<()>> + 'a>> {
|
||||
Box::pin(async { Ok(()) })
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -3847,7 +3847,11 @@ impl Sidebar {
|
||||
anyhow::bail!("Thread not found in database");
|
||||
};
|
||||
|
||||
let request = agent::build_thread_title_request(&db_thread.messages, temperature);
|
||||
let request = agent::build_thread_title_request(
|
||||
&session_id,
|
||||
&db_thread.messages,
|
||||
temperature,
|
||||
);
|
||||
let title =
|
||||
SharedString::from(agent::stream_thread_title(model, request, cx).await?);
|
||||
|
||||
|
||||
Reference in new issue
Block a user