From 0e58654dba6f4aa7c1d969cd040884b9ac3d2e6e Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sun, 26 Apr 2026 06:25:55 +0100 Subject: [PATCH] fix(agents): silence empty group model turns --- docs/channels/groups.md | 2 +- .../run.incomplete-turn.test.ts | 76 +++++++++++++++++++ .../pi-embedded-runner/run/incomplete-turn.ts | 33 +++++++- .../reply/get-reply-run.media-only.test.ts | 7 +- src/auto-reply/reply/groups.test.ts | 6 +- src/auto-reply/reply/groups.ts | 3 +- 6 files changed, 116 insertions(+), 11 deletions(-) diff --git a/docs/channels/groups.md b/docs/channels/groups.md index 51e0518c134..b977baafc46 100644 --- a/docs/channels/groups.md +++ b/docs/channels/groups.md @@ -272,7 +272,7 @@ Notes: - Surfaces that provide explicit mentions still pass; patterns are a fallback. - Per-agent override: `agents.list[].groupChat.mentionPatterns` (useful when multiple agents share a group). - Mention gating is only enforced when mention detection is possible (native mentions or `mentionPatterns` are configured). -- Always-on groups where silent replies are allowed treat a clean empty model reply as silent, equivalent to `NO_REPLY`. Mention-gated groups and direct chats still treat empty replies as a failed agent turn. +- Groups where silent replies are allowed treat clean empty or reasoning-only model turns as silent, equivalent to `NO_REPLY`. Direct chats still treat empty replies as a failed agent turn. - Discord defaults live in `channels.discord.guilds."*"` (overridable per guild/channel). - Group history context is wrapped uniformly across channels and is **pending-only** (messages skipped due to mention gating); use `messages.groupChat.historyLimit` for the global default and `channels..historyLimit` (or `channels..accounts.*.historyLimit`) for overrides. Set `0` to disable. diff --git a/src/agents/pi-embedded-runner/run.incomplete-turn.test.ts b/src/agents/pi-embedded-runner/run.incomplete-turn.test.ts index 8cacbde3814..b5637314703 100644 --- a/src/agents/pi-embedded-runner/run.incomplete-turn.test.ts +++ b/src/agents/pi-embedded-runner/run.incomplete-turn.test.ts @@ -298,6 +298,44 @@ describe("runEmbeddedPiAgent incomplete-turn safety", () => { ); }); + it("returns NO_REPLY without retrying reasoning-only assistant turns when silence is allowed", async () => { + mockedClassifyFailoverReason.mockReturnValue(null); + mockedRunEmbeddedAttempt.mockResolvedValueOnce( + makeAttemptResult({ + assistantTexts: [], + lastAssistant: { + role: "assistant", + stopReason: "end_turn", + provider: "openai-codex", + model: "gpt-5.5", + content: [ + { + type: "thinking", + thinking: "internal reasoning", + thinkingSignature: JSON.stringify({ id: "rs_silent_group", type: "reasoning" }), + }, + ], + } as unknown as EmbeddedRunAttemptResult["lastAssistant"], + }), + ); + + const result = await runEmbeddedPiAgent({ + ...overflowBaseRunParams, + allowEmptyAssistantReplyAsSilent: true, + provider: "openai-codex", + model: "gpt-5.5", + runId: "run-reasoning-only-silent", + }); + + expect(mockedRunEmbeddedAttempt).toHaveBeenCalledTimes(1); + expect(mockedLog.warn).not.toHaveBeenCalledWith( + expect.stringContaining("reasoning-only assistant turn detected"), + ); + expect(result.payloads).toEqual([{ text: "NO_REPLY" }]); + expect(result.meta.terminalReplyKind).toBe("silent-empty"); + expect(result.meta.livenessState).toBe("working"); + }); + it("does not retry or warn on reasoning-only turns when a messaging tool already delivered", async () => { mockedClassifyFailoverReason.mockReturnValue(null); mockedRunEmbeddedAttempt.mockResolvedValueOnce( @@ -1184,6 +1222,44 @@ describe("runEmbeddedPiAgent incomplete-turn safety", () => { ).toBe(false); }); + it("treats reasoning-only assistant turns as silent only when the caller allows it", () => { + const attempt = makeAttemptResult({ + assistantTexts: [], + lastAssistant: { + role: "assistant", + stopReason: "end_turn", + provider: "openai-codex", + model: "gpt-5.5", + content: [ + { + type: "thinking", + thinking: "internal reasoning", + thinkingSignature: JSON.stringify({ id: "rs_silent_helper", type: "reasoning" }), + }, + ], + } as unknown as EmbeddedRunAttemptResult["lastAssistant"], + }); + + expect( + shouldTreatEmptyAssistantReplyAsSilent({ + allowEmptyAssistantReplyAsSilent: true, + payloadCount: 0, + aborted: false, + timedOut: false, + attempt, + }), + ).toBe(true); + expect( + shouldTreatEmptyAssistantReplyAsSilent({ + allowEmptyAssistantReplyAsSilent: false, + payloadCount: 0, + aborted: false, + timedOut: false, + attempt, + }), + ).toBe(false); + }); + it("does not treat error or side-effect empty turns as silent", () => { const errorAttempt = makeAttemptResult({ assistantTexts: [], diff --git a/src/agents/pi-embedded-runner/run/incomplete-turn.ts b/src/agents/pi-embedded-runner/run/incomplete-turn.ts index 2cd0a052e0b..e8837d5c3d8 100644 --- a/src/agents/pi-embedded-runner/run/incomplete-turn.ts +++ b/src/agents/pi-embedded-runner/run/incomplete-turn.ts @@ -329,6 +329,37 @@ function isEmptyResponseAssistantTurn(params: { return true; } +function isNonVisibleAssistantTurnEligibleForSilentReply(params: { + payloadCount: number; + attempt: Pick< + IncompleteTurnAttempt, + "assistantTexts" | "currentAttemptAssistant" | "lastAssistant" + >; +}): boolean { + if (isEmptyResponseAssistantTurn(params)) { + return true; + } + if (params.payloadCount !== 0) { + return false; + } + if (params.attempt.assistantTexts.join("\n\n").trim().length > 0) { + return false; + } + const assistant = params.attempt.currentAttemptAssistant ?? params.attempt.lastAssistant; + if (!assistant || assistant.stopReason === "error") { + return false; + } + if ( + isIncompleteTerminalAssistantTurn({ + hasAssistantVisibleText: false, + lastAssistant: assistant, + }) + ) { + return false; + } + return isReasoningOnlyAssistantTurn(assistant); +} + function shouldSkipPlanningOnlyRetry(params: { aborted: boolean; timedOut: boolean; @@ -358,7 +389,7 @@ export function shouldTreatEmptyAssistantReplyAsSilent(params: { if (hasCommittedUserVisibleToolDelivery(params.attempt)) { return false; } - return isEmptyResponseAssistantTurn({ + return isNonVisibleAssistantTurnEligibleForSilentReply({ payloadCount: params.payloadCount, attempt: params.attempt, }); diff --git a/src/auto-reply/reply/get-reply-run.media-only.test.ts b/src/auto-reply/reply/get-reply-run.media-only.test.ts index 57e0e0f4d5b..e36e6e9234e 100644 --- a/src/auto-reply/reply/get-reply-run.media-only.test.ts +++ b/src/auto-reply/reply/get-reply-run.media-only.test.ts @@ -90,8 +90,7 @@ vi.mock("./groups.js", () => ({ return { activation, canUseSilentReply, - allowEmptyAssistantReplyAsSilent: - activation === "always" && params.silentReplyPolicy === "allow", + allowEmptyAssistantReplyAsSilent: params.silentReplyPolicy === "allow", }; }, ), @@ -284,7 +283,7 @@ describe("runPreparedReply media-only handling", () => { ); }); - it("propagates empty-assistant silence only for always-on group runs", async () => { + it("propagates non-visible assistant silence for group runs", async () => { await runPreparedReply(baseParams()); let call = vi.mocked(runReplyAgent).mock.calls.at(-1)?.[0]; @@ -297,7 +296,7 @@ describe("runPreparedReply media-only handling", () => { ); call = vi.mocked(runReplyAgent).mock.calls.at(-1)?.[0]; - expect(call?.followupRun.run.allowEmptyAssistantReplyAsSilent).toBe(false); + expect(call?.followupRun.run.allowEmptyAssistantReplyAsSilent).toBe(true); }); it("does not propagate empty-assistant silence for direct runs", async () => { diff --git a/src/auto-reply/reply/groups.test.ts b/src/auto-reply/reply/groups.test.ts index d4cf882c7fa..bd229fa4491 100644 --- a/src/auto-reply/reply/groups.test.ts +++ b/src/auto-reply/reply/groups.test.ts @@ -124,7 +124,7 @@ describe("group runtime loading", () => { expect(rewritten).not.toContain("Be extremely selective"); }); - it("marks empty assistant replies silent only for always-on groups with silence allowed", async () => { + it("marks non-visible assistant replies silent for groups with silence allowed", async () => { const groups = await import("./groups.js"); expect( @@ -139,7 +139,7 @@ describe("group runtime loading", () => { defaultActivation: "mention", silentReplyPolicy: "allow", }).allowEmptyAssistantReplyAsSilent, - ).toBe(false); + ).toBe(true); expect( groups.resolveGroupSilentReplyBehavior({ @@ -147,7 +147,7 @@ describe("group runtime loading", () => { defaultActivation: "always", silentReplyPolicy: "allow", }).allowEmptyAssistantReplyAsSilent, - ).toBe(false); + ).toBe(true); expect( groups.resolveGroupSilentReplyBehavior({ diff --git a/src/auto-reply/reply/groups.ts b/src/auto-reply/reply/groups.ts index ddad8d372d6..ac3524219fe 100644 --- a/src/auto-reply/reply/groups.ts +++ b/src/auto-reply/reply/groups.ts @@ -269,8 +269,7 @@ export function resolveGroupSilentReplyBehavior(params: { return { activation, canUseSilentReply, - allowEmptyAssistantReplyAsSilent: - activation === "always" && params.silentReplyPolicy === "allow", + allowEmptyAssistantReplyAsSilent: params.silentReplyPolicy === "allow", }; }