Skip to content

Commit e001838

Browse files
vincentkocvincentkoc
committed
fix(agents): reject empty completion handoffs
1 parent 69d1d78 commit e001838

3 files changed

Lines changed: 209 additions & 29 deletions

File tree

‎src/agents/subagent-announce-delivery.test.ts‎

Lines changed: 136 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1488,6 +1488,142 @@ describe("deliverSubagentAnnouncement completion delivery", () => {
14881488
});
14891489
});
14901490

1491+
it.each([
1492+
{ name: "no payloads", result: { payloads: [] } },
1493+
{
1494+
name: "tool calls without delivery evidence",
1495+
result: { payloads: [], meta: { toolSummary: { calls: 1 } } },
1496+
},
1497+
])(
1498+
"fails session-only completion handoff when the in-process agent returns $name",
1499+
async ({ result: agentResult }) => {
1500+
const dispatchGatewayMethodInProcess = createInProcessGatewayMock({
1501+
result: agentResult,
1502+
});
1503+
testing.setDepsForTest({
1504+
dispatchGatewayMethodInProcess,
1505+
getRequesterSessionActivity: () => ({
1506+
sessionId: "requester-session-local",
1507+
isActive: false,
1508+
}),
1509+
getRuntimeConfig: () => ({}) as never,
1510+
});
1511+
1512+
const result = await deliverSubagentAnnouncement({
1513+
requesterSessionKey: "agent:main:local-session",
1514+
targetRequesterSessionKey: "agent:main:local-session",
1515+
triggerMessage: "child done",
1516+
steerMessage: "child done",
1517+
requesterIsSubagent: false,
1518+
expectsCompletionMessage: true,
1519+
bestEffortDeliver: true,
1520+
directIdempotencyKey: "announce-local-empty",
1521+
});
1522+
1523+
expectRecordFields(result, {
1524+
delivered: false,
1525+
path: "direct",
1526+
reason: "visible_reply_missing",
1527+
error: "completion agent did not produce a visible reply",
1528+
});
1529+
expectInProcessAgentParams(dispatchGatewayMethodInProcess, {
1530+
deliver: false,
1531+
channel: undefined,
1532+
to: undefined,
1533+
bestEffortDeliver: true,
1534+
});
1535+
},
1536+
);
1537+
1538+
it("accepts session-only completion handoff when the in-process agent intentionally replies NO_REPLY", async () => {
1539+
const dispatchGatewayMethodInProcess = createInProcessGatewayMock({
1540+
result: {
1541+
payloads: [{ text: "NO_REPLY" }],
1542+
},
1543+
});
1544+
testing.setDepsForTest({
1545+
dispatchGatewayMethodInProcess,
1546+
getRequesterSessionActivity: () => ({
1547+
sessionId: "requester-session-local",
1548+
isActive: false,
1549+
}),
1550+
getRuntimeConfig: () => ({}) as never,
1551+
});
1552+
1553+
const result = await deliverSubagentAnnouncement({
1554+
requesterSessionKey: "agent:main:local-session",
1555+
targetRequesterSessionKey: "agent:main:local-session",
1556+
triggerMessage: "child done",
1557+
steerMessage: "child done",
1558+
requesterIsSubagent: false,
1559+
expectsCompletionMessage: true,
1560+
bestEffortDeliver: true,
1561+
directIdempotencyKey: "announce-local-silent",
1562+
});
1563+
1564+
expectRecordFields(result, {
1565+
delivered: true,
1566+
path: "direct",
1567+
});
1568+
expectInProcessAgentParams(dispatchGatewayMethodInProcess, {
1569+
deliver: false,
1570+
channel: undefined,
1571+
to: undefined,
1572+
bestEffortDeliver: true,
1573+
});
1574+
});
1575+
1576+
it.each([
1577+
{
1578+
name: "accepted session spawn",
1579+
result: {
1580+
payloads: [],
1581+
acceptedSessionSpawns: [{ runId: "run-child", childSessionKey: "agent:main:child" }],
1582+
},
1583+
},
1584+
{
1585+
name: "successful cron add",
1586+
result: {
1587+
payloads: [],
1588+
successfulCronAdds: 1,
1589+
},
1590+
},
1591+
])("accepts session-only completion handoff with $name evidence", async ({ result }) => {
1592+
const dispatchGatewayMethodInProcess = createInProcessGatewayMock({
1593+
result,
1594+
});
1595+
testing.setDepsForTest({
1596+
dispatchGatewayMethodInProcess,
1597+
getRequesterSessionActivity: () => ({
1598+
sessionId: "requester-session-local",
1599+
isActive: false,
1600+
}),
1601+
getRuntimeConfig: () => ({}) as never,
1602+
});
1603+
1604+
const delivery = await deliverSubagentAnnouncement({
1605+
requesterSessionKey: "agent:main:local-session",
1606+
targetRequesterSessionKey: "agent:main:local-session",
1607+
triggerMessage: "child done",
1608+
steerMessage: "child done",
1609+
requesterIsSubagent: false,
1610+
expectsCompletionMessage: true,
1611+
bestEffortDeliver: true,
1612+
directIdempotencyKey: "announce-local-side-effect",
1613+
});
1614+
1615+
expectRecordFields(delivery, {
1616+
delivered: true,
1617+
path: "direct",
1618+
});
1619+
expectInProcessAgentParams(dispatchGatewayMethodInProcess, {
1620+
deliver: false,
1621+
channel: undefined,
1622+
to: undefined,
1623+
bestEffortDeliver: true,
1624+
});
1625+
});
1626+
14911627
it("does not require generated media delivery for no-target cron completion handoffs", async () => {
14921628
const dispatchGatewayMethodInProcess = createInProcessGatewayMock({
14931629
result: {

‎src/agents/subagent-announce-delivery.ts‎

Lines changed: 61 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ import {
3636
isInternalMessageChannel,
3737
normalizeMessageChannel,
3838
} from "../utils/message-channel.js";
39+
import { hasAcceptedSessionSpawn } from "./accepted-session-spawn.js";
3940
import {
4041
collectDeliveredMediaUrls,
4142
collectMessagingToolDeliveredMediaUrls,
@@ -659,37 +660,56 @@ function hasGatewayAgentMessagingToolDeliveryEvidence(response: unknown): boolea
659660
return Boolean(result && hasMessagingToolDeliveryEvidence(result));
660661
}
661662

663+
function hasGatewayAgentCompletionSideEffectEvidence(response: unknown): boolean {
664+
const result = getGatewayAgentResult(response);
665+
if (!result) {
666+
return false;
667+
}
668+
return (
669+
hasMessagingToolDeliveryEvidence(result) ||
670+
(Array.isArray(result.acceptedSessionSpawns) &&
671+
hasAcceptedSessionSpawn(result.acceptedSessionSpawns)) ||
672+
hasPositiveDeliveryCount(result.successfulCronAdds)
673+
);
674+
}
675+
662676
function hasIntentionalSilentGatewayAgentPayload(response: unknown): boolean {
663677
const result = getGatewayAgentResult(response);
664678
if (!result || !Array.isArray(result.payloads)) {
665679
return false;
666680
}
667-
return result.payloads.some((payload) => {
668-
if (!payload || typeof payload !== "object" || Array.isArray(payload)) {
669-
return false;
670-
}
671-
const record = payload as {
672-
text?: unknown;
673-
mediaUrl?: unknown;
674-
mediaUrls?: unknown;
675-
presentation?: unknown;
676-
interactive?: unknown;
677-
channelData?: unknown;
678-
};
679-
if (
680-
typeof record.text !== "string" ||
681-
!isSilentReplyPayloadText(record.text, SILENT_REPLY_TOKEN)
682-
) {
683-
return false;
684-
}
685-
return !(
686-
record.mediaUrl ||
687-
(Array.isArray(record.mediaUrls) && record.mediaUrls.length > 0) ||
688-
record.presentation ||
689-
record.interactive ||
690-
record.channelData
691-
);
692-
});
681+
return result.payloads.some(isIntentionalSilentGatewayAgentPayload);
682+
}
683+
684+
function isIntentionalSilentGatewayAgentPayload(payload: unknown): boolean {
685+
if (!payload || typeof payload !== "object" || Array.isArray(payload)) {
686+
return false;
687+
}
688+
const record = payload as {
689+
text?: unknown;
690+
mediaUrl?: unknown;
691+
mediaUrls?: unknown;
692+
presentation?: unknown;
693+
interactive?: unknown;
694+
channelData?: unknown;
695+
};
696+
if (
697+
typeof record.text !== "string" ||
698+
!isSilentReplyPayloadText(record.text, SILENT_REPLY_TOKEN)
699+
) {
700+
return false;
701+
}
702+
return !(
703+
record.mediaUrl ||
704+
(Array.isArray(record.mediaUrls) && record.mediaUrls.length > 0) ||
705+
record.presentation ||
706+
record.interactive ||
707+
record.channelData
708+
);
709+
}
710+
711+
function hasPositiveDeliveryCount(value: unknown): boolean {
712+
return typeof value === "number" && Number.isFinite(value) && value > 0;
693713
}
694714

695715
function requiresAgentMediatedCompletionDelivery(params: {
@@ -1592,6 +1612,21 @@ async function sendSubagentAnnounceDirectly(params: {
15921612
error: "completion agent did not use the message tool for message-tool-only delivery",
15931613
};
15941614
}
1615+
if (
1616+
params.expectsCompletionMessage &&
1617+
!shouldDeliverAgentFinal &&
1618+
!requiresMessageToolDelivery &&
1619+
!hasVisibleGatewayAgentPayload(directAnnounceResponse) &&
1620+
!hasGatewayAgentCompletionSideEffectEvidence(directAnnounceResponse) &&
1621+
!hasIntentionalSilentGatewayAgentPayload(directAnnounceResponse)
1622+
) {
1623+
return {
1624+
delivered: false,
1625+
path: "direct",
1626+
reason: "visible_reply_missing",
1627+
error: "completion agent did not produce a visible reply",
1628+
};
1629+
}
15951630
if (
15961631
params.expectsCompletionMessage &&
15971632
shouldDeliverAgentFinal &&

‎src/agents/subagent-announce.live.test.ts‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -625,14 +625,23 @@ describeLive("subagent announce live", () => {
625625
});
626626

627627
const completedDispatch = await waitFor(
628-
"in-process subagent completion agent dispatch",
628+
"in-process subagent completion agent dispatch with parent token",
629629
() => {
630630
if (initialError) {
631631
throw toLintErrorObject(initialError, "Non-Error thrown");
632632
}
633-
return inProcessAgentDispatches.find((entry) => entry.phase === "completed");
633+
return inProcessAgentDispatches.find(
634+
(entry) => entry.phase === "completed" && entry.resultText.includes(parentToken),
635+
);
634636
},
635-
);
637+
).catch((error: unknown) => {
638+
throw new Error(
639+
`timed out waiting for parent token in completion dispatch; dispatches=${JSON.stringify(
640+
inProcessAgentDispatches,
641+
)}`,
642+
{ cause: error },
643+
);
644+
});
636645
expect(completedDispatch.resultText).toContain(parentToken);
637646
expect(
638647
inProcessAgentDispatches.some((entry) => {

0 commit comments

Comments
 (0)