Files
codex/codex-rs/app-server/tests/suite/v2/feedback.rs
T
Dmitriy Kovalenko 17a0e6f63f Route SQLite logs to an explicitly configured log_dir (#55978)
## What changed

- Store `logs_2.sqlite` in `log_dir` when explicitly configured, unless a managed `sqlite_home` requirement applies. Keep other databases in `sqlite_home` and preserve the existing SQLite log location when `log_dir` is unset.
- Initialize and lock the selected log directory only when opening its database, so an unavailable directory does not prevent authentication commands from loading configuration. Share the cleanup lock across configuration clones and allow failed initialization to be retried.
- Leave existing logs in place. Read legacy logs for diagnostic submissions when the current query returns no logs, attaching them as `codex-logs-legacy.log`. Continue retention cleanup and page reclamation for both locations, with independent maintenance locks and filesystem checks.

## Testing

Add coverage for configuration routing and managed requirements, deferred initialization and lock lifetimes, authentication with an unavailable log directory, legacy log attachments, and cleanup of current and legacy databases.

GitOrigin-RevId: 48216694596d738eff1b3c5eef6c4e7ada6d0137
2026-10-21 19:53:00 +00:00

639 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::io::Read;
use std::sync::Arc;
use app_test_support::MockResponsesConfig;
use codex_app_server_protocol::ClientNotification;
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 = codex_protocol::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, record) in [
(thread_id, review.clone()),
(codex_protocol::ThreadId::new(), json!({"unrelated": true})),
] {
let rollout_path = home.path().join(format!("{id}.jsonl"));
std::fs::write(&rollout_path, "")?;
state
.upsert_thread(
&codex_state::ThreadMetadataBuilder::new(
id,
rollout_path,
chrono::Utc::now(),
codex_protocol::protocol::SessionSource::Cli,
)
.build("mock_provider"),
)
.await?;
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();
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 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!(!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!(
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(())
}