feat(compat): version fallback for older Jamf Pro tenants + spec version embedding

[neilmartin83]

Summary

Implements issue #215 in two parts.

Part 1 — Per-endpoint version fallback

Generated GET/DELETE commands retry against lower API versions on 404. A `versionFallback` tracker (emitted into `registry.go`) tries the primary path first; on 404 it walks `FallbackPaths` in descending version order, warns once to stderr on the first successful retry, and reuses the resolved path for all subsequent paginated calls (no re-probing per page).

Generator changes:

• `Operation.FallbackPaths []string` — populated by `deduplicateVersionedOps` for GET/DELETE ops where multiple versions coexist in the same spec file
• `resourceTemplate` — emits `vft.do(...)` in place of `ctx.Client.Do(...)` when `FallbackPaths` is non-empty
• `registryTemplate` — gains `versionFallback` struct + `versionFallbackApply`, `versionFallbackIs404` helpers

Live verified on 11.27.1 tenant (CLI built against 11.28.0):

• `mobile-device-groups-smart-groups`: fell back v2→v1, returned data ✓
• `computer-groups-smart-groups`: fell back v3→v2, returned data ✓

Known limitations:

1. `computer-groups-static-groups` (v3) does not fall back to `static-computer-groups` (v2) because they live in different spec files and `deduplicateVersionedOps` only pairs versions within a single file. `static-computer-groups` continues to serve v2 on older tenants. A cross-spec post-merge fallback pass is out of scope for this PR.

2. `--name`, `apply`, `delete --name`, and `delete --from-file` (with names) resolve names by listing the primary API path only. They do not walk `FallbackPaths`. On a tenant older than the primary version, `list` and `get ` fall back correctly but `get --name` returns resource not found. Workaround: use `list` to find the numeric ID, then `get `. Threading fallback through name-resolution is deferred to a follow-up PR.

Part 2 — Spec version embedding and tenant version check

`make sync-spec` now requires `JAMF_PRO_VERSION=` (mandatory). It writes the version to `specs/.spec-version` (git-tracked) and `make build` embeds it via ldflag. On each authenticated Pro command, a non-fatal probe to `/v1/jamf-pro-version` compares the live tenant version against the embedded spec version and warns once if the tenant is older:

```
warning: tenant is on Jamf Pro 11.27.1; this CLI was built against 11.28.0 — some commands may not be available
```

The result is cached per named profile for 24 hours (`~/.config/jamf-cli/.version-cache.json`) so fast single-shot commands don't pay a round-trip on every invocation. Env-var-based auth always probes live (no profile key to cache under).

Suppressed by `--no-version-check`, `JAMF_NO_VERSION_CHECK` env, or `--quiet`. Gateway mode verified: probe rewrites correctly through the platform gateway. `jamf-cli version` / `-o json` now surfaces `specProVersion`.

Spec sync (nmartin.jamfcloud.com, Jamf Pro 11.28.0)

• New v2/v3 endpoints for mobile-device-groups and computer-groups smart/static subgroups
• New generated commands: `computer-groups-smart-groups`, `computer-groups-static-groups`, `m2m` (was `m-2-ms`)
• `smart-computer-groups` renamed to `computer-groups-smart-groups` (canonical 11.28.0 path); old name preserved as an alias so existing scripts keep working
• Static groups intentionally expose two commands: `static-computer-groups` (v2, works on older tenants via the pre-existing spec) and `computer-groups-static-groups` (v3). Smart groups expose one command + one alias because their v2 path lives in the same spec file and is reachable via fallback; the separate `static-computer-groups` spec is the only route to v2 for static groups and must remain.
• Backup resource key updated: `smart-computer-groups` → `computer-groups-smart-groups`

Test plan

• Unit tests: `TestVersionFallback_` (7 cases covering primary success, 404 fallback, pagination reuse, exhaustion, non-404 propagation), `TestCompareProVersions` (10 cases including build-suffix stripping), `TestCheckTenantVersion_` (7 cases: warn/silent/unknown-spec/HTTP-error/cache-hit/stale-cache)
• `make test` — all packages green
• `make lint` — 0 issues
• `make generate` — idempotent (two consecutive runs produce identical output)
• Live on 11.27.1: version warning fires, group commands fall back correctly
• Live on 11.28.0 (platform-nmartin): no false warnings, gateway probe returns 200
• `--no-version-check` suppresses startup warning (0 warning lines)
• `specs/.spec-version` is git-tracked, not gitignored
• `smart-computer-groups` alias resolves to `computer-groups-smart-groups`

