fix: harden memory cli manager cleanup
Co-authored-by: Nicholas Spisak <jsnsdirect@gmail.com>
This commit is contained in:
@@ -197,6 +197,34 @@ describe("memory cli", () => {
|
|||||||
expect(log).toHaveBeenCalledWith("Memory index updated.");
|
expect(log).toHaveBeenCalledWith("Memory index updated.");
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("logs close failures without failing the command", async () => {
|
||||||
|
const { registerMemoryCli } = await import("./memory-cli.js");
|
||||||
|
const { defaultRuntime } = await import("../runtime.js");
|
||||||
|
const close = vi.fn(async () => {
|
||||||
|
throw new Error("close boom");
|
||||||
|
});
|
||||||
|
const sync = vi.fn(async () => {});
|
||||||
|
getMemorySearchManager.mockResolvedValueOnce({
|
||||||
|
manager: {
|
||||||
|
sync,
|
||||||
|
close,
|
||||||
|
},
|
||||||
|
});
|
||||||
|
|
||||||
|
const error = vi.spyOn(defaultRuntime, "error").mockImplementation(() => {});
|
||||||
|
const program = new Command();
|
||||||
|
program.name("test");
|
||||||
|
registerMemoryCli(program);
|
||||||
|
await program.parseAsync(["memory", "index"], { from: "user" });
|
||||||
|
|
||||||
|
expect(sync).toHaveBeenCalledWith({ reason: "cli", force: false });
|
||||||
|
expect(close).toHaveBeenCalled();
|
||||||
|
expect(error).toHaveBeenCalledWith(
|
||||||
|
expect.stringContaining("Memory manager close failed: close boom"),
|
||||||
|
);
|
||||||
|
expect(process.exitCode).toBeUndefined();
|
||||||
|
});
|
||||||
|
|
||||||
it("closes manager after search error", async () => {
|
it("closes manager after search error", async () => {
|
||||||
const { registerMemoryCli } = await import("./memory-cli.js");
|
const { registerMemoryCli } = await import("./memory-cli.js");
|
||||||
const { defaultRuntime } = await import("../runtime.js");
|
const { defaultRuntime } = await import("../runtime.js");
|
||||||
|
|||||||
@@ -3,7 +3,7 @@ import type { Command } from "commander";
|
|||||||
import { resolveDefaultAgentId } from "../agents/agent-scope.js";
|
import { resolveDefaultAgentId } from "../agents/agent-scope.js";
|
||||||
import { loadConfig } from "../config/config.js";
|
import { loadConfig } from "../config/config.js";
|
||||||
import { withProgress, withProgressTotals } from "./progress.js";
|
import { withProgress, withProgressTotals } from "./progress.js";
|
||||||
import { getMemorySearchManager } from "../memory/index.js";
|
import { getMemorySearchManager, type MemorySearchManagerResult } from "../memory/index.js";
|
||||||
import { defaultRuntime } from "../runtime.js";
|
import { defaultRuntime } from "../runtime.js";
|
||||||
import { formatDocsLink } from "../terminal/links.js";
|
import { formatDocsLink } from "../terminal/links.js";
|
||||||
import { colorize, isRich, theme } from "../terminal/theme.js";
|
import { colorize, isRich, theme } from "../terminal/theme.js";
|
||||||
@@ -15,12 +15,42 @@ type MemoryCommandOptions = {
|
|||||||
index?: boolean;
|
index?: boolean;
|
||||||
};
|
};
|
||||||
|
|
||||||
|
type MemoryManager = NonNullable<MemorySearchManagerResult["manager"]>;
|
||||||
|
|
||||||
function resolveAgent(cfg: ReturnType<typeof loadConfig>, agent?: string) {
|
function resolveAgent(cfg: ReturnType<typeof loadConfig>, agent?: string) {
|
||||||
const trimmed = agent?.trim();
|
const trimmed = agent?.trim();
|
||||||
if (trimmed) return trimmed;
|
if (trimmed) return trimmed;
|
||||||
return resolveDefaultAgentId(cfg);
|
return resolveDefaultAgentId(cfg);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
function formatErrorMessage(err: unknown): string {
|
||||||
|
return err instanceof Error ? err.message : String(err);
|
||||||
|
}
|
||||||
|
|
||||||
|
async function closeManager(manager: MemoryManager): Promise<void> {
|
||||||
|
try {
|
||||||
|
await manager.close();
|
||||||
|
} catch (err) {
|
||||||
|
defaultRuntime.error(`Memory manager close failed: ${formatErrorMessage(err)}`);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
async function withMemoryManager(
|
||||||
|
params: { cfg: ReturnType<typeof loadConfig>; agentId: string },
|
||||||
|
run: (manager: MemoryManager) => Promise<void>,
|
||||||
|
): Promise<void> {
|
||||||
|
const { manager, error } = await getMemorySearchManager(params);
|
||||||
|
if (!manager) {
|
||||||
|
defaultRuntime.log(error ?? "Memory search disabled.");
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
try {
|
||||||
|
await run(manager);
|
||||||
|
} finally {
|
||||||
|
await closeManager(manager);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
export function registerMemoryCli(program: Command) {
|
export function registerMemoryCli(program: Command) {
|
||||||
const memory = program
|
const memory = program
|
||||||
.command("memory")
|
.command("memory")
|
||||||
@@ -41,12 +71,7 @@ export function registerMemoryCli(program: Command) {
|
|||||||
.action(async (opts: MemoryCommandOptions) => {
|
.action(async (opts: MemoryCommandOptions) => {
|
||||||
const cfg = loadConfig();
|
const cfg = loadConfig();
|
||||||
const agentId = resolveAgent(cfg, opts.agent);
|
const agentId = resolveAgent(cfg, opts.agent);
|
||||||
const { manager, error } = await getMemorySearchManager({ cfg, agentId });
|
await withMemoryManager({ cfg, agentId }, async (manager) => {
|
||||||
if (!manager) {
|
|
||||||
defaultRuntime.log(error ?? "Memory search disabled.");
|
|
||||||
return;
|
|
||||||
}
|
|
||||||
try {
|
|
||||||
const deep = Boolean(opts.deep || opts.index);
|
const deep = Boolean(opts.deep || opts.index);
|
||||||
let embeddingProbe: Awaited<ReturnType<typeof manager.probeEmbeddingAvailability>> | undefined;
|
let embeddingProbe: Awaited<ReturnType<typeof manager.probeEmbeddingAvailability>> | undefined;
|
||||||
let indexError: string | undefined;
|
let indexError: string | undefined;
|
||||||
@@ -76,7 +101,7 @@ export function registerMemoryCli(program: Command) {
|
|||||||
},
|
},
|
||||||
});
|
});
|
||||||
} catch (err) {
|
} catch (err) {
|
||||||
indexError = err instanceof Error ? err.message : String(err);
|
indexError = formatErrorMessage(err);
|
||||||
defaultRuntime.error(`Memory index failed: ${indexError}`);
|
defaultRuntime.error(`Memory index failed: ${indexError}`);
|
||||||
process.exitCode = 1;
|
process.exitCode = 1;
|
||||||
}
|
}
|
||||||
@@ -175,9 +200,7 @@ export function registerMemoryCli(program: Command) {
|
|||||||
lines.push(`${label("Index error")} ${warn(indexError)}`);
|
lines.push(`${label("Index error")} ${warn(indexError)}`);
|
||||||
}
|
}
|
||||||
defaultRuntime.log(lines.join("\n"));
|
defaultRuntime.log(lines.join("\n"));
|
||||||
} finally {
|
});
|
||||||
await manager.close();
|
|
||||||
}
|
|
||||||
});
|
});
|
||||||
|
|
||||||
memory
|
memory
|
||||||
@@ -188,22 +211,17 @@ export function registerMemoryCli(program: Command) {
|
|||||||
.action(async (opts: MemoryCommandOptions & { force?: boolean }) => {
|
.action(async (opts: MemoryCommandOptions & { force?: boolean }) => {
|
||||||
const cfg = loadConfig();
|
const cfg = loadConfig();
|
||||||
const agentId = resolveAgent(cfg, opts.agent);
|
const agentId = resolveAgent(cfg, opts.agent);
|
||||||
const { manager, error } = await getMemorySearchManager({ cfg, agentId });
|
await withMemoryManager({ cfg, agentId }, async (manager) => {
|
||||||
if (!manager) {
|
|
||||||
defaultRuntime.log(error ?? "Memory search disabled.");
|
|
||||||
return;
|
|
||||||
}
|
|
||||||
try {
|
try {
|
||||||
await manager.sync({ reason: "cli", force: opts.force });
|
await manager.sync({ reason: "cli", force: opts.force });
|
||||||
defaultRuntime.log("Memory index updated.");
|
defaultRuntime.log("Memory index updated.");
|
||||||
} catch (err) {
|
} catch (err) {
|
||||||
const message = err instanceof Error ? err.message : String(err);
|
const message = formatErrorMessage(err);
|
||||||
defaultRuntime.error(`Memory index failed: ${message}`);
|
defaultRuntime.error(`Memory index failed: ${message}`);
|
||||||
process.exitCode = 1;
|
process.exitCode = 1;
|
||||||
} finally {
|
|
||||||
await manager.close();
|
|
||||||
}
|
}
|
||||||
});
|
});
|
||||||
|
});
|
||||||
|
|
||||||
memory
|
memory
|
||||||
.command("search")
|
.command("search")
|
||||||
@@ -223,14 +241,7 @@ export function registerMemoryCli(program: Command) {
|
|||||||
) => {
|
) => {
|
||||||
const cfg = loadConfig();
|
const cfg = loadConfig();
|
||||||
const agentId = resolveAgent(cfg, opts.agent);
|
const agentId = resolveAgent(cfg, opts.agent);
|
||||||
const { manager, error } = await getMemorySearchManager({
|
await withMemoryManager({ cfg, agentId }, async (manager) => {
|
||||||
cfg,
|
|
||||||
agentId,
|
|
||||||
});
|
|
||||||
if (!manager) {
|
|
||||||
defaultRuntime.log(error ?? "Memory search disabled.");
|
|
||||||
return;
|
|
||||||
}
|
|
||||||
let results: Awaited<ReturnType<typeof manager.search>>;
|
let results: Awaited<ReturnType<typeof manager.search>>;
|
||||||
try {
|
try {
|
||||||
results = await manager.search(query, {
|
results = await manager.search(query, {
|
||||||
@@ -238,12 +249,10 @@ export function registerMemoryCli(program: Command) {
|
|||||||
minScore: opts.minScore,
|
minScore: opts.minScore,
|
||||||
});
|
});
|
||||||
} catch (err) {
|
} catch (err) {
|
||||||
const message = err instanceof Error ? err.message : String(err);
|
const message = formatErrorMessage(err);
|
||||||
defaultRuntime.error(`Memory search failed: ${message}`);
|
defaultRuntime.error(`Memory search failed: ${message}`);
|
||||||
process.exitCode = 1;
|
process.exitCode = 1;
|
||||||
return;
|
return;
|
||||||
} finally {
|
|
||||||
await manager.close();
|
|
||||||
}
|
}
|
||||||
if (opts.json) {
|
if (opts.json) {
|
||||||
defaultRuntime.log(JSON.stringify({ results }, null, 2));
|
defaultRuntime.log(JSON.stringify({ results }, null, 2));
|
||||||
@@ -267,6 +276,7 @@ export function registerMemoryCli(program: Command) {
|
|||||||
lines.push("");
|
lines.push("");
|
||||||
}
|
}
|
||||||
defaultRuntime.log(lines.join("\n").trim());
|
defaultRuntime.log(lines.join("\n").trim());
|
||||||
|
});
|
||||||
},
|
},
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user