mirror of
https://github.com/openai/codex.git
synced 2026-10-31 19:28:49 +00:00
## What changed When a diagnostic upload includes logs, include saved threads attached directly to the reported thread in the existing bounded thread selection alongside spawned descendants. Query attached threads from the state database, including archived threads, so their rollouts, logs, and retained review failures can be collected. Log a warning and continue if the lookup fails. ## Testing Extend the upload integration test to verify that the rollout archive includes the reported and directly attached threads, excludes unrelated threads, and is omitted when logs are disabled. GitOrigin-RevId: 1bcd2794938339fb4b1c433c3731f507601cd8a4
654 lines
26 KiB
Rust
654 lines
26 KiB
Rust
use std::time::Duration;
|
|
|
|
use anyhow::Result;
|
|
use app_test_support::TestAppServer;
|
|
use codex_app_server_protocol::RequestId;
|
|
use pretty_assertions::assert_eq;
|
|
use serde_json::json;
|
|
use tokio::io::AsyncBufReadExt;
|
|
use tokio::io::AsyncWriteExt;
|
|
use tokio::io::BufReader;
|
|
use tokio::net::TcpListener;
|
|
use tokio::time::timeout;
|
|
|
|
#[tokio::test]
|
|
async fn feedback_upload_limits_concurrency_and_releases_failed_uploads() -> Result<()> {
|
|
let proxy = TcpListener::bind("127.0.0.1:0").await?;
|
|
let proxy_uri = format!("http://{}", proxy.local_addr()?);
|
|
let mut app_server = TestAppServer::builder()
|
|
.with_env_overrides(&[
|
|
("HTTPS_PROXY", Some(proxy_uri.as_str())),
|
|
("https_proxy", Some(proxy_uri.as_str())),
|
|
("NO_PROXY", Some("")),
|
|
("no_proxy", Some("")),
|
|
])
|
|
.build_initialized()
|
|
.await?;
|
|
|
|
let mut pending = Vec::new();
|
|
for _ in 0..3 {
|
|
let request_id = app_server
|
|
.send_raw_request(
|
|
"feedback/upload",
|
|
Some(json!({ "classification": "bug", "includeLogs": false })),
|
|
)
|
|
.await?;
|
|
let (stream, _) = timeout(Duration::from_secs(/*secs*/ 15), proxy.accept()).await??;
|
|
let mut stream = BufReader::new(stream);
|
|
let mut request = String::new();
|
|
timeout(
|
|
Duration::from_secs(/*secs*/ 15),
|
|
stream.read_line(&mut request),
|
|
)
|
|
.await??;
|
|
assert!(request.starts_with("CONNECT "));
|
|
pending.push((request_id, stream.into_inner()));
|
|
}
|
|
|
|
let excess_id = app_server
|
|
.send_raw_request(
|
|
"feedback/upload",
|
|
Some(json!({ "classification": "bug", "includeLogs": false })),
|
|
)
|
|
.await?;
|
|
let error = timeout(
|
|
Duration::from_secs(/*secs*/ 15),
|
|
app_server.read_stream_until_error_message(RequestId::Integer(excess_id)),
|
|
)
|
|
.await??;
|
|
|
|
assert_eq!(error.error.code, -32001);
|
|
|
|
let unavailable =
|
|
b"HTTP/1.1 503 Service Unavailable\r\nContent-Length: 0\r\nConnection: close\r\n\r\n";
|
|
for (request_id, mut stream) in pending {
|
|
stream.write_all(unavailable).await?;
|
|
stream.shutdown().await?;
|
|
let error = timeout(
|
|
Duration::from_secs(/*secs*/ 15),
|
|
app_server.read_stream_until_error_message(RequestId::Integer(request_id)),
|
|
)
|
|
.await??;
|
|
assert_eq!(error.error.code, -32603);
|
|
assert!(error.error.message.contains("failed to upload feedback"));
|
|
}
|
|
|
|
let request_id = app_server
|
|
.send_raw_request(
|
|
"feedback/upload",
|
|
Some(json!({ "classification": "bug", "includeLogs": false })),
|
|
)
|
|
.await?;
|
|
let (mut stream, _) = timeout(Duration::from_secs(/*secs*/ 15), proxy.accept()).await??;
|
|
stream.write_all(unavailable).await?;
|
|
stream.shutdown().await?;
|
|
let error = timeout(
|
|
Duration::from_secs(/*secs*/ 15),
|
|
app_server.read_stream_until_error_message(RequestId::Integer(request_id)),
|
|
)
|
|
.await??;
|
|
assert_eq!(error.error.code, -32603);
|
|
Ok(())
|
|
}
|
|
|
|
#[tokio::test]
|
|
async fn feedback_upload_recovers_persisted_evidence_and_reports_sqlite_failures() -> Result<()> {
|
|
use std::collections::HashSet;
|
|
use std::io::Read;
|
|
use std::sync::Arc;
|
|
|
|
use app_test_support::MockResponsesConfig;
|
|
use codex_app_server_protocol::ClientNotification;
|
|
use codex_protocol::ThreadId;
|
|
use tokio::io::AsyncReadExt;
|
|
use tokio_rustls::TlsAcceptor;
|
|
use tokio_rustls::rustls;
|
|
|
|
// Terminate HTTPS locally and capture real Sentry envelopes. Never forward uploads.
|
|
let home = tempfile::tempdir()?;
|
|
let daemon_logs = home.path().join("app-server-daemon");
|
|
std::fs::create_dir(&daemon_logs)?;
|
|
std::fs::write(
|
|
daemon_logs.join("daemon.stderr.log"),
|
|
"current daemon diagnostic",
|
|
)?;
|
|
std::fs::write(
|
|
daemon_logs.join("daemon-updater.stderr.log.previous"),
|
|
"previous updater failure",
|
|
)?;
|
|
std::fs::write(
|
|
daemon_logs.join("daemon.stderr.log.previous"),
|
|
"previous daemon startup failure",
|
|
)?;
|
|
std::fs::write(
|
|
daemon_logs.join("daemon-updater.stderr.log"),
|
|
"current updater diagnostic",
|
|
)?;
|
|
let cert = rcgen::generate_simple_self_signed(vec!["o33249.ingest.us.sentry.io".to_string()])?;
|
|
let cert_path = home.path().join("feedback-ca.pem");
|
|
std::fs::write(&cert_path, cert.cert.pem())?;
|
|
let tls = rustls::ServerConfig::builder_with_provider(Arc::new(
|
|
rustls::crypto::aws_lc_rs::default_provider(),
|
|
))
|
|
.with_safe_default_protocol_versions()?
|
|
.with_no_client_auth()
|
|
.with_single_cert(
|
|
vec![cert.cert.der().clone()],
|
|
rustls::pki_types::PrivatePkcs8KeyDer::from(cert.signing_key.serialize_der()).into(),
|
|
)?;
|
|
let acceptor = TlsAcceptor::from(Arc::new(tls));
|
|
let listener = TcpListener::bind("127.0.0.1:0").await?;
|
|
let proxy_uri = format!("http://{}", listener.local_addr()?);
|
|
let (uploads_tx, mut uploads_rx) = tokio::sync::mpsc::unbounded_channel();
|
|
let _proxy = tokio_util::task::AbortOnDropHandle::new(tokio::spawn(async move {
|
|
while let Ok((stream, _)) = listener.accept().await {
|
|
let mut stream = BufReader::new(stream);
|
|
loop {
|
|
let mut line = String::new();
|
|
anyhow::ensure!(stream.read_line(&mut line).await? > 0, "missing CONNECT");
|
|
if line == "\r\n" {
|
|
break;
|
|
}
|
|
}
|
|
stream
|
|
.write_all(b"HTTP/1.1 200 Connection Established\r\n\r\n")
|
|
.await?;
|
|
let mut stream = BufReader::new(acceptor.accept(stream.into_inner()).await?);
|
|
let mut length = None;
|
|
loop {
|
|
let mut line = String::new();
|
|
anyhow::ensure!(
|
|
stream.read_line(&mut line).await? > 0,
|
|
"missing HTTP headers"
|
|
);
|
|
if line == "\r\n" {
|
|
break;
|
|
}
|
|
if let Some((name, value)) = line.split_once(':')
|
|
&& name.eq_ignore_ascii_case("content-length")
|
|
{
|
|
length = Some(value.trim().parse::<usize>()?);
|
|
}
|
|
}
|
|
let length = length.ok_or_else(|| anyhow::anyhow!("missing content length"))?;
|
|
anyhow::ensure!(length < 16 * 1024 * 1024, "unexpected envelope size");
|
|
let mut body = vec![0; length];
|
|
stream.read_exact(&mut body).await?;
|
|
if body.starts_with(&[0x1f, 0x8b]) {
|
|
let mut decoded = Vec::new();
|
|
flate2::read::GzDecoder::new(body.as_slice()).read_to_end(&mut decoded)?;
|
|
body = decoded;
|
|
}
|
|
uploads_tx.send(body)?;
|
|
stream
|
|
.write_all(b"HTTP/1.1 200 OK\r\nContent-Length: 2\r\nConnection: close\r\n\r\n{}")
|
|
.await?;
|
|
stream.flush().await?;
|
|
}
|
|
Ok::<(), anyhow::Error>(())
|
|
}));
|
|
|
|
let models = wiremock::MockServer::start().await;
|
|
MockResponsesConfig::new(&models.uri())
|
|
.with_root_config(&format!("log_dir = {:?}", home.path().join("log")))
|
|
.write(home.path())?;
|
|
let thread_id = ThreadId::new();
|
|
let attached_id = ThreadId::new();
|
|
let sqlite = core_test_support::load_default_config_for_test(&home)
|
|
.await
|
|
.sqlite;
|
|
let state =
|
|
codex_state::StateRuntime::init(sqlite.clone(), "mock_provider".to_string()).await?;
|
|
let review = json!({
|
|
"reviewed_thread_id": thread_id,
|
|
"status": "denied",
|
|
"action": "original reviewed action",
|
|
"instructions": "original review instructions",
|
|
"history": [{"role": "user", "content": "original authorization"}],
|
|
"decision": "original denial",
|
|
"context_omitted": false,
|
|
});
|
|
for (id, parent, record) in [
|
|
(thread_id, None, Some(review.clone())),
|
|
(attached_id, Some(thread_id), None),
|
|
(ThreadId::new(), None, Some(json!({"unrelated": true}))),
|
|
] {
|
|
let rollout_path = home
|
|
.path()
|
|
.join(format!("rollout-2026-10-01T12-00-00-{id}.jsonl"));
|
|
std::fs::write(&rollout_path, serde_json::to_vec(&id)?)?;
|
|
let mut metadata = codex_state::ThreadMetadataBuilder::new(
|
|
id,
|
|
rollout_path,
|
|
chrono::Utc::now(),
|
|
codex_protocol::protocol::SessionSource::Cli,
|
|
)
|
|
.build("mock_provider");
|
|
metadata.attached_thread_parent_id = parent;
|
|
state.upsert_thread(&metadata).await?;
|
|
if let Some(record) = record {
|
|
state
|
|
.record_guardian_review_failure(&codex_feedback::GuardianReviewRecord::new(
|
|
id,
|
|
serde_json::to_vec(&record)?,
|
|
))
|
|
.await?;
|
|
}
|
|
}
|
|
state.close().await;
|
|
let legacy_path = home.path().join("logs_2.sqlite");
|
|
std::fs::copy(sqlite.logs_db_path(), &legacy_path)?;
|
|
let legacy_pool = sqlite.open_read_write_pool(&legacy_path).await?;
|
|
let legacy_timestamp = chrono::Utc::now();
|
|
sqlx::query(
|
|
"INSERT INTO logs (ts, ts_nanos, level, target, feedback_log_body, thread_id)
|
|
VALUES (?, 0, 'INFO', 'test', 'legacy history', ?)",
|
|
)
|
|
.bind(legacy_timestamp.timestamp())
|
|
.bind(thread_id.to_string())
|
|
.execute(&legacy_pool)
|
|
.await?;
|
|
legacy_pool.close().await;
|
|
// This new process can recover review context only from SQLite.
|
|
let mut app_server = TestAppServer::builder()
|
|
.with_codex_home(home.path())
|
|
.with_env_overrides(&[
|
|
("HTTPS_PROXY", Some(proxy_uri.as_str())),
|
|
("https_proxy", Some(proxy_uri.as_str())),
|
|
("NO_PROXY", Some("127.0.0.1,localhost")),
|
|
("no_proxy", Some("127.0.0.1,localhost")),
|
|
("SSL_CERT_FILE", cert_path.to_str()),
|
|
])
|
|
.build_initialized()
|
|
.await?;
|
|
let pool = sqlite.open_read_write_pool(&sqlite.logs_db_path()).await?;
|
|
// This old history exists only in SQLite, as if it had already left the memory ring.
|
|
sqlx::query("INSERT INTO logs (ts, ts_nanos, level, target, feedback_log_body, thread_id) VALUES (unixepoch(), 0, 'INFO', 'test', 'sqlite-only-history', ?)")
|
|
.bind(thread_id.to_string()).execute(&pool).await?;
|
|
sqlx::query("CREATE TRIGGER fail_log_insert BEFORE INSERT ON logs BEGIN SELECT RAISE(ABORT, 'synthetic-secret'); END")
|
|
.execute(&pool).await?;
|
|
|
|
#[derive(Debug, PartialEq)]
|
|
enum Phase {
|
|
WithoutLogs,
|
|
WriteFailure,
|
|
Corrupt,
|
|
}
|
|
for phase in [Phase::WithoutLogs, Phase::WriteFailure, Phase::Corrupt] {
|
|
match phase {
|
|
Phase::Corrupt => {
|
|
sqlx::raw_sql("PRAGMA writable_schema = ON; UPDATE sqlite_schema SET rootpage = 2147483647 WHERE name = 'logs'; PRAGMA schema_version = 1000000;").execute(&pool).await?;
|
|
}
|
|
Phase::WithoutLogs | Phase::WriteFailure => {}
|
|
}
|
|
// Notifications emit an INFO log, ensuring feedback/upload has a batch to flush.
|
|
app_server
|
|
.send_notification(ClientNotification::Initialized)
|
|
.await?;
|
|
// The periodic writer must notify the user before they choose to submit feedback.
|
|
if phase == Phase::WriteFailure {
|
|
let notification = timeout(
|
|
Duration::from_secs(/*secs*/ 20),
|
|
app_server.read_stream_until_notification_message("warning"),
|
|
)
|
|
.await??;
|
|
let warning: codex_app_server_protocol::WarningNotification =
|
|
serde_json::from_value(notification.params.unwrap())?;
|
|
assert_eq!(warning, codex_app_server_protocol::WarningNotification {
|
|
thread_id: None,
|
|
message: "Codex couldn't save diagnostic logs to its local database. Use /feedback with logs included before closing Codex, or run `codex doctor` for diagnostics.".to_string(),
|
|
});
|
|
}
|
|
let request = app_server
|
|
.send_raw_request(
|
|
"feedback/upload",
|
|
Some(json!({
|
|
"classification": "bug", "includeLogs": phase != Phase::WithoutLogs, "threadId": thread_id
|
|
})),
|
|
)
|
|
.await?;
|
|
timeout(
|
|
Duration::from_secs(/*secs*/ 30),
|
|
app_server.read_stream_until_response_message(RequestId::Integer(request)),
|
|
)
|
|
.await??;
|
|
let mut attachments = std::collections::BTreeMap::new();
|
|
let mut uploaded_rollouts = HashSet::<ThreadId>::new();
|
|
while let Ok(envelope) = uploads_rx.try_recv() {
|
|
let mut bytes = envelope.as_slice();
|
|
// Skip the envelope header, then decode each length-delimited item.
|
|
let header_end = bytes.iter().position(|byte| *byte == b'\n').unwrap();
|
|
bytes = &bytes[header_end + 1..];
|
|
while !bytes.is_empty() {
|
|
let header_end = bytes.iter().position(|byte| *byte == b'\n').unwrap();
|
|
let header: serde_json::Value = serde_json::from_slice(&bytes[..header_end])?;
|
|
let length = header["length"].as_u64().unwrap() as usize;
|
|
bytes = &bytes[header_end + 1..];
|
|
if header["filename"] == "rollouts.tar.gz" {
|
|
let mut archive =
|
|
tar::Archive::new(flate2::read::GzDecoder::new(&bytes[..length]));
|
|
for entry in archive.entries()? {
|
|
uploaded_rollouts.insert(serde_json::from_reader(entry?)?);
|
|
}
|
|
} else if let Some(filename) = header["filename"].as_str() {
|
|
attachments.insert(
|
|
filename.to_string(),
|
|
String::from_utf8_lossy(&bytes[..length]).into_owned(),
|
|
);
|
|
}
|
|
bytes = &bytes[length..];
|
|
if bytes.starts_with(b"\n") {
|
|
bytes = &bytes[1..];
|
|
}
|
|
}
|
|
}
|
|
if phase == Phase::WithoutLogs {
|
|
assert!(uploaded_rollouts.is_empty());
|
|
assert!(!attachments.contains_key("auto-review-failures.jsonl"));
|
|
assert!(!attachments.contains_key("codex-logs.log"));
|
|
assert!(!attachments.contains_key("codex-logs-legacy.log"));
|
|
continue;
|
|
}
|
|
assert_eq!(uploaded_rollouts, HashSet::from([thread_id, attached_id]));
|
|
assert_eq!(
|
|
attachments.get("codex-logs-legacy.log"),
|
|
(phase == Phase::Corrupt)
|
|
.then(|| format!(
|
|
"{} INFO legacy history\n",
|
|
legacy_timestamp.format("%Y-%m-%dT%H:%M:%S.000000Z")
|
|
))
|
|
.as_ref(),
|
|
);
|
|
assert_eq!(
|
|
serde_json::from_str::<serde_json::Value>(&attachments["auto-review-failures.jsonl"])?,
|
|
review,
|
|
);
|
|
assert!(!attachments.contains_key("daemon.stderr.log"));
|
|
assert!(!attachments.contains_key("daemon.stderr.log.previous"));
|
|
assert_eq!(
|
|
attachments["daemon-updater.stderr.log"],
|
|
"current updater diagnostic"
|
|
);
|
|
assert_eq!(
|
|
attachments["daemon-updater.stderr.log.previous"],
|
|
"previous updater failure"
|
|
);
|
|
let logs = &attachments["codex-logs.log"];
|
|
if phase == Phase::Corrupt {
|
|
assert!(
|
|
!app_server
|
|
.pending_notification_methods()
|
|
.iter()
|
|
.any(|method| method == "warning")
|
|
);
|
|
assert!(logs.contains("failed to flush logs to SQLite error=\"corrupt\""));
|
|
assert!(logs.contains("failed to query feedback logs from sqlite"));
|
|
} else {
|
|
assert!(logs.contains("failed to flush logs to SQLite error=\"constraint\""));
|
|
assert!(!logs.contains("sqlite-only-history"));
|
|
assert!(!logs.contains("synthetic-secret"));
|
|
}
|
|
}
|
|
pool.close().await;
|
|
Ok(())
|
|
}
|
|
|
|
/// Disabling uploads still permits local ZIPs, with invalid paths and collisions leaving existing files intact.
|
|
#[tokio::test]
|
|
async fn feedback_save_is_local_and_never_overwrites() -> Result<()> {
|
|
let proxy = TcpListener::bind("127.0.0.1:0").await?;
|
|
let proxy_uri = format!("http://{}", proxy.local_addr()?);
|
|
let mut app_server = TestAppServer::builder()
|
|
.with_args(&["-c", "feedback.enabled=false"])
|
|
.with_env_overrides(&[
|
|
("HTTPS_PROXY", Some(proxy_uri.as_str())),
|
|
("https_proxy", Some(proxy_uri.as_str())),
|
|
("NO_PROXY", Some("")),
|
|
("no_proxy", Some("")),
|
|
])
|
|
.build_initialized()
|
|
.await?;
|
|
let request = app_server
|
|
.send_raw_request("feedback/upload", Some(json!({ "classification": "bug" })))
|
|
.await?;
|
|
let error = app_server
|
|
.read_stream_until_error_message(RequestId::Integer(request))
|
|
.await?;
|
|
assert_eq!(
|
|
error.error.message,
|
|
"sending feedback is disabled by configuration"
|
|
);
|
|
|
|
let directory = tempfile::tempdir()?;
|
|
let saved = directory.path().join("local.zip");
|
|
std::fs::write(&saved, b"existing export")?;
|
|
let foreign = if cfg!(windows) {
|
|
"/tmp/foreign.zip"
|
|
} else {
|
|
r"C:\Users\test\foreign.zip"
|
|
};
|
|
for (path, message) in [
|
|
(json!("relative.zip"), "absolute host path"),
|
|
(json!("~/feedback.zip"), "absolute host path"),
|
|
(json!(foreign), "absolute host path"),
|
|
(json!(&saved), "already exists"),
|
|
(
|
|
json!(directory.path().join("missing/feedback.zip")),
|
|
"failed to save feedback",
|
|
),
|
|
] {
|
|
let request = app_server
|
|
.send_raw_request(
|
|
"feedback/save",
|
|
Some(json!({
|
|
"classification": "bug", "savePath": path,
|
|
})),
|
|
)
|
|
.await?;
|
|
let error = app_server
|
|
.read_stream_until_error_message(RequestId::Integer(request))
|
|
.await?;
|
|
assert!(error.error.message.contains(message), "{error:?}");
|
|
}
|
|
assert_eq!(std::fs::read(&saved)?, b"existing export");
|
|
assert_eq!(std::fs::read_dir(directory.path())?.count(), 1);
|
|
std::fs::remove_file(&saved)?;
|
|
let request = app_server
|
|
.send_raw_request(
|
|
"feedback/save",
|
|
Some(json!({
|
|
"classification": "bug", "savePath": saved,
|
|
})),
|
|
)
|
|
.await?;
|
|
let response = app_server
|
|
.read_stream_until_response_message(RequestId::Integer(request))
|
|
.await?;
|
|
assert_eq!(response.result["savedPath"], json!(saved));
|
|
let mut archive = zip::ZipArchive::new(std::fs::File::open(&saved)?)?;
|
|
let report: serde_json::Value = serde_json::from_reader(archive.by_name("report.json")?)?;
|
|
assert_eq!(report["tags"]["classification"], "bug");
|
|
assert!(
|
|
timeout(Duration::from_millis(100), proxy.accept())
|
|
.await
|
|
.is_err()
|
|
);
|
|
Ok(())
|
|
}
|
|
|
|
#[tokio::test]
|
|
async fn feedback_save_preserves_selected_rollout_or_fails_without_upload() -> Result<()> {
|
|
use std::io::Read;
|
|
|
|
let proxy = TcpListener::bind("127.0.0.1:0").await?;
|
|
let proxy_uri = format!("http://{}", proxy.local_addr()?);
|
|
let mut app_server = TestAppServer::builder()
|
|
.with_env_overrides(&[
|
|
("HTTPS_PROXY", Some(proxy_uri.as_str())),
|
|
("https_proxy", Some(proxy_uri.as_str())),
|
|
("NO_PROXY", Some("")),
|
|
("no_proxy", Some("")),
|
|
])
|
|
.build_initialized()
|
|
.await?;
|
|
let directory = tempfile::tempdir()?;
|
|
let filename = format!(
|
|
"rollout-2026-10-19T12-00-00-{}.jsonl",
|
|
codex_protocol::ThreadId::new()
|
|
);
|
|
let rollout = directory.path().join(&filename);
|
|
let bytes = b"{\"synthetic\":true}\n";
|
|
std::fs::write(&rollout, bytes)?;
|
|
let saved = directory.path().join("selected.zip");
|
|
let request = app_server.send_raw_request("feedback/save", Some(json!({
|
|
"classification": "bug", "includeLogs": true, "extraLogFiles": [rollout], "savePath": saved,
|
|
}))).await?;
|
|
let response = app_server
|
|
.read_stream_until_response_message(RequestId::Integer(request))
|
|
.await?;
|
|
assert_eq!(response.result["savedPath"], json!(saved));
|
|
let mut zip = zip::ZipArchive::new(std::fs::File::open(&saved)?)?;
|
|
let entry = zip
|
|
.file_names()
|
|
.collect::<zip::result::ZipResult<Vec<_>>>()?
|
|
.into_iter()
|
|
.find(|name| name.ends_with(&format!("/{filename}")))
|
|
.expect("selected rollout")
|
|
.to_string();
|
|
let mut actual = Vec::new();
|
|
zip.by_name(&entry)?.read_to_end(&mut actual)?;
|
|
assert_eq!(actual, bytes);
|
|
drop(zip);
|
|
std::fs::remove_file(&rollout)?;
|
|
let missing = directory.path().join("missing.zip");
|
|
let request = app_server.send_raw_request("feedback/save", Some(json!({
|
|
"classification": "bug", "includeLogs": true, "extraLogFiles": [rollout], "savePath": missing,
|
|
}))).await?;
|
|
let error = app_server
|
|
.read_stream_until_error_message(RequestId::Integer(request))
|
|
.await?;
|
|
assert_eq!(error.error.code, -32603);
|
|
assert!(error.error.message.starts_with("failed to save feedback:"));
|
|
assert!(error.error.message.contains("selected attachment"));
|
|
assert!(!missing.exists());
|
|
assert_eq!(std::fs::read_dir(directory.path())?.count(), 1);
|
|
assert!(
|
|
timeout(Duration::from_millis(100), proxy.accept())
|
|
.await
|
|
.is_err()
|
|
);
|
|
Ok(())
|
|
}
|
|
|
|
/// Automatic untouched rollouts are omitted; explicit missing and deleted persisted files fail.
|
|
#[tokio::test]
|
|
async fn feedback_save_handles_rollout_materialization() -> Result<()> {
|
|
use codex_app_server_protocol::ThreadStartParams;
|
|
use std::io::Read;
|
|
let server = app_test_support::create_mock_responses_server_repeating_assistant("Done").await;
|
|
let home = tempfile::tempdir()?;
|
|
app_test_support::MockResponsesConfig::new(&server.uri()).write(home.path())?;
|
|
let proxy = TcpListener::bind("127.0.0.1:0").await?;
|
|
let proxy_uri = format!("http://{}", proxy.local_addr()?);
|
|
let mut app = TestAppServer::builder()
|
|
.with_codex_home(home.path())
|
|
.with_args(&["-c", "feedback.enabled=false"])
|
|
.with_env_overrides(&[
|
|
("HTTPS_PROXY", Some(proxy_uri.as_str())),
|
|
("https_proxy", Some(proxy_uri.as_str())),
|
|
("NO_PROXY", Some("127.0.0.1,localhost")),
|
|
("no_proxy", Some("127.0.0.1,localhost")),
|
|
])
|
|
.build_initialized()
|
|
.await?;
|
|
let request = app
|
|
.send_thread_start_request_with_auto_env(ThreadStartParams::default())
|
|
.await?;
|
|
let response = app
|
|
.read_stream_until_response_message(RequestId::Integer(request))
|
|
.await?;
|
|
let thread = response.result["thread"]["id"].as_str().unwrap();
|
|
let rollout = std::path::PathBuf::from(response.result["thread"]["path"].as_str().unwrap());
|
|
assert!(!rollout.exists());
|
|
let output = tempfile::tempdir()?;
|
|
for explicit in [false, true] {
|
|
let destination = output.path().join(format!("fresh-{explicit}.zip"));
|
|
let request = app.send_raw_request("feedback/save", Some(json!({"classification":"bug", "includeLogs":true, "threadId":thread, "extraLogFiles": if explicit { vec![&rollout] } else { vec![] }, "savePath":destination}))).await?;
|
|
if explicit {
|
|
let error = app
|
|
.read_stream_until_error_message(RequestId::Integer(request))
|
|
.await?;
|
|
assert!(error.error.message.contains("selected attachment"));
|
|
assert!(!destination.exists());
|
|
} else {
|
|
app.read_stream_until_response_message(RequestId::Integer(request))
|
|
.await?;
|
|
let mut zip = zip::ZipArchive::new(std::fs::File::open(destination)?)?;
|
|
let names = zip
|
|
.file_names()
|
|
.collect::<zip::result::ZipResult<Vec<_>>>()?
|
|
.into_iter()
|
|
.map(std::borrow::Cow::into_owned)
|
|
.collect::<Vec<_>>();
|
|
assert!(
|
|
!names
|
|
.iter()
|
|
.any(|name| name.ends_with(".jsonl") || name.ends_with("rollouts.tar.gz"))
|
|
);
|
|
let index_name = names
|
|
.iter()
|
|
.find(|name| name.ends_with("feedback-thread-index.json"))
|
|
.expect("thread index");
|
|
let index: serde_json::Value = serde_json::from_reader(zip.by_name(index_name)?)?;
|
|
assert_eq!(
|
|
index["threads"][0]["rollout_filename"],
|
|
serde_json::Value::Null
|
|
);
|
|
assert_eq!(index["rollout_archive_filename"], serde_json::Value::Null);
|
|
}
|
|
}
|
|
assert!(!rollout.exists(), "saving must not materialize the rollout");
|
|
let request = app
|
|
.send_raw_request(
|
|
"turn/start",
|
|
Some(json!({"threadId":thread,"input":[{"type":"text","text":"hello"}]})),
|
|
)
|
|
.await?;
|
|
app.read_stream_until_response_message(RequestId::Integer(request))
|
|
.await?;
|
|
app.read_stream_until_notification_message("turn/completed")
|
|
.await?;
|
|
assert!(rollout.exists());
|
|
let expected = std::fs::read(&rollout)?;
|
|
let saved = output.path().join("materialized.zip");
|
|
let request = app.send_raw_request("feedback/save", Some(json!({"classification":"bug", "includeLogs":true, "threadId":thread, "savePath":saved}))).await?;
|
|
app.read_stream_until_response_message(RequestId::Integer(request))
|
|
.await?;
|
|
let mut zip = zip::ZipArchive::new(std::fs::File::open(saved)?)?;
|
|
let suffix = format!("/{}", rollout.file_name().unwrap().to_string_lossy());
|
|
let entry = zip
|
|
.file_names()
|
|
.collect::<zip::result::ZipResult<Vec<_>>>()?
|
|
.into_iter()
|
|
.find(|name| name.ends_with(&suffix))
|
|
.unwrap()
|
|
.into_owned();
|
|
let mut actual = Vec::new();
|
|
zip.by_name(&entry)?.read_to_end(&mut actual)?;
|
|
assert_eq!(actual, expected);
|
|
std::fs::remove_file(&rollout)?;
|
|
let failed = output.path().join("deleted.zip");
|
|
let request = app.send_raw_request("feedback/save", Some(json!({"classification":"bug", "includeLogs":true, "threadId":thread, "savePath":failed}))).await?;
|
|
let error = app
|
|
.read_stream_until_error_message(RequestId::Integer(request))
|
|
.await?;
|
|
assert!(error.error.message.contains("selected attachment"));
|
|
assert!(!failed.exists());
|
|
assert!(
|
|
timeout(Duration::from_millis(100), proxy.accept())
|
|
.await
|
|
.is_err()
|
|
);
|
|
Ok(())
|
|
}
|