Skip to content

fix: keep contact previews independent of Paykit - #1119

Open
ben-kaufman wants to merge 4 commits into
fix/usdt-independent-recoveryfrom
fix/contact-preview-payment-dependency
Open

ben-kaufman wants to merge 4 commits into
fix/usdt-independent-recoveryfrom
fix/contact-preview-payment-dependency

Conversation

@ben-kaufman

@ben-kaufman ben-kaufman commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor

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

  • J1: new 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

  • added 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 828cb0a8 completed workflow 40142936698: 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

  • Planning/scoping: gpt-6-astra (reasoning: xhigh)
  • Implementation: gpt-6-astra (reasoning: xhigh)
  • Review: gpt-6-astra (reasoning: xhigh)
  • Review rounds: 10

@ovitrif

ovitrif commented Oct 24, 2026

Copy link
Copy Markdown
Collaborator

Triage: 3.1.0, with its Android twin synonymdev/bitkit-android#1683.

@talosmachina talosmachina left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 computes id:, so they agree.
  • Unlabelled function reference: PublicPaykitService.hasPayablePublicEndpoint has a single overload, so the reference to (String) async throws -> Bool is unambiguous; existing AddContactView(publicKey:) call sites keep compiling because the new property has a default.
  • Re-entrant save: the view is main-actor isolated and isSaving = true is set before the first await, so a second tap hits the guard.
  • Profile assigned after leaving the screen: the new Task.isCancelled check before fetchedProfile = profile covers it; isLoading = false afterwards 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants