From 09bf7b88ada02f4268d4ed9a7cd4dc8d943c450b Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Sat, 31 Oct 2026 11:13:52 -0700 Subject: [PATCH 1/2] fix(desktop): ask before an update restart from the menu, count uploads and downloads as running work, and harden sign-in prompts --- apps/desktop/e2e/fixtures/updater.ts | 1 + apps/desktop/package.json | 1 + .../main/browser-agent/page-prompts.test.ts | 20 ++++- .../src/main/browser-agent/page-prompts.ts | 67 ++++++++------- .../src/main/browser-agent/session.test.ts | 57 +++++++++++- .../desktop/src/main/browser-agent/session.ts | 31 +++++-- apps/desktop/src/main/in-flight-work.ts | 8 ++ apps/desktop/src/main/index.ts | 2 + apps/desktop/src/main/input-activity.ts | 7 +- apps/desktop/src/main/updater.test.ts | 35 ++++++++ apps/desktop/src/main/updater.ts | 14 ++- .../workspace/[workspaceId]/files/files.tsx | 2 + .../browser-session/browser-notice.tsx | 2 +- .../browser-page-prompt.test.tsx | 56 ++++++++++++ .../browser-session/browser-page-prompt.tsx | 53 ++++++++---- .../browser-session/sign-in-hosts.test.ts | 13 ++- .../browser-session/sign-in-hosts.ts | 9 +- .../browser-session/use-continue-sign-in.ts | 4 +- .../knowledge/[id]/[documentId]/document.tsx | 10 +-- .../knowledge/hooks/use-knowledge-upload.ts | 2 + .../import-csv-dialog/import-csv-dialog.tsx | 2 + .../workspace/[workspaceId]/tables/tables.tsx | 2 + .../sim/hooks/queries/browser-import.test.tsx | 86 +++++++++++++++++++ apps/sim/hooks/queries/browser-import.ts | 17 ++-- apps/sim/hooks/use-before-unload-while.ts | 19 ++++ bun.lock | 1 + packages/browser-protocol/src/index.ts | 6 +- 27 files changed, 441 insertions(+), 86 deletions(-) create mode 100644 apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/components/browser-session/browser-page-prompt.test.tsx create mode 100644 apps/sim/hooks/queries/browser-import.test.tsx create mode 100644 apps/sim/hooks/use-before-unload-while.ts diff --git a/apps/desktop/e2e/fixtures/updater.ts b/apps/desktop/e2e/fixtures/updater.ts index ff2b4703838..6bd92777510 100644 --- a/apps/desktop/e2e/fixtures/updater.ts +++ b/apps/desktop/e2e/fixtures/updater.ts @@ -105,6 +105,7 @@ void app.whenReady().then(async () => { computerUseApp: null, browserToolRunning, agentWorkRunning: false, + downloadCount: 0, ...(await countWindowsWithUnsavedChanges( hungPage ? [page.webContents, hungPage.webContents] : [page.webContents] )), diff --git a/apps/desktop/package.json b/apps/desktop/package.json index be7628bbb3d..94e75d5bf65 100644 --- a/apps/desktop/package.json +++ b/apps/desktop/package.json @@ -41,6 +41,7 @@ "@sim/utils": "workspace:*", "@xterm/headless": "6.0.0", "electron-updater": "6.8.9", + "lru-cache": "11.5.3", "micromatch": "4.0.8", "safe-regex2": "5.1.2", "tldts": "7.4.18", diff --git a/apps/desktop/src/main/browser-agent/page-prompts.test.ts b/apps/desktop/src/main/browser-agent/page-prompts.test.ts index 68ab5f06532..0c97f95fc3d 100644 --- a/apps/desktop/src/main/browser-agent/page-prompts.test.ts +++ b/apps/desktop/src/main/browser-agent/page-prompts.test.ts @@ -171,6 +171,16 @@ describe('HTTP sign-in', () => { answer(contents, { allowed: true, kind: 'http-auth', username: 'ada', password: 'pw' }) expect(callback).toHaveBeenCalledExactlyOnceWith('ada', 'pw') }) + + it('cancels instead of leaving the page waiting when an accepted answer is malformed', () => { + const contents = agentContents() + noteMainFrameNavigation(contents, url) + const callback = login(contents) + // double-cast-allowed: an IPC payload is untyped; this is a renderer's malformed answer + answer(contents, { allowed: true, kind: 'http-auth' } as unknown as BrowserPagePromptResponse) + expect(callback).toHaveBeenCalledExactlyOnceWith() + expect(pagePromptForContents(contents)).toBeUndefined() + }) }) describe('client certificates', () => { @@ -228,13 +238,19 @@ describe('client certificates', () => { expect(request(agentContents(otherSession), [first]).callback).not.toHaveBeenCalled() }) - it("remembers Don't send, but not a prompt nobody answered", () => { + it("remembers Don't send, but not a closed picker or a prompt nobody answered", () => { const declined = agentContents() const refused = request(declined, [first]) - answer(declined, { allowed: false }) + answer(declined, { allowed: false, remember: true }) expect(refused.callback).toHaveBeenCalledExactlyOnceWith() expect(request(agentContents(declined.session), [first]).callback).toHaveBeenCalledWith() + const closed = agentContents() + const dismissed = request(closed, [first]) + answer(closed, { allowed: false }) + expect(dismissed.callback).toHaveBeenCalledExactlyOnceWith() + expect(request(agentContents(closed.session), [first]).callback).not.toHaveBeenCalled() + const ignored = agentContents() const unanswered = request(ignored, [first]) vi.advanceTimersByTime(120_000) diff --git a/apps/desktop/src/main/browser-agent/page-prompts.ts b/apps/desktop/src/main/browser-agent/page-prompts.ts index 2a96b040571..822c703ba84 100644 --- a/apps/desktop/src/main/browser-agent/page-prompts.ts +++ b/apps/desktop/src/main/browser-agent/page-prompts.ts @@ -26,7 +26,9 @@ import type { WebFrameMain, } from 'electron' import { app, webContents as electronWebContents } from 'electron' +import { LRUCache } from 'lru-cache' import { isAgentWebContents } from '@/main/browser-agent/registry' +import { withinWindow } from '@/main/input-activity' import { appLinkTarget, openExternalSafe, parseHttpUrl } from '@/main/navigation' const logger = createLogger('BrowserPagePrompts') @@ -92,10 +94,11 @@ let host: PagePromptHost | null = null const pendingById = new Map() const pendingByContents = new WeakMap() const mainFrameNavigations = new WeakMap() -const recentAttemptsByContents = new WeakMap>() -/** `null` remembers an explicit "no certificate" for that host. */ -let certificateDecisions = new WeakMap>() -const alwaysAllowedLaunches = new Map() +const recentAttemptsByContents = new WeakMap>() +/** A host's remembered certificate, or `'declined'` for an explicit "Don't send". */ +type CertificateDecision = { fingerprint: string } | 'declined' +let certificateDecisions = new WeakMap>() +const alwaysAllowedLaunches = new LRUCache({ max: MAX_REMEMBERED_DECISIONS }) export function setPagePromptHost(next: PagePromptHost | null): void { host = next @@ -110,27 +113,14 @@ export function resetPagePromptDecisions(session?: Session): void { alwaysAllowedLaunches.clear() } -function withinWindow(at: number, windowMs: number): boolean { - const elapsed = Date.now() - at - return elapsed >= 0 && elapsed < windowMs -} - -function rememberBounded(map: Map, key: string, value: Value): void { - map.delete(key) - map.set(key, value) - while (map.size > MAX_REMEMBERED_DECISIONS) { - const oldest = map.keys().next().value - if (oldest === undefined) break - map.delete(oldest) - } -} - /** The live list of this tab's recent attempts under `key`, pruned to the window. */ function recentAttempts(contents: WebContents, key: string, windowMs: number): number[] { - const byKey = recentAttemptsByContents.get(contents) ?? new Map() + const byKey = + recentAttemptsByContents.get(contents) ?? + new LRUCache({ max: MAX_REMEMBERED_DECISIONS }) recentAttemptsByContents.set(contents, byKey) const recent = (byKey.get(key) ?? []).filter((at) => withinWindow(at, windowMs)) - rememberBounded(byKey, key, recent) + byKey.set(key, recent) return recent } @@ -247,6 +237,12 @@ function settle(entry: PendingRequest, response: BrowserPagePromptResponse | nul answer(response) } catch (error) { logger.warn('Browser page prompt answer failed', { error: getErrorMessage(error) }) + // A malformed response must still release the page, which otherwise waits forever. + if (response !== null) { + try { + answer(null) + } catch {} + } } } } @@ -271,8 +267,8 @@ export function cancelPagePrompts(contents: WebContents): void { /** * Tracks the URL the tab's top-level navigation is requesting (null once it - * commits or fails), so HTTP authentication can tell its own challenge from a - * frame's. + * commits, fails, or the tab stops loading), so HTTP authentication can tell its + * own challenge from a frame's. */ export function noteMainFrameNavigation(contents: WebContents, url: string | null): void { if (url) mainFrameNavigations.set(contents, url) @@ -352,6 +348,8 @@ export function handleBrowserLogin( if ( response?.allowed === true && response.kind === 'http-auth' && + typeof response.username === 'string' && + typeof response.password === 'string' && response.username.length <= MAX_CREDENTIAL_LENGTH && response.password.length <= MAX_CREDENTIAL_LENGTH ) { @@ -448,13 +446,15 @@ export function handleSelectClientCertificate( }) ) if (!requestHost || choices.length === 0) return false - const decisions = certificateDecisions.get(contents.session) ?? new Map() + const decisions = + certificateDecisions.get(contents.session) ?? + new LRUCache({ max: MAX_REMEMBERED_DECISIONS }) certificateDecisions.set(contents.session, decisions) const remembered = decisions.get(requestHost) - if (remembered === null) return false - const rememberedCertificate = certificates.find( - (certificate) => certificate.fingerprint === remembered - ) + if (remembered === 'declined') return false + const rememberedCertificate = remembered + ? certificates.find((certificate) => certificate.fingerprint === remembered.fingerprint) + : undefined if (rememberedCertificate) { answer(rememberedCertificate) return true @@ -466,19 +466,22 @@ export function handleSelectClientCertificate( `client-certificate|${requestHost}`, (response) => { if (response?.allowed === false) { - rememberBounded(decisions, requestHost, null) + // Only an explicit "Don't send" sticks; closing the picker just cancels this request. + if (response.remember === true) decisions.set(requestHost, 'declined') answer() return } const certificate = - response?.allowed === true && response.kind === 'client-certificate' + response?.allowed === true && + response.kind === 'client-certificate' && + typeof response.choiceId === 'string' ? certificates.find((candidate) => candidate.fingerprint === response.choiceId) : undefined if (!certificate) { answer() return } - rememberBounded(decisions, requestHost, certificate.fingerprint) + decisions.set(requestHost, { fingerprint: certificate.fingerprint }) answer(certificate) }, { joinable: true } @@ -568,7 +571,7 @@ export function requestExternalLaunch( (response) => { if (response?.allowed !== true || response.kind !== 'external-app') return if (response.alwaysAllow && canAlwaysAllow) { - rememberBounded(alwaysAllowedLaunches, launchKey, true) + alwaysAllowedLaunches.set(launchKey, true) } launchAppLink(target.url) }, diff --git a/apps/desktop/src/main/browser-agent/session.test.ts b/apps/desktop/src/main/browser-agent/session.test.ts index 92e4b1ff448..6f1ceff17b0 100644 --- a/apps/desktop/src/main/browser-agent/session.test.ts +++ b/apps/desktop/src/main/browser-agent/session.test.ts @@ -20,7 +20,11 @@ import { shell, WebContentsView, } from 'electron' -import { appLaunchNoticeForContents } from '@/main/browser-agent/page-prompts' +import { + appLaunchNoticeForContents, + handleBrowserLogin, + pagePromptForContents, +} from '@/main/browser-agent/page-prompts' import * as panel from '@/main/browser-agent/panel' import { agentAppOrigin, routeAgentNavigation } from '@/main/browser-agent/registry' import * as sessionModule from '@/main/browser-agent/session' @@ -1730,6 +1734,57 @@ describe('browser-agent session', () => { }) }) + it('asks about an HTTP sign-in only while its top-level navigation is still loading', () => { + const url = 'https://intranet.example/' + const contents = (session.ensureTab().view as unknown as MockView).webContents + session.enablePagePrompts() + const emit = (eventName: string, ...args: unknown[]) => { + for (const [name, handler] of contents.on.mock.calls) { + if (name === eventName && typeof handler === 'function') handler(...args) + } + } + const signIn = () => { + const callback = vi.fn() + handleBrowserLogin( + { preventDefault: () => {} }, + contents as unknown as WebContents, + { url, isRequestForNavigation: true, firstAuthAttempt: true } as never, + { isProxy: false, scheme: 'basic', host: 'intranet.example', port: 443, realm: '' }, + callback + ) + return callback + } + + emit('did-start-navigation', { isMainFrame: true, isSameDocument: false, url }) + const loading = signIn() + expect(loading).not.toHaveBeenCalled() + expect(pagePromptForContents(contents as unknown as WebContents)).toMatchObject({ + kind: 'http-auth', + }) + + // The navigation is aborted: Chromium reports ERR_ABORTED, which records no failure. + emit('did-start-navigation', { isMainFrame: true, isSameDocument: false, url }) + emit('did-stop-loading') + expect(signIn()).toHaveBeenCalledExactlyOnceWith() + }) + + it('counts a download as in flight for a restart only until it finishes', async () => { + const directory = mkdtempSync(join(tmpdir(), 'sim-browser-downloads-')) + session = freshSession(win, {}, undefined, { + getDirectory: () => directory, + getFreeDiskBytes: () => Number.MAX_SAFE_INTEGER, + }) + const contents = (session.ensureTab().view as unknown as MockView).webContents + const download = mockDownloadItem({ filename: 'report.csv', totalBytes: 100 }) + + startMockDownload(contents, download) + await vi.waitFor(() => expect(download.item.resume).toHaveBeenCalledOnce()) + expect(session.countProgressingBrowserDownloads()).toBe(1) + + download.emitDone('completed') + await vi.waitFor(() => expect(session.countProgressingBrowserDownloads()).toBe(0)) + }) + it('never overwrites a file that takes the allocated name before the download claims it', async () => { const directory = mkdtempSync(join(tmpdir(), 'sim-browser-downloads-')) writeFileSync(join(directory, 'taken.bin'), 'user data') diff --git a/apps/desktop/src/main/browser-agent/session.ts b/apps/desktop/src/main/browser-agent/session.ts index 6131d25a56f..e912baa5722 100644 --- a/apps/desktop/src/main/browser-agent/session.ts +++ b/apps/desktop/src/main/browser-agent/session.ts @@ -89,6 +89,7 @@ import { import { clearHostVerdictCache } from '@/main/browser-agent/url-guard' import type { BrowserSessionSnapshot } from '@/main/desktop-chat-session-store' import { suggestedFilename, uniqueDownloadPath } from '@/main/downloads' +import { withinWindow } from '@/main/input-activity' import { isAppOrigin, isBlankPopupUrl, parseHttpUrl } from '@/main/navigation' import { type FocusedResourceShortcut, @@ -257,12 +258,18 @@ export type BrowserShortcut = | 'hard-reload' | UserOnlyShortcut +type UserOnlyShortcut = 'back' | 'forward' | 'previous-tab' | 'next-tab' | 'stop' /** Shortcuts that move the user's history, tabs, or load; keystrokes the agent sends never trigger them. */ -const USER_ONLY_SHORTCUTS = ['back', 'forward', 'previous-tab', 'next-tab', 'stop'] as const -type UserOnlyShortcut = (typeof USER_ONLY_SHORTCUTS)[number] +const USER_ONLY_SHORTCUTS: ReadonlySet = new Set([ + 'back', + 'forward', + 'previous-tab', + 'next-tab', + 'stop', +]) function isUserOnlyShortcut(shortcut: BrowserShortcut): shortcut is UserOnlyShortcut { - return USER_ONLY_SHORTCUTS.some((candidate) => candidate === shortcut) + return USER_ONLY_SHORTCUTS.has(shortcut) } type BrowserShortcutInput = Pick< @@ -635,6 +642,15 @@ function activeBrowserDownloadCount(scopeId?: string): number { return count } +/** Agent-browser downloads still receiving bytes, which a restart would cut off. */ +export function countProgressingBrowserDownloads(): number { + let count = 0 + for (const active of activeBrowserDownloads) { + if (active.download.state === 'progressing') count += 1 + } + return count +} + function withBrowserDownloadTimeout( operation: Promise, timeoutMs: number, @@ -1569,12 +1585,6 @@ function tabIsOnScreen(scopeId: string, tab: AgentTab): boolean { ) } -/** Whether `at` lies in the last `windowMs`; a clock stepping backwards never passes. */ -function withinWindow(at: number, windowMs: number): boolean { - const elapsed = Date.now() - at - return elapsed >= 0 && elapsed < windowMs -} - function mediaRequestIsUserInitiated(scopeId: string, tab: AgentTab): boolean { const win = panelWindow() return ( @@ -2984,6 +2994,9 @@ function initializeTabView( if (details.isMainFrame && !details.isSameDocument) noteMainFrameNavigation(contents, details.url) }) + // An aborted navigation (ERR_ABORTED) reports no failure the driver records; once nothing is + // loading, no top-level request is pending, so a later frame cannot pass for it. + contents.on('did-stop-loading', () => noteMainFrameNavigation(contents, null)) contents.on( 'did-start-navigation', diff --git a/apps/desktop/src/main/in-flight-work.ts b/apps/desktop/src/main/in-flight-work.ts index c83eb8e8f2e..82f6219f36c 100644 --- a/apps/desktop/src/main/in-flight-work.ts +++ b/apps/desktop/src/main/in-flight-work.ts @@ -68,6 +68,8 @@ export interface InFlightWork { browserToolRunning: boolean /** Whether a chat has desktop work claimed on this machine. */ agentWorkRunning: boolean + /** Agent-browser downloads still in progress. */ + downloadCount: number unsavedWindowCount: number unresponsiveWindowCount: number } @@ -101,6 +103,12 @@ export function describeInFlightWork(work: InFlightWork): string[] { sentences.push('An agent is still working on this computer.') } + if (work.downloadCount === 1) { + sentences.push('A download is still in progress.') + } else if (work.downloadCount > 1) { + sentences.push(`${work.downloadCount} downloads are still in progress.`) + } + if (work.unsavedWindowCount === 1) { sentences.push('A window has unsaved changes.') } else if (work.unsavedWindowCount > 1) { diff --git a/apps/desktop/src/main/index.ts b/apps/desktop/src/main/index.ts index 331ab4df94b..e61e895fa47 100644 --- a/apps/desktop/src/main/index.ts +++ b/apps/desktop/src/main/index.ts @@ -47,6 +47,7 @@ import { setPanelOccluded as setBrowserAgentPanelOccluded, } from '@/main/browser-agent/panel' import { + countProgressingBrowserDownloads, handleFocusedShortcut as handleFocusedBrowserShortcut, isBrowserScopeSuspended, quiesceBrowserSessions, @@ -1180,6 +1181,7 @@ function main(): void { computerUseApp: computerUseActivity ? (computerUseActivity.appName ?? '') : null, browserToolRunning: isBrowserToolRunning(), agentWorkRunning, + downloadCount: countProgressingBrowserDownloads(), ...windows, }) }, diff --git a/apps/desktop/src/main/input-activity.ts b/apps/desktop/src/main/input-activity.ts index 1e1579edcfa..e67bbeebc88 100644 --- a/apps/desktop/src/main/input-activity.ts +++ b/apps/desktop/src/main/input-activity.ts @@ -132,6 +132,11 @@ function isWithin( if (contents.isDestroyed()) return false const activity = activityByContents.get(contents) if (!activity) return false - const elapsed = Date.now() - stamp(activity) + return withinWindow(stamp(activity), windowMs) +} + +/** Whether `at` lies in the last `windowMs`; a clock stepping backwards never passes. */ +export function withinWindow(at: number, windowMs: number): boolean { + const elapsed = Date.now() - at return elapsed >= 0 && elapsed < windowMs } diff --git a/apps/desktop/src/main/updater.test.ts b/apps/desktop/src/main/updater.test.ts index 5909fa3824a..8935167ace8 100644 --- a/apps/desktop/src/main/updater.test.ts +++ b/apps/desktop/src/main/updater.test.ts @@ -847,6 +847,41 @@ describe('checkForUpdatesInteractive', () => { vi.useRealTimers() }) + function readyHandle(): UpdaterHandle { + return { + setAutoDownload: () => {}, + getState: () => ({ status: 'ready', version: '9.9.9' }), + check: vi.fn(), + install: vi.fn(), + onState: () => () => {}, + } + } + + it('asks before restarting when the check finds an update already staged', async () => { + const handle = readyHandle() + + checkForUpdatesInteractive({ getWindow: () => null, events, handle }) + await vi.advanceTimersByTimeAsync(0) + + expect(dialog.showMessageBox).toHaveBeenCalledWith( + expect.objectContaining({ + message: 'Sim 9.9.9 is ready to install', + buttons: ['Restart to Update', 'Later'], + }) + ) + expect(handle.install).not.toHaveBeenCalled() + }) + + it('installs a staged update only once the user chooses Restart to Update', async () => { + const handle = readyHandle() + vi.mocked(dialog.showMessageBox).mockResolvedValueOnce({ response: 0, checkboxChecked: false }) + + checkForUpdatesInteractive({ getWindow: () => null, events, handle }) + await vi.advanceTimersByTimeAsync(0) + + expect(handle.install).toHaveBeenCalledTimes(1) + }) + it('fails a hung interactive check after twelve seconds instead of waiting thirty', async () => { const handle: UpdaterHandle = { setAutoDownload: () => {}, diff --git a/apps/desktop/src/main/updater.ts b/apps/desktop/src/main/updater.ts index b84071d3da3..caaff3e9c78 100644 --- a/apps/desktop/src/main/updater.ts +++ b/apps/desktop/src/main/updater.ts @@ -1120,7 +1120,19 @@ export function checkForUpdatesInteractive( }) return case 'ready': - handle.install() + // The user asked to check, not to restart: confirm before closing every window. + void showDialog({ + type: 'info', + buttons: ['Restart to Update', 'Later'], + defaultId: 0, + cancelId: 1, + message: state.version + ? `Sim ${state.version} is ready to install` + : 'An update is ready to install', + detail: 'Restart Sim now to finish updating, or it installs the next time you quit.', + }).then(({ response }) => { + if (response === 0) handle.install() + }) return case 'error': void showDialog({ diff --git a/apps/sim/app/workspace/[workspaceId]/files/files.tsx b/apps/sim/app/workspace/[workspaceId]/files/files.tsx index 08cf1f6be66..931234bc5a2 100644 --- a/apps/sim/app/workspace/[workspaceId]/files/files.tsx +++ b/apps/sim/app/workspace/[workspaceId]/files/files.tsx @@ -163,6 +163,7 @@ import { useWorkspaceFileRecord, workspaceFileRecordState, } from '@/hooks/queries/workspace-files' +import { useBeforeUnloadWhile } from '@/hooks/use-before-unload-while' import { useContextMenu } from '@/hooks/use-context-menu' import { useDebouncedSearchSetter } from '@/hooks/use-debounced-search-setter' import { useInlineRename } from '@/hooks/use-inline-rename' @@ -353,6 +354,7 @@ function FilesContent() { }) /** An upload batch is in flight exactly while a total is set — matches the Tables page. */ const uploading = uploadProgress.total > 0 + useBeforeUnloadWhile(uploading) const [isDraggingOver, setIsDraggingOver] = useState(false) const dragCounterRef = useRef(0) /** diff --git a/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/components/browser-session/browser-notice.tsx b/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/components/browser-session/browser-notice.tsx index 7bda8db6892..9bc28d9b63f 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/components/browser-session/browser-notice.tsx +++ b/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/components/browser-session/browser-notice.tsx @@ -11,7 +11,7 @@ export function BrowserNotice({ message, actions }: BrowserNoticeProps) { return (
-

{message}

+

{message}

{actions}
 diff --git a/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/components/browser-session/browser-page-prompt.test.tsx b/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/components/browser-session/browser-page-prompt.test.tsx new file mode 100644 index 00000000000..ab577600944 --- /dev/null +++ b/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/components/browser-session/browser-page-prompt.test.tsx @@ -0,0 +1,56 @@ +/** + * @vitest-environment jsdom + */ +import { act } from 'react' +import type { BrowserPagePrompt } from '@sim/browser-protocol' +import { createRoot, type Root } from 'react-dom/client' +import { afterEach, describe, expect, it, vi } from 'vitest' +import { BrowserPagePromptModal } from '@/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/components/browser-session/browser-page-prompt' + +const mountedRoots: Root[] = [] + +function renderPrompt(prompt: BrowserPagePrompt, onRespond = vi.fn()) { + ;(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true + const root = createRoot(document.createElement('div')) + mountedRoots.push(root) + const render = (next: BrowserPagePrompt) => + act(() => { + root.render() + }) + render(prompt) + return { onRespond, render } +} + +/** A prompt kind a newer shell could send that this renderer has no modal for. */ +function promptFromNewerShell(requestId: string): BrowserPagePrompt { + // double-cast-allowed: models a future protocol kind this renderer's types cannot name + return { requestId, kind: 'future-kind' } as unknown as BrowserPagePrompt +} + +afterEach(() => { + for (const root of mountedRoots.splice(0)) act(() => root.unmount()) +}) + +describe('BrowserPagePromptModal', () => { + it('refuses a prompt kind it cannot show at once, once per request', () => { + const { onRespond, render } = renderPrompt(promptFromNewerShell('r1')) + render(promptFromNewerShell('r1')) + expect(onRespond).toHaveBeenCalledExactlyOnceWith('r1', { allowed: false }) + + render(promptFromNewerShell('r2')) + expect(onRespond).toHaveBeenCalledTimes(2) + expect(onRespond).toHaveBeenLastCalledWith('r2', { allowed: false }) + }) + + it('leaves a prompt it can show for the user to answer', () => { + const { onRespond } = renderPrompt({ + requestId: 'r1', + kind: 'http-auth', + origin: 'https://intranet.example', + proxy: false, + insecure: false, + retry: false, + }) + expect(onRespond).not.toHaveBeenCalled() + }) +}) diff --git a/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/components/browser-session/browser-page-prompt.tsx b/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/components/browser-session/browser-page-prompt.tsx index fd0d823e864..bff47da8446 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/components/browser-session/browser-page-prompt.tsx +++ b/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/components/browser-session/browser-page-prompt.tsx @@ -1,6 +1,6 @@ 'use client' -import { type ReactNode, useEffect, useState } from 'react' +import { type ReactNode, useEffect, useRef, useState } from 'react' import type { BrowserAppLaunchNotice, BrowserClientCertificatePrompt, @@ -11,7 +11,6 @@ import type { BrowserPasskeyPrompt, } from '@sim/browser-protocol' import { - Banner, Checkbox, Chip, ChipConfirmModal, @@ -26,6 +25,7 @@ import { Label, } from '@sim/emcn' import { X } from '@sim/emcn/icons' +import { BrowserNotice } from '@/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/components/browser-session/browser-notice' /** Keeps a click or keystroke aimed at the page from answering a prompt as it appears. */ const ACTIVATION_DELAY_MS = 500 @@ -123,6 +123,8 @@ interface ChoicePromptModalProps { hintFor?: (choiceId: string) => string | undefined cancelLabel?: string onCancel: () => void + /** Closing without the Cancel button; defaults to {@link ChoicePromptModalProps.onCancel}. */ + onDismiss?: () => void onChoose: (choiceId: string) => void } @@ -141,6 +143,7 @@ function ChoicePromptModal({ hintFor, cancelLabel, onCancel, + onDismiss = onCancel, onChoose, }: ChoicePromptModalProps) { const armed = useActivationDelay(open) @@ -148,12 +151,12 @@ function ChoicePromptModal({ return ( !next && onCancel()} + onOpenChange={(next) => !next && onDismiss()} srTitle={title} size='sm' focusOnOpen={false} > - {title} + {title} {description} onRespond(prompt.requestId, { allowed: false })} + onCancel={() => onRespond(prompt.requestId, { allowed: false, remember: true })} + onDismiss={() => onRespond(prompt.requestId, { allowed: false })} onChoose={(choiceId) => onRespond(prompt.requestId, { allowed: true, kind: 'client-certificate', choiceId }) } @@ -304,11 +308,26 @@ interface BrowserPagePromptModalProps { onRespond: RespondToPrompt } -/** The sign-in or app-launch decision the visible page is waiting on. */ +/** + * The sign-in or app-launch decision the visible page is waiting on. A kind this + * renderer cannot show, from a newer shell, is refused at once rather than left + * for the page to wait out. + */ export function BrowserPagePromptModal({ prompt, open, onRespond }: BrowserPagePromptModalProps) { - if (!prompt) return null - const Modal = PROMPT_MODALS[prompt.kind] as PromptModal | undefined - if (!Modal) return null + const refusedRequestIdRef = useRef(null) + const Modal = + prompt && Object.hasOwn(PROMPT_MODALS, prompt.kind) + ? (PROMPT_MODALS[prompt.kind] as PromptModal) + : undefined + const unsupportedRequestId = prompt && !Modal ? prompt.requestId : null + + useEffect(() => { + if (!unsupportedRequestId || refusedRequestIdRef.current === unsupportedRequestId) return + refusedRequestIdRef.current = unsupportedRequestId + onRespond(unsupportedRequestId, { allowed: false }) + }, [unsupportedRequestId, onRespond]) + + if (!prompt || !Modal) return null return } @@ -321,12 +340,10 @@ interface BrowserAppLaunchNoticeBarProps { export function BrowserAppLaunchNoticeBar({ notice, onRespond }: BrowserAppLaunchNoticeBarProps) { const armed = useActivationDelay(true) return ( - -
-

- {notice.origin} wants to open {notice.appName}. -

-
+ @@ -340,8 +357,8 @@ export function BrowserAppLaunchNoticeBar({ notice, onRespond }: BrowserAppLaunc aria-label={`Don't open ${notice.appName}`} onClick={() => onRespond(notice.requestId, { allowed: false })} /> -
-
- + + } + /> ) } diff --git a/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/components/browser-session/sign-in-hosts.test.ts b/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/components/browser-session/sign-in-hosts.test.ts index 0a3ad3527a1..2263d1cdacd 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/components/browser-session/sign-in-hosts.test.ts +++ b/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/components/browser-session/sign-in-hosts.test.ts @@ -1,11 +1,13 @@ import { describe, expect, it } from 'vitest' import { - parseHttpsPage, + continuablePage, signInHintHost, } from '@/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/components/browser-session/sign-in-hosts' +const APP_ORIGIN = 'https://www.sim.ai' + function hintFor(url: string | undefined): string | null { - const page = parseHttpsPage(url) + const page = continuablePage(url, APP_ORIGIN) return page ? signInHintHost(page) : null } @@ -42,3 +44,10 @@ describe('signInHintHost', () => { expect(hintFor(url)).toBeNull() }) }) + +describe('continuablePage', () => { + it("never offers Sim's own pages, which the shell refuses to hand off", () => { + expect(continuablePage(`${APP_ORIGIN}/workspace/w1/home`, APP_ORIGIN)).toBeNull() + expect(continuablePage('https://sim.ai/login', APP_ORIGIN)?.hostname).toBe('sim.ai') + }) +}) diff --git a/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/components/browser-session/sign-in-hosts.ts b/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/components/browser-session/sign-in-hosts.ts index 2ccd4238aa7..7b8e96da150 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/components/browser-session/sign-in-hosts.ts +++ b/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/components/browser-session/sign-in-hosts.ts @@ -26,12 +26,15 @@ const SIGN_IN_HOSTS: readonly SignInHost[] = [ { host: 'auth0.com', subdomains: true }, ] -/** The page as a URL when it is https, the only kind that can be continued elsewhere. */ -export function parseHttpsPage(url: string | undefined): URL | null { +/** + * The page as a URL when another browser can continue it: https, and not one of + * Sim's own pages on `appOrigin`, which the shell refuses to hand off. + */ +export function continuablePage(url: string | undefined, appOrigin: string): URL | null { if (!url) return null try { const parsed = new URL(url) - return parsed.protocol === 'https:' ? parsed : null + return parsed.protocol === 'https:' && parsed.origin !== appOrigin ? parsed : null } catch { return null } diff --git a/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/components/browser-session/use-continue-sign-in.ts b/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/components/browser-session/use-continue-sign-in.ts index ae232cf2a41..340e840262e 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/components/browser-session/use-continue-sign-in.ts +++ b/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/components/browser-session/use-continue-sign-in.ts @@ -5,7 +5,7 @@ import type { BrowserContinueSignInError, BrowserImportProfile } from '@sim/desk import { toast } from '@sim/emcn' import { IMPORT_ERROR_MESSAGES } from '@/components/browser-import/browser-import-dialog' import { - parseHttpsPage, + continuablePage, signInHintHost, } from '@/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/components/browser-session/sign-in-hosts' import { @@ -58,7 +58,7 @@ export function useContinueSignIn({ scopeId, pageUrl }: UseContinueSignInProps) const { mutate: cancelHandoff } = useCancelSignInHandoff(scopeId) useSyncExternalStore(subscribeToDismissals, dismissalCount, dismissalCount) - const page = parseHttpsPage(pageUrl) + const page = continuablePage(pageUrl, typeof window === 'undefined' ? '' : window.location.origin) // The profiles cache is shared with the import dialog, which older shells // also support, so cached profiles alone must not reveal this feature. const canContinue = supported && profiles.length > 0 && page !== null diff --git a/apps/sim/app/workspace/[workspaceId]/knowledge/[id]/[documentId]/document.tsx b/apps/sim/app/workspace/[workspaceId]/knowledge/[id]/[documentId]/document.tsx index 7f25177ef2f..09a21912c77 100644 --- a/apps/sim/app/workspace/[workspaceId]/knowledge/[id]/[documentId]/document.tsx +++ b/apps/sim/app/workspace/[workspaceId]/knowledge/[id]/[documentId]/document.tsx @@ -77,6 +77,7 @@ import { useUpdateChunk, useUpdateDocument, } from '@/hooks/queries/kb/knowledge' +import { useBeforeUnloadWhile } from '@/hooks/use-before-unload-while' import { useContextMenu } from '@/hooks/use-context-menu' import { useDebounce } from '@/hooks/use-debounce' import { useDebouncedSearchSetter } from '@/hooks/use-debounced-search-setter' @@ -499,14 +500,7 @@ export function Document({ return () => window.removeEventListener('keydown', handleKeyDown) }, [isInEditorView]) - useEffect(() => { - if (!isDirty) return - const handler = (e: BeforeUnloadEvent) => { - e.preventDefault() - } - window.addEventListener('beforeunload', handler) - return () => window.removeEventListener('beforeunload', handler) - }, [isDirty]) + useBeforeUnloadWhile(isDirty) const navigateToChunk = useCallback( async (direction: 'prev' | 'next') => { diff --git a/apps/sim/app/workspace/[workspaceId]/knowledge/hooks/use-knowledge-upload.ts b/apps/sim/app/workspace/[workspaceId]/knowledge/hooks/use-knowledge-upload.ts index 34320dd67ab..c2ef37a9c9f 100644 --- a/apps/sim/app/workspace/[workspaceId]/knowledge/hooks/use-knowledge-upload.ts +++ b/apps/sim/app/workspace/[workspaceId]/knowledge/hooks/use-knowledge-upload.ts @@ -12,6 +12,7 @@ import { runWithConcurrency, WHOLE_FILE_PARALLEL_UPLOADS } from '@/lib/uploads/c import { uploadKnowledgeDocumentSession } from '@/lib/uploads/client/session-upload' import type { UploadProgressEvent } from '@/lib/uploads/client/types' import { knowledgeKeys } from '@/hooks/queries/utils/knowledge-keys' +import { useBeforeUnloadWhile } from '@/hooks/use-before-unload-while' const logger = createLogger('KnowledgeUpload') @@ -78,6 +79,7 @@ export function useKnowledgeUpload(options: UseKnowledgeUploadOptions = {}) { }) const [uploadError, setUploadError] = useState(null) const isUploading = uploadProgress.stage !== 'idle' + useBeforeUnloadWhile(isUploading) const updateFileStatus = (fileIndex: number, patch: Partial) => { setUploadProgress((prev) => ({ diff --git a/apps/sim/app/workspace/[workspaceId]/tables/components/import-csv-dialog/import-csv-dialog.tsx b/apps/sim/app/workspace/[workspaceId]/tables/components/import-csv-dialog/import-csv-dialog.tsx index 3d6a8e8724d..2741794cf07 100644 --- a/apps/sim/app/workspace/[workspaceId]/tables/components/import-csv-dialog/import-csv-dialog.tsx +++ b/apps/sim/app/workspace/[workspaceId]/tables/components/import-csv-dialog/import-csv-dialog.tsx @@ -33,6 +33,7 @@ import { } from '@/lib/table/import' import type { TableDefinition } from '@/lib/table/types' import { type CsvImportMode, useImportCsvIntoTable } from '@/hooks/queries/tables' +import { useBeforeUnloadWhile } from '@/hooks/use-before-unload-while' import { useImportTrayStore } from '@/stores/table/import-tray/store' const logger = createLogger('ImportCsvDialog') @@ -146,6 +147,7 @@ export function ImportCsvDialog({ const [createHeaders, setCreateHeaders] = useState>(new Set()) const [mode, setMode] = useState('append') const importMutation = useImportCsvIntoTable() + useBeforeUnloadWhile(importMutation.isPending) function resetState() { setParsed(null) diff --git a/apps/sim/app/workspace/[workspaceId]/tables/tables.tsx b/apps/sim/app/workspace/[workspaceId]/tables/tables.tsx index 01dab7d7975..6c16bf03ebd 100644 --- a/apps/sim/app/workspace/[workspaceId]/tables/tables.tsx +++ b/apps/sim/app/workspace/[workspaceId]/tables/tables.tsx @@ -101,6 +101,7 @@ import { } from '@/hooks/queries/tables' import { getCanonicalFolderPath } from '@/hooks/queries/utils/folder-tree' import { useWorkspaceMembersQuery, type WorkspaceMember } from '@/hooks/queries/workspace' +import { useBeforeUnloadWhile } from '@/hooks/use-before-unload-while' import { useContextMenu } from '@/hooks/use-context-menu' import { useDebouncedSearchSetter } from '@/hooks/use-debounced-search-setter' import { useInlineRename } from '@/hooks/use-inline-rename' @@ -326,6 +327,7 @@ function TablesContent() { const [uploadProgress, setUploadProgress] = useState({ completed: 0, total: 0 }) const uploading = uploadProgress.total > 0 + useBeforeUnloadWhile(uploading) const csvInputRef = useRef(null) /** diff --git a/apps/sim/hooks/queries/browser-import.test.tsx b/apps/sim/hooks/queries/browser-import.test.tsx new file mode 100644 index 00000000000..ce1c1269c9d --- /dev/null +++ b/apps/sim/hooks/queries/browser-import.test.tsx @@ -0,0 +1,86 @@ +/** @vitest-environment jsdom */ + +import { act } from 'react' +import type { BrowserPendingSignIn } from '@sim/desktop-bridge' +import { QueryClient, QueryClientProvider } from '@tanstack/react-query' +import { createRoot, type Root } from 'react-dom/client' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { usePendingSignIn, useStartSignInHandoff } from '@/hooks/queries/browser-import' + +/** The shell's pending sign-ins, by chat scope id. */ +let pendingByScope: Record +let finishLaunch: (() => void) | undefined +let shown: BrowserPendingSignIn | null | undefined +let start: ((profileId: string) => Promise) | undefined + +interface PanelProps { + scopeId: string +} + +function Panel({ scopeId }: PanelProps) { + shown = usePendingSignIn(scopeId, true).data + start = useStartSignInHandoff(scopeId).mutateAsync + return null +} + +let root: Root | undefined + +describe('pending sign-in after a chat is saved mid-handoff', () => { + beforeEach(() => { + vi.stubGlobal('IS_REACT_ACT_ENVIRONMENT', true) + pendingByScope = {} + Object.assign(window, { + simDesktop: { + browserImport: { + getPendingSignIn: async (scopeId: string) => pendingByScope[scopeId] ?? null, + continueSignIn: (_profileId: string, _scopeId: string) => + new Promise((resolve) => { + finishLaunch = () => { + // The provisional chat saved meanwhile; the shell moved its handoff to the saved id. + const pending = { site: 'github.com', browserLabel: 'Chrome' } + pendingByScope['chat-1'] = pending as BrowserPendingSignIn + resolve({ pending }) + } + }), + bringSignInBack: vi.fn(), + cancelSignIn: vi.fn(), + }, + }, + }) + }) + + afterEach(() => { + act(() => root?.unmount()) + root = undefined + Reflect.deleteProperty(window, 'simDesktop') + }) + + it("shows the handoff under the saved chat's id once the launch settles", async () => { + const container = document.createElement('div') + root = createRoot(container) + const client = new QueryClient() + const render = (scopeId: string) => + act(async () => { + root?.render( + + {/* The browser panel is keyed by its scope, so a saved chat id remounts it. */} + + + ) + }) + + await render('draft-1') + let launch: Promise | undefined + await act(async () => { + launch = start?.('profile-1') + }) + await render('chat-1') + await vi.waitFor(() => expect(shown).toBeNull()) + + await act(async () => { + finishLaunch?.() + await launch + }) + await vi.waitFor(() => expect(shown).toMatchObject({ site: 'github.com' })) + }) +}) diff --git a/apps/sim/hooks/queries/browser-import.ts b/apps/sim/hooks/queries/browser-import.ts index dd787ae07a6..81ffd9a1339 100644 --- a/apps/sim/hooks/queries/browser-import.ts +++ b/apps/sim/hooks/queries/browser-import.ts @@ -1,7 +1,7 @@ 'use client' import type { BrowserImportProfile, BrowserPendingSignIn } from '@sim/desktop-bridge' -import { useMutation, useQuery, useQueryClient } from '@tanstack/react-query' +import { type QueryClient, useMutation, useQuery, useQueryClient } from '@tanstack/react-query' import { getDesktopBridge } from '@/lib/desktop' /** Local browser data read through the desktop shell, never the server. */ @@ -56,6 +56,14 @@ export function usePendingSignIn(scopeId: string, enabled: boolean) { }) } +/** + * Refreshes every chat's pending sign-in, not just the one a mutation started under: + * a provisional chat id that saves mid-mutation moves the handoff to the saved id. + */ +function invalidatePendingSignIns(queryClient: QueryClient) { + return queryClient.invalidateQueries({ queryKey: browserImportKeys.pendingSignIns() }) +} + export function useStartSignInHandoff(scopeId: string) { const queryClient = useQueryClient() return useMutation({ @@ -69,8 +77,7 @@ export function useStartSignInHandoff(scopeId: string) { }, // A failed or superseded launch can still retire the chat's previous // handoff, so the shell's answer replaces whatever the cache last held. - onSettled: () => - queryClient.invalidateQueries({ queryKey: browserImportKeys.pendingSignIn(scopeId) }), + onSettled: () => invalidatePendingSignIns(queryClient), }) } @@ -82,8 +89,7 @@ export function useBringSignInBack(scopeId: string) { if (!bringSignInBack) throw new Error('Browser import is not available on this device.') return bringSignInBack(scopeId) }, - onSuccess: () => - queryClient.invalidateQueries({ queryKey: browserImportKeys.pendingSignIn(scopeId) }), + onSuccess: () => invalidatePendingSignIns(queryClient), }) } @@ -92,5 +98,6 @@ export function useCancelSignInHandoff(scopeId: string) { return useMutation({ mutationFn: async () => browserImport()?.cancelSignIn?.(scopeId), onSuccess: () => queryClient.setQueryData(browserImportKeys.pendingSignIn(scopeId), null), + onSettled: () => invalidatePendingSignIns(queryClient), }) } diff --git a/apps/sim/hooks/use-before-unload-while.ts b/apps/sim/hooks/use-before-unload-while.ts new file mode 100644 index 00000000000..dee6c9d8d25 --- /dev/null +++ b/apps/sim/hooks/use-before-unload-while.ts @@ -0,0 +1,19 @@ +'use client' + +import { useEffect } from 'react' + +/** + * Asks the browser to confirm before the page unloads while `active` is true, using the + * standard `beforeunload` prompt. The desktop app's update restart probes this same + * event, so guarded work also holds an update restart for confirmation. + */ +export function useBeforeUnloadWhile(active: boolean) { + useEffect(() => { + if (!active) return + const handleBeforeUnload = (event: BeforeUnloadEvent) => { + event.preventDefault() + } + window.addEventListener('beforeunload', handleBeforeUnload) + return () => window.removeEventListener('beforeunload', handleBeforeUnload) + }, [active]) +} diff --git a/bun.lock b/bun.lock index 6bf87cbab8c..9eed2f057c8 100644 --- a/bun.lock +++ b/bun.lock @@ -59,6 +59,7 @@ "@sim/utils": "workspace:*", "@xterm/headless": "6.0.0", "electron-updater": "6.8.9", + "lru-cache": "11.5.3", "micromatch": "4.0.8", "pdf-lib": "1.17.1", "safe-regex2": "5.1.2", diff --git a/packages/browser-protocol/src/index.ts b/packages/browser-protocol/src/index.ts index 962192bbb82..eaf488f2c5a 100644 --- a/packages/browser-protocol/src/index.ts +++ b/packages/browser-protocol/src/index.ts @@ -335,7 +335,11 @@ export interface BrowserPanelAction { * Anything but an accepted answer of the request's own kind is a refusal. */ export type BrowserPagePromptResponse = - | { allowed: false } + /** + * `remember` marks a deliberate refusal the shell may keep for the site, such as + * "Don't send" on a certificate picker; closing the prompt omits it and only cancels. + */ + | { allowed: false; remember?: boolean } | { allowed: true; kind: 'http-auth'; username: string; password: string } | { allowed: true; kind: 'passkey-account' | 'client-certificate'; choiceId: string } /** `alwaysAllow` stops asking for this site and app until restart. */ From 9c94992e962a1e8b841ee9b1f6347a1581519539 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Sat, 31 Oct 2026 11:18:23 -0700 Subject: [PATCH 2/2] fix(uploads): guard in-flight uploads from the workspace shell, not the page that started them Uploads keep sending after in-app navigation unmounts their page, so the beforeunload guard now reads a shared in-flight upload count from one guard mounted in the workspace layout. The credential detail guard reuses useBeforeUnloadWhile, and Continue in Browser reads the page origin only in the desktop shell. --- .../hooks/use-unsaved-changes-guard.ts | 12 ++---- .../workspace/[workspaceId]/files/files.tsx | 16 ++++--- .../browser-session/use-continue-sign-in.ts | 3 +- .../knowledge/hooks/use-knowledge-upload.ts | 5 ++- .../app/workspace/[workspaceId]/layout.tsx | 2 + .../providers/in-flight-upload-guard.tsx | 14 +++++++ .../import-csv-dialog/import-csv-dialog.tsx | 32 +++++++------- .../workspace/[workspaceId]/tables/tables.tsx | 5 ++- .../stores/in-flight-uploads/store.test.ts | 26 ++++++++++++ apps/sim/stores/in-flight-uploads/store.ts | 42 +++++++++++++++++++ 10 files changed, 122 insertions(+), 35 deletions(-) create mode 100644 apps/sim/app/workspace/[workspaceId]/providers/in-flight-upload-guard.tsx create mode 100644 apps/sim/stores/in-flight-uploads/store.test.ts create mode 100644 apps/sim/stores/in-flight-uploads/store.ts diff --git a/apps/sim/app/workspace/[workspaceId]/components/credential-detail/hooks/use-unsaved-changes-guard.ts b/apps/sim/app/workspace/[workspaceId]/components/credential-detail/hooks/use-unsaved-changes-guard.ts index 1dd0bb241bc..b176ccdde08 100644 --- a/apps/sim/app/workspace/[workspaceId]/components/credential-detail/hooks/use-unsaved-changes-guard.ts +++ b/apps/sim/app/workspace/[workspaceId]/components/credential-detail/hooks/use-unsaved-changes-guard.ts @@ -3,6 +3,7 @@ import type { MouseEvent } from 'react' import { useCallback, useEffect, useRef, useState } from 'react' import { useRouter } from 'next/navigation' +import { useBeforeUnloadWhile } from '@/hooks/use-before-unload-while' interface UseUnsavedChangesGuardParams { isDirty: boolean @@ -29,6 +30,8 @@ export function useUnsavedChangesGuard({ isDirty, backHref }: UseUnsavedChangesG const [isReleased, setIsReleased] = useState(false) const hasSentinelRef = useRef(false) + useBeforeUnloadWhile(isDirty && !isReleased) + useEffect(() => { // The caller is navigating away — popping the seeded entry would cancel it. But // Back during that window consumes the entry with no listener left to re-push @@ -58,19 +61,12 @@ export function useUnsavedChangesGuard({ isDirty, backHref }: UseUnsavedChangesG window.history.pushState(null, '', window.location.href) hasSentinelRef.current = true } - const handleBeforeUnload = (event: BeforeUnloadEvent) => { - event.preventDefault() - } const handlePopState = () => { window.history.pushState(null, '', window.location.href) setShowUnsavedAlert(true) } - window.addEventListener('beforeunload', handleBeforeUnload) window.addEventListener('popstate', handlePopState) - return () => { - window.removeEventListener('beforeunload', handleBeforeUnload) - window.removeEventListener('popstate', handlePopState) - } + return () => window.removeEventListener('popstate', handlePopState) }, [isDirty, isReleased]) const handleBackClick = useCallback( diff --git a/apps/sim/app/workspace/[workspaceId]/files/files.tsx b/apps/sim/app/workspace/[workspaceId]/files/files.tsx index 931234bc5a2..1932875c56c 100644 --- a/apps/sim/app/workspace/[workspaceId]/files/files.tsx +++ b/apps/sim/app/workspace/[workspaceId]/files/files.tsx @@ -163,7 +163,6 @@ import { useWorkspaceFileRecord, workspaceFileRecordState, } from '@/hooks/queries/workspace-files' -import { useBeforeUnloadWhile } from '@/hooks/use-before-unload-while' import { useContextMenu } from '@/hooks/use-context-menu' import { useDebouncedSearchSetter } from '@/hooks/use-debounced-search-setter' import { useInlineRename } from '@/hooks/use-inline-rename' @@ -171,6 +170,7 @@ import { usePermissionConfig } from '@/hooks/use-permission-config' import { useResourceListPreferences } from '@/hooks/use-resource-list-preferences' import { useSearchFilterValue } from '@/hooks/use-search-filter-value' import { useUrlSort } from '@/hooks/use-url-sort' +import { useInFlightUploadsStore } from '@/stores/in-flight-uploads/store' import type { ResourceListPreference } from '@/stores/resource-list-preferences' type SaveStatus = 'idle' | 'saving' | 'saved' | 'error' @@ -354,7 +354,6 @@ function FilesContent() { }) /** An upload batch is in flight exactly while a total is set — matches the Tables page. */ const uploading = uploadProgress.total > 0 - useBeforeUnloadWhile(uploading) const [isDraggingOver, setIsDraggingOver] = useState(false) const dragCounterRef = useRef(0) /** @@ -978,10 +977,15 @@ function FilesContent() { setUploadProgress({ completed: 0, total: 0, currentPercent: 0 }) } } - uploadQueueRef.current = uploadQueueRef.current.then(uploadBatch).catch((error) => { - logger.error('Error uploading files:', error) - toast.error(getErrorMessage(error, 'Failed to upload files')) - }) + // Counted from the moment it queues, since the batch runs on after this page unmounts. + const releaseUpload = useInFlightUploadsStore.getState().begin() + uploadQueueRef.current = uploadQueueRef.current + .then(uploadBatch) + .catch((error) => { + logger.error('Error uploading files:', error) + toast.error(getErrorMessage(error, 'Failed to upload files')) + }) + .finally(releaseUpload) return uploadQueueRef.current } diff --git a/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/components/browser-session/use-continue-sign-in.ts b/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/components/browser-session/use-continue-sign-in.ts index 340e840262e..a70d1979562 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/components/browser-session/use-continue-sign-in.ts +++ b/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/components/browser-session/use-continue-sign-in.ts @@ -58,7 +58,8 @@ export function useContinueSignIn({ scopeId, pageUrl }: UseContinueSignInProps) const { mutate: cancelHandoff } = useCancelSignInHandoff(scopeId) useSyncExternalStore(subscribeToDismissals, dismissalCount, dismissalCount) - const page = continuablePage(pageUrl, typeof window === 'undefined' ? '' : window.location.origin) + // Server rendering has no origin to compare and no shell to continue in, so it offers nothing. + const page = supported ? continuablePage(pageUrl, window.location.origin) : null // The profiles cache is shared with the import dialog, which older shells // also support, so cached profiles alone must not reveal this feature. const canContinue = supported && profiles.length > 0 && page !== null diff --git a/apps/sim/app/workspace/[workspaceId]/knowledge/hooks/use-knowledge-upload.ts b/apps/sim/app/workspace/[workspaceId]/knowledge/hooks/use-knowledge-upload.ts index c2ef37a9c9f..81c5ed3e678 100644 --- a/apps/sim/app/workspace/[workspaceId]/knowledge/hooks/use-knowledge-upload.ts +++ b/apps/sim/app/workspace/[workspaceId]/knowledge/hooks/use-knowledge-upload.ts @@ -12,7 +12,7 @@ import { runWithConcurrency, WHOLE_FILE_PARALLEL_UPLOADS } from '@/lib/uploads/c import { uploadKnowledgeDocumentSession } from '@/lib/uploads/client/session-upload' import type { UploadProgressEvent } from '@/lib/uploads/client/types' import { knowledgeKeys } from '@/hooks/queries/utils/knowledge-keys' -import { useBeforeUnloadWhile } from '@/hooks/use-before-unload-while' +import { useInFlightUploadsStore } from '@/stores/in-flight-uploads/store' const logger = createLogger('KnowledgeUpload') @@ -79,7 +79,6 @@ export function useKnowledgeUpload(options: UseKnowledgeUploadOptions = {}) { }) const [uploadError, setUploadError] = useState(null) const isUploading = uploadProgress.stage !== 'idle' - useBeforeUnloadWhile(isUploading) const updateFileStatus = (fileIndex: number, patch: Partial) => { setUploadProgress((prev) => ({ @@ -206,6 +205,7 @@ export function useKnowledgeUpload(options: UseKnowledgeUploadOptions = {}) { throw new KnowledgeUploadError('Knowledge base ID is required', 'INVALID_KB_ID') } + const releaseUpload = useInFlightUploadsStore.getState().begin() try { assertMultiFileUploadAdmission(files) setUploadError(null) @@ -248,6 +248,7 @@ export function useKnowledgeUpload(options: UseKnowledgeUploadOptions = {}) { options.onError?.(error) throw err } finally { + releaseUpload() setUploadProgress({ stage: 'idle', filesCompleted: 0, totalFiles: 0 }) } } diff --git a/apps/sim/app/workspace/[workspaceId]/layout.tsx b/apps/sim/app/workspace/[workspaceId]/layout.tsx index df091963571..b43f3647e73 100644 --- a/apps/sim/app/workspace/[workspaceId]/layout.tsx +++ b/apps/sim/app/workspace/[workspaceId]/layout.tsx @@ -37,6 +37,7 @@ import { CustomBlocksLoader } from '@/app/workspace/[workspaceId]/providers/cust import { DesktopOAuthConnectListener } from '@/app/workspace/[workspaceId]/providers/desktop-oauth-connect-listener' import { FeatureFlagsProvider } from '@/app/workspace/[workspaceId]/providers/feature-flags-provider' import { GlobalCommandsProvider } from '@/app/workspace/[workspaceId]/providers/global-commands-provider' +import { InFlightUploadGuard } from '@/app/workspace/[workspaceId]/providers/in-flight-upload-guard' import { ProviderModelsLoader } from '@/app/workspace/[workspaceId]/providers/provider-models-loader' import { SettingsLoader } from '@/app/workspace/[workspaceId]/providers/settings-loader' import { WorkspaceHostProvider } from '@/app/workspace/[workspaceId]/providers/workspace-host-provider' @@ -177,6 +178,7 @@ async function WorkspaceContent({ initialOrgSettings={initialOrgSettings} > + diff --git a/apps/sim/app/workspace/[workspaceId]/providers/in-flight-upload-guard.tsx b/apps/sim/app/workspace/[workspaceId]/providers/in-flight-upload-guard.tsx new file mode 100644 index 00000000000..533c328cb74 --- /dev/null +++ b/apps/sim/app/workspace/[workspaceId]/providers/in-flight-upload-guard.tsx @@ -0,0 +1,14 @@ +'use client' + +import { useBeforeUnloadWhile } from '@/hooks/use-before-unload-while' +import { useInFlightUploadsStore } from '@/stores/in-flight-uploads/store' + +/** + * Asks before the page unloads while any upload is still sending. Mounted once in + * the workspace layout, so it outlives the page that started the upload. + */ +export function InFlightUploadGuard() { + const uploading = useInFlightUploadsStore((state) => state.count > 0) + useBeforeUnloadWhile(uploading) + return null +} diff --git a/apps/sim/app/workspace/[workspaceId]/tables/components/import-csv-dialog/import-csv-dialog.tsx b/apps/sim/app/workspace/[workspaceId]/tables/components/import-csv-dialog/import-csv-dialog.tsx index 2741794cf07..7868f9468af 100644 --- a/apps/sim/app/workspace/[workspaceId]/tables/components/import-csv-dialog/import-csv-dialog.tsx +++ b/apps/sim/app/workspace/[workspaceId]/tables/components/import-csv-dialog/import-csv-dialog.tsx @@ -33,7 +33,7 @@ import { } from '@/lib/table/import' import type { TableDefinition } from '@/lib/table/types' import { type CsvImportMode, useImportCsvIntoTable } from '@/hooks/queries/tables' -import { useBeforeUnloadWhile } from '@/hooks/use-before-unload-while' +import { trackUpload } from '@/stores/in-flight-uploads/store' import { useImportTrayStore } from '@/stores/table/import-tray/store' const logger = createLogger('ImportCsvDialog') @@ -147,7 +147,6 @@ export function ImportCsvDialog({ const [createHeaders, setCreateHeaders] = useState>(new Set()) const [mode, setMode] = useState('append') const importMutation = useImportCsvIntoTable() - useBeforeUnloadWhile(importMutation.isPending) function resetState() { setParsed(null) @@ -317,8 +316,10 @@ export function ImportCsvDialog({ let importId: string | null = null onOpenChange(false) toast.success(`Importing "${parsed.file.name}" into "${table.name}" in the background`) - importMutation.mutate( - { + // The dialog closes before the upload ends, so its outcome is handled on the promise, + // which settles even after the dialog unmounts. + trackUpload(() => + importMutation.mutateAsync({ workspaceId, tableId: table.id, file: parsed.file, @@ -337,19 +338,18 @@ export function ImportCsvDialog({ onProgress: (percent) => { if (importId) useImportTrayStore.getState().setUploadPercent(importId, percent) }, + }) + ).then( + () => { + if (importId) { + useImportTrayStore.getState().endUpload(importId) + useImportTrayStore.getState().consumeCanceled(importId) + } + onImported?.({}) }, - { - onSuccess: () => { - if (importId) { - useImportTrayStore.getState().endUpload(importId) - useImportTrayStore.getState().consumeCanceled(importId) - } - onImported?.({}) - }, - onError: (error) => { - if (importId) useImportTrayStore.getState().endUpload(importId) - setSubmitError(summarizeImportError(error.message)) - }, + (error: unknown) => { + if (importId) useImportTrayStore.getState().endUpload(importId) + setSubmitError(summarizeImportError(getErrorMessage(error))) } ) } diff --git a/apps/sim/app/workspace/[workspaceId]/tables/tables.tsx b/apps/sim/app/workspace/[workspaceId]/tables/tables.tsx index 6c16bf03ebd..75d0c5ce977 100644 --- a/apps/sim/app/workspace/[workspaceId]/tables/tables.tsx +++ b/apps/sim/app/workspace/[workspaceId]/tables/tables.tsx @@ -101,7 +101,6 @@ import { } from '@/hooks/queries/tables' import { getCanonicalFolderPath } from '@/hooks/queries/utils/folder-tree' import { useWorkspaceMembersQuery, type WorkspaceMember } from '@/hooks/queries/workspace' -import { useBeforeUnloadWhile } from '@/hooks/use-before-unload-while' import { useContextMenu } from '@/hooks/use-context-menu' import { useDebouncedSearchSetter } from '@/hooks/use-debounced-search-setter' import { useInlineRename } from '@/hooks/use-inline-rename' @@ -110,6 +109,7 @@ import { useResourceListPreferences } from '@/hooks/use-resource-list-preference import { useSearchFilterValue } from '@/hooks/use-search-filter-value' import { useUrlSort } from '@/hooks/use-url-sort' import type { WorkflowFolder } from '@/stores/folders/types' +import { useInFlightUploadsStore } from '@/stores/in-flight-uploads/store' import type { ResourceListPreference } from '@/stores/resource-list-preferences' import { useImportTrayStore } from '@/stores/table/import-tray/store' @@ -327,7 +327,6 @@ function TablesContent() { const [uploadProgress, setUploadProgress] = useState({ completed: 0, total: 0 }) const uploading = uploadProgress.total > 0 - useBeforeUnloadWhile(uploading) const csvInputRef = useRef(null) /** @@ -1177,6 +1176,7 @@ function TablesContent() { return } + const releaseUpload = useInFlightUploadsStore.getState().begin() try { setUploadProgress({ completed: 0, total: csvFiles.length }) for (let index = 0; index < csvFiles.length; index++) { @@ -1214,6 +1214,7 @@ function TablesContent() { logger.error('Error uploading CSV:', err) toast.error('Failed to import CSV') } finally { + releaseUpload() setUploadProgress({ completed: 0, total: 0 }) if (csvInputRef.current) { csvInputRef.current.value = '' diff --git a/apps/sim/stores/in-flight-uploads/store.test.ts b/apps/sim/stores/in-flight-uploads/store.test.ts new file mode 100644 index 00000000000..c0cf279fddd --- /dev/null +++ b/apps/sim/stores/in-flight-uploads/store.test.ts @@ -0,0 +1,26 @@ +import { afterEach, describe, expect, it } from 'vitest' +import { trackUpload, useInFlightUploadsStore } from '@/stores/in-flight-uploads/store' + +const count = () => useInFlightUploadsStore.getState().count + +afterEach(() => { + useInFlightUploadsStore.setState({ count: 0 }) +}) + +describe('in-flight uploads', () => { + it('stops guarding a failed upload, so a later close is not held forever', async () => { + await expect(trackUpload(() => Promise.reject(new Error('storage limit')))).rejects.toThrow( + 'storage limit' + ) + expect(count()).toBe(0) + }) + + it('keeps guarding another upload when one release runs twice', () => { + const { begin } = useInFlightUploadsStore.getState() + const first = begin() + begin() + first() + first() + expect(count()).toBe(1) + }) +}) diff --git a/apps/sim/stores/in-flight-uploads/store.ts b/apps/sim/stores/in-flight-uploads/store.ts new file mode 100644 index 00000000000..609dedf0533 --- /dev/null +++ b/apps/sim/stores/in-flight-uploads/store.ts @@ -0,0 +1,42 @@ +import { create } from 'zustand' +import { devtools } from 'zustand/middleware' + +interface InFlightUploadsState { + /** Uploads started and not yet settled, across every page of the app. */ + count: number + /** Counts one upload as in flight; the returned release ends it, and only the first call counts. */ + begin: () => () => void +} + +/** + * Uploads that outlive the page that started them: a mutation keeps sending + * after in-app navigation unmounts its component, so the unload guard that + * protects them reads this count from the workspace shell, not from the page. + */ +export const useInFlightUploadsStore = create()( + devtools( + (set) => ({ + count: 0, + begin: () => { + set((state) => ({ count: state.count + 1 }), false, 'begin') + let released = false + return () => { + if (released) return + released = true + set((state) => ({ count: state.count - 1 }), false, 'release') + } + }, + }), + { name: 'in-flight-uploads-store' } + ) +) + +/** Counts `work` as an in-flight upload until it settles, whether it resolves or rejects. */ +export async function trackUpload(work: () => Promise): Promise { + const release = useInFlightUploadsStore.getState().begin() + try { + return await work() + } finally { + release() + } +}