From 68a1e00b73bd746f6fb577f4127fecb0ade9e228 Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Tue, 23 Jun 2026 05:48:01 +0200 Subject: [PATCH] fix(agents): retry silent subagent completion handoffs --- .../agents/subagent-fanout-synthesis.yaml | 19 +++++-- src/agents/subagent-announce-delivery.test.ts | 44 ++++++++++++++- src/agents/subagent-announce-delivery.ts | 54 +++++++++++++++++-- src/agents/subagent-announce.ts | 2 +- 4 files changed, 109 insertions(+), 10 deletions(-) diff --git a/qa/scenarios/agents/subagent-fanout-synthesis.yaml b/qa/scenarios/agents/subagent-fanout-synthesis.yaml index 31fcb4e30865..607f58c67eae 100644 --- a/qa/scenarios/agents/subagent-fanout-synthesis.yaml +++ b/qa/scenarios/agents/subagent-fanout-synthesis.yaml @@ -76,6 +76,15 @@ flow: - ref: env - 120000 - call: reset + - set: alphaLabel + value: + expr: "env.providerMode === 'mock-openai' ? config.expectedChildLabels[0] : `${config.expectedChildLabels[0]}-${attempt}`" + - set: betaLabel + value: + expr: "env.providerMode === 'mock-openai' ? config.expectedChildLabels[1] : `${config.expectedChildLabels[1]}-${attempt}`" + - set: prompt + value: + expr: "`Subagent fanout synthesis check: delegate exactly two bounded subagents sequentially using sessions_spawn, not ACP.\nFirst spawn exactly one child with label ${alphaLabel} and task: verify that \\`HEARTBEAT.md\\` exists and reply exactly \\`ok\\` if it does. Wait for that child to finish.\nThen spawn exactly one child with label ${betaLabel} and task: verify that \\`repo/qa/scenarios/agents/subagent-fanout-synthesis.yaml\\` exists and reply exactly \\`ok\\` if it does. Wait for that child to finish.\nDo not spawn any more children after ${betaLabel} finishes.\nThen reply with exactly these two lines and nothing else:\nsubagent-1: ok\nsubagent-2: ok`" - set: sessionKey value: expr: "`agent:qa:fanout:${attempt}:${randomUUID().slice(0, 8)}`" @@ -85,7 +94,7 @@ flow: - sessionKey: ref: sessionKey message: - expr: config.prompt + ref: prompt timeoutMs: expr: liveTurnTimeoutMs(env, 90000) - call: waitForAgentHistoryReply @@ -110,10 +119,10 @@ flow: expr: "Object.values(store).filter((entry) => entry.spawnedBy === sessionKey)" - set: sawAlpha value: - expr: "childRows.some((entry) => entry.label === config.expectedChildLabels[0])" + expr: "childRows.some((entry) => entry.label === alphaLabel)" - set: sawBeta value: - expr: "childRows.some((entry) => entry.label === config.expectedChildLabels[1])" + expr: "childRows.some((entry) => entry.label === betaLabel)" - assert: expr: "sawAlpha && sawBeta" message: @@ -159,10 +168,10 @@ flow: expr: "Object.values(timeoutStore).filter((entry) => entry.spawnedBy === sessionKey)" - set: timeoutSawAlpha value: - expr: "timeoutChildRows.some((entry) => entry.label === config.expectedChildLabels[0])" + expr: "timeoutChildRows.some((entry) => entry.label === alphaLabel)" - set: timeoutSawBeta value: - expr: "timeoutChildRows.some((entry) => entry.label === config.expectedChildLabels[1])" + expr: "timeoutChildRows.some((entry) => entry.label === betaLabel)" - set: timeoutSpawnRequests value: expr: "[...(await fetchJson(`${env.mock.baseUrl}/debug/requests`))].filter((request) => request.plannedToolName === 'sessions_spawn' && /subagent fanout synthesis check/i.test(String(request.allInputText ?? '')))" diff --git a/src/agents/subagent-announce-delivery.test.ts b/src/agents/subagent-announce-delivery.test.ts index ac902e4ae5ed..5ea0537336ba 100644 --- a/src/agents/subagent-announce-delivery.test.ts +++ b/src/agents/subagent-announce-delivery.test.ts @@ -1548,7 +1548,7 @@ describe("deliverSubagentAnnouncement completion delivery", () => { }, ); - it("accepts session-only completion handoff when the in-process agent intentionally replies NO_REPLY", async () => { + it("accepts non-subagent session-only completion handoff when the in-process agent intentionally replies NO_REPLY", async () => { const dispatchGatewayMethodInProcess = createInProcessGatewayMock({ result: { payloads: [{ text: "NO_REPLY" }], @@ -1572,6 +1572,7 @@ describe("deliverSubagentAnnouncement completion delivery", () => { expectsCompletionMessage: true, bestEffortDeliver: true, directIdempotencyKey: "announce-local-silent", + sourceTool: "agent_harness_task", }); expectRecordFields(result, { @@ -1586,6 +1587,47 @@ describe("deliverSubagentAnnouncement completion delivery", () => { }); }); + it("rejects session-only subagent completion handoff when the parent only replies NO_REPLY", async () => { + const dispatchGatewayMethodInProcess = createInProcessGatewayMock({ + result: { + payloads: [{ text: "NO_REPLY" }], + }, + }); + testing.setDepsForTest({ + dispatchGatewayMethodInProcess, + getRequesterSessionActivity: () => ({ + sessionId: "requester-session-local", + isActive: false, + }), + getRuntimeConfig: () => ({}) as never, + }); + + const result = await deliverSubagentAnnouncement({ + requesterSessionKey: "agent:main:local-session", + targetRequesterSessionKey: "agent:main:local-session", + triggerMessage: "child done", + steerMessage: "child done", + requesterIsSubagent: false, + expectsCompletionMessage: true, + bestEffortDeliver: true, + directIdempotencyKey: "announce-local-subagent-silent", + sourceTool: "subagent_announce", + }); + + expectRecordFields(result, { + delivered: false, + path: "direct", + reason: "visible_reply_missing", + error: "completion agent did not produce a visible reply", + }); + expectInProcessAgentParams(dispatchGatewayMethodInProcess, { + deliver: false, + channel: undefined, + to: undefined, + bestEffortDeliver: true, + }); + }); + it.each([ { name: "accepted session spawn", diff --git a/src/agents/subagent-announce-delivery.ts b/src/agents/subagent-announce-delivery.ts index 42b4312d489d..9eec384fd060 100644 --- a/src/agents/subagent-announce-delivery.ts +++ b/src/agents/subagent-announce-delivery.ts @@ -667,6 +667,46 @@ function hasVisibleGatewayAgentPayload(response: unknown): boolean { ); } +function hasVisibleNonSilentGatewayAgentPayload(response: unknown): boolean { + const result = getGatewayAgentResult(response); + if (!result) { + return false; + } + if (hasMessagingToolDeliveryEvidence(result)) { + return true; + } + const payloads = Array.isArray(result.payloads) ? result.payloads : []; + return payloads.some(isVisibleNonSilentGatewayAgentPayload); +} + +function isVisibleNonSilentGatewayAgentPayload(payload: unknown): boolean { + if (!payload || typeof payload !== "object" || Array.isArray(payload)) { + return false; + } + const record = payload as { + text?: unknown; + mediaUrl?: unknown; + mediaUrls?: unknown; + presentation?: unknown; + interactive?: unknown; + channelData?: unknown; + }; + if ( + record.mediaUrl || + (Array.isArray(record.mediaUrls) && record.mediaUrls.length > 0) || + record.presentation || + record.interactive || + record.channelData + ) { + return true; + } + return ( + typeof record.text === "string" && + record.text.trim() !== "" && + !isSilentReplyPayloadText(record.text, SILENT_REPLY_TOKEN) + ); +} + function hasGatewayAgentMessagingToolDeliveryEvidence(response: unknown): boolean { const result = getGatewayAgentResult(response); return Boolean(result && hasMessagingToolDeliveryEvidence(result)); @@ -1613,13 +1653,21 @@ async function sendSubagentAnnounceDirectly(params: { error: "completion agent did not use the message tool for message-tool-only delivery", }; } + const hasVisibleCompletionReply = + hasVisibleNonSilentGatewayAgentPayload(directAnnounceResponse); + const hasCompletionSideEffect = + hasGatewayAgentCompletionSideEffectEvidence(directAnnounceResponse); + const hasIntentionalSilentCompletionReply = + hasIntentionalSilentGatewayAgentPayload(directAnnounceResponse); + const acceptsIntentionalSilentCompletion = + hasIntentionalSilentCompletionReply && !isSubagentCompletion; if ( params.expectsCompletionMessage && !shouldDeliverAgentFinal && !requiresMessageToolDelivery && - !hasVisibleGatewayAgentPayload(directAnnounceResponse) && - !hasGatewayAgentCompletionSideEffectEvidence(directAnnounceResponse) && - !hasIntentionalSilentGatewayAgentPayload(directAnnounceResponse) + !hasVisibleCompletionReply && + !hasCompletionSideEffect && + !acceptsIntentionalSilentCompletion ) { return { delivered: false, diff --git a/src/agents/subagent-announce.ts b/src/agents/subagent-announce.ts index bf95b6182b09..28c59d154bc2 100644 --- a/src/agents/subagent-announce.ts +++ b/src/agents/subagent-announce.ts @@ -93,7 +93,7 @@ function buildAnnounceReplyInstruction(params: { return `Convert this completion into a concise internal orchestration update for your parent agent in your own words. Keep this internal context private (don't mention system/log/stats/session details or announce type). If this result is duplicate or no update is needed, reply ONLY: ${SILENT_REPLY_TOKEN}.`; } if (params.expectsCompletionMessage) { - return `A completed ${params.announceType} is ready for parent review. Review/verify the result above before deciding whether the original task is done. If additional action is required, continue the task or record a follow-up; otherwise send a truthful user-facing update. Keep this internal context private (don't mention system/log/stats/session details or announce type). Reply ONLY: ${SILENT_REPLY_TOKEN} when no user-facing update is needed.`; + return `A completed ${params.announceType} is ready for parent review. Review/verify the result above before deciding whether the original task is done. If additional action is required, continue the task or record a follow-up; otherwise send a truthful user-facing update. Keep this internal context private (don't mention system/log/stats/session details or announce type). Reply ONLY: ${SILENT_REPLY_TOKEN} only when this exact result is already visible to the user in this same turn.`; } return `A completed ${params.announceType} is ready for parent review. Review/verify the result above before deciding whether the original task is done. If additional action is required, continue the task or record a follow-up; otherwise send a truthful user-facing update. Keep this internal context private (don't mention system/log/stats/session details or announce type), and do not copy the internal event text verbatim. Reply ONLY: ${SILENT_REPLY_TOKEN} if this exact result was already delivered to the user in this same turn.`; }