Skip to content

modifyZones per-zone failures (#431) + zones/changes metaSyncToken (#430) - #443

Merged
leogdion merged 7 commits into
v1.0.0-beta.4from
431-modifyzones-per-zone-failures
Aug 29, 2026
Merged

modifyZones per-zone failures (#431) + zones/changes metaSyncToken (#430)#443
leogdion merged 7 commits into
v1.0.0-beta.4from
431-modifyzones-per-zone-failures

Conversation

@leogdion

@leogdionleogdion commented Aug 28, 2026

Copy link
Copy Markdown
Member

Closes#431. Closes#430. Also lands part 3 of #433 and a CI guard for generated output.

1. modifyZones per-zone failures (#431)

zones/modify is a batch endpoint whose realistic failure mode is partial — creating five zones where one already exists, deleting zones where one is missing. modifyZones mapped every response entry straight through as a success, so those failures were reported to the caller as a shorter-than-expected array naming no zone, with CloudKit's serverErrorCode and reason discarded.

Spec.ZonesModifyResponse.zones items become oneOf: [ZoneFetchFailure, Zone]. No new schema was needed: .claude/memory/reference_cloudkit_zone_dictionary.md records that all four zone endpoints — zones/modify included — route failures through Apple's Zone Fetch Error Dictionary, already modeled as ZoneFetchFailure for changes/database / changes/zone. The failure variant is listed first, matching every other oneOf in the file; ZoneFetchFailure requires serverErrorCode, so a success payload fails that branch and falls through to Zone.

Service.modifyZones returns a bare [ZoneChangeResult], mirroring modifyRecords returning a bare [RecordResult]. zones/modify has no batch-level metadata, so there is deliberately noDatabaseChangesResult-style wrapper struct. ZoneChangeResult / ZoneOperationFailure from #429 are reused rather than duplicated — the only new decoding code is a second internal init(from:) overload on the existing generic constraint.

⚠️ Breaking change + migration

modifyZones(_:database:) returns [ZoneChangeResult] instead of [ZoneInfo].

// before
letzones:[ZoneInfo]=tryawait service.modifyZones(ops, database:.private)
// after — successes only, same as before
letzones:[ZoneInfo]=tryawait service.modifyZones(ops, database:.private).zones
// after — the point of the change
letresults=tryawait service.modifyZones(ops, database:.private)forfailurein results.failures {print("\(failure.zoneName): \(failure.serverErrorCode.rawValue)\(failure.reason ??"")")}
// or rethrow per entry
forresultin results { _ =try result.get()} // throws .zoneOperationFailed

createZone / deleteZone keep their signatures. Verified: no call sites in Examples/BushelCloud or Examples/CelestraCloud, so no subrepo work is needed.

Two real bugs fixed as a consequence

  • createZone threw a bare CloudKitError.invalidResponse — no code, no reason, not even the zone name — when CloudKit rejected the create. It now calls .get() on the entry and throws .zoneOperationFailed carrying the full failure.
  • deleteZone did _ = try await modifyZones(...), so a ZONE_NOT_FOUND delete was reported to the caller as success. It now checks every entry.

.zones / .failures accessors — design decision

The issue asks for .zones / .failures like DatabaseChangesResult has, and the plan was one generic extension Array where Element == OperationResult<Success, Target> so it would also retro-fit [RecordResult]. That cannot be expressed in Swift — an extension's where clause cannot bind free generic parameters (error: cannot find type 'Success' in scope). Falling back to concrete extensions, which also read better at the call site than a generic .successes would:

  • Array+ZoneChangeResult.swift.zones / .failures
  • Array+RecordResult.swift.records / .failures (the requested [RecordResult] retro-fit)

[SubscriptionResult] was left alone as out of scope for this issue.

2. zones/changes uses metaSyncToken — key renamed (#430, #433 part 3)

This started as a description-only fix (#433 part 3): the request property was named syncToken but described as "Meta-sync token from previous operation". A live container run then proved the key itself is wrong, so it became an actual rename.

Live evidence

POST zones/changes against iCloud.com.brightdigit.MistDemo / development / private, with web-auth credentials:

  1. The response's top-level keys are exactly [moreComing, metaSyncToken, zones]syncToken is absent.
  2. Round-trip from the same baseline token:
    • sending {"syncToken": …} (what MistKit sent) → all 40 zones returned again. The key is silently ignored and page one replays.
    • sending {"metaSyncToken": …}0 zones. Token honored, correctly advanced.

So fetchZoneChanges / fetchAllZoneChanges pagination has never worked. Full evidence is in #430.

What changed

The wire key is renamed for zones/changes only — the request body property and ZoneChangesResponse. changes/database, changes/zone and records/changes genuinely use syncToken and are untouched.

Not source-breaking. Every Swift-facing name is unchanged: ZoneChangesResult.syncToken, its init(syncToken:) label, and the fetchZoneChanges(syncToken:) / fetchAllZoneChanges(syncToken:) argument labels. MistKitOpenAPI is an internal import, so the rename is invisible to consumers; only two lines move — the mapping in ZoneChangesResult.init(from:) and the request construction in CloudKitService+ZoneOperations.swift.

New CloudKitServiceTests.FetchZoneChanges+WireFormat.swift pins all three directions against regression, using the existing ResponseProvider.requestLog capture:

Existing zones/changes fixtures were emitting the wrong key and are corrected.

Also observed in the live response and deliberately not addressed here (being filed separately): each zone came back as {"zoneID": {"zoneName","ownerRecordName","zoneType"}, "deleted": true}. MistKit's Zone schema models neither deleted nor zoneType.

3. Generated-output reproducibility + CI guard

CodeFactor's bot committed to generated files in 61235b5, alphabetizing the Foundation imports in Sources/MistKitOpenAPI/Client.swift and Types.swift (moving import struct Foundation.URL after .Data/.Date in both #if os(Linux) arms). The generator emits URL/Data/Date order, so ./Scripts/generate-openapi.sh no longer reproduced the committed output. Regenerating on this branch repairs it — confirmed.

New .github/workflows/check-generated-openapi.yml runs ./Scripts/generate-openapi.sh then git diff --exit-code Sources/MistKitOpenAPI/ on every PR, so this cannot recur silently. It builds the generator from Scripts/OpenAPITools, whose version is pinned in sync with mise.toml, so the job is self-contained (no mise, no network beyond SwiftPM).

Note for @leogdion:.codefactor.yml already lists Sources/MistKitOpenAPI/** under exclude:, and the bot committed to those files anyway. The exclude appears to govern analysis, not the auto-fix commits — worth disabling auto-fix commits on the CodeFactor side as well; this workflow only catches the damage after the fact.

Also excluded Scripts/OpenAPITools/.build in .swiftlint.yml: the generator fallback leaves SwiftPM checkouts there, and the bare .build entry only matches the repo-root one, so a local lint run after using the fallback walked ~2,000 vendored files.

Verification

CheckResult
./Scripts/generate-openapi.sh && git diff --exit-code Sources/MistKitOpenAPI/clean
swift build && swift test (root)626 tests / 198 suites passed
swift build && swift test (Examples/MistDemo)994 tests / 295 suites passed, 1 known issue
LINT_MODE=STRICT ./Scripts/lint.sh28 violations, all pre-existing — none in files touched here
Sources/MistKitOpenAPI/Client.swift after the renameunchanged (ZoneChangesResponse is body-only)
LINT_MODE=STRICT ./Scripts/lint.sh (Examples/MistDemo)36 violations, matching baseline

Sanity-checked both changes by reverting them:

  • Reverting the service to drop error entries makes all four new modifyZones failure tests fail (results.count == 2, results.failures.first, and the createZone / deleteZone expectations).
  • Reverting the wire key to syncToken (spec + regenerate + the two mapping lines) makes all three WireFormat tests fail, plus four pre-existing FetchZoneChanges tests — including fetchAllZoneChanges() accumulates zones across three pages, which is exactly the pagination path zones/changes may use metaSyncToken, not syncToken — pagination possibly broken #430 breaks.

MistDemo call sites

  • ModifyZonesCommandoutputResults<T: Encodable> cannot render OperationResult (Sendable-only). Rejections go to stderr as warnings, matching how ModifyCommand reports per-record failures; stdout keeps the successful zones so JSON/CSV/table output stays machine-parseable.
  • CloudKitService+WebBackend.webModifyZones — collapses all-or-nothing via .get(), matching the documented decision already in webLookupRecords, so the web panel surfaces a rejection instead of silently returning fewer zones. Signature stays [ZoneInfo], so WebBackend, WebServer+Zones, MockBackend and WebServerTests+Zones needed no changes.
  • ModifyZonesPhase — previously discarded results; now fails the integration run when the create or the cleanup delete is rejected.

🤖 Generated with Claude Code

@coderabbitai

coderabbitaiBot commented Aug 28, 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: c3af3b6b-4c29-4daa-9ff6-edf676e0b440

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.

@leogdionleogdion changed the title modifyZones: surface per-zone failures (#431)modifyZones per-zone failures (#431) + zones/changes metaSyncToken (#430)Aug 28, 2026
@codecov

codecovBot commented Aug 28, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 81.75%. Comparing base (608ff50) to head (a2055af).

Additional details and impacted files
@@ Coverage Diff @@## v1.0.0-beta.4 #443 +/- ##
=================================================
+ Coverage 81.64% 81.75% +0.10% 
=================================================
Files 192 194 +2 Lines 4723 4751 +28 =================================================
+ Hits 3856 3884 +28 
Misses 867 867 
FlagCoverage Δ
mistdemo-spm-macos11.23% <0.00%> (-0.07%)⬇️
mistdemo-swift-6.2-jammy11.24% <0.00%> (-0.07%)⬇️
mistdemo-swift-6.2-noble11.24% <0.00%> (-0.07%)⬇️
mistdemo-swift-6.3-jammy11.24% <0.00%> (-0.07%)⬇️
mistdemo-swift-6.3-noble11.24% <0.00%> (-0.07%)⬇️
mistdemo-swift-6.4-jammy11.24% <0.00%> (-0.07%)⬇️
mistdemo-swift-6.4-noble11.24% <0.00%> (?)
spm80.40% <100.00%> (+0.07%)⬆️
swift-6.1-jammy80.40% <100.00%> (+0.01%)⬆️
swift-6.1-noble80.31% <100.00%> (-0.14%)⬇️
swift-6.2-jammy80.27% <100.00%> (+0.05%)⬆️
swift-6.2-noble80.42% <100.00%> (+0.11%)⬆️
swift-6.3-jammy80.29% <100.00%> (+0.07%)⬆️
swift-6.3-noble80.42% <100.00%> (+0.26%)⬆️
swift-6.4-jammy80.46% <100.00%> (+0.13%)⬆️
swift-6.4-noble80.52% <100.00%> (+0.30%)⬆️

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
leogdionforce-pushed the 431-modifyzones-per-zone-failures branch from 23639c1 to caeb912CompareAugust 28, 2026 20:15
leogdion added a commit that referenced this pull request Aug 28, 2026
Covers the retro-fit Array+RecordResult helpers so codecov patch
coverage meets the project threshold on #443.
Co-authored-by: Cursor <cursoragent@cursor.com>
leogdion added a commit that referenced this pull request Aug 28, 2026
Covers the retro-fit Array+RecordResult helpers so codecov patch
coverage meets the project threshold on #443.
Co-authored-by: Cursor <cursoragent@cursor.com>
@leogdion
leogdionforce-pushed the 431-modifyzones-per-zone-failures branch from 2e0ecc8 to 7018bd5CompareAugust 28, 2026 22:53
leogdionand others added 7 commits August 28, 2026 20:01
`ZonesModifyResponse.zones` items become `oneOf: [ZoneFetchFailure, Zone]`,
matching `changes/database` and `changes/zone`. `zones/modify` is a batch
endpoint whose realistic failure mode is partial, and Apple routes all four
zone endpoints' failures through the same Zone Fetch Error Dictionary, so the
error variant already exists — it just was not wired to this response.
The failure variant is listed first, matching every other `oneOf` in the spec.
`ZoneFetchFailure` requires `serverErrorCode`, so a success payload fails that
branch and falls through to `Zone`.
Also rewords the `zones/changes` request `syncToken` description (#433 part 3):
it was described as "Meta-sync token", a name the spec does not use for the
key. The key stays `syncToken` (#430); only the prose is corrected.
Regenerating also repairs Sources/MistKitOpenAPI reproducibility: CodeFactor's
bot alphabetized the Foundation imports in Client.swift/Types.swift in 61235b5,
so `./Scripts/generate-openapi.sh` no longer reproduced the committed output.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Xs1c8vvxjCxqZiStcmuPS2
`modifyZones` mapped every entry straight through as a success, so a batch
where some zones failed was reported as a partial success naming no zone and
discarding CloudKit's `serverErrorCode`/`reason`.
It now returns a bare `[ZoneChangeResult]` — one entry per zone the server
returned — mirroring how `modifyRecords` returns a bare `[RecordResult]`.
`zones/modify` carries no batch-level metadata, so there is deliberately no
`DatabaseChangesResult`-style wrapper struct. `ZoneChangeResult` /
`ZoneOperationFailure` from #429 are reused rather than duplicated; the only
new code is a second `init(from:)` overload keyed off the generated
`ZonesModifyResponse.zonesPayloadPayload`.
Two convenience-wrapper bugs fall out of this:
- `createZone` threw a bare `.invalidResponse` with no code, reason or zone
name when CloudKit rejected the create. It now calls `.get()`, throwing
`.zoneOperationFailed` with the full failure.
- `deleteZone` discarded the result entirely, so a `ZONE_NOT_FOUND` delete was
reported to the caller as success. It now checks every entry.
`.zones` / `.failures` accessors are added as concrete `Array` extensions
(`[ZoneChangeResult]`, plus `.records`/`.failures` on `[RecordResult]`) rather
than one generic extension over `OperationResult<Success, Target>`: Swift
cannot bind free generic parameters in an extension's `where` clause.
BREAKING: `modifyZones` returns `[ZoneChangeResult]`, not `[ZoneInfo]`.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Xs1c8vvxjCxqZiStcmuPS2
Adds a raw-dictionary `ResponseConfig.modifyZonesResponse(zones:)` builder — so
a single response can mix success entries and zone error entries — mirroring
`databaseChangesResponse(zones:syncToken:moreComing:)`, and a
`makeService(zones:)` harness over it.
New tests: a mixed batch keeps the successes and reports the failure with its
zone name/code/reason; `.get()` on a failed entry throws `.zoneOperationFailed`;
zone metadata survives the new success variant; `createZone` surfaces the
`ZoneOperationFailure` instead of `.invalidResponse`; and `deleteZone` throws on
`ZONE_NOT_FOUND` rather than reporting success.
Verified the failure tests fail when the service is reverted to dropping error
entries. `ZoneMetadataTests` now matches on the `oneOf` success variant.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Xs1c8vvxjCxqZiStcmuPS2
- `modify-zones` announces per-zone rejections on stderr (matching how `modify`
reports per-record failures) and keeps stdout to the zones that were actually
modified, so the JSON/CSV/table output stays machine-parseable.
`outputResults` requires `Encodable` and `OperationResult` is `Sendable`-only,
so the results cannot be rendered directly.
- `webModifyZones` collapses the results all-or-nothing via `.get()`, matching
the documented decision in `webLookupRecords`, so the web panel shows a
rejection instead of silently returning fewer zones than were asked for. Its
`[ZoneInfo]` signature is unchanged, so `WebBackend`, the routes and the mock
backend need no changes.
- `ModifyZonesPhase` asserted nothing about the results; it now fails the
integration run when a create or the cleanup delete is rejected. Split into
`createAndVerify` to stay under the cyclomatic-complexity limit.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Xs1c8vvxjCxqZiStcmuPS2
CodeFactor's bot committed to `Sources/MistKitOpenAPI/` in 61235b5, reordering
imports in generated files, which broke `./Scripts/generate-openapi.sh`
reproducibility until this branch regenerated them. Nothing checked that.
`check-generated-openapi.yml` now regenerates and runs
`git diff --exit-code Sources/MistKitOpenAPI/` on every PR. It builds the
generator from `Scripts/OpenAPITools`, whose version is pinned in sync with
mise.toml, so the check is self-contained.
That fallback build leaves SwiftPM checkouts in `Scripts/OpenAPITools/.build`,
which SwiftLint then walked (the bare `.build` exclude only matches the repo
root one) — added as an explicit exclude.
Docs: AGENTS.md's per-zone-failures paragraph now covers `zones/modify` and
records the oneOf ordering rationale, the operations table names the new return
type, and README points at `modifyZones` alongside `createZone`/`deleteZone`.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Xs1c8vvxjCxqZiStcmuPS2
A live container run (iCloud.com.brightdigit.MistDemo / development / private,
web-auth) proved `zones/changes` neither returns nor honors `syncToken`:
- The response's top-level keys are exactly `[moreComing, metaSyncToken,
zones]` — no `syncToken` at all.
- Round-tripping the same baseline token: sending `{"syncToken": …}` (what
MistKit sent) returned all 40 zones again — the key is silently ignored and
page one replays. Sending `{"metaSyncToken": …}` returned 0 zones — honored
and correctly advanced.
So `fetchZoneChanges` / `fetchAllZoneChanges` pagination has never worked. This
supersedes the description-only wording fix in the previous commit, which
assumed the mismatch was documentation rather than behavior.
Renames the wire key for `zones/changes` **only** — the request body property
and `ZoneChangesResponse` — and regenerates. `changes/database`, `changes/zone`
and `records/changes` legitimately use `syncToken` and are untouched.
Every Swift-facing name is deliberately unchanged: `ZoneChangesResult.syncToken`
and its `init(syncToken:)` label, and the `fetchZoneChanges(syncToken:)` /
`fetchAllZoneChanges(syncToken:)` argument labels. `MistKitOpenAPI` is an
`internal import`, so a wire-key rename is not source-breaking for consumers;
only the mapping in `ZoneChangesResult.init(from:)` and the request
construction in `CloudKitService+ZoneOperations.swift` change.
Adds `CloudKitServiceTests.FetchZoneChanges+WireFormat.swift`, which pins that
MistKit sends `metaSyncToken` and never `syncToken`, reads `metaSyncToken` in
preference to a decoy `syncToken`, and feeds the previous page's token back
under the honored key. Existing `zones/changes` fixtures were emitting the
wrong key and are updated.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Xs1c8vvxjCxqZiStcmuPS2
Covers the retro-fit Array+RecordResult helpers so codecov patch
coverage meets the project threshold on #443.
Co-authored-by: Cursor <cursoragent@cursor.com>
@leogdion
leogdionforce-pushed the 431-modifyzones-per-zone-failures branch from 7018bd5 to a2055afCompareAugust 29, 2026 00:01
@leogdion
leogdion merged commit 91f04f3 into v1.0.0-beta.4Aug 29, 2026
85 checks passed
@leogdion
leogdion deleted the 431-modifyzones-per-zone-failures branch August 29, 2026 12:05
Sign up for freeto 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

@leogdion