Skip to content
Merged
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
52 changes: 52 additions & 0 deletions server/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,16 @@ beforeAll(async () => {
unread: false,
createdAt: 3,
},
{
id: "test-cancel-room",
threadId: "test-cancel-room-thread",
name: "Cancel room",
memberIds: ["test-bot-a"],
defaultResponder: { kind: "member", botId: "test-bot-a" },
bulletin: "",
unread: false,
createdAt: 4,
},
{
id: "test-pinned-room",
threadId: "test-pinned-room-thread",
Expand Down Expand Up @@ -133,6 +143,33 @@ beforeAll(async () => {
}),
);

// A room holding an approval nobody has answered yet, so "Cancel turn"
// has something open to close.
writeFileSync(
join(home, ".openmausbot", "messages-test-cancel-room-thread.json"),
JSON.stringify({
activeLeafId: "cancel-card",
messages: [
{
id: "cancel-card",
at: 4,
parentId: null,
role: "bot",
kind: "options",
card: {
title: "Approval needed",
subtitle: "rm -rf /tmp/scratch",
options: ["Allow", "Deny"],
requestId: "cancel-request",
tool: "Bash",
allowKey: "Bash:rm",
},
from: { botId: "test-bot-a", name: "Test bot A", color: "purple" },
},
],
}),
);

boxStub = createServer(async (req, res) => {
if (req.url?.startsWith("/api/v3.1/tool_router/session")) {
if (req.headers["x-api-key"] !== "ak_good") {
Expand Down Expand Up @@ -795,6 +832,21 @@ describe("harness HTTP API", () => {
expect(nothing.status).toBe(404);
});

it("closes the approvals a cancelled turn can no longer answer", async () => {
// "Cancel turn" is a button ON the approval card, and a pending approval
// owns the composer. Stopping the turn without closing its card leaves the
// room blocked by a question whose asker is already gone.
const stopped = await api("POST", "/api/groups/test-cancel-room/interrupt");
expect(stopped.status).toBe(200);

const room = (await api("GET", "/api/bots")).body.groups.find(
(group: { id: string }) => group.id === "test-cancel-room",
);
const card = room.messages.find((message: { id: string }) => message.id === "cancel-card").card;
expect(card.dismissed).toBe(true);
expect(card.answered).toBe("unavailable");
});

it("rejects an empty message and explains an unavailable provider", async () => {
const { body } = await api("GET", "/api/bots");
const bot = body.bots[0];
Expand Down
27 changes: 24 additions & 3 deletions server/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ import { _loadPending, discardDelegations, drainDelegations, pendingThreads, que
import { drainSteeredMessages, queueSteeredMessage } from "./steer-queue.ts";
import { EventBus } from "./harness/bus.ts";
import { ProviderRegistry } from "./harness/registry.ts";
import { cancelPeerApprovalsFor, dismissStalePeerCards, requestPeerApproval, resolvePeerComms, type ApprovalBus } from "./peer-approval.ts";
import { cancelPeerApprovalsFor, cancelPeerApprovalsForThread, dismissStalePeerCards, requestPeerApproval, resolvePeerComms, type ApprovalBus } from "./peer-approval.ts";
import {
mentionedBots,
roomResponders,
Expand Down Expand Up @@ -488,6 +488,22 @@ async function answerRequest(
return outcome;
}

/** Close every approval still open on a thread. Interrupting a turn kills the
* process that raised its questions, so those cards can never be answered —
* and a pending approval owns the composer, so one left open blocks the
* conversation behind a question with nobody left to hear the answer. */
function closeOpenApprovals(threadId: string): void {
// Peer approvals also hold an in-memory promise. Resolve those first; merely
// patching their cards would leave the delegation queue waiting 15 minutes.
cancelPeerApprovalsForThread(threadId);
for (const message of store.messagesFor(threadId)) {
const card = message.card;
if (!card?.requestId || card.answered || card.dismissed) continue;
store.patchMessage(threadId, message.id, { card: { ...card, answered: "unavailable", dismissed: true } });
askMessageByRequest.delete(`${threadId}:${card.requestId}`);
}
}

function requestBehavior(value: unknown): "allow" | "deny" | "answer" | null {
return value === "allow" || value === "deny" || value === "answer" ? value : null;
}
Expand Down Expand Up @@ -2976,6 +2992,7 @@ const server = createServer(async (req, res) => {
const busy = group.busyBotId ? store.bot(group.busyBotId) : undefined;
const instance = busy ? registry.get(busy.modelSelection.instanceId) : undefined;
await instance?.adapter.interruptTurn(group.threadId).catch(() => {});
closeOpenApprovals(group.threadId);
return json(res, 200, { ok: true });
}

Expand Down Expand Up @@ -3385,8 +3402,12 @@ const server = createServer(async (req, res) => {
// a bot busy in a ROOM is running on the room's thread — stopping it
// from its own chat must reach that turn, not just the 1:1 thread
const busyGroup = store.groups.find((g) => g.busyBotId === bot.id);
if (busyGroup) await instance?.adapter.interruptTurn(busyGroup.threadId).catch(() => {});
await instance?.adapter.interruptTurn(bot.threadId);
if (busyGroup) {
await instance?.adapter.interruptTurn(busyGroup.threadId).catch(() => {});
closeOpenApprovals(busyGroup.threadId);
}
await instance?.adapter.interruptTurn(bot.threadId).catch(() => {});
closeOpenApprovals(bot.threadId);
return json(res, 200, { ok: true });
}

Expand Down
14 changes: 14 additions & 0 deletions server/peer-approval.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { DATA_DIR } from "./config.ts";
import type { ModelSelection } from "./contracts.ts";
import {
cancelPeerApprovalsFor,
cancelPeerApprovalsForThread,
dismissStalePeerCards,
peerAllowKey,
requestPeerApproval,
Expand Down Expand Up @@ -100,6 +101,19 @@ describe("peer approval card lifecycle", () => {
expect(settled?.card?.dismissed).toBe(true); // not the user's answer
});

it("denies and settles approvals owned by an interrupted thread", async () => {
const verdict = requestPeerApproval(bus, from, target, "ping", "ask_bot");
const card = pendingCard(store, from)!;

cancelPeerApprovalsForThread(from.threadId);

expect(await verdict).toBe("deny");
const settled = store.messagesFor(from.threadId).find((m) => m.id === card.id);
expect(settled?.card?.answered).toBe("deny");
expect(settled?.card?.dismissed).toBe(true);
expect(pendingCard(store, from)).toBeUndefined();
});

it("dismisses cards left by a previous run, which nothing can answer", () => {
// a card on disk whose in-memory approval died with the process
const orphan = store.appendMessage(from.threadId, {
Expand Down
13 changes: 13 additions & 0 deletions server/peer-approval.ts
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,19 @@ export function cancelPeerApprovalsFor(botId: string): void {
}
}

/** Deny every peer-communication approval owned by a thread whose turn was
* interrupted. Patching the card alone is not enough: the in-memory promise
* must resolve too, or the delegation queue waits until its 15-minute timer. */
export function cancelPeerApprovalsForThread(threadId: string): void {
for (const [requestId, pending] of pendingComms) {
if (pending.threadId !== threadId) continue;
pendingComms.delete(requestId);
clearTimeout(pending.timer);
settleCard(pending, "deny", "system");
pending.resolve("deny");
}
}

/** Cards left on disk by a previous run can never be answered — their
* in-memory approval died with the process. Settle them at boot so a
* crashed run doesn't leave a thread with a permanently blocked composer. */
Expand Down
Loading