fix(mobile): require Builderlab consent for existing identities - #8823

🤖 Found one P2 lifecycle issue: declining backup leaves the local identity selection permanently committed.

🤖 Blocking — two correctness findings at 28929613. The gate itself is sound: it sits in front of the only upload call site (hosted_flow.dart:637), the approval tuple binds subject + pubkey + generation, and stale, disposed, back, restart and replacement callbacks all fall through to a fresh prompt. The two problems are about what happens around it.

1. Declining leaves the identity choice permanently committed (blocking).
chooseIdentity(A) → _continue → HostedIdentityService.resolve(selectedPubkey: A) adopts A into the subject's create-only slot (hosted_identity_service.dart:161-173 → deviceIdentities.adopt / store.adoptSaved → AccountIdentityStore.create) before the new prompt appears. "Keep key on this device" calls leave() (hosted_flow.dart:985-999), which clears the target and intent but not that slot, and the store has no way to release it (create throws occupied for a different key, account_identity_store.dart:144-145). On re-entry, resolve returns the stored A at hosted_identity_service.dart:116-129, before the chooser is built.

Failure: with saved keys A and B, choose A, decline, start again. A is presented again and B can never be selected for this account, even though nothing was approved, bound or uploaded. createIdentity() and importIdentity() also write the key before the prompt, so declining after "Create my Buzz identity" (for example to import an existing nsec instead) locks in the generated key the same way. This confirms the inline finding at hosted_flow.dart:592.

Fix: show the disclosure before the irreversible local write (selection, generation, import), or keep the choice tentative until consent commits it. Extend hosted_backup_consent_test.dart with A → decline → re-enter → select B, plus a restart before selecting B; the current journey uses one saved key, so it can't catch this.

2. Browser-initiated binding skips the gate, which contradicts the stated contract (blocking unless narrowed).
The whole gate sits inside if (target is! NostrBindDeepLink) (hosted_flow.dart:547). The NostrBindDeepLink branch accepts needsBinding / needsBackup and calls WebConnectBindingService().complete → _verifyBinding directly (hosted_flow.dart:660-675, web_connect_binding.dart:81-87). No backup is uploaded on this path. But the new comment ("Ask before permanent binding as well as export") and the description ("cancellation also prevents the permanent account binding") don't hold here: a key pinned by finding 1 after a decline can be bound to the account through a browser bind link with no disclosure.

Fix, either: (a) move the consent check before both binding callers, with bind-only wording for this route, and add a real-flow browser-bind journey asserting no /verify before consent and none after decline; or (b) if a browser-initiated bind is meant to be its own consent, say so in the comment and narrow the description's binding claim.

Optional

• The last sentence of the disclosure ("keep using your self-hosted communities") doesn't make sense to someone who just generated a key in this flow. Neutral wording like "Cancel to stop setup without uploading this key" works for every entry point.
• "Keep key on this device" reads as "continue without backup", but it calls leave(), which drops the retained invite or community intent with no message. A label like "Cancel setup", plus a line saying joining needs the backup, would match what it does.

Scores: Minimalism 9, Elegance 9, Correctness 7 (findings 1 and 2).

🤖 :bot: Jude’s code review agent

Verdict: REQUEST CHANGES
Reviewed: aa5f3e7bde9cf4412d6494e6a89a0d49e9a6a8f2..28929613ff0395d20d2e46e9812f06bf80dbba88
Risk: critical — consent, key export, and identity/account binding.

Blocking findings

1. [P2] Declining backup permanently reserves the selected local identity. mobile/lib/shared/hosted/hosted_flow.dart:538–542,570–591 calls resolve before displaying the new prompt. The resolver adopts the selected key into a create-only subject slot (hosted_identity_service.dart:145–173; account_identity_store.dart:135–161), while leave() does not release it (hosted_flow.dart:985–999). With saved keys A and B, choose A, decline, then reconnect: the resolver returns A before presenting the chooser, so B is no longer selectable, including after restart. This independently confirms the existing inline review. Author action: keep the selection tentative until approval, or safely release only a provably uncommitted same-attempt selection without deleting a possibly committed key. Add a real-flow A → decline/back → restart/re-enter → choose B regression.

2. [P2] The person cannot identify the account receiving the secret. mobile/lib/features/onboarding/hosted_connect_sheet.dart:92–98,152–166 shows the public key and service host, but only says “your Builderlab account.” An existing account session may be restored without a fresh sign-in (hosted_flow.dart:428–435), and the displayed screen gives no recognizable account identifier. Someone using an earlier, unintended account cannot recognize the recipient before granting it recovery access. Author action: let the person verify the receiving account using a verified, recognizable identifier or equivalent reliable account-confirmation flow, with a way to cancel or change account; cover a restored-wrong-account journey. An opaque internal subject alone is not a human-readable identifier.

3. [P2] A retry can imply prior binding and export have not happened. Once /verify succeeds, /nsec-backups can be accepted but its response lost; the missing receipt causes the same prompt to reappear (hosted_flow.dart:570–590, hosted_backup_service.dart:128–155). The prompt says “Continuing links” and offers “Keep key on this device” (hosted_connect_sheet.dart:92–98,163–166), although the binding already exists and the cloud may already have a copy. Declining cannot undo either. Author action: communicate the known/uncertain prior state and that cancelling stops future attempts rather than reversing previous effects; cover accepted upload → lost response → retry → decline → restart.

4. [P2] New binding’s permanence is not disclosed on every route. The confirmation says only “links this identity”; the separate chooser warns that the app cannot change the selected identity later (hosted_connect_sheet.dart:92–108), but create/import routes can reach confirmation without that chooser (hosted_journeys_test.dart:97–132). Author action: explain the no-in-app-change consequence before a new binding is approved, including create and import journeys.

Scope / verification: A recovered receipt also skips this gate before an unbound account is bound, and NostrBindDeepLink bypasses it without uploading a backup (hosted_flow.dart:547,660–675). The PR expressly says confirmed backups skip the prompt and limits its cancellation guarantee to identities without a confirmed backup; the browser’s separate disclosure was not inspected. These are scope/verification questions, not asserted additional blockers for this backup-consent PR.

Validation: source tracing and git diff --check aa5f3e7bde9cf4412d6494e6a89a0d49e9a6a8f2..HEAD passed with git rev-parse HEAD reporting the reviewed head and a clean worktree. GitHub reports the Mobile Checks, three Mobile Tests shards, Android, and aggregate Mobile checks successful; it also reports merge conflicts with current base. These reported CI results do not prove the missing journeys above. No independent Flutter suite, native app journey, screen-reader/text-scale or mutation run was completed: the isolated reviewers' Hermit/Flutter and simulator checks timed out. No production credentials or key material were used.

Verification owner: author for the concrete fixes and regression journeys; reviewer/tooling for independent native and accessibility verification. Residual risk: the external browser-binding disclosure and exact native user experience remain unverified. Merge-conflict resolution is a separate prerequisite, and any new head needs review.

🤖 Carl, an automated reviewer, commenting via Wes's GitHub account.

Verdict: no blocking defects at dfb29fe5e7 (base ce42853982). This was an independent Court review: a security and state-machine trace, a headless mutation run, and a UX and accessibility pass. The earlier blockers hold at source. No key is written to the account slot before consent. Imported keys stay tentative. The browser bind route asks before /verify. Approval is pinned to subject, pubkey and generation.

Validation: 95/95 touched tests pass with the public hosted defines. Three temporary journeys also passed: A → cancel → restart → B; paired → decline → slot empty; and a browser bind with a saved but unconfirmed slot. These mutations were each caught: early adoption, stale approval, and early imported-nsec persistence. Removing the bind-only gate stayed green across all 95 tests (inline). At 320×568 with 2× text the layout has no overflow and all actions are reachable. Native-device and human acceptance remain unrecorded.

Before merge (recommended): add the bind-gate journey step so that gate is falsifiable. The other inline notes are optional. Also optional: the chooser copy (hosted_connect_sheet.dart:117) still says the selection "will be bound… cannot change later", but the choice can now be cancelled at the next step.

🤖 One non-blocking test coverage improvement; no new production defect found.