fix(tools): walk array item schemas and raise MaxDepth (#71, #82) - #97
Merged
juemerson-at-purestorage merged 5 commits intoAug 5, 2026
Conversation
…tems Two defects in Get-PfbSpecCapabilities' request-body walk, both changing Data/PfbCapabilityMap.json and both runtime-visible through Private/Assert-PfbApiCapability.ps1, which reads bodyProperties. dmann000#71 -- the two body-schema helper calls passed no -MaxDepth, so both took the helpers' own default of 8. The fb2.12-2.16 schemas compose through allOf chains deeper than that, so five PATCH /password-policies fields recorded introducedVersion 2.17 instead of 2.16 and would have been refused against a 2.16 array. Get-PfbSpecCapabilities now takes -MaxDepth, defaulting to 32 -- the value Get-PfbSpecResponseShapes already uses, chosen there by measuring this same truncation (184 false removals at 8 versus 7 true ones at 32). dmann000#82 -- a request body that is itself `type: array` carries its element schema on the `items` sibling keyword, not as a property, so the walk terminated immediately and four batch endpoints recorded an empty bodyProperties, hiding 23 fields from both the runtime gate and the drift report. The items hop is done at the CALL SITE, not by teaching Add-PfbSchemaPropertyNodes to descend `items`. That walker is shared with Get-PfbSpecResponseShapes, whose contract is that an envelope's properties and its items element's properties stay two separate levels; collapsing them would silently change Data/PfbResponseShapeMap.json and make its cross-version removal detection compare incomparable sets. This mirrors the hop Get-PfbSpecResponseShapes already performs at its own call site. Verified: Data/PfbResponseShapeMap.json regenerates byte-identical. Both regressions are covered by fixtures confirmed to fail before the fix. Artifact regeneration is deliberately NOT in this commit -- it is sequenced behind Fusion Phase 0 per issue dmann000#84. Refs dmann000#71, dmann000#82. Unblocks dmann000#44. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The rule has never had any effect. It was added in 69fe478 -- the same commit that first committed tools/ -- as a hedge while it was undecided whether the toolchain should be tracked ("Not yet decided whether this should be tracked"). Ignore rules do not apply to already-tracked files, so all 14 files under tools/ have been tracked from birth and the rule has been inert ever since. Removing it is a no-op for the working tree and a small improvement going forward: a NEW file added under tools/ now shows up in git status instead of being silently invisible, which is how tooling work here has previously gone missing. tools/specs/ keeps its own separate rule (.gitignore:36) and stays ignored -- verified that all 29 cached spec files remain ignored after this change, and that git surfaces no newly-untracked files. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The comment explaining why array bodies go unchecked is invalidated by the issue dmann000#82 map change in this branch, in both of its halves. It stated that every array-bodied endpoint carries "bodyProperties": {}, so a per-element check "could never fire". That is no longer true -- the four batch endpoints now carry real per-element fields. It then predicted that "this loop picks it up for free if a future map representation ever lands." That was wrong even when written: the loop is guarded on $Body being an IDictionary, and an array body arrives as [hashtable[]] (Set-PfbWorkloadTag passes -Tags straight through), so the loop is skipped before the map is consulted. A richer map alone changes nothing. Replaced with what is actually true now: the map records the fields, the type guard is the remaining blocker, and relaxing it is a real behaviour change that can refuse calls which succeed today -- deliberately not smuggled in with a generator fix. Also records that the blast radius is nil today (only Set-PfbWorkloadTag reaches such an endpoint, all its fields are 2.23) and that this stops holding once dmann000#44 adds cmdlets for the other three. Comment-only. No behaviour change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Pre-existing correction, isolated deliberately. This commit reverts nothing and fixes nothing in the generator -- it regenerates Reports/ with tools/lib/PfbSpecTools.ps1 held at the Phase-0 base, so the entire diff here is staleness that already existed on main before this branch. Reports/PfbApiDriftReport.json on main was one gap behind the committed capability map, which is why the "nothing vanishes" invariant in Tests/Build-PfbApiDriftReport.Tests.ps1 has been red locally. Root cause is New-PfbFileSystemReplicaLink's [Nullable[bool]]$RemoteDefaultExports being sent through a conditional assignment the drift tracer cannot follow, so confidence degrades high -> partial, enrichment is disabled, and systemicGaps/conventionStrength move 252 -> 246. Not spec staleness: analysedVersions is identical at 2.28. The diff here (271/19/14/5 lines across four files) is numerically identical to what origin/automated/update-api-capability-map already holds -- that workflow has failed at its "Open pull request" step on every run since 2026-07-24, so main never received this regeneration. Splitting it out means the next commit's diff contains only what this PR's code change actually causes.
…000#71 and dmann000#82 Everything in this diff is caused by the walker fix in the previous commits. Verified by construction: reverting tools/lib/PfbSpecTools.ps1 to the base and regenerating reproduces Data/PfbCapabilityMap.json byte-identical to the committed Phase-0 artifact, so nothing here is drift or nondeterminism. Capability map -- exactly 7 endpoint records change, in four groups: dmann000#71 5 x introducedVersion 2.17 -> 2.16 on PATCH /password-policies (name, id, enabled, is_local, location) dmann000#82 23 bodyProperties recovered across the 4 array-bodied endpoints: POST /nodes/batch +12, POST /resource-accesses/batch +2, PUT /workloads/tags/batch +5, POST /fleets/members/batch +4 dmann000#82 readOnlyBodyProperties on POST /nodes/batch resolves from empty to 8: capacity, chassis_serial_number, data_addresses, details, id, raw_capacity, status, unique --- 2 order-only records, no value changes: PATCH /ssh-certificate-authority-policies (bodyProperties key order) PATCH /file-systems (top-level key order; contextScope and readOnlyBodyProperties swap position) Note the four batch endpoints did not have an EMPTY bodyProperties before -- each had exactly one entry keyed by the empty string, the nameless artifact of a walk that reached the array node and could not descend it. That is what the 23 real fields replace. The map is deterministic: three consecutive regenerations produced identical SHA-256. Data/PfbResponseShapeMap.json is byte-identical (423de668a78356bad13b6d345bf9e834eca3438e7badf96577e78a5bbf93fef6). This is the guard that the fix stayed at the Get-PfbSpecCapabilities call site and did not leak into the shared Add-PfbSchemaPropertyNodes walker, whose contract is that an envelope and its items element are two separate levels. Phase 0 is undisturbed: schemaVersion still 2, 632 endpoints, and the full contextScope scope-x-provenance cross-tab is unchanged at array/default 604, array/declared 1, array/live-tested 1, fleet/declared 4, fleet/live-tested 3, unknown/unknown 19. Drift report: PUT /workloads/tags/batch gains 5 missingBodyProperties (copyable, key, namespace, resource, value); addable body properties 422 -> 427. Only this one of the four batch endpoints appears, because Set-PfbWorkloadTag is the only existing cmdlet calling any of them -- the other three are dmann000#44's scope. Reports/PfbFieldCmdletMap.json, Reports/PfbValueEnumMap.json and their .md siblings do not move under this change.
juemerson-at-purestorage
merged commit Aug 5, 2026
af8cb52
into
dmann000:integration/capability-map-2026-08
4 checks passed
This was referenced Aug 5, 2026
juemerson-at-purestorage
added a commit
that referenced
this pull request
Aug 13, 2026
…ility-map workflow (#63) No CI job has ever regenerated Data/PfbCapabilityMap.json or Data/PfbResponseShapeMap.json and compared the result against what is committed, so a generator change committed without regenerating passes silently. That comparison had to be done by hand for #96 and #97. Hashes the two maps before the build steps overwrite them, then names the outcome in the run summary. Deliberately reports rather than fails: a legitimate diff is this workflow's whole purpose, since a newly published REST version changes both maps, so a hard failure would red the scheduled run every time one lands. The value is that drift is now stated instead of inferred from the PR body. Also retires the third copy of the Pester invocation. It could not be replaced by a workflow_call to cross-platform-tests.yml -- a reusable workflow runs on a fresh runner with a fresh checkout, so it would test the committed Data/ and Reports/ rather than the ones this job just regenerated, which is the entire reason the step was inline. A script runs in this job's workspace. Step ordering verified unchanged: the response-shape map is still built after the spec fetch and before the drift report, per the load-bearing note already in this file. Refs #63
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.
Two bugs in the same schema-walking helper in
tools/lib/PfbSpecTools.ps1, landing togetherbecause they interact. Fixes #71. Closes #82's map and reporting scope.
Targets the
integration/capability-map-2026-08branch, notmain— see Sequencing below.The two bugs
#71 —
MaxDepthdefaults to 8, truncating fb2.12–2.16allOfchains. Five body fieldson
PATCH /password-policiescarriedintroducedVersion = 2.17when the specs say2.16.Runtime-visible, not cosmetic:
Assert-PfbApiCapabilityreadsbodyProperties, so it wouldwrongly refuse those five parameters against an array on REST 2.16. The 2.17 spec
restructuring flattened these chains, which is why the defect self-heals from 2.17 onward
and is invisible against current data.
#82 — the walk never descended an array body's
items. Four array-bodied endpointsrecorded no usable
bodyProperties, hiding 23 fields from both the runtime gate and thedrift report.
They compose: descending through
itemsconsumes a depth level, so #82 needs #71'sheadroom. Raising
MaxDepthalone does nothing for #82, and addingitemsalone doesnothing for #71.
Why the fix is at the call site, not in the walker
#82's obvious fix — teach
Add-PfbSchemaPropertyNodesto descenditems— would havesilently corrupted
Data/PfbResponseShapeMap.json. That walker is shared withGet-PfbSpecResponseShapes, whose entire contract is that an envelope's properties and itsitems[]element's properties are two deliberately-separate levels. Teaching the walkerto descend unconditionally collapses them, and the response-shape map's cross-version removal
detection would start comparing incomparable sets, with nothing to warn.
So the
itemshop happens at theGet-PfbSpecCapabilitiescall site, matching the precedentGet-PfbSpecResponseShapesalready sets.Add-PfbSchemaPropertyNodesis untouched.The guard that proves it:
Data/PfbResponseShapeMap.jsonis byte-identical before andafter —
423de668a78356bad13b6d345bf9e834eca3438e7badf96577e78a5bbf93fef6. If that SHA hadmoved, the fix leaked.
MaxDepth = 32is not a guess:Get-PfbSpecResponseShapesalready defaults to it, with acomment documenting the same fb2.12–2.16 truncation measured there as 184 false removals at
depth 8 versus 7 true ones at 32.
The regenerated artifact — exactly seven endpoint records change
introducedVersion2.17 → 2.16 onPATCH /password-policies(name,id,enabled,is_local,location)bodyPropertiesrecovered:POST /nodes/batch+12,POST /resource-accesses/batch+2,PUT /workloads/tags/batch+5,POST /fleets/members/batch+4readOnlyBodyPropertiesonPOST /nodes/batchresolves from empty to 8 fields:capacity,chassis_serial_number,data_addresses,details,id,raw_capacity,status,uniquePATCH /ssh-certificate-authority-policies(bodyProperties key order) andPATCH /file-systems(top-level key order —contextScopeandreadOnlyBodyPropertiesswap position)Nothing else. One correction to how #82 was described: those four endpoints did not have
an empty
bodyProperties. Each had exactly one entry keyed by the empty string — thenameless artifact of a walk that reached the array node and could not descend it. That is
what the 23 real fields replace.
Verification of the artifact itself
Because there is no CI gate comparing the committed map against generator output
(
cross-platform-tests.ymlnever fetches the gitignoredtools/specs/), the artifact waschecked by construction rather than assumed:
tools/lib/PfbSpecTools.ps1to the base and regeneratingreproduces
Data/PfbCapabilityMap.jsonbyte-identical to the committed Phase-0artifact. So every change above is caused by this PR's code — none is drift, staleness,
or nondeterminism. This also independently confirms the Phase-0 map is exactly what the
generator produces.
schemaVersionstill 2, 632 endpoints, and the fullcontextScopecross-tab unchanged:array/default604,array/declared1,array/live-tested1,fleet/declared4,fleet/live-tested3,unknown/unknown19.The
Reports/diff is two independent halves — split across two commitsConflating them is how the pre-existing half gets mistaken for damage this PR caused, so they
are separated at the commit level and each is independently reviewable.
Commit
28c34f2— pre-existing, not caused by this PR. RegeneratesReports/with thewalker held at the base.
Reports/PfbApiDriftReport.jsononmainwas one gap stale, whichis why the "nothing vanishes" invariant in
Tests/Build-PfbApiDriftReport.Tests.ps1has beenred locally. Root cause:
New-PfbFileSystemReplicaLink's[Nullable[bool]]$RemoteDefaultExportsgoes through a conditional assignment the drift tracer cannot follow → confidence
high→partial→ enrichment disabled →systemicGaps/conventionStrength252 → 246. Not specstaleness;
analysedVersionsis identical at 2.28.This diff (271/19/14/5 lines across four files) is numerically identical to what
origin/automated/update-api-capability-mapalready holds — that workflow has failed at itsOpen pull requeststep on every run since 2026-07-24, somainnever received it.Commit
e0d9a2b— this PR's own effect. Only three files move:Data/PfbCapabilityMap.jsonplus the drift-report pair.PUT /workloads/tags/batchgains 5missingBodyProperties(copyable,key,namespace,resource,value); addable bodyproperties 422 → 427. Only one of the four batch endpoints appears, because
Set-PfbWorkloadTagis the only existing cmdlet calling any of them — the other three are#44's scope.
Reports/PfbFieldCmdletMap.json,Reports/PfbValueEnumMap.jsonand their.mdsiblings do not move.
The
.gitignorechangetools/is removed from.gitignore. The rule was inert for already-tracked files,which is why every
tools/file was committed despite it. Removing it stops new filesunder
tools/being silently refused — confirmed during this work:git addon a trackedtools/file prints the ignore warning and exits non-zero while still staging the file,which will short-circuit any
&& git commitchain.Live verification — FB-A, REST 2.26
Seven ledger rows, every one stamped
gitRef = fix/issue-71-82-schema-walk-depth-arrays@e0d9a2b, so the code under test is thisbranch and not
main. Outcomes below are the harness's ownoutcomefield.New-PfbFileSystemPOST /file-systemsGet-PfbFileSystemGET /file-systemsprovisioned = 1073741824— the create really landedUpdate-PfbFileSystemPATCH /file-systems2147483648), so the key-order shuffle is inert at runtimeSet-PfbWorkloadTagPUT /workloads/tags/batchUpdate-PfbFileSystemPATCH /file-systemsRemove-PfbFileSystemDELETE /file-systemsGet-PfbFileSystemGET /file-systemsThe
Set-PfbWorkloadTagfailure is the informative result, and it confirms rather thancontradicts the #95 claim below. The array answered
HTTP 400: Workload does not exist.—a rejection of the resource, not of a body field. The call therefore reached the wire and
was never gated client-side:
Tagsis[hashtable[]], it hits the-is [IDictionary]guard, and the body-field loop is skipped before
bodyPropertiesis consulted. The 23recovered fields are inert at runtime today, exactly as stated below — this measures
that rather than asserting it.
PATCH /password-policiescould not be live-tested at all, for a second reason on top ofthe version ceiling:
password-policiesis on the harness family deny-list, so the callreturns
Skipped-Policy("write verb 'Update' is denied for family 'password-policies'")and never reaches the array. Not routed around.
What this does NOT do — stated explicitly
items) #82's runtime goal is not reachable from the map alone, and is not claimed here.Assert-PfbApiCapability's body-field loop is guarded on-is [System.Collections.IDictionary](:102), and an array body arrives as[hashtable[]], so the loop is skipped beforebodyPropertiesis ever consulted. Runtimefield-gating for these endpoints is tracked in Assert-PfbApiCapability cannot field-gate array bodies: the IDictionary guard outlives its premise once #82 lands #95.
2.26, above the version where the defect is observable, and the endpoint's family is on
the live-harness deny-list (measured above).
items) #82's batch gating is not live-verifiable — the type guard above blocks it, and of thefour endpoints only
Set-PfbWorkloadTagreaches one. Probed anyway; the array rejected theresource, not the body, which is the positive evidence that the guard short-circuits first.
is Assert-PfbApiCapability cannot field-gate array bodies: the IDictionary guard outlives its premise once #82 lands #95.
schemaVersionbump. Phase 0 claimed 2; this PR does not contest it. Capability map cannot record array-body cardinality constraints (minItems/maxItems/uniqueItems) — needs schemaVersion 2 #83 incrementsfrom whatever it finds.
CHANGELOG.md.Sequencing
This is the second of the two consecutive regenerations of
Data/PfbCapabilityMap.jsonagreed in #84.
Phase 0 (#96) went first and took
schemaVersion 2; this PR rebased onto it and regeneratedonce on the stable base.
Both are stacked on
integration/capability-map-2026-08so the combined state is builtand tested before anything reaches
main, rather than the two changes meeting for the firsttime after they have both landed. One final PR will bring the integration branch to
main.