fix(sync): enforce OptInRequired in local/CLI sync path (WillSyncResourceType)

[c1-squire-dev]

Problem

OptInRequired is the annotation a connector attaches to a resource type so it stays off by default until an operator explicitly opts in. baton-jamf's managedDevice resource type uses this today.

The platform-facing capabilities path already respects it: connectorbuilder.go surfaces the annotation into ResourceTypeCapability.OptInRequired for the platform to honor:

• pkg/connectorbuilder/connectorbuilder.go:426 — OptInRequired: annos.Contains(&v2.OptInRequired{})

But the SDK's own local/CLI sync path did not enforce it. In syncer.SyncResourceTypes, an empty (default) resource-type filter took the connector's full ListResourceTypes response verbatim:

• pkg/sync/syncer.go (pre-fix) — the else branch: resourceTypes = resp.GetList()

Because SyncResourceTypes is the single choke point that persists resource types (PutResourceTypes), and downstream SyncResources enqueues sync work by reading resource types back from the store (syncer.go:1241-1251), an opt-in-required type was persisted and fully synced on any default local/CLI sync. baton-jamf carries its own connector-side gate today precisely because of this gap.

Fix

Introduce syncer.shouldSyncResourceType, the opt-in-aware inclusion decision, and route SyncResourceTypes through it:

• Explicit --sync-resource-types filter → sync exactly the named types. Naming an opt-in-required type in the filter is the operator's opt-in, so it's included.
• No filter (default) → sync everything except resource types carrying OptInRequired, which stay off until named. A one-line Info log explains any skip.

The --sync-resource-types flag (→ connectorrunner.WithSyncResourceTypeIDs → sync.WithSyncResourceTypes → s.syncResourceTypes) is the same operator signal the platform path keys on, so behavior is now consistent across both.

Backward compatibility

A connector that has never set OptInRequired on any resource type sees zero behavior change: on the default path the annotation is simply never present, so every advertised type still syncs. Only opt-in-flagged types are affected, and only when no explicit opt-in signal is present. Explicit-filter behavior is unchanged.

Tests (pkg/sync/syncer_optin_test.go)

• TestShouldSyncResourceType — the decision matrix (no-filter/opt-in → excluded; explicit-select opt-in → included; normal types unaffected).
• TestOptInRequiredResourceTypeDefaultSyncExcluded — regression guard: default sync excludes the opt-in type, still syncs normal types. Verified this FAILS on pre-fix code (opt-in type was persisted) and passes on the fix.
• TestOptInRequiredResourceTypeExplicitlySelected — explicit selection includes the opt-in type.

All run in both sequential and parallel sync modes. go test -tags=baton_lambda_support ./pkg/sync/ and golangci-lint run are green.

Follow-up (NOT in this PR)

Once this SDK fix soaks, baton-jamf's connector-side gate — annotationsForManagedDeviceResourceType in pkg/connector/helpers.go plus the ResourceSyncers registration gate — becomes redundant defense-in-depth rather than the sole safety net. It is intentionally left in place here; removing it is a separate later cleanup. With this fix, every future opt-in connector gets the guarantee for free.

Scope note

The fix targets the top-level resource-type inclusion decision (the documented gap and the jamf use case, where managedDevice is a top-level registered ResourceSyncer). Child resource types discovered via ChildResourceType annotations are already gated by an explicit filter; the default-path child case is left as a documented boundary to avoid per-resource store lookups on the hot sync path.

🤖 Generated with Claude Code

[github-actions]

General PR Review: fix(sync): enforce OptInRequired in local/CLI sync path (WillSyncResourceType)

Blocking Issues: 0 | Suggestions: 1 | Threads Resolved: 0
Criteria: Criteria status: loaded .claude/skills/ci-review.md from trusted base 604dae09ce40.
Review mode: full
View review run

Review Summary

Scanned the full PR diff for security and correctness. The change routes SyncResourceTypes through a new shouldSyncResourceType helper so opt-in-required types are excluded from a default local/CLI sync unless named via --sync-resource-types, mirroring the platform capabilities path. The gating is effective because downstream SyncResources re-reads resource types from the store (syncer.go:1266), explicit-filter behavior is preserved, and tests cover both paths across sequential and parallel sync modes. No security or blocking correctness issues found.

Security Issues

None found.

Correctness Issues

None found.

Suggestions

• pkg/sdk/version.go:3 — This is a deliberate default-behavior change for connectors using OptInRequired, but Version remains v0.18.2. Per the repo-local criteria, default-behavior changes should carry a version signal (at least a 0.x minor bump); consider bumping if version is not set at release time.

Prompt for AI agents

Verify each finding against the current code and only fix it if needed.

## Suggestions

In pkg/sdk/version.go:
- Around line 3: This PR changes default local/CLI sync behavior (opt-in-required
  resource types are now excluded by default), which is a compatibility-relevant
  default-behavior change for downstream connectors that set OptInRequired. The
  Version constant is still v0.18.2. If version bumps are not handled by a
  separate release process, bump to at least the next 0.x minor (e.g. v0.19.0)
  so the behavior change carries a semver signal, as required by the repo-local
  review criteria.

Note: the machine-readable review-state marker was omitted because the CI sandbox blocks emitting its JSON payload; the next review will run in full mode.

[github-actions]

No blocking issues found.