diff --git a/.github/workflows/desktop-e2e.yml b/.github/workflows/desktop-e2e.yml index eb5d1316f00..3ea5df89904 100644 --- a/.github/workflows/desktop-e2e.yml +++ b/.github/workflows/desktop-e2e.yml @@ -10,7 +10,11 @@ on: - '.github/workflows/desktop-e2e.yml' - '.github/workflows/desktop-release.yml' - 'apps/desktop/**' + - 'apps/sim/app/_styles/**' + - 'apps/sim/lib/postcss/**' + - 'apps/sim/postcss.config.mjs' - 'apps/sim/public/brand/fonts/**' + - 'packages/emcn/**' - 'packages/desktop-bridge/**' - 'packages/browser-protocol/**' - 'packages/terminal-protocol/**' diff --git a/.github/workflows/migrations.yml b/.github/workflows/migrations.yml index 0235a0b8342..faa2b464522 100644 --- a/.github/workflows/migrations.yml +++ b/.github/workflows/migrations.yml @@ -25,7 +25,8 @@ jobs: migrate: name: Apply Database Migrations runs-on: ${{ (vars.CI_PROVIDER == '' || vars.CI_PROVIDER == 'blacksmith') && 'blacksmith-4vcpu-ubuntu-2404' || 'ubuntu-latest' }} - timeout-minutes: 45 + # Bulk projection loads and concurrent index builds can outlast ordinary schema changes. + timeout-minutes: 300 steps: - name: Checkout code diff --git a/.github/workflows/test-build.yml b/.github/workflows/test-build.yml index 6ad195ff86d..5fccc030659 100644 --- a/.github/workflows/test-build.yml +++ b/.github/workflows/test-build.yml @@ -96,6 +96,7 @@ jobs: lib/auth/sim-auth-adapter.postgres.test.ts ee/scim/lib/managed-membership.postgres.test.ts lib/auth/sso/application/admit-sso-user.postgres.test.ts + lib/auth/sso/primary-provider.postgres.test.ts - name: Verify cumulative billing timeout recovery in PostgreSQL working-directory: apps/sim @@ -170,15 +171,23 @@ jobs: if-no-files-found: ignore retention-days: 7 - - name: Verify durable provenance bindings and concurrent memory writes + - name: Verify durable provenance, concurrent memory writes, and attachment replay working-directory: apps/sim env: TABLE_PROVENANCE_TEST_DATABASE_URL: postgresql://postgres:postgres@127.0.0.1:5432/sim_auth_scim MEMORY_PROVENANCE_TEST_DATABASE_URL: postgresql://postgres:postgres@127.0.0.1:5432/sim_auth_scim + AGENT_MEMORY_TEST_DATABASE_URL: postgresql://postgres:postgres@127.0.0.1:5432/sim_auth_scim run: >- bunx vitest run lib/table/rows/secret-provenance.postgres.test.ts lib/memory/message-provenance.postgres.test.ts + executor/handlers/agent/memory-harness.postgres.test.ts + + - name: Verify Search vector projection upgrade in PostgreSQL + working-directory: packages/db + env: + KNOWLEDGE_ACL_TEST_DATABASE_URL: postgresql://postgres:postgres@127.0.0.1:5432/sim_auth_scim + run: bunx vitest run script-migrations/0016_backfill_search_vectors.postgres.test.ts - name: Verify Search progress, pagination, and outbox scheduling in PostgreSQL working-directory: apps/sim diff --git a/apps/desktop/e2e/browser-tools.spec.ts b/apps/desktop/e2e/browser-tools.spec.ts index 55e8c7021de..d7edc1bf521 100644 --- a/apps/desktop/e2e/browser-tools.spec.ts +++ b/apps/desktop/e2e/browser-tools.spec.ts @@ -21,6 +21,7 @@ const FORM = `Form fixtureUpdates + Other website
Wide content
@@ -40,6 +41,11 @@ test.describe('browser tools', () => { test.beforeAll(async () => { server = createServer(async (request, response) => { const path = new URL(request.url ?? '/', 'http://127.0.0.1').pathname + if (path === '/redirect') { + response.writeHead(302, { Location: `${origin.replace('127.0.0.1', 'localhost')}/landing` }) + response.end() + return + } if (path === '/api/desktop/tool/authorize') { let body = '' for await (const chunk of request) body += chunk.toString() @@ -189,6 +195,28 @@ test.describe('browser tools', () => { expect(await formState()).toMatchObject({ name: '', route: 'change route' }) }) + test('follows a link and cross-origin redirect without a website approval prompt', async () => { + await openForm() + await app.evaluate(async ({ webContents }, url) => { + const page = webContents.getAllWebContents().find((contents) => contents.getURL() === url) + if (!page) throw new Error('Missing browser fixture') + await page.executeJavaScript("document.querySelector('a').click()") + }, `${origin}/form`) + + const destination = `${origin.replace('127.0.0.1', 'localhost')}/landing` + await expect + .poll(() => + app.evaluate( + ({ webContents }, url) => + webContents.getAllWebContents().some((contents) => contents.getURL() === url), + destination + ) + ) + .toBe(true) + expect(await app.evaluate(({ BrowserWindow }) => BrowserWindow.getAllWindows().length)).toBe(1) + await expect(window.getByRole('heading')).toHaveText('Browser tools fixture') + }) + test('stops when a new popup exceeds the page summary limit', async () => { const ref = await openForm() await app.evaluate(async ({ webContents }, origin) => { diff --git a/apps/desktop/e2e/packaged-smoke.spec.ts b/apps/desktop/e2e/packaged-smoke.spec.ts index f4010e8abfa..84cefa543bc 100644 --- a/apps/desktop/e2e/packaged-smoke.spec.ts +++ b/apps/desktop/e2e/packaged-smoke.spec.ts @@ -146,8 +146,8 @@ test('packaged shell renders the bundled offline page', async () => { .toBe(true) const picker = findPage('sim-shell://pages/server.html') if (!picker) throw new Error('server picker disappeared') - await expect(picker.locator('h1')).toHaveText('Sim server') - await expect(picker.locator('#origin')).toHaveValue('http://127.0.0.1:1') + await expect(picker.getByRole('dialog', { name: 'Sim server' })).toBeVisible() + await expect(picker.getByLabel('Server URL')).toHaveValue('http://127.0.0.1:1') } finally { await browser?.close().catch(() => {}) if (child.exitCode === null && child.signalCode === null) { diff --git a/apps/desktop/e2e/smoke.spec.ts b/apps/desktop/e2e/smoke.spec.ts index df17dd4f51f..578bf6e78f0 100644 --- a/apps/desktop/e2e/smoke.spec.ts +++ b/apps/desktop/e2e/smoke.spec.ts @@ -158,8 +158,11 @@ test.describe('desktop shell smoke', () => { const window = await app.firstWindow() await window.waitForSelector('#retry', { timeout: 30_000 }) expect(window.url()).toMatch(/^sim-shell:\/\/pages\/offline\.html\?/) - await expect(window.locator('.wordmark')).toBeVisible() - await expect(window.locator('.wordmark')).toHaveAttribute('aria-label', 'Sim') + await expect(window.getByRole('img', { name: 'Sim', exact: true })).toBeVisible() + await expect(window.getByRole('img', { name: 'Sim', exact: true })).toHaveAttribute( + 'aria-label', + 'Sim' + ) await expect(window.locator('#title')).toHaveText('Can’t connect to Sim') // The recovery path for a self-hosted shell pointed at a server it cannot // reach. Exercised end to end here because it is the only coverage of the @@ -173,22 +176,74 @@ test.describe('desktop shell smoke', () => { await expect .poll(() => window.evaluate(() => document.fonts.check('16px "Season Sans"'))) .toBe(true) - await expect(window.locator('#retry')).toHaveCSS('height', '30px') - await expect(window.locator('#retry')).toHaveCSS('border-radius', '8px') - await expect(window.locator('#retry')).toHaveCSS('padding-left', '8px') - await expect(window.locator('#retry')).toHaveCSS('font-size', '14px') - await expect(window.locator('#retry')).toHaveCSS('line-height', '20px') - await expect(window.locator('#retry')).toHaveCSS('text-align', 'left') - await window.locator('#retry').focus() - await expect(window.locator('#retry')).toHaveCSS('outline-style', 'solid') await expect(window.locator('#detail')).toHaveAttribute('role', 'status') }) + test('recovery messages use an isolated EMCN dialog with a safe keyboard default', async () => { + app = await launchApp('http://127.0.0.1:1') + const window = await app.firstWindow() + await expect(window.locator('#server')).toBeVisible() + const dialogPromise = app.waitForEvent('window') + await app.evaluate(({ BrowserWindow }) => { + BrowserWindow.getAllWindows()[0].webContents.emit('unresponsive') + }) + const prompt = await dialogPromise + await expect(prompt.getByRole('dialog', { name: 'Sim', exact: true })).toBeVisible() + await expect(prompt.getByText('Sim isn’t responding')).toBeVisible() + await expect(prompt.getByRole('button', { name: 'Wait', exact: true })).toBeFocused() + await expect + .poll(() => + prompt + .getByRole('dialog') + .evaluate((element) => element.scrollHeight <= globalThis.innerHeight) + ) + .toBe(true) + await expect + .poll(() => prompt.evaluate(() => typeof (globalThis as { simDesktop?: unknown }).simDesktop)) + .toBe('undefined') + await prompt.screenshot({ + path: test.info().outputPath('recovery-dialog.png'), + animations: 'disabled', + }) + await app.evaluate(({ BrowserWindow }) => { + const win = BrowserWindow.getAllWindows().find( + (entry) => entry.webContents.getURL() === 'sim-shell://pages/dialog.html' + ) + if (!win) throw new Error('Recovery dialog is missing') + win.webContents.ipc.removeHandler('shell:configuration') + win.webContents.ipc.handle('shell:configuration', () => ({ + title: 'Long recovery message', + message: 'Recovery details', + detail: Array.from({ length: 80 }, (_, index) => `Diagnostic detail ${index + 1}`).join( + '\n' + ), + type: 'warning', + buttons: ['Wait', 'Reload'], + defaultId: 0, + cancelId: 0, + })) + win.webContents.reload() + }) + await expect( + prompt.getByRole('dialog', { name: 'Long recovery message', exact: true }) + ).toBeVisible() + await expect(prompt.getByRole('button', { name: 'Reload', exact: true })).toBeInViewport() + await expect(prompt.getByRole('button', { name: 'Wait', exact: true })).toBeFocused() + const closed = prompt.waitForEvent('close') + await prompt + .getByRole('button', { name: 'Wait', exact: true }) + .press('Enter') + .catch(() => {}) + await closed + await expect(window.locator('#server')).toBeVisible() + }) + // The picker is the only way to repoint a shell whose server is unreachable. // Its page, the pre-filled value (which crosses the local-page IPC gate) and // Escape are asserted together because the packaged build once opened it as // a blank sheet with no way out. - test('the offline page opens the server picker, pre-filled, and Escape closes it', async () => { + test('the offline server picker renders EMCN controls and handles validation and dismissal', async () => { + const testInfo = test.info() app = await launchApp('http://127.0.0.1:1') const window = await app.firstWindow() await window.waitForSelector('#server', { timeout: 30_000 }) @@ -198,8 +253,42 @@ test.describe('desktop shell smoke', () => { const picker = await pickerPromise expect(picker.url()).toBe('sim-shell://pages/server.html') - await expect(picker.locator('h1')).toHaveText('Sim server') - await expect(picker.locator('#origin')).toHaveValue('http://127.0.0.1:1') + await expect(picker.getByRole('dialog', { name: 'Sim server', exact: true })).toBeVisible() + await expect(picker.getByLabel('Server URL')).toHaveValue('http://127.0.0.1:1') + await expect(picker.getByLabel('Server URL')).toBeFocused() + await expect + .poll(() => + picker + .getByRole('dialog') + .evaluate((element) => element.scrollHeight <= globalThis.innerHeight) + ) + .toBe(true) + await picker.getByLabel('Server URL').fill('http://example.com') + await picker.getByLabel('Server URL').press('Enter') + await expect(picker.getByRole('alert')).toBeVisible() + await expect(picker.getByLabel('Server URL')).toHaveAttribute('aria-invalid', 'true') + await picker.getByLabel('Server URL').fill('http://127.0.0.1:1') + await expect(picker.getByRole('alert')).toHaveCount(0) + await picker.getByRole('button', { name: 'Connect', exact: true }).click() + await expect(picker.getByRole('status')).toHaveText('Already connected to this server.') + await expect + .poll(() => + picker + .locator('[data-chip-modal-body]') + .evaluate((element) => element.scrollHeight <= element.clientHeight) + ) + .toBe(true) + await picker.emulateMedia({ colorScheme: 'light' }) + await picker.screenshot({ + path: testInfo.outputPath('server-modal-light.png'), + animations: 'disabled', + }) + await picker.emulateMedia({ colorScheme: 'dark' }) + await expect(picker.locator('html')).toHaveClass('dark') + await picker.screenshot({ + path: testInfo.outputPath('server-modal-dark.png'), + animations: 'disabled', + }) const closed = picker.waitForEvent('close') // The main process destroys the window on the key-down, so the key-up half diff --git a/apps/desktop/package.json b/apps/desktop/package.json index c90de11d4bd..f4c24dee88c 100644 --- a/apps/desktop/package.json +++ b/apps/desktop/package.json @@ -3,7 +3,7 @@ "version": "0.0.0", "private": true, "license": "Apache-2.0", - "description": "Sim desktop app for macOS — Electron shell around the hosted web app", + "description": "Sim desktop app for macOS \u2014 Electron shell around the hosted web app", "author": "Sim ", "homepage": "https://sim.ai", "type": "module", @@ -47,13 +47,20 @@ "devDependencies": { "@electron/fuses": "1.8.0", "@playwright/test": "1.61.1", + "@sim/emcn": "workspace:*", "@sim/tsconfig": "workspace:*", "@types/micromatch": "4.0.10", "@types/node": "24.2.1", + "@types/react": "^19", + "@types/react-dom": "^19", "electron": "43.5.0", "electron-builder": "26.15.3", "esbuild": "0.28.1", "jsdom": "^26.0.0", + "postcss": "^8", + "postcss-load-config": "6.0.1", + "react": "19.2.4", + "react-dom": "19.2.4", "typescript": "^7.0.2", "vitest": "^4.1.0" } diff --git a/apps/desktop/scripts/build.ts b/apps/desktop/scripts/build.ts index f2b553661d2..2f009c58087 100644 --- a/apps/desktop/scripts/build.ts +++ b/apps/desktop/scripts/build.ts @@ -1,7 +1,9 @@ import { execFileSync } from 'node:child_process' -import { cpSync, existsSync, mkdirSync, rmSync } from 'node:fs' -import { dirname, join } from 'node:path' -import { build } from 'esbuild' +import { cpSync, existsSync, mkdirSync, readFileSync, rmSync } from 'node:fs' +import { dirname, join, resolve } from 'node:path' +import { type BuildOptions, build } from 'esbuild' +import postcss from 'postcss' +import loadPostcssConfig from 'postcss-load-config' import { identityForOrigin } from './channels' const watch = process.argv.includes('--watch') @@ -94,10 +96,56 @@ const common = { }, } +/** Bundles the shared EMCN components and app tokens for offline shell use. */ +const renderer: BuildOptions = { + entryPoints: { + server: 'src/renderer/server/index.tsx', + offline: 'src/renderer/offline/index.tsx', + dialog: 'src/renderer/dialog/index.tsx', + }, + outdir: 'dist/renderer', + bundle: true, + platform: 'browser', + format: 'iife', + target: 'chrome146', + minify: true, + tsconfig: 'tsconfig.json', + external: ['*.woff2'], + define: { 'process.env.NODE_ENV': '"production"', 'process.env': '{}' }, + loader: { '.module.css': 'local-css' }, + plugins: [ + { + name: 'desktop-tailwind', + setup(builder) { + builder.onLoad({ filter: /shell\.css$/ }, async ({ path }) => { + const config = await loadPostcssConfig({}, resolve('../sim')) + const result = await postcss(config.plugins).process(readFileSync(path, 'utf8'), { + from: path, + }) + return { + contents: result.css, + loader: 'css', + resolveDir: dirname(path), + watchFiles: result.messages.flatMap((message) => + message.type === 'dependency' ? [message.file as string] : [] + ), + } + }) + }, + }, + ], +} + async function run(): Promise { compileNativeHelpSearch() if (watch) { const { context } = await import('esbuild') + const rendererCtx = await context(renderer) + const shellPreloadCtx = await context({ + ...common, + entryPoints: ['src/preload/shell.ts'], + outfile: 'dist/shell-preload.cjs', + }) const mainCtx = await context({ ...common, entryPoints: ['src/main/index.ts'], @@ -115,10 +163,18 @@ async function run(): Promise { entryPoints: ['src/preload/browser/index.ts'], outfile: 'dist/browser-preload.cjs', }) - await Promise.all([mainCtx.watch(), preloadCtx.watch(), browserPreloadCtx.watch()]) + await Promise.all([ + mainCtx.watch(), + preloadCtx.watch(), + browserPreloadCtx.watch(), + rendererCtx.watch(), + shellPreloadCtx.watch(), + ]) return } await Promise.all([ + build(renderer), + build({ ...common, entryPoints: ['src/preload/shell.ts'], outfile: 'dist/shell-preload.cjs' }), build({ ...common, entryPoints: ['src/main/index.ts'], outfile: 'dist/main.cjs' }), build({ ...common, entryPoints: ['src/preload/index.ts'], outfile: 'dist/preload.cjs' }), build({ diff --git a/apps/desktop/src/main/browser-agent/driver.test.ts b/apps/desktop/src/main/browser-agent/driver.test.ts index bd5763cd810..5cb48a1cd3c 100644 --- a/apps/desktop/src/main/browser-agent/driver.test.ts +++ b/apps/desktop/src/main/browser-agent/driver.test.ts @@ -77,7 +77,7 @@ describe('executeTool', () => { }) it('validates navigation URLs before touching the session', async () => { - const grant = vi.spyOn(session, 'grantSiteOriginForAgentNavigation') + const prepare = vi.spyOn(session, 'prepareExplicitNavigation') const result = await driver.executeTool('chat-test', 'browser_navigate', { url: 'file:///etc/passwd', }) @@ -85,7 +85,7 @@ describe('executeTool', () => { ok: false, error: 'URL must be absolute and start with http:// or https://', }) - expect(grant).not.toHaveBeenCalled() + expect(prepare).not.toHaveBeenCalled() }) it('reports missing required parameters by name', async () => { @@ -94,8 +94,7 @@ describe('executeTool', () => { expect(result.error).toMatch(/Missing required parameter "url"/) }) - it('grants only SSRF-checked agent navigation destinations before loading them', async () => { - const grant = vi.spyOn(session, 'grantSiteOriginForAgentNavigation') + it('loads SSRF-checked agent navigation destinations', async () => { const navigations = [ ['browser_navigate', 'http://127.0.0.1:4011/navigate'], ['browser_open_url', 'http://127.0.0.1:4012/open'], @@ -106,9 +105,9 @@ describe('executeTool', () => { await expect(driver.executeTool('chat-test', tool, { url })).resolves.toMatchObject({ ok: true, }) - expect(grant).toHaveBeenCalledWith(expect.anything(), url) + const contents = session.requireAutomationTab().view.webContents + expect(contents.loadURL).toHaveBeenCalledWith(url) } - expect(grant).toHaveBeenCalledTimes(navigations.length) }) it('keeps the 400ms hydration grace without rediscovering a completed load', async () => { @@ -1140,8 +1139,8 @@ describe('executeTool', () => { expect(respond).toHaveBeenCalledWith('request-1', true) }) - it('routes an exact renderer site decision through the scoped session boundary', async () => { - const respond = vi.spyOn(session, 'respondToSitePermission').mockReturnValue(true) + it('ignores retired site decisions without changing tab ownership', async () => { + const claim = vi.spyOn(session, 'claimActiveTabForUser') await driver.handlePanelAction('chat-test', { action: 'respond-site-permission', @@ -1153,22 +1152,18 @@ describe('executeTool', () => { requestId: 'request-2', }) - expect(respond).toHaveBeenCalledOnce() - expect(respond).toHaveBeenCalledWith('request-1', true) + expect(claim).not.toHaveBeenCalled() }) - it('grants only the exact origin entered through the user omnibox', async () => { + it('loads the exact URL entered through the user omnibox', async () => { await driver.executeTool('chat-test', 'browser_open_tab', {}) const contents = session.requireTab().view.webContents - const grant = vi.spyOn(session, 'grantSiteOriginForUserNavigation') await driver.handlePanelAction('chat-test', { action: 'navigate', url: 'https://docs.example/private?token=secret', }) - expect(grant).toHaveBeenCalledOnce() - expect(grant).toHaveBeenCalledWith(contents, 'https://docs.example/private?token=secret') expect(contents.loadURL).toHaveBeenCalledWith('https://docs.example/private?token=secret') }) diff --git a/apps/desktop/src/main/browser-agent/driver.ts b/apps/desktop/src/main/browser-agent/driver.ts index 83d06102de3..807aaf81444 100644 --- a/apps/desktop/src/main/browser-agent/driver.ts +++ b/apps/desktop/src/main/browser-agent/driver.ts @@ -194,8 +194,6 @@ export interface DriverCallbacks { onPageState: (state: BrowserPageState) => void onTabsState: (state: BrowserTabsState) => void onSessionStatus: (alive: boolean, scopeId: string) => void - /** Whether a live renderer for the scope registered support for the consent prompt. */ - sitePermissionPromptSupported?: (scopeId: string) => boolean /** Whether the active tab shows a login form Sim holds a credential for. */ onFillAvailability: (available: boolean, scopeId: string) => void /** Live native download state for one isolated browser scope. */ @@ -467,7 +465,6 @@ function recordNotice(notice: string): void { function pageStateFor(contents: WebContents, tabId: string): BrowserPageState { const issue = session.pageIssueForContents(contents) const mediaPermissionRequest = session.mediaPermissionRequestForContents(contents) - const sitePermissionRequest = session.sitePermissionRequestForScope() return { scopeId: session.getBrowserScopeId(), tabId, @@ -478,7 +475,6 @@ function pageStateFor(contents: WebContents, tabId: string): BrowserPageState { canGoForward: session.canGoForward(contents), ...(issue ? { issue } : {}), ...(mediaPermissionRequest ? { mediaPermissionRequest } : {}), - ...(sitePermissionRequest ? { sitePermissionRequest } : {}), } } @@ -661,8 +657,6 @@ export function initDriver( void fillCoordinator()?.refreshAvailability(true) }, onPageStateChanged: pushPageState, - sitePermissionPromptSupported: (scopeId) => - driverCallbacks?.sitePermissionPromptSupported?.(scopeId) === true, onTabsChanged: pushTabsState, onTabThemeChanged: (contents, theme) => { void cdp.setColorScheme(contents, theme).catch((error) => { @@ -1373,7 +1367,7 @@ async function loadAgentCheckedUrlAndGetResult( url: string ): Promise> { session.prepareExplicitNavigation(contents) - if (!session.grantSiteOriginForAgentNavigation(contents, url)) { + if (contents.isDestroyed()) { throw new ToolError('The tab was closed before navigation could start.') } const beforeUrl = contents.getURL() @@ -4766,9 +4760,7 @@ export async function handlePanelAction( return } if (action.action === 'respond-site-permission') { - if (typeof action.requestId === 'string' && typeof action.allowed === 'boolean') { - session.respondToSitePermission(action.requestId, action.allowed) - } + /** Older renderers can still send a response to the retired task-navigation prompt. */ return } // Navigate bootstraps the session: the user can open the panel manually @@ -4779,7 +4771,6 @@ export async function handlePanelAction( session.claimActiveTabForUser() const contents = session.ensureTab().view.webContents session.prepareExplicitNavigation(contents) - session.grantSiteOriginForUserNavigation(contents, action.url) void contents.loadURL(action.url).catch(() => {}) } return diff --git a/apps/desktop/src/main/browser-agent/session.test.ts b/apps/desktop/src/main/browser-agent/session.test.ts index 2a0a48e58fd..39e1bc2683b 100644 --- a/apps/desktop/src/main/browser-agent/session.test.ts +++ b/apps/desktop/src/main/browser-agent/session.test.ts @@ -106,7 +106,6 @@ function freshSession( onTabCreated: vi.fn(), onActiveTabChanged: vi.fn(), onPageStateChanged: vi.fn(), - sitePermissionPromptSupported: vi.fn(() => true), onTabsChanged: vi.fn(), onTabThemeChanged: vi.fn(), onTabNavigated: vi.fn(), @@ -841,53 +840,64 @@ describe('browser-agent session', () => { } }) - it('extends an in-flight background restore without restarting its load', async () => { - vi.useFakeTimers() - try { - const tabs = Array.from({ length: 4 }, (_, index) => ({ - url: `https://active-restore-${index}.example/`, - })) - const { persistence } = memoryBrowserPersistence({ - 'chat-active-restore': { v: 1, tabs, activeIndex: 0, downloads: [] }, - }) - const createdContents: MockView['webContents'][] = [] - const selectedLoads: Array<() => void> = [] - session = freshSession( - win, - { - onTabCreated: (webContents) => { - const contents = webContents as unknown as MockView['webContents'] - const index = createdContents.push(contents) - 1 - contents.loadURL.mockImplementation( - () => - new Promise((resolve) => { - if (index === 1) selectedLoads.push(resolve) - }) - ) + it.each(['loaded', 'timed-out'] as const)( + 'gives a late foreground promotion its full loading window (%s)', + async (outcome) => { + vi.useFakeTimers() + try { + const tabs = Array.from({ length: 4 }, (_, index) => ({ + url: `https://active-restore-${index}.example/`, + })) + const { persistence } = memoryBrowserPersistence({ + 'chat-active-restore': { v: 1, tabs, activeIndex: 0, downloads: [] }, + }) + const createdContents: MockView['webContents'][] = [] + const selectedLoads: Array<() => void> = [] + session = freshSession( + win, + { + onTabCreated: (webContents) => { + const contents = webContents as unknown as MockView['webContents'] + const index = createdContents.push(contents) - 1 + contents.loadURL.mockImplementation( + () => + new Promise((resolve) => { + if (index === 1) selectedLoads.push(resolve) + }) + ) + }, }, - }, - persistence - ) - - const selected = session.withBrowserScope('chat-active-restore', () => { - session.restoreBrowserSession() - return session.switchAutomationTab('2') - }) - const selection = session.withBrowserScope('chat-active-restore', () => - session.waitForPendingTabRestore(selected) - ) + persistence + ) - expect(createdContents[1].loadURL).toHaveBeenCalledOnce() - expect(createdContents[1].stop).not.toHaveBeenCalled() - await vi.advanceTimersByTimeAsync(15_000) - expect(createdContents[1].stop).not.toHaveBeenCalled() + session.withBrowserScope('chat-active-restore', () => session.restoreBrowserSession()) + await vi.advanceTimersByTimeAsync(14_000) + const selected = session.withBrowserScope('chat-active-restore', () => + session.switchAutomationTab('2') + ) + const selection = session.withBrowserScope('chat-active-restore', () => + session.waitForPendingTabRestore(selected) + ) - selectedLoads[0]?.() - await expect(selection).resolves.toBe(true) - } finally { - vi.useRealTimers() + expect(createdContents[1].loadURL).toHaveBeenCalledOnce() + expect(createdContents[1].stop).not.toHaveBeenCalled() + await vi.advanceTimersByTimeAsync(19_999) + expect(createdContents[1].stop).not.toHaveBeenCalled() + + if (outcome === 'loaded') { + selectedLoads[0]?.() + await expect(selection).resolves.toBe(true) + } else { + session.withBrowserScope('chat-active-restore', () => session.switchAutomationTab('2')) + await vi.advanceTimersByTimeAsync(1) + await expect(selection).resolves.toBe(false) + expect(createdContents[1].stop).toHaveBeenCalledOnce() + } + } finally { + vi.useRealTimers() + } } - }) + ) it('queues a fifth foreground restore without preempting another foreground restore', async () => { const snapshots = Object.fromEntries( @@ -1031,123 +1041,39 @@ describe('browser-agent session', () => { } }) - it('gives a redirected background restore its complete site-decision window', async () => { + it('does not extend a background restore timeout for cross-origin redirects', async () => { vi.useFakeTimers() try { - const tabs = [ - { url: 'http://127.0.0.1:4601/active' }, - { url: 'http://127.0.0.1:4601/background' }, - ] const { persistence } = memoryBrowserPersistence({ - 'chat-stale-restore-prompt': { v: 1, tabs, activeIndex: 0, downloads: [] }, - }) - const createdContents: MockView['webContents'][] = [] - session = freshSession( - win, - { - onTabCreated: (webContents) => { - const contents = webContents as unknown as MockView['webContents'] - createdContents.push(contents) - contents.loadURL.mockImplementation(() => new Promise(() => {})) - }, + 'chat-test': { + v: 1, + tabs: [ + { url: 'http://127.0.0.1:4601/active' }, + { url: 'http://127.0.0.1:4601/background' }, + ], + activeIndex: 0, + downloads: [], }, - persistence - ) - - session.withBrowserScope('chat-stale-restore-prompt', () => session.restoreBrowserSession()) - session.activateBrowserScope('chat-stale-restore-prompt') - panel.setPanelBounds({ x: 100, y: 50, width: 800, height: 600 }) - const background = createdContents[1] - const redirected = beginMainFrameRequest(background, 'http://127.0.0.1:4602/redirect') - await vi.advanceTimersByTimeAsync(0) - expect( - session.withBrowserScope('chat-stale-restore-prompt', () => - session.sitePermissionRequestForScope() - ) - ).toMatchObject({ origin: 'http://127.0.0.1:4602' }) - - await vi.advanceTimersByTimeAsync(15_000) - - expect(background.stop).not.toHaveBeenCalled() - expect( - session.withBrowserScope('chat-stale-restore-prompt', () => - session.sitePermissionRequestForScope() - ) - ).toBeDefined() - - await vi.advanceTimersByTimeAsync(5_000) - - await expect(redirected).resolves.toEqual({ cancel: true }) - expect( - session.withBrowserScope('chat-stale-restore-prompt', () => - session.sitePermissionRequestForScope() - ) - ).toBeUndefined() - expect(background.stop).not.toHaveBeenCalled() - - await vi.advanceTimersByTimeAsync(15_000) - - expect(background.stop).toHaveBeenCalledOnce() - } finally { - vi.useRealTimers() - } - }) - - it('does not let repeated redirect prompts extend a restore without bound', async () => { - vi.useFakeTimers() - try { - const tabs = [ - { url: 'http://127.0.0.1:4611/active' }, - { url: 'http://127.0.0.1:4611/background' }, - ] - const { persistence } = memoryBrowserPersistence({ - 'chat-bounded-restore-prompt': { v: 1, tabs, activeIndex: 0, downloads: [] }, }) - const createdContents: MockView['webContents'][] = [] + const created: MockView['webContents'][] = [] session = freshSession( win, { onTabCreated: (webContents) => { const contents = webContents as unknown as MockView['webContents'] - createdContents.push(contents) + created.push(contents) contents.loadURL.mockImplementation(() => new Promise(() => {})) }, }, persistence ) - - session.withBrowserScope('chat-bounded-restore-prompt', () => session.restoreBrowserSession()) - session.activateBrowserScope('chat-bounded-restore-prompt') - panel.setPanelBounds({ x: 100, y: 50, width: 800, height: 600 }) - const background = createdContents[1] - const firstRedirect = beginMainFrameRequest(background, 'http://127.0.0.1:4612/first') - await vi.advanceTimersByTimeAsync(0) - expect( - session.withBrowserScope('chat-bounded-restore-prompt', () => - session.sitePermissionRequestForScope() - ) - ).toMatchObject({ origin: 'http://127.0.0.1:4612' }) - - await vi.advanceTimersByTimeAsync(20_000) - await expect(firstRedirect).resolves.toEqual({ cancel: true }) - panel.setPanelBounds({ x: 100, y: 50, width: 800, height: 600 }) - const secondRedirect = beginMainFrameRequest(background, 'http://127.0.0.1:4613/second', 2) - await vi.advanceTimersByTimeAsync(0) - expect( - session.withBrowserScope('chat-bounded-restore-prompt', () => - session.sitePermissionRequestForScope() - ) - ).toMatchObject({ origin: 'http://127.0.0.1:4613' }) - + session.restoreBrowserSession() + const background = created[1] + await expect( + beginMainFrameRequest(background, 'http://127.0.0.1:4602/login') + ).resolves.toEqual({ cancel: false }) await vi.advanceTimersByTimeAsync(15_000) - expect(background.stop).toHaveBeenCalledOnce() - await expect(secondRedirect).resolves.toEqual({ cancel: true }) - expect( - session.withBrowserScope('chat-bounded-restore-prompt', () => - session.sitePermissionRequestForScope() - ) - ).toBeUndefined() } finally { vi.useRealTimers() } @@ -2789,7 +2715,7 @@ describe('browser-agent session', () => { expect(onTabCreated).toHaveBeenLastCalledWith(userTab?.view.webContents) }) - it('does not treat an untrusted page popup as user authorization for its origin', async () => { + it('lets internal page popups navigate after the network check', async () => { panel.setPanelBounds({ x: 100, y: 50, width: 800, height: 600 }) const source = (session.ensureTab().view as unknown as MockView).webContents const openWindow = source.setWindowOpenHandler.mock.calls[0]?.[0] as (details: { @@ -2801,13 +2727,8 @@ describe('browser-agent session', () => { const popup = (session.activeTab()?.view as unknown as MockView).webContents const request = beginMainFrameRequest(popup, destination) - await vi.waitFor(() => - expect(session.sitePermissionRequestForScope()).toMatchObject({ - origin: 'http://127.0.0.1:4099', - }) - ) - session.respondToSitePermission(session.sitePermissionRequestForScope()?.requestId ?? '', false) - await expect(request).resolves.toEqual({ cancel: true }) + await expect(request).resolves.toEqual({ cancel: false }) + expect(dialog.showMessageBox).not.toHaveBeenCalled() }) it('blocks controlled pages from moving or resizing the desktop window', () => { @@ -3095,210 +3016,50 @@ describe('browser-agent session', () => { } }) - it('holds a new top-level origin for an exact task-scoped user decision', async () => { - panel.setPanelBounds({ x: 100, y: 50, width: 800, height: 600 }) - const contents = (session.ensureTab().view as unknown as MockView).webContents - const first = beginMainFrameRequest( - contents, - 'http://127.0.0.1:4101/private?token=secret#fragment' - ) - - await vi.waitFor(() => { - expect(session.sitePermissionRequestForScope()).toMatchObject({ - tabId: '1', - origin: 'http://127.0.0.1:4101', - }) - }) - const prompt = session.sitePermissionRequestForScope() - expect(prompt).not.toHaveProperty('url') - expect(win.focus).toHaveBeenCalled() - expect(win.webContents.focus).toHaveBeenCalled() - expect(session.respondToSitePermission(prompt?.requestId ?? '', true)).toBe(true) - await expect(first).resolves.toEqual({ cancel: false }) - - await expect( - beginMainFrameRequest(contents, 'http://127.0.0.1:4101/another?different=secret', 2) - ).resolves.toEqual({ cancel: false }) - expect(session.sitePermissionRequestForScope()).toBeUndefined() - - const otherOrigin = beginMainFrameRequest(contents, 'http://127.0.0.1:4102/', 3) - await vi.waitFor(() => expect(session.sitePermissionRequestForScope()).toBeDefined()) - expect(session.respondToSitePermission('not-the-live-request', true)).toBe(false) - const otherPrompt = session.sitePermissionRequestForScope() - expect(session.respondToSitePermission(otherPrompt?.requestId ?? '', false)).toBe(true) - await expect(otherOrigin).resolves.toEqual({ cancel: true }) - }) - - it('allows an SSRF-checked agent destination without granting a cross-origin redirect', async () => { - panel.setPanelBounds({ x: 100, y: 50, width: 800, height: 600 }) + it('allows public and loopback cross-origin navigation without a task prompt', async () => { const contents = (session.ensureTab().view as unknown as MockView).webContents - const destination = 'http://127.0.0.1:4111/agent-path?token=secret' - - expect( - session.grantSiteOriginForAgentNavigation(contents as unknown as WebContents, destination) - ).toBe(true) - await expect(beginMainFrameRequest(contents, destination)).resolves.toEqual({ cancel: false }) - expect(session.sitePermissionRequestForScope()).toBeUndefined() - - const redirect = beginMainFrameRequest(contents, 'http://127.0.0.1:4112/redirected', 2) - await vi.waitFor(() => - expect(session.sitePermissionRequestForScope()).toMatchObject({ - origin: 'http://127.0.0.1:4112', - }) - ) - const prompt = session.sitePermissionRequestForScope() - expect(session.respondToSitePermission(prompt?.requestId ?? '', false)).toBe(true) - await expect(redirect).resolves.toEqual({ cancel: true }) - }) - - it('uses a native exact-origin prompt when the active renderer lacks prompt support', async () => { - session = freshSession(win, { - sitePermissionPromptSupported: vi.fn(() => false), - }) - vi.mocked(dialog.showMessageBox).mockResolvedValueOnce({ - response: 1, - checkboxChecked: false, - }) - panel.setPanelBounds({ x: 100, y: 50, width: 800, height: 600 }) - const contents = (session.ensureTab().view as unknown as MockView).webContents - - const request = beginMainFrameRequest( - contents, - 'http://127.0.0.1:4151/private?token=secret#fragment' - ) - - await expect(request).resolves.toEqual({ cancel: false }) - expect(dialog.showMessageBox).toHaveBeenCalledWith( - win, - expect.objectContaining({ - buttons: ['Block', 'Allow'], - defaultId: 0, - cancelId: 0, - message: 'Allow this browser task to open http://127.0.0.1:4151?', - }) - ) - expect(JSON.stringify(vi.mocked(dialog.showMessageBox).mock.lastCall)).not.toContain('secret') - expect(session.sitePermissionRequestForScope()).toBeUndefined() - }) - - it('attaches the native fallback to the window that owns the visible panel', async () => { - const panelOwner = mainWindowMock() - session = freshSession(win, { - sitePermissionPromptSupported: vi.fn(() => false), - }) - vi.mocked(dialog.showMessageBox).mockResolvedValueOnce({ - response: 0, - checkboxChecked: false, - }) - panel.setPanelBounds({ x: 100, y: 50, width: 800, height: 600 }, panelOwner) - const contents = (session.ensureTab().view as unknown as MockView).webContents - - await expect(beginMainFrameRequest(contents, 'http://127.0.0.1:4155/private')).resolves.toEqual( - { cancel: true } - ) - - expect(dialog.showMessageBox).toHaveBeenCalledWith(panelOwner, expect.any(Object)) - }) - - it('denies a new site prompt immediately when its scope is hidden or inactive', async () => { - const hiddenContents = (session.ensureTab().view as unknown as MockView).webContents - - await expect( - beginMainFrameRequest(hiddenContents, 'http://127.0.0.1:4156/hidden') - ).resolves.toEqual({ cancel: true }) - expect(session.sitePermissionRequestForScope()).toBeUndefined() - - panel.setPanelBounds({ x: 100, y: 50, width: 800, height: 600 }) - const inactiveContents = session.withBrowserScope( - 'chat-inactive', - () => session.ensureTab().view as unknown as MockView - ).webContents - await expect( - beginMainFrameRequest(inactiveContents, 'http://127.0.0.1:4157/inactive') - ).resolves.toEqual({ cancel: true }) - expect( - session.withBrowserScope('chat-inactive', () => session.sitePermissionRequestForScope()) - ).toBeUndefined() - }) - - it('does not show the native fallback when the active renderer owns the prompt', async () => { - vi.mocked(dialog.showMessageBox).mockClear() - panel.setPanelBounds({ x: 100, y: 50, width: 800, height: 600 }) - const contents = (session.ensureTab().view as unknown as MockView).webContents - const request = beginMainFrameRequest(contents, 'http://127.0.0.1:4152/docs') - - await vi.waitFor(() => expect(session.sitePermissionRequestForScope()).toBeDefined()) - + for (const url of [ + 'https://public.example/', + 'https://redirect.example/login', + 'http://127.0.0.1:4101/', + 'http://localhost:4102/', + ]) { + await expect(beginMainFrameRequest(contents, url)).resolves.toEqual({ cancel: false }) + } expect(dialog.showMessageBox).not.toHaveBeenCalled() - const prompt = session.sitePermissionRequestForScope() - expect(session.respondToSitePermission(prompt?.requestId ?? '', false)).toBe(true) - await expect(request).resolves.toEqual({ cancel: true }) }) - it('revalidates a native allow decision after the held request becomes stale', async () => { - session = freshSession(win, { - sitePermissionPromptSupported: vi.fn(() => false), - }) - vi.mocked(dialog.showMessageBox).mockClear() - let answerPrompt: ((result: { response: number; checkboxChecked: boolean }) => void) | undefined - vi.mocked(dialog.showMessageBox).mockImplementationOnce( - () => - new Promise((resolve) => { - answerPrompt = resolve + it.each(['mainFrame', 'subFrame'])( + 'retains private-network checks for %s navigation', + async (resourceType) => { + const contents = (session.ensureTab().view as unknown as MockView).webContents + for (const url of ['http://169.254.169.254/', 'http://10.0.0.1/', 'file:///tmp/example']) { + await expect(beginSubresourceRequest(contents, url, resourceType)).resolves.toEqual({ + cancel: true, }) - ) - panel.setPanelBounds({ x: 100, y: 50, width: 800, height: 600 }) - const contents = (session.ensureTab().view as unknown as MockView).webContents - const request = beginMainFrameRequest(contents, 'http://127.0.0.1:4153/held') - await vi.waitFor(() => expect(dialog.showMessageBox).toHaveBeenCalled()) - const signal = vi.mocked(dialog.showMessageBox).mock.lastCall?.at(-1)?.signal - expect(signal?.aborted).toBe(false) - - mainFrameNavigationStarted(contents, false, 'http://127.0.0.1:4154/replacement') - await expect(request).resolves.toEqual({ cancel: true }) - expect(signal?.aborted).toBe(true) - answerPrompt?.({ response: 1, checkboxChecked: false }) - - const retried = beginMainFrameRequest(contents, 'http://127.0.0.1:4153/retried', 2) - await expect(retried).resolves.toEqual({ cancel: true }) - expect(dialog.showMessageBox).toHaveBeenCalledTimes(2) - }) - - it('keeps the held request alive through its own navigation-start event', async () => { - panel.setPanelBounds({ x: 100, y: 50, width: 800, height: 600 }) - const contents = (session.ensureTab().view as unknown as MockView).webContents - const destination = 'http://127.0.0.1:4201/docs' - const request = beginMainFrameRequest(contents, destination) - await vi.waitFor(() => expect(session.sitePermissionRequestForScope()).toBeDefined()) - - mainFrameNavigationStarted(contents, false, `${destination}#section`) - const prompt = session.sitePermissionRequestForScope() - expect(prompt).toBeDefined() - expect(session.respondToSitePermission(prompt?.requestId ?? '', true)).toBe(true) - await expect(request).resolves.toEqual({ cancel: false }) - - const replaced = beginMainFrameRequest(contents, 'http://127.0.0.1:4202/', 2) - await vi.waitFor(() => expect(session.sitePermissionRequestForScope()).toBeDefined()) - mainFrameNavigationStarted(contents, false, 'http://127.0.0.1:4203/') - await expect(replaced).resolves.toEqual({ cancel: true }) - expect(session.sitePermissionRequestForScope()).toBeUndefined() - }) + } + mockLookup.mockResolvedValue([{ address: '192.168.0.1', family: 4 }]) + await expect( + beginSubresourceRequest(contents, 'https://private-redirect.example/', resourceType) + ).resolves.toEqual({ cancel: true }) + mockLookup.mockRejectedValue(new Error('DNS unavailable')) + await expect( + beginSubresourceRequest(contents, 'https://unresolved-redirect.example/', resourceType) + ).resolves.toEqual({ cancel: true }) + } + ) - it('invalidates a held site decision before an explicit replacement navigation', async () => { - panel.setPanelBounds({ x: 100, y: 50, width: 800, height: 600 }) + it('allows checked navigation in hidden and inactive tasks without a prompt', async () => { const contents = (session.ensureTab().view as unknown as MockView).webContents - const held = beginMainFrameRequest(contents, 'http://127.0.0.1:4204/held') - await vi.waitFor(() => expect(session.sitePermissionRequestForScope()).toBeDefined()) - const requestId = session.sitePermissionRequestForScope()?.requestId - - session.prepareExplicitNavigation(contents as unknown as WebContents) - - await expect(held).resolves.toEqual({ cancel: true }) - expect(session.sitePermissionRequestForScope()).toBeUndefined() - expect(session.respondToSitePermission(requestId ?? '', true)).toBe(false) + panel.setPanelBounds(null) + session.activateBrowserScope('another-task') + await expect(beginMainFrameRequest(contents, 'http://127.0.0.1:4201/')).resolves.toEqual({ + cancel: false, + }) + expect(dialog.showMessageBox).not.toHaveBeenCalled() }) - it('seeds restored origins before loading while still holding a new redirect origin', async () => { + it('allows restored pages and their checked cross-origin redirects', async () => { const restoredUrl = 'http://127.0.0.1:4301/restored?private=value' const { persistence } = memoryBrowserPersistence({ 'chat-test': { @@ -3315,56 +3076,9 @@ describe('browser-agent session', () => { expect(contents.loadURL).toHaveBeenCalledWith(restoredUrl) await expect(beginMainFrameRequest(contents, restoredUrl)).resolves.toEqual({ cancel: false }) - expect(session.sitePermissionRequestForScope()).toBeUndefined() const redirected = beginMainFrameRequest(contents, 'http://127.0.0.1:4302/login', 2) - await vi.waitFor(() => - expect(session.sitePermissionRequestForScope()).toMatchObject({ - origin: 'http://127.0.0.1:4302', - }) - ) - const prompt = session.sitePermissionRequestForScope() - session.respondToSitePermission(prompt?.requestId ?? '', false) - await expect(redirected).resolves.toEqual({ cancel: true }) - }) - - it('bounds task grants and fails closed when a main-frame request cannot map to a live tab', async () => { - panel.setPanelBounds({ x: 100, y: 50, width: 800, height: 600 }) - const contents = (session.ensureTab().view as unknown as MockView).webContents - for (let index = 0; index <= 64; index += 1) { - expect( - session.grantSiteOriginForUserNavigation( - contents as unknown as WebContents, - `http://127.0.0.1:${4400 + index}/private` - ) - ).toBe(true) - } - - const evicted = beginMainFrameRequest(contents, 'http://127.0.0.1:4400/again') - await vi.waitFor(() => - expect(session.sitePermissionRequestForScope()).toMatchObject({ - origin: 'http://127.0.0.1:4400', - }) - ) - session.respondToSitePermission(session.sitePermissionRequestForScope()?.requestId ?? '', false) - await expect(evicted).resolves.toEqual({ cancel: true }) - - const handler = contents.session.webRequest.onBeforeRequest.mock.calls[0]?.[0] - const unmapped = new Promise<{ cancel: boolean }>((resolve) => { - handler( - { - id: 99, - url: 'http://127.0.0.1:4499/', - method: 'GET', - resourceType: 'mainFrame', - referrer: '', - timestamp: Date.now(), - uploadData: [], - }, - resolve - ) - }) - await expect(unmapped).resolves.toEqual({ cancel: true }) + await expect(redirected).resolves.toEqual({ cancel: false }) }) it('blocks an image hostname that resolves to a private address', async () => { @@ -3380,34 +3094,6 @@ describe('browser-agent session', () => { }) }) - it('default-denies pending site requests on timeout, tab close, and stale-document approval', async () => { - vi.useFakeTimers() - try { - panel.setPanelBounds({ x: 100, y: 50, width: 800, height: 600 }) - const tab = session.ensureTab() - const contents = (tab.view as unknown as MockView).webContents - const timedOut = beginMainFrameRequest(contents, 'http://127.0.0.1:4501/') - await vi.advanceTimersByTimeAsync(0) - await vi.advanceTimersByTimeAsync(20_000) - await expect(timedOut).resolves.toEqual({ cancel: true }) - - panel.setPanelBounds({ x: 100, y: 50, width: 800, height: 600 }) - const stale = beginMainFrameRequest(contents, 'http://127.0.0.1:4502/', 2) - await vi.advanceTimersByTimeAsync(0) - const stalePrompt = session.sitePermissionRequestForScope() - contents.getURL.mockReturnValue('https://changed.example/') - expect(session.respondToSitePermission(stalePrompt?.requestId ?? '', true)).toBe(true) - await expect(stale).resolves.toEqual({ cancel: true }) - - const closing = beginMainFrameRequest(contents, 'http://127.0.0.1:4503/', 3) - await vi.advanceTimersByTimeAsync(0) - session.closeTab(tab.id) - await expect(closing).resolves.toEqual({ cancel: true }) - } finally { - vi.useRealTimers() - } - }) - it('leaves nothing of the signed-out user behind in the browser profile', async () => { const clearStorageData = vi.fn(async () => {}) const clearCache = vi.fn(async () => {}) diff --git a/apps/desktop/src/main/browser-agent/session.ts b/apps/desktop/src/main/browser-agent/session.ts index bbfe654b6c9..e183137c9a0 100644 --- a/apps/desktop/src/main/browser-agent/session.ts +++ b/apps/desktop/src/main/browser-agent/session.ts @@ -10,7 +10,6 @@ import type { BrowserMediaPermissionRequest, BrowserOmniboxFocusMode, BrowserPageIssue, - BrowserSitePermissionRequest, BrowserTabState, BrowserTabsState, BrowserTheme, @@ -36,7 +35,6 @@ import type { } from 'electron' import { app, - dialog, session as electronSession, Menu, nativeTheme, @@ -98,7 +96,6 @@ export interface AgentTab { recoveringUnresponsive?: boolean pendingMediaPermission?: PendingMediaPermission mediaPermissionGrant?: MediaPermissionGrant - pendingSitePermission?: PendingSitePermission lastRealUserGestureAt?: number } @@ -114,19 +111,6 @@ interface MediaPermissionGrant { devices: Set } -interface PendingSitePermission { - request: BrowserSitePermissionRequest - /** Exact committed document from which the suspended request originated. */ - documentUrl: string - /** Exact destination retained only in main-process memory for receipt validation. */ - destinationUrl: string - contents: WebContents - networkRequestId: number - resolve: (allowed: boolean) => void - timeout: ReturnType - nativePromptController?: AbortController -} - export interface BrowserSessionPersistence { load: (scopeId: string) => BrowserSessionSnapshot | null save: (scopeId: string, snapshot: BrowserSessionSnapshot) => boolean @@ -161,8 +145,6 @@ export interface AgentSessionEvents { onActiveTabChanged: (contents: WebContents) => void /** The active tab's recoverable page state changed without a navigation. */ onPageStateChanged: (contents: WebContents) => void - /** Whether the current app renderer can present and answer a site-origin prompt. */ - sitePermissionPromptSupported: (scopeId: string) => boolean /** The tab list or active tab changed. */ onTabsChanged: () => void /** Sim's appearance preference changed for an existing tab. */ @@ -200,8 +182,6 @@ const BACKGROUND_TAB_RESTORE_TIMEOUT_MS = 15_000 const FOREGROUND_TAB_RESTORE_TIMEOUT_MS = 20_000 const MEDIA_PERMISSION_GESTURE_WINDOW_MS = 10_000 const MEDIA_PERMISSION_PROMPT_TIMEOUT_MS = 30_000 -const SITE_PERMISSION_PROMPT_TIMEOUT_MS = 20_000 -const MAX_SITE_ORIGIN_GRANTS_PER_SCOPE = 64 export type BrowserShortcut = 'focus-omnibox' | 'new-tab' | 'close-tab' | 'find' @@ -269,8 +249,6 @@ interface BrowserScopeState { */ findingTabId: string | null findingRequestId: number | null - /** Memory-bounded, task-local origins explicitly reached or approved by the user. */ - siteOriginGrants: Map } function createBrowserScopeState(): BrowserScopeState { @@ -291,7 +269,6 @@ function createBrowserScopeState(): BrowserScopeState { automationNeedsAttention: false, findingTabId: null, findingRequestId: null, - siteOriginGrants: new Map(), } } @@ -444,7 +421,6 @@ interface PendingTabRestore { settled: boolean requeueAfterPreemption: boolean cancelLoad?: () => void - grantSitePermissionGrace?: () => void promoteToForeground?: () => void } @@ -1229,11 +1205,6 @@ function mediaOrigin(candidate: unknown): string | null { } } -function withoutUrlFragment(url: string): string { - const fragmentIndex = url.indexOf('#') - return fragmentIndex < 0 ? url : url.slice(0, fragmentIndex) -} - function requestedMediaDevices(candidate: unknown): BrowserMediaDevice[] | null { if (!Array.isArray(candidate) || candidate.length === 0) return null const devices = new Set() @@ -1348,212 +1319,6 @@ export async function respondToMediaPermission(requestId: string, allowed: boole publishPageIssue(tab) } -function grantSiteOrigin(state: BrowserScopeState, origin: string): void { - state.siteOriginGrants.delete(origin) - state.siteOriginGrants.set(origin, true) - while (state.siteOriginGrants.size > MAX_SITE_ORIGIN_GRANTS_PER_SCOPE) { - const oldest = state.siteOriginGrants.keys().next().value - if (typeof oldest !== 'string') break - state.siteOriginGrants.delete(oldest) - } -} - -function hasSiteOriginGrant(state: BrowserScopeState, origin: string): boolean { - if (!state.siteOriginGrants.has(origin)) return false - grantSiteOrigin(state, origin) - return true -} - -function publishSitePermissionState(scopeId: string): void { - const resolved = resolveBrowserScopeId(scopeId) - const state = browserScopeStates.get(resolved) - if (!state) return - const active = state.tabs.find((tab) => tab.id === state.activeTabId) - if (active && !active.view.webContents.isDestroyed()) { - withBrowserScope(resolved, () => events?.onPageStateChanged(active.view.webContents)) - } -} - -function settleSitePermission(tab: AgentTab, allowed: boolean, publish = true): boolean { - const pending = tab.pendingSitePermission - if (!pending) return false - tab.pendingSitePermission = undefined - clearTimeout(pending.timeout) - pending.nativePromptController?.abort() - pending.resolve(allowed) - if (publish) publishSitePermissionState(tab.scopeId) - return true -} - -function scopedTabForRequest(details: { - webContents?: WebContents - webContentsId?: number -}): { scopeId: string; tab: AgentTab } | null { - if (details.webContents) return scopedTabForContents(details.webContents) - if (typeof details.webContentsId !== 'number') return null - for (const [scopeId, state] of browserScopeStates) { - const tab = state.tabs.find( - (candidate) => candidate.view.webContents.id === details.webContentsId - ) - if (tab) return { scopeId, tab } - } - return null -} - -/** Highest-priority exact site request: visible tab, automation tab, then task tab order. */ -export function sitePermissionRequestForScope(): BrowserSitePermissionRequest | undefined { - const state = browserScopeState() - const active = state.tabs.find((tab) => tab.id === state.activeTabId)?.pendingSitePermission - if (active) return active.request - const automation = state.tabs.find( - (tab) => tab.id === state.automationTabId - )?.pendingSitePermission - if (automation) return automation.request - return state.tabs.find((tab) => tab.pendingSitePermission)?.pendingSitePermission?.request -} - -function grantSiteOriginForExplicitNavigation(contents: WebContents, destination: string): boolean { - const scoped = scopedTabForContents(contents) - const origin = mediaOrigin(destination) - if (!scoped || !origin) return false - const state = browserScopeStates.get(scoped.scopeId) - if (!state || scoped.tab.view.webContents !== contents || contents.isDestroyed()) return false - grantSiteOrigin(state, origin) - return true -} - -/** Grants only the destination origin entered through a native-activation-gated user action. */ -export function grantSiteOriginForUserNavigation( - contents: WebContents, - destination: string -): boolean { - return grantSiteOriginForExplicitNavigation(contents, destination) -} - -/** Grants the exact destination origin after the browser driver has completed its SSRF check. */ -export function grantSiteOriginForAgentNavigation( - contents: WebContents, - destination: string -): boolean { - return grantSiteOriginForExplicitNavigation(contents, destination) -} - -/** Applies a response only to the exact live task, tab, document, and suspended network request. */ -export function respondToSitePermission(requestId: string, allowed: boolean): boolean { - const scopeId = getBrowserScopeId() - const state = browserScopeStates.get(scopeId) - const tab = state?.tabs.find( - (candidate) => candidate.pendingSitePermission?.request.requestId === requestId - ) - const pending = tab?.pendingSitePermission - if (!state || !tab || !pending) return false - - if (!allowed) return settleSitePermission(tab, false) - - const contents = tab.view.webContents - const live = - !contents.isDestroyed() && - pending.contents === contents && - pending.request.tabId === tab.id && - pending.documentUrl === contents.getURL() && - mediaOrigin(pending.destinationUrl) === pending.request.origin && - scopeId === resolveBrowserScopeId(tab.scopeId) && - scopeId === getActiveBrowserScopeId() && - isPanelVisible() - if (!live) return settleSitePermission(tab, false) - - grantSiteOrigin(state, pending.request.origin) - return settleSitePermission(tab, true) -} - -async function requestSitePermission(details: { - id: number - url: string - webContents?: WebContents - webContentsId?: number -}): Promise { - const origin = mediaOrigin(details.url) - const scoped = scopedTabForRequest(details) - if (!origin || !scoped || suspendedBrowserScopes.has(scoped.scopeId)) return false - const state = browserScopeStates.get(scoped.scopeId) - const contents = scoped.tab.view.webContents - if (!state || contents.isDestroyed()) return false - - if (mediaOrigin(contents.getURL()) === origin || hasSiteOriginGrant(state, origin)) return true - if (scoped.scopeId !== getActiveBrowserScopeId() || !isPanelVisible()) return false - const win = panelWindow() - if (!win || win.isDestroyed()) return false - - settleSitePermission(scoped.tab, false, false) - revokeTabMediaPermissions(scoped.tab, false) - const request: BrowserSitePermissionRequest = { - requestId: generateId(), - tabId: scoped.tab.id, - origin, - } - const allowed = new Promise((resolve) => { - scoped.tab.pendingSitePermission = { - request, - documentUrl: contents.getURL(), - destinationUrl: details.url, - contents, - networkRequestId: details.id, - resolve, - timeout: setTimeout( - bindToBrowserScope(scoped.scopeId, () => { - const pending = scoped.tab.pendingSitePermission - if ( - pending?.request.requestId !== request.requestId || - pending.networkRequestId !== details.id - ) { - return - } - settleSitePermission(scoped.tab, false) - }), - SITE_PERMISSION_PROMPT_TIMEOUT_MS - ), - } - }) - scoped.tab.pendingRestore?.grantSitePermissionGrace?.() - if (events?.sitePermissionPromptSupported(scoped.scopeId)) { - win.focus() - win.webContents.focus() - publishSitePermissionState(scoped.scopeId) - } else { - const nativePromptController = new AbortController() - const pending = scoped.tab.pendingSitePermission - if (!pending || pending.request.requestId !== request.requestId) return await allowed - pending.nativePromptController = nativePromptController - void dialog - .showMessageBox(win, { - type: 'warning', - buttons: ['Block', 'Allow'], - defaultId: 0, - cancelId: 0, - noLink: true, - signal: nativePromptController.signal, - message: `Allow this browser task to open ${request.origin}?`, - detail: 'Only allow this site if it is expected for the current task.', - }) - .then(({ response }) => { - withBrowserScope(scoped.scopeId, () => { - respondToSitePermission(request.requestId, response === 1) - }) - }) - .catch((error) => { - if (!nativePromptController.signal.aborted) { - logger.warn('Could not present the native site permission prompt', { - error: getErrorMessage(error), - }) - } - withBrowserScope(scoped.scopeId, () => { - respondToSitePermission(request.requestId, false) - }) - }) - } - return await allowed -} - /** * Default-deny hardening for the agent partition. Site permissions remain * denied apart from ALLOWED_SITE_PERMISSIONS. Media is granted only after a @@ -1678,25 +1443,7 @@ function configureAgentPartition(ses: Session): void { logger.warn('Could not answer an agent request', { error: getErrorMessage(error) }) } } - if (details.resourceType === 'mainFrame') { - void checkAgentUrl(details.url) - .then(async (guard) => { - if (!guard.ok) { - logger.warn('Blocked agent document navigation to a private host') - settle(true) - return - } - settle(!(await requestSitePermission(details))) - }) - .catch((error) => { - // Fail closed: an unexpected rejection must cancel, never leave the - // request suspended with no callback. - logger.error('Agent SSRF check failed; cancelling request', { error }) - settle(true) - }) - return - } - if (details.resourceType === 'subFrame') { + if (details.resourceType === 'mainFrame' || details.resourceType === 'subFrame') { void checkAgentUrl(details.url) .then((guard) => { if (!guard.ok) logger.warn('Blocked agent document navigation to a private host') @@ -2169,14 +1916,10 @@ export function stopFindInActiveTab(focusPage: boolean): void { * inside the browser resource rather than spawn a native window, and both are * reached from an untrusted page, so the scheme is checked here once. */ -function openTabWithUrl( - url: string, - { agentOwned, userAuthorized }: { agentOwned: boolean; userAuthorized: boolean } -): void { +function openTabWithUrl(url: string, { agentOwned }: { agentOwned: boolean }): void { if (!/^https?:\/\//i.test(url)) return try { const tab = agentOwned ? addAutomationTab() : addTab() - if (userAuthorized) grantSiteOriginForUserNavigation(tab.view.webContents, url) void tab.view.webContents.loadURL(url).catch(() => {}) } catch (error) { logger.warn('Could not open a link in a new browser tab', { @@ -2228,10 +1971,7 @@ function initializeTabView(view: WebContentsView, scopeId: string): WebContentsV contents.setUserAgent(browserUserAgent()) attachAgentContextMenu(contents, { addToChat: (text) => withBrowserScope(scopeId, () => addPageSelectionToChat(contents, text)), - openTab: (url) => - withBrowserScope(scopeId, () => - openTabWithUrl(url, { agentOwned: false, userAuthorized: true }) - ), + openTab: (url) => withBrowserScope(scopeId, () => openTabWithUrl(url, { agentOwned: false })), defaultZoomFactor: getBrowserDefaultZoomFactor, }) @@ -2291,7 +2031,6 @@ function initializeTabView(view: WebContentsView, scopeId: string): WebContentsV withBrowserScope(scopeId, () => openTabWithUrl(details.url, { agentOwned: agentOwnsPopupFrom(contents), - userAuthorized: false, }) ) return { action: 'deny' } @@ -2320,7 +2059,6 @@ function initializeTabView(view: WebContentsView, scopeId: string): WebContentsV return } dismissFind(tab.id) - settleSitePermission(tab, false) revokeTabMediaPermissions(tab, false) tab.pageIssue = { kind: 'crashed', @@ -2338,7 +2076,6 @@ function initializeTabView(view: WebContentsView, scopeId: string): WebContentsV const tab = tabs.find((entry) => entry.view === view) if (!tab || tab.pageIssue?.kind === 'crashed') return dismissFind(tab.id) - settleSitePermission(tab, false) revokeTabMediaPermissions(tab, false) tab.pageIssue = { kind: 'unresponsive', @@ -2445,13 +2182,6 @@ function initializeTabView(view: WebContentsView, scopeId: string): WebContentsV if (!details.isMainFrame) return const tab = tabs.find((entry) => entry.view === view) if (tab) { - if ( - tab.pendingSitePermission && - withoutUrlFragment(tab.pendingSitePermission.destinationUrl) !== - withoutUrlFragment(details.url) - ) { - settleSitePermission(tab, false) - } revokeTabMediaPermissions(tab) } notePageNavigationStarted(contents) @@ -2473,7 +2203,6 @@ function initializeTabView(view: WebContentsView, scopeId: string): WebContentsV bindToBrowserScope(scopeId, () => { const tab = tabs.find((entry) => entry.view === view) if (tab) { - settleSitePermission(tab, false) revokeTabMediaPermissions(tab, false) } events?.onTabClosed(contents) @@ -2719,19 +2448,14 @@ function loadPendingTabRestore(pending: PendingTabRestore, timeoutMs: number): P return new Promise((resolve) => { let settled = false let timeout: ReturnType | undefined - const startedAt = Date.now() - let hardDeadlineAt = startedAt + timeoutMs + SITE_PERMISSION_PROMPT_TIMEOUT_MS - let deadlineAt = startedAt + timeoutMs + let deadlineAt = Date.now() + timeoutMs let foregroundDeadlineGranted = pending.priority === 'foreground' - let sitePermissionGraceGranted = false const finish = (loaded: boolean) => { if (settled) return settled = true if (timeout) clearTimeout(timeout) pending.cancelLoad = undefined - pending.grantSitePermissionGrace = undefined pending.promoteToForeground = undefined - if (!loaded) settleSitePermission(pending.tab, false) if ( loaded && isPendingTabRestoreLive(pending) && @@ -2768,21 +2492,10 @@ function loadPendingTabRestore(pending: PendingTabRestore, timeoutMs: number): P if (timeout) clearTimeout(timeout) timeout = setTimeout(() => stopLoad(true), Math.max(0, deadlineAt - Date.now())) } - pending.grantSitePermissionGrace = () => { - if (settled || sitePermissionGraceGranted) return - sitePermissionGraceGranted = true - deadlineAt = Math.min(deadlineAt + SITE_PERMISSION_PROMPT_TIMEOUT_MS, hardDeadlineAt) - scheduleDeadline() - } pending.promoteToForeground = () => { if (settled || foregroundDeadlineGranted) return foregroundDeadlineGranted = true - hardDeadlineAt = - startedAt + FOREGROUND_TAB_RESTORE_TIMEOUT_MS + SITE_PERMISSION_PROMPT_TIMEOUT_MS - deadlineAt = Math.min( - Math.max(deadlineAt, Date.now() + FOREGROUND_TAB_RESTORE_TIMEOUT_MS), - hardDeadlineAt - ) + deadlineAt = Date.now() + FOREGROUND_TAB_RESTORE_TIMEOUT_MS scheduleDeadline() } scheduleDeadline() @@ -2947,7 +2660,6 @@ export async function waitForPendingTabRestore(tab: AgentTab): Promise export function prepareExplicitNavigation(contents: WebContents): void { const tab = tabForContents(contents) if (!tab) return - settleSitePermission(tab, false) tab.pendingRestoreUrl = undefined discardPendingTabRestore(tab) } @@ -3020,7 +2732,6 @@ export function restoreBrowserSession(): void { nextTabId: state.nextTabId, restored: state.restored, lastPersistedSnapshot: state.lastPersistedSnapshot, - siteOriginGrants: new Map(state.siteOriginGrants), } const previousDownloads = browserDownloadsByScope.get(scopeId) const restoredTabs: AgentTab[] = [] @@ -3035,8 +2746,6 @@ export function restoreBrowserSession(): void { for (const { entry } of selectedEntries) { const tab = addTabInternal({ activate: false, notify: false }) tab.pendingRestoreUrl = entry.url - const restoredOrigin = mediaOrigin(entry.url) - if (restoredOrigin) grantSiteOrigin(state, restoredOrigin) restoredTabs.push(tab) restoredLoads.push({ tab, url: entry.url }) } @@ -3057,7 +2766,6 @@ export function restoreBrowserSession(): void { state.nextTabId = previousState.nextTabId state.restored = previousState.restored state.lastPersistedSnapshot = previousState.lastPersistedSnapshot - state.siteOriginGrants = previousState.siteOriginGrants if (previousDownloads) browserDownloadsByScope.set(scopeId, previousDownloads) else browserDownloadsByScope.delete(scopeId) applyActiveTabThrottling() @@ -3146,7 +2854,6 @@ export function reopenClosedTab(): AgentTab | null { // onBeforeRequest still runs the full DNS-resolving SSRF check on the // document load. Pre-checking would only buy a nicer error, and there is // no model to report one to — this path is a user keystroke. - grantSiteOriginForUserNavigation(tab.view.webContents, url) void tab.view.webContents.loadURL(url).catch(() => {}) } return tab @@ -3236,7 +2943,6 @@ export function closeTab( clearAutomationIndicatorsForTab(tabId) const [tab] = tabs.splice(index, 1) discardPendingTabRestore(tab) - settleSitePermission(tab, false) revokeTabMediaPermissions(tab, false) recentlyClosedTabUrls.unshift(sanitizeRestorableUrl(tabUrl(tab)) ?? 'about:blank') if (recentlyClosedTabUrls.length > MAX_RECENTLY_CLOSED_TABS) { @@ -3265,7 +2971,6 @@ export function closeTab( persistBrowserSession() events?.onTabsChanged() if (!hasSession()) { - currentScope.siteOriginGrants.clear() events?.onSessionClosed() } } @@ -3421,7 +3126,6 @@ function closeLiveTabs(): void { dismissFind(currentScope.findingTabId) for (const tab of tabs.splice(0)) { discardPendingTabRestore(tab) - settleSitePermission(tab, false, false) revokeTabMediaPermissions(tab, false) detachIfAttached(tab.view) if (!tab.view.webContents.isDestroyed()) { @@ -3434,7 +3138,6 @@ function closeLiveTabs(): void { currentScope.automationActive = false currentScope.automationNeedsAttention = false currentScope.visibleTabUserSelected = false - currentScope.siteOriginGrants.clear() clearFocusedBrowserTab() } diff --git a/apps/desktop/src/main/browser-credentials/os-auth.ts b/apps/desktop/src/main/browser-credentials/os-auth.ts index 57749e6e5ac..23d9cf6c530 100644 --- a/apps/desktop/src/main/browser-credentials/os-auth.ts +++ b/apps/desktop/src/main/browser-credentials/os-auth.ts @@ -1,6 +1,7 @@ import { createLogger } from '@sim/logger' import type { MessageBoxOptions } from 'electron' -import { BrowserWindow, dialog, systemPreferences } from 'electron' +import { BrowserWindow, systemPreferences } from 'electron' +import { showShellDialog } from '@/main/dialogs' const logger = createLogger('BrowserCredentialAuth') @@ -143,8 +144,8 @@ async function promptForSecret(reason: string, action: string): Promise const parent = BrowserWindow.getFocusedWindow() const { response } = parent && !parent.isDestroyed() - ? await dialog.showMessageBox(parent, options) - : await dialog.showMessageBox(options) + ? await showShellDialog(parent, options) + : await showShellDialog(options) return response === 1 } catch (error) { // Fail closed: if the confirmation cannot be shown, nothing is revealed. diff --git a/apps/desktop/src/main/dialogs.test.ts b/apps/desktop/src/main/dialogs.test.ts new file mode 100644 index 00000000000..e2915ac5925 --- /dev/null +++ b/apps/desktop/src/main/dialogs.test.ts @@ -0,0 +1,128 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' + +vi.mock('electron', () => import('@/test/electron-mock')) +vi.unmock('@/main/dialogs') + +import { dialog, session } from 'electron' +import { showShellDialog } from '@/main/dialogs' +import { BrowserWindow } from '@/test/electron-mock' + +function latestWindow() { + const win = BrowserWindow.instances.at(-1) + if (!win) throw new Error('Dialog window was not created') + win.webContents.mainFrame.url = 'sim-shell://pages/dialog.html' + return win +} + +function sender(win: BrowserWindow) { + return { sender: win.webContents, senderFrame: win.webContents.mainFrame } +} + +function respond(win: BrowserWindow, response: unknown, event = sender(win)) { + const handler = win.webContents.ipc.on.mock.calls.find( + ([channel]) => channel === 'shell:respond' + )?.[1] + if (!handler) throw new Error('Missing dialog response handler') + handler(event, response) +} + +beforeEach(() => { + vi.clearAllMocks() + BrowserWindow.instances.length = 0 + vi.mocked(session.fromPartition).mockReturnValue({ + setPermissionRequestHandler: vi.fn(), + setPermissionCheckHandler: vi.fn(), + protocol: { isProtocolHandled: () => false, handle: vi.fn() }, + } as never) +}) + +describe('showShellDialog', () => { + it('uses an isolated preload and preserves response indices', async () => { + const result = showShellDialog({ + message: 'Continue?', + buttons: ['Later', 'Continue'], + cancelId: 0, + }) + const win = latestWindow() + expect(BrowserWindow.lastOptions).toMatchObject({ + frame: false, + webPreferences: { + partition: 'shell-dialogs', + sandbox: true, + nodeIntegration: false, + contextIsolation: true, + }, + }) + respond(win, 1) + await expect(result).resolves.toEqual({ response: 1, checkboxChecked: false }) + expect(win.destroy).toHaveBeenCalledOnce() + expect(dialog.showMessageBox).not.toHaveBeenCalled() + }) + + it('rejects foreign documents, subframes, and invalid action indices', async () => { + const result = showShellDialog({ message: 'Allow?', buttons: ['Block', 'Allow'], cancelId: 0 }) + const win = latestWindow() + respond(win, 1, { ...sender(win), senderFrame: { url: 'sim-shell://pages/dialog.html' } }) + win.webContents.mainFrame.url = 'https://untrusted.example' + respond(win, 1) + win.webContents.mainFrame.url = 'sim-shell://pages/dialog.html' + respond(win, -1) + respond(win, 2) + respond(win, '1') + expect(win.destroy).not.toHaveBeenCalled() + respond(win, 0) + await expect(result).resolves.toMatchObject({ response: 0 }) + }) + + it('settles once and refuses a late response after cancellation', async () => { + const controller = new AbortController() + const result = showShellDialog({ + message: 'Allow?', + buttons: ['Block', 'Allow'], + cancelId: 0, + signal: controller.signal, + }) + const win = latestWindow() + controller.abort() + respond(win, 1) + await expect(result).resolves.toMatchObject({ response: 0 }) + expect(win.destroy).toHaveBeenCalledOnce() + }) + + it('does not open an already-aborted prompt', async () => { + const controller = new AbortController() + controller.abort() + await expect( + showShellDialog({ message: 'Continue?', signal: controller.signal }) + ).resolves.toMatchObject({ response: 0 }) + expect(BrowserWindow.instances).toHaveLength(0) + }) + + it('uses the OS recovery fallback only when the bundled renderer fails', async () => { + const result = showShellDialog({ + message: 'Recover', + buttons: ['Restart', 'Quit'], + cancelId: 1, + }) + const win = latestWindow() + const failed = win.webContents.on.mock.calls.find(([event]) => event === 'did-fail-load')?.[1] + failed?.({}, -6, 'missing asset', 'sim-shell://pages/dialog.html', true) + await expect(result).resolves.toMatchObject({ response: 0 }) + expect(dialog.showMessageBox).toHaveBeenCalledOnce() + }) + + it('clamps content sizing and ignores untrusted resize messages', async () => { + const result = showShellDialog({ message: 'Info' }) + const win = latestWindow() + const resize = win.webContents.ipc.on.mock.calls.find( + ([channel]) => channel === 'shell:resize' + )?.[1] + resize?.({ ...sender(win), senderFrame: { url: 'https://untrusted.example' } }, 500) + resize?.(sender(win), Number.NaN) + expect(win.setContentSize).not.toHaveBeenCalled() + resize?.(sender(win), 100000) + expect(win.setContentSize).toHaveBeenCalledWith(500, 820) + respond(win, 0) + await result + }) +}) diff --git a/apps/desktop/src/main/dialogs.ts b/apps/desktop/src/main/dialogs.ts new file mode 100644 index 00000000000..23f96c2beac --- /dev/null +++ b/apps/desktop/src/main/dialogs.ts @@ -0,0 +1,135 @@ +import { join } from 'node:path' +import { createLogger } from '@sim/logger' +import { getErrorMessage } from '@sim/utils/errors' +import type { MessageBoxOptions, MessageBoxReturnValue } from 'electron' +import { app, BrowserWindow, dialog, nativeTheme, session } from 'electron' +import { attachLocalPageProtocol, localPageUrl } from '@/main/local-pages' +import { attachShellWindowSizing, isShellWindowSender } from '@/main/shell-window' +import { createSecureWebPreferences } from '@/main/window-preferences' +import type { ShellDialogConfiguration } from '@/shared/shell' + +const logger = createLogger('DesktopDialogs') +const DIALOG_WIDTH = 500 +const DIALOG_PARTITION = 'shell-dialogs' + +interface ShellDialogOptions extends MessageBoxOptions { + primaryVariant?: ShellDialogConfiguration['primaryVariant'] +} + +export function showShellDialog(options: ShellDialogOptions): Promise +export function showShellDialog( + parent: BrowserWindow, + options: ShellDialogOptions +): Promise +/** + * Presents app-owned messages in an isolated bundled EMCN window. The OS dialog + * is the last-resort fallback if this recovery renderer itself cannot load. + */ +export function showShellDialog( + parentOrOptions: BrowserWindow | ShellDialogOptions, + suppliedOptions?: ShellDialogOptions +): Promise { + const parent = suppliedOptions ? (parentOrOptions as BrowserWindow) : undefined + const options = suppliedOptions ?? (parentOrOptions as ShellDialogOptions) + const buttons = options.buttons?.length ? options.buttons : ['OK'] + const cancelId = + options.cancelId ?? + Math.max( + 0, + buttons.findIndex((label) => /^(cancel|no|close|ok)$/i.test(label)) + ) + const configuration: ShellDialogConfiguration = { + title: options.title ?? 'Sim', + message: options.message, + detail: options.detail ?? '', + buttons, + defaultId: options.defaultId ?? 0, + cancelId, + primaryVariant: options.primaryVariant ?? 'primary', + } + const cancelled = { response: cancelId, checkboxChecked: false } + if (options.signal?.aborted) return Promise.resolve(cancelled) + + return new Promise((resolve, reject) => { + const ses = session.fromPartition(DIALOG_PARTITION) + ses.setPermissionRequestHandler((_contents, _permission, callback) => callback(false)) + ses.setPermissionCheckHandler(() => false) + attachLocalPageProtocol(ses) + const win = new BrowserWindow({ + width: DIALOG_WIDTH, + height: 240, + useContentSize: true, + frame: false, + resizable: false, + minimizable: false, + maximizable: false, + fullscreenable: false, + show: false, + title: configuration.title, + backgroundColor: nativeTheme.shouldUseDarkColors ? '#1b1b1b' : '#ffffff', + ...(parent && !parent.isDestroyed() ? { parent, modal: true } : {}), + webPreferences: createSecureWebPreferences( + DIALOG_PARTITION, + join(__dirname, 'shell-preload.cjs'), + app.isPackaged + ), + }) + const pageUrl = localPageUrl('dialog.html') + let settled = false + const finish = (response: number) => { + if (settled) return + settled = true + options.signal?.removeEventListener('abort', abort) + clearTimeout(loadTimeout) + if (!win.isDestroyed()) win.destroy() + resolve({ response, checkboxChecked: false }) + } + const abort = () => finish(cancelId) + const fallback = () => { + if (settled) return + settled = true + options.signal?.removeEventListener('abort', abort) + clearTimeout(loadTimeout) + if (!win.isDestroyed()) win.destroy() + const result = + parent && !parent.isDestroyed() + ? dialog.showMessageBox(parent, options) + : dialog.showMessageBox(options) + void result.then(resolve, reject) + } + const loadTimeout = setTimeout(fallback, 10_000) + options.signal?.addEventListener('abort', abort, { once: true }) + win.on('closed', abort) + win.webContents.setWindowOpenHandler(() => ({ action: 'deny' })) + win.webContents.on('will-navigate', (event) => event.preventDefault()) + win.webContents.on('will-redirect', (event) => event.preventDefault()) + win.webContents.on('render-process-gone', fallback) + win.webContents.on('did-fail-load', (_event, code, _description, _url, isMainFrame) => { + if (isMainFrame && code !== -3) fallback() + }) + win.webContents.on('before-input-event', (event, input) => { + if (input.type === 'keyDown' && input.key === 'Escape') { + event.preventDefault() + abort() + } + }) + win.webContents.ipc.handle('shell:configuration', (event) => { + if (!isShellWindowSender(win, pageUrl, event)) throw new Error('Untrusted dialog sender') + return configuration + }) + win.webContents.ipc.on('shell:respond', (event, response: unknown) => { + if (!isShellWindowSender(win, pageUrl, event)) return + if (typeof response !== 'number' || !Number.isInteger(response) || !buttons[response]) return + finish(response) + }) + attachShellWindowSizing(win, pageUrl, DIALOG_WIDTH, () => { + if (settled) return + clearTimeout(loadTimeout) + win.show() + }) + void win.loadURL(pageUrl).catch((error) => { + logger.error('Could not load a bundled dialog', { error: getErrorMessage(error) }) + fallback() + }) + }) +} diff --git a/apps/desktop/src/main/handoff.ts b/apps/desktop/src/main/handoff.ts index 2f2e95583b5..c9295941377 100644 --- a/apps/desktop/src/main/handoff.ts +++ b/apps/desktop/src/main/handoff.ts @@ -5,7 +5,8 @@ import { safeCompare } from '@sim/security/compare' import { getErrorMessage } from '@sim/utils/errors' import { generateShortId } from '@sim/utils/id' import type { BrowserWindow } from 'electron' -import { app, dialog } from 'electron' +import { app } from 'electron' +import { showShellDialog } from '@/main/dialogs' import type { EventRecorder } from '@/main/observability' const logger = createLogger('DesktopHandoff') @@ -417,7 +418,7 @@ export function createAuthFlow(deps: AuthFlowDeps): AuthFlow { 'handoff_redeem_fail', status === undefined ? { reason } : { reason, status } ) - void dialog.showMessageBox(win, { + void showShellDialog(win, { type: 'error', message: 'Sign-in failed', detail: 'The sign-in could not be completed. Try signing in again.', @@ -433,7 +434,7 @@ export function createAuthFlow(deps: AuthFlowDeps): AuthFlow { if (!opened) { const win = await resolveWindow('begin_window') if (!win) return - void dialog.showMessageBox(win, { + void showShellDialog(win, { type: 'error', message: 'Couldn’t start sign-in', detail: 'Sim could not open your browser to sign in. Try again.', diff --git a/apps/desktop/src/main/index.ts b/apps/desktop/src/main/index.ts index 49127cf305d..b273e50f1eb 100644 --- a/apps/desktop/src/main/index.ts +++ b/apps/desktop/src/main/index.ts @@ -717,8 +717,6 @@ function main(): void { onSessionStatus: (alive, scopeId) => { scopeEvents.sendBrowser(scopeId, 'browser-agent:session-status', alive, scopeId) }, - sitePermissionPromptSupported: (scopeId) => - scopeEvents.browserSitePermissionPromptSupported(scopeId), onFillAvailability: (available, scopeId) => { scopeEvents.sendBrowser(scopeId, 'browser-credentials:fill-availability', { available, diff --git a/apps/desktop/src/main/ipc.test.ts b/apps/desktop/src/main/ipc.test.ts index cace1224b65..83deccabb27 100644 --- a/apps/desktop/src/main/ipc.test.ts +++ b/apps/desktop/src/main/ipc.test.ts @@ -299,7 +299,6 @@ describe('registerIpcHandlers', () => { scopeEvents: { activateBrowser: vi.fn(), activateTerminal: vi.fn(), - registerBrowserSitePermissionPromptSupport: vi.fn(), sendBrowser: vi.fn(), sendTerminal: vi.fn(), }, @@ -1047,56 +1046,19 @@ describe('registerIpcHandlers', () => { panelAction.mockRestore() }) - it('requires trusted input for site grants and user-origin navigation', async () => { + it('requires trusted input for user navigation', async () => { const { invoke, on } = collectHandlers() const panelAction = vi.spyOn(browserDriver, 'handlePanelAction').mockResolvedValue() const handler = on.get('browser-agent:panel-action') + const action = { action: 'navigate', url: 'https://docs.example/page' } - await invoke.get('browser-agent:activate-scope')?.(inactiveAppEvent, 'chat-sites') - handler?.( - inactiveAppEvent, - { action: 'respond-site-permission', requestId: 'request-1', allowed: true }, - 'chat-sites' - ) - handler?.( - inactiveAppEvent, - { action: 'respond-site-permission', requestId: 'request-1', allowed: false }, - 'chat-sites' - ) - handler?.( - inactiveAppEvent, - { action: 'navigate', url: 'https://docs.example/private' }, - 'chat-sites' - ) + await invoke.get('browser-agent:activate-scope')?.(inactiveAppEvent, 'chat-navigation') + handler?.(inactiveAppEvent, action, 'chat-navigation') + expect(panelAction).not.toHaveBeenCalled() - expect(panelAction).toHaveBeenCalledOnce() - expect(panelAction).toHaveBeenCalledWith('chat-sites', { - action: 'respond-site-permission', - requestId: 'request-1', - allowed: false, - }) - - await invoke.get('browser-agent:activate-scope')?.(activeAppEvent, 'chat-sites') - handler?.( - activeAppEvent, - { action: 'respond-site-permission', requestId: 'request-2', allowed: true }, - 'chat-sites' - ) - handler?.( - activeAppEvent, - { action: 'navigate', url: 'https://docs.example/private' }, - 'chat-sites' - ) - - expect(panelAction).toHaveBeenNthCalledWith(2, 'chat-sites', { - action: 'respond-site-permission', - requestId: 'request-2', - allowed: true, - }) - expect(panelAction).toHaveBeenNthCalledWith(3, 'chat-sites', { - action: 'navigate', - url: 'https://docs.example/private', - }) + await invoke.get('browser-agent:activate-scope')?.(activeAppEvent, 'chat-navigation') + handler?.(activeAppEvent, action, 'chat-navigation') + expect(panelAction).toHaveBeenCalledExactlyOnceWith('chat-navigation', action) panelAction.mockRestore() }) @@ -1123,20 +1085,6 @@ describe('registerIpcHandlers', () => { panelAction.mockRestore() }) - it('accepts site permission prompt support only from the app renderer', () => { - const { on } = collectHandlers() - const register = on.get('browser-agent:register-site-permission-prompt-support') - - register?.(evilEvent) - expect(deps.scopeEvents.registerBrowserSitePermissionPromptSupport).not.toHaveBeenCalled() - - register?.(appEvent) - expect(deps.scopeEvents.registerBrowserSitePermissionPromptSupport).toHaveBeenCalledOnce() - expect(deps.scopeEvents.registerBrowserSitePermissionPromptSupport).toHaveBeenCalledWith( - appSender - ) - }) - it('ignores browser-agent panel actions from outside the app origin', () => { const { on } = collectHandlers() const handler = on.get('browser-agent:panel-action') @@ -1376,11 +1324,10 @@ describe('registerIpcHandlers', () => { it('atomically creates and navigates a canonical user URL only from trusted input', async () => { const tabsState = { scopeId: 'chat-links', tabs: [], activeTabId: '2' } - const tabContents = { loadURL: vi.fn(async () => {}) } + const tabContents = { loadURL: vi.fn(async () => {}), isDestroyed: () => false } const add = vi.spyOn(browserSession, 'addTab').mockReturnValue({ view: { webContents: tabContents }, } as never) - const grant = vi.spyOn(browserSession, 'grantSiteOriginForUserNavigation').mockReturnValue(true) const peek = vi.spyOn(browserSession, 'peekTabsState').mockReturnValue(tabsState) const { invoke } = collectHandlers() @@ -1394,7 +1341,6 @@ describe('registerIpcHandlers', () => { ).resolves.toEqual(tabsState) expect(add).toHaveBeenCalledOnce() - expect(grant).toHaveBeenCalledWith(tabContents, CANONICAL_BROWSER_URL) expect(tabContents.loadURL).toHaveBeenCalledWith(CANONICAL_BROWSER_URL) for (const url of INVALID_BROWSER_URLS) { @@ -1415,7 +1361,6 @@ describe('registerIpcHandlers', () => { expect(add).toHaveBeenCalledOnce() add.mockRestore() - grant.mockRestore() peek.mockRestore() }) diff --git a/apps/desktop/src/main/ipc.ts b/apps/desktop/src/main/ipc.ts index e55dcb7a2c7..4586567439a 100644 --- a/apps/desktop/src/main/ipc.ts +++ b/apps/desktop/src/main/ipc.ts @@ -52,7 +52,6 @@ import { addTab, findInActiveTab, getBrowserDownloadsState, - grantSiteOriginForUserNavigation, peekTabsState, reorderTab, setBrowserAppTheme, @@ -335,11 +334,7 @@ export interface IpcDeps { terminal: TerminalRegistry scopeEvents: Pick< ScopedEventRouter, - | 'activateBrowser' - | 'activateTerminal' - | 'registerBrowserSitePermissionPromptSupport' - | 'sendBrowser' - | 'sendTerminal' + 'activateBrowser' | 'activateTerminal' | 'sendBrowser' | 'sendTerminal' > settings: DesktopSettingsService getWindowState: (sender: WebContents) => DesktopWindowState @@ -928,15 +923,6 @@ export function registerIpcHandlers(deps: IpcDeps): void { }) }, }, - 'browser-agent:register-site-permission-prompt-support': { - kind: 'send', - gate: 'app-origin', - requires: 'browser', - passSender: true, - handler: (sender) => { - deps.scopeEvents.registerBrowserSitePermissionPromptSupport(sender as WebContents) - }, - }, 'browser-agent:open-url': { kind: 'invoke', gate: 'app-origin', @@ -953,7 +939,7 @@ export function registerIpcHandlers(deps: IpcDeps): void { } return withBrowserScope(scope, () => { const tab = addTab() - if (!grantSiteOriginForUserNavigation(tab.view.webContents, destination)) { + if (tab.view.webContents.isDestroyed()) { return peekTabsState() } void tab.view.webContents.loadURL(destination).catch(() => {}) @@ -1146,11 +1132,7 @@ export function registerIpcHandlers(deps: IpcDeps): void { needsUserActivation: ([action]) => { if (!isRecordLike(action)) return false if (action.action === 'navigate') return true - return ( - (action.action === 'respond-media-permission' || - action.action === 'respond-site-permission') && - action.allowed === true - ) + return action.action === 'respond-media-permission' && action.allowed === true }, handler: (sender, action, rawScope) => { const scope = activeRendererScope(browserScopeBySender, sender as WebContents, rawScope) diff --git a/apps/desktop/src/main/local-pages.test.ts b/apps/desktop/src/main/local-pages.test.ts index a425b2f6235..fde78208fa8 100644 --- a/apps/desktop/src/main/local-pages.test.ts +++ b/apps/desktop/src/main/local-pages.test.ts @@ -49,6 +49,8 @@ describe('isLocalPageUrl', () => { 'https://www.sim.ai/offline.html', 'sim-shell://evil/offline.html', 'sim-shell://pages/SeasonSansUprightsVF.woff2', + 'sim-shell://pages/server.js', + 'sim-shell://pages/server.css', 'sim-shell://pages/static/offline.html', 'sim-shell://pages/', 'not a url', @@ -66,6 +68,8 @@ describe('createLocalPageHandler', () => { root = mkdtempSync(join(tmpdir(), 'sim-local-pages-')) writeFileSync(join(root, 'offline.html'), '

offline

') writeFileSync(join(root, 'secret.txt'), 'nope') + writeFileSync(join(root, 'server.js'), 'window.renderServerModal()') + writeFileSync(join(root, 'server.css'), 'body { margin: 0 }') }) afterAll(() => { @@ -83,6 +87,21 @@ describe('createLocalPageHandler', () => { expect(await response.text()).toBe('

offline

') }) + it.each([ + ['server.js', 'text/javascript; charset=utf-8'], + ['server.css', 'text/css; charset=utf-8'], + ])( + 'serves the bundled renderer asset %s with a strict content type', + async (name, contentType) => { + const response = await createLocalPageHandler([root])( + new Request(`${LOCAL_PAGE_ORIGIN}/${name}`) + ) + expect(response.status).toBe(200) + expect(response.headers.get('content-type')).toBe(contentType) + expect(response.headers.get('x-content-type-options')).toBe('nosniff') + } + ) + it('refuses everything outside the allowlist, however the path is spelled', async () => { const handler = createLocalPageHandler([root]) for (const path of [ diff --git a/apps/desktop/src/main/local-pages.ts b/apps/desktop/src/main/local-pages.ts index 1446b6517f0..4e34f2e9870 100644 --- a/apps/desktop/src/main/local-pages.ts +++ b/apps/desktop/src/main/local-pages.ts @@ -25,7 +25,7 @@ export const LOCAL_PAGE_SCHEME = 'sim-shell' const LOCAL_PAGE_HOST = 'pages' export const LOCAL_PAGE_ORIGIN = `${LOCAL_PAGE_SCHEME}://${LOCAL_PAGE_HOST}` -export type LocalPage = 'offline.html' | 'server.html' +export type LocalPage = 'offline.html' | 'server.html' | 'dialog.html' const LOCAL_PAGES: ReadonlySet = new Set(['offline.html', 'server.html']) @@ -34,11 +34,23 @@ const LOCAL_PAGES: ReadonlySet = new Set(['offline.html', 'se * directory walk: nothing outside it can be requested however the path is * spelled, and adding an asset is a deliberate one-line change. */ -const SERVABLE_FILES: ReadonlySet = new Set([...LOCAL_PAGES, 'SeasonSansUprightsVF.woff2']) +const SERVABLE_FILES: ReadonlySet = new Set([ + ...LOCAL_PAGES, + 'SeasonSansUprightsVF.woff2', + 'server.js', + 'server.css', + 'offline.js', + 'offline.css', + 'dialog.html', + 'dialog.js', + 'dialog.css', +]) const CONTENT_TYPES: Readonly> = { '.html': 'text/html; charset=utf-8', '.woff2': 'font/woff2', + '.js': 'text/javascript; charset=utf-8', + '.css': 'text/css; charset=utf-8', } /** Builds the URL of a bundled page, with its query encoded. */ @@ -152,14 +164,14 @@ async function readFirst(rootDirs: readonly string[], name: string): Promise { ]) expect(submenu(template, 'Sim').map((item) => item.label ?? item.role ?? item.type)).toEqual([ - 'about', + 'About Sim', 'Settings…', 'Server…', 'Check for Updates…', diff --git a/apps/desktop/src/main/menu.ts b/apps/desktop/src/main/menu.ts index cf1b94009f6..c7d61ec0ba6 100644 --- a/apps/desktop/src/main/menu.ts +++ b/apps/desktop/src/main/menu.ts @@ -1,6 +1,7 @@ import type { MenuItemConstructorOptions } from 'electron' import { app, BrowserWindow, Menu } from 'electron' import { type ConfigStore, isSimCloudOrigin } from '@/main/config' +import { showShellDialog } from '@/main/dialogs' import { DOCS_URL, STATUS_URL } from '@/main/external-links' import { openExternalSafe } from '@/main/navigation' import type { @@ -166,7 +167,18 @@ export function buildMenuTemplate(deps: MenuDeps): MenuItemConstructorOptions[] { label: app.name, submenu: [ - { role: 'about' }, + { + label: `About ${app.name}`, + click: () => { + void showShellDialog({ + title: `About ${app.name}`, + type: 'info', + message: app.name, + detail: `Version ${app.getVersion()}`, + buttons: ['OK'], + }) + }, + }, { label: 'Settings…', accelerator: 'CmdOrCtrl+,', click: deps.openSettings }, { label: 'Server…', click: deps.openServerSettings }, { label: 'Check for Updates…', click: deps.checkForUpdates }, diff --git a/apps/desktop/src/main/observability.ts b/apps/desktop/src/main/observability.ts index 142283ef9da..7dec829b156 100644 --- a/apps/desktop/src/main/observability.ts +++ b/apps/desktop/src/main/observability.ts @@ -2,7 +2,8 @@ import { appendFileSync, chmodSync, mkdirSync, renameSync, statSync } from 'node import { join } from 'node:path' import { createLogger } from '@sim/logger' import type { BrowserWindow, Details } from 'electron' -import { app, dialog } from 'electron' +import { app } from 'electron' +import { showShellDialog } from '@/main/dialogs' const logger = createLogger('DesktopEvents') @@ -124,9 +125,7 @@ export function installMainProcessFailureObservers({ } const win = getWindow() const prompt = - win && !win.isDestroyed() - ? dialog.showMessageBox(win, options) - : dialog.showMessageBox(options) + win && !win.isDestroyed() ? showShellDialog(win, options) : showShellDialog(options) void prompt .then(({ response }) => { if (response === 0) app.relaunch() diff --git a/apps/desktop/src/main/scoped-event-router.test.ts b/apps/desktop/src/main/scoped-event-router.test.ts index 700d6dbc3d5..7020b526b32 100644 --- a/apps/desktop/src/main/scoped-event-router.test.ts +++ b/apps/desktop/src/main/scoped-event-router.test.ts @@ -37,10 +37,6 @@ class FakeContents { this.emit('destroyed') } - markDestroyed(): void { - this.destroyed = true - } - private emit(channel: string, ...args: unknown[]): void { for (const listener of [...(this.listeners.get(channel) ?? [])]) listener(...args) } @@ -52,76 +48,6 @@ function webContents(): { fake: FakeContents; contents: WebContents } { } describe('ScopedEventRouter', () => { - it('defaults old renderers to no site permission prompt support', () => { - const router = new ScopedEventRouter() - const renderer = webContents() - - router.activateBrowser(renderer.contents, 'chat-a') - - expect(router.browserSitePermissionPromptSupported('chat-a')).toBe(false) - }) - - it('recognizes an active renderer site permission prompt handshake', () => { - const router = new ScopedEventRouter() - const renderer = webContents() - - router.registerBrowserSitePermissionPromptSupport(renderer.contents) - router.activateBrowser(renderer.contents, 'chat-a') - - expect(router.browserSitePermissionPromptSupported('chat-a')).toBe(true) - }) - - it('requires a fresh site permission prompt handshake after renderer reload', () => { - const router = new ScopedEventRouter() - const renderer = webContents() - - router.registerBrowserSitePermissionPromptSupport(renderer.contents) - router.activateBrowser(renderer.contents, 'chat-a') - renderer.fake.navigate() - router.activateBrowser(renderer.contents, 'chat-a') - - expect(router.browserSitePermissionPromptSupported('chat-a')).toBe(false) - - router.registerBrowserSitePermissionPromptSupport(renderer.contents) - - expect(router.browserSitePermissionPromptSupported('chat-a')).toBe(true) - }) - - it('rejects ambiguous site prompts when two live renderers share a scope', () => { - const router = new ScopedEventRouter() - const first = webContents() - const second = webContents() - - router.activateBrowser(first.contents, 'chat-a') - router.activateBrowser(second.contents, 'chat-a') - router.registerBrowserSitePermissionPromptSupport(first.contents) - - expect(router.browserSitePermissionPromptSupported('chat-a')).toBe(false) - - router.registerBrowserSitePermissionPromptSupport(second.contents) - - expect(router.browserSitePermissionPromptSupported('chat-a')).toBe(false) - }) - - it('recovers site prompt support after an extra recipient moves or is destroyed', () => { - const router = new ScopedEventRouter() - const stable = webContents() - const moving = webContents() - - router.registerBrowserSitePermissionPromptSupport(stable.contents) - router.registerBrowserSitePermissionPromptSupport(moving.contents) - router.activateBrowser(stable.contents, 'chat-a') - router.activateBrowser(moving.contents, 'chat-a') - router.activateBrowser(moving.contents, 'chat-b') - - expect(router.browserSitePermissionPromptSupported('chat-a')).toBe(true) - - router.activateBrowser(moving.contents, 'chat-a') - moving.fake.markDestroyed() - - expect(router.browserSitePermissionPromptSupported('chat-a')).toBe(true) - }) - it('sends resource events only to renderers activated for the matching scope', () => { const router = new ScopedEventRouter() const chatA = webContents() diff --git a/apps/desktop/src/main/scoped-event-router.ts b/apps/desktop/src/main/scoped-event-router.ts index 7d36ca68bc5..62541b098b2 100644 --- a/apps/desktop/src/main/scoped-event-router.ts +++ b/apps/desktop/src/main/scoped-event-router.ts @@ -14,34 +14,6 @@ export class ScopedEventRouter { private readonly browser = this.createSurfaceRoutes() private readonly terminal = this.createSurfaceRoutes() private readonly observedContents = new WeakSet() - private readonly sitePermissionPromptRenderers = new WeakSet() - - /** Records an active renderer handshake without trusting a shell-bundled preload flag. */ - registerBrowserSitePermissionPromptSupport(contents: WebContents): void { - this.sitePermissionPromptRenderers.add(contents) - this.observe(contents) - } - - /** True only when exactly one live renderer owns the scope and registered prompt support. */ - browserSitePermissionPromptSupported(scopeId: string): boolean { - const recipients = this.browser.contentsByScope.get(scopeId) - if (!recipients) return false - let liveRecipientCount = 0 - let supported = false - for (const contents of [...recipients]) { - if (contents.isDestroyed()) { - this.forget(contents) - continue - } - if (this.browser.activeByContents.get(contents) !== scopeId) { - this.removeFromScope(this.browser, contents, scopeId) - continue - } - liveRecipientCount++ - supported ||= this.sitePermissionPromptRenderers.has(contents) - } - return liveRecipientCount === 1 && supported - } activateBrowser(contents: WebContents, scopeId: string): void { this.activate(this.browser, contents, scopeId) @@ -94,7 +66,6 @@ export class ScopedEventRouter { } private forget(contents: WebContents): void { - this.sitePermissionPromptRenderers.delete(contents) this.forgetSurface(this.browser, contents) this.forgetSurface(this.terminal, contents) } diff --git a/apps/desktop/src/main/server-window.test.ts b/apps/desktop/src/main/server-window.test.ts index c6936234413..b273eac6561 100644 --- a/apps/desktop/src/main/server-window.test.ts +++ b/apps/desktop/src/main/server-window.test.ts @@ -1,4 +1,4 @@ -import { beforeEach, describe, expect, it, vi } from 'vitest' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' vi.mock('electron', () => import('@/test/electron-mock')) @@ -67,11 +67,17 @@ describe('server window', () => { let deps: ServerWindowDeps beforeEach(() => { + vi.useFakeTimers() deps = makeDeps() MockBrowserWindow.instances = [] vi.mocked(dialog.showMessageBox).mockClear() }) + afterEach(() => { + vi.clearAllTimers() + vi.useRealTimers() + }) + // The page ships inside app.asar. Loaded over `file:` it never rendered in a // packaged build (the file-protocol fuse is off), which is the blank sheet // this window used to open as. @@ -97,6 +103,24 @@ describe('server window', () => { expect(event.preventDefault).toHaveBeenCalledTimes(1) }) + it('only shows the picker once its renderer has supplied a content size', () => { + const { win } = openPicker(deps) + expect(win.show).not.toHaveBeenCalled() + win.webContents.mainFrame.url = 'sim-shell://pages/server.html' + const resize = win.webContents.ipc.on.mock.calls.find(([name]) => name === 'shell:resize')?.[1] + resize?.({ sender: win.webContents, senderFrame: win.webContents.mainFrame }, 320) + expect(win.show).toHaveBeenCalledOnce() + vi.advanceTimersByTime(10_000) + expect(dialog.showMessageBox).not.toHaveBeenCalled() + }) + + it('recovers if the HTML loads but the renderer never becomes ready', () => { + const { win } = openPicker(deps) + vi.advanceTimersByTime(10_000) + expect(win.destroy).toHaveBeenCalledOnce() + expect(dialog.showMessageBox).toHaveBeenCalledWith(expect.objectContaining({ type: 'error' })) + }) + it('never leaves a blank sheet when the page fails to load', () => { const { win, handler } = openPicker(deps) diff --git a/apps/desktop/src/main/server-window.ts b/apps/desktop/src/main/server-window.ts index db4685b9e9f..4c846cc09f2 100644 --- a/apps/desktop/src/main/server-window.ts +++ b/apps/desktop/src/main/server-window.ts @@ -1,20 +1,20 @@ +import { dirname, join } from 'node:path' import type { DesktopServerChangeResult, DesktopServerConfiguration } from '@sim/desktop-bridge' import { createLogger } from '@sim/logger' import { getErrorMessage } from '@sim/utils/errors' -import { app, BrowserWindow, dialog, nativeTheme, session } from 'electron' +import { app, BrowserWindow, nativeTheme, session } from 'electron' import type { ConfigStore, DesktopSettings } from '@/main/config' import { canonicalOrigin, isSimCloudOrigin, validateOriginInput } from '@/main/config' +import { showShellDialog } from '@/main/dialogs' import { attachLocalPageProtocol, localPageUrl } from '@/main/local-pages' -import { - backgroundColorFor, - createSecureWebPreferences, - setupPermissionHandlers, -} from '@/main/window' +import { attachShellWindowSizing } from '@/main/shell-window' +import { backgroundColorFor, setupPermissionHandlers } from '@/main/window' +import { createSecureWebPreferences } from '@/main/window-preferences' const logger = createLogger('DesktopServerWindow') -const WINDOW_WIDTH = 520 -const WINDOW_HEIGHT = 340 +const WINDOW_WIDTH = 500 +const WINDOW_HEIGHT = 300 /** * The partition the server-selection window runs in. @@ -141,12 +141,13 @@ export function createServerWindow(deps: ServerWindowDeps): ServerWindowHandle { win = new BrowserWindow({ width: WINDOW_WIDTH, height: WINDOW_HEIGHT, + useContentSize: true, resizable: false, minimizable: false, maximizable: false, fullscreenable: false, title: 'Sim Server', - titleBarStyle: 'hiddenInset', + frame: false, show: false, // System preference only, unlike the main window: that one pre-paints for // the web app it is about to load, whose theme the user picked in Sim. @@ -160,22 +161,19 @@ export function createServerWindow(deps: ServerWindowDeps): ServerWindowHandle { ...(parent && !parent.isDestroyed() ? { parent, modal: true } : {}), webPreferences: createSecureWebPreferences( SERVER_WINDOW_PARTITION, - deps.preloadPath, + join(dirname(deps.preloadPath), 'shell-preload.cjs'), deps.isPackaged ), }) - win.once('ready-to-show', () => { - win?.show() - }) - win.on('closed', () => { - win = null - }) // A sheet has no title bar, and the page owns the only Cancel button. Both // ways out must therefore work without the page: Escape is handled here, // and a page that fails to load closes the window instead of leaving a // blank sheet nothing can dismiss. const opened = win + let closed = false const closeOpened = () => { + closed = true + clearTimeout(loadTimeout) if (!opened.isDestroyed()) { opened.destroy() } @@ -183,6 +181,30 @@ export function createServerWindow(deps: ServerWindowDeps): ServerWindowHandle { win = null } } + const failed = () => { + if (closed || opened.isDestroyed()) return + closeOpened() + const options = { + type: 'error' as const, + message: 'Couldn’t open the server settings', + detail: 'Sim could not load its server settings page. Restart Sim and try again.', + } + void (parent && !parent.isDestroyed() + ? showShellDialog(parent, options) + : showShellDialog(options)) + } + const loadTimeout = setTimeout(failed, 10_000) + opened.on('closed', () => { + closed = true + clearTimeout(loadTimeout) + if (win === opened) win = null + }) + attachShellWindowSizing(opened, localPageUrl('server.html'), WINDOW_WIDTH, () => { + if (closed) return + clearTimeout(loadTimeout) + opened.show() + }) + opened.webContents.on('render-process-gone', failed) opened.webContents.on('before-input-event', (event, input) => { if (input.type === 'keyDown' && input.key === 'Escape') { event.preventDefault() @@ -195,19 +217,12 @@ export function createServerWindow(deps: ServerWindowDeps): ServerWindowHandle { // -3 is ERR_ABORTED: a load this window cancelled, not a page that failed. if (!isMainFrame || errorCode === -3) return logger.error('Server window page failed to load', { errorCode, errorDescription }) - closeOpened() - const options = { - type: 'error' as const, - message: 'Couldn’t open the server settings', - detail: 'Sim could not load its server settings page. Restart Sim and try again.', - } - void (parent && !parent.isDestroyed() - ? dialog.showMessageBox(parent, options) - : dialog.showMessageBox(options)) + failed() } ) void opened.loadURL(localPageUrl('server.html')).catch((error) => { logger.error('Could not open the server window', { error: getErrorMessage(error) }) + failed() }) } diff --git a/apps/desktop/src/main/session-lifecycle.ts b/apps/desktop/src/main/session-lifecycle.ts index 5a8eee85932..cf16a6fa675 100644 --- a/apps/desktop/src/main/session-lifecycle.ts +++ b/apps/desktop/src/main/session-lifecycle.ts @@ -1,7 +1,7 @@ import { createLogger } from '@sim/logger' import { sleep } from '@sim/utils/helpers' import type { Session, WebContents } from 'electron' -import { BrowserWindow, dialog } from 'electron' +import { BrowserWindow } from 'electron' import { beginAccountDataTeardown, completeAccountDataTeardown, @@ -9,6 +9,7 @@ import { } from '@/main/account-data-generation' import { APP_ENTRY_ROUTE } from '@/main/app-routes' import { isSafeInternalPath } from '@/main/config' +import { showShellDialog } from '@/main/dialogs' import { isAuthSurfacePath, openExternalSafe } from '@/main/navigation' import type { EventRecorder } from '@/main/observability' @@ -378,7 +379,7 @@ export function createSessionLifecycleCoordinator( }) .catch((error) => { logger.error('Session teardown failed; refusing to report a clean sign-out', { error }) - void dialog.showMessageBox({ + void showShellDialog({ type: 'error', message: 'Sim could not finish signing out', detail: @@ -466,9 +467,7 @@ export async function handleConnectIntercept( detail: 'This provider requires completing the connection in your web browser. Sim will open this page there — connect the account, then come back to the app and refresh.', } - const { response } = win - ? await dialog.showMessageBox(win, options) - : await dialog.showMessageBox(options) + const { response } = win ? await showShellDialog(win, options) : await showShellDialog(options) if (response === 0) { await openExternalSafe(pageUrl, allowHttpLocalhost) } diff --git a/apps/desktop/src/main/shell-window.ts b/apps/desktop/src/main/shell-window.ts new file mode 100644 index 00000000000..af74d8811b3 --- /dev/null +++ b/apps/desktop/src/main/shell-window.ts @@ -0,0 +1,37 @@ +import type { BrowserWindow, IpcMainEvent, IpcMainInvokeEvent } from 'electron' +import { screen } from 'electron' + +/** Only the exact bundled top-level document may operate its own host window. */ +export function isShellWindowSender( + win: BrowserWindow, + pageUrl: string, + event: IpcMainEvent | IpcMainInvokeEvent +): boolean { + return ( + !win.isDestroyed() && + event.sender === win.webContents && + event.senderFrame === win.webContents.mainFrame && + event.senderFrame?.url === pageUrl + ) +} + +/** Keeps compact native dialogs at their rendered content height, within the display. */ +export function attachShellWindowSizing( + win: BrowserWindow, + pageUrl: string, + width: number, + onReady?: () => void +) { + let ready = false + win.webContents.ipc.on('shell:resize', (event, height: unknown) => { + if (!isShellWindowSender(win, pageUrl, event)) return + if (typeof height !== 'number' || !Number.isFinite(height)) return + const available = screen.getDisplayMatching(win.getBounds()).workArea.height - 80 + const nextHeight = Math.min(Math.max(Math.ceil(height), 120), available) + if (win.getContentSize()[1] !== nextHeight) win.setContentSize(width, nextHeight) + if (!ready) { + ready = true + onReady?.() + } + }) +} diff --git a/apps/desktop/src/main/terminal/index.ts b/apps/desktop/src/main/terminal/index.ts index 3f9367a448c..6256a0bcd93 100644 --- a/apps/desktop/src/main/terminal/index.ts +++ b/apps/desktop/src/main/terminal/index.ts @@ -165,6 +165,7 @@ export class TerminalService { /** Insertion-ordered, which is also the tab order the user sees. */ private readonly sessions = new Map() private activeId: string | null = null + private readonly pendingCloseConfirmations = new Set() private agentActiveId: string | null = null private activeTerminalUserSelected = false /** True while tearing every shell down, so an exit does not respawn one. */ @@ -492,7 +493,7 @@ export class TerminalService { shortcut: FocusedResourceShortcut, ownerWindow: BrowserWindow | null, emitRendererCommand: (command: TerminalShortcutCommand, terminalId: string) => void, - confirmCloseRunning?: (running: string) => boolean + confirmCloseRunning?: (running: string) => boolean | Promise ): boolean { // Hard reload has no terminal meaning — leave it to the Browser or shell. if (shortcut === 'focus-omnibox' || shortcut === 'hard-reload') return false @@ -527,7 +528,10 @@ export class TerminalService { if (this.activeId) { const active = this.sessions.get(this.activeId) const running = active?.isBusy ? (active.foreground ?? 'A process') : null - if (running && confirmCloseRunning && !confirmCloseRunning(running)) return true + if (running && active && confirmCloseRunning) { + void this.confirmCloseRunningTerminal(active, running, confirmCloseRunning) + return true + } this.closeTerminal(this.activeId) } return true @@ -543,6 +547,27 @@ export class TerminalService { return true } + /** Revalidates the captured terminal after an asynchronous native-window confirmation. */ + private async confirmCloseRunningTerminal( + terminal: TerminalSession, + running: string, + confirm: (running: string) => boolean | Promise + ): Promise { + const id = this.activeId + if (!id || this.pendingCloseConfirmations.has(id)) return + this.pendingCloseConfirmations.add(id) + try { + if (!(await confirm(running))) return + if (this.sessions.get(id) !== terminal) return + if (terminal.isBusy && (terminal.foreground ?? 'A process') !== running) return + this.closeTerminal(id) + } catch { + logger.warn('Could not confirm closing the running terminal') + } finally { + this.pendingCloseConfirmations.delete(id) + } + } + /** * Whether the terminal panel owns keyboard focus. Menu accelerators are * global, so Cmd-W has to know whether the user is looking at a terminal or diff --git a/apps/desktop/src/main/terminal/registry.test.ts b/apps/desktop/src/main/terminal/registry.test.ts index a3f50e674c3..e463431a91e 100644 --- a/apps/desktop/src/main/terminal/registry.test.ts +++ b/apps/desktop/src/main/terminal/registry.test.ts @@ -5,6 +5,8 @@ import { tmpdir } from 'node:os' import type { TerminalCommandEvent } from '@sim/terminal-protocol' import { beforeEach, describe, expect, it, vi } from 'vitest' +vi.mock('electron', () => import('@/test/electron-mock')) + interface StubSessionControl { terminalId: string cwd: string diff --git a/apps/desktop/src/main/terminal/registry.ts b/apps/desktop/src/main/terminal/registry.ts index b6a4ac7c90c..db737cad65f 100644 --- a/apps/desktop/src/main/terminal/registry.ts +++ b/apps/desktop/src/main/terminal/registry.ts @@ -8,8 +8,9 @@ import { type TerminalToolArgs, type TerminalToolResponse, } from '@sim/terminal-protocol' -import { type BrowserWindow, dialog, type WebContents } from 'electron' +import type { BrowserWindow, WebContents } from 'electron' import type { TerminalSessionSnapshot } from '@/main/desktop-chat-session-store' +import { showShellDialog } from '@/main/dialogs' import type { FocusedResourceShortcut } from '@/main/resource-shortcuts' import { MAX_TERMINALS_PER_SCOPE, @@ -300,20 +301,19 @@ export class TerminalRegistry { terminalId ) }, - (running) => { + async (running) => { if (!ownerWindow || ownerWindow.isDestroyed()) return false - return ( - dialog.showMessageBoxSync(ownerWindow, { - type: 'warning', - title: 'Close Running Terminal?', - message: `${describeRunningCommand(running)} is still running.`, - detail: 'Closing this terminal will stop the process.', - buttons: ['Close Terminal', 'Cancel'], - defaultId: 1, - cancelId: 1, - noLink: true, - }) === 0 - ) + const { response } = await showShellDialog(ownerWindow, { + type: 'warning', + title: 'Close running terminal?', + message: `${describeRunningCommand(running)} is still running.`, + detail: 'Closing this terminal will stop the process.', + buttons: ['Close Terminal', 'Cancel'], + primaryVariant: 'destructive', + defaultId: 1, + cancelId: 1, + }) + return response === 0 && !ownerWindow.isDestroyed() } ) ) { diff --git a/apps/desktop/src/main/terminal/service.test.ts b/apps/desktop/src/main/terminal/service.test.ts index 5e6122a0b8f..174a3aa10a8 100644 --- a/apps/desktop/src/main/terminal/service.test.ts +++ b/apps/desktop/src/main/terminal/service.test.ts @@ -266,6 +266,53 @@ describe('focus-gated shortcuts', () => { expect(terminal.getTabs().activeTerminalId).toBe(activeId) }) + it('waits for a single confirmation and closes only the captured running terminal', async () => { + const terminal = service() + const started = terminal.start({ cols: 80, rows: 24 }) + const id = started.activeTerminalId as string + stubSessions.get(id)?.setBusy(true) + const renderer = rendererStub() + terminal.setPanelFocused(true, renderer.contents) + let resolvePermission: (value: boolean) => void = () => {} + const permission = { + promise: new Promise((resolve) => { + resolvePermission = resolve + }), + resolve: (value: boolean) => resolvePermission(value), + } + const confirm = vi.fn(() => permission.promise) + terminal.handleFocusedShortcut('close-tab', renderer.window, vi.fn(), confirm) + terminal.handleFocusedShortcut('close-tab', renderer.window, vi.fn(), confirm) + expect(confirm).toHaveBeenCalledOnce() + expect(terminal.getTabs().tabs.some((tab) => tab.terminalId === id)).toBe(true) + permission.resolve(true) + await vi.waitFor(() => + expect(terminal.getTabs().tabs.some((tab) => tab.terminalId === id)).toBe(false) + ) + }) + + it('does not close a replacement terminal after the original exits during confirmation', async () => { + const terminal = service() + const started = terminal.start({ cols: 80, rows: 24 }) + const id = started.activeTerminalId as string + stubSessions.get(id)?.setBusy(true) + const renderer = rendererStub() + terminal.setPanelFocused(true, renderer.contents) + let resolvePermission: (value: boolean) => void = () => {} + const permission = { + promise: new Promise((resolve) => { + resolvePermission = resolve + }), + resolve: (value: boolean) => resolvePermission(value), + } + terminal.handleFocusedShortcut('close-tab', renderer.window, vi.fn(), () => permission.promise) + stubSessions.get(id)?.exit() + const replacement = terminal.openTerminal().activeTerminalId + permission.resolve(true) + await permission.promise + expect(terminal.getTabs().activeTerminalId).toBe(replacement) + }) + it('opens tabs in main and sends canvas commands to the focused renderer', () => { const terminal = service() terminal.start({ cols: 80, rows: 24 }) diff --git a/apps/desktop/src/main/updater.ts b/apps/desktop/src/main/updater.ts index a185f6c1669..1f111282976 100644 --- a/apps/desktop/src/main/updater.ts +++ b/apps/desktop/src/main/updater.ts @@ -3,7 +3,8 @@ import type { DesktopUpdateState } from '@sim/desktop-bridge' import { createLogger } from '@sim/logger' import { getErrorMessage } from '@sim/utils/errors' import type { BrowserWindow } from 'electron' -import { app, dialog, net } from 'electron' +import { app, net } from 'electron' +import { showShellDialog } from '@/main/dialogs' import { isSafeExternalUrl, openExternalSafe } from '@/main/navigation' import type { EventRecorder } from '@/main/observability' @@ -434,9 +435,7 @@ export function initUpdater(deps: UpdaterDeps): UpdaterHandle { 'Sim will close all app windows while it updates. Running terminal commands, browser activity, downloads, uploads, and unsaved edits may be interrupted. Choose Later to install the update the next time you quit Sim.', } const win = deps.getWindow() - const confirmation = win - ? dialog.showMessageBox(win, options) - : dialog.showMessageBox(options) + const confirmation = win ? showShellDialog(win, options) : showShellDialog(options) void confirmation .then(({ response }) => { if (response === 1 && state.status === 'ready' && state.version === version) { @@ -903,7 +902,7 @@ export function checkForUpdatesInteractive( deps: Pick & { handle: UpdaterHandle | null } ): void { if (!app.isPackaged) { - void dialog.showMessageBox({ + void showShellDialog({ type: 'info', message: 'Updates are only available in packaged builds', }) @@ -917,7 +916,7 @@ export function checkForUpdatesInteractive( const showDialog = (options: Electron.MessageBoxOptions) => { const win = deps.getWindow() - return win ? dialog.showMessageBox(win, options) : dialog.showMessageBox(options) + return win ? showShellDialog(win, options) : showShellDialog(options) } const settle = (state: DesktopUpdateState) => { diff --git a/apps/desktop/src/main/window-preferences.ts b/apps/desktop/src/main/window-preferences.ts new file mode 100644 index 00000000000..9a92f224a58 --- /dev/null +++ b/apps/desktop/src/main/window-preferences.ts @@ -0,0 +1,27 @@ +import type { WebPreferences } from 'electron' +import { app } from 'electron' + +/** + * The hardened webPreferences shared by the main window and any child window. + * The preload injects nothing into the page; it only exposes a whitelisted + * IPC bridge. The shell version rides in as a preload argv flag so the web + * app can enforce its minimum shell version without an IPC round-trip. + */ +export function createSecureWebPreferences( + partition: string, + preloadPath: string, + isPackaged: boolean +): WebPreferences { + return { + contextIsolation: true, + nodeIntegration: false, + sandbox: true, + webSecurity: true, + webviewTag: false, + devTools: !isPackaged, + spellcheck: true, + partition, + preload: preloadPath, + additionalArguments: [`--sim-desktop-version=${app.getVersion()}`], + } +} diff --git a/apps/desktop/src/main/window.test.ts b/apps/desktop/src/main/window.test.ts index a92e177bc5a..2d2ca5488be 100644 --- a/apps/desktop/src/main/window.test.ts +++ b/apps/desktop/src/main/window.test.ts @@ -1,4 +1,5 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { createSecureWebPreferences } from '@/main/window-preferences' vi.mock('electron', () => import('@/test/electron-mock')) @@ -8,7 +9,6 @@ import type { EventRecorder } from '@/main/observability' import { backgroundColorFor, createMainWindow, - createSecureWebPreferences, ensureMicrophoneAccess, fitBoundsToWorkArea, resolvePermission, diff --git a/apps/desktop/src/main/window.ts b/apps/desktop/src/main/window.ts index 90f93e6882c..52fb546115e 100644 --- a/apps/desktop/src/main/window.ts +++ b/apps/desktop/src/main/window.ts @@ -1,10 +1,12 @@ import { createLogger } from '@sim/logger' import { getErrorMessage } from '@sim/utils/errors' -import type { Event, Rectangle, Session, WebPreferences } from 'electron' +import type { Event, Rectangle, Session } from 'electron' import { app, BrowserWindow, dialog, nativeTheme, screen, systemPreferences } from 'electron' import { type ConfigStore, isSafeInternalPath, type WindowBounds } from '@/main/config' +import { showShellDialog } from '@/main/dialogs' import { isAppOrigin, isAuthSurfacePath } from '@/main/navigation' import type { EventRecorder } from '@/main/observability' +import { createSecureWebPreferences } from '@/main/window-preferences' const logger = createLogger('DesktopWindow') @@ -26,31 +28,6 @@ const THEME_PROBE_SCRIPT = `(() => { } })()` -/** - * The hardened webPreferences shared by the main window and any child window. - * The preload injects nothing into the page; it only exposes a whitelisted - * IPC bridge. The shell version rides in as a preload argv flag so the web - * app can enforce its minimum shell version without an IPC round-trip. - */ -export function createSecureWebPreferences( - partition: string, - preloadPath: string, - isPackaged: boolean -): WebPreferences { - return { - contextIsolation: true, - nodeIntegration: false, - sandbox: true, - webSecurity: true, - webviewTag: false, - devTools: !isPackaged, - spellcheck: true, - partition, - preload: preloadPath, - additionalArguments: [`--sim-desktop-version=${app.getVersion()}`], - } -} - /** * The permission matrix: sanitized clipboard writes and microphone access for * the trusted app origin, default-deny for everything else including unknown @@ -203,7 +180,12 @@ export function fitBoundsToWorkArea(bounds: WindowBounds, workArea: Rectangle): return { x, y, width, height } } -/** Applies the shared renderer unload decision to main and child windows. */ +/** + * Applies the shared renderer unload decision to main and child windows. + * Electron requires this decision before the event returns. Keep this one + * synchronous OS confirmation: awaiting a renderer dialog here loses the + * pending navigation or close and can discard unsaved changes. + */ export function handleWillPreventUnload( win: BrowserWindow, event: Event, @@ -339,15 +321,14 @@ export function createMainWindow(deps: CreateMainWindowDeps): BrowserWindow { return } recoveryDialog = 'crash' - void dialog - .showMessageBox(win, { - type: 'error', - buttons: ['Reload', 'Quit Sim'], - defaultId: 0, - cancelId: 0, - message: 'Sim encountered a problem', - detail: 'The page stopped unexpectedly. Reload to pick up where you left off.', - }) + void showShellDialog(win, { + type: 'error', + buttons: ['Reload', 'Quit Sim'], + defaultId: 0, + cancelId: 0, + message: 'Sim encountered a problem', + detail: 'The page stopped unexpectedly. Reload to pick up where you left off.', + }) .then(({ response }) => { if (win.isDestroyed()) return if (response === 0) win.webContents.reload() @@ -377,15 +358,14 @@ export function createMainWindow(deps: CreateMainWindowDeps): BrowserWindow { if (recoveryDialog !== null || win.isDestroyed()) return recoveryDialog = 'hang' deps.events.record('renderer_unresponsive') - void dialog - .showMessageBox(win, { - type: 'warning', - buttons: ['Wait', 'Reload'], - defaultId: 0, - cancelId: 0, - message: 'Sim isn’t responding', - detail: 'You can wait for it to recover or reload the page.', - }) + void showShellDialog(win, { + type: 'warning', + buttons: ['Wait', 'Reload'], + defaultId: 0, + cancelId: 0, + message: 'Sim isn’t responding', + detail: 'You can wait for it to recover or reload the page.', + }) .then(({ response }) => { if (!win.isDestroyed() && response === 1) { win.webContents.reload() diff --git a/apps/desktop/src/preload/index.test.ts b/apps/desktop/src/preload/index.test.ts index 5854b1b624b..9e7f1a313d1 100644 --- a/apps/desktop/src/preload/index.test.ts +++ b/apps/desktop/src/preload/index.test.ts @@ -28,7 +28,7 @@ describe('desktop preload bridge', () => { if (!exposed) throw new Error('Expected the desktop preload API to be exposed') expect(exposed.browserAgent.supportsAtomicPanelOcclusion).toBe(true) - exposed.browserAgent.registerSitePermissionPromptSupport?.() + expect(exposed.browserAgent.registerSitePermissionPromptSupport).toBeUndefined() await exposed.browserAgent.cancelTool?.('tool-1', 'chat-default') await exposed.browserAgent.cancelActiveTool?.('chat-reloaded') await exposed.browserAgent.setPanelOccluded(true, 'chat-default') @@ -46,7 +46,6 @@ describe('desktop preload bridge', () => { ['browser-agent:search-suggestions', 'sim ai'], ['desktop:settings:set-browser-search-suggestions', false], ]) - expect(send).toHaveBeenCalledWith('browser-agent:register-site-permission-prompt-support') }) it('exposes native microphone settings only on supported platforms', async () => { diff --git a/apps/desktop/src/preload/index.ts b/apps/desktop/src/preload/index.ts index e4e78d8a19b..34c37732078 100644 --- a/apps/desktop/src/preload/index.ts +++ b/apps/desktop/src/preload/index.ts @@ -196,9 +196,6 @@ const api: SimDesktopApi = { }, browserAgent: { supportsAtomicPanelOcclusion: true, - registerSitePermissionPromptSupport: (): void => { - ipcRenderer.send('browser-agent:register-site-permission-prompt-support') - }, executeTool: ( toolCallId: string, tool: BrowserToolName, diff --git a/apps/desktop/src/preload/shell.ts b/apps/desktop/src/preload/shell.ts new file mode 100644 index 00000000000..900c7993aef --- /dev/null +++ b/apps/desktop/src/preload/shell.ts @@ -0,0 +1,14 @@ +import { contextBridge, ipcRenderer } from 'electron' +import type { ShellWindowApi } from '@/shared/shell' + +const api: ShellWindowApi = { + resizeContent: (height) => ipcRenderer.send('shell:resize', height), + getDialogConfiguration: () => ipcRenderer.invoke('shell:configuration'), + respond: (response) => ipcRenderer.send('shell:respond', response), + server: { + getConfiguration: () => ipcRenderer.invoke('server:get-configuration'), + setOrigin: (origin) => ipcRenderer.invoke('server:set-origin', origin), + }, +} + +contextBridge.exposeInMainWorld('simShell', api) diff --git a/apps/desktop/src/renderer/dialog/index.tsx b/apps/desktop/src/renderer/dialog/index.tsx new file mode 100644 index 00000000000..ee8e252e75c --- /dev/null +++ b/apps/desktop/src/renderer/dialog/index.tsx @@ -0,0 +1,77 @@ +import { ChipModalBody, ChipModalFooter, ChipModalHeader, ChipModalSurface } from '@sim/emcn' +import { createRoot } from 'react-dom/client' +import { initializeShellPage, observeShellSize, shellWindow } from '@/renderer/shell' +import type { ShellDialogConfiguration } from '@/shared/shell' +import '@/renderer/shell.css' + +interface ShellDialogProps { + configuration: ShellDialogConfiguration +} + +function ShellDialog({ configuration }: ShellDialogProps) { + const { message, detail, buttons, defaultId, cancelId } = configuration + const primaryId = buttons.length === 1 ? 0 : buttons.findIndex((_, index) => index !== cancelId) + const respond = (response: number) => shellWindow?.respond(response) + const close = () => respond(cancelId) + + return ( + { + element?.querySelector('[data-chip-modal-default-action]')?.focus() + return observeShellSize(element) + }} + role='dialog' + aria-modal='true' + aria-labelledby='dialog-title' + aria-describedby={detail ? 'dialog-message dialog-detail' : 'dialog-message'} + className='max-h-screen' + > + + {configuration.title} + + +

+ {message} +

+ {detail ? ( +

+ {detail} +

+ ) : null} + + 1 + ? { onCancel: close, cancelLabel: buttons[cancelId] } + : { hideCancel: true })} + primaryAction={{ + label: buttons[primaryId], + variant: configuration.primaryVariant, + onClick: () => respond(primaryId), + }} + secondaryActions={buttons.flatMap((label, index) => + index !== primaryId && index !== cancelId + ? [{ label, onClick: () => respond(index) }] + : [] + )} + /> + + ) +} + +initializeShellPage() +const container = document.getElementById('root') +if (!container || !shellWindow) throw new Error('Dialog host is unavailable') +void shellWindow.getDialogConfiguration().then((configuration) => { + document.title = configuration.title + document.addEventListener('keydown', (event) => { + if (event.key === 'Escape') shellWindow?.respond(configuration.cancelId) + }) + createRoot(container).render() +}) diff --git a/apps/desktop/src/renderer/offline/index.tsx b/apps/desktop/src/renderer/offline/index.tsx new file mode 100644 index 00000000000..c00417bb57c --- /dev/null +++ b/apps/desktop/src/renderer/offline/index.tsx @@ -0,0 +1,116 @@ +import { useState } from 'react' +import type { SimDesktopApi } from '@sim/desktop-bridge' +import { Chip } from '@sim/emcn' +import { ArrowUpRight, RefreshCw, Server, Wordmark } from '@sim/emcn/icons' +import { createRoot } from 'react-dom/client' +import { initializeShellPage } from '@/renderer/shell' +import '@/renderer/shell.css' + +const ERROR_COPY = { + offline: { + title: 'You’re offline', + message: + 'Sim needs an internet connection. Reconnect, then try again. We’ll also retry automatically.', + }, + dns: { + title: 'Can’t find the server', + message: + 'The server address couldn’t be resolved. Check your connection or the configured server.', + }, + tls: { + title: 'Connection isn’t secure', + message: 'The server’s TLS certificate couldn’t be verified, so the connection was refused.', + }, + timeout: { + title: 'The server isn’t responding', + message: 'The connection timed out. The server may be down or your network may be blocking it.', + }, + unreachable: { + title: 'Can’t connect to Sim', + message: 'Sim couldn’t reach the server. Check your internet connection, then try again.', + }, +} as const + +const params = new URLSearchParams(location.search) +const kind = params.get('kind') ?? 'unreachable' +const copy = Object.hasOwn(ERROR_COPY, kind) + ? ERROR_COPY[kind as keyof typeof ERROR_COPY] + : ERROR_COPY.unreachable +const detail = params.get('detail') +const bridge = (window as Window & { simDesktop?: SimDesktopApi }).simDesktop + +interface OfflinePageProps { + isSimCloud: boolean +} + +function OfflinePage({ isSimCloud }: OfflinePageProps) { + const [actionError, setActionError] = useState('') + + async function checkStatus() { + setActionError('') + try { + if (!(await bridge?.openExternal('https://status.sim.ai'))) { + setActionError('Could not open the status page. Try again.') + } + } catch { + setActionError('Could not open the status page. Try again.') + } + } + + return ( +
+
+ +
+
+
+

+ {copy.title} +

+

{copy.message}

+
+ bridge?.offlineRetry()} + > + Retry + + {isSimCloud ? ( + + Check status + + ) : null} + bridge?.server?.open()}> + Change server + +
+

+ {actionError || detail} +

+
+
+
+ ) +} + +initializeShellPage() +const container = document.getElementById('root') +if (!container) throw new Error('Offline page root is missing') +const root = createRoot(container) +root.render() +void bridge?.server + ?.getConfiguration() + .then(({ isSimCloud }) => { + root.render() + }) + .catch(() => {}) diff --git a/apps/desktop/src/renderer/server/index.tsx b/apps/desktop/src/renderer/server/index.tsx new file mode 100644 index 00000000000..3c7b1fdbb94 --- /dev/null +++ b/apps/desktop/src/renderer/server/index.tsx @@ -0,0 +1,25 @@ +import { createRoot } from 'react-dom/client' +import { ServerModal } from '@/renderer/server/server-modal' +import { initializeShellPage, shellWindow } from '@/renderer/shell' +import '@/renderer/shell.css' + +initializeShellPage() +document.addEventListener('keydown', (event) => { + if (event.key === 'Escape') window.close() +}) + +const server = shellWindow?.server +const container = document.getElementById('root') +if (!container) throw new Error('Server modal root is missing') +const root = createRoot(container) + +async function renderServerModal() { + try { + const configuration = await server?.getConfiguration() + root.render() + } catch { + root.render() + } +} + +void renderServerModal() diff --git a/apps/desktop/src/renderer/server/server-modal.tsx b/apps/desktop/src/renderer/server/server-modal.tsx new file mode 100644 index 00000000000..6eac19d7d76 --- /dev/null +++ b/apps/desktop/src/renderer/server/server-modal.tsx @@ -0,0 +1,109 @@ +import { useRef, useState } from 'react' +import type { DesktopServerConfiguration } from '@sim/desktop-bridge' +import { + ChipModalBody, + ChipModalField, + ChipModalFooter, + ChipModalHeader, + ChipModalSurface, +} from '@sim/emcn' +import { observeShellSize } from '@/renderer/shell' +import type { ShellWindowApi } from '@/shared/shell' + +interface ServerModalProps { + server: ShellWindowApi['server'] | undefined + configuration?: DesktopServerConfiguration + initialError?: string +} + +function closeWindow() { + window.close() +} + +function focusServerInput(element: HTMLDivElement | null) { + element?.querySelector('input')?.select() + return observeShellSize(element) +} + +export function ServerModal({ server, configuration, initialError }: ServerModalProps) { + const requestInFlight = useRef(false) + const [origin, setOrigin] = useState(configuration?.origin ?? '') + const [pending, setPending] = useState(false) + const [error, setError] = useState(initialError) + const [message, setMessage] = useState( + configuration && configuration.origin !== configuration.defaultOrigin + ? `This build defaults to ${configuration.defaultOrigin}` + : '' + ) + + async function connect() { + if (requestInFlight.current || !origin.trim()) return + requestInFlight.current = true + setPending(true) + setError(undefined) + setMessage('') + try { + const result = await server?.setOrigin(origin) + if (!result) { + setError('The desktop shell is unavailable.') + } else if (!result.ok) { + setError(result.error) + } else if (result.unchanged) { + setMessage('Already connected to this server.') + } + } catch { + setError('The server could not be changed.') + } finally { + requestInFlight.current = false + setPending(false) + } + } + + return ( + + + Sim server + + +

+ Point this app at your own Sim deployment. Self-hosted servers must use HTTPS; localhost + may use HTTP. +

+ { + setOrigin(value) + setError(undefined) + setMessage('') + }} + autoComplete='off' + placeholder='https://sim.example.com' + disabled={pending} + error={error} + hint={{pending ? 'Connecting…' : message}} + /> + + + + ) +} diff --git a/apps/desktop/src/renderer/shell.css b/apps/desktop/src/renderer/shell.css new file mode 100644 index 00000000000..5c49487a5d1 --- /dev/null +++ b/apps/desktop/src/renderer/shell.css @@ -0,0 +1,17 @@ +@import "../../../sim/app/_styles/globals.css"; + +@source "."; + +@font-face { + font-family: "Season Sans"; + src: url("./SeasonSansUprightsVF.woff2") format("woff2"); + font-style: normal; + font-weight: 300 800; + font-display: block; +} + +html { + font-family: "Season Sans", -apple-system, BlinkMacSystemFont, "Segoe UI", sans-serif; + -webkit-font-smoothing: antialiased; + user-select: none; +} diff --git a/apps/desktop/src/renderer/shell.ts b/apps/desktop/src/renderer/shell.ts new file mode 100644 index 00000000000..49a284719dc --- /dev/null +++ b/apps/desktop/src/renderer/shell.ts @@ -0,0 +1,32 @@ +import type { ShellWindowApi } from '@/shared/shell' + +export const shellWindow = (window as Window & { simShell?: ShellWindowApi }).simShell + +/** Local windows follow the system theme independently of any reachable deployment. */ +export function initializeShellPage() { + const theme = window.matchMedia('(prefers-color-scheme: dark)') + const syncTheme = () => document.documentElement.classList.toggle('dark', theme.matches) + syncTheme() + theme.addEventListener('change', syncTheme) +} + +/** Fits the native window to the complete modal, including changing inline messages. */ +export function observeShellSize(element: HTMLDivElement | null) { + if (!element || !shellWindow) return + const resize = () => { + const body = element.querySelector('[data-chip-modal-body]') + const overflow = body ? body.scrollHeight - body.clientHeight : 0 + shellWindow.resizeContent(Math.ceil(element.getBoundingClientRect().height + overflow)) + } + const observer = new ResizeObserver(resize) + observer.observe(element) + for (const child of element.querySelectorAll('[data-chip-modal-body] > *')) + observer.observe(child) + const mutations = new MutationObserver(resize) + mutations.observe(element, { childList: true, characterData: true, subtree: true }) + resize() + return () => { + observer.disconnect() + mutations.disconnect() + } +} diff --git a/apps/desktop/src/renderer/styles.d.ts b/apps/desktop/src/renderer/styles.d.ts new file mode 100644 index 00000000000..fa5d6b64af6 --- /dev/null +++ b/apps/desktop/src/renderer/styles.d.ts @@ -0,0 +1,6 @@ +declare module '*.css' + +declare module '*.module.css' { + const classes: Readonly> + export default classes +} diff --git a/apps/desktop/src/shared/shell.ts b/apps/desktop/src/shared/shell.ts new file mode 100644 index 00000000000..b71858f5fb9 --- /dev/null +++ b/apps/desktop/src/shared/shell.ts @@ -0,0 +1,22 @@ +import type { DesktopServerChangeResult, DesktopServerConfiguration } from '@sim/desktop-bridge' + +export interface ShellDialogConfiguration { + title: string + message: string + detail: string + buttons: string[] + defaultId: number + cancelId: number + primaryVariant: 'primary' | 'destructive' +} + +/** Narrow bridge for bundled dialogs and server settings, isolated from app and browser sessions. */ +export interface ShellWindowApi { + resizeContent(height: number): void + getDialogConfiguration(): Promise + respond(response: number): void + server: { + getConfiguration(): Promise + setOrigin(origin: string): Promise + } +} diff --git a/apps/desktop/src/test/dialog-mock.ts b/apps/desktop/src/test/dialog-mock.ts new file mode 100644 index 00000000000..deeab4be576 --- /dev/null +++ b/apps/desktop/src/test/dialog-mock.ts @@ -0,0 +1,3 @@ +import { dialog } from 'electron' + +export const showShellDialog = dialog.showMessageBox diff --git a/apps/desktop/src/test/electron-mock.ts b/apps/desktop/src/test/electron-mock.ts index 4c2b69eb4b2..fea9849d085 100644 --- a/apps/desktop/src/test/electron-mock.ts +++ b/apps/desktop/src/test/electron-mock.ts @@ -248,6 +248,8 @@ export class BrowserWindow { BrowserWindow.lastOptions = options } webContents = { + ipc: { on: vi.fn(), handle: vi.fn() }, + mainFrame: { url: '' }, on: vi.fn(), getURL: vi.fn(() => ''), loadURL: vi.fn(() => Promise.resolve()), @@ -276,6 +278,7 @@ export class BrowserWindow { getNormalBounds = vi.fn(() => ({ x: 0, y: 0, width: 1360, height: 860 })) getBounds = vi.fn(() => ({ x: 1292, y: 41, width: 420, height: 150 })) setBounds = vi.fn() + setContentSize = vi.fn() loadURL = vi.fn(() => Promise.resolve()) loadFile = vi.fn(() => Promise.resolve()) focus = vi.fn() diff --git a/apps/desktop/src/test/setup.ts b/apps/desktop/src/test/setup.ts new file mode 100644 index 00000000000..f855b236293 --- /dev/null +++ b/apps/desktop/src/test/setup.ts @@ -0,0 +1,4 @@ +import { vi } from 'vitest' + +/** Existing behavior tests stub prompt responses at the shared dialog boundary. */ +vi.mock('@/main/dialogs', () => import('@/test/dialog-mock')) diff --git a/apps/desktop/static/dialog.html b/apps/desktop/static/dialog.html new file mode 100644 index 00000000000..e6c97e1173d --- /dev/null +++ b/apps/desktop/static/dialog.html @@ -0,0 +1,17 @@ + + + + + + + Sim + + + + +
+ + diff --git a/apps/desktop/static/offline.html b/apps/desktop/static/offline.html index 87313fe2cff..6d46019d6a5 100644 --- a/apps/desktop/static/offline.html +++ b/apps/desktop/static/offline.html @@ -2,257 +2,16 @@ + Sim - Can’t connect - + + -
- - - - - - - - -
-
-
-

Can’t connect to Sim

-

- Sim couldn’t reach the server. Check your internet connection, then try again. -

-
- - - -
-
-
-
- +
diff --git a/apps/desktop/static/server.html b/apps/desktop/static/server.html index 44c50e546c2..e096b082283 100644 --- a/apps/desktop/static/server.html +++ b/apps/desktop/static/server.html @@ -2,272 +2,16 @@ + Sim - Server - + + -
-
-

Sim server

-

- Point this app at your own Sim deployment. Self-hosted servers must use HTTPS; localhost may - use HTTP. -

- - -
-
- - -
-
- +
diff --git a/apps/desktop/tsconfig.json b/apps/desktop/tsconfig.json index 77d5b0963ea..3f6edb1844f 100644 --- a/apps/desktop/tsconfig.json +++ b/apps/desktop/tsconfig.json @@ -1,10 +1,11 @@ { "extends": "@sim/tsconfig/base.json", "compilerOptions": { - "lib": ["ES2022"], + "lib": ["ES2022", "DOM", "DOM.Iterable"], "paths": { "@/*": ["./src/*"] - } + }, + "jsx": "react-jsx" }, "include": ["src/**/*", "scripts/**/*", "e2e/**/*", "playwright.config.ts", "vitest.config.ts"], "exclude": ["node_modules", "dist", "release"] diff --git a/apps/desktop/turbo.json b/apps/desktop/turbo.json new file mode 100644 index 00000000000..975eab257e2 --- /dev/null +++ b/apps/desktop/turbo.json @@ -0,0 +1,13 @@ +{ + "extends": ["//"], + "tasks": { + "build": { + "inputs": [ + "$TURBO_DEFAULT$", + "$TURBO_ROOT$/apps/sim/app/_styles/**", + "$TURBO_ROOT$/apps/sim/postcss.config.mjs", + "$TURBO_ROOT$/apps/sim/lib/postcss/**" + ] + } + } +} diff --git a/apps/desktop/vitest.config.ts b/apps/desktop/vitest.config.ts index 9718634dfc7..274f7646a01 100644 --- a/apps/desktop/vitest.config.ts +++ b/apps/desktop/vitest.config.ts @@ -5,6 +5,7 @@ export default defineConfig({ test: { environment: 'node', globals: true, + setupFiles: ['src/test/setup.ts'], include: ['src/**/*.test.ts'], exclude: ['**/node_modules/**', '**/dist/**', '**/e2e/**'], pool: 'threads', diff --git a/apps/docs/content/docs/academy/agents/memory.mdx b/apps/docs/content/docs/academy/agents/memory.mdx index 483a70c404d..48a11d441d7 100644 --- a/apps/docs/content/docs/academy/agents/memory.mdx +++ b/apps/docs/content/docs/academy/agents/memory.mdx @@ -18,6 +18,8 @@ import { AV_MEMORY_WORKFLOW } from '@/components/workflow-preview/academy-video- By default, an agent keeps nothing between runs: every conversation starts completely fresh. The Memory setting changes that: choose Conversation, give it a conversation ID, and everything said under that key is kept and loaded back before the model runs. +Uploaded attachments stay linked to the message that included them. Memory stores file references; each later run reads the accessible files again and prepares them for the selected provider. Attachments follow the selected memory window and the source file's storage retention. A replay can include up to 20 attachment references; use a smaller memory window for longer file-heavy conversations. Files omitted by older versions of memory need to be attached again. + @@ -313,11 +326,11 @@ SSO provisioning creates internal organization members but does not grant worksp }, { question: "What is the Domain field used for?", - answer: "The domain (e.g. company.com) is how Sim routes users to the right identity provider. When a user enters their email on the SSO sign-in page, Sim matches their email domain to the provider that serves it and redirects them there. Each verified domain routes to one provider, and an organization can serve different domains with different providers." + answer: "The domain (e.g. company.com) is how Sim routes users to the right identity provider. When a user enters their email on the SSO sign-in page, Sim matches their email domain to the provider that serves it and redirects them there. If a domain has more than one provider, Sim uses the one marked Primary." }, { question: "Can we use more than one identity provider?", - answer: "Yes. Add one provider per verified domain: for example Okta for eng.acme.com and Microsoft Entra ID for acme.com. Sign-in routes by email domain, so a single domain cannot be split across two providers. SCIM provisioning stays organization-wide and works alongside any number of providers." + answer: "Yes. Different domains can use different providers, for example Okta for eng.acme.com and Microsoft Entra ID for acme.com. A domain can also have several providers while you move between them: sign-in uses its primary provider, and the others are reachable through a test sign-in link. SCIM provisioning stays organization-wide and works alongside any number of providers." }, { question: "Do I need to provide OIDC endpoints manually?", diff --git a/apps/docs/content/docs/workflows/blocks/agent.mdx b/apps/docs/content/docs/workflows/blocks/agent.mdx index 0ef2b325ac5..b880916cd78 100644 --- a/apps/docs/content/docs/workflows/blocks/agent.mdx +++ b/apps/docs/content/docs/workflows/blocks/agent.mdx @@ -33,12 +33,14 @@ Files for the model to read: images for a vision-capable model, or documents for ### Tools -Capabilities the agent can call while it runs: search a knowledge base, send a Slack message, run a [Function](/workflows/blocks/function), call any of the [integrations](/integrations), or use a [custom tool](/agents/custom-tools) or [MCP server](/agents/mcp) you've added. The model decides which to call and when. (For where tools come from and when to reach for which, see [Agents](/agents).) Each tool has a usage control: +Capabilities the agent can call while it runs: search a knowledge base, send a Slack message, run a [Function](/workflows/blocks/function), call any of the [integrations](/integrations), or use a [custom tool](/agents/custom-tools) or [MCP server](/agents/mcp) you've added. The model decides which to call and when. (For where tools come from and when to reach for which, see [Agents](/agents).) Expand a tool to set its **Permission Mode**: - **Auto.** The model calls it when the context warrants. - **Force.** The model must call it on every run. - **None.** The tool is hidden from the model, which disables it without removing it from the block. +To pick the mode when the workflow runs, use the switch next to Permission Mode to change it to a variable, then enter a reference such as ``. The value must resolve to `auto`, `force`, or `none`. Any other value stops the run before the model is called. + ### Skills [Agent skills](/agents/skills) the agent can load on demand: reusable instruction packages like a coding standard or a support playbook. Only the skill names sit in context up front, and the agent loads the full instructions when it decides a skill is relevant. diff --git a/apps/docs/openapi-v2-workflows.json b/apps/docs/openapi-v2-workflows.json index b557a9b373e..c92eacd9b5e 100644 --- a/apps/docs/openapi-v2-workflows.json +++ b/apps/docs/openapi-v2-workflows.json @@ -7995,6 +7995,11 @@ "enum": ["auto", "force", "none"], "description": "When the Agent may call the tool: `auto` lets the model decide, `force` requires a call, and `none` disables it. Omitted means `auto`." }, + "usageControlExpression": { + "type": "string", + "maxLength": 2048, + "description": "Variable-capable tool mode value used when the matching canonical mode is `advanced`. It must resolve to `auto`, `force`, or `none` at execution time." + }, "params": { "type": "object", "propertyNames": { @@ -8041,6 +8046,11 @@ "type": "string", "enum": ["auto", "force", "none"], "description": "When the Agent may call the tool: `auto` lets the model decide, `force` requires a call, and `none` disables it. Omitted means `auto`." + }, + "usageControlExpression": { + "type": "string", + "maxLength": 2048, + "description": "Variable-capable tool mode value used when the matching canonical mode is `advanced`. It must resolve to `auto`, `force`, or `none` at execution time." } }, "required": ["type", "customToolId"], @@ -8109,6 +8119,11 @@ "type": "string", "enum": ["auto", "force", "none"], "description": "When the Agent may call the tool: `auto` lets the model decide, `force` requires a call, and `none` disables it. Omitted means `auto`." + }, + "usageControlExpression": { + "type": "string", + "maxLength": 2048, + "description": "Variable-capable tool mode value used when the matching canonical mode is `advanced`. It must resolve to `auto`, `force`, or `none` at execution time." } }, "required": ["type", "schema", "code"], @@ -8174,6 +8189,11 @@ "type": "string", "enum": ["auto", "force", "none"], "description": "When the Agent may call the tool: `auto` lets the model decide, `force` requires a call, and `none` disables it. Omitted means `auto`." + }, + "usageControlExpression": { + "type": "string", + "maxLength": 2048, + "description": "Variable-capable tool mode value used when the matching canonical mode is `advanced`. It must resolve to `auto`, `force`, or `none` at execution time." } }, "required": ["type", "params"], @@ -8282,6 +8302,11 @@ "type": "string", "enum": ["auto", "force", "none"], "description": "When the Agent may call the tool: `auto` lets the model decide, `force` requires a call, and `none` disables it. Omitted means `auto`." + }, + "usageControlExpression": { + "type": "string", + "maxLength": 2048, + "description": "Variable-capable tool mode value used when the matching canonical mode is `advanced`. It must resolve to `auto`, `force`, or `none` at execution time." } }, "required": ["type", "params"], diff --git a/apps/sim/app/(interfaces)/chat/[identifier]/chat.tsx b/apps/sim/app/(interfaces)/chat/[identifier]/chat.tsx index f066279140d..beadfdc58e9 100644 --- a/apps/sim/app/(interfaces)/chat/[identifier]/chat.tsx +++ b/apps/sim/app/(interfaces)/chat/[identifier]/chat.tsx @@ -6,6 +6,7 @@ import { generateId } from '@sim/utils/id' import { AGENT_STREAM_PROTOCOL_HEADER, AGENT_STREAM_PROTOCOL_V1, + CHAT_OUTPUT_PROTOCOL_V1, } from '@/lib/workflows/streaming/agent-stream-protocol' import { DesktopTitleBarLane } from '@/app/_shell/desktop-title-bar' import { @@ -236,7 +237,7 @@ export default function ChatClient({ identifier }: { identifier: string }) { headers: { 'Content-Type': 'application/json', 'X-Requested-With': 'XMLHttpRequest', - [AGENT_STREAM_PROTOCOL_HEADER]: AGENT_STREAM_PROTOCOL_V1, + [AGENT_STREAM_PROTOCOL_HEADER]: `${AGENT_STREAM_PROTOCOL_V1}, ${CHAT_OUTPUT_PROTOCOL_V1}`, }, body: JSON.stringify(payload), credentials: 'same-origin', diff --git a/apps/sim/app/(interfaces)/chat/components/message/components/file-download.test.tsx b/apps/sim/app/(interfaces)/chat/components/message/components/file-download.test.tsx new file mode 100644 index 00000000000..b386babcad2 --- /dev/null +++ b/apps/sim/app/(interfaces)/chat/components/message/components/file-download.test.tsx @@ -0,0 +1,69 @@ +/** + * @vitest-environment jsdom + */ +import { act } from 'react' +import { createRoot } from 'react-dom/client' +import { afterEach, describe, expect, it, vi } from 'vitest' +import { ChatFileDownload } from '@/app/(interfaces)/chat/components/message/components/file-download' +import type { ChatFile } from '@/app/(interfaces)/chat/components/message/message' + +const imageFile: ChatFile = { + id: 'file-image', + name: 'generated.png', + key: 'execution/generated.png', + url: 'https://files.example.com/generated.png', + size: 3, + type: 'image/png', + base64: 'YWJj', +} + +const mounts: Array<() => void> = [] + +function renderFile(file: ChatFile): HTMLDivElement { + ;(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true + const container = document.createElement('div') + const root = createRoot(container) + act(() => root.render()) + mounts.push(() => act(() => root.unmount())) + return container +} + +afterEach(() => { + while (mounts.length) mounts.pop()?.() + vi.restoreAllMocks() +}) + +describe('ChatFileDownload', () => { + it('previews returned image bytes inline without requiring a workspace session', () => { + const container = renderFile(imageFile) + const image = container.querySelector('img') + expect(image?.getAttribute('src')).toBe('data:image/png;base64,YWJj') + expect(image?.alt).toBe('generated.png') + expect(container.querySelector('button')?.textContent).toContain('generated.png') + }) + + it('uses the file URL when inline bytes are unavailable', () => { + const container = renderFile({ ...imageFile, base64: undefined }) + expect(container.querySelector('img')?.getAttribute('src')).toBe(imageFile.url) + }) + + it('uses the canonical serve route for unsafe file URLs', () => { + const container = renderFile({ ...imageFile, base64: undefined, url: 'javascript:alert(1)' }) + expect(container.querySelector('img')?.getAttribute('src')).toBe( + '/api/files/serve/execution%2Fgenerated.png?context=execution' + ) + }) + + it('keeps a download available when an image preview fails', () => { + const container = renderFile(imageFile) + act(() => container.querySelector('img')!.dispatchEvent(new Event('error'))) + expect(container.querySelector('img')).toBeNull() + expect(container.querySelector('button')?.textContent).toContain('generated.png') + }) + + it('renders documents as downloads without an image preview', () => { + const container = renderFile({ ...imageFile, name: 'report.pdf', type: 'application/pdf' }) + expect(container.querySelector('img')).toBeNull() + expect(container.querySelector('button')?.textContent).toContain('report.pdf') + }) +}) diff --git a/apps/sim/app/(interfaces)/chat/components/message/components/file-download.tsx b/apps/sim/app/(interfaces)/chat/components/message/components/file-download.tsx index a043bd433df..f9005b8b7e5 100644 --- a/apps/sim/app/(interfaces)/chat/components/message/components/file-download.tsx +++ b/apps/sim/app/(interfaces)/chat/components/message/components/file-download.tsx @@ -51,6 +51,8 @@ function isImageFile(mimeType: string): boolean { } function getFileUrl(file: ChatFile): string { + if (file.base64) return `data:${file.type};base64,${file.base64}` + if (isSafeHttpUrl(file.url)) return file.url return `/api/files/serve/${encodeURIComponent(file.key)}?context=${file.context || 'execution'}` } @@ -76,6 +78,8 @@ async function triggerDownload(url: string, filename: string): Promise { export function ChatFileDownload({ file }: ChatFileDownloadProps) { const [isDownloading, setIsDownloading] = useState(false) + const [failedPreviewUrl, setFailedPreviewUrl] = useState(null) + const fileUrl = getFileUrl(file) const handleDownload = async () => { if (isDownloading) return @@ -109,25 +113,36 @@ export function ChatFileDownload({ file }: ChatFileDownloadProps) { } return ( - +
+ {isImageFile(file.type) && failedPreviewUrl !== fileUrl && ( + {file.name} setFailedPreviewUrl(fileUrl)} + /> + )} + +
) } diff --git a/apps/sim/app/(interfaces)/chat/components/message/message.test.tsx b/apps/sim/app/(interfaces)/chat/components/message/message.test.tsx index 43daf5cd018..1726c423397 100644 --- a/apps/sim/app/(interfaces)/chat/components/message/message.test.tsx +++ b/apps/sim/app/(interfaces)/chat/components/message/message.test.tsx @@ -87,6 +87,41 @@ describe('ClientChatMessage thinking chrome (Step 6)', () => { } }) + it('renders no message row or copy action for empty assistant output', () => { + const { container, unmount } = renderMessage({ + id: 'empty-output', + type: 'assistant', + content: '', + files: [], + timestamp: new Date(), + }) + mounts.push(unmount) + expect(container.innerHTML).toBe('') + }) + + it('does not show a copy action for a file-only response', () => { + const { container, unmount } = renderMessage({ + id: 'file-output', + type: 'assistant', + content: '', + files: [ + { + id: 'file-1', + name: 'image.png', + url: '/image.png', + key: 'image.png', + size: 3, + type: 'image/png', + }, + ], + timestamp: new Date(), + }) + mounts.push(unmount) + expect(container.querySelector('[data-message-id]')).not.toBeNull() + expect(container.querySelector('[data-testid="answer"]')).toBeNull() + expect(container.textContent).not.toContain('Copy to clipboard') + }) + it('does not show thinking chrome when thinking is absent or empty', () => { const without = renderMessage({ id: '1', diff --git a/apps/sim/app/(interfaces)/chat/components/message/message.tsx b/apps/sim/app/(interfaces)/chat/components/message/message.tsx index ea2c6d4ab1a..8b4cd29647f 100644 --- a/apps/sim/app/(interfaces)/chat/components/message/message.tsx +++ b/apps/sim/app/(interfaces)/chat/components/message/message.tsx @@ -30,6 +30,7 @@ export interface ChatFile { size: number type: string context?: string + base64?: string } /** Chat surface tool chip — the shared lifecycle chip plus its block id. */ @@ -100,11 +101,13 @@ function openAttachmentPreview(name: string, dataUrl: string): void { setTimeout(() => URL.revokeObjectURL(blobUrl), 60_000) } +interface ClientChatMessageProps { + message: ChatMessage +} + export const ClientChatMessage = memo(function ClientChatMessage({ message, -}: { - message: ChatMessage -}) { +}: ClientChatMessageProps) { const [isCopied, setIsCopied] = useState(false) const isJsonObject = typeof message.content === 'object' && message.content !== null @@ -113,6 +116,12 @@ export const ClientChatMessage = memo(function ClientChatMessage({ const cleanTextContent = message.content const hasThinking = typeof message.thinking === 'string' && message.thinking.length > 0 const hasToolCalls = Array.isArray(message.toolCalls) && message.toolCalls.length > 0 + const hasContent = isJsonObject || Boolean((message.content as string).trim()) + const hasFiles = Boolean(message.files?.length) + + if (message.type === 'assistant' && !hasContent && !hasFiles && !hasThinking && !hasToolCalls) { + return null + } const content = message.type === 'user' ? ( @@ -238,15 +247,17 @@ export const ClientChatMessage = memo(function ClientChatMessage({ isStreaming={message.isToolStreaming} /> )} -
- {isJsonObject ? ( - 
-                    {JSON.stringify(cleanTextContent, null, 2)}
-
- ) : ( - - )} -
+ {hasContent && ( +
+ {isJsonObject ? ( + 
+                      {JSON.stringify(cleanTextContent, null, 2)}
+
+ ) : ( + + )} +
+ )} {message.files && message.files.length > 0 && (
@@ -257,7 +268,7 @@ export const ClientChatMessage = memo(function ClientChatMessage({ )} {message.type === 'assistant' && !isJsonObject && !message.isInitialMessage && (
- {!message.isStreaming && ( + {!message.isStreaming && hasContent && ( +
+ + Reload + +
) diff --git a/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/components/browser-session/browser-theme-notice.tsx b/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/components/browser-session/browser-theme-notice.tsx index 050f3b76228..b1c53816c34 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/components/browser-session/browser-theme-notice.tsx +++ b/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/components/browser-session/browser-theme-notice.tsx @@ -1,7 +1,7 @@ 'use client' import { useState } from 'react' -import { Banner, Button } from '@sim/emcn' +import { Banner, Chip } from '@sim/emcn' import { X } from '@sim/emcn/icons' import { sendBrowserPanelAction } from '@/lib/browser-agent/transport' @@ -22,25 +22,8 @@ export function BrowserThemeNotice({ scopeId }: BrowserThemeNoticeProps) { Some sites apply theme changes after a reload.

- - + sendBrowserPanelAction('reload', {}, scopeId)}>Reload page + setVisible(false)} />
diff --git a/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/components/terminal-session/terminal-session.tsx b/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/components/terminal-session/terminal-session.tsx index 3b058443c8b..3ce52bacb91 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/components/terminal-session/terminal-session.tsx +++ b/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/components/terminal-session/terminal-session.tsx @@ -20,7 +20,7 @@ import { type IBufferRange, Terminal } from '@xterm/xterm' import { useTheme } from 'next-themes' import { useContextMenu } from '@/hooks/use-context-menu' import '@xterm/xterm/css/xterm.css' -import { describeRunningCommand, type TerminalTabsState } from '@sim/terminal-protocol' +import type { TerminalTabsState } from '@sim/terminal-protocol' import { getDesktopBridge } from '@/lib/desktop' import { loadDesktopTerminalAppearance, @@ -47,6 +47,7 @@ import { writeToTerminal, } from '@/lib/terminal/transport' import { TerminalContextMenu } from '@/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/components/terminal-session/terminal-context-menu' +import { useTerminalCloseConfirmation } from '@/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/components/terminal-session/use-terminal-close-confirmation' import { useDesktopPreferenceMutation } from '@/hooks/use-desktop-preference-mutation' import { useCopilotTerminalStore } from '@/stores/copilot-terminal/store' import type { ChatContext, TerminalTextSelection } from '@/stores/panel' @@ -234,6 +235,7 @@ const TerminalView = memo(function TerminalView({ const hostRef = useRef(null) const terminalRef = useRef(null) const fitRef = useRef(null) + const { confirmTerminalClose, confirmationDialog } = useTerminalCloseConfirmation(scopeId) const [currentZoom, setCurrentZoom] = useState(defaultZoom) // Being the selected tab is not enough to be on screen: the whole panel is // hidden whenever another resource is open. @@ -590,20 +592,12 @@ const TerminalView = memo(function TerminalView({ }) }, [scopeId]) - // Scoped to the terminal that was right-clicked, not the active one. - const closeThisTerminal = useCallback(() => { - if ( - running && - !window.confirm( - `${describeRunningCommand(running)} is still running. Close this terminal and stop it?` - ) - ) { - return - } + async function closeThisTerminal() { + if (!(await confirmTerminalClose([terminalId]))) return void closeTerminal(terminalId, scopeId).catch(() => { toast.error('Could not close that terminal. Please try again.') }) - }, [running, terminalId, scopeId]) + } // An inactive tab is `display: none`, not merely invisible. xterm watches its // element with an IntersectionObserver and pauses rendering once it stops @@ -613,6 +607,7 @@ const TerminalView = memo(function TerminalView({ // xterm re-measures and does a full refresh when the element comes back. return ( <> + {confirmationDialog}
void)[] = [] + +function renderHook(useHook: () => ReturnType) { + let value: ReturnType | undefined + function Harness() { + value = useHook() + return null + } + const container = document.createElement('div') + const root = createRoot(container) + let mounted = true + const unmount = () => { + if (!mounted) return + mounted = false + act(() => root.unmount()) + } + cleanups.push(unmount) + act(() => root.render()) + return { + result: { + get current() { + if (!value) throw new Error('Hook was not rendered') + return value + }, + }, + rerender: () => act(() => root.render()), + unmount, + } +} + +afterEach(() => { + for (const cleanup of cleanups.splice(0)) cleanup() +}) + +const { getState } = vi.hoisted(() => ({ getState: vi.fn() })) +vi.mock('@/stores/copilot-terminal/store', () => ({ useCopilotTerminalStore: { getState } })) +vi.mock('@sim/emcn', () => ({ ChipConfirmModal: vi.fn(), toast: { warning: vi.fn() } })) + +function setRunning(running: string | null) { + getState.mockReturnValue({ + sessions: { scope: { tabs: { tabs: [{ terminalId: 'terminal', running }] } } }, + }) +} + +beforeEach(() => { + setRunning('sleep 1') +}) + +describe('useTerminalCloseConfirmation', () => { + it('waits for approval and rejects duplicate confirmation requests', async () => { + const { result } = renderHook(() => useTerminalCloseConfirmation('scope')) + let decision: Promise | undefined + act(() => { + decision = result.current.confirmTerminalClose(['terminal']) + }) + await expect(result.current.confirmTerminalClose(['terminal'])).resolves.toBe(false) + act(() => { + result.current.confirmationDialog?.props.confirm.onClick() + }) + await expect(decision).resolves.toBe(true) + expect(result.current.confirmationDialog).toBeNull() + }) + + it('refuses to close when the running command changed while the dialog was open', async () => { + const { result } = renderHook(() => useTerminalCloseConfirmation('scope')) + let decision: Promise | undefined + act(() => { + decision = result.current.confirmTerminalClose(['terminal']) + }) + setRunning('build') + act(() => { + result.current.confirmationDialog?.props.confirm.onClick() + }) + await expect(decision).resolves.toBe(false) + }) + + it('cancels pending confirmation when the caller unmounts', async () => { + const { result, unmount } = renderHook(() => useTerminalCloseConfirmation('scope')) + let decision: Promise | undefined + act(() => { + decision = result.current.confirmTerminalClose(['terminal']) + }) + unmount() + await expect(decision).resolves.toBe(false) + }) + + it('does not resurrect a cancelled dialog when returning to its scope', async () => { + let scopeId = 'scope' + const { result, rerender } = renderHook(() => useTerminalCloseConfirmation(scopeId)) + let decision: Promise | undefined + act(() => { + decision = result.current.confirmTerminalClose(['terminal']) + }) + scopeId = 'another-scope' + rerender() + await expect(decision).resolves.toBe(false) + expect(result.current.confirmationDialog).toBeNull() + scopeId = 'scope' + rerender() + expect(result.current.confirmationDialog).toBeNull() + act(() => { + decision = result.current.confirmTerminalClose(['terminal']) + }) + act(() => { + result.current.confirmationDialog?.props.confirm.onClick() + }) + await expect(decision).resolves.toBe(true) + }) +}) diff --git a/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/components/terminal-session/use-terminal-close-confirmation.tsx b/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/components/terminal-session/use-terminal-close-confirmation.tsx new file mode 100644 index 00000000000..aee7686ceb3 --- /dev/null +++ b/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/components/terminal-session/use-terminal-close-confirmation.tsx @@ -0,0 +1,87 @@ +import { useCallback, useEffect, useRef, useState } from 'react' +import { ChipConfirmModal, toast } from '@sim/emcn' +import { describeRunningCommand } from '@sim/terminal-protocol' +import { useCopilotTerminalStore } from '@/stores/copilot-terminal/store' + +interface TerminalCloseRequest { + scopeId: string + targets: { terminalId: string; running: string | null }[] + resolve: (confirmed: boolean) => void +} + +/** Confirms the captured terminals and rechecks their commands before closing them. */ +export function useTerminalCloseConfirmation(scopeId: string) { + const pendingRef = useRef(null) + const [request, setRequest] = useState(null) + const [previousScopeId, setPreviousScopeId] = useState(scopeId) + + if (previousScopeId !== scopeId) { + setPreviousScopeId(scopeId) + setRequest(null) + } + + useEffect(() => { + return () => { + pendingRef.current?.resolve(false) + pendingRef.current = null + } + }, [scopeId]) + + const confirmTerminalClose = useCallback( + (terminalIds: string[]): Promise => { + if (pendingRef.current) return Promise.resolve(false) + const tabs = useCopilotTerminalStore.getState().sessions[scopeId]?.tabs.tabs ?? [] + const targets = tabs + .filter((tab) => terminalIds.includes(tab.terminalId)) + .map(({ terminalId, running }) => ({ terminalId, running })) + if (!targets.some((target) => target.running)) return Promise.resolve(true) + return new Promise((resolve) => { + const next = { scopeId, targets, resolve } + pendingRef.current = next + setRequest(next) + }) + }, + [scopeId] + ) + + function settle(confirmed: boolean) { + const pending = pendingRef.current + if (!pending) return + if (confirmed) { + const tabs = useCopilotTerminalStore.getState().sessions[pending.scopeId]?.tabs.tabs ?? [] + confirmed = pending.targets.every((target) => { + const current = tabs.find((tab) => tab.terminalId === target.terminalId) + return current && (!current.running || current.running === target.running) + }) + if (!confirmed) toast.warning('A terminal changed. Review it before closing.') + } + pendingRef.current = null + setRequest(null) + pending.resolve(confirmed) + } + + const running = + request?.targets.flatMap((target) => (target.running ? [target.running] : [])) ?? [] + const confirmationDialog = + request?.scopeId === scopeId ? ( + { + if (!open) settle(false) + }} + title={request.targets.length === 1 ? 'Close terminal?' : 'Close terminals?'} + text={ + running.length === 1 + ? `${describeRunningCommand(running[0])} is still running. Closing the terminal will stop it.` + : `${running.length} selected terminals have a running process. Closing these terminals will stop them.` + } + confirm={{ + label: request.targets.length === 1 ? 'Close terminal' : 'Close terminals', + variant: 'destructive', + onClick: () => settle(true), + }} + /> + ) : null + + return { confirmTerminalClose, confirmationDialog } +} diff --git a/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-tabs/resource-tabs.tsx b/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-tabs/resource-tabs.tsx index 9f1d18d94ad..1e17cf742fb 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-tabs/resource-tabs.tsx +++ b/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-tabs/resource-tabs.tsx @@ -21,7 +21,7 @@ import { toast, } from '@sim/emcn' import { Columns3, Eye, Pencil } from '@sim/emcn/icons' -import { describeRunningCommand, type TerminalTabState } from '@sim/terminal-protocol' +import type { TerminalTabState } from '@sim/terminal-protocol' import { browserTabTitle } from '@/lib/browser-agent/tab-label' import { openBrowserTab, @@ -37,6 +37,7 @@ import { closeTerminal, openTerminal, reorderTerminal } from '@/lib/terminal/tra import type { PreviewMode } from '@/app/workspace/[workspaceId]/files/components/file-viewer' import { useMothershipResources } from '@/app/workspace/[workspaceId]/home/components/mothership-resources-context' import { AddResourceDropdown } from '@/app/workspace/[workspaceId]/home/components/mothership-view/components/add-resource-dropdown' +import { useTerminalCloseConfirmation } from '@/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/components/terminal-session/use-terminal-close-confirmation' import { getResourceConfig } from '@/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-registry' import { RESOURCE_HEADER_CLASSES, @@ -77,24 +78,6 @@ const ADD_RESOURCE_EXCLUDED_TYPES: readonly MothershipResourceType[] = [ const EMPTY_TERMINAL_TABS: TerminalTabState[] = [] -/** Closing a shell mid-command stops that command, so the user confirms first. */ -function confirmClosingRunningTerminals( - targets: readonly MothershipResource[], - terminalTabs: readonly TerminalTabState[] -): boolean { - const running = targets.flatMap((resource) => { - if (resource.type !== 'terminal') return [] - const tab = terminalTabs.find((entry) => terminalResourceId(entry.terminalId) === resource.id) - return tab?.running ? [tab.running] : [] - }) - if (running.length === 0) return true - return window.confirm( - running.length === 1 - ? `${describeRunningCommand(running[0])} is still running. Close this terminal and stop it?` - : `${running.length} selected terminals have a running process. Close them anyway?` - ) -} - /** * Returns the id of the nearest resource to `idx` that is in `filter` * (or any resource if `filter` is null). Returns undefined if nothing qualifies. @@ -242,6 +225,7 @@ export function ResourceTabs({ const removeResource = useRemoveChatResource(chatId) const reorderResources = useReorderChatResources(chatId) + const { confirmTerminalClose, confirmationDialog } = useTerminalCloseConfirmation(desktopScopeId) const [selectedIds, setSelectedIds] = useState>(new Set()) const anchorIdRef = useRef(null) const prevChatIdRef = useRef(chatId) @@ -407,13 +391,16 @@ export function ResourceTabs({ ) const handleClose = useCallback( - (id: string) => { + async (id: string) => { const index = resources.findIndex((r) => r.id === id) const resource = resources[index] if (!resource) return const isMulti = selectedIds.has(resource.id) && selectedIds.size > 1 const targets = isMulti ? resources.filter((r) => selectedIds.has(r.id)) : [resource] - if (!confirmClosingRunningTerminals(targets, terminalTabs)) return + const terminalIds = targets + .filter((target) => target.type === 'terminal') + .map((target) => terminalIdFromResourceId(target.id)) + if (!(await confirmTerminalClose(terminalIds))) return // Closing the shown tab moves to its neighbour, right then left, so the // strip does not fall back to its last tab and jump. For a desktop tab // this is also the neighbour the desktop app itself picks. @@ -469,7 +456,7 @@ export function ResourceTabs({ resources, selectResource, selectedIds, - terminalTabs, + confirmTerminalClose, ] ) @@ -574,38 +561,44 @@ export function ResourceTabs({ ) : null return ( - - -
- } - // A bare fragment is always truthy, so the empty case has to be `null` or - // the strip renders an empty trailing cluster. - endActions={ - actions || previewToggle ? ( - <> - {actions} - {previewToggle} - - ) : null - } - /> + <> + {confirmationDialog} + + + + } + // A bare fragment is always truthy, so the empty case has to be `null` or + // the strip renders an empty trailing cluster. + endActions={ + actions || previewToggle ? ( + <> + {actions} + {previewToggle} + + ) : null + } + /> + ) } diff --git a/apps/sim/app/workspace/[workspaceId]/not-found.tsx b/apps/sim/app/workspace/[workspaceId]/not-found.tsx index 6a29b6c5a26..58cc12614db 100644 --- a/apps/sim/app/workspace/[workspaceId]/not-found.tsx +++ b/apps/sim/app/workspace/[workspaceId]/not-found.tsx @@ -1,8 +1,7 @@ 'use client' -import { Button, buttonVariants } from '@sim/emcn' +import { Chip, ChipLink } from '@sim/emcn' import { ArrowLeft, Compass, Home } from '@sim/emcn/icons' -import Link from 'next/link' import { useParams, useRouter } from 'next/navigation' import { ErrorShell } from '@/app/workspace/[workspaceId]/components' @@ -17,14 +16,12 @@ export default function WorkspaceNotFound() { description="The page you're looking for doesn't exist or has been moved. Head back to your workspace to keep building." icon={} > - - - + + Return home - + ) } diff --git a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/row-modal/row-modal.test.tsx b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/row-modal/row-modal.test.tsx index 1c0a37a21c2..18573ba0f98 100644 --- a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/row-modal/row-modal.test.tsx +++ b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/row-modal/row-modal.test.tsx @@ -100,7 +100,7 @@ const table: TableInfo = { const row: TableRow = { id: 'row-1', - data: { expires_at: Date.parse('2026-11-01T08:00:00Z') / 1000 }, + data: { expires_at: '2026-11-01T01:00:00-07:00' }, executions: {}, position: 0, createdAt: '2026-01-01T00:00:00Z', @@ -119,7 +119,7 @@ describe('RowModal expiration editing', () => { mockUpdateRow.mockResolvedValue(undefined) }) - it('waits for the saved timezone, freezes it, and chooses the later repeated hour', async () => { + it('preserves expiration offsets while timezone settings load or change', async () => { mockUseTimezoneState.mockReturnValue({ timezone: 'Asia/Tokyo', status: 'loading' }) const container = document.createElement('div') document.body.appendChild(container) @@ -136,12 +136,9 @@ describe('RowModal expiration editing', () => { ;(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true act(() => root.render(createElement(RowModal, props))) - expect(container.querySelector('[aria-label="Edit expires_at"]')?.textContent).toBe( - 'Loading timezone…' - ) - expect(container.querySelector('[data-testid="time"]')).toBeNull() + expect(container.querySelector('[data-testid="time"]')?.value).toBe('01:00') expect(container.querySelector('[data-testid="submit"]')?.disabled).toBe( - true + false ) mockUseTimezoneState.mockReturnValue({ @@ -165,7 +162,7 @@ describe('RowModal expiration editing', () => { expect(mockUpdateRow).toHaveBeenCalledWith({ rowId: 'row-1', - data: { expires_at: Date.parse('2026-11-01T09:30:00Z') / 1000 }, + data: { expires_at: '2026-11-01T01:30:00-07:00' }, }) expect(props.onSuccess).toHaveBeenCalledTimes(1) @@ -209,7 +206,7 @@ describe('RowModal expiration editing', () => { container.remove() }) - it('blocks an invalid saved timezone with the plain-text guidance', () => { + it('allows expiration edits even when the saved timezone is invalid', () => { mockUseTimezoneState.mockReturnValue({ timezone: 'America/Los_Angeles', savedTimezone: 'Mars/Olympus', @@ -229,18 +226,11 @@ describe('RowModal expiration editing', () => { act(() => root.render(createElement(RowModal, props))) - const blockedField = container.querySelector( - '[aria-label="Edit expires_at"]' - ) - expect(blockedField?.textContent).toBe(String(row.data.expires_at)) + expect(container.querySelector('[data-testid="time"]')?.value).toBe('01:00') expect(container.querySelector('[data-testid="submit"]')?.disabled).toBe( - true + false ) expect(mockToastError).not.toHaveBeenCalled() - act(() => blockedField?.click()) - expect(mockToastError).toHaveBeenCalledWith( - 'Your saved timezone “Mars/Olympus” is invalid. Update it in Settings → General before editing Date or Expiration cells.' - ) act(() => root.unmount()) container.remove() }) @@ -259,11 +249,19 @@ describe('RowModal expiration editing', () => { schema: { columns: [ { name: 'name', type: 'string' }, + { name: 'starts_at', type: 'date' }, { name: 'expires_at', type: 'ttl' }, ], }, } - const mixedRow = { ...row, data: { name: 'Ada', expires_at: row.data.expires_at } } + const mixedRow = { + ...row, + data: { + name: 'Ada', + expires_at: row.data.expires_at, + starts_at: '2026-09-07T12:00:00-07:00', + }, + } const props = { mode: 'edit' as const, isOpen: true, @@ -276,12 +274,10 @@ describe('RowModal expiration editing', () => { act(() => root.render(createElement(RowModal, props))) const nameInput = container.querySelector('[data-testid="modal-input"]') - const blockedField = container.querySelector( - '[aria-label="Edit expires_at"]' - ) + const blockedField = container.querySelector('[aria-label="Edit starts_at"]') const submit = container.querySelector('[data-testid="submit"]') expect(nameInput?.value).toBe('Ada') - expect(blockedField?.textContent).toBe(String(row.data.expires_at)) + expect(blockedField?.textContent).toBe(mixedRow.data.starts_at) expect(submit?.disabled).toBe(false) act(() => changeInput(nameInput as HTMLInputElement, 'Grace')) @@ -289,7 +285,7 @@ describe('RowModal expiration editing', () => { expect(mockUpdateRow).toHaveBeenCalledWith({ rowId: 'row-1', - data: { name: 'Grace' }, + data: { name: 'Grace', expires_at: row.data.expires_at }, }) expect(props.onSuccess).toHaveBeenCalledTimes(1) expect(mockToastError).not.toHaveBeenCalled() diff --git a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/row-modal/row-modal.tsx b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/row-modal/row-modal.tsx index bbab5f353f2..94939e31e28 100644 --- a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/row-modal/row-modal.tsx +++ b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/row-modal/row-modal.tsx @@ -22,6 +22,7 @@ import { useParams } from 'next/navigation' import type { ColumnDefinition, TableInfo, TableRow } from '@/lib/table' import { columnTypeOf } from '@/lib/table/column-types' import { resolveCurrencyCode } from '@/lib/table/currency' +import { todayAtTtlOffset, ttlValueFromPicker, ttlValueToPickerParts } from '@/lib/table/ttl-values' import { getTimezoneEditBlockedMessage } from '@/app/workspace/[workspaceId]/tables/[tableId]/components/timezone-editing' import { type TimezoneState, useTimezoneState } from '@/hooks/queries/general-settings' import { useDeleteTableRow, useDeleteTableRows, useUpdateTableRow } from '@/hooks/queries/tables' @@ -332,7 +333,7 @@ function ColumnField({ column, value, timeZone, onChange }: ColumnFieldProps) { required={column.required} hint={hint} mono - value={formatValueForInput(value, column.type, timeZone)} + value={formatValueForInput(value, column.type)} onChange={onChange} placeholder='{"key": "value"}' rows={4} @@ -340,28 +341,37 @@ function ColumnField({ column, value, timeZone, onChange }: ColumnFieldProps) { ) } - if (definition.editor === 'date') { - const parts = dateValueToLocalParts(formatValueForInput(value, column.type, timeZone)) + if (definition.editor === 'date' || definition.editor === 'offset-date') { + const storedValue = formatValueForInput(value, column.type) + const offsetParts = + definition.editor === 'offset-date' ? ttlValueToPickerParts(storedValue) : null + const parts = offsetParts ?? dateValueToLocalParts(storedValue) + const pickerToday = offsetParts + ? todayAtTtlOffset(offsetParts.offset) + : todayLocalCalendarDate(timeZone) const valueFromParts = (day: string, time: string | null) => - column.type === 'ttl' && time ? `${day}T${time}` : localPartsToDateValue(day, time, timeZone) + offsetParts + ? ttlValueFromPicker(day, time, offsetParts.offset) + : localPartsToDateValue(day, time, timeZone) return (
onChange(valueFromParts(day, parts.time))} placeholder='Select date' className='flex-1' /> - onChange(valueFromParts(parts.day ?? todayLocalCalendarDate(timeZone), time)) - } + onChange={(time) => onChange(valueFromParts(parts.day ?? pickerToday, time))} placeholder='Add time' className='w-[110px]' /> + {offsetParts && ( + {offsetParts.offset} + )}
) @@ -387,7 +397,7 @@ function ColumnField({ column, value, timeZone, onChange }: ColumnFieldProps) { inputType={ definition.inputMode === 'decimal' && !definition.acceptsFormattedInput ? 'number' : 'text' } - value={formatValueForInput(value, column.type, timeZone)} + value={formatValueForInput(value, column.type)} onChange={onChange} placeholder={`Enter ${column.name}`} /> diff --git a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/cells/cell-content.tsx b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/cells/cell-content.tsx index bdb533d9773..53af9e2482c 100644 --- a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/cells/cell-content.tsx +++ b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/cells/cell-content.tsx @@ -14,7 +14,6 @@ interface CellContentProps { /** Current workspace id — lets string cells holding an in-workspace resource * URL render as a tagged-resource chip instead of a plain external link. */ workspaceId: string - timeZone: string timezoneStatus: TimezoneState['status'] isEditing: boolean initialCharacter?: string | null @@ -41,7 +40,6 @@ export function CellContent({ exec, column, workspaceId, - timeZone, timezoneStatus, isEditing, initialCharacter, @@ -57,7 +55,6 @@ export function CellContent({ waitingOnLabels, isEnrichmentOutput, currentWorkspaceId: workspaceId, - timeZone, timezoneStatus, }) diff --git a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/cells/cell-render.test.ts b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/cells/cell-render.test.ts index ab0789ff2d4..529edc3350e 100644 --- a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/cells/cell-render.test.ts +++ b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/cells/cell-render.test.ts @@ -23,43 +23,23 @@ function column(type: DisplayColumn['type']): DisplayColumn { } describe('resolveCellRender', () => { - it('renders TTL epoch seconds through the date presentation', () => { - expect( - resolveCellRender({ - value: 1_700_000_000, + it.each(['ready', 'loading', 'invalid', 'error'] as const)( + 'renders TTL as the exact UTC string when timezone status is %s', + (timezoneStatus) => { + const value = '2026-06-15T09:00:30Z' + const kind = resolveCellRender({ + value, exec: undefined, column: column('ttl'), waitingOnLabels: undefined, - timeZone: 'America/New_York', + timezoneStatus, }) - ).toEqual({ kind: 'date', text: '2023-11-14T17:13:20-05:00' }) - }) - - it('renders raw epoch seconds when the saved timezone is invalid', () => { - expect( - resolveCellRender({ - value: 1_700_000_000, - exec: undefined, - column: column('ttl'), - waitingOnLabels: undefined, - timeZone: 'America/Los_Angeles', - timezoneStatus: 'invalid', - }) - ).toEqual({ kind: 'date', text: '1700000000', raw: true }) - }) - - it('renders raw epoch seconds while timezone settings are loading', () => { - expect( - resolveCellRender({ - value: 1_700_000_000, - exec: undefined, - column: column('ttl'), - waitingOnLabels: undefined, - timeZone: 'America/Los_Angeles', - timezoneStatus: 'loading', - }) - ).toEqual({ kind: 'date', text: '1700000000', raw: true }) - }) + expect(kind).toEqual({ kind: 'text', text: value }) + expect(renderToStaticMarkup(createElement(CellRender, { kind, isEditing: false }))).toContain( + value + ) + } + ) it('renders the exact stored Date value when timezone settings are unavailable', () => { const stored = '2026-01-15T09:00:00-05:00' @@ -68,7 +48,6 @@ describe('resolveCellRender', () => { exec: undefined, column: column('date'), waitingOnLabels: undefined, - timeZone: 'America/Los_Angeles', timezoneStatus: 'error', }) expect(kind).toEqual({ kind: 'date', text: stored, raw: true }) diff --git a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/cells/cell-render.tsx b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/cells/cell-render.tsx index 22971e399d0..6e9045f2bd4 100644 --- a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/cells/cell-render.tsx +++ b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/cells/cell-render.tsx @@ -54,8 +54,6 @@ interface ResolveCellRenderInput { /** Current workspace id — a URL pointing to a resource in this workspace * renders as a tagged-resource chip rather than a plain external link. */ currentWorkspaceId?: string - /** Effective viewer timezone for instant-like column presentations. */ - timeZone?: string /** Invalid or unavailable preferences render time-based values without conversion. */ timezoneStatus?: TimezoneState['status'] } @@ -67,7 +65,6 @@ export function resolveCellRender({ waitingOnLabels, isEnrichmentOutput, currentWorkspaceId, - timeZone, timezoneStatus, }: ResolveCellRenderInput): CellRenderKind { const isNull = value === null || value === undefined @@ -149,7 +146,7 @@ export function resolveCellRender({ if (timezoneStatus !== undefined && timezoneStatus !== 'ready') { return { kind: 'date', text: stringifyValue(value), raw: true } } - return { kind: 'date', text: definition.formatForInput(value, column, { timezone: timeZone }) } + return { kind: 'date', text: definition.formatForInput(value, column) } } if (column.type === 'string') { const text = stringifyValue(value) diff --git a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/cells/inline-editors.test.ts b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/cells/inline-editors.test.ts index 0439e39fc75..d08d7cb5a9e 100644 --- a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/cells/inline-editors.test.ts +++ b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/cells/inline-editors.test.ts @@ -5,14 +5,23 @@ import { act, createElement, type ReactNode } from 'react' import { createRoot } from 'react-dom/client' import { beforeEach, describe, expect, it, vi } from 'vitest' import type { ColumnDefinition } from '@/lib/table' +import { TTL_FORMAT_ERROR } from '@/lib/table/ttl-values' import { dateEditorRawValue, InlineEditor, } from '@/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/cells/inline-editors' import { cleanCellValue } from '@/app/workspace/[workspaceId]/tables/[tableId]/utils' -const { mockToastError, mockUseTimezoneState } = vi.hoisted(() => ({ +const { mockToastError, mockUseTimezoneState, mockCalendar } = vi.hoisted(() => ({ mockToastError: vi.fn(), + mockCalendar: vi.fn( + (_props: { + onChange: (value: string) => void + value?: string + timeLabel?: string + today?: string + }) => null + ), mockUseTimezoneState: vi.fn(), })) @@ -20,7 +29,7 @@ vi.mock('@/hooks/queries/general-settings', () => ({ useTimezoneState: mockUseTi vi.mock('@sim/emcn', () => { const passthrough = ({ children }: { children?: ReactNode }) => children ?? null return { - Calendar: () => null, + Calendar: mockCalendar, cn: (...classes: unknown[]) => classes.filter(Boolean).join(' '), DropdownMenu: passthrough, DropdownMenuContent: passthrough, @@ -56,26 +65,109 @@ describe('dateEditorRawValue', () => { const repeatedRaw = dateEditorRawValue(repeatedWallClock, ttlColumn, timezone) expect(repeatedRaw).toBe(repeatedWallClock) - expect(cleanCellValue(repeatedRaw, ttlColumn, timezone)).toBe( - Date.parse('2026-11-01T06:30:00Z') / 1000 - ) + expect(cleanCellValue(repeatedRaw, ttlColumn, timezone)).toBeNull() const fractionalRaw = dateEditorRawValue('2023-11-14t22:13:20.001Z', ttlColumn, timezone) - expect(cleanCellValue(fractionalRaw, ttlColumn, timezone)).toBe(1_700_000_001) + expect(cleanCellValue(fractionalRaw, ttlColumn, timezone)).toBe('2023-11-14T22:13:20.001-00:00') }) + it.each([ + ['2026-11-01T01:30', '2026-11-01T01:30:00-00:00'], + ['2026-03-08T02:30:45', '2026-03-08T02:30:45-00:00'], + ['2026-09-07', '2026-09-07T00:00:00-00:00'], + ])('saves new picker selections with a zero offset %s', (picked, expected) => { + const container = document.createElement('div') + document.body.appendChild(container) + const root = createRoot(container) + const onSave = vi.fn() + act(() => + root.render( + createElement(InlineEditor, { + column: column('ttl'), + value: null, + onSave, + onCancel: vi.fn(), + }) + ) + ) + const picker = mockCalendar.mock.calls.at(-1)![0] + act(() => picker.onChange(picked)) + if (picked.includes('T')) { + const input = container.querySelector('input') as HTMLInputElement + expect(input.value).toBe(expected) + act(() => input.dispatchEvent(new KeyboardEvent('keydown', { key: 'Enter', bubbles: true }))) + } + expect(onSave).toHaveBeenCalledWith(expected, 'enter') + expect(mockUseTimezoneState).not.toHaveBeenCalled() + act(() => root.unmount()) + container.remove() + }) + + it.each(['-07:00', '-08:00', '+05:45', '-00:00', '+00:00'])( + 'retains %s when changing the date and time in the picker', + (offset) => { + const container = document.createElement('div') + document.body.appendChild(container) + const root = createRoot(container) + const onSave = vi.fn() + act(() => + root.render( + createElement(InlineEditor, { + column: column('ttl'), + value: `2026-09-07T07:30:00.123456${offset}`, + onSave, + onCancel: vi.fn(), + }) + ) + ) + const picker = mockCalendar.mock.calls.at(-1)![0] + expect(picker.timeLabel).toBe(`Time (${offset})`) + act(() => picker.onChange('2026-11-01T01:30:45')) + const input = container.querySelector('input') as HTMLInputElement + expect(input.value).toBe(`2026-11-01T01:30:45${offset}`) + act(() => input.dispatchEvent(new KeyboardEvent('keydown', { key: 'Enter', bubbles: true }))) + expect(onSave).toHaveBeenCalledWith(`2026-11-01T01:30:45${offset}`, 'enter') + act(() => root.unmount()) + container.remove() + } + ) + it('keeps ordinary date drafts on their existing display parser', () => { expect(dateEditorRawValue('11/01/2026 1:30:00 AM', column('date'), 'America/New_York')).toBe( '2026-11-01T01:30:00-04:00' ) }) - it('keeps an open TTL edit in its starting timezone when the setting changes', () => { + it('preserves a typed offset timestamp and its microseconds', () => { + const container = document.createElement('div') + document.body.appendChild(container) + const root = createRoot(container) + const onSave = vi.fn() + act(() => + root.render( + createElement(InlineEditor, { + column: column('ttl'), + value: null, + onSave, + onCancel: vi.fn(), + }) + ) + ) + const input = container.querySelector('input') as HTMLInputElement + act(() => changeInput(input, '2026-09-07T07:30:00.123456-07:00')) + act(() => input.dispatchEvent(new KeyboardEvent('keydown', { key: 'Enter', bubbles: true }))) + expect(onSave).toHaveBeenCalledWith('2026-09-07T07:30:00.123456-07:00', 'enter') + expect(mockToastError).not.toHaveBeenCalled() + act(() => root.unmount()) + container.remove() + }) + + it('keeps an open TTL edit in its supplied offset when the timezone setting changes', () => { const container = document.createElement('div') document.body.appendChild(container) const root = createRoot(container) const onSave = vi.fn() - const value = Date.parse('2026-06-15T13:00:30Z') / 1000 + const value = '2026-06-15T06:00:30-07:00' const props = { value, column: column('ttl'), @@ -92,18 +184,18 @@ describe('dateEditorRawValue', () => { act(() => root.render(createElement(InlineEditor, props))) const input = container.querySelector('input') as HTMLInputElement - expect(input?.value).toBe('06/15/2026 6:00:30 AM') - act(() => changeInput(input, '09/01/2026 9:00 AM')) + expect(input?.value).toBe('2026-06-15T06:00:30-07:00') + act(() => changeInput(input, '2026-09-01T09:00:00Z')) act(() => { input.dispatchEvent(new KeyboardEvent('keydown', { key: 'Enter', bubbles: true })) }) - expect(onSave).toHaveBeenCalledWith(Date.parse('2026-09-01T16:00:00Z') / 1000, 'enter') + expect(onSave).toHaveBeenCalledWith('2026-09-01T09:00:00-00:00', 'enter') act(() => root.unmount()) container.remove() }) - it('waits for the saved timezone before creating a TTL draft', () => { + it('converts a legacy Z value to a zero-offset draft while timezone settings are loading', () => { mockUseTimezoneState.mockReturnValue({ timezone: 'Asia/Tokyo', status: 'loading', @@ -113,7 +205,7 @@ describe('dateEditorRawValue', () => { const root = createRoot(container) const onSave = vi.fn() const props = { - value: Date.parse('2026-06-15T13:00:30Z') / 1000, + value: '2026-06-15T13:00:30Z', column: column('ttl'), onSave, onCancel: vi.fn(), @@ -121,8 +213,8 @@ describe('dateEditorRawValue', () => { act(() => root.render(createElement(InlineEditor, props))) - expect(container.querySelector('input')).toBeNull() - expect(container.querySelector('[role="status"]')?.textContent).toBe('Loading timezone…') + expect(container.querySelector('input')?.value).toBe('2026-06-15T13:00:30-00:00') + expect(container.querySelector('[role="status"]')).toBeNull() mockUseTimezoneState.mockReturnValue({ timezone: 'America/Los_Angeles', @@ -132,10 +224,10 @@ describe('dateEditorRawValue', () => { const input = container.querySelector('input') as HTMLInputElement expect(input.disabled).toBe(false) - act(() => changeInput(input, '09/01/2026 9:00 AM')) + act(() => changeInput(input, '2026-09-01T09:00:00Z')) act(() => input.dispatchEvent(new KeyboardEvent('keydown', { key: 'Enter', bubbles: true }))) - expect(onSave).toHaveBeenCalledWith(Date.parse('2026-09-01T16:00:00Z') / 1000, 'enter') + expect(onSave).toHaveBeenCalledWith('2026-09-01T09:00:00-00:00', 'enter') act(() => root.unmount()) container.remove() }) @@ -185,7 +277,7 @@ describe('dateEditorRawValue', () => { act(() => root.render( createElement(InlineEditor, { - value: Date.parse('2026-06-15T13:00:30Z') / 1000, + value: '2026-06-15T13:00:30Z', column: column('ttl'), onSave, onCancel: vi.fn(), @@ -198,19 +290,28 @@ describe('dateEditorRawValue', () => { act(() => input.dispatchEvent(new KeyboardEvent('keydown', { key: 'Enter', bubbles: true }))) expect(onSave).not.toHaveBeenCalled() - expect(mockToastError).toHaveBeenCalledWith('Invalid expiration date') + expect(mockToastError).toHaveBeenCalledWith(TTL_FORMAT_ERROR) act(() => root.unmount()) container.remove() }) it.each([ - { caseName: 'a historical sub-minute offset', timezone: 'Africa/Monrovia', value: 2670 }, + { + caseName: 'a historical sub-minute offset', + timezone: 'Africa/Monrovia', + value: '1970-01-01T00:44:30-00:00', + }, + { + caseName: 'microsecond precision', + timezone: 'America/Los_Angeles', + value: '2026-09-07T07:30:00.123456-07:00', + }, { caseName: 'the far-future representable boundary', timezone: 'Asia/Tokyo', - value: 253_402_300_799, + value: '9999-12-31T23:59:59+00:00', }, - ])('preserves the exact epoch for $caseName when untouched', ({ timezone, value }) => { + ])('preserves the exact offset string for $caseName when untouched', ({ timezone, value }) => { mockUseTimezoneState.mockReturnValue({ timezone, status: 'ready' }) const container = document.createElement('div') document.body.appendChild(container) @@ -236,7 +337,7 @@ describe('dateEditorRawValue', () => { container.remove() }) - it('cancels TTL editing when the saved timezone cannot be loaded', () => { + it('allows TTL editing when the saved timezone cannot be loaded', () => { mockUseTimezoneState.mockReturnValue({ timezone: 'America/Los_Angeles', status: 'error', @@ -249,7 +350,7 @@ describe('dateEditorRawValue', () => { act(() => root.render( createElement(InlineEditor, { - value: 2670, + value: '1970-01-01T00:44:30-00:00', column: column('ttl'), onSave: vi.fn(), onCancel, @@ -257,10 +358,9 @@ describe('dateEditorRawValue', () => { ) ) - expect(onCancel).toHaveBeenCalledOnce() - expect(mockToastError).toHaveBeenCalledWith( - 'We couldn’t load your timezone setting. Try again before editing Date or Expiration cells.' - ) + expect(container.querySelector('input')?.value).toBe('1970-01-01T00:44:30-00:00') + expect(onCancel).not.toHaveBeenCalled() + expect(mockToastError).not.toHaveBeenCalled() act(() => root.unmount()) container.remove() }) @@ -290,7 +390,7 @@ describe('dateEditorRawValue', () => { expect(container.querySelector('input')).toBeNull() expect(onCancel).toHaveBeenCalledOnce() expect(mockToastError).toHaveBeenCalledWith( - 'Your saved timezone “Mars/Olympus” is invalid. Update it in Settings → General before editing Date or Expiration cells.' + 'Your saved timezone “Mars/Olympus” is invalid. Update it in Settings → General before editing Date cells.' ) act(() => root.unmount()) container.remove() diff --git a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/cells/inline-editors.tsx b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/cells/inline-editors.tsx index fc5779089b8..d6e754beb4a 100644 --- a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/cells/inline-editors.tsx +++ b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/cells/inline-editors.tsx @@ -17,6 +17,7 @@ import { Check } from '@sim/emcn/icons' import type { ColumnDefinition } from '@/lib/table' import { columnTypeOf } from '@/lib/table/column-types' import { isCalendarDateString } from '@/lib/table/dates' +import { todayAtTtlOffset, ttlValueFromPicker, ttlValueToPickerParts } from '@/lib/table/ttl-values' import { getTimezoneEditBlockedMessage } from '@/app/workspace/[workspaceId]/tables/[tableId]/components/timezone-editing' import { useTimezoneState } from '@/hooks/queries/general-settings' import type { SaveReason } from '../../../types' @@ -120,11 +121,14 @@ function ReadyInlineDateEditor({ const editTimeZoneRef = useRef(initialTimeZone) const timeZone = editTimeZoneRef.current - const storedValue = formatValueForInput(value, column.type, timeZone) + const isOffsetDate = columnTypeOf(column).editor === 'offset-date' + const storedValue = formatValueForInput(value, column.type) const initialDraft = initialCharacter !== undefined ? initialCharacter - : storageToDisplay(storedValue, { seconds: true }) + : isOffsetDate + ? storedValue + : storageToDisplay(storedValue, { seconds: true }) const [draft, setDraft] = useState(initialDraft) const [invalid, setInvalid] = useState(false) /** Picker commits mutate the draft from timeouts/child handlers; reading it @@ -132,9 +136,9 @@ function ReadyInlineDateEditor({ const draftRef = useRef(draft) draftRef.current = draft - /** The calendar works on wall times; feed it the draft's literal wall - * representation. */ - const draftParts = dateValueToLocalParts(displayToStorage(draft, timeZone) ?? storedValue) + const offsetParts = isOffsetDate ? ttlValueToPickerParts(draft) : null + const draftParts = + offsetParts ?? dateValueToLocalParts(displayToStorage(draft, timeZone) ?? storedValue) const pickerValue = draftParts.day ? draftParts.time ? `${draftParts.day}T${draftParts.time}` @@ -160,19 +164,11 @@ function ReadyInlineDateEditor({ if (doneRef.current) return clearTimeout(blurTimeoutRef.current) const current = draftRef.current - // Untouched draft → re-save the stored value byte-identical. Re-parsing - // the display form would re-stamp the offset with THIS viewer's zone, - // silently shifting the instant of a value someone else wrote. + /** Preserve Date cells' stored offsets instead of reinterpreting their + * display text in the viewer's timezone. */ if (storageVal === undefined && initialCharacter === undefined && current === initialDraft) { doneRef.current = true - onSave( - column.type === 'ttl' - ? (value ?? null) - : storedValue - ? cleanCellValue(storedValue, column, timeZone) - : null, - reason - ) + onSave(storedValue ? cleanCellValue(storedValue, column, timeZone) : null, reason) return } const raw = dateEditorRawValue(current, column, timeZone, storageVal) @@ -193,17 +189,7 @@ function ReadyInlineDateEditor({ doneRef.current = true onSave(cleaned, reason) }, - [ - invalid, - onSave, - onCancel, - timeZone, - initialDraft, - initialCharacter, - storedValue, - column, - value, - ] + [invalid, onSave, onCancel, timeZone, initialDraft, initialCharacter, storedValue, column] ) const handleKeyDown = useCallback( @@ -249,21 +235,25 @@ function ReadyInlineDateEditor({ * immediately) or a local `YYYY-MM-DDTHH:mm[:ss]` wall time (update the * draft and keep editing). */ - const handlePickerChange = useCallback( - (picked: string) => { - clearTimeout(blurTimeoutRef.current) - if (isCalendarDateString(picked)) { - doSave('enter', picked) - return - } - const canonical = displayToStorage(picked, timeZone) - if (!canonical) return - setDraft(storageToDisplay(canonical, { seconds: true })) + const handlePickerChange = (picked: string) => { + clearTimeout(blurTimeoutRef.current) + if (isCalendarDateString(picked)) { + doSave('enter', offsetParts ? ttlValueFromPicker(picked, null, offsetParts.offset) : picked) + return + } + if (offsetParts) { + const [day, time] = picked.split('T') + setDraft(ttlValueFromPicker(day, time ?? null, offsetParts.offset)) setInvalid(false) inputRef.current?.focus() - }, - [doSave, timeZone] - ) + return + } + const canonical = displayToStorage(picked, timeZone) + if (!canonical) return + setDraft(storageToDisplay(canonical, { seconds: true })) + setInvalid(false) + inputRef.current?.focus() + } const handlePickerOpenChange = useCallback((open: boolean) => { if (!open && !doneRef.current) { @@ -284,7 +274,7 @@ function ReadyInlineDateEditor({ }} onKeyDown={handleKeyDown} onBlur={scheduleBlurSave} - placeholder='mm/dd/yyyy' + placeholder={isOffsetDate ? 'YYYY-MM-DDTHH:mm:ss±HH:mm' : 'mm/dd/yyyy'} className={cn( 'w-full min-w-0 select-text border-none bg-transparent p-0 text-[var(--text-primary)] text-small outline-hidden', invalid && 'text-[var(--text-error)]' @@ -304,7 +294,10 @@ function ReadyInlineDateEditor({ value={pickerValue} onChange={handlePickerChange} showTime - today={todayLocalCalendarDate(timeZone)} + timeLabel={offsetParts ? `Time (${offsetParts.offset})` : undefined} + today={ + offsetParts ? todayAtTtlOffset(offsetParts.offset) : todayLocalCalendarDate(timeZone) + } /> @@ -503,6 +496,8 @@ export function InlineEditor(props: InlineEditorProps) { switch (columnTypeOf(props.column).editor) { case 'date': return + case 'offset-date': + return case 'select': return // `toggle` types never open an editor — the grid flips them in place — so diff --git a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/data-row.tsx b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/data-row.tsx index 51b38ab318d..86963aa768b 100644 --- a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/data-row.tsx +++ b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/data-row.tsx @@ -27,9 +27,7 @@ export interface DataRowProps { /** Current workspace id — forwarded to cells so in-workspace resource URLs * render as tagged-resource chips. */ workspaceId: string - /** Effective viewer timezone used to render TTL instants. */ - timeZone: string - /** Whether Date and Expiration values can be formatted and edited safely. */ + /** Whether Date values can be formatted and edited safely. */ timezoneStatus: TimezoneState['status'] rowIndex: number isFirstRow: boolean @@ -119,7 +117,6 @@ function dataRowPropsAreEqual(prev: DataRowProps, next: DataRowProps): boolean { prev.row !== next.row || prev.columns !== next.columns || prev.workspaceId !== next.workspaceId || - prev.timeZone !== next.timeZone || prev.timezoneStatus !== next.timezoneStatus || prev.rowIndex !== next.rowIndex || prev.isFirstRow !== next.isFirstRow || @@ -168,7 +165,6 @@ export const DataRow = React.memo(function DataRow({ row, columns, workspaceId, - timeZone, timezoneStatus, rowIndex, isFirstRow, @@ -405,7 +401,6 @@ export const DataRow = React.memo(function DataRow({
- + ) } diff --git a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/utils.test.ts b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/utils.test.ts index c77ce7256e3..a24a71d8f77 100644 --- a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/utils.test.ts +++ b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/utils.test.ts @@ -195,30 +195,14 @@ describe('formatValueForInput', () => { expect(formatValueForInput('2026-07-06', 'date')).toBe('2026-07-06') }) - it('renders TTL instants in the editor timezone without changing the instant', () => { - expect(formatValueForInput(1_700_000_000, 'ttl', 'America/New_York')).toBe( - '2023-11-14T17:13:20-05:00' - ) - expect( - cleanCellValue('2023-11-14 17:13:20', { name: 'expires_at', type: 'ttl' }, 'America/New_York') - ).toBe(1_700_000_000) - expect( - cleanCellValue('2023-11-14', { name: 'expires_at', type: 'ttl' }, 'America/New_York') - ).toBe(1_699_938_000) - }) - - it('uses the latest effective timezone for each TTL edit', () => { + it('preserves TTL strings independently of the viewer timezone', () => { const column = { name: 'expires_at', type: 'ttl' } as const - const input = '2026-06-15 09:00:30' - - expect(cleanCellValue(input, column, 'America/New_York')).toBe( - Date.parse('2026-06-15T13:00:30Z') / 1000 - ) - expect(cleanCellValue(input, column, 'Asia/Kathmandu')).toBe( - Date.parse('2026-06-15T03:15:30Z') / 1000 - ) - expect(cleanCellValue(input, column, 'America/New_York')).toBe( - Date.parse('2026-06-15T13:00:30Z') / 1000 - ) + const input = '2026-06-15T09:00:30Z' + for (const timezone of ['UTC', 'America/New_York', 'Asia/Kathmandu', 'Mars/Olympus']) { + expect(formatValueForInput(input, 'ttl')).toBe('2026-06-15T09:00:30-00:00') + expect(cleanCellValue(input, column, timezone)).toBe('2026-06-15T09:00:30-00:00') + expect(cleanCellValue('2026-06-15 09:00:30', column, timezone)).toBeNull() + expect(cleanCellValue(1_700_000_000, column, timezone)).toBeNull() + } }) }) diff --git a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/utils.ts b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/utils.ts index c9892f8466e..30e1062d482 100644 --- a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/utils.ts +++ b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/utils.ts @@ -56,7 +56,7 @@ export function cleanCellValue( // Everything else runs the SAME coercion the server will run, so the // optimistic cache holds exactly the value that gets persisted. const columnType = columnTypeOf(column) - const coerced = columnType.coerce(value as JsonValue, column, { timezone: timeZone }) + const coerced = columnType.coerce(value as JsonValue, column) if (coerced.ok) return coerced.value const salvaged = columnType.salvage?.(value as JsonValue, column) return salvaged?.ok ? salvaged.value : null @@ -69,7 +69,7 @@ export function cleanCellValue( * row data already has the new mapping's value) would otherwise render * `[object Object]` via `String(value)`. */ -export function formatValueForInput(value: unknown, type: string, timeZone?: string): string { +export function formatValueForInput(value: unknown, type: string): string { if (value === null || value === undefined) return '' const definition = columnTypeById(type) // Shape-drift guard, kept ahead of the registry: a column whose declared type @@ -79,11 +79,7 @@ export function formatValueForInput(value: unknown, type: string, timeZone?: str if (typeof value === 'object' && !definition.storesOpaqueIds && type !== 'json') { return JSON.stringify(value) } - return definition.formatForInput( - value, - { name: '', type: type as ColumnType }, - { timezone: timeZone } - ) + return definition.formatForInput(value, { name: '', type: type as ColumnType }) } /** A canonical date-cell value split into its wall-clock editing parts. */ diff --git a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/retry-settings/retry-settings.test.tsx b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/retry-settings/retry-settings.test.tsx index eb4279b52cb..fa945575592 100644 --- a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/retry-settings/retry-settings.test.tsx +++ b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/retry-settings/retry-settings.test.tsx @@ -4,6 +4,37 @@ import { act } from 'react' import { createRoot, type Root } from 'react-dom/client' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +vi.mock( + '@/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/short-input', + () => ({ + ShortInput: ({ + config, + value, + onChange, + onBlur, + disabled, + allowReferences, + }: { + config: { id: string } + value: string + onChange: (value: string) => void + onBlur: () => void + disabled: boolean + allowReferences?: boolean + }) => ( + onChange(event.target.value)} + onBlur={onBlur} + disabled={disabled} + /> + ), + }) +) + import { RetrySettings } from './retry-settings' const policy = { enabled: true as const, maxTries: 5, waitBetweenTriesMs: 2000 } @@ -25,7 +56,15 @@ afterEach(() => { function renderSettings(props: Partial[0]> = {}) { const onChange = vi.fn() act(() => { - root.render() + root.render( + + ) }) return { onChange } } @@ -46,6 +85,33 @@ describe('RetrySettings', () => { expect(field('block-retry-max-tries')!.value).toBe('5') }) + it('commits the normalized value when the field loses focus', () => { + const { onChange } = renderSettings() + const maxTries = field('block-retry-max-tries')! + const setValue = Object.getOwnPropertyDescriptor(HTMLInputElement.prototype, 'value')!.set! + + act(() => { + setValue.call(maxTries, '2.7') + maxTries.dispatchEvent(new Event('input', { bubbles: true })) + }) + expect(field('block-retry-max-tries')!.value).toBe('2.7') + expect(onChange).not.toHaveBeenCalled() + + act(() => { + maxTries.dispatchEvent(new FocusEvent('focusout', { bubbles: true })) + }) + + expect(onChange).toHaveBeenCalledWith({ ...policy, maxTries: 2 }) + expect(field('block-retry-max-tries')!.value).toBe('5') + }) + + it('turns off the reference pickers on the numeric fields', () => { + renderSettings() + + expect(field('block-retry-max-tries')!.dataset.allowReferences).toBe('false') + expect(field('block-retry-wait')!.dataset.allowReferences).toBe('false') + }) + it('renders only the switch while retry is off', () => { renderSettings({ retry: { ...policy, enabled: false } }) diff --git a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/retry-settings/retry-settings.tsx b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/retry-settings/retry-settings.tsx index 87ac1f8271a..6f0cbefffa5 100644 --- a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/retry-settings/retry-settings.tsx +++ b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/retry-settings/retry-settings.tsx @@ -1,7 +1,7 @@ 'use client' import { useState } from 'react' -import { ChipInput, FieldDivider, Label, Switch } from '@sim/emcn' +import { FieldDivider, Label, Switch } from '@sim/emcn' import { BLOCK_RETRY_DEFAULT_TRIES, BLOCK_RETRY_DEFAULT_WAIT_MS, @@ -9,37 +9,55 @@ import { normalizeBlockRetryTries, normalizeBlockRetryWaitMs, } from '@sim/workflow-types/workflow' +import { ShortInput } from '@/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/short-input' +import type { SubBlockConfig } from '@/blocks/types' interface RetrySettingsProps { + blockId: string retry: BlockRetryConfig | undefined disabled: boolean onChange: (retry: BlockRetryConfig) => void } interface RetryNumberFieldProps { - id: string - title: string + blockId: string + config: SubBlockConfig value: number disabled: boolean normalize: (value: unknown) => number onCommit: (value: number) => void } +const MAX_TRIES_CONFIG = { + id: 'block-retry-max-tries', + title: 'Max tries', + type: 'short-input', + connectionDroppable: false, +} as const satisfies SubBlockConfig + +const WAIT_CONFIG = { + id: 'block-retry-wait', + title: 'Wait between tries (ms)', + type: 'short-input', + connectionDroppable: false, +} as const satisfies SubBlockConfig + /** * A bounded number field that commits on blur. * - * Typed as text with a numeric input mode rather than `type='number'`: the - * native spinner is all that buys, and it does not fit the field chrome the rest - * of the panel uses. Bounds are applied on commit through the same normalizer - * execution uses, so the field cannot clamp differently from the executor. + * Renders the same `ShortInput` every other sub-block text field uses, so it + * carries the panel's field chrome. Retry values are plain numbers that never + * resolve references, so the reference pickers are turned off. Bounds are + * applied on commit through the same normalizer execution uses, so the field + * cannot clamp differently from the executor. * * The draft exists only while the field is being edited; clearing it on commit * lets an external change — a collaborator's edit, or an undo — flow straight * through on the next render with no resync. */ function RetryNumberField({ - id, - title, + blockId, + config, value, disabled, normalize, @@ -57,15 +75,18 @@ function RetryNumberField({ return (
- - + +
+ setDraft(event.target.value)} + onChange={setDraft} onBlur={commit} disabled={disabled} + allowReferences={false} />
) @@ -79,7 +100,7 @@ function RetryNumberField({ * with `enabled: false` when it is switched off, so turning it back on restores * what was configured rather than snapping to the defaults. */ -export function RetrySettings({ retry, disabled, onChange }: RetrySettingsProps) { +export function RetrySettings({ blockId, retry, disabled, onChange }: RetrySettingsProps) { const enabled = retry?.enabled === true const maxTries = retry?.maxTries ?? BLOCK_RETRY_DEFAULT_TRIES const waitBetweenTriesMs = retry?.waitBetweenTriesMs ?? BLOCK_RETRY_DEFAULT_WAIT_MS @@ -106,8 +127,8 @@ export function RetrySettings({ retry, disabled, onChange }: RetrySettingsProps) <>
+ /** Whether the env-var and tag reference pickers may open. Defaults to `true`. */ + allowReferences?: boolean + /** Called when the input loses focus. */ + onBlur?: () => void } /** @@ -81,6 +85,8 @@ export const ShortInput = memo(function ShortInput({ wandControlRef, hideInternalWand = false, workflowSearchValuePath = [], + allowReferences = true, + onBlur, }: ShortInputProps) { const activeSearchTarget = useActiveSearchTarget() const [localContent, setLocalContent] = useState('') @@ -284,7 +290,8 @@ export const ShortInput = memo(function ShortInput({ const handleBlur = useCallback(() => { setIsFocused(false) - }, []) + onBlur?.() + }, [onBlur]) // Expose wand control handlers to parent via ref useImperativeHandle( @@ -325,6 +332,7 @@ export const ShortInput = memo(function ShortInput({ disabled={disabled} isStreaming={wandHook.isStreaming} previewValue={previewValue} + allowReferences={allowReferences} shouldForceEnvDropdown={shouldForceEnvDropdown} shouldForceTagDropdown={shouldForceTagDropdown} > diff --git a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/sub-block-input-controller/sub-block-input-controller.tsx b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/sub-block-input-controller/sub-block-input-controller.tsx index 18627eeb96d..995a6901787 100644 --- a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/sub-block-input-controller/sub-block-input-controller.tsx +++ b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/sub-block-input-controller/sub-block-input-controller.tsx @@ -33,6 +33,8 @@ export interface SubBlockInputControllerProps { onStreamingEnd?: () => void /** Optional preview value for read-only preview. */ previewValue?: string | null + /** Whether the env-var and tag reference pickers may open. Defaults to `true`. */ + allowReferences?: boolean /** * Optional callback to force/show the env var dropdown (e.g., API key fields). * Return { show: true, searchTerm?: string } to override defaults. @@ -82,6 +84,7 @@ export function SubBlockInputController(props: SubBlockInputControllerProps): Re isStreaming, onStreamingEnd, previewValue, + allowReferences, shouldForceEnvDropdown, shouldForceTagDropdown, children, @@ -98,6 +101,7 @@ export function SubBlockInputController(props: SubBlockInputControllerProps): Re isStreaming, onStreamingEnd, previewValue, + allowReferences, shouldForceEnvDropdown, shouldForceTagDropdown, }) diff --git a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/tool-input/components/tools/usage-control.tsx b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/tool-input/components/tools/usage-control.tsx new file mode 100644 index 00000000000..e79ab62abcd --- /dev/null +++ b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/tool-input/components/tools/usage-control.tsx @@ -0,0 +1,128 @@ +import { Combobox, cn, Label, Tooltip } from '@sim/emcn' +import { ArrowLeftRight } from '@sim/emcn/icons' +import type { CanonicalMode } from '@/lib/workflows/subblocks/visibility' +import type { StoredTool } from '@/lib/workflows/tool-input/types' +import { ShortInput } from '@/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/short-input' +import type { SubBlockConfig } from '@/blocks/types' + +type UsageControlValue = NonNullable + +interface ToolUsageControlProps { + blockId: string + aggregateSubBlockId: string + toolIndex: number + tool: StoredTool + mode: CanonicalMode + supportsForce: boolean + disabled: boolean + onFixedChange: (value: UsageControlValue) => void + onExpressionChange: (value: string) => void + onModeToggle: () => void +} + +const MODE_OPTIONS = [ + { value: 'auto', label: 'Auto', hint: '(model decides)' }, + { value: 'force', label: 'Force', hint: '(always use)' }, + { value: 'none', label: 'None', hint: '(disable tool)' }, +] as const satisfies ReadonlyArray<{ value: UsageControlValue; label: string; hint: string }> + +const EXPRESSION_CONFIG = { + id: 'usageControlExpression', + title: 'Permission Mode', + type: 'short-input', +} as const satisfies SubBlockConfig + +function isUsageControlValue(value: string): value is UsageControlValue { + return MODE_OPTIONS.some((option) => option.value === value) +} + +/** + * Permission Mode control for one agent tool. Selector mode picks a fixed `usageControl`, and + * Variable mode edits a `usageControlExpression` that must resolve to auto, force, or none. + * Both values are kept so toggling modes does not discard the inactive one. + * + * Renders the same label row, `Combobox`, and `ShortInput` as every other sub-block field, so + * the control matches the tool params beneath it. + */ +export function ToolUsageControl({ + blockId, + aggregateSubBlockId, + toolIndex, + tool, + mode, + supportsForce, + disabled, + onFixedChange, + onExpressionChange, + onModeToggle, +}: ToolUsageControlProps) { + const toggleLabel = mode === 'advanced' ? 'Switch to selector' : 'Switch to variable' + + return ( +
+
+ +
+ + + + + +

{toggleLabel}

+ + +
+
+ {mode === 'advanced' ? ( + + ) : ( + { + const unsupported = option.value === 'force' && !supportsForce + return { + value: option.value, + label: option.label, + disabled: unsupported, + suffixElement: ( + + {unsupported ? '(not supported by model)' : option.hint} + + ), + } + })} + value={tool.usageControl ?? 'auto'} + onChange={(value) => { + if (isUsageControlValue(value)) onFixedChange(value) + }} + editable={false} + disabled={disabled} + aria-label='Permission Mode' + /> + )} +
+ ) +} diff --git a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/tool-input/tool-input.tsx b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/tool-input/tool-input.tsx index 40f7c25a165..54b7d751123 100644 --- a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/tool-input/tool-input.tsx +++ b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/tool-input/tool-input.tsx @@ -2,17 +2,19 @@ import type React from 'react' import { memo, useCallback, useEffect, useMemo, useRef, useState } from 'react' import { Badge, + Button, Combobox, type ComboboxOption, type ComboboxOptionGroup, cn, + FieldDivider, Popover, PopoverContent, PopoverItem, PopoverTrigger, Tooltip, } from '@sim/emcn' -import { ArrowLeft, ChevronRight, Server, Wrench, X } from '@sim/emcn/icons' +import { ArrowLeft, ChevronRight, Pencil, Server, Wrench, X } from '@sim/emcn/icons' import { createLogger } from '@sim/logger' import { useParams } from 'next/navigation' import { McpIcon, WorkflowIcon } from '@/components/icons' @@ -32,6 +34,11 @@ import { } from '@/lib/permission-groups/operation-access' import { resolveStoredToolName } from '@/lib/workflows/subblocks/display' import { buildToolSubBlockId } from '@/lib/workflows/tool-input/synthetic-subblocks' +import { + buildAgentToolUsageControlCanonicalKey, + getAgentToolUsageControlMode, + resolveAgentToolUsageControl, +} from '@/lib/workflows/tool-input/usage-control' import { useUserPermissionsContext } from '@/app/workspace/[workspaceId]/providers/workspace-permissions-provider' import { McpServerFormModal } from '@/app/workspace/[workspaceId]/settings/components/mcp/components/mcp-server-form-modal/mcp-server-form-modal' import { formatDisplayText } from '@/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/formatted-text' @@ -40,6 +47,7 @@ import { CustomToolModal, } from '@/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/tool-input/components/custom-tool-modal/custom-tool-modal' import { ToolSubBlockRenderer } from '@/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/tool-input/components/tools/sub-block-renderer' +import { ToolUsageControl } from '@/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/tool-input/components/tools/usage-control' import { clearDependentToolParams } from '@/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/tool-input/param-dependents' import type { StoredTool } from '@/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/tool-input/types' import { @@ -382,7 +390,6 @@ export const ToolInput = memo(function ToolInput({ const [editingToolIndex, setEditingToolIndex] = useState(null) const [draggedIndex, setDraggedIndex] = useState(null) const [dragOverIndex, setDragOverIndex] = useState(null) - const [usageControlPopoverIndex, setUsageControlPopoverIndex] = useState(null) const [mcpRemovePopoverIndex, setMcpRemovePopoverIndex] = useState(null) const [mcpServerDrilldown, setMcpServerDrilldown] = useState(null) @@ -859,6 +866,7 @@ export const ToolInput = memo(function ToolInput({ type: 'custom-tool', customToolId: customTool.id, usageControl: existingTool.usageControl || 'auto', + usageControlExpression: existingTool.usageControlExpression, isExpanded: existingTool.isExpanded, } : { @@ -1009,23 +1017,36 @@ export const ToolInput = memo(function ToolInput({ [isPreview, disabled, selectedTools, getToolIdForOperation, blockId, setStoreValue] ) - const handleUsageControlChange = useCallback( - (toolIndex: number, usageControl: string) => { - if (isPreview || disabled) return + const handleUsageControlChange = ( + toolIndex: number, + usageControl: NonNullable + ) => { + if (isPreview || disabled) return - setStoreValue( - selectedTools.map((tool, index) => - index === toolIndex - ? { - ...tool, - usageControl: usageControl as 'auto' | 'force' | 'none', - } - : tool - ) + setStoreValue( + selectedTools.map((tool, index) => + index === toolIndex + ? { + ...tool, + usageControl, + } + : tool ) - }, - [isPreview, disabled, selectedTools, setStoreValue] - ) + ) + } + + const handleUsageControlExpressionChange = ( + toolIndex: number, + usageControlExpression: string + ) => { + if (isPreview || disabled) return + + setStoreValue( + selectedTools.map((tool, index) => + index === toolIndex ? { ...tool, usageControlExpression } : tool + ) + ) + } const [localExpanded, setLocalExpanded] = useState>({}) @@ -1514,6 +1535,13 @@ export const ToolInput = memo(function ToolInput({ toolIndex, tool.type ) + const toolUsageControlMode = getAgentToolUsageControlMode( + toolIndex, + canonicalModeOverrides + ) + const isToolDisabled = + supportsToolControl && + resolveAgentToolUsageControl(tool, toolIndex, canonicalModeOverrides) === 'none' const subBlocksResult: SubBlocksForToolInput | null = !isCustomTool && !isMcpFamily && currentToolId @@ -1574,12 +1602,14 @@ export const ToolInput = memo(function ToolInput({ const hasOperations = !isCustomTool && !isMcpFamily && hasMultipleOperations(toolBlock ?? undefined) - const hasToolBody = hasOperations || displaySubBlocks.length > 0 + const showToolControl = supportsToolControl && !(isMcpTool && isMcpToolUnavailable(tool)) + const hasToolBody = showToolControl || hasOperations || displaySubBlocks.length > 0 const isSearchExpanded = activeSearchTarget?.subBlockId === subBlockId && activeSearchTarget.valuePath[0] === toolIndex && - activeSearchTarget.valuePath[1] === 'params' + (activeSearchTarget.valuePath[1] === 'params' || + activeSearchTarget.valuePath[1] === 'usageControlExpression') const isExpandedForDisplay = hasToolBody ? isPreview || disabled ? isSearchExpanded || (localExpanded[toolIndex] ?? !!tool.isExpanded) @@ -1606,23 +1636,24 @@ export const ToolInput = memo(function ToolInput({
{ - if (isCustomTool) { - handleEditCustomTool(toolIndex) - } else if (hasToolBody) { + if (hasToolBody) { toggleToolExpansion(toolIndex) + } else if (isCustomTool) { + handleEditCustomTool(toolIndex) } }} onKeyDown={(e) => { if (e.key === 'Enter' || e.key === ' ') { - if (isCustomTool) { - handleEditCustomTool(toolIndex) - } else if (hasToolBody) { + if (hasToolBody) { toggleToolExpansion(toolIndex) + } else if (isCustomTool) { + handleEditCustomTool(toolIndex) } } }} @@ -1699,65 +1730,19 @@ export const ToolInput = memo(function ToolInput({ )}
- {supportsToolControl && !(isMcpTool && isMcpToolUnavailable(tool)) && ( - setUsageControlPopoverIndex(open ? toolIndex : null)} - colorScheme='inverted' + {isCustomTool && hasToolBody && ( + - - e.stopPropagation()} - className='gap-0.5' - border - > - { - handleUsageControlChange(toolIndex, 'auto') - setUsageControlPopoverIndex(null) - }} - > - Auto (model decides) - - { - handleUsageControlChange(toolIndex, 'force') - setUsageControlPopoverIndex(null) - }} - > - Force{' '} - - {supportsForce ? '(always use)' : '(not supported by model)'} - - - { - handleUsageControlChange(toolIndex, 'none') - setUsageControlPopoverIndex(null) - }} - > - None - - - + + )} {isMcpTool && selectedTools.filter( @@ -1827,8 +1812,39 @@ export const ToolInput = memo(function ToolInput({
- {!isCustomTool && isExpandedForDisplay && ( + {isExpandedForDisplay && (
+ {showToolControl && ( + <> + + handleUsageControlChange(toolIndex, usageControl) + } + onExpressionChange={(usageControlExpression) => + handleUsageControlExpressionChange(toolIndex, usageControlExpression) + } + onModeToggle={() => { + const nextMode = + toolUsageControlMode === 'advanced' ? 'basic' : 'advanced' + collaborativeSetBlockCanonicalMode( + blockId, + buildAgentToolUsageControlCanonicalKey(toolIndex), + nextMode + ) + }} + /> + {(hasOperations || displaySubBlocks.length > 0) && ( + + )} + + )} {isAdvancedMcpServer && ( { + if (displaySubBlocks.length === 0) return null + const renderSubBlock = (sb: BlockSubBlockConfig): React.ReactNode => { const effectiveParamId = sb.id const canonicalId = toolCanonicalIndex?.canonicalIdBySubBlockId[sb.id] diff --git a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/hooks/use-sub-block-input.ts b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/hooks/use-sub-block-input.ts index b91ba626b44..8723ae4922c 100644 --- a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/hooks/use-sub-block-input.ts +++ b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/hooks/use-sub-block-input.ts @@ -38,6 +38,11 @@ export interface UseSubBlockInputOptions { onStreamingEnd?: () => void /** Optional preview value for read-only preview displays. */ previewValue?: string | null + /** + * Whether the env-var and tag reference pickers may open. Defaults to `true`; pass `false` for + * fields whose value can never hold a reference. + */ + allowReferences?: boolean /** * Optional callback to force/show the env var dropdown (e.g., API key fields). * Return { show: true, searchTerm?: string } to override defaults. @@ -160,6 +165,7 @@ export function useSubBlockInput(options: UseSubBlockInputOptions): UseSubBlockI isStreaming = false, onStreamingEnd, previewValue, + allowReferences = true, shouldForceEnvDropdown, shouldForceTagDropdown, } = options @@ -558,8 +564,8 @@ export function useSubBlockInput(options: UseSubBlockInputOptions): UseSubBlockI valueString, isDisabled, cursorPosition, - showEnvVars, - showTags, + showEnvVars: allowReferences && showEnvVars, + showTags: allowReferences && showTags, searchTerm, activeSourceBlockId, handlers: { diff --git a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/editor.tsx b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/editor.tsx index dc345c60fa2..f24e926e2cd 100644 --- a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/editor.tsx +++ b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/editor.tsx @@ -801,6 +801,7 @@ export function Editor() { {showRetrySettings && ( ({ + enabled: vi.fn(), + signalChanged: vi.fn(), + fireTrigger: vi.fn(), +})) +vi.mock('@/lib/table/ttl-availability', () => ({ isTableRowTtlEnabled: enabled })) +vi.mock('@/lib/table/events', () => ({ signalTableRowsChanged: signalChanged })) +vi.mock('@/lib/table/trigger', () => ({ fireTableTrigger: fireTrigger })) + +import { db } from '@sim/db' +import { validatedTimestampSql } from '@/lib/table/column-types/timestamp-sql' +import { updateColumnConstraints } from '@/lib/table/columns/service' +import { getDeleteSnapshotBatchSize } from '@/lib/table/constants' +import { replaceTableRowsWithTx } from '@/lib/table/rows/service' +import { getTableById } from '@/lib/table/service' +import { fieldPredicate } from '@/lib/table/sql' +import { normalizeTtlTimestamp, TTL_TIMESTAMP_VALIDATION } from '@/lib/table/ttl-values' +import type { TableSchema } from '@/lib/table/types' +import { checkBatchUniqueConstraintsDb, coerceRowToSchema } from '@/lib/table/validation' +import { runCleanupTableRowTtl } from '@/background/cleanup-table-row-ttl' + +const url = process.env.TABLE_TTL_TEST_DATABASE_URL +if (url) { + const parsed = new URL(url) + const otherDatabase = Object.entries(process.env).some( + ([key, value]) => /^DATABASE_(URL|REPLICA_URL)(_|$)/.test(key) && value && value !== url + ) + if ( + !['127.0.0.1', 'localhost'].includes(parsed.hostname) || + parsed.pathname !== '/expiration_qa' || + process.env.DATABASE_URL !== url || + otherDatabase + ) { + throw new Error('This suite requires only the disposable local expiration_qa database') + } +} +const control = postgres(url ?? 'postgres://localhost/disabled_expiration_test', { + max: 4, + onnotice: () => {}, +}) +const workspaceId = generateId() +const userId = generateId() +const expired = '2020-01-01T00:00:00Z' +const future = '9998-01-01T00:00:00Z' +const schema = { columns: [{ id: 'expires', name: 'expires_at', type: 'ttl' }] } +const measurements: Record = {} + +async function createTable(columns = schema.columns): Promise { + const id = generateId() + await control`INSERT INTO user_table_definitions (id, workspace_id, name, schema, created_by, max_rows) + VALUES (${id}, ${workspaceId}, ${id}, ${control.json({ columns })}, ${userId}, 2000000)` + return id +} + +async function seedRows(tableId: string, count: number, value: string | null = expired) { + await control`INSERT INTO user_table_rows (id, table_id, workspace_id, data, position, created_at) + SELECT ${tableId} || '-' || lpad(n::text, 9, '0'), ${tableId}, ${workspaceId}, + jsonb_build_object('expires', ${value}::text), n, '2020-01-01'::timestamp + FROM generate_series(1, ${count}) AS n` +} + +async function rowCount(tableId: string): Promise { + const [result] = + await control`SELECT count(*)::int AS count FROM user_table_rows WHERE table_id = ${tableId}` + const [definition] = + await control`SELECT row_count FROM user_table_definitions WHERE id = ${tableId}` + expect(definition.row_count).toBe(result.count) + return result.count +} + +async function installDeleteFault(tableId: string, body: string) { + await control.unsafe(`CREATE OR REPLACE FUNCTION expiration_qa_delete_fault() RETURNS trigger + LANGUAGE plpgsql AS $function$ BEGIN + IF OLD.table_id = TG_ARGV[0] THEN ${body} END IF; + RETURN OLD; + END $function$`) + await control.unsafe(`CREATE TRIGGER expiration_qa_delete_fault BEFORE DELETE ON user_table_rows + FOR EACH ROW EXECUTE FUNCTION expiration_qa_delete_fault('${tableId}')`) +} + +async function removeDeleteFault() { + await control`DROP TRIGGER IF EXISTS expiration_qa_delete_fault ON user_table_rows` + await control`DROP FUNCTION IF EXISTS expiration_qa_delete_fault()` +} + +async function waitForSleepingDelete(): Promise { + for (let attempt = 0; attempt < 400; attempt++) { + const rows = await control`SELECT pid FROM pg_stat_activity + WHERE datname = current_database() AND wait_event = 'PgSleep' + AND query LIKE '%WITH locked_rows%'` + if (rows[0]) return Number(rows[0].pid) + await sleep(5) + } + throw new Error('Cleanup never reached the injected in-transaction pause') +} + +describe.skipIf(!url)('Expiration with real PostgreSQL transactions', () => { + beforeAll(async () => { + await control`INSERT INTO "user" (id, name, email, email_verified, created_at, updated_at) + VALUES (${userId}, 'Expiration integration fixture', ${`${userId}@example.test`}, true, now(), now())` + await control`INSERT INTO workspace (id, name, owner_id, billed_account_user_id) + VALUES (${workspaceId}, 'Expiration integration fixtures', ${userId}, ${userId})` + }) + + beforeEach(async () => { + vi.clearAllMocks() + enabled.mockResolvedValue(true) + fireTrigger.mockResolvedValue(undefined) + await control`DELETE FROM user_table_definitions WHERE workspace_id = ${workspaceId}` + }) + + afterEach(async () => { + await removeDeleteFault() + vi.restoreAllMocks() + }) + + afterAll(async () => { + await control`DELETE FROM workspace WHERE id = ${workspaceId}` + await control`DELETE FROM "user" WHERE id = ${userId}` + writeFileSync( + join(tmpdir(), 'expiration-qa-measurements.json'), + JSON.stringify(measurements, null, 2) + ) + await control.end() + }) + + it('does nothing with no TTL, empty tables, missing/null/invalid cells, or only future deadlines', async () => { + const plain = await createTable([{ id: 'expires', name: 'expires_at', type: 'date' }]) + await seedRows(plain, 1) + await createTable() + const table = await createTable() + await seedRows(table, 1, future) + for (const [index, value] of [ + null, + '', + 'not-a-date', + '2026-02-30T00:00:00Z', + 0, + {}, + [], + ].entries()) { + await control`INSERT INTO user_table_rows (id, table_id, workspace_id, data) + VALUES (${generateId()}, ${table}, ${workspaceId}, ${control.json(index === 0 ? {} : { expires: value })})` + } + const result = await runCleanupTableRowTtl() + expect(result.deleted).toBe(0) + expect(await rowCount(plain)).toBe(1) + expect(await rowCount(table)).toBe(8) + expect(fireTrigger).not.toHaveBeenCalled() + }) + + it('deletes exactly through the cutoff and preserves a future microsecond across offsets', async () => { + vi.spyOn(Date, 'now').mockReturnValue(Date.parse('2026-09-07T12:00:00.500Z')) + const table = await createTable() + const values = [ + '2026-09-07T12:00:00.499999Z', + '2026-09-07T12:00:00.500000Z', + '2026-09-07T05:00:00.500000-07:00', + '2026-09-07T17:45:00.500000+05:45', + '2026-09-07T12:00:00.500001Z', + '2026-09-07T05:00:00.500001-07:00', + ] + for (const value of values) { + await control`INSERT INTO user_table_rows (id, table_id, workspace_id, data) + VALUES (${generateId()}, ${table}, ${workspaceId}, ${control.json({ expires: value })})` + } + expect((await runCleanupTableRowTtl()).deleted).toBe(4) + expect(await rowCount(table)).toBe(2) + expect(fireTrigger.mock.calls[0][4]).toHaveLength(4) + }) + + it('respects feature disablement, delete locks, and archival, then catches up when restored', async () => { + const table = await createTable() + await seedRows(table, 1) + enabled.mockResolvedValue(false) + expect((await runCleanupTableRowTtl()).deleted).toBe(0) + enabled.mockResolvedValue(true) + await control`UPDATE user_table_definitions SET delete_locked = true WHERE id = ${table}` + expect((await runCleanupTableRowTtl()).deleted).toBe(0) + await control`UPDATE user_table_definitions SET delete_locked = false, archived_at = now() WHERE id = ${table}` + expect((await runCleanupTableRowTtl()).deleted).toBe(0) + expect(await rowCount(table)).toBe(1) + await control`UPDATE user_table_definitions SET archived_at = null WHERE id = ${table}` + expect((await runCleanupTableRowTtl()).deleted).toBe(1) + }) + + it('hits the real 100-batch limit and deletes the exact remaining row on the next pass', async () => { + const table = await createTable() + const capacity = 100 * getDeleteSnapshotBatchSize() + await seedRows(table, capacity + 1) + expect(await runCleanupTableRowTtl()).toEqual({ + batches: 100, + deleted: capacity, + limitReached: true, + }) + expect(await rowCount(table)).toBe(1) + expect((await runCleanupTableRowTtl()).deleted).toBe(1) + expect(await rowCount(table)).toBe(0) + measurements.singleRunCapacity = capacity + }, 30000) + + it('services more than 100 tables across passes without losing the unselected table', async () => { + const tables: string[] = [] + for (let index = 0; index < 101; index++) { + const table = await createTable() + tables.push(table) + await seedRows(table, 1) + } + const first = await runCleanupTableRowTtl() + expect(first).toEqual({ batches: 100, deleted: 100, limitReached: true }) + const remaining = + await control`SELECT count(*)::int AS count FROM user_table_rows WHERE workspace_id = ${workspaceId}` + expect(remaining[0].count).toBe(1) + expect((await runCleanupTableRowTtl()).deleted).toBe(1) + expect(await rowCount(tables[0])).toBe(0) + measurements.tableLimit = { tables: 101, firstPassDeleted: first.deleted, secondPassDeleted: 1 } + }, 30000) + + it('revisits a skipped locked row on the next pass', async () => { + const table = await createTable() + await seedRows(table, 2) + const locked = Promise.withResolvers() + const release = Promise.withResolvers() + const holding = control.begin(async (trx) => { + await trx`SELECT id FROM user_table_rows WHERE table_id = ${table} ORDER BY id LIMIT 1 FOR UPDATE` + locked.resolve() + await release.promise + }) + await locked.promise + try { + expect((await runCleanupTableRowTtl()).deleted).toBe(1) + } finally { + release.resolve() + await holding + } + expect((await runCleanupTableRowTtl()).deleted).toBe(1) + expect(await rowCount(table)).toBe(0) + }) + + it('drains 1001 expiring tables across bounded passes', async () => { + for (let index = 0; index < 1001; index++) { + await seedRows(await createTable(), 1) + } + let deleted = 0 + let passes = 0 + while (deleted < 1001) { + const result = await runCleanupTableRowTtl() + expect(result.batches).toBeLessThanOrEqual(100) + expect(result.deleted).toBeGreaterThan(0) + deleted += result.deleted + expect(++passes).toBeLessThanOrEqual(11) + } + expect(deleted).toBe(1001) + measurements.manyTables = { tables: 1001, passes } + }, 30000) + + it('gives small tables a turn before revisiting a large backlog', async () => { + const large = await createTable() + const batch = getDeleteSnapshotBatchSize() + await seedRows(large, batch * 100) + const small: string[] = [] + for (let index = 0; index < 20; index++) { + const table = await createTable() + small.push(table) + await seedRows(table, 1) + } + await runCleanupTableRowTtl() + for (const table of small) expect(await rowCount(table)).toBe(0) + const order = fireTrigger.mock.calls.map((call) => call[0]) + const firstLarge = order.indexOf(large) + const secondLarge = order.indexOf(large, firstLarge + 1) + for (const table of small) expect(order.indexOf(table)).toBeLessThan(secondLarge) + expect(await rowCount(large)).toBeGreaterThan(0) + }, 30000) + + it.each(['delete lock', 'archive', 'remove column'])( + 'rechecks a mid-run %s before the next batch', + async (change) => { + const table = await createTable() + const batch = getDeleteSnapshotBatchSize() + await seedRows(table, batch + 1) + fireTrigger.mockImplementationOnce(async () => { + if (change === 'delete lock') + await control`UPDATE user_table_definitions SET delete_locked = true WHERE id = ${table}` + if (change === 'archive') + await control`UPDATE user_table_definitions SET archived_at = now() WHERE id = ${table}` + if (change === 'remove column') + await control`UPDATE user_table_definitions SET schema = '{"columns":[]}'::jsonb WHERE id = ${table}` + }) + expect((await runCleanupTableRowTtl()).deleted).toBe(batch) + expect(await rowCount(table)).toBe(1) + await control`UPDATE user_table_definitions SET delete_locked = false, archived_at = null, schema = ${control.json(schema)} WHERE id = ${table}` + expect((await runCleanupTableRowTtl()).deleted).toBe(1) + } + ) + + it('uses one cutoff per run and finds newly expired rows behind its cursor next time', async () => { + const now = vi.spyOn(Date, 'now').mockReturnValue(Date.parse('2026-09-07T12:00:00Z')) + const table = await createTable() + await seedRows(table, getDeleteSnapshotBatchSize()) + const lateId = generateId() + await control`INSERT INTO user_table_rows (id, table_id, workspace_id, data, created_at) + VALUES (${lateId}, ${table}, ${workspaceId}, ${control.json({ expires: '2026-09-07T12:00:01Z' })}, '2021-01-01')` + fireTrigger.mockImplementationOnce(async () => { + now.mockReturnValue(Date.parse('2026-09-07T12:00:02Z')) + await control`INSERT INTO user_table_rows (id, table_id, workspace_id, data, created_at) + VALUES (${generateId()}, ${table}, ${workspaceId}, ${control.json({ expires: expired })}, '2010-01-01')` + }) + expect((await runCleanupTableRowTtl()).deleted).toBe(getDeleteSnapshotBatchSize()) + expect(await rowCount(table)).toBe(2) + expect((await runCleanupTableRowTtl()).deleted).toBe(2) + }) + + it('handles two concurrent cleanup runs without duplicate deletes or snapshots', async () => { + const table = await createTable() + const count = getDeleteSnapshotBatchSize() * 4 + 1 + await seedRows(table, count) + const results = await Promise.all([runCleanupTableRowTtl(), runCleanupTableRowTtl()]) + expect(results.reduce((sum, result) => sum + result.deleted, 0)).toBe(count) + expect(await rowCount(table)).toBe(0) + const ids = fireTrigger.mock.calls.flatMap((call) => + call[4].map((row: { id: string }) => row.id) + ) + expect(ids).toHaveLength(count) + expect(new Set(ids).size).toBe(count) + }) + + it.each([future, null])( + 'preserves a locked row whose expiration changes to %s', + async (value) => { + const table = await createTable() + await seedRows(table, 1) + const locked = Promise.withResolvers() + const release = Promise.withResolvers() + const holding = control.begin(async (trx) => { + await trx`SELECT id FROM user_table_rows WHERE table_id = ${table} FOR UPDATE` + locked.resolve() + await release.promise + await trx`UPDATE user_table_rows SET data = ${trx.json({ expires: value })} WHERE table_id = ${table}` + }) + await locked.promise + try { + expect((await runCleanupTableRowTtl()).deleted).toBe(0) + } finally { + release.resolve() + await holding + } + expect((await runCleanupTableRowTtl()).deleted).toBe(0) + expect(await rowCount(table)).toBe(1) + } + ) + + it('rolls back a failed batch, keeps prior commits, and drains the remainder after repair', async () => { + const table = await createTable() + const batch = getDeleteSnapshotBatchSize() + await seedRows(table, batch * 2) + await installDeleteFault( + table, + `IF OLD.position > ${batch} THEN RAISE EXCEPTION 'injected expiration failure'; END IF;` + ) + await expect(runCleanupTableRowTtl()).resolves.toEqual({ + batches: 2, + deleted: batch, + limitReached: false, + }) + expect(await rowCount(table)).toBe(batch) + expect(signalChanged).toHaveBeenCalledWith(table) + await removeDeleteFault() + expect((await runCleanupTableRowTtl()).deleted).toBe(batch) + expect(await rowCount(table)).toBe(0) + }) + + it('skips a persistently broken first table, drains healthy tables, and retries after repair', async () => { + const cutoff = '2026-09-07T12:00:00.000Z' + vi.spyOn(Date, 'now').mockReturnValue(Date.parse(cutoff)) + await createTable() + await createTable() + const tables = await control<{ id: string }[]>`SELECT id FROM user_table_definitions + WHERE workspace_id = ${workspaceId} ORDER BY md5(id || ${cutoff}), id` + const [broken, healthy] = tables.map(({ id }) => id) + const healthyRows = getDeleteSnapshotBatchSize() + 1 + await seedRows(broken, 3) + await seedRows(healthy, healthyRows) + await installDeleteFault(broken, "RAISE EXCEPTION 'injected table failure';") + + await expect(runCleanupTableRowTtl()).resolves.toEqual({ + batches: 4, + deleted: healthyRows, + limitReached: false, + }) + expect(await rowCount(broken)).toBe(3) + expect(await rowCount(healthy)).toBe(0) + expect(signalChanged).toHaveBeenCalledWith(healthy) + expect(signalChanged).not.toHaveBeenCalledWith(broken) + expect(fireTrigger).toHaveBeenCalledTimes(2) + + await expect(runCleanupTableRowTtl()).resolves.toEqual({ + batches: 1, + deleted: 0, + limitReached: false, + }) + expect(await rowCount(broken)).toBe(3) + await removeDeleteFault() + expect((await runCleanupTableRowTtl()).deleted).toBe(3) + expect(await rowCount(broken)).toBe(0) + }) + + it('recovers from a real backend connection loss during DELETE without partial deletion', async () => { + const table = await createTable() + await seedRows(table, 3) + await installDeleteFault(table, 'PERFORM pg_sleep(10);') + const deleting = runCleanupTableRowTtl().then( + (result) => ({ result }), + (error: unknown) => ({ error }) + ) + const pid = await waitForSleepingDelete() + await control`SELECT pg_terminate_backend(${pid})` + expect(await deleting).toEqual({ result: { batches: 1, deleted: 0, limitReached: false } }) + expect(await rowCount(table)).toBe(3) + await removeDeleteFault() + expect((await runCleanupTableRowTtl()).deleted).toBe(3) + expect(await rowCount(table)).toBe(0) + }, 15000) + + it('stops between batches on cancellation and restarts without retaining a stale cursor', async () => { + const table = await createTable() + const batch = getDeleteSnapshotBatchSize() + await seedRows(table, batch + 1) + const abort = new AbortController() + fireTrigger.mockImplementationOnce(async () => abort.abort()) + expect((await runCleanupTableRowTtl(abort.signal)).deleted).toBe(batch) + expect(await rowCount(table)).toBe(1) + expect((await runCleanupTableRowTtl()).deleted).toBe(1) + }) + + it('bounds snapshots by bytes and still progresses past an oversized stored row', async () => { + const table = await createTable() + await seedRows(table, 3) + await control`UPDATE user_table_rows SET data = data || jsonb_build_object('wide', repeat('x', 33 * 1024 * 1024)) WHERE table_id = ${table} AND position = 1` + await control`UPDATE user_table_rows SET data = data || jsonb_build_object('wide', repeat('y', 17 * 1024 * 1024)) WHERE table_id = ${table} AND position > 1` + expect((await runCleanupTableRowTtl()).deleted).toBe(3) + expect(fireTrigger.mock.calls.map((call) => call[4].length)).toEqual([1, 1, 1]) + expect(await rowCount(table)).toBe(0) + }, 30000) + + it('matches equivalent instants for equality and membership without casting malformed stored values', async () => { + const table = await createTable() + const values = [ + '2090-09-07T07:30:00.000001-07:00', + '2090-09-07T20:15:00.000001+05:45', + '2090-09-07T14:30:00.000001Z', + '2090-09-07T14:30:00.000001-00:00', + '2090-09-07T14:30:00.000002-00:00', + null, + '', + '2090-02-30T00:00:00Z', + 'not-a-date', + ] + for (const value of values) { + await control`INSERT INTO user_table_rows (id, table_id, workspace_id, data) + VALUES (${generateId()}, ${table}, ${workspaceId}, ${control.json({ expires: value })})` + } + const column = { id: 'expires', name: 'expires_at', type: 'ttl' as const } + for (const op of ['eq', 'ne', 'in', 'nin'] as const) { + const instant = '2090-09-07T14:30:00.000001+00:00' + const value = op === 'in' || op === 'nin' ? [instant] : instant + const predicate = fieldPredicate('user_table_rows', 'expires', op, value, column) + const rows = await db.execute(sql`SELECT count(*)::int AS count FROM user_table_rows + WHERE table_id = ${table} AND ${predicate}`) + expect(rows[0].count).toBe(op === 'eq' || op === 'in' ? 4 : 5) + } + const nullPredicate = fieldPredicate('user_table_rows', 'expires', 'eq', null, column) + const rows = await db.execute( + sql`SELECT count(*)::int AS count FROM user_table_rows WHERE table_id = ${table} AND ${nullPredicate}` + ) + expect(rows[0].count).toBe(1) + }) + + it('preserves offsets in storage while enforcing uniqueness by the exact instant', async () => { + const table = await createTable() + const uniqueSchema: TableSchema = { + columns: [{ id: 'expires', name: 'expires_at', type: 'ttl', unique: true }], + } + const first = { expires: '2090-09-07T07:30:00.000001-07:00' } + const equivalent = { expires: '2090-09-07T14:30:00.000001Z' } + const nextMicrosecond = { expires: '2090-09-07T20:15:00.000002+05:45' } + for (const row of [first, equivalent, nextMicrosecond]) { + expect(coerceRowToSchema(row, uniqueSchema, 'reject').valid).toBe(true) + } + expect(first.expires).toBe('2090-09-07T07:30:00.000001-07:00') + expect(equivalent.expires).toBe('2090-09-07T14:30:00.000001-00:00') + expect(nextMicrosecond.expires).toBe('2090-09-07T20:15:00.000002+05:45') + const withinBatch = await checkBatchUniqueConstraintsDb( + table, + [first, equivalent, nextMicrosecond], + uniqueSchema + ) + expect(withinBatch.errors.map(({ row }) => row)).toEqual([1]) + await seedRows(table, 1, first.expires) + const againstStored = await checkBatchUniqueConstraintsDb( + table, + [equivalent, nextMicrosecond], + uniqueSchema + ) + expect(againstStored.errors.map(({ row }) => row)).toEqual([0]) + const definition = await getTableById(table) + expect(definition).not.toBeNull() + await expect( + db.transaction((tx) => + replaceTableRowsWithTx( + tx, + { + tableId: table, + workspaceId, + rows: [first, equivalent], + secretProvenance: undefined, + }, + { ...definition!, schema: uniqueSchema }, + 'offset-qa' + ) + ) + ).rejects.toThrow('must be unique') + expect(await rowCount(table)).toBe(1) + await control`INSERT INTO user_table_rows (id, table_id, workspace_id, data, position) + VALUES (${generateId()}, ${table}, ${workspaceId}, ${control.json(equivalent)}, 2)` + await expect( + updateColumnConstraints({ tableId: table, columnName: 'expires', unique: true }, 'offset-qa') + ).rejects.toThrow('duplicate') + await control`UPDATE user_table_rows SET data = ${control.json(nextMicrosecond)} WHERE table_id = ${table} AND position = 2` + const constrained = await updateColumnConstraints( + { tableId: table, columnName: 'expires', unique: true }, + 'offset-qa' + ) + expect(constrained.schema.columns[0].unique).toBe(true) + const stored = + await control`SELECT data->>'expires' AS value FROM user_table_rows WHERE table_id = ${table} ORDER BY position` + expect(stored.map(({ value }) => value)).toEqual([first.expires, nextMicrosecond.expires]) + }) + + it('agrees with PostgreSQL for deterministic offset, leap-year, and precision samples', async () => { + const samples: string[] = [] + for (const year of ['0001', '0099', '1900', '2000', '2024', '2026', '9998']) { + for (const day of ['01-01', '02-28', '03-01', '12-31']) { + for (const offset of [ + 'Z', + '-00:00', + '+00:00', + '-07:00', + '-08:00', + '+05:45', + '+15:59', + '-15:59', + ]) { + for (const fraction of ['', '.000001', '.123400', '.999999']) { + const value = `${year}-${day}T12:34:56${fraction}${offset}` + if (normalizeTtlTimestamp(value) !== null) samples.push(value) + } + } + } + } + const normalized = samples.map((value) => normalizeTtlTimestamp(value)!) + const [result] = await control`SELECT count(*)::int AS mismatch FROM + unnest(${samples}::text[], ${normalized}::text[]) AS instants(input, normalized) + WHERE input::timestamptz != normalized::timestamptz` + expect(result.mismatch).toBe(0) + const guarded = await db.execute(sql`WITH samples AS MATERIALIZED ( + SELECT jsonb_array_elements_text(${JSON.stringify(samples)}::jsonb) AS value + ) SELECT count(*)::int AS mismatch FROM samples + WHERE ${validatedTimestampSql(sql`samples.value`, TTL_TIMESTAMP_VALIDATION)} + IS DISTINCT FROM samples.value::timestamptz`) + expect(guarded[0].mismatch).toBe(0) + measurements.postgresTimestampSamples = samples.length + }) + + it.skipIf(!process.env.TABLE_TTL_QA_STRESS_ROWS)( + 'drains a million-row backlog over bounded passes', + async () => { + const count = Number(process.env.TABLE_TTL_QA_STRESS_ROWS) + expect(count).toBeGreaterThanOrEqual(100000) + expect(count).toBeLessThanOrEqual(1000000) + const table = await createTable() + await seedRows(table, count) + await control`INSERT INTO user_table_rows (id, table_id, workspace_id, data) + VALUES (${generateId()}, ${table}, ${workspaceId}, ${control.json({ expires: future })}), + (${generateId()}, ${table}, ${workspaceId}, ${control.json({ expires: null })})` + const started = performance.now() + let deleted = 0 + let passes = 0 + let maxRss = process.memoryUsage().rss + while (deleted < count) { + const result = await runCleanupTableRowTtl() + expect(result.batches).toBeLessThanOrEqual(100) + expect(result.deleted).toBeGreaterThan(0) + deleted += result.deleted + passes++ + maxRss = Math.max(maxRss, process.memoryUsage().rss) + expect(passes).toBeLessThanOrEqual( + Math.ceil(count / (100 * getDeleteSnapshotBatchSize())) + 1 + ) + fireTrigger.mockClear() + } + expect(deleted).toBe(count) + expect(await rowCount(table)).toBe(2) + measurements.stress = { + rows: count, + passes, + deleted, + survivors: 2, + elapsedMs: Math.round(performance.now() - started), + maxRss, + } + }, + 300000 + ) +}) diff --git a/apps/sim/background/cleanup-table-row-ttl.test.ts b/apps/sim/background/cleanup-table-row-ttl.test.ts index b73e0c4748d..0e6948effbd 100644 --- a/apps/sim/background/cleanup-table-row-ttl.test.ts +++ b/apps/sim/background/cleanup-table-row-ttl.test.ts @@ -3,6 +3,7 @@ */ import type { SQL } from 'drizzle-orm' import { PgDialect } from 'drizzle-orm/pg-core' +import postgres from 'postgres' import { beforeEach, describe, expect, it, vi } from 'vitest' vi.unmock('@sim/db/schema') @@ -16,6 +17,8 @@ const { mockTask, mockWithLockedTable, mockFireTableTrigger, + mockLoggerError, + mockLoggerInfo, } = vi.hoisted(() => ({ mockDeleteExecute: vi.fn(), mockListExecute: vi.fn(), @@ -24,11 +27,16 @@ const { mockTask: vi.fn((config: unknown) => config), mockWithLockedTable: vi.fn(), mockFireTableTrigger: vi.fn(), + mockLoggerError: vi.fn(), + mockLoggerInfo: vi.fn(), })) vi.mock('@sim/db', () => ({ dbFor: vi.fn(() => ({ execute: mockListExecute })), })) +vi.mock('@sim/logger', () => ({ + createLogger: () => ({ info: mockLoggerInfo, warn: vi.fn(), error: mockLoggerError }), +})) vi.mock('@trigger.dev/sdk', () => ({ task: mockTask })) vi.mock('@/lib/table/events', () => ({ signalTableRowsChanged: mockSignalTableRowsChanged })) @@ -85,6 +93,101 @@ describe('table row TTL cleanup', () => { ) }) + it.skipIf(!process.env.TABLE_TTL_TEST_DATABASE_URL)( + 'deletes only expired UTC cells in PostgreSQL with a non-UTC session', + async () => { + const client = postgres(process.env.TABLE_TTL_TEST_DATABASE_URL!, { max: 1 }) + const nowSpy = vi.spyOn(Date, 'now').mockReturnValue(Date.parse('2026-09-07T12:00:00.500Z')) + try { + await client`SET TIME ZONE 'America/Los_Angeles'` + await client`CREATE TEMP TABLE user_table_definitions (id text, workspace_id text, schema jsonb, archived_at timestamp, delete_locked boolean)` + await client`CREATE TEMP TABLE user_table_rows (id text, table_id text, workspace_id text, data jsonb, created_at timestamp DEFAULT now())` + await client`INSERT INTO user_table_definitions VALUES (${table.id}, ${table.workspaceId}, ${client.json(table.schema)}, NULL, false)` + const values = { + expired: '2026-09-07T11:59:59Z', + equal: '2026-09-07T12:00:00Z', + future: '2026-09-07T12:00:01Z', + blank: null, + epoch: 1_700_000_000, + invalid: 'not-a-date', + invalid_day: '2026-02-30T12:00:00Z', + invalid_month: '2026-13-01T12:00:00Z', + invalid_year: '0000-01-01T00:00:00Z', + invalid_leap_day: '2025-02-29T12:00:00Z', + invalid_century_leap_day: '1900-02-29T12:00:00Z', + invalid_month_end: '2026-04-31T12:00:00Z', + invalid_hour: '2026-09-06T24:00:00Z', + invalid_minute: '2026-09-06T12:60:00Z', + invalid_second: '2026-09-06T12:00:60Z', + leap_day: '2024-02-29T12:00:00Z', + century_leap_day: '2000-02-29T12:00:00Z', + first_year: '0001-01-01T00:00:00Z', + last_year: '9999-12-31T23:59:59Z', + offset: '2026-09-06T12:00:00+00:00', + fraction: '2026-09-06T12:00:00.000Z', + negative_offset: '2026-09-07T04:59:59-07:00', + positive_offset: '2026-09-07T18:00:00+06:00', + future_offset: '2026-09-07T12:00:00-07:00', + equal_fraction: '2026-09-07T12:00:00.500000Z', + future_microsecond: '2026-09-07T12:00:00.500001Z', + minute_precision: '2026-09-07T12:00Z', + invalid_offset_day: '2026-02-30T12:00:00-07:00', + invalid_offset: '2026-09-07T12:00:00+16:00', + invalid_fraction: '2026-09-07T12:00:00.0000001Z', + rounding_future: '2026-09-07T12:00:00.5000001Z', + no_offset: '2020-01-01T00:00:00', + day_only: '2020-01-01', + relative_now: 'now', + relative_today: 'today', + relative_yesterday: 'yesterday', + epoch_alias: 'epoch', + past_infinity: '-infinity', + compact_offset: '2020-01-01T00:00:00+0000', + named_zone: '2020-01-01 00:00:00 America/Los_Angeles', + trailing_newline: '2020-01-01T00:00:00Z\n', + } + for (const [id, value] of Object.entries(values)) { + await client`INSERT INTO user_table_rows (id, table_id, workspace_id, data) VALUES (${id}, ${table.id}, ${table.workspaceId}, ${client.json({ 'col-ttl': value })})` + } + const execute = (statement: SQL) => { + const query = dialect.sqlToQuery(statement) + return client.unsafe(query.sql, query.params as (string | number)[]) + } + mockListExecute.mockImplementation(execute) + mockDeleteExecute.mockImplementation(execute) + expect(await runCleanupTableRowTtl()).toEqual({ + batches: 2, + deleted: 11, + limitReached: false, + }) + const remaining = await client<{ id: string }[]>`SELECT id FROM user_table_rows ORDER BY id` + expect(remaining.map(({ id }) => id)).toEqual( + Object.keys(values) + .filter( + (id) => + ![ + 'expired', + 'equal', + 'leap_day', + 'century_leap_day', + 'first_year', + 'offset', + 'fraction', + 'negative_offset', + 'positive_offset', + 'equal_fraction', + 'minute_precision', + ].includes(id) + ) + .sort() + ) + } finally { + nowSpy.mockRestore() + await client.end() + } + } + ) + it('deletes expired rows in locked, created-at keyset batches and signals the table', async () => { mockDeleteExecute .mockResolvedValueOnce([ @@ -123,9 +226,9 @@ describe('table row TTL cleanup', () => { ) }) - it('compares TTL values with whole Date.now epoch seconds', async () => { + it('compares TTL timestamps with the current UTC instant', async () => { const nowEpochMilliseconds = 1_700_000_000_999 - const nowEpochSeconds = 1_700_000_000 + const nowUtc = '2023-11-14T22:13:20.999Z' const nowSpy = vi.spyOn(Date, 'now').mockReturnValue(nowEpochMilliseconds) mockDeleteExecute.mockResolvedValue([]) @@ -135,12 +238,8 @@ describe('table row TTL cleanup', () => { nowSpy.mockRestore() } - expect(dialect.sqlToQuery(mockListExecute.mock.calls[0][0] as SQL).params).toContain( - nowEpochSeconds - ) - expect(dialect.sqlToQuery(mockDeleteExecute.mock.calls[0][0] as SQL).params).toContain( - nowEpochSeconds - ) + expect(dialect.sqlToQuery(mockListExecute.mock.calls[0][0] as SQL).params).toContain(nowUtc) + expect(dialect.sqlToQuery(mockDeleteExecute.mock.calls[0][0] as SQL).params).toContain(nowUtc) }) it('checks the oldest expired rows first without using creation time as an expiry rule', async () => { @@ -153,7 +252,7 @@ describe('table row TTL cleanup', () => { .sql.replace(/\s+/g, ' ') .replace(/\$\d+/g, '?') .trim() - expect(query).toContain('AND (table_row.data->>?)::numeric <= ?') + expect(query).toContain('THEN (table_row.data->>?)::timestamptz END <= ?::timestamptz') expect(query).toContain('ORDER BY table_row.created_at, table_row.id') expect(query).toContain('octet_length(table_row.data::text) AS snapshot_bytes') expect(query).toContain('cumulative_snapshot_bytes <= ?') @@ -165,12 +264,23 @@ describe('table row TTL cleanup', () => { expect(query).not.toContain('table_row.created_by') }) - it('rejects a batch without a creation-time cursor', async () => { + it('skips a table whose batch has no creation-time cursor without signaling deletion', async () => { mockDeleteExecute.mockResolvedValue([{ id: 'row-1', data: { value: 1 } }]) - await expect(runCleanupTableRowTtl()).rejects.toThrow( - 'Table row TTL cleanup did not return a creation-time cursor' + await expect(runCleanupTableRowTtl()).resolves.toEqual({ + batches: 1, + deleted: 0, + limitReached: false, + }) + expect(mockLoggerError).toHaveBeenCalledWith( + 'Table row TTL cleanup failed; skipping table for this run', + expect.objectContaining({ + tableId: table.id, + error: new Error('Table row TTL cleanup did not return a creation-time cursor'), + }) ) + expect(mockFireTableTrigger).not.toHaveBeenCalled() + expect(mockSignalTableRowsChanged).not.toHaveBeenCalled() }) it('does no work when already aborted', async () => { @@ -265,23 +375,123 @@ describe('table row TTL cleanup', () => { expect(mockSignalTableRowsChanged).toHaveBeenCalledWith(secondTable.id) }) - it('signals tables changed before a later table cleanup failure propagates', async () => { - const secondTable = { - ...table, - id: 'table-2', - } + it('skips a failed table, finishes healthy tables, and retries the failed table next run', async () => { + const secondTable = { ...table, id: 'table-2' } mockListExecute.mockResolvedValue([ { id: table.id, workspaceId: table.workspaceId }, { id: secondTable.id, workspaceId: secondTable.workspaceId }, ]) + mockDeleteExecute.mockResolvedValueOnce(returnedRows(1)).mockResolvedValue([]) mockWithLockedTable.mockImplementation(async (tableId, mutate) => { - if (tableId === secondTable.id) throw new Error('second table cleanup failed') - return mutate(table, { execute: vi.fn().mockResolvedValue(returnedRows(1)) }) + if (tableId === table.id) throw new Error('first table cleanup failed') + return mutate(secondTable, { execute: mockDeleteExecute }) }) - await expect(runCleanupTableRowTtl()).rejects.toThrow('second table cleanup failed') + await expect(runCleanupTableRowTtl()).resolves.toEqual({ + batches: 3, + deleted: 1, + limitReached: false, + }) + expect(mockWithLockedTable.mock.calls.map(([id]) => id)).toEqual([ + table.id, + secondTable.id, + secondTable.id, + ]) expect(mockSignalTableRowsChanged).toHaveBeenCalledTimes(1) + expect(mockSignalTableRowsChanged).toHaveBeenCalledWith(secondTable.id) + expect(mockLoggerError).toHaveBeenCalledWith( + 'Table row TTL cleanup failed; skipping table for this run', + { + tableId: table.id, + workspaceId: table.workspaceId, + deleted: 0, + error: new Error('first table cleanup failed'), + } + ) + expect(mockLoggerInfo).toHaveBeenCalledWith('Table row TTL cleanup completed', { + batches: 3, + deleted: 1, + failedTables: 1, + limitReached: false, + }) + + mockListExecute.mockResolvedValue([{ id: table.id, workspaceId: table.workspaceId }]) + mockWithLockedTable.mockImplementation(async (_tableId, mutate) => + mutate(table, { execute: mockDeleteExecute }) + ) + mockDeleteExecute.mockClear().mockResolvedValueOnce(returnedRows(1)) + await expect(runCleanupTableRowTtl()).resolves.toEqual({ + batches: 2, + deleted: 1, + limitReached: false, + }) + const retryQuery = dialect.sqlToQuery(mockDeleteExecute.mock.calls[0][0] as SQL) + expect(retryQuery.sql).not.toContain('(table_row.created_at, table_row.id) >') + expect(mockSignalTableRowsChanged).toHaveBeenCalledWith(table.id) + }) + + it('keeps and signals prior commits when a later batch fails while other tables finish', async () => { + const secondTable = { ...table, id: 'table-2' } + const firstExecute = vi + .fn() + .mockResolvedValueOnce(returnedRows(1)) + .mockRejectedValue(new Error('later batch failed')) + const secondExecute = vi.fn().mockResolvedValueOnce(returnedRows(1)).mockResolvedValue([]) + mockListExecute.mockResolvedValue([ + { id: table.id, workspaceId: table.workspaceId }, + { id: secondTable.id, workspaceId: secondTable.workspaceId }, + ]) + mockWithLockedTable.mockImplementation(async (tableId, mutate) => + tableId === table.id + ? mutate(table, { execute: firstExecute }) + : mutate(secondTable, { execute: secondExecute }) + ) + + await expect(runCleanupTableRowTtl()).resolves.toEqual({ + batches: 4, + deleted: 2, + limitReached: false, + }) + expect(mockWithLockedTable.mock.calls.map(([id]) => id)).toEqual([ + table.id, + secondTable.id, + table.id, + secondTable.id, + ]) + expect(mockSignalTableRowsChanged).toHaveBeenCalledTimes(2) expect(mockSignalTableRowsChanged).toHaveBeenCalledWith(table.id) + expect(mockSignalTableRowsChanged).toHaveBeenCalledWith(secondTable.id) + expect(mockFireTableTrigger).toHaveBeenCalledTimes(2) + }) + + it('counts a failed attempt toward the run limit without repeatedly retrying that table', async () => { + const secondTable = { ...table, id: 'table-2' } + mockListExecute.mockResolvedValue([ + { id: table.id, workspaceId: table.workspaceId }, + { id: secondTable.id, workspaceId: secondTable.workspaceId }, + ]) + mockDeleteExecute.mockResolvedValue(returnedRows(500)) + mockWithLockedTable.mockImplementation(async (tableId, mutate) => { + if (tableId === table.id) throw new Error('persistent table failure') + return mutate(secondTable, { execute: mockDeleteExecute }) + }) + + await expect(runCleanupTableRowTtl()).resolves.toEqual({ + batches: 100, + deleted: 49_500, + limitReached: true, + }) + expect(mockWithLockedTable).toHaveBeenCalledTimes(100) + expect(mockWithLockedTable.mock.calls.filter(([id]) => id === table.id)).toHaveLength(1) + expect(mockDeleteExecute).toHaveBeenCalledTimes(99) + }) + + it('still rejects when table discovery fails before any table can be processed', async () => { + mockListExecute.mockRejectedValue(new Error('database unavailable')) + + await expect(runCleanupTableRowTtl()).rejects.toThrow('database unavailable') + expect(mockWithLockedTable).not.toHaveBeenCalled() + expect(mockSignalTableRowsChanged).not.toHaveBeenCalled() }) it('registers one serialized Trigger.dev task', () => { diff --git a/apps/sim/background/cleanup-table-row-ttl.ts b/apps/sim/background/cleanup-table-row-ttl.ts index 9b05e26e702..9a40bbafdaf 100644 --- a/apps/sim/background/cleanup-table-row-ttl.ts +++ b/apps/sim/background/cleanup-table-row-ttl.ts @@ -2,9 +2,10 @@ import { dbFor } from '@sim/db' import { userTableDefinitions, userTableRows } from '@sim/db/schema' import { createLogger } from '@sim/logger' import { task } from '@trigger.dev/sdk' -import { sql } from 'drizzle-orm' +import { type SQL, sql } from 'drizzle-orm' import { asOrchestrationError } from '@/lib/core/orchestration/types' import { getColumnId } from '@/lib/table/column-keys' +import { validatedTimestampSql } from '@/lib/table/column-types/timestamp-sql' import { getDeleteSnapshotBatchSize, TABLE_LIMITS } from '@/lib/table/constants' import { signalTableRowsChanged } from '@/lib/table/events' import { assertRowDelete, TableLockedError } from '@/lib/table/mutation-locks' @@ -13,6 +14,7 @@ import type { DeletedTableRow } from '@/lib/table/rows/ordering' import { withLockedTable } from '@/lib/table/service' import { fireTableTrigger } from '@/lib/table/trigger' import { isTableRowTtlEnabled } from '@/lib/table/ttl-availability' +import { TTL_TIMESTAMP_VALIDATION } from '@/lib/table/ttl-values' import type { RowData, TableSchema } from '@/lib/table/types' const logger = createLogger('CleanupTableRowTtl') @@ -58,7 +60,12 @@ export interface TableRowTtlCleanupResult { limitReached: boolean } -async function listExpiredTtlTables(nowEpochSeconds: number): Promise { +/** Shares PostgreSQL's validated instant projection with Expiration comparisons. */ +function expiredTtlPredicate(cell: SQL, nowUtc: string): SQL { + return sql`${validatedTimestampSql(cell, TTL_TIMESTAMP_VALIDATION)} <= ${nowUtc}::timestamptz` +} + +async function listExpiredTtlTables(nowUtc: string): Promise { const rows = await cleanupDb.execute(sql` SELECT ${userTableDefinitions.id} AS id, @@ -75,21 +82,16 @@ async function listExpiredTtlTables(nowEpochSeconds: number): Promise>'type' = 'ttl' - AND jsonb_typeof( - table_row.data->COALESCE( - ttl_column.column_definition->>'id', - ttl_column.column_definition->>'name' - ) - ) = 'number' - AND ( - table_row.data->>COALESCE( - ttl_column.column_definition->>'id', - ttl_column.column_definition->>'name' - ) - )::numeric <= ${nowEpochSeconds} + AND ${expiredTtlPredicate( + sql`table_row.data->>COALESCE( + ttl_column.column_definition->>'id', + ttl_column.column_definition->>'name' + )`, + nowUtc + )} ) ORDER BY - md5(${userTableDefinitions.id} || ${nowEpochSeconds}::text), + md5(${userTableDefinitions.id} || ${nowUtc}::text), ${userTableDefinitions.id} LIMIT ${TTL_CLEANUP_MAX_BATCHES} `) @@ -144,7 +146,7 @@ async function deleteExpiredTableRowBatch( tableId: string, workspaceId: string, columnKey: string, - nowEpochSeconds: number, + nowUtc: string, batchSize: number, after?: TtlCleanupCursor ): Promise { @@ -164,8 +166,7 @@ async function deleteExpiredTableRowBatch( ? sql`AND (table_row.created_at, table_row.id) > (${after.createdAt}::timestamp, ${after.id})` : sql`` } - AND jsonb_typeof(table_row.data->${columnKey}) = 'number' - AND (table_row.data->>${columnKey})::numeric <= ${nowEpochSeconds} + AND ${expiredTtlPredicate(sql`table_row.data->>${columnKey}`, nowUtc)} ORDER BY table_row.created_at, table_row.id LIMIT ${batchSize} FOR UPDATE OF table_row SKIP LOCKED @@ -201,7 +202,7 @@ async function deleteExpiredTableRowBatch( async function deleteExpiredRowsForTable( ref: ExpiredTtlTableRef, - nowEpochSeconds: number, + nowUtc: string, batchSize: number, after?: TtlCleanupCursor ): Promise { @@ -226,7 +227,7 @@ async function deleteExpiredRowsForTable( table.id, table.workspaceId, getColumnId(ttlColumn), - nowEpochSeconds, + nowUtc, batchSize, after ) @@ -260,7 +261,7 @@ async function deleteExpiredRowsForTable( } } -/** Deletes rows whose table TTL cell is at or before the current Unix epoch second. */ +/** Deletes rows whose table TTL cell is at or before the current UTC instant. */ export async function runCleanupTableRowTtl( signal?: AbortSignal ): Promise { @@ -270,9 +271,9 @@ export async function runCleanupTableRowTtl( return { batches: 0, deleted: 0, limitReached: false } } - const nowEpochSeconds = Math.floor(Date.now() / 1000) + const nowUtc = new Date(Date.now()).toISOString() const batchSize = getDeleteSnapshotBatchSize() - const tableRefs = await listExpiredTtlTables(nowEpochSeconds) + const tableRefs = await listExpiredTtlTables(nowUtc) const tableStates: TtlTableCleanupState[] = tableRefs.map((ref) => ({ ref, deleted: 0, @@ -280,6 +281,7 @@ export async function runCleanupTableRowTtl( })) let deleted = 0 let batches = 0 + let failedTables = 0 try { while ( @@ -291,12 +293,21 @@ export async function runCleanupTableRowTtl( if (state.complete) continue if (batches === TTL_CLEANUP_MAX_BATCHES || signal?.aborted) break - const batch = await deleteExpiredRowsForTable( - state.ref, - nowEpochSeconds, - batchSize, - state.after - ) + let batch: DeletedTtlBatch + try { + batch = await deleteExpiredRowsForTable(state.ref, nowUtc, batchSize, state.after) + } catch (error) { + batches++ + failedTables++ + state.complete = true + logger.error('Table row TTL cleanup failed; skipping table for this run', { + tableId: state.ref.id, + workspaceId: state.ref.workspaceId, + deleted: state.deleted, + error, + }) + continue + } if (!batch.attempted) { state.complete = true continue @@ -318,7 +329,7 @@ export async function runCleanupTableRowTtl( const limitReached = batches === TTL_CLEANUP_MAX_BATCHES && (tableStates.some((state) => !state.complete) || tableRefs.length === TTL_CLEANUP_MAX_BATCHES) - logger.info('Table row TTL cleanup completed', { batches, deleted, limitReached }) + logger.info('Table row TTL cleanup completed', { batches, deleted, failedTables, limitReached }) return { batches, deleted, limitReached } } diff --git a/apps/sim/background/schedule-execution.ts b/apps/sim/background/schedule-execution.ts index 6163278867f..0a34f19d5fd 100644 --- a/apps/sim/background/schedule-execution.ts +++ b/apps/sim/background/schedule-execution.ts @@ -6,7 +6,7 @@ import { workflowExecutionLogs, workflowSchedule, } from '@sim/db' -import { createLogger, runWithRequestContext } from '@sim/logger' +import { createLogger, type RequestContext, runWithRequestContext } from '@sim/logger' import { describeError, toError } from '@sim/utils/errors' import { generateId } from '@sim/utils/id' import { task, timeout } from '@trigger.dev/sdk' @@ -822,7 +822,12 @@ export async function executeScheduleJob( const scheduledFor = payload.scheduledFor ? new Date(payload.scheduledFor) : null try { - return await runWithRequestContext({ requestId }, async () => { + /** A trigger, not a client, started this run. */ + const requestContext: RequestContext = { + requestId, + client: { surface: 'schedule', source: 'trigger' }, + } + return await runWithRequestContext(requestContext, async () => { logger.info(`[${requestId}] Starting schedule execution`, { scheduleId: payload.scheduleId, workflowId: payload.workflowId, diff --git a/apps/sim/background/webhook-execution.ts b/apps/sim/background/webhook-execution.ts index 5240bc500c9..78a2f5aa7b0 100644 --- a/apps/sim/background/webhook-execution.ts +++ b/apps/sim/background/webhook-execution.ts @@ -6,7 +6,7 @@ import { } from '@sim/auth/principal' import { db } from '@sim/db' import { account, webhook } from '@sim/db/schema' -import { createLogger, runWithRequestContext } from '@sim/logger' +import { createLogger, type RequestContext, runWithRequestContext } from '@sim/logger' import { toError } from '@sim/utils/errors' import { interruptibleSleep } from '@sim/utils/helpers' import { generateId } from '@sim/utils/id' @@ -556,7 +556,12 @@ export async function executeWebhookJob( }) } - return await runWithRequestContext({ requestId }, async () => { + /** A trigger, not a client, started this run. */ + const requestContext: RequestContext = { + requestId, + client: { surface: 'webhook', source: 'trigger' }, + } + return await runWithRequestContext(requestContext, async () => { logger.info(`[${requestId}] Starting webhook execution`, { webhookId: authenticatedPayload.webhookId, workflowId: authenticatedPayload.workflowId, diff --git a/apps/sim/background/workflow-execution.ts b/apps/sim/background/workflow-execution.ts index e4b82ca8034..b04e5a0610c 100644 --- a/apps/sim/background/workflow-execution.ts +++ b/apps/sim/background/workflow-execution.ts @@ -25,6 +25,7 @@ import { getTimeoutErrorMessage, RESERVATION_TTL_BUFFER_MS, } from '@/lib/core/execution-limits' +import type { RequestAttribution } from '@/lib/core/utils/request-attribution' import { preprocessExecution } from '@/lib/execution/preprocessing' import { LoggingSession } from '@/lib/logs/execution/logging-session' import { buildTraceSpans } from '@/lib/logs/execution/trace-spans/trace-spans' @@ -76,6 +77,8 @@ export type WorkflowExecutionPayload = { correlation?: AsyncExecutionCorrelation metadata?: Record callChain?: string[] + /** Who queued the run, restored into the job's context so its events stay attributed. */ + attribution?: RequestAttribution executionMode?: 'sync' | 'stream' | 'async' /** Upstream preprocessing already consumed rate-limit quota and owns the usage reservation. */ admissionCompleted?: boolean @@ -177,7 +180,7 @@ export async function executeWorkflowJob( } } - return await runWithRequestContext({ requestId }, async () => { + return await runWithRequestContext({ requestId, ...payload.attribution }, async () => { logger.info(`[${requestId}] Starting workflow execution job: ${workflowId}`, { userId: payload.userId, triggerType: payload.triggerType, diff --git a/apps/sim/components/integrations/slack-search-setup-wizard.tsx b/apps/sim/components/integrations/slack-search-setup-wizard.tsx index 3f33eee1fc3..3cc991913e9 100644 --- a/apps/sim/components/integrations/slack-search-setup-wizard.tsx +++ b/apps/sim/components/integrations/slack-search-setup-wizard.tsx @@ -20,6 +20,7 @@ import { useSlackSearchManifest, useStartSlackSearchOAuth } from '@/hooks/querie interface SlackSearchSetupWizardProps { organizationId: string + mode?: 'custom' | 'shared' installationId?: string appId?: string initialName?: string @@ -29,6 +30,7 @@ interface SlackSearchSetupWizardProps { /** App creation, credentials, and consent are one organization-specific setup flow. */ export function SlackSearchSetupWizard({ organizationId, + mode, installationId, appId, initialName, @@ -61,9 +63,12 @@ export function SlackSearchSetupWizard({ } } - const shared = Boolean( - prepare.data?.sharedAppId && (!configuredAppId || configuredAppId === prepare.data.sharedAppId) - ) + const shared = mode + ? mode === 'shared' + : Boolean( + prepare.data?.sharedAppId && + (!configuredAppId || configuredAppId === prepare.data.sharedAppId) + ) function installShared() { oauth.mutate( @@ -144,22 +149,40 @@ export function SlackSearchSetupWizard({ onOpenChange={(open) => { if (!open) onClose() }} - srTitle='Install Sim Search' + srTitle='Install the Sim Search app' + size='sm' > - Install Sim Search + Install the Sim Search app

- Choose your Slack workspace and approve Sim Search. + Add Sim Search to your Slack workspace to ask questions and get answers from your + connected sources.

- {error?.message} + + {error?.message ?? + (!prepare.data.sharedAppId + ? 'Sim Search installation is unavailable. Try again.' + : null)} + void prepare.refetch(), + disabled: prepare.isFetching, + }, + ] + : undefined + } primaryAction={{ - label: busy ? 'Connecting…' : 'Install Sim Search', - disabled: busy, + label: busy ? 'Connecting…' : 'Continue with Slack', + disabled: busy || !prepare.data.sharedAppId || Boolean(prepare.error), onClick: installShared, }} /> diff --git a/apps/sim/connectors/confluence/confluence.test.ts b/apps/sim/connectors/confluence/confluence.test.ts index f4c3601b9a9..894292d982a 100644 --- a/apps/sim/connectors/confluence/confluence.test.ts +++ b/apps/sim/connectors/confluence/confluence.test.ts @@ -10,15 +10,14 @@ import { buildLastModifiedClause, confluenceConnector, confluenceStorageToPlainText, + confluenceViewToPlainText, DYNAMIC_CONTENT_SKIP_REASON, escapeCql, extractConfluenceStorageText, isCurrentContent, - preserveConfluenceCallouts, readIncludedLabels, } from '@/connectors/confluence/confluence' import { extractCursor } from '@/connectors/confluence/cursor' -import { htmlToPlainText } from '@/connectors/utils' describe('Confluence service-account scopes', () => { it('requests metadata and role reads needed for complete mirrored ACLs', () => { @@ -216,14 +215,14 @@ describe('readIncludedLabels', () => { }) }) -describe('preserveConfluenceCallouts', () => { +describe('confluenceViewToPlainText', () => { it.concurrent('handles empty content', () => { - expect(preserveConfluenceCallouts('')).toBe('') + expect(confluenceViewToPlainText('')).toBe('') }) it.concurrent('leaves content with no macros unchanged', () => { const html = '

Just a normal paragraph.

' - expect(preserveConfluenceCallouts(html)).toContain('Just a normal paragraph.') + expect(confluenceViewToPlainText(html)).toContain('Just a normal paragraph.') }) it.concurrent('labels a built-in warning macro and keeps its body', () => { @@ -232,7 +231,7 @@ describe('preserveConfluenceCallouts', () => { '' + '

Do NOT use this form for GitLab access.

' + '
' - const result = preserveConfluenceCallouts(html) + const result = confluenceViewToPlainText(html) expect(result).toContain('[WARNING]') expect(result).toContain('Do NOT use this form for GitLab access.') }) @@ -242,7 +241,7 @@ describe('preserveConfluenceCallouts', () => { '
' + '

Heads up.

' + '
' - expect(preserveConfluenceCallouts(html)).toContain('[INFO] Heads up.') + expect(confluenceViewToPlainText(html)).toContain('[INFO] Heads up.') }) it.concurrent('labels a built-in note macro', () => { @@ -250,7 +249,7 @@ describe('preserveConfluenceCallouts', () => { '
' + '

See also.

' + '
' - expect(preserveConfluenceCallouts(html)).toContain('[NOTE] See also.') + expect(confluenceViewToPlainText(html)).toContain('[NOTE] See also.') }) it.concurrent('labels a built-in tip macro', () => { @@ -258,7 +257,7 @@ describe('preserveConfluenceCallouts', () => { '
' + '

Pro tip.

' + '
' - expect(preserveConfluenceCallouts(html)).toContain('[TIP] Pro tip.') + expect(confluenceViewToPlainText(html)).toContain('[TIP] Pro tip.') }) it.concurrent('labels a generic custom-colored Panel macro using its header title', () => { @@ -267,7 +266,7 @@ describe('preserveConfluenceCallouts', () => { '
Do NOT use this form for:
' + '

GitLab access requests go to the private channel instead.

' + '
' - const result = preserveConfluenceCallouts(html) + const result = confluenceViewToPlainText(html) expect(result).toContain('[CALLOUT: Do NOT use this form for:]') expect(result).toContain('GitLab access requests go to the private channel instead.') }) @@ -276,7 +275,7 @@ describe('preserveConfluenceCallouts', () => { const html = '
Warning:
' + '

See replacement form.

' - const result = preserveConfluenceCallouts(html) + const result = confluenceViewToPlainText(html) expect(result).toContain('[CALLOUT: Warning:] See replacement form.') }) @@ -286,7 +285,7 @@ describe('preserveConfluenceCallouts', () => { const html = '
Warning: Do not use
' + '

See replacement form.

' - const result = preserveConfluenceCallouts(html) + const result = confluenceViewToPlainText(html) expect(result).toContain('[CALLOUT: Warning: Do not use]') } ) @@ -294,13 +293,13 @@ describe('preserveConfluenceCallouts', () => { it.concurrent('falls back to a bare CALLOUT label when a Panel macro has no header text', () => { const html = '

Untitled panel body.

' - const result = preserveConfluenceCallouts(html) + const result = confluenceViewToPlainText(html) expect(result).toContain('[CALLOUT]') expect(result).toContain('Untitled panel body.') }) it.concurrent( - 'keeps the exclusion marker attached to its content through htmlToPlainText, even across surrounding whitespace collapse', + 'keeps the exclusion marker attached to its content across surrounding whitespace collapse', () => { const html = '

Intro paragraph.

\n\n' + @@ -309,7 +308,7 @@ describe('preserveConfluenceCallouts', () => { '
  • GitLab
' + '\n\n' + '

Trailing paragraph.

' - const plainText = htmlToPlainText(preserveConfluenceCallouts(html)) + const plainText = confluenceViewToPlainText(html) expect(plainText).toContain('[WARNING] Do NOT use this form for: GitLab') expect(plainText).toContain('Intro paragraph.') expect(plainText).toContain('Trailing paragraph.') @@ -325,7 +324,7 @@ describe('preserveConfluenceCallouts', () => { '

Do NOT use this form for:

' + '
  • GitLab
  • ServiceNow
' + '' - const result = preserveConfluenceCallouts(html) + const result = confluenceViewToPlainText(html) expect(result).not.toContain('for:GitLab') expect(result).not.toContain('GitLabServiceNow') expect(result).toContain('Do NOT use this form for: GitLab ServiceNow') @@ -339,7 +338,7 @@ describe('preserveConfluenceCallouts', () => { '
' + '

First sentence.

Second sentence.

' + '
' - const result = preserveConfluenceCallouts(html) + const result = confluenceViewToPlainText(html) expect(result).toContain('First sentence. Second sentence.') expect(result).not.toContain('sentence.Second') } @@ -355,7 +354,7 @@ describe('preserveConfluenceCallouts', () => { '
  • Nested item A
  • Nested item B
' + '
  • Outer item two
    • ' + '' - const result = preserveConfluenceCallouts(html) + const result = confluenceViewToPlainText(html) // Each nested
  • 's text must appear exactly once, not duplicated by the // outer
  • also being matched and its .text() recursing into it. const occurrences = (result.match(/Nested item A/g) ?? []).length @@ -370,7 +369,7 @@ describe('preserveConfluenceCallouts', () => { '
    ' + '
    Cell text

    quoted text

    after quote
    ' + '
    ' - const result = preserveConfluenceCallouts(html) + const result = confluenceViewToPlainText(html) expect(result).not.toContain('quotedtext') expect(result).not.toContain('textafter') expect(result).toContain('Cell text quoted text after quote') @@ -383,7 +382,7 @@ describe('preserveConfluenceCallouts', () => { '
    ' + '

    This is unbelieveable.

    ' + '
    ' - const result = preserveConfluenceCallouts(html) + const result = confluenceViewToPlainText(html) expect(result).not.toContain('un believe able') expect(result).toContain('This is unbelieveable.') } @@ -394,7 +393,7 @@ describe('preserveConfluenceCallouts', () => { '
    ' + '

    Do not proceed!

    ' + '
    ' - const result = preserveConfluenceCallouts(html) + const result = confluenceViewToPlainText(html) expect(result).not.toContain('proceed !') expect(result).toContain('[WARNING] Do not proceed!') }) @@ -404,7 +403,7 @@ describe('preserveConfluenceCallouts', () => { '
    ' + '

    Do NOT use this form.

    ' + '
    ' - const result = preserveConfluenceCallouts(html) + const result = confluenceViewToPlainText(html) expect(result).toContain('Do NOT use this form.') }) @@ -416,7 +415,7 @@ describe('preserveConfluenceCallouts', () => { '
    Inner
    ' + '

    inner body

    ' + '' - const result = preserveConfluenceCallouts(html) + const result = confluenceViewToPlainText(html) expect(result).toContain('[CALLOUT: Outer]') expect(result).toContain('[CALLOUT: Inner] inner body') } @@ -430,7 +429,7 @@ describe('preserveConfluenceCallouts', () => { '
    ' + '

    Do not use this.

    ' + '' - const result = preserveConfluenceCallouts(html) + const result = confluenceViewToPlainText(html) expect(result).toContain('[WARNING] Do not use this.') } ) @@ -443,7 +442,7 @@ describe('preserveConfluenceCallouts', () => { '
    Inner title
    ' + '

    inner body

    ' + '' - const result = preserveConfluenceCallouts(html) + const result = confluenceViewToPlainText(html) // The outer panel has no header of its own — it must fall back to a // bare [CALLOUT], not steal "Inner title" from the nested panel. expect(result).toContain('[CALLOUT] [CALLOUT: Inner title] inner body') @@ -455,10 +454,98 @@ describe('preserveConfluenceCallouts', () => { '
    ' + '

    Do NOT use this form for:
    GitLab

    ' + '
    ' - const result = preserveConfluenceCallouts(html) + const result = confluenceViewToPlainText(html) expect(result).not.toContain('for:GitLab') expect(result).toContain('[WARNING] Do NOT use this form for: GitLab') }) + + it.concurrent('drops app macro bootstrap scripts, inline styles, and chart data', () => { + const html = + '

    ' + + 'Colored text

    ' + + '' + + '
    ' + + '
    ' + + '' + + '

    After

    ' + expect(confluenceViewToPlainText(html)).toBe('Colored text After') + }) + + it.concurrent('keeps the word break a dropped script or style occupied', () => { + expect(confluenceViewToPlainText('

    BeforeAfter

    ')).toBe('Before After') + }) + + it.concurrent('treats a page holding only an app macro as having no text', () => { + const html = + '
    ' + + '
    ' + expect(confluenceViewToPlainText(html)).toBe('') + }) + + it.concurrent('reduces an unresolved Jira issue macro to its issue key', () => { + const html = + '

    Tracked in ' + + '' + + '' + + 'ENG-101 - ' + + 'Getting issue details... ' + + 'STATUS' + + ' and ' + + '' + + '' + + ' ENG-102 - ' + + '이슈 세부사항 가져오는 중... ' + + '상태' + + '.

    ' + expect(confluenceViewToPlainText(html)).toBe('Tracked in ENG-101 and ENG-102 .') + }) + + it.concurrent('falls back to the data attribute when the issue key link has no text', () => { + const html = + '' + + 'Getting issue details...' + + 'STATUS' + expect(confluenceViewToPlainText(html)).toBe('ENG-103') + }) + + it.concurrent('keeps the summary and status of a Jira issue macro Confluence resolved', () => { + const html = + '' + + '' + + 'OPS-201 - ' + + 'Rotate the signing key ' + + 'Done' + + '' + expect(confluenceViewToPlainText(html)).toBe('OPS-201 - Rotate the signing key Done') + }) + + it.concurrent('keeps the block break of a Jira issue macro inside a callout', () => { + const html = + '
    ' + + '
    Blocked by' + + '
    ' + + 'ENG-104 - ' + + 'Getting issue details...' + + 'STATUS
    ' + + 'until release
    ' + expect(confluenceViewToPlainText(html)).toBe('[WARNING] Blocked by ENG-104 until release') + }) + + it.concurrent('drops only the placeholder shell of a Jira issues table', () => { + const html = + '

    Release notes

    ' + + '
    ' + + '
    ' + + '
    typekeysummary
    ' + + '
    Loading...
    ' + + '
    ' + + '' + + '
    keysummary
    ENG-104Update the runbook
    ' + + '
    No issues found
    ' + expect(confluenceViewToPlainText(html)).toBe( + 'Release notes key summary ENG-104 Update the runbook No issues found' + ) + }) }) describe('confluence incremental CQL listing', () => { @@ -1055,7 +1142,29 @@ describe('Confluence permission-scoped content', () => { ) expect(document?.content).toContain('CONFIDENTIAL SALARY DATA') - expect(document?.contentHash).toBe('confluence:view-callouts:shared-page:1') + expect(document?.contentHash).toBe('confluence:view-text-v2:shared-page:1') + }) + + it('reports the content type of the endpoint that answered and omits unknown metadata', async () => { + vi.mocked(fetch).mockResolvedValueOnce(new Response('', { status: 404 })) + + const document = await confluenceConnector.getDocument('token', config, 'shared-page', { + cloudId: 'cloud-1', + }) + + expect(vi.mocked(fetch).mock.calls.map(([input]) => new URL(String(input)).pathname)).toEqual([ + '/ex/confluence/cloud-1/wiki/api/v2/pages/shared-page', + '/ex/confluence/cloud-1/wiki/api/v2/blogposts/shared-page', + ]) + expect(document?.metadata).toEqual({ + spaceId: 'space-1', + contentType: 'blogpost', + status: 'current', + version: 1, + labels: [], + lastModified: '', + }) + expect(document?.metadata).not.toHaveProperty('spaceKey') }) it('rejects a missing storage body without falling back to rendered content', async () => { @@ -1118,7 +1227,7 @@ describe('Confluence permission-scoped content', () => { const expectedHash = 'mirrorsSourceAcls' in mode || 'perMemberListing' in mode ? 'confluence:storage-local-body-v2:shared-page:1' - : 'confluence:view-callouts:shared-page:1' + : 'confluence:view-text-v2:shared-page:1' expect(v2.documents[0].contentHash).toBe(expectedHash) expect(cql.documents[0].contentHash).toBe(expectedHash) diff --git a/apps/sim/connectors/confluence/confluence.ts b/apps/sim/connectors/confluence/confluence.ts index 263d50c7db0..f0da25bdb44 100644 --- a/apps/sim/connectors/confluence/confluence.ts +++ b/apps/sim/connectors/confluence/confluence.ts @@ -1,5 +1,6 @@ import { createLogger } from '@sim/logger' import { getErrorMessage } from '@sim/utils/errors' +import { filterUndefined } from '@sim/utils/object' import * as cheerio from 'cheerio' import { AtlassianSiteNotAccessibleError, @@ -172,9 +173,37 @@ function extractBlockJoinedText($: cheerio.CheerioAPI, $el: cheerio.Cheerio return parts.join(' ').trim() } -/** Matches either flavor of panel/macro this function rewrites. */ +/** Matches either flavor of panel/macro {@link rewriteConfluenceCallouts} rewrites. */ const MACRO_SELECTOR = 'div.confluence-information-macro, div.panel' +/** + * Rendered-page elements whose text is never page prose. App macros render as a + * bootstrap `