Skip to content

fix: stop deletion after incomplete cleanup - #1690

Open
ben-kaufman wants to merge 24 commits into
fix/profile-delete-session-recoveryfrom
fix/profile-cleanup-outcomes
Open

ben-kaufman wants to merge 24 commits into
fix/profile-delete-session-recoveryfrom
fix/profile-cleanup-outcomes

Conversation

@ben-kaufman

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

Copy link
Copy Markdown
Contributor

November 12 · validated balance and cache-startup composition

Head 430588b9 composes 46d59014 with the parent Spark balance, payment-result and cache-startup corrections while preserving strict profile-cleanup outcomes. All 22 commits in the live-base range verify locally and on GitHub.

All five final local commands passed on tree d93cd7c9: formatting, compilation, 5,458 unit tests, lint and APK assembly, with no failures, skips or retries and zero findings in six native static reports. The first original CI, Lint and E2E runs passed on synthetic da7ca81a, whose tree matches the validated local composition. Hosted results: 5,458 units, 256 Compose tests, six zero-finding static reports and 39 enabled E2E cases passing on their first internal attempt, including Send and Receive. Settings08 remained disabled; all six migration jobs were skipped. Executed suite and selected reusable workflow were 0ca01e0a.

Complete diagnostics retain four stock restore wait guards followed by same-process success and four METADATA refusals followed by acknowledgements. Transfer UI passed despite the actual-base peer-fee disagreement, force-close and commitment broadcast; it does not prove a cooperative close or confirmed returned funds. Optional backup categories and no-session remote-cleanup warnings remain scoped to stock fixtures. Hosted unit XML and Gradle Problems HTML were not uploaded. The current run did not repeat the earlier Send/Receive failures or address-sync timeout signatures; all earlier failures, internal retries and setup failures remain retained at their original heads.

These results do not establish authenticated cleanup, native Reset Wallet, complete SDK WALLET or private-fixture acceptance. The parent's controlled cache-startup unit case is separate from installed startup timing. The inherited explicit deletion-retry limitation remains open; no broad session renewal was added. Existing authored QA notes and stable case IDs below are unchanged.

Current parent composition and validation — November 12

Signed 9a53355c8f23a502329efcf9c3f2c649dbb49c7d incorporates #1688 at 8d10218f6074517d7fae5019796a6e3f48a3eb1e. Required local format, compile, full unit tests, lint and debug APK assembly passed on the composed tree: 5,447 tests in 286 suites, no failures/errors/skips and zero introduced static findings. All 20 live-base commits verify locally and on GitHub. The original-owner cleanup, subscription refusal and acknowledgement guards remain intact.

The first original hosted CI, lint and E2E workflows passed. Synthetic checkout e24dc08ba9839cdbcd0eee03bba6c9064e1845b8 matches the validated tree. Complete diagnostics show 5,447 unit passes, all 57 scoped controls, 256 Compose passes and zero findings across six static reports. The owner verified all 405 sealed evidence files.

E2E recorded 39 distinct passes and one failed execution: the first send_2 attempt could not reach Spending activity while the Move To Spark sheet covered the wallet. Its built-in second attempt passed; two already-passed Send cases were filtered from that attempt. Settings08 remained intentionally disabled, six migration jobs skipped, and 44 helper recovery notices are retained. LNURL and Transfer both executed and passed on this head. No workflow rerun was requested.

Transfer still force-closed after the same 139/182-sat fee disagreement seen on the actual parent; UI success does not prove cooperative close or confirmed returned funds. NativeSegwit and NestedSegwit sync timeouts had later primary sync and generic wallet acknowledgements, without explicit completion of those account syncs. Three METADATA guard refusals later acknowledged in the same process. Existing compiler/JVM/cursor notices and the earlier revision's Send and emulator-setup failures below remain recorded.

These stock hosted and local component checks add no authenticated profile-cleanup, running-LDK quiescence, terminal complete-WALLET recovery or private SDK acceptance. Child #1693 is being validated separately. Existing QA IDs and authored notes remain unchanged.

Prior parent composition and validation — November 11

Signed 0bda04a6d3015ba868d7e4fecf86a0320e8d8f5d incorporates #1688 at 8a96806c29e05abe004c1c94ea163202040f66ac. The composed cleanup path preserves conservative refusal and checks subscriptions under the SDK lock before either profile-domain deletion. Identity-error responses do not refresh the session or replay deletion. Required local just format, just compile, full just test, just lint and debug APK assembly passed: 5,415 tests in 285 suites, no failures/errors/skips and zero static findings. Complete diagnostics were inspected. All 19 commits against the live parent verify locally and on GitHub; published history is preserved.

The first hosted CI and lint passed: 5,415 unit cases, 256 Compose cases and zero findings in all six static reports. Synthetic checkout 0f44fe10c31bf4c5a8a41ae7d4e49df02e824e19 has the same tree as this head. The owner verified all 436 sealed evidence files after the complete job, report and nested-runtime audit. Existing compiler/JVM/cursor notices, four METADATA refusals followed by same-process acknowledgements, and two tag-read cancellations remain recorded.

E2E remains failed. It recorded 36 distinct passes and three failed executions of send_2. Two attempts encountered the “YOUR CONNECTION IS CLOSING” sheet instead of the expected Spending activity; the other failed to reach Recipient after tapping Send, with the exact input target unresolved. The relevant app and harness sources match the tested parent, but the separate parent pass does not prove a cause or waive this failure. The LNURL/Transfer shard failed unpacking its emulator image before APK installation or tests, so neither case executed. Four repeat-marker entries and the intentionally disabled Settings case are skips; six migration jobs also skipped. No workflow rerun was requested.

Stock hosted suites and local component tests do not establish new authenticated profile-cleanup acceptance, running-LDK quiescence or terminal complete-WALLET recovery. Earlier authored cases and failures remain tied to their original revisions below. Child #1693's required public parent composition is being validated separately. Existing QA IDs and notes are unchanged.

Final deletion subscription guard — November 4

Signed 8ceaca255 fixes Piotr’s late-subscription finding. Final profile deletion now reads active subscriptions under the same existing SDK operation lock and handle as the revision-checked DELETE. An inbox receive admitted after preflight is observed before deletion; unreadable subscription state also refuses cleanup. The existing typed refusal and known-expiry message are preserved, with no subsequent sign-out or credential/settings erasure on refusal. This does not roll back contacts already removed earlier in cleanup.

The deterministic service regression queues a real locked receive before deletion and covers both payer and payee subscriptions. The no-contact repository case verifies error propagation and session preservation; the unreadable-state case now covers final deletion, and empty/expired subscriptions remain allowed. Negative control: 341 cases, exactly two expected failures on the prior implementation; corrected focused run: 341 passes. Final just format, just compile, full just test and just lint: 4,905 passes /264 suites, zero failures/errors/skips, eight fresh static reports with zero findings, 1,801 matching inputs. Complete logs, XML/HTML/SARIF and nested Gradle diagnostics were audited; prior cursor/JVM notices and verified base deprecation remain retained. Two independent affected reviews are clean, and all 17 live-parent commits verify locally and on GitHub. draft=false is preserved.

New-head CI, lint and E2E are complete and inspected on synthetic checkout 123345491ec4cc202b0a82060f659fede0c48440, whose tree equals this published head: 4,905 unit tests and 256 Compose cases passed; six static reports have zero findings. E2E recorded 39 distinct eventual passes: send_1 first timed out waiting 30 seconds for NavigationBack after amount-exceeded handling, then passed its configured second attempt. Its failure, screenshot, video and full logs remain preserved; the precise trigger is unresolved. Two previously passed send cases were filtered on retry, and settings_08 remains one genuine disabled skip. Migration jobs were skipped. All 1,044 sealed evidence files were hash-verified, including complete logs, structured reports and nested runtime diagnostics. Source equality and retry success do not establish that the failed attempt was harmless or pre-existing. The prior b2c4b9e9 Compose job failed before instrumentation because Android’s input service was unavailable, then reached its 45-minute limit; zero cases ran and the dependent gate failed. That complete 635-file failure audit remains preserved; no unchanged retry was made. These controlled JVM regressions are not a new full-app authenticated race execution. Child #1693 now contains this parent at signed c228adb7, preserving its already guarded original-owner snapshot path; its hosted audit is tracked separately. Existing manual acceptance and unrelated complete-WALLET limits below remain revision-scoped.

Evidence: work/validation/android1690-late-subscription-final-audit-20261104/report.md (1,305 verified files) and work/validation/android1690-late-subscription-20261104/publication.json.

Admitted-backup parent correction — November 4

Signed additive merge b2c4b9e9fd8c919515214976111ca62893c8bb8a incorporates dependency #1688 at a8c3fd3fc687df940d9e40037e0c52e244376325, including #1686's admitted-reservation correction. A replacement backup that never enters its original wallet can no longer strand Running after the preceding job drains. This closes the inherited cancellation ordering recorded for 97dbdffc below. Both incoming files match the validated parent; all 25 cleanup-specific paths remain unchanged.

Required final formatting, compilation, full unit tests and lint passed on 1,801 identical inputs: 4,902 unit tests /264 suites, zero failures/errors/skips, and eight fresh static reports with zero findings. Complete logs, XML/HTML case ledgers, SARIF and nested Gradle diagnostics were inspected. The existing cursor-close/JVM observations and historical Detekt deprecation match actual-parent evidence. The independent composition check reuses the exact inherited concurrency reviews. All 2,292 sealed evidence files match, and all 16 live-parent commits verify locally and on GitHub. The PR remains ready, with published history preserved.

New-head hosted CI is pending complete inspection. Prior 97dbdffc hosted results remain tied to that older tree; they are not credited to this composition. The unused authorization for the still older 84c803d8 Compose retry remains unused. Existing authenticated cleanup evidence and its limits are unchanged; no new cleanup journey, running-LDK quiescence or terminal complete-WALLET recovery is claimed.

Evidence: work/validation/android1690-admitted-parent-final-audit-20261104/report.md and work/validation/android1690-admitted-parent-20261104/publication.json.

Prior queued-backup parent composition — November 4

Signed additive merge 97dbdffc62a049077adf0ba0b595b4197e7be28e incorporates dependency #1688 at 5cf930fa70b89d0539ca3410006c252f3db608e5. All 25 child-specific paths retain their bytes; the five incoming parent files have no overlap. Required final formatting, compilation, 4,901 unit tests across 264 suites and lint passed with zero failures/errors/skips and zero static findings. All 1,801 frozen inputs match. Complete logs and structured diagnostics were inspected; all 2,286 sealed evidence files were verified. Four cursor-close chains, three JVM notices and the verified parent Detekt deprecation remain documented. All 15 live-parent range commits verify locally and on GitHub.

New-head hosted CI, lint and E2E passed on their first attempts: 4,901 units, 256 Compose cases and 39 E2E cases passed, with one explicit Settings skip and six skipped migration jobs. The actual synthetic checkout matches this exact published tree. Complete logs and structured diagnostics, ten checksum/CRC-verified report archives and 940 sealed evidence files were inspected and verified. Runtime warnings and one unmatched activity-tag cancellation observation remain documented; passing CI does not resolve the inherited source finding below. Evidence: work/validation/hosted-android1690-97db-20261104/. The earlier 84c803d8 Compose startup failure remains preserved below. Its one authorized retry was left unused because this required parent composition superseded that head; it was not rerun.

A further parent correction is required: review of #1686 identified a reachable cancellation ordering in which an unadmitted replacement backup can leave a stale Running flag after wallet retirement. The correction is being completed at the oldest affected parent before another child composition. This published tree's local/hosted results do not waive that known source finding. Existing cleanup acceptance remains tied to its original revision; no new authenticated cleanup or terminal complete-WALLET pass is claimed.

Evidence: work/validation/android1690-status-parent-final-audit-20261104/ and work/validation/android1690-status-parent-20261104/publication.json.

Prior VSS ownership composition — November 4

Signed additive merge 84c803d82aa37a6155764b04a98adf242d588f28 incorporates the live dependency #1688 at 916c6d5ef5c02805efb841b12f1e4bd1a9a527a8. All 25 child-specific files are byte-identical to the previous head; the 15 incoming parent paths have no overlap. The independent composition review confirms that conservative cleanup refusal, acknowledged-subset accounting and original-session behavior remain intact with the parent’s VSS admission and drain ordering. Published history is preserved.

Final just format, just compile, just test and just lint passed on the combined tree: 4,899 unit tests across 264 suites, zero failures/errors/skips and zero Detekt/Android Lint findings. All 1,800 frozen inputs match. Complete console, XML, HTML, SARIF and nested diagnostics were inspected, and the owner verified all 2,255 sealed evidence files. Four cursor-close chains and three JVM notices match actual-parent evidence; historical Gradle reports remain identified separately. All 14 live-parent range commits are locally and GitHub signature-verified.

New-head hosted build/unit and lint passed: 4,899 unit cases and zero static findings. All seven E2E shards passed their first internal attempt: 39 cases passed and one disabled Settings case was skipped. Migration jobs were skipped. The executed synthetic merge tree exactly matches this published head. Complete job logs, structured reports, nested E2E logs and ten checksum/CRC-verified archives were inspected; the owner verified 678 sealed evidence files. Wallet-operation timeouts remain documented nonfatal runtime observations without a pre-existing or causal-repair claim.

Hosted CI remains blocked: Compose job 126584925804 failed during emulator unlock with DeadSystemException, before the repository test script ran. Zero Compose cases executed and no result artifact exists. The original failed attempt is preserved; one scoped retry awaits authorization. A sibling’s identical configuration passed on a newer runner image, but that does not establish coverage for this head or prove the cause. Passing jobs have not been rerun.

This composition adds no authenticated runtime claim and does not establish actual running-LDK quiescence or terminal complete-WALLET availability. Existing manual/journey evidence and limitations remain below. Evidence: work/validation/android1690-vss-parent-final-audit-20261104/ and work/validation/hosted-android1690-84c8-20261104/.

Prior parent composition — November 3

Signed additive merge 2e79e1092d5b24aa6e90566c3eecbd7801f39fcf incorporates dependency #1688 at 3d8a328d6ea8e438ddd4059cca7e1de57d3a28b0, including current master 44bf50ec. All four cleanup production files, three tests, the authored journey and changelog retain their published bytes. The original cleanup-refusal text remains unchanged and unique in all 16 locales. The affected independent review verifies the incoming restore-category and allowance interactions without changing original-session authority, conservative subscription refusal or acknowledgement accounting. Published history is preserved.

Final just format, just compile, just test and just lint passed on the actual composed tree: 4,885 unit tests, zero failures/errors/skips and zero Detekt/Android Lint findings. All four phases share 1,798 frozen inputs. Complete logs and structured diagnostics were inspected; historical test-report copies from the earlier build phases are not counted as new execution. Cursor/JVM/Gradle observations remain retained in the audit. All 13 live-parent range commits are locally and GitHub signature-verified.

Current-head hosted validation is complete: 4,885 unit passes, 256 Compose passes, zero Detekt/Android Lint findings and 39 first-attempt E2E passes, with one explicitly disabled Settings case and six skipped migration jobs. All four workflows ran attempt 1; no whole-shard retry ran. The synthetic merge tree exactly equals the published tree against live parent 3d8a328d. Complete job/runtime logs and ten digest/CRC-verified report archives were inspected. Cursor/initialization warnings, routing failures, RN backup-list and restore-wait timeouts, METADATA safety refusals and npm advisories remain documented; passing stock suites do not establish authenticated cleanup or complete background recovery. The owner verified all 410 sealed evidence files. The authored acceptance below remains tied to its original 7040c899 APK and fixture; this composition supplies no new authenticated runtime claim. Evidence: work/validation/hosted-android1690-2e79-20261103/inspection/ and the local gate report in work/validation/android1690-master44bf-composition-20261103.

Author-run authenticated cleanup cases — November 2

The current head 7040c89936b354198d8aea363a54dc03fd0c1485 was exercised through the actual app on isolated disposable Android users, using native Pubky authorization/encryption and its committed Paykit RC2 dependency. Installed APK SHA-256: 6d4ba9df1b582237a9c6afa3b0312c15798f12f0967b8b62e865013a813ae33f. No funds, wallet resets, external reviewer fixtures, or product-source changes were used.

  • Private-withdrawal failure (Manual 1): one confirmation reached the selected encrypted-state write; 28 native attempts were refused before storage. The profile and grants remained unchanged, no profile DELETE occurred, and the linked peer still observed the same non-empty endpoint list. Failure UI appeared without success navigation. Back dismissed the dialog; Retry and Disconnect were not selected. The SDK's ephemeral records are pending-write markers, not lock tickets.
  • Profile DELETE 404 (Manual 2): the profile existed, but the scoped fixture returned 404 before storage. The app retained the session, sharing flags, profile and grant. After dismissing the dialog and cold-starting normally, no additional DELETE occurred during 86.7 seconds of observation. Credential-file bytes were equal across the failure, but changed on startup; byte identity across restart is not claimed.
  • Unknown contact-removal acknowledgement: the native contact transaction committed once (201), then the fixture replaced its acknowledgement with 503 and fenced 67 subsequent selected attempts. Exact RC2 source, the typed fresh-write error and the runtime call path bind this to contact removal. The app retained the cached contact, local override, credential bytes, profile and grant, with no profile DELETE. Cleanup-pending changed false→true. The encrypted remote contact result remains unknown: no confirmed removed-keys list, remote-contact equality, manual Retry, Disconnect or restart is claimed.

The sole complete audits inspected all app/server/journal/UI/snapshot records, and the owner verified the sealed results. Existing routing/RGS/receive warnings, pending-write markers and later reconciliation/backup failures remain recorded; this is not a warning-free runtime claim. Report SHA-256 prefixes: private 18384f24be137449 (plus active-control and marker-role supplements), DELETE404 6af49f8114589c28, unknown-contact f38e601fc408956a. Evidence is preserved in work/validation/android1690-{private,delete404,unknown-contact}-case-20261102/.

  • Acknowledged partial contact result: a single 60-second native lease held one linked contact while the real RC2 batch removed the other contact. Native contacts and local overrides each changed 2→1; the retained contact’s actual note and override hash stayed unchanged. The app showed failure, retained the profile, both grants, credentials and sharing settings, and sent no profile DELETE. The lease later expired without release/reclaim; no Retry or Disconnect was selected. This proves acknowledged-subset pruning, not complete deletion or cross-device CAS.
  • Fixed-term subscription admission: actual payer-only refusal and mixed payer/later-payee refusal both stopped before cleanup, with unchanged before/after server entries, events, grants and local overrides. The localized end time advanced from 10:36 to 10:39 as the later payee obligation was added. Both obligations are future-dated/unpaid and remain preserved; the mixed case is not isolated payee-only coverage.

The additional sole audits inspected 129 partial-case payloads and 159 subscription payloads, including complete diagnostics. The owner verified all 5 and 8 audit outputs respectively. Report SHA-256 prefixes: partial 4faa78ec6829268a; subscription refusals acede6354d1ddfea. Existing DHT/RGS/node warnings, setup corrections and observation limitations remain in the reports.

  • Open-ended payee refusal and subsequent removal: a separate future-dated, unpaid native Payee obligation with no end date blocked one app confirmation before cleanup. The localized refusal showed no date or generic Retry; both contacts, server entry/event/grant counts, sharing flags and credential hash remained unchanged. The original native fixture grant then cancelled that exact obligation once. A fresh app confirmation removed both contacts, received one profile DELETE/204, revoked the Bitkit grant and returned to profile setup. The companion grant remained; credential-file size changed from 1,378 to 296 bytes without reading its contents.

The final sole audit verified all 175 payloads, 85,756 app-log lines, 164 server-log lines and 138 journal records; the owner verified its seven sealed outputs (report SHA-256 a359537f1878c3ebc37df91a4aae7c6e3643124a92ef5caec3a3c142befa90ea). The initial incomplete handshake and two RecoveryRequired fixture calls remain recorded. Recovery completed the original native handshake and resumed the same request ID and exact terms under a one-use intent; it did not create a replacement proposal. An intermediate snapshot still showing the profile is retained as in-progress, before the single confirmed deletion completed.

Finite acceptance: J1 and Manual 1–3 are now covered by the separately audited current-head app cases above. Cancellation was performed through the disposable fixture's original native grant, as permitted by the journey; it does not establish Bitkit cancellation-UI coverage. No fixed-term wall-clock expiry, alternate Unlink route, malformed-expiry runtime injection, local-finalization fault, contact CAS or child #1693 ownership/recovery pass is claimed. Fixed-term expiry guidance and conservative malformed-term source tests remain separate from those unrun runtime variants. Existing endpoint-republication and fixed-term escape-path review threads remain open for the separately implemented #1693 behavior. External reports below remain attributed; these author-run cases do not authenticate the external artifacts.

Evidence: work/validation/android1690-open-ended-case-20261102/, its android1690-open-ended-case-audit-20261102/ sole audit, and the combined acceptance receipts in work/validation/android1690-authored-runtime-20261102/.

Authenticated acceptance evidence reconciliation — November 2

Review 5732044693 reports an actual 7040c899 app run: native write-ahead PUT201, shared-state503, Cancel preserving contacts/overrides/session/sharing, then normal cold startup restoring the captured owner with successful reads and no profile DELETE during more than 90 seconds of observation. It separately retains the 7ef3432 executions: payer/payee latest-expiry refusal, open-ended refusal, removal after obligations end, private-withdrawal failure, unconfirmed/partial contact cleanup, DELETE404 refusal and local-finalization failure.

These are attributed external executions at their stated revisions. The owner read both complete reports and compared 20 selected cleanup, UI, storage and build/dependency inputs: 17 are byte-identical. The three changed files contain startup-profile retry admission, allowance SDK admission and its setting; the inspected cleanup functions and native pins remain unchanged. The repeated current-head startup case is distinct from retained old-head cases. This source comparison does not verify the external installed binary or fault controls.

The remaining acceptance verification is the existing control patch, revision/build/install/native-hash receipt and per-case app/proxy/readback records, requested once in 6569526757. Their video src attributes contain a local-path prefix; usable attachment URLs were requested with the records. No videos or external wallets were accessed. The external reports alone did not check the QA boxes; the separately audited author-run cases above now satisfy the finite J1 and Manual 1–3 acceptance independently. These reported executions supersede older blanket statements that the cases had never run. Component/stock CI does not supply those missing artifacts, and the child #1693 recovery/unknown-outcome requirements remain separate.

Evidence: work/validation/android1690-retained-acceptance-reconciliation-20261102/. Head/base and ready status are unchanged; existing reviewers are preserved.

Current composed head and hosted results — November 2

Signed additive head 7040c89936b354198d8aea363a54dc03fd0c1485 incorporates #1688/2efaff0431dcf12b6d0850ee49c9cca5605b8d4e; tree cd10145486b8fbc425c859b50c7c21a2fc3e33ad. Required final compile, formatting, 4,837 tests and all six static reports pass with zero findings. The affected composition review is clean, and all 12 live-base commits verify locally and on GitHub.

All current-head hosted results are inspected: 4,837 unit passes, six static reports with zero findings, and 39 distinct eventual E2E passes. One lightning_1 execution failed because LND reported its invoice expired on October 22; the automatic second attempt passed. The invoice's origin and clock cause remain unproven. One disabled Settings case and one cached-pass skip record are distinct; six migration jobs did not execute.

The exact synthetic tree matches the published tree, with suite 611509ee. Complete logs, all nested streams, nine checksum/CRC-verified archives and failure media were inspected; 726 sealed payloads were verified by the owner. Four CloseGuard observations and other runtime diagnostics remain retained. No CI rerun, new authenticated cleanup pass, or installed-binary claim is implied. Evidence: work/validation/hosted-android1690-7040-20261102-current-a/REPORT.md.

Hosted validation completed — November 1

At 120553fd967b1b77f179d2c196be87bd4718a2fd, all seven E2E shards and the status job have completed: 39 first-attempt passes, with one disabled settings case. The earlier audited 4,836 unit tests and six zero-finding static reports remain tied to this exact head and parent 2621f81a.

The sole audit combined prior immutable evidence with only the missing complete shard logs and checksum/CRC-verified report archives. Runtime diagnostics, npm advisories and skipped migration jobs remain explicit. No CI rerun or new authenticated cleanup acceptance is claimed. Evidence: work/validation/hosted-android1690-1205-20261101-tail-b/REPORT.md.

Validated parent composition — November 1, 22:00 UTC

Signed additive merge 120553fd967b1b77f179d2c196be87bd4718a2fd incorporates #1688/2621f81afc9a671f49a79221b013666732b9b6a4. The owned cleanup patch is unchanged. The composed tree passed required compilation, formatting, 4,836 tests across 260 suites, and all six static reports with zero findings. All 1,783 frozen inputs matched. Complete diagnostics were inspected, including four retained CloseGuard stderr observations; no introduced compiler finding was identified. All 11 commits in the live-base range verify locally and on GitHub.

Current-head hosted checks are being inspected. This composition does not establish a new authenticated cleanup journey or close terminal complete-WALLET availability. Existing QA IDs, evidence limitations and unresolved acceptance below remain applicable.

Current parent composition — November 1

Signed additive merge 9bf19f194a291da749f8d8e48273ee30123e66e9 incorporates live #1688/3f4b19f7f58b8bff053f7e265ce50f0079ece197; tree ecdc690fb603281b4db818414ccd5e2690a5e38b. The owned cleanup/credential/cancellation delta is unchanged. The composition includes the parent queued-offer admission and same-active-identity correction; one affected integration review is clean, with independent shared reviews retained.

Required formatting, compile, full tests and lint passed 4,829 tests /260 suites, zero failures/errors/skips or retry nodes. All 1,782 frozen inputs match. Complete logs, nested output, fresh Detekt/Android Lint SARIF/XML and HTML were inspected: zero findings/issues/notifications. Three JVM notices and four verified unchanged-provider cursor warnings are retained; stale copied reports are excluded. All ten live-base commits verify locally and on GitHub. New-head hosted CI is pending inspection.

No new authenticated app journey or cross-device deletion transaction is claimed. Existing finite cleanup evidence, uncertain-mutation limits and the terminal complete-WALLET availability finding remain distinct. Evidence: work/validation/android1690-parent3f4b-final-a/inspection/ and work/validation/android1690-parent3f4b-20261101/publication/.

Historical parent composition — October 30, 22:49 UTC

Signed additive merge 7ef3432087568f16d34a983d90034ae4871af404 incorporates #1688 at 3001faf36a6e74e04e8cc1aac80c4bc8a7f4e714. The child patch is byte-identical. Final tree 5e23c7580c26d1418c3ed7379ec5dfceab7f07a3 passed required format, compile, 4,819 unit tests / 260 suites, and full Detekt/Android Lint with zero failures/errors/skips or findings. Production compilation and all lint analyses executed. Complete logs and structured reports were inspected; all 1,773 frozen regular inputs and six symlinks match. Known provider cursor/JVM warnings and historical Gradle deprecation records remain retained and attributed.

The independent affected-composition review remains valid: its only subsequent input difference is the separately reviewed upstream test-diagnostic change. All eight live-parent range commits verify locally and on GitHub (verified=true, reason=valid). This PR remains ready. Current-head CI passed 4,819 units, and lint has zero findings. E2E passed 39 cases on all seven first in-job attempts, with one explicit settings skip and no failed attempt. Synthetic e5335ab2 equals the published tree against exact parent 3001faf3. Complete available job/runtime logs and all uploaded unit/lint reports were inspected; nine archives match their digests and ZIP CRCs. Hosted JUnit XML and Gradle problems HTML were not uploaded. Migration skipped; stock E2E does not satisfy the authored authenticated cleanup/recovery cases. The inherited manifest failure is not causally explained by the passing local run. Earlier fixed-term/device evidence stays revision-scoped; remaining safe-recovery and authenticated fault cases stay assigned to #1693/#1687 without being marked passed here.

Twin: synonymdev/bitkit-ios#1124
Dependency: #1688

This PR stops profile deletion after incomplete cleanup and checks active subscriptions before withdrawing payment links or removing contacts.

Description

  • Stops profile deletion when private payment-link withdrawal or contact cleanup is incomplete.
  • Checks active payer and payee subscriptions before cleanup and explains that they must end before the profile can be removed. A profile-only contact batch checks again before removing any selected contact; ordinary bulk contact removal keeps its existing policy.
  • Propagates contact read errors and preserves unknown or retained contact data for reconciliation.
  • Propagates profile-deletion errors instead of interpreting 404/missing text as successful deletion and signing out.

Out of Scope

  • Original-session/attempt ownership, failure-dialog Retry/Disconnect binding and restart reconciliation remain tracked in Bind profile deletion cleanup to the original sign-in #1687 and the paired iOS investigation. This PR does not make a manual destructive retry safe.
  • Native contact removal still has no expected-record precondition for cross-device edits/re-adds/unblocks. No such atomicity is claimed.
  • Full-WALLET separation, SDK artifact adoption and the historical contact/acquisition incident cause remain separate.

Design

Existing Profile › Profile Edit in the Bitkit handoff. This changes failure handling; it adds no visual design.

Preview

N/A — existing error UI is reused.

QA Notes

Setup

Check out current 7040c89936b354198d8aea363a54dc03fd0c1485 and use its committed dependency catalog and normal devDebug build. Component tests use synthetic identities and injected service failures; no real profile, wallet or funds are needed. For J1, provision a disposable authenticated identity and a homeserver fault proxy that can hold/fail private endpoint withdrawal, lose a contact-removal acknowledgement, and return a partial acknowledgement for only some contacts, or fail profile deletion with a 404-like error without confirming the outcome. Those controls are unavailable in the standard journey fixture. For manual 3, also create and accept a future-dated recurring request with a disposable counterpart, test both payer and payee roles, and retain an unrelated saved contact. Use an open-ended disposable subscription for the cancellation case. For an accepted fixed-term subscription, preserve the existing no-cancellation policy, verify its expiry guidance, and wait for eligibility before confirming remote removal again; do not use live funds. The author-run cases tested that guidance and used the separate open-ended fixture for cancellation/subsequent removal; no natural fixed-term-expiry execution is claimed. The committed journeys/profile/profile-cleanup-failure.xml describes the sequence; do not use a personal profile or reset another worker's wallet.

Journeys

  • J1: new profile-cleanup-failure.xml — current-head actual app cases cover private failure, unknown/partial contact outcomes, subscription admission, fixture cancellation then fresh removal, DELETE404, Cancel and cold restart. Cases and limits are individually bound above; this is not a single uninterrupted run or child fix: bind profile removal to its original owner #1693 acceptance.

Manual Tests

  • 1. Enable the J1 cleanup fault → Edit Profile → confirm deletion → failure UI appears without profile deletion or success navigation — current-head author-run private-withdrawal failure and acknowledged partial-contact result are audited above.

  • 2. Inject a profile-deletion error mentioning 404 → confirm deletion → failure appears with the session and sharing settings retained — current-head author-run scoped DELETE404 case and normal cold-start observation audited above.

  • 3. With an active recurring subscription (payer and payee cases), confirm Delete Profile or Unlink → the localized active-subscription refusal appears before endpoint/contact/profile cleanup, including for an unrelated contact. For an open-ended subscription, cancel it and confirm again → normal acknowledged removal proceeds. For fixed-term subscriptions, cancellation remains unavailable: the error explains that removal is blocked and shows the latest end time only when every blocking record has a valid finite expiry; unreadable or missing expiry remains a blocking unknown. Current-head author-run payer-only and mixed later-payee finite refusals, isolated open-ended payee refusal and original-fixture cancellation followed by fresh app removal are audited above. Actual Bitkit cancellation UI, natural fixed-term expiry and malformed-expiry runtime injection are not credited.

Automated Checks

  • updated PubkyRepoTest.kt — partial/unknown contact removal and unavailable reads stop before profile deletion/sign-out and preserve remaining data; direct preflight and late subscription refusals survive service-error wrapping.
  • updated PaykitSdkServiceTest.kt — payer/payee and conservative expiry checks, unavailable reads, complete profile-batch refusal and later eligibility after a subscription ends.
  • updated EditProfileViewModelTest.kt — private cleanup failure stops the flow; local finalization failure emits no success and does not open a deletion Retry dialog.
  • removed the obsolete success-after-private-cleanup-failure expectation, replaced by the stop-before-delete regression.

Evidence

Controlled baseline: 221 repository tests included two expected failures for partial/unknown contact cleanup; 19 view-model tests included two expected failures for ignored cleanup outcomes. Final source passed just compile, just test, just lint and just format: 4,627 tests across 254 suites, zero failures/errors/skips. Complete reports contain zero Detekt/Android Lint findings. JVM sharing notices and the structured Gradle deprecation match recorded baseline diagnostics. Two consecutive independent final reviews found no outstanding issues in this scoped change.

Acknowledged contact removals clear only their requested returned keys. Failed private local finalization reports an error without a deletion Retry dialog; Disconnect also consumes that result. The broader ownership/manual-retry limitation above remains open.

The follow-up at 2daec90e2 removes broad profile-deletion error suppression. A controlled pre-fix repository suite executed 223 tests with one expected failure: a 404-like deletion error returned success and signed out. The repaired focused suite passed all 223 cases. Final validation passed compile, 4,628 tests across 254 suites with zero failures/errors/skips, full formatting, Detekt and Android Lint with zero findings. Complete diagnostics were inspected. Two consecutive independent final reviews are clean. The SDK already represents confirmed profile absence as normal completion; arbitrary thrown errors no longer authorize sign-out. Earlier acknowledged contact/endpoint removals are not rolled back.

The previous 479c53065 hosted head passed 4,627 units and all lint reports (artifact SHA-256/ZIP CRC verified, three SARIF/XML reports with zero findings). Its E2E workflow eventually passed 39 cases, but LNURL and Transfer initially timed out waiting for expected UI and passed the workflow's automatic retry. Full logs were inspected; the initial causes remain unproven. No extra rerun was requested.

Gaps

At that earlier checkpoint, author-run J1 and manual 1–2 were unrun because the authenticated fault proxy was not provisioned. The later external executions and their current artifact-verification limits are recorded above. Component tests do not establish cross-device CAS or historical incident causation. The earlier three-commit range at 2daec90e2 was verified locally and on GitHub (verified=true, reason=valid). The earlier second commit only renamed unchanged changelog bytes to this PR number. At historical head 2daec90e2, hosted CI run 40165960501 passed 4,628 units. Lint run 40165960499 passed; complete logs and artifact 12587788708 were inspected with matching SHA-256/ZIP CRC and zero findings across three SARIF/XML reports. The manifest warning matches the actual base and previous head. That historical head's E2E run 40165960710 passed 39 cases across seven shards on their first attempts. The complete 6,672-line log was inspected. This is the existing E2E suite, not the new authenticated cleanup fault journey.

