Add phone-number support to MistDemo web users/discover (#398); survey finds no protocol-extension hazard (#399)

[leogdion]

Two issues were assigned to this branch. #398 is implemented; #399 required no code change — the survey it asked for found no instances of the pattern. Details below.

---

#398 — MistDemo web: phone-number support for /users/discover

POST /api/users/discover already forwarded emails and user record names; the underlying discoverUserIdentities(lookupInfos:) also accepts phone numbers via UserIdentityLookupInfo(phoneNumber:), but the web demo didn't expose them. Implemented exactly the spec in the issue.

No Sources/MistKit/ change was needed — UserIdentityLookupInfo already has a phoneNumber property and a phoneNumber: initializer parameter. The whole change is confined to Examples/MistDemo.

Layer	File	Change
Request DTO	Server/WebRequests+Users.swift	DiscoverUsers gains phoneNumbers: [String] + .phoneNumbers coding key, decoded with decodeIfPresent(...) ?? [] mirroring emails/userRecordNames
Backend protocol	Server/WebBackend.swift	webDiscoverUsers gains a phoneNumbers: parameter
Conformance	Server/CloudKitService+WebBackend+Users.swift	Appends phoneNumbers.map { UserIdentityLookupInfo(phoneNumber: $0) } to the combined lookup-info array
Route	Server/WebServer+Users.swift	Forwards body.phoneNumbers; doc comments updated
Frontend markup	Resources/index.html	New users-discover-phone-numbers textarea in the Discover section
Frontend logic	Resources/js/users.js	Reads the new input, includes phoneNumbers in the POST body, validates "at least one email, phone number, or record name", and loops discoverUserIdentityWithPhoneNumber in the CloudKit JS parity path
Test mock	Tests/.../MockBackend+Calls.swift, MockBackend+UserOperations.swift	DiscoverUsersCall captures phoneNumbers; the mock returns a UserIdentity per phone number
Test	Tests/.../WebServerTests+Users.swift	usersDiscoverForwards posts a phone number, asserts 4 identities returned and captured?.phoneNumbers == ["+15555550123"]

The stale (phone-number support tracked in #398) comment in users.js is removed.

Closes #398

---

#399 — "Fix Over Extension Use With Protocol Extension Implementation Pattern"

The issue body and comments are empty — only a title. I interpreted it as the classic anti-pattern: a protocol requirement implemented in a protocol extension rather than on the conforming type, which is statically dispatched and can silently shadow a conformer's implementation.

I surveyed every protocol declaration in Sources/MistKit/ (excluding generated Sources/MistKitOpenAPI/), enumerated their declared requirements, and cross-referenced every extension <Protocol> in the module.

Survey results

Protocol	Extension member	Location	Declared requirement?	Verdict
RecordManaging	queryAllRecords(recordType:)	RecordManagement/RecordManaging.swift:75	Yes (declared L64)	Safe — witness-table dispatch
Authenticator	defaultStorageIdentifier	Authentication/Authenticator.swift:96	Yes (declared L59)	Safe — witness-table dispatch
RecordManaging	sync<T>, list<T>, query<T>(_:where:)	RecordManaging+Generic.swift:53,85,122	No	Legitimate convenience
RecordManaging where Self: CloudKitRecordCollection	syncAllRecords, listAllRecords, deleteAllRecords	RecordManaging+RecordCollection.swift:62,94,141	No	Legitimate constrained convenience
OperationInputPath	init(containerIdentifier:environment:database:)	OpenAPI/OperationInputPath.swift:52	No — distinct signature from the L46 requirement	Textbook-correct
ContainerOperationInputPath	init(containerIdentifier:environment:)	OpenAPI/ContainerOperationInputPath.swift:49	No	Textbook-correct
RecordTypeIterating	—	—	—	No extension at all; sole conformer RecordTypeSet implements forEach directly
CloudKitRecord, CloudKitRecordCollection, OperationFailureTarget, CloudKitErrorConvertible, CloudKitResponseType, TokenStorage, TokenManager	—	—	—	No protocol extensions at all

Finding: nothing to fix

The hazard does not occur anywhere in Sources/MistKit/. Every extension-provided member is either (a) a declared protocol requirement — so it dispatches through the witness table and a conformer's implementation correctly wins even through an existential, which is the supported "requirement + default implementation" pattern rather than the shadowing bug; or (b) a member with a genuinely different signature than anything the protocol declares, which can never shadow a requirement.

The shadowing bug needs an extension member that is not declared as a requirement but matches a conformer's method. No instance of that shape exists in the module.

Two adjacent observations were noted but deliberately not acted on, since both would be breaking public-API changes rather than clear fixes: RecordManaging.queryAllRecords's default is a semantic trap (delegates to the deprecated single-page query) already mitigated by an @available(*, deprecated) annotation, and Authenticator.defaultStorageIdentifier's default is effectively dead code since all three concrete authenticators override it. Neither is the bug the title describes.

Per the "report the evidence rather than manufacture a refactor" guidance, no code was changed for #399 and this PR does not carry Closes #399. The full survey is posted as a comment on the issue: #399 (comment) — recommend closing it as "no instances found", or re-scoping with a concrete example.

Note: the CloudKitService+* extension file split was explicitly not treated as in scope — CLAUDE.md documents it as an intentional organizational pattern, and splitting a concrete type's methods across extension files is not a dispatch concern.

---

Verification

All commands run locally on this branch.

Check	Command	Result
Root build	swift build	Pass — Build complete (23.5s)
Root tests	swift test	Pass — 550 tests in 173 suites
MistDemo build	cd Examples/MistDemo && swift build	Pass — Build complete (85.7s), includes the MistDemoApp SwiftUI target
MistDemo tests	cd Examples/MistDemo && swift test	Pass — 968 tests in 289 suites, 1 pre-existing known issue
Targeted test	swift test --filter usersDiscoverForwards	Pass — verified the updated assertions actually execute
Format	mise exec -- swift-format -i -r Sources/ Tests/ Examples/MistDemo/…	Clean — no reformatting produced
Lint pipeline	./Scripts/lint.sh	Pass — SwiftLint 0 violations in 390 files, header check clean, periphery: no unused code
MistDemo lint	cd Examples/MistDemo && mise exec -- swiftlint	5 warnings, all pre-existing in files untouched by this PR (LookupConfig.swift, ChangesRequestOptionsPhase.swift, LookupAllRecordsCommand.swift, DiscoverAllUserIdentitiesCommand.swift); none in any modified file

Conventions honored: explicit access modifiers on every import (no new imports were added), explicit ACLs, swiftlint type_contents_order, no hand-edits to Sources/MistKitOpenAPI/. No subrepo Example (BushelCloud, CelestraCloud) was touched — MistDemo is not a subrepo, so these changes belong on this branch.

🤖 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 Plus

Run ID: a01e2461-cd17-48e3-a323-1d25dcbcaeef

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

---

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

[claude]

Review: Add phone-number support to MistDemo web users/discover (#398)

Reviewed the diff (a808852 on branch 398-399-mistdemo-phone-extensions, base v1.0.0-beta.4). Solid, well-scoped change — feedback below.

Code quality & consistency

• The phoneNumbers plumbing follows the existing emails/userRecordNames pattern exactly at every layer: WebRequests.DiscoverUsers (alphabetical CodingKeys, same decodeIfPresent(...) ?? [] idiom), WebBackend protocol, the CloudKitService conformance, the route handler, and the mock. No inconsistencies found across the four Swift files that had to change in lockstep.
• Confirmed UserIdentityLookupInfo(phoneNumber:) already existed in Sources/MistKit/Models/Users/UserIdentityLookupInfo.swift — the PR's claim that no library change was needed checks out, and correctly keeps this change confined to Examples/MistDemo.
• The frontend (users.js) mirrors the email loop's try/catch-and-collect shape for the CloudKit JS parity path, and discoverUserIdentityWithPhoneNumber is a real CloudKit JS container method (verified against .claude/docs/cloudkitjs.md), not a hallucinated API.
• Doc comments in WebRequests+Users.swift and WebServer+Users.swift were updated in the same commit as the behavior change, so they won't go stale.

Bugs / correctness

• None found. Traced every webDiscoverUsers( and DiscoverUsersCall( call site — all are updated together, so nothing is left constructing the old 2-arg shape.
• Test count math checks out: 2 emails + 1 phone + 1 record name → payload.users.count == 4 in usersDiscoverForwards.

Test coverage

• MockBackend+UserOperations.swift and WebServerTests+Users.swift were extended to cover the new field, including asserting the captured phoneNumbers array on the mock — good, this isn't just a decode test, it checks forwarding end-to-end through the route.
• No JS-level test for users.js itself, but that matches existing practice in this file (the email/record-name paths aren't unit tested either), so it's not a regression introduced by this PR.

Minor / non-blocking notes

• Neither phone numbers nor emails are format-validated client- or server-side before being sent to CloudKit — pre-existing pattern, not something this PR needs to fix, just flagging since it's now clearer with phone numbers in the mix (E.164 formatting is easy to get wrong from a free-text CSV textarea).
• The PR also bundles two unrelated .claude/memory/ documentation commits (beta.4 worktree layout, MistDemoApp fails to build: NoteEditView/RecordDetailView have no accessible initializers (Swift 6.3) #419 verification note) into the same branch/PR as the MistDemo web: add phone-number support to /users/discover #398 feature work. Not a functional issue and it's consistent with the repo's memory-maintenance convention in CLAUDE.md, but worth knowing this PR is doing double duty if history/bisection matters later.
• Re Fix Over Extension Use With Protocol Extension Implementation Pattern #399: the survey concluding "no protocol-extension hazard" is reasonable — spot-checked the two protocols with extension members providing declared-requirement defaults (RecordManaging.queryAllRecords, Authenticator.defaultStorageIdentifier) and both dispatch through the witness table as claimed, not the shadowing anti-pattern. Agree with recommending the issue be closed or re-scoped with a concrete repro.

Security

No concerns — phone numbers are handled the same way as existing PII-adjacent fields (emails), over the same auth/transport path, no new logging or persistence introduced.

Nice work — this is a clean, minimal, well-tested change.

[claude]

REVIEW SUMMARY

Reviewed the diff for PR 425 against CLAUDE.md conventions. This is a clean, well-scoped change.

ISSUE 398 (phone-number support) - looks correct.

• The change is a faithful mechanical extension of the existing emails/userRecordNames pattern through every layer: DTO, protocol, conformance, route, frontend, mocks, and test. Exactly right for this kind of feature-parity addition.
• WebRequests+Users.swift: phoneNumbers decoding mirrors emails exactly (decodeIfPresent then default to empty array), consistent with the any-list-may-be-omitted doc comment.
• CloudKitService+WebBackend+Users.swift: lookup-info concatenation order (emails, then phoneNumbers, then userRecordNames) is harmless since CloudKit users/discover does not rely on order.
• Verified discoverUserIdentityWithPhoneNumber is a real, documented CloudKit JS method (see .claude/docs/cloudkitjs.md around line 6671), so the browser-parity loop in users.js is accurate and not a fabricated API call.
• Test usersDiscoverForwards is genuinely updated, not just padded - it asserts the captured phoneNumbers array and the resulting identity count going from 3 to 4, so it would actually fail if the wiring were dropped.
• No Sources/MistKit changes were needed, and the PR correctly did not touch it - UserIdentityLookupInfo phoneNumber initializer already existed.

Minor, non-blocking observations:

• No format validation on the phone-number input, e.g. E.164, but this matches the existing emails field, which also is not validated client- or server-side before being forwarded to CloudKit. Consistent with the demo existing let-CloudKit-validate posture, so not a real gap.
• Only one combined-input test was added; there is no dedicated phone-numbers-only case. Given the emails/userRecordNames path already established the empty-list/omitted-key behavior, this is a reasonable scope for a parity addition and not a coverage concern.

ISSUE 399 (protocol-extension survey) - no code change, and that is the right call.
The survey table is a legitimate way to close out a title-only issue with no reproducible example: it enumerates every protocol extension in Sources/MistKit and shows each one is either a declared-requirement default (safe, witness-table dispatch) or a disjoint signature that cannot shadow anything. Correctly avoided speculative refactors and left the deprecated-annotation and dead-code observations noted but unactioned, since they would be breaking API changes unrelated to the issue actual unstated concern.

SECURITY
No concerns. Phone numbers are passed through as opaque strings to CloudKit API, same trust boundary as the existing email and record-name inputs. No new injection surface, no secrets touched.

STYLE AND CONVENTIONS
Import ACLs, type_contents_order, and the copyright header are all intact in the diff. No hand-edits to Sources/MistKitOpenAPI. The .claude/memory additions (worktree layout, issue 419 status) are appropriately scoped as project memory rather than being smuggled into source comments.

Nothing blocking here - looks ready to merge pending normal CI.

[codecov]

Codecov Report

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

Additional details and impacted files

@@               Coverage Diff                @@
##             v1.0.0-beta.4     #425   +/-   ##
================================================
  Coverage                 ?   75.93%           
================================================
  Files                    ?      158           
  Lines                    ?     3761           
  Branches                 ?        0           
================================================
  Hits                     ?     2856           
  Misses                   ?      905           
  Partials                 ?        0           

Flag	Coverage Δ
mistdemo-spm-macos	11.30% <ø> (?)
mistdemo-swift-6.2-jammy	11.30% <ø> (?)
mistdemo-swift-6.2-noble	11.30% <ø> (?)
mistdemo-swift-6.3-jammy	11.30% <ø> (?)
mistdemo-swift-6.3-noble	11.30% <ø> (?)
spm	74.04% <ø> (?)
swift-6.1-jammy	74.12% <ø> (?)
swift-6.1-noble	74.04% <ø> (?)
swift-6.2-jammy	74.12% <ø> (?)
swift-6.2-noble	74.04% <ø> (?)
swift-6.3-jammy	74.20% <ø> (?)
swift-6.3-noble	74.04% <ø> (?)

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

☔ View full report in Codecov by Harness.
📢 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.