fix(APP-958): prevent app crash on wallet disconnect during transaction dialogs by harryburger · Pull Request #1215 · aragon/app

harryburger · 2026-07-09T04:34:57Z

Description

Fixes the application crashing when the wallet disconnects while a transaction dialog is open (APP-958).

Wallet-requiring dialogs (vote, execute, publish proposal, token approve, …) assert a connected address during render. Disconnecting the wallet from the wallet itself while one of them was open made the assertion throw during render, and since DialogRoot renders outside the page-level error boundary, the throw unmounted the entire application.

Changes:

Add a requiresWallet flag to IDialogComponentDefinitions and set it on the 15 dialogs carrying the connected-wallet assertion
DialogRoot unmounts flagged dialogs as soon as the wallet address is gone — before they can re-render and throw — and removes them from the dialog stack once the disconnect settles. Connector switches are unaffected: while reconnecting, dialogs are only hidden and restored once the address returns
Dialogs opened without a wallet (e.g. connect-wallet) are untouched, so the user can always reconnect
Add a guardrail test that fails when a dialog carries the connected-wallet assertion but its definition lacks the flag, so new dialogs cannot reintroduce the crash

Type of Change

Patch: Bug fix (non-breaking change which fixes an issue)

Developer Checklist:

Review Checklist:

(For User Stories only) Tested in a preview or locally that all Acceptance Criteria are satisfied
Confirmed that changes follow the code style guidelines of this project

linear-code · 2026-07-09T04:35:00Z

APP-958

github-actions · 2026-07-09T04:38:02Z

🚀 Preview Deployment: View Here

Copilot

Pull request overview

Prevents the app from crashing when a connected wallet disconnects while certain transaction-related dialogs are open, by introducing an explicit requiresWallet dialog definition flag and using it to unmount/close affected dialogs before they can re-render without an address.

Changes:

Add requiresWallet?: boolean to dialog definitions and use it in DialogRoot to hide/unmount and close wallet-dependent dialogs on disconnect (while not closing during reconnects).
Mark several existing dialogs across modules/plugins as requiresWallet: true.
Add DialogRoot tests covering connected/disconnected/reconnecting behavior and the “disconnect while open” crash scenario; add a changeset entry.

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
src/shared/components/dialogRoot/dialogRoot.tsx	Adds wallet-awareness to dialog rendering and closes wallet-dependent dialogs on disconnect to prevent render-time assertion crashes.
src/shared/components/dialogRoot/dialogRoot.test.tsx	Adds unit tests to verify wallet-dependent dialog behavior across disconnect/reconnect states and prevent regressions.
src/shared/components/dialogProvider/dialogProvider.api.ts	Extends dialog definition API with a `requiresWallet` flag.
src/plugins/tokenPlugin/constants/tokenPluginDialogsDefinitions.ts	Marks token plugin dialogs that require a connected wallet.
src/plugins/sppPlugin/constants/sppPluginDialogsDefinitions.ts	Marks SPP dialog(s) that require a connected wallet.
src/plugins/lockToVotePlugin/constants/lockToVotePluginDialogsDefinitions.ts	Marks lock/unlock dialog as requiring a connected wallet.
src/plugins/gaugeVoterPlugin/constants/gaugeVoterPluginDialogsDefinitions.ts	Marks lock/unlock dialog as requiring a connected wallet.
src/modules/settings/constants/settingsDialogDefinitions.ts	Marks plugin uninstallation preparation dialog as requiring a connected wallet.
src/modules/governance/constants/governanceDialogsDefinitions.ts	Marks publish/execute/vote dialogs as requiring a connected wallet.
src/modules/createDao/constants/createDaoDialogsDefinitions.ts	Marks publish/prepare dialogs as requiring a connected wallet.
src/modules/capitalFlow/constants/capitalFlowDialogsDefinitions.ts	Marks policy preparation/transaction dispatch dialogs as requiring a connected wallet.
.changeset/dialog-wallet-disconnect-crash.md	Adds a patch changeset describing the crash fix and the new `requiresWallet` behavior.
.agents/shared/metrics/hits.jsonl	Appends new guardrail-metrics log entries.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

