fix(mcp): make external-agent registration survive restarts (persistent token + stable fallback port)

desktop/embedded-port.ts

@@ -0,0 +1,106 @@
+import { mkdirSync, readFileSync, writeFileSync } from 'node:fs';
+import type { Server } from 'node:http';
+import { homedir } from 'node:os';
+import { dirname, join } from 'node:path';
+
+/** The documented external-MCP address; README and the MCP panel assume it. */
+export const CANONICAL_EMBEDDED_PORT = 5199;
+
+export interface EmbeddedPortLocation {
+  /** Overridable for tests; never for callers wiring the real server. */
+  readonly home?: string;
+  /** Isolated dev profiles keep their own memory, exactly like the MCP token:
+   *  the profile-scoped instance lock lets a packaged app and a dev checkout
+   *  run concurrently, and one memory slot shared between them would ping-pong
+   *  at every contended launch — the very instability this file removes. */
+  readonly profileId?: string;
+}
+
+/** Same HOME-anchored hidden root as the MCP token, and for the same reason:
+ *  the user-chosen data dir may be a synced folder, and machine-local wiring
+ *  state has no business following a sync service to another machine. */
+export function embeddedPortPath({ home = homedir(), profileId }: EmbeddedPortLocation = {}): string {
+  const root = profileId
+    ? join(home, '.openchatcut', 'dev-profiles', profileId)
+    : join(home, '.openchatcut');
+  return join(root, 'mcp-port');
+}
+
+export function readRememberedPort(location: EmbeddedPortLocation = {}): number | null {
+  try {
+    const port = Number(readFileSync(embeddedPortPath(location), 'utf8').trim());
+    // The canonical port is never remembered: it is always tried first anyway,
+    // and remembering it would just shadow a stale write.
+    if (!Number.isInteger(port) || port < 1024 || port > 65535 || port === CANONICAL_EMBEDDED_PORT) {
+      return null;
+    }
+    return port;
+  } catch {
+    return null;
+  }
+}
+
+export function rememberPort(port: number, location: EmbeddedPortLocation = {}): boolean {
+  try {
+    const path = embeddedPortPath(location);
+    mkdirSync(dirname(path), { recursive: true, mode: 0o700 });
+    writeFileSync(path, `${port}\n`, { mode: 0o600 });
+    return true;
+  } catch {
+    return false;
+  }
+}
+
+export interface ListenWithAffinityOptions extends EmbeddedPortLocation {
+  /** Overridable for tests; the real server always uses the documented port. */
+  readonly canonicalPort?: number;
+  readonly log?: (message: string) => void;
+}
+
+/**
+ * Bind the embedded server to a port external agents can rely on.
+ *
+ * The canonical port comes first so the documented address self-heals the
+ * moment whatever occupied it goes away. When it is taken, the port used the
+ * LAST time this happened is tried before anything random: the usual occupant
+ * is a long-lived neighbour (a dev server, another tool), so the conflict
+ * repeats at every launch, and a random port each time silently broke every
+ * registered MCP client. Only when both are busy does a fresh random port get
+ * picked, and it immediately becomes the remembered one.
+ */
+export async function listenWithAffinity(
+  server: Server,
+  { canonicalPort = CANONICAL_EMBEDDED_PORT, home, profileId, log = console.warn }: ListenWithAffinityOptions = {},
+): Promise<number> {
+  const listenOn = (port: number) => new Promise<number>((resolvePort, reject) => {
+    const onError = (err: Error) => reject(err);
+    server.once('error', onError);
+    server.listen(port, '127.0.0.1', () => {
+      server.off('error', onError);
+      const addr = server.address();
+      if (addr && typeof addr === 'object') resolvePort(addr.port);
+      else reject(new Error('embedded server failed to bind'));
+    });
+  });
+  const inUse = (err: unknown): boolean => (err as NodeJS.ErrnoException).code === 'EADDRINUSE';
+
+  try {
+    return await listenOn(canonicalPort);
+  } catch (err) {
+    if (!inUse(err)) throw err;
+  }
+  const remembered = readRememberedPort({ home, profileId });
+  if (remembered !== null) {
+    try {
+      const port = await listenOn(remembered);
+      log(`[embedded-server] port ${canonicalPort} in use — reusing remembered fallback ${port}`);
+      return port;
+    } catch (err) {
+      if (!inUse(err)) throw err;
+    }
+  }
+  const port = await listenOn(0);
+  rememberPort(port, { home, profileId });
+  log(`[embedded-server] port ${canonicalPort} in use — falling back to ${port}, kept for future launches; point external MCP clients at the origin logged below`);
+  return port;
+}

