Skip to content

fix: keep pubky sign-in links working on a cold start - #1514

Merged
ovitrif merged 7 commits into
masterfrom
fix/pubky-signin-cold-start
Oct 16, 2026
Merged

ovitrif merged 7 commits into
masterfrom
fix/pubky-signin-cold-start

Conversation

@ovitrif

@ovitrif ovitrif commented Oct 16, 2026 •

Copy link
Copy Markdown
Collaborator

Closes #1513

This PR fixes Pubky sign-in links opened while Bitkit is not running. It fixes a 3.0.0 blocker, for RC3: when the Pubky session restore failed on a cold start, Bitkit closed itself, dropped the sign-in request, and later sign-in links did nothing until Bitkit was opened by hand (repro).

Description

  • Fixes Bitkit closing and dropping the request after a failed Pubky session restore: the sign-in link handler now stays enabled while a saved session is not restored yet, as long as Bitkit stores its own secret key for it (not for a Ring-sourced identity). Disabling the handler closed the task the link had opened, and kept further links dead.
  • Keeps the guarantee from the original handler gating (fix: gate pubky auth handler #1162): the handler is still disabled when Paykit is off, when there is no identity, and for an identity without a key in Bitkit (Ring managed). Approval still checks the restored identity's own key before any Authorize sheet opens, so an early-enabled handler only routes the link.
  • Retries the session restore while a sign-in link waits, each time the device is online, for up to 60 seconds, instead of giving up after the first failed restore.
  • Adds a "Loading Your Pubky Profile" toast with a spinner while the link waits. It stays until the Authorize sheet opens or the wait fails, and comes back after other toasts (such as "Internet Connection Restored") interrupt it, however many arrive.
  • Shows "Couldn't Load Your Pubky Profile. Check your connection and try again." when the wait fails, for 6 seconds instead of 3, in place of the misleading relay timeout message.

Out of Scope

  • PubkyRepo: the "Your profile session has expired" toast that a restore failing while offline shows during the wait; it comes from the existing restore failure handling.
  • PubkyRepo: the time the session restore takes on a cold start (12-19 s in the repro); this PR makes the wait visible and survivable, not shorter.
  • bitkit-ios: no change; iOS does not register the raw pubkyauth scheme for OS link delivery.

Design

N/A — no design available.

Preview

Cold start, run after the network-cut case: loading toast, Authorize sheet, signed in to the AppNetwork cut for 25 s right after the link opens Bitkit: Bitkit stays open, retries, then signs in
cold5-2x.mp4
netcut2-2x.mp4

Recorded at 2x on an Android 16 emulator against staging, with a Pubky profile created in Bitkit.

QA Notes

Journeys

  • new signin-cold-start.xml — a cold-start sign-in link shows the loading toast until the Authorize sheet opens

Manual Tests

  • Force-stop Bitkit → open a sign-in link → turn the device network off for 25 s as Bitkit opens → Bitkit stays open with the loading toast, then the Authorize sheet opens once online; another sign-in link opens Bitkit afterwards — device network toggling not in Capabilities
  • Force-stop Bitkit → open a sign-in link → keep the device offline for more than 60 s → Bitkit stays open, the loading toast is replaced by "Couldn't Load Your Pubky Profile", and a later sign-in link still opens Bitkit — device network toggling not in Capabilities
  • With a Ring-sourced identity and no key stored in Bitkit: force-stop Bitkit → open a sign-in link → Bitkit does not show the Authorize sheet for that identity — Ring identity setup not in Capabilities

Automated Checks

  • added ToastQueueManagerTest.kt — a loading toast is dismissed by reference, never another toast, and returns after one or several toasts that interrupt it
  • updated PubkyAuthHandlerRegistrarTest.kt — a failed restore keeps the handler enabled for a stored local key, and still disables it without one
  • updated PubkyRepoTest.kt — a stored key counts as local only without a Ring source
  • updated AppViewModelSendFlowTest.kt — a cold sign-in link shows and clears the loading toast, retries a deferred restore until it succeeds, and shows the profile error after the wait times out

@ovitrif ovitrif self-assigned this Oct 16, 2026
@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-16T09:07:49.500185Z 8cc76f2 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.

@greptile-apps

greptile-apps Bot commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerView in GreptileConfidence Score: 4/5

[Medium impact] The changes affect Android's Pubky sign-in flow and shared toast display.

The PR appears safe to merge, with a non-blocking notification-order issue worth fixing.

Findings

  1. P2 Loading hides newer alerts ▶

Summary

Keeps Pubky sign-in links working while a saved session restores.

  • Cold-start Pubky sign-in links stay routed while the saved identity restores.
  • Pubky sign-in links wait visibly for the saved profile to restore.

One non-blocking issue remains: a second notification can get stuck behind the loading toast.

Acknowledged scope: ovitrif explicitly deferred the existing “Your profile session has expired” toast during an offline restore and improvements to restore speed.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Open sign-in link] --> B{Saved session ready?}
  B -->|No| C[Show loading toast]
  C --> D[Wait for initialization and retry restore]
  D -->|Unavailable| E[Wait 3 seconds and for network]
  E --> D
  D -->|60-second limit| F[Hide loading toast and show profile error]
  D -->|Ready| G[Check restored identity key]
  B -->|Yes| G
  G -->|Key available| H[Open Authorize sheet]
  G -->|No key| I[Show Ring guidance]
  H --> J[Hide loading toast]