🤖 Generated with Claude Code

[ktn-jamf]

Warning

⚠️ Needs changes — Adds per-endpoint API-version fallback (GET/DELETE retry a lower /vN path on 404) plus a startup tenant-version check, bundled with an 11.28.0 spec sync. The core fallback mechanism is well-built and well-tested; the gaps are around feature coverage, a default-on probe, and a breaking command rename.
Blocking: (1). Please also make a conscious call on (2) and (3).

The fallback engine itself is solid — pagination reuses the resolved version without re-probing, the warning fires once, fallbacks are ordered newest-first, and make verify-generated confirms the committed generated code matches the templates. The findings below are about the edges of the feature, not the core.

---

🟧 (1) (correctness/UX, c=75) — internal/commands/pro/generated/registry.go:597 (resolveNameToIDForApply): name-based operations silently bypass the fallback on older tenants.

The headline promise is "commands work on older Jamf Pro tenants." But fallback is only wired into the direct GET/DELETE-by-id paths and the list command. Every name-resolution path issues a raw client.Do against the primary /v3 path:

• get --name, delete --name, patch --name, apply (upsert-by-name) → resolveNameToIDForApply lists /v3/... directly
• bulk delete --from-file with names → same helper (computer_groups_smart_groups.go:441)

So on an older tenant a user gets an inconsistent surface: pro computer-groups-smart-groups list succeeds (falls back to v2), but pro computer-groups-smart-groups get --name "Foo" fails with a confusing looking up "Foo": resource not found (HTTP 404). The PR's "Known limitation" section documents the cross-spec static-groups case but not this one, which is broader (it affects every versioned resource that has a fallback).

Suggested fix — pick one:

• Document it: add this to the Known-limitations section so the inconsistency is intentional and discoverable, or
• Thread it through: pass the resource's FallbackPaths into resolveNameToIDForApply (and the bulk path) and route its list call through a versionFallback, mirroring the list command.

Fixed when: either the limitation is documented in the PR/README alongside the static-groups note, or get --name/apply/delete --name on a versioned resource fall back to the lower version the same way list and get <id> do.

---

🟧 (2) (design/performance, c=75) — internal/commands/root.go:631: the tenant-version probe runs on every authenticated Pro command with no cross-invocation caching.

checkTenantVersion fires in PersistentPreRunE for every Pro command (release and local builds — specs/.spec-version is populated, so specProVersion != "unknown" is true locally too). It's a synchronous /v1/jamf-pro-version GET before the command's real work, which roughly doubles latency for fast single-shot commands (get, a one-page list), and a non---quiet shell loop pays one probe per invocation.

It's reasonably gated (--quiet, --no-version-check, JAMF_NO_VERSION_CHECK, 5s timeout, non-fatal), so this may be an acceptable trade — but it's a default-on network call on a very hot path, which is exactly the kind of precedent worth deciding deliberately rather than by default.

Suggested fix: cache the resolved tenant version per-profile with a short TTL (e.g. a line in the profile state / a ~/.config/jamf-cli cache file), so the probe runs at most once per TTL window instead of once per command. Alternatively, probe lazily only after a command actually returns a 404. If the per-command cost is intended, a one-line note in the PR confirming the team accepts it is enough.

Fixed when: the version check either no longer adds a round-trip to every invocation (cached/lazy), or the per-command probe is explicitly acknowledged as the intended behavior.

---

🟧 (3) (breaking change/consistency, c=75) — command rename smart-computer-groups → computer-groups-smart-groups with no alias, and an asymmetry with static groups.

smart-computer-groups was removed (file renamed to computer_groups_smart_groups.go); any script calling pro smart-computer-groups … breaks with no alias or deprecation path. Meanwhile static-computer-groups was kept and computer-groups-static-groups was added — so static groups now expose two overlapping commands while smart exposes one. On a new tenant a user sees both static-computer-groups and computer-groups-static-groups but only computer-groups-smart-groups, which is hard to reason about.

Suggested fix:

