Skip to content

V1.0.0 beta.2#395

Open
leogdion wants to merge 35 commits into
mainfrom
v1.0.0-beta.2
Open

V1.0.0 beta.2#395
leogdion wants to merge 35 commits into
mainfrom
v1.0.0-beta.2

Conversation

@leogdion
Copy link
Copy Markdown
Member

No description provided.

leogdion and others added 26 commits May 18, 2026 11:50
commit de82483
Author: Leo Dion <leogdion@brightdigit.com>
Date:   Sun May 17 21:14:35 2026 +0100

    git subrepo push Examples/CelestraCloud

    subrepo:
      subdir:   "Examples/CelestraCloud"
      merged:   "ea897c3"
    upstream:
      origin:   "git@github.com:brightdigit/CelestraCloud.git"
      branch:   "mistkit"
      commit:   "ea897c3"
    git-subrepo:
      version:  "0.4.9"
      origin:   "https://github.com/Homebrew/brew"
      commit:   "6f293daa9f"

commit 24c8719
Author: Leo Dion <leogdion@brightdigit.com>
Date:   Sun May 17 21:14:31 2026 +0100

    git subrepo push Examples/BushelCloud

    subrepo:
      subdir:   "Examples/BushelCloud"
      merged:   "5bb4490"
    upstream:
      origin:   "git@github.com:brightdigit/BushelCloud.git"
      branch:   "mistkit"
      commit:   "5bb4490"
    git-subrepo:
      version:  "0.4.9"
      origin:   "https://github.com/Homebrew/brew"
      commit:   "6f293daa9f"

commit eee0670
Author: Leo Dion <leogdion@brightdigit.com>
Date:   Sun May 17 21:14:13 2026 +0100

    docs: sync README/CLAUDE examples to v1.0.0-beta.1 API; pin BushelCloud CI; exclude internal Python from CodeFactor

    - README.md, Examples/BushelCloud/{CLAUDE.md,.docc,.claude/s2s-auth-details.md},
      Examples/CelestraCloud/{CLAUDE.md,README.md,.claude/IMPLEMENTATION_NOTES.md}:
      drop `try CloudKitService(... database: .public)` from init examples (init is
      non-throwing, `database:` moved per-call); rewrite Quick Start auth around
      `Credentials` + `APICredentials` / `ServerToServerCredentials` and show
      `database: .public(.prefers(.serverToServer))` at the call site.
    - Examples/BushelCloud/.github/workflows/{BushelCloud.yml,bushel-cloud-build.yml}:
      pin MISTKIT_BRANCH to v1.0.0-beta.1 (matches CelestraCloud) so the subrepo PR
      builds against the branch that actually carries the new API. Revert to `main`
      once #298 merges.
    - .codefactor.yml: exclude Scripts/mermaid-to-pptx.py (internal-use helper).

    Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

commit 4d60b19
Author: Leo Dion <leogdion@brightdigit.com>
Date:   Sun May 17 20:10:45 2026 +0100

    git subrepo push Examples/CelestraCloud

    subrepo:
      subdir:   "Examples/CelestraCloud"
      merged:   "c44dc4f"
    upstream:
      origin:   "git@github.com:brightdigit/CelestraCloud.git"
      branch:   "mistkit"
      commit:   "c44dc4f"
    git-subrepo:
      version:  "0.4.9"
      origin:   "https://github.com/Homebrew/brew"
      commit:   "6f293daa9f"

commit 5bc403d
Author: Leo Dion <leogdion@brightdigit.com>
Date:   Sun May 17 20:10:40 2026 +0100

    git subrepo push Examples/BushelCloud

    subrepo:
      subdir:   "Examples/BushelCloud"
      merged:   "55f2092"
    upstream:
      origin:   "git@github.com:brightdigit/BushelCloud.git"
      branch:   "mistkit"
      commit:   "55f2092"
    git-subrepo:
      version:  "0.4.9"
      origin:   "https://github.com/Homebrew/brew"
      commit:   "6f293daa9f"

