From f48d290224c936a6aefa06bfb22ca1b9574d112a Mon Sep 17 00:00:00 2001 From: BeniBenj Date: Wed, 4 Mar 2026 23:37:42 +0100 Subject: [PATCH] better sessions terminal tracking --- .../browser/sessionsTerminalContribution.ts | 108 +++++++++++++++++- .../sessionsTerminalContribution.test.ts | 76 +++++++++++- 2 files changed, 176 insertions(+), 8 deletions(-) diff --git a/src/vs/sessions/contrib/terminal/browser/sessionsTerminalContribution.ts b/src/vs/sessions/contrib/terminal/browser/sessionsTerminalContribution.ts index ff375376d00..290fa8b309c 100644 --- a/src/vs/sessions/contrib/terminal/browser/sessionsTerminalContribution.ts +++ b/src/vs/sessions/contrib/terminal/browser/sessionsTerminalContribution.ts @@ -4,6 +4,7 @@ *--------------------------------------------------------------------------------------------*/ import { Codicon } from '../../../../base/common/codicons.js'; +import { isEqualOrParent } from '../../../../base/common/extpath.js'; import { Disposable } from '../../../../base/common/lifecycle.js'; import { autorun } from '../../../../base/common/observable.js'; import { URI } from '../../../../base/common/uri.js'; @@ -14,7 +15,7 @@ import { ILogService } from '../../../../platform/log/common/log.js'; import { IWorkbenchContribution, getWorkbenchContribution, registerWorkbenchContribution2, WorkbenchPhase } from '../../../../workbench/common/contributions.js'; import { IAgentSessionsService } from '../../../../workbench/contrib/chat/browser/agentSessions/agentSessionsService.js'; import { AgentSessionProviders } from '../../../../workbench/contrib/chat/browser/agentSessions/agentSessions.js'; -import { ITerminalService } from '../../../../workbench/contrib/terminal/browser/terminal.js'; +import { ITerminalInstance, ITerminalService } from '../../../../workbench/contrib/terminal/browser/terminal.js'; import { IPathService } from '../../../../workbench/services/path/common/pathService.js'; import { Menus } from '../../../browser/menus.js'; import { IActiveSessionItem, ISessionsManagementService } from '../../sessions/browser/sessionsManagementService.js'; @@ -83,16 +84,17 @@ export class SessionsTerminalContribution extends Disposable implements IWorkben } })); - // When terminals are restored on startup, ensure visibility matches active session + // When terminals are created externally, try to relate them to the active session this._register(this._terminalService.onDidCreateInstance(instance => { if (this._isCreatingTerminal || this._activeKey === undefined) { return; } - // If this instance is not tracked by us, hide it + // If this instance is already tracked by us, nothing to do const activeIds = this._pathToInstanceIds.get(this._activeKey); - if (!activeIds?.has(instance.instanceId)) { - this._terminalService.moveToBackground(instance); + if (activeIds?.has(instance.instanceId)) { + return; } + this._tryAdoptTerminal(instance); })); } @@ -160,6 +162,58 @@ export class SessionsTerminalContribution extends Disposable implements IWorkben ids.add(instanceId); } + /** + * Attempts to associate an externally-created terminal with the active + * session by checking whether its initial cwd falls within the active + * session's worktree or repository. Hides the terminal if it cannot be + * related. + */ + private async _tryAdoptTerminal(instance: ITerminalInstance): Promise { + let cwd: string | undefined; + try { + cwd = await instance.getInitialCwd(); + } catch { + return; + } + + if (instance.isDisposed) { + return; + } + + const activeKey = this._activeKey; + if (!activeKey) { + return; + } + + // Re-check tracking — the terminal may have been adopted while awaiting + const activeIds = this._pathToInstanceIds.get(activeKey); + if (activeIds?.has(instance.instanceId)) { + return; + } + + const session = this._sessionsManagementService.activeSession.get(); + if (cwd && this._isRelatedToSession(cwd, session, activeKey)) { + this._addInstanceToPath(activeKey, instance.instanceId); + this._logService.trace(`[SessionsTerminal] Adopted terminal ${instance.instanceId} with cwd ${cwd}`); + } else { + this._terminalService.moveToBackground(instance); + } + } + + /** + * Returns whether the given cwd falls within the active session's + * worktree, repository, or the current active key (home dir fallback). + */ + private _isRelatedToSession(cwd: string, session: IActiveSessionItem | undefined, activeKey: string): boolean { + if (isEqualOrParent(cwd, activeKey, true)) { + return true; + } + if (session?.providerType === AgentSessionProviders.Background && session.repository) { + return isEqualOrParent(cwd, session.repository.fsPath, true); + } + return false; + } + /** * Hides all foreground terminals that do not belong to the given active key * and shows all background terminals that do belong to it. @@ -199,6 +253,32 @@ export class SessionsTerminalContribution extends Disposable implements IWorkben this._pathToInstanceIds.delete(key); } } + + async dumpTracking(): Promise { + const trackedInstanceIds = new Set(); + + console.log('[SessionsTerminal] === Tracked Terminals ==='); + for (const [key, ids] of this._pathToInstanceIds) { + for (const instanceId of ids) { + trackedInstanceIds.add(instanceId); + const instance = this._terminalService.getInstanceFromId(instanceId); + let cwd = ''; + if (instance) { + try { cwd = await instance.getInitialCwd(); } catch { /* ignored */ } + } + console.log(` ${instanceId} - ${cwd} - ${key}`); + } + } + + console.log('[SessionsTerminal] === Untracked Terminals ==='); + for (const instance of this._terminalService.instances) { + if (!trackedInstanceIds.has(instance.instanceId)) { + let cwd = ''; + try { cwd = await instance.getInitialCwd(); } catch { /* ignored */ } + console.log(` ${instance.instanceId} - ${cwd}`); + } + } + } } registerWorkbenchContribution2(SessionsTerminalContribution.ID, SessionsTerminalContribution, WorkbenchPhase.AfterRestored); @@ -231,3 +311,21 @@ class OpenSessionInTerminalAction extends Action2 { } registerAction2(OpenSessionInTerminalAction); + +class DumpTerminalTrackingAction extends Action2 { + + constructor() { + super({ + id: 'agentSession.dumpTerminalTracking', + title: localize2('dumpTerminalTracking', "Dump Terminal Tracking"), + f1: true, + }); + } + + override async run(): Promise { + const contribution = getWorkbenchContribution(SessionsTerminalContribution.ID); + await contribution.dumpTracking(); + } +} + +registerAction2(DumpTerminalTrackingAction); 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 c2108711f80..7305d4591b5 100644 --- a/src/vs/sessions/contrib/terminal/test/browser/sessionsTerminalContribution.test.ts +++ b/src/vs/sessions/contrib/terminal/test/browser/sessionsTerminalContribution.test.ts @@ -51,6 +51,14 @@ function makeNonAgentSession(opts: { repository?: URI; worktree?: URI; providerT } as IActiveSessionItem; } +function makeTerminalInstance(id: number, cwd: string): ITerminalInstance { + return { + instanceId: id, + isDisposed: false, + getInitialCwd: () => Promise.resolve(cwd), + } as unknown as ITerminalInstance; +} + suite('SessionsTerminalContribution', () => { const store = new DisposableStore(); @@ -102,7 +110,9 @@ suite('SessionsTerminalContribution', () => { } override async createTerminal(opts?: any): Promise { const id = nextInstanceId++; - const instance = { instanceId: id } as ITerminalInstance; + const cwdUri: URI | undefined = opts?.config?.cwd; + const cwdStr = cwdUri?.fsPath ?? ''; + const instance = makeTerminalInstance(id, cwdStr); createdTerminals.push({ cwd: opts?.config?.cwd }); terminalInstances.set(id, instance); onDidCreateInstance.fire(instance); @@ -436,9 +446,10 @@ suite('SessionsTerminalContribution', () => { await tick(); // Simulate a terminal being restored (e.g. on startup) that is not tracked - const restoredInstance = { instanceId: nextInstanceId++ } as ITerminalInstance; + const restoredInstance = makeTerminalInstance(nextInstanceId++, '/some/other/path'); terminalInstances.set(restoredInstance.instanceId, restoredInstance); onDidCreateInstance.fire(restoredInstance); + await tick(); // The restored terminal should be moved to background assert.ok(moveToBackgroundCalls.includes(restoredInstance.instanceId), 'restored terminal should be backgrounded'); @@ -446,9 +457,10 @@ suite('SessionsTerminalContribution', () => { test('does not hide restored terminals before any session is active', async () => { // Simulate a terminal being restored before any session is active - const restoredInstance = { instanceId: nextInstanceId++ } as ITerminalInstance; + const restoredInstance = makeTerminalInstance(nextInstanceId++, '/some/path'); terminalInstances.set(restoredInstance.instanceId, restoredInstance); onDidCreateInstance.fire(restoredInstance); + await tick(); assert.strictEqual(moveToBackgroundCalls.length, 0, 'should not background before any session is active'); }); @@ -467,6 +479,64 @@ suite('SessionsTerminalContribution', () => { assert.strictEqual(createdTerminals.length, 1, 'should not create a new terminal'); assert.ok(showBackgroundCalls.includes(instanceId), 'should show the backgrounded terminal'); }); + + // --- Terminal adoption --- + + test('adopts externally-created terminal whose cwd matches the active worktree', async () => { + const worktree = URI.file('/worktree'); + activeSessionObs.set(makeAgentSession({ worktree, providerType: AgentSessionProviders.Background }), undefined); + await tick(); + + const externalInstance = makeTerminalInstance(nextInstanceId++, worktree.fsPath); + terminalInstances.set(externalInstance.instanceId, externalInstance); + onDidCreateInstance.fire(externalInstance); + await tick(); + + assert.ok(!moveToBackgroundCalls.includes(externalInstance.instanceId), 'should not be hidden'); + // Verify it was adopted — ensureTerminal should reuse it + await contribution.ensureTerminal(worktree, false); + assert.strictEqual(createdTerminals.length, 1, 'should reuse adopted terminal, not create a second'); + }); + + test('adopts externally-created terminal whose cwd is a subdirectory of the active worktree', async () => { + const worktree = URI.file('/worktree'); + activeSessionObs.set(makeAgentSession({ worktree, providerType: AgentSessionProviders.Background }), undefined); + await tick(); + + const externalInstance = makeTerminalInstance(nextInstanceId++, URI.file('/worktree/subdir').fsPath); + terminalInstances.set(externalInstance.instanceId, externalInstance); + onDidCreateInstance.fire(externalInstance); + await tick(); + + assert.ok(!moveToBackgroundCalls.includes(externalInstance.instanceId), 'subdirectory terminal should not be hidden'); + }); + + test('adopts externally-created terminal whose cwd matches the session repository', async () => { + const worktree = URI.file('/worktree'); + const repo = URI.file('/repo'); + activeSessionObs.set(makeAgentSession({ worktree, repository: repo, providerType: AgentSessionProviders.Background }), undefined); + await tick(); + + const externalInstance = makeTerminalInstance(nextInstanceId++, repo.fsPath); + terminalInstances.set(externalInstance.instanceId, externalInstance); + onDidCreateInstance.fire(externalInstance); + await tick(); + + assert.ok(!moveToBackgroundCalls.includes(externalInstance.instanceId), 'terminal at repository path should not be hidden'); + }); + + test('hides externally-created terminal whose cwd does not match the active session', async () => { + const worktree = URI.file('/worktree'); + activeSessionObs.set(makeAgentSession({ worktree, providerType: AgentSessionProviders.Background }), undefined); + await tick(); + + const externalInstance = makeTerminalInstance(nextInstanceId++, '/unrelated/path'); + terminalInstances.set(externalInstance.instanceId, externalInstance); + onDidCreateInstance.fire(externalInstance); + await tick(); + + assert.ok(moveToBackgroundCalls.includes(externalInstance.instanceId), 'unrelated terminal should be hidden'); + }); }); function tick(): Promise {