From 237fddc91f49080c0689c9b3e65f6d4b61f30f8c Mon Sep 17 00:00:00 2001
From: Twil3akine <156310846+Twil3akine@users.noreply.github.com>
Date: Thu, 15 Oct 2026 10:41:58 +0900
Subject: [PATCH] fix: recover interrupted approval-gated MCP actions

---
 src/client/ConnectionActionCard.tsx |  28 ++++--
 src/server/connection-routes.ts     |  29 +++++-
 src/server/connection-store.ts      |  59 +++++++++---
 tests/connection-card.test.tsx      |  61 ++++++++++++-
 tests/connections.test.ts           | 137 ++++++++++++++++++++++++++++
 5 files changed, 286 insertions(+), 28 deletions(-)

diff --git a/src/client/ConnectionActionCard.tsx b/src/client/ConnectionActionCard.tsx
index 3de99dc8..bbf3c275 100644
--- a/src/client/ConnectionActionCard.tsx
+++ b/src/client/ConnectionActionCard.tsx
@@ -9,7 +9,7 @@ import { api } from './api';
 import { computerToolResult } from './ComputerToolCard';
 type Receipt = {
   approvalId: string | null;
-  status: 'running' | 'done';
+  status: 'running' | 'interrupted' | 'done';
   result: ConnectionActionResult | null;
 };
 const conversation = (threadId: string) =>
