Skip to content

Add changes/database and changes/zone endpoints; deprecate zones/changes (#401, #47, #46) - #429

Open
leogdion wants to merge 3 commits into
v1.0.0-beta.4from
47-401-change-tracking-endpoints
Open

Add changes/database and changes/zone endpoints; deprecate zones/changes (#401, #47, #46)#429
leogdion wants to merge 3 commits into
v1.0.0-beta.4from
47-401-change-tracking-endpoints

Conversation

@leogdion

Copy link
Copy Markdown
Member

Summary

Implements CloudKit's two current changes/* endpoints and reconciles the change-tracking naming confusion documented in #401.

Apple endpoint Purpose Page primitive Auto-paginating Status
records/changes Fetching Record Changes fetchRecordChanges fetchAllRecordChanges already shipped
changes/database Fetching Database Changes — which zones changed fetchDatabaseChanges fetchAllDatabaseChanges new (#46)
changes/zone Fetching Record Zone Changes — records within zones fetchRecordZoneChanges fetchAllRecordZoneChanges new (#47)
zones/changes Fetching Zone Changes fetchZoneChanges fetchAllZoneChanges deprecated by Apple

Both new operations follow the existing page-primitive + auto-paginating-extension pattern, with maxPages ceilings and stuck-token detection, matching fetchAllRecordChanges.

#401 — the three requested resolutions

  1. Specchanges/database and changes/zone added to openapi.yaml and regenerated; zones/changes marked deprecated in favor of changes/database.
  2. Service — wrappers added for both. fetchZoneChanges / fetchAllZoneChanges are now @available(*, deprecated), pointing at fetchDatabaseChanges. Per the repo's @available convention, deprecated is correct here because these symbols have a replacement.
  3. Docs — the README roadmap now lists all four endpoints with both Apple's names and MistKit's method names, and reflects the deprecation. AGENTS.md (CLAUDE.md) ops table updated.

Per-zone failure handling

changes/database and changes/zone both return per-zone results that can fail individually. Rather than dropping failures, the result types expose both sides — DatabaseChangesResult.changedZones / .failures and RecordZoneChangesResult.changes / .failures, backed by ZoneChangeResult / ZoneRecordChangesResult and ZoneOperationFailure. This follows the RecordResult success-or-failure pattern the repo applies elsewhere.

Verification

Run on macOS, Apple Swift 6.3.2 (arm64-apple-macosx28.0):

Check Result
swift build Build complete
swift test 573 tests / 182 suites passing
./Scripts/lint.sh 0 violations in 416 files, no unused code (periphery ran)
swift-format Clean
./Scripts/generate-openapi.sh Reproduces the committed Sources/MistKitOpenAPI/ byte-identically

Not verified locally — relying on CI: wasm32/wasm-embedded, Windows, Android, Linux, and the non-macOS Apple platforms.

Wire format

Local .claude/docs/webservices.md is abbreviated on these endpoints, so shapes were taken from Apple's archived CloudKit Web Services Reference:

No live-service call was made. Field names come from the archived reference, not from an observed payload.

Warning

Possible pre-existing bug, deliberately not changed here. Apple documents zones/changes as using metaSyncToken, but MistKit sends and reads syncToken. If the doc is right, that pagination is silently broken. It was left alone because Apple's own page contradicts itself — the moreComing text refers back to "the included syncToken key" — so this needs a live-response check rather than a doc-driven guess. Surfaced from #386 review; worth its own issue.

Note on this branch's history

The implementing agent hit a cross-worktree git stash collision mid-run (the stash stack is repo-global, so a sibling worktree's stash pop displaced this branch's WIP) and then stalled before finishing verification. The work was recovered intact and checkpointed in 79f1f31; de59ebd then resolves the 14 SwiftLint violations that were still outstanding, plus two periphery findings introduced by the file splits needed to satisfy the file-length rule. Full verification above was re-run from scratch on the final tree, not inherited.

Closes #401
Closes #47
Closes #46

🤖 Generated with Claude Code

@coderabbitai

coderabbitai Bot commented Aug 20, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 41299ff8-2793-464b-945b-3128896ba615

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@claude

claude Bot commented Aug 20, 2026

Copy link
Copy Markdown

Review

Reviewed the diff (excluding generated Sources/MistKitOpenAPI/*). This is a clean, well-structured addition that follows the repo's established patterns closely — the OperationResult<Success, Target> / OperationFailure<Target> reuse for ZoneChangeResult / ZoneRecordChangesResult is exactly the "RecordResult pattern everywhere" convention the repo has been converging on, and the per-file splits keep everything under the length lint. Test coverage for both new operations (success, error-handling, pagination, pagination-limits/stuck-token) is thorough and mirrors the existing fetchAllRecordChanges/fetchAllZoneChanges test shape.

A few things worth a look before/after merge:

1. numberAsStrings is added to the spec but unreachable from the curated API (minor/completeness)

openapi.yaml adds numberAsStrings to both the changes/zone request body and RecordZoneChangesRequestZone (and it generates fine into MistKitOpenAPI.Types), but neither ZoneChangesRequest (Sources/MistKit/Models/Zones/ZoneChangesRequest.swift) nor CloudKitService.fetchRecordZoneChanges ever sets it — so it's permanently nil/default and callers of the curated API have no way to opt into it. Since this field doesn't exist anywhere else in the codebase yet, there's no established precedent either way. Worth either wiring it through ZoneChangesRequest/fetchRecordZoneChanges, or dropping it from the spec until it's actually supported.

2. zoneID isn't required on the new success schemas (minor/spec-accuracy)

DatabaseChangedZone and RecordZoneChangesZoneResult both declare zoneID as optional, even though CloudKit always includes it on success and the Swift side (ZoneTarget.init(from:), ZoneInfo.init(fromZoneID:)) throws ConversionError.zoneMissingID/zoneMissingName if it's absent. Not a functional bug given the fail-loud philosophy, but marking it required: would make the spec (and generated docs) reflect reality instead of relying on a runtime throw.

3. ZoneChangesAccumulator.finish() drops previously-accumulated records if a zone later fails (edge case)

In Sources/MistKit/Models/Zones/ZoneChangesAccumulator.swift, if a zone succeeds on round 1 (accumulating records) but its zone entry comes back as a ZoneFetchFailure on a later round (e.g. zone deleted mid-pagination), finish() returns .failure(...) for that zone — the records gathered in earlier rounds for it are silently discarded rather than surfaced alongside the failure. Probably rare in practice, but worth a doc note (or intentionally deciding this is fine) since it's not obvious from the call site.

4. Sibling auto-paginators return different shapes (observation, not a defect)

fetchAllDatabaseChanges returns a raw tuple (zones:, syncToken:), matching the existing fetchAllZoneChanges/fetchAllRecordChanges convention. fetchAllRecordZoneChanges, added in the same PR, returns the full RecordZoneChangesResult struct instead. This looks intentional and justified (per-zone moreComing/failures don't fit a flat tuple), but it's a visible inconsistency between two APIs shipped together — might be worth a one-line note in the PR/README explaining why they differ, since a future reader may assume it's accidental.

5. Initial zones: list isn't deduplicated before the first request (very minor)

fetchAllRecordZoneChanges seeds pending = zones directly from the caller-supplied array for round 1, while ZoneChangesAccumulator.init dedupes by zone name for its internal bookkeeping and all subsequent rounds. If a caller accidentally passes duplicate zoneIDs, only the first request actually contains the duplicates; that's likely harmless (CloudKit will just answer twice for round 1, and the accumulator's record(changes:) last-write-wins per name), but flagging in case it's not the intended contract.

Nits

  • README's new "Change Tracking" example shadows a local let database = try await service.fetchDatabaseChanges(...) against the database: parameter name used everywhere else in the doc for the Database enum. Purely cosmetic, but a quick rename (e.g. databaseChanges) would avoid a double-take.

Things that look right

  • zonePaginationLimitExceeded/paginationLimitExceeded on the two new paginators correctly reuse the pre-existing CloudKitError cases and are well-documented as dropping in-flight failures on the maxPages ceiling (consistent with existing behavior, not new).
  • Stuck-token detection in both fetchAllDatabaseChanges (empty zones + unchanged token) and ZoneChangesAccumulator.merge (no records + unchanged token) is correct and tested.
  • zones/changes deprecation is applied consistently: @available(*, deprecated) on both fetchZoneChanges and fetchAllZoneChanges, matching the repo's stated convention for symbols with a replacement, plus doc pointers to the new methods.
  • No security concerns — this is purely additional read/pagination surface over existing auth plumbing; no new credential handling, logging, or external I/O paths.

Nice work — this closes out #401/#47/#46 cleanly and the history note about the cross-worktree stash recovery is a good example of the "never git stash in this multi-worktree repo" lesson being captured back into .claude/memory/.

@codecov

codecov Bot commented Aug 20, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 93.83754% with 22 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (v1.0.0-beta.4@4fabb07). Learn more about missing BASE report.

Files with missing lines Patch % Lines
...itService/CloudKitError+ZoneErrorDescription.swift 0.00% 10 Missing ⚠️
...udKitService/CloudKitService+DatabaseChanges.swift 94.73% 3 Missing ⚠️
...oudKitService/CloudKitError+ErrorDescription.swift 33.33% 2 Missing ⚠️
Sources/MistKit/Models/ZoneTarget.swift 88.88% 2 Missing ⚠️
.../MistKit/Models/Zones/ZoneChangesAccumulator.swift 97.18% 2 Missing ⚠️
.../CloudKitService+RecordZoneChangesPagination.swift 96.87% 1 Missing ⚠️
...tions/Operations.fetchDatabaseChanges.Output.swift 93.75% 1 Missing ⚠️
...ons/Operations.fetchRecordZoneChanges.Output.swift 93.75% 1 Missing ⚠️
Additional details and impacted files
@@               Coverage Diff                @@
##             v1.0.0-beta.4     #429   +/-   ##
================================================
  Coverage                 ?   77.32%           
================================================
  Files                    ?      172           
  Lines                    ?     4115           
  Branches                 ?        0           
================================================
  Hits                     ?     3182           
  Misses                   ?      933           
  Partials                 ?        0           
Flag Coverage Δ
mistdemo-spm-macos 11.08% <9.80%> (?)
mistdemo-swift-6.2-jammy 11.08% <9.80%> (?)
mistdemo-swift-6.2-noble 11.08% <9.80%> (?)
mistdemo-swift-6.3-jammy 11.08% <9.80%> (?)
mistdemo-swift-6.3-noble 11.08% <9.80%> (?)
mistdemo-swift-6.4-jammy 11.08% <9.80%> (?)
mistdemo-swift-6.4-noble 11.08% <9.80%> (?)
spm 75.84% <93.55%> (?)
swift-6.1-jammy 75.83% <93.55%> (?)
swift-6.1-noble 75.78% <93.55%> (?)
swift-6.2-jammy 75.81% <93.55%> (?)
swift-6.2-noble 75.81% <93.55%> (?)
swift-6.3-jammy 75.81% <93.55%> (?)
swift-6.3-noble 75.83% <93.55%> (?)
swift-6.4-jammy 75.83% <93.55%> (?)
swift-6.4-noble 75.83% <93.55%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

leogdion and others added 3 commits August 21, 2026 15:14
Resolves the 14 SwiftLint violations outstanding when the implementing
agent stalled, plus two periphery findings introduced by the file splits:

- Expand `guard ... else { return nil }` one-liners
  (conditional_returns_on_newline) in DatabaseChangesResult,
  RecordZoneChangesResult, ZoneChangesAccumulator and
  CloudKitService+DatabaseChanges.
- Put each argument on its own line in the paginator's
  accumulator.merge(...) call (multiline_arguments{,_brackets}).
- Wrap the unbreakable DocC symbol link in ZoneChangesRequest with a
  scoped line_length disable/enable pair, keeping the doc comment
  contiguous so it stays attached to the declaration.
- Split CloudKitError+ZoneErrorDescription.swift out of
  CloudKitError+ErrorDescription.swift, and
  ...FetchRecordZoneChanges+PaginationLimits.swift out of
  ...+Pagination.swift, to bring both files under the length limit.
- Drop the import and test helper the splits left unused.

swift test: 573 tests / 182 suites passing. Scripts/lint.sh: 0
violations in 416 files, no unused code. generate-openapi.sh reproduces
the committed Sources/MistKitOpenAPI output.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Introduce fetch-database-changes and fetch-zone-record-changes CLI,
migrate phases/web off deprecated fetchZoneChanges, and keep
fetch-changes on records/changes.

Co-authored-by: Cursor <cursoragent@cursor.com>
@leogdion
leogdion force-pushed the 47-401-change-tracking-endpoints branch from 3546bcf to 99b9f1e Compare August 21, 2026 19:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant