fix(cron): stop marking silent agentTurn runs as errors after successful tools (#114528)

This commit is contained in:
Peter Steinberger
2026-07-27 07:11:36 -04:00
committed by GitHub
parent a4e524fae3
commit ed4ff90f84
4 changed files with 159 additions and 4 deletions
@@ -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(
@@ -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,
@@ -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,
@@ -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,