desktop/embedded-port.verify.ts

@@ -0,0 +1,111 @@
+import assert from 'node:assert/strict';
+import { mkdtempSync, readFileSync, rmSync, statSync, writeFileSync } from 'node:fs';
+import { createServer, type Server } from 'node:http';
+import { tmpdir } from 'node:os';
+import { join } from 'node:path';
+import { embeddedPortPath, listenWithAffinity, readRememberedPort, rememberPort } from './embedded-port.ts';
+
+const home = mkdtempSync(join(tmpdir(), 'occ-port-'));
+const servers: Server[] = [];
+const fresh = (): Server => {
+  const server = createServer((_req, res) => res.end('ok'));
+  servers.push(server);
+  return server;
+};
+const bindRandom = (server: Server) => new Promise<number>((resolve, reject) => {
+  server.once('error', reject);
+  server.listen(0, '127.0.0.1', () => {
+    const addr = server.address();
+    if (addr && typeof addr === 'object') resolve(addr.port);
+    else reject(new Error('bind failed'));
+  });
+});
+const close = (server: Server) => new Promise<void>((resolve) => server.close(() => resolve()));
+const silent = () => {};
+
+try {
+  // Reserve two real ports to stand in for "canonical" and its occupant, so the
+  // suite never touches the machine's actual 5199 (often busy on a dev box).
+  const canonicalBlocker = fresh();
+  const canonical = await bindRandom(canonicalBlocker);
+  await close(canonicalBlocker);
+  servers.pop();
+
+  // Canonical free: bound directly, and nothing is remembered — there is no
+  // fallback situation to remember.
+  const direct = fresh();
+  assert.equal(await listenWithAffinity(direct, { canonicalPort: canonical, home, log: silent }), canonical);
+  assert.equal(readRememberedPort({ home }), null, 'no fallback file when the canonical port worked');
+  await close(direct);
+
+  // Canonical busy, nothing remembered: a random port is picked AND persisted,
+  // owner-only, so the next conflicted launch lands on the same address.
+  const occupant = fresh();
+  await new Promise<void>((resolve, reject) => {
+    occupant.once('error', reject);
+    occupant.listen(canonical, '127.0.0.1', () => resolve());
+  });
+  const first = fresh();
+  const fallback = await listenWithAffinity(first, { canonicalPort: canonical, home, log: silent });
+  assert.notEqual(fallback, canonical);
+  assert.equal(readRememberedPort({ home }), fallback, 'the fallback is remembered');
+  assert.equal(statSync(embeddedPortPath({ home })).mode & 0o777, 0o600, 'port file is owner-only');
+  await close(first);
+
+  // Same conflict on the next launch: the SAME fallback comes back. This is the
+  // whole point — a registered MCP client keeps its address across restarts.
+  const second = fresh();
+  assert.equal(
+    await listenWithAffinity(second, { canonicalPort: canonical, home, log: silent }),
+    fallback,
+    'a repeated conflict reuses the remembered fallback',
+  );
+  await close(second);
+
+  // Canonical AND remembered both busy: a new random port, which replaces the
+  // remembered one instead of leaving a stale pointer behind.
+  const fallbackBlocker = fresh();
+  await new Promise<void>((resolve, reject) => {
+    fallbackBlocker.once('error', reject);
+    fallbackBlocker.listen(fallback, '127.0.0.1', () => resolve());
+  });
+  const third = fresh();
+  const rerolled = await listenWithAffinity(third, { canonicalPort: canonical, home, log: silent });
+  assert.ok(rerolled !== canonical && rerolled !== fallback);
+  assert.equal(readRememberedPort({ home }), rerolled, 'the new fallback replaces the old one');
+  await close(third);
+  await close(fallbackBlocker);
+
+  // The conflict goes away: the canonical, documented address self-heals, even
+  // though a remembered fallback still exists on disk.
+  await close(occupant);
+  const healed = fresh();
+  assert.equal(
+    await listenWithAffinity(healed, { canonicalPort: canonical, home, log: silent }),
+    canonical,
+    'the canonical port wins again as soon as it is free',
+  );
+  await close(healed);
+
+  // A corrupted or nonsensical port file is ignored rather than dialled.
+  writeFileSync(embeddedPortPath({ home }), 'pas-un-port\n');
+  assert.equal(readRememberedPort({ home }), null);
+  writeFileSync(embeddedPortPath({ home }), '80\n');
+  assert.equal(readRememberedPort({ home }), null, 'privileged ports are never remembered');
+  assert.ok(rememberPort(rerolled, { home }), 'a valid port can be written back');
+  assert.equal(readFileSync(embeddedPortPath({ home }), 'utf8').trim(), String(rerolled));
+
+  // A dev profile keeps its own memory: the profile-scoped instance lock lets a
+  // packaged app and a dev checkout run at once, and a shared slot would
+  // ping-pong between them at every contended launch.
+  const profileId = '9b6a2f00-1234-4abc-8def-556677889900';
+  assert.ok(rememberPort(6001, { home, profileId }));
+  assert.equal(readRememberedPort({ home, profileId }), 6001);
+  assert.equal(readRememberedPort({ home }), rerolled, 'the default profile memory is untouched');
+  assert.ok(embeddedPortPath({ home, profileId }).includes('dev-profiles'), 'profile memory lives under its profile root');
+
+  console.log('embedded-port.verify OK');
+} finally {
+  await Promise.all(servers.map((server) => new Promise<void>((resolve) => server.close(() => resolve()))));
+  rmSync(home, { recursive: true, force: true });
+}

