diff --git a/CHANGELOG.md b/CHANGELOG.md index d5d3e52859e5..ac4005800fc7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,11 +15,17 @@ Docs: https://docs.openclaw.ai ### Fixes +- **Child process output safety:** prevent stdout/stderr pipe failures from crashing agent exec sessions, local TUI shell commands, and bounded process execution. (#100407, #100406, #100410) Thanks @cxbAsDev. +- **Background refresh isolation:** keep remote skill-bin refreshes running when one node fails, and contain periodic subagent-sweeper failures without hiding errors from direct callers. (#100393, #100390) Thanks @cxbAsDev. +- **Skill scan diagnostics:** report directory enumeration failures through the existing resource diagnostics instead of silently dropping affected skills. (#100380) Thanks @wendy-chsy. +- **Exec output sanitization:** remove complete ANSI sequences and render residual C0/C1 controls as visible escapes instead of silently discarding output bytes. (#100327) Thanks @LavyaTandel. +- **Assistant visible text:** unwrap leaked standalone `` tags while preserving their content and literal code/XML examples. (#100302) Thanks @nankingjing. +- **Android microphone capture:** treat negative `AudioRecord.read` results as fatal shared-session errors so both transcription and Talk capture stop cleanly after device loss. (#100028) Thanks @NianJiuZst. - **iOS QR gateway handoff:** stop VisionKit before delivering scanned setup codes, and keep deferred auth, approval, Watch, and foreground-node work bound to its originating gateway across reconnects. (#99572) Thanks @PollyBot13. - **Agent terminal failures:** surface a safe interactive reply when an agent run ends without visible output, while preserving completed message-tool delivery and heartbeat-specific guidance. (#99304) Thanks @moeedahmed. - **MCP loopback tool results:** preserve schema-valid text, image, and embedded-resource content through HTTP tool calls while rendering malformed or protocol-incompatible blocks as safe text. (#100336) Thanks @tzy-17. - **Control UI tool-result images:** render direct image content blocks from Gateway history and make the delayed-send scroll E2E setup deterministic. (#100295) Thanks @lzyyzznl. -- **Plugin approval diagnostics:** surface Gateway validation rejection reasons while keeping transport and availability failures fail-closed. (#100337) Thanks @tzy-17. +- **Plugin approval diagnostics:** distinguish request validation rejections, expired wait decisions, and unavailable Gateways while keeping approval failures fail-closed. (#100337) Thanks @tzy-17. - **IRC Unicode messages:** split outbound PRIVMSG payloads on UTF-16 code-point boundaries so emoji cannot be cut into lone surrogates. (#96572) Thanks @llagy009. - **OpenAI realtime voice greetings:** prevent server VAD from creating a second outbound greeting while an explicit greeting response owns the turn, without disabling caller interruption. (#86285) Thanks @giodl73-repo. - **iOS Voice Wake cleanup:** avoid initializing the microphone audio pipeline while disabling inactive Voice Wake, preventing simulator launch aborts and unnecessary audio setup. diff --git a/apps/android/app/src/main/java/ai/openclaw/app/voice/AndroidAudioInputSession.kt b/apps/android/app/src/main/java/ai/openclaw/app/voice/AndroidAudioInputSession.kt index 7e08bbb02bbf..bf85a76aeaef 100644 --- a/apps/android/app/src/main/java/ai/openclaw/app/voice/AndroidAudioInputSession.kt +++ b/apps/android/app/src/main/java/ai/openclaw/app/voice/AndroidAudioInputSession.kt @@ -94,7 +94,7 @@ internal class AndroidAudioInputSession private constructor( buffer: ByteArray, offset: Int, size: Int, - ): Int = audioRecord.read(buffer, offset, size) + ): Int = checkAudioRecordReadResult(audioRecord.read(buffer, offset, size)) private fun openRoute() { audioManager.registerAudioDeviceCallback(deviceCallback, callbackHandler) @@ -203,6 +203,20 @@ private class BluetoothCommunicationRoute { private val bluetoothCommunicationRoute = BluetoothCommunicationRoute() +/** Converts AudioRecord's negative return codes into capture-session failures. */ +internal fun checkAudioRecordReadResult(result: Int): Int { + if (result >= 0) return result + val label = + when (result) { + AudioRecord.ERROR -> "ERROR" + AudioRecord.ERROR_BAD_VALUE -> "ERROR_BAD_VALUE" + AudioRecord.ERROR_INVALID_OPERATION -> "ERROR_INVALID_OPERATION" + AudioRecord.ERROR_DEAD_OBJECT -> "ERROR_DEAD_OBJECT" + else -> "code=$result" + } + throw IllegalStateException("microphone read failed: $label") +} + private fun selectBluetoothDevice( devices: List, current: AudioDeviceInfo? = null, diff --git a/apps/android/app/src/test/java/ai/openclaw/app/voice/AndroidAudioInputSessionTest.kt b/apps/android/app/src/test/java/ai/openclaw/app/voice/AndroidAudioInputSessionTest.kt index 4e1071225677..fe053bab9454 100644 --- a/apps/android/app/src/test/java/ai/openclaw/app/voice/AndroidAudioInputSessionTest.kt +++ b/apps/android/app/src/test/java/ai/openclaw/app/voice/AndroidAudioInputSessionTest.kt @@ -4,10 +4,12 @@ import android.Manifest import android.content.Context import android.media.AudioDeviceInfo import android.media.AudioManager +import android.media.AudioRecord import android.os.Looper import org.junit.After import org.junit.Assert.assertEquals import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue import org.junit.Before import org.junit.Test import org.junit.runner.RunWith @@ -107,6 +109,22 @@ class AndroidAudioInputSessionTest { assertNull(audioManager.communicationDevice) } + @Test + fun audioRecordErrorsFailTheSharedCaptureSession() { + assertEquals(0, checkAudioRecordReadResult(0)) + assertEquals(32, checkAudioRecordReadResult(32)) + + val deadObject = + runCatching { checkAudioRecordReadResult(AudioRecord.ERROR_DEAD_OBJECT) } + .exceptionOrNull() + assertTrue(deadObject is IllegalStateException) + assertEquals("microphone read failed: ERROR_DEAD_OBJECT", deadObject?.message) + + val unknown = runCatching { checkAudioRecordReadResult(-99) }.exceptionOrNull() + assertTrue(unknown is IllegalStateException) + assertEquals("microphone read failed: code=-99", unknown?.message) + } + private fun audioDevice(type: Int): AudioDeviceInfo { val device = AudioDeviceInfoBuilder diff --git a/packages/terminal-core/src/ansi.test.ts b/packages/terminal-core/src/ansi.test.ts index 43ebfa989ff0..e03b1e227c76 100644 --- a/packages/terminal-core/src/ansi.test.ts +++ b/packages/terminal-core/src/ansi.test.ts @@ -16,6 +16,9 @@ describe("terminal ansi helpers", () => { expect(stripAnsi("\u001B]8;;https://openclaw.ai\u001B\\link\u001B]8;;\u001B\\")).toBe("link"); expect(stripAnsi("\u001B]8;;https://openclaw.ai\u0007link\u001B]8;;\u0007")).toBe("link"); expect(stripAnsi("copy\u001B]52;c;YWJj\u0007safe")).toBe("copysafe"); + expect(stripAnsi("\u009B31mred\u009B0m")).toBe("red"); + expect(stripAnsi("\u009D8;;https://openclaw.ai\u009Clink\u009D8;;\u009C")).toBe("link"); + expect(stripAnsi("\u001B]unterminated")).toBe("\u001B]unterminated"); }); it("strips the agent output escape grammar without changing text policy", () => { @@ -39,6 +42,7 @@ describe("terminal ansi helpers", () => { String.fromCharCode(0x9b) + "done"; expect(sanitizeForLog(input)).toBe("warnnextlinedone"); + expect(sanitizeForLog("\u009B31mred\u009B0m")).toBe("red"); }); it("measures wide graphemes by terminal cell width", () => { diff --git a/packages/terminal-core/src/ansi.ts b/packages/terminal-core/src/ansi.ts index ff3bb2869232..87f6c676c30f 100644 --- a/packages/terminal-core/src/ansi.ts +++ b/packages/terminal-core/src/ansi.ts @@ -1,12 +1,19 @@ // Full CSI: ESC [ covers cursor movement, erase, and SGR. -const ANSI_CSI_PATTERN = "\\x1b\\[[\\x20-\\x3f]*[\\x40-\\x7e]"; +const ESC_ANSI_CSI_PATTERN = "\\x1b\\[[\\x20-\\x3f]*[\\x40-\\x7e]"; +const C1_ANSI_CSI_PATTERN = "\\x9b[\\x20-\\x3f]*[\\x40-\\x7e]"; +const PARAMETERIZED_C1_ANSI_CSI_PATTERN = "\\x9b[\\x20-\\x3f]+[\\x40-\\x7e]"; +const ANSI_CSI_PATTERN = `(?:${ESC_ANSI_CSI_PATTERN}|${C1_ANSI_CSI_PATTERN})`; // OSC: ESC ] ST. Covers OSC-8 hyperlinks and clipboard/title escapes. -// ST can be either ESC \ or BEL. -const ANSI_OSC_PATTERN = "\\x1b\\][^\\x07\\x1b]*(?:\\x1b\\\\|\\x07)"; +// ST can be ESC \, BEL, or its C1 form. +const ANSI_OSC_PATTERN = "(?:\\x1b\\]|\\x9d)[^\\x07\\x1b\\x9c]*(?:\\x1b\\\\|\\x07|\\x9c)"; const ANSI_CSI_REGEX = new RegExp(ANSI_CSI_PATTERN, "g"); const ANSI_OSC_REGEX = new RegExp(ANSI_OSC_PATTERN, "g"); const ANSI_SEQUENCE_REGEX = new RegExp(`${ANSI_OSC_PATTERN}|${ANSI_CSI_PATTERN}`, "g"); +const SANITIZATION_ANSI_SEQUENCE_REGEX = new RegExp( + `${ANSI_OSC_PATTERN}|${ESC_ANSI_CSI_PATTERN}|${PARAMETERIZED_C1_ANSI_CSI_PATTERN}`, + "g", +); /* * The following compatibility grammar is derived from ansi-regex and strip-ansi. @@ -50,6 +57,11 @@ export function stripAnsi(input: string): string { return input.replace(ANSI_OSC_REGEX, "").replace(ANSI_CSI_REGEX, ""); } +/** Strip complete ANSI while preserving ambiguous lone C1 controls for explicit escaping. */ +export function stripAnsiForSanitization(input: string): string { + return input.replace(SANITIZATION_ANSI_SEQUENCE_REGEX, ""); +} + export function stripAnsiSequences(input: string): string { if (typeof input !== "string") { throw new TypeError(`Expected a \`string\`, got \`${typeof input}\``); @@ -88,7 +100,7 @@ export function sanitizeForLog(v: string): string { const c1Start = String.fromCharCode(0x80); const c1End = String.fromCharCode(0x9f); const controlCharsRegex = new RegExp(`[${c0Start}-${c0End}${del}${c1Start}-${c1End}]`, "g"); - return stripAnsi(v).replace(controlCharsRegex, ""); + return stripAnsiForSanitization(v).replace(controlCharsRegex, ""); } function isZeroWidthCodePoint(codePoint: number): boolean { diff --git a/packages/terminal-core/src/safe-text.ts b/packages/terminal-core/src/safe-text.ts index 9a1ab6d2c8ee..bf13e2700164 100644 --- a/packages/terminal-core/src/safe-text.ts +++ b/packages/terminal-core/src/safe-text.ts @@ -1,11 +1,11 @@ // Terminal Core module implements safe text behavior. -import { stripAnsi } from "./ansi.js"; +import { stripAnsiForSanitization } from "./ansi.js"; /** * Normalize untrusted text for single-line terminal/log rendering. */ export function sanitizeTerminalText(input: string): string { - const normalized = stripAnsi(input) + const normalized = stripAnsiForSanitization(input) .replace(/\r/g, "\\r") .replace(/\n/g, "\\n") .replace(/\t/g, "\\t"); diff --git a/src/agents/agent-tools.before-tool-call.e2e.test.ts b/src/agents/agent-tools.before-tool-call.e2e.test.ts index 8d99cd97dbd9..879fbddcd5e4 100644 --- a/src/agents/agent-tools.before-tool-call.e2e.test.ts +++ b/src/agents/agent-tools.before-tool-call.e2e.test.ts @@ -1631,6 +1631,31 @@ describe("before_tool_call requireApproval handling", () => { expect(result).toHaveProperty("reason", expectedReason); }); + it("reports an expired accepted approval without calling it a request rejection", async () => { + hookRunner.runBeforeToolCall.mockResolvedValue({ + requireApproval: { title: "Approval", description: "Wait phase classification" }, + }); + mockCallGateway + .mockResolvedValueOnce({ id: "plugin:accepted", status: "accepted" }) + .mockRejectedValueOnce( + new GatewayClientRequestError({ + code: "INVALID_REQUEST", + message: "approval expired or not found", + }), + ); + + const result = await runBeforeToolCallHook({ + toolName: "bash", + params: {}, + ctx: { agentId: "main", sessionKey: "main" }, + }); + + expect(result).toHaveProperty( + "reason", + "Plugin approval no longer available: approval expired or not found", + ); + }); + it("blocks when gateway returns no id", async () => { hookRunner.runBeforeToolCall.mockResolvedValue({ requireApproval: { diff --git a/src/agents/agent-tools.before-tool-call.ts b/src/agents/agent-tools.before-tool-call.ts index e431c641ae03..9b4bff501dbc 100644 --- a/src/agents/agent-tools.before-tool-call.ts +++ b/src/agents/agent-tools.before-tool-call.ts @@ -702,6 +702,7 @@ async function requestPluginToolApproval(params: { const approval = params.approval; const timeoutMs = resolvePluginToolApprovalTimeoutMs(approval); const gatewayTimeoutMs = resolvePluginToolApprovalGatewayTimeoutMs(timeoutMs); + let gatewayApprovalPhase: "none" | "request" | "wait" = "none"; try { const embeddedApprovalBroker = isEmbeddedMode() ? getEmbeddedPluginApprovalBroker() : null; if (embeddedApprovalBroker) { @@ -767,6 +768,7 @@ async function requestPluginToolApproval(params: { }; } + gatewayApprovalPhase = "request"; const requestResult: { id?: string; status?: string; @@ -799,6 +801,7 @@ async function requestPluginToolApproval(params: { }, { expectFinal: false }, ); + gatewayApprovalPhase = "none"; const id = requestResult?.id; if (!id) { notifyPluginApprovalResolution(approval, PluginApprovalResolutions.CANCELLED); @@ -830,6 +833,7 @@ async function requestPluginToolApproval(params: { } else { // Wait for the decision, but abort early if the agent run is cancelled // so the user isn't blocked for the full approval timeout. + gatewayApprovalPhase = "wait"; const waitPromise: Promise<{ id?: string; decision?: string | null; @@ -929,13 +933,15 @@ async function requestPluginToolApproval(params: { params: params.baseParams, }; } - // UNAVAILABLE can also arrive as a structured response. Only validation - // rejection proves a healthy Gateway rejected the approval payload. - const requestRejected = + // INVALID_REQUEST means different things before and after registration. + const invalidRequest = err instanceof GatewayClientRequestError && err.gatewayCode === "INVALID_REQUEST"; - const reason = requestRejected - ? `Plugin approval request rejected: ${formatErrorMessage(err)}` - : "Plugin approval required (gateway unavailable)"; + const reason = + invalidRequest && gatewayApprovalPhase === "request" + ? `Plugin approval request rejected: ${formatErrorMessage(err)}` + : invalidRequest && gatewayApprovalPhase === "wait" + ? `Plugin approval no longer available: ${formatErrorMessage(err)}` + : "Plugin approval required (gateway unavailable)"; log.warn(`plugin approval gateway request failed; blocking tool call: ${String(err)}`); return { blocked: true, diff --git a/src/agents/sessions/exec.test.ts b/src/agents/sessions/exec.test.ts index ab2aa173eea3..c39764f52e75 100644 --- a/src/agents/sessions/exec.test.ts +++ b/src/agents/sessions/exec.test.ts @@ -168,4 +168,19 @@ describe("execCommand", () => { const result = await resultPromise; expect(result.killed).toBe(true); }); + + it("does not crash when stdout or stderr emit an error event", async () => { + const child = createStubChild(); + const wait = createDeferred(); + spawnMock.mockReturnValue(child); + waitForChildProcessMock.mockReturnValue(wait.promise); + const { execCommand } = await import("./exec.js"); + + const resultPromise = execCommand("cmd", [], "/tmp"); + child.stdout.emit("error", new Error("EPIPE")); + child.stderr.emit("error", new Error("EIO")); + wait.resolve(0); + + await expect(resultPromise).resolves.toMatchObject({ code: 0 }); + }); }); diff --git a/src/agents/sessions/exec.ts b/src/agents/sessions/exec.ts index 0ea9d753cf25..90812c8a9729 100644 --- a/src/agents/sessions/exec.ts +++ b/src/agents/sessions/exec.ts @@ -172,6 +172,11 @@ export async function execCommand( }, options.timeout); } + // Output pipes may fail independently; process termination remains authoritative. + const ignoreOutputStreamError = () => {}; + proc.stdout?.on("error", ignoreOutputStreamError); + proc.stderr?.on("error", ignoreOutputStreamError); + proc.stdout?.on("data", (data) => { const before = stdout.truncatedChars; stdout = appendCapturedOutput(stdout, data, maxOutputChars, truncateOutput); diff --git a/src/agents/shell-utils.test.ts b/src/agents/shell-utils.test.ts index 978b379b9c56..a29c6c97b3d1 100644 --- a/src/agents/shell-utils.test.ts +++ b/src/agents/shell-utils.test.ts @@ -12,10 +12,29 @@ import { resolveShellFromPath, resolveShellFromWhich, resolveWindowsBashPath, + sanitizeBinaryOutput, } from "./shell-utils.js"; const isWin = process.platform === "win32"; +describe("sanitizeBinaryOutput", () => { + it("removes ANSI wrappers while retaining printable output", () => { + expect(sanitizeBinaryOutput("\u001b[31mred\u001b[0m")).toBe("red"); + expect(sanitizeBinaryOutput("\u009b31mred\u009b0m")).toBe("red"); + }); + + it("preserves printable text after an incomplete ANSI sequence", () => { + expect(sanitizeBinaryOutput("\u001b]unterminated")).toBe("\\x1b]unterminated"); + expect(sanitizeBinaryOutput("\u009bdone")).toBe("\\x9bdone"); + }); + + it("escapes residual C0, DEL, and C1 controls", () => { + expect(sanitizeBinaryOutput("a\u0000\u0007\u007f\u0080b\t\n")).toBe( + "a\\x00\\x07\\x7f\\x80b\t\n", + ); + }); +}); + function createTempCommandDir( tempDirs: string[], files: Array<{ name: string; executable?: boolean }>, diff --git a/src/agents/shell-utils.ts b/src/agents/shell-utils.ts index caaac1bf4ee9..43ca9a502bce 100644 --- a/src/agents/shell-utils.ts +++ b/src/agents/shell-utils.ts @@ -6,6 +6,7 @@ import { spawnSync } from "node:child_process"; import fs from "node:fs"; import path from "node:path"; +import { stripAnsiForSanitization } from "../../packages/terminal-core/src/ansi.js"; import { killProcessTree as killProcessTreeGracefully, type KillProcessTreeOptions, @@ -262,7 +263,7 @@ export function detectRuntimeShell(): string | undefined { } export function sanitizeBinaryOutput(text: string): string { - const scrubbed = text.replace(/[\p{Format}\p{Surrogate}]/gu, ""); + const scrubbed = stripAnsiForSanitization(text).replace(/[\p{Format}\p{Surrogate}]/gu, ""); if (!scrubbed) { return scrubbed; } @@ -276,7 +277,8 @@ export function sanitizeBinaryOutput(text: string): string { chunks.push(char); continue; } - if (code < 0x20) { + if (code < 0x20 || (code >= 0x7f && code <= 0x9f)) { + chunks.push(`\\x${code.toString(16).padStart(2, "0")}`); continue; } chunks.push(char); diff --git a/src/agents/subagent-registry.test.ts b/src/agents/subagent-registry.test.ts index 79847214ddf8..09ec356a0687 100644 --- a/src/agents/subagent-registry.test.ts +++ b/src/agents/subagent-registry.test.ts @@ -5713,4 +5713,24 @@ describe("subagent registry seam flow", () => { expect(runs[41]?.delivery?.status).toBe("suspended"); expect(mocks.persistSubagentRunsToDisk).toHaveBeenCalled(); }); + + it("contains background sweeper failures while direct sweeps stay observable", async () => { + mod.registerSubagentRun({ + runId: "run-sweep-error", + childSessionKey: "agent:main:subagent:child", + requesterSessionKey: "agent:main:main", + requesterDisplayKey: "main", + task: "sweep error", + cleanup: "delete", + }); + const run = mod.getSubagentRunByChildSessionKey("agent:main:subagent:child"); + expect(run).toBeDefined(); + run!.startedAt = Date.now() - 2_000; + mocks.loadSessionStore.mockImplementation(() => { + throw new Error("simulated sweep failure"); + }); + + await expect(mod.testing.sweepOnceForTests()).rejects.toThrow("simulated sweep failure"); + await expect(mod.testing.runSweeperTickForTests()).resolves.toBeUndefined(); + }); }); diff --git a/src/agents/subagent-registry.ts b/src/agents/subagent-registry.ts index fb8f89bf372b..2d763edf58de 100644 --- a/src/agents/subagent-registry.ts +++ b/src/agents/subagent-registry.ts @@ -872,11 +872,19 @@ function startSweeper() { if (sweepInProgress) { return; } - void sweepSubagentRuns(); + void runSubagentSweep(); }, 60_000); sweeper.unref?.(); } +async function runSubagentSweep() { + try { + await sweepSubagentRuns(); + } catch (err) { + log.warn(`subagent run sweep failed: ${err instanceof Error ? err.message : String(err)}`); + } +} + function stopSweeper() { if (!sweeper) { return; @@ -1620,6 +1628,9 @@ export const testing = { async sweepOnceForTests() { await sweepSubagentRuns(); }, + async runSweeperTickForTests() { + await runSubagentSweep(); + }, setDepsForTest(overrides?: Partial) { subagentRegistryDeps = overrides ? { diff --git a/src/process/exec.test.ts b/src/process/exec.test.ts index 0b1fa0d357fe..b4a5dfc09988 100644 --- a/src/process/exec.test.ts +++ b/src/process/exec.test.ts @@ -346,6 +346,20 @@ describe("runCommandWithTimeout", () => { }, ); + it("does not crash when stdout or stderr emit an error event", async () => { + await loadExecModules({ mockSpawn: true }); + const child = createKilledChild(); + spawnMock.mockReturnValue(child); + + const resultPromise = runCommandWithTimeout(createSilentIdleArgv(), { timeoutMs: 2_000 }); + child.stdout?.emit("error", new Error("stdout read failed")); + child.stderr?.emit("error", new Error("stderr read failed")); + child.emit("exit", 0, null); + child.emit("close", 0, null); + + await expect(resultPromise).resolves.toMatchObject({ code: 0, termination: "exit" }); + }); + it("preserves matching output lines even when the tail capture truncates them", async () => { await loadExecModules(); const result = await runCommandWithTimeout( diff --git a/src/process/exec.ts b/src/process/exec.ts index 32e727b87958..dbbb0e7d1840 100644 --- a/src/process/exec.ts +++ b/src/process/exec.ts @@ -612,6 +612,10 @@ export async function runCommandWithTimeout( child.stdin.end(); } + // Output pipes may fail independently; child exit/close remains authoritative. + const ignoreOutputStreamError = () => {}; + child.stdout?.on("error", ignoreOutputStreamError); + child.stderr?.on("error", ignoreOutputStreamError); child.stdout?.on("data", (d) => { appendPreservedOutputLines({ capture: stdoutCapture, diff --git a/src/shared/text/assistant-visible-text.test.ts b/src/shared/text/assistant-visible-text.test.ts index fa1eef035674..4eeb8fd91bba 100644 --- a/src/shared/text/assistant-visible-text.test.ts +++ b/src/shared/text/assistant-visible-text.test.ts @@ -389,6 +389,51 @@ describe("stripAssistantInternalScaffolding", () => { ); }); + it("unwraps standalone parameter tags while preserving their content (#98557)", () => { + expectVisibleText( + 'Results: some content after.', + "Results: some content after.", + ); + expectVisibleText( + ['', "line 1", "line 2", ""].join("\n"), + "line 1\nline 2", + ); + expectVisibleText('{"key":"value"}', '{"key":"value"}'); + expectVisibleText('[1,2]', "[1,2]"); + expectVisibleText( + 'Results:\nline\nafter', + "Results:\nline\nafter", + ); + }); + + it("keeps truncated tool-call parameters fail-closed", () => { + expectVisibleText('secret', ""); + }); + + it("preserves parameter tags in code and literal function examples", () => { + expectVisibleText( + 'Use `/tmp`.', + 'Use `/tmp`.', + ); + expectVisibleText( + 'Use /tmp in docs.', + 'Use /tmp in docs.', + ); + expectVisibleText( + '/tmp', + '/tmp', + ); + expectVisibleText( + '', + '', + ); + expectVisibleText('
/tmp', "
/tmp"); + expectVisibleText( + 'Use declarations. /tmp', + "Use declarations. /tmp", + ); + }); + it("preserves XML-style explanations after lone tags", () => { expectVisibleText("Use literally.", "Use literally."); }); diff --git a/src/shared/text/assistant-visible-text.ts b/src/shared/text/assistant-visible-text.ts index 7947bf8023ed..55f1bc87e640 100644 --- a/src/shared/text/assistant-visible-text.ts +++ b/src/shared/text/assistant-visible-text.ts @@ -90,6 +90,61 @@ interface ParsedToolCallTag { isTruncated: boolean; } +function parseXmlTagAt(text: string, start: number): ParsedToolCallTag | null { + if (text[start] !== "<") { + return null; + } + + let cursor = start + 1; + while (cursor < text.length && /\s/.test(text[cursor])) { + cursor += 1; + } + + let isClose = false; + if (text[cursor] === "/") { + isClose = true; + cursor += 1; + while (cursor < text.length && /\s/.test(text[cursor])) { + cursor += 1; + } + } + + const nameStart = cursor; + if (!/[A-Za-z_:]/.test(text[cursor] ?? "")) { + return null; + } + cursor += 1; + while (cursor < text.length && /[A-Za-z0-9_.:-]/.test(text[cursor])) { + cursor += 1; + } + + const tagName = normalizeLowercaseStringOrEmpty(text.slice(nameStart, cursor)); + if (!isToolCallBoundary(text[cursor])) { + return null; + } + const contentStart = cursor; + const closeIndex = findTagCloseIndex(text, cursor); + if (closeIndex === -1) { + return { + contentStart, + end: text.length, + isClose, + isSelfClosing: false, + tagName, + isTruncated: true, + }; + } + + return { + contentStart, + end: closeIndex + 1, + isClose, + isSelfClosing: !isClose && /\/\s*$/.test(text.slice(cursor, closeIndex)), + tagName, + isTruncated: false, + }; +} + function isToolCallBoundary(char: string | undefined): boolean { return !char || /\s/.test(char) || char === "/" || char === ">"; } @@ -279,64 +334,148 @@ function findAdjacentOpeningToolCallTag( } function parseToolCallTagAt(text: string, start: number): ParsedToolCallTag | null { - if (text[start] !== "<") { - return null; - } + const tag = parseXmlTagAt(text, start); + return tag && TOOL_CALL_TAG_NAMES.has(tag.tagName) ? tag : null; +} - let cursor = start + 1; +function hasMatchingXmlCloseTag(text: string, start: number, tagName: string): boolean { + let depth = 1; + for (let idx = start; idx < text.length; idx += 1) { + if (text[idx] !== "<") { + continue; + } + const tag = parseXmlTagAt(text, idx); + if (!tag || tag.tagName !== tagName || tag.isTruncated) { + continue; + } + if (tag.isClose) { + depth -= 1; + if (depth === 0) { + return true; + } + } else if (!tag.isSelfClosing) { + depth += 1; + } + idx = Math.max(idx, tag.end - 1); + } + return false; +} + +function isDanglingFunctionParameterParent(text: string, tag: ParsedToolCallTag): boolean { + if (tag.tagName !== "function" || !/\bname\s*=/.test(text.slice(tag.contentStart, tag.end))) { + return false; + } + let cursor = tag.end; while (cursor < text.length && /\s/.test(text[cursor])) { cursor += 1; } + const nextTag = parseXmlTagAt(text, cursor); + return nextTag?.tagName === "parameter" && !nextTag.isClose; +} - let isClose = false; - if (text[cursor] === "/") { - isClose = true; +function consumeImmediateLineBreak(text: string, start: number): number | null { + if (text[start] === "\r" && text[start + 1] === "\n") { + return start + 2; + } + return text[start] === "\n" || text[start] === "\r" ? start + 1 : null; +} + +function trimImmediateLineBreakBefore(text: string, start: number, end: number): number { + if (end > start && text[end - 1] === "\n") { + return end - (end - 2 >= start && text[end - 2] === "\r" ? 2 : 1); + } + return end > start && text[end - 1] === "\r" ? end - 1 : end; +} + +function isLineStartAt(text: string, start: number): boolean { + let cursor = start - 1; + while (cursor >= 0 && (text[cursor] === " " || text[cursor] === "\t")) { + cursor -= 1; + } + return cursor < 0 || text[cursor] === "\n" || text[cursor] === "\r"; +} + +function isLineEndAfter(text: string, end: number): boolean { + let cursor = end; + while (cursor < text.length && (text[cursor] === " " || text[cursor] === "\t")) { cursor += 1; - while (cursor < text.length && /\s/.test(text[cursor])) { - cursor += 1; + } + return cursor >= text.length || text[cursor] === "\n" || text[cursor] === "\r"; +} + +function unwrapStandaloneParameterTags(text: string): string { + if (!/<\s*\/?\s*parameter\b/i.test(text)) { + return text; + } + + const codeRegions = findCodeRegions(text); + const openTags: Array<{ name: string; unwrap: boolean; trimBoundaryLineBreaks: boolean }> = []; + let result = ""; + let lastIndex = 0; + + for (let idx = 0; idx < text.length; idx += 1) { + if (text[idx] !== "<" || isInsideCode(idx, codeRegions)) { + continue; } + const tag = parseXmlTagAt(text, idx); + if (!tag || tag.isTruncated) { + continue; + } + + if (tag.isClose) { + const openIndex = openTags.findLastIndex((entry) => entry.name === tag.tagName); + if (openIndex !== -1) { + const opening = openTags[openIndex]; + if (opening.unwrap) { + const contentEnd = + opening.trimBoundaryLineBreaks && + isLineStartAt(text, idx) && + isLineEndAfter(text, tag.end) + ? trimImmediateLineBreakBefore(text, lastIndex, idx) + : idx; + result += text.slice(lastIndex, contentEnd); + lastIndex = tag.end; + } + openTags.splice(openIndex); + } + } else if (tag.isSelfClosing) { + if (tag.tagName === "parameter" && openTags.length === 0) { + result += text.slice(lastIndex, idx); + lastIndex = tag.end; + } + } else if ( + hasMatchingXmlCloseTag(text, tag.end, tag.tagName) || + isDanglingFunctionParameterParent(text, tag) + ) { + const unwrap = tag.tagName === "parameter" && openTags.length === 0; + let trimBoundaryLineBreaks = false; + if (unwrap) { + result += text.slice(lastIndex, idx); + lastIndex = tag.end; + const contentStart = isLineStartAt(text, idx) + ? consumeImmediateLineBreak(text, lastIndex) + : null; + if (contentStart !== null) { + lastIndex = contentStart; + trimBoundaryLineBreaks = true; + } + } + openTags.push({ name: tag.tagName, unwrap, trimBoundaryLineBreaks }); + } + idx = Math.max(idx, tag.end - 1); } - const nameStart = cursor; - while (cursor < text.length && /[A-Za-z_:]/.test(text[cursor])) { - cursor += 1; - } - - const tagName = normalizeLowercaseStringOrEmpty(text.slice(nameStart, cursor)); - if (!TOOL_CALL_TAG_NAMES.has(tagName) || !isToolCallBoundary(text[cursor])) { - return null; - } - const contentStart = cursor; - - const closeIndex = findTagCloseIndex(text, cursor); - if (closeIndex === -1) { - return { - contentStart, - end: text.length, - isClose, - isSelfClosing: false, - tagName, - isTruncated: true, - }; - } - - return { - contentStart, - end: closeIndex + 1, - isClose, - isSelfClosing: !isClose && /\/\s*$/.test(text.slice(cursor, closeIndex)), - tagName, - isTruncated: false, - }; + return result + text.slice(lastIndex); } export function stripToolCallXmlTags( - text: string, + input: string, options: { stripFunctionCallsXmlPayloads?: boolean; stripFunctionResponseAfterPluralToolCalls?: boolean; } = {}, ): string { + const text = input; if (!text || !TOOL_CALL_QUICK_RE.test(text)) { return text; } @@ -484,7 +623,7 @@ export function stripToolCallXmlTags( result += text.slice(toolCallBlockStart); } - return result; + return unwrapStandaloneParameterTags(result); } /** diff --git a/src/skills/loading/session.test.ts b/src/skills/loading/session.test.ts new file mode 100644 index 000000000000..653a6598aea1 --- /dev/null +++ b/src/skills/loading/session.test.ts @@ -0,0 +1,29 @@ +import fs from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; +import { afterEach, describe, expect, it } from "vitest"; +import { loadSkillsFromDir } from "./session.js"; + +describe("loadSkillsFromDir", () => { + const tempPaths: string[] = []; + + afterEach(async () => { + await Promise.all( + tempPaths.splice(0).map((entry) => fs.rm(entry, { recursive: true, force: true })), + ); + }); + + it("reports directory scan failures as diagnostics", async () => { + const tempDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-skill-scan-")); + tempPaths.push(tempDir); + const regularFile = path.join(tempDir, "not-a-directory"); + await fs.writeFile(regularFile, "not a skill directory"); + + const result = loadSkillsFromDir({ dir: regularFile, source: "test" }); + + expect(result.skills).toEqual([]); + expect(result.diagnostics).toEqual([ + expect.objectContaining({ type: "warning", path: regularFile }), + ]); + }); +}); diff --git a/src/skills/loading/session.ts b/src/skills/loading/session.ts index 71ad3b580f44..bd7e2041a2d9 100644 --- a/src/skills/loading/session.ts +++ b/src/skills/loading/session.ts @@ -225,7 +225,10 @@ function loadSkillsFromDirInternal( } diagnostics.push(...result.diagnostics); } - } catch {} + } catch (error) { + const message = error instanceof Error ? error.message : "failed to scan skill directory"; + diagnostics.push({ type: "warning", message, path: dir }); + } return { skills, diagnostics }; } diff --git a/src/skills/runtime/remote.test.ts b/src/skills/runtime/remote.test.ts index 9511f22a28e2..b19c94787524 100644 --- a/src/skills/runtime/remote.test.ts +++ b/src/skills/runtime/remote.test.ts @@ -12,6 +12,7 @@ import { recordRemoteNodeBins, recordRemoteNodeInfo, removeRemoteNodeInfo, + refreshRemoteBinsForConnectedNodes, refreshRemoteNodeBins, setSkillsRemoteRegistry, } from "./remote.js"; @@ -499,4 +500,45 @@ describe("skills-remote", () => { fs.rmSync(workspaceDir, { recursive: true, force: true }); } }); + + it("continues the connected-node refresh after one node fails", async () => { + await resetSkillsRefreshForTest(); + const nodeA = `node-${randomUUID()}`; + const nodeB = `node-${randomUUID()}`; + const bin = `bin-${randomUUID()}`; + const { cfg, workspaceDir } = createRemoteSkillWorkspace(bin); + try { + const invokeCalls: string[] = []; + setSkillsRemoteRegistry({ + listConnected: () => [ + { nodeId: nodeA, platform: "darwin", commands: ["system.run", "system.which"] }, + { nodeId: nodeB, platform: "darwin", commands: ["system.run", "system.which"] }, + ], + get: () => undefined, + checkConnectivity: (nodeId: string) => { + if (nodeId === nodeA) { + throw new Error("simulated connectivity failure"); + } + return { ok: true }; + }, + invoke: async (params: { command: string }) => { + invokeCalls.push(params.command); + return { ok: true, payloadJSON: JSON.stringify({ bins: [bin] }) }; + }, + } as unknown as NodeRegistry); + recordRemoteMacWithSystemWhich(nodeA); + recordRemoteMacWithSystemWhich(nodeB); + recordRemoteNodeBins(nodeA, ["stale-bin"]); + + await expect(refreshRemoteBinsForConnectedNodes(cfg)).resolves.toBeUndefined(); + + expect(invokeCalls).toEqual(["system.which"]); + expect(getRemoteSkillEligibility()?.hasBin(bin)).toBe(true); + expect(getRemoteSkillEligibility()?.hasBin("stale-bin")).toBe(false); + } finally { + removeRemoteNodeInfo(nodeA); + removeRemoteNodeInfo(nodeB); + fs.rmSync(workspaceDir, { recursive: true, force: true }); + } + }); }); diff --git a/src/skills/runtime/remote.ts b/src/skills/runtime/remote.ts index 4f8e4721656a..3d7a6323d4b4 100644 --- a/src/skills/runtime/remote.ts +++ b/src/skills/runtime/remote.ts @@ -372,10 +372,26 @@ async function refreshRemoteNodeBinsUncoalesced(params: { const connectivityTimeoutMs = Math.min(timeoutMs, 2_000); if (typeof remoteRegistry.checkConnectivity === "function") { const preflightConnId = remoteRegistry.get(params.nodeId)?.connId; - const connectivity = await remoteRegistry.checkConnectivity( - params.nodeId, - connectivityTimeoutMs, - ); + let connectivity: Awaited>; + try { + connectivity = await remoteRegistry.checkConnectivity(params.nodeId, connectivityTimeoutMs); + } catch (err) { + const cleared = clearRemoteNodeBins(params.nodeId); + logRemoteBinProbeFailure( + params.nodeId, + err, + { + command: "websocket.ping", + timeoutMs: connectivityTimeoutMs, + requiredBinCount: binsList.length, + }, + "preflight", + ); + if (cleared) { + bumpSkillsSnapshotVersion({ reason: "remote-node" }); + } + return; + } if (!connectivity.ok) { const latestSession = remoteRegistry.get(params.nodeId); if (preflightConnId && latestSession && latestSession.connId !== preflightConnId) { @@ -490,12 +506,17 @@ export async function refreshRemoteBinsForConnectedNodes(cfg: OpenClawConfig) { } const connected = remoteRegistry.listConnected(); for (const node of connected) { - await refreshRemoteNodeBins({ - nodeId: node.nodeId, - platform: node.platform, - deviceFamily: node.deviceFamily, - commands: node.commands, - cfg, - }); + try { + await refreshRemoteNodeBins({ + nodeId: node.nodeId, + platform: node.platform, + deviceFamily: node.deviceFamily, + commands: node.commands, + cfg, + }); + } catch (err) { + // A failed node must not abort refreshes for the remaining connected nodes. + log.warn(`failed to refresh remote bins for ${describeNode(node.nodeId)}: ${String(err)}`); + } } } diff --git a/src/tui/tui-local-shell.test.ts b/src/tui/tui-local-shell.test.ts index 8338a6bf66f1..aaeb9a190b06 100644 --- a/src/tui/tui-local-shell.test.ts +++ b/src/tui/tui-local-shell.test.ts @@ -177,4 +177,30 @@ describe("createLocalShellRunner", () => { "local shell: working directory was deleted; cd to an existing directory first", ); }); + + it("does not crash when stdout or stderr emit an error event", async () => { + const stdout = new EventEmitter(); + const stderr = new EventEmitter(); + const spawnCommand = vi.fn(() => ({ + stdout, + stderr, + on: (event: string, callback: (...args: unknown[]) => void) => { + if (event === "close") { + setImmediate(() => callback(0, null)); + } + }, + })); + const harness = createShellHarness({ + spawnCommand: spawnCommand as unknown as typeof import("node:child_process").spawn, + }); + + const run = harness.runLocalShellLine("!cmd"); + harness.getLastSelector()?.onSelect?.({ value: "yes", label: "Yes" }); + await vi.waitFor(() => expect(spawnCommand).toHaveBeenCalledTimes(1)); + stdout.emit("error", new Error("EPIPE")); + stderr.emit("error", new Error("EIO")); + + await expect(run).resolves.toBeUndefined(); + expect(harness.messages.some((message) => message.includes("exit 0"))).toBe(true); + }); }); diff --git a/src/tui/tui-local-shell.ts b/src/tui/tui-local-shell.ts index 5f7fcf0cf4df..5e40e83dc3c2 100644 --- a/src/tui/tui-local-shell.ts +++ b/src/tui/tui-local-shell.ts @@ -128,6 +128,10 @@ export function createLocalShellRunner(deps: LocalShellDeps) { let stdout = ""; let stderr = ""; + // Output pipes may fail independently; child close/error remains authoritative. + const ignoreOutputStreamError = () => {}; + child.stdout.on("error", ignoreOutputStreamError); + child.stderr.on("error", ignoreOutputStreamError); child.stdout.on("data", (buf) => { stdout = appendWithCap(stdout, buf.toString("utf8")); });