mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-12 21:53:00 -06:00
fix(agents): emit diagnostic when sessions_yield parks without continuation evidence (#100146)
* 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 <shu.zongyu@xydigit.com> * docs(agents): explain yield evidence scope Co-authored-by: SunnyShu0925 <shu.zongyu@xydigit.com> * docs(agents): bound silent-yield diagnostic Co-authored-by: SunnyShu0925 <shu.zongyu@xydigit.com> --------- Co-authored-by: Peter Steinberger <steipete@gmail.com>
This commit is contained in:
@@ -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";
|
||||
}
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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.",
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user