Skip to content

Commit b292560

Browse files
roblourensroblourens
authored
Use view state to restore InteractiveSessionViewPane session by id (microsoft#180732)
1 parent 14a24d5 commit b292560

File tree

7 files changed

+34
-67
lines changed

7 files changed

+34
-67
lines changed

‎src/vs/workbench/contrib/interactiveSession/browser/interactiveSessionEditor.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -89,20 +89,20 @@ export class InteractiveSessionEditor extends EditorPane {
8989
throw new Error('InteractiveSessionEditor lifecycle issue: no editor widget');
9090
}
9191

92-
this.updateModel(editorModel.model, options);
92+
this.updateModel(editorModel.model);
9393
}
9494

95-
private updateModel(model: IInteractiveSessionModel, options: IInteractiveSessionEditorOptions): void {
95+
private updateModel(model: IInteractiveSessionModel): void {
9696
this._memento = new Memento('interactive-session-editor-' + model.sessionId, this.storageService);
9797
this._viewState = this._memento.getMemento(StorageScope.WORKSPACE, StorageTarget.USER) as IViewState;
9898
this.widget.setModel(model, { ...this._viewState });
9999
const listener = model.onDidDispose(() => {
100100
// TODO go back to swapping out the EditorInput when the session is restarted instead of this listener
101101
listener.dispose();
102-
const newModel = this.interactiveSessionService.startSession(options.providerId, false, CancellationToken.None);
102+
const newModel = this.interactiveSessionService.startSession(model.providerId, CancellationToken.None);
103103
if (newModel) {
104104
(this.input as InteractiveSessionEditorInput).sessionId = newModel.sessionId;
105-
this.updateModel(newModel, options);
105+
this.updateModel(newModel);
106106
}
107107
});
108108
}

‎src/vs/workbench/contrib/interactiveSession/browser/interactiveSessionEditorInput.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ export class InteractiveSessionEditorInput extends EditorInput {
6969
override async resolve(): Promise<InteractiveSessionEditorModel | null> {
7070
const model = typeof this.sessionId === 'string' ?
7171
this.interactiveSessionService.retrieveSession(this.sessionId) :
72-
this.interactiveSessionService.startSession(this.options.providerId, false, CancellationToken.None);
72+
this.interactiveSessionService.startSession(this.options.providerId, CancellationToken.None);
7373

7474
if (!model) {
7575
return null;

‎src/vs/workbench/contrib/interactiveSession/browser/interactiveSessionViewPane.ts

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,12 +20,17 @@ import { IViewPaneOptions, ViewPane } from 'vs/workbench/browser/parts/views/vie
2020
import { Memento } from 'vs/workbench/common/memento';
2121
import { IViewDescriptorService } from 'vs/workbench/common/views';
2222
import { IViewState, InteractiveSessionWidget } from 'vs/workbench/contrib/interactiveSession/browser/interactiveSessionWidget';
23+
import { IInteractiveSessionModel } from 'vs/workbench/contrib/interactiveSession/common/interactiveSessionModel';
2324
import { IInteractiveSessionService } from 'vs/workbench/contrib/interactiveSession/common/interactiveSessionService';
2425

2526
export interface IInteractiveSessionViewOptions {
2627
readonly providerId: string;
2728
}
2829

30+
interface IViewPaneState extends IViewState {
31+
sessionId?: string;
32+
}
33+
2934
export const INTERACTIVE_SIDEBAR_PANEL_ID = 'workbench.panel.interactiveSessionSidebar';
3035
export class InteractiveSessionViewPane extends ViewPane {
3136
static ID = 'workbench.panel.interactiveSession.view';
@@ -35,7 +40,7 @@ export class InteractiveSessionViewPane extends ViewPane {
3540

3641
private modelDisposables = this._register(new DisposableStore());
3742
private memento: Memento;
38-
private viewState: IViewState;
43+
private viewState: IViewPaneState;
3944

4045
constructor(
4146
private readonly interactiveSessionViewOptions: IInteractiveSessionViewOptions,
@@ -56,18 +61,19 @@ export class InteractiveSessionViewPane extends ViewPane {
5661

5762
// View state for the ViewPane is currently global per-provider basically, but some other strictly per-model state will require a separate memento.
5863
this.memento = new Memento('interactive-session-view-' + this.interactiveSessionViewOptions.providerId, this.storageService);
59-
this.viewState = this.memento.getMemento(StorageScope.WORKSPACE, StorageTarget.USER) as IViewState;
64+
this.viewState = this.memento.getMemento(StorageScope.WORKSPACE, StorageTarget.USER) as IViewPaneState;
6065
}
6166

62-
private updateModel(initial = false): void {
67+
private updateModel(model?: IInteractiveSessionModel | undefined): void {
6368
this.modelDisposables.clear();
6469

65-
const model = this.interactiveSessionService.startSession(this.interactiveSessionViewOptions.providerId, initial, CancellationToken.None);
70+
model = model ?? this.interactiveSessionService.startSession(this.interactiveSessionViewOptions.providerId, CancellationToken.None);
6671
if (!model) {
6772
throw new Error('Could not start interactive session');
6873
}
6974

7075
this._widget.setModel(model, { ...this.viewState });
76+
this.viewState.sessionId = model.sessionId;
7177
this.modelDisposables.add(model.onDidDispose(() => {
7278
this.updateModel();
7379
}));
@@ -84,7 +90,8 @@ export class InteractiveSessionViewPane extends ViewPane {
8490
}));
8591
this._widget.render(parent);
8692

87-
this.updateModel(true);
93+
const initialModel = this.viewState.sessionId ? this.interactiveSessionService.retrieveSession(this.viewState.sessionId) : undefined;
94+
this.updateModel(initialModel);
8895
}
8996

9097
acceptInput(query?: string): void {

‎src/vs/workbench/contrib/interactiveSession/browser/interactiveSessionWidget.ts

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -436,14 +436,6 @@ export class InteractiveSessionWidget extends Disposable implements IInteractive
436436
this.inputPart.saveState();
437437
return { inputValue: this.inputPart.inputEditor.getValue() };
438438
}
439-
440-
public override dispose(): void {
441-
super.dispose();
442-
443-
if (this.viewModel) {
444-
this.interactiveSessionService.releaseSession(this.viewModel.sessionId);
445-
}
446-
}
447439
}
448440

449441
export class InteractiveSessionWidgetService implements IInteractiveSessionWidgetService {

‎src/vs/workbench/contrib/interactiveSession/common/interactiveSessionService.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,7 @@ export interface IInteractiveSessionService {
163163
_serviceBrand: undefined;
164164
registerProvider(provider: IInteractiveProvider): IDisposable;
165165
getProviderInfos(): IInteractiveProviderInfo[];
166-
startSession(providerId: string, allowRestoringSession: boolean, token: CancellationToken): InteractiveSessionModel | undefined;
166+
startSession(providerId: string, token: CancellationToken): InteractiveSessionModel | undefined;
167167
retrieveSession(sessionId: string): IInteractiveSessionModel | undefined;
168168

169169
/**
@@ -176,7 +176,6 @@ export interface IInteractiveSessionService {
176176
addInteractiveRequest(context: any): void;
177177
addCompleteRequest(sessionId: string, message: string, response: IInteractiveSessionCompleteResponse): void;
178178
sendInteractiveRequestToProvider(sessionId: string, message: IInteractiveSessionDynamicRequest): void;
179-
releaseSession(sessionId: string): void;
180179

181180
onDidPerformUserAction: Event<IInteractiveSessionUserActionEvent>;
182181
notifyUserAction(event: IInteractiveSessionUserActionEvent): void;

‎src/vs/workbench/contrib/interactiveSession/common/interactiveSessionServiceImpl.ts

Lines changed: 5 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,6 @@ export class InteractiveSessionService extends Disposable implements IInteractiv
109109

110110
private readonly _providers = new Map<string, IInteractiveProvider>();
111111
private readonly _sessionModels = new Map<string, InteractiveSessionModel>();
112-
private readonly _releasedSessions = new Set<string>();
113112
private readonly _pendingRequests = new Map<string, CancelablePromise<void>>();
114113
private readonly _unprocessedPersistedSessions: ISerializableInteractiveSessionsData;
115114
private readonly _hasProvider: IContextKey<boolean>;
@@ -203,19 +202,9 @@ export class InteractiveSessionService extends Disposable implements IInteractiv
203202
}
204203
}
205204

206-
startSession(providerId: string, allowRestoringSession: boolean, token: CancellationToken): InteractiveSessionModel {
207-
this.trace('startSession', `providerId=${providerId}, allowRestoringSession=${allowRestoringSession}`);
208-
209-
const restored = allowRestoringSession ? this.getNextRestoredSession(providerId) : undefined;
210-
if (restored instanceof InteractiveSessionModel) {
211-
this.trace('startSession', `Restored live session with id ${restored.sessionId}`);
212-
return restored;
213-
}
214-
215-
const someSessionHistory = restored;
216-
this.trace('startSession', `Has history: ${!!someSessionHistory}. Including provider state: ${!!someSessionHistory?.providerState}`);
217-
218-
return this._startSession(providerId, someSessionHistory, token);
205+
startSession(providerId: string, token: CancellationToken): InteractiveSessionModel {
206+
this.trace('startSession', `providerId=${providerId}`);
207+
return this._startSession(providerId, undefined, token);
219208
}
220209

221210
private _startSession(providerId: string, someSessionHistory: ISerializableInteractiveSessionData | undefined, token: CancellationToken): InteractiveSessionModel {
@@ -227,7 +216,8 @@ export class InteractiveSessionService extends Disposable implements IInteractiv
227216
model.dispose();
228217
this._sessionModels.delete(model.sessionId);
229218
}
230-
}).catch(() => {
219+
}).catch(err => {
220+
this.trace('startSession', `initializeSession failed: ${err}`);
231221
model.dispose();
232222
this._sessionModels.delete(model.sessionId);
233223
});
@@ -271,27 +261,7 @@ export class InteractiveSessionService extends Disposable implements IInteractiv
271261
return model;
272262
}
273263

274-
private getNextRestoredSession(providerId: string): InteractiveSessionModel | ISerializableInteractiveSessionData | undefined {
275-
const releasedSessionId = Iterable.find(this._releasedSessions.values(), sessionId => this._sessionModels.get(sessionId)?.providerId === providerId);
276-
if (typeof releasedSessionId === 'string') {
277-
this._releasedSessions.delete(releasedSessionId);
278-
return this._sessionModels.get(releasedSessionId);
279-
}
280-
281-
const providerData = this._unprocessedPersistedSessions[providerId] ?? [];
282-
return providerData.shift();
283-
}
284-
285-
releaseSession(sessionId: string): void {
286-
this.trace('releaseSession', `sessionId=${sessionId}`);
287-
this._releasedSessions.add(sessionId);
288-
}
289-
290264
retrieveSession(sessionId: string): InteractiveSessionModel | undefined {
291-
if (this._releasedSessions.has(sessionId)) {
292-
this._releasedSessions.delete(sessionId);
293-
}
294-
295265
const model = this._sessionModels.get(sessionId);
296266
if (model) {
297267
return model;
@@ -514,7 +484,6 @@ export class InteractiveSessionService extends Disposable implements IInteractiv
514484
model.dispose();
515485
this._sessionModels.delete(sessionId);
516486
this._pendingRequests.get(sessionId)?.cancel();
517-
this._releasedSessions.delete(sessionId);
518487
}
519488

520489
registerProvider(provider: IInteractiveProvider): IDisposable {

‎src/vs/workbench/contrib/interactiveSession/test/common/interactiveSessionService.test.ts

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -73,18 +73,18 @@ suite('InteractiveSession', () => {
7373
testDisposables.clear();
7474
});
7575

76-
test('Restores state for the correct provider', async () => {
76+
test('retrieveSession', async () => {
7777
const testService = instantiationService.createInstance(InteractiveSessionService);
7878
const provider1 = new SimpleTestProvider('provider1');
7979
const provider2 = new SimpleTestProvider('provider2');
8080
testService.registerProvider(provider1);
8181
testService.registerProvider(provider2);
8282

83-
let session1 = testService.startSession('provider1', true, CancellationToken.None);
83+
const session1 = testService.startSession('provider1', CancellationToken.None);
8484
await session1.waitForInitialization();
8585
session1!.addRequest('request 1');
8686

87-
let session2 = testService.startSession('provider2', true, CancellationToken.None);
87+
const session2 = testService.startSession('provider2', CancellationToken.None);
8888
await session2.waitForInitialization();
8989
session2!.addRequest('request 2');
9090

@@ -97,10 +97,10 @@ suite('InteractiveSession', () => {
9797
const testService2 = instantiationService.createInstance(InteractiveSessionService);
9898
testService2.registerProvider(provider1);
9999
testService2.registerProvider(provider2);
100-
session1 = testService2.startSession('provider1', true, CancellationToken.None);
101-
await session1.waitForInitialization();
102-
session2 = testService2.startSession('provider2', true, CancellationToken.None);
103-
await session2.waitForInitialization();
100+
const retrieved1 = testService2.retrieveSession(session1.sessionId);
101+
await retrieved1!.waitForInitialization();
102+
const retrieved2 = testService2.retrieveSession(session2.sessionId);
103+
await retrieved2!.waitForInitialization();
104104
assert.deepStrictEqual(provider1.lastInitialState, { state: 'provider1_state' });
105105
assert.deepStrictEqual(provider2.lastInitialState, { state: 'provider2_state' });
106106
});
@@ -127,7 +127,7 @@ suite('InteractiveSession', () => {
127127
const provider1 = getFailProvider('provider1');
128128
testService.registerProvider(provider1);
129129

130-
const session1 = testService.startSession('provider1', true, CancellationToken.None);
130+
const session1 = testService.startSession('provider1', CancellationToken.None);
131131
await assert.rejects(() => session1.waitForInitialization());
132132
});
133133

@@ -179,7 +179,7 @@ suite('InteractiveSession', () => {
179179

180180
testService.registerProvider(provider);
181181

182-
const model = testService.startSession('testProvider', true, CancellationToken.None);
182+
const model = testService.startSession('testProvider', CancellationToken.None);
183183
const commands = await testService.getSlashCommands(model.sessionId, CancellationToken.None);
184184

185185
assert.strictEqual(commands?.length, 1);
@@ -192,7 +192,7 @@ suite('InteractiveSession', () => {
192192
const testService = instantiationService.createInstance(InteractiveSessionService);
193193
testService.registerProvider(new SimpleTestProvider('testProvider'));
194194

195-
const model = testService.startSession('testProvider', true, CancellationToken.None);
195+
const model = testService.startSession('testProvider', CancellationToken.None);
196196
assert.strictEqual(model.getRequests().length, 0);
197197

198198
await testService.sendInteractiveRequestToProvider(model.sessionId, { message: 'test request' });
@@ -203,7 +203,7 @@ suite('InteractiveSession', () => {
203203
const testService = instantiationService.createInstance(InteractiveSessionService);
204204
testService.registerProvider(new SimpleTestProvider('testProvider'));
205205

206-
const model = testService.startSession('testProvider', true, CancellationToken.None);
206+
const model = testService.startSession('testProvider', CancellationToken.None);
207207
assert.strictEqual(model.getRequests().length, 0);
208208

209209
await testService.addCompleteRequest(model.sessionId, 'test request', { message: 'test response' });

0 commit comments

Comments
 (0)