Loading

Reviews (1) · Last reviewed commit: "fix: bring the sign-in wait toast back a..." · Reviewed by Greptile

Comment thread app/src/main/java/to/bitkit/ui/shared/toast/ToastQueueManager.kt

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8cc76f2415

ℹ️ 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".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread app/src/main/java/to/bitkit/ui/shared/toast/ToastQueueManager.kt Outdated
@ovitrif

ovitrif commented Oct 16, 2026

Copy link
Copy Markdown
Collaborator Author

Pushed f8bd4ec: a loading toast already waiting in the queue now moves behind each new toast, so a second toast arriving during the profile wait shows before the loader returns, instead of waiting until the wait ends. Answers the Greptile and Codex findings on ToastQueueManager; ToastQueueManagerTest.kt adds a two-interruption case.

@ovitrif

ovitrif commented Oct 16, 2026

Copy link
Copy Markdown
Collaborator Author

Pushed 080913c and 0410b34:

  • 080913c: while a saved session is not restored yet, the sign-in handler stays on only for a key Bitkit stores itself, never for a Ring-sourced identity, so a leftover local key cannot route links for a Ring identity. PubkyRepoTest.kt covers the check.
  • 0410b34: signin-cold-start.xml now delivers the link with am start, within the journey Capabilities; the network-cut case moved to Manual Tests, since device network toggling is not a journey capability.

@github-actions

github-actions Bot commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

Regtest APK

Built from d790f34 (run).

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

@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. Keeps the Pubky sign-in handler enabled while a saved session with a Bitkit-held key is still restoring, retries the restore for up to 60 s while the link waits, and shows a loading toast that survives interrupting toasts. Reviewed f8bd4ec, full tier.

What I checked, and 6 candidates I ruled out

Read: PubkyAuthHandlerRegistrar.kt, ToastQueueManager.kt, Toast.kt, the ToastView.kt hunk, and in AppViewModel.kt the scan scheduling job, awaitPubkyDeeplinkInitialization, the new wait-toast helpers, handlePubkyAuth and showPubkyIdentityUnavailableToast; in PubkyRepo.kt awaitIdentityReady, restoreSession, restoreSessionLocked, initializeSession, hasSecretKey, managedSecretKeyFor and the new hasLocalSecretKey.
CI: build, lint, detekt, build-local green; Run tests (testDevDebugUnitTest) green on this head; the e2e shards and e2e-status were still running when this was posted.
Not run here: the unit tests could not be run on this machine (the new ldk-node-android and bitkit-core-android artifacts were not in the offline Gradle cache), so CI's testDevDebugUnitTest is the evidence. The cold-start behaviour itself needs a device: the new journey and the manual network-cut test.

