From 8a83a0f87e41ada3ce03ac82f0fc639f00068175 Mon Sep 17 00:00:00 2001
From: Charan Rathore <180254320+charan-rathore@users.noreply.github.com>
Date: Mon, 12 Oct 2026 05:33:48 +0530
Subject: [PATCH] Recover approval-gated actions interrupted before their
result is saved
A server stop between claimAction and finishAction bricked the
(threadId, toolCallId) pair: the result-less running row never expired,
every retry got a misleading 'already running' 409, and the card polled
that receipt forever with the Approve buttons hidden.
Tool calls are bounded to 60 seconds, so a result-less claim past
STALE_ACTION_MS (5 minutes) cannot belong to a live run. The receipt now
surfaces such a row as interrupted - the card explains the earlier run
may have completed and offers Approve again - and claimAction reclaims
the row atomically, same approval only, refreshing createdAt so a
concurrent reclaim loses. A fresh running row still 409s, a saved result
still returns only to its own approval, and a different approval still
gets the ownership 409 even for an interrupted row.
Fixes #134
Signed-off-by: Charan Rathore <180254320+charan-rathore@users.noreply.github.com>
---
src/client/ConnectionActionCard.tsx | 13 +++--
src/server/connection-routes.ts | 9 ++-
src/server/connection-store.ts | 53 +++++++++++++++---
tests/connections.test.ts | 87 +++++++++++++++++++++++++++++
4 files changed, 146 insertions(+), 16 deletions(-)
diff --git a/src/client/ConnectionActionCard.tsx b/src/client/ConnectionActionCard.tsx
index 3de99dc8..6501807d 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' | 'done' | 'interrupted';
result: ConnectionActionResult | null;
};
const conversation = (threadId: string) =>
@@ -69,6 +69,9 @@ 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';
+ // A run the server lost before saving its result: the tool may have
+ // completed remotely, so the card says so instead of looking unrun.
+ const interrupted = own?.status === 'interrupted';
useEffect(() => {
let active = true;
const load = () =>
@@ -234,9 +237,11 @@ export function ConnectionActionCard({
{running
? 'This action is still running on the server.'
- : approved || declined || finished
- ? ''
- : 'Nothing runs until you approve. These are the exact arguments.'}
+ : interrupted
+ ? 'The earlier run was interrupted before its result was saved. It may have completed - approve again only if it did not.'
+ : approved || declined || finished
+ ? ''
+ : 'Nothing runs until you approve. These are the exact arguments.'}
diff --git a/src/server/connection-routes.ts b/src/server/connection-routes.ts
index fd0d550d..b725acda 100644
--- a/src/server/connection-routes.ts
+++ b/src/server/connection-routes.ts
@@ -125,9 +125,12 @@ export function connectionRoutes(
{ error: 'This action belongs to a different approval request.' },
409,
);
- return previous.result
- ? c.json(previous.result)
- : c.json({ error: 'This action is already running.' }, 409);
+ if (previous.result) return c.json(previous.result);
+ // An interrupted run falls through so the same approval can reclaim it;
+ // a live one keeps guarding against a double run.
+ if (previous.status !== 'interrupted')
+ return c.json({ error: 'This action is already running.' }, 409);
+ return undefined;
};
const earlier = recovered();
if (earlier) return earlier;
diff --git a/src/server/connection-store.ts b/src/server/connection-store.ts
index bc53c1b7..9d8a7aef 100644
--- a/src/server/connection-store.ts
+++ b/src/server/connection-store.ts
@@ -9,6 +9,13 @@ type Row = Omit & {
token: string | null;
tools: string;
};
+/**
+ * Longest a claimed action may stay result-less before it counts as
+ * interrupted. Tool calls are bounded to 60 seconds, so a claim this old
+ * cannot belong to a live run.
+ */
+export const STALE_ACTION_MS = 5 * 60 * 1000;
+
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);
@@ -140,17 +147,28 @@ 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 FROM mcp_actions WHERE threadId=? AND toolCallId=?',
)
.get(threadId, toolCallId);
if (!row) return undefined;
+ const status = String(row.status) as 'running' | 'done';
+ const result =
+ typeof row.result === 'string'
+ ? (JSON.parse(row.result) as ConnectionActionResult)
+ : null;
+ // A result-less claim older than the longest possible tool call belongs
+ // to a server that died mid-run. Surface it as interrupted, not absent:
+ // the owner should know the earlier attempt may have happened before
+ // approving a replay. The claim path reclaims the row atomically, so a
+ // live run is never disturbed.
+ const interrupted =
+ status === 'running' &&
+ result === null &&
+ Number(row.createdAt) < Date.now() - STALE_ACTION_MS;
return {
approvalId: typeof row.approvalId === 'string' ? row.approvalId : null,
- status: String(row.status) as 'running' | 'done',
- result:
- typeof row.result === 'string'
- ? (JSON.parse(row.result) as ConnectionActionResult)
- : null,
+ status: interrupted ? ('interrupted' as const) : status,
+ result,
};
}
// Claim an approved action once. Returns false if it was already claimed,
@@ -162,13 +180,30 @@ export class ConnectionStore {
connectionId: string,
tool: string,
) {
+ const inserted = this.db
+ .prepare(
+ "INSERT OR IGNORE INTO mcp_actions (threadId, toolCallId, connectionId, tool, status, result, createdAt, approvalId) VALUES (?, ?, ?, ?, 'running', NULL, ?, ?)",
+ )
+ .run(threadId, toolCallId, connectionId, tool, Date.now(), approvalId);
+ if (inserted.changes > 0) return true;
+ // Reclaim an interrupted run: a result-less running row past the tool
+ // timeout, and only for the approval that claimed it. Refreshing
+ // createdAt makes a concurrent reclaim lose, so a fresh running row
+ // still guards against a double click.
return (
this.db
.prepare(
- "INSERT OR IGNORE INTO mcp_actions (threadId, toolCallId, connectionId, tool, status, result, createdAt, approvalId) VALUES (?, ?, ?, ?, 'running', NULL, ?, ?)",
+ "UPDATE mcp_actions SET createdAt=?, connectionId=?, tool=? WHERE threadId=? AND toolCallId=? AND approvalId=? AND status='running' AND result IS NULL AND createdAt < ?",
)
- .run(threadId, toolCallId, connectionId, tool, Date.now(), approvalId)
- .changes > 0
+ .run(
+ Date.now(),
+ connectionId,
+ tool,
+ threadId,
+ toolCallId,
+ approvalId,
+ Date.now() - STALE_ACTION_MS,
+ ).changes > 0
);
}
finishAction(
diff --git a/tests/connections.test.ts b/tests/connections.test.ts
index 80332791..08766d95 100644
--- a/tests/connections.test.ts
+++ b/tests/connections.test.ts
@@ -10,6 +10,7 @@ import {
type McpTransportFactory,
} from '../src/server/connections.js';
import { connectionTools } from '../src/server/connection-tools.js';
+import { STALE_ACTION_MS } from '../src/server/connection-store.js';
import { connectionRoutes } from '../src/server/connection-routes.js';
import type { Connection } from '../src/shared/connection-types.js';
const resources: (() => void)[] = [];
@@ -486,3 +487,89 @@ it('returns a saved result only for the approval that produced it', async () =>
expect(sent).toHaveBeenCalledOnce();
expect(sent.mock.calls[0][0]).toMatchObject({ to: 'alice@example.com' });
});
+
+it('reclaims an interrupted approval-gated run for the same approval only', async () => {
+ const { connections, dot, request, sent } = fixture();
+ await connections.add(dot.id, {
+ name: 'Mail',
+ url: 'https://mail.example.com/mcp',
+ });
+ const approvalId = await requestApproval(
+ connections,
+ dot.id,
+ 'mail__send_mail',
+ { to: 'a@example.com', body: 'Hi' },
+ );
+ // The server claimed the action and died before saving its result.
+ const store = connections.store;
+ expect(store.claimAction('thread', 'tc1', approvalId, 'c', 'tool')).toBe(
+ true,
+ );
+ const db = (store as unknown as { db: import('node:sqlite').DatabaseSync })
+ .db;
+ db.prepare('UPDATE mcp_actions SET createdAt=? WHERE toolCallId=?').run(
+ Date.now() - STALE_ACTION_MS - 1000,
+ 'tc1',
+ );
+ // The receipt surfaces the interruption, so the card can explain it and
+ // offer Approve again.
+ expect(
+ await (
+ await request('/conversations/thread/connection-actions/tc1')
+ ).json(),
+ ).toMatchObject({ status: 'interrupted' });
+ // A different approval still cannot touch the row, interrupted or not.
+ const other = await request('/conversations/thread/connection-actions', 'POST', {
+ toolCallId: 'tc1',
+ approvalId: 'not-the-approval',
+ });
+ expect(other.status).toBe(409);
+ expect(await other.json()).toEqual({
+ error: 'This action belongs to a different approval request.',
+ });
+ expect(sent).not.toHaveBeenCalled();
+ // The owning approval reclaims and runs the stored request.
+ const rerun = await request('/conversations/thread/connection-actions', 'POST', {
+ toolCallId: 'tc1',
+ approvalId,
+ });
+ expect(await rerun.json()).toEqual({ isError: false, text: 'Sent to a@example.com' });
+ expect(sent).toHaveBeenCalledOnce();
+ expect(
+ await (
+ await request('/conversations/thread/connection-actions/tc1')
+ ).json(),
+ ).toMatchObject({ status: 'done' });
+});
+it('a fresh result-less claim still guards against a double run', async () => {
+ const { connections, dot, request, sent } = fixture();
+ await connections.add(dot.id, {
+ name: 'Mail',
+ url: 'https://mail.example.com/mcp',
+ });
+ const approvalId = await requestApproval(
+ connections,
+ dot.id,
+ 'mail__send_mail',
+ { to: 'a@example.com', body: 'Hi' },
+ );
+ const store = connections.store;
+ expect(store.claimAction('thread', 'tc1', approvalId, 'c', 'tool')).toBe(
+ true,
+ );
+ // Claimed seconds ago: this is a live run, not an interrupted one.
+ expect(
+ await (
+ await request('/conversations/thread/connection-actions/tc1')
+ ).json(),
+ ).toMatchObject({ status: 'running' });
+ expect(store.claimAction('thread', 'tc1', approvalId, 'c', 'tool')).toBe(
+ false,
+ );
+ const again = await request('/conversations/thread/connection-actions', 'POST', {
+ toolCallId: 'tc1',
+ approvalId,
+ });
+ expect(again.status).toBe(409);
+ expect(sent).not.toHaveBeenCalled();
+});