Skip to content

Commit e7dd3ab

Browse files
edgarsskoreclaudewonderwhy-er
authored
fix(remote-device): persist the session by default (#634)
* fix(remote-device): persist the session by default Off-by-default meant a full browser re-authorization on every connector start, each minting a fresh GoTrue session that nothing ever revokes. Those orphaned refresh-token families get replayed, trip GoTrue's reuse detection, and revoke the whole family including the token a healthy connector is holding, which is the upstream trigger of the anon-key downgrade wedge (#632). --persist-session stays as an accepted no-op; --no-persist-session opts back out (tokens in memory only, re-auth every start). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019fXdisjKmwoi5YGPK2YfBa * fix(remote-device): apply the persist-session default on the shipped CLI path Two review fixes: - npm-scripts/remote.ts (the path `npx desktop-commander remote` actually runs) still passed process.argv.includes('--persist-session') — an explicit false whenever the flag was absent, so the constructor's `?? true` default never applied where it matters. Parse --no-persist-session there too, matching the device.ts entry. - loadPersistedConfig() returned a previously saved session regardless of the flag, so the first opted-out run silently skipped the promised re-authorization and then discarded a possibly-rotated refresh token on save. Only return it when persistence is on. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011rAeXGKpqJVmKJaNoVook1 * fix(remote-device): drop usage figures from persist-session comment --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: Eduard Ruzga <wonderwhy.er@gmail.com>
1 parent 9bd8422 commit e7dd3ab

3 files changed

Lines changed: 30 additions & 11 deletions

File tree

src/npm-scripts/remote.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,12 @@ import { MCPDevice } from '../remote-device/device.js';
22
import os from 'os';
33

44
export async function runRemote() {
5-
const persistSession = process.argv.includes('--persist-session');
5+
// --persist-session is kept as an accepted no-op so existing invocations
6+
// and docs keep working; --no-persist-session opts back out.
7+
const persistSession = !process.argv.includes('--no-persist-session');
8+
if (!persistSession) {
9+
console.log('🔓 Session persistence disabled — re-authorization required on every start');
10+
}
611
const disableNoSleep = process.argv.includes('--disable-no-sleep');
712
const verbose = process.argv.includes('--debug');
813
console.debug('[DEBUG] Verbose mode: ', verbose);

src/remote-device/README.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -84,12 +84,12 @@ Run from the project repository without global installation:
8484
desktop-commander-device
8585
```
8686

87-
**With session persistence** (optional):
87+
**Without session persistence** (opt out):
8888
```bash
89-
desktop-commander-device --persist-session
89+
desktop-commander-device --no-persist-session
9090
```
9191

92-
> **Note**: By default, only the device ID is persisted. Use `--persist-session` to also save authentication tokens between restarts. This allows the device to reconnect automatically without re-authentication.
92+
> **Note**: The device ID and authentication tokens are persisted by default to `~/.desktop-commander-device/device.json` (mode 0600), so the device reconnects without re-authorization. Pass `--no-persist-session` to keep tokens in memory only — the device then requires a full browser re-authorization on every start, and each one leaves a live server-side session behind.
9393

9494
**If using local installation** from the project root directory:
9595

src/remote-device/device.ts

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,11 @@ export class MCPDevice {
3939
this.deviceId = undefined;
4040
this.isShuttingDown = false;
4141
this.configPath = path.join(os.homedir(), '.desktop-commander-device', 'device.json');
42-
this.persistSession = options.persistSession || false;
42+
// Default ON. Off meant a full re-authorization on every start, and each
43+
// one mints a fresh GoTrue session that nothing ever revokes; the orphaned
44+
// refresh-token families get replayed, trip GoTrue's reuse detection, and
45+
// take the whole family down including the token a healthy connector holds.
46+
this.persistSession = options.persistSession ?? true;
4347

4448
// Initialize desktop integration
4549
this.desktop = new DesktopCommanderIntegration();
@@ -202,13 +206,21 @@ export class MCPDevice {
202206
this.deviceId = config?.deviceId;
203207
console.debug('[DEBUG] Loaded device ID:', this.deviceId);
204208

205-
console.log('💾 Found persisted session for device ' + this.deviceId);
206-
if (config.session) {
209+
if (config.session && this.persistSession) {
210+
console.log('💾 Found persisted session for device ' + this.deviceId);
207211
console.debug('[DEBUG] Session found in config, returning session');
208212
return config.session;
209213
}
210214

211-
console.debug('[DEBUG] No session in config');
215+
// A previously saved session must not be reused on an opted-out run:
216+
// it would skip the re-authorization the flag promises, and the save
217+
// at the end of start() then discards a possibly-rotated refresh
218+
// token — orphaning one more live server-side session.
219+
if (config.session) {
220+
console.debug('[DEBUG] Ignoring persisted session (--no-persist-session)');
221+
} else {
222+
console.debug('[DEBUG] No session in config');
223+
}
212224
return null;
213225
} catch (error: any) {
214226

@@ -427,11 +439,13 @@ if (isMainModule) {
427439
// Parse command-line arguments
428440
const args = process.argv.slice(2);
429441
const options = {
430-
persistSession: args.includes('--persist-session')
442+
// --persist-session is kept as an accepted no-op so existing invocations
443+
// and docs keep working; --no-persist-session opts back out.
444+
persistSession: !args.includes('--no-persist-session')
431445
};
432446

433-
if (options.persistSession) {
434-
console.log('🔒 Session persistence enabled');
447+
if (!options.persistSession) {
448+
console.log('🔓 Session persistence disabled — re-authorization required on every start');
435449
}
436450

437451
const device = new MCPDevice(options);

0 commit comments

Comments
 (0)