From 8ecb609990ff191bb9258f5685f90cbdde7e6c17 Mon Sep 17 00:00:00 2001 From: SunnyShu Date: Wed, 22 Jul 2026 11:25:46 +0800 Subject: [PATCH] fix(agents): emit diagnostic when sessions_yield parks without continuation evidence (#100146) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(agents): emit diagnostic when sessions_yield parks without continuation evidence (#100146) Add a shared hasYieldContinuationEvidence predicate in incomplete-turn.ts and emit a user-visible diagnostic payload in terminal-resolution.ts when a yielded turn has no same-turn continuation source (accepted spawn, async tool, messaging delivery, cron add). Simplified per ClawSweeper P1 review: remove overbroad activeDescendantCount suppression — only same-turn evidence is considered. Pre-existing descendant waiting deferred to separate lifecycle design. Source +53, Tests +131. Total +184 across 3 files. * fix(agents): remove unused export of hasAsyncStartedToolActivity knip deadcode detected this exported function has no external consumers; all usages are internal to incomplete-turn.ts. Removing export fixes both check-dependencies and ci-gate CI failures. * ci: retrigger Telegram proof * fix(agents): preserve yielded client tool calls Co-authored-by: SunnyShu0925 * docs(agents): explain yield evidence scope Co-authored-by: SunnyShu0925 * docs(agents): bound silent-yield diagnostic Co-authored-by: SunnyShu0925 --------- Co-authored-by: Peter Steinberger --- .../run/incomplete-turn.ts | 34 +++++ .../run/terminal-resolution.ts | 12 +- .../sessions-yield.orchestration.test.ts | 141 ++++++++++++++++-- 3 files changed, 175 insertions(+), 12 deletions(-) diff --git a/src/agents/embedded-agent-runner/run/incomplete-turn.ts b/src/agents/embedded-agent-runner/run/incomplete-turn.ts index 04e2260a1d13..804a3c49b23a 100644 --- a/src/agents/embedded-agent-runner/run/incomplete-turn.ts +++ b/src/agents/embedded-agent-runner/run/incomplete-turn.ts @@ -10,6 +10,7 @@ import { } from "../../../auto-reply/tokens.js"; import { hasAcceptedSessionSpawn } from "../../accepted-session-spawn.js"; import { collectTextContentBlocks } from "../../content-blocks.js"; +import type { MessagingToolSend } from "../../embedded-agent-messaging.types.js"; import { isStrictAgenticSupportedProviderModel, stripProviderPrefix, @@ -375,6 +376,39 @@ function hasAsyncStartedToolActivity(toolMetas?: readonly { asyncStarted?: boole return (toolMetas ?? []).some((entry) => entry.asyncStarted === true); } +/** Fields needed to determine whether a yielded turn already delivered or can continue. */ +interface YieldContinuationAttempt { + clientToolCalls?: readonly unknown[]; + didSendDeterministicApprovalPrompt?: boolean; + successfulCronAdds?: number; + acceptedSessionSpawns?: readonly { runId: string; childSessionKey: string }[]; + messagingToolSentTexts?: readonly string[]; + messagingToolSentMediaUrls?: readonly string[]; + messagingToolSentTargets?: readonly MessagingToolSend[]; + toolMetas?: readonly { asyncStarted?: boolean }[]; +} + +/** Continuation evidence for a yielded turn — sources that will produce future output. */ +export function hasYieldContinuationEvidence(attempt: YieldContinuationAttempt): boolean { + // Only same-attempt evidence is causal here. Session-wide active descendants may be + // stale or unrelated and must not suppress the diagnostic for this yielded turn. + return ( + (attempt.clientToolCalls?.length ?? 0) > 0 || + attempt.didSendDeterministicApprovalPrompt === true || + hasCommittedMessagingToolDeliveryEvidence({ + messagingToolSentTexts: attempt.messagingToolSentTexts ?? [], + messagingToolSentMediaUrls: attempt.messagingToolSentMediaUrls ?? [], + messagingToolSentTargets: attempt.messagingToolSentTargets ?? [], + }) || + hasAcceptedSessionSpawn(attempt.acceptedSessionSpawns) || + hasAsyncStartedToolActivity(attempt.toolMetas) || + (attempt.successfulCronAdds ?? 0) > 0 + ); +} + +export const YIELD_DIAGNOSTIC_TEXT = + "⚠️ Turn yielded without a continuation source. Send a message to resume."; + function isToolResultRole(role: string): boolean { return role === "toolresult" || role === "tool_result" || role === "tool"; } diff --git a/src/agents/embedded-agent-runner/run/terminal-resolution.ts b/src/agents/embedded-agent-runner/run/terminal-resolution.ts index 94156a85814a..20c2b17792ce 100644 --- a/src/agents/embedded-agent-runner/run/terminal-resolution.ts +++ b/src/agents/embedded-agent-runner/run/terminal-resolution.ts @@ -20,6 +20,7 @@ import { import type { EmbeddedRunContextRecoveryState } from "./context-recovery-state.js"; import { hasAttemptTerminalState, + hasYieldContinuationEvidence, resolveAttemptReplayMetadata, resolveEmptyResponseRetryInstruction, resolveIncompleteTurnPayloadText, @@ -29,6 +30,7 @@ import { resolveToolUseTerminalContinuationInstruction, shouldRetryMissingAssistantTurn, shouldTreatEmptyAssistantReplyAsSilent, + YIELD_DIAGNOSTIC_TEXT, } from "./incomplete-turn.js"; import type { RunEmbeddedAgentParams } from "./params.js"; import { @@ -451,6 +453,8 @@ function completeEmbeddedRun( onSuccessfulAuthBinding: input.runParams.onSuccessfulAuthBinding, }); const replayInvalid = input.resolveReplayInvalid(null); + const yieldHasContinuation = + input.attempt.yieldDetected && hasYieldContinuationEvidence(input.attempt); const livenessState = input.attempt.yieldDetected ? "paused" : resolveRunLivenessState({ @@ -465,9 +469,15 @@ function completeEmbeddedRun( : input.attempt.yieldDetected ? "end_turn" : (input.attemptAssistant?.stopReason as string | undefined); + // Existing visible payloads already avoid the silent-park symptom. The diagnostic + // fills only an otherwise empty yielded turn and must not duplicate visible output. const terminalPayloads = input.emptyAssistantReplyIsSilent ? [{ text: SILENT_REPLY_TOKEN }] - : input.payloadsForTerminalPath; + : input.payloadsForTerminalPath?.length + ? input.payloadsForTerminalPath + : input.attempt.yieldDetected && !yieldHasContinuation + ? [{ text: YIELD_DIAGNOSTIC_TEXT }] + : input.payloadsForTerminalPath; input.setTerminalLifecycleMeta({ replayInvalid, livenessState, diff --git a/src/agents/embedded-agent-runner/sessions-yield.orchestration.test.ts b/src/agents/embedded-agent-runner/sessions-yield.orchestration.test.ts index 2d0c3291c623..3e991e28e743 100644 --- a/src/agents/embedded-agent-runner/sessions-yield.orchestration.test.ts +++ b/src/agents/embedded-agent-runner/sessions-yield.orchestration.test.ts @@ -3,7 +3,6 @@ * with no pending tool calls, so the parent session is idle when subagent * results arrive. */ - import { expectDefined } from "@openclaw/normalization-core"; import { beforeAll, beforeEach, describe, expect, it } from "vitest"; import { makeAttemptResult } from "./run.overflow-compaction.fixture.js"; @@ -12,6 +11,7 @@ import { mockedGlobalHookRunner, mockedRunEmbeddedAttempt, overflowBaseRunParams, + resetRunOverflowCompactionHarnessMocks, warmRunOverflowCompactionHarness, } from "./run.overflow-compaction.harness.js"; import { isEmbeddedAgentRunActive, queueEmbeddedAgentMessageWithOutcome } from "./runs.js"; @@ -25,7 +25,7 @@ describe("sessions_yield orchestration", () => { }); beforeEach(() => { - mockedRunEmbeddedAttempt.mockReset(); + resetRunOverflowCompactionHarnessMocks(); mockedGlobalHookRunner.hasHooks.mockImplementation(() => false); }); @@ -82,9 +82,10 @@ describe("sessions_yield orchestration", () => { // clientToolCalls wins — tool_calls stopReason, pendingToolCalls populated expect(result.meta.stopReason).toBe("tool_calls"); - const pendingToolCalls = expectDefined(result.meta.pendingToolCalls, "pending tool calls"); - expect(pendingToolCalls).toHaveLength(1); - expect(expectDefined(pendingToolCalls[0], "hosted tool call").name).toBe("hosted_tool"); + expect(result.meta.pendingToolCalls).toHaveLength(1); + const hostedToolCall = expectDefined(result.meta.pendingToolCalls![0], "hosted tool call"); + expect(hostedToolCall.name).toBe("hosted_tool"); + expect(result.payloads).toBeUndefined(); }); it("preserves order across multiple client tool calls in one attempt (#52288)", async () => { @@ -108,20 +109,64 @@ describe("sessions_yield orchestration", () => { }); expect(result.meta.stopReason).toBe("tool_calls"); - const pendingToolCalls = expectDefined(result.meta.pendingToolCalls, "pending tool calls"); - expect(pendingToolCalls).toHaveLength(3); - expect(pendingToolCalls.map((c) => c.name)).toEqual([ + expect(result.meta.pendingToolCalls).toHaveLength(3); + expect(result.meta.pendingToolCalls!.map((c) => c.name)).toEqual([ "create_graph", "activate_graph", "get_status", ]); - expect( - JSON.parse(expectDefined(pendingToolCalls[0], "first pending tool call").arguments), - ).toEqual({ + const firstCall = expectDefined(result.meta.pendingToolCalls![0], "first pending tool call"); + expect(JSON.parse(firstCall.arguments)).toEqual({ nodes: ["a", "b"], }); }); + describe("yield with continuation evidence", () => { + it("yield with accepted spawn — diagnostic suppressed", async () => { + // Regression: a yielded turn with an accepted spawn must NOT emit the + // diagnostic — the spawned subagent will produce results. + mockedRunEmbeddedAttempt.mockResolvedValueOnce( + makeAttemptResult({ + promptError: null, + yieldDetected: true, + assistantTexts: [], + acceptedSessionSpawns: [{ runId: "child-run", childSessionKey: "child-key" }], + }), + ); + + const result = await runEmbeddedAgent({ + ...overflowBaseRunParams, + runId: "run-yield-accepted-spawn-suppressed", + }); + + // Accepted spawn is continuation evidence → no diagnostic payload + expect(result.payloads).toBeUndefined(); + expect(result.meta.stopReason).toBe("end_turn"); + expect(result.meta.yielded).toBe(true); + }); + + it("yield with async started tool — diagnostic suppressed", async () => { + mockedRunEmbeddedAttempt.mockResolvedValueOnce( + makeAttemptResult({ + promptError: null, + yieldDetected: true, + assistantTexts: [], + toolMetas: [{ toolName: "my_async_tool", asyncStarted: true }], + }), + ); + + const result = await runEmbeddedAgent({ + ...overflowBaseRunParams, + runId: "run-yield-async-tool-suppressed", + }); + + // Async tool activity is continuation evidence → no diagnostic payload + expect(result.payloads).toBeUndefined(); + expect(result.meta.stopReason).toBe("end_turn"); + expect(result.meta.yielded).toBe(true); + }); + }); + it("normal attempt without yield has no stopReason override", async () => { mockedRunEmbeddedAttempt.mockResolvedValueOnce(makeAttemptResult({ promptError: null })); @@ -134,4 +179,78 @@ describe("sessions_yield orchestration", () => { expect(result.meta.stopReason).toBeUndefined(); expect(result.meta.pendingToolCalls).toBeUndefined(); }); + + it("emits diagnostic payload when yieldDetected has no continuation evidence", async () => { + mockedRunEmbeddedAttempt.mockResolvedValueOnce( + makeAttemptResult({ + promptError: null, + yieldDetected: true, + assistantTexts: [], + }), + ); + + const result = await runEmbeddedAgent({ + ...overflowBaseRunParams, + runId: "run-yield-no-continuation", + }); + + // yieldDetected without any continuation source → diagnostic payload + expect(result.payloads).toHaveLength(1); + const diagnosticPayload = expectDefined(result.payloads![0], "diagnostic payload"); + expect(diagnosticPayload.text).toBe( + "⚠️ Turn yielded without a continuation source. Send a message to resume.", + ); + // stopReason is still end_turn (yield semantics preserved) + expect(result.meta.stopReason).toBe("end_turn"); + // No pending tool calls + expect(result.meta.pendingToolCalls).toBeUndefined(); + }); + + it("whitespace-only delivery text does not suppress diagnostic", async () => { + // Normalized helpers filter whitespace-only text — yield still parks + mockedRunEmbeddedAttempt.mockResolvedValueOnce( + makeAttemptResult({ + promptError: null, + yieldDetected: true, + assistantTexts: [], + didSendViaMessagingTool: true, + messagingToolSentTexts: [" "], + }), + ); + + const result = await runEmbeddedAgent({ + ...overflowBaseRunParams, + runId: "run-yield-whitespace-delivery", + }); + + // Whitespace-only delivery is not committed delivery → diagnostic emitted + expect(result.payloads).toHaveLength(1); + const wsPayload = expectDefined(result.payloads![0], "whitespace diagnostic payload"); + expect(wsPayload.text).toBe( + "⚠️ Turn yielded without a continuation source. Send a message to resume.", + ); + }); + + it("empty spawn array does not suppress diagnostic", async () => { + // An explicit empty spawn array is not a valid continuation + mockedRunEmbeddedAttempt.mockResolvedValueOnce( + makeAttemptResult({ + promptError: null, + yieldDetected: true, + assistantTexts: [], + acceptedSessionSpawns: [], + }), + ); + + const result = await runEmbeddedAgent({ + ...overflowBaseRunParams, + runId: "run-yield-empty-spawn", + }); + + expect(result.payloads).toHaveLength(1); + const emptySpawnPayload = expectDefined(result.payloads![0], "empty spawn diagnostic payload"); + expect(emptySpawnPayload.text).toBe( + "⚠️ Turn yielded without a continuation source. Send a message to resume.", + ); + }); });