From 6b684c3b34f9301e24f759c3d9efa33466257da0 Mon Sep 17 00:00:00 2001 From: wsp Date: Tue, 8 Sep 2026 18:19:43 +0800 Subject: [PATCH] fix(migration): isolate invalid legacy records and recover valid data - Continue independent domains after scan or recoverable failures. - Skip malformed entries without discarding valid sibling records. - Recover readable Turns and rebuild derived workspace references. - Remove coordination business checks while preserving merge rules. - Omit legacy memory jobs and support nested memory notes. - Decouple Agent imports from optional Skills and MiniApps. - Report skipped items and partial recovery with localized messages. - Cover source preservation, target priority and journal recovery. --- src/apps/data-migrator/README.md | 27 + src/apps/data-migrator/README.zh-CN.md | 18 + src/apps/data-migrator/ui/app.js | 10 +- src/apps/data-migrator/ui/locales.json | 51 +- .../coordination/coordination_store.rs | 5 +- .../src/service/coordination_persistence.rs | 8 - .../core/src/service/workspace/persistence.rs | 4 - .../src/agent_coordination.rs | 590 ++++++------ .../src/extensions.rs | 512 +++++++---- .../legacy-migration-adapters/src/lib.rs | 16 +- .../legacy-migration-adapters/src/memory.rs | 312 +++++-- .../src/remote_connect.rs | 260 +++++- .../src/remote_ssh.rs | 153 +++- .../legacy-migration-adapters/src/settings.rs | 240 +++-- .../src/workspace_sessions.rs | 864 +++++++++++++----- .../services/legacy-migration/src/engine.rs | 213 ++++- .../tests/migration_engine_contracts.rs | 77 +- .../services-core/src/memory_store.rs | 16 +- 18 files changed, 2352 insertions(+), 1024 deletions(-) diff --git a/src/apps/data-migrator/README.md b/src/apps/data-migrator/README.md index b821573866..2e7a5b5cd7 100644 --- a/src/apps/data-migrator/README.md +++ b/src/apps/data-migrator/README.md @@ -46,6 +46,24 @@ fixture is **0.2.19**. This is format-based support, not a claim that every old release has been tested. The source must pass the probe and selected domain validators. Unsupported or corrupt data is kept and reported. +Scanning isolates invalid settings/model entries, workspace registrations, memory +rows/files, SSH profiles, Remote Connect files/Bots, Sessions, Skills, MiniApps, +and Agent definitions. Readable Turns within a damaged Session are recovered; +derived Turn counts and workspace reference lists are rebuilt. Identical Turn +copies are deduplicated; conflicting Turn identities are omitted with warnings. +Nested user memory notes are supported. Legacy memory jobs are not read or +imported: the runtime creates jobs on demand, and destination jobs stay intact. +Optional Skills and MiniApps do not block importing Agent definitions. + +Item failures during staging are omitted from the committed manifest where the +item has an independent storage boundary. Source data remains read-only and the +report shows omissions and partial history recovery. Unreadable destination +stores, unsafe paths, unsupported schemas, changed inputs, and transaction/write +failures still protect the affected domain; independent domains can continue +after successful rollback. A failed rollback stops execution. Review warnings +and the report before retrying. + + The destination is OpenBitFun: configuration schema **1**, workspace registry format **1**, coordination database schema **2**, and the session, memory, extension, and connection formats accepted by the shared storage owners in @@ -68,6 +86,15 @@ both applications closed until the run finishes. Cancellation and window close requests wait for an engine-declared safe boundary; already verified domains may remain imported. + +Agent coordination imports do not require historical Session/Turn references, +parent relationships, counters, timestamps, or Swarm lineage to remain valid. +Records that cannot be decoded or inserted under the target table constraints +are skipped individually. Tasks without a mapped Agent are skipped instead of +being attached to an unrelated target primary key. Existing target records still +win conflicts; imported Agent primary keys and task references are remapped. +Importing historical task state does not start or resume an Agent execution. + ## Resume and diagnose Plans, journals, reports, backups and staging live under: diff --git a/src/apps/data-migrator/README.zh-CN.md b/src/apps/data-migrator/README.zh-CN.md index 4ba91993b9..41a59f8540 100644 --- a/src/apps/data-migrator/README.zh-CN.md +++ b/src/apps/data-migrator/README.zh-CN.md @@ -37,6 +37,12 @@ Windows 需要 Microsoft Edge WebView2；macOS 使用系统 WebView，Linux 包 支持以实际数据格式和各领域校验为准，不代表已验证范围内的每个旧版本。 未知格式、损坏数据会明确报告并保留。 +扫描会按单个会话、运行事件日志、Skill、MiniApp 和 Agent 定义隔离异常；有效条目继续迁移， +跳过的条目保留在来源中并记录到报告。旧会话的 Turn 计数以实际文件为准，仅在迁移副本中重建。 +整个数据域不可用时，只跳过该域及依赖它的域。执行时某域失败会先回滚，再继续无依赖的数据域； +无法安全回滚时仍会停止。请在重试前查看警告与迁移报告。 + + 目标是 OpenBitFun：配置 schema **1**、工作区格式 **1**、任务协调数据库 schema **2**， 以及此源码版本共享存储模块支持的会话、记忆、扩展和连接格式。未知产品/配置格式以及 超出支持范围的数据库、会话版本会被拒绝。未来数据格式变更需要发布新的迁移器， @@ -49,6 +55,12 @@ Windows 需要 Microsoft Edge WebView2；macOS 使用系统 WebView，Linux 包 来源不会被自动删除。写入使用一致性快照、暂存、校验、备份、迁移锁和原子替换。 迁移期间请保持相关应用关闭；取消或关闭窗口会等到安全边界，已验证完成的领域可能已导入。 + +协调数据库不再要求历史 Session/Turn 引用、父子关系、计数、时间戳或 Swarm 层级完整有效。 +无法解析或不能写入目标表约束的记录会单独跳过；缺少 Agent 主键映射的任务也单独跳过， +不会误挂到目标库中同号的 Agent。合并仍保持目标已有记录优先，并映射新增 Agent 主键和任务引用。 +导入历史任务状态不会启动或恢复 Agent 执行。 + ## 中断恢复与诊断 任务保存在： @@ -98,3 +110,9 @@ cargo build -p openbitfun-data-migrator --bin openbitfun-data-migrator 独立文件签名不等于 Apple/Authenticode 系统代码签名；当前工作流尚未配置这些证书及 macOS 公证。 开发约束和针对性检查见 [AGENTS.md](AGENTS.md)。 + +### 损坏旧数据的处理粒度 + +设置和模型、工作区登记、记忆记录和文件、SSH 连接、Remote Connect 文件和 Bot、会话和扩展均按条目处理。损坏会话仍会恢复可读取的 Turn，重建 Turn 数量和工作区引用列表；相同 Turn 副本去重，身份冲突的 Turn 会跳过并报告。记忆笔记支持嵌套目录。旧记忆库的 jobs 不读取、不迁移，运行时按需创建任务，目标库已有 jobs 保留。Skill 或 MiniApp 不可用不会阻止 Agent 定义迁移。 + +有独立存储边界的条目在暂存失败时会从提交清单中排除，报告会显示遗漏和会话部分恢复提示。旧数据保持只读；无法读取的目标存储、路径越界、不支持的格式、输入变更和事务写入失败仍会保护相关域，成功回滚后其余独立域可继续。回滚失败时停止执行。 diff --git a/src/apps/data-migrator/ui/app.js b/src/apps/data-migrator/ui/app.js index ec21b9f52a..b6040034a6 100644 --- a/src/apps/data-migrator/ui/app.js +++ b/src/apps/data-migrator/ui/app.js @@ -171,7 +171,7 @@ function render(view) { const findings = document.getElementById('findings'); findings.replaceChildren(...view.findings.map((finding) => - row(finding.code, `${finding.entityCount} item(s), ${finding.logicalBytes} byte(s)`))); + row(finding.domain, `${text[finding.code] || finding.code} · ${finding.entityCount} item(s), ${finding.logicalBytes} byte(s)`))); show('scan-card', view.findings.length > 0 && !view.plan); const planSummary = document.getElementById('plan-summary'); @@ -179,6 +179,7 @@ function render(view) { planSummary.replaceChildren( row(text.steps.replace('{count}', view.plan.steps.length), view.plan.selection.groups.map(groupLabel).join(' · ')), row(text.conflicts.replace('{count}', view.plan.conflicts.length), text.confirmHelp), + ...view.plan.findings.filter((finding) => finding.severity !== 'info').map((finding) => row(finding.domain, text[finding.code] || finding.code)), ...view.plan.conflicts.map((conflict) => row(conflict.domain, conflict.code || conflict.resolution || '')), ); } @@ -201,8 +202,11 @@ function render(view) { const reportSummary = document.getElementById('report-summary'); if (view.report) { - reportSummary.replaceChildren(...view.report.domainResults.map((result) => - row(result.domain, `${result.imported} ${transferLabel(result)}, ${result.skipped} ${text.skipped}, ${result.warnings.filter((item) => item.severity !== 'info').length} ${text.warnings}`))); + reportSummary.replaceChildren(...view.report.domainResults.flatMap((result) => [ + row(result.domain, `${result.imported} ${transferLabel(result)}, ${result.skipped} ${text.skipped}, ${result.warnings.filter((item) => item.severity !== 'info').length} ${text.warnings}`), + ...[...new Set(result.warnings.filter((item) => item.severity !== 'info').map((item) => item.code))] + .map((code) => row(result.domain, text[code] || code)), + ])); } show('report-card', !view.running && (Boolean(view.report) || view.status === 'cancelled')); const canExportDiagnostics = ['failed_recoverable', 'failed_manual_action_required'].includes(view.status); diff --git a/src/apps/data-migrator/ui/locales.json b/src/apps/data-migrator/ui/locales.json index ac5217ef76..b2f507e133 100644 --- a/src/apps/data-migrator/ui/locales.json +++ b/src/apps/data-migrator/ui/locales.json @@ -58,7 +58,22 @@ "skillsRoot": "Skills", "sshRoot": "SSH data", "unreadableTask": "Unavailable — restore original locations or inspect the saved files", - "confirmHelp": "Review the destination, selected groups, and conflicts. Existing destination records take priority; the tool keeps backups for replaced files." + "confirmHelp": "Review the destination, selected groups, and conflicts. Existing destination records take priority; the tool keeps backups for replaced files.", + "domain_scan_skipped": "Could not read or validate this domain. It will be skipped; other domains can continue.", + "domain_dependency_skipped": "A required domain is unavailable. This domain will be skipped.", + "extension_items_skipped": "Incomplete or unsupported extensions will be skipped; valid extensions can continue.", + "session_items_skipped": "Some Session files will be skipped; valid Sessions can continue.", + "coordination_rows_skipped": "Some coordination records cannot be read or stored and will be skipped; other records can continue.", + "settings_items_skipped": "Invalid settings or model entries were skipped; valid settings remain available.", + "memory_items_skipped": "Unreadable memory entries were skipped; readable memories remain available.", + "memory_item_skipped": "An unreadable memory item was preserved in the source and omitted from this import.", + "remote_items_skipped": "Unreadable remote records were skipped; other connections remain available.", + "session_turns_recovered": "Session history was partially recovered or its Turn filenames repaired. Check the original for omitted content.", + "setting_field_skipped": "An invalid setting was skipped; the target or default value was retained.", + "workspace_item_skipped": "An unreadable workspace registration was skipped; other workspaces and sessions remain available.", + "session_entry_skipped": "Some Session files will be skipped; valid Sessions can continue.", + "session_item_skipped": "Some Session files will be skipped; valid Sessions can continue.", + "extension_item_skipped": "Incomplete or unsupported extensions will be skipped; valid extensions can continue." }, "zh-CN": { "eyebrow": "OpenBitFun 独立数据迁移器", @@ -119,7 +134,22 @@ "skillsRoot": "Skills", "sshRoot": "SSH 数据", "unreadableTask": "暂不可用，请恢复原目录或检查任务文件", - "confirmHelp": "请确认目标目录、迁移范围和冲突。已有目标记录优先；工具会为需要替换的文件保留备份。" + "confirmHelp": "请确认目标目录、迁移范围和冲突。已有目标记录优先；工具会为需要替换的文件保留备份。", + "domain_scan_skipped": "此数据域无法读取或校验，本次将跳过，其他数据域可继续迁移。", + "domain_dependency_skipped": "依赖的数据域不可用，本次将跳过此数据域。", + "extension_items_skipped": "将跳过不完整或不支持的扩展，其他有效扩展可继续迁移。", + "session_items_skipped": "部分会话文件将被跳过，其他有效会话可继续迁移。", + "coordination_rows_skipped": "部分协调记录无法读取或写入，将单独跳过，其他记录可继续迁移。", + "settings_items_skipped": "已跳过无效设置或模型条目，其余设置可继续迁移。", + "memory_items_skipped": "已跳过无法读取的记忆条目，其余记忆可继续迁移。", + "memory_item_skipped": "无法读取的记忆条目保留在旧数据中，本次不迁移。", + "remote_items_skipped": "已跳过无法读取的远程记录，其余连接可继续迁移。", + "session_turns_recovered": "会话历史已部分恢复，或已修正 Turn 文件名。遗漏内容请查阅原会话。", + "setting_field_skipped": "已跳过无效设置，保留目标值或默认值。", + "workspace_item_skipped": "已跳过无法读取的工作区登记，其余工作区和会话可继续迁移。", + "session_entry_skipped": "部分会话文件将被跳过，其他有效会话可继续迁移。", + "session_item_skipped": "部分会话文件将被跳过，其他有效会话可继续迁移。", + "extension_item_skipped": "将跳过不完整或不支持的扩展，其他有效扩展可继续迁移。" }, "zh-TW": { "eyebrow": "OpenBitFun 獨立資料遷移器", @@ -180,6 +210,21 @@ "skillsRoot": "Skills", "sshRoot": "SSH 資料", "unreadableTask": "暫不可用，請恢復原目錄或檢查工作檔案", - "confirmHelp": "請確認目標目錄、遷移範圍和衝突。已有目標記錄優先；工具會為需要取代的檔案保留備份。" + "confirmHelp": "請確認目標目錄、遷移範圍和衝突。已有目標記錄優先；工具會為需要取代的檔案保留備份。", + "domain_scan_skipped": "此資料域無法讀取或驗證，本次將略過，其他資料域可繼續遷移。", + "domain_dependency_skipped": "相依的資料域不可用，本次將略過此資料域。", + "extension_items_skipped": "將略過不完整或不支援的擴充功能，其他有效項目可繼續遷移。", + "session_items_skipped": "部分工作階段檔案將被略過，其他有效工作階段可繼續遷移。", + "coordination_rows_skipped": "部分協調記錄無法讀取或寫入，將單獨略過，其他記錄可繼續遷移。", + "settings_items_skipped": "已略過無效設定或模型項目，其餘設定可繼續遷移。", + "memory_items_skipped": "已略過無法讀取的記憶項目，其餘記憶可繼續遷移。", + "memory_item_skipped": "無法讀取的記憶項目保留在舊資料中，本次不遷移。", + "remote_items_skipped": "已略過無法讀取的遠端記錄，其餘連線可繼續遷移。", + "session_turns_recovered": "工作階段歷史已部分恢復，或已修正 Turn 檔名。遺漏內容請查閱原工作階段。", + "setting_field_skipped": "已略過無效設定，保留目標值或預設值。", + "workspace_item_skipped": "已略過無法讀取的工作區登記，其餘工作區和工作階段可繼續遷移。", + "session_entry_skipped": "部分工作階段檔案將被略過，其他有效工作階段可繼續遷移。", + "session_item_skipped": "部分工作階段檔案將被略過，其他有效工作階段可繼續遷移。", + "extension_item_skipped": "將略過不完整或不支援的擴充功能，其他有效項目可繼續遷移。" } } diff --git a/src/crates/assembly/core/src/agentic/coordination/coordination_store.rs b/src/crates/assembly/core/src/agentic/coordination/coordination_store.rs index 33e6283957..5a58a1697e 100644 --- a/src/crates/assembly/core/src/agentic/coordination/coordination_store.rs +++ b/src/crates/assembly/core/src/agentic/coordination/coordination_store.rs @@ -1142,9 +1142,8 @@ fn unix_time_ms() -> u64 { #[cfg(test)] mod tests { use super::*; - use crate::service::coordination_persistence::{ - coordination_table_has_column, initialize_coordination_schema, - }; + use crate::service::coordination_persistence::initialize_coordination_schema; + use openbitfun_services_core::coordination_persistence::coordination_table_has_column; fn test_tempdir() -> tempfile::TempDir { if let Some(root) = std::env::var_os("OPENBITFUN_TEST_TMPDIR") { diff --git a/src/crates/assembly/core/src/service/coordination_persistence.rs b/src/crates/assembly/core/src/service/coordination_persistence.rs index 32e547813b..c2563023f7 100644 --- a/src/crates/assembly/core/src/service/coordination_persistence.rs +++ b/src/crates/assembly/core/src/service/coordination_persistence.rs @@ -1,18 +1,10 @@ use crate::util::errors::OpenBitFunResult; use openbitfun_services_core::coordination_persistence as storage; -pub(crate) use openbitfun_services_core::coordination_persistence::COORDINATION_SCHEMA_VERSION; use rusqlite::Connection; pub(crate) fn initialize_coordination_schema(connection: &Connection) -> OpenBitFunResult<()> { storage::initialize_coordination_schema(connection).map_err(Into::into) } -pub(crate) fn coordination_table_has_column( - connection: &Connection, - table: &str, - column: &str, -) -> OpenBitFunResult { - storage::coordination_table_has_column(connection, table, column).map_err(Into::into) -} pub(crate) fn validate_coordination_agent_id(agent_id: &str) -> OpenBitFunResult<()> { storage::validate_coordination_agent_id(agent_id).map_err(Into::into) } diff --git a/src/crates/assembly/core/src/service/workspace/persistence.rs b/src/crates/assembly/core/src/service/workspace/persistence.rs index 0a545286a7..1549c49ec5 100644 --- a/src/crates/assembly/core/src/service/workspace/persistence.rs +++ b/src/crates/assembly/core/src/service/workspace/persistence.rs @@ -1,4 +1,3 @@ -use super::types::WorkspaceInfo; use crate::util::errors::{OpenBitFunError, OpenBitFunResult}; use openbitfun_services_core::workspace_persistence as storage; pub(crate) use openbitfun_services_core::workspace_persistence::{ @@ -6,9 +5,6 @@ pub(crate) use openbitfun_services_core::workspace_persistence::{ }; use std::path::Path; -pub(crate) fn current_workspace_storage_id(workspace: &WorkspaceInfo) -> OpenBitFunResult { - storage::current_workspace_storage_id(workspace).map_err(Into::into) -} pub(crate) fn validate_workspace_persistence_data( data: &WorkspacePersistenceData, miniapp_root: &Path, diff --git a/src/crates/services/legacy-migration-adapters/src/agent_coordination.rs b/src/crates/services/legacy-migration-adapters/src/agent_coordination.rs index dd3daf30d3..068ce8719b 100644 --- a/src/crates/services/legacy-migration-adapters/src/agent_coordination.rs +++ b/src/crates/services/legacy-migration-adapters/src/agent_coordination.rs @@ -6,19 +6,17 @@ use super::workspace_sessions::{ read_workspace_sessions_manifest, target_wins_session_ids, SessionImportAction, WorkspaceSessionsManifest, }; -use openbitfun_core_types::validate_session_id; use openbitfun_legacy_migration::{ atomic_write_bytes, atomic_write_json, snapshot_sqlite_read_only, validate_sqlite, DomainContext, DomainScan, LegacyDomainAdapter, LegacyMigrationError, LegacyMigrationResult, MigrationRoots, }; use openbitfun_product_domains::legacy_migration::{ - ConflictResolution, FindingSeverity, MigrationConflict, MigrationDomainId, + ConflictResolution, FindingSeverity, MigrationConflict, MigrationDiagnostic, MigrationDomainId, MigrationDomainResult, MigrationDomainState, ScanFinding, }; use openbitfun_services_core::coordination_persistence::{ - coordination_table_has_column, initialize_coordination_schema, validate_coordination_agent_id, - COORDINATION_SCHEMA_VERSION, + coordination_table_has_column, initialize_coordination_schema, COORDINATION_SCHEMA_VERSION, }; use rusqlite::{params, Connection, OpenFlags, OptionalExtension, Transaction}; use serde::{Deserialize, Serialize}; @@ -41,6 +39,8 @@ struct CoordinationManifest { imported: u64, skipped: u64, conflicts: u64, + #[serde(default)] + incompatible_rows: u64, } #[derive(Debug, Clone, Default, Serialize)] @@ -51,6 +51,8 @@ struct CoordinationData { tasks: Vec, swarm_trees: Vec, swarm_nodes: Vec, + #[serde(skip_serializing_if = "Vec::is_empty")] + unreadable_rows: Vec, } #[derive(Debug, Clone, PartialEq, Eq, Serialize)] @@ -117,6 +119,7 @@ struct MergeOutcome { imported: u64, duplicate: u64, target_wins: u64, + incompatible_rows: u64, conflicts: Vec, } @@ -166,8 +169,13 @@ impl LegacyDomainAdapter for AgentCoordinationAdapter { Ok(DomainScan { finding: ScanFinding { domain: self.domain(), - code: "legacy_agent_coordination_supported".to_string(), - severity: if conflicts.is_empty() { + code: if source.unreadable_rows.is_empty() { + "legacy_agent_coordination_supported" + } else { + "coordination_rows_skipped" + } + .to_string(), + severity: if conflicts.is_empty() && source.unreadable_rows.is_empty() { FindingSeverity::Info } else { FindingSeverity::Warning @@ -204,7 +212,6 @@ impl LegacyDomainAdapter for AgentCoordinationAdapter { initialize_snapshot(&staged_source)?; let source = load_coordination_data(&staged_source, DatabaseRole::StagedCurrent)?; let workspace_manifest = read_workspace_sessions_manifest(context)?; - validate_source_cross_references(&source, &workspace_manifest)?; let blocked_sessions = manifest_target_wins_session_ids(&workspace_manifest); let target_path = target_coordination_path(context.roots); @@ -237,8 +244,12 @@ impl LegacyDomainAdapter for AgentCoordinationAdapter { target_digest, merged_digest: coordination_digest(&merged)?, imported: outcome.imported, - skipped: outcome.duplicate.saturating_add(outcome.target_wins), + skipped: outcome + .duplicate + .saturating_add(outcome.target_wins) + .saturating_add(outcome.incompatible_rows), conflicts: outcome.target_wins, + incompatible_rows: outcome.incompatible_rows, }; atomic_write_json(&coordination_manifest_path(context), &manifest)?; @@ -248,6 +259,16 @@ impl LegacyDomainAdapter for AgentCoordinationAdapter { imported: manifest.imported, skipped: manifest.skipped, conflicts: manifest.conflicts, + warnings: if manifest.incompatible_rows > 0 { + vec![MigrationDiagnostic { + code: "coordination_rows_skipped".to_string(), severity: FindingSeverity::Warning, + domain: Some(self.domain()), + message: format!("{} coordination records could not be represented in the target store and were preserved in the source.", manifest.incompatible_rows), + ..Default::default() + }] + } else { + Vec::new() + }, ..MigrationDomainResult::default() }) } @@ -272,7 +293,7 @@ impl LegacyDomainAdapter for AgentCoordinationAdapter { "staged Agent coordination merge differs from its manifest".to_string(), )); } - validate_source_cross_references(&source, &read_workspace_sessions_manifest(context)?) + Ok(()) } fn commit(&self, context: &DomainContext<'_>) -> LegacyMigrationResult<()> { @@ -312,7 +333,7 @@ impl LegacyDomainAdapter for AgentCoordinationAdapter { } fn validate_commit(&self, context: &DomainContext<'_>) -> LegacyMigrationResult<()> { - validate_committed_coordination_cross_references(context) + validate_committed_coordination_storage(context) } fn rollback_unverified(&self, context: &DomainContext<'_>) -> LegacyMigrationResult<()> { @@ -338,7 +359,7 @@ impl LegacyDomainAdapter for AgentCoordinationAdapter { } } -pub(crate) fn validate_committed_coordination_cross_references( +pub(crate) fn validate_committed_coordination_storage( context: &DomainContext<'_>, ) -> LegacyMigrationResult<()> { let manifest = read_coordination_manifest(context)?; @@ -352,11 +373,7 @@ pub(crate) fn validate_committed_coordination_cross_references( .to_string(), )); } - let source = load_coordination_data( - &stage_domain_dir(context, "agent-coordination").join("source.sqlite"), - DatabaseRole::StagedCurrent, - )?; - validate_source_cross_references(&source, &read_workspace_sessions_manifest(context)?) + Ok(()) } fn source_coordination_path(roots: &MigrationRoots) -> PathBuf { @@ -431,8 +448,10 @@ fn load_coordination_data( "delivered_parent_dialog_turn_id", ) .map_err(|error| owner_error("inspect coordination delivery columns", error))?; + let mut unreadable_rows = Vec::new(); let sessions = query_rows( &connection, + &mut unreadable_rows, path, "SELECT parent_session_id, next_auto_agent_seq, updated_at_ms FROM coordination_sessions ORDER BY parent_session_id", |row| { @@ -445,6 +464,7 @@ fn load_coordination_data( )?; let agents = query_rows( &connection, + &mut unreadable_rows, path, "SELECT agent_pk, parent_session_id, agent_id, child_session_id, next_bg_seq, state, created_at_ms FROM agents ORDER BY agent_pk", |row| { @@ -464,7 +484,7 @@ fn load_coordination_data( } else { "SELECT task_pk, parent_session_id, agent_pk, bg_task_id, bg_ordinal, parent_dialog_turn_id, parent_tool_call_id, child_dialog_turn_id, status, error_code, error_message, execution_owner_token, created_at_ms, terminal_at_ms, NULL, NULL FROM background_tasks ORDER BY task_pk" }; - let tasks = query_rows(&connection, path, task_sql, |row| { + let tasks = query_rows(&connection, &mut unreadable_rows, path, task_sql, |row| { Ok(BackgroundTaskRow { task_pk: row.get(0)?, parent_session_id: row.get(1)?, @@ -488,6 +508,7 @@ fn load_coordination_data( ( query_rows( &connection, + &mut unreadable_rows, path, "SELECT root_session_id, created_at_ms FROM swarm_trees ORDER BY root_session_id", |row| { @@ -499,6 +520,7 @@ fn load_coordination_data( )?, query_rows( &connection, + &mut unreadable_rows, path, "SELECT session_id, root_session_id, parent_session_id, agent_type, depth, created_at_ms FROM swarm_nodes ORDER BY depth, session_id", |row| { @@ -522,16 +544,17 @@ fn load_coordination_data( tasks, swarm_trees, swarm_nodes, + unreadable_rows, }; - validate_coordination_data(&data)?; Ok(data) } fn query_rows( connection: &Connection, + unreadable_rows: &mut Vec, path: &Path, sql: &str, - mapper: F, + mut mapper: F, ) -> LegacyMigrationResult> where F: FnMut(&rusqlite::Row<'_>) -> rusqlite::Result, @@ -539,11 +562,24 @@ where let mut statement = connection .prepare(sql) .map_err(|error| db_error(path, error))?; - let rows = statement - .query_map([], mapper) - .map_err(|error| db_error(path, error))?; - rows.collect::>>() - .map_err(|error| db_error(path, error)) + let mut rows = statement.query([]).map_err(|error| db_error(path, error))?; + let mut parsed = Vec::new(); + while let Some(row) = rows.next().map_err(|error| db_error(path, error))? { + match mapper(row) { + Ok(value) => parsed.push(value), + Err(_) => { + let mut digest = Sha256::new(); + digest.update(sql.as_bytes()); + for column in 0..row.as_ref().column_count() { + let value = row.get_ref(column).map_err(|error| db_error(path, error))?; + digest.update(format!("{value:?}").as_bytes()); + digest.update([0]); + } + unreadable_rows.push(hex::encode(digest.finalize())); + } + } + } + Ok(parsed) } fn validate_required_tables( @@ -630,235 +666,6 @@ fn table_columns( .map_err(|error| db_error(path, error)) } -fn validate_coordination_data(data: &CoordinationData) -> LegacyMigrationResult<()> { - let mut session_keys = HashSet::new(); - for row in &data.sessions { - validate_session(&row.parent_session_id)?; - if row.next_auto_agent_seq < 1 || row.updated_at_ms < 0 { - return unsupported("coordination Session counters or timestamps are invalid"); - } - if !session_keys.insert(row.parent_session_id.as_str()) { - return unsupported("coordination Session identities are not unique"); - } - } - - let mut agent_pks = HashSet::new(); - let mut agent_ids = HashSet::new(); - let mut child_ids = HashSet::new(); - for row in &data.agents { - validate_session(&row.parent_session_id)?; - if let Some(child) = row.child_session_id.as_deref() { - validate_session(child)?; - if !child_ids.insert((row.parent_session_id.as_str(), child)) { - return unsupported("Agent child Session identities are not unique per parent"); - } - } - if row.agent_pk <= 0 - || row.next_bg_seq < 1 - || row.created_at_ms < 0 - || !matches!(row.state.as_str(), "active" | "historical") - { - return unsupported("Agent coordination row contains invalid owner fields"); - } - validate_coordination_agent_id(&row.agent_id) - .map_err(|error| owner_error("validate legacy agent id", error))?; - if !agent_pks.insert(row.agent_pk) - || !agent_ids.insert((row.parent_session_id.as_str(), row.agent_id.as_str())) - { - return unsupported("Agent coordination identities are not unique"); - } - } - - let agents = data - .agents - .iter() - .map(|row| (row.agent_pk, row)) - .collect::>(); - let mut task_pks = HashSet::new(); - let mut task_ids = HashSet::new(); - let mut task_ordinals = HashSet::new(); - for row in &data.tasks { - validate_session(&row.parent_session_id)?; - let Some(agent) = agents.get(&row.agent_pk) else { - return unsupported("background Task references a missing Agent"); - }; - if agent.parent_session_id != row.parent_session_id { - return unsupported("background Task parent Session differs from its Agent"); - } - if row.task_pk <= 0 - || row.bg_task_id.is_empty() - || row.bg_ordinal < 1 - || row.parent_dialog_turn_id.is_empty() - || row.parent_tool_call_id.is_empty() - || row.child_dialog_turn_id.is_empty() - || row.execution_owner_token.is_empty() - || row.created_at_ms < 0 - || row.terminal_at_ms.is_some_and(|value| value < 0) - || row.delivered_at_ms.is_some_and(|value| value < 0) - || !matches!( - row.status.as_str(), - "running" - | "completed" - | "partial_timeout" - | "failed" - | "cancelled" - | "interrupted" - ) - { - return unsupported("background Task row contains invalid owner fields"); - } - if !task_pks.insert(row.task_pk) - || !task_ids.insert((row.parent_session_id.as_str(), row.bg_task_id.as_str())) - || !task_ordinals.insert((row.agent_pk, row.bg_ordinal)) - { - return unsupported("background Task identities are not unique"); - } - } - - let tree_ids = data - .swarm_trees - .iter() - .map(|row| row.root_session_id.as_str()) - .collect::>(); - if tree_ids.len() != data.swarm_trees.len() { - return unsupported("Swarm tree identities are not unique"); - } - for row in &data.swarm_trees { - validate_session(&row.root_session_id)?; - if row.created_at_ms < 0 { - return unsupported("Swarm tree timestamp is invalid"); - } - } - let nodes = data - .swarm_nodes - .iter() - .map(|row| (row.session_id.as_str(), row)) - .collect::>(); - if nodes.len() != data.swarm_nodes.len() { - return unsupported("Swarm node identities are not unique"); - } - for row in &data.swarm_nodes { - validate_session(&row.session_id)?; - validate_session(&row.root_session_id)?; - if !tree_ids.contains(row.root_session_id.as_str()) - || row.agent_type.trim().is_empty() - || row.depth < 0 - || row.created_at_ms < 0 - { - return unsupported("Swarm node contains invalid owner fields"); - } - match row.parent_session_id.as_deref() { - None if row.session_id == row.root_session_id && row.depth == 0 => {} - Some(parent_id) => { - let Some(parent) = nodes.get(parent_id) else { - return unsupported("Swarm node references a missing parent node"); - }; - if parent.root_session_id != row.root_session_id - || parent.depth.saturating_add(1) != row.depth - { - return unsupported("Swarm node lineage is inconsistent"); - } - } - _ => return unsupported("Swarm root node shape is invalid"), - } - } - Ok(()) -} - -fn validate_source_cross_references( - data: &CoordinationData, - manifest: &WorkspaceSessionsManifest, -) -> LegacyMigrationResult<()> { - let sessions = manifest - .sessions - .iter() - .map(|entry| (entry.session_id.as_str(), entry)) - .collect::>(); - let mut turns = HashMap::<&str, BTreeSet<&str>>::new(); - for entry in &manifest.sessions { - turns - .entry(entry.session_id.as_str()) - .or_default() - .extend(entry.turn_ids.iter().map(String::as_str)); - } - for entry in &manifest.runtime_events { - turns - .entry(entry.session_id.as_str()) - .or_default() - .extend(entry.turn_ids.iter().map(String::as_str)); - } - let require_session = |session_id: &str| -> LegacyMigrationResult<()> { - if sessions.contains_key(session_id) { - Ok(()) - } else { - unsupported(format!( - "Agent coordination references missing Session {session_id}" - )) - } - }; - let require_turn = |session_id: &str, turn_id: &str| -> LegacyMigrationResult<()> { - if turns - .get(session_id) - .is_some_and(|values| values.contains(turn_id)) - { - Ok(()) - } else { - unsupported(format!( - "Agent coordination references missing Turn {turn_id} in Session {session_id}" - )) - } - }; - for row in &data.sessions { - require_session(&row.parent_session_id)?; - } - let agents = data - .agents - .iter() - .map(|row| (row.agent_pk, row)) - .collect::>(); - for row in &data.agents { - require_session(&row.parent_session_id)?; - if let Some(child_id) = row.child_session_id.as_deref() { - require_session(child_id)?; - let relationship = sessions[child_id].relationship.as_ref().ok_or_else(|| { - LegacyMigrationError::UnsupportedSource(format!( - "Agent child Session {child_id} has no parent relationship" - )) - })?; - if relationship.parent_session_id.as_deref() != Some(&row.parent_session_id) { - return unsupported(format!( - "Agent child Session {child_id} has a different persisted parent" - )); - } - } - } - for row in &data.tasks { - let agent = agents[&row.agent_pk]; - let child_session_id = agent.child_session_id.as_deref().ok_or_else(|| { - LegacyMigrationError::UnsupportedSource(format!( - "background Task {} references an Agent without a child Session", - row.bg_task_id - )) - })?; - require_turn(&row.parent_session_id, &row.parent_dialog_turn_id)?; - require_turn(child_session_id, &row.child_dialog_turn_id)?; - if let Some(delivered_turn) = row.delivered_parent_dialog_turn_id.as_deref() { - require_turn(&row.parent_session_id, delivered_turn)?; - } - } - for row in &data.swarm_trees { - require_session(&row.root_session_id)?; - } - for row in &data.swarm_nodes { - require_session(&row.session_id)?; - require_session(&row.root_session_id)?; - if let Some(parent) = row.parent_session_id.as_deref() { - require_session(parent)?; - } - } - Ok(()) -} - fn merge_coordination_database( source_path: &Path, target_path: &Path, @@ -875,7 +682,10 @@ fn merge_coordination_database( let transaction = connection .transaction() .map_err(|error| db_error(target_path, error))?; - let mut outcome = MergeOutcome::default(); + let mut outcome = MergeOutcome { + incompatible_rows: source.unreadable_rows.len() as u64, + ..Default::default() + }; merge_coordination_sessions( &transaction, &source.sessions, @@ -938,26 +748,26 @@ fn merge_coordination_sessions( .query_row( "SELECT parent_session_id, next_auto_agent_seq, updated_at_ms FROM coordination_sessions WHERE parent_session_id = ?1", params![row.parent_session_id], - |record| Ok(CoordinationSessionRow { + |record| Ok((|| -> rusqlite::Result { Ok(CoordinationSessionRow { parent_session_id: record.get(0)?, next_auto_agent_seq: record.get(1)?, updated_at_ms: record.get(2)?, - }), + }) })()), ) .optional() .map_err(|error| db_error(path, error))?; match existing { - Some(existing) if existing == *row => outcome.duplicate += 1, + Some(Ok(existing)) if existing == *row => outcome.duplicate += 1, Some(_) => record_target_win( outcome, "coordination_session_target_wins", &row.parent_session_id, ), None => { - transaction.execute( + if !insert_compatible_row(transaction.execute( "INSERT INTO coordination_sessions (parent_session_id, next_auto_agent_seq, updated_at_ms) VALUES (?1, ?2, ?3)", params![row.parent_session_id, row.next_auto_agent_seq, row.updated_at_ms], - ).map_err(|error| db_error(path, error))?; + ), outcome, path)? { continue; } outcome.imported += 1; } } @@ -1010,10 +820,10 @@ fn merge_agents( } } if candidates.is_empty() { - transaction.execute( + if !insert_compatible_row(transaction.execute( "INSERT INTO agents (parent_session_id, agent_id, child_session_id, next_bg_seq, state, created_at_ms) VALUES (?1, ?2, ?3, ?4, ?5, ?6)", params![row.parent_session_id, row.agent_id, row.child_session_id, row.next_bg_seq, row.state, row.created_at_ms], - ).map_err(|error| db_error(path, error))?; + ), outcome, path)? { continue; } let pk = transaction.last_insert_rowid(); mapping.insert(row.agent_pk, AgentMergeTarget::Imported(pk)); outcome.imported += 1; @@ -1021,8 +831,9 @@ fn merge_agents( } if candidates.len() == 1 { let pk = *candidates.iter().next().expect("one candidate exists"); - let existing = load_agent_by_pk(transaction, path, pk)?; - if agent_logically_equal(&existing, row) { + if load_agent_by_pk(transaction, path, pk) + .is_ok_and(|existing| agent_logically_equal(&existing, row)) + { mapping.insert(row.agent_pk, AgentMergeTarget::Existing(pk)); outcome.duplicate += 1; continue; @@ -1056,7 +867,8 @@ fn merge_background_tasks( continue; } let Some(agent_target) = agent_map.get(&row.agent_pk).copied() else { - return unsupported("background Task Agent mapping was not constructed"); + outcome.incompatible_rows += 1; + continue; }; let Some(agent_pk) = agent_target.agent_pk() else { record_target_win( @@ -1084,18 +896,18 @@ fn merge_background_tasks( .flatten() .collect::>(); if candidates.is_empty() { - transaction.execute( + if !insert_compatible_row(transaction.execute( "INSERT INTO background_tasks (parent_session_id, agent_pk, bg_task_id, bg_ordinal, parent_dialog_turn_id, parent_tool_call_id, child_dialog_turn_id, status, error_code, error_message, execution_owner_token, created_at_ms, terminal_at_ms, delivered_at_ms, delivered_parent_dialog_turn_id) VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9, ?10, ?11, ?12, ?13, ?14, ?15)", params![row.parent_session_id, agent_pk, row.bg_task_id, row.bg_ordinal, row.parent_dialog_turn_id, row.parent_tool_call_id, row.child_dialog_turn_id, row.status, row.error_code, row.error_message, row.execution_owner_token, row.created_at_ms, row.terminal_at_ms, row.delivered_at_ms, row.delivered_parent_dialog_turn_id], - ).map_err(|error| db_error(path, error))?; + ), outcome, path)? { continue; } outcome.imported += 1; } else if candidates.len() == 1 { let existing = load_task_by_pk( transaction, path, *candidates.iter().next().expect("one candidate exists"), - )?; - if task_logically_equal(&existing, row, agent_pk) { + ); + if existing.is_ok_and(|existing| task_logically_equal(&existing, row, agent_pk)) { outcome.duplicate += 1; } else { record_target_win(outcome, "coordination_task_target_wins", &row.bg_task_id); @@ -1128,28 +940,34 @@ fn merge_swarm_trees( "SELECT root_session_id, created_at_ms FROM swarm_trees WHERE root_session_id = ?1", params![row.root_session_id], |record| { - Ok(SwarmTreeRow { - root_session_id: record.get(0)?, - created_at_ms: record.get(1)?, - }) + Ok((|| -> rusqlite::Result { + Ok(SwarmTreeRow { + root_session_id: record.get(0)?, + created_at_ms: record.get(1)?, + }) + })()) }, ) .optional() .map_err(|error| db_error(path, error))?; match existing { - Some(existing) if existing == *row => outcome.duplicate += 1, + Some(Ok(existing)) if existing == *row => outcome.duplicate += 1, Some(_) => record_target_win( outcome, "coordination_swarm_tree_target_wins", &row.root_session_id, ), None => { - transaction - .execute( + if !insert_compatible_row( + transaction.execute( "INSERT INTO swarm_trees (root_session_id, created_at_ms) VALUES (?1, ?2)", params![row.root_session_id, row.created_at_ms], - ) - .map_err(|error| db_error(path, error))?; + ), + outcome, + path, + )? { + continue; + } outcome.imported += 1; } } @@ -1182,20 +1000,20 @@ fn merge_swarm_nodes( let existing = transaction.query_row( "SELECT session_id, root_session_id, parent_session_id, agent_type, depth, created_at_ms FROM swarm_nodes WHERE session_id = ?1", params![row.session_id], - |record| Ok(SwarmNodeRow { session_id: record.get(0)?, root_session_id: record.get(1)?, parent_session_id: record.get(2)?, agent_type: record.get(3)?, depth: record.get(4)?, created_at_ms: record.get(5)? }), + |record| Ok((|| -> rusqlite::Result { Ok(SwarmNodeRow { session_id: record.get(0)?, root_session_id: record.get(1)?, parent_session_id: record.get(2)?, agent_type: record.get(3)?, depth: record.get(4)?, created_at_ms: record.get(5)? }) })()), ).optional().map_err(|error| db_error(path, error))?; match existing { - Some(existing) if existing == *row => outcome.duplicate += 1, + Some(Ok(existing)) if existing == *row => outcome.duplicate += 1, Some(_) => record_target_win( outcome, "coordination_swarm_node_target_wins", &row.session_id, ), None => { - transaction.execute( + if !insert_compatible_row(transaction.execute( "INSERT INTO swarm_nodes (session_id, root_session_id, parent_session_id, agent_type, depth, created_at_ms) VALUES (?1, ?2, ?3, ?4, ?5, ?6)", params![row.session_id, row.root_session_id, row.parent_session_id, row.agent_type, row.depth, row.created_at_ms], - ).map_err(|error| db_error(path, error))?; + ), outcome, path)? { continue; } outcome.imported += 1; } } @@ -1203,6 +1021,23 @@ fn merge_swarm_nodes( Ok(()) } +fn insert_compatible_row( + result: rusqlite::Result, + outcome: &mut MergeOutcome, + path: &Path, +) -> LegacyMigrationResult { + match result { + Ok(_) => Ok(true), + Err(rusqlite::Error::SqliteFailure(error, _)) + if error.code == rusqlite::ErrorCode::ConstraintViolation => + { + outcome.incompatible_rows += 1; + Ok(false) + } + Err(error) => Err(db_error(path, error)), + } +} + fn load_agent_by_pk( transaction: &Transaction<'_>, path: &Path, @@ -1497,15 +1332,6 @@ fn validate_current_database(path: &Path) -> LegacyMigrationResult<()> { ))); } } - let foreign_key_error: Option = connection - .query_row("PRAGMA foreign_key_check", [], |row| row.get(0)) - .optional() - .map_err(|error| db_error(path, error))?; - if foreign_key_error.is_some() { - return Err(LegacyMigrationError::InvalidRequest( - "Agent coordination foreign-key validation failed".to_string(), - )); - } Ok(()) } @@ -1551,14 +1377,6 @@ fn manifest_target_wins_session_ids(manifest: &WorkspaceSessionsManifest) -> BTr .collect() } -fn validate_session(session_id: &str) -> LegacyMigrationResult<()> { - validate_session_id(session_id).map_err(|error| { - LegacyMigrationError::UnsupportedSource(format!( - "Agent coordination contains an invalid Session id: {error}" - )) - }) -} - fn reset_stage_file(path: &Path) -> LegacyMigrationResult<()> { remove_file_if_present(path)?; remove_sqlite_sidecars(path) @@ -1580,10 +1398,6 @@ fn remove_file_if_present(path: &Path) -> LegacyMigrationResult<()> { } } -fn unsupported(detail: impl Into) -> LegacyMigrationResult { - Err(LegacyMigrationError::UnsupportedSource(detail.into())) -} - fn owner_error(context: &str, error: impl std::fmt::Display) -> LegacyMigrationError { LegacyMigrationError::InvalidRequest(format!("{context}: {error}")) } @@ -1823,7 +1637,7 @@ mod tests { } #[test] - fn missing_or_unknown_coordination_schema_blocks_the_session_plan() { + fn missing_or_unknown_coordination_schema_skips_only_coordination_and_dependents() { let temp = test_tempdir("coordination-schema"); let roots = fixture_roots(temp.path()); copy_fixture(&roots); @@ -1834,9 +1648,13 @@ mod tests { .unwrap(); let missing = engine.plan(&source, selection.clone(), &CancellationToken::default()); assert!(missing - .unwrap_err() - .to_string() - .contains("requires a readable legacy coordination.sqlite")); + .unwrap() + .findings + .iter() + .any(|finding| !finding.migratable + && finding + .detail + .contains("requires a readable legacy coordination.sqlite"))); let connection = materialize_coordination(&roots, false); connection.pragma_update(None, "user_version", 99).unwrap(); @@ -1846,9 +1664,11 @@ mod tests { .unwrap(); let unknown = engine.plan(&source, selection, &CancellationToken::default()); assert!(unknown - .unwrap_err() - .to_string() - .contains("schema 99 is not supported")); + .unwrap() + .findings + .iter() + .any(|finding| !finding.migratable + && finding.detail.contains("schema 99 is not supported"))); } #[test] @@ -2026,7 +1846,7 @@ mod tests { } #[test] - fn failed_and_cancelled_runs_leave_the_legacy_source_bytes_unchanged() { + fn missing_turn_is_imported_and_cancellation_keeps_the_source_unchanged() { let temp = test_tempdir("coordination-source-read-only"); let roots = fixture_roots(temp.path()); copy_fixture(&roots); @@ -2047,11 +1867,16 @@ mod tests { let plan = engine .plan(&source, selection.clone(), &CancellationToken::default()) .unwrap(); - let failed = engine.execute(&plan, &CancellationToken::default(), &NoCrashInjection); - assert!(failed - .unwrap_err() - .to_string() - .contains("references missing Turn missing-turn")); + let report = engine + .execute(&plan, &CancellationToken::default(), &NoCrashInjection) + .unwrap(); + assert_eq!(report.status, MigrationRunStatus::Completed); + let imported = load_coordination_data( + &target_coordination_path(&roots), + DatabaseRole::CurrentTarget, + ) + .unwrap(); + assert_eq!(imported.tasks[0].parent_dialog_turn_id, "missing-turn"); assert_eq!(hash_source_roots(&roots), source_hash); let cancellation = CancellationToken::default(); @@ -2061,6 +1886,141 @@ mod tests { assert_eq!(hash_source_roots(&roots), source_hash); } + #[test] + fn historical_business_inconsistencies_and_bad_rows_do_not_block_merge() { + let temp = test_tempdir("permissive-coordination"); + let roots = fixture_roots(temp.path()); + copy_fixture(&roots); + let connection = materialize_coordination(&roots, false); + connection.execute_batch(r#" + UPDATE coordination_sessions SET next_auto_agent_seq = -2, updated_at_ms = -1; + UPDATE agents SET child_session_id = 'missing-child', next_bg_seq = 0, created_at_ms = -1; + UPDATE background_tasks SET parent_dialog_turn_id = 'missing-turn', parent_tool_call_id = '', child_dialog_turn_id = '', execution_owner_token = '', status = 'running', bg_ordinal = 0, created_at_ms = -1; + UPDATE swarm_nodes SET parent_session_id = 'missing-parent', depth = -9, agent_type = '', created_at_ms = -1; + INSERT INTO agents VALUES (99, 'parent-99', 'bad-type', NULL, 'not-an-integer', 'active', 0); + INSERT INTO agents VALUES (100, 'parent-100', 'unknown-state', NULL, 1, 'retired-old-state', 0); + INSERT INTO swarm_nodes VALUES ('orphan-node', 'missing-tree', NULL, '', -1, -1); + INSERT INTO background_tasks (parent_session_id, agent_pk, bg_task_id, bg_ordinal, parent_dialog_turn_id, parent_tool_call_id, child_dialog_turn_id, status, execution_owner_token, created_at_ms) + VALUES ('orphan', 999, 'orphan-task', 1, '', '', '', 'running', '', 0); + "#).unwrap(); + drop(connection); + let source_hash = hash_source_roots(&roots); + let selection = session_selection(); + let engine = MigrationEngine::new(roots.clone(), adapters_for_groups(&selection)).unwrap(); + let source = probe_legacy_source(&roots, ProbeLimits::default()) + .unwrap() + .unwrap(); + let plan = engine + .plan(&source, selection, &CancellationToken::default()) + .unwrap(); + let report = engine + .execute(&plan, &CancellationToken::default(), &NoCrashInjection) + .unwrap(); + assert_eq!(report.status, MigrationRunStatus::CompletedWithWarnings); + let result = report + .domain_results + .iter() + .find(|result| result.domain == MigrationDomainId::AgentCoordination) + .unwrap(); + assert_eq!(result.state, MigrationDomainState::Verified); + assert_eq!(result.skipped, 4); + assert_eq!(result.warnings[0].code, "coordination_rows_skipped"); + let target = load_coordination_data( + &target_coordination_path(&roots), + DatabaseRole::CurrentTarget, + ) + .unwrap(); + assert_eq!(target.sessions[0].next_auto_agent_seq, -2); + assert_eq!( + target.agents[0].child_session_id.as_deref(), + Some("missing-child") + ); + assert_eq!(target.tasks.len(), 1); + assert_eq!(target.tasks[0].parent_dialog_turn_id, "missing-turn"); + assert_eq!(target.tasks[0].status, "running"); + assert_eq!(target.tasks[0].execution_owner_token, ""); + assert!(target.swarm_nodes.iter().all(|row| row.depth == -9)); + assert_eq!(hash_source_roots(&roots), source_hash); + engine + .execute(&plan, &CancellationToken::default(), &NoCrashInjection) + .unwrap(); + assert_eq!( + coordination_digest(&target).unwrap(), + coordination_digest( + &load_coordination_data( + &target_coordination_path(&roots), + DatabaseRole::CurrentTarget + ) + .unwrap() + ) + .unwrap() + ); + let layout = openbitfun_legacy_migration::MigrationLayout::new(&roots, &plan.run_id); + let context = DomainContext { + roots: &roots, + layout: &layout, + plan: &plan, + step: plan + .steps + .iter() + .find(|step| step.domain == MigrationDomainId::AgentCoordination) + .unwrap(), + }; + let manifest = read_coordination_manifest(&context).unwrap(); + let mut old = serde_json::to_value(&manifest).unwrap(); + old.as_object_mut().unwrap().remove("incompatibleRows"); + assert_eq!( + serde_json::from_value::(old) + .unwrap() + .incompatible_rows, + 0 + ); + } + + #[test] + fn unreadable_target_record_is_preserved_and_still_participates_in_change_detection() { + let temp = test_tempdir("target-row-preserved"); + let roots = fixture_roots(temp.path()); + copy_fixture(&roots); + drop(materialize_coordination(&roots, false)); + let target = target_coordination_path(&roots); + fs::create_dir_all(target.parent().unwrap()).unwrap(); + let connection = Connection::open(&target).unwrap(); + initialize_coordination_schema(&connection).unwrap(); + connection + .execute( + "INSERT INTO coordination_sessions VALUES ('session-1', 'broken-counter', 0)", + [], + ) + .unwrap(); + drop(connection); + let selection = session_selection(); + let engine = MigrationEngine::new(roots.clone(), adapters_for_groups(&selection)).unwrap(); + let source = probe_legacy_source(&roots, ProbeLimits::default()) + .unwrap() + .unwrap(); + let plan = engine + .plan(&source, selection, &CancellationToken::default()) + .unwrap(); + engine + .execute(&plan, &CancellationToken::default(), &NoCrashInjection) + .unwrap(); + let before = coordination_digest( + &load_coordination_data(&target, DatabaseRole::CurrentTarget).unwrap(), + ) + .unwrap(); + let connection = Connection::open(&target).unwrap(); + let value: String = connection.query_row("SELECT next_auto_agent_seq FROM coordination_sessions WHERE parent_session_id = 'session-1'", [], |row| row.get(0)).unwrap(); + assert_eq!(value, "broken-counter"); + connection.execute("UPDATE coordination_sessions SET next_auto_agent_seq = 'changed-counter' WHERE parent_session_id = 'session-1'", []).unwrap(); + drop(connection); + let after = coordination_digest( + &load_coordination_data(&target, DatabaseRole::CurrentTarget).unwrap(), + ) + .unwrap(); + assert_ne!(before, after); + } + fn session_selection() -> MigrationSelection { MigrationSelection { groups: BTreeSet::from([MigrationGroupId::WorkspacesSessionsAndTasks]), diff --git a/src/crates/services/legacy-migration-adapters/src/extensions.rs b/src/crates/services/legacy-migration-adapters/src/extensions.rs index be00f2b82a..b37d9e5547 100644 --- a/src/crates/services/legacy-migration-adapters/src/extensions.rs +++ b/src/crates/services/legacy-migration-adapters/src/extensions.rs @@ -100,16 +100,21 @@ impl LegacyDomainAdapter for SkillsAdapter { } fn stage(&self, context: &DomainContext<'_>) -> LegacyMigrationResult { - let planned = plan_skills(context.roots)?; + let mut planned = plan_skills(context.roots)?; let output = stage_domain_dir(context, "skills").join("output"); let mut manifest = ImportManifest::default(); - for item in &planned { + for item in &mut planned { if matches!(item.action, ImportAction::Import | ImportAction::Remap) { - copy_declared_tree( + if copy_declared_tree( &item.source_path, &output.join(&item.target_id), &item.files, - )?; + ) + .and_then(|_| require_hash(&output.join(&item.target_id), &item.content_hash)) + .is_err() + { + item.action = ImportAction::Skip; + } } manifest.entries.push(import_entry(item)); manifest.skipped_paths.extend(item.skipped_paths.clone()); @@ -206,29 +211,36 @@ impl LegacyDomainAdapter for MiniappsAdapter { } fn stage(&self, context: &DomainContext<'_>) -> LegacyMigrationResult { - let planned = plan_miniapps(context.roots)?; + let mut planned = plan_miniapps(context.roots)?; let domain_root = stage_domain_dir(context, "miniapps"); let output = domain_root.join("output"); let mut manifest = ImportManifest::default(); - for item in &planned { - match item.action { - ImportAction::Import | ImportAction::Remap => { - let staged = output.join(&item.target_id); - write_miniapp_output(&item.source_path, &item.target_id, &staged)?; - require_hash(&staged, &item.content_hash)?; - } - ImportAction::BuiltinStorage => { - let source = item.source_path.join(STORAGE_JSON); - let value: serde_json::Value = read_bounded_json(&item.source_path, &source)?; - atomic_write_json( - &domain_root - .join("builtin-storage") - .join(&item.target_id) - .join(STORAGE_JSON), - &value, - )?; + for item in &mut planned { + let staged_result = (|| -> LegacyMigrationResult<()> { + match item.action { + ImportAction::Import | ImportAction::Remap => { + let staged = output.join(&item.target_id); + write_miniapp_output(&item.source_path, &item.target_id, &staged)?; + require_hash(&staged, &item.content_hash)?; + } + ImportAction::BuiltinStorage => { + let source = item.source_path.join(STORAGE_JSON); + let value: serde_json::Value = + read_bounded_json(&item.source_path, &source)?; + atomic_write_json( + &domain_root + .join("builtin-storage") + .join(&item.target_id) + .join(STORAGE_JSON), + &value, + )?; + } + ImportAction::Duplicate | ImportAction::TargetWins | ImportAction::Skip => {} } - ImportAction::Duplicate | ImportAction::TargetWins | ImportAction::Skip => {} + Ok(()) + })(); + if staged_result.is_err() { + item.action = ImportAction::Skip; } manifest.entries.push(import_entry(item)); manifest.skipped_paths.extend(item.skipped_paths.clone()); @@ -370,9 +382,27 @@ impl LegacyDomainAdapter for AgentsAdapter { Ok(DomainScan { finding: ScanFinding { domain: self.domain(), - code: "legacy_agents_supported".to_string(), - severity: FindingSeverity::Info, - entity_count: planned.len() as u64, + code: if planned + .iter() + .any(|item| matches!(item.action, ImportAction::Skip)) + { + "extension_items_skipped" + } else { + "legacy_agents_supported" + } + .to_string(), + severity: if planned + .iter() + .any(|item| matches!(item.action, ImportAction::Skip)) + { + FindingSeverity::Warning + } else { + FindingSeverity::Info + }, + entity_count: planned + .iter() + .filter(|item| !matches!(item.action, ImportAction::Skip)) + .count() as u64, logical_bytes: planned .iter() .map(|item| fs::metadata(&item.source_path).map_or(0, |meta| meta.len())) @@ -385,7 +415,7 @@ impl LegacyDomainAdapter for AgentsAdapter { }, conflicts, target_schema: Some("openbitfun.custom-agent.current".to_string()), - dependencies: vec![MigrationDomainId::Skills, MigrationDomainId::Miniapps], + dependencies: Vec::new(), }) } @@ -393,27 +423,19 @@ impl LegacyDomainAdapter for AgentsAdapter { let planned = plan_agents(context.roots)?; let output = stage_domain_dir(context, "agents").join("output"); let mut manifest = ImportManifest::default(); - for item in planned { + for mut item in planned { if matches!(item.action, ImportAction::Import | ImportAction::Remap) { let path = output.join(format!("{}.md", safe_component(&item.target_id))); - if let Some(parent) = path.parent() { - fs::create_dir_all(parent).map_err(|error| io(parent, error))?; + if atomic_write_bytes(&path, &item.staged_content).is_err() { + item.action = ImportAction::Skip; } - atomic_write_bytes(&path, &item.staged_content)?; - manifest.entries.push(ImportEntry { - source_id: item.source_id, - target_id: item.target_id, - action: item.action, - content_hash: item.content_hash, - }); - } else { - manifest.entries.push(ImportEntry { - source_id: item.source_id, - target_id: item.target_id, - action: item.action, - content_hash: item.content_hash, - }); } + manifest.entries.push(ImportEntry { + source_id: item.source_id, + target_id: item.target_id, + action: item.action, + content_hash: item.content_hash, + }); } atomic_write_json(&agents_manifest_path(context), &manifest)?; result_from_manifest(self.domain(), &manifest) @@ -459,38 +481,69 @@ fn plan_skills(roots: &MigrationRoots) -> LegacyMigrationResult let target_root = &roots.target_skills_root; let mut planned = Vec::new(); for source in direct_child_directories(&source_root)? { - let source_id = file_name(&source)?; - if source_id == OPENBITFUN_SYSTEM_SKILL_DIR { - continue; + let inspect = (|| -> LegacyMigrationResult<()> { + let source_id = file_name(&source)?; + if source_id == OPENBITFUN_SYSTEM_SKILL_DIR { + return Ok(()); + } + let skill_file = source.join("SKILL.md"); + let content = match fs::read_to_string(&skill_file) { + Ok(content) => content, + Err(error) if error.kind() == std::io::ErrorKind::NotFound => { + planned.push(PlannedTree { + target_id: source_id.clone(), + source_id, + action: ImportAction::Skip, + content_hash: String::new(), + source_path: source.clone(), + files: Vec::new(), + skipped_paths: Vec::new(), + }); + return Ok(()); + } + Err(error) => return Err(io(&skill_file, error)), + }; + SkillData::from_markdown( + skill_file.to_string_lossy().to_string(), + &content, + SkillLocation::User, + false, + ) + .map_err(|error| { + LegacyMigrationError::InvalidRequest(format!( + "legacy Skill {source_id} failed current owner parsing: {error}" + )) + })?; + let mut files = Vec::new(); + collect_regular_files(&source, &source, &mut files)?; + enforce_tree_limits(&source, &files)?; + let content_hash = hash_file_set(&source, &files)?; + let target = target_root.join(&source_id); + let (target_id, action) = + resolve_directory_conflict(&source_id, &content_hash, &target)?; + planned.push(PlannedTree { + source_id, + target_id, + action, + content_hash, + source_path: source.clone(), + files, + skipped_paths: Vec::new(), + }); + Ok(()) + })(); + if inspect.is_err() { + let source_id = file_name(&source)?; + planned.push(PlannedTree { + target_id: source_id.clone(), + source_id, + action: ImportAction::Skip, + content_hash: String::new(), + source_path: source, + files: Vec::new(), + skipped_paths: Vec::new(), + }); } - let skill_file = source.join("SKILL.md"); - let content = fs::read_to_string(&skill_file).map_err(|error| io(&skill_file, error))?; - SkillData::from_markdown( - skill_file.to_string_lossy().to_string(), - &content, - SkillLocation::User, - false, - ) - .map_err(|error| { - LegacyMigrationError::InvalidRequest(format!( - "legacy Skill {source_id} failed current owner parsing: {error}" - )) - })?; - let mut files = Vec::new(); - collect_regular_files(&source, &source, &mut files)?; - enforce_tree_limits(&source, &files)?; - let content_hash = hash_file_set(&source, &files)?; - let target = target_root.join(&source_id); - let (target_id, action) = resolve_directory_conflict(&source_id, &content_hash, &target)?; - planned.push(PlannedTree { - source_id, - target_id, - action, - content_hash, - source_path: source, - files, - skipped_paths: Vec::new(), - }); } Ok(planned) } @@ -504,77 +557,92 @@ fn plan_miniapps(roots: &MigrationRoots) -> LegacyMigrationResult>(); let mut planned = Vec::new(); for source in direct_child_directories(&source_root)? { - let directory_id = file_name(&source)?; - let meta_path = source.join(META_JSON); - let raw_meta: serde_json::Value = read_bounded_json(&source, &meta_path)?; - let declared_id = raw_meta - .get("id") - .and_then(serde_json::Value::as_str) - .unwrap_or_default(); - let source_id = if declared_id.trim().is_empty() { - directory_id.clone() - } else { - declared_id.to_string() - }; - let current_builtin_id = if builtin_ids.contains(source_id.as_str()) { - Some(source_id.clone()) - } else if builtin_ids.contains(directory_id.as_str()) { - Some(directory_id.clone()) - } else { - None - }; - let is_builtin = - current_builtin_id.is_some() || source.join(BUILTIN_INSTALL_MARKER).exists(); - let mut files = Vec::new(); - collect_regular_files(&source, &source, &mut files)?; - enforce_tree_limits(&source, &files)?; - let source_hash = hash_file_set(&source, &files)?; - if is_builtin { - let target_id = current_builtin_id - .clone() - .unwrap_or_else(|| remapped_id(&source_id, &source_hash)); - let target_storage = target_root.join(&target_id).join(STORAGE_JSON); + let inspect = (|| -> LegacyMigrationResult<()> { + let directory_id = file_name(&source)?; + let meta_path = source.join(META_JSON); + let raw_meta: serde_json::Value = read_bounded_json(&source, &meta_path)?; + let declared_id = raw_meta + .get("id") + .and_then(serde_json::Value::as_str) + .unwrap_or_default(); + let source_id = if declared_id.trim().is_empty() { + directory_id.clone() + } else { + declared_id.to_string() + }; + let current_builtin_id = if builtin_ids.contains(source_id.as_str()) { + Some(source_id.clone()) + } else if builtin_ids.contains(directory_id.as_str()) { + Some(directory_id.clone()) + } else { + None + }; + let is_builtin = + current_builtin_id.is_some() || source.join(BUILTIN_INSTALL_MARKER).exists(); + let mut files = Vec::new(); + collect_regular_files(&source, &source, &mut files)?; + enforce_tree_limits(&source, &files)?; + let source_hash = hash_file_set(&source, &files)?; + if is_builtin { + let target_id = current_builtin_id + .clone() + .unwrap_or_else(|| remapped_id(&source_id, &source_hash)); + let target_storage = target_root.join(&target_id).join(STORAGE_JSON); + planned.push(PlannedTree { + source_id, + target_id, + action: if current_builtin_id.is_none() || !source.join(STORAGE_JSON).exists() { + ImportAction::Skip + } else if target_storage.exists() { + ImportAction::TargetWins + } else { + ImportAction::BuiltinStorage + }, + content_hash: source_hash, + source_path: source.clone(), + files, + skipped_paths: Vec::new(), + }); + return Ok(()); + } + build_import_bundle_plan( + &source_id, + &fs::read_to_string(&meta_path).map_err(|error| io(&meta_path, error))?, + 0, + ) + .map_err(|error| { + LegacyMigrationError::InvalidRequest(format!( + "legacy MiniApp {source_id} failed current owner conversion: {error}" + )) + })?; + let (target_id, action, content_hash) = if is_safe_component(&source_id) { + resolve_miniapp_conflict(&source, &source_id, &source_hash, &target_root)? + } else { + resolve_remapped_miniapp_conflict(&source, &source_id, &source_hash, &target_root)? + }; planned.push(PlannedTree { source_id, target_id, - action: if current_builtin_id.is_none() || !source.join(STORAGE_JSON).exists() { - ImportAction::Skip - } else if target_storage.exists() { - ImportAction::TargetWins - } else { - ImportAction::BuiltinStorage - }, - content_hash: source_hash, - source_path: source, + action, + content_hash, + source_path: source.clone(), files, skipped_paths: Vec::new(), }); - continue; + Ok(()) + })(); + if inspect.is_err() { + let source_id = file_name(&source)?; + planned.push(PlannedTree { + target_id: source_id.clone(), + source_id, + action: ImportAction::Skip, + content_hash: String::new(), + source_path: source, + files: Vec::new(), + skipped_paths: Vec::new(), + }); } - build_import_bundle_plan( - &source_id, - &fs::read_to_string(&meta_path).map_err(|error| io(&meta_path, error))?, - 0, - ) - .map_err(|error| { - LegacyMigrationError::InvalidRequest(format!( - "legacy MiniApp {source_id} failed current owner conversion: {error}" - )) - })?; - let (target_id, action, content_hash) = if is_safe_component(&source_id) { - resolve_miniapp_conflict(&source, &source_id, &source_hash, &target_root)? - } else { - resolve_remapped_miniapp_conflict(&source, &source_id, &source_hash, &target_root)? - }; - planned.push(PlannedTree { - source_id, - target_id, - action, - content_hash, - source_path: source, - files, - skipped_paths: Vec::new(), - }); } Ok(planned) } @@ -594,38 +662,54 @@ fn plan_agents(roots: &MigrationRoots) -> LegacyMigrationResult { - (source_id.clone(), ImportAction::Duplicate, source_bytes) - } - Some(_) => { - let target_id = remapped_id(&source_id, &source_hash); - let remapped = rewrite_agent_id(&source_bytes, &target_id)?; - (target_id, ImportAction::Remap, remapped) - } - None => (source_id.clone(), ImportAction::Import, source_bytes), - }; - let content_hash = hash_bytes(&staged_content); - planned.push(PlannedAgent { - source_id, - target_id, - action, - content_hash, - source_path, - staged_content, - }); + let inspect = (|| -> LegacyMigrationResult<()> { + let content = + fs::read_to_string(&source_path).map_err(|error| io(&source_path, error))?; + let parsed = custom_agent_read_markdown_str(&content, CustomAgentLevel::User).map_err( + |error| { + LegacyMigrationError::InvalidRequest(format!( + "legacy Agent {} failed current owner parsing: {error}", + source_path.display() + )) + }, + )?; + let source_id = parsed.definition.id.clone(); + let source_bytes = content.into_bytes(); + let source_hash = hash_bytes(&source_bytes); + let (target_id, action, staged_content) = + match target_by_id.get(&source_id.to_ascii_lowercase()) { + Some(existing) if existing == &source_bytes => { + (source_id.clone(), ImportAction::Duplicate, source_bytes) + } + Some(_) => { + let target_id = remapped_id(&source_id, &source_hash); + let remapped = rewrite_agent_id(&source_bytes, &target_id)?; + (target_id, ImportAction::Remap, remapped) + } + None => (source_id.clone(), ImportAction::Import, source_bytes), + }; + let content_hash = hash_bytes(&staged_content); + planned.push(PlannedAgent { + source_id, + target_id, + action, + content_hash, + source_path: source_path.clone(), + staged_content, + }); + Ok(()) + })(); + if inspect.is_err() { + let source_id = file_name(&source_path)?; + planned.push(PlannedAgent { + target_id: source_id.clone(), + source_id, + action: ImportAction::Skip, + content_hash: String::new(), + source_path: source_path, + staged_content: Vec::new(), + }); + } } Ok(planned) } @@ -872,9 +956,13 @@ fn scan_from_trees( DomainScan { finding: ScanFinding { domain, - code: code.to_string(), - severity: FindingSeverity::Info, - entity_count: planned.len() as u64, + code: if planned.iter().any(|item| matches!(item.action, ImportAction::Skip)) { + "extension_items_skipped".to_string() + } else { code.to_string() }, + severity: if planned.iter().any(|item| matches!(item.action, ImportAction::Skip)) { + FindingSeverity::Warning + } else { FindingSeverity::Info }, + entity_count: planned.iter().filter(|item| !matches!(item.action, ImportAction::Skip)).count() as u64, logical_bytes: planned .iter() .map(|item| { @@ -897,10 +985,7 @@ fn scan_from_trees( }) .collect(), target_schema: Some(target_schema.to_string()), - dependencies: match domain { - MigrationDomainId::Miniapps => vec![MigrationDomainId::Skills], - _ => Vec::new(), - }, + dependencies: Vec::new(), } } @@ -979,6 +1064,12 @@ fn result_from_manifest( message: "The path is outside the current owner's import contract.".to_string(), action: Some("Review the source extension manually.".to_string()), }) + .chain(manifest.entries.iter().filter(|entry| matches!(entry.action, ImportAction::Skip)).map(|entry| MigrationDiagnostic { + code: "extension_item_skipped".to_string(), severity: FindingSeverity::Warning, + domain: Some(domain), relative_path: Some(entry.source_id.clone()), + message: "The source extension is incomplete or outside the import contract and was preserved.".to_string(), + ..Default::default() + })) .collect(); Ok(MigrationDomainResult { domain, @@ -1789,6 +1880,71 @@ mod tests { assert!(text.ends_with("Keep this body byte-for-byte.\r\n")); } + #[test] + fn invalid_extensions_do_not_block_valid_entries_in_the_same_domain() { + let temp = test_tempdir("partial-extensions"); + let roots = fixture_roots(temp.path()); + copy_fixture(&roots); + assert!(AgentsAdapter.scan(&roots).unwrap().dependencies.is_empty()); + assert!(MiniappsAdapter + .scan(&roots) + .unwrap() + .dependencies + .is_empty()); + fs::create_dir_all(roots.legacy_skills_root.join("empty-skill")).unwrap(); + atomic_write_bytes( + &roots.legacy_skills_root.join("broken-skill/SKILL.md"), + b"---\ninvalid: [\n---", + ) + .unwrap(); + atomic_write_bytes( + &roots + .legacy_user_root + .join("data/miniapps/broken/meta.json"), + b"invalid", + ) + .unwrap(); + atomic_write_bytes( + &roots.legacy_user_root.join("agents/broken.md"), + b"---\ninvalid: [\n---", + ) + .unwrap(); + let selection = MigrationSelection { + groups: BTreeSet::from([MigrationGroupId::AgentsSkillsAndMiniapps]), + }; + let source = probe_legacy_source(&roots, ProbeLimits::default()) + .unwrap() + .unwrap(); + let engine = MigrationEngine::new(roots.clone(), adapters_for_groups(&selection)).unwrap(); + let plan = engine + .plan(&source, selection, &CancellationToken::default()) + .unwrap(); + let report = engine + .execute(&plan, &CancellationToken::default(), &NoCrashInjection) + .unwrap(); + for domain in [ + MigrationDomainId::Skills, + MigrationDomainId::Miniapps, + MigrationDomainId::Agents, + ] { + let result = report + .domain_results + .iter() + .find(|result| result.domain == domain) + .unwrap(); + assert_eq!(result.state, MigrationDomainState::Verified); + assert!(result.imported > 0, "{domain:?}"); + assert!(result.skipped > 0, "{domain:?}"); + assert!(!result.warnings.is_empty(), "{domain:?}"); + } + assert!(roots + .target_skills_root + .join("user-skill/SKILL.md") + .exists()); + assert!(!roots.target_skills_root.join("empty-skill").exists()); + assert!(roots.legacy_skills_root.join("empty-skill").exists()); + } + fn fixture_roots(root: &Path) -> MigrationRoots { let legacy_user_root = root.join("legacy-user"); MigrationRoots { diff --git a/src/crates/services/legacy-migration-adapters/src/lib.rs b/src/crates/services/legacy-migration-adapters/src/lib.rs index e1e10c3cd8..3f8f9761dd 100644 --- a/src/crates/services/legacy-migration-adapters/src/lib.rs +++ b/src/crates/services/legacy-migration-adapters/src/lib.rs @@ -58,7 +58,13 @@ pub fn adapters_for_groups(selection: &MigrationSelection) -> Vec LegacyMigrationResult<()> { .map_err(LegacyMigrationError::UnsupportedTarget) } -struct CrossReferenceAdapter; +struct CrossReferenceAdapter { + dependencies: Vec, +} impl LegacyDomainAdapter for CrossReferenceAdapter { fn domain(&self) -> MigrationDomainId { @@ -99,7 +107,7 @@ impl LegacyDomainAdapter for CrossReferenceAdapter { }, conflicts: Vec::new(), target_schema: Some("openbitfun.cross-references.current".to_string()), - dependencies: Vec::new(), + dependencies: self.dependencies.clone(), }) } @@ -131,7 +139,7 @@ fn validate_selected_cross_references(context: &DomainContext<'_>) -> LegacyMigr .groups .contains(&MigrationGroupId::WorkspacesSessionsAndTasks) { - agent_coordination::validate_committed_coordination_cross_references(context)?; + agent_coordination::validate_committed_coordination_storage(context)?; } Ok(()) } diff --git a/src/crates/services/legacy-migration-adapters/src/memory.rs b/src/crates/services/legacy-migration-adapters/src/memory.rs index bf84db69d6..5942291fae 100644 --- a/src/crates/services/legacy-migration-adapters/src/memory.rs +++ b/src/crates/services/legacy-migration-adapters/src/memory.rs @@ -8,13 +8,13 @@ use openbitfun_legacy_migration::{ MigrationRoots, }; use openbitfun_product_domains::legacy_migration::{ - ConflictResolution, FindingSeverity, MigrationConflict, MigrationDomainId, + ConflictResolution, FindingSeverity, MigrationConflict, MigrationDiagnostic, MigrationDomainId, MigrationDomainResult, MigrationDomainState, ScanFinding, }; use openbitfun_services_core::memory_store::{ - classify_memory_workspace_file, initialize_memory_schema, read_memory_store_snapshot, - upsert_memory_record, MemoryRecord, MemoryStoreSnapshot, MemoryWorkspaceFileKind, - MEMORY_STORE_SCHEMA, + classify_memory_workspace_file, decode_memory_record, initialize_memory_schema, + read_memory_store_snapshot, upsert_memory_record, MemoryRecord, MemoryStoreSnapshot, + MemoryWorkspaceFileKind, MEMORY_STORE_SCHEMA, }; use rusqlite::{Connection, OpenFlags}; use serde::{Deserialize, Serialize}; @@ -31,6 +31,31 @@ const MAX_MEMORY_FILE_COUNT: usize = 4_096; const MAX_MEMORY_FILE_BYTES: u64 = 64 * 1024 * 1024; const MAX_MEMORY_TOTAL_BYTES: u64 = 512 * 1024 * 1024; +#[derive(Default, Serialize)] +struct MigrationMemorySnapshot { + #[serde(flatten)] + data: MemoryStoreSnapshot, + #[serde(skip_serializing_if = "Vec::is_empty")] + omitted: Vec, +} +impl std::ops::Deref for MigrationMemorySnapshot { + type Target = MemoryStoreSnapshot; + fn deref(&self) -> &Self::Target { + &self.data + } +} + +fn memory_warning(path: &str) -> MigrationDiagnostic { + MigrationDiagnostic { + code: "memory_item_skipped".into(), + severity: FindingSeverity::Warning, + domain: None, + relative_path: Some(path.to_string()), + message: "An unreadable memory item was retained in the source.".into(), + ..Default::default() + } +} + pub(crate) struct StructuredMemoryAdapter; pub(crate) struct FileMemoryAdapter; @@ -89,6 +114,8 @@ struct FileMemoryCommitReceipt { #[derive(Debug, Clone, Default, Serialize, Deserialize)] #[serde(default, rename_all = "camelCase")] struct FileMemoryManifest { + #[serde(skip_serializing_if = "Vec::is_empty")] + omitted: Vec, entries: Vec, imported: u64, skipped: u64, @@ -107,6 +134,7 @@ struct FileMemoryPlan { files: Vec, conflicts: Vec, logical_bytes: u64, + omitted: Vec, } struct MemoryFileFact { @@ -150,15 +178,20 @@ impl LegacyDomainAdapter for StructuredMemoryAdapter { MemoryDatabaseRole::CurrentTarget, )? } else { - MemoryStoreSnapshot::default() + MigrationMemorySnapshot::default() }; let outcome = preview_structured_merge(&source.records, &target.records); let logical_bytes = sqlite_family_size(&source_path)?; Ok(DomainScan { finding: ScanFinding { domain: self.domain(), - code: "legacy_structured_memory_supported".to_string(), - severity: if outcome.conflicts.is_empty() { + code: if source.omitted.is_empty() { + "legacy_structured_memory_supported" + } else { + "memory_items_skipped" + } + .to_string(), + severity: if outcome.conflicts.is_empty() && source.omitted.is_empty() { FindingSeverity::Info } else { FindingSeverity::Warning @@ -221,7 +254,7 @@ impl LegacyDomainAdapter for StructuredMemoryAdapter { let connection = Connection::open(&staged_merged) .map_err(|error| db_error(&staged_merged, error))?; initialize_memory_schema(&connection).map_err(owner_error)?; - (MemoryStoreSnapshot::default(), None) + (MigrationMemorySnapshot::default(), None) }; let outcome = preview_structured_merge(&source.records, &target.records); @@ -247,7 +280,10 @@ impl LegacyDomainAdapter for StructuredMemoryAdapter { target_digest, merged_digest: Some(memory_snapshot_digest(&merged)?), imported: outcome.imports.len() as u64, - skipped: outcome.duplicate.saturating_add(outcome.target_wins), + skipped: outcome + .duplicate + .saturating_add(outcome.target_wins) + .saturating_add(source.omitted.len() as u64), conflicts: outcome.conflicts.len() as u64, }; atomic_write_json(&structured_manifest_path(context), &manifest)?; @@ -257,6 +293,11 @@ impl LegacyDomainAdapter for StructuredMemoryAdapter { imported: manifest.imported, skipped: manifest.skipped, conflicts: manifest.conflicts, + warnings: source + .omitted + .iter() + .map(|hash| memory_warning(hash)) + .collect(), ..MigrationDomainResult::default() }) } @@ -271,7 +312,22 @@ impl LegacyDomainAdapter for StructuredMemoryAdapter { &domain_root.join("source.sqlite"), MemoryDatabaseRole::LegacySource, )?; - if Some(memory_snapshot_digest(&source)?) != manifest.source_digest { + let legacy_digest = || -> Option { + let connection = Connection::open_with_flags( + domain_root.join("source.sqlite"), + OpenFlags::SQLITE_OPEN_READ_ONLY, + ) + .ok()?; + let data = read_memory_store_snapshot(&connection).ok()?; + memory_snapshot_digest(&MigrationMemorySnapshot { + data, + omitted: Vec::new(), + }) + .ok() + }; + if Some(memory_snapshot_digest(&source)?) != manifest.source_digest + && (manifest.source_digest.is_none() || legacy_digest() != manifest.source_digest) + { return Err(LegacyMigrationError::InvalidRequest( "staged structured Memory source differs from its manifest".to_string(), )); @@ -450,13 +506,15 @@ impl LegacyDomainAdapter for FileMemoryAdapter { Ok(DomainScan { finding: ScanFinding { domain: self.domain(), - code: if plan.files.is_empty() { + code: if !plan.omitted.is_empty() { + "memory_items_skipped" + } else if plan.files.is_empty() { "legacy_file_memory_absent" } else { "legacy_file_memory_supported" } .to_string(), - severity: if plan.conflicts.is_empty() { + severity: if plan.conflicts.is_empty() && plan.omitted.is_empty() { FindingSeverity::Info } else { FindingSeverity::Warning @@ -477,18 +535,26 @@ impl LegacyDomainAdapter for FileMemoryAdapter { } fn stage(&self, context: &DomainContext<'_>) -> LegacyMigrationResult { - let plan = plan_file_memory(context.roots)?; + let mut plan = plan_file_memory(context.roots)?; let domain_root = stage_domain_dir(context, "file-memory"); reset_directory(&domain_root)?; let files_root = domain_root.join("files"); let mut entries = Vec::with_capacity(plan.files.len()); let mut imported = 0u64; - let mut skipped = 0u64; + let mut skipped = plan.omitted.len() as u64; for file in plan.files { if file.action == FileMemoryAction::Import { - let bytes = fs::read(&file.source_path) - .map_err(|error| io_error(&file.source_path, error))?; - atomic_write_bytes(&files_root.join(&file.target_relative), &bytes)?; + let staged_result = (|| -> LegacyMigrationResult<()> { + let bytes = fs::read(&file.source_path) + .map_err(|error| io_error(&file.source_path, error))?; + atomic_write_bytes(&files_root.join(&file.target_relative), &bytes)?; + Ok(()) + })(); + if staged_result.is_err() { + skipped += 1; + plan.omitted.push(relative_string(&file.source_relative)); + continue; + } imported = imported.saturating_add(1); } else { skipped = skipped.saturating_add(1); @@ -501,6 +567,7 @@ impl LegacyDomainAdapter for FileMemoryAdapter { }); } let manifest = FileMemoryManifest { + omitted: plan.omitted.clone(), entries, imported, skipped, @@ -513,6 +580,11 @@ impl LegacyDomainAdapter for FileMemoryAdapter { imported, skipped, conflicts: manifest.conflicts, + warnings: plan + .omitted + .iter() + .map(|path| memory_warning(path)) + .collect(), ..MigrationDomainResult::default() }) } @@ -708,7 +780,8 @@ fn structured_conflict( fn plan_file_memory(roots: &MigrationRoots) -> LegacyMigrationResult { let source_root = source_file_memory_root(roots); - let source = collect_memory_files(&roots.legacy_home_root, &source_root)?; + let mut omitted = Vec::new(); + let source = collect_source_memory_files(&roots.legacy_home_root, &source_root, &mut omitted)?; let target_root = target_file_memory_root(roots); let target = collect_memory_files(&roots.target_home_root, &target_root)?; let target_by_path = target @@ -770,55 +843,90 @@ fn plan_file_memory(roots: &MigrationRoots) -> LegacyMigrationResult, ) -> LegacyMigrationResult> { - if !path_entry_exists(memory_root)? { + if !path_entry_exists(root)? { return Ok(Vec::new()); } - validate_directory(boundary_root, memory_root)?; + validate_directory(boundary, root)?; let mut files = Vec::new(); for name in ["MEMORY.md", "memory_summary.md"] { - let path = memory_root.join(name); - if path_entry_exists(&path)? { - push_memory_file(memory_root, &path, &mut files)?; - } - } - let extensions = memory_root.join("extensions"); - let ad_hoc = extensions.join("ad_hoc"); - let notes = ad_hoc.join("notes"); - for directory in [&extensions, &ad_hoc, ¬es] { - if path_entry_exists(directory)? { - validate_directory(memory_root, directory)?; - } else { - return finish_memory_files(files); + let path = root.join(name); + if path_entry_exists(&path)? && push_memory_file(root, &path, &mut files).is_err() { + omitted.push(relative_string(Path::new(name))); } } - for entry in fs::read_dir(¬es).map_err(|error| io_error(¬es, error))? { - let entry = entry.map_err(|error| io_error(¬es, error))?; - let path = entry.path(); - let metadata = fs::symlink_metadata(&path).map_err(|error| io_error(&path, error))?; - if metadata.file_type().is_symlink() || is_reparse_point(&metadata) { - return Err(LegacyMigrationError::LinkedPath(path)); + fn visit( + root: &Path, + path: &Path, + files: &mut Vec, + omitted: &mut Vec, + depth: usize, + ) { + if depth > 16 || files.len() >= MAX_MEMORY_FILE_COUNT { + omitted.push(relative_string(path.strip_prefix(root).unwrap_or(path))); + return; } - if metadata.is_dir() { - return Err(LegacyMigrationError::InvalidRequest(format!( - "nested file Memory directories are unsupported: {}", - path.display() - ))); + let inspect = (|| -> LegacyMigrationResult<()> { + validate_directory(root, path)?; + for entry in fs::read_dir(path).map_err(|error| io_error(path, error))? { + let entry = entry.map_err(|error| io_error(path, error))?; + let child = entry.path(); + let meta = fs::symlink_metadata(&child).map_err(|error| io_error(&child, error))?; + if meta.is_dir() && !meta.file_type().is_symlink() && !is_reparse_point(&meta) { + visit(root, &child, files, omitted, depth + 1); + } else if classify_memory_workspace_file(child.strip_prefix(root).unwrap_or(&child)) + .is_some() + && push_memory_file(root, &child, files).is_err() + { + omitted.push(relative_string(child.strip_prefix(root).unwrap_or(&child))); + } + } + Ok(()) + })(); + if inspect.is_err() { + omitted.push(relative_string(path.strip_prefix(root).unwrap_or(path))); } - if classify_memory_workspace_file( - path.strip_prefix(memory_root) - .map_err(|_| LegacyMigrationError::PathEscape(path.clone()))?, - ) - .is_some() + } + let notes = root.join("extensions/ad_hoc/notes"); + if path_entry_exists(¬es)? { + visit(root, ¬es, &mut files, omitted, 0); + } + files.sort_by(|left, right| left.relative.cmp(&right.relative)); + let mut count = 0usize; + let mut total = 0u64; + files.retain(|file| { + if count >= MAX_MEMORY_FILE_COUNT + || total.saturating_add(file.bytes) > MAX_MEMORY_TOTAL_BYTES { - push_memory_file(memory_root, &path, &mut files)?; + omitted.push(relative_string(&file.relative)); + false + } else { + count += 1; + total += file.bytes; + true } + }); + Ok(files) +} + +fn collect_memory_files( + boundary_root: &Path, + memory_root: &Path, +) -> LegacyMigrationResult> { + let mut omitted = Vec::new(); + let files = collect_source_memory_files(boundary_root, memory_root, &mut omitted)?; + if !omitted.is_empty() { + return Err(LegacyMigrationError::InvalidRequest( + "target file Memory contains unreadable items; preserving target".into(), + )); } finish_memory_files(files) } @@ -950,7 +1058,7 @@ fn normalize_text(text: &str) -> String { normalized.trim_end_matches('\n').to_string() } -fn memory_snapshot_digest(snapshot: &MemoryStoreSnapshot) -> LegacyMigrationResult { +fn memory_snapshot_digest(snapshot: &MigrationMemorySnapshot) -> LegacyMigrationResult { let bytes = serde_json::to_vec(snapshot) .map_err(|error| LegacyMigrationError::InvalidRequest(error.to_string()))?; Ok(hex::encode(Sha256::digest(bytes))) @@ -962,7 +1070,7 @@ fn read_consistent_scan_snapshot( source: &Path, label: &str, role: MemoryDatabaseRole, -) -> LegacyMigrationResult { +) -> LegacyMigrationResult { validate_sqlite_family(boundary, source)?; let scan_root = roots.migration_root().join("scan-snapshots"); fs::create_dir_all(&scan_root).map_err(|error| io_error(&scan_root, error))?; @@ -990,23 +1098,55 @@ enum MemoryDatabaseRole { fn read_memory_snapshot( path: &Path, role: MemoryDatabaseRole, -) -> LegacyMigrationResult { +) -> LegacyMigrationResult { validate_sqlite(path)?; let connection = Connection::open_with_flags( path, OpenFlags::SQLITE_OPEN_READ_ONLY | OpenFlags::SQLITE_OPEN_NO_MUTEX, ) .map_err(|error| db_error(path, error))?; - read_memory_store_snapshot(&connection).map_err(|error| match role { - MemoryDatabaseRole::LegacySource => LegacyMigrationError::UnsupportedSource(format!( - "legacy structured Memory schema is not supported: {error}" - )), - MemoryDatabaseRole::CurrentTarget | MemoryDatabaseRole::StagedCurrent => { + if matches!(role, MemoryDatabaseRole::LegacySource) { + // Historical jobs are recreated by runtime acquisition and never imported. + // Only the content table is required; malformed rows do not poison peers. + let mut statement = connection.prepare("SELECT thread_id, workspace_path, rollout_path, source_updated_at, raw_memory, rollout_summary, rollout_slug, generated_at, COALESCE(usage_count, 0), last_usage, selected_for_phase2, selected_for_phase2_source_updated_at FROM stage1_outputs ORDER BY thread_id") + .map_err(|error| LegacyMigrationError::UnsupportedSource(format!("legacy structured Memory schema is not supported: {error}")))?; + let mut rows = statement.query([]).map_err(|error| db_error(path, error))?; + let mut snapshot = MigrationMemorySnapshot::default(); + while let Some(row) = rows.next().map_err(|error| db_error(path, error))? { + match decode_memory_record(row) { + Ok(mut record) if !record.session_id.trim().is_empty() => { + record.selected_for_phase2 = i64::from(record.selected_for_phase2 != 0); + record.usage_count = record.usage_count.max(0); + snapshot.data.records.push(record); + } + _ => { + let mut hash = Sha256::new(); + for index in 0..row.as_ref().column_count() { + hash.update( + format!( + "{:?}", + row.get_ref(index).map_err(|error| db_error(path, error))? + ) + .as_bytes(), + ); + hash.update([0]); + } + snapshot.omitted.push(hex::encode(hash.finalize())); + } + } + } + return Ok(snapshot); + } + read_memory_store_snapshot(&connection) + .map(|data| MigrationMemorySnapshot { + data, + omitted: Vec::new(), + }) + .map_err(|error| { LegacyMigrationError::InvalidRequest(format!( "current structured Memory schema is invalid: {error}" )) - } - }) + }) } fn verify_structured_target_state( @@ -1357,6 +1497,36 @@ mod tests { } } + #[test] + fn legacy_memory_jobs_are_optional_and_bad_rows_are_isolated() { + let temp = test_tempdir("memory-bad-row"); + let roots = fixture_roots(temp.path()); + copy_fixture(&roots); + let connection = materialize_source_memory(&roots, false); + connection.execute_batch("DROP TABLE jobs;").unwrap(); + let original = read_memory_snapshot( + &source_structured_memory_path(&roots), + MemoryDatabaseRole::LegacySource, + ) + .unwrap(); + assert!(!original.records.is_empty()); + let id = original.records[0].session_id.clone(); + connection + .execute( + "UPDATE stage1_outputs SET raw_memory = X'FF' WHERE thread_id = ?1", + [&id], + ) + .unwrap(); + let recovered = read_memory_snapshot( + &source_structured_memory_path(&roots), + MemoryDatabaseRole::LegacySource, + ) + .unwrap(); + assert_eq!(recovered.records.len() + 1, original.records.len()); + assert_eq!(recovered.omitted.len(), 1); + assert!(recovered.jobs.is_empty()); + } + #[test] fn structured_memory_uses_wal_target_priority_hash_dedup_and_owner_validation() { let temp = test_tempdir("structured-memory"); @@ -1884,9 +2054,13 @@ mod tests { let engine = MigrationEngine::new(roots.clone(), adapters_for_groups(&selection)).unwrap(); assert!(engine .plan(&source, selection.clone(), &CancellationToken::default()) - .unwrap_err() - .to_string() - .contains("legacy structured Memory schema is not supported")); + .unwrap() + .findings + .iter() + .any(|finding| !finding.migratable + && finding + .detail + .contains("legacy structured Memory schema is not supported"))); assert_eq!(hash_source_roots(&roots), source_hash); fs::remove_file(&invalid_path).unwrap(); @@ -1911,10 +2085,8 @@ mod tests { validated_memory_relative_path("../MEMORY.md"), Err(LegacyMigrationError::PathEscape(_)) )); - assert!(matches!( - validated_memory_relative_path("extensions/ad_hoc/notes/nested/note.md"), - Err(LegacyMigrationError::PathEscape(_)) - )); + assert!(validated_memory_relative_path("extensions/ad_hoc/notes/nested/note.md").is_ok()); + assert!(validated_memory_relative_path("extensions/ad_hoc/notes/../note.md").is_err()); } #[test] diff --git a/src/crates/services/legacy-migration-adapters/src/remote_connect.rs b/src/crates/services/legacy-migration-adapters/src/remote_connect.rs index eb1f7fc955..29201ee64b 100644 --- a/src/crates/services/legacy-migration-adapters/src/remote_connect.rs +++ b/src/crates/services/legacy-migration-adapters/src/remote_connect.rs @@ -48,6 +48,8 @@ enum BotSourceKind { #[derive(Debug, Clone, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] struct RemoteConnectManifest { + #[serde(default, skip_serializing_if = "Vec::is_empty")] + omitted: Vec, source_files: BTreeMap, target_before: BTreeMap>, bot_source_kind: BotSourceKind, @@ -77,6 +79,7 @@ struct RemoteConnectOutcome { #[derive(Default)] struct RemoteConnectState { + omitted: Vec, files: BTreeMap, device: Option, account_session_present: bool, @@ -108,12 +111,14 @@ impl LegacyDomainAdapter for RemoteConnectAdapter { Ok(DomainScan { finding: ScanFinding { domain: self.domain(), - code: if source.files.is_empty() { + code: if !source.omitted.is_empty() { + "remote_items_skipped".to_string() + } else if source.files.is_empty() { "legacy_remote_connect_absent".to_string() } else { "legacy_remote_connect_supported".to_string() }, - severity: if preview.conflicts.is_empty() { + severity: if preview.conflicts.is_empty() && source.omitted.is_empty() { FindingSeverity::Info } else { FindingSeverity::Warning @@ -143,6 +148,7 @@ impl LegacyDomainAdapter for RemoteConnectAdapter { }) .collect(); let manifest = RemoteConnectManifest { + omitted: source.omitted, source_files: source.files, target_before, bot_source_kind: source.bot_source_kind, @@ -160,6 +166,11 @@ impl LegacyDomainAdapter for RemoteConnectAdapter { imported: manifest.imported, skipped: manifest.skipped, conflicts: manifest.conflicts, + warnings: manifest + .omitted + .iter() + .map(|path| remote_skipped_warning(path)) + .collect(), ..MigrationDomainResult::default() }) } @@ -251,6 +262,11 @@ impl LegacyDomainAdapter for RemoteConnectAdapter { imported: manifest.imported, skipped: manifest.skipped, conflicts: manifest.conflicts, + warnings: manifest + .omitted + .iter() + .map(|path| remote_skipped_warning(path)) + .collect(), ..RemoteConnectOutcome::default() }; if manifest.conflicts > 0 { @@ -344,7 +360,7 @@ struct Preview { fn preview(source: &RemoteConnectState, target: &RemoteConnectState) -> Preview { let mut imported = 0; - let mut skipped = 0; + let mut skipped = source.omitted.len() as u64; let mut conflicts = Vec::new(); if source.device.is_some() { if target.device.is_some() { @@ -774,18 +790,34 @@ fn merge_weixin_auxiliary( fn read_state(root: &Path, legacy: bool) -> LegacyMigrationResult { let mut state = RemoteConnectState::default(); let device_path = root.join("device_identity.json"); - if existing_regular(root, &device_path, MAX_JSON_BYTES)? { - state.device = owner::read_device_identity(&device_path).map_err(owner_error)?; + if source_file_exists( + root, + &device_path, + MAX_JSON_BYTES, + legacy, + &mut state.omitted, + )? { + state.device = source_optional( + owner::read_device_identity(&device_path).map_err(owner_error), + legacy, + "device_identity.json", + &mut state.omitted, + )?; record_file(root, &device_path, &mut state.files)?; } let hint_path = root.join("account_hint.json"); - if existing_regular(root, &hint_path, MAX_JSON_BYTES)? { - state.account_hint = owner::read_account_hint(&hint_path).map_err(owner_error)?; + if source_file_exists(root, &hint_path, MAX_JSON_BYTES, legacy, &mut state.omitted)? { + state.account_hint = source_optional( + owner::read_account_hint(&hint_path).map_err(owner_error), + legacy, + "account_hint.json", + &mut state.omitted, + )?; record_file(root, &hint_path, &mut state.files)?; } for name in ["account_session.enc", "account_session.key"] { let path = root.join(name); - if existing_regular(root, &path, MAX_SECRET_BYTES)? { + if source_file_exists(root, &path, MAX_SECRET_BYTES, legacy, &mut state.omitted)? { record_file(root, &path, &mut state.files)?; } } @@ -800,7 +832,9 @@ fn read_state(root: &Path, legacy: bool) -> LegacyMigrationResult LegacyMigrationResult LegacyMigrationResult) -> LegacyMigrationResult<()> { fs::create_dir_all(&path).map_err(|error| io_error(&path, error)) } +fn source_optional( + result: LegacyMigrationResult>, + legacy: bool, + path: &str, + omitted: &mut Vec, +) -> LegacyMigrationResult> { + match result { + Err(_) if legacy => { + omitted.push(path.into()); + Ok(None) + } + other => other, + } +} +fn remote_skipped_warning(path: &str) -> MigrationDiagnostic { + MigrationDiagnostic { + severity: FindingSeverity::Warning, + domain: Some(MigrationDomainId::RemoteConnectDevices), + code: "remote_items_skipped".into(), + message: "An unreadable legacy remote record was omitted; other records remain available." + .into(), + relative_path: Some(path.into()), + ..Default::default() + } +} +fn read_source_bot( + root: &Path, + path: &Path, + omitted: &mut Vec, +) -> Option { + let name = relative_display(root, path); + let Ok(serde_json::Value::Object(mut fields)) = + read_bounded_json::(root, path) + else { + omitted.push(name); + return None; + }; + let mut bot = BotPersistenceRecord::default(); + if let Some(value) = fields.remove("form_state") { + match serde_json::from_value(value) { + Ok(value) => bot.form_state = value, + Err(_) => omitted.push(format!("{name}/form_state")), + } + } + bot.verbose_mode = fields + .get("verbose_mode") + .and_then(serde_json::Value::as_bool) + .unwrap_or_default(); + match fields.remove("connections") { + Some(serde_json::Value::Array(values)) => { + for (index, value) in values.into_iter().enumerate() { + let label = format!("{name}/connections/{index}"); + let Ok(mut connection) = serde_json::from_value::(value) + else { + omitted.push(label); + continue; + }; + let kind = match &connection.config { + BotConfigRecord::Feishu { .. } => "feishu", + BotConfigRecord::Telegram { .. } => "telegram", + BotConfigRecord::Weixin { bot_account_id, .. } => { + if !bot_account_id.is_empty() + && !owner::is_safe_weixin_account_id(bot_account_id) + { + omitted.push(label); + continue; + } + "weixin" + } + }; + connection.bot_type = kind.into(); + connection.chat_state.chat_id = connection.chat_id.clone(); + if bot.connections.iter().any(|other| other.bot_type == kind) { + omitted.push(label); + continue; + } + bot.connections.push(connection); + } + } + Some(_) => omitted.push(format!("{name}/connections")), + None => {} + } + Some(bot) +} + +fn source_file_exists( + root: &Path, + path: &Path, + limit: u64, + legacy: bool, + omitted: &mut Vec, +) -> LegacyMigrationResult { + match existing_regular(root, path, limit) { + Err(_) if legacy => { + omitted.push(relative_display(root, path)); + Ok(false) + } + other => other, + } +} + #[cfg(test)] mod tests { use super::*; @@ -1169,6 +1335,24 @@ mod tests { }; use std::sync::atomic::{AtomicBool, Ordering}; + #[test] + fn corrupt_remote_files_do_not_block_independent_data() { + let temp = tempfile::tempdir().unwrap(); + let root = temp.path(); + fs::write(root.join("device_identity.json"), b"broken JSON").unwrap(); + atomic_write_json( + &root.join("remote_connect_persistence.json"), + &serde_json::json!({"connections":[{"config":null}],"verbose_mode":true}), + ) + .unwrap(); + let source = read_state(root, true).unwrap(); + assert!(source.device.is_none()); + assert!(source.bot.as_ref().unwrap().verbose_mode); + assert_eq!(source.omitted.len(), 2); + assert_eq!(source.files.len(), 2); + assert!(read_state(root, false).is_err()); + } + #[test] fn remote_connect_merge_preserves_target_identity_and_remote_bot_context() { let temp = test_tempdir("merge"); diff --git a/src/crates/services/legacy-migration-adapters/src/remote_ssh.rs b/src/crates/services/legacy-migration-adapters/src/remote_ssh.rs index d2e6243135..4b6dc0fd18 100644 --- a/src/crates/services/legacy-migration-adapters/src/remote_ssh.rs +++ b/src/crates/services/legacy-migration-adapters/src/remote_ssh.rs @@ -46,6 +46,7 @@ enum VaultHealth { } struct SshState { + omitted: Vec, files: BTreeMap, connections: Vec, workspaces: Vec, @@ -70,6 +71,8 @@ struct VaultSource { #[derive(Debug, Clone, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] struct RemoteSshManifest { + #[serde(default, skip_serializing_if = "Vec::is_empty")] + omitted: Vec, source_files: BTreeMap, target_before: BTreeMap>, staged_files: BTreeMap, @@ -122,12 +125,15 @@ impl LegacyDomainAdapter for RemoteSshAdapter { Ok(DomainScan { finding: ScanFinding { domain: self.domain(), - code: if source.files.is_empty() { + code: if !source.omitted.is_empty() { + "remote_items_skipped".to_string() + } else if source.files.is_empty() { "legacy_remote_ssh_absent".to_string() } else { "legacy_remote_ssh_supported".to_string() }, severity: if merged.conflicts.is_empty() + && source.omitted.is_empty() && source.vault_health != VaultHealth::Invalid { FindingSeverity::Info @@ -182,6 +188,7 @@ impl LegacyDomainAdapter for RemoteSshAdapter { .map(|relative| (relative.to_string(), target.files.get(relative).cloned())) .collect(); let manifest = RemoteSshManifest { + omitted: source.omitted, source_files: source.files, target_before, staged_files, @@ -199,6 +206,11 @@ impl LegacyDomainAdapter for RemoteSshAdapter { imported: manifest.imported, skipped: manifest.skipped, conflicts: manifest.conflicts, + warnings: manifest + .omitted + .iter() + .map(|path| skipped_warning(path)) + .collect(), ..MigrationDomainResult::default() }) } @@ -279,6 +291,11 @@ impl LegacyDomainAdapter for RemoteSshAdapter { imported: manifest.imported, skipped: manifest.skipped, conflicts: manifest.conflicts, + warnings: manifest + .omitted + .iter() + .map(|path| skipped_warning(path)) + .collect(), ..RemoteSshOutcome::default() }; if manifest.conflicts > 0 { @@ -400,28 +417,60 @@ impl LegacyDomainAdapter for RemoteSshAdapter { fn read_state(root: &Path, legacy: bool) -> LegacyMigrationResult { let mut files = BTreeMap::new(); + let mut omitted = Vec::new(); for relative in FILES { let path = root.join(relative); - if existing_regular( - root, - &path, - if relative.contains("vault") { - MAX_VAULT_BYTES + let inspected = (|| -> LegacyMigrationResult> { + if existing_regular( + root, + &path, + if relative.contains("vault") { + MAX_VAULT_BYTES + } else { + MAX_JSON_BYTES + }, + )? { + Ok(Some(file_digest(&path)?)) } else { - MAX_JSON_BYTES - }, - )? { - files.insert(relative.to_string(), file_digest(&path)?); + Ok(None) + } + })(); + match inspected { + Ok(Some(digest)) => { + files.insert(relative.to_string(), digest); + } + Ok(None) => {} + Err(_) if legacy => omitted.push(relative.to_string()), + Err(error) => return Err(error), } } - let connections = if files.contains_key("ssh_connections.json") { + + let connections = if legacy { + read_source_records( + root, + "ssh_connections.json", + false, + &mut omitted, + |v: &SavedConnectionRecord| v.validate().is_ok(), + ) + } else if files.contains_key("ssh_connections.json") { owner::read_saved_connections(&root.join("ssh_connections.json")) .map_err(owner_error)? .unwrap_or_default() } else { Vec::new() }; - let workspaces = if files.contains_key("remote_workspace.json") { + let workspaces = if legacy { + read_source_records( + root, + "remote_workspace.json", + true, + &mut omitted, + |v: &RemoteWorkspaceRecord| { + !v.connection_id.trim().is_empty() && v.remote_path.starts_with('/') + }, + ) + } else if files.contains_key("remote_workspace.json") { if legacy { owner::read_legacy_remote_workspaces(&root.join("remote_workspace.json")) } else { @@ -432,7 +481,20 @@ fn read_state(root: &Path, legacy: bool) -> LegacyMigrationResult { } else { Vec::new() }; - let known_hosts = if files.contains_key("known_hosts") { + let known_hosts = if legacy { + read_source_records( + root, + "known_hosts", + false, + &mut omitted, + |v: &KnownHostRecord| { + v.port != 0 + && [&v.host, &v.key_type, &v.fingerprint, &v.public_key] + .iter() + .all(|s| !s.trim().is_empty()) + }, + ) + } else if files.contains_key("known_hosts") { owner::read_known_hosts(&root.join("known_hosts")) .map_err(owner_error)? .unwrap_or_default() @@ -451,6 +513,7 @@ fn read_state(root: &Path, legacy: bool) -> LegacyMigrationResult { VaultHealth::Absent }; Ok(SshState { + omitted, files, connections, workspaces, @@ -462,7 +525,7 @@ fn read_state(root: &Path, legacy: bool) -> LegacyMigrationResult { fn merge_states(source: &SshState, target: &SshState) -> MergePlan { let mut conflicts = Vec::new(); let mut imported = 0u64; - let mut skipped = 0u64; + let mut skipped = source.omitted.len() as u64; let mut connections = Vec::new(); let mut vault_sources = BTreeMap::new(); let mut target_ids = BTreeSet::new(); @@ -892,6 +955,50 @@ fn read_manifest(context: &DomainContext<'_>) -> LegacyMigrationResult MigrationDiagnostic { + MigrationDiagnostic { + severity: FindingSeverity::Warning, + domain: Some(MigrationDomainId::RemoteSsh), + code: "remote_items_skipped".into(), + message: "An unreadable legacy remote record was omitted; other records remain available." + .into(), + relative_path: Some(path.into()), + ..Default::default() + } +} + +fn read_source_records( + root: &Path, + name: &str, + singleton: bool, + omitted: &mut Vec, + valid: impl Fn(&T) -> bool, +) -> Vec { + let path = root.join(name); + if !path.exists() { + return Vec::new(); + } + let values = match read_bounded_json::(root, &path) { + Ok(serde_json::Value::Array(values)) => values, + Ok(value @ serde_json::Value::Object(_)) if singleton => vec![value], + _ => { + omitted.push(name.into()); + return Vec::new(); + } + }; + values + .into_iter() + .enumerate() + .filter_map(|(index, value)| match serde_json::from_value::(value) { + Ok(value) if valid(&value) => Some(value), + _ => { + omitted.push(format!("{name}/{index}")); + None + } + }) + .collect() +} + #[cfg(test)] mod tests { use super::*; @@ -903,6 +1010,24 @@ mod tests { MigrationGroupId, MigrationRunStatus, MigrationSelection, }; + #[test] + fn damaged_ssh_records_and_files_leave_valid_profiles_available() { + let temp = tempfile::tempdir().unwrap(); + let root = temp.path(); + let connection = serde_json::json!({"id":"good","name":"Good","host":"host","port":22,"username":"user","authType":{"type":"Password"}}); + atomic_write_json( + &root.join("ssh_connections.json"), + &serde_json::json!([connection, {"id":1}]), + ) + .unwrap(); + fs::write(root.join("known_hosts"), b"broken JSON").unwrap(); + let source = read_state(root, true).unwrap(); + assert_eq!(source.connections.len(), 1); + assert_eq!(source.omitted.len(), 2); + assert!(read_state(root, false).is_err()); + assert_eq!(fs::read(root.join("known_hosts")).unwrap(), b"broken JSON"); + } + #[test] fn ssh_merge_rewrites_reference_closure_and_keeps_target_host_key() { let temp = test_tempdir("merge"); diff --git a/src/crates/services/legacy-migration-adapters/src/settings.rs b/src/crates/services/legacy-migration-adapters/src/settings.rs index 92957fa121..70fc35a26d 100644 --- a/src/crates/services/legacy-migration-adapters/src/settings.rs +++ b/src/crates/services/legacy-migration-adapters/src/settings.rs @@ -49,6 +49,7 @@ struct MergeOutcome { imported: u64, skipped: u64, conflicts: Vec, + rejected: Vec, } impl LegacyDomainAdapter for SettingsAdapter { @@ -65,8 +66,8 @@ impl LegacyDomainAdapter for SettingsAdapter { Ok(DomainScan { finding: ScanFinding { domain: self.domain(), - code: "legacy_settings_supported".to_string(), - severity: FindingSeverity::Info, + code: if outcome.rejected.is_empty() { "legacy_settings_supported" } else { "settings_items_skipped" }.to_string(), + severity: if outcome.rejected.is_empty() { FindingSeverity::Info } else { FindingSeverity::Warning }, entity_count: outcome.imported + outcome.skipped, logical_bytes: bytes.len() as u64, source_schema: Some(SOURCE_SCHEMA.to_string()), @@ -101,6 +102,19 @@ impl LegacyDomainAdapter for SettingsAdapter { imported: staged.imported, skipped: staged.skipped, conflicts: staged.conflicts, + warnings: outcome + .rejected + .iter() + .map(|path| MigrationDiagnostic { + code: "setting_field_skipped".to_string(), + severity: FindingSeverity::Warning, + domain: Some(self.domain()), + relative_path: Some(path.clone()), + message: "An incompatible setting or model was preserved in the source." + .to_string(), + ..Default::default() + }) + .collect(), ..MigrationDomainResult::default() }) } @@ -227,17 +241,11 @@ impl LegacyDomainAdapter for CredentialsAdapter { LegacyMigrationError::InvalidRequest(format!("credential stage is invalid: {error}")) })?; let source = read_source_config(context.roots)?; - let mut secret_values = Vec::new(); - collect_secret_values(&source, &mut secret_values); - for secret in secret_values.into_iter().filter(|secret| secret.len() >= 4) { - if bytes - .windows(secret.len()) - .any(|window| window == secret.as_bytes()) - { - return Err(LegacyMigrationError::InvalidRequest( - "credential staging contains a prohibited secret value".to_string(), - )); - } + let expected = credential_manifest(&source, manifest.target_existed); + if serde_json::to_value(&manifest).ok() != serde_json::to_value(expected).ok() { + return Err(LegacyMigrationError::InvalidRequest( + "credential manifest differs from source selection".to_string(), + )); } if manifest.model_ids.iter().any(|id| id.trim().is_empty()) { return Err(LegacyMigrationError::InvalidRequest( @@ -265,12 +273,11 @@ impl LegacyDomainAdapter for CredentialsAdapter { if !manifest.model_ids.iter().any(|candidate| candidate == id) { continue; } - let source_model = serde_json::from_value::(source_model.clone()) - .map_err(|error| { - LegacyMigrationError::UnsupportedSource(format!( - "legacy model credential owner record is invalid: {error}" - )) - })?; + let Ok(source_model) = + serde_json::from_value::(source_model.clone()) + else { + continue; + }; if let Some(target_model) = target.ai.models.iter_mut().find(|model| model.id == id) { if target_model.api_key.is_empty() { @@ -329,13 +336,7 @@ impl LegacyDomainAdapter for CredentialsAdapter { .find(|model| model.get("id").and_then(Value::as_str) == Some(id.as_str())) }) .cloned() - .map(serde_json::from_value::) - .transpose() - .map_err(|error| { - LegacyMigrationError::UnsupportedSource(format!( - "legacy model credential owner record is invalid: {error}" - )) - })?; + .and_then(|value| serde_json::from_value::(value).ok()); let target_model = target.ai.models.iter().find(|model| model.id == *id); let Some(source_model) = source_model else { continue; @@ -426,9 +427,13 @@ fn merge_settings( source: &Value, target: GlobalConfig, ) -> LegacyMigrationResult<(GlobalConfig, MergeOutcome)> { - let (mut source_config, mut source_value) = convert_source_config(source)?; + let (mut source_config, mut source_value, rejected) = convert_source_config(source)?; let defaults = GlobalConfig::default(); - let mut outcome = MergeOutcome::default(); + let mut outcome = MergeOutcome { + skipped: rejected.len() as u64, + rejected, + ..Default::default() + }; let source_models = std::mem::take(&mut source_config.ai.models); let mut target_value = serde_json::to_value(&target) .map_err(|error| LegacyMigrationError::InvalidRequest(error.to_string()))?; @@ -457,7 +462,9 @@ fn merge_settings( Ok((merged, outcome)) } -fn convert_source_config(source: &Value) -> LegacyMigrationResult<(GlobalConfig, Value)> { +fn convert_source_config( + source: &Value, +) -> LegacyMigrationResult<(GlobalConfig, Value, Vec)> { validate_source_version(source)?; let defaults = GlobalConfig::default(); let mut normalized_source = source.clone(); @@ -474,52 +481,91 @@ fn convert_source_config(source: &Value) -> LegacyMigrationResult<(GlobalConfig, let mut converted = serde_json::to_value(&defaults) .map_err(|error| LegacyMigrationError::InvalidRequest(error.to_string()))?; - overlay_compatible_value(&mut converted, &normalized_source); - let root = converted.as_object_mut().ok_or_else(|| { - LegacyMigrationError::InvalidRequest( - "legacy configuration root is not an object".to_string(), - ) - })?; - root.insert( - "product_id".to_string(), - Value::String(defaults.product_id.clone()), - ); - root.insert( - "schema_version".to_string(), - Value::from(defaults.schema_version), - ); - root.insert( - "version".to_string(), - Value::String(defaults.version.clone()), - ); - root.insert( - "last_modified".to_string(), - Value::from(chrono::Utc::now().timestamp_millis()), - ); - let config: GlobalConfig = serde_json::from_value(converted).map_err(|error| { - LegacyMigrationError::UnsupportedSource(format!( - "legacy configuration cannot be represented by the current owner model: {error}" - )) - })?; + let mut rejected = Vec::new(); + let accepted = accept_config_patch(&mut converted, "", &normalized_source, &mut rejected) + .unwrap_or_else(|| serde_json::json!({})); + let config: GlobalConfig = serde_json::from_value(converted) + .map_err(|error| LegacyMigrationError::InvalidRequest(error.to_string()))?; let mut compatible_source = serde_json::to_value(&config) .map_err(|error| LegacyMigrationError::InvalidRequest(error.to_string()))?; - retain_source_fields(&mut compatible_source, &normalized_source); - Ok((config, compatible_source)) + retain_source_fields(&mut compatible_source, &accepted); + Ok((config, compatible_source, rejected)) } -fn overlay_compatible_value(target: &mut Value, source: &Value) { - match (target, source) { - (Value::Object(target_fields), Value::Object(source_fields)) => { - for (name, source_value) in source_fields { - if let Some(target_value) = target_fields.get_mut(name) { - overlay_compatible_value(target_value, source_value); - } else { - target_fields.insert(name.clone(), source_value.clone()); - } +// Validate candidate patches against the real owner type. Invalid leaves keep +// the default; arrays are admitted one complete entry at a time. +fn accept_config_patch( + root: &mut Value, + pointer: &str, + source: &Value, + rejected: &mut Vec, +) -> Option { + let before = root.clone(); + let mut candidate = before.clone(); + let mut applied = false; + if pointer.is_empty() { + if let (Some(target), Some(fields)) = (candidate.as_object_mut(), source.as_object()) { + for (key, value) in fields { + target.insert(key.clone(), value.clone()); } + applied = true; + } + } else if let Some((parent, key)) = pointer.rsplit_once('/') { + if let Some(fields) = candidate.pointer_mut(parent).and_then(Value::as_object_mut) { + fields.insert(key.replace("~1", "/").replace("~0", "~"), source.clone()); + applied = true; } - (target, source) => *target = source.clone(), } + if applied + && serde_json::from_value::(candidate.clone()).is_ok() + && (!pointer.is_empty() || source.is_object()) + { + *root = candidate; + return Some(source.clone()); + } + if let Some(fields) = source.as_object() { + let mut accepted = serde_json::Map::new(); + for (key, value) in fields { + let child = format!("{pointer}/{}", key.replace('~', "~0").replace('/', "~1")); + if let Some(value) = accept_config_patch(root, &child, value, rejected) { + accepted.insert(key.clone(), value); + } + } + return Some(Value::Object(accepted)); + } + if let Some(values) = source.as_array() { + let mut accepted = Vec::new(); + let mut empty = root.clone(); + let Some(slot) = empty.pointer_mut(pointer) else { + rejected.push(pointer.to_string()); + return None; + }; + *slot = Value::Array(Vec::new()); + if serde_json::from_value::(empty.clone()).is_err() { + rejected.push(pointer.to_string()); + return None; + } + *root = empty; + for (index, value) in values.iter().enumerate() { + let mut next = accepted.clone(); + next.push(value.clone()); + let mut test = root.clone(); + let Some(slot) = test.pointer_mut(pointer) else { + rejected.push(format!("{pointer}/{index}")); + continue; + }; + *slot = Value::Array(next.clone()); + if serde_json::from_value::(test.clone()).is_ok() { + *root = test; + accepted = next; + } else { + rejected.push(format!("{pointer}/{index}")); + } + } + return Some(Value::Array(accepted)); + } + rejected.push(pointer.to_string()); + None } fn retain_source_fields(value: &mut Value, source: &Value) { @@ -704,7 +750,10 @@ fn credential_manifest(source: &Value, target_existed: bool) -> CredentialManife .and_then(Value::as_array) .into_iter() .flatten() - .filter(|model| model_has_portable_credentials(model)) + .filter(|model| { + model_has_portable_credentials(model) + && serde_json::from_value::((*model).clone()).is_ok() + }) .filter_map(|model| model.get("id").and_then(Value::as_str)) .filter(|id| !id.trim().is_empty()) .map(str::to_string) @@ -721,6 +770,15 @@ fn credential_manifest(source: &Value, target_existed: bool) -> CredentialManife let mut unsupported_secret_fields = Vec::new(); collect_unsupported_secret_fields(source, "", &mut unsupported_secret_fields); unsupported_secret_fields.retain(|path| !is_portable_credential_path(path)); + if let Some(models) = source.pointer("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/ai/models").and_then(Value::as_array) { + for (index, model) in models.iter().enumerate() { + if model_has_portable_credentials(model) + && serde_json::from_value::(model.clone()).is_err() + { + unsupported_secret_fields.push(format!("/ai/models/{index}")); + } + } + } unsupported_secret_fields.sort(); unsupported_secret_fields.dedup(); CredentialManifest { @@ -784,28 +842,6 @@ fn collect_unsupported_secret_fields(value: &Value, path: &str, output: &mut Vec } } -fn collect_secret_values(value: &Value, output: &mut Vec) { - match value { - Value::Object(fields) => { - for (name, value) in fields { - let lower = name.to_ascii_lowercase(); - if matches!(lower.as_str(), "password" | "token" | "secret" | "api_key") { - if let Some(secret) = value.as_str().filter(|secret| !secret.is_empty()) { - output.push(secret.to_string()); - } - } - collect_secret_values(value, output); - } - } - Value::Array(items) => { - for value in items { - collect_secret_values(value, output); - } - } - _ => {} - } -} - fn validate_source_version(source: &Value) -> LegacyMigrationResult<()> { let schema = source.get("schema_version").and_then(Value::as_u64); let version = source.get("version").and_then(Value::as_str); @@ -880,6 +916,28 @@ mod tests { use std::collections::BTreeSet; use std::path::Path; + #[test] + fn malformed_fields_and_models_do_not_discard_valid_siblings() { + let source = serde_json::json!({"schema_version":1,"version":"0.2.19","ai":{ + "subagent_max_concurrency": 3, "stream_idle_timeout_secs":"broken", "enable_deferred_tool_loading": [], + "models":[{"id":"valid","name":"Valid"},{"id":"bad","temperature":[]}] + }}); + let (config, _, rejected) = convert_source_config(&source).unwrap(); + assert_eq!(config.ai.subagent_max_concurrency, 3); + assert_eq!(config.ai.models.len(), 1); + assert_eq!(config.ai.models[0].id, "valid"); + assert!(rejected.iter().any(|p| p == "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/ai/stream_idle_timeout_secs")); + assert!(rejected.iter().any(|p| p == "/ai/models/1")); + let mut root = serde_json::to_value(GlobalConfig::default()).unwrap(); + assert!(accept_config_patch( + &mut root, + "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/missing/field", + &serde_json::json!(true), + &mut Vec::new() + ) + .is_none()); + } + #[test] fn settings_and_credentials_convert_without_staging_secrets() { let temp = test_tempdir("settings-credentials"); diff --git a/src/crates/services/legacy-migration-adapters/src/workspace_sessions.rs b/src/crates/services/legacy-migration-adapters/src/workspace_sessions.rs index eb16a99f18..19a2684f22 100644 --- a/src/crates/services/legacy-migration-adapters/src/workspace_sessions.rs +++ b/src/crates/services/legacy-migration-adapters/src/workspace_sessions.rs @@ -166,8 +166,8 @@ impl LegacyDomainAdapter for WorkspaceSessionsAdapter { Ok(DomainScan { finding: ScanFinding { domain: self.domain(), - code: "legacy_workspace_sessions_supported".to_string(), - severity: if plan.conflicts.is_empty() { + code: if !plan.conflicts.iter().any(|conflict| conflict.resolution == ConflictResolution::ItemSkipped) { "legacy_workspace_sessions_supported" } else { "session_items_skipped" }.to_string(), + severity: if plan.conflicts.is_empty() && plan.skipped_paths.is_empty() { FindingSeverity::Info } else { FindingSeverity::Warning @@ -194,7 +194,7 @@ impl LegacyDomainAdapter for WorkspaceSessionsAdapter { } fn stage(&self, context: &DomainContext<'_>) -> LegacyMigrationResult { - let plan = plan_workspace_sessions(context.roots)?; + let mut plan = plan_workspace_sessions(context.roots)?; let domain_root = stage_domain_dir(context, "workspace-sessions"); atomic_write_json( &domain_root.join("workspace_data.json"), @@ -213,29 +213,49 @@ impl LegacyDomainAdapter for WorkspaceSessionsAdapter { } let runtime = offline_runtime()?; - for session in &plan.sessions { + let mut failed_sessions = BTreeSet::new(); + for session in &mut plan.sessions { if session.action != SessionImportAction::Import { continue; } let sessions_root = domain_root.join("home").join(&session.runtime_relative); let store = OfflineSessionImportStore::new(&sessions_root); - runtime - .block_on(store.write_bundle(&session.bundle)) - .map_err(|error| owner_error("write staged Session", error))?; - let staged_session = sessions_root.join(&session.bundle.metadata.session_id); - for (relative, source) in &session.auxiliary_files { - if relative == Path::new("state.json") { - if let Some(bytes) = &session.state_bytes_override { - atomic_write_bytes(&staged_session.join(relative), bytes)?; - continue; + let staged_result = (|| -> LegacyMigrationResult<()> { + runtime + .block_on(store.write_bundle(&session.bundle)) + .map_err(|error| owner_error("write staged Session", error))?; + let staged_session = sessions_root.join(&session.bundle.metadata.session_id); + for (relative, source) in &session.auxiliary_files { + if relative == Path::new("state.json") { + if let Some(bytes) = &session.state_bytes_override { + atomic_write_bytes(&staged_session.join(relative), bytes)?; + continue; + } } + let source_bytes = fs::read(source).map_err(|error| io_error(source, error))?; + atomic_write_bytes(&staged_session.join(relative), &source_bytes)?; } - let source_bytes = fs::read(source).map_err(|error| io_error(source, error))?; - atomic_write_bytes(&staged_session.join(relative), &source_bytes)?; + require_tree_hash(&staged_session, &session.expected_hash)?; + Ok(()) + })(); + if staged_result.is_err() { + failed_sessions.insert(session.bundle.metadata.session_id.clone()); + plan.conflicts.push(MigrationConflict { + domain: self.domain(), + code: "session_entry_skipped".into(), + source_summary: session.bundle.metadata.session_id.clone(), + target_summary: "Session could not be staged".into(), + resolution: ConflictResolution::ItemSkipped, + }); + plan.skipped_paths + .push(session.bundle.metadata.session_id.clone()); } - require_tree_hash(&staged_session, &session.expected_hash)?; } + plan.sessions + .retain(|session| !failed_sessions.contains(&session.bundle.metadata.session_id)); + plan.runtime_events + .retain(|event| !failed_sessions.contains(&event.session_id)); for event in &plan.runtime_events { if event.action != SessionImportAction::Import { continue; @@ -304,7 +324,8 @@ impl LegacyDomainAdapter for WorkspaceSessionsAdapter { .assistant_workspaces .iter() .filter(|entry| entry.action != SessionImportAction::Import) - .count(); + .count() + + manifest.skipped_paths.len(); let mut warnings = manifest .skipped_paths .iter() @@ -313,11 +334,17 @@ impl LegacyDomainAdapter for WorkspaceSessionsAdapter { severity: FindingSeverity::Info, domain: Some(self.domain()), relative_path: Some(path.clone()), - message: "A non-owned or rebuildable Session path was left in the legacy source" + message: "An unreadable, unsupported, or non-owned Session path was preserved in the legacy source" .to_string(), action: None, }) .collect::>(); + warnings.extend(plan.conflicts.iter().filter(|conflict| conflict.resolution == ConflictResolution::ItemSkipped).map(|conflict| MigrationDiagnostic { + code: conflict.code.clone(), severity: FindingSeverity::Warning, + domain: Some(self.domain()), + message: "An unreadable or unsupported source item was skipped; other Sessions were imported.".to_string(), + ..Default::default() + })); let orphaned_relationships = orphaned_session_relationship_count(&manifest); if orphaned_relationships > 0 { warnings.push(MigrationDiagnostic { @@ -589,21 +616,137 @@ impl LegacyDomainAdapter for WorkspaceSessionsAdapter { } } +fn skipped_workspace(id: &str) -> MigrationConflict { + MigrationConflict { + domain: MigrationDomainId::WorkspaceSessions, + code: "workspace_item_skipped".into(), + source_summary: id.to_string(), + target_summary: "Source workspace retained; other data can continue.".into(), + resolution: ConflictResolution::ItemSkipped, + } +} + +fn repair_workspace_lists(data: &mut WorkspacePersistenceData, miniapps_root: &Path) { + for list in [ + &mut data.opened_workspace_ids, + &mut data.recent_workspaces, + &mut data.recent_assistant_workspaces, + ] { + let mut seen = HashSet::new(); + list.retain(|id| data.workspaces.contains_key(id) && seen.insert(id.clone())); + } + let recent = data + .recent_workspaces + .iter() + .chain(&data.recent_assistant_workspaces) + .cloned() + .collect::>(); + data.recent_workspaces.retain(|id| { + data.workspaces[id].workspace_kind != WorkspaceKind::Assistant + && !data.workspaces[id].root_path.starts_with(miniapps_root) + }); + data.recent_assistant_workspaces + .retain(|id| data.workspaces[id].workspace_kind == WorkspaceKind::Assistant); + for id in recent { + let workspace = &data.workspaces[&id]; + if workspace.workspace_kind == WorkspaceKind::Assistant { + if !data.recent_assistant_workspaces.contains(&id) { + data.recent_assistant_workspaces.push(id); + } + } else if !workspace.root_path.starts_with(miniapps_root) + && !data.recent_workspaces.contains(&id) + { + data.recent_workspaces.push(id); + } + } + if data + .current_workspace_id + .as_ref() + .is_some_and(|id| !data.workspaces.contains_key(id)) + { + data.current_workspace_id = None; + } + if let Some(id) = &data.current_workspace_id { + if !data.opened_workspace_ids.contains(id) { + data.opened_workspace_ids.push(id.clone()); + } + } +} + fn plan_workspace_sessions(roots: &MigrationRoots) -> LegacyMigrationResult { let source_workspace_path = source_workspace_data_path(roots); - let legacy: LegacyWorkspacePersistenceData = - read_bounded_json(&roots.legacy_user_root, &source_workspace_path)?; + let mut conflicts = Vec::new(); + let mut raw: serde_json::Value = + match read_bounded_json(&roots.legacy_user_root, &source_workspace_path) { + Ok(value) => value, + Err(_) => { + conflicts.push(skipped_workspace("registry")); + serde_json::json!({}) + } + }; + let mut workspaces = serde_json::Map::new(); + if let Some(entries) = raw.get("workspaces").and_then(serde_json::Value::as_object) { + for (id, value) in entries { + if serde_json::from_value::(value.clone()).is_ok() { + workspaces.insert(id.clone(), value.clone()); + } else { + conflicts.push(skipped_workspace(id)); + } + } + } + if !raw.is_object() { + raw = serde_json::json!({}); + } + raw["workspaces"] = serde_json::Value::Object(workspaces); + for key in [ + "opened_workspace_ids", + "recent_workspaces", + "recent_assistant_workspaces", + ] { + raw[key] = serde_json::Value::Array( + raw.get(key) + .and_then(serde_json::Value::as_array) + .into_iter() + .flatten() + .filter(|value| value.is_string()) + .cloned() + .collect(), + ); + } + if raw + .get("current_workspace_id") + .is_some_and(|value| !value.is_string()) + { + raw["current_workspace_id"] = serde_json::Value::Null; + } + if serde_json::from_value::>( + raw.get("primary_assistant_key") + .cloned() + .unwrap_or_default(), + ) + .is_err() + { + raw["primary_assistant_key"] = serde_json::Value::Null; + } + if serde_json::from_value::>( + raw.get("saved_at").cloned().unwrap_or_default(), + ) + .is_err() + { + raw["saved_at"] = serde_json::json!(chrono::DateTime::::UNIX_EPOCH); + } + let legacy: LegacyWorkspacePersistenceData = serde_json::from_value(raw).map_err(json_error)?; let target_workspace_path = target_workspace_data_path(roots); - let target = read_optional_bounded_json::( + let mut target = read_optional_bounded_json::( &roots.target_user_root, &target_workspace_path, )?; - if let Some(target) = &target { + if let Some(target) = &mut target { + repair_workspace_lists(target, &roots.target_user_root.join("data/miniapps")); validate_workspace_persistence_data(target, &roots.target_user_root.join("data/miniapps")) .map_err(|error| owner_error("read current Workspace registry", error))?; } - let mut conflicts = Vec::new(); let assistant_workspaces = plan_assistant_workspaces(roots, &mut conflicts)?; let mut assistant_path_relocations = BTreeMap::new(); for workspace in &assistant_workspaces { @@ -622,26 +765,32 @@ fn plan_workspace_sessions(roots: &MigrationRoots) -> LegacyMigrationResult LegacyMigrationResult<()> { + let mut workspace = legacy.workspaces[&source_id].clone(); + let relocated_assistant = (workspace.workspace_kind == WorkspaceKind::Assistant) + .then(|| assistant_path_relocations.get(&native_path_key(&workspace.root_path))) + .flatten() + .cloned(); + if let Some(target_path) = &relocated_assistant { + workspace.root_path = target_path.clone(); + } + normalize_legacy_workspace_for_current(&mut workspace)?; + let target_id = current_workspace_storage_id(&workspace) + .map_err(|error| owner_error("convert legacy Workspace id", error))?; + workspace.id = target_id.clone(); + if workspace.workspace_kind != WorkspaceKind::Remote + && relocated_assistant.is_none() + && !workspace.root_path.exists() + { + requires_relocation.push(target_id.clone()); + } + workspace_id_map.insert(source_id.clone(), target_id.clone()); + converted.push((target_id, workspace)); + Ok(()) + })(); + if inspect.is_err() { + conflicts.push(skipped_workspace(&source_id)); } - workspace_id_map.insert(source_id, target_id.clone()); - converted.push((target_id, workspace)); } let mut output_workspaces = target @@ -701,7 +850,7 @@ fn plan_workspace_sessions(roots: &MigrationRoots) -> LegacyMigrationResult LegacyMigrationResult LegacyMigrationResult(); - let workspace_bytes = fs::metadata(&source_workspace_path) - .map_err(|error| io_error(&source_workspace_path, error))? - .len(); + let workspace_bytes = fs::metadata(&source_workspace_path).map_or(0, |metadata| metadata.len()); let target_workspace_existed = target_workspace_path.exists(); let target_workspace_hash = target_workspace_existed .then(|| hash_file(&target_workspace_path)) @@ -811,7 +962,16 @@ fn plan_assistant_workspaces( let mut planned = Vec::new(); for source_path in child_directories(&source_root)? { let relative_path = PathBuf::from(file_name(&source_path)?); - let (expected_hash, logical_bytes) = hash_tree_with_size(&source_path)?; + let (expected_hash, logical_bytes) = match hash_tree_with_size(&source_path) { + Ok(value) => value, + Err(_) => { + conflicts.push(skipped_workspace(&relative_display( + &roots.legacy_home_root, + &source_path, + ))); + continue; + } + }; let target_path = target_root.join(&relative_path); let action = if !target_path.exists() { SessionImportAction::Import @@ -973,130 +1133,170 @@ fn plan_sessions( .map_err(|_| LegacyMigrationError::PathEscape(sessions_root.clone()))? .to_path_buf(); for session_dir in child_directories(&sessions_root)? { - let session_id = file_name(&session_dir)?; - validate_session_id(&session_id).map_err(|error| { - LegacyMigrationError::UnsupportedSource(format!( - "legacy Session id is unsafe: {error}" - )) - })?; - let metadata_path = session_dir.join("metadata.json"); - match fs::symlink_metadata(&metadata_path) { - Ok(_) => {} - Err(error) if error.kind() == std::io::ErrorKind::NotFound => { - skipped_paths.push(relative_display(&roots.legacy_home_root, &session_dir)); - continue; + let inspect = (|| -> LegacyMigrationResult<()> { + let session_id = file_name(&session_dir)?; + validate_session_id(&session_id).map_err(|error| { + LegacyMigrationError::UnsupportedSource(format!( + "legacy Session id is unsafe: {error}" + )) + })?; + let metadata_path = session_dir.join("metadata.json"); + match fs::symlink_metadata(&metadata_path) { + Ok(_) => {} + Err(error) if error.kind() == std::io::ErrorKind::NotFound => { + skipped_paths.push(relative_display(&roots.legacy_home_root, &session_dir)); + return Ok(()); + } + Err(error) => return Err(io_error(&metadata_path, error)), } - Err(error) => return Err(io_error(&metadata_path, error)), - } - let metadata_file: StoredSessionMetadataFile = - read_bounded_json(&roots.legacy_home_root, &metadata_path)?; - if metadata_file.schema_version > SESSION_STORAGE_SCHEMA_VERSION { - return Err(LegacyMigrationError::UnsupportedSource(format!( - "Session {session_id} uses unsupported schema {}", - metadata_file.schema_version - ))); - } - if metadata_file.metadata.session_id != session_id { - return Err(LegacyMigrationError::UnsupportedSource(format!( - "Session directory id does not match its metadata: {session_id}" - ))); - } - let turns = read_session_turns(&roots.legacy_home_root, &session_dir, &session_id)?; - let mut session_metadata = metadata_file.metadata; - let relocated_assistant_path = relocate_assistant_session_metadata( - &mut session_metadata, - assistant_path_relocations, - ); - let runtime_relative = relocated_assistant_path - .clone() - .map(assistant_session_runtime_relative) - .unwrap_or_else(|| source_runtime_relative.clone()); - let bundle = OfflineSessionBundle { - metadata: session_metadata, - turns, - }; - bundle - .validate() - .map_err(|error| owner_error("validate legacy Session", error))?; - let auxiliary_files = collect_auxiliary_session_files( - &roots.legacy_home_root, - &session_dir, - &mut skipped_paths, - )?; - let state_bytes_override = if relocated_assistant_path.is_some() { - relocate_assistant_session_state( - &roots.legacy_home_root, - &session_dir.join("state.json"), - assistant_path_relocations, - )? - } else { - None - }; - let expected_hash = - expected_session_hash(&bundle, &auxiliary_files, state_bytes_override.as_deref())?; - if auxiliary_files - .len() - .saturating_add(bundle.turns.len()) - .saturating_add(1) - > MAX_SESSION_FILES - { - return Err(LegacyMigrationError::ResourceLimit(format!( - "Session contains more than {MAX_SESSION_FILES} files: {}", - session_dir.display() - ))); - } - if expected_bundle_bytes(&bundle, &auxiliary_files, state_bytes_override.as_deref())? - > MAX_SESSION_BYTES - { - return Err(LegacyMigrationError::ResourceLimit(format!( - "Session exceeds {MAX_SESSION_BYTES} bytes: {}", - session_dir.display() - ))); - } - if let Some(previous_hash) = - source_ids.insert(session_id.clone(), expected_hash.clone()) - { - if previous_hash != expected_hash { + let metadata_file: StoredSessionMetadataFile = + read_bounded_json(&roots.legacy_home_root, &metadata_path)?; + if metadata_file.schema_version > SESSION_STORAGE_SCHEMA_VERSION { return Err(LegacyMigrationError::UnsupportedSource(format!( - "legacy Session id appears with different contents: {session_id}" + "Session {session_id} uses unsupported schema {}", + metadata_file.schema_version ))); } - skipped_paths.push(relative_display(&roots.legacy_home_root, &session_dir)); - continue; - } - - let target_same_path = roots - .target_home_root - .join(&runtime_relative) - .join(&session_id); - let action = match target_sessions.get(&session_id) { - None => SessionImportAction::Import, - Some(paths) - if paths.len() == 1 - && paths[0] == target_same_path - && hash_tree(&target_same_path)? == expected_hash => - { - SessionImportAction::Duplicate + if metadata_file.metadata.session_id != session_id { + return Err(LegacyMigrationError::UnsupportedSource(format!( + "Session directory id does not match its metadata: {session_id}" + ))); } - Some(_) => { - conflicts.push(MigrationConflict { - domain: MigrationDomainId::WorkspaceSessions, - code: "session_target_wins".to_string(), - source_summary: format!("legacy Session {session_id}"), - target_summary: format!("current Session {session_id}"), - resolution: ConflictResolution::TargetWins, + let (turns, turn_issues) = + read_session_turns(&roots.legacy_home_root, &session_dir, &session_id)?; + skipped_paths.extend(turn_issues.iter().cloned()); + if !turn_issues.is_empty() { + conflicts.push(MigrationConflict { domain: MigrationDomainId::WorkspaceSessions, code: "session_turns_recovered".into(), source_summary: session_id.clone(), target_summary: "Only readable, unambiguous Turns will be imported; inspect the original Session for omitted content.".into(), resolution: ConflictResolution::ItemSkipped }); + } + let mut session_metadata = metadata_file.metadata; + // Legacy metadata counters can lag persisted Turns. Rebuild the + // derived count in the import copy without changing the source. + session_metadata.turn_count = turns.len(); + let relocated_assistant_path = relocate_assistant_session_metadata( + &mut session_metadata, + assistant_path_relocations, + ); + let runtime_relative = relocated_assistant_path + .clone() + .map(assistant_session_runtime_relative) + .unwrap_or_else(|| source_runtime_relative.clone()); + let bundle = OfflineSessionBundle { + metadata: session_metadata, + turns, + }; + bundle + .validate() + .map_err(|error| owner_error("validate legacy Session", error))?; + let mut auxiliary_files = collect_auxiliary_session_files( + &roots.legacy_home_root, + &session_dir, + &mut skipped_paths, + )?; + if !turn_issues.is_empty() { + auxiliary_files.retain(|(relative, _)| { + !matches!( + relative.to_str(), + Some("turn-catalog.json" | "token-anchors.json") + ) }); - SessionImportAction::TargetWins } - }; - sessions.push(PlannedSession { - runtime_relative: runtime_relative.clone(), - bundle, - auxiliary_files, - state_bytes_override, - action, - expected_hash, - }); + let state_bytes_override = if relocated_assistant_path.is_some() + && auxiliary_files + .iter() + .any(|(relative, _)| relative == Path::new("state.json")) + { + relocate_assistant_session_state( + &roots.legacy_home_root, + &session_dir.join("state.json"), + assistant_path_relocations, + )? + } else { + None + }; + let expected_hash = expected_session_hash( + &bundle, + &auxiliary_files, + state_bytes_override.as_deref(), + )?; + if auxiliary_files + .len() + .saturating_add(bundle.turns.len()) + .saturating_add(1) + > MAX_SESSION_FILES + { + return Err(LegacyMigrationError::ResourceLimit(format!( + "Session contains more than {MAX_SESSION_FILES} files: {}", + session_dir.display() + ))); + } + if expected_bundle_bytes( + &bundle, + &auxiliary_files, + state_bytes_override.as_deref(), + )? > MAX_SESSION_BYTES + { + return Err(LegacyMigrationError::ResourceLimit(format!( + "Session exceeds {MAX_SESSION_BYTES} bytes: {}", + session_dir.display() + ))); + } + if let Some(previous_hash) = source_ids.get(&session_id) { + if previous_hash != &expected_hash { + return Err(LegacyMigrationError::UnsupportedSource(format!( + "legacy Session id appears with different contents: {session_id}" + ))); + } + skipped_paths.push(relative_display(&roots.legacy_home_root, &session_dir)); + return Ok(()); + } + + let target_same_path = roots + .target_home_root + .join(&runtime_relative) + .join(&session_id); + let action = match target_sessions.get(&session_id) { + None => SessionImportAction::Import, + Some(paths) + if paths.len() == 1 + && paths[0] == target_same_path + && hash_tree(&target_same_path)? == expected_hash => + { + SessionImportAction::Duplicate + } + Some(_) => { + conflicts.push(MigrationConflict { + domain: MigrationDomainId::WorkspaceSessions, + code: "session_target_wins".to_string(), + source_summary: format!("legacy Session {session_id}"), + target_summary: format!("current Session {session_id}"), + resolution: ConflictResolution::TargetWins, + }); + SessionImportAction::TargetWins + } + }; + source_ids.insert(session_id.clone(), expected_hash.clone()); + sessions.push(PlannedSession { + runtime_relative: runtime_relative.clone(), + bundle, + auxiliary_files, + state_bytes_override, + action, + expected_hash, + }); + Ok(()) + })(); + if let Err(error) = inspect { + skipped_paths.push(relative_display(&roots.legacy_home_root, &session_dir)); + conflicts.push(MigrationConflict { + domain: MigrationDomainId::WorkspaceSessions, + code: "session_source_skipped".to_string(), + source_summary: error.to_string(), + target_summary: + "The original Session is preserved; other Sessions can be imported." + .to_string(), + resolution: ConflictResolution::ItemSkipped, + }); + } } } sessions.sort_by(|left, right| { @@ -1124,65 +1324,79 @@ fn plan_runtime_events( let mut planned = Vec::new(); for entry in read_dir_sorted(&source_root)? { let path = entry.path(); - let metadata = fs::symlink_metadata(&path).map_err(|error| io_error(&path, error))?; - if metadata.file_type().is_symlink() || is_reparse_point(&metadata) { - return Err(LegacyMigrationError::LinkedPath(path)); - } - if !metadata.is_file() || path.extension().and_then(|value| value.to_str()) != Some("jsonl") - { - skipped_paths.push(relative_display(&roots.legacy_home_root, &path)); - continue; - } - if metadata.len() > MAX_RUNTIME_EVENT_BYTES { - return Err(LegacyMigrationError::ResourceLimit(format!( - "runtime event log exceeds {MAX_RUNTIME_EVENT_BYTES} bytes: {}", - relative_display(&roots.legacy_home_root, &path) - ))); - } - validate_regular_file(&roots.legacy_home_root, &path)?; - let session_id = path - .file_stem() - .and_then(|value| value.to_str()) - .ok_or_else(|| { - LegacyMigrationError::UnsupportedSource( - "runtime event log name is not valid UTF-8".to_string(), - ) - })? - .to_string(); - let Some(session_action) = known_sessions.get(session_id.as_str()).copied() else { + let inspect = (|| -> LegacyMigrationResult<()> { + let metadata = fs::symlink_metadata(&path).map_err(|error| io_error(&path, error))?; + if metadata.file_type().is_symlink() || is_reparse_point(&metadata) { + return Err(LegacyMigrationError::LinkedPath(path.clone())); + } + if !metadata.is_file() + || path.extension().and_then(|value| value.to_str()) != Some("jsonl") + { + skipped_paths.push(relative_display(&roots.legacy_home_root, &path)); + return Ok(()); + } + if metadata.len() > MAX_RUNTIME_EVENT_BYTES { + return Err(LegacyMigrationError::ResourceLimit(format!( + "runtime event log exceeds {MAX_RUNTIME_EVENT_BYTES} bytes: {}", + relative_display(&roots.legacy_home_root, &path) + ))); + } + validate_regular_file(&roots.legacy_home_root, &path)?; + let session_id = path + .file_stem() + .and_then(|value| value.to_str()) + .ok_or_else(|| { + LegacyMigrationError::UnsupportedSource( + "runtime event log name is not valid UTF-8".to_string(), + ) + })? + .to_string(); + let Some(session_action) = known_sessions.get(session_id.as_str()).copied() else { + skipped_paths.push(relative_display(&roots.legacy_home_root, &path)); + return Ok(()); + }; + let summary = validate_runtime_event_log(&path, &session_id) + .map_err(|error| owner_error("read legacy runtime event log", error))?; + let expected_hash = hash_file(&path)?; + let target = roots + .target_home_root + .join("runtime-events") + .join(format!("{session_id}.jsonl")); + let action = if session_action == SessionImportAction::TargetWins { + SessionImportAction::TargetWins + } else if !target.exists() { + SessionImportAction::Import + } else if hash_file(&target)? == expected_hash { + SessionImportAction::Duplicate + } else { + conflicts.push(MigrationConflict { + domain: MigrationDomainId::WorkspaceSessions, + code: "runtime_event_target_wins".to_string(), + source_summary: format!("legacy runtime event log for {session_id}"), + target_summary: format!("current runtime event log for {session_id}"), + resolution: ConflictResolution::TargetWins, + }); + SessionImportAction::TargetWins + }; + planned.push(PlannedRuntimeEvent { + session_id, + source_path: path.clone(), + action, + expected_hash, + turn_ids: summary.turn_ids, + }); + Ok(()) + })(); + if let Err(error) = inspect { skipped_paths.push(relative_display(&roots.legacy_home_root, &path)); - continue; - }; - let summary = validate_runtime_event_log(&path, &session_id) - .map_err(|error| owner_error("read legacy runtime event log", error))?; - let expected_hash = hash_file(&path)?; - let target = roots - .target_home_root - .join("runtime-events") - .join(format!("{session_id}.jsonl")); - let action = if session_action == SessionImportAction::TargetWins { - SessionImportAction::TargetWins - } else if !target.exists() { - SessionImportAction::Import - } else if hash_file(&target)? == expected_hash { - SessionImportAction::Duplicate - } else { conflicts.push(MigrationConflict { domain: MigrationDomainId::WorkspaceSessions, - code: "runtime_event_target_wins".to_string(), - source_summary: format!("legacy runtime event log for {session_id}"), - target_summary: format!("current runtime event log for {session_id}"), - resolution: ConflictResolution::TargetWins, + code: "session_event_log_skipped".to_string(), + source_summary: error.to_string(), + target_summary: "The original event log is preserved.".to_string(), + resolution: ConflictResolution::ItemSkipped, }); - SessionImportAction::TargetWins - }; - planned.push(PlannedRuntimeEvent { - session_id, - source_path: path, - action, - expected_hash, - turn_ids: summary.turn_ids, - }); + } } planned.sort_by(|left, right| left.session_id.cmp(&right.session_id)); Ok(planned) @@ -1192,42 +1406,74 @@ fn read_session_turns( legacy_home_root: &Path, session_dir: &Path, session_id: &str, -) -> LegacyMigrationResult> { +) -> LegacyMigrationResult<( + Vec, + Vec, +)> { let turns_dir = session_dir.join("turns"); if !turns_dir.exists() { - return Ok(Vec::new()); + return Ok((Vec::new(), Vec::new())); } reject_linked_directory(&turns_dir)?; let mut turns = Vec::new(); + let mut issues = Vec::new(); for entry in read_dir_sorted(&turns_dir)? { let path = entry.path(); - validate_regular_file(legacy_home_root, &path)?; - let file_name = file_name(&path)?; - let file_index = file_name - .strip_prefix("turn-") - .and_then(|value| value.strip_suffix(".json")) - .and_then(|value| value.parse::().ok()) - .ok_or_else(|| { - LegacyMigrationError::UnsupportedSource(format!( - "unsupported legacy Turn filename: {file_name}" - )) - })?; - let stored: StoredDialogTurnFile = read_bounded_json(legacy_home_root, &path)?; - if stored.schema_version > SESSION_STORAGE_SCHEMA_VERSION { - return Err(LegacyMigrationError::UnsupportedSource(format!( - "Session {session_id} Turn uses unsupported schema {}", - stored.schema_version - ))); - } - if stored.turn.session_id != session_id || stored.turn.turn_index != file_index { - return Err(LegacyMigrationError::UnsupportedSource(format!( - "legacy Turn identity does not match its storage path in Session {session_id}" - ))); + let inspect = (|| -> LegacyMigrationResult<_> { + let stored: StoredDialogTurnFile = read_bounded_json(legacy_home_root, &path)?; + if stored.schema_version > SESSION_STORAGE_SCHEMA_VERSION + || stored.turn.session_id != session_id + { + return Err(LegacyMigrationError::UnsupportedSource( + "unsupported Turn schema or Session identity".into(), + )); + } + Ok(stored.turn) + })(); + match inspect { + Ok(turn) => { + let expected = format!("turn-{:04}.json", turn.turn_index); + if path.file_name().and_then(|name| name.to_str()) != Some(expected.as_str()) { + issues.push(relative_display(legacy_home_root, &path)); + } + turns.push(turn); + } + Err(_) => issues.push(relative_display(legacy_home_root, &path)), } - turns.push(stored.turn); } + let mut seen_payloads = BTreeSet::new(); + turns.retain(|turn| { + let unique = seen_payloads + .insert(serde_json::to_vec(turn).expect("Turn serialization is infallible")); + if !unique { + issues.push(format!( + "{}/turns/duplicate", + relative_display(legacy_home_root, session_dir) + )); + } + unique + }); + // Conflicting identities cannot safely choose a winner. Preserve every + // original and exclude the ambiguous group from this import. + let mut ids = HashMap::new(); + let mut indices = HashMap::new(); + for turn in &turns { + *ids.entry(turn.turn_id.clone()).or_insert(0usize) += 1; + *indices.entry(turn.turn_index).or_insert(0usize) += 1; + } + turns.retain(|turn| { + let keep = ids[&turn.turn_id] == 1 && indices[&turn.turn_index] == 1; + if !keep { + issues.push(format!( + "{}/turns/turn-{:04}.json", + relative_display(legacy_home_root, session_dir), + turn.turn_index + )); + } + keep + }); turns.sort_by_key(|turn| turn.turn_index); - Ok(turns) + Ok((turns, issues)) } fn collect_auxiliary_session_files( @@ -1241,7 +1487,8 @@ fn collect_auxiliary_session_files( let name = file_name(&path)?; let metadata = fs::symlink_metadata(&path).map_err(|error| io_error(&path, error))?; if metadata.file_type().is_symlink() || is_reparse_point(&metadata) { - return Err(LegacyMigrationError::LinkedPath(path)); + skipped_paths.push(relative_display(legacy_home_root, &path)); + continue; } if metadata.is_file() { if name == "metadata.json" { @@ -1251,7 +1498,9 @@ fn collect_auxiliary_session_files( continue; } if SESSION_ROOT_FILES.contains(&name.as_str()) { - validate_owned_file(legacy_home_root, session_dir, &path, &mut files)?; + if validate_owned_file(legacy_home_root, session_dir, &path, &mut files).is_err() { + skipped_paths.push(relative_display(legacy_home_root, &path)); + } } else { skipped_paths.push(relative_display(legacy_home_root, &path)); } @@ -1260,7 +1509,11 @@ fn collect_auxiliary_session_files( continue; } if SESSION_OWNED_DIRECTORIES.contains(&name.as_str()) { - collect_owned_directory(legacy_home_root, session_dir, &path, 0, &mut files)?; + if collect_owned_directory(legacy_home_root, session_dir, &path, 0, &mut files) + .is_err() + { + skipped_paths.push(relative_display(legacy_home_root, &path)); + } } else { skipped_paths.push(relative_display(legacy_home_root, &path)); } @@ -1913,6 +2166,41 @@ mod tests { use super::*; use openbitfun_core_types::SessionExecutionTarget; + #[test] + fn damaged_turns_and_filename_mismatches_recover_valid_history() { + let temp = test_tempdir("partial-turns"); + let root = temp.path(); + let session = root.join("session-1"); + fs::create_dir_all(session.join("turns")).unwrap(); + let fixture = Path::new(env!("CARGO_MANIFEST_DIR")).join("../legacy-migration/tests/fixtures/v0.2.19/home/projects/c--fixture-workspace/sessions/session-1/turns/turn-0000.json"); + let bytes = fs::read(fixture).unwrap(); + fs::write(session.join("turns/old-name.json"), &bytes).unwrap(); + fs::write(session.join("turns/turn-0001.json"), b"broken JSON").unwrap(); + let (turns, issues) = read_session_turns(root, &session, "session-1").unwrap(); + assert_eq!(turns.len(), 1); + assert_eq!(issues.len(), 2); + fs::write(session.join("turns/duplicate.json"), &bytes).unwrap(); + assert_eq!( + read_session_turns(root, &session, "session-1") + .unwrap() + .0 + .len(), + 1 + ); + let mut conflicting: StoredDialogTurnFile = serde_json::from_slice(&bytes).unwrap(); + conflicting.turn.turn_id = "conflicting-id".into(); + atomic_write_json(&session.join("turns/conflict.json"), &conflicting).unwrap(); + assert!(read_session_turns(root, &session, "session-1") + .unwrap() + .0 + .is_empty()); + + assert_eq!( + fs::read(session.join("turns/old-name.json")).unwrap(), + bytes + ); + } + #[test] fn remote_workspace_paths_are_normalized_with_posix_semantics() { let mut workspace: WorkspaceInfo = serde_json::from_value(serde_json::json!({ @@ -2135,6 +2423,86 @@ mod tests { ); } + #[test] + fn damaged_session_is_skipped_and_stale_turn_count_is_rebuilt_on_import() { + use crate::adapters_for_groups; + use openbitfun_legacy_migration::{ + probe_legacy_source, CancellationToken, MigrationEngine, NoCrashInjection, ProbeLimits, + }; + use openbitfun_product_domains::legacy_migration::{ + MigrationGroupId, MigrationRunStatus, MigrationSelection, + }; + let temp = test_tempdir("partial-sessions"); + let roots = fixture_roots(temp.path()); + let fixture = PathBuf::from(env!("CARGO_MANIFEST_DIR")) + .join("../legacy-migration/tests/fixtures/v0.2.19"); + copy_tree(&fixture.join("user-root"), &roots.legacy_user_root).unwrap(); + copy_tree(&fixture.join("home"), &roots.legacy_home_root).unwrap(); + let relative = "projects/c--fixture-workspace/sessions/session-1/metadata.json"; + let metadata_path = roots.legacy_home_root.join(relative); + let mut metadata: StoredSessionMetadataFile = + serde_json::from_slice(&fs::read(&metadata_path).unwrap()).unwrap(); + metadata.metadata.turn_count = 0; + atomic_write_json(&metadata_path, &metadata).unwrap(); + let original = fs::read(&metadata_path).unwrap(); + let damaged = roots + .legacy_home_root + .join("projects/c--fixture-workspace/sessions/broken/metadata.json"); + atomic_write_bytes(&damaged, b"invalid JSON").unwrap(); + let workspace_path = source_workspace_data_path(&roots); + let mut registry: serde_json::Value = + serde_json::from_slice(&fs::read(&workspace_path).unwrap()).unwrap(); + registry["workspaces"]["broken"] = serde_json::json!({"rootPath": 42}); + registry["opened_workspace_ids"] = + serde_json::json!(["workspace-1", "workspace-1", "missing", 7]); + atomic_write_json(&workspace_path, ®istry).unwrap(); + let broken_turn = metadata_path.parent().unwrap().join("turns/turn-0001.json"); + fs::write(&broken_turn, b"invalid Turn").unwrap(); + let selection = MigrationSelection { + groups: BTreeSet::from([MigrationGroupId::WorkspacesSessionsAndTasks]), + }; + let source = probe_legacy_source(&roots, ProbeLimits::default()) + .unwrap() + .unwrap(); + let engine = MigrationEngine::new(roots.clone(), adapters_for_groups(&selection)).unwrap(); + let plan = engine + .plan(&source, selection, &CancellationToken::default()) + .unwrap(); + assert!(plan.findings.iter().any(|finding| finding.domain + == MigrationDomainId::WorkspaceSessions + && finding.migratable + && finding.code == "session_items_skipped")); + let report = engine + .execute(&plan, &CancellationToken::default(), &NoCrashInjection) + .unwrap(); + assert_eq!(report.status, MigrationRunStatus::CompletedWithWarnings); + let imported: StoredSessionMetadataFile = + serde_json::from_slice(&fs::read(roots.target_home_root.join(relative)).unwrap()) + .unwrap(); + assert_eq!(imported.metadata.turn_count, 1); + assert!(report + .domain_results + .iter() + .flat_map(|result| &result.warnings) + .any(|warning| warning.code == "session_turns_recovered")); + assert_eq!(fs::read(&broken_turn).unwrap(), b"invalid Turn"); + assert!(!roots + .target_home_root + .join("projects/c--fixture-workspace/sessions/session-1/turns/turn-0001.json") + .exists()); + + assert_eq!(fs::read(metadata_path).unwrap(), original); + assert_eq!(fs::read(damaged).unwrap(), b"invalid JSON"); + assert!(!roots + .target_home_root + .join("projects/c--fixture-workspace/sessions/broken") + .exists()); + assert!(report.domain_results.iter().any(|result| result.domain + == MigrationDomainId::WorkspaceSessions + && result.skipped > 0 + && !result.warnings.is_empty())); + } + fn fixture_roots(root: &Path) -> MigrationRoots { let legacy_user_root = root.join("legacy-user"); MigrationRoots { diff --git a/src/crates/services/legacy-migration/src/engine.rs b/src/crates/services/legacy-migration/src/engine.rs index 81d5a61ff9..02d7e35ed4 100644 --- a/src/crates/services/legacy-migration/src/engine.rs +++ b/src/crates/services/legacy-migration/src/engine.rs @@ -158,9 +158,25 @@ impl MigrationEngine { for domain in selection.expanded_domains() { cancellation.check()?; let adapter = self.adapter(domain)?; - let scan = adapter - .scan(&self.roots) - .map_err(|error| domain_error(domain, error))?; + let scan = match adapter.scan(&self.roots) { + Ok(scan) => scan, + Err(LegacyMigrationError::Cancelled) => { + return Err(LegacyMigrationError::Cancelled) + } + Err(error) => DomainScan { + finding: openbitfun_product_domains::legacy_migration::ScanFinding { + domain, + code: "domain_scan_skipped".to_string(), + severity: FindingSeverity::Warning, + migratable: false, + detail: error.to_string(), + ..Default::default() + }, + conflicts: Vec::new(), + target_schema: None, + dependencies: Vec::new(), + }, + }; if scan.finding.domain != domain { return Err(LegacyMigrationError::InvalidPlan(format!( "adapter {domain:?} returned a finding for {:?}", @@ -169,6 +185,32 @@ impl MigrationEngine { } scans.push(scan); } + // Dependencies may appear later in an adapter's scan results. Propagate + // unavailable domains to a fixed point before authorizing any writes. + loop { + let unavailable = scans + .iter() + .filter(|scan| !scan.finding.migratable) + .map(|scan| scan.finding.domain) + .collect::>(); + let mut changed = false; + for scan in &mut scans { + if scan.finding.migratable + && scan.dependencies.iter().any(|id| unavailable.contains(id)) + { + scan.finding.migratable = false; + scan.finding.severity = FindingSeverity::Warning; + scan.finding.code = "domain_dependency_skipped".to_string(); + scan.finding.detail = + "A required domain could not be scanned; this domain will be skipped." + .to_string(); + changed = true; + } + } + if !changed { + break; + } + } Ok(scans) } @@ -218,12 +260,16 @@ impl MigrationEngine { ))); } } - estimated_write_bytes = - estimated_write_bytes.saturating_add(scan.estimated_write_bytes()); + let write_bytes = if scan.finding.migratable { + scan.estimated_write_bytes() + } else { + 0 + }; + estimated_write_bytes = estimated_write_bytes.saturating_add(write_bytes); steps.push(MigrationPlanStep { sequence: u32::try_from(index + 1).unwrap_or(u32::MAX), domain: scan.finding.domain, - estimated_write_bytes: scan.estimated_write_bytes(), + estimated_write_bytes: write_bytes, source_schema: scan.finding.source_schema.clone(), target_schema: scan.target_schema, dependencies: scan.dependencies, @@ -321,6 +367,7 @@ impl MigrationEngine { "lock_acquired", )?; + let mut last_failure = None; for (index, step) in plan.steps.iter().enumerate() { let result_index = report .domain_results @@ -330,6 +377,38 @@ impl MigrationEngine { if report.domain_results[result_index].state == MigrationDomainState::Verified { continue; } + let unavailable_dependency = step.dependencies.iter().any(|dependency| { + !report.domain_results.iter().any(|result| { + result.domain == *dependency && result.state == MigrationDomainState::Verified + }) + }); + let skipped_finding = plan + .findings + .iter() + .find(|finding| finding.domain == step.domain && !finding.migratable); + if skipped_finding.is_some() || unavailable_dependency { + let code = skipped_finding + .map(|finding| finding.code.as_str()) + .unwrap_or("domain_dependency_skipped"); + let result = &mut report.domain_results[result_index]; + result.state = MigrationDomainState::Skipped; + if !result.warnings.iter().any(|warning| warning.code == code) { + result.warnings.push(MigrationDiagnostic { + code: code.to_string(), + severity: FindingSeverity::Warning, + domain: Some(step.domain), + message: "This domain was skipped; its source data remains available." + .to_string(), + ..Default::default() + }); + } + persist_report(&layout, &report)?; + continue; + } + // A dependency skipped on a previous attempt may now be available. + if report.domain_results[result_index].state == MigrationDomainState::Skipped { + report.domain_results[result_index].state = MigrationDomainState::NotStarted; + } let context = DomainContext { roots: &self.roots, layout: &layout, @@ -379,8 +458,15 @@ impl MigrationEngine { MigrationPhase::Stage, &error, )?; - let _ = adapter.rollback_unverified(&context); - return Err(domain_error(step.domain, error)); + if adapter.rollback_unverified(&context).is_err() { + return Err(LegacyMigrationError::Domain { + domain: step.domain, + message: error.to_string(), + }); + } + cancellation.check()?; + last_failure = Some((step.domain, MigrationPhase::Stage, error)); + continue; } }; if staged.domain != step.domain { @@ -438,8 +524,15 @@ impl MigrationEngine { MigrationPhase::ValidateStage, &error, )?; - let _ = adapter.rollback_unverified(&context); - return Err(domain_error(step.domain, error)); + if adapter.rollback_unverified(&context).is_err() { + return Err(LegacyMigrationError::Domain { + domain: step.domain, + message: error.to_string(), + }); + } + cancellation.check()?; + last_failure = Some((step.domain, MigrationPhase::ValidateStage, error)); + continue; } journal( &layout, @@ -488,8 +581,15 @@ impl MigrationEngine { MigrationPhase::Commit, &error, )?; - let _ = adapter.rollback_unverified(&context); - return Err(domain_error(step.domain, error)); + if adapter.rollback_unverified(&context).is_err() { + return Err(LegacyMigrationError::Domain { + domain: step.domain, + message: error.to_string(), + }); + } + cancellation.check()?; + last_failure = Some((step.domain, MigrationPhase::Commit, error)); + continue; } inject(crash_injector, CrashPoint::AfterCommit(step.domain))?; report.domain_results[result_index].state = MigrationDomainState::Committed; @@ -535,26 +635,41 @@ impl MigrationEngine { MigrationPhase::ValidateCommit, &error, )?; - let _ = adapter.rollback_unverified(&context); - return Err(domain_error(step.domain, error)); + if adapter.rollback_unverified(&context).is_err() { + return Err(LegacyMigrationError::Domain { + domain: step.domain, + message: error.to_string(), + }); + } + cancellation.check()?; + last_failure = Some((step.domain, MigrationPhase::ValidateCommit, error)); + continue; } - let mut finalized = - match adapter.finalize_result(&context, &report.domain_results[result_index]) { - Ok(finalized) => finalized, - Err(error) => { - record_domain_failure( - &layout, - &mut report, - &mut journal_sequence, - result_index, - step.domain, - MigrationPhase::ValidateCommit, - &error, - )?; - let _ = adapter.rollback_unverified(&context); - return Err(domain_error(step.domain, error)); + let mut finalized = match adapter + .finalize_result(&context, &report.domain_results[result_index]) + { + Ok(finalized) => finalized, + Err(error) => { + record_domain_failure( + &layout, + &mut report, + &mut journal_sequence, + result_index, + step.domain, + MigrationPhase::ValidateCommit, + &error, + )?; + if adapter.rollback_unverified(&context).is_err() { + return Err(LegacyMigrationError::Domain { + domain: step.domain, + message: error.to_string(), + }); } - }; + cancellation.check()?; + last_failure = Some((step.domain, MigrationPhase::ValidateCommit, error)); + continue; + } + }; if finalized.domain != step.domain { let error = LegacyMigrationError::InvalidPlan(format!( "adapter {:?} finalized a result for {:?}", @@ -629,16 +744,40 @@ impl MigrationEngine { ) }) }); - let final_status = if has_warnings { + let has_failures = report + .domain_results + .iter() + .any(|result| result.state == MigrationDomainState::Failed); + let final_status = if has_failures { + MigrationRunStatus::FailedRecoverable + } else if has_warnings { MigrationRunStatus::CompletedWithWarnings } else { MigrationRunStatus::Completed }; - let final_code = if has_warnings { + let final_code = if has_failures { + "migration_partially_failed" + } else if has_warnings { "migration_completed_with_warnings" } else { "migration_completed" }; + if let Some((domain, phase, error)) = last_failure { + transition( + &layout, + &mut report, + &mut journal_sequence, + MigrationRunStatus::FailedRecoverable, + phase, + Some(domain), + Some(MigrationDomainState::Failed), + "domain_failed_recoverable", + )?; + return Err(LegacyMigrationError::Domain { + domain, + message: error.to_string(), + }); + } transition( &layout, &mut report, @@ -1060,16 +1199,6 @@ fn inject(injector: &dyn CrashInjector, point: CrashPoint) -> LegacyMigrationRes } } -fn domain_error(domain: MigrationDomainId, error: LegacyMigrationError) -> LegacyMigrationError { - match error { - LegacyMigrationError::Domain { .. } => error, - error => LegacyMigrationError::Domain { - domain, - message: error.to_string(), - }, - } -} - fn now_ms() -> i64 { chrono::Utc::now().timestamp_millis() } diff --git a/src/crates/services/legacy-migration/tests/migration_engine_contracts.rs b/src/crates/services/legacy-migration/tests/migration_engine_contracts.rs index ef12a7a0ac..bf21e0975d 100644 --- a/src/crates/services/legacy-migration/tests/migration_engine_contracts.rs +++ b/src/crates/services/legacy-migration/tests/migration_engine_contracts.rs @@ -42,6 +42,8 @@ struct FakeAdapter { calls: Arc>>, finalize_behavior: FinalizeBehavior, stage_error: Option, + scan_error: bool, + dependencies: Vec, } impl LegacyDomainAdapter for FakeAdapter { @@ -51,6 +53,11 @@ impl LegacyDomainAdapter for FakeAdapter { fn scan(&self, _roots: &MigrationRoots) -> LegacyMigrationResult { self.update(|counts| counts.scan += 1); + if self.scan_error { + return Err(LegacyMigrationError::UnsupportedSource( + "invalid fixture".to_string(), + )); + } Ok(DomainScan { finding: ScanFinding { domain: self.domain, @@ -63,7 +70,7 @@ impl LegacyDomainAdapter for FakeAdapter { }, conflicts: Vec::new(), target_schema: Some("fake.current".to_string()), - dependencies: Vec::new(), + dependencies: self.dependencies.clone(), }) } @@ -378,6 +385,7 @@ fn engine_rolls_back_unverified_commit_when_owner_finalization_fails() { }) )); assert!(!target_path_for_roots(&roots, MigrationDomainId::Settings).exists()); + assert!(target_path_for_roots(&roots, MigrationDomainId::Credentials).exists()); let settings_calls = calls .lock() .expect("calls should not poison") @@ -769,6 +777,8 @@ fn fake_engine_with_behaviors( } else { FinalizeBehavior::PassThrough }, + scan_error: false, + dependencies: Vec::new(), stage_error: (domain == MigrationDomainId::Settings) .then_some(settings_stage_error) .flatten(), @@ -777,6 +787,71 @@ fn fake_engine_with_behaviors( MigrationEngine::new(roots, adapters).expect("fake engine should be valid") } +#[test] +fn scan_failure_skips_dependents_but_executes_and_recovers_independent_domains() { + let temp = test_tempdir(); + let roots = fixture_roots(temp.path()); + seed_supported_source(&roots); + let calls = Arc::new(Mutex::new(BTreeMap::new())); + let selection = MigrationSelection { + groups: BTreeSet::from([MigrationGroupId::SettingsAndCredentials]), + }; + let adapters = selection.expanded_domains().into_iter().map(|domain| { + Box::new(FakeAdapter { + domain, + calls: calls.clone(), + finalize_behavior: FinalizeBehavior::PassThrough, + stage_error: None, + scan_error: domain == MigrationDomainId::Settings, + dependencies: if domain == MigrationDomainId::Credentials { + vec![MigrationDomainId::Settings] + } else { + Vec::new() + }, + }) as Box + }); + let engine = MigrationEngine::new(roots.clone(), adapters).unwrap(); + let source = probe_legacy_source(&roots, ProbeLimits::default()) + .unwrap() + .unwrap(); + let plan = engine + .plan(&source, selection, &CancellationToken::default()) + .unwrap(); + assert_eq!( + plan.findings + .iter() + .filter(|finding| !finding.migratable) + .count(), + 2 + ); + let serialized = serde_json::to_vec(&plan).unwrap(); + let restored = serde_json::from_slice(&serialized).unwrap(); + for _ in 0..2 { + let report = engine + .execute(&restored, &CancellationToken::default(), &NoCrashInjection) + .unwrap(); + assert_eq!(report.status, MigrationRunStatus::CompletedWithWarnings); + assert_eq!( + report + .domain_results + .iter() + .filter(|result| result.state == MigrationDomainState::Skipped) + .count(), + 2 + ); + } + let calls = calls.lock().unwrap(); + assert_eq!(calls[&MigrationDomainId::Settings].stage, 0); + assert_eq!(calls[&MigrationDomainId::Credentials].stage, 0); + assert_eq!(calls[&MigrationDomainId::CrossReferenceRepair].commit, 1); + let cancelled = CancellationToken::default(); + cancelled.cancel(); + assert!(matches!( + engine.scan(&plan.selection, &cancelled), + Err(LegacyMigrationError::Cancelled) + )); +} + fn fixture_roots(root: &Path) -> MigrationRoots { MigrationRoots { legacy_user_root: root.join("legacy/user"), diff --git a/src/crates/services/services-core/src/memory_store.rs b/src/crates/services/services-core/src/memory_store.rs index 85744e4fcf..70adae3885 100644 --- a/src/crates/services/services-core/src/memory_store.rs +++ b/src/crates/services/services-core/src/memory_store.rs @@ -290,12 +290,12 @@ pub fn classify_memory_workspace_file(path: &Path) -> Option None, }; } - if components.len() == 4 + if components.len() >= 4 && components[0].as_os_str() == MEMORY_EXTENSIONS_DIR_NAME && components[1].as_os_str() == AD_HOC_EXTENSION_NAME && components[2].as_os_str() == AD_HOC_NOTES_DIR_NAME { - let file_name = components[3].as_os_str().to_str()?; + let file_name = components.last()?.as_os_str().to_str()?; if !file_name.is_empty() && file_name.ends_with(".md") { return Some(MemoryWorkspaceFileKind::AdHocNote); } @@ -396,6 +396,18 @@ const UPSERT_STAGE1_OUTPUT_SQL: &str = r#" mod tests { use super::*; + #[test] + fn nested_user_notes_keep_owner_classification_without_path_escape() { + assert!(classify_memory_workspace_file(Path::new( + "extensions/ad_hoc/notes/project/note.md" + )) + .is_some()); + assert!( + classify_memory_workspace_file(Path::new("extensions/ad_hoc/notes/../note.md")) + .is_none() + ); + } + #[test] fn owner_schema_round_trips_records_and_keeps_jobs_separate() { let connection = Connection::open_in_memory().unwrap();