Squashed commit of the following: by leogdion · Pull Request #391 · brightdigit/MistKit

leogdion · 2026-05-26T14:19:36Z

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)

- Clamp the stderr batch-count preview in lookup-all/discover-all to 1...maxRecordsPerRequest, fixing a division-by-zero when --batch-size 0 - Use CloudKitService.maxRecordsPerRequest instead of the literal 200 for the batchSize defaults in LookupConfig/DiscoverConfig - Stop wrapping CloudKitError in LookupError in LookupAllRecordsCommand so it propagates structured errors like DiscoverAllUserIdentitiesCommand - Remove the transparent makeLookupRecordsService alias; call makeUserService directly and add a test asserting a failing batch throws and stops the loop - Clarify the singleBatch comment and the CLAUDE.md no-arg discover annotation https://claude.ai/code/session_01CiP2JokL5xB62ctP1iFasJ

coderabbitai · 2026-05-26T14:19:49Z

Important

Review skipped

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

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

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 87cf9ff8-df73-4db8-b26d-8fc4b038ade9

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 claude/pr-389-issues-ctqre

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 · 2026-05-26T14:22:17Z

Code Review — PR #391 (issues follow-up for #389 auto-chunking)

This review covers the 7-file delta that PR #391 adds on top of PR #389's batch-chunking implementation. The core engine (CloudKitService+BatchChunking.swift, +LookupAllRecords.swift, +UserIdentityChunking.swift) is reviewed via the PR #389 diff; PR #391 adds the MistDemo commands, config fields, and LookupAllRecords test suite.

Overview

PR #391 fills in the MistDemo surface and expands the test suite that were missing or incomplete in PR #389:

discover-all and lookup-all subcommands with --batch-size
batchSize field on DiscoverConfig / LookupConfig
ResponseProvider-based batch chunking tests for lookupAllRecords

The implementation is clean and follows project conventions. One bug and a few smaller issues below.

Bug: Division by Zero in Batch Logging

DiscoverAllUserIdentitiesCommand.swift:123 and LookupAllRecordsCommand.swift:219

let batches = (config.emails.count + config.batchSize - 1) / config.batchSize

If a user passes --batch-size 0, config.batchSize is 0 and this crashes with a division-by-zero trap. The clamping logic in chunkedBatches (library) protects the actual network calls, but the display line runs before the library call.

Fix: clamp before the log line, or derive the count post-call:

let clampedSize = max(1, min(config.batchSize, CloudKitService.maxRecordsPerRequest))
let batches = (config.emails.count + clampedSize - 1) / clampedSize

Asymmetric Error Handling Between the Two Commands

LookupAllRecordsCommand.swift:216-240 wraps all errors in LookupError.operationFailed, while DiscoverAllUserIdentitiesCommand.swift:113-135 propagates CloudKitError directly. The two commands use the same underlying call pattern, so the inconsistency will surprise callers who handle errors uniformly.

