diff --git a/README.md b/README.md index c661a66..b35b84a 100644 --- a/README.md +++ b/README.md @@ -170,7 +170,7 @@ See the [full configuration reference](https://redis.github.io/agent-memory-serv Use a deterministic smoke test before building on top of the plugin. The tool calls below are identical regardless of backend. 1. Start OpenClaw with the plugin enabled. -2. Confirm the plugin can reach the backend. The OpenClaw logs should include a registration line naming the resolved backend, e.g. `redis-memory: plugin registered (backend: cloud, server: ..., storeId: ..., namespace: ...)` or `redis-memory: plugin registered (backend: self-hosted, server: ..., namespace: ...)`, followed by `redis-memory: connected to server (...)` once the health check succeeds. +2. Confirm the plugin can reach the backend. The OpenClaw logs should include a registration line naming the resolved backend, e.g. `redis-memory: plugin registered (backend: cloud, server: ..., storeId: ..., namespace: ...)` or `redis-memory: plugin registered (backend: self-hosted, server: ..., namespace: ...)`, followed by `redis-memory: connected to server (...)` once the health check succeeds. With `eagerStartupCheck: false` that second line arrives shortly after startup instead of before it, since the check no longer blocks startup, so give it a moment when running this test. 3. In a chat or tool playground, store a known fact: ```json @@ -312,6 +312,7 @@ When multiple scopes are available, the manual memory tools expose a `scope` par | `recallRecordMaxChars` | integer | `2000` | Maximum characters from one record in automatic recall, from 128 through 10000 | | `recallContextMaxChars` | integer | `16000` | Maximum characters in the complete automatic recall envelope, from 1024 through 32000 | | `erasureSettleMs` | integer | `2000` | Delay between best-effort scope-erasure sweeps, from 0 through 60000 milliseconds | +| `eagerStartupCheck` | boolean | `true` | Await the backend health check (and self-hosted summary-view setup) during service start. Set `false` on latency-sensitive hosts to run it in the background instead: startup returns immediately and failures still appear as log warnings. | | `extractionStrategy` | string | server default | **Self-hosted only.** `discrete`, `summary`, `preferences`, or `custom`; ignored on cloud (logged once at startup) | | `customPrompt` | string | unset | **Self-hosted only.** Custom extraction prompt for `custom` strategy | | `summaryViewName` | string | `agent_user_summary` | **Self-hosted only.** Summary view name for rolling memory summaries | @@ -561,6 +562,7 @@ reconciles the remote checkpoint before retrying safe reads. - If config parsing fails with an error starting `Redis Agent Memory (cloud) is the default backend and requires serverUrl, apiKey, storeId...`, you're missing one or more cloud credentials. Either supply the missing config keys (or their `AGENT_MEMORY_*` env var fallbacks), or set `"provider": "self-hosted"` to use your own server instead. - If you see `server not reachable`, make sure the container is running (self-hosted) or that your cloud endpoint and credentials are correct, and that `serverUrl` matches the exposed port or cloud endpoint. +- If you set `eagerStartupCheck: false` and see no `connected to server` line while the gateway starts, that is expected. The check runs in the background, so that line, or a `server not reachable` warning, appears shortly after startup rather than during it. - If auto-recall seems empty, verify that you are using the same `namespace` and `userId` across sessions. - **"Why don't I see summary context on cloud?"** Summary views are self-hosted only. The cloud backend has no summary-view equivalent, so `summaryViewName`, `summaryTimeWindowDays`, and `summaryGroupBy` are ignored there. - **"Why are there no percentages in my recall results?"** The cloud backend does not return a similarity score per result, so recall output omits the `(NN%)` suffix. Self-hosted always returns scores. diff --git a/openclaw.plugin.json b/openclaw.plugin.json index 0f2c8a7..94bd6bf 100644 --- a/openclaw.plugin.json +++ b/openclaw.plugin.json @@ -65,6 +65,11 @@ "label": "Auto-Recall", "help": "Automatically inject relevant memories into context" }, + "eagerStartupCheck": { + "label": "Eager Startup Check", + "help": "Await the backend health check during service start. Disable on latency-sensitive hosts to run it in the background instead", + "advanced": true + }, "assistantCapture": { "label": "Assistant Capture", "help": "Include assistant turns for memory extraction, or exclude them to minimize retained data", @@ -181,6 +186,7 @@ "timeout": { "type": "integer", "minimum": 100, "maximum": 120000 }, "autoCapture": { "type": "boolean" }, "autoRecall": { "type": "boolean" }, + "eagerStartupCheck": { "type": "boolean" }, "assistantCapture": { "type": "string", "enum": ["exclude", "include"] }, "sensitiveDataRedaction": { "type": "boolean" }, "sessionRetentionSeconds": { "type": "integer", "minimum": 60, "maximum": 31536000 }, diff --git a/src/config.test.ts b/src/config.test.ts index c72c162..710f854 100644 --- a/src/config.test.ts +++ b/src/config.test.ts @@ -9,7 +9,7 @@ */ import { describe, test, expect, beforeEach, afterEach } from "vitest"; -import { parseMemoryConfig } from "./config.js"; +import { ALLOWED_CONFIG_KEYS, parseMemoryConfig } from "./config.js"; const ENV_KEYS = [ "AGENT_MEMORY_ENDPOINT", @@ -112,9 +112,27 @@ describe("parseMemoryConfig — provider resolution", () => { }); }); + test("manifest configSchema stays in lockstep with parser-accepted keys", async () => { + const { readFile } = await import("node:fs/promises"); + const manifest = JSON.parse( + await readFile(new URL("../openclaw.plugin.json", import.meta.url), "utf8"), + ); + const manifestKeys = Object.keys(manifest.configSchema.properties).sort(); + expect(manifestKeys).toEqual([...ALLOWED_CONFIG_KEYS].sort()); + }); + + test("eagerStartupCheck defaults to true and accepts false", () => { + expect(parseMemoryConfig({ serverUrl: "http://localhost:8000" }).eagerStartupCheck).toBe(true); + expect( + parseMemoryConfig({ serverUrl: "http://localhost:8000", eagerStartupCheck: false }) + .eagerStartupCheck, + ).toBe(false); + }); + test.each([ [{ assistantCapture: "sometimes" }, /assistantCapture must be exclude or include/], [{ sensitiveDataRedaction: "yes" }, /sensitiveDataRedaction must be a boolean/], + [{ eagerStartupCheck: "yes" }, /eagerStartupCheck must be a boolean/], [{ sessionRetentionSeconds: 59 }, /sessionRetentionSeconds must be between 60/], [{ recallRecordMaxChars: 127 }, /recallRecordMaxChars must be between 128/], [{ recallContextMaxChars: 1000 }, /recallContextMaxChars must be between 1024/], diff --git a/src/config.ts b/src/config.ts index b7bcd7e..da3f39b 100644 --- a/src/config.ts +++ b/src/config.ts @@ -128,6 +128,14 @@ export type MemoryConfig = { assistantCapture: AssistantCapturePolicy; /** Apply best-effort sensitive-data pattern redaction before capture. */ sensitiveDataRedaction: boolean; + /** + * Await the backend health check (and summary-view ensures) during service + * start (default: true). When false they run in the background so hosts + * that gate readiness on service start are not blocked by the network + * round-trips; failures still surface as log warnings and per-call tool + * errors. + */ + eagerStartupCheck: boolean; /** Self-hosted working-memory TTL. Unsupported by RAM cloud. */ sessionRetentionSeconds?: number; /** Delay between destructive erasure sweeps. */ @@ -469,7 +477,13 @@ function parseAgentMemoryRoute(key: string, value: unknown): AgentMemoryRoute { }; } -const ALLOWED_CONFIG_KEYS = [ +/** + * Every key parseMemoryConfig accepts. Exported so tests can assert the + * plugin-manifest configSchema stays in lockstep: the gateway validates + * config against the manifest before the plugin loads, so a key accepted + * here but missing there fails gateway startup. + */ +export const ALLOWED_CONFIG_KEYS = [ "provider", "serverUrl", "apiKey", @@ -487,6 +501,7 @@ const ALLOWED_CONFIG_KEYS = [ "recallContextMaxChars", "assistantCapture", "sensitiveDataRedaction", + "eagerStartupCheck", "sessionRetentionSeconds", "erasureSettleMs", "extractionStrategy", @@ -530,7 +545,12 @@ const NUMBER_CONFIG_KEYS = [ "sessionRetentionSeconds", "erasureSettleMs", ] as const; -const BOOLEAN_CONFIG_KEYS = ["autoCapture", "autoRecall", "sensitiveDataRedaction"] as const; +const BOOLEAN_CONFIG_KEYS = [ + "autoCapture", + "autoRecall", + "sensitiveDataRedaction", + "eagerStartupCheck", +] as const; function assertConfigFieldTypes(cfg: Record): void { for (const key of STRING_CONFIG_KEYS) { @@ -637,6 +657,7 @@ export function parseMemoryConfig(value: unknown): MemoryConfig { ); const autoCapture = cfg.autoCapture !== false; const autoRecall = cfg.autoRecall !== false; + const eagerStartupCheck = cfg.eagerStartupCheck !== false; const assistantCapture = cfg.assistantCapture === "exclude" ? "exclude" : "include"; const sensitiveDataRedaction = cfg.sensitiveDataRedaction === true; @@ -949,6 +970,7 @@ export function parseMemoryConfig(value: unknown): MemoryConfig { recallContextMaxChars, assistantCapture, sensitiveDataRedaction, + eagerStartupCheck, sessionRetentionSeconds, erasureSettleMs, extractionStrategy, @@ -1031,6 +1053,11 @@ export const memoryConfigSchema = { label: "Auto-Recall", help: "Automatically inject relevant memories into context", }, + eagerStartupCheck: { + label: "Eager Startup Check", + help: "Await the backend health check during service start. Disable on latency-sensitive hosts to run it in the background instead", + advanced: true, + }, assistantCapture: { label: "Assistant Capture", help: "Include assistant turns for memory extraction, or exclude them to minimize retained data", diff --git a/src/index.test.ts b/src/index.test.ts index 03995b8..4273bd2 100644 --- a/src/index.test.ts +++ b/src/index.test.ts @@ -755,6 +755,138 @@ describe("redis-memory plugin — provider integration (Story 05)", () => { expect(logs.all.join("\n")).not.toContain("super-secret-key"); }); + test("service start awaits the health check by default", async () => { + const healthCheck = vi.fn(async () => {}); + const provider = createFakeProvider({ healthCheck }); + const { services } = await registerWithFakeProvider(SELF_HOSTED_CONFIG, provider); + + await services[0].start(); + + expect(healthCheck).toHaveBeenCalledTimes(1); + }); + + test("eagerStartupCheck=false: service start returns while the health check is still pending", async () => { + let releaseHealthCheck!: () => void; + const healthGate = new Promise((resolve) => { + releaseHealthCheck = resolve; + }); + const healthCheck = vi.fn(async () => { + await healthGate; + }); + const provider = createFakeProvider({ healthCheck }); + const { services, logs } = await registerWithFakeProvider( + { ...SELF_HOSTED_CONFIG, eagerStartupCheck: false }, + provider, + ); + + // Resolves immediately even though the health check is gated open; an + // awaited check would hang this call (and time the test out). + await services[0].start(); + expect(healthCheck).toHaveBeenCalledTimes(1); + expect(logs.info.some((l) => l.includes("connected to server"))).toBe(false); + + // The background check still completes and logs once released. + releaseHealthCheck(); + await vi.waitFor(() => + expect(logs.info.some((l) => l.includes("connected to server"))).toBe(true), + ); + }); + + test("eagerStartupCheck=false: stop() waits for the in-flight verification", async () => { + const order: string[] = []; + let releaseHealthCheck!: () => void; + const healthGate = new Promise((resolve) => { + releaseHealthCheck = resolve; + }); + const healthCheck = vi.fn(async () => { + await healthGate; + order.push("health"); + }); + const provider = createFakeProvider({ healthCheck }); + const { services } = await registerWithFakeProvider( + { ...SELF_HOSTED_CONFIG, eagerStartupCheck: false }, + provider, + ); + + await services[0].start(); + const stopping = services[0].stop().then(() => { + order.push("stop"); + }); + + await new Promise((resolve) => setTimeout(resolve, 50)); + expect(order).toEqual([]); + + releaseHealthCheck(); + await stopping; + expect(order).toEqual(["health", "stop"]); + }); + + test("eagerStartupCheck=false: a second start() reuses the in-flight verification", async () => { + // Re-assigning the tracked promise would leave the first verification + // running unreferenced, so stop() would wait only for the second and the + // first could log or ensureView after the plugin reported itself stopped. + const order: string[] = []; + let releaseHealthCheck!: () => void; + const healthGate = new Promise((resolve) => { + releaseHealthCheck = resolve; + }); + const healthCheck = vi.fn(async () => { + await healthGate; + order.push("health"); + }); + const provider = createFakeProvider({ healthCheck }); + const { services } = await registerWithFakeProvider( + { ...SELF_HOSTED_CONFIG, eagerStartupCheck: false }, + provider, + ); + + await services[0].start(); + await services[0].start(); + expect(healthCheck).toHaveBeenCalledTimes(1); + + const stopping = services[0].stop().then(() => { + order.push("stop"); + }); + await new Promise((resolve) => setTimeout(resolve, 50)); + expect(order).toEqual([]); + + releaseHealthCheck(); + await stopping; + expect(order).toEqual(["health", "stop"]); + + // stop() cleared the field, so the next start() verifies again. + await services[0].start(); + expect(healthCheck).toHaveBeenCalledTimes(2); + }); + + test("eagerStartupCheck=false: a throwing logger does not escape as an unhandled rejection", async () => { + const provider = createFakeProvider({ + healthCheck: vi.fn(async () => { + throw new Error("backend down"); + }), + }); + const { api, services } = await registerWithFakeProvider( + { ...SELF_HOSTED_CONFIG, eagerStartupCheck: false }, + provider, + ); + // The warn path inside verifyBackend's own catch block throws. + api.logger.warn = () => { + throw new Error("logger exploded"); + }; + const unhandled: unknown[] = []; + const onUnhandled = (reason: unknown) => unhandled.push(reason); + process.on("unhandledRejection", onUnhandled); + + try { + await expect(services[0].start()).resolves.toBeUndefined(); + await services[0].stop(); + await new Promise((resolve) => setImmediate(resolve)); + expect(unhandled).toEqual([]); + } finally { + process.off("unhandledRejection", onUnhandled); + } + }); + test("self-hosted config logs 'backend: self-hosted' (no storeId)", async () => { const provider = createFakeProvider({ capabilities: { summaryViews: true, extractionStrategy: true, similarityScores: true }, diff --git a/src/index.ts b/src/index.ts index 54da1ed..916f689 100644 --- a/src/index.ts +++ b/src/index.ts @@ -1223,27 +1223,58 @@ const redisMemoryPlugin: PluginDefinition = { // Service // ======================================================================== + // The health check and summary-view ensures are informational network + // round-trips: a failure has never blocked startup (it only warns), and + // every tool call handles provider errors on its own. With + // eagerStartupCheck=false they run in the background so hosts that gate + // readiness on service start (e.g. warm pools) are not delayed by them. + const verifyBackend = async () => { + try { + await provider.healthCheck(); + api.logger.info?.( + `redis-memory: connected to server (${cfg.serverUrl}, namespace: ${JSON.stringify(cfg.namespace ?? "default")})`, + ); + + if (provider.capabilities.summaryViews) { + for (const scope of getConfiguredScopes(cfg)) { + await provider.summaries?.ensureView(scope); + } + } + } catch (err) { + api.logger.warn( + `redis-memory: server not reachable at ${cfg.serverUrl}: ${safeErrorMessage(err, sensitiveValues)}`, + ); + } + }; + + // Tracked so a deferred verification cannot outlive the plugin: an + // in-flight ensureView would otherwise write during teardown, and its log + // line would land after "stopped". + let startupVerification: Promise | undefined; + api.registerService({ id: "redis-memory", start: async () => { - try { - await provider.healthCheck(); - api.logger.info?.( - `redis-memory: connected to server (${cfg.serverUrl}, namespace: ${JSON.stringify(cfg.namespace ?? "default")})`, - ); - - if (provider.capabilities.summaryViews) { - for (const scope of getConfiguredScopes(cfg)) { - await provider.summaries?.ensureView(scope); - } - } - } catch (err) { - api.logger.warn( - `redis-memory: server not reachable at ${cfg.serverUrl}: ${safeErrorMessage(err, sensitiveValues)}`, - ); + // The .catch is belt and braces: verifyBackend already swallows + // provider errors, but a throw from the logger inside its own catch + // block would otherwise escape as an unhandled rejection, which is + // fatal under Node's default --unhandled-rejections=throw. + startupVerification ??= verifyBackend().catch(() => {}); + if (cfg.eagerStartupCheck) { + await startupVerification; + startupVerification = undefined; } }, stop: async () => { + if (startupVerification) { + let timer: ReturnType | undefined; + const timedOut = new Promise((resolve) => { + timer = setTimeout(resolve, 5000); + }); + await Promise.race([startupVerification, timedOut]); + if (timer) clearTimeout(timer); + startupVerification = undefined; + } await captureCoordinator.drain(5000); api.logger.info?.("redis-memory: stopped"); },