fix(mobile): bound pairing connection lifecycle - #5978
Conversation
Signed-off-by: StephGlansberg <StephGlansberg@users.noreply.github.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 68a30dd136
ℹ️ 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".
| const _pairingConnectionTimeout = Duration(seconds: 15); | ||
| const _pairingDisconnectTimeout = Duration(seconds: 1); | ||
|
|
||
| enum PairingStage { |
There was a problem hiding this comment.
Add API doc comments for the newly exported PairingStage, PairingStageLabel, PairingConnectionException, and PairingCanceledException declarations, as well as the new public PairingState.stage surface; these public additions currently violate the repository requirement that every new public API be documented.
AGENTS.md reference: AGENTS.md:L133-L136
Useful? React with 👍 / 👎.
themiguelamador
left a comment
themiguelamador
left a comment
There was a problem hiding this comment.
I found lifecycle gaps outside the socket callbacks that this change fences:
_processPayloadcontinues unawaited afterreset(). A validation completing later can authenticate the stale community, overwrite the idle/new-attempt state, and publish a completion on the wrong lifecycle.- The legacy pairing path has the same issue across both of its awaits because it does not capture an attempt generation.
pair()accepts another request while the first request is instoring, which can overlap imports and make either completion win.- The NIP-AB target publishes
complete(success: true)beforeauthenticateWithCommunitypersists the identity. If persistence fails, it then publishes failure too, so the source receives contradictory terminal acknowledgements.
I fixed all four issues in Complear:review/pr-5978-fix at efe45eaf7eee54b9cc796e75a64d431ff8e1114c. The fix carries the generation through payload processing and legacy pairing, checks it after each await, rejects requests during storage, and acknowledges success only after authentication/storage completes. It also adds a regression test that resets during in-flight validation and verifies there is no stale authentication, state overwrite, or protocol completion.
Verification:
- full Flutter suite: 1,426 tests passed
- focused pairing suite: 44 tests passed
- pairing provider regression suite after the final assertion: 18 tests passed
just mobile-check: formatting clean andflutter analyzereports no issues
2 participants
Summary
Bounds the mobile NIP-AB WebSocket connection and teardown lifecycle so pairing cannot remain on Connecting indefinitely. Adds typed, secret-free progress stages through WebSocket open, optional NIP-42 authentication, REQ subscription, and offer publication, while fencing canceled attempts from later retries.
Related issue
Fixes #4476.
Testing
flutter test test/features/pairing/pairing_socket_test.dart test/features/pairing/pairing_provider_test.dart test/features/pairing/pairing_page_test.dart(43 passed)flutter analyze(full mobile tree, no issues)flutter test(1,425 passed)The regression coverage includes a never-ready WebSocket, a close handshake that never completes, reset during authentication, stale callbacks after retry, disconnect during the offer delay, NIP-42 stage order, REQ/offer emission, and relay-path token redaction.