feat!: derive pro command names from the spec, and nest independently-writable sub-paths

[neilmartin83]

pro command names come from the spec — the API's own OpenAPI tags and URL paths — instead of from the names of the 165 files the specs were split into. specs/ becomes one normalised document.

This is breaking. CHANGELOG.md is the migration guide and carries every moved invocation; this description is the reviewer's map.

Why

A resource's name came from the filename its paths were split into — upstream's jss module names, which appear in no published spec and in no API reference. Four things followed from that filename with nothing stating them: the command name, the endpoint-version family, whether a -preview tag reached a command at all, and whether the ingest could delete the file.

Two consequences:

• The ingest was not reproducible. Wiping specs/ and re-ingesting the same live monolith gave 127 files instead of 165, 132 resources instead of 169, 73 operations dropped, 98 command groups renamed and 18 -preview tags published as commands — at exit 0.
• Version consolidation keyed on a -vN filename suffix, which is how this CLI once sent /v3/computers-inventory while /v4 was the served version, with the two v4-only operations never generated at all.

Both are now facts about the paths. The second class is inexpressible rather than guarded, which is why this PR deletes the pass that used to guard it.

The rule

The tag decides what belongs together, the path decides where the boundary falls, and the name comes from the tag. Measured on the live 11.31.1 monolith (550 paths, 820 ops) against 163 resources before:

	resources	unchanged	renamed	merges	splits
tag+path (chosen)	136	60	40	24	13
path only	148	57	44	21	19
tag only	125	52	60	19	1

Neither signal works alone and both failure modes were measured — paths split a resource upstream considers whole (computers-inventory is three path roots), tags are too coarse to be a command (computer-groups carries two complete CRUD sets that would both want get). Tags are sound rather than convenient here: no path in the document carries two different tags.

The decisive argument for naming from the tag rather than the path was the override table: path naming needed eight entries and seven were names invented in this repo (a pki- prefix the reference does not use, ldap-lookups, ddm-clients, certificate-authorities). One override survives.

Independently-writable sub-paths nest

A tag can cover several path roots, and merging them is right for the resource's identity and wrong for its verbs. A flattened sub-path produces a plain CRUD verb that silently belongs to something else, and because a lone verb collides with nothing, no naming pass reached it:

• pro sso-settings delete sent DELETE /v2/sso/cert — it deleted the SSO certificate, not the configuration its name names.
• pro managed-software-updates-plans update sent PUT …/plans/feature-toggle on a resource whose list, get and create are real plan CRUD.
• pro csa delete deleted the CSA token.

The rule is read off the methods the spec declares: a sub-path carrying PUT, PATCH or DELETE on itself owns a separately-writable object and becomes a command of its own. No override table. That admits nine sub-paths and refuses every other one, all 18 GET+POST /history pairs included — by the rule, not by a history special case.

One further condition: a sub-path holding every operation in its group does not split, there being no sibling verb for the plain one to be confused with (pro app-request get stays as it is).

Reviewing this

Start here. These three files are the change; the rest is consequence.

file	what
generator/parser/taggroup.go	the tag+path hybrid — GroupPathsByTagAndCollection
generator/parser/pathgroup.go	the path-root rule, and every override table with its reason
generator/parser/subresource.go	the sub-resource rule, its three judgement calls, and why each went the way it did

Then the guards. Each failed on a real defect during the work rather than being written afterwards to describe one:

• TestParseMonolith_LosesNoEndpoint — the guard for the whole change, against a committed snapshot of every endpoint reachable under the 165-file layout. 704 → 704, zero gained or lost.
• TestEveryFormerInvocationKeepsItsShape — every one of main's 1355 pro invocations resolved through the new tree. Resolution is not the property; shape is. cobra's Find falls back to the deepest command it matched, and a leaf can become a help-printing group at exit 0 — three did, and a resolution-only sweep called all three fine.
• TestSubResourcePartitionIsPinned — the nine admitted and the composition of what is refused, so a spec drop that adds a DELETE to a sub-path fails visibly instead of silently moving commands.
• TestNoPlainVerbLeavesItsResource — the test that would have caught pro sso-settings delete, written against the whole shipped surface with four pre-existing cases allowlisted and each carrying its reason.
• TestEveryResourceKeyedOverrideNamesALiveResource — the guard whose absence cost six user-visible defects at once when the rename moved resources out from under thirteen override keys.

Two traps worth knowing before reading the passes. Three separate passes encoded "the resource is one path root" — true while identity came from a filename, false once a tag merges several — and each shipped a differently-wrong command name at exit 0. Resource.Root is now carried rather than inferred, because every attempt to infer it was wrong in a different way. And the sub-resource split added a fourth level to that same model, which is where three more passes needed re-reading.

Compatibility

• Every former resource name works until 2027-03-09. 98 resolve to their replacement with a warning naming it, 4 redirect two tokens deep into a nested sub-resource, 3 refuse with an explanation because their endpoints are no longer ingested. The warning is not silenced by --quiet or --no-hints, and TestDeprecatedNamesHaveNotExpired fails the build on the date so the aliases cannot rot silently.
• 43 invocations an alias cannot cover — the operation name moved too. Every one is in CHANGELOG.md's migration table with its endpoint, and every target is verified to resolve to a leaf.
• 3 paths became command groups, which is the sharpest edge: still resolves, still exits 0, prints usage where it returned data. Tabulated in the CHANGELOG.
• 3 apply commands are gone and none of them worked. pro managed-software-updates-plans apply was destructive rather than absent: /plans publishes no update at all, so the only PUT the resource carried was the feature toggle's and apply composed it — given an existing plan's name it resolved the name to an id and then PUT the plan document at /plans/feature-toggle.

Verification

• make lint 0 issues; full suite, verify-generated, verify-gateway-coverage, verify-classic-schemas and verify-site all pass. Every commit builds and tests green individually.
• A full re-ingest of the live 11.31.1 monolith produces zero changes to specs/ and zero to generated code — the reproducibility property the change exists to establish.
• The resolveNameToID call sites were diffed against main: one change, the removed plans apply.
• Wire-verified on a live EU environment credential. All nine nested reads answer; pro self-service-plus settings update round-trips through a body and through --set; all four two-token redirects warn and return their own endpoint's body; -n pro sso-settings cert delete --yes previews rather than executing; --scaffold works with no credentials.

Two silent meaning changes repaired

Neither is visible to a test that only checks whether a command resolves, and both were introduced by the rename itself:

• pro sso-settings download sent GET /v3/sso/metadata/download before the rename and GET /v2/sso/cert/download after it, because the certificate's download won the collision. It sends the SAML metadata again; the certificate is at pro sso-settings cert download.
• pro sso-settings delete no longer exists rather than deleting the certificate.

Deliberately not in this PR

• Seven pre-existing list defects, each a list pointing at a single object rather than a collection (pro m2m list returns a tenant id, pro jamf-protect list the Protect plans, and five more). Both new naming passes are scoped to resources the sub-resource split touches, so these keep the names they ship under — fixing them is seven breaking renames of unrelated commands, and it is not this change.
• The wiki is a separate .wiki.git repo and carries command names. Nothing in this repository references a dead name; the wiki needs a follow-up pass after merge.

🤖 Generated with Claude Code

[ktn-jamf]

Summary

This change replaces filename-derived pro command names with names derived from the spec's own OpenAPI tags and URL path roots. The reasoning is unusually good. The three core files each state the rule, the two measured failure modes, and the option that was rejected. The sub-resource split is derived from declared methods with no override table, and it repairs three real defects (pro sso-settings delete deleted the SSO certificate, pro csa delete deleted the CSA token, pro managed-software-updates-plans update wrote the feature toggle). The guards are written against defects the work actually hit, and I confirmed that each one runs against real data and is not vacuous: 704 endpoints, 1355 invocations, 9 admitted sub-paths.

One defect must be fixed before merge. The rule that decides which tag names a resource is not the rule the PR describes. Where one path root carries two tags, the name comes from whichever path sorts last. Three resources ship a wrong name as a result, and one of them is a write surface.

Findings

❌ CRITICAL (1) (correctness) — a path root shared by two tags is named by sort order, and pro smart-user-groups is the user CRUD

generator/parser/taggroup.go:106-122

rootOf (line 89) is computed per path from the whole document. It never reads the tag. resolveRootsWithinTag folds roots only inside one tag, so it cannot stop two tags from computing the same root string. The assembly loop then groups on root alone, and g.Tag = tagOf[p] (line 118) is overwritten on every iteration. Paths arrive sorted, so the surviving tag is the tag of the alphabetically last path. nameFromTags names the resource after that tag. Nothing detects the merge and nothing reports it.

Two statements in the code assert the opposite:

• generator/parser/pathgroup.go:44 — "Tag is the OpenAPI tag every path in the group carries".
• generator/parser/taggroup.go:33-35 — "a path root never merges two tags".

I simulated the pass over the committed specs/JamfProAPI.yaml, with droppedTags and droppedPaths applied. Seven groups hold more than one tag. Three take a wrong name, and all three ship. Verified against the generated source and against the built binary:

root	tag that wins	correct tag	shipped name
users	smart-user-groups	users	pro smart-user-groups
patch-policies	patch-policy-logs	patch-policies	pro patch-policy-logs
user	jamf-pro-user-account-settings	—	pro jamf-pro-user-account-settings

bin/jamf-cli pro smart-user-groups --help reports:

Available Commands:
  apply     Create or replace a smart-user-group by name
  create    Create a new user in inventory
  delete    Deletes a user from the inventory by ID

internal/commands/pro/generated/smart_user_groups.go confirms the endpoints. list, get, create, update and delete all send /v1/users, apply resolves on username at /v1/users (line 615), and its replace branch deletes at /v1/users/{id} (line 584). The name and the help text describe a smart user group. The command reads, writes and deletes a user. pro patch-policy-logs list has the same shape: its own Short is "Retrieve Patch Policies" and its Example says "List all patch-policy-logs".

internal/commands/deprecated_names.go:167 records "users": {Now: "smart-user-groups"} beside "user-smart-groups": {Now: "smart-user-groups"} at line 166. Two unrelated former resources now point at one name, which is the merge made visible in the alias table.

Failure scenario. An operator reads pro smart-user-groups apply --help, which states that the input's username field decides whether the resource is replaced, and pipes a document of group definitions into it. The command resolves each name against the user collection and, on a match, deletes and recreates a user record. No warning names the object it acts on, because every string the command prints says "smart-user-group".

Nothing is broken today, because deprecatedNames keeps pro users and pro patch-policies working with a warning. The cost is different. This PR exists to remove names that came from an accident, and it replaces one accident (a filename) with another (sort order). After 2027-03-09 the wrong name is the only name, and correcting it then is a second breaking rename.

Suggested fix. Name a merged group from the tag of its own root collection path, and fall back to the most frequent tag. I checked this against the document: the five merged groups that have a root collection get the correct tag from it, and the two without one (user, self-service) already get the correct answer from frequency.

-		g.Tag = tagOf[p]
+		// A root can carry paths from two tags. The tag of the group's own
+		// root collection names it; last-write-wins names it by sort order.
+		g.Tag = groupTag(root, g.Paths, tagOf)

Add a guard so the next spec drop cannot repeat this silently: report every root that merges two tags as a note from make generate, or fail with the root and the tags named. A test that asserts each resource's name against the tag of its root collection is the direct form.