Ruled out

  • Early-enabled handler lets a Ring identity approve: hasLocalSecretKey() excludes SHARED_PUBKY_SOURCE, and handlePubkyAuth still requires a restored publicKey plus hasSecretKey() before Sheet.PubkyAuth opens.
  • Wait toast left on screen: hidePubkyAuthWaitToast() sits in the scan job's finally, so cancellation by a newer scan, the locked-app deferral and the timeout path all clear it; removal is by reference, so a toast that replaced it stays.
  • Wait toast evicted from a full queue: enqueue puts loading toasts after every other toast before takeLast(MAX_QUEUE_SIZE), so overflow drops the oldest non-loading toast first.
  • Retry loop spinning without the network: each Unavailable waits 3 s and then for ConnectivityState.CONNECTED, all inside the 60 s withTimeoutOrNull; Missing ends the loop and falls through to the no-identity toast.
  • Grant signup with no identity waits 60 s: awaitIdentityReady() returns Missing when hasIdentity() is false, so the loop exits at once and createsIdentity opens the sheet.
  • Contact links affected: they keep PubkyService.AUTHORIZATION_TIMEOUT, skip the wait toast and the retry loop, and keep their own error toast.

Merge confidence: 4/5, no findings and the change is unit-tested in CI, but the required e2e-status check had not finished and the cold-start path is proven on a device only by the journey and the manual test.

@ovitrif
ovitrif requested review from jvsena42 and piotr-iohk and removed request for ben-kaufman and coreyphillips October 16, 2026 11:34

@piotr-iohk piotr-iohk left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

QA review

Scope: Full review of the PR diff against its merge base, at f8bd4ecd.

No new actionable code findings.

A cold-start pubkyauth://signin_grant link keeps the auth handler enabled while a Bitkit-held secret key exists and the saved session is not restored yet, retries that restore for up to 60 seconds while the device is online, and shows a loading toast until the Authorize sheet opens or the wait fails. Approval still requires the restored identity's own key. A Ring-sourced identity does not keep the handler on. Contact links keep the previous 30-second timeout and their own error toast.

The earlier report that a second toast stays stuck behind the loader is fixed at this revision: a queued loading toast is placed behind each new toast (thread). Compared bitkit-ios eb3156ab: iOS does not register the raw pubkyauth scheme and opens sign-in through bitkit://, so this activity-alias failure has no iOS twin. During an offline wait, the existing "Your profile session has expired" toast can still appear; the loader is expected to return after it.

Validation: CI Run tests (testDevDebugUnitTest) passed on this revision. Those tests were not run locally. Device testing: not performed in this review.

Ready for device testing.

Suggested additional test cases

  • Android, dev build, Paykit on, Pubky profile created in Bitkit. Force-stop, open a fresh pubkyauth://signin_grant link, and keep the device offline for more than 60 seconds. Bitkit stays in the foreground, the loading toast is replaced by "Couldn't Load Your Pubky Profile", and a later sign-in link still opens Bitkit.
  • Android, same build with a Ring-sourced identity and no Bitkit-held key. Force-stop and open a pubkyauth://signin_grant link. Bitkit does not present the Authorize sheet for that identity.

@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 f8bd4ec: one LOW inline. I found no HIGH or MEDIUM.

I reviewed it as an auth and deep-link change.

