MistDemo improvements: test split, CRUD, auth fix, native app (#271)

[leogdion]

Summary

• MistDemo: @testable import fails because MistDemo is an executableTarget #260 — Split MistDemo into MistDemoKit library + executable so swift test works with @testable import
• Phase 3: CRUD Operations - MistDemo #214 — Complete CRUD commands: Delete, Lookup, Modify, and UpdateCommand --force flag with conflict handling
• Fix web auth token capture in index.html (URL matching + reliability) #255 — Fix auth-token: proper hostname validation in index.html, Bundle.module resource lookup, .env file loading, and request.head.authority for loopback check
• Create a Full MistDemo iOS/macOS App #272 — Native CloudKit SwiftUI demo app (MistDemoApp) for macOS/iOS with Note CRUD, zone listing, and web auth token fetch
• Documentation updates: CloudKit guide, blog posts, README

Test plan

• swift test passes (859 tests) in Examples/MistDemo
• swift run mistdemo auth-token loads index.html and captures token
• swift run mistdemo delete/lookup/modify against live container
• MistDemoApp builds and runs in Xcode (macOS + iOS)

🤖 Generated with Claude Code

---

Perform an AI-assisted review on

[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: 0ec25a7d-b7ce-4769-9dcd-b946af23a558

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 271-mistdemo

---

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 0% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 25.66%. Comparing base (9421fbc) to head (f6b8551).
⚠️ Report is 1 commits behind head on v1.0.0-beta.1.

Files with missing lines	Patch %	Lines
...tKit/Service/CloudKitService+WriteOperations.swift	0.00%	1 Missing ⚠️

Additional details and impacted files

@@                Coverage Diff                @@
##           v1.0.0-beta.1     #273      +/-   ##
=================================================
- Coverage          25.70%   25.66%   -0.04%     
=================================================
  Files                 95       95              
  Lines               8232     8232              
=================================================
- Hits                2116     2113       -3     
- Misses              6116     6119       +3     

Flag	Coverage Δ
spm	25.57% <0.00%> (-0.11%)	⬇️
swift-6.1-jammy	25.57% <0.00%> (-0.13%)	⬇️
swift-6.1-noble	25.66% <0.00%> (+0.08%)	⬆️
swift-6.2-jammy	25.57% <0.00%> (ø)
swift-6.2-noble	25.57% <0.00%> (-0.02%)	⬇️
swift-6.3-jammynightly	25.57% <0.00%> (ø)
swift-6.3-noblenightly	25.57% <0.00%> (-0.11%)	⬇️

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]

Code Review: MistDemo Improvements (#271)

This PR delivers four well-scoped phases. Here's my analysis grouped by area.

---

Overview

Phase	Change	Assessment
#260	MistDemo → MistDemoKit library split	✅ Clean, backward-compatible
#214	Delete/Lookup/Modify/UpdateCommand --force	✅ Consistent patterns
#255	Auth-token hostname fix + polling fallback	✅ Security improvement
#272	Native CloudKit SwiftUI app (MistDemoApp)	✅ Good architecture

---

Code Quality

Positives:

• Conflict error mapping is clean — all three CloudKitError cases handled with a clear mapConflict(_:) helper
• Bundle.module fix is the right approach; replaces ad-hoc path searches with the Swift Package manager's own resource API
• Config types correctly conform to Sendable across all new commands (DeleteConfig, LookupConfig, ModifyConfig, FieldInputValue, FieldsInput)
• Help text quality is excellent — USAGE, REQUIRED, OPTIONS, EXAMPLES, and recovery hints pointing to --force / --atomic

Minor Issues:

1. Structured logging missing from new CRUD commands. The project uses swift-log extensively (MistKitLogger.api, .auth, .network), but DeleteCommand, LookupCommand, and ModifyCommand appear to throw errors without any structured log output. Recommend adding logger.info(...) at operation start and logger.error(...) in catch blocks to keep observability parity with existing commands.

2. Polling in index.html doesn't early-exit on success from other capture paths. The setInterval polling container._auth._ckSession runs all 40 ticks (250ms × 10s) even if postMessage or the fetch/XHR override already succeeded. This is negligible for a one-time auth flow but worth a clearInterval(pollId) call in the resolve() path of whichever method wins.

3. _auth._ckSession is a private CloudKit JS API. This is acceptable for a demo tool, but it's worth a comment in index.html noting this is an internal property that could change with CloudKit JS SDK updates (the current code is already well-commented overall, so this is a small addition).

4. NativeCloudKitService naming shares a similar name to MistKit's CloudKitService. No import conflict since they're in separate packages, but a future reader skimming Examples/ could get confused. A one-line comment at the top of the file noting "uses Apple's CloudKit framework, not MistKit" would help.

---

Security

Hostname validation fix (index.html) — GOOD:

The original substring check was vulnerable to subdomain spoofing and query-parameter injection:

// Before (vulnerable)
url.includes('apple-cloudkit.com')  // matches http://evil.com?q=apple-cloudkit.com

// After (correct)
const parsed = new URL(url, window.location.href);
return parsed.hostname === 'apple-cloudkit.com' ||
       parsed.hostname.endsWith('.apple-cloudkit.com');

The fix correctly parses the URL before checking the hostname. No new auth vulnerabilities introduced.

Manual token paste form: Acceptable. The server-side loopback check (authority.hasPrefix("localhost") || authority.hasPrefix("127.0.0.1")) prevents non-local abuse. The head.authority API switch from the raw Host header is also more correct.

---

Performance

No regressions. The one note:

• LookupCommand silently skips nil results and only warns via stderr when the count is mismatched. This is fine, but users may wonder which record names were missing. Surfacing them explicitly in the stderr message (e.g., "Warning: not found: [recordA, recordB]") would improve debuggability.

---

Test Coverage

Good coverage:

• DeleteConfigTests, LookupConfigTests, UpdateConfigTests (270 lines!), ModifyConfigTests — all Config types are unit-tested

Gaps:

1. No command-level tests for new CRUD commands. CreateCommandTests.swift exists and tests the full command pipeline with a mock service. Equivalent tests are missing for DeleteCommand, LookupCommand, and ModifyCommand. These should verify:

   • 409 responses → conflict error mapping
   • Partial-not-found in Lookup → correct stderr + partial result set
   • Non-atomic Modify with mixed success/failure → correct partial output + warning

2. ModifyConfig atomic failure scenario not tested. The tests cover valid/invalid JSON and unknown ops, but there's no test that exercises the partial-failure code path in non-atomic mode.

3. AuthTokenCommand resource loading not re-tested post-refactor. The Bundle.module change is a behavioral fix. A test verifying AuthTokenCommand resolves the HTML resource path (even with a mock bundle) would guard against regression.

4. No JavaScript tests for isCloudKitUrl(). This is a security-relevant function. Even a simple test in a shell script (node -e "...") would catch regressions if the hostname validation logic is modified.

---

Miscellaneous

• The .claude/plans/mistdemo-improvements-271.md plan file is included in the diff. It's useful context but probably shouldn't live in the shipped PR — consider adding it to .gitignore or removing it before merge.
• Dependency bumps (swift-openapi-runtime 1.9→1.11, swift-openapi-urlsession 1.2→1.3) look routine; no concerns.
• The MistDemoApp package structure is clean — @MainActor on NativeCloudKitService is the right call, avoids scattered DispatchQueue.main.async without an actor.

---

Summary

This is solid work across all four phases. No critical bugs. The main asks before merge:

1. Add command-level tests for Delete/Lookup/Modify (mirroring CreateCommandTests)
2. Add structured logging in new CRUD commands for observability parity
3. Clarify _auth._ckSession private API usage with a comment in index.html
4. Consider removing .claude/plans/mistdemo-improvements-271.md from the PR (planning artifact)

The auth-token security fix is well done and the library/executable restructure is clean. The NativeCloudKitService architecture is a good presentation-parity foundation.

🤖 Reviewed with Claude Code

[leogdion]

can we delete this file?

[leogdion]

merge MistDemo and MistDemoApp into one directory. Use standard layout for Xcode project with Swift package

[leogdion]

We are missing a crud interface in web

[leogdion]

we are missing a public database interface in both web and app

[leogdion]

We should use a environment value for the bundleIdPrefix and Development Team

[claude]

Code Review — PR #273

This is a solid PR covering four distinct features. The library split is the most architecturally significant change and is implemented cleanly. Feedback below is grouped by area.

---

Overview

Area	Verdict
Library split (MistDemoKit)	✅ Clean, enables @testable import correctly
Delete / Lookup / Modify commands	✅ Good structure, minor issues below
Auth-token fix	✅ Looks correct
MistDemoApp	⚠️ Several issues worth addressing before merge

---

Bugs / Correctness

Security-scope leak in NoteEditView.swift

startAccessingSecurityScopedResource() is called when a file is picked but stopAccessingSecurityScopedResource() only runs inside save(). If the user picks a file and then taps Cancel, the security scope is opened and never closed. Fix:

// In fileImporter result handler:
case .success(let urls):
    if let url = urls.first {
        guard url.startAccessingSecurityScopedResource() else {
            saveError = "Couldn't access file"
            return
        }
        imageURL = url
    }

Then the existing defer { scopedURL?.stopAccessingSecurityScopedResource() } in save() needs a matching call on Cancel, or the URL should be wrapped in a type that releases on dealloc.

startAccessingSecurityScopedResource() return value ignored

The call returns Bool indicating success. The current code silently ignores a false return, meaning the app may read a file it doesn't have permission to access.

---

Code Quality / Style

@EnvironmentObject in Swift 6 context

NativeCloudKitService conforms to ObservableObject and is injected via @EnvironmentObject. Since MistDemoApp targets macOS 14+ / iOS 17+, the @Observable macro is available and preferred in Swift 6 projects — it avoids the @StateObject/@EnvironmentObject pair and is more naturally Sendable-compatible. Not blocking, but worth considering for consistency with the modern Swift stance elsewhere in this repo.

Note.hash(into:) only hashes id

This is intentional but will cause surprising behavior in Set<Note>: two Note values with the same id but different fields are considered equal by Hashable, yet == also only compares id, so they're consistent. The intent is identity-based equality. This is fine but a brief comment on the == and hash implementations would prevent future readers from "fixing" it.

Plan files in diff

.claude/plans/merge-mistdemo-directories.md and .claude/plans/mistdemo-improvements-271.md are included in the PR diff. These are planning artifacts and don't belong in the merged history. They should either be excluded or cleaned up before merge.

MistDemoApp/Package.swift uses swift-tools-version 6.0

Examples/MistDemo/Package.swift uses swift-tools-version: 6.2. The two packages should align. Also, MistDemoApp targets macOS 14+ while MistDemo targets macOS 15+ — the inconsistency is fine since they're separate packages, but worth documenting if they share a CloudKit container identifier.

---

Test Coverage

New command configs have tests; command execution does not

DeleteConfigTests, LookupConfigTests, and ModifyConfigTests test config parsing well. There are no tests for the actual command execution paths (DeleteCommand.run(), LookupCommand.run(), ModifyCommand.run()). Given that UpdateCommand's --force path (HTTP 409 → retry without change tag) is likewise untested, consider adding at least one mock-backed execution test per new command — particularly for the conflict/retry flow since that's new behavior.

Missing error case coverage

The following error cases appear to lack tests:

• DeleteError.operationFailed
• LookupError.operationFailed
• ModifyError.operationsFileError, .emptyStdin, .stdinError, .operationFailed

MistDemoApp has zero tests

Understandable for a native CloudKit app, but at minimum the model layer (Note.init(_:), ZoneRow.init(_:)) and NativeCloudKitService error paths are unit-testable without a live container.

---

Minor Suggestions

• LookupConfig comma-split for --record-names: splitting on , will silently misparse record names that contain commas (unlikely but possible). A note in the help text or a --record-name (repeatable flag) approach would be more robust.
• ModifyCommand JSON format: the help text for --operations-file / stdin would benefit from a short example JSON snippet so users don't have to guess the schema.
• Draft checklist items: the test plan still has two unchecked items (delete/lookup/modify against live container, MistDemoApp Xcode build). These should be verified before moving the PR out of draft.

---

Summary

The library split and new CRUD commands are well-structured and follow project conventions. The main issues to address before merge are the security-scope leak in NoteEditView, the ignored return value from startAccessingSecurityScopedResource(), and removing the plan files from the diff. Test coverage for command execution paths and MistDemoApp models would also strengthen the PR.

🤖 Generated with Claude Code

[claude]

Code Review

Overview

This PR delivers several improvements across MistDemo:

• MistDemo: @testable import fails because MistDemo is an executableTarget #260: Splits the monolithic MistDemo executable into a MistDemoKit library + thin MistDemo entry point, enabling @testable import in tests
• Phase 3: CRUD Operations - MistDemo #214: Adds DeleteCommand, LookupCommand, and ModifyCommand; adds --force flag to UpdateCommand
• Fix web auth token capture in index.html (URL matching + reliability) #255: Fixes AuthTokenCommand with proper authority validation and Bundle.module resource lookup
• Create a Full MistDemo iOS/macOS App #272: Adds MistDemoApp as a new SwiftUI native app example

The structural split into MistDemoKit is the right move and enables proper testability. Here are my findings:

---

🔴 Blocking Issues

1. Missing tests for all new commands

The PR plan explicitly lists test files for DeleteCommand, LookupCommand, ModifyCommand, and UpdateCommand --force as required deliverables — none are present in this diff. The MistDemoTests dependency was correctly changed from "MistDemo" to "MistDemoKit", which unblocks authoring tests, but no new test files were written. Missing coverage includes:

• DeleteCommand: --force behavior, conflict error mapping, zone passthrough
• LookupCommand: missing-names stderr warning, empty response
• ModifyCommand: JSON parsing, atomic vs non-atomic, partial failure, stdin vs file path
• UpdateCommand: --force flag, conflict mapping

2. DeleteConfig parses zone but DeleteCommand doesn't use it

DeleteConfig stores a zone field, but DeleteCommand.execute() only passes recordType and recordName to deleteRecord. If the target record lives in a named zone, this silently hits the wrong zone. The zone argument needs to be threaded through.

3. Confirm MistDemoKit target includes resources: [.copy("Resources")]

AuthTokenCommand now uses Bundle.module.url(forResource:withExtension:subdirectory:) — which is the correct SPM approach. However, Bundle.module only exists when the target declares resources: in Package.swift. If MistDemoKit is missing resources: [.copy("Resources")], every auth-token invocation will throw AuthTokenError.missingResource. Please verify the Package.swift diff includes this.

---

🟡 Medium Priority

4. mapConflict is copy-pasted between UpdateCommand and DeleteCommand

Both have an identical private static func mapConflict(_ error: CloudKitError) -> XxxError? differing only in return type. Extract this to a shared protocol or generic helper in MistDemoKit. Also note that ModifyCommand has no conflict mapping at all — its catch collapses everything to ModifyError.operationFailed(...), losing structured error context for batch operations.

5. ModifyResultRow.op is hardcoded to "applied"

ModifyResultRow(op: "applied", recordName: ..., recordType: ...)

When a delete succeeds, the output says op: "applied" instead of op: "delete". Thread the actual operation type (ModifyOperationInput.op.rawValue) through to the result row.

6. Redundant dependencies on MistDemo executable target

In Package.swift, the MistDemo executable lists "ConfigKeyKit" and "MistKit" as direct dependencies alongside "MistDemoKit" — but those are already transitive through MistDemoKit. Since MistDemo.swift only contains the entry point, its dependency list should just be ["MistDemoKit"].

---

🟢 Low Priority / Style

7. Dead-code catch pattern

Both DeleteCommand and LookupCommand have:

catch let error as DeleteError { throw error }  // dead — deleteRecord can't throw DeleteError
catch let error as CloudKitError { ... }

The typed re-throw is never reached since the library functions don't throw command-layer error types. Remove or restructure the scope.

8. ModifyCommand partial-failure heuristic is imprecise

The check results.count < operations.count as a partial-failure indicator is unreliable for mixed create/update/delete batches (CloudKit may assign new recordNames for creates, reordering the count). Acceptable for a demo tool, but worth a comment explaining the limitation rather than implying it's robust.

9. .claude/plans files committed

Two plan files (merge-mistdemo-directories.md, mistdemo-improvements-271.md) are committed as new files. Notably, merge-mistdemo-directories.md instructs deleting mistdemo-improvements-271.md, yet both appear in this PR. If committing AI working notes to .claude/plans/ is intentional, that's fine — but the stale plan should be removed before merge.

---

Positive Notes

• The AuthTokenCommand fixes are well-targeted: replacing the force-unwrap HTTPField.Name("Host")! with .authority and replacing the 5-path waterfall with Bundle.module is cleaner and more correct.
• The --force / conflict-error pattern in UpdateCommand (and DeleteCommand) is a good UX pattern — catching 409 specifically and surfacing recoverySuggestion pointing to --force is helpful.
• The LookupCommand correctly separates missing-name warnings to stderr, preserving clean stdout for parseable formats.
• Dependency version bumps (swift-openapi-runtime 1.9.0→1.11.0, swift-openapi-urlsession 1.2.0→1.3.0) look routine.

---

Summary

Priority	Issue
🔴 Blocking	No tests for new commands
🔴 Blocking	DeleteCommand ignores parsed zone field
🔴 Blocking	Verify Bundle.module resources declaration in Package.swift
🟡 Medium	Duplicate mapConflict + no conflict handling in ModifyCommand
🟡 Medium	ModifyResultRow.op hardcoded to "applied"
🟡 Medium	Trim redundant transitive deps from MistDemo executable target
🟢 Low	Dead-code catch pattern in DeleteCommand/LookupCommand
🟢 Low	Add comment on partial-failure heuristic in ModifyCommand
🟢 Low	Remove stale plan file before merge

🤖 Generated with Claude Code

[claude]

Code Review — PR #273: MistDemo improvements

Overview

This PR delivers four significant improvements: library/executable split for testability, complete CRUD commands (Delete/Lookup/Modify), auth-token fixes, and a new native SwiftUI demo app. The scope is large (3803 additions) and mostly well-structured. Below are findings grouped by severity.

---

🔴 High Priority

Security: Hostname Validation is Insufficient

File: Examples/MistDemo/Sources/MistDemoKit/Commands/AuthTokenCommand.swift

The hasPrefix("localhost") check is brittle:

• localhostX.com would pass (starts with "localhost")
• 127.0.0.1.example.com could be crafted to pass

Recommended fix:

guard let host = authority.split(separator: ":").first.map(String.init),
      ["localhost", "127.0.0.1", "::1"].contains(host) else {
    return Response(status: .forbidden)
}

Bug: Security-Scoped Resource Reference Mismatch

File: Examples/MistDemo/Sources/MistDemoApp/Views/NoteEditView.swift

startAccessingSecurityScopedResource() is called on imageURL, but the defer uses scopedURL. If imageURL is mutated or reassigned between start and stop, the cleanup won't match the resource that was opened. Use a local let copy for both calls.

Missing Tests: New CRUD Commands

DeleteCommand, LookupCommand, and ModifyCommand have no unit tests in the diff. Critical paths without coverage:

• Conflict detection (409 handling)
• Partial failure in non-atomic batch operations
• --force flag behavior in UpdateCommand
• Validation of required recordName fields

The PR test plan itself marks these as unchecked ([ ] swift run mistdemo delete/lookup/modify against live container).

Bug: Errors Silently Dropped in Native Query

File: Examples/MistDemo/Sources/MistDemoApp/Services/NativeCloudKitService.swift

return matchResults.compactMap { _, recordResult -> Note? in
    switch recordResult {
    case .success(let record): return Note(record)
    case .failure: return nil  // Silently dropped
    }
}

If records fail to parse, the view shows an empty list with no error indicator. Collect and surface failures.

---

🟡 Medium Priority

Redundant Default Values in Config Structs

File: Examples/MistDemo/Sources/MistDemoKit/Configuration/DeleteConfig.swift

configReader.string(..., default: ...) ?? MistDemoConstants.Defaults.zone

Using both default: parameter and ?? is redundant — either one is sufficient. Pick one pattern and apply it consistently across all config structs.

Non-Atomic Partial Failure Not Reflected in JSON Output

File: Examples/MistDemo/Sources/MistDemoKit/Commands/ModifyCommand.swift

Warning messages written to stderr for partial batch failures won't be detected by scripts consuming JSON stdout. Consider adding a structured "warnings" or "partialFailure": true field to the JSON output.

Inconsistent Access Level in SwiftUI Views

Several view files use public import SwiftUI (public re-export). For library views this unnecessarily widens the public API surface. Use import SwiftUI instead.

Hardcoded Container Identifier

public static let demoContainerIdentifier = "iCloud.com.brightdigit.MistDemo"

This constant is duplicated between the CLI and native app. A shared constant in MistDemoKit or the package's constants file would eliminate the risk of drift.

Unbounded Queries in Native App

The QueryView allows limits up to 200 via a Stepper, but there is no pagination. Large results load into memory without batching. Consider wrapping the query in pagination via AsyncSequence consistent with the main library's design.

---

🟢 Low Priority / Positive Notes

Positive: Bundle.module Resource Lookup

Switching from fragile path-searching to Bundle.module.url(forResource:) in AuthTokenCommand is the correct approach for Swift Package resources. Good improvement.

Positive: Sendable Conformances

New config structs (DeleteConfig, LookupConfig, ModifyConfig) all declare Sendable. Verify -strict-concurrency=complete is enabled in CI to get compiler-enforced correctness.

Naming Inconsistency

ModifyConfig.swift uses mixed suffixes: ModifyOperationKind, ModifyOperationInput, ModifyConfig. Pick one convention for related types in the same file.

Missing Doc Comments

New commands and SwiftUI views lack any doc comments explaining their role. Even a one-line /// per public type would help.

Environment Variable Security Note

The .env file is correctly gitignored, but the README and .env.example should explicitly warn that CLOUDKIT_API_TOKEN must never be baked into Xcode schemes or CI logs.

---

Summary

Category	Severity	Count
Security / auth validation	🔴 High	1
Bugs	🔴 High	2
Missing tests	🔴 High	1 area
Code quality	🟡 Medium	4
Conventions / polish	🟢 Low	5

The library/executable split and the auth-token fixes are solid changes. The main blockers before merge are the hostname validation fix and test coverage for the three new CRUD commands. The SwiftUI app (MistDemoApp) is clearly marked as a preview/demo, which is fine, but the query error-swallowing should be addressed before users hit confusing empty states.

🤖 Reviewed with Claude Code

[claude]

Code Review — PR #273: MistDemo improvements

Overview

This PR delivers four related improvements:

1. Library extraction — MistDemo executable → MistDemoKit library + thin MistDemo wrapper, enabling @testable import in tests
2. New CRUD commands — delete, lookup, modify with matching configs, error types, and tests
3. Auth-token fixes — Bundle.module for resource lookup, request.head.authority for host validation, proper URL parsing in index.html
4. Native SwiftUI demo app (MistDemoApp) with CloudKit framework CRUD and zone/auth parity

The scope is large but the changes are well-structured. Most decisions are sound. A few issues worth discussing before merge.

---

Positives

• Library split is architecturally correct. Moving code into MistDemoKit gives the test target something to import without depending on an executable target. 859 tests passing is a good signal.
• Bundle.module is the right fix for resource lookup — the old path-scanning heuristic was fragile and would break in any install layout.
• request.head.authority > request.headers["Host"] — Swift NIO HTTP/2 and HTTP/1 both expose the request authority there; the header variant was unreliable.
• URL validation in index.html is a meaningful security improvement. Using new URL(url, window.location.href) and comparing parsed.hostname eliminates the substring-match bypass (http://evil.com/?q=apple-cloudkit.com). The inline comment explains the fix well.
• atomic parameter on modifyRecords — backwards-compatible default (false), clean addition.
• Error types (DeleteError, LookupError, ModifyError) are well-designed: LocalizedError, recoverySuggestion, typed variants. Consistent with the existing pattern.
• Tests use Swift Testing (@Test, #expect, arguments: for parametrization) as required by CLAUDE.md. Coverage of new config types is thorough.

---

Issues

1. public import leaks — should be import in most files

Several files use public import for modules that are not part of the module's public API surface:

// MistDemoRunner.swift
public import ConfigKeyKit   // ← MistDemoRunner.run() doesn't expose ConfigKeyKit types

// DeleteConfig.swift, ModifyConfig.swift, error files
public import Foundation
public import ConfigKeyKit
public import MistKit

public import re-exports the module, making its symbols available to any downstream target that imports MistDemoKit. That's almost certainly unintentional here — it just bloats the public interface. Change to plain import unless you specifically need the re-export.

2. NativeCloudKitService.fetchWebAuthToken uses a deprecated completion-handler API

operation.fetchWebAuthTokenCompletionBlock = { token, error in ... }

fetchWebAuthTokenCompletionBlock was deprecated in favor of CKFetchWebAuthTokenOperation's async token(for:) class method (or the operation-based delegate pattern in newer SDK versions). If the deployment target is macOS 15+ / iOS 17+, the async variant is available and avoids the manual withCheckedThrowingContinuation wrapper entirely.

3. Silent failure swallows CloudKit errors in queryNotes

case .failure:
    return nil

Silently discarding per-record errors means a partial failure (e.g. a record that's temporarily inaccessible or hits a permission error) is invisible. At a minimum, capture and surface the errors to lastError or log them via a logger. A warning to stderr would be consistent with how LookupCommand handles missing names.

4. NativeCloudKitService internal visibility is inconsistent

Properties like accountStatus, lastError, containerIdentifier, database, and methods like refreshAccountStatus, loadZones, etc. are declared without explicit access control, defaulting to internal. The class itself is public, which is fine for the product target, but the implicit internal on the members means they're accessible from App/MistDemoApp.swift (same module) but not from any external target. This is probably intentional, but it's worth being explicit (internal keyword) so the intent is clear and a future public import MistDemoApp doesn't accidentally expose them.

5. DeleteCommand.mapConflict is brittle against CloudKitError evolution

case .httpError(let statusCode) where statusCode == 409:
case .httpErrorWithDetails(let statusCode, _, let reason) where statusCode == 409:
case .httpErrorWithRawResponse(let statusCode, _) where statusCode == 409:

This pattern knows about all three HTTP-error cases in CloudKitError. If a fourth case is added to CloudKitError, this switch won't cover it and the default: return nil will silently miss the conflict. A helper on CloudKitError itself — var isConflict: Bool or var httpStatusCode: Int? — would be more maintainable and keep the mapping in one place. The same pattern appears (without the conflict mapping) in LookupCommand/ModifyCommand.

6. MistDemoApp Package.swift product isn't used by SPM consumers

.library(name: "MistDemoApp", targets: ["MistDemoApp"])

The MistDemoApp target contains SwiftUI views and imports CloudKit, but the actual app entry point lives outside SPM's Sources/ (in App/MistDemoApp.swift). SPM can't build a runnable app from a library product — the Xcode project generated by XcodeGen handles that. The library product declaration is harmless but slightly misleading; a comment explaining that it exists for XcodeGen/Xcode integration would help.

7. modify command's partial-failure warning uses a heuristic that can misfire

if !config.atomic, results.count < operations.count {
    let missing = operations.count - results.count
    // "Warning: X of Y operation(s) did not return a record (possibly rejected by the server)."
}

CloudKit's non-atomic batch response may legitimately omit result records for delete operations (the server confirms deletion by their absence in the response). Comparing raw counts conflates "delete succeeded but has no result record" with "operation was rejected". Consider checking whether any input operations were deletes before emitting the warning, or change the message to be less alarmist.

---

Minor / Nits

• commandName.padding(toLength: 12, ...) in printGeneralHelp will misalign test-integration (16 chars) in the help output. Pre-existing, but new commands don't make it worse.
• MistDemoApp.entitlements hard-codes iCloud.com.brightdigit.MistDemo. The .env.example explains BUNDLE_ID_PREFIX substitution for other signing identities, but the entitlement won't match a different container identifier if someone forks this. A comment pointing to that would help.
• The @Suite / @Test markers in test files are consistently applied. One gap: there are no tests for DeleteCommand.execute() or ModifyCommand.execute() themselves (only their configs). Adding command-level tests with a mock CloudKitService would close that gap, but that's a follow-up item, not a blocker.

---

Test Plan Completeness

The unchecked items in the PR description (delete/lookup/modify against a live container, MistDemoApp in Xcode) are live-credential tests that can't run in CI. That's expected. The unit-test coverage for the new code is solid. Just make sure the live tests are run manually before this lands on main.

---

Overall: Approve with the public import fix being the most important change before merging. The deprecated API and silent-failure issues are worth tracking as follow-ups if not addressed here.