Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,13 @@ cargo test-all
- When orchestrating parallel agents, the lead dictates exact scoped edits, subagents execute, and the lead reviews diffs before any push.
- Subagents should not invent scope beyond what the lead dictated.

## Git

- Every non-merge commit subject must pass `scripts/check-conventional-commits.sh` before push.
- Use `<type>: <subject>` or `<type>(<scope>): <subject>` with one of:
`build`, `chore`, `ci`, `docs`, `feat`, `fix`, `perf`, `refactor`, `revert`, `style`, `test`.
- Keep the subject at 72 characters or fewer. Example: `fix(doctor): avoid false orphan warnings`.

## Learned Workspace Facts

- Parallel branch work uses git worktrees under `.worktrees/` in the repo root (for example `.worktrees/codex-cli-args-stdin`).
Expand Down
37 changes: 16 additions & 21 deletions src/doctor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -408,7 +408,7 @@ async fn check_stale_stores(dc: &mut DoctorCounters) {
}
}

check_orphan_store_manifests(dc, &project_paths);
check_orphan_store_manifests(dc, &gdb).await;
check_stale_code_projects(dc, &gdb).await;
if let Some(profile_root) = profile_root.as_deref() {
let drift = registry_drift::registry_drift_findings(&gdb, profile_root).await;
Expand Down Expand Up @@ -691,11 +691,14 @@ fn registry_profile_roots(profile_root: &Path) -> Vec<PathBuf> {
roots
}

fn check_orphan_store_manifests(dc: &mut DoctorCounters, project_paths: &[String]) {
async fn check_orphan_store_manifests(
dc: &mut DoctorCounters,
global_db: &crate::global_db::GlobalDb,
) {
let Some(profile_root) = crate::config::user_data_dir() else {
return;
};
let (orphan_count, issues) = orphan_store_manifest_report(&profile_root, project_paths);
let (orphan_count, issues) = orphan_store_manifest_report(global_db, &profile_root).await;
for issue in issues.iter().take(10) {
dc.warn(&format!("Store manifest issue: {issue}"));
}
Expand All @@ -710,41 +713,33 @@ fn check_orphan_store_manifests(dc: &mut DoctorCounters, project_paths: &[String
/// Counts profile store manifests with no matching registry row, plus any
/// manifest scan issues. Shared between `doctor` and the post-update health
/// pass.
pub(crate) fn orphan_store_manifest_report(
pub(crate) async fn orphan_store_manifest_report(
global_db: &crate::global_db::GlobalDb,
profile_root: &Path,
project_paths: &[String],
) -> (usize, Vec<String>) {
let registered: std::collections::HashSet<String> = project_paths
.iter()
.map(|path| crate::global_db::GlobalDb::canonical_project_key(std::path::Path::new(path)))
.collect();
let report = crate::migrate::registry::scan_profile_store_manifests(
profile_root,
crate::tracedecay::current_timestamp(),
);
let mut orphan_count = 0;
let mut warnings = report.issues;
for plan in report.plans {
let key = crate::global_db::GlobalDb::canonical_project_key(&plan.project.project_root);
if registered.contains(&key) {
continue;
}
let mut warnings = report.issues.clone();
for plan in &report.plans {
match plan.status {
crate::migrate::registry::RegistryReconstructionStatus::Eligible => {
orphan_count += 1;
}
crate::migrate::registry::RegistryReconstructionStatus::Blocked => {
warnings.push(format!(
"blocked store manifest '{}': {}",
plan.manifest_path.display(),
plan.status_reason.as_deref().unwrap_or("not eligible")
));
}
crate::migrate::registry::RegistryReconstructionStatus::Stale
crate::migrate::registry::RegistryReconstructionStatus::Eligible
| crate::migrate::registry::RegistryReconstructionStatus::Stale
| crate::migrate::registry::RegistryReconstructionStatus::Retired => {}
}
}
(orphan_count, warnings)
let diff =
crate::migrate::registry::diff_registry_reconstruction_report(global_db, &report).await;
warnings.extend(diff.issues);
(diff.missing_plans, warnings)
}

/// Reports git-metadata watcher health (design D3/D5).
Expand Down
3 changes: 1 addition & 2 deletions src/doctor/heal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -347,9 +347,8 @@ async fn collect_remaining_findings(
remaining_registry_drift_count: usize,
) -> (Vec<String>, Vec<String>) {
let mut findings = Vec::new();
let project_paths = global_db.list_project_paths().await;
let (orphan_count, warnings) =
super::orphan_store_manifest_report(profile_root, &project_paths);
super::orphan_store_manifest_report(global_db, profile_root).await;
if orphan_count > 0 {
findings.push(format!(
"{orphan_count} orphan profile store manifest(s) can reconstruct registry rows"
Expand Down
73 changes: 64 additions & 9 deletions src/doctor/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@ fn format_bytes_boundaries() {
assert_eq!(format_bytes(1024 * 1024 * 1024 * 2), "2.0 GB");
}

#[test]
fn orphan_reporting_counts_only_eligible_and_surfaces_blocked_reasons() {
#[tokio::test]
async fn orphan_reporting_uses_complete_registry_rows_not_token_accounting() {
let base = Path::new(env!("CARGO_MANIFEST_DIR")).parent().unwrap();
let dir = tempfile::Builder::new()
.prefix("doctor-orphans-")
Expand All @@ -44,6 +44,14 @@ fn orphan_reporting_counts_only_eligible_and_surfaces_blocked_reasons() {
let blocked_root = dir.path().join("blocked-repo");
std::fs::create_dir_all(&eligible_root).unwrap();
std::fs::create_dir_all(&blocked_root).unwrap();
for root in [&eligible_root, &blocked_root] {
let status = std::process::Command::new("git")
.args(["init", "--quiet"])
.current_dir(root)
.status()
.unwrap();
assert!(status.success());
}
write_enrollment_marker(
&eligible_root,
&EnrollmentMarker {
Expand All @@ -52,6 +60,7 @@ fn orphan_reporting_counts_only_eligible_and_surfaces_blocked_reasons() {
},
)
.unwrap();
write_repository_identity_marker(&eligible_root, "proj_eligible").unwrap();
for (project_id, project_root) in [
("proj_eligible", &eligible_root),
("proj_blocked", &blocked_root),
Expand All @@ -76,14 +85,60 @@ fn orphan_reporting_counts_only_eligible_and_surfaces_blocked_reasons() {
.unwrap();
}

let (count, warnings) = orphan_store_manifest_report(&profile_root, &[]);

assert_eq!(count, 1);
assert!(
warnings
.iter()
.any(|warning| warning.contains("no repository identity or enrollment marker"))
let db = crate::global_db::GlobalDb::open_at(&dir.path().join("global.db"))
.await
.unwrap();
let (count, warnings) = orphan_store_manifest_report(&db, &profile_root).await;

assert_eq!(count, 1, "{warnings:?}");

let scan = crate::migrate::registry::scan_profile_store_manifests(&profile_root, 1_800_000_000);
let eligible = crate::migrate::registry::RegistryReconstructionReport {
plans: scan
.plans
.into_iter()
.filter(|plan| {
plan.status == crate::migrate::registry::RegistryReconstructionStatus::Eligible
})
.collect(),
issues: Vec::new(),
};
let applied = crate::migrate::registry::apply_registry_reconstruction_report(&db, &eligible)
.await
.unwrap();
assert_eq!(applied.projects, 1);
assert_eq!(
orphan_store_manifest_report(&db, &profile_root).await.0,
0,
"a complete reconstruction registry is healthy without a legacy projects.path row"
);
assert_eq!(
crate::migrate::registry::apply_registry_reconstruction_report(&db, &eligible)
.await
.unwrap(),
crate::migrate::registry::RegistryReconstructionApplyReport::default()
);

db.conn()
.execute(
"DELETE FROM store_artifacts WHERE store_id=?1",
libsql::params![eligible.plans[0].store.store_id.as_str()],
)
.await
.unwrap();
assert_eq!(orphan_store_manifest_report(&db, &profile_root).await.0, 1);
crate::migrate::registry::apply_registry_reconstruction_report(&db, &eligible)
.await
.unwrap();

db.conn()
.execute(
"DELETE FROM store_instances WHERE store_id=?1",
libsql::params![eligible.plans[0].store.store_id.as_str()],
)
.await
.unwrap();
assert_eq!(orphan_store_manifest_report(&db, &profile_root).await.0, 1);
}

#[test]
Expand Down
133 changes: 133 additions & 0 deletions src/migrate/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,139 @@ pub struct RegistryReconstructionApplyReport {
pub artifacts: usize,
}

/// Read-only view of eligible reconstruction plans that would insert at least
/// one registry row. This uses the same conflict preflight as apply.
#[derive(Debug, Clone, Default, PartialEq, Eq)]
pub struct RegistryReconstructionDiffReport {
pub missing_plans: usize,
pub issues: Vec<String>,
}

pub async fn diff_registry_reconstruction_report(
db: &GlobalDb,
report: &RegistryReconstructionReport,
) -> RegistryReconstructionDiffReport {
let eligible = RegistryReconstructionReport {
plans: report
.plans
.iter()
.filter(|plan| plan.status == RegistryReconstructionStatus::Eligible)
.cloned()
.collect(),
issues: Vec::new(),
};
let mut diff = RegistryReconstructionDiffReport {
issues: preflight_registry_reconstruction(db.conn(), &eligible).await,
..RegistryReconstructionDiffReport::default()
};
if !diff.issues.is_empty() {
return diff;
Comment on lines +95 to +96

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Continue diffing after unrelated reconstruction conflicts

In profiles with more than one eligible store manifest, a single conflicting plan (for example an existing registry row for the same root owned by a different project) makes the preflight return an issue here and skips registry_plan_has_missing_rows for every other plan. doctor/health then report zero orphan reconstructable manifests even when other manifests simply have missing registry rows, so one bad manifest hides the actionable repairs this check is meant to surface. Keep the issue but still compute missing rows for non-conflicting plans, or preflight per plan.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Valid. Fixed with per-plan preflight plus a conflict+missing regression in #440.

}

for plan in &eligible.plans {
match registry_plan_has_missing_rows(db.conn(), plan).await {
Ok(true) => diff.missing_plans += 1,
Ok(false) => {}
Err(issue) => diff.issues.push(issue),
}
}
diff
}

async fn registry_plan_has_missing_rows(
conn: &Connection,
plan: &RegistryReconstructionPlan,
) -> std::result::Result<bool, String> {
let project = &plan.project;
let root = GlobalDb::canonical_project_key(&project.project_root);
if query_optional_text(
conn,
"SELECT canonical_root FROM code_projects WHERE project_id=?1",
params![project.project_id.as_str()],
)
.await?
.as_deref()
!= Some(root.as_str())
{
return Ok(true);
}
for alias in &project.aliases {
if query_optional_text(
conn,
"SELECT project_id FROM project_aliases WHERE alias_path=?1",
params![GlobalDb::canonical_project_key(alias)],
)
.await?
.as_deref()
!= Some(project.project_id.as_str())
{
return Ok(true);
}
}

let store_identity = serde_json::to_string(&(
&plan.store.project_id,
&plan.store.store_kind,
&plan.store.storage_mode,
&plan.store.store_relpath,
&plan.store.manifest_relpath,
))
.unwrap_or_default();
if query_optional_text(
conn,
"SELECT json_array(project_id, store_kind, storage_mode, store_relpath, manifest_relpath)
FROM store_instances WHERE store_id=?1",
params![plan.store.store_id.as_str()],
)
.await?
.as_deref()
!= Some(store_identity.as_str())
{
return Ok(true);
}

for scope in &plan.graph_scopes {
let scope_identity = serde_json::to_string(&(
&scope.project_id,
&scope.store_id,
&scope.branch_name,
&scope.db_relpath,
&scope.parent_scope_id,
))
.unwrap_or_default();
if query_optional_text(
conn,
"SELECT json_array(project_id, store_id, branch_name, db_relpath, parent_scope_id)
FROM graph_scopes WHERE graph_scope_id=?1",
params![scope.graph_scope_id.as_str()],
)
.await?
.as_deref()
!= Some(scope_identity.as_str())
{
return Ok(true);
}
}
for artifact in &plan.artifacts {
if query_optional_text(
conn,
"SELECT store_id FROM store_artifacts
WHERE store_id=?1 AND artifact_kind=?2 AND relpath=?3",
params![
artifact.store_id.as_str(),
artifact.artifact_kind.as_str(),
artifact.relpath.as_str(),
],
)
.await?
.is_none()
{
return Ok(true);
}
}
Ok(false)
}

pub async fn apply_registry_reconstruction_report(
db: &GlobalDb,
report: &RegistryReconstructionReport,
Expand Down
Loading