feat(app-installers): deployment export, history, version-update, per-computer + bulk retry (--all)

[neilmartin83]

What

Surfaces five previously-missing App Installer deployment endpoints. All were verified to exist on the live backend before adding (see How verified).

New pro app-installer-deployments commands:

Command	Endpoint
export	POST /v1/app-installers/deployments/export (CSV)
history	GET /v1/app-installers/deployments/{id}/history
add-history-note	POST /v1/app-installers/deployments/{id}/history
version-update	POST /v1/app-installers/deployments/{id}/version-update
installation-retry-by-computer-id	POST /v1/app-installers/deployments/{id}/computers/{computerId}/installation-retry
installation-retry --all	POST /v1/app-installers/deployments/computers/installation-retry (bulk, no {id})

installation-retry --all makes a single server-side call to the collection-level endpoint (not a client-side loop); it's mutually exclusive with <id>/--name.

Generator changes

• pairCollectionBulkActions (parser.go, runs before disambiguateSameTerminalOps): pairs a single-{id} x-action with a same-method collection-level sibling at the path minus {id}, records BulkActionPath on the per-{id} op (template emits --all), and drops the bulk op so it doesn't spawn a duplicate command. Guards — same method, per-{id} path ends in a literal verb (not }), exactly one path param — prevent mis-pairing CRUD ops that specs mis-tag with x-action (e.g. PUT /accounts/{id}).
• opHasNameLookup now requires exactly one path param. Multi-param actions fall to positional ExactArgs(N) instead of the single-id name-lookup path.

Incidental bug fix (drives most of the diff outside app-installers)

The opHasNameLookup change fixes five pre-existing broken commands whose multi-param paths were routed through the single-id codepath, leaving one path param unsubstituted (literal {…}) in the request URL:

• computers-inventory download <id> <attachmentId>
• notifications delete <id> <type>
• team-viewer close / resend-notification / sessions-status <configurationId> <sessionId>

How verified

Live-probed against a Jamf Pro sandbox. Existence confirmed via the method-sensitivity oracle (the modern API returns an identical generic 404 for unknown routes and legit not-found, but 405 on a wrong method proves a route exists; control paths returned 404). Every command routes correctly on the wire; non-2xx responses are server-state (e.g. no failed installs to retry, already-latest version).

Spec note

specs/AppInstallerDeployments.yaml is a monolith PreservedSpec, so these hand-added paths won't be clobbered by make sync-spec.

⚠️ Overlaps with open PRs (needs rebase, no logical conflict)

• Handle duplicate computer serial numbers (fix silent --serial resolution + surface duplicates) #275 (duplicate-serials): also edits generator/parser/generator.go and regenerates several of the same files (computers_inventory.go, app_installer_deployments.go, team_viewer_remote_administrations.go, registry.go). The source edits are in different functions/regions and the features are orthogonal; generated-file conflicts are mechanical — whoever merges second should rebase and run make generate.
• docs: fix inaccuracies in CLAUDE.md (command surface, counts, auth-skip) #278 (CLAUDE.md docs): both edit CLAUDE.md in different sections; trivial.

Review follow-up (blocking findings 1–4 addressed)

• (1) --all confirmation gate — the collection-level bulk action now requires explicit confirmation (--yes or interactive prompt; errors under --no-input without --yes), matching the codebase convention for tenant-wide mutations. flagYes is declared/wired for non-destructive BulkActionPath ops.
• (2) flagName coupling + guard tests — the --all template's --name mutual-exclusion check is now gated behind opHasNameLookup, so a BulkActionPath op on a resource without name-lookup preconditions no longer emits an undefined flagName. Added regression tests isolating pairCollectionBulkActions' two guards (each fails when its specific guard is removed, verified by mutation) plus TestGenerate_BulkActionWithoutNameLookup.
• (3) Multi-arg destructive confirmation — dry-run/confirm messages now show every positional arg (strings.Join), so notifications delete <id> <type> no longer hides <type>.
• (4) Binary-response example — now renders every path-param placeholder (extracted pathParamUsage helper), so computers-inventory download <id> <attachmentId> is copy-pasteable.

Nice-to-haves (5) privilege-equivalence, (7) export --out-file (pre-existing, 6 resources), and (8) scope note are not addressed here — (7) warrants a separate follow-up against the shared template.

Checks

