From 3dd3003f49ce71f38c0ecf84dbdbc95023498a69 Mon Sep 17 00:00:00 2001 From: samson1357924 <98934496+samson1357924@users.noreply.github.com> Date: Mon, 24 Aug 2026 05:37:09 +0800 Subject: [PATCH] fix(status): retry status runtime loads and contain render errors (Fixes #94626) (#94806) --- src/auto-reply/reply/commands-status.test.ts | 111 +++++++++++++++++++ src/auto-reply/reply/commands-status.ts | 31 ++++-- src/status/status-text.test.ts | 57 +++++++++- src/status/status-text.ts | 26 ++--- 4 files changed, 199 insertions(+), 26 deletions(-) diff --git a/src/auto-reply/reply/commands-status.test.ts b/src/auto-reply/reply/commands-status.test.ts index 3539be34d143..d42f1e433e7e 100644 --- a/src/auto-reply/reply/commands-status.test.ts +++ b/src/auto-reply/reply/commands-status.test.ts @@ -2329,6 +2329,117 @@ describe("buildStatusReply subagent summary", () => { expect(normalizeTestText(text)).toContain("Runtime: OpenAI Codex"); }); }); + +describe("buildStatusReply error handling", () => { + afterEach(() => { + vi.doUnmock("../../logger.js"); + vi.doUnmock("../../status/status-text.js"); + vi.resetModules(); + vi.restoreAllMocks(); + }); + + async function runStatusReply(fn: typeof buildStatusReply) { + const commandParams = buildCommandTestParams("/status", baseCfg); + return await fn({ + cfg: baseCfg, + command: commandParams.command, + sessionEntry: commandParams.sessionEntry, + sessionKey: commandParams.sessionKey, + parentSessionKey: commandParams.sessionKey, + sessionScope: commandParams.sessionScope, + storePath: commandParams.storePath, + provider: "anthropic", + model: "claude-opus-4-6", + contextTokens: 0, + resolvedThinkLevel: commandParams.resolvedThinkLevel, + resolvedFastMode: false, + resolvedVerboseLevel: commandParams.resolvedVerboseLevel, + resolvedReasoningLevel: commandParams.resolvedReasoningLevel, + resolvedElevatedLevel: commandParams.resolvedElevatedLevel, + resolveDefaultThinkingLevel: commandParams.resolveDefaultThinkingLevel, + isGroup: commandParams.isGroup, + defaultGroupActivation: commandParams.defaultGroupActivation, + modelAuthOverride: "api-key", + activeModelAuthOverride: "api-key", + }); + } + + it("delivers a fixed generic reply and logs details when status rendering throws", async () => { + // commands-status re-exports buildStatusText, so the mock must keep that + // binding while prod calls buildStatusReplyParts. logError stays mocked so + // containment diagnostics never leak into test stderr. + vi.doMock("../../logger.js", async (importOriginal) => ({ + ...(await importOriginal()), + logError: vi.fn(), + })); + vi.doMock("../../status/status-text.js", () => ({ + buildStatusReplyParts: vi.fn(() => Promise.reject(new Error("Unexpected rendering error"))), + buildStatusText: vi.fn(() => Promise.reject(new Error("Unexpected rendering error"))), + })); + + vi.resetModules(); + const { buildStatusReply: freshBuildStatusReply } = await import("./commands-status.js"); + const { logError } = await import("../../logger.js"); + const reply = await runStatusReply(freshBuildStatusReply); + + // Exact object equality also pins that no stale presentation or internal + // error text reaches the channel; diagnostics belong to the log sink only. + expect(reply).toEqual({ text: "⚠️ Status: error rendering response" }); + expect(logError).toHaveBeenCalledWith(expect.stringContaining("Unexpected rendering error")); + }); + + it("keeps the structured rich payload on the success path", async () => { + const presentation = { + title: "Status", + tone: "info" as const, + blocks: [{ type: "text" as const, text: "plain status" }, { type: "divider" as const }], + }; + vi.doMock("../../status/status-text.js", () => ({ + buildStatusReplyParts: vi.fn(() => Promise.resolve({ text: "plain status", presentation })), + buildStatusText: vi.fn(() => Promise.resolve("plain status")), + })); + + vi.resetModules(); + const { buildStatusReply: freshBuildStatusReply } = await import("./commands-status.js"); + const reply = await runStatusReply(freshBuildStatusReply); + + expect(reply).toMatchObject({ + text: "plain status", + presentation, + presentationTextMode: "fallback", + }); + }); + + it("returns a generic reply and logs details when plugin health collection fails", async () => { + vi.doMock("../../logger.js", async (importOriginal) => ({ + ...(await importOriginal()), + logError: vi.fn(), + })); + + vi.resetModules(); + const { buildStatusPluginsReply: freshBuildStatusPluginsReply } = + await import("./commands-status.js"); + const { logError } = await import("../../logger.js"); + pluginHealthRuntimeMock.collectInstalledPluginHealthSnapshot.mockRejectedValueOnce( + new Error("Cannot find module 'internal/path'"), + ); + + const commandParams = buildCommandTestParams("/status plugins", { + ...baseCfg, + commands: { text: true, plugins: true }, + }); + const reply = await freshBuildStatusPluginsReply({ + cfg: commandParams.cfg, + command: commandParams.command, + workspaceDir: commandParams.workspaceDir, + }); + + expect(reply?.text).toBe("⚠️ Plugins: health unavailable"); + expect(logError).toHaveBeenCalledWith( + expect.stringContaining("Cannot find module 'internal/path'"), + ); + }); +}); /* oxlint-disable max-lines -- TODO: split this grandfathered oversized file. */ async function buildKiraStatusReply(cfg: OpenClawConfig) { diff --git a/src/auto-reply/reply/commands-status.ts b/src/auto-reply/reply/commands-status.ts index e53db820c3cf..c8efde4af6bc 100644 --- a/src/auto-reply/reply/commands-status.ts +++ b/src/auto-reply/reply/commands-status.ts @@ -1,5 +1,7 @@ /** Builds /status replies using the command's authorized channel context. */ import { logVerbose } from "../../globals.js"; +import { formatErrorMessage } from "../../infra/errors.js"; +import { logError } from "../../logger.js"; import { formatDetailedPluginHealth } from "../../status/status-plugin-health.js"; import { buildStatusReplyParts } from "../../status/status-text.js"; import type { BuildStatusTextParams } from "../../status/status-text.types.js"; @@ -22,14 +24,21 @@ export async function buildStatusReply( return undefined; } - const { text, presentation } = await buildStatusReplyParts({ - ...params, - statusChannel: command.channel, - statusAccountId: command.accountId, - }); - // The text body is the authored plain rendering of the same facts; channels - // with native table support render the presentation instead. - return { text, presentation, presentationTextMode: "fallback" }; + try { + const { text, presentation } = await buildStatusReplyParts({ + ...params, + statusChannel: command.channel, + statusAccountId: command.accountId, + }); + // The text body is the authored plain rendering of the same facts; channels + // with native table support render the presentation instead. + return { text, presentation, presentationTextMode: "fallback" }; + } catch (error) { + // Diagnostics stay in logs only; the channel reply is a fixed generic + // message so internal module paths or runtime details never reach users. + logError(`/status render failed: ${formatErrorMessage(error)}`); + return { text: "⚠️ Status: error rendering response" }; + } } export async function buildStatusPluginsReply( @@ -59,8 +68,8 @@ export async function buildStatusPluginsReply( }); return { text: formatDetailedPluginHealth(snapshot) }; } catch (error) { - return { - text: `⚠️ Plugins: health unavailable (${error instanceof Error ? error.message : String(error)})`, - }; + // Match the /status fallback: fixed generic reply, diagnostics in logs only. + logError(`/status plugins render failed: ${formatErrorMessage(error)}`); + return { text: "⚠️ Plugins: health unavailable" }; } } diff --git a/src/status/status-text.test.ts b/src/status/status-text.test.ts index f4309b5edb49..7a541c7695d8 100644 --- a/src/status/status-text.test.ts +++ b/src/status/status-text.test.ts @@ -1,4 +1,4 @@ -import { beforeEach, describe, expect, it, vi } from "vitest"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { formatSqliteSessionFileMarker } from "../config/sessions/legacy-sqlite-marker.js"; import { normalizeSessionDeliveryState } from "../utils/delivery-context.shared.js"; import { appendSessionCostLine } from "./status-runtime-lines.js"; @@ -353,3 +353,58 @@ describe("buildStatusText thinking facts", () => { expect(text).not.toMatch(/think\s+off\b/); }); }); + +describe("buildStatusText lazy loader retry", () => { + afterEach(() => { + vi.doUnmock("./status-plugin-health.runtime.js"); + vi.resetModules(); + vi.restoreAllMocks(); + }); + + function retryStatusParams(sessionId: string): Parameters[0] { + return { + cfg: {}, + sessionEntry: { sessionId, updatedAt: 0 }, + sessionKey: "agent:main:main", + statusChannel: "mobilechat", + provider: "openai", + model: "gpt-5.4-mini", + resolvedHarness: "openclaw", + resolvedVerboseLevel: "off", + resolvedReasoningLevel: "off", + resolveDefaultThinkingLevel: async () => undefined, + isGroup: false, + defaultGroupActivation: () => "mention", + taskLineOverride: "", + skipDefaultTaskLookup: true, + primaryModelLabelOverride: "openai/gpt-5.4-mini", + modelAuthOverride: "api-key", + activeModelAuthOverride: "api-key", + includeTranscriptUsage: false, + }; + } + + it("falls back on import failure and retries in the same module instance", async () => { + vi.doMock("./status-plugin-health.runtime.js", async () => { + throw new Error("Module load failure"); + }); + vi.resetModules(); + + const { buildStatusText: firstLoadBuildStatusText } = await import("./status-text.js"); + const failed = await firstLoadBuildStatusText(retryStatusParams("retry-failure")); + expect(failed).toContain("Plugins: health unavailable"); + + vi.doMock("./status-plugin-health.runtime.js", () => ({ + collectRuntimePluginHealthSnapshot: () => ({ + plugins: [], + diagnostics: [], + contextEngineQuarantines: [], + runtimeToolQuarantines: [], + channelPluginFailures: [], + }), + })); + + const recovered = await firstLoadBuildStatusText(retryStatusParams("retry-recovery")); + expect(recovered).not.toContain("Plugins: health unavailable"); + }); +}); diff --git a/src/status/status-text.ts b/src/status/status-text.ts index 45feb7cb8817..550bbaa0539c 100644 --- a/src/status/status-text.ts +++ b/src/status/status-text.ts @@ -41,7 +41,7 @@ import { import { resolveActiveProviderThinkingProfile } from "../plugins/provider-thinking-active.js"; import { normalizeAccountId } from "../routing/account-id.js"; import { resolveNormalizedAccountEntry } from "../routing/account-lookup.js"; -import { createLazyPromise, createLazyRuntimeModule } from "../shared/lazy-runtime.js"; +import { createLazyPromise } from "../shared/lazy-runtime.js"; import { listTasksForAgentIdForStatus, listTasksForSessionKeyForStatus, @@ -108,22 +108,20 @@ function resolveStatusChannelFeatureLine(params: { : "Telegram rich messages: off · set channels.telegram.richMessages=true for tables/details/rich media"; } -const loadStatusMessageRuntime = createLazyPromise( - () => - import("./status-message.runtime.js").then((module) => module.loadStatusMessageRuntimeModule()), - { cacheRejections: true }, -); -const loadAgentThinkingRuntime = createLazyRuntimeModule( - () => import("../agents/thinking-runtime.js"), -); -const loadThinkingLevelRuntime = createLazyRuntimeModule(() => import("../auto-reply/thinking.js")); -const loadStatusSubagentsRuntime = createLazyRuntimeModule( - () => import("./status-subagents.runtime.js"), +// Status loaders keep the lazy-promise eviction default: a transient module-load +// failure on one /status request self-heals on the next instead of poisoning +// every reply. Deliberately not createLazyRuntimeModule, whose sticky rejection +// cache would pin the failure for the process lifetime. +const loadStatusMessageRuntime = createLazyPromise(() => + import("./status-message.runtime.js").then((module) => module.loadStatusMessageRuntimeModule()), ); +const loadAgentThinkingRuntime = createLazyPromise(() => import("../agents/thinking-runtime.js")); +const loadThinkingLevelRuntime = createLazyPromise(() => import("../auto-reply/thinking.js")); +const loadStatusSubagentsRuntime = createLazyPromise(() => import("./status-subagents.runtime.js")); -const loadStatusQueueRuntime = createLazyRuntimeModule(() => import("./status-queue.runtime.js")); +const loadStatusQueueRuntime = createLazyPromise(() => import("./status-queue.runtime.js")); -const loadStatusPluginHealthRuntime = createLazyRuntimeModule( +const loadStatusPluginHealthRuntime = createLazyPromise( () => import("./status-plugin-health.runtime.js"), );