Checked and clean:

  • The authorization gate is unchanged. handlePubkyAuth (AppViewModel.kt:7652-7665) still requires a restored publicKey and a matching managed secret key before Sheet.PubkyAuth. Enabling the alias early only routes the link.
  • Registrar (PubkyAuthHandlerRegistrar.kt:66,78-83): before restore it requires PUBKY_SECRET_KEY present and SHARED_PUBKY_SOURCE absent. adoptRingIdentity writes the source first. signOut, wipeLocalState and forgetUnrestoredIdentity go through clearLocalState. Keychain errors leave the alias disabled.
  • Lock screen: _isAuthenticated starts false, so on a cold start the link waits as mustComplete until unlock. The wait toast and the sheet never show over the PIN screen. A lock during the wait defers after it, and finally hides the toast.
  • Link replacement: a duplicate intent is skipped. A different link during a mustComplete wait goes to the single deferred slot and runs after the sheet closes. Approval uses the displayed authUrl, so there is no new TOCTOU.
  • Wait paths: newer scan, timeout, lock, return false and exceptions all pass through finally { hidePubkyAuthWaitToast() }, and dismiss matches by reference. The 60 s timeout shows the error toast and opens no sheet.
  • ToastQueueManager: the loader is always placed last and takeLast(5) drops the oldest non-loading toast first. Toast text comes from string resources only.
  • No persisted-state change, so no v2.5.1 angle.
  • l10n: two new keys (pubky_auth__loading_identity, _desc), English only per the Transifex flow.

Performance-relevant (user-facing wait raised from 30 s to 60 s, a restore retry loop, scanMutex held during the wait).

Device gate (partial): f8bd4ec — not run yet. Pending:

  • signin-cold-start.xml;
  • a 25 s and a >60 s network cut;
  • a Ring identity;
  • a payment QR during the wait, which must open only after the sign-in sheet closes;
  • a PIN lock during the wait;
  • time from am start to the Authorize sheet, and the count of restore retries.

The device result will follow.

Comment thread journeys/README.md Outdated
@ovitrif

ovitrif commented Oct 16, 2026

Copy link
Copy Markdown
Collaborator Author

Pushed d790f34: the cold-start sign-in journey now names its iOS counterpart, bitkit-ios pubky-auth/open-signin-link.xml, instead of "not ported", and lists the parts only Android has. Docs only (journeys/README.md, journeys/pubky-auth/README.md, signin-cold-start.xml). Answers the LOW finding in the review by @jvsena42.

@ovitrif
ovitrif requested a review from jvsena42 October 16, 2026 14:52

@piotr-iohk piotr-iohk left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

QA review

Scope: Follow-up review of the changes since f8bd4ecd, at d790f344, inheriting the completed baseline review. Inherited coverage is unrestored-session handler gating for a Bitkit-held key versus a Ring source, the cold-start online retry, 60-second timeout and profile-unavailable error, loading-toast show, reference dismiss and requeue, unchanged contact-link and non-grant signup behavior, and the unit tests plus the cold-start journey actions as the unexecuted runtime plan. This pass covered the documentation delta and the iOS journey it names.

No new actionable code findings.