Fixed when. pro users list and pro patch-policies list are the canonical names again, no resource name contradicts the Short text of its own verbs, and a root that merges two tags is reported rather than resolved by order.

---

⚠️ IMPORTANT (2) (correctness) — "no path carries two different tags" is the design's premise and is enforced nowhere

generator/parser/monolithparse.go:56-98, generator/parser/taggroup.go:33-35

tagsByPath accumulates op.Tags across every method on a path, and firstTagOf takes tags[0] in sorted-method order. If two methods on one path declare different tags, one tag wins and nothing reports it.

I checked the committed document: zero paths carry disagreeing tags, so the premise holds today. That is why this is IMPORTANT and not CRITICAL. The premise is load-bearing for the whole design, it is stated as a fact about the document, and the document is refreshed from upstream on every ingest.

Failure scenario. A spec drop tags GET /v3/foo as foo and PATCH /v3/foo as foo-management. firstTagOf picks one. TestParseMonolith_LosesNoEndpoint compares endpoint counts and passes. The command tree reorganises, and the diff reads as a plausible rename.

Suggested fix. Fail ParseMonolith when a path carries more than one base tag, naming the path and the tags.

Fixed when. A path with disagreeing tags fails make generate with both tags named.

---

⚠️ IMPORTANT (3) (documentation) — docs/guides/platform-api-ga.md was not updated, and it now contradicts the binary

CLAUDE.md's "Read first" section states that this guide "quotes verbatim CLI output" and must be updated whenever the refused-command list moves. This PR moves it. CLAUDE.md's own line 534 records the refused count dropping from 67 to 59 "because spec-derived resource naming folded the withdrawn versions", and the guide was not touched.

Stale rows and transcripts include pro static-computer-groups (lines 310, 358, 371, 377, 407-408), pro computers-inventory (305, 329, 362, 588-590), pro authentications (309), pro api-roles-privileges (311), pro systems (313), pro database-connections (314), pro mac-os-managed-software-updates (316) and pro oauth-token-sessions (318). Line 408 quotes a refusal message for a command that no longer exists.

CLAUDE.md itself was updated thoroughly, and its remaining computers-inventory mentions are correct past-tense narrative. The gap is this one file.

Failure scenario. A user follows the guide's own instruction at line 348 and greps commands -o json for pro mac-os-managed-software-updates. The grep returns nothing, because the command is now pro macos-managed-software-updates.

Suggested fix. Re-run the guide's transcripts against the built binary and correct every name and every count.

Fixed when. Every pro <resource> name quoted in the guide matches a name bin/jamf-cli commands -o json reports.

---

⚠️ IMPORTANT (4) (reliability) — the three leaf-to-group conversions get no runtime signal

internal/commands/deprecated_names.go

Every other breaking class in this PR gets a runtime answer. A renamed resource warns and resolves. A withdrawn one refuses and explains. The three former leaves that became groups get nothing. pro csa token -o json prints usage text and exits 0, even when -o json or --field asks for structured data. A typo exits 2, so this state is less legible than a typo.

The PR calls this "the sharpest edge" and answers it with a CHANGELOG row. The CHANGELOG is not read at runtime.

Failure scenario. A job that ran pro csas token -o json | jq -r .value now pipes cobra's help text into jq. The failure appears in jq, with nothing pointing back at the CLI.

Suggested fix. Where a structured-output flag reaches one of these three groups, refuse with an explanation that names the new leaf, in the same shape withdrawnNames uses.

Fixed when. pro csa token -o json exits non-zero and names pro csa token get.

---

⚠️ IMPORTANT (5) (usability) — the 43 moved invocations get cobra's bare "unknown command"

internal/commands/deprecated_names.go, CHANGELOG.md lines 50-115

The resource alias resolves the first token. Cobra then reports unknown command "triggers" for "pro scheduler" and exits 2, which is indistinguishable from a typo. This is the hardest class to self-diagnose, because the resource name is right and only the verb moved. The 55-row table exists for exactly this case and nothing at runtime points at it.

Suggested fix. Add a resource-plus-verb table so applyDeprecatedNames can intercept these 43 and warn with the new invocation named.

Fixed when. None of the 43 documented invocations produce cobra's generic error.

---

⚠️ IMPORTANT (6) (test-coverage) — the deprecation warning is the whole migration mechanism and no test asserts it

internal/commands/deprecated_names.go:401, wired at internal/commands/root.go:759

warnIfDeprecatedName prints the warning that tells a caller of one of the 98 retired names to migrate. No test calls it and no test captures stderr from a command run. TestProductTokenAndResourceToken covers only the two pure helpers it is built from.

Replacing the function body with a no-op leaves the whole internal/commands suite green. Every existing guard checks that the alias resolves and runs, and none checks that the warning is emitted.

Failure scenario. A later reorganisation of PersistentPreRunE drops the call site, or the lookup regresses. Scripts on the 98 deprecated names keep working with no warning until 2027-03-09, then break together with no notice. That is the exact outcome the alias mechanism exists to prevent.

Suggested fix. Assert the exact warning text on stderr for one deprecatedNames entry and one nestedAliases entry, and assert that a live name emits none.

Fixed when. A test fails when warnIfDeprecatedName is replaced with a no-op.

---

💡 NICE-TO-HAVE (7) (dead code) — GroupPathsByCollection ships with six tests and no caller

generator/parser/pathgroup.go:73-113

Nothing in the pipeline calls it. Its only callers are six tests in pathgroup_test.go. Those tests pin the path-only rule, which this PR replaced with the tag-plus-path hybrid, so they read as coverage of the shipped rule and are not. The PR's own reasoning for deleting DeduplicateVersioned applies here: "kept dormant they would have been worse than absent".

Fixed when. The function is deleted, or its tests target GroupPathsByTagAndCollection.

---

💡 NICE-TO-HAVE (8) (reliability) — the alias expiry has no advance signal

internal/commands/deprecated_names.go:34-39, deprecated_names_test.go:50-58

A test that fails the build on a date is the right forcing function, and it matches this repo's conventions. There is no schedule: trigger in .github/workflows/, so the test first fires on whatever PR runs CI on or after 2027-03-09. The work it forces is a real PR, not a one-line change.

Suggested fix. Warn 30 to 60 days ahead, or file a tracked issue now.

---

💡 NICE-TO-HAVE (9) (usability) — a split sub-resource lost its short alias

internal/commands/aliases.go

pro jcds files became pro jamf-cloud-distribution-service-files, 37 characters, with no alias, while the parent keeps jcds.

  "jamf-cloud-distribution-service": {"jcds"},
+ "jamf-cloud-distribution-service-files": {"jcds-files"},

---

✅ POSITIVE — the sub-resource rule, and the discipline around it

generator/parser/subresource.go is the best-argued file in the change. It names the rule, names the three questions the rule leaves open, and records the answer taken for each with the reason. The rule is read off declared methods, so a new path needs no table entry. It admits 9 sub-paths and refuses 131, and all 18 GET-plus-POST /history pairs are refused by the rule rather than by a special case.

The sub-resource consumers are also right, and each carries the reason. modernGatewayOps (generator/main.go:859) uses AllOperations() with the correct rationale: an unstamped operation carries no jamf:gateway annotation, so checkAPIMatch could not refuse a nested command on a withdrawn endpoint. The smoke and backup registries use FlattenResources plus QualifiedName(). I checked every consumer and found no gap.

---

Review coverage and scope

Risk: high. 366 files, breaking, generator plus whole pro command surface, one pass deleted.

• Design and architecture — read the three core passes in full
• Correctness — simulated GroupPathsByTagAndCollection over the committed document; verified findings against generated source and the built binary
• Scope symmetry and data-flow completeness — checked every Operations / AllOperations / QualifiedName consumer
• Security — no auth surface changed; the gateway-annotation path was checked because a missed operation cannot be refused
• Reliability — alias, redirect and refusal paths
• Test coverage and fidelity — ran ./generator/..., ./internal/commands/..., ./internal/gateway/...: all pass. Ran the four named guards and confirmed each processes real data (704 endpoints, 1355 invocations, 9 sub-paths). The test-quality lane independently re-derived both committed baselines by building origin/main and running the old pipeline against the old 165-file layout: endpoints-before-path-grouping.tsv matches exactly, and all 1355 rows of pro-invocations-before-nesting.tsv match main's real cobra tree. The baselines are genuine pre-images, not self-fulfilling. Four of five mutations were killed; the survivor is finding 6
• Documentation currency — CLAUDE.md, README.md, docs/GLOSSARY.md, docs/guides/platform-api-ga.md, CHANGELOG.md
• Code quality — dead code found
• [na] Performance — no hot path changed
• [na] Frontend — docs/site/ is generated from the binary

Specialist lanes dispatched: silent-failure-hunter, devil-advocate, usability-reviewer, test-quality-reviewer. All four returned. Findings 2, 4, 5, 6, 8 and 9 come from them and were each re-checked against the code before inclusion. Finding 1 is my own and is confirmed three ways.

Prior reviews: none.

Confidence: finding 1 — 5/5, verified against the built binary. Findings 2, 3, 6, 7 — 5/5, verified directly. Findings 4, 5 — 4/5. Findings 8, 9 — 4/5.

Rating: 3/5

Excellent engineering with one must-fix. The design, the reasoning and the guards are all above the bar for a change this size. Finding 1 blocks merge, because a breaking rename that ships a wrong name buys a second breaking rename later, and one of the wrong names is a destructive write surface.

Warning

Fix finding 1 before merge. Findings 2 to 6 are worth clearing in the same pass, because each is cheap now and expensive after the aliases expire.

---

Reviewed with the pr-review skill (Claude Opus 5). Findings 1, 2, 3 and 7 were verified directly against the built binary and the committed spec document; the rest come from specialist lanes and were re-checked against the code before inclusion.

[neilmartin83]

@ktn-jamf — all nine findings addressed in 1ee814f. Ready for re-review.

The CRITICAL was real and your diagnosis was exact, including the suggested fix and the check that the five merged groups with a root collection get the correct tag from it. Two things came out of chasing it that were not in the report.

❌ CRITICAL (1) — a merged root named by sort order

groupTag (generator/parser/taggroup.go) names a merged group from the tag of its own root collection, falling back to the most frequent tag where no path answers as the collection. Computed after mergeRoots rather than inside the assembly loop, because mergeRoots moves paths between groups and cannot re-derive a tag assigned before it ran.

Reproduced your simulation before changing anything — 8 merged groups over the committed document (you counted 7; the eighth is computers-inventory, which takes the right tag either way), 3 taking a wrong name. All 8 now take the tag of their root collection:

root	tags held	tag now	was
users	users×2, smart-user-groups×2	users	smart-user-groups
patch-policies	patch-policies×3, patch-policy-logs×6	patch-policies	patch-policy-logs
user	jamf-pro-user-account-settings×2, user×1	jamf-pro-user-account-settings	unchanged — no root collection, frequency decides, exactly as you predicted
computers-inventory, device-enrollments, mobile-devices, self-service, sso	—	unchanged	already correct

Wire-verified on an EU environment credential: pro users list → GET /pro/v1/users 200, pro patch-policies list → GET /pro/v2/patch-policies 200. Both merges are exact — every verb from the absorbed resource keeps its own name under the new parent — so pro user-smart-groups recalculate and pro patch-policy-logs logs both resolve through the alias with no verb move.

