Skip to content

fix: Get-* completeness gaps and README check on every develop PR - #100

Merged
mikemadeja merged 8 commits into
mainfrom
develop
Sep 22, 2026
Merged

mikemadeja merged 8 commits into
mainfrom
develop

Conversation

@mikemadeja

Copy link
Copy Markdown
Owner

Summary

Bundles three merged PRs into develop:

fix: Get-PiHoleInfoHost now returns Model and DMI fields (#97)

Was only mapping uname.* fields, silently dropping model and the entire dmi (bios/board/product/sys) section. Kept the output flat (matching Get-PiHoleDnsBlockingStatus's style) and added the missing fields.

fix: three Get-* functions with completeness/correctness gaps (#98)

  • Get-PiHoleStatsSummary: was missing UniqueDomains, Forwarded, Cached, Frequency, and the entire Clients/Gravity sections.
  • Get-PiHolePadd: missing Queries.QueryFrequency; System/Version sub-objects now properly PascalCase instead of leaking raw snake_case.
  • Get-PiHoleCurrentAuthSession: missing XForwardedFor/Cli fields; was silently filtering out the caller's own current session; its cleanup call was missing -IgnoreSsl, which was the actual root cause of an SSL warning observed earlier against a self-signed-cert server.

ci: check README command reference on every PR into develop (#99)

The README command-reference check previously only ran on develop->main promotion PRs. Now it runs on every PR into develop, catching drift where it's introduced instead of at release time.

Test plan

  • All three functions verified against a real Pi-hole server
  • Full Invoke-Pester -Path .\tests run — 77 passed, 0 failed
  • Invoke-ScriptAnalyzer -Path .\PiHoleShell -Recurse — no findings
  • Confirmed the new README-check gate fires and passes on PRs into develop

🤖 Generated with Claude Code

mikemadeja and others added 8 commits September 20, 2026 08:58
It previously only mapped uname.* fields (DomainName, Machine,
NodeName, Release, SysName, Version), silently dropping the API
response's model and dmi (bios/board/product/sys vendor info)
sections entirely - the same kind of completeness gap found in
Get-PiHoleConfig. Also fixed a latent bug where Write-Output was
called inside the foreach loop over $Response.host (a single object,
not a collection), and dropped the stray `break` in its catch block.

Kept the output flat (matching Get-PiHoleDnsBlockingStatus's simple
style) rather than nesting it like the Get-PiHoleConfig fix, adding
Model, BiosVendor, BoardName/Vendor/Version, ProductName/Family/Version,
and SysVendor as top-level properties alongside the existing uname
fields.

Verified against a real server; regenerated the README command
reference since Get-PiHoleInfoHost's placeholder docstring update
means it's no longer flagged as a work-in-progress function.

Co-Authored-By: Claude Sonnet 5 <[email protected]>
fix: Get-PiHoleInfoHost now returns Model and DMI fields
The formatted output only ever captured Total, Blocked,
PercentBlocked, Types, Status, and Replies. The API also returns
UniqueDomains, Forwarded, Cached, and Frequency alongside those (all
siblings under queries), plus two entire top-level sections -
Clients{Active,Total} and Gravity{DomainsBeingBlocked,LastUpdate} -
that were never mapped at all.

Also dropped the stray `break` in the catch block and the
$ObjectFinal += pattern in favor of a direct Write-Output, matching
the module's standard shape, and cleaned up a docstring artifact
where .PARAMETER Password had leftover RawOutput description text
pasted into it.

Verified against a real server - all previously-missing fields now
present with real values.

Co-Authored-By: Claude Sonnet 5 <[email protected]>
…names

Queries.QueryFrequency (queries.query_frequency in the raw response)
was never mapped. Separately, the System and Version sub-objects were
assigned directly from the raw response ($Response.system /
$Response.version), so their fields stayed snake_case instead of
PascalCase like every other property on this object - now run through
ConvertTo-PiHolePascalCaseObject for consistency and to avoid missing
any of their fields by hand.

Also removed the dead $ObjectFinal/$Object = $null setup and the
array-wrapping pattern (Write-Output $Object directly instead), and
dropped the stray `break` in the catch block, matching the module's
standard shape.

Co-Authored-By: Claude Sonnet 5 <[email protected]>
…wn session, and leaked SSL errors

Three separate bugs found auditing this function:

1. Missing XForwardedFor and Cli fields from the response - never
   mapped.

2. The final pipeline silently filtered results to
   `Where-Object { $_.CurrentSession -match "False" }`, excluding
   the caller's own active session from a function whose entire
   purpose is "list of all current sessions." Removed the filter
   entirely; also the -match "False" against a boolean only ever
   worked by relying on stringification, not a real comparison.

3. The finally block's cleanup call
   (Remove-PiHoleCurrentAuthSession) never passed -IgnoreSsl, so it
   always attempted strict certificate validation regardless of the
   caller's setting - this is the exact cause of the
   "Failed to close Pi-hole session: The SSL connection could not be
   established" warning seen earlier this session against a
   self-signed-cert server.

Also moved the auth call and request params inside the try block
(they were outside it, unlike every other function in the module),
and removed a dead `$ObjectFinal = @()` reset after the output had
already been written.

Verified against a real server: the caller's own current session
(previously filtered out) now appears in the results, no SSL warning
during cleanup, and XForwardedFor/Cli are present on every session.

Co-Authored-By: Claude Sonnet 5 <[email protected]>
…gaps

fix: three Get-* functions with completeness/correctness gaps
Previously this only ran on develop->main promotion PRs, so drift
introduced by a feature branch wouldn't be caught until much later,
right before a release. Since develop now requires PRs for all
changes anyway, running the same -Check gate on every PR targeting
develop catches it right where it was introduced, before it merges
anywhere - matching how PSScriptAnalyzer already gates every PR.

Kept the main-branch trigger too, as a final safety net on promotion.

Co-Authored-By: Claude Sonnet 5 <[email protected]>
ci: check README command reference on every PR into develop
@mikemadeja
mikemadeja merged commit f6928dc into main Sep 22, 2026
5 of 6 checks passed
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