mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-12 21:53:00 -06:00
fix(auto-reply): suppress false no-reply fallback during continuations (#119154)
* fix(auto-reply): keep pending continuations silent * docs(auto-reply): clarify fallback ownership --------- Co-authored-by: Ayaan Zaidi <hi@obviy.us>
This commit is contained in:
@@ -101,6 +101,11 @@ export async function prepareReplyAgentPayloads(state: {
|
||||
if (deliberateSilentTerminalReply) {
|
||||
opts?.onDeliberateSilentTerminalReply?.();
|
||||
}
|
||||
const pendingContinuation =
|
||||
runResult.meta?.yielded === true || (runResult.meta?.pendingToolCalls?.length ?? 0) > 0;
|
||||
if (pendingContinuation) {
|
||||
opts?.onPendingContinuation?.();
|
||||
}
|
||||
|
||||
const successfulSourceReplyDelivery = hasSuccessfulSourceReplyDelivery({
|
||||
blockReplyPipeline,
|
||||
@@ -144,8 +149,7 @@ export async function prepareReplyAgentPayloads(state: {
|
||||
isMessageToolOnly:
|
||||
(opts?.sourceReplyDeliveryMode ?? followupRun.run.sourceReplyDeliveryMode) ===
|
||||
"message_tool_only",
|
||||
hasPendingContinuation:
|
||||
runResult.meta?.yielded === true || (runResult.meta?.pendingToolCalls?.length ?? 0) > 0,
|
||||
hasPendingContinuation: pendingContinuation,
|
||||
hasExplicitSilentReply: deliberateSilentTerminalReply,
|
||||
hasCommittedDelivery: successfulTerminalDelivery,
|
||||
sessionCtx,
|
||||
|
||||
@@ -14,10 +14,7 @@ import {
|
||||
replaceSessionEntry,
|
||||
} from "../../config/sessions/session-accessor.js";
|
||||
import type { TypingMode } from "../../config/types.js";
|
||||
import {
|
||||
HEARTBEAT_RUN_SCOPE,
|
||||
type ReplyOptionsWithHeartbeatRunScope,
|
||||
} from "../../infra/heartbeat-run-scope.js";
|
||||
import { HEARTBEAT_RUN_SCOPE } from "../../infra/heartbeat-run-scope.js";
|
||||
import {
|
||||
buildHandledBeforeAgentReplyPayloads,
|
||||
runBeforeAgentReplyForTurn,
|
||||
@@ -26,11 +23,11 @@ import { createUserTurnTranscriptRecorder } from "../../sessions/user-turn-trans
|
||||
import { createTestUserTurnTranscriptTarget } from "../../sessions/user-turn-transcript.test-support.js";
|
||||
import type { TemplateContext } from "../templating.js";
|
||||
import { SILENT_REPLY_TOKEN } from "../tokens.js";
|
||||
import type { GetReplyOptions } from "../types.js";
|
||||
import {
|
||||
GENERIC_EXTERNAL_RUN_FAILURE_TEXT,
|
||||
HEARTBEAT_EXTERNAL_RUN_FAILURE_TEXT,
|
||||
} from "./agent-runner-failure-copy.js";
|
||||
import type { InternalGetReplyOptions } from "./get-reply.types.js";
|
||||
import {
|
||||
enqueueFollowupRun,
|
||||
refreshQueuedFollowupSession,
|
||||
@@ -49,10 +46,6 @@ import { consumeReplyUsageState } from "./reply-usage-state.js";
|
||||
import { buildChannelSourceTurnId, setChannelSourceTurnId } from "./source-turn-id.js";
|
||||
import { createMockTypingController } from "./test-helpers.js";
|
||||
|
||||
type ReplyOptionsWithOperationRunState = {
|
||||
[REPLY_OPERATION_RUN_STATE]?: ReplyOperationRunState;
|
||||
};
|
||||
|
||||
type AgentRunParams = {
|
||||
sessionId?: string;
|
||||
sessionFile?: string;
|
||||
@@ -268,7 +261,7 @@ beforeEach(() => {
|
||||
});
|
||||
|
||||
function createMinimalRun(params?: {
|
||||
opts?: GetReplyOptions & ReplyOptionsWithOperationRunState & ReplyOptionsWithHeartbeatRunScope;
|
||||
opts?: InternalGetReplyOptions;
|
||||
resolvedVerboseLevel?: "off" | "on";
|
||||
sessionStore?: Record<string, SessionEntry>;
|
||||
sessionEntry?: SessionEntry;
|
||||
@@ -3690,6 +3683,7 @@ describe("runReplyAgent typing (heartbeat)", () => {
|
||||
it.each([
|
||||
{
|
||||
label: "NO_REPLY",
|
||||
pendingContinuation: false,
|
||||
result: {
|
||||
payloads: [{ text: "NO_REPLY" }],
|
||||
meta: { finalAssistantVisibleText: "NO_REPLY" },
|
||||
@@ -3697,22 +3691,30 @@ describe("runReplyAgent typing (heartbeat)", () => {
|
||||
},
|
||||
{
|
||||
label: "accepted child spawn",
|
||||
pendingContinuation: false,
|
||||
result: {
|
||||
payloads: [],
|
||||
meta: {},
|
||||
acceptedSessionSpawns: [{ runId: "child", childSessionKey: "agent:main:child" }],
|
||||
},
|
||||
},
|
||||
{ label: "yielded continuation", result: { payloads: [], meta: { yielded: true } } },
|
||||
{
|
||||
label: "yielded continuation",
|
||||
pendingContinuation: true,
|
||||
result: { payloads: [], meta: { yielded: true } },
|
||||
},
|
||||
{
|
||||
label: "pending tool continuation",
|
||||
pendingContinuation: true,
|
||||
result: { payloads: [], meta: { pendingToolCalls: [{ name: "hosted_tool" }] } },
|
||||
},
|
||||
])("keeps successful $label completions silent", async ({ result }) => {
|
||||
])("keeps successful $label completions silent", async ({ result, pendingContinuation }) => {
|
||||
state.runEmbeddedAgentMock.mockResolvedValueOnce(result);
|
||||
const { run } = createMinimalRun();
|
||||
const onPendingContinuation = vi.fn();
|
||||
const { run } = createMinimalRun({ opts: { onPendingContinuation } });
|
||||
|
||||
await expect(run()).resolves.toBeUndefined();
|
||||
expect(onPendingContinuation).toHaveBeenCalledTimes(pendingContinuation ? 1 : 0);
|
||||
});
|
||||
|
||||
it.each([
|
||||
|
||||
@@ -4,6 +4,7 @@ import type { ReplySessionBinding } from "./get-reply.types.js";
|
||||
|
||||
export type InternalReplyResolverOptions = {
|
||||
onDeliberateSilentTerminalReply?: () => void;
|
||||
onPendingContinuation?: () => void;
|
||||
onSessionMetadataChanges?: (changes: CommandSessionMetadataChange[]) => void;
|
||||
onSessionPrepared?: (binding: ReplySessionBinding) => void;
|
||||
};
|
||||
|
||||
@@ -66,6 +66,7 @@ export async function executeDispatch(state: PrepareDispatchExecutionReadyState)
|
||||
wrapProgressCallback,
|
||||
} = state;
|
||||
let deliberateSilentTerminalReply = false;
|
||||
let pendingContinuation = false;
|
||||
let didDeliverVisiblePartialReply = false;
|
||||
const replyResult = await runWithDispatchLifecycleAdmission(
|
||||
async () =>
|
||||
@@ -84,6 +85,9 @@ export async function executeDispatch(state: PrepareDispatchExecutionReadyState)
|
||||
onDeliberateSilentTerminalReply: () => {
|
||||
deliberateSilentTerminalReply = true;
|
||||
},
|
||||
onPendingContinuation: () => {
|
||||
pendingContinuation = true;
|
||||
},
|
||||
onSessionMetadataChanges: notifySessionMetadataChanges,
|
||||
onSessionPrepared: state.notePreparedSession,
|
||||
} satisfies InternalReplyResolverOptions),
|
||||
@@ -597,6 +601,7 @@ export async function executeDispatch(state: PrepareDispatchExecutionReadyState)
|
||||
}
|
||||
const nextState = extendPreparedDispatchState(state, {
|
||||
deliberateSilentTerminalReply,
|
||||
pendingContinuation,
|
||||
replyResult,
|
||||
});
|
||||
return { status: "ready" as const, state: nextState };
|
||||
|
||||
@@ -36,6 +36,7 @@ export async function finalizeDispatchAndAudit(state: ExecuteDispatchReadyState)
|
||||
isRoutedReplyDelivered,
|
||||
markInboundDedupeReplayUnsafe,
|
||||
noVisibleReplyFallbackDirected,
|
||||
pendingContinuation,
|
||||
replyResult,
|
||||
replyRoute,
|
||||
routeReplyToOriginating,
|
||||
@@ -273,6 +274,7 @@ export async function finalizeDispatchAndAudit(state: ExecuteDispatchReadyState)
|
||||
state.sourceReplyDeliveryMode !== "message_tool_only" &&
|
||||
!emptyFinalAllowedAsSilent &&
|
||||
!deliberateSilentTerminalReply &&
|
||||
!pendingContinuation &&
|
||||
!getObservedReplyDelivery() &&
|
||||
!replyAcceptedByActiveRun &&
|
||||
!turnLedger.hasVisibleDelivery() &&
|
||||
@@ -288,8 +290,8 @@ export async function finalizeDispatchAndAudit(state: ExecuteDispatchReadyState)
|
||||
}
|
||||
let counts = dispatcher.getQueuedCounts();
|
||||
let noVisibleReplyFallbackDelivered = false;
|
||||
// The agent-result classifier owns terminal silence; carry that fact here
|
||||
// because reply payloads are filtered projections and cannot safely rederive it.
|
||||
// The agent-result classifier owns deliberate silence and pending continuation;
|
||||
// carry those facts here because filtered reply payloads cannot safely rederive either.
|
||||
// An aborted or timed-out settle leaves delivery state unknown; admission
|
||||
// then keeps its legacy trust and the turn ends without a fallback.
|
||||
if (queuedSettleResult === "settled" && noVisibleReplyFallbackAllowed()) {
|
||||
@@ -375,7 +377,8 @@ export async function finalizeDispatchAndAudit(state: ExecuteDispatchReadyState)
|
||||
!getObservedReplyDelivery() &&
|
||||
!replyAcceptedByActiveRun &&
|
||||
!emptyFinalAllowedAsSilent &&
|
||||
!deliberateSilentTerminalReply
|
||||
!deliberateSilentTerminalReply &&
|
||||
!pendingContinuation
|
||||
? { noVisibleReplyFallbackEligible: true }
|
||||
: {}),
|
||||
...(noVisibleReplyFallbackDelivered ? { noVisibleReplyFallbackDelivered: true } : {}),
|
||||
|
||||
@@ -514,6 +514,33 @@ describe("sendPolicy deny — suppress delivery, not processing (#53328)", () =>
|
||||
expect(result.noVisibleReplyFallbackDelivered).toBe(true);
|
||||
});
|
||||
|
||||
it("does not report a pending continuation as an empty terminal reply", async () => {
|
||||
setNoAbort();
|
||||
const dispatcher = createDispatcher();
|
||||
const replyResolver = vi.fn(async (_ctx: MsgContext, opts?: InternalGetReplyOptions) => {
|
||||
opts?.onPendingContinuation?.();
|
||||
return undefined;
|
||||
});
|
||||
|
||||
const result = await dispatchReplyFromConfig({
|
||||
ctx: buildTestCtx({
|
||||
ChatType: "direct",
|
||||
Surface: "telegram",
|
||||
Provider: "telegram",
|
||||
SessionKey: "agent:main:telegram:direct:test",
|
||||
}),
|
||||
cfg: emptyConfig,
|
||||
dispatcher,
|
||||
replyResolver,
|
||||
});
|
||||
|
||||
expect(dispatcher.sendFinalReply).not.toHaveBeenCalled();
|
||||
expect(result).toEqual({
|
||||
queuedFinal: false,
|
||||
counts: { tool: 0, block: 0, final: 0 },
|
||||
});
|
||||
});
|
||||
|
||||
it("delivers core no-visible-reply fallback for disallowed empty mentioned group turns", async () => {
|
||||
setNoAbort();
|
||||
const dispatcher = createDispatcher();
|
||||
|
||||
@@ -18,6 +18,7 @@ export type ReplySessionBinding = {
|
||||
type InternalReplySessionOptions = {
|
||||
expectedExistingSessionId?: string;
|
||||
onDeliberateSilentTerminalReply?: () => void;
|
||||
onPendingContinuation?: () => void;
|
||||
onSessionPrepared?: (binding: ReplySessionBinding) => void;
|
||||
/** Prevent implicit rollover after a caller has durably admitted this exact session. */
|
||||
pinExpectedExistingSession?: boolean;
|
||||
|
||||
Reference in New Issue
Block a user