Two guards, and I mutated groupTag back to last-write-wins to check they are not vacuous. TestMergedTagGroupsAreNamedByTheirRootCollection fails naming both wrong groups; TestUserAndPatchPolicyResourcesAreNamedAfterWhatTheyServe fails four ways. The first also asserts the frequency fallback rather than skipping it, and t.Fatals if no merged group is left, so it cannot pass vacuously on a future drop.

The finding had a sibling in the same shape. Untagged paths all shared the empty string as their bounding key, so resolveRootsWithinTag's no-survivor fallback would fold every unrelated untagged root into the largest — /v1/health-check and /v1/health-status coming out as one health-check resource. Same class as finding 1: a shared key merging things nothing declared to be together. boundKey bounds an untagged path by its own root. No live path is untagged, so this is a guard against a drop; TestPathRootRule_TopLevelCollectionIsAlwaysARoot is what holds it, and it fails without the fix.

⚠️ IMPORTANT (2) — the premise is enforced now

soleBaseTagOf (generator/parser/monolithparse.go) fails ParseMonolith naming the path and both tags. firstTagOf is gone. -preview is stripped before comparing, so foo and foo-preview on one path stay one tag rather than reading as a disagreement.

Confirmed your count independently — zero live paths carry disagreeing base tags — so it refuses nothing today. TestEveryLivePathDeclaresOneBaseTag states that, and TestSoleBaseTagOfRefusesAPathCarryingTwoTags covers your exact foo / foo-management scenario plus both non-cases.

⚠️ IMPORTANT (3) — the GA guide

Rewritten against the built binary. The refusal table is recounted from commands -o json: 59, not 67, and none of the eight was un-refused by the gateway, which the guide now says rather than leaving the reader to wonder:

• six were pro static-computer-groups, the withdrawn v2 command, which stops existing once every version of an endpoint lands in one resource — there is no second command to refuse;
• two were pro policy-properties, whose unversioned legacy twin /settings/obj/policyProperties is no longer ingested at all, while the /v1/policy-properties it shared a tag with is published. Verified: pro policy-properties get → GET /pro/v1/policy-properties 200.

The static-computer-groups successor walkthrough was the larger problem — it described a case that no longer exists, and the successors table is empty. Rewritten around that, with the mechanism kept and the reason it will fill again. Every renamed row corrected (pro computer-inventory, pro api-authentication, pro api-role-privileges, pro jamf-pro-initialization, pro macos-managed-software-updates, pro mdm commands, pro sso-oauth-session-tokens, pro app-installers-titles, pro icon upload, pro health-check, pro jamf-pro-version), and both quoted transcripts replaced with real output. All nine refusals re-run on the wire at exit 8.

The CHANGELOG had the same table and is corrected with it. Its 40 renamed is now 38 and 102 resolve is 100 — users and patch-policies moved out of the renamed set, and jcds out of the expiring one (see 9).

⚠️ IMPORTANT (4) — the three former-leaf groups

formerLeafGroups + guardFormerLeafGroups (internal/commands/moved_invocations.go, wired in root.go after guardUnknownSubcommands, whose RunE it wraps — installing a RunE first would make the group Runnable() and lose the "did you mean" refusal beneath it).

$ jamf-cli pro csa token -o json
`pro csa token` is a command group and returns no data; it returned data before the sub-resource split
hint: run `jamf-cli pro csa token get`
$ echo $?
2

Any of --output, --field, --select, --out-file triggers it — -o table included, since a group renders nothing whatever format is named and a caller piping -o table into awk is in the same position. A bare pro csa token still prints help at 0, and a typo beneath it still gets the unknown-subcommand refusal; both are asserted. Read through cmd.Root().PersistentFlags() and by Changed rather than by value, so a config setting default-output: json does not turn every bare invocation into a refusal.

⚠️ IMPORTANT (5) — the 43 moved invocations

movedInvocations covers all 43, keyed on the command path after resource-alias resolution, so one entry serves every spelling:

$ jamf-cli pro schedulers triggers
`pro scheduler triggers` no longer exists: the operation moved when command names started coming from the spec
hint: the endpoint is unchanged; run `jamf-cli pro scheduler-jobs triggers`

Three things worth flagging:

• Exit 2, not a code of its own. A wrapper branching on the code would then see two codes for one class, and what the caller is missing is the pointer rather than a classification. Say the word if you want 8 instead.
• Two old invocations collapse onto pro enrollment list (access-managements list and enrollment-settings list), and their endpoints went to different places, so that entry names both. It is the only collision; the table carries a []string so it is visible in the table rather than discovered at runtime.
• A nested alias is a second command instance, so a stub on the canonical subtree is not on its copy. pro sso-settings-cert cert needed movedKeySpellings to reach it — the one live case, and TestMovedInvocationsReachTheNestedAliasSpellings t.Fatals if the overlap ever disappears rather than passing vacuously.

Verified none of the 42 keys shadows a live command and every one of the 43 replacements ships, both as a test.

⚠️ IMPORTANT (6) — the warning is asserted

You were right that replacing the body with a no-op left the package green; I checked before and after. TestWarnIfDeprecatedNameNamesTheReplacement pins the exact text for a deprecatedNames entry and a nestedAliases entry and that a live name says nothing; TestEveryDeprecatedNameWarns sweeps all 100. The no-op mutation now fails both.

💡 NICE-TO-HAVE (7) — dead code

GroupPathsByCollection deleted. The six tests run against GroupPathsByTagAndCollection with untagged paths, which is the path half of the hybrid and the rule they were written for — so they pin the shipped rule instead of a retired one, and one of them is now boundKey's guard.

💡 NICE-TO-HAVE (8) — advance signal

.github/workflows/expiry-notice.yaml, Mondays 08:00 UTC, arming a test that fails inside a 60-day window and lists everything to delete. Separate from CI so it never blocks a pull request, since Go has no warning level for a test. TestDeprecatedNamesNoticeWindowOpensSixtyDaysAhead exercises the window the same way the existing test exercises the deadline, and the notice goes quiet after the date so there is one failure per cause rather than two.

💡 NICE-TO-HAVE (9) — the missing alias

pro jcds-files added, and pro jcds promoted from an expiring deprecation alias to a curated one — the reverse would have left the split half permanent and its parent expiring.

That surfaced a defect: both tables named jcds, applyDeprecatedNames runs before applyAliases, and neither deduped, so the alias printed twice in --help and twice in the catalog. appendNewAliases fixes it, with TestNoCommandAnswersToAnAliasTwice and TestNoAliasShadowsASiblingName walking the whole tree.

---

Two existing guards needed to learn about this, and both are stronger for it. The invocation sweep gains a fourth shape, refused, deliberately not folded into gone: a row moving back from refused to gone is the pointer being lost, which is the whole value of the migration table, and a resolution-only check reads the two as identical. 43 rows moved leaf->gone → leaf->refused, and leaf->refused joins the transitions asserted present so a regenerated baseline cannot make a failure disappear. TestEveryLeafRefusesAnUndocumentedPositional exempts the stubs — their arity is not a contract, and clamping them would answer pro csa delete 5 with "takes no positional arguments" instead of naming where the operation went — against a count derived from the table, so a stub that stops being registered fails rather than being quietly skipped.

make test (34 packages, -race), make lint, make verify-generated, make verify-gateway-coverage and make verify-site all pass. Wire checks were run on an EU environment-scoped credential; pro rules list --baseline-id cis_lvl1 was re-run as the stale-spec canary and still sends the kebab-case parameter.

[ktn-jamf]

Re-review of 1ee814f (round 2)

All nine round-1 findings are fixed, and I verified each one independently rather than on the strength of the reply. The CRITICAL fix is exactly right: I re-ran my own simulation of the naming pass over specs/JamfProAPI.yaml against the new groupTag, and all seven merged roots now take the tag of their own root collection. pro users apply reads "Create or replace a user by name" and pro patch-policies list reads "Retrieve Patch Policies". The two self-reported extras — boundKey for untagged paths, and the mostFrequentTag tie-break — are both real improvements I had not asked for.

One new CRITICAL, and it is not from the fix commit. It is in the PR as a whole and I missed it in round 1. Seven write operations that main ships are dropped by this PR at generate time, with a warning and nothing else. Three of them have no replacement command, so the capability is gone. TestParseMonolith_LosesNoEndpoint passes throughout, because it measures parsing and the loss happens after it.

Warning

Round-1 findings are all clear. Do not merge on that alone — finding (1) below is a capability regression against main, verified command by command on both binaries.

Rating: 3/5

Prior findings status (9)

#	Location	State	Notes
(1)	generator/parser/taggroup.go:118	✅ Fixed	groupTag names a merged root from its own root collection, most-frequent tag as fallback, tie broken lexicographically. My independent simulation gives the correct tag for all 7 merged roots. pro users and pro patch-policies restored, and every verb's Short now matches its endpoint
(2)	generator/parser/monolithparse.go:56-98	✅ Fixed	soleBaseTagOf refuses a path carrying two base tags, naming both. firstTagOf is gone from the tree. Non-vacuous: my suite run caught the lane's own mutation of it failing TestSoleBaseTagOfRefusesAPathCarryingTwoTags
(3)	docs/guides/platform-api-ga.md	✅ Fixed	Seven of eight stale names gone; the three remaining static-computer-groups mentions are correct past-tense narrative. The refused count now reads 59, and commands -o json on the built binary returns exactly 59
(4)	leaf-to-group conversions	✅ Fixed	pro csa token -o json exits 2 with a JSON error naming pro csa token get; a bare invocation still prints help at 0. Changed() on the root persistent flags is read rather than the value, so a profile default-output does not over-trigger
(5)	the 43 moved invocations	✅ Fixed	movedInvocations, 42 entries covering 43 invocations (one entry names the two that collapse onto pro enrollment list). pro schedulers triggers now names pro scheduler-jobs triggers
(6)	internal/commands/deprecated_names.go:401	✅ Fixed	TestWarnIfDeprecatedNameNamesTheReplacement and TestEveryDeprecatedNameWarns exist in moved_invocations_test.go and assert the exact stderr text for a deprecatedNames entry, a nestedAliases entry, the expiry date, and silence for a live name. A no-op body cannot pass them
(7)	generator/parser/pathgroup.go	✅ Fixed	GroupPathsByCollection deleted; the six tests now exercise GroupPathsByTagAndCollection with untagged paths, which is the shipped rule
(8)	alias expiry signal	✅ Fixed	.github/workflows/expiry-notice.yaml, Mondays 08:00 UTC, armed by JAMF_CLI_EXPIRY_NOTICE and separate from CI so it cannot block a pull request
(9)	internal/commands/aliases.go	✅ Fixed	pro jcds-files resolves. appendNewAliases deduped the double jcds registration, and pro jamf-cloud-distribution-service --help lists the alias once

Active findings

❌ CRITICAL (1) (correctness) — seven write operations main ships are dropped at generate time, three with no replacement

generator/parser/parser.go (the duplicate-operation-name drop), surfaced at generator/parser/monolithparse_test.go

