From 04345785970640c225d6787e617863cc7d992ba0 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Wed, 26 Aug 2026 16:13:44 -0700 Subject: [PATCH 1/6] feat(desktop,cli): support self-hosted desktop installs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The desktop shell was already origin-agnostic at runtime — navigation, CSP, cookie partition, and the update feed all derive from the configured origin, and every deployment already serves /api/desktop/update/download and its updater manifest. The one thing missing was a way to change that origin: ConfigStore.setOrigin had no IPC channel, menu item, or UI behind it, so a self-hoster installing the signed build was stuck on the baked default. Adds the native server picker (Sim → Server…, plus a "Change server" button on the offline page, since a shell pointed at an unreachable origin lands there with nothing else to click). Its IPC family is gated to bundled file: senders: the surface that repoints the shell must keep working when the current server cannot be reached, and must never be drivable by a page that server serves. A confirmed change relaunches rather than swapping in place — the origin keys the cookie partition, update feed, encrypted per-origin task state, and every live browser view and PTY. Adds `sim-setup desktop`, which resolves the installer from the operator's own deployment, checks that the update feed resolves too, and prints the server URL to paste in. Documents the whole path under self-hosting, including the build-your-own escape hatch for organizations with their own Developer ID. --- apps/desktop/src/main/index.ts | 21 ++ apps/desktop/src/main/ipc.test.ts | 5 + apps/desktop/src/main/ipc.ts | 32 ++ apps/desktop/src/main/menu.test.ts | 2 + apps/desktop/src/main/menu.ts | 3 + apps/desktop/src/main/server-window.test.ts | 79 +++++ apps/desktop/src/main/server-window.ts | 140 ++++++++ apps/desktop/src/preload/index.ts | 11 + apps/desktop/static/offline.html | 5 + apps/desktop/static/server.html | 244 ++++++++++++++ .../docs/en/platform/self-hosting/desktop.mdx | 97 ++++++ .../docs/en/platform/self-hosting/meta.json | 1 + .../lib/copilot/generated/docs-manifest.ts | 1 + packages/desktop-bridge/contract-snapshot.ts | 300 +++++++++++++++--- packages/desktop-bridge/src/index.ts | 40 +++ packages/sim-setup/README.md | 16 + packages/sim-setup/src/arguments.test.ts | 24 ++ packages/sim-setup/src/arguments.ts | 21 ++ packages/sim-setup/src/desktop.test.ts | 96 ++++++ packages/sim-setup/src/desktop.ts | 206 ++++++++++++ packages/sim-setup/src/index.ts | 7 + 21 files changed, 1315 insertions(+), 36 deletions(-) create mode 100644 apps/desktop/src/main/server-window.test.ts create mode 100644 apps/desktop/src/main/server-window.ts create mode 100644 apps/desktop/static/server.html create mode 100644 apps/docs/content/docs/en/platform/self-hosting/desktop.mdx create mode 100644 packages/sim-setup/src/desktop.test.ts create mode 100644 packages/sim-setup/src/desktop.ts diff --git a/apps/desktop/src/main/index.ts b/apps/desktop/src/main/index.ts index cf82af3da67..6a634583b46 100644 --- a/apps/desktop/src/main/index.ts +++ b/apps/desktop/src/main/index.ts @@ -51,6 +51,7 @@ import { openExternalSafe } from '@/main/navigation' import { createEventLog } from '@/main/observability' import { ScopedEventRouter } from '@/main/scoped-event-router' import { installGlobalGuards } from '@/main/security-guards' +import { createServerWindow, relaunchApp } from '@/main/server-window' import { createSessionLifecycleCoordinator, decideStartRoute, @@ -473,6 +474,20 @@ function main(): void { }, }) + /** + * The native server picker. Self-hosted operators install the same signed + * build as everyone else and repoint it here — the bundle bakes only a + * DEFAULT origin, and every runtime guard reads the configured one. + */ + const serverWindow = createServerWindow({ + config, + defaultOrigin: DEFAULT_ORIGIN, + preloadPath, + isPackaged: app.isPackaged, + getParentWindow: getMainWindow, + relaunch: relaunchApp, + }) + /** * Routes through the coordinator rather than tearing down directly: the * coordinator holds the in-progress guard, clears the same handoff and grant @@ -659,6 +674,11 @@ function main(): void { check: () => updater?.check(), install: () => updater?.install(), }, + server: { + open: () => serverWindow.open(), + getConfiguration: () => serverWindow.getConfiguration(), + setOrigin: (origin) => serverWindow.setOrigin(origin), + }, }) await ensureMainWindow() installApplicationMenu({ @@ -666,6 +686,7 @@ function main(): void { getMainWindow, allowHttpLocalhost, openSettings, + openServerSettings: () => serverWindow.open(), newWindow: () => void createAndLoadAppWindow(), newChat: () => void openMainWindowAt(newChatRoute(config.get('lastRoute'))), handleFocusedResourceShortcut: (win, shortcut) => diff --git a/apps/desktop/src/main/ipc.test.ts b/apps/desktop/src/main/ipc.test.ts index c20e907e55d..5c08811376b 100644 --- a/apps/desktop/src/main/ipc.test.ts +++ b/apps/desktop/src/main/ipc.test.ts @@ -316,6 +316,11 @@ describe('registerIpcHandlers', () => { check: vi.fn(), install: vi.fn(), }, + server: { + open: vi.fn(), + getConfiguration: vi.fn(() => ({ origin: APP, defaultOrigin: APP })), + setOrigin: vi.fn(() => ({ ok: true as const, origin: APP, unchanged: true })), + }, } registerIpcHandlers(deps) }) diff --git a/apps/desktop/src/main/ipc.ts b/apps/desktop/src/main/ipc.ts index b230af7ad4d..6eb61333da0 100644 --- a/apps/desktop/src/main/ipc.ts +++ b/apps/desktop/src/main/ipc.ts @@ -9,6 +9,8 @@ import { } from '@sim/browser-protocol' import { type DesktopNotificationPayload, + type DesktopServerChangeResult, + type DesktopServerConfiguration, type DesktopUpdateState, type DesktopWindowState, type DesktopZoomPercent, @@ -301,6 +303,11 @@ export interface IpcDeps { check: () => void install: () => void } + server: { + open: () => void + getConfiguration: () => DesktopServerConfiguration + setOrigin: (origin: string) => DesktopServerChangeResult + } } /** @@ -1737,6 +1744,31 @@ export function registerIpcHandlers(deps: IpcDeps): void { passSender: true, handler: (sender) => deps.retryLoad(sender as WebContents), }, + // The `server:` family is local-page only, and deliberately so: the one + // surface that repoints the shell at another deployment must keep working + // when the current one is unreachable (the offline page is where a + // self-hoster with a typo'd origin actually lands), and must never be + // drivable by a page the current server serves. + 'server:open': { + kind: 'send', + gate: 'local-page', + handler: () => deps.server.open(), + }, + 'server:get-configuration': { + kind: 'invoke', + gate: 'local-page', + denied: null, + handler: () => deps.server.getConfiguration(), + }, + 'server:set-origin': { + kind: 'invoke', + gate: 'local-page', + denied: { ok: false, error: 'The server can only be changed from the Sim app itself.' }, + handler: (origin) => + typeof origin === 'string' + ? deps.server.setOrigin(origin) + : { ok: false, error: 'Server URL is required' }, + }, } const senderAllowed = (event: IpcMainEvent | IpcMainInvokeEvent, gate: ChannelGate): boolean => { diff --git a/apps/desktop/src/main/menu.test.ts b/apps/desktop/src/main/menu.test.ts index b98be440434..499cec7f523 100644 --- a/apps/desktop/src/main/menu.test.ts +++ b/apps/desktop/src/main/menu.test.ts @@ -18,6 +18,7 @@ function makeDeps(): MenuDeps { getMainWindow: vi.fn(() => null), allowHttpLocalhost: vi.fn(() => false), openSettings: vi.fn(), + openServerSettings: vi.fn(), newWindow: vi.fn(), newChat: vi.fn(), handleFocusedResourceShortcut: vi.fn(() => false), @@ -51,6 +52,7 @@ describe('buildMenuTemplate', () => { expect(submenu(template, 'Sim').map((item) => item.label ?? item.role ?? item.type)).toEqual([ 'about', 'Settings…', + 'Server…', 'Check for Updates…', 'Sign Out', 'separator', diff --git a/apps/desktop/src/main/menu.ts b/apps/desktop/src/main/menu.ts index d5594f2c5d1..8ee441a5d96 100644 --- a/apps/desktop/src/main/menu.ts +++ b/apps/desktop/src/main/menu.ts @@ -15,6 +15,8 @@ export interface MenuDeps { getMainWindow: () => BrowserWindow | null allowHttpLocalhost: () => boolean openSettings: () => void + /** Opens the native server picker (see main/server-window.ts). */ + openServerSettings: () => void newWindow: () => void newChat: () => void /** @@ -155,6 +157,7 @@ export function buildMenuTemplate(deps: MenuDeps): MenuItemConstructorOptions[] submenu: [ { role: 'about' }, { label: 'Settings…', accelerator: 'CmdOrCtrl+,', click: deps.openSettings }, + { label: 'Server…', click: deps.openServerSettings }, { label: 'Check for Updates…', click: deps.checkForUpdates }, { label: 'Sign Out', click: deps.signOut }, { type: 'separator' }, diff --git a/apps/desktop/src/main/server-window.test.ts b/apps/desktop/src/main/server-window.test.ts new file mode 100644 index 00000000000..16dedce1eb8 --- /dev/null +++ b/apps/desktop/src/main/server-window.test.ts @@ -0,0 +1,79 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' + +vi.mock('electron', () => import('@/test/electron-mock')) + +import type { ConfigStore, OriginValidation } from '@/main/config' +import { createServerWindow, type ServerWindowDeps } from '@/main/server-window' + +const CURRENT = 'https://sim.example.com' +const DEFAULT = 'https://www.sim.ai' + +function makeConfig(origin: string, validate: (raw: string) => OriginValidation): ConfigStore { + let stored = origin + return { + filePath: '/tmp/settings.json', + getOrigin: () => stored, + setOrigin: vi.fn((raw: string) => { + const result = validate(raw) + if (result.ok) stored = result.origin + return result + }), + get: vi.fn(() => undefined), + set: vi.fn(), + flush: vi.fn(), + } as unknown as ConfigStore +} + +function makeDeps(overrides: Partial = {}): ServerWindowDeps { + return { + config: makeConfig(CURRENT, (raw) => + raw.startsWith('https://') ? { ok: true, origin: raw } : { ok: false, error: 'bad origin' } + ), + defaultOrigin: DEFAULT, + preloadPath: '/tmp/preload.cjs', + isPackaged: false, + getParentWindow: () => null, + relaunch: vi.fn(), + ...overrides, + } +} + +describe('server window', () => { + let deps: ServerWindowDeps + + beforeEach(() => { + deps = makeDeps() + }) + + it('reports the configured origin alongside the build default', () => { + expect(createServerWindow(deps).getConfiguration()).toEqual({ + origin: CURRENT, + defaultOrigin: DEFAULT, + }) + }) + + it('relaunches after storing a different origin', () => { + const result = createServerWindow(deps).setOrigin('https://sim.other.example') + + expect(result).toEqual({ ok: true, origin: 'https://sim.other.example', unchanged: false }) + expect(deps.config.flush).toHaveBeenCalled() + expect(deps.relaunch).toHaveBeenCalledTimes(1) + }) + + // Re-confirming the URL already in the field is the most likely thing a user + // does in this window; restarting the app for it would be pure disruption. + it('does not relaunch when the origin is unchanged', () => { + const result = createServerWindow(deps).setOrigin(CURRENT) + + expect(result).toEqual({ ok: true, origin: CURRENT, unchanged: true }) + expect(deps.relaunch).not.toHaveBeenCalled() + }) + + it('surfaces a rejected origin without relaunching', () => { + const result = createServerWindow(deps).setOrigin('ftp://sim.example.com') + + expect(result).toEqual({ ok: false, error: 'bad origin' }) + expect(deps.relaunch).not.toHaveBeenCalled() + expect(deps.config.getOrigin()).toBe(CURRENT) + }) +}) diff --git a/apps/desktop/src/main/server-window.ts b/apps/desktop/src/main/server-window.ts new file mode 100644 index 00000000000..e3233effb9b --- /dev/null +++ b/apps/desktop/src/main/server-window.ts @@ -0,0 +1,140 @@ +import type { DesktopServerChangeResult, DesktopServerConfiguration } from '@sim/desktop-bridge' +import { createLogger } from '@sim/logger' +import { getErrorMessage } from '@sim/utils/errors' +import { app, BrowserWindow } from 'electron' +import type { ConfigStore } from '@/main/config' +import { createSecureWebPreferences } from '@/main/window' + +const logger = createLogger('DesktopServerWindow') + +/** The bundled local page, resolved the same way the offline page is. */ +const SERVER_PAGE = 'static/server.html' + +const WINDOW_WIDTH = 520 +const WINDOW_HEIGHT = 340 + +/** + * The partition the server-selection window runs in. + * + * Deliberately NOT the app session's partition. This window exists to move the + * shell between deployments, so binding it to the partition of the deployment + * being left would tie the escape hatch to the state it is escaping — and the + * page is a bundled `file:` document that stores nothing, so it has no reason + * to touch a persistent jar at all. + */ +const SERVER_WINDOW_PARTITION = 'server-selection' + +export interface ServerWindowDeps { + config: ConfigStore + defaultOrigin: string + preloadPath: string + isPackaged: boolean + getParentWindow: () => BrowserWindow | null + /** + * Relaunches the shell against the newly stored origin. A full restart + * rather than an in-place swap: the origin decides the cookie partition, the + * update feed, the encrypted per-origin task state, and the identity every + * live browser view and PTY was opened under, and there is no partial + * teardown of that set which is obviously correct. + */ + relaunch: () => void +} + +export interface ServerWindowHandle { + open(): void + getConfiguration(): DesktopServerConfiguration + setOrigin(origin: string): DesktopServerChangeResult + close(): void +} + +/** + * The native server picker: how a self-hosted operator points the shell at + * their own deployment. + * + * Native rather than a page in the web app, because the web app is served BY + * the origin being changed. Someone whose stored origin is unreachable — a + * typo, a VPN-only host, an instance that moved — can never reach an in-app + * settings route to fix it, which is exactly when they need this most. The + * same reasoning gates its IPC channels to bundled `file:` senders. + */ +export function createServerWindow(deps: ServerWindowDeps): ServerWindowHandle { + let win: BrowserWindow | null = null + + const getConfiguration = (): DesktopServerConfiguration => ({ + origin: deps.config.getOrigin(), + defaultOrigin: deps.defaultOrigin, + }) + + const close = (): void => { + if (win && !win.isDestroyed()) { + win.destroy() + } + win = null + } + + const open = (): void => { + if (win && !win.isDestroyed()) { + win.show() + win.focus() + return + } + const parent = deps.getParentWindow() + win = new BrowserWindow({ + width: WINDOW_WIDTH, + height: WINDOW_HEIGHT, + resizable: false, + minimizable: false, + maximizable: false, + fullscreenable: false, + title: 'Sim Server', + titleBarStyle: 'hiddenInset', + show: false, + // Modal only when there is a live parent to attach to. A shell whose + // window is gone (or never opened, because the origin failed to load) + // still has to be able to reach this. + ...(parent && !parent.isDestroyed() ? { parent, modal: true } : {}), + webPreferences: createSecureWebPreferences( + SERVER_WINDOW_PARTITION, + deps.preloadPath, + deps.isPackaged + ), + }) + win.once('ready-to-show', () => { + win?.show() + }) + win.on('closed', () => { + win = null + }) + void win.loadFile(SERVER_PAGE).catch((error) => { + logger.error('Could not open the server window', { error: getErrorMessage(error) }) + }) + } + + const setOrigin = (raw: string): DesktopServerChangeResult => { + const current = deps.config.getOrigin() + const validated = deps.config.setOrigin(raw) + if (!validated.ok) { + return validated + } + if (validated.origin === current) { + // Nothing moved, so nothing is torn down. Relaunching anyway would make + // "confirm the URL I already use" restart the app for no reason. + return { ok: true, origin: validated.origin, unchanged: true } + } + logger.info('Server origin changed; relaunching', { from: current, to: validated.origin }) + // setOrigin writes through immediately, but the rest of the settings file + // (window bounds, last route) is debounced — flush before the process goes. + deps.config.flush() + close() + deps.relaunch() + return { ok: true, origin: validated.origin, unchanged: false } + } + + return { open, getConfiguration, setOrigin, close } +} + +/** Restarts the process in place. Split out so tests can drive the seam. */ +export function relaunchApp(): void { + app.relaunch() + app.quit() +} diff --git a/apps/desktop/src/preload/index.ts b/apps/desktop/src/preload/index.ts index afe84f2a42e..07d4e9e59e4 100644 --- a/apps/desktop/src/preload/index.ts +++ b/apps/desktop/src/preload/index.ts @@ -33,6 +33,8 @@ import type { DesktopOAuthConnectScope, DesktopPreferenceKey, DesktopPreferences, + DesktopServerChangeResult, + DesktopServerConfiguration, DesktopUpdateState, DesktopWindowState, DesktopZoomPercent, @@ -124,6 +126,15 @@ const api: SimDesktopApi = { offlineRetry: (): void => { ipcRenderer.send('offline:retry') }, + server: { + open: (): void => { + ipcRenderer.send('server:open') + }, + getConfiguration: (): Promise => + ipcRenderer.invoke('server:get-configuration'), + setOrigin: (origin: string): Promise => + ipcRenderer.invoke('server:set-origin', origin), + }, localFilesystem: (request: LocalFilesystemRequest): Promise => ipcRenderer.invoke('desktop:local-filesystem', request), onCommand: (callback: (command: DesktopCommand) => void): (() => void) => { diff --git a/apps/desktop/static/offline.html b/apps/desktop/static/offline.html index 231c415a83e..98d084431e6 100644 --- a/apps/desktop/static/offline.html +++ b/apps/desktop/static/offline.html @@ -191,6 +191,7 @@

Can’t connect to Sim

+
@@ -231,6 +232,10 @@

Can’t connect to Sim

