fix(mcp): recover orphaned sessions and preserve multimodal results

server/external-agent/broker-registry.ts

@@ -105,14 +105,9 @@ export class EditorConnectionRegistry {
     if (!trustedInternalCall && registrationCapability && !validRenewal) {
       throw new ExternalEditorCallError('stale', 'Editor registration capability is stale.');
     }
-    // A different browser window may take over the active connection for the
-    // same project. Single-window desktop users never open one project in two
-    // windows, but a reloaded/fresh window (a new random editor id) must be able
-    // to (re)connect without a persistent "already has an active editor"
-    // rejection. The old entry is replaced below by this.editors.set, and the
-    // persisted ownership claim (validated at validateOwnershipClaim) still
-    // fences stale takeovers. Offline/multi-writer safety comes from the
-    // serialized project-store mutations and the ownership epoch checks.
+    // The project-store ownership claim fences competing tabs/windows before
+    // this registry is reached. A refresh reuses the tab-scoped editor id;
+    // registrations with a different identity cannot obtain a live claim.
     validateOwnershipClaim(ownership, { projectId, editorInstanceId, baseRevision });
     const ownershipEpoch = ownership?.epoch;
     if (previous && bindingChanged(previous, editorInstanceId, baseRevision, ownershipEpoch)) {

server/external-agent/broker.ts

@@ -48,7 +48,7 @@ export interface ExternalEditorCancellation {
   id: string;
   outcome: Exclude<ExternalCallTerminalOutcome, 'applied'>;
   message: string;
-  /** Edit sessions orphaned by the owner transport disconnect; the editor discards them. */
+  /** Legacy field accepted by older editors. New brokers preserve orphaned drafts for recovery. */
   ownerGone?: string[];
 }
 
@@ -62,6 +62,7 @@ const queues = new Map<string, QueuedCall[]>();
 const pending = new Map<string, QueuedCall>();
 const waiters = new Map<string, Set<() => void>>();
 const editSessionOwners = new Map<string, EditSessionOwner>();
+const orphanedEditSessions = new Map<string, EditorBinding>();
 const cancellationQueues = new Map<string, ExternalEditorCancellation[]>();
 const cancellationWaiters = new Map<string, Set<() => void>>();
 
@@ -133,18 +134,47 @@ function terminalMessage(value: unknown): string {
 
 function recordEditSessionOwner(call: QueuedCall, value: unknown): void {
   if (
-    call.name !== 'begin_edit_session'
+    call.name !== 'begin_edit_session' && call.name !== 'recover_edit_session'
     || !value
     || typeof value !== 'object'
     || Array.isArray(value)
   ) return;
   if (!('editSessionId' in value)) return;
+  if (call.name === 'recover_edit_session' && call.arguments.action !== 'resume') return;
   const editSessionId = value.editSessionId;
   if (typeof editSessionId !== 'string' || !editSessionId.trim()) return;
   editSessionOwners.set(editSessionId.trim(), {
     ownerId: call.ownerId,
     binding: { ...call.binding },
   });
+  orphanedEditSessions.delete(editSessionId.trim());
+}
+
+function sessionRecoveryResult(ownerId: string, binding: EditorBinding, value: unknown): unknown {
+  if (!Array.isArray(value)) return value;
+  return value.map((entry) => {
+    if (!entry || typeof entry !== 'object' || Array.isArray(entry)) return entry;
+    const record = entry as Record<string, unknown>;
+    const id = typeof record.editSessionId === 'string' ? record.editSessionId : '';
+    const owner = editSessionOwners.get(id);
+    // The editor persists draft sessions, while transport ownership is held in
+    // this broker process. After an MCP/server crash or restart the durable
+    // draft can therefore come back without an in-memory owner entry. Treat an
+    // ownerless active draft as orphaned so a new authenticated transport can
+    // explicitly resume or discard it instead of leaving the project locked.
+    const activeDraft = record.status === 'drafting' || record.status === 'awaiting_review';
+    const orphaned = activeDraft && (orphanedEditSessions.has(id) || !owner);
+    if (orphaned && !orphanedEditSessions.has(id)) {
+      orphanedEditSessions.set(id, { ...binding });
+    }
+    return {
+      ...record,
+      ownerOnline: Boolean(owner),
+      orphaned,
+      recoveryActions: orphaned ? (record.stale === true ? ['discard'] : ['resume', 'discard']) : [],
+      ownedByCurrentTransport: owner?.ownerId === ownerId && sameBinding(owner.binding, binding),
+    };
+  });
 }
 
 function finishCall(
@@ -159,7 +189,15 @@ function finishCall(
   wake(waiters, call.binding.projectId);
   if (outcome === 'applied') {
     recordEditSessionOwner(call, value);
-    call.resolve(value);
+    if (call.name === 'recover_edit_session' && call.arguments.action === 'discard') {
+      const sessionId = typeof call.arguments.editSessionId === 'string'
+        ? call.arguments.editSessionId.trim()
+        : '';
+      if (sessionId) orphanedEditSessions.delete(sessionId);
+    }
+    call.resolve(call.name === 'list_edit_sessions'
+      ? sessionRecoveryResult(call.ownerId, call.binding, value)
+      : value);
     return true;
   }
   const message = terminalMessage(value);
@@ -312,6 +350,19 @@ function requireOwnedEditSession(
   args: Record<string, unknown>,
 ): void {
   if (editSessionOwnerMatches(ownerId, binding, args.editSessionId)) return;
+  const editSessionId = typeof args.editSessionId === 'string' ? args.editSessionId.trim() : '';
+  const owner = editSessionOwners.get(editSessionId);
+  // A page refresh may renew the browser ownership lease and therefore change
+  // only the epoch. The tab-scoped editor identity and unchanged project
+  // revision prove this is the same editor, while a competing tab has a
+  // different identity and still fails closed.
+  if (owner
+    && owner.ownerId === ownerId
+    && sameEditorIdentity(owner.binding, binding)
+    && owner.binding.baseRevision === binding.baseRevision) {
+    owner.binding = { ...binding };
+    return;
+  }
   throw new ExternalEditorCallError(
     'rejected',
     'The requested edit session does not belong to this MCP transport and editor binding.',
@@ -355,7 +406,14 @@ export function invokeEditorTool(
   timeoutMs = DEFAULT_TIMEOUT_MS,
 ): Promise<unknown> {
   const allowRevisionDrift = name === 'get_edit_session';
-  const ownsSession = name !== 'begin_edit_session' && 'editSessionId' in args;
+  const recoveryTool = name === 'list_edit_sessions' || name === 'recover_edit_session';
+  const ownsSession = name !== 'begin_edit_session' && !recoveryTool && 'editSessionId' in args;
+  if (name === 'recover_edit_session') {
+    const sessionId = typeof args.editSessionId === 'string' ? args.editSessionId.trim() : '';
+    if (!sessionId || !orphanedEditSessions.has(sessionId)) {
+      throw new ExternalEditorCallError('rejected', 'Only an orphaned edit session can be recovered.');
+    }
+  }
   if (ownsSession) {
     requireOwnedEditSession(ownerId, binding, args);
   }
@@ -519,27 +577,10 @@ export function cancelEditorCallsForOwner(
   outcome: Extract<ExternalCallTerminalOutcome, 'cancelled' | 'stale' | 'failed'> = 'cancelled',
   message = 'MCP transport session closed before the editor call completed.',
 ): number {
-  const orphanedByEditor = new Map<string, string[]>();
   for (const [sessionId, owner] of editSessionOwners) {
     if (owner.ownerId !== ownerId) continue;
     editSessionOwners.delete(sessionId);
-    const key = editorKey(owner.binding.projectId, owner.binding.editorInstanceId);
-    const list = orphanedByEditor.get(key) ?? [];
-    list.push(sessionId);
-    orphanedByEditor.set(key, list);
-  }
-  // Let each connected editor discard the sessions its transport orphaned, so a
-  // crashed or closed MCP client cannot leave a drafting session wedged forever.
-  for (const [key, sessionIds] of orphanedByEditor) {
-    const queue = cancellationQueues.get(key) ?? [];
-    queue.push({
-      id: '',
-      outcome,
-      message: `MCP transport session closed; ${sessionIds.length} edit session(s) orphaned.`,
-      ownerGone: sessionIds,
-    });
-    cancellationQueues.set(key, queue);
-    wake(cancellationWaiters, key);
+    orphanedEditSessions.set(sessionId, { ...owner.binding });
   }
   return cancelCalls((call) => call.ownerId === ownerId, outcome, message);
 }
@@ -562,4 +603,5 @@ export function resetExternalAgentBrokerForTest(): void {
   cancellationQueues.clear();
   cancellationWaiters.clear();
   editSessionOwners.clear();
+  orphanedEditSessions.clear();
 }

server/external-agent/broker.verify.ts

@@ -111,6 +111,39 @@ adoptedPromise.catch(() => undefined);
 assert.equal(pendingEditorCallsForTest().length, 1, 'adopted call is queued');
 const adopted = pendingEditorCallsForTest()[0];
 assert.notEqual(adopted, undefined);
+assert.equal(cancelEditorCallsForOwner('owner-old-binding'), 1);
+await assert.rejects(adoptedPromise, hasOutcome('cancelled'));
+
+const currentBinding = editorBinding(projectId);
+assert(currentBinding);
+const ownerlessListPromise = invokeEditorTool(
+  'owner-after-broker-restart',
+  currentBinding,
+  'list_edit_sessions',
+  {},
+);
+const ownerlessListCall = await nextEditorCall(
+  projectId,
+  editorId,
+  currentBinding.baseRevision,
+  new AbortController().signal,
+  registrationCapability,
+);
+assert(ownerlessListCall);
+assert.equal(settleEditorCall(ownerlessListCall.id, 'applied', [{
+  editSessionId: 'persisted-ownerless-draft',
+  status: 'drafting',
+  stale: false,
+}], registrationCapability), true);
+assert.deepEqual(await ownerlessListPromise, [{
+  editSessionId: 'persisted-ownerless-draft',
+  status: 'drafting',
+  stale: false,
+  ownerOnline: false,
+  orphaned: true,
+  recoveryActions: ['resume', 'discard'],
+  ownedByCurrentTransport: false,
+}], 'a persisted draft without an in-memory transport owner is recoverable after broker restart');
 
 await import('./mcp.verify.ts');
 await import('./broker-poll-refresh.verify.ts');

server/external-agent/mcp-binding.ts

@@ -59,6 +59,7 @@ export function validateBrowserBinding(
   session: McpBindingSession,
   allowRevisionDrift = false,
   adoptSameIdentity = true,
+  requireSameRevisionForAdopt = false,
 ): EditorBinding | null {
   if (session.staleReason) throw new ExternalEditorCallError('stale', session.staleReason);
   if (!session.binding) return null;
@@ -75,6 +76,7 @@ export function validateBrowserBinding(
   if (adoptSameIdentity
     && current
     && sameEditorIdentity(current, session.binding)
+    && (!requireSameRevisionForAdopt || current.baseRevision === session.binding.baseRevision)
     && editorBindingMatches(current)) {
     session.binding = current;
     return current;

server/external-agent/mcp-check.verify.ts

@@ -1,5 +1,6 @@
 import assert from 'node:assert/strict';
 import { toMcpContent, toStructuredContent } from './mcp.ts';
+import { projectMcpReply } from './mcp-result.ts';
 
 const object = { ok: true };
 assert.equal(toStructuredContent(object), object);
@@ -30,5 +31,7 @@ assert.deepEqual(toMcpContent(imageResult), [
     mimeType: 'image/jpeg',
   },
 ]);
+const projectedImage = projectMcpReply(imageResult) as typeof imageResult;
+assert.equal(projectedImage.__images[0]!.base64, 'jpeg-data', 'MCP projection preserves image bytes');
 
 console.log('external-agent MCP structured content check passed');

server/external-agent/mcp-result.ts

@@ -26,7 +26,12 @@ function embeddedImages(result: unknown): EmbeddedImage[] {
 }
 
 export function projectMcpReply(value: unknown): unknown {
-  const sanitized = sanitizeJsonForArtifact(value);
+  const images = embeddedImages(value);
+  const source = images.length && value && typeof value === 'object' && !Array.isArray(value)
+    ? Object.fromEntries(Object.entries(value as Record<string, unknown>)
+      .filter(([key]) => key !== '__images'))
+    : value;
+  const sanitized = sanitizeJsonForArtifact(source);
   if (!sanitized) {
     throw new ExternalEditorCallError(
       'failed',
@@ -39,7 +44,10 @@ export function projectMcpReply(value: unknown): unknown {
       'The external result was too large and no recoverable artifact reference was available.',
     );
   }
-  return JSON.parse(sanitized.body);
+  const projected = JSON.parse(sanitized.body) as unknown;
+  return images.length && projected && typeof projected === 'object' && !Array.isArray(projected)
+    ? { ...projected as Record<string, unknown>, __images: images }
+    : projected;
 }
 
 export function mcpToolError(error: unknown): {

server/external-agent/mcp.ts

@@ -204,7 +204,8 @@ async function callTool(
   validateBrowserBinding(
     session,
     allowRevisionDrift,
-    MCP_CONTROL_TOOL_NAMES[name] !== true && !carriesSession,
+    MCP_CONTROL_TOOL_NAMES[name] !== true,
+    carriesSession,
   );
   const control = await callControlTool(session, name, args, baseUrl);
   if (control !== undefined) return control;
@@ -259,7 +260,7 @@ function makeServer(baseUrl: string, session: McpSession): Server {
         'Bind this MCP transport with target_project before editing. A connected browser is preferred; an existing stored project can use the offline fallback when no browser owns it.',
         'The target response and openchatcut_status report bindingMode. Offline bindings expose only server-direct data tools and require approvalMode="auto".',
         session.exposure.mode === 'progressive'
-          ? 'This client negotiated progressive tool exposure. Call ToolSearch or load_skill to reveal task tools; tools/list_changed is sent when the visible set grows.'
+          ? 'This client negotiated progressive tool exposure. Call ToolSearch for list_edit_sessions and recover_edit_session before session recovery; tools/list_changed is sent when the visible set grows.'
           : 'This client uses the compatibility tool surface. All currently available tools are listed.',
         'Call begin_edit_session first, pass editSessionId to every editor tool, then call review_edit_session. Do not claim success until status is applied.',
         'Manual approval and visual/canvas inspection, generation, upload, network, preset, render, and export tools require opening the returned editorUrl.',

server/external-agent/offline-project-store.verify.ts

@@ -251,6 +251,13 @@ try {
     serializedCommit.revision!,
   );
   assert.equal(browserTakeover.status, 'claimed');
+  const competingBrowser = await claimBrowserProjectOwnership(
+    projectId,
+    'browser-competing-owner',
+    serializedCommit.revision!,
+  );
+  assert.equal(competingBrowser.status, 'blocked',
+    'a second browser editor cannot silently replace a live browser lease');
   // A browser window re-claiming its OWN project (same ownerId, same revision)
   // recovers (claimed) even without a capability, because the route-level
   // capability check (broker capabilityMatches) already rejects forged/mismatched

server/external-agent/project-edit-ownership.ts

@@ -84,17 +84,10 @@ export async function claimBrowserProjectOwnership(
     // Genuine anti-spoof is enforced at the registration-route capability layer
     // (broker capabilityMatches / the route's renewing check), so this claim
     // gate only needs to fence cross-layer writes, not same-owner recovery.
-    // A genuinely DIFFERENT browser window that holds the expected revision also
-    // takes over from a previously registered browser window. Single-window
-    // desktop users never open the same project in two windows, so there is no
-    // cross-browser exclusivity to enforce. We still refuse to steal from a live
-    // OFFLINE writer (external MCP / a serialized offline commit) or an
-    // epoch-pinned owner, so the browser cannot clobber a non-browser write in
-    // flight. Lost-update protection additionally holds via the CAS revision
-    // match in createProjectDocumentStoreOperation.
-    if (current && current.leaseExpiresAt > Date.now() && current.ownerKind !== 'browser') {
-      return { status: 'blocked' };
-    }
+    // A different live browser/editor must never be silently replaced. Page
+    // refresh uses the same tab-scoped editor id and is covered by sameOwner;
+    // a second tab/window has a different id and must wait for lease release.
+    if (current && current.leaseExpiresAt > Date.now() && !sameOwner) return { status: 'blocked' };
     if (current && current.epoch === Number.MAX_SAFE_INTEGER) return { status: 'blocked' };
     const claim: ProjectEditOwnershipClaim = {
       projectId,