diff --git a/scripts/data-migrator-report.test.mjs b/scripts/data-migrator-report.test.mjs new file mode 100644 index 0000000000..7eb56deab0 --- /dev/null +++ b/scripts/data-migrator-report.test.mjs @@ -0,0 +1,23 @@ +import assert from 'node:assert/strict'; +import { test } from 'node:test'; +import { reportRows } from '../src/apps/data-migrator/ui/report.mjs'; + +const text = { sessions: 'Sessions', workspaces: 'Workspaces', assistantDirectories: 'Assistants', imported: 'imported', staged: 'staged', skipped: 'skipped', itemCountsUnavailable: 'unavailable' }; +const report = { domainResults: [{ domain: 'workspace_sessions', state: 'verified', imported: 175, skipped: 173, warnings: [{ severity: 'info', code: 'session_path_not_migrated' }] }] }; +const counts = { sessions: { imported: 143, skipped: 0 }, workspaces: { imported: 30, skipped: 0 }, assistantDirectories: { imported: 2, skipped: 0 } }; +test('renders separate entity counts and hides auxiliary exclusions', () => { + assert.deepEqual(reportRows(report, counts, text), [ + ['Sessions', '143 imported, 0 skipped'], ['Workspaces', '30 imported, 0 skipped'], ['Assistants', '2 imported, 0 skipped'], + ]); +}); +test('retains real warnings and does not label staged data imported', () => { + const staged = structuredClone(report); + staged.domainResults[0].state = 'failed'; + staged.domainResults[0].warnings.push({ severity: 'warning', code: 'session_source_skipped' }); + const rows = reportRows(staged, counts, text); + assert.equal(rows[0][1], '143 staged, 0 skipped'); + assert.deepEqual(rows.at(-1), ['workspace_sessions', 'session_source_skipped']); +}); +test('missing historical manifest is explicit instead of inventing zero counts', () => { + assert.deepEqual(reportRows(report, null, text), [['Sessions', 'unavailable'], ['Workspaces', 'unavailable'], ['Assistants', 'unavailable']]); +}); diff --git a/src/apps/data-migrator/AGENTS.md b/src/apps/data-migrator/AGENTS.md index 2128eca3de..3677830e73 100644 --- a/src/apps/data-migrator/AGENTS.md +++ b/src/apps/data-migrator/AGENTS.md @@ -34,6 +34,7 @@ cargo test -p openbitfun-data-migrator -p openbitfun-legacy-migration-adapters - cargo test -p openbitfun-legacy-migration --test migration_engine_contracts node --test scripts/data-migrator-tauri-build.test.mjs node --check src/apps/data-migrator/ui/app.js +node --test scripts/data-migrator-report.test.mjs pnpm run theme:color-audit:all ``` diff --git a/src/apps/data-migrator/README.md b/src/apps/data-migrator/README.md index 2e7a5b5cd7..50bb391bee 100644 --- a/src/apps/data-migrator/README.md +++ b/src/apps/data-migrator/README.md @@ -35,6 +35,11 @@ needed for migration. ARM Windows/Linux packages are not currently produced. 5. Read the report. Sign in again or repair paths where indicated, close the tool, and open OpenBitFun yourself. +The report lists Sessions, workspace records, and assistant directories separately. +Expected auxiliary exclusions (such as request traces and lock files) do not +contribute to these displayed counts. Historical runs use their saved manifests +for the same breakdown; missing manifests are shown as unavailable. + The UI uses the shared design-system tokens bundled offline, follows the system light/dark/high-contrast setting, and offers English, Simplified Chinese, and Traditional Chinese. diff --git a/src/apps/data-migrator/src/app_state.rs b/src/apps/data-migrator/src/app_state.rs index 1b2eb172b0..5f7696ba49 100644 --- a/src/apps/data-migrator/src/app_state.rs +++ b/src/apps/data-migrator/src/app_state.rs @@ -167,6 +167,7 @@ pub(crate) struct MigratorView { pub findings: Vec, pub plan: Option, pub report: Option, + pub workspace_counts: Option, pub progress: Option, pub blockers: Vec, pub status: MigrationRunStatus, @@ -794,6 +795,9 @@ fn validate_selection(selection: &MigrationSelection) -> Result<(), CommandError fn snapshot(session: &MigratorSession) -> MigratorView { let plan = session.plan.as_ref().map(redact_plan_for_ui); let report = session.report.as_ref().map(redact_report_for_ui); + let workspace_counts = session.report.as_ref().and_then(|report| { + openbitfun_legacy_migration_adapters::workspace_report_counts(&session.roots, report).ok() + }); MigratorView { tool_version: env!("CARGO_PKG_VERSION").to_string(), locations: session.roots.clone(), @@ -818,6 +822,7 @@ fn snapshot(session: &MigratorSession) -> MigratorView { ), plan, report, + workspace_counts, progress: session.progress.clone(), blockers: session.blockers.clone(), status: session.status, diff --git a/src/apps/data-migrator/ui/app.js b/src/apps/data-migrator/ui/app.js index b6040034a6..8dd76b04be 100644 --- a/src/apps/data-migrator/ui/app.js +++ b/src/apps/data-migrator/ui/app.js @@ -1,4 +1,5 @@ import { invoke } from './transport.js'; +import { reportRows } from './report.mjs'; async function loadTranslations() { try { @@ -98,10 +99,6 @@ function row(title, detail) { return item; } -function transferLabel(result) { - return result.state === 'verified' ? text.imported : text.staged; -} - function groupLabel(id) { const labels = groups.find(([group]) => group === id)?.[1]; return (labels?.[locale] || labels?.en || [id])[0]; @@ -202,11 +199,8 @@ function render(view) { const reportSummary = document.getElementById('report-summary'); if (view.report) { - 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)), - ])); + reportSummary.replaceChildren(...reportRows(view.report, view.workspaceCounts, text) + .map(([title, detail]) => row(title, detail))); } 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 b2f507e133..fb8079dab0 100644 --- a/src/apps/data-migrator/ui/locales.json +++ b/src/apps/data-migrator/ui/locales.json @@ -1,5 +1,9 @@ { "en": { + "sessions": "Sessions", + "workspaces": "Workspaces", + "assistantDirectories": "Assistant directories", + "itemCountsUnavailable": "Item counts are unavailable for this run.", "eyebrow": "OpenBitFun Data Migrator", "title": "Import data from BitFun", "intro": "A separate, offline tool. Choose what to import; your original BitFun data is preserved.", @@ -76,6 +80,10 @@ "extension_item_skipped": "Incomplete or unsupported extensions will be skipped; valid extensions can continue." }, "zh-CN": { + "sessions": "会话", + "workspaces": "工作区", + "assistantDirectories": "助手目录", + "itemCountsUnavailable": "本次迁移的分类计数不可用。", "eyebrow": "OpenBitFun 独立数据迁移器", "title": "从 BitFun 导入数据", "intro": "独立运行，离线导入。选择需要迁移的内容，原始 BitFun 数据会被保留。", @@ -152,6 +160,10 @@ "extension_item_skipped": "将跳过不完整或不支持的扩展，其他有效扩展可继续迁移。" }, "zh-TW": { + "sessions": "工作階段", + "workspaces": "工作區", + "assistantDirectories": "助理目錄", + "itemCountsUnavailable": "本次遷移的分類計數無法取得。", "eyebrow": "OpenBitFun 獨立資料遷移器", "title": "從 BitFun 匯入資料", "intro": "獨立執行，離線匯入。選擇需要遷移的內容，原始 BitFun 資料會被保留。", diff --git a/src/apps/data-migrator/ui/report.mjs b/src/apps/data-migrator/ui/report.mjs new file mode 100644 index 0000000000..f51de90012 --- /dev/null +++ b/src/apps/data-migrator/ui/report.mjs @@ -0,0 +1,21 @@ +export function reportRows(report, workspaceCounts, text) { + return report.domainResults.flatMap((result) => { + const transferred = result.state === 'verified' ? text.imported : text.staged; + const warnings = result.warnings.filter((item) => item.severity !== 'info'); + const rows = result.domain === 'workspace_sessions' + ? [ + ['sessions', text.sessions], + ['workspaces', text.workspaces], + ['assistantDirectories', text.assistantDirectories], + ].map(([key, label]) => { + const counts = workspaceCounts?.[key]; + return [label, counts + ? `${counts.imported} ${transferred}, ${counts.skipped} ${text.skipped}` + : text.itemCountsUnavailable]; + }) + : [[result.domain, `${result.imported} ${transferred}, ${result.skipped} ${text.skipped}, ${warnings.length} ${text.warnings}`]]; + rows.push(...[...new Set(warnings.map((item) => item.code))] + .map((code) => [result.domain, text[code] || code])); + return rows; + }); +} diff --git a/src/crates/assembly/core/src/service/config/manager.rs b/src/crates/assembly/core/src/service/config/manager.rs index 010cfa454c..fb2c9c07a0 100644 --- a/src/crates/assembly/core/src/service/config/manager.rs +++ b/src/crates/assembly/core/src/service/config/manager.rs @@ -399,10 +399,14 @@ impl ConfigManager { return Err(contract_error); } - let config: GlobalConfig = serde_json::from_value(config_value).map_err(|error| { + let mut config: GlobalConfig = serde_json::from_value(config_value).map_err(|error| { OpenBitFunError::config(format!("Failed to deserialize config file: {error}")) })?; - let validation_result = self.providers.validate_config(&config).await?; + let mut diagnostics = + openbitfun_config_contracts::normalization::recover_persisted_config(&mut config); + let validation_result = self + .validate_recovered_config(&mut config, &mut diagnostics) + .await?; if !validation_result.valid { return Err(invalid_config_error( "Invalid configuration file", @@ -411,11 +415,57 @@ impl ConfigManager { } self.config = config; - self.load_diagnostics.clear(); + self.load_diagnostics = diagnostics; debug!("Loaded OpenBitFun config from file without rewriting it"); Ok(()) } + // Catalog/provider-dependent reasoning checks cannot run in the offline + // importer. Preserve unusable model records and surface a diagnostic while + // allowing the remaining configuration to load. Each retry disables at + // least one enabled model; unrelated validation errors remain errors. + async fn validate_recovered_config( + &self, + config: &mut GlobalConfig, + diagnostics: &mut Vec, + ) -> OpenBitFunResult { + loop { + let result = self.providers.validate_config(config).await?; + let mut recovered = false; + for error in &result.errors { + let Some(index) = error + .path + .strip_prefix("ai.models[") + .and_then(|path| path.split_once(']')) + .and_then(|(index, _)| index.parse::().ok()) + else { + continue; + }; + let Some(model) = config + .ai + .models + .get_mut(index) + .filter(|model| model.enabled) + else { + continue; + }; + model.enabled = false; + diagnostics.push(super::types::ConfigDiagnostic { + path: error.path.clone(), + message: error.message.clone(), + code: "CONFIG_MODEL_DISABLED".into(), + severity: super::types::ConfigDiagnosticSeverity::Warning, + recoverability: super::types::ConfigDiagnosticRecoverability::ModelDisabled, + }); + recovered = true; + } + if !recovered { + return Ok(result); + } + diagnostics.extend(reconcile_model_references(config).diagnostics); + } + } + async fn try_repair_sparse_installer_config( &mut self, content: &str, @@ -1034,6 +1084,54 @@ mod tests { } } + #[tokio::test] + async fn existing_config_recovers_semantics_without_rewriting_source() { + let temp = tempfile::tempdir().unwrap(); + let path_manager = Arc::new(PathManager::with_user_root_for_tests( + temp.path().join("recovery"), + )); + path_manager.initialize_user_directories().await.unwrap(); + let mut config = GlobalConfig::default(); + config.ai.stream_idle_timeout_secs = Some(0); + config.ai.default_models.primary = Some("missing".into()); + config.ai.agent_model_defaults.subagents.default_selection = + super::super::types::SubagentModelSelection::fixed("missing-model"); + config.app.logging.level = "invalid".into(); + config.ai.models.push(super::super::types::AIModelConfig { + id: "broken".into(), + name: "Broken".into(), + provider: "openai".into(), + enabled: true, + context_window: Some(1), + api_key: "retained-secret".into(), + ..Default::default() + }); + let original = serde_json::to_vec(&config).unwrap(); + tokio::fs::write(path_manager.app_config_file(), &original) + .await + .unwrap(); + let manager = ConfigManager::new(manager_settings(path_manager.clone())) + .await + .unwrap(); + assert!(!manager.config.ai.models[0].enabled); + assert_eq!(manager.config.ai.models[0].api_key, "retained-secret"); + assert!( + manager + .providers + .validate_config(&manager.config) + .await + .unwrap() + .valid + ); + assert!(!manager.load_diagnostics.is_empty()); + assert_eq!( + tokio::fs::read(path_manager.app_config_file()) + .await + .unwrap(), + original + ); + } + #[tokio::test] async fn sparse_installer_config_is_backed_up_and_repaired() { let temp = tempfile::tempdir().unwrap(); diff --git a/src/crates/assembly/core/src/service/config/normalization.rs b/src/crates/assembly/core/src/service/config/normalization.rs index 868cd6d61e..b10889b5d8 100644 --- a/src/crates/assembly/core/src/service/config/normalization.rs +++ b/src/crates/assembly/core/src/service/config/normalization.rs @@ -1,519 +1,2 @@ -use super::types::{ - ConfigDiagnostic, ConfigDiagnosticRecoverability, ConfigDiagnosticSeverity, GlobalConfig, - ModelCapability, SubagentModelSelection, -}; -use std::collections::HashSet; - -/// Canonicalizes typed model fields whose meaning is capability-dependent. -pub fn normalize_typed_config(config: &mut GlobalConfig) -> Vec { - let mut diagnostics = Vec::new(); - - for (index, model) in config.ai.models.iter_mut().enumerate() { - model.ensure_category_and_capabilities(); - let model_id = model.id.clone(); - for field in model.normalize_inapplicable_generation_fields() { - diagnostics.push(ConfigDiagnostic { - path: format!("ai.models[{index}].{field}"), - message: format!( - "Cleared text-generation-only field from model '{}' because it does not support text_chat", - model_id - ), - code: "MODEL_FIELD_NOT_APPLICABLE".to_string(), - severity: ConfigDiagnosticSeverity::Warning, - recoverability: ConfigDiagnosticRecoverability::AutoFix, - }); - } - } - - diagnostics -} - -#[derive(Debug, Clone, Default)] -pub struct ModelReferenceReconcileResult { - pub invalidated_model_ids: Vec, - pub default_models_changed: bool, - pub task_models_changed: bool, - pub agent_model_defaults_changed: bool, - pub diagnostics: Vec, -} - -impl ModelReferenceReconcileResult { - pub fn is_noop(&self) -> bool { - !self.default_models_changed - && !self.task_models_changed - && !self.agent_model_defaults_changed - } -} - -fn enabled_model_with_capability( - config: &GlobalConfig, - model_id: &str, - capability: ModelCapability, -) -> bool { - config.ai.models.iter().any(|model| { - model.enabled && model.id == model_id && model.supports_capability(capability.clone()) - }) -} - -fn first_enabled_model_with_capability( - config: &GlobalConfig, - capability: ModelCapability, -) -> Option { - config - .ai - .models - .iter() - .find(|model| model.enabled && model.supports_capability(capability.clone())) - .map(|model| model.id.clone()) -} - -fn diagnose_reference_repair( - diagnostics: &mut Vec, - path: &str, - previous: Option<&str>, - replacement: Option<&str>, -) { - diagnostics.push(ConfigDiagnostic { - path: path.to_string(), - message: format!( - "Repaired model reference from {:?} to {:?} to match the slot capability", - previous, replacement - ), - code: "MODEL_REFERENCE_REPAIRED".to_string(), - severity: ConfigDiagnosticSeverity::Warning, - recoverability: ConfigDiagnosticRecoverability::AutoFix, - }); -} - -#[derive(Clone, Copy)] -enum MissingSlotPolicy { - FillFromFirstCapableModel, - Preserve, -} - -/// Reconciles every product model reference against both enablement and the -/// capability required by its consumer. -pub fn reconcile_model_references(config: &mut GlobalConfig) -> ModelReferenceReconcileResult { - let snapshot = config.clone(); - let mut result = ModelReferenceReconcileResult::default(); - let mut invalidated = HashSet::new(); - - let direct_text_reference_is_valid = |reference: &str| { - matches!(reference, "primary" | "fast") - || enabled_model_with_capability(&snapshot, reference, ModelCapability::TextChat) - }; - - let mut reconcile_task_model = - |selection: &mut crate::service::config::types::TaskModelSelection, - path: &str, - allow_inherit: bool| { - let previous = selection.fixed_model_id().map(str::to_string); - let valid = match selection { - crate::service::config::types::TaskModelSelection::Inherit => allow_inherit, - crate::service::config::types::TaskModelSelection::Fixed { model_id } => { - direct_text_reference_is_valid(model_id) - } - }; - if !valid { - if let Some(model_id) = previous.as_ref() { - invalidated.insert(model_id.clone()); - } - *selection = crate::service::config::types::TaskModelSelection::Fixed { - model_id: "fast".to_string(), - }; - result.task_models_changed = true; - diagnose_reference_repair( - &mut result.diagnostics, - path, - previous.as_deref().or(Some("inherit")), - Some("fast"), - ); - } - }; - reconcile_task_model( - &mut config.ai.task_models.session_title, - "ai.task_models.session_title", - true, - ); - reconcile_task_model( - &mut config.ai.task_models.git_commit, - "ai.task_models.git_commit", - false, - ); - - if !direct_text_reference_is_valid(&config.ai.agent_model_defaults.mode) { - invalidated.insert(config.ai.agent_model_defaults.mode.clone()); - let previous = std::mem::replace( - &mut config.ai.agent_model_defaults.mode, - "primary".to_string(), - ); - result.agent_model_defaults_changed = true; - diagnose_reference_repair( - &mut result.diagnostics, - "ai.agent_model_defaults.mode", - Some(&previous), - Some("primary"), - ); - } - - if config - .ai - .agent_model_defaults - .subagents - .default_selection - .fixed_model_id() - .is_some_and(|model_id| !direct_text_reference_is_valid(model_id)) - { - let previous = config - .ai - .agent_model_defaults - .subagents - .default_selection - .fixed_model_id() - .map(str::to_string); - if let Some(previous) = previous.as_ref() { - invalidated.insert(previous.clone()); - } - config.ai.agent_model_defaults.subagents.default_selection = - SubagentModelSelection::fixed("fast"); - result.agent_model_defaults_changed = true; - diagnose_reference_repair( - &mut result.diagnostics, - "ai.agent_model_defaults.subagents.default", - previous.as_deref(), - Some("fast"), - ); - } - - if let std::collections::hash_map::Entry::Vacant(entry) = config - .ai - .agent_model_defaults - .subagents - .builtin - .entry("ResearchSpecialist".to_string()) - { - entry.insert(SubagentModelSelection::Inherit); - result.agent_model_defaults_changed = true; - result.diagnostics.push(ConfigDiagnostic { - path: "ai.agent_model_defaults.subagents.builtin.ResearchSpecialist".to_string(), - message: "Configured ResearchSpecialist to inherit the parent model so DeepResearch does not depend on an unrelated fast-model endpoint".to_string(), - code: "RESEARCH_SPECIALIST_MODEL_DEFAULT_RESTORED".to_string(), - severity: ConfigDiagnosticSeverity::Warning, - recoverability: ConfigDiagnosticRecoverability::AutoFix, - }); - } - - config - .ai - .agent_model_defaults - .subagents - .builtin - .retain(|subagent_id, selection| { - let invalid = selection - .fixed_model_id() - .is_some_and(|model_id| !direct_text_reference_is_valid(model_id)); - if invalid { - if let Some(model_id) = selection.fixed_model_id() { - invalidated.insert(model_id.to_string()); - diagnose_reference_repair( - &mut result.diagnostics, - &format!("ai.agent_model_defaults.subagents.builtin.{subagent_id}"), - Some(model_id), - None, - ); - } - result.agent_model_defaults_changed = true; - } - !invalid - }); - - if config - .ai - .agent_model_defaults - .subagents - .fork - .fixed_model_id() - .is_some_and(|model_id| !direct_text_reference_is_valid(model_id)) - { - let previous = config - .ai - .agent_model_defaults - .subagents - .fork - .fixed_model_id() - .map(str::to_string); - if let Some(previous) = previous.as_ref() { - invalidated.insert(previous.clone()); - } - config.ai.agent_model_defaults.subagents.fork = SubagentModelSelection::Inherit; - result.agent_model_defaults_changed = true; - diagnose_reference_repair( - &mut result.diagnostics, - "ai.agent_model_defaults.subagents.fork", - previous.as_deref(), - Some("inherit"), - ); - } - - let mut reconcile_slot = |slot: &mut Option, - path: &str, - capability: ModelCapability, - missing_policy: MissingSlotPolicy| { - let previous = slot.clone(); - let valid = previous - .as_deref() - .is_some_and(|id| enabled_model_with_capability(&snapshot, id, capability.clone())); - if valid || (previous.is_none() && matches!(missing_policy, MissingSlotPolicy::Preserve)) { - return; - } - let replacement = first_enabled_model_with_capability(&snapshot, capability); - if replacement == previous { - return; - } - if let Some(previous) = previous.as_ref().filter(|id| !id.is_empty()) { - invalidated.insert(previous.clone()); - } - *slot = replacement; - result.default_models_changed = true; - diagnose_reference_repair( - &mut result.diagnostics, - path, - previous.as_deref(), - slot.as_deref(), - ); - }; - - reconcile_slot( - &mut config.ai.default_models.primary, - "ai.default_models.primary", - ModelCapability::TextChat, - MissingSlotPolicy::FillFromFirstCapableModel, - ); - reconcile_slot( - &mut config.ai.default_models.fast, - "ai.default_models.fast", - ModelCapability::TextChat, - MissingSlotPolicy::Preserve, - ); - reconcile_slot( - &mut config.ai.default_models.image_understanding, - "ai.default_models.image_understanding", - ModelCapability::ImageUnderstanding, - MissingSlotPolicy::Preserve, - ); - reconcile_slot( - &mut config.ai.default_models.image_generation, - "ai.default_models.image_generation", - ModelCapability::ImageGeneration, - MissingSlotPolicy::Preserve, - ); - reconcile_slot( - &mut config.ai.default_models.search, - "ai.default_models.search", - ModelCapability::Search, - MissingSlotPolicy::Preserve, - ); - reconcile_slot( - &mut config.ai.default_models.speech_recognition, - "ai.default_models.speech_recognition", - ModelCapability::SpeechRecognition, - MissingSlotPolicy::Preserve, - ); - - result.invalidated_model_ids = invalidated.into_iter().collect(); - result.invalidated_model_ids.sort(); - result -} - -#[cfg(test)] -mod tests { - use super::*; - use crate::service::config::types::{AIModelConfig, ModelCapability, ModelCategory}; - - #[test] - fn pure_speech_models_drop_text_generation_sentinels() { - let mut config = GlobalConfig::default(); - config.ai.models.push(AIModelConfig { - id: "speech-cloud".to_string(), - name: "Qwen ASR".to_string(), - category: ModelCategory::SpeechRecognition, - capabilities: vec![ModelCapability::SpeechRecognition], - context_window: Some(0), - max_tokens: Some(0), - enabled: true, - ..AIModelConfig::default() - }); - - let diagnostics = normalize_typed_config(&mut config); - - assert_eq!(config.ai.models[0].context_window, None); - assert_eq!(config.ai.models[0].max_tokens, None); - assert_eq!(diagnostics.len(), 2); - assert!(diagnostics - .iter() - .all(|diagnostic| diagnostic.code == "MODEL_FIELD_NOT_APPLICABLE")); - } - - #[test] - fn mixed_text_and_speech_models_keep_generation_fields() { - let mut config = GlobalConfig::default(); - config.ai.models.push(AIModelConfig { - id: "mixed".to_string(), - category: ModelCategory::GeneralChat, - capabilities: vec![ - ModelCapability::TextChat, - ModelCapability::SpeechRecognition, - ], - context_window: Some(64_000), - max_tokens: Some(8_000), - ..AIModelConfig::default() - }); - - assert!(normalize_typed_config(&mut config).is_empty()); - assert_eq!(config.ai.models[0].context_window, Some(64_000)); - assert_eq!(config.ai.models[0].max_tokens, Some(8_000)); - } - - #[test] - fn default_slots_reconcile_by_capability() { - let mut config = GlobalConfig::default(); - config.ai.models = vec![ - AIModelConfig { - id: "speech".to_string(), - enabled: true, - category: ModelCategory::SpeechRecognition, - capabilities: vec![ModelCapability::SpeechRecognition], - ..AIModelConfig::default() - }, - AIModelConfig { - id: "text".to_string(), - enabled: true, - category: ModelCategory::GeneralChat, - capabilities: vec![ModelCapability::TextChat], - ..AIModelConfig::default() - }, - ]; - config.ai.default_models.primary = Some("speech".to_string()); - config.ai.default_models.fast = Some("speech".to_string()); - config.ai.default_models.speech_recognition = Some("text".to_string()); - - let result = reconcile_model_references(&mut config); - - assert_eq!(config.ai.default_models.primary.as_deref(), Some("text")); - assert_eq!(config.ai.default_models.fast.as_deref(), Some("text")); - assert_eq!( - config.ai.default_models.speech_recognition.as_deref(), - Some("speech") - ); - assert!(result.default_models_changed); - } - - #[test] - fn missing_fast_slot_is_preserved_and_resolves_to_primary() { - let mut config = GlobalConfig::default(); - config.ai.models = vec![ - AIModelConfig { - id: "first-text".to_string(), - enabled: true, - category: ModelCategory::GeneralChat, - capabilities: vec![ModelCapability::TextChat], - ..AIModelConfig::default() - }, - AIModelConfig { - id: "primary-text".to_string(), - enabled: true, - category: ModelCategory::GeneralChat, - capabilities: vec![ModelCapability::TextChat], - ..AIModelConfig::default() - }, - ]; - config.ai.default_models.primary = Some("primary-text".to_string()); - config.ai.default_models.fast = None; - - let result = reconcile_model_references(&mut config); - - assert_eq!(config.ai.default_models.fast, None); - assert_eq!( - config.ai.resolve_model_selection("fast").as_deref(), - Some("primary-text") - ); - assert!(!result.default_models_changed); - } - - #[test] - fn task_models_reconcile_invalid_fixed_and_git_inherit_to_fast() { - let mut config = GlobalConfig::default(); - config.ai.task_models.session_title = - crate::service::config::types::TaskModelSelection::Fixed { - model_id: "missing".to_string(), - }; - config.ai.task_models.git_commit = - crate::service::config::types::TaskModelSelection::Inherit; - - let result = reconcile_model_references(&mut config); - - assert!(result.task_models_changed); - assert_eq!( - config.ai.task_models.session_title.fixed_model_id(), - Some("fast") - ); - assert_eq!( - config.ai.task_models.git_commit.fixed_model_id(), - Some("fast") - ); - assert!(result - .invalidated_model_ids - .contains(&"missing".to_string())); - } - - #[test] - fn missing_research_specialist_default_is_restored_for_current_writes() { - let mut config = GlobalConfig::default(); - config - .ai - .agent_model_defaults - .subagents - .builtin - .remove("ResearchSpecialist"); - - let result = reconcile_model_references(&mut config); - - assert!(result.agent_model_defaults_changed); - assert_eq!( - config - .ai - .agent_model_defaults - .subagents - .builtin - .get("ResearchSpecialist"), - Some(&SubagentModelSelection::Inherit) - ); - assert!(result - .diagnostics - .iter() - .any(|diagnostic| { diagnostic.code == "RESEARCH_SPECIALIST_MODEL_DEFAULT_RESTORED" })); - } - - #[test] - fn explicit_research_specialist_model_override_is_preserved() { - let mut config = GlobalConfig::default(); - config.ai.agent_model_defaults.subagents.builtin.insert( - "ResearchSpecialist".to_string(), - SubagentModelSelection::fixed("fast"), - ); - - let result = reconcile_model_references(&mut config); - - assert!(!result.agent_model_defaults_changed); - assert_eq!( - config - .ai - .agent_model_defaults - .subagents - .builtin - .get("ResearchSpecialist"), - Some(&SubagentModelSelection::fixed("fast")) - ); - } - -} +//! Shared pure configuration normalization; runtime callers retain this facade. +pub use openbitfun_config_contracts::normalization::*; diff --git a/src/crates/assembly/core/src/service/config/providers.rs b/src/crates/assembly/core/src/service/config/providers.rs index ffb6d71051..ce84e38671 100644 --- a/src/crates/assembly/core/src/service/config/providers.rs +++ b/src/crates/assembly/core/src/service/config/providers.rs @@ -13,6 +13,7 @@ use crate::infrastructure::ai::AIClient; use crate::util::errors::*; use async_trait::async_trait; use log::{error, info}; +#[cfg(any(feature = "ai-adapter-runtime", test))] use openbitfun_core_types::ReasoningCatalogBinding; #[cfg(test)] use openbitfun_core_types::{ReasoningConfig, ReasoningPreset, ReasoningPresetAction}; @@ -180,56 +181,15 @@ impl ConfigProvider for AIConfigProvider { } for (index, model) in ai_config.models.iter().enumerate() { - if !model.supports_text_generation() { - for (field, present) in [ - ("context_window", model.context_window.is_some()), - ("max_tokens", model.max_tokens.is_some()), - ("temperature", model.temperature.is_some()), - ("top_p", model.top_p.is_some()), - ] { - if present { - return Err(OpenBitFunError::validation(format!( - "Model '{}' has text-generation-only field {field} at index {}", - model.name, index - ))); - } - } - } + openbitfun_config_contracts::normalization::validate_model_config(model, index) + .map_err(OpenBitFunError::validation)?; if !model.enabled { continue; } - if model.name.trim().is_empty() { - return Err(OpenBitFunError::validation(format!( - "Model name is required at index {}", - index - ))); - } - if model.provider.trim().is_empty() { - return Err(OpenBitFunError::validation(format!( - "Model provider is required at index {}", - index - ))); - } if model.api_key.trim().is_empty() { warnings.push(format!("Model '{}' has empty API key", model.name)); } if model.supports_text_generation() { - if let Some(context_window) = model.context_window { - if context_window < MIN_MODEL_CONTEXT_WINDOW_TOKENS { - return Err(OpenBitFunError::validation(format!( - "Model '{}' context_window must be at least {} at index {}", - model.name, MIN_MODEL_CONTEXT_WINDOW_TOKENS, index - ))); - } - } - if let Some(max_tokens) = model.max_tokens { - if max_tokens == 0 { - return Err(OpenBitFunError::validation(format!( - "Model '{}' max_tokens must be greater than 0 at index {}", - model.name, index - ))); - } - } if let Some(temperature) = model.temperature { if !temperature.is_nan() && !(0.0..=2.0).contains(&temperature) { warnings.push(format!( @@ -239,15 +199,9 @@ impl ConfigProvider for AIConfigProvider { } } } - + #[cfg(feature = "ai-adapter-runtime")] if let Some(reasoning) = model.reasoning.as_ref() { - reasoning.validate_schema().map_err(|message| { - OpenBitFunError::validation(format!( - "Model '{}' reasoning config is invalid at index {}: {}", - model.name, index, message - )) - })?; - + #[cfg(feature = "ai-adapter-runtime")] if let Some(default_preset) = reasoning.default_preset.as_deref() { #[cfg(feature = "ai-adapter-runtime")] { @@ -264,16 +218,6 @@ impl ConfigProvider for AIConfigProvider { ))); } } - - #[cfg(not(feature = "ai-adapter-runtime"))] - if reasoning.preset(default_preset).is_none() - && matches!(reasoning.catalog, ReasoningCatalogBinding::Disabled) - { - return Err(OpenBitFunError::validation(format!( - "Model '{}' reasoning default preset '{}' is not available at index {}", - model.name, default_preset, index - ))); - } } #[cfg(feature = "ai-adapter-runtime")] @@ -321,14 +265,13 @@ impl ConfigProvider for AIConfigProvider { } } - let enabled_model_with_capability = - |model_id: &str, capability: ModelCapability| { - ai_config.models.iter().any(|model| { - model.enabled - && model.id == model_id - && model.supports_capability(capability.clone()) - }) - }; + let enabled_model_with_capability = |model_id: &str, capability: ModelCapability| { + ai_config.models.iter().any(|model| { + model.enabled + && model.id == model_id + && model.supports_capability(capability.clone()) + }) + }; for (field, model_id, capability) in [ ( "primary", @@ -400,19 +343,20 @@ impl ConfigProvider for AIConfigProvider { crate::service::config::types::TaskModelSelection::Fixed { .. } => {} } - let validate_agent_selection = - |path: &str, selection: &SubagentModelSelection| -> OpenBitFunResult<()> { - if selection - .fixed_model_id() - .is_some_and(|model_id| !valid_task_model(model_id)) - { - return Err(OpenBitFunError::validation(format!( + let validate_agent_selection = |path: &str, + selection: &SubagentModelSelection| + -> OpenBitFunResult<()> { + if selection + .fixed_model_id() + .is_some_and(|model_id| !valid_task_model(model_id)) + { + return Err(OpenBitFunError::validation(format!( "ai.agent_model_defaults.{path} references unavailable or incapable model '{}'", selection.fixed_model_id().expect("checked above") ))); - } - Ok(()) - }; + } + Ok(()) + }; if !valid_task_model(&ai_config.agent_model_defaults.mode) { return Err(OpenBitFunError::validation(format!( "ai.agent_model_defaults.mode references unavailable or incapable model '{}'", @@ -458,7 +402,6 @@ impl ConfigProvider for AIConfigProvider { } Ok(()) } - } /// Web UI appearance selection provider. @@ -507,7 +450,6 @@ impl ConfigProvider for AppearanceConfigProvider { } Ok(()) } - } /// Editor configuration provider. @@ -560,7 +502,6 @@ impl ConfigProvider for EditorConfigProvider { } Ok(()) } - } /// Terminal configuration provider. @@ -617,7 +558,6 @@ impl ConfigProvider for TerminalConfigProvider { } Ok(()) } - } /// Workspace configuration provider. @@ -668,7 +608,6 @@ impl ConfigProvider for WorkspaceConfigProvider { } Ok(()) } - } /// App configuration provider. @@ -728,7 +667,6 @@ impl ConfigProvider for AppConfigProvider { } Ok(()) } - } /// Configuration provider registry. diff --git a/src/crates/assembly/core/src/service/config/service.rs b/src/crates/assembly/core/src/service/config/service.rs index f3b64df301..96fa30783d 100644 --- a/src/crates/assembly/core/src/service/config/service.rs +++ b/src/crates/assembly/core/src/service/config/service.rs @@ -1752,7 +1752,7 @@ mod tests { } #[tokio::test] - async fn startup_rejects_speech_generation_fields_without_mutating_the_file() { + async fn startup_recovers_speech_generation_fields_without_mutating_the_file() { let dir = tempfile::tempdir().expect("tempdir"); let user_root = dir.path().join("speech-startup-repair"); let path_manager = Arc::new(PathManager::with_user_root_for_tests(user_root)); @@ -1781,17 +1781,22 @@ mod tests { .await .expect("seed config"); - let error = match ConfigService::with_settings(ConfigManagerSettings { + let service = ConfigService::with_settings(ConfigManagerSettings { path_manager: Some(path_manager.clone()), auto_save: true, backup_count: 5, }) .await - { - Ok(_) => panic!("inapplicable speech model fields must fail startup"), - Err(error) => error, - }; - assert!(error.to_string().contains("context_window"), "{error}"); + .expect("legacy speech fields should recover"); + let models = service.get_ai_models().await.unwrap(); + assert!(models[0].enabled); + assert_eq!(models[0].context_window, None); + assert_eq!(models[0].max_tokens, None); + assert!(service + .load_diagnostics() + .await + .iter() + .any(|d| d.path.ends_with("context_window"))); assert_eq!( tokio::fs::read_to_string(path_manager.app_config_file()) .await @@ -2261,7 +2266,7 @@ mod tests { } #[tokio::test] - async fn startup_rejects_invalid_canonical_reasoning_config() { + async fn startup_disables_invalid_reasoning_model_without_mutating_the_file() { let dir = tempfile::tempdir().expect("tempdir"); let user_root = dir.path().join("invalid-reasoning-startup"); let path_manager = Arc::new(PathManager::with_user_root_for_tests(user_root)); @@ -2300,13 +2305,31 @@ mod tests { .await .expect("invalid config should be written"); - let error = match ConfigService::with_settings(settings()).await { - Ok(_) => panic!("invalid canonical reasoning must fail startup"), - Err(error) => error, - }; - - assert!(error - .to_string() - .contains("default preset 'missing' is not available")); + let original = tokio::fs::read(path_manager.app_config_file()) + .await + .unwrap(); + let service = ConfigService::with_settings(settings()).await.unwrap(); + let models = service.get_ai_models().await.unwrap(); + assert_eq!(models.len(), 1); + assert!(!models[0].enabled); + assert_eq!(models[0].api_key, "key"); + assert_eq!( + models[0] + .reasoning + .as_ref() + .unwrap() + .default_preset + .as_deref(), + Some("missing") + ); + assert!(service.load_diagnostics().await.iter().any(|d| d + .message + .contains("default preset 'missing' is not available"))); + assert_eq!( + tokio::fs::read(path_manager.app_config_file()) + .await + .unwrap(), + original + ); } } diff --git a/src/crates/contracts/config-contracts/AGENTS.md b/src/crates/contracts/config-contracts/AGENTS.md index 26131b5e80..63fb65e322 100644 --- a/src/crates/contracts/config-contracts/AGENTS.md +++ b/src/crates/contracts/config-contracts/AGENTS.md @@ -6,6 +6,15 @@ Keep existing schema versions and tolerant field defaults. Core retains the live ConfigProvider interface and re-exports the records from its old paths. The ts feature preserves TypeScript export capability without enabling runtime. +`normalization` owns pure model-field validation, capability normalization, +reference reconciliation, and persisted-value recovery shared by startup and +offline migration. Recovery retains unusable model records disabled and returns +diagnostics; callers own persistence. Catalog/provider-dependent reasoning +validation remains in the runtime configuration provider. +Keep 0.2.x model-selector translations, such as `auto` to `primary`, in the +legacy migration adapter. Shared recovery handles invalid current model +references without recognizing historical selector aliases. + Focused verification: ```bash diff --git a/src/crates/contracts/config-contracts/src/lib.rs b/src/crates/contracts/config-contracts/src/lib.rs index dae379f4fc..1bb974e1f0 100644 --- a/src/crates/contracts/config-contracts/src/lib.rs +++ b/src/crates/contracts/config-contracts/src/lib.rs @@ -1,5 +1,6 @@ //! Persisted configuration DTOs and pure compatibility validation. pub mod agent_identity_migration; pub mod conversion; +pub mod normalization; mod types; pub use types::*; diff --git a/src/crates/contracts/config-contracts/src/normalization.rs b/src/crates/contracts/config-contracts/src/normalization.rs new file mode 100644 index 0000000000..adeb0cd438 --- /dev/null +++ b/src/crates/contracts/config-contracts/src/normalization.rs @@ -0,0 +1,703 @@ +use crate::types::{ + ConfigDiagnostic, ConfigDiagnosticRecoverability, ConfigDiagnosticSeverity, GlobalConfig, + ModelCapability, SubagentModelSelection, +}; +use std::collections::HashSet; + +/// Provider-neutral model validation, shared by startup and offline import. +pub fn validate_model_config(model: &crate::AIModelConfig, index: usize) -> Result<(), String> { + if !model.supports_text_generation() { + for (field, present) in [ + ("context_window", model.context_window.is_some()), + ("max_tokens", model.max_tokens.is_some()), + ("temperature", model.temperature.is_some()), + ("top_p", model.top_p.is_some()), + ] { + if present { + return Err(format!( + "Model '{}' has text-generation-only field {field} at index {index}", + model.name + )); + } + } + } + if !model.enabled { + return Ok(()); + } + if model.name.trim().is_empty() { + return Err(format!("Model name is required at index {index}")); + } + if model.provider.trim().is_empty() { + return Err(format!("Model provider is required at index {index}")); + } + if model.supports_text_generation() { + if model + .context_window + .is_some_and(|size| size < crate::MIN_MODEL_CONTEXT_WINDOW_TOKENS) + { + return Err(format!( + "Model '{}' context_window must be at least {} at index {index}", + model.name, + crate::MIN_MODEL_CONTEXT_WINDOW_TOKENS + )); + } + if model.max_tokens == Some(0) { + return Err(format!( + "Model '{}' max_tokens must be greater than 0 at index {index}", + model.name + )); + } + } + if let Some(reasoning) = &model.reasoning { + reasoning.validate_schema().map_err(|error| { + format!( + "Model '{}' reasoning config is invalid at index {index}: {error}", + model.name + ) + })?; + if let Some(preset) = &reasoning.default_preset { + if matches!( + reasoning.catalog, + openbitfun_core_types::ReasoningCatalogBinding::Disabled + ) && reasoning.preset(preset).is_none() + { + return Err(format!( + "Model '{}' reasoning default preset '{}' is not available at index {index}", + model.name, preset + )); + } + } + } + Ok(()) +} + +/// Recover old persisted values without dropping model records or credentials. +/// The caller owns reporting and persistence; this function performs no IO. +pub fn recover_persisted_config(config: &mut GlobalConfig) -> Vec { + let mut diagnostics = normalize_typed_config(config); + let defaults = GlobalConfig::default(); + let mut repaired = |path: &str| { + diagnostics.push(ConfigDiagnostic { + path: path.into(), + message: "An invalid persisted value was replaced with the current default.".into(), + code: "CONFIG_VALUE_RECOVERED".into(), + severity: ConfigDiagnosticSeverity::Warning, + recoverability: ConfigDiagnosticRecoverability::DefaultsUsed, + }) + }; + if config.ai.stream_idle_timeout_secs == Some(0) { + config.ai.stream_idle_timeout_secs = defaults.ai.stream_idle_timeout_secs; + repaired("ai.stream_idle_timeout_secs"); + } + if config.ai.stream_ttft_timeout_secs == Some(0) { + config.ai.stream_ttft_timeout_secs = defaults.ai.stream_ttft_timeout_secs; + repaired("ai.stream_ttft_timeout_secs"); + } + if config.appearance.selection.trim().is_empty() { + config.appearance.selection = defaults.appearance.selection; + repaired("appearance.selection"); + } + if !matches!( + config.app.logging.level.to_lowercase().as_str(), + "trace" | "debug" | "info" | "warn" | "error" | "off" + ) { + config.app.logging.level = defaults.app.logging.level; + repaired("app.logging.level"); + } + for (index, model) in config.ai.models.iter_mut().enumerate() { + if let Err(message) = validate_model_config(model, index) { + model.enabled = false; + diagnostics.push(ConfigDiagnostic { + path: format!("ai.models[{index}]"), + message, + code: "CONFIG_MODEL_DISABLED".into(), + severity: ConfigDiagnosticSeverity::Warning, + recoverability: ConfigDiagnosticRecoverability::ModelDisabled, + }); + } + } + diagnostics.extend(reconcile_model_references(config).diagnostics); + diagnostics +} + +/// Canonicalizes typed model fields whose meaning is capability-dependent. +pub fn normalize_typed_config(config: &mut GlobalConfig) -> Vec { + let mut diagnostics = Vec::new(); + + for (index, model) in config.ai.models.iter_mut().enumerate() { + model.ensure_category_and_capabilities(); + let model_id = model.id.clone(); + for field in model.normalize_inapplicable_generation_fields() { + diagnostics.push(ConfigDiagnostic { + path: format!("ai.models[{index}].{field}"), + message: format!( + "Cleared text-generation-only field from model '{}' because it does not support text_chat", + model_id + ), + code: "MODEL_FIELD_NOT_APPLICABLE".to_string(), + severity: ConfigDiagnosticSeverity::Warning, + recoverability: ConfigDiagnosticRecoverability::AutoFix, + }); + } + } + + diagnostics +} + +#[derive(Debug, Clone, Default)] +pub struct ModelReferenceReconcileResult { + pub invalidated_model_ids: Vec, + pub default_models_changed: bool, + pub task_models_changed: bool, + pub agent_model_defaults_changed: bool, + pub diagnostics: Vec, +} + +impl ModelReferenceReconcileResult { + pub fn is_noop(&self) -> bool { + !self.default_models_changed + && !self.task_models_changed + && !self.agent_model_defaults_changed + } +} + +fn enabled_model_with_capability( + config: &GlobalConfig, + model_id: &str, + capability: ModelCapability, +) -> bool { + config.ai.models.iter().any(|model| { + model.enabled && model.id == model_id && model.supports_capability(capability.clone()) + }) +} + +fn first_enabled_model_with_capability( + config: &GlobalConfig, + capability: ModelCapability, +) -> Option { + config + .ai + .models + .iter() + .find(|model| model.enabled && model.supports_capability(capability.clone())) + .map(|model| model.id.clone()) +} + +fn diagnose_reference_repair( + diagnostics: &mut Vec, + path: &str, + previous: Option<&str>, + replacement: Option<&str>, +) { + diagnostics.push(ConfigDiagnostic { + path: path.to_string(), + message: format!( + "Repaired model reference from {:?} to {:?} to match the slot capability", + previous, replacement + ), + code: "MODEL_REFERENCE_REPAIRED".to_string(), + severity: ConfigDiagnosticSeverity::Warning, + recoverability: ConfigDiagnosticRecoverability::AutoFix, + }); +} + +#[derive(Clone, Copy)] +enum MissingSlotPolicy { + FillFromFirstCapableModel, + Preserve, +} + +/// Reconciles every product model reference against both enablement and the +/// capability required by its consumer. +pub fn reconcile_model_references(config: &mut GlobalConfig) -> ModelReferenceReconcileResult { + let snapshot = config.clone(); + let mut result = ModelReferenceReconcileResult::default(); + let mut invalidated = HashSet::new(); + + let direct_text_reference_is_valid = |reference: &str| { + matches!(reference, "primary" | "fast") + || enabled_model_with_capability(&snapshot, reference, ModelCapability::TextChat) + }; + + let mut reconcile_task_model = + |selection: &mut crate::types::TaskModelSelection, path: &str, allow_inherit: bool| { + let previous = selection.fixed_model_id().map(str::to_string); + let valid = match selection { + crate::types::TaskModelSelection::Inherit => allow_inherit, + crate::types::TaskModelSelection::Fixed { model_id } => { + direct_text_reference_is_valid(model_id) + } + }; + if !valid { + if let Some(model_id) = previous.as_ref() { + invalidated.insert(model_id.clone()); + } + *selection = crate::types::TaskModelSelection::Fixed { + model_id: "fast".to_string(), + }; + result.task_models_changed = true; + diagnose_reference_repair( + &mut result.diagnostics, + path, + previous.as_deref().or(Some("inherit")), + Some("fast"), + ); + } + }; + reconcile_task_model( + &mut config.ai.task_models.session_title, + "ai.task_models.session_title", + true, + ); + reconcile_task_model( + &mut config.ai.task_models.git_commit, + "ai.task_models.git_commit", + false, + ); + + if !direct_text_reference_is_valid(&config.ai.agent_model_defaults.mode) { + invalidated.insert(config.ai.agent_model_defaults.mode.clone()); + let previous = std::mem::replace( + &mut config.ai.agent_model_defaults.mode, + "primary".to_string(), + ); + result.agent_model_defaults_changed = true; + diagnose_reference_repair( + &mut result.diagnostics, + "ai.agent_model_defaults.mode", + Some(&previous), + Some("primary"), + ); + } + + if config + .ai + .agent_model_defaults + .subagents + .default_selection + .fixed_model_id() + .is_some_and(|model_id| !direct_text_reference_is_valid(model_id)) + { + let previous = config + .ai + .agent_model_defaults + .subagents + .default_selection + .fixed_model_id() + .map(str::to_string); + if let Some(previous) = previous.as_ref() { + invalidated.insert(previous.clone()); + } + config.ai.agent_model_defaults.subagents.default_selection = + SubagentModelSelection::fixed("fast"); + result.agent_model_defaults_changed = true; + diagnose_reference_repair( + &mut result.diagnostics, + "ai.agent_model_defaults.subagents.default", + previous.as_deref(), + Some("fast"), + ); + } + + if let std::collections::hash_map::Entry::Vacant(entry) = config + .ai + .agent_model_defaults + .subagents + .builtin + .entry("ResearchSpecialist".to_string()) + { + entry.insert(SubagentModelSelection::Inherit); + result.agent_model_defaults_changed = true; + result.diagnostics.push(ConfigDiagnostic { + path: "ai.agent_model_defaults.subagents.builtin.ResearchSpecialist".to_string(), + message: "Configured ResearchSpecialist to inherit the parent model so DeepResearch does not depend on an unrelated fast-model endpoint".to_string(), + code: "RESEARCH_SPECIALIST_MODEL_DEFAULT_RESTORED".to_string(), + severity: ConfigDiagnosticSeverity::Warning, + recoverability: ConfigDiagnosticRecoverability::AutoFix, + }); + } + + config + .ai + .agent_model_defaults + .subagents + .builtin + .retain(|subagent_id, selection| { + let invalid = selection + .fixed_model_id() + .is_some_and(|model_id| !direct_text_reference_is_valid(model_id)); + if invalid { + if let Some(model_id) = selection.fixed_model_id() { + invalidated.insert(model_id.to_string()); + diagnose_reference_repair( + &mut result.diagnostics, + &format!("ai.agent_model_defaults.subagents.builtin.{subagent_id}"), + Some(model_id), + None, + ); + } + result.agent_model_defaults_changed = true; + } + !invalid + }); + + if config + .ai + .agent_model_defaults + .subagents + .fork + .fixed_model_id() + .is_some_and(|model_id| !direct_text_reference_is_valid(model_id)) + { + let previous = config + .ai + .agent_model_defaults + .subagents + .fork + .fixed_model_id() + .map(str::to_string); + if let Some(previous) = previous.as_ref() { + invalidated.insert(previous.clone()); + } + config.ai.agent_model_defaults.subagents.fork = SubagentModelSelection::Inherit; + result.agent_model_defaults_changed = true; + diagnose_reference_repair( + &mut result.diagnostics, + "ai.agent_model_defaults.subagents.fork", + previous.as_deref(), + Some("inherit"), + ); + } + + let mut reconcile_slot = |slot: &mut Option, + path: &str, + capability: ModelCapability, + missing_policy: MissingSlotPolicy| { + let previous = slot.clone(); + let valid = previous + .as_deref() + .is_some_and(|id| enabled_model_with_capability(&snapshot, id, capability.clone())); + if valid || (previous.is_none() && matches!(missing_policy, MissingSlotPolicy::Preserve)) { + return; + } + let replacement = first_enabled_model_with_capability(&snapshot, capability); + if replacement == previous { + return; + } + if let Some(previous) = previous.as_ref().filter(|id| !id.is_empty()) { + invalidated.insert(previous.clone()); + } + *slot = replacement; + result.default_models_changed = true; + diagnose_reference_repair( + &mut result.diagnostics, + path, + previous.as_deref(), + slot.as_deref(), + ); + }; + + reconcile_slot( + &mut config.ai.default_models.primary, + "ai.default_models.primary", + ModelCapability::TextChat, + MissingSlotPolicy::FillFromFirstCapableModel, + ); + reconcile_slot( + &mut config.ai.default_models.fast, + "ai.default_models.fast", + ModelCapability::TextChat, + MissingSlotPolicy::Preserve, + ); + reconcile_slot( + &mut config.ai.default_models.image_understanding, + "ai.default_models.image_understanding", + ModelCapability::ImageUnderstanding, + MissingSlotPolicy::Preserve, + ); + reconcile_slot( + &mut config.ai.default_models.image_generation, + "ai.default_models.image_generation", + ModelCapability::ImageGeneration, + MissingSlotPolicy::Preserve, + ); + reconcile_slot( + &mut config.ai.default_models.search, + "ai.default_models.search", + ModelCapability::Search, + MissingSlotPolicy::Preserve, + ); + reconcile_slot( + &mut config.ai.default_models.speech_recognition, + "ai.default_models.speech_recognition", + ModelCapability::SpeechRecognition, + MissingSlotPolicy::Preserve, + ); + + result.invalidated_model_ids = invalidated.into_iter().collect(); + result.invalidated_model_ids.sort(); + result +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn recovery_preserves_models_and_repairs_all_invalid_reference_families() { + let mut value = serde_json::to_value(GlobalConfig::default()).unwrap(); + let patch = serde_json::json!({"ai": { + "stream_idle_timeout_secs": 0, "stream_ttft_timeout_secs": 0, + "models": [ + {"id":"text", "name":"Text", "provider":"openai", "enabled":true}, + {"id":"speech", "name":"Speech", "provider":"openai", "enabled":true, + "capabilities":["speech_recognition"], "context_window":0, "max_tokens":0, "temperature":1.0, "top_p":1.0}, + {"id":"broken", "name":"", "enabled":true, "api_key":"retained-secret"} + ], + "default_models":{"primary":"missing", "fast":"broken", "search":"text", "speech_recognition":"text"}, + "task_models":{"session_title":{"kind":"fixed","model_id":"speech"}, "git_commit":{"kind":"inherit"}}, + "agent_model_defaults":{"mode":"missing", "subagents":{ + "default":{"kind":"fixed","model_id":"missing-model"}, + "fork":{"kind":"fixed","model_id":"broken"}, + "builtin":{"Explore":{"kind":"fixed","model_id":"missing"}} + }} + }, "app":{"logging":{"level":"invalid"}}, "appearance":{"selection":" "}}); + for (key, field) in patch.as_object().unwrap() { + value[key] = field.clone(); + } + let mut config: GlobalConfig = serde_json::from_value(value).unwrap(); + let diagnostics = recover_persisted_config(&mut config); + assert!(!diagnostics.is_empty()); + assert_eq!(config.ai.models.len(), 3); + assert!(!config.ai.models[2].enabled); + assert_eq!(config.ai.models[2].api_key, "retained-secret"); + assert!(config.ai.models[1].enabled); + assert_eq!(config.ai.models[1].context_window, None); + assert_eq!(config.ai.models[1].top_p, None); + assert_eq!(config.ai.default_models.primary.as_deref(), Some("text")); + assert_eq!(config.ai.default_models.fast.as_deref(), Some("text")); + assert_eq!(config.ai.default_models.search, None); + assert_eq!( + config.ai.default_models.speech_recognition.as_deref(), + Some("speech") + ); + assert_eq!( + config.ai.task_models.session_title.fixed_model_id(), + Some("fast") + ); + assert_eq!( + config.ai.task_models.git_commit.fixed_model_id(), + Some("fast") + ); + assert_eq!(config.ai.agent_model_defaults.mode, "primary"); + assert_eq!( + config + .ai + .agent_model_defaults + .subagents + .default_selection + .fixed_model_id(), + Some("fast") + ); + assert_eq!( + config.ai.agent_model_defaults.subagents.fork, + SubagentModelSelection::Inherit + ); + assert!(!config + .ai + .agent_model_defaults + .subagents + .builtin + .contains_key("Explore")); + assert!(recover_persisted_config(&mut config).is_empty()); + let saved = serde_json::to_value(&config).unwrap(); + let mut reloaded: GlobalConfig = serde_json::from_value(saved.clone()).unwrap(); + assert!(recover_persisted_config(&mut reloaded).is_empty()); + assert_eq!(serde_json::to_value(reloaded).unwrap(), saved); + } + use crate::types::{AIModelConfig, ModelCapability, ModelCategory}; + + #[test] + fn pure_speech_models_drop_text_generation_sentinels() { + let mut config = GlobalConfig::default(); + config.ai.models.push(AIModelConfig { + id: "speech-cloud".to_string(), + name: "Qwen ASR".to_string(), + category: ModelCategory::SpeechRecognition, + capabilities: vec![ModelCapability::SpeechRecognition], + context_window: Some(0), + max_tokens: Some(0), + enabled: true, + ..AIModelConfig::default() + }); + + let diagnostics = normalize_typed_config(&mut config); + + assert_eq!(config.ai.models[0].context_window, None); + assert_eq!(config.ai.models[0].max_tokens, None); + assert_eq!(diagnostics.len(), 2); + assert!(diagnostics + .iter() + .all(|diagnostic| diagnostic.code == "MODEL_FIELD_NOT_APPLICABLE")); + } + + #[test] + fn mixed_text_and_speech_models_keep_generation_fields() { + let mut config = GlobalConfig::default(); + config.ai.models.push(AIModelConfig { + id: "mixed".to_string(), + category: ModelCategory::GeneralChat, + capabilities: vec![ + ModelCapability::TextChat, + ModelCapability::SpeechRecognition, + ], + context_window: Some(64_000), + max_tokens: Some(8_000), + ..AIModelConfig::default() + }); + + assert!(normalize_typed_config(&mut config).is_empty()); + assert_eq!(config.ai.models[0].context_window, Some(64_000)); + assert_eq!(config.ai.models[0].max_tokens, Some(8_000)); + } + + #[test] + fn default_slots_reconcile_by_capability() { + let mut config = GlobalConfig::default(); + config.ai.models = vec![ + AIModelConfig { + id: "speech".to_string(), + enabled: true, + category: ModelCategory::SpeechRecognition, + capabilities: vec![ModelCapability::SpeechRecognition], + ..AIModelConfig::default() + }, + AIModelConfig { + id: "text".to_string(), + enabled: true, + category: ModelCategory::GeneralChat, + capabilities: vec![ModelCapability::TextChat], + ..AIModelConfig::default() + }, + ]; + config.ai.default_models.primary = Some("speech".to_string()); + config.ai.default_models.fast = Some("speech".to_string()); + config.ai.default_models.speech_recognition = Some("text".to_string()); + + let result = reconcile_model_references(&mut config); + + assert_eq!(config.ai.default_models.primary.as_deref(), Some("text")); + assert_eq!(config.ai.default_models.fast.as_deref(), Some("text")); + assert_eq!( + config.ai.default_models.speech_recognition.as_deref(), + Some("speech") + ); + assert!(result.default_models_changed); + } + + #[test] + fn missing_fast_slot_is_preserved_and_resolves_to_primary() { + let mut config = GlobalConfig::default(); + config.ai.models = vec![ + AIModelConfig { + id: "first-text".to_string(), + enabled: true, + category: ModelCategory::GeneralChat, + capabilities: vec![ModelCapability::TextChat], + ..AIModelConfig::default() + }, + AIModelConfig { + id: "primary-text".to_string(), + enabled: true, + category: ModelCategory::GeneralChat, + capabilities: vec![ModelCapability::TextChat], + ..AIModelConfig::default() + }, + ]; + config.ai.default_models.primary = Some("primary-text".to_string()); + config.ai.default_models.fast = None; + + let result = reconcile_model_references(&mut config); + + assert_eq!(config.ai.default_models.fast, None); + assert_eq!( + config.ai.resolve_model_selection("fast").as_deref(), + Some("primary-text") + ); + assert!(!result.default_models_changed); + } + + #[test] + fn task_models_reconcile_invalid_fixed_and_git_inherit_to_fast() { + let mut config = GlobalConfig::default(); + config.ai.task_models.session_title = crate::types::TaskModelSelection::Fixed { + model_id: "missing".to_string(), + }; + config.ai.task_models.git_commit = crate::types::TaskModelSelection::Inherit; + + let result = reconcile_model_references(&mut config); + + assert!(result.task_models_changed); + assert_eq!( + config.ai.task_models.session_title.fixed_model_id(), + Some("fast") + ); + assert_eq!( + config.ai.task_models.git_commit.fixed_model_id(), + Some("fast") + ); + assert!(result + .invalidated_model_ids + .contains(&"missing".to_string())); + } + + #[test] + fn missing_research_specialist_default_is_restored_for_current_writes() { + let mut config = GlobalConfig::default(); + config + .ai + .agent_model_defaults + .subagents + .builtin + .remove("ResearchSpecialist"); + + let result = reconcile_model_references(&mut config); + + assert!(result.agent_model_defaults_changed); + assert_eq!( + config + .ai + .agent_model_defaults + .subagents + .builtin + .get("ResearchSpecialist"), + Some(&SubagentModelSelection::Inherit) + ); + assert!(result + .diagnostics + .iter() + .any(|diagnostic| { diagnostic.code == "RESEARCH_SPECIALIST_MODEL_DEFAULT_RESTORED" })); + } + + #[test] + fn explicit_research_specialist_model_override_is_preserved() { + let mut config = GlobalConfig::default(); + config.ai.agent_model_defaults.subagents.builtin.insert( + "ResearchSpecialist".to_string(), + SubagentModelSelection::fixed("fast"), + ); + + let result = reconcile_model_references(&mut config); + + assert!(!result.agent_model_defaults_changed); + assert_eq!( + config + .ai + .agent_model_defaults + .subagents + .builtin + .get("ResearchSpecialist"), + Some(&SubagentModelSelection::fixed("fast")) + ); + } +} diff --git a/src/crates/contracts/config-contracts/src/types.rs b/src/crates/contracts/config-contracts/src/types.rs index 6ddbd4985d..ebd5b37bea 100644 --- a/src/crates/contracts/config-contracts/src/types.rs +++ b/src/crates/contracts/config-contracts/src/types.rs @@ -2894,6 +2894,32 @@ mod tests { assert_eq!(inherited, SubagentModelSelection::Inherit); } + #[test] + fn mode_model_selector_preserves_configured_ids_on_round_trip() { + for (stored, expected) in [ + ("custom-selector", "custom-selector"), + ("primary", "primary"), + ("fast", "fast"), + ("custom-model", "custom-model"), + ] { + let mut value = serde_json::to_value(GlobalConfig::default()).unwrap(); + value["ai"]["agent_model_defaults"]["mode"] = serde_json::json!(stored); + let config: GlobalConfig = serde_json::from_value(value).unwrap(); + assert_eq!(config.ai.agent_model_defaults.mode, expected); + let saved = serde_json::to_value(&config).unwrap(); + assert_eq!(saved["ai"]["agent_model_defaults"]["mode"], expected); + let reloaded: GlobalConfig = serde_json::from_value(saved).unwrap(); + assert_eq!(reloaded.ai.agent_model_defaults.mode, expected); + } + let missing: AgentModelDefaultsConfig = + serde_json::from_value(serde_json::json!({})).unwrap(); + assert_eq!(missing.mode, "primary"); + assert!(serde_json::from_value::( + serde_json::json!({"mode": 42}) + ) + .is_err()); + } + #[test] fn builtin_subagent_without_override_uses_the_shared_default() { let mut defaults = AgentModelDefaultsConfig::default(); diff --git a/src/crates/services/legacy-migration-adapters/AGENTS.md b/src/crates/services/legacy-migration-adapters/AGENTS.md index 0194c780e9..08fb25f833 100644 --- a/src/crates/services/legacy-migration-adapters/AGENTS.md +++ b/src/crates/services/legacy-migration-adapters/AGENTS.md @@ -8,6 +8,10 @@ Keep original source-read-only behavior, atomic domain writes, idempotent journa recovery, reference repair and target conflict rules. Share storage formats with config-contracts and services-core; do not copy private application schemas. +Settings recovery runs on the fully merged configuration after target model +conflicts are resolved. Use config-contracts normalization and include recovery +diagnostics in the migration report; never mutate the legacy source to repair it. + Focused verification: ```bash 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 068ce8719b..f034f139ee 100644 --- a/src/crates/services/legacy-migration-adapters/src/agent_coordination.rs +++ b/src/crates/services/legacy-migration-adapters/src/agent_coordination.rs @@ -1439,6 +1439,66 @@ mod tests { } } + #[test] + fn deep_assistant_content_does_not_skip_session_domains() { + let temp = test_tempdir("deep-assistant-content"); + let roots = fixture_roots(temp.path()); + copy_fixture(&roots); + let _connection = materialize_coordination(&roots, false); + let mut deep = roots + .legacy_home_root + .join("personal_assistant/workspace/build"); + for _ in 0..20 { + deep.push("d"); + } + fs::create_dir_all(&deep).unwrap(); + fs::write(deep.join("content.txt"), b"preserve deep workspace content").unwrap(); + let selection = session_selection(); + 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().all(|finding| finding.migratable)); + let report = engine + .execute(&plan, &CancellationToken::default(), &NoCrashInjection) + .unwrap(); + assert!(report + .domain_results + .iter() + .all(|result| result.state == MigrationDomainState::Verified)); + assert!(roots + .target_home_root + .join("projects/c--fixture-workspace/sessions/session-1/metadata.json") + .is_file()); + assert!(roots + .target_user_root + .join(COORDINATION_RELATIVE_PATH) + .is_file()); + assert!(deep.is_dir()); + let counts = crate::workspace_report_counts(&roots, &report).unwrap(); + assert_eq!(counts.sessions.imported, 2); + assert_eq!(counts.sessions.skipped, 0); + assert_eq!(counts.workspaces.imported, 1); + assert_eq!(counts.assistant_directories.imported, 1); + let relative = deep.strip_prefix(&roots.legacy_home_root).unwrap(); + assert_eq!( + fs::read(roots.target_home_root.join(relative).join("content.txt")).unwrap(), + b"preserve deep workspace content" + ); + assert_eq!( + fs::read(deep.join("content.txt")).unwrap(), + b"preserve deep workspace content" + ); + assert!(!report + .domain_results + .iter() + .flat_map(|r| &r.warnings) + .any(|w| w.code == "assistant_workspace_not_migrated")); + } + #[test] fn session_group_migrates_owner_data_wal_and_relationship_closure() { let temp = test_tempdir("session-group"); diff --git a/src/crates/services/legacy-migration-adapters/src/lib.rs b/src/crates/services/legacy-migration-adapters/src/lib.rs index 3f8f9761dd..03a44e5273 100644 --- a/src/crates/services/legacy-migration-adapters/src/lib.rs +++ b/src/crates/services/legacy-migration-adapters/src/lib.rs @@ -12,6 +12,7 @@ mod remote_connect; mod remote_ssh; mod settings; mod workspace_sessions; +pub use workspace_sessions::{workspace_report_counts, MigrationItemCounts, WorkspaceReportCounts}; use openbitfun_legacy_migration::{ DomainContext, DomainScan, LegacyDomainAdapter, LegacyMigrationResult, MigrationRoots, 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 77dd02069b..f6329a1aad 100644 --- a/src/crates/services/legacy-migration-adapters/src/remote_connect.rs +++ b/src/crates/services/legacy-migration-adapters/src/remote_connect.rs @@ -361,13 +361,15 @@ fn preview(source: &RemoteConnectState, target: &RemoteConnectState) -> Preview let mut skipped = source.omitted.len() as u64; let mut conflicts = Vec::new(); if source.device.is_some() { - if target.device.is_some() { + if let Some(target_device) = &target.device { skipped += 1; - conflicts.push(target_wins( - "device_identity_target_wins", - "legacy device identity", - "current device identity", - )); + if source.device.as_ref() != Some(target_device) { + conflicts.push(target_wins( + "device_identity_target_wins", + "legacy device identity", + "current device identity", + )); + } } else { imported += 1; } @@ -1192,6 +1194,32 @@ mod tests { assert!(read_state(root, false).is_err()); } + #[test] + fn identical_device_identity_is_a_duplicate_without_a_conflict() { + let device = owner::DeviceIdentityRecord { + device_id: "11111111111111111111111111111111".into(), + device_name: "device".into(), + mac_address: "02:00:00:00:00:11".into(), + }; + let source = RemoteConnectState { + device: Some(device.clone()), + ..Default::default() + }; + let mut target = RemoteConnectState { + device: Some(device), + ..Default::default() + }; + let duplicate = preview(&source, &target); + assert_eq!(duplicate.imported, 0); + assert_eq!(duplicate.skipped, 1); + assert!(duplicate.conflicts.is_empty()); + target.device.as_mut().unwrap().device_name = "different".into(); + assert_eq!( + preview(&source, &target).conflicts[0].code, + "device_identity_target_wins" + ); + } + #[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/settings.rs b/src/crates/services/legacy-migration-adapters/src/settings.rs index aa7c1ea837..807ffc346e 100644 --- a/src/crates/services/legacy-migration-adapters/src/settings.rs +++ b/src/crates/services/legacy-migration-adapters/src/settings.rs @@ -50,6 +50,7 @@ struct MergeOutcome { skipped: u64, conflicts: Vec, rejected: Vec, + repairs: Vec, } impl LegacyDomainAdapter for SettingsAdapter { @@ -67,7 +68,7 @@ impl LegacyDomainAdapter for SettingsAdapter { finding: ScanFinding { domain: self.domain(), 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 }, + severity: if outcome.rejected.is_empty() && outcome.repairs.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()), @@ -114,6 +115,14 @@ impl LegacyDomainAdapter for SettingsAdapter { .to_string(), ..Default::default() }) + .chain(outcome.repairs.iter().map(|repair| MigrationDiagnostic { + code: repair.code.clone(), + severity: FindingSeverity::Warning, + domain: Some(self.domain()), + relative_path: Some(repair.path.clone()), + message: repair.message.clone(), + ..Default::default() + })) .collect(), ..MigrationDomainResult::default() }) @@ -455,6 +464,8 @@ fn merge_settings( )) })?; merge_models(source_models, &mut merged.ai.models, &mut outcome)?; + outcome.repairs = + openbitfun_config_contracts::normalization::recover_persisted_config(&mut merged); merged.product_id = defaults.product_id; merged.schema_version = defaults.schema_version; merged.version = defaults.version; @@ -609,6 +620,15 @@ fn normalize_legacy_config_value(value: &mut Value) { let Some(ai) = value.get_mut("ai").and_then(Value::as_object_mut) else { return; }; + // The retired 0.2.x selector is translated only at the legacy import boundary. + if let Some(mode) = ai + .get_mut("agent_model_defaults") + .and_then(|defaults| defaults.get_mut("mode")) + { + if mode.as_str() == Some("auto") { + *mode = Value::String("primary".to_string()); + } + } if let Some(profiles) = ai.get_mut("agent_profiles").and_then(Value::as_object_mut) { for (profile_id, profile) in profiles { let Some(profile) = profile.as_object_mut() else { @@ -928,6 +948,122 @@ mod tests { use std::collections::BTreeSet; use std::path::Path; + #[test] + fn settings_recovery_is_reported_and_source_credentials_are_preserved() { + let temp = test_tempdir("settings-recovery"); + let roots = test_roots(temp.path()); + seed_source(&roots, true); + let mut source = read_source_config(&roots).unwrap(); + source["ai"]["stream_idle_timeout_secs"] = serde_json::json!(0); + source["ai"]["default_models"]["primary"] = serde_json::json!("missing-model"); + atomic_write_json(&source_config_path(&roots), &source).unwrap(); + let original = fs::read(source_config_path(&roots)).unwrap(); + let fingerprint = probe_legacy_source(&roots, ProbeLimits::default()) + .unwrap() + .unwrap(); + let selection = MigrationSelection { + groups: BTreeSet::from([MigrationGroupId::SettingsAndCredentials]), + }; + let engine = MigrationEngine::new(roots.clone(), adapters_for_groups(&selection)).unwrap(); + let plan = engine + .plan(&fingerprint, selection, &CancellationToken::default()) + .unwrap(); + let report = engine + .execute(&plan, &CancellationToken::default(), &NoCrashInjection) + .unwrap(); + let settings = report + .domain_results + .iter() + .find(|r| r.domain == MigrationDomainId::Settings) + .unwrap(); + assert_eq!(settings.state, MigrationDomainState::Verified); + assert!(settings + .warnings + .iter() + .any(|d| d.relative_path.as_deref() == Some("ai.stream_idle_timeout_secs"))); + assert!(settings + .warnings + .iter() + .any(|d| d.relative_path.as_deref() == Some("ai.default_models.primary"))); + let target = read_target_config(&roots).unwrap(); + assert_eq!( + target.ai.default_models.primary.as_deref(), + Some("legacy-model") + ); + assert_eq!( + target.ai.models[0].api_key, + source["ai"]["models"][0]["api_key"].as_str().unwrap() + ); + assert_eq!(fs::read(source_config_path(&roots)).unwrap(), original); + } + + #[test] + fn merged_references_use_the_preserved_target_model_capabilities() { + let source = serde_json::json!({"schema_version":1,"version":"0.2.19", "ai": { + "models":[{"id":"same", "name":"Source", "provider":"openai", "enabled":true}], + "default_models":{"primary":"same"}, + "agent_model_defaults":{"subagents":{"default":{"kind":"fixed","model_id":"same"}}}, + "stream_idle_timeout_secs":0 + }}); + let original = source.clone(); + let mut target = GlobalConfig::default(); + target.ai.models.push(AIModelConfig { + id: "same".into(), + name: "Target".into(), + enabled: false, + ..AIModelConfig::default() + }); + let (merged, outcome) = merge_settings(&source, target).unwrap(); + assert_eq!(merged.ai.models[0].name, "Target"); + assert!(!merged.ai.models[0].enabled); + assert_eq!(merged.ai.default_models.primary, None); + assert_eq!( + merged + .ai + .agent_model_defaults + .subagents + .default_selection + .fixed_model_id(), + Some("fast") + ); + assert!(outcome + .repairs + .iter() + .any(|d| d.path == "ai.default_models.primary")); + assert!(outcome + .repairs + .iter() + .any(|d| d.path == "ai.stream_idle_timeout_secs")); + assert_eq!(source, original); + } + + #[test] + fn legacy_auto_mode_migrates_to_primary_without_overriding_target_model() { + let source = serde_json::json!({ + "schema_version": 1, "version": "0.2.19", + "ai": {"agent_model_defaults": {"mode": "auto"}} + }); + let original = source.clone(); + let (config, fields, rejected) = convert_source_config(&source).unwrap(); + assert!(rejected.is_empty()); + assert_eq!(config.ai.agent_model_defaults.mode, "primary"); + assert_eq!(fields["ai"]["agent_model_defaults"]["mode"], "primary"); + let (merged, _) = merge_settings(&source, GlobalConfig::default()).unwrap(); + assert_eq!(merged.ai.agent_model_defaults.mode, "primary"); + let mut target = GlobalConfig::default(); + target.ai.agent_model_defaults.mode = "custom-model".into(); + target.ai.models.push(AIModelConfig { + id: "custom-model".into(), + name: "Custom".into(), + provider: "openai".into(), + enabled: true, + ..Default::default() + }); + let (merged, _) = merge_settings(&source, target).unwrap(); + assert_eq!(merged.ai.agent_model_defaults.mode, "custom-model"); + assert_eq!(source, original); + } + #[test] fn malformed_fields_and_models_do_not_discard_valid_siblings() { let source = serde_json::json!({"schema_version":1,"version":"0.2.19","ai":{ @@ -1157,11 +1293,16 @@ mod tests { AIModelConfig { id: "target-model".to_string(), name: "Target model".to_string(), + provider: "openai".to_string(), + enabled: true, ..AIModelConfig::default() }, AIModelConfig { id: "legacy-model".to_string(), api_key: "target-secret".to_string(), + name: "Legacy model".to_string(), + provider: "openai".to_string(), + enabled: true, custom_headers: Some(Default::default()), custom_request_body: Some(String::new()), ..AIModelConfig::default() 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 19a2684f22..b167821f15 100644 --- a/src/crates/services/legacy-migration-adapters/src/workspace_sessions.rs +++ b/src/crates/services/legacy-migration-adapters/src/workspace_sessions.rs @@ -4,6 +4,7 @@ use super::common::{ }; use openbitfun_core_types::product_identity::product_id; use openbitfun_core_types::validate_session_id; +use openbitfun_legacy_migration::copy_directory as copy_tree; use openbitfun_legacy_migration::{ atomic_write_bytes, atomic_write_json, DomainContext, DomainScan, LegacyDomainAdapter, LegacyMigrationError, LegacyMigrationResult, MigrationRoots, @@ -53,6 +54,83 @@ const SESSION_OWNED_DIRECTORIES: &[&str] = &["snapshots", "artifacts", "tool-res pub(crate) struct WorkspaceSessionsAdapter; +#[derive(Debug, Clone, Default, Serialize)] +pub struct MigrationItemCounts { + pub imported: u64, + pub skipped: u64, +} + +#[derive(Debug, Clone, Default, Serialize)] +#[serde(rename_all = "camelCase")] +pub struct WorkspaceReportCounts { + pub sessions: MigrationItemCounts, + pub workspaces: MigrationItemCounts, + pub assistant_directories: MigrationItemCounts, +} + +/// Project both old and new run manifests into user-facing entity counts. +/// Auxiliary exclusions remain in the original manifest, not in these totals. +pub fn workspace_report_counts( + roots: &MigrationRoots, + report: &openbitfun_product_domains::legacy_migration::MigrationRunReport, +) -> LegacyMigrationResult { + uuid::Uuid::parse_str(&report.run_id) + .map_err(|_| LegacyMigrationError::InvalidRequest("invalid migration run id".into()))?; + let layout = openbitfun_legacy_migration::MigrationLayout::new(roots, &report.run_id); + let root = layout.stage_root().join("workspace-sessions"); + let manifest: WorkspaceSessionsManifest = + read_bounded_json(&root, &root.join("manifest.json"))?; + let plan: openbitfun_product_domains::legacy_migration::MigrationPlan = layout + .read_json(&layout.plan_path())? + .ok_or_else(|| LegacyMigrationError::InvalidPlan("migration plan is missing".into()))?; + let counts = |actions: Vec| MigrationItemCounts { + imported: actions + .iter() + .filter(|a| **a == SessionImportAction::Import) + .count() as u64, + skipped: actions + .iter() + .filter(|a| **a != SessionImportAction::Import) + .count() as u64, + }; + let mut result = WorkspaceReportCounts { + sessions: counts(manifest.sessions.iter().map(|e| e.action).collect()), + workspaces: MigrationItemCounts { + imported: manifest.workspace_id_map.len() as u64, + skipped: 0, + }, + assistant_directories: counts( + manifest + .assistant_workspaces + .iter() + .map(|e| e.action) + .collect(), + ), + }; + let target_workspaces = plan + .conflicts + .iter() + .filter(|c| c.code == "workspace_target_wins") + .count() as u64; + result.workspaces.imported = result.workspaces.imported.saturating_sub(target_workspaces); + result.workspaces.skipped += target_workspaces; + if let Some(domain) = report + .domain_results + .iter() + .find(|r| r.domain == MigrationDomainId::WorkspaceSessions) + { + for diagnostic in &domain.warnings { + match diagnostic.code.as_str() { + "session_source_skipped" | "session_entry_skipped" => result.sessions.skipped += 1, + "workspace_item_skipped" => result.workspaces.skipped += 1, + "assistant_workspace_not_migrated" => result.assistant_directories.skipped += 1, + _ => {} + } + } + } + Ok(result) +} + #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] #[serde(rename_all = "snake_case")] pub(crate) enum SessionImportAction { @@ -339,12 +417,19 @@ impl LegacyDomainAdapter for WorkspaceSessionsAdapter { 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() - })); + 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()), + relative_path: Some(conflict.source_summary.clone()), + message: conflict.target_summary.clone(), + ..Default::default() + }), + ); let orphaned_relationships = orphaned_session_relationship_count(&manifest); if orphaned_relationships > 0 { warnings.push(MigrationDiagnostic { @@ -964,11 +1049,14 @@ fn plan_assistant_workspaces( let relative_path = PathBuf::from(file_name(&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, - ))); + Err(error) => { + conflicts.push(MigrationConflict { + domain: MigrationDomainId::WorkspaceSessions, + code: "assistant_workspace_not_migrated".into(), + source_summary: relative_display(&roots.legacy_home_root, &source_path), + target_summary: format!("Source workspace retained: {error}"), + resolution: ConflictResolution::ItemSkipped, + }); continue; } }; @@ -1616,12 +1704,34 @@ fn index_target_sessions( fn find_session_roots(home_root: &Path) -> LegacyMigrationResult> { let mut found = Vec::new(); let mut visited = 0usize; - for name in ["projects", "remote_ssh", "personal_assistant"] { - let root = home_root.join(name); - if root.exists() { - find_session_roots_recursive(&root, 0, &mut visited, &mut found)?; + // Local and assistant Sessions share projects//sessions. Workspace + // content (including personal_assistant) is not a runtime discovery root. + // Do not descend into snapshots, plugin data, or user build/dependency trees. + let projects = home_root.join("projects"); + if projects.exists() { + for runtime in child_directories(&projects)? { + visited += 1; + if visited > MAX_RUNTIME_DIRECTORIES { + return Err(LegacyMigrationError::ResourceLimit(format!( + "workspace runtime contains more than {MAX_RUNTIME_DIRECTORIES} directories" + ))); + } + let sessions = runtime.join("sessions"); + match fs::symlink_metadata(&sessions) { + Ok(_) => { + reject_linked_directory(&sessions)?; + found.push(sessions); + } + Err(error) if error.kind() == std::io::ErrorKind::NotFound => {} + Err(error) => return Err(io_error(&sessions, error)), + } } } + // SSH mirrors encode a variable number of POSIX remote path components. + let remote = home_root.join("remote_ssh"); + if remote.exists() { + find_session_roots_recursive(&remote, 0, &mut visited, &mut found)?; + } found.sort(); Ok(found) } @@ -1821,61 +1931,53 @@ fn hash_entries(mut entries: Vec<(PathBuf, Vec)>) -> String { format!("sha256:{}", hex::encode(hasher.finalize())) } -fn hash_tree_with_size(root: &Path) -> LegacyMigrationResult<(String, u64)> { +fn tree_entries(root: &Path) -> LegacyMigrationResult> { let mut entries = Vec::new(); - collect_tree_entries(root, root, 0, &mut entries)?; - let logical_bytes = entries - .iter() - .map(|(_, bytes)| bytes.len() as u64) - .sum::(); - Ok((hash_entries(entries), logical_bytes)) -} - -fn hash_tree(root: &Path) -> LegacyMigrationResult { - hash_tree_with_size(root).map(|(hash, _)| hash) -} - -fn collect_tree_entries( - root: &Path, - directory: &Path, - depth: usize, - entries: &mut Vec<(PathBuf, Vec)>, -) -> LegacyMigrationResult<()> { - if depth > MAX_RUNTIME_DEPTH { - return Err(LegacyMigrationError::ResourceLimit(format!( - "target tree depth exceeds {MAX_RUNTIME_DEPTH}: {}", - root.display() - ))); - } - reject_linked_directory(directory)?; - for entry in read_dir_sorted(directory)? { - 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)); + openbitfun_legacy_migration::visit_directory(root, |path, directory| { + if path != root { + entries.push((path.to_path_buf(), directory)); } - if metadata.is_dir() { - collect_tree_entries(root, &path, depth + 1, entries)?; - } else if metadata.is_file() { - let relative = path - .strip_prefix(root) - .map_err(|_| LegacyMigrationError::PathEscape(path.clone()))? - .to_path_buf(); - entries.push(( - relative, - fs::read(&path).map_err(|error| io_error(&path, error))?, - )); + Ok(()) + })?; + entries.sort_by(|a, b| a.0.cmp(&b.0)); + Ok(entries) +} +fn hash_tree_with_size(root: &Path) -> LegacyMigrationResult<(String, u64)> { + use std::io::Read; + let mut hasher = Sha256::new(); + let mut logical_bytes = 0u64; + let mut buffer = [0u8; 64 * 1024]; + for (path, directory) in tree_entries(root)? { + if directory { + continue; } - if entries.len() > MAX_SESSION_FILES { - return Err(LegacyMigrationError::ResourceLimit(format!( - "target tree contains more than {MAX_SESSION_FILES} files: {}", - root.display() - ))); + let relative = path + .strip_prefix(root) + .map_err(|_| LegacyMigrationError::PathEscape(path.clone()))?; + hasher.update(relative.to_string_lossy().replace('\\', "/").as_bytes()); + hasher.update([0]); + let mut file = fs::File::open(&path).map_err(|error| io_error(&path, error))?; + loop { + let read = file + .read(&mut buffer) + .map_err(|error| io_error(&path, error))?; + if read == 0 { + break; + } + hasher.update(&buffer[..read]); + logical_bytes += read as u64; } + hasher.update([0]); } - Ok(()) + Ok(( + format!("sha256:{}", hex::encode(hasher.finalize())), + logical_bytes, + )) } +fn hash_tree(root: &Path) -> LegacyMigrationResult { + hash_tree_with_size(root).map(|(hash, _)| hash) +} fn require_tree_hash(path: &Path, expected: &str) -> LegacyMigrationResult<()> { let actual = hash_tree(path)?; if actual != expected { @@ -1938,27 +2040,6 @@ fn install_directory_idempotent( install_result } -fn copy_tree(source: &Path, target: &Path) -> LegacyMigrationResult<()> { - reject_linked_directory(source)?; - fs::create_dir_all(target).map_err(|error| io_error(target, error))?; - for entry in read_dir_sorted(source)? { - let source_path = entry.path(); - let target_path = target.join(entry.file_name()); - let metadata = - fs::symlink_metadata(&source_path).map_err(|error| io_error(&source_path, error))?; - if metadata.file_type().is_symlink() || is_reparse_point(&metadata) { - return Err(LegacyMigrationError::LinkedPath(source_path)); - } - if metadata.is_dir() { - copy_tree(&source_path, &target_path)?; - } else if metadata.is_file() { - let bytes = fs::read(&source_path).map_err(|error| io_error(&source_path, error))?; - atomic_write_bytes(&target_path, &bytes)?; - } - } - Ok(()) -} - fn install_file_idempotent( staged: &Path, target: &Path, @@ -2231,6 +2312,48 @@ mod tests { .starts_with("remote_")); } + #[test] + fn streamed_tree_hash_preserves_the_existing_manifest_format() { + let temp = test_tempdir("tree-hash"); + let entries = vec![ + (PathBuf::from("z"), vec![42; 150_000]), + (PathBuf::from("a/b"), b"hello".to_vec()), + ]; + for (path, bytes) in &entries { + atomic_write_bytes(&temp.path().join(path), bytes).unwrap(); + } + let (hash, size) = hash_tree_with_size(temp.path()).unwrap(); + assert_eq!(hash, hash_entries(entries)); + assert_eq!(size, 150_005); + } + + #[test] + fn session_discovery_ignores_workspace_content_and_runtime_artifacts() { + let temp = test_tempdir("session-discovery"); + let home = temp.path(); + let expected = vec![ + home.join("projects/assistant/sessions"), + home.join("projects/local/sessions"), + home.join("remote_ssh/host/srv/nested/repo/sessions"), + ]; + for path in &expected { + fs::create_dir_all(path).unwrap(); + } + for relative in [ + "personal_assistant/workspace/build", + "projects/local/snapshots", + "projects/local/plugin-runtime", + ] { + let mut deep = home.join(relative); + for _ in 0..=MAX_RUNTIME_DEPTH { + deep.push("d"); + } + // A user directory named sessions is not product Session storage. + fs::create_dir_all(deep.join("sessions")).unwrap(); + } + assert_eq!(find_session_roots(home).unwrap(), expected); + } + #[test] fn assistant_runtime_slug_is_stable_before_and_after_target_creation() { let temp = test_tempdir("assistant-slug"); diff --git a/src/crates/services/legacy-migration/AGENTS.md b/src/crates/services/legacy-migration/AGENTS.md index 871c650cb5..cbe1574353 100644 --- a/src/crates/services/legacy-migration/AGENTS.md +++ b/src/crates/services/legacy-migration/AGENTS.md @@ -7,6 +7,11 @@ new standalone tasks use save_task/load_task and do not depend on request expiry Do not delete or reset corrupt plans, reports or user data. Validate UUIDs and plan identities before using task paths. Keep source/destination isolation. +`directory` owns ordinary tree traversal and OS-backed file copying for staging. +Workspace trees have no product-specific depth/count/size admission limits. +Adapters own conversion, conflicts and manifest verification; the engine retains +phase progress and cancellation boundaries. Links remain explicit errors. + Process inspection must fail explicitly when inventory is unavailable. Never interpret an inspection error as an empty writer list. On macOS, inspect executable names with the system `ps`, preserving full bundle paths and excluding arguments. diff --git a/src/crates/services/legacy-migration/src/directory.rs b/src/crates/services/legacy-migration/src/directory.rs new file mode 100644 index 0000000000..9b9743c665 --- /dev/null +++ b/src/crates/services/legacy-migration/src/directory.rs @@ -0,0 +1,133 @@ +//! Ordinary directory IO for migration staging. No product/schema inspection. +use crate::{LegacyMigrationError, LegacyMigrationResult}; +use std::fs; +use std::path::Path; + +fn io_error(path: &Path, source: std::io::Error) -> LegacyMigrationError { + LegacyMigrationError::Io { + path: path.into(), + source, + } +} + +/// Visit directories and regular files without content, depth, or count limits. +/// Links remain explicit errors: migration must not follow content outside its +/// declared source. Callers own conflict handling, cancellation and publication. +pub fn visit_directory( + root: &Path, + mut visit: impl FnMut(&Path, bool) -> LegacyMigrationResult<()>, +) -> LegacyMigrationResult<()> { + let mut pending = vec![root.to_path_buf()]; + while let Some(path) = pending.pop() { + let metadata = fs::symlink_metadata(&path).map_err(|e| io_error(&path, e))?; + #[cfg(windows)] + let reparse = { + use std::os::windows::fs::MetadataExt; + metadata.file_attributes() & 0x0400 != 0 + }; + #[cfg(not(windows))] + let reparse = false; + if metadata.file_type().is_symlink() || reparse { + return Err(LegacyMigrationError::LinkedPath(path)); + } + if !metadata.is_dir() && !metadata.is_file() { + return Err(LegacyMigrationError::UnsupportedSource(format!( + "unsupported filesystem entry: {}", + path.display() + ))); + } + visit(&path, metadata.is_dir())?; + if metadata.is_dir() { + for entry in fs::read_dir(&path).map_err(|e| io_error(&path, e))? { + pending.push(entry.map_err(|e| io_error(&path, e))?.path()); + } + } + } + Ok(()) +} + +/// Copy a source tree to an isolated staging directory using OS-backed file IO. +/// The migration layout must keep destination outside source; the caller owns +/// hash validation and atomic directory publication/recovery. +pub fn copy_directory(source: &Path, target: &Path) -> LegacyMigrationResult<()> { + let source_root = fs::canonicalize(source).map_err(|e| io_error(source, e))?; + if !source_root.is_dir() { + return Err(LegacyMigrationError::UnsupportedSource(format!( + "expected directory: {}", + source.display() + ))); + } + // Resolve the existing destination ancestor before creating anything, so an + // accidental destination inside source cannot recursively copy its own output. + let mut ancestor = target; + let mut suffix = Vec::new(); + while !ancestor.exists() { + let name = ancestor + .file_name() + .ok_or_else(|| LegacyMigrationError::PathEscape(target.into()))?; + suffix.push(name); + ancestor = ancestor + .parent() + .filter(|p| !p.as_os_str().is_empty()) + .unwrap_or(Path::new(".")); + } + let mut destination_root = fs::canonicalize(ancestor).map_err(|e| io_error(ancestor, e))?; + for name in suffix.into_iter().rev() { + destination_root.push(name); + } + if destination_root.starts_with(&source_root) || source_root.starts_with(&destination_root) { + return Err(LegacyMigrationError::PathEscape(target.into())); + } + visit_directory(source, |path, directory| { + let relative = path + .strip_prefix(source) + .map_err(|_| LegacyMigrationError::PathEscape(path.into()))?; + let destination = target.join(relative); + if directory { + fs::create_dir_all(&destination).map_err(|e| io_error(&destination, e))?; + } else { + fs::copy(path, &destination).map_err(|e| io_error(&destination, e))?; + } + Ok(()) + }) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn copies_large_deep_trees_and_empty_directories_without_size_gates() { + let temp = tempfile::tempdir().unwrap(); + let source = temp.path().join("source"); + fs::create_dir_all(&source).unwrap(); + // Exceed the former workspace entry ceiling; these are ordinary files. + for index in 0..32_769 { + fs::write(source.join(format!("file-{index}")), []).unwrap(); + } + let deep = (0..24).fold(source.clone(), |path, _| path.join("d")); + fs::create_dir_all(deep.join("empty")).unwrap(); + fs::write(deep.join("content"), b"copied unchanged").unwrap(); + let target = temp.path().join("target"); + copy_directory(&source, &target).unwrap(); + assert!(target.join("file-32768").is_file()); + let relative = deep.strip_prefix(&source).unwrap(); + assert!(target.join(relative).join("empty").is_dir()); + assert_eq!( + fs::read(target.join(relative).join("content")).unwrap(), + b"copied unchanged" + ); + assert_eq!(fs::read(deep.join("content")).unwrap(), b"copied unchanged"); + } + + #[test] + fn refuses_overlapping_copy_before_creating_output() { + let temp = tempfile::tempdir().unwrap(); + let target = temp.path().join("nested/output"); + assert!(matches!( + copy_directory(temp.path(), &target), + Err(LegacyMigrationError::PathEscape(_)) + )); + assert!(!target.exists()); + } +} diff --git a/src/crates/services/legacy-migration/src/lib.rs b/src/crates/services/legacy-migration/src/lib.rs index ad1e316f42..12d3dabb8d 100644 --- a/src/crates/services/legacy-migration/src/lib.rs +++ b/src/crates/services/legacy-migration/src/lib.rs @@ -1,6 +1,7 @@ //! Offline, source-read-only import primitives for legacy BitFun data. mod diagnostics; +mod directory; mod engine; mod error; mod handoff; @@ -12,6 +13,7 @@ mod storage; mod tasks; pub use diagnostics::{export_failure_diagnostics, release_observation}; +pub use directory::{copy_directory, visit_directory}; pub use engine::{ compute_plan_hash, CancellationToken, CrashInjector, CrashPoint, DomainContext, DomainScan, LegacyDomainAdapter, MigrationEngine, NoCrashInjection,