desktop/embedded-server.ts

@@ -14,6 +14,8 @@ import { trustedEditorRequest } from '../server/editor-auth.ts';
 import { proxyMiddleware, type ProxyRoute } from '../server/proxy.ts';
 import { parseEnvText } from './env-file.ts';
 import { createMiniConnect, type MiniConnect } from './mini-connect.ts';
+import { listenWithAffinity } from './embedded-port.ts';
+import { runtimeProfile } from '../server/runtime-profile.ts';
 import { distStaticMiddleware, uploadsMiddleware } from './static-files.ts';
 
 export interface EmbeddedServer {
@@ -101,23 +103,14 @@ export async function startEmbeddedServer(distDir: string): Promise<EmbeddedServ
   app.use('/media/uploads', uploadsMiddleware());
   app.use(distStaticMiddleware(distDir));
 
-  // Port policy: priority 5199 (document address of external MCP client in README); occupied (web page dev
-  // server / second desktop instance), fall back to a random port - the App must be able to start, and the MCP client is started instead.
-  // The actual port in the log. Other listen errors are still thrown.
-  const listenOn = (port: number) => new Promise<number>((resolvePort, reject) => {
-    const onError = (err: Error) => reject(err);
-    server.once('error', onError);
-    server.listen(port, '127.0.0.1', () => {
-      server.off('error', onError);
-      const addr = server.address();
-      if (addr && typeof addr === 'object') resolvePort(addr.port);
-      else reject(new Error('embedded server failed to bind'));
-    });
-  });
-  const port = await listenOn(5199).catch((err: NodeJS.ErrnoException) => {
-    if (err.code !== 'EADDRINUSE') throw err;
-    console.warn('[embedded-server] port 5199 in use — falling back to a random port; point external MCP clients at the origin logged below');
-    return listenOn(0);
-  });
+  // Port policy: canonical 5199 first (the documented external-MCP address),
+  // then the fallback used last time it was busy, then a fresh random port that
+  // becomes the remembered one. See listenWithAffinity for why the fallback has
+  // to be stable: a random port per launch silently broke registered agents.
+  const profile = runtimeProfile();
+  const port = await listenWithAffinity(
+    server,
+    profile.mode === 'isolated-dev' ? { profileId: profile.id } : {},
+  );
   return { server, port, origin: `http://127.0.0.1:${port}` };
 }

