Merge origin/staging into codex/integration-actions-simplify

Author: waleedlatif1

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 () => {

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)

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,32 +451,24 @@ 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()
     this.sessions.delete(terminalId)
     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
     }
@@ -641,9 +633,17 @@ 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 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.visibleOwner === owner && 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. */