Repository navigation
fix: keep contact previews independent of Paykit - #1119
ben-kaufman wants to merge 4 commits into
Conversation
|
Triage: 3.1.0, with its Android twin synonymdev/bitkit-android#1683. |
talosmachina
left a comment
There was a problem hiding this comment.
No findings. Moves the public payment-endpoint lookup out of loadProfile() into a .task(id:) keyed on the fetched profile, so the profile and Save render while the lookup runs, and guards saveContact() against re-entry. Reviewed 024d379, full tier, reasoned from the code and CI (iOS does not build on this reviewer's platform).
What I checked, and 5 candidates I ruled out
Read in full: AddContactView.swift, PublicPaykitService.hasPayablePublicEndpoint, the new AddContactViewTests.swift
Call sites traced: loadProfile() (initial .task and the error-state retry), saveContact(), hasPayableEndpoint
CI: SwiftFormat green; Run Tests still pending at review time
Ruled out
- Stale endpoint result after retry: retry sets
fetchedProfile = nil, which changes the task id, cancels the running lookup and restarts the task with a nil key that returns at once; the post-await guard also rechecks the key. - Capture list reading a different key than the task id:
[profileKey = fetchedProfile?.publicKey]is evaluated with the same body pass that computesid:, so they agree. - Unlabelled function reference:
PublicPaykitService.hasPayablePublicEndpointhas a single overload, so the reference to(String) async throws -> Boolis unambiguous; existingAddContactView(publicKey:)call sites keep compiling because the new property has a default. - Re-entrant save: the view is main-actor isolated and
isSaving = trueis set before the firstawait, so a second tap hits the guard. - Profile assigned after leaving the screen: the new
Task.isCancelledcheck beforefetchedProfile = profilecovers it;isLoading = falseafterwards is skipped too, but the view is gone.
Merge confidence: 4/5, Run Tests was still pending when this was read and the branch was not built here; the change itself is covered by the new hosted view tests.
Twin: synonymdev/bitkit-android#1683
Dependency: #1118
Description
A contact profile already fetched from Pubky stayed on the loading screen while optional payment-endpoint discovery ran. Show the profile and allow Save immediately; expose Pay only after a payable endpoint is confirmed. Ignore cancelled or superseded reads and prevent repeated Save calls from creating overlapping writes.
The confirmed contact write and existing background private-link preparation are unchanged. Those post-save helpers already schedule the handshake rather than waiting for it; this PR repairs the separate preview wait.
Out of Scope
Full-wallet backup separation, ordinary receive/startup/payment-result waits and historical contact-failure attribution are separate investigations. This PR does not report a saved contact as synced or payment-ready.
Design
N/A — no design available. Existing layout, updated loading behavior.
Preview
Existing contact preview with Save visible while optional payment discovery remains pending. Native test artifacts retain rendered iOS screenshots.
QA Notes
Setup
Use the committed Debug dependencies: Core 0.8.0-rc11, Paykit 0.1.0, VSS 0.5.23. Component tests inject held/failed public-endpoint reads and do not require funds. J1 requires a disposable authenticated identity, an unsaved peer with a readable profile, and a compatible homeserver fault fixture that delays only public payment-endpoint lookup while allowing contact writes. That complete service fixture is not currently provisioned; do not treat the component tests as its execution. No payment is needed.
Journeys
save-during-endpoint-discovery.xml— save a fetched contact while optional endpoint discovery is delayed, without claiming payment readiness.Manual Tests
N/A — the controlled service prerequisite and journey are described above.
Automated Checks
AddContactViewTests.swift— held endpoint discovery, failure and cancellation; Android also covers superseded reads, save completion and duplicate prevention.Evidence
Hosted SwiftUI screenshots/OCR prove profile and Save render while an endpoint read is held, Pay appears only after success, failure keeps the profile, and leaving cancels the task. The unchanged flow failed the held-read test.
Final native unit suite under simulator light appearance: 2,893 passed, zero failures/skips. Full SwiftFormat 0.64: 0/848 files. Finalized XCResult and complete diagnostics inspected; compiler warnings are limited to the unchanged AppIntents metadata notices. The simulator's original appearance was restored.
Hosted CI exposed a standalone test host that did not match the app's forced dark appearance. Its two failing contact rendering tests reproduce under light and now pass with production-equivalent host traits; assertions are unchanged. The full rerun also exposed an inherited payment-options test reading saved preferences, repaired by explicitly enabling its three options and restoring the defaults domain. The same isolation repair is in companion iOS #1120. Two fresh independent reviews cover the final combined test-only maintenance diff. Application source is unchanged by these CI repairs.
The subsequent hosted UI failure was in an unchanged profile-recovery clipboard test. The recording shows the Copy action ran, while the fixture label was asserted 47 ms after sampling. The test now waits for its reset sentinel, popup appearance/dismissal and exact clipboard value, with one action each and no arbitrary sleep. The exact hosted timing/interception mechanism remains unproven; the unchanged local baseline passed. Final edited-head validation passed all 2,893 selected units plus this focused UI test (2,894 total, zero failures/skips), with full SwiftFormat 0/848 and two clean independent reviews. Production behavior and exact expectations are unchanged. Runtime QoS warnings on unchanged unit paths remain documented; their original timing causes are not claimed resolved.
Hosted final head
828cb0a8completed workflow40142936698: 2,893 unit tests and 39 UI executions passed, with one native-fixture unit and four UI fixture skips. The complete log was inspected; all applicable current-head checks passed. These existing suites do not execute the authenticated fault journey below.Gaps
J1 has not run against an authenticated homeserver fault fixture. iOS hosting tests inspect rendered UI and cancellation without tapping Save through real persistence; Android save tests mock persistence/link preparation. No claim that this caused the historical contact incident.
Models used
gpt-6-astra(reasoning:xhigh)gpt-6-astra(reasoning:xhigh)gpt-6-astra(reasoning:xhigh)