package.json

@@ -28,7 +28,7 @@
     "verify:affected": "node scripts/run-affected-verifies.mjs",
     "verify:media-drag": "tsx src/media/drag.verify.ts",
     "verify:semantic-sampling": "tsx src/media/semantic-search/samplingConfig.verify.ts",
-    "verify:runtime-profile": "node scripts/dev-profile.verify.mjs && tsx server/runtime-profile.verify.ts && tsx desktop/runtime-profile.verify.ts && tsx server/keystore-profile.verify.ts && tsx server/keystore-default-profile.verify.ts && tsx server/media-dir-profile.verify.ts && tsx server/data-dir.verify.ts && tsx server/r2-profile.verify.ts",
+    "verify:runtime-profile": "node scripts/dev-profile.verify.mjs && tsx server/runtime-profile.verify.ts && tsx desktop/runtime-profile.verify.ts && tsx server/keystore-profile.verify.ts && tsx server/keystore-default-profile.verify.ts && tsx server/media-dir-profile.verify.ts && tsx server/data-dir.verify.ts && tsx server/r2-profile.verify.ts && tsx server/mcp-token.verify.ts && tsx desktop/embedded-port.verify.ts",
     "verify:directory-watch": "tsx shared/directory-import.verify.ts && tsx desktop/directory-watch-errors.verify.ts && tsx desktop/directory-watch-controller.verify.ts && tsx desktop/directory-watch-import.verify.ts && tsx desktop/directory-watch.verify.ts && tsx src/media/directoryDrop.verify.ts && tsx src/media/directoryImportAsset.verify.ts && tsx src/media/useDirectoryImport.verify.ts && tsx src/media/directoryImportIntegration.verify.ts && tsx src/media/media-folder-menu.verify.ts && tsx src/media/media-pool-batch-timeline.verify.ts",
     "verify:clip-fx-export": "node src/gl/clipFxExport.verify.mjs",
     "verify:approval-policy": "tsx src/agent/approval-policy.verify.ts",

server/editor-auth.ts

@@ -1,20 +1,39 @@
-import { randomBytes, timingSafeEqual } from 'node:crypto';
+import { timingSafeEqual } from 'node:crypto';
 import type { IncomingMessage } from 'node:http';
 import { TLSSocket } from 'node:tls';
 import type { EditorBootstrapInfo } from '../shared/editor-auth-transport.ts';
 import { isLoopbackAddress } from './loopback-address.ts';
+import { loadOrCreateMcpToken } from './mcp-token.ts';
+import { runtimeProfile } from './runtime-profile.ts';
 
 export const EDITOR_BOOTSTRAP_HEADER = 'x-openchatcut-editor-bootstrap';
 
