Resolve #328: MistDemoApp CloudKit refresh (CKRecord-first, @Observable)

[leogdion]

Summary

Refresh MistDemoApp to use SwiftUI-idiomatic CloudKit-framework patterns and finish the app side of #275 (public/private picker). Closes #328.

• CloudKitStore (renamed from NativeCloudKitService): @Observable + @MainActor. Drops ObservableObject / @Published / Combine. Errors renamed to CloudKitStoreError.
• Note wraps CKRecord as the source of truth. Update mutates the held record and saves — no extra record(for:) fetch to refresh the change tag.
• Views switch to @Environment(CloudKitStore.self); AccountView uses @Bindable for the picker binding; sheets re-inject with .environment(_:).
• Public/private picker in AccountView; QueryView and ZoneListView re-fetch on .onChange(of: store.databaseScope) and display the active scope in their navigation title.
• Auth-token UI dropped: removes AccountView+Actions.swift, related state, and the CLOUDKIT_API_TOKEN scheme env var in project.yml. The native app authenticates via the signed-in iCloud user — no token plumbing applies.
• Package.swift: MistDemoApp target already had no MistKit dep — confirmed clean. The web demo and CLI continue to use MistKit.

Notes

• Stayed on direct CKRecord (per the issue's API surface) rather than SwiftData+CloudKit; SwiftData would shift the comparison axis of the Interactive MistDemo: web (MistKit vs CloudKit JS) + native app (pure CloudKit framework) #330 story and was an explicit non-goal here.
• Branch name predates the rename (328-native-cloudkit-framework). Happy to rename if preferred — let me know.

Test plan

• swift build in Examples/MistDemo (passes)
• ./Scripts/lint.sh from repo root (passes; no warnings on touched files)
• Regenerate xcodeproj (make generate or xcodegen) and run MistDemoApp-macOS/-iOS against a real container
  • Account view shows iCloud status + Picker for Public/Private
  • Flipping the picker re-runs the query on next visit / refreshes the title
  • Create / edit / delete Note round-trips against both publicCloudDatabase and privateCloudDatabase
  • Edit no longer triggers an extra record(for:) round-trip (network panel)
  • Zone list refreshes when scope changes

🤖 Generated with Claude Code

[coderabbitai]

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: afda5a5d-af23-4953-a8a7-5fa409aae7bd

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 328-native-cloudkit-framework

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[codecov]

Codecov Report

❌ Patch coverage is 54.62185% with 108 lines in your changes missing coverage. Please review.
✅ Project coverage is 70.51%. Comparing base (bad7b1e) to head (4f74f78).
⚠️ Report is 1 commits behind head on 330-interactive-mistdemo.

Files with missing lines	Patch %	Lines
...Sources/MistDemoKit/Server/WebBackendFactory.swift	11.76%	15 Missing ⚠️
...ces/MistDemoKit/Commands/DemoInFilterCommand.swift	0.00%	11 Missing ⚠️
...ources/MistDemoKit/Commands/DemoErrorsRunner.swift	0.00%	8 Missing ⚠️
...rces/MistDemoKit/Commands/UploadAssetCommand.swift	0.00%	8 Missing ⚠️
...ce/Extensions/CloudKitService+Classification.swift	28.57%	5 Missing ⚠️
...ces/MistDemoKit/Commands/FetchChangesCommand.swift	0.00%	4 Missing ⚠️
...mo/Sources/MistDemoKit/Commands/QueryCommand.swift	0.00%	4 Missing ⚠️
.../MistDemoKit/Integration/Phases/CleanupPhase.swift	0.00%	4 Missing ⚠️
...oKit/Integration/Phases/IncrementalSyncPhase.swift	0.00%	4 Missing ⚠️
...emoKit/Integration/Phases/LookupRecordsPhase.swift	0.00%	4 Missing ⚠️
... and 18 more

Additional details and impacted files

@@                     Coverage Diff                      @@
##           330-interactive-mistdemo     #339      +/-   ##
============================================================
- Coverage                     70.54%   70.51%   -0.04%     
============================================================
  Files                           549      551       +2     
  Lines                         15317    15422     +105     
============================================================
+ Hits                          10806    10875      +69     
- Misses                         4511     4547      +36     

Flag	Coverage Δ
mistdemo-spm-macos	60.02% <19.74%> (-0.29%)	⬇️
mistdemo-swift-6.2-jammy	?
mistdemo-swift-6.2-noble	?
mistdemo-swift-6.3-jammy	?
mistdemo-swift-6.3-noble	60.03% <19.74%> (-0.31%)	⬇️
spm	52.67% <82.40%> (+0.76%)	⬆️
swift-6.1-jammy	?
swift-6.1-noble	?
swift-6.2-jammy	?
swift-6.2-noble	?
swift-6.3-jammy	?
swift-6.3-noble	52.47% <86.11%> (+1.10%)	⬆️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[claude]

PR 339 Review: MistDemoApp CloudKit refresh

Summary: Renames NativeCloudKitService to CloudKitStore, migrates from ObservableObject/@Published/Combine to @observable, replaces @EnvironmentObject with @Environment, and adds a public/private database scope picker that re-fetches on change. A focused, clean SwiftUI modernisation.

Strengths

• @observable migration is correct throughout. @ObservationIgnored on container is the right call — it is immutable and does not need tracking. @bindable in AccountView.body is the idiomatic way to get two-way bindings from an @observable living in the environment.
• Note wraps CKRecord as source of truth. updateNote no longer performs an extra record(for:) round-trip to refresh the change tag — it mutates the held record directly, which is both correct and faster.
• Scope switcher re-fetch pattern (onChange clears the list then dispatches Task { await runQuery() }) is clean and avoids stale data from the previous scope.
• Renaming to CloudKitStore / CloudKitStoreError is a better fit for the @observable model — "store" conveys mutable state, whereas "service" implied a stateless helper.

Issues

1. DatabaseScope vs CKDatabase.Scope. CloudKitStore.DatabaseScope is a hand-rolled enum for public/private. CloudKit ships CKDatabase.Scope (.public, .private, .shared) which is already CaseIterable and maps to CKContainer.database(with:). Re-using it would remove the translation layer in the database computed property. If .shared is intentionally excluded, the custom enum is justified — but a comment noting that decision would help future contributors.

2. fetchWebAuthToken runs against self.database, but CKFetchWebAuthTokenOperation is documented to require the private database. The scope picker now makes it possible to call fetchWebAuthToken while databaseScope == .public, which would fail or return unexpected results. Consider always routing this operation to container.privateCloudDatabase regardless of databaseScope.

3. Minor: AccountView.envVarName may be unused after AccountView+Actions.swift was removed. Verify it still has a consumer; if not, remove it.

No-action notes

• @observable requires iOS 17 / macOS 14. The original ObservableObject path worked on older OS versions, but since @observable is already used this floor is accepted.
• Race between .task initial load and rapid onChange: .task is cancelled on disappear but onChange-spawned Tasks are unstructured. Fine for a demo; worth bearing in mind if strict ordering ever matters.
• Test plan items are still unchecked as expected for a UI PR — the manual verification steps look thorough.

Overall: Clean and correct modernisation. The CKDatabase.Scope question and the fetchWebAuthToken database routing are the two items worth confirming before merge; the rest is minor.

[claude]

Code Review — PR #339: MistDemoApp CloudKit refresh (CKRecord-first, @observable)

Overview

This PR has two distinct layers of change worth reviewing separately:

1. MistDemoApp SwiftUI refresh — migrates from ObservableObject/@Published/Combine to @Observable/@MainActor, renames NativeCloudKitService → CloudKitStore, adds a public/private database picker, and drops the auth-token UI scaffolding that no longer applies to a native app.
2. Database / PublicAuthPreference refactor in MistKit core — removes the old Database: String, RawRepresentable simple enum and replaces it with a richer enum that carries PublicAuthPreference on the .public case, eliminates requiresUserContext, and makes all database: parameters required (no default).

Both layers are clean and well-motivated. Notes below are mostly minor.

---

Strengths

• The PublicAuthPreference design is correct and well-documented. Encoding the signing choice directly in Database.public(PublicAuthPreference) is the right call — it eliminates a whole class of "forgot to pass requiresUserContext: true" bugs and makes the asymmetry explicit at the call site. The four factory combinations (prefers/requires × serverToServer/webAuth) are exactly the right surface.
• All database: parameters now required (no default). Previously everything defaulted to .public, which was a silent footgun for private-database callers. Removing the defaults forces every call site to be explicit. The churn in integration test / demo files is the right tradeoff.
• CredentialAvailability adds useful diagnostic granularity — preferenceRequired vs. notConfigured lets callers give different error messages for "you're missing credentials" vs. "you have credentials but picked the wrong flavor."
• fetchWebAuthToken switched to nonisolated and uses the modern fetchWebAuthTokenResultBlock (not the deprecated completion block). The comment explaining why it must run against privateCloudDatabase regardless of scope is a keeper.
• @ObservationIgnored on container is exactly right — CKContainer is a reference type and doesn't need observation tracking.
• ownerBadge accessibility label (accessibilityLabel(\"Created by you\")) is a nice touch that most reviewers would skip.

---

Issues & Suggestions

1. updateNote still does a redundant record(for:) fetch

The PR description says:

Edit no longer triggers an extra record(for:) round-trip

But CloudKitStore.updateNote still calls record(for:):

// CloudKitStore.swift ~line 170
internal func updateNote(_ existing: Note, title: String, index: Int64, imageURL: URL?) async throws -> Note {
  let record = try await database.record(for: CKRecord.ID(recordName: existing.id))
  Self.apply(title: title, index: index, imageURL: imageURL, to: record)
  ...
}

The test-plan checkbox for this is unchecked, which is honest — but the PR description implies it's solved. Either the description needs updating, or Note should be refactored to hold the CKRecord directly (as the title says "CKRecord-first") so the fetch is unnecessary. If the CKRecord were embedded in Note, updateNote could apply changes straight to existing.record and save, matching the stated intent.

2. __defaultOwner__ sentinel is a private CloudKit implementation detail

// QueryView.swift
if creator == "__defaultOwner__" { return true }

This sentinel is undocumented in Apple's public APIs and could change silently. It would be safer to compare against service.currentUserRecordName only, or document this sentinel with a // CloudKit stamps newly-created records with this sentinel before the ownership record propagates comment so future maintainers understand the dependency.

3. Sort descriptors using string keys instead of field constants

query.sortDescriptors = [
  NSSortDescriptor(key: "creationDate", ascending: false),
  NSSortDescriptor(key: "modificationDate", ascending: false),
]

"creationDate" / "modificationDate" are magic strings. Note.Fields already has a pattern for field name constants — these should either use existing constants or get new ones (Fields.creationDate, Fields.modificationDate). A typo here silently produces unsorted results with no compile-time check.

4. @Bindable var bindable = service inside body — minor inefficiency

internal var body: some View {
  @Bindable var bindable = service
  ...
}

body is called on every redraw. Declaring @Bindable inside body allocates a wrapper on every call. A @Bindable property on AccountView itself (@Bindable var service: CloudKitStore) would allocate once. This is a small cost since AccountView isn't a list row, but it's worth flagging as a pattern to avoid.

5. WebBackendFactory.swift wrapped in #if canImport(Hummingbird) without explanation

The guard was added silently. The connection to the PR's stated goals isn't obvious — was this a pre-existing compile error on non-Hummingbird targets that got folded in? A brief note in the PR body or commit message would help reviewers understand the scope.

6. CKDatabase.Scope.selectable comment mentions .shared is excluded because the demo schema has no shared zones

/// Scopes exposed in the MistDemoApp picker. `.shared` is intentionally
/// excluded because the demo's `schema.ckdb` has no shared zones.
internal static let selectable: [CKDatabase.Scope] = [.public, .private]

This is correct and the comment is clear. No action needed — just calling it out as a good pattern worth preserving in future scope additions.

7. Integration test pathSegment comparison is fragile

// PhasedIntegrationTest.swift
let dbLabel = database.pathSegment == "public" ? "public" : "private"

Now that Database has pathSegment, a switch over the enum cases would be safer and would generate a compiler warning if .shared is ever added without updating this code. Alternatively, use pathSegment directly as the label:

let dbLabel = database.pathSegment  // "public" | "private" | "shared"

8. project.yml — environment variable comment removed but variable kept

The comment explaining why CLOUDKIT_API_TOKEN was baked in was removed (correctly — it no longer applies), but the variable itself remains. The AccountView still reads CLOUDKIT_API_TOKEN via @AppStorage and the env-var fallback. If the env var is intentionally kept for the in-app token capture flow, a one-line comment explaining it would help (# Still read by AccountView's token capture flow).

---

Test Coverage

• The new PublicAuthPreference resolution logic has excellent unit test coverage in CredentialsTokenManagerTests+PublicDatabase.swift — all eight combinations of prefers/requires × credential presence are tested.
• The CloudKitStore changes have no unit tests (expected — it's a demo app targeting the native framework, not MistKit's public API). The test plan's integration steps are the right substitute.
• CKDatabaseScope+Demo.swift is trivial and untested — acceptable.

---

Summary

The PublicAuthPreference / Database refactor is solid API design that eliminates a class of silent bugs. The MistDemoApp SwiftUI refresh is clean and idiomatic. The main actionable items are:

1. Decide and document whether updateNote intentionally still fetches the latest CKRecord (for change-tag safety) or whether the PR title's "CKRecord-first" promise means embedding the record in Note.
2. Document or remove the __defaultOwner__ sentinel.
3. Replace magic sort-key strings with field constants.

🤖 Generated with Claude Code