From ca88fd74f5662f0555784ccb1a48dc82c4136527 Mon Sep 17 00:00:00 2001 From: Pavel Korolev Date: Tue, 21 Jul 2026 21:45:04 +0300 Subject: [PATCH 1/9] chore(dev): ignore local worktrees - keep isolated worktree directories out of version control --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 3f895b6..374a90c 100644 --- a/.gitignore +++ b/.gitignore @@ -24,6 +24,7 @@ claude-set-api.sh .idea/ .mcp.json .tmp/ +.worktrees/ .vscode/ # OS-specific files From af04b4c88affc3e4f632b45468f43947f93c316b Mon Sep 17 00:00:00 2001 From: Pavel Korolev Date: Sun, 26 Jul 2026 11:01:23 +0300 Subject: [PATCH 2/9] docs(build): design CDFI rollback - define transactional snapshot and recovery boundaries\n- split successful reconcile into issue #46 --- .../specs/2026-07-26-cdfi-rollback-design.md | 64 +++++++++++++++++++ 1 file changed, 64 insertions(+) create mode 100644 docs/superpowers/specs/2026-07-26-cdfi-rollback-design.md diff --git a/docs/superpowers/specs/2026-07-26-cdfi-rollback-design.md b/docs/superpowers/specs/2026-07-26-cdfi-rollback-design.md new file mode 100644 index 0000000..b3c243e --- /dev/null +++ b/docs/superpowers/specs/2026-07-26-cdfi-rollback-design.md @@ -0,0 +1,64 @@ +# ConfigDumpInfo Failure Rollback Design + +## Goal + +Protect the tracked `ConfigDumpInfo.xml` in a Designer source set from mutations made by +`/LoadConfigFromFiles -updateConfigDumpInfo` when the encompassing build does not complete +successfully. + +## Scope + +This design implements GitHub issue #24 after its scope split. It covers a byte-exact +snapshot and rollback for failed, cancelled, and pre-`UpdateDBCfg` timeout paths. It does +not alter a successful platform-produced `ConfigDumpInfo.xml` and does not implement the +`preserve-unrelated` strategy; that work is issue #46. + +## Design + +Before the Designer load command, the build use case reads the existing +`/ConfigDumpInfo.xml` as raw bytes and stores those bytes in a private, +run-scoped recovery artifact under the build runtime directory. The artifact is never +written into the tracked source root. + +The build keeps a small recovery record: source path, snapshot path, whether the source +file existed before load, and whether recovery was attempted and completed. If the Designer +load fails, cancellation is observed at the safe point before `/UpdateDBCfg`, that safe +point's timeout expires, or `/UpdateDBCfg` fails, the use case restores the original state: +it atomically replaces the current file with the captured bytes, or removes the file when +the original state was absent. A recovery failure is attached to the primary build error; +it never turns the operation into apparent success. + +On successful `UpdateDBCfg`, the snapshot is removed best-effort. Cleanup failure is +reported as a warning/degraded outcome without changing the build's successful status. + +## Result Contract + +`BuildResult` exposes an optional recovery summary for a Designer load that created a +snapshot. The summary contains the artifact path, the recovery action (`not_needed`, +`restored`, `removed_created_file`, or `failed`), and the count of byte-different CDFI +entries when it can be obtained without modifying XML. CLI JSON and MCP inherit the same +domain result; their human-readable output remains concise and points to the artifact on +recovery failure. + +## Error Handling + +The original platform/build error remains the primary error. Recovery is attempted after +the process reaches a safe outcome, never by killing Designer during the filesystem +critical section. If the snapshot cannot be created before load, the build fails before +starting Designer. If restoration fails, the error reports both the original failure and +the retained recovery artifact for manual repair. + +## Testing + +Tests use the existing Designer script harness to mutate `ConfigDumpInfo.xml` during load +and then simulate: load failure, cancellation/timeout before update, and update failure. +Each verifies byte-for-byte restoration of a BOM + CRLF + terminal-newline fixture. A +successful build verifies that the platform output is retained and the private snapshot is +cleaned up. A repeated failed run verifies idempotent restoration. Result serialization +tests cover the recovery summary and artifact location. + +## Non-goals + +- Parsing, rewriting, or reconciling XML on a successful build. +- Synthesizing UUIDs or `configVersion` values. +- Changing change-detection rules that intentionally ignore `ConfigDumpInfo.xml`. From ed15c7bd37544253c459b1b40a3bee5ad944224c Mon Sep 17 00:00:00 2001 From: Pavel Korolev Date: Sun, 26 Jul 2026 11:03:41 +0300 Subject: [PATCH 3/9] docs(build): plan CDFI rollback - define test-first recovery tasks\n- include verification and review gates --- .../plans/2026-07-26-cdfi-rollback.md | 158 ++++++++++++++++++ 1 file changed, 158 insertions(+) create mode 100644 docs/superpowers/plans/2026-07-26-cdfi-rollback.md diff --git a/docs/superpowers/plans/2026-07-26-cdfi-rollback.md b/docs/superpowers/plans/2026-07-26-cdfi-rollback.md new file mode 100644 index 0000000..f78e4d3 --- /dev/null +++ b/docs/superpowers/plans/2026-07-26-cdfi-rollback.md @@ -0,0 +1,158 @@ +# ConfigDumpInfo Rollback Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Restore tracked `ConfigDumpInfo.xml` byte-for-byte whenever a Designer build fails after the platform has begun its load. + +**Architecture:** A focused recovery helper owns private snapshot creation, atomic restoration, and cleanup. `build_project` creates a guard before `/LoadConfigFromFiles`, routes every failed path through it, and exposes recovery metadata through the existing `BuildResult` contract. + +**Tech Stack:** Rust, serde, tempfile, existing `support::fs` atomic file helpers, unit tests in `src/use_cases/build_project.rs` and CLI integration tests. + +## Global Constraints + +- Scope is GitHub #24 only; successful-build XML reconcile belongs to #46. +- Preserve original CDFI bytes, including BOM, EOL and terminal-newline state. +- Never fabricate XML, UUIDs, or platform versions. +- The primary platform error remains primary; recovery failure is attached as diagnostic context. +- TDD is mandatory: every production change begins with a failing test. +- Update `SKILL/SKILL.md` only for externally relevant command/workflow/diagnostic changes. + +--- + +### Task 1: Define transactional CDFI recovery helper + +**Files:** +- Create: `src/use_cases/build_project/cdfi_recovery.rs` +- Modify: `src/use_cases/build_project.rs` +- Test: `src/use_cases/build_project/cdfi_recovery.rs` + +**Interfaces:** +- Produces `CdfiRecoveryGuard::capture(source_root: &Path, work_path: &Path) -> Result`. +- Produces `restore(&mut self) -> Result` and `cleanup(&mut self) -> Result<(), AppError>`. +- `CdfiRecoverySummary` contains the tracked path, private snapshot path and explicit `CdfiRecoveryAction` enum. + +- [ ] **Step 1: Write failing helper tests** + +Add tests creating a `ConfigDumpInfo.xml` fixture containing UTF-8 BOM, CRLF and a terminal newline. Assert that capture followed by source mutation and `restore()` recreates the exact original bytes. Add a separate absent-file test: capture, create the file, restore, then assert it is absent. + +- [ ] **Step 2: Run the helper tests and verify RED** + +Run: `cargo test --bin v8-runner cdfi_recovery -- --format terse` + +Expected: compilation/test failure because `cdfi_recovery` and its guard do not exist. + +- [ ] **Step 3: Implement the smallest recovery guard** + +Create a run-scoped private snapshot under `work_path`, record whether the original CDFI existed, and use the existing atomic file publication helper (or its same-directory equivalent) to restore raw bytes. Do not parse XML. Return typed recovery errors instead of panicking. + +- [ ] **Step 4: Run the helper tests and verify GREEN** + +Run: `cargo test --bin v8-runner cdfi_recovery -- --format terse` + +Expected: all helper tests pass. + +- [ ] **Step 5: Commit** + +```bash +git add src/use_cases/build_project/cdfi_recovery.rs src/use_cases/build_project.rs +git commit -m "feat(build): add CDFI recovery guard" -m "- snapshot tracked ConfigDumpInfo before Designer load\n- restore raw bytes without XML rewriting" +``` + +### Task 2: Route Designer build failures through recovery + +**Files:** +- Modify: `src/use_cases/build_project.rs:420-597` +- Modify: `src/use_cases/build_project/coordinator.rs` +- Test: `src/use_cases/build_project.rs` + +**Interfaces:** +- Consumes `CdfiRecoveryGuard` from Task 1. +- Each Designer load creates its guard immediately before `/LoadConfigFromFiles`. +- Failed load, cancellation/timeout at the pre-update safe point, and failed `/UpdateDBCfg` call `restore`; completed update calls `cleanup`. + +- [ ] **Step 1: Write failing build-flow tests** + +Extend the existing fake Designer script harness so a load invocation overwrites `ConfigDumpInfo.xml`. Add tests for: non-zero load exit; interruption before `UpdateDBCfg`; and non-zero update exit. Each test must assert the original fixture bytes after `run_build` returns failure. Add a successful-load test asserting the fake platform replacement remains after success. + +- [ ] **Step 2: Run the focused tests and verify RED** + +Run: `cargo test --bin v8-runner build_project::tests::execute_build -- --format terse` + +Expected: new rollback assertions fail because build currently returns before restoring CDFI. + +- [ ] **Step 3: Integrate the recovery guard** + +Capture before the Designer load starts. Refactor early returns in `execute_source_set_step` through one recovery-aware failure path. Preserve existing partial-list diagnostic attachment. Restore only after the process/safe-point outcome is known; on recovery failure, attach its message to the original `AppError` without replacing it. + +- [ ] **Step 4: Run focused tests and verify GREEN** + +Run: `cargo test --bin v8-runner build_project::tests::execute_build -- --format terse` + +Expected: new recovery cases and existing build tests pass. + +- [ ] **Step 5: Commit** + +```bash +git add src/use_cases/build_project.rs src/use_cases/build_project/coordinator.rs +git commit -m "fix(build): restore CDFI after failed Designer build" -m "- recover snapshots on load and update failure paths\n- retain platform output after successful update" +``` + +### Task 3: Publish recovery diagnostics and documentation + +**Files:** +- Modify: `src/domain/build.rs` +- Modify: `src/cli/execute.rs` +- Modify: `tests/cli_build.rs` +- Modify: `SKILL/SKILL.md` + +**Interfaces:** +- `BuildResult` gains `cdfi_recovery: Option` with serde snake_case names. +- JSON and MCP inherit this field from `BuildResult`; human CLI output points to a retained artifact only when recovery fails. + +- [ ] **Step 1: Write failing result/CLI tests** + +Add a serialization assertion for recovery action, snapshot path, and failure diagnostic. Add one CLI JSON regression that simulates a failed Designer load and asserts recovery metadata is returned with the command failure payload. + +- [ ] **Step 2: Run result/CLI tests and verify RED** + +Run: `cargo test --test cli_build -- --format terse` + +Expected: compilation or assertion failure because `BuildResult` has no recovery field. + +- [ ] **Step 3: Implement typed result wiring and concise guidance** + +Add an exhaustive action enum and optional summary to `BuildResult`; thread the summary through coordinator failure payloads without changing successful result semantics. Document that failed Designer build restores CDFI and reports a retained artifact only if automatic restoration failed. + +- [ ] **Step 4: Run result/CLI tests and verify GREEN** + +Run: `cargo test --test cli_build -- --format terse` + +Expected: all CLI build tests pass. + +- [ ] **Step 5: Commit** + +```bash +git add src/domain/build.rs src/cli/execute.rs tests/cli_build.rs SKILL/SKILL.md +git commit -m "feat(build): report CDFI recovery diagnostics" -m "- expose typed recovery status in build results\n- document failed-build source protection" +``` + +### Task 4: Verify and prepare the pull request + +**Files:** +- Modify only if formatter or review identifies an issue. + +- [ ] **Step 1: Run quality gates** + +Run: `cargo fmt --all -- --check`, `cargo check --all-targets`, `cargo clippy --all-targets -- -D warnings`, and `git diff --check`. + +- [ ] **Step 2: Run targeted and full test suites** + +Run: `cargo test --test cli_build -- --format terse` and `cargo test --workspace -- --format terse`. Record any existing environment-specific failures separately from the new CDFI cases. + +- [ ] **Step 3: Independent Rust and whole-branch review** + +Review the branch against the design, issue #24, and the Rust best-practices checklist. Resolve every Important/Critical finding or record an explicit waiver. + +- [ ] **Step 4: Create PR** + +Push `fix/issue-24-cdfi-rollback`, create a PR against the intended upstream branch, reference `Closes #24`, and include exact verification evidence plus known baseline failures. From 00a925e341053d2c2ee6e9c77e51440b0c387029 Mon Sep 17 00:00:00 2001 From: Pavel Korolev Date: Sun, 26 Jul 2026 11:12:22 +0300 Subject: [PATCH 4/9] feat(build): add CDFI recovery guard - snapshot tracked ConfigDumpInfo before Designer load\n- restore raw bytes without XML rewriting --- src/use_cases/build_project.rs | 1 + src/use_cases/build_project/cdfi_recovery.rs | 285 +++++++++++++++++++ 2 files changed, 286 insertions(+) create mode 100644 src/use_cases/build_project/cdfi_recovery.rs diff --git a/src/use_cases/build_project.rs b/src/use_cases/build_project.rs index ead66b2..82c8c53 100644 --- a/src/use_cases/build_project.rs +++ b/src/use_cases/build_project.rs @@ -32,6 +32,7 @@ use crate::use_cases::tool_extension; use tempfile::NamedTempFile; use tracing::debug; +mod cdfi_recovery; mod coordinator; mod helpers; diff --git a/src/use_cases/build_project/cdfi_recovery.rs b/src/use_cases/build_project/cdfi_recovery.rs new file mode 100644 index 0000000..fc85645 --- /dev/null +++ b/src/use_cases/build_project/cdfi_recovery.rs @@ -0,0 +1,285 @@ +use std::fs; +use std::io::{ErrorKind, Write}; +use std::path::{Path, PathBuf}; + +use tempfile::{Builder, TempDir}; + +use crate::support::error::AppError; +use crate::support::fs::publish_file_atomically; + +const CDFI_FILE_NAME: &str = "ConfigDumpInfo.xml"; + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(super) enum CdfiRecoveryAction { + RestoredOriginal, + RemovedCreatedFile, +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub(super) struct CdfiRecoverySummary { + pub(super) tracked_path: PathBuf, + pub(super) snapshot_path: PathBuf, + pub(super) action: CdfiRecoveryAction, +} + +#[derive(Debug)] +pub(super) struct CdfiRecoveryGuard { + tracked_path: PathBuf, + snapshot_path: PathBuf, + snapshot_dir: Option, + original_exists: bool, + original_permissions: Option, +} + +impl CdfiRecoveryGuard { + pub(super) fn capture(source_root: &Path, work_path: &Path) -> Result { + let tracked_path = source_root.join(CDFI_FILE_NAME); + fs::create_dir_all(work_path).map_err(|error| { + AppError::Runtime(format!( + "failed to create CDFI recovery work directory '{}': {error}", + work_path.display() + )) + })?; + let snapshot_dir = Builder::new() + .prefix("cdfi-recovery-") + .tempdir_in(work_path) + .map_err(|error| { + AppError::Runtime(format!( + "failed to create CDFI recovery snapshot under '{}': {error}", + work_path.display() + )) + })?; + let snapshot_path = snapshot_dir.path().join(CDFI_FILE_NAME); + + let original_permissions = match fs::metadata(&tracked_path) { + Ok(metadata) => { + let bytes = fs::read(&tracked_path).map_err(|error| { + AppError::Runtime(format!( + "failed to capture CDFI '{}': {error}", + tracked_path.display() + )) + })?; + fs::write(&snapshot_path, bytes).map_err(|error| { + AppError::Runtime(format!( + "failed to write CDFI recovery snapshot '{}': {error}", + snapshot_path.display() + )) + })?; + Some(metadata.permissions()) + } + Err(error) if error.kind() == ErrorKind::NotFound => None, + Err(error) => { + return Err(AppError::Runtime(format!( + "failed to capture CDFI '{}': {error}", + tracked_path.display() + ))); + } + }; + let original_exists = original_permissions.is_some(); + + Ok(Self { + tracked_path, + snapshot_path, + snapshot_dir: Some(snapshot_dir), + original_exists, + original_permissions, + }) + } + + pub(super) fn restore(&mut self) -> Result { + let action = if self.original_exists { + self.restore_snapshot()?; + CdfiRecoveryAction::RestoredOriginal + } else { + self.remove_created_file()?; + CdfiRecoveryAction::RemovedCreatedFile + }; + + Ok(CdfiRecoverySummary { + tracked_path: self.tracked_path.clone(), + snapshot_path: self.snapshot_path.clone(), + action, + }) + } + + pub(super) fn cleanup(&mut self) -> Result<(), AppError> { + let Some(snapshot_dir) = self.snapshot_dir.as_ref() else { + return Ok(()); + }; + fs::remove_dir_all(snapshot_dir.path()).map_err(|error| { + AppError::Runtime(format!( + "failed to remove CDFI recovery snapshot '{}': {error}", + self.snapshot_path.display() + )) + })?; + self.snapshot_dir = None; + Ok(()) + } + + fn restore_snapshot(&self) -> Result<(), AppError> { + let bytes = fs::read(&self.snapshot_path).map_err(|error| { + AppError::Runtime(format!( + "failed to read CDFI recovery snapshot '{}': {error}", + self.snapshot_path.display() + )) + })?; + let parent = self.tracked_path.parent().ok_or_else(|| { + AppError::Runtime(format!( + "CDFI path has no parent: '{}'", + self.tracked_path.display() + )) + })?; + fs::create_dir_all(parent).map_err(|error| { + AppError::Runtime(format!( + "failed to create CDFI directory '{}': {error}", + parent.display() + )) + })?; + let mut staging_file = Builder::new() + .prefix(".ConfigDumpInfo.xml.restore-") + .tempfile_in(parent) + .map_err(|error| { + AppError::Runtime(format!( + "failed to create CDFI restore staging file in '{}': {error}", + parent.display() + )) + })?; + staging_file.write_all(&bytes).map_err(|error| { + AppError::Runtime(format!( + "failed to write CDFI restore staging file for '{}': {error}", + self.tracked_path.display() + )) + })?; + if let Some(permissions) = self.original_permissions.as_ref() { + staging_file + .as_file() + .set_permissions(permissions.clone()) + .map_err(|error| { + AppError::Runtime(format!( + "failed to preserve CDFI permissions for '{}': {error}", + self.tracked_path.display() + )) + })?; + } + staging_file.as_file().sync_all().map_err(|error| { + AppError::Runtime(format!( + "failed to write CDFI restore staging file for '{}': {error}", + self.tracked_path.display() + )) + })?; + publish_file_atomically(staging_file.path(), &self.tracked_path).map_err(|error| { + AppError::Runtime(format!( + "failed to restore CDFI '{}': {error}", + self.tracked_path.display() + )) + }) + } + + fn remove_created_file(&self) -> Result<(), AppError> { + match fs::remove_file(&self.tracked_path) { + Ok(()) => Ok(()), + Err(error) if error.kind() == ErrorKind::NotFound => Ok(()), + Err(error) => Err(AppError::Runtime(format!( + "failed to remove CDFI created during build '{}': {error}", + self.tracked_path.display() + ))), + } + } +} + +#[cfg(test)] +mod tests { + use std::fs; + + use tempfile::tempdir; + + use super::CdfiRecoveryGuard; + + const CDFI_FILE_NAME: &str = "ConfigDumpInfo.xml"; + + #[test] + fn restore_recreates_original_cdfi_bytes_without_rewriting_xml() { + let temp = tempdir().expect("tempdir"); + let source_root = temp.path().join("source"); + let work_path = temp.path().join("work"); + let tracked_path = source_root.join(CDFI_FILE_NAME); + let original = b"\xEF\xBB\xBF\r\n\r\n 1\r\n\r\n"; + + fs::create_dir_all(&source_root).expect("source root"); + fs::write(&tracked_path, original).expect("original CDFI"); + let mut guard = CdfiRecoveryGuard::capture(&source_root, &work_path).expect("capture"); + fs::write( + &tracked_path, + b"changed", + ) + .expect("mutate CDFI"); + + guard.restore().expect("restore"); + + assert_eq!(fs::read(&tracked_path).expect("restored CDFI"), original); + } + + #[test] + fn restore_removes_cdfi_created_after_absent_capture() { + let temp = tempdir().expect("tempdir"); + let source_root = temp.path().join("source"); + let work_path = temp.path().join("work"); + let tracked_path = source_root.join(CDFI_FILE_NAME); + + fs::create_dir_all(&source_root).expect("source root"); + let mut guard = CdfiRecoveryGuard::capture(&source_root, &work_path).expect("capture"); + fs::write(&tracked_path, b"").expect("create CDFI"); + + guard.restore().expect("restore"); + + assert!(!tracked_path.exists()); + } + + #[test] + fn cleanup_removes_private_snapshot() { + let temp = tempdir().expect("tempdir"); + let source_root = temp.path().join("source"); + let work_path = temp.path().join("work"); + + fs::create_dir_all(&source_root).expect("source root"); + let mut guard = CdfiRecoveryGuard::capture(&source_root, &work_path).expect("capture"); + let snapshot_path = guard.snapshot_path.clone(); + + guard.cleanup().expect("cleanup"); + + assert!(!snapshot_path.exists()); + } + + #[cfg(unix)] + #[test] + fn restore_preserves_original_cdfi_permissions() { + use std::os::unix::fs::PermissionsExt; + + let temp = tempdir().expect("tempdir"); + let source_root = temp.path().join("source"); + let work_path = temp.path().join("work"); + let tracked_path = source_root.join(CDFI_FILE_NAME); + + fs::create_dir_all(&source_root).expect("source root"); + fs::write(&tracked_path, b"").expect("original CDFI"); + fs::set_permissions(&tracked_path, fs::Permissions::from_mode(0o640)) + .expect("set permissions"); + let mut guard = CdfiRecoveryGuard::capture(&source_root, &work_path).expect("capture"); + fs::write( + &tracked_path, + b"", + ) + .expect("mutate CDFI"); + + guard.restore().expect("restore"); + + assert_eq!( + fs::metadata(&tracked_path) + .expect("restored metadata") + .permissions() + .mode() + & 0o777, + 0o640 + ); + } +} From e8626920012c3d5e6a888789d623b820b88f6f5b Mon Sep 17 00:00:00 2001 From: Pavel Korolev Date: Sun, 26 Jul 2026 11:17:58 +0300 Subject: [PATCH 5/9] fix(build): retain CDFI recovery snapshots - persist snapshots after failed restoration\n- use durable atomic CDFI replacement --- src/use_cases/build_project/cdfi_recovery.rs | 48 ++++++++++++++++++-- 1 file changed, 43 insertions(+), 5 deletions(-) diff --git a/src/use_cases/build_project/cdfi_recovery.rs b/src/use_cases/build_project/cdfi_recovery.rs index fc85645..138a0c7 100644 --- a/src/use_cases/build_project/cdfi_recovery.rs +++ b/src/use_cases/build_project/cdfi_recovery.rs @@ -2,10 +2,11 @@ use std::fs; use std::io::{ErrorKind, Write}; use std::path::{Path, PathBuf}; -use tempfile::{Builder, TempDir}; +use tempfile::Builder; +use uuid::Uuid; use crate::support::error::AppError; -use crate::support::fs::publish_file_atomically; +use crate::support::fs::replace_file_atomically; const CDFI_FILE_NAME: &str = "ConfigDumpInfo.xml"; @@ -26,7 +27,7 @@ pub(super) struct CdfiRecoverySummary { pub(super) struct CdfiRecoveryGuard { tracked_path: PathBuf, snapshot_path: PathBuf, - snapshot_dir: Option, + snapshot_dir: Option, original_exists: bool, original_permissions: Option, } @@ -76,6 +77,7 @@ impl CdfiRecoveryGuard { } }; let original_exists = original_permissions.is_some(); + let snapshot_dir = snapshot_dir.keep(); Ok(Self { tracked_path, @@ -106,7 +108,7 @@ impl CdfiRecoveryGuard { let Some(snapshot_dir) = self.snapshot_dir.as_ref() else { return Ok(()); }; - fs::remove_dir_all(snapshot_dir.path()).map_err(|error| { + fs::remove_dir_all(snapshot_dir).map_err(|error| { AppError::Runtime(format!( "failed to remove CDFI recovery snapshot '{}': {error}", self.snapshot_path.display() @@ -167,12 +169,19 @@ impl CdfiRecoveryGuard { self.tracked_path.display() )) })?; - publish_file_atomically(staging_file.path(), &self.tracked_path).map_err(|error| { + replace_file_atomically( + staging_file.path(), + &self.tracked_path, + &Uuid::new_v4().to_string(), + "cdfi-recovery", + ) + .map_err(|error| { AppError::Runtime(format!( "failed to restore CDFI '{}': {error}", self.tracked_path.display() )) }) + .map(|_| ()) } fn remove_created_file(&self) -> Result<(), AppError> { @@ -250,6 +259,35 @@ mod tests { assert!(!snapshot_path.exists()); } + #[cfg(unix)] + #[test] + fn failed_restore_keeps_pristine_snapshot_after_guard_is_dropped() { + use std::os::unix::fs::PermissionsExt; + + let temp = tempdir().expect("tempdir"); + let source_root = temp.path().join("source"); + let work_path = temp.path().join("work"); + let tracked_path = source_root.join(CDFI_FILE_NAME); + let original = b"\xEF\xBB\xBF\r\n"; + + fs::create_dir_all(&source_root).expect("source root"); + fs::write(&tracked_path, original).expect("original CDFI"); + let mut guard = CdfiRecoveryGuard::capture(&source_root, &work_path).expect("capture"); + let snapshot_path = guard.snapshot_path.clone(); + fs::set_permissions(&source_root, fs::Permissions::from_mode(0o500)) + .expect("block restore staging"); + + guard.restore().expect_err("restore must fail"); + drop(guard); + fs::set_permissions(&source_root, fs::Permissions::from_mode(0o700)) + .expect("restore source permissions"); + + assert_eq!( + fs::read(snapshot_path).expect("retained snapshot"), + original + ); + } + #[cfg(unix)] #[test] fn restore_preserves_original_cdfi_permissions() { From 831938db97f53e97b5b6942f76478b4c2537254f Mon Sep 17 00:00:00 2001 From: Pavel Korolev Date: Sun, 26 Jul 2026 11:28:53 +0300 Subject: [PATCH 6/9] fix(build): restore CDFI after failed Designer build - recover snapshots on load and update failure paths - retain platform output after successful update --- src/use_cases/build_project.rs | 306 +++++++++++++++++++++++++++++---- 1 file changed, 274 insertions(+), 32 deletions(-) diff --git a/src/use_cases/build_project.rs b/src/use_cases/build_project.rs index 82c8c53..fb8502d 100644 --- a/src/use_cases/build_project.rs +++ b/src/use_cases/build_project.rs @@ -36,6 +36,7 @@ mod cdfi_recovery; mod coordinator; mod helpers; +use self::cdfi_recovery::CdfiRecoveryGuard; pub(crate) use self::helpers::ensure_platform_success; use self::helpers::{ build_designer_dsl, build_ibcmd_dsl, commit_step_state, deferred_interruption_warning, @@ -453,7 +454,7 @@ fn execute_source_set_step( TimelineStageStatus::Running, ); } - let load_result = if let Some(paths) = partial_paths { + let (load_result, partial_list_file, mut recovery_guard) = if let Some(paths) = partial_paths { let list_file = partial_list_file(&config.work_path).map_err(|error| { AppError::Runtime(format!("failed to create partial list file: {error}")) })?; @@ -474,29 +475,22 @@ fn execute_source_set_step( return Err(attach_partial_load_list_path(error, partial_list)); } }; + let recovery_guard = + match CdfiRecoveryGuard::capture(load_context.path(), &config.work_path) { + Ok(recovery_guard) => recovery_guard, + Err(error) => { + let partial_list = preserve_partial_load_list(list_file); + return Err(attach_partial_load_list_path(error, partial_list)); + } + }; let load_result = designer_dsl.load_config_from_files_partial( load_context.path(), list_file.path(), extension_name(source_set), ); - match load_result { - Ok(result) if result.process.exit_code == 0 => result, - Ok(result) => { - let partial_list = preserve_partial_load_list(list_file); - ensure_platform_success("load", source_set, &result) - .map_err(|error| attach_partial_load_list_path(error, partial_list))?; - result - } - Err(error) => { - let partial_list = preserve_partial_load_list(list_file); - return Err(attach_partial_load_list_path( - AppError::from(error), - partial_list, - )); - } - } + (load_result, Some(list_file), recovery_guard) } else { - build_designer_dsl( + let designer_dsl = build_designer_dsl( context, config, binary, @@ -505,17 +499,39 @@ fn execute_source_set_step( step_index, "load", InterruptionSafetyClass::CriticalNonAbortable, - )? - .load_config_from_files_full(load_context.path(), extension_name(source_set)) - .map_err(AppError::from)? + )?; + let recovery_guard = CdfiRecoveryGuard::capture(load_context.path(), &config.work_path)?; + let load_result = designer_dsl + .load_config_from_files_full(load_context.path(), extension_name(source_set)); + (load_result, None, recovery_guard) }; - ensure_platform_success("load", source_set, &load_result)?; + let load_result = match load_result { + Ok(result) => result, + Err(error) => { + let error = + attach_partial_load_list_if_present(AppError::from(error), partial_list_file); + return Err(restore_cdfi_after_designer_failure( + error, + &mut recovery_guard, + )); + } + }; + if let Err(error) = ensure_platform_success("load", source_set, &load_result) { + let error = attach_partial_load_list_if_present(error, partial_list_file); + return Err(restore_cdfi_after_designer_failure( + error, + &mut recovery_guard, + )); + } if let Some(error) = interruption_before_safe_point( context, format!("update_db_cfg for source-set '{}'", source_set.name), ) { - return Err(error); + return Err(restore_cdfi_after_designer_failure( + error, + &mut recovery_guard, + )); } debug!( @@ -528,7 +544,7 @@ fn execute_source_set_step( "[Конфигуратор] Применение изменений", TimelineStageStatus::Running, ); - let update_result = build_designer_dsl( + let update_dsl = match build_designer_dsl( context, config, binary, @@ -537,22 +553,74 @@ fn execute_source_set_step( step_index, "update", InterruptionSafetyClass::CriticalNonAbortable, - )? - .update_db_cfg(extension_name(source_set)) - .map_err(AppError::from)?; - ensure_platform_success("update_db_cfg", source_set, &update_result)?; - - commit_step_state(source_set, commit_context, &config.work_path, commit)?; + ) { + Ok(dsl) => dsl, + Err(error) => { + return Err(restore_cdfi_after_designer_failure( + error, + &mut recovery_guard, + )) + } + }; + let update_result = match update_dsl.update_db_cfg(extension_name(source_set)) { + Ok(result) => result, + Err(error) => { + return Err(restore_cdfi_after_designer_failure( + AppError::from(error), + &mut recovery_guard, + )); + } + }; + if let Err(error) = ensure_platform_success("update_db_cfg", source_set, &update_result) { + return Err(restore_cdfi_after_designer_failure( + error, + &mut recovery_guard, + )); + } + if let Err(error) = commit_step_state(source_set, commit_context, &config.work_path, commit) { + return Err(restore_cdfi_after_designer_failure( + error, + &mut recovery_guard, + )); + } + let cleanup_warning = recovery_guard.cleanup().err().map(|error| { + format!("failed to remove CDFI recovery snapshot after successful Designer build: {error}") + }); Ok([ deferred_interruption_warning("load", &load_result), deferred_interruption_warning("update_db_cfg", &update_result), + cleanup_warning, ] .into_iter() .flatten() .collect()) } +fn restore_cdfi_after_designer_failure( + error: AppError, + recovery_guard: &mut CdfiRecoveryGuard, +) -> AppError { + match recovery_guard.restore() { + Ok(_) => error, + Err(recovery_error) => error.with_context(format!( + "failed to restore CDFI after Designer build failure: {recovery_error}" + )), + } +} + +fn attach_partial_load_list_if_present( + error: AppError, + list_file: Option, +) -> AppError { + match list_file { + Some(list_file) => { + attach_partial_load_list_path(error, preserve_partial_load_list(list_file)) + } + None => error, + } +} + fn write_partial_load_list_or_preserve( paths: &[PathBuf], source_root: &Path, @@ -734,6 +802,16 @@ mod tests { #[cfg(unix)] fn write_designer_script(path: &Path, calls_log: &Path, fail_pattern: Option<&str>) { + write_designer_script_with_load_delay(path, calls_log, fail_pattern, None); + } + + #[cfg(unix)] + fn write_designer_script_with_load_delay( + path: &Path, + calls_log: &Path, + fail_pattern: Option<&str>, + load_delay: Option, + ) { let pattern_branch = fail_pattern .map(|pattern| { format!( @@ -742,10 +820,20 @@ mod tests { ) }) .unwrap_or_default(); + let load_delay_branch = load_delay + .map(|delay| { + format!( + "if [ -n \"$load_dir\" ]; then sleep {}.{:03}; fi", + delay.as_secs(), + delay.subsec_millis() + ) + }) + .unwrap_or_default(); let body = format!( - "args=\"$*\"\nout=\"\"\nprev=\"\"\nfor arg in \"$@\"; do\n if [ \"$prev\" = \"/Out\" ]; then out=\"$arg\"; fi\n prev=\"$arg\"\ndone\nif [ -n \"$out\" ]; then printf 'designer log for %s\\n' \"$args\" > \"$out\"; fi\nprintf '%s\\n' \"$args\" >> \"{}\"\n{}\nexit 0", + "args=\"$*\"\nout=\"\"\nload_dir=\"\"\nprev=\"\"\nfor arg in \"$@\"; do\n if [ \"$prev\" = \"/Out\" ]; then out=\"$arg\"; fi\n if [ \"$prev\" = \"/LoadConfigFromFiles\" ]; then load_dir=\"$arg\"; fi\n prev=\"$arg\"\ndone\nif [ -n \"$out\" ]; then printf 'designer log for %s\\n' \"$args\" > \"$out\"; fi\nif [ -n \"$load_dir\" ]; then printf 'platform replacement\\n' > \"$load_dir/ConfigDumpInfo.xml\"; fi\nprintf '%s\\n' \"$args\" >> \"{}\"\n{}\n{}\nexit 0", calls_log.display(), - pattern_branch + load_delay_branch, + pattern_branch, ); if let Some(parent) = path.parent() { @@ -1025,6 +1113,160 @@ mod tests { ); } + #[cfg(unix)] + #[test] + fn execute_build_restores_cdfi_after_failed_designer_load() { + let dir = tempdir().expect("tempdir"); + let base = dir.path().join("base"); + let work = dir.path().join("work"); + let platform = dir.path().join("1cv8"); + let calls_log = dir.path().join("designer.calls.log"); + let original_cdfi = + b"\xEF\xBB\xBForiginal load fixture\r\n"; + create_source_tree(&base); + fs::create_dir_all(&work).expect("work"); + fs::write(base.join("main").join("ConfigDumpInfo.xml"), original_cdfi) + .expect("original CDFI"); + write_designer_script(&platform, &calls_log, Some("/LoadConfigFromFiles")); + let config = build_config( + &base, + &work, + &platform, + 20, + SourceFormat::Designer, + BuilderBackend::Designer, + ); + + let failure = run_build(&config, &build_args(true)).expect_err("load must fail"); + + assert!(failure.error.message().contains("exit code 17")); + assert_eq!( + fs::read(base.join("main").join("ConfigDumpInfo.xml")).expect("restored CDFI"), + original_cdfi + ); + } + + #[cfg(unix)] + #[test] + fn execute_build_restores_cdfi_after_interruption_before_designer_update() { + let dir = tempdir().expect("tempdir"); + let base = dir.path().join("base"); + let work = dir.path().join("work"); + let platform = dir.path().join("1cv8"); + let calls_log = dir.path().join("designer.calls.log"); + let original_cdfi = + b"\xEF\xBB\xBForiginal interruption fixture\r\n"; + create_source_tree(&base); + fs::create_dir_all(&work).expect("work"); + fs::write(base.join("main").join("ConfigDumpInfo.xml"), original_cdfi) + .expect("original CDFI"); + write_designer_script_with_load_delay( + &platform, + &calls_log, + None, + Some(Duration::from_millis(100)), + ); + let config = build_config( + &base, + &work, + &platform, + 20, + SourceFormat::Designer, + BuilderBackend::Designer, + ); + let cancellation = CancellationToken::new(); + let delayed_cancel = cancellation.clone(); + thread::spawn(move || { + thread::sleep(Duration::from_millis(20)); + delayed_cancel.cancel(); + }); + + let failure = super::execute( + &ExecutionContext::cli(CommandName::Build).with_cancellation(cancellation), + &config, + &build_args(true), + ) + .expect_err("build must stop before update"); + + assert!(failure + .error + .message() + .contains("before entering update_db_cfg for source-set 'main' safe point")); + assert_eq!( + fs::read(base.join("main").join("ConfigDumpInfo.xml")).expect("restored CDFI"), + original_cdfi + ); + let calls = fs::read_to_string(&calls_log).expect("calls"); + assert!(calls.contains("/LoadConfigFromFiles")); + assert!(!calls.contains("/UpdateDBCfg")); + } + + #[cfg(unix)] + #[test] + fn execute_build_restores_cdfi_after_failed_designer_update() { + let dir = tempdir().expect("tempdir"); + let base = dir.path().join("base"); + let work = dir.path().join("work"); + let platform = dir.path().join("1cv8"); + let calls_log = dir.path().join("designer.calls.log"); + let original_cdfi = + b"\xEF\xBB\xBForiginal update fixture\r\n"; + create_source_tree(&base); + fs::create_dir_all(&work).expect("work"); + fs::write(base.join("main").join("ConfigDumpInfo.xml"), original_cdfi) + .expect("original CDFI"); + write_designer_script(&platform, &calls_log, Some("/UpdateDBCfg")); + let config = build_config( + &base, + &work, + &platform, + 20, + SourceFormat::Designer, + BuilderBackend::Designer, + ); + + let failure = run_build(&config, &build_args(true)).expect_err("update must fail"); + + assert!(failure.error.message().contains("exit code 17")); + assert_eq!( + fs::read(base.join("main").join("ConfigDumpInfo.xml")).expect("restored CDFI"), + original_cdfi + ); + } + + #[cfg(unix)] + #[test] + fn execute_build_keeps_designer_cdfi_replacement_after_successful_update() { + let dir = tempdir().expect("tempdir"); + let base = dir.path().join("base"); + let work = dir.path().join("work"); + let platform = dir.path().join("1cv8"); + let calls_log = dir.path().join("designer.calls.log"); + let original_cdfi = + b"\xEF\xBB\xBForiginal success fixture\r\n"; + create_source_tree(&base); + fs::create_dir_all(&work).expect("work"); + fs::write(base.join("main").join("ConfigDumpInfo.xml"), original_cdfi) + .expect("original CDFI"); + write_designer_script(&platform, &calls_log, None); + let config = build_config( + &base, + &work, + &platform, + 20, + SourceFormat::Designer, + BuilderBackend::Designer, + ); + + let result = run_build(&config, &build_args(true)).expect("build"); + + assert!(result.ok); + assert_eq!( + fs::read(base.join("main").join("ConfigDumpInfo.xml")).expect("platform CDFI"), + b"platform replacement\n" + ); + } + #[cfg(unix)] #[test] fn execute_ibcmd_build_honors_interruption_before_apply_safe_point() { From 21b6f212ec1afb4823bb9e6650957cc706486c99 Mon Sep 17 00:00:00 2001 From: Pavel Korolev Date: Sun, 26 Jul 2026 11:48:43 +0300 Subject: [PATCH 7/9] feat(build): report CDFI recovery diagnostics - expose typed recovery status in build results - document failed-build source protection --- SKILL/SKILL.md | 1 + src/cli/execute.rs | 21 ++- src/domain/build.rs | 58 +++++++++ src/mcp/service.rs | 20 ++- src/use_cases/build_project.rs | 128 +++++++++++++++++-- src/use_cases/build_project/cdfi_recovery.rs | 39 +++--- src/use_cases/build_project/coordinator.rs | 22 +++- src/use_cases/build_project/helpers.rs | 1 + tests/cli_build.rs | 34 +++++ 9 files changed, 289 insertions(+), 35 deletions(-) diff --git a/SKILL/SKILL.md b/SKILL/SKILL.md index af667c6..eb9c5f8 100644 --- a/SKILL/SKILL.md +++ b/SKILL/SKILL.md @@ -72,6 +72,7 @@ v8-runner init - Source files changed and infobase may be stale: run `v8-runner build`. - Only one source-set changed: use commands that accept `--source-set ` instead of rebuilding or materializing everything. - Branch switch, rebase, large object moves, stale source-backed tool extension state, or suspicious incremental state: run `v8-runner build --full-rebuild`. +- A failed Designer build restores the prior `ConfigDumpInfo.xml` automatically. In JSON/MCP output, inspect `data.cdfi_recovery`; a retained `snapshot_path` means automatic recovery or its snapshot cleanup could not finish and manual diagnosis may be needed. - Syntax check: inspect `format` and `builder`, then choose `syntax designer-modules`, `syntax designer-config`, or `syntax edt`. - Behavior validation: run the relevant `v8-runner test ...` command; tests build first. - Missing local YAxUnit, Vanessa Automation, or onec-client-mcp-devkit setup: run diff --git a/src/cli/execute.rs b/src/cli/execute.rs index 99dc2ac..7995087 100644 --- a/src/cli/execute.rs +++ b/src/cli/execute.rs @@ -21,7 +21,7 @@ use crate::domain::artifact::{ ArtifactRef, ArtifactSet, ARTIFACT_ROLE_PACKAGE_FILE, ARTIFACT_ROLE_PLATFORM_LOG, }; use crate::domain::artifacts::{ArtifactBuildMetadata, ArtifactBuildMode, ArtifactsResult}; -use crate::domain::build::{BuildMode, BuildResult}; +use crate::domain::build::{BuildMode, BuildResult, CdfiRecoveryAction}; use crate::domain::convert::{ConvertDirection, ConvertResult, ConvertScope}; use crate::domain::dump::{DumpMode, DumpResult}; use crate::domain::execution::{ @@ -1601,7 +1601,7 @@ fn test_report(result: &TestRunResult) -> Option<&TestReport> { } fn render_build_text(result: &BuildResult, presenter: &Presenter, succeeded: bool) { - let summary = if !succeeded { + let mut summary = if !succeeded { TimelineItem::new(TimelineStatus::Failed, "Build failed") } else if result .steps @@ -1612,6 +1612,23 @@ fn render_build_text(result: &BuildResult, presenter: &Presenter, succeeded: boo } else { TimelineItem::new(TimelineStatus::Succeeded, "Build completed successfully") }; + if let Some((action, snapshot_path)) = result.cdfi_recovery.as_ref().and_then(|recovery| { + recovery + .snapshot_path + .as_ref() + .map(|snapshot_path| (&recovery.action, snapshot_path)) + }) { + let label = match action { + CdfiRecoveryAction::RestoreFailed => "CDFI recovery failed", + CdfiRecoveryAction::RestoredOriginal | CdfiRecoveryAction::RemovedCreatedFile => { + "CDFI recovery snapshot cleanup failed" + } + }; + summary = summary.with_detail(format!( + "{label}; retained snapshot: {}", + snapshot_path.display(), + )); + } presenter.print_timeline(&[summary]); } diff --git a/src/domain/build.rs b/src/domain/build.rs index 06abd99..6408810 100644 --- a/src/domain/build.rs +++ b/src/domain/build.rs @@ -5,6 +5,8 @@ pub struct BuildResult { pub ok: bool, pub steps: Vec, pub duration_ms: u64, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub cdfi_recovery: Option, } #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] @@ -24,3 +26,59 @@ pub enum BuildMode { Partial { file_count: usize }, Skipped, } + +/// Diagnostics emitted when a failed Designer build needed to protect CDFI. +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] +pub struct CdfiRecoverySummary { + pub action: CdfiRecoveryAction, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub snapshot_path: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub failure: Option, +} + +/// The outcome of attempting CDFI recovery after a failed Designer build. +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] +#[serde(rename_all = "snake_case")] +pub enum CdfiRecoveryAction { + RestoredOriginal, + RemovedCreatedFile, + RestoreFailed, +} + +#[cfg(test)] +mod tests { + use std::path::PathBuf; + + use serde_json::json; + + use super::{BuildResult, CdfiRecoveryAction, CdfiRecoverySummary}; + + #[test] + fn build_result_serializes_retained_cdfi_recovery_failure() { + let result = BuildResult { + ok: false, + steps: vec![], + duration_ms: 42, + cdfi_recovery: Some(CdfiRecoverySummary { + action: CdfiRecoveryAction::RestoreFailed, + snapshot_path: Some(PathBuf::from("/work/cdfi-recovery-42/ConfigDumpInfo.xml")), + failure: Some("permission denied while restoring CDFI".to_owned()), + }), + }; + + assert_eq!( + serde_json::to_value(result).expect("serialize build result"), + json!({ + "ok": false, + "steps": [], + "duration_ms": 42, + "cdfi_recovery": { + "action": "restore_failed", + "snapshot_path": "/work/cdfi-recovery-42/ConfigDumpInfo.xml", + "failure": "permission denied while restoring CDFI", + }, + }) + ); + } +} diff --git a/src/mcp/service.rs b/src/mcp/service.rs index 6487287..c0978e6 100644 --- a/src/mcp/service.rs +++ b/src/mcp/service.rs @@ -989,7 +989,9 @@ mod tests { AppConfig, BuildConfig, BuilderBackend, PlatformToolConfig, SourceFormat, SourceSetConfig, SourceSetPurpose, TestsConfig, ToolsConfig, }; - use crate::domain::build::{BuildMode, BuildResult, BuildStep}; + use crate::domain::build::{ + BuildMode, BuildResult, BuildStep, CdfiRecoveryAction, CdfiRecoverySummary, + }; use crate::domain::dump::{DumpMode, DumpResult}; use crate::domain::execution::{ExecutionStepKind, StepResult}; use crate::domain::issue::{Issue, IssueSeverity, ModuleIssue}; @@ -1159,6 +1161,7 @@ mod tests { duration_ms: 17, }], duration_ms: 42, + cdfi_recovery: None, })); let config = sample_config(); let service = McpService::with_port(&config, port); @@ -1200,6 +1203,11 @@ mod tests { duration_ms: 9, }], duration_ms: 19, + cdfi_recovery: Some(CdfiRecoverySummary { + action: CdfiRecoveryAction::RestoreFailed, + snapshot_path: Some("/work/cdfi-recovery/ConfigDumpInfo.xml".into()), + failure: Some("permission denied".to_owned()), + }), }, ))); let config = sample_config(); @@ -1216,6 +1224,14 @@ mod tests { assert_eq!(failure.response.command, "build"); assert_eq!(failure.response.duration_ms, 19); assert_eq!(failure.response.data["steps"][0]["ok"], false); + assert_eq!( + failure.response.data["cdfi_recovery"]["action"], + "restore_failed" + ); + assert_eq!( + failure.response.data["cdfi_recovery"]["snapshot_path"], + "/work/cdfi-recovery/ConfigDumpInfo.xml" + ); assert_eq!( failure .response @@ -2452,6 +2468,7 @@ mod tests { ok: true, steps: vec![], duration_ms: 0, + cdfi_recovery: None, })), ); @@ -2480,6 +2497,7 @@ mod tests { ok: true, steps: vec![], duration_ms: 0, + cdfi_recovery: None, })), ); diff --git a/src/use_cases/build_project.rs b/src/use_cases/build_project.rs index fb8502d..b8cd1c1 100644 --- a/src/use_cases/build_project.rs +++ b/src/use_cases/build_project.rs @@ -7,7 +7,7 @@ use std::time::Instant; use crate::change_detection::analyzer::{self, AnalysisOutcome}; use crate::change_detection::partial_load; use crate::config::model::{AppConfig, BuilderBackend, SourceFormat, SourceSetConfig}; -use crate::domain::build::{BuildMode, BuildResult}; +use crate::domain::build::{BuildMode, BuildResult, CdfiRecoverySummary}; use crate::domain::source_set::SourceSetContext; use crate::platform::edt::EdtDsl; use crate::platform::edt_session::{EdtSessionHostOptions, EdtSessionManager}; @@ -67,6 +67,30 @@ pub fn execute( pub(crate) type BuildExecutionFailure = UseCaseFailure; +#[derive(Debug)] +struct BuildStepFailure { + error: AppError, + cdfi_recovery: Option, +} + +impl BuildStepFailure { + fn with_cdfi_recovery(error: AppError, cdfi_recovery: CdfiRecoverySummary) -> Self { + Self { + error, + cdfi_recovery: Some(cdfi_recovery), + } + } +} + +impl From for BuildStepFailure { + fn from(error: AppError) -> Self { + Self { + error, + cdfi_recovery: None, + } + } +} + #[cfg(test)] pub(crate) fn run_build(config: &AppConfig, args: &BuildArgs) -> UseCaseResult { run_build_unlocked( @@ -93,6 +117,7 @@ pub(crate) fn run_build_unlocked( ok: false, steps: vec![], duration_ms: 0, + cdfi_recovery: None, }, )); } @@ -430,12 +455,12 @@ fn execute_source_set_step( step_index: usize, partial_paths: Option<&[PathBuf]>, commit: &StepCommit, -) -> Result, AppError> { +) -> Result, BuildStepFailure> { if let Some(error) = interruption_before_safe_point( context, format!("build load for source-set '{}'", source_set.name), ) { - return Err(error); + return Err(error.into()); } if let Some(paths) = partial_paths { log_timeline_stage( @@ -472,7 +497,7 @@ fn execute_source_set_step( Ok(dsl) => dsl, Err(error) => { let partial_list = preserve_partial_load_list(list_file); - return Err(attach_partial_load_list_path(error, partial_list)); + return Err(attach_partial_load_list_path(error, partial_list).into()); } }; let recovery_guard = @@ -480,7 +505,7 @@ fn execute_source_set_step( Ok(recovery_guard) => recovery_guard, Err(error) => { let partial_list = preserve_partial_load_list(list_file); - return Err(attach_partial_load_list_path(error, partial_list)); + return Err(attach_partial_load_list_path(error, partial_list).into()); } }; let load_result = designer_dsl.load_config_from_files_partial( @@ -600,12 +625,21 @@ fn execute_source_set_step( fn restore_cdfi_after_designer_failure( error: AppError, recovery_guard: &mut CdfiRecoveryGuard, -) -> AppError { +) -> BuildStepFailure { match recovery_guard.restore() { - Ok(_) => error, - Err(recovery_error) => error.with_context(format!( - "failed to restore CDFI after Designer build failure: {recovery_error}" - )), + Ok(summary) => BuildStepFailure::with_cdfi_recovery( + error, + recovery_guard.finalize_successful_restore(summary), + ), + Err(recovery_error) => { + let failure = error.with_context(format!( + "failed to restore CDFI after Designer build failure: {recovery_error}" + )); + BuildStepFailure::with_cdfi_recovery( + failure, + recovery_guard.failed_summary(&recovery_error), + ) + } } } @@ -787,7 +821,7 @@ mod tests { use std::io::ErrorKind; use std::path::{Path, PathBuf}; use std::thread; - use std::time::Duration; + use std::time::{Duration, Instant}; use tempfile::tempdir; use tokio_util::sync::CancellationToken; @@ -1144,6 +1178,16 @@ mod tests { fs::read(base.join("main").join("ConfigDumpInfo.xml")).expect("restored CDFI"), original_cdfi ); + assert!( + !fs::read_dir(&work) + .expect("work entries") + .flatten() + .any(|entry| entry + .file_name() + .to_string_lossy() + .starts_with("cdfi-recovery-")), + "successful CDFI restoration must remove its private snapshot" + ); } #[cfg(unix)] @@ -1201,6 +1245,56 @@ mod tests { assert!(!calls.contains("/UpdateDBCfg")); } + #[cfg(unix)] + #[test] + fn execute_build_restores_cdfi_after_deadline_before_designer_update() { + let dir = tempdir().expect("tempdir"); + let base = dir.path().join("base"); + let work = dir.path().join("work"); + let platform = dir.path().join("1cv8"); + let calls_log = dir.path().join("designer.calls.log"); + let original_cdfi = + b"\xEF\xBB\xBForiginal deadline fixture\r\n"; + create_source_tree(&base); + fs::create_dir_all(&work).expect("work"); + fs::write(base.join("main").join("ConfigDumpInfo.xml"), original_cdfi) + .expect("original CDFI"); + write_designer_script_with_load_delay( + &platform, + &calls_log, + None, + Some(Duration::from_millis(100)), + ); + let config = build_config( + &base, + &work, + &platform, + 20, + SourceFormat::Designer, + BuilderBackend::Designer, + ); + + let failure = super::execute( + &ExecutionContext::cli(CommandName::Build) + .with_deadline(Some(Instant::now() + Duration::from_millis(20))), + &config, + &build_args(true), + ) + .expect_err("build must stop before update after deadline"); + + assert!(failure + .error + .message() + .contains("execution timeout expired before reaching a safe completion point")); + assert_eq!( + fs::read(base.join("main").join("ConfigDumpInfo.xml")).expect("restored CDFI"), + original_cdfi + ); + let calls = fs::read_to_string(&calls_log).expect("calls"); + assert!(calls.contains("/LoadConfigFromFiles")); + assert!(!calls.contains("/UpdateDBCfg")); + } + #[cfg(unix)] #[test] fn execute_build_restores_cdfi_after_failed_designer_update() { @@ -1265,6 +1359,16 @@ mod tests { fs::read(base.join("main").join("ConfigDumpInfo.xml")).expect("platform CDFI"), b"platform replacement\n" ); + assert!( + !fs::read_dir(&work) + .expect("work entries") + .flatten() + .any(|entry| entry + .file_name() + .to_string_lossy() + .starts_with("cdfi-recovery-")), + "successful build must remove its private CDFI recovery snapshot" + ); } #[cfg(unix)] @@ -3331,10 +3435,12 @@ mod tests { }, ], duration_ms: 42, + cdfi_recovery: None, }; let json = serde_json::to_value(result).expect("json"); assert_eq!(BUILD_COMMAND, "build"); assert_eq!(json["steps"][0]["mode"], "full"); + assert!(json.get("cdfi_recovery").is_none()); } } diff --git a/src/use_cases/build_project/cdfi_recovery.rs b/src/use_cases/build_project/cdfi_recovery.rs index 138a0c7..c4d6d4e 100644 --- a/src/use_cases/build_project/cdfi_recovery.rs +++ b/src/use_cases/build_project/cdfi_recovery.rs @@ -5,24 +5,12 @@ use std::path::{Path, PathBuf}; use tempfile::Builder; use uuid::Uuid; +use crate::domain::build::{CdfiRecoveryAction, CdfiRecoverySummary}; use crate::support::error::AppError; use crate::support::fs::replace_file_atomically; const CDFI_FILE_NAME: &str = "ConfigDumpInfo.xml"; -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub(super) enum CdfiRecoveryAction { - RestoredOriginal, - RemovedCreatedFile, -} - -#[derive(Debug, Clone, PartialEq, Eq)] -pub(super) struct CdfiRecoverySummary { - pub(super) tracked_path: PathBuf, - pub(super) snapshot_path: PathBuf, - pub(super) action: CdfiRecoveryAction, -} - #[derive(Debug)] pub(super) struct CdfiRecoveryGuard { tracked_path: PathBuf, @@ -98,12 +86,33 @@ impl CdfiRecoveryGuard { }; Ok(CdfiRecoverySummary { - tracked_path: self.tracked_path.clone(), - snapshot_path: self.snapshot_path.clone(), action, + snapshot_path: None, + failure: None, }) } + pub(super) fn failed_summary(&self, error: &AppError) -> CdfiRecoverySummary { + CdfiRecoverySummary { + action: CdfiRecoveryAction::RestoreFailed, + snapshot_path: Some(self.snapshot_path.clone()), + failure: Some(error.to_string()), + } + } + + pub(super) fn finalize_successful_restore( + &mut self, + mut summary: CdfiRecoverySummary, + ) -> CdfiRecoverySummary { + if let Err(error) = self.cleanup() { + summary.snapshot_path = Some(self.snapshot_path.clone()); + summary.failure = Some(format!( + "failed to remove CDFI recovery snapshot after restoration: {error}" + )); + } + summary + } + pub(super) fn cleanup(&mut self) -> Result<(), AppError> { let Some(snapshot_dir) = self.snapshot_dir.as_ref() else { return Ok(()); diff --git a/src/use_cases/build_project/coordinator.rs b/src/use_cases/build_project/coordinator.rs index 4f06468..8b16ae0 100644 --- a/src/use_cases/build_project/coordinator.rs +++ b/src/use_cases/build_project/coordinator.rs @@ -23,6 +23,7 @@ pub(super) fn run_build_designer( ok: false, steps: vec![], duration_ms: started.elapsed().as_millis() as u64, + cdfi_recovery: None, }, )); } @@ -184,16 +185,17 @@ pub(super) fn run_build_designer( step_started.elapsed().as_millis() as u64, ), Err(error) => { - let result = fail_from_source_set_index( + let mut result = fail_from_source_set_index( started, steps, &ordered_source_sets, index, source_set, mode, - error.to_string(), + error.error.to_string(), ); - return Err(BuildExecutionFailure::with_payload(error, result)); + result.cdfi_recovery = error.cdfi_recovery; + return Err(BuildExecutionFailure::with_payload(error.error, result)); } } } @@ -204,6 +206,7 @@ pub(super) fn run_build_designer( ok: true, steps, duration_ms: started.elapsed().as_millis() as u64, + cdfi_recovery: None, }) } @@ -230,6 +233,7 @@ pub(super) fn run_build_ibcmd( ok: false, steps: vec![], duration_ms: started.elapsed().as_millis() as u64, + cdfi_recovery: None, }, )); } @@ -375,6 +379,7 @@ pub(super) fn run_build_ibcmd( ok: true, steps, duration_ms: started.elapsed().as_millis() as u64, + cdfi_recovery: None, }) } @@ -395,6 +400,7 @@ pub(super) fn run_build_edt( ok: false, steps: vec![], duration_ms: 0, + cdfi_recovery: None, }, )); } @@ -411,6 +417,7 @@ pub(super) fn run_build_edt( ok: false, steps: vec![], duration_ms: started.elapsed().as_millis() as u64, + cdfi_recovery: None, }, )); } @@ -927,6 +934,7 @@ pub(super) fn run_build_edt( partial_paths.as_deref(), &commit, ) + .map_err(BuildStepFailure::from) } }; match load_result { @@ -939,16 +947,17 @@ pub(super) fn run_build_edt( load_started.elapsed().as_millis() as u64, ), Err(error) => { - let result = fail_from_source_set_index( + let mut result = fail_from_source_set_index( started, steps, &ordered_source_sets, index, source_set, mode, - error.to_string(), + error.error.to_string(), ); - return Err(BuildExecutionFailure::with_payload(error, result)); + result.cdfi_recovery = error.cdfi_recovery; + return Err(BuildExecutionFailure::with_payload(error.error, result)); } } } @@ -959,5 +968,6 @@ pub(super) fn run_build_edt( ok: true, steps, duration_ms: started.elapsed().as_millis() as u64, + cdfi_recovery: None, }) } diff --git a/src/use_cases/build_project/helpers.rs b/src/use_cases/build_project/helpers.rs index f6f1134..05bba6e 100644 --- a/src/use_cases/build_project/helpers.rs +++ b/src/use_cases/build_project/helpers.rs @@ -608,6 +608,7 @@ pub(super) fn fail_with_remaining_steps( ok: false, steps: completed_steps, duration_ms: started.elapsed().as_millis() as u64, + cdfi_recovery: None, } } diff --git a/tests/cli_build.rs b/tests/cli_build.rs index c0409f8..1e62a48 100644 --- a/tests/cli_build.rs +++ b/tests/cli_build.rs @@ -411,6 +411,40 @@ fn build_json_failure_returns_step_payload() { .contains("exit code 17")); } +#[test] +fn build_json_failure_reports_successful_cdfi_recovery() { + let (dir, config_path, binary_path, _work_path) = setup_project(); + let cdfi_path = dir + .path() + .join("project") + .join("main") + .join("ConfigDumpInfo.xml"); + let original_cdfi = b"\xEF\xBB\xBForiginal\r\n"; + fs::write(&cdfi_path, original_cdfi).expect("original CDFI"); + write_build_script(&binary_path, Some("/LoadConfigFromFiles")); + + let output = v8_runner_command() + .args([ + "--config", + &config_path.display().to_string(), + "--json-message", + "build", + "--full-rebuild", + ]) + .output() + .expect("run command"); + + assert!(!output.status.success()); + let payload: Value = serde_json::from_slice(&output.stdout).expect("json"); + assert_eq!( + payload["data"]["cdfi_recovery"]["action"], + "restored_original" + ); + assert!(payload["data"]["cdfi_recovery"]["snapshot_path"].is_null()); + assert!(payload["data"]["cdfi_recovery"]["failure"].is_null()); + assert_eq!(fs::read(cdfi_path).expect("restored CDFI"), original_cdfi); +} + #[test] fn build_ibcmd_json_failure_reports_operation_target_and_exit_code() { let (_dir, config_path, binary_path, _work_path, _base_path, calls_log) = setup_ibcmd_project(); From 9fcb661ff88d02f393a4265194e4effbb79112a9 Mon Sep 17 00:00:00 2001 From: Pavel Korolev Date: Sun, 26 Jul 2026 17:27:58 +0300 Subject: [PATCH 8/9] fix(build): complete CDFI recovery contract - expose truthful typed recovery diagnostics on success and failure - preserve cleanup warnings and retained recovery artifacts - cover idempotent and absent-baseline recovery paths --- SKILL/SKILL.md | 2 +- src/cli/execute.rs | 39 +- src/domain/build.rs | 68 ++- src/mcp/service.rs | 19 +- src/use_cases/build_project.rs | 257 +++++++++++- src/use_cases/build_project/cdfi_recovery.rs | 416 ++++++++++++++++++- src/use_cases/build_project/coordinator.rs | 100 ++++- tests/cli_build.rs | 11 +- 8 files changed, 827 insertions(+), 85 deletions(-) diff --git a/SKILL/SKILL.md b/SKILL/SKILL.md index eb9c5f8..218c442 100644 --- a/SKILL/SKILL.md +++ b/SKILL/SKILL.md @@ -72,7 +72,7 @@ v8-runner init - Source files changed and infobase may be stale: run `v8-runner build`. - Only one source-set changed: use commands that accept `--source-set ` instead of rebuilding or materializing everything. - Branch switch, rebase, large object moves, stale source-backed tool extension state, or suspicious incremental state: run `v8-runner build --full-rebuild`. -- A failed Designer build restores the prior `ConfigDumpInfo.xml` automatically. In JSON/MCP output, inspect `data.cdfi_recovery`; a retained `snapshot_path` means automatic recovery or its snapshot cleanup could not finish and manual diagnosis may be needed. +- A failed Designer build restores the prior `ConfigDumpInfo.xml` automatically. In JSON/MCP output, inspect `data.cdfi_recovery` for the tracked path, prior-file state, changed-entry count, action, and cleanup warning. A retained `snapshot_path` is a real recovery artifact; `original_existed: false` means manual repair concerns the tracked path instead. - Syntax check: inspect `format` and `builder`, then choose `syntax designer-modules`, `syntax designer-config`, or `syntax edt`. - Behavior validation: run the relevant `v8-runner test ...` command; tests build first. - Missing local YAxUnit, Vanessa Automation, or onec-client-mcp-devkit setup: run diff --git a/src/cli/execute.rs b/src/cli/execute.rs index 7995087..9f440e0 100644 --- a/src/cli/execute.rs +++ b/src/cli/execute.rs @@ -1612,22 +1612,31 @@ fn render_build_text(result: &BuildResult, presenter: &Presenter, succeeded: boo } else { TimelineItem::new(TimelineStatus::Succeeded, "Build completed successfully") }; - if let Some((action, snapshot_path)) = result.cdfi_recovery.as_ref().and_then(|recovery| { - recovery - .snapshot_path - .as_ref() - .map(|snapshot_path| (&recovery.action, snapshot_path)) - }) { - let label = match action { - CdfiRecoveryAction::RestoreFailed => "CDFI recovery failed", - CdfiRecoveryAction::RestoredOriginal | CdfiRecoveryAction::RemovedCreatedFile => { - "CDFI recovery snapshot cleanup failed" - } + if let Some(recovery) = result.cdfi_recovery.as_deref() { + let action_label = match recovery.action { + CdfiRecoveryAction::NotNeeded => "not needed", + CdfiRecoveryAction::Restored => "restored", + CdfiRecoveryAction::RemovedCreatedFile => "removed created file", + CdfiRecoveryAction::Failed => "failed", }; - summary = summary.with_detail(format!( - "{label}; retained snapshot: {}", - snapshot_path.display(), - )); + if let Some(failure) = recovery.failure.as_deref() { + summary = summary.with_detail(format!( + "CDFI recovery {action_label} for {}: {failure}", + recovery.tracked_path.display() + )); + } + if let Some(warning) = recovery.cleanup_warning.as_deref() { + summary = summary.with_detail(format!( + "CDFI recovery cleanup warning for {}: {warning}", + recovery.tracked_path.display() + )); + } + if let Some(snapshot_path) = recovery.snapshot_path.as_ref() { + summary = summary.with_detail(format!( + "retained CDFI recovery snapshot: {}", + snapshot_path.display() + )); + } } presenter.print_timeline(&[summary]); } diff --git a/src/domain/build.rs b/src/domain/build.rs index 6408810..8fd0912 100644 --- a/src/domain/build.rs +++ b/src/domain/build.rs @@ -6,7 +6,7 @@ pub struct BuildResult { pub steps: Vec, pub duration_ms: u64, #[serde(default, skip_serializing_if = "Option::is_none")] - pub cdfi_recovery: Option, + pub cdfi_recovery: Option>, } #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] @@ -27,13 +27,18 @@ pub enum BuildMode { Skipped, } -/// Diagnostics emitted when a failed Designer build needed to protect CDFI. +/// Diagnostics emitted by a Designer build's CDFI recovery guard. #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] pub struct CdfiRecoverySummary { + pub tracked_path: std::path::PathBuf, + pub original_existed: bool, + pub changed_entry_count: Option, pub action: CdfiRecoveryAction, #[serde(default, skip_serializing_if = "Option::is_none")] pub snapshot_path: Option, #[serde(default, skip_serializing_if = "Option::is_none")] + pub cleanup_warning: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] pub failure: Option, } @@ -41,9 +46,12 @@ pub struct CdfiRecoverySummary { #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] #[serde(rename_all = "snake_case")] pub enum CdfiRecoveryAction { - RestoredOriginal, + NotNeeded, + #[serde(alias = "restored_original")] + Restored, RemovedCreatedFile, - RestoreFailed, + #[serde(alias = "restore_failed")] + Failed, } #[cfg(test)] @@ -60,11 +68,15 @@ mod tests { ok: false, steps: vec![], duration_ms: 42, - cdfi_recovery: Some(CdfiRecoverySummary { - action: CdfiRecoveryAction::RestoreFailed, + cdfi_recovery: Some(Box::new(CdfiRecoverySummary { + tracked_path: PathBuf::from("/src/ConfigDumpInfo.xml"), + original_existed: true, + changed_entry_count: Some(1), + action: CdfiRecoveryAction::Failed, snapshot_path: Some(PathBuf::from("/work/cdfi-recovery-42/ConfigDumpInfo.xml")), + cleanup_warning: None, failure: Some("permission denied while restoring CDFI".to_owned()), - }), + })), }; assert_eq!( @@ -74,11 +86,51 @@ mod tests { "steps": [], "duration_ms": 42, "cdfi_recovery": { - "action": "restore_failed", + "tracked_path": "/src/ConfigDumpInfo.xml", + "original_existed": true, + "changed_entry_count": 1, + "action": "failed", "snapshot_path": "/work/cdfi-recovery-42/ConfigDumpInfo.xml", "failure": "permission denied while restoring CDFI", }, }) ); } + + #[test] + fn build_result_serializes_successful_designer_recovery_summary() { + let result = BuildResult { + ok: true, + steps: vec![], + duration_ms: 7, + cdfi_recovery: Some(Box::new(CdfiRecoverySummary { + tracked_path: PathBuf::from("/src/ConfigDumpInfo.xml"), + original_existed: false, + changed_entry_count: Some(1), + action: CdfiRecoveryAction::NotNeeded, + snapshot_path: None, + cleanup_warning: Some( + "failed to remove CDFI recovery snapshot after successful Designer build" + .to_owned(), + ), + failure: None, + })), + }; + + assert_eq!( + serde_json::to_value(result).expect("serialize build result"), + json!({ + "ok": true, + "steps": [], + "duration_ms": 7, + "cdfi_recovery": { + "tracked_path": "/src/ConfigDumpInfo.xml", + "original_existed": false, + "changed_entry_count": 1, + "action": "not_needed", + "cleanup_warning": "failed to remove CDFI recovery snapshot after successful Designer build", + }, + }) + ); + } } diff --git a/src/mcp/service.rs b/src/mcp/service.rs index c0978e6..41a9d97 100644 --- a/src/mcp/service.rs +++ b/src/mcp/service.rs @@ -1203,11 +1203,15 @@ mod tests { duration_ms: 9, }], duration_ms: 19, - cdfi_recovery: Some(CdfiRecoverySummary { - action: CdfiRecoveryAction::RestoreFailed, + cdfi_recovery: Some(Box::new(CdfiRecoverySummary { + tracked_path: "/src/ConfigDumpInfo.xml".into(), + original_existed: true, + changed_entry_count: Some(1), + action: CdfiRecoveryAction::Failed, snapshot_path: Some("/work/cdfi-recovery/ConfigDumpInfo.xml".into()), + cleanup_warning: None, failure: Some("permission denied".to_owned()), - }), + })), }, ))); let config = sample_config(); @@ -1224,9 +1228,14 @@ mod tests { assert_eq!(failure.response.command, "build"); assert_eq!(failure.response.duration_ms, 19); assert_eq!(failure.response.data["steps"][0]["ok"], false); + assert_eq!(failure.response.data["cdfi_recovery"]["action"], "failed"); assert_eq!( - failure.response.data["cdfi_recovery"]["action"], - "restore_failed" + failure.response.data["cdfi_recovery"]["tracked_path"], + "/src/ConfigDumpInfo.xml" + ); + assert_eq!( + failure.response.data["cdfi_recovery"]["changed_entry_count"], + 1 ); assert_eq!( failure.response.data["cdfi_recovery"]["snapshot_path"], diff --git a/src/use_cases/build_project.rs b/src/use_cases/build_project.rs index b8cd1c1..f12ba61 100644 --- a/src/use_cases/build_project.rs +++ b/src/use_cases/build_project.rs @@ -70,14 +70,14 @@ pub(crate) type BuildExecutionFailure = UseCaseFailure; #[derive(Debug)] struct BuildStepFailure { error: AppError, - cdfi_recovery: Option, + cdfi_recovery: Option>, } impl BuildStepFailure { fn with_cdfi_recovery(error: AppError, cdfi_recovery: CdfiRecoverySummary) -> Self { Self { error, - cdfi_recovery: Some(cdfi_recovery), + cdfi_recovery: Some(Box::new(cdfi_recovery)), } } } @@ -91,6 +91,27 @@ impl From for BuildStepFailure { } } +impl From for Box { + fn from(error: AppError) -> Self { + Box::new(BuildStepFailure::from(error)) + } +} + +#[derive(Debug)] +struct BuildStepOutcome { + warnings: Vec, + cdfi_recovery: Option>, +} + +impl From> for BuildStepOutcome { + fn from(warnings: Vec) -> Self { + Self { + warnings, + cdfi_recovery: None, + } + } +} + #[cfg(test)] pub(crate) fn run_build(config: &AppConfig, args: &BuildArgs) -> UseCaseResult { run_build_unlocked( @@ -455,7 +476,7 @@ fn execute_source_set_step( step_index: usize, partial_paths: Option<&[PathBuf]>, commit: &StepCommit, -) -> Result, BuildStepFailure> { +) -> Result> { if let Some(error) = interruption_before_safe_point( context, format!("build load for source-set '{}'", source_set.name), @@ -608,37 +629,40 @@ fn execute_source_set_step( &mut recovery_guard, )); } - let cleanup_warning = recovery_guard.cleanup().err().map(|error| { - format!("failed to remove CDFI recovery snapshot after successful Designer build: {error}") - }); - - Ok([ + let recovery = recovery_guard.finalize_successful_build(); + let warnings = [ deferred_interruption_warning("load", &load_result), deferred_interruption_warning("update_db_cfg", &update_result), - cleanup_warning, + recovery.cleanup_warning.clone(), ] .into_iter() .flatten() - .collect()) + .collect(); + + Ok(BuildStepOutcome { + warnings, + cdfi_recovery: Some(Box::new(recovery)), + }) } fn restore_cdfi_after_designer_failure( error: AppError, recovery_guard: &mut CdfiRecoveryGuard, -) -> BuildStepFailure { +) -> Box { + let changed_entry_count = recovery_guard.changed_entry_count(); match recovery_guard.restore() { - Ok(summary) => BuildStepFailure::with_cdfi_recovery( + Ok(summary) => Box::new(BuildStepFailure::with_cdfi_recovery( error, recovery_guard.finalize_successful_restore(summary), - ), + )), Err(recovery_error) => { let failure = error.with_context(format!( "failed to restore CDFI after Designer build failure: {recovery_error}" )); - BuildStepFailure::with_cdfi_recovery( + Box::new(BuildStepFailure::with_cdfi_recovery( failure, - recovery_guard.failed_summary(&recovery_error), - ) + recovery_guard.failed_summary(&recovery_error, changed_entry_count), + )) } } } @@ -813,6 +837,7 @@ mod tests { ToolExtensionInput, ToolExtensionSourceConfig, ToolsConfig, }; use crate::domain::build::BuildMode; + use crate::domain::build::CdfiRecoveryAction; use crate::domain::source_set::SourceSetContext; use crate::use_cases::context::{CommandName, ExecutionContext}; use crate::use_cases::request::BuildRequest as BuildArgs; @@ -1103,6 +1128,13 @@ mod tests { } } + fn source_set_build_args(name: &str) -> BuildArgs { + BuildArgs { + full_rebuild: true, + source_set: Some(name.to_owned()), + } + } + #[cfg(unix)] #[test] fn execute_build_honors_interruption_before_load_safe_point() { @@ -1190,6 +1222,89 @@ mod tests { ); } + #[cfg(unix)] + #[test] + fn repeated_failed_designer_builds_restore_cdfi_idempotently() { + let dir = tempdir().expect("tempdir"); + let base = dir.path().join("base"); + let work = dir.path().join("work"); + let platform = dir.path().join("1cv8"); + let calls_log = dir.path().join("designer.calls.log"); + let tracked_path = base.join("main").join("ConfigDumpInfo.xml"); + let original_cdfi = b"\xEF\xBB\xBFrepeatable fixture\r\n"; + create_source_tree(&base); + fs::create_dir_all(&work).expect("work"); + fs::write(&tracked_path, original_cdfi).expect("original CDFI"); + write_designer_script(&platform, &calls_log, Some("/LoadConfigFromFiles")); + let config = build_config( + &base, + &work, + &platform, + 20, + SourceFormat::Designer, + BuilderBackend::Designer, + ); + + for attempt in 1..=2 { + let failure = run_build(&config, &source_set_build_args("main")) + .expect_err("load must fail on every run"); + let recovery = failure + .payload + .as_ref() + .and_then(|result| result.cdfi_recovery.as_deref()) + .expect("typed recovery"); + + assert_eq!( + fs::read(&tracked_path).expect("restored CDFI"), + original_cdfi, + "attempt {attempt} must restore the same baseline" + ); + assert_eq!(recovery.action, CdfiRecoveryAction::Restored); + assert_eq!(recovery.changed_entry_count, Some(1)); + assert_eq!(recovery.tracked_path, tracked_path); + assert!(recovery.original_existed); + assert!(recovery.snapshot_path.is_none()); + } + } + + #[cfg(unix)] + #[test] + fn failed_designer_build_restores_absent_cdfi_and_reports_result_contract() { + let dir = tempdir().expect("tempdir"); + let base = dir.path().join("base"); + let work = dir.path().join("work"); + let platform = dir.path().join("1cv8"); + let calls_log = dir.path().join("designer.calls.log"); + let tracked_path = base.join("main").join("ConfigDumpInfo.xml"); + create_source_tree(&base); + fs::create_dir_all(&work).expect("work"); + assert!(!tracked_path.exists()); + write_designer_script(&platform, &calls_log, Some("/LoadConfigFromFiles")); + let config = build_config( + &base, + &work, + &platform, + 20, + SourceFormat::Designer, + BuilderBackend::Designer, + ); + + let failure = run_build(&config, &source_set_build_args("main")) + .expect_err("load must fail after creating CDFI"); + let recovery = failure + .payload + .as_ref() + .and_then(|result| result.cdfi_recovery.as_deref()) + .expect("typed recovery"); + + assert!(!tracked_path.exists()); + assert_eq!(recovery.action, CdfiRecoveryAction::RemovedCreatedFile); + assert_eq!(recovery.tracked_path, tracked_path); + assert!(!recovery.original_existed); + assert_eq!(recovery.changed_entry_count, Some(1)); + assert!(recovery.snapshot_path.is_none()); + } + #[cfg(unix)] #[test] fn execute_build_restores_cdfi_after_interruption_before_designer_update() { @@ -1355,6 +1470,16 @@ mod tests { let result = run_build(&config, &build_args(true)).expect("build"); assert!(result.ok); + let recovery = result.cdfi_recovery.as_deref().expect("recovery summary"); + assert_eq!(recovery.action, CdfiRecoveryAction::NotNeeded); + assert_eq!( + recovery.tracked_path, + base.join("ext").join("ConfigDumpInfo.xml") + ); + assert!(!recovery.original_existed); + assert_eq!(recovery.changed_entry_count, Some(1)); + assert!(recovery.snapshot_path.is_none()); + assert!(recovery.cleanup_warning.is_none()); assert_eq!( fs::read(base.join("main").join("ConfigDumpInfo.xml")).expect("platform CDFI"), b"platform replacement\n" @@ -1371,6 +1496,106 @@ mod tests { ); } + #[cfg(unix)] + #[test] + fn successful_designer_build_reports_snapshot_cleanup_failure() { + let dir = tempdir().expect("tempdir"); + let base = dir.path().join("base"); + let work = dir.path().join("work"); + let platform = dir.path().join("1cv8"); + let calls_log = dir.path().join("designer.calls.log"); + let tracked_path = base.join("main").join("ConfigDumpInfo.xml"); + create_source_tree(&base); + fs::create_dir_all(&work).expect("work"); + fs::write( + &tracked_path, + b"cleanup baseline\n", + ) + .expect("original CDFI"); + write_designer_script(&platform, &calls_log, None); + let config = build_config( + &base, + &work, + &platform, + 20, + SourceFormat::Designer, + BuilderBackend::Designer, + ); + + let _cleanup_failure = super::cdfi_recovery::simulate_cleanup_failure( + "simulated snapshot cleanup permission failure", + ); + let result = + run_build(&config, &source_set_build_args("main")).expect("degraded successful build"); + let recovery = result.cdfi_recovery.as_deref().expect("recovery summary"); + let snapshot_path = recovery + .snapshot_path + .as_ref() + .expect("retained snapshot path"); + + assert!(result.ok); + assert_eq!(recovery.action, CdfiRecoveryAction::NotNeeded); + assert_eq!(recovery.tracked_path, tracked_path); + assert!(recovery.original_existed); + assert_eq!(recovery.changed_entry_count, Some(1)); + assert!(recovery + .cleanup_warning + .as_deref() + .is_some_and(|warning| warning.contains("failed to remove CDFI recovery snapshot"))); + assert!(snapshot_path.exists()); + } + + #[cfg(unix)] + #[test] + fn later_failure_keeps_earlier_cdfi_cleanup_artifact_visible() { + let dir = tempdir().expect("tempdir"); + let base = dir.path().join("base"); + let work = dir.path().join("work"); + let platform = dir.path().join("1cv8"); + let calls_log = dir.path().join("designer.calls.log"); + let main_cdfi = base.join("main").join("ConfigDumpInfo.xml"); + create_source_tree(&base); + fs::create_dir_all(&work).expect("work"); + fs::write( + &main_cdfi, + b"cleanup baseline\n", + ) + .expect("original CDFI"); + write_designer_script(&platform, &calls_log, Some("-Extension ext")); + let config = build_config( + &base, + &work, + &platform, + 20, + SourceFormat::Designer, + BuilderBackend::Designer, + ); + + let _cleanup_failure = super::cdfi_recovery::simulate_cleanup_failure( + "simulated first snapshot cleanup failure", + ); + let failure = run_build(&config, &build_args(true)).expect_err("extension load must fail"); + let recovery = failure + .payload + .as_ref() + .and_then(|result| result.cdfi_recovery.as_deref()) + .expect("typed recovery"); + let warning = recovery + .cleanup_warning + .as_deref() + .expect("prior cleanup warning"); + + assert!(warning.contains(&main_cdfi.display().to_string())); + assert!(warning.contains("cdfi-recovery-")); + assert!(fs::read_dir(&work) + .expect("work entries") + .flatten() + .any(|entry| entry + .file_name() + .to_string_lossy() + .starts_with("cdfi-recovery-"))); + } + #[cfg(unix)] #[test] fn execute_ibcmd_build_honors_interruption_before_apply_safe_point() { diff --git a/src/use_cases/build_project/cdfi_recovery.rs b/src/use_cases/build_project/cdfi_recovery.rs index c4d6d4e..8545d10 100644 --- a/src/use_cases/build_project/cdfi_recovery.rs +++ b/src/use_cases/build_project/cdfi_recovery.rs @@ -1,16 +1,45 @@ +use std::collections::{BTreeMap, BTreeSet}; use std::fs; use std::io::{ErrorKind, Write}; use std::path::{Path, PathBuf}; +use quick_xml::events::{BytesStart, Event}; +use quick_xml::Reader; use tempfile::Builder; use uuid::Uuid; use crate::domain::build::{CdfiRecoveryAction, CdfiRecoverySummary}; use crate::support::error::AppError; -use crate::support::fs::replace_file_atomically; +use crate::support::fs::{replace_file_atomically, ReplaceFileOutcome}; const CDFI_FILE_NAME: &str = "ConfigDumpInfo.xml"; +#[cfg(test)] +thread_local! { + static TEST_CLEANUP_FAILURE: std::cell::RefCell> = + const { std::cell::RefCell::new(None) }; +} + +#[cfg(test)] +pub(super) struct TestCleanupFailureGuard; + +#[cfg(test)] +impl Drop for TestCleanupFailureGuard { + fn drop(&mut self) { + TEST_CLEANUP_FAILURE.with(|failure| { + failure.borrow_mut().take(); + }); + } +} + +#[cfg(test)] +pub(super) fn simulate_cleanup_failure(message: &str) -> TestCleanupFailureGuard { + TEST_CLEANUP_FAILURE.with(|failure| { + *failure.borrow_mut() = Some(message.to_owned()); + }); + TestCleanupFailureGuard +} + #[derive(Debug)] pub(super) struct CdfiRecoveryGuard { tracked_path: PathBuf, @@ -77,25 +106,63 @@ impl CdfiRecoveryGuard { } pub(super) fn restore(&mut self) -> Result { - let action = if self.original_exists { - self.restore_snapshot()?; - CdfiRecoveryAction::RestoredOriginal + self.restore_with(replace_file_atomically) + } + + fn restore_with(&mut self, replace: F) -> Result + where + F: FnOnce(&Path, &Path, &str, &str) -> std::io::Result, + { + let changed_entry_count = self.changed_entry_count(); + if self.original_state_is_unchanged() { + return Ok(CdfiRecoverySummary { + tracked_path: self.tracked_path.clone(), + original_existed: self.original_exists, + changed_entry_count, + action: CdfiRecoveryAction::NotNeeded, + snapshot_path: None, + cleanup_warning: None, + failure: None, + }); + } + let cleanup_warning = if self.original_exists { + self.restore_snapshot_with(replace)?.cleanup_warning } else { self.remove_created_file()?; + None + }; + let action = if self.original_exists { + CdfiRecoveryAction::Restored + } else { CdfiRecoveryAction::RemovedCreatedFile }; Ok(CdfiRecoverySummary { + tracked_path: self.tracked_path.clone(), + original_existed: self.original_exists, + changed_entry_count, action, snapshot_path: None, + cleanup_warning, failure: None, }) } - pub(super) fn failed_summary(&self, error: &AppError) -> CdfiRecoverySummary { + pub(super) fn failed_summary( + &self, + error: &AppError, + changed_entry_count: Option, + ) -> CdfiRecoverySummary { CdfiRecoverySummary { - action: CdfiRecoveryAction::RestoreFailed, - snapshot_path: Some(self.snapshot_path.clone()), + tracked_path: self.tracked_path.clone(), + original_existed: self.original_exists, + changed_entry_count, + action: CdfiRecoveryAction::Failed, + snapshot_path: self + .original_exists + .then(|| self.snapshot_path.clone()) + .filter(|path| path.exists()), + cleanup_warning: None, failure: Some(error.to_string()), } } @@ -105,29 +172,90 @@ impl CdfiRecoveryGuard { mut summary: CdfiRecoverySummary, ) -> CdfiRecoverySummary { if let Err(error) = self.cleanup() { - summary.snapshot_path = Some(self.snapshot_path.clone()); - summary.failure = Some(format!( - "failed to remove CDFI recovery snapshot after restoration: {error}" + summary.snapshot_path = self + .snapshot_path + .exists() + .then(|| self.snapshot_path.clone()); + append_warning( + &mut summary.cleanup_warning, + format!("failed to remove CDFI recovery snapshot after restoration: {error}"), + ); + } + summary + } + + pub(super) fn finalize_successful_build(&mut self) -> CdfiRecoverySummary { + let mut summary = CdfiRecoverySummary { + tracked_path: self.tracked_path.clone(), + original_existed: self.original_exists, + changed_entry_count: self.changed_entry_count(), + action: CdfiRecoveryAction::NotNeeded, + snapshot_path: None, + cleanup_warning: None, + failure: None, + }; + if let Err(error) = self.cleanup() { + summary.snapshot_path = self + .snapshot_path + .exists() + .then(|| self.snapshot_path.clone()); + summary.cleanup_warning = Some(format!( + "failed to remove CDFI recovery snapshot after successful Designer build: {error}" )); } summary } pub(super) fn cleanup(&mut self) -> Result<(), AppError> { + #[cfg(test)] + if let Some(message) = TEST_CLEANUP_FAILURE.with(|failure| failure.borrow_mut().take()) { + return Err(AppError::Runtime(message)); + } let Some(snapshot_dir) = self.snapshot_dir.as_ref() else { return Ok(()); }; fs::remove_dir_all(snapshot_dir).map_err(|error| { AppError::Runtime(format!( - "failed to remove CDFI recovery snapshot '{}': {error}", - self.snapshot_path.display() + "failed to remove CDFI recovery directory '{}': {error}", + snapshot_dir.display() )) })?; self.snapshot_dir = None; Ok(()) } - fn restore_snapshot(&self) -> Result<(), AppError> { + pub(super) fn changed_entry_count(&self) -> Option { + let original = if self.original_exists { + Some(fs::read(&self.snapshot_path).ok()?) + } else { + None + }; + let current = match fs::read(&self.tracked_path) { + Ok(bytes) => Some(bytes), + Err(error) if error.kind() == ErrorKind::NotFound => None, + Err(_) => return None, + }; + changed_cdfi_entry_count(original.as_deref(), current.as_deref()) + } + + fn original_state_is_unchanged(&self) -> bool { + if self.original_exists { + match (fs::read(&self.snapshot_path), fs::read(&self.tracked_path)) { + (Ok(original), Ok(current)) => original == current, + (Ok(_), Err(_)) | (Err(_), Ok(_)) | (Err(_), Err(_)) => false, + } + } else { + matches!( + fs::metadata(&self.tracked_path), + Err(error) if error.kind() == ErrorKind::NotFound + ) + } + } + + fn restore_snapshot_with(&self, replace: F) -> Result + where + F: FnOnce(&Path, &Path, &str, &str) -> std::io::Result, + { let bytes = fs::read(&self.snapshot_path).map_err(|error| { AppError::Runtime(format!( "failed to read CDFI recovery snapshot '{}': {error}", @@ -178,7 +306,7 @@ impl CdfiRecoveryGuard { self.tracked_path.display() )) })?; - replace_file_atomically( + replace( staging_file.path(), &self.tracked_path, &Uuid::new_v4().to_string(), @@ -190,7 +318,6 @@ impl CdfiRecoveryGuard { self.tracked_path.display() )) }) - .map(|_| ()) } fn remove_created_file(&self) -> Result<(), AppError> { @@ -205,16 +332,147 @@ impl CdfiRecoveryGuard { } } +fn append_warning(target: &mut Option, warning: String) { + match target { + Some(existing) => { + existing.push_str("; "); + existing.push_str(&warning); + } + None => *target = Some(warning), + } +} + +type CdfiEntrySignature = Vec<(String, String)>; + +fn changed_cdfi_entry_count(original: Option<&[u8]>, current: Option<&[u8]>) -> Option { + if original == current { + return Some(0); + } + + let original_entries = match original { + Some(bytes) => parse_cdfi_entries(bytes)?, + None => BTreeMap::new(), + }; + let current_entries = match current { + Some(bytes) => parse_cdfi_entries(bytes)?, + None => BTreeMap::new(), + }; + if original_entries.is_empty() && current_entries.is_empty() { + return Some(1); + } + + let names = original_entries + .keys() + .chain(current_entries.keys()) + .collect::>(); + Some( + names + .into_iter() + .filter(|name| original_entries.get(*name) != current_entries.get(*name)) + .count(), + ) +} + +fn parse_cdfi_entries(bytes: &[u8]) -> Option> { + let mut reader = Reader::from_reader(bytes); + let mut entries = BTreeMap::new(); + + loop { + match reader.read_event().ok()? { + Event::Start(element) | Event::Empty(element) + if element.local_name().as_ref() == b"Metadata" => + { + let (name, signature) = parse_metadata_signature(&reader, &element)?; + if entries.insert(name, signature).is_some() { + return None; + } + } + Event::Eof => return Some(entries), + Event::Start(_) + | Event::End(_) + | Event::Empty(_) + | Event::Text(_) + | Event::CData(_) + | Event::Comment(_) + | Event::Decl(_) + | Event::PI(_) + | Event::DocType(_) => {} + } + } +} + +fn parse_metadata_signature( + reader: &Reader<&[u8]>, + element: &BytesStart<'_>, +) -> Option<(String, CdfiEntrySignature)> { + let mut signature = element + .attributes() + .map(|attribute| { + let attribute = attribute.ok()?; + let key = std::str::from_utf8(attribute.key.as_ref()).ok()?.to_owned(); + let value = attribute + .decode_and_unescape_value(reader.decoder()) + .ok()? + .into_owned(); + Some((key, value)) + }) + .collect::>>()?; + signature.sort(); + let name = signature + .iter() + .find_map(|(key, value)| (key == "name").then(|| value.clone())) + .filter(|value| !value.is_empty())?; + Some((name, signature)) +} + #[cfg(test)] mod tests { use std::fs; use tempfile::tempdir; - use super::CdfiRecoveryGuard; + use super::{changed_cdfi_entry_count, CdfiRecoveryGuard}; + use crate::domain::build::CdfiRecoveryAction; + use crate::support::error::AppError; + use crate::support::fs::ReplaceFileOutcome; const CDFI_FILE_NAME: &str = "ConfigDumpInfo.xml"; + #[test] + fn changed_entry_count_tracks_metadata_add_remove_and_attribute_change() { + let baseline = br#" + + + "#; + let current = br#" + + + "#; + + assert_eq!( + changed_cdfi_entry_count(Some(baseline), Some(current)), + Some(3) + ); + } + + #[test] + fn changed_entry_count_rejects_ambiguous_duplicate_metadata_names() { + let duplicate = br#" + + + "#; + + assert_eq!( + changed_cdfi_entry_count(Some(duplicate), Some(duplicate)), + Some(0), + "byte-identical snapshots are known unchanged without parsing" + ); + assert_eq!( + changed_cdfi_entry_count(Some(b""), Some(duplicate)), + None + ); + } + #[test] fn restore_recreates_original_cdfi_bytes_without_rewriting_xml() { let temp = tempdir().expect("tempdir"); @@ -237,6 +495,65 @@ mod tests { assert_eq!(fs::read(&tracked_path).expect("restored CDFI"), original); } + #[test] + fn restore_reports_not_needed_when_present_cdfi_is_unchanged() { + let temp = tempdir().expect("tempdir"); + let source_root = temp.path().join("source"); + let work_path = temp.path().join("work"); + let tracked_path = source_root.join(CDFI_FILE_NAME); + + fs::create_dir_all(&source_root).expect("source root"); + fs::write(&tracked_path, b"").expect("original CDFI"); + let mut guard = CdfiRecoveryGuard::capture(&source_root, &work_path).expect("capture"); + + let summary = guard.restore().expect("restore"); + + assert_eq!(summary.action, CdfiRecoveryAction::NotNeeded); + assert_eq!(summary.changed_entry_count, Some(0)); + } + + #[test] + fn restore_uses_raw_bytes_even_when_metadata_entry_count_is_zero() { + let temp = tempdir().expect("tempdir"); + let source_root = temp.path().join("source"); + let work_path = temp.path().join("work"); + let tracked_path = source_root.join(CDFI_FILE_NAME); + let original = br#" + + "#; + let changed = br#" + + "#; + + fs::create_dir_all(&source_root).expect("source root"); + fs::write(&tracked_path, original).expect("original CDFI"); + let mut guard = CdfiRecoveryGuard::capture(&source_root, &work_path).expect("capture"); + fs::write(&tracked_path, changed).expect("changed CDFI"); + + let summary = guard.restore().expect("restore"); + + assert_eq!(summary.action, CdfiRecoveryAction::Restored); + assert_eq!(summary.changed_entry_count, Some(0)); + assert_eq!(fs::read(tracked_path).expect("restored CDFI"), original); + } + + #[test] + fn restore_reports_not_needed_when_absent_cdfi_remains_absent() { + let temp = tempdir().expect("tempdir"); + let source_root = temp.path().join("source"); + let work_path = temp.path().join("work"); + + fs::create_dir_all(&source_root).expect("source root"); + let mut guard = CdfiRecoveryGuard::capture(&source_root, &work_path).expect("capture"); + + let summary = guard.restore().expect("restore"); + + assert_eq!(summary.action, CdfiRecoveryAction::NotNeeded); + assert_eq!(summary.changed_entry_count, Some(0)); + assert!(!summary.original_existed); + assert!(summary.snapshot_path.is_none()); + } + #[test] fn restore_removes_cdfi_created_after_absent_capture() { let temp = tempdir().expect("tempdir"); @@ -248,9 +565,72 @@ mod tests { let mut guard = CdfiRecoveryGuard::capture(&source_root, &work_path).expect("capture"); fs::write(&tracked_path, b"").expect("create CDFI"); - guard.restore().expect("restore"); + let summary = guard.restore().expect("restore"); assert!(!tracked_path.exists()); + assert_eq!(summary.tracked_path, tracked_path); + assert!(!summary.original_existed); + assert_eq!(summary.changed_entry_count, Some(1)); + assert_eq!(summary.action, CdfiRecoveryAction::RemovedCreatedFile); + assert!(summary.snapshot_path.is_none()); + } + + #[test] + fn failed_absent_baseline_recovery_points_to_tracked_file_not_missing_snapshot() { + let temp = tempdir().expect("tempdir"); + let source_root = temp.path().join("source"); + let work_path = temp.path().join("work"); + let tracked_path = source_root.join(CDFI_FILE_NAME); + + fs::create_dir_all(&source_root).expect("source root"); + let guard = CdfiRecoveryGuard::capture(&source_root, &work_path).expect("capture"); + fs::write(&tracked_path, b"").expect("created CDFI"); + + let summary = guard.failed_summary( + &AppError::Runtime("failed to remove created CDFI".to_owned()), + Some(1), + ); + + assert_eq!(summary.action, CdfiRecoveryAction::Failed); + assert_eq!(summary.tracked_path, tracked_path); + assert!(!summary.original_existed); + assert_eq!(summary.changed_entry_count, Some(1)); + assert!(summary.snapshot_path.is_none()); + } + + #[test] + fn atomic_restore_cleanup_warning_is_preserved_in_summary() { + let temp = tempdir().expect("tempdir"); + let source_root = temp.path().join("source"); + let work_path = temp.path().join("work"); + let tracked_path = source_root.join(CDFI_FILE_NAME); + let original = b"original"; + + fs::create_dir_all(&source_root).expect("source root"); + fs::write(&tracked_path, original).expect("original CDFI"); + let mut guard = CdfiRecoveryGuard::capture(&source_root, &work_path).expect("capture"); + fs::write(&tracked_path, b"changed") + .expect("changed CDFI"); + + let summary = guard + .restore_with(|staging_file, target_file, _run_id, _target_identity| { + fs::write(target_file, fs::read(staging_file)?)?; + fs::remove_file(staging_file)?; + Ok(ReplaceFileOutcome { + cleanup_warning: Some( + "failed to remove backup file '.ConfigDumpInfo.xml.backup-test'".to_owned(), + ), + }) + }) + .expect("restore"); + + assert_eq!(fs::read(&tracked_path).expect("restored CDFI"), original); + assert_eq!(summary.action, CdfiRecoveryAction::Restored); + assert_eq!(summary.changed_entry_count, Some(1)); + assert!(summary + .cleanup_warning + .as_deref() + .is_some_and(|warning| warning.contains(".ConfigDumpInfo.xml.backup-test"))); } #[test] @@ -283,6 +663,8 @@ mod tests { fs::write(&tracked_path, original).expect("original CDFI"); let mut guard = CdfiRecoveryGuard::capture(&source_root, &work_path).expect("capture"); let snapshot_path = guard.snapshot_path.clone(); + fs::write(&tracked_path, b"changed") + .expect("changed CDFI"); fs::set_permissions(&source_root, fs::Permissions::from_mode(0o500)) .expect("block restore staging"); diff --git a/src/use_cases/build_project/coordinator.rs b/src/use_cases/build_project/coordinator.rs index 8b16ae0..81b97e6 100644 --- a/src/use_cases/build_project/coordinator.rs +++ b/src/use_cases/build_project/coordinator.rs @@ -43,6 +43,7 @@ pub(super) fn run_build_designer( let mut utilities = PlatformUtilities::from_config(config); let mut designer_binary: Option = None; let mut steps = Vec::new(); + let mut cdfi_recovery = None; for (index, source_set) in ordered_source_sets.iter().enumerate() { let Some(source_context) = inventory.designer_context(&source_set.name).cloned() else { @@ -176,14 +177,17 @@ pub(super) fn run_build_designer( partial_paths.as_deref(), &commit, ) { - Ok(warnings) => push_build_step( - &mut steps, - &source_set.name, - mode, - true, - merge_step_message(message, &warnings), - step_started.elapsed().as_millis() as u64, - ), + Ok(outcome) => { + push_build_step( + &mut steps, + &source_set.name, + mode, + true, + merge_step_message(message, &outcome.warnings), + step_started.elapsed().as_millis() as u64, + ); + retain_cdfi_recovery(&mut cdfi_recovery, outcome.cdfi_recovery); + } Err(error) => { let mut result = fail_from_source_set_index( started, @@ -194,7 +198,8 @@ pub(super) fn run_build_designer( mode, error.error.to_string(), ); - result.cdfi_recovery = error.cdfi_recovery; + result.cdfi_recovery = + merge_cdfi_recovery(cdfi_recovery, error.cdfi_recovery); return Err(BuildExecutionFailure::with_payload(error.error, result)); } } @@ -206,7 +211,7 @@ pub(super) fn run_build_designer( ok: true, steps, duration_ms: started.elapsed().as_millis() as u64, - cdfi_recovery: None, + cdfi_recovery, }) } @@ -436,6 +441,7 @@ pub(super) fn run_build_edt( let mut edt_binary: Option = None; let mut interactive_edt = None; let mut steps = Vec::new(); + let mut cdfi_recovery = None; for (index, source_set) in ordered_source_sets.iter().enumerate() { let Some(edt_context) = inventory.edt_context(&source_set.name).cloned() else { @@ -934,18 +940,22 @@ pub(super) fn run_build_edt( partial_paths.as_deref(), &commit, ) - .map_err(BuildStepFailure::from) + .map(BuildStepOutcome::from) + .map_err(|error| Box::new(BuildStepFailure::from(error))) } }; match load_result { - Ok(warnings) => push_build_step( - &mut steps, - &source_set.name, - mode, - true, - merge_step_message(message, &warnings), - load_started.elapsed().as_millis() as u64, - ), + Ok(outcome) => { + push_build_step( + &mut steps, + &source_set.name, + mode, + true, + merge_step_message(message, &outcome.warnings), + load_started.elapsed().as_millis() as u64, + ); + retain_cdfi_recovery(&mut cdfi_recovery, outcome.cdfi_recovery); + } Err(error) => { let mut result = fail_from_source_set_index( started, @@ -956,7 +966,8 @@ pub(super) fn run_build_edt( mode, error.error.to_string(), ); - result.cdfi_recovery = error.cdfi_recovery; + result.cdfi_recovery = + merge_cdfi_recovery(cdfi_recovery, error.cdfi_recovery); return Err(BuildExecutionFailure::with_payload(error.error, result)); } } @@ -968,6 +979,53 @@ pub(super) fn run_build_edt( ok: true, steps, duration_ms: started.elapsed().as_millis() as u64, - cdfi_recovery: None, + cdfi_recovery, }) } + +fn retain_cdfi_recovery( + current: &mut Option>, + candidate: Option>, +) { + let Some(candidate) = candidate else { + return; + }; + if current + .as_ref() + .is_none_or(|summary| summary.cleanup_warning.is_none()) + { + *current = Some(candidate); + } +} + +fn merge_cdfi_recovery( + prior: Option>, + current: Option>, +) -> Option> { + let Some(mut current) = current else { + return prior; + }; + let Some(prior) = prior.filter(|summary| summary.cleanup_warning.is_some()) else { + return Some(current); + }; + + let mut prior_diagnostic = format!( + "earlier CDFI cleanup warning for {}: {}", + prior.tracked_path.display(), + prior.cleanup_warning.as_deref().unwrap_or_default() + ); + if let Some(snapshot_path) = prior.snapshot_path.as_ref() { + prior_diagnostic.push_str(&format!( + "; retained earlier CDFI recovery snapshot: {}", + snapshot_path.display() + )); + } + match current.cleanup_warning.as_mut() { + Some(warning) => { + warning.push_str("; "); + warning.push_str(&prior_diagnostic); + } + None => current.cleanup_warning = Some(prior_diagnostic), + } + Some(current) +} diff --git a/tests/cli_build.rs b/tests/cli_build.rs index 1e62a48..24ec5e4 100644 --- a/tests/cli_build.rs +++ b/tests/cli_build.rs @@ -436,11 +436,18 @@ fn build_json_failure_reports_successful_cdfi_recovery() { assert!(!output.status.success()); let payload: Value = serde_json::from_slice(&output.stdout).expect("json"); + assert_eq!(payload["data"]["cdfi_recovery"]["action"], "not_needed"); assert_eq!( - payload["data"]["cdfi_recovery"]["action"], - "restored_original" + payload["data"]["cdfi_recovery"]["tracked_path"], + fs::canonicalize(&cdfi_path) + .expect("canonical CDFI") + .display() + .to_string() ); + assert_eq!(payload["data"]["cdfi_recovery"]["original_existed"], true); + assert_eq!(payload["data"]["cdfi_recovery"]["changed_entry_count"], 0); assert!(payload["data"]["cdfi_recovery"]["snapshot_path"].is_null()); + assert!(payload["data"]["cdfi_recovery"]["cleanup_warning"].is_null()); assert!(payload["data"]["cdfi_recovery"]["failure"].is_null()); assert_eq!(fs::read(cdfi_path).expect("restored CDFI"), original_cdfi); } From f2d9629d583c60ab401d16e92e70201fc9d3bb93 Mon Sep 17 00:00:00 2001 From: Pavel Korolev Date: Sun, 26 Jul 2026 18:06:37 +0300 Subject: [PATCH 9/9] test(build): exercise CDFI rollback after Designer mutation --- tests/cli_build.rs | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/tests/cli_build.rs b/tests/cli_build.rs index 24ec5e4..e6d857a 100644 --- a/tests/cli_build.rs +++ b/tests/cli_build.rs @@ -13,6 +13,11 @@ const V8_EXTENSION_NATURE: &str = "com._1c.g5.v8.dt.core.V8ExtensionNature"; const EDT_RUNTIME_VERSION: &str = "8.3.27"; fn write_build_script(path: &Path, fail_pattern: Option<&str>) { + let cdfi_mutation_branch = (fail_pattern == Some("/LoadConfigFromFiles")) + .then_some( + "if [ -n \"$load_dir\" ]; then printf 'mutated by fake Designer\\n' > \"$load_dir/ConfigDumpInfo.xml\"; fi", + ) + .unwrap_or_default(); let pattern_branch = fail_pattern .map(|pattern| { format!( @@ -22,8 +27,8 @@ fn write_build_script(path: &Path, fail_pattern: Option<&str>) { }) .unwrap_or_default(); let body = format!( - "args=\"$*\"\nout=\"\"\nprev=\"\"\nfor arg in \"$@\"; do\n if [ \"$prev\" = \"/Out\" ]; then out=\"$arg\"; fi\n prev=\"$arg\"\ndone\nif [ -n \"$out\" ]; then printf 'designer log for %s\\n' \"$args\" > \"$out\"; fi\n{}\nexit 0", - pattern_branch + "args=\"$*\"\nout=\"\"\nload_dir=\"\"\nprev=\"\"\nfor arg in \"$@\"; do\n if [ \"$prev\" = \"/Out\" ]; then out=\"$arg\"; fi\n if [ \"$prev\" = \"/LoadConfigFromFiles\" ]; then load_dir=\"$arg\"; fi\n prev=\"$arg\"\ndone\nif [ -n \"$out\" ]; then printf 'designer log for %s\\n' \"$args\" > \"$out\"; fi\n{}\n{}\nexit 0", + cdfi_mutation_branch, pattern_branch ); write_script(path, &body); } @@ -436,7 +441,7 @@ fn build_json_failure_reports_successful_cdfi_recovery() { assert!(!output.status.success()); let payload: Value = serde_json::from_slice(&output.stdout).expect("json"); - assert_eq!(payload["data"]["cdfi_recovery"]["action"], "not_needed"); + assert_eq!(payload["data"]["cdfi_recovery"]["action"], "restored"); assert_eq!( payload["data"]["cdfi_recovery"]["tracked_path"], fs::canonicalize(&cdfi_path) @@ -445,7 +450,7 @@ fn build_json_failure_reports_successful_cdfi_recovery() { .to_string() ); assert_eq!(payload["data"]["cdfi_recovery"]["original_existed"], true); - assert_eq!(payload["data"]["cdfi_recovery"]["changed_entry_count"], 0); + assert_eq!(payload["data"]["cdfi_recovery"]["changed_entry_count"], 1); assert!(payload["data"]["cdfi_recovery"]["snapshot_path"].is_null()); assert!(payload["data"]["cdfi_recovery"]["cleanup_warning"].is_null()); assert!(payload["data"]["cdfi_recovery"]["failure"].is_null());