Skip to content

feat(oem-dell): add server lifecycle upgrade-vs-replace audit collector #86

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
AlrightLad wants to merge 3 commits into
base: main
Choose a base branch
from
Open

feat(oem-dell): add server lifecycle upgrade-vs-replace audit collector #86

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c08307e
feat(oem-dell): add server lifecycle upgrade-vs-replace audit collector
AlrightLad Jun 15, 2026
bc96f45
fix(oem-dell): report exact device CAL count, not 5-packs
AlrightLad Jul 6, 2026
6a5e67c
fix(oem-dell): remove PS 5.0-only constructs; set PS 3.0 floor
AlrightLad Jul 7, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
278 changes: 278 additions & 0 deletions oem-dell/get-server-lifecycle-audit.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,278 @@
<#
.SYNOPSIS
Read-only server lifecycle audit collector for upgrade-vs-replace quoting.
.DESCRIPTION
Gathers every input a senior technician needs to quote and review a PowerEdge
server OS upgrade vs. replacement: identity/hardware, boot/firmware (2022 gate),
iDRAC remote-recovery readiness, OS, disk, roles, SQL edition, dental/LOB stack,
and CAL sizing counts. Model-agnostic (T130/T230/T330/T340/T440/etc.). Makes NO
changes to the system. Pairs with KB page 3801 (decision gates and quote paths).

Output is a single paste-ready text report on the Output stream (captured by
NinjaOne stdout and the transcript log). The engineer applies the KB 3801 gates.
.PARAMETER ClientName
Client/practice name for the report header. Resolution order:
explicit -ClientName, then $env:ClientName (NinjaOne RMM variable named ClientName),
then an interactive prompt ONLY when a user session is present, then 'UNSPECIFIED'.
Not mandatory by design - a mandatory parameter would hang an unattended NinjaOne run.
.EXAMPLE
.\get-server-lifecycle-audit.ps1 -ClientName 'Centreville Endo'
.EXAMPLE
# NinjaOne: define a script variable named ClientName; it arrives as $env:ClientName.
.NOTES
Author: Zach Boogher
Repo: msp-script-library/ninjaone/audit/get-server-lifecycle-audit.ps1 (propose ninjaone/audit/ in PR)
Standards: KB 3299 (PowerShell Engineering Patterns), KB 3344 (Contributing a Script)
Read-only; inherently idempotent. PowerShell 3.0+ compatible (audits downlevel WMF 3/4 hosts).
#>

#Requires -Version 3.0

[CmdletBinding()]
param(
[string]$ClientName
)

$ErrorActionPreference = 'Stop'
$ProgressPreference = 'SilentlyContinue'

