Fix P1 cmdlet correctness issues (#102, #88, #55) - #115
Merged
juemerson-at-purestorage merged 34 commits intoAug 14, 2026
Merged
Conversation
Remove the public switch from 26 Get-Pfb cmdlets whose endpoints do not declare total_only. Unsupported keys were silently ignored and returned full item lists, so retaining the parameter exposed behavior the API cannot honor. The 12 cmdlets targeting endpoints that declare total_only remain unchanged. This is pre-existing behavior, not a regression from the common query helper. Co-Authored-By: Claude <noreply@anthropic.com>
Add documented ids and remote_ids filters across array-connection reads and writes while preserving no-selector list calls and legal ids plus remote_ids composition. Reject remote_names plus remote_ids, which the API declares mutually exclusive. Keep remote query assembly endpoint-specific and preserve the whole-object coercion guard for objects without bindable selector properties. Co-Authored-By: Claude <noreply@anthropic.com>
Cover standalone remote IDs, legal ID composition, mutually exclusive remote name and remote ID filters, no-selector reads, exact plural wire keys, and whole-object coercion protection in both PowerShell editions. Co-Authored-By: Claude <noreply@anthropic.com>
…00#88) Fix round 1. Assert the Mandatory half of the write parameter-set topology so an optional selector cannot silently allow a no-selector DELETE or PATCH, add the observable selector-less rejection tests, and pair the Remove and Get coercion rejections with exact zero-call assertions. Co-Authored-By: Claude <noreply@anthropic.com>
Add only the documented ids, remote_ids, and remote_names filters missing from replica-link, transfer, and snapshot-transfer cmdlets. Preserve composite creation/removal identities and reject mutually exclusive remote selectors. Co-Authored-By: Claude <noreply@anthropic.com>
…mann000#88) State only that -Id sends the endpoint's published ids query parameter, since the spec does not document what ids means on a create. Note that the snapshot-transfer remote selectors qualify an identity rather than supplying one. Co-Authored-By: Claude <noreply@anthropic.com>
Cover exact ids, remote_ids, and remote_names serialization, omission, mutual exclusion, and composite selector requirements across replica-link and snapshot-transfer cmdlets. Also widens the PfbApiDriftTools top-10 aggregation ceiling from 0.55 to 0.60. Closing a real parameter gap shrinks the denominator, so the top-10 share rises mechanically with every genuine dmann000#88 fix (54.84% -> 55.24%); the bound is documented in-file as institutional memory, not a bit-for-bit pin. Co-Authored-By: Claude <noreply@anthropic.com>
Remove four undeclared MemberName filters that were ignored by their endpoints, retain member ID selection, and add documented remote ID/name filters across replica-link policy membership cmdlets. This intentionally removes unsupported public parameters so narrow-looking calls fail at binding rather than silently widening their scope. Co-Authored-By: Claude <noreply@anthropic.com>
…mann000#88) Expose the declared remote_names and remote_ids filters on Get-PfbPolicyAllMember, which issue dmann000#88 lists and the spec declares from 2.2, so the whole five-cmdlet membership set now covers the same documented remote dimension. Rewrite the help on all five cmdlets as present-tense endpoint rules rather than change-history narrative, and drop the claim that a remote selector makes a removal conditional. Remote selectors are described only as declared query parameters; how the array treats a non-matching qualifier is unverified and the delete paths cannot be live-tested. Co-Authored-By: Claude <noreply@anthropic.com>
Pin exact policy membership wire keys, fail-closed MemberName removal, composite selector exceptions, and standalone reachability across the 18 cmdlets in the issue scope. Co-Authored-By: Claude <noreply@anthropic.com>
…000#88) Classify New-PfbFileSystemReplicaLink -Id, which the table omitted, and add a completeness assertion that every published selector on the 18 in-scope cmdlets falls into exactly one reachability classification. Split binder refusals out from cmdlet guards: Remove-PfbFileSystemSnapshotTransfer refuses a lone remote qualifier through parameter binding, now asserted on the AmbiguousParameterSet ErrorId rather than a localizable engine message. Make the composite table load-bearing by asserting each declared companion is mandatory in a parameter set carrying its selector and that the companions together satisfy a complete set. Tighten the fail-closed guard patterns to the full guard sentences. Co-Authored-By: Claude <noreply@anthropic.com>
Enumerate every Get-Pfb cmdlet exposing TotalOnly and require its committed capability-map endpoint to declare total_only. The guard reads committed data, executes on both PowerShell editions, and fails rather than skipping when its inputs are missing or empty. Cover bind-time rejection on corrected cmdlets and preserve the 12 supported surfaces so this cannot become a blanket removal. Co-Authored-By: Claude <noreply@anthropic.com>
Remove-PfbArrayConnection and Update-PfbArrayConnection chose a single
ShouldProcess target with RemoteId ahead of Id. -Id and -RemoteId are
legally combinable (that is the point of -RemoteId being optional in the
ById sets), so a call like
Remove-PfbArrayConnection -Id conn-1 -RemoteId r-77
sent ids=conn-1&remote_ids=r-77 but told the operator, in the High-impact
confirmation prompt and in -WhatIf, that the target was "r-77" -- the
remote array, not the connection being deleted.
Both cmdlets now prefer the connection identity and compose the remote as
a qualifier when both are bound, rendering "conn-1 (remote r-77)".
Single-selector targets are unchanged, and parameter sets, query keys and
request bodies are untouched.
Tests: a new shared file pins the exact target string for both cmdlets
under both editions. WhatIf text goes to the PSHost and not to any
redirectable stream, so a merged-stream capture can never observe it --
the tests instead run each cmdlet in a private runspace whose host records
UI writes, and assert on the recorded string. The suite also pins that
merged-stream capture sees no WhatIf text, so the mechanism cannot pass
vacuously. Both per-cmdlet suites gain a composite -WhatIf assertion that
the API is not called.
Co-Authored-By: Claude <noreply@anthropic.com>
…eline PR dmann000#107 added Tests/coverage-baseline.psd1 after this lane branched. The three TotalOnly guard Describes are ungated and read only committed capability data, so they must contribute executed tests on both editions; list them in both RequiredDescribes allowlists so their disappearance reds the build instead of passing silently. No ceiling change: the guards add no skipped tests. Co-Authored-By: Claude <noreply@anthropic.com>
Establish the first dedicated wire-contract suite for source-side file-system creation before changing defaults and exposed parameters. Co-Authored-By: Claude <noreply@anthropic.com>
Reject default SMB export creation unless the request names a share policy, so source-side file-system creation cannot silently attach the array's broad built-in policy. Co-Authored-By: Claude <noreply@anthropic.com>
The comment claimed a generic dictionary's typed indexer throws under PowerShell, which is not the case. State only that the cast pins the lookup to the non-generic IDictionary indexer explicitly. Comment only -- no behavior change. Co-Authored-By: Claude <noreply@anthropic.com>
Send an explicit false value unless callers request snapshot-directory visibility, replacing the array's surprising implicit true default. Co-Authored-By: Claude <noreply@anthropic.com>
…nn000#55) Add the missing SMB protocol toggle without implicitly enabling SMB when the caller only configures continuous-availability behavior. Co-Authored-By: Claude <noreply@anthropic.com>
…ent main Rebasing this lane onto a main carrying PR dmann000#98 (regenerated capability map) and PR dmann000#107 changes the drift denominator, so the measured figures quoted as the justification for the 0.55 -> 0.60 top-10 aggregation ceiling were stale. Record the re-measured values -- main alone 54.81% (570/1040), this lane on top of it 55.79% (564/1011) -- alongside the original pre-dmann000#98 pair. The conclusion is unchanged: 0.55 is genuinely exceeded and 0.60 still holds. Co-Authored-By: Claude <noreply@anthropic.com>
New-PfbFileSystem sends default_exports on every create, using the API's empty value when -DefaultExports is omitted so no default NFS or SMB export is created implicitly. That empty value was sent as a bare empty query value (`default_exports=`), which the array rejects with HTTP 400 "Missing or invalid parameter" -- so every create that omitted -DefaultExports failed and no file system landed. The accepted empty value is a quoted empty string as the single array item, i.e. `default_exports=%27%27` on the wire. Send that instead. Explicit 'nfs', 'smb' and 'nfs','smb' values are unchanged. Tests were updated first and reproduced the failure (5 failing tests per edition), then pass under both PowerShell 7 and Windows PowerShell 5.1. The serialization assertion pins the decoded value and accepts either encoding, because .NET Framework leaves the single quotes literal in the query string while .NET percent-encodes them; both decode to the same value. Co-Authored-By: Claude <noreply@anthropic.com>
Keep public help and tooling fixtures focused on current behavior rather than superseded review history. Co-Authored-By: Claude <noreply@anthropic.com>
The hand-maintained File:Line values in the reconciliation table cited the preceding [Parameter()] attribute instead of the ValidateSet being reconciled, so a reader following the published file:line landed one line short. - New-PfbFileSystem -MultiProtocolAccessControlStyle 175 -> 176 - New-PfbFileSystem -GroupOwnership 188 -> 189 - Get-PfbArrayPerformance -Protocol 28 -> 29 (same defect, found by the new test) Adds a guard that resolves each record's Parameter in its source file via the AST and pins the record's Line to that parameter's ValidateSet attribute, so a drifting citation now fails instead of shipping. The guard needs neither the spec cache nor the generated manifest, so it runs on both editions. Regenerates Reports/PfbValueEnumReconciliation.md only; the extracted values, statuses and notes are unchanged. Co-Authored-By: Claude <noreply@anthropic.com>
The new ValidateSet-citation Describe runs on every leg and has no skip path, so if it is ever filtered out, renamed, or its BeforeAll throws silently, it contributes neither a pass nor a skip and the skip ceiling cannot see it. Only RequiredDescribes catches that, so the entry belongs in both editions' allowlists alongside the other ungated guards. Adds a focused assertion for the registration itself: the existing baseline test only checks the reverse direction (every allowlisted name exists in Tests/), which cannot detect a missing entry. MaxSkipped is unchanged on both editions -- the guard adds executed tests, not skipped ones. Co-Authored-By: Claude <noreply@anthropic.com>
Refresh field, response-shape, drift, and value-enum reports after integrating issues dmann000#102, dmann000#88, and dmann000#55. Co-Authored-By: Claude <noreply@anthropic.com>
Regenerate the integrated API reports against the Fusion Phase 1 baseline and update the shifted value-enum source citation. Co-Authored-By: Claude <noreply@anthropic.com>
Omit the REST 2.16-only default_exports control on older arrays when callers leave it unspecified, while retaining capability rejection for explicit use and safe quoted-empty suppression on supported versions. Co-Authored-By: Claude <noreply@anthropic.com>
Update generated report references after New-PfbFileSystem help changes shifted parameter line numbers, keeping AST-validated enum citations and drift reports aligned with the current source. Co-Authored-By: Claude <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
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.
Summary
-TotalOnlyfrom 26Get-Pfb*cmdlets whose endpoints do not publishtotal_only, while preserving the 12 supported surfaces and adding a committed capability-map guard.ids,remote_ids, andremote_namesselectors across the replication family; remove four ignoredMemberNameselectors; and make destructive policy detach operations fail closed onMemberId.New-PfbFileSystemsafe by default: suppress implicit exports, default snapshot-directory visibility to false on the typed path, require an explicit SMB share policy for SMB default exports, expose SMB continuous availability, and remove incorrectly scoped per-filesystem eradication input.Closes #102
Closes #88
Closes #55
Compatibility and maintainer review
TotalOnlyswitches is an intentional breaking binding change.MemberNamefrom four replica-link policy membership cmdlets is an intentional breaking correction. The two detach cmdlets now requireMemberId, preventing an ignored selector from widening a delete.New-PfbFileSystemReplicaLink -Idnow forwards the endpoint's publishedidskey. The published specification does not document its create semantics; this is exposed for wire completeness and should receive explicit maintainer approval.New-PfbFileSystemremains usable on REST 2.0–2.15: omittingDefaultExportssends no unsupported query key and preserves that API era's filesystem access semantics. ExplicitDefaultExportsrequires REST 2.16 and fails capability validation before HTTP on older arrays.DefaultExportssuppresses both default exports. Explicit NFS or SMB policy fields can still create their corresponding export.New-PfbFileSystemcalls now sendsnapshot_directory_enabled = falseunless explicitly overridden. Caller-ownedAttributesbodies remain unchanged.EradicationModewas removed fromNew-PfbFileSystemwith no replacement. Remove incorrectly scoped per-filesystem eradication input fromNew-PfbFileSystem.Live verification
FB-A, REST 2.26, local array identity:
RemoteIdvalues. Six produced positive shrinking subsets: 4 to 2, 8 to 4, 2 to 1, 2 to 1, 42 to 20, and 14 to 1. Three one-row fixtures remained non-empty 1-to-1 matches. No replication write or delete was attempted because harness policy denies that family.New-PfbFileSystemdefault-safe, explicit snapshot-directory, continuous-availability-only, and SMB-with-explicit-policy lifecycles all passed create/read/destroy/eradicate with exact readbacks and complete cleanup.default_exports=is rejected with HTTP 400. The corrected quoted-empty formdefault_exports=%27%27passed.SourceSnapshotclone attempts by snapshot name and ID returned the pre-existing HTTP 400File system does not exist; no clone landed and all parent/snapshot fixtures were cleaned. This remains a separate contract investigation.Test plan
🤖 Generated with Claude Code