From 2e1002063459f33837f90d6e4b3489e99255b899 Mon Sep 17 00:00:00 2001 From: kavyansh18 Date: Fri, 11 Sep 2026 09:42:19 +0530 Subject: [PATCH 1/4] Fix terminal split layout restoration for agent sessions --- .../browser/sessionsTerminalContribution.ts | 34 ++++- .../sessionsTerminalContribution.test.ts | 130 +++++++++++++++++- .../contrib/terminal/browser/terminal.ts | 2 +- .../terminal/browser/terminalService.ts | 9 +- 4 files changed, 171 insertions(+), 4 deletions(-) diff --git a/src/vs/sessions/contrib/terminal/browser/sessionsTerminalContribution.ts b/src/vs/sessions/contrib/terminal/browser/sessionsTerminalContribution.ts index 59062c64b91cb1..652948d3e44d78 100644 --- a/src/vs/sessions/contrib/terminal/browser/sessionsTerminalContribution.ts +++ b/src/vs/sessions/contrib/terminal/browser/sessionsTerminalContribution.ts @@ -16,7 +16,7 @@ import { IFileService } from '../../../../platform/files/common/files.js'; import { ILogService } from '../../../../platform/log/common/log.js'; import { IWorkbenchContribution, getWorkbenchContribution, registerWorkbenchContribution2, WorkbenchPhase } from '../../../../workbench/common/contributions.js'; import { IAgentHostTerminalService } from '../../../../workbench/contrib/terminal/browser/agentHostTerminalService.js'; -import { ITerminalInstance, ITerminalService } from '../../../../workbench/contrib/terminal/browser/terminal.js'; +import { ITerminalGroupService, ITerminalInstance, ITerminalService } from '../../../../workbench/contrib/terminal/browser/terminal.js'; import { TerminalCapability } from '../../../../platform/terminal/common/capabilities/capabilities.js'; import { IPathService } from '../../../../workbench/services/path/common/pathService.js'; import { isAgentHostProvider, LOCAL_AGENT_HOST_PROVIDER_ID } from '../../../common/agentHostSessionsProvider.js'; @@ -104,6 +104,7 @@ export class SessionsTerminalContribution extends Disposable implements IWorkben @ISessionsService private readonly _sessionsService: ISessionsService, @ISessionsProvidersService private readonly _sessionsProvidersService: ISessionsProvidersService, @ITerminalService private readonly _terminalService: ITerminalService, + @ITerminalGroupService private readonly _terminalGroupService: ITerminalGroupService, @IAgentHostTerminalService private readonly _agentHostTerminalService: IAgentHostTerminalService, @ILogService private readonly _logService: ILogService, @IPathService private readonly _pathService: IPathService, @@ -215,6 +216,9 @@ export class SessionsTerminalContribution extends Disposable implements IWorkben if (instance.shellLaunchConfig.hideFromUser) { return; } + if (this._activeSessionId && !instance.shellLaunchConfig.attachPersistentProcess) { + this._trackTerminalsForSession(this._activeSessionId, [instance]); + } if (instance.shellLaunchConfig.attachPersistentProcess && this._activeKey) { instance.getInitialCwd().then(cwd => { if (cwd.toLowerCase() !== this._activeKey) { @@ -653,6 +657,34 @@ export class SessionsTerminalContribution extends Disposable implements IWorkben } } + for (const instance of toHide) { + const group = this._terminalGroupService.getGroupForInstance(instance); + const index = group ? group.terminalInstances.indexOf(instance) : -1; + instance.shellLaunchConfig.parentTerminalId = index > 0 ? group!.terminalInstances[index - 1].instanceId : undefined; + } + + // Sort toShow so parent terminals are restored before child/split terminals + const idToInstance = new Map(); + for (const instance of toShow) { + idToInstance.set(instance.instanceId, instance); + } + const getDepth = (instance: ITerminalInstance): number => { + let depth = 0; + let currentParentId = instance.shellLaunchConfig.parentTerminalId; + const visited = new Set([instance.instanceId]); + while (currentParentId !== undefined) { + depth++; + const parent = idToInstance.get(currentParentId); + if (!parent || visited.has(currentParentId)) { + break; + } + visited.add(currentParentId); + currentParentId = parent.shellLaunchConfig.parentTerminalId; + } + return depth; + }; + toShow.sort((a, b) => getDepth(a) - getDepth(b)); + for (const instance of toShow) { const availableInstance = this._getAvailableTerminal(instance, 'show background terminal'); if (availableInstance) { diff --git a/src/vs/sessions/contrib/terminal/test/browser/sessionsTerminalContribution.test.ts b/src/vs/sessions/contrib/terminal/test/browser/sessionsTerminalContribution.test.ts index a2ca55ad79d697..e330a62c1d506a 100644 --- a/src/vs/sessions/contrib/terminal/test/browser/sessionsTerminalContribution.test.ts +++ b/src/vs/sessions/contrib/terminal/test/browser/sessionsTerminalContribution.test.ts @@ -16,7 +16,7 @@ import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../../base/tes import { mock } from '../../../../../base/test/common/mock.js'; import { TestInstantiationService } from '../../../../../platform/instantiation/test/common/instantiationServiceMock.js'; import { NullLogService, ILogService } from '../../../../../platform/log/common/log.js'; -import { ITerminalInstance, ITerminalService } from '../../../../../workbench/contrib/terminal/browser/terminal.js'; +import { ITerminalGroup, ITerminalGroupService, ITerminalInstance, ITerminalService } from '../../../../../workbench/contrib/terminal/browser/terminal.js'; import { ITerminalCapabilityStore, ICommandDetectionCapability, TerminalCapability } from '../../../../../platform/terminal/common/capabilities/capabilities.js'; import { toAgentHostUri } from '../../../../../platform/agentHost/common/agentHostUri.js'; import { AgentSessionProviders } from '../../../../../workbench/contrib/chat/browser/agentSessions/agentSessions.js'; @@ -262,6 +262,7 @@ suite('SessionsTerminalContribution', () => { let disposedInstances: ITerminalInstance[]; let nextInstanceId: number; let terminalInstances: Map; + let terminalGroups: { terminalInstances: ITerminalInstance[] }[]; let backgroundedInstances: Set; let moveToBackgroundCalls: number[]; let showBackgroundCalls: number[]; @@ -286,6 +287,7 @@ suite('SessionsTerminalContribution', () => { disposedInstances = []; nextInstanceId = 1; terminalInstances = new Map(); + terminalGroups = []; backgroundedInstances = new Set(); moveToBackgroundCalls = []; showBackgroundCalls = []; @@ -339,6 +341,7 @@ suite('SessionsTerminalContribution', () => { const instance = makeTerminalInstance(id, cwdStr); createdTerminals.push({ cwd: opts?.config?.cwd }); terminalInstances.set(id, instance); + terminalGroups.push({ terminalInstances: [instance] }); if (disposeOnCreatePaths.has(cwdStr)) { instance._testSetDisposed(true); terminalInstances.delete(id); @@ -364,6 +367,19 @@ suite('SessionsTerminalContribution', () => { (instance as TestTerminalInstance)._testSetDisposed(true); terminalInstances.delete(instance.instanceId); backgroundedInstances.delete(instance.instanceId); + const group = terminalGroups.find(g => g.terminalInstances.some(i => i.instanceId === instance.instanceId)); + if (group) { + const idx = group.terminalInstances.findIndex(i => i.instanceId === instance.instanceId); + if (idx !== -1) { + group.terminalInstances.splice(idx, 1); + } + if (group.terminalInstances.length === 0) { + const gIdx = terminalGroups.indexOf(group); + if (gIdx !== -1) { + terminalGroups.splice(gIdx, 1); + } + } + } if (activeInstanceId === instance.instanceId) { activeInstanceId = undefined; } @@ -371,10 +387,39 @@ suite('SessionsTerminalContribution', () => { override moveToBackground(instance: ITerminalInstance): void { backgroundedInstances.add(instance.instanceId); moveToBackgroundCalls.push(instance.instanceId); + const group = terminalGroups.find(g => g.terminalInstances.some(i => i.instanceId === instance.instanceId)); + if (group) { + const idx = group.terminalInstances.findIndex(i => i.instanceId === instance.instanceId); + if (idx !== -1) { + group.terminalInstances.splice(idx, 1); + } + if (group.terminalInstances.length === 0) { + const gIdx = terminalGroups.indexOf(group); + if (gIdx !== -1) { + terminalGroups.splice(gIdx, 1); + } + } + } } override async showBackgroundTerminal(instance: ITerminalInstance): Promise { backgroundedInstances.delete(instance.instanceId); showBackgroundCalls.push(instance.instanceId); + const parentTerminalId = instance.shellLaunchConfig?.parentTerminalId; + const parentTerminal = parentTerminalId !== undefined ? terminalInstances.get(parentTerminalId) : undefined; + const parentGroup = parentTerminal && !parentTerminal.isDisposed + ? terminalGroups.find(g => g.terminalInstances.some(i => i.instanceId === parentTerminal.instanceId)) + : undefined; + if (parentGroup) { + parentGroup.terminalInstances.push(instance); + } else { + terminalGroups.push({ terminalInstances: [instance] }); + } + } + }); + + instantiationService.stub(ITerminalGroupService, new class extends mock() { + override getGroupForInstance(instance: ITerminalInstance): ITerminalGroup | undefined { + return terminalGroups.find(g => g.terminalInstances.some(i => i.instanceId === instance.instanceId)) as unknown as ITerminalGroup | undefined; } }); @@ -398,6 +443,7 @@ suite('SessionsTerminalContribution', () => { agentHostTerminalAddresses.push(address); createdTerminals.push({ cwd }); terminalInstances.set(instance.instanceId, instance); + terminalGroups.push({ terminalInstances: [instance] }); return instance; } }); @@ -2094,6 +2140,88 @@ suite('SessionsTerminalContribution', () => { // rather than left in the background assert.ok(showBackgroundCalls.includes(restoredTerminal.instanceId), 'untracked restored terminal at matching cwd should be shown'); }); + + test('restores side-by-side split terminal layout into one group when switching back to session (#335252)', async () => { + const cwd1 = URI.file('/worktree1'); + const cwd2 = URI.file('/worktree2'); + const session1 = makeAgentSession({ sessionId: 'test:session-1', worktree: cwd1, providerType: AgentSessionProviders.Background }); + const session2 = makeAgentSession({ sessionId: 'test:session-2', worktree: cwd2, providerType: AgentSessionProviders.Background }); + + // Activate Session 1 — terminal 1 is created for session 1 + activeSessionObs.set(session1, undefined); + await tick(); + assert.strictEqual(createdTerminals.length, 1); + const t1 = terminalInstances.get(1)!; + + // Create a second terminal in Session 1 and split it into the same group as t1 + const t2 = makeTerminalInstance(nextInstanceId++, cwd1.fsPath); + terminalInstances.set(t2.instanceId, t2); + const group = terminalGroups.find(g => g.terminalInstances.some(i => i.instanceId === t1.instanceId))!; + assert.ok(group, 'group for t1 should exist'); + group.terminalInstances.push(t2); + onDidCreateInstance.fire(t2); + await tick(); + + // Verify initially both terminals are in the same group + assert.deepStrictEqual(group.terminalInstances.map(i => i.instanceId), [t1.instanceId, t2.instanceId]); + + // Switch to Session 2 — t1 and t2 should be moved to background + activeSessionObs.set(session2, undefined); + await tick(); + + assert.ok(backgroundedInstances.has(t1.instanceId), 't1 should be backgrounded'); + assert.ok(backgroundedInstances.has(t2.instanceId), 't2 should be backgrounded'); + + // Switch back to Session 1 — t1 and t2 should be restored into the same group + activeSessionObs.set(session1, undefined); + await tick(); + + assert.ok(!backgroundedInstances.has(t1.instanceId), 't1 should be in foreground'); + assert.ok(!backgroundedInstances.has(t2.instanceId), 't2 should be in foreground'); + + const restoredGroup = terminalGroups.find(g => g.terminalInstances.some(i => i.instanceId === t1.instanceId)); + assert.ok(restoredGroup, 'restored group should exist'); + assert.deepStrictEqual(restoredGroup.terminalInstances.map(i => i.instanceId), [t1.instanceId, t2.instanceId], 'both terminals should be restored into the same split group'); + }); + + test('restoration gracefully falls back to standalone group when parent terminal is disposed (#335252)', async () => { + const cwd1 = URI.file('/worktree1'); + const cwd2 = URI.file('/worktree2'); + const session1 = makeAgentSession({ sessionId: 'test:session-1', worktree: cwd1, providerType: AgentSessionProviders.Background }); + const session2 = makeAgentSession({ sessionId: 'test:session-2', worktree: cwd2, providerType: AgentSessionProviders.Background }); + + // Activate Session 1 + activeSessionObs.set(session1, undefined); + await tick(); + const t1 = terminalInstances.get(1)!; + + // Create a second terminal in Session 1 and split into t1's group + const t2 = makeTerminalInstance(nextInstanceId++, cwd1.fsPath); + terminalInstances.set(t2.instanceId, t2); + const group = terminalGroups.find(g => g.terminalInstances.some(i => i.instanceId === t1.instanceId))!; + group.terminalInstances.push(t2); + onDidCreateInstance.fire(t2); + await tick(); + + // Switch away to Session 2 + activeSessionObs.set(session2, undefined); + await tick(); + + // Dispose parent terminal t1 while in background + (t1 as TestTerminalInstance)._testSetDisposed(true); + terminalInstances.delete(t1.instanceId); + backgroundedInstances.delete(t1.instanceId); + onDidDisposeInstance.fire(t1); + + // Switch back to Session 1 — t2 should restore gracefully into its own standalone group + activeSessionObs.set(session1, undefined); + await tick(); + + assert.ok(!backgroundedInstances.has(t2.instanceId), 't2 should be in foreground'); + const t2Group = terminalGroups.find(g => g.terminalInstances.some(i => i.instanceId === t2.instanceId)); + assert.ok(t2Group, 't2 should be restored in a group'); + assert.deepStrictEqual(t2Group.terminalInstances.map(i => i.instanceId), [t2.instanceId], 't2 should be in a standalone group'); + }); }); function tick(): Promise { diff --git a/src/vs/workbench/contrib/terminal/browser/terminal.ts b/src/vs/workbench/contrib/terminal/browser/terminal.ts index 5ecd0ab420b7a5..6e29fea815ceb3 100644 --- a/src/vs/workbench/contrib/terminal/browser/terminal.ts +++ b/src/vs/workbench/contrib/terminal/browser/terminal.ts @@ -415,7 +415,7 @@ export interface ITerminalGroup { resizePanes(relativeSizes: number[]): void; setActiveInstanceByIndex(index: number, force?: boolean): void; attachToElement(element: HTMLElement): void; - addInstance(instance: ITerminalInstance): void; + addInstance(instance: ITerminalInstance, parentTerminalId?: number): void; removeInstance(instance: ITerminalInstance): void; moveInstance(instances: SingleOrMany, index: number, position: 'before' | 'after'): void; setVisible(visible: boolean): void; diff --git a/src/vs/workbench/contrib/terminal/browser/terminalService.ts b/src/vs/workbench/contrib/terminal/browser/terminalService.ts index 83ac3261b33ffa..32eb3502826957 100644 --- a/src/vs/workbench/contrib/terminal/browser/terminalService.ts +++ b/src/vs/workbench/contrib/terminal/browser/terminalService.ts @@ -1337,7 +1337,14 @@ export class TerminalService extends Disposable implements ITerminalService { this._backgroundedTerminalInstances.splice(index, 1); this._backgroundedTerminalDisposables.deleteAndDispose(instance.instanceId); if (instance.target === TerminalLocation.Panel) { - this._terminalGroupService.createGroup(instance); + const parentTerminalId = instance.shellLaunchConfig.parentTerminalId; + const parentTerminal = parentTerminalId ? this.getInstanceFromId(parentTerminalId) : undefined; + const parentGroup = parentTerminal ? this._terminalGroupService.getGroupForInstance(parentTerminal) : undefined; + if (parentGroup) { + parentGroup.addInstance(instance, parentTerminalId); + } else { + this._terminalGroupService.createGroup(instance); + } // Make active automatically if it's the first instance if (this.instances.length === 1 && !suppressSetActive) { From 88a225af23299e70bf0107392db212d6b35b1f7c Mon Sep 17 00:00:00 2001 From: kavyansh18 Date: Fri, 11 Sep 2026 15:35:21 +0530 Subject: [PATCH 2/4] Address terminal restoration review feedback --- .../browser/sessionsTerminalContribution.ts | 3 - .../test/browser/terminalService.test.ts | 131 +++++++++++++++++- 2 files changed, 129 insertions(+), 5 deletions(-) diff --git a/src/vs/sessions/contrib/terminal/browser/sessionsTerminalContribution.ts b/src/vs/sessions/contrib/terminal/browser/sessionsTerminalContribution.ts index 652948d3e44d78..fa95ad9ff3b9d6 100644 --- a/src/vs/sessions/contrib/terminal/browser/sessionsTerminalContribution.ts +++ b/src/vs/sessions/contrib/terminal/browser/sessionsTerminalContribution.ts @@ -216,9 +216,6 @@ export class SessionsTerminalContribution extends Disposable implements IWorkben if (instance.shellLaunchConfig.hideFromUser) { return; } - if (this._activeSessionId && !instance.shellLaunchConfig.attachPersistentProcess) { - this._trackTerminalsForSession(this._activeSessionId, [instance]); - } if (instance.shellLaunchConfig.attachPersistentProcess && this._activeKey) { instance.getInitialCwd().then(cwd => { if (cwd.toLowerCase() !== this._activeKey) { diff --git a/src/vs/workbench/contrib/terminal/test/browser/terminalService.test.ts b/src/vs/workbench/contrib/terminal/test/browser/terminalService.test.ts index c57270652eef4f..0fb00e7b54988d 100644 --- a/src/vs/workbench/contrib/terminal/test/browser/terminalService.test.ts +++ b/src/vs/workbench/contrib/terminal/test/browser/terminalService.test.ts @@ -11,11 +11,11 @@ import { TestConfigurationService } from '../../../../../platform/configuration/ import { IDialogService } from '../../../../../platform/dialogs/common/dialogs.js'; import { TestDialogService } from '../../../../../platform/dialogs/test/common/testDialogService.js'; import { TerminalLocation, TitleEventSource, type ITerminalBackend, type TerminalIcon } from '../../../../../platform/terminal/common/terminal.js'; -import { ITerminalInstance, ITerminalInstanceService, ITerminalService } from '../../browser/terminal.js'; +import { ITerminalGroup, ITerminalGroupService, ITerminalInstance, ITerminalInstanceService, ITerminalService } from '../../browser/terminal.js'; import { TerminalService } from '../../browser/terminalService.js'; import { TERMINAL_CONFIG_SECTION } from '../../common/terminal.js'; import { IRemoteAgentService } from '../../../../services/remote/common/remoteAgentService.js'; -import { workbenchInstantiationService } from '../../../../test/browser/workbenchTestServices.js'; +import { TestTerminalGroupService, workbenchInstantiationService } from '../../../../test/browser/workbenchTestServices.js'; import type { IConfigurationChangeEvent } from '../../../../../platform/configuration/common/configuration.js'; suite('Workbench - TerminalService', () => { @@ -80,6 +80,133 @@ suite('Workbench - TerminalService', () => { strictEqual(disposalEmitters[i].hasListeners(), false); } }); + + test('should rejoin parent group when split terminal is restored from background', async () => { + const groupService = instantiationService.get(ITerminalGroupService) as TestTerminalGroupService; + const parentDisposalEmitter = store.add(new Emitter()); + const splitDisposalEmitter = store.add(new Emitter()); + + const parentInstance = { + instanceId: 1, + target: TerminalLocation.Panel, + shellLaunchConfig: {}, + onDisposed: parentDisposalEmitter.event, + detachFromElement: () => { } + } satisfies Partial as unknown as ITerminalInstance; + + const splitInstance = { + instanceId: 2, + target: TerminalLocation.Panel, + shellLaunchConfig: { parentTerminalId: 1 }, + onDisposed: splitDisposalEmitter.event, + detachFromElement: () => { } + } satisfies Partial as unknown as ITerminalInstance; + + const addedToParentCalls: { inst: ITerminalInstance; parentId?: number }[] = []; + const createdGroups: ITerminalGroup[] = []; + + const parentGroup = { + terminalInstances: [parentInstance, splitInstance], + removeInstance: (inst: ITerminalInstance) => { + const idx = parentGroup.terminalInstances.indexOf(inst); + if (idx !== -1) { + parentGroup.terminalInstances.splice(idx, 1); + } + }, + addInstance: (inst: ITerminalInstance, parentId?: number) => { + addedToParentCalls.push({ inst, parentId }); + parentGroup.terminalInstances.push(inst); + } + } satisfies Partial as unknown as ITerminalGroup; + + const groups: ITerminalGroup[] = [parentGroup]; + groupService.groups = groups; + Object.defineProperty(groupService, 'instances', { + get: () => groups.flatMap(g => g.terminalInstances), + configurable: true + }); + groupService.getGroupForInstance = (inst: ITerminalInstance) => groups.find(g => g.terminalInstances.includes(inst)); + groupService.createGroup = (inst?: unknown) => { + const group = { + terminalInstances: inst ? [inst as ITerminalInstance] : [] + } satisfies Partial as unknown as ITerminalGroup; + createdGroups.push(group); + groups.push(group); + return group; + }; + groupService.setActiveInstance = () => { }; + groupService.setActiveInstanceByIndex = () => { }; + + // Move split terminal to background + terminalService.moveToBackground(splitInstance); + strictEqual(parentGroup.terminalInstances.length, 1); + strictEqual(parentGroup.terminalInstances[0], parentInstance); + + // Restore split terminal using real showBackgroundTerminal + await terminalService.showBackgroundTerminal(splitInstance); + + // Verify it rejoined parent group and did not create a new group + strictEqual(addedToParentCalls.length, 1); + strictEqual(addedToParentCalls[0].inst, splitInstance); + strictEqual(addedToParentCalls[0].parentId, 1); + strictEqual(createdGroups.length, 0); + strictEqual(parentGroup.terminalInstances.includes(splitInstance), true); + }); + + test('should create standalone group when split terminal parent no longer exists', async () => { + const groupService = instantiationService.get(ITerminalGroupService) as TestTerminalGroupService; + const splitDisposalEmitter = store.add(new Emitter()); + + const splitInstance = { + instanceId: 2, + target: TerminalLocation.Panel, + shellLaunchConfig: { parentTerminalId: 1 }, + onDisposed: splitDisposalEmitter.event, + detachFromElement: () => { } + } satisfies Partial as unknown as ITerminalInstance; + + const createdGroups: ITerminalGroup[] = []; + const groups: ITerminalGroup[] = []; + groupService.groups = groups; + Object.defineProperty(groupService, 'instances', { + get: () => groups.flatMap(g => g.terminalInstances), + configurable: true + }); + groupService.getGroupForInstance = (inst: ITerminalInstance) => groups.find(g => g.terminalInstances.includes(inst)); + groupService.createGroup = (inst?: unknown) => { + const group = { + terminalInstances: inst ? [inst as ITerminalInstance] : [] + } satisfies Partial as unknown as ITerminalGroup; + createdGroups.push(group); + groups.push(group); + return group; + }; + groupService.setActiveInstance = () => { }; + groupService.setActiveInstanceByIndex = () => { }; + + // Place in initial group and move to background + const initialGroup = { + terminalInstances: [splitInstance], + removeInstance: (inst: ITerminalInstance) => { + const idx = initialGroup.terminalInstances.indexOf(inst); + if (idx !== -1) { + initialGroup.terminalInstances.splice(idx, 1); + } + } + } satisfies Partial as unknown as ITerminalGroup; + groups.push(initialGroup); + + terminalService.moveToBackground(splitInstance); + // Parent terminal 1 does not exist in any group + groups.splice(0, groups.length); + + // Restore split terminal using real showBackgroundTerminal + await terminalService.showBackgroundTerminal(splitInstance); + + // Verify it falls back to createGroup because parent terminal does not exist + strictEqual(createdGroups.length, 1); + strictEqual(createdGroups[0].terminalInstances[0], splitInstance); + }); }); suite('safeDisposeTerminal', () => { From 3582f8ac73b476d710531e579ead8a541a267040 Mon Sep 17 00:00:00 2001 From: kavyansh18 Date: Fri, 11 Sep 2026 15:58:02 +0530 Subject: [PATCH 3/4] Preserve mixed terminal split layouts --- .../browser/sessionsTerminalContribution.ts | 27 ++++- .../sessionsTerminalContribution.test.ts | 113 ++++++++++++++++++ 2 files changed, 139 insertions(+), 1 deletion(-) diff --git a/src/vs/sessions/contrib/terminal/browser/sessionsTerminalContribution.ts b/src/vs/sessions/contrib/terminal/browser/sessionsTerminalContribution.ts index fa95ad9ff3b9d6..b893a0937d5137 100644 --- a/src/vs/sessions/contrib/terminal/browser/sessionsTerminalContribution.ts +++ b/src/vs/sessions/contrib/terminal/browser/sessionsTerminalContribution.ts @@ -86,6 +86,7 @@ export class SessionsTerminalContribution extends Disposable implements IWorkben private _activeSessionId: string | undefined; private readonly _sessionTerminals = new Map>(); private readonly _standaloneTerminalIds = new Set(); + private readonly _pendingBeforeTerminalIds = new Set(); /** In-flight terminal work for drafts, retained only until each operation settles. */ private readonly _pendingTerminalOperations = new Map(); private readonly _sessionTerminalGenerations = new Map(); @@ -206,6 +207,7 @@ export class SessionsTerminalContribution extends Disposable implements IWorkben this._register(this._terminalService.onDidDisposeInstance(instance => { this._removeTerminalFromTrackedSessions(instance.instanceId); this._standaloneTerminalIds.delete(instance.instanceId); + this._pendingBeforeTerminalIds.delete(instance.instanceId); })); // Hide restored terminals from a previous window session that don't @@ -657,7 +659,22 @@ export class SessionsTerminalContribution extends Disposable implements IWorkben for (const instance of toHide) { const group = this._terminalGroupService.getGroupForInstance(instance); const index = group ? group.terminalInstances.indexOf(instance) : -1; - instance.shellLaunchConfig.parentTerminalId = index > 0 ? group!.terminalInstances[index - 1].instanceId : undefined; + if (index > 0) { + instance.shellLaunchConfig.parentTerminalId = group!.terminalInstances[index - 1].instanceId; + this._pendingBeforeTerminalIds.delete(instance.instanceId); + } else if (index === 0 && group && group.terminalInstances.length > 1) { + const survivingSibling = group.terminalInstances.find((inst, i) => i > 0 && !toHide.includes(inst)); + if (survivingSibling) { + instance.shellLaunchConfig.parentTerminalId = survivingSibling.instanceId; + this._pendingBeforeTerminalIds.add(instance.instanceId); + } else { + instance.shellLaunchConfig.parentTerminalId = undefined; + this._pendingBeforeTerminalIds.delete(instance.instanceId); + } + } else { + instance.shellLaunchConfig.parentTerminalId = undefined; + this._pendingBeforeTerminalIds.delete(instance.instanceId); + } } // Sort toShow so parent terminals are restored before child/split terminals @@ -686,6 +703,14 @@ export class SessionsTerminalContribution extends Disposable implements IWorkben const availableInstance = this._getAvailableTerminal(instance, 'show background terminal'); if (availableInstance) { await this._terminalService.showBackgroundTerminal(availableInstance, true); + if (this._pendingBeforeTerminalIds.has(availableInstance.instanceId)) { + this._pendingBeforeTerminalIds.delete(availableInstance.instanceId); + const parentId = availableInstance.shellLaunchConfig.parentTerminalId; + const parentTerminal = parentId !== undefined ? this._terminalService.getInstanceFromId(parentId) : undefined; + if (parentTerminal) { + this._terminalGroupService.moveInstance(availableInstance, parentTerminal, 'before'); + } + } } } for (const instance of toHide) { diff --git a/src/vs/sessions/contrib/terminal/test/browser/sessionsTerminalContribution.test.ts b/src/vs/sessions/contrib/terminal/test/browser/sessionsTerminalContribution.test.ts index e330a62c1d506a..aa8c2a7f11851e 100644 --- a/src/vs/sessions/contrib/terminal/test/browser/sessionsTerminalContribution.test.ts +++ b/src/vs/sessions/contrib/terminal/test/browser/sessionsTerminalContribution.test.ts @@ -421,6 +421,27 @@ suite('SessionsTerminalContribution', () => { override getGroupForInstance(instance: ITerminalInstance): ITerminalGroup | undefined { return terminalGroups.find(g => g.terminalInstances.some(i => i.instanceId === instance.instanceId)) as unknown as ITerminalGroup | undefined; } + override moveInstance(source: ITerminalInstance, target: ITerminalInstance, side: 'before' | 'after'): void { + const sourceGroup = terminalGroups.find(g => g.terminalInstances.some(i => i.instanceId === source.instanceId)); + const targetGroup = terminalGroups.find(g => g.terminalInstances.some(i => i.instanceId === target.instanceId)); + if (!sourceGroup || !targetGroup) { + return; + } + if (sourceGroup !== targetGroup) { + const sIdx = sourceGroup.terminalInstances.indexOf(source); + if (sIdx !== -1) { + sourceGroup.terminalInstances.splice(sIdx, 1); + } + targetGroup.terminalInstances.push(source); + } + const sIdx = targetGroup.terminalInstances.indexOf(source); + if (sIdx !== -1) { + targetGroup.terminalInstances.splice(sIdx, 1); + } + const tIdx = targetGroup.terminalInstances.indexOf(target); + const insertIndex = side === 'before' ? tIdx : tIdx + 1; + targetGroup.terminalInstances.splice(insertIndex, 0, source); + } }); instantiationService.stub(IPathService, new TestPathService(HOME_DIR)); @@ -2222,6 +2243,98 @@ suite('SessionsTerminalContribution', () => { assert.ok(t2Group, 't2 should be restored in a group'); assert.deepStrictEqual(t2Group.terminalInstances.map(i => i.instanceId), [t2.instanceId], 't2 should be in a standalone group'); }); + + test('restores mixed session and standalone split group with correct order (#335252)', async () => { + const cwd1 = URI.file('/worktree1'); + const cwd2 = URI.file('/worktree2'); + const standaloneCwd = URI.file('/standalone-cwd'); + const session1 = makeAgentSession({ sessionId: 'test:session-1', worktree: cwd1, providerType: AgentSessionProviders.Background }); + const session2 = makeAgentSession({ sessionId: 'test:session-2', worktree: cwd2, providerType: AgentSessionProviders.Background }); + const draftSession = makeAgentSession({ sessionId: 'test:draft', worktree: standaloneCwd, providerType: AgentSessionProviders.Background }); + + // Create a standalone terminal (via draft replacement rehoming) + const [tStandalone] = await contribution.ensureTerminal(standaloneCwd, false, draftSession); + onDidReplaceNewDraftSession.fire({ from: draftSession, to: session1 }); + + // Activate Session 1 — terminal tSession is created for session 1 + activeSessionObs.set(session1, undefined); + await tick(); + const tSession = terminalInstances.get(tStandalone.instanceId + 1)!; + + // Arrange them in a split group: [tSession, tStandalone] + const standaloneGroupIdx = terminalGroups.findIndex(g => g.terminalInstances.some(i => i.instanceId === tStandalone.instanceId)); + if (standaloneGroupIdx !== -1) { + terminalGroups.splice(standaloneGroupIdx, 1); + } + const sessionGroup = terminalGroups.find(g => g.terminalInstances.some(i => i.instanceId === tSession.instanceId))!; + sessionGroup.terminalInstances = [tSession, tStandalone]; + + assert.deepStrictEqual(sessionGroup.terminalInstances.map(i => i.instanceId), [tSession.instanceId, tStandalone.instanceId]); + + // Switch away to Session 2 — tSession should be moved to background, tStandalone remains foreground + activeSessionObs.set(session2, undefined); + await tick(); + + assert.ok(backgroundedInstances.has(tSession.instanceId), 'tSession should be backgrounded'); + assert.ok(!backgroundedInstances.has(tStandalone.instanceId), 'tStandalone should remain in foreground'); + assert.deepStrictEqual(sessionGroup.terminalInstances.map(i => i.instanceId), [tStandalone.instanceId]); + + // Switch back to Session 1 — tSession should restore and rejoin before tStandalone + activeSessionObs.set(session1, undefined); + await tick(); + + assert.ok(!backgroundedInstances.has(tSession.instanceId), 'tSession should be restored to foreground'); + const restoredGroup = terminalGroups.find(g => g.terminalInstances.some(i => i.instanceId === tSession.instanceId)); + assert.ok(restoredGroup, 'restored group should exist'); + assert.deepStrictEqual(restoredGroup.terminalInstances.map(i => i.instanceId), [tSession.instanceId, tStandalone.instanceId], 'tSession should be restored before tStandalone'); + }); + + test('restores 3-level split terminals in parent-first order even when enumerated in reverse order', async () => { + const cwd1 = URI.file('/worktree1'); + const cwd2 = URI.file('/worktree2'); + const session1 = makeAgentSession({ sessionId: 'test:session-1', worktree: cwd1, providerType: AgentSessionProviders.Background }); + const session2 = makeAgentSession({ sessionId: 'test:session-2', worktree: cwd2, providerType: AgentSessionProviders.Background }); + + // Activate Session 1 — t1 is created + activeSessionObs.set(session1, undefined); + await tick(); + const t1 = terminalInstances.get(1)!; + + // Create t2 and t3 and split into the same group: [t1, t2, t3] + const t2 = makeTerminalInstance(nextInstanceId++, cwd1.fsPath); + const t3 = makeTerminalInstance(nextInstanceId++, cwd1.fsPath); + terminalInstances.set(t2.instanceId, t2); + terminalInstances.set(t3.instanceId, t3); + const group = terminalGroups.find(g => g.terminalInstances.some(i => i.instanceId === t1.instanceId))!; + group.terminalInstances.push(t2, t3); + + // Switch to Session 2 — all 3 terminals are backgrounded + activeSessionObs.set(session2, undefined); + await tick(); + + assert.ok(backgroundedInstances.has(t1.instanceId)); + assert.ok(backgroundedInstances.has(t2.instanceId)); + assert.ok(backgroundedInstances.has(t3.instanceId)); + + // Reorder terminalInstances map in reverse order: [t3, t2, t1] + terminalInstances.delete(t1.instanceId); + terminalInstances.delete(t2.instanceId); + terminalInstances.delete(t3.instanceId); + terminalInstances.set(t3.instanceId, t3); + terminalInstances.set(t2.instanceId, t2); + terminalInstances.set(t1.instanceId, t1); + + showBackgroundCalls.length = 0; + + // Switch back to Session 1 — must restore in [t1, t2, t3] order + activeSessionObs.set(session1, undefined); + await tick(); + + assert.deepStrictEqual(showBackgroundCalls, [t1.instanceId, t2.instanceId, t3.instanceId], 'showBackgroundTerminal must be called parent-first'); + const restoredGroup = terminalGroups.find(g => g.terminalInstances.some(i => i.instanceId === t1.instanceId)); + assert.ok(restoredGroup, 'restored group should exist'); + assert.deepStrictEqual(restoredGroup.terminalInstances.map(i => i.instanceId), [t1.instanceId, t2.instanceId, t3.instanceId], 'all 3 terminals should rejoin the same group in order'); + }); }); function tick(): Promise { From f4c0e1bef76b9ad5725443f7b2766db837e975d1 Mon Sep 17 00:00:00 2001 From: kavyansh18 Date: Fri, 11 Sep 2026 16:16:44 +0530 Subject: [PATCH 4/4] Centralize terminal restoration ordering --- .../browser/sessionsTerminalContribution.ts | 90 ++++++++++--------- .../sessionsTerminalContribution.test.ts | 46 ++++++++++ 2 files changed, 95 insertions(+), 41 deletions(-) diff --git a/src/vs/sessions/contrib/terminal/browser/sessionsTerminalContribution.ts b/src/vs/sessions/contrib/terminal/browser/sessionsTerminalContribution.ts index b893a0937d5137..c4458c3e210794 100644 --- a/src/vs/sessions/contrib/terminal/browser/sessionsTerminalContribution.ts +++ b/src/vs/sessions/contrib/terminal/browser/sessionsTerminalContribution.ts @@ -677,42 +677,7 @@ export class SessionsTerminalContribution extends Disposable implements IWorkben } } - // Sort toShow so parent terminals are restored before child/split terminals - const idToInstance = new Map(); - for (const instance of toShow) { - idToInstance.set(instance.instanceId, instance); - } - const getDepth = (instance: ITerminalInstance): number => { - let depth = 0; - let currentParentId = instance.shellLaunchConfig.parentTerminalId; - const visited = new Set([instance.instanceId]); - while (currentParentId !== undefined) { - depth++; - const parent = idToInstance.get(currentParentId); - if (!parent || visited.has(currentParentId)) { - break; - } - visited.add(currentParentId); - currentParentId = parent.shellLaunchConfig.parentTerminalId; - } - return depth; - }; - toShow.sort((a, b) => getDepth(a) - getDepth(b)); - - for (const instance of toShow) { - const availableInstance = this._getAvailableTerminal(instance, 'show background terminal'); - if (availableInstance) { - await this._terminalService.showBackgroundTerminal(availableInstance, true); - if (this._pendingBeforeTerminalIds.has(availableInstance.instanceId)) { - this._pendingBeforeTerminalIds.delete(availableInstance.instanceId); - const parentId = availableInstance.shellLaunchConfig.parentTerminalId; - const parentTerminal = parentId !== undefined ? this._terminalService.getInstanceFromId(parentId) : undefined; - if (parentTerminal) { - this._terminalGroupService.moveInstance(availableInstance, parentTerminal, 'before'); - } - } - } - } + await this._restoreBackgroundTerminals(toShow); for (const instance of toHide) { const availableInstance = this._getAvailableTerminal(instance, 'move terminal to background'); if (availableInstance) { @@ -874,14 +839,57 @@ export class SessionsTerminalContribution extends Disposable implements IWorkben } } - async showAllTerminals(): Promise { - for (const instance of this._terminalService.instances) { - if (!this._terminalService.foregroundInstances.includes(instance)) { - await this._terminalService.showBackgroundTerminal(instance, true); - this._logService.trace(`[SessionsTerminal] Moved terminal ${instance.instanceId} to foreground`); + /** + * Restores the given background terminals in parent-first order based on + * `parentTerminalId`, handles split relationship restoration, and applies + * `_pendingBeforeTerminalIds` repositioning when restoring before an anchor terminal. + */ + private async _restoreBackgroundTerminals(terminals: ITerminalInstance[]): Promise { + const toShow = [...terminals]; + const idToInstance = new Map(); + for (const instance of toShow) { + idToInstance.set(instance.instanceId, instance); + } + const getDepth = (instance: ITerminalInstance): number => { + let depth = 0; + let currentParentId = instance.shellLaunchConfig.parentTerminalId; + const visited = new Set([instance.instanceId]); + while (currentParentId !== undefined) { + depth++; + const parent = idToInstance.get(currentParentId); + if (!parent || visited.has(currentParentId)) { + break; + } + visited.add(currentParentId); + currentParentId = parent.shellLaunchConfig.parentTerminalId; + } + return depth; + }; + toShow.sort((a, b) => getDepth(a) - getDepth(b)); + + for (const instance of toShow) { + const availableInstance = this._getAvailableTerminal(instance, 'show background terminal'); + if (availableInstance) { + await this._terminalService.showBackgroundTerminal(availableInstance, true); + this._logService.trace(`[SessionsTerminal] Moved terminal ${availableInstance.instanceId} to foreground`); + if (this._pendingBeforeTerminalIds.has(availableInstance.instanceId)) { + this._pendingBeforeTerminalIds.delete(availableInstance.instanceId); + const parentId = availableInstance.shellLaunchConfig.parentTerminalId; + const parentTerminal = parentId !== undefined ? this._terminalService.getInstanceFromId(parentId) : undefined; + if (parentTerminal) { + this._terminalGroupService.moveInstance(availableInstance, parentTerminal, 'before'); + } + } } } } + + async showAllTerminals(): Promise { + const backgroundTerminals = this._terminalService.instances.filter( + instance => !this._terminalService.foregroundInstances.includes(instance) + ); + await this._restoreBackgroundTerminals(backgroundTerminals); + } } registerWorkbenchContribution2(SessionsTerminalContribution.ID, SessionsTerminalContribution, WorkbenchPhase.AfterRestored); diff --git a/src/vs/sessions/contrib/terminal/test/browser/sessionsTerminalContribution.test.ts b/src/vs/sessions/contrib/terminal/test/browser/sessionsTerminalContribution.test.ts index aa8c2a7f11851e..e75f8072b7dc08 100644 --- a/src/vs/sessions/contrib/terminal/test/browser/sessionsTerminalContribution.test.ts +++ b/src/vs/sessions/contrib/terminal/test/browser/sessionsTerminalContribution.test.ts @@ -2335,6 +2335,52 @@ suite('SessionsTerminalContribution', () => { assert.ok(restoredGroup, 'restored group should exist'); assert.deepStrictEqual(restoredGroup.terminalInstances.map(i => i.instanceId), [t1.instanceId, t2.instanceId, t3.instanceId], 'all 3 terminals should rejoin the same group in order'); }); + + test('showAllTerminals restores 3-level split terminals in parent-first order even when enumerated in reverse order', async () => { + const cwd1 = URI.file('/worktree1'); + const cwd2 = URI.file('/worktree2'); + const session1 = makeAgentSession({ sessionId: 'test:session-1', worktree: cwd1, providerType: AgentSessionProviders.Background }); + const session2 = makeAgentSession({ sessionId: 'test:session-2', worktree: cwd2, providerType: AgentSessionProviders.Background }); + + // Activate Session 1 — t1 is created + activeSessionObs.set(session1, undefined); + await tick(); + const t1 = terminalInstances.get(1)!; + + // Create t2 and t3 and split into the same group: [t1, t2, t3] + const t2 = makeTerminalInstance(nextInstanceId++, cwd1.fsPath); + const t3 = makeTerminalInstance(nextInstanceId++, cwd1.fsPath); + terminalInstances.set(t2.instanceId, t2); + terminalInstances.set(t3.instanceId, t3); + const group = terminalGroups.find(g => g.terminalInstances.some(i => i.instanceId === t1.instanceId))!; + group.terminalInstances.push(t2, t3); + + // Switch to Session 2 — all 3 terminals are backgrounded + activeSessionObs.set(session2, undefined); + await tick(); + + assert.ok(backgroundedInstances.has(t1.instanceId)); + assert.ok(backgroundedInstances.has(t2.instanceId)); + assert.ok(backgroundedInstances.has(t3.instanceId)); + + // Reorder terminalInstances map in reverse order: [t3, t2, t1] + terminalInstances.delete(t1.instanceId); + terminalInstances.delete(t2.instanceId); + terminalInstances.delete(t3.instanceId); + terminalInstances.set(t3.instanceId, t3); + terminalInstances.set(t2.instanceId, t2); + terminalInstances.set(t1.instanceId, t1); + + showBackgroundCalls.length = 0; + + // Invoke Show All Terminals + await contribution.showAllTerminals(); + + assert.deepStrictEqual(showBackgroundCalls, [t1.instanceId, t2.instanceId, t3.instanceId], 'showBackgroundTerminal must be called parent-first'); + const restoredGroup = terminalGroups.find(g => g.terminalInstances.some(i => i.instanceId === t1.instanceId)); + assert.ok(restoredGroup, 'restored group should exist'); + assert.deepStrictEqual(restoredGroup.terminalInstances.map(i => i.instanceId), [t1.instanceId, t2.instanceId, t3.instanceId], 'all 3 terminals should rejoin the same group in order'); + }); }); function tick(): Promise {