Scaffold MistDemo (CLI + App + Web) for v1.0.0-beta.2 endpoints

[leogdion]

Part of #370 — scaffolds the MistDemo CLI, MistDemoApp (SwiftUI/native CloudKit), and Web app (Hummingbird) so every CloudKit Web Services endpoint in the v1.0.0-beta.2 milestone has a visible surface, with the MistKit-vs-CloudKit-JS asymmetry made explicit in the web app.

What's included

CLI — 6 pending stub commands registered in MistDemoRunner (resolve, rereference-asset, list-subscriptions, lookup-subscription, create-token, register-token); each prints "pending #N" and exits 0. Shared PendingStub helper.

Integration phases — 6 stub IntegrationPhase files (not wired into test-public/test-private, kept green).

MistDemoApp (native CloudKit) — sidebar extended from 3 → 8 items; new Records, Subscriptions, PushTokens, Assets, Users views + CompositionDisclosure.

Web app — index.html refactored into styles.css + per-area js/*.js modules. Both-mode panels (MistKit server vs CloudKit JS browser) across the milestone surface.

• Real POST /api/zones/modify route (create/delete) via CloudKitService.modifyZones + tests.
• CloudKit JS calls corrected against the bundled reference (fetchRecordZoneChanges, fetchCurrentUserIdentity, discoverUserIdentityWith{EmailAddress,UserRecordName}, saveRecordZones zone shape, fetchDatabaseChanges, cycle-safe JSON serializer).
• Zones/Subscriptions rendered as tables; Subscriptions Create (query/zone) + Delete.
• 501 pending stubs for the 6 unlanded endpoints, plus 501 stubs for landed-but-unwired endpoints (records/lookup, records/changes, zones/{list,lookup,changes}, users/*).

Status vs acceptance criteria

Done: CLI commands + registration, integration phases, app sidebar/views, HTML/JS/CSS refactor, web pending stubs, real zones/modify, CloudKit JS coverage.

Remaining (why this is still a draft):

• Wire the 9 landed-but-unwired MistKit server routes for real (currently 501) so they're exercisable in both modes: records/lookup, records/changes, zones/{list,lookup,changes} (mirror WebServer+Zones), and users/{caller,discover,lookup/email,lookup/id} (needs the public+web-auth userContextService in WebBackendFactory).
• Confirm the 5 new app views drive real native CloudKit calls and composed views carry the "📎 Composition" disclosure.
• Repo-root gates: ./Scripts/lint.sh (build + swiftlint + header.sh + periphery) and full swift test clean.

Implementing the underlying MistKit wrappers for #31/#41/#49/#50/#52/#53 stays out of scope (each lands in its own PR).

🤖 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: 5642df48-e6c6-49f8-9b7a-a519d653e4f6

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 370-scaffold-demo-apps

---

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.}

[claude]

Code Review

Scope note: This is a draft scaffolding PR, so some observations below are "when you wire this up" rather than blockers. Focusing on the real, landed code first.

---

Overview

Good structural work. The three layers (CLI stubs, SwiftUI app extensions, web refactor) are well-separated. The only "real" production code landed here is POST /api/zones/modify + static-asset serving; everything else is intentionally stub/scaffold. That's an honest split and the draft status is appropriate.

---

Real code: WebServer+Zones.swift + tests ✅

The zones modify endpoint is clean and follows the existing pattern precisely. Tests cover the three meaningful cases (create, delete, unauthenticated) with good MockBackend capture verification.

Minor gap: no test for a malformed request body (missing required fields → should be 400/422, not a 500). Worth adding before wiring this into the web UI as the primary path.

// The delete test doesn't assert the response body — it only checks status 200.
// Suggest asserting payload.zones.isEmpty (or the deleted zone's absence)
// to make the contract explicit, consistent with the create test.
#expect(response.status == .ok)
// ← no payload assertions

---

PushNotificationDelegate — static weak receiver pattern

The pattern works but has a hidden invariant: CloudKitStore.init must run before the first APNs callback arrives, and the store must remain alive for the app's lifetime. Both are true in the current SwiftUI lifecycle, but nothing enforces it.

// CloudKitStore.swift
PushNotificationDelegate.receiver = self  // weak — drops silently if store deinits

For a demo app this is fine. If this pattern migrates to library code, consider making the wiring explicit (e.g., an install(into:) method that asserts receiver == nil before writing).

---

Entitlements: duplicate APNs keys

<key>aps-environment</key>          <!-- legacy, deprecated -->
<string>development</string>
<key>com.apple.developer.aps-environment</key>  <!-- current -->
<string>development</string>

aps-environment is the old provisioning-profile key. com.apple.developer.aps-environment is the modern one. Having both is harmless on current Xcode, but aps-environment can be removed to avoid confusion.

---

CloudKitStore+Users.swift — deprecated API

discoverUserIdentity(withEmailAddress:) is deprecated in iOS 17 / macOS 14. The withCheckedThrowingContinuation wrapper is the correct approach for callback-based APIs, but it should be guarded:

// Suggested pattern:
#if swift(>=5.9)
// use the async overload if available, or check availability at runtime
#endif

Or at minimum suppress the deprecation warning with a comment explaining why (the async replacement requires a newer deployment target than this demo targets). Right now it will produce a compiler warning that could obscure other warnings.

---

PendingStub visibility

PendingStub is public in MistDemoKit. That's correct since MistDemoApp imports it, but PendingStub.Body and PendingStub.responseJSON are internal — the public surface is just printPending and responseJSON. Consider whether the Body struct needs to be exposed at all (it appears private already — ✅).

---

WebServer+Pending.swift — preconditionFailure on encoding

preconditionFailure(
  "Failed to encode pending-stub body for \(endpoint): \(error)"
)

This is the right call here — the stub body is a fixed known shape and a failure would be a programmer error caught at startup. Good use of the pattern.

---

Import modifiers

A few files in Sources/MistDemoApp/ use public import Foundation where internal import would be more appropriate (the types from Foundation don't appear in any public API surface in those files). Examples: PushNotificationDelegate.swift, CloudKitStore+PushTokens.swift. Per the project convention in CLAUDE.md, public import is only for cases where the module's types appear in a public signature. The delegate class is public but Foundation types don't flow through any of its published API here.

---

Test coverage gap for pending endpoints

WebServer+Pending.swift registers ~15 stub routes. None of them have tests verifying they return 501. A single parameterized test over the known-pending paths would lock in the contract cheaply:

@Test("Pending endpoints return 501", arguments: [
  "/api/records/lookup",
  "/api/records/changes",
  "/api/subscriptions/list",
  // ...
])
internal func pendingReturns501(path: String) async throws {
  // ...
  #expect(response.status == .notImplemented)
}

---

body(at:) test helper refactor ✅

The WebServerTests+Index.swift refactor is a genuine improvement — the helper eliminates repeated Application/app.test boilerplate and the tests are cleaner. The updated test for CloudKit JS now correctly checks app.js rather than index.html for the function names, which matches the new split.

---

Remaining draft checklist alignment

The three open items in the PR description are well-scoped and realistic. No concerns there. One suggestion: when wiring the userContextService into WebBackendFactory, ensure the public+web-auth CloudKitService is constructed lazily (or guarded behind a capability check) so the web server still starts cleanly if CLOUDKIT_WEB_AUTH_TOKEN isn't set — matching the existing graceful degradation pattern.

---

Summary

Area	Status
zones/modify endpoint + tests	✅ Ready
Static asset serving	✅ Ready
APNs delegate wiring	✅ Acceptable for demo
CLI stub commands	✅ Correct scaffolding
Integration phases	✅ Correct scaffolding
Duplicate APNs entitlement key	Minor — clean up before merge
Deprecated API warning	Minor — add suppression comment
public import in app-only files	Minor — flip to internal import
Missing 501 tests	Worth adding before merging
zones modify: no body assertion in delete test	Worth a one-liner

Overall the scaffold is clean and well-organized. The conventions from the main library (extension-per-concern, internal ACL, async/await throughout) are applied consistently throughout. The main items to address before merging are the import modifiers, the deprecated-API warning, and adding the 501 smoke tests.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (v1.0.0-beta.2@4ccdee2). Learn more about missing BASE report.

Additional details and impacted files

@@               Coverage Diff                @@
##             v1.0.0-beta.2     #371   +/-   ##
================================================
  Coverage                 ?   66.94%           
================================================
  Files                    ?      132           
  Lines                    ?     2971           
  Branches                 ?        0           
================================================
  Hits                     ?     1989           
  Misses                   ?      982           
  Partials                 ?        0           

Flag	Coverage Δ
mistdemo-spm-macos	11.67% <ø> (?)
mistdemo-swift-6.2-jammy	11.81% <ø> (?)
mistdemo-swift-6.2-noble	11.67% <ø> (?)
mistdemo-swift-6.3-jammy	11.67% <ø> (?)
mistdemo-swift-6.3-noble	11.67% <ø> (?)
spm	67.96% <ø> (?)
swift-6.1-jammy	68.03% <ø> (?)
swift-6.1-noble	68.10% <ø> (?)
swift-6.2-jammy	68.03% <ø> (?)
swift-6.2-noble	68.00% <ø> (?)
swift-6.3-jammy	67.96% <ø> (?)
swift-6.3-noble	67.89% <ø> (?)

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.

[leogdion]

can we use type aliases to reduce this?

[leogdion]

use a shared type alias or protocol

[leogdion]

use protocols and extensions to unify this

[leogdion]

let's unify this with protocols and extensions

[leogdion]

move this to a separate file

[leogdion]

fix this issue instead of ignoring it

[leogdion]

put this in a separate file

[leogdion]

let's replace this with a dictionary instead

[claude]

Code Review — PR #371: Scaffold MistDemo for v1.0.0-beta.2 endpoints

Overall this is a well-structured scaffolding PR. The PlatformApplicationDelegate abstraction across AppKit/UIKit/watchOS is clean, the PendingStub helper keeps the "not yet implemented" messaging consistent and grep-able, and the zones/modify implementation + tests follow the existing patterns well. The comments in PlatformAliases.swift explaining the SwiftLint file_name exemption are a nice touch.

A few issues worth addressing before this leaves draft:

---

Bug / Correctness

public import Foundation in CloudKitStore+PushTokens.swift (line 291)

public import Foundation

The CLAUDE.md convention is explicit: use public import only when the module's types appear in a public API signature in that file. Every method in this extension is internal; Foundation types only appear in function bodies. This should be internal import Foundation to match the rest of the file and the project convention.

---

UserIdentityRow.id falls back to UUID().uuidString (CloudKitStore+Users.swift, line 663–664)

?? UUID().uuidString

Using a randomly-generated UUID as a stable Identifiable.id breaks SwiftUI's list diffing — each time the view re-renders, an unresolvable identity gets a fresh UUID, causing unnecessary cell recreation and potentially animation glitches. If no stable identifier can be derived, consider using a deterministic fallback (e.g. lookupHint ?? "unknown") or structuring the call site to always guarantee at least one of recordName, emailAddress, or phoneNumber is present.

---

Leftover Commented-Out Code

CloudKitStore+PushTokens.swift lines 309–312

//      #else
//        pushTokenStatus = .failed(
//          message: "APNs registration is unavailable on this platform."
//        )
//      #endif

This block looks like a draft of platform-gated fallback that was left in. If it's deferred intentionally, a // TODO: note linking a tracking issue would be clearer; otherwise remove it before merge.

---

Performance / API usage

lookupRecords(names:) fetches records one at a time (CloudKitStore+Records.swift, lines 398–407)

The method loops sequentially over record names, issuing one database.record(for:) call per name. CKFetchRecordsOperation accepts a batch of CKRecord.ID values in a single operation — or at minimum withThrowingTaskGroup could run the fetches concurrently. This maps closer to the batch semantics of the REST records/lookup endpoint and is noticeably faster for even a handful of records.

discoverUsers(byEmails:) loops sequentially (CloudKitStore+Users.swift, lines 729–737)

Same pattern — each email lookup is awaited before the next starts. withThrowingTaskGroup would give concurrency here without extra complexity.

---

Silent error suppression

lookupSubscriptions ignores per-subscription failures (CloudKitStore+Subscriptions.swift, lines 570–573)

operation.perSubscriptionResultBlock = { _, result in
    if case .success(let subscription) = result {
        rows.append(SubscriptionRow(subscription))
    }
}

Individual .failure cases are silently dropped, so a subscription that fails to load (e.g. it was deleted between list and lookup) is invisible to the caller and the UI. For a demo this might be acceptable, but at minimum the method should document this behaviour — or collect errors and surface them alongside the successful rows.

---

Minor

Duplicate APNs entitlement keys (MistDemoApp.entitlements)

Both aps-environment (legacy) and com.apple.developer.aps-environment (current) are set to "development". The legacy key is redundant on modern SDKs. No functional impact, but removing aps-environment keeps the file tidy.

---

Remaining acceptance criteria (per PR description)

The three draft checklist items are clearly called out — wiring the 9 landed-but-501 server routes, confirming the new app views drive real native CloudKit calls, and a clean ./Scripts/lint.sh + swift test run. No concerns there beyond what's already noted.

🤖 Generated with Claude Code

[claude]

Code Review - PR 371

Draft scaffold for v1.0.0-beta.2 endpoints (CLI stubs, SwiftUI app views, Hummingbird web app + real zones/modify route). Review focuses on what has landed here, not the open checklist items in the PR description.

---

Positive Highlights

• zones/modify implementation in WebServer+Zones.swift is clean: guards on tokenStore.currentToken, decodes via typed WebRequests.ModifyZones, delegates to backend.webModifyZones. No security issues.
• Test coverage for the real route is solid. WebServerTests+Zones.swift covers create-only, delete-only, and unauthenticated (401) paths using the MockBackend pattern consistent with the rest of the suite.
• Stub design is intentional and clearly documented. PendingStub + pending exit-0 pattern is easy to land and replace incrementally.
• tvOS correctly added to isFlakyTimeoutSimulator in TestPlatform.swift.

---

Issues

1. Commented-out code in CloudKitStore+PushTokens.swift

requestPushNotificationRegistration contains a dead commented-out preprocessor block. CLAUDE.md prohibits commented-out code. This is a half-wired conditional abandoned after the branch was removed. Either wire the fallback with a real conditional block, or delete the comment entirely.

2. Indentation mismatch in App/MistDemoApp.swift

The @PlatformApplicationDelegateAdaptor property uses 4-space indent while the rest of the struct uses 2-space. Running mise exec -- swift-format -i App/MistDemoApp.swift will fix this automatically before promoting from draft.

3. Non-obvious comment removed from TestPlatform.swift

The diff removes the explanation of how CI reaches the test-runner process via the TEST_RUNNER_ prefix re-export mechanism. Per CLAUDE.md, comments belong exactly when the WHY is non-obvious and would surprise a future reader. The SIMCTL_CHILD_CI-does-not-work story cost real effort to discover and will cost it again without the note. Recommend restoring those lines, or at minimum the sentence about TEST_RUNNER_CI.

4. Missing test for concurrent create+delete in WebServerTests+Zones.swift

Tests cover create-only and delete-only, but not a request that simultaneously creates and deletes zones in one batch. CloudKit allows this in a single zones/modify call; the mock backend already tracks both fields independently. Easy to add while the fixture is fresh. Not a blocker for a scaffold PR.

---

Questions

• public import Foundation: CloudKitStore+PushTokens.swift, PushTokenReceiver.swift, and PushNotificationDelegate.swift use public import Foundation. Data and [AnyHashable: Any] appear in public protocol signatures so these are likely correct. Worth a quick pass to confirm no file marks the import public where Foundation types only appear in internal methods.

• Duplicate APNs entitlement keys: MistDemoApp.entitlements sets both aps-environment and com.apple.developer.aps-environment to development. The latter is the Xcode-canonical key; the former is the raw codesigning blob key. Having both is unusual — please confirm this is intentional and required on your target platforms.

---

Verdict

The scaffold is well-structured and the zones/modify route is production-quality. Issues 1–3 should be resolved before promoting from draft, alongside the existing PR checklist. The APNs entitlement question is worth a quick confirmation.

[claude]

Code Review - PR 371: Scaffold MistDemo for v1.0.0-beta.2 endpoints

Good scaffold PR overall. The pending-stub pattern is clean and grep-able, the real zones/modify route has solid test coverage, and the HTML/CSS/JS split is well-motivated. A few issues worth addressing before this merges.

---

BUGS / CORRECTNESS

1. Commented-out code in CloudKitStore+PushTokens.swift

Inside requestPushNotificationRegistration() there are three stray comment lines (the else/endif from a half-removed conditional). Delete them. The intended fallback is unreachable and misleads readers into thinking there is a conditional that got cut.

---

2. Missing GET route for users/discover in WebServer+Pending.swift

addUnwiredLandedEndpoints only registers POST for users/discover. Per CLAUDE.md, discoverAllUserIdentities() maps to GET /users/discover while discoverUserIdentities(lookupInfos:) maps to POST /users/discover. Registering only POST leaves the GET variant returning 404. Both need a stub entry:
(.get, users/discover) -- discoverAllUserIdentities
(.post, users/discover) -- discoverUserIdentities(lookupInfos:)

---

3. Sequential O(N) lookups in CloudKitStore+Records.swift

lookupRecords(names:) issues one database.record(for:) call per name. CloudKit supports batch fetch via CKFetchRecordsOperation. Worth leaving a TODO at minimum - the asymmetry vs the REST records/lookup endpoint (which batches in one call) is exactly the kind of divergence this demo is meant to surface.

---

CODE CONVENTIONS (CLAUDE.md)

4. Redundant docstrings on stub commands

Every new command file has comments that restate what the names already say: The configuration type, Creates a new instance, Executes the command. CLAUDE.md: Do not explain WHAT the code does, since well-named identifiers already do that. The type-level doc on the struct (endpoint name and tracking issue) is the right level; the per-member docs add noise without value.

---

5. PendingStub.Body missing Sendable

Minor: PendingStub.Body is a private struct captured only inside JSONEncoder().encode(...) so there is no actual race. But since PendingStub is public and the project enforces Sendable compliance, adding Sendable to the conformance list keeps the pattern consistent.

---

DESIGN / ARCHITECTURE

6. body(at:) test helper creates a new Application per call

Tests like indexExposesCloudKitJsHandlers call body(at:) twice, spinning up two full application instances per test. This will slow the suite as more tests accumulate. Consider making app a single test-scoped fixture shared across calls within one test function.

---

7. Hardcoded JS module list in WebIndexHTML.swift is fragile

The preconditionFailure only fires when a listed name is missing from the bundle. Adding a new js module without updating this list silently results in a 404. A comment noting this list must stay in sync with the script tags in index.html would prevent the footgun.

---

8. Static-weak receiver on PushNotificationDelegate

PushNotificationDelegate.receiver = self in CloudKitStore.init means any CloudKitStore created during testing stomps the receiver on the shared delegate. No tests exercise this path yet so it is fine for now, but noting this singleton side-effect will help whoever writes those tests.

---

WHAT IS GOOD

• QueryCommand+FilterParsing.swift refactor: replacing the long switch with a static lookup table is cleaner; the @sendable annotation on the closure type is correct.
• PendingStub helper: one consistent pending banner from CLI commands, integration phases, and web 501 responses is a genuinely useful grep anchor.
• Zones/modify tests (WebServerTests+Zones.swift): three scenarios (create, delete, unauthenticated) give real confidence in the routing and auth middleware.
• Static asset serving from memory: pre-loading CSS/JS at startup and serving from ByteBuffer is the right pattern for these small, stable files.
• isFlakyTimeoutSimulator fix to include tvOS: small but correct, and the comment trim is an improvement.
• PlatformAliases.swift: consolidating platform-specific typealiases with the SwiftLint file_name exclusion rationale documented in-file is clean.

---

BEFORE MARKING READY

Issue 2 (missing GET route for users/discover) is a correctness gap in the stub wiring; fix that alongside the commented-out code (issue 1) before promoting from draft.

Generated with Claude Code