Fix Stop-Transcript error when transcript not active#34
Open
Fix Stop-Transcript error when transcript not active#34
Conversation
Start-Transcript can fail silently in certain RMM environments. Added $TranscriptStarted flag to track if transcription is active and only call Stop-Transcript when it was successfully started. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Contributor
📝 WalkthroughWalkthroughAdds guarded transcript handling across disable scripts and introduces two new enable scripts: one to re-enable Multiplane Overlay (MPO) and one to re-enable Core Isolation (HVCI/VBS/Hypervisor). Both new scripts perform admin checks, stepwise remediation via registry and boot config, verification, and logging; final notes require restart. Changes
Sequence Diagram(s)sequenceDiagram
rect rgba(200,200,255,0.5)
participant Operator
end
rect rgba(200,255,200,0.5)
participant Script
end
rect rgba(255,200,200,0.5)
participant Registry
end
rect rgba(255,255,200,0.5)
participant WinMgmt
end
rect rgba(200,255,255,0.5)
participant BootConfig
end
Operator->>Script: Invoke enable script (optional Description)
Script->>Script: Check Administrator privileges
Script->>Script: Start-Transcript (try/catch → set TranscriptStarted)
Script->>WinMgmt: Query Win32_DeviceGuard / DeviceGuard status
Script->>Registry: Modify HVCI/VBS keys (set Enabled/EnableVirtualizationBasedSecurity)
Script->>BootConfig: Run bcdedit to set hypervisorlaunchtype=Auto / restore vsmlaunchtype
Script->>Registry: Remove OverlayTestMode or Kernel DMA policy override if applicable
Script->>WinMgmt: Re-query status / verify changes
Script->>Script: Stop-Transcript if TranscriptStarted
Script-->>Operator: Summary (HVCI, VBS, Hypervisor, MPO) + restart required
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
Disabling MPO can cause screen flickering on older machines. This script removes the OverlayTestMode registry value to restore Windows default MPO behavior. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Added $TranscriptStarted flag to disable script - Created enable script to reverse core isolation changes - Restores HVCI, VBS, and hypervisor settings to defaults Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@msft-windows/msft-windows-enable-mpo.ps1`:
- Around line 1-4: The header comment only documents $Description but the script
also reads $RMM and $RMMScriptPath when running under RMM; update the
top-of-file RMM variables block to list every required RMM environment variable
by name (including $Description, $RMM, and $RMMScriptPath) and add short inline
comments explaining their purpose so operators know which variables must be set
when running from an RMM.
- Around line 115-120: The verification currently logs a warning when
Get-ItemProperty returns a value for "OverlayTestMode" but allows the script to
continue; change this so the verification is a hard failure: in the try block
where you set $verifyValue and detect $verifyValue.OverlayTestMode, replace the
Write-Host warning with a hard error path that stops execution (for example use
Write-Error and then Exit 1 or throw) so the script returns a non-zero exit code
when OverlayTestMode still exists; keep the existing success message in the
catch branch that confirms removal.
🧹 Nitpick comments (2)
msft-windows/msft-windows-enable-mpo.ps1 (2)
11-52: Normalize local variable names to camelCase (keep RMM inputs compatible).Locals like
$ScriptLogName,$ValidInput,$Description,$LogPath, and$TranscriptStarteduse PascalCase; repo standard is camelCase. Consider aliasing$RMM/$RMMScriptPathand renaming locals, then apply consistently through the file.♻️ Example refactor pattern
-$ScriptLogName = "msft-windows-enable-mpo.log" +$scriptLogName = "msft-windows-enable-mpo.log" +$rmm = $RMM +$rmmScriptPath = $RMMScriptPath -if ($RMM -ne 1) { - $ValidInput = 0 +if ($rmm -ne 1) { + $validInput = 0 @@ - $Description = Read-Host "Please enter the ticket # and, or your initials. Its used as the Description for the job" - if ($Description) { - $ValidInput = 1 + $description = Read-Host "Please enter the ticket # and, or your initials. Its used as the Description for the job" + if ($description) { + $validInput = 1 @@ - $LogPath = "$ENV:WINDIR\logs\$ScriptLogName" + $logPath = "$ENV:WINDIR\logs\$scriptLogName" @@ - if ($null -ne $RMMScriptPath) { - $LogPath = "$RMMScriptPath\logs\$ScriptLogName" + if ($null -ne $rmmScriptPath) { + $logPath = "$rmmScriptPath\logs\$scriptLogName" @@ - if ($null -eq $Description) { + if ($null -eq $description) { Write-Host "Description is null. This was most likely run automatically from the RMM and no information was passed." - $Description = "Windows Multiplane Overlay (MPO) Enable" + $description = "Windows Multiplane Overlay (MPO) Enable" } @@ -$TranscriptStarted = $false +$transcriptStarted = $falseAs per coding guidelines, use camelCase for PowerShell variable names.
42-48: Create the log directory beforeStart-Transcript.
Start-Transcriptfails if the folder doesn’t exist (common with$RMMScriptPath\logs). Consider ensuring the directory is present first.🛠️ Suggested change
try { + $logDir = Split-Path -Path $LogPath -Parent + if (-not (Test-Path $logDir)) { + New-Item -ItemType Directory -Path $logDir -Force | Out-Null + } Start-Transcript -Path $LogPath -ErrorAction Stop $TranscriptStarted = $true } catch {
Comment on lines
+1
to
+4
| ## PLEASE COMMENT YOUR VARIABLES DIRECTLY BELOW HERE IF YOU'RE RUNNING FROM A RMM | ||
| ## THIS IS HOW WE EASILY LET PEOPLE KNOW WHAT VARIABLES NEED SET IN THE RMM | ||
| ## $Description | ||
|
|
Contributor
There was a problem hiding this comment.
Document all RMM variables used by the script.
Only $Description is listed, but the script also reads $RMM and $RMMScriptPath when running under RMM. The header should list every required RMM variable.
📝 Proposed fix
## PLEASE COMMENT YOUR VARIABLES DIRECTLY BELOW HERE IF YOU'RE RUNNING FROM A RMM
## THIS IS HOW WE EASILY LET PEOPLE KNOW WHAT VARIABLES NEED SET IN THE RMM
## $Description
+## $RMM
+## $RMMScriptPathAs per coding guidelines, ensure all required RMM variables are documented.
📝 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.
Suggested change
| ## PLEASE COMMENT YOUR VARIABLES DIRECTLY BELOW HERE IF YOU'RE RUNNING FROM A RMM | |
| ## THIS IS HOW WE EASILY LET PEOPLE KNOW WHAT VARIABLES NEED SET IN THE RMM | |
| ## $Description | |
| ## PLEASE COMMENT YOUR VARIABLES DIRECTLY BELOW HERE IF YOU'RE RUNNING FROM A RMM | |
| ## THIS IS HOW WE EASILY LET PEOPLE KNOW WHAT VARIABLES NEED SET IN THE RMM | |
| ## $Description | |
| ## $RMM | |
| ## $RMMScriptPath | |
🤖 Prompt for AI Agents
In `@msft-windows/msft-windows-enable-mpo.ps1` around lines 1 - 4, The header
comment only documents $Description but the script also reads $RMM and
$RMMScriptPath when running under RMM; update the top-of-file RMM variables
block to list every required RMM environment variable by name (including
$Description, $RMM, and $RMMScriptPath) and add short inline comments explaining
their purpose so operators know which variables must be set when running from an
RMM.
Comment on lines
+115
to
+120
| try { | ||
| $verifyValue = Get-ItemProperty -Path $dwmPath -Name "OverlayTestMode" -ErrorAction Stop | ||
| Write-Host " Warning: OverlayTestMode still exists = $($verifyValue.OverlayTestMode)" -ForegroundColor Yellow | ||
| } catch { | ||
| Write-Host " Verified: OverlayTestMode registry value removed" -ForegroundColor Green | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "msft-windows-enable-mpo.ps1" | head -5Repository: DTC-Inc/msp-script-library
Length of output: 112
🏁 Script executed:
cat -n ./msft-windows/msft-windows-enable-mpo.ps1 | head -150Repository: DTC-Inc/msp-script-library
Length of output: 6806
Treat verification failure as a hard error.
If OverlayTestMode still exists after removal, the script currently warns but continues to report success on line 130. This creates a false-positive RMM report. The verification block must exit with a non-zero code if the value persists.
🐛 Proposed fix
try {
$verifyValue = Get-ItemProperty -Path $dwmPath -Name "OverlayTestMode" -ErrorAction Stop
Write-Host " Warning: OverlayTestMode still exists = $($verifyValue.OverlayTestMode)" -ForegroundColor Yellow
+ Write-Error " OverlayTestMode still exists; MPO may not be enabled."
+ if ($TranscriptStarted) { Stop-Transcript }
+ exit 1
} catch {
Write-Host " Verified: OverlayTestMode registry value removed" -ForegroundColor Green
}🤖 Prompt for AI Agents
In `@msft-windows/msft-windows-enable-mpo.ps1` around lines 115 - 120, The
verification currently logs a warning when Get-ItemProperty returns a value for
"OverlayTestMode" but allows the script to continue; change this so the
verification is a hard failure: in the try block where you set $verifyValue and
detect $verifyValue.OverlayTestMode, replace the Write-Host warning with a hard
error path that stops execution (for example use Write-Error and then Exit 1 or
throw) so the script returns a non-zero exit code when OverlayTestMode still
exists; keep the existing success message in the catch branch that confirms
removal.
The bcdedit hypervisorlaunchtype Off command was causing black screens on systems where GPU drivers depend on the hypervisor being present (Intel 11th gen+, some AMD APUs). Disabling HVCI/VBS via registry is sufficient to remove the security overhead. The hypervisor is now preserved for driver compatibility. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The disable script now explicitly sets hypervisorlaunchtype to Auto and removes vsmlaunchtype. This fixes systems where the old script disabled the hypervisor, causing black screens on GPUs that depend on it. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@msft-windows/msft-windows-enable-core-isolation.ps1`:
- Around line 126-151: The inline comment for RequirePlatformSecurityFeatures is
incorrect: it says value 3 (Secure Boot + DMA) but the script is setting Value 1
(Secure Boot only). Edit the code near the Set-ItemProperty call that sets
"RequirePlatformSecurityFeatures" on $deviceGuardPath to either (A) update the
comment to state "Value 1 = Secure Boot only; Value 3 = Secure Boot + DMA
Protection" if you intend to keep Value 1, or (B) change the Value from 1 to 3
if you intend to enforce DMA protection as well; ensure the comment and the
Set-ItemProperty call for "RequirePlatformSecurityFeatures" remain consistent.
🧹 Nitpick comments (1)
msft-windows/msft-windows-enable-core-isolation.ps1 (1)
1-38: Consider normalizing variable casing to camelCase.New variables like
$ScriptLogName,$LogPath, and$ValidInputuse PascalCase; the repo guideline calls for camelCase. Please confirm if you want to align naming in this new script to avoid style drift. As per coding guidelines, ...
Comment on lines
+126
to
+151
| # Step 3: Enable Virtualization Based Security (VBS) | ||
| Write-Host "Step 3: Enabling Virtualization Based Security (VBS)..." -ForegroundColor Yellow | ||
|
|
||
| $deviceGuardPath = "HKLM:\SYSTEM\CurrentControlSet\Control\DeviceGuard" | ||
|
|
||
| if (!(Test-Path $deviceGuardPath)) { | ||
| New-Item -Path $deviceGuardPath -Force | Out-Null | ||
| } | ||
|
|
||
| try { | ||
| Set-ItemProperty -Path $deviceGuardPath -Name "EnableVirtualizationBasedSecurity" -Value 1 -Type DWord -Force | ||
| Write-Host " EnableVirtualizationBasedSecurity = 1 (Enabled)" -ForegroundColor Green | ||
| $vbsSuccess = $true | ||
| } catch { | ||
| Write-Host " Failed to enable VBS: $($_.Exception.Message)" -ForegroundColor Red | ||
| } | ||
|
|
||
| try { | ||
| # Set to require Secure Boot and DMA protection (value 3) | ||
| # Value 1 = Secure Boot only, Value 3 = Secure Boot + DMA Protection | ||
| Set-ItemProperty -Path $deviceGuardPath -Name "RequirePlatformSecurityFeatures" -Value 1 -Type DWord -Force | ||
| Write-Host " RequirePlatformSecurityFeatures = 1 (Secure Boot)" -ForegroundColor Green | ||
| } catch { | ||
| Write-Host " Failed to set RequirePlatformSecurityFeatures: $($_.Exception.Message)" -ForegroundColor Yellow | ||
| } | ||
|
|
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Microsoft RequirePlatformSecurityFeatures registry value 1 3 Secure Boot DMA protection documentation
💡 Result:
Short answer
- RequirePlatformSecurityFeatures is a REG_DWORD under HKLM\SOFTWARE\Policies\Microsoft\Windows\DeviceGuard that selects the platform security level for Virtualization‑Based Security (VBS). [1]
- Value = 1: enable VBS with Secure Boot. [1]
- Value = 3: enable VBS with Secure Boot and DMA protection (requires hardware/firmware support). [1]
Notes
- VBS must also be enabled (EnableVirtualizationBasedSecurity = 1) and hardware/firmware (TPM, UEFI Secure Boot, DMA protection-capable hardware) must be present for the settings to take effect. [2]
Sources
[1] Microsoft Learn — DeviceGuard / RequirePlatformSecurityFeatures policy.
[2] Microsoft/STIG guidance referencing RequirePlatformSecurityFeatures and VBS requirements.
Align RequirePlatformSecurityFeatures comment with code.
The comment states "Set to require Secure Boot and DMA protection (value 3)" but the code sets value 1 (Secure Boot only). According to Microsoft documentation, value 1 enables VBS with Secure Boot only, while value 3 adds DMA protection. Update the comment to reflect that this script enforces Secure Boot only, or change the value to 3 if DMA protection is required.
🛠️ Suggested fix (if Secure Boot only is intended)
- # Set to require Secure Boot and DMA protection (value 3)
- # Value 1 = Secure Boot only, Value 3 = Secure Boot + DMA Protection
+ # Require Secure Boot only (value 1). Use 3 if DMA protection is desired.📝 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.
Suggested change
| # Step 3: Enable Virtualization Based Security (VBS) | |
| Write-Host "Step 3: Enabling Virtualization Based Security (VBS)..." -ForegroundColor Yellow | |
| $deviceGuardPath = "HKLM:\SYSTEM\CurrentControlSet\Control\DeviceGuard" | |
| if (!(Test-Path $deviceGuardPath)) { | |
| New-Item -Path $deviceGuardPath -Force | Out-Null | |
| } | |
| try { | |
| Set-ItemProperty -Path $deviceGuardPath -Name "EnableVirtualizationBasedSecurity" -Value 1 -Type DWord -Force | |
| Write-Host " EnableVirtualizationBasedSecurity = 1 (Enabled)" -ForegroundColor Green | |
| $vbsSuccess = $true | |
| } catch { | |
| Write-Host " Failed to enable VBS: $($_.Exception.Message)" -ForegroundColor Red | |
| } | |
| try { | |
| # Set to require Secure Boot and DMA protection (value 3) | |
| # Value 1 = Secure Boot only, Value 3 = Secure Boot + DMA Protection | |
| Set-ItemProperty -Path $deviceGuardPath -Name "RequirePlatformSecurityFeatures" -Value 1 -Type DWord -Force | |
| Write-Host " RequirePlatformSecurityFeatures = 1 (Secure Boot)" -ForegroundColor Green | |
| } catch { | |
| Write-Host " Failed to set RequirePlatformSecurityFeatures: $($_.Exception.Message)" -ForegroundColor Yellow | |
| } | |
| # Step 3: Enable Virtualization Based Security (VBS) | |
| Write-Host "Step 3: Enabling Virtualization Based Security (VBS)..." -ForegroundColor Yellow | |
| $deviceGuardPath = "HKLM:\SYSTEM\CurrentControlSet\Control\DeviceGuard" | |
| if (!(Test-Path $deviceGuardPath)) { | |
| New-Item -Path $deviceGuardPath -Force | Out-Null | |
| } | |
| try { | |
| Set-ItemProperty -Path $deviceGuardPath -Name "EnableVirtualizationBasedSecurity" -Value 1 -Type DWord -Force | |
| Write-Host " EnableVirtualizationBasedSecurity = 1 (Enabled)" -ForegroundColor Green | |
| $vbsSuccess = $true | |
| } catch { | |
| Write-Host " Failed to enable VBS: $($_.Exception.Message)" -ForegroundColor Red | |
| } | |
| try { | |
| # Require Secure Boot only (value 1). Use 3 if DMA protection is desired. | |
| Set-ItemProperty -Path $deviceGuardPath -Name "RequirePlatformSecurityFeatures" -Value 1 -Type DWord -Force | |
| Write-Host " RequirePlatformSecurityFeatures = 1 (Secure Boot)" -ForegroundColor Green | |
| } catch { | |
| Write-Host " Failed to set RequirePlatformSecurityFeatures: $($_.Exception.Message)" -ForegroundColor Yellow | |
| } |
🤖 Prompt for AI Agents
In `@msft-windows/msft-windows-enable-core-isolation.ps1` around lines 126 - 151,
The inline comment for RequirePlatformSecurityFeatures is incorrect: it says
value 3 (Secure Boot + DMA) but the script is setting Value 1 (Secure Boot
only). Edit the code near the Set-ItemProperty call that sets
"RequirePlatformSecurityFeatures" on $deviceGuardPath to either (A) update the
comment to state "Value 1 = Secure Boot only; Value 3 = Secure Boot + DMA
Protection" if you intend to keep Value 1, or (B) change the Value from 1 to 3
if you intend to enforce DMA protection as well; ensure the comment and the
Set-ItemProperty call for "RequirePlatformSecurityFeatures" remain consistent.
- Added $TranscriptStarted flag to fix transcript errors in RMM - Set Balanced power plan as active at start - Disable display timeout (monitor-timeout-ac/dc = 0) - Updated header comments and summary Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Disable display timeout using GUIDs on ALL power plans, not just active - Removed /setactive from scheme loop (was overwriting Balanced) - Set Balanced plan active at the END after all configuration Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@msft-windows/msft-windows-power-management-config.ps1`:
- Around line 5-19: The numbered header at the top of
msft-windows-power-management-config.ps1 is out of sync with the script body (it
lists activating the Balanced plan first but the script activates it later);
update the header to reflect the actual execution order or reorder the script so
the Balanced plan activation happens where the header states; locate the header
comment block and the code that activates the Balanced plan (the power plan
activation command such as powercfg /setactive or the
Set-ItemProperty/Set-PowerPlan call) and make them consistent so step numbers
and descriptions match actual execution.
- Around line 105-118: The summary always reports success even if Step 1b or
Step 8 fail; add explicit success flags (e.g., $step1bSuccess, $step8Success)
and set them true at start, set to $false inside the catch blocks or when any
powercfg /setacvalueindex or /setdcvalueindex call for a $powerSchemes entry
returns a non-zero result; for the foreach over $powerSchemes in Step 1b (and
the analogous logic in Step 8), mark failure if any iteration fails and still
log per-scheme success/failure, then change the final summary logic to read
these flags and report success or failure accordingly instead of always printing
success.
Comment on lines
+5
to
+19
| # 1. Sets Balanced power plan as active | ||
| # 2. Disables display timeout (never turn off display) | ||
| # 3. Disables hybrid sleep across all plans | ||
| # 4. Disables fast startup globally | ||
| # 5. Disables hibernation completely | ||
| # 6. Stops hard disks from turning off on all plans | ||
| # 7. Disables sleeping completely across all plans | ||
| # 8. Allows sleeping only when the lid is shut for laptops across all plans | ||
| # 9. Sets critical battery action to shutdown across all plans | ||
| # 10. Disables USB selective suspend across all plans | ||
| # 11. Disables PCIE Link State Power Management across all plans | ||
| # 12. Enables all wake timers across all plans | ||
| # 13. Sets wireless adapters to maximum performance across all plans | ||
| # 14. Sets video playback to maximum quality across all plans | ||
| # 15. Optimizes multimedia settings for best performance across all plans |
Contributor
There was a problem hiding this comment.
Align the header step list with the actual execution order.
The list implies the Balanced plan is activated first, but the script does it at Step 8. That can mislead operators and log reviewers.
📝 Suggested doc tweak
-# 1. Sets Balanced power plan as active
+# 1. Sets Balanced power plan as active (after all configuration steps)📝 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.
Suggested change
| # 1. Sets Balanced power plan as active | |
| # 2. Disables display timeout (never turn off display) | |
| # 3. Disables hybrid sleep across all plans | |
| # 4. Disables fast startup globally | |
| # 5. Disables hibernation completely | |
| # 6. Stops hard disks from turning off on all plans | |
| # 7. Disables sleeping completely across all plans | |
| # 8. Allows sleeping only when the lid is shut for laptops across all plans | |
| # 9. Sets critical battery action to shutdown across all plans | |
| # 10. Disables USB selective suspend across all plans | |
| # 11. Disables PCIE Link State Power Management across all plans | |
| # 12. Enables all wake timers across all plans | |
| # 13. Sets wireless adapters to maximum performance across all plans | |
| # 14. Sets video playback to maximum quality across all plans | |
| # 15. Optimizes multimedia settings for best performance across all plans | |
| # 1. Sets Balanced power plan as active (after all configuration steps) | |
| # 2. Disables display timeout (never turn off display) | |
| # 3. Disables hybrid sleep across all plans | |
| # 4. Disables fast startup globally | |
| # 5. Disables hibernation completely | |
| # 6. Stops hard disks from turning off on all plans | |
| # 7. Disables sleeping completely across all plans | |
| # 8. Allows sleeping only when the lid is shut for laptops across all plans | |
| # 9. Sets critical battery action to shutdown across all plans | |
| # 10. Disables USB selective suspend across all plans | |
| # 11. Disables PCIE Link State Power Management across all plans | |
| # 12. Enables all wake timers across all plans | |
| # 13. Sets wireless adapters to maximum performance across all plans | |
| # 14. Sets video playback to maximum quality across all plans | |
| # 15. Optimizes multimedia settings for best performance across all plans |
🤖 Prompt for AI Agents
In `@msft-windows/msft-windows-power-management-config.ps1` around lines 5 - 19,
The numbered header at the top of msft-windows-power-management-config.ps1 is
out of sync with the script body (it lists activating the Balanced plan first
but the script activates it later); update the header to reflect the actual
execution order or reorder the script so the Balanced plan activation happens
where the header states; locate the header comment block and the code that
activates the Balanced plan (the power plan activation command such as powercfg
/setactive or the Set-ItemProperty/Set-PowerPlan call) and make them consistent
so step numbers and descriptions match actual execution.
Comment on lines
+105
to
+118
| # Step 1b: Disable display timeout on ALL power plans (never turn off display) | ||
| Write-Host "Step 1b: Disabling display timeout on all power plans..." -ForegroundColor Yellow | ||
| try { | ||
| # SUB_VIDEO = 7516b95f-f776-4464-8c53-06167f40cc99 | ||
| # VIDEOIDLE (display timeout) = 3c0bc021-c8a8-4e07-a973-6b14cbcb2b7e | ||
| foreach ($scheme in $powerSchemes) { | ||
| powercfg /setacvalueindex $($scheme.GUID) 7516b95f-f776-4464-8c53-06167f40cc99 3c0bc021-c8a8-4e07-a973-6b14cbcb2b7e 0 | Out-Null | ||
| powercfg /setdcvalueindex $($scheme.GUID) 7516b95f-f776-4464-8c53-06167f40cc99 3c0bc021-c8a8-4e07-a973-6b14cbcb2b7e 0 | Out-Null | ||
| Write-Host " Display timeout disabled for '$($scheme.Name)'" -ForegroundColor Green | ||
| } | ||
| } catch { | ||
| Write-Host " Failed to disable display timeout: $($_.Exception.Message)" -ForegroundColor Yellow | ||
| } | ||
| Write-Host "" |
Contributor
There was a problem hiding this comment.
Make the summary reflect actual outcomes.
Step 1b and Step 8 can fail, but the summary always reports success. This can mislead RMM logs. Consider tracking success flags and conditionally summarizing.
✅ Example: track success and report accurately
- # Step 1b: Disable display timeout on ALL power plans (never turn off display)
+ $displayTimeoutDisabled = $false
+ # Step 1b: Disable display timeout on ALL power plans (never turn off display)
Write-Host "Step 1b: Disabling display timeout on all power plans..." -ForegroundColor Yellow
try {
# SUB_VIDEO = 7516b95f-f776-4464-8c53-06167f40cc99
# VIDEOIDLE (display timeout) = 3c0bc021-c8a8-4e07-a973-6b14cbcb2b7e
foreach ($scheme in $powerSchemes) {
powercfg /setacvalueindex $($scheme.GUID) 7516b95f-f776-4464-8c53-06167f40cc99 3c0bc021-c8a8-4e07-a973-6b14cbcb2b7e 0 | Out-Null
powercfg /setdcvalueindex $($scheme.GUID) 7516b95f-f776-4464-8c53-06167f40cc99 3c0bc021-c8a8-4e07-a973-6b14cbcb2b7e 0 | Out-Null
Write-Host " Display timeout disabled for '$($scheme.Name)'" -ForegroundColor Green
}
+ $displayTimeoutDisabled = $true
} catch {
Write-Host " Failed to disable display timeout: $($_.Exception.Message)" -ForegroundColor Yellow
}
- # Step 8: Set Balanced power plan as active (do this LAST after all configuration)
+ $balancedActivated = $false
+ # Step 8: Set Balanced power plan as active (do this LAST after all configuration)
Write-Host "Step 8: Setting Balanced power plan as active..." -ForegroundColor Yellow
try {
# Balanced power plan GUID is the same on all Windows installations
$balancedGUID = "381b4222-f694-41f0-9685-ff5bb260df2e"
powercfg /setactive $balancedGUID
if ($LASTEXITCODE -eq 0) {
+ $balancedActivated = $true
Write-Host " Balanced power plan activated" -ForegroundColor Green
} else {
Write-Host " Could not set Balanced plan (may not exist)" -ForegroundColor Yellow
}
} catch {
Write-Host " Failed to set Balanced power plan: $($_.Exception.Message)" -ForegroundColor Yellow
}
- Write-Host "Balanced power plan set as active" -ForegroundColor Green
- Write-Host "Display timeout disabled (never turn off)" -ForegroundColor Green
+ if ($balancedActivated) {
+ Write-Host "Balanced power plan set as active" -ForegroundColor Green
+ } else {
+ Write-Host "Balanced power plan not activated (see warnings above)" -ForegroundColor Yellow
+ }
+ if ($displayTimeoutDisabled) {
+ Write-Host "Display timeout disabled (never turn off)" -ForegroundColor Green
+ } else {
+ Write-Host "Display timeout not fully configured (see warnings above)" -ForegroundColor Yellow
+ }Also applies to: 371-385, 389-403
🤖 Prompt for AI Agents
In `@msft-windows/msft-windows-power-management-config.ps1` around lines 105 -
118, The summary always reports success even if Step 1b or Step 8 fail; add
explicit success flags (e.g., $step1bSuccess, $step8Success) and set them true
at start, set to $false inside the catch blocks or when any powercfg
/setacvalueindex or /setdcvalueindex call for a $powerSchemes entry returns a
non-zero result; for the foreach over $powerSchemes in Step 1b (and the
analogous logic in Step 8), mark failure if any iteration fails and still log
per-scheme success/failure, then change the final summary logic to read these
flags and report success or failure accordingly instead of always printing
success.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Start-Transcript can fail silently in certain RMM environments. Added $TranscriptStarted flag to track if transcription is active and only call Stop-Transcript when it was successfully started.
Summary by CodeRabbit
New Features
Bug Fixes
Other
✏️ Tip: You can customize this high-level summary in your review settings.