From 947df6d082c071fc2aadeaa1eb683ce505e25c4c Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Tue, 4 Aug 2026 19:12:47 -0700 Subject: [PATCH 1/3] improvement(emcn): let a modal refuse every dismissal while an action runs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ChipConfirmModal's docs promised "a single dismiss path shared by the header X / dismiss button / Escape … and disabling dismiss while the confirm is in flight". Only the dismiss button was ever guarded — Escape, outside-click and the header X all still closed a confirmation mid-delete. Two knowledge-base connector modals had the same shape: they guarded onOpenChange against a pending save, then handed the header X a direct onOpenChange(false) that skipped the guard. A modal now states the interlock once, as `dismissDisabled` on ChipModal or ModalContent, and the primitive holds all four exits shut. ModalContent owns the Radix paths because `{...props}` is spread after its own handlers, so a consumer-passed onEscapeKeyDown/onInteractOutside would silently drop the floating-layer guard; it publishes the flag through a context that ChipModalHeader, ChipModalFooter and ModalHeader read. The two narrow props compose with `||`, so an explicit `true` still disables a single button and an explicit `false` cannot punch a hole in the root's guarantee. Also turns on `turbo run type-check` for every workspace. packages/emcn, packages/utils, apps/desktop and apps/docs had no type check in CI at all — only @sim/realtime did — and apps/sim's source was covered solely as a side effect of `next build`. All 23 workspaces pass today, so it lands green. --- .github/workflows/test-build.yml | 10 +- .../add-connector-modal.tsx | 4 +- .../edit-connector-modal.tsx | 4 +- .../components/chip-modal/chip-modal.test.tsx | 187 ++++++++++++++++++ .../src/components/chip-modal/chip-modal.tsx | 76 ++++--- packages/emcn/src/components/index.ts | 1 + packages/emcn/src/components/modal/modal.tsx | 82 +++++--- turbo.json | 11 +- 8 files changed, 322 insertions(+), 53 deletions(-) create mode 100644 packages/emcn/src/components/chip-modal/chip-modal.test.tsx diff --git a/.github/workflows/test-build.yml b/.github/workflows/test-build.yml index 5d971ab15fc..ac98f41f62a 100644 --- a/.github/workflows/test-build.yml +++ b/.github/workflows/test-build.yml @@ -178,8 +178,14 @@ jobs: fi bun run check:migrations "$BASE_REF" - - name: Type-check realtime server - run: bunx turbo run type-check --filter=@sim/realtime + # Every workspace, not just realtime. packages/emcn, packages/utils, + # apps/desktop and apps/docs had no type check in CI at all; apps/sim's + # source was covered only as a side effect of `next build` in the separate + # Build App job. Note this does NOT cover apps/sim's tests — its tsconfig + # excludes *.test.ts(x), and including them today surfaces ~2.2k errors, + # so that is its own cleanup rather than a gate to switch on here. + - name: Type-check all workspaces + run: bunx turbo run type-check # cloud-review-tools.test.ts runs the real helper on the runner, which shells # out to rg. Blacksmith's image ships it, GitHub's doesn't. diff --git a/apps/sim/app/workspace/[workspaceId]/knowledge/[id]/components/add-connector-modal/add-connector-modal.tsx b/apps/sim/app/workspace/[workspaceId]/knowledge/[id]/components/add-connector-modal/add-connector-modal.tsx index 529ac4567aa..705746d348f 100644 --- a/apps/sim/app/workspace/[workspaceId]/knowledge/[id]/components/add-connector-modal/add-connector-modal.tsx +++ b/apps/sim/app/workspace/[workspaceId]/knowledge/[id]/components/add-connector-modal/add-connector-modal.tsx @@ -235,9 +235,10 @@ export function AddConnectorModal({ <> !isCreating && onOpenChange(val)} + onOpenChange={onOpenChange} srTitle={step === 'select-type' ? 'Connect Source' : `Configure ${connectorConfig?.name}`} size='md' + dismissDisabled={isCreating} > onOpenChange(false)}> {step === 'configure' ? ( @@ -428,7 +429,6 @@ export function AddConnectorModal({ {step === 'configure' && ( onOpenChange(false)} - cancelDisabled={isCreating} primaryAction={{ label: isCreating ? 'Connecting…' : 'Connect & Sync', onClick: handleSubmit, diff --git a/apps/sim/app/workspace/[workspaceId]/knowledge/[id]/components/edit-connector-modal/edit-connector-modal.tsx b/apps/sim/app/workspace/[workspaceId]/knowledge/[id]/components/edit-connector-modal/edit-connector-modal.tsx index 5efe253f401..2bdf1e118c9 100644 --- a/apps/sim/app/workspace/[workspaceId]/knowledge/[id]/components/edit-connector-modal/edit-connector-modal.tsx +++ b/apps/sim/app/workspace/[workspaceId]/knowledge/[id]/components/edit-connector-modal/edit-connector-modal.tsx @@ -269,9 +269,10 @@ export function EditConnectorModal({ return ( !isSaving && onOpenChange(val)} + onOpenChange={onOpenChange} srTitle={`Edit ${displayName}`} size='md' + dismissDisabled={isSaving} > onOpenChange(false)}> Edit {displayName} @@ -312,7 +313,6 @@ export function EditConnectorModal({ {activeTab === 'settings' && ( onOpenChange(false)} - cancelDisabled={isSaving} primaryAction={{ label: isSaving ? 'Saving…' : 'Save', onClick: handleSave, diff --git a/packages/emcn/src/components/chip-modal/chip-modal.test.tsx b/packages/emcn/src/components/chip-modal/chip-modal.test.tsx new file mode 100644 index 00000000000..21fa89a9698 --- /dev/null +++ b/packages/emcn/src/components/chip-modal/chip-modal.test.tsx @@ -0,0 +1,187 @@ +/** + * @vitest-environment jsdom + */ +import { act, type ReactNode } from 'react' +import { createRoot, type Root } from 'react-dom/client' +import { afterEach, describe, expect, it, vi } from 'vitest' +import { ChipConfirmModal, ChipModal, ChipModalFooter, ChipModalHeader } from './chip-modal' + +vi.mock('next/navigation', () => ({ + usePathname: () => '/workspace/workspace-1/home', +})) + +let root: Root | null = null +let container: HTMLDivElement | null = null + +function mount(ui: ReactNode) { + ;(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true + container = document.createElement('div') + document.body.appendChild(container) + root = createRoot(container) + act(() => root?.render(ui)) +} + +afterEach(() => { + if (root) act(() => root?.unmount()) + container?.remove() + root = null + container = null +}) + +/** The dialog panel Radix renders, which owns the Escape/outside-click handlers. */ +function dialog(): HTMLElement { + const node = document.querySelector('[role="dialog"]') + if (!node) throw new Error('Dialog did not render') + return node +} + +function buttonByText(text: string): HTMLButtonElement { + const match = Array.from(document.querySelectorAll('button')).find((button) => + button.textContent?.includes(text) + ) + if (!match) throw new Error(`No button containing "${text}"`) + return match as HTMLButtonElement +} + +function closeButton(): HTMLButtonElement { + const match = Array.from(document.querySelectorAll('button')).find((button) => + button.querySelector('.sr-only')?.textContent?.includes('Close') + ) + if (!match) throw new Error('Close button did not render') + return match as HTMLButtonElement +} + +function pressEscape() { + act(() => { + dialog().dispatchEvent( + new KeyboardEvent('keydown', { key: 'Escape', bubbles: true, cancelable: true }) + ) + }) +} + +function Harness({ + onOpenChange, + dismissDisabled, +}: { + onOpenChange: (open: boolean) => void + dismissDisabled?: boolean +}) { + return ( + + onOpenChange(false)}>Title + onOpenChange(false)} + primaryAction={{ label: 'Save', onClick: () => {} }} + /> + + ) +} + +describe('ChipModal dismissDisabled', () => { + it('closes through every path when not set', () => { + const onOpenChange = vi.fn() + mount() + + expect(closeButton().disabled).toBe(false) + expect(buttonByText('Cancel').disabled).toBe(false) + + pressEscape() + expect(onOpenChange).toHaveBeenCalledWith(false) + }) + + // Outside-click is guarded by the same flag but jsdom cannot drive Radix's + // outside-interaction path, so asserting it here could never fail. + it('blocks the close button, Cancel and Escape when set', () => { + const onOpenChange = vi.fn() + mount() + + expect(closeButton().disabled).toBe(true) + expect(buttonByText('Cancel').disabled).toBe(true) + + pressEscape() + expect(onOpenChange).not.toHaveBeenCalled() + }) + + // Either flag disables: an explicit `false` must not re-enable a button whose + // click Radix has already been told to ignore. + it('cannot be re-enabled by an explicit closeDisabled or cancelDisabled of false', () => { + const onOpenChange = vi.fn() + mount( + + onOpenChange(false)} closeDisabled={false}> + Title + + onOpenChange(false)} + cancelDisabled={false} + primaryAction={{ label: 'Save', onClick: () => {} }} + /> + + ) + + expect(closeButton().disabled).toBe(true) + expect(buttonByText('Cancel').disabled).toBe(true) + }) + + it('still lets an explicit true disable a button on its own', () => { + const onOpenChange = vi.fn() + mount( + + onOpenChange(false)} closeDisabled> + Title + + onOpenChange(false)} + primaryAction={{ label: 'Save', onClick: () => {} }} + /> + + ) + + expect(closeButton().disabled).toBe(true) + expect(buttonByText('Cancel').disabled).toBe(false) + }) +}) + +describe('ChipConfirmModal pending', () => { + it('holds every exit shut while the confirm runs', () => { + const onOpenChange = vi.fn() + mount( + {}, pending: true, pendingLabel: 'Deleting...' }} + /> + ) + + expect(closeButton().disabled).toBe(true) + expect(buttonByText('Cancel').disabled).toBe(true) + expect(buttonByText('Deleting...').disabled).toBe(true) + + pressEscape() + expect(onOpenChange).not.toHaveBeenCalled() + }) + + it('dismisses normally when the confirm is idle', () => { + const onOpenChange = vi.fn() + mount( + {} }} + /> + ) + + expect(closeButton().disabled).toBe(false) + act(() => closeButton().click()) + expect(onOpenChange).toHaveBeenCalledWith(false) + }) +}) diff --git a/packages/emcn/src/components/chip-modal/chip-modal.tsx b/packages/emcn/src/components/chip-modal/chip-modal.tsx index 85256fc2d46..85418066f65 100644 --- a/packages/emcn/src/components/chip-modal/chip-modal.tsx +++ b/packages/emcn/src/components/chip-modal/chip-modal.tsx @@ -51,7 +51,7 @@ import { ChipInput } from '../chip-input/chip-input' import { ChipSwitch } from '../chip-switch/chip-switch' import { ChipTextarea } from '../chip-textarea/chip-textarea' import { Label } from '../label/label' -import { Modal, ModalContent } from '../modal/modal' +import { Modal, ModalContent, useModalDismissDisabled } from '../modal/modal' import { Tooltip } from '../tooltip/tooltip' /** @@ -108,6 +108,14 @@ export interface ChipModalProps { size?: 'sm' | 'md' | 'lg' | 'xl' | 'full' /** Optional className forwarded to the outer panel ring. */ className?: string + /** + * Refuses every exit while an action is in flight — Escape, outside-click, + * the header close button, and the footer Cancel. Stating it once here is the + * point: disabling only the buttons leaves Escape and outside-click open, + * which reads as handled without being handled. + * @default false + */ + dismissDisabled?: boolean children?: React.ReactNode } @@ -124,13 +132,20 @@ function ChipModal({ srTitle = 'Dialog', size = 'md', className, + dismissDisabled = false, children, }: ChipModalProps) { const submitRef = React.useRef(null) return ( - +
| null /** Invoked when the trailing close button is activated. Always rendered. */ onClose: () => void - /** Disables the trailing close button while an operation is in flight. */ + /** + * Disables the trailing close button. Combines with + * {@link ChipModalProps.dismissDisabled}, which also blocks Escape and + * outside-click — prefer that for an in-flight operation. + */ closeDisabled?: boolean /** Accessible label for the close button. */ closeAriaLabel?: string @@ -171,32 +190,35 @@ const ChipModalHeader = React.forwardRef( children, icon: Icon = null, onClose, - closeDisabled = false, + closeDisabled, closeAriaLabel = 'Close', ...props }, ref - ) => ( -
-
-
- {Icon ? : null} - {children} + ) => { + const dismissDisabled = useModalDismissDisabled() + return ( +
+
+
+ {Icon ? : null} + {children} +
+
- +
- -
- ) + ) + } ) ChipModalHeader.displayName = 'ChipModalHeader' @@ -924,6 +946,10 @@ export interface ChipModalFooterProps { * Disables the Cancel button. Set this while a primary/secondary action is * in flight (e.g. an async delete or save) so the user cannot dismiss the * modal and assume the operation was aborted while the mutation keeps running. + * + * This covers the Cancel button only. For an in-flight operation reach for + * {@link ChipModalProps.dismissDisabled} instead, which also blocks Escape, + * outside-click and the header's X. * @default false */ cancelDisabled?: boolean @@ -1025,6 +1051,7 @@ function ChipModalFooter({ primaryAdjacentAction, secondaryActions, }: ChipModalFooterProps) { + const dismissDisabled = useModalDismissDisabled() const showsDisabledTooltip = Boolean(primaryAction.disabled && primaryAction.disabledTooltip) /** @@ -1079,7 +1106,7 @@ function ChipModalFooter({ } > {hideCancel ? null : ( - + Cancel )} @@ -1303,6 +1330,7 @@ function ChipConfirmModal({ onOpenChange={onOpenChange} size={size} srTitle={srTitle ?? (typeof title === 'string' ? title : 'Confirm')} + dismissDisabled={confirm.pending} > {title} diff --git a/packages/emcn/src/components/index.ts b/packages/emcn/src/components/index.ts index 44bd954aa2f..681d3c628e4 100644 --- a/packages/emcn/src/components/index.ts +++ b/packages/emcn/src/components/index.ts @@ -151,6 +151,7 @@ export { ModalTrigger, NATIVE_SURFACE_OCCLUSION_PREPARE_EVENT, type NativeSurfaceOcclusionPrepareDetail, + useModalDismissDisabled, useNativeSurfaceOcclusionReady, } from './modal/modal' export { diff --git a/packages/emcn/src/components/modal/modal.tsx b/packages/emcn/src/components/modal/modal.tsx index fc9903a235b..60a1608a245 100644 --- a/packages/emcn/src/components/modal/modal.tsx +++ b/packages/emcn/src/components/modal/modal.tsx @@ -275,6 +275,21 @@ function ModalBodyLockReleaser() { */ const InsideModalContext = React.createContext(false) +/** + * Broadcasts {@link ModalContentProps.dismissDisabled} to every dismiss control + * in the subtree, so a modal states the interlock once on the content rather + * than each button remembering to disable itself. + */ +const ModalDismissDisabledContext = React.createContext(false) + +/** + * Whether an enclosing modal is currently refusing dismissal. Dismiss controls + * (a close X, a Cancel) should disable themselves when this is `true`. + */ +export function useModalDismissDisabled(): boolean { + return React.useContext(ModalDismissDisabledContext) +} + /** * Root modal component. Manages open state. */ @@ -449,6 +464,18 @@ export interface ModalContentProps * can fall into states where the dialog can't be re-opened cleanly. */ srTitle?: string + /** + * Refuses every dismissal while an action is in flight: the Escape key, + * clicking outside, and `ModalHeader`'s close button. Descendants read it via + * {@link useModalDismissDisabled}. + * + * The Radix paths are handled here rather than through a consumer-passed + * `onEscapeKeyDown` / `onInteractOutside` because `{...props}` is spread after + * this component's own handlers, so a consumer overriding either would + * silently drop the floating-layer guard below. + * @default false + */ + dismissDisabled?: boolean } /** @@ -467,6 +494,7 @@ const ModalContent = React.forwardRef< size = 'md', bare = false, srTitle, + dismissDisabled = false, style, onOpenAutoFocus, 'aria-describedby': ariaDescribedBy, @@ -570,6 +598,8 @@ const ModalContent = React.forwardRef< visibility: nativeSurfaceReady ? style?.visibility : 'hidden', }} onEscapeKeyDown={(e) => { + // Radix reads `defaultPrevented`; stopPropagation alone would not block it. + if (dismissDisabled) e.preventDefault() e.stopPropagation() }} onPointerDown={(e) => { @@ -593,7 +623,7 @@ const ModalContent = React.forwardRef< * are merely animating closed, so a follow-up click during the * exit animation still dismisses the modal. */ - if (hasOpenFloatingLayer()) { + if (dismissDisabled || hasOpenFloatingLayer()) { e.preventDefault() } }} @@ -613,7 +643,11 @@ const ModalContent = React.forwardRef< {srTitle ? ( {srTitle} ) : null} - {children} + + + {children} + +
@@ -627,26 +661,30 @@ ModalContent.displayName = 'ModalContent' * Modal header component for title and description. */ const ModalHeader = React.forwardRef>( - ({ className, children, ...props }, ref) => ( -
- - {children} - - - - -
- ) + ({ className, children, ...props }, ref) => { + const dismissDisabled = useModalDismissDisabled() + return ( +
+ + {children} + + + + +
+ ) + } ) ModalHeader.displayName = 'ModalHeader' diff --git a/turbo.json b/turbo.json index bdb8730df0f..957b97fd0ab 100644 --- a/turbo.json +++ b/turbo.json @@ -38,7 +38,16 @@ }, "type-check": { "dependsOn": ["transit"], - "outputs": [] + "outputs": [], + "inputs": [ + "**/*.ts", + "**/*.tsx", + "**/*.mts", + "**/*.cts", + "**/*.mdx", + "tsconfig*.json", + "package.json" + ] } } } From 03f6449b3a90a514e9f22a7b42833815945ffa16 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Tue, 4 Aug 2026 19:19:28 -0700 Subject: [PATCH 2/3] fix(emcn): compose consumer dismiss handlers instead of replacing the guard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ModalContent's own onEscapeKeyDown/onInteractOutside sit before the `{...props}` spread, so a consumer passing either replaced them — dropping both the dismissDisabled interlock and the floating-layer guard that keeps a popper dismissal from closing the modal and freezing the page. The TSDoc argued the guard had to live here for exactly that reason, then left the same spread able to defeat it. Both handlers are now destructured out of props and invoked after the guard, so the guard always runs and a consumer can still observe or extend the event. No consumer passes either today, so this was latent rather than live. --- .../components/chip-modal/chip-modal.test.tsx | 32 +++++++++++++++++++ packages/emcn/src/components/modal/modal.tsx | 11 ++++--- 2 files changed, 39 insertions(+), 4 deletions(-) diff --git a/packages/emcn/src/components/chip-modal/chip-modal.test.tsx b/packages/emcn/src/components/chip-modal/chip-modal.test.tsx index 21fa89a9698..85b7c79e059 100644 --- a/packages/emcn/src/components/chip-modal/chip-modal.test.tsx +++ b/packages/emcn/src/components/chip-modal/chip-modal.test.tsx @@ -4,6 +4,7 @@ import { act, type ReactNode } from 'react' import { createRoot, type Root } from 'react-dom/client' import { afterEach, describe, expect, it, vi } from 'vitest' +import { Modal, ModalContent, ModalHeader } from '../modal/modal' import { ChipConfirmModal, ChipModal, ChipModalFooter, ChipModalHeader } from './chip-modal' vi.mock('next/navigation', () => ({ @@ -147,6 +148,37 @@ describe('ChipModal dismissDisabled', () => { }) }) +describe('ModalContent dismissDisabled', () => { + it('runs a consumer escape handler without letting it drop the guard', () => { + const onOpenChange = vi.fn() + const onEscapeKeyDown = vi.fn() + mount( + + + + + + ) + + pressEscape() + expect(onEscapeKeyDown).toHaveBeenCalled() + expect(onOpenChange).not.toHaveBeenCalled() + }) + + it('disables the built-in ModalHeader close button', () => { + const onOpenChange = vi.fn() + mount( + + + Title + + + ) + + expect(closeButton().disabled).toBe(true) + }) +}) + describe('ChipConfirmModal pending', () => { it('holds every exit shut while the confirm runs', () => { const onOpenChange = vi.fn() diff --git a/packages/emcn/src/components/modal/modal.tsx b/packages/emcn/src/components/modal/modal.tsx index 60a1608a245..feef6965c33 100644 --- a/packages/emcn/src/components/modal/modal.tsx +++ b/packages/emcn/src/components/modal/modal.tsx @@ -469,10 +469,9 @@ export interface ModalContentProps * clicking outside, and `ModalHeader`'s close button. Descendants read it via * {@link useModalDismissDisabled}. * - * The Radix paths are handled here rather than through a consumer-passed - * `onEscapeKeyDown` / `onInteractOutside` because `{...props}` is spread after - * this component's own handlers, so a consumer overriding either would - * silently drop the floating-layer guard below. + * A consumer's own `onEscapeKeyDown` / `onInteractOutside` runs after this + * guard rather than replacing it, so neither the interlock nor the + * floating-layer guard can be dropped by passing a handler. * @default false */ dismissDisabled?: boolean @@ -497,6 +496,8 @@ const ModalContent = React.forwardRef< dismissDisabled = false, style, onOpenAutoFocus, + onEscapeKeyDown, + onInteractOutside, 'aria-describedby': ariaDescribedBy, ...props }, @@ -601,6 +602,7 @@ const ModalContent = React.forwardRef< // Radix reads `defaultPrevented`; stopPropagation alone would not block it. if (dismissDisabled) e.preventDefault() e.stopPropagation() + onEscapeKeyDown?.(e) }} onPointerDown={(e) => { e.stopPropagation() @@ -626,6 +628,7 @@ const ModalContent = React.forwardRef< if (dismissDisabled || hasOpenFloatingLayer()) { e.preventDefault() } + onInteractOutside?.(e) }} onOpenAutoFocus={(event) => { // Radix fires this once when the (still invisible) Content From 993e8fa7df5bc1789d7423041eef12fee140ae73 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Tue, 4 Aug 2026 19:28:16 -0700 Subject: [PATCH 3/3] revert(ci): drop the type-check inputs allowlist MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The allowlist traded correctness for a modest cache win, in a gate whose only job is catching type errors. `resolveJsonModule` and `allowJs` are both on, so .json and .js files participate in type checking and were absent from the list — `lib/integrations/availability.ts` imports the generated `integrations.json`, which means regenerating that file would not have invalidated the cache and CI would have replayed a stale pass over a changed type. Back to Turbo's default (every non-gitignored file in the package): conservative, but a type-check gate that can serve a stale success is worse than a slow one. --- turbo.json | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/turbo.json b/turbo.json index 957b97fd0ab..bdb8730df0f 100644 --- a/turbo.json +++ b/turbo.json @@ -38,16 +38,7 @@ }, "type-check": { "dependsOn": ["transit"], - "outputs": [], - "inputs": [ - "**/*.ts", - "**/*.tsx", - "**/*.mts", - "**/*.cts", - "**/*.mdx", - "tsconfig*.json", - "package.json" - ] + "outputs": [] } } }