Identify failed SQLite databases in codex doctor output (#47330)

## Why

The generic state integrity failure message does not identify which database needs attention.

## What changed

- Include the paths of all databases that fail integrity checks in a structured issue, shown in text summaries and JSON output, with the existing recovery guidance.
- Redact sensitive details using `: ` as the field-label separator so Windows drive letters are not mistaken for labels and unlabeled sensitive paths are fully redacted.

## Testing

Add coverage for single and multiple failed databases, summary and detailed text output, JSON output, preservation of damaged database contents, and sensitive path redaction on Unix and Windows.

GitOrigin-RevId: ac5fec9473d22b6b137a9fc8a751aba8605d940f
This commit is contained in:
dkovalenko-oai
2026-09-22 17:59:02 +00:00
committed by copyberry
parent 00d92ad3ee
commit 3b7a8520ef
5 changed files with 203 additions and 17 deletions
+35 -15
View File
@@ -2096,36 +2096,56 @@ async fn state_check(config: &Config, command: &DoctorCommand) -> DoctorCheck {
path_readiness(&mut details, "CODEX_HOME", &config.codex_home);
path_readiness(&mut details, "log dir", &config.log_dir);
path_readiness(&mut details, "sqlite home", config.sqlite_config().home());
let mut status = CheckStatus::Ok;
let mut failed_databases = Vec::new();
for db in config.sqlite_config().runtime_db_paths() {
path_readiness(&mut details, db.label, &db.path);
// Feedback collection gives each database its own budget; direct runs scan fully.
let deadline = command
.feedback
.then(|| Instant::now() + Duration::from_secs(1));
status = status.max(
sqlite_integrity_detail(
config.sqlite_config(),
&mut details,
db.label,
&db.path,
deadline,
)
.await,
);
let db_status = sqlite_integrity_detail(
config.sqlite_config(),
&mut details,
db.label,
&db.path,
deadline,
)
.await;
if db_status == CheckStatus::Fail {
failed_databases.push(db.path.display().to_string());
}
status = status.max(db_status);
}
rollout_stats_details(&mut details, &config.codex_home);
standalone_release_cache_details(&mut details);
let summary = match status {
CheckStatus::Ok => "state paths and databases are inspectable",
CheckStatus::Warning => "some database integrity checks exceeded their time limit",
CheckStatus::Fail => "state database integrity check failed",
CheckStatus::Ok => "state paths and databases are inspectable".to_string(),
CheckStatus::Warning => {
"some database integrity checks exceeded their time limit".to_string()
}
CheckStatus::Fail => "state database integrity check failed".to_string(),
};
let mut check = DoctorCheck::new("state.paths", "state", status, summary).details(details);
if status == CheckStatus::Fail {
check = check.remediation(
"Move the damaged SQLite database aside, then restart the interactive CLI or app server so it can rebuild that runtime database from saved data. Other entry points may not rebuild automatically.",
let noun = if failed_databases.len() == 1 {
"database"
} else {
"databases"
};
check = check.issue(
DoctorIssue::new(
CheckStatus::Fail,
format!(
"{} {noun} failed integrity check",
failed_databases.join(", ")
),
)
.remedy(
"Move the damaged SQLite database aside, then restart the interactive CLI or app server so it can rebuild that runtime database from saved data. Other entry points may not rebuild automatically.",
),
);
}
check
+26 -2
View File
@@ -890,8 +890,11 @@ pub(super) fn redact_detail(detail: &str) -> String {
"secret",
];
if secret_keys.iter().any(|key| lower.contains(key)) {
let name = detail.split(':').next().unwrap_or(detail);
format!("{name}: <redacted>")
// make sure that windows drives in paths are not matches C:/ if they are a first token
detail.split_once(": ").map_or_else(
|| "<redacted>".to_string(),
|(name, _)| format!("{name}: <redacted>"),
)
} else {
redact_urls(detail)
}
@@ -1778,6 +1781,27 @@ Run codex doctor without --summary for detailed diagnostics.
assert!(rendered.contains("\u{1b}[2m(current 0.0.0, dismissed 0.128.0)"));
}
#[test]
fn redact_detail_distinguishes_database_paths_from_field_labels() {
let details = [
r"C:\doctor-secret-sqlite\logs_2.sqlite database failed integrity check",
"C:/doctor-secret-sqlite/logs_2.sqlite database failed integrity check",
"/tmp/doctor-secret-sqlite/logs_2.sqlite database failed integrity check",
r"log database: C:\doctor-secret-sqlite\logs_2.sqlite",
];
assert_eq!(
details.map(redact_detail),
[
"<redacted>",
"<redacted>",
"<redacted>",
"log database: <redacted>",
]
.map(str::to_string)
);
}
#[test]
fn redact_detail_sanitizes_urls() {
let redacted = redact_detail(
+130
View File
@@ -10,6 +10,8 @@ use anyhow::Result;
use app_test_support::TestAppServer;
use codex_app_server_protocol::RequestId;
use codex_config::loader::project_trust_key;
use codex_state::SqliteConfig;
use codex_utils_absolute_path::AbsolutePathBuf;
use pretty_assertions::assert_eq;
use serde_json::Value;
use serde_json::json;
@@ -198,6 +200,134 @@ fn non_interactive_dumb_terminal_preserves_other_doctor_failures() -> Result<()>
Ok(())
}
#[test]
fn doctor_reports_failed_database_paths() -> Result<()> {
let fixture = Fixture::new()?;
let sqlite_home = fixture.root.path().join("sqlite");
std::fs::create_dir(&sqlite_home)?;
let sqlite = SqliteConfig::from_sqlite_home(AbsolutePathBuf::try_from(sqlite_home.clone())?);
let config_file = fixture.home.join("config.toml");
let config = std::fs::read_to_string(&config_file)?;
let sqlite_home = toml::Value::String(sqlite_home.display().to_string());
std::fs::write(
&config_file,
format!("sqlite_home = {sqlite_home}\n{config}"),
)?;
for (snapshot_name, failed_paths) in [
("doctor_failed_log_database", vec![sqlite.logs_db_path()]),
(
"doctor_failed_multiple_databases",
vec![sqlite.logs_db_path(), sqlite.goals_db_path()],
),
] {
for path in &failed_paths {
std::fs::write(path, "not a SQLite database")?;
}
let paths = failed_paths
.iter()
.map(|path| path.display().to_string())
.collect::<Vec<_>>()
.join(", ");
let noun = if failed_paths.len() == 1 {
"database"
} else {
"databases"
};
let expected_cause = format!("{paths} {noun} failed integrity check");
for args in [
vec!["doctor", "--json"],
vec!["doctor", "--json", "--feedback"],
] {
let output = fixture.command()?.args(args).output()?;
assert_eq!(output.status.code(), Some(1));
let report: Value = serde_json::from_slice(&output.stdout)?;
assert_eq!(report["checks"]["state.paths"]["status"], "fail");
assert_eq!(
report["checks"]["state.paths"]["summary"],
"state database integrity check failed"
);
assert_eq!(
report["checks"]["state.paths"]["issues"][0]["cause"],
expected_cause
);
}
for summary_only in [false, true] {
let mut command = fixture.command()?;
command.args(["doctor", "--ascii", "--no-color"]);
if summary_only {
command.arg("--summary");
}
let output = command.output()?;
assert_eq!(output.status.code(), Some(1));
let stdout = String::from_utf8(output.stdout)?;
let state_row = stdout
.lines()
.filter(|line| {
line.starts_with(" [XX] state ")
|| (!summary_only
&& line.starts_with(" -> Move the damaged SQLite database aside"))
})
.collect::<Vec<_>>()
.join("\n")
.replace(&fixture.root.path().display().to_string(), "FIXTURE")
.replace('\\', "/");
if summary_only {
assert_eq!(
state_row,
format!(
" [XX] state {}",
expected_cause
.replace(&fixture.root.path().display().to_string(), "FIXTURE")
)
.replace('\\', "/")
);
} else {
insta::assert_snapshot!(snapshot_name, state_row);
}
}
for path in &failed_paths {
assert_eq!(std::fs::read_to_string(path)?, "not a SQLite database");
}
}
Ok(())
}
#[test]
fn doctor_redacts_failed_database_paths_in_json() -> Result<()> {
let fixture = Fixture::new()?;
let sqlite_home = fixture.root.path().join("doctor-secret-sqlite");
std::fs::create_dir(&sqlite_home)?;
let sqlite = SqliteConfig::from_sqlite_home(AbsolutePathBuf::try_from(sqlite_home.clone())?);
let config_file = fixture.home.join("config.toml");
let config = std::fs::read_to_string(&config_file)?;
let sqlite_home = toml::Value::String(sqlite_home.display().to_string());
std::fs::write(
&config_file,
format!("sqlite_home = {sqlite_home}\n{config}"),
)?;
std::fs::write(sqlite.logs_db_path(), "not a SQLite database")?;
let output = fixture
.command()?
.args(["doctor", "--json", "--feedback"])
.output()?;
assert_eq!(output.status.code(), Some(1));
let stdout = String::from_utf8(output.stdout)?;
assert!(!stdout.contains("doctor-secret-sqlite"));
let report: Value = serde_json::from_str(&stdout)?;
assert_eq!(
report["checks"]["state.paths"]["summary"],
"state database integrity check failed"
);
assert_eq!(
report["checks"]["state.paths"]["issues"][0]["cause"],
"<redacted>"
);
Ok(())
}
#[test]
fn doctor_reports_only_safe_config_error_metadata() -> Result<()> {
let fixture = Fixture::new()?;
@@ -0,0 +1,6 @@
---
source: cli/tests/doctor_path_safety.rs
expression: state_row
---
[XX] state FIXTURE/sqlite/logs_2.sqlite database failed integrity check
-> Move the damaged SQLite database aside, then restart the interactive CLI or app server so it can rebuild that runtime database from saved data. Other entry points may not rebuild automatically.
@@ -0,0 +1,6 @@
---
source: cli/tests/doctor_path_safety.rs
expression: state_row
---
[XX] state FIXTURE/sqlite/logs_2.sqlite, FIXTURE/sqlite/goals_1.sqlite databases failed integrity check
-> Move the damaged SQLite database aside, then restart the interactive CLI or app server so it can rebuild that runtime database from saved data. Other entry points may not rebuild automatically.