Remove diff sync support

[kans]

Summary

Diff syncs never ran in production. The generation half had exactly one downstream consumer — the c1 monorepo's partial-uplift feature — and that feature is gated behind DEV_MANUAL feature flags, which cannot be enabled for production customers. The apply half was never built: nothing in any sync pipeline ever started a connector sync from, or applied, a partial_upserts / partial_deletions run. In this repo the feature was reachable only through hidden CLI flags. This removes the whole feature.

Net: 64 files changed, ~4,000 lines deleted (58 files / ~3,560 lines excluding generated pb/).

What's removed

Generation

• (*C1File).GenerateSyncDiff (in-file delta) — pkg/dotc1z/diff.go deleted
• (*C1FileAttached).GenerateSyncDiffFromFile (cross-file upserts/deletions pair) and its table helpers
• FileOps.GenerateSyncDiff removed from the c1zstore.FileOps interface and from both engine implementations

Entry points

• --diff-syncs / --base-sync-id / --applied-sync-id flags and their field definitions
• connectorrunner.WithDiffSyncs, syncDifferConfig, and the runner branch
• pkg/tasks/local/differ.go and the CreateSyncDiff task type (a blank placeholder pins the ordinals of the TaskType values that followed it)

Types and storage

• SyncTypePartialUpserts / SyncTypePartialDeletions and their retention, compaction, and convert guards
• The linked_sync_id API surfaces: SyncRun.LinkedSyncID, SetSyncLink on both engines, and the v3 proto field
• The pebble Put*RecordsIfNewer layer (if_newer.go), which existed only to serve the apply path that was never built and has no other caller

Proto — deleted fields and enum values are reserved by number and name:

• Task.create_sync_diff (field 119)
• SyncType values 4 and 5
• SyncRunRecord.linked_sync_id (field 8)

buf.yaml gains an ENUM_VALUE_NO_DELETE exception, mirroring the FIELD_NO_DELETE exception already there for the same reason: the FILE category's blanket rule would otherwise reject the reserved-and-removed enum values.

What deliberately stays

• The SQLite linked_sync_id column — vestigial but load-bearing for schema uniformity. CloneSync/SnapshotTo build their INSERT column list from the source's PRAGMA table_info and insert into a fresh-DDL destination, so removing the column from the DDL broke cloning of every pre-existing c1z (all of which physically carry it). The column and its add-if-missing migration stay; nothing reads or writes it, and TestLinkedSyncIDColumnMigratedAndCloneable pins both the migration and the clone/snapshot paths.
• supports_diff — despite the name, this marker no longer has anything to do with diff syncs. It records that a sync's data collection finished with SQL-layer grant metadata populated, and it gates baton rollback-expansion. The parallel syncer still writes it and the command still works unchanged; the sanitizer's carry-over is now named preserveSupportsDiffMarkers to match.
• SyncTypePartial ("partial") — targeted connector syncs, unrelated to diff syncs.
• baton diff — the read-only CLI comparison of two full syncs.
• Compaction's keep-newer merge (mergeBucketRawIfNewer), which is its own implementation.

Compatibility

• c1z files: no production file was ever written with a diff sync type or a non-empty linked_sync_id, so no legacy-read tolerance is needed for the removed sync types. Cross-version schema skew is covered: older SDKs re-add linked_sync_id via their shipped migration, and this SDK keeps the column (see above) so clones of older files keep working in both directions.
• Downstream (c1 monorepo): verified, not assumed. The partial-uplift feature consumes the removed surfaces — including GenerateSyncDiffFromFile from the temporal incremental-sync compactor — and upgrading the SDK will produce compile breaks in those files. The feature's flags are DEV_MANUAL (internal/dev accounts only, never production customers), so the breakage is confined to retiring that dev-only code as part of the SDK bump.

Test plan

• go build ./...
• make lint — 0 issues (also fixed 4 pre-existing revive unhandled-error findings in pkg/sync/external_principal_index.go and pkg/uhttp/xml_test.go)
• go test ./pkg/dotc1z/... ./pkg/c1zsanitize/... ./pkg/synccompactor/... ./pkg/field/... ./pkg/tasks/... ./pkg/connectorrunner/... ./cmd/baton/...
• go test ./pkg/sync/... (full suite)
• New regression coverage: TestLinkedSyncIDColumnMigratedAndCloneable (schema uniformity + clone/snapshot), TestToPebbleNoConvertibleSyncEmptySyncID (sync runs exist but none convertible ⇒ error, no partial output)
• Downstream verified: c1's partial-uplift is the only consumer of the removed surfaces, gated DEV_MANUAL; SDK bump there requires retiring that dev-only code

[github-actions]

🟡 Suggestion: this PR removes a large set of exported symbols (connectorstore.SyncTypePartialUpserts/Deletions, connectorrunner.WithDiffSyncs, c1zstore.FileOps.GenerateSyncDiff, c1zstore.SyncRun.LinkedSyncID, C1File.SetSyncLink, C1FileAttached.GenerateSyncDiffFromFile, pebble.Put*RecordsIfNewer/ErrDiffUnsupported, and tasks.CreateSyncDiff here) plus a proto oneof field. pkg/sdk/version.go is bumped automatically by CI as a patch, which won't signal the break to downstream connectors — worth calling out a 0.x minor bump and a migration note in the PR/release description. (Removing CreateSyncDiff from this iota block also renumbers ListStaticEntitlementsType/IssueCredentialType; harmless today since TaskType is process-local metrics-only, but it's worth not relying on that silently.)

[kans]

Nothing uses these things in practice.

[github-actions]

General PR Review: Remove diff sync support

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

