Resolve #312: public+web-auth user-identity endpoints (#310, #311, #27, #28, #34, #35)#315
Conversation
…aller 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>
…text 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>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Code Review — PR #315: public+web-auth user-identity endpointsOverviewThis PR correctly identifies and implements that CloudKit's user-identity endpoints require a public DB + web-auth combination that the previous Code Quality & ConventionsPositives:
Suggestions:
Potential Bugs / Risks
SecurityNo concerns. The refactor reduces surface area by eliminating the impossible Test Coverage
Gap: There are no unit tests for SummarySolid PR. The architecture is correct and the implementation is clean. The main actionable items are: (1) the silent error swallow in |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## v1.0.0-beta.1 #315 +/- ##
=================================================
+ Coverage 67.16% 67.31% +0.15%
=================================================
Files 495 503 +8
Lines 13869 14062 +193
=================================================
+ Hits 9315 9466 +151
- Misses 4554 4596 +42
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
test ping |
Code Review: PR #315 — Public + Web-Auth User-Identity EndpointsOverviewThis PR fixes a real gap: CloudKit's user-identity endpoints require public-database routing with web-auth credentials — a combination the old StrengthsArchitecture refactor ( Deprecation shim Initializer defaults Test coverage IssuesBug: DemoErrorsRunner demos the wrong error for The service in Minor: generated files vs. CLAUDE.md claim Nits
SummaryThe core logic is correct and well-tested. The DemoErrorsRunner issue is worth fixing before merge since it changes the observable behavior of Reviewed with Claude Code |
…igation 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>
Code Review: PR #315 — Public+web-auth user-identity endpointsOverviewThis PR correctly models CloudKit's orthogonal database/authentication axes and unblocks the family of user-identity endpoints ( Strengths
Issues and Suggestions1. Dead
2. Variable name
3. Before this PR the comment read "API-only supports public database". The initializer now accepts 4. Integration phase ordering in User-context phases are appended after After 5. Neither 6. Minor: misleading comment in The comment says "private-database pipeline already authenticates with web-auth, so the primary service is sufficient", but the real reason for Test CoverageThe unit tests for the new lookup methods follow the project's established pattern well (success/empty/auth-error). One gap: no test for SummaryWell-executed PR. The credentials split is the right long-term design, the backward compatibility story is clean, and the live-test evidence is thorough. The items above are mostly polish (naming, a dead 🤖 Reviewed with Claude Code |
| case .unsupportedDatabaseAuthCombination(let database, let authentication): | ||
| "Database '\(database)' does not accept '\(authentication)' authentication. " | ||
| + "CloudKit allows server-to-server only against the public database; " | ||
| + "private and shared databases require web-auth credentials." |
There was a problem hiding this comment.
isn't this incorrect now?
| apiToken: String, | ||
| webAuthToken: String, | ||
| environment: Environment = .development, | ||
| database: Database = .private, |
There was a problem hiding this comment.
should database even be part of the CloudKitServer since calls could be for either
There was a problem hiding this comment.
It should be an optional argument for various calls
| containerIdentifier: String, | ||
| apiToken: String, | ||
| webAuthToken: String | ||
| webAuthToken: String, |
There was a problem hiding this comment.
We should allow any set of authentication values (private key, id, API Token, web authentication, etc...)
| // OTHER DEALINGS IN THE SOFTWARE. | ||
| // | ||
|
|
||
| extension Operations.discoverAllUserIdentities.Output: CloudKitResponseType { |
There was a problem hiding this comment.
should this be marked unavailable?
… 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>
Code Review — PR #315: Public+web-auth user-identity endpointsOverviewThis is a well-structured PR that addresses a real architectural gap: CloudKit's user-identity endpoints require a public database + web-auth credential combination that MistKit couldn't previously express. The new Issues🔴 Breaking API changes (needs acknowledgement / migration note)Two previously
Any consumer that reads 🟡
|
…ip 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 ([skip ci]): 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>
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>
leogdion
force-pushed
the
312-public-database-web-authuser-identity
branch
from
a9bdb37 to
22a1760
Compare
Code Review — PR #315: public+web-auth user-identity endpointsOverviewThis is a well-scoped PR that unblocks the family of user-identity issues (#27, #34, #35, #310, #311) by addressing a real CloudKit API constraint: user-identity endpoints only work on the public database with web-auth credentials. The architectural response — moving from a fixed Strengths
Issues / SuggestionsMedium —
} else {
throw CloudKitError.missingCredentials(
database: database,
reason: "service has neither credentials nor a fixed token manager"
)
}By reading the initializers, this state should be unreachable (every init sets one or the other), but there is no Medium — Validation test coverage for new lookup methods is thin
These would round out coverage without much effort. Minor — The Minor — The Correctness Check
SummaryThe core architecture is sound, well-tested, and well-documented. The two medium items (construction-time invariant assertion, expanded validation tests for lookup methods) would make this more robust but are not blockers. The minor items are polish. Overall this is merge-ready after consideration of the above. 🤖 Generated with Claude Code |
…iesResponse 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>
Code Review — PR #315: public+web-auth user-identity endpointsVerified against Architecture / Design
Issues1.
|
…ng (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>
…okup/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>
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>
…ailable 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>
Code Review — PR #315: public+web-auth user-identity endpointsOverviewThis PR delivers a clean, well-thought-out solution to a genuinely tricky CloudKit constraint: user-identity endpoints live on the public database but require web-auth credentials, which the existing Live-tested with 12/12 integration phases passing and 469 unit tests green. Strengths
Issues1. Per-call
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
#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>
2 participants
Closes #310, #311, #27, #34, #35.
Defers #28 (kept open with live-verification findings — see #28 comment).
Summary
CloudKit's user-identity endpoints (`/users/caller`, `/users/discover`, `/users/lookup/*`) require public-database routing with web-auth credentials — a combination MistKit's convenience layer and MistDemo's `DatabaseCredentials` enum couldn't previously express. This PR unblocks the family.
Verified live (`mistdemo test-integration --verbose`, 12/12 phases green)
database:parameter #310 — `CloudKitService` convenience initializers now accept `database:` and `environment:` parameters. Defaults preserve current behavior; source-compatible.users/currenttousers/caller#311 — `users/current` → `users/caller` rename in `openapi.yaml` and the generated client. New `fetchCaller()` plus an `@available(*, deprecated, renamed: "fetchCaller")` shim on `fetchCurrentUser()` that forwards to the new method. Returns the caller's user record live.`mistdemo test-private --verbose` is also fully green (12/12). `FetchCurrentUserPhase` was removed from the private pipeline because CloudKit rejects `users/caller` against the private database.
Deferred: #28
The Swift wrapper for GET `/users/discover` (`discoverAllUserIdentities()`) ships behind `@available(*, unavailable, message: ...)` pointing at #28. Live testing returned HTTP 500 from Apple, and the GET form does not appear in Apple's REST documentation — only in CloudKitJS. The OpenAPI definition, generated client, path builder, response processor, and Output extension all remain in place; unblocking is a one-line `@available` removal once the correct REST shape is determined. Findings written up on #28.
MistDemo refactor
Replaced `DatabaseCredentials` enum (which baked in public⇒S2S/private⇒webAuth) with two orthogonal types:
`PhaseContext.userContextService` threads an optional public+web-auth service through the integration runner. `PublicDatabaseTest` conditionally appends user-identity phases when the credential is available.
Test plan
🤖 Generated with Claude Code