Remove deprecated API, model server error codes, refactor FieldValue conversion, add cloud toolchain - #424
Conversation
|
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 Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## v1.0.0-beta.4 #424 +/- ##
================================================
Coverage ? 78.60%
================================================
Files ? 159
Lines ? 3818
Branches ? 0
================================================
Hits ? 3001
Misses ? 817
Partials ? 0
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Review of #424 (milestone v1.0.0-beta.4) This is a large, well-organized rollup of four issues (#421, #378, #358, #295). Overall the changes are consistent with the codebase conventions in CLAUDE.md — good exhaustive-dispatch cleanup in Bugs — stale docs referencing the removed API (breaking, since #421 deletes it in this same PR)
Any reader who copies these snippets gets a compile error. Worth a follow-up pass to wrap these in Minor — Two conflict-mapping helpers touched by this PR don't account for
Both are currently dead paths in practice, since Other notes
Nothing above blocks the merge in my view, but I'd fix the four doc snippets before shipping since they're now actively wrong (not just outdated), and log a quick follow-up issue for the |
Claude Code on the web sessions start without a Swift toolchain, so any build, test, or lint step the agent attempts fails immediately. Add a SessionStart hook that provisions Swift 6.1 (matching Package.swift's swift-tools-version) plus the tooling pinned in mise.toml. The script is guarded by CLAUDE_CODE_REMOTE so it is a no-op for local sessions, and each step is idempotent so a warm container re-runs it in seconds rather than minutes. Note that download.swift.org spells the platform two different ways: the URL path segment is dotless (ubuntu2404) while the archive and extracted directory keep the dot (ubuntu24.04). Using a single variable for both returns a 404, so they are kept separate. Provisioning failures exit 0 rather than blocking the session — the agent can still read and edit code, it just cannot build. Registering the hook additionally requires a .claude/settings.json SessionStart entry, which is not included here. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LxPMhShjWhhDgPkt7CNHPy
Parallel agent runs create git worktrees under .claude/worktrees/. Those are local scratch checkouts, not project content, so keep them out of git status. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LxPMhShjWhhDgPkt7CNHPy
The root Package.swift declares swift-tools-version 6.1, but every example
package (MistDemo, BushelCloud, CelestraCloud) declares 6.2. Installing 6.1
left those unbuildable:
error: 'mistdemo': package 'mistdemo' is using Swift tools version
6.2.0 but the installed version is 6.1.0
which meant the example targets could not be compiled or verified at all.
Install the highest tools-version any package in the repo requires; a newer
toolchain still builds the older root manifest.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LxPMhShjWhhDgPkt7CNHPy
Delete every `@available(*, deprecated)` public declaration from Sources/MistKit: - Delete CloudKitService+Operations+Deprecated.swift, which housed the two deprecated `queryRecords(recordType:filters:sortBy:...)` overloads. Callers use `queryRecords(_:limit:desiredKeys:continuationMarker:...)` with a `Query` value, or `queryAllRecords` to auto-paginate. - Drop the deprecated `queryRecords(recordType:)` requirement from `RecordManaging` plus its deprecated `queryAllRecords(recordType:)` default implementation (which silently returned a single page), and the matching conformance on `CloudKitService`. The protocol survives with `queryAllRecords(recordType:)` and `executeBatchOperations(_:)` as its two requirements, which is all its generic extensions (`sync`, `list`, `query`, the CloudKitRecordCollection helpers) need. - Drop `fetchCurrentUser()`; `users/current` is deprecated by Apple and `fetchCaller()` is the replacement. Follow-up cleanup: remove the two tests that only exercised the deprecated query overloads, migrate the remaining tests and the Examples call sites (MistDemo, BushelCloud, CelestraCloud) to the surviving API, and refresh CLAUDE.md, README.md and the DocC articles. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
#378) Replace the hand-rolled switch/if-else chains in the FieldValue conversion layer with two small classifier enums, so every dispatch point is total and `default`-free and the compiler forces new cases to be handled. Response side: - Add `FieldValue.ResponseTypeTag`, a single total mapping from the generated `FieldValueResponse._typePayload` to the value category the tag demands (`.numeric` / `.text` / `.complex`). This replaces four separate switches with `default` fallthroughs: `makeTypedScalar`, `makeTypedNumericScalar`, `makeTypedStringScalar`, and `ExpectedComplexValue.init?`. `ExpectedComplexValue` moves alongside it, unchanged, and still gates the #376 complex/list contradiction check. - Add a private `ScalarPayload` that narrows a decoded `valuePayload` to its five scalar cases. `requireNumeric`, `requireString`, and `makeInferredScalar` now project off it instead of each walking the payload with its own if-chain. Inference stays lazy so the `Int64 -> Int` narrowing only happens on the inference path, as before. - `makeTypedScalar` collapses from three nested functions to one flat switch. Request side: - `Components.Schemas.FieldValueRequest.init(from:)` becomes one exhaustive switch over `FieldValue`, dropping `makeScalarRequest` / `makeComplexRequest` and their unreachable `default` branch. FilterBuilder: - `cloudKitListType(for:)` / `cloudKitComplexListType(for:)` collapse into one exhaustive switch. Behavior is unchanged: request type tagging (TIMESTAMP/BYTES/DOUBLE only), response type recovery over first-match-wins decoding, and the fail-loud `typeValueMismatch` on scalar and complex/list contradictions all keep their existing semantics. Docs referencing the removed helper names are updated. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The eleven documented `serverErrorCode` values that still landed in `.httpErrorWithDetails` now each get a dedicated `CloudKitError` case, so consumers pattern-match by intent instead of string-matching a wire code: ACCESS_DENIED -> .accessDenied(reason:) AUTHENTICATION_FAILED -> .authenticationFailed(reason:) AUTHENTICATION_REQUIRED -> .authenticationRequired(reason:) CONFLICT -> .conflict(reason:) EXISTS -> .exists(reason:) INTERNAL_ERROR -> .internalServerError(reason:) NOT_FOUND -> .notFound(reason:) THROTTLED -> .throttled(reason:) TRY_AGAIN_LATER -> .tryAgainLater(reason:) VALIDATING_REFERENCE_ERROR -> .validatingReferenceError(reason:) ZONE_NOT_FOUND -> .zoneNotFound(reason:) together with the three added in #357 (QUOTA_EXCEEDED, BAD_REQUEST, ATOMIC_ERROR), that covers all fourteen codes enumerated by `ErrorResponse.serverErrorCode` in openapi.yaml. A code MistKit does not model becomes the new `.unknownServerError(code:statusCode:reason:)`, keeping the raw string and the status actually observed, so a future spec revision loses nothing. `.httpErrorWithDetails` is kept but narrowed to `(statusCode:reason:)`: it now means "an HTTP failure whose CloudKit JSON body carried no serverErrorCode". Dropping the `serverErrorCode: String?` payload is what makes the refactor airtight -- with every real code routed to a dedicated case, no `CloudKitError` case hands a caller a code string to switch on, and the `reason` from a codeless body is still preserved rather than degraded to a bare `.httpError`. Supporting changes: - `ServerErrorCodeDetail` + `CloudKitError.serverErrorDetail` hold the single exhaustive case -> (code, documented HTTP status, summary) table. Adding a case to `CloudKitError` fails to compile until it is classified there. - `CloudKitError.init(serverErrorCode:statusCode:reason:)` is the single code -> case dispatch; `init(_:statusCode:)` just delegates to it. - `httpStatusCode` and `errorDescription` are both derived from that table, so descriptions read uniformly ("CloudKit not found (HTTP 404 / NOT_FOUND)"). - New public `serverErrorCode: String?` reads the raw code back off any coded case for logging, documented as diagnostics-only. Migrated every site that matched on `.httpErrorWithDetails(_, "<code>", _)`: MistDemo's delete/update conflict mapping, its error demo output, and the two integration phases that tolerate a 404; CelestraCloud's retriability check now keys off `httpStatusCode`, which picks up `.throttled` / `.tryAgainLater` / `.internalServerError` correctly for the first time. Tests: a parameterized MockTransport roundtrip over all fourteen codes asserts case identity, `serverErrorCode`, `httpStatusCode`, and description; plus forward-compat coverage for `.unknownServerError`, for a codeless body, and for what an unmodelled code does end-to-end today (the generated closed enum rejects it at decode time, so it surfaces as `.decodingError` -- never as a wrong modelled case). This is a deliberate breaking API change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The per-serverErrorCode tests (#358) and the deprecated-API removal (#421) were developed in parallel and merged without a textual conflict, because they touch disjoint files. They are still incompatible: the new ServerErrorCodes tests call queryRecords(recordType:database:), which #421 deleted, so the merged tree built but failed to compile its tests: error: extraneous argument label 'recordType:' in call error: cannot convert value of type 'String' to expected argument type 'Query' Migrate both call sites to the surviving Query-value overload, matching how #421 migrated the other query tests. Also fix a doc comment left pointing at the removed overload. Verified: swift build, swift build --build-tests, and swift test all pass under Swift 6.2 — 552 tests in 176 suites (550 baseline, -2 removed by #421, +4 added by #358). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LxPMhShjWhhDgPkt7CNHPy
…f mise Replaces the per-session toolchain install with the two-tier approach from brightdigit/ConfigKeyKit#7: Scripts/cloud-setup.sh runs once per cloud environment and is captured in the filesystem snapshot, while the SessionStart hook short-circuits when Swift is already present and re-exports PATH into CLAUDE_ENV_FILE (a snapshot restores files, not environment variables). The hook installs via swiftly against the new .swift-version pin rather than a hardcoded tarball URL. Pins .swift-version to 6.3.2, matching MistDemo-Integration.yml's swift:6.3.2-noble container. The previous 6.1 install could not build the example packages at all, which declare swift-tools-version 6.2. Adds Scripts/OpenAPITools, a standalone manifest pinning the same swift-openapi-generator version as mise.toml. mise resolves `spm:` tools through api.github.com, which cloud sessions cannot reach; SwiftPM resolves this over plain git, which they can. Keeping it in its own manifest means the generator never enters MistKit's dependency graph, preserving the no-build-plugin decision. Verified: regeneration reproduces the committed Sources/MistKitOpenAPI output byte-identically. Gates SwiftLint and periphery in Scripts/lint.sh on CLAUDE_CODE_REMOTE, so web sessions run swift-format, the header check and --build-tests instead of failing outright on tooling they cannot install. Refs #295
Share raw string, status, and summary via a single catalog used by both CloudKitError init and serverErrorDetail, and fix MemberImportVisibility CI failures in the ServerErrorCodes tests. Co-authored-by: Cursor <cursoragent@cursor.com>
9059919 to
d01e2c4
Compare
Summary
Four milestone v1.0.0-beta.4 issues, developed on separate worktrees and merged onto this branch: #421, #378, #358, and #295.
Warning
This PR contains breaking public API removals (#421). Four deprecated declarations are gone:
CloudKitService.queryRecords(recordType:),CloudKitService.queryAllRecords(recordType:),CloudKitService.fetchCurrentUser(), andRecordManaging.queryRecords(recordType:). Callers must move to the paginatingqueryRecords(recordType:filters:sortBy:limit:desiredKeys:continuationMarker:database:)and tofetchCaller(). No@available(*, deprecated)declarations remain inSources/MistKit.What's in here
#421 — Remove deprecated public API before v1.0.0
Removes the four declarations above.
RecordManagingitself is retained — it is not deprecated and still has conforming users.#358 — Model every documented
serverErrorCodeGives 14 documented CloudKit server error codes their own
CloudKitErrorcase, replacing string-matching against a generic catch-all. Unknown codes map to.unknownServerError(code:statusCode:reason:). AddsServerErrorCodeDetailand aCloudKitServiceTests.ServerErrorCodessuite;HandlingErrors.mdnow shows pattern-matching instead of string comparison.httpErrorWithDetails(statusCode:reason:)is now used exclusively for failures that carried no server code.#378 — Refactor
FieldValue↔ComponentsconversionReplaces large switch/if-else chains with exhaustive dispatch, removing 5
default:branches that were silently swallowing unhandled cases. AddsFieldValue.ResponseTypeTagand a privateScalarPayloadenum consolidating the scalar conversions.#295 — Cloud toolchain setup
Two-tier setup adopted from ConfigKeyKit#7:
Scripts/cloud-setup.shruns once per cloud environment (snapshotted), and.claude/hooks/session-start.shfalls back to installing via swiftly and re-exportsPATHintoCLAUDE_ENV_FILE. Pins.swift-versionto6.3.2, matchingMistDemo-Integration.yml.Also adds
Scripts/OpenAPITools/, a standalone manifest pinning the same generator version asmise.toml, soopenapi.yamlcan be regenerated in environments where mise cannot reachapi.github.com. Verified: regeneration reproduces the committedSources/MistKitOpenAPIoutput byte-identically.Scripts/lint.shnow gates SwiftLint and periphery onCLAUDE_CODE_REMOTE, so web sessions run swift-format, the header check and--build-testsrather than failing on tooling they cannot install.Verification
Run locally on Linux x86-64 / Swift 6.2 against the fully merged tree:
swift-formatlint + header check +--build-testsNot verified locally — relying on CI: wasm32 / wasm-embedded, Windows, Android, and all Apple platforms. SwiftLint and periphery did not run (no Linux binaries for periphery; mise cannot reach
api.github.comfrom a web session) — they need a local./Scripts/lint.sh.Follow-ups (not blocking)
Int64→Intnarrowing that would trap on 32-bit WASI for large timestamps. There is no 32-bit coverage, so the fix is unverified — a green wasm CI lane proves it compiles, not that the path is safe. Worth a review.NoteEditViewandRecordDetailViewshipped in5a58120(v1.0.0 beta.3). The issue looks closable pending aswift build --target MistDemoAppon macOS with Swift 6.3.2, which Linux cannot run (no SwiftUI).openapi.yamldeclaresserverErrorCodeas a closed enum, so unrecognized codes fail to decode before.unknownServerErroris ever reached. Deferred to theopenapi.yamlworkstream.Manual step required
.claude/settings.jsonis not in this branch and must be added for the SessionStart hook to run at all:{ "hooks": { "SessionStart": [ { "hooks": [ { "type": "command", "command": "\"$CLAUDE_PROJECT_DIR/.claude/hooks/session-start.sh\"" } ] } ] } }Closes #421
Closes #378
Closes #358
Closes #295