Updated chkdsk script for better logging and error handling TOOLING…

[anmocanu]

… 59932

Refactored the script to improve logging and error handling. Ensured that the return statement is outside the try/catch/finally blocks. https://dev.azure.com/Azure-VM-POD/Verticals/_workitems/edit/59932

[anmocanu]

This PR also addresses #68

[EdwinBernal1]

Issues Found

🔴 Bug: else block misplaced — Hyper-V skip message outside the if check

if ($hyperVModuleAvailable -and (Get-Command -Name 'Get-VM' ...)) {
    # ... VM handling ...
}
else {
    Log-Info "Hyper-V module/cmdlets not available on this host -> skipping nested VM discovery"
}

This else is paired with the Hyper-V check, but it's immediately followed by the partition enumeration code outside any block scope. The structure is technically correct — the else just logs and continues — but visually it looks like the partition logic might be inside the else. This is fine functionally, but adding a blank line or comment separator after the else block would improve readability.

Severity: Not a bug, just readability. Low.

🟡 Concern: No check for chkdsk exit code

$chkdskResults = chkdsk $letter /f 2>&1

After running chkdsk, the script parses the output text for summary lines but doesn't check $LASTEXITCODE. chkdsk returns specific exit codes:

• 0 = no errors found
• 1 = errors found and fixed
• 2 = disk cleanup performed
• 3 = could not check the disk

The script should check $LASTEXITCODE and set $script_final_status = $STATUS_ERROR if chkdsk reports unfixable corruption (exit code 3).

🟡 Concern: $letter colon handling

$letter = $partition.DriveLetter
if ($letter -notmatch ":") { $letter = "$letter" + ":" }

Get-Disk-Partitions returns DriveLetter as a single character (e.g., C), so this always appends :. The -notmatch ":" check is defensive but may be unnecessary complexity. Consider just always formatting: $letter = "$($partition.DriveLetter):".

🟢 Minor: Summary regex may miss important lines

if ($str -match '(no problems|correcting|replacing|deleting|recovering|inserting|truncating|adjusting|resetting|Windows has|No further action|Cleaning up|could not fix|Errors detected|corrupt|found no)') {

This regex covers many cases but could miss localized Windows installations where chkdsk output is in a different language. This is a known limitation and not unique to this PR, but worth documenting.

🟢 Minor: Add-Content for log vs Tee-Object pattern

The script uses Add-Content -Path $logFile for chkdsk output but the rest of the repo's scripts use Tee-Object -FilePath $logFile -Append. This inconsistency is understandable (separating stdout from log-only content is intentional) but could confuse future maintainers. Consider a brief inline comment explaining why Add-Content is used here instead of Tee-Object.

---

[EdwinBernal1]

@anmocanu could you help us with the review of these findings

Findings

Critical

File	Context	Issue	Recommendation
—	—	None	—

Warning

File	Context	Issue	Recommendation
win-chkdsk-fs-corruption.ps1	init sourcing	Convention shifted from . .\src\windows\common\setup\init.ps1 (CWD-relative) to $PSScriptRoot\common\setup\init.ps1. Deviates from the reference win-hello-world.ps1 / doc/adding_new_scripts.md.	Confirm this is the agreed new convention; update reference/docs repo-wide (tracked in the batch README).
win-chkdsk-fs-corruption.ps1	logging	Logs to Desktop/TEMP only, not the CustomScriptExtension plugin dir that az vm repair run auto-collects.	Consider dual-writing (as #127 does) so logs are harvested automatically.

Info

File	Context	Suggestion
win-chkdsk-fs-corruption.ps1	unlettered partitions	v2 helper returns letter-less partitions as [char]0; script correctly skips them (if (-not ($partition -and $partition.DriveLetter))) — chkdsk/fsutil need a drive letter. Good.
win-chkdsk-fs-corruption.ps1	return 1 before init	Returning 1 when init.ps1 is missing (before $STATUS_ERROR is defined) is acceptable since status codes originate from init.ps1.

Operational Risk Assessment

Factor	Rating	Notes
Scope	Low	Single script; no helper/init changes.
Destructive ops	Low-Medium	chkdsk /f alters the target file system — but that is the script's purpose and it is idempotent/standard.
Rollback possible	N/A	chkdsk repairs are not "undoable" but are the intended remediation.
Testing documented	Unknown	No test evidence quoted in the diff; confirm in the PR description.
Gen compatibility	Gen1+Gen2	Operates on drive letters of attached partitions; both generations covered.