Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 18 additions & 10 deletions src/client/ConnectionActionCard.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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) =>
Expand Down Expand Up @@ -68,7 +68,9 @@ export function ConnectionActionCard({
const mismatch = !!receipt && receipt.approvalId !== approvalId;
const own = mismatch ? null : receipt;
// A receipt that is still running is checked until the server finishes.
// An interrupted receipt waits for the owner to review and retry instead.
const running = own?.status === 'running';
const interrupted = own?.status === 'interrupted';
useEffect(() => {
let active = true;
const load = () =>
Expand Down Expand Up @@ -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">
Expand Down Expand Up @@ -218,7 +222,9 @@ export function ConnectionActionCard({
? 'Running…'
: outcome
? 'Continue conversation'
: 'Approve & run'}
: interrupted
? 'Retry action'
: 'Approve & run'}
</button>
{!outcome && (
<button
Expand All @@ -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
? 'The previous attempt was interrupted before it finished. Review the arguments, then retry to run it again.'
: approved || declined || finished
? ''
: 'Nothing runs until you approve. These are the exact arguments.'}
</small>
</footer>
</section>
Expand Down
21 changes: 19 additions & 2 deletions src/server/connection-routes.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { Hono } from 'hono';
import { z } from 'zod';
import {
ACTION_TTL_MS,
APPROVAL_TTL_MS,
connectionActionSchema,
type PendingApproval,
Expand Down Expand Up @@ -103,9 +104,21 @@ export function connectionRoutes(
});
app.get('/conversations/:id/connection-actions/:toolCallId', (c) => {
const thread = workspace.requireThread(c.req.param('id'));
return c.json(
connections.store.action(thread.id, c.req.param('toolCallId')) ?? null,
const receipt = connections.store.action(
thread.id,
c.req.param('toolCallId'),
);
if (!receipt) return c.json(null);
return c.json({
approvalId: receipt.approvalId,
status:
receipt.status === 'running' &&
!receipt.result &&
Date.now() - receipt.createdAt > ACTION_TTL_MS
? 'interrupted'
: receipt.status,
result: receipt.result,
});
});
// The only path that runs an approval-gated tool: the owner approving a
// stored request. It runs that request's connection, tool, and arguments,
Expand All @@ -117,6 +130,8 @@ export function connectionRoutes(
.parse(await c.req.json());
const thread = workspace.requireThread(c.req.param('id'));
// A saved result is returned only for the approval that produced it.
// A result-less claim older than the action TTL was interrupted before
// it could finish, so the same approval may reclaim it below.
const recovered = () => {
const previous = connections.store.action(thread.id, body.toolCallId);
if (!previous) return undefined;
Expand All @@ -125,6 +140,8 @@ export function connectionRoutes(
{ error: 'This action belongs to a different approval request.' },
409,
);
if (!previous.result && Date.now() - previous.createdAt > ACTION_TTL_MS)
return undefined;
return previous.result
? c.json(previous.result)
: c.json({ error: 'This action is already running.' }, 409);
Expand Down
33 changes: 28 additions & 5 deletions src/server/connection-store.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import type { DatabaseSync } from 'node:sqlite';
import { randomUUID } from 'node:crypto';
import { ACTION_TTL_MS } from '../shared/connection-types.js';
import type {
Connection,
ConnectionActionResult,
Expand Down Expand Up @@ -140,7 +141,7 @@ 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;
Expand All @@ -151,24 +152,46 @@ export class ConnectionStore {
typeof row.result === 'string'
? (JSON.parse(row.result) as ConnectionActionResult)
: null,
createdAt: Number(row.createdAt),
};
}
// Claim an approved action once. Returns false if it was already claimed,
// so a double click or a retried request never executes twice.
// so a double click or a retried request never executes twice. A
// result-less claim older than `ttlMs` was interrupted before it could
// finish, so it is reclaimed atomically: the UPDATE only matches a row no
// live run owns, and concurrent reclaimers serialize on the write.
claimAction(
threadId: string,
toolCallId: string,
approvalId: string,
connectionId: string,
tool: string,
ttlMs = ACTION_TTL_MS,
) {
return (
const now = Date.now();
if (
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)
.run(threadId, toolCallId, connectionId, tool, now, approvalId)
.changes > 0
)
return true;
return (
this.db
.prepare(
"UPDATE mcp_actions SET connectionId=?, tool=?, status='running', createdAt=?, approvalId=? WHERE threadId=? AND toolCallId=? AND status='running' AND result IS NULL AND createdAt<=?",
)
.run(
connectionId,
tool,
now,
approvalId,
threadId,
toolCallId,
now - ttlMs,
).changes > 0
);
}
finishAction(
Expand All @@ -178,7 +201,7 @@ export class ConnectionStore {
) {
this.db
.prepare(
"UPDATE mcp_actions SET status='done', result=? WHERE threadId=? AND toolCallId=?",
"UPDATE mcp_actions SET status='done', result=? WHERE threadId=? AND toolCallId=? AND result IS NULL",
)
.run(JSON.stringify(result), threadId, toolCallId);
}
Expand Down
3 changes: 3 additions & 0 deletions src/shared/connection-types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,9 @@ export interface PendingApproval {
expiresAt: number;
}
export const APPROVAL_TTL_MS = 60 * 60_000;
// A claimed action settles in about a minute (one bounded MCP call), so a
// result-less claim older than this was interrupted before it could finish.
export const ACTION_TTL_MS = 10 * 60_000;
export const connectionActionSchema = z
.object({
approvalId: z.string().min(1).max(64),
Expand Down
119 changes: 119 additions & 0 deletions tests/connections.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
} from '../src/server/connections.js';
import { connectionTools } from '../src/server/connection-tools.js';
import { connectionRoutes } from '../src/server/connection-routes.js';
import { ACTION_TTL_MS } from '../src/shared/connection-types.js';
import type { Connection } from '../src/shared/connection-types.js';
const resources: (() => void)[] = [];
afterEach(() => resources.splice(0).forEach((close) => close()));
Expand Down Expand Up @@ -486,3 +487,121 @@ 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('lets the owner retry an action interrupted before its result was saved', async () => {
const { dot, connections, 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 approval = connections.store.approval(approvalId)!;
// A crash between claim and finish leaves a result-less running row.
expect(
connections.store.claimAction(
'thread',
'tc1',
approvalId,
approval.connectionId,
approval.tool,
),
).toBe(true);
// A fresh claim still belongs to its in-flight run: retries wait.
const busy = await request(
'/conversations/thread/connection-actions',
'POST',
{
toolCallId: 'tc1',
approvalId,
},
);
expect(busy.status).toBe(409);
expect(await busy.json()).toEqual({
error: 'This action is already running.',
});
expect(sent).not.toHaveBeenCalled();
vi.useFakeTimers({ toFake: ['Date'] });
try {
vi.setSystemTime(Date.now() + ACTION_TTL_MS + 1);
// The receipt names the interruption so the card can offer a retry...
const receipt = await request(
'/conversations/thread/connection-actions/tc1',
);
expect(await receipt.json()).toMatchObject({
approvalId,
status: 'interrupted',
});
// ...and retrying the same approval runs the stored request exactly once.
const retried = await request(
'/conversations/thread/connection-actions',
'POST',
{ toolCallId: 'tc1', approvalId },
);
expect(retried.status).toBe(200);
expect(await retried.json()).toEqual({
isError: false,
text: 'Sent to a@example.com',
});
expect(sent).toHaveBeenCalledOnce();
expect(sent.mock.calls[0][0]).toEqual({
to: 'a@example.com',
body: 'Hi',
});
expect(
await (
await request('/conversations/thread/connection-actions/tc1')
).json(),
).toMatchObject({ status: 'done' });
} finally {
vi.useRealTimers();
}
});
it('still refuses a stale claim under a different approval', async () => {
const { dot, connections, request, sent } = fixture();
await connections.add(dot.id, {
name: 'Mail',
url: 'https://mail.example.com/mcp',
});
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',
});
const approval = connections.store.approval(alice)!;
expect(
connections.store.claimAction(
'thread',
'tc1',
alice,
approval.connectionId,
approval.tool,
),
).toBe(true);
vi.useFakeTimers({ toFake: ['Date'] });
try {
vi.setSystemTime(Date.now() + ACTION_TTL_MS + 1);
// Interruption never hands Alice's claim to Bob's approval.
const mismatched = await request(
'/conversations/thread/connection-actions',
'POST',
{ toolCallId: 'tc1', approvalId: bob },
);
expect(mismatched.status).toBe(409);
expect(await mismatched.json()).toEqual({
error: 'This action belongs to a different approval request.',
});
expect(sent).not.toHaveBeenCalled();
} finally {
vi.useRealTimers();
}
});