Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions apps/desktop/src/main/ipc.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand Down
41 changes: 11 additions & 30 deletions apps/desktop/src/main/ipc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
}
},
},
Expand Down Expand Up @@ -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 }
},
Expand Down Expand Up @@ -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)
Expand Down
60 changes: 30 additions & 30 deletions apps/desktop/src/main/terminal/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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)) {
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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. */
Expand Down
Loading
Loading