make generate prints, and exits 0:

  Warning: duplicate operation name "scope-by-id" — dropping POST /v2/computer-prestages/{id}/scope
  Warning: duplicate operation name "scope-by-id" — dropping PUT /v2/computer-prestages/{id}/scope
  Warning: duplicate operation name "create" — dropping POST /v2/enrollment-customizations
  Warning: duplicate operation name "delete" — dropping DELETE /v2/enrollment-customizations/{id}
  Warning: duplicate operation name "update" — dropping PUT /v2/enrollment-customizations/{id}
  Warning: duplicate operation name "scope-by-id" — dropping POST /v2/mobile-device-prestages/{id}/scope
  Warning: duplicate operation name "scope-by-id" — dropping PUT /v2/mobile-device-prestages/{id}/scope

I built both revisions and diffed the dropped-operation lists: origin/main drops 32, this PR drops 34, and the seven above are new in this PR while five others main dropped are now kept. So this is not the pre-existing 32 under a new spelling.

All seven are recorded in the PR's own baseline fixture as endpoints the CLI shipped before, generator/parser/testdata/endpoints-before-path-grouping.tsv (first four columns of each row):

computer-prestage-scopes	create-scope	POST	/v2/computer-prestages/{id}/scope
computer-prestage-scopes	update-scope	PUT	/v2/computer-prestages/{id}/scope
enrollment-customizations	create	POST	/v2/enrollment-customizations
enrollment-customizations	delete	DELETE	/v2/enrollment-customizations/{id}
enrollment-customizations	update	PUT	/v2/enrollment-customizations/{id}
mobile-device-prestage-scopes	create-scope	POST	/v2/mobile-device-prestages/{id}/scope
mobile-device-prestage-scopes	update-scope	PUT	/v2/mobile-device-prestages/{id}/scope

Command by command, on the two built binaries:

capability	origin/main	this PR
create an enrollment customization	pro enrollment-customizations create — "Create an Enrollment Customization"	gone. pro enrollment-customization create is "Create an LDAP Panel", POST /v1/enrollment-customization/{id}/ldap
update one	update — "Update an Enrollment Customization"	gone. update is "Update a single LDAP Panel"
delete one	delete — "Delete an Enrollment Customization with the supplied id"	gone. delete is "Delete a single Panel", DELETE /v1/enrollment-customization/{id}/all/{panel-id}
add devices to a computer-prestage scope	pro computer-prestage-scopes create-scope	gone. Only scope, scope-by-id (both GET) and delete-multiple remain
replace a computer-prestage scope	update-scope	gone
the same two for mobile-device prestages	pro mobile-device-prestage-scopes create-scope / update-scope	gone

I grepped the whole of internal/commands for a replacement. /v2/enrollment-customizations appears only as a list and name-resolution path, never a POST; both prestage /scope paths appear only as GET, plus a read-only ScopePath in pro_resources.go for backup. There is no other command.

The apply built on top of the displaced verbs is worse than the loss. internal/commands/pro/generated/enrollment_customization.go:

id, err := resolveNameToIDForApply(reqCtx, ctx.Client, "/v2/enrollment-customizations", "displayName", "id", name, noInput)
updatePath := strings.Replace("/v1/enrollment-customization/{id}/ldap/{panel-id}", "{panel-id}", url.PathEscape(id), 1)
resp, err := ctx.Client.Do(reqCtx, "PUT", updatePath, bytes.NewReader(data))

pro enrollment-customization apply resolves a customization name to a customization id, substitutes it into {panel-id}, and PUTs the document at an LDAP panel path — with {id} never substituted, so the request URL carries a literal {id}. That is the pro managed-software-updates-plans apply defect this PR's own description says it removed, reproduced on a different resource: list the collection, resolve a name to an id, then write the document at something else.

Why the guards do not see it, and this is the part worth reading. TestNoPlainVerbLeavesItsResource did fire on the three enrollment-customization verbs. They sit in its allowlist, each with a reason:

// The enrollment-customization panel families: /v1/enrollment-customization
// (singular) under a group rooted at /v1/enrollment-customizations
"POST /v1/enrollment-customization/{id}/ldap":             "a panel under {id}; the singular and plural roots are one tag",
"PUT /v1/enrollment-customization/{id}/ldap/{panel-id}":   "a panel under {id}; the singular and plural roots are one tag",
"DELETE /v1/enrollment-customization/{id}/all/{panel-id}": "a panel under {id}; the singular and plural roots are one tag",

The allowlist's own preamble frames the class as a naming question deferred to a later change: "Splitting them would need a rule about parameterised sub-paths and a judgement about which root is the resource, and neither is this change." That framing is reasonable for a name. It is not reasonable for what actually happens, and the gap between the two is the finding: deciding which of two candidates takes the name create does not delete the loser — but here the loser is deleted, by the duplicate-name drop, and the allowlist entry says nothing about that because nothing told its author. The reason reads as "the panel keeps the name"; the effect is "the customization CRUD stops existing".

The other two guards are silent for their own reasons:

• The four prestage scope writes are not in the allowlist and were never considered, because the operation that survives on /scope is scope-by-id, a GET, and scope-by-id is not a plain verb. Nothing in that test looked at them.
• TestParseMonolith_LosesNoEndpoint compares the parse result against the fixture. The duplicate-name drop runs later, in Generate. So "704 → 704, zero gained or lost" is true of parsing and not of the shipped surface, which is how the claim reads and how I read it in round 1.

The sub-resource split cannot rescue the panel case either, and correctly does not try: ldap carries only GET and POST on itself, with the writes on ldap/{panel-id}, so the PUT/PATCH/DELETE rule declines to split. The rule is right. What is missing is anything that notices the displaced operations were dropped rather than renamed.

Failure scenario. A Jamf admin's provisioning script runs pro enrollment-customizations create --from-file customization.json to stand up an enrollment customization, then pro computer-prestage-scopes update-scope <id> --from-file serials.json to scope a prestage. After this PR, the first name resolves through the deprecation alias to a command that creates an LDAP panel on a customization that does not exist yet, and the second no longer exists. Neither loss is in CHANGELOG.md, because nothing reported it.

Suggested fix. Two parts, and the first is the one that matters.

1. Make the duplicate-name drop a failure, not a warning, or at minimum resolve it in favour of the operation that addresses the resource's own root. A resource's own create, update and delete must outrank a sub-path's. resolveNoParamConflicts already renames a colliding verb rather than dropping one, and that is the behaviour wanted here: the panel operations should be renamed (create-ldap-panel, or a nested pro enrollment-customization ldap create) and the customization CRUD kept.
2. Extend TestParseMonolith_LosesNoEndpoint to run after Generate's drops, so the fixture measures the shipped surface. That is the guard the PR describes; it is one pipeline stage short of being it.

Also make an allowlist entry state what happens to the loser. An entry in plainVerbsOutsideTheirRoot currently records why a verb may keep a foreign noun; it should not be satisfiable while the operation it displaced is being dropped. Asserting that every allowlisted verb's displaced sibling still ships would have failed here.

Fixed when. pro enrollment-customization create|update|delete address the customization again, the four prestage scope writes ship, make generate reports no dropped operation that the baseline fixture records as previously shipped, and a test fails if one is dropped.

Review coverage and scope

Round 2. Incremental diff 56bc41c..1ee814f, one commit, 25 files. Clean fast-forward, no force-push.

• Prior findings — all nine verified independently, not from the reply. Simulation of the new naming rule over the committed document; both binaries built and compared; counts reconciled against the code (96 deprecatedNames + 4 nestedAliases = the corrected "100 resolve"; 42 entries covering 43 invocations; 43 refused rows in the sweep baseline)
• Baseline integrity — pro-invocations-before-nesting.tsv is 1355 rows and its old-shape column is byte-identical to 56bc41c. Only the expected-new column moved, so the fixture was not weakened to fit. endpoints-before-path-grouping.tsv is untouched
• Correctness of the new code — groupTag, boundKey, soleBaseTagOf, movedInvocations, formerLeafGroups, appendNewAliases
• Silent failure — dispatched. No findings: every "skip silently" branch in the new tables is paired with a named test that fails on a stale entry, and the Changed("output") claim was traced to the assignment site at root.go:784, confirming a profile default-output cannot over-trigger the refusal
• Regression against main — this is what found finding (1). Dropped-operation lists diffed across main, 56bc41c and 1ee814f; every difference traced to a command on both binaries
• Gates — go test ./... green on a clean tree, make verify-generated, make verify-gateway-coverage and make verify-site all pass
• Documentation currency — CLAUDE.md gained a "Where to Make Changes" row per new mechanism, each with its reason; CHANGELOG counts corrected 40→38 and 102→100
• [na] Performance, frontend — unchanged
• [?] Test-quality mutation lane — dispatched for round 2 and had not returned when this was written. Its round-1 finding is fixed and I verified the replacement guards by inspection: both tests capture stderr and assert the literal message, so the no-op mutation that survived round 1 cannot pass them. I also observed the lane's own in-flight mutation of soleBaseTagOf failing its guard, which confirms finding (2)'s test is not vacuous

Findings not carried forward: none. Findings 1 to 9 are all ✅ Fixed.

Confidence: finding (1) — 5/5. Verified on two built binaries, with the dropped-operation lists diffed across three revisions, the PR's own baseline fixture quoted, and no replacement command found anywhere in internal/commands.

What is done well

The response is the best fix round I have reviewed on this repo. Three things stand out.

The fix went to the mechanism rather than the symptom. groupTag is the rule I suggested, and the tie-break is deterministic by name with the reason stated: "an unstable answer would move a resource's name between two runs of make generate with no change to the document."

Both self-reported extras are real. boundKey is the same class as the CRITICAL — a shared key merging things nothing declared to be together — found by asking where else the pattern occurred rather than by fixing what was reported. Without it /v1/health-check and /v1/health-status would have come out as one resource.

The guards were mutated before being claimed sound, and each carries a non-vacuity t.Fatal. TestMergedTagGroupsAreNamedByTheirRootCollection fails if no merged group is left to exercise, so a future spec drop cannot make it pass by making it irrelevant. The invocation sweep gained a fourth shape, refused, deliberately not folded into gone — because a row moving back would be the migration pointer being lost, which a resolution-only check reads as identical.

Finding (1) is not a criticism of any of that. It is a hole in a guard whose name reads as covering it, in a pass that predates this round.

---

Re-reviewed with the pr-review skill (Claude Opus 5). Round-1 findings were re-verified against the built binary and a re-run of the naming simulation; finding (1) was found by diffing the generated command surface against origin/main.

[neilmartin83]

Finding (1) is right and it is fixed. Two root causes, not one, and the fix restores nineteen write operations rather than the seven you named — twelve of them main never shipped either, all losers of the same collision.

Pushed as 666ba61.

The two causes

enrollment-customization. The tag covers a singular root carrying four panel families under {id} and the plural /v2/enrollment-customizations carrying the customization's own CRUD. Nine operations wanted three names. disambiguateSameTerminalOps declines the group (terminals differ), resolveNoParamConflicts declines it (every path is parameterised), and dedupeOperations then dropped eight of the nine. Your reading of the apply was exact.

The prestage scopes. GET, POST and PUT all sit on /v2/computer-prestages/{id}/scope, against a collection-level GET .../scope that keeps scope. buildDisambiguatedName produced scope-by-id for all three — their paths are identical, so it has nothing left to distinguish them with — and the second and third were dropped. main reached create-scope/update-scope through the same-path branch, which only fires when the collision is on group[0]'s own path, and after the rename group[0] is the collection-level GET.