github-actions · 2026-07-09T04:43:31Z

E2E results (preview)

Smoke


Base URL	https://app-next-83w7jauph-aragon-app.vercel.app
Suite	smoke
Playwright result	⚠️ completed with flaky tests
Summary	76 passed, 3 flaky, 1 skipped
HTML report	View report
GitHub job	✅ completed

View run #4539

…ring-transaction-dialog

thekidnamedkd

Really nice fix overall — the unmount-before-throw ordering is race-free (we re-render DialogRoot before the dialog child on the same wagmi store update), the integration test reproducing the actual invariant crash is great, and I checked the flag coverage: all 5 dialogs with render-time address invariants are flagged, and the unflagged address != null matches elsewhere are benign. One thing we should fix before merge (the wagmi state-model comment inline), the rest is non-blocking polish.

Non-blocking extras:

Test gaps we could close cheaply: reconnect-restore (address returns → dialog re-renders, close never called), multi-dialog stack (flagged top closes, unflagged below becomes visible), and open-while-already-disconnected.
Follow-up ticket material rather than this PR: DialogRoot still renders outside the page-level error boundary, so the flag only protects dialogs someone remembered to flag — an error boundary around the dialog layer would kill the whole crash class. Might also be a good rule-skill ("dialog asserts wallet ⇒ set requiresWallet").

milosh86

Hey @harryburger, it looks good!

What I mentioned today, I was thinking about wrapping ActiveDialogComponent in an ErrorBoundary which would catch all errors, log em, and close the dialog. After thinking more about it, I think it's better to start with your more focused approach and we can consider this more general approach if needed later.

One thing only regarding your approach. Since now one must know about requiresWallet flag when creating a new dialog, let's add something like this to AGENTS.md: "When creating a new dialog which uses TransactionDialog internally pass requiresWallet flag to dialog definition.

…f-wallet-disconnects-during-transaction-dialog # Conflicts: # .agents/shared/metrics/hits.jsonl

isReconnecting is only true for wagmi's 'reconnecting' status, which is the mount-time reconnectOnMount path. A mid-session connector switch goes through 'connecting', where the address can be undefined while isReconnecting is false, so wallet-requiring dialogs were closed instead of being waited out. Expose isConnecting from useWalletAccount and close only when neither transient state is active. Also reword the requiresWallet JSDoc and the changeset to describe the behavioural contract rather than an assertion.

…e components

…disconnect

…ring-transaction-dialog

…f-wallet-disconnects-during-transaction-dialog # Conflicts: # .agents/shared/metrics/hits.jsonl

…ction-dialog' of github.com:aragon/app into app-958-app-crashes-if-wallet-disconnects-during-transaction-dialog

milosh86

Looks good 👍

Copilot AI review requested due to automatic review settings July 9, 2026 04:34

harryburger requested a review from a team as a code owner July 9, 2026 04:34

harryburger temporarily deployed to preview July 9, 2026 04:35 — with GitHub Actions Inactive

Copilot started reviewing on behalf of harryburger July 9, 2026 04:35 View session

Copilot AI reviewed Jul 9, 2026

View reviewed changes

Comment thread src/shared/components/dialogRoot/dialogRoot.tsx Outdated

harryburger temporarily deployed to preview July 9, 2026 04:43 — with GitHub Actions Inactive

harryburger temporarily deployed to preview July 9, 2026 04:54 — with GitHub Actions Inactive

harryburger added 3 commits July 10, 2026 13:54

fix: prevent app crash on wallet disconnect during transaction dialogs

34ae903

fix: ensure dialog unmounts correctly when wallet address is missing

45f0db1

fix: drop guardrail test mention from changeset.