At prior composed head f72e0c03b76352a7c6db8e332b4dec7ca52b91ae, an additive signed merge incorporates parent #1688 at 5fe0bf6c4f0851035632abd87dd5fb14b43507d0. All 238 cleanup feature line changes remain unchanged. The exact combined tree 83ca499c244f6c5049e2a957548d7f44ed44d842 passed final just format, just compile, unfiltered just test (4,610 tests / 254 suites, zero failures/errors/skips) and just lint, with zero Detekt/Android Lint findings. The sole complete diagnostic audit inspected every log and XML/SARIF/HTML report; all 1,757 frozen entries and 2,859 evidence hashes match. Three JVM notices, the manifest notice, four complete cursor-allocation stacks and the Detekt deprecation recur on the exact validated parent tree; they are retained rather than called warning-free. An independent affected-composition review is clean, with prior unchanged feature reviews retained. All four commits against the live parent are locally verified and GitHub verified=true, reason=valid. At that revision, hosted CI passed 4,610 unit tests in 254 classes with zero failures/skips, and lint has zero Detekt/Android Lint findings. E2E reached 39 distinct passes and one explicit settings skip: send_1 first failed its 30-second NavigationBack assertion, then passed automatic attempt 2. Earlier failures and process annotations are retained; their causes remain unproven. This is not an all-first-attempt pass or the authored authenticated fault journey. The complete 14 executed job logs, all unit/lint reports and four SHA-256/CRC-verified archives were inspected; the executed merge tree equals this head and its tested parent matches the live stack. Failure-only media was not uploaded after eventual success. No workflow rerun was requested. Migration was skipped, not passed. At that checkpoint, author-run J1 and manual 1–2 remained unexecuted; the later external evidence and remaining artifact checks are recorded above.

Active-subscription follow-up at 7afdd3e3ba88ab5dded792073a44b4a975728ebb

The actual Edit Profile deletion path now checks raw SDK recurring obligations before private pause or cleanup. Active payer and payee records, including obligations without a saved contact, block removal; missing or unreadable end dates fail closed. The existing typed reason is normalized through service wrappers and shown with localized guidance in all 16 resource locales, without the generic Retry dialog. The repository also protects direct deletion callers. A dedicated profile contact batch rechecks subscriptions before native batch removal, preventing unrelated contacts from being removed on a known refusal. If a subscription appears after early preflight, this later refusal does not roll back already acknowledged endpoint withdrawal; no cross-device transaction claim is made.

A controlled old-path regression failed because deletion never invoked the preflight. The final combined tree passed required compile, 4,617 fresh unit tests / 254 suites with zero failures/errors/skips, full lint and final unchanged formatting. Complete diagnostics and all structured reports were inspected; Detekt and Android Lint have zero findings. All 1,756 frozen inputs matched. Earlier candidate compilation, sandbox-launch and formatter failures remain retained and are not behavioral pass evidence. Two independent source reviews found no outstanding issue in this scoped repair. Five commits against live parent 5fe0bf6c4f0851035632abd87dd5fb14b43507d0 verify locally and report GitHub verified=true/reason=valid. Current-head CI passed 4,617 tests and lint has zero findings; complete logs and checksum/CRC-verified report archives were inspected. E2E finished with 39 distinct eventual passes and one explicit settings skip. Its first workflow attempt failed Docker setup on occupied port 43782 before multi-address tests began. One exact-job infrastructure retry was requested after diagnosis. That retry reached the tests, where multi_address_3 first timed out after 30 seconds waiting for “Sent” after SendSuccess/Close; its automatic second attempt passed, while cases 1/4 were marker-skipped after their earlier actual passes. Both failures remain preserved; no further rerun was requested. All 13 actual job executions and 7,617 lines were inspected without double-counting GitHub's recreated historical job IDs. The retry used the same harness and original APK digest. Twelve direct send/activity/data-source/pin files match the actual base, but shared identity dependencies changed; no base runtime equivalence or cause is claimed. Failure media were not uploaded after eventual success. Migration was skipped. These results do not satisfy authenticated cleanup J1.

These checks use mocked native/network resources. J1 and manual 1–3 still need the authenticated fault/subscription fixture; no new full-app journey or historical incident-cause claim is made. Child #1693 incorporates this repair at signed e1cc3b1717b2a4ca5365b41ff4acb954d129d1d6, preserving its original-owner/attempt orchestration. Its exact-head validation and remaining remote-Disconnect finding are documented there.

Fixed-term guidance and current parent at 815e85c0b2990a701999828ff2a691aff4646ff6

The typed profile-removal refusal now distinguishes aggregate subscription eligibility from ordinary contact cancellation. When all blocking records have valid finite end dates, the existing localized date is shown for the latest expiry. Missing or unreadable dates remain blocking without inventing an end time. Fixed-term cancellation policy, the all-record guard and the prohibition on cleanup after refusal are unchanged. Fresh local-only Disconnect is being integrated in child #1693; this parent does not claim that escape path is complete.

A signed additive merge includes current parent #1688 at 2aac53edc1faf2b4fd509b7e8a36701ce4289231. Exact tree ed07a3f07b6765cb73246daf245ffea613e877aa passed final format, compile, 4,721 tests / 256 suites with zero failures/errors/skips, Detekt and Android Lint with zero findings. Complete 9,834-line logs and structured reports were inspected; all 1,763 frozen files and six symlinks matched. Three JVM notices, existing cursor stacks, Detekt configuration notices and cache diagnostics are retained with actual-base comparisons. Independent feature and affected-composition reviews are clean. All seven live-base commits verify locally and on GitHub. Current-head hosted CI passes 4,721 unit tests, zero failures/skips, and lint reports zero findings. E2E passes 39 cases on their first in-job attempts, with one settings suite skip. Complete 14 logs, 17 notice annotations and four checksum/CRC-verified artifacts were inspected; synthetic a759400c has the exact published tree against 2aac53ed. Existing JVM/cursor/manifest and generic Gradle notices remain documented; missing hosted Gradle problem details are not claimed inspected. Migration skipped. Stock E2E does not execute the new fixed-term refusal/local Disconnect journey. Prior hosted results above remain revision-scoped. That checkpoint did not include author-run authenticated J1/manual 1–3. Later external runs and the remaining artifact-verification gaps are recorded above.

Models used

  • Planning/scoping: gpt-6-astra (reasoning: xhigh)
  • Implementation: gpt-6-astra (reasoning: xhigh)
  • Review: gpt-6-astra (reasoning: xhigh), gpt-6.1-sol (reasoning: high)

Validated parent composition (2026-11-02): signed additive 7040c89936b354198d8aea363a54dc03fd0c1485 incorporates live #1688 2efaff0431dcf12b6d0850ee49c9cca5605b8d4e. The complete PR delta is unchanged. Final composed tree cd10145486b8fbc425c859b50c7c21a2fc3e33ad passed all 4,837 unit tests across 260 suites, required compile, formatting and lint; all six fresh static reports contain zero findings. Complete logs and 649 sealed evidence payloads were inspected, with runtime CloseGuard/JDK notices and historical failures retained. All 1,782 frozen file hashes plus the directory symlink match. All 12 live-base commits are locally and GitHub verified. This is combined native/component validation; existing authenticated journey limits remain as documented. Current-head hosted results are fully inspected in the current-results section above.

@greptile-apps

greptile-apps Bot commented Oct 25, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerView in GreptileConfidence Score: 5/5

[Medium impact] This PR appears safe to merge within its stated cleanup scope.

Summary

This PR stops profile deletion when private payment-link withdrawal or contact removal is incomplete.

  • Profile deletion stops when payment-link or contact cleanup is incomplete.
  • Profile removal waits for local Paykit cleanup before reporting success.

No actionable issues were found. Tests were inspected but not run.

ben-kaufman explicitly deferred session ownership, manual-retry binding, restart reconciliation, and cross-device conditional removal. This assessment does not claim those limitations are resolved.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Confirm profile deletion] --> B[Withdraw private payment endpoints]
    B -->|Failure| F[Show deletion failure options]
    B -->|Success| C[Read and remove contacts]
    C -->|Read error or uncertain result| F
    C -->|Partial removal| D[Keep unacknowledged contact data]
    D --> F
    C -->|Complete| E[Delete profile and sign out]
    E -->|Failure| F
    E -->|Success| G[Clear local private payment state]
    G -->|Failure| H[Show error without success or deletion retry]
    G -->|Success| I[Report success and leave profile]
Loading

Reviews (1) · Last reviewed commit: "fix: stop profile deletion after incompl..." · Reviewed by Greptile

@ovitrif ovitrif added this to the 3.1.0 milestone Oct 25, 2026
@ovitrif

ovitrif commented Oct 25, 2026

Copy link
Copy Markdown
Collaborator

Triage: 3.1.0. It fixes shipped profile deletion, and 3.1.0 is due 28 Oct.

@github-actions

github-actions Bot commented Oct 25, 2026 •

Copy link
Copy Markdown
Contributor

Regtest APK

Built from 274bb68 (run).

Download bitkit-dev-debug universal APK (expires in 30 days).

@talosmachina talosmachina left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No findings. Profile deletion now stops when private endpoint withdrawal, the contact read or the contact removal is incomplete, keeps the contacts and overrides that were not acknowledged as removed, and no longer reports success when local finalization fails. Reviewed 479c530, full tier, stacked on #1688 (fix/profile-delete-session-recovery), so the delta against that base is what I read.

What I checked, and 6 candidates I ruled out

Read in full: PubkyRepo.deleteProfile / deleteAllContacts / signOut / clearAuthenticatedState, EditProfileViewModel.attemptDeleteProfile / disconnectProfile, PrivatePaykitRepo.removePublishedEndpointsForCleanup / removePublishedEndpointsLocked / closeAndClear, PaykitSdkService.contactRecords / removeContacts, and remove_contacts_and_block_peers / contact_records in paykit-rs at v0.1.1-rc1 (the pinned paykit version).
Call sites traced: deleteAllContacts (only deleteProfile), removePublishedEndpointsForCleanup(..., deferAppSync = true) (only the delete flow), closeAndClear (delete and disconnect).
CI: build-local green; build, lint and the e2e matrix still running when this was posted.

Ruled out

  • A missing-storage read now blocks deletion: dropping the isMissingPubkyData() fallback is safe because contact_records is a local SDK storage transaction; it fails only on an uninitialized identity or a storage error, neither of which is evidence of an empty contact list.
  • Overrides of local-only contacts survive a full success: the old code cleared them to emptyMap(), the new code subtracts only removedKeys; on success signOut() reaches clearAuthenticatedState, whose pubkyStore.reset() drops them anyway.
  • Key format mismatch between records, overrides and _contacts: all three are compared after ensurePubkyPrefix(), and override keys are written prefixed (PubkyRepo.kt:2054); the new partial-removal test exercises the subtraction.
  • Contacts the SDK retains by design (active recurring request, peer lease, published marker) now make deletion fail on every retry: this is the stated intent of the PR and its journey (partial removal preserves contacts and prevents deletion), not an accident.
  • User left on Edit Profile after closeAndClear fails post-deletion: deliberate per the new failed local finalization does not report deletion success test; the failure dialog is not shown because the remote deletion did happen.
  • Endpoint withdrawal failure leaves the deletion flag set: endProfileDeletion() stays in the finally under NonCancellable, and removePublishedEndpointsForCleanup marks contactSharingCleanupPending on failure.

Not built here: on the Bitkit track an auto review does not run a PR branch's Gradle build, so this is reasoned from code and CI.

Merge confidence: 4/5, no findings, but the change is reasoned from code rather than built here, and build, lint and e2e had not finished.

Reviewer model: claude-opus-5-5
Reasoning effort: Unknown (not exposed to the reviewing session)

@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: one LOW inline, reproduced with a failing unit test.

Checked and clean:

  • When cleanup is incomplete, the user gets the existing Retry / Disconnect / Cancel dialog.
  • Retry is idempotent after a late signOut failure, because fetch_paykit_profile returns None for an absent profile, so dropping the 404 tolerance is safe.
  • No path locks the user out of deletion or sign-in for good.
  • Private endpoints are now withdrawn before the profile delete, which closes a gap on master.
  • With #1688 removing the automatic replay, each stop is final until a manual Retry; the two don't conflict.
  • profile-cleanup-failure.xml's fault point is right: contact removal goes through the shared-state storage adapter (PubkyShared, put_locked), so a homeserver proxy can fail it.
  • Twin synonymdev/bitkit-ios#1124 clears the cleanup flag on a successful withdrawal, so it doesn't have the LOW.

Device gate (partial): 2daec90 — not run yet. profile-cleanup-failure.xml needs a homeserver fault proxy. The partial-removal stop can be driven without one, using a contact with an active recurring payment request on two devices: deletion stops, the dialog shows, and after Cancel the contact can or can't pay privately. Device results will follow in a separate review.

@ovi-reviewer ovi-reviewer Bot 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.

Verdict: ✅ Approve

Findings: I found no additional defects. The existing publication-pause finding remains acknowledged at this head; the author's follow-up tracks its recovery separately.

QA for 2daec90: Android 16 / API 36, Core 0.8.0-rc11 and Paykit 0.1.1-rc1. Device cases used this head plus the test fixture below, with disposable identities and an unfunded regtest wallet. The real Edit Profile screen, ViewModel, repositories and native SDK handled deletion; a homeserver proxy supplied the faults. The installed APK hash matched the built fixture APK.

Test 1: Manual 1 / J1 — private cleanup fails ✅ Failure stops before contact or profile deletion.

The proxy returned 503 for the native shared-state read during private cleanup. After Cancel, the refreshed inspector confirmed the session, both contacts and overrides, and both sharing settings remained. Call counters and the proxy trace confirmed no contact or profile deletion.

1a.2x.mp4

Private cleanup failure and retained state, at 2× speed.

Test 2: Manual 1 / J1 — unconfirmed contact removal ✅ Local evidence survives without entering profile deletion.

The proxy accepted the native write-ahead record, then rejected shared-state publication with 503. The failure dialog appeared. The post-failure inspector and call trace confirmed both contacts and overrides remained, with no profile deletion or success feedback.

Failure flow Refreshed post-failure inspector
1b.2x.mp4

Unconfirmed cleanup at 2× speed; the still shows the refreshed counters after Cancel.

Test 3: Manual 1 / J1 — partial native removal ✅ Only the acknowledged contact and override disappear.

The SDK retained a contact with a published public marker and acknowledged the other removal. The inspector showed one remaining contact and override; the persisted override key matched the retained marker. Failure appeared without entering profile deletion. The marker was provisioned through the native SDK.

1c.2x.mp4

Partial removal and retained evidence, at 2× speed.

Test 4: Manual 2 — profile deletion returns 404 ✅ Failure preserves the session and sharing settings.

The proxy trace confirmed profile GET 200 followed by DELETE 404. The app showed failure and retained its session and both sharing settings, with no success feedback. Earlier acknowledged contact removals remained applied, as the refreshed inspector shows.

2.2x.mp4

Rejected profile deletion and retained session, at 2× speed.

Test 5: J1 — Cancel and cold restart ✅ Normal startup restores the same identity without replaying profile deletion.

After unconfirmed contact cleanup and Cancel, a cold launch through the normal app entry restored the same authenticated identity on Home and Profile. The profile QR identifier and restoration log confirmed the owner. Proxy sampling covered over 85 seconds, with successful profile reads and no profile DELETE.

restart.2x.mp4

Normal cold startup and restored profile, at 2× speed.

Check 1: unmodified-head build and regression checks ✅ The app build and affected suites pass.

assembleDevDebug and testDevDebugUnitTest with PubkyRepoTest and EditProfileViewModelTest passed on the unmodified head. git diff --check passed. These focused checks include unavailable contact reads, uncertain/partial removals, 404-like profile errors, private cleanup failure and failed local finalization.

Full review of 99b446a..2daec90, all six changed files and reached cleanup paths. Contact-read errors and local-finalization storage failures had source/unit coverage rather than separate device cases. Active-recurring and peer-lease retention variants were not separately driven. Manual Retry/Disconnect ownership and native cross-device conditional removal remain outside this PR's scope.

Twin: bitkit-ios#1124 fix: stop deletion after incomplete cleanup matches the scoped cleanup/error propagation behavior at a062618; its authenticated device report supplies the iOS evidence.

Test fixture used by Tests 1–5

Not pushed. Provisions disposable native identities and contacts, displays call/state evidence, and renders the actual Edit Profile screen. Public-contact publication is enabled to stage the retained-marker variant. SDK calls return native results. Test 5 launches the normal app entry.

diff --git a/app/src/debug/AndroidManifest.xml b/app/src/debug/AndroidManifest.xml
index b88fe73a7..304e984d2 100644
--- a/app/src/debug/AndroidManifest.xml
+++ b/app/src/debug/AndroidManifest.xml
@@ -6,6 +6,8 @@
         android:usesCleartextTraffic="true"
         tools:ignore="MissingApplicationIcon">
 
+        <activity android:name="to.bitkit.dev.CleanupProbeActivity" android:exported="true" /> <!-- test hook -->
+
         <provider
             android:name="to.bitkit.dev.DevToolsProvider"
             android:authorities="${applicationId}.devtools"