• Add an alias so smart-computer-groups keeps resolving (see internal/commands/aliases.go), or call out the rename in the PR as an intentional breaking change.
• Add a short note (README/help or the PR body) explaining why static keeps both commands (the cross-spec fallback gap) while smart has one, so the asymmetry reads as deliberate.

Fixed when: the old smart-groups invocation still works (alias) or is documented as removed, and the static-groups duplication is explained.

---

Nice-to-have suggestions (4 items)

🟩 (4) (cleanup, c=100) — internal/commands/groups.go:177: stale proGroupMap entry. "smart-computer-groups": groupComputers references a command that no longer exists. TestApplyProGroups_AllCommandsGrouped only checks the forward direction (every command has a group), so the orphan passes CI but is dead config.

-	"smart-computer-groups":                  groupComputers,
 	"static-computer-groups":                 groupComputers,
 	"computer-groups-smart-groups":           groupComputers,
 	"computer-groups-static-groups":          groupComputers,

Fixed when: the orphaned entry is removed.

🟩 (5) (code-quality/UX, c=100) — the new m2m tag auto-pluralizes to the awkward command m-2-ms (pro m-2-ms get for a single /v1/m2m/tenant-id GET). Per CLAUDE.md, add a resourceNameOverrides entry in generator/parser/parser.go to map m2m to something readable (e.g. m2m or m2m-tenant-id), then make generate.

Fixed when: the command name is no longer m-2-ms.

🟩 (6) (test coverage, c=75) — checkTenantVersion has no direct test. compareProVersions is well covered, but the probe/parse/warn/suppress flow (including the specProVersion == "unknown" and --no-version-check gates) isn't exercised. A small test with a mock cliClient returning a version JSON, asserting the warning fires below / is silent at-or-above the spec version, would lock in the behavior.

Fixed when: a test covers the warn-vs-silent decision and at least one suppression gate.

🟩 (7) (correctness, c=75) — generator/main.go:474: backup fallback is one level deep. FallbackListPath/FallbackGetPath take only op.FallbackPaths[0], so backup/diff of a resource with a v3→v2→v1 chain only tries v2. The generated commands walk the full chain; backup doesn't. Fine for the current two-version resources, but worth a comment noting the single-level limitation so it's not mistaken for full parity.

---

Review coverage

• Design and architecture: reviewed the fallback engine (versionFallback in registry template), the parser deduplicateVersionedOps chain-building + descending sort, and the backup-registry plumbing. Clean separation; flagged the name-resolution coverage gap (1) and the every-command probe precedent (2).
• Correctness: traced do/versionFallbackApply/compareProVersions. Path-prefix swap, pagination reuse, 404-vs-non-404 handling, and version parsing (incl. build-suffix strip) all check out. nil-resp-before-defer in checkTenantVersion is safe.
• Security: new /v1/jamf-pro-version probe uses existing auth, LimitReader(512), reads only version. RSQL escaping in resolveNameToIDForApply is pre-existing/unchanged. No new injection or secrets exposure.
• Performance: substantiated the extra per-command round-trip (2). Pagination correctly avoids per-page re-probing.
• Test coverage: TestVersionFallback_* (7 cases) and TestCompareProVersions (10 cases) are solid; ran both green. checkTenantVersion itself untested (6).
• Reliability: version check is non-fatal on every error path (auth, timeout, parse, empty version); fallback returns the original 404 when exhausted. Good.
• Code quality: flagged stale group entry (4) and m-2-ms naming (5).
• [na] Simplification: isBackup404/versionFallbackIs404 duplication is across package boundaries (commands vs pro/generated) — unavoidable without a shared helper, not worth churning.
• [na] Frontend concerns: no UI.
• Project rules compliance: respects the generated-code boundary (changes are in templates + parser; make verify-generated confirms committed output matches). Credential policy untouched. No .claude/rules/ dir exists.

Scope of review

