Skip to content

fix: stop automatic profile deletion replay - #1122

Open
ben-kaufman wants to merge 6 commits into
fix/backup-observer-lifecyclefrom
fix/profile-delete-session-recovery
Open

ben-kaufman wants to merge 6 commits into
fix/backup-observer-lifecyclefrom
fix/profile-delete-session-recovery

Conversation

@ben-kaufman

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

Copy link
Copy Markdown
Contributor

Twin: synonymdev/bitkit-android#1688
Dependency: #1123

Description

  • Stop restarting the complete profile deletion flow after an error: earlier contact or profile mutations may already have succeeded when their acknowledgement is lost.
  • Remove the deletion-only session refresh helper while retaining normal initialization/recovery and the existing success/failure controls.

Out of Scope

  • Original-session ownership through Android cleanup/finalization and iOS failure-dialog Retry/Disconnect, plus safe reconciliation of partial contact mutations before manual retries: tracked in Bind profile deletion cleanup to the original sign-in bitkit-android#1687. This change removes automatic replay; it does not claim an identity-bound, resumable profile-deletion workflow.
  • Compatible native confirmed-401 request recovery remains dependent on fix: recover native grant requests after 401 pubky/pubky-homeserver#741 and subsequent Paykit artifact adoption. An invalid session may require normal recovery or re-authentication before an explicit deletion succeeds; broad transport errors do not justify replaying the destructive flow.

Design

N/A — existing progress, failure options and navigation.

Preview

An uncertain deletion failure remains a failure instead of silently repeating contact cleanup and profile deletion.

QA Notes

Setup

Use committed released dependencies and synthetic identities for unit tests. J1 requires a disposable test identity and a homeserver fault proxy that accepts one profile deletion, drops its acknowledgement and retains a redacted request counter. That complete-app fixture is unavailable; do not use a personal profile, reset wallets or use live funds.

Journeys

  • J1: new delete-profile-uncertain-response.xml — lost acknowledgement, honest failure feedback, no automatic deletion/sign-in replay, cancellation and restart without selecting Retry.

Automated Checks

  • Updated PubkyProfileManagerTests.swift — retained deletion/session guards after removing the replay-only helper.
  • Updated SendConfirmationLifecycleTests.swift — retain navigation state, view/controller hierarchy and screenshots only after the existing appearance assertion fails; preserve its deadline and passing-path scheduling.
  • Updated UsdtWalletUITests.swift through parent fix: serialize backup observation lifecycle #1123 — await hittable back navigation while preserving the existing payment, amount and recipient assertions.

Evidence

Signed additive merge 65774372 adopts #1123 at 1ad665f8; the original five-file profile patch remains byte-for-byte identical against that parent. No published history was rewritten. At that revision, the complete five-commit range verifies locally and on GitHub (verified=true, reason=valid); the previous-base six-commit range also verifies locally. Two consecutive independent composition reviews are clean.

Final combined native build/unit validation passed 2,906 selected tests with zero failures/skips. Full SwiftFormat 0.64.0 checked 847 files with zero violations. The two affected ordinary USDT UI cases passed with unchanged assertions in a bounded comparison after staging reachability preflight. Complete logs and nested XCResult diagnostics were inspected. Two AppIntents notices, four unit QoS observations and four UI QoS observations remain; timing causes are unproven. That revision's hosted tests failed as detailed below.

Android twin #1688 has controlled fail-before evidence: the original 223-test repository suite had two expected counterexamples (uncertain deletion replay and stale refresh reauthentication). Its repaired 219-test repository suite passes, including one-delete/error preservation and held failure after sign-out. iOS source tracing proves the equivalent automatic replay path is removed; native suites are regression coverage, not execution of J1.

The adopted parent repairs the previously captured subscription teardown SIGSEGV and has six released-native rejected-backup/lost-acknowledgement prepare/resume/restore passes at 538f45c7. Its test-only 1ad665f8 follow-up passed hosted run 40162108190: 2,909 unit passes and 39 UI passes, with one native-fixture unit and four UI skips. Those are dependency-revision evidence, not a new profile-deletion journey.

