Skip to content

Commit 04ad66b

Browse files
anyechanyech
committed
fix(tasks): deliver ACP completions to bound Discord threads
1 parent 7967a35 commit 04ad66b

2 files changed

Lines changed: 171 additions & 5 deletions

File tree

‎src/tasks/task-registry.test.ts‎

Lines changed: 143 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,7 @@ vi.mock("../agents/subagent-control.js", () => ({
131131

132132
vi.mock("../utils/message-channel.js", () => ({
133133
isDeliverableMessageChannel: (channel: string) =>
134-
channel === "notifychat" || channel === "guildchat",
134+
channel === "notifychat" || channel === "guildchat" || channel === "discord",
135135
}));
136136

137137
function configureTaskRegistryMaintenanceRuntimeForTest(params: {
@@ -1337,6 +1337,148 @@ describe("task-registry", () => {
13371337
});
13381338
});
13391339

1340+
it("delivers delegated ACP completion directly to an explicitly bound Discord thread", async () => {
1341+
await withTaskRegistryTempDir(async (root) => {
1342+
process.env.OPENCLAW_STATE_DIR = root;
1343+
resetTaskRegistryForTests();
1344+
const runId = "run-bound-discord-thread-terminal";
1345+
hoisted.sendMessageMock.mockResolvedValue({
1346+
channel: "discord",
1347+
to: "channel:parent-channel",
1348+
via: "direct",
1349+
});
1350+
1351+
createTaskRecord({
1352+
runtime: "acp",
1353+
ownerKey: "agent:main:discord:guild-123:channel-parent-channel",
1354+
scopeKind: "session",
1355+
requesterOrigin: {
1356+
channel: "discord",
1357+
to: "channel:parent-channel",
1358+
threadId: "thread-84022",
1359+
},
1360+
childSessionKey: "agent:main:acp:child",
1361+
runId,
1362+
task: "Investigate thread-bound ACP delivery",
1363+
status: "running",
1364+
deliveryStatus: "pending",
1365+
terminalSummary: "ACP final answer",
1366+
startedAt: 100,
1367+
});
1368+
1369+
emitAgentEvent({
1370+
runId,
1371+
stream: "lifecycle",
1372+
data: {
1373+
phase: "end",
1374+
endedAt: 250,
1375+
},
1376+
});
1377+
1378+
await waitForAssertion(() => {
1379+
const task = findTaskByRunId(runId);
1380+
if (!task) {
1381+
throw new Error(`Expected task for run ${runId}`);
1382+
}
1383+
expect(task.status).toBe("succeeded");
1384+
expect(task.deliveryStatus).toBe("delivered");
1385+
});
1386+
await waitForAssertion(() => expect(hoisted.sendMessageMock).toHaveBeenCalledTimes(1));
1387+
const message = sentMessageCall();
1388+
expectRecordFields(message, {
1389+
channel: "discord",
1390+
to: "channel:parent-channel",
1391+
threadId: "thread-84022",
1392+
});
1393+
expect(String(message.content)).toContain(
1394+
"Background task ready for review: ACP background task",
1395+
);
1396+
expect(String(message.content)).toContain("ACP final answer");
1397+
expect(String(message.content)).toContain(
1398+
"Next: parent will review/verify before calling it done.",
1399+
);
1400+
expect(peekSystemEvents("agent:main:discord:guild-123:channel-parent-channel")).toStrictEqual(
1401+
[],
1402+
);
1403+
});
1404+
});
1405+
1406+
it.each([
1407+
{
1408+
id: "missing-thread",
1409+
requesterOrigin: {
1410+
channel: "discord",
1411+
to: "channel:parent-channel",
1412+
},
1413+
},
1414+
{
1415+
id: "non-channel-target",
1416+
requesterOrigin: {
1417+
channel: "discord",
1418+
to: "user:U123",
1419+
threadId: "thread-84022",
1420+
},
1421+
},
1422+
{
1423+
id: "non-discord-channel",
1424+
requesterOrigin: {
1425+
channel: "guildchat",
1426+
to: "guildchat:channel:parent-channel",
1427+
threadId: "thread-84022",
1428+
},
1429+
},
1430+
])(
1431+
"keeps delegated ACP completion queued without an explicit bound Discord thread ($id)",
1432+
async ({ requesterOrigin }) => {
1433+
await withTaskRegistryTempDir(async (root) => {
1434+
process.env.OPENCLAW_STATE_DIR = root;
1435+
resetTaskRegistryForTests();
1436+
const runId = `run-non-bound-discord-thread-terminal-${requesterOrigin.channel}-${requesterOrigin.to}`;
1437+
hoisted.sendMessageMock.mockResolvedValue({
1438+
channel: requesterOrigin.channel,
1439+
to: requesterOrigin.to,
1440+
via: "direct",
1441+
});
1442+
1443+
createTaskRecord({
1444+
runtime: "acp",
1445+
ownerKey: "agent:main:discord:guild-123:channel-parent-channel",
1446+
scopeKind: "session",
1447+
requesterOrigin,
1448+
childSessionKey: "agent:main:acp:child",
1449+
runId,
1450+
task: "Investigate thread-bound ACP delivery",
1451+
status: "running",
1452+
deliveryStatus: "pending",
1453+
terminalSummary: "ACP final answer",
1454+
startedAt: 100,
1455+
});
1456+
1457+
emitAgentEvent({
1458+
runId,
1459+
stream: "lifecycle",
1460+
data: {
1461+
phase: "end",
1462+
endedAt: 250,
1463+
},
1464+
});
1465+
1466+
await waitForAssertion(() => {
1467+
const task = findTaskByRunId(runId);
1468+
if (!task) {
1469+
throw new Error(`Expected task for run ${runId}`);
1470+
}
1471+
expect(task.status).toBe("succeeded");
1472+
expect(task.deliveryStatus).toBe("session_queued");
1473+
});
1474+
expect(hoisted.sendMessageMock).not.toHaveBeenCalled();
1475+
expect(peekSystemEvents("agent:main:discord:guild-123:channel-parent-channel")).toEqual([
1476+
expect.stringContaining("Background task ready for review: ACP background task"),
1477+
]);
1478+
});
1479+
},
1480+
);
1481+
13401482
it.each([
13411483
{
13421484
id: "channel",

‎src/tasks/task-registry.ts‎

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1233,12 +1233,34 @@ function canDeliverTaskToRequesterOrigin(task: TaskRecord): boolean {
12331233
if (shouldRouteCompletionThroughRequesterSession(owner.sessionKey)) {
12341234
return false;
12351235
}
1236-
const origin = owner.requesterOrigin;
1236+
return canDeliverToRequesterOrigin(owner.requesterOrigin);
1237+
}
1238+
1239+
function canDeliverToRequesterOrigin(origin: TaskDeliveryState["requesterOrigin"]): boolean {
12371240
const channel = origin?.channel?.trim();
12381241
const to = origin?.to?.trim();
12391242
return Boolean(channel && to && isDeliverableMessageChannel(channel));
12401243
}
12411244

1245+
function canDeliverParentReviewTaskToBoundDiscordThread(task: TaskRecord): boolean {
1246+
if (!shouldUseParentReviewTaskTerminalMessage(task)) {
1247+
return false;
1248+
}
1249+
const owner = resolveTaskDeliveryOwner(task);
1250+
const origin = owner.requesterOrigin;
1251+
const channel = origin?.channel?.trim().toLowerCase();
1252+
const to = origin?.to?.trim().toLowerCase();
1253+
const threadId = String(origin?.threadId ?? "").trim();
1254+
// This is a narrow transport exception for explicitly bound Discord threads,
1255+
// not a general parent-review direct-delivery relaxation.
1256+
return Boolean(
1257+
channel === "discord" &&
1258+
to?.startsWith("channel:") &&
1259+
threadId &&
1260+
canDeliverToRequesterOrigin(origin),
1261+
);
1262+
}
1263+
12421264
function resolveMissingOwnerDeliveryStatus(task: TaskRecord): TaskDeliveryStatus {
12431265
return task.scopeKind === "system" ? "not_applicable" : "parent_missing";
12441266
}
@@ -1322,13 +1344,15 @@ export async function maybeDeliverTaskTerminalUpdate(taskId: string): Promise<Ta
13221344
});
13231345
}
13241346
const shouldRouteParentReview = shouldUseParentReviewTaskTerminalMessage(latest);
1325-
const canDeliverDirect = canDeliverTaskToRequesterOrigin(latest);
1347+
const shouldDeliverParentReviewDirect = canDeliverParentReviewTaskToBoundDiscordThread(latest);
1348+
const canDeliverDirect =
1349+
canDeliverTaskToRequesterOrigin(latest) || shouldDeliverParentReviewDirect;
13261350
const directEventText = formatTaskTerminalMessage(latest);
13271351
const sessionEventText = formatTaskTerminalMessage(
13281352
latest,
13291353
shouldRouteParentReview ? { surface: "parent_session" } : undefined,
13301354
);
1331-
if (shouldRouteParentReview || !canDeliverDirect) {
1355+
if ((shouldRouteParentReview && !shouldDeliverParentReviewDirect) || !canDeliverDirect) {
13321356
try {
13331357
queueTaskSystemEvent(latest, sessionEventText);
13341358
if (latest.terminalOutcome === "blocked") {
@@ -1360,7 +1384,7 @@ export async function maybeDeliverTaskTerminalUpdate(taskId: string): Promise<Ta
13601384
to: owner.requesterOrigin?.to ?? "",
13611385
accountId: owner.requesterOrigin?.accountId,
13621386
threadId: owner.requesterOrigin?.threadId,
1363-
content: directEventText,
1387+
content: shouldDeliverParentReviewDirect ? sessionEventText : directEventText,
13641388
agentId: requesterAgentId,
13651389
idempotencyKey,
13661390
mirror: {

0 commit comments

Comments
 (0)