• Diff: 50 files, +3584/−565. High-risk (touches generator templates, API version logic, build embedding).
• Read in full: generator/parser/{generator,parser,types}.go, generator/main.go, internal/commands/{root,version}.go, cmd/jamf-cli/main.go, internal/commands/pro_backup.go, Makefile, the new versionFallback runtime + tests, generated computer_groups_smart_groups.go (delete/list paths), resolveNameToIDForApply.
• Verified: go test for both new test files (green), make verify-generated (generated code up to date), go build ./internal/commands/... ./cmd/... (clean).
• Treated as derivative (not line-by-line): the bulk of internal/commands/pro/generated/*.go and specs/*.yaml — verified for consistency via verify-generated rather than reading each. Spot-checked the MdmCommands spec change (metadata-only) and mobile-device-group fallback presence.
• The PR bundles two features (fallback + version check) with a spec sync. Defensible (the sync provides the multi-version endpoints that demonstrate fallback), but it inflates the diff; a reviewer should lean on verify-generated for the generated/spec portion.

What's done well

✅ Pagination correctly resolves the version once and reuses it (activePath), avoiding a re-probe per page — and there's an explicit test for it (TestVersionFallback_ReusesResolvedPath).
✅ The version check is defensively non-fatal across every failure mode and offers three independent suppression switches plus a dev-build guard (specProVersion != "unknown").
✅ make verify-generated passing means the generator changes and committed output are genuinely in sync — no hand-edited generated files.

This covers all findings — addressing the above gets this PR to merge-ready.

_{🤖 Reviewed with the Jamf Claude Code skill (pr-review v1.7.0) · model: claude-opus-4-8[1m]}

[neilmartin83]

All findings addressed — here's the rundown:

(1) --name fallback bypass — documented as Known limitation #2 in the PR body with a workaround. Threading it through resolveNameToIDForApply is deferred to a follow-up.

(2) Per-command version probe — checkTenantVersion now caches the tenant version per named profile for 24 h at ~/.config/jamf-cli/.version-cache.json. Fast commands no longer pay a round-trip after the first call within the TTL window. Env-var-based auth (no named profile) always probes live.

(3) Breaking rename + static asymmetry — Added smart-computer-groups as an alias for computer-groups-smart-groups in aliases.go. PR body updated explaining why static groups keep two commands (cross-spec fallback gap; static-computer-groups is the only v2 route) while smart has one canonical command plus an alias.

(4) Stale proGroupMap entry — Removed.

(5) m-2-ms naming — Added "m-2-ms": "m2m" override in resourceNameOverrides; regenerated. Command is now pro m2m.

(6) checkTenantVersion tests — 7 cases added: warn below spec, silent at/above spec, silent on unknown spec (no HTTP call made), silent on HTTP error, cache hit (no HTTP call), stale cache (triggers live probe).

(7) Single-level backup fallback — Added comments at both FallbackPaths[0] lines in generator/main.go noting the limitation vs the full-chain walk in generated commands.

CI was also failing post-push on two unrelated things that came in via the main merge: errcheck on fmt.Fprintf(w, ...) (generic io.Writer isn't excluded the way os.Stderr is) and a missing 'Products' pillar entry in catalog.js — both fixed in the follow-up commit.

[ktn-jamf]

Tip

✅ Merge-ready (re-review of 675ac91..eba9cd0) — All 7 findings from the prior review are addressed: 6 fully, and the per-command probe (2) is substantially improved with a 24h per-profile cache. One non-blocking follow-up remains — see (8). No new bugs, security issues, or regressions; verify-generated, the new tests, and go build all pass.

Verified the fix commit (10aed4a) and the CI follow-up (eba9cd0). Generated code is consistent after the m2m rename, the cache tests are hermetic (t.TempDir() + XDG_CONFIG_HOME), and the errcheck/catalog.js CI fixes are benign.

Prior findings status (7)

#	Finding (original c)	Status	Notes
(1)	--name/apply don't fall back on older tenants (c=75)	✅ Fixed	Documented as Known Limitation #2 in the PR body with a list→get <id> workaround; threading through resolveNameToIDForApply deferred to a follow-up. This was one of the two accepted options.
(2)	Version probe on every command, uncached (c=75)	🟡 Partially fixed	24h per-profile cache added (.version-cache.json, atomic write, fresh tests). Works for explicit -p, but the default-profile / JAMF_PROFILE path still probes every command — see new finding (8).
(3)	Breaking rename + static-groups asymmetry (c=75)	✅ Fixed	smart-computer-groups alias added in aliases.go; asymmetry explained in the PR body.
(4)	Stale proGroupMap entry (c=100)	✅ Fixed	Removed (groups.go).
(5)	Awkward m-2-ms command name (c=100)	✅ Fixed	resourceNameOverrides maps m-2-ms→m2m; regenerated to m2m.go. verify-generated clean.
(6)	checkTenantVersion untested (c=75)	✅ Fixed	7 hermetic tests: warn/silent/unknown/HTTP-error/cache-hit/stale-cache.
(7)	Backup fallback one level deep (c=75)	✅ Fixed	Documented with comments at both FallbackPaths[0] sites in generator/main.go.

Nice-to-have suggestions (1 item)

🟩 (8) (design/performance, c=100) — internal/commands/root.go:694: the version cache keys on the raw profile flag, so the most common interactive path bypasses it.

The call is checkTenantVersion(proClient, specProVersion, profile, os.Stderr), but profile is the unresolved -p flag. The effective profile (resolvedProfile, already computed at line 664 and stored in cliCtx.ProfileName) falls back to JAMF_PROFILE and then cfg.DefaultProfile. After pro setup, a user typically runs commands without -p, so profile == "" → the cache is neither read nor written → they pay a live /v1/jamf-pro-version round-trip on every command. The cache only engages when -p <name> is passed explicitly, which is the minority case — so the optimization that finding (2) prompted doesn't reach the users it was meant to help.

Suggested fix: key the cache on cliCtx.ProfileName (the resolved profile) instead of the raw flag:

- checkTenantVersion(proClient, specProVersion, profile, os.Stderr)
+ checkTenantVersion(proClient, specProVersion, cliCtx.ProfileName, os.Stderr)

One nuance to decide consciously: if a user supplies env-token auth (JAMF_TOKEN) and has an unrelated DefaultProfile configured, resolvedProfile would key the env tenant's version under that profile name. If that edge matters, key on the resolved instance URL instead — it identifies the tenant regardless of auth source and covers every case. Either is strictly broader than the current -p-only behavior.

Fixed when: running a command with a configured default profile (no -p) reads/writes the cache and skips the probe within the TTL window — verifiable by asserting only one HTTP call across two invocations, or by inspecting .version-cache.json after a default-profile command.

Scope of re-review

• Baseline: my prior review at 675ac91 (CHANGES_REQUESTED). Diffed 675ac91..eba9cd0.
• New commits reviewed: 10aed4a (fix: address findings) and eba9cd0 (CI: errcheck + catalog.js). The intervening main merge (7a5b516) brought in an unrelated jamfprotect-go-sdk 0.5.0 bump — not part of this feature, not re-reviewed.
• Focused on the new surface: the per-profile version cache in root.go (+88), the 7 new tests, the aliases.go alias, and the m2m regeneration. Confirmed no new findings beyond (8).
• Verified: make verify-generated (up to date), go test ./internal/commands -run 'TestCheckTenantVersion|TestCompareProVersions' (green), go build ./... (clean), m_2_ms.go fully removed with no stale references.
• New comments since baseline: one from the author (@neilmartin83) summarizing the fixes — no open questions for the reviewer. No other reviewers commented.

Review coverage

• Design and architecture: cache is a self-contained, non-fatal layer with atomic writes; the one gap is the cache key (8).
• Correctness: cache read/write gating (profileName != ""), TTL freshness check, and re-comparison against specVersion on cache hit all verified. The unknown guard moved cleanly inside the function.
• Security: cache file written 0o600 in the config dir; stores only a version string + timestamp. No secrets, no injection surface. A corrupt cache self-heals (unmarshal error → live probe).
• Performance: the cache's reach is the substance of (8); pagination/fallback hot paths unchanged from the approved baseline.
• Test coverage: 7 new tests are hermetic and cover the warn/silent/cache/stale matrix. ""-profile tests never touch the filesystem.
• Reliability: every error path in checkTenantVersion remains silent/non-fatal; the io.Writer refactor preserves behavior.
• Code quality: alias comment is clear; m2m override regenerated consistently.
• [na] Simplification: no new redundancy introduced.
• [na] Frontend concerns: the catalog.js change is a one-line pillar registration, not UI logic.
• Project rules compliance: generated-code boundary respected (verify-generated clean); credential policy untouched. No .claude/rules/ dir.

_{🤖 Reviewed with the Jamf Claude Code skill (pr-review v1.7.0) · model: claude-opus-4-8[1m] · re-review}