Uh oh!
There was an error while loading. Please reload this page.
test(plugin-detail): pin record:alert's CTA honours resultDialog through the shared runner - #6039
Merged
yinlianghui merged 2 commits intoAug 24, 2026
Merged
Conversation
…the shared runner `record:alert`'s CTA resolves its action from object metadata and dispatches through `useActionEngine`, which reuses the surrounding ActionProvider's ActionRunner when one is mounted and falls back to a local, unwired one otherwise. `resultDialog` is honoured centrally in `ActionRunner.handlePostExecution`, whose toast gate keys on the key BEING SET (`!!(action.resultDialog && result.success)`), not on a handler existing — so a refactor that dropped the ambient provider would leave the CTA reporting success, the toast suppressed and the user shown nothing, traced only by a console.warn. Mirrors the sibling pin record-quick-actions.resultDialog.test.tsx, with both legs the triage fence names (provider-mounted, and the fallback-runner branch) plus the counter-probes that make each leg a measurement rather than an empty harness. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CSoz9uGhaaSgiq3hshtN7L
…ing nothing Empty frontmatter — this repo's explicit "releases nothing" declaration for a test-only change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CSoz9uGhaaSgiq3hshtN7L
Contributor
✅ Console Performance Budget
The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it. 📦 Bundle Size Report
Size Limits
|
yinlianghui
marked this pull request as ready for review
August 24, 2026 14:35
Uh oh!
There was an error while loading. Please reload this page.
yinlianghui
deleted the
claude/issue-5808-record-alert-resultdialog-pin
branch
August 24, 2026 14:47
This was referenced Aug 24, 2026
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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Fixes#5808
Test-only. Adds the missing pin the triage fence names:
record:alert's CTA honours anaction's
resultDialogthrough the sharedActionRunnerwhen an action provider ismounted — mirroring the sibling pin
record-quick-actions.resultDialog.test.tsx(#5711 /PR #5807), including the fallback-runner (unwired) branch.
New file:
packages/plugin-detail/src/renderers/__tests__/record-alert.resultDialog.test.tsx(+ an empty-frontmatter changeset — this repo's "releases nothing" declaration).
No source file is touched:
record-alert.tsx,useActionEngine.tsandActionRunner.tsare read-only in this PR.
The gap, re-derived on this branch's merge-base (not recalled)
The card is from 2026-08-23 and
packages/plugin-detailmoved twice since (#6000, andPR #6015 / #5998). Re-running the card's own grep on merge-base
594704f78:One hit, unchanged, and it is a comment — no assertion in either direction. Per-file
counts:
record-alert.test.tsx1,record-alert.rowBinding.test.tsx0,record-alert.visibleWhen.evidence.test.tsx0,record-alert.loadingFrameDiagnostic.test.tsx0. No pin appeared in the interim; the cardis still open on the merits.
The suppression claim, verified in running code
The card's load-bearing claim is that toast suppression keys on
resultDialogbeingset, not on a handler existing. Confirmed in
ActionRunner.handlePostExecution(
packages/core/src/actions/ActionRunner.ts:1135,:1142):hasResultDialogreads the action key and the result only —this.resultDialogHandlerisconsulted afterwards, at
:1182, purely to decide between opening the dialog andconsole.warn-ing. So the silent-success framing stands: with the key set and no handler,the action reports success, the toast stays suppressed, the value is discarded, and the
only trace is a warn. That branch is now pinned directly (spec 5 below).
What the suite pins
resultDialogon the same provider does toastconsole.warnfiresresultDialog, no provider, still executes and does not produce the runner's resultDialog warninghasResultDialoggate itselfOnly the DATA layer is doubled —
useMetadataItem, the CTA's metadata fetch — the samesurgical strategy
record-alert.test.tsxestablished under #3941.useActionEngine, theprovider,
ActionRunnerandRecordContextProviderare the real shipped ones; doublingany of them would replace the mechanism under test with a second copy of it.
On spec 4 and the counter-probe's toast half: the fence asks that an action without
resultDialogstill show its success toast on both legs. On the provider-less leg atoast is not observable at all — the fallback runner is constructed with no
toastHandlerinstalled, so there is no channel for one. That is a fact about running code, not a gap in
this file. Spec 4 therefore substitutes the strongest observable that leg has: the action
provably executed (an
onClickspy fired) while the resultDialog warning provably did not.Specs 2 and 5 carry the toast half on the legs where a toast can exist.
Failure on demand — three mutations, each restored under
trap … EXIT INT TERMVitest aliases
@object-ui/reactand@object-ui/coreto src (rootvitest.config.mtsresolve.alias), so these mutations take effect with no rebuild step. Each mutation'slanding site was printed, and the injected text and the removed text were grepped
separately.
const sharedRunner = providerCtx?.runner ?? nullbecomesnullpackages/react/src/hooks/useActionEngine.ts:70packages/react/src/hooks/useActionEngine.ts:103false &&on the toast gate)packages/core/src/actions/ActionRunner.ts:1142Mutation C is the one that justifies the counter-probes: it makes spec 1 ("toast
suppressed") pass in a world where no toast could ever appear, and only spec 2 notices.
git diff HEAD --statwas empty after each restore.Gates — exit codes captured before any pipe, at final HEAD
470c79ca9pnpm --filter @object-ui/plugin-detail type-check> @object-ui/plugin-detail@17.6.0 type-checkthentsc --noEmit && tsc -p tsconfig.test.json, so this is a real run, not a zero-match silent passpnpm exec vitest run packages/plugin-detail/src --maxWorkers=2(root form)Test Files 101 passed (101),Tests 943 passed (943)pnpm --filter @object-ui/plugin-detail lint(eslint ., no--no-inline-config)823 problems (0 errors, 823 warnings), all pre-existing; the new file contributes 2no-explicit-anywarnings, matching its sibling suitesnode scripts/check-changeset-presence.mjsnode scripts/check-changeset-no-major.mjsnode scripts/check-control-bytes.mjsMerge-base delta vs
594704f78: two files, both added —packages/plugin-detail/src/renderers/__tests__/record-alert.resultDialog.test.tsxand.changeset/wise-pandas-pin.md.Not widened (fence assumption 4)
useActionEnginehas exactly three non-test call sites:record-alert.tsx,record-quick-actions.tsx(both now pinned) andplugin-dashboard/src/DashboardRenderer.tsx:283. The dashboard one takes the samefallback, but it builds its action defs in-component from a four-key projection of
schema.header.actions({ name, type, target, label }, line 273), which cannot carryresultDialogat all — so it has no exposure to this defect class. Named here rather thancovered; triage can grade whether the same projection dropping
confirmText/paramsisworth its own card.
Generated by Claude Code
Generated by Claude Code