document .getElementById('status') .addEventListener('click', () => bridge?.openExternal('https://status.sim.ai')) + // The recovery path for a shell pointed at a server it cannot reach — + // a mistyped self-hosted origin strands the app here with nothing else + // to click. + document.getElementById('server').addEventListener('click', () => bridge?.server?.open()) diff --git a/apps/desktop/static/server.html b/apps/desktop/static/server.html new file mode 100644 index 00000000000..d7bdeb19d0f --- /dev/null +++ b/apps/desktop/static/server.html @@ -0,0 +1,244 @@ + + + + + + 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/docs/content/docs/en/platform/self-hosting/desktop.mdx b/apps/docs/content/docs/en/platform/self-hosting/desktop.mdx new file mode 100644 index 00000000000..ccc4f2e938f --- /dev/null +++ b/apps/docs/content/docs/en/platform/self-hosting/desktop.mdx @@ -0,0 +1,97 @@ +--- +title: Desktop App +description: Point the macOS desktop app at your own Sim deployment +--- + +import { Callout } from 'fumadocs-ui/components/callout' +import { Steps, Step } from 'fumadocs-ui/components/steps' + +The Sim desktop app is a native macOS shell around a Sim deployment. It is **not** tied to sim.ai — the build bakes in only a *default* server, and every runtime boundary (navigation, content security policy, cookie storage, the update feed) is derived from the server you point it at. + +So self-hosting the desktop app takes no build of your own: install the same signed, notarized app everyone else installs, then point it at your deployment. + + + The desktop app is macOS-only today. The web app works in any browser on any platform. + + +## Your deployment already serves the installer + +Every Sim deployment exposes two public endpoints: + +| Endpoint | What it does | +|---|---| +| `/api/desktop/update/download` | Redirects to the newest stable installer for your deployment's channel | +| `/api/desktop/update/latest-mac.yml` | The update manifest installed apps poll | + +Both resolve against public GitHub release assets. Nothing is built, signed, or hosted by you — your deployment only decides *which* release its clients are offered, and installed apps then update from your server instead of from sim.ai. + +The Sim server needs outbound access to `api.github.com` and `github.com` for these to resolve. Unauthenticated GitHub API requests are capped at 60/hour per IP; set `GITHUB_TOKEN` on the Sim server to raise that to 5000/hour. + +## Install and connect + + + + + +### Get the installer link + +```bash +npx sim-setup desktop +``` + +This reads your deployment URL from your configuration, checks that the installer and update feed both resolve, and prints the download link plus the server URL to enter. Pass `--url https://sim.example.com` when running the CLI somewhere that reaches Sim at a different address. + +Without the CLI, open `https://your-sim-url/api/desktop/update/download` in a browser. + + + + + +### Install it + +Open the `.dmg` and drag Sim to Applications. The build is signed and notarized by Sim, so Gatekeeper accepts it with no override. + + + + + +### Point it at your server + +Launch Sim, then choose **Sim → Server…** in the menu bar. Enter your deployment URL and press **Connect**. + +The app relaunches against your server and stays there — the setting persists across updates, and every later update is fetched from your deployment's feed. + + + + + + + Enter the origin your server actually **serves**, not one that redirects to it. If your load balancer redirects `sim.example.com` to `www.sim.example.com`, use the `www` form. The app compares origins exactly, so a redirecting origin leaves every page off-origin and strands sign-in. + + +## Requirements for the server URL + +- **HTTPS is required**, except for `localhost` and `127.0.0.1`, which may use HTTP for local testing. +- No credentials in the URL. +- Paths are ignored — only scheme, host, and port are stored. + +Each server gets its own isolated cookie and storage partition, so you can move between deployments without either one seeing the other's session. + +## Recovering from a wrong server URL + +If the app is pointed at a server it cannot reach, it shows its **Can't connect** page. That page has a **Change server** button that opens the same picker, so a typo is always recoverable without touching the filesystem. + +## Building your own shell + +You almost certainly do not need this. It is worth it only if you need your own bundle identity or your own signing identity — for example, to distribute through MDM under your organization's Developer ID. + +```bash +cd apps/desktop +SIM_DESKTOP_DEFAULT_ORIGIN=https://sim.example.com bun run package:share +``` + +This bakes your origin in as the default for fresh installs, so users never see the server picker. Artifacts land in `apps/desktop/release/`. + + + A build packaged this way is signed with whatever identity is on the build machine and is **not** notarized, so macOS quarantines it on download. Supply `CSC_LINK` and `CSC_KEY_PASSWORD` with your own Developer ID, and notarize it yourself, before distributing it. + diff --git a/apps/docs/content/docs/en/platform/self-hosting/meta.json b/apps/docs/content/docs/en/platform/self-hosting/meta.json index b2639411663..f1373a9682f 100644 --- a/apps/docs/content/docs/en/platform/self-hosting/meta.json +++ b/apps/docs/content/docs/en/platform/self-hosting/meta.json @@ -18,6 +18,7 @@ "networking", "security", "verify", + "desktop", "---Operate---", "observability", "scaling", diff --git a/apps/sim/lib/copilot/generated/docs-manifest.ts b/apps/sim/lib/copilot/generated/docs-manifest.ts index 4b9576c6d6d..3a1a57c8d0b 100644 --- a/apps/sim/lib/copilot/generated/docs-manifest.ts +++ b/apps/sim/lib/copilot/generated/docs-manifest.ts @@ -376,6 +376,7 @@ export const DOCS_MANIFEST: readonly string[] = [ 'platform/self-hosting/architecture.mdx', 'platform/self-hosting/authentication.mdx', 'platform/self-hosting/background-jobs.mdx', + 'platform/self-hosting/desktop.mdx', 'platform/self-hosting/docker.mdx', 'platform/self-hosting/email.mdx', 'platform/self-hosting/environment-variables.mdx', diff --git a/packages/desktop-bridge/contract-snapshot.ts b/packages/desktop-bridge/contract-snapshot.ts index 6008d32d719..429e6020a09 100644 --- a/packages/desktop-bridge/contract-snapshot.ts +++ b/packages/desktop-bridge/contract-snapshot.ts @@ -48,11 +48,14 @@ export const BROWSER_TOOL_NAMES = [ 'browser_screenshot', 'browser_extract', 'browser_click', + 'browser_click_at', 'browser_type', + 'browser_insert_text', 'browser_press_key', 'browser_scroll', 'browser_select_option', 'browser_hover', + 'browser_drag', 'browser_request_takeover', ] as const @@ -138,11 +141,20 @@ export interface BrowserPanelSnapshot { zoomPercent: number /** Chat scope that owns the captured tab. */ scopeId: string + /** + * Exact native-view rectangle in the Sim renderer's viewport CSS pixels. + * + * The native surface is integer-positioned in Electron DIP, while its React + * host can end on fractional CSS pixels. Rendering the replacement at this + * viewport rectangle avoids clipping or stretching it to the host box. + * Optional for compatibility with installed shells from before this field. + */ + viewportBounds?: BrowserPanelBounds } /** * Browser-chrome commands from the panel header (URL bar, back/forward, - * reload) plus `takeover-done`, sent by the Done chip on the chat's + * reload) plus `takeover-done`, sent by the question card on the chat's * `browser_request_takeover` tool row when the user finishes a * hand-control-back request. Page interactions need no protocol — the user * acts on the real embedded page directly, and its right-click menu is native @@ -167,6 +179,8 @@ export interface BrowserPanelAction { url?: string /** Stable tab id for `duplicate-tab`, `switch-tab`, and `close-tab`. */ tabId?: string + /** Optional free-text instruction submitted with `takeover-done`. */ + takeoverResponse?: string } /** Live state of the active page, pushed to the panel header. */ @@ -231,6 +245,12 @@ export interface BrowserTabState { export interface BrowserTabsState { tabs: BrowserTabState[] activeTabId: string | null + /** Tab currently driven by the agent when it differs from the user's visible tab. */ + automationTabId?: string | null + /** True while a browser tool is actively driving that tab. */ + automationActive?: boolean + /** True while automation is paused for the user on this tab. */ + automationNeedsAttention?: boolean /** Chat scope that owns this tab set. */ scopeId: string } @@ -294,6 +314,8 @@ export function isBrowserDataKind(value: unknown): value is BrowserDataKind { * the source of truth for how those calls travel to the desktop main process. */ +import { truncate } from '@sim/utils/string' + /** The single tool the model calls; what it does is in `operation`. */ export const TERMINAL_TOOL_NAME = 'terminal' @@ -567,6 +589,105 @@ export interface TerminalTabState { tmuxSession?: string | null } +/** Longest program name {@link describeRunningCommand} will return. */ +const MAX_RUNNING_COMMAND_LABEL = 32 + +/** + * Shell words that precede the program rather than being it, so a label reads + * `claude` and not `env` or `sudo`. + */ +const COMMAND_PREFIX_WORDS = new Set([ + 'command', + 'doas', + 'env', + 'exec', + 'nice', + 'nohup', + 'sudo', + 'time', +]) + +/** `NAME=value`, the other thing that can sit in front of the program. */ +const ENVIRONMENT_ASSIGNMENT = /^[A-Za-z_][A-Za-z0-9_]*=/ + +/** + * The last command in a shell line, ignoring separators inside quotes. A + * quote-blind split would cut `claude "a && b"` in half and report `b"` as the + * program. + */ +function lastCommandSegment(command: string): string { + let start = 0 + let quote: "'" | '"' | null = null + for (let index = 0; index < command.length; index++) { + const char = command[index] + if (quote) { + // Only double quotes honor backslash escapes; inside single quotes a + // backslash is a literal character and cannot hide the closing quote. + if (char === '\\' && quote === '"') index++ + else if (char === quote) quote = null + continue + } + if (char === "'" || char === '"') { + quote = char + continue + } + if (char === '\\') { + index++ + continue + } + if (char === ';' || char === '&' || char === '|') { + if (command[index + 1] === char) index++ + start = index + 1 + } + } + return command.slice(start).trim() +} + +/** + * A short name for whatever is holding a terminal's foreground. + * + * `running` is the literal line the shell was given, and an agent-launched one + * runs long: `cd && export PATH= && claude ""` + * is a single command several hundred characters wide. That is the right thing + * to hand an agent and the wrong thing to put in a sentence — a confirmation + * built around it stops being a question and becomes a wall of shell. This + * keeps the part a person recognizes, the program they are waiting on, and + * drops the environment preamble around it. + * + * Best-effort by construction: the input is a shell line, not a parsed argv, + * so a command this cannot read falls back to the line itself, bounded. Use it + * for prose about a terminal, never to decide anything. + * + * @example + * describeRunningCommand('cd /repo && export PATH=/bin && claude "fix it"') // 'claude' + * describeRunningCommand('sudo /usr/bin/docker compose up') // 'docker' + */ +export function describeRunningCommand(running: string): string { + const line = running.trim() + if (!line) return 'a command' + + const segment = lastCommandSegment(line) || line + const program = segment + .split(/\s+/) + .filter(Boolean) + .find( + (word) => + !ENVIRONMENT_ASSIGNMENT.test(word) && + !COMMAND_PREFIX_WORDS.has(word) && + !word.startsWith('-') + ) + + const label = + (program + ? (program + .replace(/^['"]|['"]$/g, '') + .split('/') + .filter(Boolean) + .pop() ?? '') + : '') || segment + return truncate(label, MAX_RUNNING_COMMAND_LABEL, '…') +} + /** One tmux pane, as reported by the `panes` operation. */ export interface TerminalPaneState { /** tmux target (`session:window.pane`), usable as the `pane` argument. */ @@ -607,6 +728,8 @@ export interface TerminalPanesResult { export interface TerminalTabsState { tabs: TerminalTabState[] activeTerminalId: string | null + /** Terminal currently driven by the agent when it differs from the user's visible terminal. */ + agentActiveTerminalId?: string | null } /** A tab strip crossing the desktop bridge, tagged with its owning chat. */ @@ -725,6 +848,12 @@ export interface SimDesktopTerminalApi { /** Open an additional terminal and make it active. */ openTerminal(cwd: string | undefined, scopeId: string): Promise switchTerminal(terminalId: string, scopeId: string): Promise + /** Move a terminal to its final position. Optional for older installed shells. */ + reorderTerminal?( + terminalId: string, + targetIndex: number, + scopeId: string + ): Promise closeTerminal(terminalId: string, scopeId: string): Promise getTabs(scopeId: string): Promise /** Makes a chat's terminal group the renderer-visible group. */ @@ -748,11 +877,12 @@ export interface SimDesktopTerminalApi { /** Forget retained output for one terminal. */ clearScrollback(terminalId: string, scopeId: string): Promise /** - * Reports whether the terminal panel owns keyboard focus, so global menu - * accelerators can tell a Cmd-W meant for a terminal from one meant for the - * window. + * Reports whether the visible terminal panel owns resource shortcuts, so a + * transient DOM blur cannot turn Cmd-W into a window-level command. */ setFocused(focused: boolean, scopeId: string): void + /** Reports whether this renderer is currently displaying the terminal resource. */ + setVisible?(visible: boolean, scopeId: string): void /** * The user finishing a handoff — the hand-back chip on the waiting tool row. */ @@ -778,6 +908,8 @@ export interface SimDesktopTerminalApi { * over the chat's browser panel so the user interacts with the real page. */ export interface SimDesktopBrowserAgentApi { + /** New shells can atomically force-hide a native page before renderer effects paint. */ + readonly supportsAtomicPanelOcclusion?: true /** * Execute one browser tool. Resolves with the tool's outcome; never * rejects for tool-level failures (those ride `ok: false`). @@ -788,8 +920,17 @@ export interface SimDesktopBrowserAgentApi { params: Record, scopeId: string ): Promise - /** Browser-chrome commands from the panel (URL bar, back, reload, takeover Done). */ + /** Cancel one exact in-flight tool. Optional for compatibility with older shells. */ + cancelTool?(toolCallId: string, scopeId: string): Promise + /** Cancel the currently active tool in a scope after renderer state was lost. */ + cancelActiveTool?(scopeId: string): Promise + /** Browser-chrome commands from the panel (URL bar, back, reload, takeover hand-back). */ panelAction(action: BrowserPanelAction, scopeId: string): void + /** + * Create and activate a blank tab, returning the authoritative list. + * Optional for compatibility with installed shells that predate acknowledged tab creation. + */ + openTab?(scopeId: string): Promise /** Makes a chat's browser tab set the renderer-visible set. */ activateScope(scopeId: string): Promise /** Materializes a lazily activated chat's persisted tabs without showing its panel. */ @@ -822,8 +963,8 @@ export interface SimDesktopBrowserAgentApi { ): void /** Capture the current page before opening renderer-owned UI above it. */ capturePanelSnapshot(scopeId: string): Promise - /** Hide/reveal the native page only after its replacement frame has painted. */ - setPanelOccluded(occluded: boolean, scopeId: string): Promise + /** Hide/reveal the native page after its replacement frame has painted. */ + setPanelOccluded(occluded: boolean, scopeId: string, force?: boolean): Promise /** Report whether renderer-owned browser chrome owns the user's interaction context. */ setPanelFocused(focused: boolean, scopeId: string): void /** Mirror Sim's light/dark/system preference into embedded pages. */ @@ -863,6 +1004,11 @@ export interface SimDesktopBrowserAgentApi { getTabsState(scopeId: string): Promise /** Read a privacy-preserving hint of websites that may have a usable session. */ getKnownSessions(): Promise + /** + * Live search completions for the omnibox. Optional while installed shells + * that predate search suggestions remain supported. + */ + getSearchSuggestions?(query: string): Promise /** * Erase browsing data from the dedicated profile and resolve the resulting * session list. Pass the kinds to clear; omit for all of them. Saved @@ -1261,17 +1407,25 @@ export interface DesktopOAuthConnectResult { ok: boolean /** OAuth error slug forwarded from the provider callback, when the flow failed. */ error?: string + /** + * Chat attempt correlated by the desktop handoff, or null for a non-chat + * connect. Absent only on older desktop shells that predate correlation. + */ + chatAttemptId?: string | null } /** * Optional scope for an OAuth connect handoff. Chip-initiated connects carry * the workspace (the browser flow creates the workspace connect draft * server-side) and, for reconnects, the credential to rebind. Modal-initiated - * connects omit both — the app already created the draft. + * connects carry the exact draft the app already created. */ export interface DesktopOAuthConnectScope { workspaceId?: string credentialId?: string + draftId?: string + /** Mothership credential-chip attempt to echo on desktop completion. */ + chatAttemptId?: string } export interface TerminalThemePalette { @@ -1371,7 +1525,15 @@ export interface TerminalSelectedProfile { id: string name: string source: TerminalThemeSource + /** + * Palette used when the source does not provide appearance-specific colors. + * Ignored once both `lightPalette` and `darkPalette` are present. + */ palette: TerminalThemePalette + /** Optional palette used while Sim is in light appearance. */ + lightPalette?: TerminalThemePalette + /** Optional palette used while Sim is in dark appearance. */ + darkPalette?: TerminalThemePalette } export type TerminalThemeProfile = TerminalSelectedProfile @@ -1384,41 +1546,60 @@ const TERMINAL_THEME_PALETTE_KEYS: readonly (keyof TerminalThemePalette)[] = [ ...TERMINAL_THEME_ANSI_KEYS, ] +const TERMINAL_THEME_OPTIONAL_PALETTE_KEYS = ['cursorAccent', 'selectionForeground'] as const + const TERMINAL_THEME_COLOR_PATTERN = /^#[0-9a-f]{6}$/i +function isTerminalThemeColor(value: unknown): value is string { + return typeof value === 'string' && TERMINAL_THEME_COLOR_PATTERN.test(value) +} + +function isTerminalThemePalette(value: unknown): value is TerminalThemePalette { + if (typeof value !== 'object' || value === null) return false + const palette = value as Partial + return ( + TERMINAL_THEME_PALETTE_KEYS.every((key) => isTerminalThemeColor(palette[key])) && + TERMINAL_THEME_OPTIONAL_PALETTE_KEYS.every( + (key) => palette[key] === undefined || isTerminalThemeColor(palette[key]) + ) + ) +} + export function isTerminalSelectedProfile(value: unknown): value is TerminalSelectedProfile { if (typeof value !== 'object' || value === null) return false const candidate = value as Partial - if ( - typeof candidate.id !== 'string' || - candidate.id.length === 0 || - candidate.id.length > 300 || - typeof candidate.name !== 'string' || - candidate.name.length === 0 || - candidate.name.length > 200 || - (candidate.source !== 'terminal' && candidate.source !== 'iterm2') || - typeof candidate.palette !== 'object' || - candidate.palette === null - ) { - return false - } - if ( - !TERMINAL_THEME_PALETTE_KEYS.every( - (key) => - typeof candidate.palette?.[key] === 'string' && - TERMINAL_THEME_COLOR_PATTERN.test(candidate.palette[key]) - ) - ) { - return false - } - return (['cursorAccent', 'selectionForeground'] as const).every( - (key) => - candidate.palette?.[key] === undefined || - (typeof candidate.palette[key] === 'string' && - TERMINAL_THEME_COLOR_PATTERN.test(candidate.palette[key])) + return ( + typeof candidate.id === 'string' && + candidate.id.length > 0 && + candidate.id.length <= 300 && + typeof candidate.name === 'string' && + candidate.name.length > 0 && + candidate.name.length <= 200 && + (candidate.source === 'terminal' || candidate.source === 'iterm2') && + isTerminalThemePalette(candidate.palette) && + (candidate.lightPalette === undefined || isTerminalThemePalette(candidate.lightPalette)) && + (candidate.darkPalette === undefined || isTerminalThemePalette(candidate.darkPalette)) ) } +/** + * Copies only the known profile fields, so untrusted source output and stored + * config never carry extra keys. The single definition of a profile's shape — + * new palette slots are added here rather than at each call site. + */ +export function cloneTerminalSelectedProfile( + profile: TerminalSelectedProfile +): TerminalSelectedProfile { + return { + id: profile.id, + name: profile.name, + source: profile.source, + palette: { ...profile.palette }, + ...(profile.lightPalette ? { lightPalette: { ...profile.lightPalette } } : {}), + ...(profile.darkPalette ? { darkPalette: { ...profile.darkPalette } } : {}), + } +} + export interface DesktopPreferences { notificationsEnabled: boolean notificationSounds: boolean @@ -1429,6 +1610,8 @@ export interface DesktopPreferences { trayEnabled: boolean /** Let Chat drive the built-in agent browser on this device. */ browserEnabled: boolean + /** Whether typing in the omnibox may request live Google search completions. */ + browserSearchSuggestionsEnabled?: boolean /** Let Chat run commands in local shells. */ terminalEnabled: boolean /** @@ -1513,6 +1696,11 @@ export interface SimDesktopSettingsApi { key: K, value: DesktopPreferences[K] ): Promise + /** + * Controls whether partial omnibox queries may be sent to Google. Optional + * for compatibility with installed shells that predate live suggestions. + */ + setBrowserSearchSuggestionsEnabled?(enabled: boolean): Promise notify(payload: DesktopNotificationPayload): Promise /** Overrides the appearance requested by browser pages. */ setBrowserTheme(theme: DesktopAppearanceTheme): Promise @@ -1574,7 +1762,7 @@ export interface SimDesktopUpdatesApi { onState(callback: (state: DesktopUpdateState) => void): () => void } -export type DesktopCommand = 'toggle-sidebar' +export type DesktopCommand = 'toggle-sidebar' | 'open-search' export interface DesktopWindowState { isFullScreen: boolean @@ -1585,6 +1773,41 @@ export interface SimDesktopWindowStateApi { onStateChange(callback: (state: DesktopWindowState) => void): () => void } +/** + * The Sim deployment an installed shell is pointed at. Self-hosted operators + * install the same signed build everyone else does and repoint it here — the + * bundle bakes only a DEFAULT origin, and every runtime guard (navigation, + * CSP, cookie partition, update feed) is derived from the configured one. + */ +export interface DesktopServerConfiguration { + /** The origin the shell is currently pointed at. */ + origin: string + /** The origin this build falls back to when nothing is stored. */ + defaultOrigin: string +} + +/** Outcome of a server change. On success the shell relaunches immediately. */ +export type DesktopServerChangeResult = + | { ok: true; origin: string; unchanged: boolean } + | { ok: false; error: string } + +/** + * Reading and changing the server origin. Exposed only to the shell's own + * bundled `file:` pages: the surface that changes which server the app talks + * to must stay reachable when that server cannot be reached at all, and must + * never be drivable by a page the current server serves. + */ +export interface SimDesktopServerApi { + /** Opens the shell's native server-selection window. */ + open(): void + getConfiguration(): Promise + /** + * Validates and persists a new server origin, then relaunches the shell. + * Resolves with an error message when the origin is rejected. + */ + setOrigin(origin: string): Promise +} + export interface SimDesktopApi { /** Installed shell version (plain semver, e.g. `0.3.1`). */ version: string @@ -1601,6 +1824,11 @@ export interface SimDesktopApi { */ onOAuthConnectComplete(callback: (result: DesktopOAuthConnectResult) => void): () => void offlineRetry(): void + /** + * Optional because shells older than this surface do not expose it. Only + * the shell's own bundled pages can call it — see {@link SimDesktopServerApi}. + */ + server?: SimDesktopServerApi localFilesystem(request: LocalFilesystemRequest): Promise /** Subscribe to commands initiated by the native application menu. */ onCommand(callback: (command: DesktopCommand) => void): () => void diff --git a/packages/desktop-bridge/src/index.ts b/packages/desktop-bridge/src/index.ts index 694256f7ade..96ecd0612da 100644 --- a/packages/desktop-bridge/src/index.ts +++ b/packages/desktop-bridge/src/index.ts @@ -1001,6 +1001,41 @@ export interface SimDesktopWindowStateApi { onStateChange(callback: (state: DesktopWindowState) => void): () => void } +/** + * The Sim deployment an installed shell is pointed at. Self-hosted operators + * install the same signed build everyone else does and repoint it here — the + * bundle bakes only a DEFAULT origin, and every runtime guard (navigation, + * CSP, cookie partition, update feed) is derived from the configured one. + */ +export interface DesktopServerConfiguration { + /** The origin the shell is currently pointed at. */ + origin: string + /** The origin this build falls back to when nothing is stored. */ + defaultOrigin: string +} + +/** Outcome of a server change. On success the shell relaunches immediately. */ +export type DesktopServerChangeResult = + | { ok: true; origin: string; unchanged: boolean } + | { ok: false; error: string } + +/** + * Reading and changing the server origin. Exposed only to the shell's own + * bundled `file:` pages: the surface that changes which server the app talks + * to must stay reachable when that server cannot be reached at all, and must + * never be drivable by a page the current server serves. + */ +export interface SimDesktopServerApi { + /** Opens the shell's native server-selection window. */ + open(): void + getConfiguration(): Promise + /** + * Validates and persists a new server origin, then relaunches the shell. + * Resolves with an error message when the origin is rejected. + */ + setOrigin(origin: string): Promise +} + export interface SimDesktopApi { /** Installed shell version (plain semver, e.g. `0.3.1`). */ version: string @@ -1017,6 +1052,11 @@ export interface SimDesktopApi { */ onOAuthConnectComplete(callback: (result: DesktopOAuthConnectResult) => void): () => void offlineRetry(): void + /** + * Optional because shells older than this surface do not expose it. Only + * the shell's own bundled pages can call it — see {@link SimDesktopServerApi}. + */ + server?: SimDesktopServerApi localFilesystem(request: LocalFilesystemRequest): Promise /** Subscribe to commands initiated by the native application menu. */ onCommand(callback: (command: DesktopCommand) => void): () => void diff --git a/packages/sim-setup/README.md b/packages/sim-setup/README.md index d21e8555ede..944115c75cc 100644 --- a/packages/sim-setup/README.md +++ b/packages/sim-setup/README.md @@ -153,6 +153,22 @@ npx sim-setup doctor --json Use `config` to answer “what is configured?” and `status` to answer “what is running and healthy?” +### Install the desktop app + +`desktop` resolves the macOS installer from your own deployment and prints the +server URL to enter in the app: + +```bash +npx sim-setup desktop +npx sim-setup desktop --url https://sim.example.com +npx sim-setup desktop --no-open +``` + +The desktop app is not tied to sim.ai — it bakes in only a default server, and +every Sim deployment already serves `/api/desktop/update/download` and the +update feed installed apps poll. Install the signed build, then point it at your +deployment with **Sim → Server…**. Nothing has to be built or signed by you. + ### Add or change capabilities Configure one capability without walking through the complete wizard: diff --git a/packages/sim-setup/src/arguments.test.ts b/packages/sim-setup/src/arguments.test.ts index e0630b62434..fb809190b6f 100644 --- a/packages/sim-setup/src/arguments.test.ts +++ b/packages/sim-setup/src/arguments.test.ts @@ -42,6 +42,30 @@ describe('parseSetupArguments', () => { ) }) + it('parses desktop options', () => { + expect(parseSetupArguments(['desktop'])).toEqual({ kind: 'desktop', noOpen: false }) + expect(parseSetupArguments(['desktop', '--no-open'])).toEqual({ + kind: 'desktop', + noOpen: true, + }) + expect(parseSetupArguments(['desktop', '--url', 'https://sim.example.com'])).toEqual({ + kind: 'desktop', + noOpen: false, + url: 'https://sim.example.com', + }) + expect(parseSetupArguments(['desktop', '--url=https://sim.example.com'])).toEqual({ + kind: 'desktop', + noOpen: false, + url: 'https://sim.example.com', + }) + expect(() => parseSetupArguments(['desktop', '--url'])).toThrow( + '--url requires a deployment URL' + ) + expect(() => parseSetupArguments(['desktop', '--nope'])).toThrow( + 'Unknown desktop option: --nope' + ) + }) + it('validates add operands before loading configuration', () => { expect(parseSetupArguments(['add', 'email'])).toEqual({ kind: 'add', diff --git a/packages/sim-setup/src/arguments.ts b/packages/sim-setup/src/arguments.ts index 65a9f2accd2..0bebf57403e 100644 --- a/packages/sim-setup/src/arguments.ts +++ b/packages/sim-setup/src/arguments.ts @@ -20,6 +20,7 @@ export type SetupInvocation = | { kind: 'config' } | { kind: 'add'; feature: string; args: string[] } | { kind: 'doctor'; fix: boolean; json: boolean } + | { kind: 'desktop'; url?: string; noOpen: boolean } | { kind: 'lifecycle'; command: LifecycleCommand } export class SetupArgumentError extends Error { @@ -136,6 +137,26 @@ function parseCore( return { kind: 'add', feature, args: featureArgs } } + if (command === 'desktop') { + const noOpen = oneFlag(commandArgs, '--no-open') + let url: string | undefined + const remaining: string[] = [] + for (let index = 0; index < commandArgs.length; index += 1) { + const arg = commandArgs[index] + if (arg === '--no-open') continue + if (arg !== '--url' && !arg.startsWith('--url=')) { + remaining.push(arg) + continue + } + if (url) fail('--url may only be provided once') + const value = arg === '--url' ? commandArgs[++index] : arg.slice('--url='.length) + if (!value || value.startsWith('-')) fail('--url requires a deployment URL') + url = value + } + if (remaining.length > 0) fail(`Unknown desktop option: ${remaining[0]}`) + return { kind: 'desktop', noOpen, ...(url ? { url } : {}) } + } + if (command === 'doctor') { const fix = oneFlag(commandArgs, '--fix') const json = oneFlag(commandArgs, '--json') diff --git a/packages/sim-setup/src/desktop.test.ts b/packages/sim-setup/src/desktop.test.ts new file mode 100644 index 00000000000..804a224781b --- /dev/null +++ b/packages/sim-setup/src/desktop.test.ts @@ -0,0 +1,96 @@ +import { describe, expect, it, vi } from 'vitest' +import { probeDownload, probeFeed, resolveDeploymentUrl } from './desktop' +import { SetupError } from './errors' + +const ASSET = 'https://github.com/simstudioai/sim/releases/download/v1.2.3/Sim-1.2.3-universal.dmg' + +function source(appUrl?: string) { + return { values: appUrl ? new Map([['NEXT_PUBLIC_APP_URL', appUrl]]) : new Map() } +} + +function respond(status: number, headers: Record = {}): typeof fetch { + return vi.fn(async () => new Response(null, { status, headers })) as unknown as typeof fetch +} + +describe('resolveDeploymentUrl', () => { + it('reads the deployment origin from the discovered configuration', () => { + expect(resolveDeploymentUrl([source(), source('https://sim.example.com')])).toBe( + 'https://sim.example.com' + ) + }) + + it('strips any path so the API paths append cleanly', () => { + expect(resolveDeploymentUrl([source('https://sim.example.com/workspace/')])).toBe( + 'https://sim.example.com' + ) + }) + + it('prefers an explicit override over the configured value', () => { + expect(resolveDeploymentUrl([source('https://sim.example.com')], 'https://other.example')).toBe( + 'https://other.example' + ) + }) + + // Compose supplies this via `${VAR:-default}` interpolation, so an absent + // key means "the wizard default", not a broken install. + it('falls back to the local wizard origin when nothing is configured', () => { + expect(resolveDeploymentUrl([source()])).toBe('http://localhost:3000') + }) + + it('rejects a value that is not an http(s) URL', () => { + expect(() => resolveDeploymentUrl([source('sim.example.com')])).toThrow(SetupError) + expect(() => resolveDeploymentUrl([source('ftp://sim.example.com')])).toThrow(SetupError) + }) +}) + +describe('probeDownload', () => { + it('reports the artifact the deployment redirects to', async () => { + const result = await probeDownload( + 'https://sim.example.com/api/desktop/update/download', + respond(302, { location: ASSET }) + ) + + expect(result).toEqual({ + status: 'ok', + installerUrl: ASSET, + installerName: 'Sim-1.2.3-universal.dmg', + }) + }) + + it('distinguishes no-release from a broken release feed', async () => { + expect(await probeDownload('https://sim.example.com/x', respond(404))).toEqual({ + status: 'no-release', + }) + expect(await probeDownload('https://sim.example.com/x', respond(502))).toEqual({ + status: 'feed-unavailable', + }) + }) + + it('reports an unreachable deployment rather than throwing', async () => { + const failing = vi.fn(async () => { + throw new Error('connect ECONNREFUSED') + }) as unknown as typeof fetch + + expect(await probeDownload('https://sim.example.com/x', failing)).toEqual({ + status: 'unreachable', + error: 'connect ECONNREFUSED', + }) + }) + + it('does not follow the redirect', async () => { + const impl = respond(302, { location: ASSET }) + await probeDownload('https://sim.example.com/x', impl) + + expect(impl).toHaveBeenCalledWith( + 'https://sim.example.com/x', + expect.objectContaining({ redirect: 'manual' }) + ) + }) +}) + +describe('probeFeed', () => { + it('is true only when the manifest resolves', async () => { + expect(await probeFeed('https://sim.example.com/f', respond(200))).toBe(true) + expect(await probeFeed('https://sim.example.com/f', respond(404))).toBe(false) + }) +}) diff --git a/packages/sim-setup/src/desktop.ts b/packages/sim-setup/src/desktop.ts new file mode 100644 index 00000000000..80ea139570f --- /dev/null +++ b/packages/sim-setup/src/desktop.ts @@ -0,0 +1,206 @@ +import { spawnSync } from 'node:child_process' +import { getErrorMessage } from '@sim/utils/errors' +import { discoverConfigurationSources } from './configuration-sources' +import { SetupError } from './errors' +import * as p from './prompter' +import { glyph, theme } from './theme' +import { APP_URL } from './urls' + +/** + * Where a deployment redirects to the newest installer for its own channel, + * and the manifest installed shells poll. Both ship in every Sim deployment + * (`app/api/desktop/update/*`), so a self-hosted install already serves them — + * there is nothing to build or host. + */ +const DOWNLOAD_PATH = '/api/desktop/update/download' +const FEED_PATH = '/api/desktop/update/latest-mac.yml' + +const PROBE_TIMEOUT_MS = 15_000 + +/** The env var every deployment sets to its own public origin. */ +const APP_URL_KEY = 'NEXT_PUBLIC_APP_URL' + +export interface DesktopFlags { + /** Overrides the deployment origin when the CLI runs away from the install. */ + url?: string + /** Skips opening the installer in a browser. */ + noOpen: boolean +} + +/** + * The deployment origin the desktop app should be pointed at. + * + * Read from every discovered source, not only the one `add` may write: an + * operator running a Helm release or an external Compose project still needs + * the URL, and reading it changes nothing. + */ +export function resolveDeploymentUrl( + sources: readonly { values?: Map | null }[], + override?: string +): string { + const raw = + override ?? sources.find((source) => source.values?.get(APP_URL_KEY))?.values?.get(APP_URL_KEY) + if (!raw) { + // A wizard-provisioned local install has the compose interpolation default + // rather than an explicit value, so an absent key is not a misconfiguration. + return APP_URL + } + let url: URL + try { + url = new URL(raw.trim()) + } catch { + throw new SetupError(`${APP_URL_KEY} is not a valid URL: ${raw}`, [ + 'Set it to the origin browsers use to reach Sim, e.g. https://sim.example.com', + ]) + } + if (url.protocol !== 'https:' && url.protocol !== 'http:') { + throw new SetupError(`${APP_URL_KEY} must be an http(s) URL: ${raw}`) + } + return url.origin +} + +export type DesktopProbe = + | { status: 'ok'; installerUrl: string; installerName: string } + | { status: 'no-release' } + | { status: 'feed-unavailable' } + | { status: 'unreachable'; error: string } + | { status: 'unexpected'; code: number } + +/** + * Asks the deployment to resolve its own installer, following no redirects: + * the 302's Location IS the answer, and downloading the artifact here would + * pull hundreds of megabytes to check a link. + */ +export async function probeDownload( + downloadUrl: string, + fetchImpl: typeof fetch = fetch +): Promise { + let response: Response + try { + response = await fetchImpl(downloadUrl, { + redirect: 'manual', + signal: AbortSignal.timeout(PROBE_TIMEOUT_MS), + }) + } catch (error) { + return { status: 'unreachable', error: getErrorMessage(error, 'request failed') } + } + if (response.status === 302 || response.status === 301 || response.status === 307) { + const location = response.headers.get('location') + if (!location) return { status: 'unexpected', code: response.status } + let name = location + try { + name = decodeURIComponent(new URL(location).pathname.split('/').pop() ?? location) + } catch { + // Keep the raw Location; it is still the most useful thing to print. + } + return { status: 'ok', installerUrl: location, installerName: name } + } + if (response.status === 404) return { status: 'no-release' } + if (response.status === 502) return { status: 'feed-unavailable' } + return { status: 'unexpected', code: response.status } +} + +/** Whether the update feed the installed app polls resolves on this deployment. */ +export async function probeFeed( + feedUrl: string, + fetchImpl: typeof fetch = fetch +): Promise { + try { + const response = await fetchImpl(feedUrl, { signal: AbortSignal.timeout(PROBE_TIMEOUT_MS) }) + return response.ok + } catch { + return false + } +} + +function describeProbe(probe: DesktopProbe, appUrl: string): string { + switch (probe.status) { + case 'ok': + return `${glyph.pass} Installer resolved: ${probe.installerName}` + case 'no-release': + return `${glyph.fail} This deployment reports no desktop release for its channel.` + case 'feed-unavailable': + return `${glyph.fail} The deployment could not reach the GitHub release feed.` + case 'unreachable': + return `${glyph.fail} Could not reach ${appUrl} — ${probe.error}` + case 'unexpected': + return `${glyph.fail} The download endpoint answered ${probe.code}.` + } +} + +function probeHints(probe: DesktopProbe, appUrl: string): string[] { + switch (probe.status) { + case 'no-release': + return [ + 'Stable desktop builds are published on GitHub releases of simstudioai/sim.', + 'A brand-new fork with no releases of its own will report this.', + ] + case 'feed-unavailable': + return [ + 'The Sim server needs outbound access to api.github.com and github.com.', + 'Unauthenticated GitHub API calls are capped at 60/hour per IP — set GITHUB_TOKEN on the Sim server to raise it to 5000/hour.', + ] + case 'unreachable': + return [ + `Check that Sim is running and reachable at ${appUrl} (npx sim-setup status).`, + `Pass --url if this machine reaches Sim at a different address.`, + ] + default: + return [] + } +} + +export async function runDesktop(flags: DesktopFlags): Promise { + const appUrl = resolveDeploymentUrl(discoverConfigurationSources(), flags.url) + const downloadUrl = `${appUrl}${DOWNLOAD_PATH}` + + p.log.step(`Deployment: ${theme.accent(appUrl)}`) + + const spin = p.spinner() + spin.start('Resolving the desktop installer…') + const [probe, feedOk] = await Promise.all([ + probeDownload(downloadUrl), + probeFeed(`${appUrl}${FEED_PATH}`), + ]) + spin.stop(describeProbe(probe, appUrl)) + + if (probe.status !== 'ok') { + for (const hint of probeHints(probe, appUrl)) { + p.log.info(hint) + } + p.outro(theme.error('The desktop installer could not be resolved.')) + return 1 + } + + if (!feedOk) { + p.log.warn( + `${FEED_PATH} did not resolve — the app will install but will not auto-update from this deployment.` + ) + } + + p.note( + [ + `1. Download and install Sim:`, + ` ${theme.accent(downloadUrl)}`, + '', + `2. Open Sim, then choose ${theme.command('Sim → Server…')} in the menu bar.`, + '', + `3. Enter your server URL and press Connect:`, + ` ${theme.accent(appUrl)}`, + '', + theme.muted('Sim relaunches against your deployment and updates from it from then on.'), + theme.muted('The desktop app is macOS-only today; the web app works everywhere.'), + ].join('\n'), + 'Connect the desktop app' + ) + + if (!flags.noOpen && process.platform === 'darwin') { + const open = await p.confirm({ message: 'Download it now?', initialValue: true }) + if (open) { + spawnSync('open', [downloadUrl], { stdio: 'ignore' }) + } + } + + p.outro(theme.accent('Ready.')) + return 0 +} diff --git a/packages/sim-setup/src/index.ts b/packages/sim-setup/src/index.ts index 31f63354b14..6177b110e16 100644 --- a/packages/sim-setup/src/index.ts +++ b/packages/sim-setup/src/index.ts @@ -13,6 +13,7 @@ const USAGE = `Usage: sim-setup [--quick] [--dir ] [--mode compose|dev|k8s] sim-setup config show configured capabilities and integrations sim-setup add configure ${SETUP_FEATURES} + sim-setup desktop [--url ] install the macOS desktop app against this deployment sim-setup doctor [--fix] [--json] check your setup sim-setup start | stop | restart bring your install up / down / cycle sim-setup update pull/rebuild and apply Compose images @@ -47,6 +48,12 @@ async function main(): Promise { return } + if (invocation.kind === 'desktop') { + const { runDesktop } = await import('./desktop') + process.exitCode = await runDesktop({ url: invocation.url, noOpen: invocation.noOpen }) + return + } + if (invocation.kind === 'doctor') { const { runDoctor } = await import('./doctor') process.exitCode = await runDoctor({ fix: invocation.fix, json: invocation.json }) From 76e697983f612c419961e1f6e438faca9d91a6e5 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Wed, 26 Aug 2026 16:41:48 -0700 Subject: [PATCH 2/6] refactor(desktop,cli): review pass on self-hosted desktop support MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two real defects found while auditing the change for hardcoded assumptions. `lastRoute` is a single global setting that carries a workspace id, so it survived an origin change and opened /workspace/ on the new server. resolveStartRoute cannot rescue that — it discards a route only on a confirmed 403, and a fresh partition draws a 401. Cleared on change, via a named list that is now the documented home for deployment-scoped settings; the agent browser's jar and its known-sites metadata are deliberately kept, together, since changing deployments does not imply the account changed. The offline page's "Check status" sent self-hosters to status.sim.ai, which reports on Sim's deployments and is always green for theirs. Withheld for a non-sim.ai origin, as is the same link in the Help menu, through one isSimCloudOrigin predicate. Hiding it needed `button[hidden]{display:none}`: the page's own `button{display:inline-flex}` is an author rule and outranks the UA `[hidden]`, so the attribute alone left it rendering. The e2e offline test now asserts the whole path, which covers the `server:` local-page IPC gate. Review cleanups: setOrigin no longer rewrites settings when handed the origin it already stores; the picker window installs a permission handler and pre- paints its background like every other window, and its page is theme-aware so that background is not a flash; the CLI reuses httpHealth and the cross-platform openBrowser instead of reimplementing both, skips Compose/Helm discovery when --url makes it dead, and folds two parallel switches into one exhaustive one. Value-flag parsing is now one helper instead of a third copy. --- apps/desktop/e2e/smoke.spec.ts | 10 +- apps/desktop/src/main/config.test.ts | 40 +++++++ apps/desktop/src/main/config.ts | 22 ++++ apps/desktop/src/main/index.ts | 7 +- apps/desktop/src/main/ipc.test.ts | 2 +- apps/desktop/src/main/menu.test.ts | 11 +- apps/desktop/src/main/menu.ts | 16 ++- apps/desktop/src/main/server-window.test.ts | 30 ++++- apps/desktop/src/main/server-window.ts | 83 ++++++++++---- apps/desktop/static/offline.html | 23 +++- apps/desktop/static/server.html | 29 +++-- .../docs/en/platform/self-hosting/desktop.mdx | 14 ++- packages/desktop-bridge/contract-snapshot.ts | 13 ++- packages/desktop-bridge/src/index.ts | 13 ++- packages/sim-setup/src/arguments.ts | 56 +++++---- packages/sim-setup/src/cli-auth.ts | 2 +- packages/sim-setup/src/desktop.test.ts | 35 +++++- packages/sim-setup/src/desktop.ts | 107 +++++++++--------- 18 files changed, 365 insertions(+), 148 deletions(-) diff --git a/apps/desktop/e2e/smoke.spec.ts b/apps/desktop/e2e/smoke.spec.ts index 6e1277dbeb6..4bc71a1ee87 100644 --- a/apps/desktop/e2e/smoke.spec.ts +++ b/apps/desktop/e2e/smoke.spec.ts @@ -113,7 +113,15 @@ test.describe('desktop shell smoke', () => { await expect(window.locator('.wordmark')).toBeVisible() await expect(window.locator('.wordmark')).toHaveAttribute('aria-label', 'Sim') await expect(window.locator('#title')).toHaveText('Can’t connect to Sim') - await expect(window.locator('#status')).toHaveText('Check status') + // 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 + // `server:` local-page IPC gate: the bundled page reads the configuration + // over the real preload bridge, and status.sim.ai is withheld because this + // origin is not one of Sim's own. `toBeHidden` is load-bearing — the page's + // own `button { display: inline-flex }` outranks the UA `[hidden]` rule, so + // the attribute alone does not hide it. + await expect(window.locator('#server')).toBeVisible() + await expect(window.locator('#status')).toBeHidden() await expect .poll(() => window.evaluate(() => document.fonts.check('16px "Season Sans"'))) .toBe(true) diff --git a/apps/desktop/src/main/config.test.ts b/apps/desktop/src/main/config.test.ts index fefba510536..ed60924a261 100644 --- a/apps/desktop/src/main/config.test.ts +++ b/apps/desktop/src/main/config.test.ts @@ -9,6 +9,7 @@ import { createConfigStore, DEFAULT_ORIGIN, isSafeInternalPath, + isSimCloudOrigin, partitionForOrigin, validateOriginInput, } from '@/main/config' @@ -145,6 +146,26 @@ describe('createConfigStore', () => { expect(reloaded.getOrigin()).toBe('https://self-hosted.example') }) + // setOrigin writes the whole settings file synchronously on the main thread, + // and re-confirming the URL already in the field is the common case in the + // server picker. + it('does not rewrite settings when setOrigin is given the stored origin', () => { + const filePath = tempSettingsPath() + const store = createConfigStore(filePath, {}) + store.setOrigin('https://self-hosted.example') + // A sentinel only this test could have written. A rewrite serializes the + // in-memory settings over it, so its survival proves no write happened — + // unlike an mtime comparison, which two writes a fraction of a millisecond + // apart can pass by accident. + writeFileSync(filePath, `${readFileSync(filePath, 'utf8')}\n// sentinel\n`) + + expect(store.setOrigin('https://self-hosted.example')).toEqual({ + ok: true, + origin: 'https://self-hosted.example', + }) + expect(readFileSync(filePath, 'utf8')).toContain('// sentinel') + }) + it('canonicalizes the apex production origin on setOrigin, not just on load', () => { // Entering https://sim.ai mid-session must not persist the apex: the // running session would use the wrong cookie partition and misclassify @@ -223,6 +244,25 @@ describe('createConfigStore', () => { }) }) +describe('isSimCloudOrigin', () => { + it('recognizes Sim-operated origins and nothing else', () => { + for (const origin of ['https://sim.ai', 'https://www.sim.ai', 'https://www.staging.sim.ai']) { + expect(isSimCloudOrigin(origin)).toBe(true) + } + // A lookalike host must not pass — the suffix check is on the parsed + // hostname, never a prefix or substring of the raw string. + for (const origin of [ + 'https://sim.example.com', + 'https://sim.ai.evil.example', + 'https://notsim.ai', + 'http://localhost:3000', + 'not a url', + ]) { + expect(isSimCloudOrigin(origin)).toBe(false) + } + }) +}) + describe('channelForOrigin', () => { it('maps each environment origin to its channel', () => { expect(channelForOrigin('https://sim.ai')).toBe('prod') diff --git a/apps/desktop/src/main/config.ts b/apps/desktop/src/main/config.ts index 654fbacfa35..14ce4b00f8d 100644 --- a/apps/desktop/src/main/config.ts +++ b/apps/desktop/src/main/config.ts @@ -179,6 +179,22 @@ export function canonicalOrigin(origin: string): string { return ORIGIN_REWRITES[origin] ?? origin } +/** + * Whether an origin is one of Sim's own deployments rather than a self-hosted + * one. Sim-operated resources — the public status page above all — describe + * only these, so a shell pointed elsewhere must not be offered them: telling a + * self-hoster whose server is down to consult a page that is always green + * sends the person who most needs an answer to the one place that has none. + */ +export function isSimCloudOrigin(origin: string): boolean { + try { + const host = new URL(origin).hostname.toLowerCase() + return host === 'sim.ai' || host.endsWith('.sim.ai') + } catch { + return false + } +} + /** * Maps a server origin to its cookie/storage partition. Each origin gets an * isolated persistent partition so sessions never leak across instances. @@ -339,6 +355,12 @@ export function createConfigStore( // only repairs it on the next launch. The canonical origin is also // returned so the caller sees what was actually stored. const origin = canonicalOrigin(validated.origin) + // Re-confirming the origin already stored is the common case in the + // server picker, and setOrigin's write is a synchronous mkdir + whole-file + // write + rename on the main thread. There is nothing to persist. + if (origin === settings.origin) { + return { ok: true, origin } + } settings.origin = origin // Not debounced: changing the origin tears the session down and // reloads, so a pending write could be lost on the way out — and this diff --git a/apps/desktop/src/main/index.ts b/apps/desktop/src/main/index.ts index 6a634583b46..7d7cf0cc738 100644 --- a/apps/desktop/src/main/index.ts +++ b/apps/desktop/src/main/index.ts @@ -79,6 +79,7 @@ function reportHandoffFailure(error: unknown): void { } const OFFLINE_PAGE = 'static/offline.html' +const SERVER_PAGE = 'static/server.html' const DOCK_ICON_FOR_CHANNEL = { prod: 'dock-icon.png', staging: 'dock-icon-staging.png', @@ -474,14 +475,10 @@ function main(): void { }, }) - /** - * The native server picker. Self-hosted operators install the same signed - * build as everyone else and repoint it here — the bundle bakes only a - * DEFAULT origin, and every runtime guard reads the configured one. - */ const serverWindow = createServerWindow({ config, defaultOrigin: DEFAULT_ORIGIN, + pagePath: SERVER_PAGE, preloadPath, isPackaged: app.isPackaged, getParentWindow: getMainWindow, diff --git a/apps/desktop/src/main/ipc.test.ts b/apps/desktop/src/main/ipc.test.ts index 5c08811376b..5034ea9dc58 100644 --- a/apps/desktop/src/main/ipc.test.ts +++ b/apps/desktop/src/main/ipc.test.ts @@ -318,7 +318,7 @@ describe('registerIpcHandlers', () => { }, server: { open: vi.fn(), - getConfiguration: vi.fn(() => ({ origin: APP, defaultOrigin: APP })), + getConfiguration: vi.fn(() => ({ origin: APP, defaultOrigin: APP, isSimCloud: true })), setOrigin: vi.fn(() => ({ ok: true as const, origin: APP, unchanged: true })), }, } diff --git a/apps/desktop/src/main/menu.test.ts b/apps/desktop/src/main/menu.test.ts index 499cec7f523..63dbc05357d 100644 --- a/apps/desktop/src/main/menu.test.ts +++ b/apps/desktop/src/main/menu.test.ts @@ -6,11 +6,11 @@ import { BrowserWindow, type MenuItemConstructorOptions } from 'electron' import type { ConfigStore } from '@/main/config' import { buildMenuTemplate, type MenuDeps } from '@/main/menu' -function makeDeps(): MenuDeps { +function makeDeps(origin = 'https://sim.ai'): MenuDeps { return { config: { filePath: '/tmp/settings.json', - getOrigin: vi.fn(() => 'https://sim.ai'), + getOrigin: vi.fn(() => origin), setOrigin: vi.fn(), get: vi.fn(() => undefined), set: vi.fn(), @@ -107,6 +107,13 @@ describe('buildMenuTemplate', () => { expect(help.map((item) => item.label)).toEqual(['Sim Documentation', 'Sim Status']) }) + // status.sim.ai reports on Sim's deployments only, so it is worse than + // useless to an operator whose own server is the one that is down. + it('drops Sim status for a self-hosted server', () => { + const help = submenu(buildMenuTemplate(makeDeps('https://sim.example.com')), 'Help') + expect(help.map((item) => item.label)).toEqual(['Sim Documentation']) + }) + it('never exposes developer tools in the application menu', () => { const view = submenu(buildMenuTemplate(makeDeps()), 'View') expect(view.some((item) => item.role === 'toggleDevTools')).toBe(false) diff --git a/apps/desktop/src/main/menu.ts b/apps/desktop/src/main/menu.ts index 8ee441a5d96..4223e8a8b7b 100644 --- a/apps/desktop/src/main/menu.ts +++ b/apps/desktop/src/main/menu.ts @@ -1,6 +1,6 @@ import type { MenuItemConstructorOptions } from 'electron' import { app, BrowserWindow, Menu } from 'electron' -import type { ConfigStore } from '@/main/config' +import { type ConfigStore, isSimCloudOrigin } from '@/main/config' import { DOCS_URL, STATUS_URL } from '@/main/external-links' import { openExternalSafe } from '@/main/navigation' import type { @@ -253,10 +253,16 @@ export function buildMenuTemplate(deps: MenuDeps): MenuItemConstructorOptions[] label: 'Sim Documentation', click: () => void openExternalSafe(DOCS_URL, deps.allowHttpLocalhost()), }, - { - label: 'Sim Status', - click: () => void openExternalSafe(STATUS_URL, deps.allowHttpLocalhost()), - }, + // Omitted for a self-hosted shell, like the offline page's status + // button — see isSimCloudOrigin. + ...(isSimCloudOrigin(deps.config.getOrigin()) + ? [ + { + label: 'Sim Status', + click: () => void openExternalSafe(STATUS_URL, deps.allowHttpLocalhost()), + }, + ] + : []), ], }, ] diff --git a/apps/desktop/src/main/server-window.test.ts b/apps/desktop/src/main/server-window.test.ts index 16dedce1eb8..eed80063cd9 100644 --- a/apps/desktop/src/main/server-window.test.ts +++ b/apps/desktop/src/main/server-window.test.ts @@ -30,6 +30,7 @@ function makeDeps(overrides: Partial = {}): ServerWindowDeps { raw.startsWith('https://') ? { ok: true, origin: raw } : { ok: false, error: 'bad origin' } ), defaultOrigin: DEFAULT, + pagePath: 'static/server.html', preloadPath: '/tmp/preload.cjs', isPackaged: false, getParentWindow: () => null, @@ -49,9 +50,20 @@ describe('server window', () => { expect(createServerWindow(deps).getConfiguration()).toEqual({ origin: CURRENT, defaultOrigin: DEFAULT, + isSimCloud: false, }) }) + // Drives whether the offline page offers Sim's status page, which describes + // only Sim's own deployments. + it('marks a sim.ai origin as Sim cloud', () => { + const cloud = makeDeps({ + config: makeConfig('https://www.sim.ai', (raw) => ({ ok: true, origin: raw })), + }) + + expect(createServerWindow(cloud).getConfiguration().isSimCloud).toBe(true) + }) + it('relaunches after storing a different origin', () => { const result = createServerWindow(deps).setOrigin('https://sim.other.example') @@ -60,8 +72,22 @@ describe('server window', () => { expect(deps.relaunch).toHaveBeenCalledTimes(1) }) - // Re-confirming the URL already in the field is the most likely thing a user - // does in this window; restarting the app for it would be pure disruption. + // The saved route carries the previous deployment's workspace id, and + // resolveStartRoute only discards a route on a confirmed 403 — a fresh + // partition answers 401, so a kept route would survive onto the new server. + it('drops the saved route when the origin changes', () => { + createServerWindow(deps).setOrigin('https://sim.other.example') + + expect(deps.config.set).toHaveBeenCalledWith('lastRoute', undefined) + }) + + it('keeps the saved route when the origin is unchanged', () => { + createServerWindow(deps).setOrigin(CURRENT) + + expect(deps.config.set).not.toHaveBeenCalled() + }) + + // Re-confirming the pre-filled URL is the common case here. it('does not relaunch when the origin is unchanged', () => { const result = createServerWindow(deps).setOrigin(CURRENT) diff --git a/apps/desktop/src/main/server-window.ts b/apps/desktop/src/main/server-window.ts index e3233effb9b..f0c08d658c0 100644 --- a/apps/desktop/src/main/server-window.ts +++ b/apps/desktop/src/main/server-window.ts @@ -1,15 +1,17 @@ import type { DesktopServerChangeResult, DesktopServerConfiguration } from '@sim/desktop-bridge' import { createLogger } from '@sim/logger' import { getErrorMessage } from '@sim/utils/errors' -import { app, BrowserWindow } from 'electron' -import type { ConfigStore } from '@/main/config' -import { createSecureWebPreferences } from '@/main/window' +import { app, BrowserWindow, nativeTheme, session } from 'electron' +import type { ConfigStore, DesktopSettings } from '@/main/config' +import { isSimCloudOrigin } from '@/main/config' +import { + backgroundColorFor, + createSecureWebPreferences, + setupPermissionHandlers, +} from '@/main/window' const logger = createLogger('DesktopServerWindow') -/** The bundled local page, resolved the same way the offline page is. */ -const SERVER_PAGE = 'static/server.html' - const WINDOW_WIDTH = 520 const WINDOW_HEIGHT = 340 @@ -24,18 +26,45 @@ const WINDOW_HEIGHT = 340 */ const SERVER_WINDOW_PARTITION = 'server-selection' +/** + * Settings that describe the deployment rather than the device, and so must not + * survive a move to a different one. The home for this rule: `DesktopSettings` + * is a single global record with no per-origin namespace, so anything added + * there that names a Sim resource belongs in this list. + * + * `lastRoute` carries a workspace id in its path, so keeping it would open + * `/workspace/` on the new server. `resolveStartRoute` cannot rescue + * that: it discards a route only on a confirmed 403, and a fresh partition has + * no session, so the new server answers 401 and the stale route survives the + * probe. + * + * The agent browser is deliberately untouched — both its cookie jar and the + * `browserKnownSites` inference metadata that describes it. Sign-out clears the + * pair because the ACCOUNT changed; pointing the shell at another deployment + * does not imply that, and signing the operator out of every unrelated site in + * the built-in browser is not a reasonable side effect of correcting a server + * URL. They are kept together on purpose: clearing the metadata alone would + * leave Sim blind to sign-ins that are still live in the profile. + */ +const ORIGIN_SCOPED_SETTINGS: readonly (keyof DesktopSettings)[] = ['lastRoute'] + export interface ServerWindowDeps { config: ConfigStore defaultOrigin: string + /** The bundled page to load, resolved by the caller like the offline page. */ + pagePath: string preloadPath: string isPackaged: boolean getParentWindow: () => BrowserWindow | null /** - * Relaunches the shell against the newly stored origin. A full restart - * rather than an in-place swap: the origin decides the cookie partition, the - * update feed, the encrypted per-origin task state, and the identity every - * live browser view and PTY was opened under, and there is no partial - * teardown of that set which is obviously correct. + * Relaunches the shell against the newly stored origin. A full restart rather + * than an in-place swap: the origin decides the cookie partition, the update + * feed, the encrypted per-origin task state, and the identity every live + * browser view and PTY was opened under. Nothing in the app exposes a reset + * for that set — `ensureAppSession` and the partition cache are one-way + * memoizations, and the sign-out coordinator revokes server-side, which is + * wrong here (the old server's session should stay valid). The quit path + * already performs the orderly teardown, so relaunching reuses it. */ relaunch: () => void } @@ -44,7 +73,6 @@ export interface ServerWindowHandle { open(): void getConfiguration(): DesktopServerConfiguration setOrigin(origin: string): DesktopServerChangeResult - close(): void } /** @@ -60,10 +88,10 @@ export interface ServerWindowHandle { export function createServerWindow(deps: ServerWindowDeps): ServerWindowHandle { let win: BrowserWindow | null = null - const getConfiguration = (): DesktopServerConfiguration => ({ - origin: deps.config.getOrigin(), - defaultOrigin: deps.defaultOrigin, - }) + const getConfiguration = (): DesktopServerConfiguration => { + const origin = deps.config.getOrigin() + return { origin, defaultOrigin: deps.defaultOrigin, isSimCloud: isSimCloudOrigin(origin) } + } const close = (): void => { if (win && !win.isDestroyed()) { @@ -79,6 +107,11 @@ export function createServerWindow(deps: ServerWindowDeps): ServerWindowHandle { return } const parent = deps.getParentWindow() + // Every other session in the app installs a permission handler; without one + // Electron decides for itself what a page may ask the OS for. The page here + // asks for nothing, and a foreign origin can never load in this window, so + // the shared handler resolves to a deny-all — which is the intent. + setupPermissionHandlers(session.fromPartition(SERVER_WINDOW_PARTITION), deps.config.getOrigin) win = new BrowserWindow({ width: WINDOW_WIDTH, height: WINDOW_HEIGHT, @@ -89,6 +122,12 @@ export function createServerWindow(deps: ServerWindowDeps): ServerWindowHandle { title: 'Sim Server', titleBarStyle: 'hiddenInset', 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. + // This window loads a bundled page that follows `prefers-color-scheme`, + // so honouring the stored web-app theme here would pre-paint dark behind + // a page about to render light whenever the two disagree. + backgroundColor: backgroundColorFor(undefined, nativeTheme.shouldUseDarkColors), // Modal only when there is a live parent to attach to. A shell whose // window is gone (or never opened, because the origin failed to load) // still has to be able to reach this. @@ -105,7 +144,7 @@ export function createServerWindow(deps: ServerWindowDeps): ServerWindowHandle { win.on('closed', () => { win = null }) - void win.loadFile(SERVER_PAGE).catch((error) => { + void win.loadFile(deps.pagePath).catch((error) => { logger.error('Could not open the server window', { error: getErrorMessage(error) }) }) } @@ -122,15 +161,19 @@ export function createServerWindow(deps: ServerWindowDeps): ServerWindowHandle { return { ok: true, origin: validated.origin, unchanged: true } } logger.info('Server origin changed; relaunching', { from: current, to: validated.origin }) - // setOrigin writes through immediately, but the rest of the settings file - // (window bounds, last route) is debounced — flush before the process goes. + for (const key of ORIGIN_SCOPED_SETTINGS) { + deps.config.set(key, undefined) + } + // setOrigin writes through immediately; the clears above are debounced like + // every other setting. `before-quit` flushes too, but doing it here keeps + // the write independent of the Electron quit sequence. deps.config.flush() close() deps.relaunch() return { ok: true, origin: validated.origin, unchanged: false } } - return { open, getConfiguration, setOrigin, close } + return { open, getConfiguration, setOrigin } } /** Restarts the process in place. Split out so tests can drive the seam. */ diff --git a/apps/desktop/static/offline.html b/apps/desktop/static/offline.html index 98d084431e6..20d82e4bab9 100644 --- a/apps/desktop/static/offline.html +++ b/apps/desktop/static/offline.html @@ -126,6 +126,12 @@ stroke 150ms cubic-bezier(0.4, 0, 0.2, 1); -webkit-app-region: no-drag; } + /* `button { display: inline-flex }` is an author rule, so it beats the UA + stylesheet's `[hidden] { display: none }` no matter the specificity — + without this the hidden status button renders anyway. */ + button[hidden] { + display: none; + } button .label { min-width: 0; flex: 1; @@ -190,7 +196,7 @@