The fix

qualifyDuplicateVerbsOutsideTheRoot (generator/parser/parser.go, last in buildResourceShell) resolves a surviving collision in favour of the operation that addresses the resource's own root or root/{id}, and qualifies the loser by the last non-param segment of its own path — ldap-create, sso-update, all-delete, dashboard-delete. Noun-first rather than verb-first, matching buildDisambiguatedName's own convention and because delete-all reads as "delete everything" for an endpoint that deletes one panel.

It touches writes only, and that is load-bearing: a colliding GET is left to dedupeOperations, whose collection-path preference is what resourceGetDetailPathOverrides depends on. My first version renamed the loser there too, which split pro computer-inventory get in two and took the override's default detail path with it. TestQualifyDuplicateVerbsLeavesGETsToDedupe pins that.

disambiguateSameTerminalOps gains a method-prefix fallback for the same-path case. That reproduces main's create-scope/update-scope exactly, so no movedInvocations entry was needed — the four rows in pro-invocations-before-nesting.tsv go back from gone to leaf.

Zero new drops against main: 34 → 15 warnings, and the diff origin/main → this commit restores 19 and adds none.

Your suggestion 2, and why the guard is a pinned list instead

TestParseMonolith_LosesNoEndpoint's fixture records the parse, so all 34 pre-existing drops appear in it as previously-shipped — including ones that are fine (POST /v1/packages/{id}/upload losing to a multipart sibling) and one that is deliberate (GET /v4/computers-inventory-detail/{id}, which resourceGetDetailPathOverrides fuses into get and still sends). "No dropped operation the fixture records" would therefore have demanded 34 fixes, not 7.

So TestTheDroppedOperationSetIsPinned (generator/parser/dropped_operations_test.go) does two things. It runs dedupeOperations — the stage the old test is short of — and pins the resulting set to testdata/dropped-operations.tsv, so a new drop is a diff to explain rather than a line in make generate's output. And it asserts the invariant your finding actually exposes: a dropped operation may not lose its name to an operation outside the resource's root. The drop set is computed as the difference between what dedupeOperations is given and what it keeps, so the rule lives in one place and cannot drift.

That invariant found an eighth case immediately — app-request-form-input-fields, where the collection-level PUT .../form-input-fields keeps update and the item PUT .../{id} is dropped, so update replaces the whole list. Both address the root, so it is a naming judgement (update <id> against a bulk update-all) rather than the precedence rule, it is pre-existing on main, and it is recorded in the fixture with that reasoning rather than fixed here. The invariant is scoped to "root loses to a sub-path", which is the half that is always wrong.

The allowlist

The three plainVerbsOutsideTheirRoot entries are gone, and TestNoPlainVerbLeavesItsResource caught them as stale on the first run — the panel writes are ldap-create, sso-update, all-delete, not plain verbs. Your point that the entry's reason was true of the name and silent about the loser is recorded where the entries were, and the "displaced sibling still ships" assertion you asked for is TestTheDroppedOperationSetIsPinned's second half.

One thing you did not ask about and I had to add

Fixing the precedence moves the breakage rather than removing it: enrollment-customizations and enrollment-customization-panels both resolve onto the one enrollment-customization, both shipped create/update/delete, and only one can keep the plain verb. So pro enrollment-customization-panels create would resolve, exit 0 and create a customization — the formerLeafGroups shape one level down.

deprecatedNameVerbMoves + guardDeprecatedNameVerbMoves refuse those three under the panels spelling and name the panel command. movedInvocations cannot cover it, being keyed post-alias-resolution where the two spellings are identical. Two details:

• It is wired after guardStrayPositionals, because it fronts the leaf's Args as well as its RunE and that walk installs a validator only on a leaf that has none — running first made pro enrollment-customization create look already-guarded and it accepted a stray positional again. TestEveryLeafRefusesAnUndocumentedPositional caught that.
• It relaxes Args for the old spelling only. Cobra validates Args before RunE, so pro enrollment-customization-panels update <id> <panel-id> — the form the old command took — answered "accepts at most 1 arg(s), received 2" and the pointer was unreachable from the invocation a caller actually types.

The sweep classifies those three as refused from the invocation rather than from the command, since the same leaf serves both spellings; leaf->refused is now 46, and leaf->gone 15.

Wire verification

EU-region Jamf Pro instance, one session, full -vv:

command	wire
enrollment-customization create	POST /v2/enrollment-customizations → 201, id 1188
update 1188	PUT /v2/enrollment-customizations/1188 → 200, description changed
apply --yes	GET …?filter=displayName=="pr377-ecust" → 200, then PUT /v2/enrollment-customizations/1188 → 200 — the resolve-then-write-elsewhere defect is gone
delete 1188 --yes	DELETE /v2/enrollment-customizations/1188 → 204
text-create 1188	POST /v1/enrollment-customization/1188/text → 201, panel 258 (dropped on main)
text-update 1188 258	PUT …/text/258 → 200, read back through text
ldap-create 1188	POST …/1188/ldap → 201, panel 259
all-delete 1188 259 / text-delete 1188 258	204 each; all 1188 then returns {"panels": []}
packages manifest-delete <id>	DELETE /v1/packages/1568/manifest → 204 (dropped on main)
computer-prestages create-scope 1 / update-scope 1	POST/PUT /v2/computer-prestages/1/scope → 400 DEVICE_DOES_NOT_EXIST_ON_TOKEN, field: serialNumbers
venafi proxy-trust-store-delete / patch-software-title-configurations dashboard-delete / computer-inventory attachments-delete	route and answer Jamf's own field-attributed 400/404

The two create-scope/update-scope rows are the honest limit of this: that instance holds no ADE devices, so a 2xx is not reachable there. Jamf's own field-attributed 400 establishes that the request routed and the body and versionLock parsed, which is what the drop had removed. -n previews [dry-run] POST /v2/computer-prestages/1/scope with the body.

Also verified: pro enrollment-customization-panels create|update 1 2|delete 1 2 each refuse at exit 2 naming ldap-create/ldap-update/all-delete; the panel reads under that spelling are unchanged; pro enrollment-customization create junk still gets the stray-positional refusal.

make lint 0 issues, full suite green, verify-generated, verify-gateway-coverage, verify-classic-schemas and verify-site all pass. The refused-command count is unchanged at 59.

[neilmartin83]

@ktn-jamf re-review please — finding (1) is fixed in 666ba61, with the detail in the comment above.

Three things worth aiming at, since each is a judgement rather than a mechanical fix:

1. The invariant, not the fix. TestTheDroppedOperationSetIsPinned refuses a dropped operation that addresses its resource's own root while the winner does not. That second clause is deliberate — app-request-form-input-fields has both sides addressing the root — and it is the clause that decides whether the guard is honest or merely narrow.
2. The GET carve-out. qualifyDuplicateVerbsOutsideTheRoot skips GETs so resourceGetDetailPathOverrides keeps working. My first version did not, and it split pro computer-inventory get in two. Worth checking there is no write the same reasoning should exempt.
3. The new refusal. guardDeprecatedNameVerbMoves fronts a leaf's Args as well as its RunE, which is a sharper edge than guardFormerLeafGroups has — the ordering against guardStrayPositionals and classifyArgsErrors is the part I would attack.

Twelve of the nineteen restored operations never shipped on main, so they are new surface rather than a restoration: pro enrollment-customization {sso,text}-{create,update,delete}, ldap-delete, packages manifest-delete, venafi proxy-trust-store-{create,delete}, patch-software-title-configurations dashboard-delete, computer-inventory attachments-delete. Nine of those I exercised on a live instance; the venafi, dashboard and attachment deletes only got as far as Jamf's own field-attributed error, there being nothing on that tenant to delete.

[ktn-jamf]

Re-review of 666ba61 (round 3)

Round 2's CRITICAL is fixed, and the fix is better than what I asked for. I verified the central claim independently: origin/main drops 32 operations, this commit drops 15, and the set difference contains no new drops in either direction beyond restorations. All seven operations I named ship, create/update/delete address the customization again, both prestage scope writes are back under main's own names, and the malformed apply now resolves and writes at /v2/enrollment-customizations/{id} with {id} correctly substituted.

The three points you invited attack on are the right three. Two of them hold. On the third — a write the GET carve-out's reasoning should also exempt — I did not find one, but I found something adjacent and worse, and it has been in the branch since 56bc41c. I missed it in rounds 1 and 2.

pro.go wires two destructive commands by name, the rename staled both names, and neither helper reports it. The generated erase and remove-mdm-profile now ship alongside the hand-written pair they were meant to replace, under v-4-computers-inventory-erase and v-4-computers-inventory-remove-mdm-profile, without the --confirm-destructive gate the hand-written pair carry for bulk.

Warning

Prior finding is clear. Finding (1) is a second, weaker-gated path to a fleet-wide Mac wipe, reachable with --group … --yes, and its confirmation prompt names the wrong action. Finding (2) is smaller but touches this round's own work: a flag after pro silences both the new refusal and the deprecation warning.

Rating: 3/5

Prior findings status

#	Location	State	Notes
(1)	duplicate-operation-name drop	✅ Fixed	Verified independently: main drops 32, this commit 15, zero new. All seven named operations ship; panel writes qualified to ldap-create / sso-update / all-delete; create-scope / update-scope restored on both prestage resources; apply corrected. The new deprecatedNameVerbMoves refusal names the exact replacement per verb and the two-positional old form reaches it

Active findings

❌ CRITICAL (1) (correctness, safety) — the rename staled pro.go's destructive-command wiring, and both helpers fail silently

internal/commands/pro.go:107-108, against removeSubcommand / replaceSubcommand at internal/commands/pro.go:177-203

replaceSubcommand(cmd, []string{"computer-inventory"}, "erase", newComputerEraseCmd(cliCtx))
removeSubcommand(cmd, []string{"computer-inventory"}, "remove-mdm-profile")

Both key on a child name. After this PR the generated operations are named v-4-computers-inventory-erase and v-4-computers-inventory-remove-mdm-profile, so neither key matches, and neither helper says so:

func removeSubcommand(root *cobra.Command, parentPath []string, childName string) {
	parent, _, err := root.Find(parentPath)
	if err != nil { return }
	for _, child := range parent.Commands() {
		if child.Name() == childName { parent.RemoveCommand(child); return }
	}
}                                    // no match → returns silently

func replaceSubcommand(...) {
	...
	for _, child := range parent.Commands() {
		if child.Name() == childName { parent.RemoveCommand(child); break }
	}
	parent.AddCommand(replacement)   // adds even when nothing was removed
}

So the removal is a no-op and the replacement becomes an addition. pro computer-inventory --help on the built binary:

  erase                                      Erase a computer
  remove-mdm                                 Remove the MDM profile from a computer
  v-4-computers-inventory-erase              Erase a computer
  v-4-computers-inventory-remove-mdm-profile Remove a computer's MDM profile

CLAUDE.md states the intent this defeats: "pro.go now replaceSubcommands erase and drops the generated remove-mdm-profile in favour of the hand-written remove-mdm — the hand-written pair target by serial, name or group, confirm the action, honour --dry-run and carry the Find My PIN body, where the generated pair take an <id>. A duplicate subcommand name is not a choice cobra can make."