@@ -69,6 +69,7 @@ export function ConnectionActionCard({
   const own = mismatch ? null : receipt;
   // A receipt that is still running is checked until the server finishes.
   const running = own?.status === 'running';
+  const interrupted = own?.status === 'interrupted';
   useEffect(() => {
     let active = true;
     const load = () =>
@@ -110,6 +111,7 @@ export function ConnectionActionCard({
         });
         return;
       }
+      if (!outcome) setReceipt({ approvalId, status: 'running', result: null });
       // A previous approval may have run even if its response never arrived.
       const value =
         outcome ??
@@ -151,11 +153,13 @@ export function ConnectionActionCard({
               ? 'Declined'
               : running
                 ? 'Running'
-                : finished
-                  ? 'Ended'
-                  : !ready
-                    ? 'Checking'
-                    : 'Needs your approval'}
+                : interrupted
+                  ? 'Interrupted'
+                  : finished
+                    ? 'Ended'
+                    : !ready
+                      ? 'Checking'
+                      : 'Needs your approval'}
         </span>
       </header>
       <div className="page-review-body">
@@ -218,7 +222,9 @@ export function ConnectionActionCard({
                 ? 'Running…'
                 : outcome
                   ? 'Continue conversation'
-                  : 'Approve & run'}
+                  : interrupted
+                    ? 'Retry & run'
+                    : 'Approve & run'}
             </button>
             {!outcome && (
               <button
@@ -234,9 +240,11 @@ export function ConnectionActionCard({
         <small>
           {running
             ? 'This action is still running on the server.'
-            : approved || declined || finished
-              ? ''
-              : 'Nothing runs until you approve. These are the exact arguments.'}
+            : interrupted
+              ? 'No result was saved for the previous run. Retrying can repeat the action if the service completed it before the interruption.'
+              : approved || declined || finished
+                ? ''
+                : 'Nothing runs until you approve. These are the exact arguments.'}
         </small>
       </footer>
     </section>
diff --git a/src/server/connection-routes.ts b/src/server/connection-routes.ts
index d15669de..995c8c88 100644
--- a/src/server/connection-routes.ts
+++ b/src/server/connection-routes.ts
@@ -125,12 +125,14 @@ export function connectionRoutes(
           { error: 'This action belongs to a different approval request.' },
           409,
         );
-      return previous.result
-        ? c.json(previous.result)
+      if (previous.result) return c.json(previous.result);
+      return previous.status === 'interrupted'
+        ? undefined
         : c.json({ error: 'This action is already running.' }, 409);
     };
     const earlier = recovered();
     if (earlier) return earlier;
+    const previous = connections.store.action(thread.id, body.toolCallId);
     const { approval, exposed } = approvalFor(thread.id, body.approvalId);
     // An approval runs its stored request once. The claim below binds one
     // tool call id, so without this check the same approval could execute
@@ -141,7 +143,21 @@ export function connectionRoutes(
         { error: 'This approval request already ran its stored action.' },
         409,
       );
-    if (
+    if (previous) {
+      // Only the same approval can reclaim its interrupted action. The store's
+      // conditional update also prevents two retries from starting together.
+      if (
+        !connections.store.reclaimInterruptedAction(
+          thread.id,
+          body.toolCallId,
+          approval.id,
+        )
+      )
+        return (
+          recovered() ??
+          c.json({ error: 'This action is already running.' }, 409)
+        );
+    } else if (
       !connections.store.claimAction(
         thread.id,
         body.toolCallId,
@@ -149,8 +165,11 @@ export function connectionRoutes(
         exposed.connection.id,
         exposed.tool.name,
       )
-    )
-      return recovered()!;
+    ) {
+      return (
+        recovered() ?? c.json({ error: 'This action is already running.' }, 409)
+      );
+    }
     // Not tied to the request: once approved, finish even if the tab closes.
     const result = await connections.call(exposed, approval.arguments);
     connections.store.finishAction(thread.id, body.toolCallId, result);
diff --git a/src/server/connection-store.ts b/src/server/connection-store.ts
index 2fc3908d..9ce7c8f1 100644
--- a/src/server/connection-store.ts
+++ b/src/server/connection-store.ts
@@ -9,19 +9,26 @@ type Row = Omit<Connection, 'hasToken' | 'tools'> & {
   token: string | null;
   tools: string;
 };
+// MCP calls are bounded to 60 seconds. Keep a generous margin before treating
+// a result-less claim as interrupted, so a slow but live call cannot be run twice.
+export const INTERRUPTED_ACTION_THRESHOLD_MS = 5 * 60_000;
+
 export class ConnectionStore {
   constructor(private db: DatabaseSync) {
     db.exec(`CREATE TABLE IF NOT EXISTS mcp_connections(id TEXT PRIMARY KEY, dotId TEXT NOT NULL, name TEXT NOT NULL, url TEXT NOT NULL, token TEXT, tools TEXT NOT NULL, error TEXT, createdAt INTEGER NOT NULL, updatedAt INTEGER NOT NULL);
       CREATE TABLE IF NOT EXISTS mcp_approvals(id TEXT PRIMARY KEY, threadId TEXT NOT NULL, dotId TEXT NOT NULL, connectionId TEXT NOT NULL, tool TEXT NOT NULL, arguments TEXT NOT NULL, createdAt INTEGER NOT NULL);
-      CREATE TABLE IF NOT EXISTS mcp_actions(threadId TEXT NOT NULL, toolCallId TEXT NOT NULL, connectionId TEXT NOT NULL, tool TEXT NOT NULL, status TEXT NOT NULL, result TEXT, createdAt INTEGER NOT NULL, PRIMARY KEY(threadId, toolCallId));`);
+      CREATE TABLE IF NOT EXISTS mcp_actions(threadId TEXT NOT NULL, toolCallId TEXT NOT NULL, connectionId TEXT NOT NULL, tool TEXT NOT NULL, status TEXT NOT NULL, result TEXT, createdAt INTEGER NOT NULL, claimedAt INTEGER, approvalId TEXT, PRIMARY KEY(threadId, toolCallId));`);
     // A receipt belongs to the approval that produced it.
-    if (
-      !db
-        .prepare('PRAGMA table_info(mcp_actions)')
-        .all()
-        .some((column) => column.name === 'approvalId')
-    )
+    const actionColumns = db
+      .prepare('PRAGMA table_info(mcp_actions)')
+      .all()
+      .map((column) => column.name);
+    if (!actionColumns.includes('approvalId'))
       db.exec('ALTER TABLE mcp_actions ADD COLUMN approvalId TEXT');
+    // claimedAt tracks the most recent execution attempt. Older rows fall
+    // back to createdAt, which was their original claim time.
+    if (!actionColumns.includes('claimedAt'))
+      db.exec('ALTER TABLE mcp_actions ADD COLUMN claimedAt INTEGER');
   }
   private rows(dotId?: string) {
     return this.db
@@ -140,13 +147,20 @@ export class ConnectionStore {
   action(threadId: string, toolCallId: string) {
     const row = this.db
       .prepare(
-        'SELECT status, result, approvalId FROM mcp_actions WHERE threadId=? AND toolCallId=?',
+        'SELECT status, result, approvalId, createdAt, claimedAt FROM mcp_actions WHERE threadId=? AND toolCallId=?',
       )
       .get(threadId, toolCallId);
     if (!row) return undefined;
+    const claimedAt = Number(row.claimedAt ?? row.createdAt);
+    const status = String(row.status) as 'running' | 'done';
     return {
       approvalId: typeof row.approvalId === 'string' ? row.approvalId : null,
-      status: String(row.status) as 'running' | 'done',
+      status:
+        status === 'running' &&
+        row.result === null &&
+        Date.now() - claimedAt >= INTERRUPTED_ACTION_THRESHOLD_MS
+          ? ('interrupted' as const)
+          : status,
       result:
         typeof row.result === 'string'
           ? (JSON.parse(row.result) as ConnectionActionResult)
@@ -174,15 +188,38 @@ export class ConnectionStore {
     connectionId: string,
     tool: string,
   ) {
+    const now = Date.now();
     return (
       this.db
         .prepare(
-          "INSERT OR IGNORE INTO mcp_actions (threadId, toolCallId, connectionId, tool, status, result, createdAt, approvalId) VALUES (?, ?, ?, ?, 'running', NULL, ?, ?)",
+          "INSERT OR IGNORE INTO mcp_actions (threadId, toolCallId, connectionId, tool, status, result, createdAt, claimedAt, approvalId) VALUES (?, ?, ?, ?, 'running', NULL, ?, ?, ?)",
         )
-        .run(threadId, toolCallId, connectionId, tool, Date.now(), approvalId)
+        .run(threadId, toolCallId, connectionId, tool, now, now, approvalId)
         .changes > 0
     );
   }
+  // Reclaim only this approval's stale, result-less claim. The conditional
+  // update makes concurrent retries race safely: only one can refresh claimedAt.
+  reclaimInterruptedAction(
+    threadId: string,
+    toolCallId: string,
+    approvalId: string,
+  ) {
+    const now = Date.now();
+    return (
+      this.db
+        .prepare(
+          "UPDATE mcp_actions SET claimedAt=? WHERE threadId=? AND toolCallId=? AND approvalId=? AND status='running' AND result IS NULL AND COALESCE(claimedAt, createdAt)<=?",
+        )
+        .run(
+          now,
+          threadId,
+          toolCallId,
+          approvalId,
+          now - INTERRUPTED_ACTION_THRESHOLD_MS,
+        ).changes > 0
+    );
+  }
   finishAction(
     threadId: string,
     toolCallId: string,
diff --git a/tests/connection-card.test.tsx b/tests/connection-card.test.tsx
index e673cfa6..dd5b2ae9 100644
--- a/tests/connection-card.test.tsx
+++ b/tests/connection-card.test.tsx
@@ -1,7 +1,25 @@
-import { expect, it, vi } from 'vitest';
+import { createElement } from 'react';
+import { act, create } from 'react-test-renderer';
+import { beforeEach, expect, it, vi } from 'vitest';
 import { renderToStaticMarkup } from 'react-dom/server';
-vi.mock('../src/client/api', () => ({ api: vi.fn() }));
+const api = vi.hoisted(() => vi.fn());
+vi.mock('../src/client/api', () => ({ api }));
 import { ConnectionActionCard } from '../src/client/ConnectionActionCard';
+
+const text = (node: unknown): string =>
+  typeof node === 'string'
+    ? node
+    : Array.isArray(node)
+      ? node.map(text).join('')
+      : node && typeof node === 'object' && 'children' in node
+        ? text((node as { children: unknown }).children)
+        : '';
+
+beforeEach(() => {
+  api.mockReset();
+  vi.stubGlobal('IS_REACT_ACT_ENVIRONMENT', true);
+});
+
 it('shows the summary and waits for the server record before deciding', () => {
   const html = renderToStaticMarkup(
     <ConnectionActionCard
@@ -20,3 +38,42 @@ it('shows the summary and waits for the server record before deciding', () => {
   // Decisions wait until the server confirms the action has not already run.
   expect(html).not.toContain('Approve &amp; run');
 });
+
+it('offers an explicit retry when the server marks a claim interrupted', async () => {
+  api.mockImplementation(async (path: string) =>
+    path.includes('/connection-approvals/')
+      ? {
+          id: 'approval-1',
+          connection: 'Mail',
+          title: 'send mail!',
+          description: 'Send an email.',
+          arguments: { to: 'a@example.com', body: 'Hi' },
+          expiresAt: Date.now() + 60_000,
+        }
+      : { approvalId: 'approval-1', status: 'interrupted', result: null },
+  );
+  const respond = vi.fn(async () => {});
+  let root!: ReturnType<typeof create>;
+  await act(async () => {
+    root = create(
+      createElement(ConnectionActionCard, {
+        args: { approvalId: 'approval-1', summary: 'Send an email.' },
+        status: 'executing',
+        respond,
+        threadId: 'thread',
+        toolCallId: 'call',
+      }),
+    );
+  });
+  const view = text(root.toJSON());
+  expect(view).toContain('Interrupted');
+  expect(view).toContain('No result was saved for the previous run.');
+  expect(view).toContain('Retrying can repeat the action');
+  const retry = root.root.findAll(
+    (node) =>
+      node.type === 'button' && text(node.props.children) === 'Retry & run',
+  )[0];
+  expect(retry.props.disabled).toBe(false);
+  expect(respond).not.toHaveBeenCalled();
+  await act(async () => root.unmount());
+});
diff --git a/tests/connections.test.ts b/tests/connections.test.ts
index f07cb0e2..50266504 100644
--- a/tests/connections.test.ts
+++ b/tests/connections.test.ts
@@ -11,7 +11,12 @@ import {
 } from '../src/server/connections.js';
 import { connectionTools } from '../src/server/connection-tools.js';
 import { connectionRoutes } from '../src/server/connection-routes.js';
+import {
+  ConnectionStore,
+  INTERRUPTED_ACTION_THRESHOLD_MS,
+} from '../src/server/connection-store.js';
 import type { Connection } from '../src/shared/connection-types.js';
+import { DatabaseSync } from 'node:sqlite';
 const resources: (() => void)[] = [];
 afterEach(() => resources.splice(0).forEach((close) => close()));
 function fixture({ twin = false } = {}) {
@@ -487,6 +492,138 @@ it('returns a saved result only for the approval that produced it', async () =>
   expect(sent.mock.calls[0][0]).toMatchObject({ to: 'alice@example.com' });
 });
 
+it('surfaces stale running claims and lets only their approval retry', async () => {
+  const { dot, connections, request, sent } = fixture();
+  const connection = await connections.add(dot.id, {
+    name: 'Mail',
+    url: 'https://mail.example.com/mcp',
+  });
+  vi.useFakeTimers({ toFake: ['Date'] });
+  try {
+    const startedAt = Date.now();
+    const alice = await requestApproval(
+      connections,
+      dot.id,
+      'mail__send_mail',
+      { to: 'alice@example.com', body: 'Hi' },
+    );
+    const bob = await requestApproval(connections, dot.id, 'mail__send_mail', {
+      to: 'bob@example.com',
+      body: 'Hi',
+    });
+    expect(
+      connections.store.claimAction(
+        'thread',
+        'tc1',
+        alice,
+        connection.id,
+        'send mail!',
+      ),
+    ).toBe(true);
+    const approve = (approvalId: string) =>
+      request('/conversations/thread/connection-actions', 'POST', {
+        toolCallId: 'tc1',
+        approvalId,
+      });
+
+    const running = await request(
+      '/conversations/thread/connection-actions/tc1',
+    );
+    expect(await running.json()).toMatchObject({
+      approvalId: alice,
+      status: 'running',
+      result: null,
+    });
+    const tooSoon = await approve(alice);
+    expect(tooSoon.status).toBe(409);
+    expect(await tooSoon.json()).toEqual({
+      error: 'This action is already running.',
+    });
+
+    const stolen = await approve(bob);
+    expect(stolen.status).toBe(409);
+    expect(await stolen.json()).toEqual({
+      error: 'This action belongs to a different approval request.',
+    });
+    expect(sent).not.toHaveBeenCalled();
+
+    vi.setSystemTime(startedAt + INTERRUPTED_ACTION_THRESHOLD_MS - 1);
+    const stillRunning = await request(
+      '/conversations/thread/connection-actions/tc1',
+    );
+    expect(await stillRunning.json()).toMatchObject({ status: 'running' });
+
+    vi.setSystemTime(startedAt + INTERRUPTED_ACTION_THRESHOLD_MS);
+    const interrupted = await request(
+      '/conversations/thread/connection-actions/tc1',
+    );
+    expect(await interrupted.json()).toMatchObject({
+      approvalId: alice,
+      status: 'interrupted',
+      result: null,
+    });
+
+    const retries = await Promise.all([approve(alice), approve(alice)]);
+    expect(retries.filter((response) => response.status === 200)).toHaveLength(
+      1,
+    );
+    expect(retries.filter((response) => response.status === 409)).toHaveLength(
+      1,
+    );
+    expect(
+      await retries.find((response) => response.status === 200)!.json(),
+    ).toEqual({
+      isError: false,
+      text: 'Sent to alice@example.com',
+    });
+    const mismatched = await approve(bob);
+    expect(mismatched.status).toBe(409);
+    expect(await mismatched.json()).toEqual({
+      error: 'This action belongs to a different approval request.',
+    });
+    const completed = await request(
+      '/conversations/thread/connection-actions/tc1',
+    );
+    expect(await completed.json()).toMatchObject({
+      approvalId: alice,
+      status: 'done',
+      result: { text: 'Sent to alice@example.com' },
+    });
+    expect(sent).toHaveBeenCalledOnce();
+  } finally {
+    vi.useRealTimers();
+  }
+});
+
+it('migrates existing action receipts with their original claim time', () => {
+  const db = new DatabaseSync(':memory:');
+  try {
+    db.exec(`CREATE TABLE mcp_actions(
+      threadId TEXT NOT NULL,
+      toolCallId TEXT NOT NULL,
+      connectionId TEXT NOT NULL,
+      tool TEXT NOT NULL,
+      status TEXT NOT NULL,
+      result TEXT,
+      createdAt INTEGER NOT NULL,
+      approvalId TEXT,
+      PRIMARY KEY(threadId, toolCallId)
+    );`);
+    db.prepare(
+      "INSERT INTO mcp_actions VALUES ('thread', 'tc1', 'connection', 'send mail!', 'running', NULL, ?, 'approval-1')",
+    ).run(Date.now() - INTERRUPTED_ACTION_THRESHOLD_MS);
+
+    const store = new ConnectionStore(db);
+    expect(store.action('thread', 'tc1')).toMatchObject({
+      approvalId: 'approval-1',
+      status: 'interrupted',
+      result: null,
+    });
+  } finally {
+    db.close();
+  }
+});
+
 it('runs an approved request once, even under a fresh toolCallId', async () => {
   const { dot, connections, request, sent } = fixture();
   await connections.add(dot.id, {