Repository navigation
fix: bind profile removal to its original owner - #1693
ben-kaufman wants to merge 12 commits into
Conversation
|
Triage: 3.1.0, with #1687 that it fixes (shipped profile deletion). |
 [High impact] The change reaches profile removal, authenticated Paykit work, and wallet teardown. Fix the stuck recovery warning and stale wallet state after credential erasure before merging. Findings
SummaryThis PR binds profile removal to the confirmed owner and adds durable local recovery.
Acknowledged scope: ben-kaufman explicitly deferred native cross-device conditional contact deletion and complete iOS integration under #1687. Complete authenticated journeys and controlled teardown tests are also listed as unfinished; these were not treated as new findings. Diagram%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Capture owner and records] --> B[User confirms once]
B --> C[Check owner and records]
C --> D[Persist pending cleanup]
D --> E[Remove remote data]
E --> F[Erase local credentials]
F --> G[Clear private state]
G --> H[Clear pending cleanup]
E -->|Failure| I[Review or local Disconnect]
F -->|Failure| J[Local Disconnect]
G -->|Failure| J
I -->|Review| A
Reviews (1) · Last reviewed commit: "fix: bind profile removal to its origina..." · Reviewed by Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
talosmachina
left a comment
talosmachina
left a comment
There was a problem hiding this comment.
1 finding (1 P1). Binds profile deletion and disconnect to the confirmed identity, with a durable local cleanup obligation, a read-only Review, and wallet-wipe draining. Reviewed ff4dc35e, oversized tier: 59 files. I reasoned from the code; I did not build here, per the Bitkit track's rule for auto reviews.
What I checked, and 3 candidates I ruled out
Read in full: ProfileRemovalStore.kt, ProfileRemovalCoordinator.kt, PubkyIdentityOperations.kt.
Read as diff: PubkyRepo.kt, PaykitSdkService.kt, PaykitSdkOperationLock.kt, ContactPaymentSettingsRepo.kt, WipeWalletUseCase.kt, AppViewModel.kt, ProfileViewModel.kt, PubkyService.kt.
Not read: the remaining UI, backup and test files.
CI: build is red. One unit test failed, PubkyAuthManifestTest > signup and authorization links resolve only through their enabled aliases (pubkyring://signup?hs=homeserver resolved where none was expected): 4725 tests, 1 failed. The diff doesn't touch the manifest or the alias registrar, and build passed on #1690, so I couldn't tie the failure to this change. It still needs a look or a re-run. detekt and build-local are green; e2e was still running.
Ruled out
- Wipe deadlock against an in-flight removal:
withWalletWiperevokes the removal and awaitsdrainedbeforeacquire, andacquire(admittedGeneration)refuses a stale caller instead of queueing it behind the wipe. - Review widening the target set:
reviewCeilingis shared across confirmations and only replaced by a subset (containsAll).remove()rejects a confirmation whose deletion is no longer the ceiling. - Local disconnect replaying remote work after restart:
prepareSignOutsetslocalOnlywhenever a pending attempt exists, so onlyfinishProfileRemovalLocallyand the private and marker completion run. - Cold reader: it timed out on the finding, so the P1 stands on my own trace of the call chain.
Merge confidence: 3/5, one P1, the oversized tier, and build red for a reason I could not tie to the change.
Reviewer model: claude-opus-5-5
Reasoning effort: Unknown (not exposed to the reviewing session)
Regtest APKDownload bitkit-dev-debug universal APK (expires in 30 days). |
![ovi-reviewer[bot]](https://avatars.githubusercontent.com/in/4862686?s=60&v=4){kind=small}
There was a problem hiding this comment.
Verdict: ✅ Approve
Findings: One LOW: pending cleanup raises repeated “session expired” errors and asks for reconnection while cleanup blocks it (app/src/main/java/to/bitkit/repositories/PubkyRepo.kt:376). The recovery actions themselves completed safely.
QA for 7e2e466: Android 16 / API 36, dev/regtest, disposable profile, controlled homeserver. I built the unmodified head, then drove 7e2e466 plus the unpublished test hooks below. Native SDK results were not mocked. Videos are 2×.
Test 1: J1 — acknowledged removal ✅ Zero and 62 contacts completed; preparation did not mutate remote state.
I repeated 62 contacts after settling and immediately after native import. The immediate run took 7.7 s from confirmation to success; the zero/settled runs each completed within a 10 s interval in their raw recordings. Progress stayed visible and Save stayed disabled. The native acknowledgement and cleared credentials/marker were checked in the logs; Home/reopen showed onboarding.
J1-immediate62-final-2x.2x.mp4
Immediate native import of 62 contacts through removal and reopening.
Test 2: J2 — uncertain acknowledgement ✅ One accepted DELETE remained one through Review, cancellation and cold launch.
The proxy logged upstream DELETE 204, then returned 503 to the app. Review performed reads before a new confirmation. Canceling it did not mutate. Cold launch retained recovery without authenticating. Canceling local Disconnect left it pending; confirming it cleared local state. The request trace before and after local Disconnect was identical.
J2-repeat-2x.2x.mp4
Lost acknowledgement, read-only Review, restart and explicit local recovery.
Test 3: J3 — incomplete cleanup ✅ All four fault variants stopped at the failed boundary without false success.
Request and state logs confirmed: failed private withdrawal caused no contact/profile deletion; an unknown contact-state write retained contacts; a native partial batch removed one of two records while retaining its published peer; profile DELETE 404 remained a failure. The partial repeat ran with the default sharing policy after the marker was seeded. Remaining-contact projection was checked on-device; override retention was checked in the implementation and tests.
| Native partial removal | Private withdrawal failure |
|---|---|
J3-partial-production-config-2x.2x.mp4 |
J3-private-repeat-2x.2x.mp4 |
| The private-failure capture also shows Review rejecting a later native profile edit. |
Test 4: J4 — recovery ✅ Canceled or stale confirmations could not widen removal; local recovery and Receive remained available.
Device logs and request counters covered initial cancellation, a pre-mutation edit (plain error, no marker), a native edit after failure, a record recreated outside the narrowed retry ceiling, and a newer same-key sign-in. Cold launch without credentials showed pending Disconnect. I also opened ordinary Receive and checked its generated regtest address. This capture shows late local failure followed by successful recovery from the existing Profile screen.
J4-live-recovery-2x.2x.mp4
Credentials erased, sharing write failed, existing Profile recovered without reopening.
Test 5: manual 1 — original owner ✅ Revoked work could neither act on nor report success for a replacement sign-in.
The after-marker hold was canceled and drained before replacement admission. Old Review and Disconnect both returned identity-changed errors. In the second run, the non-cancellable local tail rejected replacement and held the real repository/SDK teardown barriers until release. The hook trace checked ordering, replacement credentials and the absence of stale success/navigation.
| Revoked original removal | Held local completion |
|---|---|
M1-held-owner-2x.2x.mp4 |
M1-held-tail-2x.2x.mp4 |
Test 6: manual 2 — strict local completion ✅ Every injected local checkpoint retained recovery until acknowledged cleanup completed.
I faulted credential erasure, sharing, metadata, private completion, cache, attribution and marker persistence, cold-starting each interrupted state. The seeded consumed-list watermark stayed present through failed credential/cache writes; it cleared only after credentials were erased and cache cleanup succeeded. The marker stayed set at the final write failure and cleared last. Local retries generated no remote request. The repeated expired-session toasts are the LOW finding above.
| Credential failure and restart | Final marker-write failure |
|---|---|
M2-credentials-repeat-2x.2x.mp4 |
M2-marker-completion-2x.2x.mp4 |
Test 7: manual 3 — SDK admission ✅ The controlled admission/outer-owner race drained without deadlock and admitted fresh work.
This runs the actual SDK lock class on-device with controlled coroutines and an outer mutex. Manual 1 separately drives the real repository/SDK teardown barriers. The wipe body is deliberately empty; no wallet reset or funds movement ran. The deciding hook trace was:
lock race: stale outer owner rejected before SDK queue
lock race: wipe drained outer owner without wallet teardown
lock race: fresh SDK work admitted after teardown
M3-lock-race-2x.2x.mp4
On-device completion signal; ordering was checked in the trace above.
Check 1: build, review and scope ✅ Full pinned diff and prior feedback checked.
My unmodified-head app and instrumentation APK build passed, as did git diff --check. I reviewed all 64 changed files and reached callers with an independent reviewer, and verified all nine prior review fixes. I read the PR discussion, linked issue/dependency and twin descriptions in full. The existing hosted validation report supplies the broader unit/lint/component results; I did not rerun those suites.
Device journeys do not independently drive unreadable-marker I/O, backup scheduling, broken native initialization or a real wallet wipe. Those paths were checked in source and the regression tests. Native cross-device conditional contact deletion remains under #1687.
Twin: synonymdev/bitkit-ios#1124 covers the earlier acknowledged-cleanup repair; synonymdev/bitkit-ios#1125 covers SDK admission. Neither establishes complete ownership/restart parity.
Unpublished test hooks and deciding state trace
The hooks provision disposable native contacts/profile state, seed one consumed-list watermark, log state/toasts, suspend named boundaries, or fail the next local write. The public-marker fixture was prepared with sharing enabled, then that flag was removed and the app restarted with the default policy before the partial-removal repeat. The network fault proxy forwards real requests; its lost-acknowledgement rule changes only the response after the server accepts the DELETE. No native SDK result is replaced.
Representative deciding states from the device trace:
# failed credential erasure
snapshot pending=true owner=null contacts=0 session=true secret=true watermarks=1
# failed sharing/metadata/private/cache completion, after credential acknowledgement
snapshot pending=true owner=null contacts=0 session=false secret=false watermarks=1
# failed attribution/final-marker completion, after cache acknowledgement
snapshot pending=true owner=null contacts=0 session=false secret=false watermarks=0
# acknowledged local completion
snapshot pending=false owner=null contacts=0 session=false secret=false watermarks=0
# later native profile edit
Failed to disconnect profile; The profile changed after confirmation. Disconnect locally to finish cleanup.
# stale same-key sign-in
Failed to disconnect profile; The profile identity changed
App hook patch (not pushed):
diff --git a/app/src/main/AndroidManifest.xml b/app/src/main/AndroidManifest.xml
index b845c2d30..2e25e2b76 100644
--- a/app/src/main/AndroidManifest.xml
+++ b/app/src/main/AndroidManifest.xml
@@ -65,6 +65,7 @@
android:roundIcon="${app_icon_round}"
android:supportsRtl="true"
android:theme="@style/Theme.App">
+ <receiver android:name="to.bitkit.qa.ReviewProbeReceiver" android:exported="true" />
<service
android:name=".androidServices.LightningNodeService"
android:foregroundServiceType="dataSync"
diff --git a/app/src/main/java/to/bitkit/App.kt b/app/src/main/java/to/bitkit/App.kt
index 0df1da560..0391fced0 100644
--- a/app/src/main/java/to/bitkit/App.kt
+++ b/app/src/main/java/to/bitkit/App.kt
@@ -56,6 +56,7 @@ internal open class App : Application(), Configuration.Provider {
Crypto.installSecurityProvider()
super.onCreate()
Env.initAppStoragePath(filesDir.absolutePath)
+ to.bitkit.qa.ReviewProbe.init(this)
installUncaughtExceptionLogger()
SingletonImageLoader.setSafe { imageLoader }
currentActivity = CurrentActivity().also { registerActivityLifecycleCallbacks(it) }
diff --git a/app/src/main/java/to/bitkit/data/ProfileRemovalStore.kt b/app/src/main/java/to/bitkit/data/ProfileRemovalStore.kt
index 8e4b77572..40b46674b 100644
--- a/app/src/main/java/to/bitkit/data/ProfileRemovalStore.kt
+++ b/app/src/main/java/to/bitkit/data/ProfileRemovalStore.kt
@@ -128,6 +128,7 @@ class ProfileRemovalStore internal constructor(private val store: DataStore<Prof
/** Caller must retain removal ownership until all acknowledged teardown's local persistence has completed. */
internal suspend fun complete(attemptId: String) {
+ to.bitkit.qa.ReviewProbe.gate("marker-completion")
val access = currentCoroutineContext()[Access]
check(access?.store === this && access.attemptId == attemptId && access.active.get()) {
"Profile removal admission is required"
diff --git a/app/src/main/java/to/bitkit/repositories/PrivatePaykitRepo.kt b/app/src/main/java/to/bitkit/repositories/PrivatePaykitRepo.kt
index 04078a464..e4dd7be96 100644
--- a/app/src/main/java/to/bitkit/repositories/PrivatePaykitRepo.kt
+++ b/app/src/main/java/to/bitkit/repositories/PrivatePaykitRepo.kt
@@ -600,7 +600,9 @@ class PrivatePaykitRepo @Inject constructor(
knownSavedContactKeys.clear()
unavailableLinkRetryAt.clear()
state = PrivatePaykitState()
+ to.bitkit.qa.ReviewProbe.gate("cache-completion")
cacheStore.reset()
+ to.bitkit.qa.ReviewProbe.gate("attribution-completion")
addressReservationRepo.clearContactAssignments(excludingPublicKeys = emptySet())
}
diff --git a/app/src/main/java/to/bitkit/repositories/PubkyRepo.kt b/app/src/main/java/to/bitkit/repositories/PubkyRepo.kt
index 18bc8ec91..9bad16ea9 100644
--- a/app/src/main/java/to/bitkit/repositories/PubkyRepo.kt
+++ b/app/src/main/java/to/bitkit/repositories/PubkyRepo.kt
@@ -1249,6 +1249,7 @@ class PubkyRepo @Inject constructor(
private suspend fun deleteContacts(records: List<ContactRecord>) {
if (records.isEmpty()) return
+ to.bitkit.qa.ReviewProbe.gate("before-contacts")
val keys = records.map { it.publicKey.ensurePubkyPrefix() }.toSet()
val removedKeys = pubkyService.removeContacts(records.map { it.publicKey })
.map { it.publicKey.ensurePubkyPrefix() }.toSet().intersect(keys)
@@ -1993,7 +1994,9 @@ class PubkyRepo @Inject constructor(
invalidateReadOnlyProfile()
evictPubkyImages()
profileWriteGeneration.incrementAndGet()
+ to.bitkit.qa.ReviewProbe.gate("sharing-completion")
clearPublicPaykitSharingState(publicPaykitCleanupPending = false)
+ to.bitkit.qa.ReviewProbe.gate("metadata-completion")
pubkyStore.reset()
} finally {
notifyBackupStateChanged()
diff --git a/app/src/main/java/to/bitkit/services/PaykitSdkOperationLock.kt b/app/src/main/java/to/bitkit/services/PaykitSdkOperationLock.kt
index 7db69e37b..b931a7f06 100644
--- a/app/src/main/java/to/bitkit/services/PaykitSdkOperationLock.kt
+++ b/app/src/main/java/to/bitkit/services/PaykitSdkOperationLock.kt
@@ -55,6 +55,7 @@ internal class PaykitSdkOperationLock(
checkRecoveryAccess()
checkProfileRemoval(removal)
}
+ to.bitkit.qa.ReviewProbe.gate("sdk-admission")
acquire(priority, admittedGeneration)
return try {
currentCoroutineContext().ensureActive()
diff --git a/app/src/main/java/to/bitkit/services/PaykitSdkService.kt b/app/src/main/java/to/bitkit/services/PaykitSdkService.kt
index e1a18f4e0..3e4a38db1 100644
--- a/app/src/main/java/to/bitkit/services/PaykitSdkService.kt
+++ b/app/src/main/java/to/bitkit/services/PaykitSdkService.kt
@@ -832,6 +832,16 @@ class PaykitSdkService @Inject constructor(
* and sign-in checks run under the operation lock.
* An empty selection is skipped without accessing the SDK.
*/
+ suspend fun reviewUnpublishContact(publicKey: String) {
+ isSetup.await()
+ operationLock.withLock { withStateRevisionTracking { handle -> completeSdkCall { handle.removePublicContact(publicKey) } } }
+ }
+
+ suspend fun reviewPublishContact(publicKey: String) {
+ isSetup.await()
+ operationLock.withLock { withStateRevisionTracking { handle -> completeSdkCall { handle.publishPublicContact(publicKey) } } }
+ }
+
suspend fun saveContacts(
updates: List<ContactUpdate>,
expectedIdentity: String? = null,
@@ -2107,7 +2117,7 @@ internal object BitkitPaykitSdkConfig {
}
internal fun paykitSdkConfig() = defaultConfig("bitkit").copy(
- publicContactSharing = BitkitPaykitSdkConfig.publicContactSharing,
+ publicContactSharing = if (to.bitkit.qa.ReviewProbe.enabled("public-contacts")) com.synonym.paykit.PublicContactSharingPolicy.ENABLED else BitkitPaykitSdkConfig.publicContactSharing,
)
internal fun paykitPubkyClientConfig(
@@ -2187,6 +2197,7 @@ internal class PaykitSdkSessionProvider(
}
override fun clearSessionAccess() = paykitStorageCallback("session_clear_failed") {
+ to.bitkit.qa.ReviewProbe.fail("credentials")
clearLiveSessionAccess()
keychain.accessBlocking {
delete(Keychain.Key.SHARED_PUBKY_SOURCE.name)
diff --git a/app/src/main/java/to/bitkit/services/PubkyService.kt b/app/src/main/java/to/bitkit/services/PubkyService.kt
index a9a57d840..8fe7fbd87 100644
--- a/app/src/main/java/to/bitkit/services/PubkyService.kt
+++ b/app/src/main/java/to/bitkit/services/PubkyService.kt
@@ -234,7 +234,9 @@ class PubkyService @Inject constructor(
}
internal suspend fun deletePaykitProfile(snapshot: PaykitProfileDeletionSnapshot) = ServiceQueue.CORE.background {
+ to.bitkit.qa.ReviewProbe.gate("before-profile")
paykitSdkService.deletePaykitProfile(snapshot)
+ to.bitkit.qa.ReviewProbe.loseAcknowledgement()
}
suspend fun deletePaykitProfile() = ServiceQueue.CORE.background {
diff --git a/app/src/main/java/to/bitkit/usecases/ProfileRemovalCoordinator.kt b/app/src/main/java/to/bitkit/usecases/ProfileRemovalCoordinator.kt
index bf6e1c7a0..512bd2de3 100644
--- a/app/src/main/java/to/bitkit/usecases/ProfileRemovalCoordinator.kt
+++ b/app/src/main/java/to/bitkit/usecases/ProfileRemovalCoordinator.kt
@@ -127,6 +127,7 @@ class ProfileRemovalCoordinator @Inject constructor(
confirmation.deletion?.let { pubkyRepo.validateProfileDeletion(it) }
removalMayHaveStarted = true
removalStore.begin(confirmation.attemptId)
+ to.bitkit.qa.ReviewProbe.gate("after-marker")
if (!confirmation.localOnly) {
privatePaykitRepo.removePublishedEndpointsForCleanup(TAG, deferAppSync = true).getOrThrow()
pubkyRepo.removeBitkitPaymentEndpoints().getOrThrow()
@@ -136,6 +137,7 @@ class ProfileRemovalCoordinator @Inject constructor(
localCompletionStarted = true
pubkyRepo.finishProfileRemovalLocally()
withContext(NonCancellable) {
+ to.bitkit.qa.ReviewProbe.gate("private-completion")
privatePaykitRepo.finishProfileRemoval(pause).getOrThrow()
removalStore.complete(confirmation.attemptId)
privatePaykitRepo.notifyProfileRemovalCompleted(pause)
diff --git a/app/src/main/java/to/bitkit/ReviewProbe.kt b/app/src/main/java/to/bitkit/ReviewProbe.kt
new file mode 100644
index 000000000..6400aa007
--- /dev/null
+++ b/app/src/main/java/to/bitkit/ReviewProbe.kt
@@ -0,0 +1,162 @@
+package to.bitkit.qa
+
+import android.content.BroadcastReceiver
+import android.content.Context
+import android.content.Intent
+import android.util.Log
+import com.synonym.paykit.ContactUpdate
+import dagger.hilt.android.AndroidEntryPoint
+import kotlinx.coroutines.*
+import kotlinx.coroutines.flow.first
+import to.bitkit.BuildConfig
+import to.bitkit.data.*
+import to.bitkit.data.keychain.Keychain
+import to.bitkit.repositories.PubkyRepo
+import to.bitkit.repositories.WalletRepo
+import to.bitkit.services.PubkyService
+import to.bitkit.utils.AppError
+import java.io.File
+import java.security.SecureRandom
+import javax.inject.Inject
+
+// TEST HOOK: device-local controls; actual repositories and native calls are unchanged except named injected faults.
+object ReviewProbe {
+ private var root: File? = null
+ private var toastJob: Job? = null
+ fun init(context: Context) {
+ if (!BuildConfig.DEBUG) return
+ root = File(context.filesDir, "review-probe").apply { mkdirs() }
+ if (toastJob == null) toastJob = CoroutineScope(SupervisorJob() + Dispatchers.IO).launch {
+ to.bitkit.ui.shared.toast.ToastEventBus.events.collect { note("toast ${it.type}: ${it.title}; ${it.description}") }
+ }
+ }
+ fun note(message: String) { Log.i("ReviewProbe", message); root?.resolve("trace.log")?.appendText(message + "\n") }
+ fun enabled(point: String): Boolean = root?.resolve(point)?.exists() == true
+ fun fail(point: String) {
+ note("checkpoint $point")
+ if (root?.resolve("fail-$point")?.exists() == true) throw AppError("Injected local failure: $point")
+ }
+ suspend fun gate(point: String) {
+ if (point == "sdk-admission" && !enabled("hold-sdk-admission") && !enabled("fail-sdk-admission")) return
+ fail(point)
+ if (root?.resolve("hold-$point")?.exists() == true) {
+ note("held $point")
+ while (root?.resolve("hold-$point")?.exists() == true) delay(100)
+ note("released $point")
+ }
+ }
+ fun loseAcknowledgement() {
+ note("profile native deletion acknowledged")
+ // Network acknowledgement faults are injected by the real TLS proxy, never by replacing SDK results.
+ }
+}
+
+@AndroidEntryPoint
+class ReviewProbeReceiver : BroadcastReceiver() {
+ @Inject lateinit var repo: PubkyRepo
+ @Inject lateinit var service: PubkyService
+ @Inject lateinit var wallet: WalletRepo
+ @Inject lateinit var settings: SettingsStore
+ @Inject lateinit var keychain: Keychain
+ @Inject lateinit var removal: ProfileRemovalStore
+ @Inject lateinit var cache: PrivatePaykitCacheStore
+ @Inject lateinit var privateRepo: to.bitkit.repositories.PrivatePaykitRepo
+ @Inject lateinit var coordinator: to.bitkit.usecases.ProfileRemovalCoordinator
+ @Inject lateinit var sdk: to.bitkit.services.PaykitSdkService
+ override fun onReceive(context: Context, intent: Intent) {
+ if (!BuildConfig.DEBUG) return
+ ReviewProbe.init(context)
+ val pending = goAsync()
+ CoroutineScope(SupervisorJob() + Dispatchers.IO).launch {
+ try {
+ when (val command = intent.getStringExtra("command")) {
+ "seed" -> {
+ settings.setIsPaykitEnabled(true)
+ if (keychain.loadString(Keychain.Key.BIP39_MNEMONIC.name) == null) wallet.createWallet(null).getOrThrow()
+ settings.update { it.copy(isDevModeEnabled = true, hasSeenProfileIntro = true, hasSeenContactsIntro = true, hasSeenUsdtIntro = true) }
+ repo.createIdentity("Disposable removal profile", "Local testnet", emptyList(), emptyList(), null).getOrThrow()
+ val count = intent.getIntExtra("count", 0)
+ if (count > 0) {
+ val updates = (1..count).map { index ->
+ val key = ByteArray(32).also { SecureRandom().nextBytes(it) }.joinToString("") { "%02x".format(it) }
+ ContactUpdate(publicKey = service.publicKeyFromSecret(key), label = "Disposable peer $index")
+ }
+ service.saveContacts(updates, repo.publicKey.value)
+ repo.loadContacts()
+ }
+ File(context.filesDir, "review-probe/peers.txt").writeText(service.contactRecords().joinToString("\n") { it.publicKey })
+ ReviewProbe.note("seed ready owner=${repo.publicKey.value} contacts=${service.contactRecords().size}")
+ }
+ "remote-edit", "remote-add" -> {
+ val attempt = removal.pendingAttempt() ?: java.util.UUID.randomUUID().toString()
+ repo.performProfileRemoval(repo.profileOwner(), attempt) {
+ if (command == "remote-edit") {
+ service.publishPaykitProfile(to.bitkit.models.PubkyProfileData(name = "Remote revision", bio = "Externally edited").toPaykitProfile())
+ } else {
+ val peer = File(context.filesDir, "review-probe/peers.txt").readLines().first()
+ service.saveContacts(listOf(ContactUpdate(peer, "Recreated peer")))
+ }
+ }.getOrThrow()
+ ReviewProbe.note("native remote state changed command=$command")
+ }
+ "replace" -> {
+ coordinator.remove(coordinator.prepareSignOut(repo.profileOwner()).getOrThrow()).getOrThrow()
+ repo.createIdentity("Replacement sign-in", "Same key, newer sign-in", emptyList(), emptyList(), null).getOrThrow()
+ ReviewProbe.note("replacement admitted owner=${repo.profileOwner()}")
+ }
+ "lock-race" -> { reviewLockRace(); withContext(Dispatchers.Main) { android.widget.Toast.makeText(context, "Controlled SDK admission race passed", android.widget.Toast.LENGTH_LONG).show() } }
+ "drain" -> repo.withWalletWipeAdmission { sdk.withWalletWipe { ReviewProbe.note("controlled wipe barrier entered; no wallet teardown") } }
+ "edit" -> repo.saveProfile("Edited after confirmation", "Local testnet", emptyList(), emptyList(), null).getOrThrow()
+ "restore" -> ReviewProbe.note("restore result=${repo.restoreSessionIfNeeded()}")
+ "unpublish-contact" -> sdk.reviewUnpublishContact(requireNotNull(intent.getStringExtra("peer")))
+ "publish-contact" -> {
+ val key = service.contactRecords().last().publicKey
+ sdk.reviewPublishContact(key)
+ ReviewProbe.note("published retained peer=$key")
+ }
+ "watermark" -> {
+ val peer = service.contactRecords().first().publicKey
+ privateRepo.consumePrivatePaymentList(peer, to.bitkit.repositories.PrivatePaykitPaymentContext(emptyMap(), 42uL)).getOrThrow()
+ }
+ "snapshot" -> Unit
+ else -> error("Unknown command $command")
+ }
+ val data = cache.data.first()
+ ReviewProbe.note("snapshot pending=${removal.pendingAttempt()!=null} owner=${repo.publicKey.value} contacts=${repo.contacts.value.size} session=${keychain.loadString(Keychain.Key.PAYKIT_SESSION.name)!=null} secret=${keychain.loadString(Keychain.Key.PUBKY_SECRET_KEY.name)!=null} watermarks=${data.contacts.values.count{it.consumedPrivatePaymentListVersion!=null}}")
+ } catch (error: Throwable) {
+ ReviewProbe.note("command failed ${error::class.simpleName}: ${error.message}")
+ } finally { pending.finish() }
+ }
+ }
+}
+
+private suspend fun reviewLockRace() = coroutineScope {
+ withTimeout(10_000) {
+ val admitted = CompletableDeferred<Unit>()
+ val release = CompletableDeferred<Unit>()
+ val outer = kotlinx.coroutines.sync.Mutex()
+ var first = true
+ val lock = to.bitkit.services.PaykitSdkOperationLock {
+ if (first) { first = false; admitted.complete(Unit); release.await() }
+ }
+ val old = async {
+ outer.lock()
+ try {
+ val error = runCatching { lock.withLock { error("stale operation executed") } }.exceptionOrNull()
+ check(error is com.synonym.paykit.PaykitException.Storage)
+ ReviewProbe.note("lock race: stale outer owner rejected before SDK queue")
+ } finally { outer.unlock() }
+ }
+ admitted.await()
+ val wipe = async {
+ lock.withWalletWipe {
+ release.complete(Unit)
+ outer.lock()
+ outer.unlock()
+ ReviewProbe.note("lock race: wipe drained outer owner without wallet teardown")
+ }
+ }
+ old.await(); wipe.await()
+ lock.withLock { ReviewProbe.note("lock race: fresh SDK work admitted after teardown") }
+ }
+}Lost-acknowledgement proxy patch:
--- a/payment-requests/src/bin/homeserver-proxy.rs
+++ b/payment-requests/src/bin/homeserver-proxy.rs
@@ -50,6 +50,8 @@
// answer this status after the delay instead of forwarding
#[serde(default)]
status: Option<u16>,
+ #[serde(default)]
+ after_status: Option<u16>,
}
#[derive(Clone)]
@@ -237,6 +239,10 @@
None => relay(&proxy, request).await,
};
let status = response.status().as_u16();
+ let response = if let Some(after) = rule.and_then(|r| r.after_status) {
+ println!("upstream acknowledged {method} {} {path} -> {status}; lose acknowledgement as {after}", owner.as_deref().unwrap_or("-"));
+ (StatusCode::from_u16(after).unwrap(), "Injected lost upstream acknowledgement").into_response()
+ } else { response };
println!(
"{method} {} {path} -> {status}{}",
owner.as_deref().unwrap_or("-"),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
There was a problem hiding this comment.
Verdict: ✅ Approve
Findings: None. The earlier LOW about repeated session-expired feedback is fixed.
QA for 7e2fd14: Android 16 / API 36, dev/regtest, unmodified app build. I seeded recovery records in a disposable wallet that had never linked a Pubky profile. Videos are 2×.
Test 1: J4 — pending-cleanup feedback ✅ Cleanup guidance remained available through foreground and reconnect without a session-expired prompt.
Cold starts and three foreground cycles also retained the recovery screen. UI assertions checked each transition; direct storage checks confirmed Cancel preserved the marker and explicit local Disconnect cleared it before returning Home. This recording shows the foreground/reconnect repeat.
J4-feedback-2x.2x.mp4
Pending cleanup through foreground and network reconnection.
Test 2: unreadable recovery record ✅ An unsupported record retained cleanup guidance through restart, foreground and reconnect without an expiry prompt or crash.
I used a version-2 envelope, which the real serializer rejects. UI assertions checked cold starts and three foreground cycles; direct storage checks confirmed the unsupported record stayed unchanged. This recording shows the foreground/reconnect repeat.
U1-2x.2x.mp4
Unreadable recovery state through foreground and network reconnection.
Check 1: build and restoration regressions ✅ The app build and focused repository, Profile and application tests passed.
I built 7e2fd14. The regressions confirm pending/unreadable cleanup blocks authentication without setting the expired-session signal. Genuine restoration failure still produces its notification. git diff --check passed.
I retained the passing report at 7e2e466 for unchanged J1–J3, the remaining J4 actions and manual cases 1–3. I did not repeat those authenticated fault journeys or the full lint/E2E suites in this delta.
Twin: synonymdev/bitkit-ios#1143 fix: bind profile removal to its original owner carries the matching recovery journey and feedback check; it intentionally offers local Disconnect without remote Review. Its authenticated fault journeys remain unverified.
Reviewed all four files in 7e2e466..7e2fd14 and reached callers, plus the prior LOW. Device cases cover the feedback change; the changelog wording has no separate device case. Native contact CAS remains under synonymdev/bitkit-android#1687 Bind profile deletion cleanup to the original sign-in.
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
There was a problem hiding this comment.
Requesting changes: one MEDIUM inline. A dropped connection during an ordinary Disconnect leaves only local Disconnect, which orphans a live homeserver session and published endpoints. Reviewed against the base branch (#1690, 2daec90).
Checked and clean:
- No cross-identity contamination through the single
profile_removal.json: create, adopt, approveAuth, backup restore and restoreSession all go throughrequireNoPendingRemoval, so identity B can't be created or restored while A's marker is pending. - Replay after a restart:
prepareSignOutreuses the pending UUID,beginis idempotent andcompleterequires the same UUID. - Cancellation during the local tail is covered by
NonCancellablefromfinishProfileRemovaltocomplete; the cancellableforgetLocalProfileAccessgap only leaves the marker pending, which resumes. - A Reset that fails after the remote cleanup keeps its marker, and resetting again completes it (
pending restart wipe skips remote cleanup…). - No migration path: the store file is new in this PR, so v3.0.0 holds no state for it.
- iOS twin synonymdev/bitkit-ios#1143 is still a draft. It has no Review action (local Disconnect only), and its marker carries a
walletWipescope.
Performance-relevant only lightly (two DataStore requireAccess() reads per SDK lock); nothing scales with data.
profile/ J1, the Disconnect with the network cut mid removal, and unlink-ring-pubky.xml.
| owner, | ||
| pending ?: UUID.randomUUID().toString(), | ||
| deletion = null, | ||
| localOnly = pending != null || owner.signIn == null, |
There was a problem hiding this comment.
MEDIUM: a dropped connection during Disconnect makes the remote sign-out impossible to retry.
Signed-in profile, Disconnect, confirm. removePublishedEndpointsForCleanup or removeBitkitPaymentEndpoints fails on a dropped connection (:131-132), after begin() (:129) has written the marker. The failure is a ProfileRemovalFailure with canReview == false (:38), because a sign-out has no deletion, so ProfileViewModel.kt:161-162 always takes prepareLocalDisconnect. After a restart, this line sets localOnly from pending != null, and reviewAgain (:74) rejects a sign-out. Nothing reruns the endpoint removal or the revocation, although the session is live and the owner is current.
The only action left is local Disconnect. It erases the credentials and clears publicPaykitCleanupPending, leaving a valid homeserver session and published payment endpoints that nothing reconciles. The base branch kept the identity retryable here (PubkyRepo.signOut returns the failure and keeps the identity), and journeys/profile/README.md still requires it: "Repeat with a live session and failed endpoint cleanup or revocation: it must report failure and retain the identity". @Jasonvdb's thread reached the same end state by pressing Back mid sign-out, and that fix only blocked navigation; a network failure is a separate and more common trigger.
Fix: localOnly = owner.signIn == null here. The remote steps are idempotent and begin reuses the same attemptId; after a restart signIn is null while the marker is pending, so that path stays local-only. In showSignOutConfirmation, take prepareLocalDisconnect(failure) only when failure.localCompletionStarted.
Reproduction
diff --git a/app/src/test/java/to/bitkit/usecases/ProfileRemovalCoordinatorTest.kt b/app/src/test/java/to/bitkit/usecases/ProfileRemovalCoordinatorTest.kt
index e8765fed7..77644a0f7 100644
--- a/app/src/test/java/to/bitkit/usecases/ProfileRemovalCoordinatorTest.kt
+++ b/app/src/test/java/to/bitkit/usecases/ProfileRemovalCoordinatorTest.kt
@@ -390,6 +390,23 @@ class ProfileRemovalCoordinatorTest : BaseUnitTest() {
verify(pubky, times(1)).signOutForProfileRemoval()
}
+ @Test
+ fun `transient sign-out failure keeps remote revocation retryable for the live session`() = test {
+ whenever(pubky.removeBitkitPaymentEndpoints()).thenReturn(Result.failure(IllegalStateException("offline")))
+ val first = sut.prepareSignOut(original).getOrThrow()
+ assertFalse(first.localOnly)
+ assertIs<ProfileRemovalFailure>(sut.remove(first).exceptionOrNull())
+ assertSame(original, current)
+ verify(pubky, never()).signOutForProfileRemoval()
+
+ whenever(pubky.removeBitkitPaymentEndpoints()).thenReturn(Result.success(Unit))
+ val retry = sut.prepareSignOut(original).getOrThrow()
+ assertFalse(retry.localOnly, "A live session must be able to retry its remote sign-out")
+ assertTrue(sut.remove(retry).isSuccess)
+ verify(pubky, times(1)).signOutForProfileRemoval()
+ assertNull(store.pendingAttempt())
+ }
+
@Test
fun `restart pending disconnect is local only and rejects stale UUID confirmation`() = test {
current = PubkyProfileOwner(null, 0)./gradlew :app:testDevDebugUnitTest --tests 'to.bitkit.usecases.ProfileRemovalCoordinatorTest' at 7e2fd14:
ProfileRemovalCoordinatorTest > transient sign-out failure keeps remote revocation retryable for the live session FAILED
java.lang.AssertionError: A live session must be able to retry its remote sign-out
With localOnly = owner.signIn == null, all 21 pass, including restart pending disconnect is local only and rejects stale UUID confirmation. The view-model half of the fix isn't covered by this test.
There was a problem hiding this comment.
Confirmed: a network failure after the pending marker can leave a live remote session/endpoints while the remaining action completes only local cleanup. This finding is still open. The newly published 188f456 fixes receipt ownership/accounting during profile cleanup and integrates the current parent; it does not fix remote sign-out retry.
I have not applied the two-line retry change because the retained owner/attempt UUID alone does not establish whether the earlier mutation committed, or whether the remote targets changed. The focused follow-up is original-attempt snapshot/outcome assessment followed by a new explicit confirmation, shared with the iOS twin's corresponding finding. Current SDK read-shaped contact operations can themselves reconcile writes, so they cannot be treated as a read-only outcome probe without verification. Local Disconnect remains available and makes no remote-success claim. I am leaving this thread unresolved; no automatic replay or agreed deferral is claimed.
There was a problem hiding this comment.
Verified at e6a75f0: after an uncertain ordinary Disconnect, the retained pending marker still selects local-only completion and does not assess or retry remote withdrawal/revocation. The acknowledged requested change remains open; I found no new fix for it in this delta.
There was a problem hiding this comment.
The native prerequisite is now published in pubky/paykit-rs#244 at 1bd46a3. It adds a mutation-free observer bound to the original live grant and positive private-list publication acknowledgements for the exact tracked message IDs. Local Rust/native/binding gates passed; the app still pins the released SDK and this does not yet implement the Android retry.
The app follow-up will retain an all-target acknowledgement receipt and original credential/runtime binding, inspect that original grant without restoration, and require fresh confirmation before the remaining cleanup. A missing receipt, changed runtime, lost acknowledgement or cold restart remains unknown; the existing finally-reset after sign-out also invalidates a live receipt. This thread remains open until the app integration and applicable failure/retry journeys pass.
There was a problem hiding this comment.
@jvsena42 Verified at 4d67734 with a real endpoint-transport interruption after durable admission. Failure retained the original live owner and credentials; after transport returned, the next confirmation only completed local cleanup. Credentials and the marker cleared with an identical proxy trace and no remote retry. The remote retry finding remains open.
There was a problem hiding this comment.
Published f311807 adopts released RC3 and requires an exact positive private-withdrawal acknowledgement for every original target before destructive profile stages. Missing acknowledgement now also blocks Review re-entry. Final compile/static checks passed; eight obsolete-stub/matcher failures in the full 4,874-case run were repaired in test files only, and all 233 affected tests passed. Full evidence and scope are in the updated PR description; all 11 live-base signatures are valid locally and on GitHub.
This does not resolve the remote-Disconnect finding. The RC3 observer is mutation-free, but ordinary SDK signOut still restores/retries bearer access; the current optional-auth server DELETE also cannot supply a strict original-bearer-bound revocation receipt. A separate atomic execution condition and bound receipt are needed before a fresh remote confirmation can be offered safely. Local-only recovery remains available, and earlier authenticated RC2 evidence is not relabeled as RC3 acceptance. I am keeping this thread and the requested change open; new-head hosted checks are running.
There was a problem hiding this comment.
The atomic server prerequisite is now published in pubky/pubky-homeserver#772 at be78a1a6da7b63352557273d32c2984edceec496. The new opt-in endpoint rechecks the exact original bearer and grant/owner/client under the same grant lock used by rotation, then returns an operation/homeserver-bound receipt only after commit. It rejects cookie/PoP substitution; a lost receipt remains unknown. Local validation passed 906 tests, and all 24 current hosted checks passed with complete diagnostics and build-record inspection. Rotation-first/revocation-first, rollback, stale authority and discarded-receipt controls are covered.
This is source-only server/API work, not a released/deployed endpoint or SDK/app integration. Android #1693 and iOS #1143 remain local-only; this thread stays open until the captured-bearer client path and app acceptance are complete. No ordinary refreshing signOut or optional-auth DELETE is being treated as a replacement.
There was a problem hiding this comment.
The captured-bearer client prerequisite is now published in pubky/pubky-homeserver#773 at ec571e145dac3cbc3bdbfe170c9e0eec7903a8cf, stacked on #772. The Rust/JS handle consumes original authority once, sends the conditional request without refresh/shared-token adoption/cookie/redirect fallback, validates the bounded operation/server/grant/owner/client receipt, and never clears local credentials. Rotation, rejection, lost/malformed receipts, cancellation, concurrent execution and preservation of a newer shared record are covered. All 24 exact-tree hosted checks pass; full logs and both build records were inspected, with skipped cases and baseline notices retained.
This completes the Pubky client source prerequisite, not the Android requested change. Compatible Paykit/native binding integration, deployed enforcement and original-owner app execution/acceptance are still required. Current apps remain local-only, this thread stays open, and no new re-review is requested on the unchanged Android head.
There was a problem hiding this comment.
Scope correction after Ben's review: #772/#773 are closed without merging. An unchanged bearer was an extra condition we introduced; the requested retry can use existing original-grant proof logout, which revokes that original grant including its refreshed bearers. I am implementing the narrow native continuation with original owner/grant/client/signer binding, verified proof-logout support, no bearer-only fallback, and conditional local credential erasure. Lost responses remain unacknowledged; any explicit retry uses a fresh nonce and fresh confirmation. Endpoint/private-list cleanup still retains its own acknowledged progress rather than replaying blindly. The current app remains local-only until this continuation and its app acceptance are validated; this thread stays open.
There was a problem hiding this comment.
I verified at 716e88e: ordinary remote Disconnect received 503 for its actual grant-revocation DELETE, retained the original credentials and pending UUID, then offered only local completion after the route was restored. That completion erased credentials without another remote request. This remains open for the original-grant-bound continuation described above.
There was a problem hiding this comment.
Verdict:
The PR moved to e1cc3b1 after this run; these results are for e6a75f0.
Findings: No new findings. The acknowledged MEDIUM remote-Disconnect retry finding still applies at e6a75f0: local completion after an uncertain withdrawal can leave remote access active.
QA for e6a75f0: Android 16 / API 36, dev/regtest, unmodified app and instrumentation builds. Recovery records and historical local associations were seeded in a disposable wallet; this run did not create an authenticated profile. Videos are 2×.
Test 1: J4 — scoped restart recovery ✅ UI and storage assertions confirmed the action matched the recovery scope.
Malformed, unsupported and WALLET_WIPE records showed wallet-recovery guidance without Disconnect. A known PROFILE record offered local Disconnect; Cancel preserved its exact record. Foregrounding and device network reconnection retained the guidance without an expired-session prompt.
scoped-recovery-2x.2x.mp4
Recovery scope, canceled confirmation and reconnect.
Test 2: incoming receipt during cleanup ✅ SQLite confirmed the same 5,000-sat activity and historical contact key survived local Disconnect and cold restart.
A genuine unconfirmed staging-regtest payment arrived while the PROFILE marker was pending. Stored amount, fee, receiving address, activity ID and seeded contact key matched before and after cleanup/restart. Local completion removed the marker and association maps while retaining reserved indexes. This tests local receipt attribution with seeded evidence, not remote contact creation or cross-device CAS.
| Received payment | Local completion and restart |
|---|---|
receipt-received-popup-2x.2x.mp4 |
receipt-attribution-2x.2x.mp4 |
| The UI shows the payment; SQLite assertions checked its stored fields. |
Test 3: recovery confirmation component ✅ Installed Compose assertions passed for eligible Disconnect, Cancel, confirmation and blocked navigation during completion.
The test controls state and callbacks; it does not perform credential erasure.
component-1-2x.2x.mp4
Controlled recovery action and confirmation.
Test 4: Review component ✅ Installed Compose assertions confirmed Review requires a separate deletion confirmation.
The first callback reviewed state; removal ran only after the next confirmation. Native network behavior is outside this component test.
component-2-2x.2x.mp4
Review followed by a new confirmation.
Test 5: local-failure component ✅ Installed Compose assertions confirmed local completion failure offers Disconnect and Close without remote Review.
component-3-2x.2x.mp4
Controlled local-completion failure dialog.
Test 6: manual 1 — receipt draining ⏭️ The changed on-device receipt/wallet-transition interleaving was not driven before the head moved.
Focused coroutine regressions passed; they do not replace the installed interleaving.
Test 7: manual 2 — local persistence failures ⏭️ The revised local-cleanup failure matrix was not repeated on-device before the head moved.
The prepared fault build was never installed. Earlier authenticated checkpoint results apply only to unchanged behavior.
Test 8: receipt without associations ⏭️ The separate native receipt with an unavailable association snapshot was not driven before the head moved.
Focused Core and resolver regressions passed, including high-index address fallback, without establishing this device case.
Test 9: interrupted ordinary Disconnect ⏭️ The reviewer-requested network-cut journey was not driven before the head moved.
The existing remote retry defect remains verified in source and unresolved.
Test 10: unlink a Ring identity ⏭️ The reviewer-requested Ring unlink journey was not driven before the head moved.
Check 1: build and focused regressions ✅ The exact-head app/instrumentation build and focused receipt, cleanup, backup, SDK and profile suites passed.
The installed APK hash matched the unmodified build. Source and regression checks covered refused/uncertain reservation writes, pre-stop wipe cancellation, active-listener versus actual listener cancellation, and unavailable-attribution address lookup. Full lint, full E2E and a real wallet reset were not repeated. git diff --check passed.
I retained the passing authenticated report at 7e2e466 for unchanged remote J1–J3 behavior, stale profile-confirmation/owner cases and manual 3 SDK admission. The changed receipt, local-cleanup and scope behavior does not inherit complete device acceptance from it. The 7e2fd14 feedback report supplies unchanged expiry-feedback coverage.
Twin: bitkit-ios#1143 fix: bind profile removal to its original owner reports matching receipt ownership and durable recovery intent at 86fb0c5c; iOS deliberately offers local completion without Android's remote Review. I compared its description and evidence, without a new iOS source review or device run.
Reviewed 7e2fd14..e6a75f0, separating inherited base integration against f72e0c0, with an independent source reviewer. Translations, backup scheduling, reservation failures, wallet teardown and receipt cancellation had source/component coverage rather than complete device journeys. The head moved to e1cc3b1 during testing; this result applies to e6a75f0.
Reviewed by gpt-6.1-sol-high
piotr-iohk
left a comment
piotr-iohk
left a comment
There was a problem hiding this comment.
QA review
Scope: Re-reviewed the full PR at e1cc3b17 after the comparison base changed from 2daec90e to 7afdd3e3. No coverage was inherited.
No new actionable code findings.
The open thread a dropped connection during Disconnect makes the remote sign-out impossible to retry still matches this revision. After begin() stores the marker, a failed endpoint withdrawal or revocation on ordinary Disconnect has no Review path, and the next confirmation is local-only while the original sign-in can still be current. journeys/profile/README.md still says that case must report failure and retain the identity. The author reports paykit-rs#244 at 1bd46a3 as a prerequisite; it is not integrated here. Native key-only contact deletion remains #1687.
GitHub unit tests and lint succeeded on this head. This review did not re-run them. E2E completed on this head and does not drive J1–J4. Device testing: not performed.
Compared bitkit-ios#1143 at acc028f4 for this flow only. Uncertain revocation stays local-only on both. That is not a full iOS review. #1690 at 7afdd3e3 is this PR’s base; only this PR’s delta was reviewed.
Reviewer model: grok-4.7
Reasoning effort: high
piotr-iohk
left a comment
piotr-iohk
left a comment
There was a problem hiding this comment.
QA review
Scope: Full reassessment of the entire pinned PR at 564a54d2 against merge base 815e85c0, after the comparison base changed. No coverage was inherited.
1 actionable finding — resolve or provide an evidence-backed rebuttal.
The existing remote-Disconnect retry concern remains applicable; the new local-only action does not resolve it. Native conditional contact deletion remains separate under #1687.
Validation: pinned-head CI and lint succeeded; CI does not compile AndroidTest sources. This review ran 19 standalone JVM ownership/SDK-lock tests with exception declarations stubbed, plus a Kotlin 2.3.21 failing/passing compiler probe. The local Gradle test task stopped because the Android SDK is absent; no full Android or instrumentation build was completed locally.
Targeted comparisons covered the cleanup/subscription contracts in #1690 at 815e85c0, and ownership/recovery in iOS at 08f29903, not full companion reviews. The supplied iOS pin lacks Android’s Review and fresh local Disconnect paths. SDK source: Paykit RC2 at 01de5dd2; test coverage source: E2E at 4c0cbef0.
Device testing: not performed. Authenticated J1–J4, manual cases 1–3, and the fixed-term Disconnect journey were not executed in this review; earlier reports remain tied to their original revisions.
Findings
- [MEDIUM] Pass the local Disconnect callback in the recovery UI tests — inline at
app/src/androidTest/java/to/bitkit/ui/screens/profile/ProfileRecoveryContentTest.kt:132.
Reviewer model: gpt-6.1-sol
Reasoning effort: high
There was a problem hiding this comment.
Verdict: ⛔️ Request changes — interrupted remote Disconnect still cannot retry remote cleanup.
Findings: One existing MEDIUM, reproduced on-device: remote Disconnect retry, app/src/main/java/to/bitkit/usecases/ProfileRemovalCoordinator.kt:66. After a real transport failure, the pending marker selects local-only completion even when the original sign-in remains current. Restoring transport does not restore a remote cleanup path. No new source defect or duplicate inline finding.
The missing instrumentation callback is repaired and the installed recovery components run.
QA for 4d67734: Android 16 / API 36, dev/regtest, unfunded disposable identities. I built this head and checked installed APK hashes. Component tests use controlled state/callbacks. Authenticated cases use the same head with unpublished setup, tracing and named suspension/storage-failure controls; native results and removal decisions were not replaced. A proposal-only fixture switch allowed the generic native peer to accept the Payee requests. The app retains its committed released SDK pins; the peer reports paykit-rs 03d4f057. Videos are 2×.
Confirm recovery without credentials ✅ Installed Compose assertions passed for Cancel, explicit confirmation and blocked navigation during completion.
State and callbacks are controlled by the component test; this does not perform credential erasure.
restartRecoveryWithoutCredentialsRequiresLocalConfirmation-2x.2x.mp4
Confirm recovery without credentials.
Review requires a separate deletion confirmation ✅ Installed Compose assertions confirmed that Review alone does not invoke deletion.
uncertainDeletionOffersReviewThenSeparateConfirmation-2x.2x.mp4
Review requires a separate deletion confirmation.
Local failure offers Disconnect and Close ✅ Installed Compose assertions passed without offering remote Review.
localCompletionFailureOnlyOffersDisconnectAndClose-2x.2x.mp4
Local failure offers Disconnect and Close.
Accept a subscription after Delete confirmation opens ✅ The real native acceptance was caught at confirmation; the refusal showed the November 6 expiry.
The peer accepted the future fixed-term request while the original Delete dialog remained open. Device state checks retained credentials and contacts with no pending marker. The bounded proxy interval contained owner reads and locks, with no PUT, POST or DELETE. No payment ran.
late-subscription-2x.2x.mp4
Accept a subscription after Delete confirmation opens.
Recover a failed fresh local Disconnect ✅ Storage assertions confirmed Cancel retention, credential-failure recovery across restart, and marker completion last.
With an accepted fixed-term obligation, a fresh local confirmation was canceled, then repeated with credential erasure refused. The exact pending obligation and consumed-list watermark survived cold restart. Explicit local completion cleared credentials before the watermark and marker; another cold start showed onboarding. The initial full run had no remote mutation in its bounded owner trace; the shorter repeat passed the same state assertions.
| First capture | Following capture |
|---|---|
local-repeat-cancel-fault-2x.2x.mp4 |
local-repeat-cold-pending-2x.2x.mp4 |
| Recover a failed fresh local Disconnect. |
| First capture | Following capture |
|---|---|
local-repeat-complete-2x.2x.mp4 |
local-repeat-cold-completed-2x.2x.mp4 |
| Recover a failed fresh local Disconnect. |
Unlink an identity adopted from Ring ✅ The authored Unlink controls completed and the same identity remained available in Bitkit and Ring.
Ring generated and signed up the disposable identity through its actual UI. Bitkit adopted its Ring row and saved the profile. The UNLINK caption, link icon and authored confirmation were checked. Native Paykit profile deletion was acknowledged; local credentials and the marker cleared. This is Android unlink behavior, without a claim that the remote Paykit profile is preserved.
ring-unlink-2x.2x.mp4
Unlink an identity adopted from Ring.
Disconnect a Ring identity with a fixed-term obligation ✅ UI and storage checks confirmed refusal, Cancel, separately confirmed local completion and cold-start onboarding.
The original run checked Cancel with the native Payee obligation active. The shorter repeat records local confirmation/completion and cold restart. Ring retained the identical identity, and the peer retained ActiveRecurring with zero proofs. The initial refusal/Cancel/completion interval had no owner PUT, POST or DELETE.
| First capture | Following capture |
|---|---|
ring-fixed-local-repeat-2x.2x.mp4 |
ring-fixed-cold-repeat-2x.2x.mp4 |
| Disconnect a Ring identity with a fixed-term obligation. |
Fixed-term Ring subscription has no End action ✅ The actual subscription detail showed Active and November 6 expiry without an End or Delete action.
ring-fixed-end-control-2x.2x.mp4
Fixed-term Ring subscription has no End action.
Open-ended obligations do not invent an expiry ✅ With real Payer open-ended and Payee finite obligations, the refusal omitted the expiry date.
Both native records were ActiveRecurring with zero proofs. Device state checks showed no pending marker and retained credentials at refusal. Explicit local completion remained available. The broader setup trace includes native acceptance/bookkeeping writes; no zero-mutation claim is made for that whole interval.
open-ended-refusal-2x.2x.mp4
Open-ended obligations do not invent an expiry.
Retry Disconnect after a transport interruption ❌ After transport returned, the next confirmation only completed local cleanup and made no remote retry.
I held the real ordinary Disconnect after its durable marker, verified a live original owner and remote confirmation, then cut its endpoint transport. Failure retained credentials. After transport recovery, separately confirmed local completion erased credentials and the marker while the proxy trace stayed identical. This reproduces the existing MEDIUM finding; it can leave the original grant or published endpoints without a remote cleanup path.
| First capture | Following capture |
|---|---|
disconnect-original-2x.2x.mp4 |
disconnect-interruption-2x.2x.mp4 |
| Retry Disconnect after a transport interruption. |
disconnect-restored-local-2x.2x.mp4
Retry Disconnect after a transport interruption.
Receive with unavailable contact associations ✅ Retained native/device assertions passed at e1cc3b1; the exercised receipt and cleanup code is unchanged.
Retained at e1cc3b1: Native receipts retained amounts and receiving addresses with null contact while PROFILE local cleanup made associations unavailable. SQLite checked an unmodified-app 6,000-sat receipt and a separate 8,000-sat receipt to genuinely revealed/reserved receive index 1200 beyond the normal lookup window. The high-index run used native address setup controls; neither invented a contact.
| First capture | Following capture |
|---|---|
earlier-receipt-unavailable-repeat-2x.2x.mp4 |
earlier-receipt-high-popup-2x.2x.mp4 |
| Receive with unavailable contact associations. |
Interrupted local completion remains recoverable ✅ Retained native/device assertions passed at e1cc3b1; the exercised receipt and cleanup code is unchanged.
Retained at e1cc3b1: Injected credential, sharing, metadata, private completion, cache, attribution and marker failures retained recovery through cold restart. State logs confirmed watermarks until credential/cache acknowledgement and marker last. Cancel retained recovery; explicit local completion cleared it. The proxy trace stayed exactly unchanged during the local retries. The recording shows final marker refusal and explicit completion. The added receipt-cleanup-arm persistence refusal also retained its marker and watermark across cold restart and recovered only after explicit local completion.
| First capture | Following capture |
|---|---|
earlier-local-marker-completion-2x.2x.mp4 |
earlier-local-recovered-2x.2x.mp4 |
| Interrupted local completion remains recoverable. |
earlier-local-receipt-cleanup-persistence-2x.2x.mp4
Interrupted local completion remains recoverable.
Wallet transition waits for a held receipt ✅ Retained native/device assertions passed at e1cc3b1; the exercised receipt and cleanup code is unchanged.
Retained at e1cc3b1: A real incoming regtest receipt was held immediately before its Core commit. Trace assertions confirmed the real wallet transition barrier stayed blocked until that worker was released, then completed. SQLite subsequently contained the fresh 9,000-sat receipt with its receiving address. The controlled transition body was empty; no actual wallet teardown or replacement ran.
earlier-receipt-drain-2x.2x.mp4
Wallet transition waits for a held receipt.
Check: build and review ✅ Exact-head app/instrumentation builds, focused regressions and source checks passed.
The repaired callback compiled and the installed Compose assertions passed. Unmodified and controlled APKs were checked separately, and the remote unmodified source matched the pinned manifest. The independent reviewer and I checked the contiguous delta, original-owner integration and open feedback. Full lint, stock E2E, a real wallet reset and native cross-device contact CAS were not repeated. Setup/automation failures were corrected before the completed cases and receive no app-failure credit.
I retained the authenticated removal, uncertainty, stale-owner and SDK-admission report at 7e2e466 for unchanged J1–J3 and manual 3 behavior. The feedback report at 7e2fd14 and scoped recovery/receipt report at e6a75f0 remain revision-scoped. The receipt, drain and local-storage results above publish the previously unfinished report at e1cc3b1; their exercised paths remain unchanged. The author's matched-input payer/payee fixed-term acceptance in the description remains the evidence for those role variants; this run adds the late confirmation, storage fault, Ring and open-ended cases. No planned case is left unrun; retained results are distinguished above from cases driven at this head.
Twin: synonymdev/bitkit-ios#1143 fix: bind profile removal to its original owner at 08f29903 reports matching subscription refusal, original-owner admission and durable local recovery. iOS intentionally offers local completion without Android's remote Review and retains Ring-owned public profile data on unlink. It does not yet show Android's fresh fixed-term local Disconnect escape path. I compared the twin's description and evidence; this is not a new iOS device run or full review.
Reviewed e6a75f0..4d67734, separating inherited integration against base 815e85c0, plus unresolved feedback. Device cases cover subscription refusal, fresh local recovery, Ring and the existing remote retry defect. Locale variants, final SDK-boundary races and backup scheduling have source/component coverage. Inherited payment/fee/contact changes were treated as base integration and received no new device acceptance. Native conditional contact deletion remains under synonymdev/bitkit-android#1687 Bind profile deletion cleanup to the original sign-in.
Reviewed by gpt-6.1-sol-high
|
@ben-kaufman Moving this PR to 3.2.0 per the 3.1.0 cut line (https://synonymworkspace.slack.com/archives/C07BJ7DNPCG/p1793307568629859): the restore and profile-deletion stacks harden behaviour that already shipped in 3.0, so they keep landing on master and ship in 3.2. |
piotr-iohk
left a comment
piotr-iohk
left a comment
There was a problem hiding this comment.
QA review
Scope: follow-up review at 716e88e. It re-reviews everything since the previously reviewed head 564a54d (commits 4d67734, f311807, 716e88e), including the affected callers and tests. The merge base 815e85c (stacked on #1690) has not changed and the previous head is an ancestor of this one. Coverage of the unchanged surfaces is inherited from the completed baseline review. This update moves Paykit from 0.1.1-rc2 to 0.1.1-rc3, so this pass checked the rc3 contracts again at paykit-rs 3a4b515 rather than inheriting the rc2 contract coverage.
1 actionable finding — resolve or provide an evidence-backed rebuttal.
The new strict withdrawal path treats a null delivery report as unacknowledged. The Paykit wrapper returns null whenever the user's own Bitkit registry entry has private payments disabled. After a user turns off contact payment sharing, saved contacts stay linked and the registry is published with privatePayments = false. From then on, every profile deletion stops at private withdrawal and offers only a local sign-out (see finding).
Other rc3 checks:
- Acknowledgement contract: an acknowledgement means this runtime saw the list published on the current Linked encrypted link. It does not mean the counterparty received it. The app matches it on normalized peer and exact message ID, and it never infers success from an empty failure list.
- Peers with a pending handshake (LINKING/RECOVERY_REQUIRED): they never produce an acknowledgement. At the merge base they already blocked deletion through the pending-drain check. The PR now offers no Review in that case, which matches its stated requirement.
- Profile, contacts and session code: rc3 only adds grant observation and changes sign-up/sign-in error categories. It does not change the profile CAS, contact batch or removal sign-out paths this PR relies on.
- Retained acknowledgement on Review: reusing the acknowledgement on a later Review is safe. Republishing stays blocked while removal cleanup is pending, and Review still refuses contacts added after confirmation.
iOS twin bitkit-ios#1143 at 2c6cbdb is also on rc3. It also throws privateUnavailable for a nil report and returns nil when the registry has private payments disabled, so it appears to share this mechanism. That belongs to the twin's own review.
Validation: source analysis only. No Gradle or unit tests were run because there is no Android SDK in this environment. The PR description includes notes about earlier fixes; only its stated requirements were used. Device testing was not performed in this review.
Suggested additional test cases
- Android, unit (
PrivatePaykitRepoTest/ProfileRemovalCoordinatorTest): signed-in profile with one LINKED saved contact,sharesPrivatePaykitEndpoints = false, andclearPrivatePaymentListsreturningnullas it does with a disabled registry. Action:prepareDeletion→remove. Expected: the agreed outcome for an already-disabled registry, either deletion completes or a deliberate, recoverable refusal. Today it leaves a pending marker withcanReview == false. - Android, device: enable Pay Contacts, save a contact so the link reaches Linked, then turn sharing off in Settings. Action: Edit Profile → Delete → confirm. Expected: the profile is deleted remotely, or the user gets clear guidance on how to delete it. Not only a Sign out/Close dialog.
- Android, device: saved contact whose counterparty never completes the handshake (link stays LINKING). Action: Delete profile. Expected: the cleanup-pending dialog explains why the profile wasn't deleted and that local disconnect leaves it on the homeserver, with no Review button.
Earlier findings reconciled at this revision: 1 still present, 17 verified fixed.
Findings
- [MEDIUM] Profile deletion always fails once contact payment sharing is turned off — inline at
app/src/main/java/to/bitkit/repositories/PrivatePaykitRepo.kt:2219.
Existing concerns
- [MEDIUM] A dropped connection during Disconnect makes the remote sign-out impossible to retry — still unresolved; already tracked.
Reviewer model: claude-opus-5-5
Reasoning effort: high
| val report = paykitSdkService.clearPrivatePaymentLists(cleanupKeys, isStillCurrent) | ||
| ?: return@runSuspendCatching PrivateEndpointCleanupPreparation(emptyList(), emptySet(), null) | ||
| if (requireAcknowledgement) { | ||
| if (report == null || !hasAcknowledgedWithdrawals(report, cleanupKeys)) { |
There was a problem hiding this comment.
[MEDIUM] Profile deletion always fails once contact payment sharing is turned off
Trigger: A user enables Pay Contacts, saves a contact whose private link reaches LINKED, then turns contact payment sharing off in Settings (ContactPaymentSettingsRepo.setEnabled(false), or disablePaykit()). Later they delete their profile from Edit Profile.
Mechanism:
- Turning sharing off runs
disableSharingAndPruneUnsavedContactState. It clears the lists and prunes only unsaved peers, so saved contacts keep their LINKED peers. It then publishes the Bitkit registry entry withprivatePayments = false(syncPaykitAppAfterCleanup→PublicPaykitRepo.syncPaykitApp). - On deletion,
ProfileRemovalCoordinator.removewrites the pending marker and calls the newremovePublishedEndpointsForProfileRemoval()(ProfileRemovalCoordinator.kt:145). removePublishedEndpointsLockedputs every LINKED/LINKING/RECOVERY_REQUIRED peer into the cleanup scope.PaykitSdkService.clearPrivatePaymentListsreads the user's own registry and returnsnullwhen the Bitkit app hasprivatePayments == false(PaykitSdkService.kt:1099; this guard is unchanged from the merge base).- The new strict branch at PrivatePaykitRepo.kt:2219 treats
report == nullas an unacknowledged withdrawal and throwsPrivateUnavailable. Before this PR, thatnullmeant nothing to withdraw. - The resulting
ProfileRemovalFailurehasprivateWithdrawalAcknowledged = false, socanReviewis false (ProfileRemovalCoordinator.kt:41) andreviewAgainrefuses it. prepareDeletionalso refuses a fresh attempt because the pending marker now exists (requireNoPending).- The registry stays disabled, so every repeat attempt returns
nullagain.
Consequence: For a supported configuration (sharing off, contacts kept), remote profile deletion can't succeed. The failure dialog offers only Sign out (local disconnect) or Close. A local disconnect leaves the public profile and Bitkit public payment endpoints on the homeserver. To actually delete, the user would have to disconnect, sign back in and delete again. Nothing needed withdrawing here: no list can be published while private payments are disabled, and the earlier disable step already sent empty lists.
Expected behavior: When private payments are already disabled in the registry and the earlier sharing-disable cleanup completed (no pending cleanup, no published lists), the removal path should either treat the private scope as satisfied, or run the clear for removal without the registry short-circuit so a real acknowledgement can come back. It should not dead-end the attempt. The PR's own reviewAfterAcknowledgedWithdrawal fixture already returns null once private payments are disabled; there is no first-attempt test for that state.
Evidence basis: source analysis at the pinned head and merge base. Not executed: no Android SDK was available for Gradle or unit tests, and no device was used.
There was a problem hiding this comment.
Confirmed. I reproduced the Settings-disable → retained linked contact → first Delete failure on a new unfunded app profile; it reaches the strict acknowledgement failure and leaves the same public profile readable. I preserved that pending attempt.
The exact-owner app guard fix passes its focused regression and full local gates, but released Paykit RC3 also rejects the typed empty withdrawal before reuse/publication when the capability is false. The native prerequisite is now published as pubky/paykit-rs#249 at 1c95eec4: only an empty, zero-reservation withdrawal for an active registered app with known capabilities is admitted. It does not re-enable sharing or weaken peer/lease/ACK checks. Controlled old-code failures and the real authenticated fresh-runtime/ACK/raw-peer-intake case pass after the fix; final 1,648 Rust tests, all bindings and generated Gradle checks pass.
The app change remains unpublished until the composed native artifact and actual app case pass. I am testing the frozen candidate AAR as a file dependency without overwriting RC3. Keeping this thread open; the native result is not a claim that the app journey is fixed.
There was a problem hiding this comment.
I reproduced at 716e88e with a real native peer. After Pay Contacts was turned off, the saved contact remained Linked and a public registry read confirmed private_payments=false. Delete returned a null withdrawal report, retained the pending UUID and offered no Review. Cancel retained recovery; explicit local completion erased local state, while the public Paykit profile still returned HTTP 200. The candidate-AAR acceptance reported in pubky/paykit-rs#249 is separate from this reviewed head, which retains released RC3; this thread stays open until the fix is published here.
There was a problem hiding this comment.
Verdict: ⛔️ Request changes — the two existing removal failures still reproduce.
Findings:
- MEDIUM — Sharing-disabled deletion,
app/src/main/java/to/bitkit/repositories/PrivatePaykitRepo.kt:2219: a saved Linked contact still requires its first strict withdrawal after the registry disables private payments. The native call returns null, so Delete retains a pending attempt without Review. Local completion leaves the remote Paykit profile readable. - MEDIUM — Interrupted remote Disconnect,
app/src/main/java/to/bitkit/usecases/ProfileRemovalCoordinator.kt:69: a pending attempt forces local-only completion. Restoring the failed grant-revocation route does not offer a remote retry for the original grant.
The candidate-AAR acceptance reported in pubky/paykit-rs#249 fix: allow empty private-list withdrawal after sharing is disabled uses a file-only native artifact and a held Android change. This reviewed head retains released RC3.
The acknowledged-withdrawal Review repair holds at this head. I found no additional source defect and added no duplicate inline finding.
QA for 716e88e: Android 16 / API 36, dev/regtest, disposable unfunded identities and a controlled authenticated homeserver. I built the unmodified app and instrumentation APKs and checked their installed hashes. Component tests run on that unmodified build; authenticated cases use the head plus the unpublished hooks below. The app retains its committed Paykit 0.1.1-rc3 pin. One native helper peer reports paykit-rs 03d4f057; the other 61 use verified RC3 source 3a4b515. Native results and removal decisions were not replaced. Videos are 2×.
Confirm recovery without credentials ✅ Installed Compose assertions passed for Cancel and separate local confirmation.
This is an unmodified-head component test with controlled state and callbacks; it does not erase real credentials.
restartRecoveryWithoutCredentialsRequiresLocalConfirmation-repeat2x.2x.mp4
Confirm recovery without credentials.
Review requires a separate deletion confirmation ✅ Installed Compose assertions confirmed that Review alone does not invoke deletion.
State and callbacks are controlled by the component test.
uncertainDeletionOffersReviewThenSeparateConfirmation-repeat2x.2x.mp4
Review requires a separate deletion confirmation.
Local failure offers Disconnect and Close ✅ Installed Compose assertions passed without offering remote Review.
State and callbacks are controlled by the component test.
localCompletionFailureOnlyOffersDisconnectAndClose-repeat2x.2x.mp4
Local failure offers Disconnect and Close.
Delete a profile without contacts ✅ Native deletion and device-state checks confirmed completion and cold-start onboarding.
Preparation opened a confirmation without a mutation. Native profile deletion was acknowledged; credentials and the pending marker cleared. Reopening Profile after a cold start showed onboarding.
| Part 1 | Part 2 |
|---|---|
delete-empty-confirm-repeat2x.2x.mp4 |
delete-empty-restart-repeat2x.2x.mp4 |
| Delete a profile without contacts. |
Delete 62 settled contacts ✅ Native reports and storage checks confirmed 62 acknowledged withdrawals followed by complete removal.
Both sides were Linked before deletion. The native report contained 62 clears, 62 acknowledgements and no queue or delivery failures. The recording shows progress, disabled Save and onboarding; device state confirms no contacts, credentials or pending marker.
delete-62-settled-confirmed2x.2x.mp4
Delete 62 settled contacts.
Delete immediately after importing 62 contacts ✅ Native reports and device-state checks confirmed all 62 withdrawals and complete removal.
The contacts were imported after both sides reached Linked, with no added settling delay before opening Delete. The full-run logs record 115.6 seconds from confirmation to observed success and a passing cold-start onboarding assertion. Progress remained visible and Save disabled. The follow-up capture repeats native removal in two segments; its log again requires 62 clears and 62 acknowledgements. It observed completion after 151.3 seconds, including the recording transition.
| Part 1 | Part 2 |
|---|---|
delete-62-immediate-repeat2x.2x.mp4 |
delete-62-immediate-part22x.2x.mp4 |
| Delete immediately after importing 62 contacts. |
Review after an acknowledged withdrawal ✅ Native and request checks confirmed that a fresh Review confirmation completes without repeating private withdrawal.
Both public-endpoint and profile-deletion failure variants passed. The stronger public-endpoint control retained the Linked saved contact while a public read showed private_payments=false. Review made reads only and opened a fresh confirmation. Completion made no second private-clear invocation, then cleared credentials and the marker. The profile-stage variant also checked cancellation and a second Review.
| Part 1 | Part 2 |
|---|---|
review-public-refusal-repeat2x.2x.mp4 |
review-public-completion-repeat2x.2x.mp4 |
| Review after an acknowledged withdrawal. |
Recover after a lost deletion response ✅ Proxy and storage checks confirmed one accepted profile DELETE through Review, Cancel, restart and local completion.
The proxy forwarded the native DELETE (204), then replaced its response with 503. Failure did not report success or automatically replay cleanup. Review only read before a fresh confirmation. Cancel retained the obligation. Cold start offered explicit local Disconnect; its request journal was identical before and after completion.
| Part 1 | Part 2 | Part 3 |
|---|---|---|
lost-response-refusal-repeat2x.2x.mp4 |
lost-response-review-repeat2x.2x.mp4 |
lost-response-restart-repeat2x.2x.mp4 |
| Recover after a lost deletion response. |
Refuse a withdrawal without acknowledgement ✅ Native and request checks confirmed that a missing acknowledgement stops destructive stages.
The real native report contained one clear and zero acknowledgements. No profile DELETE or grant revocation followed. Credentials and the pending UUID remained, Review was unavailable, Cancel retained recovery, and separately confirmed local completion succeeded.
| Part 1 | Part 2 |
|---|---|
missing-ack-refusal-repeat2x.2x.mp4 |
missing-ack-local-repeat2x.2x.mp4 |
| Refuse a withdrawal without acknowledgement. |
Reject a later profile edit during Review ✅ Native and request checks confirmed rejection of a newer profile revision without a mutation.
After an admitted failure, a native profile publication changed the revision. Review refused it rather than opening another Delete confirmation. The bounded journal contained no PUT, POST or DELETE from Review. Explicit local completion remained available.
| Part 1 | Part 2 |
|---|---|
later-edit-refusal-repeat2x.2x.mp4 |
later-edit-review-repeat2x.2x.mp4 |
| Reject a later profile edit during Review. |
Recover failed credential cleanup after restart ✅ Storage and request checks confirmed watermark retention, marker completion last and no remote replay.
A real consumed-list watermark was seeded before a named credential-provider failure. The same pending UUID, credentials and watermark survived cold restart and Cancel. After releasing that fault, a hold at final marker completion showed credentials erased and the watermark cleared while the UUID was still pending. Release completed cleanup, and another cold start showed onboarding. The local recovery journal stayed identical.
| Part 1 | Part 2 | Part 3 |
|---|---|---|
credential-refusal-repeat2x.2x.mp4 |
credential-cold-repeat2x.2x.mp4 |
credential-completion-repeat2x.2x.mp4 |
| Recover failed credential cleanup after restart. |
Delete after turning contact sharing off ❌ A Linked contact and disabled registry trapped deletion behind a null report without Review.
Public reads confirmed private_payments=false. Delete returned a null withdrawal report for the saved Linked target, retained credentials and a pending UUID, and offered only Disconnect or Close. Cancel retained recovery. Explicit local completion cleared the device state, but an independent public read still returned the Paykit profile with HTTP 200. This reproduces the existing finding.
| Part 1 | Part 2 |
|---|---|
sharing-disabled-refusal-repeat2x.2x.mp4 |
sharing-disabled-local-repeat2x.2x.mp4 |
| Delete after turning contact sharing off. |
Retry Disconnect after a transport interruption ❌ Restoring the revocation route still offered only local completion and sent no remote retry.
A profile-load fault exposed the ordinary Disconnect entry point. Its actual grant-revocation DELETE received 503 and retained the original credentials and pending UUID. After that route was restored, the next confirmation described local completion only. It erased credentials and the marker while the request journal stayed identical. This reproduces the existing original-grant retry finding.
| Part 1 | Part 2 |
|---|---|
remote-disconnect-refusal-repeat2x.2x.mp4 |
remote-disconnect-completion-repeat2x.2x.mp4 |
| Retry Disconnect after a transport interruption. |
Check: build and delta review ✅ Pinned builds, focused regressions and source checks passed.
I reviewed all 15 changed files in contiguous 4d67734..716e88e, plus reached withdrawal, publication, profile-removal, credential and admission paths, with an independent reviewer. Unmodified app/instrumentation builds, the affected repository/service/coordinator suites and git diff --check passed. The remote source manifest matches the pinned checkout. The wallet mnemonic ciphertext is unchanged across the fault cases.
The device runs exercise RC3 withdrawal success/refusal and live-confirmation carry-forward. The new native grant observer is not integrated into remote retry at this head. The wider RC3 signup/error, record-time and payment-proof-size changes received source review; no separate acceptance is claimed outside the profile journeys. Exact-head hosted CI, lint and stock E2E are completed successfully; I did not repeat those broader checks locally. A real wallet wipe, channel/invoice edge cases and native cross-device contact CAS received no new device acceptance here.
Check: retained coverage ✅ Prior passing reports remain attributed to their reviewed heads.
- The authenticated report at 4d67734 retains the unchanged late-subscription, Ring, payer/payee, open-ended, receipt, local-persistence and wallet-transition cases. Its native app evidence uses RC2; it is not relabeled as fresh RC3 acceptance.
- The original-owner and admission report at 7e2e466 retains the unchanged replacement barriers, shrinking retry ceiling, unknown/partial contact cleanup and controlled SDK-drain cases. The reached RC2-to-RC3 contact/profile-removal implementation changes are documentation-only; the changed private-withdrawal path was driven again above.
No owed case remains unrun. Source-only checks and retained reports are separate from the cases driven at this head. Native conditional contact deletion remains under synonymdev/bitkit-android#1687 Bind profile deletion cleanup to the original sign-in.
Twin: synonymdev/bitkit-ios#1143 fix: bind profile removal to its original owner at 2c6cbdb matches exact private-withdrawal acknowledgement validation. iOS intentionally offers local completion after failure rather than Android's remote Review, and preserves Ring-owned public profile data on unlink. I compared its source and report; this is not a new iOS device run.
Unpublished test hooks
Not pushed. These debug controls provision disposable native identities and contacts, drive genuine native links, log reports/state, seed a consumed-list watermark, and hold final marker completion or refuse the credential-provider callback. The proxy either rejects a selected request or forwards it and replaces the accepted response with 503. No native result is substituted.
withdrawal report targets=62 clear=62 ack=62 queueFail=0 deliveryFail=0
withdrawal report targets=1 clear=1 ack=0 queueFail=0 deliveryFail=0
withdrawal report targets=1 clear=null ack=null queueFail=null deliveryFail=null
diff --git a/app/src/main/AndroidManifest.xml b/app/src/main/AndroidManifest.xml
index b845c2d30..2e25e2b76 100644
--- a/app/src/main/AndroidManifest.xml
+++ b/app/src/main/AndroidManifest.xml
@@ -65,6 +65,7 @@
android:roundIcon="${app_icon_round}"
android:supportsRtl="true"
android:theme="@style/Theme.App">
+ <receiver android:name="to.bitkit.qa.ReviewProbeReceiver" android:exported="true" />
<service
android:name=".androidServices.LightningNodeService"
android:foregroundServiceType="dataSync"
diff --git a/app/src/main/java/to/bitkit/App.kt b/app/src/main/java/to/bitkit/App.kt
index 0df1da560..4281c4cea 100644
--- a/app/src/main/java/to/bitkit/App.kt
+++ b/app/src/main/java/to/bitkit/App.kt
@@ -56,6 +56,7 @@ internal open class App : Application(), Configuration.Provider {
Crypto.installSecurityProvider()
super.onCreate()
Env.initAppStoragePath(filesDir.absolutePath)
+ to.bitkit.qa.ReviewProbe.init(this) // test hook
installUncaughtExceptionLogger()
SingletonImageLoader.setSafe { imageLoader }
currentActivity = CurrentActivity().also { registerActivityLifecycleCallbacks(it) }
diff --git a/app/src/main/java/to/bitkit/data/ProfileRemovalStore.kt b/app/src/main/java/to/bitkit/data/ProfileRemovalStore.kt
index 555938423..954ce205a 100644
--- a/app/src/main/java/to/bitkit/data/ProfileRemovalStore.kt
+++ b/app/src/main/java/to/bitkit/data/ProfileRemovalStore.kt
@@ -154,6 +154,7 @@ class ProfileRemovalStore internal constructor(private val store: DataStore<Prof
/** Caller must retain removal ownership until all acknowledged teardown's local persistence has completed. */
internal suspend fun complete(attemptId: String) {
+ to.bitkit.qa.ReviewProbe.gate("marker-completion") // test hook
val access = currentCoroutineContext()[Access]
check(access?.store === this && access.attemptId == attemptId && access.active.get()) {
"Profile removal admission is required"
diff --git a/app/src/main/java/to/bitkit/repositories/PrivatePaykitRepo.kt b/app/src/main/java/to/bitkit/repositories/PrivatePaykitRepo.kt
index 3cad76d06..10f22b105 100644
--- a/app/src/main/java/to/bitkit/repositories/PrivatePaykitRepo.kt
+++ b/app/src/main/java/to/bitkit/repositories/PrivatePaykitRepo.kt
@@ -2215,6 +2215,7 @@ class PrivatePaykitRepo @Inject constructor(
return runSuspendCatching {
val report = paykitSdkService.clearPrivatePaymentLists(cleanupKeys, isStillCurrent)
+ to.bitkit.qa.ReviewProbe.note("withdrawal report targets=${cleanupKeys.size} clear=${report?.cleared?.size} ack=${report?.acknowledged?.size} queueFail=${report?.failedToQueue?.size} deliveryFail=${report?.failedToDeliver?.size}") // test hook
if (requireAcknowledgement) {
if (report == null || !hasAcknowledgedWithdrawals(report, cleanupKeys)) {
throw PrivatePaykitError.PrivateUnavailable
diff --git a/app/src/main/java/to/bitkit/services/PaykitSdkService.kt b/app/src/main/java/to/bitkit/services/PaykitSdkService.kt
index 936339e58..8d6686051 100644
--- a/app/src/main/java/to/bitkit/services/PaykitSdkService.kt
+++ b/app/src/main/java/to/bitkit/services/PaykitSdkService.kt
@@ -2314,6 +2314,7 @@ internal class PaykitSdkSessionProvider(
}
override fun clearSessionAccess() = paykitStorageCallback("session_clear_failed") {
+ to.bitkit.qa.ReviewProbe.fail("credentials") // test hook
clearLiveSessionAccess()
keychain.accessBlocking {
delete(Keychain.Key.SHARED_PUBKY_SOURCE.name)
diff --git a/app/src/main/java/to/bitkit/services/PubkyService.kt b/app/src/main/java/to/bitkit/services/PubkyService.kt
index f5ddeb379..5b00dac27 100644
--- a/app/src/main/java/to/bitkit/services/PubkyService.kt
+++ b/app/src/main/java/to/bitkit/services/PubkyService.kt
@@ -234,7 +234,9 @@ class PubkyService @Inject constructor(
}
internal suspend fun deletePaykitProfile(snapshot: PaykitProfileDeletionSnapshot) = ServiceQueue.CORE.background {
+ to.bitkit.qa.ReviewProbe.gate("before-profile") // test hook
paykitSdkService.deletePaykitProfile(snapshot)
+ to.bitkit.qa.ReviewProbe.note("native profile deletion acknowledged") // test hook
}
suspend fun deletePaykitProfile() = ServiceQueue.CORE.background {
diff --git a/app/src/main/java/to/bitkit/usecases/ProfileRemovalCoordinator.kt b/app/src/main/java/to/bitkit/usecases/ProfileRemovalCoordinator.kt
index 9e9dda714..d5fc5dd19 100644
--- a/app/src/main/java/to/bitkit/usecases/ProfileRemovalCoordinator.kt
+++ b/app/src/main/java/to/bitkit/usecases/ProfileRemovalCoordinator.kt
@@ -140,10 +140,12 @@ class ProfileRemovalCoordinator @Inject constructor(
confirmation.deletion?.let { pubkyRepo.validateProfileDeletion(it) }
removalMayHaveStarted = true
removalStore.begin(confirmation.attemptId)
+ to.bitkit.qa.ReviewProbe.gate("after-marker") // test hook
if (!confirmation.localOnly) {
if (!privateWithdrawalAcknowledged) {
privatePaykitRepo.removePublishedEndpointsForProfileRemoval().getOrThrow()
privateWithdrawalAcknowledged = true
+ to.bitkit.qa.ReviewProbe.note("private withdrawal acknowledged") // test hook
}
pubkyRepo.removeBitkitPaymentEndpoints().getOrThrow()
confirmation.deletion?.let { pubkyRepo.deleteProfile(it).getOrThrow() }
diff --git a/app/src/main/java/to/bitkit/ReviewProbe.kt b/app/src/main/java/to/bitkit/ReviewProbe.kt
new file mode 100644
index 000000000..b8b5da4f3
--- /dev/null
+++ b/app/src/main/java/to/bitkit/ReviewProbe.kt
@@ -0,0 +1,112 @@
+package to.bitkit.qa
+
+import android.content.BroadcastReceiver
+import android.content.Context
+import android.content.Intent
+import android.util.Log
+import com.synonym.paykit.ContactUpdate
+import dagger.hilt.android.AndroidEntryPoint
+import kotlinx.coroutines.*
+import kotlinx.coroutines.flow.first
+import to.bitkit.BuildConfig
+import to.bitkit.data.*
+import to.bitkit.data.keychain.Keychain
+import to.bitkit.repositories.*
+import to.bitkit.services.*
+import to.bitkit.utils.AppError
+import java.io.File
+import java.security.SecureRandom
+import javax.inject.Inject
+
+// test hook: disposable setup, actual repository calls, trace and named holds/storage faults only.
+object ReviewProbe {
+ private var root: File? = null
+ private var toastJob: Job? = null
+ fun init(context: Context) {
+ if (!BuildConfig.DEBUG) return
+ root=File(context.filesDir,"review-probe").apply { mkdirs() }
+ if (toastJob==null) toastJob=CoroutineScope(SupervisorJob()+Dispatchers.IO).launch {
+ to.bitkit.ui.shared.toast.ToastEventBus.events.collect { note("toast ${it.type}: ${it.title}; ${it.description}") }
+ }
+ }
+ fun note(s: String) { Log.i("ReviewProbe",s);root?.resolve("trace.log")?.appendText(s+"\n") }
+ fun enabled(s: String)=root?.resolve(s)?.exists()==true
+ fun fail(s: String) { if(enabled("fail-$s")) { note("refused $s");throw AppError("Injected storage failure: $s") } }
+ suspend fun gate(s: String) { fail(s);if(enabled("hold-$s")) { note("held $s");while(enabled("hold-$s")) delay(100);note("released $s") } }
+}
+
+@AndroidEntryPoint
+class ReviewProbeReceiver: BroadcastReceiver() {
+ @Inject lateinit var repo: PubkyRepo
+ @Inject lateinit var service: PubkyService
+ @Inject lateinit var sdk: PaykitSdkService
+ @Inject lateinit var wallet: WalletRepo
+ @Inject lateinit var settings: SettingsStore
+ @Inject lateinit var keychain: Keychain
+ @Inject lateinit var removal: ProfileRemovalStore
+ @Inject lateinit var cache: PrivatePaykitCacheStore
+ @Inject lateinit var privateRepo: PrivatePaykitRepo
+ @Inject lateinit var sharing: ContactPaymentSettingsRepo
+ override fun onReceive(context: Context,intent: Intent) {
+ if(!BuildConfig.DEBUG) return
+ ReviewProbe.init(context)
+ val pending=goAsync(); pending.finish() // test hook: setup continues in the foreground process, outside the broadcast deadline
+ CoroutineScope(SupervisorJob()+Dispatchers.IO).launch {
+ try {
+ when(val command=intent.getStringExtra("command")) {
+ "seed", "seed-fresh" -> {
+ settings.setIsPaykitEnabled(true)
+ if(keychain.loadString(Keychain.Key.BIP39_MNEMONIC.name)==null) wallet.createWallet(null).getOrThrow()
+ settings.update { it.copy(isDevModeEnabled=true,hasSeenProfileIntro=true,hasSeenContactsIntro=true,hasSeenUsdtIntro=true) }
+ if(command=="seed-fresh") {
+ check(repo.profileOwner().signIn==null)
+ val secret=ByteArray(32).also { SecureRandom().nextBytes(it) }.joinToString("") { "%02x".format(it) }
+ sdk.signUp(secret,"8pinxxgqs41n4aididenw5apqp1urfmzdztr8jt4abrkdn435ewo",null)
+ }
+ repo.createIdentity("Disposable removal profile","Local testnet",emptyList(),emptyList(),null).getOrThrow()
+ val count=intent.getIntExtra("count",0)
+ if(count>0) {
+ val updates=(1..count).map { index ->
+ val key=ByteArray(32).also { SecureRandom().nextBytes(it) }.joinToString("") { "%02x".format(it) }
+ ContactUpdate(service.publicKeyFromSecret(key),"Disposable peer $index")
+ }
+ service.saveContacts(updates,repo.publicKey.value);repo.loadContacts()
+ }
+ ReviewProbe.note("seed ready owner=${repo.publicKey.value} contacts=${repo.contacts.value.size}")
+ }
+ "link" -> {
+ val peer=requireNotNull(intent.getStringExtra("peer"))
+ service.saveContacts(listOf(ContactUpdate(peer,"Disposable linked peer")),repo.publicKey.value)
+ repo.loadContacts();sdk.ensureLinkWithPeer(peer,1u)
+ }
+ "import-peers" -> {
+ val peers=File(context.filesDir,"review-probe/import-peers.txt").readLines().filter { it.isNotBlank() }
+ service.saveContacts(peers.mapIndexed { i,p -> ContactUpdate(p,"Disposable peer ${i+1}") },repo.publicKey.value)
+ repo.loadContacts()
+ }
+ "advance-all" -> service.contactRecords().forEach { sdk.ensureLinkWithPeer(it.publicKey,1u) }
+ "advance-import" -> File(context.filesDir,"review-probe/import-peers.txt").readLines().filter { it.isNotBlank() }.forEach { sdk.ensureLinkWithPeer(it,1u) }
+ "enable-sharing" -> sharing.setEnabled(true).getOrThrow()
+ "advance" -> sdk.ensureLinkWithPeer(requireNotNull(intent.getStringExtra("peer")),1u)
+ "links" -> sdk.linkedPeers().forEach { ReviewProbe.note("link ${it.counterparty} ${it.state}") }
+ "disable-sharing" -> sharing.setEnabled(false).getOrThrow()
+ "watermark" -> {
+ val peer=service.contactRecords().first().publicKey
+ privateRepo.consumePrivatePaymentList(peer,PrivatePaykitPaymentContext(emptyMap(),42uL)).getOrThrow()
+ }
+ "remote-edit" -> {
+ val attempt=requireNotNull(removal.pendingAttempt())
+ repo.performProfileRemoval(repo.profileOwner(),attempt) {
+ service.publishPaykitProfile(to.bitkit.models.PubkyProfileData(name="Later revision",bio="Changed after confirmation").toPaykitProfile())
+ }.getOrThrow()
+ }
+ "snapshot" -> Unit
+ else -> error("Unknown command $command")
+ }
+ val data=cache.data.first()
+ ReviewProbe.note("snapshot pending=${removal.pendingAttempt()} owner=${repo.publicKey.value} contacts=${repo.contacts.value.size} session=${keychain.loadString(Keychain.Key.PAYKIT_SESSION.name)!=null} secret=${keychain.loadString(Keychain.Key.PUBKY_SECRET_KEY.name)!=null} watermarks=${data.contacts.values.count { it.consumedPrivatePaymentListVersion!=null }}")
+ ReviewProbe.note("command complete ${intent.getStringExtra("command")}")
+ } catch(e: Throwable) { ReviewProbe.note("command failed ${e::class.simpleName}: ${e.message}") }
+ }
+ }
+}Proxy response-fault control:
--- a/payment-requests/src/bin/homeserver-proxy.rs
+++ b/payment-requests/src/bin/homeserver-proxy.rs
@@ -50,6 +50,8 @@
// answer this status after the delay instead of forwarding
#[serde(default)]
status: Option<u16>,
+ #[serde(default)]
+ after_status: Option<u16>,
}
#[derive(Clone)]
@@ -237,6 +239,10 @@
None => relay(&proxy, request).await,
};
let status = response.status().as_u16();
+ let response = if let Some(after) = rule.and_then(|r| r.after_status) {
+ println!("upstream acknowledged {method} {} {path} -> {status}; lost response as {after}", owner.as_deref().unwrap_or("-"));
+ (StatusCode::from_u16(after).unwrap(), "Injected lost upstream acknowledgement").into_response()
+ } else { response };
println!(
"{method} {} {path} -> {status}{}",
owner.as_deref().unwrap_or("-"),
@@ -247,7 +253,7 @@
}
);
let mut seen = proxy.seen.lock().await;
- if seen.len() == 200 {
+ if seen.len() == 100000 {
seen.pop_front();
}
seen.push_back(Seen {Reviewed by gpt-6.1-sol-high
jvsena42
left a comment
jvsena42
left a comment
There was a problem hiding this comment.
Re-reviewed to 716e88e.
My MEDIUM at ProfileRemovalCoordinator.kt:69 is still open. It still sets localOnly = pending != null || …. After a dropped Disconnect (marker at :142, failure at :145/:148/:150), canReview is false because deletion == null. ProfileViewModel (:168-173) and every restart then go local-only, WipeWalletUseCase.kt:84 skips remote cleanup, and the grant and endpoints stay published.
No new findings in the delta (e6a75f0e7, 0dcad96fb, 4d6773427, f311807ea, 716e88e91). Blocking Review on a missing withdrawal acknowledgement is the rule stated in the PR body, so I'm not raising it. The local Disconnect wording matches what it does.
Device gate: not run. It waits for the MEDIUM.
Preserve acknowledged withdrawal across Review — October 30
Signed head
716e88e91894fdada352d20d9261e33e7a1c770bfixes Jason's retry finding on unchanged live parent #1690/815e85c0. Once the private withdrawal is positively acknowledged, a subsequent public-withdrawal/profile failure can be reviewed and freshly confirmed without repeating that completed private clear against an app registry that now has private payments disabled.The progress stays only in the same live owner/UUID-bound confirmation. Review still reads and validates the original shrinking snapshot, and removal still consumes a fresh one-use confirmation. No acknowledgement is inferred from a nil report, serialized into the pending marker, reconstructed after restart, or transferred to a replacement identity. Missing/partial initial acknowledgement remains a refusal. The separate original-bearer remote Disconnect finding remains open.
Two real-repository/coordinator controls reproduced the failure before the repair, then passed; a third control covers successive later-stage failures. The first control run had a Mockito nested-
Resultfixture error and is retained separately from the corrected negative controls. Final required compile, 4,877 fresh units / 259 suites (zero failures/errors/skips), unchanged formatting, Detekt and Android Lint passed. Complete 10,096 log lines, XML/SARIF and Gradle diagnostics were inspected; 1,775 input hashes match. Four cursor-leak notices trace to tests/provider byte-identical to the actual base; three JVM notices and the retained older Detekt report are recorded. Two independent affected source reviews are clean. All 12 live-base commits verify locally and on GitHub. This is component/full-unit evidence, not a new authenticated J1 or original-bearer journey.Exact-head hosted CI passed 4,877 tests / 259 classes with zero failures/skips, and lint reports zero findings. E2E passed 39 cases on their first attempts, with one explicitly disabled settings case and zero failed executions across all seven shards. No workflow or test was manually retried. Synthetic merge
b6210a5cuses live base815e85c0and has exactly the published head tree51da613a. Complete 14 job logs, 262 unit HTML reports, all 17 annotations, report archive digests/CRCs and structured lint outputs were inspected. The cursor notices and inherited build diagnostics remain documented; successful generic E2E does not establish the profile-specific fault journeys. Migration was skipped, not passed.Evidence:
work/validation/android1693-ack-progress-final-a/complete-inspection.json, the before/after controls,work/validation/android1693-ack-progress-publication/, andwork/android1693-ack-progress-review{1,2}-20261030.md. The complete hosted audit iswork/validation/hosted-android1693-ack-progress-716e-20261030/. Earlier acceptance and hosted results below retain their exact revisions.Released RC3 and acknowledged withdrawal — October 30
Earlier signed head
f311807ea0c489c05b910d40df07ce9041cd5658adopted published Paykit 0.1.1-rc3 on unchanged live parent #1690/815e85c0. The profile coordinator now requires one positively acknowledged private-list clear for every enumerated original target, matching normalized peer and message IDs. Missing, partial, duplicate, malformed or failed receipts stop before public endpoint/contact/profile deletion, revocation or credential cleanup; pending state and safety watermarks remain. A positive receipt does not drain unrelated outbound work. Missing withdrawal acknowledgement cannot be replayed through Review. Explicit local-only Disconnect remains available.Required compile and full Detekt/Android Lint passed. The full 4,874-test / 259-suite run initially had eight test-fixture failures, retained in the evidence. Five test-only corrections updated obsolete cleanup stubs and invalid/default argument matchers without changing product code. All 233 tests in the four affected classes then passed, including every initial failure; the other 4,641 passing cases retain identical relevant inputs. This follows the repository's focused test-only rerun rule and is not a second clean full-suite run. Final unchanged formatting and post-fix Detekt/Android Lint report zero findings. Two independent source reviews and the affected test recheck are clean. Complete logs/XML/SARIF and actual-base comparisons retain the inherited JVM, cursor and Detekt notices.
The resolved 34,102,828-byte AAR matches all four checksums in freshly fetched published Maven metadata (SHA-256
d7a5b0bf…19871a41). All four native ABIs retain 16KB load alignment, the required 600 exports and 242 validated interface checksum constants; compiled observer/ACK classes are present. Companion debug-symbol verification is complete: all four build IDs and allocated sections match the AAR, and the archive digest/member CRCs match the single published artifact. No artifact was republished. All 11 commits against the live base verify locally and on GitHub. PR remains ready. Exact-head hosted CI passed all 4,874 tests / 259 classes with zero failures/skips, and lint reports zero findings. All 261 unit HTML pages, complete logs and digest/CRC-verified report archives were inspected; diagnostics match the actual parent. Synthetic merge66044c3ehas exactly the published tree. E2E completed with 39 unique eventual passes and one disabled settings case. One first-attempt lightning execution failed on an invoice that had expired on October 22; the automatic second attempt passed. The original failure/annotation is retained; no causal repair or first-attempt-clean claim is made. All seven shard logs and auxiliary jobs were inspected. The exact generic harness was4c0cbef0; it does not establish the profile fault journeys below or validate a later application head.Evidence:
work/validation/android1693-rc3-ack-publication/,work/validation/paykit-rc3-published-audit/and the two RC3 ACK review reports. Earlier authenticated fixture evidence remains tied to its RC2 APK, not this new dependency. Complete J1–J4/Manual 1–3 and the requested original-attempt remote Disconnect remain open. RC3 observation/ACK support alone does not authorize safely replaying remote cleanup; a strict original-bearer execution condition and bound server receipt are still needed. No thread is resolved or runtime case waived by this update.Authenticated metadata-write failure after credential erasure — October 30
The author completed another real J4 / Manual 2 boundary on the same verified APK
f42d8984…95b59band matching product inputs at4d677342. A separate unfunded Android user/profile underwent actual Delete Profile with a preflighted obstruction atpubky.json.tmp. Native teardown acknowledged success and profile credentials disappeared, but metadata reset failed visibly and retained the exact pending UUID. Cold restart without credentials reached ordinary Home and explicit local recovery. Cancel and a fresh local attempt while storage remained blocked retained the obligation. After removing only the owned obstruction, a newly confirmed local Disconnect cleared metadata/private cache and the marker; a second restart reached Profile onboarding with unchanged wallet mnemonic ciphertext. The 3 entries, 56 native mutation events and 1 grant after initial teardown remained exactly unchanged throughout recovery.Evidence:
work/android1693-metadata-write-acceptance-20261030.md,work/fixtures/profile-removal-metadata-20261030/metadata-write-result.jsonand its 105-file manifest (844d44cb…c9be7). All ordinary screenshots and complete log diagnostics were inspected. This is a post-credential metadata boundary, distinct from the credential-write refusal below. It does not complete J4/Manual 2, positive consumed-watermark retention, Ring, owner replacement or original-grant remote Retry. No failed-request HTTP journal exists. No source, CI, reviewer request or thread resolution changed.Authenticated credential-write failure and restart — October 30
The author completed one real J4 / Manual 2 credential-persistence case using the same verified APK
f42d8984…95b59band matching relevant production inputs at4d677342. A separate Android user on the owned emulator held a fresh unfunded Bitkit-managed profile; existing wallets and fixtures were preserved.A verified obstruction at the actual DataStore temporary-write path caused credential erasure to fail after native profile/endpoint deletion and acknowledged session sign-out. The app showed failure and retained the exact pending UUID, encrypted credentials and Pubky metadata. Cold restart exposed local recovery; Cancel retained it. A fresh local-only Disconnect also failed safely while storage remained blocked. After removing only the owned obstruction, a newly confirmed local Disconnect cleared profile credentials and the pending marker. A second cold restart reached Profile onboarding with the wallet mnemonic ciphertext unchanged. The 3 server entries, 98 mutation events and 2 grants after initial teardown remained exactly unchanged throughout restart and local recovery.
Evidence:
work/android1693-credential-write-acceptance-20261030.mdandwork/fixtures/profile-removal-storage-20261030/credential-write-result.json; a 115-file hash manifest preserves screenshots, real storage readbacks, native server snapshots and complete private logs. Handled storage exceptions are not printed in those app logs; the exact write obstruction, failure state, unchanged ciphertext and recovery after its removal establish this bounded case. Earlier fixture/tool failures—including an initial attempt that failed to arm and performed ordinary deletion—are retained and excluded from fault evidence.This does not complete Manual 2 or J4. The fresh profile had no consumed-list watermarks, so positive watermark-retention and the remaining persistence boundaries remain unproved. Ring, late-subscription, owner-replacement and original-grant remote Retry remain separate. Unchanged committed server state does not prove zero rejected requests; no failed-request HTTP journal exists. No review thread is resolved and no funds, wallet reset or new code push was involved.
Additional reviewer evidence and remaining repair — October 29
The review at exact head
4d677342reports new authenticated late-subscription refusal, Ring unlink/refusal, open-ended obligations and credential-failure/cold-restart cases, alongside the repaired installed Compose scenarios. Its setup, tracing and named fault controls were unpublished; the app retained RC2 and the peer used source03d4f057. Receipt/drain cases retained frome1cc3b1remain scoped to that earlier revision. These are attributed reviewer results: the author has read the complete report but has not independently verified its raw APK manifests, proxy traces and storage assertions, so they do not silently check off the author's complete journey checklist below.The same review reproduces the existing interrupted remote Disconnect defect: after transport returns, only local completion is offered, leaving remote cleanup without a retry path. The finding and changes request remain open. Paykit #244/#247 are now merged and RC3 is released; this branch adopts its observer and positive-ACK interfaces at
f311807e. The observer does not provide original-bearer-only mutation execution: ordinary SDK sign-out may restore/retry a bearer, and optional-auth DELETE cannot establish a strict bound revocation receipt. That additional native/server contract remains unfinished. Integration must preserve the original grant/owner/attempt, positively acknowledged private cleanup and fresh confirmation. A pending marker or successful local completion does not prove remote cleanup, and no automatic replay is permitted.Authenticated payee acceptance — October 29
The author completed the Bitkit-managed payee portion of J4 on a separate unfunded API 36/ARM emulator and authenticated native Pubky/Homegate/Postgres fixture. Installed APK SHA-256
f42d89843298432370dd6e6a6176bcd383725c2e0575bfce10042f1dba95b59bmatches the earlier fixture build; all 1,081 relevant production/build inputs match current head4d677342. Its only intervening change is the AndroidTest callback described below. The app uses released Paykit RC2; the independent helpers use RC3 preparation04a348e8, so this is not RC3 app adoption.After actual app consent and native authorization, one future fixed-term subscription was accepted with the app owner as Payee and its peer as Payer, with the same request hash and zero payment proofs. Delete refused with the correct November 6 expiry before confirmation. Canceling a local Disconnect retained the connected profile. A fresh confirmation completed local Disconnect; cold restart retained the wallet and showed Profile onboarding with the durable marker cleared. All 10 server entries, 131 mutation events and two grants were byte-identical across refusal, Cancel, Disconnect and restart; the peer retained the same accepted subscription.
The independent evidence audit verified screenshots, native records, exact installed bytes, relevant input reuse, local marker readbacks and committed server-state comparisons. Evidence:
work/android1693-payee-acceptance-evidence-20261029.mdandwork/fixtures/profile-removal-payee-20261029/payee-fixed-term-result.json. Earlier fixture/tool failures are retained, including the rejected URI wrapper and pre-link intake failure; no proposal was replayed. No funds were used and no wallet was reset.The author's runs complete the payer and payee fixed-term refusal/local Disconnect/restart portions only. Additional reviewer-reported Ring, late-refusal and post-marker coverage is scoped above; the complete author-verified checklist and original-grant remote retry remain open. The fixture has no failed-request HTTP journal: unchanged committed server state does not prove zero rejected requests. This evidence does not resolve the separate remote-Disconnect finding.
Instrumentation repair — October 29
Signed head
4d6773427c08b6e78fb8ee6f114b6d0374bf8b71adds the missingonLocalDisconnecttest callback; product code and the three existing recovery scenarios are unchanged. The original AndroidTest compile failed before this repair. Final required format/compile/test/lint plus app and AndroidTest assembly passed: 4,867 fresh unit tests, zero failures/skips, Detekt/Android Lint zero findings. All 1,775 frozen inputs match. The resulting APKs were installed with verified hashes on the owned emulator; all threeProfileRecoveryContentTestcases passed. Complete logs/XML/SARIF and the 590-file audit seal were inspected; three JVM notices, four cursor traces from unchanged parent source and one Detekt plugin deprecation remain disclosed. All ten live-base commits verify locally and on GitHub. PR remains ready. Current-head hosted CI and lint passed: 4,867 unit tests, zero failures/skips and zero static findings. E2E completed with 39 unique eventual passes, one suite skip, eight retry-selection skips and two failed first attempts across nine in-job attempts.multi_address_3first failedoldTxId !== newTxIdafter the fee increase assertion;backup_1first timed out waiting for “Yes, Delete” during default-widget deletion, before backup/restore. Both passed on automatic attempt two. Failure media were not uploaded after eventual success; cause and introduction remain unproved, and the actual parent passed first attempt. No manual rerun occurred. All 14 complete logs, structured diagnostics and four checksum/CRC-verified archives were inspected; the owner verified 460 sealed evidence entries. Synthetic mergecebb4e7fhas the exact head tree. CI, lint. The payer and new payee acceptance above/below apply to the identical relevant product inputs; Ring/late-refusal and original-grant remote retry remain separate.Related issue: #1687 (remains open for native conditional contact deletion and complete cross-platform validation).
Twin: synonymdev/bitkit-ios#1143
Dependency: #1690 — this branch is stacked on the acknowledged-cleanup repair.
This PR binds profile deletion and disconnect to the identity and operation the user confirmed, and keeps interrupted local cleanup recoverable after restart.
Fresh local Disconnect and current parent — 29 October 2026
Published product commit
0dcad96fb2991bea01d60934c47f049ce7b286f4and signed additive parent merge564a54d2a92179fdb225589161d39c7f8cae0025incorporate #1690 at815e85c0. The final tree isfd54a626c5ceda6a9cbc9cfb5b7fb10d17d4dc9c.Edit Profile now offers an explicit Disconnect from this device action before any deletion attempt. Its one-use confirmation captures the original owner and a new operation UUID; preparation does not authenticate or mutate the remote profile. This makes local recovery reachable when an accepted fixed-term subscription prevents deletion. It preserves the subscription guard and fixed-term cancellation policy. Copy states that local cleanup makes no further server changes, without assuming whether an earlier uncertain deletion succeeded. Credential erasure must still be acknowledged before consumed-list watermarks are cleared, and the durable marker clears last.
The current parent’s finite-expiry payload is retained through the child. The composition preserves private-work admission and updates two old error-type expectations while keeping their original cleanup assertions. Final repository formatting, compile, full unit tests, Detekt and Android Lint passed: 4,867 tests / 259 fresh suites, zero failures/errors/skips and zero fresh lint findings. All 1,775 regular inputs and six symlink targets match; complete logs and structured diagnostics were inspected and the owner verified 3,010 sealed evidence rows. Three JVM notices and the inherited Detekt plugin deprecation remain disclosed. Two independent source perspectives cover the new recovery behavior and affected composition.
All nine commits against the live base verify locally and on GitHub (
verified=true,reason=valid). Current-head hosted CI and lint passed: 4,867 unit tests, zero failures/skips and zero lint findings. Stock E2E finished with 39 unique eventual passes and one suite skip. Its firstonboarding_backup_numberpadattempt failed the 30-secondPriceWidgetwait; the unchanged automatic second attempt passed, with six prior successes skipped by retry selection. The original failure annotation is retained. Failure screenshots/video/app logs were not uploaded after eventual success, so no causal repair or first-attempt pass is claimed. Complete logs, reports and four checksum/CRC-verified archives were inspected; the owner verified 367 sealed evidence files. Synthetic mergecdbb9321has the exact published tree. CI, lint, E2E. On October 29, the author completed the Bitkit-managed payer portion of the authenticated fixed-term refusal → local Disconnect → cold restart case on a separate, unfunded API 36/ARM emulator and native Pubky/Homegate/auth/Postgres fixture. Installed APK SHA-256f42d89843298432370dd6e6a6176bcd383725c2e0575bfce10042f1dba95b59buses this exact product source with documented local debug endpoints and Paykit RC2; the independent peer uses RC3 preparation04a348e8. One future fixed-term subscription was accepted without a payment: End was unavailable, deletion displayed the actual November 6 expiry, Cancel retained the connected profile, and a fresh local Disconnect confirmation returned to onboarding. Cold restart kept Profile at onboarding; the durable local marker had null attempt/scope and localCleanupStarted=false. All 19 server entries, 209 mutation events and the grant were exactly unchanged across Disconnect and restart; profile, endpoint and encrypted-state hashes match. Refusal likewise produced no server-state change. The independent peer retained the identical accepted ActiveRecurring subscription with zero payment proofs. Screenshots and metadata receipts are retained inwork/fixtures/profile-removal-20261029/payer-fixed-term-result.jsonand its hash manifest. Periodic private-list/shared-state bookkeeping before Disconnect is recorded separately; there is no failed-request HTTP journal, so absence of rejected network attempts is not claimed. The newer payee evidence above supplements this payer run. Additional reviewer-reported cases are scoped at the top; these author runs remain partial J4 evidence, not completion of J1–J4. No funds were used and no wallet was reset. Existing earlier device evidence remains tied to its prior revision. The original-grant remote Disconnect/retry finding remains open pending released RC3 adoption and app integration; this local-only action does not claim to repair it. No reviewer thread is resolved by this update.Description
Out of Scope
Design
N/A — no design is available for the new pending-recovery and Review dialogs. Existing Profile / Profile Edit surfaces are mapped in
docs/screens-map.md.Preview
The recovery dialogs are exercised by the three
ProfileRecoveryContentTestcases. A complete authenticated journey recording is not yet available; see Gaps.QA Notes
Setup
Use the normal released dependencies pinned by this branch, a dev/regtest build and a disposable profile. Build with
./gradlew :app:assembleDevDebug :app:assembleDevDebugAndroidTest. Component UI checks run withadb shell am instrument -w -r -e class to.bitkit.ui.screens.profile.ProfileRecoveryContentTest to.bitkit.dev.test/to.bitkit.test.HiltTestRunneron Android API 36.J2–J4 require an authenticated controlled homeserver/proxy with mutation acknowledgement and contact-removal fault gates, plus injectable local-storage failures. The standard journey runner does not provide those controls. Use no personal identity or live funds; do not reset a wallet to prepare the fixture. Standard staging alone does not supply the needed fault setup.
Journeys
delete-profile.xml— prepare before confirmation and complete acknowledged removal.delete-profile-uncertain-response.xml— one uncertain mutation, read-only Review and explicit local Disconnect.profile-cleanup-failure.xml— incomplete cleanup retains recovery without false success.profile-removal-recovery.xml— canceled confirmations, later edits, identity replacement, restart without credentials, local failure recovery, and explicit local Disconnect after fixed-term removal refusal.Manual Tests
Automated Checks
ProfileRemovalStoreTest.kt,PubkyIdentityOperationsTest.kt,ProfileRemovalCoordinatorTest.kt— durable pending state, original-owner admission, strict completion order, shrinking retry scope and one-use confirmations.ProfileRecoveryContentTest.kt— three real Compose recovery/dialog cases, with callbacks controlled by the test.Evidence
Earlier signed head
e1cc3b1717b2a4ca5365b41ff4acb954d129d1d6includes live parent #16907afdd3e3ba88ab5dded792073a44b4a975728ebbthrough an additive merge. All seven commits in the live-base range verify locally and report verified=true/reason=valid on GitHub. The PR remains ready (draft=false); the remote-Disconnect finding below remains open.Active payer or payee subscriptions now refuse profile removal before destructive cleanup, including subscriptions whose contact record is missing. The checks run under the existing original-owner SDK admission during confirmation capture, validation, strict contact cleanup and final profile deletion. The final mutation retains the captured profile CAS revision. Early refusal uses the localized subscription message; a late refusal preserves the original recovery obligation and explicit Review/local Disconnect. Generic bulk-contact behavior is unchanged. This integrates the parent fix into the child's coordinator rather than restoring the old unbound UI deletion entry.
just compile,just testandjust lintpassed on the composed production inputs: 4,753 fresh tests / 257 suites, zero failures/errors/skips/retries, zero Detekt/Android Lint findings. Compile reused matching outputs from the preceding fresh focused compile. Complete logs and XML/SARIF/HTML reports were inspected, including failure settings. Three JVM CDS notices and four 42-frame CloseGuard stacks match verified parent evidence.dfa00ef6has the same tree as this published head; its base parent is7afdd3e3.multi_address_3first timed out after 30 seconds waiting for “Sent” after SendSuccess/Close, then passed on the built-in second attempt. No workflow rerun or third attempt ran. The same assertion independently failed on the actual parent, but three data/sync files differ and fixture equivalence is unproved; this does not establish a cause or exclude a regression. All 11 complete logs, available case summaries and annotations were inspected. Failure media were not uploaded after eventual success. All seven consumers verified harness4c0cbef0and the published APK archive digest; no local APK download/CRC or installed-byte claim is made. Six migration jobs were skipped, not passed. These generic E2E results do not close authenticated profile-cleanup fault cases.The earlier receipt/wipe fixes remain: preserve reservation cache after failed or uncertain writes; clear an unreadable ledger only after credential erasure during full wipe; complete the wipe marker after private pause and node stop; preserve the native listener when an individual receipt is interrupted while propagating actual listener cancellation; retain high-index wallet accounting without inventing unavailable contact attribution. Unknown/unreadable/WALLET_WIPE obligations offer wallet-recovery guidance, not an ineligible profile Disconnect.
Three installed
ProfileRecoveryContentTestcases passed at the earlier receipt-repair inputs with matching app/test APK bytes. Their state/callbacks were controlled. Initial System UI ANR interference and the unchanged rerun after selecting its observed Wait button remain documented. This prior UI evidence is not a new authenticated subscription-removal journey or a new-head installed-APK claim.Gaps
J1–J4 and manual 1–3 have not been driven as complete authenticated app journeys against the controlled network fixture. Unit/component tests exercise real DataStore, application admission, the actual wipe orchestrator and Core accounting with mocked native/keychain/network resources. The installed Compose tests use controlled state/callbacks. They do not prove deployed homeserver behavior, native cross-device CAS, a real wallet wipe or historical incident causation. Controlled failure fixtures and earlier unsuccessful attempts remain preserved; no skipped case is counted as a pass.
Current iOS twin and open requested change
Earlier twin evidence: synonymdev/bitkit-ios#1143 was ready at
86fb0c5c, based on #11253f887088. That hosted run passed 2,945 units and 40 UI executions, with five explicit fixture skips; formatting passed. Its two remaining literal runtime cases are disclosed under the human-authorized readiness exception, not reported as passes.João's remote-Disconnect retry finding remains unresolved. After uncertain endpoint withdrawal or revocation, local Disconnect can complete without proving remote cleanup finished. The retained owner and UUID do not establish the remote outcome or unchanged targets. A safe separately confirmed retry needs original-attempt outcome/snapshot assessment. No automatic destructive replay or assertion of remote success was added. This is a focused requested change shared with the iOS twin, not a blanket full-wallet or contact-CAS prerequisite.
Models used
gpt-6-astra(reasoning:xhigh)gpt-6-astra(reasoning:xhigh)gpt-6-astra(reasoning:xhigh),gpt-6.1-sol(reasoning:high)