Gaps

  • J1 remains unrun: the authenticated full-app lost-acknowledgement proxy fixture is unavailable. Neither local tests nor existing hosted suites prove that journey. No historical contact-failure attribution is claimed.
  • Prior-head hosted 40158857938 failed the three-second visible-route deadline in testPendingWalletSwitchResumesAfterTags; later preparation/path assertions and all 39 UI executions passed. A concurrent keyboard queue timeout does not establish causation. Three timestamped local diagnostic trials passed without changing assertions; local iPhone 17/iOS 26.5 differs from hosted iPhone 16/iOS 26.2. This parent adoption does not claim to fix that unknown cause.
  • The first composition UI run passed one case and failed recipient entry while the native node remained starting. Saved hierarchy/recording show the network-startup screen; app logs show the VSS-backed builder had not completed and separate staging requests failed. After unauthenticated endpoint preflight, one unchanged comparison passed both cases and logged normal node startup. The initial underlying cause remains unproven; its failure evidence is retained, and no timeout/assertion was weakened.
  • The local-fork payment case was excluded because upstream Arbitrum no longer serves state at its pinned block. A separate healthy fixture with account/nonce preflight is needed; existing wallets and fixture state were preserved.
  • Manual Retry/Disconnect ownership, conditional cleanup and cross-device mutation safety remain the separately tracked work in Bind profile deletion cleanup to the original sign-in bitkit-android#1687 and follow-up fix: stop deletion after incomplete cleanup #1124. No native SDK upgrade or release is included.

Models used

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

Latest hosted failure investigation

Earlier-head 65774372 run 40166925539 failed only SendConfirmationLifecycleTests.testPendingWalletSwitchResumesAfterTags at the three-second destination-appearance assertion. All later wallet/preparation/navigation-path assertions and all 39 UI executions passed; one native-fixture unit and four UI fixtures were skipped. Complete logs and SHA-256/CRC-verified XCResults, including nested issue groups, were inspected. Both this run and the paired profile PR reproduce a keyboard queue timeout after the tag input is removed before keyboard presentation completes. The precise causal ordering and a safe repair remain unproven. A subsequent detached diagnostic checkout at d1745fb9, with trace/failure-capture additions and unchanged product behavior/assertions, passed 2,906 selected tests on a fresh iPhone 16 simulator using CI's exact iOS 26.2 build 23C54 and Xcode 16.4. The affected case completed in 0.737 seconds. Complete local logs and nested XCResult issues were inspected; four QoS observations remain. Local macOS 27 and hardware/load still differ from the hosted macOS 15 arm64 runner. This comparison does not repair the hosted failure or validate the current PR head. No deadline was weakened and no unchanged CI retry was requested. Existing local full-suite passes remain revision-scoped evidence, not a green hosted result.

Current diagnostic follow-up

Signed ca10ace7 adds failure-only XCTest attachments to capture the actual navigation state at the appearance deadline. The three-second deadline, polling interval, assertion and production code are unchanged. A temporary wrong-route expectation still produced both original assertion failures and retained two state snapshots, seven hierarchy records and seven screenshots; the temporary expectation was removed before final validation.

The final candidate passed all 2,906 selected native unit tests, zero failures/skips, on iPhone 16/iOS 26.2 23C54 with Xcode 16.4. Full SwiftFormat checked 847 files with zero violations. Complete 20,319-line logs and nested XCResult issues were inspected: two AppIntents notices and five runtime QoS observations remain, with their timing causes unproven. Both consecutive independent final reviews are clean. The exact six-commit current-base range is verified locally and on GitHub (verified=true, reason=valid). Current-head hosted SwiftFormat passed 0/847, with its complete log inspected; the checked merge tree equals the published head. Hosted tests passed: 2,906 unit executions and 39 UI executions passed, with one native-fixture unit and four UI skips. The complete 5,850-line log and SHA-256/CRC-verified XCResult were inspected, including nested issues. Its 571 duplicate-symbol log entries (SQLite and BLAKE3; 286 distinct structured warnings) match the actual parent run’s symbol multiset; package/project inputs are unchanged. No runtime warnings or failures were recorded in this hosted XCResult. The earlier timeout did not recur, so the new failure attachments were not triggered and its cause remains unproven. This is a diagnostic change, not a demonstrated repair of the hosted timeout; no unchanged CI retry or deadline relaxation was used. The Android twin does not share this XCTest harness.

@ovitrif

ovitrif commented Oct 24, 2026

Copy link
Copy Markdown
Collaborator

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