commit bce1f23
Author: leogdion <leogdion@brightdigit.com>
Date:   Sun May 17 20:09:47 2026 +0100

    refactor!: prep for talk — shrink API, refactor auth, split OpenAPI (#279)

commit 7023a31
Author: leogdion <leogdion@brightdigit.com>
Date:   Fri May 15 12:56:58 2026 -0400

    Fixed Nonisolated Web Auth Token (#347)

commit f799128
Author: leogdion <leogdion@brightdigit.com>
Date:   Thu May 14 20:27:28 2026 -0400

    Add MistDemo-Integration workflow for live CloudKit runs (#345)

commit 418e2e4
Author: leogdion <leogdion@brightdigit.com>
Date:   Thu May 14 16:03:04 2026 -0400

    Resolve #342: v1.0.0-beta.1 follow-ups (#341 #327 #321 #317) + CI fixes (#343)

commit d65d20b
Author: leogdion <leogdion@brightdigit.com>
Date:   Thu May 14 11:25:10 2026 -0400

    Resolve #330: interactive MistDemo (web toggle + native app refresh) (#332)

commit a28ab3c
Author: leogdion <leogdion@brightdigit.com>
Date:   Mon May 11 16:31:10 2026 -0400

    Resolve #313: paginationLimitExceeded carries accumulated records (#326)

commit 7a5da7a
Author: leogdion <leogdion@brightdigit.com>
Date:   Sat May 9 17:09:53 2026 -0400

    Fix CI failures + Claude review nits on PR #298 (v1.0.0-beta.1) (#322)

commit b3626c0
Author: leogdion <leogdion@brightdigit.com>
Date:   Sat May 9 16:06:20 2026 -0400

    Resolve #312: public+web-auth user-identity endpoints (#310, #311, #27, #28, #34, #35) (#315)

    * #312 library: add public+web-auth user-identity endpoints and users/caller migration

    Implements the library side of #312 — adding/renaming user-identity endpoints
    that require public-database routing with web-auth (user-context) credentials,
    and unblocking the convenience initializers from their hardcoded database/
    environment defaults.

    #310: `CloudKitService` convenience initializers now accept `database:` and
    `environment:` parameters with defaults that preserve current behavior.

    #311: `users/current` → `users/caller`. Renamed in openapi.yaml and the
    generated client; added a hand-written `fetchCaller()` plus an
    `@available(*, deprecated, renamed: "fetchCaller")` `fetchCurrentUser()`
    shim that forwards to the new method.

    #28: GET `/users/discover` (`discoverAllUserIdentities`).

    #34: POST `/users/lookup/email` (`lookupUsersByEmail`).

    #35: POST `/users/lookup/id` (`lookupUsersByRecordName`).

    The three new endpoints reuse `DiscoverResponse` for parsing — Apple returns
    `{ users: [UserIdentity] }` for all of them. Each ships with a 5-file
    test suite mirroring the existing `DiscoverUserIdentities` pattern.

    #33 (`users/lookup/contacts`) intentionally not implemented: Apple has marked
    the endpoint deprecated. To be closed as not-planned with a pointer to #34/#35.

    Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

    * #312 MistDemo: separate database from authentication and add user-context phases

    Refactors MistDemo's CloudKit configuration model and integration runner to
    support the public+web-auth combination required by the user-identity
    endpoints landed in the prior commit.

    **Configuration refactor.** Replaces the `DatabaseCredentials` enum (which
    coupled database choice to a single auth method per case, baking in a
    public⇒S2S/private⇒webAuth assumption) with two orthogonal types:

      - `AuthenticationCredentials` — `serverToServer(keyID:privateKey:)` /
        `webAuth(apiToken:webAuthToken:)`
      - `DatabaseConfiguration` — pairs a `MistKit.Database` with an
        `AuthenticationCredentials`. The `make(database:authentication:)` factory
        rejects private+S2S and shared+S2S (which CloudKit rejects) so invalid
        combinations remain unrepresentable, while public+webAuth is now a valid
        construction.

    `MistKitClientFactory.create(for:)` consumes `toPrimaryConfiguration()`;
    the new `createUserContext(for:)` returns the optional public+web-auth
    service from `toUserContextConfiguration()` when web-auth tokens are
    configured.

    **Phase plumbing.** `PhaseContext` and `IntegrationTestRunner` now thread an
    optional `userContextService: CloudKitService?`. `PublicDatabaseTest` takes
    `includeUserContextPhases:` and conditionally appends the new user-identity
    phases:

      - `FetchCallerPhase` (renamed from `FetchCurrentUserPhase`)
      - `DiscoverUserIdentitiesPhase` (existed; updated to use userContextService)
      - `DiscoverAllUserIdentitiesPhase` (#28)
      - `LookupUsersByEmailPhase` (#34)
      - `LookupUsersByRecordNamePhase` (#35)

    `PrivateDatabaseTest` no longer includes `FetchCurrentUserPhase`: CloudKit
    rejects `users/caller` against the private database, matching the rest of
    the user-identity family.

    **Call-site updates.** `CurrentUserCommand` and `DemoErrorsRunner` swap
    `fetchCurrentUser()` → `fetchCaller()`. `TestIntegrationCommand` and
    `TestPrivateCommand` now build and pass `userContextService`.

    Tests for `AuthenticationCredentials`, `DatabaseConfiguration.make`
    validation, and `MistDemoConfig.toPrimaryConfiguration` /
    `toUserContextConfiguration` ship alongside.

    Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

    * #312: mark discoverAllUserIdentities() unavailable pending #28 investigation

    Live verification on 2026-05-08 against iCloud.com.brightdigit.MistDemo
    returned HTTP 500 from Apple's GET /users/discover. The first 12 phases
    of mistdemo test-integration --verbose run green (the 8 base public+S2S
    phases plus FetchCallerPhase, DiscoverUserIdentitiesPhase,
    LookupUsersByEmailPhase, LookupUsersByRecordNamePhase) — only
    discoverAllUserIdentities fails, blocking phases beyond it.

    The endpoint is referenced in CloudKitJS but does not appear in Apple's
    CloudKit Web Services REST documentation. The actual REST shape is still
    under investigation under #28.

    Changes:
    - Marked `CloudKitService.discoverAllUserIdentities()`
      `@available(*, unavailable, message: ...)` with a pointer to #28.
    - Removed `DiscoverAllUserIdentitiesPhase` from MistDemo and from
      `PublicDatabaseTest.phases`.
    - Removed the `CloudKitServiceDiscoverAllUserIdentities` test directory
      (the unavailable method cannot be called from Swift code).

    The OpenAPI definition, generated client, path builder, response
    processor, Output extension, and Swift wrapper are all retained.
    Unblocking is a one-line `@available` removal once the correct REST
    shape is determined under #28.

    Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

    * #315: resolve PR review — Credentials API, per-call database, cascade unavailable

    Addresses all four review threads on PR #315:

    - Comment #1 (error wording): removed `unsupportedDatabaseAuthCombination`
      along with `MistDemo.DatabaseConfiguration`; invalid combos now surface as
      `CloudKitError.missingCredentials` from the library.
    - Comment #2 (per-call database): user-identity ops in
      `CloudKitService+UserOperations` hardcode `.public`; record/zone/asset/sync
      ops accept `database: Database? = nil` falling back to a service-level
      default.
    - Comment #3 (unified credentials): new `Credentials` /
      `ServerToServerCredentials` / `APICredentials` value types replace the
      legacy `apiToken:`/`webAuthToken:` initializers. The token manager is
      selected based on the target database (S2S for `.public`, web-auth for
      `.private`/`.shared`). Lifted `PrivateKeyMaterial` into the library.
    - Comment #4 (cascade unavailable): removed
      `Operations.discoverAllUserIdentities.Output: CloudKitResponseType`
      conformance entirely; `processDiscoverAllUserIdentitiesResponse` is now
      `@available(*, unavailable)` with a `fatalError` body.

    Also migrates ~15 MistKit test helpers and the MistDemo factory to the new
    Credentials API.

    Breaking changes (pre-1.0): removed legacy `CloudKitService` initializers
    taking `apiToken:`/`webAuthToken:`; `CloudKitService.apiToken` is removed,
    `.database` is now `internal`.

    Out of scope: per-call `TokenManager` dispatch (would let one service mix
    S2S-for-public and web-auth-for-user-context). MistDemo still constructs a
    separate `userContextService` for that scenario.

    Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

    * #315: drop service-level database, per-call credential resolution [skip ci]

    Resolves the architectural feedback in the PR-315 review:

    * CloudKitService no longer carries `database` — operations take
      `database:` per call (defaulting to `.public`); user-identity routes
      drop the parameter since CloudKit pins them to `.public`. Subsumes
      Claude's "fetchCaller bypasses self.database" finding.
    * Credentials.makeTokenManager(for:requiresUserContext:) resolves the
      appropriate token manager at dispatch time. A single service can now
      serve public-database S2S record ops and user-identity web-auth
      routes from one fully-populated `Credentials`. MistKitClient.swift is
      obsolete and removed; per-call dispatch lives in
      CloudKitService+ClientDispatch.
    * Credentials.swift split per SwiftLint one_file_per_declaration into
      ServerToServerCredentials.swift + APICredentials.swift +
      Credentials.swift. New typed CredentialsValidationError; init asserts
      in debug, throws in release (no more precondition crash for dynamic
      config).
    * MistDemo: userContextService workaround collapsed — single service
      handles all phases via per-call resolution.
    * CI hotfix: 11 unused `public import` lines demoted to `internal`
      (the warnings-as-errors regression flagged in the review).
    * Tests: 12-case routing-matrix unit suite for makeTokenManager and a
      fetchCaller suite parallel to LookupUsers* (success + validation).
      Obsolete MistKitClient tests removed.
    * Polish: shorter @available message on discoverAllUserIdentities,
      structural comment for GET /users/discover in openapi.yaml,
      ConfigurationError.missingAPIToken (unused) removed.

    475/475 tests pass. Library + MistDemo build clean.

    Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

    * Per review on PR #315: listZones, lookupZones, fetchZoneChanges now
    default to .private since the public database only contains
    _defaultZone, making .public a degenerate default. MistDemo callers
    pass context.database / config.base.database explicitly so the
    --database flag still drives the test runs.

    Also repairs MistDemo test breakage from 7debe8d:
    toUserContextCredentials() was removed but tests still referenced it;
    rewritten against the replacement surface (toPrimaryCredentials embeds
    apiAuth on .public, plus the new hasUserContextCredentials boolean).
    The CredentialsValidationTests suite was deleted — it asserted
    init-time validation that no longer exists under per-call credential
    resolution; the equivalent .missingCredentials behavior is covered in
    MistKitTests.

    Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

    * #312: gate @available(*,unavailable) on processDiscoverAllUserIdentitiesResponse to Swift 6.2+

    Swift 6.1 rejects calls to an unavailable function from within another
    unavailable function; 6.2 relaxed that rule. The internal helper
    processDiscoverAllUserIdentitiesResponse is unavailable in lockstep with
    its only caller — the also-unavailable CloudKitService.discoverAllUserIdentities() —
    which built fine on 6.2+ but failed on Swift 6.1 with:

        error: 'processDiscoverAllUserIdentitiesResponse' is unavailable:
               Pending #28: discoverAllUserIdentities is not yet ready.

    Wrap just the attribute in `#if swift(>=6.2)` so the body is shared and
    6.1 compiles. Inline doc records the intent and the one-line cleanup
    (delete the #if/#endif) once 6.1 is dropped from the matrix.

    A `swiftlint:disable:next unavailable_function` is required because
    swiftlint does not evaluate #if and otherwise sees a fatalError-only
    function without the attribute.

    Verified: swift build + swift test pass on Swift 6.1.3 (Linux container)
    and on macOS Swift 6.2+ (475/475 tests).

    Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

    * #315: split unhandled-response logging into debug (full body) + warning (type/status only)

    CodeQL's swift/cleartext-logging flagged the existing warning logs
    because lookupUsersByEmail(_:) propagates email-PII taint through the
    response object. Move full \(response) interpolation to .debug so the
    detail stays available for development without flowing into ops logs;
    keep .warning at type(of:) + HTTP status code only.

    Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

    * #312: add --lookup-email / CLOUDKIT_LOOKUP_EMAIL to exercise users/lookup/email

    LookupUsersByEmailPhase previously skipped whenever fetchCaller() didn't
    return an email (which is the common case). Plumb a configurable lookup
    email through TestIntegrationConfig / TestPrivateConfig → PhaseContext so
    the phase can be driven against a known-discoverable iCloud account.
    Falls back to caller email, then to a clearer skip message naming the
    flag/env var.

    Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

    * docs: point CLAUDE.md lint section at mise (and Scripts/lint.sh)

    swift-format / swiftlint / periphery are pinned in mise.toml; the
    previous "requires swiftlint installation" wording led to PATH lookups
    that fail in this repo. Replace with `mise exec --` invocations and
    flag the full ./Scripts/lint.sh pipeline.

    Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

    * #315: address review punch list — invalidPrivateKey, recoverable unavailable response, supportsUserContextPhases derivation

    - CloudKitError: add invalidPrivateKey(path:underlying:) so PEM-load failures
      carry the file path + original error instead of bare Foundation NSError.
      Wrap loadPEM() at the single call site in Credentials+TokenManager. Add
      PrivateKeyMaterial.filePath accessor for the diagnostic.

    - processDiscoverAllUserIdentitiesResponse: replace fatalError with
      throw CloudKitError.unsupportedOperationType so a stray Swift 6.1 caller
      (where the @available cascade does not apply) gets a recoverable error
      instead of a crash.

    - TestPrivateCommand: derive supportsUserContextPhases from
      config.base.hasUserContextCredentials, mirroring TestIntegrationCommand,
      so user-identity phases skip cleanly when web-auth env vars are absent.

    - toPrimaryCredentials: replace try? with do/catch + stderr INFO line so
      operators see when web-auth is missing on a .public setup.

    - CLAUDE.md: annotate discoverAllUserIdentities() as unavailable pending #28.

    - CredentialsTokenManagerTests: fill the missing routing-matrix branches
      (user-context × .private/.shared, .shared + token-only) and cover the new
      .invalidPrivateKey path.

    Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

    ---------

    Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

commit 6f92a71
Author: leogdion <leogdion@brightdigit.com>
Date:   Fri May 8 13:16:56 2026 -0400

    Resolve #308: docs refresh + CI fixes + sub-issues #165, #285 (#309)

commit a1e2162
Author: leogdion <leogdion@brightdigit.com>
Date:   Fri May 8 07:16:10 2026 -0400

    Add query pagination support with continuation markers (#306)

commit c62bf44
Author: leogdion <leogdion@brightdigit.com>
Date:   Thu May 7 15:52:45 2026 -0400

    Improve error handling: typed TokenManagerError and safe RecordOperation conversion (#305)

commit 7c4b678
Author: leogdion <leogdion@brightdigit.com>
Date:   Thu May 7 11:27:10 2026 -0400

    git subrepo push Examples/CelestraCloud

    subrepo:
      subdir:   "Examples/CelestraCloud"
      merged:   "4244497"
    upstream:
      origin:   "git@github.com:brightdigit/CelestraCloud.git"
      branch:   "mistkit"
      commit:   "4244497"
    git-subrepo:
      version:  "0.4.9"
      origin:   "https://github.com/Homebrew/brew"
      commit:   "b9763ee528"

commit f14e751
Author: leogdion <leogdion@brightdigit.com>
Date:   Thu May 7 11:27:07 2026 -0400

    git subrepo push Examples/BushelCloud

    subrepo:
      subdir:   "Examples/BushelCloud"
      merged:   "123a732"
    upstream:
      origin:   "git@github.com:brightdigit/BushelCloud.git"
      branch:   "mistkit"
      commit:   "123a732"
    git-subrepo:
      version:  "0.4.9"
      origin:   "https://github.com/Homebrew/brew"
      commit:   "b9763ee528"

commit a0f0af9
Author: leogdion <leogdion@brightdigit.com>
Date:   Thu May 7 11:26:32 2026 -0400

    updating example packages

commit 125dab5
Author: leogdion <leogdion@brightdigit.com>
Date:   Thu May 7 11:01:18 2026 -0400

    Refactor AuthenticationMiddleware so each Authenticator applies itself (#294)

commit f989fd1
Author: leogdion <leogdion@brightdigit.com>
Date:   Thu May 7 10:23:23 2026 -0400

    Strengthen environment and database configuration validation (#293)

commit b0f00a7
Author: leogdion <leogdion@brightdigit.com>
Date:   Thu May 7 10:18:52 2026 -0400

    Add operation classification and batch sync result tracking (#296)

commit 63a4e50
Author: leogdion <leogdion@brightdigit.com>
Date:   Thu May 7 10:09:27 2026 -0400

    Move CloudKitResponseType default implementations to protocol extension (#292)

commit ae1af15
Author: leogdion <leogdion@brightdigit.com>
Date:   Wed May 6 20:20:44 2026 -0400

    Test suite improvements for v1.0.0-beta.1 (#286) (#287)

commit 5475bfa
Author: leogdion <leogdion@brightdigit.com>
Date:   Tue May 5 20:21:32 2026 -0400

    MistDemo: --database flag + demo-errors command (closes #259, #269) (#282)

commit 8b21425
Author: leogdion <leogdion@brightdigit.com>
Date:   Tue May 5 20:21:17 2026 -0400

    Refactor IntegrationTestRunner into protocol-based phase pipeline (#254) (#283)

commit 9709f3d
Author: leogdion <leogdion@brightdigit.com>
Date:   Tue May 5 08:54:16 2026 -0400

    Replace custom AsyncChannel with swift-async-algorithms (#280)

commit d53467a
Author: leogdion <leogdion@brightdigit.com>
Date:   Mon May 4 12:49:25 2026 -0400

    CI Updates for May 2026 (#277)

commit d7b1a21
Author: Leo Dion <leogdion@brightdigit.com>
Date:   Thu Apr 30 09:39:09 2026 -0400

    MistDemo improvements: test split, CRUD, auth fix, native app (#271) (#273)

commit 0ab2ab6
Author: leogdion <leogdion@brightdigit.com>
Date:   Wed Apr 29 15:49:34 2026 -0400

    First Draft Revision of Docs (#268)
Adds four DocC articles to close out the open documentation gaps
tracked by parent issue #361:

- HandlingErrors.md — three-layer error model (construction, token,
  request) with retry/recovery guidance.
- ConfiguringMistKit.md — container/environment/transport/logging
  inputs; defers credential construction to AuthenticationAndDatabases.
- WorkingWithRecords.md — CRUD, batch, lookup, and sync-via-token walks.
- CloudKitLimitsAndPerformance.md — pagination guard, batch sizing,
  asset upload transport split, rate limits.

Also adds inline `# Example` blocks to createRecord, updateRecord
(CloudKitService+WriteOperations.swift) and lookupRecords
(CloudKitService+LookupOperations.swift) to match the existing example
style on queryRecords. modifyRecords and deleteRecord examples live in
the WorkingWithRecords article rather than inline to keep the write-
operations file under the file_length cap. Updates Documentation.md
Topics to surface the new articles and adds an Error Handling group
covering the typed error and reason enums.

Migration guide (mentioned in #160) is deferred — MistKit is at
1.0.0-beta.x with no stable predecessor; that article should land
alongside a future release transition.

Closes #115
Closes #116
Closes #160

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Updates the internals doc to match the current source:

- `RequestSignature` parameter is `webServiceSubpath`, not `webServiceURL`
  (S2S sequence diagram, signing-payload template, both initializer labels,
  and the embedded code example).
- Token-Manager-Selection flowchart no longer labels the
  `APITokenAuthenticator` arrow as "user-attributed" — API-token-only is
  neither of the two CloudKit attribution modes defined by
  `PublicAuthPreference`.
- Drop stale reference to `downgradeToAPIOnly()` /
  `updateWebAuthToken(_:)`; only `upgradeToWebAuthentication(_:)` exists.
- Fix `Credentials/Credentials+TokenManager.swift` path — the file lives
  directly under `Authentication/`, not in a `Credentials/` subdirectory.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Finishes the Zone API surface called out in #367 as "not shipped":
single-zone create/delete convenience helpers over modifyZones, and
the auto-paginating fetchAllZoneChanges (carve-out from #307).

Library:
- createZone / deleteZone (flat-param, mirrors createRecord pattern)
- fetchAllZoneChanges with maxPages ceiling, cancellation check,
  stuck-token detection, and invalid-response guard
- zonePaginationLimitExceeded sibling case on CloudKitError so the
  partial-results contract stays typed [ZoneInfo] rather than [RecordInfo]

MistDemo:
- create-zone / delete-zone CLI subcommands
- ZoneRoundtripPhase + FetchAllZoneChangesPhase in PrivateDatabaseTest
  so the new endpoints are exercised against live CloudKit

Closes #45, #47, #48 (audited as shipped per the parent issue),
plus the zone-changes paginator carve-out from #307.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The live MistDemo integration tests targeted a CloudKit record type
"MistKitIntegrationTest" that is not defined in the container schema,
causing CI failures. The deployed schema only defines a Note record type
with title/index/image fields, which the phases already populate.

https://claude.ai/code/session_01EvLrWZwcSs1MjiCrUx8KjU
subrepo:
  subdir:   "Examples/BushelCloud"
  merged:   "7f026ac"
upstream:
  origin:   "git@github.com:brightdigit/BushelCloud.git"
  branch:   "mistkit"
  commit:   "7f026ac"
git-subrepo:
  version:  "0.4.9"
  origin:   "https://github.com/Homebrew/brew"
  commit:   "6f293daa9f"
subrepo:
  subdir:   "Examples/CelestraCloud"
  merged:   "dbdba1a"
upstream:
  origin:   "git@github.com:brightdigit/CelestraCloud.git"
  branch:   "mistkit"
  commit:   "dbdba1a"
git-subrepo:
  version:  "0.4.9"
  origin:   "https://github.com/Homebrew/brew"
  commit:   "6f293daa9f"
The 375-fieldvaluerequest-timestamp source branch was deleted after PR
#377 squash-merged into v1.0.0-beta.2 (a7a5654), leaving the CelestraCloud
workflow envs pointing at a missing ref.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The recorded parent (7d98901) was orphaned when PR #377 squash-merged into
v1.0.0-beta.2 as a7a5654. Reset to the last successful subrepo-push commit
(86590e0) per git-subrepo's recovery hint.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
subrepo:
  subdir:   "Examples/CelestraCloud"
  merged:   "910b9fb"
upstream:
  origin:   "git@github.com:brightdigit/CelestraCloud.git"
  branch:   "mistkit"
  commit:   "910b9fb"
git-subrepo:
  version:  "0.4.9"
  origin:   "https://github.com/Homebrew/brew"
  commit:   "af835384ac"
subrepo:
  subdir:   "Examples/BushelCloud"
  merged:   "fa92cff"
upstream:
  origin:   "git@github.com:brightdigit/BushelCloud.git"
  branch:   "main"
  commit:   "fa92cff"
git-subrepo:
  version:  "0.4.9"
  origin:   "https://github.com/Homebrew/brew"
  commit:   "af835384ac"
The preceding force-pull (27eebaf) used clone --force semantics and
silently retargeted to upstream's default branch (main). The recorded
commit fa92cff is upstream mistkit's HEAD, so only the branch field
needed correcting.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 26, 2026

Important

Review skipped

Too many files!

This PR contains 300 files, which is 150 over the limit of 150.

To get a review, narrow the scope:
• coderabbit review --type committed # exclude uncommitted changes
• coderabbit review --dir # limit to a subdirectory
• coderabbit review --base # compare against a closer base

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 28df5963-a522-4622-814c-0eab85d2f588

📥 Commits

Reviewing files that changed from the base of the PR and between 2537f39 and b0b361a.

📒 Files selected for processing (300)
  • .codefactor.yml
  • .github/actions/setup-mistkit/action.yml
  • .github/workflows/MistDemo.yml
  • .swiftlint.yml
  • CLAUDE.md
  • Examples/BushelCloud/.claude/implementation-patterns.md
  • Examples/BushelCloud/.claude/s2s-auth-details.md
  • Examples/BushelCloud/.github/actions/cloudkit-sync/action.yml
  • Examples/BushelCloud/.github/workflows/BushelCloud.yml
  • Examples/BushelCloud/.github/workflows/bushel-cloud-build.yml
  • Examples/BushelCloud/.github/workflows/codeql.yml
  • Examples/BushelCloud/.gitrepo
  • Examples/BushelCloud/CLAUDE.md
  • Examples/BushelCloud/Package.resolved
  • Examples/BushelCloud/Package.swift
  • Examples/BushelCloud/README.md
  • Examples/BushelCloud/Sources/BushelCloudCLI/BushelCloudCLI.swift
  • Examples/BushelCloud/Sources/BushelCloudCLI/Commands/ClearCommand.swift
  • Examples/BushelCloud/Sources/BushelCloudCLI/Commands/ExportCommand.swift
  • Examples/BushelCloud/Sources/BushelCloudCLI/Commands/ListCommand.swift
  • Examples/BushelCloud/Sources/BushelCloudCLI/Commands/StatusCommand.swift
  • Examples/BushelCloud/Sources/BushelCloudCLI/Commands/SyncCommand.swift
  • Examples/BushelCloud/Sources/BushelCloudKit/BushelCloud.docc/CloudKitIntegration.md
  • Examples/BushelCloud/Sources/BushelCloudKit/CloudKit/BushelCloudKitService.swift
  • Examples/BushelCloud/Sources/BushelCloudKit/CloudKit/CloudKitAuthMethod.swift
  • Examples/BushelCloud/Sources/BushelCloudKit/CloudKit/KeyIDValidator.swift
  • Examples/BushelCloud/Sources/BushelCloudKit/CloudKit/OperationClassification.swift
  • Examples/BushelCloud/Sources/BushelCloudKit/CloudKit/PEMValidator.swift
  • Examples/BushelCloud/Sources/BushelCloudKit/CloudKit/SyncEngine+Export.swift
  • Examples/BushelCloud/Sources/BushelCloudKit/CloudKit/SyncEngine.swift
  • Examples/BushelCloud/Sources/BushelCloudKit/Configuration/BushelConfiguration.swift
  • Examples/BushelCloud/Sources/BushelCloudKit/Configuration/CloudKitConfiguration.swift
  • Examples/BushelCloud/Sources/BushelCloudKit/Configuration/CommandConfigurations.swift
  • Examples/BushelCloud/Sources/BushelCloudKit/Configuration/ConfigKey+BUSHEL.swift
  • Examples/BushelCloud/Sources/BushelCloudKit/Configuration/ConfigurationKeys.swift
  • Examples/BushelCloud/Sources/BushelCloudKit/Configuration/ConfigurationLoader+Loading.swift
  • Examples/BushelCloud/Sources/BushelCloudKit/Configuration/ConfigurationLoader.swift
  • Examples/BushelCloud/Sources/BushelCloudKit/DataSources/AppleDB/AppleDBEntry.swift
  • Examples/BushelCloud/Sources/BushelCloudKit/DataSources/AppleDB/AppleDBFetcher.swift
  • Examples/BushelCloud/Sources/BushelCloudKit/DataSources/AppleDB/AppleDBHashes.swift
  • Examples/BushelCloud/Sources/BushelCloudKit/DataSources/AppleDB/AppleDBLink.swift
  • Examples/BushelCloud/Sources/BushelCloudKit/DataSources/AppleDB/AppleDBSource.swift
  • Examples/BushelCloud/Sources/BushelCloudKit/DataSources/AppleDB/GitHubCommit.swift
  • Examples/BushelCloud/Sources/BushelCloudKit/DataSources/AppleDB/GitHubCommitsResponse.swift
  • Examples/BushelCloud/Sources/BushelCloudKit/DataSources/AppleDB/GitHubCommitter.swift
  • Examples/BushelCloud/Sources/BushelCloudKit/DataSources/AppleDB/SignedStatus.swift
  • Examples/BushelCloud/Sources/BushelCloudKit/DataSources/DataSourcePipeline+Deduplication.swift
  • Examples/BushelCloud/Sources/BushelCloudKit/DataSources/DataSourcePipeline+Fetchers.swift
  • Examples/BushelCloud/Sources/BushelCloudKit/DataSources/DataSourcePipeline+ReferenceResolution.swift
  • Examples/BushelCloud/Sources/BushelCloudKit/DataSources/DataSourcePipeline.swift
  • Examples/BushelCloud/Sources/BushelCloudKit/DataSources/IPSWFetcher.swift
  • Examples/BushelCloud/Sources/BushelCloudKit/DataSources/MESUFetcher.swift
  • Examples/BushelCloud/Sources/BushelCloudKit/DataSources/MrMacintoshFetcher.swift
  • Examples/BushelCloud/Sources/BushelCloudKit/DataSources/SwiftVersionFetcher.swift
  • Examples/BushelCloud/Sources/BushelCloudKit/DataSources/TheAppleWiki/IPSWParser.swift
  • Examples/BushelCloud/Sources/BushelCloudKit/DataSources/TheAppleWiki/Models/IPSWVersion.swift
  • Examples/BushelCloud/Sources/BushelCloudKit/DataSources/TheAppleWiki/Models/ParseContent.swift
  • Examples/BushelCloud/Sources/BushelCloudKit/DataSources/TheAppleWiki/Models/ParseResponse.swift
  • Examples/BushelCloud/Sources/BushelCloudKit/DataSources/TheAppleWiki/Models/TextContent.swift
  • Examples/BushelCloud/Sources/BushelCloudKit/DataSources/TheAppleWiki/TheAppleWikiFetcher.swift
  • Examples/BushelCloud/Sources/BushelCloudKit/DataSources/VirtualBuddyFetcher.swift
  • Examples/BushelCloud/Sources/BushelCloudKit/DataSources/XcodeReleasesFetcher.swift
  • Examples/BushelCloud/Sources/BushelCloudKit/Utilities/ConsoleOutput.swift
  • Examples/BushelCloud/Sources/ConfigKeyKit/ConfigKey.swift
  • Examples/BushelCloud/Sources/ConfigKeyKit/ConfigurationKey.swift
  • Examples/BushelCloud/Sources/ConfigKeyKit/OptionalConfigKey.swift
  • Examples/BushelCloud/Tests/BushelCloudKitTests/CloudKit/MockCloudKitServiceTests.swift
  • Examples/BushelCloud/Tests/BushelCloudKitTests/CloudKit/PEMValidatorTests.swift
  • Examples/BushelCloud/Tests/BushelCloudKitTests/Configuration/ConfigurationLoaderTests.swift
  • Examples/BushelCloud/Tests/BushelCloudKitTests/Configuration/FetchConfigurationTests.swift
  • Examples/BushelCloud/Tests/BushelCloudKitTests/DataSources/MockAppleDBFetcherTests.swift
  • Examples/BushelCloud/Tests/BushelCloudKitTests/DataSources/MockIPSWFetcherTests.swift
  • Examples/BushelCloud/Tests/BushelCloudKitTests/DataSources/MockMESUFetcherTests.swift
  • Examples/BushelCloud/Tests/BushelCloudKitTests/DataSources/MockSwiftVersionFetcherTests.swift
  • Examples/BushelCloud/Tests/BushelCloudKitTests/DataSources/MockXcodeReleasesFetcherTests.swift
  • Examples/BushelCloud/Tests/BushelCloudKitTests/DataSources/RestoreImageDeduplicationTests.swift
  • Examples/BushelCloud/Tests/BushelCloudKitTests/DataSources/RestoreImageMergeTests.swift
  • Examples/BushelCloud/Tests/BushelCloudKitTests/DataSources/SwiftVersionDeduplicationTests.swift
  • Examples/BushelCloud/Tests/BushelCloudKitTests/DataSources/VirtualBuddyFetcherTests.swift
  • Examples/BushelCloud/Tests/BushelCloudKitTests/DataSources/XcodeVersionDeduplicationTests.swift
  • Examples/BushelCloud/Tests/BushelCloudKitTests/DataSources/XcodeVersionReferenceResolutionTests.swift
  • Examples/BushelCloud/Tests/BushelCloudKitTests/ErrorHandling/AuthenticationErrorHandlingTests.swift
  • Examples/BushelCloud/Tests/BushelCloudKitTests/ErrorHandling/CloudKitErrorHandlingTests.swift
  • Examples/BushelCloud/Tests/BushelCloudKitTests/ErrorHandling/GracefulDegradationTests.swift
  • Examples/BushelCloud/Tests/BushelCloudKitTests/ErrorHandling/NetworkErrorHandlingTests.swift
  • Examples/BushelCloud/Tests/BushelCloudKitTests/Extensions/FieldValueURLTests.swift
  • Examples/BushelCloud/Tests/BushelCloudKitTests/Mocks/MockAppleDBFetcher.swift
  • Examples/BushelCloud/Tests/BushelCloudKitTests/Mocks/MockCloudKitService.swift
  • Examples/BushelCloud/Tests/BushelCloudKitTests/Mocks/MockFetcherError.swift
  • Examples/BushelCloud/Tests/BushelCloudKitTests/Mocks/MockIPSWFetcher.swift
  • Examples/BushelCloud/Tests/BushelCloudKitTests/Mocks/MockMESUFetcher.swift
  • Examples/BushelCloud/Tests/BushelCloudKitTests/Mocks/MockSwiftVersionFetcher.swift
  • Examples/BushelCloud/Tests/BushelCloudKitTests/Mocks/MockURLProtocol.swift
  • Examples/BushelCloud/Tests/BushelCloudKitTests/Mocks/MockXcodeReleasesFetcher.swift
  • Examples/BushelCloud/Tests/BushelCloudKitTests/Models/DataSourceMetadataTests.swift
  • Examples/BushelCloud/Tests/BushelCloudKitTests/Models/RestoreImageRecordTests.swift
  • Examples/BushelCloud/Tests/BushelCloudKitTests/Models/SwiftVersionRecordTests.swift
  • Examples/BushelCloud/Tests/BushelCloudKitTests/Models/XcodeVersionRecordTests.swift
  • Examples/BushelCloud/Tests/BushelCloudKitTests/Utilities/FieldValue+Assertions.swift
  • Examples/BushelCloud/Tests/BushelCloudKitTests/Utilities/MockRecordInfo.swift
  • Examples/BushelCloud/Tests/ConfigKeyKitTests/ConfigKeySourceTests.swift
  • Examples/BushelCloud/Tests/ConfigKeyKitTests/ConfigKeyTests.swift
  • Examples/BushelCloud/Tests/ConfigKeyKitTests/NamingStyleTests.swift
  • Examples/BushelCloud/Tests/ConfigKeyKitTests/OptionalConfigKeyTests.swift
  • Examples/CelestraCloud/.github/workflows/CelestraCloud.yml
  • Examples/CelestraCloud/.github/workflows/codeql.yml
  • Examples/CelestraCloud/.github/workflows/update-feeds.yml
  • Examples/CelestraCloud/.gitrepo
  • Examples/CelestraCloud/CHANGELOG.md
  • Examples/CelestraCloud/Scripts/lint.sh
  • Examples/CelestraCloud/Sources/CelestraCloud/Celestra.swift
  • Examples/CelestraCloud/Sources/CelestraCloud/Commands/AddFeedCommand.swift
  • Examples/CelestraCloud/Sources/CelestraCloud/Commands/ClearCommand.swift
  • Examples/CelestraCloud/Sources/CelestraCloud/Commands/UpdateCommand+Reporting.swift
  • Examples/CelestraCloud/Sources/CelestraCloud/Commands/UpdateCommand.swift
  • Examples/CelestraCloud/Sources/CelestraCloud/Commands/UpdateCommandError.swift
  • Examples/CelestraCloud/Sources/CelestraCloud/Commands/UpdateSummary.swift
  • Examples/CelestraCloud/Sources/CelestraCloud/Services/FeedUpdateProcessor+Fetch.swift
  • Examples/CelestraCloud/Sources/CelestraCloud/Services/FeedUpdateProcessor.swift
  • Examples/CelestraCloud/Sources/CelestraCloud/Services/FeedUpdateResult.swift
  • Examples/CelestraCloud/Sources/CelestraCloudKit/CelestraConfig.swift
  • Examples/CelestraCloud/Sources/CelestraCloudKit/Configuration/ConfigurationLoader.swift
  • Examples/CelestraCloud/Sources/CelestraCloudKit/Protocols/CloudKitRecordOperating.swift
  • Examples/CelestraCloud/Sources/CelestraCloudKit/Services/ArticleCategorizer.swift
  • Examples/CelestraCloud/Sources/CelestraCloudKit/Services/ArticleCloudKitService.swift
  • Examples/CelestraCloud/Sources/CelestraCloudKit/Services/ArticleOperationBuilder.swift
  • Examples/CelestraCloud/Sources/CelestraCloudKit/Services/ArticleSyncService.swift
  • Examples/CelestraCloud/Sources/CelestraCloudKit/Services/CelestraError.swift
  • Examples/CelestraCloud/Sources/CelestraCloudKit/Services/CloudKitService+Celestra.swift
  • Examples/CelestraCloud/Sources/CelestraCloudKit/Services/FeedCloudKitService.swift
  • Examples/CelestraCloud/Tests/CelestraCloudTests/Configuration/CloudKitConfigurationTests.swift
  • Examples/CelestraCloud/Tests/CelestraCloudTests/Configuration/UpdateCommandConfigurationTests.swift
  • Examples/CelestraCloud/Tests/CelestraCloudTests/Errors/CelestraErrorTests+Description.swift
  • Examples/CelestraCloud/Tests/CelestraCloudTests/Errors/CelestraErrorTests+RecoverySuggestion.swift
  • Examples/CelestraCloud/Tests/CelestraCloudTests/Errors/CelestraErrorTests.swift
  • Examples/CelestraCloud/Tests/CelestraCloudTests/Errors/CloudKitConversionErrorTests.swift
  • Examples/CelestraCloud/Tests/CelestraCloudTests/Extensions/ArticleConversion+FromCloudKit.swift
  • Examples/CelestraCloud/Tests/CelestraCloudTests/Extensions/ArticleConversion+ToCloudKit.swift
  • Examples/CelestraCloud/Tests/CelestraCloudTests/Extensions/ArticleConversion.swift
  • Examples/CelestraCloud/Tests/CelestraCloudTests/Extensions/FeedConversion+FromCloudKit.swift
  • Examples/CelestraCloud/Tests/CelestraCloudTests/Extensions/FeedConversion+RoundTrip.swift
  • Examples/CelestraCloud/Tests/CelestraCloudTests/Extensions/FeedConversion+ToCloudKit.swift
  • Examples/CelestraCloud/Tests/CelestraCloudTests/Extensions/FeedConversion.swift
  • Examples/CelestraCloud/Tests/CelestraCloudTests/Mocks/MockCloudKitRecordOperator.swift
  • Examples/CelestraCloud/Tests/CelestraCloudTests/Models/BatchOperationResultTests.swift
  • Examples/CelestraCloud/Tests/CelestraCloudTests/Services/ArticleCategorizer+Advanced.swift
  • Examples/CelestraCloud/Tests/CelestraCloudTests/Services/ArticleCategorizer+Basic.swift
  • Examples/CelestraCloud/Tests/CelestraCloudTests/Services/ArticleCategorizer.swift
  • Examples/CelestraCloud/Tests/CelestraCloudTests/Services/ArticleCloudKitService+Mutations.swift
  • Examples/CelestraCloud/Tests/CelestraCloudTests/Services/ArticleCloudKitService+Query.swift
  • Examples/CelestraCloud/Tests/CelestraCloudTests/Services/ArticleCloudKitService.swift
  • Examples/CelestraCloud/Tests/CelestraCloudTests/Services/FeedCloudKitService+CRUD.swift
  • Examples/CelestraCloud/Tests/CelestraCloudTests/Services/FeedCloudKitService+Query.swift
  • Examples/CelestraCloud/Tests/CelestraCloudTests/Services/FeedCloudKitService.swift
  • Examples/CelestraCloud/Tests/CelestraCloudTests/Services/FeedMetadataBuilder+Error.swift
  • Examples/CelestraCloud/Tests/CelestraCloudTests/Services/FeedMetadataBuilder+NotModified.swift
  • Examples/CelestraCloud/Tests/CelestraCloudTests/Services/FeedMetadataBuilder+Success.swift
  • Examples/CelestraCloud/Tests/CelestraCloudTests/Services/FeedMetadataBuilder.swift
  • Examples/MistDemo/.swiftlint.yml
  • Examples/MistDemo/App/MistDemoApp.swift
  • Examples/MistDemo/MistDemoApp.entitlements
  • Examples/MistDemo/Package.resolved
  • Examples/MistDemo/Package.swift
  • Examples/MistDemo/Sources/MistDemo/MistDemo.swift
  • Examples/MistDemo/Sources/MistDemoApp/Models/CKRecord+TypedField.swift
  • Examples/MistDemo/Sources/MistDemoApp/Models/Note.swift
  • Examples/MistDemo/Sources/MistDemoApp/Models/RecordZoneChangesSnapshot.swift
  • Examples/MistDemo/Sources/MistDemoApp/Models/ResolveResult.swift
  • Examples/MistDemo/Sources/MistDemoApp/Models/ZoneRow.swift
  • Examples/MistDemo/Sources/MistDemoApp/Services/CKDatabase+WebAuthToken.swift
  • Examples/MistDemo/Sources/MistDemoApp/Services/CKDatabase.Scope+Demo.swift
  • Examples/MistDemo/Sources/MistDemoApp/Services/CloudKitStore+Assets.swift
  • Examples/MistDemo/Sources/MistDemoApp/Services/CloudKitStore+PushTokens.swift
  • Examples/MistDemo/Sources/MistDemoApp/Services/CloudKitStore+Records.swift
  • Examples/MistDemo/Sources/MistDemoApp/Services/CloudKitStore+Subscriptions.swift
  • Examples/MistDemo/Sources/MistDemoApp/Services/CloudKitStore+Users.swift
  • Examples/MistDemo/Sources/MistDemoApp/Services/CloudKitStore+Zones.swift
  • Examples/MistDemo/Sources/MistDemoApp/Services/CloudKitStore.swift
  • Examples/MistDemo/Sources/MistDemoApp/Services/CloudKitStoreError.swift
  • Examples/MistDemo/Sources/MistDemoApp/Services/PlatformAliases.swift
  • Examples/MistDemo/Sources/MistDemoApp/Services/PlatformApplicationDelegate+NSApplicationDelegate.swift
  • Examples/MistDemo/Sources/MistDemoApp/Services/PlatformApplicationDelegate+RemoteNotifications.swift
  • Examples/MistDemo/Sources/MistDemoApp/Services/PlatformApplicationDelegate+UIApplicationDelegate.swift
  • Examples/MistDemo/Sources/MistDemoApp/Services/PlatformApplicationDelegate+WKApplicationDelegate.swift
  • Examples/MistDemo/Sources/MistDemoApp/Services/PlatformApplicationDelegate.swift
  • Examples/MistDemo/Sources/MistDemoApp/Services/PushNotificationDelegate.swift
  • Examples/MistDemo/Sources/MistDemoApp/Services/PushTokenReceiver.swift
  • Examples/MistDemo/Sources/MistDemoApp/Services/RemoteNotificationRegistering.swift
  • Examples/MistDemo/Sources/MistDemoApp/Views/AccountView+Actions.swift
  • Examples/MistDemo/Sources/MistDemoApp/Views/AccountView.swift
  • Examples/MistDemo/Sources/MistDemoApp/Views/AssetsView.swift
  • Examples/MistDemo/Sources/MistDemoApp/Views/CompositionDisclosure.swift
  • Examples/MistDemo/Sources/MistDemoApp/Views/DetailColumnRoot.swift
  • Examples/MistDemo/Sources/MistDemoApp/Views/NoteEditView.swift
  • Examples/MistDemo/Sources/MistDemoApp/Views/PushTokensView.swift
  • Examples/MistDemo/Sources/MistDemoApp/Views/QueryView.swift
  • Examples/MistDemo/Sources/MistDemoApp/Views/RecordDetailView.swift
  • Examples/MistDemo/Sources/MistDemoApp/Views/RecordsView.swift
  • Examples/MistDemo/Sources/MistDemoApp/Views/SidebarItem.swift
  • Examples/MistDemo/Sources/MistDemoApp/Views/SidebarView.swift
  • Examples/MistDemo/Sources/MistDemoApp/Views/SubscriptionsView.swift
  • Examples/MistDemo/Sources/MistDemoApp/Views/UsersView.swift
  • Examples/MistDemo/Sources/MistDemoApp/Views/View+DeleteSwipeAction.swift
  • Examples/MistDemo/Sources/MistDemoApp/Views/ZoneListView.swift
  • Examples/MistDemo/Sources/MistDemoKit/Commands/AtomicBool.swift
  • Examples/MistDemo/Sources/MistDemoKit/Commands/CreateCommand.swift
  • Examples/MistDemo/Sources/MistDemoKit/Commands/CreateTokenCommand.swift
  • Examples/MistDemo/Sources/MistDemoKit/Commands/CreateZoneCommand.swift
  • Examples/MistDemo/Sources/MistDemoKit/Commands/CurrentUserCommand.swift
  • Examples/MistDemo/Sources/MistDemoKit/Commands/DeleteCommand.swift
  • Examples/MistDemo/Sources/MistDemoKit/Commands/DeleteResult.swift
  • Examples/MistDemo/Sources/MistDemoKit/Commands/DeleteZoneCommand.swift
  • Examples/MistDemo/Sources/MistDemoKit/Commands/DemoErrorsCommand.swift
  • Examples/MistDemo/Sources/MistDemoKit/Commands/DemoErrorsRunner+Output.swift
  • Examples/MistDemo/Sources/MistDemoKit/Commands/DemoErrorsRunner.swift
  • Examples/MistDemo/Sources/MistDemoKit/Commands/DemoInFilterCommand.swift
  • Examples/MistDemo/Sources/MistDemoKit/Commands/DiscoverAllUserIdentitiesCommand.swift
  • Examples/MistDemo/Sources/MistDemoKit/Commands/DiscoverCommand.swift
  • Examples/MistDemo/Sources/MistDemoKit/Commands/FetchChangesCommand.swift
  • Examples/MistDemo/Sources/MistDemoKit/Commands/ListSubscriptionsCommand.swift
  • Examples/MistDemo/Sources/MistDemoKit/Commands/ListZonesCommand.swift
  • Examples/MistDemo/Sources/MistDemoKit/Commands/LookupAllRecordsCommand.swift
  • Examples/MistDemo/Sources/MistDemoKit/Commands/LookupCommand.swift
  • Examples/MistDemo/Sources/MistDemoKit/Commands/LookupSubscriptionCommand.swift
  • Examples/MistDemo/Sources/MistDemoKit/Commands/LookupZonesCommand.swift
  • Examples/MistDemo/Sources/MistDemoKit/Commands/MistDemoCommand.swift
  • Examples/MistDemo/Sources/MistDemoKit/Commands/MistDemoLoggingBootstrap.swift
  • Examples/MistDemo/Sources/MistDemoKit/Commands/ModifyCommand.swift
  • Examples/MistDemo/Sources/MistDemoKit/Commands/ModifyOutput.swift
  • Examples/MistDemo/Sources/MistDemoKit/Commands/ModifyResultRow.swift
  • Examples/MistDemo/Sources/MistDemoKit/Commands/ModifySubscriptionsCommand.swift
  • Examples/MistDemo/Sources/MistDemoKit/Commands/ModifyZonesCommand.swift
  • Examples/MistDemo/Sources/MistDemoKit/Commands/ProbeDuplicateSubscriptionCommand+Experiment.swift
  • Examples/MistDemo/Sources/MistDemoKit/Commands/ProbeDuplicateSubscriptionCommand.swift
  • Examples/MistDemo/Sources/MistDemoKit/Commands/ProbeExperiment.swift
  • Examples/MistDemo/Sources/MistDemoKit/Commands/ProbeSubscriptionTemplate.swift
  • Examples/MistDemo/Sources/MistDemoKit/Commands/QueryCommand+FilterParsing.swift
  • Examples/MistDemo/Sources/MistDemoKit/Commands/QueryCommand.swift
  • Examples/MistDemo/Sources/MistDemoKit/Commands/RegisterTokenCommand.swift
  • Examples/MistDemo/Sources/MistDemoKit/Commands/RereferenceAssetCommand.swift
  • Examples/MistDemo/Sources/MistDemoKit/Commands/RereferenceAssetError.swift
  • Examples/MistDemo/Sources/MistDemoKit/Commands/ResolveCommand.swift
  • Examples/MistDemo/Sources/MistDemoKit/Commands/TestPrivateCommand.swift
  • Examples/MistDemo/Sources/MistDemoKit/Commands/TestPublicCommand.swift
  • Examples/MistDemo/Sources/MistDemoKit/Commands/UpdateCommand.swift
  • Examples/MistDemo/Sources/MistDemoKit/Commands/UploadAssetCommand.swift
  • Examples/MistDemo/Sources/MistDemoKit/Commands/ValidateCommand.swift
  • Examples/MistDemo/Sources/MistDemoKit/Configuration/AuthTokenConfig.swift
  • Examples/MistDemo/Sources/MistDemoKit/Configuration/BrowserFlagResolver.swift
  • Examples/MistDemo/Sources/MistDemoKit/Configuration/ConfigurationError.swift
  • Examples/MistDemo/Sources/MistDemoKit/Configuration/CreateConfig.swift
  • Examples/MistDemo/Sources/MistDemoKit/Configuration/CreateTokenConfig.swift
  • Examples/MistDemo/Sources/MistDemoKit/Configuration/CreateZoneConfig.swift
  • Examples/MistDemo/Sources/MistDemoKit/Configuration/CurrentUserConfig.swift
  • Examples/MistDemo/Sources/MistDemoKit/Configuration/DeleteConfig.swift
  • Examples/MistDemo/Sources/MistDemoKit/Configuration/DeleteZoneConfig.swift
  • Examples/MistDemo/Sources/MistDemoKit/Configuration/DemoErrorsConfig.swift
  • Examples/MistDemo/Sources/MistDemoKit/Configuration/DemoErrorsError.swift
  • Examples/MistDemo/Sources/MistDemoKit/Configuration/DiscoverConfig.swift
  • Examples/MistDemo/Sources/MistDemoKit/Configuration/Field.swift
  • Examples/MistDemo/Sources/MistDemoKit/Configuration/FieldType.swift
  • Examples/MistDemo/Sources/MistDemoKit/Configuration/ListSubscriptionsConfig.swift
  • Examples/MistDemo/Sources/MistDemoKit/Configuration/ListZonesConfig.swift
  • Examples/MistDemo/Sources/MistDemoKit/Configuration/LookupConfig.swift
  • Examples/MistDemo/Sources/MistDemoKit/Configuration/LookupSubscriptionConfig.swift
  • Examples/MistDemo/Sources/MistDemoKit/Configuration/LookupZonesConfig.swift
  • Examples/MistDemo/Sources/MistDemoKit/Configuration/MistDemoConfig+DatabaseConfiguration.swift
  • Examples/MistDemo/Sources/MistDemoKit/Configuration/MistDemoConfig+Parsing.swift
  • Examples/MistDemo/Sources/MistDemoKit/Configuration/MistDemoConfig.swift
  • Examples/MistDemo/Sources/MistDemoKit/Configuration/MistDemoConfiguration.swift
  • Examples/MistDemo/Sources/MistDemoKit/Configuration/ModifySubscriptionsConfig.swift
  • Examples/MistDemo/Sources/MistDemoKit/Configuration/ModifyZonesConfig.swift
  • Examples/MistDemo/Sources/MistDemoKit/Configuration/ProbeDuplicateSubscriptionConfig.swift
  • Examples/MistDemo/Sources/MistDemoKit/Configuration/QueryConfig+Parsing.swift
  • Examples/MistDemo/Sources/MistDemoKit/Configuration/QueryConfig.swift
  • Examples/MistDemo/Sources/MistDemoKit/Configuration/RegisterTokenConfig.swift
  • Examples/MistDemo/Sources/MistDemoKit/Configuration/RereferenceAssetConfig.swift
  • Examples/MistDemo/Sources/MistDemoKit/Configuration/ResolveConfig.swift
  • Examples/MistDemo/Sources/MistDemoKit/Configuration/TestPrivateConfig.swift
  • Examples/MistDemo/Sources/MistDemoKit/Configuration/UpdateConfig.swift
  • Examples/MistDemo/Sources/MistDemoKit/Configuration/UploadAssetConfig.swift
  • Examples/MistDemo/Sources/MistDemoKit/Configuration/ValidateConfig.swift
  • Examples/MistDemo/Sources/MistDemoKit/Configuration/WebConfig.swift
  • Examples/MistDemo/Sources/MistDemoKit/Configuration/ZoneOperationInput.swift
  • Examples/MistDemo/Sources/MistDemoKit/Configuration/ZoneOperationsEnvelope.swift
  • Examples/MistDemo/Sources/MistDemoKit/Constants/MistDemoConstants+Defaults.swift
  • Examples/MistDemo/Sources/MistDemoKit/Constants/MistDemoConstants+Messages.swift
  • Examples/MistDemo/Sources/MistDemoKit/Constants/MistDemoConstants.swift
  • Examples/MistDemo/Sources/MistDemoKit/Errors/DiscoverError.swift
  • Examples/MistDemo/Sources/MistDemoKit/Errors/ErrorOutput+Convenience.swift
  • Examples/MistDemo/Sources/MistDemoKit/Errors/ErrorOutput.swift
  • Examples/MistDemo/Sources/MistDemoKit/Errors/FieldConversionError.swift
  • Examples/MistDemo/Sources/MistDemoKit/Errors/ListZonesError.swift
  • Examples/MistDemo/Sources/MistDemoKit/Errors/MistDemoError.swift
  • Examples/MistDemo/Sources/MistDemoKit/Errors/ModifyZonesError.swift
  • Examples/MistDemo/Sources/MistDemoKit/Errors/SubscriptionCommandError.swift
  • Examples/MistDemo/Sources/MistDemoKit/Errors/TokenCommandError.swift
  • Examples/MistDemo/Sources/MistDemoKit/Errors/ValidateError.swift
  • Examples/MistDemo/Sources/MistDemoKit/Extensions/Array+Field.swift
  • Examples/MistDemo/Sources/MistDemoKit/Extensions/Command+AnyCommand.swift

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
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch v1.0.0-beta.2

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 and usage tips.

@claude
Copy link
Copy Markdown

claude Bot commented May 26, 2026

Code Review — v1.0.0-beta.2

This is a large, well-structured release that adds subscriptions, APNs token management, web-courier long-polling, asset re-referencing, a richer error model, and a generic phantom-typed batch-result system. The overall quality is high. Below are findings organized by area.


Overview of Changes

  • Subscriptions: Full CRUD API (listSubscriptions, lookupSubscriptions, modifySubscriptions, createSubscription, deleteSubscription) with domain model (SubscriptionInfo, SubscriptionInfo.Kind, SubscriptionFireEvents, NotificationInfo)
  • APNs Token Operations: createAPNsToken + registerAPNsToken, with the important /device/ path correction replacing the incorrect /database/ path that was in the spec
  • Web Courier: Courier, CourierNotification, WebCourierPoller, and URLSession+CourierPoll — headless long-poll for subscription-triggered notifications
  • Asset Re-referencing: rereferenceAssets / rereferenceAsset composing assets/rereference + records/modify
  • Error model expansion: quotaExceeded, badRequest, atomicFailure, incompleteResponse, conversionFailed, recordOperationFailed, subscriptionOperationFailed, subscriptionLikelyDuplicate, zonePaginationLimitExceeded with an extracted CloudKitError+ErrorDescription.swift
  • Generic operation result: OperationResult<Success, Target> + OperationFailure<Target> with phantom OperationFailureTarget type — prevents mixing record and subscription failures at compile time
  • modifyRecords return type change: [RecordInfo][RecordResult] (breaking change from beta.1)
  • OpenAPI spec: /database//device/ for token endpoints, new rereferenceAssets endpoint, Query schema refactored into a $ref
  • LoggingMiddleware: Body logging capped at 64 KB, skips non-JSON content types

What's Working Well

  1. Phantom type safety on batch resultsOperationFailure<RecordTarget> vs OperationFailure<SubscriptionTarget> prevents mixing failure kinds in batch arrays at compile time. Elegant design.

  2. Wire/domain asymmetry for Kind.database — The subscription's .database case serializing as subscriptionType: "zone" + zoneWide: true is correctly documented and handled in both Codable and OpenAPI schema paths.

  3. Transport separation for Courier — Follows the same connection-pool isolation pattern as asset uploads, with the reasoning clearly documented.

  4. subscriptionLikelyDuplicate naming — The "likely" qualifier and the warning in the docs/error description are appropriately cautious. Preserving the raw SubscriptionOperationFailure for caller inspection is good.

  5. /device/ path correction — The OpenAPI spec now correctly models tokens/create and tokens/register under /device/{version}/{container}/{environment}/ with the discovery rationale documented inline. This is an important correctness fix.

  6. LoggingMiddleware improvements — Capping at 64 KB and skipping binary bodies are sensible memory-safety measures.

  7. precondition for empty firesOn — Appropriate for a programmer-error guard (fatal in both debug and release per CloudKit's semantics that a zero-event subscription is nonsensical).


Issues and Suggestions

Bug: CourierNotification.init(data:) allocates a new JSONDecoder on every call

// Sources/MistKit/Models/Notifications/CourierNotification.swift:76
let wire = try JSONDecoder().decode(Wire.self, from: data)

Every long-poll iteration allocates a fresh JSONDecoder. The project already has JSONDecoder.shared (marked thread-safe in its own header). Under a sustained long-poll loop this creates unnecessary allocator pressure.

Suggestion: Replace with:

let wire = try JSONDecoder.shared.decode(Wire.self, from: data)

Breaking change: modifyRecords return type

modifyRecords(_:atomic:database:) changed from [RecordInfo] to [RecordResult]. This is the right direction (per-record failures used to be silently dropped), but it is a source-breaking change from beta.1. It should be called out prominently in the release notes / changelog, since any consumer calling modifyRecords will need to update to switch on .success/.failure.


Courier.notifications()Task.sleep cancellation propagates as a thrown error

When a consumer cancels the stream, onTermination cancels the inner Task. If the inner task is sleeping (keepalive path), Task.sleep throws CancellationError, which the catch error block passes to continuation.finish(throwing:). Since the continuation is already terminated, this is a no-op in practice — but conceptually the stream "ends with an error" from the inner task's perspective rather than cleanly.

More concretely: if a consumer exits the for await loop early (via break) while notifications() is in the 250 ms sleep, the stream terminates cleanly for the consumer, but the inner task finishes via finish(throwing: CancellationError()). This is harmless today but could cause confusion if the inner task logic ever needs to distinguish clean stop from error.

Suggestion: Catch CancellationError separately in the inner task:

} catch is CancellationError {
    continuation.finish()       // clean stop
} catch {
    continuation.finish(throwing: error)
}

isLikelyDuplicate string literal dependency

// Sources/MistKit/Models/SubscriptionTarget.swift
internal static let duplicateMarker = "could not find subscription we just created"

The detection depends on an exact server-returned reason string. The code comment acknowledges this and marks it as a hint. A few suggestions:

  • Add a test that verifies the detection logic with the exact string so it fails loudly if the string constant drifts.
  • Consider logging the raw reason alongside subscriptionLikelyDuplicate at .warning level so operators can detect if Apple changes the wording.

SubscriptionInfo not Equatable

SubscriptionInfo is Sendable and Codable but not Equatable. Given its use in test assertions (#expect(recovered.subscriptionID == "sub-1") etc., checking individual fields), this is workable, but an Equatable conformance would simplify round-trip test assertions and let callers compare subscription objects directly. SubscriptionInfo.Kind would need it too, which requires Query, ZoneID etc. to be Equatable — these already appear to have enough value-type structure.


deleteSubscription silently succeeds if CloudKit returns per-subscription errors on a delete

// CloudKitService+ModifySubscriptions.swift
for result in results {
    _ = try result.get()
}

This correctly surfaces per-subscription failures. However, it also calls result.get() on any success entries. If CloudKit returns the subscription back in a delete response (non-standard but defensive to handle), result.get() succeeds and the value is discarded (the discard is explicit via _ = try). This is fine, just noting it's deliberately discarding.


CourierTests missing coverage for notifications() stream

CourierTests.swift covers pollOnce (3 cases) but not notifications(). The streaming function has meaningful logic (keepalive backoff, error propagation, cancellation). A minimal test verifying that notifications() yields one notification then terminates when cancelled would improve confidence.


Minor / Nits

  • clientId UUID fallback: The resolveClientId docs say "fresh UUID per call with no continuity". This is correct but worth ensuring the docs mention that server-to-server callers who want push de-dup should persist and reuse the same clientId across createAPNsToken / registerAPNsToken calls.

  • QuotaHint.description integer division: maxBytes / 1_024 / 1_024 truncates. For maxRecordDataBytes = 1_048_576, this renders correctly as 1 MB. If either constant ever changes to a non-power-of-two value, the display could be confusing. Not a bug, just fragile.

  • import visibility corrections throughout (e.g., import OpenAPIRuntimeinternal import OpenAPIRuntime) — good hygiene, reduces symbol leakage.

  • swiftlint:disable line_length in `CloudKitService+ModifySubscriptions.swift**: The disable is narrow (method-only) and explained by the DocC link. Acceptable.


Test Coverage Assessment

The PR ships a large number of new tests (209 test files changed). Coverage for authentication, token management, credential combinations, concurrency, and subscription/token conversion is thorough. The SubscriptionConversionTests, SubscriptionInfoCodableTests, and APNsTokenResultTests look solid.

The notable gap is notifications() stream behavior (see above). Service-level integration tests for subscriptions are not included (expected since those require a live container).


Summary

This is a well-engineered beta release. The main actionable items before shipping:

  1. Fix JSONDecoder() allocation in CourierNotification.init(data:) — simple one-liner swap.
  2. Document the modifyRecords return-type breaking change in the changelog.
  3. Add a Courier.notifications() test for the streaming/cancellation path.
  4. Consider CancellationError handling in notifications() to clean up the theoretical ambiguity.

Everything else is minor.

@codecov
Copy link
Copy Markdown

codecov Bot commented May 26, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 71.40%. Comparing base (d11c6c5) to head (b0b361a).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #395      +/-   ##
==========================================
+ Coverage   68.95%   71.40%   +2.45%     
==========================================
  Files         111      168      +57     
  Lines        2641     3844    +1203     
==========================================
+ Hits         1821     2745     +924     
- Misses        820     1099     +279     
Flag Coverage Δ
mistdemo-spm-macos 11.42% <ø> (-0.40%) ⬇️
mistdemo-swift-6.2-jammy 11.42% <ø> (-0.43%) ⬇️
mistdemo-swift-6.2-noble 11.42% <ø> (-0.40%) ⬇️
mistdemo-swift-6.3-jammy 11.42% <ø> (-0.40%) ⬇️
mistdemo-swift-6.3-noble 11.42% <ø> (-0.40%) ⬇️
spm 71.57% <ø> (+4.40%) ⬆️
swift-6.1-jammy 71.59% <ø> (+4.42%) ⬆️
swift-6.1-noble 71.62% <ø> (+4.37%) ⬆️
swift-6.2-jammy 71.62% <ø> (+4.34%) ⬆️
swift-6.2-noble 71.59% <ø> (+4.12%) ⬆️
swift-6.3-jammy 71.62% <ø> (+4.15%) ⬆️
swift-6.3-noble 71.62% <ø> (+4.15%) ⬆️

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

☔ View full report in Codecov by Sentry.
📢 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.

@claude
Copy link
Copy Markdown

claude Bot commented May 26, 2026

Code Review — v1.0.0-beta.2

Reviewed at high effort (3 finder angles × verify). Six findings survived verification, ranked by severity.


🔴 Bug 1 — CDN asset upload uses POST instead of PUT (breaks all production uploads)

File: Sources/MistKit/Models/AssetUploading/URLRequest+AssetUpload.swift:44

self.httpMethod = "POST"   // ← wrong

The project's own DocC files state the CDN step requires PUT:

  • AbstractionLayerArchitecture.md: "ask CloudKit for a CDN URL, then PUT the bytes to the CDN"
  • CloudKitLimitsAndPerformance.md: "uploadAssetData PUTs the bytes there"

Failure scenario: Every call to uploadAssets() against a real CloudKit container will fail at the CDN step with 405 Method Not Allowed. The test suite uses injected mock closures and never exercises the actual URLRequest construction, so this is undetected by tests.

Fix:

self.httpMethod = "PUT"

🔴 Bug 2 — fetchAllRecordChanges silently discards a malformed first-page response

File: Sources/MistKit/CloudKitService/CloudKitService+SyncOperations.swift:190

if result.records.isEmpty && result.moreComing && result.syncToken == currentToken {
    break   // fires when currentToken == nil and result.syncToken == nil
}
// line 194 — invalidResponse throw is unreachable in this path
if result.moreComing && result.syncToken == nil {
    throw CloudKitError.invalidResponse
}

On the first call (syncToken: nil), currentToken is nil. If the server returns moreComing: true, syncToken: nil, and an empty records array, the condition nil == nil evaluates to true and the loop breaks silently, returning ([], nil) with no error. The invalidResponse guard that should fire is unreachable because break exits the loop first.

Failure scenario: Initial sync where CloudKit returns a malformed stuck response on the first page. The caller silently believes the sync completed with no records.

Fix: Add && currentToken != nil to the stuck-token guard, or reorder so invalidResponse is checked before the stuck-token break:

// Check validity first
if result.moreComing && result.syncToken == nil {
    throw CloudKitError.invalidResponse
}
// Now the stuck-token check — currentToken must be non-nil to compare meaningfully
if let current = currentToken, result.records.isEmpty && result.moreComing && result.syncToken == current {
    break
}

🔴 Bug 3 — Same nil==nil false positive in fetchAllZoneChanges

File: Sources/MistKit/CloudKitService/CloudKitService+FetchAllZoneChanges.swift:98

Identical structure to Bug 2 — the zone-changes pagination loop has the same guard ordering:

if result.zones.isEmpty && result.moreComing && result.syncToken == currentToken {
    break   // nil == nil on first call
}
if result.moreComing && result.syncToken == nil {
    throw CloudKitError.invalidResponse   // unreachable for the empty-zones case
}

Apply the same fix as Bug 2.


🟠 Bug 4 — modifyRecords wraps ConversionError as .underlyingError instead of .conversionFailed

File: Sources/MistKit/CloudKitService/CloudKitService+WriteOperations.swift:117

All other operations (queryRecords, lookupRecords, fetchRecordChanges, etc.) end their catch clause with throw mapToCloudKitError(error, context: "..."), which dispatches CloudKitErrorConvertible errors (including ConversionError) to the correctly-typed .conversionFailed. modifyRecords uses a hand-rolled two-branch catch that skips mapToCloudKitError:

} catch let cloudKitError as CloudKitError {
    throw cloudKitError.addingQuotaHint(...)
} catch {
    throw CloudKitError.underlyingError(error)  // ← ConversionError lands here
}

Failure scenario: modifyRecords is called and the server returns a record missing its recordName. RecordResult(from:) throws ConversionError.recordMissingRecordName. The caller receives CloudKitError.underlyingError(conversionError) instead of CloudKitError.conversionFailed(conversionError), breaking any call-site pattern matching on .conversionFailed.

Fix: Replace both catch clauses in modifyRecords with the standard pattern used everywhere else:

} catch {
    throw mapToCloudKitError(error, context: "modifyRecords").addingQuotaHint(...)
}

🟠 Bug 5 — ServerToServerAuthManager.hasCredentials accepts short key IDs that always fail at request time

File: Sources/MistKit/Authentication/ServerToServerAuthManager.swift:45

public var hasCredentials: Bool {
    get async { !keyID.isEmpty }   // passes for keyID.count 1–7
}

ServerToServerAuthenticator.init throws .keyIdTooShort for any keyID.count < 8 (line 94–99 of ServerToServerAuthenticator.swift). A key ID of 1–7 characters passes hasCredentials but always throws at request time.

Failure scenario: S2S credentials configured with a short key ID. The credential fallback logic in Credentials+TokenManager calls await hasCredentials, gets true, selects the S2S manager, and every subsequent request throws TokenManagerError.invalidCredentials(.keyIdTooShort) rather than gracefully falling back to the other configured credential type.

Fix:

public var hasCredentials: Bool {
    get async { keyID.count >= 8 }
}

🟡 Bug 6 — PEM error classification uses localizedDescription string matching (fragile on Linux)

File: Sources/MistKit/Authentication/ServerToServerAuthManager.swift:97 (also ServerToServerAuthenticator.swift:130)

if error.localizedDescription.contains("PEM") || error.localizedDescription.contains("format") {
    throw TokenManagerError.invalidCredentials(.invalidPEMFormat(error))
}
throw TokenManagerError.invalidCredentials(.privateKeyParseFailed(error))

MistKit explicitly targets Linux (per CLAUDE.md and Package.swift). On Linux, swift-crypto's CryptoKitError messages are English constants that currently contain these strings, but this is an implementation detail of the library, not a contractual API. A swift-crypto update or a different error type could silently misclassify a PEM-format error as .privateKeyParseFailed, causing callers that handle .invalidPEMFormat to miss it.

Fix: Check the error type directly where possible (e.g. via is CryptoKitError), or use a dedicated validation step before key parsing that catches format issues structurally rather than via message text.


Review generated with Claude Sonnet 4.6 — 3 finder angles × verify at high effort.

claude and others added 2 commits May 26, 2026 17:26
Handle the incompleteResponse, subscriptionOperationFailed, and
subscriptionLikelyDuplicate cases added to CloudKitError so the
CelestraCloud example builds. All three are classified as non-retriable.

https://claude.ai/code/session_01WY9rXuEkERRMMnLDz2872H
* Wire landed MistKit endpoints into MistDemo web app (#394)

Replace the nine 501 "pending" stubs in MistKit-server mode with real
Hummingbird handlers that forward to the already-shipped CloudKitService
wrappers, restoring parity with CloudKit JS mode:

  records/lookup, records/changes, zones/list, zones/lookup, zones/changes,
  users/caller, users/discover, users/lookup/email, users/lookup/id

- WebBackend gains nine webXxx methods; CloudKitService conforms via thin
  forwards split across +Reads (records/zones) and +Users extensions.
- New request DTOs (WebRequests+Records/+Users, extended +Zones) and response
  DTOs (RecordChanges, ZoneChanges, Caller, Users). User routes carry no
  database selector — they run on the public DB with web-auth.
- records/lookup surfaces per-record failures via try get(), matching the
  webModifySubscriptions precedent.
- addUnwiredLandedEndpoints removed; WebServer+Pending now lists only
  records/resolve (#41, no wrapper yet).
- The browser already branched to these /api/* routes in MistKit mode, so no
  front-end change was needed — wiring the server flips them off the stub.

Tests: MockBackend extended with the nine methods + call captures; forwarding
and 401-unauthorized tests per route. 963 MistDemo tests pass; swift-format,
swiftlint, and header checks clean.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* Address PR #396 review: discover-only user lookup + honest lookup semantics

Demo web app now exposes only the non-deprecated POST /users/discover for
user-identity lookup; the Apple-deprecated /users/lookup/email and
/users/lookup/id routes are removed from every layer (route, protocol,
conformance, DTO, mock, tests, frontend). Discover accepts emails AND user
record names, forwarded as UserIdentityLookupInfo entries; phone-number
support is tracked in #398.

webLookupRecords keeps its all-or-nothing behavior, but the comment now
honestly describes it (dropping the misleading "matches webModifySubscriptions"
claim) and a new test locks the semantic: a per-record backend failure surfaces
as 500, not a partial 200.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@claude
Copy link
Copy Markdown

claude Bot commented May 27, 2026

Code Review — v1.0.0-beta.2 (#395)

Scope note: This PR spans 1,110 files / 35k+ lines, exceeding the GitHub diff API limit. Review is based on direct inspection of the key new source files and their tests.

Overview

Beta.2 adds the following new surface area on top of beta.1:

  • SubscriptionslistSubscriptions, lookupSubscriptions, modifySubscriptions, createSubscription, deleteSubscription
  • Zone mutationmodifyZones, createZone, deleteZone, fetchAllZoneChanges
  • APNs tokenscreateAPNsToken, registerAPNsToken
  • Asset re-referencingrereferenceAssets, rereferenceAsset (two overloads)
  • Batch convenienceslookupAllRecords, discoverAllUserIdentities(lookupInfos:) via shared chunkedBatches engine
  • Web CourierCourier, WebCourierPoller, CourierNotification for headless long-poll notification delivery

The overall code quality is high. Architecture is coherent, typed throws are used throughout, import access modifiers are consistent, platform guards are present where needed, and the documentation is detailed and correct.

Issues and Suggestions

1. modifyZones doc promises client-side public-DB rejection, but there is no guard (Moderate)

The phrase "rejected here without a network round-trip" in CloudKitService+ModifyZones.swift implies a pre-flight guard, but modifyZones(_:database:) has no such guard — the error will come back as a server-side response. This is misleading. Either add the guard or remove that phrase from the doc comment.

Suggested guard at the top of modifyZones (adjust error type to match the codebase):

if case .public = database {
    throw CloudKitError.badRequest(reason: ...)
}

Same applies to createZone and deleteZone.

2. precondition for empty firesOn crashes in production (Moderate)

SubscriptionInfo.swift:102 uses precondition(!firesOn.isEmpty, ...) in the public designated initializer. A precondition crash in a library's public initializer gives callers no recovery path. The public factory methods already guard this, but a caller constructing via init(subscriptionID:kind:notificationInfo:) directly with a .query case that has an empty firesOn will crash in production (not just DEBUG).

Alternatives:

  • Make the initializer throws on this constraint violation.
  • At minimum, use assert instead of precondition to confine the crash to DEBUG builds.

3. try? silently drops decode errors in Courier.pollOnce (Minor)

Courier.swift:105: return try? CourierNotification(data: data) treats a JSON decode error the same as an empty keepalive body — both return nil and re-poll. If Apple changes the courier wire format, notifications will silently vanish with no diagnostic signal. Since pollOnce is already throws, a failed decode could be surfaced or at minimum logged before returning nil.

4. Hardcoded 250 ms backoff in Courier.notifications() (Minor)

Courier.swift:143: Task.sleep(nanoseconds: 250_000_000) is hardcoded and not configurable. Not a blocker for beta.2, but worth a constant or parameter before 1.0.0 to let callers tune polling cadence.

5. Stuck-token detection silently returns a partial result (Minor)

CloudKitService+FetchAllZoneChanges.swift:65: when the stuck-token condition fires, the function breaks and returns whatever zones were collected with no indication to the caller that pagination stopped early. A logger.warning at the break point would make this observable in production.

What's done well

  • Batch chunking engine (chunkedBatches) is clean: deterministic batch count (no runaway loop risk), Task.checkCancellation() between batches, no maxPages cliff, and correctly documented in CLAUDE.md.
  • rereferenceAsset two-step uses recordChangeTag for optimistic concurrency — a concurrent write on the target surfaces as a proper CloudKit error, not a silent overwrite.
  • Subscription duplicate detection (subscriptionLikelyDuplicate) is appropriately labeled as a hint, with clear caveats in the error docs.
  • WebCourierPoller correctly isolates its URLSession from the CloudKit API transport; ephemeralConfiguration reinforces the separation.
  • deleteSubscription correctly handles the empty-ack case and iterates results to surface any per-subscription failure.
  • Test coverage is thorough: success cases, error/failure paths, batchSize clamping (low, high, exact), empty input, failing mid-batch, stuck-token escape, subscription likely-duplicate, and Courier HTTP-error vs. keepalive behavior.
  • Import access modifiers are consistently present on all new files.

Summary

Priority Issue
Moderate modifyZones doc claims client-side public-DB rejection that does not exist in the code
Moderate precondition in public SubscriptionInfo.init hard-crashes in production
Minor try? in Courier.pollOnce silently drops decode errors
Minor 250 ms backoff and stuck-token break are not observable

Overall the PR is solid and the feature set matches the release notes. The two moderate items are worth addressing before tagging.

Generated with Claude Code

@claude
Copy link
Copy Markdown

claude Bot commented May 27, 2026

Code Review — PR #395 · V1.0.0 beta.2

Scope: 1,110 files, +35,533/−5,390 lines. The diff is too large to retrieve via the API, so this review focuses on the new MistKit core surface (subscriptions, zones, APNs tokens, asset re-reference, batch chunking, web courier) based on direct inspection of the source tree.


Overview

This release adds substantial new API surface on top of CloudKit Web Services:

  • SubscriptionslistSubscriptions, lookupSubscriptions, modifySubscriptions, createSubscription, deleteSubscription with the full SubscriptionInfo / SubscriptionOperation / SubscriptionResult type stack.
  • Zone modificationmodifyZones, createZone, deleteZone, fetchAllZoneChanges.
  • APNs tokenscreateAPNsToken, registerAPNsToken.
  • Asset re-referencerereferenceAssets (batch) and two rereferenceAsset overloads (compose).
  • Batch chunking — generic chunkedBatches engine behind lookupAllRecords and discoverAllUserIdentities(lookupInfos:batchSize:).
  • Web courierCourier, WebCourierPoller, CourierNotification for long-poll notification delivery.

The architecture is consistent with beta.1. Every public function uses typed throws(CloudKitError), all models are Sendable, and test coverage is deep with purpose-built suites per feature.


Code Quality & Style ✅

  • Explicit import access modifiers (internal import, public import) applied throughout — no bare imports.
  • swiftlint:disable discouraged_optional_boolean regions are correctly scoped and explained (NotificationInfo, SubscriptionInfo+Schema).
  • OperationResult<Success, Target> avoids forcing Failure: Error (Swift.Result's constraint) while preserving per-item failure semantics — a clean design choice that pays off when deletion acks are filtered out via init? on SubscriptionResult.
  • SubscriptionTarget.duplicateMarker uses exact string matching (reason == marker) rather than substring containment — correctly conservative given the heuristic nature of the inference.
  • Courier.notifications() correctly wires continuation.onTermination to task.cancel() so stream teardown propagates.

Issues & Suggestions

1. Misleading doc comment in modifyZones — "rejected here without a network round-trip"

File: CloudKitService+ModifyZones.swift:47

/// CloudKit's `zones/modify` endpoint is only supported on the `.private`
/// and `.shared` databases — `.public` has only `_defaultZone`, so any
/// modify against it is rejected here without a network round-trip.

No local guard against .public exists in modifyZones — the rejection comes from the server (or client(for:)). "Rejected here" implies a local throw that doesn't happen. Either add a guard:

guard case .public = database else { /* proceed */ }
throw CloudKitError.badRequest(reason: "modifyZones is not supported on the public database")

…or update the comment to "rejected by the server."


2. precondition in public SubscriptionInfo.init traps on empty firesOn

File: SubscriptionInfo.swift:102

precondition(!firesOn.isEmpty, "Query subscription firesOn must not be empty.")

This is a deliberate, documented design choice, and the static factory methods shield the common path. However, since Kind is public, library consumers who construct a .query case directly and pass it to init(subscriptionID:kind:notificationInfo:) will get a crash in both debug and release builds. Consider whether an unconditional trap is appropriate for a library, or whether this should be a throwing initializer or at minimum a preconditionFailure that documents the production behavior explicitly.


3. fetchAllZoneChanges has a default database: .private — inconsistent with the rest of the API

File: CloudKitService+FetchAllZoneChanges.swift:72

public func fetchAllZoneChanges(
  syncToken: String? = nil,
  maxPages: Int = 1_000,
  database: Database = .private        // ← only operation with a default database

Per CLAUDE.md: "There is no default on the operation database: parameter — every call must pick explicitly." Every other operation in the service requires an explicit database:. This one default could silently route calls to the wrong database if a caller forgets the argument. Removing the default makes the call site self-documenting.


4. Hardcoded 250 ms keepalive backoff in Courier.notifications()

File: Courier.swift:143

try await Task.sleep(nanoseconds: 250_000_000)

The polling backoff on empty/keepalive frames is not configurable. For most uses this is fine, but a server/CI environment that polls aggressively or a slow connection that returns frequent keepalives would benefit from a configurable delay. Consider promoting it to a parameter with a 250ms default, or at minimum a named constant.


5. Empty-operations guard absent in modifyZones / modifySubscriptions

Calling either method with [] sends a valid but pointless network round-trip. chunkedBatches already short-circuits for empty input; the zone and subscription modify paths don't. Not a bug, but a minor efficiency gap worth noting for consistency.


6. registerAPNsToken trims apnsToken before sending — potential data loss for non-hex tokens

File: CloudKitService+TokenOperations.swift:132

let trimmed = apnsToken.trimmingCharacters(in: .whitespacesAndNewlines)

For standard 32-byte hex-encoded device tokens, trimming is safe and sensible. If CloudKit ever returns a token with significant trailing characters in a future format, trimming would silently corrupt it. Low risk in practice, but worth a brief comment explaining why trimming is safe here (e.g., "hex device tokens never contain whitespace").


Performance Considerations

  • chunkedBatches is sequential by design (CloudKit rate-limits) and correctly checks Task.checkCancellation() between batches — no concerns.
  • rereferenceAsset (first overload) issues three sequential requests (lookup → rereference → update). This is documented and unavoidable without caching the target record's metadata externally. The second overload is the escape hatch for callers who already have it.
  • WebCourierPoller.ephemeralConfiguration uses a dedicated URLSession — correctly isolated from the CloudKit API transport per the HTTP/2 connection-pool separation requirement.

Test Coverage

Good coverage across all new features:

Feature Test Files
Subscriptions CloudKitServiceTests.Subscriptions+{SuccessCases,FailureCases,LikelyDuplicateCases,Helpers}
Zones (create/delete/modify) CloudKitServiceTests.{CreateZone,DeleteZone,ModifyZones,FetchZoneChanges}+*
APNs Tokens CloudKitServiceTests.Tokens+{SuccessCases,FailureCases}
Asset Rereference CloudKitServiceTests.Rereference+{SuccessCases,Compose,ComposeErrors}
Batch Chunking CloudKitServiceTests.BatchChunking+{LookupAllRecords,UserIdentityChunking}

Possible gap: the compose overloads in rereferenceAsset use only results.first — confirm Rereference+ComposeErrors covers the case where lookupRecords returns more than one result (and verifies only the first is used), not just the case where zero are returned.


Security

  • APNs token values are transmitted only to CloudKit API endpoints (no third-party logging exposure beyond what LoggingMiddleware emits at .debug). Consistent with the existing "no built-in redaction" policy.
  • webcourierURL from createAPNsToken is validated as a URL before storing. No SSRF risk since the value originates from a trusted Apple API.
  • No new input-validation surface that could introduce injection issues.

Summary

This is high-quality, well-structured code. The three items worth acting on before merge:

  1. Fix or remove the "rejected here" claim in modifyZones docs (correctness).
  2. Remove the database: .private default from fetchAllZoneChanges to stay consistent with the rest of the API (convention, silent-bug risk).
  3. Evaluate the precondition in SubscriptionInfo.init — if production crash on misuse is intentional, document it clearly in the public API docs; if not, consider a throwing init.

Everything else is minor polish. The subscription machinery (especially the delete-ack filtering via init? and the isLikelyDuplicate heuristic) is particularly thoughtful. LGTM with the above addressed.

🤖 Generated with Claude Code

@claude
Copy link
Copy Markdown

claude Bot commented May 29, 2026

PR Review: V1.0.0 beta.2

This is a substantial release branch touching ~1100 files. I focused on the new library surface and the structural changes.


What's solid

ConfigKeyKit extraction (Examples/BushelCloud/)
Replacing the 382-line in-tree ConfigKeyKit target (ConfigKey.swift, ConfigurationKey.swift, OptionalConfigKey.swift) with a proper library dependency + 60-line extension (ConfigKey+BUSHEL.swift) is a clean win. The public import ConfigKeyKit in the extension is intentional and correct — consumers using bushelPrefixed: factories need to see ConfigKey/OptionalConfigKey symbols transitively.

chunkedBatches() implementation (CloudKitService+BatchChunking.swift)
Well-designed: empty-input guard issues no requests, batch size is clamped to 1...maxRecordsPerRequest, and cancellation is checked at the top of each iteration. Because the item count is known up-front there's no runaway-loop risk, making the maxPages-style ceiling unnecessary here. Test suite (BatchChunking+LookupAllRecords.swift) covers empty input, single/multi-batch, custom size, both clamping bounds, and mid-loop failure propagation — this is thorough.

OperationResult<Success, Target>
Good generic design. The rationale for not using Swift.Result (avoiding the Failure: Error constraint on a data payload) is stated clearly in the docstring. get() throws(CloudKitError) is the right surface.

isLikelyDuplicate heuristic
The exact-string match on "could not find subscription we just created" (vs. substring) is the right call to avoid false positives on unrelated INTERNAL_ERROR variants. The comment in SubscriptionTarget.swift explaining the empirical basis and the duplicateMarker constant are good. The CloudKitError.subscriptionLikelyDuplicate doc comment is appropriately honest: "This is MistKit's inference, not a guaranteed cause."

setup-mistkit action revision pinning
Resolving branch → SHA at action time prevents a stale cache hit when MistKit advances on the same branch. Good.

Zone API additions
createZone/deleteZone and fetchAllZoneChanges (with maxPages ceiling + stuck-token detection) fill the gaps noted in #367.


Issues / suggestions

1. precondition trap for empty firesOn on a public API (medium)

// SubscriptionInfo.swift:102
precondition(!firesOn.isEmpty, "Query subscription firesOn must not be empty.")

precondition crashes in release builds. A public library API taking caller-supplied input is typically better served by a throws factory that returns CloudKitError.badRequest (or a dedicated validation error) so the caller can recover. The issue isn't obscure — subscriptions fetched from an external source and re-registered could plausibly arrive with an empty set.

The four static factories (query(subscriptionID:...), etc.) call this initializer, so making the initializer throws would propagate to them too. Alternatively, keep the current shape but note it in the @available / migration notes.

2. Third-party CI actions not SHA-pinned (low — security best practice)

In MistDemo.yml (and the BushelCloud workflows):

uses: codecov/codecov-action@v6            # should be pinned to full SHA
uses: sersoft-gmbh/swift-coverage-action@v5
uses: jlumbroso/free-disk-space@v1.3.1

brightdigit/swift-build@v1 is first-party so lower risk, but the others are third-party and a mutable tag means the action content can change under a passing CI run. GitHub's guidance is to pin to a full commit SHA (uses: codecov/codecov-action@abc1234...). This doesn't block the PR but is worth a follow-up issue.

3. modifySubscriptions silently drops deletion acknowledgements (low — documentation gap)

SubscriptionResult(from:) returns nil for delete-ack entries (items where subscriptionType is missing), and compactMap silently drops them. This is the correct behavior — CloudKit doesn't return a full subscription object for deletes — but modifySubscriptions's doc comment doesn't mention it. Callers who call modifySubscriptions([.delete(id:)]) and expect to verify success by checking the result count will be confused. A one-liner in the doc comment ("deletions are typically acknowledged with a bare entry and omitted from the return array") would be enough.

4. listSubscriptions has no pagination (low — CloudKit limit awareness)

CloudKit caps subscriptions at 100 per database, so a listAllSubscriptions paginator probably isn't needed. But unlike lookupRecords/lookupAllRecords (which have both the bounded and unbounded variants), there's only one listSubscriptions. If the cap ever moves or a container somehow accumulates more entries, there's no escape hatch. Probably fine, just noting the asymmetry.

5. Minor: ConfigKey+BUSHEL.swift multi-parameter docstrings

/// - Parameters:
///   - base: Base key string (e.g., "sync.dry_run")
///   - defaultVal: Required default value

CLAUDE.md says "no multi-paragraph docstrings or multi-line comment blocks — one short line max." These are multi-line parameter docs. Very minor, but worth flagging for consistency with the project's lean comment convention.


Summary

The core library additions (subscriptions, zone CRUD, batch auto-chunking, RecordResult/OperationResult) are well-designed and well-tested. The ConfigKeyKit extraction is clean. The two items worth acting on before 1.0.0 stable are (1) the precondition on firesOn — a crash from a public API is surprising — and (2) the third-party action SHA pinning (can be a follow-up issue rather than a blocker here).

Otherwise this looks ready to merge.

leogdion and others added 5 commits May 29, 2026 10:49
Add data-flow, component-architecture, MistKit-integration, and
CloudKit schema-relationship diagrams to the Architecture and
CloudKit Schema sections, replacing the prior ASCII art.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
subrepo:
  subdir:   "Examples/CelestraCloud"
  merged:   "cfab860"
upstream:
  origin:   "git@github.com:brightdigit/CelestraCloud.git"
  branch:   "mistkit"
  commit:   "a0c2488"
git-subrepo:
  version:  "0.4.9"
  origin:   "https://github.com/Homebrew/brew"
  commit:   "6f293daa9f"
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The recorded parent 3b84901 (PR #379/#381 review commit) was squashed
out of history, breaking git-subrepo's ancestor check. Repoint to the
still-valid sync anchor 3452060 per git-subrepo's recovery suggestion.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
subrepo:
  subdir:   "Examples/BushelCloud"
  merged:   "1ab45e4"
upstream:
  origin:   "git@github.com:brightdigit/BushelCloud.git"
  branch:   "mistkit"
  commit:   "1ab45e4"
git-subrepo:
  version:  "0.4.9"
  origin:   "https://github.com/Homebrew/brew"
  commit:   "6f293daa9f"
@claude
Copy link
Copy Markdown

claude Bot commented May 29, 2026

Code Review: V1.0.0 beta.2

Summary: This is a substantial feature release (36K additions, 1,121 files, 35 commits) that lands Push Notifications & Subscriptions, Zone API additions (createZone/deleteZone/fetchAllZoneChanges), auto-chunking batch conveniences, asset rereference, APNs token operations, and a comprehensive ConversionError regime. The architecture is sound, the test coverage for new features is thorough, and the typed-error discipline throughout is excellent. A few items below warrant attention before ship.


Positive Highlights

  • Loud conversion failures — replacing compactMap with map + throws(ConversionError) throughout response parsing (e.g. RecordChangesResult.init(from:), zone lookups) is the right call. Silent data drops were a latent correctness bug.
  • OperationResult<Success, Target> — a clean abstraction that avoids the Failure: Error constraint of Swift.Result while still propagating per-item failures. The get() throws(CloudKitError) bridge is ergonomic.
  • chunkedBatches engine — the implementation in CloudKitService+BatchChunking.swift is correct and well-commented. Cooperative cancellation before each batch chunk is the right pattern.
  • Courier connection-pool separation — the #if !os(WASI) guard, the ephemeral URLSessionConfiguration, and the detailed inline docs mirror the asset-upload transport-separation pattern and are clearly reasoned.
  • Explicit access-level imports — the sweep from bare import to internal import / public import throughout tests and source is a good correctness habit.

Concerns

1. Breaking platform version bump — document clearly in release notes

Package.swift bumps the minimum from macOS 10.15 / iOS 13 to macOS 11 / iOS 14 / tvOS 14 / watchOS 7. This is a source-breaking change for any consumer still supporting the old floors. The PR body is empty — this should be explicitly called out in ReleaseNotes.md (which is included in the diff) with a migration note.

The @available(macOS 12.0, iOS 15.0, ...) guards on URLSession.data(for:) in URLSession+CourierPoll.swift and WebCourierPoller's session-backed inits are also worth surfacing in the API docs: callers on macOS 11 can import MistKit but cannot use these initialisers without an explicit availability guard.

2. SubscriptionInfo.Kind.query uses precondition for an empty firesOn — prefer a thrown error

// From the inline doc: "precondition-trap empty sets"

A precondition is a fatal crash in production. An empty firesOn is a caller mistake that a caller could reasonably try to handle (e.g., build the set from user input and validate it). This should throw CloudKitError.badRequest(reason:) or a SubscriptionValidationError instead of crashing. The schema/Codable decoder path already throws ConversionError.subscriptionQueryMissingFiresOn — the public factory should be consistent.

3. discoverAllUserIdentities(lookupInfos:) naming collision

The no-argument discoverAllUserIdentities() (GET /users/discover, enumerates the caller's address book) and the new discoverAllUserIdentities(lookupInfos:) (POST /users/discover, chunked batch over supplied infos) share a base name but have fundamentally different semantics. The CLAUDE.md note already flags this as an intentional overload, but callers reading API docs or autocomplete will be confused by two methods named discoverAllUserIdentities that do entirely different things. Consider batchDiscoverUserIdentities(lookupInfos:) or discoverUserIdentities(allLookupInfos:) for the chunked variant.

4. fetchAllZoneChanges stuck-token detection silently breaks out

// Stuck-token detection
if result.zones.isEmpty && result.moreComing && result.syncToken == currentToken {
    break
}

This break produces a normal return with whatever zones were collected before the stuck token — indistinguishable from a legitimately empty result. Callers cannot tell whether the loop completed cleanly or bailed early. At minimum this should emit a logger.warning(...). A stronger option is to throw a dedicated CloudKitError case (similar to zonePaginationLimitExceeded) so callers can distinguish and react.

5. modifySubscriptions uses compactMap where map is meant

return try (subscriptionsData.subscriptions ?? []).compactMap {
    try SubscriptionResult(from: $0)
}

SubscriptionResult is OperationResult<SubscriptionInfo, SubscriptionTarget> — a non-optional enum. compactMap here acts identically to map (the closure never returns nil). This is not a bug, but it misleads readers into expecting some elements might be filtered. Use map.

6. Courier.notifications 250 ms fixed backoff on persistent keepalives

try await Task.sleep(nanoseconds: 250_000_000) // on empty/unparseable body

If CloudKit consistently returns empty/malformed bodies (e.g. an upstream issue or misconfiguration), the loop busy-waits at 4 Hz indefinitely. A brief exponential backoff (cap: ~30 s) or a retry counter with a terminal throw would prevent spinning. At a minimum, a logger.debug(...) on the keepalive path would make this visible.

7. lookupZones and modifyZones validation removed without CloudKit error verification

The old code explicitly rejected empty zoneIDs and blank zone names with httpErrorWithRawResponse(statusCode: 400, ...). Those guards were removed in this PR. The removal is reasonable if CloudKit returns a clear error for these cases, but that should be confirmed empirically (ideally via an integration test that passes an empty array and asserts a specific CloudKitError). Otherwise callers may get an opaque error from the server.

8. createAPNsToken generates a fresh UUID per call when clientId is nil

The docs note this clearly, but the consequence is worth re-stating: two successive calls to createAPNsToken(environment:database:) with clientId: nil produce two independent tokens with no server-side continuity. Callers building long-lived notification pipelines need to persist their clientId. Consider whether the default should instead auto-persist a stable UUID (e.g. via UserDefaults on Apple platforms), or at least add an explicit doc warning about token sprawl.


Minor / Nits

  • APNsTokenResult.init(from:) guard let … else { try … } pattern — the try ConversionError.tokenMissingField(fieldName:).reportAndThrow() inside a guard else is slightly surprising; try in a guard else is valid Swift but unusual. A plain if let webcourierURL … else { throw … } or a dedicated helper would be clearer.
  • CourierTests coverage is thin — three tests cover pollOnce (throw on non-2xx, nil on empty, decode on 2xx). The Courier.notifications stream (cancellation, yielding, error propagation) is not tested. Given the AsyncThrowingStream + Task lifecycle, at least one test for cancellation and one for error propagation would strengthen confidence.
  • No test for fetchAllZoneChanges stuck-token break — the behaviour is documented but unverified. A test that sends moreComing: true with the same token and asserts the method returns rather than looping infinitely would catch regressions.
  • WebCourierPoller.ephemeralConfiguration is a computed static var — each call allocates a new URLSessionConfiguration. This is fine for the typical single-construction use, but documenting it as "allocates a new configuration; cache if needed" would prevent any surprise allocations in tight loops.

Summary

The core additions are well-implemented with strong typed-error discipline and good test coverage. The two highest-priority items before shipping are: (1) document the platform floor bump prominently in release notes, and (2) replace the precondition in SubscriptionInfo.Kind.query with a thrown error. The naming collision on discoverAllUserIdentities and the silent stuck-token break are also worth addressing before 1.0 GA.

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.

2 participants