mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-28 05:16:23 -06:00
fix: surface cron errors for empty isolated agent replies (#100229)
* fix(cron): fail empty isolated agent completions * fix: surface cron errors for empty isolated agent replies * fix(cron): preserve intentional terminal progress --------- Co-authored-by: Peter Steinberger <steipete@gmail.com> Co-authored-by: Peter Steinberger <peter@steipete.me>
This commit is contained in:
@@ -4,7 +4,10 @@ import { makeIsolatedAgentJobFixture, makeIsolatedAgentParamsFixture } from "./j
|
||||
import { setupRunCronIsolatedAgentTurnSuite } from "./run.suite-helpers.js";
|
||||
import {
|
||||
cleanupDirectCronSessionMock,
|
||||
dispatchCronDeliveryMock,
|
||||
loadRunCronIsolatedAgentTurn,
|
||||
resolveCronDeliveryPlanMock,
|
||||
resolveCronPayloadOutcomeMock,
|
||||
runWithModelFallbackMock,
|
||||
} from "./run.test-harness.js";
|
||||
|
||||
@@ -94,6 +97,277 @@ describe("runCronIsolatedAgentTurn - meta.error status propagation", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("marks a completed embedded run with no final payload as a cron error", async () => {
|
||||
runWithModelFallbackMock.mockResolvedValueOnce({
|
||||
result: {
|
||||
payloads: [],
|
||||
meta: {
|
||||
agentMeta: { usage: { input: 10, output: 0 } },
|
||||
},
|
||||
},
|
||||
provider: "anthropic",
|
||||
model: "claude-opus-4-8",
|
||||
attempts: [],
|
||||
});
|
||||
resolveCronDeliveryPlanMock.mockReturnValue({
|
||||
requested: true,
|
||||
mode: "announce",
|
||||
channel: "messagechat",
|
||||
to: "test-target",
|
||||
});
|
||||
resolveCronPayloadOutcomeMock.mockReturnValue({
|
||||
summary: undefined,
|
||||
outputText: undefined,
|
||||
synthesizedText: undefined,
|
||||
deliveryPayload: undefined,
|
||||
deliveryPayloads: [],
|
||||
deliveryPayloadHasStructuredContent: false,
|
||||
hasFatalErrorPayload: false,
|
||||
hasFatalStructuredErrorPayload: false,
|
||||
embeddedRunError: undefined,
|
||||
});
|
||||
|
||||
const result = await runCronIsolatedAgentTurn(makeIsolatedAgentParamsFixture());
|
||||
|
||||
expect(dispatchCronDeliveryMock).not.toHaveBeenCalled();
|
||||
expect(result.status).toBe("error");
|
||||
expect(result.error).toBe("cron isolated run completed without a final assistant payload");
|
||||
expect(result.delivered).toBe(false);
|
||||
});
|
||||
|
||||
it("marks empty message-tool attempts without source delivery as cron errors", async () => {
|
||||
runWithModelFallbackMock.mockResolvedValueOnce({
|
||||
result: {
|
||||
payloads: [],
|
||||
didSendViaMessagingTool: true,
|
||||
messagingToolSentTargets: [],
|
||||
meta: {
|
||||
agentMeta: { usage: { input: 10, output: 0 } },
|
||||
},
|
||||
},
|
||||
provider: "anthropic",
|
||||
model: "claude-opus-4-8",
|
||||
attempts: [],
|
||||
});
|
||||
resolveCronDeliveryPlanMock.mockReturnValue({
|
||||
requested: true,
|
||||
mode: "announce",
|
||||
channel: "messagechat",
|
||||
to: "test-target",
|
||||
});
|
||||
resolveCronPayloadOutcomeMock.mockReturnValue({
|
||||
summary: undefined,
|
||||
outputText: undefined,
|
||||
synthesizedText: undefined,
|
||||
deliveryPayload: undefined,
|
||||
deliveryPayloads: [],
|
||||
deliveryPayloadHasStructuredContent: false,
|
||||
hasFatalErrorPayload: false,
|
||||
hasFatalStructuredErrorPayload: false,
|
||||
embeddedRunError: undefined,
|
||||
});
|
||||
|
||||
const result = await runCronIsolatedAgentTurn(makeIsolatedAgentParamsFixture());
|
||||
|
||||
expect(dispatchCronDeliveryMock).not.toHaveBeenCalled();
|
||||
expect(result.status).toBe("error");
|
||||
expect(result.error).toBe("cron isolated run completed without a final assistant payload");
|
||||
expect(result.delivered).toBe(false);
|
||||
});
|
||||
|
||||
it("keeps explicit silent replies as successful cron completions", async () => {
|
||||
runWithModelFallbackMock.mockResolvedValueOnce({
|
||||
result: {
|
||||
payloads: [],
|
||||
meta: {
|
||||
finalAssistantRawText: "NO_REPLY",
|
||||
finalAssistantVisibleText: "NO_REPLY",
|
||||
agentMeta: { usage: { input: 10, output: 1 } },
|
||||
},
|
||||
},
|
||||
provider: "anthropic",
|
||||
model: "claude-opus-4-8",
|
||||
attempts: [],
|
||||
});
|
||||
resolveCronDeliveryPlanMock.mockReturnValue({
|
||||
requested: true,
|
||||
mode: "announce",
|
||||
channel: "messagechat",
|
||||
to: "test-target",
|
||||
});
|
||||
resolveCronPayloadOutcomeMock.mockReturnValue({
|
||||
summary: undefined,
|
||||
outputText: undefined,
|
||||
synthesizedText: undefined,
|
||||
deliveryPayload: undefined,
|
||||
deliveryPayloads: [],
|
||||
deliveryPayloadHasStructuredContent: false,
|
||||
hasFatalErrorPayload: false,
|
||||
hasFatalStructuredErrorPayload: false,
|
||||
embeddedRunError: undefined,
|
||||
});
|
||||
|
||||
const result = await runCronIsolatedAgentTurn(makeIsolatedAgentParamsFixture());
|
||||
|
||||
expect(dispatchCronDeliveryMock).toHaveBeenCalled();
|
||||
expect(result.status).toBe("ok");
|
||||
expect(result.error).toBeUndefined();
|
||||
});
|
||||
|
||||
it("keeps committed message-tool deliveries as successful cron completions", async () => {
|
||||
runWithModelFallbackMock.mockResolvedValueOnce({
|
||||
result: {
|
||||
payloads: [],
|
||||
didSendViaMessagingTool: true,
|
||||
messagingToolSentTexts: ["Delivered to an intentional recipient"],
|
||||
messagingToolSentTargets: [],
|
||||
meta: {
|
||||
agentMeta: { usage: { input: 10, output: 0 } },
|
||||
},
|
||||
},
|
||||
provider: "anthropic",
|
||||
model: "claude-opus-4-8",
|
||||
attempts: [],
|
||||
});
|
||||
resolveCronDeliveryPlanMock.mockReturnValue({
|
||||
requested: true,
|
||||
mode: "announce",
|
||||
channel: "messagechat",
|
||||
to: "test-target",
|
||||
});
|
||||
resolveCronPayloadOutcomeMock.mockReturnValue({
|
||||
summary: undefined,
|
||||
outputText: undefined,
|
||||
synthesizedText: undefined,
|
||||
deliveryPayload: undefined,
|
||||
deliveryPayloads: [],
|
||||
deliveryPayloadHasStructuredContent: false,
|
||||
hasFatalErrorPayload: false,
|
||||
hasFatalStructuredErrorPayload: false,
|
||||
embeddedRunError: undefined,
|
||||
});
|
||||
|
||||
const result = await runCronIsolatedAgentTurn(makeIsolatedAgentParamsFixture());
|
||||
|
||||
expect(dispatchCronDeliveryMock).toHaveBeenCalled();
|
||||
expect(result.status).toBe("ok");
|
||||
expect(result.error).toBeUndefined();
|
||||
});
|
||||
|
||||
it("does not mark empty deterministic approval prompts as cron errors", async () => {
|
||||
runWithModelFallbackMock.mockResolvedValueOnce({
|
||||
result: {
|
||||
payloads: [],
|
||||
didSendDeterministicApprovalPrompt: true,
|
||||
meta: {
|
||||
agentMeta: { usage: { input: 10, output: 0 } },
|
||||
},
|
||||
},
|
||||
provider: "anthropic",
|
||||
model: "claude-opus-4-8",
|
||||
attempts: [],
|
||||
});
|
||||
resolveCronDeliveryPlanMock.mockReturnValue({
|
||||
requested: true,
|
||||
mode: "announce",
|
||||
channel: "messagechat",
|
||||
to: "test-target",
|
||||
});
|
||||
resolveCronPayloadOutcomeMock.mockReturnValue({
|
||||
summary: undefined,
|
||||
outputText: undefined,
|
||||
synthesizedText: undefined,
|
||||
deliveryPayload: undefined,
|
||||
deliveryPayloads: [],
|
||||
deliveryPayloadHasStructuredContent: false,
|
||||
hasFatalErrorPayload: false,
|
||||
hasFatalStructuredErrorPayload: false,
|
||||
embeddedRunError: undefined,
|
||||
});
|
||||
|
||||
const result = await runCronIsolatedAgentTurn(makeIsolatedAgentParamsFixture());
|
||||
|
||||
expect(dispatchCronDeliveryMock).toHaveBeenCalled();
|
||||
expect(result.status).toBe("ok");
|
||||
expect(result.error).toBeUndefined();
|
||||
});
|
||||
|
||||
it("does not mark empty accepted child-session handoffs as cron errors", async () => {
|
||||
runWithModelFallbackMock.mockResolvedValueOnce({
|
||||
result: {
|
||||
payloads: [],
|
||||
acceptedSessionSpawns: [{ runId: "run-child", childSessionKey: "agent:default:child" }],
|
||||
meta: {
|
||||
agentMeta: { usage: { input: 10, output: 0 } },
|
||||
},
|
||||
},
|
||||
provider: "anthropic",
|
||||
model: "claude-opus-4-8",
|
||||
attempts: [],
|
||||
});
|
||||
resolveCronDeliveryPlanMock.mockReturnValue({
|
||||
requested: true,
|
||||
mode: "announce",
|
||||
channel: "messagechat",
|
||||
to: "test-target",
|
||||
});
|
||||
resolveCronPayloadOutcomeMock.mockReturnValue({
|
||||
summary: undefined,
|
||||
outputText: undefined,
|
||||
synthesizedText: undefined,
|
||||
deliveryPayload: undefined,
|
||||
deliveryPayloads: [],
|
||||
deliveryPayloadHasStructuredContent: false,
|
||||
hasFatalErrorPayload: false,
|
||||
hasFatalStructuredErrorPayload: false,
|
||||
embeddedRunError: undefined,
|
||||
});
|
||||
|
||||
const result = await runCronIsolatedAgentTurn(makeIsolatedAgentParamsFixture());
|
||||
|
||||
expect(dispatchCronDeliveryMock).toHaveBeenCalled();
|
||||
expect(result.status).toBe("ok");
|
||||
expect(result.error).toBeUndefined();
|
||||
});
|
||||
|
||||
it("does not mark empty successful cron-add completions as cron errors", async () => {
|
||||
runWithModelFallbackMock.mockResolvedValueOnce({
|
||||
result: {
|
||||
payloads: [],
|
||||
successfulCronAdds: 1,
|
||||
meta: {
|
||||
agentMeta: { usage: { input: 10, output: 0 } },
|
||||
},
|
||||
},
|
||||
provider: "anthropic",
|
||||
model: "claude-opus-4-8",
|
||||
attempts: [],
|
||||
});
|
||||
resolveCronDeliveryPlanMock.mockReturnValue({
|
||||
requested: true,
|
||||
mode: "announce",
|
||||
channel: "messagechat",
|
||||
to: "test-target",
|
||||
});
|
||||
resolveCronPayloadOutcomeMock.mockReturnValue({
|
||||
summary: undefined,
|
||||
outputText: undefined,
|
||||
synthesizedText: undefined,
|
||||
deliveryPayload: undefined,
|
||||
deliveryPayloads: [],
|
||||
deliveryPayloadHasStructuredContent: false,
|
||||
hasFatalErrorPayload: false,
|
||||
hasFatalStructuredErrorPayload: false,
|
||||
embeddedRunError: undefined,
|
||||
});
|
||||
|
||||
const result = await runCronIsolatedAgentTurn(makeIsolatedAgentParamsFixture());
|
||||
|
||||
expect(dispatchCronDeliveryMock).toHaveBeenCalled();
|
||||
expect(result.status).toBe("ok");
|
||||
expect(result.error).toBeUndefined();
|
||||
});
|
||||
|
||||
it("surfaces cron timeout result when the cron-nested lane watchdog fires", async () => {
|
||||
runWithModelFallbackMock.mockRejectedValueOnce(
|
||||
makeCommandLaneTaskTimeoutError("cron-nested", 330_000),
|
||||
|
||||
@@ -1,7 +1,9 @@
|
||||
import { isDeepStrictEqual } from "node:util";
|
||||
import { normalizeOptionalString } from "@openclaw/normalization-core/string-coerce";
|
||||
import { hasAcceptedSessionSpawn } from "../../agents/accepted-session-spawn.js";
|
||||
import { retireSessionMcpRuntime } from "../../agents/agent-bundle-mcp-tools.js";
|
||||
import { hasAnyAuthProfileStoreSource } from "../../agents/auth-profiles/source-check.js";
|
||||
import { hasCommittedMessagingToolDeliveryEvidence } from "../../agents/embedded-agent-runner/delivery-evidence.js";
|
||||
import { findModelInCatalog } from "../../agents/model-catalog-lookup.js";
|
||||
import type { ModelCatalogEntry } from "../../agents/model-catalog.types.js";
|
||||
import { listOpenAIAuthProfileProvidersForAgentRuntime } from "../../agents/openai-routing.js";
|
||||
@@ -9,6 +11,7 @@ import { createAgentRunRestartAbortError } from "../../agents/run-termination.js
|
||||
import { expandToolGroups, normalizeToolName } from "../../agents/tool-policy.js";
|
||||
import { deriveContextPromptTokens } from "../../agents/usage.js";
|
||||
import type { ThinkLevel } from "../../auto-reply/thinking.js";
|
||||
import { isSilentReplyPayloadText } from "../../auto-reply/tokens.js";
|
||||
import type { CliDeps } from "../../cli/outbound-send-deps.js";
|
||||
import { resolveAgentModelPrimaryValue } from "../../config/model-input.js";
|
||||
import type { SessionEntry } from "../../config/sessions.js";
|
||||
@@ -1442,6 +1445,39 @@ async function finalizeCronRun(params: {
|
||||
sourceDeliveryOutcome,
|
||||
});
|
||||
}
|
||||
const hasCommittedTerminalProgress =
|
||||
hasCommittedMessagingToolDeliveryEvidence(finalRunResult) ||
|
||||
finalRunResult.didSendDeterministicApprovalPrompt === true ||
|
||||
hasAcceptedSessionSpawn(finalRunResult.acceptedSessionSpawns) ||
|
||||
(finalRunResult.successfulCronAdds ?? 0) > 0;
|
||||
const hasIntentionalSilentReply =
|
||||
finalRunResult.meta?.terminalReplyKind === "silent-empty" ||
|
||||
isSilentReplyPayloadText(finalRunResult.meta?.finalAssistantRawText) ||
|
||||
isSilentReplyPayloadText(finalRunResult.meta?.finalAssistantVisibleText);
|
||||
if (
|
||||
prepared.deliveryRequested &&
|
||||
!hasFatalErrorPayload &&
|
||||
!sourceDeliveryOutcome.satisfiesSourceDelivery &&
|
||||
!hasCommittedTerminalProgress &&
|
||||
!hasIntentionalSilentReply &&
|
||||
deliveryPayloads.length === 0 &&
|
||||
normalizeOptionalString(synthesizedText) === undefined
|
||||
) {
|
||||
const error = "cron isolated run completed without a final assistant payload";
|
||||
return prepared.withRunSession({
|
||||
status: "error",
|
||||
error,
|
||||
summary: error,
|
||||
outputText: error,
|
||||
delivered: false,
|
||||
deliveryAttempted: false,
|
||||
diagnostics: mergeCronRunDiagnostics(
|
||||
runDiagnostics,
|
||||
createCronRunDiagnosticsFromError("agent-run", error),
|
||||
),
|
||||
...telemetry,
|
||||
});
|
||||
}
|
||||
if (hasFatalStructuredErrorPayload && prepared.deliveryRequested) {
|
||||
// Structured run error payloads belong in cron state and failure alerts,
|
||||
// not the normal completion announce path where provider JSON can leak.
|
||||
|
||||
Reference in New Issue
Block a user