Skip to content

Commit 08752e9

Browse files
authored
chore(cli): remove the notion of stopped session (microsoft#39143)
1 parent 8f4007c commit 08752e9

3 files changed

Lines changed: 40 additions & 20 deletions

File tree

packages/playwright/src/mcp/terminal/program.ts

Lines changed: 29 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -95,14 +95,7 @@ to restart the session daemon.`);
9595
return;
9696
}
9797

98-
await this._send('stop').catch(e => {
99-
if (e.message !== 'Session closed')
100-
throw e;
101-
});
102-
this.disconnect();
103-
104-
if (os.platform() !== 'win32')
105-
await fs.promises.unlink(this._config.socketPath).catch(() => {});
98+
await this._stopDaemon();
10699
console.log(`Session '${this.name}' stopped.`);
107100
}
108101

@@ -215,11 +208,15 @@ to restart the session daemon.`);
215208
}
216209
}
217210

211+
private _sessionConfigFile() {
212+
return path.resolve(this._clientInfo.daemonProfilesDir, `${this.name}.session`);
213+
}
214+
218215
private async _startDaemon(): Promise<net.Socket> {
219216
await fs.promises.mkdir(this._clientInfo.daemonProfilesDir, { recursive: true });
220217
const cliPath = path.join(__dirname, '../../../cli.js');
221218

222-
const sessionConfigFile = path.resolve(this._clientInfo.daemonProfilesDir, `${this.name}.session`);
219+
const sessionConfigFile = this._sessionConfigFile();
223220
this._config.version = this._clientInfo.version;
224221
await fs.promises.writeFile(sessionConfigFile, JSON.stringify(this._config, null, 2));
225222

@@ -278,6 +275,21 @@ to restart the session daemon.`);
278275
console.error(errData);
279276
process.exit(1);
280277
}
278+
279+
private async _stopDaemon(): Promise<void> {
280+
let error: Error | undefined;
281+
await this._send('stop').catch(e => { error = e; });
282+
this.disconnect();
283+
await this.deleteSession();
284+
if (error && !error?.message?.includes('Session closed'))
285+
throw error;
286+
}
287+
288+
async deleteSession() {
289+
if (os.platform() !== 'win32')
290+
await fs.promises.unlink(this._config.socketPath).catch(() => {});
291+
await fs.promises.rm(this._sessionConfigFile()).catch(() => {});
292+
}
281293
}
282294

283295
class SessionManager {
@@ -491,9 +503,14 @@ export async function program(packageLocation: string) {
491503
const sessions = sessionManager.sessions;
492504
console.log('Sessions:');
493505
for (const session of sessions.values()) {
494-
const liveMarker = await session.canConnect() ? `[running] ` : '[stopped] ';
495-
const restartMarker = !session.isCompatible() ? ` - v${session.config().version}, needs restart` : '';
496-
console.log(` ${liveMarker}${session.name}${restartMarker}`);
506+
const canConnect = await session.canConnect();
507+
if (!canConnect) {
508+
console.log(` ${session.name} is stale, removing`);
509+
await session.deleteSession();
510+
} else {
511+
const restartMarker = !session.isCompatible() ? ` - v${session.config().version}, please reopen` : '';
512+
console.log(` ${session.name}${restartMarker}`);
513+
}
497514
}
498515
if (sessions.size === 0)
499516
console.log(' (no sessions)');

tests/mcp/cli-isolated.spec.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ test('should not save user data by default (in-memory mode)', async ({ cli, serv
3434

3535
const { output: listOutput } = await cli('session-list');
3636
expect(listOutput).toContain('Sessions:');
37-
expect(listOutput).toContain(' [running] default');
37+
expect(listOutput).toContain(' default');
3838
});
3939

4040
test('should save user data with --persistent flag', async ({ cli, server, mcpBrowser }, testInfo) => {

tests/mcp/cli-session.spec.ts

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ test('session-list', async ({ cli, server }) => {
2626

2727
const { output: listOutput } = await cli('session-list');
2828
expect(listOutput).toContain('Sessions:');
29-
expect(listOutput).toContain(' [running] default');
29+
expect(listOutput).toContain(' default');
3030
});
3131

3232
test('close', async ({ cli, server }) => {
@@ -36,7 +36,7 @@ test('close', async ({ cli, server }) => {
3636
expect(output).toContain(`Session 'default' stopped.`);
3737

3838
const { output: listOutput } = await cli('session-list');
39-
expect(listOutput).toContain('[stopped] default');
39+
expect(listOutput).toContain('(no sessions)');
4040
});
4141

4242
test('close named session', async ({ cli, server }) => {
@@ -56,13 +56,13 @@ test('session-close-all', async ({ cli, server }) => {
5656
await cli('--session=session2', 'open', server.HELLO_WORLD);
5757

5858
const { output: listBefore } = await cli('session-list');
59-
expect(listBefore).toContain('[running] session1');
60-
expect(listBefore).toContain('[running] session2');
59+
expect(listBefore).toContain('session1');
60+
expect(listBefore).toContain('session2');
6161

6262
await cli('session-close-all');
6363

6464
const { output: listAfter } = await cli('session-list');
65-
expect(listAfter).not.toContain('[running]');
65+
expect(listAfter).not.toContain('session1');
6666
});
6767

6868
test('delete-data', async ({ cli, server, mcpBrowser }, testInfo) => {
@@ -98,12 +98,15 @@ test('session stops when browser exits', async ({ cli, server }) => {
9898
await cli('open', server.HELLO_WORLD);
9999

100100
const { output: listBefore } = await cli('session-list');
101-
expect(listBefore).toContain('[running] default');
101+
expect(listBefore).toContain('default');
102102

103103
// Close the browser - this will cause the daemon to exit so the command may fail
104104
await cli('run-code', '() => page.context().browser().close()').catch(() => {});
105105

106-
await expect.poll(() => cli('session-list').then(r => r.output)).toContain('[stopped]');
106+
await expect.poll(() => cli('session-list').then(r => r.output)).toContain('default is stale, removing');
107+
await cli('close');
108+
const { output: listAfter } = await cli('session-list');
109+
expect(listAfter).toContain('(no sessions)');
107110
});
108111

109112
test('session reopen with different config', async ({ cli, server }, testInfo) => {

0 commit comments

Comments
 (0)