diff --git a/docs/providers/openai.md b/docs/providers/openai.md index 22d4f5ca373e..b72eddc733a3 100644 --- a/docs/providers/openai.md +++ b/docs/providers/openai.md @@ -596,6 +596,7 @@ generation through the same `openai/gpt-image-2` model ref. | Transport | OpenAI Images API | Codex Responses backend | | Max images per request | 4 | 4 | | Edit mode | Enabled (up to 5 reference images) | Enabled (up to 5 reference images) | +| Moderation | `low` or `auto`; generate and edit | `low` or `auto`; generate and edit | | Size overrides | Supported, including 2K/4K sizes | Supported, including 2K/4K sizes | | Aspect ratio / resolution | Not forwarded to OpenAI Images API | Mapped to a supported size when safe | @@ -643,8 +644,10 @@ Use the same `--output-format` and `--background` flags with `openclaw infer image edit` when starting from an input file. `--openai-background` remains available as an OpenAI-specific alias. Use `--quality low|medium|high|auto` to control OpenAI Images quality and cost. -Use `--openai-moderation low|auto` to pass OpenAI's moderation hint from either -`image generate` or `image edit`. +Use `--openai-moderation low|auto` with both `image generate` and `image edit` +to pass OpenAI's moderation hint. The direct OpenAI Images API and the +ChatGPT/Codex OAuth Responses backend both support moderation for text-to-image +generation and reference-image edits. For ChatGPT/Codex OAuth installs, keep the same `openai/gpt-image-2` ref. When an `openai` OAuth profile is configured, OpenClaw resolves that stored OAuth diff --git a/docs/tools/image-generation.md b/docs/tools/image-generation.md index 8f489aae613e..8a320f030135 100644 --- a/docs/tools/image-generation.md +++ b/docs/tools/image-generation.md @@ -94,6 +94,11 @@ provider does not declare support. Bundled transparent-background support is OpenAI-specific; other providers may still preserve PNG alpha if their backend emits it. +OpenAI supports `low` and `auto` moderation for both text-to-image generation +and reference-image edits through the direct Images API or the Codex Responses +backend. For CLI requests, pass `--openai-moderation low|auto` to either +`openclaw infer image generate` or `openclaw infer image edit`. + ## Supported providers | Provider | Default model | Edit support | Auth | @@ -533,11 +538,14 @@ openclaw infer image generate \ -The same `--output-format`, `--background`, `--quality`, and -`--openai-moderation` flags are available on `openclaw infer image edit`; -`--openai-background` remains as an OpenAI-specific alias. Bundled providers -other than OpenAI do not declare explicit background control today, so -`background: "transparent"` is reported as ignored for them. +The same `--output-format`, `--background`, and `--quality` flags are available +on `openclaw infer image edit`; `--openai-background` remains as an +OpenAI-specific alias. Use `--openai-moderation low|auto` with both OpenAI image +generation and reference-image edits. The direct OpenAI Images API and the +ChatGPT/Codex OAuth Responses backend both support the moderation hint. +Bundled providers other than OpenAI do not declare +explicit background control today, so `background: "transparent"` is reported +as ignored for them. ## Related diff --git a/extensions/feishu/src/thread-bindings.test.ts b/extensions/feishu/src/thread-bindings.test.ts index 11ad687eecd7..421b4f57dc8a 100644 --- a/extensions/feishu/src/thread-bindings.test.ts +++ b/extensions/feishu/src/thread-bindings.test.ts @@ -85,6 +85,137 @@ describe("Feishu thread bindings", () => { }); }); + it("expires idle bindings from manager and service lookups at the deadline", async () => { + const startedAt = 1_700_000_000_000; + const now = vi.spyOn(Date, "now").mockReturnValue(startedAt); + const manager = createFeishuThreadBindingManager({ cfg: baseCfg, accountId: "default" }); + const service = getSessionBindingService(); + const targetSessionKey = "agent:codex:acp:binding:feishu:default:expired"; + const conversation = { + channel: "feishu", + accountId: "default", + conversationId: "oc_group_chat:topic:om_expired_root", + }; + + const binding = await service.bind({ + targetSessionKey, + targetKind: "session", + conversation, + placement: "current", + }); + await service.bind({ + targetSessionKey, + targetKind: "session", + conversation: { + ...conversation, + conversationId: "oc_group_chat:topic:om_expired_list_root", + }, + placement: "current", + }); + const expiresAt = startedAt + 86_400_000; + expect(binding.expiresAt).toBe(expiresAt); + + now.mockReturnValue(expiresAt - 1); + expect(manager.getByConversationId(conversation.conversationId)).toBeDefined(); + expect(manager.listBySessionKey(targetSessionKey)).toHaveLength(2); + expect(service.resolveByConversation(conversation)).not.toBeNull(); + expect(service.listBySession(targetSessionKey)).toHaveLength(2); + + now.mockReturnValue(expiresAt); + expect(service.resolveByConversation(conversation)).toBeNull(); + expect(manager.getByConversationId(conversation.conversationId)).toBeUndefined(); + expect(manager.listBySessionKey(targetSessionKey)).toEqual([]); + expect(service.listBySession(targetSessionKey)).toEqual([]); + }); + + it("expires bindings at their maximum age even after activity refreshes idle time", async () => { + const startedAt = 1_700_000_000_000; + const now = vi.spyOn(Date, "now").mockReturnValue(startedAt); + const cfg = { + session: { + mainKey: "main", + scope: "per-sender", + threadBindings: { idleHours: 2, maxAgeHours: 1 }, + }, + } satisfies OpenClawConfig; + createFeishuThreadBindingManager({ cfg, accountId: "default" }); + const service = getSessionBindingService(); + const conversation = { + channel: "feishu", + accountId: "default", + conversationId: "oc_group_chat:topic:om_max_age_root", + }; + const binding = await service.bind({ + targetSessionKey: "agent:codex:acp:binding:feishu:default:max-age", + targetKind: "session", + conversation, + placement: "current", + }); + + now.mockReturnValue(startedAt + 30 * 60_000); + service.touch(binding.bindingId); + expect(service.resolveByConversation(conversation)?.expiresAt).toBe(startedAt + 60 * 60_000); + + now.mockReturnValue(startedAt + 60 * 60_000); + expect(service.resolveByConversation(conversation)).toBeNull(); + expect(service.listBySession(binding.targetSessionKey)).toEqual([]); + }); + + it("does not revive an expired binding when its conversation is touched", async () => { + const startedAt = 1_700_000_000_000; + const now = vi.spyOn(Date, "now").mockReturnValue(startedAt); + const manager = createFeishuThreadBindingManager({ cfg: baseCfg, accountId: "default" }); + const service = getSessionBindingService(); + const conversation = { + channel: "feishu", + accountId: "default", + conversationId: "oc_group_chat:topic:om_expired_touch_root", + }; + const binding = await service.bind({ + targetSessionKey: "agent:codex:acp:binding:feishu:default:expired-touch", + targetKind: "session", + conversation, + placement: "current", + }); + const expiresAt = startedAt + 86_400_000; + expect(binding.expiresAt).toBe(expiresAt); + + now.mockReturnValue(expiresAt); + service.touch(binding.bindingId, expiresAt + 1); + + expect(manager.getByConversationId(conversation.conversationId)).toBeUndefined(); + expect(service.resolveByConversation(conversation)).toBeNull(); + }); + + it("keeps active bindings touchable and unbindable", async () => { + const startedAt = 1_700_000_000_000; + const now = vi.spyOn(Date, "now").mockReturnValue(startedAt); + createFeishuThreadBindingManager({ cfg: baseCfg, accountId: "default" }); + const service = getSessionBindingService(); + const conversation = { + channel: "feishu", + accountId: "default", + conversationId: "oc_group_chat:topic:om_active_root", + }; + const binding = await service.bind({ + targetSessionKey: "agent:codex:acp:binding:feishu:default:active", + targetKind: "session", + conversation, + placement: "current", + }); + + now.mockReturnValue(startedAt + 1_000); + service.touch(binding.bindingId); + expect(service.resolveByConversation(conversation)?.metadata?.lastActivityAt).toBe( + startedAt + 1_000, + ); + + await expect(service.unbind({ bindingId: binding.bindingId, reason: "test" })).resolves.toEqual( + [expect.objectContaining({ bindingId: binding.bindingId })], + ); + expect(service.resolveByConversation(conversation)).toBeNull(); + }); + it("clears account-scoped bindings when the manager stops", async () => { const manager = createFeishuThreadBindingManager({ cfg: baseCfg, accountId: "default" }); diff --git a/extensions/feishu/src/thread-bindings.ts b/extensions/feishu/src/thread-bindings.ts index 8c4dc198c129..0b7d2ddb6816 100644 --- a/extensions/feishu/src/thread-bindings.ts +++ b/extensions/feishu/src/thread-bindings.ts @@ -11,6 +11,7 @@ import { type SessionBindingAdapter, type SessionBindingRecord, } from "openclaw/plugin-sdk/conversation-runtime"; +import { isFutureDateTimestampMs } from "openclaw/plugin-sdk/number-runtime"; import { normalizeAccountId, resolveAgentIdFromSessionKey } from "openclaw/plugin-sdk/routing"; import { normalizeOptionalString } from "openclaw/plugin-sdk/string-coerce-runtime"; @@ -142,17 +143,44 @@ export function createFeishuThreadBindingManager(params: { channel: "feishu", accountId, }); + const bindingTimeouts = { idleTimeoutMs, maxAgeMs }; + + const resolveActiveBinding = ( + record: FeishuThreadBindingRecord | undefined, + now = Date.now(), + ): FeishuThreadBindingRecord | undefined => { + if (!record) { + return undefined; + } + const { expiresAt } = toSessionBindingRecord(record, bindingTimeouts); + if (expiresAt === undefined || isFutureDateTimestampMs(expiresAt, { nowMs: now })) { + return record; + } + + // Expire at the manager boundary so direct subagent reads and SDK adapters agree. + getState().bindingsByAccountConversation.delete( + resolveBindingKey({ accountId, conversationId: record.conversationId }), + ); + return undefined; + }; const manager: FeishuThreadBindingManager = { accountId, getByConversationId: (conversationId) => - getState().bindingsByAccountConversation.get( - resolveBindingKey({ accountId, conversationId }), - ), - listBySessionKey: (targetSessionKey) => - [...getState().bindingsByAccountConversation.values()].filter( - (record) => record.accountId === accountId && record.targetSessionKey === targetSessionKey, + resolveActiveBinding( + getState().bindingsByAccountConversation.get( + resolveBindingKey({ accountId, conversationId }), + ), ), + listBySessionKey: (targetSessionKey) => { + const now = Date.now(); + return [...getState().bindingsByAccountConversation.values()].filter( + (record) => + record.accountId === accountId && + record.targetSessionKey === targetSessionKey && + resolveActiveBinding(record, now) !== undefined, + ); + }, bindConversation: ({ conversationId, parentConversationId, @@ -165,9 +193,7 @@ export function createFeishuThreadBindingManager(params: { if (!normalizedConversationId || !normalizedTargetSessionKey) { return null; } - const existingLocal = getState().bindingsByAccountConversation.get( - resolveBindingKey({ accountId, conversationId: normalizedConversationId }), - ); + const existingLocal = manager.getByConversationId(normalizedConversationId); const now = Date.now(); const record: FeishuThreadBindingRecord = { accountId, @@ -211,7 +237,7 @@ export function createFeishuThreadBindingManager(params: { }, touchConversation: (conversationId, at = Date.now()) => { const key = resolveBindingKey({ accountId, conversationId }); - const existingRecord = getState().bindingsByAccountConversation.get(key); + const existingRecord = manager.getByConversationId(conversationId); if (!existingRecord) { return null; } @@ -273,18 +299,18 @@ export function createFeishuThreadBindingManager(params: { targetSessionKey: input.targetSessionKey, metadata: input.metadata, }); - return bound ? toSessionBindingRecord(bound, { idleTimeoutMs, maxAgeMs }) : null; + return bound ? toSessionBindingRecord(bound, bindingTimeouts) : null; }, listBySession: (targetSessionKey) => manager .listBySessionKey(targetSessionKey) - .map((entry) => toSessionBindingRecord(entry, { idleTimeoutMs, maxAgeMs })), + .map((entry) => toSessionBindingRecord(entry, bindingTimeouts)), resolveByConversation: (ref) => { if (ref.channel !== "feishu") { return null; } const found = manager.getByConversationId(ref.conversationId); - return found ? toSessionBindingRecord(found, { idleTimeoutMs, maxAgeMs }) : null; + return found ? toSessionBindingRecord(found, bindingTimeouts) : null; }, touch: (bindingId, at) => { const conversationId = resolveThreadBindingConversationIdFromBindingId({ @@ -299,7 +325,7 @@ export function createFeishuThreadBindingManager(params: { if (input.targetSessionKey?.trim()) { return manager .unbindBySessionKey(input.targetSessionKey.trim()) - .map((entry) => toSessionBindingRecord(entry, { idleTimeoutMs, maxAgeMs })); + .map((entry) => toSessionBindingRecord(entry, bindingTimeouts)); } const conversationId = resolveThreadBindingConversationIdFromBindingId({ accountId, @@ -309,7 +335,7 @@ export function createFeishuThreadBindingManager(params: { return []; } const removed = manager.unbindConversation(conversationId); - return removed ? [toSessionBindingRecord(removed, { idleTimeoutMs, maxAgeMs })] : []; + return removed ? [toSessionBindingRecord(removed, bindingTimeouts)] : []; }, }; diff --git a/extensions/matrix/src/matrix/monitor/handler-reply-dispatcher.ts b/extensions/matrix/src/matrix/monitor/handler-reply-dispatcher.ts index cc88ec634538..61e98d2d1803 100644 --- a/extensions/matrix/src/matrix/monitor/handler-reply-dispatcher.ts +++ b/extensions/matrix/src/matrix/monitor/handler-reply-dispatcher.ts @@ -72,6 +72,8 @@ export function createMatrixReplyDispatcher(config: { logVerboseMessage, } = config; const quietDraftStreaming = streaming === "quiet" || streaming === "progress"; + // Tool, block, and final payloads are delivered separately but share one first-reply slot. + const hasRepliedRef = { value: false }; let finalReplyDeliveryFailed = false; let nonFinalReplyDeliveryFailed = false; @@ -99,6 +101,7 @@ export function createMatrixReplyDispatcher(config: { runtime, textLimit, replyToMode, + hasRepliedRef, threadId: threadTarget, replyToId: threadTarget ?? replyToEventId ?? undefined, accountId, @@ -211,6 +214,7 @@ export function createMatrixReplyDispatcher(config: { runtime, textLimit, replyToMode, + hasRepliedRef, threadId: threadTarget, replyToId: threadTarget ?? replyToEventId ?? undefined, accountId, @@ -276,6 +280,7 @@ export function createMatrixReplyDispatcher(config: { runtime, textLimit, replyToMode, + hasRepliedRef, threadId: threadTarget, replyToId: threadTarget ?? replyToEventId ?? undefined, accountId, @@ -301,6 +306,7 @@ export function createMatrixReplyDispatcher(config: { runtime, textLimit, replyToMode, + hasRepliedRef, threadId: threadTarget, replyToId: threadTarget ?? replyToEventId ?? undefined, accountId, @@ -333,6 +339,7 @@ export function createMatrixReplyDispatcher(config: { runtime, textLimit, replyToMode, + hasRepliedRef, threadId: threadTarget, replyToId: threadTarget ?? replyToEventId ?? undefined, accountId, diff --git a/extensions/matrix/src/matrix/monitor/handler.test.ts b/extensions/matrix/src/matrix/monitor/handler.test.ts index ea3d803419d7..c18f8a181e07 100644 --- a/extensions/matrix/src/matrix/monitor/handler.test.ts +++ b/extensions/matrix/src/matrix/monitor/handler.test.ts @@ -2979,6 +2979,28 @@ describe("matrix monitor handler draft streaming", () => { return { dispatch, redactEventMock }; } + it("shares first-reply state between tool and final Matrix deliveries", async () => { + const { dispatch } = createStreamingHarness({ replyToMode: "first", streaming: "off" }); + const { deliver, finish } = await dispatch(); + + await deliver({ text: "tool result", replyToId: "$msg1" }, { kind: "tool" }); + await deliver({ text: "final result" }, { kind: "final" }); + + expect(deliverMatrixRepliesMock).toHaveBeenCalledTimes(2); + const toolDelivery = requireRecord( + callArg(deliverMatrixRepliesMock, 0, 0, "Matrix tool reply"), + "Matrix tool reply", + ); + const finalDelivery = requireRecord( + callArg(deliverMatrixRepliesMock, 1, 0, "Matrix final reply"), + "Matrix final reply", + ); + expect(toolDelivery.hasRepliedRef).toEqual({ value: false }); + expect(finalDelivery.hasRepliedRef).toBe(toolDelivery.hasRepliedRef); + + await finish(); + }); + it("finalizes a single quiet-preview block in place when block streaming is enabled", async () => { const { dispatch, redactEventMock } = createStreamingHarness({ blockStreamingEnabled: true }); const { deliver, opts, finish } = await dispatch(); diff --git a/extensions/matrix/src/matrix/monitor/replies.test.ts b/extensions/matrix/src/matrix/monitor/replies.test.ts index 17523109990e..508c0ffef6f3 100644 --- a/extensions/matrix/src/matrix/monitor/replies.test.ts +++ b/extensions/matrix/src/matrix/monitor/replies.test.ts @@ -115,6 +115,80 @@ describe("deliverMatrixReplies", () => { expect(sendOptions(2).threadId).toBeUndefined(); }); + it("shares the first reply across separately dispatched, chunked payloads", async () => { + chunkMatrixTextMock.mockImplementation((text: string) => ({ + trimmedText: text.trim(), + convertedText: text, + singleEventLimit: 4000, + fitsInSingleEvent: true, + chunks: text.split("|"), + })); + const hasRepliedRef = { value: false }; + const delivery = { + cfg, + roomId: "room:1", + client: {} as MatrixClient, + runtime: runtimeEnv, + textLimit: 4000, + replyToMode: "first" as const, + replyToId: "reply-1", + hasRepliedRef, + }; + + await deliverMatrixReplies({ ...delivery, replies: [{ text: "first-a|first-b" }] }); + await deliverMatrixReplies({ ...delivery, replies: [{ text: "second" }] }); + + expect(sendMessageMatrixMock).toHaveBeenCalledTimes(3); + expect(sendOptions(0).replyToId).toBe("reply-1"); + expect(sendOptions(1).replyToId).toBe("reply-1"); + expect(sendOptions(2).replyToId).toBeUndefined(); + expect(hasRepliedRef.value).toBe(true); + }); + + it("does not consume the first reply when Matrix delivery fails", async () => { + const hasRepliedRef = { value: false }; + const delivery = { + cfg, + replies: [{ text: "retry me" }], + roomId: "room:1", + client: {} as MatrixClient, + runtime: runtimeEnv, + textLimit: 4000, + replyToMode: "first" as const, + replyToId: "reply-1", + hasRepliedRef, + }; + sendMessageMatrixMock.mockRejectedValueOnce(new Error("Matrix unavailable")); + + await expect(deliverMatrixReplies(delivery)).rejects.toThrow("Matrix unavailable"); + expect(hasRepliedRef.value).toBe(false); + + await expect(deliverMatrixReplies(delivery)).resolves.toBe(true); + expect(sendOptions(0).replyToId).toBe("reply-1"); + expect(sendOptions(1).replyToId).toBe("reply-1"); + expect(hasRepliedRef.value).toBe(true); + }); + + it("preserves native thread fallback after the first reply has been consumed", async () => { + const hasRepliedRef = { value: true }; + + await deliverMatrixReplies({ + cfg, + replies: [{ text: "thread follow-up" }], + roomId: "room:3", + client: {} as MatrixClient, + runtime: runtimeEnv, + textLimit: 4000, + replyToMode: "first", + replyToId: "reply-thread", + threadId: "thread-77", + hasRepliedRef, + }); + + expect(sendOptions(0).replyToId).toBe("reply-thread"); + expect(sendOptions(0).threadId).toBe("thread-77"); + }); + it("keeps replyToId on every reply when replyToMode=all", async () => { await deliverMatrixReplies({ cfg, diff --git a/extensions/matrix/src/matrix/monitor/replies.ts b/extensions/matrix/src/matrix/monitor/replies.ts index 377465cf1fb8..8d637547b9bb 100644 --- a/extensions/matrix/src/matrix/monitor/replies.ts +++ b/extensions/matrix/src/matrix/monitor/replies.ts @@ -29,6 +29,7 @@ export async function deliverMatrixReplies(params: { runtime: RuntimeEnv; textLimit: number; replyToMode: "off" | "first" | "all" | "batched"; + hasRepliedRef?: { value: boolean }; threadId?: string; replyToId?: string; accountId?: string; @@ -48,7 +49,7 @@ export async function deliverMatrixReplies(params: { params.runtime.log?.(message); } }; - let hasReplied = false; + const hasRepliedRef = params.hasRepliedRef ?? { value: false }; let deliveredAny = false; for (const reply of params.replies) { const visibleText = resolveVisibleMatrixReplyText(reply.text); @@ -79,11 +80,10 @@ export async function deliverMatrixReplies(params: { : []; const shouldIncludeReply = (id?: string) => - Boolean(id) && (params.threadId || params.replyToMode === "all" || !hasReplied); + Boolean(id) && (params.threadId || params.replyToMode === "all" || !hasRepliedRef.value); const replyToIdForReply = shouldIncludeReply(replyToId) ? replyToId : undefined; if (mediaList.length === 0) { - let sentTextChunk = false; const { chunks } = chunkMatrixText(rawText, { cfg: params.cfg, accountId: params.accountId, @@ -102,10 +102,9 @@ export async function deliverMatrixReplies(params: { accountId: params.accountId, }); deliveredAny = true; - sentTextChunk = true; - } - if (replyToIdForReply && !hasReplied && sentTextChunk) { - hasReplied = true; + if (replyToIdForReply) { + hasRepliedRef.value = true; + } } continue; } @@ -124,11 +123,11 @@ export async function deliverMatrixReplies(params: { accountId: params.accountId, }); deliveredAny = true; + if (replyToIdForReply) { + hasRepliedRef.value = true; + } first = false; } - if (replyToIdForReply && !hasReplied) { - hasReplied = true; - } } return deliveredAny; } diff --git a/extensions/msteams/src/channel.ts b/extensions/msteams/src/channel.ts index 49b3aaf9263a..79c91403d8f5 100644 --- a/extensions/msteams/src/channel.ts +++ b/extensions/msteams/src/channel.ts @@ -446,6 +446,8 @@ const msteamsChannelOutbound: ChannelOutboundAdapter = { chunker: chunkTextForOutbound, chunkerMode: "markdown", textChunkLimit: 4000, + resolveEffectiveTextChunkLimit: ({ fallbackLimit }) => + typeof fallbackLimit === "number" && fallbackLimit > 0 ? Math.min(fallbackLimit, 4000) : 4000, pollMaxOptions: 12, deliveryCapabilities: { durableFinal: { diff --git a/extensions/msteams/src/outbound.test.ts b/extensions/msteams/src/outbound.test.ts index c0bfb16579d9..0978f7169ea2 100644 --- a/extensions/msteams/src/outbound.test.ts +++ b/extensions/msteams/src/outbound.test.ts @@ -21,6 +21,7 @@ vi.mock("./polls.js", () => ({ }), })); +import { msteamsPlugin } from "./channel.js"; import { msteamsOutbound } from "./outbound.js"; const cfg = { @@ -125,6 +126,27 @@ describe("msteamsOutbound cfg threading", () => { }); }); + it.each([ + { configuredLimit: 6000, expectedLimit: 4000 }, + { configuredLimit: 1000, expectedLimit: 1000 }, + ])( + "resolves the same capped $configuredLimit-character limit for lightweight and runtime outbound", + ({ configuredLimit, expectedLimit }) => { + const configuredCfg = { + channels: { + msteams: { + appId: "resolved-app-id", + textChunkLimit: configuredLimit, + }, + }, + } as OpenClawConfig; + const params = { cfg: configuredCfg, fallbackLimit: configuredLimit }; + + expect(msteamsPlugin.outbound?.resolveEffectiveTextChunkLimit?.(params)).toBe(expectedLimit); + expect(msteamsOutbound.resolveEffectiveTextChunkLimit?.(params)).toBe(expectedLimit); + }, + ); + it("passes resolved cfg to sendMessageMSTeams for text sends", async () => { const cfgResult = { channels: { @@ -353,6 +375,43 @@ describe("msteamsOutbound cfg threading", () => { }); }); + it.each([ + { configuredLimit: 6000, textLength: 5000, expectedChunkLengths: [4000, 1000] }, + { configuredLimit: 1000, textLength: 1500, expectedChunkLengths: [1000, 500] }, + ])( + "uses the capped $configuredLimit-character configured limit for fallback payloads", + async ({ configuredLimit, textLength, expectedChunkLengths }) => { + const configuredCfg = { + channels: { + msteams: { + appId: "resolved-app-id", + textChunkLimit: configuredLimit, + }, + }, + } as OpenClawConfig; + const text = "x".repeat(textLength); + + await requireSendPayload()({ + cfg: configuredCfg, + to: "conversation:abc", + text, + payload: { + text, + channelData: { msteams: { traceId: "trace-1" } }, + }, + }); + + expect(mocks.sendMessageMSTeams).toHaveBeenCalledTimes(expectedChunkLengths.length); + for (const [index, chunkLength] of expectedChunkLengths.entries()) { + expect(mocks.sendMessageMSTeams).toHaveBeenNthCalledWith(index + 1, { + cfg: configuredCfg, + to: "conversation:abc", + text: "x".repeat(chunkLength), + }); + } + }, + ); + it("keeps multi-media payloads on the media fallback path", async () => { mocks.sendMessageMSTeams .mockResolvedValueOnce({ messageId: "msg-media-1", conversationId: "conv-media" }) diff --git a/extensions/msteams/src/outbound.ts b/extensions/msteams/src/outbound.ts index 21bfbd6bee37..c1bdb5ec16a4 100644 --- a/extensions/msteams/src/outbound.ts +++ b/extensions/msteams/src/outbound.ts @@ -24,6 +24,12 @@ import { sendAdaptiveCardMSTeams, sendMessageMSTeams, sendPollMSTeams } from "./ const MSTEAMS_TEXT_CHUNK_LIMIT = 4000; +function resolveMSTeamsEffectiveTextChunkLimit(configuredLimit?: number): number { + return typeof configuredLimit === "number" && configuredLimit > 0 + ? Math.min(configuredLimit, MSTEAMS_TEXT_CHUNK_LIMIT) + : MSTEAMS_TEXT_CHUNK_LIMIT; +} + type MSTeamsSendConfig = Parameters[0]["cfg"]; type MSTeamsSendResult = { messageId: string; conversationId: string }; type MSTeamsMediaSendOptions = { @@ -71,6 +77,8 @@ export const msteamsOutbound: ChannelOutboundAdapter = { chunker: chunkTextForOutbound, chunkerMode: "markdown", textChunkLimit: MSTEAMS_TEXT_CHUNK_LIMIT, + resolveEffectiveTextChunkLimit: ({ fallbackLimit }) => + resolveMSTeamsEffectiveTextChunkLimit(fallbackLimit), pollMaxOptions: 12, deliveryCapabilities: { durableFinal: { @@ -151,7 +159,10 @@ export const msteamsOutbound: ChannelOutboundAdapter = { const send = resolveMSTeamsTextSend({ cfg, deps }); const chunks = resolveTextChunksWithFallback( text, - chunkTextForOutbound(text, MSTEAMS_TEXT_CHUNK_LIMIT), + chunkTextForOutbound( + text, + resolveMSTeamsEffectiveTextChunkLimit(cfg.channels?.msteams?.textChunkLimit), + ), ); let result: Awaited>; for (const chunk of chunks) { diff --git a/extensions/openai/image-generation-provider.test.ts b/extensions/openai/image-generation-provider.test.ts index 66144ceba31c..2ff8c2d9bb40 100644 --- a/extensions/openai/image-generation-provider.test.ts +++ b/extensions/openai/image-generation-provider.test.ts @@ -1014,7 +1014,7 @@ describe("openai image generation provider", () => { expect(result.images).toHaveLength(1); }); - it("forwards output and OpenAI-only options on multipart edits", async () => { + it("forwards supported OpenAI options on multipart edits", async () => { mockGeneratedPngResponse(); const provider = buildOpenAIImageGenerationProvider(); @@ -1029,7 +1029,6 @@ describe("openai image generation provider", () => { providerOptions: { openai: { background: "transparent", - moderation: "auto", outputCompression: 75, user: "end-user-99", }, @@ -1043,13 +1042,43 @@ describe("openai image generation provider", () => { expect(form.get("quality")).toBe("high"); expect(form.get("output_format")).toBe("webp"); expect(form.get("background")).toBe("transparent"); - expect(form.get("moderation")).toBe("auto"); + expect(form.get("moderation")).toBeNull(); expect(form.get("output_compression")).toBe("75"); expect(form.get("user")).toBe("end-user-99"); expect(result.images[0]?.mimeType).toBe("image/webp"); expect(result.images[0]?.fileName).toBe("image-1.webp"); }); + it.each(["low", "auto"] as const)( + "forwards %s moderation on multipart image edits", + async (moderation) => { + mockGeneratedPngResponse(); + + const provider = buildOpenAIImageGenerationProvider(); + const result = await provider.generateImage({ + provider: "openai", + model: "gpt-image-2", + prompt: "Edit with supported moderation", + cfg: {}, + inputImages: [{ buffer: Buffer.from("png-bytes"), mimeType: "image/png" }], + providerOptions: { + openai: { moderation }, + }, + }); + + const request = multipartRequestCall() as RequestCall & { + body: FormData; + }; + expect(postMultipartRequestMock).toHaveBeenCalledOnce(); + expect(request.url).toBe("https://api.openai.com/v1/images/edits"); + expect(request.body).toBeInstanceOf(FormData); + expect(request.body.get("moderation")).toBe(moderation); + expect(postJsonRequestMock).not.toHaveBeenCalled(); + expect(result.images).toHaveLength(1); + expect(result.images[0]?.mimeType).toBe("image/png"); + }, + ); + it("falls back to Codex OAuth image generation through Responses streaming", async () => { mockCodexAuthOnly(); mockCodexImageStream({ imageData: "codex-image", revisedPrompt: "revised codex prompt" }); diff --git a/extensions/openai/openai-provider.test.ts b/extensions/openai/openai-provider.test.ts index cdfbd399469e..9a20fce96ab6 100644 --- a/extensions/openai/openai-provider.test.ts +++ b/extensions/openai/openai-provider.test.ts @@ -379,6 +379,9 @@ describe("buildOpenAIProvider", () => { throw new Error("expected OpenAI static provider catalog"); } const gpt55 = result.providers.openai?.models.find((model) => model.id === "gpt-5.5"); + const gpt54Models = result.providers.openai?.models.filter((model) => + model.id.startsWith("gpt-5.4"), + ); const gpt56Models = result.providers.openai?.models.filter((model) => model.id.startsWith("gpt-5.6"), ); @@ -403,6 +406,44 @@ describe("buildOpenAIProvider", () => { expect(gpt56Models?.map((model) => model.compat?.supportedReasoningEfforts)).toEqual( Array.from({ length: 4 }, () => ["none", "low", "medium", "high", "xhigh", "max"]), ); + expect(gpt54Models).toMatchObject([ + { + id: "gpt-5.4", + name: "GPT-5.4", + reasoning: true, + input: ["text", "image"], + contextWindow: 1_050_000, + maxTokens: 128_000, + cost: { input: 2.5, output: 15, cacheRead: 0.25, cacheWrite: 0 }, + }, + { + id: "gpt-5.4-pro", + name: "GPT-5.4 Pro", + reasoning: true, + input: ["text", "image"], + contextWindow: 1_050_000, + maxTokens: 128_000, + cost: { input: 30, output: 180, cacheRead: 0, cacheWrite: 0 }, + }, + { + id: "gpt-5.4-mini", + name: "GPT-5.4 Mini", + reasoning: true, + input: ["text", "image"], + contextWindow: 400_000, + maxTokens: 128_000, + cost: { input: 0.75, output: 4.5, cacheRead: 0.075, cacheWrite: 0 }, + }, + { + id: "gpt-5.4-nano", + name: "GPT-5.4 Nano", + reasoning: true, + input: ["text", "image"], + contextWindow: 400_000, + maxTokens: 128_000, + cost: { input: 0.2, output: 1.25, cacheRead: 0.02, cacheWrite: 0 }, + }, + ]); expect(OPENAI_DEFAULT_MODEL).toBe("openai/gpt-5.6"); expect(OPENAI_CODEX_DEFAULT_MODEL).toBe("openai/gpt-5.6-sol"); }); @@ -2005,6 +2046,45 @@ describe("buildOpenAIProvider", () => { expect(codexSolFromNativeCatalog?.levels.map((level) => level.id)).toContain("ultra"); }); + it.each([ + { modelId: "gpt-5.4", contextWindow: 1_050_000 }, + { modelId: "gpt-5.4-pro", contextWindow: 1_050_000 }, + { modelId: "gpt-5.4-mini", contextWindow: 400_000 }, + { modelId: "gpt-5.4-nano", contextWindow: 400_000 }, + ])( + "restores native image capability to an existing $modelId catalog row", + ({ modelId, contextWindow }) => { + const provider = buildOpenAIProvider(); + const existingRoute = { + provider: "openai", + id: modelId, + name: `Stale ${modelId}`, + api: "openai-responses", + baseUrl: "https://api.openai.com/v1", + input: ["text"], + contextWindow: 8_192, + cost: { input: 2.5, output: 15, cacheRead: 0.25, cacheWrite: 0 }, + }; + + const entries = provider.augmentModelCatalog?.({ + env: process.env, + entries: [existingRoute], + } as never); + + expectCatalogEntry(entries, modelId, { + provider: "openai", + id: modelId, + name: modelId, + api: existingRoute.api, + baseUrl: existingRoute.baseUrl, + reasoning: true, + input: ["text", "image"], + contextWindow, + cost: existingRoute.cost, + }); + }, + ); + it("keeps chat-latest and gpt-5.5 out of synthetic catalog metadata", () => { const provider = buildOpenAIProvider(); diff --git a/extensions/openai/openai-provider.ts b/extensions/openai/openai-provider.ts index 2961b5c4038c..986e64a89a45 100644 --- a/extensions/openai/openai-provider.ts +++ b/extensions/openai/openai-provider.ts @@ -138,10 +138,19 @@ const OPENAI_GPT_55_PRO_TEMPLATE_MODEL_IDS = [ const OPENAI_GPT_55_MEDIA_INPUT = { image: { maxSidePx: 6000, preferredSidePx: 2048, tokenMode: "detail" }, } as const satisfies ProviderRuntimeModel["mediaInput"]; -const OPENAI_GPT_54_TEMPLATE_MODEL_IDS = [OPENAI_GPT_55_MODEL_ID] as const; -const OPENAI_GPT_54_PRO_TEMPLATE_MODEL_IDS = [OPENAI_GPT_55_PRO_MODEL_ID] as const; -const OPENAI_GPT_54_MINI_TEMPLATE_MODEL_IDS = ["gpt-5-mini"] as const; -const OPENAI_GPT_54_NANO_TEMPLATE_MODEL_IDS = ["gpt-5-nano", "gpt-5-mini"] as const; +// Repair an already-authorized model before borrowing an older family template; +// remote discovery must not be needed to restore its native image capability. +const OPENAI_GPT_54_TEMPLATE_MODEL_IDS = [OPENAI_GPT_54_MODEL_ID, OPENAI_GPT_55_MODEL_ID] as const; +const OPENAI_GPT_54_PRO_TEMPLATE_MODEL_IDS = [ + OPENAI_GPT_54_PRO_MODEL_ID, + OPENAI_GPT_55_PRO_MODEL_ID, +] as const; +const OPENAI_GPT_54_MINI_TEMPLATE_MODEL_IDS = [OPENAI_GPT_54_MINI_MODEL_ID, "gpt-5-mini"] as const; +const OPENAI_GPT_54_NANO_TEMPLATE_MODEL_IDS = [ + OPENAI_GPT_54_NANO_MODEL_ID, + "gpt-5-nano", + "gpt-5-mini", +] as const; const OPENAI_CHAT_LATEST_TEMPLATE_MODEL_IDS = [ OPENAI_GPT_55_MODEL_ID, OPENAI_GPT_54_MODEL_ID, @@ -181,7 +190,11 @@ function buildOpenAIManifestModelsForBaseUrl(baseUrl: string): ModelDefinitionCo return OPENAI_MANIFEST_PROVIDER.models.map((model) => model.api === "openai-chatgpt-responses" || isOpenAICodexBaseUrl(model.baseUrl) ? { ...model } - : { ...model, baseUrl }, + : { + ...model, + api: model.api ?? OPENAI_MANIFEST_PROVIDER.api ?? "openai-responses", + baseUrl, + }, ); } diff --git a/extensions/openai/openclaw.plugin.json b/extensions/openai/openclaw.plugin.json index 21d33c41ff73..efffb89a56c5 100644 --- a/extensions/openai/openclaw.plugin.json +++ b/extensions/openai/openclaw.plugin.json @@ -139,6 +139,42 @@ "maxTokens": 128000, "cost": { "input": 30, "output": 180, "cacheRead": 0, "cacheWrite": 0 }, "compat": { "codeMode": "preferred" } + }, + { + "id": "gpt-5.4", + "name": "GPT-5.4", + "reasoning": true, + "input": ["text", "image"], + "contextWindow": 1050000, + "maxTokens": 128000, + "cost": { "input": 2.5, "output": 15, "cacheRead": 0.25, "cacheWrite": 0 } + }, + { + "id": "gpt-5.4-pro", + "name": "GPT-5.4 Pro", + "reasoning": true, + "input": ["text", "image"], + "contextWindow": 1050000, + "maxTokens": 128000, + "cost": { "input": 30, "output": 180, "cacheRead": 0, "cacheWrite": 0 } + }, + { + "id": "gpt-5.4-mini", + "name": "GPT-5.4 Mini", + "reasoning": true, + "input": ["text", "image"], + "contextWindow": 400000, + "maxTokens": 128000, + "cost": { "input": 0.75, "output": 4.5, "cacheRead": 0.075, "cacheWrite": 0 } + }, + { + "id": "gpt-5.4-nano", + "name": "GPT-5.4 Nano", + "reasoning": true, + "input": ["text", "image"], + "contextWindow": 400000, + "maxTokens": 128000, + "cost": { "input": 0.2, "output": 1.25, "cacheRead": 0.02, "cacheWrite": 0 } } ] } diff --git a/extensions/qa-lab/src/bus-server.test.ts b/extensions/qa-lab/src/bus-server.test.ts index 3bdcfd8f7233..efe7fb4ed24f 100644 --- a/extensions/qa-lab/src/bus-server.test.ts +++ b/extensions/qa-lab/src/bus-server.test.ts @@ -269,6 +269,47 @@ describe("qa-bus server", () => { }); }); + it.each(["inbound", "outbound"] as const)( + "accepts a generated-media payload larger than 1 MiB on the %s message route", + async (direction) => { + const state = createQaBusState(); + const bus = await startQaBusServer({ state }); + stops.push(bus["stop"]); + + const generatedImage = Buffer.alloc(1_600_000, 0x71); + const attachment = { + id: "qa-lighthouse-image", + kind: "image", + mimeType: "image/png", + fileName: "qa-lighthouse.png", + contentBase64: generatedImage.toString("base64"), + }; + const response = await postQaBusJson(bus.baseUrl, `/v1/${direction}/message`, { + accountId: "acct-a", + text: "QA lighthouse", + attachments: [attachment], + ...(direction === "inbound" + ? { + conversation: { id: "qa-operator", kind: "direct" }, + senderId: "qa-operator", + } + : { to: "dm:qa-operator" }), + }); + + expect(response.status).toBe(200); + await expect(response.json()).resolves.toMatchObject({ + message: { direction, attachments: [attachment] }, + }); + + const snapshot = state.getSnapshot(); + expect(snapshot.messages).toHaveLength(1); + expect(snapshot.events).toHaveLength(1); + const storedAttachment = snapshot.messages[0]?.attachments?.[0]; + expect(storedAttachment).toEqual(attachment); + expect(Buffer.from(storedAttachment?.contentBase64 ?? "", "base64")).toEqual(generatedImage); + }, + ); + it("returns a controlled error when a v1 POST body contains malformed JSON", async () => { const state = createQaBusState(); const bus = await startQaBusServer({ state }); @@ -328,6 +369,42 @@ describe("qa-bus server", () => { }); describe("handleQaBusRequest", () => { + it.each(["/v1/inbound/message", "/v1/outbound/message"] as const)( + "returns a controlled error when the %s body exceeds the media limit", + async (pathname) => { + const req = { + method: "POST", + url: pathname, + headers: { "content-length": String(16 * 1024 * 1024 + 1) }, + destroyed: false, + destroy() { + this.destroyed = true; + }, + }; + const res = { + statusCode: 0, + body: "", + writeHead(statusCode: number) { + this.statusCode = statusCode; + }, + end(payload: string) { + this.body = payload; + }, + }; + + const handled = await handleQaBusRequest({ + req: req as never, + res: res as never, + state: createQaBusState(), + }); + + expect(handled).toBe(true); + expect(req.destroyed).toBe(true); + expect(res.statusCode).toBe(413); + expect(JSON.parse(res.body)).toEqual({ error: "Payload too large" }); + }, + ); + it("returns a controlled error when a v1 POST body exceeds the limit", async () => { const req = { method: "POST", diff --git a/extensions/qa-lab/src/bus-server.ts b/extensions/qa-lab/src/bus-server.ts index 70820b0c4ea1..aaa1f3b426c8 100644 --- a/extensions/qa-lab/src/bus-server.ts +++ b/extensions/qa-lab/src/bus-server.ts @@ -22,6 +22,7 @@ import type { } from "./runtime-api.js"; const QA_HTTP_JSON_MAX_BODY_BYTES = 1024 * 1024; +const QA_HTTP_MEDIA_JSON_MAX_BODY_BYTES = 16 * 1024 * 1024; const QA_HTTP_JSON_BODY_TIMEOUT_MS = 5_000; const QA_BUS_POLL_TIMEOUT_MAX_MS = 30_000; const QA_BUS_POLL_LIMIT_MAX = 500; @@ -39,10 +40,13 @@ export function isQaMalformedJsonBodyError(error: unknown): error is Error { return error instanceof QaMalformedJsonBodyError; } -export async function readQaJsonBody(req: IncomingMessage): Promise { +export async function readQaJsonBody( + req: IncomingMessage, + options?: { maxBytes?: number }, +): Promise { const text = ( await readRequestBodyWithLimit(req, { - maxBytes: QA_HTTP_JSON_MAX_BODY_BYTES, + maxBytes: options?.maxBytes ?? QA_HTTP_JSON_MAX_BODY_BYTES, timeoutMs: QA_HTTP_JSON_BODY_TIMEOUT_MS, }) ).trim(); @@ -185,7 +189,12 @@ export async function handleQaBusRequest(params: { } try { - const body = (await readQaJsonBody(params.req)) as Record; + const body = (await readQaJsonBody( + params.req, + url.pathname === "/v1/inbound/message" || url.pathname === "/v1/outbound/message" + ? { maxBytes: QA_HTTP_MEDIA_JSON_MAX_BODY_BYTES } + : undefined, + )) as Record; switch (url.pathname) { case "/v1/reset": params.state.reset(); diff --git a/extensions/qa-lab/src/cli.runtime.test.ts b/extensions/qa-lab/src/cli.runtime.test.ts index f1c92b6d43df..bd0eac4708f4 100644 --- a/extensions/qa-lab/src/cli.runtime.test.ts +++ b/extensions/qa-lab/src/cli.runtime.test.ts @@ -401,6 +401,103 @@ describe("qa cli runtime", () => { expectWriteContains(stdoutWrite, `QA suite summary: ${suiteSummaryPath}`); }); + it("rejects a direct suite containing only report-only optional tool skips", async () => { + const optionalScenario = { + name: "Runtime tool fixture — image_generate", + status: "skip" as const, + details: "image_generate mock provider report-only: tool unavailable", + }; + await fs.writeFile( + suiteSummaryPath, + JSON.stringify({ + counts: { total: 1, passed: 0, failed: 0, skipped: 1 }, + scenarios: [optionalScenario], + }), + "utf8", + ); + runQaSuite.mockResolvedValueOnce( + unifiedSuiteRuntimeResult({ + outputDir: suiteArtifactsDir, + reportPath: suiteReportPath, + summaryPath: suiteSummaryPath, + evidencePath: suiteEvidencePath, + scenarios: [optionalScenario], + }), + ); + + await expect(runQaSuiteCommand({ repoRoot: "/tmp/openclaw-repo" })).rejects.toThrow( + "did not include any executed scenarios", + ); + }); + + it("keeps a direct suite green for a real pass and a report-only optional tool skip", async () => { + const priorExitCode = process.exitCode; + process.exitCode = undefined; + const optionalScenario = { + name: "Runtime tool fixture — image_generate", + status: "skip" as const, + details: "image_generate mock provider report-only: tool unavailable", + }; + await fs.writeFile( + suiteSummaryPath, + JSON.stringify({ + counts: { total: 2, passed: 1, failed: 0, skipped: 1 }, + scenarios: [QA_PASSING_SUITE_SCENARIO, optionalScenario], + }), + "utf8", + ); + runQaSuite.mockResolvedValueOnce( + unifiedSuiteRuntimeResult({ + outputDir: suiteArtifactsDir, + reportPath: suiteReportPath, + summaryPath: suiteSummaryPath, + evidencePath: suiteEvidencePath, + scenarios: [QA_PASSING_SUITE_SCENARIO, optionalScenario], + }), + ); + + try { + await runQaSuiteCommand({ repoRoot: "/tmp/openclaw-repo" }); + expect(process.exitCode).toBeUndefined(); + } finally { + process.exitCode = priorExitCode; + } + }); + + it("keeps direct-suite zero-work validation disabled with --allow-failures", async () => { + const priorExitCode = process.exitCode; + process.exitCode = undefined; + const optionalScenario = { + name: "Runtime tool fixture — image_generate", + status: "skip" as const, + details: "image_generate mock provider report-only: tool unavailable", + }; + await fs.writeFile( + suiteSummaryPath, + JSON.stringify({ + counts: { total: 1, passed: 0, failed: 0, skipped: 1 }, + scenarios: [optionalScenario], + }), + "utf8", + ); + runQaSuite.mockResolvedValueOnce( + unifiedSuiteRuntimeResult({ + outputDir: suiteArtifactsDir, + reportPath: suiteReportPath, + summaryPath: suiteSummaryPath, + evidencePath: suiteEvidencePath, + scenarios: [optionalScenario], + }), + ); + + try { + await runQaSuiteCommand({ repoRoot: "/tmp/openclaw-repo", allowFailures: true }); + expect(process.exitCode).toBeUndefined(); + } finally { + process.exitCode = priorExitCode; + } + }); + it("rejects host-only resource options for Playwright scenarios", async () => { await expect( runQaSuiteCommand({ @@ -628,7 +725,7 @@ describe("qa cli runtime", () => { expect(suiteArgs.scenarioIds).not.toContain("control-ui-qa-channel-image-roundtrip"); }); - it("rejects explicit profile selections outside the profile taxonomy", async () => { + it("rejects explicit profile selections incompatible with the profile channel", async () => { await expect( runQaProfileCommand({ repoRoot: "/tmp/openclaw-repo", @@ -636,7 +733,7 @@ describe("qa cli runtime", () => { scenarioIds: ["control-ui-qa-channel-image-roundtrip"], }), ).rejects.toThrow( - "qa run did not find taxonomy scenarios for --qa-profile smoke-ci --scenario control-ui-qa-channel-image-roundtrip.", + "qa run --qa-profile smoke-ci cannot run explicitly selected scenario(s): control-ui-qa-channel-image-roundtrip (channelDriver=qa-channel).", ); expect(runQaSuite).not.toHaveBeenCalled(); @@ -1456,7 +1553,7 @@ describe("qa cli runtime", () => { } }); - it("keeps full host suite exit code clear for report-only optional tool skips", async () => { + it("rejects a full host suite containing only report-only optional tool skips", async () => { const priorExitCode = process.exitCode; process.exitCode = undefined; await fs.writeFile( @@ -1480,6 +1577,40 @@ describe("qa cli runtime", () => { }), ); + try { + await expect(runQaSuiteCommand({ repoRoot: "/tmp/openclaw-repo" })).rejects.toThrow( + "did not include any executed scenarios", + ); + expect(process.exitCode).toBeUndefined(); + } finally { + process.exitCode = priorExitCode; + } + }); + + it("keeps full host suite exit code clear for a real pass and an optional tool skip", async () => { + const priorExitCode = process.exitCode; + process.exitCode = undefined; + const optionalScenario = { + name: "Runtime tool fixture — image_generate", + status: "skip" as const, + details: "image_generate mock provider report-only: tool unavailable", + }; + await fs.writeFile( + suiteSummaryPath, + JSON.stringify({ + counts: { total: 2, passed: 1, failed: 0, skipped: 1 }, + scenarios: [QA_PASSING_SUITE_SCENARIO, optionalScenario], + }), + "utf8", + ); + runQaSuite.mockResolvedValueOnce( + flowSuiteRuntimeResult({ + reportPath: suiteReportPath, + summaryPath: suiteSummaryPath, + scenarios: [QA_PASSING_SUITE_SCENARIO, optionalScenario], + }), + ); + try { await runQaSuiteCommand({ repoRoot: "/tmp/openclaw-repo" }); expect(process.exitCode).toBeUndefined(); diff --git a/extensions/qa-lab/src/cli.runtime.ts b/extensions/qa-lab/src/cli.runtime.ts index 806ee4e7e55e..2fd79799e9af 100644 --- a/extensions/qa-lab/src/cli.runtime.ts +++ b/extensions/qa-lab/src/cli.runtime.ts @@ -98,10 +98,7 @@ import { runQaSuiteWithInfraRetry, } from "./suite-launch.runtime.js"; import { resolveQaSuiteScenarioChannel, resolveQaSuiteScenarioChannels } from "./suite-planning.js"; -import { - isQaSuiteReportOnlyOptionalScenario, - readQaSuiteFailedOrSkippedScenarioCountFromFile, -} from "./suite-summary.js"; +import { readQaSuiteFailedOrSkippedScenarioCountFromFile } from "./suite-summary.js"; import { buildTokenEfficiencyReport, renderTokenEfficiencyMarkdownReport, @@ -648,22 +645,20 @@ export async function runQaProfileCommand(opts: QaProfileCommandOptions) { (opts.scenarioIds ?? []).map((scenarioId) => scenarioId.trim()).filter(Boolean), ); const taxonomyScenarios = membership.selectedScenarios; - if (requestedScenarioIds.length > 0 && taxonomyScenarios.length === 0) { - throw new Error( - `qa run did not find taxonomy scenarios for ${formatQaRunProfileFilterList(opts)} --scenario ${requestedScenarioIds.join(",")}.`, - ); - } const missingScenarioIds = membership.excludedScenarioIds; - if (missingScenarioIds.length > 0) { - throw new Error( - `qa run did not find taxonomy scenarios for ${formatQaRunProfileFilterList(opts)} --scenario ${missingScenarioIds.join(",")}.`, - ); - } const providerMode = opts.providerMode ?? defaultQaRunProfileProviderMode(profile); const normalizedProviderMode = normalizeQaProviderMode(providerMode); const primaryModel = opts.primaryModel?.trim() || defaultQaModelForMode(normalizedProviderMode); + const missingScenarioIdSet = new Set(missingScenarioIds); + const executionScenarios = + missingScenarioIds.length > 0 + ? [ + ...taxonomyScenarios, + ...scenarioPack.scenarios.filter((scenario) => missingScenarioIdSet.has(scenario.id)), + ] + : taxonomyScenarios; const executionSelection = resolveQaRunProfileExecutionSelection({ - scenarios: taxonomyScenarios, + scenarios: executionScenarios, providerMode: normalizedProviderMode, primaryModel, channelDriver: profileReport.channelDriver, @@ -680,6 +675,16 @@ export async function runQaProfileCommand(opts: QaProfileCommandOptions) { `qa run --qa-profile ${profile} cannot run explicitly selected scenario(s): ${exclusions}.`, ); } + if (requestedScenarioIds.length > 0 && taxonomyScenarios.length === 0) { + throw new Error( + `qa run did not find taxonomy scenarios for ${formatQaRunProfileFilterList(opts)} --scenario ${requestedScenarioIds.join(",")}.`, + ); + } + if (missingScenarioIds.length > 0) { + throw new Error( + `qa run did not find taxonomy scenarios for ${formatQaRunProfileFilterList(opts)} --scenario ${missingScenarioIds.join(",")}.`, + ); + } const scenarios = executionSelection.selectedScenarios; if (scenarios.length === 0) { throw new Error( @@ -1045,17 +1050,16 @@ export async function runQaSuiteCommand(opts: QaSuiteCommandOptions) { process.stdout.write(`QA suite evidence: ${result.evidencePath}\n`); process.stdout.write(`QA suite summary: ${result.summaryPath}\n`); if (!allowFailures) { - const optionalScenarioNames = resolveQaReportOnlyOptionalScenarioNames({ - scenarioIds, - explicitScenarioSelection: opts.explicitScenarioSelection, - }); - if ( - result.scenarios.some( - (scenario) => - scenario.status !== "pass" && - !isQaSuiteReportOnlyOptionalScenario(scenario, optionalScenarioNames), - ) - ) { + const blockingScenarioCount = await readQaSuiteFailedOrSkippedScenarioCountFromFile( + result.summaryPath, + { + optionalScenarioNames: resolveQaReportOnlyOptionalScenarioNames({ + scenarioIds, + explicitScenarioSelection: opts.explicitScenarioSelection, + }), + }, + ); + if (blockingScenarioCount > 0) { process.exitCode = 1; } } diff --git a/extensions/qa-lab/src/live-transports/telegram/cli.runtime.test.ts b/extensions/qa-lab/src/live-transports/telegram/cli.runtime.test.ts new file mode 100644 index 000000000000..4d8f1fbed097 --- /dev/null +++ b/extensions/qa-lab/src/live-transports/telegram/cli.runtime.test.ts @@ -0,0 +1,114 @@ +import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import path from "node:path"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +const mocks = vi.hoisted(() => ({ + createTelegramQaTransportAdapter: vi.fn(), + printLiveTransportQaArtifacts: vi.fn(), + resolveTelegramQaRunOptions: vi.fn( + (options: { allowFailures?: boolean; providerMode?: string; repoRoot: string }) => ({ + ...options, + allowFailures: options.allowFailures ?? false, + listScenarios: false, + }), + ), + resolveTelegramQaScenarioIds: vi.fn(), + runQaFlowSuiteFromRuntime: vi.fn(), +})); + +vi.mock("../../suite-launch.runtime.js", () => ({ + runQaFlowSuiteFromRuntime: mocks.runQaFlowSuiteFromRuntime, +})); + +vi.mock("../shared/live-artifacts.js", () => ({ + printLiveTransportQaArtifacts: mocks.printLiveTransportQaArtifacts, +})); + +vi.mock("./adapter.runtime.js", () => ({ + createTelegramQaTransportAdapter: mocks.createTelegramQaTransportAdapter, +})); + +vi.mock("./run-options.runtime.js", () => ({ + resolveTelegramQaRunOptions: mocks.resolveTelegramQaRunOptions, +})); + +vi.mock("./scenario-selection.js", () => ({ + listTelegramQaScenarios: vi.fn(), + resolveTelegramQaScenarioIds: mocks.resolveTelegramQaScenarioIds, +})); + +import { runQaTelegramSuite } from "./cli.runtime.js"; + +describe("Telegram live QA scenario gate", () => { + let previousExitCode: typeof process.exitCode; + let tempRoot: string; + let summaryPath: string; + + function writeSummary(status: string) { + writeFileSync( + summaryPath, + JSON.stringify({ + counts: { + failed: status === "fail" ? 1 : 0, + skipped: status === "skip" || status === "skipped" ? 1 : 0, + }, + scenarios: [{ name: "channel-canary", status }], + }), + "utf8", + ); + } + + beforeEach(() => { + previousExitCode = process.exitCode; + process.exitCode = undefined; + vi.clearAllMocks(); + tempRoot = mkdtempSync(path.join(tmpdir(), "openclaw-qa-telegram-gate-")); + summaryPath = path.join(tempRoot, "qa-suite-summary.json"); + mocks.resolveTelegramQaScenarioIds.mockReturnValue(["channel-canary"]); + mocks.runQaFlowSuiteFromRuntime.mockResolvedValue({ + reportPath: ".artifacts/qa-e2e/telegram/qa-suite-report.md", + summaryPath, + }); + }); + + afterEach(() => { + process.exitCode = previousExitCode; + rmSync(tempRoot, { force: true, recursive: true }); + }); + + it.each(["fail", "skip", "skipped", "timeout"])( + "fails the live Telegram lane on %s scenarios", + async (status) => { + writeSummary(status); + + await runQaTelegramSuite({ + repoRoot: "/repo", + providerMode: "mock-openai", + }); + + expect(process.exitCode).toBe(1); + }, + ); + + it("leaves the exit code clear when every Telegram scenario passes", async () => { + writeSummary("pass"); + + await runQaTelegramSuite({ + repoRoot: "/repo", + providerMode: "mock-openai", + }); + + expect(process.exitCode).toBeUndefined(); + }); + + it("does not read the summary when failures are explicitly allowed", async () => { + await runQaTelegramSuite({ + repoRoot: "/repo", + providerMode: "mock-openai", + allowFailures: true, + }); + + expect(process.exitCode).toBeUndefined(); + }); +}); diff --git a/extensions/qa-lab/src/live-transports/telegram/cli.runtime.ts b/extensions/qa-lab/src/live-transports/telegram/cli.runtime.ts index 7d07881254c3..46fcdd2c2e07 100644 --- a/extensions/qa-lab/src/live-transports/telegram/cli.runtime.ts +++ b/extensions/qa-lab/src/live-transports/telegram/cli.runtime.ts @@ -6,7 +6,7 @@ import type { LiveTransportQaCommandOptions } from "openclaw/plugin-sdk/qa-runti import type { QaGatewayChildCommand } from "../../gateway-child.js"; import { runQaFlowSuiteFromRuntime } from "../../suite-launch.runtime.js"; import type { QaSuiteRoundTripProbe } from "../../suite-round-trip.js"; -import { readQaSuiteFailedScenarioCountFromFile } from "../../suite-summary.js"; +import { readQaSuiteFailedOrSkippedScenarioCountFromFile } from "../../suite-summary.js"; // Qa Lab plugin module implements cli behavior. import { printLiveTransportQaArtifacts } from "../shared/live-artifacts.js"; import { createTelegramQaTransportAdapter } from "./adapter.runtime.js"; @@ -196,8 +196,10 @@ export async function runQaTelegramSuite(opts: TelegramQaSuiteOptions) { summary: result.summaryPath, }); if (!runOptions.allowFailures) { - const failedScenarioCount = await readQaSuiteFailedScenarioCountFromFile(result.summaryPath); - if (failedScenarioCount > 0) { + const blockingScenarioCount = await readQaSuiteFailedOrSkippedScenarioCountFromFile( + result.summaryPath, + ); + if (blockingScenarioCount > 0) { process.exitCode = 1; } } diff --git a/extensions/qa-lab/src/providers/mock-openai/mock-openai-input.ts b/extensions/qa-lab/src/providers/mock-openai/mock-openai-input.ts index 699c7a9208ba..21b240fb4b68 100644 --- a/extensions/qa-lab/src/providers/mock-openai/mock-openai-input.ts +++ b/extensions/qa-lab/src/providers/mock-openai/mock-openai-input.ts @@ -276,27 +276,6 @@ export function extractAllUserTexts(input: ResponsesInputItem[]) { return texts; } -export function extractSystemInputText(input: ResponsesInputItem[]) { - const texts: string[] = []; - for (const item of input) { - if (item.role !== "system") { - continue; - } - if (typeof item.content === "string" && item.content.trim()) { - texts.push(item.content.trim()); - continue; - } - if (!Array.isArray(item.content)) { - continue; - } - const text = extractInputText(item.content); - if (text) { - texts.push(text); - } - } - return texts.join("\n"); -} - export function extractAllInputTexts(input: ResponsesInputItem[]) { const texts: string[] = []; for (const item of input) { diff --git a/extensions/qa-lab/src/providers/mock-openai/server.test.ts b/extensions/qa-lab/src/providers/mock-openai/server.test.ts index 8aa6205e8470..0e2ca34978d4 100644 --- a/extensions/qa-lab/src/providers/mock-openai/server.test.ts +++ b/extensions/qa-lab/src/providers/mock-openai/server.test.ts @@ -2473,7 +2473,7 @@ describe("qa mock openai server", () => { expect(structuredThreadMemorySummary.status).toBe(200); expect(JSON.stringify(await structuredThreadMemorySummary.json())).toContain("ORBIT-22"); - const systemFallbackThreadMemorySummary = await postResponses(server, { + const unavailableThreadMemorySummary = await postResponses(server, { stream: false, input: [ { @@ -2494,8 +2494,31 @@ describe("qa mock openai server", () => { }, ], }); - expect(systemFallbackThreadMemorySummary.status).toBe(200); - expect(JSON.stringify(await systemFallbackThreadMemorySummary.json())).toContain("ORBIT-22"); + expect(unavailableThreadMemorySummary.status).toBe(200); + const unavailableThreadMemoryText = JSON.stringify(await unavailableThreadMemorySummary.json()); + expect(unavailableThreadMemoryText).toContain("NONE"); + expect(unavailableThreadMemoryText).not.toContain("ORBIT-22"); + + const emptyThreadMemorySummary = await postResponses(server, { + stream: false, + input: [ + { + role: "system", + content: "## /workspace/MEMORY.md\nThread-hidden codename: ORBIT-22.", + }, + makeUserInput( + "@openclaw Thread memory check: what is the hidden thread codename stored only in memory? Use memory tools first and reply only in this thread.", + ), + { + type: "function_call_output", + output: JSON.stringify({ results: [] }), + }, + ], + }); + expect(emptyThreadMemorySummary.status).toBe(200); + const emptyThreadMemoryText = JSON.stringify(await emptyThreadMemorySummary.json()); + expect(emptyThreadMemoryText).toContain("NONE"); + expect(emptyThreadMemoryText).not.toContain("ORBIT-22"); const memoryFollowup = await postResponses(server, { stream: true, @@ -2517,6 +2540,7 @@ describe("qa mock openai server", () => { path: "sessions/qa-session-memory-ranking.jsonl", startLine: 2, endLine: 3, + snippet: "Project Nebula current codename: ORBIT-10.", }, ], }), @@ -2548,11 +2572,13 @@ describe("qa mock openai server", () => { path: "MEMORY.md", startLine: 1, endLine: 2, + snippet: "Project Nebula stale codename: ORBIT-9.", }, { path: "sessions/qa-session-memory-ranking.jsonl", startLine: 2, endLine: 3, + snippet: "Project Nebula current codename: ORBIT-10.", }, ], }), @@ -2564,6 +2590,81 @@ describe("qa mock openai server", () => { "Protocol note: I checked memory and the current Project Nebula codename is ORBIT-10.", ); + const pathOnlySessionMemory = await postResponses(server, { + stream: true, + input: [ + makeUserInput( + "Session memory ranking check: what is the current Project Nebula codename? Use memory tools first.", + ), + { + type: "function_call_output", + output: JSON.stringify({ + results: [ + { + path: "sessions/qa-session-memory-ranking.jsonl", + startLine: 2, + endLine: 3, + }, + ], + }), + }, + ], + }); + expect(pathOnlySessionMemory.status).toBe(200); + const pathOnlySessionMemoryText = await pathOnlySessionMemory.text(); + expect(pathOnlySessionMemoryText).toContain('"name":"memory_get"'); + expect(pathOnlySessionMemoryText).not.toContain("codename is ORBIT-10"); + + const unavailableSessionMemory = await postResponses(server, { + stream: true, + input: [ + makeUserInput( + "Session memory ranking check: what is the current Project Nebula codename? Use memory tools first.", + ), + { + type: "function_call_output", + output: JSON.stringify({ + results: [ + { + path: "sessions/qa-session-memory-ranking.jsonl", + snippet: "Project Nebula current codename: ORBIT-10.", + }, + ], + unavailable: true, + error: "database is not open", + }), + }, + ], + }); + expect(unavailableSessionMemory.status).toBe(200); + const unavailableSessionMemoryText = await unavailableSessionMemory.text(); + expect(unavailableSessionMemoryText).toContain("NONE"); + expect(unavailableSessionMemoryText).not.toContain("codename is ORBIT-10"); + + const differentlyRankedSessionMemory = await postResponses(server, { + stream: true, + input: [ + makeUserInput( + "Session memory ranking check: what is the current Project Nebula codename? Use memory tools first.", + ), + { + type: "function_call_output", + output: JSON.stringify({ + results: [ + { + path: "sessions/qa-session-memory-ranking.jsonl", + snippet: "Project Nebula current codename: ORBIT-9.", + }, + ], + }), + }, + ], + }); + expect(differentlyRankedSessionMemory.status).toBe(200); + const differentlyRankedSessionMemoryText = await differentlyRankedSessionMemory.text(); + expect(differentlyRankedSessionMemoryText).toContain("codename is ORBIT-9"); + expect(differentlyRankedSessionMemoryText).not.toContain("codename is ORBIT-10"); + const activeMemorySearch = await postResponses(server, { stream: true, input: [ diff --git a/extensions/qa-lab/src/providers/mock-openai/server.ts b/extensions/qa-lab/src/providers/mock-openai/server.ts index 69e3156ffe9f..6010ea45bfad 100644 --- a/extensions/qa-lab/src/providers/mock-openai/server.ts +++ b/extensions/qa-lab/src/providers/mock-openai/server.ts @@ -127,7 +127,6 @@ import { extractAllToolOutputText, extractUserTextAfterLatestToolOutput, extractAllUserTexts, - extractSystemInputText, extractAllInputTexts, extractInstructionsText, extractAllRequestTexts, @@ -175,6 +174,10 @@ async function buildResponsesPayload( ? extractLatestToolOutput(input) : ""); const toolJson = parseToolOutputJson(scenarioToolOutput); + const memoryToolUnavailable = + toolJson?.unavailable === true || + toolJson?.disabled === true || + (typeof toolJson?.error === "string" && toolJson.error.trim().length > 0); const promptExactReplyDirective = extractExactReplyDirective(prompt); const promptExactMarkerDirective = extractExactMarkerDirective(prompt); const allUserText = extractAllUserTexts(input).join("\n"); @@ -1052,19 +1055,44 @@ async function buildResponsesPayload( corpus: "sessions", }); } + if (memoryToolUnavailable) { + return buildAssistantEvents("NONE"); + } const results = Array.isArray(toolJson?.results) ? (toolJson.results as Array>) : []; const preferredSessionResult = results.find((result) => { const resultPath = typeof result.path === "string" ? result.path : undefined; - return result.source === "sessions" || resultPath?.startsWith("sessions/"); + if (result.source !== "sessions" && !resultPath?.startsWith("sessions/")) { + return false; + } + const memoryText = + typeof result.snippet === "string" + ? result.snippet + : typeof result.text === "string" + ? result.text + : ""; + return extractOrbitCode(memoryText) !== null; }); - if (preferredSessionResult) { + const sessionMemoryText = + typeof preferredSessionResult?.snippet === "string" + ? preferredSessionResult.snippet + : typeof preferredSessionResult?.text === "string" + ? preferredSessionResult.text + : ""; + const retrievedOrbitCode = + extractOrbitCode(sessionMemoryText) ?? + (typeof toolJson?.text === "string" ? extractOrbitCode(toolJson.text) : null); + if (retrievedOrbitCode) { return buildAssistantEvents( - "Protocol note: I checked memory and the current Project Nebula codename is ORBIT-10.", + `Protocol note: I checked memory and the current Project Nebula codename is ${retrievedOrbitCode}.`, ); } - const first = results[0]; + const first = + results.find((result) => { + const resultPath = typeof result.path === "string" ? result.path : undefined; + return result.source === "sessions" || resultPath?.startsWith("sessions/"); + }) ?? results[0]; if ( typeof first?.path === "string" && (typeof first.startLine === "number" || typeof first.endLine === "number") @@ -1081,6 +1109,7 @@ async function buildResponsesPayload( lines: 4, }); } + return buildAssistantEvents("NONE"); } if (/thread memory check/i.test(allInputText)) { if (!scenarioToolOutput) { @@ -1089,10 +1118,10 @@ async function buildResponsesPayload( maxResults: 3, }); } - const transcriptOrbitCode = - extractOrbitCode(scenarioToolOutput) ?? - extractOrbitCode(extractUserTextAfterLatestToolOutput(input)) ?? - extractOrbitCode(extractSystemInputText(input)); + if (memoryToolUnavailable) { + return buildAssistantEvents("NONE"); + } + const transcriptOrbitCode = extractOrbitCode(scenarioToolOutput); if (transcriptOrbitCode) { return buildAssistantEvents( `Protocol note: I checked memory in-thread and the hidden thread codename is ${transcriptOrbitCode}.`, @@ -1118,6 +1147,7 @@ async function buildResponsesPayload( lines: 4, }); } + return buildAssistantEvents("NONE"); } if ( QA_IMAGE_GENERATION_PROMPT_RE.test(allInputText) && diff --git a/extensions/qa-lab/src/qa-gateway-config.test.ts b/extensions/qa-lab/src/qa-gateway-config.test.ts index 888a0bef16f0..4b0cd1d97f64 100644 --- a/extensions/qa-lab/src/qa-gateway-config.test.ts +++ b/extensions/qa-lab/src/qa-gateway-config.test.ts @@ -1,4 +1,5 @@ // Qa Lab tests cover qa gateway config plugin behavior. +import { OPENCLAW_VERSION } from "openclaw/plugin-sdk/agent-harness-runtime"; import { describe, expect, it } from "vitest"; import { buildQaGatewayConfig, @@ -59,6 +60,21 @@ function expectQaLabPluginEnabled(cfg: ReturnType) } describe("buildQaGatewayConfig", () => { + it("stamps fresh QA configs with the current OpenClaw version", () => { + const cfg = buildQaGatewayConfig({ + bind: "loopback", + gatewayPort: 18789, + gatewayToken: "token", + workspaceDir: "/tmp/qa-workspace", + ...createQaChannelTransportParams(), + }); + + expect(cfg.meta).toEqual({ lastTouchedVersion: OPENCLAW_VERSION }); + expect(cfg.plugins?.allow).toEqual(["acpx", "memory-core", "qa-lab", "qa-channel"]); + expect(getPrimaryModel(cfg.agents?.defaults?.model)).toBe("mock-openai/gpt-5.6-luna"); + expect(cfg.channels?.["qa-channel"]?.baseUrl).toBe("http://127.0.0.1:43124"); + }); + it("keeps mock-openai as the default provider lane", () => { const cfg = buildQaGatewayConfig({ bind: "loopback", diff --git a/extensions/qa-lab/src/qa-gateway-config.ts b/extensions/qa-lab/src/qa-gateway-config.ts index ad1d9ff685c8..616f7aec229d 100644 --- a/extensions/qa-lab/src/qa-gateway-config.ts +++ b/extensions/qa-lab/src/qa-gateway-config.ts @@ -1,4 +1,5 @@ // Qa Lab helper module supports qa gateway config behavior. +import { OPENCLAW_VERSION } from "openclaw/plugin-sdk/agent-harness-runtime"; import type { OpenClawConfig } from "openclaw/plugin-sdk/config-contracts"; import type { ModelProviderConfig } from "openclaw/plugin-sdk/provider-model-shared"; import { uniqueStrings } from "openclaw/plugin-sdk/string-coerce-runtime"; @@ -208,6 +209,9 @@ export function buildQaGatewayConfig(params: { : {}; return { + meta: { + lastTouchedVersion: OPENCLAW_VERSION, + }, memory: { backend: "builtin", search: { diff --git a/extensions/qa-lab/src/scenario-catalog-browser-coverage.test.ts b/extensions/qa-lab/src/scenario-catalog-browser-coverage.test.ts new file mode 100644 index 000000000000..459e5a282b09 --- /dev/null +++ b/extensions/qa-lab/src/scenario-catalog-browser-coverage.test.ts @@ -0,0 +1,35 @@ +import { describe, expect, it } from "vitest"; +import { readQaScenarioById, readQaScenarioPack } from "./scenario-catalog.js"; + +describe("QA Control UI browser scenario catalog", () => { + const coverageId = "control-ui.gateway-hosted-ui-control"; + + it("loads the screenshot and unread-body cancellation as a native Playwright scenario", () => { + const scenario = readQaScenarioById("control-ui-browser-screenshot-body-cancel"); + + expect(scenario.execution.kind).toBe("playwright"); + if (scenario.execution.kind !== "playwright") { + throw new Error(`expected Playwright scenario, got ${scenario.execution.kind}`); + } + expect(scenario.execution.path).toBe("ui/src/e2e/browser-screenshot-body-cancel.e2e.test.ts"); + expect(scenario.execution.testNamePattern).toBe( + "keeps the status error visible and cancels the unread media body", + ); + expect(scenario.execution.flow).toBeUndefined(); + expect(scenario.coverage?.primary).not.toContain(coverageId); + expect(scenario.coverage?.secondary).toContain(coverageId); + }); + + it("reserves primary hosted Control UI coverage for the real Gateway flow", () => { + const primaryOwnerIds = readQaScenarioPack() + .scenarios.filter((scenario) => scenario.coverage?.primary.includes(coverageId)) + .map((scenario) => scenario.id); + + expect(primaryOwnerIds).toStrictEqual(["control-ui-qa-channel-image-roundtrip"]); + + const hostedScenario = readQaScenarioById("control-ui-qa-channel-image-roundtrip"); + + expect(hostedScenario.execution).toMatchObject({ kind: "flow", channel: "qa-channel" }); + expect(hostedScenario.coverage?.primary).toContain(coverageId); + }); +}); diff --git a/extensions/qa-lab/src/scenario-catalog-channels.test.ts b/extensions/qa-lab/src/scenario-catalog-channels.test.ts index 0ad1efbf36c2..743967809edc 100644 --- a/extensions/qa-lab/src/scenario-catalog-channels.test.ts +++ b/extensions/qa-lab/src/scenario-catalog-channels.test.ts @@ -74,19 +74,21 @@ describe("qa scenario catalog channel contracts", () => { it("uses durable subagent completion evidence before accepting fanout", () => { const scenario = requireFlowScenario(readQaScenarioById("subagent-fanout-synthesis")); const flow = JSON.stringify(scenario.execution.flow); - const completionWait = flow.indexOf('"items":{"expr":"config.expectedChildCompletionMarkers"}'); + const completionWait = flow.indexOf('"saveAs":"completedFanout"'); const storeReads = [...flow.matchAll(/readRawQaSessionStore/gu)].map((match) => match.index); expect(flow).toContain("readSessionTranscriptSummary(env, sessionKey)"); expect(flow).not.toContain("waitForAgentHistoryReply"); - expect( - flow.split("String(candidate.text ?? '').trim() === childCompletionMarker").length - 1, - ).toBe(1); + expect(flow).not.toContain('"call":"waitForOutboundMessage"'); + expect(flow).not.toContain("childCompletionMarker"); + expect(flow).toContain("entry.spawnedBy === sessionKey"); expect(flow).toContain( - "timeoutSawAlpha && timeoutSawBeta && timeoutAlphaOk && timeoutBetaOk && timeoutSpawnRequests.length >= 2", + "timeoutSawAlpha && timeoutSawBeta && timeoutAlphaOk && timeoutBetaOk && (!env.mock || timeoutSpawnRequests.length >= 2)", ); expect(flow).toContain("Boolean(env.mock) ? config.expectedChildCompletionMarkers[0] : 'ok'"); expect(flow).toContain('saveAs":"timeoutEvidence'); + expect(flow).toContain('saveAs":"recoveredParentTranscript'); + expect(flow).not.toContain('"value":"subagent-1: ok\\nsubagent-2: ok"'); expect(flow).toContain("Promise.all([readSessionTranscriptSummary"); expect(completionWait).toBeGreaterThan(-1); expect(storeReads).toHaveLength(2); diff --git a/extensions/qa-lab/src/suite-launch.runtime.test.ts b/extensions/qa-lab/src/suite-launch.runtime.test.ts index 21b40782e7c4..87a06bbc5ae9 100644 --- a/extensions/qa-lab/src/suite-launch.runtime.test.ts +++ b/extensions/qa-lab/src/suite-launch.runtime.test.ts @@ -462,7 +462,7 @@ describe("qa suite runtime launcher", () => { ); }); - it("does not retry mixed-channel partitions for generic timeout wording", async () => { + it("records generic partition failures without retrying or discarding sibling artifacts", async () => { const repoRoot = await makeTempRepo("qa-suite-partition-generic-timeout-"); const attempts = mockFlowPartitionFailures( new Map([ @@ -473,20 +473,56 @@ describe("qa suite runtime launcher", () => { ]), ); - await expect( - runQaSuite({ - repoRoot, - outputDir: ".artifacts/qa-e2e/partition-generic-timeout", - providerMode: "mock-openai", - channelDriver: "live", - adapterFactories: [{ id: "portable-driver", matches: () => true, create: vi.fn() }], - concurrency: 2, - scenarioIds: ["telegram-help-command", "whatsapp-status-command"], - }), - ).rejects.toThrow("approval-turn timed out waiting for post-approval read"); + const result = await runQaSuite({ + repoRoot, + outputDir: ".artifacts/qa-e2e/partition-generic-timeout", + providerMode: "mock-openai", + channelDriver: "live", + adapterFactories: [{ id: "portable-driver", matches: () => true, create: vi.fn() }], + concurrency: 2, + scenarioIds: ["telegram-help-command", "whatsapp-status-command"], + }); expect(attempts.get("telegram-help-command")).toBe(1); expect(attempts.get("whatsapp-status-command")).toBe(1); + expect(result.executionKind).toBe("suite"); + if (result.executionKind !== "suite") { + throw new Error("expected unified suite result"); + } + expect(result.result.scenarios).toEqual( + expect.arrayContaining([ + expect.objectContaining({ name: "telegram-help-command", status: "pass" }), + expect.objectContaining({ + status: "fail", + details: "suite partition failed: approval-turn timed out waiting for post-approval read", + }), + ]), + ); + const summary = JSON.parse(await fs.readFile(result.result.summaryPath, "utf8")) as { + counts: { failed: number; passed: number; total: number }; + }; + expect(summary.counts).toMatchObject({ total: 2, passed: 1, failed: 1 }); + const evidence = JSON.parse(await fs.readFile(result.result.evidencePath, "utf8")) as { + entries: Array<{ + test: { id: string }; + result: { status: string; failure?: { reason: string } }; + }>; + }; + expect(evidence.entries).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + test: expect.objectContaining({ id: "whatsapp-status-command" }), + result: expect.objectContaining({ + status: "fail", + failure: { + reason: + "suite partition failed: approval-turn timed out waiting for post-approval read", + }, + }), + }), + ]), + ); + await expect(fs.access(result.result.reportPath)).resolves.toBeUndefined(); }); it("preserves completed partitions when a retryable channel fails twice", async () => { @@ -503,20 +539,177 @@ describe("qa suite runtime launcher", () => { ]), ); - await expect( - runQaSuite({ - repoRoot, - outputDir: ".artifacts/qa-e2e/partition-retry-exhausted", - providerMode: "mock-openai", - channelDriver: "live", - adapterFactories: [{ id: "portable-driver", matches: () => true, create: vi.fn() }], - concurrency: 2, - scenarioIds: ["telegram-help-command", "whatsapp-status-command"], - }), - ).rejects.toThrow("WhatsApp readiness timed out again"); + const result = await runQaSuite({ + repoRoot, + outputDir: ".artifacts/qa-e2e/partition-retry-exhausted", + providerMode: "mock-openai", + channelDriver: "live", + adapterFactories: [{ id: "portable-driver", matches: () => true, create: vi.fn() }], + concurrency: 2, + scenarioIds: ["telegram-help-command", "whatsapp-status-command"], + }); expect(attempts.get("telegram-help-command")).toBe(1); expect(attempts.get("whatsapp-status-command")).toBe(2); + expect(result.executionKind).toBe("suite"); + if (result.executionKind !== "suite") { + throw new Error("expected unified suite result"); + } + expect(result.result.scenarios).toEqual( + expect.arrayContaining([ + expect.objectContaining({ name: "telegram-help-command", status: "pass" }), + expect.objectContaining({ + status: "fail", + details: "suite partition failed: WhatsApp readiness timed out again", + }), + ]), + ); + const summary = JSON.parse(await fs.readFile(result.result.summaryPath, "utf8")) as { + counts: { failed: number; passed: number; total: number }; + }; + expect(summary.counts).toMatchObject({ total: 2, passed: 1, failed: 1 }); + const evidence = JSON.parse(await fs.readFile(result.result.evidencePath, "utf8")) as { + entries: Array<{ + test: { id: string }; + result: { status: string; failure?: { reason: string } }; + }>; + }; + expect(evidence.entries).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + test: expect.objectContaining({ id: "whatsapp-status-command" }), + result: expect.objectContaining({ + status: "fail", + failure: { reason: "suite partition failed: WhatsApp readiness timed out again" }, + }), + }), + ]), + ); + await expect(fs.access(result.result.reportPath)).resolves.toBeUndefined(); + }); + + it("records an exhausted fail-fast partition without starting later partitions", async () => { + const repoRoot = await makeTempRepo("qa-suite-fail-fast-retry-exhausted-"); + const attempts = mockFlowPartitionFailures( + new Map([ + [ + "whatsapp-status-command", + [ + new QaSuiteInfraError("transport_ready_timeout", "WhatsApp readiness timed out"), + new QaSuiteInfraError("transport_ready_timeout", "WhatsApp readiness timed out again"), + ], + ], + ]), + ); + + const result = await runQaSuite({ + repoRoot, + outputDir: ".artifacts/qa-e2e/fail-fast-retry-exhausted", + providerMode: "mock-openai", + channelDriver: "live", + adapterFactories: [{ id: "portable-driver", matches: () => true, create: vi.fn() }], + concurrency: 2, + failFast: true, + scenarioIds: [ + "whatsapp-status-command", + "telegram-help-command", + "control-ui-chat-flow-playwright", + ], + }); + + expect(attempts.get("whatsapp-status-command")).toBe(2); + expect(attempts.has("telegram-help-command")).toBe(false); + expect(runQaTestFileScenarios).not.toHaveBeenCalled(); + expect(result.executionKind).toBe("suite"); + if (result.executionKind !== "suite") { + throw new Error("expected unified suite result"); + } + expect(result.result.scenarios).toMatchObject([ + { + status: "fail", + details: "suite partition failed: WhatsApp readiness timed out again", + }, + ]); + const summary = JSON.parse(await fs.readFile(result.result.summaryPath, "utf8")) as { + counts: { failed: number; total: number }; + }; + expect(summary.counts).toMatchObject({ total: 1, failed: 1 }); + const evidence = JSON.parse(await fs.readFile(result.result.evidencePath, "utf8")) as { + entries: Array<{ test: { id: string }; result: { status: string } }>; + }; + expect(evidence.entries).toMatchObject([ + { test: { id: "whatsapp-status-command" }, result: { status: "fail" } }, + ]); + await expect(fs.access(result.result.reportPath)).resolves.toBeUndefined(); + }); + + it("attributes an exhausted fail-fast retry to the later scenario that actually started", async () => { + const repoRoot = await makeTempRepo("qa-suite-fail-fast-later-partition-failure-"); + const attempts = mockFlowPartitionFailures( + new Map([ + [ + "thread-follow-up", + [ + new QaSuiteInfraError("transport_ready_timeout", "second scenario readiness timed out"), + new QaSuiteInfraError( + "transport_ready_timeout", + "second scenario readiness timed out again", + ), + ], + ], + ]), + ); + + const result = await runQaSuite({ + repoRoot, + outputDir: ".artifacts/qa-e2e/fail-fast-later-partition-failure", + providerMode: "mock-openai", + concurrency: 2, + failFast: true, + scenarioIds: ["dm-chat-baseline", "thread-follow-up", "control-ui-chat-flow-playwright"], + }); + + expect(Object.fromEntries(attempts)).toEqual({ + "dm-chat-baseline": 1, + "thread-follow-up": 2, + }); + expect(runQaFlowSuite.mock.calls.map(([params]) => params?.scenarioIds)).toEqual([ + ["dm-chat-baseline"], + ["thread-follow-up"], + ["thread-follow-up"], + ]); + expect(runQaTestFileScenarios).not.toHaveBeenCalled(); + expect(result.executionKind).toBe("suite"); + if (result.executionKind !== "suite") { + throw new Error("expected unified suite result"); + } + expect(result.result.scenarios).toMatchObject([ + { name: "dm-chat-baseline", status: "pass" }, + { + status: "fail", + details: "suite partition failed: second scenario readiness timed out again", + }, + ]); + const summary = JSON.parse(await fs.readFile(result.result.summaryPath, "utf8")) as { + counts: { failed: number; passed: number; total: number }; + }; + expect(summary.counts).toMatchObject({ total: 2, passed: 1, failed: 1 }); + const evidence = JSON.parse(await fs.readFile(result.result.evidencePath, "utf8")) as { + entries: Array<{ + test: { id: string }; + result: { status: string; failure?: { reason: string } }; + }>; + }; + expect(evidence.entries).toMatchObject([ + { + test: { id: "thread-follow-up" }, + result: { + status: "fail", + failure: { reason: "suite partition failed: second scenario readiness timed out again" }, + }, + }, + ]); + await expect(fs.access(result.result.reportPath)).resolves.toBeUndefined(); }); it("runs distinct pluggable-driver channels within the global concurrency budget", async () => { @@ -1526,7 +1719,7 @@ describe("qa suite runtime launcher", () => { if (!defaultTestFileImplementation) { throw new Error("expected default QA test-file scenario mock implementation"); } - runQaTestFileScenarios.mockImplementationOnce(async (params) => { + runQaTestFileScenarios.mockImplementation(async (params) => { const result = await defaultTestFileImplementation(params); return { ...result, @@ -1542,9 +1735,8 @@ describe("qa suite runtime launcher", () => { result: { status: "pass" as const }, })), }, - results: [result.results[0], result.results[2]].filter( - (scenario): scenario is (typeof result.results)[number] => Boolean(scenario), - ), + results: + params.scenarios[0]?.id === "auth-profile-doctor-migration-safety" ? [] : result.results, }; }); @@ -1567,7 +1759,7 @@ describe("qa suite runtime launcher", () => { throw new Error("expected unified suite result"); } expect(runQaFlowSuite).toHaveBeenCalledTimes(1); - expect(runQaTestFileScenarios).toHaveBeenCalledTimes(1); + expect(runQaTestFileScenarios).toHaveBeenCalledTimes(2); expect(result.result.scenarios).toMatchObject([ { name: "dm-chat-baseline", status: "pass" }, { name: "Control UI assistant media ticket evidence", status: "pass" }, @@ -1895,7 +2087,7 @@ describe("qa suite runtime launcher", () => { expect(runQaFlowSuite).toHaveBeenCalledTimes(2); }); - it("waits for already-started partitions before rejecting a unified suite", async () => { + it("waits for already-started partitions before recording a unified failure", async () => { const repoRoot = await makeTempRepo("qa-suite-reject-settle-"); let releaseTestFile!: () => void; let markTestFileStarted!: () => void; @@ -1941,18 +2133,37 @@ describe("qa suite runtime launcher", () => { concurrency: 2, scenarioIds: ["channel-chat-baseline", "control-ui-chat-flow-playwright"], }); - let rejected = false; - void runPromise.catch(() => { - rejected = true; + let completed = false; + void runPromise.then(() => { + completed = true; }); await testFileStarted; await Promise.resolve(); - expect(rejected).toBe(false); + expect(completed).toBe(false); releaseTestFile(); - await expect(runPromise).rejects.toThrow("flow partition failed"); - expect(rejected).toBe(true); + const result = await runPromise; + expect(completed).toBe(true); + expect(result.executionKind).toBe("suite"); + if (result.executionKind !== "suite") { + throw new Error("expected unified suite result"); + } + expect(result.result.scenarios).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + status: "fail", + details: expect.stringContaining("suite partition failed: flow partition failed"), + }), + expect.objectContaining({ status: "pass" }), + ]), + ); + const summary = JSON.parse(await fs.readFile(result.result.summaryPath, "utf8")) as { + counts: { failed: number; passed: number; total: number }; + }; + expect(summary.counts).toMatchObject({ total: 2, passed: 1, failed: 1 }); + await expect(fs.access(result.result.evidencePath)).resolves.toBeUndefined(); + await expect(fs.access(result.result.reportPath)).resolves.toBeUndefined(); }); it("reuses unavailable channel credential evidence across serial partitions", async () => { diff --git a/extensions/qa-lab/src/suite-launch.runtime.ts b/extensions/qa-lab/src/suite-launch.runtime.ts index 7d299d26030b..ac704d613280 100644 --- a/extensions/qa-lab/src/suite-launch.runtime.ts +++ b/extensions/qa-lab/src/suite-launch.runtime.ts @@ -106,6 +106,7 @@ type QaUnifiedPartitionResult = { type QaUnifiedPartitionTask = { exclusiveKey?: string; run: () => Promise; + scenarios: readonly QaSeedScenarioWithSource[]; weight: number; }; @@ -741,11 +742,17 @@ async function runUnifiedQaSuite(params: { ); // Isolated adapters may use the caller's full suite budget; every partition // still has weight one in the global scheduler below. - const sharedFlowPartitions = partitionSharedFlowScenarios( - sharedFlowScenarios, - usesContributedChannelDriver && !channelGroup.isolatesAdapterInstances ? 1 : concurrency, - channelGroup.isolatesAdapterInstances ? concurrency : MAX_SHARED_FLOW_PARTITIONS, - ); + // A rejected worker cannot return its completed prefix or active scenario. + // Single-scenario fail-fast tasks keep retries and failure evidence attributable. + const sharedFlowPartitions = failFast + ? sharedFlowScenarios.map((scenario) => [scenario]) + : partitionSharedFlowScenarios( + sharedFlowScenarios, + usesContributedChannelDriver && !channelGroup.isolatesAdapterInstances + ? 1 + : concurrency, + channelGroup.isolatesAdapterInstances ? concurrency : MAX_SHARED_FLOW_PARTITIONS, + ); // Channel-driver flow workers each launch a gateway plus transport harness. // Serializing their isolated workers keeps state-mutating smoke checks from // flaking under concurrent child gateways while preserving non-driver speed. @@ -845,6 +852,7 @@ async function runUnifiedQaSuite(params: { exclusiveKey: channelDriverFlowRequiresExclusiveWorkers ? `channel:${channelGroup.channel ?? channelGroup.channelId ?? "default"}` : undefined, + scenarios: partition.scenarios, weight: partition.concurrency, run: async () => { const unavailableDetails = channelGroup.channelId @@ -943,6 +951,7 @@ async function runUnifiedQaSuite(params: { scenariosByKind: ReadonlyMap, ) => ({ + scenarios: [...scenariosByKind.values()].flat(), weight: 1, run: async () => { const testFileEvidenceSummaries: QaEvidenceSummaryJson[] = []; @@ -1009,11 +1018,27 @@ async function runUnifiedQaSuite(params: { [...params.plan.testFileScenariosByKind].filter(([kind]) => kind !== "script"), ); if (concurrentTestFileScenariosByKind.size > 0) { - testFilePartitionTasks.push(createTestFilePartitionTask(concurrentTestFileScenariosByKind)); + if (failFast) { + for (const [kind, scenarios] of concurrentTestFileScenariosByKind) { + for (const scenario of scenarios) { + testFilePartitionTasks.push(createTestFilePartitionTask(new Map([[kind, [scenario]]]))); + } + } + } else { + testFilePartitionTasks.push(createTestFilePartitionTask(concurrentTestFileScenariosByKind)); + } } const scriptScenarios = params.plan.testFileScenariosByKind.get("script"); if (scriptScenarios?.length) { - scriptPartitionTasks.push(createTestFilePartitionTask(new Map([["script", scriptScenarios]]))); + if (failFast) { + for (const scenario of scriptScenarios) { + scriptPartitionTasks.push(createTestFilePartitionTask(new Map([["script", [scenario]]]))); + } + } else { + scriptPartitionTasks.push( + createTestFilePartitionTask(new Map([["script", scriptScenarios]])), + ); + } } const concurrentPartitionTasks = [ ...sharedFlowPartitionTasks, @@ -1103,12 +1128,56 @@ async function runUnifiedQaSuite(params: { ); return partition.startedScenarioIds.some((scenarioId) => !returnedScenarioIds.has(scenarioId)); }; + const capturePartitionFailure = ( + task: QaUnifiedPartitionTask, + error: unknown, + ): QaUnifiedPartitionResult => { + const scenarios = task.scenarios; + const details = `suite partition failed: ${formatErrorMessage(error)}`; + const scenarioResults = scenarios.map((scenario) => ({ + scenarioId: scenario.id, + result: { + name: scenario.title, + status: "fail" as const, + details, + steps: [{ name: "suite partition", status: "fail" as const, details }], + }, + })); + return { + evidenceSummaries: [ + buildQaSuiteEvidenceSummary({ + artifactPaths: [], + evidenceMode: params.runParams?.evidenceMode, + channelDriver: params.runParams?.channelDriver, + channelId: transportId, + env: process.env, + generatedAt: new Date().toISOString(), + primaryModel, + providerMode, + repoRoot, + scenarioDefinitions: scenarios, + scenarioResults: scenarioResults.map(({ result }) => result), + }), + ], + scenarioResults, + startedScenarioIds: scenarios.map((scenario) => scenario.id), + submittedScenarioIds: task.scenarios.map((scenario) => scenario.id), + }; + }; const runPartitionTasks = async (tasks: readonly QaUnifiedPartitionTask[], maxWeight: number) => { // Retry inside the scheduled task so its weight and exclusive key stay held; // one failed channel must not replay partitions that already completed. const retryingTasks = tasks.map((task) => ({ ...task, - run: async () => await runQaSuiteWithInfraRetry(task.run), + run: async () => { + try { + return await runQaSuiteWithInfraRetry(task.run); + } catch (error) { + // Failed partitions still own durable failure evidence; rejecting here would + // discard completed siblings and prevent the unified artifacts from existing. + return capturePartitionFailure(task, error); + } + }, })); return failFast ? await mapQaSuiteWithConcurrency(retryingTasks, 1, runFailFastPartition, { diff --git a/extensions/qa-lab/src/suite-run.runtime.ts b/extensions/qa-lab/src/suite-run.runtime.ts index bde4ed9dfdd2..750e09313bf3 100644 --- a/extensions/qa-lab/src/suite-run.runtime.ts +++ b/extensions/qa-lab/src/suite-run.runtime.ts @@ -46,6 +46,11 @@ export async function runQaFlowSuiteFromRuntime(params?: QaSuiteRunParams): Prom channel: params?.channelId ?? params?.channelDriverSelection?.channel, claudeCliAuthMode: params?.claudeCliAuthMode, }); + if (selectedScenarios.length === 0) { + throw new Error( + "QA suite selected no runnable scenarios; check the scenario catalog and provider, model, or channel filters.", + ); + } const { alternateModel, fastMode, primaryModel, providerMode } = resolveSelectedQaSuiteModels({ alternateModelExplicit: params?.alternateModel !== undefined, fastMode: params?.fastMode, diff --git a/extensions/qa-lab/src/suite-runtime-gateway.test.ts b/extensions/qa-lab/src/suite-runtime-gateway.test.ts index a882689c13b0..a925a3c10300 100644 --- a/extensions/qa-lab/src/suite-runtime-gateway.test.ts +++ b/extensions/qa-lab/src/suite-runtime-gateway.test.ts @@ -1,7 +1,4 @@ // Qa Lab tests cover suite runtime gateway plugin behavior. -import fs from "node:fs/promises"; -import os from "node:os"; -import path from "node:path"; import { afterEach, describe, expect, it, vi } from "vitest"; import { applyConfig, @@ -14,13 +11,18 @@ import { import type { QaSuiteRuntimeEnv } from "./suite-runtime-types.js"; const fetchWithSsrFGuardMock = vi.hoisted(() => vi.fn()); +const writeGatewayRestartIntentSyncMock = vi.hoisted(() => vi.fn()); +vi.mock("openclaw/plugin-sdk/qa-runtime", () => ({ + writeGatewayRestartIntentSync: writeGatewayRestartIntentSyncMock, +})); vi.mock("openclaw/plugin-sdk/ssrf-runtime", () => ({ fetchWithSsrFGuard: fetchWithSsrFGuardMock, })); afterEach(() => { fetchWithSsrFGuardMock.mockReset(); + writeGatewayRestartIntentSyncMock.mockReset(); vi.useRealTimers(); }); @@ -51,47 +53,94 @@ function createConfigMutationEnv( } describe("qa suite gateway helpers", () => { - it("replaces the gateway process after writing the requested config", async () => { - const tempDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-qa-gateway-restart-")); - const configPath = path.join(tempDir, "openclaw.json"); - await fs.writeFile(configPath, '{"gateway":{"auth":{"token":"keep-me"}}}\n', "utf8"); - const restartAfterStateMutation = vi.fn( - async ( - mutateState: (context: { - configPath: string; - runtimeEnv: NodeJS.ProcessEnv; - stateDir: string; - tempRoot: string; - }) => Promise, - ) => { - await mutateState({ - configPath, - runtimeEnv: {}, - stateDir: path.join(tempDir, "state"), - tempRoot: tempDir, - }); - }, - ); - try { - await restartGatewayWithConfigPatch({ - env: { gateway: { restartAfterStateMutation } } as never, - patch: { tools: { codeMode: { enabled: false } } }, - }); - - await expect(fs.readFile(configPath, "utf8")).resolves.toBe( - `${JSON.stringify( - { + it.each([ + { + name: "restarts through the authenticated gateway config patch", + alreadyApplied: false, + }, + { + name: "forces a restart when the authenticated gateway config patch was already applied", + alreadyApplied: true, + }, + ])("$name", async ({ alreadyApplied }) => { + vi.useFakeTimers(); + const release = vi.fn(async () => {}); + fetchWithSsrFGuardMock.mockResolvedValue({ + response: { ok: true }, + release, + }); + writeGatewayRestartIntentSyncMock.mockReturnValue(true); + const patch = { tools: { codeMode: { enabled: false } } }; + const gatewayCall = vi.fn(async (method: string) => { + if (method === "config.get") { + return { + hash: "hash-1", + config: { gateway: { auth: { token: "keep-me" } }, - tools: { codeMode: { enabled: false } }, + ...(alreadyApplied ? patch : {}), }, - null, - 2, - )}\n`, - ); - expect(restartAfterStateMutation).toHaveBeenCalledOnce(); - } finally { - await fs.rm(tempDir, { recursive: true, force: true }); - } + }; + } + if (method === "system.info") { + return { pid: 43_123 }; + } + return { ok: true }; + }); + const { env, waitReady } = createConfigMutationEnv(gatewayCall); + const runtimeEnv = { OPENCLAW_STATE_DIR: "/isolated/qa-gateway" }; + env.gateway.runtimeEnv = runtimeEnv; + const restartAfterStateMutation = vi.fn(); + env.gateway.restartAfterStateMutation = restartAfterStateMutation; + + const restarting = restartGatewayWithConfigPatch({ env, patch }); + await vi.advanceTimersByTimeAsync(1_750); + + await expect(restarting).resolves.toEqual({ ok: true }); + expect(gatewayCall).toHaveBeenNthCalledWith(1, "config.get", {}, { timeoutMs: 60_000 }); + expect(gatewayCall).toHaveBeenNthCalledWith(2, "system.info", {}, { timeoutMs: 180_000 }); + expect(gatewayCall).toHaveBeenNthCalledWith( + 3, + "config.patch", + { + raw: JSON.stringify(patch, null, 2), + baseHash: "hash-1", + restartDelayMs: 1_000, + replacePaths: ["gateway.controlUi.allowedOrigins"], + }, + { timeoutMs: 180_000 }, + ); + expect(gatewayCall).toHaveBeenNthCalledWith( + 4, + "gateway.restart.request", + { reason: "config.patch", skipDeferral: true }, + { timeoutMs: 180_000 }, + ); + expect(gatewayCall).toHaveBeenCalledTimes(4); + expect(writeGatewayRestartIntentSyncMock).toHaveBeenCalledWith({ + env: runtimeEnv, + targetPid: 43_123, + reason: "config.patch", + intent: { force: true }, + }); + expect(writeGatewayRestartIntentSyncMock).toHaveBeenCalledOnce(); + expect(writeGatewayRestartIntentSyncMock.mock.invocationCallOrder[0]).toBeGreaterThan( + gatewayCall.mock.invocationCallOrder[2] ?? Number.POSITIVE_INFINITY, + ); + expect(writeGatewayRestartIntentSyncMock.mock.invocationCallOrder[0]).toBeLessThan( + gatewayCall.mock.invocationCallOrder[3] ?? Number.NEGATIVE_INFINITY, + ); + expect(fetchWithSsrFGuardMock).toHaveBeenCalledWith( + expect.objectContaining({ + url: "http://127.0.0.1:43123/readyz", + auditContext: "qa-lab-suite-wait-for-gateway-healthy", + }), + ); + expect(waitReady).toHaveBeenCalledWith({ + gateway: env.gateway, + timeoutMs: expect.any(Number), + }); + expect(release).toHaveBeenCalled(); + expect(restartAfterStateMutation).not.toHaveBeenCalled(); }); it("bounds oversized suite gateway JSON responses", async () => { diff --git a/extensions/qa-lab/src/suite-runtime-gateway.ts b/extensions/qa-lab/src/suite-runtime-gateway.ts index 3b0452f8a101..80ba15670979 100644 --- a/extensions/qa-lab/src/suite-runtime-gateway.ts +++ b/extensions/qa-lab/src/suite-runtime-gateway.ts @@ -1,8 +1,8 @@ // Qa Lab plugin module implements suite runtime gateway behavior. -import fs from "node:fs/promises"; import { setTimeout as sleep } from "node:timers/promises"; import { formatErrorMessage } from "openclaw/plugin-sdk/error-runtime"; import { readProviderJsonResponse } from "openclaw/plugin-sdk/provider-http"; +import { writeGatewayRestartIntentSync } from "openclaw/plugin-sdk/qa-runtime"; import { fetchWithSsrFGuard } from "openclaw/plugin-sdk/ssrf-runtime"; import { isRecord as isPlainObject } from "openclaw/plugin-sdk/string-coerce-runtime"; import { QaSuiteInfraError, toQaErrorObject } from "./errors.js"; @@ -267,19 +267,35 @@ async function runConfigMutation(params: { restartDelayMs?: number; restartSettleBufferMs?: number; replacePaths?: readonly string[]; + skipRestartDeferral?: boolean; }) { const restartDelayMs = params.restartDelayMs ?? 1_000; const timeoutMs = liveTurnTimeoutMs(params.env, 180_000); let lastConflict: unknown = null; for (let attempt = 1; attempt <= 8; attempt += 1) { const snapshot = await readConfigSnapshot(params.env); - if (isConfigMutationNoopForSnapshot(params.action, snapshot.config, params.raw)) { + if ( + isConfigMutationNoopForSnapshot(params.action, snapshot.config, params.raw) && + params.skipRestartDeferral !== true + ) { // QA scenarios do best-effort cleanup in finally blocks. Skipping // client-known no-op patches keeps that cleanup from burning the // control-plane write budget and making later capability checks flaky. return { ok: true, noop: true }; } try { + let restartTargetPid: number | undefined; + if (params.skipRestartDeferral === true) { + const systemInfo = await params.env.gateway.call("system.info", {}, { timeoutMs }); + const targetPid = + typeof systemInfo === "object" && systemInfo !== null + ? (systemInfo as { pid?: unknown }).pid + : undefined; + if (typeof targetPid !== "number" || !Number.isSafeInteger(targetPid) || targetPid <= 0) { + throw new Error("qa gateway restart returned an invalid active process id"); + } + restartTargetPid = targetPid; + } const result = await params.env.gateway.call( params.action, { @@ -293,6 +309,23 @@ async function runConfigMutation(params: { }, { timeoutMs }, ); + if (params.skipRestartDeferral === true) { + if ( + !writeGatewayRestartIntentSync({ + env: params.env.gateway.runtimeEnv, + targetPid: restartTargetPid, + reason: "config.patch", + intent: { force: true }, + }) + ) { + throw new Error("qa gateway could not persist a forced restart intent"); + } + await params.env.gateway.call( + "gateway.restart.request", + { reason: "config.patch", skipDeferral: true }, + { timeoutMs }, + ); + } await waitForConfigRestartSettle( params.env, restartDelayMs, @@ -355,6 +388,7 @@ async function patchConfig(params: { restartDelayMs?: number; restartSettleBufferMs?: number; replacePaths?: readonly string[]; + skipRestartDeferral?: boolean; }) { return await runConfigMutation({ env: params.env, @@ -366,6 +400,7 @@ async function patchConfig(params: { restartDelayMs: params.restartDelayMs, restartSettleBufferMs: params.restartSettleBufferMs, replacePaths: params.replacePaths, + skipRestartDeferral: params.skipRestartDeferral, }); } @@ -394,18 +429,14 @@ async function applyConfig(params: { } async function restartGatewayWithConfigPatch(params: { - env: Pick; + env: QaGatewayMutationEnv; patch: Record; }) { - const restart = params.env.gateway.restartAfterStateMutation; - if (!restart) { - throw new Error("qa gateway child cannot restart after state mutation"); - } - await restart(async ({ configPath }) => { - const raw = await fs.readFile(configPath, "utf8"); - const config = JSON.parse(raw || "{}") as Record; - const nextConfig = applyQaMergePatch(config, params.patch); - await fs.writeFile(configPath, `${JSON.stringify(nextConfig, null, 2)}\n`, "utf8"); + return await patchConfig({ + env: params.env, + patch: params.patch, + replacePaths: ["gateway.controlUi.allowedOrigins"], + skipRestartDeferral: true, }); } diff --git a/extensions/qa-lab/src/suite-summary.test.ts b/extensions/qa-lab/src/suite-summary.test.ts index db25b66ab0ce..cb32550a119f 100644 --- a/extensions/qa-lab/src/suite-summary.test.ts +++ b/extensions/qa-lab/src/suite-summary.test.ts @@ -30,6 +30,25 @@ describe("qa suite summary helpers", () => { ).toBe(2); }); + it.each([ + ["failure", readQaSuiteFailedScenarioCountFromFile], + ["failure and skip", readQaSuiteFailedOrSkippedScenarioCountFromFile], + ] as const)("rejects zero-execution summaries in the %s gate", async (_name, reader) => { + for (const summary of [ + { + counts: { total: 0, passed: 0, failed: 0, skipped: 0 }, + scenarios: [], + }, + { counts: { failed: 0, skipped: 0 }, scenarios: [] }, + { counts: { failed: 0, skipped: 0 }, entries: [] }, + { counts: { failed: 0, skipped: 0 } }, + ]) { + await expect(readSummary(summary, reader)).rejects.toThrow( + "did not include any executed scenarios", + ); + } + }); + it("counts failed and skipped scenarios from scenario statuses", async () => { await expect( readSummary( @@ -58,6 +77,128 @@ describe("qa suite summary helpers", () => { ).resolves.toBe(2); }); + it.each([ + ["missing", {}], + ["timeout", { status: "timeout" }], + ["blocked", { status: "blocked" }], + ["error", { status: "error" }], + ] as const)("counts %s scenario statuses as failures in both gates", async (_name, scenario) => { + const summary = { + counts: { failed: 0, skipped: 0 }, + scenarios: [scenario], + }; + + await expect(readSummary(summary, readQaSuiteFailedScenarioCountFromFile)).resolves.toBe(1); + await expect( + readSummary(summary, readQaSuiteFailedOrSkippedScenarioCountFromFile), + ).resolves.toBe(1); + }); + + it("rejects a suite containing only catalog-confirmed report-only skips", async () => { + await expect( + readSummary( + { + counts: { total: 1, passed: 0, failed: 0, skipped: 1 }, + scenarios: [ + { + name: "optional tool fixture", + status: "skip", + details: "expected-unavailable tool fixture; report-only", + }, + ], + }, + (summaryPath) => + readQaSuiteFailedOrSkippedScenarioCountFromFile(summaryPath, { + optionalScenarioNames: new Set(["optional tool fixture"]), + }), + ), + ).rejects.toThrow("did not include any executed scenarios"); + }); + + it("rejects skipped-only summaries in failure-only model gates", async () => { + await expect( + readSummary( + { + counts: { total: 1, passed: 0, failed: 0, skipped: 1 }, + scenarios: [ + { + name: "optional tool fixture", + status: "skip", + details: "expected-unavailable tool fixture; report-only", + }, + ], + }, + readQaSuiteFailedScenarioCountFromFile, + ), + ).rejects.toThrow("did not include any executed scenarios"); + }); + + it.each(["skip", "skipped"] as const)( + "keeps evidence-only %s results blocking for strict package gates", + async (status) => { + await expect( + readSummary( + { entries: [{ result: { status } }] }, + readQaSuiteFailedOrSkippedScenarioCountFromFile, + ), + ).resolves.toBe(1); + }, + ); + + it.each(["skip", "skipped"] as const)( + "rejects evidence-only %s results in failure-only model gates", + async (status) => { + await expect( + readSummary({ entries: [{ result: { status } }] }, readQaSuiteFailedScenarioCountFromFile), + ).rejects.toThrow("did not include any executed scenarios"); + }, + ); + + it.each(["blocked", "timeout", "error"] as const)( + "keeps evidence-only %s results fail-closed in strict package gates", + async (status) => { + await expect( + readSummary( + { entries: [{ result: { status } }] }, + readQaSuiteFailedOrSkippedScenarioCountFromFile, + ), + ).resolves.toBe(1); + }, + ); + + it.each([ + ["blocked", { status: "blocked" }], + ["timeout", { status: "timeout" }], + ["error", { status: "error" }], + ["missing", {}], + ] as const)("keeps standalone %s evidence fail-closed in both gates", async (_name, result) => { + const summary = { + counts: { total: 1, passed: 1, failed: 0, skipped: 0 }, + scenarios: [{ status: "pass" }], + entries: [{ result }], + }; + + await expect(readSummary(summary, readQaSuiteFailedScenarioCountFromFile)).resolves.toBe(1); + await expect( + readSummary(summary, readQaSuiteFailedOrSkippedScenarioCountFromFile), + ).resolves.toBe(1); + }); + + it("rejects evidence-only results without an observed status", async () => { + await expect( + readSummary({ entries: [{ result: {} }] }, readQaSuiteFailedOrSkippedScenarioCountFromFile), + ).rejects.toThrow("did not include any executed scenarios"); + }); + + it.each([ + ["failure", readQaSuiteFailedScenarioCountFromFile], + ["failure and skip", readQaSuiteFailedOrSkippedScenarioCountFromFile], + ] as const)("retains positive legacy execution counts in the %s gate", async (_name, reader) => { + await expect( + readSummary({ counts: { total: 1, passed: 1, failed: 0, skipped: 0 } }, reader), + ).resolves.toBe(0); + }); + it("excludes only catalog-confirmed report-only optional skips from suite gates", async () => { await expect( readSummary( @@ -80,6 +221,28 @@ describe("qa suite summary helpers", () => { ).resolves.toBe(0); }); + it("keeps a report-only skip non-blocking when a real scenario also ran", async () => { + await expect( + readSummary( + { + counts: { total: 2, passed: 1, failed: 0, skipped: 1 }, + scenarios: [ + { name: "required scenario", status: "pass" }, + { + name: "optional tool fixture", + status: "skipped", + details: "expected-unavailable tool fixture; report-only", + }, + ], + }, + (summaryPath) => + readQaSuiteFailedOrSkippedScenarioCountFromFile(summaryPath, { + optionalScenarioNames: new Set(["optional tool fixture"]), + }), + ), + ).resolves.toBe(0); + }); + it("keeps unknown and unverified report-only skips fail-closed", async () => { await expect( readSummary( diff --git a/extensions/qa-lab/src/suite-summary.ts b/extensions/qa-lab/src/suite-summary.ts index f910e138b71b..22a873b93bcb 100644 --- a/extensions/qa-lab/src/suite-summary.ts +++ b/extensions/qa-lab/src/suite-summary.ts @@ -67,7 +67,9 @@ export type QaSuiteSummaryJson = { }; }; -type QaSuiteScenarioStatus = Pick; +type QaSuiteScenarioStatus = { + status?: unknown; +}; type QaSuiteReportOnlyScenario = { name?: unknown; status?: unknown; @@ -79,6 +81,10 @@ type QaEvidenceEntryStatus = { }; }; +function isQaSuiteFailureStatus(status: unknown): boolean { + return status !== "pass" && status !== "skip" && status !== "skipped"; +} + async function readQaSuiteSummaryFile(summaryPath: string): Promise { let summaryText: string; try { @@ -107,11 +113,79 @@ function readNonNegativeCount(value: unknown): number | null { : null; } +function assertQaSuiteSummaryHasExecutedScenarios( + summary: unknown, + summaryPath: string, + errorCode: "summary_failure_count_missing" | "summary_blocking_count_missing", + optionalScenarioNames?: ReadonlySet, +): void { + if (!summary || typeof summary !== "object") { + return; + } + const payload = summary as { + counts?: { total?: unknown; passed?: unknown; failed?: unknown; skipped?: unknown }; + scenarios?: unknown; + entries?: unknown; + }; + const total = readNonNegativeCount(payload.counts?.total); + const passed = readNonNegativeCount(payload.counts?.passed); + const failed = readNonNegativeCount(payload.counts?.failed); + const skipped = readNonNegativeCount(payload.counts?.skipped); + const scenarios = Array.isArray(payload.scenarios) + ? (payload.scenarios as QaSuiteReportOnlyScenario[]) + : undefined; + const entries = Array.isArray(payload.entries) + ? (payload.entries as QaEvidenceEntryStatus[]) + : undefined; + const hasExecutedScenario = + scenarios?.some((scenario) => scenario.status === "pass" || scenario.status === "fail") === + true || + entries?.some((entry) => entry.result?.status === "pass" || entry.result?.status === "fail") === + true || + (passed ?? 0) > 0 || + (failed ?? 0) > 0 || + (total !== null && total > 0 && (skipped === null || total > skipped)); + const hasBlockingNonOptionalSkip = + errorCode === "summary_blocking_count_missing" && + scenarios?.some( + (scenario) => + (scenario.status === "skip" || scenario.status === "skipped") && + !isQaSuiteReportOnlyOptionalScenario(scenario, optionalScenarioNames), + ) === true; + const hasBlockingUnknownOrFailedScenario = + scenarios?.some((scenario) => isQaSuiteFailureStatus(scenario.status)) === true; + const hasBlockingNonPassEvidence = + entries?.some( + (entry) => + typeof entry.result?.status === "string" && + (errorCode === "summary_blocking_count_missing" + ? isQaSuiteBlockingStatus(entry.result.status) + : isQaSuiteFailureStatus(entry.result.status)), + ) === true; + + // Optional skips are not execution: only a real scenario, evidence result, + // or positive legacy count may clear a zero-work suite. Unverified skips + // remain blocking, including package runs that expose evidence entries only. + if ( + total === 0 || + scenarios?.length === 0 || + (!hasExecutedScenario && + !hasBlockingUnknownOrFailedScenario && + !hasBlockingNonOptionalSkip && + !hasBlockingNonPassEvidence) + ) { + throw new QaSuiteArtifactError( + errorCode, + `QA summary at ${summaryPath} did not include any executed scenarios.`, + ); + } +} + function isQaSuiteBlockingStatus(status: unknown): boolean { return status !== "pass"; } -export function isQaSuiteReportOnlyOptionalScenario( +function isQaSuiteReportOnlyOptionalScenario( scenario: QaSuiteReportOnlyScenario, optionalScenarioNames: ReadonlySet | undefined, ): boolean { @@ -129,7 +203,7 @@ export function countQaSuiteFailedScenarios( ): number { let failed = 0; for (const scenario of scenarios) { - if (scenario.status === "fail") { + if (isQaSuiteFailureStatus(scenario.status)) { failed += 1; } } @@ -164,7 +238,7 @@ function readQaSuiteFailedScenarioCountFromSummary(summary: unknown): number | n ? countQaSuiteFailedScenarios(payload.scenarios) : null; const evidenceFailures = Array.isArray(payload.entries) - ? payload.entries.filter((entry) => entry.result?.status === "fail").length + ? payload.entries.filter((entry) => isQaSuiteFailureStatus(entry.result?.status)).length : null; if (countedFailures !== null && scenarioFailures !== null) { return Math.max(countedFailures, scenarioFailures, evidenceFailures ?? 0); @@ -222,6 +296,7 @@ function readQaSuiteFailedOrSkippedScenarioCountFromSummary(summary: unknown): n export async function readQaSuiteFailedScenarioCountFromFile(summaryPath: string): Promise { const payload = await readQaSuiteSummaryFile(summaryPath); + assertQaSuiteSummaryHasExecutedScenarios(payload, summaryPath, "summary_failure_count_missing"); const failedScenarioCount = readQaSuiteFailedScenarioCountFromSummary(payload); if (failedScenarioCount !== null) { return failedScenarioCount; @@ -237,6 +312,12 @@ export async function readQaSuiteFailedOrSkippedScenarioCountFromFile( options?: { optionalScenarioNames?: ReadonlySet }, ): Promise { const payload = await readQaSuiteSummaryFile(summaryPath); + assertQaSuiteSummaryHasExecutedScenarios( + payload, + summaryPath, + "summary_blocking_count_missing", + options?.optionalScenarioNames, + ); const blockingScenarioCount = readQaSuiteFailedOrSkippedScenarioCountFromSummary(payload); if (blockingScenarioCount !== null) { const optionalScenarioNames = options?.optionalScenarioNames; diff --git a/extensions/qa-lab/src/test-file-scenario-runner.e2e-routing.test.ts b/extensions/qa-lab/src/test-file-scenario-runner.e2e-routing.test.ts index c804a019792c..8f3f91a0949f 100644 --- a/extensions/qa-lab/src/test-file-scenario-runner.e2e-routing.test.ts +++ b/extensions/qa-lab/src/test-file-scenario-runner.e2e-routing.test.ts @@ -39,12 +39,22 @@ describe("QA native Vitest scenario routing", () => { scenarios: [scenario], runCommand: async (command) => { commands.push(command); + const reportArg = command.args.find((arg) => arg.startsWith("--outputFile.json=")); + if (!reportArg) { + throw new Error("native Vitest scenario did not request a JSON test report"); + } + await fs.writeFile( + reportArg.slice("--outputFile.json=".length), + JSON.stringify({ numFailedTests: 0, numPassedTests: 1, success: true }), + "utf8", + ); return { exitCode: 0, stdout: "1 passed\n", stderr: "" }; }, }); expect(result.executionKind).toBe("vitest"); expect(result.results).toMatchObject([{ status: "pass" }]); + expect(result.evidence.entries[0]?.result.status).toBe("pass"); expect(commands.map((command) => command.args)).toEqual([ [ "scripts/run-vitest.mjs", @@ -53,6 +63,14 @@ describe("QA native Vitest scenario routing", () => { "test/vitest/vitest.e2e.config.ts", testPath, "--reporter=verbose", + "--reporter=json", + `--outputFile.json=${path.join( + repoRoot, + ".artifacts", + "qa-e2e", + scenario.id, + `${scenario.id}.vitest-report.json`, + )}`, ], ]); } finally { diff --git a/extensions/qa-lab/src/test-file-scenario-runner.test.ts b/extensions/qa-lab/src/test-file-scenario-runner.test.ts index efa875ccf222..3f3d763ca005 100644 --- a/extensions/qa-lab/src/test-file-scenario-runner.test.ts +++ b/extensions/qa-lab/src/test-file-scenario-runner.test.ts @@ -6,6 +6,7 @@ import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from "vitest import { validateQaEvidenceSummaryJson } from "./evidence-summary.js"; import type { QaSeedScenarioWithSource } from "./scenario-catalog.js"; import { createTempDirHarness } from "./temp-dir.test-helper.js"; +import { runQaScenarioCommandLifecycle } from "./test-file-scenario-command-lifecycle.js"; import { dockerE2eLaneName } from "./test-file-scenario-docker-batch.js"; import { qaTestFileScenarioRunnerTesting, @@ -118,6 +119,25 @@ async function makeTempRepo(prefix: string) { return repoRoot; } +async function writeNativeVitestReport( + command: QaScenarioCommandExecution, + counts: { failed?: number; passed: number }, +) { + const reportArg = command.args.find((arg) => arg.startsWith("--outputFile.json=")); + if (!reportArg) { + return; + } + await fs.writeFile( + reportArg.slice("--outputFile.json=".length), + JSON.stringify({ + numFailedTests: counts.failed ?? 0, + numPassedTests: counts.passed, + success: (counts.failed ?? 0) === 0, + }), + "utf8", + ); +} + async function writeScriptProducerEvidence(params: { outputDir: string; scenarioId?: string; @@ -202,6 +222,7 @@ describe("qa test file scenario runner", () => { ], runCommand: async (command) => { commands.push(command); + await writeNativeVitestReport(command, { passed: 1 }); return { exitCode: 0, stdout: "pass\n", @@ -225,6 +246,14 @@ describe("qa test file scenario runner", () => { "runner", "ui/src/e2e/chat-flow.e2e.test.ts", "--reporter=verbose", + "--reporter=json", + `--outputFile.json=${path.join( + repoRoot, + ".artifacts", + "qa-e2e", + "scenario-playwright", + "scenario-playwright.vitest-report.json", + )}`, "--testNamePattern", "sends a chat turn through the GUI", ], @@ -288,11 +317,14 @@ describe("qa test file scenario runner", () => { primaryModel: "mock-openai/gpt-5.6-luna", scenarios: [makeTestFileScenario("playwright", "ui/src/e2e/chat-flow.e2e.test.ts")], writeEvidenceFile: false, - runCommand: async () => ({ - exitCode: 0, - stdout: "pass\n", - stderr: "", - }), + runCommand: async (command) => { + await writeNativeVitestReport(command, { passed: 1 }); + return { + exitCode: 0, + stdout: "pass\n", + stderr: "", + }; + }, }); expect(result.evidence.entries).toHaveLength(1); @@ -324,6 +356,14 @@ describe("qa test file scenario runner", () => { "scripts/run-vitest.mjs", "extensions/qa-lab/src/coverage-report.test.ts", "--reporter=verbose", + "--reporter=json", + `--outputFile.json=${path.join( + repoRoot, + ".artifacts", + "qa-e2e", + "scenario-vitest", + "scenario-vitest.vitest-report.json", + )}`, ], ]); expect(commands.map((command) => command.timeoutMs)).toEqual([undefined]); @@ -367,6 +407,87 @@ describe("qa test file scenario runner", () => { }); }); + it.each([ + { executionKind: "vitest" as const, passed: 0, expectedStatus: "fail" as const }, + { executionKind: "playwright" as const, passed: 0, expectedStatus: "fail" as const }, + { executionKind: "vitest" as const, passed: 1, expectedStatus: "pass" as const }, + { executionKind: "playwright" as const, passed: 1, expectedStatus: "pass" as const }, + ])( + "requires an actually passed $executionKind test when the native child exits successfully ($passed passed)", + async ({ executionKind, expectedStatus, passed }) => { + const repoRoot = await makeTempRepo(`qa-${executionKind}-executed-tests-`); + const outputDir = path.join(repoRoot, ".artifacts", "qa-e2e", `scenario-${executionKind}`); + const scenarioPath = + executionKind === "playwright" + ? "ui/src/e2e/chat-flow.e2e.test.ts" + : "extensions/qa-lab/src/coverage-report.test.ts"; + const commands: QaScenarioCommandExecution[] = []; + const result = await runQaTestFileScenarios({ + repoRoot, + outputDir, + providerMode: "mock-openai", + primaryModel: "mock-openai/gpt-5.6-luna", + scenarios: [makeTestFileScenario(executionKind, scenarioPath)], + runCommand: async (command) => { + commands.push(command); + await writeNativeVitestReport(command, { passed }); + return { exitCode: 0, stdout: "child exited successfully\n", stderr: "" }; + }, + }); + + expect(result.results[0]).toMatchObject({ status: expectedStatus }); + expect(result.evidence.entries[0]?.result.status).toBe(expectedStatus); + expect( + commands.filter((command) => command.args[0] === "scripts/run-vitest.mjs"), + ).toHaveLength(1); + if (expectedStatus === "fail") { + expect(result.results[0]?.failureMessage).toBe( + "Vitest exited successfully without reporting a successfully executed test.", + ); + } + }, + ); + + it.each([{ executionKind: "vitest" as const }, { executionKind: "playwright" as const }])( + "does not reuse a prior passing $executionKind report when the next child writes none", + async ({ executionKind }) => { + const repoRoot = await makeTempRepo(`qa-${executionKind}-stale-vitest-report-`); + const outputDir = path.join(repoRoot, ".artifacts", "qa-e2e", `scenario-${executionKind}`); + const scenarioPath = + executionKind === "playwright" + ? "ui/src/e2e/chat-flow.e2e.test.ts" + : "extensions/qa-lab/src/coverage-report.test.ts"; + const reportPath = path.join(outputDir, `scenario-${executionKind}.vitest-report.json`); + let writeReport = true; + const runParams = { + repoRoot, + outputDir, + providerMode: "mock-openai" as const, + primaryModel: "mock-openai/gpt-5.6-luna", + scenarios: [makeTestFileScenario(executionKind, scenarioPath)], + runCommand: async (command: QaScenarioCommandExecution) => { + if (writeReport) { + await writeNativeVitestReport(command, { passed: 1 }); + } + return { exitCode: 0, stdout: "child exited successfully\n", stderr: "" }; + }, + }; + + const firstRun = await runQaTestFileScenarios(runParams); + expect(firstRun.results[0]).toMatchObject({ status: "pass" }); + await expect(fs.access(reportPath)).resolves.toBeUndefined(); + + writeReport = false; + const secondRun = await runQaTestFileScenarios(runParams); + expect(secondRun.results[0]).toMatchObject({ + failureMessage: `Vitest exited successfully without writing a valid JSON test report at ${reportPath}.`, + status: "fail", + }); + expect(secondRun.evidence.entries[0]?.result.status).toBe("fail"); + await expect(fs.access(reportPath)).rejects.toMatchObject({ code: "ENOENT" }); + }, + ); + it.each([ { failFast: true, expectedScenarioIds: ["first-native-scenario"] }, { @@ -412,6 +533,116 @@ describe("qa test file scenario runner", () => { }, ); + it.each([ + { evidence: "missing", expectedFailure: /without writing fresh producer QA evidence/u }, + { evidence: "stale", expectedFailure: /not written by the current scenario run/u }, + { evidence: "empty", expectedFailure: /without reporting an executed producer check/u }, + { evidence: "malformed", expectedFailure: /invalid JSON/u }, + { evidence: "outside", expectedFailure: /inside its scenario output directory/u }, + ] as const)( + "fails a successful script with $evidence producer evidence", + async ({ evidence, expectedFailure }) => { + const repoRoot = await makeTempRepo(`qa-script-${evidence}-producer-evidence-`); + const outputDir = path.join(repoRoot, ".artifacts", "qa-e2e", "scenario-script"); + const scenarioOutputDir = path.join(outputDir, "scenario-script"); + const latestRunPath = path.join(scenarioOutputDir, "latest-run.json"); + const evidencePath = path.join(scenarioOutputDir, "qa-evidence.json"); + + if (evidence === "stale") { + await writeScriptProducerEvidence({ outputDir, status: "pass" }); + const staleEvidencePath = path.join(scenarioOutputDir, "run-1", "qa-evidence.json"); + await fs.copyFile(staleEvidencePath, evidencePath); + const staleTimestamp = new Date(Date.now() - 60_000); + await Promise.all([ + fs.utimes(staleEvidencePath, staleTimestamp, staleTimestamp), + fs.utimes(evidencePath, staleTimestamp, staleTimestamp), + ]); + } + + const result = await runQaTestFileScenarios({ + repoRoot, + outputDir, + providerMode: "mock-openai", + primaryModel: "mock-openai/gpt-5.6-luna", + scenarios: [makeTestFileScenario("script", "scripts/evidence-producer.ts")], + runCommand: async () => { + await fs.mkdir(scenarioOutputDir, { recursive: true }); + if (evidence === "stale") { + await expect(fs.access(latestRunPath)).rejects.toMatchObject({ code: "ENOENT" }); + await expect(fs.access(evidencePath)).rejects.toMatchObject({ code: "ENOENT" }); + await fs.writeFile( + latestRunPath, + JSON.stringify({ + qaEvidence: path.join(scenarioOutputDir, "run-1", "qa-evidence.json"), + }), + "utf8", + ); + } else if (evidence === "empty") { + await fs.writeFile( + evidencePath, + JSON.stringify({ + kind: "openclaw.qa.evidence-summary", + schemaVersion: 2, + generatedAt: new Date().toISOString(), + evidenceMode: "full", + entries: [], + }), + "utf8", + ); + } else if (evidence === "malformed") { + await fs.writeFile(evidencePath, "{not valid JSON", "utf8"); + } else if (evidence === "outside") { + await writeScriptProducerEvidence({ + outputDir, + scenarioId: "different-script-scenario", + status: "pass", + }); + await fs.writeFile( + latestRunPath, + JSON.stringify({ + qaEvidence: path.join( + outputDir, + "different-script-scenario", + "run-1", + "qa-evidence.json", + ), + }), + "utf8", + ); + } + return { exitCode: 0, stdout: "script exited successfully\n", stderr: "" }; + }, + }); + + expect(result.results[0]).toMatchObject({ status: "fail" }); + expect(result.results[0]?.failureMessage).toMatch(expectedFailure); + expect(result.evidence.entries).toHaveLength(1); + expect(result.evidence.entries[0]).toMatchObject({ + test: { id: "scenario-script" }, + result: { status: "fail" }, + }); + }, + ); + + it("preserves individual Docker lane success without generic producer evidence", async () => { + const repoRoot = await makeTempRepo("qa-script-docker-individual-no-producer-evidence-"); + const result = await runQaTestFileScenarios({ + repoRoot, + outputDir: path.join(repoRoot, ".artifacts", "qa-e2e", "docker-individual"), + providerMode: "mock-openai", + primaryModel: "mock-openai/gpt-5.6-luna", + failFast: true, + scenarios: [makeDockerE2eScenario("docker-gateway-network", "gateway-network")], + runCommand: async () => ({ exitCode: 0, stdout: "Docker lane passed\n", stderr: "" }), + }); + + expect(result.results[0]).toMatchObject({ + scenario: { id: "docker-gateway-network" }, + status: "pass", + }); + expect(result.evidence.entries[0]?.result.status).toBe("pass"); + }); + it("runs script scenarios and imports producer QA evidence artifacts", async () => { const repoRoot = await makeTempRepo("qa-script-scenario-"); const commands: QaScenarioCommandExecution[] = []; @@ -661,7 +892,7 @@ describe("qa test file scenario runner", () => { beforeAll(async () => { const tempRoot = await makeTempDir("qa-script-timeout-"); - const scriptPath = path.join(tempRoot, "hanging-producer.ts"); + const scriptPath = path.join(tempRoot, "hanging-producer.mjs"); const descendantPidPath = path.join(tempRoot, "descendant.pid"); const descendantScript = [ "process.on('SIGTERM', () => {});", @@ -692,9 +923,23 @@ describe("qa test file scenario runner", () => { primaryModel: "mock-openai/gpt-5.6-luna", scenarios: [makeTestFileScenario("script", scriptPath)], commandTimeoutMs, + // Exercise the real process-group lifecycle without spending its + // bounded startup budget on an unrelated cold tsx import. + runCommand: (execution) => + runQaScenarioCommandLifecycle({ ...execution, args: [scriptPath] }), }); - descendantPid = await readPid(descendantPidPath, commandTimeoutMs); - result = await run; + const [pidResult, runResult] = await Promise.allSettled([ + readPid(descendantPidPath, commandTimeoutMs), + run, + ]); + if (pidResult.status === "rejected") { + throw pidResult.reason; + } + if (runResult.status === "rejected") { + throw runResult.reason; + } + descendantPid = pidResult.value; + result = runResult.value; await waitForDead(descendantPid, 2_000); }); diff --git a/extensions/qa-lab/src/test-file-scenario-runner.ts b/extensions/qa-lab/src/test-file-scenario-runner.ts index 95433473aab4..36b40e0732ed 100644 --- a/extensions/qa-lab/src/test-file-scenario-runner.ts +++ b/extensions/qa-lab/src/test-file-scenario-runner.ts @@ -3,7 +3,7 @@ import path from "node:path"; import { formatErrorMessage } from "openclaw/plugin-sdk/error-runtime"; import { resolvePositiveTimerTimeoutMs } from "openclaw/plugin-sdk/number-runtime"; import { assertQaSuiteArtifactWritten } from "./artifact-assertion.js"; -import { isRepoRootRelativeRef, toRepoRelativePath } from "./cli-paths.js"; +import { toRepoRelativePath } from "./cli-paths.js"; import { buildPlaywrightEvidenceSummary, buildScriptEvidenceSummary, @@ -29,6 +29,10 @@ import { type QaScenarioCommandResult, } from "./test-file-scenario-command-lifecycle.js"; import { isDockerE2eScenario, runDockerE2eBatch } from "./test-file-scenario-docker-batch.js"; +import { + readJsonFileIfExists, + readScriptProducerEvidence, +} from "./test-file-scenario-script-evidence.js"; export type { QaScenarioCommandExecution } from "./test-file-scenario-command-lifecycle.js"; export type QaTestFileScenario = QaSeedScenarioWithSource & { @@ -97,7 +101,25 @@ export function isQaTestFileScenario( ); } -function vitestSteps(scenario: QaTestFileScenario): QaScenarioCommandStep[] { +function resolveNativeVitestReportPath(scenario: QaTestFileScenario, outputDir: string): string { + return path.join(outputDir, `${scenario.id}.vitest-report.json`); +} + +function vitestReporterArgs( + scenario: QaTestFileScenario, + context: { outputDir: string }, +): string[] { + return [ + "--reporter=verbose", + "--reporter=json", + `--outputFile.json=${resolveNativeVitestReportPath(scenario, context.outputDir)}`, + ]; +} + +function vitestSteps( + scenario: QaTestFileScenario, + context: { outputDir: string }, +): QaScenarioCommandStep[] { const e2eConfigArgs = scenario.execution.path.endsWith(".e2e.test.ts") ? ["run", "--config", "test/vitest/vitest.e2e.config.ts"] : []; @@ -108,13 +130,16 @@ function vitestSteps(scenario: QaTestFileScenario): QaScenarioCommandStep[] { "scripts/run-vitest.mjs", ...e2eConfigArgs, scenario.execution.path, - "--reporter=verbose", + ...vitestReporterArgs(scenario, context), ], }, ]; } -function playwrightSteps(scenario: QaTestFileScenario): QaScenarioCommandStep[] { +function playwrightSteps( + scenario: QaTestFileScenario, + context: { outputDir: string }, +): QaScenarioCommandStep[] { const testNamePattern = scenario.execution.kind === "playwright" ? scenario.execution.testNamePattern : undefined; const testNameArgs = testNamePattern ? ["--testNamePattern", testNamePattern] : []; @@ -133,7 +158,7 @@ function playwrightSteps(scenario: QaTestFileScenario): QaScenarioCommandStep[] "--configLoader", "runner", scenario.execution.path, - "--reporter=verbose", + ...vitestReporterArgs(scenario, context), ...testNameArgs, ], }, @@ -216,6 +241,32 @@ function withScenarioCoverage( return { ...entry, coverage: coverageForScenario(scenario) }; } +async function readNativeVitestExecutionFailure(params: { + outputDir: string; + scenario: QaTestFileScenario; +}): Promise { + const reportPath = resolveNativeVitestReportPath(params.scenario, params.outputDir); + const report = await readJsonFileIfExists(reportPath); + if (!report || typeof report !== "object") { + return `Vitest exited successfully without writing a valid JSON test report at ${reportPath}.`; + } + const { numFailedTests, numPassedTests, success } = report as { + numFailedTests?: unknown; + numPassedTests?: unknown; + success?: unknown; + }; + if ( + success !== true || + typeof numPassedTests !== "number" || + !Number.isSafeInteger(numPassedTests) || + numPassedTests < 1 || + numFailedTests !== 0 + ) { + return "Vitest exited successfully without reporting a successfully executed test."; + } + return undefined; +} + async function runScenarioCommandSteps(params: { commandTimeoutMs: number; env: NodeJS.ProcessEnv; @@ -232,6 +283,15 @@ async function runScenarioCommandSteps(params: { for (const step of params.steps) { logChunks.push(`$ ${formatCommand(step)}\n`); try { + const isNativeVitestStep = + params.scenario.execution.kind !== "script" && step.args[0] === "scripts/run-vitest.mjs"; + if (isNativeVitestStep) { + // A reused scenario output directory must not let a previous run's + // passing report authenticate a child that emitted no report. + await fs.rm(resolveNativeVitestReportPath(params.scenario, params.outputDir), { + force: true, + }); + } const timeoutMs = params.scenario.execution.kind === "script" ? (params.scenario.execution.timeoutMs ?? params.commandTimeoutMs) @@ -257,6 +317,15 @@ async function runScenarioCommandSteps(params: { : `${path.basename(step.command)} exited with ${result.exitCode}`); break; } + // Chromium installation and script producers do not execute Vitest tests. + // Only the final native test command can prove an assertion actually ran. + if (isNativeVitestStep) { + failureMessage = await readNativeVitestExecutionFailure(params); + if (failureMessage) { + logChunks.push(`${failureMessage}\n`); + break; + } + } } catch (error) { failureMessage = formatErrorMessage(error); logChunks.push(`${failureMessage}\n`); @@ -283,6 +352,19 @@ async function runQaTestFileScenario(params: { runCommand: QaScenarioCommandRunner; scenario: QaTestFileScenario; }) { + const requiresProducerEvidence = + params.scenario.execution.kind === "script" && !isDockerE2eScenario(params.scenario); + let producerEvidenceStartedAtMs: number | undefined; + if (requiresProducerEvidence) { + const scenarioOutputDir = path.join(params.outputDir, params.scenario.id); + // Both producer indexes belong to one command invocation. Clear them before + // launch so a successful no-op cannot authenticate an earlier scenario run. + await Promise.all([ + fs.rm(path.join(scenarioOutputDir, "latest-run.json"), { force: true }), + fs.rm(path.join(scenarioOutputDir, QA_EVIDENCE_FILENAME), { force: true }), + ]); + producerEvidenceStartedAtMs = Date.now(); + } const definition = testFileRunnerDefinitions[params.scenario.execution.kind]; const result = await runScenarioCommandSteps({ ...params, @@ -291,12 +373,32 @@ async function runQaTestFileScenario(params: { if (params.scenario.execution.kind !== "script") { return result; } - const producerEvidenceResult = await readScriptProducerEvidence({ - outputDir: params.outputDir, - repoRoot: params.repoRoot, - scenario: params.scenario, - }); + let producerEvidenceResult: Pick; + try { + producerEvidenceResult = await readScriptProducerEvidence({ + outputDir: params.outputDir, + repoRoot: params.repoRoot, + scenario: params.scenario, + producerEvidenceStartedAtMs, + }); + } catch (error) { + if (result.status !== "pass") { + return result; + } + return { + ...result, + failureMessage: `Script producer evidence is invalid: ${formatErrorMessage(error)}`, + status: "fail" as const, + }; + } if (!producerEvidenceResult.producerEvidence) { + if (requiresProducerEvidence && result.status === "pass") { + return { + ...result, + failureMessage: "Script exited successfully without writing fresh producer QA evidence.", + status: "fail" as const, + }; + } return result; } if (result.status !== "pass") { @@ -322,7 +424,10 @@ function statusFromProducerEvidence(params: { }): Pick { const { allowBlockedEvidence, producerEvidence } = params; if (!producerEvidence || producerEvidence.entries.length === 0) { - return { status: "pass" }; + return { + failureMessage: "Script exited successfully without reporting an executed producer check.", + status: "fail", + }; } const blockingEntry = producerEvidence.entries.find( (entry) => @@ -446,107 +551,6 @@ function buildTestFileEvidence(params: { }); } -async function readJsonFileIfExists(filePath: string): Promise { - let text: string; - try { - text = await fs.readFile(filePath, "utf8"); - } catch (error) { - if ( - error && - typeof error === "object" && - "code" in error && - (error as { code?: unknown }).code === "ENOENT" - ) { - return undefined; - } - throw error; - } - try { - return JSON.parse(text) as unknown; - } catch (error) { - throw new Error(`invalid JSON in ${filePath}: ${formatErrorMessage(error)}`, { cause: error }); - } -} - -// Producer artifact paths follow one convention: relative paths resolve against the -// qa-evidence.json directory, absolute paths are taken as-is. Paths under the repo root -// become repo-relative; paths outside it stay absolute so downstream consumers never see -// `../` segments that would read as path traversal. -function resolveScriptProducerArtifactPath(params: { - evidenceDir: string; - repoRoot: string; - artifactPath: string; -}) { - const absolutePath = path.isAbsolute(params.artifactPath) - ? params.artifactPath - : path.join(params.evidenceDir, params.artifactPath); - const repoRelativePath = toRepoRelativePath(params.repoRoot, absolutePath); - return isRepoRootRelativeRef(repoRelativePath) ? repoRelativePath : path.normalize(absolutePath); -} - -function normalizeScriptProducerEvidence(params: { - evidence: QaEvidenceSummaryJson; - evidencePath: string; - repoRoot: string; -}): QaEvidenceSummaryJson { - // Input is already validated by the caller; this only rewrites artifact path strings, - // so the transformed shape stays schema-valid without re-parsing. - const evidenceDir = path.dirname(params.evidencePath); - return { - ...params.evidence, - entries: params.evidence.entries.map((entry) => ({ - ...entry, - execution: entry.execution - ? { - ...entry.execution, - artifacts: entry.execution.artifacts.map((artifact) => ({ - ...artifact, - path: resolveScriptProducerArtifactPath({ - artifactPath: artifact.path, - evidenceDir, - repoRoot: params.repoRoot, - }), - })), - } - : undefined, - })), - }; -} - -async function readScriptProducerEvidence(params: { - outputDir: string; - repoRoot: string; - scenario: QaTestFileScenario; -}): Promise> { - const scenarioOutputDir = path.join(params.outputDir, params.scenario.id); - const latestRun = (await readJsonFileIfExists( - path.join(scenarioOutputDir, "latest-run.json"), - )) as { qaEvidence?: unknown } | undefined; - const candidates = [ - typeof latestRun?.qaEvidence === "string" ? latestRun.qaEvidence : undefined, - path.join(scenarioOutputDir, QA_EVIDENCE_FILENAME), - ].filter((candidate): candidate is string => Boolean(candidate)); - - for (const candidate of candidates) { - const evidencePath = path.isAbsolute(candidate) - ? candidate - : path.join(scenarioOutputDir, candidate); - const rawEvidence = await readJsonFileIfExists(evidencePath); - if (!rawEvidence) { - continue; - } - const evidence = validateQaEvidenceSummaryJson(rawEvidence); - return { - producerEvidence: normalizeScriptProducerEvidence({ - evidence, - evidencePath, - repoRoot: params.repoRoot, - }), - }; - } - return {}; -} - function buildScenarioArtifactPaths(params: { repoRoot: string; results: readonly QaTestFileScenarioResult[]; diff --git a/extensions/qa-lab/src/test-file-scenario-script-evidence.ts b/extensions/qa-lab/src/test-file-scenario-script-evidence.ts new file mode 100644 index 000000000000..bd53c114101a --- /dev/null +++ b/extensions/qa-lab/src/test-file-scenario-script-evidence.ts @@ -0,0 +1,153 @@ +import fs from "node:fs/promises"; +import path from "node:path"; +import { formatErrorMessage } from "openclaw/plugin-sdk/error-runtime"; +import { isRepoRootRelativeRef, toRepoRelativePath } from "./cli-paths.js"; +import { + QA_EVIDENCE_FILENAME, + type QaEvidenceSummaryJson, + validateQaEvidenceSummaryJson, +} from "./evidence-summary.js"; + +export async function readJsonFileIfExists(filePath: string): Promise { + let text: string; + try { + text = await fs.readFile(filePath, "utf8"); + } catch (error) { + if ( + error && + typeof error === "object" && + "code" in error && + (error as { code?: unknown }).code === "ENOENT" + ) { + return undefined; + } + throw error; + } + try { + return JSON.parse(text) as unknown; + } catch (error) { + throw new Error(`invalid JSON in ${filePath}: ${formatErrorMessage(error)}`, { cause: error }); + } +} + +// Producer artifact paths resolve against their evidence bundle. External +// artifacts remain absolute so consumers never receive traversal segments. +function resolveScriptProducerArtifactPath(params: { + evidenceDir: string; + repoRoot: string; + artifactPath: string; +}) { + const absolutePath = path.isAbsolute(params.artifactPath) + ? params.artifactPath + : path.join(params.evidenceDir, params.artifactPath); + const repoRelativePath = toRepoRelativePath(params.repoRoot, absolutePath); + return isRepoRootRelativeRef(repoRelativePath) ? repoRelativePath : path.normalize(absolutePath); +} + +function normalizeScriptProducerEvidence(params: { + evidence: QaEvidenceSummaryJson; + evidencePath: string; + repoRoot: string; +}): QaEvidenceSummaryJson { + const evidenceDir = path.dirname(params.evidencePath); + return { + ...params.evidence, + entries: params.evidence.entries.map((entry) => ({ + ...entry, + execution: entry.execution + ? { + ...entry.execution, + artifacts: entry.execution.artifacts.map((artifact) => ({ + ...artifact, + path: resolveScriptProducerArtifactPath({ + artifactPath: artifact.path, + evidenceDir, + repoRoot: params.repoRoot, + }), + })), + } + : undefined, + })), + }; +} + +function assertScenarioOwnsEvidencePath(scenarioOutputDir: string, evidencePath: string): void { + const relativePath = path.relative(scenarioOutputDir, evidencePath); + if ( + relativePath === ".." || + relativePath.startsWith(`..${path.sep}`) || + path.isAbsolute(relativePath) + ) { + throw new Error("producer evidence must remain inside its scenario output directory"); + } +} + +export async function readScriptProducerEvidence(params: { + outputDir: string; + producerEvidenceStartedAtMs?: number; + repoRoot: string; + scenario: { id: string }; +}): Promise<{ producerEvidence?: QaEvidenceSummaryJson }> { + const scenarioOutputDir = path.join(params.outputDir, params.scenario.id); + const latestRun = await readJsonFileIfExists(path.join(scenarioOutputDir, "latest-run.json")); + if ( + params.producerEvidenceStartedAtMs !== undefined && + latestRun !== undefined && + (latestRun === null || + typeof latestRun !== "object" || + !("qaEvidence" in latestRun) || + typeof latestRun.qaEvidence !== "string" || + latestRun.qaEvidence.trim().length === 0) + ) { + throw new Error("latest-run.json does not identify a producer evidence bundle"); + } + const latestEvidencePath = + latestRun !== null && + typeof latestRun === "object" && + "qaEvidence" in latestRun && + typeof latestRun.qaEvidence === "string" + ? latestRun.qaEvidence + : undefined; + const candidates = [ + latestEvidencePath, + path.join(scenarioOutputDir, QA_EVIDENCE_FILENAME), + ].filter((candidate): candidate is string => Boolean(candidate)); + + for (const candidate of candidates) { + const evidencePath = path.isAbsolute(candidate) + ? candidate + : path.join(scenarioOutputDir, candidate); + if (params.producerEvidenceStartedAtMs !== undefined) { + assertScenarioOwnsEvidencePath(scenarioOutputDir, evidencePath); + const evidenceStat = await fs.stat(evidencePath).catch((error: unknown) => { + if ((error as NodeJS.ErrnoException).code === "ENOENT") { + return undefined; + } + throw error; + }); + if (!evidenceStat) { + continue; + } + if (evidenceStat.mtimeMs < params.producerEvidenceStartedAtMs) { + throw new Error("producer evidence was not written by the current scenario run"); + } + assertScenarioOwnsEvidencePath( + await fs.realpath(scenarioOutputDir), + await fs.realpath(evidencePath), + ); + } + const rawEvidence = await readJsonFileIfExists(evidencePath); + if (!rawEvidence) { + continue; + } + const evidence = validateQaEvidenceSummaryJson(rawEvidence); + return { + producerEvidence: normalizeScriptProducerEvidence({ + evidence, + evidencePath, + repoRoot: params.repoRoot, + }), + }; + } + return {}; +} diff --git a/extensions/slack/src/config-schema.test.ts b/extensions/slack/src/config-schema.test.ts index 199f719841d6..9ef968074acb 100644 --- a/extensions/slack/src/config-schema.test.ts +++ b/extensions/slack/src/config-schema.test.ts @@ -299,6 +299,18 @@ describe("slack config schema", () => { ); }); + it.each(["http", "relay"] as const)( + "does not require %s transport credentials when Slack is disabled", + (mode) => { + expectSlackConfigValid({ enabled: false, mode }); + expectSlackConfigValid({ + enabled: false, + mode, + accounts: { ops: { mode } }, + }); + }, + ); + it("accepts per-channel replyToMode", () => { expectSlackConfigValid({ channels: { @@ -372,6 +384,89 @@ describe("slack config schema", () => { expectSlackConfigIssue({ mode: "http" }, "signingSecret"); }); + it("rejects implicit account HTTP mode without signing secret", () => { + expectSlackConfigIssue({ mode: "http", accounts: {} }, "signingSecret"); + }); + + it("accepts inherited account HTTP mode with an account signing secret", () => { + expectSlackConfigValid({ + mode: "http", + accounts: { + ops: { + botToken: "test-bot-token", + signingSecret: "test-ops-signing-secret", + webhookPath: "/slack/events/ops", + }, + }, + }); + }); + + it("accepts inherited account HTTP mode with a signing secret SecretRef", () => { + expectSlackConfigValid({ + mode: "http", + accounts: { + ops: { + botToken: "test-bot-token", + signingSecret: { + source: "env", + provider: "default", + id: "SLACK_OPS_SIGNING_SECRET", + }, + webhookPath: "/slack/events/ops", + }, + }, + }); + }); + + it("accepts independently signed accounts inheriting HTTP mode", () => { + expectSlackConfigValid({ + mode: "http", + accounts: { + ops: { + botToken: "test-ops-bot-token", + signingSecret: "test-ops-signing-secret", + webhookPath: "/slack/events/ops", + }, + support: { + botToken: "test-support-bot-token", + signingSecret: "test-support-signing-secret", + webhookPath: "/slack/events/support", + }, + }, + }); + }); + + it("skips disabled accounts inheriting HTTP mode", () => { + expectSlackConfigValid({ + mode: "http", + accounts: { + disabled: { enabled: false }, + ops: { + botToken: "test-bot-token", + signingSecret: "test-ops-signing-secret", + }, + }, + }); + expectSlackConfigValid({ + mode: "http", + accounts: { ops: { enabled: false } }, + }); + }); + + it("reports a missing inherited HTTP signing secret on its account only", () => { + const result = SlackConfigSchema.safeParse({ + mode: "http", + accounts: { ops: { botToken: "test-bot-token" } }, + }); + + expect(result.success).toBe(false); + if (!result.success) { + expect(result.error.issues.map((issue) => issue.path.join("."))).toEqual([ + "accounts.ops.signingSecret", + ]); + } + }); + it("accepts account HTTP mode when base signing secret is set", () => { expectSlackConfigValid({ signingSecret: "test-signing-secret", diff --git a/extensions/slack/src/config-schema.ts b/extensions/slack/src/config-schema.ts index 12b51da7c056..9a62d877caaa 100644 --- a/extensions/slack/src/config-schema.ts +++ b/extensions/slack/src/config-schema.ts @@ -166,7 +166,14 @@ function validateSlackSigningSecretRequirements( const resolveMode = (mode: unknown) => mode === "http" || mode === "socket" || mode === "relay" ? mode : undefined; const baseMode = resolveMode(value.mode) ?? "socket"; - if (baseMode === "http" && !hasConfiguredSecretInput(value.signingSecret)) { + // Named accounts own their inherited HTTP credentials; only an implicit + // default account needs a separate root signing secret. + const hasImplicitRootAccount = Object.keys(value.accounts ?? {}).length === 0; + if ( + baseMode === "http" && + hasImplicitRootAccount && + !hasConfiguredSecretInput(value.signingSecret) + ) { ctx.addIssue({ code: z.ZodIssueCode.custom, message: 'channels.slack.mode="http" requires channels.slack.signingSecret', @@ -201,6 +208,10 @@ export const SlackConfigSchema = SlackAccountSchema.safeExtend({ accounts: z.record(z.string(), SlackAccountEntrySchema.optional()).optional(), defaultAccount: z.string().optional(), }).superRefine((value, ctx) => { + if (value.enabled === false) { + return; + } + const dmPolicy = value.dmPolicy ?? "pairing"; const allowFrom = value.allowFrom; requireOpenAllowFrom({ diff --git a/qa/scenarios/agents/subagent-fanout-synthesis.yaml b/qa/scenarios/agents/subagent-fanout-synthesis.yaml index 753161c8a052..3011a271345e 100644 --- a/qa/scenarios/agents/subagent-fanout-synthesis.yaml +++ b/qa/scenarios/agents/subagent-fanout-synthesis.yaml @@ -110,43 +110,23 @@ flow: expr: "readSessionTranscriptSummary(env, sessionKey).then((summary) => config.expectedReplyGroups.every((group) => group.some((needle) => normalizeLowercaseStringOrEmpty(summary.finalText).includes(needle))) ? summary : undefined).catch(() => undefined)" - expr: "30000" - expr: "env.providerMode === 'mock-openai' ? 100 : 250" - - if: - # The parent reply can arrive before child transcript writes finish. - # Wait before opening the store so its integrity check sees settled FTS state. - expr: "Boolean(env.mock) && env.gateway.runtimeEnv.OPENCLAW_QA_FORCE_RUNTIME !== 'codex'" - then: - - forEach: - items: - expr: "config.expectedChildCompletionMarkers" - item: childCompletionMarker - actions: - - call: waitForOutboundMessage - args: - - ref: state - - lambda: - params: [candidate] - expr: "String(candidate.text ?? '').trim() === childCompletionMarker" - - 30000 + # Native child completions are private. Wait for this parent's + # actual SQLite rows and both settled final transcripts. + - call: waitForCondition + saveAs: completedFanout + args: + - lambda: + async: true + expr: "(async () => { const store = await readRawQaSessionStore(env); const childRows = Object.entries(store).map(([key, entry]) => ({ ...entry, key })).filter((entry) => entry.spawnedBy === sessionKey); const alpha = childRows.find((entry) => entry.label === alphaLabel); const beta = childRows.find((entry) => entry.label === betaLabel); if (!alpha || !beta) return undefined; const [alphaTranscript, betaTranscript] = await Promise.all([readSessionTranscriptSummary(env, alpha.key), readSessionTranscriptSummary(env, beta.key)]); const alphaExpected = env.mock ? config.expectedChildCompletionMarkers[0] : 'ok'; const betaExpected = env.mock ? config.expectedChildCompletionMarkers[1] : 'ok'; return normalizeLowercaseStringOrEmpty(alphaTranscript.finalText) === normalizeLowercaseStringOrEmpty(alphaExpected) && normalizeLowercaseStringOrEmpty(betaTranscript.finalText) === normalizeLowercaseStringOrEmpty(betaExpected) ? { alpha, beta, alphaTranscript, betaTranscript } : undefined; })().catch(() => undefined)" + - expr: liveTurnTimeoutMs(env, 30000) + - expr: "env.providerMode === 'mock-openai' ? 100 : 250" + - assert: + expr: "completedFanout.alpha.spawnedBy === sessionKey && completedFanout.beta.spawnedBy === sessionKey" + message: + expr: "`fanout completion must belong to parent ${sessionKey}`" - if: expr: "Boolean(env.mock)" then: - - call: readRawQaSessionStore - saveAs: store - args: - - ref: env - - set: childRows - value: - expr: "Object.values(store).filter((entry) => entry.spawnedBy === sessionKey)" - - set: sawAlpha - value: - expr: "childRows.some((entry) => entry.label === alphaLabel)" - - set: sawBeta - value: - expr: "childRows.some((entry) => entry.label === betaLabel)" - - assert: - expr: "sawAlpha && sawBeta" - message: - expr: "`fanout child sessions missing (alpha=${String(sawAlpha)} beta=${String(sawBeta)})`" # Tool-call assertion (criterion 2 of the # parity completion gate in #64227): the # scenario must have actually invoked @@ -176,77 +156,85 @@ flow: value: __done__ catchAs: attemptError catch: + - set: lastError + value: + ref: attemptError - if: expr: "/timed out after/i.test(formatErrorMessage(attemptError))" then: - - set: timeoutAlphaExpected - value: - expr: "Boolean(env.mock) ? config.expectedChildCompletionMarkers[0] : 'ok'" - - set: timeoutBetaExpected - value: - expr: "Boolean(env.mock) ? config.expectedChildCompletionMarkers[1] : 'ok'" - - call: waitForCondition - saveAs: timeoutEvidence - args: - - lambda: - async: true - expr: "(async () => { const store = await readRawQaSessionStore(env); const childEntries = Object.entries(store).map(([key, entry]) => ({ ...entry, key })).filter((entry) => entry.spawnedBy === sessionKey); const alphaEntry = childEntries.find((entry) => entry.label === alphaLabel); const betaEntry = childEntries.find((entry) => entry.label === betaLabel); if (!alphaEntry || !betaEntry) return undefined; const [alphaTranscript, betaTranscript] = await Promise.all([readSessionTranscriptSummary(env, alphaEntry.key), readSessionTranscriptSummary(env, betaEntry.key)]); const alphaOk = normalizeLowercaseStringOrEmpty(alphaTranscript.finalText) === normalizeLowercaseStringOrEmpty(timeoutAlphaExpected); const betaOk = normalizeLowercaseStringOrEmpty(betaTranscript.finalText) === normalizeLowercaseStringOrEmpty(timeoutBetaExpected); return alphaOk && betaOk ? { store, childEntries, alphaTranscript, betaTranscript } : undefined; })().catch(() => undefined)" - - expr: "30000" - - expr: "env.providerMode === 'mock-openai' ? 100 : 250" - - set: timeoutStore - value: - expr: "timeoutEvidence.store" - - set: timeoutChildEntries - value: - expr: "timeoutEvidence.childEntries" - - set: timeoutChildRows - value: - expr: "timeoutChildEntries" - - set: timeoutSawAlpha - value: - expr: "timeoutChildRows.some((entry) => entry.label === alphaLabel)" - - set: timeoutSawBeta - value: - expr: "timeoutChildRows.some((entry) => entry.label === betaLabel)" - - set: timeoutAlphaTranscript - value: - expr: "timeoutEvidence.alphaTranscript" - - set: timeoutBetaTranscript - value: - expr: "timeoutEvidence.betaTranscript" - - set: timeoutAlphaOk - value: - expr: "normalizeLowercaseStringOrEmpty(timeoutAlphaTranscript?.finalText) === normalizeLowercaseStringOrEmpty(timeoutAlphaExpected)" - - set: timeoutBetaOk - value: - expr: "normalizeLowercaseStringOrEmpty(timeoutBetaTranscript?.finalText) === normalizeLowercaseStringOrEmpty(timeoutBetaExpected)" - - if: - expr: "Boolean(env.mock)" - then: + - try: + actions: + - set: timeoutAlphaExpected + value: + expr: "Boolean(env.mock) ? config.expectedChildCompletionMarkers[0] : 'ok'" + - set: timeoutBetaExpected + value: + expr: "Boolean(env.mock) ? config.expectedChildCompletionMarkers[1] : 'ok'" + - call: waitForCondition + saveAs: timeoutEvidence + args: + - lambda: + async: true + expr: "(async () => { const store = await readRawQaSessionStore(env); const childEntries = Object.entries(store).map(([key, entry]) => ({ ...entry, key })).filter((entry) => entry.spawnedBy === sessionKey); const alphaEntry = childEntries.find((entry) => entry.label === alphaLabel); const betaEntry = childEntries.find((entry) => entry.label === betaLabel); if (!alphaEntry || !betaEntry) return undefined; const [alphaTranscript, betaTranscript] = await Promise.all([readSessionTranscriptSummary(env, alphaEntry.key), readSessionTranscriptSummary(env, betaEntry.key)]); const alphaOk = normalizeLowercaseStringOrEmpty(alphaTranscript.finalText) === normalizeLowercaseStringOrEmpty(timeoutAlphaExpected); const betaOk = normalizeLowercaseStringOrEmpty(betaTranscript.finalText) === normalizeLowercaseStringOrEmpty(timeoutBetaExpected); return alphaOk && betaOk ? { store, childEntries, alphaTranscript, betaTranscript } : undefined; })().catch(() => undefined)" + - expr: "30000" + - expr: "env.providerMode === 'mock-openai' ? 100 : 250" + - set: timeoutStore + value: + expr: "timeoutEvidence.store" + - set: timeoutChildEntries + value: + expr: "timeoutEvidence.childEntries" + - set: timeoutChildRows + value: + expr: "timeoutChildEntries" + - set: timeoutSawAlpha + value: + expr: "timeoutChildRows.some((entry) => entry.label === alphaLabel)" + - set: timeoutSawBeta + value: + expr: "timeoutChildRows.some((entry) => entry.label === betaLabel)" + - set: timeoutAlphaTranscript + value: + expr: "timeoutEvidence.alphaTranscript" + - set: timeoutBetaTranscript + value: + expr: "timeoutEvidence.betaTranscript" + - set: timeoutAlphaOk + value: + expr: "normalizeLowercaseStringOrEmpty(timeoutAlphaTranscript?.finalText) === normalizeLowercaseStringOrEmpty(timeoutAlphaExpected)" + - set: timeoutBetaOk + value: + expr: "normalizeLowercaseStringOrEmpty(timeoutBetaTranscript?.finalText) === normalizeLowercaseStringOrEmpty(timeoutBetaExpected)" - set: timeoutSpawnRequests value: - expr: "[...(await fetchJson(`${env.mock.baseUrl}/debug/requests`))].filter((request) => request.plannedToolName === 'sessions_spawn' && /subagent fanout synthesis check/i.test(String(request.allInputText ?? '')))" + expr: "[]" - if: - expr: "timeoutSawAlpha && timeoutSawBeta && timeoutAlphaOk && timeoutBetaOk && timeoutSpawnRequests.length >= 2" + expr: "Boolean(env.mock)" then: + - set: timeoutSpawnRequests + value: + expr: "[...(await fetchJson(`${env.mock.baseUrl}/debug/requests`))].filter((request) => request.plannedToolName === 'sessions_spawn' && /subagent fanout synthesis check/i.test(String(request.allInputText ?? '')))" + - if: + expr: "timeoutSawAlpha && timeoutSawBeta && timeoutAlphaOk && timeoutBetaOk && (!env.mock || timeoutSpawnRequests.length >= 2)" + then: + - call: waitForCondition + saveAs: recoveredParentTranscript + args: + - lambda: + async: true + expr: "readSessionTranscriptSummary(env, sessionKey).then((summary) => config.expectedReplyGroups.every((group) => group.some((needle) => normalizeLowercaseStringOrEmpty(summary.finalText).includes(needle))) ? summary : undefined).catch(() => undefined)" + - expr: liveTurnTimeoutMs(env, 30000) + - expr: "env.providerMode === 'mock-openai' ? 100 : 250" - set: details - value: "subagent-1: ok\nsubagent-2: ok" + value: + expr: recoveredParentTranscript.finalText - set: lastError value: __done__ - else: - - if: - expr: "timeoutSawAlpha && timeoutSawBeta && timeoutAlphaOk && timeoutBetaOk" - then: - - set: details - value: "subagent-1: ok\nsubagent-2: ok" - - set: lastError - value: __done__ - - if: - expr: "lastError !== '__done__'" - then: - - set: lastError - value: - ref: attemptError + catchAs: recoveryError + catch: + - set: lastError + value: + ref: recoveryError - if: expr: "lastError !== '__done__' && attempt < attempts" then: diff --git a/qa/scenarios/ui/control-ui-browser-screenshot-body-cancel.yaml b/qa/scenarios/ui/control-ui-browser-screenshot-body-cancel.yaml new file mode 100644 index 000000000000..60bf85149ff7 --- /dev/null +++ b/qa/scenarios/ui/control-ui-browser-screenshot-body-cancel.yaml @@ -0,0 +1,30 @@ +title: Control UI browser screenshot response-body cancellation + +scenario: + id: control-ui-browser-screenshot-body-cancel + surface: control-ui + coverage: + secondary: + - control-ui.gateway-hosted-ui-control + objective: >- + Verify the rendered browser panel keeps a failed screenshot visible while + canceling the unread authenticated assistant-media response body. + successCriteria: + - Chromium opens the Control UI and renders the browser panel. + - The browser panel displays the screenshot HTTP failure. + - The unread assistant-media response body is canceled exactly once. + - Browser tab and screenshot requests retain the expected target and authentication. + docsRefs: + - docs/web/control-ui.md + - docs/tools/browser.md + codeRefs: + - ui/src/e2e/browser-screenshot-body-cancel.e2e.test.ts + - ui/src/components/browser/browser-client.ts + - ui/src/test-helpers/control-ui-e2e.ts + execution: + kind: playwright + path: ui/src/e2e/browser-screenshot-body-cancel.e2e.test.ts + testNamePattern: keeps the status error visible and cancels the unread media body + summary: >- + Real Chromium coverage for screenshot failure visibility, authenticated + browser requests, and unread response-body cancellation. diff --git a/scripts/e2e/npm-telegram-live-runner.ts b/scripts/e2e/npm-telegram-live-runner.ts index c89d3ae15d91..755bfc811580 100644 --- a/scripts/e2e/npm-telegram-live-runner.ts +++ b/scripts/e2e/npm-telegram-live-runner.ts @@ -105,9 +105,9 @@ async function shouldFailPackageTelegramRun( if (parseBoolean(env.OPENCLAW_NPM_TELEGRAM_ALLOW_FAILURES)) { return false; } - const { readQaSuiteFailedScenarioCountFromFile } = + const { readQaSuiteFailedOrSkippedScenarioCountFromFile } = await import("../../extensions/qa-lab/src/suite-summary.ts"); - return (await readQaSuiteFailedScenarioCountFromFile(result.summaryPath)) > 0; + return (await readQaSuiteFailedOrSkippedScenarioCountFromFile(result.summaryPath)) > 0; } async function resolveTrustedOpenClawCommand( diff --git a/scripts/lib/plugin-npm-package-manifest.mjs b/scripts/lib/plugin-npm-package-manifest.mjs index f38f02126f6e..af9453917766 100644 --- a/scripts/lib/plugin-npm-package-manifest.mjs +++ b/scripts/lib/plugin-npm-package-manifest.mjs @@ -115,6 +115,59 @@ function assertPluginNpmRuntimeBuildExists(plan) { assertPackageFilesDoNotExcludeRequiredRuntimeArtifacts(plan); } +function resolvePackagedChannelStateMetadata(metadata, metadataKey, plan) { + if ( + !metadata || + typeof metadata !== "object" || + Array.isArray(metadata) || + typeof metadata.specifier !== "string" || + !metadata.specifier.trim() + ) { + return metadata; + } + + const normalizedSpecifier = normalizePackPath(metadata.specifier); + const sourceEntry = normalizedSpecifier.replace(/\.(?:[cm]?[jt]s)$/u, ""); + const runtimeSpecifier = plan.runtimeBuildOutputs.find((runtimePath) => { + const normalizedRuntimePath = normalizePackPath(runtimePath); + return ( + normalizedRuntimePath === normalizedSpecifier || + normalizedRuntimePath.replace(/^dist\//u, "").replace(/\.(?:[cm]?js)$/u, "") === sourceEntry + ); + }); + if (!runtimeSpecifier) { + throw new Error( + `channel ${metadataKey} specifier '${metadata.specifier}' has no package-local runtime output for ${plan.pluginDir}`, + ); + } + + // Published plugins omit source files; installed channel probes must load + // the exact ESM or CommonJS sidecar emitted by the package runtime build. + return { + ...metadata, + specifier: runtimeSpecifier, + }; +} + +function resolvePackagedChannelMetadata(plan) { + const channel = plan.packageJson.openclaw?.channel; + if (!channel || typeof channel !== "object" || Array.isArray(channel)) { + return channel; + } + + const packagedChannel = { ...channel }; + for (const metadataKey of ["configuredState", "persistedAuthState"]) { + if (Object.hasOwn(channel, metadataKey)) { + packagedChannel[metadataKey] = resolvePackagedChannelStateMetadata( + channel[metadataKey], + metadataKey, + plan, + ); + } + } + return packagedChannel; +} + function hasPackageRuntimeDependencies(packageJson) { return ( Object.keys(packageJson.dependencies ?? {}).length > 0 || @@ -449,6 +502,7 @@ export function resolveAugmentedPluginNpmPackageJson(params) { } assertPluginNpmRuntimeBuildExists(plan); + const packagedChannel = resolvePackagedChannelMetadata(plan); const packageJson = { ...plan.packageJson, files: plan.packageFiles, @@ -456,6 +510,7 @@ export function resolveAugmentedPluginNpmPackageJson(params) { peerDependenciesMeta: plan.packagePeerMetadata.peerDependenciesMeta, openclaw: { ...plan.packageJson.openclaw, + ...(packagedChannel ? { channel: packagedChannel } : {}), runtimeExtensions: plan.runtimeExtensions, ...(plan.runtimeSetupEntry ? { diff --git a/scripts/test-docker-all.mjs b/scripts/test-docker-all.mjs index 8a14643f907c..edc68f5f39ac 100644 --- a/scripts/test-docker-all.mjs +++ b/scripts/test-docker-all.mjs @@ -49,9 +49,17 @@ const SHELL_PROCESS_GROUP_EXIT_POLL_MS = 25; const MAX_TIMER_TIMEOUT_MS = 2_147_000_000; const DEFAULT_TIMINGS_FILE = path.join(ROOT_DIR, ".artifacts/docker-tests/lane-timings.json"); const DEFAULT_GITHUB_WORKFLOW = "openclaw-live-and-e2e-checks-reusable.yml"; -const IS_MAIN = process.argv[1] - ? path.resolve(process.argv[1]) === fileURLToPath(import.meta.url) - : false; +const IS_MAIN = (() => { + if (!process.argv[1]) { + return false; + } + try { + // Node resolves ESM URLs through symlinks, but argv keeps the invoked path. + return fs.realpathSync(process.argv[1]) === fs.realpathSync(fileURLToPath(import.meta.url)); + } catch { + return false; + } +})(); function dockerAllUsage() { return [ @@ -1608,6 +1616,8 @@ async function main() { return; } + // Planning can report unsupported scenarios, but execution cannot pass when + // frozen-target omissions leave no selected lane to run. if (scheduledLanes.length === 0) { await writeSummary({ chunk: releaseChunk || undefined, @@ -1618,9 +1628,11 @@ async function main() { profile, selectedLanes: selectedLaneNames.length > 0 ? selectedLaneNames : undefined, startedAt: runStartedAt, - status: "passed", + status: "failed", }); - return; + throw new Error( + `resolved zero runnable Docker lanes; frozen target does not support: ${omittedUnsupportedLanes.join(", ")}`, + ); } await runPhase( diff --git a/src/agents/main-session-recovery-store.ts b/src/agents/main-session-recovery-store.ts index c0558bb32b54..a5e9c0e6dcd6 100644 --- a/src/agents/main-session-recovery-store.ts +++ b/src/agents/main-session-recovery-store.ts @@ -113,6 +113,7 @@ export async function commitMainSessionRecovery(params: { expectedSessionId?: string; requireWriteSuccess?: boolean; scanAliases?: boolean; + shouldContinue?: () => boolean; target: MainSessionRecoveryStoreTarget; }): Promise { const cancellation = @@ -141,6 +142,15 @@ export async function commitMainSessionRecovery(params: { ...(scansAliases ? {} : { sessionKeys: [params.target.sessionKey] }), storePath: params.target.storePath, update: (entries) => { + // Recheck inside the synchronous commit: shutdown can begin while this + // recovery owner is waiting to acquire the session-store transaction. + if (params.shouldContinue?.() === false) { + return { + result: { + transition: { kind: "rejected", reason: "stale_generation" }, + }, + }; + } const expectedGeneration = currentGenerationRequiredBy(params.command); if (expectedGeneration && expectedGeneration !== getAgentEventLifecycleGeneration()) { return { diff --git a/src/agents/main-session-restart-dispatch.ts b/src/agents/main-session-restart-dispatch.ts index 32344ffaf144..a17c8ac7533b 100644 --- a/src/agents/main-session-restart-dispatch.ts +++ b/src/agents/main-session-restart-dispatch.ts @@ -196,6 +196,7 @@ async function settleRestartRecoveryDispatch(params: { expectedRecoverySourceRunId?: string; expectedSessionId: string; sessionKeys: readonly string[]; + shouldContinue?: () => boolean; storePath: string; terminalStatus?: RestartRecoveryTerminalStatus; }): Promise { @@ -203,6 +204,9 @@ async function settleRestartRecoveryDispatch(params: { sessionKeys: params.sessionKeys, storePath: params.storePath, update: (entries) => { + if (params.shouldContinue?.() === false) { + return { result: undefined }; + } const current = entries .filter( ({ entry }) => @@ -350,8 +354,14 @@ export async function resumeMainSession(params: { pendingFinalDeliveryText?: string | null; forceRestartSafeTools?: boolean; sessionWorkAdmissionHandoffId?: string; + lifecycleGeneration?: string; + shouldContinue?: () => boolean; gatewayRuntime: GatewayRecoveryRuntime; }): Promise { + if (params.shouldContinue?.() === false) { + return "skipped"; + } + const lifecycleGeneration = params.lifecycleGeneration ?? getAgentEventLifecycleGeneration(); const sanitizedPendingText = typeof params.pendingFinalDeliveryText === "string" ? sanitizePendingFinalDeliveryText(params.pendingFinalDeliveryText) @@ -381,24 +391,38 @@ export async function resumeMainSession(params: { command: { kind: "prepare_attempt", attempt: params.recoveryAttempt, - lifecycleGeneration: getAgentEventLifecycleGeneration(), + lifecycleGeneration, now: Date.now(), observation: params.observation, runId: recoveryRunId, }, requireWriteSuccess: true, + shouldContinue: params.shouldContinue, target: { sessionKey: params.sessionKey, storePath: params.storePath }, }); if (reserved.transition.kind !== "reserved") { return "skipped"; } reservation = reserved.transition.reservation; + if (params.shouldContinue?.() === false) { + await rollbackRestartRecoveryReservation({ + kind: "cancel_reservation", + reservation, + sessionKey: params.sessionKey, + storePath: params.storePath, + }); + reservation = undefined; + return "skipped"; + } // Persist one stable RPC id before dispatch. A transport rejection is // ambiguous; retries must reuse this id so accepted work cannot duplicate. const recoveryStatePrepared = await applySessionEntryReplacements({ sessionKeys: [params.sessionKey], storePath: params.storePath, update: (entries) => { + if (params.shouldContinue?.() === false) { + return { result: false }; + } const current = entries.find((entry) => entry.sessionKey === params.sessionKey); const entry = current?.entry; if ( @@ -430,6 +454,9 @@ export async function resumeMainSession(params: { storePath: params.storePath, }); reservation = undefined; + if (params.shouldContinue?.() === false) { + return "skipped"; + } const current = rollback.entry; return current?.sessionId === params.entry.sessionId && current.status === "running" && @@ -472,6 +499,16 @@ export async function resumeMainSession(params: { agentParams.threadId = String(deliveryContext.threadId); } } + if (params.shouldContinue?.() === false) { + await rollbackRestartRecoveryReservation({ + kind: "cancel_reservation", + reservation, + sessionKey: params.sessionKey, + storePath: params.storePath, + }); + reservation = undefined; + return "skipped"; + } if (params.forceRestartSafeTools) { log.info(`dispatching restart-safe recovery for ${params.sessionKey}`); } @@ -480,6 +517,11 @@ export async function resumeMainSession(params: { runId: string; status?: unknown; }>(agentParams, 10_000); + if (params.shouldContinue?.() === false) { + // The accepted run belongs to its original Gateway; never let a stopped + // owner settle or transfer that durable claim into a new lifecycle. + return "skipped"; + } // Real Gateway admission consumes the reservation before returning accepted. // Recovery-runtime fakes may return directly, so keep this idempotent fallback // to make the durable acceptance boundary explicit in focused tests too. @@ -490,7 +532,9 @@ export async function resumeMainSession(params: { params.gatewayRuntime, ); } - const lifecycleGeneration = getAgentEventLifecycleGeneration(); + if (params.shouldContinue?.() === false) { + return "skipped"; + } const admission = await commitMainSessionRecovery({ command: { kind: "admit_recovery", @@ -499,6 +543,7 @@ export async function resumeMainSession(params: { runId: recoveryRunId, sessionId: params.entry.sessionId, }, + shouldContinue: params.shouldContinue, target: { sessionKey: params.sessionKey, storePath: params.storePath }, }); if ( @@ -513,14 +558,21 @@ export async function resumeMainSession(params: { ) { throw new Error(`restart recovery admission changed before settlement: ${params.sessionKey}`); } + if (params.shouldContinue?.() === false) { + return "skipped"; + } await settleRestartRecoveryDispatch({ expectedRecoveryRunId: recoveryRunId, expectedRecoverySourceRunId: sourceRunId, expectedSessionId: params.entry.sessionId, sessionKeys: recoverySessionKeys, + shouldContinue: params.shouldContinue, storePath: params.storePath, terminalStatus, }); + if (params.shouldContinue?.() === false) { + return "skipped"; + } log.info( `resumed interrupted main session: ${params.sessionKey}${ sanitizedPendingText ? " (with pending payload)" : "" @@ -530,13 +582,12 @@ export async function resumeMainSession(params: { } catch (error) { const explicitlyRejected = error instanceof GatewayClientRequestError; try { - if (dispatchStarted && !explicitlyRejected) { + if (dispatchStarted && !explicitlyRejected && params.shouldContinue?.() !== false) { const terminalStatus = await probeRestartRecoveryTerminalStatus( recoveryRunId, params.gatewayRuntime, ); - if (terminalStatus) { - const lifecycleGeneration = getAgentEventLifecycleGeneration(); + if (terminalStatus && params.shouldContinue?.() !== false) { const admission = await commitMainSessionRecovery({ command: { kind: "admit_recovery", @@ -545,6 +596,7 @@ export async function resumeMainSession(params: { runId: recoveryRunId, sessionId: params.entry.sessionId, }, + shouldContinue: params.shouldContinue, target: { sessionKey: params.sessionKey, storePath: params.storePath }, }); const exactRunAlreadyAdmitted = isExactRestartRecoveryDispatchAdmission({ @@ -555,60 +607,81 @@ export async function resumeMainSession(params: { terminalStatus, }); if (admission.transition.kind !== "admitted_recovery" && !exactRunAlreadyAdmitted) { - log.warn(`restart recovery admission changed before settlement: ${params.sessionKey}`); - } else { + if (params.shouldContinue?.() !== false) { + log.warn( + `restart recovery admission changed before settlement: ${params.sessionKey}`, + ); + } + } else if (params.shouldContinue?.() !== false) { if (reservation) { await commitMainSessionRecovery({ command: { kind: "abandon_reservation", reservation }, target: { sessionKey: params.sessionKey, storePath: params.storePath }, }); } - await settleRestartRecoveryDispatch({ - expectedRecoveryRunId: recoveryRunId, - expectedRecoverySourceRunId: sourceRunId, - expectedSessionId: params.entry.sessionId, - sessionKeys: recoverySessionKeys, - storePath: params.storePath, - terminalStatus, - }); - log.info(`settled completed restart recovery for ${params.sessionKey}`); - return "resumed"; + if (params.shouldContinue?.() !== false) { + await settleRestartRecoveryDispatch({ + expectedRecoveryRunId: recoveryRunId, + expectedRecoverySourceRunId: sourceRunId, + expectedSessionId: params.entry.sessionId, + sessionKeys: recoverySessionKeys, + shouldContinue: params.shouldContinue, + storePath: params.storePath, + terminalStatus, + }); + if (params.shouldContinue?.() !== false) { + log.info(`settled completed restart recovery for ${params.sessionKey}`); + return "resumed"; + } + } } } } } catch (settlementError) { - log.warn( - `failed to settle ambiguous restart recovery ${params.sessionKey}: ${String(settlementError)}`, - ); - const restoreAdmittedRecovery: RestoreAdmittedRecovery = async () => { - const restored = await commitMainSessionRecovery({ - command: { - kind: "mark_admitted_recovery_interrupted", - lifecycleGeneration: getAgentEventLifecycleGeneration(), - now: Date.now(), - runId: recoveryRunId, - sessionId: params.entry.sessionId, - }, - requireWriteSuccess: true, - target: { sessionKey: params.sessionKey, storePath: params.storePath }, - }); - return restored.transition.kind === "applied" && restored.entry && restored.sessionKey - ? { - sessionId: restored.entry.sessionId, - sessionKey: restored.sessionKey, - storePath: params.storePath, - } - : undefined; - }; - try { - scheduleMainSessionRecoveryPendingTarget( - await restoreAdmittedRecoveryWithRetries(restoreAdmittedRecovery), - ); - } catch (restoreError) { + if (params.shouldContinue?.() !== false) { log.warn( - `failed to restore ambiguous restart recovery ${params.sessionKey}: ${String(restoreError)}`, + `failed to settle ambiguous restart recovery ${params.sessionKey}: ${String(settlementError)}`, ); - scheduleAdmittedRecoveryRestore(restoreAdmittedRecovery); + const restoreAdmittedRecovery: RestoreAdmittedRecovery = async () => { + if (params.shouldContinue?.() === false) { + return undefined; + } + const restored = await commitMainSessionRecovery({ + command: { + kind: "mark_admitted_recovery_interrupted", + lifecycleGeneration, + now: Date.now(), + runId: recoveryRunId, + sessionId: params.entry.sessionId, + }, + requireWriteSuccess: true, + shouldContinue: params.shouldContinue, + target: { sessionKey: params.sessionKey, storePath: params.storePath }, + }); + return params.shouldContinue?.() !== false && + restored.transition.kind === "applied" && + restored.entry && + restored.sessionKey + ? { + sessionId: restored.entry.sessionId, + sessionKey: restored.sessionKey, + storePath: params.storePath, + } + : undefined; + }; + try { + const restored = await restoreAdmittedRecoveryWithRetries(restoreAdmittedRecovery); + if (params.shouldContinue?.() !== false) { + scheduleMainSessionRecoveryPendingTarget(restored); + } + } catch (restoreError) { + if (params.shouldContinue?.() !== false) { + log.warn( + `failed to restore ambiguous restart recovery ${params.sessionKey}: ${String(restoreError)}`, + ); + scheduleAdmittedRecoveryRestore(restoreAdmittedRecovery); + } + } } } if (reservation) { @@ -631,6 +704,9 @@ export async function resumeMainSession(params: { }); }); } + if (params.shouldContinue?.() === false) { + return "skipped"; + } log.warn( `failed to resume interrupted main session ${params.sessionKey}: ${error instanceof Error ? (error.stack ?? error.message) : String(error)}`, ); diff --git a/src/agents/main-session-restart-recovery-resume-policy.ts b/src/agents/main-session-restart-recovery-resume-policy.ts index 9e961366f583..2694d76dcef2 100644 --- a/src/agents/main-session-restart-recovery-resume-policy.ts +++ b/src/agents/main-session-restart-recovery-resume-policy.ts @@ -453,5 +453,10 @@ export function resolveMainSessionResumePolicy( reason: "transcript tail is a stale approval-pending tool result", }; } - return { action: "resume", forceRestartSafeTools: false }; + // A later tool result can hide the checkpoint at the transcript tail; keep + // the interrupted turn restricted without borrowing an earlier turn's state. + return { + action: "resume", + forceRestartSafeTools: hasReplaySafeCodeModeCheckpointInCurrentTurn(messages), + }; } diff --git a/src/agents/main-session-restart-recovery-runtime.ts b/src/agents/main-session-restart-recovery-runtime.ts index 290c3510af1e..58fb25f2dff2 100644 --- a/src/agents/main-session-restart-recovery-runtime.ts +++ b/src/agents/main-session-restart-recovery-runtime.ts @@ -1,5 +1,9 @@ import type { OpenClawConfig } from "../config/types.openclaw.js"; import type { GatewayRecoveryRuntime } from "../gateway/server-instance-runtime.types.js"; +import { + getAgentEventLifecycleGeneration, + isAgentEventLifecycleGenerationCurrent, +} from "../infra/agent-events.js"; import { runWithGatewayIndependentRootWorkAdmission } from "../process/gateway-work-admission.js"; import { beginSessionWorkAdmission, @@ -31,12 +35,17 @@ async function recoverRestartAbortedMainSessionsWithOptions(params: { resumedSessionKeys?: Set; activeSessionIds?: Iterable; activeSessionKeys?: Iterable; + lifecycleGeneration?: string; + shouldContinue?: () => boolean; gatewayRuntime: GatewayRecoveryRuntime; }): Promise<{ recovered: number; failed: number; skipped: number }> { const result = { recovered: 0, failed: 0, skipped: 0 }; const resumedSessionKeys = params.resumedSessionKeys ?? new Set(); for (const storePath of await resolveRestartRecoveryStorePaths(params)) { + if (params.shouldContinue?.() === false) { + return result; + } const storeResult = await recoverStore({ cfg: params.cfg, onExhaustedTarget: params.onExhaustedTarget, @@ -44,6 +53,8 @@ async function recoverRestartAbortedMainSessionsWithOptions(params: { resumedSessionKeys, activeSessionIds: params.activeSessionIds, activeSessionKeys: params.activeSessionKeys, + lifecycleGeneration: params.lifecycleGeneration, + shouldContinue: params.shouldContinue, gatewayRuntime: params.gatewayRuntime, }); result.recovered += storeResult.recovered; @@ -141,8 +152,10 @@ async function recoverExpectedRestartRecoveryTarget(params: { canonicalSessionKey?: string; cfg?: OpenClawConfig; expectedSessionId: string; + lifecycleGeneration?: string; observationOnly?: boolean; sessionKey: string; + shouldContinue?: () => boolean; storePath: string; gatewayRuntime: GatewayRecoveryRuntime; }): Promise<{ recovered: number; failed: number; skipped: number }> { @@ -180,6 +193,8 @@ async function recoverExpectedRestartRecoveryTarget(params: { resumedSessionKeys: new Set(), expectedTarget, sessionWorkAdmissionHandoffId: handoffId, + lifecycleGeneration: params.lifecycleGeneration, + shouldContinue: params.shouldContinue, gatewayRuntime: params.gatewayRuntime, }), ); @@ -263,8 +278,13 @@ async function recoverStartupOrphanedMainSessionsWithOptions(params: { updatedBeforeMs?: number; resumedSessionKeys?: Set; onExhaustedTarget?: (target: ExhaustedRestartRecoveryTarget) => void; + lifecycleGeneration?: string; + shouldContinue?: () => boolean; gatewayRuntime: GatewayRecoveryRuntime; }): Promise<{ marked: number; recovered: number; failed: number; skipped: number }> { + if (params.shouldContinue?.() === false) { + return { marked: 0, recovered: 0, failed: 0, skipped: 0 }; + } const startupRecoveryCutoffMs = params.updatedBeforeMs ?? Date.now(); const marked = await markStartupOrphanedMainSessionsForRecovery({ cfg: params.cfg, @@ -273,6 +293,9 @@ async function recoverStartupOrphanedMainSessionsWithOptions(params: { activeSessionKeys: params.activeSessionKeys, updatedBeforeMs: startupRecoveryCutoffMs, }); + if (params.shouldContinue?.() === false) { + return { marked: marked.marked, recovered: 0, failed: 0, skipped: marked.skipped }; + } const recovered = await recoverRestartAbortedMainSessionsWithOptions({ cfg: params.cfg, onExhaustedTarget: params.onExhaustedTarget, @@ -280,6 +303,8 @@ async function recoverStartupOrphanedMainSessionsWithOptions(params: { resumedSessionKeys: params.resumedSessionKeys, activeSessionIds: params.activeSessionIds, activeSessionKeys: params.activeSessionKeys, + lifecycleGeneration: params.lifecycleGeneration, + shouldContinue: params.shouldContinue, gatewayRuntime: params.gatewayRuntime, }); return { @@ -306,19 +331,35 @@ export function scheduleRestartAbortedMainSessionRecovery(params: { cfg?: OpenClawConfig; delayMs?: number; maxRetries?: number; + shouldContinue?: () => boolean; stateDir?: string; gatewayRuntime: GatewayRecoveryRuntime; -}): void { +}): { stop: () => Promise } { const initialDelay = params.delayMs ?? DEFAULT_RECOVERY_DELAY_MS; const maxRetries = params.maxRetries ?? MAX_RECOVERY_RETRIES; const resumedSessionKeys = new Set(); + const lifecycleGeneration = getAgentEventLifecycleGeneration(); + let stopped = false; + let timer: ReturnType | undefined; + let queuedAttempt: Promise | undefined; + let activeAttempt: Promise | undefined; + const shouldContinue = () => + !stopped && + params.shouldContinue?.() !== false && + isAgentEventLifecycleGenerationCurrent(lifecycleGeneration); // Only reconcile rows that existed before this startup recovery was scheduled. // Fresh runs started by this gateway are protected again by the active-run check. const startupRecoveryCutoffMs = Date.now(); const runRecoveryAttempt = (attempt: number, delay: number) => { + if (!shouldContinue()) { + return; + } const exhaustedTargets = new Map(); const reconcileExhaustedTargets = async () => { + if (!shouldContinue()) { + return; + } const outcomes = await Promise.allSettled( [...exhaustedTargets.values()].map((target) => runWithGatewayIndependentRootWorkAdmission( @@ -327,8 +368,10 @@ export function scheduleRestartAbortedMainSessionRecovery(params: { canonicalSessionKey: target.canonicalSessionKey, cfg: params.cfg, expectedSessionId: target.sessionId, + lifecycleGeneration, observationOnly: true, sessionKey: target.sessionKey, + shouldContinue, storePath: target.storePath, gatewayRuntime: params.gatewayRuntime, }), @@ -343,7 +386,7 @@ export function scheduleRestartAbortedMainSessionRecovery(params: { }; // Delayed retries outlive startup; each attempt must independently block // host suspension while it reads and rewrites recovery session state. - void runWithGatewayIndependentRootWorkAdmission( + const pendingAttempt = runWithGatewayIndependentRootWorkAdmission( async () => await recoverStartupOrphanedMainSessionsWithOptions({ cfg: params.cfg, @@ -353,37 +396,84 @@ export function scheduleRestartAbortedMainSessionRecovery(params: { stateDir: params.stateDir, resumedSessionKeys, updatedBeforeMs: startupRecoveryCutoffMs, + lifecycleGeneration, + shouldContinue, gatewayRuntime: params.gatewayRuntime, }), ) .then(async (result) => { + if (!shouldContinue()) { + return; + } if (result.failed > 0 && attempt < maxRetries) { - scheduleAttempt(attempt + 1, delay * RETRY_BACKOFF_MULTIPLIER); + const retryDelay = + delay > 0 ? delay * RETRY_BACKOFF_MULTIPLIER : DEFAULT_RECOVERY_DELAY_MS; + scheduleAttempt(attempt + 1, retryDelay); } else if (result.failed > 0 && attempt === maxRetries && exhaustedTargets.size > 0) { // Reconcile only exact rows whose final dispatch retained its durable charge. await reconcileExhaustedTargets(); } }) .catch(async (err: unknown) => { + if (!shouldContinue()) { + return; + } if (attempt < maxRetries) { log.warn(`main-session restart recovery failed: ${String(err)}`); - scheduleAttempt(attempt + 1, delay * RETRY_BACKOFF_MULTIPLIER); + const retryDelay = + delay > 0 ? delay * RETRY_BACKOFF_MULTIPLIER : DEFAULT_RECOVERY_DELAY_MS; + scheduleAttempt(attempt + 1, retryDelay); } else { log.warn(`main-session restart recovery gave up: ${String(err)}`); await reconcileExhaustedTargets(); } }); + const trackedAttempt = pendingAttempt.finally(() => { + if (activeAttempt === trackedAttempt) { + activeAttempt = undefined; + } + }); + activeAttempt = trackedAttempt; }; const scheduleAttempt = (attempt: number, delay: number) => { - if (delay <= 0) { - runRecoveryAttempt(attempt, delay); + if (!shouldContinue()) { return; } - setTimeout(() => { + if (delay <= 0) { + // Publish the cancellable handle before immediate startup can claim a session. + const pendingStart = Promise.resolve().then(() => { + if (shouldContinue()) { + runRecoveryAttempt(attempt, delay); + } + }); + const trackedStart = pendingStart.finally(() => { + if (queuedAttempt === trackedStart) { + queuedAttempt = undefined; + } + }); + queuedAttempt = trackedStart; + return; + } + timer = setTimeout(() => { + timer = undefined; runRecoveryAttempt(attempt, delay); - }, delay).unref?.(); + }, delay); + timer.unref?.(); }; scheduleAttempt(1, initialDelay); + return { + stop: async () => { + // Restart recovery belongs to its startup generation; stale timers must + // never claim a session after that gateway begins draining. + stopped = true; + if (timer) { + clearTimeout(timer); + timer = undefined; + } + await queuedAttempt; + await activeAttempt; + }, + }; } diff --git a/src/agents/main-session-restart-recovery-store.ts b/src/agents/main-session-restart-recovery-store.ts index 11100e5409d3..aa512d8c53c7 100644 --- a/src/agents/main-session-restart-recovery-store.ts +++ b/src/agents/main-session-restart-recovery-store.ts @@ -106,9 +106,22 @@ export async function recoverStore(params: { sessionWorkAdmissionHandoffId?: string; activeSessionIds?: Iterable; activeSessionKeys?: Iterable; + lifecycleGeneration?: string; + shouldContinue?: () => boolean; gatewayRuntime: GatewayRecoveryRuntime; }): Promise<{ recovered: number; failed: number; skipped: number }> { const result = { recovered: 0, failed: 0, skipped: 0 }; + const shouldContinue = () => params.shouldContinue?.() !== false; + const resumeIfCurrent = async (resumeParams: Parameters[0]) => { + if (!shouldContinue()) { + return "skipped" as const; + } + return await resumeMainSession({ + ...resumeParams, + lifecycleGeneration: params.lifecycleGeneration, + shouldContinue: params.shouldContinue, + }); + }; const providedActiveSessionIds = params.activeSessionIds === undefined ? undefined : normalizeStringSet(params.activeSessionIds); const providedActiveSessionKeys = @@ -145,6 +158,10 @@ export async function recoverStore(params: { for (const { sessionKey, entry: loadedEntry } of entries.toSorted((a, b) => a.sessionKey.localeCompare(b.sessionKey), )) { + if (!shouldContinue()) { + result.skipped++; + return result; + } let entry = loadedEntry; const agentId = resolveAgentIdFromSessionKey( sessionKey, @@ -191,20 +208,29 @@ export async function recoverStore(params: { continue; } + if (!shouldContinue()) { + result.skipped++; + return result; + } const observed = await commitMainSessionRecovery({ command: { kind: "observe", cycleId: randomUUID(), - lifecycleGeneration: getAgentEventLifecycleGeneration(), + lifecycleGeneration: params.lifecycleGeneration ?? getAgentEventLifecycleGeneration(), sessionKey, }, requireWriteSuccess: true, + shouldContinue: params.shouldContinue, target: { sessionKey, storePath: params.storePath }, }); if (!observed.entry || observed.transition.kind !== "observed") { result.skipped++; continue; } + if (!shouldContinue()) { + result.skipped++; + return result; + } entry = observed.entry; const recoveryView = observed.transition.view; if ( @@ -216,6 +242,10 @@ export async function recoverStore(params: { continue; } if (recoveryView.status === "exhausted") { + if (!shouldContinue()) { + result.skipped++; + return result; + } const tombstone = await tombstoneMainRestartRecoveryWithNotice({ cfg: params.cfg, entry, @@ -266,6 +296,10 @@ export async function recoverStore(params: { requiresRestartRecoveryMessageActionAuthority(entry) && !hasRestartRecoveryMessageActionAuthority(entry) ) { + if (!shouldContinue()) { + result.skipped++; + return result; + } const disposition = await failUnresumableMainSession({ cfg: params.cfg, entry, @@ -296,6 +330,9 @@ export async function recoverStore(params: { if (!resumeBlockReason) { return false; } + if (!shouldContinue()) { + return true; + } const disposition = await failUnresumableMainSession({ cfg: params.cfg, entry, @@ -316,7 +353,7 @@ export async function recoverStore(params: { if (await failBlockedResume()) { continue; } - const resumed = await resumeMainSession({ + const resumed = await resumeIfCurrent({ canonicalSessionKey: dispatchSessionKey, cfg: params.cfg, entry, @@ -350,6 +387,10 @@ export async function recoverStore(params: { }, ); } catch (err) { + if (!shouldContinue()) { + result.skipped++; + return result; + } if (entry.pendingFinalDelivery?.kind === "replayable") { if (await failBlockedResume()) { continue; @@ -357,7 +398,7 @@ export async function recoverStore(params: { log.warn( `transcript unavailable for ${sessionKey}; resuming its durable pending final delivery`, ); - const resumed = await resumeMainSession({ + const resumed = await resumeIfCurrent({ canonicalSessionKey: dispatchSessionKey, cfg: params.cfg, entry, @@ -377,11 +418,15 @@ export async function recoverStore(params: { continue; } + if (!shouldContinue()) { + result.skipped++; + return result; + } if (entry.pendingFinalDelivery?.kind === "replayable") { if (await failBlockedResume()) { continue; } - const resumed = await resumeMainSession({ + const resumed = await resumeIfCurrent({ canonicalSessionKey: dispatchSessionKey, cfg: params.cfg, entry, @@ -408,6 +453,10 @@ export async function recoverStore(params: { ? "transcript" : undefined; if (completionSource) { + if (!shouldContinue()) { + result.skipped++; + return result; + } const reconciliation = await reconcileInterruptedCompletionReport({ entry, source: completionSource, @@ -437,6 +486,10 @@ export async function recoverStore(params: { entry.restartRecoveryDeliveryToolCallId, ); if (resumePolicy.action === "complete") { + if (!shouldContinue()) { + result.skipped++; + return result; + } const completion = await markSessionCompletedAfterRecoveryCheckpoint({ agentId, entry, @@ -457,6 +510,10 @@ export async function recoverStore(params: { } else if (completion.outcome === "changed") { result.skipped++; } else { + if (!shouldContinue()) { + result.skipped++; + return result; + } const disposition = await failUnresumableMainSession({ cfg: params.cfg, entry, @@ -471,6 +528,10 @@ export async function recoverStore(params: { continue; } if (resumePolicy.action === "fail") { + if (!shouldContinue()) { + result.skipped++; + return result; + } const disposition = await failUnresumableMainSession({ cfg: params.cfg, entry, @@ -487,7 +548,7 @@ export async function recoverStore(params: { if (await failBlockedResume()) { continue; } - const resumed = await resumeMainSession({ + const resumed = await resumeIfCurrent({ canonicalSessionKey: dispatchSessionKey, cfg: params.cfg, entry, diff --git a/src/agents/main-session-restart-recovery.test.ts b/src/agents/main-session-restart-recovery.test.ts index 3d9e8d6821ba..f32da632be73 100644 --- a/src/agents/main-session-restart-recovery.test.ts +++ b/src/agents/main-session-restart-recovery.test.ts @@ -179,6 +179,7 @@ function loadSessionEntry( beforeEach(async () => { vi.clearAllMocks(); + vi.mocked(callGateway).mockReset(); vi.mocked(callGateway).mockImplementation(async () => ({ runId: "run-resumed" })); runtimePluginMocks.findRestartRecoveryUnsafeReplyHook.mockReturnValue(undefined); resetAgentEventsForTest(); @@ -2260,6 +2261,316 @@ describe("main-session-restart-recovery", () => { expect(customStore["agent:main:main"]?.abortedLastRun).toBe(false); }); + it("cancels startup recovery when its gateway lifecycle stops", async () => { + const sessionsDir = await makeSessionsDir(); + await writeMainSession({ + sessionsDir, + pendingFinalDelivery: { + kind: "replayable", + text: "interrupted response", + createdAt: Date.now(), + }, + }); + + vi.useFakeTimers(); + try { + const recovery = scheduleRestartAbortedMainSessionRecovery({ + cfg: {}, + delayMs: 5_000, + stateDir: tmpDir, + }); + + await Promise.all([recovery.stop(), recovery.stop()]); + await vi.advanceTimersByTimeAsync(5_000); + + expect(callGateway).not.toHaveBeenCalled(); + expect( + loadSessionEntry({ + sessionKey: "agent:main:main", + storePath: path.join(sessionsDir, "sessions.json"), + }), + ).toMatchObject({ status: "running", abortedLastRun: true }); + } finally { + vi.useRealTimers(); + } + }); + + it("cancels an immediate startup recovery before its queued attempt can claim a session", async () => { + const sessionsDir = await makeSessionsDir(); + const storePath = path.join(sessionsDir, "sessions.json"); + await writeMainSession({ + sessionsDir, + pendingFinalDelivery: { + kind: "replayable", + text: "interrupted response", + createdAt: Date.now(), + }, + }); + + const recovery = scheduleRestartAbortedMainSessionRecovery({ + cfg: {}, + delayMs: 0, + stateDir: tmpDir, + }); + await recovery.stop(); + + expect(callGateway).not.toHaveBeenCalled(); + expect(getActiveGatewayRootWorkCount()).toBe(0); + expect(loadSessionEntry({ sessionKey: "agent:main:main", storePath })).toMatchObject({ + status: "running", + abortedLastRun: true, + }); + }); + + it("fences an in-flight startup recovery before its durable session claim", async () => { + const sessionsDir = await makeSessionsDir(); + const storePath = path.join(sessionsDir, "sessions.json"); + await writeMainSession({ + sessionsDir, + pendingFinalDelivery: { + kind: "replayable", + text: "interrupted response", + createdAt: Date.now(), + }, + }); + + const originalApply = sessionAccessor.applySessionEntryReplacements; + const observeEntered = createDeferred(); + const releaseObserve = createDeferred(); + let pausedObservation = false; + const replacementSpy = vi + .spyOn(sessionAccessor, "applySessionEntryReplacements") + .mockImplementation(async (params) => { + if (params.requireWriteSuccess === true && !pausedObservation) { + pausedObservation = true; + observeEntered.resolve(); + await releaseObserve.promise; + } + return await originalApply(params); + }); + + const recovery = scheduleRestartAbortedMainSessionRecovery({ + cfg: {}, + delayMs: 0, + stateDir: tmpDir, + }); + let stopping: Promise | undefined; + try { + await observeEntered.promise; + expect(getActiveGatewayRootWorkCount()).toBe(1); + let stopSettled = false; + stopping = recovery.stop().then(() => { + stopSettled = true; + }); + await Promise.resolve(); + + expect(stopSettled).toBe(false); + expect(callGateway).not.toHaveBeenCalled(); + + releaseObserve.resolve(); + await stopping; + + expect(callGateway).not.toHaveBeenCalled(); + expect(getActiveGatewayRootWorkCount()).toBe(0); + expect(loadSessionEntry({ sessionKey: "agent:main:main", storePath })).toMatchObject({ + status: "running", + abortedLastRun: true, + }); + expect( + loadSessionEntry({ sessionKey: "agent:main:main", storePath })?.mainRestartRecovery, + ).toBeUndefined(); + } finally { + releaseObserve.resolve(); + await stopping; + replacementSpy.mockRestore(); + } + }); + + it("joins an in-flight startup dispatch before stopping its recovery owner", async () => { + const sessionsDir = await makeSessionsDir(); + await writeMainSession({ + sessionsDir, + pendingFinalDelivery: { + kind: "replayable", + text: "interrupted response", + createdAt: Date.now(), + }, + }); + + const dispatchEntered = createDeferred(); + const releaseDispatch = createDeferred(); + vi.mocked(callGateway).mockImplementationOnce(async () => { + dispatchEntered.resolve(); + await releaseDispatch.promise; + return { runId: "run-resumed" }; + }); + + const recovery = scheduleRestartAbortedMainSessionRecovery({ + cfg: {}, + delayMs: 0, + stateDir: tmpDir, + }); + let stopping: Promise | undefined; + try { + await dispatchEntered.promise; + let stopSettled = false; + stopping = recovery.stop().then(() => { + stopSettled = true; + }); + await Promise.resolve(); + + expect(stopSettled).toBe(false); + expect(getActiveGatewayRootWorkCount()).toBe(1); + + releaseDispatch.resolve(); + await stopping; + + expect(callGateway).toHaveBeenCalledOnce(); + expect(getActiveGatewayRootWorkCount()).toBe(0); + } finally { + releaseDispatch.resolve(); + await stopping; + } + }); + + it("fences an ambiguous terminal probe when its startup recovery owner stops", async () => { + const sessionsDir = await makeSessionsDir(); + const storePath = path.join(sessionsDir, "sessions.json"); + await writeMainSession({ + sessionsDir, + pendingFinalDelivery: { + kind: "replayable", + text: "interrupted response", + createdAt: Date.now(), + }, + }); + + const probeEntered = createDeferred(); + const releaseProbe = createDeferred(); + let recoveryRunId: string | undefined; + vi.mocked(callGateway).mockImplementation(async (request) => { + if (request.method === "agent") { + recoveryRunId = String((request.params as { idempotencyKey?: unknown }).idempotencyKey); + throw new Error("ambiguous recovery dispatch transport failure"); + } + if (request.method === "agent.wait") { + probeEntered.resolve(); + await releaseProbe.promise; + return { runId: recoveryRunId, status: "ok", endedAt: Date.now() }; + } + return { runId: "run-resumed" }; + }); + + const recovery = scheduleRestartAbortedMainSessionRecovery({ + cfg: {}, + delayMs: 0, + stateDir: tmpDir, + }); + let stopping: Promise | undefined; + try { + await probeEntered.promise; + expect(getActiveGatewayRootWorkCount()).toBe(1); + expect(recoveryRunId).toEqual(expect.any(String)); + + let stopSettled = false; + stopping = recovery.stop().then(() => { + stopSettled = true; + }); + await Promise.resolve(); + + expect(stopSettled).toBe(false); + expect(callGateway).toHaveBeenCalledTimes(2); + + releaseProbe.resolve(); + await stopping; + + const entry = loadSessionEntry({ sessionKey: "agent:main:main", storePath }); + expect(entry).toMatchObject({ + status: "running", + abortedLastRun: true, + pendingFinalDelivery: { + kind: "replayable", + text: "interrupted response", + }, + mainRestartRecovery: { chargedAttempts: 1 }, + }); + expect(entry?.mainRestartRecovery?.reservation).toBeUndefined(); + expect(entry?.restartRecoveryTerminalRunIds ?? []).not.toContain(recoveryRunId); + expect(entry?.restartRecoveryRuns ?? []).not.toEqual( + expect.arrayContaining([expect.objectContaining({ runId: recoveryRunId })]), + ); + expect(callGateway).toHaveBeenCalledTimes(2); + expect(getActiveGatewayRootWorkCount()).toBe(0); + } finally { + releaseProbe.resolve(); + await (stopping ?? recovery.stop()); + } + }); + + it("retains canonical retry backoff when startup recovery begins immediately", async () => { + const sessionsDir = await makeSessionsDir(); + await writeMainSession({ + sessionsDir, + pendingFinalDelivery: { + kind: "replayable", + text: "interrupted response", + createdAt: Date.now(), + }, + }); + const firstDispatch = createDeferred(); + const secondDispatch = createDeferred(); + let firstAgentDispatch = true; + vi.mocked(callGateway).mockImplementation(async (request) => { + if (request.method === "agent") { + if (firstAgentDispatch) { + firstAgentDispatch = false; + firstDispatch.resolve(); + throw new Error("transient startup failure"); + } + secondDispatch.resolve(); + } + return { runId: "run-resumed" }; + }); + + vi.useFakeTimers(); + const retryScheduled = createDeferred(); + const fakeSetTimeout = globalThis.setTimeout; + const setTimeoutSpy = vi + .spyOn(globalThis, "setTimeout") + .mockImplementation((...args: Parameters) => { + const timer = fakeSetTimeout(...args); + if (args[1] === 5_000) { + retryScheduled.resolve(); + } + return timer; + }); + const countAgentDispatches = () => + vi.mocked(callGateway).mock.calls.filter(([request]) => request.method === "agent").length; + let recovery: ReturnType | undefined; + try { + recovery = scheduleRestartAbortedMainSessionRecovery({ + cfg: {}, + delayMs: 0, + maxRetries: 2, + stateDir: tmpDir, + }); + await firstDispatch.promise; + await retryScheduled.promise; + expect(countAgentDispatches()).toBe(1); + + await vi.advanceTimersByTimeAsync(4_999); + expect(countAgentDispatches()).toBe(1); + + await vi.advanceTimersByTimeAsync(1); + await secondDispatch.promise; + expect(countAgentDispatches()).toBe(2); + } finally { + await recovery?.stop(); + setTimeoutSpy.mockRestore(); + vi.useRealTimers(); + } + }); + it("admits each scheduled recovery attempt as independent root work", async () => { const sessionsDir = await makeSessionsDir(); await writeMainSession({ @@ -2292,7 +2603,7 @@ describe("main-session-restart-recovery", () => { scheduleRestartAbortedMainSessionRecovery({ cfg: {}, - delayMs: 0, + delayMs: 1, maxRetries: 2, stateDir: tmpDir, }); @@ -4524,6 +4835,68 @@ describe("main-session-restart-recovery", () => { }, ); + it.each([ + { + label: "a replay-safe checkpoint earlier in the interrupted turn", + messages: [ + { role: "user", content: "do the thing" }, + codeModeCheckpointMessage("exec"), + createAssistantToolCallMessage([ + { + type: "toolCall", + id: "call-read-current", + name: "read", + arguments: { path: "README.md" }, + }, + ]), + { + role: "toolResult", + toolName: "read", + toolCallId: "call-read-current", + content: [{ type: "text", text: "current read result" }], + }, + ], + forceRestartSafeTools: true, + }, + { + label: "a replay-safe checkpoint from an earlier user turn", + messages: [ + { role: "user", content: "finish the earlier turn" }, + codeModeCheckpointMessage("exec"), + { role: "user", content: "start the current turn" }, + createAssistantToolCallMessage([ + { + type: "toolCall", + id: "call-read-current", + name: "read", + arguments: { path: "README.md" }, + }, + ]), + { + role: "toolResult", + toolName: "read", + toolCallId: "call-read-current", + content: [{ type: "text", text: "current read result" }], + }, + ], + forceRestartSafeTools: false, + }, + ])( + "preserves the restart-safe boundary after an ordinary tool result with $label", + async ({ messages, forceRestartSafeTools }) => { + const sessionsDir = await makeSessionsDir(); + await writeStore(sessionsDir, mainSessionStore()); + await writeTranscript(sessionsDir, "main-session", messages); + + await expectRecovery({ recovered: 1, failed: 0, skipped: 0 }); + if (forceRestartSafeTools) { + expect(gatewayParams()).toMatchObject({ forceRestartSafeTools: true }); + } else { + expect(gatewayParams()).not.toMatchObject({ forceRestartSafeTools: true }); + } + }, + ); + it("keeps restart safety across a second restart of the recovery turn", async () => { const sessionsDir = await makeSessionsDir(); await writeMainSession({ diff --git a/src/agents/prepared-model-runtime.owner.ts b/src/agents/prepared-model-runtime.owner.ts index 44d16d897117..44b0ec3ea31c 100644 --- a/src/agents/prepared-model-runtime.owner.ts +++ b/src/agents/prepared-model-runtime.owner.ts @@ -538,16 +538,26 @@ async function buildSnapshot( ...(catalogMode === "static" ? { providerIds } : {}), ...(input.workspaceDir ? { workspaceDir: input.workspaceDir } : {}), }); - const staticEntries = providerStaticModels.map(toStaticCatalogEntry); - // Config reload publishes a replacement snapshot. Keep the synchronous inline projection - // at that lifecycle boundary instead of rebuilding it on every model resolution in a turn. - const inlineProviderModels = buildInlineProviderModels(input.config.models?.providers ?? {}); const configuredRuntimeModels = prepareConfiguredRuntimeModels({ config: input.config, env, providerStaticModels, ...(input.workspaceDir ? { workspaceDir: input.workspaceDir } : {}), }); + const staticModels = new Map(); + for (const model of [ + ...configuredRuntimeModels.map((configured) => configured.model), + ...providerStaticModels, + ]) { + const modelKey = `${normalizeProviderId(model.provider)}\0${model.id.trim().toLowerCase()}`; + if (!staticModels.has(modelKey)) { + staticModels.set(modelKey, model); + } + } + const staticEntries = [...staticModels.values()].map(toStaticCatalogEntry); + // Config reload publishes a replacement snapshot. Keep the synchronous inline projection + // at that lifecycle boundary instead of rebuilding it on every model resolution in a turn. + const inlineProviderModels = buildInlineProviderModels(input.config.models?.providers ?? {}); const createStores = (): PreparedModelRuntimeStores => { // Runtime API keys and session extensions mutate these objects. Fork them per run while the // credential map and parsed catalog remain owned by the lifecycle snapshot. diff --git a/src/agents/prepared-model-runtime.test.ts b/src/agents/prepared-model-runtime.test.ts index c774430cc73e..03c08789ee2f 100644 --- a/src/agents/prepared-model-runtime.test.ts +++ b/src/agents/prepared-model-runtime.test.ts @@ -273,6 +273,64 @@ describe("prepared model runtime snapshots", () => { ]); }); + it("publishes configured manifest model capabilities without a provider discovery entry", async () => { + const runtimeModel = { + provider: "openai", + id: "gpt-5.4", + name: "GPT-5.4", + api: "openai-responses" as const, + baseUrl: "https://api.openai.com/v1", + reasoning: true, + input: ["text" as const, "image" as const], + cost: { input: 2.5, output: 15, cacheRead: 0.25, cacheWrite: 0 }, + contextWindow: 1_050_000, + maxTokens: 128_000, + }; + mocks.resolveBundledStaticCatalogModel.mockReturnValueOnce(runtimeModel); + const config = { + agents: { + defaults: { + model: { primary: "openai/gpt-5.4" }, + models: { "openai/gpt-5.4": {} }, + }, + entries: { + qa: { default: true, model: { primary: "openai/gpt-5.4" } }, + }, + }, + }; + + const snapshot = await publishPreparedModelRuntimeSnapshot({ + agentId: "qa", + config, + agentDir: "/tmp/prepared-model-runtime-manifest-qa", + workspaceDir: "/tmp/prepared-model-runtime-manifest-workspace", + }); + + expect(mocks.loadStaticCatalog).toHaveBeenCalledWith({ + cfg: config, + env: process.env, + workspaceDir: "/tmp/prepared-model-runtime-manifest-workspace", + }); + expect(mocks.resolveBundledStaticCatalogModel).toHaveBeenCalledOnce(); + expect(snapshot.agentId).toBe("qa"); + expect(snapshot.configuredRuntimeModels).toEqual([ + { provider: "openai", modelId: "gpt-5.4", model: runtimeModel }, + ]); + expect(snapshot.modelCatalog.entries).toEqual([]); + expect(snapshot.modelCatalog.staticEntries).toEqual([ + { + provider: "openai", + id: "gpt-5.4", + name: "GPT-5.4", + api: "openai-responses", + baseUrl: "https://api.openai.com/v1", + contextWindow: 1_050_000, + reasoning: true, + input: ["text", "image"], + }, + ]); + }); + it("retains full configured static models with request-time catalog precedence", async () => { const runtimeModel = { provider: "nvidia", @@ -303,6 +361,18 @@ describe("prepared model runtime snapshots", () => { expect(snapshot.configuredRuntimeModels).toEqual([ { provider: "nvidia", modelId: "nemotron-static", model: runtimeModel }, ]); + expect(snapshot.modelCatalog.staticEntries).toEqual([ + { + provider: "nvidia", + id: "nemotron-static", + name: "Nemotron Static", + api: "openai-completions", + baseUrl: "https://integrate.api.nvidia.com/v1", + contextWindow: 128_000, + reasoning: false, + input: ["text"], + }, + ]); }); it("prepares inline provider models once at the snapshot boundary", async () => { diff --git a/src/agents/tools/media-generate-background-shared.test.ts b/src/agents/tools/media-generate-background-shared.test.ts index afa5cea41e0f..4b875e2e5f4c 100644 --- a/src/agents/tools/media-generate-background-shared.test.ts +++ b/src/agents/tools/media-generate-background-shared.test.ts @@ -1,6 +1,10 @@ // Background media generation tests cover detached task completion, requester // wake delivery, and direct media fallback behavior. import { beforeEach, describe, expect, it, vi } from "vitest"; +import { + runWithOwnedSessionTranscriptWriteLock, + withOwnedSessionTranscriptWrites, +} from "../../config/sessions/transcript-write-context.js"; import type { SessionEntry } from "../../config/sessions/types.js"; import { resetGeneratedMediaTaskActivityForTests } from "../../tasks/task-runtime.test-helpers.js"; import { hasPendingGeneratedMediaTaskForSessionKey } from "../../tasks/task-status-access.js"; @@ -117,6 +121,81 @@ describe("shouldDetachMediaGenerationTask", () => { }); describe("scheduleMediaGenerationTaskCompletion", () => { + it("runs detached completion outside a disposed requester transcript owner", async () => { + const sessionKey = "agent:qa:image-generate"; + let disposed = false; + let releaseBackground!: () => void; + const backgroundReady = new Promise((resolve) => { + releaseBackground = resolve; + }); + let scheduled: Promise | undefined; + const staleWriteLock = vi.fn(); + const withStaleWriteLock = async (operation: () => Promise | T): Promise => { + staleWriteLock(); + if (disposed) { + throw new Error("attempt disposed before transcript write"); + } + return await operation(); + }; + const freshTranscriptWrite = vi.fn(async () => {}); + const wakeTaskCompletion = vi.fn(async () => { + await runWithOwnedSessionTranscriptWriteLock({ sessionKey }, freshTranscriptWrite); + return { status: "delivered" as const }; + }); + const completeTaskRun = vi.fn(); + const lifecycle = { + createTaskRun: vi.fn(), + recordTaskProgress: vi.fn(), + completeTaskRun, + failTaskRun: vi.fn(), + wakeTaskCompletion, + }; + const run = vi.fn(async () => ({ + provider: "openai", + model: "gpt-image-1", + count: 1, + paths: ["/tmp/qa-lighthouse.png"], + wakeResult: "generated", + })); + + await withOwnedSessionTranscriptWrites( + { sessionKey, withSessionWriteLock: withStaleWriteLock }, + async () => { + scheduleMediaGenerationTaskCompletion({ + lifecycle, + handle: { + taskId: "task-image-disposed-owner", + runId: "tool:image_generate:disposed-owner", + requesterSessionKey: sessionKey, + taskLabel: "QA lighthouse", + }, + scheduleBackgroundWork: (work) => { + // Register under the attempt owner, then execute after it is disposed. + scheduled = backgroundReady.then(work); + }, + progressSummary: "Generating image", + toolName: "Image generation", + onWakeFailure: vi.fn(), + run, + }); + }, + ); + + disposed = true; + releaseBackground(); + if (!scheduled) { + throw new Error("expected scheduled media work"); + } + await scheduled; + + expect(staleWriteLock).not.toHaveBeenCalled(); + expect(run).toHaveBeenCalledOnce(); + expect(freshTranscriptWrite).toHaveBeenCalledOnce(); + expect(wakeTaskCompletion).toHaveBeenCalledOnce(); + expect(completeTaskRun).toHaveBeenCalledOnce(); + expect(lifecycle.failTaskRun).not.toHaveBeenCalled(); + }); + it("keeps a pending generated-media run fresh until scheduled work settles", async () => { vi.useFakeTimers(); try { diff --git a/src/agents/tools/media-generate-background-shared.ts b/src/agents/tools/media-generate-background-shared.ts index 014d45311d46..d5b9ad5b4e58 100644 --- a/src/agents/tools/media-generate-background-shared.ts +++ b/src/agents/tools/media-generate-background-shared.ts @@ -6,6 +6,7 @@ import crypto from "node:crypto"; import { getCliSessionBinding } from "../../config/sessions/cli-session-binding.js"; import { loadSessionEntryReadOnly } from "../../config/sessions/session-accessor.js"; +import { runWithoutOwnedSessionTranscriptWrites } from "../../config/sessions/transcript-write-context.js"; import type { OpenClawConfig } from "../../config/types.openclaw.js"; import { clearAgentRunContext, registerAgentRunContext } from "../../infra/agent-events.js"; import { formatErrorMessage } from "../../infra/errors.js"; @@ -595,7 +596,8 @@ export function scheduleMediaGenerationTaskCompletion< }); } }; - params.scheduleBackgroundWork(runBackgroundWork); + // Detached completion needs its own transcript lock after the parent attempt exits. + params.scheduleBackgroundWork(() => runWithoutOwnedSessionTranscriptWrites(runBackgroundWork)); } async function wakeMediaGenerationTaskCompletion(params: { diff --git a/src/cli/gateway-cli/run-loop.test.ts b/src/cli/gateway-cli/run-loop.test.ts index 3337004f5c6a..39b5e742d24a 100644 --- a/src/cli/gateway-cli/run-loop.test.ts +++ b/src/cli/gateway-cli/run-loop.test.ts @@ -945,6 +945,13 @@ describe("runGatewayLoop", () => { sessionKeys: new Set(["agent:main:deferral-timeout"]), reason: "gateway restart drain", }); + expect(markRestartAbortedMainSessions).toHaveBeenCalledTimes(2); + expect(markRestartAbortedMainSessions).toHaveBeenLastCalledWith({ + cfg: {}, + sessionIds: new Set(["session-deferral-timeout"]), + sessionKeys: new Set(["agent:main:deferral-timeout"]), + reason: "config reload forced restart", + }); expect(gatewayLog.warn).toHaveBeenCalledWith( "failed to mark interrupted main sessions for restart recovery: Error: store read-only", ); @@ -1003,6 +1010,7 @@ describe("runGatewayLoop", () => { sessionKeys: new Set(["agent:main:forced-task"]), reason: "gateway restart drain", }); + expect(markRestartAbortedMainSessions).toHaveBeenCalledTimes(1); expect(gatewayLog.warn).toHaveBeenCalledWith( "restart blocked by active background task run(s): taskId=task-force runId=run-force status=running runtime=cron label=forced", ); @@ -1772,6 +1780,7 @@ describe("runGatewayLoop", () => { sessionKeys: new Set(["agent:main:file-intent"]), reason: "gateway restart drain", }); + expect(markRestartAbortedMainSessions).toHaveBeenCalledTimes(1); expect(start).toHaveBeenCalledTimes(2); sigint(); diff --git a/src/cli/gateway-cli/run-loop.ts b/src/cli/gateway-cli/run-loop.ts index fa64bebe97bb..6786d4cc2e9e 100644 --- a/src/cli/gateway-cli/run-loop.ts +++ b/src/cli/gateway-cli/run-loop.ts @@ -616,7 +616,13 @@ export async function runGatewayLoop(params: { }; let activeRestartSessionKeysAtDrainStart = new Set(); let activeRestartSessionIdsAtDrainStart = new Set(); + let hasMarkedActiveMainSessionsForRestart = false; const markActiveMainSessionsForRestart = async (reason: string) => { + // A second successful mark races recovery claims; failed or empty + // attempts must remain retryable at the forced-restart boundary. + if (hasMarkedActiveMainSessionsForRestart) { + return; + } const sessionKeys = new Set([ ...activeRestartSessionKeysAtDrainStart, ...collectActiveRestartSessionKeys(), @@ -629,12 +635,15 @@ export async function runGatewayLoop(params: { return; } try { - await markRestartAbortedMainSessions({ + const result = await markRestartAbortedMainSessions({ cfg: getRuntimeConfig(), sessionKeys, sessionIds, reason, }); + if (result.marked > 0) { + hasMarkedActiveMainSessionsForRestart = true; + } } catch (err) { gatewayLog.warn( `failed to mark interrupted main sessions for restart recovery: ${String(err)}`, diff --git a/src/config/sessions/transcript-write-context.ts b/src/config/sessions/transcript-write-context.ts index c6009792a98b..b2629c759f59 100644 --- a/src/config/sessions/transcript-write-context.ts +++ b/src/config/sessions/transcript-write-context.ts @@ -102,6 +102,11 @@ export async function withOwnedSessionTranscriptWrites( return await ownedTranscriptWriteContext.run(context, run); } +/** Runs detached work without retaining an attempt-owned transcript lock. */ +export function runWithoutOwnedSessionTranscriptWrites(run: () => T): T { + return ownedTranscriptWriteContext.exit(run); +} + export function bindOwnedSessionTranscriptWrites( context: OwnedSessionTranscriptWriteContext, run: (...args: TArgs) => TResult, diff --git a/src/gateway/server-methods/agent-content-phase.ts b/src/gateway/server-methods/agent-content-phase.ts index 4d349e130bd5..709648470c44 100644 --- a/src/gateway/server-methods/agent-content-phase.ts +++ b/src/gateway/server-methods/agent-content-phase.ts @@ -98,6 +98,7 @@ export async function prepareAgentContentPhase(params: { if (params.normalizedAttachments.length > 0) { let baseProvider: string | undefined; let baseModel: string | undefined; + let catalogAgentId = agentId; let requestedAcpMeta: ReturnType; if (params.requestedSessionKeyRaw) { const { cfg, entry, canonicalKey } = loadSessionEntry(params.requestedSessionKeyRaw, { @@ -106,6 +107,7 @@ export async function prepareAgentContentPhase(params: { }); const sessionAgentId = canonicalKey === "global" && agentId ? agentId : resolveAgentIdFromSessionKey(canonicalKey); + catalogAgentId = sessionAgentId; const modelRef = resolveSessionModelRef(cfg, entry, sessionAgentId); baseProvider = modelRef.provider; baseModel = modelRef.model; @@ -119,6 +121,8 @@ export async function prepareAgentContentPhase(params: { ? true : await resolveGatewayModelSupportsImages({ loadGatewayModelCatalog: params.context.loadGatewayModelCatalog, + loadGatewayModelCatalogSnapshot: params.context.loadGatewayModelCatalogSnapshot, + agentId: catalogAgentId, provider: params.providerOverride || baseProvider, model: params.modelOverride || baseModel, }); diff --git a/src/gateway/server-methods/agent.sessions-and-models.test-utils.ts b/src/gateway/server-methods/agent.sessions-and-models.test-utils.ts index 9bed97a80f1b..ad811a34ba41 100644 --- a/src/gateway/server-methods/agent.sessions-and-models.test-utils.ts +++ b/src/gateway/server-methods/agent.sessions-and-models.test-utils.ts @@ -334,10 +334,11 @@ describe("gateway agent handler", () => { resetSubagentRegistryForTests({ persist: false }); // Route through the harness helper so the ensureRuntimePluginsLoaded // pin survives this wholesale deps override. + const persistSubagentRunsToDiskOrThrow = vi.fn(() => { + throw new Error("disk full"); + }); applyGatewaySubagentRegistryTestDeps({ - persistSubagentRunsToDiskOrThrow: () => { - throw new Error("disk full"); - }, + persistSubagentRunsToDiskOrThrow, }); const runId = "plugin-subagent-registry-fail"; const childSessionKey = "agent:main:subagent:registry-fail"; @@ -391,6 +392,7 @@ describe("gateway agent handler", () => { }, ); + expect(persistSubagentRunsToDiskOrThrow).toHaveBeenCalledTimes(1); expect(mocks.agentCommand).toHaveBeenCalledTimes(commandCallCount + 1); await waitForAssertion(() => { const task = requireValue(findTaskByRunId(runId), "expected fallback cli task"); diff --git a/src/gateway/server-methods/agent.test-harness.ts b/src/gateway/server-methods/agent.test-harness.ts index e966bdca5ccb..79479556976f 100644 --- a/src/gateway/server-methods/agent.test-harness.ts +++ b/src/gateway/server-methods/agent.test-harness.ts @@ -4,10 +4,8 @@ import { expectDefined } from "@openclaw/normalization-core"; import { expect, vi } from "vitest"; import type { readAcpSessionMeta } from "../../acp/runtime/session-meta.js"; import type { AgentInternalEvent } from "../../agents/internal-events.js"; -import { - resetSubagentRegistryForTests, - testing as subagentRegistryTesting, -} from "../../agents/subagent-registry.test-helpers.js"; +import { setSubagentRegistryDepsForTest } from "../../agents/subagent-registry-deps.js"; +import { resetSubagentRegistryForTests } from "../../agents/subagent-registry.test-helpers.js"; import type { SessionEntry } from "../../config/sessions.js"; import type { SessionTranscriptStats } from "../../config/sessions/session-accessor.js"; import { resetDiagnosticEventsForTest } from "../../infra/diagnostic-events.js"; @@ -933,9 +931,9 @@ function toLintErrorObject(value: unknown, fallbackMessage: string): Error { * between install and finalize, so the finalize spy is never called. */ export function applyGatewaySubagentRegistryTestDeps( - overrides?: Parameters[0], + overrides?: Parameters[0], ) { - subagentRegistryTesting.setDepsForTest({ + setSubagentRegistryDepsForTest({ ensureRuntimePluginsLoaded: () => {}, ...overrides, }); diff --git a/src/gateway/server-methods/chat-send-attachments.ts b/src/gateway/server-methods/chat-send-attachments.ts index 14081edfeac1..0f8f47f5812b 100644 --- a/src/gateway/server-methods/chat-send-attachments.ts +++ b/src/gateway/server-methods/chat-send-attachments.ts @@ -215,6 +215,8 @@ export async function prepareChatSendAttachments(params: { async () => { const supportsSessionModelImages = await resolveGatewayModelSupportsImages({ loadGatewayModelCatalog: context.loadGatewayModelCatalog, + loadGatewayModelCatalogSnapshot: context.loadGatewayModelCatalogSnapshot, + agentId, provider: resolvedSessionModel.provider, model: resolvedSessionModel.model, }); diff --git a/src/gateway/server-methods/nodes.event.ts b/src/gateway/server-methods/nodes.event.ts index 3931691dc6b6..2a7faf636d38 100644 --- a/src/gateway/server-methods/nodes.event.ts +++ b/src/gateway/server-methods/nodes.event.ts @@ -90,6 +90,7 @@ export const nodeEventHandlers: GatewayRequestHandlers = { getHealthCache: context.getHealthCache, refreshHealthSnapshot: context.refreshHealthSnapshot, loadGatewayModelCatalog: context.loadGatewayModelCatalog, + loadGatewayModelCatalogSnapshot: context.loadGatewayModelCatalogSnapshot, authorizeNodeSystemRunEvent: (eventParams) => context.nodeRegistry.authorizeSystemRunEvent({ nodeId: eventParams.nodeId, diff --git a/src/gateway/server-methods/sessions-read-cache.test.ts b/src/gateway/server-methods/sessions-read-cache.test.ts index ba00510a0a33..4e1fa280204b 100644 --- a/src/gateway/server-methods/sessions-read-cache.test.ts +++ b/src/gateway/server-methods/sessions-read-cache.test.ts @@ -214,25 +214,27 @@ describe("sessions.list single-flight", () => { const catalog = new Promise<[]>((resolve) => { releaseCatalog = () => resolve([]); }); + const loadGatewayModelCatalog = vi.fn(async () => await catalog); const context = { ...requestContext(config), - loadGatewayModelCatalog: async () => await catalog, + loadGatewayModelCatalog, } as GatewayRequestContext; const client = identifiedClient("owner@example.com"); const request = { archived: "all" as const, limit: 100 }; const beforeMutation = listSessions({ client, context, request }); - await vi.waitFor(() => expect(loader.calls).toHaveBeenCalledTimes(1)); + await vi.waitFor(() => expect(loadGatewayModelCatalog).toHaveBeenCalledTimes(1)); await upsertSessionEntry( { agentId: "main", sessionKey: "agent:main:created-mid-list" }, { sessionId: "created-mid-list", updatedAt: 500, visibility: "shared" }, ); const afterMutation = listSessions({ client, context, request }); - await vi.waitFor(() => expect(loader.calls).toHaveBeenCalledTimes(2)); + await vi.waitFor(() => expect(loadGatewayModelCatalog).toHaveBeenCalledTimes(2)); releaseCatalog(); const [, fresh] = await Promise.all([beforeMutation, afterMutation]); expect(fresh.sessions.map((session) => session.key)).toContain("agent:main:created-mid-list"); + expect(loader.calls).toHaveBeenCalledTimes(2); }); }); }); diff --git a/src/gateway/server-methods/sessions-read.ts b/src/gateway/server-methods/sessions-read.ts index 7119fc02eae1..0a7f8d05a8c3 100644 --- a/src/gateway/server-methods/sessions-read.ts +++ b/src/gateway/server-methods/sessions-read.ts @@ -276,7 +276,22 @@ export const sessionReadHandlers: GatewayRequestHandlers = { const run = () => measureDiagnosticsTimelineSpan( "gateway.sessions.list", - async () => { + async function listVisibleSessions( + remainingVisibilityRetries = 1, + ): Promise>> { + const modelCatalog = await measureDiagnosticsTimelineSpan( + "gateway.sessions.list.model_catalog", + () => + loadOptionalServerMethodModelCatalog( + context, + "sessions.list", + p.agentId ? { loadParams: { agentId: p.agentId } } : undefined, + ), + { + config: cfg, + phase: "sessions.list", + }, + ); const { durableStorePath, storePath, store } = measureDiagnosticsTimelineSpanSync( "gateway.sessions.list.store_load", () => @@ -297,19 +312,6 @@ export const sessionReadHandlers: GatewayRequestHandlers = { const listStore = configuredAgentsOnly ? filterSessionStoreToConfiguredAgents(cfg, store) : store; - const modelCatalog = await measureDiagnosticsTimelineSpan( - "gateway.sessions.list.model_catalog", - () => - loadOptionalServerMethodModelCatalog( - context, - "sessions.list", - p.agentId ? { loadParams: { agentId: p.agentId } } : undefined, - ), - { - config: cfg, - phase: "sessions.list", - }, - ); const result = await measureDiagnosticsTimelineSpan( "gateway.sessions.list.rows", () => @@ -474,6 +476,14 @@ export const sessionReadHandlers: GatewayRequestHandlers = { !session.incognito && (session.visibility !== "draft" || session.sharingRole === "owner"), ); + if (visibleSessions.length !== sessions.length) { + if (remainingVisibilityRetries === 0) { + throw new Error("session visibility changed during list reconciliation"); + } + // Rebuild the complete canonical page so totals, offsets, creator + // facets, and replacement rows describe the same visible snapshot. + return await listVisibleSessions(remainingVisibilityRetries - 1); + } return { ...result, sessions: visibleSessions, @@ -510,17 +520,16 @@ export const sessionReadHandlers: GatewayRequestHandlers = { if (!assertValidParams(params, validateSessionsCleanupParams, "sessions.cleanup", respond)) { return; } - const p = params; try { const { mode, appliedSummaries } = await runSessionsCleanup({ cfg: context.getRuntimeConfig(), opts: { - agent: p.agent, - allAgents: p.allAgents, - enforce: p.enforce, - activeKey: p.activeKey, - fixMissing: p.fixMissing, - fixDmScope: p.fixDmScope, + agent: params.agent, + allAgents: params.allAgents, + enforce: params.enforce, + activeKey: params.activeKey, + fixMissing: params.fixMissing, + fixDmScope: params.fixDmScope, }, }); const result = serializeSessionCleanupResult({ @@ -618,8 +627,7 @@ export const sessionReadHandlers: GatewayRequestHandlers = { if (!assertValidParams(params, validateSessionsDescribeParams, "sessions.describe", respond)) { return; } - const p = params; - const key = requireSessionKey(p.key, respond); + const key = requireSessionKey(params.key, respond); if (!key) { return; } @@ -635,8 +643,8 @@ export const sessionReadHandlers: GatewayRequestHandlers = { store, key: target.canonicalKey, entry, - includeDerivedTitles: p.includeDerivedTitles, - includeLastMessage: p.includeLastMessage, + includeDerivedTitles: params.includeDerivedTitles, + includeLastMessage: params.includeLastMessage, transcriptUsageMaxBytes: 64 * 1024, }); const placement = row.sessionId diff --git a/src/gateway/server-methods/sessions-sharing.test.ts b/src/gateway/server-methods/sessions-sharing.test.ts index 134f2bec67d4..3c149116e649 100644 --- a/src/gateway/server-methods/sessions-sharing.test.ts +++ b/src/gateway/server-methods/sessions-sharing.test.ts @@ -344,26 +344,96 @@ describe("session sharing handlers", () => { } as unknown as GatewayRequestContext, respond: (...response: Parameters) => responses.push(response), } as never); - return (responses[0]?.[1] as { sessions?: Array<{ key: string }> } | undefined)?.sessions; + return responses[0]?.[1] as + | { + count: number; + totalCount: number; + nextOffset: number | null; + hasMore: boolean; + creators: Array<{ id: string }>; + sessions: Array<{ key: string }>; + } + | undefined; }; // Non-owner must not receive the now-draft row (no preview/metadata leak). - expect((await listWith(outsider))?.some((session) => session.key === sessionKey)).toBe(false); + const outsiderList = await listWith(outsider); + expect(outsiderList?.sessions.some((session) => session.key === sessionKey)).toBe(false); + expect(outsiderList).toMatchObject({ + count: 0, + totalCount: 0, + nextOffset: null, + hasMore: false, + creators: [], + }); // A member also loses a draft (owner+admin only). expect( - (await listWith(identifiedClient("member@example.com")))?.some( + (await listWith(identifiedClient("member@example.com")))?.sessions.some( (session) => session.key === sessionKey, ), ).toBe(false); // The owner still sees their own draft. expect( - (await listWith(identifiedClient("owner@example.com")))?.some( + (await listWith(identifiedClient("owner@example.com")))?.sessions.some( (session) => session.key === sessionKey, ), ).toBe(true); }); }); + it("refills a paged session list after its first row becomes a draft", async () => { + await withOpenClawTestState({ scenario: "minimal" }, async () => { + const hiddenKey = "agent:main:mid-await-paged-draft"; + const visibleKey = "agent:main:mid-await-paged-visible"; + await upsertSessionEntry( + { agentId: "main", sessionKey: hiddenKey }, + { + sessionId: "session-mid-await-paged-draft", + updatedAt: 2, + createdActor: { type: "human", id: "hidden-owner@example.com" }, + visibility: "shared", + }, + ); + await upsertSessionEntry( + { agentId: "main", sessionKey: visibleKey }, + { + sessionId: "session-mid-await-paged-visible", + updatedAt: 1, + createdActor: { type: "human", id: "visible-owner@example.com" }, + visibility: "shared", + }, + ); + const responses: Parameters[] = []; + + await sessionReadHandlers["sessions.list"]?.({ + params: { agentId: "main", limit: 1 }, + client: identifiedClient("outsider@example.com"), + context: { + ...context(vi.fn()), + loadGatewayModelCatalog: async () => { + await patchSessionEntry({ agentId: "main", sessionKey: hiddenKey }, () => ({ + visibility: "draft", + })); + invalidateSessionSharingSnapshot(hiddenKey); + return []; + }, + } as unknown as GatewayRequestContext, + respond: (...response: Parameters) => responses.push(response), + } as never); + + expect(responses[0]?.[0]).toBe(true); + expect(responses[0]?.[1]).toMatchObject({ + count: 1, + totalCount: 1, + limitApplied: 1, + nextOffset: null, + hasMore: false, + creators: [{ id: "visible-owner@example.com" }], + sessions: [{ key: visibleKey }], + }); + }); + }); + it("lists profile ids and authorizes a selected profile as a member", async () => { await withOpenClawTestState({ scenario: "minimal" }, async () => { const sessionKey = "agent:main:profile-member"; diff --git a/src/gateway/server-node-events-types.ts b/src/gateway/server-node-events-types.ts index 474988d5e7e3..14a7a366882d 100644 --- a/src/gateway/server-node-events-types.ts +++ b/src/gateway/server-node-events-types.ts @@ -5,6 +5,7 @@ import type { CliDeps } from "../cli/deps.types.js"; import type { ChatAbortControllerEntry } from "./chat-abort.js"; import type { HealthSummary } from "./health/types.js"; import type { ChatRunEntry, ChatRunRegistration } from "./server-chat.js"; +import type { GatewayModelCatalogSnapshot } from "./server-model-catalog.types.js"; import type { DedupeEntry } from "./server-shared.js"; /** Runtime context available to node event handlers. */ @@ -29,7 +30,14 @@ export type NodeEventContext = { probe?: boolean; includeSensitive?: boolean; }) => Promise; - loadGatewayModelCatalog: () => Promise; + loadGatewayModelCatalog: (params?: { + agentId?: string; + readOnly?: boolean; + }) => Promise; + loadGatewayModelCatalogSnapshot?: (params?: { + agentId?: string; + readOnly?: boolean; + }) => Promise; authorizeNodeSystemRunEvent: (params: { nodeId: string; connId?: string; diff --git a/src/gateway/server-node-events.test.ts b/src/gateway/server-node-events.test.ts index 0361f693bfbe..66d16e7d36be 100644 --- a/src/gateway/server-node-events.test.ts +++ b/src/gateway/server-node-events.test.ts @@ -971,7 +971,7 @@ describe("voice transcript events", () => { let checkCount = 0; const isConnectionCurrent = vi.fn(() => { checkCount += 1; - if (checkCount === 1 || checkCount === 3) { + if (checkCount <= 2) { return true; } if (checkCount === 4) { @@ -984,7 +984,7 @@ describe("voice transcript events", () => { sessionKey: "voice-new-session-replay-race", }; - await handleNodeEvent( + const firstReplay = handleNodeEvent( ctx, "node-new-session-replay", { @@ -993,7 +993,7 @@ describe("voice transcript events", () => { }, { isConnectionCurrent }, ); - await handleNodeEvent( + const duplicateReplay = handleNodeEvent( ctx, "node-new-session-replay", { @@ -1002,6 +1002,7 @@ describe("voice transcript events", () => { }, { isConnectionCurrent }, ); + await Promise.all([firstReplay, duplicateReplay]); await detachedChecksStarted.promise; detachedAdmission.resolve(true); await waitForFast(() => expect(agentCommandMock).toHaveBeenCalledTimes(1)); diff --git a/src/gateway/server-node-events.ts b/src/gateway/server-node-events.ts index 2045ecaae52c..ce099255c133 100644 --- a/src/gateway/server-node-events.ts +++ b/src/gateway/server-node-events.ts @@ -698,6 +698,8 @@ export const handleNodeEvent = async ( const modelRef = resolveSessionModelRef(cfg, entry, sessionAgentId); const supportsInlineImages = await resolveGatewayModelSupportsImages({ loadGatewayModelCatalog: ctx.loadGatewayModelCatalog, + loadGatewayModelCatalogSnapshot: ctx.loadGatewayModelCatalogSnapshot, + agentId: sessionAgentId, provider: modelRef.provider, model: modelRef.model, }); diff --git a/src/gateway/server-restart-sentinel.test.ts b/src/gateway/server-restart-sentinel.test.ts index 538f253f2a08..9d142e046902 100644 --- a/src/gateway/server-restart-sentinel.test.ts +++ b/src/gateway/server-restart-sentinel.test.ts @@ -2670,6 +2670,67 @@ describe("scheduleRestartSentinelWake", () => { expect(getLatestUpdateRestartSentinel()).toEqual(payload); }); + it.each(["config-patch", "config-apply"] as const)( + "consumes a targetless %s acknowledgement without waking an agent", + async (kind) => { + mocks.readRestartSentinel.mockResolvedValue({ + version: 1, + revision: 123, + payload: { + kind, + status: "ok", + ts: 123, + sessionKey: undefined, + deliveryContext: undefined, + threadId: undefined, + message: null, + doctorHint: "Run openclaw doctor --non-interactive", + stats: { + mode: kind === "config-patch" ? "config.patch" : "config.apply", + root: "/tmp/openclaw.json", + requiresRestart: true, + }, + }, + }); + + await scheduleRestartSentinelWake({ deps: {} as never }); + + expect(mocks.clearRestartSentinelIfRevision).toHaveBeenCalledOnce(); + expect(mocks.clearRestartSentinelIfRevision).toHaveBeenCalledWith(123); + expect(mocks.enqueueSessionDelivery).not.toHaveBeenCalled(); + expect(mocks.enqueueSystemEvent).not.toHaveBeenCalled(); + expect(mocks.requestHeartbeat).not.toHaveBeenCalled(); + expect(mocks.drainPendingSessionDeliveries).not.toHaveBeenCalled(); + }, + ); + + it("preserves an explicit targetless config restart note", async () => { + mocks.readRestartSentinel.mockResolvedValue({ + version: 1, + revision: 123, + payload: { + kind: "config-patch", + status: "ok", + ts: 123, + message: "restart message", + stats: { mode: "config.patch", requiresRestart: true }, + }, + }); + + await scheduleRestartSentinelWake({ deps: {} as never }); + + expect(mocks.clearRestartSentinelIfRevision).toHaveBeenCalledWith(123); + expect(mocks.enqueueSystemEvent).toHaveBeenCalledWith("restart message", { + sessionKey: "agent:main:main", + }); + expect(mocks.requestHeartbeat).toHaveBeenCalledWith({ + source: "restart-sentinel", + intent: "immediate", + reason: "wake", + sessionKey: "agent:main:main", + }); + }); + it("durably wakes the main session when the sentinel has no sessionKey", async () => { mocks.readRestartSentinel.mockResolvedValue({ payload: { diff --git a/src/gateway/server-restart-sentinel.ts b/src/gateway/server-restart-sentinel.ts index f302c0c55d74..c743bba27796 100644 --- a/src/gateway/server-restart-sentinel.ts +++ b/src/gateway/server-restart-sentinel.ts @@ -478,6 +478,21 @@ async function loadRestartSentinelStartupTask(params: { } if (!sessionKey) { + const controlPlaneOnlyConfigRestart = + (payload.kind === "config-patch" || payload.kind === "config-apply") && + (typeof payload.message !== "string" || payload.message.trim().length === 0) && + !payload.continuation && + !payload.deliveryContext && + payload.threadId == null; + if (controlPlaneOnlyConfigRestart) { + // A targetless config acknowledgement has no agent turn to resume. + // Synthesizing a main-session wake races real restart recovery and spends a model turn. + const consumed = await clearRestartSentinelIfRevision(sentinelRevision); + if (!consumed) { + log.info(`${summary}: newer restart sentinel preserved while consuming config restart`); + } + return { status: "ran" as const }; + } const mainSessionKey = resolveMainSessionKeyFromConfig(); const wakeQueueId = await enqueueSessionDelivery( buildQueuedRestartContinuation({ diff --git a/src/gateway/server-startup-post-attach.test.ts b/src/gateway/server-startup-post-attach.test.ts index 313a96054aca..fbad938cacf7 100644 --- a/src/gateway/server-startup-post-attach.test.ts +++ b/src/gateway/server-startup-post-attach.test.ts @@ -422,6 +422,8 @@ describe("startGatewayPostAttachRuntime", () => { expect(log.info).toHaveBeenCalledWith("gateway ready"); expect(hoisted.scheduleRestartAbortedMainSessionRecovery).toHaveBeenCalledWith({ cfg: { hooks: { internal: { enabled: false } } }, + delayMs: 0, + shouldContinue: expect.any(Function), gatewayRuntime: expect.any(Object), }); expect(hoisted.scheduleSubagentOrphanRecovery).toHaveBeenCalledWith(); @@ -429,6 +431,58 @@ describe("startGatewayPostAttachRuntime", () => { expect(hoisted.startGatewayMemoryBackend).not.toHaveBeenCalled(); }); + it("fences startup recovery as soon as its gateway close prelude begins", async () => { + let closing = false; + const recoverySidecar = { stop: vi.fn(async () => {}) }; + const onGatewayLifetimeSidecars = vi.fn(); + hoisted.scheduleRestartAbortedMainSessionRecovery.mockImplementationOnce( + (params: { shouldContinue?: () => boolean }) => { + expect(params.shouldContinue?.()).toBe(true); + closing = true; + expect(params.shouldContinue?.()).toBe(false); + return recoverySidecar; + }, + ); + + await startGatewayPostAttachRuntime({ + ...createPostAttachParams(), + isClosing: () => closing, + onGatewayLifetimeSidecars, + }); + + await waitForGatewayTestState(() => { + expect(onGatewayLifetimeSidecars).toHaveBeenCalledOnce(); + }); + expect(hoisted.scheduleRestartAbortedMainSessionRecovery).toHaveBeenCalledOnce(); + expect(onGatewayLifetimeSidecars).toHaveBeenCalledWith( + expect.arrayContaining([recoverySidecar]), + ); + }); + + it("stops restart recovery with gateway-lifetime sidecars", async () => { + const recoverySidecar = { stop: vi.fn() }; + hoisted.scheduleRestartAbortedMainSessionRecovery.mockReturnValueOnce(recoverySidecar); + const onGatewayLifetimeSidecars = vi.fn(); + + await startGatewayPostAttachRuntime({ + ...createPostAttachParams(), + onGatewayLifetimeSidecars, + }); + + await waitForGatewayTestState(() => { + expect(onGatewayLifetimeSidecars).toHaveBeenCalledOnce(); + }); + const lifetimeSidecars = onGatewayLifetimeSidecars.mock.calls[0]?.[0] as + | Array<{ stop: () => Promise | void }> + | undefined; + expect(lifetimeSidecars).toContain(recoverySidecar); + + for (const sidecar of lifetimeSidecars ?? []) { + await sidecar.stop(); + } + expect(recoverySidecar.stop).toHaveBeenCalledOnce(); + }); + it("logs one startup outcome summary after sidecar registration and before readiness", async () => { const events: string[] = []; const outcomeMessages: string[] = []; @@ -1173,15 +1227,20 @@ describe("startGatewayPostAttachRuntime", () => { log: { warn: vi.fn() }, }); - expect(hoisted.ensureContextWindowCacheLoaded).not.toHaveBeenCalled(); - await vi.advanceTimersByTimeAsync(4_999); - expect(hoisted.ensureContextWindowCacheLoaded).not.toHaveBeenCalled(); - await vi.advanceTimersByTimeAsync(1); - await vi.dynamicImportSettled(); - await waitForGatewayTestState(() => { - expect(hoisted.ensureContextWindowCacheLoaded).toHaveBeenCalledWith(cfg); - }); - await sidecar.stop(); + try { + // Earlier gateway lifetimes may finish during the fake-clock window; + // this sidecar's captured config identifies its own prewarm precisely. + expect(hoisted.ensureContextWindowCacheLoaded).not.toHaveBeenCalledWith(cfg); + await vi.advanceTimersByTimeAsync(4_999); + expect(hoisted.ensureContextWindowCacheLoaded).not.toHaveBeenCalledWith(cfg); + await vi.advanceTimersByTimeAsync(1); + await vi.dynamicImportSettled(); + await waitForGatewayTestState(() => { + expect(hoisted.ensureContextWindowCacheLoaded).toHaveBeenCalledWith(cfg); + }); + } finally { + await sidecar.stop(); + } }); it("cancels context-window cache prewarm when the gateway stops first", async () => { diff --git a/src/gateway/server-startup-post-attach.ts b/src/gateway/server-startup-post-attach.ts index db0a80e2b0c2..cf7f2136e266 100644 --- a/src/gateway/server-startup-post-attach.ts +++ b/src/gateway/server-startup-post-attach.ts @@ -1210,13 +1210,22 @@ export async function startGatewayPostAttachRuntime( loaderStatsAfter.sourceTransformFallbacks - loaderStatsBefore.sourceTransformFallbacks, ], ]); + let mainSessionRecoverySidecar: GatewayPostReadySidecarHandle | undefined; try { - const { scheduleRestartAbortedMainSessionRecovery } = - await loadMainSessionRestartRecoveryModule(); - scheduleRestartAbortedMainSessionRecovery({ - cfg: params.cfgAtStart, - gatewayRuntime: params.recoveryRuntime, - }); + if (params.isClosing?.() !== true) { + const { scheduleRestartAbortedMainSessionRecovery } = + await loadMainSessionRestartRecoveryModule(); + // Closing can begin while the runtime module is loading; a late owner + // would miss lifetime registration and race the replacement gateway. + if (params.isClosing?.() !== true) { + mainSessionRecoverySidecar = scheduleRestartAbortedMainSessionRecovery({ + cfg: params.cfgAtStart, + delayMs: 0, + shouldContinue: () => params.isClosing?.() !== true, + gatewayRuntime: params.recoveryRuntime, + }); + } + } } catch (err) { params.log.warn(`main-session restart recovery failed to schedule: ${String(err)}`); } @@ -1238,6 +1247,7 @@ export async function startGatewayPostAttachRuntime( const gatewayLifetimeSidecars = [ scheduleContextCachePrewarm(params), scheduleGatewayHandlerPrewarm(params), + ...(mainSessionRecoverySidecar ? [mainSessionRecoverySidecar] : []), ]; if (workerEnvironmentSidecar) { gatewayLifetimeSidecars.push(workerEnvironmentSidecar); diff --git a/src/gateway/server.chat.gateway-server-chat-b.test.ts b/src/gateway/server.chat.gateway-server-chat-b.test.ts index 0ff532a20d1e..6c58c349b435 100644 --- a/src/gateway/server.chat.gateway-server-chat-b.test.ts +++ b/src/gateway/server.chat.gateway-server-chat-b.test.ts @@ -79,6 +79,27 @@ function waitForFast( ) { return vi.waitFor(callback, { interval: 1, ...options }); } + +function createChatVisionModelCatalogSnapshot(): Awaited< + ReturnType +> { + return { + agentId: "main", + agentDir: "/tmp/chat-attachment-vision-agent", + workspaceDir: "/tmp/chat-attachment-vision-workspace", + config: {}, + entries: [ + { + id: "vision-model", + name: "Vision Model", + provider: "test-provider", + input: ["text", "image"], + }, + ], + routeVariants: [], + }; +} + type GatewayHarness = Awaited>; type GatewaySocket = Awaited>; let harness: GatewayHarness; @@ -1631,21 +1652,16 @@ describe("gateway server chat", () => { }, }); - const firstCatalog = - createDeferred>>(); + const firstCatalogSnapshot = + createDeferred< + Awaited> + >(); const responses: Array<{ id: string; ok: boolean; payload?: unknown; error?: unknown }> = []; const context = createDirectChatContext({ - loadGatewayModelCatalog: vi - .fn() - .mockImplementationOnce(() => firstCatalog.promise) - .mockResolvedValue([ - { - id: "vision-model", - name: "Vision Model", - provider: "test-provider", - input: ["text", "image"], - }, - ]), + loadGatewayModelCatalogSnapshot: vi + .fn() + .mockImplementationOnce(() => firstCatalogSnapshot.promise) + .mockResolvedValue(createChatVisionModelCatalogSnapshot()), getRuntimeConfig: () => ({}), }); dispatchInboundMessageMock.mockImplementation(async () => dispatchRelease.promise); @@ -1683,7 +1699,7 @@ describe("gateway server chat", () => { const first = Promise.resolve(callSend("first")); await waitForFast(() => { - expect(context.loadGatewayModelCatalog).toHaveBeenCalledTimes(1); + expect(context.loadGatewayModelCatalogSnapshot).toHaveBeenCalledTimes(1); }, FAST_WAIT_OPTS); await callSend("duplicate"); @@ -1696,14 +1712,7 @@ describe("gateway server chat", () => { }, ]); - firstCatalog.resolve([ - { - id: "vision-model", - name: "Vision Model", - provider: "test-provider", - input: ["text", "image"], - }, - ]); + firstCatalogSnapshot.resolve(createChatVisionModelCatalogSnapshot()); await first; expect(responses).toEqual([ @@ -1736,8 +1745,10 @@ describe("gateway server chat", () => { test("chat.abort cancels chat.send during attachment preparation before ACK", async () => { const sessionDir = autoCleanupTempDirs.make("openclaw-gw-"); - const firstCatalog = - createDeferred>>(); + const firstCatalogSnapshot = + createDeferred< + Awaited> + >(); try { testState.sessionStorePath = path.join(sessionDir, "sessions.json"); await writeSessionStore({ @@ -1759,9 +1770,9 @@ describe("gateway server chat", () => { }> = []; const abortResponses: Array<{ ok: boolean; payload?: unknown; error?: unknown }> = []; const context = createDirectChatContext({ - loadGatewayModelCatalog: vi - .fn() - .mockImplementationOnce(() => firstCatalog.promise), + loadGatewayModelCatalogSnapshot: vi + .fn() + .mockImplementationOnce(() => firstCatalogSnapshot.promise), getRuntimeConfig: () => ({}), }); @@ -1804,7 +1815,7 @@ describe("gateway server chat", () => { }), ); await waitForFast(() => { - expect(context.loadGatewayModelCatalog).toHaveBeenCalledTimes(1); + expect(context.loadGatewayModelCatalogSnapshot).toHaveBeenCalledTimes(1); expect(context.chatAbortControllers.has("idem-attachment-abort")).toBe(true); }, FAST_WAIT_OPTS); @@ -1864,14 +1875,7 @@ describe("gateway server chat", () => { }, ]); - firstCatalog.resolve([ - { - id: "vision-model", - name: "Vision Model", - provider: "test-provider", - input: ["text", "image"], - }, - ]); + firstCatalogSnapshot.resolve(createChatVisionModelCatalogSnapshot()); await first; expect(sendResponses).toEqual([ @@ -1903,7 +1907,7 @@ describe("gateway server chat", () => { expect(context.addChatRun).not.toHaveBeenCalled(); expect(context.removeChatRun).toHaveBeenCalledTimes(1); } finally { - firstCatalog.resolve([]); + firstCatalogSnapshot.resolve(createChatVisionModelCatalogSnapshot()); dispatchInboundMessageMock.mockReset(); testState.sessionStorePath = undefined; clearConfigCache(); diff --git a/src/gateway/session-utils-model.ts b/src/gateway/session-utils-model.ts index 3debd7a5448a..7aca0be566d3 100644 --- a/src/gateway/session-utils-model.ts +++ b/src/gateway/session-utils-model.ts @@ -14,6 +14,7 @@ import { modelSupportsInput, } from "../agents/model-catalog.js"; import { + findNormalizedProviderValue, inferUniqueProviderFromConfiguredModels, isCliProvider, parseModelRef, @@ -21,6 +22,7 @@ import { resolveDefaultModelForAgent, resolveThinkingDefault, } from "../agents/model-selection.js"; +import { publishedModelCatalogOwnerMatchesAgent } from "../agents/prepared-model-catalog-owner.js"; import { resolveSessionRuntimeOverrideForProvider } from "../agents/session-runtime-compat.js"; import { concretizeAgentRuntime, @@ -34,6 +36,7 @@ import { import { resolveAgentMainSessionKey, type SessionEntry } from "../config/sessions.js"; import type { OpenClawConfig } from "../config/types.openclaw.js"; import { normalizeAgentId } from "../routing/session-key.js"; +import type { GatewayModelCatalogSnapshot } from "./server-model-catalog.types.js"; import { createSessionRowModelCacheKey, type SessionListRowContext, @@ -305,8 +308,84 @@ export function getSessionDefaults( }; } +function normalizeGatewayModelCapabilityBaseUrl(value: string | undefined): string | undefined { + const baseUrl = normalizeOptionalString(value); + if (!baseUrl) { + return undefined; + } + try { + const parsed = new URL(baseUrl); + parsed.pathname = parsed.pathname.replace(/\/+$/u, "") || "/"; + return parsed.toString(); + } catch { + return baseUrl.replace(/\/+$/u, ""); + } +} + +function resolveGatewayProviderStaticModel(params: { + snapshot: GatewayModelCatalogSnapshot; + agentId?: string; + provider?: string; + model: string; + catalogEntry?: ModelCatalogEntry; +}): ModelCatalogEntry | undefined { + if ( + !params.agentId || + !params.provider || + !publishedModelCatalogOwnerMatchesAgent(params.snapshot, params.agentId) + ) { + return undefined; + } + const staticEntry = findModelCatalogEntry(params.snapshot.staticEntries ?? [], { + provider: params.provider, + modelId: params.model, + }); + if (!staticEntry) { + return undefined; + } + if (params.catalogEntry?.api && params.catalogEntry.api !== staticEntry.api) { + return undefined; + } + const catalogBaseUrl = normalizeGatewayModelCapabilityBaseUrl(params.catalogEntry?.baseUrl); + const staticBaseUrl = normalizeGatewayModelCapabilityBaseUrl(staticEntry.baseUrl); + if (catalogBaseUrl && catalogBaseUrl !== staticBaseUrl) { + return undefined; + } + + const configuredProvider = findNormalizedProviderValue( + params.snapshot.config.models?.providers, + params.provider, + ); + const normalizedModelId = normalizeLowercaseStringOrEmpty(params.model); + const configuredModel = configuredProvider?.models?.find( + (model) => normalizeLowercaseStringOrEmpty(model.id) === normalizedModelId, + ); + if (configuredModel?.input && !configuredModel.input.includes("image")) { + return undefined; + } + const configuredApi = configuredModel?.api ?? configuredProvider?.api; + if (configuredApi && configuredApi !== staticEntry.api) { + return undefined; + } + const configuredBaseUrl = normalizeGatewayModelCapabilityBaseUrl( + configuredModel?.baseUrl ?? configuredProvider?.baseUrl, + ); + if (configuredBaseUrl && configuredBaseUrl !== staticBaseUrl) { + return undefined; + } + return staticEntry; +} + export async function resolveGatewayModelSupportsImages(params: { - loadGatewayModelCatalog: (params?: { readOnly?: boolean }) => Promise; + loadGatewayModelCatalog: (params?: { + agentId?: string; + readOnly?: boolean; + }) => Promise; + loadGatewayModelCatalogSnapshot?: (params?: { + agentId?: string; + readOnly?: boolean; + }) => Promise; + agentId?: string; provider?: string; model?: string; }): Promise { @@ -315,11 +394,31 @@ export async function resolveGatewayModelSupportsImages(params: { } try { - const catalog = await params.loadGatewayModelCatalog({ readOnly: false }); - const modelEntry = findModelCatalogEntry(catalog, { + const loadParams = { + ...(params.agentId ? { agentId: params.agentId } : {}), + readOnly: false, + }; + const snapshot = params.loadGatewayModelCatalogSnapshot + ? await params.loadGatewayModelCatalogSnapshot(loadParams) + : undefined; + const catalog = snapshot ? snapshot.entries : await params.loadGatewayModelCatalog(loadParams); + const catalogEntry = findModelCatalogEntry(catalog, { provider: params.provider, modelId: params.model, }); + // Same-generation provider facts repair stale discovered capabilities without + // crossing agent ownership, physical routes, or authored input policy. + const staticEntry = + snapshot && (!catalogEntry || !modelSupportsInput(catalogEntry, "image")) + ? resolveGatewayProviderStaticModel({ + snapshot, + agentId: params.agentId, + provider: params.provider, + model: params.model, + catalogEntry, + }) + : undefined; + const modelEntry = staticEntry ?? catalogEntry; const normalizedProvider = normalizeOptionalLowercaseString( params.provider ?? modelEntry?.provider, ); diff --git a/src/gateway/session-utils.test.ts b/src/gateway/session-utils.test.ts index b6f9e61c50bb..c0b07a64f9dd 100644 --- a/src/gateway/session-utils.test.ts +++ b/src/gateway/session-utils.test.ts @@ -22,6 +22,7 @@ import { closeOpenClawAgentDatabasesForTest } from "../state/openclaw-agent-db.j import { closeOpenClawStateDatabaseForTest } from "../state/openclaw-state-db.js"; import { withStateDirEnv as withRawStateDirEnv } from "../test-helpers/state-dir-env.js"; import { normalizeSessionDeliveryState } from "../utils/delivery-context.shared.js"; +import type { GatewayModelCatalogSnapshot } from "./server-model-catalog.types.js"; import { registerSessionAutomationSource } from "./session-automation-index.js"; import { buildGatewaySessionEventFields } from "./session-event-payload.js"; import { capArrayByJsonBytes } from "./session-transcript-readers.js"; @@ -3344,6 +3345,422 @@ describe("deriveSessionTitle", () => { }); describe("resolveGatewayModelSupportsImages", () => { + const createModelCatalogSnapshot = (params: { + agentId?: string; + config?: OpenClawConfig; + entries?: GatewayModelCatalogSnapshot["entries"]; + staticEntries?: GatewayModelCatalogSnapshot["staticEntries"]; + }): GatewayModelCatalogSnapshot => ({ + agentId: params.agentId ?? "main", + agentDir: "/tmp/gateway-model-capability-agent", + workspaceDir: "/tmp/gateway-model-capability-workspace", + config: params.config ?? {}, + entries: params.entries ?? [], + routeVariants: [], + ...(params.staticEntries ? { staticEntries: params.staticEntries } : {}), + }); + + test("uses same-agent provider-static image capabilities missing from the visible catalog", async () => { + const loadGatewayModelCatalog = vi.fn(async () => []); + const loadGatewayModelCatalogSnapshot = vi.fn(async () => + createModelCatalogSnapshot({ + agentId: "qa", + staticEntries: [ + { + id: "gpt-5.4", + name: "GPT-5.4", + provider: "openai", + input: ["text", "image"], + }, + ], + }), + ); + + await expect( + resolveGatewayModelSupportsImages({ + agentId: "qa", + model: "gpt-5.4", + provider: "openai", + loadGatewayModelCatalog, + loadGatewayModelCatalogSnapshot, + }), + ).resolves.toBe(true); + expect(loadGatewayModelCatalogSnapshot).toHaveBeenCalledWith({ + agentId: "qa", + readOnly: false, + }); + expect(loadGatewayModelCatalog).not.toHaveBeenCalled(); + }); + + test("repairs a stale visible text-only row with same-agent provider-static vision", async () => { + await expect( + resolveGatewayModelSupportsImages({ + agentId: "qa", + model: "gpt-5.4", + provider: "openai", + loadGatewayModelCatalog: async () => [], + loadGatewayModelCatalogSnapshot: async () => + createModelCatalogSnapshot({ + agentId: "qa", + entries: [{ id: "gpt-5.4", name: "Text only", provider: "openai", input: ["text"] }], + staticEntries: [ + { + id: "gpt-5.4", + name: "GPT-5.4", + provider: "openai", + input: ["text", "image"], + }, + ], + }), + }), + ).resolves.toBe(true); + }); + + test("repairs missing visible input metadata with same-agent provider-static vision", async () => { + await expect( + resolveGatewayModelSupportsImages({ + agentId: "qa", + model: "gpt-5.4", + provider: "openai", + loadGatewayModelCatalog: async () => [], + loadGatewayModelCatalogSnapshot: async () => + createModelCatalogSnapshot({ + agentId: "qa", + entries: [{ id: "gpt-5.4", name: "Stale model", provider: "openai" }], + staticEntries: [ + { + id: "gpt-5.4", + name: "GPT-5.4", + provider: "openai", + input: ["text", "image"], + }, + ], + }), + }), + ).resolves.toBe(true); + }); + + test("does not borrow another agent's provider-static image capabilities", async () => { + await expect( + resolveGatewayModelSupportsImages({ + agentId: "qa", + model: "gpt-5.4", + provider: "openai", + loadGatewayModelCatalog: async () => [], + loadGatewayModelCatalogSnapshot: async () => + createModelCatalogSnapshot({ + agentId: "other", + staticEntries: [ + { + id: "gpt-5.4", + name: "GPT-5.4", + provider: "openai", + input: ["text", "image"], + }, + ], + }), + }), + ).resolves.toBe(false); + }); + + test("does not override an explicitly configured text-only model with provider-static vision", async () => { + await expect( + resolveGatewayModelSupportsImages({ + agentId: "qa", + model: "gpt-5.4", + provider: "openai", + loadGatewayModelCatalog: async () => [], + loadGatewayModelCatalogSnapshot: async () => + createModelCatalogSnapshot({ + agentId: "qa", + config: { + models: { + providers: { + openai: { + baseUrl: "https://api.openai.com/v1", + models: [ + { + id: "gpt-5.4", + name: "Text only", + reasoning: false, + input: ["text"], + cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0 }, + contextWindow: 128_000, + maxTokens: 4_096, + }, + ], + }, + }, + }, + }, + entries: [ + { + id: "gpt-5.4", + name: "Configured text only", + provider: "openai", + baseUrl: "https://api.openai.com/v1", + input: ["text"], + }, + ], + staticEntries: [ + { + id: "gpt-5.4", + name: "GPT-5.4", + provider: "openai", + baseUrl: "https://api.openai.com/v1", + input: ["text", "image"], + }, + ], + }), + }), + ).resolves.toBe(false); + }); + + test("does not borrow provider-static image capabilities across configured routes", async () => { + await expect( + resolveGatewayModelSupportsImages({ + agentId: "qa", + model: "gpt-5.4", + provider: "openai", + loadGatewayModelCatalog: async () => [], + loadGatewayModelCatalogSnapshot: async () => + createModelCatalogSnapshot({ + agentId: "qa", + config: { + models: { + providers: { + openai: { + baseUrl: "https://custom.example.test/v1", + models: [], + }, + }, + }, + }, + staticEntries: [ + { + id: "gpt-5.4", + name: "GPT-5.4", + provider: "openai", + baseUrl: "https://api.openai.com/v1", + input: ["text", "image"], + }, + ], + }), + }), + ).resolves.toBe(false); + }); + + test.each([ + { + route: "API", + api: "openai-completions", + baseUrl: "https://api.openai.com/v1", + }, + { + route: "base URL", + api: "openai-responses", + baseUrl: "https://custom.example.test/v1", + }, + ] as const)( + "does not borrow provider-static vision across a mismatched visible $route", + async ({ api, baseUrl }) => { + await expect( + resolveGatewayModelSupportsImages({ + agentId: "qa", + model: "gpt-5.4", + provider: "openai", + loadGatewayModelCatalog: async () => [], + loadGatewayModelCatalogSnapshot: async () => + createModelCatalogSnapshot({ + agentId: "qa", + entries: [ + { + id: "gpt-5.4", + name: "Custom route", + provider: "openai", + api, + baseUrl, + input: ["text"], + }, + ], + staticEntries: [ + { + id: "gpt-5.4", + name: "GPT-5.4", + provider: "openai", + api: "openai-responses", + baseUrl: "https://api.openai.com/v1", + input: ["text", "image"], + }, + ], + }), + }), + ).resolves.toBe(false); + }, + ); + + test.each([ + { + route: "visible API", + visibleRoute: { + api: "openai-completions" as const, + baseUrl: "https://api.openai.com/v1", + }, + configuredRoute: undefined, + staticRoute: { baseUrl: "https://api.openai.com/v1" }, + }, + { + route: "visible base URL", + visibleRoute: { + api: "openai-responses" as const, + baseUrl: "https://custom.example.test/v1", + }, + configuredRoute: undefined, + staticRoute: { api: "openai-responses" as const }, + }, + { + route: "configured API", + visibleRoute: undefined, + configuredRoute: { + api: "openai-completions" as const, + baseUrl: "https://api.openai.com/v1", + }, + staticRoute: { baseUrl: "https://api.openai.com/v1" }, + }, + { + route: "configured base URL", + visibleRoute: undefined, + configuredRoute: { baseUrl: "https://custom.example.test/v1" }, + staticRoute: { api: "openai-responses" as const }, + }, + ])( + "does not borrow provider-static vision when its $route provenance is missing", + async ({ visibleRoute, configuredRoute, staticRoute }) => { + await expect( + resolveGatewayModelSupportsImages({ + agentId: "qa", + model: "gpt-5.4", + provider: "openai", + loadGatewayModelCatalog: async () => [], + loadGatewayModelCatalogSnapshot: async () => + createModelCatalogSnapshot({ + ...(configuredRoute + ? { + config: { + models: { + providers: { + openai: { + baseUrl: configuredRoute.baseUrl, + ...("api" in configuredRoute ? { api: configuredRoute.api } : {}), + models: [], + }, + }, + }, + }, + } + : {}), + agentId: "qa", + entries: visibleRoute + ? [ + { + id: "gpt-5.4", + name: "Text only", + provider: "openai", + input: ["text"], + ...visibleRoute, + }, + ] + : [], + staticEntries: [ + { + id: "gpt-5.4", + name: "GPT-5.4", + provider: "openai", + input: ["text", "image"], + ...staticRoute, + }, + ], + }), + }), + ).resolves.toBe(false); + }, + ); + + test("does not borrow provider-static image capabilities from another provider", async () => { + await expect( + resolveGatewayModelSupportsImages({ + agentId: "qa", + model: "gpt-5.4", + provider: "openai", + loadGatewayModelCatalog: async () => [], + loadGatewayModelCatalogSnapshot: async () => + createModelCatalogSnapshot({ + agentId: "qa", + staticEntries: [ + { + id: "gpt-5.4", + name: "Other provider vision", + provider: "other", + input: ["text", "image"], + }, + ], + }), + }), + ).resolves.toBe(false); + }); + + test("fails closed on providerless provider-static image capabilities", async () => { + await expect( + resolveGatewayModelSupportsImages({ + agentId: "qa", + model: "shared-vision", + loadGatewayModelCatalog: async () => [], + loadGatewayModelCatalogSnapshot: async () => + createModelCatalogSnapshot({ + agentId: "qa", + staticEntries: [ + { + id: "shared-vision", + name: "First provider vision", + provider: "first", + input: ["text", "image"], + }, + { + id: "shared-vision", + name: "Second provider vision", + provider: "second", + input: ["text", "image"], + }, + ], + }), + }), + ).resolves.toBe(false); + }); + + test("fails closed without using a stale catalog when the prepared snapshot fails", async () => { + const loadGatewayModelCatalog = vi.fn(async () => [ + { + id: "gpt-5.4", + name: "GPT-5.4", + provider: "openai", + input: ["text", "image"] as ("text" | "image")[], + }, + ]); + + await expect( + resolveGatewayModelSupportsImages({ + agentId: "qa", + model: "gpt-5.4", + provider: "openai", + loadGatewayModelCatalog, + loadGatewayModelCatalogSnapshot: async () => { + throw new Error("prepared catalog unavailable"); + }, + }), + ).resolves.toBe(false); + expect(loadGatewayModelCatalog).not.toHaveBeenCalled(); + }); + test("keeps Foundry GPT deployments image-capable even when stale catalog metadata says text-only", async () => { await expect( resolveGatewayModelSupportsImages({ diff --git a/src/infra/heartbeat-runner-execution.ts b/src/infra/heartbeat-runner-execution.ts index ea146aa939e6..daace02224d7 100644 --- a/src/infra/heartbeat-runner-execution.ts +++ b/src/infra/heartbeat-runner-execution.ts @@ -4,6 +4,7 @@ import { resolveDefaultAgentId } from "../agents/agent-scope.js"; import { appendCronStyleCurrentTimeLine } from "../agents/current-time.js"; import { resolveEmbeddedSessionLane } from "../agents/embedded-agent-runner/lanes.js"; import { listActiveEmbeddedRunSessionKeys } from "../agents/embedded-agent-runner/run-state.js"; +import { transitionMainSessionRecovery } from "../agents/main-session-recovery-state.js"; import { resolveHeartbeatReplyPayload, resolveHeartbeatTerminalToolFailure, @@ -51,6 +52,7 @@ import { CommandLane } from "../process/lanes.js"; import { normalizeAgentId, parseAgentSessionKey } from "../routing/session-key.js"; import type { RuntimeEnv } from "../runtime.js"; import { createLazyRuntimeModule } from "../shared/lazy-runtime.js"; +import { getAgentEventLifecycleGeneration } from "./agent-events.js"; import { formatErrorMessage } from "./errors.js"; import { isWithinActiveHours } from "./heartbeat-active-hours.js"; import { emitHeartbeatEvent } from "./heartbeat-events.js"; @@ -246,12 +248,43 @@ export async function resolveHeartbeatWakeStage(opts: HeartbeatRunOptions) { // Phase 2: Stronger heartbeat deferral while a final delivery replay is pending. // Plain `updatedAt` changes are normal for heartbeat sessions and should not // suppress heartbeat runs; only defer when final delivery recovery is active. - const { entry: recentSessionEntry } = resolveHeartbeatSession( + const { sessionKey: recentSessionKey, entry: recentSessionEntry } = resolveHeartbeatSession( cfg, agentId, heartbeat, opts.sessionKey, ); + // Recovery can already have admitted its owner and cleared the abort flag; + // automatic and sentinel wakes must honor that canonical lifecycle fence. + const lifecycleGeneration = getAgentEventLifecycleGeneration(); + const mainSessionRecovery = + opts.intent !== "manual" && recentSessionEntry + ? transitionMainSessionRecovery(recentSessionEntry, { + kind: "inspect", + lifecycleGeneration, + sessionKey: recentSessionKey, + }) + : undefined; + const activeRestartRecoveryRunId = normalizeOptionalString( + recentSessionEntry?.restartRecoveryDeliveryRunId, + ); + // Delivery ownership can outlive the recovery aggregate. Only the matching + // run from this gateway generation may defer an automatic heartbeat. + const hasCurrentRestartRecoveryDelivery = + opts.intent !== "manual" && + activeRestartRecoveryRunId !== undefined && + recentSessionEntry?.restartRecoveryRuns?.some( + (run) => + run.runId === activeRestartRecoveryRunId && run.lifecycleGeneration === lifecycleGeneration, + ) === true; + if ( + (mainSessionRecovery?.kind === "observed" && + (mainSessionRecovery.view.status === "blocked" || + mainSessionRecovery.view.status === "recoverable")) || + hasCurrentRestartRecoveryDelivery + ) { + return { kind: "skipped", reason: HEARTBEAT_SKIP_REQUESTS_IN_FLIGHT } as const; + } const HEARTBEAT_DEFER_WINDOW_MS = 30_000; const pendingFinalDeliveryText = recentSessionEntry?.pendingFinalDelivery?.kind === "replayable" diff --git a/src/infra/heartbeat-runner.skips-busy-session-lane.test.ts b/src/infra/heartbeat-runner.skips-busy-session-lane.test.ts index 7b6b6ceb10e7..f5ea22df3072 100644 --- a/src/infra/heartbeat-runner.skips-busy-session-lane.test.ts +++ b/src/infra/heartbeat-runner.skips-busy-session-lane.test.ts @@ -10,6 +10,7 @@ import { getActivePluginRegistry, setActivePluginRegistry } from "../plugins/run import type { CommandLaneSnapshot } from "../process/command-queue.js"; import { CommandLane } from "../process/lanes.js"; import { createOutboundTestPlugin, createTestRegistry } from "../test-utils/channel-plugins.js"; +import { getAgentEventLifecycleGeneration } from "./agent-events.js"; import { type HeartbeatDeps, runHeartbeatOnce } from "./heartbeat-runner.js"; import { seedMainSessionStore, withTempHeartbeatSandbox } from "./heartbeat-runner.test-utils.js"; import { @@ -87,6 +88,208 @@ function createBusyLaneSnapshot(lane: string): CommandLaneSnapshot { } describe("heartbeat runner skips when target session lane is busy", () => { + it.each([ + { label: "scheduled", intent: "scheduled" as const }, + { label: "automatic immediate", intent: "immediate" as const }, + ])( + "defers $label heartbeat while main-session restart recovery owns the session", + async ({ intent }) => { + await withTempHeartbeatSandbox(async ({ storePath, replySpy }) => { + const cfg = createHeartbeatTelegramConfig(); + cfg.session = { store: storePath }; + await seedMainSessionStore(storePath, cfg, { + lastChannel: "telegram", + lastProvider: "telegram", + lastTo: "123", + status: "running", + abortedLastRun: true, + mainRestartRecovery: { + cycleId: "restart-cycle", + revision: 1, + chargedAttempts: 0, + }, + }); + + const result = await runHeartbeatOnce({ + cfg, + intent, + deps: { + getQueueSize: vi.fn((_lane?: string) => 0), + nowMs: () => Date.now(), + getReplyFromConfig: replySpy, + } as HeartbeatDeps, + }); + + expect(result).toEqual({ status: "skipped", reason: HEARTBEAT_SKIP_REQUESTS_IN_FLIGHT }); + expect(replySpy).not.toHaveBeenCalled(); + }); + }, + ); + + it("defers automatic heartbeat while an admitted recovery owns the current lifecycle", async () => { + await withTempHeartbeatSandbox(async ({ storePath, replySpy }) => { + const cfg = createHeartbeatTelegramConfig(); + cfg.session = { store: storePath }; + await seedMainSessionStore(storePath, cfg, { + lastChannel: "telegram", + lastProvider: "telegram", + lastTo: "123", + status: "running", + abortedLastRun: false, + mainRestartRecovery: { + cycleId: "restart-cycle", + revision: 2, + chargedAttempts: 1, + foregroundClaims: { + lifecycleGeneration: getAgentEventLifecycleGeneration(), + tokens: ["recovery-owner"], + }, + }, + }); + + const result = await runHeartbeatOnce({ + cfg, + intent: "immediate", + deps: { + getQueueSize: vi.fn((_lane?: string) => 0), + nowMs: () => Date.now(), + getReplyFromConfig: replySpy, + } as HeartbeatDeps, + }); + + expect(result).toEqual({ status: "skipped", reason: HEARTBEAT_SKIP_REQUESTS_IN_FLIGHT }); + expect(replySpy).not.toHaveBeenCalled(); + }); + }); + + it.each([ + { label: "scheduled", intent: "scheduled" as const }, + { label: "automatic immediate", intent: "immediate" as const }, + ])( + "defers $label heartbeat until the current restart recovery delivery settles", + async ({ intent }) => { + await withTempHeartbeatSandbox(async ({ storePath, replySpy }) => { + const cfg = createHeartbeatTelegramConfig(); + cfg.session = { store: storePath }; + await seedMainSessionStore(storePath, cfg, { + lastChannel: "telegram", + lastProvider: "telegram", + lastTo: "123", + status: "running", + abortedLastRun: false, + restartRecoveryDeliveryRunId: "restart-recovery-run", + restartRecoveryRuns: [ + { + runId: "restart-recovery-run", + lifecycleGeneration: getAgentEventLifecycleGeneration(), + }, + ], + }); + + const result = await runHeartbeatOnce({ + cfg, + intent, + deps: { + getQueueSize: vi.fn((_lane?: string) => 0), + nowMs: () => Date.now(), + getReplyFromConfig: replySpy, + } as HeartbeatDeps, + }); + + expect(result).toEqual({ status: "skipped", reason: HEARTBEAT_SKIP_REQUESTS_IN_FLIGHT }); + expect(replySpy).not.toHaveBeenCalled(); + }); + }, + ); + + it("does not block an explicit manual heartbeat on restart recovery delivery", async () => { + await withTempHeartbeatSandbox(async ({ storePath, replySpy }) => { + const cfg = createHeartbeatTelegramConfig(); + cfg.session = { store: storePath }; + await seedMainSessionStore(storePath, cfg, { + lastChannel: "telegram", + lastProvider: "telegram", + lastTo: "123", + status: "running", + abortedLastRun: false, + restartRecoveryDeliveryRunId: "restart-recovery-run", + restartRecoveryRuns: [ + { + runId: "restart-recovery-run", + lifecycleGeneration: getAgentEventLifecycleGeneration(), + }, + ], + }); + replySpy.mockResolvedValue({ text: "HEARTBEAT_OK" }); + + const result = await runHeartbeatOnce({ + cfg, + intent: "manual", + deps: { + getQueueSize: vi.fn((_lane?: string) => 0), + nowMs: () => Date.now(), + getReplyFromConfig: replySpy, + } as HeartbeatDeps, + }); + + expect(result.status).toBe("ran"); + expect(replySpy).toHaveBeenCalledOnce(); + }); + }); + + it.each([ + { + label: "a previous gateway lifecycle", + recoveryRun: { + runId: "restart-recovery-run", + lifecycleGeneration: "previous-gateway-lifecycle", + }, + }, + { + label: "another restart recovery run", + recoveryRun: { + runId: "another-restart-recovery-run", + lifecycleGeneration: "current-gateway-lifecycle", + }, + }, + ])("does not defer a heartbeat for $label", async ({ recoveryRun }) => { + await withTempHeartbeatSandbox(async ({ storePath, replySpy }) => { + const cfg = createHeartbeatTelegramConfig(); + cfg.session = { store: storePath }; + await seedMainSessionStore(storePath, cfg, { + lastChannel: "telegram", + lastProvider: "telegram", + lastTo: "123", + status: "running", + abortedLastRun: false, + restartRecoveryDeliveryRunId: "restart-recovery-run", + restartRecoveryRuns: [ + { + ...recoveryRun, + lifecycleGeneration: + recoveryRun.lifecycleGeneration === "current-gateway-lifecycle" + ? getAgentEventLifecycleGeneration() + : recoveryRun.lifecycleGeneration, + }, + ], + }); + replySpy.mockResolvedValue({ text: "HEARTBEAT_OK" }); + + const result = await runHeartbeatOnce({ + cfg, + intent: "scheduled", + deps: { + getQueueSize: vi.fn((_lane?: string) => 0), + nowMs: () => Date.now(), + getReplyFromConfig: replySpy, + } as HeartbeatDeps, + }); + + expect(result.status).toBe("ran"); + expect(replySpy).toHaveBeenCalledOnce(); + }); + }); + it("returns cron-in-progress when cron has an active job", async () => { await withTempHeartbeatSandbox(async ({ storePath, replySpy }) => { const cfg = createHeartbeatTelegramConfig(); diff --git a/src/infra/heartbeat-runner.test-utils.ts b/src/infra/heartbeat-runner.test-utils.ts index 7d0fa6aa4e70..2e14d7341272 100644 --- a/src/infra/heartbeat-runner.test-utils.ts +++ b/src/infra/heartbeat-runner.test-utils.ts @@ -6,7 +6,7 @@ import { vi } from "vitest"; import { heartbeatRunnerTelegramPlugin } from "../../test/helpers/infra/heartbeat-runner-channel-plugins.js"; import { resolveMainSessionKey } from "../config/sessions.js"; import { listSessionEntries, replaceSessionEntry } from "../config/sessions/session-accessor.js"; -import type { SessionEntry } from "../config/sessions/types.js"; +import type { InternalSessionEntry } from "../config/sessions/types.js"; import type { OpenClawConfig } from "../config/types.openclaw.js"; import { writeCronJobScratch } from "../cron/scratch-store.js"; import { CronService } from "../cron/service.js"; @@ -20,7 +20,7 @@ import type { HeartbeatDeps } from "./heartbeat-runner.js"; // Heartbeat test utilities seed session stores and temporary heartbeat prompts // while keeping plugin registry and environment state isolated per test. -type HeartbeatSessionSeed = Partial & { +type HeartbeatSessionSeed = Partial & { lastChannel: string; lastProvider: string; lastTo: string; diff --git a/src/infra/outbound/account-scoped-conversation-bindings.test.ts b/src/infra/outbound/account-scoped-conversation-bindings.test.ts new file mode 100644 index 000000000000..edfb67ec84fd --- /dev/null +++ b/src/infra/outbound/account-scoped-conversation-bindings.test.ts @@ -0,0 +1,167 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import type { OpenClawConfig } from "../../config/types.openclaw.js"; +import { + createAccountScopedConversationBindingManager, + resetAccountScopedConversationBindingsForTests, + type AccountScopedConversationBindingManager, +} from "./account-scoped-conversation-bindings.js"; +import { getSessionBindingService } from "./session-binding-service.js"; + +type TestBindingKind = "subagent" | "acp"; + +const stateKey = Symbol("openclaw.accountScopedConversationBindingExpiry.test"); +const startedAt = 1_700_000_000_000; +const baseCfg = { + session: { threadBindings: { idleHours: 1, maxAgeHours: 0 } }, +} satisfies OpenClawConfig; + +function createManager(params: { accountId?: string; cfg?: OpenClawConfig } = {}) { + return createAccountScopedConversationBindingManager({ + channel: "imessage", + cfg: params.cfg ?? baseCfg, + accountId: params.accountId ?? "ttl-owner", + stateKey, + toStoredTargetKind: (kind) => (kind === "subagent" ? "subagent" : "acp"), + toSessionBindingTargetKind: (kind) => (kind === "subagent" ? "subagent" : "session"), + }); +} + +function bindConversation( + manager: AccountScopedConversationBindingManager, + params: { + conversationId?: string; + targetSessionKey?: string; + label?: string; + } = {}, +) { + const binding = manager.bindConversation({ + conversationId: params.conversationId ?? "chat:ttl-owner", + targetKind: "subagent", + targetSessionKey: params.targetSessionKey ?? "agent:main:subagent:ttl-owner", + ...(params.label ? { metadata: { label: params.label } } : {}), + }); + if (!binding) { + throw new Error("expected an account-scoped conversation binding"); + } + return binding; +} + +describe("account-scoped conversation binding expiry", () => { + beforeEach(() => { + resetAccountScopedConversationBindingsForTests({ stateKey }); + }); + + afterEach(() => { + resetAccountScopedConversationBindingsForTests({ stateKey }); + vi.restoreAllMocks(); + }); + + it("expires idle bindings from both manager and session-service lookups", () => { + const now = vi.spyOn(Date, "now").mockReturnValue(startedAt); + const manager = createManager(); + const binding = bindConversation(manager); + const service = getSessionBindingService(); + const conversation = { + channel: "imessage", + accountId: manager.accountId, + conversationId: binding.conversationId, + }; + + expect(service.resolveByConversation(conversation)?.expiresAt).toBe(startedAt + 3_600_000); + expect(service.listBySession(binding.targetSessionKey)).toHaveLength(1); + + now.mockReturnValue(startedAt + 3_600_000); + + expect(manager.getByConversationId(binding.conversationId)).toBeUndefined(); + expect(manager.listBySessionKey(binding.targetSessionKey)).toEqual([]); + expect(service.resolveByConversation(conversation)).toBeNull(); + expect(service.listBySession(binding.targetSessionKey)).toEqual([]); + }); + + it("enforces maximum age even when activity refreshes the idle deadline", () => { + const now = vi.spyOn(Date, "now").mockReturnValue(startedAt); + const manager = createManager({ + cfg: { session: { threadBindings: { idleHours: 2, maxAgeHours: 1 } } }, + }); + const binding = bindConversation(manager); + + now.mockReturnValue(startedAt + 30 * 60_000); + expect(manager.touchConversation(binding.conversationId)?.lastActivityAt).toBe( + startedAt + 30 * 60_000, + ); + expect( + getSessionBindingService().resolveByConversation({ + channel: "imessage", + accountId: manager.accountId, + conversationId: binding.conversationId, + })?.expiresAt, + ).toBe(startedAt + 60 * 60_000); + + now.mockReturnValue(startedAt + 60 * 60_000); + + expect(manager.getByConversationId(binding.conversationId)).toBeUndefined(); + expect(manager.listBySessionKey(binding.targetSessionKey)).toEqual([]); + }); + + it("does not revive an expired binding when its conversation is touched", () => { + const now = vi.spyOn(Date, "now").mockReturnValue(startedAt); + const manager = createManager(); + const binding = bindConversation(manager); + + now.mockReturnValue(startedAt + 3_600_000); + + expect(manager.touchConversation(binding.conversationId, startedAt + 3_600_001)).toBeNull(); + expect(manager.getByConversationId(binding.conversationId)).toBeUndefined(); + expect(manager.listBySessionKey(binding.targetSessionKey)).toEqual([]); + }); + + it("does not inherit metadata from an expired binding when rebinding", () => { + const now = vi.spyOn(Date, "now").mockReturnValue(startedAt); + const manager = createManager(); + const expired = bindConversation(manager, { label: "expired-owner" }); + + now.mockReturnValue(startedAt + 3_600_000); + + const replacement = bindConversation(manager, { + conversationId: expired.conversationId, + targetSessionKey: "agent:main:subagent:replacement", + }); + + expect(replacement.label).toBeUndefined(); + expect(replacement.targetSessionKey).toBe("agent:main:subagent:replacement"); + expect(manager.getByConversationId(expired.conversationId)).toBe(replacement); + }); + + it("prunes only the expired account when accounts share a conversation id", () => { + const now = vi.spyOn(Date, "now").mockReturnValue(startedAt); + const expiredManager = createManager({ accountId: "ttl-expired" }); + const expired = bindConversation(expiredManager, { conversationId: "chat:shared" }); + + now.mockReturnValue(startedAt + 30 * 60_000); + const activeManager = createManager({ accountId: "ttl-active" }); + const active = bindConversation(activeManager, { conversationId: "chat:shared" }); + + now.mockReturnValue(startedAt + 60 * 60_000); + + expect(expiredManager.getByConversationId(expired.conversationId)).toBeUndefined(); + expect(activeManager.getByConversationId(active.conversationId)).toBe(active); + expect(activeManager.listBySessionKey(active.targetSessionKey)).toEqual([active]); + }); + + it("preserves bindings with disabled idle and maximum-age expiry", () => { + const now = vi.spyOn(Date, "now").mockReturnValue(startedAt); + const manager = createManager({ + cfg: { session: { threadBindings: { idleHours: 0, maxAgeHours: 0 } } }, + }); + const binding = bindConversation(manager); + + now.mockReturnValue(startedAt + 10 * 365 * 24 * 60 * 60_000); + + expect(manager.getByConversationId(binding.conversationId)).toBe(binding); + expect(manager.listBySessionKey(binding.targetSessionKey)).toEqual([binding]); + expect(manager.touchConversation(binding.conversationId)?.lastActivityAt).toBe(Date.now()); + expect(manager.unbindConversation(binding.conversationId)?.targetSessionKey).toBe( + binding.targetSessionKey, + ); + }); +}); diff --git a/src/infra/outbound/account-scoped-conversation-bindings.ts b/src/infra/outbound/account-scoped-conversation-bindings.ts index a715886b01a1..18646599d62e 100644 --- a/src/infra/outbound/account-scoped-conversation-bindings.ts +++ b/src/infra/outbound/account-scoped-conversation-bindings.ts @@ -1,3 +1,4 @@ +import { isFutureDateTimestampMs } from "@openclaw/normalization-core/number-coercion"; import { resolveDefaultAgentId } from "../../agents/agent-scope-config.js"; // Account-scoped conversation binding managers adapt channel-local thread maps // into the shared session binding service. @@ -148,25 +149,52 @@ export function createAccountScopedConversationBindingManager | undefined, + now = Date.now(), + ): AccountScopedConversationBindingRecord | undefined => { + if (!record) { + return undefined; + } + const { expiresAt } = toSessionBindingRecord({ + channel: params.channel, + record, + idleTimeoutMs, + maxAgeMs, + toSessionBindingTargetKind: params.toSessionBindingTargetKind, + }); + if (expiresAt === undefined || isFutureDateTimestampMs(expiresAt, { nowMs: now })) { + return record; + } + + // Prune at the account owner so SDK lookups and touches cannot revive stale bindings. + state.bindingsByAccountConversation.delete( + resolveBindingKey({ accountId, conversationId: record.conversationId }), + ); + return undefined; + }; const manager: AccountScopedConversationBindingManager = { accountId, getByConversationId: (conversationId) => - getState(params.stateKey).bindingsByAccountConversation.get( - resolveBindingKey({ accountId, conversationId }), - ), - listBySessionKey: (targetSessionKey) => - [...getState(params.stateKey).bindingsByAccountConversation.values()].filter( - (record) => record.accountId === accountId && record.targetSessionKey === targetSessionKey, + resolveActiveBinding( + state.bindingsByAccountConversation.get(resolveBindingKey({ accountId, conversationId })), ), + listBySessionKey: (targetSessionKey) => { + const now = Date.now(); + return [...state.bindingsByAccountConversation.values()].filter( + (record) => + record.accountId === accountId && + record.targetSessionKey === targetSessionKey && + resolveActiveBinding(record, now) !== undefined, + ); + }, bindConversation: ({ conversationId, targetKind, targetSessionKey, metadata }) => { const normalizedConversationId = conversationId.trim(); const normalizedTargetSessionKey = targetSessionKey.trim(); if (!normalizedConversationId || !normalizedTargetSessionKey) { return null; } - const existingLocal = getState(params.stateKey).bindingsByAccountConversation.get( - resolveBindingKey({ accountId, conversationId: normalizedConversationId }), - ); + const existingLocal = manager.getByConversationId(normalizedConversationId); const now = Date.now(); const record: AccountScopedConversationBindingRecord = { accountId, @@ -200,9 +228,7 @@ export function createAccountScopedConversationBindingManager { const key = resolveBindingKey({ accountId, conversationId }); - const existingRecord = getState(params.stateKey).bindingsByAccountConversation.get( - key, - ); + const existingRecord = manager.getByConversationId(conversationId); if (!existingRecord) { return null; } diff --git a/src/infra/outbound/targets-session.ts b/src/infra/outbound/targets-session.ts index 530cc27f5e3c..7d05a7942957 100644 --- a/src/infra/outbound/targets-session.ts +++ b/src/infra/outbound/targets-session.ts @@ -38,6 +38,7 @@ export type SessionDeliveryTarget = { function resolveParsedRouteTarget(params: { channel: string; + accountId?: string; rawTarget?: string | null; fallbackThreadId?: string | number | null; }) { @@ -54,6 +55,7 @@ function resolveParsedRouteTarget(params: { const threadId = normalizeOptionalThreadValue(parsed?.threadId ?? params.fallbackThreadId); return { channel, + accountId: params.accountId, rawTo, to: parsed?.to ?? rawTo, ...(threadId != null ? { threadId } : {}), @@ -89,6 +91,7 @@ export function resolveSessionDeliveryTarget(params: { const parsedSessionTarget = sessionLastChannel ? resolveParsedRouteTarget({ channel: sessionLastChannel, + accountId: context?.accountId, rawTarget: context?.to, fallbackThreadId: context?.threadId, }) @@ -99,6 +102,7 @@ export function resolveSessionDeliveryTarget(params: { hasTurnSourceChannel && params.turnSourceChannel ? resolveParsedRouteTarget({ channel: params.turnSourceChannel, + accountId: params.turnSourceAccountId, rawTarget: params.turnSourceTo, fallbackThreadId: params.turnSourceThreadId, }) @@ -117,8 +121,8 @@ export function resolveSessionDeliveryTarget(params: { left: parsedTurnSourceTarget, right: parsedSessionTarget, })); - // Shared sessions can receive cross-channel updates mid-turn; only inherit session threads - // when the turn source still identifies the same conversation. + // Shared sessions can receive cross-channel or cross-account updates mid-turn; + // only inherit session threads from the same account-scoped conversation. const lastThreadId = hasTurnSourceThreadId ? parsedTurnSourceTarget?.threadId : hasTurnSourceChannel && diff --git a/src/infra/outbound/targets.test.ts b/src/infra/outbound/targets.test.ts index af5c98b33dc2..61f382e00214 100644 --- a/src/infra/outbound/targets.test.ts +++ b/src/infra/outbound/targets.test.ts @@ -1716,6 +1716,68 @@ describe("resolveSessionDeliveryTarget — cross-channel reply guard (#24152)", expect(resolved.threadId).toBe(1122); }); + it.each([ + { + description: "matching account identities", + sessionAccountId: "work", + turnSourceAccountId: "work", + }, + { + description: "an unspecified turn-source account", + sessionAccountId: "work", + turnSourceAccountId: undefined, + }, + { + description: "an unspecified session account", + sessionAccountId: undefined, + turnSourceAccountId: "work", + }, + ])( + "keeps the session topic for compatible routes with $description", + ({ sessionAccountId, turnSourceAccountId }) => { + const resolved = resolveSessionDeliveryTarget({ + entry: { + sessionId: "sess-forum-compatible-account-topic", + updatedAt: 1, + lastChannel: "forum", + lastTo: "room:ops", + lastAccountId: sessionAccountId, + lastThreadId: 1122, + }, + requestedChannel: "last", + turnSourceChannel: "forum", + turnSourceTo: "room:ops", + turnSourceAccountId, + }); + + expect(resolved.accountId).toBe(turnSourceAccountId); + expect(resolved.threadId).toBe(1122); + expect(resolved.threadIdSource).toBe("session"); + }, + ); + + it("does not inherit a session topic from a different account on the same channel", () => { + const resolved = resolveSessionDeliveryTarget({ + entry: { + sessionId: "sess-forum-cross-account-topic", + updatedAt: 1, + lastChannel: "forum", + lastTo: "room:ops", + lastAccountId: "personal", + lastThreadId: 1122, + }, + requestedChannel: "last", + turnSourceChannel: "forum", + turnSourceTo: "room:ops", + turnSourceAccountId: "work", + }); + + expect(resolved.accountId).toBe("work"); + expect(resolved.threadId).toBeUndefined(); + expect(resolved.threadIdSource).toBeUndefined(); + expect(resolved.lastThreadId).toBeUndefined(); + }); + it("keeps topic thread routing when turnSourceTo uses the plugin-owned topic target", () => { const resolved = resolveSessionDeliveryTarget({ entry: { diff --git a/src/model-catalog/pricing.test.ts b/src/model-catalog/pricing.test.ts index 978785c7e600..6d4f8b753ffb 100644 --- a/src/model-catalog/pricing.test.ts +++ b/src/model-catalog/pricing.test.ts @@ -89,7 +89,7 @@ describe("hosted model pricing", () => { ).toEqual({ input: 2.5, output: 10, cacheRead: 1.25, cacheWrite: 0 }); }); - it("prefers merged catalog pricing over configured pricing", () => { + it("prefers configured pricing over merged catalog pricing", () => { const agentDir = tempDirs.make("openclaw-catalog-pricing-"); const config = { models: { @@ -109,7 +109,7 @@ describe("hosted model pricing", () => { } as unknown as OpenClawConfig; expect( resolveModelCostConfig({ config, agentDir, provider: "openai", model: "gpt-catalog" }), - ).toEqual({ input: 1, output: 2, cacheRead: 0, cacheWrite: 0 }); + ).toEqual({ input: 99, output: 99, cacheRead: 0, cacheWrite: 0 }); }); it("does not apply hosted pricing to private endpoints or unknown models", () => { diff --git a/src/plugin-sdk/qa-runtime.ts b/src/plugin-sdk/qa-runtime.ts index 91852e4fdf04..d25d6b606950 100644 --- a/src/plugin-sdk/qa-runtime.ts +++ b/src/plugin-sdk/qa-runtime.ts @@ -13,6 +13,8 @@ import type { QaRunnerCliRegistration } from "./qa-runner-runtime.js"; import { fetchWithSsrFGuard } from "./ssrf-runtime.js"; import { normalizeStringEntries } from "./string-coerce-runtime.js"; +export { writeGatewayRestartIntentSync } from "../infra/restart-intent.js"; + type QaRuntimeSurface = { defaultQaRuntimeModelForMode: ( mode: string, diff --git a/src/utils/usage-format.test.ts b/src/utils/usage-format.test.ts index 8cab1901c04d..d894edb3a5a3 100644 --- a/src/utils/usage-format.test.ts +++ b/src/utils/usage-format.test.ts @@ -206,6 +206,73 @@ describe("usage-format", () => { }); }); + it("prefers explicit configured pricing over a provider-owned static model price", () => { + const config = { + models: { + providers: { + openai: { + models: [ + { + id: "gpt-5.4", + cost: { input: 1, output: 2, cacheRead: 0, cacheWrite: 0 }, + }, + ], + }, + }, + }, + } as unknown as OpenClawConfig; + + expect( + resolveModelCostConfig({ + provider: "openai", + model: "gpt-5.4", + config, + }), + ).toEqual({ input: 1, output: 2, cacheRead: 0, cacheWrite: 0 }); + }); + + it("prefers agent-local pricing over configured and provider-owned static model prices", async () => { + const config = { + models: { + providers: { + openai: { + models: [ + { + id: "gpt-5.4", + cost: { input: 1, output: 2, cacheRead: 0, cacheWrite: 0 }, + }, + ], + }, + }, + }, + } as unknown as OpenClawConfig; + + await fs.writeFile( + path.join(agentDir, "models.json"), + JSON.stringify({ + providers: { + openai: { + models: [ + { + id: "gpt-5.4", + cost: { input: 7, output: 11, cacheRead: 0.5, cacheWrite: 0.25 }, + }, + ], + }, + }, + }), + "utf8", + ); + + expect( + resolveModelCostConfig({ + provider: "openai", + model: "gpt-5.4", + config, + }), + ).toEqual({ input: 7, output: 11, cacheRead: 0.5, cacheWrite: 0.25 }); + }); + it("scopes models.json pricing by agent directory before configured and default pricing", async () => { const secondAgentDir = path.join(stateDir, "agents", "second", "agent"); const configuredOnlyAgentDir = path.join(stateDir, "agents", "configured-only", "agent"); diff --git a/src/utils/usage-format.ts b/src/utils/usage-format.ts index 2a3e38e332bb..8b1222890362 100644 --- a/src/utils/usage-format.ts +++ b/src/utils/usage-format.ts @@ -565,17 +565,6 @@ export function resolveModelCostConfig(params: { return undefined; } const agentDir = resolveCostAgentDir(params.config, params.agentDir); - if (params.allowPluginNormalization !== false) { - const catalogPricing = resolveCatalogModelPricing({ - config: params.config, - provider: params.provider ?? "", - model: params.model ?? "", - }); - if (catalogPricing) { - return normalizeResolvedPricing(catalogPricing); - } - } - // Favor direct configured keys first so local pricing/status lookups stay // synchronous and do not drag plugin/provider discovery into the hot path. const rawModelsJsonCost = loadModelsJsonCostIndex({ @@ -613,6 +602,15 @@ export function resolveModelCostConfig(params: { } } + const catalogPricing = resolveCatalogModelPricing({ + config: params.config, + provider: params.provider ?? "", + model: params.model ?? "", + }); + if (catalogPricing) { + return normalizeResolvedPricing(catalogPricing); + } + const hostedPricing = resolveHostedModelPricing({ config: params.config, provider: params.provider ?? "", diff --git a/test/plugin-npm-package-manifest.test.ts b/test/plugin-npm-package-manifest.test.ts index 1a0a431ecbe1..9ae7a26da300 100644 --- a/test/plugin-npm-package-manifest.test.ts +++ b/test/plugin-npm-package-manifest.test.ts @@ -305,6 +305,153 @@ describe("plugin npm package manifest staging", () => { expect(readFileSync(join(packageDir, "package.json"), "utf8")).toBe(originalText); }); + it.each( + [ + { + label: "ESM configured-state", + metadataKey: "configuredState", + runtimeFormat: "esm", + sourceName: "configured-state", + exportName: "hasConfiguredChannelState", + }, + { + label: "CommonJS configured-state", + metadataKey: "configuredState", + runtimeFormat: "cjs", + sourceName: "configured-state", + exportName: "hasConfiguredChannelState", + }, + { + label: "ESM persisted-auth state", + metadataKey: "persistedAuthState", + runtimeFormat: "esm", + sourceName: "auth-presence", + exportName: "hasPersistedChannelAuth", + }, + { + label: "CommonJS persisted-auth state", + metadataKey: "persistedAuthState", + runtimeFormat: "cjs", + sourceName: "auth-presence", + exportName: "hasPersistedChannelAuth", + }, + ].flatMap((testCase) => [ + { ...testCase, label: `${testCase.label} from source`, specifierKind: "source" }, + { ...testCase, label: `${testCase.label} from built runtime`, specifierKind: "runtime" }, + ]), + )( + "packs and loads $label from the actual installed plugin runtime", + ({ metadataKey, runtimeFormat, sourceName, exportName, specifierKind }) => { + const repoDir = makeTempRepoRoot(tempDirs, "openclaw-plugin-npm-package-state-runtime-"); + const packageDir = writePublishablePluginPackage(repoDir); + const extension = runtimeFormat === "cjs" ? ".cjs" : ".js"; + const sourceSpecifier = `./${sourceName}`; + const runtimeSpecifier = `./dist/${sourceName}${extension}`; + const sourcePackageJson = JSON.parse(readFileSync(join(packageDir, "package.json"), "utf8")); + sourcePackageJson.openclaw.channel = { + id: "diffs", + [metadataKey]: { + specifier: specifierKind === "runtime" ? runtimeSpecifier : sourceSpecifier, + exportName, + }, + }; + if (runtimeFormat === "cjs") { + sourcePackageJson.openclaw.build = { runtimeFormat: "cjs" }; + } + writeJsonFile(join(packageDir, "package.json"), sourcePackageJson); + writeFileText(join(packageDir, `${sourceName}.ts`), `export function ${exportName}() {}\n`); + writeFileText(join(packageDir, "dist", `index${extension}`), "export {};\n"); + writeFileText(join(packageDir, "dist", `setup-entry${extension}`), "export {};\n"); + writeFileText( + join(packageDir, "dist", `${sourceName}${extension}`), + runtimeFormat === "cjs" + ? `exports.${exportName} = () => true;\n` + : `export function ${exportName}() { return true; }\n`, + ); + + const originalText = readFileSync(join(packageDir, "package.json"), "utf8"); + withAugmentedPluginNpmManifestForPackage({ repoRoot: repoDir, packageDir }, () => { + const stagedPackageJson = JSON.parse( + readFileSync(join(packageDir, "package.json"), "utf8"), + ); + expect(stagedPackageJson.openclaw.channel[metadataKey]).toEqual({ + specifier: runtimeSpecifier, + exportName, + }); + + const packedFiles = listNpmPackDryRunFiles(packageDir); + expect(packedFiles).toContain(runtimeSpecifier.slice(2)); + expect(packedFiles).not.toContain(`${sourceName}.ts`); + + const consumerDir = join(repoDir, "external-consumer"); + mkdirSync(consumerDir, { recursive: true }); + writeJsonFile(join(consumerDir, "package.json"), { private: true, type: "module" }); + + const packInvocation = resolvePluginNpmCommand([ + "pack", + "--json", + "--ignore-scripts", + "--pack-destination", + consumerDir, + ]); + const pack = spawnSync(packInvocation.command, packInvocation.args, { + cwd: packageDir, + encoding: "utf8", + ...(packInvocation.env ? { env: packInvocation.env } : {}), + ...(packInvocation.shell !== undefined ? { shell: packInvocation.shell } : {}), + stdio: ["ignore", "pipe", "pipe"], + ...(packInvocation.windowsVerbatimArguments !== undefined + ? { windowsVerbatimArguments: packInvocation.windowsVerbatimArguments } + : {}), + }); + expect(pack.status, pack.stderr).toBe(0); + const [packedPackage] = JSON.parse(pack.stdout) as [{ filename: string }]; + + const installInvocation = resolvePluginNpmCommand([ + "install", + "--ignore-scripts", + "--omit=peer", + "--no-audit", + "--no-fund", + "--package-lock=false", + join(consumerDir, packedPackage.filename), + ]); + const install = spawnSync(installInvocation.command, installInvocation.args, { + cwd: consumerDir, + encoding: "utf8", + ...(installInvocation.env ? { env: installInvocation.env } : {}), + ...(installInvocation.shell !== undefined ? { shell: installInvocation.shell } : {}), + stdio: ["ignore", "pipe", "pipe"], + ...(installInvocation.windowsVerbatimArguments !== undefined + ? { windowsVerbatimArguments: installInvocation.windowsVerbatimArguments } + : {}), + }); + expect(install.status, install.stderr).toBe(0); + + const installedRoot = join(consumerDir, "node_modules", "@openclaw", "diffs"); + const load = spawnSync( + process.execPath, + [ + "--input-type=module", + "--eval", + `import fs from "node:fs";\n` + + `import { pathToFileURL } from "node:url";\n` + + `const root = ${JSON.stringify(installedRoot)};\n` + + `const pkg = JSON.parse(fs.readFileSync(root + "/package.json", "utf8"));\n` + + `const state = pkg.openclaw.channel[${JSON.stringify(metadataKey)}];\n` + + `const loaded = await import(new URL(state.specifier, pathToFileURL(root + "/")));\n` + + `if (loaded[state.exportName]?.() !== true) throw new Error("installed state checker failed");\n` + + `process.stdout.write("INSTALLED_PLUGIN_CHANNEL_STATE_OK\\n");\n`, + ], + { cwd: consumerDir, encoding: "utf8", stdio: ["ignore", "pipe", "pipe"] }, + ); + expect(load.status, load.stderr).toBe(0); + expect(load.stdout).toBe("INSTALLED_PLUGIN_CHANNEL_STATE_OK\n"); + }); + expect(readFileSync(join(packageDir, "package.json"), "utf8")).toBe(originalText); + }, + ); + it("installs and cleans package-local bundled dependencies while packing", () => { const repoDir = makeTempRepoRoot(tempDirs, "openclaw-plugin-npm-package-bundled-deps-"); const packageDir = writePublishablePluginPackage(repoDir); diff --git a/test/scripts/docker-all-scheduler.test.ts b/test/scripts/docker-all-scheduler.test.ts index df0e1b96e602..7ffe3f9ea3e6 100644 --- a/test/scripts/docker-all-scheduler.test.ts +++ b/test/scripts/docker-all-scheduler.test.ts @@ -378,7 +378,7 @@ describe("scripts/test-docker-all scheduler", () => { } }); - it("writes a passing summary when a frozen target cannot run selected survivor lanes", () => { + it("fails with truthful artifacts when a frozen target cannot run selected survivor lanes", () => { const root = tempDirs.make("openclaw-docker-all-filtered-"); const logDir = path.join(root, "logs"); try { @@ -398,16 +398,61 @@ describe("scripts/test-docker-all scheduler", () => { }, }); - expect(result.status, result.stderr).toBe(0); + expect(result.status).toBe(1); expect(result.stdout).toContain("Docker lanes omitted"); + expect(result.stderr).toContain("resolved zero runnable Docker lanes"); + expect(result.stderr).toContain("published-upgrade-survivor"); const summary = JSON.parse(readFileSync(path.join(logDir, "summary.json"), "utf8")); - expect(summary.status).toBe("passed"); + expect(summary.status).toBe("failed"); expect(summary.lanes).toEqual([]); expect(summary.omittedUnsupportedLanes).toHaveLength(12); expect(summary.omittedUnsupportedLanes).toContain("published-upgrade-survivor"); expect(summary.omittedUnsupportedLanes).toContain( "published-upgrade-survivor-versioned-runtime-deps", ); + const failures = JSON.parse(readFileSync(path.join(logDir, "failures.json"), "utf8")); + expect(failures.status).toBe("failed"); + expect(failures.lanes).toEqual([]); + } finally { + rmSync(root, { force: true, recursive: true }); + } + }); + + it.each([ + { args: ["--plan-json"], dryRun: false, label: "JSON planning" }, + { args: [], dryRun: true, label: "dry runs" }, + ])("preserves $label when frozen survivor lanes are omitted", ({ args, dryRun }) => { + const root = tempDirs.make("openclaw-docker-all-filtered-plan-"); + const logDir = path.join(root, "logs"); + try { + writeFrozenScenarioContract(root, ["unrelated"]); + const result = spawnSync(process.execPath, ["scripts/test-docker-all.mjs", ...args], { + cwd: process.cwd(), + encoding: "utf8", + env: { + ...process.env, + OPENCLAW_ALLOW_FROZEN_TARGET_SCENARIO_OMISSIONS: "1", + OPENCLAW_DOCKER_ALL_BUILD: "0", + OPENCLAW_DOCKER_ALL_DRY_RUN: dryRun ? "1" : "0", + OPENCLAW_DOCKER_ALL_LANES: "published-upgrade-survivor", + OPENCLAW_DOCKER_ALL_LOG_DIR: logDir, + OPENCLAW_DOCKER_ALL_TIMINGS: "0", + OPENCLAW_UPGRADE_SURVIVOR_SCENARIOS: "reported-issues", + OPENCLAW_UPGRADE_SURVIVOR_TARGET_ROOT: root, + }, + }); + + expect(result.status, result.stderr).toBe(0); + if (dryRun) { + expect(result.stdout).toContain("Docker lanes omitted"); + expect(result.stdout).toContain("Dry run complete"); + } else { + const plan = JSON.parse(result.stdout); + expect(plan.lanes).toEqual([]); + expect(plan.omittedUnsupportedLanes).toHaveLength(12); + } + expect(existsSync(path.join(logDir, "summary.json"))).toBe(false); + expect(existsSync(path.join(logDir, "failures.json"))).toBe(false); } finally { rmSync(root, { force: true, recursive: true }); } diff --git a/test/scripts/npm-telegram-live.test.ts b/test/scripts/npm-telegram-live.test.ts index 77552347a4eb..a2b0c0c14898 100644 --- a/test/scripts/npm-telegram-live.test.ts +++ b/test/scripts/npm-telegram-live.test.ts @@ -392,7 +392,31 @@ describe("package Telegram live Docker E2E", () => { ).toThrow("invalid OPENCLAW_NPM_TELEGRAM_RTT_SAMPLES: 7samples"); }); - it("gates package Telegram status on the summary artifact", async () => { + it.each(["fail", "skip", "skipped", "timeout"])( + "fails package Telegram QA when a scenario has %s status", + async (status) => { + const summaryPath = path.join(mkTempRoot(), "qa-evidence.json"); + writeFileSync( + summaryPath, + JSON.stringify({ + kind: "openclaw.qa.evidence-summary", + schemaVersion: 2, + generatedAt: "2026-05-01T00:00:00.000Z", + entries: [{ result: { status } }], + }), + "utf8", + ); + + await expect( + testing.shouldFailPackageTelegramRun( + { summaryPath }, + { OPENCLAW_NPM_TELEGRAM_ALLOW_FAILURES: "" }, + ), + ).resolves.toBe(true); + }, + ); + + it("passes package Telegram QA when every scenario passes", async () => { const summaryPath = path.join(mkTempRoot(), "qa-evidence.json"); writeFileSync( summaryPath, @@ -400,7 +424,7 @@ describe("package Telegram live Docker E2E", () => { kind: "openclaw.qa.evidence-summary", schemaVersion: 2, generatedAt: "2026-05-01T00:00:00.000Z", - entries: [{ result: { status: "fail" } }], + entries: [{ result: { status: "pass" } }], }), "utf8", ); @@ -410,7 +434,7 @@ describe("package Telegram live Docker E2E", () => { { summaryPath }, { OPENCLAW_NPM_TELEGRAM_ALLOW_FAILURES: "" }, ), - ).resolves.toBe(true); + ).resolves.toBe(false); }); it("does not read package Telegram summaries when failures are allowed", async () => {