function Get-DTCServerLifecycleAudit {
[CmdletBinding()]
param(
[string]$ClientName = 'UNSPECIFIED'
)

$lines = New-Object -TypeName 'System.Collections.Generic.List[string]'

$lines.Add('========================================================================')
$lines.Add((" SERVER LIFECYCLE AUDIT (read-only) {0}" -f (Get-Date -Format 'yyyy-MM-dd HH:mm')))
$lines.Add((" Client : {0}" -f $ClientName))
$lines.Add((" Host : {0}" -f $env:COMPUTERNAME))
$lines.Add(' Apply KB 3801 Section 4 gates. Verify warranty + iDRAC license out-of-band.')
$lines.Add('========================================================================')

# ----- IDENTITY / HARDWARE -----
$lines.Add('')
$lines.Add('== IDENTITY / HARDWARE ==')
$cs = Get-CimInstance -ClassName Win32_ComputerSystem
$bios = Get-CimInstance -ClassName Win32_BIOS
$cpu = Get-CimInstance -ClassName Win32_Processor
$cores = ($cpu | Measure-Object -Property NumberOfCores -Sum).Sum
$logical = ($cpu | Measure-Object -Property NumberOfLogicalProcessors -Sum).Sum
$ramGB = [math]::Round((Get-CimInstance -ClassName Win32_PhysicalMemory | Measure-Object -Property Capacity -Sum).Sum / 1GB, 0)
$memArray = Get-CimInstance -ClassName Win32_PhysicalMemoryArray | Select-Object -First 1
$slotsUsed = (Get-CimInstance -ClassName Win32_PhysicalMemory | Measure-Object).Count
$maxRamGB = $null
if ($memArray.MaxCapacityEx) { $maxRamGB = [math]::Round($memArray.MaxCapacityEx / 1MB, 0) }
elseif ($memArray.MaxCapacity) { $maxRamGB = [math]::Round($memArray.MaxCapacity / 1MB, 0) }

$lines.Add(("Manufacturer : {0}" -f $cs.Manufacturer))
$lines.Add(("Model : {0}" -f $cs.Model))
$lines.Add(("Service Tag : {0} (look up warranty/ship date at dell.com/support)" -f $bios.SerialNumber))
$lines.Add(("BIOS version : {0}" -f $bios.SMBIOSBIOSVersion))
$lines.Add(("CPU : {0}" -f (($cpu | Select-Object -First 1).Name).Trim()))
$lines.Add(("Sockets/Cores : {0} socket(s), {1} physical, {2} logical" -f ($cpu | Measure-Object).Count, $cores, $logical))
$lines.Add(("RAM installed : {0} GB (slots used {1} of {2})" -f $ramGB, $slotsUsed, $memArray.MemoryDevices))
if ($maxRamGB) { $lines.Add(("RAM max : {0} GB (firmware-reported; confirm against model spec)" -f $maxRamGB)) }

# Generation / iDRAC inference from the 3-digit model number (T340 -> 14G/iDRAC9, T330 -> 13G/iDRAC8)
Comment thread
claude[bot] marked this conversation as resolved.
$genText = 'verify'
$idracGen = 'verify'
if ($cs.Model -match '[A-Za-z](\d)(\d)0(\b|$)') {
$gen = 10 + [int]$Matches[2]
$genText = "{0}G" -f $gen
if ($gen -le 13) { $idracGen = 'iDRAC8' } else { $idracGen = 'iDRAC9' }
}
Comment on lines +81 to +85

@claude claude Bot Jul 6, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 The iDRAC generation inference at line 82 collapses 11G, 12G, and 13G all into iDRAC8 via if ($gen -le 13) { $idracGen = 'iDRAC8' }. Dell's actual mapping is 11G→iDRAC6, 12G→iDRAC7, 13G→iDRAC8, 14G+→iDRAC9, so unsuffixed 11G/12G models that match the regex (T410, R610, T320, R620, etc.) get labeled iDRAC8 (inferred from model) on hardware that actually ships iDRAC6/iDRAC7 — framed as authoritative, no verify hint. Distinct from the suffixed-model regex issue already flagged: that path fails safely to verify; this one succeeds with wrong data. Fix is a 3-line elseif ladder.

Extended reasoning...

What the bug is. Lines 79-83:

if ($cs.Model -match '[A-Za-z](\d)(\d)0(\b|$)') {
    $gen = 10 + [int]$Matches[2]
    $genText = "{0}G" -f $gen
    if ($gen -le 13) { $idracGen = 'iDRAC8' } else { $idracGen = 'iDRAC9' }
}

The final if ($gen -le 13) collapses three distinct Dell hardware generations into iDRAC8. Dell's documented iDRAC-to-generation mapping is well established:

  • 11G PowerEdge (T110/T310/T410/R610/R710/R910, ~2009-2011) → iDRAC6
  • 12G PowerEdge (T20/T320/T420/T620/R220-R920, ~2012-2014) → iDRAC7
  • 13G PowerEdge (T130/T330/T430/T630/R230-R930, ~2014-2017) → iDRAC8
  • 14G+ PowerEdge (T140/T340/T440/T640/R240-R940 and newer) → iDRAC9

Why the existing code doesn't prevent it. The regex [A-Za-z](\d)(\d)0(\b|$) at line 79 successfully matches unsuffixed 11G/12G models — their last char is '0' followed by end-of-string, so (\b|$) fires on $. The generation number is computed correctly (11 for T410, 12 for T320); only the iDRAC mapping is wrong.

Distinct from the already-flagged suffixed-model bug. The prior comment about R750xa/R760xs covers the case where the regex fails and both $genText and $idracGen fall back to 'verify' — that path is fail-safe. This bug is the opposite: the regex succeeds, $genText is correct ('11G', '12G'), but $idracGen is silently wrong. The report emits iDRAC gen : iDRAC8 (inferred from model) — authoritative framing, no verify cue for the reader to double-check.

Impact. Limited but real for the target use case. The .SYNOPSIS claims Model-agnostic and the audit is specifically designed for upgrade-vs-replace quoting — 11G/12G PowerEdges (2009-2014) are exactly the population most likely to be flagged for replacement, so this collector will frequently run on the affected models. Mitigating: the downstream iDRAC license line (racadm license view) pulls ground-truth from the hardware when racadm.exe is present, and the READINESS NOTE tells the technician to confirm licensing in the iDRAC web UI. Aggravating: a senior tech reading iDRAC8 on an actual iDRAC6 R710 could waste time hunting for an Enterprise license SKU that never existed on that generation, or map to the wrong KB section since iDRAC6/7 have materially different Virtual Console/Virtual Media licensing models than iDRAC8/9.

Step-by-step proof.
Take $cs.Model = 'PowerEdge T410' (11G, actual hardware ships iDRAC6):

  1. Regex [A-Za-z](\d)(\d)0(\b|$) scans and matches T-4-1-0 at the end of the string; $ in the alternation fires.
  2. $Matches[2] = '1', so $gen = 10 + 1 = 11.
  3. $genText = '11G' — correct.
  4. 11 -le 13 is $true → $idracGen = 'iDRAC8' — wrong (T410 has iDRAC6).
  5. Report emits: iDRAC gen : iDRAC8 (inferred from model) with no verify hedge.

Same trace for PowerEdge R720 (12G, actual iDRAC7): $Matches[2]='2', $gen=12, 12 -le 13 → iDRAC8. Wrong. And for PowerEdge T330 (13G, actual iDRAC8): $Matches[2]='3', $gen=13, 13 -le 13 → iDRAC8. Correct — 13G is the only generation this branch labels right.

Fix. Three-line elseif ladder, doesn't touch the primary 13G/14G+ case:

if     ($gen -le 11) { $idracGen = 'iDRAC6' }
elseif ($gen -eq 12) { $idracGen = 'iDRAC7' }
elseif ($gen -eq 13) { $idracGen = 'iDRAC8' }
else                 { $idracGen = 'iDRAC9' }

$lines.Add(("Generation : {0} (inferred from model)" -f $genText))

# ----- iDRAC / REMOTE-RECOVERY READINESS -----
$lines.Add('')
$lines.Add('== iDRAC / REMOTE-RECOVERY READINESS (required before any in-place OS upgrade) ==')
$lines.Add(("iDRAC gen : {0} (inferred from model)" -f $idracGen))
$ism = Get-Service -ErrorAction SilentlyContinue | Where-Object { $_.DisplayName -match 'iDRAC Service Module' -or $_.Name -eq 'dcism' }
if ($ism) { $lines.Add(("iDRAC Svc Module: present [{0}] (OS<->iDRAC pass-through available)" -f ($ism | Select-Object -First 1).Status)) }
else { $lines.Add('iDRAC Svc Module: not installed (host cannot read iDRAC over USB pass-through)') }
$idracNic = Get-CimInstance -ClassName Win32_NetworkAdapter -ErrorAction SilentlyContinue | Where-Object { $_.Name -match 'Remote NDIS|iDRAC' }
if ($idracNic) { $lines.Add(("iDRAC USB NIC : present ({0}) (iDRAC reachable from host)" -f ($idracNic | Select-Object -First 1).Name)) }
else { $lines.Add('iDRAC USB NIC : not detected (OS-to-iDRAC pass-through off, or no iDRAC)') }

# racadm (native command) - read iDRAC license if the binary is present
$racadm = Get-Command -Name 'racadm.exe' -ErrorAction SilentlyContinue
$racPath = $null
if ($racadm) { $racPath = $racadm.Source }
if (-not $racPath) {
foreach ($p in 'C:\Program Files\Dell\SysMgt\iDRACTools\racadm\racadm.exe', 'C:\Program Files\Dell\SysMgt\rac5\racadm.exe') {
if (Test-Path $p) { $racPath = $p; break }
}
}
$idracLicense = 'UNKNOWN'
if ($racPath) {
$lic = & $racPath 'license' 'view' 2>$null
if ($LASTEXITCODE -eq 0 -and $lic) {
$licLine = $lic | Where-Object { $_ -match 'License Description|Enterprise|Express|Datacenter' } | Select-Object -First 1
if ($licLine) { $idracLicense = ($licLine -replace '\s+', ' ').Trim() }
}
$nic = & $racPath 'getniccfg' 2>$null
if ($LASTEXITCODE -eq 0 -and $nic) {
$ip = $nic | Where-Object { $_ -match 'IP Address' } | Select-Object -First 1
if ($ip) { $lines.Add(("iDRAC NIC cfg : {0}" -f ($ip -replace '\s+', ' ').Trim())) }
}
}
$lines.Add(("iDRAC license : {0}" -f $idracLicense))
$lines.Add('READINESS NOTE : Virtual Console + Virtual Media (remote OS-upgrade monitoring and')
$lines.Add(' catastrophic-failure recovery) require iDRAC Enterprise or Datacenter.')
$lines.Add(' Express does NOT include them. If UNKNOWN, confirm in the iDRAC web UI')
$lines.Add(' before committing to an in-place upgrade.')

# ----- OPERATING SYSTEM -----
$lines.Add('')
$lines.Add('== OPERATING SYSTEM ==')
$os = Get-CimInstance -ClassName Win32_OperatingSystem
$lines.Add(("OS : {0}" -f $os.Caption))
$lines.Add(("Version/Build : {0}" -f $os.Version))
$lines.Add(("Architecture : {0}" -f $os.OSArchitecture))
$lines.Add(("Installed : {0}" -f $os.InstallDate))

# ----- BOOT / FIRMWARE (2022 eligibility) -----
$lines.Add('')
$lines.Add('== BOOT / FIRMWARE (in-place 2022 on 14G needs TPM 2.0 + UEFI + Secure Boot) ==')
$bootMode = 'UNKNOWN'
try {
$sb = Confirm-SecureBootUEFI
$bootMode = 'UEFI'
$lines.Add('Boot firmware : UEFI')
if ($sb) { $lines.Add('Secure Boot : ON') } else { $lines.Add('Secure Boot : OFF') }
} catch {
$bootMode = 'LEGACY'
$lines.Add('Boot firmware : LEGACY/BIOS (not UEFI) -> BIOS->UEFI conversion is its own project')
$lines.Add('Secure Boot : N/A (legacy boot)')
}
try {
$tpm = Get-Tpm
$lines.Add(("TPM present : {0}" -f $tpm.TpmPresent))
$lines.Add(("TPM ready : {0}" -f $tpm.TpmReady))
$tpmSpec = (Get-CimInstance -Namespace 'root\cimv2\security\microsofttpm' -ClassName Win32_Tpm -ErrorAction SilentlyContinue).SpecVersion
$lines.Add(("TPM spec : {0}" -f $tpmSpec))
} catch {
$lines.Add('TPM : query failed (run elevated; or no TPM present)')
}

# ----- DISK / STORAGE (SSD-swap input) -----
$lines.Add('')
$lines.Add('== DISK / STORAGE ==')
try {
Get-PhysicalDisk | Sort-Object -Property DeviceId | ForEach-Object {
$lines.Add(("PhysDisk {0}: {1} GB Media={2} Bus={3} Health={4}" -f $_.DeviceId, [math]::Round($_.Size / 1GB, 0), $_.MediaType, $_.BusType, $_.HealthStatus))
}
} catch { $lines.Add('PhysDisk : Get-PhysicalDisk unavailable on this OS') }
Get-CimInstance -ClassName Win32_LogicalDisk -Filter 'DriveType=3' | ForEach-Object {
$lines.Add(("Volume {0} : {1} GB free of {2} GB" -f $_.DeviceID, [math]::Round($_.FreeSpace / 1GB, 1), [math]::Round($_.Size / 1GB, 1)))
}

# ----- SERVER ROLES (CAL + workload drivers) -----
$lines.Add('')
$lines.Add('== SERVER ROLES ==')
$domainRole = $cs.DomainRole # 4/5 = DC
$isDC = $domainRole -in 4, 5
if ($isDC) { $lines.Add(("Domain role : {0} (DOMAIN CONTROLLER)" -f $domainRole)) }
else { $lines.Add(("Domain role : {0} (member/standalone)" -f $domainRole)) }
$features = $null
try { Import-Module ServerManager -ErrorAction Stop; $features = Get-WindowsFeature | Where-Object { $_.Installed } } catch { $features = $null }
if ($features) {
$watch = 'AD-Domain-Services', 'DNS', 'DHCP', 'FS-FileServer', 'Hyper-V', 'RDS-RD-Server', 'RDS-Connection-Broker', 'RDS-Gateway', 'Remote-Desktop-Services', 'Web-Server', 'Print-Services'
$features | Where-Object { $watch -contains $_.Name } | ForEach-Object { $lines.Add(("Role/Feature : {0} ({1})" -f $_.DisplayName, $_.Name)) }
$rds = $features | Where-Object { $_.Name -like 'RDS-*' -or $_.Name -eq 'Remote-Desktop-Services' }
if ($rds) { $lines.Add('RDS role active : TRUE -> RDS User CALs (SFT-083 2022 / SFT-112 2025) apply') }
else { $lines.Add('RDS role active : FALSE -> do NOT quote RDS User CALs') }
if ($features | Where-Object { $_.Name -eq 'Hyper-V' }) { $lines.Add('Hyper-V HOST : TRUE -> treat per host build standard, not a simple upgrade') }
} else {
$lines.Add('Roles : Get-WindowsFeature unavailable (non-Server SKU or module missing) - confirm manually')
}

# ----- SQL (license trigger) -----
$lines.Add('')
$lines.Add('== SQL SERVER INSTANCES (SFT-107 applies ONLY for a licensed edition; Express/LocalDB are free) ==')
$sqlInstances = New-Object -TypeName 'System.Collections.Generic.List[string]'
try {
$instKey = Get-ItemProperty -Path 'HKLM:\SOFTWARE\Microsoft\Microsoft SQL Server\Instance Names\SQL' -ErrorAction Stop
$instKey.PSObject.Properties | Where-Object { $_.Name -notmatch '^PS' } | ForEach-Object {
$instId = $_.Value
$setup = Get-ItemProperty -Path ("HKLM:\SOFTWARE\Microsoft\Microsoft SQL Server\{0}\Setup" -f $instId) -ErrorAction SilentlyContinue
$sqlInstances.Add($_.Name)
$lines.Add(("SQL instance : {0} Edition='{1}' Version={2}" -f $_.Name, $setup.Edition, $setup.Version))
}
} catch {
Write-Verbose "SQL instance enumeration skipped: $_"
}
if ($sqlInstances.Count -eq 0) { $lines.Add('SQL instance : none detected via registry') }
$lines.Add('NOTE: PBS Endo IS SQL-backed and is a known SFT-107 driver. SoftDent/Dentrix use')
$lines.Add(' FairCom c-tree (no SQL license). Quote SFT-107 only when Edition reads')
$lines.Add(' Standard/Web/Enterprise; confirm exact edition + version first.')

# ----- DENTAL / LOB SOFTWARE -----
$lines.Add('')
$lines.Add('== DENTAL / LOB SOFTWARE DETECTED (map each to its compat page in Dental Apps Book 70) ==')
$patterns = 'SoftDent', 'Carestream', 'CS Imaging', 'PracticeWorks', 'Dentrix', 'Eaglesoft', 'Open Dental', 'OpenDental', 'PBS Endo', 'PBSEndo', 'DEXIS', 'Sidexis', 'EZDent', 'Vatech', 'EzServer', 'Apteryx', 'Romexis', 'Planmeca', 'MiPACS'
$uninstallPaths = 'HKLM:\SOFTWARE\Microsoft\Windows\CurrentVersion\Uninstall\*', 'HKLM:\SOFTWARE\WOW6432Node\Microsoft\Windows\CurrentVersion\Uninstall\*'
$installed = foreach ($path in $uninstallPaths) {
Get-ItemProperty -Path $path -ErrorAction SilentlyContinue | Where-Object { $_.DisplayName } | ForEach-Object { [pscustomobject]@{ Name = $_.DisplayName; Version = $_.DisplayVersion } }
}
$matched = $installed | Where-Object { $name = $_.Name; ($patterns | Where-Object { $name -match [regex]::Escape($_) }) } | Sort-Object -Property Name -Unique
if ($matched) { $matched | ForEach-Object { $lines.Add(("Installed app : {0} v{1}" -f $_.Name, $_.Version)) } }
else { $lines.Add('Installed app : no known dental LOB matched in uninstall registry (verify; some do not register)') }
Get-Service -ErrorAction SilentlyContinue | Where-Object { $svc = $_.DisplayName; ($patterns | Where-Object { $svc -match [regex]::Escape($_) }) } | ForEach-Object { $lines.Add(("Service : {0} [{1}]" -f $_.DisplayName, $_.Status)) }
if (Get-Process -ErrorAction SilentlyContinue | Where-Object { $_.ProcessName -in 'ctreesql', 'DentrixACEServer', 'FairComEdge' }) { $lines.Add('DB engine : FairCom c-tree present (no SQL license needed)') }

# ----- CAL SIZING INPUTS -----
$lines.Add('')
$lines.Add('== CAL SIZING INPUTS (Device CALs billed per device; marked individually, do NOT round to 5-packs) ==')
if ($isDC) {
try {
Import-Module ActiveDirectory -ErrorAction Stop
$userCount = (Get-ADUser -Filter 'Enabled -eq $true' | Measure-Object).Count
$wsCount = (Get-ADComputer -Filter "OperatingSystem -notlike '*Server*'" | Measure-Object).Count
$lines.Add(("AD enabled users : {0}" -f $userCount))
$lines.Add(("AD workstations : {0}" -f $wsCount))
$lines.Add(("Device CALs needed : {0} (1 per device, marked individually - do NOT round to 5-packs) [SFT-109 for 2022 / SFT-111 for 2025]" -f $wsCount))
Comment on lines +232 to +236

@claude claude Bot Jul 6, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 The workstation query on line 231 is missing the Enabled -eq $true filter that the sibling Get-ADUser on line 230 uses, so disabled/decommissioned workstation computer accounts (and prestaged accounts whose OperatingSystem attribute is still null) get counted. That inflated $wsCount feeds directly into the Device CALs needed line, which the format string explicitly warns must be counted individually and NOT rounded to 5-packs — so any inflation lands 1:1 on the quoted CAL total with no rounding cushion. Fix is a one-clause consistency change: -Filter "Enabled -eq \$true -and OperatingSystem -notlike '*Server*'" to match the neighboring user query.

Extended reasoning...

What the bug is. Lines 230-231 of oem-dell/get-server-lifecycle-audit.ps1:\n\npowershell\n$userCount = (Get-ADUser -Filter 'Enabled -eq $true' | Measure-Object).Count\n$wsCount = (Get-ADComputer -Filter "OperatingSystem -notlike '*Server*'" | Measure-Object).Count\n\n\nThe user query correctly filters out disabled accounts; the workstation query directly below does not. The mismatch is even visible in the labels the report emits on lines 232-233: AD enabled users vs. just AD workstations (no enabled qualifier).\n\nTwo inflation vectors. First, disabled workstation computer accounts still in AD get counted. This is the common case — MSP shops routinely disable rather than tombstone-delete decommissioned machines for auditability, so stale-workstation counts of 5-30% above the active fleet are typical. Second, computer accounts where OperatingSystem is $null match -notlike '*Server*' because in LDAP the predicate compiles to (!(operatingSystem=*Server*)), and RFC 4515 negation of an Undefined attribute match returns the entry — freshly-joined machines that haven't reported their OS, prestaged accounts, and orphaned computer objects all get counted.\n\nWhy existing code doesn't prevent it. There is no post-filter in the pipeline — $wsCount is the raw Get-ADComputer count fed straight into the Device CALs needed line at 234. The parallel user query already established the correct pattern one line above, so this is a pure consistency omission.\n\nImpact — the specific field where inflation matters most. The format string on line 234 reads: Device CALs needed : {0} (1 per device, marked individually - do NOT round to 5-packs) [SFT-109 for 2022 / SFT-111 for 2025]. The header on line 226 reinforces this: Device CALs billed per device; marked individually, do NOT round to 5-packs. The whole reason this collector exists is to produce the exact per-device count a senior tech pastes into an SFT-109/SFT-111 quote line, and the header explicitly instructs not to round — so unlike most CAL counts, there is no 5-pack cushion to absorb an over-count. A tech reading AD workstations : 27 / Device CALs needed : 27 has no independent signal that 4 of those 27 are decommissioned machines in the Disabled Computers OU.\n\nWhy this is still a nit, not a blocker. The report is explicitly designed for senior-tech review before it becomes a quote (line 242: senior tech makes the call), the audit is read-only, and no data is written back to the system. A senior tech eyeballing an unusually high count would sanity-check it. This is a data-quality inconsistency in the primary output field, not a functional break — but it directly counteracts the audit's stated purpose and the fix is trivially consistent with the immediately preceding line, so it's worth fixing before merge.\n\nStep-by-step proof.\n1. Domain has 30 workstation computer objects. 25 are Enabled with OS = Windows 11 Pro; 4 are Disabled with OS = Windows 10 Pro (retired but not deleted); 1 is prestaged Enabled with OS = $null.\n2. Get-ADUser -Filter 'Enabled -eq $true' → returns only enabled users (correct).\n3. Get-ADComputer -Filter "OperatingSystem -notlike '*Server*'" compiles to LDAP (!(operatingSystem=*Server*)) with no Enabled predicate. It matches all 25 enabled + 4 disabled + 1 null-OS = 30 objects.\n4. $wsCount = 30, formatted into: Device CALs needed : 30 (1 per device, marked individually - do NOT round to 5-packs) [SFT-109 for 2022 / SFT-111 for 2025].\n5. Senior tech pastes this into the quote workup. Client is over-quoted by 5 Device CALs (~$40 each list = ~$200) with no rounding cushion to absorb the error.\n6. With the fix (Enabled -eq $true -and OperatingSystem -notlike '*Server*'), the workstation count is 25 (correct).\n\nFix. One clause matching the sibling user query pattern:\n\npowershell\n$wsCount = (Get-ADComputer -Filter "Enabled -eq \$true -and OperatingSystem -notlike '*Server*'" | Measure-Object).Count\n

} catch { $lines.Add('AD counts : ActiveDirectory module unavailable - count users/devices at the DC') }
} else {
$lines.Add('Not a DC - pull enabled user + workstation counts from the domain DC to size CALs.')
}

# ----- DECISION INPUTS SUMMARY -----
$lines.Add('')
$lines.Add('== DECISION INPUTS -> KB 3801 Section 4 gates (senior tech makes the call) ==')
$lines.Add(("Gate 1 OS ceiling : model {0} ({1}) -> ceiling per KB 3801 matrix vs dental-stack OS support" -f $cs.Model, $genText))
$lines.Add(("Gate 2 current OS : {0}" -f $os.Caption))
$lines.Add(("Gate 3 hardware : warranty -> look up tag {0} at dell.com/support" -f $bios.SerialNumber))
$lines.Add(("Gate 4 capacity : {0} GB RAM, {1} cores (max {2} GB per firmware)" -f $ramGB, $cores, $maxRamGB))

@claude claude Bot Jul 6, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 The Gate 4 capacity summary line has no guard when $maxRamGB is null, producing Gate 4 capacity : 128 GB RAM, 16 cores (max GB per firmware) with a visible blank slot. The IDENTITY block at line 74 already guards its RAM-max line with if ($maxRamGB) — the summary should match by either defaulting to a marker (e.g. 'unknown') or gating the parenthetical clause. Rare on modern Dell firmware but reachable on virtualized/edge hosts where both MaxCapacityEx and MaxCapacity are 0/null.

Extended reasoning...

What the bug is. At oem-dell/get-server-lifecycle-audit.ps1:63-65, $maxRamGB is initialized to $null and only assigned when either $memArray.MaxCapacityEx or $memArray.MaxCapacity is truthy. The IDENTITY block at line 74 correctly guards its detail line with if ($maxRamGB) { $lines.Add((...)) }, so when firmware doesn't report the ceiling, that line is simply omitted. But the Gate 4 summary at line 246 has no such guard:

$lines.Add(("Gate 4 capacity     : {0} GB RAM, {1} cores (max {2} GB per firmware)" -f $ramGB, $cores, $maxRamGB))

PowerShell's -f operator renders $null as an empty string (no exception, no placeholder), so the line becomes literally:

Gate 4 capacity     : 128 GB RAM, 16 cores (max  GB per firmware)

with a visible double-space gap between max and GB.

Trigger conditions. Win32_PhysicalMemoryArray.MaxCapacityEx (UInt64) is the modern property; MaxCapacity (UInt32, KB) is the legacy fallback. On the T130/T230/T330/T340/T440 family the collector explicitly targets, most Dell firmwares populate at least one. But older BIOS revisions, some virtualized hosts, and cases where Get-CimInstance ... | Select-Object -First 1 returns $null all leave both properties null/zero, at which point the elseif chain leaves $maxRamGB at its $null initial value.

Why existing code doesn't prevent this. The inconsistency is the strongest evidence: the author knew about the null case (they added the if ($maxRamGB) guard at line 74) and simply forgot to apply the same guard at line 246.

Step-by-step proof.

  1. Get-CimInstance -ClassName Win32_PhysicalMemoryArray | Select-Object -First 1 returns an object where both .MaxCapacityEx and .MaxCapacity are $null or 0.
  2. Line 63: $maxRamGB = $null.
  3. Line 64: if ($memArray.MaxCapacityEx) — evaluates falsy, branch skipped.
  4. Line 65: elseif ($memArray.MaxCapacity) — evaluates falsy, branch skipped.
  5. $maxRamGB remains $null.
  6. Line 74: if ($maxRamGB) { ... } — falsy, RAM max detail line is (correctly) suppressed.
  7. Line 246: "... (max {2} GB per firmware)" -f $ramGB, $cores, $maxRamGB — $null interpolates as empty string, producing (max GB per firmware) with the double-space gap.

Impact. Cosmetic. A senior tech reading the paste-ready report sees a blank slot between max and GB and understands it as "not reported", not as zero — and the RAM max detail line above is already suppressed in the same run, so context is consistent. No wrong data ships; no exception is thrown. But this is a visible defect in a report the PR description explicitly frames as "paste-ready", and it violates the pattern the author established at line 74.

How to fix. One-line change to match the line-74 pattern. Either default $maxRamGB to a marker before the summary is built:

if (-not $maxRamGB) { $maxRamGB = 'verify' }  # or 'unknown'

or gate the whole parenthetical clause so it drops out when unknown:

$capLine = "Gate 4 capacity     : {0} GB RAM, {1} cores" -f $ramGB, $cores
if ($maxRamGB) { $capLine += (" (max {0} GB per firmware)" -f $maxRamGB) }
$lines.Add($capLine)

The 'verify' marker is a good fit — the report already uses verify elsewhere (generation/iDRAC gen) as the "look this up out-of-band" convention.

$lines.Add(("Upgrade gate (2022) : Boot={0}; iDRAC license={1} (Enterprise/Datacenter needed for remote recovery)" -f $bootMode, $idracLicense))
$lines.Add('========================================================================')

return ($lines -join "`r`n")
}

