From ed4ff90f84785022b3c454e234e4d23c45ce9972 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Mon, 27 Jul 2026 07:11:36 -0400 Subject: [PATCH] fix(cron): stop marking silent agentTurn runs as errors after successful tools (#114528) --- .../run.incomplete-turn.test.ts | 139 ++++++++++++++++++ .../run/attempt-result.ts | 1 + .../run/incomplete-turn.ts | 21 ++- .../run/terminal-resolution.ts | 2 + 4 files changed, 159 insertions(+), 4 deletions(-) diff --git a/src/agents/embedded-agent-runner/run.incomplete-turn.test.ts b/src/agents/embedded-agent-runner/run.incomplete-turn.test.ts index 56ae03b3eb35..949046af360f 100644 --- a/src/agents/embedded-agent-runner/run.incomplete-turn.test.ts +++ b/src/agents/embedded-agent-runner/run.incomplete-turn.test.ts @@ -4649,6 +4649,145 @@ describe("runEmbeddedAgent incomplete-turn safety", () => { expect(result.meta.livenessState).toBe("working"); }); + it("treats reply-optional post-tool empty stops as silent even after side-effecting tools", () => { + // Regression: a cron agentTurn without a delivery route ran a successful + // replay-unsafe sessions patch and intentionally sent no final text; the run + // must finish silent, not as an incomplete-turn error. + const sideEffectToolAttempt = makeAttemptResult({ + assistantTexts: [], + toolMetas: [{ toolName: "sessions", meta: "patch archived", replaySafe: false }], + lastAssistant: { + role: "assistant", + stopReason: "stop", + provider: "openai", + model: "gpt-5.5", + content: [{ type: "text", text: "" }], + } as unknown as EmbeddedRunAttemptResult["lastAssistant"], + }); + + expect( + shouldTreatEmptyAssistantReplyAsSilent({ + allowEmptyAssistantReplyAsSilent: true, + terminalReplyExpectation: "optional", + payloadCount: 0, + aborted: false, + timedOut: false, + attempt: sideEffectToolAttempt, + }), + ).toBe(true); + // A required or unspecified terminal reply keeps the ambiguous-failure path. + expect( + shouldTreatEmptyAssistantReplyAsSilent({ + allowEmptyAssistantReplyAsSilent: true, + terminalReplyExpectation: "required", + payloadCount: 0, + aborted: false, + timedOut: false, + attempt: sideEffectToolAttempt, + }), + ).toBe(false); + expect( + shouldTreatEmptyAssistantReplyAsSilent({ + allowEmptyAssistantReplyAsSilent: true, + payloadCount: 0, + aborted: false, + timedOut: false, + attempt: sideEffectToolAttempt, + }), + ).toBe(false); + }); + + it("keeps reply-optional runs erroring on real failure states", () => { + const toolErrorAttempt = makeAttemptResult({ + assistantTexts: [], + toolMetas: [{ toolName: "sessions", meta: "patch failed", replaySafe: false, isError: true }], + lastToolError: { toolName: "sessions", error: "patch failed" }, + lastAssistant: { + role: "assistant", + stopReason: "stop", + provider: "openai", + model: "gpt-5.5", + content: [{ type: "text", text: "" }], + } as unknown as EmbeddedRunAttemptResult["lastAssistant"], + }); + const errorStopAttempt = makeAttemptResult({ + assistantTexts: [], + toolMetas: [{ toolName: "sessions", meta: "patch archived", replaySafe: false }], + lastAssistant: { + role: "assistant", + stopReason: "error", + provider: "openai", + model: "gpt-5.5", + content: [], + } as unknown as EmbeddedRunAttemptResult["lastAssistant"], + }); + + expect( + shouldTreatEmptyAssistantReplyAsSilent({ + allowEmptyAssistantReplyAsSilent: true, + terminalReplyExpectation: "optional", + payloadCount: 0, + aborted: false, + timedOut: false, + attempt: toolErrorAttempt, + }), + ).toBe(false); + expect( + shouldTreatEmptyAssistantReplyAsSilent({ + allowEmptyAssistantReplyAsSilent: true, + terminalReplyExpectation: "optional", + payloadCount: 0, + aborted: false, + timedOut: false, + attempt: errorStopAttempt, + }), + ).toBe(false); + expect( + shouldTreatEmptyAssistantReplyAsSilent({ + allowEmptyAssistantReplyAsSilent: true, + terminalReplyExpectation: "optional", + payloadCount: 0, + aborted: true, + timedOut: false, + attempt: errorStopAttempt, + }), + ).toBe(false); + }); + + it("returns NO_REPLY for reply-optional cron-style runs whose side-effecting tools succeeded", async () => { + mockedClassifyFailoverReason.mockReturnValue(null); + mockedRunEmbeddedAttempt.mockResolvedValue( + makeAttemptResult({ + assistantTexts: [], + toolMetas: [{ toolName: "sessions", meta: "patch archived", replaySafe: false }], + itemLifecycle: { startedCount: 1, completedCount: 1, activeCount: 0 }, + lastAssistant: { + role: "assistant", + stopReason: "stop", + provider: "openai", + model: "gpt-5.5", + content: [{ type: "text", text: "" }], + } as unknown as EmbeddedRunAttemptResult["lastAssistant"], + }), + ); + + const result = await runEmbeddedAgent({ + ...overflowBaseRunParams, + allowEmptyAssistantReplyAsSilent: true, + terminalReplyExpectation: "optional", + provider: "openai", + model: "gpt-5.5", + runId: "run-reply-optional-post-tool-silent", + }); + + expect(mockedRunEmbeddedAttempt).toHaveBeenCalledTimes(1); + expectNoWarnMessageWith("incomplete turn detected"); + expect(result.payloads).toEqual([{ text: "NO_REPLY" }]); + expect(result.meta.error).toBeUndefined(); + expect(result.meta.terminalReplyKind).toBe("silent-empty"); + expect(result.meta.livenessState).toBe("working"); + }); + it("keeps retrying and surfacing clean empty assistant turns without the silence flag", async () => { mockedClassifyFailoverReason.mockReturnValue(null); mockedRunEmbeddedAttempt.mockResolvedValue( diff --git a/src/agents/embedded-agent-runner/run/attempt-result.ts b/src/agents/embedded-agent-runner/run/attempt-result.ts index 2f89c5ad5543..9dafbfede761 100644 --- a/src/agents/embedded-agent-runner/run/attempt-result.ts +++ b/src/agents/embedded-agent-runner/run/attempt-result.ts @@ -347,6 +347,7 @@ export function completeEmbeddedAttemptResult( (silentToolResultReplyPayload ? 1 : 0); const emptyAssistantReplyIsSilent = shouldTreatEmptyAssistantReplyAsSilent({ allowEmptyAssistantReplyAsSilent: attempt.allowEmptyAssistantReplyAsSilent, + terminalReplyExpectation: attempt.terminalReplyExpectation, payloadCount: 0, aborted: terminal.aborted, timedOut: terminal.timedOut, diff --git a/src/agents/embedded-agent-runner/run/incomplete-turn.ts b/src/agents/embedded-agent-runner/run/incomplete-turn.ts index 0896a7c025c1..b8ee5fbf0ec7 100644 --- a/src/agents/embedded-agent-runner/run/incomplete-turn.ts +++ b/src/agents/embedded-agent-runner/run/incomplete-turn.ts @@ -667,6 +667,8 @@ function shouldSkipNonVisibleTurnRetry(params: { aborted: boolean; timedOut: boolean; attempt: IncompleteTurnAttempt; + /** Reply-optional silent classification tolerates committed side effects; retries never can. */ + tolerateSideEffects?: boolean; }): boolean { return Boolean( params.aborted || @@ -676,7 +678,8 @@ function shouldSkipNonVisibleTurnRetry(params: { params.attempt.didSendDeterministicApprovalPrompt || params.attempt.lastToolError || hasAcceptedSessionSpawn(params.attempt.acceptedSessionSpawns) || - resolveAttemptReplayMetadata(params.attempt).hadPotentialSideEffects, + (params.tolerateSideEffects !== true && + resolveAttemptReplayMetadata(params.attempt).hadPotentialSideEffects), ); } @@ -684,12 +687,21 @@ function shouldSkipNonVisibleTurnRetry(params: { export function shouldTreatEmptyAssistantReplyAsSilent(params: { allowEmptyAssistantReplyAsSilent?: boolean; onlyExplicitSilentReply?: boolean; + terminalReplyExpectation?: "required" | "optional"; payloadCount: number; aborted: boolean; timedOut: boolean; attempt: IncompleteTurnAttempt; }): boolean { - if (!params.allowEmptyAssistantReplyAsSilent || shouldSkipNonVisibleTurnRetry(params)) { + // "optional" is the run consumer's declaration that no user-facing reply is + // owed (e.g. cron without a delivery route). Silence after side-effecting + // tools is intentional there; retry is replay-unsafe, so erroring would mark + // successful tool-only runs as failures. + const terminalReplyOptional = params.terminalReplyExpectation === "optional"; + if ( + !params.allowEmptyAssistantReplyAsSilent || + shouldSkipNonVisibleTurnRetry({ ...params, tolerateSideEffects: terminalReplyOptional }) + ) { return false; } if (hasCommittedMessagingToolDeliveryEvidence(params.attempt)) { @@ -706,9 +718,10 @@ export function shouldTreatEmptyAssistantReplyAsSilent(params: { if (params.onlyExplicitSilentReply) { return false; } - // Post-tool empty stops are ambiguous provider failures, not intentional silence. - // Let the retry/incomplete-turn paths decide whether replay is safe. + // Post-tool empty stops are ambiguous provider failures when a reply is still + // expected; reply-optional runs settle their work in the tools themselves. if ( + !terminalReplyOptional && params.attempt.toolMetas.length > 0 && isEmptyResponseAssistantTurn({ payloadCount: params.payloadCount, diff --git a/src/agents/embedded-agent-runner/run/terminal-resolution.ts b/src/agents/embedded-agent-runner/run/terminal-resolution.ts index 4da5d0419186..538d75a58ddf 100644 --- a/src/agents/embedded-agent-runner/run/terminal-resolution.ts +++ b/src/agents/embedded-agent-runner/run/terminal-resolution.ts @@ -97,6 +97,7 @@ export function resolveSettledTurnFinalizationRequest(input: { : 0; const emptyAssistantReplyIsSilent = shouldTreatEmptyAssistantReplyAsSilent({ allowEmptyAssistantReplyAsSilent: input.runParams.allowEmptyAssistantReplyAsSilent, + terminalReplyExpectation: input.runParams.terminalReplyExpectation, onlyExplicitSilentReply: false, payloadCount, aborted: terminalAborted, @@ -206,6 +207,7 @@ export async function resolveEmbeddedRunTerminal(input: { const settledTurnFinalizationAttempted = input.settledTurnFinalizationAttempted; const emptyAssistantReplyIsSilent = shouldTreatEmptyAssistantReplyAsSilent({ allowEmptyAssistantReplyAsSilent: runParams.allowEmptyAssistantReplyAsSilent, + terminalReplyExpectation: runParams.terminalReplyExpectation, onlyExplicitSilentReply: settledTurnFinalizationAttempted, payloadCount, aborted: terminalAborted,