fix(ui): make alert confirmations reliable - #2072
Merged
Merged
Conversation
📄 Knowledge reviewDosu skipped reviewing this PR because your organization has used its
|
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (18)
📝 WalkthroughWalkthroughThis PR standardizes alert-dialog confirmation behavior, adds asynchronous confirmation actions, replaces boolean memory mutations with structured results, updates feedback and reconciliation handling, adds localized messages, and expands static and runtime test coverage. ChangesAlert confirmation contract
Memory command result migration
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant ConfirmationDialog
participant MemoryClient
participant MemoryRouteService
participant MemoryService
participant FeedbackController
User->>ConfirmationDialog: confirm destructive action
ConfirmationDialog->>MemoryClient: invoke memory command
MemoryClient->>MemoryRouteService: send command
MemoryRouteService->>MemoryService: execute mutation
MemoryService-->>MemoryRouteService: applied or rejected result
MemoryRouteService-->>MemoryClient: MemoryCommandResult
MemoryClient-->>FeedbackController: return command result
FeedbackController-->>ConfirmationDialog: show feedback or reconcile state
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/memory/routes.ts (1)
524-546: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winGate
deleteMemoryto DeepChat agents likearchiveUserMemory.
deleteMemoryaccepts any agent for whichcanManageClaimMemory(agentId)succeeds, which does not checkagentTypeormemoryEnabled. A DeepChat route dependency with this guard makes the mutation behavior consistent witharchive, or this should be removed from the DeepChat route behind a service-level gate.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/memory/routes.ts` around lines 524 - 546, Update the memoryDeleteRoute handler to apply the same DeepChat agent guard used by memoryArchiveRoute before calling memoryService.deleteMemory: check deps.getAgentType(input.agentId), return the existing rejected/unavailable response for non-DeepChat agents, and only perform deletion for eligible agents.
🧹 Nitpick comments (5)
test/renderer/components/SkillInstallTargetScope.test.ts (1)
343-358: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePrefer observable signals over the internal
conflictRequestshape.The assertions on
conflictRequest.statusread component internals. The observable behavior is already covered byinstalling, by the rendered conflict dialog, and by the emittedupdate:open. If you keep the internal assertions, a later rename of the state machine breaks this test without a behavior change. Consider asserting that the conflict dialog is not rendered and that the install stays pending instead.As per coding guidelines: "Keep committed tests lean and focused on project reliability, stability, and observable contracts; remove temporary checks that only test implementation internals before handoff."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/renderer/components/SkillInstallTargetScope.test.ts` around lines 343 - 358, Remove the internal conflictRequest.status assertions from this test and rely on observable behavior instead: retain the installing assertion for the pending state, assert the rendered conflict dialog’s visibility as appropriate, and keep the emitted update:open assertion after overwrite.resolve completes. Use the existing dialog selector or component reference rather than inspecting conflictRequest.Source: Coding guidelines
test/renderer/settings/useMemoryInlineFeedback.test.ts (1)
42-64: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMake the rejection-reason coverage exhaustive at compile time.
The reason list is typed as
MemoryCommandRejectionReason[]. If a new rejection reason joins the union, this test still compiles and still passes, so the new reason gets no reconciliation coverage. Key the expectation by a full record so TypeScript reports a missing reason.♻️ Proposed exhaustive expectation
- const reasons: MemoryCommandRejectionReason[] = [ - 'unavailable', - 'not-found', - 'invalid-state', - 'conflict', - 'stale', - 'anchored' - ] - - expect( - Object.fromEntries( - reasons.map((reason) => [reason, shouldReconcileMemoryCommandRejection(reason)]) - ) - ).toEqual({ - unavailable: false, - 'not-found': true, - 'invalid-state': true, - conflict: false, - stale: true, - anchored: false - }) + const expected: Record<MemoryCommandRejectionReason, boolean> = { + unavailable: false, + 'not-found': true, + 'invalid-state': true, + conflict: false, + stale: true, + anchored: false + } + + for (const reason of Object.keys(expected) as MemoryCommandRejectionReason[]) { + expect(shouldReconcileMemoryCommandRejection(reason)).toBe(expected[reason]) + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/renderer/settings/useMemoryInlineFeedback.test.ts` around lines 42 - 64, Update the expectation in the test around shouldReconcileMemoryCommandRejection to use a fully keyed Record<MemoryCommandRejectionReason, boolean> rather than deriving keys from the manually maintained reasons array. Ensure every union member is explicitly represented so adding a new rejection reason causes a compile-time failure until its reconciliation behavior is covered.src/renderer/settings/components/MemoryListView.vue (1)
308-321: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the duplicated confirmation request state machine. Three panels declare the same
idle | confirming | pendingunion, the sameshallowRefstate, the same open computed, and the same stale-request identity comparison. The guard is correct only becauseshallowRefdoes not wrap the request object in a proxy, so this rule belongs in one place.
src/renderer/settings/components/MemoryListView.vue#L308-L321: moveDeleteRequest,deleteRequest, anddeleteDialogOpeninto a shared helper undersrc/renderer/settings/lib/and consume it here.src/renderer/settings/components/MemoryDirectivesPanel.vue#L346-L350: replaceDirectiveDeleteRequestanddeleteRequestwith the shared helper, parameterized by the target type.src/renderer/settings/components/MemoryPersonaPanel.vue#L171-L175: replaceRollbackRequestandrollbackRequestwith the shared helper.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/renderer/settings/components/MemoryListView.vue` around lines 308 - 321, Extract the duplicated idle/confirming/pending request state, shallowRef storage, open computed, and stale-request identity logic into a shared helper under src/renderer/settings/lib/, parameterized by target type. Update src/renderer/settings/components/MemoryListView.vue lines 308-321 to consume it for deleteRequest/deleteDialogOpen, src/renderer/settings/components/MemoryDirectivesPanel.vue lines 346-350 to replace DirectiveDeleteRequest/deleteRequest, and src/renderer/settings/components/MemoryPersonaPanel.vue lines 171-175 to replace RollbackRequest/rollbackRequest; preserve shallowRef semantics and existing request behavior at all sites.test/renderer/components/MemoryDiagnosticsPanel.test.ts (1)
24-44: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover the async alert action with the real primitive in three test files. All three files stub
AlertDialogAsyncActionwith a plain button, so the click-before-close contract added by this PR is asserted only against component state.test/renderer/components/MemoryInlinePanel.test.tsandtest/renderer/components/MemoryListView.test.tsalready add arealAlertDialogmode.
test/renderer/components/MemoryDiagnosticsPanel.test.ts#L24-L44: add arealAlertDialogsetup option and one clear-all test that mounts the real alert-dialog components.test/renderer/components/MemoryDirectivesPanel.test.ts#L26-L35: add the same option and one real-primitive test for the directive delete flow.test/renderer/components/MemoryPersonaPanel.test.ts#L16-L25: add the same option and one real-primitive test for the rollback flow.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/renderer/components/MemoryDiagnosticsPanel.test.ts` around lines 24 - 44, Update the test setup in test/renderer/components/MemoryDiagnosticsPanel.test.ts (lines 24-44), test/renderer/components/MemoryDirectivesPanel.test.ts (lines 26-35), and test/renderer/components/MemoryPersonaPanel.test.ts (lines 16-25) to support a realAlertDialog option that uses the real AlertDialogAsyncAction primitive instead of the ButtonStub. Add one test in each file covering the clear-all, directive-delete, and persona-rollback flows respectively, mounting with realAlertDialog enabled and asserting the action does not close before the async operation completes.src/renderer/src/stores/ui/memoryActivity.ts (1)
634-663: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract duplicated restore-handling logic in
amend.The restore-outcome handling at lines 634-640 (normal failure path) and lines 646-652 (exception-recovery path) is identical: both check
restoreResult.action === 'applied', then apply the samemarkMemoryStatusInViewsandsetChipItemErrorcalls. Extract this into a single helper function to prevent the two paths from drifting apart on a future change.♻️ Proposed refactor to remove the duplication
+ function applyRestoreOutcome(memoryId: string, restoreResult: MemoryCommandResult): void { + if (restoreResult.action === 'applied') { + markMemoryStatusInViews(memoryId, 'pending_embedding') + setChipItemError(memoryId, 'amend_failed_retry') + } else { + setChipItemError(memoryId, 'amend_restore_failed') + } + } + async function amend(memoryId: string, content: string): Promise<MemoryAddResult | null> { ... const restoreResult = await memoryClient.restore(agentId, memoryId) - if (restoreResult.action === 'applied') { - markMemoryStatusInViews(memoryId, 'pending_embedding') - setChipItemError(memoryId, 'amend_failed_retry') - } else { - setChipItemError(memoryId, 'amend_restore_failed') - } + applyRestoreOutcome(memoryId, restoreResult) return null } catch (error) { console.warn('[MemoryActivity] failed to amend memory', error) if (archived) { try { const restoreResult = await memoryClient.restore(agentId, memoryId) - if (restoreResult.action === 'applied') { - markMemoryStatusInViews(memoryId, 'pending_embedding') - setChipItemError(memoryId, 'amend_failed_retry') - } else { - setChipItemError(memoryId, 'amend_restore_failed') - } + applyRestoreOutcome(memoryId, restoreResult) } catch (restoreError) {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/renderer/src/stores/ui/memoryActivity.ts` around lines 634 - 663, Extract the duplicated restore-result handling from amend into a single local helper, preserving the existing restoreResult.action === 'applied' branching and its markMemoryStatusInViews/setChipItemError calls. Invoke that helper in both the normal restore path and the exception-recovery path, leaving restore-error logging and handling unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/alert-dialog-contract-guard.mjs`:
- Around line 217-239: Update the alert-dialog contract guard’s binding
detection around CLICK_HANDLER and BOUND_CLICK_HANDLER to also identify dynamic
@[] and v-on:[] event bindings on AlertDialogAction and AlertDialogCancel,
rejecting them with the existing violation mechanism when the dynamic event
could resolve to click. Add a focused regression test covering a dynamic
event-name binding bypass.
In `@src/renderer/settings/components/MemoryInboxBar.vue`:
- Around line 422-423: Update the MemoryClient directive action to return the
full structured command result instead of only result.directive, then update
MemoryInboxBar to apply the existing rejection handling and localized feedback
to both directive actions, including not-found, invalid-state, and stale
reasons. Preserve capacity handling and reconciliation behavior consistently
across approveDirective and the other directive action.
In `@src/renderer/src/i18n/pl-PL/settings.json`:
- Line 263: Update the “anchored” translation in the Polish settings locale to
use the established “Odkotwicz” unanchor term, matching the existing unanchor
label while preserving the rest of the message.
In `@src/shadcn/components/ui/alert-dialog/index.ts`:
- Line 3: Update the AlertDialogAsyncAction export in the module’s index file to
use single quotes instead of double quotes, preserving the existing no-semicolon
formatting and export behavior.
---
Outside diff comments:
In `@src/main/memory/routes.ts`:
- Around line 524-546: Update the memoryDeleteRoute handler to apply the same
DeepChat agent guard used by memoryArchiveRoute before calling
memoryService.deleteMemory: check deps.getAgentType(input.agentId), return the
existing rejected/unavailable response for non-DeepChat agents, and only perform
deletion for eligible agents.
---
Nitpick comments:
In `@src/renderer/settings/components/MemoryListView.vue`:
- Around line 308-321: Extract the duplicated idle/confirming/pending request
state, shallowRef storage, open computed, and stale-request identity logic into
a shared helper under src/renderer/settings/lib/, parameterized by target type.
Update src/renderer/settings/components/MemoryListView.vue lines 308-321 to
consume it for deleteRequest/deleteDialogOpen,
src/renderer/settings/components/MemoryDirectivesPanel.vue lines 346-350 to
replace DirectiveDeleteRequest/deleteRequest, and
src/renderer/settings/components/MemoryPersonaPanel.vue lines 171-175 to replace
RollbackRequest/rollbackRequest; preserve shallowRef semantics and existing
request behavior at all sites.
In `@src/renderer/src/stores/ui/memoryActivity.ts`:
- Around line 634-663: Extract the duplicated restore-result handling from amend
into a single local helper, preserving the existing restoreResult.action ===
'applied' branching and its markMemoryStatusInViews/setChipItemError calls.
Invoke that helper in both the normal restore path and the exception-recovery
path, leaving restore-error logging and handling unchanged.
In `@test/renderer/components/MemoryDiagnosticsPanel.test.ts`:
- Around line 24-44: Update the test setup in
test/renderer/components/MemoryDiagnosticsPanel.test.ts (lines 24-44),
test/renderer/components/MemoryDirectivesPanel.test.ts (lines 26-35), and
test/renderer/components/MemoryPersonaPanel.test.ts (lines 16-25) to support a
realAlertDialog option that uses the real AlertDialogAsyncAction primitive
instead of the ButtonStub. Add one test in each file covering the clear-all,
directive-delete, and persona-rollback flows respectively, mounting with
realAlertDialog enabled and asserting the action does not close before the async
operation completes.
In `@test/renderer/components/SkillInstallTargetScope.test.ts`:
- Around line 343-358: Remove the internal conflictRequest.status assertions
from this test and rely on observable behavior instead: retain the installing
assertion for the pending state, assert the rendered conflict dialog’s
visibility as appropriate, and keep the emitted update:open assertion after
overwrite.resolve completes. Use the existing dialog selector or component
reference rather than inspecting conflictRequest.
In `@test/renderer/settings/useMemoryInlineFeedback.test.ts`:
- Around line 42-64: Update the expectation in the test around
shouldReconcileMemoryCommandRejection to use a fully keyed
Record<MemoryCommandRejectionReason, boolean> rather than deriving keys from the
manually maintained reasons array. Ensure every union member is explicitly
represented so adding a new rejection reason causes a compile-time failure until
its reconciliation behavior is covered.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: b24b8480-edd2-4ac6-afa1-72d89961920b
📒 Files selected for processing (87)
docs/architecture/alert-dialog-confirmation-contract/plan.mddocs/architecture/alert-dialog-confirmation-contract/spec.mddocs/architecture/alert-dialog-confirmation-contract/tasks.mdpackage.jsonscripts/alert-dialog-contract-guard.mjssrc/main/memory/domain/commandResult.tssrc/main/memory/index.tssrc/main/memory/routes.tssrc/main/memory/services/conflictService.tssrc/main/memory/services/directiveService.tssrc/main/memory/services/managementService.tssrc/main/memory/services/personaService.tssrc/main/tool/agentTools/agentMemoryTools.tssrc/main/tool/runtimePorts.tssrc/renderer/api/MemoryClient.tssrc/renderer/settings/components/BuiltinKnowledgeSettings.vuesrc/renderer/settings/components/DataSettings.vuesrc/renderer/settings/components/MemoryDiagnosticsPanel.vuesrc/renderer/settings/components/MemoryDirectivesPanel.vuesrc/renderer/settings/components/MemoryInboxBar.vuesrc/renderer/settings/components/MemoryInlinePanel.vuesrc/renderer/settings/components/MemoryListView.vuesrc/renderer/settings/components/MemoryPersonaPanel.vuesrc/renderer/settings/components/OcrSettings.vuesrc/renderer/settings/components/ProviderRateLimitConfig.vuesrc/renderer/settings/components/skills/SkillInstallDialog.vuesrc/renderer/settings/lib/useKnowledgeConfigOperation.tssrc/renderer/settings/lib/useMemoryInlineFeedback.tssrc/renderer/src/features/chat-page/ChatPage.vuesrc/renderer/src/i18n/da-DK/settings.jsonsrc/renderer/src/i18n/de-DE/settings.jsonsrc/renderer/src/i18n/en-US/settings.jsonsrc/renderer/src/i18n/es-ES/settings.jsonsrc/renderer/src/i18n/fa-IR/settings.jsonsrc/renderer/src/i18n/fr-FR/settings.jsonsrc/renderer/src/i18n/he-IL/settings.jsonsrc/renderer/src/i18n/id-ID/settings.jsonsrc/renderer/src/i18n/it-IT/settings.jsonsrc/renderer/src/i18n/ja-JP/settings.jsonsrc/renderer/src/i18n/ko-KR/settings.jsonsrc/renderer/src/i18n/ms-MY/settings.jsonsrc/renderer/src/i18n/pl-PL/settings.jsonsrc/renderer/src/i18n/pt-BR/settings.jsonsrc/renderer/src/i18n/ru-RU/settings.jsonsrc/renderer/src/i18n/tr-TR/settings.jsonsrc/renderer/src/i18n/vi-VN/settings.jsonsrc/renderer/src/i18n/zh-CN/settings.jsonsrc/renderer/src/i18n/zh-HK/settings.jsonsrc/renderer/src/i18n/zh-TW/settings.jsonsrc/renderer/src/stores/ui/memoryActivity.tssrc/shadcn/components/ui/alert-dialog/AlertDialogAction.vuesrc/shadcn/components/ui/alert-dialog/AlertDialogAsyncAction.vuesrc/shadcn/components/ui/alert-dialog/AlertDialogCancel.vuesrc/shadcn/components/ui/alert-dialog/index.tssrc/shared/contracts/routes/memory.routes.tstest/main/memory/conflictService.test.tstest/main/memory/directiveService.test.tstest/main/memory/embeddingPipeline.test.tstest/main/memory/lifecycleRegression.test.tstest/main/memory/maintenanceService.test.tstest/main/memory/managementService.test.tstest/main/memory/memory-persona-eval.test.tstest/main/memory/memoryAdd.test.tstest/main/memory/memoryBehavior.eval.test.tstest/main/memory/memoryUpdate.test.tstest/main/memory/personaService.test.tstest/main/memory/workingMemoryService.test.tstest/main/routes/dispatcher.test.tstest/main/routes/memoryDto.test.tstest/main/scripts/alertDialogContractGuard.test.tstest/main/tool/agentTools/agentMemoryTools.test.tstest/main/tool/agentTools/agentToolDependencies.tstest/renderer/api/clients.test.tstest/renderer/components/AlertDialogContract.test.tstest/renderer/components/BuiltinKnowledgeSettings.test.tstest/renderer/components/DataSettings.test.tstest/renderer/components/MemoryDiagnosticsPanel.test.tstest/renderer/components/MemoryDirectivesPanel.test.tstest/renderer/components/MemoryInboxBar.test.tstest/renderer/components/MemoryInlinePanel.test.tstest/renderer/components/MemoryListView.test.tstest/renderer/components/MemoryPersonaPanel.test.tstest/renderer/components/OcrSettings.test.tstest/renderer/components/ProviderRateLimitConfig.test.tstest/renderer/components/SkillInstallTargetScope.test.tstest/renderer/settings/useMemoryInlineFeedback.test.tstest/renderer/stores/memoryActivityStore.test.ts
Comment on lines
+422
to
+423
| shouldReload = | ||
| result.reason !== 'capacity' && shouldReconcileMemoryCommandRejection(result.reason) |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Propagate structured rejection results to directive rejection.
This branch handles structured rejection only for approveDirective. src/renderer/api/MemoryClient.ts Lines 321-327 still returns result.directive, and src/renderer/settings/components/MemoryInboxBar.vue Lines 451-455 treats null as generic failure. A not-found, invalid-state, or stale rejection therefore loses localized feedback and skips reconciliation. Return the full command result and apply the same rejection handling to both directive actions.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/renderer/settings/components/MemoryInboxBar.vue` around lines 422 - 423,
Update the MemoryClient directive action to return the full structured command
result instead of only result.directive, then update MemoryInboxBar to apply the
existing rejection handling and localized feedback to both directive actions,
including not-found, invalid-state, and stale reasons. Preserve capacity
handling and reconciliation behavior consistently across approveDirective and
the other directive action.
| @@ -1,5 +1,6 @@ | |||
| export { default as AlertDialog } from "./AlertDialog.vue" | |||
| export { default as AlertDialogAction } from "./AlertDialogAction.vue" | |||
| export { default as AlertDialogAsyncAction } from "./AlertDialogAsyncAction.vue" | |||
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use Oxfmt single quotes.
Line 3 uses double quotes. Format this export with single quotes.
As per coding guidelines, **/*.{ts,tsx,vue} must use “single quotes, no semicolons, and a 100-column width.”
Proposed fix
-export { default as AlertDialogAsyncAction } from "./AlertDialogAsyncAction.vue"
+export { default as AlertDialogAsyncAction } from './AlertDialogAsyncAction.vue'📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| export { default as AlertDialogAsyncAction } from "./AlertDialogAsyncAction.vue" | |
| export { default as AlertDialogAsyncAction } from './AlertDialogAsyncAction.vue' |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/shadcn/components/ui/alert-dialog/index.ts` at line 3, Update the
AlertDialogAsyncAction export in the module’s index file to use single quotes
instead of double quotes, preserving the existing no-semicolon formatting and
export behavior.
Source: Coding guidelines
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
AlertDialogActionandAlertDialogCancelwrappers.AlertDialogAsyncActionfor operations that must retain pending, failure, and retry state.applied | rejectedresults and reason-specific feedback.Root Cause
Reka's
DialogClosebubbling handler ran before consumer@clickhandlers. Controlled dialogs therefore emittedupdate:open=falsefirst, allowing open-change handlers to clear the confirmation target before the business operation ran.Async operations had a related ownership problem: the dialog could close before pending or failure state settled, hiding errors and removing retry context.
UI Behavior
BEFORE
AFTER
Summary by CodeRabbit
New Features
Documentation
Quality Improvements