# ---- main (skipped when dot-sourced for Pester: '. .\get-server-lifecycle-audit.ps1') ----
if ($MyInvocation.InvocationName -ne '.') {
if (-not $ClientName) { $ClientName = $env:ClientName }
if (-not $ClientName -and [Environment]::UserInteractive) { $ClientName = Read-Host -Prompt 'Client name' }
if (-not $ClientName) { $ClientName = 'UNSPECIFIED' }

$logDir = 'C:\DTC\Logs'
$logPath = Join-Path -Path $logDir -ChildPath ("server-lifecycle-audit-{0}.log" -f (Get-Date -Format 'yyyyMMdd-HHmmss'))
New-Item -ItemType Directory -Path $logDir -Force | Out-Null
Start-Transcript -Path $logPath -Append | Out-Null

try {
Write-Output ("Auditing {0} for client '{1}'" -f $env:COMPUTERNAME, $ClientName)
$report = Get-DTCServerLifecycleAudit -ClientName $ClientName
Comment on lines +260 to +268

@claude claude Bot Jul 6, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 The script does not implement the CLAUDE.md three-section RMM template: no $env:RMM detection (uses [Environment]::UserInteractive instead), no RMM variable declaration block at the top, and $LogPath is hardcoded to C:\DTC\Logs\ rather than branching between $env:RMMScriptPath\logs\ (RMM) and $env:WINDIR\logs\ (interactive). The author already flagged this as Standards Deviation #1 in the PR description and offered to conform in a follow-up commit — this comment records the reviewer decision. Recommend conforming before merge to keep the library consistent, but the script is functionally correct and read-only, so this does not block merge on its own.

Extended reasoning...

What the deviation is

CLAUDE.md requires every script to follow a three-section structure derived from script-template-powershell.ps1: (1) an RMM variable declaration comment block, (2) an input-handling section that branches on $env:RMM -eq "1" to pick RMM vs interactive mode and sets $LogPath accordingly, and (3) the script logic wrapped in Start-Transcript/Stop-Transcript. This script's main block (oem-dell/get-server-lifecycle-audit.ps1:258-266) instead does:

if (-not $ClientName) { $ClientName = $env:ClientName }
if (-not $ClientName -and [Environment]::UserInteractive) { $ClientName = Read-Host -Prompt 'Client name' }
if (-not $ClientName) { $ClientName = 'UNSPECIFIED' }

$logDir = 'C:\DTC\Logs'
$logPath = Join-Path -Path $logDir -ChildPath ("server-lifecycle-audit-{0}.log" -f (Get-Date -Format 'yyyyMMdd-HHmmss'))

That is a valid pattern in isolation, but it diverges from the repo standard on three specific points: (a) it uses [Environment]::UserInteractive instead of $env:RMM -eq "1" as the mode discriminator, (b) it has no comment block enumerating expected RMM variables ($env:ClientName, and by convention $env:Description), and (c) it hardcodes the log directory instead of resolving to $env:RMMScriptPath\logs\ (RMM) or $env:WINDIR\logs\ (interactive/SYSTEM).

Why this is a nit, not a blocker

The PR description explicitly calls this out under 'Standards deviations to flag #1' and asks the reviewer for a decision — the author is not unaware, they are asking. The script is read-only (an audit collector; no writes to the target system), works correctly in both interactive and RMM contexts via the parameter → $env:ClientName → Read-Host → 'UNSPECIFIED' fallback chain, and the log-path deviation only affects where the transcript lands, not functional behavior. On NinjaOne this will run as SYSTEM and C:\DTC\Logs\ is writable, so nothing breaks.

Step-by-step: what the deviation actually costs

  1. A future maintainer copies this file as a starting template because it is in oem-dell/ and looks canonical — they now propagate the non-standard pattern.
  2. Anyone grepping for $env:RMM to enumerate RMM-mode scripts (e.g. for audit tooling) will miss this one.
  3. The transcript log lands in C:\DTC\Logs\ on every host, which does not match the log-collection paths used by the rest of the library ($env:WINDIR\logs\ or $env:RMMScriptPath\logs\), so centralized log-scraping will need a special case.

None of these are a functional break — they are consistency costs paid over time.

Recommended fix

Conform in a follow-up commit as the author offered. Concretely: add the ## $env:ClientName (and ## $env:Description) comment block at the top; replace the [Environment]::UserInteractive check with the standard if ($env:RMM -eq "1") { ... } else { ... } shape; and set $LogPath to $env:RMMScriptPath\logs\ (fallback $env:WINDIR\logs\) in RMM mode, $env:WINDIR\logs\ in interactive mode. The existing Get-DTCServerLifecycleAudit function itself does not need to change.

Write-Output $report
exit 0
} catch {
Write-Error ("FAILED: {0}" -f $_)
Write-Error $_.ScriptStackTrace
exit 1
} finally {
Stop-Transcript | Out-Null
}
Comment on lines +271 to +277

@claude claude Bot Jul 6, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 The catch block at lines 269-272 has dead code: with $ErrorActionPreference = 'Stop' set at line 34, the first Write-Error ("FAILED: {0}" -f $_) at line 270 promotes to a terminating error and propagates out immediately, so Write-Error $_.ScriptStackTrace at line 271 and exit 1 at line 272 never execute. The finally block still runs and the uncaught error still yields a non-zero exit, so the exit contract is satisfied by accident — but the stack trace, the actual useful artifact for debugging a remote audit failure, is silently dropped from both stdout and the transcript. Fold both messages into a single Write-Error, or add -ErrorAction Continue to the first call.

Extended reasoning...

What the bug is. $ErrorActionPreference = 'Stop' is set at script scope on line 34. Under Stop, PowerShell promotes any Write-Error (a non-terminating error record via PSCmdlet.WriteError()) into a terminating error — this is documented PowerShell 3.0+ behavior and applies everywhere in the script, including inside catch blocks. So in the main-block catch at lines 269-275:\n\npowershell\n} catch {\n Write-Error ("FAILED: {0}" -f $_) # <-- terminates here\n Write-Error $_.ScriptStackTrace # dead code\n exit 1 # dead code\n} finally {\n Stop-Transcript | Out-Null\n}\n\n\nThe specific code path that triggers it. Any exception raised inside the try block (line 264: $report = Get-DTCServerLifecycleAudit ...) — e.g. a CIM query failure, Confirm-SecureBootUEFI throwing on an unusual host, or a registry read failing — routes to this catch. The very first statement in the catch is a Write-Error, which under Stop throws immediately.\n\nWhy the existing code doesn't prevent it. There is no inner try/catch around the Write-Error calls, and neither call passes -ErrorAction Continue to opt out of the preference. The line-34 preference applies uniformly.\n\nImpact. Low but real, and exactly opposite the reader's expectation. The FAILED message still lands in the transcript before the throw, and the finally block still runs Stop-Transcript, so the transcript closes cleanly and the process exits non-zero by way of the uncaught exception. What is lost is the ScriptStackTrace — the single most useful diagnostic when this collector fails on a remote NinjaOne target the tech cannot log into to reproduce. exit 1 being skipped is cosmetic (same non-zero exit either way).\n\nStep-by-step proof. Simulate the failure path with any exception in the try:\n1. Get-DTCServerLifecycleAudit throws. Control transfers to the catch.\n2. Line 270 evaluates Write-Error ("FAILED: {0}" -f $_). Write-Error calls PSCmdlet.WriteError() which emits the error record to $Error and stdout.\n3. Under $ErrorActionPreference = 'Stop', the runtime immediately promotes that non-terminating record to a terminating error and throws.\n4. The throw originates inside the catch, so the exception does not re-enter the same catch — it unwinds toward the enclosing scope (script scope).\n5. Statement at line 271 (Write-Error $_.ScriptStackTrace) is skipped. Statement at line 272 (exit 1) is skipped.\n6. The finally block runs on unwind — Stop-Transcript | Out-Null executes, transcript is closed.\n7. The unhandled terminating error propagates out of script scope. The PowerShell host writes the error to stderr and sets a non-zero exit code.\n\nReproducer:\npowershell\n$ErrorActionPreference = 'Stop'\ntry { throw 'boom' } catch {\n Write-Error 'first'\n Write-Error 'second' # never runs\n 'after' # never runs\n}\n\nOnly first is written; the RemoteException propagates.\n\nHow to fix. One line, three viable shapes:\n\n1. Fold both into one Write-Error (simplest, preserves single error record):\npowershell\nWrite-Error ("FAILED: {0}`n{1}" -f $_, $_.ScriptStackTrace)\n\n2. Opt the first call out of the preference:\npowershell\nWrite-Error ("FAILED: {0}" -f $_) -ErrorAction Continue\nWrite-Error $_.ScriptStackTrace -ErrorAction Continue\nexit 1\n\n3. Use Write-Host/Write-Warning for the trace (also written to the transcript, not subject to Stop):\npowershell\nWrite-Warning $_.ScriptStackTrace\n\n\nAny of these keeps the stack trace on stdout and in the transcript. Recommend option 1 — smallest diff, single error record for consumers.

}
48 changes: 48 additions & 0 deletions oem-dell/tests/get-server-lifecycle-audit.Tests.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
#requires -Modules @{ ModuleName = 'Pester'; ModuleVersion = '5.0.0' }
<#
Pester tests for get-server-lifecycle-audit.ps1
Repo path: oem-dell/tests/get-server-lifecycle-audit.Tests.ps1
Structural validation only - parses the script via the AST and checks the
shippable-script invariants. Does NOT execute the collection logic or mock
CIM/CDXML cmdlets (Get-PhysicalDisk/Get-Tpm/Confirm-SecureBootUEFI are not
cleanly mockable in Pester 5; live integration is validated on a real server).
#>

