Skip to content

refactor(machine-health): derive the elevation banner coverage from the catalog - #8910

Merged
kyle-sexton merged 2 commits into
mainfrom
refactor/machine-health-elevation-single-source
Oct 21, 2026
Merged

kyle-sexton merged 2 commits into
mainfrom
refactor/machine-health-elevation-single-source

Conversation

@kyle-sexton

Copy link
Copy Markdown
Member

No related issue: Phase 1 of the owner-approved machine-health PC-checkup plan (ADR 0081 landed in #8906).

Summary

The banner's "Covered non-elevated" list was a hardcoded copy of the catalog. It had drifted to 9 phrases against 17 enabled needs_admin: false checks. The prose table in elevation-matrix.md also copied Get-ElevationMatrix.ps1 and had drifted to 4 rows against 5. This change removes both copies, so Get-ElevationMatrix.ps1 and the catalog are the only sources before later phases add matrix rows.

Fix

  • New lib/Get-NonElevatedCoverage.ps1 derives the covered list from the run set (Windows, enabled, not deprecated) filtered on needs_admin: false, labeled through Format-CheckLabel.
  • Format-CheckLabel moves to lib/Format-CheckLabel.ps1. -CustomIds is now a mandatory parameter instead of a script-scope read.
  • The banner call moves after the catalog load. If the catalog fails to load, the covered section is left out. An overlay warning now prints before the banner rather than after.
  • elevation-matrix.md drops its prose table and its "update both files" and drift-check lines, and points to Get-ElevationMatrix.ps1 and Get-NonElevatedCoverage.
  • Changelog fragment (patch).

Dead-code search: every function under scripts/windows/lib/ was counted against scripts, SKILL.md, reference and skills/setup. Twenty functions had no caller outside their own file, but each has two or more uses inside it, so none is dead and nothing is deleted for being uncalled.

Verification

  • run-tests.ps1 exits 0: 545 passed, 0 failed, 1 skipped (baseline 536 + 9 new).
  • The filtered suites Get-NonElevatedCoverage, Format-CheckLabel and Write-ElevationBanner each list one file and exit 0.
  • Host smoke (-DryRun), with and without -SkipBanner, exits 0 and latest.json parses.
  • check-changelog-fragments.sh --check exits 0.
  • A code-design review at the phase boundary found 6 suggestions and no blocking findings; all 6 are fixed in the second commit. A fresh-context phase verifier passed 9 of 9 criteria at f12a8f8.

Related

Later phases add matrix rows for startup-items, app-last-use and backup-status against this single source.

🤖 Generated with Claude Code

kyle-sexton and others added 2 commits October 21, 2026 14:33
…he catalog

The banner's "Covered non-elevated" list was a hardcoded nine-line list that
had drifted behind the catalog. Get-NonElevatedCoverage now derives it from
the enabled needs_admin: false entries through Format-CheckLabel, which moves
to its own lib file so the banner and the report label checks the same way.
The banner call moves after the catalog load. The prose table in
elevation-matrix.md is removed in favor of Get-ElevationMatrix.ps1.

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

Get-NonElevatedCoverage filters only on needs_admin, since the caller passes
the run set; Format-CheckLabel requires -CustomIds so a call that omits it
fails instead of mislabeling custom checks. Its tests use literal cases. The
elevation-matrix doc, the banner docstring and the Test-Drivers comment cite
the helpers by name instead of restating them.

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

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 🔄 Running since 2026-10-21T18:48:31.298885Z f12a8f8 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.

@github-actions

Copy link
Copy Markdown
Contributor

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

No security issues found in PR #8910 at head f12a8f8.

The PR is a refactor in the machine-health audit skill:

  • The pre-run elevation banner now reads its "Covered non-elevated" list from the loaded catalog (Get-NonElevatedCoverage).
  • Format-CheckLabel is moved into its own lib file.
  • elevation-matrix.md loses its duplicate table.
  • Tests and a changelog fragment are added.

What I checked:

  • Data flow: check ids from the machine-local overlay are only string-compared and formatted into display labels. They are written to stderr, report markdown and the summary table. They reach no command execution, path construction, deserialization or network call. The -CustomIds check is a string -contains.
  • Trust boundary: the banner call moved after the catalog load, and a failed load degrades to an empty section inside the existing try/catch. That changes no authorization, privilege or elevation logic. Which checks run elevated, and needs_admin, are unchanged.
  • Instruction surface: the deleted prose is an elevation-matrix.md table that the code already reproduces through Get-ElevationMatrix. No guardrail, hook block list, allowlist or standing agent instruction is removed or softened. I did not need the instruction exception register.
  • Tests: they use an in-process runspace with fixed strings and no untrusted input.
  • Actions and workflows: the diff touches none.

@github-actions

Copy link
Copy Markdown
Contributor

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

No findings. I read the full diff and the surrounding orchestrator code. I did not run the Pester suites or the host smoke test, so the PR's verification counts (545 passed, 0 failed) are the author's claims and unverified by me.

  • Banner ordering: $customIds, $windowsChecks and $elevated are all defined before the moved banner call (Invoke-MachineHealthCheck.ps1:168, :220, :271). On a catalog load failure the catalog becomes an empty checks array, so the coverage list is empty and the banner omits that section, as the code comment says.
  • Format-CheckLabel callers: the three call sites in the orchestrator all pass -CustomIds $customIds, which is now mandatory. A grep across plugins/machine-health found no other callers.
  • Get-NonElevatedCoverage: it dot-sources Format-CheckLabel.ps1 from $PSScriptRoot, and the orchestrator dot-sources both files. Entries without needs_admin are excluded, because $null -eq $false is false. The filter matches the earlier behaviour, where only entries with needs_admin: false were listed.
  • Tests: the suites load tests/helpers/Initialize-CheckSuite.ps1 via ..\..\helpers, and that helper defines $script:LibRoot, which the -CustomIds missing-parameter test uses. The new cases cover ordering, the [custom] suffix, the empty catalog, the banner coverage section and its omission when empty.
  • Docs and changelog: elevation-matrix.md now points to Get-ElevationMatrix.ps1, the Test-Drivers.ps1 comment is updated to match, and a patch changelog fragment is included.
  • PR body: it states No related issue with a reason, so there is no closing issue to check criteria against. The PR context contained no embedded instructions.

@kyle-sexton
kyle-sexton added this pull request to the merge queue Oct 21, 2026
Merged via the queue into main with commit 0b9fc7a Oct 21, 2026
30 checks passed
@kyle-sexton
kyle-sexton deleted the refactor/machine-health-elevation-single-source branch October 21, 2026 18:57
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