If LookupError.operationFailed is load-bearing (the command's upstream code parses it), document why. Otherwise align to the simpler direct-propagation style used in DiscoverAllUserIdentitiesCommand.

Test: `singleBatch` Result Count Assertion is Misleading

CloudKitServiceTests.BatchChunking+LookupAllRecords.swift:840-844

#expect(count == 1)
#expect(sizes == [5])
#expect(results.count == Self.recordsPerCall)  // == 2

The test sends 5 names but asserts results.count == 2 (the mock's per-call count). This passes but the comment-free assertion looks like a typo — a reader would expect results.count == 5. If the intent is to verify mock fidelity rather than accumulation correctness (accumulation is covered in multiBatchAccumulation), add a brief comment explaining that the result count reflects the mock, not the input count.

Minor Nit: `makeLookupRecordsService` is an Identity Alias

CloudKitServiceTests.BatchChunking+Helpers.swift:699-702

internal static func makeLookupRecordsService(
  provider: ResponseProvider
) throws -> CloudKitService {
  try makeUserService(provider: provider)
}

This is a thin pass-through with no differentiation. Either remove it and call makeUserService directly at the test sites, or give it a distinct body (e.g. different credentials or database scope) that justifies the name.

Positive Notes

chunkedBatches is a clean, DRY generic engine; the cancellation check on each iteration is correctly placed.
batchSize defaults and clamping (1...maxRecordsPerRequest) are handled in one place.
Promoting maxRecordsPerRequest to public is the right call; callers now have a typed constant instead of a magic 200.
The LookupAllRecords test suite thoroughly covers: single batch, multi-batch accumulation, order preservation, clamping low, clamping high, and empty input — good coverage.
CLAUDE.md update is thorough and accurate.

Summary

One bug (division by zero on --batch-size 0), one inconsistency (error handling asymmetry), and two minor nits. Everything else is solid. The bug should be fixed before merging.

codecov · 2026-05-26T14:25:16Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 69.77%. Comparing base (0bea4ae) to head (2e6eeb5).

Additional details and impacted files

@@                        Coverage Diff                        @@
##           307-auto-paginating-extension     #391      +/-   ##
=================================================================
+ Coverage                          69.58%   69.77%   +0.18%     
=================================================================
  Files                                165      165              
  Lines                               3758     3758              
=================================================================
+ Hits                                2615     2622       +7     
+ Misses                              1143     1136       -7

Flag	Coverage Δ
mistdemo-spm-macos	`10.85% <ø> (ø)`
mistdemo-swift-6.2-jammy	`?`
mistdemo-swift-6.2-noble	`?`
mistdemo-swift-6.3-jammy	`?`
mistdemo-swift-6.3-noble	`10.85% <ø> (ø)`
spm	`70.48% <ø> (+0.52%)`	⬆️
swift-6.1-jammy	`?`
swift-6.1-noble	`?`
swift-6.2-jammy	`?`
swift-6.2-noble	`?`
swift-6.3-jammy	`?`
swift-6.3-noble	`70.53% <ø> (+0.55%)`	⬆️

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.

The batchSize default arguments on the public LookupConfig/DiscoverConfig initializers reference CloudKitService.maxRecordsPerRequest, which requires the MistKit import to be public rather than internal. https://claude.ai/code/session_01CiP2JokL5xB62ctP1iFasJ

leogdion changed the base branch from main to 307-auto-paginating-extension May 26, 2026 14:20

leogdion merged commit db2691a into 307-auto-paginating-extension May 26, 2026
24 checks passed

leogdion deleted the claude/pr-389-issues-ctqre branch May 26, 2026 15:35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Squashed commit of the following:#391

Squashed commit of the following:#391
leogdion merged 2 commits into
307-auto-paginating-extensionfrom
claude/pr-389-issues-ctqre

leogdion commented May 26, 2026

Uh oh!

coderabbitai Bot commented May 26, 2026 •

edited

Loading

Review skipped

Uh oh!

claude Bot commented May 26, 2026

Uh oh!

codecov Bot commented May 26, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

leogdion commented May 26, 2026

Uh oh!

coderabbitai Bot commented May 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

Uh oh!

claude Bot commented May 26, 2026

Code Review — PR #391 (issues follow-up for #389 auto-chunking)

Overview

Bug: Division by Zero in Batch Logging

Asymmetric Error Handling Between the Two Commands

Test: singleBatch Result Count Assertion is Misleading

Minor Nit: makeLookupRecordsService is an Identity Alias

Positive Notes

Summary

Uh oh!

codecov Bot commented May 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

coderabbitai Bot commented May 26, 2026 •

edited

Loading

Test: `singleBatch` Result Count Assertion is Misleading

Minor Nit: `makeLookupRecordsService` is an Identity Alias

codecov Bot commented May 26, 2026 •

edited

Loading