BeforeAll {
$script:ScriptPath = (Resolve-Path (Join-Path $PSScriptRoot '..\get-server-lifecycle-audit.ps1')).Path
$script:Raw = Get-Content -Path $script:ScriptPath -Raw
$parseErrors = $null
$script:Ast = [System.Management.Automation.Language.Parser]::ParseFile($script:ScriptPath, [ref]$null, [ref]$parseErrors)
$script:ParseErrors = $parseErrors
}

Describe 'get-server-lifecycle-audit shippable-script invariants' {

It 'parses with no syntax errors' {
$script:ParseErrors | Should -BeNullOrEmpty
}

It 'has a comment-based help SYNOPSIS' {
$script:Raw | Should -Match '\.SYNOPSIS'
}

It 'sets $ErrorActionPreference to Stop' {
$script:Raw | Should -Match "ErrorActionPreference\s*=\s*'Stop'"
}

It 'declares no mandatory parameters (would hang an unattended RMM run)' {
# match the attribute form only, not prose mentions of "mandatory"
$script:Raw | Should -Not -Match '\[Parameter\([^)]*Mandatory'
}

It 'defines the Get-DTCServerLifecycleAudit function' {
$fn = $script:Ast.FindAll(
{ param($n) $n -is [System.Management.Automation.Language.FunctionDefinitionAst] -and $n.Name -eq 'Get-DTCServerLifecycleAudit' },
$true)
$fn | Should -Not -BeNullOrEmpty
}

It 'guards the main block so dot-sourcing for tests does not execute it' {
$script:Raw | Should -Match "MyInvocation\.InvocationName -ne '\.'"
}
}