The two paths are not equally gated. --confirm-destructive exists only in the hand-written files (pro_device_actions.go:276, pro_bulk.go, pro_bulk_commands.go), declared "required for bulk destructive operations". The generated template has no such flag. Its bulk path stops at one --yes-skippable prompt, so

pro comp v-4-computers-inventory-erase --group "All Managed Clients" --yes

wipes every Mac in the group behind a single flag, where pro comp erase --group … --yes still demands --confirm-destructive. The generated command is annotated jamf:destructive (computer_inventory.go:605), so this is not a missing annotation — it is a second gate the hand-written path has and the generated one does not.

The prompt that stands between the operator and the wipe names the wrong action. internal/commands/pro/generated/computer_inventory.go, inside the v-4-computers-inventory-erase command:

2821:  "⚠️  This will delete %d computer-inventory from group %q. Type 'yes' to confirm: "
2918:  "⚠️  This will v-4-computers-inventory-erase computer-inventory %q (id: %s). Type 'yes' to confirm: "

The fleet-wide path says it will delete N "computer-inventory", which reads as removing inventory records rather than wiping devices. The single-device path interpolates the version-leaking operation name into the sentence, so the operator is asked to confirm "This will v-4-computers-inventory-erase computer-inventory". Line 2749's dry-run text has the same wording.

Why no test sees it. TestProWiringNamesCommandsThatShip (internal/commands/deprecated_names_test.go:288) greps pro.go for exactly this class and checks only the parent:

parents := regexp.MustCompile(`(?:add|remove|replace)Subcommand\(cmd, \[\]string\{"([a-z0-9-]+)"\}`)
...
if byName[m[1]] == nil && byAlias[m[1]] == nil {
    t.Errorf("pro.go wires onto parent %q, which `pro` does not ship — the wiring is silently discarded", m[1])
}

The capture group stops at the parent path. computer-inventory ships, so the check passes while both child keys are dead. The test's own comment already reasons about a case the assembled tree cannot judge — a whole-resource removal — and this is the sibling case: for replaceSubcommand the child exists after wiring, because the replacement was added, so the assembled tree cannot tell a successful replace from a stale key either. That is answerable here, unlike the case the comment defers: internal/commands imports internal/commands/pro/generated, so the pre-wiring parent can be built directly and its children inspected.

This is the same class the PR added TestEveryResourceKeyedOverrideNamesALiveResource for on the generator side — a name-keyed table whose key stops matching, silently. pro.go's wiring is a name-keyed table and got the parent half of that guard only.

Failure scenario. An admin knows pro comp erase requires --confirm-destructive for a group and has a CI policy that greps for that flag. Shell completion now offers v-4-computers-inventory-erase beside erase; either the admin picks it, or a script generated from commands -o json does. --group "All Managed Clients" --yes then wipes the fleet with the policy flag never involved, after a prompt that said it would "delete computer-inventory".

Suggested fix.

1. Make the two wiring calls match the current names, and stop them failing silently: removeSubcommand and replaceSubcommand should report or fail when the named child is absent. A t.Fatal-backed test is enough — these run once at startup, so a hard failure is not a runtime risk.
2. Extend TestProWiringNamesCommandsThatShip to capture the child name as well and assert it exists under that parent in the generated tree before wiring, built from generated.NewComputerInventoryCmd(cliCtx) rather than from the assembled root. That distinguishes a successful replace from a stale key, which the assembled tree cannot.
3. Prefer keying the wiring on something the rename cannot move. The endpoint is stable where the name is not: POST /v1/computer-inventory/{id}/erase did not change spelling in this PR. A path-keyed removal would have survived it, and would survive the next rename.
4. Fix the bulk prompt and dry-run wording in resourceTemplate so an x-action names its own action rather than "delete", and does not interpolate the raw operation name into an English sentence.

Fixed when. pro computer-inventory --help lists erase and remove-mdm and neither v-4- twin; a stale child key in pro.go fails a test rather than being discarded; and no confirmation prompt for an x-action says "delete" or contains a generated operation name.

⚠️ IMPORTANT (2) (correctness) — a global flag after pro defeats the new refusal and silences the deprecation warning for all 100 aliases

internal/commands/moved_invocations.go:166-208 (resourceTokenAfter), used by guardDeprecatedNameVerbMoves and by warnIfDeprecatedName (internal/commands/deprecated_names.go:438-448)

resourceTokenAfter scans raw os.Args for the first token after the product name that does not start with -. It does not know that a flag takes a value, so any flag placed between pro and the resource makes it return the flag's value as the resource token. Cobra does not require global flags before the subcommand, and -p is the documented way to select a profile, so pro -p <profile> <resource> … is an ordinary shape.

Verified on the built binary. The refusal:

$ jamf-cli pro enrollment-customization-panels update 1 2
message: `pro enrollment-customization-panels update` no longer names this operation …
hint:    run `jamf-cli pro enrollment-customization ldap-update`

$ jamf-cli pro --profile ci-svc enrollment-customization-panels update 1 2
message: accepts at most 1 arg(s), received 2          # same for -p ci-svc

typed != called, so the Args relaxation is skipped and the strict validator guardStrayPositionals installed runs instead. The two positionals the old command took are rejected before RunE, so the refusal never executes and the operator gets exactly the bare cobra arity error this guard exists to replace.

The warning has the same bug, and it costs more because it reaches all 100 retired names:

$ jamf-cli pro enrollment-customizations create --scaffold
warning: `enrollment-customizations` is a deprecated name … Use `pro enrollment-customization`.

$ jamf-cli -p ci-svc pro enrollment-customizations create --scaffold
warning: `enrollment-customizations` is a deprecated name … Use `pro enrollment-customization`.

$ jamf-cli pro -p ci-svc enrollment-customizations create --scaffold
(no warning)

So the migration signal — the mechanism the whole 2027-03-09 deprecation window rests on, and the one round 1 asked to be made testable — is silently absent for a caller who puts the flag after the product. A CI job in that shape gets no warning for the entire window and then breaks on the expiry date, which is the outcome the aliases exist to prevent.

Suggested fix. Do not re-derive the invocation from os.Args. Cobra has already parsed past every flag by the time either hook runs, so take the resource token from the resolved command's own lineage (cmd.Parent().Name(), walking up to the product) or from cmd.Flags().Args(). Re-deriving flag arity from spelling cannot be made correct — a value that does not start with - is indistinguishable from a positional at that level.

Fixed when. A global flag with a non-dash value, placed anywhere between the product token and the resource token, changes neither which invocation guardDeprecatedNameVerbMoves believes was typed nor whether warnIfDeprecatedName fires. Worth a test row per placement, since the plain form already passes.

💡 NICE-TO-HAVE (3) — a third version-leaking name, and a plain verb whose noun is now ambiguous

internal/commands/pro/generated/mobile_device_prestages.go

pro mobile-device-prestages v-2-scope-delete-multiple is the third and last v-N- name in the binary (main has none). The collision behind it is genuine — /v3/mobile-device-prestages/{id}/attachments/delete-multiple and /v2/mobile-device-prestages/{id}/scope/delete-multiple are different operations — so a rename is required. Only the name chosen is the problem, and this round introduced a better convention for exactly this shape: under qualifyDuplicateVerbsOutsideTheRoot's rule the pair would read attachments-delete-multiple and scope-delete-multiple.

Two reasons it is worth taking with that convention rather than leaving:

• A version segment in a command name is the thing this PR exists to remove. CLAUDE.md's own history is that a name carrying version information is how the CLI came to send /v3 while /v4 was served.
• The winner is the attachments operation, so pro mobile-device-prestages delete-multiple deletes attachments while its computer-prestage sibling's delete-multiple removes scope. delete-multiple is not in plainVerbs, so TestNoPlainVerbLeavesItsResource does not look at it. Two sibling resources answering the same verb with different nouns is the round-2 finding in miniature.

Not blocking: both operations ship and both send the right path.

Review coverage and scope

Round 3. Incremental diff 1ee814f..666ba61, one commit, 20 files, +3305/−446.

• Prior finding — verified independently rather than from the reply. Built origin/main and this head, ran the generator on both, diffed the dropped-operation lists (32 → 15, zero new), and compared the command surface verb by verb on both binaries. apply's corrected paths read off the generated source
• The new refusal — exercised on the binary: pro enrollment-customization-panels create|update 1 2|delete 1 2 each refuse at exit 2 naming ldap-create / ldap-update / all-delete; the two-positional old form reaches the refusal rather than being clamped by Args; pro enrollment-customization create junk still gets the stray-positional refusal; the old plural alias still scaffolds the customization body
• Correctness and safety of the wiring — this is what found finding (1). pro.go's helpers read in full, both keys traced against the generated names, both confirmation paths compared flag by flag
• Naming consistency — swept the whole binary for version-leaking command names; three exist, all new in this PR
• Silent failure — dispatched on the new pass and the new guard, including your three invited targets
• Test quality — mutation-verified, six items, all confirmed and none vacuous. A no-op qualifyDuplicateVerbsOutsideTheRoot fails its own test on six assertions and makes TestTheDroppedOperationSetIsPinned list exactly the 15 root-losing operations. Removing the GET carve-out fails TestQualifyDuplicateVerbsLeavesGETsToDedupe, renaming GET /v4/computers-inventory-detail/{id} and splitting the fusion. Both halves of the pinned drop-set guard fire independently — the second clause does not swallow the first. Removing the disambiguateSameTerminalOps method fallback reproduces the four-operation prestage regression. Swapping the guard ahead of guardStrayPositionals fails TestEveryLeafRefusesAnUndocumentedPositional with the exact message you predicted. readPinnedDrops fatals on an empty fixture
• create-scope / update-scope against main — compared byte for byte: identical Use, Short, Long, annotations, path, method and version-lock body handling, plus a --name lookup main's separated resource did not have. main shipped them under a separate computer-prestage-scopes resource, which the sub-resource merge folded in at 1ee814f; this commit restores both
• Baseline fixture — the was column is byte-identical from its introduction through 1ee814f to this head; only the expected-new column moves. Four rows go gone → leaf as claimed, and three more go leaf → refused, which the new deprecatedNameVerbMoves entries account for. TestEveryFormerInvocationKeepsItsShape passes on 1355 invocations and asserts all three transition kinds non-empty
• Documentation currency — CLAUDE.md gained a row for the new pass; CHANGELOG updated
• [na] Performance, frontend — unchanged
• The GET carve-out you flagged — no write needs the same exemption, and the reason is structural rather than a survey: resourceCreateOpOverrides and resourceUpdateTokenOpOverrides match on (Path, Method) rather than on Name (generator/parser/parser.go:213-263), so renaming a write earlier in the pipeline cannot break either. resourceGetDetailPathOverrides is the only fusing override and it is GET-keyed, which is exactly what the carve-out protects
• qualifyDuplicateVerbsOutsideTheRoot's candidate-exhaustion fallback — if the qualified name were also taken, the loser's name goes unresolved, but it is a non-root write, so TestTheDroppedOperationSetIsPinned surfaces it as a set diff rather than letting it vanish

I did not re-review the round-1 and round-2 findings; all twelve were verified fixed in their rounds and nothing in this diff touches them.