Can’t connect to Sim

- +
@@ -228,14 +234,23 @@

Can’t connect to Sim

if (detail) document.getElementById('detail').textContent = detail const bridge = window.simDesktop + const statusButton = document.getElementById('status') document.getElementById('retry').addEventListener('click', () => bridge?.offlineRetry()) - document - .getElementById('status') - .addEventListener('click', () => bridge?.openExternal('https://status.sim.ai')) + statusButton.addEventListener('click', () => bridge?.openExternal('https://status.sim.ai')) // The recovery path for a shell pointed at a server it cannot reach — // a mistyped self-hosted origin strands the app here with nothing else // to click. document.getElementById('server').addEventListener('click', () => bridge?.server?.open()) + + // Hidden in the markup and only ever revealed, never the reverse: an + // older shell with no `server` bridge, or a configuration read that + // fails, must not leave a status link a self-hoster cannot use. + bridge?.server + ?.getConfiguration() + .then(({ isSimCloud }) => { + if (isSimCloud) statusButton.hidden = false + }) + .catch(() => {}) diff --git a/apps/desktop/static/server.html b/apps/desktop/static/server.html index d7bdeb19d0f..49b105ede12 100644 --- a/apps/desktop/static/server.html +++ b/apps/desktop/static/server.html @@ -17,8 +17,11 @@ font-weight: 300 800; font-display: block; } + /* The window pre-paints backgroundColorFor(...) before this document + renders, so --bg must track the same system preference or a dark-mode + operator sees the picker flash from dark to white on open. */ :root { - color-scheme: light; + color-scheme: light dark; --bg: #fefefe; --text-primary: #1a1a1a; --text-body: #434343; @@ -29,6 +32,19 @@ --border-strong: #b4b4b4; --error: #b42318; } + @media (prefers-color-scheme: dark) { + :root { + --bg: #0c0c0c; + --text-primary: #fafafa; + --text-body: #d4d4d4; + --text-muted: #8a8a8a; + --text-inverse: #0c0c0c; + --surface-hover: #1f1f1f; + --border: #2a2a2a; + --border-strong: #4a4a4a; + --error: #f97066; + } + } * { box-sizing: border-box; } @@ -181,8 +197,6 @@