make build, make lint (0 issues), make test (0 failures), make verify-generated all pass. New/updated tests: TestPairCollectionBulkActions (incl. mis-tagged-CRUD, cross-method, and the two isolated guard regressions), TestStripParamSegments, multi-param TestOpHasNameLookup, TestGenerate_BulkActionAllFlag (now also asserts the confirmation gate), TestGenerate_BulkActionWithoutNameLookup.

🤖 Generated with Claude Code

[ktn-jamf]

Tip

✅ Merge-ready — All 4 blocking findings from the prior review are fixed and independently re-verified (build, full test suite, lint, make verify-generated, and manual runtime checks of the new confirmation gate and example text).

Rating: 5/5

• Clean fixes: each blocking finding got a targeted change plus a regression test that specifically isolates the guard it protects, and the 7 other single-param resources mechanically touched by the confirmation-message fix are functionally unchanged (verified by diff).

Prior findings status

#	Location	State	Notes
(1)	app_installer_deployments.go / generator.go (--all confirmation)	✅ Fixed	flagYes + --no-input-aware prompt added, matching pro_bulk.go's convention. Verified live: installation-retry --all --no-input now refuses with "pass --yes to confirm when --no-input is set" instead of firing.
(2)	parser.go/generator.go (flagName/opHasNameLookup coupling)	✅ Fixed	The --all branch's flagName != "" check is now gated behind {{ if opHasNameLookup . $ }}, so a BulkActionPath op on a resource lacking name-lookup preconditions no longer references an undeclared flagName. New TestGenerate_BulkActionWithoutNameLookup reproduces exactly the scenario I used to trigger the compile failure and asserts no flagName reference; two new TestPairCollectionBulkActions sub-tests isolate the HasSuffix/Count guards I found untested (each now fails when its specific guard, not an incidental method/param mismatch, is removed).
(3)	notifications.go (destructive confirmation message)	✅ Fixed	Both the dry-run and --yes prompt now use strings.Join(args, " ") instead of args[0]. Verified live: notifications delete 1 2 --dry-run now prints "Would delete resource 1 2", showing both id and type.
(4)	computers_inventory.go (download example)	✅ Fixed	Extracted a shared pathParamUsage helper (used by both Use and the example generator) so the example always matches the real arg count. Verified live: --help now shows download <id> <attachmentId> -O output.bin.
(5)	privilege-equivalence check in pairCollectionBulkActions	🔴 Open	Not addressed; no reply given beyond "not addressed here" in the updated PR description. Low priority — nice-to-have, no current spec triggers it.
(6)	PR description undercounted the fix (4 vs actual 5 commands)	✅ Fixed	Description now says "fixes five pre-existing broken commands" and explicitly lists sessions-status.
(7)	pre-existing export --out-file bug	💬 Dismissed	Author: "pre-existing (6 resources); warrants a separate follow-up against the shared template." Reasonable — out of scope for this PR, affects other resources equally.
(8)	scope bundling observation (feature + unrelated bugfix)	🔴 Open	Acknowledged only ("not addressed here"); this was a process observation for future PRs, not a code change, so nothing further to do here.

No new comments from other reviewers appeared between my initial review and this one, and the fix commit didn't touch any file outside the generator/test/generated surface already in scope.

Scope of this re-review

• Diffed the single fix commit (9494250, "fix(generator): address PR #279 review (blocking findings 1-4)") against my baseline review's commit (447fc41).
• Re-ran go build ./..., go test ./generator/... ./internal/commands/... (all pass, exit 0 — verified directly, not through a piped/truncated check), make lint (0 issues), and make verify-generated (zero diff after regeneration).
• Manually rebuilt the binary and exercised the actual fixes: installation-retry --all --no-input (confirms the new refusal), installation-retry <id> --all (confirms mutual exclusivity still works), notifications delete 1 2 --dry-run (confirms both args now shown), and computers-inventory download --help (confirms the corrected example).
• Spot-checked all 7 other generated files mechanically touched by the finding-(3) template fix (computer_prestage_scopes.go, enrollment_customization_panels.go, erase_device_computers.go, erase_device_mobiles.go, mobile_device_prestage_scopes.go, remove_computer_mdm_profiles.go, remove_mobile_device_mdm_profiles.go) — all single-arg ops, so strings.Join(args, " ") is functionally identical to the old args[0], confirmed by diff.
• Did not re-run the full specialist panel (usability/scope/test-quality/devil-advocate) since the fix is narrowly scoped to the 4 findings those agents' input fed into, and each fix was independently verifiable by direct source reading, build, and execution.

This covers all findings — the PR is merge-ready.

Generated by pr-review v1.12.1, a Jamf Claude Code skill