Confidence: finding (1) — 5/5. Verified on two built binaries, with the wiring helpers, both keys, the flag sets, the prompt strings and the half-guard all read directly. Findings (2) and (3) — 5/5, both reproduced on the binary.

What is done well

The fix went past what I reported in the direction that matters. I named seven operations; the two root causes turned out to account for nineteen, twelve of which main also dropped, and the fix takes all of them rather than the seven with a reviewer's name on them.

The correction to my suggestion 2 is right and I was wrong. I proposed failing on "any dropped operation the baseline fixture records as previously shipped". That fixture records the parse, so it holds all 34 pre-existing drops including benign ones and one that is deliberate — the suggestion would have demanded 34 fixes to satisfy a rule about 7. Pinning the drop set after dedupeOperations and asserting the narrower invariant is the better shape, and the invariant found an eighth case immediately.

Two judgements I would have got wrong. Restricting the new pass to writes, so resourceGetDetailPathOverrides keeps fusing pro computer-inventory get — with TestQualifyDuplicateVerbsLeavesGETsToDedupe pinning the version that failed. And scoping the invariant to "root loses to a sub-path" rather than to every drop, which is what keeps app-request-form-input-fields an honest recorded judgement instead of a forced fix.

Finding (1) is not a criticism of any of that. It is an older name-keyed table that the rename staled two rounds ago, with a guard that checks the parent and not the child.

---

Re-reviewed with the pr-review skill (Claude Opus 5). The prior finding was re-verified by generating on origin/main and this head and diffing the dropped-operation sets; finding (1) was found by sweeping the shipped command surface for names the rename moved.

[neilmartin83]

@ktn-jamf — round 3 addressed in d336a00. Both findings fixed, the NICE-TO-HAVE taken, and finding (1)'s root cause turned out to be one layer below the wiring.

(1) CRITICAL — the destructive twins

Your diagnosis of the symptom was exactly right and the cause is one step further back: both keys matched, they just matched the wrong operations.

POST /v1/computer-inventory/{id}/erase and POST /v4/computers-inventory/{id}/erase are the same endpoint at two path shapes — v4 renamed the collection segment from computer-inventory to computers-inventory — so deduplicateVersionedOps, which keys on the version-stripped shape, saw two unrelated endpoints and kept both. Upstream declares the v1 pair deprecated: true with an x-deprecation-date, and the gateway withdrew them.

The deprecated pair won the plain names (same segment count, same method, document order decided it), so:

• pro.go's replaceSubcommand(…, "erase", …) and removeSubcommand(…, "remove-mdm-profile") matched and removed the deprecated operations;
• the served v4 pair fell through to buildDisambiguatedName, which named it from the whole path difference including the version segment;
• and those two paths are the only ones in the document whose collection segment is singular, so they alone were deciding the resource's own name.