@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. Removes refreshSessionIfPossible and isSessionRefreshableError, and folds deleteProfileWithSessionRetry into EditProfileView.deleteProfile, so one failed performDeleteProfile shows the failure options instead of signing in again and repeating contact cleanup and deletion. Reviewed 1b8a6db, full tier, as the delta on fix/restore-identity-background (stacked on #1121). Not built here: iOS does not build on Linux, so this is reasoned from the code and CI.

What I checked, and 3 candidates I ruled out

Read in full: EditProfileView.deleteProfile and performDeleteProfile, PubkyProfileManager.performProfileDeletion and deleteProfile(cleanPrivatePaykitEndpoints:expectedSession:)
Call sites traced: refreshSessionIfPossible and isSessionRefreshableError have no reference left in Bitkit/ or BitkitTests/ (grep on the PR head); the three removed tests covered only those two functions
CI: SwiftFormat green; Run Tests still running when reviewed

Ruled out

  • The restructure changes which guard decides whether the failure options appear: an error from performDeleteProfile is still caught inside the operation and gated on publicKey == session.publicKey, as before; an error thrown by performProfileDeletion itself is still gated on currentSession == session. endProfileDeletion still runs on both paths because the inner catch does not rethrow.
  • An expired session now leaves deletion failing until normal recovery: deliberate and stated in Out of Scope, with Retry and Disconnect still offered.
  • Parity with the Android twin: both now make one deletion attempt per user action, and the journey file is identical on both platforms.

Merge confidence: 4/5, the EditProfileView restructure has no test of its own (J1 is unverified) and Run Tests had not finished.

@ben-kaufman
ben-kaufman changed the base branch from fix/restore-identity-background to fix/backup-observer-lifecycle October 25, 2026 01:41

@jvsena42 jvsena42 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Code scan: no findings.

Checked and clean:

  • With the replay gone, a failed deletion surfaces the failure dialog after one attempt. Local identity is cleared only inside signOut(), after the profile delete and the public-endpoint removal, so an interrupted deletion leaves the user signed in with a retryable state and no orphaned remote data from this path.
  • v3.0.0 holds no persisted pending-deletion state (PrivatePaykitService.swift:90 is an in-memory var), so there's nothing to migrate.
  • Nothing still references the removed refreshSessionIfPossible (instance and static) and isSessionRefreshableError, and startup session recovery stays in resolveSessionInitialization.
  • EditProfileView.deleteProfile() keeps its isDeleting guard and session checks, and performProfileDeletion still resets the deletion flags on every path, so Paykit automation resumes after a failure.
  • Twin synonymdev/bitkit-android#1688 does the same. #1124 edits the same lines and builds on this one.

Device gate (partial): ca10ace — not run yet. profile/delete-profile-uncertain-response.xml needs a homeserver fault proxy. I'll approximate it: airplane mode during the delete, expecting the failure dialog once, exactly one Failed to delete profile and no re-sign-in or second delete; Cancel keeps the profile; Retry online completes; Disconnect from the dialog. A plain successful deletion runs too. Device results will follow in a separate review.

@jvsena42 jvsena42 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Approving at ca10ace. My code scan stands with no findings, and the device gate found none.

Device gate: ca10ace — iPhone 17 Pro Max simulator, fresh wallet and profile on staging, run through a local homeserver fault proxy that drops the server's response after it applies the request. journeys: profile/delete-profile-uncertain-response.xml 6/6 passed. perf: not relevant; a plain deletion takes 12 s. Figma: n/a; l10n: no UI text changed.

  • After the lost acknowledgement, "Unable to Delete Profile" shows once. Over 70 s there's exactly one Failed to delete profile, one contact cleanup and one endpoint removal, with no sign-in and no second delete. Cancel and a relaunch run no deletion and keep the profile.
  • An online Retry right after fails once with SharedStateBusy, while the server-side write lock from the dropped request is still held. The next Retry deletes the profile and reaches onboarding.
  • Disconnect while the lock is held also fails, keeping the profile, because signOut() removes private endpoints first; that's unchanged here. Once the lock expires, Disconnect signs out in 10 s.

⚠️ Not verified: the proxy can't tell requests apart inside TLS, so the response it drops is the write-lock acquisition's, not the profile DELETE's. The app-side contract (one attempt, no replay) is the same on that path.

Not a finding: after a lost acknowledgement the write lock blocks both Retry and Disconnect for about 2 minutes. The follow-up is tracked in #1687.

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.

4 participants