From d4d2c55d800441f65946dcd3d022a97674ac10c7 Mon Sep 17 00:00:00 2001 From: Vijay Kumar Date: Mon, 10 Aug 2026 20:09:27 +0530 Subject: [PATCH 1/4] fix: recover empty completions and export long responses --- .env.example | 3 + PRODUCT.md | 1 + README.md | 2 + .../managers/summary-aggregation-manager.ts | 90 ++++++++++++- src/bot/commands/definitions.ts | 1 + src/bot/commands/lastfile-command.ts | 28 ++++ src/bot/handlers/prompt.ts | 123 ++++++++++++++++-- src/bot/routers/command-router.ts | 2 + .../assistant-response-export-service.ts | 72 ++++++++++ src/bot/services/empty-completion-policy.ts | 34 +++++ .../services/event-subscription-service.ts | 57 +++++++- src/config.ts | 4 + src/i18n/ar.ts | 8 ++ src/i18n/de.ts | 10 ++ src/i18n/en.ts | 9 ++ src/i18n/es.ts | 9 ++ src/i18n/fr.ts | 10 ++ src/i18n/it.ts | 10 ++ src/i18n/pt.ts | 10 ++ src/i18n/ru.ts | 8 ++ src/i18n/zh.ts | 7 + .../summary-aggregation-manager.test.ts | 90 +++++++++++++ tests/bot/commands/lastfile.test.ts | 57 ++++++++ tests/bot/handlers/prompt.test.ts | 21 +++ tests/bot/routers/command-router.test.ts | 1 + .../assistant-response-export-service.test.ts | 42 ++++++ .../services/empty-completion-policy.test.ts | 45 +++++++ 27 files changed, 738 insertions(+), 16 deletions(-) create mode 100644 src/bot/commands/lastfile-command.ts create mode 100644 src/bot/services/assistant-response-export-service.ts create mode 100644 src/bot/services/empty-completion-policy.ts create mode 100644 tests/bot/commands/lastfile.test.ts create mode 100644 tests/bot/services/assistant-response-export-service.test.ts create mode 100644 tests/bot/services/empty-completion-policy.test.ts diff --git a/.env.example b/.env.example index ebeacf780..d0c0fc297 100644 --- a/.env.example +++ b/.env.example @@ -90,6 +90,9 @@ OPENCODE_MODEL_ID=big-pickle # Higher value = fewer Telegram edit requests, lower value = more real-time updates # RESPONSE_STREAM_THROTTLE_MS=1000 +# Automatically attach assistant responses longer than this many characters as Markdown (default: 5000) +# ASSISTANT_RESPONSE_FILE_THRESHOLD=5000 + # Maximum displayed length for bash tool commands in Telegram summaries (default: 128) # Longer commands are truncated with "..." # BASH_TOOL_DISPLAY_MAX_LENGTH=128 diff --git a/PRODUCT.md b/PRODUCT.md index f4d535187..a03324fc7 100644 --- a/PRODUCT.md +++ b/PRODUCT.md @@ -105,6 +105,7 @@ No public inbound ports are required for normal usage. Current command set: - `/status` - server, project, and session status +- `/lastfile` - export the latest delivered assistant response as Markdown - `/new` - create a new session - `/abort` - stop the current task - `/detach` - detach the bot from the current session without stopping it diff --git a/README.md b/README.md index 5257c4e5f..6eaaaed5d 100644 --- a/README.md +++ b/README.md @@ -135,6 +135,7 @@ opencode-telegram config | Command | Description | | ----------------- | ------------------------------------------------------- | | `/status` | Server health, current project, session, and model info | +| `/lastfile` | Export the latest delivered assistant response as Markdown | | `/new` | Create a new session | | `/abort` | Abort the current task | | `/detach` | Detach from the current session without stopping it | @@ -235,6 +236,7 @@ Configuration can be provided through process environment variables or an `.env` | `BASH_TOOL_DISPLAY_MAX_LENGTH` | Maximum displayed length for `bash` tool commands in Telegram summaries; longer commands are truncated | No | `128` | | `TRACK_BACKGROUND_SESSIONS` | Track detached/non-current sessions in the current selected project/worktree and send short notifications | No | `true` | | `RESPONSE_STREAM_THROTTLE_MS` | Stream update throttle in milliseconds for assistant, thinking, and tool message edits | No | `1000` | +| `ASSISTANT_RESPONSE_FILE_THRESHOLD` | Automatically attach assistant replies longer than this many characters as Markdown | No | `5000` | | `MESSAGE_FORMAT_MODE` | Assistant reply formatting mode: `markdown` (native Telegram rich blocks) or `raw` (plain text) | No | `markdown` | | `MESSAGE_MERGE_WINDOW_MS` | Merge Telegram-split long text messages into one prompt after this wait window (ms); `0` disables merging | No | `1500` | | `INITIAL_SETTINGS_PRESET` | JSON object that seeds default `/settings` values on first run (keys not yet persisted); see [Runtime Settings](#runtime-settings) | No | `{}` | diff --git a/src/app/managers/summary-aggregation-manager.ts b/src/app/managers/summary-aggregation-manager.ts index 7bf72ffcc..02018b0ab 100644 --- a/src/app/managers/summary-aggregation-manager.ts +++ b/src/app/managers/summary-aggregation-manager.ts @@ -23,6 +23,11 @@ export interface MessageCompletionInfo { modelID?: string; createdAt?: number; completedAt?: number; + finishReason?: string; + tokens?: TokensInfo; + cost?: number; + hasToolActivity: boolean; + hasReasoningActivity: boolean; } type MessageCompleteCallback = ( @@ -183,6 +188,17 @@ interface TextMessageState { optimisticUpdateCount: number; } +interface MessageActivityState { + finishReason?: string; + hasToolActivity: boolean; + hasReasoningActivity: boolean; +} + +interface PendingEmptyCompletion { + messageId: string; + info: MessageCompletionInfo; +} + interface ThinkingMessageState { orderedPartIds: string[]; sections: Map; @@ -324,6 +340,8 @@ class SummaryAggregator { private typingIndicatorEnabled = true; private partHashes: Map> = new Map(); private trackedSessionParents: Map = new Map(); + private messageActivityStates: Map = new Map(); + private pendingEmptyCompletions: Map = new Map(); private subagentStates: Map = new Map(); private subagentOrder: string[] = []; private subagentCardIdBySessionId: Map = new Map(); @@ -556,6 +574,8 @@ class SummaryAggregator { this.partHashes.clear(); this.knownTextPartIds.clear(); this.syntheticPartIds.clear(); + this.messageActivityStates.clear(); + this.pendingEmptyCompletions.clear(); this.processedToolStates.clear(); this.thinkingFiredForMessages.clear(); this.thinkingFinishedForMessages.clear(); @@ -1160,6 +1180,7 @@ class SummaryAggregator { const time = info.time; const isCompleted = Boolean(time?.completed); const messageText = this.getCombinedMessageText(messageID, isCompleted); + const activity = this.getOrCreateMessageActivityState(messageID); if (!isCompleted && textState.optimisticUpdateCount === 1) { this.emitPartialText(info.sessionID, messageID, messageText); @@ -1187,6 +1208,29 @@ class SummaryAggregator { if (isCompleted) { const finalText = messageText; + const completionInfo: MessageCompletionInfo = { + agent: info.agent, + providerID: info.providerID, + modelID: info.modelID, + createdAt: time?.created, + completedAt: time?.completed, + finishReason: + typeof info.finish === "string" && info.finish.trim() + ? info.finish.trim() + : activity.finishReason, + tokens: info.tokens + ? { + input: info.tokens.input, + output: info.tokens.output, + reasoning: info.tokens.reasoning, + cacheRead: info.tokens.cache?.read || 0, + cacheWrite: info.tokens.cache?.write || 0, + } + : undefined, + cost: typeof info.cost === "number" ? info.cost : undefined, + hasToolActivity: activity.hasToolActivity, + hasReasoningActivity: activity.hasReasoningActivity, + }; logger.debug( `[Aggregator] Message part completed: messageId=${messageID}, textLength=${finalText.length}, totalParts=${textState.orderedPartIds.length}, session=${this.currentSessionId}`, @@ -1209,13 +1253,15 @@ class SummaryAggregator { this.onCostCallback(assistantInfo.cost); } - if (this.onCompleteCallback && finalText.length > 0) { + if (this.onCompleteCallback && finalText.trim().length > 0) { + this.pendingEmptyCompletions.delete(this.currentSessionId!); this.onCompleteCallback(this.currentSessionId!, messageID, finalText, { - agent: info.agent, - providerID: info.providerID, - modelID: info.modelID, - createdAt: time?.created, - completedAt: time?.completed, + ...completionInfo, + }); + } else if (finalText.trim().length === 0) { + this.pendingEmptyCompletions.set(this.currentSessionId!, { + messageId: messageID, + info: completionInfo, }); } @@ -1287,6 +1333,7 @@ class SummaryAggregator { const messageID = part.messageID; const messageInfo = this.messages.get(messageID); + const activity = this.getOrCreateMessageActivityState(messageID); // OpenCode injects synthetic text parts of its own: expanded file attachments, // MCP resource dumps, plan-mode hints. They are context for the model, never content @@ -1303,6 +1350,7 @@ class SummaryAggregator { } if (part.type === "reasoning") { + activity.hasReasoningActivity = true; this.registerThinkingPart( messageID, part.id, @@ -1386,6 +1434,7 @@ class SummaryAggregator { } } } else if (part.type === "tool") { + activity.hasToolActivity = true; const state = part.state; const input = state.input; const title = "title" in state ? state.title : undefined; @@ -1489,6 +1538,10 @@ class SummaryAggregator { } } + if (part.type === "step-finish" && typeof part.reason === "string" && part.reason.trim()) { + activity.finishReason = part.reason.trim(); + } + this.lastUpdated = Date.now(); } @@ -1818,6 +1871,20 @@ class SummaryAggregator { return state; } + private getOrCreateMessageActivityState(messageID: string): MessageActivityState { + const existing = this.messageActivityStates.get(messageID); + if (existing) { + return existing; + } + + const state: MessageActivityState = { + hasToolActivity: false, + hasReasoningActivity: false, + }; + this.messageActivityStates.set(messageID, state); + return state; + } + private registerKnownTextPart(messageID: string, partID: string): void { if (!this.knownTextPartIds.has(messageID)) { this.knownTextPartIds.set(messageID, new Set()); @@ -2052,6 +2119,17 @@ class SummaryAggregator { logger.info(`[Aggregator] Session became idle: ${sessionID}`); + const pendingEmptyCompletion = this.pendingEmptyCompletions.get(sessionID); + this.pendingEmptyCompletions.delete(sessionID); + if (pendingEmptyCompletion && this.onCompleteCallback) { + this.onCompleteCallback( + sessionID, + pendingEmptyCompletion.messageId, + "", + pendingEmptyCompletion.info, + ); + } + // Stop typing indicator when session goes idle this.stopTypingIndicator(); diff --git a/src/bot/commands/definitions.ts b/src/bot/commands/definitions.ts index 324393833..cac5590c8 100644 --- a/src/bot/commands/definitions.ts +++ b/src/bot/commands/definitions.ts @@ -22,6 +22,7 @@ interface BotCommandI18nDefinition { */ const COMMAND_DEFINITIONS: BotCommandI18nDefinition[] = [ { command: "status", descriptionKey: "cmd.description.status" }, + { command: "lastfile", descriptionKey: "cmd.description.lastfile" }, { command: "new", descriptionKey: "cmd.description.new" }, { command: "abort", descriptionKey: "cmd.description.stop" }, { command: "detach", descriptionKey: "cmd.description.detach" }, diff --git a/src/bot/commands/lastfile-command.ts b/src/bot/commands/lastfile-command.ts new file mode 100644 index 000000000..d90e4b460 --- /dev/null +++ b/src/bot/commands/lastfile-command.ts @@ -0,0 +1,28 @@ +import { Context } from "grammy"; +import { getCurrentSession } from "../../app/services/session-service.js"; +import { t } from "../../i18n/index.js"; +import { + getRememberedAssistantResponse, + sendAssistantResponseDocument, +} from "../services/assistant-response-export-service.js"; + +export async function lastfileCommand(ctx: Context): Promise { + const chatId = ctx.chat?.id; + const sessionId = getCurrentSession()?.id; + if (!chatId || !sessionId) { + await ctx.reply(t("bot.lastfile_empty")); + return; + } + + const response = getRememberedAssistantResponse(chatId, sessionId); + if (!response) { + await ctx.reply(t("bot.lastfile_empty")); + return; + } + + try { + await sendAssistantResponseDocument(ctx.api, chatId, response); + } catch { + await ctx.reply(t("bot.lastfile_error")); + } +} diff --git a/src/bot/handlers/prompt.ts b/src/bot/handlers/prompt.ts index 803b9b0e7..80598f9a6 100644 --- a/src/bot/handlers/prompt.ts +++ b/src/bot/handlers/prompt.ts @@ -39,6 +39,27 @@ let botInstance: Bot | null = null; let chatIdInstance: number | null = null; const promptResponseModes = new Map(); +export interface PromptDispatchOptions { + sessionID: string; + directory: string; + parts: Array; + model?: { providerID: string; modelID: string }; + agent?: string; + variant?: string; +} + +interface PromptRetryState { + bot: Bot; + chatId: number; + promptOptions: PromptDispatchOptions; + promptText: string; + responseMode: PromptResponseMode; + attempted: boolean; + retryIdlePending: boolean; +} + +const promptRetryStates = new Map(); + export type PromptResponseMode = "text_only" | "text_and_tts"; type ProcessPromptOptions = { @@ -67,6 +88,85 @@ export function consumePromptResponseMode(sessionId: string): PromptResponseMode return responseMode; } +export function registerPromptRetry( + sessionId: string, + state: Omit, +): void { + promptRetryStates.set(sessionId, { + ...state, + attempted: false, + retryIdlePending: false, + }); +} + +export function clearPromptRetry(sessionId: string): void { + promptRetryStates.delete(sessionId); +} + +export function hasPromptRetryAttempted(sessionId: string): boolean { + return promptRetryStates.get(sessionId)?.attempted ?? false; +} + +export function consumePromptRetryIdle(sessionId: string): boolean { + const state = promptRetryStates.get(sessionId); + if (!state?.retryIdlePending) { + return false; + } + + state.retryIdlePending = false; + return true; +} + +export function retryPromptOnce(sessionId: string): boolean { + const state = promptRetryStates.get(sessionId); + if (!state || state.attempted) { + return false; + } + + state.attempted = true; + state.retryIdlePending = true; + foregroundSessionState.markBusy(sessionId, state.promptOptions.directory); + void markAttachedSessionBusy(sessionId); + assistantRunState.startRun(sessionId, { + startedAt: Date.now(), + configuredAgent: state.promptOptions.agent, + configuredProviderID: state.promptOptions.model?.providerID, + configuredModelID: state.promptOptions.model?.modelID, + }); + setPromptResponseMode(sessionId, state.responseMode); + if (state.promptText.trim().length > 0) { + externalUserInputSuppressionManager.register(sessionId, state.promptText); + } + + safeBackgroundTask({ + taskName: "session.promptAsync.retry", + task: () => opencodeClient.session.promptAsync(state.promptOptions), + onSuccess: ({ error }) => { + if (!error) { + logger.info(`[Bot] Automatic empty-completion retry accepted: session=${sessionId}`); + return; + } + + clearPromptRetry(sessionId); + foregroundSessionState.markIdle(sessionId); + void markAttachedSessionIdle(sessionId); + assistantRunState.clearRun(sessionId, "session_prompt_retry_api_error"); + clearPromptResponseMode(sessionId); + void state.bot.api.sendMessage(state.chatId, t("bot.prompt_send_error")).catch(() => {}); + }, + onError: () => { + clearPromptRetry(sessionId); + foregroundSessionState.markIdle(sessionId); + void markAttachedSessionIdle(sessionId); + assistantRunState.clearRun(sessionId, "session_prompt_retry_background_error"); + clearPromptResponseMode(sessionId); + void state.bot.api.sendMessage(state.chatId, t("bot.prompt_send_error")).catch(() => {}); + }, + }); + + return true; +} + async function isSessionBusy(sessionId: string, directory: string): Promise { try { const { data, error } = await opencodeClient.session.status({ directory }); @@ -95,6 +195,10 @@ async function resetMismatchedSessionContext(): Promise { summaryAggregator.clear(); foregroundSessionState.clearAll("session_mismatch_reset"); assistantRunState.clearAll("session_mismatch_reset"); + const currentSession = getCurrentSession(); + if (currentSession) { + clearPromptRetry(currentSession.id); + } clearAllInteractionState("session_mismatch_reset"); clearSession(); keyboardManager.clearContext(); @@ -287,14 +391,7 @@ export async function processUserPrompt( // above and would otherwise be missing from the logs. const filePartCount = parts.filter((part) => part.type === "file").length; - const promptOptions: { - sessionID: string; - directory: string; - parts: Array; - model?: { providerID: string; modelID: string }; - agent?: string; - variant?: string; - } = { + const promptOptions: PromptDispatchOptions = { sessionID: currentSession.id, directory: currentSession.directory, parts, @@ -338,6 +435,13 @@ export async function processUserPrompt( configuredModelID: storedModel.modelID, }); setPromptResponseMode(currentSession.id, responseMode); + registerPromptRetry(currentSession.id, { + bot, + chatId: ctx.chat!.id, + promptOptions, + promptText: text, + responseMode, + }); if (text.trim().length > 0) { externalUserInputSuppressionManager.register(currentSession.id, text); @@ -353,6 +457,7 @@ export async function processUserPrompt( task: () => opencodeClient.session.promptAsync(promptOptions), onSuccess: ({ error }) => { if (error) { + clearPromptRetry(currentSession.id); foregroundSessionState.markIdle(currentSession.id); void markAttachedSessionIdle(currentSession.id); assistantRunState.clearRun(currentSession.id, "session_prompt_api_error"); @@ -373,6 +478,7 @@ export async function processUserPrompt( logger.info("[Bot] session.promptAsync accepted"); }, onError: (error) => { + clearPromptRetry(currentSession.id); foregroundSessionState.markIdle(currentSession.id); void markAttachedSessionIdle(currentSession.id); assistantRunState.clearRun(currentSession.id, "session_prompt_background_error"); @@ -391,6 +497,7 @@ export async function processUserPrompt( foregroundSessionState.markIdle(currentSession.id); await markAttachedSessionIdle(currentSession.id); assistantRunState.clearRun(currentSession.id, "session_prompt_handler_error"); + clearPromptRetry(currentSession.id); } logger.error("Error in prompt handler:", err); if (interactionManager.getSnapshot()) { diff --git a/src/bot/routers/command-router.ts b/src/bot/routers/command-router.ts index 4e2faf1cc..32ab86871 100644 --- a/src/bot/routers/command-router.ts +++ b/src/bot/routers/command-router.ts @@ -21,6 +21,7 @@ import { mcpsCommand } from "../commands/mcp-catalog-command.js"; import { startCommand } from "../commands/start-command.js"; import { helpCommand } from "../commands/help-command.js"; import { statusCommand } from "../commands/status-command.js"; +import { lastfileCommand } from "../commands/lastfile-command.js"; import { BOT_COMMANDS } from "../commands/definitions.js"; import { logger } from "../../utils/logger.js"; import { flushPendingPrompt } from "../handlers/message-merger.js"; @@ -71,6 +72,7 @@ export function registerCommandRouter(bot: Bot, deps: CommandRouterDeps bot.command("start", startCommand); bot.command("help", helpCommand); bot.command("status", statusCommand); + bot.command("lastfile", lastfileCommand); bot.command("settings", settingsCommand); bot.command("opencode_start", opencodeStartCommand); bot.command("opencode_stop", opencodeStopCommand); diff --git a/src/bot/services/assistant-response-export-service.ts b/src/bot/services/assistant-response-export-service.ts new file mode 100644 index 000000000..c513f8479 --- /dev/null +++ b/src/bot/services/assistant-response-export-service.ts @@ -0,0 +1,72 @@ +import { InputFile } from "grammy"; +import { config } from "../../config.js"; + +const MAX_CACHED_RESPONSES = 128; + +export interface AssistantResponseExportApi { + sendDocument: (chatId: number, document: InputFile) => Promise; +} + +interface CachedAssistantResponse { + chatId: number; + sessionId: string; + text: string; +} + +const cachedResponses = new Map(); + +function getCacheKey(chatId: number, sessionId: string): string { + return `${chatId}:${sessionId}`; +} + +function createResponseFilename(now: Date): string { + const pad = (value: number): string => String(value).padStart(2, "0"); + + return `opencode-response-${now.getFullYear()}-${pad(now.getMonth() + 1)}-${pad(now.getDate())}-${pad(now.getHours())}${pad(now.getMinutes())}.md`; +} + +export function createAssistantResponseDocument( + text: string, + now: Date = new Date(), +): { filename: string; buffer: Buffer } { + return { + filename: createResponseFilename(now), + buffer: Buffer.from(text, "utf8"), + }; +} + +export function rememberAssistantResponse(chatId: number, sessionId: string, text: string): void { + const key = getCacheKey(chatId, sessionId); + cachedResponses.delete(key); + cachedResponses.set(key, { chatId, sessionId, text }); + + while (cachedResponses.size > MAX_CACHED_RESPONSES) { + const oldestKey = cachedResponses.keys().next().value; + if (!oldestKey) { + break; + } + + cachedResponses.delete(oldestKey); + } +} + +export function getRememberedAssistantResponse(chatId: number, sessionId: string): string | null { + return cachedResponses.get(getCacheKey(chatId, sessionId))?.text ?? null; +} + +export async function sendAssistantResponseDocument( + api: AssistantResponseExportApi, + chatId: number, + text: string, +): Promise { + const { filename, buffer } = createAssistantResponseDocument(text); + await api.sendDocument(chatId, new InputFile(buffer, filename)); +} + +export function shouldAutomaticallyExportAssistantResponse(text: string): boolean { + return text.length > config.bot.assistantResponseFileThreshold; +} + +export function __resetAssistantResponseExportsForTests(): void { + cachedResponses.clear(); +} diff --git a/src/bot/services/empty-completion-policy.ts b/src/bot/services/empty-completion-policy.ts new file mode 100644 index 000000000..0547b8641 --- /dev/null +++ b/src/bot/services/empty-completion-policy.ts @@ -0,0 +1,34 @@ +import type { MessageCompletionInfo } from "../../app/managers/summary-aggregation-manager.js"; + +function isZero(value: number | undefined): boolean { + return value !== undefined && Number.isFinite(value) && value === 0; +} + +function hasNoTokenUsage(info: MessageCompletionInfo): boolean { + return ( + isZero(info.tokens?.input) && + isZero(info.tokens?.output) && + isZero(info.tokens?.reasoning) && + isZero(info.tokens?.cacheRead) && + isZero(info.tokens?.cacheWrite) + ); +} + +function hasUnknownFinishReason(finishReason: string | undefined): boolean { + const normalized = finishReason?.trim().toLowerCase() ?? ""; + return normalized.length === 0 || ["unknown", "invalid", "none", "null"].includes(normalized); +} + +export function isGenuinelyEmptyAssistantResponse(messageText: string): boolean { + return messageText.trim().length === 0; +} + +export function isSafeZeroWorkEmptyCompletion(info: MessageCompletionInfo): boolean { + return ( + hasNoTokenUsage(info) && + isZero(info.cost) && + !info.hasToolActivity && + !info.hasReasoningActivity && + hasUnknownFinishReason(info.finishReason) + ); +} diff --git a/src/bot/services/event-subscription-service.ts b/src/bot/services/event-subscription-service.ts index bf613939f..15cc4d159 100644 --- a/src/bot/services/event-subscription-service.ts +++ b/src/bot/services/event-subscription-service.ts @@ -33,7 +33,13 @@ import { logger } from "../../utils/logger.js"; import { safeBackgroundTask } from "../../utils/safe-background-task.js"; import { pinnedMessageManager } from "../pinned/pinned-message-manager.js"; import { keyboardManager } from "../keyboards/keyboard-manager.js"; -import { clearPromptResponseMode } from "../handlers/prompt.js"; +import { + clearPromptResponseMode, + clearPromptRetry, + consumePromptRetryIdle, + hasPromptRetryAttempted, + retryPromptOnce, +} from "../handlers/prompt.js"; import { reconcileBusyState, setPromptResponseModeClearerForReconciliation, @@ -90,6 +96,15 @@ import { interactionManager, } from "../../app/managers/interaction-manager.js"; import { stopEventListening, subscribeToEvents } from "../../opencode/events.js"; +import { + rememberAssistantResponse, + sendAssistantResponseDocument, + shouldAutomaticallyExportAssistantResponse, +} from "./assistant-response-export-service.js"; +import { + isGenuinelyEmptyAssistantResponse, + isSafeZeroWorkEmptyCompletion, +} from "./empty-completion-policy.js"; const TELEGRAM_DOCUMENT_CAPTION_MAX_LENGTH = 1024; const RESPONSE_STREAM_THROTTLE_MS = config.bot.responseStreamThrottleMs; @@ -601,6 +616,25 @@ class EventSubscriptionService implements BotEventSubscriptionService { const chatId = this.chatIdInstance; try { + if (isGenuinelyEmptyAssistantResponse(messageText)) { + this.clearAssistantResponseStream(sessionId, messageId, "empty_completion"); + this.clearThinkingStream(sessionId, messageId, "empty_completion"); + this.compactProgressStreamer.clearSession(sessionId, "empty_completion"); + + if (isSafeZeroWorkEmptyCompletion(completionInfo) && retryPromptOnce(sessionId)) { + await botApi.sendMessage(chatId, t("bot.empty_completion_retry")); + } else if (hasPromptRetryAttempted(sessionId)) { + clearPromptRetry(sessionId); + await botApi.sendMessage(chatId, t("bot.empty_completion_failed")); + } else { + clearPromptRetry(sessionId); + await botApi.sendMessage(chatId, t("bot.empty_completion_no_retry")); + } + + return; + } + + clearPromptRetry(sessionId); assistantRunState.markResponseCompleted(sessionId, { agent: completionInfo.agent, providerID: completionInfo.providerID, @@ -651,6 +685,16 @@ class EventSubscriptionService implements BotEventSubscriptionService { }, }); + rememberAssistantResponse(chatId, sessionId, messageText); + if (shouldAutomaticallyExportAssistantResponse(messageText)) { + await sendAssistantResponseDocument(botApi, chatId, messageText).catch((error) => { + logger.warn( + `[Bot] Failed to send automatic Markdown response export: session=${sessionId}`, + error, + ); + }); + } + await sendTtsResponseForSession({ api: botApi, sessionId, @@ -1105,12 +1149,20 @@ class EventSubscriptionService implements BotEventSubscriptionService { }); summaryAggregator.setOnSessionIdle(async (sessionId) => { - await markAttachedSessionIdle(sessionId); // Cleared unconditionally: a session can go idle after it stopped being // the current one, and the early returns below would leak the tracker. this.clearToolElapsedState(sessionId, "session_idle"); await this.sessionCompletionTasks.get(sessionId)?.catch(() => undefined); + if (consumePromptRetryIdle(sessionId)) { + logger.debug( + `[Bot] Ignoring idle event that closed the retried empty completion: session=${sessionId}`, + ); + return; + } + + await markAttachedSessionIdle(sessionId); + const completedRun = assistantRunState.finishRun(sessionId, "session_idle"); clearPromptResponseMode(sessionId); @@ -1165,6 +1217,7 @@ class EventSubscriptionService implements BotEventSubscriptionService { summaryAggregator.setOnSessionError(async (sessionId, message) => { await markAttachedSessionIdle(sessionId); this.clearToolElapsedState(sessionId, "session_error"); + clearPromptRetry(sessionId); if (!this.botInstance || !this.chatIdInstance) { clearPromptResponseMode(sessionId); diff --git a/src/config.ts b/src/config.ts index c5d37ae45..e03df823e 100644 --- a/src/config.ts +++ b/src/config.ts @@ -222,6 +222,10 @@ export const config = { false, ), responseStreamThrottleMs: getOptionalPositiveIntEnvVar("RESPONSE_STREAM_THROTTLE_MS", 1000), + assistantResponseFileThreshold: getOptionalPositiveIntEnvVar( + "ASSISTANT_RESPONSE_FILE_THRESHOLD", + 5000, + ), bashToolDisplayMaxLength: getOptionalPositiveIntEnvVar("BASH_TOOL_DISPLAY_MAX_LENGTH", 128), locale: getOptionalLocaleEnvVar("BOT_LOCALE", "en"), trackBackgroundSessions: getOptionalBooleanEnvVar("TRACK_BACKGROUND_SESSIONS", true), diff --git a/src/i18n/ar.ts b/src/i18n/ar.ts index 22b6895fb..f8b5c08a2 100644 --- a/src/i18n/ar.ts +++ b/src/i18n/ar.ts @@ -9,6 +9,7 @@ import type { I18nDictionary } from "./en.js"; */ export const ar: I18nDictionary = { "cmd.description.status": "عرض حالة الخادم والجلسة", + "cmd.description.lastfile": "تصدير آخر رد للمساعد", "cmd.description.new": "بدء جلسة جديدة", "cmd.description.stop": "إيقاف المهمة الحالية", "cmd.description.detach": "الخروج من الجلسة دون إيقافها", @@ -86,6 +87,13 @@ export const ar: I18nDictionary = { "bot.session_reset_project_mismatch": "⚠️ الجلسة النشطة مرتبطة بمشروع مختلف، لذلك تمت إعادة ضبطها. استخدم /sessions لاختيار جلسة أو /new لبدء جلسة جديدة.", "bot.prompt_send_error": "تعذر إرسال الطلب إلى OpenCode.", + "bot.empty_completion_retry": "⚠️ أعاد OpenCode إكمالًا فارغًا.\nجارٍ إعادة المحاولة مرة واحدة…", + "bot.empty_completion_failed": + "⚠️ انتهى OpenCode دون رد صالح بعد إعادة محاولة واحدة. لن تتم إعادة المحاولة.", + "bot.empty_completion_no_retry": + "⚠️ انتهى OpenCode دون رد صالح.\nلم تتم إعادة المحاولة تلقائيًا لأن المهمة ربما نفذت عملًا بالفعل. أعد المحاولة يدويًا.", + "bot.lastfile_empty": "لا يوجد رد مساعد مُسلّم بنجاح لهذه الجلسة.", + "bot.lastfile_error": "⚠️ تعذر تصدير آخر رد للمساعد.", "bot.session_error": "🔴 أعاد OpenCode الخطأ التالي: {message}", "bot.session_retry": "🔁 {message}\n\nاستمر مزوّد الخدمة في إرجاع الخطأ نفسه بعد عدة محاولات. استخدم /abort لإيقاف المهمة.", diff --git a/src/i18n/de.ts b/src/i18n/de.ts index aba736c1e..d1b03901a 100644 --- a/src/i18n/de.ts +++ b/src/i18n/de.ts @@ -2,6 +2,7 @@ import type { I18nDictionary } from "./en.js"; export const de: I18nDictionary = { "cmd.description.status": "Server- und Sitzungsstatus", + "cmd.description.lastfile": "Letzte Assistentenantwort exportieren", "cmd.description.new": "Neue Sitzung erstellen", "cmd.description.stop": "Aktuelle Aktion stoppen", "cmd.description.detach": "Von aktueller Sitzung trennen", @@ -86,6 +87,15 @@ export const de: I18nDictionary = { "bot.session_reset_project_mismatch": "⚠️ Die aktive Sitzung passt nicht zum ausgewählten Projekt und wurde daher zurückgesetzt. Nutze /sessions zur Auswahl oder /new, um eine neue Sitzung zu erstellen.", "bot.prompt_send_error": "Anfrage konnte nicht an OpenCode gesendet werden.", + "bot.empty_completion_retry": + "⚠️ OpenCode hat eine leere Antwort geliefert.\nEin erneuter Versuch wird einmal ausgeführt…", + "bot.empty_completion_failed": + "⚠️ OpenCode endete auch nach einem erneuten Versuch ohne brauchbare Antwort. Kein weiterer Versuch.", + "bot.empty_completion_no_retry": + "⚠️ OpenCode endete ohne brauchbare Antwort.\nKein automatischer Versuch, da die Aufgabe möglicherweise bereits Änderungen ausgeführt hat. Bitte manuell erneut versuchen.", + "bot.lastfile_empty": + "Für diese Sitzung ist keine erfolgreich zugestellte Assistentenantwort verfügbar.", + "bot.lastfile_error": "⚠️ Die letzte Assistentenantwort konnte nicht exportiert werden.", "bot.session_error": "🔴 OpenCode meldete einen Fehler: {message}", "bot.session_retry": "🔁 {message}\n\nDer Provider liefert bei wiederholten Versuchen immer wieder denselben Fehler. Mit /abort abbrechen.", diff --git a/src/i18n/en.ts b/src/i18n/en.ts index ba0a444cb..cbbfbe80d 100644 --- a/src/i18n/en.ts +++ b/src/i18n/en.ts @@ -1,5 +1,6 @@ export const en = { "cmd.description.status": "Server and session status", + "cmd.description.lastfile": "Export the last assistant response", "cmd.description.new": "Create a new session", "cmd.description.stop": "Stop current action", "cmd.description.detach": "Detach from current session", @@ -83,6 +84,14 @@ export const en = { "bot.session_reset_project_mismatch": "⚠️ Active session does not match the selected project, so it was reset. Use /sessions to pick one or /new to create a new session.", "bot.prompt_send_error": "Failed to send request to OpenCode.", + "bot.empty_completion_retry": "⚠️ OpenCode returned an empty completion.\nRetrying once…", + "bot.empty_completion_failed": + "⚠️ OpenCode ended without a usable response after one retry. No further retry was attempted.", + "bot.empty_completion_no_retry": + "⚠️ OpenCode ended without a usable response.\nNo automatic retry was attempted because the task may already have performed work. Please retry manually.", + "bot.lastfile_empty": + "No successfully delivered assistant response is available for this session.", + "bot.lastfile_error": "⚠️ Failed to export the last assistant response.", "bot.session_error": "🔴 OpenCode returned an error: {message}", "bot.session_retry": "🔁 {message}\n\nProvider keeps returning the same error on repeated retries. Use /abort to abort.", diff --git a/src/i18n/es.ts b/src/i18n/es.ts index f9f453f2c..a385d294c 100644 --- a/src/i18n/es.ts +++ b/src/i18n/es.ts @@ -2,6 +2,7 @@ import type { I18nDictionary } from "./en.js"; export const es: I18nDictionary = { "cmd.description.status": "Estado del servidor y de la sesión", + "cmd.description.lastfile": "Exportar la última respuesta del asistente", "cmd.description.new": "Crear una sesión nueva", "cmd.description.stop": "Detener la acción actual", "cmd.description.detach": "Desconectar de la sesión actual", @@ -87,6 +88,14 @@ export const es: I18nDictionary = { "bot.session_reset_project_mismatch": "⚠️ La sesión activa no coincide con el proyecto seleccionado, así que se reinició. Usa /sessions para elegir una o /new para crear una nueva.", "bot.prompt_send_error": "No se pudo enviar la solicitud a OpenCode.", + "bot.empty_completion_retry": "⚠️ OpenCode devolvió una respuesta vacía.\nReintentando una vez…", + "bot.empty_completion_failed": + "⚠️ OpenCode terminó sin una respuesta utilizable después de un reintento. No habrá más reintentos.", + "bot.empty_completion_no_retry": + "⚠️ OpenCode terminó sin una respuesta utilizable.\nNo se reintentó automáticamente porque la tarea quizá ya realizó cambios. Inténtalo de nuevo manualmente.", + "bot.lastfile_empty": + "No hay una respuesta del asistente entregada correctamente para esta sesión.", + "bot.lastfile_error": "⚠️ No se pudo exportar la última respuesta del asistente.", "bot.session_error": "🔴 OpenCode devolvió un error: {message}", "bot.session_retry": "🔁 {message}\n\nEl proveedor devuelve el mismo error en intentos repetidos. Usa /abort para detenerlo.", diff --git a/src/i18n/fr.ts b/src/i18n/fr.ts index e7e17e629..f5387652c 100644 --- a/src/i18n/fr.ts +++ b/src/i18n/fr.ts @@ -2,6 +2,7 @@ import type { I18nDictionary } from "./en.js"; export const fr: I18nDictionary = { "cmd.description.status": "Statut du serveur et de la session", + "cmd.description.lastfile": "Exporter la dernière réponse de l’assistant", "cmd.description.new": "Créer une nouvelle session", "cmd.description.stop": "Arrêter l'action en cours", "cmd.description.detach": "Se détacher de la session actuelle", @@ -86,6 +87,15 @@ export const fr: I18nDictionary = { "bot.session_reset_project_mismatch": "⚠️ La session active ne correspond pas au projet sélectionné, elle a donc été réinitialisée. Utilisez /sessions pour en choisir une ou /new pour créer une nouvelle session.", "bot.prompt_send_error": "Impossible d'envoyer la requête à OpenCode.", + "bot.empty_completion_retry": + "⚠️ OpenCode a renvoyé une réponse vide.\nNouvelle tentative unique…", + "bot.empty_completion_failed": + "⚠️ OpenCode s’est terminé sans réponse exploitable après une nouvelle tentative. Aucune autre tentative.", + "bot.empty_completion_no_retry": + "⚠️ OpenCode s’est terminé sans réponse exploitable.\nAucune nouvelle tentative automatique, car la tâche a peut-être déjà effectué des changements. Réessayez manuellement.", + "bot.lastfile_empty": + "Aucune réponse de l’assistant livrée avec succès n’est disponible pour cette session.", + "bot.lastfile_error": "⚠️ Échec de l’export de la dernière réponse de l’assistant.", "bot.session_error": "🔴 OpenCode a renvoyé une erreur : {message}", "bot.session_retry": "🔁 {message}\n\nLe fournisseur renvoie la même erreur à chaque nouvelle tentative. Utilisez /abort pour arrêter.", diff --git a/src/i18n/it.ts b/src/i18n/it.ts index 09e88d3a7..ffbc524a3 100644 --- a/src/i18n/it.ts +++ b/src/i18n/it.ts @@ -2,6 +2,7 @@ import type { I18nDictionary } from "./en.js"; export const it: I18nDictionary = { "cmd.description.status": "Stato del server e della sessione", + "cmd.description.lastfile": "Esporta l'ultima risposta dell'assistente", "cmd.description.new": "Crea una nuova sessione", "cmd.description.stop": "Interrompi l'azione corrente", "cmd.description.detach": "Scollega la sessione corrente", @@ -88,6 +89,15 @@ export const it: I18nDictionary = { "bot.session_reset_project_mismatch": "⚠️ La sessione attiva non corrisponde al progetto selezionato, quindi è stata reimpostata. Usa /sessions per sceglierne una o /new per crearne una nuova.", "bot.prompt_send_error": "Invio della richiesta a OpenCode non riuscito.", + "bot.empty_completion_retry": + "⚠️ OpenCode ha restituito una risposta vuota.\nNuovo tentativo, una sola volta…", + "bot.empty_completion_failed": + "⚠️ OpenCode è terminato senza una risposta utilizzabile dopo un nuovo tentativo. Nessun altro tentativo.", + "bot.empty_completion_no_retry": + "⚠️ OpenCode è terminato senza una risposta utilizzabile.\nNessun nuovo tentativo automatico: l'attività potrebbe aver già eseguito modifiche. Riprova manualmente.", + "bot.lastfile_empty": + "Non è disponibile alcuna risposta dell'assistente consegnata correttamente per questa sessione.", + "bot.lastfile_error": "⚠️ Impossibile esportare l'ultima risposta dell'assistente.", "bot.session_error": "🔴 OpenCode ha restituito un errore: {message}", "bot.session_retry": "🔁 {message}\n\nIl provider restituisce sempre lo stesso errore dopo ripetuti tentativi. Usa /abort per annullare.", diff --git a/src/i18n/pt.ts b/src/i18n/pt.ts index f0adda4bb..0a66d37d6 100644 --- a/src/i18n/pt.ts +++ b/src/i18n/pt.ts @@ -2,6 +2,7 @@ import type { I18nDictionary } from "./en.js"; export const pt: I18nDictionary = { "cmd.description.status": "Status do servidor e da sessão", + "cmd.description.lastfile": "Exportar a última resposta do assistente", "cmd.description.new": "Criar uma nova sessão", "cmd.description.stop": "Parar a ação atual", "cmd.description.detach": "Desconectar da sessão atual", @@ -86,6 +87,15 @@ export const pt: I18nDictionary = { "bot.session_reset_project_mismatch": "⚠️ A sessão ativa não corresponde ao projeto selecionado, então ela foi redefinida. Use /sessions para escolher uma ou /new para criar uma nova sessão.", "bot.prompt_send_error": "Não foi possível enviar a solicitação ao OpenCode.", + "bot.empty_completion_retry": + "⚠️ O OpenCode retornou uma conclusão vazia.\nTentando novamente uma vez…", + "bot.empty_completion_failed": + "⚠️ O OpenCode terminou sem uma resposta utilizável após uma nova tentativa. Nenhuma outra tentativa será feita.", + "bot.empty_completion_no_retry": + "⚠️ O OpenCode terminou sem uma resposta utilizável.\nNenhuma nova tentativa automática: a tarefa pode já ter realizado alterações. Tente novamente manualmente.", + "bot.lastfile_empty": + "Não há uma resposta do assistente entregue com sucesso disponível para esta sessão.", + "bot.lastfile_error": "⚠️ Não foi possível exportar a última resposta do assistente.", "bot.session_error": "🔴 O OpenCode retornou um erro: {message}", "bot.session_retry": "🔁 {message}\n\nO provedor continua retornando o mesmo erro nas novas tentativas. Use /abort para abortar.", diff --git a/src/i18n/ru.ts b/src/i18n/ru.ts index b3780e8ae..8ff80d512 100644 --- a/src/i18n/ru.ts +++ b/src/i18n/ru.ts @@ -2,6 +2,7 @@ import type { I18nDictionary } from "./en.js"; export const ru: I18nDictionary = { "cmd.description.status": "Статус сервера и сессии", + "cmd.description.lastfile": "Экспортировать последний ответ ассистента", "cmd.description.new": "Создать новую сессию", "cmd.description.stop": "Прервать текущее действие", "cmd.description.detach": "Отсоединиться от текущей сессии", @@ -82,6 +83,13 @@ export const ru: I18nDictionary = { "bot.session_reset_project_mismatch": "⚠️ Активная сессия не соответствует выбранному проекту, поэтому была сброшена. Используйте /sessions для выбора или /new для создания новой сессии.", "bot.prompt_send_error": "Не удалось отправить запрос в OpenCode.", + "bot.empty_completion_retry": "⚠️ OpenCode вернул пустой ответ.\nПовторяю один раз…", + "bot.empty_completion_failed": + "⚠️ OpenCode завершил работу без пригодного ответа после одной повторной попытки. Дальше повторов не будет.", + "bot.empty_completion_no_retry": + "⚠️ OpenCode завершил работу без пригодного ответа.\nАвтоматический повтор не выполнен, поскольку задача могла уже внести изменения. Повторите вручную.", + "bot.lastfile_empty": "Для этой сессии нет успешно доставленного ответа ассистента.", + "bot.lastfile_error": "⚠️ Не удалось экспортировать последний ответ ассистента.", "bot.session_error": "🔴 OpenCode вернул ошибку: {message}", "bot.session_retry": "🔁 {message}\n\nПровайдер возвращает одну и ту же ошибку при повторных запросах. Используйте /abort для остановки.", diff --git a/src/i18n/zh.ts b/src/i18n/zh.ts index 3b511d873..bd7428e00 100644 --- a/src/i18n/zh.ts +++ b/src/i18n/zh.ts @@ -2,6 +2,7 @@ import type { I18nDictionary } from "./en.js"; export const zh: I18nDictionary = { "cmd.description.status": "服务器和会话状态", + "cmd.description.lastfile": "导出助手的最后一条回复", "cmd.description.new": "创建新会话", "cmd.description.stop": "停止当前操作", "cmd.description.detach": "从当前会话分离", @@ -75,6 +76,12 @@ export const zh: I18nDictionary = { "bot.session_reset_project_mismatch": "⚠️ 活动会话与所选项目不匹配，因此已重置。使用 /sessions 选择一个会话，或 /new 创建新会话。", "bot.prompt_send_error": "向 OpenCode 发送请求失败。", + "bot.empty_completion_retry": "⚠️ OpenCode 返回了空完成结果。\n将自动重试一次…", + "bot.empty_completion_failed": "⚠️ OpenCode 重试一次后仍未返回可用内容。不会继续重试。", + "bot.empty_completion_no_retry": + "⚠️ OpenCode 结束时没有可用回复。\n未自动重试，因为任务可能已经执行了操作。请手动重试。", + "bot.lastfile_empty": "当前会话没有可导出的已成功发送的助手回复。", + "bot.lastfile_error": "⚠️ 导出助手最后一条回复失败。", "bot.session_error": "🔴 OpenCode 返回错误：{message}", "bot.session_retry": "🔁 {message}\n\n提供方在重复重试时持续返回同一错误。使用 /abort 可停止。", "bot.external_user_input": "外部用户输入", diff --git a/tests/app/managers/summary-aggregation-manager.test.ts b/tests/app/managers/summary-aggregation-manager.test.ts index fb3d41f4c..9b143247d 100644 --- a/tests/app/managers/summary-aggregation-manager.test.ts +++ b/tests/app/managers/summary-aggregation-manager.test.ts @@ -2144,6 +2144,96 @@ describe("summary/aggregator", () => { expect(onComplete).not.toHaveBeenCalled(); }); + it("reports an empty completed response at session idle with final metadata", () => { + const onComplete = vi.fn(); + summaryAggregator.setOnComplete(onComplete); + summaryAggregator.setSession("session-empty-final"); + + summaryAggregator.processEvent({ + type: "message.updated", + properties: { + info: { + id: "message-empty-final", + sessionID: "session-empty-final", + role: "assistant", + finish: "unknown", + cost: 0, + tokens: { + input: 0, + output: 0, + reasoning: 0, + cache: { read: 0, write: 0 }, + }, + time: { created: 1 }, + }, + }, + } as unknown as Event); + + for (const type of ["step-start", "step-finish"] as const) { + summaryAggregator.processEvent({ + type: "message.part.updated", + properties: { + part: { + id: `${type}-1`, + sessionID: "session-empty-final", + messageID: "message-empty-final", + type, + ...(type === "step-finish" + ? { + reason: "unknown", + cost: 0, + tokens: { + input: 0, + output: 0, + reasoning: 0, + cache: { read: 0, write: 0 }, + }, + } + : {}), + }, + }, + } as unknown as Event); + } + + summaryAggregator.processEvent({ + type: "message.updated", + properties: { + info: { + id: "message-empty-final", + sessionID: "session-empty-final", + role: "assistant", + finish: "unknown", + cost: 0, + tokens: { + input: 0, + output: 0, + reasoning: 0, + cache: { read: 0, write: 0 }, + }, + time: { created: 1, completed: 2 }, + }, + }, + } as unknown as Event); + + summaryAggregator.processEvent({ + type: "session.idle", + properties: { sessionID: "session-empty-final" }, + } as unknown as Event); + + expect(onComplete).toHaveBeenCalledWith( + "session-empty-final", + "message-empty-final", + "", + expect.objectContaining({ + finishReason: "unknown", + cost: 0, + tokens: { input: 0, output: 0, reasoning: 0, cacheRead: 0, cacheWrite: 0 }, + hasToolActivity: false, + hasReasoningActivity: false, + }), + ); + }); + it("drops the empty-response placeholder while it is still streaming in", () => { const onPartial = vi.fn(); summaryAggregator.setOnPartial(onPartial); diff --git a/tests/bot/commands/lastfile.test.ts b/tests/bot/commands/lastfile.test.ts new file mode 100644 index 000000000..e13df1fd9 --- /dev/null +++ b/tests/bot/commands/lastfile.test.ts @@ -0,0 +1,57 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; +import type { Context } from "grammy"; + +const mocked = vi.hoisted(() => ({ + currentSessionMock: vi.fn(), + getResponseMock: vi.fn(), + sendDocumentMock: vi.fn(), +})); + +vi.mock("../../../src/app/services/session-service.js", () => ({ + getCurrentSession: mocked.currentSessionMock, +})); + +vi.mock("../../../src/bot/services/assistant-response-export-service.js", () => ({ + getRememberedAssistantResponse: mocked.getResponseMock, + sendAssistantResponseDocument: mocked.sendDocumentMock, +})); + +import { lastfileCommand } from "../../../src/bot/commands/lastfile-command.js"; + +function createContext(): Context { + return { + chat: { id: 123 }, + api: {}, + reply: vi.fn().mockResolvedValue(undefined), + } as unknown as Context; +} + +describe("/lastfile", () => { + beforeEach(() => { + mocked.currentSessionMock.mockReset(); + mocked.getResponseMock.mockReset(); + mocked.sendDocumentMock.mockReset(); + mocked.currentSessionMock.mockReturnValue({ id: "session-1" }); + }); + + it("explains when no response is available", async () => { + mocked.getResponseMock.mockReturnValue(null); + const ctx = createContext(); + + await lastfileCommand(ctx); + + expect(ctx.reply).toHaveBeenCalledWith( + "No successfully delivered assistant response is available for this session.", + ); + }); + + it("exports only the current chat and session response", async () => { + mocked.getResponseMock.mockReturnValue("# Final answer"); + const ctx = createContext(); + + await lastfileCommand(ctx); + + expect(mocked.getResponseMock).toHaveBeenCalledWith(123, "session-1"); + expect(mocked.sendDocumentMock).toHaveBeenCalledWith(ctx.api, 123, "# Final answer"); + }); +}); diff --git a/tests/bot/handlers/prompt.test.ts b/tests/bot/handlers/prompt.test.ts index 2a5fd6ef7..6c6f601f1 100644 --- a/tests/bot/handlers/prompt.test.ts +++ b/tests/bot/handlers/prompt.test.ts @@ -2,7 +2,9 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; import type { Bot, Context } from "grammy"; import { consumePromptResponseMode, + hasPromptRetryAttempted, processUserPrompt, + retryPromptOnce, type ProcessPromptDeps, } from "../../../src/bot/handlers/prompt.js"; import { promptAttachment } from "../../../src/app/managers/prompt-attachment-manager.js"; @@ -238,6 +240,25 @@ describe("bot/handlers/prompt", () => { expect(mocked.suppressionRegisterMock).toHaveBeenCalledWith("session-1", "Review README"); }); + it("replays the same prompt at most once", async () => { + await processUserPrompt(createContext(), "Review README", createDeps()); + + expect(retryPromptOnce("session-1")).toBe(true); + expect(retryPromptOnce("session-1")).toBe(false); + expect(hasPromptRetryAttempted("session-1")).toBe(true); + expect(mocked.safeBackgroundTaskMock).toHaveBeenCalledTimes(2); + expect(mocked.safeBackgroundTaskMock.mock.calls[1][0].task).toBeTypeOf("function"); + await mocked.safeBackgroundTaskMock.mock.calls[1][0].task(); + expect(mocked.sessionPromptAsyncMock).toHaveBeenLastCalledWith({ + sessionID: "session-1", + directory: "D:\\Projects\\Repo", + parts: [{ type: "text", text: "Review README" }], + agent: "build", + model: { providerID: "openai", modelID: "gpt-5" }, + variant: "default", + }); + }); + it("starts prompts through promptAsync instead of the streaming prompt endpoint", async () => { const handled = await processUserPrompt(createContext(), "Review README", createDeps()); diff --git a/tests/bot/routers/command-router.test.ts b/tests/bot/routers/command-router.test.ts index 61fb2653b..5b9b2daaf 100644 --- a/tests/bot/routers/command-router.test.ts +++ b/tests/bot/routers/command-router.test.ts @@ -25,6 +25,7 @@ describe("bot/routers/command-router", () => { "start", "help", "status", + "lastfile", "settings", "opencode_start", "opencode_stop", diff --git a/tests/bot/services/assistant-response-export-service.test.ts b/tests/bot/services/assistant-response-export-service.test.ts new file mode 100644 index 000000000..d17ba7cb6 --- /dev/null +++ b/tests/bot/services/assistant-response-export-service.test.ts @@ -0,0 +1,42 @@ +import { describe, expect, it, beforeEach } from "vitest"; +import { + __resetAssistantResponseExportsForTests, + createAssistantResponseDocument, + getRememberedAssistantResponse, + rememberAssistantResponse, + shouldAutomaticallyExportAssistantResponse, +} from "../../../src/bot/services/assistant-response-export-service.js"; + +describe("assistant response Markdown exports", () => { + beforeEach(() => { + __resetAssistantResponseExportsForTests(); + }); + + it("does not automatically export responses at or below the default threshold", () => { + expect(shouldAutomaticallyExportAssistantResponse("a".repeat(5000))).toBe(false); + }); + + it("automatically exports responses above the default threshold", () => { + expect(shouldAutomaticallyExportAssistantResponse("a".repeat(5001))).toBe(true); + }); + + it("preserves UTF-8 Markdown exactly once in the document buffer", () => { + const text = '# Résumé\n\n```ts\nconst value = "日本語";\n```'; + const document = createAssistantResponseDocument(text, new Date(2026, 7, 10, 19, 45)); + + expect(document.filename).toBe("opencode-response-2026-08-10-1945.md"); + expect(document.buffer.toString("utf8")).toBe(text); + }); + + it("keeps only the latest response per chat and session", () => { + rememberAssistantResponse(10, "session-a", "first"); + rememberAssistantResponse(10, "session-a", "second"); + rememberAssistantResponse(10, "session-b", "other chat session"); + rememberAssistantResponse(11, "session-a", "other chat"); + + expect(getRememberedAssistantResponse(10, "session-a")).toBe("second"); + expect(getRememberedAssistantResponse(10, "session-b")).toBe("other chat session"); + expect(getRememberedAssistantResponse(11, "session-a")).toBe("other chat"); + expect(getRememberedAssistantResponse(12, "session-a")).toBeNull(); + }); +}); diff --git a/tests/bot/services/empty-completion-policy.test.ts b/tests/bot/services/empty-completion-policy.test.ts new file mode 100644 index 000000000..05dfe2b7e --- /dev/null +++ b/tests/bot/services/empty-completion-policy.test.ts @@ -0,0 +1,45 @@ +import { describe, expect, it } from "vitest"; +import type { MessageCompletionInfo } from "../../../src/app/managers/summary-aggregation-manager.js"; +import { + isGenuinelyEmptyAssistantResponse, + isSafeZeroWorkEmptyCompletion, +} from "../../../src/bot/services/empty-completion-policy.js"; + +function createInfo(overrides: Partial = {}): MessageCompletionInfo { + return { + tokens: { input: 0, output: 0, reasoning: 0, cacheRead: 0, cacheWrite: 0 }, + cost: 0, + finishReason: "unknown", + hasToolActivity: false, + hasReasoningActivity: false, + ...overrides, + }; +} + +describe("empty completion policy", () => { + it("recognizes the observed zero-work empty completion", () => { + expect(isGenuinelyEmptyAssistantResponse(" ")).toBe(true); + expect(isSafeZeroWorkEmptyCompletion(createInfo())).toBe(true); + }); + + it("does not classify meaningful assistant text as empty", () => { + expect(isGenuinelyEmptyAssistantResponse("Useful answer")).toBe(false); + }); + + it("does not retry when tool activity occurred", () => { + expect(isSafeZeroWorkEmptyCompletion(createInfo({ hasToolActivity: true }))).toBe(false); + }); + + it("does not retry when reasoning activity occurred", () => { + expect(isSafeZeroWorkEmptyCompletion(createInfo({ hasReasoningActivity: true }))).toBe(false); + }); + + it("does not retry an empty response with non-zero work metadata", () => { + expect( + isSafeZeroWorkEmptyCompletion( + createInfo({ tokens: { input: 1, output: 0, reasoning: 0, cacheRead: 0, cacheWrite: 0 } }), + ), + ).toBe(false); + expect(isSafeZeroWorkEmptyCompletion(createInfo({ finishReason: "stop" }))).toBe(false); + }); +}); From ba5e2dcc88580a5efae37654cea18a2b39fe0201 Mon Sep 17 00:00:00 2001 From: Vijay Kumar Date: Mon, 10 Aug 2026 22:09:35 +0530 Subject: [PATCH 2/4] fix: harden empty-completion retry safety and lifecycle - Aggregate zero-work evidence across every assistant turn of an attempt, so an earlier tool or reasoning call or any token/cost usage blocks the automatic replay instead of judging only the final empty message. - Replace the loose retryIdlePending flag with retryDispatched + idleGuard lifecycle state: exactly one retry, stale or duplicate session.idle events are consumed while the retry is in flight, and the retry completion always clears the state so its own idle produces the normal footer. - Invalidate the retry state on abort, detach, session error, bot-context loss, session mismatch, runtime cleanup, original/retry API failure, and replacement by a newer prompt; retry API failure restores the idle state and resumes the prompt queue. - Store only the terminal successfully delivered response for /lastfile and the automatic Markdown export, committing at session idle; failed, empty, or intermediate responses never replace the previous last good one. - Bind retry and export ownership to the originating Telegram chat. --- src/bot/commands/abort-command.ts | 3 +- src/bot/commands/detach-command.ts | 3 +- src/bot/handlers/prompt.ts | 149 ++++-- src/bot/services/empty-completion-policy.ts | 91 +++- .../services/event-subscription-service.ts | 97 +++- tests/bot/commands/abort.test.ts | 4 + tests/bot/commands/detach.test.ts | 4 + .../services/empty-completion-policy.test.ts | 70 +++ .../event-subscription-service.test.ts | 447 +++++++++++++++++- 9 files changed, 803 insertions(+), 65 deletions(-) diff --git a/src/bot/commands/abort-command.ts b/src/bot/commands/abort-command.ts index 718038849..706670cb5 100644 --- a/src/bot/commands/abort-command.ts +++ b/src/bot/commands/abort-command.ts @@ -7,7 +7,7 @@ import { t } from "../../i18n/index.js"; import { foregroundSessionState } from "../../app/managers/foreground-session-state-manager.js"; import { assistantRunState } from "../../app/managers/assistant-run-state-manager.js"; import { markAttachedSessionIdle } from "../../app/services/attach-service.js"; -import { clearPromptResponseMode } from "../handlers/prompt.js"; +import { clearPromptResponseMode, clearPromptRetry } from "../handlers/prompt.js"; import { markUserAbortRequested } from "../../app/managers/abort-suppression-manager.js"; import { promptQueue } from "../../app/managers/prompt-queue-manager.js"; import { promptAttachment } from "../../app/managers/prompt-attachment-manager.js"; @@ -29,6 +29,7 @@ async function releaseAbortBusyState(sessionId: string, reason: string): Promise assistantRunState.clearRun(sessionId, reason); await markAttachedSessionIdle(sessionId); clearPromptResponseMode(sessionId); + clearPromptRetry(sessionId); } async function pollSessionStatus( diff --git a/src/bot/commands/detach-command.ts b/src/bot/commands/detach-command.ts index 24e6ec5db..92f466ce6 100644 --- a/src/bot/commands/detach-command.ts +++ b/src/bot/commands/detach-command.ts @@ -7,7 +7,7 @@ import { pinnedMessageManager } from "../pinned/pinned-message-manager.js"; import { keyboardManager } from "../keyboards/keyboard-manager.js"; import { foregroundSessionState } from "../../app/managers/foreground-session-state-manager.js"; import { assistantRunState } from "../../app/managers/assistant-run-state-manager.js"; -import { clearPromptResponseMode } from "../handlers/prompt.js"; +import { clearPromptResponseMode, clearPromptRetry } from "../handlers/prompt.js"; import { logger } from "../../utils/logger.js"; import { t } from "../../i18n/index.js"; @@ -27,6 +27,7 @@ export async function detachCommand(ctx: CommandContext): Promise detachAttachedSession("detach_command"); clearPromptResponseMode(currentSession.id); + clearPromptRetry(currentSession.id); foregroundSessionState.markIdle(currentSession.id); assistantRunState.clearRun(currentSession.id, "detach_command"); clearAllInteractionState("detach_command"); diff --git a/src/bot/handlers/prompt.ts b/src/bot/handlers/prompt.ts index 80598f9a6..67ebfd519 100644 --- a/src/bot/handlers/prompt.ts +++ b/src/bot/handlers/prompt.ts @@ -33,6 +33,14 @@ import { import { externalUserInputSuppressionManager } from "../../app/managers/external-input-suppression-manager.js"; import { promptAttachment } from "../../app/managers/prompt-attachment-manager.js"; import { resolvePendingAttachment } from "../../app/services/prompt-attachment-service.js"; +import { scheduledTaskRuntime } from "../../app/services/scheduled-task-runtime-service.js"; +import { dispatchNextQueuedPrompt } from "./prompt-queue-dispatch.js"; +import { + createEmptyTaskAttemptEvidence, + isSafeZeroWorkEmptyCompletion, + mergeTaskAttemptEvidence, + type TaskAttemptEvidence, +} from "../services/empty-completion-policy.js"; /** Module-level references for async callbacks that don't have ctx. */ let botInstance: Bot | null = null; @@ -48,17 +56,26 @@ export interface PromptDispatchOptions { variant?: string; } -interface PromptRetryState { +/** + * Lifecycle of a single prompt attempt. Distinguishes the original attempt from + * the one automatic retry, guards session.idle events that close the original + * attempt while the retry is in flight, and accumulates conservative work + * evidence across every assistant turn of the current attempt. + */ +interface PromptAttemptState { bot: Bot; chatId: number; promptOptions: PromptDispatchOptions; promptText: string; responseMode: PromptResponseMode; - attempted: boolean; - retryIdlePending: boolean; + retryDispatched: boolean; + idleGuard: boolean; + workEvidence: TaskAttemptEvidence; } -const promptRetryStates = new Map(); +const promptRetryStates = new Map(); + +export type EmptyCompletionOutcome = "retried" | "failed" | "no_retry" | "ignored"; export type PromptResponseMode = "text_only" | "text_and_tts"; @@ -90,12 +107,13 @@ export function consumePromptResponseMode(sessionId: string): PromptResponseMode export function registerPromptRetry( sessionId: string, - state: Omit, + state: Omit, ): void { promptRetryStates.set(sessionId, { ...state, - attempted: false, - retryIdlePending: false, + retryDispatched: false, + idleGuard: false, + workEvidence: createEmptyTaskAttemptEvidence(), }); } @@ -103,28 +121,105 @@ export function clearPromptRetry(sessionId: string): void { promptRetryStates.delete(sessionId); } +export function clearAllPromptRetry(): void { + promptRetryStates.clear(); +} + export function hasPromptRetryAttempted(sessionId: string): boolean { - return promptRetryStates.get(sessionId)?.attempted ?? false; + return promptRetryStates.get(sessionId)?.retryDispatched ?? false; +} + +export function getPromptRetryChatId(sessionId: string): number | null { + return promptRetryStates.get(sessionId)?.chatId ?? null; +} + +/** + * Merges the completion evidence of one assistant turn into the running + * attempt-wide evidence, so a later empty completion is judged by everything the + * run actually did, not by the final message alone. + */ +export function recordAttemptEvidence( + sessionId: string, + evidence: TaskAttemptEvidence, +): void { + const state = promptRetryStates.get(sessionId); + if (!state) { + return; + } + + state.workEvidence = mergeTaskAttemptEvidence(state.workEvidence, evidence); } +/** + * Guards the idle event that closes the original empty completion while the + * automatic retry is still in flight. A stale or duplicate idle must never + * finish the active retry early, emit a footer, or dispatch queued work. The + * guard stays up until the retry completion clears the state. + */ export function consumePromptRetryIdle(sessionId: string): boolean { + return promptRetryStates.get(sessionId)?.idleGuard === true; +} + +/** + * Decides what an empty completion means for the current prompt attempt: + * retried (zero-work original, retry dispatched), failed (the retry itself came + * back empty), no_retry (work evidence says the run is not provably zero-work), + * or ignored (no prompt attempt is registered for this session). + */ +export function handleEmptyCompletion(sessionId: string): EmptyCompletionOutcome { const state = promptRetryStates.get(sessionId); - if (!state?.retryIdlePending) { - return false; + if (!state) { + return "ignored"; } - state.retryIdlePending = false; - return true; + if (!state.retryDispatched && isSafeZeroWorkEmptyCompletion(state.workEvidence)) { + return retryPromptOnce(sessionId) ? "retried" : "no_retry"; + } + + if (state.retryDispatched) { + clearPromptRetry(sessionId); + return "failed"; + } + + clearPromptRetry(sessionId); + return "no_retry"; +} + +/** + * Clears the retry state and restores a coherent idle state after the retry API + * call itself failed. Only the state this attempt registered is invalidated, so + * a slower duplicate callback can never wipe out a newer prompt's state. + */ +function abandonRetryAttempt( + sessionId: string, + state: PromptAttemptState, + reason: string, +): void { + if (promptRetryStates.get(sessionId) !== state) { + return; + } + + clearPromptRetry(sessionId); + foregroundSessionState.markIdle(sessionId); + void markAttachedSessionIdle(sessionId); + assistantRunState.clearRun(sessionId, reason); + clearPromptResponseMode(sessionId); + void state.bot.api.sendMessage(state.chatId, t("bot.prompt_send_error")).catch(() => {}); + // The idle that would normally drive the queue was consumed by the guard, so + // the canonical lifecycle is resumed from here. + void dispatchNextQueuedPrompt(); + void scheduledTaskRuntime.flushDeferredDeliveries(); } export function retryPromptOnce(sessionId: string): boolean { const state = promptRetryStates.get(sessionId); - if (!state || state.attempted) { + if (!state || state.retryDispatched) { return false; } - state.attempted = true; - state.retryIdlePending = true; + state.retryDispatched = true; + state.idleGuard = true; + state.workEvidence = createEmptyTaskAttemptEvidence(); foregroundSessionState.markBusy(sessionId, state.promptOptions.directory); void markAttachedSessionBusy(sessionId); assistantRunState.startRun(sessionId, { @@ -147,20 +242,18 @@ export function retryPromptOnce(sessionId: string): boolean { return; } - clearPromptRetry(sessionId); - foregroundSessionState.markIdle(sessionId); - void markAttachedSessionIdle(sessionId); - assistantRunState.clearRun(sessionId, "session_prompt_retry_api_error"); - clearPromptResponseMode(sessionId); - void state.bot.api.sendMessage(state.chatId, t("bot.prompt_send_error")).catch(() => {}); + logger.error( + `[Bot] Automatic empty-completion retry rejected by OpenCode: session=${sessionId}`, + error, + ); + abandonRetryAttempt(sessionId, state, "session_prompt_retry_api_error"); }, - onError: () => { - clearPromptRetry(sessionId); - foregroundSessionState.markIdle(sessionId); - void markAttachedSessionIdle(sessionId); - assistantRunState.clearRun(sessionId, "session_prompt_retry_background_error"); - clearPromptResponseMode(sessionId); - void state.bot.api.sendMessage(state.chatId, t("bot.prompt_send_error")).catch(() => {}); + onError: (error) => { + logger.error( + `[Bot] Automatic empty-completion retry background failure: session=${sessionId}`, + error, + ); + abandonRetryAttempt(sessionId, state, "session_prompt_retry_background_error"); }, }); diff --git a/src/bot/services/empty-completion-policy.ts b/src/bot/services/empty-completion-policy.ts index 0547b8641..2fbf28823 100644 --- a/src/bot/services/empty-completion-policy.ts +++ b/src/bot/services/empty-completion-policy.ts @@ -1,16 +1,16 @@ -import type { MessageCompletionInfo } from "../../app/managers/summary-aggregation-manager.js"; +import type { TokensInfo } from "../../app/managers/summary-aggregation-manager.js"; function isZero(value: number | undefined): boolean { return value !== undefined && Number.isFinite(value) && value === 0; } -function hasNoTokenUsage(info: MessageCompletionInfo): boolean { +function hasNoTokenUsage(evidence: TaskAttemptEvidence): boolean { return ( - isZero(info.tokens?.input) && - isZero(info.tokens?.output) && - isZero(info.tokens?.reasoning) && - isZero(info.tokens?.cacheRead) && - isZero(info.tokens?.cacheWrite) + isZero(evidence.tokens?.input) && + isZero(evidence.tokens?.output) && + isZero(evidence.tokens?.reasoning) && + isZero(evidence.tokens?.cacheRead) && + isZero(evidence.tokens?.cacheWrite) ); } @@ -19,16 +19,81 @@ function hasUnknownFinishReason(finishReason: string | undefined): boolean { return normalized.length === 0 || ["unknown", "invalid", "none", "null"].includes(normalized); } +/** + * Work evidence accumulated across every assistant turn of a single prompt + * attempt. Any unknown value stays undefined so the zero-work check below fails + * closed: if the run cannot be proven to have done nothing, it is never retried. + * `turnCount` is internal aggregation bookkeeping and is only read by the + * accumulator, never by the safety check. + */ +export interface TaskAttemptEvidence { + tokens?: TokensInfo; + cost?: number; + finishReason?: string; + hasToolActivity: boolean; + hasReasoningActivity: boolean; + turnCount?: number; +} + +export function createEmptyTaskAttemptEvidence(): TaskAttemptEvidence { + return { + hasToolActivity: false, + hasReasoningActivity: false, + turnCount: 0, + }; +} + +/** + * Conservative attempt-wide accumulation: activity is OR-ed across turns, and a + * token/cost field only stays known when every turn reported it. The sum is only + * ever compared against zero, so it cannot hide work performed by any turn. + */ +export function mergeTaskAttemptEvidence( + current: TaskAttemptEvidence, + next: TaskAttemptEvidence, +): TaskAttemptEvidence { + const currentTurns = current.turnCount ?? 0; + + let tokens: TokensInfo | undefined; + if (currentTurns === 0) { + tokens = next.tokens; + } else if (current.tokens !== undefined && next.tokens !== undefined) { + tokens = { + input: current.tokens.input + next.tokens.input, + output: current.tokens.output + next.tokens.output, + reasoning: current.tokens.reasoning + next.tokens.reasoning, + cacheRead: current.tokens.cacheRead + next.tokens.cacheRead, + cacheWrite: current.tokens.cacheWrite + next.tokens.cacheWrite, + }; + } + + let cost: number | undefined; + if (currentTurns === 0) { + cost = next.cost; + } else if (current.cost !== undefined && next.cost !== undefined) { + cost = current.cost + next.cost; + } + + return { + tokens, + cost, + finishReason: next.finishReason ?? current.finishReason, + hasToolActivity: current.hasToolActivity || next.hasToolActivity, + hasReasoningActivity: current.hasReasoningActivity || next.hasReasoningActivity, + turnCount: currentTurns + 1, + }; +} + export function isGenuinelyEmptyAssistantResponse(messageText: string): boolean { return messageText.trim().length === 0; } -export function isSafeZeroWorkEmptyCompletion(info: MessageCompletionInfo): boolean { +export function isSafeZeroWorkEmptyCompletion(evidence: TaskAttemptEvidence): boolean { return ( - hasNoTokenUsage(info) && - isZero(info.cost) && - !info.hasToolActivity && - !info.hasReasoningActivity && - hasUnknownFinishReason(info.finishReason) + hasNoTokenUsage(evidence) && + isZero(evidence.cost) && + !evidence.hasToolActivity && + !evidence.hasReasoningActivity && + hasUnknownFinishReason(evidence.finishReason) ); } diff --git a/src/bot/services/event-subscription-service.ts b/src/bot/services/event-subscription-service.ts index 15cc4d159..3a535ea09 100644 --- a/src/bot/services/event-subscription-service.ts +++ b/src/bot/services/event-subscription-service.ts @@ -34,11 +34,14 @@ import { safeBackgroundTask } from "../../utils/safe-background-task.js"; import { pinnedMessageManager } from "../pinned/pinned-message-manager.js"; import { keyboardManager } from "../keyboards/keyboard-manager.js"; import { + clearAllPromptRetry, clearPromptResponseMode, clearPromptRetry, consumePromptRetryIdle, + getPromptRetryChatId, + handleEmptyCompletion, hasPromptRetryAttempted, - retryPromptOnce, + recordAttemptEvidence, } from "../handlers/prompt.js"; import { reconcileBusyState, @@ -60,7 +63,10 @@ import { import { formatAssistantRunFooter } from "../../app/formatters/assistant-run-footer-formatter.js"; import { foregroundSessionState } from "../../app/managers/foreground-session-state-manager.js"; import { scheduledTaskRuntime } from "../../app/services/scheduled-task-runtime-service.js"; -import { assistantRunState } from "../../app/managers/assistant-run-state-manager.js"; +import { + assistantRunState, + type AssistantRunInfo, +} from "../../app/managers/assistant-run-state-manager.js"; import { ResponseStreamer, type StreamingMessagePayload } from "../streaming/response-streamer.js"; import { ToolCallStreamer, type ToolStreamKey } from "../streaming/tool-call-streamer.js"; import { RunningToolTracker, type RunningToolTick } from "../streaming/running-tool-tracker.js"; @@ -103,7 +109,6 @@ import { } from "./assistant-response-export-service.js"; import { isGenuinelyEmptyAssistantResponse, - isSafeZeroWorkEmptyCompletion, } from "./empty-completion-policy.js"; const TELEGRAM_DOCUMENT_CAPTION_MAX_LENGTH = 1024; @@ -159,6 +164,15 @@ class EventSubscriptionService implements BotEventSubscriptionService { { callId: string; activity: string } >(); private readonly subagentSnapshots = new Map(); + // Terminal assistant response of the in-flight run, keyed by session. Replaced + // by each delivered non-empty completion and marked null when a completion is + // empty, so a run that ends without a deliverable never overwrites the + // previous last good response. The chatId is captured from the originating + // completion rather than the mutable service-wide chat context. + private readonly pendingFinalResponses = new Map< + string, + { chatId: number; text: string } | null + >(); constructor() { this.runningToolTracker = new RunningToolTracker({ @@ -503,8 +517,10 @@ class EventSubscriptionService implements BotEventSubscriptionService { this.compactProgressFinalizationTasks.clear(); this.thinkingSections.clear(); this.sessionCompletionTasks.clear(); + this.pendingFinalResponses.clear(); this.clearToolElapsedState(null, reason); assistantRunState.clearAll(reason); + clearAllPromptRetry(); } cleanup(reason: string): void { @@ -588,6 +604,8 @@ class EventSubscriptionService implements BotEventSubscriptionService { if (!this.botInstance || !this.chatIdInstance) { logger.error("Bot or chat ID not available for sending message"); clearPromptResponseMode(sessionId); + clearPromptRetry(sessionId); + this.pendingFinalResponses.delete(sessionId); this.clearAssistantResponseStream(sessionId, messageId, "bot_context_missing"); this.clearThinkingStream(sessionId, messageId, "bot_context_missing"); this.toolCallStreamer.clearSession(sessionId, "bot_context_missing"); @@ -601,6 +619,8 @@ class EventSubscriptionService implements BotEventSubscriptionService { const currentSession = getCurrentSession(); if (currentSession?.id !== sessionId) { clearPromptResponseMode(sessionId); + clearPromptRetry(sessionId); + this.pendingFinalResponses.delete(sessionId); this.clearAssistantResponseStream(sessionId, messageId, "session_mismatch"); this.clearThinkingStream(sessionId, messageId, "session_mismatch"); this.toolCallStreamer.clearSession(sessionId, "session_mismatch"); @@ -616,25 +636,30 @@ class EventSubscriptionService implements BotEventSubscriptionService { const chatId = this.chatIdInstance; try { + recordAttemptEvidence(sessionId, completionInfo); + if (isGenuinelyEmptyAssistantResponse(messageText)) { this.clearAssistantResponseStream(sessionId, messageId, "empty_completion"); this.clearThinkingStream(sessionId, messageId, "empty_completion"); this.compactProgressStreamer.clearSession(sessionId, "empty_completion"); - if (isSafeZeroWorkEmptyCompletion(completionInfo) && retryPromptOnce(sessionId)) { - await botApi.sendMessage(chatId, t("bot.empty_completion_retry")); - } else if (hasPromptRetryAttempted(sessionId)) { - clearPromptRetry(sessionId); - await botApi.sendMessage(chatId, t("bot.empty_completion_failed")); - } else { - clearPromptRetry(sessionId); - await botApi.sendMessage(chatId, t("bot.empty_completion_no_retry")); + this.pendingFinalResponses.set(sessionId, null); + const originChatId = getPromptRetryChatId(sessionId) ?? chatId; + const outcome = handleEmptyCompletion(sessionId); + if (outcome === "retried") { + await botApi.sendMessage(originChatId, t("bot.empty_completion_retry")); + } else if (outcome === "failed") { + await botApi.sendMessage(originChatId, t("bot.empty_completion_failed")); + } else if (outcome === "no_retry") { + await botApi.sendMessage(originChatId, t("bot.empty_completion_no_retry")); } return; } - clearPromptRetry(sessionId); + if (hasPromptRetryAttempted(sessionId)) { + clearPromptRetry(sessionId); + } assistantRunState.markResponseCompleted(sessionId, { agent: completionInfo.agent, providerID: completionInfo.providerID, @@ -685,15 +710,7 @@ class EventSubscriptionService implements BotEventSubscriptionService { }, }); - rememberAssistantResponse(chatId, sessionId, messageText); - if (shouldAutomaticallyExportAssistantResponse(messageText)) { - await sendAssistantResponseDocument(botApi, chatId, messageText).catch((error) => { - logger.warn( - `[Bot] Failed to send automatic Markdown response export: session=${sessionId}`, - error, - ); - }); - } + this.pendingFinalResponses.set(sessionId, { chatId, text: messageText }); await sendTtsResponseForSession({ api: botApi, @@ -1165,6 +1182,8 @@ class EventSubscriptionService implements BotEventSubscriptionService { const completedRun = assistantRunState.finishRun(sessionId, "session_idle"); clearPromptResponseMode(sessionId); + await this.commitPendingFinalResponse(sessionId, completedRun); + clearPromptRetry(sessionId); if (!this.botInstance || !this.chatIdInstance) { foregroundSessionState.markIdle(sessionId); @@ -1218,6 +1237,7 @@ class EventSubscriptionService implements BotEventSubscriptionService { await markAttachedSessionIdle(sessionId); this.clearToolElapsedState(sessionId, "session_error"); clearPromptRetry(sessionId); + this.pendingFinalResponses.delete(sessionId); if (!this.botInstance || !this.chatIdInstance) { clearPromptResponseMode(sessionId); @@ -1657,6 +1677,41 @@ class EventSubscriptionService implements BotEventSubscriptionService { return nextTask; } + /** + * Stores the terminal successfully delivered assistant response of a finished + * run for /lastfile and automatic Markdown export. Intermediate commentary + * only overwrites the pending slot during the run, and a failed or empty run + * never commits, so the previous last good response is preserved. + */ + private async commitPendingFinalResponse( + sessionId: string, + completedRun: AssistantRunInfo | null, + ): Promise { + const pending = this.pendingFinalResponses.get(sessionId); + this.pendingFinalResponses.delete(sessionId); + + if ( + pending === undefined || + pending === null || + !completedRun?.hasCompletedResponse || + !this.botInstance + ) { + return; + } + + rememberAssistantResponse(pending.chatId, sessionId, pending.text); + if (shouldAutomaticallyExportAssistantResponse(pending.text)) { + await sendAssistantResponseDocument(this.botInstance.api, pending.chatId, pending.text).catch( + (error) => { + logger.warn( + `[Bot] Failed to send automatic Markdown response export: session=${sessionId}`, + error, + ); + }, + ); + } + } + private finalizeCompactProgress(sessionId: string): Promise { const existingTask = this.compactProgressFinalizationTasks.get(sessionId); if (existingTask) { diff --git a/tests/bot/commands/abort.test.ts b/tests/bot/commands/abort.test.ts index 5e2725b46..ce49e307d 100644 --- a/tests/bot/commands/abort.test.ts +++ b/tests/bot/commands/abort.test.ts @@ -24,6 +24,7 @@ const mocked = vi.hoisted(() => ({ clearRunMock: vi.fn(), markAttachedSessionIdleMock: vi.fn(), clearPromptResponseModeMock: vi.fn(), + clearPromptRetryMock: vi.fn(), })); vi.mock("../../../src/app/services/session-service.js", () => ({ @@ -51,6 +52,7 @@ vi.mock("../../../src/app/services/attach-service.js", () => ({ vi.mock("../../../src/bot/handlers/prompt.js", () => ({ clearPromptResponseMode: mocked.clearPromptResponseModeMock, + clearPromptRetry: mocked.clearPromptRetryMock, })); const TEST_QUESTION: Question = { @@ -93,6 +95,7 @@ describe("bot/commands/abort", () => { mocked.markAttachedSessionIdleMock.mockReset(); mocked.markAttachedSessionIdleMock.mockResolvedValue(undefined); mocked.clearPromptResponseModeMock.mockReset(); + mocked.clearPromptRetryMock.mockReset(); __resetUserAbortErrorSuppressionForTests(); }); @@ -105,6 +108,7 @@ describe("bot/commands/abort", () => { expect(mocked.clearRunMock).toHaveBeenCalledWith("session-1", reason); expect(mocked.markAttachedSessionIdleMock).toHaveBeenCalledWith("session-1"); expect(mocked.clearPromptResponseModeMock).toHaveBeenCalledWith("session-1"); + expect(mocked.clearPromptRetryMock).toHaveBeenCalledWith("session-1"); } it("clears interaction state even when there is no active session", async () => { diff --git a/tests/bot/commands/detach.test.ts b/tests/bot/commands/detach.test.ts index 4404c4af8..6dcb89ade 100644 --- a/tests/bot/commands/detach.test.ts +++ b/tests/bot/commands/detach.test.ts @@ -19,6 +19,7 @@ const mocked = vi.hoisted(() => ({ foregroundMarkIdleMock: vi.fn(), assistantClearRunMock: vi.fn(), clearPromptResponseModeMock: vi.fn(), + clearPromptRetryMock: vi.fn(), })); vi.mock("../../../src/app/stores/settings-store.js", () => ({ @@ -70,6 +71,7 @@ vi.mock("../../../src/app/managers/assistant-run-state-manager.js", () => ({ vi.mock("../../../src/bot/handlers/prompt.js", () => ({ clearPromptResponseMode: mocked.clearPromptResponseModeMock, + clearPromptRetry: mocked.clearPromptRetryMock, })); function createContext(): Context { @@ -107,6 +109,7 @@ describe("bot/commands/detach", () => { mocked.foregroundMarkIdleMock.mockClear(); mocked.assistantClearRunMock.mockClear(); mocked.clearPromptResponseModeMock.mockClear(); + mocked.clearPromptRetryMock.mockClear(); }); it("detaches selected session locally without stopping the OpenCode session", async () => { @@ -120,6 +123,7 @@ describe("bot/commands/detach", () => { expect(mocked.foregroundMarkIdleMock).toHaveBeenCalledWith("session-1"); expect(mocked.assistantClearRunMock).toHaveBeenCalledWith("session-1", "detach_command"); expect(mocked.clearPromptResponseModeMock).toHaveBeenCalledWith("session-1"); + expect(mocked.clearPromptRetryMock).toHaveBeenCalledWith("session-1"); expect(mocked.pinnedClearMock).toHaveBeenCalledTimes(1); expect(mocked.pinnedRefreshContextLimitMock).toHaveBeenCalledTimes(1); expect(mocked.pinnedGetContextLimitMock).toHaveBeenCalledTimes(1); diff --git a/tests/bot/services/empty-completion-policy.test.ts b/tests/bot/services/empty-completion-policy.test.ts index 05dfe2b7e..e26b939cc 100644 --- a/tests/bot/services/empty-completion-policy.test.ts +++ b/tests/bot/services/empty-completion-policy.test.ts @@ -1,8 +1,10 @@ import { describe, expect, it } from "vitest"; import type { MessageCompletionInfo } from "../../../src/app/managers/summary-aggregation-manager.js"; import { + createEmptyTaskAttemptEvidence, isGenuinelyEmptyAssistantResponse, isSafeZeroWorkEmptyCompletion, + mergeTaskAttemptEvidence, } from "../../../src/bot/services/empty-completion-policy.js"; function createInfo(overrides: Partial = {}): MessageCompletionInfo { @@ -42,4 +44,72 @@ describe("empty completion policy", () => { ).toBe(false); expect(isSafeZeroWorkEmptyCompletion(createInfo({ finishReason: "stop" }))).toBe(false); }); + + it("fails closed when any turn is missing token or cost data", () => { + expect(isSafeZeroWorkEmptyCompletion(createInfo({ tokens: undefined }))).toBe(false); + expect(isSafeZeroWorkEmptyCompletion(createInfo({ cost: undefined }))).toBe(false); + }); + + describe("attempt-wide evidence aggregation", () => { + it("keeps a single zero-work empty turn retry-safe", () => { + const evidence = mergeTaskAttemptEvidence( + createEmptyTaskAttemptEvidence(), + createInfo(), + ); + expect(isSafeZeroWorkEmptyCompletion(evidence)).toBe(true); + }); + + it("blocks retry when an earlier turn used a tool", () => { + const earlier = createInfo({ hasToolActivity: true, tokens: { ...createInfo().tokens! } }); + const evidence = mergeTaskAttemptEvidence(createEmptyTaskAttemptEvidence(), earlier); + const final = mergeTaskAttemptEvidence(evidence, createInfo()); + expect(isSafeZeroWorkEmptyCompletion(final)).toBe(false); + }); + + it("blocks retry when an earlier turn reasoned", () => { + const earlier = createInfo({ hasReasoningActivity: true }); + const evidence = mergeTaskAttemptEvidence(createEmptyTaskAttemptEvidence(), earlier); + const final = mergeTaskAttemptEvidence(evidence, createInfo()); + expect(isSafeZeroWorkEmptyCompletion(final)).toBe(false); + }); + + it("blocks retry when an earlier turn consumed tokens even if the final is empty", () => { + const earlier = createInfo({ + tokens: { input: 42, output: 0, reasoning: 0, cacheRead: 0, cacheWrite: 0 }, + }); + const evidence = mergeTaskAttemptEvidence(createEmptyTaskAttemptEvidence(), earlier); + const final = mergeTaskAttemptEvidence(evidence, createInfo()); + expect(isSafeZeroWorkEmptyCompletion(final)).toBe(false); + }); + + it("blocks retry when any turn lacked token or cost reporting", () => { + const missingTokens = createInfo({ tokens: undefined }); + const evidence = mergeTaskAttemptEvidence(createEmptyTaskAttemptEvidence(), missingTokens); + expect(isSafeZeroWorkEmptyCompletion(mergeTaskAttemptEvidence(evidence, createInfo()))).toBe( + false, + ); + + const missingCost = createInfo({ cost: undefined }); + const evidence2 = mergeTaskAttemptEvidence(createEmptyTaskAttemptEvidence(), missingCost); + expect(isSafeZeroWorkEmptyCompletion(mergeTaskAttemptEvidence(evidence2, createInfo()))).toBe( + false, + ); + }); + + it("uses the last known finish reason and ORs activity flags", () => { + const evidence = mergeTaskAttemptEvidence( + createEmptyTaskAttemptEvidence(), + createInfo({ finishReason: "stop" }), + ); + const final = mergeTaskAttemptEvidence(evidence, createInfo({ finishReason: "length" })); + expect(final.finishReason).toBe("length"); + expect(final.hasToolActivity).toBe(false); + + const tooled = mergeTaskAttemptEvidence( + createEmptyTaskAttemptEvidence(), + createInfo({ hasToolActivity: true }), + ); + expect(mergeTaskAttemptEvidence(tooled, createInfo()).hasToolActivity).toBe(true); + }); + }); }); diff --git a/tests/bot/services/event-subscription-service.test.ts b/tests/bot/services/event-subscription-service.test.ts index fc7f636f0..d494bd88e 100644 --- a/tests/bot/services/event-subscription-service.test.ts +++ b/tests/bot/services/event-subscription-service.test.ts @@ -10,6 +10,9 @@ import { resetSingletonState } from "../../helpers/reset-singleton-state.js"; const mocked = vi.hoisted(() => ({ subscribeToEvents: vi.fn(), stopEventListening: vi.fn(), + safeBackgroundTask: vi.fn(), + dispatchNextQueuedPrompt: vi.fn(), + promptAsyncMock: vi.fn(), })); vi.mock("../../../src/opencode/events.js", () => ({ @@ -17,6 +20,24 @@ vi.mock("../../../src/opencode/events.js", () => ({ stopEventListening: mocked.stopEventListening, })); +vi.mock("../../../src/opencode/client.js", () => ({ + opencodeClient: { + session: { + status: vi.fn().mockResolvedValue({ data: {}, error: null }), + promptAsync: mocked.promptAsyncMock, + }, + }, +})); + +vi.mock("../../../src/utils/safe-background-task.js", () => ({ + safeBackgroundTask: mocked.safeBackgroundTask, +})); + +vi.mock("../../../src/bot/handlers/prompt-queue-dispatch.js", () => ({ + dispatchNextQueuedPrompt: mocked.dispatchNextQueuedPrompt, + __resetPromptQueueDispatchForTests: () => {}, +})); + type FakeBotApi = { sendMessage: ReturnType; sendMessageDraft: ReturnType; @@ -330,6 +351,13 @@ describe("bot/services/event-subscription-service", () => { mocked.subscribeToEvents.mockReset(); mocked.stopEventListening.mockReset(); mocked.subscribeToEvents.mockResolvedValue(undefined); + mocked.safeBackgroundTask.mockReset(); + mocked.dispatchNextQueuedPrompt.mockReset(); + mocked.promptAsyncMock.mockReset(); + mocked.promptAsyncMock.mockResolvedValue({ data: {}, error: null }); + + const exportService = await import("../../../src/bot/services/assistant-response-export-service.js"); + exportService.__resetAssistantResponseExportsForTests(); const settingsStore = await import("../../../src/app/stores/settings-store.js"); settingsStore.__resetSettingsForTests(); @@ -362,6 +390,7 @@ describe("bot/services/event-subscription-service", () => { } = {}, ): Promise<{ api: FakeBotApi; + service: ReturnType; summaryAggregator: { setSession(sessionId: string): void; processEvent(event: Event): void }; }> { const [ @@ -406,7 +435,7 @@ describe("bot/services/event-subscription-service", () => { summaryAggregator.setSession("session-1"); emitAssistantMessage(summaryAggregator); - return { api, summaryAggregator }; + return { api, service, summaryAggregator }; } it("sends write tool output as a document attachment when diff files are enabled", async () => { @@ -785,4 +814,420 @@ describe("bot/services/event-subscription-service", () => { }); expect(interactionManager.getSnapshot()?.kind).toBe("rename"); }); + + describe("empty completion retry lifecycle", () => { + type RetryTaskOptions = { + taskName: string; + task: () => Promise; + onSuccess?: (value: { error: unknown | null }) => void; + onError?: (error: unknown) => void; + }; + + function emitAssistantText( + aggregator: { processEvent(event: Event): void }, + text: string, + messageId: string, + ): void { + aggregator.processEvent({ + type: "message.part.updated", + properties: { + part: { + id: `text-${messageId}`, + sessionID: "session-1", + messageID: messageId, + type: "text", + text, + }, + }, + } as unknown as Event); + } + + function emitAssistantCompleted( + aggregator: { processEvent(event: Event): void }, + messageId: string, + overrides: Record = {}, + ): void { + aggregator.processEvent({ + type: "message.updated", + properties: { + info: { + id: messageId, + sessionID: "session-1", + role: "assistant", + agent: "test-agent", + providerID: "test-provider", + modelID: "test-model", + time: { created: Date.now() - 1000, completed: Date.now() }, + ...overrides, + }, + }, + } as unknown as Event); + } + + function emitZeroWorkEmptyCompletion( + aggregator: { processEvent(event: Event): void }, + messageId: string, + ): void { + emitAssistantCompleted(aggregator, messageId, { + finish: "unknown", + cost: 0, + tokens: { input: 0, output: 0, reasoning: 0, cache: { read: 0, write: 0 } }, + }); + } + + async function registerPromptAttempt(api: FakeBotApi, chatId = 42): Promise { + const { registerPromptRetry } = await import("../../../src/bot/handlers/prompt.js"); + registerPromptRetry("session-1", { + bot: { api } as unknown as Bot, + chatId, + promptOptions: { + sessionID: "session-1", + directory: "D:/repo", + parts: [{ type: "text", text: "Review README" }], + agent: "build", + }, + promptText: "Review README", + responseMode: "text_only", + }); + } + + function getRetryTaskOptions(): RetryTaskOptions { + const calls = mocked.safeBackgroundTask.mock.calls as unknown as [[RetryTaskOptions]]; + const options = calls.find(([entry]) => entry.taskName === "session.promptAsync.retry")?.[0]; + if (!options) { + throw new Error("retry background task was not captured"); + } + return options; + } + + function countFooters(api: FakeBotApi): number { + return api.sendMessage.mock.calls.filter(([, text]) => + String(text).includes("test-provider/test-model"), + ).length; + } + + async function flushRealDispatch(): Promise { + for (let attempt = 0; attempt < 5; attempt++) { + await new Promise((resolve) => setImmediate(resolve)); + await new Promise((resolve) => setTimeout(resolve, 0)); + } + } + + it("replays a provably zero-work empty completion exactly once", async () => { + const { api, summaryAggregator } = await setupService(false); + await registerPromptAttempt(api); + + emitZeroWorkEmptyCompletion(summaryAggregator, "message-1"); + emitSessionIdle(summaryAggregator); + + await vi.waitFor(() => { + expect( + api.sendMessage.mock.calls.some(([, text]) => + String(text).includes("Retrying once"), + ), + ).toBe(true); + }); + await vi.waitFor(() => { + expect(mocked.safeBackgroundTask).toHaveBeenCalledWith( + expect.objectContaining({ taskName: "session.promptAsync.retry" }), + ); + }); + + // The idle that closed the original attempt was consumed by the guard. + expect(countFooters(api)).toBe(0); + expect(mocked.dispatchNextQueuedPrompt).not.toHaveBeenCalled(); + }); + + it("emits exactly one normal footer after a successful retry", async () => { + const { api, summaryAggregator } = await setupService(false); + await registerPromptAttempt(api); + + emitZeroWorkEmptyCompletion(summaryAggregator, "message-1"); + emitSessionIdle(summaryAggregator); + + await vi.waitFor(() => { + expect(mocked.safeBackgroundTask).toHaveBeenCalledWith( + expect.objectContaining({ taskName: "session.promptAsync.retry" }), + ); + }); + + const retryOptions = getRetryTaskOptions(); + await retryOptions.task(); + retryOptions.onSuccess?.({ error: null }); + + emitAssistantText(summaryAggregator, "Final answer", "message-2"); + emitAssistantCompleted(summaryAggregator, "message-2"); + emitSessionIdle(summaryAggregator); + + await vi.waitFor(() => { + expect( + api.sendMessage.mock.calls.some(([, text]) => String(text) === "Final answer"), + ).toBe(true); + }); + expect(countFooters(api)).toBe(1); + expect(mocked.safeBackgroundTask).toHaveBeenCalledTimes(1); + }); + + it("does not retry or emit a footer when the retry itself comes back empty", async () => { + const { api, summaryAggregator } = await setupService(false); + await registerPromptAttempt(api); + + emitZeroWorkEmptyCompletion(summaryAggregator, "message-1"); + emitSessionIdle(summaryAggregator); + + await vi.waitFor(() => { + expect(mocked.safeBackgroundTask).toHaveBeenCalledTimes(1); + }); + + emitZeroWorkEmptyCompletion(summaryAggregator, "message-2"); + emitSessionIdle(summaryAggregator); + + await vi.waitFor(() => { + expect( + api.sendMessage.mock.calls.some(([, text]) => + String(text).includes("No further retry was attempted"), + ), + ).toBe(true); + }); + expect(mocked.safeBackgroundTask).toHaveBeenCalledTimes(1); + expect(countFooters(api)).toBe(0); + }); + + it("never replays the task when an earlier turn used a tool", async () => { + const { api, summaryAggregator } = await setupService(false); + await registerPromptAttempt(api); + + emitBashTool(summaryAggregator, "completed"); + emitZeroWorkEmptyCompletion(summaryAggregator, "message-1"); + emitSessionIdle(summaryAggregator); + + await vi.waitFor(() => { + expect( + api.sendMessage.mock.calls.some(([, text]) => + String(text).includes("No automatic retry was attempted"), + ), + ).toBe(true); + }); + expect(mocked.safeBackgroundTask).not.toHaveBeenCalled(); + }); + + it("never replays the task when an earlier turn reasoned", async () => { + const { api, summaryAggregator } = await setupService(false); + await registerPromptAttempt(api); + + emitThinkingPart(summaryAggregator, "Careful thought"); + emitZeroWorkEmptyCompletion(summaryAggregator, "message-1"); + emitSessionIdle(summaryAggregator); + + await vi.waitFor(() => { + expect( + api.sendMessage.mock.calls.some(([, text]) => + String(text).includes("No automatic retry was attempted"), + ), + ).toBe(true); + }); + expect(mocked.safeBackgroundTask).not.toHaveBeenCalled(); + }); + + it("ignores stale duplicate idle events while the retry is in flight", async () => { + const { api, summaryAggregator } = await setupService(false); + await registerPromptAttempt(api); + + emitZeroWorkEmptyCompletion(summaryAggregator, "message-1"); + emitSessionIdle(summaryAggregator); + + await vi.waitFor(() => { + expect(mocked.safeBackgroundTask).toHaveBeenCalledTimes(1); + }); + + emitSessionIdle(summaryAggregator); + emitSessionIdle(summaryAggregator); + await flushRealDispatch(); + + expect(countFooters(api)).toBe(0); + expect(mocked.dispatchNextQueuedPrompt).not.toHaveBeenCalled(); + + const retryOptions = getRetryTaskOptions(); + await retryOptions.task(); + retryOptions.onSuccess?.({ error: null }); + + emitAssistantText(summaryAggregator, "Recovered", "message-2"); + emitAssistantCompleted(summaryAggregator, "message-2"); + emitSessionIdle(summaryAggregator); + + await vi.waitFor(() => { + expect(countFooters(api)).toBe(1); + }); + expect(mocked.safeBackgroundTask).toHaveBeenCalledTimes(1); + }); + + it("restores idle state and releases the queue when the retry API call fails", async () => { + const { api, summaryAggregator } = await setupService(false); + const promptModule = await import("../../../src/bot/handlers/prompt.js"); + const { foregroundSessionState } = await import( + "../../../src/app/managers/foreground-session-state-manager.js" + ); + await registerPromptAttempt(api); + + emitZeroWorkEmptyCompletion(summaryAggregator, "message-1"); + emitSessionIdle(summaryAggregator); + + await vi.waitFor(() => { + expect(mocked.safeBackgroundTask).toHaveBeenCalledTimes(1); + }); + + const retryOptions = getRetryTaskOptions(); + await retryOptions.task(); + retryOptions.onSuccess?.({ error: new Error("retry rejected") }); + + await vi.waitFor(() => { + expect(mocked.dispatchNextQueuedPrompt).toHaveBeenCalled(); + }); + expect(foregroundSessionState.isBusy()).toBe(false); + expect(promptModule.hasPromptRetryAttempted("session-1")).toBe(false); + expect( + api.sendMessage.mock.calls.some(([, text]) => + String(text).includes("Failed to send request to OpenCode."), + ), + ).toBe(true); + }); + + it("invalidates the retry lifecycle on runtime cleanup", async () => { + const { service } = await setupService(false); + const promptModule = await import("../../../src/bot/handlers/prompt.js"); + await registerPromptAttempt({ sendMessage: vi.fn().mockResolvedValue(undefined) } as never); + + service.clearRuntimeState("test_cleanup"); + expect(promptModule.hasPromptRetryAttempted("session-1")).toBe(false); + }); + + it("invalidates the retry lifecycle on session errors", async () => { + const { api, summaryAggregator } = await setupService(false); + const promptModule = await import("../../../src/bot/handlers/prompt.js"); + await registerPromptAttempt(api); + + summaryAggregator.processEvent({ + type: "session.error", + properties: { sessionID: "session-1", error: "boom" }, + } as unknown as Event); + await flushRealDispatch(); + + expect(promptModule.hasPromptRetryAttempted("session-1")).toBe(false); + + emitZeroWorkEmptyCompletion(summaryAggregator, "message-2"); + emitSessionIdle(summaryAggregator); + await flushRealDispatch(); + + expect(mocked.safeBackgroundTask).not.toHaveBeenCalled(); + expect( + api.sendMessage.mock.calls.some(([, text]) => String(text).includes("empty response")), + ).toBe(false); + }); + + it("invalidates the retry lifecycle when the completing session is no longer current", async () => { + const { api, summaryAggregator } = await setupService(false); + const promptModule = await import("../../../src/bot/handlers/prompt.js"); + const sessionService = await import("../../../src/app/services/session-service.js"); + await registerPromptAttempt(api); + + sessionService.setCurrentSession({ + id: "session-2", + title: "Other session", + directory: "D:/repo", + }); + emitZeroWorkEmptyCompletion(summaryAggregator, "message-1"); + emitSessionIdle(summaryAggregator); + await flushRealDispatch(); + + expect(promptModule.hasPromptRetryAttempted("session-1")).toBe(false); + expect(mocked.safeBackgroundTask).not.toHaveBeenCalled(); + }); + + it("does not export an intermediate long response and keeps the last good one", async () => { + const { api, summaryAggregator } = await setupService(false, { startAssistantRun: true }); + const exportService = await import( + "../../../src/bot/services/assistant-response-export-service.js" + ); + + emitAssistantText(summaryAggregator, "Good answer", "message-1"); + emitAssistantCompleted(summaryAggregator, "message-1"); + emitSessionIdle(summaryAggregator); + await vi.waitFor(() => { + expect(exportService.getRememberedAssistantResponse(42, "session-1")).toBe("Good answer"); + }); + + await registerPromptAttempt(api); + emitAssistantText(summaryAggregator, "x".repeat(6000), "message-2"); + emitAssistantCompleted(summaryAggregator, "message-2"); + emitZeroWorkEmptyCompletion(summaryAggregator, "message-3"); + emitSessionIdle(summaryAggregator); + + await vi.waitFor(() => { + expect( + api.sendMessage.mock.calls.some(([, text]) => + String(text).includes("No automatic retry was attempted"), + ), + ).toBe(true); + }); + expect(exportService.getRememberedAssistantResponse(42, "session-1")).toBe("Good answer"); + expect(api.sendDocument).not.toHaveBeenCalled(); + }); + + it("exports a long final response as a supplemental Markdown document", async () => { + const { api, summaryAggregator } = await setupService(false, { startAssistantRun: true }); + const exportService = await import( + "../../../src/bot/services/assistant-response-export-service.js" + ); + + const longText = `# Heading\n\n${"x".repeat(6000)}`; + emitAssistantText(summaryAggregator, longText, "message-1"); + emitAssistantCompleted(summaryAggregator, "message-1"); + emitSessionIdle(summaryAggregator); + + await vi.waitFor(() => { + expect(api.sendDocument).toHaveBeenCalledTimes(1); + }); + expect(exportService.getRememberedAssistantResponse(42, "session-1")).toBe(longText); + }); + + it("does not export a short final response", async () => { + const { api, summaryAggregator } = await setupService(false, { startAssistantRun: true }); + const exportService = await import( + "../../../src/bot/services/assistant-response-export-service.js" + ); + + emitAssistantText(summaryAggregator, "Short answer", "message-1"); + emitAssistantCompleted(summaryAggregator, "message-1"); + emitSessionIdle(summaryAggregator); + + await vi.waitFor(() => { + expect(exportService.getRememberedAssistantResponse(42, "session-1")).toBe("Short answer"); + }); + expect(api.sendDocument).not.toHaveBeenCalled(); + }); + + it("scopes retry and export state by chat and session", async () => { + const { api, summaryAggregator } = await setupService(false, { startAssistantRun: true }); + const promptModule = await import("../../../src/bot/handlers/prompt.js"); + const exportService = await import( + "../../../src/bot/services/assistant-response-export-service.js" + ); + + emitAssistantText(summaryAggregator, "For session one", "message-1"); + emitAssistantCompleted(summaryAggregator, "message-1"); + emitSessionIdle(summaryAggregator); + await vi.waitFor(() => { + expect(exportService.getRememberedAssistantResponse(42, "session-1")).toBe( + "For session one", + ); + }); + expect(exportService.getRememberedAssistantResponse(43, "session-1")).toBeNull(); + expect(exportService.getRememberedAssistantResponse(42, "session-2")).toBeNull(); + + await registerPromptAttempt(api); + expect(promptModule.hasPromptRetryAttempted("session-1")).toBe(false); + expect(promptModule.handleEmptyCompletion("session-2")).toBe("ignored"); + promptModule.clearPromptRetry("session-1"); + }); + }); }); From bc798d460c2e662f6ee7989b5a22dc661c6e76cc Mon Sep 17 00:00:00 2001 From: Vijay Kumar Date: Mon, 10 Aug 2026 23:29:11 +0530 Subject: [PATCH 3/4] fix: never report intermediate or truncated runs as complete - Add an assistant-message-started signal to the aggregator and only treat a completed message as a terminal final-response candidate when it survived with no upstream error, performed no tool activity, and did not end on a known non-terminal finish reason. Intermediate commentary and truncated or aborted output are still streamed to the user but never committed. - Invalidate a pending final-response candidate whenever a newer assistant message starts, so commentary followed by more tool work never becomes /lastfile, an automatic Markdown export, or evidence of a successful task. - Gate the assistant-run footer on an actually committed terminal response instead of any completed message, so a run that ends without a deliverable is never announced as complete. - Harden stale-idle ordering across the automatic retry with an explicit response-delivered phase and a settle timer: every session.idle, including a stale duplicate of the original attempt's idle, is consumed while the retry state is alive, and the retry is finalized exactly once by its own settle rather than by the first idle that happens to arrive. - Keep the retry idle guard off until the retry is actually dispatched so a registered-but-unreplayed attempt never swallows a normal run's idle. - Bind the committed /lastfile, automatic Markdown export, success footer, and TTS reply to the originating Telegram chat preserved for the prompt instead of the mutable service-wide chat context. --- .../managers/summary-aggregation-manager.ts | 19 ++ src/bot/handlers/prompt.ts | 115 ++++++++- src/bot/services/empty-completion-policy.ts | 52 ++++ .../services/event-subscription-service.ts | 226 ++++++++++------ .../services/empty-completion-policy.test.ts | 44 ++++ .../event-subscription-service.test.ts | 241 +++++++++++++++++- 6 files changed, 613 insertions(+), 84 deletions(-) diff --git a/src/app/managers/summary-aggregation-manager.ts b/src/app/managers/summary-aggregation-manager.ts index 02018b0ab..78e265d9e 100644 --- a/src/app/managers/summary-aggregation-manager.ts +++ b/src/app/managers/summary-aggregation-manager.ts @@ -28,6 +28,11 @@ export interface MessageCompletionInfo { cost?: number; hasToolActivity: boolean; hasReasoningActivity: boolean; + /** True when the completed assistant message carries an upstream error + * (aborted, output length, provider error, ...). Such a completion is not a + * successfully delivered task response. */ + hasError?: boolean; + errorName?: string; } type MessageCompleteCallback = ( @@ -37,6 +42,8 @@ type MessageCompleteCallback = ( completionInfo: MessageCompletionInfo, ) => void; +type AssistantMessageStartedCallback = (sessionId: string, messageId: string) => void; + type MessagePartialCallback = (sessionId: string, messageId: string, messageText: string) => void; export interface ThinkingSection { @@ -304,6 +311,7 @@ class SummaryAggregator { private messageCount = 0; private lastUpdated = 0; private onCompleteCallback: MessageCompleteCallback | null = null; + private onAssistantMessageStartedCallback: AssistantMessageStartedCallback | null = null; private onPartialCallback: MessagePartialCallback | null = null; private onExternalUserInputCallback: ExternalUserInputCallback | null = null; private onToolCallback: ToolCallback | null = null; @@ -359,6 +367,10 @@ class SummaryAggregator { this.onCompleteCallback = callback; } + setOnAssistantMessageStarted(callback: AssistantMessageStartedCallback): void { + this.onAssistantMessageStartedCallback = callback; + } + setOnPartial(callback: MessagePartialCallback): void { this.onPartialCallback = callback; } @@ -1173,6 +1185,11 @@ class SummaryAggregator { }); this.messageCount++; this.startTypingIndicator(); + + // Fired synchronously, like the completion callback: the consumer must + // observe the message start in event order so it can invalidate an + // older pending final response before any newer completion lands. + this.onAssistantMessageStartedCallback?.(info.sessionID, messageID); } const textState = this.getOrCreateTextMessageState(messageID); @@ -1230,6 +1247,8 @@ class SummaryAggregator { cost: typeof info.cost === "number" ? info.cost : undefined, hasToolActivity: activity.hasToolActivity, hasReasoningActivity: activity.hasReasoningActivity, + hasError: info.error !== undefined, + errorName: info.error !== undefined ? String(info.error.name) : undefined, }; logger.debug( diff --git a/src/bot/handlers/prompt.ts b/src/bot/handlers/prompt.ts index 67ebfd519..fc4b38c5e 100644 --- a/src/bot/handlers/prompt.ts +++ b/src/bot/handlers/prompt.ts @@ -69,12 +69,24 @@ interface PromptAttemptState { promptText: string; responseMode: PromptResponseMode; retryDispatched: boolean; - idleGuard: boolean; + /** True once the retry produced a terminal-eligible non-empty response. From + * that point the settle timer is the only thing that finalizes the retry, so + * every session.idle - including a stale duplicate of the original attempt's + * idle - is consumed and cannot finish the retry early. */ + retryResponseDelivered: boolean; + settleTimer: ReturnType | null; workEvidence: TaskAttemptEvidence; } const promptRetryStates = new Map(); +// How long the retry stays "open" after its terminal response before the settle +// timer finalizes it. Long enough to absorb a burst of stale/duplicate idle +// events from the original attempt, short enough to stay imperceptible. +const RETRY_SETTLE_MS = 300; + +let onRetrySettleCallback: ((sessionId: string) => void) | null = null; + export type EmptyCompletionOutcome = "retried" | "failed" | "no_retry" | "ignored"; export type PromptResponseMode = "text_only" | "text_and_tts"; @@ -107,21 +119,39 @@ export function consumePromptResponseMode(sessionId: string): PromptResponseMode export function registerPromptRetry( sessionId: string, - state: Omit, + state: Omit< + PromptAttemptState, + "retryDispatched" | "retryResponseDelivered" | "settleTimer" | "workEvidence" + >, ): void { promptRetryStates.set(sessionId, { ...state, retryDispatched: false, - idleGuard: false, + retryResponseDelivered: false, + settleTimer: null, workEvidence: createEmptyTaskAttemptEvidence(), }); } +function clearRetrySettleTimer(state: PromptAttemptState): void { + if (state.settleTimer) { + clearTimeout(state.settleTimer); + state.settleTimer = null; + } +} + export function clearPromptRetry(sessionId: string): void { + const state = promptRetryStates.get(sessionId); + if (state) { + clearRetrySettleTimer(state); + } promptRetryStates.delete(sessionId); } export function clearAllPromptRetry(): void { + for (const state of promptRetryStates.values()) { + clearRetrySettleTimer(state); + } promptRetryStates.clear(); } @@ -133,6 +163,59 @@ export function getPromptRetryChatId(sessionId: string): number | null { return promptRetryStates.get(sessionId)?.chatId ?? null; } +/** + * Arms (or re-arms) the retry settle timer. When it fires, the retry is + * finalized through the registered callback - the same idle finalization used + * by a normal run. The timer is cancelled on every retry lifecycle invalidation + * and replaced by a fresh one on every consumed idle and new message start, so + * a stale idle from the original attempt can never finalize the retry before + * its own idle. + */ +function armRetrySettle(state: PromptAttemptState, sessionId: string): void { + clearRetrySettleTimer(state); + + state.settleTimer = setTimeout(() => { + state.settleTimer = null; + if (promptRetryStates.get(sessionId) !== state || !state.retryResponseDelivered) { + return; + } + onRetrySettleCallback?.(sessionId); + }, RETRY_SETTLE_MS); +} + +export function setOnRetrySettle(callback: ((sessionId: string) => void) | null): void { + onRetrySettleCallback = callback; +} + +/** + * Records that the retry produced a terminal-eligible non-empty response. The + * retry state is kept alive so every subsequent idle is still consumed, and the + * settle timer becomes the single point where the retry finalizes. + */ +export function markPromptRetryResponseDelivered(sessionId: string): void { + const state = promptRetryStates.get(sessionId); + if (!state) { + return; + } + + state.retryResponseDelivered = true; + armRetrySettle(state, sessionId); +} + +/** + * Extends the retry settle window because the retry produced more activity + * (a newer assistant message started), meaning the current candidate was + * intermediate. No-op unless a retry response was already delivered. + */ +export function resetPromptRetrySettle(sessionId: string): void { + const state = promptRetryStates.get(sessionId); + if (!state || !state.retryResponseDelivered) { + return; + } + + armRetrySettle(state, sessionId); +} + /** * Merges the completion evidence of one assistant turn into the running * attempt-wide evidence, so a later empty completion is judged by everything the @@ -151,13 +234,26 @@ export function recordAttemptEvidence( } /** - * Guards the idle event that closes the original empty completion while the - * automatic retry is still in flight. A stale or duplicate idle must never - * finish the active retry early, emit a footer, or dispatch queued work. The - * guard stays up until the retry completion clears the state. + * Guards every idle event that belongs to a retry lifecycle. The guard only + * activates once the retry has actually been dispatched - a registered but + * never-replayed attempt must not swallow the normal idle finalization. While + * the guard is up the idle is consumed (never finalized); once a retry response + * was delivered the consume also extends the settle window, so the retry is + * finalized by its settle timer rather than by the first idle that happens to + * arrive. A stale or duplicate idle from the original attempt therefore cannot + * finish the retry early, emit a footer, commit /lastfile, export Markdown, + * mark the foreground idle, or dispatch queued prompts. */ export function consumePromptRetryIdle(sessionId: string): boolean { - return promptRetryStates.get(sessionId)?.idleGuard === true; + const state = promptRetryStates.get(sessionId); + if (!state || !state.retryDispatched) { + return false; + } + + if (state.retryResponseDelivered) { + armRetrySettle(state, sessionId); + } + return true; } /** @@ -218,7 +314,8 @@ export function retryPromptOnce(sessionId: string): boolean { } state.retryDispatched = true; - state.idleGuard = true; + clearRetrySettleTimer(state); + state.retryResponseDelivered = false; state.workEvidence = createEmptyTaskAttemptEvidence(); foregroundSessionState.markBusy(sessionId, state.promptOptions.directory); void markAttachedSessionBusy(sessionId); diff --git a/src/bot/services/empty-completion-policy.ts b/src/bot/services/empty-completion-policy.ts index 2fbf28823..10e8494b4 100644 --- a/src/bot/services/empty-completion-policy.ts +++ b/src/bot/services/empty-completion-policy.ts @@ -97,3 +97,55 @@ export function isSafeZeroWorkEmptyCompletion(evidence: TaskAttemptEvidence): bo hasUnknownFinishReason(evidence.finishReason) ); } + +/** + * Finish reasons that prove a completed assistant message did NOT end with a + * terminal, successfully delivered answer. Anything else (including an unknown + * or missing reason) is treated as potentially terminal: the primary signals + * for a truncated or interrupted response are the upstream message error and + * tool activity, so the finish reason only ever disqualifies a known + * non-terminal value and never rejects a normal answer. + */ +const KNOWN_NON_TERMINAL_FINISH_REASONS = new Set([ + "max_tokens", + "length", + "error", + "content_filter", + "tool_use", + "function_call", + "aborted", + "incomplete", +]); + +export function isKnownNonTerminalFinishReason(finishReason: string | undefined): boolean { + const normalized = finishReason?.trim().toLowerCase() ?? ""; + return normalized.length > 0 && KNOWN_NON_TERMINAL_FINISH_REASONS.has(normalized); +} + +/** + * A completed non-empty assistant message is a terminal final-response + * candidate only when it survived with no upstream error, performed no tool + * activity (a message that called a tool is never the closing answer of a + * run), and did not end on a known non-terminal finish reason. The check is + * deliberately conservative: a run whose last message fails any of these + * signals is never reported as a completed task. + */ +export function isTerminalAssistantResponse(completion: { + hasError?: boolean; + hasToolActivity?: boolean; + finishReason?: string; +}): boolean { + if (completion.hasError) { + return false; + } + + if (completion.hasToolActivity) { + return false; + } + + if (isKnownNonTerminalFinishReason(completion.finishReason)) { + return false; + } + + return true; +} diff --git a/src/bot/services/event-subscription-service.ts b/src/bot/services/event-subscription-service.ts index 3a535ea09..4cef8bc58 100644 --- a/src/bot/services/event-subscription-service.ts +++ b/src/bot/services/event-subscription-service.ts @@ -41,7 +41,10 @@ import { getPromptRetryChatId, handleEmptyCompletion, hasPromptRetryAttempted, + markPromptRetryResponseDelivered, recordAttemptEvidence, + resetPromptRetrySettle, + setOnRetrySettle, } from "../handlers/prompt.js"; import { reconcileBusyState, @@ -109,6 +112,7 @@ import { } from "./assistant-response-export-service.js"; import { isGenuinelyEmptyAssistantResponse, + isTerminalAssistantResponse, } from "./empty-completion-policy.js"; const TELEGRAM_DOCUMENT_CAPTION_MAX_LENGTH = 1024; @@ -164,14 +168,16 @@ class EventSubscriptionService implements BotEventSubscriptionService { { callId: string; activity: string } >(); private readonly subagentSnapshots = new Map(); - // Terminal assistant response of the in-flight run, keyed by session. Replaced - // by each delivered non-empty completion and marked null when a completion is - // empty, so a run that ends without a deliverable never overwrites the - // previous last good response. The chatId is captured from the originating - // completion rather than the mutable service-wide chat context. + // Terminal assistant response of the in-flight run, keyed by session. A + // candidate is only established by a terminal-eligible (non-empty, no error, + // no tool activity) completion and is invalidated whenever a newer assistant + // message starts; marked null when a completion is empty, so a run that ends + // without a deliverable never overwrites the previous last good response. The + // chatId is captured from the originating prompt rather than the mutable + // service-wide chat context. private readonly pendingFinalResponses = new Map< string, - { chatId: number; text: string } | null + { chatId: number; text: string; messageId: string; hasError: boolean } | null >(); constructor() { @@ -247,6 +253,9 @@ class EventSubscriptionService implements BotEventSubscriptionService { hasActiveStream: (sessionId) => this.hasActiveAssistantResponseStream(sessionId), }); setPromptResponseModeClearerForReconciliation(clearPromptResponseMode); + setOnRetrySettle((sessionId) => { + void this.handleRetrySettle(sessionId); + }); this.compactProgressStreamer = new CompactProgressStreamer({ throttleMs: RESPONSE_STREAM_THROTTLE_MS, @@ -554,6 +563,17 @@ class EventSubscriptionService implements BotEventSubscriptionService { this.clearToolElapsedState(null, "summary_aggregator_clear"); }); + summaryAggregator.setOnAssistantMessageStarted((sessionId, messageId) => { + // Routed through the same serialized task queue as completions so the + // invalidation is observed in event order: an older completion that set + // the candidate always lands before the start that supersedes it. + void this.enqueueSessionCompletionTask(sessionId, () => { + this.invalidateIntermediateFinalCandidate(sessionId, messageId); + resetPromptRetrySettle(sessionId); + return Promise.resolve(); + }); + }); + summaryAggregator.setOnPartial((sessionId, messageId, messageText) => { if (!this.botInstance || !this.chatIdInstance) { return; @@ -657,14 +677,17 @@ class EventSubscriptionService implements BotEventSubscriptionService { return; } - if (hasPromptRetryAttempted(sessionId)) { - clearPromptRetry(sessionId); + const isTerminalResponse = isTerminalAssistantResponse(completionInfo); + if (isTerminalResponse) { + if (hasPromptRetryAttempted(sessionId)) { + markPromptRetryResponseDelivered(sessionId); + } + assistantRunState.markResponseCompleted(sessionId, { + agent: completionInfo.agent, + providerID: completionInfo.providerID, + modelID: completionInfo.modelID, + }); } - assistantRunState.markResponseCompleted(sessionId, { - agent: completionInfo.agent, - providerID: completionInfo.providerID, - modelID: completionInfo.modelID, - }); await this.completeThinkingStream(sessionId, messageId); @@ -710,12 +733,23 @@ class EventSubscriptionService implements BotEventSubscriptionService { }, }); - this.pendingFinalResponses.set(sessionId, { chatId, text: messageText }); + // Intermediate commentary, truncated output, or an errored/aborted + // message is streamed so the user sees it, but never becomes the + // /lastfile candidate or the automatic Markdown export. + const originChatId = getPromptRetryChatId(sessionId) ?? chatId; + if (isTerminalResponse) { + this.pendingFinalResponses.set(sessionId, { + chatId: originChatId, + text: messageText, + messageId, + hasError: false, + }); + } await sendTtsResponseForSession({ api: botApi, sessionId, - chatId, + chatId: originChatId, text: messageText, }); } catch (err) { @@ -1173,64 +1207,12 @@ class EventSubscriptionService implements BotEventSubscriptionService { if (consumePromptRetryIdle(sessionId)) { logger.debug( - `[Bot] Ignoring idle event that closed the retried empty completion: session=${sessionId}`, + `[Bot] Ignoring idle event while a retry lifecycle is active: session=${sessionId}`, ); return; } - await markAttachedSessionIdle(sessionId); - - const completedRun = assistantRunState.finishRun(sessionId, "session_idle"); - clearPromptResponseMode(sessionId); - await this.commitPendingFinalResponse(sessionId, completedRun); - clearPromptRetry(sessionId); - - if (!this.botInstance || !this.chatIdInstance) { - foregroundSessionState.markIdle(sessionId); - return; - } - - const currentSession = getCurrentSession(); - if (!currentSession || currentSession.id !== sessionId) { - foregroundSessionState.markIdle(sessionId); - await scheduledTaskRuntime.flushDeferredDeliveries(); - return; - } - - try { - await Promise.all([ - this.toolMessageBatcher.flushSession(sessionId, "session_idle"), - this.toolCallStreamer.flushSession(sessionId, "session_idle"), - ]); - - if (getShowAssistantRunFooter() && completedRun?.hasCompletedResponse) { - const agent = completedRun.actualAgent || completedRun.configuredAgent; - const providerID = completedRun.actualProviderID || completedRun.configuredProviderID; - const modelID = completedRun.actualModelID || completedRun.configuredModelID; - - if (agent && providerID && modelID) { - const keyboard = this.getCurrentReplyKeyboard(); - await this.botInstance.api.sendMessage( - this.chatIdInstance, - formatAssistantRunFooter({ - agent, - providerID, - modelID, - elapsedMs: Date.now() - completedRun.startedAt, - }), - { - ...(keyboard ? { reply_markup: keyboard } : {}), - }, - ); - } - } - } catch (err) { - logger.error("[Bot] Failed to send session idle footer:", err); - } finally { - foregroundSessionState.markIdle(sessionId); - await scheduledTaskRuntime.flushDeferredDeliveries(); - void dispatchNextQueuedPrompt(); - } + await this.finalizeIdleSession(sessionId); }); summaryAggregator.setOnSessionError(async (sessionId, message) => { @@ -1677,26 +1659,121 @@ class EventSubscriptionService implements BotEventSubscriptionService { return nextTask; } + /** + * A newer assistant message starting proves the previous pending candidate + * was only intermediate commentary. The candidate is dropped so a run that + * never delivers a terminal answer cannot commit it as /lastfile, export it, + * or report the task as complete. + */ + private invalidateIntermediateFinalCandidate(sessionId: string, messageId: string): void { + const pending = this.pendingFinalResponses.get(sessionId); + if (!pending || pending.messageId === messageId) { + return; + } + + logger.debug( + `[Bot] Invalidated intermediate final response candidate: session=${sessionId}, supersededMessageId=${pending.messageId}, startedMessageId=${messageId}`, + ); + this.pendingFinalResponses.delete(sessionId); + } + + /** + * Finalizes a retried run once its settle window has passed. The retry's own + * idle events are all consumed by the guard, so this is the single point + * where the retry is committed and its footer is emitted - a stale idle from + * the original attempt can never reach this path. + */ + private async handleRetrySettle(sessionId: string): Promise { + logger.debug(`[Bot] Finalizing retried completion after settle window: session=${sessionId}`); + this.clearToolElapsedState(sessionId, "retry_settle"); + await this.sessionCompletionTasks.get(sessionId)?.catch(() => undefined); + await this.finalizeIdleSession(sessionId); + } + + /** + * Shared idle finalization for a normal run and for a retried run whose + * settle timer fired. The success footer is emitted only when a terminal + * final response was actually committed, so a run that ended on intermediate + * or truncated output is never announced as a completed task. + */ + private async finalizeIdleSession(sessionId: string): Promise { + await markAttachedSessionIdle(sessionId); + + const completedRun = assistantRunState.finishRun(sessionId, "session_idle"); + clearPromptResponseMode(sessionId); + const committedChatId = await this.commitPendingFinalResponse(sessionId, completedRun); + clearPromptRetry(sessionId); + + if (!this.botInstance || !this.chatIdInstance) { + foregroundSessionState.markIdle(sessionId); + return; + } + + const currentSession = getCurrentSession(); + if (!currentSession || currentSession.id !== sessionId) { + foregroundSessionState.markIdle(sessionId); + await scheduledTaskRuntime.flushDeferredDeliveries(); + return; + } + + try { + await Promise.all([ + this.toolMessageBatcher.flushSession(sessionId, "session_idle"), + this.toolCallStreamer.flushSession(sessionId, "session_idle"), + ]); + + if (getShowAssistantRunFooter() && completedRun && committedChatId !== null) { + const agent = completedRun.actualAgent || completedRun.configuredAgent; + const providerID = completedRun.actualProviderID || completedRun.configuredProviderID; + const modelID = completedRun.actualModelID || completedRun.configuredModelID; + + if (agent && providerID && modelID) { + const keyboard = this.getCurrentReplyKeyboard(); + await this.botInstance.api.sendMessage( + committedChatId, + formatAssistantRunFooter({ + agent, + providerID, + modelID, + elapsedMs: Date.now() - completedRun.startedAt, + }), + { + ...(keyboard ? { reply_markup: keyboard } : {}), + }, + ); + } + } + } catch (err) { + logger.error("[Bot] Failed to send session idle footer:", err); + } finally { + foregroundSessionState.markIdle(sessionId); + await scheduledTaskRuntime.flushDeferredDeliveries(); + void dispatchNextQueuedPrompt(); + } + } + /** * Stores the terminal successfully delivered assistant response of a finished - * run for /lastfile and automatic Markdown export. Intermediate commentary - * only overwrites the pending slot during the run, and a failed or empty run - * never commits, so the previous last good response is preserved. + * run for /lastfile and automatic Markdown export, returning the originating + * chat the response is bound to (or null when nothing was committed). + * Intermediate commentary, errored/truncated messages, and empty runs never + * commit, so the previous last good response is preserved. */ private async commitPendingFinalResponse( sessionId: string, completedRun: AssistantRunInfo | null, - ): Promise { + ): Promise { const pending = this.pendingFinalResponses.get(sessionId); this.pendingFinalResponses.delete(sessionId); if ( pending === undefined || pending === null || + pending.hasError || !completedRun?.hasCompletedResponse || !this.botInstance ) { - return; + return null; } rememberAssistantResponse(pending.chatId, sessionId, pending.text); @@ -1710,6 +1787,7 @@ class EventSubscriptionService implements BotEventSubscriptionService { }, ); } + return pending.chatId; } private finalizeCompactProgress(sessionId: string): Promise { diff --git a/tests/bot/services/empty-completion-policy.test.ts b/tests/bot/services/empty-completion-policy.test.ts index e26b939cc..d0ad74271 100644 --- a/tests/bot/services/empty-completion-policy.test.ts +++ b/tests/bot/services/empty-completion-policy.test.ts @@ -4,6 +4,7 @@ import { createEmptyTaskAttemptEvidence, isGenuinelyEmptyAssistantResponse, isSafeZeroWorkEmptyCompletion, + isTerminalAssistantResponse, mergeTaskAttemptEvidence, } from "../../../src/bot/services/empty-completion-policy.js"; @@ -112,4 +113,47 @@ describe("empty completion policy", () => { expect(mergeTaskAttemptEvidence(tooled, createInfo()).hasToolActivity).toBe(true); }); }); + + describe("terminal response detection", () => { + it("treats a clean non-empty completion as terminal", () => { + expect(isTerminalAssistantResponse({ hasError: false, hasToolActivity: false })).toBe(true); + expect(isTerminalAssistantResponse({})).toBe(true); + }); + + it("rejects an errored or aborted completion", () => { + expect(isTerminalAssistantResponse({ hasError: true })).toBe(false); + expect(isTerminalAssistantResponse({ hasError: true, hasToolActivity: false })).toBe(false); + }); + + it("rejects a completion whose message called a tool", () => { + expect(isTerminalAssistantResponse({ hasToolActivity: true })).toBe(false); + }); + + it("rejects known non-terminal finish reasons but accepts unknown ones", () => { + expect(isTerminalAssistantResponse({ finishReason: "max_tokens" })).toBe(false); + expect(isTerminalAssistantResponse({ finishReason: "length" })).toBe(false); + expect(isTerminalAssistantResponse({ finishReason: "content_filter" })).toBe(false); + expect(isTerminalAssistantResponse({ finishReason: "tool_use" })).toBe(false); + expect(isTerminalAssistantResponse({ finishReason: "end_turn" })).toBe(true); + expect(isTerminalAssistantResponse({ finishReason: undefined })).toBe(true); + expect(isTerminalAssistantResponse({ finishReason: "unusual-reason" })).toBe(true); + }); + + it("fails closed when error and tool activity are combined with otherwise clean signals", () => { + expect( + isTerminalAssistantResponse({ + hasError: true, + hasToolActivity: false, + finishReason: "end_turn", + }), + ).toBe(false); + expect( + isTerminalAssistantResponse({ + hasError: false, + hasToolActivity: true, + finishReason: "end_turn", + }), + ).toBe(false); + }); + }); }); diff --git a/tests/bot/services/event-subscription-service.test.ts b/tests/bot/services/event-subscription-service.test.ts index d494bd88e..db06257c8 100644 --- a/tests/bot/services/event-subscription-service.test.ts +++ b/tests/bot/services/event-subscription-service.test.ts @@ -864,6 +864,49 @@ describe("bot/services/event-subscription-service", () => { } as unknown as Event); } + function emitAssistantStarted( + aggregator: { processEvent(event: Event): void }, + messageId: string, + ): void { + aggregator.processEvent({ + type: "message.updated", + properties: { + info: { + id: messageId, + sessionID: "session-1", + role: "assistant", + time: { created: Date.now() }, + }, + }, + } as unknown as Event); + } + + function emitToolPart( + aggregator: { processEvent(event: Event): void }, + messageId: string, + callId: string, + ): void { + aggregator.processEvent({ + type: "message.part.updated", + properties: { + part: { + id: `tool-${callId}`, + sessionID: "session-1", + messageID: messageId, + type: "tool", + callID: callId, + tool: "bash", + state: { + status: "completed", + input: { command: "npm test" }, + metadata: {}, + output: "ok", + }, + }, + }, + } as unknown as Event); + } + function emitZeroWorkEmptyCompletion( aggregator: { processEvent(event: Event): void }, messageId: string, @@ -964,7 +1007,12 @@ describe("bot/services/event-subscription-service", () => { api.sendMessage.mock.calls.some(([, text]) => String(text) === "Final answer"), ).toBe(true); }); - expect(countFooters(api)).toBe(1); + await vi.waitFor( + () => { + expect(countFooters(api)).toBe(1); + }, + { timeout: 5000 }, + ); expect(mocked.safeBackgroundTask).toHaveBeenCalledTimes(1); }); @@ -1229,5 +1277,196 @@ describe("bot/services/event-subscription-service", () => { expect(promptModule.handleEmptyCompletion("session-2")).toBe("ignored"); promptModule.clearPromptRetry("session-1"); }); + + describe("final response terminality", () => { + async function startFreshRun(): Promise { + const { assistantRunState } = + await import("../../../src/app/managers/assistant-run-state-manager.js"); + assistantRunState.startRun("session-1", { + startedAt: Date.now(), + configuredAgent: "test-agent", + configuredProviderID: "test-provider", + configuredModelID: "test-model", + }); + } + + it("never reports an intermediate commentary run as complete when tool work follows", async () => { + const { api, summaryAggregator } = await setupService(false, { startAssistantRun: true }); + const exportService = + await import("../../../src/bot/services/assistant-response-export-service.js"); + + emitAssistantText(summaryAggregator, "Good answer", "message-0"); + emitAssistantCompleted(summaryAggregator, "message-0"); + emitSessionIdle(summaryAggregator); + await vi.waitFor(() => { + expect(exportService.getRememberedAssistantResponse(42, "session-1")).toBe("Good answer"); + }); + const footersBefore = countFooters(api); + + await startFreshRun(); + + emitAssistantText(summaryAggregator, "Let me check the files", "message-1"); + emitAssistantCompleted(summaryAggregator, "message-1"); + + emitAssistantStarted(summaryAggregator, "message-2"); + emitAssistantText(summaryAggregator, "Running the tests", "message-2"); + emitToolPart(summaryAggregator, "message-2", "call-tests"); + emitAssistantCompleted(summaryAggregator, "message-2"); + emitSessionIdle(summaryAggregator); + + await flushRealDispatch(); + await vi.waitFor(() => { + expect(exportService.getRememberedAssistantResponse(42, "session-1")).toBe("Good answer"); + }); + expect(countFooters(api)).toBe(footersBefore); + expect(api.sendDocument).not.toHaveBeenCalled(); + expect(exportService.getRememberedAssistantResponse(42, "session-1")).toBe("Good answer"); + }); + + it("does not announce a truncated/errored completion as a completed task", async () => { + const { api, summaryAggregator } = await setupService(false, { startAssistantRun: true }); + const exportService = + await import("../../../src/bot/services/assistant-response-export-service.js"); + + emitAssistantText(summaryAggregator, "Good answer", "message-0"); + emitAssistantCompleted(summaryAggregator, "message-0"); + emitSessionIdle(summaryAggregator); + await vi.waitFor(() => { + expect(exportService.getRememberedAssistantResponse(42, "session-1")).toBe("Good answer"); + }); + const footersBefore = countFooters(api); + + await startFreshRun(); + + emitAssistantText(summaryAggregator, "Partial answer that got cut off", "message-1"); + emitAssistantCompleted(summaryAggregator, "message-1", { + error: { name: "MessageAbortedError", data: { message: "aborted" } }, + }); + emitSessionIdle(summaryAggregator); + + await flushRealDispatch(); + await vi.waitFor(() => { + expect(exportService.getRememberedAssistantResponse(42, "session-1")).toBe("Good answer"); + }); + expect(countFooters(api)).toBe(footersBefore); + expect(api.sendDocument).not.toHaveBeenCalled(); + expect(exportService.getRememberedAssistantResponse(42, "session-1")).toBe("Good answer"); + }); + + it("emits exactly one footer and updates /lastfile for a normal short terminal response", async () => { + const { api, summaryAggregator } = await setupService(false, { startAssistantRun: true }); + const exportService = + await import("../../../src/bot/services/assistant-response-export-service.js"); + + emitAssistantText(summaryAggregator, "Short answer", "message-1"); + emitAssistantCompleted(summaryAggregator, "message-1"); + emitSessionIdle(summaryAggregator); + + await vi.waitFor(() => { + expect(exportService.getRememberedAssistantResponse(42, "session-1")).toBe( + "Short answer", + ); + }); + expect(countFooters(api)).toBe(1); + expect(api.sendDocument).not.toHaveBeenCalled(); + }); + + it("emits one footer, updates /lastfile, and attaches one document for a long terminal response", async () => { + const { api, summaryAggregator } = await setupService(false, { startAssistantRun: true }); + const exportService = + await import("../../../src/bot/services/assistant-response-export-service.js"); + + const longText = `# Heading\n\n${"x".repeat(6000)}`; + emitAssistantText(summaryAggregator, longText, "message-1"); + emitAssistantCompleted(summaryAggregator, "message-1"); + emitSessionIdle(summaryAggregator); + + await vi.waitFor(() => { + expect(api.sendDocument).toHaveBeenCalledTimes(1); + }); + expect(exportService.getRememberedAssistantResponse(42, "session-1")).toBe(longText); + expect(countFooters(api)).toBe(1); + }); + + it("keeps a stale original idle inert after the retry response and finalizes exactly once", async () => { + const { api, summaryAggregator } = await setupService(false, { startAssistantRun: true }); + const exportService = + await import("../../../src/bot/services/assistant-response-export-service.js"); + const { foregroundSessionState } = + await import("../../../src/app/managers/foreground-session-state-manager.js"); + await registerPromptAttempt(api); + + emitZeroWorkEmptyCompletion(summaryAggregator, "message-1"); + emitSessionIdle(summaryAggregator); + + await vi.waitFor(() => { + expect(mocked.safeBackgroundTask).toHaveBeenCalledWith( + expect.objectContaining({ taskName: "session.promptAsync.retry" }), + ); + }); + + const retryOptions = getRetryTaskOptions(); + await retryOptions.task(); + retryOptions.onSuccess?.({ error: null }); + + emitAssistantText(summaryAggregator, "Recovered answer", "message-2"); + emitAssistantCompleted(summaryAggregator, "message-2"); + + // Stale duplicate idle of the ORIGINAL attempt arrives AFTER the retry + // produced its response. It must be fully inert. + emitSessionIdle(summaryAggregator); + await flushRealDispatch(); + expect(countFooters(api)).toBe(0); + expect(exportService.getRememberedAssistantResponse(42, "session-1")).toBeNull(); + expect(api.sendDocument).not.toHaveBeenCalled(); + expect(foregroundSessionState.isBusy()).toBe(true); + expect(mocked.dispatchNextQueuedPrompt).not.toHaveBeenCalled(); + + // The real retry idle then finalizes exactly once. + emitSessionIdle(summaryAggregator); + await vi.waitFor( + () => { + expect(countFooters(api)).toBe(1); + }, + { timeout: 5000 }, + ); + expect(exportService.getRememberedAssistantResponse(42, "session-1")).toBe( + "Recovered answer", + ); + expect(foregroundSessionState.isBusy()).toBe(false); + expect(countFooters(api)).toBe(1); + expect(mocked.safeBackgroundTask).toHaveBeenCalledTimes(1); + }); + + it("binds the final response, export, and footer to the originating chat", async () => { + const { api, summaryAggregator, service } = await setupService(false, { + startAssistantRun: true, + }); + const exportService = + await import("../../../src/bot/services/assistant-response-export-service.js"); + const newBot = createFakeBot(); + await registerPromptAttempt(api, 42); + + // The mutable service-wide chat context moves to another chat mid-run. + service.setTelegramContext(newBot.bot, 43); + + const longText = `# Heading\n\n${"x".repeat(6000)}`; + emitAssistantText(summaryAggregator, longText, "message-1"); + emitAssistantCompleted(summaryAggregator, "message-1"); + emitSessionIdle(summaryAggregator); + + await vi.waitFor(() => { + expect(newBot.api.sendDocument).toHaveBeenCalledWith(42, expect.anything()); + }); + expect(exportService.getRememberedAssistantResponse(42, "session-1")).toBe(longText); + expect(exportService.getRememberedAssistantResponse(43, "session-1")).toBeNull(); + + const footerCalls = newBot.api.sendMessage.mock.calls.filter(([, text]) => + String(text).includes("test-provider/test-model"), + ); + expect(footerCalls).toHaveLength(1); + expect(footerCalls[0][0]).toBe(42); + }); + }); }); }); From 35ac9559ee3fd33afe8d46b0cadd045dd9979be8 Mon Sep 17 00:00:00 2001 From: Vijay Kumar Date: Tue, 11 Aug 2026 00:12:55 +0530 Subject: [PATCH 4/4] fix: require explicit terminal finish and authoritative retry idle - Terminality now fails closed: only the OpenCode FinishReason value "stop" (the sole successful terminal value in the @opencode-ai/llm literal schema) qualifies a non-empty assistant message as a final response. Missing, unknown, or arbitrary finish reasons are never treated as terminal success, and the per-step step-finish reason is no longer substituted for the authoritative message-level info.finish. - Remove the 300ms retry settle timer heuristic. A retry is finalized only when a session.idle is confirmed against the authoritative OpenCode session status: missing/"idle" means genuinely finished, while "busy"/"retry" (or a failed or ambiguous lookup) treats the idle as stale and consumes it without finalizing anything. Exactly one automatic retry remains the maximum. --- .../managers/summary-aggregation-manager.ts | 13 +- src/bot/handlers/prompt.ts | 158 +++++++-------- src/bot/services/empty-completion-policy.ts | 56 +++--- .../services/event-subscription-service.ts | 35 +--- .../services/empty-completion-policy.test.ts | 56 ++++-- ...ent-subscription-service.lifecycle.test.ts | 2 + .../event-subscription-service.test.ts | 186 +++++++++++++++++- 7 files changed, 333 insertions(+), 173 deletions(-) diff --git a/src/app/managers/summary-aggregation-manager.ts b/src/app/managers/summary-aggregation-manager.ts index 78e265d9e..efd6b57d2 100644 --- a/src/app/managers/summary-aggregation-manager.ts +++ b/src/app/managers/summary-aggregation-manager.ts @@ -196,7 +196,6 @@ interface TextMessageState { } interface MessageActivityState { - finishReason?: string; hasToolActivity: boolean; hasReasoningActivity: boolean; } @@ -1231,10 +1230,12 @@ class SummaryAggregator { modelID: info.modelID, createdAt: time?.created, completedAt: time?.completed, + // Authoritative message finish only: OpenCode always stamps the + // message-level finish reason when a run ends; a missing value means + // the message was interrupted and must fail closed, so a per-step + // finish reason is never substituted here. finishReason: - typeof info.finish === "string" && info.finish.trim() - ? info.finish.trim() - : activity.finishReason, + typeof info.finish === "string" && info.finish.trim() ? info.finish.trim() : undefined, tokens: info.tokens ? { input: info.tokens.input, @@ -1557,10 +1558,6 @@ class SummaryAggregator { } } - if (part.type === "step-finish" && typeof part.reason === "string" && part.reason.trim()) { - activity.finishReason = part.reason.trim(); - } - this.lastUpdated = Date.now(); } diff --git a/src/bot/handlers/prompt.ts b/src/bot/handlers/prompt.ts index fc4b38c5e..d20acd72f 100644 --- a/src/bot/handlers/prompt.ts +++ b/src/bot/handlers/prompt.ts @@ -70,23 +70,15 @@ interface PromptAttemptState { responseMode: PromptResponseMode; retryDispatched: boolean; /** True once the retry produced a terminal-eligible non-empty response. From - * that point the settle timer is the only thing that finalizes the retry, so - * every session.idle - including a stale duplicate of the original attempt's - * idle - is consumed and cannot finish the retry early. */ + * that point an idle is only finalized after the authoritative OpenCode + * session status confirms the session is genuinely idle; a stale idle from + * the original attempt while the retry is still busy is consumed instead. */ retryResponseDelivered: boolean; - settleTimer: ReturnType | null; workEvidence: TaskAttemptEvidence; } const promptRetryStates = new Map(); -// How long the retry stays "open" after its terminal response before the settle -// timer finalizes it. Long enough to absorb a burst of stale/duplicate idle -// events from the original attempt, short enough to stay imperceptible. -const RETRY_SETTLE_MS = 300; - -let onRetrySettleCallback: ((sessionId: string) => void) | null = null; - export type EmptyCompletionOutcome = "retried" | "failed" | "no_retry" | "ignored"; export type PromptResponseMode = "text_only" | "text_and_tts"; @@ -119,39 +111,21 @@ export function consumePromptResponseMode(sessionId: string): PromptResponseMode export function registerPromptRetry( sessionId: string, - state: Omit< - PromptAttemptState, - "retryDispatched" | "retryResponseDelivered" | "settleTimer" | "workEvidence" - >, + state: Omit, ): void { promptRetryStates.set(sessionId, { ...state, retryDispatched: false, retryResponseDelivered: false, - settleTimer: null, workEvidence: createEmptyTaskAttemptEvidence(), }); } -function clearRetrySettleTimer(state: PromptAttemptState): void { - if (state.settleTimer) { - clearTimeout(state.settleTimer); - state.settleTimer = null; - } -} - export function clearPromptRetry(sessionId: string): void { - const state = promptRetryStates.get(sessionId); - if (state) { - clearRetrySettleTimer(state); - } promptRetryStates.delete(sessionId); } export function clearAllPromptRetry(): void { - for (const state of promptRetryStates.values()) { - clearRetrySettleTimer(state); - } promptRetryStates.clear(); } @@ -163,34 +137,10 @@ export function getPromptRetryChatId(sessionId: string): number | null { return promptRetryStates.get(sessionId)?.chatId ?? null; } -/** - * Arms (or re-arms) the retry settle timer. When it fires, the retry is - * finalized through the registered callback - the same idle finalization used - * by a normal run. The timer is cancelled on every retry lifecycle invalidation - * and replaced by a fresh one on every consumed idle and new message start, so - * a stale idle from the original attempt can never finalize the retry before - * its own idle. - */ -function armRetrySettle(state: PromptAttemptState, sessionId: string): void { - clearRetrySettleTimer(state); - - state.settleTimer = setTimeout(() => { - state.settleTimer = null; - if (promptRetryStates.get(sessionId) !== state || !state.retryResponseDelivered) { - return; - } - onRetrySettleCallback?.(sessionId); - }, RETRY_SETTLE_MS); -} - -export function setOnRetrySettle(callback: ((sessionId: string) => void) | null): void { - onRetrySettleCallback = callback; -} - /** * Records that the retry produced a terminal-eligible non-empty response. The - * retry state is kept alive so every subsequent idle is still consumed, and the - * settle timer becomes the single point where the retry finalizes. + * retry state is kept alive so the retry is only finalized once the + * authoritative OpenCode session status confirms the session is genuinely idle. */ export function markPromptRetryResponseDelivered(sessionId: string): void { const state = promptRetryStates.get(sessionId); @@ -199,21 +149,77 @@ export function markPromptRetryResponseDelivered(sessionId: string): void { } state.retryResponseDelivered = true; - armRetrySettle(state, sessionId); } /** - * Extends the retry settle window because the retry produced more activity - * (a newer assistant message started), meaning the current candidate was - * intermediate. No-op unless a retry response was already delivered. + * Queries the authoritative OpenCode session status for a retried run. OpenCode + * keeps a session in its active status map while it is busy and deletes it when + * it goes idle (publishing session.idle at the same time), and its own + * `SessionStatus.get()` defaults a missing session to "idle" - so a missing or + * "idle" entry is positive proof the run finished. A failed or unexpected + * lookup returns "unknown". */ -export function resetPromptRetrySettle(sessionId: string): void { +async function queryAuthoritativeSessionState( + sessionId: string, + directory: string, +): Promise<"idle" | "busy" | "unknown"> { + try { + const { data, error } = await opencodeClient.session.status({ directory }); + + if (error || !data) { + logger.warn(`[Bot] Failed to verify retry session status: session=${sessionId}`, error); + return "unknown"; + } + + const status = (data as Record)[sessionId]; + if (!status || status.type === "idle") { + return "idle"; + } + + if (status.type === "busy" || status.type === "retry") { + return "busy"; + } + + return "unknown"; + } catch (err) { + logger.warn(`[Bot] Error verifying retry session status: session=${sessionId}`, err); + return "unknown"; + } +} + +export type RetryIdleDecision = "none" | "consumed" | "finalize"; + +/** + * Decides how a session.idle during a retry lifecycle should be handled. + * + * - `none`: no active retry for this session; the idle is a normal one. + * - `consumed`: the idle belongs to the retry (the original attempt's idle, a + * stale duplicate, or the retry still busy per the authoritative status). It + * must not finalize anything. + * - `finalize`: the retry produced a terminal response AND the authoritative + * OpenCode session status confirms the session is genuinely idle, so the + * caller finalizes the run exactly once. + * + * The guard only activates once the retry is actually dispatched, so a + * registered-but-unreplayed attempt never swallows a normal run's idle. When + * the status lookup fails or is ambiguous the idle is consumed and success is + * never finalized (fail closed). + */ +export async function decidePromptRetryIdle(sessionId: string): Promise { const state = promptRetryStates.get(sessionId); - if (!state || !state.retryResponseDelivered) { - return; + if (!state || !state.retryDispatched) { + return "none"; } - armRetrySettle(state, sessionId); + if (!state.retryResponseDelivered) { + return "consumed"; + } + + const sessionState = await queryAuthoritativeSessionState( + sessionId, + state.promptOptions.directory, + ); + return sessionState === "idle" ? "finalize" : "consumed"; } /** @@ -233,29 +239,6 @@ export function recordAttemptEvidence( state.workEvidence = mergeTaskAttemptEvidence(state.workEvidence, evidence); } -/** - * Guards every idle event that belongs to a retry lifecycle. The guard only - * activates once the retry has actually been dispatched - a registered but - * never-replayed attempt must not swallow the normal idle finalization. While - * the guard is up the idle is consumed (never finalized); once a retry response - * was delivered the consume also extends the settle window, so the retry is - * finalized by its settle timer rather than by the first idle that happens to - * arrive. A stale or duplicate idle from the original attempt therefore cannot - * finish the retry early, emit a footer, commit /lastfile, export Markdown, - * mark the foreground idle, or dispatch queued prompts. - */ -export function consumePromptRetryIdle(sessionId: string): boolean { - const state = promptRetryStates.get(sessionId); - if (!state || !state.retryDispatched) { - return false; - } - - if (state.retryResponseDelivered) { - armRetrySettle(state, sessionId); - } - return true; -} - /** * Decides what an empty completion means for the current prompt attempt: * retried (zero-work original, retry dispatched), failed (the retry itself came @@ -314,7 +297,6 @@ export function retryPromptOnce(sessionId: string): boolean { } state.retryDispatched = true; - clearRetrySettleTimer(state); state.retryResponseDelivered = false; state.workEvidence = createEmptyTaskAttemptEvidence(); foregroundSessionState.markBusy(sessionId, state.promptOptions.directory); diff --git a/src/bot/services/empty-completion-policy.ts b/src/bot/services/empty-completion-policy.ts index 10e8494b4..618ab99db 100644 --- a/src/bot/services/empty-completion-policy.ts +++ b/src/bot/services/empty-completion-policy.ts @@ -99,36 +99,36 @@ export function isSafeZeroWorkEmptyCompletion(evidence: TaskAttemptEvidence): bo } /** - * Finish reasons that prove a completed assistant message did NOT end with a - * terminal, successfully delivered answer. Anything else (including an unknown - * or missing reason) is treated as potentially terminal: the primary signals - * for a truncated or interrupted response are the upstream message error and - * tool activity, so the finish reason only ever disqualifies a known - * non-terminal value and never rejects a normal answer. + * Finish reasons OpenCode actually emits on a completed assistant message, + * from the @opencode-ai/llm `FinishReason` literal schema + * (`packages/llm/src/schema/ids.ts`): + * + * ["stop", "length", "tool-calls", "content-filter", "error", "unknown"] + * + * Only `"stop"` means the model ended its turn with a successfully delivered + * answer. Every other value means the run stopped early, called a tool, was + * filtered, errored, or has an unknown reason. session/llm/ai-sdk.ts also maps + * any unrecognized AI SDK finish reason to `"unknown"`, so no other value can + * appear on a real message. */ -const KNOWN_NON_TERMINAL_FINISH_REASONS = new Set([ - "max_tokens", - "length", - "error", - "content_filter", - "tool_use", - "function_call", - "aborted", - "incomplete", -]); - -export function isKnownNonTerminalFinishReason(finishReason: string | undefined): boolean { - const normalized = finishReason?.trim().toLowerCase() ?? ""; - return normalized.length > 0 && KNOWN_NON_TERMINAL_FINISH_REASONS.has(normalized); +const TERMINAL_FINISH_REASONS = new Set(["stop"]); + +export function isKnownTerminalFinishReason(finishReason: string | undefined): boolean { + if (finishReason === undefined) { + return false; + } + + const normalized = finishReason.trim().toLowerCase(); + return normalized.length > 0 && TERMINAL_FINISH_REASONS.has(normalized); } /** * A completed non-empty assistant message is a terminal final-response - * candidate only when it survived with no upstream error, performed no tool - * activity (a message that called a tool is never the closing answer of a - * run), and did not end on a known non-terminal finish reason. The check is - * deliberately conservative: a run whose last message fails any of these - * signals is never reported as a completed task. + * candidate only when it has positive evidence of success: no upstream error, + * no tool activity (a message that called a tool is never the closing answer + * of a run), and an explicitly known successful finish reason. Missing, + * unknown, or arbitrary finish reasons fail closed - a non-empty message by + * itself is never treated as a completed task. */ export function isTerminalAssistantResponse(completion: { hasError?: boolean; @@ -143,9 +143,5 @@ export function isTerminalAssistantResponse(completion: { return false; } - if (isKnownNonTerminalFinishReason(completion.finishReason)) { - return false; - } - - return true; + return isKnownTerminalFinishReason(completion.finishReason); } diff --git a/src/bot/services/event-subscription-service.ts b/src/bot/services/event-subscription-service.ts index 4cef8bc58..abc2a9836 100644 --- a/src/bot/services/event-subscription-service.ts +++ b/src/bot/services/event-subscription-service.ts @@ -37,14 +37,12 @@ import { clearAllPromptRetry, clearPromptResponseMode, clearPromptRetry, - consumePromptRetryIdle, + decidePromptRetryIdle, getPromptRetryChatId, handleEmptyCompletion, hasPromptRetryAttempted, markPromptRetryResponseDelivered, recordAttemptEvidence, - resetPromptRetrySettle, - setOnRetrySettle, } from "../handlers/prompt.js"; import { reconcileBusyState, @@ -253,9 +251,6 @@ class EventSubscriptionService implements BotEventSubscriptionService { hasActiveStream: (sessionId) => this.hasActiveAssistantResponseStream(sessionId), }); setPromptResponseModeClearerForReconciliation(clearPromptResponseMode); - setOnRetrySettle((sessionId) => { - void this.handleRetrySettle(sessionId); - }); this.compactProgressStreamer = new CompactProgressStreamer({ throttleMs: RESPONSE_STREAM_THROTTLE_MS, @@ -569,7 +564,6 @@ class EventSubscriptionService implements BotEventSubscriptionService { // the candidate always lands before the start that supersedes it. void this.enqueueSessionCompletionTask(sessionId, () => { this.invalidateIntermediateFinalCandidate(sessionId, messageId); - resetPromptRetrySettle(sessionId); return Promise.resolve(); }); }); @@ -1205,13 +1199,16 @@ class EventSubscriptionService implements BotEventSubscriptionService { this.clearToolElapsedState(sessionId, "session_idle"); await this.sessionCompletionTasks.get(sessionId)?.catch(() => undefined); - if (consumePromptRetryIdle(sessionId)) { + const retryDecision = await decidePromptRetryIdle(sessionId); + if (retryDecision === "consumed") { logger.debug( `[Bot] Ignoring idle event while a retry lifecycle is active: session=${sessionId}`, ); return; } + // "none" (no retry lifecycle) and "finalize" (genuine retry idle, + // confirmed against the authoritative session status) both finalize. await this.finalizeIdleSession(sessionId); }); @@ -1678,23 +1675,11 @@ class EventSubscriptionService implements BotEventSubscriptionService { } /** - * Finalizes a retried run once its settle window has passed. The retry's own - * idle events are all consumed by the guard, so this is the single point - * where the retry is committed and its footer is emitted - a stale idle from - * the original attempt can never reach this path. - */ - private async handleRetrySettle(sessionId: string): Promise { - logger.debug(`[Bot] Finalizing retried completion after settle window: session=${sessionId}`); - this.clearToolElapsedState(sessionId, "retry_settle"); - await this.sessionCompletionTasks.get(sessionId)?.catch(() => undefined); - await this.finalizeIdleSession(sessionId); - } - - /** - * Shared idle finalization for a normal run and for a retried run whose - * settle timer fired. The success footer is emitted only when a terminal - * final response was actually committed, so a run that ended on intermediate - * or truncated output is never announced as a completed task. + * Shared idle finalization for a normal run and for a retried run whose idle + * was confirmed against the authoritative OpenCode session status. The + * success footer is emitted only when a terminal final response was actually + * committed, so a run that ended on intermediate or truncated output is never + * announced as a completed task. */ private async finalizeIdleSession(sessionId: string): Promise { await markAttachedSessionIdle(sessionId); diff --git a/tests/bot/services/empty-completion-policy.test.ts b/tests/bot/services/empty-completion-policy.test.ts index d0ad74271..b5d6dd03e 100644 --- a/tests/bot/services/empty-completion-policy.test.ts +++ b/tests/bot/services/empty-completion-policy.test.ts @@ -115,43 +115,61 @@ describe("empty completion policy", () => { }); describe("terminal response detection", () => { - it("treats a clean non-empty completion as terminal", () => { - expect(isTerminalAssistantResponse({ hasError: false, hasToolActivity: false })).toBe(true); - expect(isTerminalAssistantResponse({})).toBe(true); + it("treats a clean non-empty completion with the successful finish reason as terminal", () => { + expect(isTerminalAssistantResponse({ hasError: false, hasToolActivity: false, finishReason: "stop" })).toBe( + true, + ); + expect(isTerminalAssistantResponse({ finishReason: "stop" })).toBe(true); + }); + + it("rejects an errored or aborted completion even with a successful finish", () => { + expect(isTerminalAssistantResponse({ hasError: true, finishReason: "stop" })).toBe(false); + expect(isTerminalAssistantResponse({ hasError: true, hasToolActivity: false, finishReason: "stop" })).toBe( + false, + ); + }); + + it("rejects a completion whose message called a tool even with a successful finish", () => { + expect(isTerminalAssistantResponse({ hasToolActivity: true, finishReason: "stop" })).toBe(false); }); - it("rejects an errored or aborted completion", () => { - expect(isTerminalAssistantResponse({ hasError: true })).toBe(false); - expect(isTerminalAssistantResponse({ hasError: true, hasToolActivity: false })).toBe(false); + it("fails closed when the finish reason is missing", () => { + expect(isTerminalAssistantResponse({ finishReason: undefined })).toBe(false); + expect(isTerminalAssistantResponse({})).toBe(false); + expect(isTerminalAssistantResponse({ hasError: false, hasToolActivity: false })).toBe(false); }); - it("rejects a completion whose message called a tool", () => { - expect(isTerminalAssistantResponse({ hasToolActivity: true })).toBe(false); + it("rejects unknown and arbitrary unexpected finish reasons", () => { + expect(isTerminalAssistantResponse({ finishReason: "unknown" })).toBe(false); + expect(isTerminalAssistantResponse({ finishReason: "unusual-reason" })).toBe(false); + expect(isTerminalAssistantResponse({ finishReason: "end_turn" })).toBe(false); + expect(isTerminalAssistantResponse({ finishReason: "" })).toBe(false); }); - it("rejects known non-terminal finish reasons but accepts unknown ones", () => { - expect(isTerminalAssistantResponse({ finishReason: "max_tokens" })).toBe(false); - expect(isTerminalAssistantResponse({ finishReason: "length" })).toBe(false); - expect(isTerminalAssistantResponse({ finishReason: "content_filter" })).toBe(false); - expect(isTerminalAssistantResponse({ finishReason: "tool_use" })).toBe(false); - expect(isTerminalAssistantResponse({ finishReason: "end_turn" })).toBe(true); - expect(isTerminalAssistantResponse({ finishReason: undefined })).toBe(true); - expect(isTerminalAssistantResponse({ finishReason: "unusual-reason" })).toBe(true); + it("rejects every known non-terminal OpenCode finish reason", () => { + // The authoritative @opencode-ai/llm FinishReason literal schema. + for (const reason of ["length", "tool-calls", "content-filter", "error", "unknown"]) { + expect(isTerminalAssistantResponse({ finishReason: reason })).toBe(false); + } + // Reviewer-flagged truncation/interruption names must also fail closed. + for (const reason of ["max_tokens", "aborted", "tool_use", "function_call", "incomplete"]) { + expect(isTerminalAssistantResponse({ finishReason: reason })).toBe(false); + } }); - it("fails closed when error and tool activity are combined with otherwise clean signals", () => { + it("fails closed when error and tool activity are combined with a successful finish", () => { expect( isTerminalAssistantResponse({ hasError: true, hasToolActivity: false, - finishReason: "end_turn", + finishReason: "stop", }), ).toBe(false); expect( isTerminalAssistantResponse({ hasError: false, hasToolActivity: true, - finishReason: "end_turn", + finishReason: "stop", }), ).toBe(false); }); diff --git a/tests/bot/services/event-subscription-service.lifecycle.test.ts b/tests/bot/services/event-subscription-service.lifecycle.test.ts index 56f4721ba..b3dc24216 100644 --- a/tests/bot/services/event-subscription-service.lifecycle.test.ts +++ b/tests/bot/services/event-subscription-service.lifecycle.test.ts @@ -98,6 +98,8 @@ function emitAssistantCompleted(aggregator: Aggregator): void { agent: "test-agent", providerID: "test-provider", modelID: "test-model", + // The authoritative OpenCode message finish for a successful run. + finish: "stop", time: { created: Date.now() - 1000, completed: Date.now() }, }, }, diff --git a/tests/bot/services/event-subscription-service.test.ts b/tests/bot/services/event-subscription-service.test.ts index db06257c8..f560a4e79 100644 --- a/tests/bot/services/event-subscription-service.test.ts +++ b/tests/bot/services/event-subscription-service.test.ts @@ -13,6 +13,7 @@ const mocked = vi.hoisted(() => ({ safeBackgroundTask: vi.fn(), dispatchNextQueuedPrompt: vi.fn(), promptAsyncMock: vi.fn(), + sessionStatusMock: vi.fn(), })); vi.mock("../../../src/opencode/events.js", () => ({ @@ -23,7 +24,7 @@ vi.mock("../../../src/opencode/events.js", () => ({ vi.mock("../../../src/opencode/client.js", () => ({ opencodeClient: { session: { - status: vi.fn().mockResolvedValue({ data: {}, error: null }), + status: mocked.sessionStatusMock, promptAsync: mocked.promptAsyncMock, }, }, @@ -143,6 +144,8 @@ function emitAssistantCompleted(summaryAggregator: { processEvent(event: Event): agent: "test-agent", providerID: "test-provider", modelID: "test-model", + // The authoritative OpenCode message finish for a successful run. + finish: "stop", time: { created: Date.now() - 1000, completed: Date.now() }, }, }, @@ -355,6 +358,8 @@ describe("bot/services/event-subscription-service", () => { mocked.dispatchNextQueuedPrompt.mockReset(); mocked.promptAsyncMock.mockReset(); mocked.promptAsyncMock.mockResolvedValue({ data: {}, error: null }); + mocked.sessionStatusMock.mockReset(); + mocked.sessionStatusMock.mockResolvedValue({ data: {}, error: null }); const exportService = await import("../../../src/bot/services/assistant-response-export-service.js"); exportService.__resetAssistantResponseExportsForTests(); @@ -857,6 +862,9 @@ describe("bot/services/event-subscription-service", () => { agent: "test-agent", providerID: "test-provider", modelID: "test-model", + // The authoritative OpenCode message finish for a successful run; + // overrides may replace it with a non-terminal reason. + finish: "stop", time: { created: Date.now() - 1000, completed: Date.now() }, ...overrides, }, @@ -1413,7 +1421,12 @@ describe("bot/services/event-subscription-service", () => { emitAssistantCompleted(summaryAggregator, "message-2"); // Stale duplicate idle of the ORIGINAL attempt arrives AFTER the retry - // produced its response. It must be fully inert. + // produced its response, while the authoritative session status says the + // retry is still busy. It must be fully inert. + mocked.sessionStatusMock.mockResolvedValueOnce({ + data: { "session-1": { type: "busy" } }, + error: null, + }); emitSessionIdle(summaryAggregator); await flushRealDispatch(); expect(countFooters(api)).toBe(0); @@ -1422,7 +1435,9 @@ describe("bot/services/event-subscription-service", () => { expect(foregroundSessionState.isBusy()).toBe(true); expect(mocked.dispatchNextQueuedPrompt).not.toHaveBeenCalled(); - // The real retry idle then finalizes exactly once. + // The real retry idle, with the authoritative status now genuinely idle, + // finalizes exactly once. + mocked.sessionStatusMock.mockResolvedValueOnce({ data: {}, error: null }); emitSessionIdle(summaryAggregator); await vi.waitFor( () => { @@ -1467,6 +1482,171 @@ describe("bot/services/event-subscription-service", () => { expect(footerCalls).toHaveLength(1); expect(footerCalls[0][0]).toBe(42); }); + + it("fails closed for a non-empty response with a missing or unknown finish reason", async () => { + const { api, summaryAggregator } = await setupService(false, { startAssistantRun: true }); + const exportService = + await import("../../../src/bot/services/assistant-response-export-service.js"); + const { assistantRunState } = await import( + "../../../src/app/managers/assistant-run-state-manager.js" + ); + + emitAssistantText(summaryAggregator, "Good answer", "message-0"); + emitAssistantCompleted(summaryAggregator, "message-0"); + emitSessionIdle(summaryAggregator); + await vi.waitFor(() => { + expect(exportService.getRememberedAssistantResponse(42, "session-1")).toBe("Good answer"); + }); + + for (const finish of [undefined, "unknown"]) { + assistantRunState.startRun("session-1", { + startedAt: Date.now(), + configuredAgent: "test-agent", + configuredProviderID: "test-provider", + configuredModelID: "test-model", + }); + const footersBefore = countFooters(api); + const messageId = `message-${finish === undefined ? "missing" : "unknown"}`; + + // Partial/truncated-looking response: non-empty, no explicit error, + // finish missing or unknown. Must never be reported as complete. + emitAssistantText(summaryAggregator, "Partial answer that looks truncated", messageId); + emitAssistantCompleted(summaryAggregator, messageId, { finish }); + emitSessionIdle(summaryAggregator); + + await flushRealDispatch(); + await vi.waitFor(() => { + expect(exportService.getRememberedAssistantResponse(42, "session-1")).toBe( + "Good answer", + ); + }); + expect(countFooters(api)).toBe(footersBefore); + expect(api.sendDocument).not.toHaveBeenCalled(); + expect(exportService.getRememberedAssistantResponse(42, "session-1")).toBe("Good answer"); + } + }); + + it("does not finalize the retry on a stale idle even after a long delay while the session is busy", async () => { + const { api, summaryAggregator } = await setupService(false, { startAssistantRun: true }); + const exportService = + await import("../../../src/bot/services/assistant-response-export-service.js"); + const { foregroundSessionState } = await import( + "../../../src/app/managers/foreground-session-state-manager.js" + ); + await registerPromptAttempt(api); + + emitZeroWorkEmptyCompletion(summaryAggregator, "message-1"); + emitSessionIdle(summaryAggregator); + + await vi.waitFor(() => { + expect(mocked.safeBackgroundTask).toHaveBeenCalledWith( + expect.objectContaining({ taskName: "session.promptAsync.retry" }), + ); + }); + + const retryOptions = getRetryTaskOptions(); + await retryOptions.task(); + retryOptions.onSuccess?.({ error: null }); + + const longText = `# Heading\n\n${"x".repeat(6000)}`; + emitAssistantText(summaryAggregator, longText, "message-2"); + emitAssistantCompleted(summaryAggregator, "message-2"); + + // Stale original idle while the authoritative status says the retry is + // still busy. It must be fully inert. + mocked.sessionStatusMock.mockResolvedValueOnce({ + data: { "session-1": { type: "busy" } }, + error: null, + }); + emitSessionIdle(summaryAggregator); + await flushRealDispatch(); + expect(countFooters(api)).toBe(0); + expect(exportService.getRememberedAssistantResponse(42, "session-1")).toBeNull(); + expect(api.sendDocument).not.toHaveBeenCalled(); + expect(foregroundSessionState.isBusy()).toBe(true); + expect(mocked.dispatchNextQueuedPrompt).not.toHaveBeenCalled(); + + // Wait LONGER than the old 300ms timer window: nothing may finalize. + await new Promise((resolve) => setTimeout(resolve, 600)); + await flushRealDispatch(); + expect(countFooters(api)).toBe(0); + expect(exportService.getRememberedAssistantResponse(42, "session-1")).toBeNull(); + expect(api.sendDocument).not.toHaveBeenCalled(); + expect(foregroundSessionState.isBusy()).toBe(true); + expect(mocked.dispatchNextQueuedPrompt).not.toHaveBeenCalled(); + + // The genuine retry idle, with authoritative idle status, finalizes + // exactly once and exports the long response. + mocked.sessionStatusMock.mockResolvedValueOnce({ data: {}, error: null }); + emitSessionIdle(summaryAggregator); + await vi.waitFor( + () => { + expect(api.sendDocument).toHaveBeenCalledTimes(1); + }, + { timeout: 5000 }, + ); + expect(exportService.getRememberedAssistantResponse(42, "session-1")).toBe(longText); + expect(foregroundSessionState.isBusy()).toBe(false); + expect(countFooters(api)).toBe(1); + expect(mocked.dispatchNextQueuedPrompt).toHaveBeenCalledTimes(1); + expect(mocked.safeBackgroundTask).toHaveBeenCalledTimes(1); + }); + + it("fails closed when the authoritative session status lookup fails during retry finalization", async () => { + const { api, summaryAggregator } = await setupService(false, { startAssistantRun: true }); + const exportService = + await import("../../../src/bot/services/assistant-response-export-service.js"); + const { foregroundSessionState } = await import( + "../../../src/app/managers/foreground-session-state-manager.js" + ); + await registerPromptAttempt(api); + + emitZeroWorkEmptyCompletion(summaryAggregator, "message-1"); + emitSessionIdle(summaryAggregator); + + await vi.waitFor(() => { + expect(mocked.safeBackgroundTask).toHaveBeenCalledWith( + expect.objectContaining({ taskName: "session.promptAsync.retry" }), + ); + }); + + const retryOptions = getRetryTaskOptions(); + await retryOptions.task(); + retryOptions.onSuccess?.({ error: null }); + + emitAssistantText(summaryAggregator, "Recovered answer", "message-2"); + emitAssistantCompleted(summaryAggregator, "message-2"); + + for (const failingStatus of [ + { data: null, error: new Error("status lookup failed") }, + { data: { "session-1": { type: "unexpected" } }, error: null }, + ]) { + mocked.sessionStatusMock.mockResolvedValueOnce(failingStatus); + emitSessionIdle(summaryAggregator); + await flushRealDispatch(); + expect(countFooters(api)).toBe(0); + expect(exportService.getRememberedAssistantResponse(42, "session-1")).toBeNull(); + expect(api.sendDocument).not.toHaveBeenCalled(); + expect(foregroundSessionState.isBusy()).toBe(true); + expect(mocked.dispatchNextQueuedPrompt).not.toHaveBeenCalled(); + } + + // A later genuine idle with authoritative idle status still finalizes + // exactly once. + mocked.sessionStatusMock.mockResolvedValueOnce({ data: {}, error: null }); + emitSessionIdle(summaryAggregator); + await vi.waitFor( + () => { + expect(countFooters(api)).toBe(1); + }, + { timeout: 5000 }, + ); + expect(exportService.getRememberedAssistantResponse(42, "session-1")).toBe( + "Recovered answer", + ); + expect(foregroundSessionState.isBusy()).toBe(false); + expect(countFooters(api)).toBe(1); + }); }); }); });