Skip to content

feat(machine-health): add the storage-top-n check - #8922

Merged
kyle-sexton merged 4 commits into
mainfrom
feat/machine-health-storage-top-n
Oct 21, 2026
Merged

kyle-sexton merged 4 commits into
mainfrom
feat/machine-health-storage-top-n

Conversation

@kyle-sexton

Copy link
Copy Markdown
Member

No related issue: Phase 4 of the owner-approved machine-health PC-checkup plan (ADR 0081, #8906; builds on #8917).

Summary

Adds storage-top-n, which answers "what's eating my storage". It lists the 15 largest folders under each reclaim location and the user profile as mutable FindingItems of kind folder, each with a reclaim hint.

The scan never downloads OneDrive cloud-only files. It never descends a reparse directory or a cloud sync root, and it never opens a file it enumerates. Each sync root it skips is listed as one item, "skipped: cloud sync root", with no size.

Fix

  • lib/Measure-FolderSize.ps1 walks each root with a manual directory stack.
    • AttributesToSkip covers Offline plus both RECALL bits, and never Hidden, System or ReparsePoint.
    • The never-descend rule is applied to the roots as well as to the folders inside them.
    • A second pass reads allocated size with GetCompressedFileSizeW. It never runs on a file that is itself a reparse point.
  • One 60 s budget, held in the budget object, covers both the scan and the allocated pass. Each root's top level is listed before any walk starts, and each root gets an equal share of the budget. A folder cut by the budget reports allocated_bytes = null.
  • checks/Test-StorageTopN.ps1 holds its numbers once (budget, top-N, file cap) and passes them to the adapter. The summary and command text interpolate them. It declares -MutedKeys.
  • Detail scalars: scan_complete, allocated_complete, inaccessible_count, skipped_reparse_dirs, skipped_sync_roots, items_count, muted_count.
  • Catalog entry storage-top-n (category storage, needs_admin: false) and check-catalog.md section 20. Section 20 states the OneDrive rule as a decision plus a pointer, with an as-of date and a recheck trigger.
  • Changelog fragment (minor).

Verification

  • run-tests.ps1 exits 0: 654 passed, 0 failed, 1 skipped. The Measure-FolderSize suite passes 22 tests and Test-StorageTopN 10. One test is unmocked and runs over a TestDrive tree.
  • Named cases:
    • a Hidden folder is counted;
    • a junction is not descended (skipped_reparse_dirs = 1);
    • a folder set as $env:OneDrive is never descended and is listed as one skipped item;
    • an access-denied folder is counted in inaccessible_count;
    • a zero budget gives scan_complete = false;
    • a budget cut inside the allocated pass leaves allocated_bytes = null;
    • a folder over the file cap gets allocated_bytes = null;
    • a root that is itself a junction is not walked.
  • The adapter contains no Get-Content, OpenRead, LinkTarget or ResolveLinkTarget, and the check contains no Stopwatch.
  • Hydration guard probe on a OneDrive host: the probe counted placeholder files with its own attribute read. The count was 942 before and 942 after, for both OneDrive and OneDriveCommercial, with skipped_sync_roots = 1. That means the refusal path ran and nothing was downloaded.
  • Host smoke (-DryRun) exits 0, and storage-top-n ran in elapsed_ms=40403, under the 75,000 ms ceiling against the 90 s dispatch timeout.
  • The code-design review found 4 CRITICAL, 2 IMPORTANT and 5 SUGGESTION findings. The four CRITICAL findings were numbers and rules written in two places, and the never-descend rule recoded for the roots. All of them, both IMPORTANTs and four suggestions are fixed in 5b71fb3.
  • A fresh-context phase verifier passed 9 of 9 criteria at 5b71fb3.

Related

The next checks (startup items, app last-use, Defender, intruder inventories, top processes, backup) follow the same FindingItem contract.

🤖 Generated with Claude Code

kyle-sexton and others added 3 commits October 21, 2026 15:41
Size the folders directly under the profile and the reclaim locations
and list the 15 largest per root as mutable folder items. The walk is a
manual directory stack that never descends a cloud sync root or another
reparse directory and never opens an enumerated file outside the
allocated-size pass; each skipped sync root is listed with no size. One
60 s budget in Measure-FolderSize covers the scan and the allocated
pass, shared equally across roots.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…place

Test-StorageTopN.ps1 now holds the budget, top-N and allocated file cap
and passes them to Measure-FolderSize, which no longer defaults them.
The skip and cloud-attribute masks are named values that the code, its
comments and the check's commands read. Each root and folder record
carries the caller's root string as `input`, so the check joins reclaim
hints without re-normalizing paths. A root itself goes through
Test-FolderDescent, budget checks share one form, and Test-PathUnder is
the one path-prefix rule. The catalog names attributes only and cites
the SKILL.md per-check timeout. Adds a junction-root case and an
end-to-end check run over a TestDrive tree through a test-only
-RootPath override.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 21, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-21T19:57:46.500445Z 5b71fb3 PR opened
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5b71fb3b0f

ℹ️ 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".

@github-actions

github-actions Bot commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

claude-security-review reviewed 40ef6820ee618d22770d98f5769e8a0c5a560e9c: 0 finding(s), 0 as inline comments.

No security issues found in the incremental change since 5b71fb3 (5 files).

I read .claude-lane/incremental.diff and the surrounding Measure-FolderSize.ps1 walk code.

  • Placeholder files: Measure-FolderSize.ps1 now filters placeholder files per entry (Offline, RECALL_ON_OPEN, RECALL_ON_DATA_ACCESS) instead of through AttributesToSkip. The filter sits before the byte count and before Get-AllocatedSize in both the logical and allocated walks. Measure-FolderSize itself applies it before $listed.Bytes/$listed.Files. A placeholder is therefore never opened, so the scan cannot trigger a cloud recall.
  • Placeholder directories: Test-FolderDescent now refuses to descend into a directory with a placeholder attribute, as it already did for reparse points. That keeps the scan from following a hydrated or cloud-backed directory.
  • Reparse points and sync roots: The never-descend rules for reparse points and cloud sync roots are unchanged. Only the placeholder directory case was added.
  • Other changes: EnumerateFileSystemInfos is now called directly, with no $listing = @() branch. The @(foreach …) wrapper and the test fixtures (X:\Profile paths, new tests) change no trust boundary. The updated $commands string and the catalog text are documentation only.

No command, path or template injection, credential handling, or authorization surface is touched. The instruction-surface lens had nothing to check, because no standing instruction or guardrail was removed.

@github-actions

github-actions Bot commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

claude-review reviewed 40ef6820ee618d22770d98f5769e8a0c5a560e9c: 0 finding(s), 0 as inline comments.

No findings: the incremental changes since 5b71fb3 hold up on a read of the code. I did not run the Pester suite; the Windows-only tests and the OneDrive probe in the PR body are author-claimed and unverified by me.

Placeholder handling. The enumeration no longer skips placeholders. Instead a directory carrying Offline, RECALL_ON_OPEN or RECALL_ON_DATA_ACCESS is refused in Test-FolderDescent and counted in SkippedReparse. A placeholder file is skipped before $files++ in Invoke-FolderWalk (Measure-FolderSize.ps1:80) and in the root-level listing (Measure-FolderSize.ps1:337), so it is neither counted nor opened. The logical pass, the allocated pass and the root listing apply the same rule, and the never-descend rule still lives in one place. check-catalog.md, the command text in Test-StorageTopN.ps1 and the $PlaceholderSkipMask comment describe the new behavior consistently.

Files-only root. Wrapping the child walk in @(foreach …) (Measure-FolderSize.ps1:355) stops a root with no subfolders from piping $null through Sort-Object and producing a phantom folder entry. Test-StorageTopN.ps1 handles the empty item list: Get-UnmutedSeverity returns OK, and the summary falls back to "No folders sized". The two new tests ("returns no folder entries for a root that holds only files" and "reports OK … only files") cover this.

Removed budget pre-check. The check in the root listing is gone, so a spent budget now opens the directory before stopping. A spent budget with an empty root leaves Complete true, which is correct because nothing was missed. A non-empty root still breaks on its first entry and reports incomplete, so the zero-budget case is unchanged.

Tests. The new Offline directory and Offline file cases assert both the count and that Get-AllocatedSize is never called on the placeholder. The C:\Users\Example to X:\Profile swap is cosmetic.

…in storage-top-n

The enumeration no longer skips placeholder attributes, so a directory
carrying Offline or a RECALL bit reaches the never-descend rule and is
counted instead of vanishing; placeholder files are filtered per entry
and never sized. Each root's listing is consumed lazily so the budget
can cut it, and a root with no subfolders no longer adds a null folder
entry. Test paths use an X:\Profile placeholder.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kyle-sexton
kyle-sexton added this pull request to the merge queue Oct 21, 2026
Merged via the queue into main with commit 83a6407 Oct 21, 2026
27 checks passed
@kyle-sexton
kyle-sexton deleted the feat/machine-health-storage-top-n branch October 21, 2026 20:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant