Skip to content

fix: open pubky sign-in links while the session restore is deferred - #1543

Merged
ovitrif merged 4 commits into
masterfrom
fix/pubky-signin-shared-state-busy
Oct 16, 2026
Merged

ovitrif merged 4 commits into
masterfrom
fix/pubky-signin-shared-state-busy

Conversation

@ovitrif

@ovitrif ovitrif commented Oct 16, 2026 •

Copy link
Copy Markdown
Collaborator

Closes #1541

Refs:

This PR opens the Authorize sheet for a Pubky sign-in link that cold-starts Bitkit while the Pubky session restore is deferred. Today the link waits for the restore and fails after 60 s when the Paykit shared state stays locked (device logs).

Description

  • Fixes sign-in links failing with "Couldn't Load Your Pubky Profile" when the session restore is deferred with shared_state_busy. The session import itself takes no lock, but the Paykit runtime start that follows it (initialize and the noise key publication) takes the homeserver lock on the shared state. That lock has a 60 s lease, and a killed process leaves it held until the lease runs out (up to 5 minutes after an interrupted write). The restore is deferred for that whole time.
  • Lets the sign-in wait stop after a deferred restore when the device is online and Bitkit stores its own key for the saved session. The link then continues with that key's identity, and the restore keeps retrying in the background as before. This matches iOS, which opens the sheet from the saved session and the stored key without waiting for the restore. Offline, the wait keeps the loading toast until the device reconnects or the 60 s run out, as before.
  • Keeps the approval guarantee: the early path needs both a saved session and a stored key, and the key is the one saved with the session (persistSessionAccess writes both, and deletes the key when the session has none). A Ring-sourced identity is still excluded (hasLocalSecretKey), so it keeps waiting for the restore as before. Approval still signs only with that key.
  • Lets sign-in approval wait only for the Paykit platform setup instead of the full runtime setup. Approval, with or without a companion claim, signs with the stored key through a fresh session bootstrap, which takes no shared-state lock. An approval whose claim grants Paykit access still waits for the full setup, since it reads the Paykit app registry.
  • Shows the stored key's identity on the Authorize sheet while the session is not restored, and keeps a grant signup for that identity out of identity creation.

Out of Scope

  • paykit-rs: the lock lease and the 5-minute wait after an interrupted write; the unreleased paykit-rs change that drops the 5-minute wait needs a newer homeserver.
  • AppViewModel: a later link replacing a pending sign-in link, now tracked in bug: a link opened during a pending pubky sign-in drops the sign-in #1542.
  • PubkyRepo: the restore retry schedule for the rest of the app (profile, contacts, payments) while the shared state stays locked.
  • PaykitSdkService: sign-in links whose Bitkit claim grants Paykit access still wait for the full Paykit setup; the Pubky Shop sign-in carries no Bitkit claim.

Design

N/A — no UI changes.

Preview

N/A — no new UI; the existing Authorize sheet opens sooner. The locked shared state cannot be produced on demand, so the change is covered by unit tests and the manual test below.

QA Notes

Journeys

  • updated signin-cold-start.xml — the sign-in link opens the Authorize sheet after the loading toast, also when the restore is deferred while online

Manual Tests

  • With a Pubky profile created in Bitkit: force-stop Bitkit and at once open a fresh sign-in link, several times in a row → the Authorize sheet opens each time, also on runs whose log shows Deferred session restoration … shared_state_busy, and Authorize succeeds — a locked shared state is not in Capabilities
  • Force-stop Bitkit → open a sign-in link → turn the device network off for 25 s as Bitkit opens → the loading toast stays until the device reconnects, then the Authorize sheet opens — device network toggling not in Capabilities

Automated Checks

  • added AppViewModelSendFlowTest.kt — a cold sign-in link opens the sheet with the stored key after one deferred restore while online, and keeps waiting offline
  • added PubkyAuthApprovalViewModelTest.kt — a grant signup with only a stored key shows that identity and authorizes without identity creation
  • added PaykitSdkServiceTest.kt — approval waits only for platform setup, unless its claim grants Paykit access
  • added PubkyRepoTest.kt — the stored key's identity is read only with a saved session and without a Ring source

@ovitrif ovitrif added this to the 3.0.5 milestone Oct 16, 2026
@ovitrif ovitrif self-assigned this Oct 16, 2026
@ovitrif
ovitrif requested review from a team, ben-kaufman and pwltr and removed request for a team October 16, 2026 17:40
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 16, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-16T17:44:40.671379Z 23e8a11 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@ovitrif ovitrif removed this from the 3.0.5 milestone Oct 16, 2026
Comment thread app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt Fixed
Comment thread app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt Fixed
Comment thread app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt Fixed
Comment thread app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt Fixed
Comment thread app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt Fixed
@greptile-apps

greptile-apps Bot commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerView in GreptileConfidence Score: 4/5

[Medium impact] The changes are centered on Pubky sign-in and authorization.

The PR appears safe to merge, with a non-blocking gap in tests for the approval setup rule.

Findings

  1. P2 Approval setup split lacks tests ▶

Summary

Opens Pubky authorization from a stored local key while session restoration is deferred.

  • Pubky sign-in links open with a saved key before session restore finishes.
  • The Authorize sheet uses the saved identity and can approve without full Paykit setup.

Acknowledged scope: ovitrif explicitly deferred later links replacing pending sign-in links, the general restore retry schedule, and changes to the shared-state lock lease.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Pubky sign-in link] --> B[Attempt session restore]
    B --> C{Identity ready?}
    C -->|Yes| F[Open Authorize sheet]
    C -->|No| D{Stored local key without Ring source?}
    D -->|Yes| F
    D -->|No| E[Keep waiting for restore]
    F --> G[User approves]
    G --> H{Claim includes Paykit access?}
    H -->|Yes| I[Wait for full runtime setup]
    H -->|No| J[Ensure platform setup]
    I --> K[Send signed approval]
    J --> K
Loading

Reviews (1) · Last reviewed commit: "fix: open pubky sign-in links while the ..." · Reviewed by Greptile

Comment thread app/src/main/java/to/bitkit/services/PaykitSdkService.kt
@ovitrif

ovitrif commented Oct 16, 2026

Copy link
Copy Markdown
Collaborator Author

Pushed 51339d0:

  • The sign-in link continues on the stored key only while the device is online. Offline it keeps the loading toast and the 60 s limit from fix: keep pubky sign-in links working on a cold start #1514, since an offline restore is deferred too.
  • The stored key counts only with a saved session, so a key written before any session exists never routes a link.
  • PaykitSdkServiceTest.kt covers the approval setup split, which answers the Greptile finding. The long log line flagged by detekt is shortened.

Comment thread app/src/test/java/to/bitkit/repositories/PubkyRepoTest.kt Fixed
Comment thread app/src/test/java/to/bitkit/repositories/PubkyRepoTest.kt Fixed
@ovitrif

ovitrif commented Oct 16, 2026

Copy link
Copy Markdown
Collaborator Author

Pushed 0f20f71: a PubkyRepoTest.kt test name is shorter, so detekt passes the line limit. Tests only, no behaviour change.

@talosmachina talosmachina left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

No findings. Lets a cold-start sign-in link open the Authorize sheet with the stored key's identity while the session restore is deferred, and lets approval wait only for the platform setup unless the claim grants Paykit access. Reviewed 51339d0, full tier (auth and keys path). Reasoned from the code and CI; the branch was not built here.

What I checked, and 5 candidates I ruled out

Read in full: AppViewModel.kt (awaitPubkyAuthIdentity, handlePubkyAuth, showPubkyIdentityUnavailableToast), PubkyAuthApprovalViewModel.kt (load, authorize, approveSignInRequest), PubkyRepo.kt (localIdentityPublicKey, approveAuth, approveAuthWithCompanionClaim, activeSecretKeyHex, managedSecretKeyFor, initializeSession, restoreSessionBackupState), PaykitSdkService.kt (initialize, approveAuth, approveAuthWithCompanionClaim, ensurePlatformSetup, persistSessionAccess, activateBootstrapResult)
Call sites traced: localIdentityPublicKey (3), ensurePlatformSetup (2), every writer of PUBKY_SECRET_KEY and PAYKIT_SESSION
CI: lint, detekt, build-local green at 51339d0; the build job with the unit tests was still running when this was posted

Ruled out

  • Sheet shows one identity, approval signs with another: with _publicKey null, activeSecretKeyHex() returns the same PUBKY_SECRET_KEY that localIdentityPublicKey() derives from, and every writer (persistSessionAccess, restoreSessionBackupState, createLocalIdentitySession) writes or deletes it together with the session. Narrowest of the five.
  • Ring-sourced identity signs with a stale local key: hasLocalSecretKey() excludes SHARED_PUBKY_SOURCE, and persistSessionAccess deletes the local key when an adopted pubky exists.
  • Grant signup re-creating an identity: requiresIdentityCreation(identityPublicKey != null) now routes it to approveAuth; before, it went to approveSignupAuth and failed with PubkyAlreadySignedInError.
  • ensurePlatformSetup racing initialize() outside setupMutex: PaykitAndroid.initializeOrThrow already runs outside that lock in launchIdentityRepublish (PaykitSdkService.kt:389), so a repeated call is an existing pattern.
  • Offline cold start skipping the wait: 51339d0 gates the early return on ConnectivityState.CONNECTED, and the new AppViewModelSendFlowTest case covers it.

Merge confidence: 4/5, no findings, but the change is on the auth path and the unit-test job had not finished on this head.

ben-kaufman
ben-kaufman previously approved these changes Oct 16, 2026
@github-actions

github-actions Bot commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

Regtest APK

Built from 0c623ec (run).

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

@jvsena42 jvsena42 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Code scan at 0f20f71: no code findings. I reviewed it as an auth and consent change. It addresses #1541.

Checked and clean:

  • Signer equals displayed identity. The early path requires hasSavedSession() && hasLocalSecretKey() (PubkyRepo.kt:1539-1545) and shows the identity derived from PUBKY_SECRET_KEY. With _publicKey == null, approval signs with activeSecretKeyHex(), the same key (:2094).
    • Every writer keeps PAYKIT_SESSION and PUBKY_SECRET_KEY paired: persistSessionAccess (PaykitSdkService.kt:1409-1417), restoreSessionBackupState and every clear path.
    • If the restore lands between load() and Authorize, managedSecretKeyFor (:2055-2081) returns only a key whose pubkey equals the restored identity, so it fails closed rather than signing as another identity.
  • Ring and stale keys. hasLocalSecretKey() requires no SHARED_PUBKY_SOURCE, and Ring adoption writes the source first. A session with no key, or a key with no session, is excluded.
  • Signup. requiresIdentityCreation(identityPublicKey != null) (VM :129) matches handlePubkyAuth. If the key vanishes, approveSignupAuth rechecks under initializeMutex.
  • Lock screen. Unchanged: launchScan defers while not authenticated, and the sheet's local-auth step is untouched.
  • Lifecycle. Background restore retries continue with notifyFailure = false. The early open is gated on CONNECTED; offline is covered by a test.
  • SDK lock. Approval calls ensurePlatformSetup() (:1560-1562) outside setupMutex and then waits FIFO on operationLock, so there is no lock-order inversion or deadlock. Paykit-access claims still wait for full setup, as the body says.
  • Parity. iOS already opens the sheet from the keychain session and secret key (AppViewModel.swift:1168-1215).

LOW, journeys: #### Journeys says N/A, not drivable. journeys/pubky-auth/signin-cold-start.xml drives exactly this route: force-stop, fresh signin_grant link, unlock, loading toast, Authorize, PubkyAuthOK, and the requester receives the grant. List it. Its <description> still says the app "waits visibly for the saved Pubky session", but the toast now ends after the first deferred attempt; update that sentence.

Performance-relevant (user-facing wait): the sign-in wait now ends after the first deferred restore attempt instead of up to 60 s. An Authorize tap during an in-flight restore attempt waits behind operationLock for up to one attempt (~20 s on the #1514 gate log).

Device gate (partial): 0f20f71 — not run yet. Pending: the #1541 repro (create a profile, then force-stop and open a fresh signin_grant link, ~8 times). On runs logging Deferred session restoration … shared_state_busy, expect the sheet within seconds, the sheet's pubkey equal to the later Restored paykit session key, Authorize → PubkyAuthOK, and the grant from that key. Also: the PIN-on variant, an offline 25 s cut, and the time from Authorize to OK during an in-flight attempt. The device result will follow.

@ovitrif

ovitrif commented Oct 16, 2026

Copy link
Copy Markdown
Collaborator Author

LOW, journeys: #### Journeys says N/A, not drivable. journeys/pubky-auth/signin-cold-start.xml drives exactly this route

@jvsena42 done in 0c623ec: the Journeys list names signin-cold-start.xml, and its description and the suite README now say the sheet opens with the stored key's identity when the restore is deferred while online, rather than after waiting for the session. Docs only.

@ovitrif
ovitrif requested a review from ben-kaufman October 16, 2026 18:24

@jvsena42 jvsena42 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Approving at 0c623ec. My journeys LOW is addressed: signin-cold-start.xml is listed and its description now covers the deferred restore, a docs-only change on top of the gated 0f20f71.

Device gate: 0f20f71 (app code identical at 0c623ec) — journeys: signin-cold-start.xml 9/9 pass, with the restore deferred twice in that run. perf: am start → sheet 14.7–16 s without a deferral, ~22–26 s with one; Authorize → PubkyAuthOK 8–14 s, and 5.5 s when tapped with a deferral in flight. No crash or ANR. l10n: no UI text changed.

  • #1541 repro: 9 force-stop + am start runs on a fresh profile. Every run opened the Authorize sheet, and none showed "Couldn't Load Your Pubky Profile". The 3 runs that deferred (r5, r8, r9) continued with the stored key at ~22 s:
    18:35:36.939 WARN  [PubkyRepo.kt:531] Deferred session restoration, keeping saved session [SharedStateBusy='code=shared_state_busy, …']
    18:35:50.585 WARN  [PubkyRepo.kt:531] Deferred session restoration, keeping saved session [AppError='code=shared_state_busy, …']
    18:35:50.605 DEBUG [AppViewModel.kt:3153] Continuing sign-in link with the stored Pubky key
    18:35:54.832 INFO  [PubkyAuthApprovalViewModel.kt:251] Auth approved for 'gate.test'
    18:36:44.167 INFO  [PubkyRepo.kt:479] Restored paykit session for 'pubkygb…4a9xmto'
    
  • Identity: in all 10 runs, the sheet's key GBD1…XMTO equals the later restored pubkygb…4a9xmto and the key the requester received.
  • PIN on: the PIN pad shows first, with no sheet behind it for 37 s. The sheet opens about 3 s after unlock, and Authorize asks for the PIN before PubkyAuthOK.
  • Airplane cut (~27 s): the loading toast stays up offline with no failure toast, and the stored-key shortcut is not taken offline. Retrying Pubky session restore fires on CONNECTED, and the sheet follows.

⚠️ Not verified on device: the PIN-on path combined with a deferral (neither PIN run deferred).

Seen but outside this PR: BackupRepo.kt:600 Backup failed for: 'WALLET' … shared_state_busy is logged at ERROR while the shared state is locked.

CI: 13 checks green; one was still pending when I posted.

@ben-kaufman ben-kaufman left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Reviewed 0c623ec and the existing feedback. No blocking findings. The saved-session identity path and separation of platform authorization from Paykit setup are covered by 779 passing focused repository, service, authorization, and navigation tests. Current hosted checks are green. Device journeys were not rerun as part of this review.

@ovitrif
ovitrif merged commit a0fe3fd into master Oct 16, 2026
21 checks passed
@ovitrif
ovitrif deleted the fix/pubky-signin-shared-state-busy branch October 16, 2026 20:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: pubky sign-in link can still time out on cold start while paykit holds the shared-state lock

5 participants