Review Summary

Full PR diff (64 files, ~4k lines deleted, including pb/ and both .proto sources) scanned for security and correctness. The feature removal is internally consistent — no dangling references to GenerateSyncDiff, SetSyncLink, LinkedSyncID, Put*RecordsIfNewer, SyncTypePartialUpserts/Deletions, CreateSyncDiff, or the --diff-syncs flag family remain in Go, proto, YAML, or docs; the checked-in pb/ output matches the reserved ranges in both .proto files; the pebble if_newer.go deletion is safe because synccompactor/pebble has its own mergeBucketRawIfNewer and timestamp decoder; and the buf.yaml ENUM_VALUE_NO_DELETE exception is the correct mirror of the existing FIELD_NO_DELETE one, since ENUM_VALUE_NO_DELETE_UNLESS_{NAME,NUMBER}_RESERVED are both still in breaking.use. Prior feedback is partly addressed: the TaskType ordinal placeholder and TestToPebbleNoConvertibleSyncEmptySyncID landed in 5a673948, and the vestigial linked_sync_id column plus TestLinkedSyncIDColumnMigratedAndCloneable landed in 44c978c9. No new issues found; the three items below are prior findings verified as still outstanding against this HEAD.

Risk triage (per docs/BUG_CATCHING.md §2) — Silence: yes for the retention-bucketing change (a wrongly-evicted sync is an absent record, not an error); the API removals themselves are loud compile breaks. Durability: yes — sync_runs DDL, v3 proto wire types, and c1z contents. Uncontrolled dimensions: yes — version-pair dependence (files written by older SDKs read by this one, and vice versa). Consumer distance: yes — downstream connectors and the c1 platform. Verdict: HIGH, driven by version-pair dependence; remediation rung 2 (re-sync). The version-skew class is instrumented: TestLinkedSyncIDColumnMigratedAndCloneable covers the schema-shape/clone seam and the proto reserved ranges cover the wire seam. The gap is the legacy-row read path — no fixture exercises opening a c1z that physically carries partial_upserts/partial_deletions rows, which is exactly the input the retention finding below concerns.

Security Issues

None found.

Correctness Issues

None found.

Suggestions

• (carried over, still outstanding) pkg/c1zsanitize/sanitize.go:557-567 — deleting diffsync_test.go removed the only coverage of the sanitizer's sync-run parent_sync_id mapping, including the external-reference HMAC branch at line 565, which stays reachable via cross-file partials whose parent full sync is not in the same c1z. No remaining test in pkg/c1zsanitize asserts either branch. (confidence: high)
• (carried over, still outstanding) pkg/dotc1z/c1zstore/cleanup_policy.go:49-54 — legacy partial_upserts / partial_deletions rows now fall through the default branch into fullSyncs and can consume the retention budget, evicting a genuine full sync. Dev-only exposure given the DEV_MANUAL gating stated in the PR, but the dedicated bucket and its fixtures were deleted with no replacement. (confidence: medium)
• (carried over, still outstanding) pkg/sdk/version.go:3 — v0.24.6, untouched by this PR. This change removes exported symbols (connectorrunner.WithDiffSyncs, connectorstore.SyncTypePartialUpserts/Deletions, c1zstore.FileOps.GenerateSyncDiff, c1zstore.SyncRun.LinkedSyncID, taskTypes.CreateSyncDiff, (*C1File).SetSyncLink, (*C1FileAttached).GenerateSyncDiffFromFile, the pebble Put*RecordsIfNewer layer) and deletes proto fields; the repo criteria ask for a 0.x minor bump as the compatibility signal. (confidence: high)

Prompt for AI agents

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

## Suggestions

In `pkg/c1zsanitize/sanitize.go`:
- Around lines 557-567: The parent_sync_id remapping in sanitizeSync has no test
  coverage since pkg/c1zsanitize/diffsync_test.go was deleted. Add a test in
  pkg/c1zsanitize that builds a source c1z with (a) a partial sync whose parent
  is another sync in the same file, and (b) a partial sync whose parent_sync_id
  names a sync NOT present in the file. After Sanitize, assert that case (a)'s
  destination ParentSyncID equals the destination id of the mapped parent, and
  that case (b)'s destination ParentSyncID equals SanitizeID(secret, srcParentID)
  — i.e. the external reference is HMAC-transformed, not dropped and not leaked
  raw.

In `pkg/dotc1z/c1zstore/cleanup_policy.go`:
- Around lines 49-54: The switch now sends the removed diff sync types through
  `default` into the `fullSyncs` bucket. A legacy (DEV_MANUAL-era) c1z carrying
  `partial_upserts` / `partial_deletions` rows will therefore count them against
  the full-sync retention budget and can delete a genuine full sync. Either add
  an explicit case that routes the literal string types "partial_upserts" and
  "partial_deletions" (and the empty/unknown type produced by
  syncTypeV3ToConnectorstore for reserved v3 enum values 4/5) into the
  `partials` bucket or skips them entirely, or add a table-driven test in
  cleanup_policy_test.go that pins the intended behavior for legacy rows so the
  choice is deliberate rather than incidental.

In `pkg/sdk/version.go`:
- Line 3: Version is v0.24.6 and unchanged by this PR, but the PR removes
  exported Go symbols and deletes proto fields/enum values. Per the repo review
  criteria, a 0.x minor bump is the compatibility signal for a break — bump to
  v0.25.0 (or state explicitly in the PR why a minor bump is not warranted).

[github-actions]

Blocking issues found — see review comments.

[github-actions]

Blocking issues found — see review comments.

[github-actions]

No blocking issues found.

[github-actions]

No blocking issues found.