From 2da33b425a1d5b21926ac1b2bbba84d02500c42b Mon Sep 17 00:00:00 2001 From: ScriptedAlchemy Date: Sat, 11 Jul 2026 12:29:22 +0000 Subject: [PATCH] fix(agents): recover post-update integration state --- src/agent_cmd.rs | 87 ++++- src/automation/memory_digest.rs | 107 +++--- src/automation/skill_targets.rs | 140 +++++++- src/migrate/hermes.rs | 425 ++++++++++++++++++++---- tests/agent_suite/memory_digest_test.rs | 96 ++++++ tests/agent_suite/skill_targets_test.rs | 165 ++++++++- 6 files changed, 894 insertions(+), 126 deletions(-) diff --git a/src/agent_cmd.rs b/src/agent_cmd.rs index a360014eb..d4012a434 100644 --- a/src/agent_cmd.rs +++ b/src/agent_cmd.rs @@ -155,6 +155,49 @@ pub(crate) async fn migrate_legacy_hermes_data(home: &Path) -> tracedecay::error }) } +/// Automated upgrade reinstalls must not remain wedged on preserved legacy +/// stores whose project evidence no longer resolves. Genuine read, integrity, +/// or copy failures still block the reinstall; an explicit Hermes install +/// continues to use the strict cutover above. +async fn migrate_legacy_hermes_data_for_reinstall(home: &Path) -> tracedecay::errors::Result<()> { + let report = tracedecay::migrate::hermes::migrate_legacy_hermes_stores(home).await; + finish_legacy_hermes_reinstall_migration(report) +} + +fn finish_legacy_hermes_reinstall_migration( + report: tracedecay::migrate::hermes::LegacyHermesMigrationReport, +) -> tracedecay::errors::Result<()> { + for migration in report.migrated { + eprintln!( + " \x1b[32m✔\x1b[0m Migrated legacy Hermes session store {} -> {} ({} rows)", + migration.source_db.display(), + migration.target_project.display(), + migration.rows_copied + ); + } + for issue in report.unresolved { + eprintln!( + " \x1b[33mwarning:\x1b[0m preserving unresolved legacy Hermes session store {}: {}", + issue.source_db.display(), + issue.reason + ); + } + if report.failed.is_empty() { + return Ok(()); + } + let issues = report + .failed + .into_iter() + .map(|issue| format!("{}: {}", issue.source_db.display(), issue.reason)) + .collect::>() + .join("; "); + Err(tracedecay::errors::TraceDecayError::Config { + message: format!( + "legacy Hermes session data migration failed; source data and project pins were preserved: {issues}" + ), + }) +} + pub(crate) async fn handle_install_command( agent: Option, local: bool, @@ -384,7 +427,9 @@ pub(crate) async fn handle_reinstall_command() -> tracedecay::errors::Result<()> /// loop forever — `migrate_installed_agents` only ever adds ids, never prunes, /// so a stale id would never resolve and the markers would never advance. Only /// genuine `install()` failures are reported as `Err` so they still gate -/// markers. +/// markers. Unresolved legacy Hermes project evidence is preserved and warned +/// during automated reinstall; actual source-integrity or copy failures still +/// gate Hermes so corrupted or partially copied data cannot be hidden. pub(crate) async fn reinstall_agent_integrations( agent_ids: &[String], home: &Path, @@ -393,7 +438,7 @@ pub(crate) async fn reinstall_agent_integrations( let project_path = std::env::current_dir().ok(); let mut results = Vec::new(); let hermes_migration_error = if agent_ids.iter().any(|id| id == "hermes") { - migrate_legacy_hermes_data(home) + migrate_legacy_hermes_data_for_reinstall(home) .await .err() .map(|error| error.to_string()) @@ -497,3 +542,41 @@ pub(crate) async fn handle_uninstall_command( } Ok(()) } + +#[cfg(test)] +mod tests { + use std::path::PathBuf; + + use tracedecay::migrate::hermes::{LegacyHermesMigrationIssue, LegacyHermesMigrationReport}; + + use super::finish_legacy_hermes_reinstall_migration; + + #[test] + fn automated_reinstall_preserves_unresolved_legacy_store_without_gating() { + let report = LegacyHermesMigrationReport { + unresolved: vec![LegacyHermesMigrationIssue { + source_db: PathBuf::from("legacy-sessions.db"), + reason: "project evidence is unresolved".to_string(), + }], + ..LegacyHermesMigrationReport::default() + }; + + assert!(finish_legacy_hermes_reinstall_migration(report).is_ok()); + } + + #[test] + fn automated_reinstall_gates_legacy_store_failures() { + let report = LegacyHermesMigrationReport { + failed: vec![LegacyHermesMigrationIssue { + source_db: PathBuf::from("legacy-sessions.db"), + reason: "integrity check failed".to_string(), + }], + ..LegacyHermesMigrationReport::default() + }; + + let error = finish_legacy_hermes_reinstall_migration(report) + .unwrap_err() + .to_string(); + assert!(error.contains("integrity check failed")); + } +} diff --git a/src/automation/memory_digest.rs b/src/automation/memory_digest.rs index c1330abc9..c8eb72a05 100644 --- a/src/automation/memory_digest.rs +++ b/src/automation/memory_digest.rs @@ -47,6 +47,12 @@ use crate::user_config::UserConfig; pub const MEMORY_DIGEST_START: &str = ""; pub const MEMORY_DIGEST_END: &str = ""; +const MEMORY_DIGEST_HEADING: &str = "## TraceDecay memory digest"; +const MEMORY_DIGEST_BODY_PREAMBLE: &str = "Curated durable facts from TraceDecay project memory (trust-ranked, newest first). \ + For deeper recall call MCP tool `tracedecay_recall`.\n\ + Rate facts you rely on with `tracedecay_fact_feedback` (fact_id, helpful/unhelpful) \ + — flagging a wrong or misleading fact unhelpful matters as much as confirming a \ + helpful one; trust is earned only from this feedback.\n"; /// Minimum trust score for a fact to enter the digest. pub const DEFAULT_DIGEST_MIN_TRUST: f64 = 0.6; @@ -285,13 +291,7 @@ pub fn compose_digest_body(snapshot: &MemoryDigestSnapshot, char_budget: usize) .then(left.project_key.cmp(&right.project_key)) }); - let mut body = String::from( - "Curated durable facts from TraceDecay project memory (trust-ranked, newest first). \ - For deeper recall call MCP tool `tracedecay_recall`.\n\ - Rate facts you rely on with `tracedecay_fact_feedback` (fact_id, helpful/unhelpful) \ - — flagging a wrong or misleading fact unhelpful matters as much as confirming a \ - helpful one; trust is earned only from this feedback.\n", - ); + let mut body = String::from(MEMORY_DIGEST_BODY_PREAMBLE); let mut budget_hit = false; for section in sections { let header = format!("\n## {}\n\n", section.project_label); @@ -569,59 +569,86 @@ fn remove_native_digest(target: SkillInstallTarget, plugin_root: &Path) -> Resul } fn render_prompt_digest_block(body: &str) -> String { - format!("{MEMORY_DIGEST_START}\n## TraceDecay memory digest\n\n{body}{MEMORY_DIGEST_END}\n") + format!("{MEMORY_DIGEST_START}\n{MEMORY_DIGEST_HEADING}\n\n{body}{MEMORY_DIGEST_END}\n") } fn replace_or_append_digest_block(existing: &str, block: &str) -> Result { - match ( - existing.find(MEMORY_DIGEST_START), - existing.find(MEMORY_DIGEST_END), - ) { - (Some(start), Some(end)) if start <= end => { - let end = end + MEMORY_DIGEST_END.len(); + if let Some((start, end)) = digest_block_range(existing)? { + let mut updated = String::new(); + updated.push_str(existing[..start].trim_end()); + if !updated.is_empty() { + updated.push_str("\n\n"); + } + updated.push_str(block.trim_end()); + updated.push_str("\n\n"); + updated.push_str(existing[end..].trim_start()); + let trimmed = updated.trim_end().to_string(); + Ok(trimmed + "\n") + } else { + let mut updated = String::new(); + updated.push_str(existing.trim_end()); + if !updated.is_empty() { + updated.push_str("\n\n"); + } + updated.push_str(block); + Ok(updated) + } +} + +fn remove_digest_block(existing: &str) -> Result { + match digest_block_range(existing)? { + Some((start, end)) => { let mut updated = String::new(); updated.push_str(existing[..start].trim_end()); - if !updated.is_empty() { - updated.push_str("\n\n"); - } - updated.push_str(block.trim_end()); updated.push_str("\n\n"); updated.push_str(existing[end..].trim_start()); - let trimmed = updated.trim_end().to_string(); - Ok(trimmed + "\n") - } - (None, None) => { - let mut updated = String::new(); - updated.push_str(existing.trim_end()); - if !updated.is_empty() { - updated.push_str("\n\n"); + if !updated.trim().is_empty() && !updated.ends_with('\n') { + updated.push('\n'); } - updated.push_str(block); Ok(updated) } - _ => Err(config_error( - "memory digest prompt markers are unbalanced".to_string(), - )), + None => Ok(existing.to_string()), } } -fn remove_digest_block(existing: &str) -> Result { +fn digest_block_range(existing: &str) -> Result> { match ( existing.find(MEMORY_DIGEST_START), existing.find(MEMORY_DIGEST_END), ) { (Some(start), Some(end)) if start <= end => { - let end = end + MEMORY_DIGEST_END.len(); - let mut updated = String::new(); - updated.push_str(existing[..start].trim_end()); - updated.push_str("\n\n"); - updated.push_str(existing[end..].trim_start()); - if !updated.trim().is_empty() && !updated.ends_with('\n') { - updated.push('\n'); + if existing.match_indices(MEMORY_DIGEST_START).count() != 1 + || existing.match_indices(MEMORY_DIGEST_END).count() != 1 + { + return Err(config_error( + "memory digest prompt markers are ambiguous".to_string(), + )); } - Ok(updated) + Ok(Some((start, end + MEMORY_DIGEST_END.len()))) + } + (None, Some(end)) => { + if existing.match_indices(MEMORY_DIGEST_END).count() != 1 { + return Err(config_error( + "memory digest prompt markers are unbalanced".to_string(), + )); + } + let preamble = format!("{MEMORY_DIGEST_HEADING}\n\n{MEMORY_DIGEST_BODY_PREAMBLE}"); + let mut matches = existing[..end].match_indices(&preamble); + let Some((start, _)) = matches.next() else { + return Err(config_error( + "memory digest prompt markers are unbalanced".to_string(), + )); + }; + if matches.next().is_some() + || existing[end + MEMORY_DIGEST_END.len()..].contains(&preamble) + { + return Err(config_error( + "memory digest prompt markers are unbalanced".to_string(), + )); + } + Ok(Some((start, end + MEMORY_DIGEST_END.len()))) } - (None, None) => Ok(existing.to_string()), + (None, None) => Ok(None), _ => Err(config_error( "memory digest prompt markers are unbalanced".to_string(), )), diff --git a/src/automation/skill_targets.rs b/src/automation/skill_targets.rs index d4956695c..0f08c76cd 100644 --- a/src/automation/skill_targets.rs +++ b/src/automation/skill_targets.rs @@ -274,12 +274,7 @@ fn render_prompt_index_block(target: SkillInstallTarget, skills: &[ManagedSkill] let (start, end) = prompt_index_markers(target); block.push_str(&start); block.push('\n'); - block.push_str("## TraceDecay managed skills\n\n"); - let _ = write!( - block, - "This {} index lists approved profile-managed skills. For full instructions, call MCP tool `tracedecay_skill_view` with the listed `id`.\n\n", - target.prompt_label() - ); + block.push_str(&prompt_index_preamble(target)); if skills.is_empty() { block.push_str("- No approved managed skills are currently exported.\n"); @@ -305,9 +300,9 @@ fn replace_or_append_marked_block( ) -> Result { let (start_marker, end_marker) = prompt_index_markers(target); // Prefer this target's slugged block; fall back to the legacy unslugged one. - let existing_range = match marked_block_range(existing, &start_marker, &end_marker)? { + let existing_range = match managed_block_range(existing, target, &start_marker, &end_marker)? { Some(range) => Some(range), - None => marked_block_range(existing, PROMPT_INDEX_START, PROMPT_INDEX_END)?, + None => managed_block_range(existing, target, PROMPT_INDEX_START, PROMPT_INDEX_END)?, }; if let Some((start, end)) = existing_range { Ok(splice_range(existing, start, end, block)) @@ -339,7 +334,7 @@ fn splice_range(existing: &str, start: usize, end: usize, block: &str) -> String fn remove_marked_block_for_target(existing: &str, target: SkillInstallTarget) -> Result { let (start_marker, end_marker) = prompt_index_markers(target); - if let Some((start, end)) = marked_block_range(existing, &start_marker, &end_marker)? { + if let Some((start, end)) = managed_block_range(existing, target, &start_marker, &end_marker)? { return Ok(remove_range(existing, start, end)); } // Legacy fallback: older installs wrote an unslugged block. Only claim it as @@ -349,7 +344,7 @@ fn remove_marked_block_for_target(existing: &str, target: SkillInstallTarget) -> // leave it untouched and let the remove-all path handle it instead. if !has_other_slugged_block(existing, target) { if let Some((start, end)) = - marked_block_range(existing, PROMPT_INDEX_START, PROMPT_INDEX_END)? + managed_block_range(existing, target, PROMPT_INDEX_START, PROMPT_INDEX_END)? { return Ok(remove_range(existing, start, end)); } @@ -373,10 +368,14 @@ fn remove_all_marked_blocks(existing: &str) -> Result { .into_iter() .filter(|target| target.writes_prompt_index()) { - updated = remove_marked_block_for_target(&updated, target)?; + let (start_marker, end_marker) = prompt_index_markers(target); + if let Some((start, end)) = + managed_block_range(&updated, target, &start_marker, &end_marker)? + { + updated = remove_range(&updated, start, end); + } } - if let Some((start, end)) = marked_block_range(&updated, PROMPT_INDEX_START, PROMPT_INDEX_END)? - { + if let Some((start, end)) = legacy_managed_block_range(&updated)? { updated = remove_range(&updated, start, end); } Ok(updated) @@ -388,14 +387,127 @@ fn marked_block_range( end_marker: &str, ) -> Result> { match (existing.find(start_marker), existing.find(end_marker)) { - (Some(start), Some(end)) if start <= end => Ok(Some((start, end + end_marker.len()))), + (Some(start), Some(end)) if start <= end => { + if existing.match_indices(start_marker).count() != 1 + || existing.match_indices(end_marker).count() != 1 + { + return Err(config_error( + "managed skill prompt index markers are ambiguous".to_string(), + )); + } + Ok(Some((start, end + end_marker.len()))) + } + (None, None) => Ok(None), + _ => Err(config_error( + "managed skill prompt index markers are unbalanced".to_string(), + )), + } +} + +/// Finds a normal marker-delimited block, or a generated block whose start +/// marker was lost while its exact preamble and end marker remain. Recovery +/// begins at the preamble, so preceding user-authored text is never claimed. +fn managed_block_range( + existing: &str, + target: SkillInstallTarget, + start_marker: &str, + end_marker: &str, +) -> Result> { + match (existing.find(start_marker), existing.find(end_marker)) { + (Some(start), Some(end)) if start <= end => { + if existing.match_indices(start_marker).count() != 1 + || existing.match_indices(end_marker).count() != 1 + { + return Err(config_error( + "managed skill prompt index markers are ambiguous".to_string(), + )); + } + Ok(Some((start, end + end_marker.len()))) + } (None, None) => Ok(None), + (None, Some(end)) => orphaned_generated_block_range(existing, target, end, end_marker), _ => Err(config_error( "managed skill prompt index markers are unbalanced".to_string(), )), } } +fn legacy_managed_block_range(existing: &str) -> Result> { + match ( + existing.find(PROMPT_INDEX_START), + existing.find(PROMPT_INDEX_END), + ) { + (Some(_), Some(_)) => marked_block_range(existing, PROMPT_INDEX_START, PROMPT_INDEX_END), + (None, None) => Ok(None), + (None, Some(end)) => { + if existing.match_indices(PROMPT_INDEX_END).count() != 1 { + return Err(config_error( + "managed skill prompt index markers are ambiguous".to_string(), + )); + } + let mut starts = Vec::new(); + for target in ALL_SKILL_INSTALL_TARGETS + .into_iter() + .filter(|target| target.writes_prompt_index()) + { + let preamble = prompt_index_preamble(target); + starts.extend( + existing[..end] + .match_indices(&preamble) + .map(|(start, _)| start), + ); + } + let Some(start) = starts.first().copied() else { + return Err(config_error( + "managed skill prompt index markers are unbalanced".to_string(), + )); + }; + if starts.len() != 1 { + return Err(config_error( + "managed skill prompt index markers are ambiguous".to_string(), + )); + } + Ok(Some((start, end + PROMPT_INDEX_END.len()))) + } + _ => Err(config_error( + "managed skill prompt index markers are unbalanced".to_string(), + )), + } +} + +fn orphaned_generated_block_range( + existing: &str, + target: SkillInstallTarget, + end: usize, + end_marker: &str, +) -> Result> { + if existing.match_indices(end_marker).count() != 1 { + return Err(config_error( + "managed skill prompt index markers are unbalanced".to_string(), + )); + } + let preamble = prompt_index_preamble(target); + let mut matches = existing[..end].match_indices(&preamble); + let Some((start, _)) = matches.next() else { + return Err(config_error( + "managed skill prompt index markers are unbalanced".to_string(), + )); + }; + if matches.next().is_some() || existing[end + end_marker.len()..].contains(&preamble) { + return Err(config_error( + "managed skill prompt index markers are unbalanced".to_string(), + )); + } + Ok(Some((start, end + end_marker.len()))) +} + +fn prompt_index_preamble(target: SkillInstallTarget) -> String { + format!( + "## TraceDecay managed skills\n\nThis {} index lists approved profile-managed skills. For full instructions, call MCP tool `tracedecay_skill_view` with the listed `id`.\n\n", + target.prompt_label() + ) +} + fn remove_range(existing: &str, start: usize, end: usize) -> String { let mut updated = String::new(); updated.push_str(existing[..start].trim_end()); diff --git a/src/migrate/hermes.rs b/src/migrate/hermes.rs index bd0c58cb7..e0e4d1d1b 100644 --- a/src/migrate/hermes.rs +++ b/src/migrate/hermes.rs @@ -112,9 +112,13 @@ async fn migrate_legacy_hermes_stores_inner( ) .await { - Ok(CandidateOutcome::Migrated(migration)) => report.migrated.push(migration), - Ok(CandidateOutcome::AlreadyMigrated(migration)) => { + Ok(CandidateOutcome::Migrated(migration, preserved_memory)) => { + report.migrated.push(migration); + report.unresolved.extend(preserved_memory); + } + Ok(CandidateOutcome::AlreadyMigrated(migration, preserved_memory)) => { report.already_migrated.push(migration); + report.unresolved.extend(preserved_memory); } Err(CandidateError::Unresolved(reason)) => { report @@ -146,9 +150,13 @@ async fn migrate_legacy_hermes_stores_inner( ) .await { - Ok(CandidateOutcome::Migrated(migration)) => report.migrated.push(migration), - Ok(CandidateOutcome::AlreadyMigrated(migration)) => { + Ok(CandidateOutcome::Migrated(migration, preserved_memory)) => { + report.migrated.push(migration); + report.unresolved.extend(preserved_memory); + } + Ok(CandidateOutcome::AlreadyMigrated(migration, preserved_memory)) => { report.already_migrated.push(migration); + report.unresolved.extend(preserved_memory); } Err(CandidateError::Unresolved(reason)) => { report.unresolved.push(LegacyHermesMigrationIssue { @@ -289,8 +297,8 @@ fn legacy_profile_dirs_for_homes(hermes_homes: &[PathBuf]) -> Vec { } enum CandidateOutcome { - Migrated(LegacyHermesMigration), - AlreadyMigrated(LegacyHermesMigration), + Migrated(LegacyHermesMigration, Option), + AlreadyMigrated(LegacyHermesMigration, Option), } enum CandidateError { @@ -301,6 +309,13 @@ enum CandidateError { struct ResolvedTargetProject { root: PathBuf, registry_project_id: Option, + user_scope: bool, +} + +struct ResolvedTargetLayout { + sessions_db_path: PathBuf, + graph_db_path: Option, + project_id: String, } async fn migrate_candidate( @@ -384,9 +399,9 @@ async fn migrate_legacy_state_store( rows_copied, }; Ok(if rows_copied == 0 { - CandidateOutcome::AlreadyMigrated(migration) + CandidateOutcome::AlreadyMigrated(migration, None) } else { - CandidateOutcome::Migrated(migration) + CandidateOutcome::Migrated(migration, None) }) } @@ -425,6 +440,18 @@ async fn migrate_candidate_snapshot( ) .await .map_err(CandidateError::Unresolved)?; + let preserved_memory = if target_project.user_scope { + candidate + .source_memory_db + .as_ref() + .map(|source_db| LegacyHermesMigrationIssue { + source_db: source_db.clone(), + reason: "unscoped legacy memory was preserved because no durable project attribution exists" + .to_string(), + }) + } else { + None + }; let target_layout = resolve_target_layout(&target_project, tracedecay_profile_root) .await .map_err(|error| { @@ -442,13 +469,22 @@ async fn migrate_candidate_snapshot( if candidate .source_memory_db .as_deref() - .is_some_and(|source_path| same_path(source_path, &target_layout.graph_db_path)) + .zip(target_layout.graph_db_path.as_deref()) + .is_some_and(|(source_path, target_path)| same_path(source_path, target_path)) { return Err(CandidateError::Failed( "source and target memory databases resolve to the same path".to_string(), )); } - let source_memory = match candidate.source_memory_db.as_deref() { + // Projectless sessions are safe to retain in the profile user-session + // store. Legacy memory facts are not: without a project pin or durable + // session attribution their scope cannot be proven, so leave that source + // database untouched for a later explicit recovery. + let source_memory = match candidate + .source_memory_db + .as_deref() + .filter(|_| !target_project.user_scope) + { Some(path) => { let (db, _) = Database::open_read_only(path).await.map_err(|error| { CandidateError::Failed(format!( @@ -490,11 +526,14 @@ async fn migrate_candidate_snapshot( .map_err(CandidateError::Failed)?; } let memory_rows = match source_memory.as_ref() { - Some(source_memory) => { - merge_memory_snapshot(source_memory.conn(), &target_layout.graph_db_path) - .await - .map_err(CandidateError::Failed)? - } + Some(source_memory) => merge_memory_snapshot( + source_memory.conn(), + target_layout.graph_db_path.as_deref().ok_or_else(|| { + CandidateError::Failed("project memory target disappeared".to_string()) + })?, + ) + .await + .map_err(CandidateError::Failed)?, None => 0, }; @@ -504,15 +543,7 @@ async fn migrate_candidate_snapshot( target_db.conn(), &target_layout.sessions_db_path, &target_project.root, - target_layout - .identity - .project_id - .as_deref() - .ok_or_else(|| { - CandidateError::Failed( - "target user-profile shard has no durable project id".to_string(), - ) - })?, + &target_layout.project_id, &fingerprint, source_schema_version, memory_rows, @@ -535,16 +566,23 @@ async fn migrate_candidate_snapshot( rows_copied: result.rows_copied, }; Ok(if result.already_migrated { - CandidateOutcome::AlreadyMigrated(migration) + CandidateOutcome::AlreadyMigrated(migration, preserved_memory) } else { - CandidateOutcome::Migrated(migration) + CandidateOutcome::Migrated(migration, preserved_memory) }) } async fn resolve_target_layout( target_project: &ResolvedTargetProject, tracedecay_profile_root: &Path, -) -> crate::errors::Result { +) -> crate::errors::Result { + if target_project.user_scope { + return Ok(ResolvedTargetLayout { + sessions_db_path: crate::sessions::user_sessions_db_path(tracedecay_profile_root), + graph_db_path: None, + project_id: "user".to_string(), + }); + } if let Some(project_id) = target_project.registry_project_id.as_deref() { if let Some(layout) = crate::storage::resolve_persisted_layout(&target_project.root, tracedecay_profile_root)? @@ -558,25 +596,41 @@ async fn resolve_target_layout( ), }); } - return Ok(layout); + return project_layout(layout); } - return crate::storage::profile_sharded_layout( + return project_layout(crate::storage::profile_sharded_layout( &target_project.root, tracedecay_profile_root, &crate::storage::EnrollmentMarker { project_id: project_id.to_string(), storage_mode: crate::storage::StorageMode::ProfileSharded, }, - ); + )?); } let production_profile = crate::storage::default_profile_root() .is_ok_and(|default| same_path(&default, tracedecay_profile_root)); - if production_profile { + let layout = if production_profile { crate::tracedecay::TraceDecay::resolve_store_layout_for_identity(&target_project.root).await } else { crate::storage::resolve_layout(&target_project.root, tracedecay_profile_root) - } + }?; + project_layout(layout) +} + +fn project_layout( + layout: crate::storage::StoreLayout, +) -> crate::errors::Result { + let project_id = layout.identity.project_id.clone().ok_or_else(|| { + crate::errors::TraceDecayError::Config { + message: "target project shard has no durable project id".to_string(), + } + })?; + Ok(ResolvedTargetLayout { + sessions_db_path: layout.sessions_db_path, + graph_db_path: Some(layout.graph_db_path), + project_id, + }) } async fn verify_source(source: &Connection) -> Result<(), String> { @@ -749,12 +803,13 @@ async fn resolve_target_project( .query(&sql, ()) .await .map_err(|error| format!("could not read source project metadata: {error}"))?; - let mut candidates = BTreeSet::new(); + let mut candidate_rows = Vec::new(); while let Some(row) = rows .next() .await .map_err(|error| format!("could not read source project metadata row: {error}"))? { + let mut candidates = BTreeSet::new(); for candidate in [row.get::>(0), row.get::>(1)] .into_iter() .flatten() @@ -762,50 +817,117 @@ async fn resolve_target_project( { candidates.insert(PathBuf::from(candidate)); } - if let Ok(Some(metadata)) = row.get::>(2) { - collect_metadata_project_candidates(&metadata, &mut candidates); - } + let malformed_metadata = match row.get::>(2) { + Ok(Some(metadata)) => { + collect_metadata_project_candidates(&metadata, &mut candidates).is_err() + } + Ok(None) => false, + Err(_) => true, + }; + candidate_rows.push((candidates, malformed_metadata)); } let mut targets: BTreeMap = BTreeMap::new(); - for candidate in candidates { - let Some(target) = - resolve_project_candidate(&candidate, user_home, hermes_homes, registry.as_ref()) - .await? - else { - continue; - }; - let key = target - .registry_project_id - .clone() - .unwrap_or_else(|| format!("path:{}", GlobalDb::canonical_project_key(&target.root))); - if let Some(existing) = targets.get(&key) - && !same_path(&existing.root, &target.root) - { + let mut has_projectless_evidence = false; + let mut has_unresolved_project_evidence = false; + for (candidates, malformed_metadata) in candidate_rows { + let mut row_targets: BTreeMap = BTreeMap::new(); + let mut row_has_unresolved_project_evidence = malformed_metadata; + for candidate in candidates { + if is_projectless_candidate(&candidate, user_home, hermes_homes) { + continue; + } + let Some(target) = + resolve_project_candidate(&candidate, user_home, hermes_homes, registry.as_ref()) + .await? + else { + row_has_unresolved_project_evidence = true; + continue; + }; + let key = target_key(&target); + if let Some(existing) = row_targets.get(&key) + && !same_path(&existing.root, &target.root) + { + return Err(project_identity_collision(&key, existing, &target)); + } + row_targets.insert(key, target); + } + if row_targets.len() > 1 { return Err(format!( - "registered project identity '{key}' maps to both '{}' and '{}'; refusing a collision", - existing.root.display(), - target.root.display() + "one source session maps to {} projects; refusing an ambiguous migration", + row_targets.len() )); } - targets.insert(key, target); + if row_has_unresolved_project_evidence { + has_unresolved_project_evidence = true; + } + if let Some((key, target)) = row_targets.into_iter().next() { + if let Some(existing) = targets.get(&key) + && !same_path(&existing.root, &target.root) + { + return Err(project_identity_collision(&key, existing, &target)); + } + targets.insert(key, target); + } else if !row_has_unresolved_project_evidence { + has_projectless_evidence = true; + } } match targets.len() { - 1 => targets + 1 if !has_projectless_evidence && !has_unresolved_project_evidence => targets .into_values() .next() .ok_or_else(|| "resolved project target disappeared".to_string()), + 0 if !has_unresolved_project_evidence => Ok(ResolvedTargetProject { + root: PathBuf::from("user"), + registry_project_id: None, + user_scope: true, + }), 0 => Err("no durable real project path exists in source session metadata".to_string()), + 1 => Err( + "source session metadata mixes projectless or unresolved evidence with a project; refusing an ambiguous migration" + .to_string(), + ), count => Err(format!( "source session metadata maps to {count} projects; refusing an ambiguous migration" )), } } -fn collect_metadata_project_candidates(raw: &str, candidates: &mut BTreeSet) { - let Ok(metadata) = serde_json::from_str::(raw) else { - return; - }; +fn target_key(target: &ResolvedTargetProject) -> String { + target + .registry_project_id + .clone() + .unwrap_or_else(|| format!("path:{}", GlobalDb::canonical_project_key(&target.root))) +} + +fn project_identity_collision( + key: &str, + existing: &ResolvedTargetProject, + target: &ResolvedTargetProject, +) -> String { + format!( + "registered project identity '{key}' maps to both '{}' and '{}'; refusing a collision", + existing.root.display(), + target.root.display() + ) +} + +fn is_projectless_candidate(candidate: &Path, user_home: &Path, hermes_homes: &[PathBuf]) -> bool { + if candidate.as_os_str().is_empty() || candidate == Path::new("user") { + return true; + } + same_path(candidate, user_home) + || hermes_homes + .iter() + .any(|hermes_home| same_path(candidate, hermes_home)) +} + +fn collect_metadata_project_candidates( + raw: &str, + candidates: &mut BTreeSet, +) -> Result<(), ()> { + let metadata = serde_json::from_str::(raw).map_err(|_| ())?; + let metadata = metadata.as_object().ok_or(())?; for key in [ "hermes_session_cwd", "hermes_session_worktree", @@ -813,10 +935,12 @@ fn collect_metadata_project_candidates(raw: &str, candidates: &mut BTreeSet ManagedSkillDraft { @@ -445,6 +445,169 @@ async fn prompt_index_preserves_user_content_and_routes_full_body_through_mcp() assert!(second.contains("This Claude index lists")); } +#[tokio::test] +async fn prompt_index_repairs_slugged_orphan_end_without_claiming_user_text() { + let temp = tempfile::tempdir().unwrap(); + let profile_root = temp.path().join("profile"); + let prompt_path = temp.path().join("AGENTS.md"); + + create_managed_skill_draft( + &profile_root, + targeted_draft( + "repo-hygiene", + "Repository hygiene", + vec![SkillInstallTarget::Agents], + ), + ) + .await + .unwrap(); + approve_managed_skill(&profile_root, "repo-hygiene") + .await + .unwrap(); + std::fs::write(&prompt_path, "# User rules\n\nKeep before.\n").unwrap(); + export_prompt_skill_index(&profile_root, SkillInstallTarget::Agents, &prompt_path).unwrap(); + + let stale = std::fs::read_to_string(&prompt_path) + .unwrap() + .replace("\n", ""); + std::fs::write(&prompt_path, format!("{stale}\nKeep after.\n")).unwrap(); + + export_prompt_skill_index(&profile_root, SkillInstallTarget::Agents, &prompt_path).unwrap(); + let repaired = std::fs::read_to_string(&prompt_path).unwrap(); + assert!(repaired.contains("Keep before.")); + assert!(repaired.contains("Keep after.")); + assert_eq!( + repaired + .matches("") + .count(), + 1 + ); + + export_prompt_skill_index(&profile_root, SkillInstallTarget::Agents, &prompt_path).unwrap(); + assert_eq!(std::fs::read_to_string(&prompt_path).unwrap(), repaired); +} + +#[test] +fn uninstall_repairs_legacy_orphan_end_without_claiming_user_text() { + let temp = tempfile::tempdir().unwrap(); + let prompt_path = temp.path().join("AGENTS.md"); + let contents = concat!( + "# User rules\n\nKeep before.\n\n", + "## TraceDecay managed skills\n\n", + "This AGENTS.md index lists approved profile-managed skills. For full instructions, call MCP tool `tracedecay_skill_view` with the listed `id`.\n\n", + "- `generated`: Generated.\n", + "\n\n", + "Keep after.\n", + ); + std::fs::write(&prompt_path, contents).unwrap(); + + remove_prompt_skill_index_for_target(&prompt_path, SkillInstallTarget::Agents).unwrap(); + + let repaired = std::fs::read_to_string(&prompt_path).unwrap(); + assert!(repaired.contains("Keep before.")); + assert!(repaired.contains("Keep after.")); + assert!(!repaired.contains("TraceDecay managed skills")); + assert!(!repaired.contains("`generated`")); + + remove_prompt_skill_index_for_target(&prompt_path, SkillInstallTarget::Agents).unwrap(); + assert_eq!(std::fs::read_to_string(&prompt_path).unwrap(), repaired); +} + +#[test] +fn prompt_index_start_only_remains_ambiguous_and_fails_closed() { + let temp = tempfile::tempdir().unwrap(); + let prompt_path = temp.path().join("AGENTS.md"); + let contents = concat!( + "# User rules\n\n", + "\n", + "## TraceDecay managed skills\n\n", + "This AGENTS.md index lists approved profile-managed skills. For full instructions, call MCP tool `tracedecay_skill_view` with the listed `id`.\n", + ); + std::fs::write(&prompt_path, contents).unwrap(); + + let error = + remove_prompt_skill_index_for_target(&prompt_path, SkillInstallTarget::Agents).unwrap_err(); + assert!(error.to_string().contains("markers are unbalanced")); + assert_eq!(std::fs::read_to_string(&prompt_path).unwrap(), contents); +} + +#[test] +fn uninstall_all_removes_legacy_orphan_alongside_slugged_block() { + let temp = tempfile::tempdir().unwrap(); + let prompt_path = temp.path().join("AGENTS.md"); + let contents = concat!( + "# User rules\n\nKeep before.\n\n", + "## TraceDecay managed skills\n\n", + "This AGENTS.md index lists approved profile-managed skills. For full instructions, call MCP tool `tracedecay_skill_view` with the listed `id`.\n\n", + "- `legacy`: Legacy.\n", + "\n\n", + "\n", + "## TraceDecay managed skills\n\n", + "This Claude index lists approved profile-managed skills. For full instructions, call MCP tool `tracedecay_skill_view` with the listed `id`.\n\n", + "- `slugged`: Slugged.\n", + "\n\n", + "Keep after.\n", + ); + std::fs::write(&prompt_path, contents).unwrap(); + + remove_prompt_skill_index(&prompt_path).unwrap(); + + let repaired = std::fs::read_to_string(&prompt_path).unwrap(); + assert!(repaired.contains("Keep before.")); + assert!(repaired.contains("Keep after.")); + assert!(!repaired.contains("TraceDecay managed skills")); + assert!(!repaired.contains("`legacy`")); + assert!(!repaired.contains("`slugged`")); +} + +#[test] +fn uninstall_all_removes_inverse_order_legacy_orphan_and_slugged_block() { + let temp = tempfile::tempdir().unwrap(); + let prompt_path = temp.path().join("AGENTS.md"); + let contents = concat!( + "# User rules\n\nKeep before.\n\n", + "## TraceDecay managed skills\n\n", + "This Claude index lists approved profile-managed skills. For full instructions, call MCP tool `tracedecay_skill_view` with the listed `id`.\n\n", + "- `legacy`: Legacy.\n", + "\n\n", + "\n", + "## TraceDecay managed skills\n\n", + "This AGENTS.md index lists approved profile-managed skills. For full instructions, call MCP tool `tracedecay_skill_view` with the listed `id`.\n\n", + "- `slugged`: Slugged.\n", + "\n\n", + "Keep after.\n", + ); + std::fs::write(&prompt_path, contents).unwrap(); + + remove_prompt_skill_index(&prompt_path).unwrap(); + + let repaired = std::fs::read_to_string(&prompt_path).unwrap(); + assert!(repaired.contains("Keep before.")); + assert!(repaired.contains("Keep after.")); + assert!(!repaired.contains("TraceDecay managed skills")); +} + +#[test] +fn prompt_index_duplicate_balanced_blocks_fail_closed() { + let temp = tempfile::tempdir().unwrap(); + let prompt_path = temp.path().join("AGENTS.md"); + let block = concat!( + "\n", + "## TraceDecay managed skills\n\n", + "This AGENTS.md index lists approved profile-managed skills. For full instructions, call MCP tool `tracedecay_skill_view` with the listed `id`.\n\n", + "\n", + ); + let contents = format!("# User rules\n\n{block}\n{block}"); + std::fs::write(&prompt_path, &contents).unwrap(); + + let error = remove_prompt_skill_index_for_target(&prompt_path, SkillInstallTarget::Agents) + .unwrap_err() + .to_string(); + + assert!(error.contains("markers are ambiguous")); + assert_eq!(std::fs::read_to_string(&prompt_path).unwrap(), contents); +} + #[tokio::test] async fn prompt_index_keeps_separate_sections_for_shared_agents_md_hosts() { let temp = tempfile::tempdir().unwrap();