✨ feat: make AutoUnattend-minimal and defer setup OOBE#14
Conversation
📝 WalkthroughWalkthroughThe PR converts the installer to an oobeSystem-only flow, removes the windowsPE pass and its bypasses, moves OEM payload into Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as "Developer / CI"
participant Builder as "build-iso.ps1"
participant StagedISO as "Staged ISO (sources\\$OEM$)"
participant Installer as "Windows Setup (autounattend)"
participant User as "First User Logon"
participant Bootstrap as "C:\\Setup\\bootstrap.ps1"
rect rgba(200,200,255,0.5)
Dev->>Builder: provide staged payload (sources\\$OEM$)
Builder->>Builder: Get-UnattendSetupFileReferences -> Validate-StagedIsoLayout
Builder->>StagedISO: produce ISO (oscdimg)
end
rect rgba(200,255,200,0.5)
StagedISO->>Installer: boot / run Setup (oobeSystem-only)
Installer->>User: present OOBE / sign-in screens
User->>Installer: complete first logon
Installer->>Bootstrap: FirstLogonCommands -> powershell -File C:\\Setup\\bootstrap.ps1
Bootstrap->>Bootstrap: perform installs, debloat, customize
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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
Here are some automated review suggestions for this pull request.
Reviewed commit: c8e5fad956
ℹ️ 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".
| <settings pass="oobeSystem"> | ||
| <component name="Microsoft-Windows-Shell-Setup" processorArchitecture="amd64" publicKeyToken="31bf3856ad364e35" language="neutral" versionScope="nonSxS"> |
There was a problem hiding this comment.
Removing the entire windowsPE pass drops the LabConfig registry writes that bypass TPM/Secure Boot/RAM checks, so on unsupported hardware (especially many default VirtualBox/VMware test VMs) Windows 11 setup fails before OOBE with “This PC can’t run Windows 11.” In that scenario FirstLogonCommands never execute and bootstrap.ps1 cannot run, which regresses the repo’s VM-first installation workflow unless those bypasses are preserved or TPM-capable VMs are made an explicit requirement.
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: 2
🧹 Nitpick comments (1)
AUTOUNATTEND.md (1)
31-33: Mention the desktop shortcut method first for re-running bootstrap.Line 33 should guide users to "Run Windows Setup.lnk" on the desktop as the primary option, with manual script execution (
C:\Setup\bootstrap.ps1) as a fallback for advanced users. This aligns with the desktop shortcuts thatbootstrap.ps1actually creates.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@AUTOUNATTEND.md` around lines 31 - 33, Update the AUTOUNATTEND.md text so the primary guidance for re-running bootstrap points users to the desktop shortcut "Windows Setup.lnk" (created by bootstrap.ps1) and list running C:\Setup\bootstrap.ps1 manually only as an advanced fallback; specifically change the sentence that currently says “Just run C:\Setup\bootstrap.ps1 manually” to first instruct users to click "Run Windows Setup.lnk" on the desktop and then note the manual bootstrap.ps1 invocation as an alternative for advanced users.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@AUTOUNATTEND.md`:
- Around line 7-10: Update the opening sentence in AUTOUNATTEND.md that states
"The file runs in three phases during Windows installation" to correctly reflect
the current content which documents only the single unattended phase
"oobeSystem"; locate the phrase near the start of the document and change it to
indicate a single phase (e.g., "The file runs in a single phase during Windows
installation: oobeSystem") and ensure any subsequent references to multiple
phases are adjusted or removed to avoid confusion about "oobeSystem".
- Line 3: The intro of AUTOUNATTEND.md lacks an explicit platform target; update
the top of AUTOUNATTEND.md to state the repository policy that the document and
associated scripts target Windows 11 (24H2+) only (e.g., add a one-line header
or note: "Target platform: Windows 11 (24H2+)") so the file unambiguously
enforces the Windows 11-only requirement referenced by the guidelines; ensure
this one-line callout appears at the very top of the file and that no other
lines in AUTOUNATTEND.md imply Windows 10 compatibility.
---
Nitpick comments:
In `@AUTOUNATTEND.md`:
- Around line 31-33: Update the AUTOUNATTEND.md text so the primary guidance for
re-running bootstrap points users to the desktop shortcut "Windows Setup.lnk"
(created by bootstrap.ps1) and list running C:\Setup\bootstrap.ps1 manually only
as an advanced fallback; specifically change the sentence that currently says
“Just run C:\Setup\bootstrap.ps1 manually” to first instruct users to click "Run
Windows Setup.lnk" on the desktop and then note the manual bootstrap.ps1
invocation as an alternative for advanced users.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 497ff9dc-06c2-455f-a69b-ec8d94ca3484
📒 Files selected for processing (2)
AUTOUNATTEND.mdautounattend.xml
| # AutoUnattend.xml Explained | ||
|
|
||
| This file automates only the minimal parts of Windows 11 setup needed for this repo. It intentionally avoids aggressive install-time customization so Windows Setup stays as reliable as possible. | ||
| This file is now intentionally ultra-minimal. It avoids changing Windows Setup itself and only kicks off post-install automation after first login. |
There was a problem hiding this comment.
Call out Windows 11 (24H2+) explicitly in the intro.
Please make the platform target explicit at the top so the doc enforces the repo policy unambiguously.
✏️ Suggested doc fix
-This file is now intentionally ultra-minimal. It avoids changing Windows Setup itself and only kicks off post-install automation after first login.
+This file is now intentionally ultra-minimal and targets **Windows 11 (24H2+) only**. It avoids changing Windows Setup itself and only kicks off post-install automation after first login.As per coding guidelines: "**/*.{md,ps1}: Ensure all documentation, scripts, and configurations assume Windows 11 as the target platform and do not include references to Windows 10 compatibility".
📝 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.
| This file is now intentionally ultra-minimal. It avoids changing Windows Setup itself and only kicks off post-install automation after first login. | |
| This file is now intentionally ultra-minimal and targets **Windows 11 (24H2+) only**. It avoids changing Windows Setup itself and only kicks off post-install automation after first login. |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@AUTOUNATTEND.md` at line 3, The intro of AUTOUNATTEND.md lacks an explicit
platform target; update the top of AUTOUNATTEND.md to state the repository
policy that the document and associated scripts target Windows 11 (24H2+) only
(e.g., add a one-line header or note: "Target platform: Windows 11 (24H2+)") so
the file unambiguously enforces the Windows 11-only requirement referenced by
the guidelines; ensure this one-line callout appears at the very top of the file
and that no other lines in AUTOUNATTEND.md imply Windows 10 compatibility.
| The file runs in three phases during Windows installation: | ||
|
|
||
| ### 1. windowsPE (Installation Phase) | ||
|
|
||
| Runs while Windows is being installed to the disk. | ||
|
|
||
| - Bypasses TPM/Secure Boot/RAM checks (helpful for VMs) | ||
| - Leaves disk and partition selection to the user in Windows Setup | ||
| - Configures language/locale to en-US | ||
|
|
||
| ### 2. oobeSystem (First Boot) | ||
| ### 1. oobeSystem (First Boot) | ||
|
|
There was a problem hiding this comment.
Fix stale phase count in setup flow description.
Line 7 says “three phases,” but the updated content now documents a single unattended phase (oobeSystem). This is confusing for readers following the new model.
✏️ Suggested doc fix
-The file runs in three phases during Windows installation:
+The file now runs in a single unattended phase during Windows installation:📝 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.
| The file runs in three phases during Windows installation: | |
| ### 1. windowsPE (Installation Phase) | |
| Runs while Windows is being installed to the disk. | |
| - Bypasses TPM/Secure Boot/RAM checks (helpful for VMs) | |
| - Leaves disk and partition selection to the user in Windows Setup | |
| - Configures language/locale to en-US | |
| ### 2. oobeSystem (First Boot) | |
| ### 1. oobeSystem (First Boot) | |
| The file now runs in a single unattended phase during Windows installation: | |
| ### 1. oobeSystem (First Boot) | |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@AUTOUNATTEND.md` around lines 7 - 10, Update the opening sentence in
AUTOUNATTEND.md that states "The file runs in three phases during Windows
installation" to correctly reflect the current content which documents only the
single unattended phase "oobeSystem"; locate the phrase near the start of the
document and change it to indicate a single phase (e.g., "The file runs in a
single phase during Windows installation: oobeSystem") and ensure any subsequent
references to multiple phases are adjusted or removed to avoid confusion about
"oobeSystem".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4a4b63d1eb
ℹ️ 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".
| [bool]$HasOptionalApps | ||
| ) | ||
|
|
||
| $stagedSetupRoot = Join-Path $WorkRoot 'sources\`$OEM`$\`$1\Setup' |
There was a problem hiding this comment.
In Validate-StagedIsoLayout, $stagedSetupRoot is defined with a single-quoted string ('sources\$OEM$\$1\Setup'). In PowerShell single-quoted strings are literal, so the backticks are kept in the path instead of escaping $, which makes this value differ from the directory actually created earlier (sources$OEM$$1\Setup). This causes Test-Path` checks to fail and the ISO build to abort during staged layout validation in normal runs.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/Autounattend.Tests.ps1 (1)
23-27: Assert the XML shape, not just a few strings.This still passes if another
SynchronousCommandgets added or if the bootstrap command moves out ofoobeSystem. Parsing the XML and asserting exactly oneFirstLogonCommands/SynchronousCommandwould make the “minimal” contract much harder to regress.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/Autounattend.Tests.ps1` around lines 23 - 27, The test currently checks for a few strings but not the XML structure; update the test to parse the Autounattend XML ($fileContent) and assert the exact shape: locate the oobeSystem pass, then the FirstLogonCommands element and verify there is exactly one SynchronousCommand element (and that it contains the expected bootstrap command text), and also assert that forbidden commands like "Set-ExecutionPolicy" and "Wait for Network Connectivity" are not present; use XML parsing APIs to count FirstLogonCommands/SynchronousCommand nodes and validate their content instead of relying on regex matches.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/BuildIso.Tests.ps1`:
- Around line 45-64: The order-check is anchored to the function declaration
because IndexOf('Validate-StagedIsoLayout') returns the first match; change the
lookup to target the actual invocation by using
IndexOf('Validate-StagedIsoLayout -WorkRoot') when computing $validationIndex so
it compares the real call site against the build step (Write-Step "Building
custom ISO with oscdimg") ensuring the validation runs before the ISO build.
---
Nitpick comments:
In `@tests/Autounattend.Tests.ps1`:
- Around line 23-27: The test currently checks for a few strings but not the XML
structure; update the test to parse the Autounattend XML ($fileContent) and
assert the exact shape: locate the oobeSystem pass, then the FirstLogonCommands
element and verify there is exactly one SynchronousCommand element (and that it
contains the expected bootstrap command text), and also assert that forbidden
commands like "Set-ExecutionPolicy" and "Wait for Network Connectivity" are not
present; use XML parsing APIs to count FirstLogonCommands/SynchronousCommand
nodes and validate their content instead of relying on regex matches.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 15966adf-c844-4aad-9c74-e2de1478ef88
📒 Files selected for processing (6)
CLAUDE.mdautounattend.xmlbuild-iso.ps1docs/ISO-GENERATION.mdtests/Autounattend.Tests.ps1tests/BuildIso.Tests.ps1
✅ Files skipped from review due to trivial changes (2)
- CLAUDE.md
- docs/ISO-GENERATION.md
🚧 Files skipped from review as they are similar to previous changes (1)
- autounattend.xml
| It "validates the staged ISO layout before oscdimg" { | ||
| $scriptContent | Should -Match "Validate-StagedIsoLayout" | ||
| $scriptContent | Should -Match "Validating staged ISO layout" | ||
| $scriptContent | Should -Match "Staged ISO layout validation passed" | ||
| } | ||
|
|
||
| It "checks unattend C:\\Setup references against the staged OEM payload" { | ||
| $scriptContent | Should -Match "Get-UnattendSetupFileReferences" | ||
| $scriptContent | Should -Match "C:\\Setup\\" | ||
| $scriptContent | Should -Match "autounattend\.xml references .* staged ISO is missing" | ||
| } | ||
|
|
||
| It "runs staged layout validation before building the ISO" { | ||
| $validationIndex = $scriptContent.IndexOf('Validate-StagedIsoLayout') | ||
| $buildIndex = $scriptContent.IndexOf('Write-Step "Building custom ISO with oscdimg"') | ||
|
|
||
| $validationIndex | Should -BeGreaterThan -1 | ||
| $buildIndex | Should -BeGreaterThan -1 | ||
| $validationIndex | Should -BeLessThan $buildIndex | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
python - <<'PY'
from pathlib import Path
text = Path("build-iso.ps1").read_text(encoding="utf-8")
needle = "Validate-StagedIsoLayout"
positions = []
start = 0
while True:
idx = text.find(needle, start)
if idx == -1:
break
positions.append(idx)
start = idx + 1
print("All 'Validate-StagedIsoLayout' positions:", positions)
print("Call-site index:", text.find("Validate-StagedIsoLayout -WorkRoot"))
print('Build-step index:', text.find('Write-Step "Building custom ISO with oscdimg"'))
PYRepository: Timpan4/declarative-windows
Length of output: 173
Anchor the order check on the actual call site, not the function declaration.
Line 58 uses the first occurrence of Validate-StagedIsoLayout, which is the function declaration at byte position 3415. The actual invocation (Validate-StagedIsoLayout -WorkRoot) is at position 13851. Since .IndexOf() finds the first match, this test can pass even if the real call site comes after the build step. Anchor on "Validate-StagedIsoLayout -WorkRoot" instead to validate the actual call order.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/BuildIso.Tests.ps1` around lines 45 - 64, The order-check is anchored
to the function declaration because IndexOf('Validate-StagedIsoLayout') returns
the first match; change the lookup to target the actual invocation by using
IndexOf('Validate-StagedIsoLayout -WorkRoot') when computing $validationIndex so
it compares the real call site against the build step (Write-Step "Building
custom ISO with oscdimg") ensuring the validation runs before the ISO build.
There was a problem hiding this comment.
🧹 Nitpick comments (3)
build-iso.ps1 (3)
103-103: Optional: Use approved PowerShell verb and singular noun.PSScriptAnalyzer flags two naming convention issues:
Validate-StagedIsoLayout→ preferTest-StagedIsoLayout(approved verb)Get-UnattendSetupFileReferences→ preferGet-UnattendSetupFileReference(singular noun)These are internal functions so the impact is minimal, but following conventions improves consistency with the PowerShell ecosystem.
Also applies to: 125-125
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@build-iso.ps1` at line 103, Rename the two internal functions to follow PowerShell approved verb/noun conventions: change Validate-StagedIsoLayout to Test-StagedIsoLayout and change Get-UnattendSetupFileReferences to Get-UnattendSetupFileReference; update every reference to those symbols (calls, splatting, tests, comment/docs, and any exported/function lists) to the new names and run PSScriptAnalyzer to confirm the warnings are resolved.
117-119: Regex may truncate paths containing spaces.The pattern
[^\s"'';]+stops at whitespace, so a quoted path like"C:\Setup\My Script.ps1"would extractC:\Setup\Myinstead of the full filename. Current staged files don't have spaces, but this could cause silent validation misses if such files are added later.Consider using a more robust pattern that handles quoted paths:
💡 Optional: Handle quoted paths
- foreach ($match in [regex]::Matches($commandNode.InnerText, '(?i)\bC:\\Setup\\[^\s"'';]+')) { + # Match both quoted and unquoted C:\Setup paths + foreach ($match in [regex]::Matches($commandNode.InnerText, '(?i)(?:"(C:\\Setup\\[^"]+)"|''(C:\\Setup\\[^'']+)''|\b(C:\\Setup\\[^\s"'';]+))')) { + $capturedPath = $match.Groups[1].Value, $match.Groups[2].Value, $match.Groups[3].Value | Where-Object { $_ } | Select-Object -First 1 $references.Add($match.Value)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@build-iso.ps1` around lines 117 - 119, The regex used in the foreach over $commandNode.InnerText currently stops at whitespace and will truncate quoted paths; update the pattern in that foreach (the [regex]::Matches call) to accept either a quoted path (allowing spaces inside the quotes) or an unquoted token, and when adding to $references strip any surrounding quotes; i.e., replace the single-token pattern with an alternation that captures quoted "C:\Setup\..." entries as well as unquoted paths so $match.Value yields the full filename for both cases.
66-74: Consider adding UTF-8 BOM for Unicode characters.The file contains Unicode symbols (✓, ✗, box-drawing characters) but lacks a BOM. While modern editors handle BOM-less UTF-8 fine, some Windows tools (including older PowerShell hosts) may misinterpret these characters. Saving with UTF-8 BOM ensures consistent rendering.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@build-iso.ps1` around lines 66 - 74, The script uses Unicode glyphs in Write-Success and Write-ErrorMessage (the ✓ and ✗ characters) but the file lacks a UTF-8 BOM; re-save the file encoded as "UTF-8 with BOM" so Windows PowerShell hosts render these characters reliably (or alternatively replace the literal glyphs in Write-Success/Write-ErrorMessage with explicit Unicode escapes if BOM cannot be used), ensuring the change is applied to build-iso.ps1 so the functions Write-Success and Write-ErrorMessage display correctly across Windows tools.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@build-iso.ps1`:
- Line 103: Rename the two internal functions to follow PowerShell approved
verb/noun conventions: change Validate-StagedIsoLayout to Test-StagedIsoLayout
and change Get-UnattendSetupFileReferences to Get-UnattendSetupFileReference;
update every reference to those symbols (calls, splatting, tests, comment/docs,
and any exported/function lists) to the new names and run PSScriptAnalyzer to
confirm the warnings are resolved.
- Around line 117-119: The regex used in the foreach over $commandNode.InnerText
currently stops at whitespace and will truncate quoted paths; update the pattern
in that foreach (the [regex]::Matches call) to accept either a quoted path
(allowing spaces inside the quotes) or an unquoted token, and when adding to
$references strip any surrounding quotes; i.e., replace the single-token pattern
with an alternation that captures quoted "C:\Setup\..." entries as well as
unquoted paths so $match.Value yields the full filename for both cases.
- Around line 66-74: The script uses Unicode glyphs in Write-Success and
Write-ErrorMessage (the ✓ and ✗ characters) but the file lacks a UTF-8 BOM;
re-save the file encoded as "UTF-8 with BOM" so Windows PowerShell hosts render
these characters reliably (or alternatively replace the literal glyphs in
Write-Success/Write-ErrorMessage with explicit Unicode escapes if BOM cannot be
used), ensuring the change is applied to build-iso.ps1 so the functions
Write-Success and Write-ErrorMessage display correctly across Windows tools.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 98649814-f514-412c-9684-08919ee647cd
📒 Files selected for processing (2)
build-iso.ps1tests/BuildIso.Tests.ps1
✅ Files skipped from review due to trivial changes (1)
- tests/BuildIso.Tests.ps1
1 participant
Summary by CodeRabbit
Documentation
Configuration
Tests