From e169520fef2575164be6de8a604c869f999a6543 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Mon, 17 Aug 2026 17:14:05 -0700 Subject: [PATCH] fix: surface swallowed failures on action paths (#125319) * fix: surface swallowed failures on action paths * fix(memory): propagate directory traversal failures --- config/assertion-safety-baseline.txt | 1 - .../memory-core/src/memory/index.test.ts | 35 +++++++++++ extensions/qa-lab/src/bus-server.test.ts | 16 +++++ extensions/qa-lab/src/bus-server.ts | 16 ++++- extensions/qa-lab/src/lab-server.test.ts | 15 +++++ extensions/qa-lab/src/lab-server.ts | 5 +- packages/memory-host-sdk/src/host/fs-utils.ts | 15 ++--- .../memory-host-sdk/src/host/internal.test.ts | 63 +++++++++++++++++++ packages/memory-host-sdk/src/host/internal.ts | 22 ++++++- src/memory/root-memory-files.ts | 7 ++- 10 files changed, 179 insertions(+), 16 deletions(-) diff --git a/config/assertion-safety-baseline.txt b/config/assertion-safety-baseline.txt index e98eca3e639a..c98fdac7a6f7 100644 --- a/config/assertion-safety-baseline.txt +++ b/config/assertion-safety-baseline.txt @@ -1621,7 +1621,6 @@ packages/memory-host-sdk/src/host/batch-http.ts 2 packages/memory-host-sdk/src/host/batch-output.ts 2 packages/memory-host-sdk/src/host/batch-upload.ts 1 packages/memory-host-sdk/src/host/config-utils.ts 2 -packages/memory-host-sdk/src/host/fs-utils.ts 3 packages/memory-host-sdk/src/host/internal.ts 1 packages/memory-host-sdk/src/host/memory-schema-fts.ts 2 packages/memory-host-sdk/src/host/memory-schema-migration.ts 1 diff --git a/extensions/memory-core/src/memory/index.test.ts b/extensions/memory-core/src/memory/index.test.ts index efaf3de65d49..e842e12308c0 100644 --- a/extensions/memory-core/src/memory/index.test.ts +++ b/extensions/memory-core/src/memory/index.test.ts @@ -549,6 +549,41 @@ describe("memory index", () => { } }); + it("keeps indexed memory searchable when source discovery fails", async () => { + const memoryPath = path.join(fixture.paths.workspace, "MEMORY.md"); + const query = "harbor seal migration ritual"; + await fs.writeFile(memoryPath, `Remember the ${query}.\n`); + const manager = await getFreshManager(createCfg({})); + try { + await manager.sync({ reason: "test", force: true }); + await expect(manager.search(query)).resolves.toEqual([ + expect.objectContaining({ path: "MEMORY.md" }), + ]); + + const scanError = Object.assign(new Error("workspace scan failed"), { code: "EIO" }); + const realReaddir = fs.readdir; + const readdirSpy = vi + .spyOn(fs, "readdir") + .mockImplementation(async (...args: Parameters) => { + if (path.resolve(String(args[0])) === fixture.paths.workspace) { + throw scanError; + } + return await realReaddir(...args); + }); + try { + await expect(manager.sync({ reason: "cli", force: true })).rejects.toBe(scanError); + } finally { + readdirSpy.mockRestore(); + } + + await expect(manager.search(query)).resolves.toEqual([ + expect.objectContaining({ path: "MEMORY.md" }), + ]); + } finally { + await manager.close?.(); + } + }); + it("reindexes memory tables in place without deleting unrelated agent rows", async () => { const stateDir = path.join(fixture.paths.workspace, "managed-memory-state"); fixture.setStateDir(stateDir); diff --git a/extensions/qa-lab/src/bus-server.test.ts b/extensions/qa-lab/src/bus-server.test.ts index aa99374e40d2..ae303dc929a6 100644 --- a/extensions/qa-lab/src/bus-server.test.ts +++ b/extensions/qa-lab/src/bus-server.test.ts @@ -112,6 +112,22 @@ describe("qa-bus server", () => { await Promise.all(stops.splice(0).map((stop) => stop())); }); + it("returns a 500 JSON response when request handling rejects", async () => { + const state = createQaBusState(); + const requestError = new Error("snapshot unavailable"); + state.getSnapshot = () => { + throw requestError; + }; + const bus = await startQaBusServer({ state }); + stops.push(async () => await bus.stop()); + + const response = await fetch(`${bus.baseUrl}/v1/state`, { + signal: AbortSignal.timeout(1_000), + }); + expect(response.status).toBe(500); + await expect(response.json()).resolves.toEqual({ error: requestError.message }); + }); + it("wakes matching polls and fences late polls and writes during shutdown", async () => { const state = createQaBusState(); const bus = await startQaBusServer({ state }); diff --git a/extensions/qa-lab/src/bus-server.ts b/extensions/qa-lab/src/bus-server.ts index 3ab5e3fa733c..bc62466c515a 100644 --- a/extensions/qa-lab/src/bus-server.ts +++ b/extensions/qa-lab/src/bus-server.ts @@ -211,6 +211,18 @@ export function writeError(res: ServerResponse, statusCode: number, error: unkno }); } +export function dispatchQaHttpRequest(res: ServerResponse, task: () => Promise): void { + // Node does not observe promises returned by request listeners. Own rejection here so + // every admitted request receives an HTTP failure or an explicit connection close. + void task().catch((error: unknown) => { + if (res.headersSent) { + res.destroy(error instanceof Error ? error : new Error(formatErrorMessage(error))); + return; + } + writeError(res, 500, error); + }); +} + export function writeQaRequestBodyLimitError(res: ServerResponse, error: unknown): boolean { if (!isRequestBodyLimitError(error)) { return false; @@ -448,12 +460,12 @@ export async function handleQaBusRequest(params: { export function createQaBusServer(state: QaBusState): Server { return createServer((req, res) => { - void (async () => { + dispatchQaHttpRequest(res, async () => { const handled = await handleQaBusRequest({ req, res, state }); if (!handled) { writeError(res, 404, "not found"); } - })(); + }); }); } diff --git a/extensions/qa-lab/src/lab-server.test.ts b/extensions/qa-lab/src/lab-server.test.ts index c7229172374b..697562f372bd 100644 --- a/extensions/qa-lab/src/lab-server.test.ts +++ b/extensions/qa-lab/src/lab-server.test.ts @@ -402,6 +402,21 @@ async function createQaLabSuiteResultFixture(params?: { } describe("qa-lab server", () => { + it("returns a 500 JSON response when a shared bus route rejects", async () => { + const lab = await startQaLabServerForTest(); + cleanups.push(async () => await lab.stop()); + const requestError = new Error("combined snapshot unavailable"); + lab.state.getSnapshot = () => { + throw requestError; + }; + + const response = await fetch(`${lab.baseUrl}/v1/state`, { + signal: AbortSignal.timeout(1_000), + }); + expect(response.status).toBe(500); + await expect(response.json()).resolves.toEqual({ error: requestError.message }); + }); + it("dispatches explicit mixed-kind selections through the suite planner", async () => { const lab = await startQaLabServerForTest(); cleanups.push(async () => { diff --git a/extensions/qa-lab/src/lab-server.ts b/extensions/qa-lab/src/lab-server.ts index 5dd693a187c9..039b0611dbf6 100644 --- a/extensions/qa-lab/src/lab-server.ts +++ b/extensions/qa-lab/src/lab-server.ts @@ -10,6 +10,7 @@ import { } from "openclaw/plugin-sdk/proxy-capture"; import { closeQaHttpServer, + dispatchQaHttpRequest, handleQaBusRequest, isQaMalformedJsonBodyError, readQaJsonBody, @@ -449,7 +450,7 @@ export async function startQaLabServer( } const server = createServer((req, res) => { - void (async () => { + dispatchQaHttpRequest(res, async () => { const url = new URL(req.url ?? "/", "http://127.0.0.1"); if (await handleQaBusRequest({ req, res, state })) { @@ -933,7 +934,7 @@ export async function startQaLabServer( } catch (error) { writeQaLabServerError(res, error); } - })(); + }); }); const releaseCaptureStore = () => { diff --git a/packages/memory-host-sdk/src/host/fs-utils.ts b/packages/memory-host-sdk/src/host/fs-utils.ts index 47a410e12364..c34d37b542ab 100644 --- a/packages/memory-host-sdk/src/host/fs-utils.ts +++ b/packages/memory-host-sdk/src/host/fs-utils.ts @@ -28,12 +28,13 @@ if (!hasModeOverride) { export function isFileMissingError( err: unknown, ): err is NodeJS.ErrnoException & { code: "ENOENT" | "ENOTDIR" | "not-file" | "not-found" } { - return Boolean( - err && - typeof err === "object" && - "code" in err && - ((err as Partial).code === "ENOENT" || - (err as Partial).code === "ENOTDIR" || - (err as { code?: unknown }).code === "not-found"), + if (!err || typeof err !== "object" || !("code" in err)) { + return false; + } + return ( + err.code === "ENOENT" || + err.code === "ENOTDIR" || + err.code === "not-file" || + err.code === "not-found" ); } diff --git a/packages/memory-host-sdk/src/host/internal.test.ts b/packages/memory-host-sdk/src/host/internal.test.ts index bc7567c13a05..610292d42551 100644 --- a/packages/memory-host-sdk/src/host/internal.test.ts +++ b/packages/memory-host-sdk/src/host/internal.test.ts @@ -223,6 +223,69 @@ describe("memory host SDK package internals", () => { ]); }); + it.each([ + { + label: "primary memory file", + target: (workspaceDir: string) => path.join(workspaceDir, "USER.md"), + extraPaths: (_workspaceDir: string) => undefined, + }, + { + label: "workspace memory directory", + target: (workspaceDir: string) => path.join(workspaceDir, "memory"), + extraPaths: (_workspaceDir: string) => undefined, + }, + { + label: "configured extra path", + target: (workspaceDir: string) => path.join(workspaceDir, "extra"), + extraPaths: (workspaceDir: string) => [path.join(workspaceDir, "extra")], + }, + ])("propagates operational scan failures for $label", async ({ target, extraPaths }) => { + const workspaceDir = getTmpDir(); + const failedPath = target(workspaceDir); + const scanError = Object.assign(new Error(`I/O failure: ${failedPath}`), { code: "EIO" }); + const realLstat = fs.lstat; + vi.spyOn(fs, "lstat").mockImplementation( + async (...args: Parameters): ReturnType => { + if (path.resolve(String(args[0])) === failedPath) { + throw scanError; + } + return await realLstat(...args); + }, + ); + + await expect(listMemoryFiles(workspaceDir, extraPaths(workspaceDir))).rejects.toBe(scanError); + }); + + it("propagates operational failures while discovering the canonical memory file", async () => { + const workspaceDir = getTmpDir(); + const scanError = Object.assign(new Error(`I/O failure: ${workspaceDir}`), { code: "EIO" }); + const realReaddir = fs.readdir; + vi.spyOn(fs, "readdir").mockImplementation(async (...args: Parameters) => { + if (path.resolve(String(args[0])) === workspaceDir) { + throw scanError; + } + return await realReaddir(...args); + }); + + await expect(listMemoryFiles(workspaceDir)).rejects.toBe(scanError); + }); + + it("propagates operational failures while traversing a memory directory", async () => { + const workspaceDir = getTmpDir(); + const memoryDir = path.join(workspaceDir, "memory"); + await fs.mkdir(memoryDir); + const scanError = Object.assign(new Error(`I/O failure: ${memoryDir}`), { code: "EIO" }); + const realReaddir = fs.readdir; + vi.spyOn(fs, "readdir").mockImplementation(async (...args: Parameters) => { + if (path.resolve(String(args[0])) === memoryDir) { + throw scanError; + } + return await realReaddir(...args); + }); + + await expect(listMemoryFiles(workspaceDir)).rejects.toBe(scanError); + }); + it("filters extra directories by glob while preserving symlink skips", async () => { const tmpDir = getTmpDir(); const extraDir = path.join(tmpDir, "extra"); diff --git a/packages/memory-host-sdk/src/host/internal.ts b/packages/memory-host-sdk/src/host/internal.ts index 5e1cd9f3a17b..5a7dc5e2069c 100644 --- a/packages/memory-host-sdk/src/host/internal.ts +++ b/packages/memory-host-sdk/src/host/internal.ts @@ -196,6 +196,10 @@ async function collectMemoryFilesFromDir( isAllowedMemoryFilePath(entry.path, multimodal) && (!extraPathEntry || matchesExtraMemoryPathEntry(extraPathEntry, entry.path)), }); + const operationalFailure = scan.failedDirs.find((failure) => !isFileMissingError(failure.error)); + if (operationalFailure) { + throw operationalFailure.error; + } files.push(...scan.entries.map((entry) => entry.path)); } @@ -220,7 +224,11 @@ export async function listMemoryFiles( return; } result.push(absPath); - } catch {} + } catch (error) { + if (!isFileMissingError(error)) { + throw error; + } + } }; const memoryFile = await resolveCanonicalRootMemoryFile(workspaceDir); @@ -234,7 +242,11 @@ export async function listMemoryFiles( // Default memory roots stay Markdown-only; multimodal discovery is an extraPaths opt-in. await collectMemoryFilesFromDir(memoryDir, result, undefined, shouldSkipWorkspaceMemoryPath); } - } catch {} + } catch (error) { + if (!isFileMissingError(error)) { + throw error; + } + } const normalizedExtraPaths = normalizeExtraMemoryPathEntries(workspaceDir, extraPaths); if (normalizedExtraPaths.length > 0) { @@ -265,7 +277,11 @@ export async function listMemoryFiles( ) { result.push(inputPath); } - } catch {} + } catch (error) { + if (!isFileMissingError(error)) { + throw error; + } + } } } if (result.length <= 1) { diff --git a/src/memory/root-memory-files.ts b/src/memory/root-memory-files.ts index 4815333d2f9e..c9de04295b79 100644 --- a/src/memory/root-memory-files.ts +++ b/src/memory/root-memory-files.ts @@ -1,6 +1,7 @@ // Locates root memory files that seed agent context. import fs from "node:fs/promises"; import path from "node:path"; +import { isMissingPathError } from "../infra/errno.js"; /** Canonical root memory file name used by current workspaces. */ export const CANONICAL_ROOT_MEMORY_FILENAME = "MEMORY.md"; @@ -50,7 +51,11 @@ export async function resolveCanonicalRootMemoryFile(workspaceDir: string): Prom return path.join(workspaceDir, entry.name); } } - } catch {} + } catch (error) { + if (!isMissingPathError(error)) { + throw error; + } + } return null; }