diff --git a/crates/forge_app/src/dto/google/request.rs b/crates/forge_app/src/dto/google/request.rs index c148e28414..20be6fde9f 100644 --- a/crates/forge_app/src/dto/google/request.rs +++ b/crates/forge_app/src/dto/google/request.rs @@ -360,6 +360,8 @@ impl From for Request { contents.push(Content { role: Some(Role::User), parts: pending_tool_parts }); } + let conversation_id = context.conversation_id.map(|id| id.to_string()); + // Convert tools let tools = if !context.tools.is_empty() { Some(vec![Tool::FunctionDeclarations { @@ -424,7 +426,11 @@ impl From for Request { tool_config, safety_settings: None, cached_content: None, - labels: None, + labels: conversation_id.map(|conversation_id| { + serde_json::json!({ + "conversation_id": conversation_id, + }) + }), } } } @@ -561,10 +567,39 @@ impl From for Part { #[cfg(test)] mod tests { - use forge_domain::{ToolCallArguments, ToolCallFull, ToolCallId, ToolName, ToolResult}; + use forge_domain::{ + Context, ConversationId, ToolCallArguments, ToolCallFull, ToolCallId, ToolName, ToolResult, + }; + use pretty_assertions::assert_eq; use super::*; + #[test] + fn test_request_includes_conversation_id_label_when_present() { + let fixture = ConversationId::generate(); + let actual = + serde_json::to_value(Request::from(Context::default().conversation_id(fixture))) + .unwrap(); + let expected = serde_json::Value::String(fixture.to_string()); + + assert_eq!(actual["labels"]["conversation_id"], expected); + } + + #[test] + fn test_request_omits_conversation_id_label_when_absent() { + let fixture = Context::default(); + let actual = serde_json::to_value(Request::from(fixture)).unwrap(); + let expected = serde_json::Value::Null; + + assert_eq!( + actual + .get("labels") + .cloned() + .unwrap_or(serde_json::Value::Null), + expected + ); + } + #[test] fn test_tool_call_args_serialization() { // Create a ToolCallFull with Unparsed JSON arguments (as it would come from diff --git a/crates/forge_app/src/infra.rs b/crates/forge_app/src/infra.rs index aab687c1c8..6ac4e3040f 100644 --- a/crates/forge_app/src/infra.rs +++ b/crates/forge_app/src/infra.rs @@ -99,6 +99,10 @@ pub trait FileWriterInfra: Send + Sync { /// Writes the content of a file at the specified path. async fn write(&self, path: &Path, contents: Bytes) -> anyhow::Result<()>; + /// Appends content to a file at the specified path, creating the file if it + /// does not already exist. + async fn append(&self, path: &Path, contents: Bytes) -> anyhow::Result<()>; + /// Writes content to a temporary file with the given prefix and extension, /// and returns its path. The file will be kept (not deleted) after /// creation. diff --git a/crates/forge_fs/src/write.rs b/crates/forge_fs/src/write.rs index e5019041bc..efd9adad07 100644 --- a/crates/forge_fs/src/write.rs +++ b/crates/forge_fs/src/write.rs @@ -1,6 +1,7 @@ use std::path::Path; use anyhow::{Context, Result}; +use tokio::io::AsyncWriteExt; impl crate::ForgeFS { pub async fn create_dir_all>(path: T) -> Result<()> { @@ -15,6 +16,21 @@ impl crate::ForgeFS { .with_context(|| format!("Failed to write file {}", path.as_ref().display())) } + pub async fn append, U: AsRef<[u8]>>(path: T, contents: U) -> Result<()> { + let mut file = tokio::fs::OpenOptions::new() + .create(true) + .append(true) + .open(path.as_ref()) + .await + .with_context(|| { + format!("Failed to open file {} for append", path.as_ref().display()) + })?; + + file.write_all(contents.as_ref()) + .await + .with_context(|| format!("Failed to append file {}", path.as_ref().display())) + } + pub async fn remove_file>(path: T) -> Result<()> { tokio::fs::remove_file(path.as_ref()) .await diff --git a/crates/forge_infra/src/forge_infra.rs b/crates/forge_infra/src/forge_infra.rs index b899eee341..7e4c0b14e9 100644 --- a/crates/forge_infra/src/forge_infra.rs +++ b/crates/forge_infra/src/forge_infra.rs @@ -148,6 +148,10 @@ impl FileWriterInfra for ForgeInfra { self.file_write_service.write(path, contents).await } + async fn append(&self, path: &Path, contents: Bytes) -> anyhow::Result<()> { + self.file_write_service.append(path, contents).await + } + async fn write_temp(&self, prefix: &str, ext: &str, content: &str) -> anyhow::Result { self.file_write_service .write_temp(prefix, ext, content) diff --git a/crates/forge_infra/src/fs_write.rs b/crates/forge_infra/src/fs_write.rs index b1836fe3b1..442a9a2529 100644 --- a/crates/forge_infra/src/fs_write.rs +++ b/crates/forge_infra/src/fs_write.rs @@ -38,6 +38,11 @@ impl FileWriterInfra for ForgeFileWriteService { Ok(forge_fs::ForgeFS::write(path, contents.to_vec()).await?) } + async fn append(&self, path: &Path, contents: Bytes) -> anyhow::Result<()> { + self.create_parent_dirs(path).await?; + Ok(forge_fs::ForgeFS::append(path, contents.to_vec()).await?) + } + async fn write_temp(&self, prefix: &str, ext: &str, content: &str) -> anyhow::Result { let path = tempfile::Builder::new() .disable_cleanup(true) diff --git a/crates/forge_infra/src/http.rs b/crates/forge_infra/src/http.rs index 3baf911e30..d3934e6f16 100644 --- a/crates/forge_infra/src/http.rs +++ b/crates/forge_infra/src/http.rs @@ -4,12 +4,14 @@ use std::time::Duration; use anyhow::Context; use bytes::Bytes; +use chrono::Utc; use forge_app::HttpInfra; use forge_domain::{Environment, TlsBackend, TlsVersion}; use reqwest::header::{AUTHORIZATION, HeaderMap, HeaderValue}; use reqwest::redirect::Policy; use reqwest::{Certificate, Client, Response, StatusCode, Url}; use reqwest_eventsource::{EventSource, RequestBuilderExt}; +use serde_json::{Map, Value, json}; use tracing::{debug, warn}; const VERSION: &str = match option_env!("APP_VERSION") { @@ -118,7 +120,7 @@ impl ForgeHttpInfra { let mut request_headers = self.headers(headers); request_headers.insert("Content-Type", HeaderValue::from_static("application/json")); - self.write_debug_request(&body); + self.write_debug_request("POST", url, &request_headers, &body); self.execute_request("POST", url, |client| { client.post(url.clone()).headers(request_headers).body(body) @@ -190,7 +192,7 @@ impl ForgeHttpInfra { } fn sanitize_headers(headers: &HeaderMap) -> HeaderMap { - let sensitive_headers = [AUTHORIZATION.as_str()]; + let sensitive_headers = [AUTHORIZATION.as_str(), "x-goog-api-key"]; headers .iter() .map(|(name, value)| { @@ -207,13 +209,39 @@ impl ForgeHttpInfra { } impl ForgeHttpInfra { - fn write_debug_request(&self, body: &Bytes) { + fn write_debug_request(&self, method: &str, url: &Url, headers: &HeaderMap, body: &Bytes) { if let Some(debug_path) = &self.env.debug_requests { let file_writer = self.file.clone(); - let body_clone = body.clone(); let debug_path = debug_path.clone(); + let method = method.to_string(); + let url = url.clone(); + let headers = Self::sanitize_headers(headers); + let body = body.clone(); + tokio::spawn(async move { - let _ = file_writer.write(&debug_path, body_clone).await; + let request = serde_json::from_slice::(&body) + .unwrap_or_else(|_| Value::String(String::from_utf8_lossy(&body).into_owned())); + + let headers = headers + .iter() + .map(|(name, value)| { + let value = value.to_str().unwrap_or("[INVALID]").to_string(); + (name.as_str().to_string(), Value::String(value)) + }) + .collect::>(); + + let entry = json!({ + "timestamp": Utc::now().to_rfc3339(), + "method": method, + "url": url, + "headers": headers, + "request": request, + }); + + if let Ok(mut line) = serde_json::to_vec(&entry) { + line.push(b'\n'); + let _ = file_writer.append(&debug_path, Bytes::from(line)).await; + } }); } } @@ -227,7 +255,7 @@ impl ForgeHttpInfra { let mut request_headers = self.headers(headers); request_headers.insert("Content-Type", HeaderValue::from_static("application/json")); - self.write_debug_request(&body); + self.write_debug_request("POST", url, &request_headers, &body); self.client .post(url.clone()) @@ -282,9 +310,13 @@ mod tests { use std::path::PathBuf; use std::sync::Arc; + use bytes::Bytes; use fake::{Fake, Faker}; use forge_app::FileWriterInfra; use forge_domain::{Environment, HttpConfig}; + use pretty_assertions::assert_eq; + use reqwest::header::HeaderValue; + use serde_json::Value; use tokio::sync::Mutex; use super::*; @@ -299,7 +331,7 @@ mod tests { Self { writes: Arc::new(Mutex::new(Vec::new())) } } - async fn get_writes(&self) -> Vec<(PathBuf, Bytes)> { + async fn writes(&self) -> Vec<(PathBuf, Bytes)> { self.writes.lock().await.clone() } } @@ -314,6 +346,14 @@ mod tests { Ok(()) } + async fn append(&self, path: &std::path::Path, contents: Bytes) -> anyhow::Result<()> { + self.writes + .lock() + .await + .push((path.to_path_buf(), contents)); + Ok(()) + } + async fn write_temp( &self, _prefix: &str, @@ -328,139 +368,89 @@ mod tests { Environment { debug_requests, http: HttpConfig::default(), ..Faker.fake() } } - #[tokio::test] - async fn test_debug_requests_none_does_not_write() { - let file_writer = MockFileWriter::new(); - let env = create_test_env(None); - let http = ForgeHttpInfra::new(env, Arc::new(file_writer.clone())); - - let body = Bytes::from("test request body"); - let url = Url::parse("https://api.test.com/messages").unwrap(); - - // Attempt to create eventsource (which triggers debug write if enabled) - let _ = http.eventsource(&url, None, body).await; - - // Give async task time to complete + async fn wait_for_background_write() { tokio::time::sleep(tokio::time::Duration::from_millis(100)).await; - - let writes = file_writer.get_writes().await; - assert_eq!( - writes.len(), - 0, - "No files should be written when debug_requests is None" - ); } - #[tokio::test] - async fn test_debug_requests_with_valid_path() { - let file_writer = MockFileWriter::new(); - let debug_path = PathBuf::from("/tmp/forge-test/debug.json"); - let env = create_test_env(Some(debug_path.clone())); - let http = ForgeHttpInfra::new(env, Arc::new(file_writer.clone())); - - let body = Bytes::from("test request body"); - let url = Url::parse("https://api.test.com/messages").unwrap(); - - let _ = http.eventsource(&url, None, body.clone()).await; - - // Give async task time to complete - tokio::time::sleep(tokio::time::Duration::from_millis(100)).await; - - let writes = file_writer.get_writes().await; - assert_eq!(writes.len(), 1, "Should write one file"); - assert_eq!(writes[0].0, debug_path); - assert_eq!(writes[0].1, body); + fn parse_jsonl_entry(contents: &Bytes) -> Value { + let fixture = String::from_utf8(contents.to_vec()).unwrap(); + let actual = fixture.trim_end(); + let expected: Value = serde_json::from_str(actual).unwrap(); + expected } #[tokio::test] - async fn test_debug_requests_with_relative_path() { - let file_writer = MockFileWriter::new(); - let debug_path = PathBuf::from("./debug/requests.json"); - let env = create_test_env(Some(debug_path.clone())); - let http = ForgeHttpInfra::new(env, Arc::new(file_writer.clone())); - - let body = Bytes::from("test request body"); + async fn test_debug_requests_none_does_not_write() { + let fixture = MockFileWriter::new(); + let http = ForgeHttpInfra::new(create_test_env(None), Arc::new(fixture.clone())); let url = Url::parse("https://api.test.com/messages").unwrap(); - let _ = http.eventsource(&url, None, body.clone()).await; + let _actual = http + .eventsource(&url, None, Bytes::from("{\"ok\":true}")) + .await; + wait_for_background_write().await; - // Give async task time to complete - tokio::time::sleep(tokio::time::Duration::from_millis(100)).await; - - let writes = file_writer.get_writes().await; - assert_eq!(writes.len(), 1, "Should write one file"); - assert_eq!(writes[0].0, debug_path); - assert_eq!(writes[0].1, body); + let actual = fixture.writes().await; + let expected = Vec::<(PathBuf, Bytes)>::new(); + assert_eq!(actual, expected); } #[tokio::test] - async fn test_debug_requests_post_none_does_not_write() { - let file_writer = MockFileWriter::new(); - let env = create_test_env(None); - let http = ForgeHttpInfra::new(env, Arc::new(file_writer.clone())); - - let body = Bytes::from("test request body"); - let url = Url::parse("http://127.0.0.1:9/responses").unwrap(); + async fn test_eventsource_debug_requests_append_jsonl_entry() { + let fixture = MockFileWriter::new(); + let debug_path = PathBuf::from("/tmp/forge-test/debug.jsonl"); + let http = ForgeHttpInfra::new( + create_test_env(Some(debug_path.clone())), + Arc::new(fixture.clone()), + ); + let url = Url::parse("https://api.test.com/messages").unwrap(); + let body = Bytes::from_static(br#"{"request":"body"}"#); - let _ = http.post(&url, None, body).await; + let _actual = http.eventsource(&url, None, body).await; + wait_for_background_write().await; - // Give async task time to complete - tokio::time::sleep(tokio::time::Duration::from_millis(100)).await; + let actual = fixture.writes().await; + assert_eq!(actual.len(), 1); + assert_eq!(actual[0].0, debug_path); - let writes = file_writer.get_writes().await; + let actual = parse_jsonl_entry(&actual[0].1); + assert_eq!(actual["method"], Value::String("POST".to_string())); + assert_eq!(actual["url"], Value::String(url.to_string())); + assert_eq!(actual["request"], serde_json::json!({"request": "body"})); assert_eq!( - writes.len(), - 0, - "No files should be written for POST when debug_requests is None" + actual["headers"]["content-type"], + Value::String("application/json".to_string()) ); } #[tokio::test] - async fn test_debug_requests_post_writes_body() { - let file_writer = MockFileWriter::new(); - let debug_path = PathBuf::from("/tmp/forge-test/debug-post.json"); - let env = create_test_env(Some(debug_path.clone())); - let http = ForgeHttpInfra::new(env, Arc::new(file_writer.clone())); - - let body = Bytes::from("test request body"); + async fn test_post_debug_requests_append_jsonl_entry_and_redact_headers() { + let fixture = MockFileWriter::new(); + let debug_path = PathBuf::from("/tmp/forge-test/debug-post.jsonl"); + let http = ForgeHttpInfra::new( + create_test_env(Some(debug_path.clone())), + Arc::new(fixture.clone()), + ); let url = Url::parse("http://127.0.0.1:9/responses").unwrap(); - - let _ = http.post(&url, None, body.clone()).await; - - // Give async task time to complete - tokio::time::sleep(tokio::time::Duration::from_millis(100)).await; - - let writes = file_writer.get_writes().await; + let mut headers = HeaderMap::new(); + headers.insert("x-goog-api-key", HeaderValue::from_static("secret")); + + let _actual = http + .post(&url, Some(headers), Bytes::from("not-json")) + .await; + wait_for_background_write().await; + + let actual = fixture.writes().await; + assert_eq!(actual.len(), 1); + assert_eq!(actual[0].0, debug_path); + + let actual = parse_jsonl_entry(&actual[0].1); + assert_eq!(actual["method"], Value::String("POST".to_string())); + assert_eq!(actual["url"], Value::String(url.to_string())); + assert_eq!(actual["request"], Value::String("not-json".to_string())); assert_eq!( - writes.len(), - 1, - "Should write one file for POST when debug_requests is set" + actual["headers"]["x-goog-api-key"], + Value::String("[REDACTED]".to_string()) ); - assert_eq!(writes[0].0, debug_path); - assert_eq!(writes[0].1, body); - } - - #[tokio::test] - async fn test_debug_requests_fallback_on_dir_creation_failure() { - let file_writer = MockFileWriter::new(); - // Use a path with a parent that doesn't exist and can't be created - // (in practice, this would be a permission issue) - let debug_path = PathBuf::from("test_debug.json"); - let env = create_test_env(Some(debug_path.clone())); - let http = ForgeHttpInfra::new(env, Arc::new(file_writer.clone())); - - let body = Bytes::from("test request body"); - let url = Url::parse("https://api.test.com/messages").unwrap(); - - let _ = http.eventsource(&url, None, body.clone()).await; - - // Give async task time to complete - tokio::time::sleep(tokio::time::Duration::from_millis(100)).await; - - let writes = file_writer.get_writes().await; - // Should write to debug_path (no parent dir needed) - assert_eq!(writes.len(), 1, "Should write one file"); - assert_eq!(writes[0].0, debug_path); - assert_eq!(writes[0].1, body); } } diff --git a/crates/forge_repo/src/forge_repo.rs b/crates/forge_repo/src/forge_repo.rs index 82df21eda0..cf38648aec 100644 --- a/crates/forge_repo/src/forge_repo.rs +++ b/crates/forge_repo/src/forge_repo.rs @@ -311,6 +311,11 @@ where async fn write(&self, path: &Path, contents: Bytes) -> anyhow::Result<()> { self.infra.write(path, contents).await } + + async fn append(&self, path: &Path, contents: Bytes) -> anyhow::Result<()> { + self.infra.append(path, contents).await + } + async fn write_temp(&self, prefix: &str, ext: &str, content: &str) -> anyhow::Result { self.infra.write_temp(prefix, ext, content).await } diff --git a/crates/forge_repo/src/provider/provider_repo.rs b/crates/forge_repo/src/provider/provider_repo.rs index 0738fa8c89..2e58e39a8d 100644 --- a/crates/forge_repo/src/provider/provider_repo.rs +++ b/crates/forge_repo/src/provider/provider_repo.rs @@ -791,6 +791,10 @@ mod env_tests { Ok(()) } + async fn append(&self, path: &std::path::Path, content: Bytes) -> anyhow::Result<()> { + self.write(path, content).await + } + async fn write_temp( &self, _prefix: &str, @@ -1267,6 +1271,10 @@ mod env_tests { Ok(()) } + async fn append(&self, _path: &std::path::Path, _content: Bytes) -> anyhow::Result<()> { + Ok(()) + } + async fn write_temp( &self, _prefix: &str, diff --git a/crates/forge_services/src/attachment.rs b/crates/forge_services/src/attachment.rs index d1ee4a63a4..675d17a026 100644 --- a/crates/forge_services/src/attachment.rs +++ b/crates/forge_services/src/attachment.rs @@ -386,6 +386,23 @@ pub mod tests { Ok(()) } + async fn append(&self, path: &Path, contents: Bytes) -> anyhow::Result<()> { + let index = self.files.lock().unwrap().iter().position(|v| v.0 == path); + if let Some(index) = index { + let mut files = self.files.lock().unwrap(); + let existing = files[index].1.clone(); + let mut merged = existing.to_vec(); + merged.extend_from_slice(contents.as_ref()); + files[index] = (path.to_path_buf(), Bytes::from(merged)); + } else { + self.files + .lock() + .unwrap() + .push((path.to_path_buf(), contents)); + } + Ok(()) + } + async fn write_temp(&self, _: &str, _: &str, content: &str) -> anyhow::Result { let temp_dir = crate::utils::TempDir::new().unwrap(); let path = temp_dir.path(); diff --git a/scripts/prompt-cache-miss-analyzer.html b/scripts/prompt-cache-miss-analyzer.html new file mode 100644 index 0000000000..00e60b04ac --- /dev/null +++ b/scripts/prompt-cache-miss-analyzer.html @@ -0,0 +1,1611 @@ + + + + + + Prompt Cache Miss Analyzer + + + + +
+ + +
+
+

Diff results

+
+
+
+
+ + + + + +