The only change since the baseline is documentation. The cross-platform row, journeys/pubky-auth/README.md, and the signin-cold-start.xml description now point at bitkit-ios pubky-auth/open-signin-link.xml (bitkit://pubky-auth/signin) as an adapted counterpart, and list the raw pubkyauth scheme, the "Loading Your Pubky Profile" toast, and Authorize through PubkyAuthOK as Android-only. That matches the requested wording in the review thread. Compared with bitkit-ios afdc534b, where that journey file was already present at eb3156ab: iOS registers bitkit for this handoff, and open-signin-link.xml launches a terminated app with a local bitkit://pubky-auth/signin grant, shows the Authorize sheet, and cancels. It does not include the loading toast or PubkyAuthOK. Its description still says Android needs no twin. Android journey actions are unchanged.

Validation: no tests were executed in this review. The delta does not change app code. Unit-test evidence for the unchanged code remains the baseline run CI Run tests on f8bd4ec. Device testing: not performed in this review.

Ready for device testing.

@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 d790f34. My journey-parity LOW is fixed in d790f34 and confirmed on its thread. No HIGH or MEDIUM.

Device gate: d790f34 — on the CI APK with a Bitkit-held identity, against a requester built on @synonymdev/pubky 0.14.0 with a fresh signin_grant URL each run.

  • signin-cold-start.xml passes: loading toast with spinner until the sheet, the sheet shows gate.test with /pub/gate.test READ/WRITE, Authorize reaches PubkyAuthOK. The requester received {"status":"approved","pubky":"pubky3bx9…4fe9ujo","caps":["/pub/gate.test/:rw"]}.
  • 25 s airplane cut: Bitkit stays open and the loading toast returns after "Internet Connection Restored". One Retrying Pubky session restore line, and the sheet opens ~16 s after reconnect.
  • 60 s cut: "Couldn't Load Your Pubky Profile" at ~+62 s with no sheet and Bitkit on Home. A later link opens the sheet.

  • PIN on: the PIN screen shows first, and 9 dumps over 27 s show no toast and no sheet over it. The sheet opens within 3 s of unlock, and Authorize asks for the PIN.
  • Payment URI during the wait: it is queued (Queuing 'deeplink' scan for deferred handling) and opens Bitcoin Amount 1 000 sats only after the sign-in sheet is cancelled.
  • No crash or ANR.

perf: am start → sheet 15.6 s online; 39–43.5 s with a 15–25 s network cut; 0–1 retry lines per run (3 in the failed run below). l10n: two new English-only keys in the toast, no overflow.

⚠️ Not verified: a Ring-sourced identity. It needs Pubky Ring signed with Bitkit's debug key.

The first cold start right after profile creation reached the 60 s limit and showed "Couldn't Load Your Pubky Profile". A re-run passed. Details are in a separate comment, since the cause is outside this PR.

CI green at d790f34.

@jvsena42

jvsena42 commented Oct 16, 2026 •

Copy link
Copy Markdown
Member

Outside this PR's scope, no change needed here.

On a cold start, Paykit shared-state contention can outlast the 60 s sign-in wait. In 2 of 8 cold starts during the gate, the session restore was deferred with shared_state_busy until after the window. The first run, right after profile creation:

16:44:27.308 DEBUG [AppViewModel.kt:3087] Starting scan from 'deeplink'
16:44:32.718 DEBUG [PaykitSdkService.kt:405] Republished Pubky identity
16:44:35.260 WARN  [PubkyRepo.kt:531] Deferred session restoration, keeping saved session [SharedStateBusy='code=shared_state_busy, context=Pubky shared state remains locked; retry later']
16:44:42.087 WARN  [AppViewModel.kt:852] Failed to refresh public Paykit endpoints [SharedStateBusy='code=shared_state_busy, …']
16:44:59.611 DEBUG [AppViewModel.kt:3154] Retrying Pubky session restore for a sign-in link
16:45:20.187 WARN  [PubkyRepo.kt:531] Deferred session restoration, keeping saved session [AppError='code=shared_state_busy, …']
16:45:27.719 WARN  [AppViewModel.kt:3125] Failed to initialize Pubky deeplink
16:45:33.695 INFO  [PubkyRepo.kt:479] Restored paykit session for 'pubky3b…4fe9ujo'

The identity republish and the public endpoint refresh on startup contend with the restore for the SDK's shared-state lock. The restore succeeded 6 s after the wait expired. The same pattern appeared in a later PIN run, where the restore took 65 s. On iOS, a second device restored from the same seed also hit SharedStateBusy on Pay while the first device's app was running.

Reproduction: create a profile, then am force-stop and immediately am start a fresh pubkyauth://signin_grant link on staging. Repeat a few times and watch PubkyRepo.kt:531 lines. Likely repo: pubky/paykit-rs for the lock behaviour, or bitkit-android for deferring the startup republish and endpoint refresh while a sign-in link is waiting.

Two pre-existing deferral behaviours seen in the same runs:

  • A bitcoin: URI arriving while the PIN screen holds a deferred sign-in link replaces it (Replacing deferred scan from 'deeplink', 17:00:05.131), and the sign-in link is dropped.
  • A URI arriving while the Authorize sheet is open replaces the sheet with Send.

Tracked in #1541 (sign-in wait vs shared-state lock) and #1545 (a later link drops a pending sign-in).

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 from a link fails on the first try when bitkit cold-starts

4 participants