From 5a85409e8eb86dc3e4fbd7c5ad5a2faf191d8f4d Mon Sep 17 00:00:00 2001 From: limityan Date: Mon, 13 Jul 2026 23:26:46 +0800 Subject: [PATCH] fix(plugins): clean residual activation state --- .../plugin-runtime-host-design.md | 2 +- docs/architecture/product-architecture.md | 5 +- docs/plans/core-decomposition-completed.md | 1 + docs/plans/core-decomposition-plan.md | 18 +- .../rules/source/forbidden-rules.mjs | 4 +- .../rules/source/public-api-rules.mjs | 6 +- scripts/core-boundaries/self-test.mjs | 4 +- src/apps/cli/src/management.rs | 135 +++- src/apps/cli/tests/plugin_source_cli.rs | 150 +++- .../tests/opencode_source_adapter.rs | 8 +- .../assembly/core/src/plugin_runtime.rs | 250 ++++--- .../product-domains/src/plugin_source.rs | 99 ++- .../tests/plugin_source_contracts.rs | 121 +++- .../src/plugin_source.rs | 682 ++++++++++++++---- 14 files changed, 1158 insertions(+), 327 deletions(-) diff --git a/docs/architecture/plugin-runtime-host-design.md b/docs/architecture/plugin-runtime-host-design.md index 9851bcebb7..05085c60d4 100644 --- a/docs/architecture/plugin-runtime-host-design.md +++ b/docs/architecture/plugin-runtime-host-design.md @@ -186,7 +186,7 @@ sequenceDiagram - 主机不得伪造权限通过、工具成功、审计成功或产品状态变更。 - `HostRestarted` 是 P0-B 唯一隔离清除条件；用户可执行清除、重试、重新审核和打开日志等动作必须在具有归属接口、审计事实和真实消费方后再暴露。 - `restart(project_domain_id, workspace_id)` 是内部清理路径，用于清除对应执行域的隔离、诊断只读视图和幂等缓存。 -- 后续显式停用或清理残留激活记录时，必须按项目、工作区、包和可选激活代次定位记录；包已缺失或损坏时不能要求重新读取包内容。清理激活状态不等于删除来源审核历史。 +- 显式停用按项目、工作区和包清除激活记录，内部回滚同时校验激活代次。停用记录先写入信任文件，再通过受限发现判断包是否可用；发现同 ID 的不同来源时，使旧来源审核记录失效。包缺失、损坏或扫描不完整时仍可停用，并保留无法安全协调的来源审核历史。后续发现问题作为诊断返回，持久化结果不确定时返回错误；重复操作和旧代次请求不改变状态。 ## 7. 目录与来源原则 diff --git a/docs/architecture/product-architecture.md b/docs/architecture/product-architecture.md index 57bbf71c4f..837f6ae3ae 100644 --- a/docs/architecture/product-architecture.md +++ b/docs/architecture/product-architecture.md @@ -164,13 +164,14 @@ flowchart LR | 随产品携带包 | 未建立独立扫描根 | 由构建配置、安装器和产品组装提供来源后接入同一校验接口 | | OpenCode 兼容内容 | 包清单可声明 `opencode_compatible`；来源模块重新校验并固定声明文件，适配器只解释该输入 | 外部目录需经独立导入流程转换为受管包 | | 来源审核 | 工作区 `SourceApproved`、`Denied`、`Revoked`；内容变化使旧审核与激活失效 | 组织策略、签名和撤销列表 | -| 激活 | 预览后按精确内容哈希确认；激活代次与来源审核代次独立；停用或内容变化立即使既有 Binding 失效 | GUI/Web 管理入口、组织策略，以及包缺失或损坏后的显式记录清理体验 | +| 激活 | 预览后按精确内容哈希确认；激活代次与来源审核代次独立；停用或内容变化立即使既有 Binding 失效；CLI 可清理缺失或损坏包的残留激活记录 | GUI/Web 管理入口和组织策略 | | 插件运行 | 只通过 Host 读取 custom tool 候选；候选始终需要权限；不执行 JS/TS，不注册最终工具 | 先建立受限执行单元和真实工具提供方，再接入工具快照与权限裁决；执行能力不可用时只返回诊断 | OpenCode 适配接入规则： - OpenCode 适配器的公开入口只接收来源服务重新校验并固定的受管包输入，不直接扫描工作区或用户 OpenCode 目录。 -- 来源服务只为当前 `SourceApproved` 的包生成固定内容输入。每条激活记录保存自己的签发代次；激活授权信息只包含项目、工作区、精确来源和该代次，包内容不在授权信息中重复保存，其他包的状态变化不会使当前授权失效。 +- 来源服务只为当前 `SourceApproved` 的包生成固定内容输入。每条激活记录保存自己的签发代次；激活授权信息只包含项目、工作区、精确来源和该代次，包内容不在授权信息中重复保存，其他包的状态变化不会使当前授权失效。包暂时缺失或损坏时保留来源审核记录；发现同 ID 的不同来源后，旧审核与激活失效。 +- 停用按项目、工作区和包清除激活记录，内部回滚同时校验激活代次。停用记录先写入信任文件，再通过受限发现判断包是否可用；发现同 ID 的不同来源时，使旧来源审核记录失效。包缺失、损坏或扫描不完整时仍可停用，并保留无法安全协调的来源审核历史。后续发现问题作为诊断返回，持久化结果不确定时返回错误；实际清除推进一次激活代次，重复操作和旧代次请求不改变状态。 - 固定内容输入只保证结构、大小和哈希自洽，不作为审核凭据。生产组装必须从来源服务取得输入；即使其他进程内调用方构造了有效输入，适配器仍只能返回未激活状态。 - Host 来源 URI 使用来源模块生成的路径摘要区分用户级包、项目级包和后续其他来源，不暴露原始本地路径。 - 普通包输入只能产生来源与诊断视图。只有产品组装点持有来源服务生成的当前激活授权信息时，适配器才将受支持 custom tool 映射为权限候选。 diff --git a/docs/plans/core-decomposition-completed.md b/docs/plans/core-decomposition-completed.md index 969fc77ff9..273a73f169 100644 --- a/docs/plans/core-decomposition-completed.md +++ b/docs/plans/core-decomposition-completed.md @@ -32,6 +32,7 @@ - 产品组装已承接 `DeliveryProfile`、当前交付形态入口矩阵、`CapabilitySet`、feature group matrix、profile-scoped capability plan、product-full provider plan、service availability report、profile-scoped harness registry 入口与 legacy-route 行为保护，以及 `ProductAssembler` 对 explicit profile input、runtime services、harness registry 和 service requirement 的验证；core 只保留兼容 re-export。ProductFull / Desktop / CLI / ACP 保留完整能力；Server / Remote / Web / MobileWeb 不再 materialize product-full capability packs、feature groups、runtime services、tool groups 或 harness routes。 - 插件运行时边界基础已建立：`runtime-ports` 持有 `PluginRuntimeClient`、binding、availability、dispatch / response envelope、disabled stub 和 projection-only stub；产品组装输出扩展可用性事实与插件运行时绑定，并通过 Agent Runtime 内部 builder 注入该 binding；Agent Runtime SDK 门面不导出插件运行时主机 ABI。默认产品启动不运行 JS/TS、工作进程或子进程。 - OpenCode-compatible P0-C.1/P0-C.2 已建立受管包发现、完整性校验、工作区来源审核、精确内容哈希激活、CLI 管理与诊断，以及按需创建 OpenCode 适配器、插件运行时主机和 `PluginRuntimeBinding` 的唯一生产组装点。当前组装只返回需要权限的 custom tool 静态候选，不注册工具或执行插件代码。 +- 插件停用已支持按工作区和包清理缺失或损坏包的残留激活记录；停用状态在扫描前提交，后续受限发现负责结果分类，并在稳定发现同 ID 不同来源时协调旧审核记录。包暂时缺失或损坏时保留来源审核记录，重复操作和旧激活代次请求保持幂等，持久化结果不确定时不报告成功。 - LSP plugin runtime target 和命令占位符解析已从 `services-core` 收口到 `core-types`；`services-core` 保留兼容 re-export、registry、current-target detection 和 filesystem / runtime service 逻辑。 - Agent session/workspace owner routing 已继续收敛：`AgentRuntime` 提供 port-backed session workspace resolution entrypoint；Cron、SessionControl、SessionMessage 和 SessionHistory 不再在工具实现中直接解析目标 session workspace，Cron 保留 target session 可见性验证，workspace identity 中的 `workspace_id` / remote connection / remote host 通过 runtime contract 传递。 diff --git a/docs/plans/core-decomposition-plan.md b/docs/plans/core-decomposition-plan.md index a0f49cf702..f3979713b7 100644 --- a/docs/plans/core-decomposition-plan.md +++ b/docs/plans/core-decomposition-plan.md @@ -24,13 +24,12 @@ - `runtime-ports` 的插件主机 ABI 已有公开接口预算脚本；后续不能绕过预算新增插件、hook、event、UI 或生态兼容对象。 - `opencode-adapter` 当前解释固定内容的受管包，提供诊断只读视图和 custom tool 候选映射；来源发现归 `services-integrations/plugin_source`。 - `services-integrations/plugin_source` 提供受管包发现、完整性校验、来源审核、激活持久化和实时凭证复核；`bitfun-core/plugin_runtime` 是唯一 OpenCode 生产组装点。 -- CLI 已能预览、按精确内容哈希激活和停用包。激活路径只通过 Plugin Runtime Host 返回需要权限的 custom tool 候选，不执行 JS/TS，也不依赖外部 OpenCode CLI。 +- CLI 已能预览、按精确内容哈希激活和停用包，并可在包缺失或损坏后清理残留激活记录。激活路径只通过 Plugin Runtime Host 返回需要权限的 custom tool 候选，不执行 JS/TS，也不依赖外部 OpenCode CLI。 ## 3. 当前差距 | 差距 | 影响 | 收敛要求 | |---|---|---| -| 激活记录依赖当前包内容清理 | 包缺失或损坏后，普通停用路径无法读取目标来源，可能留下用户无法清理的激活记录 | 增加按工作区、包和可选激活代次清理记录的产品操作；清理不依赖重新读取包内容，也不删除来源审核历史 | | 激活写入在跨进程锁内执行稳定性复核 | 操作受统一期限约束，慢文件系统可能延长同一工作区的授权检查等待，但当前没有等待时间基线 | 先增加锁等待与慢文件系统测试；只有数据证明存在问题时再调整锁范围，同时保持来源、优先级、内容摘要和审核代次在提交点一致 | | custom tool 只有静态候选，没有执行实现 | 候选不能形成可调用工具；直接注册会向模型暴露无法执行的伪工具 | 先完成一种明确制品的受限执行单元和真实工具提供方，再复用现有工具 ABI、权限与陈旧快照保护；执行不可用时只返回诊断 | | 运行时插件没有安装和卸载流程 | 用户只能手工放置包，无法形成完整的动态插件体验 | 安装、卸载和状态清理作为独立产品流程；安装不自动审核或激活，也不复制外部生态凭据和配置批准 | @@ -46,21 +45,12 @@ | 插件运行时主机 | `availability`、`read_plugins`、`dispatch`，以及期限、代次、幂等、隔离、诊断和重启清理 | | P0-C.1 | 受管包发现、完整性校验、工作区来源审核、CLI 管理和诊断 | | P0-C.2 | 精确内容哈希激活、生产组装、OpenCode custom tool 静态候选和权限提示 | +| 停用恢复 | 按工作区和包清理缺失或损坏包的残留激活记录；保留来源审核历史并提供幂等、代次和持久化保护 | 上述基线不包含插件代码执行、工具注册、安装卸载、产品内置来源或外部 OpenCode 目录导入。详细完成事实归档在 [`core-decomposition-completed.md`](core-decomposition-completed.md)。 ## 5. 后续 PR 顺序与范围 -### PR1：残留激活记录清理 - -范围： - -- 包缺失或损坏时，按工作区、包和可选激活代次清理残留激活记录；不要求重新读取包内容。 -- CLI 停用和诊断明确区分“包已停用”“残留记录已清理”和“持久化结果不确定”。 -- 只增加上述流程真实消费的最小操作，不增加通用注册中心、管理器或新的插件状态模型。 - -完成条件：缺失、损坏、旧激活代次、重复清理和持久化失败均有测试；失败时不误报停用成功，不删除来源审核历史。 - ### PR2：首个可执行 custom tool 范围： @@ -99,13 +89,13 @@ 完成条件：先确认目标用户和一个实际 CLI 任务，再验证构建产物中的真实内置扩展可完成该任务；Manifest 摘要、签名凭据和产品策略均通过校验。缺失、摘要或签名不匹配、同 ID 冲突和运行期隔离均产生可观察降级，且不会读取用户插件审核或激活状态。演示工具不能作为阶段完成依据。 -以上四个 PR 均不包含在线插件仓库、隐式 npm 安装、原始 OpenCode 目录批量导入、可写钩子、GUI/TUI 通用界面接口、Server/Remote 执行或 Codex/Claude 插件运行时。这些能力必须在出现独立产品场景和真实消费方后重新排期。 +以上三个后续 PR 均不包含在线插件仓库、隐式 npm 安装、原始 OpenCode 目录批量导入、可写钩子、GUI/TUI 通用界面接口、Server/Remote 执行或 Codex/Claude 插件运行时。这些能力必须在出现独立产品场景和真实消费方后重新排期。 代码合入不等于产品能力已经发布。只有执行路径与对应来源路径都形成完整闭环后，产品入口才能显示“可用插件”；仅完成候选、安装或内置来源时，入口必须保持候选预览或 `projection-only`，并明确显示执行能力不可用。 ## 6. 待确认决策 -PR1 不依赖以下决策。技术依赖为：PR3 依赖 PR1 的残留记录清理，并在作为可用插件体验发布前依赖 PR2；PR4 依赖 PR2 以及独立交付的 Customization-C0/C2 最小前置能力，PR3 不是其前置。开始 PR2 前必须确认执行载体、主机调用入口和首个真实工具场景。 +残留激活记录清理已完成，不依赖以下决策。技术依赖为：PR3 在作为可用插件体验发布前依赖 PR2；PR4 依赖 PR2 以及独立交付的 Customization-C0/C2 最小前置能力，PR3 不是其前置。开始 PR2 前必须确认执行载体、主机调用入口和首个真实工具场景。 | 决策 | 方案 | 主要影响 | 建议 | |---|---|---|---| diff --git a/scripts/core-boundaries/rules/source/forbidden-rules.mjs b/scripts/core-boundaries/rules/source/forbidden-rules.mjs index bc69faa734..b58626d113 100644 --- a/scripts/core-boundaries/rules/source/forbidden-rules.mjs +++ b/scripts/core-boundaries/rules/source/forbidden-rules.mjs @@ -32,7 +32,7 @@ export const forbiddenContentRules = [ patterns: [ { regex: - /\bpub\s+(?:async\s+)?fn\s+(?!(?:new|refresh|set_trust|load_package|set_activation|load_activated_package|has_activation_authority)\b)[A-Za-z_][A-Za-z0-9_]*\b/, + /\bpub\s+(?:async\s+)?fn\s+(?!(?:new|refresh|set_trust|load_package|activate|deactivate|load_activated_package|has_activation_authority)\b)[A-Za-z_][A-Za-z0-9_]*\b/, message: 'unexpected public ManagedPluginSourceService method; update the reviewed method budget before exposing more API', }, @@ -45,7 +45,7 @@ export const forbiddenContentRules = [ patterns: [ { regex: - /\bpub\s+(?:const\s+)?fn\s+(?!(?:parse_json|validate|content_hash|new|into_parts|epoch|activation_epoch|trust_level_for|apply_decision|reconcile_sources|is_activated|activation_authority|is_activation_current|set_activation)\b)[A-Za-z_][A-Za-z0-9_]*\b/, + /\bpub\s+(?:const\s+)?fn\s+(?!(?:parse_json|validate|content_hash|new|into_parts|epoch|activation_epoch|activation_sources|trust_level_for|apply_decision|reconcile_sources|is_activated|activation_authority|is_activation_current|activate|clear_activation_record)\b)[A-Za-z_][A-Za-z0-9_]*\b/, message: 'unexpected public plugin source contract method; update the reviewed method budget before exposing more API', }, diff --git a/scripts/core-boundaries/rules/source/public-api-rules.mjs b/scripts/core-boundaries/rules/source/public-api-rules.mjs index 076a08dc6d..ae927f0903 100644 --- a/scripts/core-boundaries/rules/source/public-api-rules.mjs +++ b/scripts/core-boundaries/rules/source/public-api-rules.mjs @@ -232,8 +232,10 @@ export const managedPluginSourcePublicApiEntries = [ export const managedPluginActivationPublicApiEntries = [ 'ManagedPluginCandidateView', 'ManagedPluginActivationView', + 'ManagedPluginDeactivationResult', 'preview_managed_plugin_activation', - 'set_managed_plugin_activation', + 'activate_managed_plugin', + 'deactivate_managed_plugin', ].map((symbol) => pluginSourceEntry( symbol, @@ -308,7 +310,7 @@ export const publicApiAllowlistRules = [ { path: 'src/crates/assembly/core/src/plugin_runtime.rs', reason: - 'core managed plugin activation API must stay limited to product status projection and one state transition', + 'core managed plugin activation API must stay limited to product status projection and explicit activation or deactivation transitions', allowedSymbolEntries: managedPluginActivationPublicApiEntries, }, ]; diff --git a/scripts/core-boundaries/self-test.mjs b/scripts/core-boundaries/self-test.mjs index 3d3446e595..f2214f4efb 100644 --- a/scripts/core-boundaries/self-test.mjs +++ b/scripts/core-boundaries/self-test.mjs @@ -980,9 +980,9 @@ export function runManifestParserSelfTest({ (managedPluginActivationPublicApiRule?.allowedSymbolEntries || []) .map((entry) => entry.symbol) .join(',') !== - 'ManagedPluginCandidateView,ManagedPluginActivationView,preview_managed_plugin_activation,set_managed_plugin_activation' + 'ManagedPluginCandidateView,ManagedPluginActivationView,ManagedPluginDeactivationResult,preview_managed_plugin_activation,activate_managed_plugin,deactivate_managed_plugin' ) { - throw new Error('managed plugin activation API budget must stay limited to four product-facing symbols'); + throw new Error('managed plugin activation API budget must stay limited to six product-facing symbols'); } const appHostAbiRule = forbiddenContentUnderRules.find((rule) => rule.path === 'src/apps'); if (!appHostAbiRule) { diff --git a/src/apps/cli/src/management.rs b/src/apps/cli/src/management.rs index eb0222bbad..3c0b7fb7e5 100644 --- a/src/apps/cli/src/management.rs +++ b/src/apps/cli/src/management.rs @@ -6,11 +6,13 @@ use bitfun_core::agentic::get_agent_registry; use bitfun_core::agentic::persistence::PersistenceManager; use bitfun_core::infrastructure::try_get_path_manager_arc; use bitfun_core::plugin_runtime::{ - preview_managed_plugin_activation, set_managed_plugin_activation, ManagedPluginActivationView, + activate_managed_plugin, deactivate_managed_plugin, preview_managed_plugin_activation, + ManagedPluginActivationView, ManagedPluginDeactivationResult, }; use bitfun_core::plugin_source::{ - refresh_managed_plugin_sources, set_managed_plugin_trust, ManagedPluginSourceSnapshot, - ManagedPluginTrustDecision, ManagedPluginTrustLevel, + refresh_managed_plugin_sources, set_managed_plugin_trust, ManagedPluginSourceError, + ManagedPluginSourceIssue, ManagedPluginSourceSnapshot, ManagedPluginTrustDecision, + ManagedPluginTrustLevel, }; use bitfun_core::service::config::initialize_global_config; use bitfun_core::service::session_usage::{ @@ -328,7 +330,7 @@ pub(crate) async fn set_plugin_trust( pub(crate) async fn activate_plugin(package_id: &str, confirm: Option<&str>) -> Result<()> { let workspace = std::env::current_dir().context("Failed to resolve current directory")?; let view = if let Some(content_hash) = confirm { - set_managed_plugin_activation(&workspace, package_id, true, Some(content_hash)).await + activate_managed_plugin(&workspace, package_id, Some(content_hash)).await } else { preview_managed_plugin_activation(&workspace, package_id).await } @@ -359,27 +361,88 @@ pub(crate) async fn activate_plugin(package_id: &str, confirm: Option<&str>) -> pub(crate) async fn deactivate_plugin(package_id: &str) -> Result<()> { let workspace = std::env::current_dir().context("Failed to resolve current directory")?; - let view = set_managed_plugin_activation(&workspace, package_id, false, None) + let result = deactivate_managed_plugin(&workspace, package_id) .await .map_err(|error| { - anyhow!(crate::plugin_diagnostics::escape_terminal_text( - &error.to_string() - )) + let diagnostic = crate::plugin_diagnostics::escape_terminal_text(&error.to_string()); + if matches!( + error, + ManagedPluginSourceError::DeactivationPersistenceUncertain { .. } + ) { + anyhow!( + "{diagnostic}\nThe saved state may already be cleared. Retry `bitfun-cli plugins deactivate {}` to confirm the result; the operation is idempotent.", + crate::plugin_diagnostics::escape_terminal_text(package_id) + ) + } else { + anyhow!(diagnostic) + } })?; - println!( - "Plugin package {} is inactive.", - crate::plugin_diagnostics::escape_terminal_text(&view.package_id) - ); - for diagnostic in &view.diagnostics { - println!( - "- [warning] {}", - crate::plugin_diagnostics::escape_terminal_text(diagnostic) - ); + match result { + ManagedPluginDeactivationResult::Deactivated { + package_id, + diagnostics, + } => { + let package_id = crate::plugin_diagnostics::escape_terminal_text(&package_id); + println!("Plugin package {package_id} was deactivated."); + print_deactivation_diagnostics(&diagnostics); + } + ManagedPluginDeactivationResult::ResidualActivationCleared { + package_id, + current_package_available, + diagnostics, + } => { + let package_id = crate::plugin_diagnostics::escape_terminal_text(&package_id); + match current_package_available { + Some(true) => println!( + "Plugin package {package_id} previous source's saved activation state was cleared; the current package was not active." + ), + Some(false) => println!( + "Plugin package {package_id} is unavailable; its saved activation state was cleared." + ), + None => println!( + "Plugin package {package_id} saved activation state was cleared; current package availability could not be determined." + ), + } + print_deactivation_diagnostics(&diagnostics); + } + ManagedPluginDeactivationResult::AlreadyInactive { + package_id, + current_package_available, + diagnostics, + } => { + let package_id = crate::plugin_diagnostics::escape_terminal_text(&package_id); + match current_package_available { + Some(true) => println!("Plugin package {package_id} was already inactive."), + Some(false) => println!( + "Plugin package {package_id} is unavailable and has no saved activation state." + ), + None => println!( + "Plugin package {package_id} has no saved activation state; current package availability could not be determined." + ), + } + print_deactivation_diagnostics(&diagnostics); + } } println!("No plugin code or candidate effect was executed."); Ok(()) } +fn print_deactivation_diagnostics(diagnostics: &[ManagedPluginSourceIssue]) { + for diagnostic in diagnostics { + println!("- {}", render_plugin_source_issue(diagnostic)); + } +} + +fn render_plugin_source_issue(issue: &ManagedPluginSourceIssue) -> String { + format!( + "[{}:{}] {}: {}", + if issue.is_error { "error" } else { "warn" }, + crate::plugin_diagnostics::escape_terminal_text(&issue.code), + crate::plugin_diagnostics::escape_terminal_text(&issue.source_path), + crate::plugin_diagnostics::escape_terminal_text(&issue.message) + ) +} + fn print_plugin_activation(view: &ManagedPluginActivationView, preview: bool) { println!( "Plugin activation {}", @@ -478,7 +541,11 @@ fn print_plugin_snapshot(snapshot: &ManagedPluginSourceSnapshot) { crate::plugin_diagnostics::escape_terminal_text(&package.package_id), crate::plugin_diagnostics::escape_terminal_text(&package.version), package.source_scope, - plugin_trust_label(package.trust_level), + if snapshot.discovery_complete { + plugin_trust_label(package.trust_level) + } else { + "review state unavailable" + }, ); println!( " Source: {}", @@ -491,7 +558,9 @@ fn print_plugin_snapshot(snapshot: &ManagedPluginSourceSnapshot) { println!(" Content hash: {}", package.content_hash); println!( " Activation: {}", - if package.activated { + if !snapshot.discovery_complete { + "unknown; source discovery is incomplete" + } else if package.activated { "active for candidate projection; plugin code is not executed" } else { "inactive; source review does not activate this package" @@ -499,13 +568,7 @@ fn print_plugin_snapshot(snapshot: &ManagedPluginSourceSnapshot) { ); } for issue in &snapshot.issues { - println!( - "- [{}:{}] {}: {}", - if issue.is_error { "error" } else { "warn" }, - crate::plugin_diagnostics::escape_terminal_text(&issue.code), - crate::plugin_diagnostics::escape_terminal_text(&issue.source_path), - crate::plugin_diagnostics::escape_terminal_text(&issue.message) - ); + println!("- {}", render_plugin_source_issue(issue)); } println!( "{}", @@ -606,19 +669,19 @@ pub(crate) async fn print_doctor() -> Result { plugin_error_count, ) ); - println!( - "[ok] Managed plugin source integrity checked; {} active. Candidate projection was not probed.", - active_plugin_count - ); - for issue in plugin_sources.issues.iter().take(10) { + if plugin_sources.discovery_complete { + println!( + "[ok] Managed plugin source integrity checked; {} active. Candidate projection was not probed.", + active_plugin_count + ); + } else { println!( - " - [{}:{}] {}: {}", - if issue.is_error { "error" } else { "warn" }, - crate::plugin_diagnostics::escape_terminal_text(&issue.code), - crate::plugin_diagnostics::escape_terminal_text(&issue.source_path), - crate::plugin_diagnostics::escape_terminal_text(&issue.message) + "[error] Managed plugin source scan is incomplete; review and activation status are unavailable. Candidate projection was not probed." ); } + for issue in plugin_sources.issues.iter().take(10) { + println!(" - {}", render_plugin_source_issue(issue)); + } if plugin_sources.issues.len() > 10 { println!( " - {} additional plugin diagnostics omitted", diff --git a/src/apps/cli/tests/plugin_source_cli.rs b/src/apps/cli/tests/plugin_source_cli.rs index 42509d1b09..2a159582cf 100644 --- a/src/apps/cli/tests/plugin_source_cli.rs +++ b/src/apps/cli/tests/plugin_source_cli.rs @@ -73,6 +73,30 @@ fn activation_content_hash(output: &Output) -> String { .to_string() } +fn activate_package(workspace: &Path, user_root: &Path, home_root: &Path) { + let preview = run_cli( + workspace, + user_root, + home_root, + &["plugins", "activate", "acme.demo"], + ); + assert!(preview.status.success(), "{}", stderr(&preview)); + let content_hash = activation_content_hash(&preview); + let activate = run_cli( + workspace, + user_root, + home_root, + &[ + "plugins", + "activate", + "acme.demo", + "--confirm", + &content_hash, + ], + ); + assert!(activate.status.success(), "{}", stderr(&activate)); +} + fn find_trust_file(root: &Path) -> Option { let entries = std::fs::read_dir(root).ok()?; for entry in entries.flatten() { @@ -204,7 +228,7 @@ fn plugin_source_cli_lifecycle_and_doctor_exit_codes() { &["plugins", "deactivate", "acme.demo"], ); assert!(deactivate.status.success(), "{}", stderr(&deactivate)); - assert!(stdout(&deactivate).contains("is inactive")); + assert!(stdout(&deactivate).contains("was deactivated")); let healthy = run_cli(&workspace, &user_root, &home_root, &["doctor"]); assert!(healthy.status.success(), "{}", stderr(&healthy)); @@ -256,3 +280,127 @@ fn plugin_source_cli_lifecycle_and_doctor_exit_codes() { assert_eq!(unhealthy.status.code(), Some(1), "{}", stderr(&unhealthy)); assert!(stdout(&unhealthy).contains("hash_mismatch")); } + +#[test] +fn plugin_deactivate_cleans_residual_records_without_revoking_source_approval() { + let temp = tempfile::tempdir().expect("tempdir"); + let workspace = temp.path().join("workspace"); + let user_root = temp.path().join("user-root"); + let home_root = temp.path().join("home-root"); + std::fs::create_dir_all(&workspace).expect("create workspace"); + write_package(&workspace, PLUGIN_SOURCE, &sha256(PLUGIN_SOURCE)); + + let approve = run_cli( + &workspace, + &user_root, + &home_root, + &["plugins", "approve-source", "acme.demo"], + ); + assert!(approve.status.success(), "{}", stderr(&approve)); + activate_package(&workspace, &user_root, &home_root); + + std::fs::remove_dir_all(workspace.join(".bitfun/plugins/acme.demo")).expect("remove package"); + let missing = run_cli( + &workspace, + &user_root, + &home_root, + &["plugins", "deactivate", "acme.demo"], + ); + assert!(missing.status.success(), "{}", stderr(&missing)); + assert!(stdout(&missing).contains("is unavailable")); + assert!(stdout(&missing).contains("saved activation state was cleared")); + + let repeated = run_cli( + &workspace, + &user_root, + &home_root, + &["plugins", "deactivate", "acme.demo"], + ); + assert!(repeated.status.success(), "{}", stderr(&repeated)); + assert!(stdout(&repeated).contains("is unavailable")); + assert!(stdout(&repeated).contains("has no saved activation state")); + + let missing_list = run_cli(&workspace, &user_root, &home_root, &["plugins", "list"]); + assert!(missing_list.status.success(), "{}", stderr(&missing_list)); + let missing_doctor = run_cli(&workspace, &user_root, &home_root, &["doctor"]); + assert!( + missing_doctor.status.success(), + "{}", + stderr(&missing_doctor) + ); + + write_package(&workspace, PLUGIN_SOURCE, &sha256(PLUGIN_SOURCE)); + activate_package(&workspace, &user_root, &home_root); + std::fs::write( + workspace.join(".bitfun/plugins/acme.demo/bitfun.plugin.json"), + "{not-json", + ) + .expect("corrupt package manifest"); + + let corrupt = run_cli( + &workspace, + &user_root, + &home_root, + &["plugins", "deactivate", "acme.demo"], + ); + assert!(corrupt.status.success(), "{}", stderr(&corrupt)); + assert!(stdout(&corrupt).contains("is unavailable")); + assert!(stdout(&corrupt).contains("saved activation state was cleared")); + assert!(stdout(&corrupt).contains("[error:invalid_manifest]")); + assert!(stdout(&corrupt).contains("bitfun.plugin.json")); + + write_package(&workspace, PLUGIN_SOURCE, &sha256(PLUGIN_SOURCE)); + activate_package(&workspace, &user_root, &home_root); + std::fs::remove_dir_all(workspace.join(".bitfun/plugins")).expect("remove plugin root"); + std::fs::write(workspace.join(".bitfun/plugins"), "not a directory") + .expect("make plugin root unreadable"); + + let incomplete = run_cli( + &workspace, + &user_root, + &home_root, + &["plugins", "deactivate", "acme.demo"], + ); + assert!(incomplete.status.success(), "{}", stderr(&incomplete)); + assert!(stdout(&incomplete).contains("saved activation state was cleared")); + assert!(stdout(&incomplete).contains("availability could not be determined")); + assert!(stdout(&incomplete).contains("[error:root_read_failed]")); + + let incomplete_doctor = run_cli(&workspace, &user_root, &home_root, &["doctor"]); + assert_eq!(incomplete_doctor.status.code(), Some(1)); + assert!(stdout(&incomplete_doctor).contains("source scan is incomplete")); + assert!(!stdout(&incomplete_doctor).contains("[ok] Managed plugin source integrity checked")); +} + +#[test] +fn plugin_deactivate_does_not_claim_a_same_id_replacement_was_active() { + let temp = tempfile::tempdir().expect("tempdir"); + let workspace = temp.path().join("workspace"); + let user_root = temp.path().join("user-root"); + let home_root = temp.path().join("home-root"); + std::fs::create_dir_all(&workspace).expect("create workspace"); + write_package(&workspace, PLUGIN_SOURCE, &sha256(PLUGIN_SOURCE)); + + let approve = run_cli( + &workspace, + &user_root, + &home_root, + &["plugins", "approve-source", "acme.demo"], + ); + assert!(approve.status.success(), "{}", stderr(&approve)); + activate_package(&workspace, &user_root, &home_root); + + let replacement = b"export const ReplacementPlugin = async () => ({})"; + write_package(&workspace, replacement, &sha256(replacement)); + let deactivate = run_cli( + &workspace, + &user_root, + &home_root, + &["plugins", "deactivate", "acme.demo"], + ); + + assert!(deactivate.status.success(), "{}", stderr(&deactivate)); + assert!(stdout(&deactivate).contains("previous source's saved activation state was cleared")); + assert!(stdout(&deactivate).contains("current package was not active")); + assert!(!stdout(&deactivate).contains("was deactivated")); +} diff --git a/src/crates/adapters/opencode-adapter/tests/opencode_source_adapter.rs b/src/crates/adapters/opencode-adapter/tests/opencode_source_adapter.rs index adceed6841..88f6fc7749 100644 --- a/src/crates/adapters/opencode-adapter/tests/opencode_source_adapter.rs +++ b/src/crates/adapters/opencode-adapter/tests/opencode_source_adapter.rs @@ -137,13 +137,7 @@ impl ManagedPackageFixture { let input = self.approved_input().await; let content_hash = input.clone().into_parts().1.content_hash; self.service - .set_activation( - &self.workspace, - "acme.demo", - true, - Some(&content_hash), - None, - ) + .activate(&self.workspace, "acme.demo", Some(&content_hash)) .await .expect("activate package"); self.service diff --git a/src/crates/assembly/core/src/plugin_runtime.rs b/src/crates/assembly/core/src/plugin_runtime.rs index 422ba255d9..829d2234b1 100644 --- a/src/crates/assembly/core/src/plugin_runtime.rs +++ b/src/crates/assembly/core/src/plugin_runtime.rs @@ -18,7 +18,7 @@ use bitfun_runtime_ports::{ PortResult, }; use bitfun_services_integrations::plugin_source::{ - ManagedPluginSourceError, ManagedPluginSourceService, + ManagedPluginSourceError, ManagedPluginSourceIssue, ManagedPluginSourceService, }; use std::path::{Path, PathBuf}; use std::sync::Arc; @@ -55,6 +55,24 @@ pub struct ManagedPluginActivationView { pub diagnostics: Vec, } +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum ManagedPluginDeactivationResult { + Deactivated { + package_id: String, + diagnostics: Vec, + }, + ResidualActivationCleared { + package_id: String, + current_package_available: Option, + diagnostics: Vec, + }, + AlreadyInactive { + package_id: String, + current_package_available: Option, + diagnostics: Vec, + }, +} + pub async fn preview_managed_plugin_activation( workspace: &Path, package_id: &str, @@ -65,23 +83,25 @@ pub async fn preview_managed_plugin_activation( preview_with_service(service, workspace, package_id).await } -pub async fn set_managed_plugin_activation( +pub async fn activate_managed_plugin( workspace: &Path, package_id: &str, - activated: bool, expected_content_hash: Option<&str>, ) -> Result { let service = Arc::new(crate::plugin_source::managed_plugin_source_service( workspace, )?); - set_activation_with_service( - service, + activate_with_service(service, workspace, package_id, expected_content_hash).await +} + +pub async fn deactivate_managed_plugin( + workspace: &Path, + package_id: &str, +) -> Result { + let service = Arc::new(crate::plugin_source::managed_plugin_source_service( workspace, - package_id, - activated, - expected_content_hash, - ) - .await + )?); + deactivate_with_service(service, workspace, package_id).await } async fn preview_with_service( @@ -110,41 +130,12 @@ async fn preview_with_service( )) } -async fn set_activation_with_service( +async fn activate_with_service( service: Arc, workspace: &Path, package_id: &str, - activated: bool, expected_content_hash: Option<&str>, ) -> Result { - if !activated { - let (snapshot, _) = service - .set_activation(workspace, package_id, false, None, None) - .await?; - let package = snapshot - .packages - .into_iter() - .find(|package| package.package_id == package_id) - .ok_or_else(|| ManagedPluginSourceError::PackageNotFound(package_id.to_string()))?; - return Ok(ManagedPluginActivationView { - package_id: package.package_id, - version: package.version, - adapter: package.adapter, - content_hash: package.content_hash, - activated: false, - activation_epoch: None, - entry_ids: Vec::new(), - provider_candidates_supported: false, - permission_required: false, - candidates: Vec::new(), - diagnostics: snapshot - .issues - .into_iter() - .map(|issue| issue.message) - .collect(), - }); - } - let expected_content_hash = expected_content_hash.ok_or_else(|| { invalid_package( package_id, @@ -166,13 +157,7 @@ async fn set_activation_with_service( } let (activation, activation_changed) = service - .set_activation( - workspace, - package_id, - true, - Some(expected_content_hash), - None, - ) + .activate(workspace, package_id, Some(expected_content_hash)) .await?; let activation_epoch = activation.activation_epoch.ok_or_else(|| { unavailable( @@ -205,21 +190,10 @@ async fn set_activation_with_service( Ok(view) => Ok(view), Err(error) if activation_changed => { let rollback = service - .set_activation(workspace, package_id, false, None, Some(activation_epoch)) + .deactivate(workspace, package_id, Some(activation_epoch)) .await; match rollback { - Ok((snapshot, _)) - if snapshot - .packages - .iter() - .any(|package| package.package_id == package_id && !package.activated) => - { - Err(error) - } - Ok(_) => Err(unavailable( - package_id, - format!("{error}; activation changed concurrently and was not rolled back"), - )), + Ok(_) => Err(error), Err(rollback_error) => Err(unavailable( package_id, format!("{error}; activation rollback failed: {rollback_error}"), @@ -230,6 +204,40 @@ async fn set_activation_with_service( } } +async fn deactivate_with_service( + service: Arc, + workspace: &Path, + package_id: &str, +) -> Result { + let (snapshot, changed, cleared_source_available) = + service.deactivate(workspace, package_id, None).await?; + let current_package_available = snapshot.discovery_complete.then(|| { + snapshot + .packages + .iter() + .any(|package| package.package_id == package_id) + }); + let diagnostics = snapshot.issues; + if changed && cleared_source_available == Some(true) { + Ok(ManagedPluginDeactivationResult::Deactivated { + package_id: package_id.to_string(), + diagnostics, + }) + } else if changed { + Ok(ManagedPluginDeactivationResult::ResidualActivationCleared { + package_id: package_id.to_string(), + current_package_available, + diagnostics, + }) + } else { + Ok(ManagedPluginDeactivationResult::AlreadyInactive { + package_id: package_id.to_string(), + current_package_available, + diagnostics, + }) + } +} + async fn project_activated( service: Arc, workspace: &Path, @@ -642,6 +650,21 @@ export const WorkspaceToolsPlugin: Plugin = async () => ({ service, } } + + async fn activate(&self) { + let preview = + preview_with_service(Arc::clone(&self.service), &self.workspace, "acme.demo") + .await + .expect("preview package"); + activate_with_service( + Arc::clone(&self.service), + &self.workspace, + "acme.demo", + Some(&preview.content_hash), + ) + .await + .expect("activate package"); + } } #[tokio::test] @@ -663,11 +686,10 @@ export const WorkspaceToolsPlugin: Plugin = async () => ({ assert!(preview.provider_candidates_supported); assert!(preview.permission_required); - let activated = set_activation_with_service( + let activated = activate_with_service( Arc::clone(&fixture.service), &fixture.workspace, "acme.demo", - true, Some(&preview.content_hash), ) .await @@ -677,17 +699,71 @@ export const WorkspaceToolsPlugin: Plugin = async () => ({ assert!(!activated.candidates.is_empty()); assert!(activated.permission_required); - let deactivated = set_activation_with_service( + let deactivated = deactivate_with_service( + Arc::clone(&fixture.service), + &fixture.workspace, + "acme.demo", + ) + .await + .expect("deactivate package"); + assert!(matches!( + deactivated, + ManagedPluginDeactivationResult::Deactivated { .. } + )); + } + + #[tokio::test] + async fn deactivation_distinguishes_current_residual_and_inactive_states() { + let fixture = Fixture::new().await; + fixture.activate().await; + + let result = deactivate_with_service( Arc::clone(&fixture.service), &fixture.workspace, "acme.demo", - false, - None, ) .await .expect("deactivate package"); - assert!(!deactivated.activated); - assert_eq!(deactivated.activation_epoch, None); + + assert!(matches!( + result, + ManagedPluginDeactivationResult::Deactivated { package_id, .. } + if package_id == "acme.demo" + )); + fixture.activate().await; + fs::remove_dir_all(fixture.workspace.join(".bitfun/plugins")).expect("remove plugin root"); + fs::write(fixture.workspace.join(".bitfun/plugins"), "not a directory") + .expect("make plugin root unreadable"); + + let cleared = deactivate_with_service( + Arc::clone(&fixture.service), + &fixture.workspace, + "acme.demo", + ) + .await + .expect("clear residual activation"); + assert!(matches!( + cleared, + ManagedPluginDeactivationResult::ResidualActivationCleared { + current_package_available: None, + .. + } + )); + + let repeated = deactivate_with_service( + Arc::clone(&fixture.service), + &fixture.workspace, + "acme.demo", + ) + .await + .expect("repeat deactivation"); + assert!(matches!( + repeated, + ManagedPluginDeactivationResult::AlreadyInactive { + current_package_available: None, + .. + } + )); } #[tokio::test] @@ -705,11 +781,10 @@ export const WorkspaceToolsPlugin: Plugin = async () => ({ .expect("preview unsupported package"); assert!(!preview.provider_candidates_supported); - let error = set_activation_with_service( + let error = activate_with_service( Arc::clone(&fixture.service), &fixture.workspace, "acme.demo", - true, Some(&preview.content_hash), ) .await @@ -721,16 +796,17 @@ export const WorkspaceToolsPlugin: Plugin = async () => ({ let snapshot = fixture.service.refresh(&fixture.workspace).await; assert!(!snapshot.packages[0].activated); - let inactive = set_activation_with_service( + let inactive = deactivate_with_service( Arc::clone(&fixture.service), &fixture.workspace, "acme.demo", - false, - None, ) .await .expect("keep package inactive"); - assert_eq!(inactive.activation_epoch, None); + assert!(matches!( + inactive, + ManagedPluginDeactivationResult::AlreadyInactive { .. } + )); } #[tokio::test] @@ -746,13 +822,7 @@ export const WorkspaceToolsPlugin: Plugin = async () => ({ .content_hash; fixture .service - .set_activation( - &fixture.workspace, - "acme.demo", - true, - Some(&first_content_hash), - None, - ) + .activate(&fixture.workspace, "acme.demo", Some(&first_content_hash)) .await .expect("activate package"); let (input, authority) = fixture @@ -781,7 +851,7 @@ export const WorkspaceToolsPlugin: Plugin = async () => ({ fixture .service - .set_activation(&fixture.workspace, "acme.demo", false, None, None) + .deactivate(&fixture.workspace, "acme.demo", None) .await .expect("deactivate package"); assert!(binding @@ -804,13 +874,7 @@ export const WorkspaceToolsPlugin: Plugin = async () => ({ .content_hash; fixture .service - .set_activation( - &fixture.workspace, - "acme.demo", - true, - Some(¤t_content_hash), - None, - ) + .activate(&fixture.workspace, "acme.demo", Some(¤t_content_hash)) .await .expect("reactivate package"); let (current_input, current_authority) = fixture @@ -853,13 +917,7 @@ export const WorkspaceToolsPlugin: Plugin = async () => ({ .expect("preview package"); fixture .service - .set_activation( - &fixture.workspace, - "acme.demo", - true, - Some(&preview.content_hash), - None, - ) + .activate(&fixture.workspace, "acme.demo", Some(&preview.content_hash)) .await .expect("activate package"); let (input, authority) = fixture @@ -907,7 +965,7 @@ export const WorkspaceToolsPlugin: Plugin = async () => ({ started.notified().await; fixture .service - .set_activation(&fixture.workspace, "acme.demo", false, None, None) + .deactivate(&fixture.workspace, "acme.demo", None) .await .expect("deactivate while dispatch is in flight"); release.notify_one(); diff --git a/src/crates/contracts/product-domains/src/plugin_source.rs b/src/crates/contracts/product-domains/src/plugin_source.rs index 69a5e7bd0d..d3a2fd1ad7 100644 --- a/src/crates/contracts/product-domains/src/plugin_source.rs +++ b/src/crates/contracts/product-domains/src/plugin_source.rs @@ -370,6 +370,31 @@ impl PluginTrustStore { self.activation_epoch } + pub fn activation_sources( + &self, + project_domain_id: &str, + workspace_id: &str, + ) -> Vec { + let mut sources = self + .activation_records + .iter() + .filter(|record| { + record.project_domain_id == project_domain_id && record.workspace_id == workspace_id + }) + .map(|record| record.source.clone()) + .collect::>(); + sources.sort_by(|left, right| { + left.package_id + .cmp(&right.package_id) + .then_with(|| left.version.cmp(&right.version)) + .then_with(|| left.adapter.cmp(&right.adapter)) + .then_with(|| left.source_path.cmp(&right.source_path)) + .then_with(|| left.content_hash.cmp(&right.content_hash)) + }); + sources.dedup(); + sources + } + pub fn validate(&self) -> Result<(), PluginSourceContractError> { if self.schema_version != PLUGIN_TRUST_STORE_SCHEMA_VERSION { return Err(PluginSourceContractError::UnsupportedTrustStoreSchema( @@ -505,53 +530,75 @@ impl PluginTrustStore { .is_some_and(|record| record.activation_epoch == authority.activation_epoch) } - pub fn set_activation( + pub fn activate( &mut self, project_domain_id: &str, workspace_id: &str, source: PluginPackageSourceIdentity, - activated: bool, updated_at_ms: u64, ) -> Result { validate_scope(project_domain_id, workspace_id)?; source.validate()?; - if activated - && self.trust_level_for(project_domain_id, workspace_id, &source) - != PluginPackageTrustLevel::SourceApproved + if self.trust_level_for(project_domain_id, workspace_id, &source) + != PluginPackageTrustLevel::SourceApproved { return Err(PluginSourceContractError::ActivationRequiresSourceApproval); } - let existing_index = self.activation_records.iter().position(|record| { + if self.activation_records.iter().any(|record| { record.project_domain_id == project_domain_id && record.workspace_id == workspace_id && record.source == source - }); - if activated == existing_index.is_some() { + }) { return Ok(false); } let mut next = self.clone(); - if activated { - if next.activation_records.len() >= MAX_ACTIVATION_RECORDS { - return Err(PluginSourceContractError::TooManyActivationRecords); - } - next.advance_activation_epoch()?; - next.activation_records.push(PluginActivationRecord { - project_domain_id: project_domain_id.to_string(), - workspace_id: workspace_id.to_string(), - source, - activation_epoch: next.activation_epoch, - updated_at_ms, - }); - } else if let Some(index) = existing_index { - next.activation_records.remove(index); - next.advance_activation_epoch()?; + if next.activation_records.len() >= MAX_ACTIVATION_RECORDS { + return Err(PluginSourceContractError::TooManyActivationRecords); } + next.advance_activation_epoch()?; + next.activation_records.push(PluginActivationRecord { + project_domain_id: project_domain_id.to_string(), + workspace_id: workspace_id.to_string(), + source, + activation_epoch: next.activation_epoch, + updated_at_ms, + }); *self = next; Ok(true) } + pub fn clear_activation_record( + &mut self, + project_domain_id: &str, + workspace_id: &str, + package_id: &str, + expected_activation_epoch: Option, + ) -> Result, PluginSourceContractError> { + validate_scope(project_domain_id, workspace_id)?; + validate_package_id(package_id)?; + let Some(index) = self.activation_records.iter().position(|record| { + record.project_domain_id == project_domain_id + && record.workspace_id == workspace_id + && record.source.package_id == package_id + }) else { + return Ok(None); + }; + if expected_activation_epoch + .is_some_and(|expected| self.activation_records[index].activation_epoch != expected) + { + return Ok(None); + } + + let mut next = self.clone(); + let removed_source = next.activation_records[index].source.clone(); + next.activation_records.remove(index); + next.advance_activation_epoch()?; + *self = next; + Ok(Some(removed_source)) + } + fn activation_record( &self, project_domain_id: &str, @@ -649,11 +696,16 @@ impl PluginTrustStore { source.validate()?; } let current = current_sources.iter().collect::>(); + let current_package_ids = current_sources + .iter() + .map(|source| source.package_id.as_str()) + .collect::>(); let mut next = self.clone(); let previous_len = next.records.len(); next.records.retain(|record| { record.project_domain_id != project_domain_id || record.workspace_id != workspace_id + || !current_package_ids.contains(record.source.package_id.as_str()) || current.contains(&record.source) }); let changed = next.records.len() != previous_len; @@ -665,6 +717,7 @@ impl PluginTrustStore { next.activation_records.retain(|record| { record.project_domain_id != project_domain_id || record.workspace_id != workspace_id + || !current_package_ids.contains(record.source.package_id.as_str()) || current.contains(&record.source) }); let activation_changed = next.activation_records.len() != previous_activation_len; diff --git a/src/crates/contracts/product-domains/tests/plugin_source_contracts.rs b/src/crates/contracts/product-domains/tests/plugin_source_contracts.rs index 18722ae16d..8e17b22bc1 100644 --- a/src/crates/contracts/product-domains/tests/plugin_source_contracts.rs +++ b/src/crates/contracts/product-domains/tests/plugin_source_contracts.rs @@ -35,7 +35,7 @@ fn approve_source(store: &mut PluginTrustStore, package: &PluginPackageSourceIde fn activate_source(store: &mut PluginTrustStore, package: &PluginPackageSourceIdentity) { store - .set_activation(PROJECT, WORKSPACE, package.clone(), true, 101) + .activate(PROJECT, WORKSPACE, package.clone(), 101) .expect("activate source"); } @@ -251,6 +251,39 @@ fn trust_store_invalidates_changed_package_identity_and_advances_epoch_once() { assert_eq!(store.epoch(), 3); } +#[test] +fn absent_sources_preserve_review_history_until_a_replacement_is_discovered() { + let original = source(HASH_A, SOURCE_PATH); + let replacement = source(HASH_B, SOURCE_PATH); + let mut store = PluginTrustStore::new(1); + approve_source(&mut store, &original); + activate_source(&mut store, &original); + let epochs = (store.epoch(), store.activation_epoch()); + + assert!(!store + .reconcile_sources(PROJECT, WORKSPACE, &[]) + .expect("reconcile absent source")); + assert_eq!((store.epoch(), store.activation_epoch()), epochs); + assert_eq!( + store.trust_level_for(PROJECT, WORKSPACE, &original), + PluginPackageTrustLevel::SourceApproved + ); + assert!(store.is_activated(PROJECT, WORKSPACE, &original)); + assert_eq!( + store.activation_sources(PROJECT, WORKSPACE), + vec![original.clone()] + ); + + assert!(store + .reconcile_sources(PROJECT, WORKSPACE, &[replacement]) + .expect("reconcile replacement source")); + assert_eq!( + store.trust_level_for(PROJECT, WORKSPACE, &original), + PluginPackageTrustLevel::Unknown + ); + assert!(!store.is_activated(PROJECT, WORKSPACE, &original)); +} + #[test] fn trust_decisions_are_scoped_to_project_and_workspace() { let mut store = PluginTrustStore::new(1); @@ -371,7 +404,7 @@ fn activation_lifecycle_is_exact_independent_and_idempotent() { assert_eq!(store.activation_epoch(), 7); assert_eq!( store - .set_activation(PROJECT, WORKSPACE, package.clone(), true, 100) + .activate(PROJECT, WORKSPACE, package.clone(), 100) .expect_err("unapproved source must not activate") .to_string(), "only a source-approved plugin package can be activated" @@ -385,19 +418,91 @@ fn activation_lifecycle_is_exact_independent_and_idempotent() { assert!(!store.is_activated(PROJECT, "workspace-2", &package)); assert!(!store.is_activated(PROJECT, WORKSPACE, &source(HASH_B, SOURCE_PATH))); assert!(!store - .set_activation(PROJECT, WORKSPACE, package.clone(), true, 102) + .activate(PROJECT, WORKSPACE, package.clone(), 102) .expect("repeat activation")); assert!(store - .set_activation(PROJECT, WORKSPACE, package.clone(), false, 103) - .expect("deactivate source")); + .clear_activation_record(PROJECT, WORKSPACE, &package.package_id, None) + .expect("deactivate source") + .is_some()); assert_eq!((store.epoch(), store.activation_epoch()), (trust_epoch, 9)); assert!(!store - .set_activation(PROJECT, WORKSPACE, package, false, 104) - .expect("repeat deactivation")); + .clear_activation_record(PROJECT, WORKSPACE, &package.package_id, None) + .expect("repeat deactivation") + .is_some()); assert_eq!((store.epoch(), store.activation_epoch()), (trust_epoch, 9)); } +#[test] +fn residual_activation_cleanup_preserves_source_approval_and_is_idempotent() { + let package = source(HASH_A, SOURCE_PATH); + let mut store = PluginTrustStore::new(7); + approve_source(&mut store, &package); + activate_source(&mut store, &package); + let trust_epoch = store.epoch(); + let activated_epoch = store + .activation_authority(PROJECT, WORKSPACE, &package) + .expect("read activation authority") + .activation_epoch(); + + assert_eq!( + store + .clear_activation_record( + PROJECT, + WORKSPACE, + &package.package_id, + Some(activated_epoch), + ) + .expect("clear residual activation"), + Some(package.clone()) + ); + assert_eq!(store.epoch(), trust_epoch); + assert_eq!(store.activation_epoch(), activated_epoch + 1); + assert_eq!( + store.trust_level_for(PROJECT, WORKSPACE, &package), + PluginPackageTrustLevel::SourceApproved + ); + assert!(!store.is_activated(PROJECT, WORKSPACE, &package)); + + let cleanup_epoch = store.activation_epoch(); + assert_eq!( + store + .clear_activation_record(PROJECT, WORKSPACE, &package.package_id, None) + .expect("repeat residual cleanup"), + None + ); + assert_eq!(store.activation_epoch(), cleanup_epoch); +} + +#[test] +fn stale_residual_cleanup_cannot_clear_a_newer_activation() { + let package = source(HASH_A, SOURCE_PATH); + let mut store = PluginTrustStore::new(1); + approve_source(&mut store, &package); + activate_source(&mut store, &package); + let stale_epoch = store + .activation_authority(PROJECT, WORKSPACE, &package) + .expect("read first activation authority") + .activation_epoch(); + store + .clear_activation_record(PROJECT, WORKSPACE, &package.package_id, None) + .expect("deactivate source"); + store + .activate(PROJECT, WORKSPACE, package.clone(), 103) + .expect("reactivate source"); + let current_epoch = store + .activation_authority(PROJECT, WORKSPACE, &package) + .expect("read current activation authority") + .activation_epoch(); + + assert!(!store + .clear_activation_record(PROJECT, WORKSPACE, &package.package_id, Some(stale_epoch),) + .expect("stale cleanup is a no-op") + .is_some()); + assert!(store.is_activated(PROJECT, WORKSPACE, &package)); + assert_eq!(store.activation_epoch(), current_epoch); +} + #[test] fn source_changes_deny_and_revoke_invalidate_activation_atomically() { let original = source(HASH_A, SOURCE_PATH); @@ -547,7 +652,7 @@ fn activation_authority_requires_the_exact_activated_package() { let retained = authority.clone(); store - .set_activation(PROJECT, WORKSPACE, package.clone(), false, 102) + .clear_activation_record(PROJECT, WORKSPACE, &package.package_id, None) .expect("deactivate source"); assert!(!store.is_activation_current(&retained)); diff --git a/src/crates/services/services-integrations/src/plugin_source.rs b/src/crates/services/services-integrations/src/plugin_source.rs index d518f1c0ad..1c99174e66 100644 --- a/src/crates/services/services-integrations/src/plugin_source.rs +++ b/src/crates/services/services-integrations/src/plugin_source.rs @@ -58,6 +58,7 @@ pub struct ManagedPluginSourceIssue { pub struct ManagedPluginSourceSnapshot { pub packages: Vec, pub issues: Vec, + pub discovery_complete: bool, pub trust_epoch: Option, pub activation_epoch: Option, } @@ -76,6 +77,13 @@ pub enum ManagedPluginSourceError { package_id: String, diagnostic: String, }, + #[error( + "managed plugin package {package_id} deactivation persistence is uncertain: {diagnostic}" + )] + DeactivationPersistenceUncertain { + package_id: String, + diagnostic: String, + }, #[error("managed plugin trust update failed: {0}")] TrustStore(String), } @@ -142,6 +150,22 @@ fn map_activation_store_error( } } +fn map_deactivation_store_error( + package_id: &str, + error: PluginSourceStoreError, +) -> ManagedPluginSourceError { + match error { + error @ (PluginSourceStoreError::TrustDurabilityUncertain { .. } + | PluginSourceStoreError::TrustCommitUnconfirmed { .. }) => { + ManagedPluginSourceError::DeactivationPersistenceUncertain { + package_id: package_id.to_string(), + diagnostic: error.to_string(), + } + } + error => map_activation_store_error(package_id, error), + } +} + pub struct ManagedPluginSourceService { store: ProductPluginSourceStore, load_gate: tokio::sync::Semaphore, @@ -216,25 +240,21 @@ impl ManagedPluginSourceService { Ok(build_snapshot(discovery, Some(trust_store), None, &scope)) } - /// Activate or deactivate one exact managed package without changing source approval. - pub async fn set_activation( + /// Activate one exact managed package without changing source approval. + pub async fn activate( &self, workspace: &Path, package_id: &str, - activated: bool, expected_content_hash: Option<&str>, - expected_activation_epoch: Option, ) -> Result<(ManagedPluginSourceSnapshot, bool), ManagedPluginSourceError> { let scope = workspace_scope(workspace); let result = self .store - .apply_activation( + .activate( &scope.project_domain_id, &scope.workspace_id, package_id, - activated, expected_content_hash, - expected_activation_epoch, current_time_ms(), ) .await; @@ -256,6 +276,88 @@ impl ManagedPluginSourceService { } } + /// Deactivate persisted state before inspecting current package availability. + pub async fn deactivate( + &self, + workspace: &Path, + package_id: &str, + expected_activation_epoch: Option, + ) -> Result<(ManagedPluginSourceSnapshot, bool, Option), ManagedPluginSourceError> { + let scope = workspace_scope(workspace); + let (cleared_source, had_trust_store, activation_epoch_mismatch) = self + .store + .clear_activation_record( + &scope.project_domain_id, + &scope.workspace_id, + package_id, + expected_activation_epoch, + ) + .await + .map_err(|error| map_deactivation_store_error(package_id, error))?; + if activation_epoch_mismatch { + return Err(ManagedPluginSourceError::TemporarilyUnavailable { + package_id: package_id.to_string(), + diagnostic: + "activation state changed while deactivation was being reconciled; retry the operation" + .to_string(), + }); + } + let (discovery, trust_store, trust_issue) = if had_trust_store { + let (discovery, trust_result) = self + .store + .reconcile_trust(&scope.project_domain_id, &scope.workspace_id) + .await; + match trust_result { + Ok(trust_store) => (discovery, Some(trust_store), None), + Err(error) => { + if matches!( + &error, + PluginSourceStoreError::TrustDurabilityUncertain { .. } + | PluginSourceStoreError::TrustCommitUnconfirmed { .. } + ) { + return Err(map_deactivation_store_error(package_id, error)); + } + if expected_activation_epoch.is_some() && cleared_source.is_none() { + return Err(map_load_store_error(package_id, error)); + } + let issue = ManagedPluginSourceIssue { + code: trust_store_issue_code(&error).to_string(), + source_path: self.store.trust_path.to_string_lossy().to_string(), + message: error.to_string(), + is_error: true, + }; + (discovery, None, Some(issue)) + } + } + } else { + let mut scan_budget = OperationScanBudget::new(); + ( + self.store.discover_with_budget(&mut scan_budget).await, + None, + None, + ) + }; + let discovery_complete = discovery.is_complete(); + let cleared_source_available = cleared_source.as_ref().and_then(|cleared_source| { + discovery_complete.then(|| { + discovery + .packages + .iter() + .any(|package| package.identity == *cleared_source) + }) + }); + let snapshot = build_snapshot(discovery, trust_store, trust_issue, &scope); + if snapshot_has_activation(&snapshot, package_id) { + return Err(ManagedPluginSourceError::TemporarilyUnavailable { + package_id: package_id.to_string(), + diagnostic: + "activation state changed while deactivation was being reconciled; retry the operation" + .to_string(), + }); + } + Ok((snapshot, cleared_source.is_some(), cleared_source_available)) + } + /// Load one selected package as fixed content for an ecosystem adapter. pub async fn load_package( &self, @@ -466,6 +568,14 @@ fn build_snapshot( scope: &PluginTrustScope, ) -> ManagedPluginSourceSnapshot { let discovery_complete = discovery.is_complete(); + let activation_sources = trust_store.as_ref().map_or_else(Vec::new, |trust_store| { + trust_store.activation_sources(&scope.project_domain_id, &scope.workspace_id) + }); + let available_sources = discovery + .packages + .iter() + .map(|package| package.identity.clone()) + .collect::>(); let mut issues = discovery .issues .into_iter() @@ -507,6 +617,23 @@ fn build_snapshot( }), }) .collect::>(); + if discovery_complete { + for source in activation_sources { + if !available_sources.contains(&source) { + let package_id = source.package_id; + issues.push(ManagedPluginSourceIssue { + code: PluginSourceIssueCode::ActivationSourceUnavailable + .as_str() + .to_string(), + source_path: package_id.clone(), + message: format!( + "saved activation state exists for {package_id}, but its package source is unavailable" + ), + is_error: false, + }); + } + } + } packages.sort_by(|left, right| left.package_id.cmp(&right.package_id)); issues.sort_by(|left, right| { left.source_path @@ -517,11 +644,23 @@ fn build_snapshot( ManagedPluginSourceSnapshot { packages, issues, + discovery_complete, trust_epoch: trust_store.as_ref().map(PluginTrustStore::epoch), activation_epoch: trust_store.as_ref().map(PluginTrustStore::activation_epoch), } } +fn snapshot_has_activation(snapshot: &ManagedPluginSourceSnapshot, package_id: &str) -> bool { + snapshot + .packages + .iter() + .any(|package| package.package_id == package_id && package.activated) + || snapshot.issues.iter().any(|issue| { + issue.code == PluginSourceIssueCode::ActivationSourceUnavailable.as_str() + && issue.source_path == package_id + }) +} + fn trust_store_issue_code(error: &PluginSourceStoreError) -> &'static str { match error { PluginSourceStoreError::Contract(_) | PluginSourceStoreError::TrustDeserialize { .. } => { @@ -998,6 +1137,7 @@ enum PluginSourceIssueCode { FileReadFailed, HashMismatch, ShadowedPackage, + ActivationSourceUnavailable, } impl PluginSourceIssueCode { @@ -1015,11 +1155,15 @@ impl PluginSourceIssueCode { Self::FileReadFailed => "file_read_failed", Self::HashMismatch => "hash_mismatch", Self::ShadowedPackage => "shadowed_package", + Self::ActivationSourceUnavailable => "activation_source_unavailable", } } const fn is_error(self) -> bool { - !matches!(self, Self::ShadowedPackage) + !matches!( + self, + Self::ShadowedPackage | Self::ActivationSourceUnavailable + ) } } @@ -1637,14 +1781,12 @@ impl ProductPluginSourceStore { Ok((verified, store)) } - async fn apply_activation( + async fn activate( &self, project_domain_id: &str, workspace_id: &str, package_id: &str, - activated: bool, expected_content_hash: Option<&str>, - expected_activation_epoch: Option, updated_at_ms: u64, ) -> Result { let mut scan_budget = OperationScanBudget::new(); @@ -1658,8 +1800,7 @@ impl ProductPluginSourceStore { .find(|package| package.identity.package_id == package_id) .map(|package| package.identity.clone()) .ok_or_else(|| PluginSourceStoreError::PackageNotFound(package_id.to_string()))?; - if activated && expected_content_hash.is_none_or(|expected| expected != source.content_hash) - { + if expected_content_hash.is_none_or(|expected| expected != source.content_hash) { return Err(PluginSourceStoreError::ActivationContentMismatch); } let mut file_guard = self.acquire_trust_file_lock(&scan_budget).await?; @@ -1668,30 +1809,8 @@ impl ProductPluginSourceStore { .await?; let mut store = loaded.store; let before = store.clone(); - if !activated { - if let Some(expected) = expected_activation_epoch { - let current = store - .activation_authority(project_domain_id, workspace_id, &source) - .ok() - .map(|authority| authority.activation_epoch()); - if current != Some(expected) { - return Ok(ActivationStoreOutcome { - discovery, - trust_store: store, - changed: false, - durability_warning: None, - }); - } - } - } store.reconcile_sources(project_domain_id, workspace_id, &discovery.identities())?; - let changed = store.set_activation( - project_domain_id, - workspace_id, - source, - activated, - updated_at_ms, - )?; + let changed = store.activate(project_domain_id, workspace_id, source, updated_at_ms)?; let verified = self.discover_with_budget(&mut scan_budget).await; if !verified.is_complete() || discovery.identities() != verified.identities() @@ -1712,13 +1831,7 @@ impl ProductPluginSourceStore { .await { Ok(()) => None, - Err(error @ PluginSourceStoreError::TrustDurabilityUncertain { .. }) => { - if activated { - Some(error) - } else { - return Err(error); - } - } + Err(error @ PluginSourceStoreError::TrustDurabilityUncertain { .. }) => Some(error), Err(error) => return Err(error), } } else { @@ -1732,6 +1845,52 @@ impl ProductPluginSourceStore { }) } + async fn clear_activation_record( + &self, + project_domain_id: &str, + workspace_id: &str, + package_id: &str, + expected_activation_epoch: Option, + ) -> Result<(Option, bool, bool), PluginSourceStoreError> { + let budget = OperationScanBudget::new(); + let mut file_guard = self.acquire_trust_file_lock(&budget).await?; + let loaded = self + .load_trust_store_generation_locked(&mut file_guard, false) + .await?; + let had_trust_store = loaded.identity.is_some(); + let mut store = loaded.store; + let had_activation = store + .activation_sources(project_domain_id, workspace_id) + .iter() + .any(|source| source.package_id == package_id); + let cleared_source = store.clear_activation_record( + project_domain_id, + workspace_id, + package_id, + expected_activation_epoch, + )?; + if cleared_source.is_none() { + if had_trust_store { + self.sync_trust_parent_locked(&mut file_guard).await?; + } + return Ok(( + None, + had_trust_store, + expected_activation_epoch.is_some() && had_activation, + )); + } + self.persist_trust_store_locked( + &store, + &mut file_guard, + loaded + .identity + .map(TrustFileExpectation::Identity) + .unwrap_or(TrustFileExpectation::Missing), + ) + .await?; + Ok((cleared_source, had_trust_store, false)) + } + async fn reconcile_trust( &self, project_domain_id: &str, @@ -1950,6 +2109,28 @@ impl ProductPluginSourceStore { .await } + async fn sync_trust_parent_locked( + &self, + file_guard: &mut TrustFileGuard, + ) -> Result<(), PluginSourceStoreError> { + let trust_path = self.trust_path.clone(); + let parent = trust_path + .parent() + .map(Path::to_path_buf) + .ok_or(PluginSourceStoreError::InvalidTrustPath)?; + let parent_sync = self.parent_sync; + file_guard + .run_blocking(move || { + parent_sync(&parent).map_err(|source| { + PluginSourceStoreError::TrustDurabilityUncertain { + path: trust_path, + source, + } + }) + }) + .await + } + #[cfg(test)] async fn load_trust_store(&self) -> Result { let budget = OperationScanBudget::new(); @@ -2026,6 +2207,12 @@ enum PluginSourceStoreError { #[source] source: io::Error, }, + #[error("plugin trust store {path} could not be verified after atomic replacement: {source}")] + TrustCommitUnconfirmed { + path: PathBuf, + #[source] + source: io::Error, + }, #[error("managed plugin package not found: {0}")] PackageNotFound(String), #[error("managed plugin package {package_id} is invalid: {diagnostic}")] @@ -2088,24 +2275,45 @@ fn persist_trust_bytes_with_parent_sync( path: path.to_path_buf(), source, })?; - if let Err(source) = sync_parent(parent) { - let committed = std::fs::read(path) - .map(|persisted| persisted == bytes) - .unwrap_or(false); - let source = if committed { - source - } else { - io::Error::new( - source.kind(), - format!("{source}; unable to verify the committed trust state"), - ) - }; - return Err(PluginSourceStoreError::TrustDurabilityUncertain { + let parent_sync_error = sync_parent(parent).err(); + let readback = open_regular_file_no_follow(path).and_then(|file| { + read_bounded_file_metered(file, MAX_TRUST_STORE_BYTES).map_err(|error| error.source) + }); + match readback { + Ok(persisted) if persisted == bytes => match parent_sync_error { + Some(source) => Err(PluginSourceStoreError::TrustDurabilityUncertain { + path: path.to_path_buf(), + source, + }), + None => Ok(()), + }, + Ok(_) => Err(PluginSourceStoreError::TrustCommitUnconfirmed { path: path.to_path_buf(), - source, - }); + source: io::Error::new( + parent_sync_error + .as_ref() + .map_or(ErrorKind::InvalidData, io::Error::kind), + parent_sync_error.map_or_else( + || "persisted bytes differ from the requested trust state".to_string(), + |error| { + format!("{error}; persisted bytes differ from the requested trust state") + }, + ), + ), + }), + Err(read_error) => Err(PluginSourceStoreError::TrustCommitUnconfirmed { + path: path.to_path_buf(), + source: io::Error::new( + read_error.kind(), + parent_sync_error.map_or_else( + || format!("failed to read the replaced trust store: {read_error}"), + |error| { + format!("{error}; failed to read the replaced trust store: {read_error}") + }, + ), + ), + }), } - Ok(()) } #[cfg(windows)] @@ -3218,7 +3426,7 @@ mod tests { .expect("approve source"); let content_hash = self.content_hash(service, id).await; service - .set_activation(&self.workspace, id, true, Some(&content_hash), None) + .activate(&self.workspace, id, Some(&content_hash)) .await .expect("activate package"); service @@ -3260,13 +3468,7 @@ mod tests { .content_hash .clone(); let error = service - .set_activation( - &fixture.workspace, - "acme.demo", - true, - Some(&content_hash), - None, - ) + .activate(&fixture.workspace, "acme.demo", Some(&content_hash)) .await .expect_err("unapproved source must not activate"); @@ -3292,7 +3494,7 @@ mod tests { for expected in [None, Some("sha256:stale")] { let error = service - .set_activation(&fixture.workspace, "acme.demo", true, expected, None) + .activate(&fixture.workspace, "acme.demo", expected) .await .expect_err("confirmation must match the selected package"); assert!(matches!( @@ -3368,23 +3570,11 @@ mod tests { let content_hash = fixture.content_hash(&service, "acme.demo").await; let (activated, activated_changed) = service - .set_activation( - &fixture.workspace, - "acme.demo", - true, - Some(&content_hash), - None, - ) + .activate(&fixture.workspace, "acme.demo", Some(&content_hash)) .await .expect("activate package"); let (activated_again, activated_again_changed) = service - .set_activation( - &fixture.workspace, - "acme.demo", - true, - Some(&content_hash), - None, - ) + .activate(&fixture.workspace, "acme.demo", Some(&content_hash)) .await .expect("repeat activation"); assert!(activated.packages[0].activated); @@ -3396,12 +3586,12 @@ mod tests { assert!(recreated.packages[0].activated); assert_eq!(activated.activation_epoch, recreated.activation_epoch); - let (deactivated, deactivated_changed) = service - .set_activation(&fixture.workspace, "acme.demo", false, None, None) + let (deactivated, deactivated_changed, _) = service + .deactivate(&fixture.workspace, "acme.demo", None) .await .expect("deactivate package"); - let (deactivated_again, deactivated_again_changed) = service - .set_activation(&fixture.workspace, "acme.demo", false, None, None) + let (deactivated_again, deactivated_again_changed, _) = service + .deactivate(&fixture.workspace, "acme.demo", None) .await .expect("repeat deactivation"); assert!(!deactivated.packages[0].activated); @@ -3428,46 +3618,199 @@ mod tests { .expect("approve source"); let content_hash = fixture.content_hash(&service, "acme.demo").await; let (first, _) = service - .set_activation( - &fixture.workspace, - "acme.demo", - true, - Some(&content_hash), - None, - ) + .activate(&fixture.workspace, "acme.demo", Some(&content_hash)) .await .expect("activate package"); let first_epoch = first.activation_epoch.expect("first activation epoch"); service - .set_activation(&fixture.workspace, "acme.demo", false, None, None) + .deactivate(&fixture.workspace, "acme.demo", None) .await .expect("deactivate package"); - let (current, _) = service - .set_activation( - &fixture.workspace, - "acme.demo", - true, - Some(&content_hash), - None, - ) + service + .activate(&fixture.workspace, "acme.demo", Some(&content_hash)) .await .expect("reactivate package"); - let current_epoch = current.activation_epoch.expect("current activation epoch"); + let current_authority = service + .load_activated_package(&fixture.workspace, "acme.demo") + .await + .expect("load current activation") + .1; + tokio::fs::remove_dir_all(&fixture.workspace_root) + .await + .expect("remove plugin root"); + tokio::fs::write(&fixture.workspace_root, b"not a directory") + .await + .expect("make plugin root unreadable"); - let (rollback, rollback_changed) = service - .set_activation( - &fixture.workspace, + let error = service + .deactivate(&fixture.workspace, "acme.demo", Some(first_epoch)) + .await + .expect_err("stale rollback must report the newer activation"); + assert!(matches!( + error, + ManagedPluginSourceError::TemporarilyUnavailable { .. } + )); + assert!(service + .store + .load_trust_store() + .await + .expect("load current trust state") + .is_activation_current(¤t_authority)); + } + + #[tokio::test] + async fn deactivation_clears_a_missing_package_and_preserves_source_approval() { + let fixture = ManagedPluginFixture::new().await; + let service = fixture.service(); + let authority = fixture.activate(&service, "acme.demo").await; + let (_, _, source, _) = authority.into_parts(); + let scope = workspace_scope(&fixture.workspace); + tokio::fs::remove_dir_all(fixture.workspace_root.join("acme.demo")) + .await + .expect("remove package"); + + let unavailable = service.refresh(&fixture.workspace).await; + assert!(unavailable.issues.iter().any(|issue| { + issue.code == "activation_source_unavailable" && issue.source_path == "acme.demo" + })); + let (deactivated, changed, cleared_source_available) = service + .deactivate(&fixture.workspace, "acme.demo", None) + .await + .expect("clear residual activation"); + assert!(changed); + assert_eq!(cleared_source_available, Some(false)); + assert!(deactivated.discovery_complete); + assert!(deactivated.packages.is_empty()); + + let refreshed = service.refresh(&fixture.workspace).await; + assert!(refreshed.packages.is_empty()); + + let after = service + .store + .load_trust_store() + .await + .expect("load cleaned trust store"); + assert_eq!( + after.trust_level_for(&scope.project_domain_id, &scope.workspace_id, &source), + PluginPackageTrustLevel::SourceApproved + ); + assert!(!after.is_activated(&scope.project_domain_id, &scope.workspace_id, &source)); + } + + #[tokio::test] + async fn deactivation_clears_state_for_a_corrupt_package() { + let fixture = ManagedPluginFixture::new().await; + let service = fixture.service(); + fixture.activate(&service, "acme.demo").await; + tokio::fs::write( + fixture.workspace_root.join("acme.demo/bitfun.plugin.json"), + b"{not-json", + ) + .await + .expect("corrupt package manifest"); + + let (deactivated, changed, cleared_source_available) = service + .deactivate(&fixture.workspace, "acme.demo", None) + .await + .expect("clear activation for corrupt package"); + assert!(changed); + assert_eq!(cleared_source_available, Some(false)); + assert!(deactivated.discovery_complete); + assert!(deactivated.packages.is_empty()); + assert!(deactivated + .issues + .iter() + .any(|issue| issue.code == "invalid_manifest")); + } + + #[tokio::test] + async fn deactivation_invalidates_approval_for_a_replacement_source() { + let fixture = ManagedPluginFixture::new().await; + let service = fixture.service(); + let authority = fixture.activate(&service, "acme.demo").await; + let (_, _, original, _) = authority.into_parts(); + fixture + .write("acme.demo", b"export const Version = 2;") + .await; + + let (deactivated, changed, cleared_source_available) = service + .deactivate(&fixture.workspace, "acme.demo", None) + .await + .expect("deactivate replaced package"); + assert!(changed); + assert_eq!(cleared_source_available, Some(false)); + assert!(deactivated.discovery_complete); + assert!(deactivated + .packages + .iter() + .any(|package| package.package_id == "acme.demo")); + let reconciled = service + .store + .load_trust_store() + .await + .expect("load reconciled trust store"); + assert_eq!( + reconciled.trust_level_for( + &workspace_scope(&fixture.workspace).project_domain_id, + &workspace_scope(&fixture.workspace).workspace_id, + &original, + ), + PluginPackageTrustLevel::Unknown + ); + } + + #[tokio::test] + async fn stale_residual_cleanup_cannot_clear_a_newer_activation() { + let fixture = ManagedPluginFixture::new().await; + let service = fixture.service(); + let stale = fixture.activate(&service, "acme.demo").await; + service + .deactivate(&fixture.workspace, "acme.demo", None) + .await + .expect("deactivate package"); + let current = fixture.activate(&service, "acme.demo").await; + + let scope = workspace_scope(&fixture.workspace); + let (cleared_source, _, activation_epoch_mismatch) = service + .store + .clear_activation_record( + &scope.project_domain_id, + &scope.workspace_id, "acme.demo", - false, - None, - Some(first_epoch), + Some(stale.activation_epoch()), ) .await - .expect("stale rollback is a no-op"); - assert!(rollback.packages[0].activated); - assert!(!rollback_changed); - assert_eq!(rollback.activation_epoch, Some(current_epoch)); + .expect("stale cleanup is a no-op"); + assert!(cleared_source.is_none()); + assert!(activation_epoch_mismatch); + assert!(fixture.authority(&service, ¤t).await); + } + + #[tokio::test] + async fn uncertain_residual_cleanup_does_not_report_success() { + fn fail_parent_sync(_parent: &Path) -> io::Result<()> { + Err(io::Error::other("injected directory sync failure")) + } + + let fixture = ManagedPluginFixture::new().await; + let mut service = fixture.service(); + fixture.activate(&service, "acme.demo").await; + tokio::fs::remove_dir_all(fixture.workspace_root.join("acme.demo")) + .await + .expect("remove package"); + service.store.parent_sync = fail_parent_sync; + + let error = service + .deactivate(&fixture.workspace, "acme.demo", None) + .await + .expect_err("uncertain cleanup must not report success"); + + assert!(matches!( + error, + ManagedPluginSourceError::DeactivationPersistenceUncertain { .. } + )); + assert!(error.to_string().contains("may not survive a system crash")); } #[tokio::test] @@ -3490,13 +3833,7 @@ mod tests { service.store.parent_sync = fail_parent_sync; let (snapshot, changed) = service - .set_activation( - &fixture.workspace, - "acme.demo", - true, - Some(&content_hash), - None, - ) + .activate(&fixture.workspace, "acme.demo", Some(&content_hash)) .await .expect("committed activation remains successful"); @@ -3510,22 +3847,70 @@ mod tests { #[tokio::test] async fn uncertain_deactivation_does_not_report_success() { - fn fail_parent_sync(_parent: &Path) -> io::Result<()> { - Err(io::Error::other("injected directory sync failure")) + static SYNC_CALLS: std::sync::atomic::AtomicUsize = std::sync::atomic::AtomicUsize::new(0); + fn fail_parent_sync_once(_parent: &Path) -> io::Result<()> { + if SYNC_CALLS.fetch_add(1, std::sync::atomic::Ordering::SeqCst) == 0 { + Err(io::Error::other("injected directory sync failure")) + } else { + Ok(()) + } } let fixture = ManagedPluginFixture::new().await; let mut service = fixture.service(); fixture.activate(&service, "acme.demo").await; - service.store.parent_sync = fail_parent_sync; + SYNC_CALLS.store(0, std::sync::atomic::Ordering::SeqCst); + service.store.parent_sync = fail_parent_sync_once; let error = service - .set_activation(&fixture.workspace, "acme.demo", false, None, None) + .deactivate(&fixture.workspace, "acme.demo", None) .await .expect_err("uncertain deactivation must not report success"); - assert!(matches!(error, ManagedPluginSourceError::TrustStore(_))); + assert!(matches!( + error, + ManagedPluginSourceError::DeactivationPersistenceUncertain { .. } + )); assert!(error.to_string().contains("may not survive a system crash")); + + let (_, changed, _) = service + .deactivate(&fixture.workspace, "acme.demo", None) + .await + .expect("retry must confirm the committed deactivation"); + assert!(!changed); + assert_eq!(SYNC_CALLS.load(std::sync::atomic::Ordering::SeqCst), 2); + } + + #[tokio::test] + async fn uncertain_reconciliation_after_deactivation_does_not_report_success() { + static SYNC_CALLS: std::sync::atomic::AtomicUsize = std::sync::atomic::AtomicUsize::new(0); + fn fail_second_parent_sync(_parent: &Path) -> io::Result<()> { + if SYNC_CALLS.fetch_add(1, std::sync::atomic::Ordering::SeqCst) == 1 { + Err(io::Error::other("injected reconciliation sync failure")) + } else { + Ok(()) + } + } + + let fixture = ManagedPluginFixture::new().await; + let mut service = fixture.service(); + fixture.activate(&service, "acme.demo").await; + fixture + .write("acme.demo", b"export const Version = 2;") + .await; + SYNC_CALLS.store(0, std::sync::atomic::Ordering::SeqCst); + service.store.parent_sync = fail_second_parent_sync; + + let error = service + .deactivate(&fixture.workspace, "acme.demo", None) + .await + .expect_err("uncertain source reconciliation must not report success"); + + assert!(matches!( + error, + ManagedPluginSourceError::DeactivationPersistenceUncertain { .. } + )); + assert_eq!(SYNC_CALLS.load(std::sync::atomic::Ordering::SeqCst), 2); } #[tokio::test] @@ -3589,18 +3974,12 @@ mod tests { let first = fixture.activate(&service, "acme.demo").await; service - .set_activation(&fixture.workspace, "acme.demo", false, None, None) + .deactivate(&fixture.workspace, "acme.demo", None) .await .expect("deactivate package"); let content_hash = fixture.content_hash(&service, "acme.demo").await; service - .set_activation( - &fixture.workspace, - "acme.demo", - true, - Some(&content_hash), - None, - ) + .activate(&fixture.workspace, "acme.demo", Some(&content_hash)) .await .expect("recreate activation"); let second = service @@ -3847,6 +4226,43 @@ mod tests { ); } + #[test] + fn mismatched_readback_is_not_reported_as_committed() { + let temp = tempfile::tempdir().expect("tempdir"); + let target = temp.path().join("trust.json"); + std::fs::write(&target, b"old").expect("write old trust file"); + let changed_target = target.clone(); + + let error = persist_trust_bytes_with_parent_sync(&target, b"new", move |_| { + std::fs::write(&changed_target, b"different") + }) + .expect_err("mismatched readback must fail verification"); + + assert!(matches!( + error, + PluginSourceStoreError::TrustCommitUnconfirmed { .. } + )); + } + + #[test] + fn failed_parent_sync_with_mismatched_readback_is_not_a_durability_warning() { + let temp = tempfile::tempdir().expect("tempdir"); + let target = temp.path().join("trust.json"); + std::fs::write(&target, b"old").expect("write old trust file"); + let changed_target = target.clone(); + + let error = persist_trust_bytes_with_parent_sync(&target, b"new", move |_| { + std::fs::write(&changed_target, b"different")?; + Err(io::Error::other("injected directory sync failure")) + }) + .expect_err("mismatched readback must fail verification"); + + assert!(matches!( + error, + PluginSourceStoreError::TrustCommitUnconfirmed { .. } + )); + } + #[tokio::test] async fn trust_generation_identity_detects_atomic_replacement() { let temp = tempfile::tempdir().expect("tempdir");