feat(registry): implement automatic registry rollback on execution failure

[HetCreep]

Resolves #612

Changes

• Intercepts script execution failures during parameter execution.
• If a failure is caught, automatically restores the pre-execution registry state using the JSON backup file via Restore-RegistryBackupState.

Summary by CodeRabbit

• Bug Fixes
  • Improved recovery when applying debloat changes fails by capturing the pre-execution registry backup path from New-RegistrySettingsBackup, wrapping the “execute actionable parameters” and “execute undo operations” loops in a try/catch, and triggering an automatic rollback via Restore-RegistryBackupState when parameter execution fails or when $script:RegistryImportFailures increases during either phase; rollback errors are logged and the original failure is re-thrown.

[Raphire]

Heya,

I do like this idea, I'll take a look at this soon when I have time. Thanks!

[Raphire]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

ExecuteAllChanges in Scripts/Features/ExecuteChanges.ps1 now retains the filesystem path returned by New-RegistrySettingsBackup in a $backupFile variable. The feature execution and undo loops are wrapped in a try/catch that monitors $script:RegistryImportFailures for increases during execution; on failure, if $backupFile is set, the catch block calls Load-RegistryBackupFromFile and Restore-RegistryBackupState to restore the registry, then re-throws the error.

Changes

Automatic Registry Rollback on Execution Failure

Layer / File(s)

Summary

Backup capture, execution monitoring, and rollback Scripts/Features/ExecuteChanges.ps1

$backupFile is initialized to $null and assigned the return value of New-RegistrySettingsBackup instead of discarding it. The feature-execution and undo-operation loops are placed inside a try block that captures the initial $script:RegistryImportFailures counter and throws if it increases during any parameter execution or undo operation. A new catch block logs the failure, conditionally invokes Load-RegistryBackupFromFile and Restore-RegistryBackupState using the captured $backupFile, logs rollback success or failure, and re-throws the original error.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly describes the main change: implementing automatic registry rollback on execution failure, matching the PR's core objective.
Linked Issues check	✅ Passed	The implementation fully addresses issue #612 by wrapping execution logic in try-catch, detecting failures, and performing automatic registry rollback using the backup file.
Out of Scope Changes check	✅ Passed	All changes are focused on implementing automatic registry rollback functionality as specified in issue #612; no unrelated modifications detected.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@Scripts/Features/ExecuteChanges.ps1`:
- Around line 206-252: The ImportRegistryFile function call within the undo
operations section records failures in $script:RegistryImportFailures without
throwing an exception, which prevents the catch block from executing and rolling
back earlier registry writes. To fix this, either modify ImportRegistryFile to
throw an exception when it increments the failure counter, or add a check inside
the try block after the ImportRegistryFile call to throw an exception if
$script:RegistryImportFailures has been incremented. This ensures that registry
import failures trigger the catch block for proper rollback handling.
- Line 255: The Test-Path check for the backup file on line 255 needs to use the
-LiteralPath parameter instead of treating the path as a pattern. Change the
condition that checks "if ($backupFile -and (Test-Path $backupFile))" to use
"-LiteralPath $backupFile" in the Test-Path call, which will prevent wildcard
characters like brackets from being interpreted as patterns and ensure the
rollback attempt is not skipped for valid paths containing these characters.
This should match the approach already used in the Load-RegistryBackupFromFile
function.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 45c65df7-0016-44ac-a8f1-82e3cb495fe3

📥 Commits

Reviewing files that changed from the base of the PR and between 2b97021 and 328a6e3.

📒 Files selected for processing (1)

• Scripts/Features/ExecuteChanges.ps1

[coderabbitai]

🧹 Nitpick comments (1)

Scripts/Features/ExecuteChanges.ps1 (1)

287-290: 💤 Low value

Dead code: this warning block is now unreachable.

With the new failure detection at lines 242-244 and 266-268 throwing when $script:RegistryImportFailures increases, this block can never execute with a non-zero failure count:

• If failures occur → exception thrown → function exits before reaching here
• If no failures → $script:RegistryImportFailures remains 0 → condition is false

Consider removing this block since failure messaging is now handled by the exception path.

♻️ Suggested removal

     }
     throw
 }
-
-if ($script:RegistryImportFailures -gt 0) {
-    Write-Host ""
-    Write-Host "$($script:RegistryImportFailures) registry import change(s) failed. See output above for details." -ForegroundColor Yellow
-}
 }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Scripts/Features/ExecuteChanges.ps1` around lines 287 - 290, The warning
block that checks if $script:RegistryImportFailures is greater than 0 is now
unreachable dead code. With the new failure detection logic that throws
exceptions when $script:RegistryImportFailures increases (added in the preceding
validation blocks), the function will exit via exception before reaching this
warning block, so the condition can never be true. Remove the entire if block
(the condition check and the Write-Host statements within it) since failure
messaging is now handled by the exception throwing paths in the earlier
validation logic.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@Scripts/Features/ExecuteChanges.ps1`:
- Around line 287-290: The warning block that checks if
$script:RegistryImportFailures is greater than 0 is now unreachable dead code.
With the new failure detection logic that throws exceptions when
$script:RegistryImportFailures increases (added in the preceding validation
blocks), the function will exit via exception before reaching this warning
block, so the condition can never be true. Remove the entire if block (the
condition check and the Write-Host statements within it) since failure messaging
is now handled by the exception throwing paths in the earlier validation logic.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: cf61878b-3d83-4033-aead-398f21c3b0be

📥 Commits

Reviewing files that changed from the base of the PR and between 71ef6c2 and d4a57cb.

📒 Files selected for processing (1)

• Scripts/Features/ExecuteChanges.ps1

[HetCreep]

Heads-up on the merge conflict here: #641 refactored ExecuteChanges.ps1 into InvokeChanges.ps1 and split the execute/undo loops out into Invoke-ApplyFeatures / Invoke-UndoFeatures (called from Invoke-AllChanges). This PR's rollback wraps those loops in a single try/catch inside the old monolithic function, so it needs re-applying onto the new phased structure rather than a mechanical rebase.

Since it's the registry-rollback safety path and I can't run-test the restore on my end, I'd rather not force-push an untested rewrite of it. Would you prefer I redo it against the new structure (you'd want to verify the rollback/restore path before merge), or would you rather fold it into your own refactor? Either works for me -- just didn't want to decide that for you.