diff --git a/app/src/main/java/to/bitkit/services/PaykitSdkService.kt b/app/src/main/java/to/bitkit/services/PaykitSdkService.kt
index 87c452f64..011d57041 100644
--- a/app/src/main/java/to/bitkit/services/PaykitSdkService.kt
+++ b/app/src/main/java/to/bitkit/services/PaykitSdkService.kt
@@ -666,6 +666,11 @@ class PaykitSdkService @Inject constructor(
     suspend fun fetchPubkyFollows(publicKey: String): List<String> =
         publicRead { it.fetchPubkyFollows(publicKey, maxEntries = 10_000u) }
 
+    internal suspend fun reviewPublishContact(key: String) { // test hook: provision real native retained marker
+        isSetup.await() // test hook
+        operationLock.withLock { withStateRevisionTracking { handle -> completeSdkCall { handle.publishPublicContact(key) } } } // test hook
+    } // test hook
+
     suspend fun contactRecords(): List<ContactRecord> {
         isSetup.await()
         return operationLock.withLock {
@@ -1952,7 +1957,7 @@ internal object BitkitPaykitSdkConfig {
 }
 
 internal fun paykitSdkConfig() = defaultConfig("bitkit").copy(
-    publicContactSharing = BitkitPaykitSdkConfig.publicContactSharing,
+    publicContactSharing = com.synonym.paykit.PublicContactSharingPolicy.ENABLED, // test hook: native public-marker fixture
 )
 
 internal fun paykitPubkyClientConfig(
diff --git a/app/src/main/java/to/bitkit/services/PubkyService.kt b/app/src/main/java/to/bitkit/services/PubkyService.kt
index e9105a40c..560146420 100644
--- a/app/src/main/java/to/bitkit/services/PubkyService.kt
+++ b/app/src/main/java/to/bitkit/services/PubkyService.kt
@@ -208,6 +208,7 @@ class PubkyService @Inject constructor(
     }
 
     suspend fun deletePaykitProfile() = ServiceQueue.CORE.background {
+        to.bitkit.dev.CleanupProbe.deletes++; to.bitkit.dev.CleanupProbe.note("profile delete entered") // test hook
         paykitSdkService.deletePaykitProfile()
     }
 
@@ -216,6 +217,7 @@ class PubkyService @Inject constructor(
     }
 
     suspend fun contactRecords(): List<ContactRecord> = ServiceQueue.CORE.background {
+        to.bitkit.dev.CleanupProbe.reads++; to.bitkit.dev.CleanupProbe.note("contact read entered") // test hook
         paykitSdkService.contactRecords()
     }
 
@@ -252,6 +254,7 @@ class PubkyService @Inject constructor(
     }
 
     suspend fun removeContacts(publicKeys: List<String>): List<ContactRecord> = ServiceQueue.CORE.background {
+        to.bitkit.dev.CleanupProbe.removals++; to.bitkit.dev.CleanupProbe.note("contact remove entered count=${publicKeys.size}") // test hook
         paykitSdkService.removeContacts(publicKeys)
     }
 
diff --git a/app/src/debug/java/to/bitkit/dev/CleanupProbeActivity.kt b/app/src/debug/java/to/bitkit/dev/CleanupProbeActivity.kt
new file mode 100644
index 000000000..f0c70a077
--- /dev/null
+++ b/app/src/debug/java/to/bitkit/dev/CleanupProbeActivity.kt
@@ -0,0 +1,113 @@
+package to.bitkit.dev
+
+// test hook: disposable native fixture and inspector; deletion decisions use the unchanged screen/viewmodel/repositories.
+import android.os.Bundle
+import android.util.Log
+import androidx.activity.ComponentActivity
+import androidx.activity.compose.setContent
+import androidx.compose.foundation.layout.*
+import androidx.compose.material3.*
+import androidx.compose.runtime.*
+import androidx.compose.ui.Modifier
+import androidx.compose.ui.unit.dp
+import androidx.lifecycle.lifecycleScope
+import dagger.hilt.android.AndroidEntryPoint
+import kotlinx.coroutines.*
+import kotlinx.coroutines.flow.first
+import to.bitkit.data.PubkyStore
+import to.bitkit.data.SettingsStore
+import to.bitkit.data.keychain.Keychain
+import to.bitkit.repositories.*
+import to.bitkit.services.*
+import to.bitkit.ui.screens.profile.*
+import to.bitkit.ui.theme.AppThemeSurface
+import java.security.SecureRandom
+import javax.inject.Inject
+
+object CleanupProbe {
+    var reads = 0
+    var removals = 0
+    var deletes = 0
+    fun note(s: String) { Log.i("CleanupProbe", s) }
+}
+
+@AndroidEntryPoint
+class CleanupProbeActivity : ComponentActivity() {
+    @Inject lateinit var service: PubkyService
+    @Inject lateinit var sdk: PaykitSdkService
+    @Inject lateinit var repo: PubkyRepo
+    @Inject lateinit var privateRepo: PrivatePaykitRepo
+    @Inject lateinit var pubkyStore: PubkyStore
+    @Inject lateinit var settings: SettingsStore
+    @Inject lateinit var keychain: Keychain
+    @Inject lateinit var wallet: WalletRepo
+    private var status by mutableStateOf("Preparing disposable native identity")
+    private var evidence by mutableStateOf("")
+    private var vm by mutableStateOf<EditProfileViewModel?>(null)
+    private var exited by mutableStateOf(false)
+    private var successToasts = 0 // test hook
+
+    override fun onCreate(savedInstanceState: Bundle?) {
+        super.onCreate(savedInstanceState)
+        setContent {
+            AppThemeSurface {
+                Column(Modifier.fillMaxSize().padding(top = 28.dp)) {
+                    Text(status, Modifier.padding(horizontal = 12.dp))
+                    Text(evidence, Modifier.padding(horizontal = 12.dp))
+                    Row {
+                        TextButton(onClick = { lifecycleScope.launch { inspect() } }) { Text("Inspect evidence") }
+                        TextButton(onClick = { vm?.dismissDeleteFailureDialog() }) { Text("Cancel failure") }
+                    }
+                    if (exited) Text("Deletion success navigation")
+                    vm?.let { current ->
+                        Box(Modifier.weight(1f)) {
+                            EditProfileScreen(current, onBackClick = {}, onExitProfile = { exited = true })
+                        }
+                    }
+                }
+            }
+        }
+        lifecycleScope.launch { to.bitkit.ui.shared.toast.ToastEventBus.events.collect { toast -> if (toast.type == to.bitkit.models.Toast.ToastType.SUCCESS) successToasts++; CleanupProbe.note("toast ${toast.type}: ${toast.title}") } } // test hook
+        lifecycleScope.launch {
+            try {
+                if (!keychain.exists(Keychain.Key.BIP39_MNEMONIC.name)) wallet.createWallet(null).getOrThrow() // test hook: unfunded wallet for normal cold startup
+                service.initialize()
+                if (intent.getBooleanExtra("resume", false)) {
+                    repo.initialize().getOrThrow()
+                    status = "Restored through normal profile initialization"
+                } else {
+                    val secret = ByteArray(32).also { SecureRandom().nextBytes(it) }.joinToString("") { "%02x".format(it) }
+                    val home = service.publicKeyFromSecret("00".repeat(32))
+                    service.signUp(secret, home, null)
+                    service.publishPaykitProfile(com.synonym.paykit.PaykitProfile("Disposable cleanup identity", null, null))
+                    repo.initialize().getOrThrow()
+                    val peers = listOf("11", "22").map { service.publicKeyFromSecret(it.repeat(32)) }
+                    for ((index, peer) in peers.withIndex()) {
+                        service.saveContact(peer, "Peer ${index + 1}", restorePrivateConnection = true)
+                    }
+                    if (intent.getBooleanExtra("retained", false)) sdk.reviewPublishContact(peers.last())
+                    repo.loadContacts()
+                    pubkyStore.update { it.copy(contactProfileOverrides = repo.contacts.value.associate { c -> c.publicKey to c.toProfileData() }) }
+                    settings.update { it.copy(sharesPrivatePaykitEndpoints = true, sharesPublicPaykitEndpoints = true) }
+                    status = "Authenticated disposable identity"
+                }
+                CleanupProbe.reads = 0; CleanupProbe.removals = 0; CleanupProbe.deletes = 0
+                vm = EditProfileViewModel(applicationContext, repo, privateRepo)
+                CleanupProbe.note("READY owner=${repo.publicKey.value}")
+                inspect()
+            } catch (e: Exception) {
+                status = "Provisioning failed: ${e.message}"
+                CleanupProbe.note(status)
+                Log.e("CleanupProbe", "setup", e)
+            }
+        }
+    }
+
+    private suspend fun inspect() {
+        val state = settings.data.first()
+        val session = keychain.loadString(Keychain.Key.PAYKIT_SESSION.name) != null
+        val overrides = pubkyStore.data.first().contactProfileOverrides.size
+        evidence = "Session $session; contacts ${repo.contacts.value.size}; overrides $overrides; sharing ${state.sharesPublicPaykitEndpoints}/${state.sharesPrivatePaykitEndpoints}; reads ${CleanupProbe.reads}, removals ${CleanupProbe.removals}, deletes ${CleanupProbe.deletes}; success $exited; success toasts $successToasts"
+        CleanupProbe.note(evidence)
+    }
+}
Case 1a: Session true; contacts 2; overrides 2; sharing true/true; reads 0, removals 0, deletes 0; success false; success toasts 0
Case 1b: Session true; contacts 2; overrides 2; sharing true/true; reads 1, removals 1, deletes 0; success false; success toasts 0
Case 1c: Session true; contacts 1; overrides 1; sharing true/true; reads 1, removals 1, deletes 0; success false; success toasts 0
Case 2: Session true; contacts 0; overrides 0; sharing true/true; reads 1, removals 1, deletes 1; success false; success toasts 0

Reviewer model: gpt-6.1-sol
Reasoning effort: high


Reviewed by gpt-6.1-sol-high via gh-pr-review-loop skill
Commands: @ovi-reviewer review · test · retest

@ovitrif ovitrif left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

utAck

@piotr-iohk piotr-iohk left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

QA review

Scope: full review of the complete PR diff against its merge base 99b446a (the head of dependency #1688), at 2daec90. No earlier QA baseline existed. Coverage: PubkyRepo.deleteProfile / deleteAllContacts; the error paths in EditProfileViewModel for profile deletion and Disconnect; the callers PrivatePaykitRepo.removePublishedEndpointsForCleanup / closeAndClear, signOut / clearAuthenticatedState, and the PaykitSdkService contact and profile wrappers; the updated unit tests and the new journey. Also checked: the Paykit SDK contracts at paykit-rs v0.1.1-rc1 (f384694) and parity with iOS twin bitkit-ios#1124 at a062618.

1 actionable finding — resolve or provide an evidence-backed rebuttal.

The stricter stop-on-incomplete-cleanup behaviour looks sound for transient failures. Dropping the 404/"missing" tolerance is safe: in the SDK, fetch_paykit_profile returns None when the profile is already gone, so deletePaykitProfile completes normally, and contact_records is a local storage read. Contact keys are compared after normalising the pubky prefix in every place. Returning early when there are no contacts leaves no stale state, because the successful signOut() path calls pubkyStore.reset() and clears the contact list. One problem remains: contacts that the SDK keeps on purpose, because of an active recurring subscription, now block profile deletion permanently without saying why (see the inline finding). The iOS twin has the same behaviour: removeContacts skips subscribed contacts and the best-effort wrapper is gone, so the concern applies to both platforms.

Existing unresolved thread: deferred withdrawal leaves private endpoints withdrawn after a stopped deletion. The author says it is tracked in #1687 / #1693. I did not re-report it here.

Validation: no build or tests were run in this review. The author's CI at this head passed: CI, Lint, E2E. None of these exercises the new cleanup-fault journey.

Device testing: not performed in this review.

Suggested additional test cases

  • Android, two wallets with different Pubky identities (A and B), plus a third saved contact C on A. A saves B and C as contacts and holds an active recurring subscription with B, as payer or payee. On A, go to Edit Profile and delete the profile. Expected: deletion is refused with a message naming the active subscription, and C plus the private payment links are still intact. Then tap Retry. Expected: it does not loop on the same generic failure.

Findings

  • [MEDIUM] An active subscription with a saved contact now blocks profile deletion permanently, with no explanation — inline at app/src/main/java/to/bitkit/repositories/PubkyRepo.kt:1116.

Reviewer model: claude-opus-5-5
Reasoning effort: high

@ovi-reviewer ovi-reviewer Bot 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.

Verdict: ⛔️ Request changes — the existing active-subscription failure reproduces on device.

Findings:

  • MEDIUM — Active subscriptions leave deletion in an unexplained Retry loop (app/src/main/java/to/bitkit/repositories/PubkyRepo.kt:1123). I confirmed the existing finding with two authenticated native wallets, in both payer and payee roles. Bulk removal skips the subscribed peer, removes and blocks the unrelated contact, then this comparison throws the generic error. The dialog never explains the subscription; Retry repeats the failure. Check active subscriptions before destructive cleanup and show the actionable reason. Test 6 supplies the new evidence; I am keeping it in the existing thread.

I found no additional defect in the cleanup patch or its affected parent integration. The acknowledged publication-pause finding remains open; the author's reply tracks recovery separately.

QA for f72e0c0: Android 16 / API 36, Core 0.8.0-rc11 and Paykit 0.1.1-rc2. The unmodified head was built separately. Device cases used this head plus the fixture below: disposable authenticated identities, unfunded regtest wallets, the real Edit Profile screen, ViewModel, repositories and native SDK, with a homeserver fault proxy. Both installed APK hashes matched the built fixture APK.

Test 1: Manual 1 / J1 — private cleanup fails ✅ Failure stops before contact or profile deletion.

The proxy returned 503 for the native shared-state GET during private cleanup. After Cancel, the refreshed local inspector retained the session, both contacts and overrides, both sharing flags and no success feedback. Call counters and the proxy trace confirmed that contact/profile deletion was not entered.

1a.2x.mp4

Private cleanup failure and retained state, at 2× speed.

Test 2: Manual 1 / J1 — unconfirmed contact removal ✅ Unconfirmed removal keeps local evidence and stops profile deletion.

The trace showed the native write-ahead PUT accepted with 201, followed by shared-state publication rejected with 503. The failure dialog appeared. The refreshed inspector retained both contacts and overrides, session and sharing flags, with no profile deletion or success feedback.

1b.2x.mp4

Unconfirmed cleanup and refreshed state after Cancel, at 2× speed.

Test 3: Manual 1 / J1 — partial native removal ✅ Only the acknowledged contact and override disappear.

A native published marker retained one contact while the SDK acknowledged the other removal. Failure appeared before profile deletion. The refreshed inspector showed one contact and override; the persisted override key matched the retained marker. Public-contact publication was enabled only to stage this fixture variant.

1c.2x.mp4

Partial removal and retained evidence, at 2× speed.

Test 4: Manual 2 — profile deletion returns 404 ✅ Failure retains the session and sharing settings.

The trace confirmed profile GET 200 followed by DELETE 404. The app showed failure with no success feedback. The refreshed inspector retained its session and both sharing flags; previously acknowledged contact removals stayed applied.

2.2x.mp4

Rejected profile deletion and retained session, at 2× speed.

Test 5: J1 — Cancel and cold restart ✅ Normal startup restores the same identity without replaying profile deletion.

After unconfirmed contact cleanup and Cancel, a cold launch through the normal app entry restored the same identity on Home and Profile. The loaded profile and QR identifier matched the original owner. A profile GET returned 200, and no profile DELETE appeared during the 90-second proxy observation.

restart.2x.mp4

Normal cold startup and restored profile, at 2× speed.

Test 6: Active subscription — delete and Retry ❌ Both payer and payee get generic failure after unrelated-contact removal.

Two linked native wallets proposed and accepted a recurring request whose first period starts the following day; no payment or funds were needed. Each wallet had its subscribed peer plus an unrelated saved contact C. I ran Delete Profile and then Retry on both wallets.

Both runs removed C and its override, blocked C, and retained the active subscription and its peer. The dialog offered Retry/Disconnect without explaining the subscription. Retry produced the same failure. The inspectors showed two cleanup attempts and no profile deletion, success toast or success navigation. Native request records confirmed the payer/payee roles; persisted override keys confirmed which contact survived. This adds device evidence to the existing finding.

Payee Payer
subscription-payee.2x.mp4
subscription-payer.2x.mp4

Initial deletion, retained subscription and repeated failure on Retry, at 2× speed. Private payment-link usability was not separately measured in this unfunded fixture.

Check 1: unmodified-head build and focused regressions ✅ The app build and affected component suites pass.

assembleDevDebug and focused testDevDebugUnitTest runs covering PubkyRepoTest, EditProfileViewModelTest, PrivatePaykitRepoTest and PaykitSdkServiceTest passed. XML reports contain no failures, errors or skips. These include contact-read errors, uncertain/partial removals, 404-like profile errors, private cleanup failure, local-finalization failure and active-subscription retention. git diff --check passed.

Check 2: delta and twin comparison ✅ The cleanup feature patch survives the additive parent merge.

2daec90 is an ancestor of f72e0c0. The six-file cleanup feature patch is preserved; the merged Paykit and restoration dependencies were reviewed with the reached cleanup paths. The native dependency upgrade from rc1 to rc2 and changed startup composition prompted fresh device runs instead of carrying the earlier revision-bound report.

Delta review of 2daec90..f72e0c0, the cleanup feature and its changed dependencies, plus open findings. Unrelated changes imported from the parent were not independently re-reviewed in full. Contact-read and local-finalization storage failures had component coverage rather than separate device cases. Original-owner/manual-retry reconciliation and native cross-device conditional removal remain outside this PR's stated scope.

Twin: bitkit-ios#1124 fix: stop deletion after incomplete cleanup matches the scoped cleanup/error propagation at c26d7b13. This run adds no iOS device result.

Test fixture used by Tests 1–6

Not pushed. Provisions disposable native identities and contacts, establishes a native recurring request between two wallets, renders the actual Edit Profile screen, and exposes call/local-state evidence. Native calls return their real results. The public-contact policy is enabled to stage Test 3's retained marker. The fault proxy supplies Tests 1, 2 and 4; Test 5 launches the normal app entry. Fixture APK SHA-256: 915d1997fdb45dc11c8f69b7c4ccf6291a85d0b66c71be5f1f14df9044bd9a20.

diff --git a/app/src/debug/AndroidManifest.xml b/app/src/debug/AndroidManifest.xml
index b88fe73a7..7b18a44e0 100644
--- a/app/src/debug/AndroidManifest.xml
+++ b/app/src/debug/AndroidManifest.xml
@@ -6,6 +6,8 @@
         android:usesCleartextTraffic="true"
         tools:ignore="MissingApplicationIcon">
 
+        <activity android:name="to.bitkit.dev.CleanupProbeActivity" android:exported="true" android:launchMode="singleTop" /> <!-- test hook -->
+
         <provider
             android:name="to.bitkit.dev.DevToolsProvider"
             android:authorities="${applicationId}.devtools"
diff --git a/app/src/main/java/to/bitkit/services/PaykitSdkService.kt b/app/src/main/java/to/bitkit/services/PaykitSdkService.kt
index 87c452f64..011d57041 100644
--- a/app/src/main/java/to/bitkit/services/PaykitSdkService.kt
+++ b/app/src/main/java/to/bitkit/services/PaykitSdkService.kt
@@ -666,6 +666,11 @@ class PaykitSdkService @Inject constructor(
     suspend fun fetchPubkyFollows(publicKey: String): List<String> =
         publicRead { it.fetchPubkyFollows(publicKey, maxEntries = 10_000u) }
 
+    internal suspend fun reviewPublishContact(key: String) { // test hook: provision real native retained marker
+        isSetup.await() // test hook
+        operationLock.withLock { withStateRevisionTracking { handle -> completeSdkCall { handle.publishPublicContact(key) } } } // test hook
+    } // test hook
+
     suspend fun contactRecords(): List<ContactRecord> {
         isSetup.await()
         return operationLock.withLock {
@@ -1952,7 +1957,7 @@ internal object BitkitPaykitSdkConfig {
 }
 
 internal fun paykitSdkConfig() = defaultConfig("bitkit").copy(
-    publicContactSharing = BitkitPaykitSdkConfig.publicContactSharing,
+    publicContactSharing = com.synonym.paykit.PublicContactSharingPolicy.ENABLED, // test hook: native public-marker fixture
 )
 
 internal fun paykitPubkyClientConfig(
diff --git a/app/src/main/java/to/bitkit/services/PubkyService.kt b/app/src/main/java/to/bitkit/services/PubkyService.kt
index e9105a40c..560146420 100644
--- a/app/src/main/java/to/bitkit/services/PubkyService.kt
+++ b/app/src/main/java/to/bitkit/services/PubkyService.kt
@@ -208,6 +208,7 @@ class PubkyService @Inject constructor(
     }
 
     suspend fun deletePaykitProfile() = ServiceQueue.CORE.background {
+        to.bitkit.dev.CleanupProbe.deletes++; to.bitkit.dev.CleanupProbe.note("profile delete entered") // test hook
         paykitSdkService.deletePaykitProfile()
     }
 
@@ -216,6 +217,7 @@ class PubkyService @Inject constructor(
     }
 
     suspend fun contactRecords(): List<ContactRecord> = ServiceQueue.CORE.background {
+        to.bitkit.dev.CleanupProbe.reads++; to.bitkit.dev.CleanupProbe.note("contact read entered") // test hook
         paykitSdkService.contactRecords()
     }
 
@@ -252,6 +254,7 @@ class PubkyService @Inject constructor(
     }
 
     suspend fun removeContacts(publicKeys: List<String>): List<ContactRecord> = ServiceQueue.CORE.background {
+        to.bitkit.dev.CleanupProbe.removals++; to.bitkit.dev.CleanupProbe.note("contact remove entered count=${publicKeys.size}") // test hook
         paykitSdkService.removeContacts(publicKeys)
     }
 
diff --git a/app/src/debug/java/to/bitkit/dev/CleanupProbeActivity.kt b/app/src/debug/java/to/bitkit/dev/CleanupProbeActivity.kt
new file mode 100644
--- /dev/null
+++ b/app/src/debug/java/to/bitkit/dev/CleanupProbeActivity.kt
@@ -0,0 +1,175 @@
+package to.bitkit.dev
+
+// test hook: disposable native fixture and inspector; deletion decisions use the unchanged screen/viewmodel/repositories.
+import android.os.Bundle
+import android.content.Intent
+import com.synonym.paykit.*
+import android.util.Log
+import androidx.activity.ComponentActivity
+import androidx.activity.compose.setContent
+import androidx.compose.foundation.layout.*
+import androidx.compose.material3.*
+import androidx.compose.runtime.*
+import androidx.compose.ui.Modifier
+import androidx.compose.ui.unit.dp
+import androidx.lifecycle.lifecycleScope
+import dagger.hilt.android.AndroidEntryPoint
+import kotlinx.coroutines.*
+import kotlinx.coroutines.flow.first
+import to.bitkit.data.PubkyStore
+import to.bitkit.data.SettingsStore
+import to.bitkit.data.keychain.Keychain
+import to.bitkit.repositories.*
+import to.bitkit.services.*
+import to.bitkit.ui.screens.profile.*
+import to.bitkit.ui.theme.AppThemeSurface
+import java.security.SecureRandom
+import javax.inject.Inject
+
+object CleanupProbe {
+    var reads = 0
+    var removals = 0
+    var deletes = 0
+    fun note(s: String) { Log.i("CleanupProbe", s) }
+}
+
+@AndroidEntryPoint
+class CleanupProbeActivity : ComponentActivity() {
+    @Inject lateinit var service: PubkyService
+    @Inject lateinit var sdk: PaykitSdkService
+    @Inject lateinit var repo: PubkyRepo
+    @Inject lateinit var privateRepo: PrivatePaykitRepo
+    @Inject lateinit var pubkyStore: PubkyStore
+    @Inject lateinit var settings: SettingsStore
+    @Inject lateinit var keychain: Keychain
+    @Inject lateinit var wallet: WalletRepo
+    private var status by mutableStateOf("Preparing disposable native identity")
+    private var evidence by mutableStateOf("")
+    private var vm by mutableStateOf<EditProfileViewModel?>(null)
+    private var exited by mutableStateOf(false)
+    private var successToasts = 0 // test hook
+
+    override fun onNewIntent(intent: Intent) {
+        super.onNewIntent(intent)
+        lifecycleScope.launch {
+            try {
+                val peer = intent.getStringExtra("peer").orEmpty()
+                when (intent.getStringExtra("action")) {
+                    "link" -> {
+                        service.saveContact(peer, "Subscription peer", restorePrivateConnection = true)
+                        sdk.ensureLinkWithPeer(peer, 8u)
+                        sdk.processOutboundPrivateMessages(peer)
+                        sdk.receivePrivateMessages(peer)
+                        repo.loadContacts()
+                        pubkyStore.update { it.copy(contactProfileOverrides = repo.contacts.value.associate { c -> c.publicKey to c.toProfileData() }) }
+                    }
+                    "pump" -> {
+                        sdk.ensureLinkWithPeer(peer, 8u)
+                        sdk.receivePrivateMessages(peer)
+                        sdk.processOutboundPrivateMessages(peer)
+                    }
+                    "propose" -> {
+                        val now = java.time.Instant.now()
+                        val start = now.plusSeconds(86400).toString()
+                        val proposal = PaykitPaymentRequestProposalTerms(
+                            amountValue = "0.00005", amountAsset = "btc",
+                            paymentReference = "cleanup-${java.util.UUID.randomUUID()}",
+                            proposalExpiresAt = now.plusSeconds(259200).toString(),
+                            acceptedPaymentEndpointIdentifiers = listOf("btc-lightning-bolt11"),
+                            metadataJson = """{"note":"Cleanup subscription","subscription":{"version":1,"description":"Disposable future subscription","benefits":[]}}""",
+                            recurrence = PaykitPaymentRequestRecurrenceTerms(1u, "month", start, start)
+                        )
+                        val record = sdk.proposePaymentRequest(peer, proposal, repo.publicKey.value!!)
+                        CleanupProbe.note("proposed ${record.paymentRequestId} state=${record.state}")
+                        sdk.processOutboundPrivateMessages(peer)
+                    }
+                    "accept" -> {
+                        sdk.receivePrivateMessages(peer)
+                        val record = sdk.paymentRequests().first { it.localRole == PaymentRequestLocalRole.PAYER && it.state == PaymentRequestLifecycleState.PROPOSED }
+                        val accepted = sdk.acceptPaymentRequest(peer, record.paymentRequestId, record.proposalStreamItemId!!)
+                        sdk.processOutboundPrivateMessages(peer)
+                        CleanupProbe.note("accepted ${accepted.paymentRequestId} state=${accepted.state}")
+                    }
+                    "inspect" -> Unit
+                }
+                status = "Native fixture action completed: ${intent.getStringExtra("action")}"
+                inspect()
+            } catch (e: Exception) {
+                status = "Native fixture action failed: ${e.message}"
+                CleanupProbe.note(status)
+                Log.e("CleanupProbe", "command", e)
+            }
+        }
+    }
+
+    override fun onCreate(savedInstanceState: Bundle?) {
+        super.onCreate(savedInstanceState)
+        setContent {
+            AppThemeSurface {
+                Column(Modifier.fillMaxSize().padding(top = 28.dp)) {
+                    Text(status, Modifier.padding(horizontal = 12.dp))
+                    Text(evidence, Modifier.padding(horizontal = 12.dp))
+                    Row {
+                        TextButton(onClick = { lifecycleScope.launch { inspect() } }) { Text("Inspect evidence") }
+                        TextButton(onClick = { vm?.dismissDeleteFailureDialog() }) { Text("Cancel failure") }
+                    }
+                    if (exited) Text("Deletion success navigation")
+                    vm?.let { current ->
+                        Box(Modifier.weight(1f)) {
+                            EditProfileScreen(current, onBackClick = {}, onExitProfile = { exited = true })
+                        }
+                    }
+                }
+            }
+        }
+        lifecycleScope.launch { to.bitkit.ui.shared.toast.ToastEventBus.events.collect { toast -> if (toast.type == to.bitkit.models.Toast.ToastType.SUCCESS) successToasts++; CleanupProbe.note("toast ${toast.type}: ${toast.title}") } } // test hook
+        lifecycleScope.launch {
+            try {
+                if (!keychain.exists(Keychain.Key.BIP39_MNEMONIC.name)) wallet.createWallet(null).getOrThrow() // test hook: unfunded wallet for normal cold startup
+                service.initialize()
+                if (intent.getBooleanExtra("resume", false)) {
+                    repo.initialize().getOrThrow()
+                    status = "Restored through normal profile initialization"
+                } else {
+                    val secret = ByteArray(32).also { SecureRandom().nextBytes(it) }.joinToString("") { "%02x".format(it) }
+                    val home = service.publicKeyFromSecret("00".repeat(32))
+                    service.signUp(secret, home, null)
+                    service.publishPaykitProfile(com.synonym.paykit.PaykitProfile("Disposable cleanup identity", null, null))
+                    repo.initialize().getOrThrow()
+                    val peers = (if (intent.getBooleanExtra("subscription", false)) listOf("22") else listOf("11", "22")).map { service.publicKeyFromSecret(it.repeat(32)) }
+                    for ((index, peer) in peers.withIndex()) {
+                        service.saveContact(peer, "Peer ${index + 1}", restorePrivateConnection = true)
+                    }
+                    if (intent.getBooleanExtra("retained", false)) sdk.reviewPublishContact(peers.last())
+                    repo.loadContacts()
+                    pubkyStore.update { it.copy(contactProfileOverrides = repo.contacts.value.associate { c -> c.publicKey to c.toProfileData() }) }
+                    settings.update { it.copy(sharesPrivatePaykitEndpoints = true, sharesPublicPaykitEndpoints = true) }
+                    status = "Authenticated disposable identity"
+                }
+                CleanupProbe.reads = 0; CleanupProbe.removals = 0; CleanupProbe.deletes = 0
+                vm = EditProfileViewModel(applicationContext, repo, privateRepo)
+                CleanupProbe.note("READY owner=${repo.publicKey.value}")
+                inspect()
+            } catch (e: Exception) {
+                status = "Provisioning failed: ${e.message}"
+                CleanupProbe.note(status)
+                Log.e("CleanupProbe", "setup", e)
+            }
+        }
+    }
+
+    private suspend fun inspect() {
+        val state = settings.data.first()
+        val session = keychain.loadString(Keychain.Key.PAYKIT_SESSION.name) != null
+        val overrides = pubkyStore.data.first().contactProfileOverrides.size
+        evidence = "Session $session; contacts ${repo.contacts.value.size}; overrides $overrides; sharing ${state.sharesPublicPaykitEndpoints}/${state.sharesPrivatePaykitEndpoints}; reads ${CleanupProbe.reads}, removals ${CleanupProbe.removals}, deletes ${CleanupProbe.deletes}; success $exited; success toasts $successToasts"
+        if (intent.getBooleanExtra("subscription", false)) {
+            val requests = sdk.paymentRequests()
+            val active = requests.filter { it.state == PaymentRequestLifecycleState.ACTIVE_RECURRING }
+            val peers = sdk.linkedPeers()
+            evidence += "; active subscriptions ${active.size}; linked ${peers.count { it.state == LinkedPeerState.LINKED }}; blocked ${peers.count { it.state == LinkedPeerState.BLOCKED }}"
+            CleanupProbe.note("owner=${repo.publicKey.value} contacts=${repo.contacts.value.map { it.publicKey }} overrides=${pubkyStore.data.first().contactProfileOverrides.keys} active=${active.map { it.localRole to it.counterparty }}")
+        }
+        CleanupProbe.note(evidence)
+    }
+}

Decisive inspector lines after deletion/Retry (native role records and persisted keys were checked as described above):

Private cleanup failure: Session true; contacts 2; overrides 2; sharing true/true; reads 0, removals 0, deletes 0; success false; success toasts 0
Unconfirmed contact removal: Session true; contacts 2; overrides 2; sharing true/true; reads 1, removals 1, deletes 0; success false; success toasts 0
Partial native removal: Session true; contacts 1; overrides 1; sharing true/true; reads 1, removals 1, deletes 0; success false; success toasts 0
DELETE 404: Session true; contacts 0; overrides 0; sharing true/true; reads 1, removals 1, deletes 1; success false; success toasts 0
PAYEE after Retry: Session true; contacts 1; overrides 1; sharing true/true; reads 2, removals 2, deletes 0; success false; success toasts 0; active subscriptions 1; linked 1; blocked 1
PAYER after Retry: Session true; contacts 1; overrides 1; sharing true/true; reads 2, removals 2, deletes 0; success false; success toasts 0; active subscriptions 1; linked 1; blocked 1

Reviewed by gpt-6.1-sol-high

@ovi-reviewer ovi-reviewer Bot 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.

Verdict: ✅ Approve

Findings: I found no new defects. The active-subscription finding is fixed at 7afdd3e: both native roles refuse before cleanup, and fresh confirmations succeed after cancellation. The acknowledged publication-pause issue remains tracked in the separate recovery follow-up.

QA for 7afdd3e: Android 16 / API 36, bitkit-core 0.8.0-rc11 and Paykit 0.1.1-rc2. The unmodified head was built separately. Device cases used this head plus the fixture below, with disposable authenticated identities, unfunded regtest wallets and a controlled homeserver proxy. Both installed APK hashes matched the built fixture APK.

Test 1: Manual 3 / J1 — active payee subscription ✅ The subscription instruction appears before any cleanup.

A future-dated native recurring request was accepted between two authenticated wallets. The payee saw “End active subscriptions before removing this profile.” No generic Retry dialog appeared. The refreshed inspector retained both contacts and overrides, the subscription, session and sharing flags. Native role records and call counters confirmed zero private pause, withdrawal, contact removal or profile deletion.


Recorded flow and state evidence, at 2× speed.

Test 2: Manual 3 / J1 — active payer subscription ✅ The payer receives the same instruction and keeps both contacts.

The counterpart wallet held the native payer record and an unrelated saved contact. Confirming Delete Profile emitted the localized instruction. The refreshed inspector and native records retained the active obligation, both contacts and overrides, session and sharing flags; all cleanup counters stayed zero. No generic Retry dialog appeared.


Recorded flow and state evidence, at 2× speed.

Test 3: Manual 3 / J1 — cancellation permits payee removal ✅ A new confirmation succeeds after the subscription ends.

The native SDK cancelled the disposable subscription, and its cancellation was delivered to the counterpart. After confirming that no active subscription remained, I explicitly confirmed removal again. The inspector showed one contact batch and one profile deletion, no contacts or overrides, cleared session/sharing flags, and the success toast/effect. The fixture displays the delivered navigation effect as a marker.


Recorded flow and state evidence, at 2× speed.

Test 4: Manual 3 / J1 — cancellation permits payer removal ✅ A new payer confirmation also completes removal.

After the native cancellation reached this wallet, its active-subscription count was zero. A fresh confirmation acknowledged contact and profile removal. The refreshed inspector showed the session and sharing flags cleared, no contacts or overrides, one contact batch, one profile deletion and the success toast/effect.


Recorded flow and state evidence, at 2× speed.

Test 5: Subscription preflight cannot read storage ✅ An unavailable subscription read stops before private cleanup.

An identity-scoped homeserver rule returned 503 for the shared-state GET. The existing failure dialog appeared. After Cancel, both contacts and overrides, session and sharing flags remained. The proxy trace confirmed the rejected read; the inspector recorded one preflight and zero private pauses, withdrawals, contact removals or profile deletions.


Recorded flow and state evidence, at 2× speed.

Test 6: Manual 1 / J1 — unconfirmed contact removal ✅ Unconfirmed publication retains local evidence and stops profile deletion.

The proxy accepted the native write-ahead PUT with 201, then rejected shared-state publication with 503. The failure dialog appeared. After Cancel, the refreshed inspector retained both contacts and overrides, session and sharing flags, with one contact-removal attempt and no profile deletion or success feedback.


Recorded flow and state evidence, at 2× speed.

Test 7: Manual 1 / J1 — partial native removal ✅ The retained-marker fixture keeps one contact and override.

A native published marker retained one contact while the SDK acknowledged the other removal. The profile-only batch reached the native SDK; the app then showed failure. After Cancel, the inspector showed one remaining contact and override, retained session/sharing flags and zero profile deletions or success feedback. Public-contact publication was enabled only to stage this fixture variant.


Recorded flow and state evidence, at 2× speed.

Test 8: Manual 2 — profile deletion returns 404 ✅ Failure retains the session and sharing settings.

The proxy trace confirmed profile GET 200 followed by DELETE 404. The app showed failure with no success feedback. After Cancel, the refreshed inspector retained the session and both sharing flags. Earlier acknowledged contact removals remained applied.


Recorded flow and state evidence, at 2× speed.

Check 1: exact-head build and component regressions ✅ The app build and affected suites pass.

assembleDevDebug and focused testDevDebugUnitTest runs covering PubkyRepoTest, PaykitSdkServiceTest and EditProfileViewModelTest passed on the unmodified head. XML reports contain no failures, errors or skips. These cover conservative subscription expiry, payer/payee records, unavailable reads, service-error wrapping, cancellation, incomplete contact cleanup and failed local finalization. git diff --check passed.

Check 2: earlier evidence and delta scope ✅ The unchanged private-withdrawal and cold-restart cases retain their earlier evidence.

I rely on the prior authenticated report for private-withdrawal failure and Cancel plus normal cold startup without deletion replay. Their cleanup/restart implementations and native dependency pins are unchanged. Contact publication, partial removal and profile-404 cases were driven again through the new profile-only batch.

Delta review of f72e0c0..7afdd3e, all 25 changed files, reached cleanup/cancellation paths and prior feedback. Unknown/malformed expiry, service wrappers, contact-read and local-finalization failures have component coverage rather than separate device fault cases. Manual-retry ownership and native cross-device conditional removal remain outside this PR’s stated scope.

Twin: bitkit-ios#1124 fix: stop deletion after incomplete cleanup at a2b2186 matches early subscription refusal, localized guidance and strict contact cleanup; iOS also checks at final SDK profile deletion. This run adds no iOS device result.

Native test fixture

Not pushed. Provisions disposable native identities and contacts, links two wallets, proposes/accepts/cancels a future-dated recurring request, renders the real Edit Profile screen with the normal toast components, and displays native/local state plus call counters. Deletion decisions and SDK results are unchanged. A native public marker stages partial retention; the proxy supplies the storage and DELETE faults. The success-navigation marker records the real ViewModel effect. Fixture APK SHA-256: f0e4191613c2ca51003932c9c4f8972cb04cd03f41443bf501fc646262f24d13.

diff --git a/app/src/debug/AndroidManifest.xml b/app/src/debug/AndroidManifest.xml
index b88fe73a7..7b18a44e0 100644
--- a/app/src/debug/AndroidManifest.xml
+++ b/app/src/debug/AndroidManifest.xml
@@ -6,6 +6,8 @@
         android:usesCleartextTraffic="true"
         tools:ignore="MissingApplicationIcon">
 
+        <activity android:name="to.bitkit.dev.CleanupProbeActivity" android:exported="true" android:launchMode="singleTop" /> <!-- test hook -->
+
         <provider
             android:name="to.bitkit.dev.DevToolsProvider"
             android:authorities="${applicationId}.devtools"
diff --git a/app/src/debug/java/to/bitkit/dev/CleanupProbeActivity.kt b/app/src/debug/java/to/bitkit/dev/CleanupProbeActivity.kt
new file mode 100644
index 000000000..63ab9e245
--- /dev/null
+++ b/app/src/debug/java/to/bitkit/dev/CleanupProbeActivity.kt
@@ -0,0 +1,186 @@
+package to.bitkit.dev
+
+// test hook: disposable native fixture and inspector; deletion decisions use the unchanged screen/viewmodel/repositories.
+import android.os.Bundle
+import android.content.Intent
+import com.synonym.paykit.*
+import android.util.Log
+import androidx.activity.ComponentActivity
+import androidx.activity.compose.setContent
+import androidx.compose.foundation.layout.*
+import androidx.compose.material3.*
+import androidx.compose.runtime.*
+import androidx.compose.ui.Modifier
+import androidx.compose.ui.unit.dp
+import androidx.lifecycle.lifecycleScope
+import dagger.hilt.android.AndroidEntryPoint
+import kotlinx.coroutines.*
+import kotlinx.coroutines.flow.first
+import to.bitkit.data.PubkyStore
+import to.bitkit.data.SettingsStore
+import to.bitkit.data.keychain.Keychain
+import to.bitkit.repositories.*
+import to.bitkit.services.*
+import to.bitkit.ui.screens.profile.*
+import to.bitkit.ui.theme.AppThemeSurface
+import java.security.SecureRandom
+import javax.inject.Inject
+
+object CleanupProbe {
+    var reads = 0
+    var removals = 0
+    var withdrawals = 0
+    var pauses = 0
+    var preflights = 0
+    var deletes = 0
+    fun note(s: String) { Log.i("CleanupProbe", s) }
+}
+
+@AndroidEntryPoint
+class CleanupProbeActivity : ComponentActivity() {
+    @Inject lateinit var service: PubkyService
+    @Inject lateinit var sdk: PaykitSdkService
+    @Inject lateinit var repo: PubkyRepo
+    @Inject lateinit var privateRepo: PrivatePaykitRepo
+    @Inject lateinit var pubkyStore: PubkyStore
+    @Inject lateinit var settings: SettingsStore
+    @Inject lateinit var keychain: Keychain
+    @Inject lateinit var wallet: WalletRepo
+    private var status by mutableStateOf("Preparing disposable native identity")
+    private var evidence by mutableStateOf("")
+    private var vm by mutableStateOf<EditProfileViewModel?>(null)
+    private var exited by mutableStateOf(false)
+    private var successToasts = 0 // test hook
+    private val toastQueue by lazy { to.bitkit.ui.shared.toast.ToastQueueManager(lifecycleScope) } // test hook: same app toast host
+
+    override fun onNewIntent(intent: Intent) {
+        super.onNewIntent(intent)
+        lifecycleScope.launch {
+            try {
+                val peer = intent.getStringExtra("peer").orEmpty()
+                when (intent.getStringExtra("action")) {
+                    "link" -> {
+                        service.saveContact(peer, "Subscription peer", restorePrivateConnection = true)
+                        sdk.ensureLinkWithPeer(peer, 8u)
+                        sdk.processOutboundPrivateMessages(peer)
+                        sdk.receivePrivateMessages(peer)
+                        repo.loadContacts()
+                        pubkyStore.update { it.copy(contactProfileOverrides = repo.contacts.value.associate { c -> c.publicKey to c.toProfileData() }) }
+                    }
+                    "pump" -> {
+                        sdk.ensureLinkWithPeer(peer, 8u)
+                        sdk.receivePrivateMessages(peer)
+                        sdk.processOutboundPrivateMessages(peer)
+                    }
+                    "propose" -> {
+                        val now = java.time.Instant.now()
+                        val start = now.plusSeconds(86400).toString()
+                        val proposal = PaykitPaymentRequestProposalTerms(
+                            amountValue = "0.00005", amountAsset = "btc",
+                            paymentReference = "cleanup-${java.util.UUID.randomUUID()}",
+                            proposalExpiresAt = now.plusSeconds(259200).toString(),
+                            acceptedPaymentEndpointIdentifiers = listOf("btc-lightning-bolt11"),
+                            metadataJson = """{"note":"Cleanup subscription","subscription":{"version":1,"description":"Disposable future subscription","benefits":[]}}""",
+                            recurrence = PaykitPaymentRequestRecurrenceTerms(1u, "month", start, start)
+                        )
+                        val record = sdk.proposePaymentRequest(peer, proposal, repo.publicKey.value!!)
+                        CleanupProbe.note("proposed ${record.paymentRequestId} state=${record.state}")
+                        sdk.processOutboundPrivateMessages(peer)
+                    }
+                    "accept" -> {
+                        sdk.receivePrivateMessages(peer)
+                        val record = sdk.paymentRequests().first { it.localRole == PaymentRequestLocalRole.PAYER && it.state == PaymentRequestLifecycleState.PROPOSED }
+                        val accepted = sdk.acceptPaymentRequest(peer, record.paymentRequestId, record.proposalStreamItemId!!)
+                        sdk.processOutboundPrivateMessages(peer)
+                        CleanupProbe.note("accepted ${accepted.paymentRequestId} state=${accepted.state}")
+                    }
+                    "cancel" -> {
+                        val record = sdk.paymentRequests().first { it.state == PaymentRequestLifecycleState.ACTIVE_RECURRING }
+                        sdk.cancelPaymentRequest(record.counterparty, record.paymentRequestId, "Disposable cleanup test ended")
+                        sdk.processOutboundPrivateMessages(peer)
+                    }
+                    "inspect" -> Unit
+                }
+                status = "Native fixture action completed: ${intent.getStringExtra("action")}"
+                inspect()
+            } catch (e: Exception) {
+                status = "Native fixture action failed: ${e.message}"
+                CleanupProbe.note(status)
+                Log.e("CleanupProbe", "command", e)
+            }
+        }
+    }
+
+    override fun onCreate(savedInstanceState: Bundle?) {
+        super.onCreate(savedInstanceState)
+        setContent {
+            AppThemeSurface {
+                val currentToast by toastQueue.currentToast.collectAsState() // test hook
+                Column(Modifier.fillMaxSize().padding(top = 28.dp)) {
+                    Text(status, Modifier.padding(horizontal = 12.dp))
+                    Text(evidence, Modifier.padding(horizontal = 12.dp))
+                    Row {
+                        TextButton(onClick = { lifecycleScope.launch { inspect() } }) { Text("Inspect evidence") }
+                        TextButton(onClick = { vm?.dismissDeleteFailureDialog() }) { Text("Cancel failure") }
+                    }
+                    if (exited) Text("Deletion success navigation")
+                    vm?.let { current ->
+                        Box(Modifier.weight(1f)) {
+                            EditProfileScreen(current, onBackClick = {}, onExitProfile = { exited = true })
+                        }
+                    }
+                }
+                to.bitkit.ui.components.ToastOverlay(toast = currentToast, onDismiss = toastQueue::dismissCurrentToast) // test hook
+            }
+        }
+        lifecycleScope.launch { to.bitkit.ui.shared.toast.ToastEventBus.events.collect { toast -> toastQueue.enqueue(toast); if (toast.type == to.bitkit.models.Toast.ToastType.SUCCESS) successToasts++; CleanupProbe.note("toast ${toast.type}: ${toast.title}") } } // test hook
+        lifecycleScope.launch {
+            try {
+                if (!keychain.exists(Keychain.Key.BIP39_MNEMONIC.name)) wallet.createWallet(null).getOrThrow() // test hook: unfunded wallet for normal cold startup
+                service.initialize()
+                if (intent.getBooleanExtra("resume", false)) {
+                    repo.initialize().getOrThrow()
+                    status = "Restored through normal profile initialization"
+                } else {
+                    val secret = ByteArray(32).also { SecureRandom().nextBytes(it) }.joinToString("") { "%02x".format(it) }
+                    val home = service.publicKeyFromSecret("00".repeat(32))
+                    service.signUp(secret, home, null)
+                    service.publishPaykitProfile(com.synonym.paykit.PaykitProfile("Disposable cleanup identity", null, null))
+                    repo.initialize().getOrThrow()
+                    val peers = (if (intent.getBooleanExtra("subscription", false)) listOf("22") else listOf("11", "22")).map { service.publicKeyFromSecret(it.repeat(32)) }
+                    for ((index, peer) in peers.withIndex()) {
+                        service.saveContact(peer, "Peer ${index + 1}", restorePrivateConnection = true)
+                    }
+                    if (intent.getBooleanExtra("retained", false)) sdk.reviewPublishContact(peers.last())
+                    repo.loadContacts()
+                    pubkyStore.update { it.copy(contactProfileOverrides = repo.contacts.value.associate { c -> c.publicKey to c.toProfileData() }) }
+                    settings.update { it.copy(sharesPrivatePaykitEndpoints = true, sharesPublicPaykitEndpoints = true) }
+                    status = "Authenticated disposable identity"
+                }
+                CleanupProbe.reads = 0; CleanupProbe.removals = 0; CleanupProbe.deletes = 0; CleanupProbe.withdrawals = 0; CleanupProbe.pauses = 0; CleanupProbe.preflights = 0
+                vm = EditProfileViewModel(applicationContext, repo, privateRepo)
+                CleanupProbe.note("READY owner=${repo.publicKey.value}")
+                inspect()
+            } catch (e: Exception) {
+                status = "Provisioning failed: ${e.message}"
+                CleanupProbe.note(status)
+                Log.e("CleanupProbe", "setup", e)
+            }
+        }
+    }
+
+    private suspend fun inspect() {
+        val state = settings.data.first()
+        val session = keychain.loadString(Keychain.Key.PAYKIT_SESSION.name) != null
+        val overrides = pubkyStore.data.first().contactProfileOverrides.size
+        evidence = "Session $session; contacts ${repo.contacts.value.size}; overrides $overrides; sharing ${state.sharesPublicPaykitEndpoints}/${state.sharesPrivatePaykitEndpoints}; reads ${CleanupProbe.reads}, removals ${CleanupProbe.removals}, deletes ${CleanupProbe.deletes}; success $exited; success toasts $successToasts; preflights ${CleanupProbe.preflights}, pauses ${CleanupProbe.pauses}, withdrawals ${CleanupProbe.withdrawals}"
+        if (session && intent.getBooleanExtra("subscription", false)) {
+            val requests = sdk.paymentRequests()
+            val active = requests.filter { it.state == PaymentRequestLifecycleState.ACTIVE_RECURRING }
+            val peers = sdk.linkedPeers()
+            evidence += "; active subscriptions ${active.size}; linked ${peers.count { it.state == LinkedPeerState.LINKED }}; blocked ${peers.count { it.state == LinkedPeerState.BLOCKED }}"
+            CleanupProbe.note("owner=${repo.publicKey.value} contacts=${repo.contacts.value.map { it.publicKey }} overrides=${pubkyStore.data.first().contactProfileOverrides.keys} active=${active.map { it.localRole to it.counterparty }}")
+        }
+        CleanupProbe.note(evidence)
+    }
+}
diff --git a/app/src/main/java/to/bitkit/repositories/PrivatePaykitRepo.kt b/app/src/main/java/to/bitkit/repositories/PrivatePaykitRepo.kt
index ec6558f32..e99799126 100644
--- a/app/src/main/java/to/bitkit/repositories/PrivatePaykitRepo.kt
+++ b/app/src/main/java/to/bitkit/repositories/PrivatePaykitRepo.kt
@@ -361,6 +361,7 @@ class PrivatePaykitRepo @Inject constructor(
     }
 
     suspend fun beginProfileDeletion() = withContext(serializedDispatcher) {
+        to.bitkit.dev.CleanupProbe.pauses++ // test hook
         _isProfileDeletionInProgress.update { true }
         invalidateContactPreparation()
     }
@@ -506,6 +507,7 @@ class PrivatePaykitRepo @Inject constructor(
         deferAppSync: Boolean,
     ): Result<Unit> = withContext(serializedDispatcher) {
         runSuspendCatching {
+            to.bitkit.dev.CleanupProbe.withdrawals++; to.bitkit.dev.CleanupProbe.note("private withdrawal entered") // test hook
             updateContactSharingCleanupPending(true)
             removePublishedEndpoints().getOrThrow()
             if (!deferAppSync) {
diff --git a/app/src/main/java/to/bitkit/services/PaykitSdkService.kt b/app/src/main/java/to/bitkit/services/PaykitSdkService.kt
index 076e3fa65..00968fc62 100644
--- a/app/src/main/java/to/bitkit/services/PaykitSdkService.kt
+++ b/app/src/main/java/to/bitkit/services/PaykitSdkService.kt
@@ -666,6 +666,11 @@ class PaykitSdkService @Inject constructor(
     suspend fun fetchPubkyFollows(publicKey: String): List<String> =
         publicRead { it.fetchPubkyFollows(publicKey, maxEntries = 10_000u) }
 
+    internal suspend fun reviewPublishContact(key: String) { // test hook: stage a real native retained marker
+        isSetup.await()
+        operationLock.withLock { withStateRevisionTracking { handle -> completeSdkCall { handle.publishPublicContact(key) } } }
+    }
+
     suspend fun contactRecords(): List<ContactRecord> {
         isSetup.await()
         return operationLock.withLock {
@@ -1977,7 +1982,7 @@ internal object BitkitPaykitSdkConfig {
 }
 
 internal fun paykitSdkConfig() = defaultConfig("bitkit").copy(
-    publicContactSharing = BitkitPaykitSdkConfig.publicContactSharing,
+    publicContactSharing = com.synonym.paykit.PublicContactSharingPolicy.ENABLED, // test hook: retained-marker fixture
 )
 
 internal fun paykitPubkyClientConfig(
diff --git a/app/src/main/java/to/bitkit/services/PubkyService.kt b/app/src/main/java/to/bitkit/services/PubkyService.kt
index 18cf81711..04610523d 100644
--- a/app/src/main/java/to/bitkit/services/PubkyService.kt
+++ b/app/src/main/java/to/bitkit/services/PubkyService.kt
@@ -208,14 +208,17 @@ class PubkyService @Inject constructor(
     }
 
     suspend fun checkProfileDeletionAllowed() = ServiceQueue.CORE.background {
+        to.bitkit.dev.CleanupProbe.preflights++; to.bitkit.dev.CleanupProbe.note("profile preflight entered") // test hook
         paykitSdkService.checkProfileDeletionAllowed()
     }
 
     suspend fun removeContactsForProfile(publicKeys: List<String>): List<ContactRecord> = ServiceQueue.CORE.background {
+        to.bitkit.dev.CleanupProbe.removals++; to.bitkit.dev.CleanupProbe.note("profile contact batch entered") // test hook
         paykitSdkService.removeContactsForProfile(publicKeys)
     }
 
     suspend fun deletePaykitProfile() = ServiceQueue.CORE.background {
+        to.bitkit.dev.CleanupProbe.deletes++; to.bitkit.dev.CleanupProbe.note("profile delete entered") // test hook
         paykitSdkService.deletePaykitProfile()
     }
 
@@ -224,6 +227,7 @@ class PubkyService @Inject constructor(
     }
 
     suspend fun contactRecords(): List<ContactRecord> = ServiceQueue.CORE.background {
+        to.bitkit.dev.CleanupProbe.reads++ // test hook
         paykitSdkService.contactRecords()
     }
 

Decisive refreshed inspector lines (native roles and proxy responses checked as described above):

3-payee: Session true; contacts 2; overrides 2; sharing true/true; reads 0, removals 0, deletes 0; success false; success toasts 0; preflights 1, pauses 0, withdrawals 0; active subscriptions 1; linked 1; blocked 0
3-payer: Session true; contacts 2; overrides 2; sharing true/true; reads 0, removals 0, deletes 0; success false; success toasts 0; preflights 1, pauses 0, withdrawals 0; active subscriptions 1; linked 1; blocked 0
3-ended-payee: Session false; contacts 0; overrides 0; sharing false/false; reads 1, removals 1, deletes 1; success true; success toasts 1; preflights 3, pauses 1, withdrawals 1
3-ended-payer: Session false; contacts 0; overrides 0; sharing false/false; reads 1, removals 1, deletes 1; success true; success toasts 1; preflights 3, pauses 1, withdrawals 1
subscription-read: Session true; contacts 2; overrides 2; sharing true/true; reads 0, removals 0, deletes 0; success false; success toasts 0; preflights 1, pauses 0, withdrawals 0
1-unknown: Session true; contacts 2; overrides 2; sharing true/true; reads 1, removals 1, deletes 0; success false; success toasts 0; preflights 2, pauses 1, withdrawals 1
1-partial: Session true; contacts 1; overrides 1; sharing true/true; reads 1, removals 1, deletes 0; success false; success toasts 0; preflights 2, pauses 1, withdrawals 1
2-404: Session true; contacts 0; overrides 0; sharing true/true; reads 1, removals 1, deletes 1; success false; success toasts 0; preflights 2, pauses 1, withdrawals 1

Reviewed by gpt-6.1-sol-high

@ovitrif ovitrif left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

utAck

@piotr-iohk piotr-iohk left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

QA review

Scope: full reassessment of the whole PR at 7afdd3e, compared with its merge base 5fe0bf6. That merge base is the head of dependency #1688. A full reassessment was needed because the base changed since the earlier assessment (99b446a → 5fe0bf6) and the target-branch changes touch PubkyRepo.kt and PubkyRepoTest.kt. No coverage was carried over.

1 actionable finding — resolve or provide an evidence-backed rebuttal.

The stricter cleanup handling works as intended. A failed private-endpoint withdrawal, an unreadable contact list, a partial or unknown contact removal, or a profile-deletion error now stops deletion before sign-out. Only contacts confirmed as removed have their local data and overrides cleared. If clearing local state fails after deletion, no success effect is emitted. Because Bitkit sets PrivateOnly contact sharing by default, the SDK should not keep contacts behind public markers. One gap remains in the new active-subscription preflight: it blocks deletion on fixed-term subscriptions, but Bitkit gives users no way to cancel those (see finding). iOS twin #1124 at a2b2186 has the same mechanism: its expiry predicate is the same, and SubscriptionsView also hides Cancel when endsAt is set.

Validation: source analysis at the pinned revisions, including the SDK contract in paykit-rs v0.1.1-rc2 (01de5dd), which is the version pinned in the catalog. That covers contact_records, remove_contacts_and_block_peers skip rules, payment_requests and cancel_payment_request. No Android SDK was available locally, so I did not run any tests. At this head, hosted CI and lint passed. The new repository, service and view-model tests were inspected but not run by me. Journey J1 and manual tests 1–3 need the authenticated fault/subscription fixture and have not been run.

Device testing: not performed in this review.

Suggested additional test cases

  • Android: accept an incoming recurring subscription whose terms include a future ends_at (from an external issuer, disposable identities). Open Subscriptions and confirm no Cancel action is offered. Then try Edit Profile → Delete (or Unlink for a Ring identity). Expected: the user gets a way forward (for example a Disconnect option or guidance that matches the subscription's end date), not an instruction they cannot follow.
  • Android: make closeAndClear fail after a successful remote deletion. Expected: the error toast appears, the screen stays open, and a second Delete tap leads to the failure dialog, where Disconnect clears local state and leaves the profile screens.

Earlier findings reconciled at this revision: 1 still present, 1 verified fixed.

Findings

  • [MEDIUM] Fixed-term subscriptions block profile removal, but Bitkit offers no way to end them — inline at app/src/main/java/to/bitkit/services/PaykitSdkService.kt:772.

Existing concerns

Reviewer model: claude-opus-5-5
Reasoning effort: high

operationLock.withLock {
withStateRevisionTracking { handle ->
val now = nowMillis()
if (completeSdkCall { handle.paymentRequests() }.any { it.hasActiveSubscription(now) }) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[MEDIUM] Fixed-term subscriptions block profile removal, but Bitkit offers no way to end them

Trigger: the user has an accepted recurring subscription (as payer or payee) whose terms carry a future ends_at. Bitkit accepts incoming proposals like this: PaykitPaymentRequestRepo.accept has no endsAt guard, and SubscriptionsScreen shows them with an expiry date. The user then confirms Delete Profile, or Unlink for a Ring identity.

Mechanism: checkProfileDeletionAllowed throws ActiveSubscription for any ACTIVE_RECURRING record where endsAt == null || endsAt > now. An unparseable endsAt also counts as active. EditProfileViewModel.attemptDeleteProfile (line 277) and PubkyRepo.deleteProfile (line 1108) both call it. On that error the view model shows only the profile__delete_active_subscription toast ("End active subscriptions before removing this profile.") and does not open the failure dialog. That dialog is the only place disconnectProfile can be reached (EditProfileScreen.kt:199-206). Meanwhile, the unchanged PaykitSubscription.canCancel (PaykitSubscription.kt:223) returns false whenever recurrence.endsAt != null. PaykitPaymentRequestRepo.cancel (line 1335) enforces this, and SubscriptionsScreen.kt:652 hides the Cancel button.

Consequence: this path would leave the user unable to delete or unlink the profile, and without the Disconnect fallback, until the term expires or the counterparty cancels. If ends_at cannot be parsed, removal is blocked permanently. The instruction in the toast cannot be followed in the app. No funds or data are lost.

Expected: the preflight and the cancellation policy should agree. Either let fixed-term subscriptions be ended in the app, or for subscriptions the user cannot end, show their end date and still offer a recovery path (for example the Disconnect option). Manual case 3 itself expects the user to "End the subscription and confirm again." The new test profile removal conservatively blocks unknown or future subscription expiry confirms the blocking side, but no test covers whether such a subscription can be ended.

Evidence basis: source analysis at 7afdd3e and paykit-rs v0.1.1-rc2. I did not execute any tests. iOS twin #1124 (a2b2186) has the same mechanism in PaykitSdkService.requireNoActiveSubscriptions together with SubscriptionsView.swift:509.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The parent guidance repair is published in 98797f2, composed with current #1688 in 815e85c. It preserves the fixed-term cancellation policy and conservative removal guard. The typed profile refusal shows the latest end time only when every blocking obligation has a valid finite expiry; unreadable/missing expiry stays unknown and blocking. Final combined-tree validation passed 4,721 units plus compile/format/Detekt/Android Lint with zero findings; all seven live-base commits are verified. The fresh original-owner local-only Disconnect option is still being composed and validated in child #1693, so I am keeping this thread open until that complete path and its applicable acceptance are verified. No authenticated fixed-term journey pass is claimed yet.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Checked at 815e85c with authenticated native payer/payee wallets: two finite obligations show the latest expiry before any cleanup, and adding an open-ended obligation removes the date. Fresh confirmations succeed after open-ended cancellation and natural finite expiry. The guidance part holds, but this head still has no independent local Disconnect; the recovery described in child #1693 is not part of this head. Leaving this thread open.

@piotr-iohk piotr-iohk left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

QA review

Scope: Re-reviewed the full PR at 815e85c after the comparison base changed from 5fe0bf6c to 2aac53ed. No prior coverage was inherited.

1 actionable finding — resolve or provide an evidence-backed rebuttal.

Profile deletion stops before sign-out when the subscription read, private endpoint withdrawal, contact read, or contact acknowledgement is incomplete. An active payer or payee recurring subscription, including one with no saved contact, is refused before that cleanup, and the toast shows the latest finite expiry only when every blocking record has a readable end. Ordinary bulk contact removal still skips subscribed keys and continues with the others. A 404-like profile deletion error stays a failure and does not sign out. That matches bitkit-android#1688 at 2aac53ed, which removed session-retry deletion: this head calls deleteProfile and does not sign in again after an uncertain response.

Compared with bitkit-ios#1124 at a2b21861, stopping on incomplete cleanup and blocking on active subscriptions match. That iOS profile toast still says to end subscriptions and does not show an expiry; this Android head shows “Active subscriptions prevent removing this profile.” plus the latest finite expiry, which is the fixed-term behavior in this PR. The journey files differ only in that refusal action.

Device testing: not performed in this review. J1 and manual cases 1–3 still need the authenticated fault proxy and disposable subscription fixture named in the PR. PubkyRepoTest.kt, PaykitSdkServiceTest.kt, and EditProfileViewModelTest.kt were inspected and not executed here. Hosted build and lint had passed on this head; e2e shards were still pending.

Non-blocking consistency suggestions

  • At iOS a2b21861, profile refusal uses “End active subscriptions before removing this profile.” and omits the expiry. Android shows the prevent-removal sentence and the latest finite expiry. Align the twin profile toast and profile-cleanup-failure.xml action when that fixed-term guidance lands.

Earlier findings reconciled at this revision: 2 still present.

Findings

  • [MEDIUM] Fixed-term subscriptions block profile removal, but Bitkit offers no way to end them — inline at app/src/main/java/to/bitkit/services/PaykitSdkService.kt:796. Still present from the earlier review.

Existing concerns

Reviewer model: grok-4.7
Reasoning effort: high

if (subscriptions.isEmpty()) return
val endDates = subscriptions.mapNotNull { it.subscriptionEndsAtMillis() }
val latestEnd = if (endDates.size == subscriptions.size) endDates.maxOrNull() else null
throw PubkyContactError.ActiveProfileSubscriptions(latestEnd)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[MEDIUM] Fixed-term subscriptions block profile removal, but Bitkit offers no way to end them

Trigger: the user has an accepted recurring subscription (as payer or payee) whose terms carry a future ends_at. Bitkit accepts incoming proposals like this: PaykitPaymentRequestRepo.accept has no endsAt guard, and SubscriptionsScreen shows them with an expiry date. The user then confirms Delete Profile, or Unlink for a Ring identity.

Mechanism: checkProfileDeletionAllowed throws ActiveSubscription for any ACTIVE_RECURRING record where endsAt == null || endsAt > now. An unparseable endsAt also counts as active. EditProfileViewModel.attemptDeleteProfile (line 277) and PubkyRepo.deleteProfile (line 1108) both call it. On that error the view model shows only the profile__delete_active_subscription toast ("End active subscriptions before removing this profile.") and does not open the failure dialog. That dialog is the only place disconnectProfile can be reached (EditProfileScreen.kt:199-206). Meanwhile, the unchanged PaykitSubscription.canCancel (PaykitSubscription.kt:223) returns false whenever recurrence.endsAt != null. PaykitPaymentRequestRepo.cancel (line 1335) enforces this, and SubscriptionsScreen.kt:652 hides the Cancel button.

Consequence: this path would leave the user unable to delete or unlink the profile, and without the Disconnect fallback, until the term expires or the counterparty cancels. If ends_at cannot be parsed, removal is blocked permanently. The instruction in the toast cannot be followed in the app. No funds or data are lost.

Expected: the preflight and the cancellation policy should agree. Either let fixed-term subscriptions be ended in the app, or for subscriptions the user cannot end, show their end date and still offer a recovery path (for example the Disconnect option). Manual case 3 itself expects the user to "End the subscription and confirm again." The new test profile removal conservatively blocks unknown or future subscription expiry confirms the blocking side, but no test covers whether such a subscription can be ended.

Evidence basis: source analysis at 7afdd3e and paykit-rs v0.1.1-rc2. I did not execute any tests. iOS twin #1124 (a2b2186) has the same mechanism in PaykitSdkService.requireNoActiveSubscriptions together with SubscriptionsView.swift:509.

Still present at 815e85c (first reported at 7afdd3e): At 815e85c a future finite endsAt still refuses Delete Profile and Unlink before cleanup, and Bitkit still offers no Cancel for that subscription. The toast now says active subscriptions prevent removal and, when every blocking record has a readable finite end, adds the latest expiry. An unparseable endsAt still blocks with no date. The failure dialog stays closed, so Disconnect is still unreachable on this path. The open thread #1690 (comment) records the author's plan to add that recovery in #1693; that follow-up is not in this head. No tests were executed.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The local-only recovery path is now published in #1693: product commit 0dcad96, composed with this parent at 564a54d. Edit Profile offers a fresh explicit Disconnect from this device confirmation without first attempting deletion or entering the failure dialog. It binds the original owner/UUID, performs no remote cleanup, preserves strict credential-before-watermark cleanup, and leaves fixed-term cancellation policy unchanged. Final combined-tree validation passed 4,867 tests and required format/compile/lint; all nine live-base signatures verify locally and on GitHub. The dedicated authenticated fixed-term/refusal/Disconnect/restart journey is still unfinished, and new hosted CI is pending, so this thread remains open. The separate original-grant remote-retry finding is also not claimed fixed. Manual case 3 now consistently says active-subscription refusal; its existing fixed-term/open-ended distinctions are preserved.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Confirmed the finite-expiry guidance at 815e85c on both native roles, with contacts/overrides/session/sharing retained and zero cleanup. This head still does not offer Disconnect on subscription refusal; the separately published local-only recovery is in child #1693. This review does not certify the child, so the finding remains open here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The missing local recovery path is now verified on the final composed child #1693 at a5cd94f, which includes this parent 2e79e10. In the installed RC4 app, one genuine future fixed-term Payer subscription caused the localized expiry refusal; Cancel retained credentials and sharing, a fresh local Disconnect returned to onboarding, and a cold restart remained disconnected through the final 103-second UI/state observation. The same subscription stayed active/unpaid. From immediately before local confirmation through the final snapshot, all 12 selected server entries, 139 event rows and two grants were identical. Earlier background publications and setup corrections remain recorded; zero rejected HTTP requests, Ring, actual expiry and full J4 are not claimed.

The independent full-log/state audit is sealed (eb37c918dfbb615e5ac922a1ea009b50338ee6ae855518fd8811bf7e0708af57), and child hosted CI passed 5,135 units, 274 Compose cases and 39 selected E2E cases with zero static findings. Its PR description contains the exact APK and evidence receipts. The local Disconnect implementation remains in the child, not this parent's tree; I am retaining this thread as that integration dependency rather than claiming the parent independently contains it.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Checked at 2e79e10: this head is a base merge and the PR's own patch is unchanged, so checkProfileDeletionAllowed still refuses every active recurring record and the refusal still offers no Disconnect here. The local-only recovery you describe lives in #1693, which this review doesn't certify, so the finding stays open on this parent.

@piotr-iohk piotr-iohk left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

QA review

Scope: Full reassessment of the whole PR diff against its merge base 3d8a328d, at 2e79e109. A full reassessment was needed because the comparison base moved from 2efaff04 to 3d8a328d. No earlier coverage is inherited. Coverage includes the Edit Profile delete/unlink flow in EditProfileViewModel, the active-subscription preflight and the strict contact and profile cleanup in PubkyRepo, the shared subscription predicate and profile contact batch in PaykitSdkService, the PubkyService wrappers, the Disconnect local-finalization change, all 16 locale strings, the new journey, the changelog and the three updated unit-test classes. The parity check covered the deletion paths of the iOS twin bitkit-ios#1124 at 3991f3a2. Dependency #1688 is this PR's base and merge base at 3d8a328d, so it is covered by the diff.

Code review completed with unverified earlier concerns.

The SDK claims were checked against paykit-rs v0.1.1-rc2 (01de5dd2), the version this PR builds against. In that source fetch_paykit_profile returns no profile on a 404, and deletePaykitProfile skips the delete when nothing is found. Removing the error-text check therefore doesn't block a retry after a partly finished deletion. remove_contacts_and_block_peers skips any contact while another contact operation holds it (a lease that expires after 60 s), and the new all-or-nothing check turns that skip into a stopped deletion instead of a silent continue. iOS matches Android on the order of operations: the subscription check runs before endpoint withdrawal, endpoint and contact failures stop deletion, and the profile delete no longer ignores 404 errors.

Non-blocking consistency suggestion: the expiry on the active-subscription refusal shows date and time on Android (toLocalizedTimestamp, LONG date + SHORT time) but date only on iOS (.long, time omitted). Consider using the same format on both.

Validation: no tests or builds were run in this review, and no device was used. Hosted checks at this head passed: unit build-and-test, Compose UI and build, lint and E2E. Migration jobs were skipped. Those are the stock suites; they don't run the new cleanup-failure journey. The author's device results are tied to 7040c899.

Device testing: not performed in this review.

Suggested additional test cases

  • Android, existing journeys/profile/delete-profile.xml: import 62 contacts, then immediately go to Edit Profile and confirm Delete Profile. Deletion should still reach onboarding, or else show the failure dialog with every unremoved contact kept and no profile DELETE. If it fails, Retry after about 60 s should succeed. Contact cleanup now stops on any contact the SDK skips because another operation holds it.
  • Android, Ring-adopted identity (Unlink pubky) with an active open-ended subscription: confirm Unlink. The localized active-subscription refusal should appear with no Retry dialog, and endpoints and contacts should be unchanged. The author's runs list no alternate Unlink route as covered.
  • Android, an active subscription whose counterparty is not a saved contact: confirm Delete Profile. The refusal should appear before any endpoint withdrawal.

Earlier findings reconciled at this revision: 2 still present, 1 could not be verified.

Reconciliation incomplete: the following earlier concerns remain unverified. This report does not establish that they are fixed or invalid, or establish approval or device readiness.

Could not verify earlier concern — Supporting records for external device reports remain unverified: This is an evidence limitation, not a code defect, so it has no introduced/worsened code mechanism to anchor. The full snapshot shows no later reply supplying the fixture-control patch, build/install/native-artifact receipts or per-case records requested in issuecomment-6569526757, so source review cannot authenticate the ovi-reviewer device runs (2daec90, 815e85c, 7ef3432, 7040c89). Their video src values still carry a local-path prefix. Since then, jvsena42 posted an approval at 2e79e10 with their own device-gate results run at 7040c89 (actions 1-6, 10, 11, plus a closest-case 9). That review explicitly marks actions 7, 8, 9-as-written and the expiry-date refusal as not run by them, and relies on ovi-reviewer's 7ef3432 report for those. The author's 2026-11-03 thread reply reports a fixed-term Disconnect run on child #1693, not on this parent. All of these are attributed external reports, not executions performed here and not verified acceptance evidence. This review ran no device or tests, so the reports remain unverified and must be disclosed as a limitation.

Existing concerns

Reviewer model: claude-opus-5-5
Reasoning effort: high

@ovi-reviewer ovi-reviewer Bot 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.

Verdict: 💬 Comment. No new defects. The three driven cases pass; the per-contact busy lease was not reproduced. The fixed-term subscription finding (MEDIUM) is still open on this PR; its recovery lives in #1693.

Findings: No new defects in this delta.

This push is one merge of the new base (#1688 at 3d8a328, with master 44bf50e). The PR's own patch is byte-identical to 7040c89.

The base move leaves PubkyRepo, PrivatePaykitRepo, PaykitSdkService, PubkyService and the native pins unchanged. In the PR's files it only adds locale strings under other keys. I checked its allowance, backup/restore and wallet code against deletion:

  • Allowances pay only non-recurring requests, so the subscription check neither misses nor overcounts them.
  • A stopped deletion touches no allowance or restore state.
  • No new caller reaches deleteProfile, checkProfileDeletionAllowed or removeContactsForProfile.

Still open at this head:

QA for 2e79e10:

  • Setup: Android 16 / API 36 emulators running a dev debug build of this head. The APK carries E2E=true and an unpublished debug probe, which provisions disposable Pubky identities against the local Pubky testnet and the homeserver fault proxy.
  • Installed arm64 APK SHA-256: 3bb29a91d793b7f9729edfb6948abbfda894cf8c8b0c31001ea6079cda19f8a2.
  • The probe hosts the unchanged EditProfileScreen and EditProfileViewModel, and adds read-only counters on the preflight, contact read/removal, profile delete, publication pause and endpoint withdrawal calls.
  • The subscription in the first two cases is a real native open-ended subscription that wallet A proposed and wallet B accepted.
Unlink of a Ring-linked pubky is refused while a subscription is active ✅ Unlink shows the subscription refusal with no date, and nothing is withdrawn, removed or deleted.
  • Wallet A is the payee. The probe writes the Ring source marker for A's own key, so the screen shows Unlink pubky and Unlink pubky? / Yes, Unlink.
  • Confirming shows the toast "Active subscriptions prevent removing this profile." with no description, and no failure dialog.
  • The counters, before → after: preflights 0 → 1. Contact reads, removals, profile deletes, pauses and withdrawals all stay at 0.
  • The session, both sharing flags, both contacts and overrides, and the active subscription are all unchanged.
unlink-refused.2x.mp4
Delete is refused when the subscription's counterparty is not a saved contact ✅ The payer gets the same refusal before any cleanup, and keeps its one unrelated contact.
  • Wallet B is the payer. It is linked to A but never saved A as a contact: its contact list holds only one unrelated contact, alongside one linked peer and the active subscription.
  • Delete Profile → Yes, Delete shows the same refusal toast with no date.
  • Preflights go to 1, and every cleanup counter stays at 0. The contact, override, session and sharing flags are kept.

This matches the code: checkProfileDeletionAllowed scans every payment request and doesn't consult contacts.

payer-not-contact.2x.mp4
Delete during another operation's lock stops, and Retry after the lock expires succeeds ✅ Delete fails safely while the shared state is locked, and Retry after expiry deletes the profile.
  • Setup: the proxy held A's private-message PUT to B, and the app was force-stopped mid-write. That left the Paykit shared-state lock held.
  • After a normal relaunch I confirmed Delete 29 s after the kill. The preflight waited on the lock, and Unable to Delete Profile with Disconnect / Retry appeared at about 51 s.
  • The log shows shared_state_busy at the subscription preflight. Contact reads, removals, profile deletes, pauses and withdrawals stayed at 0, and the session, sharing and both overrides were kept.
  • The video shows contacts 0 before Delete. That is the in-memory list: contact loading after relaunch also hit the lock. Nothing was read or removed.
  • This is the first device run of an unreadable preflight. Earlier it was a component check only.
  • Retry at 75 s deleted the profile within 15 s: one contact read, one removal, one profile delete, one success toast, signed out, sharing off.
busy-contact.2x.mp4
Check: earlier device reports still hold at this head ✅ The cleanup code they exercised and the Paykit pin are unchanged since 7ef3432.

Across 7ef3432..2e79e10, deleteProfile, deleteAllContacts, both checkProfileDeletionAllowed, removeContacts(requireAll), hasActiveSubscription, attemptDeleteProfile, disconnectProfile, removePublishedEndpointsForCleanup and beginProfileDeletion are unchanged. The only differences in those files are allowance admission and a startup profile load.

These reports carry over:

  • ovi-reviewer at 7ef3432: finite-expiry payer/payee refusal, open-ended date suppression, removal after obligations end, private-withdrawal failure, unconfirmed and partial contact removal, profile DELETE 404, failed local finalization.
  • ovi-reviewer at 7040c89: Cancel and cold restart with no deletion replay.
  • jvsena42 at 7040c89: actions 1–6, 10 and 11 with airplane mode as the fault.

The 7040c89 report's video is https://github.com/user-attachments/assets/b3d9afc3-55d3-4422-be71-88dd12c3b9bb. Its src there carried a stray local prefix.

Cases from piotr-iohk's suggestions: the Unlink and non-contact-counterparty refusals are the first two cases above.

Not driven: a contact held by another operation stops deletion ⏭️ (piotr-iohk's 62-contact suggestion).

  • One try: holding A's private-message PUT to B and force-stopping mid-write held the whole shared-state lock, not a per-peer lease, so no contact was skipped.
  • The Kotlin stop on a skipped contact (removedKeys != keys, PubkyRepo.kt:1156) is the path the 7ef3432 partial-contact case drove.
  • Retry after expiry is shown by the lock case above.

Review covered the 7040c89..2e79e10 delta and the open findings. No device case drove the base's allowance, backup or restore changes; they are outside this PR's flow.

Twin: bitkit-ios#1124 is now at e8aaa4b. It moved only by backup-preservation merges since 3991f3a, so it still matches on strict cleanup, early subscription refusal and latest-expiry selection. It formats only the date, where Android shows date and time.


Reviewed by claude-opus-5-5-medium

@piotr-iohk piotr-iohk left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

QA review

Scope: Re-reviewed the full PR at b2c4b9e after the merge base changed since the saved assessment. No earlier coverage was inherited.

1 actionable finding — resolve or provide an evidence-backed rebuttal.

The new subscription refusal is not held for the profile DELETE itself. A foreground payment-request receive waiting on the Paykit operation lock can commit an active subscription after the preflight read, and profile deletion still continues when no contact batch runs. The iOS twin at 21d6a22 was compared only for this cleanup flow. bitkit-android#1688 is the merge base and was not reviewed as its own change.

Device testing: not performed in this review. PubkyRepoTest.kt, PaykitSdkServiceTest.kt, and EditProfileViewModelTest.kt were inspected and not executed.

Suggested additional test cases

  • Android: disposable identity with no saved contacts, foreground payment-request polling running. Confirm Delete Profile while an inbox receive is in flight and let that receive commit one active payer or payee subscription after the preflight read. Expected: the profile and session remain, the active-subscription message is shown, and there is no success navigation.

Earlier findings reconciled at this revision: 2 still present, 1 could not be verified.

Reconciliation incomplete: the following earlier concerns remain unverified. This report does not establish that they are fixed or invalid, or establish approval or device readiness.

Could not verify earlier concern — Supporting records for external device reports remain unverified: This is an evidence limitation, not a code defect. The author request in #1690 (comment) still has no reply attaching the fixture-control patch, build/install/native-artifact receipts, and per-case proxy/state records for the 7040c89 report and the retained 7ef3432 cases. Review #1690 (review) later reports three device cases at 2e79e10 with an APK SHA-256, counters, and GitHub-hosted videos. It does not include that fixture-control patch or those requested receipts. jvsena42's approval text says the device gate ran at 7040c89 for actions 1-6, 10, 11, and a closest-case 9, and marks actions 7, 8, 9-as-written and the expiry-date refusal as not run by them. The review object is currently associated with commit b2c4b9e. Those are attributed reports. This reconciliation ran no device or unit test, so the reports remain unverified.

Findings

  • [MEDIUM] Profile delete can proceed after a late active subscription — inline at app/src/main/java/to/bitkit/repositories/PubkyRepo.kt:1130.

Existing concerns

Reviewer model: grok-4.7
Reasoning effort: high

@ovi-reviewer ovi-reviewer Bot 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.

Verdict: ⛔️ Request changes — a late subscription still allows profile deletion, confirmed in the existing finding.

The PR moved to 8ceaca2 after this run; these results are for b2c4b9e.

Findings: No new defects introduced by this delta. The existing MEDIUM late-subscription gap is reproduced: PaykitSdkService.kt:653 deletes without checking subscriptions under the same operation lock. An accepted recurring obligation received after both preflights therefore survives until profile DELETE and sign-out. The empty-contact path skips the contact-batch check. This implementation is unchanged from the base and the previous reviewed head; the case below adds runtime evidence to the existing thread. Recheck active subscriptions inside the final locked delete operation.

The fixed-term subscription recovery finding also remains open on this parent. Independent local Disconnect belongs to bitkit-android#1693 fix: bind profile removal to its original owner, which this review does not certify. The publication-pause thread is resolved; I have left it closed.

QA for b2c4b9e: Android 16 / API 36, unfunded disposable wallets and authenticated native Pubky identities on a local testnet. I built the unmodified head, then built the unpublished test controls below. All native libraries match the unmodified APK, and installed APK hashes match their builds. The debug host renders the unchanged Edit Profile screen and ViewModel; its success marker displays the actual exit effect.

Delete immediately after importing 62 contacts ✅ Fresh confirmation completes contact and profile deletion.

The normal repository importer saved 62 synthetic contact records. I started ordinary foreground preparation and confirmed deletion about 12 seconds later. The inspector changed from 62 contacts/62 overrides to zero, with one contact batch, one profile delete, signed-out session, sharing off and one success effect. The proxy recorded profile DELETE 204 and session-grant revocation. Preparation logged missing remote Noise-authorization warnings for the generated keys; no lease blocked this batch, so this is not a lease-expiry Retry execution.

contacts-immediate2x.2x.mp4

A separate normal cold launch reached Home, then Profile onboarding and New Pubky, confirming the deleted profile did not return. The USDT introduction was declined before opening Profile.

contacts-onboarding2x.2x.mp4

Both recordings are at 2× speed.

An active subscription arriving after preflight prevents deletion ❌ The profile is deleted despite a newly active subscription.

Two native wallets linked without saving each other as contacts. Wallet A proposed an open-ended subscription whose first period starts tomorrow. While its proposal was still inactive, I confirmed Delete Profile and held the final service wrapper immediately before the SDK lock. Logs and the inspector confirmed both preflights had completed, zero active subscriptions, zero contacts, and no profile DELETE.

Wallet B then accepted that exact proposal. On A, the real background-priority inbox receive/observation primitive received the acceptance and committed one active Payee subscription. Before releasing deletion, logs and the refreshed inspector confirmed the session remained authenticated, with that active obligation and no profile DELETE. Releasing the hold produced profile DELETE 204, sign-out and success. B's final native read still showed its active Payer obligation.

late-subscription2x.2x.mp4

At 2× speed. The hook stages the ordering explicitly; it does not replace SDK results or deletion decisions. It invokes the same inbox primitive used by polling, rather than reproducing the natural AppViewModel scheduling race. The missing refusal is the app failure.

Check: unmodified build and affected component regressions ✅ The build and focused suites pass.

assembleDevDebug and focused testDevDebugUnitTest passed for the profile repository/ViewModel, Paykit SDK, backup repository, Lightning repository, wallet-connection recovery and VSS clients/coordinator. JUnit XML contains no failures, errors or skips. These include the admitted-reservation cancellation regression. Full unit, lint and instrumentation suites were not rerun here. Native-symbol stripping, JVM and Gradle-deprecation notices remain in the build output. Existing component checks do not cover the late native acceptance above.

Check: retained device evidence ✅ Earlier reports cover the unchanged cleanup paths and the inherited backup status repair.
  • Cleanup report at 7ef3432: finite payer/payee expiry, open-ended date suppression, removal after obligations end, private withdrawal failure, unknown/partial contact removal, DELETE404 and failed local finalization.
  • Cold-start report at 7040c89: Cancel and normal restart without deletion replay.
  • Report at 2e79e10: Unlink refusal, a subscription with a non-contact counterparty, unreadable preflight and Retry after shared-state lock expiry.
  • Independent device gate: open-ended refusal/removal and offline stop/Cancel/restart.
  • bitkit-android#1686 fix: decouple wallet restore from profile refresh: the author's J3 at a2c58f2 shows Settings Running during its queued backup, then Latest Backup after acknowledgement. Its seven production inputs and journey match this head. This supersedes the earlier failed status case; that failure is not counted as a pass.

These are attributed original-head results. Their raw historical artifacts were not reauthenticated here; the existing evidence request remains distinct from this run's receipts. The author also reports a real per-contact lease causing partial removal at 7040c89 in the PR description. This run adds the 62-contact success variant, not a forced per-contact lease or Retry result.

Review covered the contiguous 2e79e10..b2c4b9e delta, all changed production code and reached ownership/cleanup callers, with an independent reviewer. The 25 cleanup-specific paths and native pins are unchanged. The incoming production code matches bitkit-android#1688 fix: stop automatic profile deletion replay at a8c3fd3. No planned case remains unrun. Held native VSS cancellation/shutdown and imported routing-reset/recovery flows were not separately driven by these cases; their relevant component checks ran.

Twin: bitkit-ios#1124 fix: stop deletion after incomplete cleanup at 21d6a22 matches early subscription refusal and strict cleanup. It additionally rechecks subscriptions inside final SDK deletion and for an empty strict batch. Its expiry toast shows date only; Android includes the time. No new iOS device result is claimed.

Unpublished test controls and binary receipts

Not pushed. The shared fixture provisions disposable native identities, imports contacts through the real repository, exposes counters/state, and hosts the actual Edit Profile screen. The late-case increment adds a releasable hold before the final SDK lock and an explicit call to its real inbox receive/observation primitive. Native responses and product decisions remain unchanged.

  • Contact/import and normal-start APK SHA-256: 06e66ca675a70a7dafcd520623dbe90b1ac5f742998c3584c0c9359222d2f963.
  • Late-subscription APK SHA-256, verified on both installed copies: 0d1e50e867b7112c231aec5683cd7dccae8dae7cc6382df379e6c71fad1e2aa0.
  • Every packaged native library matches the unmodified head build in both variants.

Shared fixture patch:

diff --git a/app/src/debug/AndroidManifest.xml b/app/src/debug/AndroidManifest.xml
index b88fe73a7..610556f22 100644
--- a/app/src/debug/AndroidManifest.xml
+++ b/app/src/debug/AndroidManifest.xml
@@ -6,6 +6,8 @@
         android:usesCleartextTraffic="true"
         tools:ignore="MissingApplicationIcon">
 
+        <activity android:name="to.bitkit.dev.CleanupProbeActivity" android:exported="true" android:launchMode="singleTop" /> <!-- unpublished QA hook -->
+
         <provider
             android:name="to.bitkit.dev.DevToolsProvider"
             android:authorities="${applicationId}.devtools"
diff --git a/app/src/main/java/to/bitkit/repositories/PrivatePaykitRepo.kt b/app/src/main/java/to/bitkit/repositories/PrivatePaykitRepo.kt
index f6c18ad46..ba90d7f8b 100644
--- a/app/src/main/java/to/bitkit/repositories/PrivatePaykitRepo.kt
+++ b/app/src/main/java/to/bitkit/repositories/PrivatePaykitRepo.kt
@@ -375,6 +375,7 @@ class PrivatePaykitRepo @Inject constructor(
     }
 
     suspend fun beginProfileDeletion() = withContext(serializedDispatcher) {
+        to.bitkit.dev.CleanupProbe.pauses++ // unpublished QA counter
         _isProfileDeletionInProgress.update { true }
         invalidateContactPreparation()
     }
@@ -528,6 +529,7 @@ class PrivatePaykitRepo @Inject constructor(
         deferAppSync: Boolean,
     ): Result<Unit> = withContext(serializedDispatcher) {
         runSuspendCatching {
+            to.bitkit.dev.CleanupProbe.withdrawals++ // unpublished QA counter
             updateContactSharingCleanupPending(true)
             removePublishedEndpoints().getOrThrow()
             if (!deferAppSync) {
diff --git a/app/src/main/java/to/bitkit/services/PubkyService.kt b/app/src/main/java/to/bitkit/services/PubkyService.kt
index 18cf81711..0f098fbd1 100644
--- a/app/src/main/java/to/bitkit/services/PubkyService.kt
+++ b/app/src/main/java/to/bitkit/services/PubkyService.kt
@@ -208,14 +208,17 @@ class PubkyService @Inject constructor(
     }
 
     suspend fun checkProfileDeletionAllowed() = ServiceQueue.CORE.background {
+        to.bitkit.dev.CleanupProbe.preflights++ // unpublished QA counter
         paykitSdkService.checkProfileDeletionAllowed()
     }
 
     suspend fun removeContactsForProfile(publicKeys: List<String>): List<ContactRecord> = ServiceQueue.CORE.background {
+        to.bitkit.dev.CleanupProbe.removals++ // unpublished QA counter
         paykitSdkService.removeContactsForProfile(publicKeys)
     }
 
     suspend fun deletePaykitProfile() = ServiceQueue.CORE.background {
+        to.bitkit.dev.CleanupProbe.deletes++ // unpublished QA counter
         paykitSdkService.deletePaykitProfile()
     }
 
@@ -224,6 +227,7 @@ class PubkyService @Inject constructor(
     }
 
     suspend fun contactRecords(): List<ContactRecord> = ServiceQueue.CORE.background {
+        to.bitkit.dev.CleanupProbe.reads++ // unpublished QA counter
         paykitSdkService.contactRecords()
     }
 
diff --git a/app/src/debug/java/to/bitkit/dev/CleanupProbeActivity.kt b/app/src/debug/java/to/bitkit/dev/CleanupProbeActivity.kt
new file mode 100644
index 000000000..23f27d71f
--- /dev/null
+++ b/app/src/debug/java/to/bitkit/dev/CleanupProbeActivity.kt
@@ -0,0 +1,205 @@
+package to.bitkit.dev
+
+// test hook: disposable native fixture and inspector; deletion decisions use the unchanged screen/viewmodel/repositories.
+import android.os.Bundle
+import android.content.Intent
+import com.synonym.paykit.*
+import android.util.Log
+import androidx.activity.ComponentActivity
+import androidx.activity.compose.setContent
+import androidx.compose.foundation.layout.*
+import androidx.compose.material3.*
+import androidx.compose.runtime.*
+import androidx.compose.ui.Modifier
+import androidx.compose.ui.unit.dp
+import androidx.lifecycle.lifecycleScope
+import dagger.hilt.android.AndroidEntryPoint
+import kotlinx.coroutines.*
+import kotlinx.coroutines.flow.first
+import to.bitkit.data.PubkyStore
+import to.bitkit.data.SettingsStore
+import to.bitkit.data.keychain.Keychain
+import to.bitkit.repositories.*
+import to.bitkit.services.*
+import to.bitkit.ui.screens.profile.*
+import to.bitkit.ui.theme.AppThemeSurface
+import java.security.SecureRandom
+import javax.inject.Inject
+
+object CleanupProbe {
+    var finalizationGate: CompletableDeferred<Unit>? = null
+    var withdrawalGate: CompletableDeferred<Unit>? = null
+    var reads = 0
+    var removals = 0
+    var withdrawals = 0
+    var pauses = 0
+    var preflights = 0
+    var deletes = 0
+    fun note(s: String) { Log.i("CleanupProbe", s) }
+}
+
+@AndroidEntryPoint
+class CleanupProbeActivity : ComponentActivity() {
+    @Inject lateinit var service: PubkyService
+    @Inject lateinit var sdk: PaykitSdkService
+    @Inject lateinit var repo: PubkyRepo
+    @Inject lateinit var privateRepo: PrivatePaykitRepo
+    @Inject lateinit var pubkyStore: PubkyStore
+    @Inject lateinit var settings: SettingsStore
+    @Inject lateinit var keychain: Keychain
+    @Inject lateinit var wallet: WalletRepo
+    private var status by mutableStateOf("Preparing disposable native identity")
+    private var evidence by mutableStateOf("")
+    private var vm by mutableStateOf<EditProfileViewModel?>(null)
+    private var exited by mutableStateOf(false)
+    private var successToasts = 0 // test hook
+    private val toastQueue by lazy { to.bitkit.ui.shared.toast.ToastQueueManager(lifecycleScope) } // test hook: same app toast host
+
+    override fun onNewIntent(intent: Intent) {
+        super.onNewIntent(intent)
+        lifecycleScope.launch {
+            try {
+                val peer = intent.getStringExtra("peer").orEmpty()
+                when (intent.getStringExtra("action")) {
+                    "link" -> {
+                        if (!intent.getBooleanExtra("nocontact", false)) service.saveContact(peer, "Subscription peer", restorePrivateConnection = true)
+                        sdk.ensureLinkWithPeer(peer, 8u)
+                        sdk.processOutboundPrivateMessages(peer)
+                        sdk.receivePrivateMessages(peer)
+                        repo.loadContacts()
+                        pubkyStore.update { it.copy(contactProfileOverrides = repo.contacts.value.associate { c -> c.publicKey to c.toProfileData() }) }
+                    }
+                    "pump" -> {
+                        sdk.ensureLinkWithPeer(peer, 8u)
+                        sdk.receivePrivateMessages(peer)
+                        sdk.processOutboundPrivateMessages(peer)
+                    }
+                    "propose" -> {
+                        val now = java.time.Instant.now()
+                        val start = intent.getStringExtra("start") ?: now.plusSeconds(86400).toString()
+                        val end = if (intent.getBooleanExtra("open", false)) null else intent.getStringExtra("end") ?: start
+                        val proposal = PaykitPaymentRequestProposalTerms(
+                            amountValue = "0.00005", amountAsset = "btc",
+                            paymentReference = "cleanup-${java.util.UUID.randomUUID()}",
+                            proposalExpiresAt = now.plusSeconds(259200).toString(),
+                            acceptedPaymentEndpointIdentifiers = listOf("btc-lightning-bolt11"),
+                            metadataJson = """{"note":"Cleanup subscription","subscription":{"version":1,"description":"Disposable future subscription","benefits":[]}}""",
+                            recurrence = PaykitPaymentRequestRecurrenceTerms(1u, "month", start, start, end)
+                        )
+                        val record = sdk.proposePaymentRequest(peer, proposal, repo.publicKey.value!!)
+                        CleanupProbe.note("proposed ${record.paymentRequestId} state=${record.state}")
+                        sdk.processOutboundPrivateMessages(peer)
+                    }
+                    "accept" -> {
+                        sdk.receivePrivateMessages(peer)
+                        val record = sdk.paymentRequests().first { it.localRole == PaymentRequestLocalRole.PAYER && it.state == PaymentRequestLifecycleState.PROPOSED }
+                        val accepted = sdk.acceptPaymentRequest(peer, record.paymentRequestId, record.proposalStreamItemId!!)
+                        sdk.processOutboundPrivateMessages(peer)
+                        CleanupProbe.note("accepted ${accepted.paymentRequestId} state=${accepted.state}")
+                    }
+                    "cancel" -> {
+                        val record = sdk.paymentRequests().first { it.state == PaymentRequestLifecycleState.ACTIVE_RECURRING && it.terms?.recurrence?.endsAt == null }
+                        sdk.cancelPaymentRequest(record.counterparty, record.paymentRequestId, "Disposable cleanup test ended")
+                        sdk.processOutboundPrivateMessages(peer)
+                    }
+                    "hold-finalization" -> { CleanupProbe.finalizationGate = CompletableDeferred() }
+                    "release-finalization" -> { CleanupProbe.finalizationGate?.complete(Unit); CleanupProbe.finalizationGate = null }
+                    "hold-withdrawal" -> { CleanupProbe.withdrawalGate = CompletableDeferred() }
+                    "release-withdrawal" -> { CleanupProbe.withdrawalGate?.complete(Unit); CleanupProbe.withdrawalGate = null }
+                    "import62" -> {
+                        val profiles = (1..62).map { index ->
+                            val key = service.publicKeyFromSecret("%02x".format(index).repeat(32))
+                            to.bitkit.models.PubkyProfile.forDisplay(key, "Imported contact $index", null)
+                        }
+                        repo.importContacts(profiles).getOrThrow()
+                        pubkyStore.update { it.copy(contactProfileOverrides = repo.contacts.value.associate { c -> c.publicKey to c.toProfileData() }) }
+                        privateRepo.setContactPreparationActive(true) // test hook: foreground preparation
+                        privateRepo.scheduleSavedContactPreparation(repo.contacts.value.map { it.publicKey }).getOrThrow()
+                        CleanupProbe.note("Imported62 contacts=${repo.contacts.value.size}; starting normal contact preparation")
+                    }
+                    "inspect" -> Unit
+                }
+                status = "Native fixture action completed: ${intent.getStringExtra("action")}"
+                inspect()
+            } catch (e: Exception) {
+                status = "Native fixture action failed: ${e.message}"
+                CleanupProbe.note(status)
+                Log.e("CleanupProbe", "command", e)
+            }
+        }
+    }
+
+    override fun onCreate(savedInstanceState: Bundle?) {
+        super.onCreate(savedInstanceState)
+        setContent {
+            AppThemeSurface {
+                val currentToast by toastQueue.currentToast.collectAsState() // test hook
+                Column(Modifier.fillMaxSize().padding(top = 28.dp)) {
+                    Text(status, Modifier.padding(horizontal = 12.dp))
+                    Text(evidence, Modifier.padding(horizontal = 12.dp))
+                    Row {
+                        TextButton(onClick = { lifecycleScope.launch { inspect() } }) { Text("Inspect evidence") }
+                        TextButton(onClick = { vm?.dismissDeleteFailureDialog() }) { Text("Cancel failure") }
+                    }
+                    if (exited) Text("Deletion success navigation")
+                    vm?.let { current ->
+                        Box(Modifier.weight(1f)) {
+                            EditProfileScreen(current, onBackClick = {}, onExitProfile = { exited = true })
+                        }
+                    }
+                }
+                to.bitkit.ui.components.ToastOverlay(toast = currentToast, onDismiss = toastQueue::dismissCurrentToast) // test hook
+            }
+        }
+        lifecycleScope.launch { to.bitkit.ui.shared.toast.ToastEventBus.events.collect { toast -> toastQueue.enqueue(toast); if (toast.type == to.bitkit.models.Toast.ToastType.SUCCESS) successToasts++; CleanupProbe.note("toast ${toast.type}: ${toast.title}; description=${toast.description}") } } // test hook
+        lifecycleScope.launch {
+            try {
+                if (!keychain.exists(Keychain.Key.BIP39_MNEMONIC.name)) wallet.createWallet(null).getOrThrow() // test hook: unfunded wallet for normal cold startup
+                service.initialize()
+                if (intent.getBooleanExtra("resume", false)) {
+                    repo.initialize().getOrThrow()
+                    status = "Restored through normal profile initialization"
+                } else {
+                    val secret = ByteArray(32).also { SecureRandom().nextBytes(it) }.joinToString("") { "%02x".format(it) }
+                    val home = service.publicKeyFromSecret("00".repeat(32))
+                    service.signUp(secret, home, null)
+                    service.publishPaykitProfile(com.synonym.paykit.PaykitProfile("Disposable cleanup identity", null, null))
+                    repo.initialize().getOrThrow()
+                    val peers = emptyList<String>().map { service.publicKeyFromSecret(it.repeat(32)) }
+                    for ((index, peer) in peers.withIndex()) {
+                        service.saveContact(peer, "Peer ${index + 1}", restorePrivateConnection = true)
+                    }
+                    repo.loadContacts()
+                    pubkyStore.update { it.copy(contactProfileOverrides = repo.contacts.value.associate { c -> c.publicKey to c.toProfileData() }) }
+                    settings.update { it.copy(sharesPrivatePaykitEndpoints = true, sharesPublicPaykitEndpoints = true) }
+                    status = "Authenticated disposable identity"
+                }
+                CleanupProbe.reads = 0; CleanupProbe.removals = 0; CleanupProbe.deletes = 0; CleanupProbe.withdrawals = 0; CleanupProbe.pauses = 0; CleanupProbe.preflights = 0
+                if (intent.getBooleanExtra("ring", false)) keychain.upsertString(Keychain.Key.SHARED_PUBKY_SOURCE.name, to.bitkit.data.sharedpubky.SharedPubkyContract.RING_SOURCE_PREFIX + repo.publicKey.value!!) // test hook: Ring-adopted marker
+                CleanupProbe.note("ring adopted=${repo.isRingAdoptedIdentity()}")
+                vm = EditProfileViewModel(applicationContext, repo, privateRepo)
+                CleanupProbe.note("READY owner=${repo.publicKey.value}")
+                inspect()
+            } catch (e: Exception) {
+                status = "Provisioning failed: ${e.message}"
+                CleanupProbe.note(status)
+                Log.e("CleanupProbe", "setup", e)
+            }
+        }
+    }
+
+    private suspend fun inspect() {
+        val state = settings.data.first()
+        val session = keychain.loadString(Keychain.Key.PAYKIT_SESSION.name) != null
+        val overrides = pubkyStore.data.first().contactProfileOverrides.size
+        evidence = "Session $session; contacts ${repo.contacts.value.size}; overrides $overrides; sharing ${state.sharesPublicPaykitEndpoints}/${state.sharesPrivatePaykitEndpoints}; reads ${CleanupProbe.reads}, removals ${CleanupProbe.removals}, deletes ${CleanupProbe.deletes}; success $exited; success toasts $successToasts; preflights ${CleanupProbe.preflights}, pauses ${CleanupProbe.pauses}, withdrawals ${CleanupProbe.withdrawals}"
+        if (session && intent.getBooleanExtra("subscription", false)) {
+            val requests = sdk.paymentRequests()
+            val active = requests.filter { it.state == PaymentRequestLifecycleState.ACTIVE_RECURRING }
+            val peers = sdk.linkedPeers()
+            evidence += "; active subscriptions ${active.size}; linked ${peers.count { it.state == LinkedPeerState.LINKED }}; blocked ${peers.count { it.state == LinkedPeerState.BLOCKED }}"
+            CleanupProbe.note("owner=${repo.publicKey.value} contacts=${repo.contacts.value.map { it.publicKey }} overrides=${pubkyStore.data.first().contactProfileOverrides.keys} active=${active.map { Triple(it.localRole, it.counterparty, it.terms?.recurrence?.endsAt) }}")
+        }
+        CleanupProbe.note(evidence)
+    }
+}

Late-case increment over that fixture:

--- a/app/src/debug/java/to/bitkit/dev/CleanupProbeActivity.kt
+++ b/app/src/debug/java/to/bitkit/dev/CleanupProbeActivity.kt
@@ -27,6 +27,7 @@
 import javax.inject.Inject
 
 object CleanupProbe {
+    var profileDeleteGate: CompletableDeferred<Unit>? = null
     var finalizationGate: CompletableDeferred<Unit>? = null
     var withdrawalGate: CompletableDeferred<Unit>? = null
     var reads = 0
@@ -116,6 +117,11 @@
                         privateRepo.setContactPreparationActive(true) // test hook: foreground preparation
                         privateRepo.scheduleSavedContactPreparation(repo.contacts.value.map { it.publicKey }).getOrThrow()
                         CleanupProbe.note("Imported62 contacts=${repo.contacts.value.size}; starting normal contact preparation")
+                    }
+                    "hold-delete" -> { CleanupProbe.profileDeleteGate = CompletableDeferred() }
+                    "release-delete" -> { CleanupProbe.profileDeleteGate?.complete(Unit); CleanupProbe.profileDeleteGate = null }
+                    "receive-state" -> {
+                        sdk.receiveAndObserveState(repo.publicKey.value!!) { reports -> CleanupProbe.note("Native inbox received reports=${reports.size}") }
                     }
                     "inspect" -> Unit
                 }
--- a/app/src/main/java/to/bitkit/services/PubkyService.kt
+++ b/app/src/main/java/to/bitkit/services/PubkyService.kt
@@ -220,3 +220,4 @@
     suspend fun deletePaykitProfile() = ServiceQueue.CORE.background {
+        to.bitkit.dev.CleanupProbe.profileDeleteGate?.let { to.bitkit.dev.CleanupProbe.note("Held final profile deletion before SDK lock"); it.await() } // test hook
         to.bitkit.dev.CleanupProbe.deletes++ // unpublished QA counter
         paykitSdkService.deletePaykitProfile()

Decisive observed states:

After import: contacts 62; overrides 62; session true; sharing true/true
After deletion: contacts 0; overrides 0; session false; sharing false/false; removals 1; deletes 1; success true
Late case after both preflights: contacts 0; preflights 2; active subscriptions 0; deletes 0; session true
After real inbox acceptance: contacts 0; preflights 2; active subscriptions 1 (PAYEE); deletes 0; session true
After release: profile DELETE 204; session false; sharing false/false; deletes 1; success true
Counterpart final read: active subscriptions 1 (PAYER)

Reviewed by gpt-6.1-sol-high

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

Re-approving at 8ceaca2. I reviewed "fix: recheck subscriptions before profile deletion".

Checked and clean:

  • deletePaykitProfile now runs requireNoActiveSubscriptions(handle) inside operationLock and withPaykitKey, right before reading and deleting the profile. A subscription that turns active after checkProfileDeletionAllowed therefore stops the delete under the same lock. PubkyRepo.deleteProfile's recoverCatching maps the thrown ActiveProfileSubscriptions to the explanation the user already sees.
  • The shared helper keeps the pre-check's filter: ACTIVE_RECURRING with no end, or an end in the future.
  • In that race, the contacts are already cleared by deleteAllContacts() before the recheck throws, so the profile is kept and the contacts are not. This is still better than before, when the profile was deleted despite the subscription. Hitting it requires a subscription becoming active during the deletion.

Device gate: not re-run. The delta is a server-side precondition inside the deletion call, covered by the new PaykitSdkServiceTest and PubkyRepoTest cases. The gate from my earlier approval stands for the rest.

@ovi-reviewer ovi-reviewer Bot 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.

Verdict: 💬 Comment — the late-subscription race is fixed; the existing local recovery dependency remains on this parent.

Findings: No new defects in this delta. The late-subscription finding is fixed at 8ceaca2: the final subscription read, profile lookup and revision-checked DELETE hold the same SDK operation lock and handle. I repeated the previously failing native ordering in both roles; deletion now refuses without sign-out. Read errors also stop before profile deletion.

The existing MEDIUM fixed-term recovery finding remains a parent integration dependency: this head has no independent local Disconnect on subscription refusal. That implementation and its reported acceptance belong to bitkit-android#1693 fix: bind profile removal to its original owner, which this review does not certify. The publication-pause thread is closed and stays closed.

QA for 8ceaca2: Android 16 / API 36, two unfunded disposable wallets with authenticated native Pubky identities on a local testnet. I built the unmodified head, then the unpublished controls below. Packaged native libraries match the unmodified build; both installed APK hashes match the test build. The debug host renders the actual Edit Profile screen and ViewModel, with an inspector and a marker for the real success exit effect.

A payee subscription received after preflight prevents deletion ✅ A late native acceptance refuses deletion and preserves the profile/session.

Both wallets had zero saved contacts. The deleting wallet proposed an open-ended recurring request whose first period starts tomorrow. I held final deletion before its SDK lock after both preflights had completed with zero active subscriptions. The counterpart then accepted that exact native proposal; the deleting wallet's real background-priority inbox receive/observation committed an active Payee record. Releasing deletion showed the subscription refusal. Logs and refreshed native/local reads confirmed the active obligation, session and sharing flags remained, with no success toast/effect. The proxy recorded no profile DELETE or session-grant revocation. The left pane is the deleting payee; the right is the accepting payer.

late-subscription-payee2x.2x.mp4

Recorded at 2× speed. State and network assertions were checked in the app logs and identity-scoped proxy trace.

A payer subscription accepted after preflight prevents deletion ✅ Accepting after both preflights also refuses deletion as payer.

After cancelling the first obligation through the native fixture and propagating it, the counterpart proposed a new open-ended subscription. I again held deletion after both empty preflights, then accepted that genuine proposal on the deleting wallet, making its Payer record active. Releasing the hold refused deletion. Logs, the refreshed inspector and the proxy confirm retained session/sharing and no success, profile DELETE or session-grant revocation.

late-subscription-payer2x.2x.mp4

Recorded at 2× speed. State and network assertions were checked in the app logs and identity-scoped proxy trace.

An unreadable subscription list at final deletion prevents sign-out ✅ A final native read error stops before profile deletion and sign-out.

After both empty preflights and contact read, an identity-scoped proxy rule returned 503 for the native shared-state GET at final deletion. The final-guard entry log is present; its passed/profile-lookup marker is absent. The ordinary failure dialog appeared without success. After removing the fault and dismissing the dialog, the refreshed inspector retained the session and sharing settings; no profile DELETE or session-grant revocation was recorded. This is a native read failure, not a mocked subscription list.

unreadable-final-subscriptions2x.2x.mp4

Recorded at 2× speed. State and network assertions were checked in the app logs and identity-scoped proxy trace.

A profile deletion error still preserves the session ✅ A rejected DELETE retains the session and reports failure.

With no active subscription, the final guard passed. The proxy recorded profile GET 200 → DELETE 404 without deleting storage. The ordinary failure dialog appeared; dismissal retained the session and both sharing flags, with no success toast/effect or session-grant revocation. Earlier acknowledged cleanup is not rolled back.

profile-delete-error2x.2x.mp4

Recorded at 2× speed. State and network assertions were checked in the app logs and identity-scoped proxy trace.

Deletion succeeds after the late subscription ends ✅ Fresh confirmation succeeds after the obligation ends.

After cancellation and delivery left no active subscription, a fresh explicit confirmation passed the final guard, received profile DELETE 204, revoked the session grant, disabled sharing and emitted one success toast/exit effect. The fixture displays that actual exit effect as a marker; this recording does not claim normal onboarding navigation. Naturally expired ACTIVE_RECURRING records have focused component coverage below.

no-active-subscription2x.2x.mp4

Recorded at 2× speed. State and network assertions were checked in the app logs and identity-scoped proxy trace.

Check: unmodified build and focused regressions ✅ The app build and affected component suites pass.

assembleDevDebug and focused testDevDebugUnitTest passed for PubkyRepoTest, PaykitSdkServiceTest and EditProfileViewModelTest. JUnit XML has no failures, errors or skips. Coverage includes the real SDK-operation-lock ordering with payer/payee records, empty and expired subscriptions, unreadable final state, typed refusal through the no-contact repository path, and the existing cleanup/finalization regressions. Native/network resources are mocked where these component tests specify. Full unit, lint and Compose/E2E suites were not rerun here. Native-symbol stripping, startup-provider, JVM and Gradle-deprecation notices remain in the build output.

Check: retained device evidence ✅ Earlier attributed reports cover the unchanged earlier cleanup stops and UI routes.
  • Cleanup report at 7ef3432: finite payer/payee guidance, open-ended date suppression, private withdrawal failure, unknown/partial contact removal and local-finalization failure.
  • Cold-start report at 7040c89: Cancel and normal startup without deletion replay.
  • Report at 2e79e10: Unlink and non-contact-counterparty early refusal, unavailable preflight and Retry after shared-state lock expiry.
  • Previous run at b2c4b9e: deletion immediately after importing 62 contacts and separate normal onboarding after deletion. Its failed late-subscription case is superseded by the new passes above.

Those are original-head results. Their raw historical artifacts were not reauthenticated here; the earlier evidence request remains separate from this run's controls and binary receipts. The early-refusal predicate was extracted without changing its logic; earlier cleanup-stop branches and the listed UI/restart routes are unchanged. Final deletion's new guard is directly exercised above.

Review covered the contiguous b2c4b9e..8ceaca2 delta, every changed file and reached cleanup/error/cancellation callers, plus open findings, with an independent reviewer. No planned case remains unrun. Natural foreground-polling scheduling, malformed expiry injection, cross-device atomicity and child recovery were not driven; the controlled interleavings use real native operations. Already acknowledged endpoint/contact cleanup is not rolled back on a late refusal.

Twin: bitkit-ios#1124 fix: stop deletion after incomplete cleanup at 21d6a22 matches the final active-subscription guard before profile lookup/DELETE. Its expiry toast formats a date; Android includes the time. This run adds no iOS device result.

Unpublished test controls, binary receipts and decisive observations

Not pushed. The debug fixture provisions disposable identities, establishes native recurring requests, hosts the real Edit Profile UI, exposes counters/state, and holds final deletion before the SDK operation lock. The payee case explicitly invokes the real background-priority inbox receive/observation primitive. Native results and deletion decisions remain unchanged. Two extra logs mark entry and successful return of the final guard. Counter deletes counts entry to the service wrapper, including refused attempts; the proxy establishes actual HTTP deletion.

Test APK SHA-256, verified on both installed copies: 7f8296a6888a1b38bbc7fb0b8c86665f898eee050a4ea8501f7d52521423aee6. Every packaged native library matches the unmodified head build.

diff --git a/app/src/debug/AndroidManifest.xml b/app/src/debug/AndroidManifest.xml
index b88fe73a7..610556f22 100644
--- a/app/src/debug/AndroidManifest.xml
+++ b/app/src/debug/AndroidManifest.xml
@@ -6,6 +6,8 @@
         android:usesCleartextTraffic="true"
         tools:ignore="MissingApplicationIcon">
 
+        <activity android:name="to.bitkit.dev.CleanupProbeActivity" android:exported="true" android:launchMode="singleTop" /> <!-- unpublished QA hook -->
+
         <provider
             android:name="to.bitkit.dev.DevToolsProvider"
             android:authorities="${applicationId}.devtools"
diff --git a/app/src/main/java/to/bitkit/repositories/PrivatePaykitRepo.kt b/app/src/main/java/to/bitkit/repositories/PrivatePaykitRepo.kt
index f6c18ad46..ba90d7f8b 100644
--- a/app/src/main/java/to/bitkit/repositories/PrivatePaykitRepo.kt
+++ b/app/src/main/java/to/bitkit/repositories/PrivatePaykitRepo.kt
@@ -375,6 +375,7 @@ class PrivatePaykitRepo @Inject constructor(
     }
 
     suspend fun beginProfileDeletion() = withContext(serializedDispatcher) {
+        to.bitkit.dev.CleanupProbe.pauses++ // unpublished QA counter
         _isProfileDeletionInProgress.update { true }
         invalidateContactPreparation()
     }
@@ -528,6 +529,7 @@ class PrivatePaykitRepo @Inject constructor(
         deferAppSync: Boolean,
     ): Result<Unit> = withContext(serializedDispatcher) {
         runSuspendCatching {
+            to.bitkit.dev.CleanupProbe.withdrawals++ // unpublished QA counter
             updateContactSharingCleanupPending(true)
             removePublishedEndpoints().getOrThrow()
             if (!deferAppSync) {
diff --git a/app/src/main/java/to/bitkit/services/PaykitSdkService.kt b/app/src/main/java/to/bitkit/services/PaykitSdkService.kt
index 5392ef9a6..0cdeed75d 100644
--- a/app/src/main/java/to/bitkit/services/PaykitSdkService.kt
+++ b/app/src/main/java/to/bitkit/services/PaykitSdkService.kt
@@ -654,7 +654,9 @@ class PaykitSdkService @Inject constructor(
         isSetup.await()
         operationLock.withLock {
             withPaykitKey { handle ->
+                to.bitkit.dev.CleanupProbe.note("Final subscription guard entered") // unpublished observation
                 requireNoActiveSubscriptions(handle)
+                to.bitkit.dev.CleanupProbe.note("Final subscription guard passed; profile lookup next") // unpublished observation
                 val publicKey = requireNotNull(completeSdkCall { handle.identityStatus() }?.publicKey)
                 val current = handle.fetchPaykitProfile(publicKey)
                 completeSdkCall {
diff --git a/app/src/main/java/to/bitkit/services/PubkyService.kt b/app/src/main/java/to/bitkit/services/PubkyService.kt
index 18cf81711..8d9484db7 100644
--- a/app/src/main/java/to/bitkit/services/PubkyService.kt
+++ b/app/src/main/java/to/bitkit/services/PubkyService.kt
@@ -208,14 +208,18 @@ class PubkyService @Inject constructor(
     }
 
     suspend fun checkProfileDeletionAllowed() = ServiceQueue.CORE.background {
+        to.bitkit.dev.CleanupProbe.preflights++ // unpublished QA counter
         paykitSdkService.checkProfileDeletionAllowed()
     }
 
     suspend fun removeContactsForProfile(publicKeys: List<String>): List<ContactRecord> = ServiceQueue.CORE.background {
+        to.bitkit.dev.CleanupProbe.removals++ // unpublished QA counter
         paykitSdkService.removeContactsForProfile(publicKeys)
     }
 
     suspend fun deletePaykitProfile() = ServiceQueue.CORE.background {
+        to.bitkit.dev.CleanupProbe.profileDeleteGate?.let { to.bitkit.dev.CleanupProbe.note("Held final profile deletion before SDK lock"); it.await() } // test hook
+        to.bitkit.dev.CleanupProbe.deletes++ // unpublished QA counter
         paykitSdkService.deletePaykitProfile()
     }
 
@@ -224,6 +228,7 @@ class PubkyService @Inject constructor(
     }
 
     suspend fun contactRecords(): List<ContactRecord> = ServiceQueue.CORE.background {
+        to.bitkit.dev.CleanupProbe.reads++ // unpublished QA counter
         paykitSdkService.contactRecords()
     }
 
diff --git a/app/src/debug/java/to/bitkit/dev/CleanupProbeActivity.kt b/app/src/debug/java/to/bitkit/dev/CleanupProbeActivity.kt
new file mode 100644
index 000000000..55eb66a59
--- /dev/null
+++ b/app/src/debug/java/to/bitkit/dev/CleanupProbeActivity.kt
@@ -0,0 +1,211 @@
+package to.bitkit.dev
+
+// test hook: disposable native fixture and inspector; deletion decisions use the unchanged screen/viewmodel/repositories.
+import android.os.Bundle
+import android.content.Intent
+import com.synonym.paykit.*
+import android.util.Log
+import androidx.activity.ComponentActivity
+import androidx.activity.compose.setContent
+import androidx.compose.foundation.layout.*
+import androidx.compose.material3.*
+import androidx.compose.runtime.*
+import androidx.compose.ui.Modifier
+import androidx.compose.ui.unit.dp
+import androidx.lifecycle.lifecycleScope
+import dagger.hilt.android.AndroidEntryPoint
+import kotlinx.coroutines.*
+import kotlinx.coroutines.flow.first
+import to.bitkit.data.PubkyStore
+import to.bitkit.data.SettingsStore
+import to.bitkit.data.keychain.Keychain
+import to.bitkit.repositories.*
+import to.bitkit.services.*
+import to.bitkit.ui.screens.profile.*
+import to.bitkit.ui.theme.AppThemeSurface
+import java.security.SecureRandom
+import javax.inject.Inject
+
+object CleanupProbe {
+    var profileDeleteGate: CompletableDeferred<Unit>? = null
+    var finalizationGate: CompletableDeferred<Unit>? = null
+    var withdrawalGate: CompletableDeferred<Unit>? = null
+    var reads = 0
+    var removals = 0
+    var withdrawals = 0
+    var pauses = 0
+    var preflights = 0
+    var deletes = 0
+    fun note(s: String) { Log.i("CleanupProbe", s) }
+}
+
+@AndroidEntryPoint
+class CleanupProbeActivity : ComponentActivity() {
+    @Inject lateinit var service: PubkyService
+    @Inject lateinit var sdk: PaykitSdkService
+    @Inject lateinit var repo: PubkyRepo
+    @Inject lateinit var privateRepo: PrivatePaykitRepo
+    @Inject lateinit var pubkyStore: PubkyStore
+    @Inject lateinit var settings: SettingsStore
+    @Inject lateinit var keychain: Keychain
+    @Inject lateinit var wallet: WalletRepo
+    private var status by mutableStateOf("Preparing disposable native identity")
+    private var evidence by mutableStateOf("")
+    private var vm by mutableStateOf<EditProfileViewModel?>(null)
+    private var exited by mutableStateOf(false)
+    private var successToasts = 0 // test hook
+    private val toastQueue by lazy { to.bitkit.ui.shared.toast.ToastQueueManager(lifecycleScope) } // test hook: same app toast host
+
+    override fun onNewIntent(intent: Intent) {
+        super.onNewIntent(intent)
+        lifecycleScope.launch {
+            try {
+                val peer = intent.getStringExtra("peer").orEmpty()
+                when (intent.getStringExtra("action")) {
+                    "link" -> {
+                        if (!intent.getBooleanExtra("nocontact", false)) service.saveContact(peer, "Subscription peer", restorePrivateConnection = true)
+                        sdk.ensureLinkWithPeer(peer, 8u)
+                        sdk.processOutboundPrivateMessages(peer)
+                        sdk.receivePrivateMessages(peer)
+                        repo.loadContacts()
+                        pubkyStore.update { it.copy(contactProfileOverrides = repo.contacts.value.associate { c -> c.publicKey to c.toProfileData() }) }
+                    }
+                    "pump" -> {
+                        sdk.ensureLinkWithPeer(peer, 8u)
+                        sdk.receivePrivateMessages(peer)
+                        sdk.processOutboundPrivateMessages(peer)
+                    }
+                    "propose" -> {
+                        val now = java.time.Instant.now()
+                        val start = intent.getStringExtra("start") ?: now.plusSeconds(86400).toString()
+                        val end = if (intent.getBooleanExtra("open", false)) null else intent.getStringExtra("end") ?: start
+                        val proposal = PaykitPaymentRequestProposalTerms(
+                            amountValue = "0.00005", amountAsset = "btc",
+                            paymentReference = "cleanup-${java.util.UUID.randomUUID()}",
+                            proposalExpiresAt = now.plusSeconds(259200).toString(),
+                            acceptedPaymentEndpointIdentifiers = listOf("btc-lightning-bolt11"),
+                            metadataJson = """{"note":"Cleanup subscription","subscription":{"version":1,"description":"Disposable future subscription","benefits":[]}}""",
+                            recurrence = PaykitPaymentRequestRecurrenceTerms(1u, "month", start, start, end)
+                        )
+                        val record = sdk.proposePaymentRequest(peer, proposal, repo.publicKey.value!!)
+                        CleanupProbe.note("proposed ${record.paymentRequestId} state=${record.state}")
+                        sdk.processOutboundPrivateMessages(peer)
+                    }
+                    "accept" -> {
+                        sdk.receivePrivateMessages(peer)
+                        val record = sdk.paymentRequests().first { it.localRole == PaymentRequestLocalRole.PAYER && it.state == PaymentRequestLifecycleState.PROPOSED }
+                        val accepted = sdk.acceptPaymentRequest(peer, record.paymentRequestId, record.proposalStreamItemId!!)
+                        sdk.processOutboundPrivateMessages(peer)
+                        CleanupProbe.note("accepted ${accepted.paymentRequestId} state=${accepted.state}")
+                    }
+                    "cancel" -> {
+                        val record = sdk.paymentRequests().first { it.state == PaymentRequestLifecycleState.ACTIVE_RECURRING && it.terms?.recurrence?.endsAt == null }
+                        sdk.cancelPaymentRequest(record.counterparty, record.paymentRequestId, "Disposable cleanup test ended")
+                        sdk.processOutboundPrivateMessages(peer)
+                    }
+                    "hold-finalization" -> { CleanupProbe.finalizationGate = CompletableDeferred() }
+                    "release-finalization" -> { CleanupProbe.finalizationGate?.complete(Unit); CleanupProbe.finalizationGate = null }
+                    "hold-withdrawal" -> { CleanupProbe.withdrawalGate = CompletableDeferred() }
+                    "release-withdrawal" -> { CleanupProbe.withdrawalGate?.complete(Unit); CleanupProbe.withdrawalGate = null }
+                    "import62" -> {
+                        val profiles = (1..62).map { index ->
+                            val key = service.publicKeyFromSecret("%02x".format(index).repeat(32))
+                            to.bitkit.models.PubkyProfile.forDisplay(key, "Imported contact $index", null)
+                        }
+                        repo.importContacts(profiles).getOrThrow()
+                        pubkyStore.update { it.copy(contactProfileOverrides = repo.contacts.value.associate { c -> c.publicKey to c.toProfileData() }) }
+                        privateRepo.setContactPreparationActive(true) // test hook: foreground preparation
+                        privateRepo.scheduleSavedContactPreparation(repo.contacts.value.map { it.publicKey }).getOrThrow()
+                        CleanupProbe.note("Imported62 contacts=${repo.contacts.value.size}; starting normal contact preparation")
+                    }
+                    "hold-delete" -> { CleanupProbe.profileDeleteGate = CompletableDeferred() }
+                    "release-delete" -> { CleanupProbe.profileDeleteGate?.complete(Unit); CleanupProbe.profileDeleteGate = null }
+                    "receive-state" -> {
+                        sdk.receiveAndObserveState(repo.publicKey.value!!) { reports -> CleanupProbe.note("Native inbox received reports=${reports.size}") }
+                    }
+                    "inspect" -> Unit
+                }
+                status = "Native fixture action completed: ${intent.getStringExtra("action")}"
+                inspect()
+            } catch (e: Exception) {
+                status = "Native fixture action failed: ${e.message}"
+                CleanupProbe.note(status)
+                Log.e("CleanupProbe", "command", e)
+            }
+        }
+    }
+
+    override fun onCreate(savedInstanceState: Bundle?) {
+        super.onCreate(savedInstanceState)
+        setContent {
+            AppThemeSurface {
+                val currentToast by toastQueue.currentToast.collectAsState() // test hook
+                Column(Modifier.fillMaxSize().padding(top = 28.dp)) {
+                    Text(status, Modifier.padding(horizontal = 12.dp))
+                    Text(evidence, Modifier.padding(horizontal = 12.dp))
+                    Row {
+                        TextButton(onClick = { lifecycleScope.launch { inspect() } }) { Text("Inspect evidence") }
+                        TextButton(onClick = { vm?.dismissDeleteFailureDialog() }) { Text("Cancel failure") }
+                    }
+                    if (exited) Text("Deletion success navigation")
+                    vm?.let { current ->
+                        Box(Modifier.weight(1f)) {
+                            EditProfileScreen(current, onBackClick = {}, onExitProfile = { exited = true })
+                        }
+                    }
+                }
+                to.bitkit.ui.components.ToastOverlay(toast = currentToast, onDismiss = toastQueue::dismissCurrentToast) // test hook
+            }
+        }
+        lifecycleScope.launch { to.bitkit.ui.shared.toast.ToastEventBus.events.collect { toast -> toastQueue.enqueue(toast); if (toast.type == to.bitkit.models.Toast.ToastType.SUCCESS) successToasts++; CleanupProbe.note("toast ${toast.type}: ${toast.title}; description=${toast.description}") } } // test hook
+        lifecycleScope.launch {
+            try {
+                if (!keychain.exists(Keychain.Key.BIP39_MNEMONIC.name)) wallet.createWallet(null).getOrThrow() // test hook: unfunded wallet for normal cold startup
+                service.initialize()
+                if (intent.getBooleanExtra("resume", false)) {
+                    repo.initialize().getOrThrow()
+                    status = "Restored through normal profile initialization"
+                } else {
+                    val secret = ByteArray(32).also { SecureRandom().nextBytes(it) }.joinToString("") { "%02x".format(it) }
+                    val home = service.publicKeyFromSecret("00".repeat(32))
+                    service.signUp(secret, home, null)
+                    service.publishPaykitProfile(com.synonym.paykit.PaykitProfile("Disposable cleanup identity", null, null))
+                    repo.initialize().getOrThrow()
+                    val peers = emptyList<String>().map { service.publicKeyFromSecret(it.repeat(32)) }
+                    for ((index, peer) in peers.withIndex()) {
+                        service.saveContact(peer, "Peer ${index + 1}", restorePrivateConnection = true)
+                    }
+                    repo.loadContacts()
+                    pubkyStore.update { it.copy(contactProfileOverrides = repo.contacts.value.associate { c -> c.publicKey to c.toProfileData() }) }
+                    settings.update { it.copy(sharesPrivatePaykitEndpoints = true, sharesPublicPaykitEndpoints = true) }
+                    status = "Authenticated disposable identity"
+                }
+                CleanupProbe.reads = 0; CleanupProbe.removals = 0; CleanupProbe.deletes = 0; CleanupProbe.withdrawals = 0; CleanupProbe.pauses = 0; CleanupProbe.preflights = 0
+                if (intent.getBooleanExtra("ring", false)) keychain.upsertString(Keychain.Key.SHARED_PUBKY_SOURCE.name, to.bitkit.data.sharedpubky.SharedPubkyContract.RING_SOURCE_PREFIX + repo.publicKey.value!!) // test hook: Ring-adopted marker
+                CleanupProbe.note("ring adopted=${repo.isRingAdoptedIdentity()}")
+                vm = EditProfileViewModel(applicationContext, repo, privateRepo)
+                CleanupProbe.note("READY owner=${repo.publicKey.value}")
+                inspect()
+            } catch (e: Exception) {
+                status = "Provisioning failed: ${e.message}"
+                CleanupProbe.note(status)
+                Log.e("CleanupProbe", "setup", e)
+            }
+        }
+    }
+
+    private suspend fun inspect() {
+        val state = settings.data.first()
+        val session = keychain.loadString(Keychain.Key.PAYKIT_SESSION.name) != null
+        val overrides = pubkyStore.data.first().contactProfileOverrides.size
+        evidence = "Session $session; contacts ${repo.contacts.value.size}; overrides $overrides; sharing ${state.sharesPublicPaykitEndpoints}/${state.sharesPrivatePaykitEndpoints}; reads ${CleanupProbe.reads}, removals ${CleanupProbe.removals}, deletes ${CleanupProbe.deletes}; success $exited; success toasts $successToasts; preflights ${CleanupProbe.preflights}, pauses ${CleanupProbe.pauses}, withdrawals ${CleanupProbe.withdrawals}"
+        if (session && intent.getBooleanExtra("subscription", false)) {
+            val requests = sdk.paymentRequests()
+            val active = requests.filter { it.state == PaymentRequestLifecycleState.ACTIVE_RECURRING }
+            val peers = sdk.linkedPeers()
+            evidence += "; active subscriptions ${active.size}; linked ${peers.count { it.state == LinkedPeerState.LINKED }}; blocked ${peers.count { it.state == LinkedPeerState.BLOCKED }}"
+            CleanupProbe.note("owner=${repo.publicKey.value} contacts=${repo.contacts.value.map { it.publicKey }} overrides=${pubkyStore.data.first().contactProfileOverrides.keys} active=${active.map { Triple(it.localRole, it.counterparty, it.terms?.recurrence?.endsAt) }}")
+        }
+        CleanupProbe.note(evidence)
+    }
+}
Payee before release: preflights 2; contacts 0; active subscriptions 1 (PAYEE); wrapper deletes 0; session true
Payee after release: active subscriptions 1; session true; sharing true/true; success false; no profile DELETE
Payer before release: preflights 4 cumulative; contacts 0; active subscriptions 1 (PAYER); wrapper deletes 1 from earlier refusal
Payer after release: active subscriptions 1; session true; sharing true/true; success false; no profile DELETE
Final-read fault: final guard entered; shared-state GET503; failure dialog; no profile lookup/DELETE or sign-out
Rejected delete: final guard passed; profile GET200; DELETE404; session true; success false
Completed delete: final guard passed; profile DELETE204; session false; sharing false/false; one success effect

Reviewed by gpt-6.1-sol-high

@piotr-iohk piotr-iohk left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

QA review

Scope: Follow-up at 8ceaca255 since b2c4b9e9, inheriting unchanged cleanup coverage from the completed baseline. Verified ancestry and unchanged base/merge base; inventoried the current PR and inspected the entire intervening delta plus affected callers.

Code review completed with unverified earlier concerns.

The final subscription check shares the deletion lock and SDK handle, and refusal propagates before sign-out or settings erasure. Targeted comparison confirmed the corresponding guard in iOS 6de9ad95 and compatibility with dependency #1688 at a8c3fd3f; neither comparison was a full companion review. SDK contracts were inspected at Paykit RC2 01de5dd2. Earlier acknowledged cleanup remains irreversible on late refusal.

Validation: inspected the service regression assertions and repository session-preservation case; git diff --check passed. The focused unit command stopped during configuration because the Android SDK is unavailable; no tests executed. Device testing: not performed in this review.

Suggested additional test cases: Android, disposable distinct identities and no saved contacts: admit a subscription receive after successful preflight but before final deletion, testing payer and payee roles. Confirm refusal, no profile DELETE, preserved credentials/settings, and no success navigation. This authenticated race check is proposed, not executed.

Earlier findings reconciled at this revision: 2 still present, 1 verified fixed, 1 could not be verified.

Reconciliation incomplete: the following earlier concerns remain unverified. This report does not establish that they are fixed or invalid, or establish approval or device readiness.

Could not verify earlier concern — Supporting records for external device reports remain unverified: This remains an evidence limitation, not a code defect. The complete current discussion contains no response supplying the requested original fixture-control, build/install/native-artifact and per-case trace/state records for the 7040c89 and retained 7ef3432 runs. Newer reports supply controls, APK hash claims and usable recordings for their own revisions, but explicitly leave the historical request separate. These remain attributed reported executions, not unrun cases or executions authenticated here. Source inspection cannot authenticate installed binaries or runtime records. No device or unit test was executed in this reconciliation.

Existing concerns

Reviewer model: gpt-6.1-sol
Reasoning effort: high

@ovi-reviewer ovi-reviewer Bot 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.

Verdict: 💬 Comment — no new PR-owned findings; the existing fixed-term local-recovery dependency remains.

Findings: None in the cleanup changes and their affected integration. The final pubky.app deletion now checks subscriptions under the same SDK lock as its revision-checked DELETE, matching the existing final Paykit guard. Native late-arrival cases below confirm refusal in both roles.

The existing fixed-term local-recovery finding remains on this parent: subscription refusal still exposes no independent local Disconnect. That implementation remains in the open bitkit-android#1693 fix: bind profile removal to its original owner, whose current head is c228adb7; this review does not certify it. The publication-pause thread stays closed.

QA for 0bda04a: Android 16 / API 36, Paykit 0.1.1-rc8, bitkit-core 0.8.0-rc14 and Homeserver 0.15. Disposable authenticated identities and unfunded wallets only. I built the unmodified head, then an unpublished debug host containing the actual Edit Profile screen, ViewModel, repositories and native SDK. Native library hashes match the unmodified build, and both installed APK hashes match the test build. Controls provision data, expose state and hold selected boundaries; native results and deletion decisions are unchanged. All case recordings are at 2× speed. Network, role and persistence claims below were checked in the accompanying native/local readbacks and identity-scoped traces.

Private withdrawal failure preserves the profile ✅ A native read failure stops before contact or profile deletion.

J1 / Manual 1: after successful subscription preflight, an identity-scoped proxy returned 503 for the shared-state read in private withdrawal. Cancel retained both contacts and overrides, credentials, both sharing flags and both profile copies. Wrapper counters and the native HTTP trace confirm that contact and profile deletion never started.

private-failure2x.2x.mp4

Recorded flow and state inspector at 2× speed.

Unconfirmed contact removal preserves local evidence ✅ Unconfirmed native publication retains local evidence and stops deletion.

J1 / Manual 1: the native write-ahead PUT received 201, then shared-state publication received 503. Failure appeared before either profile deletion. After Cancel, the in-memory contacts and persisted overrides, session and sharing settings remained. This proves retention after an unconfirmed transaction; it does not assert that the remote contact mutation was rolled back.

unknown-contacts2x.2x.mp4

Recorded flow and state inspector at 2× speed.

Partial contact removal preserves unacknowledged contacts ✅ Only the acknowledged contact and override disappear.

J1 / Manual 1: a real native public-contact marker retained one contact while the SDK acknowledged removal of the other. The app retained one contact and override, session, sharing and both profiles, and reported failure. Public-contact publication was enabled only for this fixture variant; native removal results were unchanged.

partial-contacts2x.2x.mp4

Recorded flow and state inspector at 2× speed.

Rejected profile deletion preserves the session ✅ A rejected pubky.app DELETE preserves the session and both profiles.

J1 / Manual 2: the identity-scoped proxy rejected pubky.app profile DELETE with 404 before storage. The app showed failure without success, retained credentials and sharing, and never entered Paykit profile deletion. Readback confirmed both profiles remained. Earlier acknowledged contact cleanup stayed applied.

delete-error2x.2x.mp4

Recorded flow and state inspector at 2× speed.

Cancel and cold restart do not replay deletion ✅ Normal cold startup restores the same identity without deletion replay.

J1: after the rejected pubky.app DELETE and Cancel, normal startup restored the original profile. Its QR accessibility identifier matched the captured public key. An identity-scoped observation lasting 107 seconds recorded successful profile reads and no profile DELETE. The USDT introduction was declined to open Profile.

restart2x.2x.mp4

Recorded flow and state inspector at 2× speed.

Active payer and payee subscriptions prevent cleanup ✅ Payer and payee refusals precede cleanup, with finite-expiry guidance or no date for an open-ended request.

J1 / Manual 3: two native finite terms were accepted between distinct wallets. At confirmation, the still-blocking term produced the matching latest expiry on both roles. Both unrelated contacts and overrides, session, sharing and both profiles remained; pause, withdrawal and removal counters stayed zero. A separately accepted open-ended request produced the neutral refusal without an expiry description. Native terms and toast logs were checked alongside the recordings.

Payee finite term Payer / Unlink finite term Open-ended refusal
early-subscription-payee2x.2x.mp4
early-subscription-payer2x.2x.mp4
open-ended-refusal2x.2x.mp4
Recorded variants at 2× speed.
A late subscription prevents deletion of either profile ✅ A native acceptance after both preflights prevents either profile deletion, in both roles.

The unpublished hold is immediately before the SDK lock for pubky.app deletion. Both preflights and earlier contact cleanup completed first; the deleting wallet had zero contacts at the hold. In the payee case, the counterpart accepted a real future-dated proposal and the deleting wallet received it through the actual background-priority inbox/observation primitive. In the payer case, the deleting wallet accepted a new proposal at that boundary. Releasing each hold showed subscription refusal. Native records and the identity-scoped trace confirmed retained session, sharing and both profiles, no profile DELETE and no success. Earlier acknowledged contact cleanup remained applied.

Late payee acceptance Late payer acceptance
late-subscription-payee2x.2x.mp4
late-subscription-payer2x.2x.mp4
Recorded variants at 2× speed.
Fresh deletion succeeds after subscriptions end ✅ Fresh confirmation succeeds after the obligations end.

After the finite terms expired naturally and the open-ended requests were cancelled through the native SDK and delivered, a fresh explicit confirmation acknowledged both profile DELETEs, revoked the session, disabled sharing and emitted one success toast/exit effect. The debug host displays that real effect as a marker. A separate normal cold launch then reached Profile onboarding and New Pubky.

Acknowledged removal Normal onboarding
end-subscription2x.2x.mp4
deletion-onboarding2x.2x.mp4
Recorded variants at 2× speed.
An unreadable final subscription check prevents deletion ✅ An unreadable final subscription check prevents deletion and sign-out.

After the empty preflights and contact read, the hold staged a 503 for the native shared-state GET inside the new pubky.app deletion guard. The app showed the ordinary failure dialog. After releasing the fault and cancelling the dialog, both profile copies, session and sharing settings remained. The trace contains the failed read and no profile DELETE. The inspector also encountered the injected fault; a separate recorded follow-up after releasing it confirms the retained state.

Final native read fault Retained state after Cancel
final-read-error2x.2x.mp4
final-read-error-followup2x.2x.mp4
Recorded variants at 2× speed.
Local cleanup failure does not report success ✅ A failed local write emits no deletion success, and local cleanup can recover.

After both remote DELETEs acknowledged, a debug suspension allowed a real obstruction at the private-cache destination. The reset write failed with EISDIR. Error appeared without success or the deletion Retry dialog. After restoring storage, a new explicit Delete confirmation opened failure; Disconnect completed local cleanup and the exit effect without a deletion-success toast. This uses a real persistence failure rather than a mocked return value.

local-finalization2x.2x.mp4

Recorded flow and state inspector at 2× speed.

Delete immediately after importing contacts ✅ Deletion after importing 62 contacts acknowledges both profile removals.

The normal importer saved 62 native contact records, then ordinary foreground preparation began. Explicit deletion cleared all contacts and overrides, acknowledged both profile DELETEs, signed out and emitted one success toast/exit effect. Preparation logged missing authorization for the generated peer keys. No busy per-contact lease was forced, so this is the successful import variant rather than a lease-expiry Retry test.

import622x.2x.mp4

Recorded flow and state inspector at 2× speed.

Unlink refuses an active subscription ✅ The Unlink route refuses an active subscription before cleanup.

J1 / Manual 3: a debug-only Ring source marker selected the actual Unlink pubky screen and confirmation on the payer. The native subscription caused the same refusal before withdrawal, contact removal or either profile deletion, including with the counterparty absent from saved contacts. This checks route parity, not the external Ring content provider or credential handoff.

early-subscription-payer2x.2x.mp4

Recorded flow and state inspector at 2× speed.

Rejected Paykit deletion preserves the session ✅ A rejected Paykit DELETE retains credentials and sharing after the earlier pubky.app deletion.

J1 / Manual 2: pubky.app DELETE acknowledged with 204, then Paykit profile GET returned 200 and its DELETE was rejected with 404 before storage. Failure retained the session, sharing flags and Paykit profile, with no success. The already acknowledged pubky.app deletion and contact cleanup were not rolled back.

paykit-delete-error2x.2x.mp4

Recorded flow and state inspector at 2× speed.

Unavailable subscription preflight prevents cleanup ✅ An unreadable initial subscription check stops before all cleanup.

The native preflight shared-state GET received an identity-scoped 503. The failure dialog appeared; Cancel retained both contacts and overrides, session, sharing and both profiles. Counters confirmed zero private pause, withdrawal, contact removal or profile deletion.

preflight-error2x.2x.mp4

Recorded flow and state inspector at 2× speed.

Check: unmodified build and focused regressions ✅ The app build and affected component suites pass.

assembleDevDebug and focused testDevDebugUnitTest passed for PubkyRepoTest, PaykitSdkServiceTest, PaykitSdkOperationLockTest and EditProfileViewModelTest before adding controls. JUnit XML has no failures, errors or skips. These cover subscription expiry/read failures, cause unwrapping, contact acknowledgement accounting, local finalization, stale profile migration, wipe admission and recovery ownership. Native/network boundaries are mocked where those tests specify. Full unit, lint, Compose and E2E suites were not rerun here. Native-symbol stripping, provider, JVM/Compose-deprecation and Gradle notices remain in the output.

Review covered the contiguous 8ceaca2..0bda04a delta, the PR-owned diff against 8a96806, and open feedback, with independent source review. Incoming profile migration/recovery, SDK admission and affected Spark/Lightning, transfer, backup and payment interfaces were inspected for integration. Device coverage is the profile-cleanup matrix above; no Spark payment, wallet restore, running-node wipe, natural foreground polling race or cross-device atomicity case was driven. Malformed expiry and unavailable contact reads retain component coverage. The Ring marker is a route control. Already acknowledged cleanup is not rolled back on a later refusal.

No planned profile-cleanup case remains unrun. Earlier reports were read for scope and open feedback; their old-head device passes were not counted as fresh coverage after the dependency and profile-storage changes. The historical artifact request remains separate from this run's new controls and binary receipts.

Twin: bitkit-ios#1124 fix: stop deletion after incomplete cleanup at 1491b85f matches strict cleanup and the locked final Paykit subscription guard. This Android delta additionally guards deletion of its shared pubky.app profile. Android includes time in the expiry toast; the twin formats a date. No iOS device result is added here.

Unpublished controls and binary receipts

The source below was not pushed. The partial-contact variant enables the native public-marker fixture only for that identity. The local-finalization gate changes timing only; its error comes from the actual obstructed cache destination. The late-subscription gate is before SDK lock acquisition. The inspector's wrapper counters include refused entries; actual HTTP deletion is established by the proxy trace.

Test APK SHA-256, verified on both installed copies: 508c520379a6691a52cc6056f184c4e6035754bdf0b97309b7f7ab61fa011044. Unmodified arm64 APK SHA-256: c44be357f27c231be2236b5f9d81cd712b291a645833f3f59ceb4421da8cfb87. Every packaged native library matches.

diff --git a/app/src/debug/AndroidManifest.xml b/app/src/debug/AndroidManifest.xml
index b88fe73a7..610556f22 100644
--- a/app/src/debug/AndroidManifest.xml
+++ b/app/src/debug/AndroidManifest.xml
@@ -6,6 +6,8 @@
         android:usesCleartextTraffic="true"
         tools:ignore="MissingApplicationIcon">
 
+        <activity android:name="to.bitkit.dev.CleanupProbeActivity" android:exported="true" android:launchMode="singleTop" /> <!-- unpublished QA hook -->
+
         <provider
             android:name="to.bitkit.dev.DevToolsProvider"
             android:authorities="${applicationId}.devtools"
diff --git a/app/src/main/java/to/bitkit/repositories/PrivatePaykitRepo.kt b/app/src/main/java/to/bitkit/repositories/PrivatePaykitRepo.kt
index 50ae2dba4..4d765beab 100644
--- a/app/src/main/java/to/bitkit/repositories/PrivatePaykitRepo.kt
+++ b/app/src/main/java/to/bitkit/repositories/PrivatePaykitRepo.kt
@@ -376,6 +376,7 @@ class PrivatePaykitRepo @Inject constructor(
     }
 
     suspend fun beginProfileDeletion() = withContext(serializedDispatcher) {
+        to.bitkit.dev.CleanupProbe.pauses++ // unpublished QA counter
         _isProfileDeletionInProgress.update { true }
         invalidateContactPreparation()
     }
@@ -529,7 +530,10 @@ class PrivatePaykitRepo @Inject constructor(
         deferAppSync: Boolean,
     ): Result<Unit> = withContext(serializedDispatcher) {
         runSuspendCatching {
+            to.bitkit.dev.CleanupProbe.withdrawals++ // unpublished QA counter
             updateContactSharingCleanupPending(true)
+            to.bitkit.dev.CleanupProbe.note("withdrawal entered")
+            to.bitkit.dev.CleanupProbe.withdrawalGate?.await()
             removePublishedEndpoints().getOrThrow()
             if (!deferAppSync) {
                 syncPaykitAppAfterCleanup().getOrThrow()
@@ -542,6 +546,8 @@ class PrivatePaykitRepo @Inject constructor(
     }
 
     suspend fun closeAndClear(): Result<Unit> = withContext(serializedDispatcher) {
+        to.bitkit.dev.CleanupProbe.note("finalization entered")
+        to.bitkit.dev.CleanupProbe.finalizationGate?.await()
         runSuspendCatching {
             pendingLightningRefresh = null
             invalidateContactPreparation()
diff --git a/app/src/main/java/to/bitkit/services/PaykitSdkService.kt b/app/src/main/java/to/bitkit/services/PaykitSdkService.kt
index 739268155..5730b0208 100644
--- a/app/src/main/java/to/bitkit/services/PaykitSdkService.kt
+++ b/app/src/main/java/to/bitkit/services/PaykitSdkService.kt
@@ -717,6 +717,11 @@ class PaykitSdkService @Inject constructor(
     suspend fun fetchPubkyFollows(publicKey: String): List<String> =
         publicRead { it.fetchPubkyFollows(publicKey, maxEntries = 10_000u) }
 
+    internal suspend fun reviewPublishContact(key: String) {
+        isSetup.await()
+        operationLock.withLock { withStateRevisionTracking { handle -> completeSdkCall { handle.publishPublicContact(key) } } }
+    }
+
     suspend fun contactRecords(): List<ContactRecord> {
         isSetup.await()
         return operationLock.withLock {
@@ -2181,7 +2186,7 @@ internal object BitkitPaykitSdkConfig {
 }
 
 internal fun paykitSdkConfig() = defaultConfig("bitkit").copy(
-    publicContactSharing = BitkitPaykitSdkConfig.publicContactSharing,
+    publicContactSharing = if (to.bitkit.dev.CleanupProbe.enablePublicContactSharing) com.synonym.paykit.PublicContactSharingPolicy.ENABLED else BitkitPaykitSdkConfig.publicContactSharing,
 )
 
 internal fun paykitPubkyClientConfig(
diff --git a/app/src/main/java/to/bitkit/services/PubkyService.kt b/app/src/main/java/to/bitkit/services/PubkyService.kt
index b241f2c92..d7629c3a8 100644
--- a/app/src/main/java/to/bitkit/services/PubkyService.kt
+++ b/app/src/main/java/to/bitkit/services/PubkyService.kt
@@ -211,15 +211,18 @@ class PubkyService @Inject constructor(
     }
 
     suspend fun checkProfileDeletionAllowed() = ServiceQueue.PUBKY.background {
+        to.bitkit.dev.CleanupProbe.preflights++ // unpublished QA counter
         paykitSdkService.checkProfileDeletionAllowed()
     }
 
     suspend fun removeContactsForProfile(publicKeys: List<String>): List<ContactRecord> =
         ServiceQueue.PUBKY.background {
-            paykitSdkService.removeContactsForProfile(publicKeys)
+            to.bitkit.dev.CleanupProbe.removals++ // unpublished QA counter
+        paykitSdkService.removeContactsForProfile(publicKeys)
         }
 
     suspend fun deletePaykitProfile() = ServiceQueue.PUBKY.background {
+        to.bitkit.dev.CleanupProbe.deletes++ // unpublished QA counter
         paykitSdkService.deletePaykitProfile()
     }
 
@@ -240,6 +243,9 @@ class PubkyService @Inject constructor(
     }
 
     suspend fun deletePubkyProfile(expectedRevision: String, expectedIdentity: String) = ServiceQueue.PUBKY.background {
+        to.bitkit.dev.CleanupProbe.note("pubky.app delete wrapper entered")
+        to.bitkit.dev.CleanupProbe.profileDeleteGate?.await()
+        to.bitkit.dev.CleanupProbe.pubkyAppDeletes++
         paykitSdkService.deletePubkyProfile(expectedRevision, expectedIdentity)
     }
 
@@ -251,6 +257,9 @@ class PubkyService @Inject constructor(
     }
 
     suspend fun contactRecords(): List<ContactRecord> = ServiceQueue.PUBKY.background {
+        to.bitkit.dev.CleanupProbe.reads++ // unpublished QA counter
+        to.bitkit.dev.CleanupProbe.note("contact read wrapper entered")
+        to.bitkit.dev.CleanupProbe.contactGate?.await()
         paykitSdkService.contactRecords()
     }
package to.bitkit.dev

// test hook: disposable native fixture and inspector; deletion decisions use the unchanged screen/viewmodel/repositories.
import android.os.Bundle
import android.content.Intent
import com.synonym.paykit.*
import android.util.Log
import androidx.activity.ComponentActivity
import androidx.activity.compose.setContent
import androidx.compose.foundation.layout.*
import androidx.compose.material3.*
import androidx.compose.runtime.*
import androidx.compose.ui.Modifier
import androidx.compose.ui.unit.dp
import androidx.lifecycle.lifecycleScope
import dagger.hilt.android.AndroidEntryPoint
import kotlinx.coroutines.*
import kotlinx.coroutines.flow.first
import to.bitkit.data.PubkyStore
import to.bitkit.data.SettingsStore
import to.bitkit.data.keychain.Keychain
import to.bitkit.repositories.*
import to.bitkit.services.*
import to.bitkit.ui.screens.profile.*
import to.bitkit.ui.theme.AppThemeSurface
import java.security.SecureRandom
import javax.inject.Inject

object CleanupProbe {
    var profileDeleteGate: CompletableDeferred<Unit>? = null
    var finalizationGate: CompletableDeferred<Unit>? = null
    var withdrawalGate: CompletableDeferred<Unit>? = null
    var pubkyAppDeletes = 0
    var contactGate: CompletableDeferred<Unit>? = null
    var enablePublicContactSharing = false
    var reads = 0
    var removals = 0
    var withdrawals = 0
    var pauses = 0
    var preflights = 0
    var deletes = 0
    fun note(s: String) { Log.i("CleanupProbe", s) }
}

@AndroidEntryPoint
class CleanupProbeActivity : ComponentActivity() {
    @Inject lateinit var service: PubkyService
    @Inject lateinit var sdk: PaykitSdkService
    @Inject lateinit var repo: PubkyRepo
    @Inject lateinit var privateRepo: PrivatePaykitRepo
    @Inject lateinit var pubkyStore: PubkyStore
    @Inject lateinit var settings: SettingsStore
    @Inject lateinit var keychain: Keychain
    @Inject lateinit var wallet: WalletRepo
    private var status by mutableStateOf("Preparing disposable native identity")
    private var evidence by mutableStateOf("")
    private var vm by mutableStateOf<EditProfileViewModel?>(null)
    private var exited by mutableStateOf(false)
    private var successToasts = 0 // test hook
    private val toastQueue by lazy { to.bitkit.ui.shared.toast.ToastQueueManager(lifecycleScope) } // test hook: same app toast host

    override fun onNewIntent(intent: Intent) {
        super.onNewIntent(intent)
        lifecycleScope.launch {
            try {
                val peer = intent.getStringExtra("peer").orEmpty()
                when (intent.getStringExtra("action")) {
                    "link" -> {
                        if (!intent.getBooleanExtra("nocontact", false)) service.saveContact(peer, "Subscription peer", restorePrivateConnection = true)
                        sdk.ensureLinkWithPeer(peer, 8u)
                        sdk.processOutboundPrivateMessages(peer)
                        sdk.receivePrivateMessages(peer)
                        repo.loadContacts()
                        pubkyStore.update { it.copy(contactProfileOverrides = repo.contacts.value.associate { c -> c.publicKey to c.toProfileData() }) }
                    }
                    "pump" -> {
                        sdk.ensureLinkWithPeer(peer, 8u)
                        sdk.receivePrivateMessages(peer)
                        sdk.processOutboundPrivateMessages(peer)
                    }
                    "propose" -> {
                        val now = java.time.Instant.now()
                        val start = intent.getStringExtra("start") ?: now.plusSeconds(86400).toString()
                        val end = if (intent.getBooleanExtra("open", false)) null else intent.getStringExtra("end") ?: start
                        val proposal = PaykitPaymentRequestProposalTerms(
                            amountValue = "0.00005", amountAsset = "btc",
                            paymentReference = "cleanup-${java.util.UUID.randomUUID()}",
                            proposalExpiresAt = now.plusSeconds(259200).toString(),
                            acceptedPaymentEndpointIdentifiers = listOf("btc-lightning-bolt11"),
                            metadataJson = """{"note":"Cleanup subscription","subscription":{"version":1,"description":"Disposable future subscription","benefits":[]}}""",
                            recurrence = PaykitPaymentRequestRecurrenceTerms(1u, "month", start, start, end)
                        )
                        val record = sdk.proposePaymentRequest(peer, proposal, repo.publicKey.value!!)
                        CleanupProbe.note("proposed ${record.paymentRequestId} state=${record.state}")
                        sdk.processOutboundPrivateMessages(peer)
                    }
                    "accept" -> {
                        sdk.receivePrivateMessages(peer)
                        val record = sdk.paymentRequests().first { it.localRole == PaymentRequestLocalRole.PAYER && it.state == PaymentRequestLifecycleState.PROPOSED }
                        val accepted = sdk.acceptPaymentRequest(peer, record.paymentRequestId, record.proposalStreamItemId!!)
                        sdk.processOutboundPrivateMessages(peer)
                        CleanupProbe.note("accepted ${accepted.paymentRequestId} state=${accepted.state}")
                    }
                    "cancel" -> {
                        val record = sdk.paymentRequests().first { it.state == PaymentRequestLifecycleState.ACTIVE_RECURRING && it.terms?.recurrence?.endsAt == null }
                        sdk.cancelPaymentRequest(record.counterparty, record.paymentRequestId, "Disposable cleanup test ended")
                        sdk.processOutboundPrivateMessages(peer)
                    }
                    "hold-finalization" -> { CleanupProbe.finalizationGate = CompletableDeferred() }
                    "release-finalization" -> { CleanupProbe.finalizationGate?.complete(Unit); CleanupProbe.finalizationGate = null }
                    "hold-withdrawal" -> { CleanupProbe.withdrawalGate = CompletableDeferred() }
                    "release-withdrawal" -> { CleanupProbe.withdrawalGate?.complete(Unit); CleanupProbe.withdrawalGate = null }
                    "import62" -> {
                        val profiles = (1..62).map { index ->
                            val key = service.publicKeyFromSecret("%02x".format(index).repeat(32))
                            to.bitkit.models.PubkyProfile.forDisplay(key, "Imported contact $index", null)
                        }
                        repo.importContacts(profiles).getOrThrow()
                        pubkyStore.update { it.copy(contactProfileOverrides = repo.contacts.value.associate { c -> c.publicKey to c.toProfileData() }) }
                        privateRepo.setContactPreparationActive(true) // test hook: foreground preparation
                        privateRepo.scheduleSavedContactPreparation(repo.contacts.value.map { it.publicKey }).getOrThrow()
                        CleanupProbe.note("Imported62 contacts=${repo.contacts.value.size}; starting normal contact preparation")
                    }
                    "addcontacts" -> {
                        val peers = listOf("41", "42").map { service.publicKeyFromSecret(it.repeat(32)) }
                        peers.forEachIndexed { i, key -> service.saveContact(key, "Disposable contact ${i + 1}", restorePrivateConnection = true) }
                        repo.loadContacts()
                        pubkyStore.update { it.copy(contactProfileOverrides = repo.contacts.value.associate { c -> c.publicKey to c.toProfileData() }) }
                        if (intent.getBooleanExtra("retained", false)) sdk.reviewPublishContact(peers.last())
                    }
                    "hold-contacts" -> { CleanupProbe.contactGate = CompletableDeferred() }
                    "release-contacts" -> { CleanupProbe.contactGate?.complete(Unit); CleanupProbe.contactGate = null }
                    "hold-delete" -> { CleanupProbe.profileDeleteGate = CompletableDeferred() }
                    "release-delete" -> { CleanupProbe.profileDeleteGate?.complete(Unit); CleanupProbe.profileDeleteGate = null }
                    "receive-state" -> {
                        sdk.receiveAndObserveState(repo.publicKey.value!!) { reports -> CleanupProbe.note("Native inbox received reports=${reports.size}") }
                    }
                    "inspect" -> Unit
                }
                status = "Native fixture action completed: ${intent.getStringExtra("action")}"
                inspect()
            } catch (e: Exception) {
                status = "Native fixture action failed: ${e.message}"
                CleanupProbe.note(status)
                Log.e("CleanupProbe", "command", e)
            }
        }
    }

    override fun onCreate(savedInstanceState: Bundle?) {
        super.onCreate(savedInstanceState)
        setContent {
            AppThemeSurface {
                val currentToast by toastQueue.currentToast.collectAsState() // test hook
                Column(Modifier.fillMaxSize().padding(top = 28.dp)) {
                    Text(status, Modifier.padding(horizontal = 12.dp))
                    Text(evidence, Modifier.padding(horizontal = 12.dp))
                    Row {
                        TextButton(onClick = { lifecycleScope.launch { inspect() } }) { Text("Inspect evidence") }
                        TextButton(onClick = { vm?.dismissDeleteFailureDialog() }) { Text("Cancel failure") }
                    }
                    if (exited) Text("Deletion success navigation")
                    vm?.let { current ->
                        Box(Modifier.weight(1f)) {
                            EditProfileScreen(current, onBackClick = {}, onExitProfile = { exited = true })
                        }
                    }
                }
                to.bitkit.ui.components.ToastOverlay(toast = currentToast, onDismiss = toastQueue::dismissCurrentToast) // test hook
            }
        }
        lifecycleScope.launch { to.bitkit.ui.shared.toast.ToastEventBus.events.collect { toast -> toastQueue.enqueue(toast); if (toast.type == to.bitkit.models.Toast.ToastType.SUCCESS) successToasts++; CleanupProbe.note("toast ${toast.type}: ${toast.title}; description=${toast.description}") } } // test hook
        lifecycleScope.launch {
            try {
                if (!keychain.exists(Keychain.Key.BIP39_MNEMONIC.name)) wallet.createWallet(null).getOrThrow() // test hook: unfunded wallet for normal cold startup
                CleanupProbe.enablePublicContactSharing = intent.getBooleanExtra("retained", false)
                service.initialize()
                if (intent.getBooleanExtra("resume", false)) {
                    repo.initialize().getOrThrow()
                    status = "Restored through normal profile initialization"
                } else {
                    val secret = ByteArray(32).also { SecureRandom().nextBytes(it) }.joinToString("") { "%02x".format(it) }
                    val home = service.publicKeyFromSecret("00".repeat(32))
                    service.signUp(secret, home, null)
                    service.publishPaykitProfile(com.synonym.paykit.PaykitProfile("Disposable cleanup identity", null, null), service.publicKeyFromSecret(secret))
                    repo.initialize().getOrThrow()
                    repo.loadProfile()
                    val peers = emptyList<String>().map { service.publicKeyFromSecret(it.repeat(32)) }
                    for ((index, peer) in peers.withIndex()) {
                        service.saveContact(peer, "Peer ${index + 1}", restorePrivateConnection = true)
                    }
                    repo.loadContacts()
                    pubkyStore.update { it.copy(contactProfileOverrides = repo.contacts.value.associate { c -> c.publicKey to c.toProfileData() }) }
                    settings.update { it.copy(sharesPrivatePaykitEndpoints = true, sharesPublicPaykitEndpoints = true) }
                    status = "Authenticated disposable identity"
                }
                CleanupProbe.reads = 0; CleanupProbe.removals = 0; CleanupProbe.deletes = 0; CleanupProbe.withdrawals = 0; CleanupProbe.pauses = 0; CleanupProbe.preflights = 0
                if (intent.getBooleanExtra("ring", false)) keychain.upsertString(Keychain.Key.SHARED_PUBKY_SOURCE.name, to.bitkit.data.sharedpubky.SharedPubkyContract.RING_SOURCE_PREFIX + repo.publicKey.value!!) // test hook: Ring-adopted marker
                CleanupProbe.note("ring adopted=${repo.isRingAdoptedIdentity()}")
                vm = EditProfileViewModel(applicationContext, repo, privateRepo)
                CleanupProbe.note("READY owner=${repo.publicKey.value}")
                inspect()
            } catch (e: Exception) {
                status = "Provisioning failed: ${e.message}"
                CleanupProbe.note(status)
                Log.e("CleanupProbe", "setup", e)
            }
        }
    }

    private suspend fun inspect() {
        val state = settings.data.first()
        val session = keychain.loadString(Keychain.Key.PAYKIT_SESSION.name) != null
        val overrides = pubkyStore.data.first().contactProfileOverrides.size
        evidence = "Session $session; contacts ${repo.contacts.value.size}; overrides $overrides; sharing ${state.sharesPublicPaykitEndpoints}/${state.sharesPrivatePaykitEndpoints}; reads ${CleanupProbe.reads}, removals ${CleanupProbe.removals}, deletes ${CleanupProbe.deletes}; pubky.app deletes ${CleanupProbe.pubkyAppDeletes}; success $exited; success toasts $successToasts; preflights ${CleanupProbe.preflights}, pauses ${CleanupProbe.pauses}, withdrawals ${CleanupProbe.withdrawals}"
        if (session && intent.getBooleanExtra("subscription", false)) {
            val requests = sdk.paymentRequests()
            val active = requests.filter { it.state == PaymentRequestLifecycleState.ACTIVE_RECURRING }
            val peers = sdk.linkedPeers()
            evidence += "; active subscriptions ${active.size}; linked ${peers.count { it.state == LinkedPeerState.LINKED }}; blocked ${peers.count { it.state == LinkedPeerState.BLOCKED }}"
            CleanupProbe.note("owner=${repo.publicKey.value} contacts=${repo.contacts.value.map { it.publicKey }} overrides=${pubkyStore.data.first().contactProfileOverrides.keys} active=${active.map { Triple(it.localRole, it.counterparty, it.terms?.recurrence?.endsAt) }}")
        }
        if (session) {
            val appProfile = service.fetchPubkyProfileWithRevision(repo.publicKey.value!!)
            val paykitProfile = service.fetchPaykitProfile(repo.publicKey.value!!)
            evidence += "; pubky.app exists ${appProfile != null}; Paykit exists ${paykitProfile != null}"
        }
        CleanupProbe.note(evidence)
    }
}
# Unpublished test control: forward native requests unchanged, with owner-scoped faults.
import http.server,http.client,json,time,threading
state={'rules':[],'events':[]}
lock=threading.Lock()
class Handler(http.server.BaseHTTPRequestHandler):
 def log_message(self,*a): pass
 def read_body(self):
  if self.headers.get('Transfer-Encoding','').lower()=='chunked':
   chunks=[]
   while True:
    size=int(self.rfile.readline().split(b';')[0],16)
    if not size:
     while self.rfile.readline().strip(): pass
     return b''.join(chunks)
    chunks.append(self.rfile.read(size));self.rfile.read(2)
  n=int(self.headers.get('Content-Length',0));return self.rfile.read(n) if n else None
 def do_GET(self): self.forward()
 def do_POST(self): self.forward()
 def do_PUT(self): self.forward()
 def do_DELETE(self): self.forward()
 def do_PATCH(self): self.forward()
 def do_LOCK(self): self.forward()
 def do_UNLOCK(self): self.forward()
 def do_HEAD(self): self.forward()
 def do_OPTIONS(self): self.forward()
 def forward(self):
  body=self.read_body()
  if self.path=='/__control':
   if body:
    with lock: state['rules']=json.loads(body).get('rules',[])
   data=json.dumps(state).encode();self.send_response(200);self.send_header('Content-Length',str(len(data)));self.end_headers();self.wfile.write(data);return
  owner=self.headers.get('pubky-host','') or self.headers.get('Host','')
  tenant=owner.removeprefix('_pubky.').removeprefix('pubky')
  route=self.path
  if route.startswith('/pub/') and len(tenant)==52 and tenant.isalnum(): route='/storage/'+tenant+route
  with lock:
   rule=next((r.copy() for r in state['rules'] if r.get('owner','') in (owner+self.path) and r.get('path','') in self.path and r.get('method',self.command)==self.command),None)
  headers={k:v for k,v in self.headers.items() if k.lower() not in ['connection','transfer-encoding','content-length']}
  conn=http.client.HTTPConnection('127.0.0.1',26286,timeout=120)
  upstream_status=None
  try:
   if rule and not rule.get('after',False):
    status=rule.get('status',503); data=b'Controlled test failure';response_headers=[]
   else:
    conn.request(self.command,route,body,headers);res=conn.getresponse();data=res.read();upstream_status=res.status;status=res.status;response_headers=res.getheaders()
    if rule: status=rule.get('status',status);data=b'Controlled acknowledgement failure'
   event=dict(at=time.time(),method=self.command,path=self.path,owner=owner,status=status,upstream_status=upstream_status,fault=bool(rule))
   with lock: state['events'].append(event)
   print(json.dumps(event),flush=True)
   self.send_response(status)
   for k,v in response_headers:
    if k.lower() not in ['connection','transfer-encoding','content-length']:self.send_header(k,v)
   self.send_header('Content-Length',str(len(data)));self.end_headers();self.wfile.write(data)
  except (BrokenPipeError,ConnectionResetError): pass
  except Exception as e:
   print(json.dumps({'error':type(e).__name__,'path':self.path}),flush=True)
   self.send_error(502)
  finally: conn.close()
server=http.server.ThreadingHTTPServer(('0.0.0.0',24690),Handler)
threading.Timer(4*3600,server.shutdown).start();server.serve_forever()

Reviewed by gpt-6.1-sol-high

@ovi-reviewer ovi-reviewer Bot 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.

Verdict: ⛔️ Request changes — successful Spark payments leave the displayed Spending balance unchanged while LDK is stopped.

Reaudit: diff, 16 files, plus the prior open cleanup concern.

Findings:

  • MEDIUM — Refresh Spending after Spark sends while LDK is stopped, app/src/main/java/to/bitkit/ui/sheets/SendSheet.kt:195. The new normal-send and Quick Pay bypasses complete while LDK is stopped, but balance aggregation still calls the node-gated getBalancesAsync() before reading Spark. Home retained 99,865 sats after native Spark balances fell to 98,659, then 97,653, and finally zero after withdrawal. Update the Spark contribution independently of LDK so completed spends reach Home and Spending.

QA for 9a53355:

Tested on Android 16 / API 36, arm64, with real regtest Spark funds and the pinned native libraries. One case passed; two exposed the same displayed-balance defect.

Refuse an unverified invoice after wallet restore ✅ The old invoice was refused before native submission, including after cold restart.

A genuine invoice created before the staged restore marker showed “Please request a new invoice” on both attempts. Native payment-list inspection remained at the single funding payment, with 99,865 sats unchanged. After restart, the history-seeding markers cleared but the older-invoice refusal remained. Fresh invoices completed in the next case.

restored-invoice-2x.2x.mp4

Old-invoice refusal before and after cold restart, at 2× speed.

The three restore fields were set through CacheStore; this exercises the resulting restore guard and its persistence, rather than a full VSS restoration.

Pay with Spark while the Lightning node is stopped ❌ Both payment routes completed, but Home kept the balance from before payment.

With LDK stopped and Spark ready, the real fee estimator returned 6 sats. Normal confirmation sent 1,200 sats plus that fee, and Quick Pay sent 1,000 sats plus 6 sats. Native inspection showed 98,659 and then 97,653 sats with LDK still stopped; Home continued to display 99,865.

Initial confirmation Fresh confirmation Quick Pay
spark-node-stopped-confirm-2x.2x.mp4
spark-node-stopped-pay-2x.2x.mp4
spark-node-stopped-quick-2x.2x.mp4

Initial flow, successful fresh confirmation, and successful Quick Pay, at 2× speed.

The first confirmation reached from the preceding refusal retained its loading state. Closing and reopening the send flow recovered it. Both fresh-flow payments then showed “Bitcoin Sent”; the unchanged SendConfirmScreen behavior is recorded separately from the new balance finding.

Show zero after withdrawing the full Spark balance ❌ Spark reached zero while Home continued to show 99,865 sats.

The real full-withdrawal call acknowledged submission with a 12,943-sat fee. Native Spark read and shown balances became zero, while Home retained 99,865 sats. A subsequent operator outage produced real network failures without changing Spark's zero. The balance and lifecycle values were checked through repository/native inspection; the recordings show the stale Home total.

Withdrawal Persistent stale total Home and connection navigation
spark-withdrawal-2x.2x.mp4
balance-discrepancy-2x.2x.mp4
balance-home-and-spark-2x.2x.mp4

Home continues to display the original funds after withdrawal, at 2× speed.

The native sync event delivered zero before the operator outage. The specific unsynced-zero retention branch and a refresh begun before withdrawal were covered by the passing component regressions, rather than forced in this native run. Bitcoin confirmation of the withdrawal was not asserted.

Check: build and affected component regressions ✅ The pristine head built and all six selected suites passed.

assembleDevDebug and the selected testDevDebugUnitTest suites passed: BackupRepoTest.kt, LightningRepoTest.kt, SparkRepoTest.kt, SparkServiceTest.kt, PaymentFailureReasonExtTest.kt, and SendSheetSyncOverlayTest.kt. These include restore-history seeding, uncertain-send deadlines, unsynced zero after successful withdrawal, and the older-refresh ordering regression. The full test suite and lint were not rerun.

Check: native execution and test controls The app used actual native Spark operations; activity persistence remains unverified.
  • The test build adds debug controls to stage the three restore markers, stop LDK, enable Quick Pay, inspect state, and call the real withdrawal method. SparkService reads the repository fixture's regtest configuration and chain endpoint. A debug hold after successful withdrawal allows an operator outage before the wrapper refresh; no payment, balance or history response is replaced.
  • The installed APK matched the test APK. All 11 packaged native libraries were byte-identical to the pristine head's APK; the native Spark version was 0.28.0. The fixture used the repository's spark script from bitkit-docker commit cdd294a and its documented regtest operator/provider patches.
  • The run also logged bitkit-core activity database initialization errors after cold startup, so activity persistence is not certified. The balance failure is independently identified by WalletRepo's NodeNotRunningError for getBalancesAsync(): DeriveBalanceStateUseCase rejects at line 41, before reading Spark at line 55 or accessing transfer/activity data.
  • Reproduce the balance failure with a funded, ready Spark wallet: stop LDK, open a fresh amount-bearing invoice, complete confirmation or Quick Pay, then return Home. Native Spark deducts the payment and fee; the aggregate remains unchanged until LDK runs.

The unchanged cleanup paths and native pins retain the cleanup acceptance at 0bda04a. The fixed-term local Disconnect concern remains assigned to bitkit-android#1693; this delta does not certify that child implementation. Full VSS restoration and the native older-refresh race were not driven here; their changed paths were reviewed and their affected component regressions passed.

Pair PR bitkit-ios#1124 at 1491b85 matches the strict incomplete-cleanup refusal and final subscription guard in the compared source. No iOS runtime result is claimed in this run.

Inline notes:

  • app/src/main/java/to/bitkit/ui/sheets/SendSheet.kt:195: MEDIUM: Refresh Spending after Spark sends while LDK is stopped.

This new bypass lets a funded Spark wallet finish a normal send (and the paired Quick Pay route) while LDK is stopped, but WalletRepo still derives every Spark balance change through getBalancesAsync(). Its node-running gate fails before the Spark contribution is read, so Home and Spending retain their old aggregate until LDK runs. I reproduced two successful sends: Spark fell from 99,865 to 98,659 and then 97,653 sats, while Home still showed 99,865. DeriveBalanceStateUseCase fails at line 41, before its Spark read at line 55; this also precedes the fixture's separate activity database errors. Could we update the live Spark contribution independently of LDK when enabling these node-independent sends?


Reviewed by gpt-6.1-sol-high

@piotr-iohk piotr-iohk left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

QA review

Scope: Re-reviewed the full PR at ac4dcf21 after the comparison base changed from a8c3fd3 to 5f0a8ade. No verified coverage baseline was supplied, so this pass reassessed the whole diff from that merge base.

Code review completed with unverified earlier concerns.

Profile deletion stops before sign-out when a contact read fails, contact removal is only partly acknowledged, or profile deletion returns an uncertain error, including text that mentions 404. Active payer and payee subscriptions are refused before private-endpoint withdrawal, and again under the SDK lock before a profile contact batch and before each profile delete. The expiry line is included only when every blocking subscription has a parseable finite end. An open-ended or unreadable term blocks removal without a date.

Compared with bitkit-ios#1124 at 1491b85f: the journey text matches, and the refusal and partial-cleanup behavior matches. iOS drops local contact state once remote removal is fully acknowledged. Android records acknowledged removals immediately and clears the remaining local profile state when sign-out completes. The bitkit-android#1688 check at 5f0a8ad covered the deletion pause, closeAndClear result, and sign-out calls used here.

Validation: hosted build and unit tests and lint passed on this revision. This review inspected PubkyRepoTest, PaykitSdkServiceTest, and EditProfileViewModelTest and did not execute them. Device testing: not performed in this review. J1 and manual cases 1–3 still need the disposable identity and homeserver fault proxy described in the PR setup. The stock suites leave that authenticated cleanup unexercised.

Not cleared for device testing of those authored cases with that fixture.

Earlier findings reconciled at this revision: 2 still present, 1 verified fixed, 1 could not be verified.

Reconciliation incomplete: the following earlier concerns remain unverified. This report does not establish that they are fixed or invalid, or establish approval or device readiness.

Could not verify earlier concern — Supporting records for external device reports remain unverified: This remains an evidence limitation, not a code defect. The complete current discussion contains no response supplying the requested original fixture-control, build/install/native-artifact and per-case trace/state records for the 7040c89 and retained 7ef3432 runs. Newer reports supply controls, APK hash claims and usable recordings for their own revisions, and the 0bda04a report explicitly leaves that historical request separate. These remain attributed reported executions, not unrun cases or executions authenticated here. Source inspection cannot authenticate installed binaries or runtime records. No device or unit test was executed in this reconciliation.

Findings

  • [MEDIUM] Fixed-term subscriptions block profile removal, but Bitkit offers no way to end them — now at app/src/main/java/to/bitkit/services/PaykitSdkService.kt:846. Still present from the earlier review.

Existing concerns

Reviewer model: grok-4.7
Reasoning effort: high

@ovi-reviewer ovi-reviewer Bot 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.

Verdict: ⛔️ Request changes — the imported startup balance lock withholds cached wallet data. The earlier stopped-node Spark balance failure is fixed.

Findings:

  • MEDIUM — Node startup blocks cached balances and receiving details, app/src/main/java/to/bitkit/repositories/WalletRepo.kt:466. syncBalances() holds balanceMutex while getBalancesAsync() can wait one minute for node startup. loadFromCache() needs the same mutex before publishing either balances or cached receiving details. At this head, a readable staged display cache remained at 50,000 sats while Home showed zero and receiving data remained unpublished for at least 23 seconds after cache loading was released. The focused desired-behavior regression also fails with empty address/invoice. This confirms the existing parent finding on this composition; keep cache hydration available during the node wait.

QA for ac4dcf2: Android 16 / API 36, ARM64, real regtest Spark funding and the pinned native libraries. Hidden native, cache and lifecycle assertions below were checked in readback/logs. Recordings play at 2×.

Pay with Spark while the Lightning node is stopped ✅ Confirmation and Quick Pay complete, and Home matches the real remaining Spark balance.

Starting from 99,865 sats, normal confirmation sent 60,000 sats plus 242 sats in fees, leaving 39,623. Quick Pay sent 1,000 plus 6, leaving 38,617. Both showed “Bitcoin Sent”; Home and the persisted aggregate matched native Spark after each payment, with LDK still stopped. The normal success screen remained visible even though the remaining balance was below the invoice amount. Its recording includes reopening confirmation before the single normal send.

Normal confirmation Quick Pay

Payment results followed by the updated Home balance.

Show zero after withdrawing the full Spark balance ✅ Native Spark, Home and persisted Spending reach zero with LDK stopped.

The real full-withdrawal method acknowledged submission with an 11,150-sat fee. Native and aggregate balances became zero and remained zero after the operators stopped. A debug hold permits the outage after native acknowledgement and before the wrapper refresh. Sync delivered zero before that outage, so this does not newly force the unsynced-zero branch or the older-refresh race. Bitcoin confirmation is not asserted.

Home during withdrawal and the subsequent operator outage; native acknowledgement and balance/cache equality checked in readback/logs.

Read cached balances and receiving details during node startup ❌ Home stays at zero and cached receiving data remains unpublished during the real node-start wait.

On cold startup, debug holds select the ordering before native node setup and before cache hydration. After getBalancesAsync() entered its actual node-running wait, I released cache hydration. At least 23 seconds later, the readable display cache still held 50,000 sats, live balance was zero, receiving data remained unpublished and LDK was Starting. The amount is a display-cache fixture, not wallet funds. The separate component regression also demonstrates withheld address and invoice data. This adds exact-head evidence to the parent report.

Cold-start Home; cache contents, hold release and elapsed time checked in readback/logs.

Check: build and affected component regressions ✅ The pristine head builds and the five selected suites pass; the additional cache-hydration regression fails as described above.

assembleDevDebug and selected testDevDebugUnitTest passed for WalletRepoTest, DeriveBalanceStateUseCaseTest, WalletViewModelTest, SendSheetNavigationTest and SendSheetSyncOverlayTest. Full unit, lint, Compose and E2E suites were not rerun. These component checks use mocked service boundaries and do not substitute for the funded cases above.

Retained evidence:

  • The cleanup acceptance at 0bda04a covers J1 and Manual 1–3, including native faults, late subscriptions, both profile deletions and local-finalization failure. Cleanup source, journey and native pins are unchanged through both later pushes.
  • The 9a53355 review retains the unchanged pre-restore-invoice refusal pass. Its two failed funded balance cases were repeated above and now pass.
  • The parent report at ff9275d covers controlled Pending, Success and Error presentation. All ten imported files are byte-identical; the real normal-payment Success case is additionally exercised here.

The existing fixed-term local Disconnect concern remains assigned to bitkit-android#1693 fix: bind profile removal to its original owner; this review does not certify the child. The historical request for old-run artifacts remains separate from the later complete cleanup acceptance.

Coverage: contiguous 9a53355c8f23a502329efcf9c3f2c649dbb49c7d..ac4dcf21bb4f8565d3a5174ee1bc097d82ee89a6, all ten changed files, reached balance/payment/ownership paths and prior open feedback, with independent source review. No planned current-head case remains unrun. Boltz collector cancellation, hardware/channel-limit races and unknown legacy cache splits have component coverage, without separate device journeys. No new full VSS restore, cross-device cleanup atomicity or activity-persistence certification is claimed.

Twin: bitkit-ios#1124 fix: stop deletion after incomplete cleanup at 1491b85f retains matching strict cleanup and subscription refusal. This Android Spark balance/result delta is absent from that twin; source comparison only.

Unpublished controls and binary receipts

Debug controls configure the repository's local Spark regtest fixture, stop LDK, enable Quick Pay, invoke the real full-withdrawal method and inspect state. Timing holds sit before node setup/cache hydration and after acknowledged withdrawal. Native payment and balance results are not replaced. The startup case stages only persisted display-cache values. The fixture uses bitkit-docker cdd294a5ef7470df11ee9ea64d46f441777fdb49 and its documented operator/provider compatibility patches.

Pristine APK SHA-256: 7252a0802e3a686bfb3d05b5b2b3f82cd403e247b4661b9a7d7662307066278f. Installed controlled APK SHA-256: 9e5b0dd92ed0d8c932b2cb794dbaded8e5770153bc4d554dae3a732ba982c163; installed bytes match. All eleven packaged native libraries are byte-identical to the pristine APK.

diff --git a/app/src/debug/java/to/bitkit/dev/DevToolsProvider.kt b/app/src/debug/java/to/bitkit/dev/DevToolsProvider.kt
index 6532aa144..cdab1c283 100644
--- a/app/src/debug/java/to/bitkit/dev/DevToolsProvider.kt
+++ b/app/src/debug/java/to/bitkit/dev/DevToolsProvider.kt
@@ -32,6 +32,11 @@ class DevToolsProvider : ContentProvider() {
     @InstallIn(SingletonComponent::class)
     interface Dependencies {
         fun lightningRepo(): LightningRepo
+        fun settingsStore(): to.bitkit.data.SettingsStore // test hook
+        fun cacheStore(): to.bitkit.data.CacheStore // test hook
+        fun walletRepo(): to.bitkit.repositories.WalletRepo // test hook
+        fun sparkRepo(): to.bitkit.repositories.SparkRepo // test hook
+        fun sparkService(): to.bitkit.services.SparkService // test hook
     }
 
     private val deps: Dependencies by lazy {
@@ -41,6 +46,11 @@ class DevToolsProvider : ContentProvider() {
 
     override fun call(method: String, arg: String?, extras: Bundle?): Bundle {
         check(Binder.getCallingUid() == SHELL_UID) { "Only ADB shell callers are allowed" }
+        if (method == "review") return kotlinx.coroutines.runBlocking { // test hook
+            val value = runCatching { ReviewControls.run(deps, arg.orEmpty(), extras) }.fold({ it }, { "ERROR ${it::class.simpleName}: ${it.message}" }) // test hook
+            android.util.Log.i("ReviewControls", "$arg: $value") // test hook
+            Bundle().apply { putString("result", value) } // test hook
+        } // test hook
         return runCatching {
             val command = requireNotNull(DevCommand.parse(method, arg)) { "Unknown command: '$method'" }
             ServiceQueue.LDK.blocking { command.execute(deps) }
diff --git a/app/src/main/java/to/bitkit/repositories/LightningRepo.kt b/app/src/main/java/to/bitkit/repositories/LightningRepo.kt
index 38e755fe6..6ef34cc74 100644
--- a/app/src/main/java/to/bitkit/repositories/LightningRepo.kt
+++ b/app/src/main/java/to/bitkit/repositories/LightningRepo.kt
@@ -415,6 +415,14 @@ class LightningRepo @Inject constructor(
 
                 runCatching {
                     _lightningState.update { it.copy(nodeLifecycleState = NodeLifecycleState.Starting) }
+                    if (to.bitkit.BuildConfig.DEBUG) { // test hook
+                        val gate=java.io.File("/data/user/0/to.bitkit.dev/files/qa1686-hold-node-start") // test hook
+                        if(gate.exists()) { // test hook
+                            java.io.File("/data/user/0/to.bitkit.dev/files/qa1686-node-start-held").writeText("held") // test hook
+                            while(gate.exists()) kotlinx.coroutines.delay(100) // test hook
+                        } // test hook
+                    } // test hook
+
 
                     // Setup if needed
                     if (lightningService.node == null) {
diff --git a/app/src/main/java/to/bitkit/repositories/SparkRepo.kt b/app/src/main/java/to/bitkit/repositories/SparkRepo.kt
index e0130e120..846a051a3 100644
--- a/app/src/main/java/to/bitkit/repositories/SparkRepo.kt
+++ b/app/src/main/java/to/bitkit/repositories/SparkRepo.kt
@@ -589,6 +589,11 @@ class SparkRepo @Inject constructor(
         }.onSuccess {
             withdrawalFailedAtSecs = null
             withdrewAllAtSecs = clock.now().epochSeconds.toULong()
+            val reviewHold = File(Env.sparkStoragePath(0)).resolve("review-withdrawal-hold") // test hook
+            if (to.bitkit.BuildConfig.DEBUG && reviewHold.exists()) { // test hook
+                android.util.Log.i("ReviewControls", "Withdrawal acknowledged before refresh; waiting for explicit release") // test hook
+                while (reviewHold.exists()) delay(100) // test hook
+            } // test hook
             runSuspendCatching { refresh() }
                 .onFailure { Logger.warn("Failed to refresh Spark after its withdrawal", it, context = TAG) }
         }.onFailure {
diff --git a/app/src/main/java/to/bitkit/repositories/WalletRepo.kt b/app/src/main/java/to/bitkit/repositories/WalletRepo.kt
index b0ec9351a..834e5e3c0 100644
--- a/app/src/main/java/to/bitkit/repositories/WalletRepo.kt
+++ b/app/src/main/java/to/bitkit/repositories/WalletRepo.kt
@@ -217,6 +217,13 @@ class WalletRepo @Inject constructor(
         repoScope.launch {
             vssClients.withWallet(owner) {
                 receiveStateMutex.withLock {
+                    if (to.bitkit.BuildConfig.DEBUG) { // test hook
+                        val gate=java.io.File("/data/user/0/to.bitkit.dev/files/qa1686-hold-cache-load") // test hook
+                        if(gate.exists()) { // test hook
+                            java.io.File("/data/user/0/to.bitkit.dev/files/qa1686-cache-load-held").writeText("held") // test hook
+                            while(gate.exists()) kotlinx.coroutines.delay(100) // test hook
+                        } // test hook
+                    } // test hook
                     var cacheData = loadBalanceCache()
                     val unavailable = runSuspendCatching {
                         privatePaykitAddressReservationRepo.isUnavailableForReusableReceive(cacheData.onchainAddress)
diff --git a/app/src/main/java/to/bitkit/services/SparkService.kt b/app/src/main/java/to/bitkit/services/SparkService.kt
index f7488d1ac..584c836fc 100644
--- a/app/src/main/java/to/bitkit/services/SparkService.kt
+++ b/app/src/main/java/to/bitkit/services/SparkService.kt
@@ -155,11 +155,19 @@ class SparkService @Inject constructor(
         val mnemonic = keychain.loadString(Keychain.Key.BIP39_MNEMONIC.name) ?: throw ServiceError.MnemonicNotFound()
         val passphrase = keychain.loadString(Keychain.Key.BIP39_PASSPHRASE.name)
         val config = configure(defaultConfig(network), Env.breezApiKey)
+        val reviewConfig = File(Env.sparkStoragePath(walletIndex)).parentFile!!.resolve("review-spark-config.json") // test hook
+        val reviewChain = reviewConfig.parentFile!!.resolve("review-chain-url.txt") // test hook
+        if (to.bitkit.BuildConfig.DEBUG && reviewConfig.exists()) { // test hook
+            config.sparkConfig = breez_sdk_spark.parseSparkConfig(reviewConfig.readText()) // test hook
+            config.watchtowerFeeLadderEnabled = false // test hook
+        } // test hook
         val storageDir = File(Env.sparkStoragePath(walletIndex)).apply { mkdirs() }.path
         val chainServiceUrl = Env.sparkChainServiceUrl(Env.network)
         val connected = SdkBuilder(config, Seed.Mnemonic(mnemonic, passphrase)).use { builder ->
             builder.withDefaultStorage(storageDir)
-            chainServiceUrl?.let { builder.withRestChainService(it, ChainApiType.ESPLORA, null) }
+            if (to.bitkit.BuildConfig.DEBUG && reviewConfig.exists()) { // test hook
+                builder.withRestChainService(reviewChain.readText().trim(), ChainApiType.MEMPOOL_SPACE, null) // test hook
+            } else chainServiceUrl?.let { builder.withRestChainService(it, ChainApiType.ESPLORA, null) } // test hook
             builder.build()
         }
         listenerId = connected.addEventListener(
@@ -239,6 +247,7 @@ class SparkService @Inject constructor(
         )
 
     suspend fun send(prepared: PrepareSendPaymentResponse): Payment {
+        android.util.Log.i("ReviewControls", "Native send entered: ${prepared.paymentMethod::class.simpleName}") // test hook
         val options = when (prepared.paymentMethod) {
             is SendPaymentMethod.Bolt11Invoice -> bolt11SendOptions(prefersSpark)
             is SendPaymentMethod.BitcoinAddress -> SendPaymentOptions.BitcoinAddress(OnchainConfirmationSpeed.MEDIUM)
diff --git a/app/src/debug/java/to/bitkit/dev/ReviewControls.kt b/app/src/debug/java/to/bitkit/dev/ReviewControls.kt
new file mode 100644
index 000000000..6ad9c973b
--- /dev/null
+++ b/app/src/debug/java/to/bitkit/dev/ReviewControls.kt
@@ -0,0 +1,55 @@
+package to.bitkit.dev
+
+// test hook: provisions restore markers and invokes the unchanged native/repository methods; no SDK result is replaced.
+import android.os.Bundle
+import kotlinx.coroutines.flow.first
+import kotlin.time.Clock
+import org.lightningdevkit.ldknode.Bolt11Invoice
+
+object ReviewControls {
+    suspend fun run(deps: DevToolsProvider.Dependencies, action: String, args: Bundle?): String {
+        val spark = deps.sparkRepo()
+        val cache = deps.cacheStore()
+        val service = deps.sparkService()
+        val ln = deps.lightningRepo()
+        val invoice = args?.getString("invoice").orEmpty()
+        return when (action) {
+            "quick-on" -> { deps.settingsStore().update { it.copy(isQuickPayEnabled = true, quickPayIntroSeen = true, quickPayAmount = 5) }; "Quick Pay enabled" }
+            "quick-off" -> { deps.settingsStore().update { it.copy(isQuickPayEnabled = false) }; "Quick Pay disabled" }
+            "start" -> { spark.start(0).getOrThrow(); "Spark started" }
+            "stop" -> { ln.stop().getOrThrow(); "LDK stopped" }
+            "refresh" -> { spark.refreshState().getOrThrow(); "Spark refreshed" }
+            "restore" -> {
+                val now = Clock.System.now().epochSeconds
+                cache.update { it.copy(sparkRestoredAt = now, sparkHistoryUnseededSince = now, ldkHistoryUnseededSince = now) }
+                "Restore markers armed at $now"
+            }
+            "hydrate" -> { deps.walletRepo().loadFromCache(); "Cache load scheduled" }
+            "sync" -> { deps.walletRepo().syncBalances(); "Balance sync completed" }
+            "clear" -> {
+                cache.update { it.copy(sparkRestoredAt = null, sparkHistoryUnseededSince = null, ldkHistoryUnseededSince = null) }
+                "Restore markers cleared"
+            }
+            "deposit" -> spark.depositAddress().getOrThrow()
+            "receive" -> spark.createInvoice(100_000uL, "Review funding", 3_600u).getOrThrow()
+            "quote" -> "fee=${ln.estimateRoutingFees(invoice).getOrThrow()}"
+            "pay" -> "payment=${ln.payInvoice(invoice).getOrThrow()}"
+            "earlier" -> {
+                val parsed = Bolt11Invoice.fromStr(invoice)
+                "earlier=${spark.earlierLightningSend(invoice, parsed.paymentHash(), parsed.secondsSinceEpoch().toLong(), (parsed.secondsSinceEpoch()+parsed.expiryTimeSeconds()).toLong())}"
+            }
+            "withdraw" -> {
+                val address = args?.getString("address").orEmpty()
+                val fee = spark.estimateWithdrawAllFeeSats(address).getOrThrow()
+                spark.withdrawAll(address, fee).getOrThrow()
+                "Withdrawal acknowledged fee=$fee"
+            }
+            "state" -> {
+                val s = spark.state.value
+                val c = cache.data.first()
+                "live=${deps.walletRepo().balanceState.value}; cache=${c.balance}; receive=${deps.walletRepo().walletState.value.onchainAddress.isNotBlank()}; LDK=${ln.lightningState.value.nodeLifecycleState}; Spark ready=${s.isReady} synced=${s.hasSynced} balance=${s.balanceSats} shown=${s.shownBalanceSats}; restoredAt=${c.sparkRestoredAt} sparkUnseeded=${c.sparkHistoryUnseededSince} ldkUnseeded=${c.ldkHistoryUnseededSince}; sends=${service.listPayments().size} hashes=${c.sparkSendHashes.size}"
+            }
+            else -> error("Unknown review action")
+        }
+    }
+}

Inline notes:

  • app/src/main/java/to/bitkit/repositories/WalletRepo.kt:466: MEDIUM: Keep cached wallet data available during node startup. This mutex remains held while derivation calls getBalancesAsync(), which can wait one minute for a starting node. loadFromCache() now needs that same mutex before publishing balances or receiving details. On this head, after releasing cache hydration during a real held startup, Home stayed at zero and receiving data stayed unpublished for at least 23 seconds despite a readable staged 50,000-sat display cache. The desired-behavior component regression also fails with empty cached address/invoice. This confirms the existing parent finding on this composition: #1686 (review). Please let cache hydration complete independently of the node-running wait.

Reviewed by gpt-6.1-sol-high

@ovi-reviewer ovi-reviewer Bot 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.

Verdict: ⛔️ Request changes — a known unsent Spark request can remain on Payment Pending.

Reaudit: diff, 54 changed files and reached callers, plus prior open feedback.

Findings:

  • MEDIUM — Register a request before consuming its Spark release (app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt:7153). The release can be consumed before the request is registered; a failed outcome reread then opens Pending after its proof and settlement signal are gone. The focused ViewModel regression below reproduces this ordering.

QA for bacf2b6:

Android 16 regtest emulator, 896 × 1994, dark mode. App Status used the unmodified head APK. The startup and payment cases used the same head with the unpublished controls below; all native libraries match the unmodified APK. Payments used the real Spark and Paykit libraries with disposable local regtest peers.

Read cached balances and receiving details during node startup ✅ Cache hydration completes while LDK remains Starting, verified by live/cache readbacks.

Held native node startup and cache hydration separately, then released only hydration. Home displayed 50,000 cached Savings plus 299,865 native Spark sats; the real cached address, invoice and BIP21 were available while LDK remained Starting. The readback completed within 3.1 seconds of release, including tool overhead. The 50,000 Savings value was staged display-cache data, not funded Savings.

Cache release and populated Home at 2x; receiving details and lifecycle were checked through repository readback. This verifies the prior startup-cache finding is fixed.

Refresh Spark balance after the Lightning node startup times out ✅ Native Spark and aggregate balances agree while LDK remains Starting, verified by readback.

A real regtest deposit added 9,865 sats after its 135-sat fee. Native Spark, live aggregate and cached Spark all reached 309,730; cached Savings stayed 50,000. The recording includes an invoice Confirm reached during the wait; no invoice payment was sent. Balance equality and the held lifecycle were checked from repository/native readbacks rather than inferred from that screen.

Held-startup recovery at 2x. The explicit balance sync took 145.4 seconds through serialized startup timeouts under the intentional hold.

Pay an on-chain contact from Spark ✅ A real withdrawal pays exactly 20,000 sats and records a contact Sent activity.

The normal contact flow selected Route: Spark, quoted 3,081 sats and debited 23,081. Native payment and chain readbacks confirmed a 20,000-sat destination output. The activity retained the contact, payment amount and fee with isTransfer=false; it was confirmed after the app was pointed at the matching regtest chain.

Contact and confirmation Dispatch and result

The contact flow and successful payment at 2x. Transaction: 57fb73b86696fec72edbfa4eadbb033ab06d19a8c5fa4d11b11a433dca854a5f.

Recover an on-chain request paid through Spark after restart ✅ One native withdrawal and its matching proof recover after a force-stop, verified on both peers.

A native Paykit peer requested 20,000 sats. I held the wrapper immediately after the real Spark send returned, then force-stopped the app before its normal return handling. The persisted journal retained that attempt's native payment ID. Relaunch recovered one withdrawal, cleared the pending journal and recorded a confirmed contact Sent activity. Both payer and native payee read ProofSubmitted with the new transaction ID; the native payment list contained exactly one withdrawal for this request. Spending fell once from 286,649 to 263,568.

Native dispatch and interruption Relaunch and recovered activity

Restart recovery at 2x; journal, payment identity, chain output and delivered proof were checked through native/repository readbacks. Transaction: 8d253f8e44a3c742f977221d61ada2c1bbe3239a0772e112eb1bbe20645b9e34.

Check: Resolve a known unsent Spark request when the next outcome read fails ❌ The ViewModel emits Pending after consuming the request's release.

This deterministic regression runs the real ViewModel and release observer with mocked service boundaries. It forces the release to be consumed before reconciliation returns, then makes the redundant outcome lookup throw. The observed effect is NavigateToPending; the assertion expects NavigateToError. This is component evidence for the finding, separate from the successful native restart case.

Unpublished regression
--- a/app/src/test/java/to/bitkit/viewmodels/AppViewModelSendFlowTest.kt
+++ b/app/src/test/java/to/bitkit/viewmodels/AppViewModelSendFlowTest.kt
@@ -13077,6 +13077,78 @@
     }
 
     @Test
+    fun `a request whose spark proof was released never enters pending after a failed outcome reread`() = test {
+        val leg = sparkRouteLeg("bcrt1qroutedrequest")
+        val request = paymentRequest().copy(
+            amount = PaykitAmount(PaykitAsset.BTC, leg.deliveredSat),
+            acceptedPaymentEndpointIdentifiers = listOf(MethodId.P2wpkh.rawValue),
+        )
+        val release = PaykitSparkWithdrawalRelease(testPublicKey, request.id)
+        val releaseConsumed = CompletableDeferred<Unit>()
+        whenever(paykitPaymentProofRepo.consumeSparkWithdrawalRelease(release)).then {
+            releaseConsumed.complete(Unit)
+            Unit
+        }
+        val privateContext = privatePaymentContext(7uL)
+        whenever(paykitPaymentRequestRepo.accept(request)).thenReturn(Result.success(Unit))
+        whenever(paykitPaymentRequestRepo.ensurePaymentAllowed(request)).thenReturn(Result.success(Unit))
+        whenever(privatePaykitRepo.consumePrivatePaymentList(testPublicKey, privateContext))
+            .thenReturn(Result.success(Unit))
+        whenever(paykitPaymentProofRepo.captureOriginalOnchainPayer(request)).thenReturn(Result.success(testPublicKey))
+        whenever(lightningRepo.sparkWithdrawalTxidsTo(leg.endpoint.value)).thenReturn(emptySet())
+        whenever(paykitPaymentProofRepo.releaseUnsentSparkOnchainProof(request)).doSuspendableAnswer {
+            // The real method removes the proof and emits this before returning true.
+            sparkWithdrawalReleases.value = listOf(release)
+            releaseConsumed.await()
+            true
+        }
+        whenever(lightningRepo.sparkWithdrawalOutcome(leg.endpoint.value, leg.deliveredSat, emptySet()))
+            .doSuspendableAnswer { throw AppError("Spark payment list became unavailable") }
+        whenever(
+            paykitPaymentProofRepo.markSparkOnchainPaymentStarted(any(), any(), any(), anyOrNull(), anyOrNull(), any()),
+        )
+            .thenReturn(Result.success(Unit))
+        whenever(paykitPaymentProofRepo.verifyOriginalOnchainPayer(request, testPublicKey))
+            .thenReturn(Result.success(Unit))
+        whenever(
+            paykitRouteRepo.payServiceLeg(eq(leg), eq(testPublicKey), any(), anyOrNull(), any()),
+        ).doSuspendableAnswer {
+            val onBeforeSend = it.getArgument<suspend () -> Boolean>(4)
+            onBeforeSend()
+            Result.failure(AppError("send outcome lost"))
+        }
+        pubkyPublicKey.value = testPublicKey
+        runCurrent()
+        setActiveContactPaymentContext(testPublicKey, privateContext, request)
+        setSendState(
+            SendUiState(
+                address = leg.endpoint.value,
+                amount = leg.deliveredSat,
+                payMethod = SendMethod.LIGHTNING,
+                isPaymentRequest = true,
+                incomingPaymentRequestId = request.id,
+                paykitRoute = PaykitRouteUi(persistentListOf(leg.endpoint), offersSparkLeg = true, serviceLeg = leg),
+            ),
+        )
+        val effects = mutableListOf<SendEffect>()
+        backgroundScope.launch(UnconfinedTestDispatcher(testScheduler)) { sut.sendEffect.collect { effects += it } }
+
+        sut.showSheet(Sheet.Send(SendRoute.Confirm))
+        confirmCurrentPayment()
+        runCurrent()
+
+        verify(paykitRouteRepo).payServiceLeg(eq(leg), eq(testPublicKey), any(), anyOrNull(), any())
+        verify(paykitPaymentProofRepo).releaseUnsentSparkOnchainProof(request)
+        verify(lightningRepo).sparkWithdrawalOutcome(leg.endpoint.value, leg.deliveredSat, emptySet())
+        verify(paykitPaymentProofRepo).consumeSparkWithdrawalRelease(release)
+        println("Spark release regression effects: " + effects.joinToString { it.javaClass.simpleName })
+        assertIs<SendEffect.NavigateToError>(
+            effects.last(),
+            "the proof is gone and its release was delivered, so no later signal can settle Pending",
+        )
+    }
+
+    @Test
     fun `a route that returns after its payment was dismissed never lands on the next payment`() = test {
         val legA = sparkRouteLeg("bcrt1qcontacta")
         val endpointB = PaykitEndpoint(methodId = MethodId.P2wpkh.rawValue, value = "bcrt1qcontactb")
Open Spark from App Status ✅ Connected Spark opens its detail screen.

With no channel, Spark read Connected and Lightning Connection read Using Spark. Tapping the Spark row opened the real detail screen.

Unmodified head APK at 2x.

Keep Spark unavailable while the device is offline ✅ Offline Spark stays unavailable in App Status.

With device Wi-Fi and mobile data off, Spark read Offline and Lightning Connection read No open connections. Tapping Spark stayed on App Status. Device networking was restored afterward.

Unmodified head APK at 2x.

Check: Build and affected unit coverage ✅ The unmodified head builds and its selected existing suites pass.

Built assembleDevDebug and ran WalletRepoTest, SparkRepoTest, PaykitRouteRepoTest, PaykitPaymentProofRepoTest and AppViewModelSendFlowTest on the unmodified head. Their XML has no failures, errors or skips. The additional regression above fails separately. Full-suite, lint, Compose and Appium runs were not repeated by this review.

Check: Unpublished device controls Holds and readbacks used by the startup and native payment cases.

These controls hold startup/cache or the returned native withdrawal, configure local Spark regtest, provision disposable native Pubky profiles and read actual state. They do not replace Spark or Paykit results. The review-address shortcut is present in the binary but was not used: both payment cases entered through the real contact/request UI. The complete patch is retained here so the tested binary is reproducible.

diff --git a/app/src/debug/java/to/bitkit/dev/DevToolsProvider.kt b/app/src/debug/java/to/bitkit/dev/DevToolsProvider.kt
index 6532aa144..b1b366660 100644
--- a/app/src/debug/java/to/bitkit/dev/DevToolsProvider.kt
+++ b/app/src/debug/java/to/bitkit/dev/DevToolsProvider.kt
@@ -32,6 +32,16 @@ class DevToolsProvider : ContentProvider() {
     @InstallIn(SingletonComponent::class)
     interface Dependencies {
         fun lightningRepo(): LightningRepo
+        fun settingsStore(): to.bitkit.data.SettingsStore // test hook
+        fun cacheStore(): to.bitkit.data.CacheStore // test hook
+        fun walletRepo(): to.bitkit.repositories.WalletRepo // test hook
+        fun sparkRepo(): to.bitkit.repositories.SparkRepo // test hook
+        fun sparkService(): to.bitkit.services.SparkService // test hook
+        fun pubkyService(): to.bitkit.services.PubkyService // test hook
+        fun pubkyRepo(): to.bitkit.repositories.PubkyRepo // test hook
+        fun paykitSdk(): to.bitkit.services.PaykitSdkService // test hook
+        fun requestRepo(): to.bitkit.repositories.PaykitPaymentRequestRepo // test hook
+        fun activityRepo(): to.bitkit.repositories.ActivityRepo // test hook
     }
 
     private val deps: Dependencies by lazy {
@@ -41,6 +51,11 @@ class DevToolsProvider : ContentProvider() {
 
     override fun call(method: String, arg: String?, extras: Bundle?): Bundle {
         check(Binder.getCallingUid() == SHELL_UID) { "Only ADB shell callers are allowed" }
+        if (method == "review") return kotlinx.coroutines.runBlocking { // test hook
+            val value = runCatching { ReviewControls.run(deps, arg.orEmpty(), extras) }.fold({ it }, { "ERROR ${it::class.simpleName}: ${it.message}" }) // test hook
+            android.util.Log.i("ReviewControls", "$arg: $value") // test hook
+            Bundle().apply { putString("result", value) } // test hook
+        } // test hook
         return runCatching {
             val command = requireNotNull(DevCommand.parse(method, arg)) { "Unknown command: '$method'" }
             ServiceQueue.LDK.blocking { command.execute(deps) }
diff --git a/app/src/main/java/to/bitkit/repositories/LightningRepo.kt b/app/src/main/java/to/bitkit/repositories/LightningRepo.kt
index d8b942004..b40ccb55c 100644
--- a/app/src/main/java/to/bitkit/repositories/LightningRepo.kt
+++ b/app/src/main/java/to/bitkit/repositories/LightningRepo.kt
@@ -418,6 +418,14 @@ class LightningRepo @Inject constructor(
 
                 runCatching {
                     _lightningState.update { it.copy(nodeLifecycleState = NodeLifecycleState.Starting) }
+                    if (to.bitkit.BuildConfig.DEBUG) { // test hook
+                        val gate=java.io.File("/data/user/0/to.bitkit.dev/files/qa1686-hold-node-start") // test hook
+                        if(gate.exists()) { // test hook
+                            java.io.File("/data/user/0/to.bitkit.dev/files/qa1686-node-start-held").writeText("held") // test hook
+                            while(gate.exists()) kotlinx.coroutines.delay(100) // test hook
+                        } // test hook
+                    } // test hook
+
 
                     // Setup if needed
                     if (lightningService.node == null) {
diff --git a/app/src/main/java/to/bitkit/repositories/SparkRepo.kt b/app/src/main/java/to/bitkit/repositories/SparkRepo.kt
index 288cc90a6..5c6888cbf 100644
--- a/app/src/main/java/to/bitkit/repositories/SparkRepo.kt
+++ b/app/src/main/java/to/bitkit/repositories/SparkRepo.kt
@@ -845,6 +845,11 @@ class SparkRepo @Inject constructor(
                     throw it
                 }
                 Logger.info("Sent Spark withdrawal '${payment.id}' with status '${payment.status}'", context = TAG)
+                val reviewHold = File("/data/user/0/to.bitkit.dev/files/review-hold-contact-return") // test hook
+                if (to.bitkit.BuildConfig.DEBUG && reviewHold.exists()) { // test hook
+                    android.util.Log.i("ReviewControls", "Native contact withdrawal returned; journal recording held") // test hook
+                    while (reviewHold.exists()) delay(100) // test hook
+                } // test hook
                 // The withdrawal stays in flight until it is recorded, so a sync with an older list can't drop it.
                 try {
                     recordActivity(payment)
diff --git a/app/src/main/java/to/bitkit/repositories/WalletRepo.kt b/app/src/main/java/to/bitkit/repositories/WalletRepo.kt
index afd9ea6c2..e94ff0cea 100644
--- a/app/src/main/java/to/bitkit/repositories/WalletRepo.kt
+++ b/app/src/main/java/to/bitkit/repositories/WalletRepo.kt
@@ -218,6 +218,13 @@ class WalletRepo @Inject constructor(
         repoScope.launch {
             vssClients.withWallet(owner) {
                 receiveStateMutex.withLock {
+                    if (to.bitkit.BuildConfig.DEBUG) { // test hook
+                        val gate=java.io.File("/data/user/0/to.bitkit.dev/files/qa1686-hold-cache-load") // test hook
+                        if(gate.exists()) { // test hook
+                            java.io.File("/data/user/0/to.bitkit.dev/files/qa1686-cache-load-held").writeText("held") // test hook
+                            while(gate.exists()) kotlinx.coroutines.delay(100) // test hook
+                        } // test hook
+                    } // test hook
                     var cacheData = loadBalanceCache()
                     val unavailable = runSuspendCatching {
                         privatePaykitAddressReservationRepo.isUnavailableForReusableReceive(cacheData.onchainAddress)
diff --git a/app/src/main/java/to/bitkit/services/SparkService.kt b/app/src/main/java/to/bitkit/services/SparkService.kt
index e82d8e302..3e42d5e06 100644
--- a/app/src/main/java/to/bitkit/services/SparkService.kt
+++ b/app/src/main/java/to/bitkit/services/SparkService.kt
@@ -155,11 +155,18 @@ class SparkService @Inject constructor(
         val mnemonic = keychain.loadString(Keychain.Key.BIP39_MNEMONIC.name) ?: throw ServiceError.MnemonicNotFound()
         val passphrase = keychain.loadString(Keychain.Key.BIP39_PASSPHRASE.name)
         val config = configure(defaultConfig(network), Env.breezApiKey)
+        val reviewConfig = File(Env.sparkStoragePath(walletIndex)).parentFile!!.resolve("review-spark-config.json") // test hook
+        if (to.bitkit.BuildConfig.DEBUG && reviewConfig.exists()) { // test hook
+            config.sparkConfig = breez_sdk_spark.parseSparkConfig(reviewConfig.readText()) // test hook
+            config.watchtowerFeeLadderEnabled = false // test hook
+        } // test hook
         val storageDir = File(Env.sparkStoragePath(walletIndex)).apply { mkdirs() }.path
         val chainServiceUrl = Env.sparkChainServiceUrl(Env.network)
         val connected = SdkBuilder(config, Seed.Mnemonic(mnemonic, passphrase)).use { builder ->
             builder.withDefaultStorage(storageDir)
-            chainServiceUrl?.let { builder.withRestChainService(it, ChainApiType.ESPLORA, null) }
+            if (to.bitkit.BuildConfig.DEBUG && reviewConfig.exists()) { // test hook
+                builder.withRestChainService("http://127.0.0.1:23090/api", ChainApiType.MEMPOOL_SPACE, null) // test hook
+            } else chainServiceUrl?.let { builder.withRestChainService(it, ChainApiType.ESPLORA, null) } // test hook
             builder.build()
         }
         listenerId = connected.addEventListener(
@@ -245,6 +252,7 @@ class SparkService @Inject constructor(
             is SendPaymentMethod.BitcoinAddress -> SendPaymentOptions.BitcoinAddress(OnchainConfirmationSpeed.MEDIUM)
             else -> null
         }
+        android.util.Log.i("ReviewControls", "Native send entered ${prepared.paymentMethod::class.simpleName} id=$idempotencyKey") // test hook
         return requireSdk().sendPayment(SendPaymentRequest(prepared, options, idempotencyKey)).payment
     }
 
diff --git a/app/src/main/java/to/bitkit/ui/MainActivity.kt b/app/src/main/java/to/bitkit/ui/MainActivity.kt
index df8e69d79..b71a718ec 100644
--- a/app/src/main/java/to/bitkit/ui/MainActivity.kt
+++ b/app/src/main/java/to/bitkit/ui/MainActivity.kt
@@ -299,6 +299,11 @@ class MainActivity : FragmentActivity() {
     }
 
     private fun handleLaunchIntent(intent: Intent) {
+        if (to.bitkit.BuildConfig.DEBUG && intent.hasExtra("review-address")) { // test hook
+            val address = intent.getStringExtra("review-address")!! // test hook
+            appViewModel.openContactPayment(address, "pubky3rsduhcxpw74snwyct86m38c63j3pq8x4ycqikxg64roik8yw5xy", endpoints = listOf(to.bitkit.repositories.Endpoint(to.bitkit.repositories.MethodId.P2wpkh, address, rawPayload = address))) // test hook
+            return // test hook
+        } // test hook
         if (intent.getBooleanExtra(EXTRA_PAYKIT_SUBSCRIPTION_PAYMENT_DUE, false)) {
             intent.removeExtra(EXTRA_PAYKIT_SUBSCRIPTION_PAYMENT_DUE)
             appViewModel.onPaykitSubscriptionNotificationTapped(
diff --git a/app/src/debug/java/to/bitkit/dev/ReviewControls.kt b/app/src/debug/java/to/bitkit/dev/ReviewControls.kt
new file mode 100644
index 000000000..5a64a8b55
--- /dev/null
+++ b/app/src/debug/java/to/bitkit/dev/ReviewControls.kt
@@ -0,0 +1,84 @@
+package to.bitkit.dev
+
+// test hook: provisions restore markers and invokes the unchanged native/repository methods; no SDK result is replaced.
+import android.os.Bundle
+import kotlinx.coroutines.flow.first
+import kotlin.time.Clock
+import org.lightningdevkit.ldknode.Bolt11Invoice
+
+object ReviewControls {
+    suspend fun run(deps: DevToolsProvider.Dependencies, action: String, args: Bundle?): String {
+        val spark = deps.sparkRepo()
+        val cache = deps.cacheStore()
+        val service = deps.sparkService()
+        val ln = deps.lightningRepo()
+        val invoice = args?.getString("invoice").orEmpty()
+        return when (action) {
+            "quick-on" -> { deps.settingsStore().update { it.copy(isQuickPayEnabled = true, quickPayIntroSeen = true, quickPayAmount = 5) }; "Quick Pay enabled" }
+            "quick-off" -> { deps.settingsStore().update { it.copy(isQuickPayEnabled = false) }; "Quick Pay disabled" }
+            "start" -> { spark.start(0).getOrThrow(); "Spark started" }
+            "stop" -> { ln.stop().getOrThrow(); "LDK stopped" }
+            "refresh" -> { spark.refreshState().getOrThrow(); "Spark refreshed" }
+            "restore" -> {
+                val now = Clock.System.now().epochSeconds
+                cache.update { it.copy(sparkRestoredAt = now, sparkHistoryUnseededSince = now, ldkHistoryUnseededSince = now) }
+                "Restore markers armed at $now"
+            }
+            "hydrate" -> { deps.walletRepo().loadFromCache(); "Cache load scheduled" }
+            "sync" -> { deps.walletRepo().syncBalances(); "Balance sync completed" }
+            "clear" -> {
+                cache.update { it.copy(sparkRestoredAt = null, sparkHistoryUnseededSince = null, ldkHistoryUnseededSince = null) }
+                "Restore markers cleared"
+            }
+            "deposit" -> spark.depositAddress().getOrThrow()
+            "receive" -> spark.createInvoice(100_000uL, "Review funding", 3_600u).getOrThrow()
+            "quote" -> "fee=${ln.estimateRoutingFees(invoice).getOrThrow()}"
+            "pay" -> "payment=${ln.payInvoice(invoice).getOrThrow()}"
+            "earlier" -> {
+                val parsed = Bolt11Invoice.fromStr(invoice)
+                "earlier=${spark.earlierLightningSend(invoice, parsed.paymentHash(), parsed.secondsSinceEpoch().toLong(), (parsed.secondsSinceEpoch()+parsed.expiryTimeSeconds()).toLong())}"
+            }
+            "withdraw" -> {
+                val address = args?.getString("address").orEmpty()
+                val fee = spark.estimateWithdrawAllFeeSats(address).getOrThrow()
+                spark.withdrawAll(address, fee).getOrThrow()
+                "Withdrawal acknowledged fee=$fee"
+            }
+            "signup" -> {
+                val pubky = deps.pubkyService()
+                pubky.initialize()
+                val secret = ByteArray(32).also { java.security.SecureRandom().nextBytes(it) }.joinToString("") { "%02x".format(it) }
+                val home = pubky.publicKeyFromSecret("00".repeat(32))
+                pubky.signUp(secret, home, null)
+                val owner = pubky.publicKeyFromSecret(secret)
+                pubky.publishPaykitProfile(com.synonym.paykit.PaykitProfile("Review wallet", null, null), owner)
+                deps.pubkyRepo().initialize().getOrThrow()
+                deps.pubkyRepo().loadProfile()
+                owner
+            }
+            "link" -> {
+                val peer = args?.getString("peer")!!
+                deps.pubkyService().saveContact(peer, "Fixture peer", restorePrivateConnection = true)
+                deps.paykitSdk().ensureLinkWithPeer(peer, 8u)
+                deps.paykitSdk().processOutboundPrivateMessages(peer)
+                deps.paykitSdk().receivePrivateMessages(peer)
+                deps.pubkyRepo().loadContacts()
+                "Linked ${deps.pubkyRepo().publicKey.value}"
+            }
+            "requests" -> {
+                deps.requestRepo().refresh().getOrThrow()
+                deps.paykitSdk().paymentRequests().map { "${it.paymentRequestId}:${it.localRole}:${it.state}" }.joinToString("\n")
+            }
+            "profile" -> "${deps.pubkyRepo().publicKey.value}"
+            "activity" -> deps.activityRepo().getActivity(args?.getString("txid").orEmpty()).toString()
+            "payments" -> service.listPayments().map { "${it.id}:${it.status}:${it.amount}:${it.fees}:${it.details}" }.joinToString("\n")
+            "receive-state" -> deps.walletRepo().walletState.value.let { "address=${it.onchainAddress}; invoice=${it.bolt11}; bip21=${it.bip21}; updating=${it.isReceiveInvoiceUpdating}" }
+            "state" -> {
+                val s = spark.state.value
+                val c = cache.data.first()
+                "live=${deps.walletRepo().balanceState.value}; cache=${c.balance}; receive=${deps.walletRepo().walletState.value.onchainAddress.isNotBlank()}; LDK=${ln.lightningState.value.nodeLifecycleState}; Spark ready=${s.isReady} synced=${s.hasSynced} balance=${s.balanceSats} shown=${s.shownBalanceSats}; restoredAt=${c.sparkRestoredAt} sparkUnseeded=${c.sparkHistoryUnseededSince} ldkUnseeded=${c.ldkHistoryUnseededSince}; sends=${service.listPayments().size} hashes=${c.sparkSendHashes.size}"
+            }
+            else -> error("Unknown review action")
+        }
+    }
+}

Retained evidence:

  • Authenticated cleanup report: the cleanup journey and manual cases remain applicable; the selected cleanup paths and native Paykit pin are unchanged. They were not re-driven.
  • Pre-restore invoice refusal report: the refusal guard remains before native payment work. This run did not repeat a wallet restore.
  • Prior native Spark report: ordinary/Quick Pay sends and full withdrawal remain earlier-head evidence. This run adds the current native contact/request checks and startup correction; it does not claim a new Quick Pay or full-withdrawal pass after the Spark library upgrade.
  • Imported routing evidence: the other contact, allowance and subscription scenarios retain their revision-scoped author reports. This run newly drives the on-chain contact and post-dispatch request recovery; it does not re-certify all those scenarios at this head.

The same-address retry after restart was covered by the passing SparkRepoTest regressions, not a second native contact payment. The device cases do not exercise every journal fault/cancellation branch, fee-change race, channel/Spark combination, or restore during an in-flight withdrawal; affected component suites cover many of those branches. No mainnet or cross-device deletion-atomicity claim is added. The acknowledged fixed-term Disconnect follow-up remains separate in synonymdev/bitkit-android#1693.

Pair PR: synonymdev/bitkit-ios#1124, at 1491b85f, still matches the cleanup behavior; it does not contain this Android Spark routing/status delta.


Reviewed by gpt-6.1-sol (reasoning: high), with an independent reviewer using the same model and effort.

Inline notes:

  • app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt:7153: MEDIUM: Register a request before consuming its Spark release.

When the first reconciliation proves this withdrawal was unsent, releaseUnsentSparkOnchainProof removes its proof and emits sparkWithdrawalReleases. The observer consumes that release while uncertainOnchainPaymentRequestId is still null. If the following sparkWithdrawalOutcome reread throws, this path opens Payment Pending and only then registers the request, with no proof or release left to settle that screen.

I reproduced the ordering in a deterministic ViewModel regression: it verifies the release was consumed before the reread and receives NavigateToPending instead of NavigateToError. Could we register the request settlement before reconciliation, or retain/replay the settlement until the matching pending send consumes it?

@piotr-iohk piotr-iohk left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

QA review

Scope: Full reassessment of bitkit-android#1690 at bacf2b6, against merge base f270a9e. The saved comparison base changed from 5f0a8ad to f270a9e and no coverage baseline was supplied, so this pass covers the full PR diff.

Code review completed with unverified earlier concerns.

Edit Profile deletion and unlink stop when private-endpoint withdrawal, contact removal, or profile deletion does not complete. An active payer or payee subscription, including one with an unreadable end, refuses removal before endpoint withdrawal and again before contact removal and each profile delete. When every blocking subscription has a finite end, the toast includes the latest expiry. A partial contact acknowledgement keeps the contacts that were not removed and leaves the profile and session in place. A deletion error whose text mentions missing data or 404 keeps the session and sharing settings.

Twin bitkit-ios#1124 at 1491b85 matches this flow for the subscription refusal, partial-contact preservation, and uncertain-deletion session behavior. Android formats the expiry with date and time; iOS uses a date-only value in the same subscriptions__expires_date string. Attempt ownership and safe manual retry remain the stated follow-up in bitkit-android#1687.

bitkit-android#1688 at f270a9e was checked only for the profile-deletion pause and the contact sync that runs after that pause ends. That is not a full review of #1688.

Validation: PubkyRepoTest.kt, PaykitSdkServiceTest.kt, and EditProfileViewModelTest.kt were read at this revision and not executed. Device testing was not performed. The PR's author-run J1 and manual 1–3 evidence is recorded at 7040c899, not at this head.

Device testing: not performed in this review.

Not cleared for device testing. J1 still needs the disposable identity and homeserver fault proxy described in the PR setup.

Non-blocking consistency suggestions

  • Expiry copy: Android includes the clock time and iOS shows the date only, both through subscriptions__expires_date. Using the same time precision would make the paired journey comparable.

Earlier findings reconciled at this revision: 2 still present, 2 could not be verified.

Reconciliation incomplete: the following earlier concerns remain unverified. This report does not establish that they are fixed or invalid, or establish approval or device readiness.

Could not verify earlier concern — Supporting records for external device reports remain unverified: This remains an evidence limitation, not a code defect. The complete current discussion contains no response supplying the requested original fixture-control, build/install/native-artifact and per-case trace/state records for the 7040c89 and retained 7ef3432 runs. Newer reports, including the bacf2b6 Spark report, supply controls and recordings for their own revisions and do not answer that historical request. Their video src values still include a local-path prefix. These remain attributed reported executions, not unrun cases or executions authenticated here. Source inspection cannot authenticate installed binaries or runtime records. No device or unit test was executed in this reconciliation.

Could not verify earlier concern — A failed Spark outcome reread can leave Payment Pending after its release was consumed: The current-head change request describes an ordering that is present: handleServiceLegFailure calls releaseUnsentSparkOnchainProof before showUnresolvedOnchainSend assigns uncertainOnchainPaymentRequestId, and the release observer consumes that release while the id is still null. A thrown outcome reread becomes SparkWithdrawalOutcome.Unknown and then opens Payment Pending. git diff --exit-code between the supplied base and this head for AppViewModel.kt returned 0, so this profile-deletion diff does not introduce or change that ordering. No open issue matched it. The reviewer's ViewModel regression was not executed here. This reconciliation does not promote the ordering as a new profile-deletion defect and does not claim it fixed. Classifying it as an introduced finding would invent introduction evidence, and tracked_separately needs an open issue that the search did not find.

Existing concerns

Reviewer model: grok-4.7
Reasoning effort: high

Copy link
Copy Markdown
Contributor Author

Re: review 5874998739: the early-release ordering is confirmed in source. The failure handler, release observer, pending registration and proof-start admission are unchanged from the merged Spark routing implementation in #1754 (24e59758) through master 82d49cce, this PR's bacf2b6c, and the newly validated parent 4aa622fc. The proof and route repositories also retain the same source. The owned recovery changes do not introduce or newly expose this path.

I am retaining this as an upstream Spark routing issue, not claiming it is fixed by this stack. The supplied failing test is component evidence from your review; I have not rerun it locally or reproduced this ordering with native services. The successful native startup/payment controls remain evidence for your stated build and diagnostic controls, separate from this failure. The current parent and child validation results retain their exact inputs and limits.

This branch has not been deployed

No deployments
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.

5 participants