0ef07de

harryburger force-pushed the app-958-app-crashes-if-wallet-disconnects-during-transaction-dialog branch from 5e37589 to 0ef07de Compare July 10, 2026 08:18

harryburger temporarily deployed to preview July 10, 2026 08:18 — with GitHub Actions Inactive

harryburger changed the title ~~fix: prevent app crash on wallet disconnect during transaction dialogs~~ fix(APP-958): prevent app crash on wallet disconnect during transaction dialogs Jul 10, 2026

Merge branch 'main' into app-958-app-crashes-if-wallet-disconnects-du…

b39a6b2

…ring-transaction-dialog

harryburger temporarily deployed to preview July 12, 2026 05:28 — with GitHub Actions Inactive

thekidnamedkd reviewed Jul 14, 2026

View reviewed changes

Comment thread apps/app/src/shared/components/dialogRoot/dialogRoot.tsx Outdated

Comment thread apps/app/src/shared/components/dialogProvider/dialogProvider.api.ts

Comment thread .changeset/dialog-wallet-disconnect-crash.md Outdated

milosh86 reviewed Jul 14, 2026

View reviewed changes

harryburger added 3 commits July 14, 2026 19:18

Merge remote-tracking branch 'origin/main' into app-958-app-crashes-i…

fa01d06

…f-wallet-disconnects-during-transaction-dialog # Conflicts: # .agents/shared/metrics/hits.jsonl

fix: require wallet for specific dialogs in gauge voter and governanc…

0479c33

…e components

harryburger temporarily deployed to preview July 14, 2026 13:42 — with GitHub Actions Inactive

fix: require wallet for additional dialogs to prevent app crashes on …

1591ca3

…disconnect

harryburger had a problem deploying to preview July 14, 2026 13:51 — with GitHub Actions Error

Merge branch 'main' into app-958-app-crashes-if-wallet-disconnects-du…

c21af94

…ring-transaction-dialog

harryburger temporarily deployed to preview July 14, 2026 13:52 — with GitHub Actions Inactive

harryburger added 2 commits July 15, 2026 14:56

Merge remote-tracking branch 'origin/main' into app-958-app-crashes-i…

e1e11d2

…f-wallet-disconnects-during-transaction-dialog # Conflicts: # .agents/shared/metrics/hits.jsonl

Merge branch 'app-958-app-crashes-if-wallet-disconnects-during-transa…

4da1583

…ction-dialog' of github.com:aragon/app into app-958-app-crashes-if-wallet-disconnects-during-transaction-dialog

harryburger temporarily deployed to preview July 15, 2026 09:16 — with GitHub Actions Inactive

milosh86 approved these changes Jul 15, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(APP-958): prevent app crash on wallet disconnect during transaction dialogs#1215

fix(APP-958): prevent app crash on wallet disconnect during transaction dialogs#1215
harryburger wants to merge 11 commits into
mainfrom
app-958-app-crashes-if-wallet-disconnects-during-transaction-dialog

harryburger commented Jul 9, 2026 •

edited

Loading

Uh oh!

linear-code Bot commented Jul 9, 2026

Uh oh!

github-actions Bot commented Jul 9, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

github-actions Bot commented Jul 9, 2026 •

edited

Loading

Uh oh!

thekidnamedkd left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

milosh86 left a comment •

edited

Loading

Uh oh!

milosh86 left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

harryburger commented Jul 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Type of Change

Developer Checklist:

Review Checklist:

Uh oh!

linear-code Bot commented Jul 9, 2026

Uh oh!

github-actions Bot commented Jul 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

github-actions Bot commented Jul 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thekidnamedkd left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

milosh86 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

milosh86 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

harryburger commented Jul 9, 2026 •

edited

Loading

github-actions Bot commented Jul 9, 2026 •

edited

Loading

github-actions Bot commented Jul 9, 2026 •

edited

Loading

milosh86 left a comment •

edited

Loading