mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-25 11:55:47 -06:00
fix: surface swallowed failures on action paths (#125319)
* fix: surface swallowed failures on action paths * fix(memory): propagate directory traversal failures
This commit is contained in:
committed by
GitHub
parent
3cc55589e3
commit
e169520fef
@@ -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
|
||||
|
||||
@@ -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<typeof fs.readdir>) => {
|
||||
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);
|
||||
|
||||
@@ -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 });
|
||||
|
||||
@@ -211,6 +211,18 @@ export function writeError(res: ServerResponse, statusCode: number, error: unkno
|
||||
});
|
||||
}
|
||||
|
||||
export function dispatchQaHttpRequest(res: ServerResponse, task: () => Promise<void>): 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");
|
||||
}
|
||||
})();
|
||||
});
|
||||
});
|
||||
}
|
||||
|
||||
|
||||
@@ -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 () => {
|
||||
|
||||
@@ -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 = () => {
|
||||
|
||||
@@ -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<NodeJS.ErrnoException>).code === "ENOENT" ||
|
||||
(err as Partial<NodeJS.ErrnoException>).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"
|
||||
);
|
||||
}
|
||||
|
||||
@@ -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<typeof fs.lstat>): ReturnType<typeof fs.lstat> => {
|
||||
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<typeof fs.readdir>) => {
|
||||
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<typeof fs.readdir>) => {
|
||||
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");
|
||||
|
||||
@@ -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) {
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user