Audit pipeline selector binding across the module, plus a regression rail (#90) - #116
Merged
juemerson-at-purestorage merged 17 commits intoAug 16, 2026
Conversation
…nventory The candidate predicate for the issue dmann000#90 pipeline-selector audit needs to tell a query selector from a request-body property. Get-PfbWireNameForParameter already resolves this -- its TargetVariable is literally 'queryParams' or 'body' -- but the inventory discarded it. Additive only: Build-PfbFieldCmdletMap.ps1 projects a fixed field set, so Reports/PfbFieldCmdletMap.json and Reports/PfbFieldCmdletMapping.md regenerate byte-identical.
…lector audit Reflection rather than AST: reflection is what the engine binds on, and this module uses the bare attribute form (ValueFromPipeline with no '= \True'), which a naive regex misses in all 544 files. The 303/214 assertion is a tripwire, not a vanity count -- if the population moves, either the module changed or the walk is wrong, and both must be noticed.
…ples Two independent producer sources. Family resolution alone misses cross-family chains the module advertises in its own help -- Get-PfbFileSystem | New-PfbFileSystemSnapshot is exactly such a case. Primary and family-wide producers are reported separately: 'fails every producer in its family' and 'fails its primary producer' are different signals and neither is collapsed into the other.
The predicate decides nothing about defects -- it selects what the harness puts on trial, and records a Gate value saying why each pair was excluded so the control metrics are auditable rather than a bare count. The selector-hood gate is load-bearing: measured across the module, the scalar-type filter alone takes 303 pipeline-bound pairs to 302. A vacuous predicate is the dmann000#89 failure mode, so a candidate rate at or above 0.75 fails the build.
Types matter for probe fidelity. An all-string probe lets a selector bind cleanly by property name to a field like array-connections' 'remote' that is really an object, concealing the stringification this audit exists to find. Two edition/spec traps found while implementing: - Get-PfbSchemaPropertyDetails deliberately refuses to follow a property's own \ (its PIN rule, so a referenced schema's readOnly cannot leak onto the referrer), so a \'d object property reports no Type. Defaulting that to 'string' would rebuild the all-string probe. This reads the raw property nodes and resolves one level FOR TYPE ONLY -- no readOnly/deprecated/required semantics -- via the same walker. - ConvertFrom-Json gained -Depth in PowerShell 6.2, so passing it unconditionally broke the whole Describe on Windows PowerShell 5.1. Measured: 5.1 parses the 2.04 MB fb2.28.json correctly without it, so the edition guard keeps the sweep runnable on both rather than forcing a #Requires -Version 7.0.
Every behavioural claim in the dmann000#90 audit comes from piping a probe into the real imported cmdlet with the HTTP layer shadowed inside module scope -- no hand-written stand-ins, which is the direct lesson of dmann000#89. The shadowing has a silent failure mode: '& \ { function Invoke-PfbApiRequest {...} }' defines the function in the script block's own scope, which is discarded on exit, so the module keeps calling the real one and opens a socket. Measured during design: a probe reached 'No such host is known. (fb.example.test:443)'. With a real array in scope, the same fall-through on a Remove-Pfb* probe is a live DELETE. So the shim is bound with Set-Item function:script: and VERIFIED live before Initialize-PfbSelectorHarness returns; a harness that cannot prove its own isolation produces no output. The guard was confirmed to fire by deliberately installing the broken form.
Findings are Coerced and WrongScalar only. Guarded is dmann000#64's fix working. NoSelector is a reported observation, not a finding: if any selector is pipeline-bound then a non-matching object falls through to pass 3 and coerces, so NoSelector can only arise where the cmdlet never claimed to accept a chain at all. Bound-vs-WrongScalar is decided by which PROPERTY a value came from, never by which wire key it landed on -- a sentinel from an unrelated property arriving on the expected key is exactly the WrongScalar defect, so a key match must not license a Bound verdict. Aliases are the legitimate reason a source property may differ from the parameter name (-RemoteName carries the alias Name), so they are passed explicitly via -Alias.
… ordinally Three defects found by running the sweep rather than by reading the code. 1. The harness could BLOCK ON A CONSOLE PROMPT. An unbound [Parameter(Mandatory)] throws in a non-interactive host but prompts in a real console, and that prompt renders on the user's desktop where the sweep can neither see nor answer it. Measured: 16 of 286 pipeline-bound cmdlets, e.g. Update-PfbSmbShareRule, whose mandatory -Attributes no probe object can ever supply. Test-PfbSelectorProbeBindable now proves a parameter set is satisfiable before the cmdlet is invoked at all, and reports the refusal as a BindError. 2. Outcome classification read whichever query key enumerated first instead of the row's own wire key, so a row claimed another parameter's correct binding as its own defect -- Get-PfbArrayConnectionPath's -RemoteName was reported WrongScalar on evidence 'ids=PROBE-id', which is -Id binding exactly as it should while remote_names was never emitted. It also re-counted one coercion once per pipeline-bound parameter, and depended on hashtable enumeration order. Verdicts now come from the row's own key. WrongScalar drops from 16 to 0; all 16 were this artifact. 3. Sort-Object -Culture '' is invariant LINGUISTIC comparison and the two gated editions disagree on it: 5.1 (.NET Framework) ignores the hyphen in 'file-system-snapshots', 7 (.NET Core/ICU) does not, while StringComparer.Ordinal returns the same -70 on both. It moved 10 rows between editions with every verdict identical -- the same churn class issue dmann000#85 fixed. Everything this analysis layer emits is now ordered ordinally. Cross-edition sweep now agrees on all 1179 rows with zero verdict mismatches.
…7 only This is developer- and CI-side tooling -- run once by a developer, or by the CI automation, which is pwsh 7 on ubuntu. It never needs to run under Windows PowerShell 5.1, and all five existing tools/Build-*.ps1 generators already carry the same #Requires -Version 7.0. Drops the edition guard around ConvertFrom-Json -Depth, which existed only to keep the sweep 5.1-runnable. Test Describes take the repo's usual -Skip:(\System.Management.Automation.PSVersionHashTable.PSVersion.Major -lt 7). The file-level BeforeAll guards its own body as well, because a skipped Describe does not stop a file-level BeforeAll from running and dot-sourcing a 7-only script under 5.1 kills the whole container. The SHIPPED module still supports 5.1 and its own tests still gate on both editions. Pipeline binding is engine behaviour, so that question was answered before this change: the full sweep was run under both engines and all 1179 rows agreed on gate, outcome, evidence and order.
The design justified the selector-hood gate on the grounds that it keeps request-body properties out of the candidate set, and that without it the rate inflates into dmann000#89's vacuous range. Measured against the real module, that is falsified: the gate excluded 0 rows, because NO pipeline-bound parameter here targets the body -- 291 of 303 pairs resolve to Query and the other 12 do not resolve at all. The scalar gate also excluded 0. The surface gate's real contribution is the 34 rows / 11 pairs with no resolvable wire name. The discrimination from 1145 evaluated rows to 565 candidates is almost entirely the name/alias-versus-response-field comparison. Both gates stay -- as guards against a future body-bound or non-scalar pipeline parameter -- but the help now says what they measurably do rather than what they were predicted to do.
The probe harness refused to invoke any cmdlet declaring a mandatory parameter a probe object cannot carry, because an unbound mandatory parameter prompts and blocks in an interactive host. That left 33 of 303 pipeline-bound (cmdlet, parameter) pairs without a behavioural verdict, concentrated in the object-store access-policy, quota and *-Rule families where the confirmed defect clusters. Two changes close it. Split BindError into verdicts. InputObjectNotBound now classifies as Unbindable and a cmdlet throwing before the shim as CmdletError, read from the error record's type and FullyQualifiedErrorId rather than its culture-dependent message. Eight of the 33 already had a verdict that was being filed as a harness failure. Add New-PfbSelectorFillerArgument, which synthesises throwaway values for the mandatory parameters a probe cannot supply, so the cmdlet runs far enough to build a request. Fillers carry a FILLER- prefix that the outcome classifier never reads as evidence, and go only to parameters Test-PfbSelectorParameterSatisfied reports as unreachable by the probe -- now the single definition shared with Test-PfbSelectorProbeBindable, since the two lists disagreeing would break the safety argument silently. Per-parameter safety proved insufficient on its own. Diffing all 1179 rows with fillers on against fillers off found six perturbed verdicts: filling a mandatory parameter of a different parameter set made that set satisfiable, PowerShell selected it, and Update-PfbArrayConnection flipped from Bound to Unbindable -- a verdict manufactured rather than observed. A filler is now supplied only when no parameter set is satisfiable unaided. The diff reports zero perturbations. Coverage goes from 269 to 302 of 303 pairs. The findings do not move: Coerced stays at 389 rows and 127 distinct pairs. Of 152 assisted rows, none coerced -- those selectors are ValueFromPipelineByPropertyName-only and pass 3 coerces ByValue, so they cannot exhibit the defect. That falsifies the earlier inference that the unmeasured pairs would behave like their Get-* siblings and push the count toward 160. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Commits the issue dmann000#90 audit as a regenerable artifact: Reports/PfbPipelineSelectorMap.json and .md, produced by tools/Build-PfbPipelineSelectorMap.ps1. The generator imports the shipped module, shadows Invoke-PfbApiRequest inside module scope with a capture shim, pipes a schema-derived probe object into each real cmdlet, and records what reached the wire. It makes no network request and refuses to run if it cannot verify its own isolation. Every verdict is observed rather than inferred, which is the direct lesson of dmann000#89. Rows carry ProbeProperties and ProbeTypes so Rail A can rebuild a probe without reading the gitignored tools/specs cache, plus ErrorKind and FilledParameter so it can tell an unmeasured row from a measured one. Also fixes a silent no-op in Sort-PfbSelectorRecord. It used [array]::Sort($keys, $items, $comparer), which sorts the keys array in place and leaves the items array untouched, so the helper returned its input unsorted while its help claimed ordinal ordering. Nothing threw, and no test covered either sort helper, so every artifact it ordered was in natural iteration order. Caught only when the generated report was asserted against an independently computed ordinal ordering. Records are now sorted directly via List<T>.Sort with a [string]::CompareOrdinal comparison, with the original index as a tie-breaker because List<T>.Sort is unstable. Five tests cover both helpers. No verdict changed: 1179 probe rows, 565 candidates, 389 Coerced rows across 127 distinct (cmdlet, parameter) pairs, control leakage 2. Repeat runs are byte-identical. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Rail A re-probes every pair in the committed selector map and fails on any Coerced/WrongScalar outcome that is not waived, plus on a waiver whose pair no longer coerces. It reads the report, never tools/specs, so the gitignored spec cache cannot silently skip it. Waivers are keyed by (cmdlet, parameter) pair -- 127 entries for the 389 finding rows, which are producer multiplicity over 127 real defects -- and are grouped by the audit's four root-cause clusters with the producing endpoint and the wire key named in each Why line. Every entry cites dmann000#90; the fix issue does not exist yet. Verified the rail actually reds by removing one waiver, and asserts BindError stays at 2 rows so a harness that starts refusing cmdlets cannot pass as clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Rail B regenerates the map and compares both artifacts -- JSON and the Markdown a human actually reads -- against the committed ones. It skips when the gitignored tools/specs cache is absent and asserts the cache is complete at 29 files when present, so a partial cache cannot masquerade as drift. The spec count is computed at file scope because Pester evaluates -Skip: during discovery; a count set in a BeforeAll would still be $null and never skip. Verified by hiding tools/specs: Rail A runs and passes, Rail B skips. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Rail A waives by (cmdlet, parameter) pair, which was blind to WHERE a coercion happens. All 100 Family-scoped waivers have a primary-producer row that measures Bound today; if one flipped to Coerced -- the obvious chain, and 39 of them are Remove-* -- the existing waiver would wave it through and the drift rail would read as a stale report. Scope and Producers are now load-bearing: the rail fails on a Family waiver that escalates onto its primary producer, and on a pair whose producer count moves off the number the waiver was granted for. Both proved red before being made green. The rail also documented a ProbeTypes-degradation defence it never enforced. Forcing every probe property to string flips 2 of 389 findings out of Coerced, and pair-level waiving means all 127 pairs still look defective, so nothing else would have caught it; the count is now asserted per row. Markdown findings were ordered by Sort-Object on a single boolean with no tiebreaker -- unstable, so a runtime change could reshuffle 389 rows with every verdict identical and red the drift rail as drift. Now sorted ordinally and partitioned explicitly. JSON is unchanged; only the .md row order moved. Also: Assert-PfbConnection's isolation check was fail-open (a negative match on a message string in the real function), now positive-match on its own marker like the Invoke-PfbApiRequest half; spec counting matches the generator's fb*.json filter so a stray file cannot stand in for a missing spec; 44 waiver Why lines said "only the family chain X" where 2-9 endpoints coerce; and the BindError comment claimed ErrorKind is null everywhere when it is non-null on 193 rows. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
fix(test): make Rail B drift check independent of checkout line endings CI run 31830362870 failed the Windows pwsh leg on the Markdown hash while Linux and macOS passed on an identical report. The generator joins the Markdown with a hard "`n", but git checks the artifact out with the platform newline (core.autocrlf is true on the GitHub Windows runners), so a raw byte hash was measuring the checkout policy rather than drift. The JSON leg only passed by accident -- ConvertTo-Json emits CRLF on Windows, matching the CRLF checkout -- and would have failed the other way round for anyone with core.autocrlf=false, so both artifacts are now compared through a newline-normalised hash. Verified red-green: the raw hashes of a CRLF and an LF rendering of the committed report differ; the normalised hashes match. The helper lives in BeforeAll because a function defined at file scope runs during discovery and is not in scope when an It executes -- the file-scope form failed with "the term ... is not recognized". Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> @
test: raise the winps51 skip ceiling for the PS7-gated selector tests CI run 31830362870 failed the 5.1 coverage gate: 252 skipped against a ceiling of 206. The main run this branch forked from (31755862501) measured 192, and the +60 is exactly the three PS7-gated files this branch adds -- 33 in PfbPipelineSelectorTools.Tests.ps1, 16 in PfbSelectorProbeHarness.Tests.ps1, 11 in PfbPipelineSelectorRail.Tests.ps1. All three RUN on pwsh 7 (that leg still skips 2), so this is the edition gate working, not lost coverage. Build-PfbPipelineSelectorMap.Tests.ps1 reads the committed report rather than the spec cache and runs on both editions, adding no skips. 268 keeps the same +16 headroom over measured that the previous raise did. Also adds both rail Describes to the pwsh7 RequiredDescribes allowlist. Rail B is the reason that list exists: it skips gracefully when tools/specs is absent, contributing neither a pass nor a skip, which is the issue dmann000#63 shape a skip ceiling cannot see. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> @
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes step 1 and step 3 of #90. It contains no cmdlet fixes — see "Proposed scope split" below.
The problem
When an object is piped into a cmdlet, PowerShell resolves the binding in three passes. Pass 3 is
ByValue with coercion: if no parameter matched by value or by property name, PowerShell will
stringify the whole object into a
[string[]]selector. A caller who writescan end up sending
names=@{id=...; policy=}on the wire — the filter never matches, or matches thewrong record. On a
Remove-*cmdlet that is a DELETE aimed at something the user did not name.What was measured
tools/Build-PfbPipelineSelectorMap.ps1probes every pipeline-bound parameter in the module againstevery object shape that can reach it, and records how PowerShell actually bound it. Probe objects are
built from the published OpenAPI response schemas, so the field names and types are the spec's, not a
hand-written guess. Nothing touches a live array: the harness shadows the request function inside
module scope and verifies the shadow took effect before probing, refusing to run if it did not.
BindError)The 389 rows are producer multiplicity over 127 real defects — the same parameter reached from
several endpoints. Full report:
Reports/PfbPipelineSelectorMap.md(human) and.json(machine).CoercedandWrongScalarare the findings.UnbindableandCmdletErrorare verdicts, not blindspots — only
BindErrormeans unmeasured, and it stands at 2.Root causes (report §3.3)
selector never sees a name (16 pairs).
namefield at all (10 pairs).Get-PfbLocalGroupMember -Group(1 pair).family coerce (100 pairs).
The rail
Tests/PfbPipelineSelectorRail.Tests.ps1:Coerced/WrongScalar. It is are-measurement rail, not a snapshot diff — a real fix makes its waiver stale rather than
reddening CI, and a reintroduced coercion reds immediately. It never reads
tools/specs.Tests/Fixtures/PfbSelectorWaivers.psd1registers today's 127 findings, one entry per pair, each withan issue reference and a prose reason. Pair-level rather than triple-level because 389 entries sits
against psd1's hard 500-element parse cap and would list one defect a dozen times. Because that key
discards the producer, the rail separately asserts each waiver's
Scopeand producer count — withoutthat, a Family-scoped coercion escalating onto its primary producer (39 of those pairs are
Remove-*)would be absorbed by the existing waiver.
The waivers cite #90 because the fix issue does not exist yet; re-pointing them is a one-line
follow-up.
Proposed scope split (maintainer's call)
#90 asks for three things: audit, per-cmdlet fixes, regression rail. This PR delivers the audit and
the rail. I'd suggest the 127 fixes become their own issue, sized off the report's §3.4 dispositions —
several are API-behaviour questions rather than local fixes, and each needs live verification. #90's
own text supports this ("Step 1 is the valuable part and is read-only"), but the split is a proposal,
not a decision.
Limits
.EXAMPLEchains. A user can pipe anything intoanything; 127 is the total for the chains probed, not for every chain conceivable.
New-*/Update-*was evaluated and rejected with measurement — 230 of 241non-GET endpoints return a shape identical to their GET counterpart (report §5b).
response shapes should be compared against a live array; findings on those are marked provisional.
Testing
alters what any cmdlet sends, so there is no live-array behaviour to verify; the outstanding live
work is the probe-fidelity cross-check above, which is called out in the report rather than claimed.
Tests/coverage-baseline.psd1is raised 206 → 268. Measured 252 against 192on the main run this branched from; the +60 is exactly the three PS7-gated files added here, which
all run on 7. Both rail Describes are added to the pwsh7
RequiredDescribesallowlist, since Rail Bskips gracefully without the spec cache — the issue CI silently skips ~23% of the test suite, including the absolute-path regression guards #63 shape a skip ceiling cannot see.
🤖 Generated with Claude Code