(pre): pre backup PowerShell scriptIntroduce preflight-backup.ps1 to orchestrate backupand manifest generation forative-windows backups. The scriptadds argument parsing, config resolution, helper to- resolve effective and repository paths- map folders and extra into normalized backup rules- detect drive paths and destination usage unless forced- query git remote URL and resolve common known-folder locations- normalize rule IDs and build backup rule- assert and create destination root and scaffold a robocopy-based directory copy helper (partial)#7
(pre): pre backup PowerShell scriptIntroduce preflight-backup.ps1 to orchestrate backupand manifest generation forative-windows backups. The scriptadds argument parsing, config resolution, helper to- resolve effective and repository paths- map folders and extra into normalized backup rules- detect drive paths and destination usage unless forced- query git remote URL and resolve common known-folder locations- normalize rule IDs and build backup rule- assert and create destination root and scaffold a robocopy-based directory copy helper (partial)#7
Conversation
…orchestrate backupand manifest generation forative-windows backups. The scriptadds argument parsing, config resolution, helper to- resolve effective and repository paths- map folders and extra into normalized backup rules- detect drive paths and destination usage unless forced- query git remote URL and resolve common known-folder locations- normalize rule IDs and build backup rule- assert and create destination root and scaffold a robocopy-based directory copy helper (partial) These changes centralize pre checks, ensure consistent rulegeneration templates, and prevent accidental backups thesystem drive. This provides a safer, more declarative foundation forsubsequent backup and steps
📝 WalkthroughWalkthroughThis PR introduces a comprehensive backup-and-restore workflow for declarative-windows. It adds new PowerShell scripts ( Changes
Sequence DiagramsequenceDiagram
participant User
participant Autounattend as Autounattend.xml
participant Bootstrap as bootstrap.ps1
participant Backup as preflight-backup.ps1
participant Registry as apply-registry.ps1
participant Sophia as Sophia Script
participant Restore as restore-backup.ps1
participant Desktop as Desktop Shortcuts
User->>Backup: Run preflight-backup.ps1<br/>(before ISO)
Backup->>Backup: Read backup.json config
Backup->>Backup: Copy known folders +<br/>extra paths to manifest
Backup->>Backup: Export WinGet inventory
Backup->>Backup: Write backup-manifest.json
User->>Autounattend: Boot Windows 11 ISO
Autounattend->>Autounattend: Wait for network (300s)
Autounattend->>Bootstrap: Run FirstLogonCommand
Bootstrap->>Bootstrap: Initialize state.json
Bootstrap->>Bootstrap: Check/restore from<br/>backup-manifest.json
Bootstrap->>Bootstrap: Download & extract<br/>Sophia Script
Bootstrap->>Bootstrap: Run Sophia preset
Bootstrap->>Registry: Apply registry.json
Registry->>Registry: Normalize & set<br/>registry values
Registry->>Bootstrap: Return results
Bootstrap->>Desktop: Create rerun shortcut
Bootstrap->>Desktop: Create restore shortcut
Bootstrap->>Bootstrap: Save state (done)
User->>Desktop: Click "Restore My Files"
Desktop->>Restore: Invoke restore-backup.ps1
Restore->>Restore: Find backup-manifest.json
Restore->>Restore: Restore files by mode<br/>(Merge/SkipExisting/Overwrite)
Restore->>Restore: Write restore-report.json
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
declarative-windows/build-iso.ps1
Line 224 in ab02e84
Define $MountDir before creating temp directories
build-iso.ps1 removes the $MountDir declaration but still calls New-Item -Path $MountDir, so this argument is null at runtime. With $ErrorActionPreference = "Stop", ISO builds fail immediately during temp directory setup before mounting the source ISO.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| Set-Content -Path $WingetMarker -Value $appsHash -Force | ||
| Add-SummaryItem -Step "WinGet" -Status "WARN" -Message "$failCount package(s) failed - see Failed Installs.txt" | ||
| Set-StepState -StepId $stepId -Status "done" -Message "$failCount package(s) failed" |
There was a problem hiding this comment.
When winget import leaves packages unresolved, the code still sets the step state to done and writes the completion marker hash. Because Should-RunStep skips done steps, a normal rerun will not retry those failed packages (unless the user remembers -Force), which breaks the resume/retry behavior for transient installation failures.
Useful? React with 👍 / 👎.
| $currentProfile = [Environment]::ExpandEnvironmentVariables("%USERPROFILE%") | ||
| if ($expandedPath.StartsWith($currentProfile, [System.StringComparison]::OrdinalIgnoreCase)) { | ||
| $relativePath = $expandedPath.Substring($currentProfile.Length).TrimStart('\\') |
There was a problem hiding this comment.
Resolve-RestoreTargetPath only rewrites paths when they start with the current %USERPROFILE%, but backup manifests store absolute source paths from the machine/account that created the backup. If the username/profile root changes after reinstall, this condition is false and restore targets stay pointed at the old profile path instead of -DestinationProfileRoot, so user files can be restored into the wrong location.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
🧹 Nitpick comments (1)
autounattend.xml (1)
156-165: Network wait implementation looks good with one consideration.The 300-second network wait with 5-second intervals is well-designed and complements
bootstrap.ps1's ownWait-ForNetworkfunction. The timeout matches the bootstrap script's default.Consider: On timeout,
exit 1will cause the SynchronousCommand to fail, which may prevent subsequent FirstLogonCommands (including bootstrap.ps1) from running. If this is intentional (fail-fast when no network), that's a valid design choice. If you prefer bootstrap.ps1 to still run and handle the network-unavailable state gracefully, considerexit 0instead.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@autounattend.xml` around lines 156 - 165, The current SynchronousCommand that polls network connectivity uses "exit 1" on timeout which will mark the command as failed and prevent later FirstLogonCommands (including the powershell call to bootstrap.ps1) from running; decide desired behavior and update the CommandLine in the SynchronousCommand (the powershell invocation that implements the timeout loop) to either return success on timeout (change the terminal exit to exit 0 or remove the non-zero exit) so bootstrap.ps1 still runs and can handle network unavailability, or keep exit 1 if you intentionally want a fail-fast behavior—ensure any change references the same SynchronousCommand/CommandLine that runs powershell and the bootstrap.ps1 FirstLogonCommand so the chosen flow is consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apply-registry.ps1`:
- Around line 55-65: The current block validates only path/name/type and then
converts $entry.value to [int] for DWord, which lets missing or malformed values
become 0; update the logic around $desiredValue (used after
Normalize-RegistryPath and Convert-RegistryType) to explicitly validate that
$entry.value exists and is of the correct form before applying: for DWord types
ensure $entry.value is present and numeric (reject or throw with a clear message
if not), for other types ensure non-null/expected type, and avoid silently
casting invalid/missing values to defaults; use Convert-RegistryType,
Normalize-RegistryPath, the $valueType check for "DWord" and the $desiredValue
variable to locate and enforce these validations.
In `@bootstrap.ps1`:
- Around line 431-451: The Find-BackupManifest function currently returns the
first match found per drive, which can pick an older manifest if a later drive
has a newer file; change it to collect all matching "backup-manifest.json" files
from every candidate drive (accumulate $match results instead of returning
immediately), then sort the combined set by LastWriteTimeUtc descending and
return the FullName of the first (newest) item; apply the same fix to the
equivalent helper in restore-backup.ps1 so both helpers choose the newest
manifest across all drives.
- Around line 646-663: The code writes $WingetMarker and marks the step "done"
even when $stillMissing is non-empty, causing reruns to skip WinGet despite
partial failures; change the logic so Set-Content -Path $WingetMarker and the
success Set-StepState/Add-SummaryItem calls only occur in the if (-not
$stillMissing) branch, and in the else branch do NOT call Set-Content or mark
the step as "done" — instead set the step status to a failing/pending state (use
Set-StepState -StepId $stepId with a non-"done" status and update
Add-SummaryItem to reflect WARN/FAIL) so the run remains retryable and the
marker is only persisted after a clean import.
- Around line 900-920: The summary/reporting block is currently skipped when
Should-RunStep returns false, leaving desktop reports stale on non-dry reruns;
remove or bypass the Should-RunStep check so that when -not $DryRun the code
always calls Write-FailedInstallsReport and Write-SummaryReport (using
$desktopPath and inspecting $FailedItems) to overwrite the existing "Setup
Summary.txt" and "Failed Installs.txt", retain the DryRun branch that sets
Set-StepState -StepId $stepId -Status "pending", and ensure the final
Set-StepState -StepId $stepId -Status "done" -Message "Summary report written"
is still called after the reports are written.
In `@build-iso.ps1`:
- Around line 278-283: The script currently copies the entire config folder
($configSource -> $configDestination) with Copy-Item, which bakes user-specific
files (e.g. config\backup.json) into the ISO; change this to a whitelist-based
copy: define an explicit list of allowed config filenames (or patterns) and only
copy those into $configDestination (use Get-ChildItem/Copy-Item per entry or
loop over the whitelist), ensuring $configSource, $configDestination and the
Copy-Item usage are updated to skip any non-whitelisted files like backup.json
so the ISO payload is deterministic and user data is not included.
- Around line 136-168: The download block that fetches $DownloadUrl (producing
$downloadPath / $oscdimgFile) must validate authenticity before
returning/executing the binary: require HTTPS on $DownloadUrl, then after
downloading compute a SHA-256 hash (or call Get-AuthenticodeSignature) for the
resolved oscdimg.exe and compare it to an allowlist or require a Valid signature
status; if verification fails, log a clear error via Write-ErrorMessage and
throw instead of returning the path. Update the branches that return
$oscdimgFile.FullName or $downloadPath to perform this verification step (refer
to variables $DownloadUrl, $downloadPath, $oscdimgFile and the
download/Expand-Archive logic) and only return when signature/hash check passes.
Ensure failures abort with an explanatory message.
In `@docs/ISO-GENERATION.md`:
- Around line 58-72: Update the Installed System section of
docs/ISO-GENERATION.md to reflect the actual payload produced by build-iso.ps1:
add restore-backup.ps1 to the root list and enumerate all files now copied under
the config\ directory (and any subfiles) so the C:\Setup tree matches what
build-iso.ps1 produces; locate the current tree example in ISO-GENERATION.md and
cross-check build-iso.ps1, restore-backup.ps1 and the config\ contents to ensure
the document lists every file users will expect when restoring or
troubleshooting.
- Around line 21-27: Replace the invalid PowerShell line-continuation
backslashes in the example command with proper PowerShell backticks or present
the command as a single line; specifically update the example invocation of
build-iso.ps1 (the block showing .\build-iso.ps1 -SourceISO -OutputISO
-SourceIsoHash -IsoLabel) so each continued line ends with a backtick (`) or
collapse the entire invocation onto one line to ensure the example runs in
PowerShell.
In `@docs/TROUBLESHOOTING.md`:
- Around line 220-228: Add a markdown language specifier to the code block that
shows the USB tree by changing the opening fence from ``` to ```text so linters
(MD040) recognize it, and fix the inconsistent path elsewhere by replacing the
incorrect `$OEM$\$$\Setup\` reference with the corrected `$OEM$\$1\Setup\`
(ensure both occurrences match the `build-iso.ps1` convention and the snippet
containing `USB:\ ├── autounattend.xml` and `└── $OEM$\$1\Setup\` are updated).
In `@preflight-backup.ps1`:
- Around line 366-367: The assignment to the backup.destinationRoot uses
Resolve-Path which throws for non-existent paths and breaks dry-run; change it
to safely resolve only when the path exists or fall back to the literal input
(e.g., replace (Resolve-Path $DestinationRoot).Path with a non-throwing
expression such as: if (Test-Path $DestinationRoot) { (Resolve-Path
$DestinationRoot).Path } else { $DestinationRoot } or use Resolve-Path
-ErrorAction SilentlyContinue and default to $DestinationRoot so -WhatIf runs
without error; update the code around the backup = [ordered]@{ destinationRoot
assignment accordingly.
- Around line 71-82: Get-RepoRemoteUrl currently returns the raw output of `git
remote get-url origin`, which may include embedded userinfo/PATs; update
Get-RepoRemoteUrl to sanitize the URL before returning by removing any userinfo
(username[:password]@) portion. Locate the Get-RepoRemoteUrl function and, after
obtaining $remoteUrl, parse it (e.g., via .NET [uri] or a regex) to strip the
UserInfo component and return/emit only the scheme://host[:port]/path form (no
credentials) so that any value written later to backup-manifest.json or
backup-report.txt contains no embedded secrets.
- Around line 207-212: The foreach over $ExcludePatterns wrongly strips patterns
to their leaf and rejects anything containing '*'/'?', so no excludes reach
robocopy and /XF (file-exclude) is used for directory patterns; update the loop
that references $ExcludePatterns, Split-Path and $leaf so it does not discard
globbed directory patterns but instead recognizes directory globs (e.g. patterns
containing '\**' or ending/starting with '**\...') and maps them to robocopy
directory excludes by appending "/XD <dirName>" to $robocopyArgs (use the base
directory name like "node_modules" or ".git"), while preserving file patterns to
"/XF" where appropriate; remove the $leaf -notmatch '[\*\?]' gate that filters
out globs and ensure robocopyArgs uses "/XD" for directories and "/XF" only for
explicit file names.
In `@restore-backup.ps1`:
- Around line 133-135: The restore currently expands $manifest.repo.restorePath
directly into $repoTargetRoot (using the active %USERPROFILE%) which ignores the
-DestinationProfileRoot option; instead call Resolve-RestoreTargetPath (the
helper that respects -DestinationProfileRoot) to compute the repo root and
assign that to $repoTargetRoot, then use Join-Path with $repoTargetRoot and
$repoFile.relativePath to build $destination (and keep $destinationParent =
Split-Path -Path $destination -Parent unchanged); update references to
$manifest.repo.restorePath in this block to use the resolved path returned by
Resolve-RestoreTargetPath.
- Around line 19-27: Update the helper logging functions so they write both to
the console and to a file-backed install log (C:\Setup\install.log); modify or
replace Write-Info and Write-Success (and add corresponding
Write-Warning/Write-Failure if missing) to append entries to the file using the
required symbols ("✓ completed", "⚠ skipped", "✗ failed") and include timestamps
and a clear message, while preserving the existing console output (Write-Host).
Ensure file creation/append uses safe file locking/encoding and that all restore
steps call these file-backed helpers so every restore run produces comprehensive
entries in C:\Setup\install.log.
- Around line 78-85: Copy-Tree currently creates $destinationParent before
calling $PSCmdlet.ShouldProcess and reports success even when the operation is
skipped; move all filesystem mutations (e.g., New-Item for $destinationParent
and any other writes) to occur only after ShouldProcess returns true, and make
Copy-Tree return a boolean or status indicating whether the copy actually
executed (performed vs skipped). Update the repo-file branch where you write to
restore-report.json so it records "restored" only when Copy-Tree reported
performed, and "skipped" otherwise; similarly update the rules branch to use
Copy-Tree's performed flag (not just non-error return) when setting status.
Ensure method names/variables to change include Copy-Tree, $destinationParent,
$PSCmdlet.ShouldProcess, and the logic that writes restore-report.json so the
status reflects actual execution.
---
Nitpick comments:
In `@autounattend.xml`:
- Around line 156-165: The current SynchronousCommand that polls network
connectivity uses "exit 1" on timeout which will mark the command as failed and
prevent later FirstLogonCommands (including the powershell call to
bootstrap.ps1) from running; decide desired behavior and update the CommandLine
in the SynchronousCommand (the powershell invocation that implements the timeout
loop) to either return success on timeout (change the terminal exit to exit 0 or
remove the non-zero exit) so bootstrap.ps1 still runs and can handle network
unavailability, or keep exit 1 if you intentionally want a fail-fast
behavior—ensure any change references the same SynchronousCommand/CommandLine
that runs powershell and the bootstrap.ps1 FirstLogonCommand so the chosen flow
is consistent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a25a9522-5aa8-43b4-b315-6447b3c75fda
📒 Files selected for processing (18)
.gitignoreFAQ.mdREADME.mdTODO.mdapply-registry.ps1autounattend.xmlbootstrap.ps1build-iso.ps1config/backup.template.jsonconfig/registry.jsondocs/ISO-GENERATION.mddocs/TROUBLESHOOTING.mdpreflight-backup.ps1restore-backup.ps1tests/Autounattend.Tests.ps1tests/BackupRestore.Tests.ps1tests/Bootstrap.Tests.ps1tests/BuildIso.Tests.ps1
| if (-not $entry.path -or -not $entry.name -or -not $entry.type) { | ||
| throw "Registry entry missing required fields (path, name, type)" | ||
| } | ||
|
|
||
| $registryPath = Normalize-RegistryPath -Path $entry.path | ||
| $valueType = Convert-RegistryType -Type $entry.type | ||
| $desiredValue = $entry.value | ||
|
|
||
| if ($valueType -eq "DWord") { | ||
| $desiredValue = [int]$desiredValue | ||
| } |
There was a problem hiding this comment.
Validate value explicitly before applying an entry.
Right now only path, name, and type are required. A malformed DWORD entry with no value falls through to [int]$desiredValue and can silently apply 0.
🛡️ Guard against malformed entries
- if (-not $entry.path -or -not $entry.name -or -not $entry.type) {
- throw "Registry entry missing required fields (path, name, type)"
+ $hasValue = $entry.PSObject.Properties.Match('value').Count -gt 0
+ if (-not $entry.path -or -not $entry.name -or -not $entry.type -or -not $hasValue) {
+ throw "Registry entry missing required fields (path, name, type, value)"
}Based on learnings Configuration should be declarative - define system state in JSON and XML configuration files (apps.json, registry.json, features.json, settings.json, Sophia.ps1) rather than imperative scripts.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (-not $entry.path -or -not $entry.name -or -not $entry.type) { | |
| throw "Registry entry missing required fields (path, name, type)" | |
| } | |
| $registryPath = Normalize-RegistryPath -Path $entry.path | |
| $valueType = Convert-RegistryType -Type $entry.type | |
| $desiredValue = $entry.value | |
| if ($valueType -eq "DWord") { | |
| $desiredValue = [int]$desiredValue | |
| } | |
| $hasValue = $entry.PSObject.Properties.Match('value').Count -gt 0 | |
| if (-not $entry.path -or -not $entry.name -or -not $entry.type -or -not $hasValue) { | |
| throw "Registry entry missing required fields (path, name, type, value)" | |
| } | |
| $registryPath = Normalize-RegistryPath -Path $entry.path | |
| $valueType = Convert-RegistryType -Type $entry.type | |
| $desiredValue = $entry.value | |
| if ($valueType -eq "DWord") { | |
| $desiredValue = [int]$desiredValue | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apply-registry.ps1` around lines 55 - 65, The current block validates only
path/name/type and then converts $entry.value to [int] for DWord, which lets
missing or malformed values become 0; update the logic around $desiredValue
(used after Normalize-RegistryPath and Convert-RegistryType) to explicitly
validate that $entry.value exists and is of the correct form before applying:
for DWord types ensure $entry.value is present and numeric (reject or throw with
a clear message if not), for other types ensure non-null/expected type, and
avoid silently casting invalid/missing values to defaults; use
Convert-RegistryType, Normalize-RegistryPath, the $valueType check for "DWord"
and the $desiredValue variable to locate and enforce these validations.
| function Find-BackupManifest { | ||
| $drives = Get-PSDrive -PSProvider FileSystem -ErrorAction SilentlyContinue | Where-Object { | ||
| $_.Root -ne "$($env:SystemDrive)\" | ||
| } | ||
|
|
||
| foreach ($drive in $drives) { | ||
| $candidateRoot = Join-Path $drive.Root "declarative-windows-backup" | ||
| if (-not (Test-Path $candidateRoot)) { | ||
| continue | ||
| } | ||
|
|
||
| $match = Get-ChildItem -Path $candidateRoot -Filter "backup-manifest.json" -Recurse -File -ErrorAction SilentlyContinue | | ||
| Sort-Object LastWriteTimeUtc -Descending | | ||
| Select-Object -First 1 | ||
|
|
||
| if ($match) { | ||
| return $match.FullName | ||
| } | ||
| } | ||
|
|
||
| return $null |
There was a problem hiding this comment.
Choose the newest manifest across all backup drives.
This helper returns as soon as the first drive has any backup-manifest.json, so an older backup on D: wins over a newer one on E:. That can bootstrap from stale repo metadata. The copy in restore-backup.ps1:29-50 has the same selection bug, so both helpers should sort the combined candidate set by LastWriteTimeUtc.
💡 Minimal fix
function Find-BackupManifest {
$drives = Get-PSDrive -PSProvider FileSystem -ErrorAction SilentlyContinue | Where-Object {
$_.Root -ne "$($env:SystemDrive)\"
}
- foreach ($drive in $drives) {
+ $match = foreach ($drive in $drives) {
$candidateRoot = Join-Path $drive.Root "declarative-windows-backup"
if (-not (Test-Path $candidateRoot)) {
continue
}
- $match = Get-ChildItem -Path $candidateRoot -Filter "backup-manifest.json" -Recurse -File -ErrorAction SilentlyContinue |
- Sort-Object LastWriteTimeUtc -Descending |
- Select-Object -First 1
-
- if ($match) {
- return $match.FullName
- }
- }
-
- return $null
+ Get-ChildItem -Path $candidateRoot -Filter "backup-manifest.json" -Recurse -File -ErrorAction SilentlyContinue
+ } | Sort-Object LastWriteTimeUtc -Descending | Select-Object -First 1
+
+ if ($match) {
+ return $match.FullName
+ }
+
+ return $null
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function Find-BackupManifest { | |
| $drives = Get-PSDrive -PSProvider FileSystem -ErrorAction SilentlyContinue | Where-Object { | |
| $_.Root -ne "$($env:SystemDrive)\" | |
| } | |
| foreach ($drive in $drives) { | |
| $candidateRoot = Join-Path $drive.Root "declarative-windows-backup" | |
| if (-not (Test-Path $candidateRoot)) { | |
| continue | |
| } | |
| $match = Get-ChildItem -Path $candidateRoot -Filter "backup-manifest.json" -Recurse -File -ErrorAction SilentlyContinue | | |
| Sort-Object LastWriteTimeUtc -Descending | | |
| Select-Object -First 1 | |
| if ($match) { | |
| return $match.FullName | |
| } | |
| } | |
| return $null | |
| function Find-BackupManifest { | |
| $drives = Get-PSDrive -PSProvider FileSystem -ErrorAction SilentlyContinue | Where-Object { | |
| $_.Root -ne "$($env:SystemDrive)\" | |
| } | |
| $match = foreach ($drive in $drives) { | |
| $candidateRoot = Join-Path $drive.Root "declarative-windows-backup" | |
| if (-not (Test-Path $candidateRoot)) { | |
| continue | |
| } | |
| Get-ChildItem -Path $candidateRoot -Filter "backup-manifest.json" -Recurse -File -ErrorAction SilentlyContinue | |
| } | Sort-Object LastWriteTimeUtc -Descending | Select-Object -First 1 | |
| if ($match) { | |
| return $match.FullName | |
| } | |
| return $null | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@bootstrap.ps1` around lines 431 - 451, The Find-BackupManifest function
currently returns the first match found per drive, which can pick an older
manifest if a later drive has a newer file; change it to collect all matching
"backup-manifest.json" files from every candidate drive (accumulate $match
results instead of returning immediately), then sort the combined set by
LastWriteTimeUtc descending and return the FullName of the first (newest) item;
apply the same fix to the equivalent helper in restore-backup.ps1 so both
helpers choose the newest manifest across all drives.
| foreach ($packageId in $stillMissing) { | ||
| Add-FailedItem -Category "WinGet Packages" -Item $packageId -Reason "Not installed after import" | ||
| Write-Log "WARNING: $packageId still not installed after WinGet import" -Level WARNING | ||
| } | ||
|
|
||
| if ($LASTEXITCODE -eq 0) { | ||
| if (-not $stillMissing) { | ||
| Write-Log "WinGet import completed successfully" -Level SUCCESS | ||
| Set-Content -Path $WingetMarker -Value $appsHash -Force | ||
| Add-SummaryItem -Step "WinGet" -Status "✓" -Message "Installed $($missingPackages.Count) packages ($WingetMarker)" | ||
| Add-SummaryItem -Step "WinGet" -Status "OK" -Message "Installed $($missingPackages.Count) packages" | ||
| Set-StepState -StepId $stepId -Status "done" -Message "Installed $($missingPackages.Count) packages" | ||
| } | ||
| else { | ||
| Write-Log "WinGet import completed with warnings (exit code: $LASTEXITCODE)" -Level WARNING | ||
| Add-SummaryItem -Step "WinGet" -Status "⚠" -Message "WinGet import completed with warnings" | ||
| $failCount = @($stillMissing).Count | ||
| Write-Log "WinGet import finished; $failCount package(s) failed" -Level WARNING | ||
| Set-Content -Path $WingetMarker -Value $appsHash -Force | ||
| Add-SummaryItem -Step "WinGet" -Status "WARN" -Message "$failCount package(s) failed - see Failed Installs.txt" | ||
| Set-StepState -StepId $stepId -Status "done" -Message "$failCount package(s) failed" | ||
| } |
There was a problem hiding this comment.
Keep WinGet retryable after partial failures.
When $stillMissing is non-empty, this branch still writes $WingetMarker and sets the step to "done". A normal rerun then skips WinGet even though some packages are still absent. Leave the step failed/pending and only persist the marker after a clean import.
💡 Minimal fix
- Set-Content -Path $WingetMarker -Value $appsHash -Force
Add-SummaryItem -Step "WinGet" -Status "WARN" -Message "$failCount package(s) failed - see Failed Installs.txt"
- Set-StepState -StepId $stepId -Status "done" -Message "$failCount package(s) failed"
+ Set-StepState -StepId $stepId -Status "failed" -Message "$failCount package(s) failed"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@bootstrap.ps1` around lines 646 - 663, The code writes $WingetMarker and
marks the step "done" even when $stillMissing is non-empty, causing reruns to
skip WinGet despite partial failures; change the logic so Set-Content -Path
$WingetMarker and the success Set-StepState/Add-SummaryItem calls only occur in
the if (-not $stillMissing) branch, and in the else branch do NOT call
Set-Content or mark the step as "done" — instead set the step status to a
failing/pending state (use Set-StepState -StepId $stepId with a non-"done"
status and update Add-SummaryItem to reflect WARN/FAIL) so the run remains
retryable and the marker is only persisted after a clean import.
| $stepId = "summary" | ||
| if (-not (Should-RunStep -StepId $stepId)) { | ||
| Write-Log "Step 7: Skipping summary report (already completed)" -Level INFO | ||
| } | ||
| elseif ($DryRun) { | ||
| Write-Log "Step 7: Dry run - skipping summary report" -Level WARNING | ||
| Set-StepState -StepId $stepId -Status "pending" -Message "Dry run: summary skipped" | ||
| } | ||
| else { | ||
| try { | ||
| $failedReportPath = Write-FailedInstallsReport -DesktopPath $desktopPath | ||
| if ($FailedItems.Count -gt 0) { | ||
| Write-Log "Failed installs report written to $failedReportPath ($($FailedItems.Count) item(s))" -Level WARNING | ||
| } | ||
| else { | ||
| Write-Log "No failed installs - report written to $failedReportPath" -Level SUCCESS | ||
| } | ||
|
|
||
| $summaryPath = Write-SummaryReport -DesktopPath $desktopPath | ||
| Write-Log "Summary report written to $summaryPath" -Level SUCCESS | ||
| Set-StepState -StepId $stepId -Status "done" -Message "Summary report written" |
There was a problem hiding this comment.
Always rewrite the desktop reports on non-dry reruns.
Once summary is marked "done", later reruns skip this block, so Setup Summary.txt and Failed Installs.txt stay stale even if earlier failures were fixed. The reporting step should run on every non--DryRun execution and just overwrite the previous files.
Based on learnings, "bootstrap.ps1 must generate a desktop summary report at C:\Users[User]\Desktop\Setup Summary.txt with quick visual status of installation".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@bootstrap.ps1` around lines 900 - 920, The summary/reporting block is
currently skipped when Should-RunStep returns false, leaving desktop reports
stale on non-dry reruns; remove or bypass the Should-RunStep check so that when
-not $DryRun the code always calls Write-FailedInstallsReport and
Write-SummaryReport (using $desktopPath and inspecting $FailedItems) to
overwrite the existing "Setup Summary.txt" and "Failed Installs.txt", retain the
DryRun branch that sets Set-StepState -StepId $stepId -Status "pending", and
ensure the final Set-StepState -StepId $stepId -Status "done" -Message "Summary
report written" is still called after the reports are written.
| if ($DownloadUrl) { | ||
| Write-Step "Downloading oscdimg.exe" | ||
|
|
||
| $cacheDir = Join-Path $env:TEMP "declarative-windows-tools" | ||
| if (-not (Test-Path $cacheDir)) { | ||
| New-Item -Path $cacheDir -ItemType Directory -Force | Out-Null | ||
| } | ||
|
|
||
| $extension = [System.IO.Path]::GetExtension($DownloadUrl) | ||
| $downloadPath = $null | ||
|
|
||
| if ($extension -eq ".zip") { | ||
| $downloadPath = Join-Path $cacheDir "oscdimg.zip" | ||
| Invoke-WebRequest -Uri $DownloadUrl -OutFile $downloadPath | ||
| Expand-Archive -Path $downloadPath -DestinationPath $cacheDir -Force | ||
|
|
||
| $oscdimgFile = Get-ChildItem -Path $cacheDir -Filter "oscdimg.exe" -Recurse | Select-Object -First 1 | ||
| if ($oscdimgFile) { | ||
| Write-Success "Downloaded oscdimg.exe to: $($oscdimgFile.FullName)" | ||
| return $oscdimgFile.FullName | ||
| } | ||
| } | ||
| else { | ||
| $downloadPath = Join-Path $cacheDir "oscdimg.exe" | ||
| Invoke-WebRequest -Uri $DownloadUrl -OutFile $downloadPath | ||
| if (Test-Path $downloadPath) { | ||
| Write-Success "Downloaded oscdimg.exe to: $downloadPath" | ||
| return $downloadPath | ||
| } | ||
| } | ||
|
|
||
| Write-ErrorMessage "oscdimg.exe download failed" | ||
| throw "Failed to download oscdimg.exe from $DownloadUrl" |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Is oscdimg.exe officially distributed as part of the Windows ADK, and is the binary Authenticode-signed by Microsoft when obtained from official sources?
💡 Result:
Yes, oscdimg.exe is officially distributed as part of the Windows Assessment and Deployment Kit (ADK), specifically in the Deployment Tools component. It is installed at paths like C:\Program Files (x86)\Windows Kits\10\Assessment and Deployment Kit\Deployment Tools\amd64\Oscdimg\oscdimg.exe after selecting Deployment Tools during ADK installation. Microsoft provides official documentation on its usage for creating ISO images. Yes, the oscdimg.exe binary obtained from official Microsoft ADK sources is Authenticode-signed by Microsoft. Multiple sources confirm it has a valid digital signature from Microsoft Corporation with a trusted certificate chain to a root CA. Older versions may have had signature issues that were resolved in updates, but current official versions are properly signed. Users are advised to verify the signature using tools like sigcheck or file properties to ensure authenticity.
Citations:
- 1: https://learn.microsoft.com/en-us/windows-hardware/get-started/adk-install
- 2: https://learn.microsoft.com/en-us/answers/questions/4330520/downloading-the-oscdimg-utility-for-windows-11-upd
- 3: Download oscdimg directly from Microsoft ChrisTitusTech/winutil#3313
- 4: https://learn.microsoft.com/en-us/windows-hardware/manufacture/desktop/oscdimg-command-line-options?view=windows-11
- 5: Beware: oscdimg.exe in this repo is not the original from Microsoft ntdevlabs/tiny11builder#47
- 6: https://windowsbulletin.com/files/exe/microsoft-corporation/windows-deployment-tools/oscdimg-exe
Implement signature or hash verification for the downloaded oscdimg.exe.
The script accepts -OscdimgDownloadUrl as an arbitrary URL and executes the downloaded binary as administrator without verifying its authenticity. While oscdimg.exe from official Microsoft ADK sources is Authenticode-signed by Microsoft, the code does not validate this signature or enforce HTTPS. Add Authenticode signature verification (using Get-AuthenticodeSignature) or SHA-256 hash validation before executing the downloaded tool.
🧰 Tools
🪛 PSScriptAnalyzer (1.24.0)
[warning] Missing BOM encoding for non-ASCII encoded file 'build-iso.ps1'
(PSUseBOMForUnicodeEncodedFile)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@build-iso.ps1` around lines 136 - 168, The download block that fetches
$DownloadUrl (producing $downloadPath / $oscdimgFile) must validate authenticity
before returning/executing the binary: require HTTPS on $DownloadUrl, then after
downloading compute a SHA-256 hash (or call Get-AuthenticodeSignature) for the
resolved oscdimg.exe and compare it to an allowlist or require a Valid signature
status; if verification fails, log a clear error via Write-ErrorMessage and
throw instead of returning the path. Update the branches that return
$oscdimgFile.FullName or $downloadPath to perform this verification step (refer
to variables $DownloadUrl, $downloadPath, $oscdimgFile and the
download/Expand-Archive logic) and only return when signature/hash check passes.
Ensure failures abort with an explanatory message.
| foreach ($pattern in $ExcludePatterns) { | ||
| $leaf = Split-Path -Path $pattern -Leaf | ||
| if ($leaf -and $leaf -notmatch '[\*\?]') { | ||
| $robocopyArgs += @("/XF", $leaf) | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In robocopy, what is the difference between /XD and /XF, and can a glob like **\node_modules\** be passed directly to exclude a directory tree?
💡 Result:
/XD excludes entire directories (and their subtrees) matching the given names or paths anywhere in the source tree. /XF excludes individual files matching the given names or paths. A glob like \node_modules\ cannot be passed directly; robocopy does not support recursive wildcard patterns like . Use /XD node_modules to exclude any directory named "node_modules" at any depth (and its entire subtree).
Citations:
- 1: https://learn.microsoft.com/en-us/windows-server/administration/windows-commands/robocopy
- 2: https://docs.microsoft.com/en-us/windows-server/administration/windows-commands/robocopy
- 3: https://learn.microsoft.com/en-us/windows-server/administration/windows-commands/robocopy?redirectedfrom=MSDN
- 4: https://ss64.com/nt/robocopy.html
- 5: https://serverfault.com/questions/304896/wildcard-directory-exclusions-with-robocopy-weird-case
- 6: https://serverfault.com/questions/778763/how-to-exclude-files-in-a-folder-with-pattern-with-robocopy
- 7: https://gist.github.com/anuraghazra/95a4150789f6166f75ff566b494c72d9?permalink_comment_id=4571562
- 8: https://stackoverflow.com/questions/35060669/xcopy-with-excluding-folders-sub-directories
- 9: https://raviswdev.blogspot.com/2024/03/using-robocopy-to-copy-folder-without.html
- 10: https://superuser.com/questions/940804/copy-large-number-of-files-but-exclude-certain-sub-folders-on-windows
🏁 Script executed:
# First, let's locate and examine the preflight-backup.ps1 file
fd "preflight-backup.ps1" --type fRepository: Timpan4/declarative-windows
Length of output: 91
🏁 Script executed:
# Read the file to see the context around lines 207-212
cat -n preflight-backup.ps1 | head -250 | tail -60Repository: Timpan4/declarative-windows
Length of output: 1864
🏁 Script executed:
# Let's find where $ExcludePatterns is defined and what patterns are passed
grep -n "ExcludePatterns" preflight-backup.ps1 | head -20Repository: Timpan4/declarative-windows
Length of output: 292
🏁 Script executed:
# Let's see the function signature and the function call at line 306
cat -n preflight-backup.ps1 | sed -n '170,180p'Repository: Timpan4/declarative-windows
Length of output: 436
🏁 Script executed:
# Let's see where this function is called and what patterns are passed
cat -n preflight-backup.ps1 | sed -n '300,315p'Repository: Timpan4/declarative-windows
Length of output: 761
🏁 Script executed:
# Let's find where $excludePatterns is defined
grep -n "excludePatterns\|ExcludePatterns" preflight-backup.ps1Repository: Timpan4/declarative-windows
Length of output: 342
🏁 Script executed:
# Let's see the config and where excludePatterns comes from
cat -n preflight-backup.ps1 | sed -n '275,290p'Repository: Timpan4/declarative-windows
Length of output: 937
🏁 Script executed:
# Let's find where config is loaded and look for default exclude patterns
grep -n "config" preflight-backup.ps1 | head -20Repository: Timpan4/declarative-windows
Length of output: 878
🏁 Script executed:
# Let's examine the backup.template.json to see what patterns are configured
cat -n config/backup.template.jsonRepository: Timpan4/declarative-windows
Length of output: 2235
The configured excludes never reach robocopy and would fail even if they did.
The template defines directory globs like **\node_modules\**, but the loop at lines 207–212 extracts only the leaf segment (e.g., ** from **\node_modules\**). Since all patterns end with **, they all contain wildcards and are filtered out by the $leaf -notmatch '[\*\?]' check. Consequently, no exclusions are passed to robocopy—.git, .venv, caches, and node_modules are copied regardless of configuration.
Additionally, even if patterns did reach robocopy, the code uses /XF (file exclude) instead of /XD (directory exclude). Robocopy does not support recursive glob syntax like **\node_modules\**; the correct approach is /XD node_modules to exclude the directory at any depth.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@preflight-backup.ps1` around lines 207 - 212, The foreach over
$ExcludePatterns wrongly strips patterns to their leaf and rejects anything
containing '*'/'?', so no excludes reach robocopy and /XF (file-exclude) is used
for directory patterns; update the loop that references $ExcludePatterns,
Split-Path and $leaf so it does not discard globbed directory patterns but
instead recognizes directory globs (e.g. patterns containing '\**' or
ending/starting with '**\...') and maps them to robocopy directory excludes by
appending "/XD <dirName>" to $robocopyArgs (use the base directory name like
"node_modules" or ".git"), while preserving file patterns to "/XF" where
appropriate; remove the $leaf -notmatch '[\*\?]' gate that filters out globs and
ensure robocopyArgs uses "/XD" for directories and "/XF" only for explicit file
names.
| backup = [ordered]@{ | ||
| destinationRoot = (Resolve-Path $DestinationRoot).Path |
There was a problem hiding this comment.
-WhatIf will fail on a fresh destination root.
When the earlier ShouldProcess calls skip directory creation, Resolve-Path $DestinationRoot still runs here and throws for a new path. That breaks the dry-run path this script advertises.
🧪 Keep `-WhatIf` path resolution non-throwing
- destinationRoot = (Resolve-Path $DestinationRoot).Path
+ destinationRoot = if (Test-Path $DestinationRoot) {
+ (Resolve-Path $DestinationRoot).Path
+ }
+ else {
+ $ExecutionContext.SessionState.Path.GetUnresolvedProviderPathFromPSPath($DestinationRoot)
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| backup = [ordered]@{ | |
| destinationRoot = (Resolve-Path $DestinationRoot).Path | |
| backup = [ordered]@{ | |
| destinationRoot = if (Test-Path $DestinationRoot) { | |
| (Resolve-Path $DestinationRoot).Path | |
| } | |
| else { | |
| $ExecutionContext.SessionState.Path.GetUnresolvedProviderPathFromPSPath($DestinationRoot) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@preflight-backup.ps1` around lines 366 - 367, The assignment to the
backup.destinationRoot uses Resolve-Path which throws for non-existent paths and
breaks dry-run; change it to safely resolve only when the path exists or fall
back to the literal input (e.g., replace (Resolve-Path $DestinationRoot).Path
with a non-throwing expression such as: if (Test-Path $DestinationRoot) {
(Resolve-Path $DestinationRoot).Path } else { $DestinationRoot } or use
Resolve-Path -ErrorAction SilentlyContinue and default to $DestinationRoot so
-WhatIf runs without error; update the code around the backup = [ordered]@{
destinationRoot assignment accordingly.
| function Write-Info { | ||
| param([string]$Message) | ||
| Write-Host "[INFO] $Message" | ||
| } | ||
|
|
||
| function Write-Success { | ||
| param([string]$Message) | ||
| Write-Host "[ OK ] $Message" -ForegroundColor Green | ||
| } |
There was a problem hiding this comment.
Add the required install-log output for restore runs.
These helpers only write to the host, so a restore launched from the desktop shortcut leaves no execution trail in C:\Setup\install.log. Please route restore logging through a file-backed logger that records ✓ completed, ⚠ skipped, and ✗ failed events.
As per coding guidelines, "PowerShell scripts must create comprehensive logging with detailed execution logs to C:\Setup\install.log showing completion status (✓ completed, ⚠ skipped, ✗ failed)."
🧰 Tools
🪛 PSScriptAnalyzer (1.24.0)
[warning] 21-21: File 'restore-backup.ps1' uses Write-Host. Avoid using Write-Host because it might not work in all hosts, does not work when there is no host, and (prior to PS 5.0) cannot be suppressed, captured, or redirected. Instead, use Write-Output, Write-Verbose, or Write-Information.
(PSAvoidUsingWriteHost)
[warning] 26-26: File 'restore-backup.ps1' uses Write-Host. Avoid using Write-Host because it might not work in all hosts, does not work when there is no host, and (prior to PS 5.0) cannot be suppressed, captured, or redirected. Instead, use Write-Output, Write-Verbose, or Write-Information.
(PSAvoidUsingWriteHost)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@restore-backup.ps1` around lines 19 - 27, Update the helper logging functions
so they write both to the console and to a file-backed install log
(C:\Setup\install.log); modify or replace Write-Info and Write-Success (and add
corresponding Write-Warning/Write-Failure if missing) to append entries to the
file using the required symbols ("✓ completed", "⚠ skipped", "✗ failed") and
include timestamps and a clear message, while preserving the existing console
output (Write-Host). Ensure file creation/append uses safe file locking/encoding
and that all restore steps call these file-backed helpers so every restore run
produces comprehensive entries in C:\Setup\install.log.
| $destinationParent = Split-Path -Path $Destination -Parent | ||
| if ($destinationParent -and -not (Test-Path $destinationParent)) { | ||
| New-Item -Path $destinationParent -ItemType Directory -Force | Out-Null | ||
| } | ||
|
|
||
| if (-not $PSCmdlet.ShouldProcess($Destination, "Restore files from $Source")) { | ||
| return $true | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "restore-backup.ps1" -type fRepository: Timpan4/declarative-windows
Length of output: 91
🏁 Script executed:
cat -n restore-backup.ps1Repository: Timpan4/declarative-windows
Length of output: 6520
Move filesystem mutations behind ShouldProcess and report accurate status outcomes.
Copy-Tree creates $destinationParent (lines 79–80) before checking ShouldProcess (line 83), then returns success when the copy is skipped (line 84). The repo-file branch also unconditionally records "restored" (line 152) regardless of whether the ShouldProcess check passed. This means -WhatIf and declined confirmations still create directories and appear successful in restore-report.json.
Move all filesystem mutations behind the ShouldProcess check in Copy-Tree. For the repo-file branch (lines 148–152), only record "restored" if the copy actually executed; otherwise record "skipped". For the rules branch (line 165–170), the status must reflect whether Copy-Tree actually performed the copy, not just whether it returned without error.
Also applies to: 148–152, 165–170
🧰 Tools
🪛 PSScriptAnalyzer (1.24.0)
[warning] 83-83: 'Copy-Tree' calls ShouldProcess/ShouldContinue but does not have the ShouldProcess attribute.
(PSShouldProcess)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@restore-backup.ps1` around lines 78 - 85, Copy-Tree currently creates
$destinationParent before calling $PSCmdlet.ShouldProcess and reports success
even when the operation is skipped; move all filesystem mutations (e.g.,
New-Item for $destinationParent and any other writes) to occur only after
ShouldProcess returns true, and make Copy-Tree return a boolean or status
indicating whether the copy actually executed (performed vs skipped). Update the
repo-file branch where you write to restore-report.json so it records "restored"
only when Copy-Tree reported performed, and "skipped" otherwise; similarly
update the rules branch to use Copy-Tree's performed flag (not just non-error
return) when setting status. Ensure method names/variables to change include
Copy-Tree, $destinationParent, $PSCmdlet.ShouldProcess, and the logic that
writes restore-report.json so the status reflects actual execution.
| $repoTargetRoot = [Environment]::ExpandEnvironmentVariables($manifest.repo.restorePath) | ||
| $destination = Join-Path $repoTargetRoot $repoFile.relativePath | ||
| $destinationParent = Split-Path -Path $destination -Parent |
There was a problem hiding this comment.
Honor -DestinationProfileRoot when restoring repo files.
$manifest.repo.restorePath is expanded against the current %USERPROFILE% here, so repo files still land in the active account even when the caller targets a different profile root. Resolve the repo root through Resolve-RestoreTargetPath before joining relativePath.
💡 Minimal fix
- $repoTargetRoot = [Environment]::ExpandEnvironmentVariables($manifest.repo.restorePath)
+ $repoTargetRoot = Resolve-RestoreTargetPath -Path $manifest.repo.restorePath -ProfileRoot $DestinationProfileRoot
$destination = Join-Path $repoTargetRoot $repoFile.relativePath🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@restore-backup.ps1` around lines 133 - 135, The restore currently expands
$manifest.repo.restorePath directly into $repoTargetRoot (using the active
%USERPROFILE%) which ignores the -DestinationProfileRoot option; instead call
Resolve-RestoreTargetPath (the helper that respects -DestinationProfileRoot) to
compute the repo root and assign that to $repoTargetRoot, then use Join-Path
with $repoTargetRoot and $repoFile.relativePath to build $destination (and keep
$destinationParent = Split-Path -Path $destination -Parent unchanged); update
references to $manifest.repo.restorePath in this block to use the resolved path
returned by Resolve-RestoreTargetPath.
1 participant
These changes centralize pre checks, ensure consistent rulegeneration templates, and prevent accidental backups thesystem drive. This provides a safer, more declarative foundation forsubsequent backup and steps
Summary by CodeRabbit
Release Notes
New Features
Documentation
Improvements