From 75c52b6c41adbf31a8c19741b8951941d5ba3770 Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Mon, 27 Apr 2026 00:33:29 -0700 Subject: [PATCH] fix(ci): expose package deps to Telegram QA harness (#72680) * fix(ci): expose package deps to telegram QA harness * fix(ci): link QA package runtime deps * fix(agents): guard replay metadata in empty retries * fix(ci): keep plugin update smoke migration-stable --- .../qa-lab/src/gateway-rpc-client.test.ts | 2 +- extensions/qa-lab/src/gateway-rpc-client.ts | 2 +- scripts/e2e/npm-telegram-live-docker.sh | 21 ++++++ scripts/e2e/plugin-update-unchanged-docker.sh | 15 +---- src/agents/pi-embedded-runner/replay-state.ts | 8 ++- .../run.incomplete-turn.test.ts | 30 +++++++++ src/agents/pi-embedded-runner/run.ts | 14 ++-- .../pi-embedded-runner/run/incomplete-turn.ts | 66 ++++++++++++------- test/scripts/npm-telegram-live.test.ts | 14 ++++ .../plugin-update-unchanged-docker.test.ts | 20 ++++++ 10 files changed, 145 insertions(+), 47 deletions(-) create mode 100644 test/scripts/plugin-update-unchanged-docker.test.ts diff --git a/extensions/qa-lab/src/gateway-rpc-client.test.ts b/extensions/qa-lab/src/gateway-rpc-client.test.ts index 885d646de17..eab529a961f 100644 --- a/extensions/qa-lab/src/gateway-rpc-client.test.ts +++ b/extensions/qa-lab/src/gateway-rpc-client.test.ts @@ -10,7 +10,7 @@ const gatewayRpcMock = vi.hoisted(() => { }; }); -vi.mock("./runtime-api.js", () => ({ +vi.mock("openclaw/plugin-sdk/browser-node-runtime", () => ({ callGatewayFromCli: gatewayRpcMock.callGatewayFromCli, })); diff --git a/extensions/qa-lab/src/gateway-rpc-client.ts b/extensions/qa-lab/src/gateway-rpc-client.ts index 56b4889e538..e4b16d95c5e 100644 --- a/extensions/qa-lab/src/gateway-rpc-client.ts +++ b/extensions/qa-lab/src/gateway-rpc-client.ts @@ -1,6 +1,6 @@ import { formatErrorMessage } from "openclaw/plugin-sdk/error-runtime"; +import { callGatewayFromCli } from "openclaw/plugin-sdk/browser-node-runtime"; import { formatQaGatewayLogsForError } from "./gateway-log-redaction.js"; -import { callGatewayFromCli } from "./runtime-api.js"; type QaGatewayRpcRequestOptions = { expectFinal?: boolean; diff --git a/scripts/e2e/npm-telegram-live-docker.sh b/scripts/e2e/npm-telegram-live-docker.sh index 4b4ef890770..d2f13c392dc 100755 --- a/scripts/e2e/npm-telegram-live-docker.sh +++ b/scripts/e2e/npm-telegram-live-docker.sh @@ -279,6 +279,27 @@ for deps_dir in "$openclaw_package_dir/node_modules" /npm-global/lib/node_module done done +link_installed_package_dependency() { + local name="$1" + local source="/npm-global/lib/node_modules/openclaw/node_modules/$name" + local target="/app/node_modules/$name" + if [ ! -e "$source" ]; then + echo "Installed package dependency is missing: $name" >&2 + return 1 + fi + mkdir -p "$(dirname "$target")" + ln -sfn "$source" "$target" +} + +# QA Lab is intentionally mounted as harness source, so its package-local +# runtime imports must resolve from the installed package dependency tree. +for dependency in \ + @modelcontextprotocol/sdk \ + yaml \ + zod; do + link_installed_package_dependency "$dependency" +done + echo "Running installed-package onboarding recovery hot path..." OPENAI_API_KEY="${OPENAI_API_KEY:-sk-openclaw-npm-telegram-hotpath}" openclaw onboard --non-interactive --accept-risk \ --mode local \ diff --git a/scripts/e2e/plugin-update-unchanged-docker.sh b/scripts/e2e/plugin-update-unchanged-docker.sh index 25e2ca49e26..41918a65896 100755 --- a/scripts/e2e/plugin-update-unchanged-docker.sh +++ b/scripts/e2e/plugin-update-unchanged-docker.sh @@ -43,20 +43,7 @@ JSON if [ \"\$OPENCLAW_PACKAGE_ACCEPTANCE_LEGACY_COMPAT\" = \"1\" ]; then cat > \"\$HOME/.openclaw/openclaw.json\" <<'JSON' { - \"plugins\": { - \"installs\": { - \"lossless-claw\": { - \"source\": \"npm\", - \"spec\": \"@example/lossless-claw@0.9.0\", - \"installPath\": \"~/.openclaw/extensions/lossless-claw\", - \"resolvedName\": \"@example/lossless-claw\", - \"resolvedVersion\": \"0.9.0\", - \"resolvedSpec\": \"@example/lossless-claw@0.9.0\", - \"integrity\": \"sha512-same\", - \"shasum\": \"same\" - } - } - } + \"plugins\": {} } JSON else diff --git a/src/agents/pi-embedded-runner/replay-state.ts b/src/agents/pi-embedded-runner/replay-state.ts index 19e68d11821..26822c74b95 100644 --- a/src/agents/pi-embedded-runner/replay-state.ts +++ b/src/agents/pi-embedded-runner/replay-state.ts @@ -33,8 +33,14 @@ export function mergeEmbeddedRunReplayState( export function observeReplayMetadata( current: EmbeddedRunReplayState, - metadata: EmbeddedRunReplayMetadata, + metadata?: EmbeddedRunReplayMetadata | null, ): EmbeddedRunReplayState { + if (!metadata) { + return mergeEmbeddedRunReplayState(current, { + replayInvalid: true, + hadPotentialSideEffects: true, + }); + } return mergeEmbeddedRunReplayState(current, { replayInvalid: !metadata.replaySafe, hadPotentialSideEffects: metadata.hadPotentialSideEffects, diff --git a/src/agents/pi-embedded-runner/run.incomplete-turn.test.ts b/src/agents/pi-embedded-runner/run.incomplete-turn.test.ts index 0a9c0383368..c0f1f77589d 100644 --- a/src/agents/pi-embedded-runner/run.incomplete-turn.test.ts +++ b/src/agents/pi-embedded-runner/run.incomplete-turn.test.ts @@ -922,6 +922,13 @@ describe("runEmbeddedPiAgent incomplete-turn safety", () => { ).toBe("abandoned"); }); + it("treats missing replay metadata as replay-invalid", () => { + const attempt = makeAttemptResult(); + delete (attempt as Partial).replayMetadata; + + expect(resolveReplayInvalidFlag({ attempt })).toBe(true); + }); + it("detects reasoning-only GPT turns from signed thinking blocks", () => { const retryInstruction = resolveReasoningOnlyRetryInstruction({ provider: "openai", @@ -1073,6 +1080,29 @@ describe("runEmbeddedPiAgent incomplete-turn safety", () => { expect(retryInstruction).toBe(EMPTY_RESPONSE_RETRY_INSTRUCTION); }); + it("does not retry clean zero-token Ollama stop turns", () => { + const retryInstruction = resolveEmptyResponseRetryInstruction({ + provider: "ollama", + modelId: "glm-5.1:cloud", + payloadCount: 0, + aborted: false, + timedOut: false, + attempt: makeAttemptResult({ + assistantTexts: [], + lastAssistant: { + role: "assistant", + stopReason: "stop", + provider: "ollama", + model: "glm-5.1:cloud", + content: [], + usage: { input: 100, output: 0, totalTokens: 100 }, + } as unknown as EmbeddedRunAttemptResult["lastAssistant"], + }), + }); + + expect(retryInstruction).toBeNull(); + }); + it("treats exact NO_REPLY as a deliberate silent assistant reply", () => { const incompleteTurnText = resolveIncompleteTurnPayloadText({ payloadCount: 0, diff --git a/src/agents/pi-embedded-runner/run.ts b/src/agents/pi-embedded-runner/run.ts index 5f2bf0c83dd..be3135f4e40 100644 --- a/src/agents/pi-embedded-runner/run.ts +++ b/src/agents/pi-embedded-runner/run.ts @@ -191,10 +191,10 @@ function createEmptyAuthProfileStore(): AuthProfileStore { } function buildTraceToolSummary(params: { - toolMetas: Array<{ toolName: string; meta?: string }>; + toolMetas?: Array<{ toolName: string; meta?: string }>; hadFailure: boolean; }): ToolSummaryTrace | undefined { - if (params.toolMetas.length === 0) { + if (!params.toolMetas?.length) { return undefined; } const tools: string[] = []; @@ -208,7 +208,7 @@ function buildTraceToolSummary(params: { tools.push(toolName); } return { - calls: params.toolMetas.length, + calls: params.toolMetas?.length ?? 0, tools, failures: params.hadFailure ? 1 : 0, }; @@ -1067,8 +1067,8 @@ export async function runEmbeddedPiAgent( !attempt.didSendViaMessagingTool && !attempt.didSendDeterministicApprovalPrompt && !attempt.lastToolError && - attempt.toolMetas.length === 0 && - attempt.assistantTexts.length === 0; + (attempt.toolMetas?.length ?? 0) === 0 && + (attempt.assistantTexts?.length ?? 0) === 0; if (preflightRecovery?.handled) { log.info( `[context-overflow-precheck] early recovery route=${preflightRecovery.route} ` + @@ -2000,7 +2000,7 @@ export async function runEmbeddedPiAgent( nextPlanningOnlyRetryInstruction && planningOnlyRetryAttempts < maxPlanningOnlyRetryAttempts ) { - const planningOnlyText = attempt.assistantTexts.join("\n\n").trim(); + const planningOnlyText = (attempt.assistantTexts ?? []).join("\n\n").trim(); const planDetails = extractPlanningOnlyPlanDetails(planningOnlyText); if (planDetails) { emitAgentPlanEvent({ @@ -2222,7 +2222,7 @@ export async function runEmbeddedPiAgent( sessionLastAssistant?.stopReason === "error" && ((sessionLastAssistant?.usage as { output?: number } | undefined)?.output ?? 0) === 0 && (silentErrorContent?.length ?? 0) === 0 && - !attempt.replayMetadata.hadPotentialSideEffects && + (attempt.replayMetadata ? !attempt.replayMetadata.hadPotentialSideEffects : false) && emptyErrorRetries < MAX_EMPTY_ERROR_RETRIES ) { emptyErrorRetries += 1; diff --git a/src/agents/pi-embedded-runner/run/incomplete-turn.ts b/src/agents/pi-embedded-runner/run/incomplete-turn.ts index 81ea308b3a0..112dfecd1c3 100644 --- a/src/agents/pi-embedded-runner/run/incomplete-turn.ts +++ b/src/agents/pi-embedded-runner/run/incomplete-turn.ts @@ -274,8 +274,12 @@ export function resolveIncompleteTurnPayloadText(params: { : "⚠️ Agent couldn't generate a response. Please try again."; } -function hasOnlySilentAssistantReply(assistantTexts: readonly string[]): boolean { - const nonEmptyTexts = assistantTexts.filter((text) => text.trim().length > 0); +function joinAssistantTexts(assistantTexts?: readonly string[]): string { + return (assistantTexts ?? []).join("\n\n").trim(); +} + +function hasOnlySilentAssistantReply(assistantTexts?: readonly string[]): boolean { + const nonEmptyTexts = (assistantTexts ?? []).filter((text) => text.trim().length > 0); return ( nonEmptyTexts.length > 0 && nonEmptyTexts.every((text) => isSilentReplyPayloadText(text, SILENT_REPLY_TOKEN)) @@ -342,12 +346,12 @@ export function resolveSilentToolResultReplyPayload(params: { params.payloadCount !== 0 || params.aborted || params.timedOut || - params.attempt.toolMetas.length === 0 || + (params.attempt.toolMetas?.length ?? 0) === 0 || params.attempt.clientToolCall || params.attempt.yieldDetected || params.attempt.didSendDeterministicApprovalPrompt || params.attempt.lastToolError || - params.attempt.messagesSnapshot.length === 0 + (params.attempt.messagesSnapshot?.length ?? 0) === 0 ) { return null; } @@ -411,7 +415,7 @@ function isEmptyResponseAssistantTurn(params: { if (params.payloadCount !== 0) { return false; } - if (params.attempt.assistantTexts.join("\n\n").trim().length > 0) { + if (joinAssistantTexts(params.attempt.assistantTexts).length > 0) { return false; } const assistant = params.attempt.currentAttemptAssistant ?? params.attempt.lastAssistant; @@ -446,7 +450,7 @@ function isNonVisibleAssistantTurnEligibleForSilentReply(params: { if (params.payloadCount !== 0) { return false; } - if (params.attempt.assistantTexts.join("\n\n").trim().length > 0) { + if (joinAssistantTexts(params.attempt.assistantTexts).length > 0) { return false; } const assistant = params.attempt.currentAttemptAssistant ?? params.attempt.lastAssistant; @@ -522,7 +526,7 @@ export function resolveReasoningOnlyRetryInstruction(params: { } const assistant = params.attempt.currentAttemptAssistant ?? params.attempt.lastAssistant; - if (params.attempt.assistantTexts.join("\n\n").trim().length > 0) { + if (joinAssistantTexts(params.attempt.assistantTexts).length > 0) { return null; } if (assistant?.stopReason === "error") { @@ -557,6 +561,16 @@ export function resolveEmptyResponseRetryInstruction(params: { return null; } + const assistant = params.attempt.currentAttemptAssistant ?? params.attempt.lastAssistant ?? null; + if ( + assistant?.stopReason === "stop" && + OLLAMA_INCOMPLETE_TURN_PROVIDER_ID_PATTERN.test( + normalizeLowercaseStringOrEmpty(params.provider ?? ""), + ) + ) { + return null; + } + if ( shouldApplyNonVisibleTurnRetryGuard({ provider: params.provider, @@ -566,9 +580,7 @@ export function resolveEmptyResponseRetryInstruction(params: { // Keep the generic zero-usage stop retry for providers that expose a // provider-neutral "nothing was generated" signal, even outside the // provider allowlist above. - isZeroUsageEmptyStopAssistantTurn( - params.attempt.currentAttemptAssistant ?? params.attempt.lastAssistant ?? null, - ) + isZeroUsageEmptyStopAssistantTurn(assistant) ) { return EMPTY_RESPONSE_RETRY_INSTRUCTION; } @@ -717,20 +729,28 @@ export function extractPlanningOnlyPlanDetails(text: string): PlanningOnlyPlanDe }; } -function countPlanOnlyToolMetas(toolMetas: PlanningOnlyAttempt["toolMetas"]): number { - return toolMetas.filter((entry) => entry.toolName === "update_plan").length; +function normalizePlanningToolMetas( + toolMetas?: PlanningOnlyAttempt["toolMetas"], +): PlanningOnlyAttempt["toolMetas"] { + return toolMetas ?? []; } -function countNonPlanToolCalls(toolMetas: PlanningOnlyAttempt["toolMetas"]): number { - return toolMetas.filter((entry) => entry.toolName !== "update_plan").length; +function countPlanOnlyToolMetas(toolMetas?: PlanningOnlyAttempt["toolMetas"]): number { + return normalizePlanningToolMetas(toolMetas).filter((entry) => entry.toolName === "update_plan") + .length; } -function hasNonPlanToolActivity(toolMetas: PlanningOnlyAttempt["toolMetas"]): boolean { - return toolMetas.some((entry) => entry.toolName !== "update_plan"); +function countNonPlanToolCalls(toolMetas?: PlanningOnlyAttempt["toolMetas"]): number { + return normalizePlanningToolMetas(toolMetas).filter((entry) => entry.toolName !== "update_plan") + .length; } -function hasSingleRetrySafeNonPlanTool(toolMetas: PlanningOnlyAttempt["toolMetas"]): boolean { - const nonPlanToolNames = toolMetas +function hasNonPlanToolActivity(toolMetas?: PlanningOnlyAttempt["toolMetas"]): boolean { + return normalizePlanningToolMetas(toolMetas).some((entry) => entry.toolName !== "update_plan"); +} + +function hasSingleRetrySafeNonPlanTool(toolMetas?: PlanningOnlyAttempt["toolMetas"]): boolean { + const nonPlanToolNames = normalizePlanningToolMetas(toolMetas) .map((entry) => normalizeLowercaseStringOrEmpty(entry.toolName)) .filter((toolName) => toolName && toolName !== "update_plan"); return ( @@ -746,14 +766,14 @@ function hasSingleRetrySafeNonPlanTool(toolMetas: PlanningOnlyAttempt["toolMetas * call path, which still counts as real multi-step progress. */ function isSingleActionThenNarrativePattern(params: { - toolMetas: PlanningOnlyAttempt["toolMetas"]; - assistantTexts: readonly string[]; + toolMetas?: PlanningOnlyAttempt["toolMetas"]; + assistantTexts?: readonly string[]; }): boolean { const nonPlanCount = countNonPlanToolCalls(params.toolMetas); if (nonPlanCount !== 1) { return false; } - const text = params.assistantTexts.join("\n\n").trim(); + const text = (params.assistantTexts ?? []).join("\n\n").trim(); if (!text || text.length > PLANNING_ONLY_MAX_VISIBLE_TEXT) { return false; } @@ -805,7 +825,7 @@ export function resolvePlanningOnlyRetryInstruction(params: { params.attempt.didSendViaMessagingTool || params.attempt.lastToolError || (hasNonPlanToolActivity(params.attempt.toolMetas) && !allowSingleActionRetryBypass) || - (params.attempt.itemLifecycle.startedCount > planOnlyToolMetaCount && + ((params.attempt.itemLifecycle?.startedCount ?? 0) > planOnlyToolMetaCount && !allowSingleActionRetryBypass) || resolveAttemptReplayMetadata(params.attempt).hadPotentialSideEffects ) { @@ -817,7 +837,7 @@ export function resolvePlanningOnlyRetryInstruction(params: { return null; } - const text = params.attempt.assistantTexts.join("\n\n").trim(); + const text = (params.attempt.assistantTexts ?? []).join("\n\n").trim(); if (!text || text.length > PLANNING_ONLY_MAX_VISIBLE_TEXT || text.includes("```")) { return null; } diff --git a/test/scripts/npm-telegram-live.test.ts b/test/scripts/npm-telegram-live.test.ts index e74ec6d727d..bd5bf18f18b 100644 --- a/test/scripts/npm-telegram-live.test.ts +++ b/test/scripts/npm-telegram-live.test.ts @@ -68,6 +68,20 @@ describe("package Telegram live Docker E2E", () => { expect(script).toContain('"./extensions/qa-channel/src/protocol.ts"'); }); + it("exposes installed package dependencies to the mounted QA harness", () => { + const script = readFileSync(DOCKER_SCRIPT_PATH, "utf8"); + + expect(script).toContain("link_installed_package_dependency()"); + expect(script).toContain( + 'local source="/npm-global/lib/node_modules/openclaw/node_modules/$name"', + ); + expect(script).toContain('ln -sfn "$source" "$target"'); + expect(script).toContain("link_installed_package_dependency \"$dependency\""); + expect(script).toContain("@modelcontextprotocol/sdk"); + expect(script).toContain("yaml"); + expect(script).toContain("zod"); + }); + it("lets npm-specific credential aliases override shared QA env", () => { expect( __testing.resolveCredentialSource({ diff --git a/test/scripts/plugin-update-unchanged-docker.test.ts b/test/scripts/plugin-update-unchanged-docker.test.ts new file mode 100644 index 00000000000..848edde9e34 --- /dev/null +++ b/test/scripts/plugin-update-unchanged-docker.test.ts @@ -0,0 +1,20 @@ +import { readFileSync } from "node:fs"; +import { describe, expect, it } from "vitest"; + +const PLUGIN_UPDATE_DOCKER_SCRIPT = "scripts/e2e/plugin-update-unchanged-docker.sh"; + +describe("plugin update unchanged Docker E2E", () => { + it("seeds current plugin install ledger state before checking config stability", () => { + const script = readFileSync(PLUGIN_UPDATE_DOCKER_SCRIPT, "utf8"); + const configSeedStart = script.indexOf('cat > \\"\\$HOME/.openclaw/openclaw.json\\"'); + const configSeedEnd = script.indexOf('cat > \\"\\$HOME/.openclaw/plugins/installs.json\\"'); + const configSeed = script.slice(configSeedStart, configSeedEnd); + + expect(configSeedStart).toBeGreaterThanOrEqual(0); + expect(configSeedEnd).toBeGreaterThan(configSeedStart); + expect(configSeed).toContain('\\"plugins\\": {}'); + expect(configSeed).not.toContain('\\"installs\\"'); + expect(script).toContain('\\"installRecords\\": {'); + expect(script).toContain('\\"lossless-claw\\": {'); + }); +});