Sim server

const cancel = document.getElementById('cancel') const message = document.getElementById('message') - let currentOrigin = '' - function setMessage(text, tone) { message.textContent = text || '' if (tone) message.setAttribute('data-tone', tone) @@ -229,13 +243,12 @@

Sim server

bridge ?.getConfiguration() - .then((configuration) => { - currentOrigin = configuration.origin - input.value = currentOrigin + .then(({ origin, defaultOrigin }) => { + input.value = origin input.select() syncConnectEnabled() - if (currentOrigin !== configuration.defaultOrigin) { - setMessage(`This build defaults to ${configuration.defaultOrigin}`) + if (origin !== defaultOrigin) { + setMessage(`This build defaults to ${defaultOrigin}`) } }) .catch(() => setMessage('Could not read the current server.', 'error')) diff --git a/apps/docs/content/docs/en/platform/self-hosting/desktop.mdx b/apps/docs/content/docs/en/platform/self-hosting/desktop.mdx index ccc4f2e938f..2b7cfb11b80 100644 --- a/apps/docs/content/docs/en/platform/self-hosting/desktop.mdx +++ b/apps/docs/content/docs/en/platform/self-hosting/desktop.mdx @@ -4,7 +4,7 @@ description: Point the macOS desktop app at your own Sim deployment --- import { Callout } from 'fumadocs-ui/components/callout' -import { Steps, Step } from 'fumadocs-ui/components/steps' +import { Step, Steps } from 'fumadocs-ui/components/steps' The Sim desktop app is a native macOS shell around a Sim deployment. It is **not** tied to sim.ai — the build bakes in only a *default* server, and every runtime boundary (navigation, content security policy, cookie storage, the update feed) is derived from the server you point it at. @@ -20,10 +20,10 @@ Every Sim deployment exposes two public endpoints: | Endpoint | What it does | |---|---| -| `/api/desktop/update/download` | Redirects to the newest stable installer for your deployment's channel | +| `/api/desktop/update/download` | Redirects to the newest installer for this deployment's release channel — stable, for a self-hosted install | | `/api/desktop/update/latest-mac.yml` | The update manifest installed apps poll | -Both resolve against public GitHub release assets. Nothing is built, signed, or hosted by you — your deployment only decides *which* release its clients are offered, and installed apps then update from your server instead of from sim.ai. +Both resolve against Sim's public GitHub releases, and the installers themselves are downloaded from GitHub. Nothing is built, signed, or hosted by you: your deployment decides *which* release its clients are offered and serves the manifest, so installed apps poll your server instead of sim.ai — but they cannot be served artifacts of your own from this path. To ship your own build, see [Building your own shell](#building-your-own-shell). The Sim server needs outbound access to `api.github.com` and `github.com` for these to resolve. Unauthenticated GitHub API requests are capped at 60/hour per IP; set `GITHUB_TOKEN` on the Sim server to raise that to 5000/hour. @@ -61,6 +61,8 @@ Launch Sim, then choose **Sim → Server…** in the menu bar. Enter your deploy The app relaunches against your server and stays there — the setting persists across updates, and every later update is fetched from your deployment's feed. +Each server keeps its own session, so you sign in again on the new one, and the relaunched app opens on the workspace picker rather than whatever workspace the old server had open. Everything else — window size, zoom, notification preferences, and the built-in browser's own profile — is device state and is kept. + @@ -71,7 +73,7 @@ The app relaunches against your server and stays there — the setting persists ## Requirements for the server URL -- **HTTPS is required**, except for `localhost` and `127.0.0.1`, which may use HTTP for local testing. +- **HTTPS is required**, except for the loopback hosts `localhost`, `127.0.0.1`, and `::1`, which may use HTTP for local testing. - No credentials in the URL. - Paths are ignored — only scheme, host, and port are stored. @@ -90,8 +92,8 @@ cd apps/desktop SIM_DESKTOP_DEFAULT_ORIGIN=https://sim.example.com bun run package:share ``` -This bakes your origin in as the default for fresh installs, so users never see the server picker. Artifacts land in `apps/desktop/release/`. +This bakes your origin in as the default for fresh installs, so users never see the server picker. Artifacts land in `apps/desktop/release/sim/`, named `sim--.dmg`. - A build packaged this way is signed with whatever identity is on the build machine and is **not** notarized, so macOS quarantines it on download. Supply `CSC_LINK` and `CSC_KEY_PASSWORD` with your own Developer ID, and notarize it yourself, before distributing it. + A build packaged this way is signed with whatever identity is on the build machine, and without App Store Connect credentials it is **not** notarized — so macOS quarantines it on download. Before distributing it, supply your own Developer ID via `CSC_LINK` and `CSC_KEY_PASSWORD`, and notarization credentials via `APPLE_API_KEY`, `APPLE_API_KEY_ID`, `APPLE_API_ISSUER`, and `APPLE_TEAM_ID`. diff --git a/packages/desktop-bridge/contract-snapshot.ts b/packages/desktop-bridge/contract-snapshot.ts index 429e6020a09..886076a0397 100644 --- a/packages/desktop-bridge/contract-snapshot.ts +++ b/packages/desktop-bridge/contract-snapshot.ts @@ -1774,16 +1774,21 @@ export interface SimDesktopWindowStateApi { } /** - * The Sim deployment an installed shell is pointed at. Self-hosted operators - * install the same signed build everyone else does and repoint it here — the - * bundle bakes only a DEFAULT origin, and every runtime guard (navigation, - * CSP, cookie partition, update feed) is derived from the configured one. + * The Sim deployment an installed shell is pointed at. The bundle bakes only a + * DEFAULT origin; navigation, CSP, cookie partition, and the update feed are + * all derived from the configured one. */ export interface DesktopServerConfiguration { /** The origin the shell is currently pointed at. */ origin: string /** The origin this build falls back to when nothing is stored. */ defaultOrigin: string + /** + * Whether the configured origin is one of Sim's own deployments. Sim-operated + * resources (the public status page) describe only those, so a self-hosted + * shell must not be pointed at them. + */ + isSimCloud: boolean } /** Outcome of a server change. On success the shell relaunches immediately. */ diff --git a/packages/desktop-bridge/src/index.ts b/packages/desktop-bridge/src/index.ts index 96ecd0612da..36b3f433c03 100644 --- a/packages/desktop-bridge/src/index.ts +++ b/packages/desktop-bridge/src/index.ts @@ -1002,16 +1002,21 @@ export interface SimDesktopWindowStateApi { } /** - * The Sim deployment an installed shell is pointed at. Self-hosted operators - * install the same signed build everyone else does and repoint it here — the - * bundle bakes only a DEFAULT origin, and every runtime guard (navigation, - * CSP, cookie partition, update feed) is derived from the configured one. + * The Sim deployment an installed shell is pointed at. The bundle bakes only a + * DEFAULT origin; navigation, CSP, cookie partition, and the update feed are + * all derived from the configured one. */ export interface DesktopServerConfiguration { /** The origin the shell is currently pointed at. */ origin: string /** The origin this build falls back to when nothing is stored. */ defaultOrigin: string + /** + * Whether the configured origin is one of Sim's own deployments. Sim-operated + * resources (the public status page) describe only those, so a self-hosted + * shell must not be pointed at them. + */ + isSimCloud: boolean } /** Outcome of a server change. On success the shell relaunches immediately. */ diff --git a/packages/sim-setup/src/arguments.ts b/packages/sim-setup/src/arguments.ts index 0bebf57403e..cc08714b045 100644 --- a/packages/sim-setup/src/arguments.ts +++ b/packages/sim-setup/src/arguments.ts @@ -69,26 +69,43 @@ function oneFlag(args: readonly string[], flag: string): boolean { return count === 1 } -function parseMode(args: readonly string[]): { mode?: WizardMode; remaining: string[] } { - let mode: WizardMode | undefined +/** + * Pulls one `--flag value` / `--flag=value` option out of an argument list, + * returning it alongside everything the caller still has to account for. + * Accepts both spellings, rejects a repeat, and rejects a missing or + * flag-shaped value. + */ +function parseValueOption( + args: readonly string[], + flag: string, + requirement: string +): { value?: string; remaining: string[] } { + let value: string | undefined const remaining: string[] = [] + const prefix = `${flag}=` for (let index = 0; index < args.length; index += 1) { const arg = args[index] - if (arg !== '--mode' && !arg.startsWith('--mode=')) { + if (arg !== flag && !arg.startsWith(prefix)) { remaining.push(arg) continue } - if (mode) fail('--mode may only be provided once') + if (value !== undefined) fail(`${flag} may only be provided once`) - const value = arg === '--mode' ? args[++index] : arg.slice('--mode='.length) - if (value !== 'compose' && value !== 'dev' && value !== 'k8s') { - fail(`invalid --mode "${value ?? ''}" — expected compose, dev, or k8s`) - } - mode = value + const candidate = arg === flag ? args[++index] : arg.slice(prefix.length) + if (!candidate || candidate.startsWith('-')) fail(requirement) + value = candidate } - return { mode, remaining } + return { value, remaining } +} + +function parseMode(args: readonly string[]): { mode?: WizardMode; remaining: string[] } { + const { value, remaining } = parseValueOption(args, '--mode', '--mode requires a value') + if (value !== undefined && value !== 'compose' && value !== 'dev' && value !== 'k8s') { + fail(`invalid --mode "${value}" — expected compose, dev, or k8s`) + } + return { mode: value as WizardMode | undefined, remaining } } function expectNoArguments(command: string, args: readonly string[]): void { @@ -139,20 +156,11 @@ function parseCore( if (command === 'desktop') { const noOpen = oneFlag(commandArgs, '--no-open') - let url: string | undefined - const remaining: string[] = [] - for (let index = 0; index < commandArgs.length; index += 1) { - const arg = commandArgs[index] - if (arg === '--no-open') continue - if (arg !== '--url' && !arg.startsWith('--url=')) { - remaining.push(arg) - continue - } - if (url) fail('--url may only be provided once') - const value = arg === '--url' ? commandArgs[++index] : arg.slice('--url='.length) - if (!value || value.startsWith('-')) fail('--url requires a deployment URL') - url = value - } + const { value: url, remaining } = parseValueOption( + commandArgs.filter((arg) => arg !== '--no-open'), + '--url', + '--url requires a deployment URL' + ) if (remaining.length > 0) fail(`Unknown desktop option: ${remaining[0]}`) return { kind: 'desktop', noOpen, ...(url ? { url } : {}) } } diff --git a/packages/sim-setup/src/cli-auth.ts b/packages/sim-setup/src/cli-auth.ts index a8f230c3335..6d00dca4137 100644 --- a/packages/sim-setup/src/cli-auth.ts +++ b/packages/sim-setup/src/cli-auth.ts @@ -23,7 +23,7 @@ export type AuthPollResult = | { status: 'complete'; apiKey: string } /** Opens a safely encoded URL across platforms, including Windows' cmd-backed `start`. */ -function openBrowser(url: string): void { +export function openBrowser(url: string): void { if (process.env.SIM_SETUP_NO_BROWSER) return if (process.platform === 'win32') { spawnSync('cmd', ['/c', 'start', '""', `"${url}"`], { diff --git a/packages/sim-setup/src/desktop.test.ts b/packages/sim-setup/src/desktop.test.ts index 804a224781b..78768e13a67 100644 --- a/packages/sim-setup/src/desktop.test.ts +++ b/packages/sim-setup/src/desktop.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it, vi } from 'vitest' -import { probeDownload, probeFeed, resolveDeploymentUrl } from './desktop' +import { describeProbe, probeDownload, resolveDeploymentUrl } from './desktop' import { SetupError } from './errors' const ASSET = 'https://github.com/simstudioai/sim/releases/download/v1.2.3/Sim-1.2.3-universal.dmg' @@ -57,6 +57,14 @@ describe('probeDownload', () => { }) }) + it('accepts every redirect status the endpoint may answer with', async () => { + for (const status of [301, 302, 307, 308]) { + expect( + await probeDownload('https://sim.example.com/x', respond(status, { location: ASSET })) + ).toMatchObject({ status: 'ok' }) + } + }) + it('distinguishes no-release from a broken release feed', async () => { expect(await probeDownload('https://sim.example.com/x', respond(404))).toEqual({ status: 'no-release', @@ -88,9 +96,26 @@ describe('probeDownload', () => { }) }) -describe('probeFeed', () => { - it('is true only when the manifest resolves', async () => { - expect(await probeFeed('https://sim.example.com/f', respond(200))).toBe(true) - expect(await probeFeed('https://sim.example.com/f', respond(404))).toBe(false) +describe('describeProbe', () => { + // Failure statuses are the ones an operator has to act on, so each must + // arrive with something to try. + it('gives every failure status actionable hints', () => { + for (const probe of [ + { status: 'no-release' }, + { status: 'feed-unavailable' }, + { status: 'unreachable', error: 'boom' }, + ] as const) { + expect(describeProbe(probe, 'https://sim.example.com').hints.length).toBeGreaterThan(0) + } + }) + + it('names the resolved artifact on success and asks for nothing', () => { + const described = describeProbe( + { status: 'ok', installerUrl: ASSET, installerName: 'Sim-1.2.3-universal.dmg' }, + 'https://sim.example.com' + ) + + expect(described.headline).toContain('Sim-1.2.3-universal.dmg') + expect(described.hints).toEqual([]) }) }) diff --git a/packages/sim-setup/src/desktop.ts b/packages/sim-setup/src/desktop.ts index 80ea139570f..e1a257e7c04 100644 --- a/packages/sim-setup/src/desktop.ts +++ b/packages/sim-setup/src/desktop.ts @@ -1,7 +1,8 @@ -import { spawnSync } from 'node:child_process' import { getErrorMessage } from '@sim/utils/errors' +import { openBrowser } from './cli-auth' import { discoverConfigurationSources } from './configuration-sources' import { SetupError } from './errors' +import { httpHealth } from './probes' import * as p from './prompter' import { glyph, theme } from './theme' import { APP_URL } from './urls' @@ -17,13 +18,14 @@ const FEED_PATH = '/api/desktop/update/latest-mac.yml' const PROBE_TIMEOUT_MS = 15_000 +const REDIRECT_STATUSES: ReadonlySet = new Set([301, 302, 307, 308]) + /** The env var every deployment sets to its own public origin. */ const APP_URL_KEY = 'NEXT_PUBLIC_APP_URL' export interface DesktopFlags { /** Overrides the deployment origin when the CLI runs away from the install. */ url?: string - /** Skips opening the installer in a browser. */ noOpen: boolean } @@ -38,8 +40,7 @@ export function resolveDeploymentUrl( sources: readonly { values?: Map | null }[], override?: string ): string { - const raw = - override ?? sources.find((source) => source.values?.get(APP_URL_KEY))?.values?.get(APP_URL_KEY) + const raw = override ?? sources.map((source) => source.values?.get(APP_URL_KEY)).find(Boolean) if (!raw) { // A wizard-provisioned local install has the compose interpolation default // rather than an explicit value, so an absent key is not a misconfiguration. @@ -68,8 +69,8 @@ export type DesktopProbe = /** * Asks the deployment to resolve its own installer, following no redirects: - * the 302's Location IS the answer, and downloading the artifact here would - * pull hundreds of megabytes to check a link. + * the redirect's Location IS the answer, and downloading the artifact here + * would pull hundreds of megabytes to check a link. */ export async function probeDownload( downloadUrl: string, @@ -84,7 +85,7 @@ export async function probeDownload( } catch (error) { return { status: 'unreachable', error: getErrorMessage(error, 'request failed') } } - if (response.status === 302 || response.status === 301 || response.status === 307) { + if (REDIRECT_STATUSES.has(response.status)) { const location = response.headers.get('location') if (!location) return { status: 'unexpected', code: response.status } let name = location @@ -100,58 +101,52 @@ export async function probeDownload( return { status: 'unexpected', code: response.status } } -/** Whether the update feed the installed app polls resolves on this deployment. */ -export async function probeFeed( - feedUrl: string, - fetchImpl: typeof fetch = fetch -): Promise { - try { - const response = await fetchImpl(feedUrl, { signal: AbortSignal.timeout(PROBE_TIMEOUT_MS) }) - return response.ok - } catch { - return false - } -} - -function describeProbe(probe: DesktopProbe, appUrl: string): string { +/** + * One exhaustive switch for both the headline and its follow-up hints, so a + * new {@link DesktopProbe} status cannot compile with its hints silently + * missing — which a `default` arm would have allowed. + */ +export function describeProbe( + probe: DesktopProbe, + appUrl: string +): { headline: string; hints: readonly string[] } { switch (probe.status) { case 'ok': - return `${glyph.pass} Installer resolved: ${probe.installerName}` + return { headline: `${glyph.pass} Installer resolved: ${probe.installerName}`, hints: [] } case 'no-release': - return `${glyph.fail} This deployment reports no desktop release for its channel.` + return { + headline: `${glyph.fail} This deployment reports no desktop release for its channel.`, + hints: [ + 'Stable desktop builds are published on GitHub releases of simstudioai/sim.', + 'A brand-new fork with no releases of its own will report this.', + ], + } case 'feed-unavailable': - return `${glyph.fail} The deployment could not reach the GitHub release feed.` + return { + headline: `${glyph.fail} The deployment could not reach the GitHub release feed.`, + hints: [ + 'The Sim server needs outbound access to api.github.com and github.com.', + 'Unauthenticated GitHub API calls are capped at 60/hour per IP — set GITHUB_TOKEN on the Sim server to raise it to 5000/hour.', + ], + } case 'unreachable': - return `${glyph.fail} Could not reach ${appUrl} — ${probe.error}` + return { + headline: `${glyph.fail} Could not reach ${appUrl} — ${probe.error}`, + hints: [ + `Check that Sim is running and reachable at ${appUrl} (npx sim-setup status).`, + 'Pass --url if this machine reaches Sim at a different address.', + ], + } case 'unexpected': - return `${glyph.fail} The download endpoint answered ${probe.code}.` - } -} - -function probeHints(probe: DesktopProbe, appUrl: string): string[] { - switch (probe.status) { - case 'no-release': - return [ - 'Stable desktop builds are published on GitHub releases of simstudioai/sim.', - 'A brand-new fork with no releases of its own will report this.', - ] - case 'feed-unavailable': - return [ - 'The Sim server needs outbound access to api.github.com and github.com.', - 'Unauthenticated GitHub API calls are capped at 60/hour per IP — set GITHUB_TOKEN on the Sim server to raise it to 5000/hour.', - ] - case 'unreachable': - return [ - `Check that Sim is running and reachable at ${appUrl} (npx sim-setup status).`, - `Pass --url if this machine reaches Sim at a different address.`, - ] - default: - return [] + return { headline: `${glyph.fail} The download endpoint answered ${probe.code}.`, hints: [] } } } export async function runDesktop(flags: DesktopFlags): Promise { - const appUrl = resolveDeploymentUrl(discoverConfigurationSources(), flags.url) + // Discovery shells out to `docker compose ls/config` and `helm list/get + // values`, so it is seconds of blocking work — and every bit of it is + // discarded when --url already names the deployment. + const appUrl = resolveDeploymentUrl(flags.url ? [] : discoverConfigurationSources(), flags.url) const downloadUrl = `${appUrl}${DOWNLOAD_PATH}` p.log.step(`Deployment: ${theme.accent(appUrl)}`) @@ -160,12 +155,13 @@ export async function runDesktop(flags: DesktopFlags): Promise { spin.start('Resolving the desktop installer…') const [probe, feedOk] = await Promise.all([ probeDownload(downloadUrl), - probeFeed(`${appUrl}${FEED_PATH}`), + httpHealth(`${appUrl}${FEED_PATH}`, PROBE_TIMEOUT_MS), ]) - spin.stop(describeProbe(probe, appUrl)) + const { headline, hints } = describeProbe(probe, appUrl) + spin.stop(headline) if (probe.status !== 'ok') { - for (const hint of probeHints(probe, appUrl)) { + for (const hint of hints) { p.log.info(hint) } p.outro(theme.error('The desktop installer could not be resolved.')) @@ -194,10 +190,9 @@ export async function runDesktop(flags: DesktopFlags): Promise { 'Connect the desktop app' ) - if (!flags.noOpen && process.platform === 'darwin') { - const open = await p.confirm({ message: 'Download it now?', initialValue: true }) - if (open) { - spawnSync('open', [downloadUrl], { stdio: 'ignore' }) + if (!flags.noOpen) { + if (await p.confirm({ message: 'Download it now?', initialValue: true })) { + openBrowser(downloadUrl) } } From e8c71d14b378d07b5c49821dac57981430b93be8 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Wed, 26 Aug 2026 16:53:32 -0700 Subject: [PATCH 3/6] fix(desktop,cli): scope deployment capabilities to their origin MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Changing the server left two device-global stores in place that grant the INCOMING deployment authority the user only handed the outgoing one: local filesystem grants (directories its agent may read, plus security-scoped bookmarks) and the agent browser's cookie jar (live third-party sessions its agent may drive). Sign-out clears exactly this pair; an origin change is the same boundary, so it now clears it too — awaited before the relaunch, since a quit racing an async clear could leave either behind. browserKnownSites goes with the jar it describes, so Sim is never left believing in sign-ins the profile no longer has. The CLI printed the redirect's filename straight to the terminal. It is read out of a Location the deployment chose, so percent-encoded ANSI or OSC survives decodeURIComponent as real control bytes and could forge CLI output; control characters are stripped and the name is bounded before it reaches the spinner. resolveDeploymentUrl took the first source naming an app URL. A machine with both a local checkout and a real deployment would be probed, printed, and opened at whichever enumerated first, silently — so disagreeing sources are now an error naming each candidate and asking for --url, the way resolveFeatureSetupDestination already refuses ambiguity. --- apps/desktop/src/main/index.ts | 4 ++ apps/desktop/src/main/ipc.test.ts | 2 +- apps/desktop/src/main/ipc.ts | 2 +- apps/desktop/src/main/server-window.test.ts | 54 +++++++++++++---- apps/desktop/src/main/server-window.ts | 42 ++++++++++---- packages/sim-setup/src/desktop.test.ts | 64 ++++++++++++++++++++- packages/sim-setup/src/desktop.ts | 41 ++++++++++++- 7 files changed, 179 insertions(+), 30 deletions(-) diff --git a/apps/desktop/src/main/index.ts b/apps/desktop/src/main/index.ts index 7d7cf0cc738..6d7d884b6e6 100644 --- a/apps/desktop/src/main/index.ts +++ b/apps/desktop/src/main/index.ts @@ -482,6 +482,10 @@ function main(): void { preloadPath, isPackaged: app.isPackaged, getParentWindow: getMainWindow, + clearDeploymentScopedState: async () => { + await localFilesystem.forgetAll() + await clearAgentBrowserProfile() + }, relaunch: relaunchApp, }) diff --git a/apps/desktop/src/main/ipc.test.ts b/apps/desktop/src/main/ipc.test.ts index 5034ea9dc58..ba248a3f46b 100644 --- a/apps/desktop/src/main/ipc.test.ts +++ b/apps/desktop/src/main/ipc.test.ts @@ -319,7 +319,7 @@ describe('registerIpcHandlers', () => { server: { open: vi.fn(), getConfiguration: vi.fn(() => ({ origin: APP, defaultOrigin: APP, isSimCloud: true })), - setOrigin: vi.fn(() => ({ ok: true as const, origin: APP, unchanged: true })), + setOrigin: vi.fn(async () => ({ ok: true as const, origin: APP, unchanged: true })), }, } registerIpcHandlers(deps) diff --git a/apps/desktop/src/main/ipc.ts b/apps/desktop/src/main/ipc.ts index 6eb61333da0..8d5861eb506 100644 --- a/apps/desktop/src/main/ipc.ts +++ b/apps/desktop/src/main/ipc.ts @@ -306,7 +306,7 @@ export interface IpcDeps { server: { open: () => void getConfiguration: () => DesktopServerConfiguration - setOrigin: (origin: string) => DesktopServerChangeResult + setOrigin: (origin: string) => Promise } } diff --git a/apps/desktop/src/main/server-window.test.ts b/apps/desktop/src/main/server-window.test.ts index eed80063cd9..48cb983cdf9 100644 --- a/apps/desktop/src/main/server-window.test.ts +++ b/apps/desktop/src/main/server-window.test.ts @@ -34,6 +34,7 @@ function makeDeps(overrides: Partial = {}): ServerWindowDeps { preloadPath: '/tmp/preload.cjs', isPackaged: false, getParentWindow: () => null, + clearDeploymentScopedState: vi.fn(async () => {}), relaunch: vi.fn(), ...overrides, } @@ -64,8 +65,8 @@ describe('server window', () => { expect(createServerWindow(cloud).getConfiguration().isSimCloud).toBe(true) }) - it('relaunches after storing a different origin', () => { - const result = createServerWindow(deps).setOrigin('https://sim.other.example') + it('relaunches after storing a different origin', async () => { + const result = await createServerWindow(deps).setOrigin('https://sim.other.example') expect(result).toEqual({ ok: true, origin: 'https://sim.other.example', unchanged: false }) expect(deps.config.flush).toHaveBeenCalled() @@ -75,28 +76,61 @@ describe('server window', () => { // The saved route carries the previous deployment's workspace id, and // resolveStartRoute only discards a route on a confirmed 403 — a fresh // partition answers 401, so a kept route would survive onto the new server. - it('drops the saved route when the origin changes', () => { - createServerWindow(deps).setOrigin('https://sim.other.example') + it('drops the saved route when the origin changes', async () => { + await createServerWindow(deps).setOrigin('https://sim.other.example') expect(deps.config.set).toHaveBeenCalledWith('lastRoute', undefined) }) - it('keeps the saved route when the origin is unchanged', () => { - createServerWindow(deps).setOrigin(CURRENT) + it('keeps the saved route when the origin is unchanged', async () => { + await createServerWindow(deps).setOrigin(CURRENT) expect(deps.config.set).not.toHaveBeenCalled() }) // Re-confirming the pre-filled URL is the common case here. - it('does not relaunch when the origin is unchanged', () => { - const result = createServerWindow(deps).setOrigin(CURRENT) + it('does not relaunch when the origin is unchanged', async () => { + const result = await createServerWindow(deps).setOrigin(CURRENT) expect(result).toEqual({ ok: true, origin: CURRENT, unchanged: true }) expect(deps.relaunch).not.toHaveBeenCalled() }) - it('surfaces a rejected origin without relaunching', () => { - const result = createServerWindow(deps).setOrigin('ftp://sim.example.com') + // Filesystem grants and the agent browser's jar are device-global with no + // origin key, so without this the incoming deployment inherits directory + // access and live third-party sessions the user granted the outgoing one. + it('clears deployment-scoped capabilities before relaunching', async () => { + await createServerWindow(deps).setOrigin('https://sim.other.example') + + expect(deps.clearDeploymentScopedState).toHaveBeenCalledTimes(1) + expect(vi.mocked(deps.clearDeploymentScopedState).mock.invocationCallOrder[0]).toBeLessThan( + vi.mocked(deps.relaunch).mock.invocationCallOrder[0] + ) + }) + + it('does not clear them when the origin is unchanged', async () => { + await createServerWindow(deps).setOrigin(CURRENT) + + expect(deps.clearDeploymentScopedState).not.toHaveBeenCalled() + }) + + // The origin is already persisted by this point, so a failed clear must not + // strand the shell on the old server — but it is logged, not swallowed. + it('still relaunches when the teardown fails', async () => { + const failing = makeDeps({ + clearDeploymentScopedState: vi.fn(async () => { + throw new Error('keychain unavailable') + }), + }) + + const result = await createServerWindow(failing).setOrigin('https://sim.other.example') + + expect(result).toMatchObject({ ok: true, unchanged: false }) + expect(failing.relaunch).toHaveBeenCalledTimes(1) + }) + + it('surfaces a rejected origin without relaunching', async () => { + const result = await createServerWindow(deps).setOrigin('ftp://sim.example.com') expect(result).toEqual({ ok: false, error: 'bad origin' }) expect(deps.relaunch).not.toHaveBeenCalled() diff --git a/apps/desktop/src/main/server-window.ts b/apps/desktop/src/main/server-window.ts index f0c08d658c0..f80ad13ce42 100644 --- a/apps/desktop/src/main/server-window.ts +++ b/apps/desktop/src/main/server-window.ts @@ -36,17 +36,14 @@ const SERVER_WINDOW_PARTITION = 'server-selection' * `/workspace/` on the new server. `resolveStartRoute` cannot rescue * that: it discards a route only on a confirmed 403, and a fresh partition has * no session, so the new server answers 401 and the stale route survives the - * probe. - * - * The agent browser is deliberately untouched — both its cookie jar and the - * `browserKnownSites` inference metadata that describes it. Sign-out clears the - * pair because the ACCOUNT changed; pointing the shell at another deployment - * does not imply that, and signing the operator out of every unrelated site in - * the built-in browser is not a reasonable side effect of correcting a server - * URL. They are kept together on purpose: clearing the metadata alone would - * leave Sim blind to sign-ins that are still live in the profile. + * probe. `browserKnownSites` describes the agent-browser profile that + * {@link ServerWindowDeps.clearDeploymentScopedState} clears, and is dropped + * with it so Sim is never left believing in sign-ins the profile no longer has. */ -const ORIGIN_SCOPED_SETTINGS: readonly (keyof DesktopSettings)[] = ['lastRoute'] +const ORIGIN_SCOPED_SETTINGS: readonly (keyof DesktopSettings)[] = [ + 'lastRoute', + 'browserKnownSites', +] export interface ServerWindowDeps { config: ConfigStore @@ -56,6 +53,19 @@ export interface ServerWindowDeps { preloadPath: string isPackaged: boolean getParentWindow: () => BrowserWindow | null + /** + * Drops the capabilities the OUTGOING deployment was granted, before the new + * one can inherit them. + * + * Local-filesystem grants and the agent browser's cookie jar live in + * device-global stores with no origin key, and both are capabilities the user + * handed to a specific Sim server: directories its agent may read, and live + * third-party sessions its agent may drive. Carrying them across would let + * the next deployment act with authority it was never given — which is why + * sign-out clears exactly this pair. Awaited before the relaunch, since a + * quit racing an async clear could leave either behind. + */ + clearDeploymentScopedState: () => Promise /** * Relaunches the shell against the newly stored origin. A full restart rather * than an in-place swap: the origin decides the cookie partition, the update @@ -72,7 +82,7 @@ export interface ServerWindowDeps { export interface ServerWindowHandle { open(): void getConfiguration(): DesktopServerConfiguration - setOrigin(origin: string): DesktopServerChangeResult + setOrigin(origin: string): Promise } /** @@ -149,7 +159,7 @@ export function createServerWindow(deps: ServerWindowDeps): ServerWindowHandle { }) } - const setOrigin = (raw: string): DesktopServerChangeResult => { + const setOrigin = async (raw: string): Promise => { const current = deps.config.getOrigin() const validated = deps.config.setOrigin(raw) if (!validated.ok) { @@ -164,6 +174,14 @@ export function createServerWindow(deps: ServerWindowDeps): ServerWindowHandle { for (const key of ORIGIN_SCOPED_SETTINGS) { deps.config.set(key, undefined) } + // Failing to clear must not strand the shell on the old origin — that is + // already persisted — but it must be loud, because what survives is access + // the next deployment did not earn. + await deps.clearDeploymentScopedState().catch((error) => { + logger.error('Could not clear deployment-scoped state before relaunch', { + error: getErrorMessage(error), + }) + }) // setOrigin writes through immediately; the clears above are debounced like // every other setting. `before-quit` flushes too, but doing it here keeps // the write independent of the Electron quit sequence. diff --git a/packages/sim-setup/src/desktop.test.ts b/packages/sim-setup/src/desktop.test.ts index 78768e13a67..0cd36af3575 100644 --- a/packages/sim-setup/src/desktop.test.ts +++ b/packages/sim-setup/src/desktop.test.ts @@ -1,11 +1,14 @@ import { describe, expect, it, vi } from 'vitest' -import { describeProbe, probeDownload, resolveDeploymentUrl } from './desktop' +import { describeProbe, probeDownload, resolveDeploymentUrl, sanitizeForTerminal } from './desktop' import { SetupError } from './errors' const ASSET = 'https://github.com/simstudioai/sim/releases/download/v1.2.3/Sim-1.2.3-universal.dmg' -function source(appUrl?: string) { - return { values: appUrl ? new Map([['NEXT_PUBLIC_APP_URL', appUrl]]) : new Map() } +function source(appUrl?: string, label = 'configuration') { + return { + label, + values: appUrl ? new Map([['NEXT_PUBLIC_APP_URL', appUrl]]) : new Map(), + } } function respond(status: number, headers: Record = {}): typeof fetch { @@ -37,6 +40,27 @@ describe('resolveDeploymentUrl', () => { expect(resolveDeploymentUrl([source()])).toBe('http://localhost:3000') }) + // This command prints one URL as the one to trust and offers to open it, so + // preferring whichever source happened to enumerate first would quietly send + // an operator with a local checkout AND a real deployment to localhost. + it('refuses to guess when sources name different deployments', () => { + expect(() => + resolveDeploymentUrl([source('http://localhost:3000'), source('https://sim.example.com')]) + ).toThrow(SetupError) + }) + + it('accepts agreeing sources and an override that settles the ambiguity', () => { + expect( + resolveDeploymentUrl([source('https://sim.example.com'), source('https://sim.example.com')]) + ).toBe('https://sim.example.com') + expect( + resolveDeploymentUrl( + [source('http://localhost:3000'), source('https://sim.example.com')], + 'https://sim.example.com' + ) + ).toBe('https://sim.example.com') + }) + it('rejects a value that is not an http(s) URL', () => { expect(() => resolveDeploymentUrl([source('sim.example.com')])).toThrow(SetupError) expect(() => resolveDeploymentUrl([source('ftp://sim.example.com')])).toThrow(SetupError) @@ -65,6 +89,24 @@ describe('probeDownload', () => { } }) + // The end-to-end path that matters: a deployment can percent-encode ANSI in + // the redirect, and decodeURIComponent turns it into real control bytes on + // their way to the spinner. + it('sanitizes a redirect filename before it reaches the terminal', async () => { + const hostile = 'https://example.com/d/v1/Sim%1b%5b2K%1b%5b1Gforged.dmg' + + const result = await probeDownload( + 'https://sim.example.com/x', + respond(302, { location: hostile }) + ) + + expect(result).toEqual({ + status: 'ok', + installerUrl: hostile, + installerName: 'Sim[2K[1Gforged.dmg', + }) + }) + it('distinguishes no-release from a broken release feed', async () => { expect(await probeDownload('https://sim.example.com/x', respond(404))).toEqual({ status: 'no-release', @@ -96,6 +138,22 @@ describe('probeDownload', () => { }) }) +describe('sanitizeForTerminal', () => { + // The name comes out of a redirect the deployment chose, so it is remote + // input on its way to a TTY. + it('strips control characters a deployment could smuggle through the redirect', () => { + expect(sanitizeForTerminal('Sim\u001b[2K\u001b[1G forged.dmg')).toBe('Sim[2K[1G forged.dmg') + expect(sanitizeForTerminal('a\u0000b\u007fc\u009fd')).toBe('abcd') + expect(sanitizeForTerminal('Sim-1.2.3-universal.dmg')).toBe('Sim-1.2.3-universal.dmg') + }) + + it('caps a name that would overrun the spinner line', () => { + const capped = sanitizeForTerminal('x'.repeat(500)) + + expect(capped).toBe(`${'x'.repeat(120)}...`) + }) +}) + describe('describeProbe', () => { // Failure statuses are the ones an operator has to act on, so each must // arrive with something to try. diff --git a/packages/sim-setup/src/desktop.ts b/packages/sim-setup/src/desktop.ts index e1a257e7c04..e61df544009 100644 --- a/packages/sim-setup/src/desktop.ts +++ b/packages/sim-setup/src/desktop.ts @@ -1,4 +1,5 @@ import { getErrorMessage } from '@sim/utils/errors' +import { truncate } from '@sim/utils/string' import { openBrowser } from './cli-auth' import { discoverConfigurationSources } from './configuration-sources' import { SetupError } from './errors' @@ -23,6 +24,21 @@ const REDIRECT_STATUSES: ReadonlySet = new Set([301, 302, 307, 308]) /** The env var every deployment sets to its own public origin. */ const APP_URL_KEY = 'NEXT_PUBLIC_APP_URL' +/** Keeps a rendered artifact name from overrunning the spinner line. */ +const MAX_INSTALLER_NAME = 120 + +/** + * Strips anything that could move the cursor, repaint, or retitle the terminal. + * + * The artifact name is read out of a redirect the deployment chose, so it is + * remote input on its way to a TTY: percent-encoded ANSI or OSC bytes survive + * `decodeURIComponent` as real control characters and would let a compromised + * deployment forge CLI output. C0 (including ESC), DEL, and C1 all go. + */ +export function sanitizeForTerminal(value: string): string { + return truncate(value.replace(/[\u0000-\u001f\u007f-\u009f]/g, ''), MAX_INSTALLER_NAME) +} + export interface DesktopFlags { /** Overrides the deployment origin when the CLI runs away from the install. */ url?: string @@ -35,12 +51,31 @@ export interface DesktopFlags { * Read from every discovered source, not only the one `add` may write: an * operator running a Helm release or an external Compose project still needs * the URL, and reading it changes nothing. + * + * Sources that disagree are an error rather than a first-match win. This + * command probes a URL, prints it as the one to trust, and offers to open it — + * so silently preferring whichever source enumerated first would point an + * operator with both a local checkout and a real deployment at localhost and + * never say so. `resolveFeatureSetupDestination` refuses ambiguity the same way. */ export function resolveDeploymentUrl( - sources: readonly { values?: Map | null }[], + sources: readonly { label?: string; values?: Map | null }[], override?: string ): string { - const raw = override ?? sources.map((source) => source.values?.get(APP_URL_KEY)).find(Boolean) + const discovered = sources.filter((source) => source.values?.get(APP_URL_KEY)) + const distinct = new Set(discovered.map((source) => source.values?.get(APP_URL_KEY)?.trim())) + if (!override && distinct.size > 1) { + throw new SetupError( + `Found ${distinct.size} configurations naming different ${APP_URL_KEY} values.`, + [ + ...discovered.map( + (source) => `${source.label ?? 'configuration'}: ${source.values?.get(APP_URL_KEY)}` + ), + 'Re-run with --url to say which one the desktop app should use.', + ] + ) + } + const raw = override ?? discovered[0]?.values?.get(APP_URL_KEY) if (!raw) { // A wizard-provisioned local install has the compose interpolation default // rather than an explicit value, so an absent key is not a misconfiguration. @@ -94,7 +129,7 @@ export async function probeDownload( } catch { // Keep the raw Location; it is still the most useful thing to print. } - return { status: 'ok', installerUrl: location, installerName: name } + return { status: 'ok', installerUrl: location, installerName: sanitizeForTerminal(name) } } if (response.status === 404) return { status: 'no-release' } if (response.status === 502) return { status: 'feed-unavailable' } From d351ee375306330e45bb169478f7e7b6d9c5ccc3 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Wed, 26 Aug 2026 17:08:34 -0700 Subject: [PATCH 4/6] fix(desktop,cli): fail closed on origin change, widen terminal sanitizer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The capability teardown could partially fail and still let the shell move. Sequential awaits meant a filesystem-grant rejection skipped the browser-profile clear entirely, and the new origin was already persisted by then, so the incoming deployment inherited whatever survived — and startup restores it. Now the two stores clear independently via allSettled and report which ones survived, and the whole teardown runs BEFORE anything is written. A store that cannot be emptied refuses the change outright and names what it could not clear. Nothing is persisted at that point, so refusing leaves the shell exactly where it was rather than half-applying. Validation moved up front for the same reason: a typo now costs no teardown. The terminal sanitizer matched only C0/DEL/C1 by range, so percent-encoded bidi overrides and isolates survived decodeURIComponent and could still reorder what the reader sees without emitting one control byte. Matched by Unicode class instead — Cc covers the cursor controls, Cf covers the bidi ones. Configuration discovery compared raw strings, so a trailing slash, a default port, a host-case difference, or an ignored path read as two different servers and demanded a --url override to settle an ambiguity that did not exist. Now compared on the parsed origin, which is what the command ends up using. --- apps/desktop/src/main/index.ts | 19 ++++++- apps/desktop/src/main/server-window.test.ts | 39 ++++++++++---- apps/desktop/src/main/server-window.ts | 60 +++++++++++++-------- packages/sim-setup/src/desktop.test.ts | 23 ++++++++ packages/sim-setup/src/desktop.ts | 52 +++++++++++++----- 5 files changed, 148 insertions(+), 45 deletions(-) diff --git a/apps/desktop/src/main/index.ts b/apps/desktop/src/main/index.ts index 6d7d884b6e6..8af9f55d550 100644 --- a/apps/desktop/src/main/index.ts +++ b/apps/desktop/src/main/index.ts @@ -483,8 +483,23 @@ function main(): void { isPackaged: app.isPackaged, getParentWindow: getMainWindow, clearDeploymentScopedState: async () => { - await localFilesystem.forgetAll() - await clearAgentBrowserProfile() + // allSettled, not sequential awaits: these are independent stores, and a + // rejection from the first must not skip the second — leaving the store + // that would have cleared fine still holding the outgoing deployment's + // access. Each failure is named so the picker can say what survived. + const stores = [ + { label: 'local file access', clear: () => localFilesystem.forgetAll() }, + { label: 'built-in browser sessions', clear: () => clearAgentBrowserProfile() }, + ] + const outcomes = await Promise.allSettled(stores.map((store) => store.clear())) + return outcomes.flatMap((outcome, index) => { + if (outcome.status === 'fulfilled') return [] + logger.error('Could not clear deployment-scoped state', { + store: stores[index].label, + error: getErrorMessage(outcome.reason), + }) + return [stores[index].label] + }) }, relaunch: relaunchApp, }) diff --git a/apps/desktop/src/main/server-window.test.ts b/apps/desktop/src/main/server-window.test.ts index 48cb983cdf9..a061e0247c8 100644 --- a/apps/desktop/src/main/server-window.test.ts +++ b/apps/desktop/src/main/server-window.test.ts @@ -34,7 +34,7 @@ function makeDeps(overrides: Partial = {}): ServerWindowDeps { preloadPath: '/tmp/preload.cjs', isPackaged: false, getParentWindow: () => null, - clearDeploymentScopedState: vi.fn(async () => {}), + clearDeploymentScopedState: vi.fn(async (): Promise => []), relaunch: vi.fn(), ...overrides, } @@ -114,25 +114,46 @@ describe('server window', () => { expect(deps.clearDeploymentScopedState).not.toHaveBeenCalled() }) - // The origin is already persisted by this point, so a failed clear must not - // strand the shell on the old server — but it is logged, not swallowed. - it('still relaunches when the teardown fails', async () => { + // Fail closed. A store that could not be emptied is access the incoming + // deployment would inherit and that startup would restore, so the change is + // refused outright — and because nothing is persisted until the teardown + // succeeds, refusing leaves the shell exactly where it was. + it('refuses the change when a store could not be cleared', async () => { const failing = makeDeps({ + clearDeploymentScopedState: vi.fn(async () => ['local file access']), + }) + + const result = await createServerWindow(failing).setOrigin('https://sim.other.example') + + expect(result).toMatchObject({ ok: false }) + expect(result).toHaveProperty('error', expect.stringContaining('local file access')) + expect(failing.relaunch).not.toHaveBeenCalled() + expect(failing.config.setOrigin).not.toHaveBeenCalled() + expect(failing.config.getOrigin()).toBe(CURRENT) + }) + + it('refuses the change when the teardown throws outright', async () => { + const throwing = makeDeps({ clearDeploymentScopedState: vi.fn(async () => { throw new Error('keychain unavailable') }), }) - const result = await createServerWindow(failing).setOrigin('https://sim.other.example') + const result = await createServerWindow(throwing).setOrigin('https://sim.other.example') - expect(result).toMatchObject({ ok: true, unchanged: false }) - expect(failing.relaunch).toHaveBeenCalledTimes(1) + expect(result).toMatchObject({ ok: false }) + expect(throwing.relaunch).not.toHaveBeenCalled() + expect(throwing.config.getOrigin()).toBe(CURRENT) }) - it('surfaces a rejected origin without relaunching', async () => { + // Validated up front with the shell's own rule, before anything is torn down + // or written, so a typo costs nothing. + it('surfaces a rejected origin without tearing anything down', async () => { const result = await createServerWindow(deps).setOrigin('ftp://sim.example.com') - expect(result).toEqual({ ok: false, error: 'bad origin' }) + expect(result).toMatchObject({ ok: false }) + expect(result).toHaveProperty('error', expect.stringContaining('HTTPS')) + expect(deps.clearDeploymentScopedState).not.toHaveBeenCalled() expect(deps.relaunch).not.toHaveBeenCalled() expect(deps.config.getOrigin()).toBe(CURRENT) }) diff --git a/apps/desktop/src/main/server-window.ts b/apps/desktop/src/main/server-window.ts index f80ad13ce42..fe28ef11b1e 100644 --- a/apps/desktop/src/main/server-window.ts +++ b/apps/desktop/src/main/server-window.ts @@ -3,7 +3,7 @@ import { createLogger } from '@sim/logger' import { getErrorMessage } from '@sim/utils/errors' import { app, BrowserWindow, nativeTheme, session } from 'electron' import type { ConfigStore, DesktopSettings } from '@/main/config' -import { isSimCloudOrigin } from '@/main/config' +import { canonicalOrigin, isSimCloudOrigin, validateOriginInput } from '@/main/config' import { backgroundColorFor, createSecureWebPreferences, @@ -54,18 +54,21 @@ export interface ServerWindowDeps { isPackaged: boolean getParentWindow: () => BrowserWindow | null /** - * Drops the capabilities the OUTGOING deployment was granted, before the new - * one can inherit them. + * Drops the capabilities the OUTGOING deployment was granted, and reports + * what it could not drop. * * Local-filesystem grants and the agent browser's cookie jar live in * device-global stores with no origin key, and both are capabilities the user * handed to a specific Sim server: directories its agent may read, and live * third-party sessions its agent may drive. Carrying them across would let * the next deployment act with authority it was never given — which is why - * sign-out clears exactly this pair. Awaited before the relaunch, since a - * quit racing an async clear could leave either behind. + * sign-out clears exactly this pair. + * + * Returns the human-readable name of each store that survived; empty means + * everything is gone. Reporting rather than throwing is what lets one store's + * failure not hide another's, and lets the caller refuse to move. */ - clearDeploymentScopedState: () => Promise + clearDeploymentScopedState: () => Promise /** * Relaunches the shell against the newly stored origin. A full restart rather * than an in-place swap: the origin decides the cookie partition, the update @@ -160,35 +163,50 @@ export function createServerWindow(deps: ServerWindowDeps): ServerWindowHandle { } const setOrigin = async (raw: string): Promise => { - const current = deps.config.getOrigin() - const validated = deps.config.setOrigin(raw) + const validated = validateOriginInput(raw) if (!validated.ok) { return validated } - if (validated.origin === current) { - // Nothing moved, so nothing is torn down. Relaunching anyway would make + // Same canonicalization the store applies, so the comparison below matches + // what would actually be written. + const origin = canonicalOrigin(validated.origin) + const current = deps.config.getOrigin() + if (origin === current) { + // Nothing moves, so nothing is torn down. Relaunching anyway would make // "confirm the URL I already use" restart the app for no reason. - return { ok: true, origin: validated.origin, unchanged: true } + return { ok: true, origin, unchanged: true } + } + + // Fail closed, and clear BEFORE persisting. If a store cannot be emptied, + // the shell must not move: the incoming deployment would otherwise inherit + // folder grants and authenticated browser sessions the outgoing one was + // given, and they are restored on the next startup. Nothing has been + // written at this point, so refusing leaves the shell exactly where it was + // rather than stranding it on a half-applied change. + const surviving = await deps.clearDeploymentScopedState().catch((error) => { + logger.error('Deployment-scoped teardown threw', { error: getErrorMessage(error) }) + return ['local file access and built-in browser sessions'] + }) + if (surviving.length > 0) { + logger.error('Refusing to change server; deployment-scoped state survived', { surviving }) + return { + ok: false, + error: `Could not clear ${surviving.join(' or ')} from the current server, so the server was not changed. Try again, or sign out first.`, + } } - logger.info('Server origin changed; relaunching', { from: current, to: validated.origin }) + + logger.info('Server origin changed; relaunching', { from: current, to: origin }) + deps.config.setOrigin(raw) for (const key of ORIGIN_SCOPED_SETTINGS) { deps.config.set(key, undefined) } - // Failing to clear must not strand the shell on the old origin — that is - // already persisted — but it must be loud, because what survives is access - // the next deployment did not earn. - await deps.clearDeploymentScopedState().catch((error) => { - logger.error('Could not clear deployment-scoped state before relaunch', { - error: getErrorMessage(error), - }) - }) // setOrigin writes through immediately; the clears above are debounced like // every other setting. `before-quit` flushes too, but doing it here keeps // the write independent of the Electron quit sequence. deps.config.flush() close() deps.relaunch() - return { ok: true, origin: validated.origin, unchanged: false } + return { ok: true, origin, unchanged: false } } return { open, getConfiguration, setOrigin } diff --git a/packages/sim-setup/src/desktop.test.ts b/packages/sim-setup/src/desktop.test.ts index 0cd36af3575..795e6399300 100644 --- a/packages/sim-setup/src/desktop.test.ts +++ b/packages/sim-setup/src/desktop.test.ts @@ -49,6 +49,19 @@ describe('resolveDeploymentUrl', () => { ).toThrow(SetupError) }) + // These spell one deployment four ways. Treating them as a conflict would + // demand a --url override to settle an ambiguity that does not exist. + it('treats equivalent spellings of one deployment as agreement', () => { + expect( + resolveDeploymentUrl([ + source('https://sim.example.com'), + source('https://sim.example.com/'), + source('https://SIM.example.com'), + source('https://sim.example.com:443/workspace'), + ]) + ).toBe('https://sim.example.com') + }) + it('accepts agreeing sources and an override that settles the ambiguity', () => { expect( resolveDeploymentUrl([source('https://sim.example.com'), source('https://sim.example.com')]) @@ -147,6 +160,16 @@ describe('sanitizeForTerminal', () => { expect(sanitizeForTerminal('Sim-1.2.3-universal.dmg')).toBe('Sim-1.2.3-universal.dmg') }) + // Bidi overrides and isolates reorder what the reader sees without emitting a + // single control byte, so a range-based C0/C1 filter lets them straight + // through — `gpj.dmg` can be made to render as `dmg.jpg`. + it('strips bidi controls, not just cursor controls', () => { + expect(sanitizeForTerminal('Sim\u202e gmd.eno\u202c.dmg')).toBe('Sim gmd.eno.dmg') + for (const control of ['\u200e', '\u200f', '\u202a', '\u202d', '\u2066', '\u2069', '\u061c']) { + expect(sanitizeForTerminal(`a${control}b`)).toBe('ab') + } + }) + it('caps a name that would overrun the spinner line', () => { const capped = sanitizeForTerminal('x'.repeat(500)) diff --git a/packages/sim-setup/src/desktop.ts b/packages/sim-setup/src/desktop.ts index e61df544009..e79b38cb841 100644 --- a/packages/sim-setup/src/desktop.ts +++ b/packages/sim-setup/src/desktop.ts @@ -24,19 +24,35 @@ const REDIRECT_STATUSES: ReadonlySet = new Set([301, 302, 307, 308]) /** The env var every deployment sets to its own public origin. */ const APP_URL_KEY = 'NEXT_PUBLIC_APP_URL' +/** + * The deployment a configured URL names, for comparison only. Unparseable + * values fall back to their raw text so they compare equal to themselves and + * unequal to everything else. + */ +function deploymentKey(value: string): string { + try { + return new URL(value).origin.toLowerCase() + } catch { + return value + } +} + /** Keeps a rendered artifact name from overrunning the spinner line. */ const MAX_INSTALLER_NAME = 120 /** - * Strips anything that could move the cursor, repaint, or retitle the terminal. + * Strips anything that could move the cursor, repaint or retitle the terminal, + * or reorder what the reader sees. * * The artifact name is read out of a redirect the deployment chose, so it is - * remote input on its way to a TTY: percent-encoded ANSI or OSC bytes survive - * `decodeURIComponent` as real control characters and would let a compromised - * deployment forge CLI output. C0 (including ESC), DEL, and C1 all go. + * remote input on its way to a TTY, and `decodeURIComponent` turns percent- + * encoded bytes into the real characters. Matched by Unicode class rather than + * by hand-listed ranges: `Cc` covers C0 (including ESC), DEL, and C1, while + * `Cf` covers the bidi overrides and isolates that would otherwise survive and + * let a name render in an order it is not written in. */ export function sanitizeForTerminal(value: string): string { - return truncate(value.replace(/[\u0000-\u001f\u007f-\u009f]/g, ''), MAX_INSTALLER_NAME) + return truncate(value.replace(/[\p{Cc}\p{Cf}]/gu, ''), MAX_INSTALLER_NAME) } export interface DesktopFlags { @@ -62,20 +78,30 @@ export function resolveDeploymentUrl( sources: readonly { label?: string; values?: Map | null }[], override?: string ): string { - const discovered = sources.filter((source) => source.values?.get(APP_URL_KEY)) - const distinct = new Set(discovered.map((source) => source.values?.get(APP_URL_KEY)?.trim())) - if (!override && distinct.size > 1) { + const discovered = sources.flatMap((source) => { + const value = source.values?.get(APP_URL_KEY)?.trim() + return value ? [{ label: source.label ?? 'configuration', value }] : [] + }) + // Compared on the parsed origin, which is what the command ultimately uses: + // a trailing slash, a default port, a different host case, or an ignored path + // all name the same deployment, and calling those a conflict would demand a + // --url override to resolve an ambiguity that does not exist. A value that + // will not parse is its own bucket so it still reaches the error below, + // which says something more useful than "these disagree". + const byDeployment = new Map() + for (const { value } of discovered) { + byDeployment.set(deploymentKey(value), value) + } + if (!override && byDeployment.size > 1) { throw new SetupError( - `Found ${distinct.size} configurations naming different ${APP_URL_KEY} values.`, + `Found ${byDeployment.size} configurations naming different ${APP_URL_KEY} values.`, [ - ...discovered.map( - (source) => `${source.label ?? 'configuration'}: ${source.values?.get(APP_URL_KEY)}` - ), + ...discovered.map(({ label, value }) => `${label}: ${value}`), 'Re-run with --url to say which one the desktop app should use.', ] ) } - const raw = override ?? discovered[0]?.values?.get(APP_URL_KEY) + const raw = override ?? byDeployment.values().next().value if (!raw) { // A wizard-provisioned local install has the compose interpolation default // rather than an explicit value, so an absent key is not a misconfiguration. From fc11d12a3b53d86164303f8554cee70bc281d664 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Wed, 26 Aug 2026 17:23:29 -0700 Subject: [PATCH 5/6] fix(cli): sanitize the installer name by Unicode group, not by escape list MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit U+2028 and U+2029 are Zl/Zp, so the Cc/Cf filter let them through and a deployment-controlled redirect filename could still forge a status line. Enumerating what to strip had cost a patch per class found — C0 and C1, then the bidi overrides, now the line separators — so this keeps whole groups instead. `C` removes every control, format, surrogate, private-use, and unassigned code point, covering ESC and OSC, the bidi overrides and isolates, zero-width characters, and the BOM; `Z` removes every space, line, and paragraph separator. Separators become a plain space rather than vanishing so a name is not run together at the seam, and runs are collapsed so the result cannot be padded to push text off the line. The regression table now names each class that reached the terminal in an earlier round, so a future bypass says which one came back. --- packages/sim-setup/src/desktop.test.ts | 30 +++++++++++++++++++------- packages/sim-setup/src/desktop.ts | 29 +++++++++++++++++-------- 2 files changed, 42 insertions(+), 17 deletions(-) diff --git a/packages/sim-setup/src/desktop.test.ts b/packages/sim-setup/src/desktop.test.ts index 795e6399300..e6264c8e98c 100644 --- a/packages/sim-setup/src/desktop.test.ts +++ b/packages/sim-setup/src/desktop.test.ts @@ -160,14 +160,28 @@ describe('sanitizeForTerminal', () => { expect(sanitizeForTerminal('Sim-1.2.3-universal.dmg')).toBe('Sim-1.2.3-universal.dmg') }) - // Bidi overrides and isolates reorder what the reader sees without emitting a - // single control byte, so a range-based C0/C1 filter lets them straight - // through — `gpj.dmg` can be made to render as `dmg.jpg`. - it('strips bidi controls, not just cursor controls', () => { - expect(sanitizeForTerminal('Sim\u202e gmd.eno\u202c.dmg')).toBe('Sim gmd.eno.dmg') - for (const control of ['\u200e', '\u200f', '\u202a', '\u202d', '\u2066', '\u2069', '\u061c']) { - expect(sanitizeForTerminal(`a${control}b`)).toBe('ab') - } + // Every class that reached the terminal in an earlier round, kept as one + // table so a regression names which one came back. Enumerating escapes cost + // a patch per round, which is why the implementation strips whole Unicode + // groups rather than a list. + it.each([ + ['bidi override', '\u202e', ''], + ['bidi isolate', '\u2066', ''], + ['bidi mark', '\u200e', ''], + ['arabic letter mark', '\u061c', ''], + ['zero-width space', '\u200b', ''], + ['byte-order mark', '\ufeff', ''], + ['line separator', '\u2028', ' '], + ['paragraph separator', '\u2029', ' '], + ['no-break space', '\u00a0', ' '], + ])('neutralizes a %s', (_name, character, expected) => { + expect(sanitizeForTerminal(`a${character}b`)).toBe(`a${expected}b`) + }) + + // Separators become a space rather than vanishing, so a name is not silently + // run together at the seam. + it('reduces a name built from several classes at once to printable text', () => { + expect(sanitizeForTerminal('Sim\u001b[2K\u202e\u2028\u00a0X.dmg')).toBe('Sim[2K X.dmg') }) it('caps a name that would overrun the spinner line', () => { diff --git a/packages/sim-setup/src/desktop.ts b/packages/sim-setup/src/desktop.ts index e79b38cb841..60132f633ad 100644 --- a/packages/sim-setup/src/desktop.ts +++ b/packages/sim-setup/src/desktop.ts @@ -41,18 +41,29 @@ function deploymentKey(value: string): string { const MAX_INSTALLER_NAME = 120 /** - * Strips anything that could move the cursor, repaint or retitle the terminal, - * or reorder what the reader sees. + * Reduces an artifact name to printable text before it reaches a TTY. * - * The artifact name is read out of a redirect the deployment chose, so it is - * remote input on its way to a TTY, and `decodeURIComponent` turns percent- - * encoded bytes into the real characters. Matched by Unicode class rather than - * by hand-listed ranges: `Cc` covers C0 (including ESC), DEL, and C1, while - * `Cf` covers the bidi overrides and isolates that would otherwise survive and - * let a name render in an order it is not written in. + * The name is read out of a redirect the deployment chose, so it is remote + * input, and `decodeURIComponent` turns percent-encoded bytes into the real + * characters. Enumerating what to strip invited a patch per escape found — C0 + * and C1, then the bidi overrides, then the line separators — so this keeps + * whole Unicode groups instead: `C` (Other) removes every control, format, + * surrogate, private-use, and unassigned code point, covering ESC and OSC, the + * bidi overrides and isolates, zero-width characters, and the BOM; `Z` + * (Separator) removes every space, line, and paragraph separator, covering + * U+2028/U+2029 and no-break spaces. + * + * Separators become a plain space rather than vanishing, so a name is not + * silently run together at the seam, and runs are collapsed so the result + * cannot be padded out to push text off the line. */ export function sanitizeForTerminal(value: string): string { - return truncate(value.replace(/[\p{Cc}\p{Cf}]/gu, ''), MAX_INSTALLER_NAME) + const printable = value + .replace(/\p{C}/gu, '') + .replace(/\p{Z}/gu, ' ') + .replace(/ {2,}/g, ' ') + .trim() + return truncate(printable, MAX_INSTALLER_NAME) } export interface DesktopFlags { From d1ecc6a935ae90bb28ed767c3bd8d1cf6d8b986d Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Wed, 26 Aug 2026 17:36:20 -0700 Subject: [PATCH 6/6] fix(desktop): serialize server changes and report partial teardown honestly MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two problems in the same transaction. The picker re-enabled Connect whenever the field changed, including while a request was in flight, so typing and pressing Enter could start a second change that interleaved its teardown and its write with the first — the later write, not a single transition, deciding the next server. The transaction is now serialized in the main process, the way the sign-out coordinator guards its own teardown, since the IPC boundary is reachable regardless of what the page does; the page keeps its button disabled for the whole request so it never asks for something it will only be refused. The stores clear independently, so one can succeed while the other fails. There is nothing to roll back to — a revoked cookie jar and deleted security-scoped bookmarks cannot be un-deleted — and moving anyway would hand the incoming deployment whatever survived. So the change is still refused, but the message no longer names only the failed store as though nothing else had happened: it says some local access may already have been cleared, and that retrying finishes the job. Clearing an already-empty store succeeds, so a retry is safe. --- apps/desktop/src/main/server-window.test.ts | 46 ++++++++++++ apps/desktop/src/main/server-window.ts | 77 +++++++++++++-------- apps/desktop/static/server.html | 11 ++- 3 files changed, 106 insertions(+), 28 deletions(-) diff --git a/apps/desktop/src/main/server-window.test.ts b/apps/desktop/src/main/server-window.test.ts index a061e0247c8..ced9788b13c 100644 --- a/apps/desktop/src/main/server-window.test.ts +++ b/apps/desktop/src/main/server-window.test.ts @@ -114,6 +114,48 @@ describe('server window', () => { expect(deps.clearDeploymentScopedState).not.toHaveBeenCalled() }) + // The picker re-enables its button while a request is pending, and the IPC + // boundary is reachable regardless of what the page does, so the transaction + // has to be serialized here rather than in the renderer. + it('refuses a second change while one is in flight', async () => { + let release: (() => void) | undefined + const gate = new Promise((resolve) => { + release = resolve + }) + const slow = makeDeps({ + clearDeploymentScopedState: vi.fn(async (): Promise => { + await gate + return [] + }), + }) + const handle = createServerWindow(slow) + + const first = handle.setOrigin('https://sim.other.example') + const second = await handle.setOrigin('https://sim.third.example') + + expect(second).toMatchObject({ ok: false }) + expect(second).toHaveProperty('error', expect.stringContaining('already in progress')) + release?.() + await expect(first).resolves.toMatchObject({ ok: true, unchanged: false }) + expect(slow.relaunch).toHaveBeenCalledTimes(1) + expect(slow.config.setOrigin).toHaveBeenCalledTimes(1) + expect(slow.config.setOrigin).toHaveBeenCalledWith('https://sim.other.example') + }) + + // The guard must not latch: a refused change has to leave the picker usable. + it('allows a later change once the first has settled', async () => { + const failing = makeDeps({ + clearDeploymentScopedState: vi.fn(async () => ['local file access']), + }) + const handle = createServerWindow(failing) + + await handle.setOrigin('https://sim.other.example') + const second = await handle.setOrigin('https://sim.third.example') + + expect(second).toMatchObject({ ok: false }) + expect(second).toHaveProperty('error', expect.stringContaining('local file access')) + }) + // Fail closed. A store that could not be emptied is access the incoming // deployment would inherit and that startup would restore, so the change is // refused outright — and because nothing is persisted until the teardown @@ -127,6 +169,10 @@ describe('server window', () => { expect(result).toMatchObject({ ok: false }) expect(result).toHaveProperty('error', expect.stringContaining('local file access')) + // The stores clear independently, so the other one may already be empty and + // cannot be restored. Naming only the failure would read as "nothing + // happened", which is not what happened. + expect(result).toHaveProperty('error', expect.stringContaining('may already have been cleared')) expect(failing.relaunch).not.toHaveBeenCalled() expect(failing.config.setOrigin).not.toHaveBeenCalled() expect(failing.config.getOrigin()).toBe(CURRENT) diff --git a/apps/desktop/src/main/server-window.ts b/apps/desktop/src/main/server-window.ts index fe28ef11b1e..03d6eb003b5 100644 --- a/apps/desktop/src/main/server-window.ts +++ b/apps/desktop/src/main/server-window.ts @@ -100,6 +100,14 @@ export interface ServerWindowHandle { */ export function createServerWindow(deps: ServerWindowDeps): ServerWindowHandle { let win: BrowserWindow | null = null + /** + * Serializes the destructive part of a change, the way the sign-out + * coordinator guards its own teardown. The picker re-enables its button + * while a request is pending, and the IPC boundary is reachable regardless + * of what the page does, so without this two changes could interleave their + * teardown and their write and let the later write pick the next server. + */ + let changeInFlight = false const getConfiguration = (): DesktopServerConfiguration => { const origin = deps.config.getOrigin() @@ -177,36 +185,51 @@ export function createServerWindow(deps: ServerWindowDeps): ServerWindowHandle { return { ok: true, origin, unchanged: true } } - // Fail closed, and clear BEFORE persisting. If a store cannot be emptied, - // the shell must not move: the incoming deployment would otherwise inherit - // folder grants and authenticated browser sessions the outgoing one was - // given, and they are restored on the next startup. Nothing has been - // written at this point, so refusing leaves the shell exactly where it was - // rather than stranding it on a half-applied change. - const surviving = await deps.clearDeploymentScopedState().catch((error) => { - logger.error('Deployment-scoped teardown threw', { error: getErrorMessage(error) }) - return ['local file access and built-in browser sessions'] - }) - if (surviving.length > 0) { - logger.error('Refusing to change server; deployment-scoped state survived', { surviving }) - return { - ok: false, - error: `Could not clear ${surviving.join(' or ')} from the current server, so the server was not changed. Try again, or sign out first.`, - } + if (changeInFlight) { + return { ok: false, error: 'A server change is already in progress.' } } + changeInFlight = true + try { + // Fail closed, and clear BEFORE persisting. If a store cannot be emptied, + // the shell must not move: the incoming deployment would otherwise + // inherit folder grants and authenticated browser sessions the outgoing + // one was given, and they are restored on the next startup. Nothing has + // been written at this point, so refusing leaves the shell on the server + // it was already using rather than half-applying the change. + const surviving = await deps.clearDeploymentScopedState().catch((error) => { + logger.error('Deployment-scoped teardown threw', { error: getErrorMessage(error) }) + return ['local file access and built-in browser sessions'] + }) + if (surviving.length > 0) { + logger.error('Refusing to change server; deployment-scoped state survived', { surviving }) + // Deliberately describes the whole teardown, not just what failed. The + // stores clear independently, so one may already be empty by now, and + // there is nothing to roll back to — a revoked cookie jar and deleted + // security-scoped bookmarks cannot be un-deleted. Saying "some may have + // been cleared" is the honest account, and a retry is safe: clearing an + // already-empty store succeeds, so it finishes the job rather than + // repeating it. + return { + ok: false, + error: `Could not clear ${surviving.join(' or ')} from the current server, so the server was not changed. Some local access may already have been cleared. Try again to finish, or sign out first.`, + } + } - logger.info('Server origin changed; relaunching', { from: current, to: origin }) - deps.config.setOrigin(raw) - for (const key of ORIGIN_SCOPED_SETTINGS) { - deps.config.set(key, undefined) + logger.info('Server origin changed; relaunching', { from: current, to: origin }) + deps.config.setOrigin(raw) + for (const key of ORIGIN_SCOPED_SETTINGS) { + deps.config.set(key, undefined) + } + // setOrigin writes through immediately; the clears above are debounced + // like every other setting. `before-quit` flushes too, but doing it here + // keeps the write independent of the Electron quit sequence. + deps.config.flush() + close() + deps.relaunch() + return { ok: true, origin, unchanged: false } + } finally { + changeInFlight = false } - // setOrigin writes through immediately; the clears above are debounced like - // every other setting. `before-quit` flushes too, but doing it here keeps - // the write independent of the Electron quit sequence. - deps.config.flush() - close() - deps.relaunch() - return { ok: true, origin, unchanged: false } } return { open, getConfiguration, setOrigin } diff --git a/apps/desktop/static/server.html b/apps/desktop/static/server.html index 49b105ede12..ab39ce2e523 100644 --- a/apps/desktop/static/server.html +++ b/apps/desktop/static/server.html @@ -204,8 +204,14 @@

Sim server

input.setAttribute('aria-invalid', tone === 'error' ? 'true' : 'false') } + // Set for the whole life of a request. Without it the input handler below + // re-enables Connect mid-flight, letting a second change race the first. + // The main process guards the transaction itself; this keeps the page + // from asking for something it will only be refused. + let pending = false + function syncConnectEnabled() { - connect.disabled = input.value.trim().length === 0 + connect.disabled = pending || input.value.trim().length === 0 } input.addEventListener('input', () => { @@ -219,6 +225,8 @@

Sim server

cancel.addEventListener('click', () => window.close()) async function submit() { + if (pending) return + pending = true connect.disabled = true setMessage('Connecting…') try { @@ -235,6 +243,7 @@

Sim server

} catch { setMessage('The server could not be changed.', 'error') } finally { + pending = false syncConnectEnabled() } }