fix(goose): only send agent-session-id when a session exists (#6657)
Signed-off-by: Adrian Cole <adrian@tetrate.io>
This commit is contained in:
@@ -196,7 +196,7 @@ pub struct ApiRequestBuilder<'a> {
|
||||
client: &'a ApiClient,
|
||||
path: &'a str,
|
||||
headers: HeaderMap,
|
||||
session_id: &'a str,
|
||||
session_id: Option<&'a str>,
|
||||
}
|
||||
|
||||
impl ApiClient {
|
||||
@@ -273,10 +273,15 @@ impl ApiClient {
|
||||
Ok(self)
|
||||
}
|
||||
|
||||
pub fn request<'a>(&'a self, session_id: &'a str, path: &'a str) -> ApiRequestBuilder<'a> {
|
||||
/// - `session_id`: Use `None` only for configuration or pre-session tasks.
|
||||
pub fn request<'a>(
|
||||
&'a self,
|
||||
session_id: Option<&'a str>,
|
||||
path: &'a str,
|
||||
) -> ApiRequestBuilder<'a> {
|
||||
ApiRequestBuilder {
|
||||
client: self,
|
||||
session_id,
|
||||
session_id: session_id.filter(|id| !id.is_empty()),
|
||||
path,
|
||||
headers: HeaderMap::new(),
|
||||
}
|
||||
@@ -284,7 +289,7 @@ impl ApiClient {
|
||||
|
||||
pub async fn api_post(
|
||||
&self,
|
||||
session_id: &str,
|
||||
session_id: Option<&str>,
|
||||
path: &str,
|
||||
payload: &Value,
|
||||
) -> Result<ApiResponse> {
|
||||
@@ -293,18 +298,18 @@ impl ApiClient {
|
||||
|
||||
pub async fn response_post(
|
||||
&self,
|
||||
session_id: &str,
|
||||
session_id: Option<&str>,
|
||||
path: &str,
|
||||
payload: &Value,
|
||||
) -> Result<Response> {
|
||||
self.request(session_id, path).response_post(payload).await
|
||||
}
|
||||
|
||||
pub async fn api_get(&self, session_id: &str, path: &str) -> Result<ApiResponse> {
|
||||
pub async fn api_get(&self, session_id: Option<&str>, path: &str) -> Result<ApiResponse> {
|
||||
self.request(session_id, path).api_get().await
|
||||
}
|
||||
|
||||
pub async fn response_get(&self, session_id: &str, path: &str) -> Result<Response> {
|
||||
pub async fn response_get(&self, session_id: Option<&str>, path: &str) -> Result<Response> {
|
||||
self.request(session_id, path).response_get().await
|
||||
}
|
||||
|
||||
@@ -373,10 +378,16 @@ impl<'a> ApiRequestBuilder<'a> {
|
||||
F: FnOnce(url::Url, &Client) -> reqwest::RequestBuilder,
|
||||
{
|
||||
let url = self.client.build_url(self.path)?;
|
||||
let mut request = request_builder(url, &self.client.client);
|
||||
request = request.headers(self.headers.clone());
|
||||
let mut headers = self.headers.clone();
|
||||
headers.remove(SESSION_ID_HEADER);
|
||||
if let Some(session_id) = self.session_id {
|
||||
let header_name = HeaderName::from_static(SESSION_ID_HEADER);
|
||||
let header_value = HeaderValue::from_str(session_id)?;
|
||||
headers.insert(header_name, header_value);
|
||||
}
|
||||
|
||||
request = request.header(SESSION_ID_HEADER, self.session_id);
|
||||
let mut request = request_builder(url, &self.client.client);
|
||||
request = request.headers(headers);
|
||||
|
||||
request = match &self.client.auth {
|
||||
AuthMethod::BearerToken(token) => {
|
||||
@@ -411,69 +422,40 @@ impl fmt::Debug for ApiClient {
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use test_case::test_case;
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_session_id_header_injection() {
|
||||
let client = ApiClient::new(
|
||||
"http://localhost:8080".to_string(),
|
||||
AuthMethod::BearerToken("test-token".to_string()),
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
let builder = client.request("test-session_id-456", "/test");
|
||||
let request = builder
|
||||
.send_request(|url, client| client.get(url))
|
||||
.await
|
||||
#[test_case(Some("test-session_id-456"), None, Some("test-session_id-456"); "header set")]
|
||||
#[test_case(Some("new-session"), Some(("Agent-Session-Id", "old-session")), Some("new-session"); "replaces existing")]
|
||||
#[test_case(None, Some(("Agent-Session-Id", "old-session")), None; "removes existing on none")]
|
||||
#[test_case(Some(""), Some(("agent-session-id", "old-session")), None; "removes existing on empty")]
|
||||
fn test_session_id_header(
|
||||
session_id: Option<&str>,
|
||||
existing_header: Option<(&str, &str)>,
|
||||
expected: Option<&str>,
|
||||
) {
|
||||
let runtime = tokio::runtime::Runtime::new().unwrap();
|
||||
runtime.block_on(async {
|
||||
let client = ApiClient::new(
|
||||
"http://localhost:8080".to_string(),
|
||||
AuthMethod::BearerToken("test-token".to_string()),
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
let headers = request.build().unwrap().headers().clone();
|
||||
let mut builder = client.request(session_id, "/test");
|
||||
if let Some((key, value)) = existing_header {
|
||||
builder = builder.header(key, value).unwrap();
|
||||
}
|
||||
let request = builder
|
||||
.send_request(|url, client| client.get(url))
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
assert!(headers.contains_key(SESSION_ID_HEADER));
|
||||
assert_eq!(
|
||||
headers.get(SESSION_ID_HEADER).unwrap().to_str().unwrap(),
|
||||
"test-session_id-456"
|
||||
);
|
||||
}
|
||||
let headers = request.build().unwrap().headers().clone();
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_session_id_header_with_different_id() {
|
||||
let client = ApiClient::new(
|
||||
"http://localhost:8080".to_string(),
|
||||
AuthMethod::BearerToken("test-token".to_string()),
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
let builder = client.request("another-session_id-789", "/test");
|
||||
let request = builder
|
||||
.send_request(|url, client| client.get(url))
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
let headers = request.build().unwrap().headers().clone();
|
||||
|
||||
assert!(headers.contains_key(SESSION_ID_HEADER));
|
||||
assert_eq!(
|
||||
headers.get(SESSION_ID_HEADER).unwrap().to_str().unwrap(),
|
||||
"another-session_id-789"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_session_id_header_always_present() {
|
||||
let client = ApiClient::new(
|
||||
"http://localhost:8080".to_string(),
|
||||
AuthMethod::BearerToken("test-token".to_string()),
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
let builder = client.request("required-session_id", "/test");
|
||||
let request = builder
|
||||
.send_request(|url, client| client.get(url))
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
let headers = request.build().unwrap().headers().clone();
|
||||
|
||||
assert!(headers.contains_key(SESSION_ID_HEADER));
|
||||
let actual = headers
|
||||
.get(SESSION_ID_HEADER)
|
||||
.and_then(|value| value.to_str().ok());
|
||||
assert_eq!(actual, expected);
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user