Fixed at the root: the v1 pair is dropped at ingest (droppedPaths, generator/parser/pathgroup.go), which is the inventory-preload case with the noun changed — the entry beside it already records "the same endpoint at two path shapes, and deduplicateVersionedOps matches on the shape, so it sees two unrelated endpoints and keeps both", down to "left in, they were actively worse than absent: the v1 paths took the plain names while being refused on a gateway profile". main shipped neither (both were removeSubcommand'd there as whole resources), so no capability moves.

$ bin/jamf-cli pro computer-inventory --help
  attachments-delete          Remove attachment
  erase                       Erase a computer          # hand-written
  remove-mdm                  Remove the MDM profile…   # hand-written

No v-4- twin, and pro comp erase --help still carries --confirm-destructive.

Your four suggestions

1. Names match, and a stale key is no longer silent. All three helpers record every miss at the moment of the failed lookup and TestProWiringResolvesEveryNameItUses fails on a non-empty record. It found two more dead keys immediately — apply on the former jamf-protects and get-by-name on jamf-protect-deployment-tasks (#45), re-keyed onto jamf-protect earlier in this branch, where neither name is generated: the resource has no nameResolutionPath so no apply is synthesized, and the tasks lookup ships as pro jamf-protect tasks. Both removed.

2. Child names are asserted, and against the pre-wiring tree in the sense that matters. Rather than rebuilding the generated parent, the miss is recorded before any replacement is added — so the record distinguishes a successful replace from a stale key, which is the thing you correctly said the assembled tree cannot. It also covers the whole-resource removals the source grep skips (removeSubcommand(cmd, []string{}, …)), which is the case that guard's comment deferred. TestStaleProWiringIsRecorded drives one stale child key and one stale parent path through the real helpers and requires both recorded, plus a resolving key not recorded, so the passing case is not a broken accumulator.

3. Keying on the endpoint — I did not take this, and the reason is the noun. The two twins are …/computer-inventory/{id}/erase and …/computers-inventory/{id}/erase; an endpoint key is stable against a name rename and not against this, so it would have needed both spellings listed anyway, and the same edit would have been required. What it is replaced by is a pin on the property itself: TestTheDestructiveComputerActionsAreOneOperationEachOnTheServedVersion (generator/parser) requires computer-inventory to hold exactly one erase and one remove-mdm-profile, each POST /v4/computers-inventory/{id}/…. It fails if an ingest brings the deprecated shape back, and if the surviving operation moves off the name pro.go keys on. Reverting the two droppedPaths entries fails it; it is the only guard that does — see the mutation table.

4. The wording is fixed, and fixing it exposed a correctness bug in the same block. The destructive bulk block is shared by a plain delete and by a destructive x-action, and it hardcoded the method as well as the prose:

resp, err := ctx.Client.Do(reqCtx, "DELETE", delPath, nil)   // on /v2/mobile-device-groups/{id}/erase

So pro mobile-device-groups erase --from-file ids.txt sent DELETE to a POST-only endpoint and printed Deleted … per entry. Present in v1.28.0 and earlier; eight POST actions had it, of which that one and the two prestage delete-multiples ship. It sends {{ actionMethod . }} now.

For the prose: a DELETE still reads This will delete N …; anything else quotes its own action rather than being conjugated, because there is no English verb for remove-mdm-profile and inventing one is how a prompt comes to describe a different action from the one it performs.

⚠️  This will run "erase" on 12 mobile-device-groups from group "…". Type 'yes' to confirm:
[dry-run] Would run "erase" on mobile-device-group "…" (id: 4)
--from-file string   Path to file listing IDs or names to run "erase" on …

(2) IMPORTANT — the flag placement

Fixed, and you were right that arity cannot be re-derived from spelling: it is now asked of the command's own flag set, by cobra's stripFlags rules — including cobra's treatment of an unknown long flag as value-taking, since the guard has to agree with cobra about where the resource token is rather than about what a valid command line looks like.

One trap worth recording, because it reintroduced the same bug one layer down: cmd.Flags() holds only the command's own flags until cobra's unexported mergePersistentFlags has run, so --quiet and -n looked unrecognised, were treated as value-taking, and ate the resource token — a test row caught it. visibleFlags assembles the set from the command and every ancestor's persistent set.

Verified on the built binary, all placements:

$ jamf-cli pro -p pro-nmartin enrollment-customization-panels update 1 2
  "exitCode": 2,
  "hint": "run `jamf-cli pro enrollment-customization ldap-update`",
  "message": "`pro enrollment-customization-panels update` no longer names this operation…"
$ jamf-cli pro --profile pro-nmartin enrollment-customization-panels create 1 2   → ldap-create
$ jamf-cli pro -o json -p pro-nmartin enrollment-customization-panels delete 1 2  → all-delete
$ jamf-cli pro -p ci-svc enrollment-customizations create --scaffold
warning: `enrollment-customizations` is a deprecated name for `enrollment-customization`…

TestProductTokenAndResourceToken gained a row per placement: -p value, --profile value, --profile=value, -o json, -ojson, boolean -n, the shorthand run -nq, several flags mixed, an unknown long flag, --, and -p with nothing after it.

(3) NICE-TO-HAVE — taken, with your convention

pro mobile-device-prestages v-2-scope-delete-multiple is gone and so is the bare delete-multiple. Where a collision is between two sub-paths and neither addresses the resource's own root, no operation has a claim on the plain verb, so each is qualified by the thing it acts on — attachments-delete-multiple and scope-delete-multiple, which is the pair you named. Both old invocations (pro mobile-device-prestages delete-multiple and pro mobile-device-prestage-scopes delete-multiple, one path after alias resolution) refuse naming both replacements. The sibling ambiguity goes with it: there is no longer a bare delete-multiple answering with a different noun on one prestage resource than the other.

Two details that took a second pass, both from qualifying by the wrong segment:

• The owning noun is the last segment strictly between the root and the shared terminal, and it must be a literal. A parameter there means the members differ only by an item selector, which is no noun — without that clause /v1/pki/certificate-authority/{id}/der reported certificate-authority and pro certificate-authority der/der-by-id became active-der/certificate-authority-der.
• Both sides are version-stripped, because a PathGroup root keeps a leading preview (splitVersionSegment strips only a numeric vN) while stripVersionSegments removes it — the same asymmetry TestNoPlainVerbLeavesItsResource already documents. Comparing one against the other turned pro team-viewer-remote-administration status into team-viewer-status.

The resulting surface diff is exactly four lines, and nothing else in 1757 commands moved.

buildDisambiguatedName also stops putting a version segment in a name, and I should flag that no live case reaches it. With the skip disabled the generated surface is byte-identical — the root-less branch answers the one shape that used to leak. I kept it because a version segment names no resource and is never a correct part of a command name, and tested it directly (TestBuildDisambiguatedNameSkipsTheVersionSegment) rather than leaving an untested branch; the guard that actually watches the shipped surface is TestNoCommandNameCarriesAnAPIVersion, which walks all 1757 names in the assembled tree — every product, not just the Pro generator. Say the word if you'd rather it came out until a real case arrives.

Mutation checks

Six, each reverted after:

mutation	fails
droppedPaths entries removed (v1 erase pair back)	TestTheDestructiveComputerActionsAreOneOperationEachOnTheServedVersion — and nothing else, which is why that pin exists: TestNoCommandNameCarriesAnAPIVersion and TestProWiringResolvesEveryNameItUses both still pass, because the version skip renames the twin to computers-inventory-erase and the stale-key guard sees a key that matches
root-less qualification branch disabled	TestEveryFormerInvocationKeepsItsShape (2 rows)
version-segment skip disabled	TestBuildDisambiguatedNameSkipsTheVersionSegment; generated surface unchanged, as above
actionMethod → hardcoded DELETE	TestDestructiveActionNamesItsOwnActionAndMethod, both assertions
flag-aware token → naive argv scan	TestProductTokenAndResourceToken, 5 rows
recordStaleProWiring → no-op	TestStaleProWiringIsRecorded

Wire verification

EU tenant-scoped platform credentials and the pro-nmartin instance profile, one run each. Nothing destructive was executed — every device action was exercised with --dry-run, which returns before the request.

• pro computer-inventory list against the instance: 4 records, v4 path.
• pro categories list through the gateway: 42 categories, 200.
• pro api-roles list: still refused, exit 8, unpublished wording intact.
• pro comp erase --serial FVFC41HCLYWP --dry-run --yes → [dry-run] Would erase computer "FVFC41HCLYWP" (serial: …, id: 5); --confirm-destructive still on the flag set.
• pro mobile-device-groups erase 1 --dry-run → [dry-run] Would run "erase" on mobile-device-group 1.
• pro mobile-device-prestages scope-delete-multiple 1 --dry-run and attachments-delete-multiple both resolve and preview.
• The hand-written erase sends /v4/computers-inventory/{id}/erase (pro_device_actions.go:267), unchanged.

make test, make lint (0 issues), make verify-generated, make verify-gateway-coverage and make verify-site all clean on the commit.

Documentation

CHANGELOG gained three sections (the version-free names with a before/after table, the v4 device actions and the v1 drop, and two Fixed entries for the bulk method and the flag placement); the ingested-endpoint count went 7 → 9 and the moved-invocation count 46 → 48 with both new rows in the migration table. CLAUDE.md gained six rows to the "Where to Make Changes" table.

Two CLAUDE.md staleness fixes fell out of this, both pre-existing on the branch rather than introduced by this round: the unpublished list said 37 Pro operations where the table now holds 28, and the difference is not a gateway change — the table is the intersection of "the gateway omits it" and "a generated command sends it", so spec-derived naming folding withdrawn versions took seven of them out of it. And the paragraph claiming pro policy-properties policy-properties and update-policy-properties are refused while working names two commands that no longer exist: /settings/obj/policyProperties is dropped at ingest, so pro policy-properties get/update serve the versioned /v1/policy-properties. The refused-command count is unchanged at 59, because none of those was a shipped command's endpoint.

Ready for re-review.

[ktn-jamf]

Re-review of d336a00 (round 4)

All three round-3 findings are fixed, and the CRITICAL's root cause was a layer below where I put it. I had the symptom right and the diagnosis wrong: both pro.go keys matched, and they matched the deprecated operations. POST /v1/computer-inventory/{id}/erase and POST /v4/computers-inventory/{id}/erase are one endpoint at two path shapes, deduplicateVersionedOps keys on the version-stripped shape and so saw two unrelated endpoints, and the deprecated pair won the plain names on document order. Dropping the v1 pair at ingest is the right fix and is the inventory-preload entry already sitting beside it with the noun changed.

Verified independently, all on the built binary:

• pro computer-inventory lists erase and remove-mdm and no v-4- twin; pro comp erase --help still carries --confirm-destructive.
• No version-bearing command name survives anywhere in the tree, so the NICE-TO-HAVE went with it.
• The hand-written erase still sends /v4/computers-inventory/{id}/erase, byte-identical to origin/main at the same line, so dropping the v1 pair costs no capability.
• origin/main drops 32 operations, this head 15, with zero new drops in either direction.
• The moved-verb refusal and the deprecation warning both fire for every flag form I could construct: space-separated value, = form, short form, boolean, and combined shorts.

One thing to record about my round-3 evidence. My first re-test of the flag placement reported five failures, and all five were my own artefact — I used a nonexistent -p ci-svc, which fails in PersistentPreRunE before the leaf guard runs. The warning fired in that output, which is what showed the token had been found correctly. Re-run with a harmless value-taking flag, every placement passes.

No new findings of my own this round. Two things below are an answer to the question you asked and a note on what the fix turned up, not findings.

Tip

Merge-ready. Twelve findings across four rounds are all fixed and independently verified, every gate is green, and this round found nothing new.

Rating: 5/5

Prior findings status

#	Location	State	Notes
(1)	internal/commands/pro.go:107-108	✅ Fixed	Root cause corrected one layer below my diagnosis: the deprecated v1 pair is dropped at ingest, so both keys now match the served v4 operations. No v-4- twin ships, --confirm-destructive is intact on the hand-written pair, and the hand-written path is unchanged from main. A stale key is no longer silent — all three helpers record a miss and TestProWiringResolvesEveryNameItUses fails on a non-empty record, which found two more dead keys immediately
(2)	internal/commands/moved_invocations.go:166-208	✅ Fixed	Flag arity is asked of the command's own flag set by cobra's stripFlags rules. Verified across every placement and form; --quiet correctly still does not silence the warning
(3)	mobile_device_prestages.go	✅ Fixed	scope-delete-multiple and attachments-delete-multiple, both in the migration table. The ambiguous plain delete-multiple is gone, and the two prestage resources no longer answer the same verb with different nouns

The question you asked

Keep the version-segment skip in buildDisambiguatedName. You are right that no live case reaches it and right to flag that rather than leave it unsaid, and the repo has a precedent that looks like it points the other way — DeduplicateVersioned was deleted rather than left dormant, on the grounds that "kept dormant they would have been worse than absent: an exported entry point with passing tests, describing a rule about filenames in a generator that no longer reads one."

The distinction is what the dormant code is about. DeduplicateVersioned described a rule about filenames, and filenames had stopped existing — its subject matter was gone, so no future input could reach it. The version skip describes a rule about version segments in paths, and those are everywhere in the document. It is dormant because a different mechanism (two droppedPaths entries) currently removes the only input that reached it, not because its subject is gone. A spec drop that reintroduces a versioned twin — which is exactly what an upstream deprecation cycle does — puts a live case back.

So the two mechanisms are not redundant. The skip makes a version-free name correct by construction; TestNoCommandNameCarriesAnAPIVersion catches one that slips through. Deleting the skip leaves the property resting on a test that reports the problem after the fact rather than on a rule that prevents it. It is also directly tested, so it is not an untested branch — which was the real objection to a dormant mechanism.

One thing I chased and am not raising

A lane flagged that firstPositional mis-locates the resource token for a clustered short-flag group whose last flag takes a value — pro -qp ci-svc icons get reads ci-svc as the resource, since the switch special-cases a single-character shorthand only. As a unit fact that is correct, and it looked like round 3's finding returning through a different door.

It is not a defect, and the reason is the design rule you stated. Cobra does not resolve that form either. On a command with no deprecation involved at all:

$ jamf-cli pro -qp ci-svc categories create --scaffold
unknown flag: --scaffold            # exit 2 — never reached `categories create`
$ jamf-cli pro -q -p ci-svc categories create --scaffold
{ … }                               # exit 0

origin/main behaves identically, so this is cobra's own stripFlags carrying the same single-character limitation, and the invocation is broken before any guard is consulted. The guard is therefore agreeing with cobra, which is exactly what you said it must do — "the guard has to agree with cobra about where the resource token is rather than about what a valid command line looks like". Fixing the guard alone would make it disagree: it would report the resource as categories while cobra had resolved to ci-svc.

Worth one cheap thing, if you agree: a test row pinning that shape as agreeing with cobra rather than as correct, so a future reader who spots the same asymmetry does not "fix" it into a divergence. The existing -nq row passes because both flags are boolean, which is what made this look open.

Two smaller lane observations I am also not raising. staleProWiring is read only by its test, with no runtime signal — but a build-time wiring invariant enforced by a test is this repo's convention throughout, and a stderr warning on every invocation would be noise for a defect only a build can have. And a droppedPaths entry cannot detect its path being re-purposed as a live operation, only disappearing — a pre-existing property of that table rather than of these two entries, and the patch-family round-trip in CLAUDE.md shows the reversal case is watched by hand.

Two notes, not findings

The actionMethod bug is the more valuable half of this round. pro mobile-device-groups erase --from-file ids.txt sent DELETE to a POST-only endpoint and reported Deleted for each entry, present in v1.28.0 and earlier. I asked only for the prose to be fixed; the hardcoded method in the same block is a real shipped defect and a worse one. Verified: the generated bulk and single paths now send POST, and the prose reads This will run "erase" on 12 mobile-device-groups. Both halves carry a Fixed CHANGELOG entry with the "present in v1.28.0 and earlier" note, which is the right place for a pre-existing bug a refactor uncovers.

Declining my suggestion 3 was correct. I proposed keying the wiring on the endpoint rather than the name, on the reasoning that the endpoint is stable where the name is not. The two twins are …/computer-inventory/{id}/erase and …/computers-inventory/{id}/erase, so an endpoint key would have needed both spellings listed and the same edit would have been required — it is stable against a name rename and not against this. Pinning the property instead (TestTheDestructiveComputerActionsAreOneOperationEachOnTheServedVersion, one erase and one remove-mdm-profile, each on the v4 path) is the stronger answer, and the mutation table shows it is the only guard that fails when the two droppedPaths entries are reverted.

Review coverage and scope

Round 4. Incremental diff 666ba61..d336a00, one commit, 40 files.

• Prior findings — all three verified independently on the built binary rather than from the reply, with origin/main built alongside for comparison
• Capability — dropped-operation lists diffed main against this head (32 → 15, zero new); the hand-written erase path compared line for line against main
• The flag-placement fix — eight invocation shapes, after discarding my own faulty first test
• The actionMethod fix — template and generated output both read; bulk, group and single branches all send the operation's own method
• Documentation — both staleness fixes checked against the binary: CLAUDE.md now says 28 unpublished Pro operations, the refused count is 59 and the binary agrees, and pro policy-properties ships get/update on the versioned path with no refusal, which is what the corrected paragraph says
• Silent failure — dispatched. Three observations, all resolved above as non-defects after I reproduced each against origin/main; it independently confirmed the actionMethod fix covers all three destructive branches and that the two bulk blocks were the only hardcoded DELETEs in the template
• Test quality — your mutation table reproduced independently, all six rows. The load-bearing one holds: reverting the two droppedPaths entries fails TestTheDestructiveComputerActionsAreOneOperationEachOnTheServedVersion and nothing else, with TestNoCommandNameCarriesAnAPIVersion and TestProWiringResolvesEveryNameItUses both still passing — which is the argument for that pin existing. Disabling the root-less qualification at parser.go:1690 fails TestEveryFormerInvocationKeepsItsShape with exactly the 2 rows claimed, and changes exactly one generated file. TestStaleProWiringIsRecorded's third assertion — that a resolving key is not recorded — is real and fires, so the passing case is not a broken accumulator. TestNoCommandNameCarriesAnAPIVersion walks every product, not just Pro, with a checked < 1500 floor
• Claim accuracy — one number in your reply is off and nothing depends on it: that walk examines 1822 names, not 1757. The test asserts only the floor
• [na] Performance, frontend — unchanged

Rounds 1 to 3 findings are not re-reviewed; all twelve were verified fixed in their rounds and nothing in this diff touches them.

What is done well, across four rounds

Each round fixed the mechanism rather than the reported instance, and three times the fix reached further than the report. Round 2 restored nineteen operations where I named seven. Round 3 corrected my proposed guard to a better shape and the correction found an eighth case. Round 4 found the root cause a layer below my diagnosis and fixed a shipped DELETE-on-POST bug that my complaint about prose happened to sit next to.

Three habits are worth naming because they are why this converged. Every guard was mutated before being claimed sound, and the mutation table states which guard fails for which mutation — including the one row that reads as a weakness and is the argument for the pin existing. Every fix that could not be exercised on the wire says so and says why, rather than implying coverage. And each round reported a defect found by the fix that nobody had asked about: boundKey, the eighth collision case, the hardcoded DELETE.

The one thing I would carry forward is that my own round-1 and round-2 reviews missed two defects that a comparison against origin/main's generated surface would have surfaced immediately. Diffing the shipped command surface against the base branch, not only the diff, is what found both.

---

Re-reviewed with the pr-review skill (Claude Opus 5). Prior findings were re-verified on the built binary with origin/main built alongside; the dropped-operation sets were regenerated on both.

Changes are complete