Skip to content

refactor (connectApp): replace status state machine with storage-as-source-of-truth - #118

Open
litury wants to merge 7 commits into
pshenmic:developfrom
litury:fix/reject-cache-retry
Open

litury wants to merge 7 commits into
pshenmic:developfrom
litury:fix/reject-cache-retry

Conversation

@litury

@litury litury commented Apr 19, 2026 •

Copy link
Copy Markdown
Collaborator

Closes #117

Problem

AppConnect stored status: pending | approved | rejected | error, used as a state machine between popup and content-script via storage. Side effects:

  • Rejection persists — subsequent signer.connect() calls return the cached rejected status without opening a popup (Rejected app connection status is cached permanently, preventing retry #117).
  • ExtensionSigner.connect() polled the storage status every 500ms for up to MESSAGING_TIMEOUT (3 min) — fragile and slow to react.
  • Storage conflated durable state (is this dApp approved) with transient flow state.

Solution

Treat presence of an AppConnect record as the single source of truth:

  • Record exists in appConnects_* → connection is approved.
  • Record absent → not connected.

Flow changes:

  • Popup Approve writes the AppConnect record directly via AppConnectRepository (storage is cross-context in MV3 extensions).
  • Popup Reject simply closes the popup — no storage write.
  • ConnectAppHandler in the content-script opens the popup, subscribes to chrome.storage.onChanged, and polls popup.closed. First signal wins: record appears → resolve with WalletInfo; popup closed without a record → reject with App connection was rejected.

Reject cache bug disappears by design: rejection never writes to storage, so the next connect() call starts a fresh flow.

Changes

Removed — state machine artifacts:

  • AppConnectStatus enum
  • ApproveAppConnectPayload, RejectAppConnectPayload
  • ApproveAppConnectHandler, RejectAppConnectHandler (popup writes directly now)
  • APPROVE_APP_CONNECT, REJECT_APP_CONNECT from MessagingMethods
  • approveAppConnect / rejectAppConnect from PrivateAPIClient

Simplified:

  • AppConnect, AppConnectStorageSchema, ConnectAppResponse — no status field
  • ExtensionSigner.connect() — single awaited call, no polling loop
  • AppConnectRepository.create() — idempotent, overwrites if already approved

Rewritten:

  • ConnectAppHandler.handle() — storage-event-driven resolution
  • AppConnectState.tsx — reads URL from useSearchParams(), writes record via repository on Approve, just closes on Reject

Added:

  • Migration 0010_drop_app_connect_status — strips status field from existing records, removes non-approved entries, bumps SCHEMA_VERSION to 10

Verification

  • npm run lint clean
  • npm test — 104 tests green, including new storage-event coverage
  • npm run build — successful
  • Manual testing in Chrome 130+:
    • Vote → Approve → icons appear, wallet connected
    • Vote → Reject or close popup → Vote button returns immediately (no 3-min timeout)
    • Re-Vote after Reject → fresh popup opens
    • Vote on already-connected site → icons appear without popup

Follow-up

During implementation we found that chrome.runtime.onMessage.dispatch — the undocumented method used across PrivateAPIClient / PrivateAPI — does not reliably cross contexts in Chrome 130+. This PR works around it by using storage-based signaling for the connect flow specifically (documented MV3 API). A separate issue will be opened proposing a refactor of the messaging layer to an MV3 service worker pattern.

@litury
litury force-pushed the fix/reject-cache-retry branch from f45e0de to 97dedd4 Compare April 19, 2026 04:39
@litury

litury commented Apr 19, 2026

Copy link
Copy Markdown
Collaborator Author

Follow-up: the initial fix in 8ad5faa was too aggressive — it reset terminal status on every connectApp call, including the 500ms polling loop inside ExtensionSigner.connect(). Result: after user clicks Reject in the popup, the next poll would rewrite rejected → pending, the loop kept running until MESSAGING_TIMEOUT (3 minutes), and the dApp spinner hung.

Added 2 commits (5be1470, 0416de3) that gate the reset behind an explicit reset: boolean flag in the payload:

  • ExtensionSigner.connect() passes reset: true on the initial call (start of a new connect session)
  • Polling calls inside the loop pass no flag (default false) — terminal status flows through unchanged, loop exits normally on rejected / error
  • ConnectAppHandler only does the removeById + recreate when payload.reset === true

Verified manually: Reject → spinner returns to Vote button immediately; Approve → proceeds as before.

Happy to squash into a single commit on merge if you prefer — kept them separate so the evolution is visible.

@litury
litury force-pushed the fix/reject-cache-retry branch from 0416de3 to 52b2ae6 Compare April 19, 2026 12:38
@litury

litury commented Apr 19, 2026

Copy link
Copy Markdown
Collaborator Author

Force-pushed with a storage-based approach. Previous commits replaced entirely. See updated PR description.

@litury litury changed the title fix (connectApp): reset terminal status records on retry refactor (connectApp): replace status state machine with storage-as-source-of-truth Apr 19, 2026
@litury

litury commented Apr 20, 2026

Copy link
Copy Markdown
Collaborator Author

Routed popup approve through PrivateAPI / ApproveAppConnectHandler instead of direct repository access. Also added silent skip for unregistered methods in PrivateAPI listener so popup-only handlers don't conflict with the content-script listener. Added test coverage for the handler.

litury added a commit that referenced this pull request Apr 20, 2026
Reject previously wrote AppConnect{status:'rejected'} to storage,
so the next connect() saw the cached rejection and never reopened
the popup — user was stuck until manual cleanup.

Record presence is now the source of truth: reject deletes the
record. ConnectAppHandler tracks open popup sessions in memory and
detects rejection when a session's record disappears, returning
status='rejected' once so the signer polling loop exits.

Preserves AppConnectStatus enum and signer polling (no protocol
change). Minimal alternative to #118.
pshenmic added a commit that referenced this pull request Apr 20, 2026
fix (appConnect): minimal patch for reject cache (alternative to #118)
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.

1 participant