From 4a2d3877c73a710f1240c04a108d67c399101788 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Thu, 10 Sep 2026 17:19:19 -0700 Subject: [PATCH 1/3] improvement(desktop): make each terminal its own resource tab --- apps/desktop/src/main/ipc.test.ts | 10 +- apps/desktop/src/main/ipc.ts | 41 +- apps/desktop/src/main/terminal/index.ts | 48 +-- .../src/main/terminal/registry.test.ts | 85 ++-- apps/desktop/src/main/terminal/registry.ts | 55 ++- .../desktop/src/main/terminal/service.test.ts | 35 +- apps/desktop/src/preload/index.ts | 25 +- .../app/api/copilot/chat/resources/route.ts | 3 +- .../chats/[chatId]/fork/route.test.ts | 3 +- .../add-resource-dropdown.tsx | 29 +- .../terminal-session/terminal-session.test.ts | 50 --- .../terminal-session/terminal-session.tsx | 408 +----------------- .../terminal-session/terminal-tab-icon.tsx | 23 - .../resource-content/resource-content.tsx | 4 +- .../resource-registry/resource-registry.tsx | 10 +- .../terminal-tab-icon.test.tsx | 21 +- .../resource-registry/terminal-tab-icon.tsx | 52 +++ .../resource-tabs/resource-tabs.test.ts | 53 --- .../resource-tabs/resource-tabs.tsx | 166 ++++--- .../mothership-view/mothership-view.tsx | 23 +- .../user-input/components/constants.ts | 12 +- .../plus-menu-dropdown.test.tsx | 62 ++- .../plus-menu-dropdown/plus-menu-dropdown.tsx | 25 +- .../resource-mention-items.test.ts | 61 +-- .../resource-mention-items.ts | 46 +- .../components/resource-context.test.ts | 15 +- .../app/workspace/[workspaceId]/home/home.tsx | 18 +- .../stream/handle-resource-event.test.ts | 24 ++ .../hooks/stream/handle-resource-event.ts | 16 +- .../home/hooks/use-browser-tab-resources.ts | 149 ++----- .../[workspaceId]/home/hooks/use-chat.test.ts | 63 +-- .../[workspaceId]/home/hooks/use-chat.ts | 86 +--- .../home/hooks/use-desktop-tab-resources.ts | 160 +++++++ .../hooks/use-terminal-tab-resources.test.tsx | 174 ++++++++ .../home/hooks/use-terminal-tab-resources.ts | 77 ++++ .../hooks/use-settled-terminal-commands.ts | 59 +++ .../lib/copilot/chat/process-contents.test.ts | 10 +- apps/sim/lib/copilot/chat/process-contents.ts | 9 - .../sim/lib/copilot/resources/availability.ts | 9 +- apps/sim/lib/copilot/resources/types.test.ts | 34 +- apps/sim/lib/copilot/resources/types.ts | 64 +-- apps/sim/lib/terminal/focus.ts | 30 ++ apps/sim/lib/terminal/resource-id.test.ts | 12 + apps/sim/lib/terminal/resource-id.ts | 17 + apps/sim/lib/terminal/tab-label.test.ts | 53 +++ apps/sim/lib/terminal/tab-label.ts | 23 + apps/sim/lib/terminal/transport.test.ts | 168 ++++++-- apps/sim/lib/terminal/transport.ts | 34 +- packages/desktop-bridge/contract-snapshot.ts | 24 +- packages/desktop-bridge/src/index.ts | 24 +- 50 files changed, 1474 insertions(+), 1228 deletions(-) delete mode 100644 apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/components/terminal-session/terminal-tab-icon.tsx rename apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/{resource-content/components/terminal-session => resource-registry}/terminal-tab-icon.test.tsx (76%) create mode 100644 apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-registry/terminal-tab-icon.tsx delete mode 100644 apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-tabs/resource-tabs.test.ts create mode 100644 apps/sim/app/workspace/[workspaceId]/home/hooks/use-desktop-tab-resources.ts create mode 100644 apps/sim/app/workspace/[workspaceId]/home/hooks/use-terminal-tab-resources.test.tsx create mode 100644 apps/sim/app/workspace/[workspaceId]/home/hooks/use-terminal-tab-resources.ts create mode 100644 apps/sim/hooks/use-settled-terminal-commands.ts create mode 100644 apps/sim/lib/terminal/focus.ts create mode 100644 apps/sim/lib/terminal/resource-id.test.ts create mode 100644 apps/sim/lib/terminal/resource-id.ts create mode 100644 apps/sim/lib/terminal/tab-label.test.ts create mode 100644 apps/sim/lib/terminal/tab-label.ts diff --git a/apps/desktop/src/main/ipc.test.ts b/apps/desktop/src/main/ipc.test.ts index 959fb436fcf..cace1224b65 100644 --- a/apps/desktop/src/main/ipc.test.ts +++ b/apps/desktop/src/main/ipc.test.ts @@ -468,20 +468,20 @@ describe('registerIpcHandlers', () => { deps.accountDataAvailable = () => false const { invoke } = collectHandlers() const localFilesystemHandle = vi.spyOn(deps.localFilesystem, 'handle') - const terminalStart = vi.spyOn(deps.terminal, 'start') + const terminalRestore = vi.spyOn(deps.terminal, 'restoreScope') await expect( invoke.get('desktop:local-filesystem')?.(appEvent, { operation: 'list_mounts' }) ).resolves.toMatchObject({ ok: false, code: 'ACCESS_DENIED' }) await expect(invoke.get('browser-credentials:list')?.(appEvent)).resolves.toEqual([]) - await expect(invoke.get('terminal:start')?.(appEvent, {}, 'chat-a')).resolves.toMatchObject({ - ok: false, - code: 'ACCESS_DENIED', + await expect(invoke.get('terminal:restore-scope')?.(appEvent, 'chat-a')).resolves.toEqual({ + tabs: [], + activeTerminalId: null, }) expect(localFilesystemHandle).not.toHaveBeenCalled() expect(listCredentials).not.toHaveBeenCalled() - expect(terminalStart).not.toHaveBeenCalled() + expect(terminalRestore).not.toHaveBeenCalled() }) it('requires an active user gesture for granting or revoking folder access', async () => { diff --git a/apps/desktop/src/main/ipc.ts b/apps/desktop/src/main/ipc.ts index 049527f5d1a..e55dcb7a2c7 100644 --- a/apps/desktop/src/main/ipc.ts +++ b/apps/desktop/src/main/ipc.ts @@ -1516,39 +1516,20 @@ export function registerIpcHandlers(deps: IpcDeps): void { return fillCoordinator()?.fillCredential(id, scope) ?? false }, }, - 'terminal:start': { + 'terminal:restore-scope': { kind: 'invoke', gate: 'app-origin', requires: 'terminal', passSender: true, - denied: { ok: false, code: 'ACCESS_DENIED', error: 'Not allowed from this page.' }, - handler: (sender, raw, rawScope) => { - const contents = sender as WebContents - const scope = rendererScope(terminalScopeBySender, contents, rawScope) - if (!scope) { - return { ok: false, code: 'STALE_SCOPE', error: 'This terminal chat is not active.' } - } - const options = isRecordLike(raw) ? raw : {} - const cols = Number(options.cols) - const rows = Number(options.rows) + denied: { tabs: [], activeTerminalId: null }, + handler: (sender, rawScope) => { + const scope = rendererScope(terminalScopeBySender, sender as WebContents, rawScope) + if (!scope) return { tabs: [], activeTerminalId: null } try { - return { - ok: true, - tabs: { - ...deps.terminal.start(scope, { - cols: toCellCount(cols, 80), - rows: toCellCount(rows, 24), - }), - scopeId: scope, - }, - } + return { ...deps.terminal.restoreScope(scope), scopeId: scope } } catch (error) { - const failure = error as { code?: string; message?: string } - return { - ok: false, - code: failure.code ?? 'SPAWN_FAILED', - error: failure.message ?? 'Could not open a terminal.', - } + logger.warn('Could not restore saved terminals', { error: getErrorMessage(error) }) + return { ...deps.terminal.getTabs(scope), scopeId: scope } } }, }, @@ -1778,12 +1759,13 @@ export function registerIpcHandlers(deps: IpcDeps): void { requires: 'terminal', passSender: true, denied: { tabs: [], activeTerminalId: null }, - handler: (sender, terminalId, rawScope) => { + handler: (sender, terminalId, rawScope, rawOptions) => { const scope = rendererScope(terminalScopeBySender, sender as WebContents, rawScope) if (!scope) return { tabs: [], activeTerminalId: null } + const claim = !(isRecordLike(rawOptions) && rawOptions.claim === false) const tabs = typeof terminalId === 'string' - ? deps.terminal.switchTerminal(scope, terminalId) + ? deps.terminal.switchTerminal(scope, terminalId, { claim }) : deps.terminal.getTabs(scope) return { ...tabs, scopeId: scope } }, @@ -1850,7 +1832,6 @@ export function registerIpcHandlers(deps: IpcDeps): void { handler: (sender, terminalId, cols, rows, rawScope) => { // `typeof NaN === 'number'`, and the downstream `cols <= 0` guard is // false for NaN, so an unfinite value reached pty.resize() intact. - // Matches the clamping terminal:start already applies to these fields. if (typeof terminalId !== 'string') return if (!isPositiveFinite(cols) || !isPositiveFinite(rows)) return const scope = rendererScope(terminalScopeBySender, sender as WebContents, rawScope) diff --git a/apps/desktop/src/main/terminal/index.ts b/apps/desktop/src/main/terminal/index.ts index e22a8911cd7..49435ca3b00 100644 --- a/apps/desktop/src/main/terminal/index.ts +++ b/apps/desktop/src/main/terminal/index.ts @@ -347,12 +347,20 @@ export class TerminalService { return this.getAgentTabs() } - switchTerminal(terminalId: string): TerminalTabsState { + /** + * Shows a terminal. `claim` records it as the user's own; a switch that only + * mirrors the renderer's resource-strip selection passes false so the agent + * can still close or adopt the shell as its own. + */ + switchTerminal( + terminalId: string, + { claim = true }: { claim?: boolean } = {} + ): TerminalTabsState { if (!this.sessions.has(terminalId)) { throw new TerminalError('NO_SUCH_TERMINAL', unknownTerminal(terminalId)) } this.activeId = terminalId - this.activeTerminalUserSelected = true + if (claim) this.activeTerminalUserSelected = true this.emitTabs() void this.sessions.get(terminalId)?.refreshCwd() return this.getTabs() @@ -387,19 +395,11 @@ export class TerminalService { } /** - * Closes a terminal, or resets it when it is the only one left. - * - * Emptying the panel is not an option the close button should have: the - * resource IS a terminal, so a panel with no shell in it is a dead end the - * user has to close and reopen to escape. Replacing the last shell with a - * fresh one in the same directory gives the button a sensible meaning at - * every count — the same shape as closing a browser's last tab, which - * leaves you a tab rather than an empty window. - * - * A shell that ends by itself — `exit`, or Ctrl-D — goes the same way. It - * leaves behind a session that can no longer do anything, so it has to be - * reaped either way; treating it as a close means the last one is replaced - * rather than leaving a dead tab that cannot be typed into. + * Closes a terminal. Each shell is its own resource tab in the renderer, so + * closing the last one simply leaves none; the strip drops the tab and a new + * shell comes back through `+ Terminal` or the agent. A shell that ends by + * itself — `exit`, or Ctrl-D — goes the same way: it leaves behind a session + * that can no longer do anything, so it is reaped like a close. */ closeTerminal(terminalId: string): TerminalTabsState { if (!this.sessions.has(terminalId)) { @@ -451,16 +451,13 @@ export class TerminalService { } /** - * Drops a terminal and decides what replaces it. Closing and exiting share - * this so the two cannot drift into different answers for "what happens to - * the last one". + * Drops a terminal and moves both cursors to a neighbour. Closing and + * exiting share this so the two cannot drift into different answers. */ private retire(terminalId: string): TerminalTabsState { const session = this.sessions.get(terminalId) if (!session) return this.getTabs() const closedCwd = session.currentCwd - const cols = session.cols - const rows = session.rows const order = [...this.sessions.keys()] const index = order.indexOf(terminalId) session.dispose() @@ -468,15 +465,10 @@ export class TerminalService { this.tmuxCache.delete(terminalId) this.releasePendingRuns(terminalId) - if (this.sessions.size === 0) { - this.spawn(this.resolveCwd(closedCwd), cols, rows, { - activateVisible: true, - activateAgent: true, - }) - return this.getTabs() - } - this.rememberClosed(closedCwd) + // Nothing is left for the user to hold on to; the next shell the agent + // opens must not inherit a claim on a terminal that no longer exists. + if (this.sessions.size === 0) this.activeTerminalUserSelected = false if (this.activeId === terminalId) { this.activeId = order[index + 1] ?? order[index - 1] ?? null } diff --git a/apps/desktop/src/main/terminal/registry.test.ts b/apps/desktop/src/main/terminal/registry.test.ts index 6300a1adb00..3697e02420e 100644 --- a/apps/desktop/src/main/terminal/registry.test.ts +++ b/apps/desktop/src/main/terminal/registry.test.ts @@ -120,9 +120,9 @@ describe('TerminalRegistry', () => { const events = sink() terminals.setSink(events) - const firstA = terminals.start('chat-A', { cols: 80, rows: 24 }) + const firstA = terminals.openTerminal('chat-A') terminals.openTerminal('chat-A', '/tmp') - const firstB = terminals.start('chat-B', { cols: 100, rows: 30 }) + const firstB = terminals.openTerminal('chat-B') expect(firstA.activeTerminalId).toBe('1') expect(firstB.activeTerminalId).toBe('1') @@ -165,7 +165,7 @@ describe('TerminalRegistry', () => { const terminals = registry() const events = sink() terminals.setSink(events) - terminals.start('pending:new', { cols: 80, rows: 24 }) + terminals.openTerminal('pending:new') terminals.openTerminal('pending:new', '/tmp') const before = terminals.getTabs('pending:new') const originalSessions = [...stubSessions] @@ -195,7 +195,7 @@ describe('TerminalRegistry', () => { disposeScope: vi.fn(), } const terminals = new TerminalRegistry(persistence) - terminals.start('pending:new', { cols: 80, rows: 24 }) + terminals.openTerminal('pending:new') expect(terminals.migrateScope('pending:new', 'chat-existing')).toBe(false) expect(terminals.peekTabs('pending:new').tabs).toHaveLength(1) @@ -204,7 +204,40 @@ describe('TerminalRegistry', () => { terminals.dispose() }) - it('restores saved tab directories lazily as fresh shells', () => { + it('empties the saved shells once the user closes them all', () => { + const persistence: TerminalScopePersistence = { + load: vi.fn(() => ({ v: 1 as const, tabs: [{ cwd: tmpdir() }], activeIndex: 0 })), + save: vi.fn(() => true), + migrate: vi.fn(() => true), + disposeScope: vi.fn(), + } + const terminals = new TerminalRegistry(persistence) + terminals.setSink(sink()) + const restored = terminals.restoreScope('chat-A') + + terminals.closeTerminal('chat-A', restored.activeTerminalId as string) + + expect(persistence.save).toHaveBeenLastCalledWith('chat-A', { v: 1, tabs: [], activeIndex: 0 }) + terminals.dispose() + expect(persistence.save).toHaveBeenLastCalledWith('chat-A', { v: 1, tabs: [], activeIndex: 0 }) + }) + + it('keeps a saved descriptor that was never applied', () => { + const persistence: TerminalScopePersistence = { + load: vi.fn(() => ({ v: 1 as const, tabs: [{ cwd: tmpdir() }], activeIndex: 0 })), + save: vi.fn(() => true), + migrate: vi.fn(() => true), + disposeScope: vi.fn(), + } + const terminals = new TerminalRegistry(persistence) + terminals.peekTabs('chat-A') + + terminals.dispose() + + expect(persistence.save).not.toHaveBeenCalled() + }) + + it('restores saved tab directories as fresh shells when the scope is hydrated', () => { const persistedTabs = Array.from({ length: 12 }, (_, index) => ({ cwd: index % 2 === 0 ? tmpdir() : process.cwd(), })) @@ -226,7 +259,7 @@ describe('TerminalRegistry', () => { }) expect(stubSessions).toHaveLength(0) - const restored = terminals.start('chat-A', { cols: 120, rows: 40 }) + const restored = terminals.restoreScope('chat-A') expect(stubSessions.map(({ cwd }) => cwd)).toEqual(persistedTabs.map(({ cwd }) => cwd)) expect(restored).toMatchObject({ @@ -267,7 +300,7 @@ describe('TerminalRegistry', () => { terminals.setPanelFocused('chat-A', true, owner as never) createControl.failAt = 2 - expect(() => terminals.start('chat-A', { cols: 120, rows: 40 })).toThrow('PTY spawn failed') + expect(() => terminals.restoreScope('chat-A')).toThrow('PTY spawn failed') expect(stubSessions).toHaveLength(1) expect(stubSessions[0].disposed).toBe(true) expect(terminals.peekTabs('chat-A')).toEqual({ tabs: [], activeTerminalId: null }) @@ -275,7 +308,7 @@ describe('TerminalRegistry', () => { expect(events.tabs).not.toHaveBeenCalled() createControl.failAt = null - const restored = terminals.start('chat-A', { cols: 120, rows: 40 }) + const restored = terminals.restoreScope('chat-A') expect(restored.tabs.map(({ cwd }) => cwd)).toEqual(persistedTabs.map(({ cwd }) => cwd)) expect(restored.activeTerminalId).toBe('2') @@ -317,7 +350,7 @@ describe('TerminalRegistry', () => { } const terminals = new TerminalRegistry(persistence) - const restored = terminals.start('chat-A', { cols: 120, rows: 40 }) + const restored = terminals.restoreScope('chat-A') expect(restored.tabs).toHaveLength(16) expect(restored.activeTerminalId).toBe('16') @@ -335,10 +368,10 @@ describe('TerminalRegistry', () => { it('enforces the process-wide terminal ceiling without evicting live scopes', () => { const terminals = registry() for (let index = 0; index < 48; index++) { - terminals.start(`chat-${index}`, { cols: 80, rows: 24 }) + terminals.openTerminal(`chat-${index}`) } - expect(() => terminals.start('chat-overflow', { cols: 80, rows: 24 })).toThrow( + expect(() => terminals.openTerminal('chat-overflow')).toThrow( expect.objectContaining({ code: 'RESOURCE_LIMIT' }) ) expect(stubSessions).toHaveLength(48) @@ -359,16 +392,16 @@ describe('TerminalRegistry', () => { } const terminals = new TerminalRegistry(persistence) for (let index = 0; index < 46; index++) { - terminals.start(`chat-live-${index}`, { cols: 80, rows: 24 }) + terminals.openTerminal(`chat-live-${index}`) } - expect(() => terminals.start('chat-pending-restore', { cols: 80, rows: 24 })).toThrow( + expect(() => terminals.restoreScope('chat-pending-restore')).toThrow( expect.objectContaining({ code: 'RESOURCE_LIMIT' }) ) expect(persistence.save).not.toHaveBeenCalledWith('chat-pending-restore', expect.anything()) terminals.disposeScope('chat-live-0') - const restored = terminals.start('chat-pending-restore', { cols: 80, rows: 24 }) + const restored = terminals.restoreScope('chat-pending-restore') expect(restored.tabs.map(({ cwd }) => cwd)).toEqual(persistedTabs.map(({ cwd }) => cwd)) expect(restored.activeTerminalId).toBe('2') @@ -393,7 +426,7 @@ describe('TerminalRegistry', () => { } const terminals = new TerminalRegistry(persistence) - const restored = terminals.start('chat-A', { cols: 120, rows: 40 }) + const restored = terminals.restoreScope('chat-A') expect(stubSessions.map(({ cwd }) => cwd)).toEqual([tmpdir(), tmpdir(), process.cwd()]) expect(restored.activeTerminalId).toBe('2') @@ -415,7 +448,7 @@ describe('TerminalRegistry', () => { disposeScope: vi.fn(), } const terminals = new TerminalRegistry(persistence) - terminals.start('pending:new', { cols: 80, rows: 24 }) + terminals.openTerminal('pending:new') vi.mocked(persistence.save).mockClear() terminals.disposeScope('pending:new') @@ -444,7 +477,7 @@ describe('TerminalRegistry', () => { } const terminals = new TerminalRegistry(persistence) const rememberedCwd = tmpdir() - terminals.start('chat-deleted', { cols: 80, rows: 24 }) + terminals.openTerminal('chat-deleted') terminals.openTerminal('chat-deleted', rememberedCwd) terminals.switchTerminal('chat-deleted', '1') const originalSessions = [...stubSessions] @@ -465,7 +498,7 @@ describe('TerminalRegistry', () => { activeTerminalId: null, }) - expect(terminals.start('chat-deleted', { cols: 100, rows: 30 })).toEqual({ + expect(terminals.restoreScope('chat-deleted')).toEqual({ tabs: [], activeTerminalId: null, }) @@ -476,7 +509,7 @@ describe('TerminalRegistry', () => { expect(stubSessions).toHaveLength(2) terminals.activateScope('chat-deleted') - const restored = terminals.start('chat-deleted', { cols: 100, rows: 30 }) + const restored = terminals.restoreScope('chat-deleted') expect(stubSessions).toHaveLength(4) expect(stubSessions.slice(2).map(({ cwd }) => cwd)).toEqual([initialCwd, rememberedCwd]) expect(restored.activeTerminalId).toBe('1') @@ -490,7 +523,7 @@ describe('TerminalRegistry', () => { disposeScope: vi.fn(), } const terminals = new TerminalRegistry(persistence) - terminals.start('chat-deleted', { cols: 80, rows: 24 }) + terminals.openTerminal('chat-deleted') // Suspension accompanies chat deletion: a failed descriptor save must // never leave the deleted chat's shells running invisibly. @@ -507,7 +540,7 @@ describe('TerminalRegistry', () => { disposeScope: vi.fn(), } const terminals = new TerminalRegistry(persistence) - terminals.start('chat-deleted', { cols: 80, rows: 24 }) + terminals.openTerminal('chat-deleted') vi.mocked(persistence.save).mockImplementation(() => { throw new Error('keychain locked') }) @@ -518,8 +551,8 @@ describe('TerminalRegistry', () => { it('routes renderer shortcuts only to the focused terminal scope', () => { const terminals = registry() - terminals.start('chat-A', { cols: 80, rows: 24 }) - terminals.start('chat-B', { cols: 80, rows: 24 }) + terminals.openTerminal('chat-A') + terminals.openTerminal('chat-B') const listeners = new Map void>() const send = vi.fn() const contents = { @@ -545,9 +578,9 @@ describe('TerminalRegistry', () => { it('writes user input only for the visible focused owner and active terminal', () => { const terminals = registry() - const first = terminals.start('chat-A', { cols: 80, rows: 24 }).activeTerminalId as string + const first = terminals.openTerminal('chat-A').activeTerminalId as string const second = terminals.openTerminal('chat-A').activeTerminalId as string - terminals.start('chat-B', { cols: 80, rows: 24 }) + terminals.openTerminal('chat-B') const owner = { isDestroyed: () => false, once: vi.fn(), @@ -571,7 +604,7 @@ describe('TerminalRegistry', () => { it('closes tabs only for the renderer displaying their terminal scope', () => { const terminals = registry() - const first = terminals.start('chat-A', { cols: 80, rows: 24 }).activeTerminalId as string + const first = terminals.openTerminal('chat-A').activeTerminalId as string const second = terminals.openTerminal('chat-A').activeTerminalId as string const owner = { isDestroyed: () => false, diff --git a/apps/desktop/src/main/terminal/registry.ts b/apps/desktop/src/main/terminal/registry.ts index 0dec570e95b..59e91c4d84d 100644 --- a/apps/desktop/src/main/terminal/registry.ts +++ b/apps/desktop/src/main/terminal/registry.ts @@ -131,10 +131,23 @@ export class TerminalRegistry { return this.peekTabs(scope) } - start(scope: string, options: TerminalStartOptions): TerminalTabsState { + /** + * Materializes a chat's saved shells so the renderer can list them as tabs, + * without opening a shell for a chat that had none. Every opened chat is + * hydrated this way; a fresh shell only comes from the user or the agent. + */ + restoreScope(scope: string): TerminalTabsState { if (this.suspendedScopes.has(scope)) return { tabs: [], activeTerminalId: null } const entry = this.entryFor(scope) - return this.restoreOrStart(entry, options) + this.ensureRestored(entry) + return entry.service.getTabs() + } + + /** Applies a pending saved descriptor before anything else touches the scope's shells. */ + private ensureRestored(entry: TerminalRegistryEntry): void { + if (entry.persisted?.tabs.length && !entry.restoreApplied) { + this.restoreOrStart(entry, { cols: 80, rows: 24 }) + } } getScrollback(scope: string, terminalId: string): string { @@ -148,11 +161,18 @@ export class TerminalRegistry { } openTerminal(scope: string, cwd?: string): TerminalTabsState { - return this.serviceFor(scope).openTerminal(cwd) + if (this.suspendedScopes.has(scope)) return { tabs: [], activeTerminalId: null } + const entry = this.entryFor(scope) + this.ensureRestored(entry) + return entry.service.openTerminal(cwd) } - switchTerminal(scope: string, terminalId: string): TerminalTabsState { - return this.serviceFor(scope).switchTerminal(terminalId) + switchTerminal( + scope: string, + terminalId: string, + options?: { claim?: boolean } + ): TerminalTabsState { + return this.serviceFor(scope).switchTerminal(terminalId, options) } reorderTerminal(scope: string, terminalId: string, targetIndex: number): TerminalTabsState { @@ -209,9 +229,7 @@ export class TerminalRegistry { }) } const entry = this.entryFor(scope) - if (entry.persisted && !entry.restoreApplied) { - this.restoreOrStart(entry, { cols: 80, rows: 24 }) - } + this.ensureRestored(entry) return entry.service.executeTool(toolCallId, operation, args) } @@ -394,8 +412,6 @@ export class TerminalRegistry { entry: TerminalRegistryEntry, options: TerminalStartOptions ): TerminalTabsState { - if (entry.restoreApplied) return entry.service.start(options) - const requiredSlots = entry.persisted?.tabs.length ?? 1 const availableSlots = Math.max(0, MAX_TERMINALS_PER_PROCESS - this.liveTerminalCount()) if (requiredSlots > availableSlots) { @@ -487,16 +503,29 @@ export class TerminalRegistry { } private persistTabs(entry: TerminalRegistryEntry, state: TerminalTabsState): boolean { - if (entry.restoring || !this.persistence || state.tabs.length === 0) return true + if (entry.restoring || !this.persistence) return true const tabs = state.tabs.flatMap((tab) => typeof tab.cwd === 'string' && tab.cwd.length > 0 ? [{ cwd: tab.cwd }] : [] ) - if (tabs.length === 0) return true + if (tabs.length === 0) { + // Shells the user closed must stay closed across a relaunch, so a + // descriptor that has been applied is emptied rather than kept. One + // still waiting to be applied is the only copy of those shells. + const pending = Boolean(entry.persisted?.tabs.length) && !entry.restoreApplied + if (pending || !entry.persisted?.tabs.length) return true + return this.save(entry, { v: 1, tabs: [], activeIndex: 0 }) + } const activeIndex = Math.max( 0, state.tabs.findIndex((tab) => tab.terminalId === state.activeTerminalId) ) - return this.persistence.save(entry.scope, { v: 1, tabs, activeIndex }) + return this.save(entry, { v: 1, tabs, activeIndex }) + } + + private save(entry: TerminalRegistryEntry, snapshot: TerminalSessionSnapshot): boolean { + if (!this.persistence?.save(entry.scope, snapshot)) return false + entry.persisted = snapshot + return true } private liveTerminalCount(): number { diff --git a/apps/desktop/src/main/terminal/service.test.ts b/apps/desktop/src/main/terminal/service.test.ts index 60a3060bb79..5e6122a0b8f 100644 --- a/apps/desktop/src/main/terminal/service.test.ts +++ b/apps/desktop/src/main/terminal/service.test.ts @@ -108,17 +108,17 @@ describe('closing terminals', () => { expect(after.activeTerminalId).not.toBe(secondId) }) - it('resets the last terminal instead of emptying the panel', () => { - // A panel whose resource IS a terminal must never be left with no shell: - // there is nothing to show and no way back from inside it. + it('leaves no terminal behind when the last one closes', () => { + // Each shell is its own resource tab, so the strip simply drops the last + // one; nothing is spawned in its place. const terminal = service() const started = terminal.start({ cols: 80, rows: 24 }) const onlyId = started.activeTerminalId as string const after = terminal.closeTerminal(onlyId) - expect(after.tabs).toHaveLength(1) - expect(after.activeTerminalId).not.toBe(onlyId) + expect(after.tabs).toHaveLength(0) + expect(after.activeTerminalId).toBeNull() }) it('refuses to close a terminal that does not exist', () => { @@ -542,6 +542,20 @@ describe('closing', () => { expect(terminal.getTabs().tabs).toHaveLength(2) }) + it('lets the agent close its own shell after the user closed the last one', async () => { + const terminal = service() + const claimed = terminal.start({ cols: 80, rows: 24 }).activeTerminalId as string + terminal.switchTerminal(claimed) + terminal.closeTerminal(claimed) + + const opened = await terminal.executeTool('call-new', 'new', {}) + const agentId = (opened.result as { activeTerminalId: string }).activeTerminalId + const closed = await terminal.executeTool('call-close', 'close', { terminalId: agentId }) + + expect(closed.ok).toBe(true) + expect(terminal.getTabs().tabs).toHaveLength(0) + }) + it('opens and closes an agent terminal without changing visible selection', async () => { const terminal = service() const started = terminal.start({ cols: 80, rows: 24 }) @@ -582,19 +596,18 @@ describe('closing', () => { }) describe('a shell that ends by itself', () => { - it('replaces the only terminal instead of leaving a dead tab', () => { + it('drops the only terminal instead of leaving a dead tab', () => { const terminal = service() const { activeTerminalId } = terminal.start({ cols: 80, rows: 24 }) const original = activeTerminalId as string stubSessions.get(original)?.exit() - // The panel's whole content is the terminal, so an exited last shell used - // to sit there unusable — nothing to type into and no way to get it back. + // An exited shell can no longer do anything, so its tab goes away rather + // than sitting there unusable; the strip is free to offer a new one. const after = terminal.getTabs() - expect(after.tabs).toHaveLength(1) - expect(after.activeTerminalId).not.toBe(original) - expect(after.tabs[0]?.terminalId).toBe(after.activeTerminalId) + expect(after.tabs).toHaveLength(0) + expect(after.activeTerminalId).toBeNull() }) it('removes one of several and activates a neighbour', () => { diff --git a/apps/desktop/src/preload/index.ts b/apps/desktop/src/preload/index.ts index c3714f0f3f4..e4e78d8a19b 100644 --- a/apps/desktop/src/preload/index.ts +++ b/apps/desktop/src/preload/index.ts @@ -49,7 +49,6 @@ import { type ScopedTerminalTabsState, TERMINAL_TOOL_NAME, type TerminalOperation, - type TerminalStartOptions, type TerminalToolArgs, type TerminalToolResponse, } from '@sim/terminal-protocol' @@ -419,20 +418,8 @@ const api: SimDesktopApi = { onFillAvailability: subscribeFillAvailability, }, terminal: { - start: async ( - options: TerminalStartOptions, - scopeId: string - ): Promise => { - const response = (await ipcRenderer.invoke('terminal:start', options, scopeId)) as - | { ok: true; tabs: ScopedTerminalTabsState } - | { ok: false; code?: string; error?: string } - if (!response?.ok) { - const failure = new Error(response?.error ?? 'Could not open a terminal.') - failure.name = response?.code ?? 'SPAWN_FAILED' - throw failure - } - return response.tabs - }, + restoreScope: (scopeId: string): Promise => + ipcRenderer.invoke('terminal:restore-scope', scopeId), // The tool name rides alongside the call because the main process // re-fetches the server's authorized arguments by tool call id and uses // those, not these — what the renderer passes is only a request. @@ -462,8 +449,12 @@ const api: SimDesktopApi = { }, openTerminal: (cwd: string | undefined, scopeId: string): Promise => ipcRenderer.invoke('terminal:open', cwd, scopeId), - switchTerminal: (terminalId: string, scopeId: string): Promise => - ipcRenderer.invoke('terminal:switch', terminalId, scopeId), + switchTerminal: ( + terminalId: string, + scopeId: string, + options?: { claim?: boolean } + ): Promise => + ipcRenderer.invoke('terminal:switch', terminalId, scopeId, options), reorderTerminal: ( terminalId: string, targetIndex: number, diff --git a/apps/sim/app/api/copilot/chat/resources/route.ts b/apps/sim/app/api/copilot/chat/resources/route.ts index 52f5c1e6c5e..b367affa0c6 100644 --- a/apps/sim/app/api/copilot/chat/resources/route.ts +++ b/apps/sim/app/api/copilot/chat/resources/route.ts @@ -23,7 +23,6 @@ import { } from '@/lib/copilot/resources/persistence' import type { MothershipResourceUpdate } from '@/lib/copilot/resources/types' import { - canonicalizeDesktopSessionResource, mergeChatResource, reorderStoredChatResources, sanitizeChatResources, @@ -50,7 +49,7 @@ export const POST = withRouteHandler(async (req: NextRequest) => { ) if (!parsed.success) return parsed.response const { chatId, resource: requestedResource, clearViewId } = parsed.data.body - const resource = canonicalizeDesktopSessionResource(requestedResource) + const resource = requestedResource const resourceUpdate: MothershipResourceUpdate = clearViewId === true ? { ...resource, clearViewId: true } : resource diff --git a/apps/sim/app/api/mothership/chats/[chatId]/fork/route.test.ts b/apps/sim/app/api/mothership/chats/[chatId]/fork/route.test.ts index a5eaa19c66e..4e806387e8c 100644 --- a/apps/sim/app/api/mothership/chats/[chatId]/fork/route.test.ts +++ b/apps/sim/app/api/mothership/chats/[chatId]/fork/route.test.ts @@ -307,7 +307,7 @@ describe('POST /api/mothership/chats/[chatId]/fork', () => { expect(dbChainMockFns.values.mock.calls[0][0].title).toBe('Fork | Generate Logs') }) - it('drops legacy browser rows while forking, since the desktop app owns those pages', async () => { + it('drops legacy browser and terminal rows while forking, since the desktop app owns them', async () => { dbChainMockFns.limit.mockResolvedValue([ { ...parentRow, @@ -318,6 +318,7 @@ describe('POST /api/mothership/chats/[chatId]/fork', () => { title: 'mship-todo (Channel) - sim - Slack', }, { type: 'browser', id: 'browser-session', title: 'Browser' }, + { type: 'terminal', id: 'terminal-session', title: 'Terminal' }, { type: 'file', id: 'file-1', title: 'report.csv' }, ], }, diff --git a/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/add-resource-dropdown/add-resource-dropdown.tsx b/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/add-resource-dropdown/add-resource-dropdown.tsx index f90203afeac..3f6b6dbbe62 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/add-resource-dropdown/add-resource-dropdown.tsx +++ b/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/add-resource-dropdown/add-resource-dropdown.tsx @@ -18,7 +18,6 @@ import { } from '@sim/emcn' import { Folder, Plus } from '@sim/emcn/icons' import { isBrowserAgentAvailable } from '@/lib/browser-agent/transport' -import { TERMINAL_SESSION_RESOURCE_ID } from '@/lib/copilot/resources/types' import { subscribeDesktopPreferences } from '@/lib/desktop' import { isTerminalAvailable } from '@/lib/terminal/transport' import { @@ -56,11 +55,12 @@ import { useWorkspaceFiles } from '@/hooks/queries/workspace-files' */ export const BROWSER_LAUNCHER_ID = 'browser' +/** Placeholder id for the Terminal launcher row; the shell the desktop app opens becomes the tab. */ +export const TERMINAL_LAUNCHER_ID = 'terminal' + export interface AddResourceDropdownProps { workspaceId: string - existingKeys: Set onAdd: (resource: MothershipResource) => void - onOpenExisting?: (resource: MothershipResource) => void /** * Resource types to hide from the dropdown. Must be referentially stable * (a module constant) — it keys the underlying group memo. @@ -326,7 +326,7 @@ export function useAvailableResources( type: 'terminal' as const, items: [ { - id: TERMINAL_SESSION_RESOURCE_ID, + id: TERMINAL_LAUNCHER_ID, name: 'Terminal', }, ], @@ -544,10 +544,13 @@ export function ResourceMenuSections({ const Icon = config.icon const section = sectionByType.get(type) - // The Browser launcher and the Terminal panel are flat rows: one opens - // a new page, the other the terminal and its inner shells. Live browser - // pages offered as context are an ordinary picker submenu. - if (!section && (type === 'terminal' || items[0]?.id === BROWSER_LAUNCHER_ID)) { + // The Browser and Terminal launchers are flat rows that open a new page + // or shell. Live pages and shells offered as context are an ordinary + // picker submenu. + if ( + !section && + (items[0]?.id === BROWSER_LAUNCHER_ID || items[0]?.id === TERMINAL_LAUNCHER_ID) + ) { const item = items[0] return ( onSelect(resourceFromItem(type, item))}> @@ -591,9 +594,7 @@ export function ResourceMenuSections({ export function AddResourceDropdown({ workspaceId, - existingKeys, onAdd, - onOpenExisting, excludeTypes, onRequestOpen, onClose, @@ -642,13 +643,7 @@ export function AddResourceDropdown({ } const select = (resource: MothershipResource) => { - void closeMenu().then(() => { - if (onOpenExisting && existingKeys.has(`${resource.type}:${resource.id}`)) { - onOpenExisting(resource) - } else { - onAdd(resource) - } - }) + void closeMenu().then(() => onAdd(resource)) } const filtered = useMemo(() => { diff --git a/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/components/terminal-session/terminal-session.test.ts b/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/components/terminal-session/terminal-session.test.ts index 345b4b86dff..4e5fcb69095 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/components/terminal-session/terminal-session.test.ts +++ b/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/components/terminal-session/terminal-session.test.ts @@ -5,61 +5,11 @@ import { resolveDesktopZoom } from '@sim/desktop-bridge' import { describe, expect, it } from 'vitest' import { - shouldRemoveTerminalResource, terminalFontSizeForZoom, terminalSelectionLabel, terminalSelectionSnapshot, - terminalTooltip, } from '@/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/components/terminal-session/terminal-session' -describe('terminal tab tooltips', () => { - it('summarizes a long compound heredoc command by its foreground program', () => { - const running = `mkdir -p ~/.doordash-bot/bin && cat > ~/.doordash-bot/bin/dd-cli-mock <<'EOF' -#!/usr/bin/env node -const carts = new Map() -process.stdout.write(JSON.stringify([...carts])) -EOF -chmod +x ~/.doordash-bot/bin/dd-cli-mock && echo '--- smoke test ---' && ~/.doordash-bot/bin/dd-cli-mock submit mock_123` - const tooltip = terminalTooltip({ - terminalId: 'terminal-1', - title: 'mkdir', - cwd: '/Users/emirkarabeg', - running, - interactive: false, - active: false, - }) - - expect(tooltip).toBe('/Users/emirkarabeg — dd-cli-mock') - expect(tooltip).not.toContain('const carts') - }) - - it('preserves the working-directory tooltip for idle terminals', () => { - const idleTab = { - terminalId: 'terminal-1', - title: 'sim', - cwd: '/Users/emirkarabeg/sim', - running: null, - interactive: false, - active: true, - } - - expect(terminalTooltip(idleTab)).toBe('/Users/emirkarabeg/sim') - expect(terminalTooltip({ ...idleTab, cwd: null })).toBe('Terminal') - }) -}) - -describe('suspended terminal resource lifecycle', () => { - it('does not remove a resource when administrative suspension clears its PTYs', () => { - expect(shouldRemoveTerminalResource(0, true, true)).toBe(false) - expect(shouldRemoveTerminalResource(0, true, false)).toBe(true) - }) - - it('keeps resources that never observed a live PTY', () => { - expect(shouldRemoveTerminalResource(0, false, false)).toBe(false) - expect(shouldRemoveTerminalResource(1, true, false)).toBe(false) - }) -}) - describe('terminal resource zoom', () => { it('scales xterm from its 12px actual size', () => { expect(terminalFontSizeForZoom(100)).toBe(12) diff --git a/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/components/terminal-session/terminal-session.tsx b/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/components/terminal-session/terminal-session.tsx index 2a735f3c53a..3b058443c8b 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/components/terminal-session/terminal-session.tsx +++ b/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/components/terminal-session/terminal-session.tsx @@ -1,15 +1,6 @@ 'use client' -import { - memo, - type DragEvent as ReactDragEvent, - type MouseEvent as ReactMouseEvent, - useCallback, - useEffect, - useMemo, - useRef, - useState, -} from 'react' +import { memo, useCallback, useEffect, useMemo, useRef, useState } from 'react' import { type DesktopZoomAction, type DesktopZoomPercent, @@ -18,15 +9,7 @@ import { type TerminalShortcutCommand, type TerminalThemeProfile, } from '@sim/desktop-bridge' -import { - cn, - NATIVE_SURFACE_OCCLUSION_PREPARE_EVENT, - TabStrip, - type TabStripItem, - type TabStripSelectionSource, - toast, -} from '@sim/emcn' -import { TerminalWindow } from '@sim/emcn/icons' +import { cn, NATIVE_SURFACE_OCCLUSION_PREPARE_EVENT, toast } from '@sim/emcn' import { createLogger } from '@sim/logger' import { formatPasteLimit, PASTE_LIMITS } from '@sim/utils/paste' import { FitAddon } from '@xterm/addon-fit' @@ -37,13 +20,7 @@ import { type IBufferRange, Terminal } from '@xterm/xterm' import { useTheme } from 'next-themes' import { useContextMenu } from '@/hooks/use-context-menu' import '@xterm/xterm/css/xterm.css' -import { - describeRunningCommand, - type TerminalTabState, - type TerminalTabsState, -} from '@sim/terminal-protocol' -import { SIM_RESOURCE_DRAG_TYPE } from '@/lib/copilot/resource-types' -import { TERMINAL_SESSION_RESOURCE_ID } from '@/lib/copilot/resources/types' +import { describeRunningCommand, type TerminalTabsState } from '@sim/terminal-protocol' import { getDesktopBridge } from '@/lib/desktop' import { loadDesktopTerminalAppearance, @@ -54,6 +31,7 @@ import { } from '@/lib/desktop/appearance' import { trackPanelFocus } from '@/lib/desktop/panel-focus' import { addMothershipContext } from '@/lib/mothership/events' +import { onTerminalFocusRequest } from '@/lib/terminal/focus' import { clearTerminalScrollback, closeTerminal, @@ -63,25 +41,18 @@ import { onTerminalShortcutCommand, openTerminal, pasteIntoTerminal, - reorderTerminal, reportTerminalFocused, reportTerminalVisible, resizeTerminal, - startTerminalSession, - switchTerminal, writeToTerminal, } from '@/lib/terminal/transport' -import { useMothershipResources } from '@/app/workspace/[workspaceId]/home/components/mothership-resources-context' import { TerminalContextMenu } from '@/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/components/terminal-session/terminal-context-menu' -import { TerminalTabIcon } from '@/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/components/terminal-session/terminal-tab-icon' -import { ContextMenu } from '@/app/workspace/[workspaceId]/w/components/sidebar/components/workflow-list/components/context-menu/context-menu' import { useDesktopPreferenceMutation } from '@/hooks/use-desktop-preference-mutation' import { useCopilotTerminalStore } from '@/stores/copilot-terminal/store' import type { ChatContext, TerminalTextSelection } from '@/stores/panel' const logger = createLogger('TerminalSession') const EMPTY_TERMINAL_TABS: TerminalTabsState = { tabs: [], activeTerminalId: null } -const EMPTY_AGENT_COMMAND_TERMINAL_IDS: Record = {} const TERMINAL_BASE_FONT_SIZE = 12 const TERMINAL_ZOOM_BOUNDS = { min: 50, max: 300 } as const @@ -105,78 +76,6 @@ function hideMountedMenuSurfaces(): void { } } -/** - * How long a command must run before the tab names it. - * - * A tab that says what it is busy with is useful for a build you left running - * in the background, and pure noise for `ls` — swapping the label and spinning - * the icon for thirty milliseconds reads as a glitch. Waiting a beat keeps the - * signal and drops the flicker. - */ -const COMMAND_SETTLE_MS = 1_000 - -/** Full working directory, plus a concise name for whatever the shell is running. */ -export function terminalTooltip(tab: TerminalTabState): string { - const where = tab.cwd ?? 'Terminal' - return tab.running ? `${where} — ${describeRunningCommand(tab.running)}` : where -} - -function sameIds(a: ReadonlySet, b: ReadonlySet): boolean { - return a.size === b.size && [...a].every((id) => b.has(id)) -} - -/** - * Whether a tab should be named after what it is running rather than where it - * is. A full-screen program is named the moment it appears: the delay exists - * to stop `ls` flickering the label, and an editor or coding agent is not a - * transient command — it holds the terminal until it is quit, so there is - * nothing to wait out. - */ -function namesItsCommand(tab: TerminalTabState, settled: ReadonlySet): boolean { - return Boolean(tab.running) && (tab.interactive || settled.has(tab.terminalId)) -} - -/** - * The terminals whose command has been running long enough to show. Returns a - * stable set, so a tab strip that would render identically does not re-render. - */ -function useSettledCommands(tabs: TerminalTabState[]): ReadonlySet { - const [settled, setSettled] = useState>(() => new Set()) - const startedAt = useRef(new Map()) - - useEffect(() => { - const started = startedAt.current - const live = new Set(tabs.map((tab) => tab.terminalId)) - for (const id of [...started.keys()]) { - if (!live.has(id)) started.delete(id) - } - for (const tab of tabs) { - if (!tab.running) started.delete(tab.terminalId) - else if (!started.has(tab.terminalId)) started.set(tab.terminalId, Date.now()) - } - - const recompute = () => { - const now = Date.now() - const next = new Set() - let soonest = Number.POSITIVE_INFINITY - for (const [id, at] of started) { - const elapsed = now - at - if (elapsed >= COMMAND_SETTLE_MS) next.add(id) - else soonest = Math.min(soonest, COMMAND_SETTLE_MS - elapsed) - } - setSettled((current) => (sameIds(current, next) ? current : next)) - return soonest - } - - const soonest = recompute() - if (!Number.isFinite(soonest)) return - const timer = setTimeout(recompute, Math.max(0, soonest)) - return () => clearTimeout(timer) - }, [tabs]) - - return settled -} - /** * How long the panel must stop changing size before the PTY is told about it. * Long enough to cover a divider drag, short enough that a deliberate resize @@ -692,10 +591,6 @@ const TerminalView = memo(function TerminalView({ }, [scopeId]) // Scoped to the terminal that was right-clicked, not the active one. - // Offered even for the only terminal: closing the last one restarts its - // shell in place rather than removing the tab, so there is always something - // for the action to do — and hiding it here while the tab strip's own close - // stays available would just be the two menus disagreeing. const closeThisTerminal = useCallback(() => { if ( running && @@ -760,15 +655,6 @@ interface TerminalSessionProps { scopeId: string } -/** Administrative suspension retains the resource even though live PTYs are gone. */ -export function shouldRemoveTerminalResource( - tabCount: number, - hasStarted: boolean, - suspended: boolean -): boolean { - return !suspended && tabCount === 0 && hasStarted -} - export function TerminalSession({ visible, scopeId }: TerminalSessionProps) { const panelRef = useRef(null) const [appearanceTheme, setAppearanceTheme] = useState('app') @@ -781,20 +667,7 @@ export function TerminalSession({ visible, scopeId }: TerminalSessionProps) { (state) => state.sessions[scopeId]?.tabs ?? EMPTY_TERMINAL_TABS ) const suspended = useCopilotTerminalStore((state) => state.sessions[scopeId]?.suspended ?? false) - const agentCommandTerminalIds = useCopilotTerminalStore( - (state) => state.sessions[scopeId]?.agentCommandTerminalIds ?? EMPTY_AGENT_COMMAND_TERMINAL_IDS - ) - const activityResetEpoch = useCopilotTerminalStore( - (state) => state.sessions[scopeId]?.activityResetEpoch ?? 0 - ) const { tabs, activeTerminalId } = tabsState - const agentCommandTargets = useMemo( - () => new Set(Object.values(agentCommandTerminalIds)), - [agentCommandTerminalIds] - ) - const settledCommands = useSettledCommands(tabs) - const { removeResource } = useMothershipResources() - const [startError, setStartError] = useState(null) const [focusRequest, setFocusRequest] = useState({ terminalId: '', nonce: 0 }) const availableProfiles = useMemo( () => withSelectedProfile(profiles, appearanceTheme), @@ -857,271 +730,18 @@ export function TerminalSession({ visible, scopeId }: TerminalSessionProps) { return () => reportTerminalVisible(false, scopeId) }, [scopeId, suspended, visible]) - useEffect(() => { - if (suspended) { - setStartError(null) - return - } - let active = true - startTerminalSession({ cols: 80, rows: 24 }, scopeId) - .then(() => { - if (active) setStartError(null) - }) - .catch((error: Error) => { - if (active) setStartError(error.message) - }) - return () => { - active = false - } - }, [scopeId, suspended]) - - // Closing the last terminal closes the panel: there is nothing left to show - // and no way back from inside it. - const hasStarted = useRef(false) - useEffect(() => { - if (suspended) { - hasStarted.current = false - return - } - if (tabs.length > 0) { - hasStarted.current = true - return - } - if (shouldRemoveTerminalResource(tabs.length, hasStarted.current, suspended)) { - hasStarted.current = false - removeResource('terminal', TERMINAL_SESSION_RESOURCE_ID) - } - }, [tabs.length, suspended, removeResource]) - - // Shell process state is not activity state: a coding tool can run for hours. - // The agent-command lifecycle is precise, though, so the targeted terminal - // replaces its regular glyph while Mothership is actively driving it. - const items = useMemo(() => { - const labels = tabs.map((tab) => - namesItsCommand(tab, settledCommands) ? (tab.running ?? tab.title) : tab.title - ) - const counts = new Map() - for (const label of labels) counts.set(label, (counts.get(label) ?? 0) + 1) - const occurrences = new Map() - return tabs.map((tab, index) => { - const label = labels[index] - const occurrence = (occurrences.get(label) ?? 0) + 1 - occurrences.set(label, occurrence) - const isAgentCommandRunning = agentCommandTargets.has(tab.terminalId) - return { - id: tab.terminalId, - title: counts.get(label) === 1 ? label : `${label} ${occurrence}`, - // The label is a basename, and the tab may be running something it - // is not naming yet, so hovering identifies the working directory and - // foreground program without exposing the literal command. - tooltip: terminalTooltip(tab), - icon: ( - - ), - active: tab.terminalId === activeTerminalId, - } - }) - }, [tabs, activeTerminalId, agentCommandTargets, activityResetEpoch, settledCommands]) - - const [contextTerminalId, setContextTerminalId] = useState(null) - const { - isOpen: isContextMenuOpen, - position: contextMenuPosition, - menuRef: contextMenuRef, - handleContextMenu, - closeMenu: closeContextMenu, - } = useContextMenu() - - useEffect(() => { - if (!visible) return - const handlePrepare = () => { - if (isContextMenuOpen || contextMenuRef.current) hideMountedMenuSurfaces() - if (isContextMenuOpen) closeContextMenu() - } - window.addEventListener(NATIVE_SURFACE_OCCLUSION_PREPARE_EVENT, handlePrepare) - return () => window.removeEventListener(NATIVE_SURFACE_OCCLUSION_PREPARE_EVENT, handlePrepare) - }, [closeContextMenu, isContextMenuOpen, visible]) - const contextTab = tabs.find((tab) => tab.terminalId === contextTerminalId) - const canReorderTabs = Boolean(getDesktopBridge()?.terminal.reorderTerminal) - - useEffect(() => { - if (isContextMenuOpen && contextTerminalId && !contextTab) { - setContextTerminalId(null) - closeContextMenu() - } - }, [closeContextMenu, contextTab, contextTerminalId, isContextMenuOpen]) - - const handleNew = useCallback(() => { - void openTerminal(undefined, scopeId) - .then((state) => { - if (state.activeTerminalId) { - setFocusRequest((current) => ({ - terminalId: state.activeTerminalId ?? '', - nonce: current.nonce + 1, - })) - } - }) - .catch(() => { - toast.error('Could not open a new terminal. Please try again.') - }) - }, [scopeId]) - const handleSwitch = useCallback( - (terminalId: string, source?: TabStripSelectionSource) => { - if (source !== 'keyboard') { + // The strip asks for the keyboard when the user picks a shell with the + // pointer or opens one; the request lands once that shell is on screen. + useEffect( + () => + onTerminalFocusRequest((terminalId) => { setFocusRequest((current) => ({ terminalId, nonce: current.nonce + 1 })) - } - void switchTerminal(terminalId, scopeId).catch(() => { - toast.error('Could not switch terminals. Please try again.') - }) - }, - [scopeId] - ) - const handleReorder = useCallback( - (terminalId: string, targetIndex: number) => { - void reorderTerminal(terminalId, targetIndex, scopeId).catch(() => { - toast.error('Could not reorder that terminal. Please try again.') - }) - }, - [scopeId] - ) - // Closing the only terminal resets it rather than emptying the panel; the - // desktop app decides that, so the button means the same thing at any count. - const handleClose = useCallback( - (terminalId: string) => { - const tab = tabs.find((entry) => entry.terminalId === terminalId) - if ( - tab?.running && - !window.confirm( - `${describeRunningCommand(tab.running)} is still running. Close this terminal and stop it?` - ) - ) { - return - } - void closeTerminal(terminalId, scopeId).catch(() => { - toast.error('Could not close that terminal. Please try again.') - }) - }, - [scopeId, tabs] - ) - - // A duplicate is a new shell in the same directory, not a copy of the - // session: scrollback and whatever is running belong to the original pty. - const handleDuplicate = useCallback( - (cwd: string | null) => { - void openTerminal(cwd ?? undefined, scopeId) - .then((state) => { - if (state.activeTerminalId) { - setFocusRequest((current) => ({ - terminalId: state.activeTerminalId ?? '', - nonce: current.nonce + 1, - })) - } - }) - .catch(() => { - toast.error('Could not duplicate that terminal. Please try again.') - }) - }, - [scopeId] - ) - - const handleCloseMany = useCallback( - (terminalIds: string[]) => { - const runningCount = tabs.filter( - (tab) => terminalIds.includes(tab.terminalId) && Boolean(tab.running) - ).length - if ( - runningCount > 0 && - !window.confirm( - `${runningCount} selected ${runningCount === 1 ? 'terminal has' : 'terminals have'} a running process. Close ${runningCount === 1 ? 'it' : 'them'} anyway?` - ) - ) { - return - } - for (const terminalId of terminalIds) { - void closeTerminal(terminalId, scopeId).catch(() => { - toast.error('Could not close one of those terminals. Please try again.') - }) - } - }, - [scopeId, tabs] - ) - - const closeOtherTabs = useCallback(() => { - if (!contextTab) return - handleCloseMany( - tabs.filter((tab) => tab.terminalId !== contextTab.terminalId).map((tab) => tab.terminalId) - ) - }, [contextTab, handleCloseMany, tabs]) - - const closeTabsToRight = useCallback(() => { - if (!contextTab) return - const contextIndex = tabs.findIndex((tab) => tab.terminalId === contextTab.terminalId) - handleCloseMany(tabs.slice(contextIndex + 1).map((tab) => tab.terminalId)) - }, [contextTab, handleCloseMany, tabs]) - - const contextIndex = contextTab - ? tabs.findIndex((tab) => tab.terminalId === contextTab.terminalId) - : -1 - - // A terminal can move inside the strip or be copied into chat as context. - const startTabDrag = useCallback( - (event: ReactDragEvent, terminalId: string) => { - const tab = tabs.find((entry) => entry.terminalId === terminalId) - if (!tab) return - event.dataTransfer.effectAllowed = 'copyMove' - event.dataTransfer.setData( - SIM_RESOURCE_DRAG_TYPE, - JSON.stringify({ type: 'terminal', id: tab.terminalId, title: tab.title }) - ) - }, - [tabs] - ) - - const openTabContextMenu = useCallback( - (event: ReactMouseEvent, terminalId: string) => { - window.getSelection()?.removeAllRanges() - setContextTerminalId(terminalId) - handleContextMenu(event) - }, - [handleContextMenu] + }), + [] ) return (
- handleDuplicate(contextTab.cwd) : undefined} - onCloseOtherTabs={contextTab ? closeOtherTabs : undefined} - onCloseTabsToRight={contextTab ? closeTabsToRight : undefined} - disableCloseOtherTabs={tabs.length <= 1} - disableCloseTabsToRight={contextIndex < 0 || contextIndex === tabs.length - 1} - {...(contextTab - ? { onCloseTab: () => handleClose(contextTab.terminalId), showCloseTab: true } - : {})} - onDelete={() => {}} - showRename={false} - showDuplicate={Boolean(contextTab)} - showDelete={false} - /> - } - /> -
{tabs.map((tab) => ( ))} - {startError && ( -
- -

{startError}

-
- )}
) diff --git a/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/components/terminal-session/terminal-tab-icon.tsx b/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/components/terminal-session/terminal-tab-icon.tsx deleted file mode 100644 index 5b88cc2ba1d..00000000000 --- a/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/components/terminal-session/terminal-tab-icon.tsx +++ /dev/null @@ -1,23 +0,0 @@ -'use client' - -import { TerminalWindow } from '@sim/emcn/icons' -import { ThinkingLoader } from '@/components/ui' -import { useStableFlag } from '@/hooks/use-stable-flag' - -const TERMINAL_ACTIVITY_MIN_VISIBLE_MS = 1_000 - -interface TerminalTabIconProps { - active: boolean -} - -/** Keeps brief terminal activity visible long enough to register. */ -export function TerminalTabIcon({ active }: TerminalTabIconProps) { - const visible = useStableFlag(active, { minVisibleMs: TERMINAL_ACTIVITY_MIN_VISIBLE_MS }) - return visible ? ( - - - - ) : ( - - ) -} diff --git a/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/resource-content.tsx b/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/resource-content.tsx index c06afcbdd37..a3d8cecd74e 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/resource-content.tsx +++ b/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/resource-content.tsx @@ -347,7 +347,9 @@ export const ResourceContent = memo(function ResourceContent({ ) case 'terminal': - return + // One panel serves every terminal tab of the chat, keeping each shell's + // emulator alive across tab switches. + return default: return null diff --git a/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-registry/resource-registry.tsx b/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-registry/resource-registry.tsx index 71ea6e7ac65..74713bd7e2d 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-registry/resource-registry.tsx +++ b/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-registry/resource-registry.tsx @@ -16,7 +16,9 @@ import { } from '@sim/emcn/icons' import type { QueryClient } from '@tanstack/react-query' import { getDocumentIcon } from '@/components/icons/document-icons' +import { terminalIdFromResourceId } from '@/lib/terminal/resource-id' import { BrowserTabIcon } from '@/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-registry/browser-tab-icon' +import { TerminalTabIcon } from '@/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-registry/terminal-tab-icon' import type { MothershipResource, MothershipResourceType, @@ -148,8 +150,12 @@ export const RESOURCE_REGISTRY: Record ( - + renderTabIcon: (resource, className, desktopScopeId) => ( + ), renderDropdownItem: (props) => , }, diff --git a/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/components/terminal-session/terminal-tab-icon.test.tsx b/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-registry/terminal-tab-icon.test.tsx similarity index 76% rename from apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/components/terminal-session/terminal-tab-icon.test.tsx rename to apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-registry/terminal-tab-icon.test.tsx index 0c7d4f49ee6..fb47f96f555 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/components/terminal-session/terminal-tab-icon.test.tsx +++ b/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-registry/terminal-tab-icon.test.tsx @@ -4,7 +4,8 @@ import { act } from 'react' import { createRoot, type Root } from 'react-dom/client' import { afterEach, describe, expect, it, vi } from 'vitest' -import { TerminalTabIcon } from '@/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/components/terminal-session/terminal-tab-icon' +import { TerminalTabIcon } from '@/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-registry/terminal-tab-icon' +import { useCopilotTerminalStore } from '@/stores/copilot-terminal/store' vi.mock('@/components/ui', async () => { const { createElement } = await import('react') @@ -16,6 +17,9 @@ vi.mock('@/components/ui', async () => { let root: Root | null = null let container: HTMLDivElement | null = null +const SCOPE = 'chat-1' + +/** Drives the icon the way the store does: an agent command targeting the shell, and reset epochs. */ function render(active: boolean, resetEpoch = 0): void { ;(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true if (!container) { @@ -23,7 +27,20 @@ function render(active: boolean, resetEpoch = 0): void { document.body.appendChild(container) root = createRoot(container) } - act(() => root?.render()) + act(() => { + useCopilotTerminalStore.setState({ + activeScopeId: SCOPE, + sessions: { + [SCOPE]: { + tabs: { tabs: [], activeTerminalId: null }, + agentCommandTerminalIds: active ? { 'tool-1': '7' } : {}, + activityResetEpoch: resetEpoch, + suspended: false, + }, + }, + }) + root?.render() + }) } afterEach(() => { diff --git a/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-registry/terminal-tab-icon.tsx b/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-registry/terminal-tab-icon.tsx new file mode 100644 index 00000000000..9e7c03ce127 --- /dev/null +++ b/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-registry/terminal-tab-icon.tsx @@ -0,0 +1,52 @@ +'use client' + +import { cn } from '@sim/emcn' +import { TerminalWindow } from '@sim/emcn/icons' +import { ThinkingLoader } from '@/components/ui' +import { useStableFlag } from '@/hooks/use-stable-flag' +import { useCopilotTerminalStore } from '@/stores/copilot-terminal/store' + +const TERMINAL_ACTIVITY_MIN_VISIBLE_MS = 1_000 + +interface TerminalTabIconProps { + /** Native terminal id, which is also the terminal resource's id. */ + terminalId: string + /** Desktop terminal scope the shell lives in; without one the icon is a plain glyph. */ + scopeId?: string + className?: string +} + +interface TerminalActivityIconProps { + active: boolean + className?: string +} + +/** Keeps brief terminal activity visible long enough to register. */ +function TerminalActivityIcon({ active, className }: TerminalActivityIconProps) { + const visible = useStableFlag(active, { minVisibleMs: TERMINAL_ACTIVITY_MIN_VISIBLE_MS }) + return visible ? ( + + + + ) : ( + + ) +} + +/** + * Resource-strip icon for one live shell. Shell process state is not activity + * state — a coding tool can run for hours — but the agent-command lifecycle is + * precise, so the targeted terminal replaces its glyph with the thinking loader + * while the agent is driving it. The activity epoch remounts the loader so a + * hard reset at a stream boundary settles the chrome at once. + */ +export function TerminalTabIcon({ terminalId, scopeId, className }: TerminalTabIconProps) { + const active = useCopilotTerminalStore((state) => { + const session = scopeId ? state.sessions[scopeId] : undefined + return session ? Object.values(session.agentCommandTerminalIds).includes(terminalId) : false + }) + const activityResetEpoch = useCopilotTerminalStore((state) => + scopeId ? (state.sessions[scopeId]?.activityResetEpoch ?? 0) : 0 + ) + return +} diff --git a/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-tabs/resource-tabs.test.ts b/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-tabs/resource-tabs.test.ts deleted file mode 100644 index 6a872d25f33..00000000000 --- a/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-tabs/resource-tabs.test.ts +++ /dev/null @@ -1,53 +0,0 @@ -import { beforeEach, describe, expect, it, vi } from 'vitest' - -const { mockOpenTerminal, mockSendBrowserPanelAction } = vi.hoisted(() => ({ - mockOpenTerminal: vi.fn(), - mockSendBrowserPanelAction: vi.fn(), -})) - -vi.mock('@/lib/browser-agent/transport', () => ({ - isBrowserAgentAvailable: vi.fn(() => true), - openBrowserTab: vi.fn(), - sendBrowserPanelAction: mockSendBrowserPanelAction, -})) - -vi.mock('@/lib/terminal/transport', () => ({ - isTerminalAvailable: vi.fn(() => true), - openTerminal: mockOpenTerminal, -})) - -import { openExistingResourceTab } from './resource-tabs' - -describe('openExistingResourceTab', () => { - beforeEach(() => { - vi.clearAllMocks() - }) - - it('selects an existing terminal resource and opens a new terminal tab', () => { - const selectResource = vi.fn() - - openExistingResourceTab( - { type: 'terminal', id: 'terminal-session', title: 'Terminal' }, - 'chat-2', - selectResource - ) - - expect(selectResource).toHaveBeenCalledWith('terminal-session') - expect(mockOpenTerminal).toHaveBeenCalledWith(undefined, 'chat-2') - expect(mockSendBrowserPanelAction).not.toHaveBeenCalled() - }) - - it('only selects other existing resource types', () => { - const selectResource = vi.fn() - - openExistingResourceTab( - { type: 'workflow', id: 'workflow-1', title: 'Workflow' }, - 'chat-3', - selectResource - ) - - expect(selectResource).toHaveBeenCalledWith('workflow-1') - expect(mockSendBrowserPanelAction).not.toHaveBeenCalled() - expect(mockOpenTerminal).not.toHaveBeenCalled() - }) -}) diff --git a/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-tabs/resource-tabs.tsx b/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-tabs/resource-tabs.tsx index dc1adc0e509..6ce74c4a141 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-tabs/resource-tabs.tsx +++ b/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-tabs/resource-tabs.tsx @@ -21,6 +21,7 @@ import { toast, } from '@sim/emcn' import { Columns3, Eye, Pencil } from '@sim/emcn/icons' +import { describeRunningCommand, type TerminalTabState } from '@sim/terminal-protocol' import { browserTabTitle } from '@/lib/browser-agent/tab-label' import { openBrowserTab, @@ -29,7 +30,10 @@ import { } from '@/lib/browser-agent/transport' import { SIM_RESOURCE_DRAG_TYPE, SIM_RESOURCES_DRAG_TYPE } from '@/lib/copilot/resource-types' import { isEphemeralResource } from '@/lib/copilot/resources/types' -import { openTerminal } from '@/lib/terminal/transport' +import { requestTerminalFocus } from '@/lib/terminal/focus' +import { terminalIdFromResourceId, terminalResourceId } from '@/lib/terminal/resource-id' +import { terminalTabTitle, terminalTooltip } from '@/lib/terminal/tab-label' +import { closeTerminal, openTerminal, reorderTerminal } from '@/lib/terminal/transport' import type { PreviewMode } from '@/app/workspace/[workspaceId]/files/components/file-viewer' import { useMothershipResources } from '@/app/workspace/[workspaceId]/home/components/mothership-resources-context' import { AddResourceDropdown } from '@/app/workspace/[workspaceId]/home/components/mothership-view/components/add-resource-dropdown' @@ -54,19 +58,9 @@ import { import { useTablesList } from '@/hooks/queries/tables' import { useWorkflows } from '@/hooks/queries/workflows' import { useWorkspaceFiles } from '@/hooks/queries/workspace-files' +import { useSettledTerminalCommands } from '@/hooks/use-settled-terminal-commands' import { useBrowserSessionStore } from '@/stores/browser-session/store' - -/** Opens another inner tab when the singleton terminal resource already exists. */ -export function openExistingResourceTab( - resource: MothershipResource, - desktopScopeId: string, - selectResource: (id: string) => void -): void { - selectResource(resource.id) - if (resource.type === 'terminal') { - void openTerminal(undefined, desktopScopeId) - } -} +import { useCopilotTerminalStore } from '@/stores/copilot-terminal/store' /** * Types that cannot be opened as a resource tab. Folders and chats have no tab @@ -81,6 +75,26 @@ const ADD_RESOURCE_EXCLUDED_TYPES: readonly MothershipResourceType[] = [ 'integration', ] as const +const EMPTY_TERMINAL_TABS: TerminalTabState[] = [] + +/** Closing a shell mid-command stops that command, so the user confirms first. */ +function confirmClosingRunningTerminals( + targets: readonly MothershipResource[], + terminalTabs: readonly TerminalTabState[] +): boolean { + const running = targets.flatMap((resource) => { + if (resource.type !== 'terminal') return [] + const tab = terminalTabs.find((entry) => terminalResourceId(entry.terminalId) === resource.id) + return tab?.running ? [tab.running] : [] + }) + if (running.length === 0) return true + return window.confirm( + running.length === 1 + ? `${describeRunningCommand(running[0])} is still running. Close this terminal and stop it?` + : `${running.length} selected terminals have a running process. Close them anyway?` + ) +} + /** * Returns the id of the nearest resource to `idx` that is in `filter` * (or any resource if `filter` is null). Returns undefined if nothing qualifies. @@ -252,37 +266,59 @@ export function ResourceTabs({ anchorIdRef.current = null } - const existingKeys = useMemo( - () => new Set(resources.map((r) => `${r.type}:${r.id}`)), - [resources] - ) - - // A browser tab's title is the live page title, owned by the desktop app. + // Browser and terminal tab titles are live page and shell state, owned by + // the desktop app; a terminal is named after its settled foreground program. const browserTabs = useBrowserSessionStore((state) => state.sessions[desktopScopeId]?.tabs) + const terminalTabs = useCopilotTerminalStore( + (state) => state.sessions[desktopScopeId]?.tabs.tabs ?? EMPTY_TERMINAL_TABS + ) + const settledCommands = useSettledTerminalCommands(terminalTabs) const tabs = useMemo(() => { const browserTitles = new Map(browserTabs?.map((tab) => [tab.tabId, browserTabTitle(tab)])) - return resources.map((resource) => ({ - id: resource.id, - title: - (resource.type === 'browser' - ? browserTitles.get(resource.id) - : nameLookup.get(`${resource.type}:${resource.id}`)) ?? resource.title, - icon: getResourceConfig(resource.type).renderTabIcon( - resource, - 'size-[16px] shrink-0', - desktopScopeId - ), - active: activeId === resource.id, - selected: selectedIds.size > 1 && selectedIds.has(resource.id), - attention: activityIds?.has(resource.id) ?? false, - })) - }, [resources, nameLookup, browserTabs, desktopScopeId, activeId, selectedIds, activityIds]) + const terminalsById = new Map( + terminalTabs.map((tab) => [terminalResourceId(tab.terminalId), tab]) + ) + return resources.map((resource) => { + const terminal = resource.type === 'terminal' ? terminalsById.get(resource.id) : undefined + return { + id: resource.id, + title: + (resource.type === 'browser' + ? browserTitles.get(resource.id) + : terminal + ? terminalTabTitle(terminal, settledCommands) + : nameLookup.get(`${resource.type}:${resource.id}`)) ?? resource.title, + // A shell's label is a basename, and it may be running something it is + // not naming yet, so hovering identifies the directory and program. + ...(terminal ? { tooltip: terminalTooltip(terminal) } : {}), + icon: getResourceConfig(resource.type).renderTabIcon( + resource, + 'size-[16px] shrink-0', + desktopScopeId + ), + active: activeId === resource.id, + selected: selectedIds.size > 1 && selectedIds.has(resource.id), + attention: activityIds?.has(resource.id) ?? false, + } + }) + }, [ + resources, + nameLookup, + browserTabs, + terminalTabs, + settledCommands, + desktopScopeId, + activeId, + selectedIds, + activityIds, + ]) const handleAdd = useCallback( (resource: MothershipResource) => { - // A browser tab is a live page the desktop app creates; it joins the - // strip through the tab list rather than as a resource of its own. + // A browser tab or terminal is a live page or shell the desktop app + // creates; it joins the strip through the tab list rather than as a + // resource of its own. if (resource.type === 'browser') { void openBrowserTab(desktopScopeId) .then((state) => { @@ -291,6 +327,16 @@ export function ResourceTabs({ .catch(() => toast.error('Could not open a new browser tab. Please try again.')) return } + if (resource.type === 'terminal') { + void openTerminal(undefined, desktopScopeId) + .then((state) => { + if (!state.activeTerminalId) return + selectResource(terminalResourceId(state.activeTerminalId)) + requestTerminalFocus(state.activeTerminalId) + }) + .catch(() => toast.error('Could not open a new terminal. Please try again.')) + return + } // Opening a resource before the first message is sent is allowed: there // is simply no chat to attach it to yet. `onAddResource` queues it and // persists once the chat exists, so only the server call is conditional. @@ -304,15 +350,8 @@ export function ResourceTabs({ [chatId, desktopScopeId, onAddResource, selectResource] ) - const handleOpenExisting = useCallback( - (resource: MothershipResource) => { - openExistingResourceTab(resource, desktopScopeId, selectResource) - }, - [desktopScopeId, selectResource] - ) - const handleSelect = useCallback( - (id: string, _source?: TabStripSelectionSource, e?: ReactMouseEvent) => { + (id: string, source?: TabStripSelectionSource, e?: ReactMouseEvent) => { const idx = resources.findIndex((r) => r.id === id) const resource = resources[idx] if (!resource) return @@ -358,6 +397,11 @@ export function ResourceTabs({ anchorIdRef.current = resource.id setSelectedIds(new Set([resource.id])) selectResource(resource.id) + // A pointer pick of a shell also hands it the keyboard; arrow-key + // navigation along the strip keeps its own focus. + if (resource.type === 'terminal' && source !== 'keyboard') { + requestTerminalFocus(terminalIdFromResourceId(resource.id)) + } }, [resources, selectResource, selectedIds, activeId] ) @@ -368,12 +412,18 @@ export function ResourceTabs({ if (!resource) return const isMulti = selectedIds.has(resource.id) && selectedIds.size > 1 const targets = isMulti ? resources.filter((r) => selectedIds.has(r.id)) : [resource] + if (!confirmClosingRunningTerminals(targets, terminalTabs)) return // Update parent state immediately for all targets. A browser tab's page - // is closed natively too; the tab list then confirms the removal. + // or a terminal's shell is closed natively too; the tab list then + // confirms the removal. for (const r of targets) { onRemoveResource(r.type, r.id) if (r.type === 'browser') { sendBrowserPanelAction('close-tab', { tabId: r.id }, desktopScopeId) + } else if (r.type === 'terminal') { + void closeTerminal(terminalIdFromResourceId(r.id), desktopScopeId).catch(() => + toast.error('Could not close that terminal. Please try again.') + ) } } // Clear stale selection and anchor for all removed targets @@ -397,10 +447,13 @@ export function ResourceTabs({ } }, // eslint-disable-next-line react-hooks/exhaustive-deps - [chatId, desktopScopeId, onRemoveResource, resources, selectedIds] + [chatId, desktopScopeId, onRemoveResource, resources, selectedIds, terminalTabs] ) - /** The strip's own title for a resource — a browser tab's is its live page title. */ + /** + * The strip's own title for a resource: a browser tab's live page title, a + * terminal's settled program or directory. + */ const withStripTitle = useCallback( (resource: MothershipResource): MothershipResource => { const title = tabs.find((tab) => tab.id === resource.id)?.title @@ -454,11 +507,18 @@ export function ResourceTabs({ const [moved] = reordered.splice(fromIndex, 1) reordered.splice(targetIndex, 0, moved) onReorderResources(reordered) - // Browser tabs are not stored with the chat; their order lives in the - // desktop's native list, which restore and the agent read back. + // Browser tabs and terminals are not stored with the chat; their order + // lives in the desktop's native lists, which restore and the agent read + // back. + const nativeIndex = () => reordered.filter((r) => r.type === moved.type).indexOf(moved) if (moved.type === 'browser') { - const browserIndex = reordered.filter((r) => r.type === 'browser').indexOf(moved) - reorderBrowserTab(moved.id, browserIndex, desktopScopeId) + reorderBrowserTab(moved.id, nativeIndex(), desktopScopeId) + } else if (moved.type === 'terminal') { + void reorderTerminal( + terminalIdFromResourceId(moved.id), + nativeIndex(), + desktopScopeId + ).catch(() => toast.error('Could not reorder that terminal. Please try again.')) } if (chatId) { const persistable = reordered.filter((r) => !isEphemeralResource(r)) @@ -506,9 +566,7 @@ export function ResourceTabs({
() for (const resource of resources) { - if (!isPersistentPanel(resource)) continue - if (resource.type === 'browser') { - if (browserSeen) continue - browserSeen = true - } + if (!isPersistentPanel(resource) || seen.has(resource.type)) continue + seen.add(resource.type) panels.push(resource) } return panels @@ -241,13 +239,10 @@ export const MothershipView = memo( itself, and the terminals stop being measured. */} {persistentResources.map((resource) => { - const panelVisible = - resource.type === 'browser' - ? active?.type === 'browser' - : resource.id === active?.id + const panelVisible = active?.type === resource.type return (
{/* diff --git a/apps/sim/app/workspace/[workspaceId]/home/components/user-input/components/constants.ts b/apps/sim/app/workspace/[workspaceId]/home/components/user-input/components/constants.ts index 4f73444d513..a36370dd9b9 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/components/user-input/components/constants.ts +++ b/apps/sim/app/workspace/[workspaceId]/home/components/user-input/components/constants.ts @@ -1,4 +1,5 @@ import { cn } from '@sim/emcn' +import { terminalIdFromResourceId } from '@/lib/terminal/resource-id' import type { MothershipResource, MothershipResourceType, @@ -109,15 +110,18 @@ export const SPEECH_RECOGNITION_LANG = 'en-US' * so adding a new resource type fails compilation here until a conversion is * supplied — preventing silent drift between the two taxonomies. */ -// A browser resource is one live page, so its id is a precise tab pointer. A -// terminal resource names either the singleton panel, which asks the agent to -// inspect the whole resource, or one live shell. +// A browser resource is one live page and a terminal resource one live shell, +// so each id is a precise pointer the agent can act on directly. const RESOURCE_TO_CONTEXT: Record< MothershipResourceType, (resource: MothershipResource) => ChatContext > = { browser: (r) => ({ kind: 'browser_tab', tabId: r.id, label: r.title }), - terminal: (r) => ({ kind: 'terminal_tab', terminalId: r.id, label: r.title }), + terminal: (r) => ({ + kind: 'terminal_tab', + terminalId: terminalIdFromResourceId(r.id), + label: r.title, + }), workflow: (r) => ({ kind: 'workflow', workflowId: r.id, label: r.title }), knowledgebase: (r) => ({ kind: 'knowledge', knowledgeId: r.id, label: r.title }), table: (r) => ({ kind: 'table', tableId: r.id, label: r.title }), diff --git a/apps/sim/app/workspace/[workspaceId]/home/components/user-input/components/plus-menu-dropdown/plus-menu-dropdown.test.tsx b/apps/sim/app/workspace/[workspaceId]/home/components/user-input/components/plus-menu-dropdown/plus-menu-dropdown.test.tsx index 09d5988fa6d..2c6a3944288 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/components/user-input/components/plus-menu-dropdown/plus-menu-dropdown.test.tsx +++ b/apps/sim/app/workspace/[workspaceId]/home/components/user-input/components/plus-menu-dropdown/plus-menu-dropdown.test.tsx @@ -22,7 +22,14 @@ const fixtures = vi.hoisted(() => ({ data: [] as { id: string; name: string; parentId: string | null }[], isPending: false, }, - tabs: [], + tabs: [] as Array<{ + terminalId: string + title: string + cwd: string | null + running: string | null + interactive: boolean + active: boolean + }>, browserTabs: [] as Array<{ tabId: string title: string @@ -75,7 +82,6 @@ vi.mock('@/stores/browser-session/store', () => ({ })) vi.mock('@/stores/copilot-terminal/store', () => ({ useCopilotTerminalStore: () => fixtures.tabs })) -import { TERMINAL_SESSION_RESOURCE_ID } from '@/lib/copilot/resources/types' import { setDesktopPreferencesSnapshot } from '@/lib/desktop' import { mapResourceToContext, @@ -153,6 +159,7 @@ describe('PlusMenuDropdown desktop resources', () => { fixtures.browserAvailable.mockReturnValue(true) fixtures.terminalAvailable.mockReturnValue(true) fixtures.browserTabs.length = 0 + fixtures.tabs.length = 0 setDesktopPreferencesSnapshot(PREFERENCES) Object.defineProperty(Element.prototype, 'scrollIntoView', { configurable: true, @@ -175,6 +182,14 @@ describe('PlusMenuDropdown desktop resources', () => { }) it('keeps shared categories in the same order in browse and mention modes', () => { + fixtures.tabs.push({ + terminalId: '9', + title: 'sim', + cwd: '/code/sim', + running: null, + interactive: false, + active: true, + }) fixtures.browserTabs.push({ tabId: '7', title: 'Sim Docs', @@ -237,14 +252,28 @@ describe('PlusMenuDropdown desktop resources', () => { }) it.each([false, true])( - 'offers no browser row without a live page in mention=%s mode', + 'offers no desktop rows without a live page or shell in mention=%s mode', (mention) => { openMenu(mention) - expect(menuItems().map((item) => item.textContent)).not.toContain('Browser') - expect(menuItems().map((item) => item.textContent)).toContain('Terminal') + const names = menuItems().map((item) => item.textContent) + expect(names).not.toContain('Browser') + expect(names).not.toContain('Terminal') } ) + it('lists a live shell under the Terminal category in browse mode', () => { + fixtures.tabs.push({ + terminalId: '9', + title: 'sim', + cwd: '/code/sim', + running: null, + interactive: false, + active: true, + }) + openMenu() + expect(menuItems().map((item) => item.textContent)).toContain('Terminal') + }) + it('finds a browser tab through plus-menu search by its family and selects it with Enter', () => { fixtures.browserTabs.push({ tabId: '7', @@ -272,7 +301,7 @@ describe('PlusMenuDropdown desktop resources', () => { }) }) - it.each([false, true])('keeps unavailable Browser hidden in mention=%s mode', (mention) => { + it('keeps unavailable Browser hidden while offering live shells in mention mode', () => { fixtures.browserAvailable.mockReturnValue(false) fixtures.browserTabs.push({ tabId: '7', @@ -281,13 +310,26 @@ describe('PlusMenuDropdown desktop resources', () => { loading: false, active: true, }) - const { onResourceSelect } = openMenu(mention) + fixtures.tabs.push({ + terminalId: '9', + title: 'sim', + cwd: '/code/sim', + running: null, + interactive: false, + active: true, + }) + const { onResourceSelect } = openMenu(true) expect(menuItems().some((item) => item.textContent === 'Sim Docs')).toBe(false) - selectItem('Terminal') + selectItem('sim') expect(onResourceSelect).toHaveBeenCalledExactlyOnceWith({ type: 'terminal', - id: TERMINAL_SESSION_RESOURCE_ID, - title: 'Terminal', + id: 'terminal:9', + title: 'sim', + }) + expect(mapResourceToContext(onResourceSelect.mock.calls[0][0])).toEqual({ + kind: 'terminal_tab', + terminalId: '9', + label: 'sim', }) }) diff --git a/apps/sim/app/workspace/[workspaceId]/home/components/user-input/components/plus-menu-dropdown/plus-menu-dropdown.tsx b/apps/sim/app/workspace/[workspaceId]/home/components/user-input/components/plus-menu-dropdown/plus-menu-dropdown.tsx index 446a6d2564b..665af481bdb 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/components/user-input/components/plus-menu-dropdown/plus-menu-dropdown.tsx +++ b/apps/sim/app/workspace/[workspaceId]/home/components/user-input/components/plus-menu-dropdown/plus-menu-dropdown.tsx @@ -32,6 +32,7 @@ import type { MothershipResource, MothershipResourceType, } from '@/app/workspace/[workspaceId]/home/types' +import { useSettledTerminalCommands } from '@/hooks/use-settled-terminal-commands' import { useBrowserSessionStore } from '@/stores/browser-session/store' import { useCopilotTerminalStore } from '@/stores/copilot-terminal/store' @@ -123,16 +124,26 @@ export const PlusMenuDropdown = React.memo( setOpen(false) }, []) + const settledCommands = useSettledTerminalCommands(terminalTabs) const visibleResources = useMemo(() => { - const resources = withBrowserTabMentions( - withFolderMentions(availableResources, structureFolders), - browserTabs + const resources = withTerminalTabMentions( + withBrowserTabMentions( + withFolderMentions(availableResources, structureFolders), + browserTabs + ), + terminalTabs, + settledCommands ) - if (isMention) { - return withTerminalTabMentions(resources, terminalTabs) - } + if (isMention) return resources return resources.filter(({ type }) => !MENTION_ONLY_RESOURCE_TYPES.has(type)) - }, [availableResources, structureFolders, browserTabs, isMention, terminalTabs]) + }, [ + availableResources, + structureFolders, + browserTabs, + isMention, + settledCommands, + terminalTabs, + ]) const treeSections = useResourceTreeSections({ groups: availableResources, diff --git a/apps/sim/app/workspace/[workspaceId]/home/components/user-input/components/plus-menu-dropdown/resource-mention-items.test.ts b/apps/sim/app/workspace/[workspaceId]/home/components/user-input/components/plus-menu-dropdown/resource-mention-items.test.ts index 3ba69a834dd..f83880f0df7 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/components/user-input/components/plus-menu-dropdown/resource-mention-items.test.ts +++ b/apps/sim/app/workspace/[workspaceId]/home/components/user-input/components/plus-menu-dropdown/resource-mention-items.test.ts @@ -1,5 +1,4 @@ import { describe, expect, it } from 'vitest' -import { TERMINAL_SESSION_RESOURCE_ID } from '@/lib/copilot/resources/types' import type { AvailableItem } from '@/app/workspace/[workspaceId]/home/components/mothership-view/components/add-resource-dropdown/resource-folder-tree' import { byResourceMenuOrder } from '@/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-registry' import { @@ -18,7 +17,7 @@ const groups = [ }, { type: 'terminal' as const, - items: [{ id: TERMINAL_SESSION_RESOURCE_ID, name: 'Terminal' }], + items: [{ id: 'terminal', name: 'Terminal' }], }, ] @@ -64,38 +63,40 @@ describe('withBrowserTabMentions', () => { }) describe('withTerminalTabMentions', () => { - it('keeps Terminal as a flat resource mention with no live shells', () => { - const result = withTerminalTabMentions(groups, []) + it('drops the Terminal launcher when no shell is open', () => { + const result = withTerminalTabMentions(groups, [], new Set()) - expect(result.find((group) => group.type === 'terminal')?.items).toEqual([ - expect.objectContaining({ id: TERMINAL_SESSION_RESOURCE_ID, name: 'Terminal' }), - ]) + expect(result.find((group) => group.type === 'terminal')?.items).toEqual([]) + expect(result.find((group) => group.type === 'workflow')).toBe(groups[0]) }) - it('offers the whole Terminal first and every live shell after it', () => { - const result = withTerminalTabMentions(groups, [ - { - terminalId: 'terminal-1', - title: 'sim', - cwd: '/code/sim', - running: null, - interactive: false, - active: true, - }, - { - terminalId: 'terminal-2', - title: 'sim', - cwd: '/tmp/sim', - running: null, - interactive: false, - active: false, - }, - ]) + it('offers every live shell as its own Terminal mention, named like the strip', () => { + const result = withTerminalTabMentions( + groups, + [ + { + terminalId: 'terminal-1', + title: 'sim', + cwd: '/code/sim', + running: 'bun run build', + interactive: false, + active: true, + }, + { + terminalId: 'terminal-2', + title: 'sim', + cwd: '/tmp/sim', + running: null, + interactive: false, + active: false, + }, + ], + new Set(['terminal-1']) + ) - expect(result.find((group) => group.type === 'terminal')?.items).toMatchObject([ - { id: TERMINAL_SESSION_RESOURCE_ID, name: 'Terminal' }, - { id: 'terminal-1', name: 'sim 1' }, - { id: 'terminal-2', name: 'sim 2' }, + expect(result.find((group) => group.type === 'terminal')?.items).toEqual([ + { id: 'terminal:terminal-1', name: 'bun run build', mentionFamily: 'Terminal' }, + { id: 'terminal:terminal-2', name: 'sim', mentionFamily: 'Terminal' }, ]) }) }) diff --git a/apps/sim/app/workspace/[workspaceId]/home/components/user-input/components/plus-menu-dropdown/resource-mention-items.ts b/apps/sim/app/workspace/[workspaceId]/home/components/user-input/components/plus-menu-dropdown/resource-mention-items.ts index 1ba78aed760..585f805f719 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/components/user-input/components/plus-menu-dropdown/resource-mention-items.ts +++ b/apps/sim/app/workspace/[workspaceId]/home/components/user-input/components/plus-menu-dropdown/resource-mention-items.ts @@ -1,7 +1,8 @@ import type { BrowserTabState } from '@sim/browser-protocol' import type { TerminalTabState } from '@sim/terminal-protocol' import { browserTabTitle } from '@/lib/browser-agent/tab-label' -import { TERMINAL_SESSION_RESOURCE_ID } from '@/lib/copilot/resources/types' +import { terminalResourceId } from '@/lib/terminal/resource-id' +import { terminalTabTitle } from '@/lib/terminal/tab-label' import type { AvailableItem } from '@/app/workspace/[workspaceId]/home/components/mothership-view/components/add-resource-dropdown/resource-folder-tree' import type { MothershipResourceType } from '@/app/workspace/[workspaceId]/home/types' @@ -80,35 +81,32 @@ export function withBrowserTabMentions( ) } -/** Adds live shells after the always-present Terminal mention. */ +/** + * Replaces the Terminal launcher row with the live shells, which are the only + * terminal things that can be attached or mentioned. With no shell open the + * family disappears from the menu. A shell is named after its settled + * foreground program, else its directory; the strip settles the same way. + */ export function withTerminalTabMentions( groups: readonly ResourceMentionGroup[], - terminalTabs: readonly TerminalTabState[] + terminalTabs: readonly TerminalTabState[], + settledCommands: ReadonlySet ): ResourceMentionGroup[] { - const terminalNames = uniqueTabNames(terminalTabs, (tab) => tab.title.trim() || 'Terminal') - - return groups.map((group) => { - if (group.type === 'terminal') { - const existing = group.items.find((item) => item.id === TERMINAL_SESSION_RESOURCE_ID) - return { - ...group, - items: [ - { - ...existing, - id: TERMINAL_SESSION_RESOURCE_ID, - name: 'Terminal', - mentionFamily: 'Terminal', - }, - ...terminalTabs.map((tab, index) => ({ - id: tab.terminalId, + const terminalNames = uniqueTabNames(terminalTabs, (tab) => + terminalTabTitle(tab, settledCommands) + ) + return groups.map((group) => + group.type === 'terminal' + ? { + ...group, + items: terminalTabs.map((tab, index) => ({ + id: terminalResourceId(tab.terminalId), name: terminalNames[index], mentionFamily: 'Terminal', })), - ], - } - } - return group - }) + } + : group + ) } /** One row of the `@` list: an item plus the family it came from. */ diff --git a/apps/sim/app/workspace/[workspaceId]/home/components/user-input/components/resource-context.test.ts b/apps/sim/app/workspace/[workspaceId]/home/components/user-input/components/resource-context.test.ts index 47e0216319f..8eaab2d26f5 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/components/user-input/components/resource-context.test.ts +++ b/apps/sim/app/workspace/[workspaceId]/home/components/user-input/components/resource-context.test.ts @@ -10,15 +10,12 @@ function resource(partial: Partial & Pick { - it('turns the singleton panels into whole-resource pointers', () => { - expect( - mapResourceToContext(resource({ type: 'browser', id: 'browser-session', title: 'Browser' })) - ).toEqual({ kind: 'browser_tab', tabId: 'browser-session', label: 'Browser' }) - expect( - mapResourceToContext( - resource({ type: 'terminal', id: 'terminal-session', title: 'Terminal' }) - ) - ).toEqual({ kind: 'terminal_tab', terminalId: 'terminal-session', label: 'Terminal' }) + it('turns a terminal tab into a pointer at that shell', () => { + expect(mapResourceToContext(resource({ type: 'terminal', id: '3', title: 'sim' }))).toEqual({ + kind: 'terminal_tab', + terminalId: '3', + label: 'sim', + }) }) it('turns a dragged browser tab into a pointer at that tab', () => { diff --git a/apps/sim/app/workspace/[workspaceId]/home/home.tsx b/apps/sim/app/workspace/[workspaceId]/home/home.tsx index 62081483940..c169af084e8 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/home.tsx +++ b/apps/sim/app/workspace/[workspaceId]/home/home.tsx @@ -37,6 +37,7 @@ import { persistImportedWorkflow } from '@/lib/workflows/operations/import-expor import { RESOURCE_HEADER_CLASSES } from '@/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-tabs/resource-tab-controls' import { SuggestedActions } from '@/app/workspace/[workspaceId]/home/components/suggested-actions' import { useBrowserTabResources } from '@/app/workspace/[workspaceId]/home/hooks/use-browser-tab-resources' +import { useTerminalTabResources } from '@/app/workspace/[workspaceId]/home/hooks/use-terminal-tab-resources' import { resolveWorkspaceResourceRef } from '@/app/workspace/[workspaceId]/home/resolve-resource-ref' import { resolveResourceEventPresentation, @@ -333,14 +334,23 @@ export function Home({ chatId, userName, userId }: HomeProps) { [setActiveResourceId, clearResourceActivity] ) - useBrowserTabResources({ - scopeId: desktopScopeId, - resources, - activeResourceId, + const desktopTabResourceCallbacks = { addResource, removeResource, selectResource: selectResourceFromUser, onResourceEvent: handleResourceEvent, + } + useBrowserTabResources({ + scopeId: desktopScopeId, + resources, + activeResourceId, + ...desktopTabResourceCallbacks, + }) + useTerminalTabResources({ + scopeId: desktopScopeId, + resources, + activeResourceId, + ...desktopTabResourceCallbacks, }) const addResourceFromUser = useCallback( diff --git a/apps/sim/app/workspace/[workspaceId]/home/hooks/stream/handle-resource-event.test.ts b/apps/sim/app/workspace/[workspaceId]/home/hooks/stream/handle-resource-event.test.ts index fac03a8f98c..abb0c6dd488 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/hooks/stream/handle-resource-event.test.ts +++ b/apps/sim/app/workspace/[workspaceId]/home/hooks/stream/handle-resource-event.test.ts @@ -45,6 +45,17 @@ function browserUpsertEvent(id: string, title: string): PersistedStreamEventEnve } as PersistedStreamEventEnvelope } +function terminalUpsertEvent(id: string, title: string): PersistedStreamEventEnvelope { + return { + type: 'resource', + v: 1, + seq: 1, + ts: '', + stream: { streamId: 's', cursor: '1' }, + payload: { op: 'upsert', resource: { type: 'terminal', id, title } }, + } as PersistedStreamEventEnvelope +} + describe('handleResourceEvent removal', () => { beforeEach(() => { vi.clearAllMocks() @@ -98,6 +109,19 @@ describe('handleResourceEvent removal', () => { browserUpsertEvent('browser-session:slack-tab', 'mship-todo (Channel) - sim - Slack') ) + expect(deps.addResource).not.toHaveBeenCalled() + expect(deps.setActiveResourceId).not.toHaveBeenCalled() + expect(onResourceEvent).not.toHaveBeenCalled() + }) + it('ignores terminal events because terminal tabs come from the desktop tab list', () => { + const onResourceEvent = vi.fn() + const deps = makeStreamLoopDeps({ + onResourceEventRef: { current: onResourceEvent }, + }) + const ctx = { deps } as StreamLoopContext + + handleResourceEvent(ctx, terminalUpsertEvent('terminal-session', 'Terminal')) + expect(deps.addResource).not.toHaveBeenCalled() expect(deps.setActiveResourceId).not.toHaveBeenCalled() expect(onResourceEvent).not.toHaveBeenCalled() diff --git a/apps/sim/app/workspace/[workspaceId]/home/hooks/stream/handle-resource-event.ts b/apps/sim/app/workspace/[workspaceId]/home/hooks/stream/handle-resource-event.ts index 0e507ee3513..67ce8a15c72 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/hooks/stream/handle-resource-event.ts +++ b/apps/sim/app/workspace/[workspaceId]/home/hooks/stream/handle-resource-event.ts @@ -4,14 +4,16 @@ import { } from '@/lib/copilot/generated/mothership-stream-v1' import type { FilePreviewSession } from '@/lib/copilot/request/session' import type { PersistedStreamEventEnvelope } from '@/lib/copilot/request/session/contract' -import { canonicalizeDesktopSessionResource } from '@/lib/copilot/resources/types' import { invalidateResourceQueries } from '@/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-registry' import { hasRenderableFilePreviewContent, shouldReplaceSession, } from '@/app/workspace/[workspaceId]/home/hooks/preview' import type { StreamLoopContext } from '@/app/workspace/[workspaceId]/home/hooks/stream/stream-context' -import type { MothershipResourceType } from '@/app/workspace/[workspaceId]/home/types' +import type { + MothershipResource, + MothershipResourceType, +} from '@/app/workspace/[workspaceId]/home/types' import { removeWorkflowFromActiveCache } from '@/hooks/queries/utils/workflow-cache' import { useTableViewPinStore } from '@/stores/table/view-pin/store' import { useWorkflowRegistry } from '@/stores/workflows/registry/store' @@ -46,9 +48,9 @@ export function handleResourceEvent(ctx: StreamLoopContext, parsed: ResourceEven if (!workspaceId) return const onResourceEvent = onResourceEventRef.current const payload = parsed.payload - // Browser tabs are projected from the desktop app's live page list, never - // from the stream; older servers announced them as resources. - if (payload.resource.type === 'browser') return + // Browser and terminal tabs are projected from the desktop app's live + // lists, never from the stream; older servers announced them as resources. + if (payload.resource.type === 'browser' || payload.resource.type === 'terminal') return const shouldClearViewId = payload.resource.type === 'table' && payload.resource.clearViewId === true // A saved view the agent just created or edited: the table opens on it, and @@ -60,13 +62,13 @@ export function handleResourceEvent(ctx: StreamLoopContext, parsed: ResourceEven payload.resource.viewId.trim() ? payload.resource.viewId : undefined - const resource = canonicalizeDesktopSessionResource({ + const resource: MothershipResource = { type: payload.resource.type as MothershipResourceType, id: payload.resource.id, title: typeof payload.resource.title === 'string' ? payload.resource.title : payload.resource.id, ...(pinnedViewId ? { viewId: pinnedViewId } : {}), - }) + } const resourceUpdate = shouldClearViewId ? { ...resource, clearViewId: true as const } : resource if (payload.op === MothershipStreamV1ResourceOp.remove) { diff --git a/apps/sim/app/workspace/[workspaceId]/home/hooks/use-browser-tab-resources.ts b/apps/sim/app/workspace/[workspaceId]/home/hooks/use-browser-tab-resources.ts index 3bac91ec5cc..51885bd161f 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/hooks/use-browser-tab-resources.ts +++ b/apps/sim/app/workspace/[workspaceId]/home/hooks/use-browser-tab-resources.ts @@ -1,47 +1,35 @@ -import { useEffect, useRef } from 'react' +import { useEffect, useMemo, useRef } from 'react' import type { BrowserTabState } from '@sim/browser-protocol' import { createLogger } from '@sim/logger' import { getErrorMessage } from '@sim/utils/errors' import { onOpenInBrowserPanel } from '@/lib/browser-agent/open-in-panel' import { browserTabTitle } from '@/lib/browser-agent/tab-label' import { openUrlInNewBrowserTab, sendBrowserPanelAction } from '@/lib/browser-agent/transport' -import type { MothershipResource, MothershipResourceType } from '@/lib/copilot/resources/types' -import type { ResourceEventHandler } from '@/app/workspace/[workspaceId]/home/hooks/use-chat' +import type { MothershipResource } from '@/lib/copilot/resources/types' +import { + type DesktopTabResourceCallbacks, + useDesktopTabResources, +} from '@/app/workspace/[workspaceId]/home/hooks/use-desktop-tab-resources' import { useBrowserSessionStore } from '@/stores/browser-session/store' const logger = createLogger('BrowserTabResources') const EMPTY_BROWSER_TABS: BrowserTabState[] = [] -interface UseBrowserTabResourcesOptions { +interface UseBrowserTabResourcesOptions extends DesktopTabResourceCallbacks { /** Desktop browser scope whose pages back this chat's browser tabs. */ scopeId: string resources: readonly MothershipResource[] activeResourceId: string | null - /** Adds a tab without activating it; activation goes through {@link onResourceEvent}. */ - addResource: (resource: MothershipResource) => void - removeResource: (resourceType: MothershipResourceType, resourceId: string) => void - /** Explicit user selection, which claims the strip's selection for the user. */ - selectResource: (resourceId: string) => void - /** Agent activity on a tab, subject to the panel's user-ownership policy. */ - onResourceEvent: ResourceEventHandler +} + +function switchBrowserTab(tabId: string, scopeId: string): void { + sendBrowserPanelAction('switch-tab', { tabId, claim: false }, scopeId) } /** - * Keeps the chat's `browser` resource tabs equal to the desktop app's live - * page list, one resource per native tab. - * - * The desktop app owns the pages, so its tab list is the source of truth: a - * page appearing there (agent, `+ Browser`, popup, restore) gains a resource - * tab and a page leaving it loses one. Closing a browser resource tab closes - * its page at the strip, which then comes back through the same list. Visible - * selection is routed the same way — choosing a browser resource tab switches - * the native page, and a native switch (Ctrl+Tab in the page, a popup the user - * opened) follows into the strip while the user is on the browser. - * - * The agent never moves the visible page itself. Its tab is announced as - * resource activity, so the existing view policy decides whether to show it or - * only badge it while the user is reading something else. + * Projects the desktop app's live browser pages into `browser` resource tabs, + * one per page. See {@link useDesktopTabResources} for the shared model. */ export function useBrowserTabResources({ scopeId, @@ -52,113 +40,44 @@ export function useBrowserTabResources({ selectResource, onResourceEvent, }: UseBrowserTabResourcesOptions): void { - // A missing bucket means the scope has not been activated yet or was just - // migrated to its durable id; it says nothing about the pages themselves. const hasSession = useBrowserSessionStore((state) => state.sessions[scopeId] !== undefined) - const tabs = useBrowserSessionStore( + const browserTabs = useBrowserSessionStore( (state) => state.sessions[scopeId]?.tabs ?? EMPTY_BROWSER_TABS ) const activeTabId = useBrowserSessionStore( (state) => state.sessions[scopeId]?.activeTabId ?? null ) - const automationTabId = useBrowserSessionStore((state) => { + const agentTabId = useBrowserSessionStore((state) => { const session = state.sessions[scopeId] if (!session) return null return session.automationActive || session.agentRunIds.length > 0 ? session.automationTabId : null }) - /** - * Tab ids whose resource has been seen in the strip for the current scope. - * A tab is projected until its resource shows up — chat hydration can - * replace the list underneath a fresh add — and once it has been seen, its - * absence means the user closed it and the native close is in flight. - */ - const knownTabIdsRef = useRef | null>(null) - knownTabIdsRef.current ??= new Set() - const knownScopeRef = useRef(scopeId) - /** The native switch this hook asked for and has not seen land yet. */ - const requestedTabIdRef = useRef(null) + const tabs = useMemo( + () => browserTabs.map((tab) => ({ id: tab.tabId, title: browserTabTitle(tab) })), + [browserTabs] + ) const scopeIdRef = useRef(scopeId) scopeIdRef.current = scopeId - const tabsRef = useRef(tabs) - tabsRef.current = tabs - const activeTabIdRef = useRef(activeTabId) - activeTabIdRef.current = activeTabId - const resourcesRef = useRef(resources) - resourcesRef.current = resources - const activeResourceIdRef = useRef(activeResourceId) - activeResourceIdRef.current = activeResourceId const selectResourceRef = useRef(selectResource) selectResourceRef.current = selectResource - const onResourceEventRef = useRef(onResourceEvent) - onResourceEventRef.current = onResourceEvent - - useEffect(() => { - const known = knownTabIdsRef.current - if (!known) return - if (knownScopeRef.current !== scopeId) { - knownScopeRef.current = scopeId - known.clear() - requestedTabIdRef.current = null - } - const resourceTabIds = new Set( - resources.filter((resource) => resource.type === 'browser').map((resource) => resource.id) - ) - - for (const tab of tabs) { - if (resourceTabIds.has(tab.tabId)) { - known.add(tab.tabId) - continue - } - if (!known.has(tab.tabId)) { - addResource({ type: 'browser', id: tab.tabId, title: browserTabTitle(tab) }) - } - } - - if (!hasSession) return - const liveTabIds = new Set(tabs.map((tab) => tab.tabId)) - for (const tabId of known) { - if (liveTabIds.has(tabId)) continue - known.delete(tabId) - if (resourceTabIds.has(tabId)) removeResource('browser', tabId) - } - }, [addResource, hasSession, removeResource, resources, scopeId, tabs]) - // Selecting a browser resource tab shows its native page. Keyed on the - // selection alone: a native push must not re-assert a selection it just - // moved away from, or the two sides would trade switches forever. - useEffect(() => { - if (!activeResourceId || activeResourceId === activeTabIdRef.current) return - if (!tabsRef.current.some((tab) => tab.tabId === activeResourceId)) return - requestedTabIdRef.current = activeResourceId - sendBrowserPanelAction( - 'switch-tab', - { tabId: activeResourceId, claim: false }, - scopeIdRef.current - ) - }, [activeResourceId]) - - // A native switch while the user is on the browser follows into the strip. - // The switch this hook requested itself is not a native change of mind. - useEffect(() => { - if (requestedTabIdRef.current === activeTabId) { - requestedTabIdRef.current = null - return - } - const activeResource = resourcesRef.current.find( - (resource) => resource.id === activeResourceIdRef.current - ) - if (!activeTabId || activeResource?.type !== 'browser' || activeResource.id === activeTabId) { - return - } - selectResourceRef.current(activeTabId) - }, [activeTabId]) - - // The agent's tab surfaces like any other agent activity. - useEffect(() => { - if (automationTabId) onResourceEventRef.current(automationTabId, { activate: true }) - }, [automationTabId]) + useDesktopTabResources({ + type: 'browser', + scopeId, + tabs, + hasSession, + activeTabId, + agentTabId, + switchTab: switchBrowserTab, + resources, + activeResourceId, + addResource, + removeResource, + selectResource, + onResourceEvent, + }) // Chat links clicked in the desktop app open in a new browser tab. The user // asked to see it, so it is selected as their own choice rather than offered diff --git a/apps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.test.ts b/apps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.test.ts index 00e59d00ce8..b833f38856a 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.test.ts +++ b/apps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.test.ts @@ -10,7 +10,6 @@ import { import type { StreamBatchEvent } from '@/lib/copilot/request/session/types' import { getReplayCompletedWorkflowToolCallIds, - panelForExecutingClientTool, reconcileLiveAssistantTurn, selectDeletedWorkflowResources, selectReconnectReplayState, @@ -18,11 +17,7 @@ import { shouldQueueOutgoingMessage, waitForDetachedChatResolution, } from '@/app/workspace/[workspaceId]/home/hooks/use-chat' -import type { - ChatMessage, - ContentBlock, - ToolCallStatus, -} from '@/app/workspace/[workspaceId]/home/types' +import type { ContentBlock } from '@/app/workspace/[workspaceId]/home/types' vi.mock('next/navigation', () => ({ usePathname: () => '/workspace/workspace-1/home', @@ -326,59 +321,3 @@ describe('getReplayCompletedWorkflowToolCallIds', () => { expect(result).toEqual(new Set(['workflow-complete'])) }) }) - -describe('panelForExecutingClientTool', () => { - function toolCallMessage(id: string, name: string, status: ToolCallStatus): ChatMessage { - return { - id, - role: 'assistant', - content: '', - contentBlocks: [{ type: 'tool_call', toolCall: { id: `${id}-tool`, name, status } }], - } - } - - it('detects a browser tool call that is still executing', () => { - const messages = [ - toolCallMessage('m1', 'browser_click', 'success'), - toolCallMessage('m2', 'browser_navigate', 'executing'), - ] - - expect(panelForExecutingClientTool(messages)).toBe('browser') - }) - - it('detects a terminal tool call that is still executing', () => { - const messages = [ - toolCallMessage('m1', 'terminal', 'success'), - toolCallMessage('m2', 'terminal', 'executing'), - ] - - expect(panelForExecutingClientTool(messages)).toBe('terminal') - }) - - it('ignores completed calls and executing tools that own no panel', () => { - const messages = [ - toolCallMessage('m1', 'browser_click', 'success'), - toolCallMessage('m2', 'terminal', 'success'), - toolCallMessage('m3', 'run_workflow', 'executing'), - { id: 'm4', role: 'assistant' as const, content: 'no blocks' }, - ] - - expect(panelForExecutingClientTool(messages)).toBe(null) - }) - - // Both panels can be in flight at once; the later call is the one the user - // was watching when they navigated away. - it('picks the later panel when both are mid-action', () => { - const browserFirst = [ - toolCallMessage('m1', 'browser_navigate', 'executing'), - toolCallMessage('m2', 'terminal', 'executing'), - ] - const terminalFirst = [ - toolCallMessage('m1', 'terminal', 'executing'), - toolCallMessage('m2', 'browser_navigate', 'executing'), - ] - - expect(panelForExecutingClientTool(browserFirst)).toBe('terminal') - expect(panelForExecutingClientTool(terminalFirst)).toBe('browser') - }) -}) diff --git a/apps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.ts b/apps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.ts index 7c53e9e7892..6752c4cd6e3 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.ts +++ b/apps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.ts @@ -73,7 +73,6 @@ import { type MothershipResourceUpdate, mergeChatResource, sanitizeChatResources, - TERMINAL_SESSION_RESOURCE_ID, } from '@/lib/copilot/resources/types' import { executeBrowserToolOnClient } from '@/lib/copilot/tools/client/browser-tool-execution' import { @@ -1146,31 +1145,6 @@ export function getReplayCompletedWorkflowToolCallIds(events: StreamBatchEvent[] return completedToolCallIds } -/** - * Which live panel the transcript is mid-action on, or null for neither. - * - * Used on reconnect the way workflow-run recovery restores workflows: a - * mid-command terminal is re-focused, while a mid-action browser tab announces - * itself through the desktop's automation state and only needs to keep the - * terminal from taking over. A completed browser or terminal call is - * suppressed on replay, so it never re-opens its own panel. When calls against - * both are in flight the later one wins, being the one the user was watching. - */ -export function panelForExecutingClientTool( - messages: ChatMessage[] -): 'browser' | 'terminal' | null { - let panel: 'browser' | 'terminal' | null = null - for (const message of messages) { - for (const block of message.contentBlocks ?? []) { - const call = block.toolCall - if (call === undefined || call.status !== 'executing') continue - if (isBrowserToolName(call.name)) panel = 'browser' - else if (isTerminalToolName(call.name)) panel = 'terminal' - } - } - return panel -} - /** * Runs a browser tool on the desktop client. The agent's tab reaches the * resource strip through the desktop tab list, so nothing is opened here. @@ -1189,6 +1163,23 @@ function startClientBrowserTool( executeBrowserToolOnClient(toolCallId, toolName, toolArgs, scopeId, eventTs, signal) } +/** + * Runs a terminal tool on the desktop client. The agent's shell reaches the + * resource strip through the desktop tab list, so nothing is opened here. + * Replay/exactly-once guarding lives in executeTerminalToolOnClient + * (sessionStorage-backed, so reloads cannot re-run a command). + */ +function startClientTerminalTool( + toolCallId: string, + toolName: string, + toolArgs: Record, + scopeId: string, + eventTs?: string +): void { + if (!isTerminalToolName(toolName)) return + executeTerminalToolOnClient(toolCallId, toolArgs, scopeId, eventTs) +} + function buildRecoverySubjectKey( chatId: string | undefined, selectedChatId: string | undefined @@ -2200,34 +2191,6 @@ export function useChat( [workspaceId, organizationId, scopeKey] ) - const openTerminalResource = useCallback(() => { - addResource({ - type: 'terminal', - id: TERMINAL_SESSION_RESOURCE_ID, - title: 'Terminal', - }) - onResourceEventRef.current?.(TERMINAL_SESSION_RESOURCE_ID) - }, [addResource]) - - const startClientTerminalTool = useCallback( - ( - toolCallId: string, - toolName: string, - toolArgs: Record, - scopeId: string, - eventTs?: string - ) => { - if (!isTerminalToolName(toolName)) { - return - } - openTerminalResource() - // Replay/exactly-once guarding lives in executeTerminalToolOnClient - // (sessionStorage-backed, so reloads cannot re-run a command). - executeTerminalToolOnClient(toolCallId, toolArgs, scopeId, eventTs) - }, - [openTerminalResource] - ) - const recoverPendingClientWorkflowTools = useCallback( async (nextMessages: ChatMessage[]) => { const pending: ToolCallInfo[] = [] @@ -2474,8 +2437,8 @@ export function useChat( flushPendingResources(chatHistory.id) - // Browser rows stored by older clients are dropped: the live tab list is - // what puts browser tabs in the strip now. + // Browser and terminal rows stored by older clients are dropped: the + // desktop app's live tab lists are what put those tabs in the strip now. const persistedResources = sanitizeChatResources( chatHistory.resources.filter((r) => r.id !== 'streaming-file') ) @@ -2546,16 +2509,6 @@ export function useChat( setActiveResourceId(null) } - // Live-panel counterpart of the workflow-run recovery above: returning to - // a chat whose turn is mid-command re-focuses the terminal and re-expands - // a collapsed panel. Runs after the resource hydration so it wins over the - // "last resource" active fallback. A mid-action browser tab announces - // itself through the desktop's automation state instead, and a browser - // action in flight after the command keeps the terminal from taking over. - if (shouldReconnectActiveStream) { - if (panelForExecutingClientTool(mappedMessages) === 'terminal') openTerminalResource() - } - const snapshotPreviewSessions = Array.isArray(chatHistory.streamSnapshot?.previewSessions) ? (chatHistory.streamSnapshot.previewSessions as FilePreviewSession[]) : [] @@ -2621,7 +2574,6 @@ export function useChat( cancelActiveStreamReader, cancelActiveStreamRecovery, flushPendingResources, - openTerminalResource, reconcileHydratedWorkflowResources, recoverPendingClientWorkflowTools, seedPreviewSessions, diff --git a/apps/sim/app/workspace/[workspaceId]/home/hooks/use-desktop-tab-resources.ts b/apps/sim/app/workspace/[workspaceId]/home/hooks/use-desktop-tab-resources.ts new file mode 100644 index 00000000000..865ad133f15 --- /dev/null +++ b/apps/sim/app/workspace/[workspaceId]/home/hooks/use-desktop-tab-resources.ts @@ -0,0 +1,160 @@ +import { useEffect, useRef } from 'react' +import type { MothershipResource, MothershipResourceType } from '@/lib/copilot/resources/types' +import type { ResourceEventHandler } from '@/app/workspace/[workspaceId]/home/hooks/use-chat' + +/** One live desktop tab, as the strip needs to know it. */ +export interface DesktopTab { + id: string + title: string +} + +export interface DesktopTabResourceCallbacks { + /** Adds a tab without activating it; activation goes through {@link onResourceEvent}. */ + addResource: (resource: MothershipResource) => void + removeResource: (resourceType: MothershipResourceType, resourceId: string) => void + /** Explicit user selection, which claims the strip's selection for the user. */ + selectResource: (resourceId: string) => void + /** Agent activity on a tab, subject to the panel's user-ownership policy. */ + onResourceEvent: ResourceEventHandler +} + +interface UseDesktopTabResourcesOptions extends DesktopTabResourceCallbacks { + type: 'browser' | 'terminal' + /** Desktop scope whose live tabs back this chat's resource tabs. */ + scopeId: string + /** The desktop app's live tab list for the scope, in its order. */ + tabs: readonly DesktopTab[] + /** + * Whether the renderer holds a bucket for the scope at all. A missing bucket + * means the scope has not been activated yet or was just migrated to its + * durable id; it says nothing about the tabs themselves. + */ + hasSession: boolean + /** The tab the desktop app currently shows for the scope. */ + activeTabId: string | null + /** The tab the agent is working in, while it is working. */ + agentTabId: string | null + /** Shows a tab natively without claiming it for the user. */ + switchTab: (tabId: string, scopeId: string) => void + resources: readonly MothershipResource[] + activeResourceId: string | null +} + +/** + * Keeps one kind of desktop-backed resource tab equal to the desktop app's + * live tab list, one resource per native tab. + * + * The desktop app owns the tabs, so its list is the source of truth: a tab + * appearing there gains a resource tab and a tab leaving it loses one. Closing + * a resource tab closes its native tab at the strip, which then comes back + * through the same list. Visible selection is routed the same way — choosing + * a resource tab switches the native tab, and a native switch follows into the + * strip while the user is on that kind of tab. + * + * The agent never moves the visible tab itself. Its tab is announced as + * resource activity, so the existing view policy decides whether to show it or + * only badge it while the user is reading something else. + */ +export function useDesktopTabResources({ + type, + scopeId, + tabs, + hasSession, + activeTabId, + agentTabId, + switchTab, + resources, + activeResourceId, + addResource, + removeResource, + selectResource, + onResourceEvent, +}: UseDesktopTabResourcesOptions): void { + /** + * Tab ids whose resource has been seen in the strip for the current scope. + * A tab is projected until its resource shows up — chat hydration can + * replace the list underneath a fresh add — and once it has been seen, its + * absence means the user closed it and the native close is in flight. + */ + const knownTabIdsRef = useRef | null>(null) + knownTabIdsRef.current ??= new Set() + const knownScopeRef = useRef(scopeId) + /** The native switch this hook asked for and has not seen land yet. */ + const requestedTabIdRef = useRef(null) + const scopeIdRef = useRef(scopeId) + scopeIdRef.current = scopeId + const tabsRef = useRef(tabs) + tabsRef.current = tabs + const activeTabIdRef = useRef(activeTabId) + activeTabIdRef.current = activeTabId + const resourcesRef = useRef(resources) + resourcesRef.current = resources + const activeResourceIdRef = useRef(activeResourceId) + activeResourceIdRef.current = activeResourceId + const switchTabRef = useRef(switchTab) + switchTabRef.current = switchTab + const selectResourceRef = useRef(selectResource) + selectResourceRef.current = selectResource + const onResourceEventRef = useRef(onResourceEvent) + onResourceEventRef.current = onResourceEvent + + useEffect(() => { + const known = knownTabIdsRef.current + if (!known) return + if (knownScopeRef.current !== scopeId) { + knownScopeRef.current = scopeId + known.clear() + requestedTabIdRef.current = null + } + const resourceTabIds = new Set( + resources.filter((resource) => resource.type === type).map((resource) => resource.id) + ) + + for (const tab of tabs) { + if (resourceTabIds.has(tab.id)) { + known.add(tab.id) + continue + } + if (!known.has(tab.id)) addResource({ type, id: tab.id, title: tab.title }) + } + + if (!hasSession) return + const liveTabIds = new Set(tabs.map((tab) => tab.id)) + for (const tabId of known) { + if (liveTabIds.has(tabId)) continue + known.delete(tabId) + if (resourceTabIds.has(tabId)) removeResource(type, tabId) + } + }, [addResource, hasSession, removeResource, resources, scopeId, tabs, type]) + + // Selecting a resource tab shows its native tab. Keyed on the selection + // alone: a native push must not re-assert a selection it just moved away + // from, or the two sides would trade switches forever. + useEffect(() => { + if (!activeResourceId || activeResourceId === activeTabIdRef.current) return + if (!tabsRef.current.some((tab) => tab.id === activeResourceId)) return + requestedTabIdRef.current = activeResourceId + switchTabRef.current(activeResourceId, scopeIdRef.current) + }, [activeResourceId]) + + // A native switch while the user is on this kind of tab follows into the + // strip. The switch this hook requested itself is not a native change of mind. + useEffect(() => { + if (requestedTabIdRef.current === activeTabId) { + requestedTabIdRef.current = null + return + } + const activeResource = resourcesRef.current.find( + (resource) => resource.id === activeResourceIdRef.current + ) + if (!activeTabId || activeResource?.type !== type || activeResource.id === activeTabId) { + return + } + selectResourceRef.current(activeTabId) + }, [activeTabId, type]) + + // The agent's tab surfaces like any other agent activity. + useEffect(() => { + if (agentTabId) onResourceEventRef.current(agentTabId, { activate: true }) + }, [agentTabId]) +} diff --git a/apps/sim/app/workspace/[workspaceId]/home/hooks/use-terminal-tab-resources.test.tsx b/apps/sim/app/workspace/[workspaceId]/home/hooks/use-terminal-tab-resources.test.tsx new file mode 100644 index 00000000000..ef79b3ed7c2 --- /dev/null +++ b/apps/sim/app/workspace/[workspaceId]/home/hooks/use-terminal-tab-resources.test.tsx @@ -0,0 +1,174 @@ +/** + * @vitest-environment jsdom + */ +import { act } from 'react' +import type { TerminalTabState } from '@sim/terminal-protocol' +import { createRoot, type Root } from 'react-dom/client' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import type { MothershipResource } from '@/lib/copilot/resources/types' +import { useTerminalTabResources } from '@/app/workspace/[workspaceId]/home/hooks/use-terminal-tab-resources' +import { useCopilotTerminalStore } from '@/stores/copilot-terminal/store' + +const { switchTerminal } = vi.hoisted(() => ({ + switchTerminal: vi.fn(async () => {}), +})) + +vi.mock('@/lib/terminal/transport', () => ({ switchTerminal })) + +const SCOPE = 'chat-1' + +function shell( + terminalId: string, + active = false, + running: string | null = null +): TerminalTabState { + return { + terminalId, + title: `dir-${terminalId}`, + cwd: `/code/${terminalId}`, + running, + interactive: false, + active, + } +} + +function pushTabs(scopeId: string, tabs: TerminalTabState[], activeTerminalId: string | null) { + act(() => { + useCopilotTerminalStore.getState().setTabs({ scopeId, tabs, activeTerminalId }) + }) +} + +interface HostProps { + scopeId: string + resources: MothershipResource[] + activeResourceId: string | null + addResource: (resource: MothershipResource) => void + removeResource: (type: MothershipResource['type'], id: string) => void + selectResource: (id: string) => void + onResourceEvent: (id: string, options?: { activate?: boolean }) => void +} + +function Host(props: HostProps) { + useTerminalTabResources(props) + return null +} + +describe('useTerminalTabResources', () => { + let root: Root + let container: HTMLDivElement + const addResource = vi.fn() + const removeResource = vi.fn() + const selectResource = vi.fn() + const onResourceEvent = vi.fn() + + function render(overrides: Partial = {}) { + const props: HostProps = { + scopeId: SCOPE, + resources: [], + activeResourceId: null, + addResource, + removeResource, + selectResource, + onResourceEvent, + ...overrides, + } + act(() => root.render()) + return (next: Partial) => act(() => root.render()) + } + + beforeEach(() => { + vi.stubGlobal('IS_REACT_ACT_ENVIRONMENT', true) + vi.clearAllMocks() + useCopilotTerminalStore.setState({ + activeScopeId: SCOPE, + sessions: {}, + settledAgentCommandIds: [], + }) + act(() => useCopilotTerminalStore.getState().activateScope(SCOPE)) + container = document.createElement('div') + document.body.appendChild(container) + root = createRoot(container) + }) + + afterEach(() => { + act(() => root.unmount()) + container.remove() + vi.unstubAllGlobals() + }) + + it('projects each live shell into a terminal resource and removes closed shells', () => { + const rerender = render() + pushTabs(SCOPE, [shell('1', true), shell('2')], '1') + + expect(addResource.mock.calls.map(([resource]) => resource)).toEqual([ + { type: 'terminal', id: 'terminal:1', title: 'dir-1' }, + { type: 'terminal', id: 'terminal:2', title: 'dir-2' }, + ]) + + rerender({ + resources: [ + { type: 'terminal', id: 'terminal:1', title: 'dir-1' }, + { type: 'terminal', id: 'terminal:2', title: 'dir-2' }, + ], + }) + pushTabs(SCOPE, [shell('1', true)], '1') + expect(removeResource).toHaveBeenCalledExactlyOnceWith('terminal', 'terminal:2') + }) + + it('shows the selected shell without claiming it, and ignores the switch landing', () => { + const resources: MothershipResource[] = [ + { type: 'terminal', id: 'terminal:1', title: 'dir-1' }, + { type: 'terminal', id: 'terminal:2', title: 'dir-2' }, + ] + const rerender = render({ resources, activeResourceId: 'terminal:1' }) + pushTabs(SCOPE, [shell('1', true), shell('2')], '1') + expect(switchTerminal).not.toHaveBeenCalled() + + rerender({ activeResourceId: 'terminal:2' }) + expect(switchTerminal).toHaveBeenCalledExactlyOnceWith('2', SCOPE, { claim: false }) + + pushTabs(SCOPE, [shell('1'), shell('2', true)], '2') + expect(selectResource).not.toHaveBeenCalled() + }) + + it('follows a native switch into the strip only while the user is on a terminal', () => { + const resources: MothershipResource[] = [ + { type: 'terminal', id: 'terminal:1', title: 'dir-1' }, + { type: 'terminal', id: 'terminal:2', title: 'dir-2' }, + { type: 'file', id: 'f', title: 'notes.md' }, + ] + const rerender = render({ resources, activeResourceId: 'terminal:1' }) + pushTabs(SCOPE, [shell('1', true), shell('2')], '1') + + pushTabs(SCOPE, [shell('1'), shell('2', true)], '2') + expect(selectResource).toHaveBeenCalledExactlyOnceWith('terminal:2') + + selectResource.mockClear() + rerender({ activeResourceId: 'f' }) + pushTabs(SCOPE, [shell('1', true), shell('2')], '1') + expect(selectResource).not.toHaveBeenCalled() + }) + + it('announces the shell running an agent command as activity', () => { + render({ + resources: [ + { type: 'terminal', id: 'terminal:1', title: 'dir-1' }, + { type: 'terminal', id: 'terminal:2', title: 'dir-2' }, + ], + activeResourceId: 'terminal:1', + }) + pushTabs(SCOPE, [shell('1', true), shell('2')], '1') + act(() => { + useCopilotTerminalStore.getState().applyCommandEvent({ + scopeId: SCOPE, + terminalId: '2', + phase: 'start', + command: 'bun test', + toolCallId: 'tool-1', + }) + }) + + expect(onResourceEvent).toHaveBeenCalledExactlyOnceWith('terminal:2', { activate: true }) + expect(switchTerminal).not.toHaveBeenCalled() + }) +}) diff --git a/apps/sim/app/workspace/[workspaceId]/home/hooks/use-terminal-tab-resources.ts b/apps/sim/app/workspace/[workspaceId]/home/hooks/use-terminal-tab-resources.ts new file mode 100644 index 00000000000..e794b0debd7 --- /dev/null +++ b/apps/sim/app/workspace/[workspaceId]/home/hooks/use-terminal-tab-resources.ts @@ -0,0 +1,77 @@ +import { useMemo } from 'react' +import type { TerminalTabState } from '@sim/terminal-protocol' +import type { MothershipResource } from '@/lib/copilot/resources/types' +import { terminalIdFromResourceId, terminalResourceId } from '@/lib/terminal/resource-id' +import { switchTerminal } from '@/lib/terminal/transport' +import { + type DesktopTabResourceCallbacks, + useDesktopTabResources, +} from '@/app/workspace/[workspaceId]/home/hooks/use-desktop-tab-resources' +import { useCopilotTerminalStore } from '@/stores/copilot-terminal/store' + +const EMPTY_TERMINAL_TABS: TerminalTabState[] = [] + +interface UseTerminalTabResourcesOptions extends DesktopTabResourceCallbacks { + /** Desktop terminal scope whose shells back this chat's terminal tabs. */ + scopeId: string + resources: readonly MothershipResource[] + activeResourceId: string | null +} + +function showTerminal(resourceId: string, scopeId: string): void { + void switchTerminal(terminalIdFromResourceId(resourceId), scopeId, { claim: false }).catch( + () => {} + ) +} + +/** + * Projects the desktop app's live shells into `terminal` resource tabs, one + * per shell. See {@link useDesktopTabResources} for the shared model. + */ +export function useTerminalTabResources({ + scopeId, + resources, + activeResourceId, + addResource, + removeResource, + selectResource, + onResourceEvent, +}: UseTerminalTabResourcesOptions): void { + const hasSession = useCopilotTerminalStore((state) => state.sessions[scopeId] !== undefined) + const terminalTabs = useCopilotTerminalStore( + (state) => state.sessions[scopeId]?.tabs.tabs ?? EMPTY_TERMINAL_TABS + ) + const activeTerminalId = useCopilotTerminalStore( + (state) => state.sessions[scopeId]?.tabs.activeTerminalId ?? null + ) + // A running agent command is the precise signal that the agent is working + // in a shell; the desktop's agent cursor alone only says where it would. + const agentTerminalId = useCopilotTerminalStore((state) => { + const session = state.sessions[scopeId] + if (!session) return null + const [terminalId] = Object.values(session.agentCommandTerminalIds) + return terminalId ?? null + }) + // The stored title is only a fallback: the strip derives the live label, + // including a settled command name, from the same store itself. + const tabs = useMemo( + () => terminalTabs.map((tab) => ({ id: terminalResourceId(tab.terminalId), title: tab.title })), + [terminalTabs] + ) + + useDesktopTabResources({ + type: 'terminal', + scopeId, + tabs, + hasSession, + activeTabId: activeTerminalId && terminalResourceId(activeTerminalId), + agentTabId: agentTerminalId && terminalResourceId(agentTerminalId), + switchTab: showTerminal, + resources, + activeResourceId, + addResource, + removeResource, + selectResource, + onResourceEvent, + }) +} diff --git a/apps/sim/hooks/use-settled-terminal-commands.ts b/apps/sim/hooks/use-settled-terminal-commands.ts new file mode 100644 index 00000000000..e6be82b99cd --- /dev/null +++ b/apps/sim/hooks/use-settled-terminal-commands.ts @@ -0,0 +1,59 @@ +import { useEffect, useRef, useState } from 'react' +import type { TerminalTabState } from '@sim/terminal-protocol' + +/** + * How long a command must run before the tab names it. + * + * A tab that says what it is busy with is useful for a build you left running + * in the background, and pure noise for `ls` — swapping the label and spinning + * the icon for thirty milliseconds reads as a glitch. Waiting a beat keeps the + * signal and drops the flicker. + */ +const COMMAND_SETTLE_MS = 1_000 + +function sameIds(a: ReadonlySet, b: ReadonlySet): boolean { + return a.size === b.size && [...a].every((id) => b.has(id)) +} + +/** + * The terminals whose command has been running long enough to show. Returns a + * stable set, so a tab strip that would render identically does not re-render. + */ +export function useSettledTerminalCommands(tabs: readonly TerminalTabState[]): ReadonlySet { + const [settled, setSettled] = useState>(() => new Set()) + const startedAt = useRef | null>(null) + startedAt.current ??= new Map() + + useEffect(() => { + const started = startedAt.current + if (!started) return + const live = new Set(tabs.map((tab) => tab.terminalId)) + for (const id of [...started.keys()]) { + if (!live.has(id)) started.delete(id) + } + for (const tab of tabs) { + if (!tab.running) started.delete(tab.terminalId) + else if (!started.has(tab.terminalId)) started.set(tab.terminalId, Date.now()) + } + + const recompute = () => { + const now = Date.now() + const next = new Set() + let soonest = Number.POSITIVE_INFINITY + for (const [id, at] of started) { + const elapsed = now - at + if (elapsed >= COMMAND_SETTLE_MS) next.add(id) + else soonest = Math.min(soonest, COMMAND_SETTLE_MS - elapsed) + } + setSettled((current) => (sameIds(current, next) ? current : next)) + return soonest + } + + const soonest = recompute() + if (!Number.isFinite(soonest)) return + const timer = setTimeout(recompute, Math.max(0, soonest)) + return () => clearTimeout(timer) + }, [tabs]) + + return settled +} diff --git a/apps/sim/lib/copilot/chat/process-contents.test.ts b/apps/sim/lib/copilot/chat/process-contents.test.ts index 7bb3629eff7..1e58c2db153 100644 --- a/apps/sim/lib/copilot/chat/process-contents.test.ts +++ b/apps/sim/lib/copilot/chat/process-contents.test.ts @@ -529,11 +529,11 @@ describe('processContextsServer - MCP contexts', () => { }) describe('processContextsServer - browser and terminal selections', () => { - it('points every browser mention at its exact tab and describes the whole Terminal', async () => { + it('points every browser and terminal mention at its exact tab', async () => { const result = await processContextsServer( [ { kind: 'browser_tab', tabId: '3', label: 'Sim Docs' }, - { kind: 'terminal_tab', terminalId: 'terminal-session', label: 'Terminal' }, + { kind: 'terminal_tab', terminalId: '4', label: 'sim' }, ], 'user-1' ) @@ -546,12 +546,12 @@ describe('processContextsServer - browser and terminal selections', () => { }, { type: 'terminal_tab', - tag: '@Terminal', - content: expect.stringContaining('resource as a whole'), + tag: '@sim', + content: expect.stringContaining('terminalId 4'), }, ]) expect(result[0].content).toContain('browser_switch_tab') - expect(result[1].content).toContain('terminal list operation') + expect(result[1].content).toContain('pass that terminalId') }) it('keeps the live browser pointer and appends quoted untrusted page text', async () => { diff --git a/apps/sim/lib/copilot/chat/process-contents.ts b/apps/sim/lib/copilot/chat/process-contents.ts index 64ee4492775..028b499bdf9 100644 --- a/apps/sim/lib/copilot/chat/process-contents.ts +++ b/apps/sim/lib/copilot/chat/process-contents.ts @@ -19,7 +19,6 @@ import { truncateSelectionText, } from '@/lib/copilot/chat/selection-context' import { QueryLogs } from '@/lib/copilot/generated/tool-catalog-v1' -import { TERMINAL_SESSION_RESOURCE_ID } from '@/lib/copilot/resources/types' import { canonicalBlockVfsPath, canonicalKnowledgeBaseVfsDir, @@ -233,14 +232,6 @@ export async function processContextsServer( } } if (ctx.kind === 'terminal_tab' && ctx.terminalId) { - if (ctx.terminalId === TERMINAL_SESSION_RESOURCE_ID) { - return { - type: 'terminal_tab', - tag: ctx.label ? `@${ctx.label}` : '@Terminal', - content: - 'The user tagged the Terminal resource as a whole, not a specific shell. Inspect the live terminals with the terminal list operation and choose the relevant one from their request. If no terminal is open yet, create one as needed.', - } - } const pointer = `The user pointed at an open terminal: "${ctx.label}" (terminalId ${ctx.terminalId}). Act on THIS terminal — pass that terminalId to the terminal tool, and read its screen before assuming what is in it.` return { type: 'terminal_tab', diff --git a/apps/sim/lib/copilot/resources/availability.ts b/apps/sim/lib/copilot/resources/availability.ts index 442ce06e2eb..53445e10b9a 100644 --- a/apps/sim/lib/copilot/resources/availability.ts +++ b/apps/sim/lib/copilot/resources/availability.ts @@ -5,12 +5,9 @@ import { isTerminalAvailable } from '@/lib/terminal/transport' /** * Whether this client can show the resource's panel at all. * - * The browser and terminal panels are stored with the chat like any other - * resource, so the tab is still there when the chat is reopened. But they are - * windows onto something the desktop app owns — an embedded browser view, a - * pty — and opening that same chat in the web app would otherwise restore a - * tab that leads to an error. Such resources stay in the chat's stored - * resources either way; this only decides whether to put them on screen. + * Browser and terminal tabs are windows onto something the desktop app owns — + * an embedded browser view, a pty — so the web app has nothing to show for + * them. This only decides whether to put a resource on screen. */ export function canDisplayResource(resource: MothershipResource): boolean { if (!isDesktopOnlyResource(resource)) return true diff --git a/apps/sim/lib/copilot/resources/types.test.ts b/apps/sim/lib/copilot/resources/types.test.ts index 737f4628659..e8febfb1a93 100644 --- a/apps/sim/lib/copilot/resources/types.test.ts +++ b/apps/sim/lib/copilot/resources/types.test.ts @@ -1,7 +1,6 @@ import { describe, expect, it } from 'vitest' import { addCopilotChatResourceBodySchema } from '@/lib/api/contracts/copilot' import { - canonicalizeDesktopSessionResource, isAddressableResource, isDesktopOnlyResource, isEphemeralResource, @@ -12,7 +11,6 @@ import { PERSISTED_RESOURCE_TYPES, reorderStoredChatResources, sanitizeChatResources, - TERMINAL_SESSION_RESOURCE_ID, } from './types' function resource(overrides: Partial = {}): MothershipResource { @@ -20,16 +18,9 @@ function resource(overrides: Partial = {}): MothershipResour } describe('isEphemeralResource', () => { - it('persists the terminal panel so its tab survives reopening the chat', () => { - expect( - isEphemeralResource( - resource({ type: 'terminal', id: TERMINAL_SESSION_RESOURCE_ID, title: 'Terminal' }) - ) - ).toBe(false) - }) - - it('keeps browser tabs client-only because the desktop app restores its own pages', () => { + it('keeps browser and terminal tabs client-only because the desktop app restores them', () => { expect(isEphemeralResource(resource({ type: 'browser', id: '3', title: 'Slack' }))).toBe(true) + expect(isEphemeralResource(resource({ type: 'terminal', id: '3', title: 'sim' }))).toBe(true) }) it('keeps synthetic panels client-only', () => { @@ -69,15 +60,13 @@ describe('desktop session resource identity', () => { ).toEqual([{ type: 'file', id: 'file-1', title: 'report.csv' }]) }) - it('canonicalizes terminal inner-tab metadata without changing regular resources', () => { + it('drops stored terminal rows the same way', () => { expect( - canonicalizeDesktopSessionResource( - resource({ type: 'terminal', id: 'terminal-session:2', title: 'zsh' }) - ) - ).toEqual({ type: 'terminal', id: TERMINAL_SESSION_RESOURCE_ID, title: 'Terminal' }) - - const file = resource({ type: 'file', id: 'file-1', title: 'report.csv' }) - expect(canonicalizeDesktopSessionResource(file)).toBe(file) + sanitizeChatResources([ + resource({ type: 'terminal', id: 'terminal-session', title: 'Terminal' }), + resource({ type: 'file', id: 'file-1', title: 'report.csv' }), + ]) + ).toEqual([{ type: 'file', id: 'file-1', title: 'report.csv' }]) }) }) @@ -160,13 +149,6 @@ describe('unaddressable resources', () => { ]) }) - it('keeps the terminal panel, which is given its id by canonicalization', () => { - const sanitized = sanitizeChatResources([ - resource({ type: 'terminal', id: '', title: 'Terminal' }), - ]) - expect(sanitized.map((r) => r.id)).toEqual([TERMINAL_SESSION_RESOURCE_ID]) - }) - it('refuses a blank id at the write boundary, matching the send path', () => { const parsed = addCopilotChatResourceBodySchema.safeParse({ chatId: 'chat-1', diff --git a/apps/sim/lib/copilot/resources/types.ts b/apps/sim/lib/copilot/resources/types.ts index 6f40c59dc6d..159fe16cc62 100644 --- a/apps/sim/lib/copilot/resources/types.ts +++ b/apps/sim/lib/copilot/resources/types.ts @@ -86,11 +86,11 @@ const RESOURCE_POLICY: Record = { integration: { persisted: true }, // A synthetic panel with no addressable entity behind it to reopen. generic: { persisted: false }, - // One tab per live desktop browser page, keyed by the native tab id. The - // desktop app owns the page list and restores it itself, so the chat row - // never stores these; they are re-derived from the live tab list on open. + // One tab per live desktop page or shell, keyed by the native id. The + // desktop app owns those lists and restores them itself, so the chat row + // never stores these; they are re-derived from the live lists on open. browser: { persisted: false, desktopOnly: true }, - terminal: { persisted: true, desktopOnly: true }, + terminal: { persisted: false, desktopOnly: true }, } /** @@ -119,27 +119,6 @@ export function isEphemeralResource(resource: MothershipResource): boolean { return !RESOURCE_POLICY[resource.type]?.persisted } -/** - * Singleton id for the live terminal panel. Only the metadata is stored — - * reopening the chat brings the panel back with a fresh shell, since the pty - * and its scrollback belong to the desktop app and do not outlive it. - */ -export const TERMINAL_SESSION_RESOURCE_ID = 'terminal-session' - -/** - * Collapses shell-shaped metadata onto the one top-level terminal panel each - * chat can restore. Terminal tabs are inner tabs, not independently - * addressable Mothership resources. - */ -export function canonicalizeDesktopSessionResource( - resource: MothershipResource -): MothershipResource { - if (resource.type === 'terminal') { - return { type: 'terminal', id: TERMINAL_SESSION_RESOURCE_ID, title: 'Terminal' } - } - return resource -} - /** * Whether an id value names something the app can act on. * @@ -165,42 +144,27 @@ export function isAddressableResource(resource: MothershipResource): boolean { } /** - * Canonicalizes and deduplicates the singleton terminal panel in display - * order, and drops browser rows: older clients stored one per page, but the - * live tab list is derived from the desktop app rather than the chat row. - * Module-private: callers want {@link sanitizeChatResources}, which also drops - * unaddressable resources. + * Drops browser and terminal rows: older clients stored the desktop panels on + * the chat, but their live tabs are derived from the desktop app rather than + * the chat row. Module-private: callers want {@link sanitizeChatResources}, + * which also drops unaddressable resources. */ -function canonicalizeDesktopSessionResources( +function withoutDesktopSessionResources( resources: readonly MothershipResource[] ): MothershipResource[] { - let seenTerminal = false - const canonical: MothershipResource[] = [] - - for (const resource of resources) { - if (resource.type === 'browser') continue - if (resource.type === 'terminal') { - if (seenTerminal) continue - seenTerminal = true - } - canonical.push(canonicalizeDesktopSessionResource(resource)) - } - - return canonical + return resources.filter((resource) => !RESOURCE_POLICY[resource.type]?.desktopOnly) } /** - * The canonical form of a chat's resource list: the terminal panel collapsed, - * legacy browser rows and unaddressable resources dropped. Every path that - * reads or writes stored resources goes through this, which is what heals - * chats that already hold one. Canonicalization runs first, so the terminal - * panel — which is given its id there — is never dropped for arriving without + * The canonical form of a chat's resource list: legacy desktop panel rows and + * unaddressable resources dropped. Every path that reads or writes stored + * resources goes through this, which is what heals chats that already hold * one. */ export function sanitizeChatResources( resources: readonly MothershipResource[] ): MothershipResource[] { - return canonicalizeDesktopSessionResources(resources).filter(isAddressableResource) + return withoutDesktopSessionResources(resources).filter(isAddressableResource) } /** diff --git a/apps/sim/lib/terminal/focus.ts b/apps/sim/lib/terminal/focus.ts new file mode 100644 index 00000000000..91036bbc64d --- /dev/null +++ b/apps/sim/lib/terminal/focus.ts @@ -0,0 +1,30 @@ +/** + * "Put the keyboard in this terminal" — sent by the resource strip when the + * user picks a terminal tab with the pointer or opens a new shell. The strip + * cannot reach the panel's xterm instances, so the request travels as a + * window CustomEvent and the terminal panel subscribes via + * {@link onTerminalFocusRequest}. Keyboard navigation along the strip does + * not send one, so arrow keys keep working there. + */ +const TERMINAL_FOCUS_EVENT = 'sim:focus-terminal' + +interface TerminalFocusDetail { + terminalId: string +} + +/** Asks the terminal panel to focus one shell once it is on screen. */ +export function requestTerminalFocus(terminalId: string): void { + window.dispatchEvent( + new CustomEvent(TERMINAL_FOCUS_EVENT, { detail: { terminalId } }) + ) +} + +/** Subscribes the terminal panel to focus requests; returns an unsubscribe. */ +export function onTerminalFocusRequest(callback: (terminalId: string) => void): () => void { + const listener = (event: Event) => { + const terminalId = (event as CustomEvent).detail?.terminalId + if (typeof terminalId === 'string' && terminalId) callback(terminalId) + } + window.addEventListener(TERMINAL_FOCUS_EVENT, listener) + return () => window.removeEventListener(TERMINAL_FOCUS_EVENT, listener) +} diff --git a/apps/sim/lib/terminal/resource-id.test.ts b/apps/sim/lib/terminal/resource-id.test.ts new file mode 100644 index 00000000000..8aea74bce7d --- /dev/null +++ b/apps/sim/lib/terminal/resource-id.test.ts @@ -0,0 +1,12 @@ +/** + * @vitest-environment node + */ +import { describe, expect, it } from 'vitest' +import { terminalIdFromResourceId, terminalResourceId } from '@/lib/terminal/resource-id' + +describe('terminal resource ids', () => { + it('keeps a shell apart from a browser page with the same native id', () => { + expect(terminalResourceId('1')).not.toBe('1') + expect(terminalIdFromResourceId(terminalResourceId('1'))).toBe('1') + }) +}) diff --git a/apps/sim/lib/terminal/resource-id.ts b/apps/sim/lib/terminal/resource-id.ts new file mode 100644 index 00000000000..76b86d27e2f --- /dev/null +++ b/apps/sim/lib/terminal/resource-id.ts @@ -0,0 +1,17 @@ +const TERMINAL_RESOURCE_PREFIX = 'terminal:' + +/** + * The resource id for a live shell. Native terminal ids and browser tab ids + * are both small per-chat counters, and the strip resolves resources by id + * alone, so a shell's resource carries a namespace the page's does not. + */ +export function terminalResourceId(terminalId: string): string { + return `${TERMINAL_RESOURCE_PREFIX}${terminalId}` +} + +/** The native terminal id behind a terminal resource. */ +export function terminalIdFromResourceId(resourceId: string): string { + return resourceId.startsWith(TERMINAL_RESOURCE_PREFIX) + ? resourceId.slice(TERMINAL_RESOURCE_PREFIX.length) + : resourceId +} diff --git a/apps/sim/lib/terminal/tab-label.test.ts b/apps/sim/lib/terminal/tab-label.test.ts new file mode 100644 index 00000000000..6c03dcadafb --- /dev/null +++ b/apps/sim/lib/terminal/tab-label.test.ts @@ -0,0 +1,53 @@ +/** + * @vitest-environment node + */ +import type { TerminalTabState } from '@sim/terminal-protocol' +import { describe, expect, it } from 'vitest' +import { terminalTabTitle, terminalTooltip } from '@/lib/terminal/tab-label' + +const idleTab: TerminalTabState = { + terminalId: 'terminal-1', + title: 'sim', + cwd: '/Users/ada/sim', + running: null, + interactive: false, + active: true, +} + +describe('terminalTooltip', () => { + it('summarizes a long compound heredoc command by its foreground program', () => { + const running = `mkdir -p ~/.bot/bin && cat > ~/.bot/bin/cli-mock <<'END' +#!/usr/bin/env node +const carts = new Map() +process.stdout.write(JSON.stringify([...carts])) +END +chmod +x ~/.bot/bin/cli-mock && echo '--- smoke test ---' && ~/.bot/bin/cli-mock submit mock_123` + const tooltip = terminalTooltip({ ...idleTab, title: 'mkdir', cwd: '/Users/ada', running }) + + expect(tooltip).toBe('/Users/ada — cli-mock') + expect(tooltip).not.toContain('const carts') + }) + + it('preserves the working-directory tooltip for idle terminals', () => { + expect(terminalTooltip(idleTab)).toBe('/Users/ada/sim') + expect(terminalTooltip({ ...idleTab, cwd: null })).toBe('Terminal') + }) +}) + +describe('terminalTabTitle', () => { + it('names an idle shell after its directory', () => { + expect(terminalTabTitle(idleTab, new Set())).toBe('sim') + }) + + it('names a shell after a command only once it has settled', () => { + const building = { ...idleTab, running: 'bun run build' } + expect(terminalTabTitle(building, new Set())).toBe('sim') + expect(terminalTabTitle(building, new Set(['terminal-1']))).toBe('bun run build') + }) + + it('names a full-screen program immediately', () => { + expect(terminalTabTitle({ ...idleTab, running: 'vim', interactive: true }, new Set())).toBe( + 'vim' + ) + }) +}) diff --git a/apps/sim/lib/terminal/tab-label.ts b/apps/sim/lib/terminal/tab-label.ts new file mode 100644 index 00000000000..08f783888f7 --- /dev/null +++ b/apps/sim/lib/terminal/tab-label.ts @@ -0,0 +1,23 @@ +import { describeRunningCommand, type TerminalTabState } from '@sim/terminal-protocol' + +/** Full working directory, plus a concise name for whatever the shell is running. */ +export function terminalTooltip(tab: TerminalTabState): string { + const where = tab.cwd ?? 'Terminal' + return tab.running ? `${where} — ${describeRunningCommand(tab.running)}` : where +} + +/** + * Whether a tab should be named after what it is running rather than where it + * is. A full-screen program is named the moment it appears: the delay exists + * to stop `ls` flickering the label, and an editor or coding agent is not a + * transient command — it holds the terminal until it is quit, so there is + * nothing to wait out. + */ +export function namesItsCommand(tab: TerminalTabState, settled: ReadonlySet): boolean { + return Boolean(tab.running) && (tab.interactive || settled.has(tab.terminalId)) +} + +/** The strip label for a terminal: its settled foreground program, else its cwd basename. */ +export function terminalTabTitle(tab: TerminalTabState, settled: ReadonlySet): string { + return namesItsCommand(tab, settled) ? (tab.running ?? tab.title) : tab.title +} diff --git a/apps/sim/lib/terminal/transport.test.ts b/apps/sim/lib/terminal/transport.test.ts index 637f8b95c5a..d2a8ac788dd 100644 --- a/apps/sim/lib/terminal/transport.test.ts +++ b/apps/sim/lib/terminal/transport.test.ts @@ -3,6 +3,7 @@ import { beforeAll, beforeEach, describe, expect, it, vi } from 'vitest' const { activateScope, + activateStoreScope, applyCommandEvent, clearScrollback, discardScope, @@ -11,15 +12,19 @@ const { markScopeSuspended, migrateStoreScope, nativeMigrateScope, + nativeOpenTerminal, nativeReorderTerminal, + nativeStart, onCommand, onData, onDefaultZoomChanged, onShortcutCommand, onTabs, onScopeSuspended, + restoreScope, setTabs, nativeSuspendScope, + nativeSwitchTerminal, write, } = vi.hoisted(() => ({ activateScope: vi.fn(async (scopeId: string) => ({ @@ -27,6 +32,7 @@ const { tabs: [], activeTerminalId: null, })), + activateStoreScope: vi.fn(), applyCommandEvent: vi.fn(), clearScrollback: vi.fn(async () => true), discardScope: vi.fn(), @@ -39,45 +45,58 @@ const { markScopeSuspended: vi.fn(), migrateStoreScope: vi.fn(), nativeMigrateScope: vi.fn(), + nativeOpenTerminal: vi.fn(async (_cwd: string | undefined, scopeId: string) => ({ + scopeId, + tabs: [], + activeTerminalId: null, + })), nativeReorderTerminal: vi.fn(), + nativeStart: vi.fn(), onCommand: vi.fn(), onData: vi.fn(() => vi.fn()), onDefaultZoomChanged: vi.fn(() => vi.fn()), onShortcutCommand: vi.fn(() => vi.fn()), onTabs: vi.fn(), onScopeSuspended: vi.fn(), + restoreScope: vi.fn(async (scopeId: string) => ({ + scopeId, + tabs: [], + activeTerminalId: null, + })), setTabs: vi.fn(), nativeSuspendScope: vi.fn(async () => true), + nativeSwitchTerminal: vi.fn(async () => {}), write: vi.fn(), })) +const bridgeTerminal = vi.hoisted(() => ({}) as Record) +Object.assign(bridgeTerminal, { + activateScope, + closeTerminal: vi.fn(), + clearScrollback, + dispose: vi.fn(), + disposeScope, + executeTool: vi.fn(), + getScrollback: vi.fn(), + getTabs, + migrateScope: nativeMigrateScope, + onCommand, + onData, + onDefaultZoomChanged, + onShortcutCommand, + onTabs, + onScopeSuspended, + openTerminal: nativeOpenTerminal, + reorderTerminal: nativeReorderTerminal, + resize: vi.fn(), + restoreScope, + switchTerminal: nativeSwitchTerminal, + suspendScope: nativeSuspendScope, + write, +}) + vi.mock('@/lib/desktop', () => ({ - getDesktopBridge: () => ({ - terminal: { - activateScope, - closeTerminal: vi.fn(), - clearScrollback, - dispose: vi.fn(), - disposeScope, - executeTool: vi.fn(), - getScrollback: vi.fn(), - getTabs, - migrateScope: nativeMigrateScope, - onCommand, - onData, - onDefaultZoomChanged, - onShortcutCommand, - onTabs, - onScopeSuspended, - openTerminal: vi.fn(), - reorderTerminal: nativeReorderTerminal, - resize: vi.fn(), - start: vi.fn(), - switchTerminal: vi.fn(), - suspendScope: nativeSuspendScope, - write, - }, - }), + getDesktopBridge: () => ({ terminal: bridgeTerminal }), isTerminalEnabled: () => true, })) @@ -85,6 +104,8 @@ vi.mock('@/stores/copilot-terminal/store', () => ({ useCopilotTerminalStore: { getState: () => ({ activeScopeId: null, + sessions: {}, + activateScope: activateStoreScope, applyCommandEvent, discardScope, migrateScope: migrateStoreScope, @@ -95,6 +116,7 @@ vi.mock('@/stores/copilot-terminal/store', () => ({ })) import { + activateTerminalScope, clearTerminalScrollback, discardTerminalScope, initTerminalTransport, @@ -102,8 +124,10 @@ import { onTerminalData, onTerminalDefaultZoomChanged, onTerminalShortcutCommand, + openTerminal, reorderTerminal, suspendTerminalScope, + switchTerminal, writeToTerminal, } from '@/lib/terminal/transport' @@ -124,9 +148,101 @@ describe('terminal transport chat scopes', () => { migrateStoreScope.mockClear() nativeMigrateScope.mockReset() nativeReorderTerminal.mockReset() + activateScope.mockClear() + restoreScope.mockClear() + nativeSwitchTerminal.mockClear() + nativeOpenTerminal.mockClear() + nativeStart.mockClear() write.mockClear() }) + it('restores a chat with no live shells when its scope is activated', async () => { + restoreScope.mockResolvedValueOnce({ + scopeId: 'chat-restore', + tabs: [ + { + terminalId: 'restored-1', + title: 'sim', + cwd: '/code/sim', + running: null, + interactive: false, + active: true, + }, + ], + activeTerminalId: 'restored-1', + }) + + await activateTerminalScope('chat-restore') + + expect(restoreScope).toHaveBeenCalledWith('chat-restore') + expect(setTabs).toHaveBeenLastCalledWith( + expect.objectContaining({ scopeId: 'chat-restore', activeTerminalId: 'restored-1' }) + ) + }) + + it('does not restore when the chat already has live shells', async () => { + activateScope.mockResolvedValueOnce({ + scopeId: 'chat-live', + tabs: [ + { + terminalId: 'live-1', + title: 'sim', + cwd: '/code/sim', + running: null, + interactive: false, + active: true, + }, + ], + activeTerminalId: 'live-1', + }) + + await activateTerminalScope('chat-live') + + expect(restoreScope).not.toHaveBeenCalled() + }) + + it('skips the restore when the user moved to another chat during activation', async () => { + let finishActivation: (tabs: ScopedTerminalTabsState) => void = () => {} + activateScope.mockImplementationOnce( + () => new Promise((resolve) => (finishActivation = resolve)) + ) + + const first = activateTerminalScope('chat-first') + await activateTerminalScope('chat-second') + finishActivation({ scopeId: 'chat-first', tabs: [], activeTerminalId: null }) + await first + + expect(restoreScope).toHaveBeenCalledExactlyOnceWith('chat-second') + }) + + it('opens a fresh shell through openTerminal on shells that restore on activation', async () => { + await openTerminal(undefined, 'chat-b') + + expect(nativeOpenTerminal).toHaveBeenCalledWith(undefined, 'chat-b') + expect(nativeStart).not.toHaveBeenCalled() + }) + + it('adopts a chat through start on shells that cannot restore on activation', async () => { + const { restoreScope: modern } = bridgeTerminal + bridgeTerminal.restoreScope = undefined + bridgeTerminal.start = nativeStart + try { + await openTerminal(undefined, 'chat-b') + } finally { + bridgeTerminal.restoreScope = modern + bridgeTerminal.start = undefined + } + + expect(nativeStart).toHaveBeenCalledWith({ cols: 80, rows: 24 }, 'chat-b') + expect(nativeOpenTerminal).not.toHaveBeenCalled() + }) + + it('forwards a terminal switch with its claim option', async () => { + await switchTerminal('terminal-b', 'chat-b', { claim: false }) + + expect(nativeSwitchTerminal).toHaveBeenCalledWith('terminal-b', 'chat-b', { claim: false }) + }) + it('routes pushed tab and command state to the scope carried by each event', () => { const tabsListener = onTabs.mock.calls[0][0] as (state: ScopedTerminalTabsState) => void const commandListener = onCommand.mock.calls[0][0] as ( diff --git a/apps/sim/lib/terminal/transport.ts b/apps/sim/lib/terminal/transport.ts index b8ffa1aa29e..f2435b1668f 100644 --- a/apps/sim/lib/terminal/transport.ts +++ b/apps/sim/lib/terminal/transport.ts @@ -19,7 +19,6 @@ import { import type { ScopedTerminalTabsState, TerminalOperation, - TerminalStartOptions, TerminalToolArgs, } from '@sim/terminal-protocol' import { getDesktopBridge, isTerminalEnabled } from '@/lib/desktop' @@ -68,14 +67,22 @@ export function initTerminalTransport(): void { terminal.onScopeSuspended(applyTerminalScopeSuspended) } -/** Makes one chat's terminal group active in both renderer and desktop. */ +/** + * Makes one chat's terminal group active in both renderer and desktop, then + * materializes its saved shells. Each live shell is a resource tab, so the + * tab list has to exist before any terminal panel is mounted. + */ export async function activateTerminalScope(scopeId: string): Promise { activeScopeId = scopeId useCopilotTerminalStore.getState().activateScope(scopeId) const terminal = bridge() if (!terminal) return const tabs = await terminal.activateScope(scopeId) + if (tabs.scopeId !== scopeId) return useCopilotTerminalStore.getState().setTabs(tabs) + if (tabs.tabs.length > 0 || activeScopeId !== scopeId || !terminal.restoreScope) return + const restored = await terminal.restoreScope(scopeId) + if (restored.scopeId === scopeId) useCopilotTerminalStore.getState().setTabs(restored) } /** Rebinds a pending new-chat terminal group to the chat id assigned by the server. */ @@ -212,17 +219,6 @@ export async function clearTerminalScrollback( return (await bridge()?.clearScrollback(terminalId, scopeId)) ?? false } -export async function startTerminalSession( - options: TerminalStartOptions, - scopeId = currentTerminalScopeId() -): Promise { - const terminal = bridge() - if (!terminal) { - throw new Error('The Sim desktop terminal is unavailable.') - } - return terminal.start(options, scopeId) -} - export function writeToTerminal( terminalId: string, data: string, @@ -261,14 +257,22 @@ export async function openTerminal( ): Promise { const terminal = bridge() if (!terminal) throw new Error('The Sim desktop terminal is unavailable.') + const live = useCopilotTerminalStore.getState().sessions[scopeId]?.tabs.tabs.length ?? 0 + // A shell without `restoreScope` only applies a chat's saved shells through + // `start`; opening a fresh one first would overwrite that saved set. + if (!terminal.restoreScope && terminal.start && live === 0 && cwd === undefined) { + return terminal.start({ cols: 80, rows: 24 }, scopeId) + } return terminal.openTerminal(cwd, scopeId) } +/** Shows a terminal; `claim: false` mirrors a strip selection without claiming the shell. */ export async function switchTerminal( terminalId: string, - scopeId = currentTerminalScopeId() + scopeId = currentTerminalScopeId(), + options?: { claim?: boolean } ): Promise { - await bridge()?.switchTerminal(terminalId, scopeId) + await bridge()?.switchTerminal(terminalId, scopeId, options) } /** Moves a terminal tab when the installed shell supports ordering. */ diff --git a/packages/desktop-bridge/contract-snapshot.ts b/packages/desktop-bridge/contract-snapshot.ts index f402810285a..f81d290db5f 100644 --- a/packages/desktop-bridge/contract-snapshot.ts +++ b/packages/desktop-bridge/contract-snapshot.ts @@ -959,8 +959,18 @@ export function isPendingDesktopScopeId(scopeId: string): boolean { * environment stay consistent between the two. */ export interface SimDesktopTerminalApi { - /** Open the first terminal, or adopt the ones already running. */ - start(options: TerminalStartOptions, scopeId: string): Promise + /** + * Materializes a chat's saved shells without opening one for a chat that + * had none. Optional for compatibility with installed shells that only + * restored when the terminal panel started. + */ + restoreScope?(scopeId: string): Promise + /** + * Opens the first terminal, or adopts the chat's saved shells. Only shells + * without {@link restoreScope} still expose it; newer ones restore on + * activation and open shells one at a time. + */ + start?(options: TerminalStartOptions, scopeId: string): Promise /** * Execute one terminal operation. Resolves with the outcome; never rejects * for tool-level failures (those ride `ok: false`). @@ -986,7 +996,15 @@ export interface SimDesktopTerminalApi { resize(terminalId: string, cols: number, rows: number, scopeId: string): void /** Open an additional terminal and make it active. */ openTerminal(cwd: string | undefined, scopeId: string): Promise - switchTerminal(terminalId: string, scopeId: string): Promise + /** + * Show a terminal. `claim: false` mirrors a resource-strip selection without + * recording the shell as the user's own; older shells treat every switch as a claim. + */ + switchTerminal( + terminalId: string, + scopeId: string, + options?: { claim?: boolean } + ): Promise /** Move a terminal to its final position. Optional for older installed shells. */ reorderTerminal?( terminalId: string, diff --git a/packages/desktop-bridge/src/index.ts b/packages/desktop-bridge/src/index.ts index f1be676d39c..f91713aec1e 100644 --- a/packages/desktop-bridge/src/index.ts +++ b/packages/desktop-bridge/src/index.ts @@ -51,8 +51,18 @@ export function isPendingDesktopScopeId(scopeId: string): boolean { * environment stay consistent between the two. */ export interface SimDesktopTerminalApi { - /** Open the first terminal, or adopt the ones already running. */ - start(options: TerminalStartOptions, scopeId: string): Promise + /** + * Materializes a chat's saved shells without opening one for a chat that + * had none. Optional for compatibility with installed shells that only + * restored when the terminal panel started. + */ + restoreScope?(scopeId: string): Promise + /** + * Opens the first terminal, or adopts the chat's saved shells. Only shells + * without {@link restoreScope} still expose it; newer ones restore on + * activation and open shells one at a time. + */ + start?(options: TerminalStartOptions, scopeId: string): Promise /** * Execute one terminal operation. Resolves with the outcome; never rejects * for tool-level failures (those ride `ok: false`). @@ -78,7 +88,15 @@ export interface SimDesktopTerminalApi { resize(terminalId: string, cols: number, rows: number, scopeId: string): void /** Open an additional terminal and make it active. */ openTerminal(cwd: string | undefined, scopeId: string): Promise - switchTerminal(terminalId: string, scopeId: string): Promise + /** + * Show a terminal. `claim: false` mirrors a resource-strip selection without + * recording the shell as the user's own; older shells treat every switch as a claim. + */ + switchTerminal( + terminalId: string, + scopeId: string, + options?: { claim?: boolean } + ): Promise /** Move a terminal to its final position. Optional for older installed shells. */ reorderTerminal?( terminalId: string, From 8c276f8d999b2ad6a64fddb9091d7083c47c3809 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Thu, 10 Sep 2026 17:54:38 -0700 Subject: [PATCH 2/3] fix(desktop): close hidden terminal tabs from the strip and keep the activity icon on terminal tabs --- apps/desktop/src/main/terminal/index.ts | 8 ++++-- .../src/main/terminal/registry.test.ts | 27 +++++++++++++++---- apps/desktop/src/main/terminal/registry.ts | 5 +++- .../resource-registry/resource-registry.tsx | 16 +++++------ .../resource-tabs/resource-tabs.tsx | 18 ++++++++----- 5 files changed, 51 insertions(+), 23 deletions(-) diff --git a/apps/desktop/src/main/terminal/index.ts b/apps/desktop/src/main/terminal/index.ts index 49435ca3b00..1a36b5e7658 100644 --- a/apps/desktop/src/main/terminal/index.ts +++ b/apps/desktop/src/main/terminal/index.ts @@ -633,9 +633,13 @@ export class TerminalService { ) } - /** Whether one renderer may close a tab in the terminal panel it displays. */ + /** + * Whether one renderer may close a tab. The strip that lists shells sits + * outside the terminal panel, so the shell need not be on screen; IPC has + * already checked that the renderer is on the shell's chat. + */ acceptsUserClose(owner: WebContents, terminalId: string): boolean { - return !owner.isDestroyed() && this.visibleOwner === owner && this.sessions.has(terminalId) + return !owner.isDestroyed() && this.sessions.has(terminalId) } /** Drops the claim and unsubscribes from the owner's lifecycle. */ diff --git a/apps/desktop/src/main/terminal/registry.test.ts b/apps/desktop/src/main/terminal/registry.test.ts index 3697e02420e..a4266c854f9 100644 --- a/apps/desktop/src/main/terminal/registry.test.ts +++ b/apps/desktop/src/main/terminal/registry.test.ts @@ -222,6 +222,24 @@ describe('TerminalRegistry', () => { expect(persistence.save).toHaveBeenLastCalledWith('chat-A', { v: 1, tabs: [], activeIndex: 0 }) }) + it('does not bring back a shell closed earlier in the same session', () => { + const persistence: TerminalScopePersistence = { + load: vi.fn(() => undefined), + save: vi.fn(() => true), + migrate: vi.fn(() => true), + disposeScope: vi.fn(), + } + const terminals = new TerminalRegistry(persistence) + terminals.setSink(sink()) + const first = terminals.openTerminal('chat-A').activeTerminalId as string + terminals.closeTerminal('chat-A', first) + + const reopened = terminals.openTerminal('chat-A') + + expect(reopened.tabs).toHaveLength(1) + expect(stubSessions.filter((session) => !session.disposed)).toHaveLength(1) + }) + it('keeps a saved descriptor that was never applied', () => { const persistence: TerminalScopePersistence = { load: vi.fn(() => ({ v: 1 as const, tabs: [{ cwd: tmpdir() }], activeIndex: 0 })), @@ -602,7 +620,7 @@ describe('TerminalRegistry', () => { expect(activeSession?.writes).toEqual(['a']) }) - it('closes tabs only for the renderer displaying their terminal scope', () => { + it('closes a tab from the strip while the shell panel is hidden', () => { const terminals = registry() const first = terminals.openTerminal('chat-A').activeTerminalId as string const second = terminals.openTerminal('chat-A').activeTerminalId as string @@ -612,15 +630,14 @@ describe('TerminalRegistry', () => { on: vi.fn(), removeListener: vi.fn(), } - const other = { ...owner, once: vi.fn(), on: vi.fn(), removeListener: vi.fn() } + const gone = { ...owner, isDestroyed: () => true } - expect(terminals.closeUserTerminal('chat-A', first, owner as never).tabs).toHaveLength(2) - terminals.setPanelVisible('chat-A', true, owner as never) - expect(terminals.closeUserTerminal('chat-A', first, other as never).tabs).toHaveLength(2) + // The strip lives outside the panel, so a hidden shell is still closable. expect(terminals.closeUserTerminal('chat-B', '1', owner as never)).toEqual({ tabs: [], activeTerminalId: null, }) + expect(terminals.closeUserTerminal('chat-A', first, gone as never).tabs).toHaveLength(2) const closed = terminals.closeUserTerminal('chat-A', first, owner as never) expect(closed.tabs).toHaveLength(1) diff --git a/apps/desktop/src/main/terminal/registry.ts b/apps/desktop/src/main/terminal/registry.ts index 59e91c4d84d..b6a4ac7c90c 100644 --- a/apps/desktop/src/main/terminal/registry.ts +++ b/apps/desktop/src/main/terminal/registry.ts @@ -183,7 +183,7 @@ export class TerminalRegistry { return this.serviceFor(scope).closeTerminal(terminalId) } - /** Closes a tab only from the renderer that currently displays its scope. */ + /** Closes a tab for a renderer on its scope; IPC checks the scope claim. */ closeUserTerminal(scope: string, terminalId: string, owner: WebContents): TerminalTabsState { if (this.suspendedScopes.has(scope)) return { tabs: [], activeTerminalId: null } const service = this.entries.get(scope)?.service @@ -524,7 +524,10 @@ export class TerminalRegistry { private save(entry: TerminalRegistryEntry, snapshot: TerminalSessionSnapshot): boolean { if (!this.persistence?.save(entry.scope, snapshot)) return false + // A descriptor written from live shells describes them, so there is + // nothing left to apply; only one loaded from disk can still be pending. entry.persisted = snapshot + entry.restoreApplied = true return true } diff --git a/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-registry/resource-registry.tsx b/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-registry/resource-registry.tsx index 74713bd7e2d..f4f2b11dd68 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-registry/resource-registry.tsx +++ b/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-registry/resource-registry.tsx @@ -150,12 +150,8 @@ export const RESOURCE_REGISTRY: Record ( - + renderTabIcon: (_resource, className) => ( + ), renderDropdownItem: (props) => , }, @@ -254,8 +250,12 @@ export const RESOURCE_REGISTRY: Record ( - + renderTabIcon: (resource, className, desktopScopeId) => ( + ), renderDropdownItem: (props) => , }, diff --git a/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-tabs/resource-tabs.tsx b/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-tabs/resource-tabs.tsx index 6ce74c4a141..66d91c0e0be 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-tabs/resource-tabs.tsx +++ b/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-tabs/resource-tabs.tsx @@ -413,17 +413,21 @@ export function ResourceTabs({ const isMulti = selectedIds.has(resource.id) && selectedIds.size > 1 const targets = isMulti ? resources.filter((r) => selectedIds.has(r.id)) : [resource] if (!confirmClosingRunningTerminals(targets, terminalTabs)) return - // Update parent state immediately for all targets. A browser tab's page - // or a terminal's shell is closed natively too; the tab list then - // confirms the removal. + // A browser tab's page is closed natively and its resource dropped at + // once; the tab list then confirms the removal. A shell's close answers + // with the tab list, so its resource follows that list instead — a + // close the desktop app refuses must not leave a running shell with no + // tab. for (const r of targets) { - onRemoveResource(r.type, r.id) - if (r.type === 'browser') { - sendBrowserPanelAction('close-tab', { tabId: r.id }, desktopScopeId) - } else if (r.type === 'terminal') { + if (r.type === 'terminal') { void closeTerminal(terminalIdFromResourceId(r.id), desktopScopeId).catch(() => toast.error('Could not close that terminal. Please try again.') ) + continue + } + onRemoveResource(r.type, r.id) + if (r.type === 'browser') { + sendBrowserPanelAction('close-tab', { tabId: r.id }, desktopScopeId) } } // Clear stale selection and anchor for all removed targets From ede877fe986a3da7257b179059214dd08f5c9fb8 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Thu, 10 Sep 2026 18:13:14 -0700 Subject: [PATCH 3/3] fix(desktop): keep terminal close ownership with the window showing the panel --- apps/desktop/src/main/terminal/index.ts | 10 +++++++--- apps/desktop/src/main/terminal/registry.test.ts | 5 +++++ 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/apps/desktop/src/main/terminal/index.ts b/apps/desktop/src/main/terminal/index.ts index 1a36b5e7658..3f9367a448c 100644 --- a/apps/desktop/src/main/terminal/index.ts +++ b/apps/desktop/src/main/terminal/index.ts @@ -635,11 +635,15 @@ export class TerminalService { /** * Whether one renderer may close a tab. The strip that lists shells sits - * outside the terminal panel, so the shell need not be on screen; IPC has - * already checked that the renderer is on the shell's chat. + * outside the terminal panel, so a renderer on the chat may close a shell + * nobody is displaying; while a window does display the panel, only that + * window may close, so a second window on the same chat cannot end a shell + * someone is using. */ acceptsUserClose(owner: WebContents, terminalId: string): boolean { - return !owner.isDestroyed() && this.sessions.has(terminalId) + if (owner.isDestroyed() || !this.sessions.has(terminalId)) return false + const shown = this.visibleOwner && !this.visibleOwner.isDestroyed() ? this.visibleOwner : null + return shown === null || shown === owner } /** Drops the claim and unsubscribes from the owner's lifecycle. */ diff --git a/apps/desktop/src/main/terminal/registry.test.ts b/apps/desktop/src/main/terminal/registry.test.ts index a4266c854f9..a3f50e674c3 100644 --- a/apps/desktop/src/main/terminal/registry.test.ts +++ b/apps/desktop/src/main/terminal/registry.test.ts @@ -631,6 +631,7 @@ describe('TerminalRegistry', () => { removeListener: vi.fn(), } const gone = { ...owner, isDestroyed: () => true } + const other = { ...owner, once: vi.fn(), on: vi.fn(), removeListener: vi.fn() } // The strip lives outside the panel, so a hidden shell is still closable. expect(terminals.closeUserTerminal('chat-B', '1', owner as never)).toEqual({ @@ -638,6 +639,10 @@ describe('TerminalRegistry', () => { activeTerminalId: null, }) expect(terminals.closeUserTerminal('chat-A', first, gone as never).tabs).toHaveLength(2) + // While another window displays the panel, only that window may close. + terminals.setPanelVisible('chat-A', true, other as never) + expect(terminals.closeUserTerminal('chat-A', first, owner as never).tabs).toHaveLength(2) + terminals.setPanelVisible('chat-A', false, other as never) const closed = terminals.closeUserTerminal('chat-A', first, owner as never) expect(closed.tabs).toHaveLength(1)