Uh oh!
There was an error while loading. Please reload this page.
Add records/resolve and records/accept share operations (#41, #42) - #428
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 |
Review: |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@## v1.0.0-beta.4 #428 +/- ##
================================================
Coverage ? 80.49% ================================================
Files ? 177 Lines ? 4364 Branches ? 0 ================================================
Hits ? 3513 Misses ? 851 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:
|
Code Review — PR #428 (Standards + Spec)Reviewed via two independent parallel passes: does the diff follow this repo's documented conventions (Standards), and does it faithfully implement issues #41/#42 (Spec)? StandardsNo hard violations of documented CLAUDE.md rules found — ACLs are explicit throughout, every Judgement-call smells (baseline, not standards breaches — repo conventions already override where relevant):
Spec(a) Missing/partial requirements: none. Both endpoints, both schema groups, request-side share-creation keys, and response-side share fields are all present. Both curated methods ( (b) Scope beyond the issues' literal text (not flagged as a defect): the MistDemo (c) Implementation accuracy: endpoint paths/operationIds match ( SummaryStandards: 0 hard violations, 4 judgement-call smells (worst: the Both self-flagged verification caveats from the issue threads (no live-service call made against a real CloudKit container; 🤖 Generated with Claude Code |
26968f9 to
dd8054cCompareReviewReviewed against Bugs / CorrectnessNone found. API DesignNo issues. Hard-coding SecurityNone found. Test CoverageGood breadth overall: success-path full-field mapping, request-order preservation for A couple of small gaps worth a follow-up (non-blocking):
Style / ConventionsNone found. Every new file correctly uses One minor nit: OverallSolid, well-tested addition that follows the codebase's established patterns closely (error mapping, no- 🤖 Generated with Claude Code |
Implements CloudKit Web Services' two sharing endpoints, both documented only in Apple's archived CloudKit Web Services Reference: - `records/resolve` (#41) — resolves share short GUIDs into information about the shared records: root record, `cloudKit.share` record, owner identity, and the caller's participation. - `records/accept` (#42) — accepts shares on behalf of the current user, returning the same result shape with the caller's resulting participation. Both take `{ shortGUIDs: [ShortGUID] }` and return `{ results: [ShortGUIDResult] }`. Apple's reference fixes the path's database scope to `public`, and both act on behalf of the *current* user, so — like `fetchCaller()` — they hard-code `.public(.requires(.webAuth))` and expose no `database:` parameter. Both validate the request as a whole (a bad short GUID fails the entire call), so there is no per-item RecordResult-style failure variant. Spec changes (openapi.yaml, regenerated via Scripts/generate-openapi.sh): - New paths `records/resolve` + `records/accept`. - New schemas `ShortGUID`, `ShortGUIDResult`, `ShortGUIDResultResponse`, `ShareParticipant`, `ShareReference`, `ShareTargetReference`. - Share request keys on `RecordRequest` (`createShortGUID`, `forRecord`, `publicPermission`, `participants`) and share response keys on `RecordResponse` (`shortGUID`, `share`, `publicPermission`, `participants`, `owner`, `currentUserParticipant`), per the #42 gap analysis. Domain models land in Sources/MistKit/Models/Sharing/. `ShareInfo` lifts the share-specific keys off a `cloudKit.share` record, since `RecordInfo` models a plain record and carries no sharing metadata. `Environment` gains `Codable` so `ShareRecordInfo` can synthesize it. Verified: swift build, swift test (571 tests, 178 suites, all passing), swift-format, and ./Scripts/lint.sh (0 violations, no unused code). Closes#41Closes#42 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Flip ResolveCommand off PendingStub, add AcceptCommand, wire integration phases and web routes so records/resolve and records/accept can be tested. Co-authored-by: Cursor <cursoragent@cursor.com>
dd8054c to
85d5713Compareleogdion
commented
Aug 25, 2026
Review/merge order noteProposed order for the This PR should go last — and it's still marked draft, so that lines up. Reasons, none of them about quality:
TestingBoth
The |
Supports #437 with a curated createShare path (cloudkit.share wire type), MistDemo sharer/sharee integration, and required share fields that fail loudly when incomplete. Co-authored-by: Cursor <cursoragent@cursor.com>
leogdion
left a comment
There was a problem hiding this comment.
Look for types with similar properties and think about whether we should create a single shared struct or a protocol.
| // swiftlint:disable:next force_unwrapping | ||
| // swift-format-ignore: NeverForceUnwrap | ||
| URL(string: "https://www.icloud.com/share/\(shortGUID)")! |
There was a problem hiding this comment.
Let's make this a baseURL or baseURLComponent we can just append to
| /// Contact details CloudKit holds for a potential participant. | ||
| public struct ContactInformation: Codable, Sendable, Equatable, Hashable { | ||
| /// The candidate's email address, when known. | ||
| public let emailAddress: String? | ||
| /// The candidate's phone number, when known. | ||
| public let phoneNumber: String? | ||
| /// Initialize contact information. | ||
| /// - Parameters: | ||
| /// - emailAddress: The candidate's email address. | ||
| /// - phoneNumber: The candidate's phone number. | ||
| public init(emailAddress: String? = nil, phoneNumber: String? = nil) { | ||
| self.emailAddress = emailAddress | ||
| self.phoneNumber = phoneNumber | ||
| } | ||
| } |
There was a problem hiding this comment.
don't we already have structure like this? Either way it should not be an inner type
| private static func environment( | ||
| from payload: Components.Schemas.ShortGUIDResult.environmentPayload | ||
| ) -> Environment { | ||
| switch payload { | ||
| case .development: .development | ||
| case .production: .production | ||
| } | ||
| } | ||
| private static func zoneID(from schema: Components.Schemas.ZoneID) -> ZoneID { | ||
| ZoneID( | ||
| zoneName: schema.zoneName ?? ZoneID.defaultZone.zoneName, | ||
| ownerName: schema.ownerName | ||
| ) | ||
| } | ||
| private static func sharePair( | ||
| from schema: Components.Schemas.RecordResponse? | ||
| ) throws(ConversionError) -> (RecordInfo?, ShareInfo?) { | ||
| guard let schema else { return (nil, nil) } | ||
| let share = try RecordInfo(from: schema) | ||
| guard let shareInfo = ShareInfo(from: schema) else { | ||
| try ConversionError.shareIncomplete.reportAndThrow() | ||
| } | ||
| return (share, shareInfo) | ||
| } | ||
| private static func potentialMatches( | ||
| from schemas: Components.Schemas.ShortGUIDResult.potentialMatchListPayload? | ||
| ) throws(ConversionError) -> [SharePotentialMatch] { | ||
| let wireMatches = schemas ?? [] | ||
| var matches: [SharePotentialMatch] = [] | ||
| matches.reserveCapacity(wireMatches.count) | ||
| for matchSchema in wireMatches { | ||
| guard let match = SharePotentialMatch(from: matchSchema) else { | ||
| try ConversionError.sharePotentialMatchMissingParticipantId.reportAndThrow() | ||
| } | ||
| matches.append(match) | ||
| } | ||
| return matches | ||
| } |
There was a problem hiding this comment.
static method are usually a good sign of possibly going in an initializer
| ) throws(ConversionError) -> [SharePotentialMatch] { | ||
| let wireMatches = schemas ?? [] | ||
| var matches: [SharePotentialMatch] = [] | ||
| matches.reserveCapacity(wireMatches.count) |
There was a problem hiding this comment.
is reserveCapacity actually a good practice?
| return (share, shareInfo) | ||
| } | ||
| private static func potentialMatches( |
There was a problem hiding this comment.
this should be moved to a static method on SharePotentialMatch
| private static func sharePair( | ||
| from schema: Components.Schemas.RecordResponse? | ||
| ) throws(ConversionError) -> (RecordInfo?, ShareInfo?) { |
There was a problem hiding this comment.
never use tuples when possible. This should be on a new type with an initializer.
| /// (``CloudKitService/acceptShares(_:)``) a share. | ||
| public struct ShortGUID: Codable, Sendable, Equatable, Hashable { | ||
| /// The value of the short global ID. | ||
| public let value: String |
There was a problem hiding this comment.
shouldn't this be a Data struct or UUID?
…GUID.Value, and typed conversions. Lift shared contact details, replace conversion tuples/helpers with proper types and inits, and build invite URLs from a reusable base. Co-authored-by: Cursor <cursoragent@cursor.com>
Make ShortGUID a String typealias and rename the resolve/accept dictionary shape to ShortGUIDDictionary. Co-authored-by: Cursor <cursoragent@cursor.com>
Uh oh!
There was an error while loading. Please reload this page.
* Add phone-number support to MistDemo web users/discover (#398) * Remove deprecated API, model server error codes, refactor FieldValue conversion, add cloud toolchain (#424, #421, #378, #358, #295) * Add confirmed zone metadata (syncToken, atomic) to zone schemas (#427) * Add records/resolve and records/accept share operations (#41, #42, #428) * Add custom/shared zone support to the query path (#146) (#426) * Add changes/database and changes/zone endpoints; deprecate zones/changes (#401, #47, #46) * Clear two Linux build warnings (#433) * modifyZones per-zone failures (#431) + zones/changes metaSyncToken (#430) (#443)
Summary
Implements CloudKit Web Services' two record-sharing endpoints:
records/resolve(Fetching Record Information (records/resolve) #41) — resolves share short GUIDs into information about the shared records: root record,cloudKit.sharerecord, owner identity, and the caller's participation.records/accept(Accepting Share Records (records/accept) #42) — accepts shares on behalf of the current user, returning the same result shape with the caller's resulting participation.Public API:
Auth: no
database:parameterApple's reference fixes both paths' database scope to
public, and both operations act on behalf of the current user. So — following the existingfetchCaller()precedent — they hard-code.public(.requires(.webAuth))and expose nodatabase:parameter. This is not a silent policy default (cf.feedback_no_silent_policy_defaults): there is no valid alternative for the caller to choose, so the choice is removed from the API rather than defaulted.Both endpoints validate the request as a whole — a bad short GUID fails the entire call rather than producing a per-item error — so there is deliberately no
RecordResult-style per-item failure variant (same reasoning asassets/rereference).How the wire format was verified
Both endpoints are absent from
.claude/docs/webservices.md(and from Apple's current online docs), so per project memoryreference_cloudkit_archived_endpoints.mdthe shapes were confirmed against Apple's archived CloudKit Web Services Reference:POST …/public/records/resolve, request{ shortGUIDs: [ShortGUID] }, response{ results: [ShortGUIDResult] }POST …/public/records/accept, same request/response shapes.claude/docs/webservices.md:1610-1672createShortGUID,forRecord,publicPermission,participants) and the share response keys.claude/docs/cloudkitjs.md(CloudKit.RecordInfo,CloudKit.Share,CloudKit.ShareParticipant)Changes
openapi.yaml(regenerated via./Scripts/generate-openapi.sh—Sources/MistKitOpenAPI/was never hand-edited):records/resolve(operationId: resolveShortGUIDs) andrecords/accept(operationId: acceptShares).ShortGUID,ShortGUIDResult,ShortGUIDResultResponse,ShareParticipant,ShareReference,ShareTargetReference.RecordRequest(createShortGUID,forRecord,publicPermission,participants) and share response keys onRecordResponse(shortGUID,share,publicPermission,participants,owner,currentUserParticipant).Domain models (
Sources/MistKit/Models/Sharing/):ShortGUID,ShareRecordInfo,ShareInfo,ShareParticipant,SharePermission,ShareParticipantType,ShareAcceptanceStatus,ShareDatabaseScope,SharePotentialMatch.ShareInfolifts the share-specific keys off acloudKit.sharerecord, becauseRecordInfomodels a plain record and intentionally carries no sharing metadata.ShareDatabaseScopeis deliberately separate fromDatabase— it is a plain descriptor CloudKit returns, carrying noPublicAuthPreference.EnvironmentgainsCodable(it is already aStringraw-value enum) soShareRecordInfocan synthesize its conformance.Operations:
CloudKitService+ShareOperations.swift,CloudKitResponseProcessor+Sharing.swift, plus the twoOperations.*.Outputerror-mapping extensions andOperationInputPathconformances.Docs: CLAUDE.md/AGENTS.md operations table + a "Share Operations" section; README roadmap moves both issues into a new
v1.0.0-beta.4section.Verification
swift buildswift testmise exec -- swift-format./Scripts/lint.sh(swiftlint + swift-format lint + header.sh + periphery)./Scripts/generate-openapi.shre-runopenapi.yamlTests assert the serialized request body (that
shortGUIDsis sent in order, thatshouldFetchRootRecord/rootRecordDesiredKeysare carried, and that omitted optionals are not emitted as nulls), full response-field mapping, thepotentialMatchListambiguous-caller path,ShareInfoextraction, and top-levelBAD_REQUESThandling for both operations.Not verified
potentialMatchListis the least-documented part of the response — Types.html describesparticipantId+contactInformation{emailAddress, phoneNumber}but shows no example. Modeled as described; the fields are all optional so an unexpected shape degrades rather than throws.createShortGUID/forRecord/publicPermission/participantsrequest keys are modeled in the schema (as Accepting Share Records (records/accept) #42's gap analysis asks) and are reachable viaMistKitOpenAPI, but no curated "create a share" API is exposed onCloudKitService— that is a larger surface deserving its own issue.Closes#41
Closes#42
🤖 Generated with Claude Code