-const generatedMcpToken = randomBytes(32).toString('base64url');
+/** Lazy so tests and the env override never touch the filesystem. */
+let persistentMcpToken: string | undefined;
+
+function resolvePersistentMcpToken(): string {
+  if (persistentMcpToken === undefined) {
+    const profile = runtimeProfile();
+    const result = loadOrCreateMcpToken(
+      profile.mode === 'isolated-dev' ? { profileId: profile.id } : {},
+    );
+    if (!result.persisted) {
+      // The MCP guide promises a stable token; when the filesystem breaks that
+      // promise the user deserves one line saying so and how to pin it.
+      console.warn('[mcp] token could not be persisted and will change on restart; set OPENCHATCUT_MCP_TOKEN to pin it');
+    }
+    persistentMcpToken = result.token;
+  }
+  return persistentMcpToken;
+}
 const LOCAL_EDITOR_HOSTS: Readonly<Record<string, true>> = {
   localhost: true,
   '127.0.0.1': true,
   '[::1]': true,
 };
 
 export function externalMcpToken(): string {
-  return process.env.OPENCHATCUT_MCP_TOKEN?.trim() || generatedMcpToken;
+  return process.env.OPENCHATCUT_MCP_TOKEN?.trim() || resolvePersistentMcpToken();
 }
 
 function secretMatches(actual: string | undefined, expected: string): boolean {

server/mcp-token.ts

@@ -0,0 +1,85 @@
+import { randomBytes } from 'node:crypto';
+import { mkdirSync, readFileSync, unlinkSync, writeFileSync } from 'node:fs';
+import { homedir } from 'node:os';
+import { dirname, join } from 'node:path';
+
+/** 32 random bytes as base64url, the shape externalMcpToken always minted. */
+const TOKEN_PATTERN = /^[A-Za-z0-9_-]{43}$/;
+
+export interface McpTokenLocation {
+  /** Overridable for tests; never for callers wiring the real server. */
+  readonly home?: string;
+  /** Isolated dev profiles keep their own token so checkouts stay independent. */
+  readonly profileId?: string;
+}
+
+/**
+ * Where the persistent MCP token lives. Deliberately under the HOME-anchored
+ * hidden root and NOT under the user-chosen data dir: the data dir may sit in a
+ * synced folder (that is the point of making it configurable), and a bearer
+ * secret must not ride a sync service onto other machines. Same reasoning as
+ * the data-dir pointer file itself.
+ */
+export function mcpTokenPath({ home = homedir(), profileId }: McpTokenLocation = {}): string {
+  const root = profileId
+    ? join(home, '.openchatcut', 'dev-profiles', profileId)
+    : join(home, '.openchatcut');
+  return join(root, 'mcp-token');
+}
+
+export interface McpTokenResult {
+  readonly token: string;
+  /** False when the filesystem refused: the token is valid but per-process. */
+  readonly persisted: boolean;
+}
+
+/**
+ * The token external agents authenticate with, minted once and reused.
+ *
+ * It used to be random per process, which quietly broke every registered agent
+ * on each app restart: the copied `claude mcp add` command carried a token that
+ * no longer existed, and for a Claude Code subscriber that command is the only
+ * way into the app. Persisting the first token makes registration a one-time
+ * step. OPENCHATCUT_MCP_TOKEN still overrides (handled by the caller), and a
+ * filesystem failure falls back to the old per-process behaviour rather than
+ * refusing to serve.
+ */
+export function loadOrCreateMcpToken(location: McpTokenLocation = {}): McpTokenResult {
+  const path = mcpTokenPath(location);
+  const readValid = (): string | null => {
+    try {
+      const existing = readFileSync(path, 'utf8').trim();
+      return TOKEN_PATTERN.test(existing) ? existing : null;
+    } catch {
+      return null;
+    }
+  };
+  const existing = readValid();
+  if (existing !== null) return { token: existing, persisted: true };
+  // A malformed file is replaced rather than trusted: serving whatever ended up
+  // in it would turn a corrupted write into the endpoint's credential. Removing
+  // it (instead of overwriting) also sheds whatever loose permissions the old
+  // file carried, so the healed credential is always written fresh at 0600.
+  try {
+    unlinkSync(path);
+  } catch {
+    // Missing file: first launch.
+  }
+  const token = randomBytes(32).toString('base64url');
+  try {
+    mkdirSync(dirname(path), { recursive: true, mode: 0o700 });
+    // Exclusive create: the profile-scoped instance lock does not serialize a
+    // packaged app against a dev server sharing the same HOME, so two first
+    // launches can race to mint. 'wx' lets exactly one write win; the loser
+    // adopts the winner below, and both processes end up serving the token the
+    // file actually holds.
+    writeFileSync(path, `${token}\n`, { mode: 0o600, flag: 'wx' });
+    return { token, persisted: true };
+  } catch (err) {
+    if ((err as NodeJS.ErrnoException).code === 'EEXIST') {
+      const winner = readValid();
+      if (winner !== null) return { token: winner, persisted: true };
+    }
+    return { token, persisted: false };
+  }
+}