diff --git a/plugin/pi/index.ts b/plugin/pi/index.ts index 33a5523d2..5dd8fb4f8 100644 --- a/plugin/pi/index.ts +++ b/plugin/pi/index.ts @@ -178,10 +178,9 @@ function isTimeoutError(error: unknown): boolean { return error instanceof Error && (error.name === "TimeoutError" || error.name === "AbortError"); } -// engramFetch resolves to null on failure and ~20 call sites depend on that fallthrough — -// ensureSession in particular must not abort a mem_save just because session creation blipped. -// So the timeout detail travels out-of-band instead of changing what any caller receives, -// letting executeMemoryTool tell the truth about an ambiguous write without blast radius. +// Timed-out requests resolve to null so existing best-effort call sites can continue, while +// ordinary transport failures throw so the tool layer can report a real provider outage. +// The timeout detail travels out-of-band so callers can distinguish an ambiguous write. let lastFetchTimeoutMethod: string | undefined; function takeLastFetchTimeoutMethod(): string | undefined { @@ -190,60 +189,95 @@ function takeLastFetchTimeoutMethod(): string | undefined { return method; } -async function engramFetch(path: string, opts: FetchOptions = {}): Promise { - const method = opts.method ?? "GET"; - // This call's outcome supersedes any earlier one. A tool call can issue several fetches - // (mem_save creates the session, then writes the observation); without this reset a timeout - // on the first leg would mislabel an unrelated failure on the second as "may already have - // been applied", telling the agent not to retry a write that never left the machine. - lastFetchTimeoutMethod = undefined; - let res: Response | undefined; - let timedOut = false; - for (let attempt = 0; attempt < ENGRAM_FETCH_MAX_ATTEMPTS; attempt += 1) { - try { - res = await fetch(`${ENGRAM_URL}${redactUrlPath(path)}`, { - method, - headers: opts.body ? { "Content-Type": "application/json" } : undefined, - body: opts.body ? JSON.stringify(redactValue(opts.body)) : undefined, - signal: AbortSignal.timeout(ENGRAM_FETCH_TIMEOUT_MS), - }); - break; - } catch (error) { - // A timeout means the request may already have reached the server, so re-sending it - // could duplicate a non-idempotent write (mem_save and friends carry no idempotency - // key). Only pre-send connection failures — the macOS wake-settle case this retry - // exists for — are safe to repeat, and a hung server will not recover by retrying. - if (isTimeoutError(error)) { - timedOut = true; - break; - } - if (attempt < ENGRAM_FETCH_MAX_ATTEMPTS - 1) await wait(ENGRAM_FETCH_BACKOFF_BASE_MS * 2 ** attempt); - } +// Outcome of the transport layer: a completed HTTP exchange, a timeout/abort rejection, +// or an ordinary connection failure that survived every retry. +type EngramRequestOutcome = + | { kind: "responded"; response: Response } + | { kind: "timed-out" } + | { kind: "unreachable" }; + +// Prefers the server's own error message over a generic status-line summary. +function httpErrorMessage(data: unknown, status: number): string { + if (data && typeof data === "object" && typeof (data as { error?: unknown }).error === "string") { + return (data as { error: string }).error; } + return `Engram request failed with HTTP ${status}`; +} - // A timeout is NOT the same failure as an unreachable server, and reporting both as "could - // not reach" invites the caller to retry a write whose outcome is genuinely unknown. Record - // which it was so the tool layer can say what we do and do not know. - if (timedOut) lastFetchTimeoutMethod = method; - if (!res) return null; +// Same method, headers, and redacted body on every attempt, so a retry re-sends the identical +// request. The timeout signal is deliberately not built here: it is attached per attempt so +// each retry keeps the full timeout budget instead of inheriting already-spent milliseconds. +function engramRequestInit(method: string, opts: FetchOptions): RequestInit { + return { + method, + headers: opts.body ? { "Content-Type": "application/json" } : undefined, + body: opts.body ? JSON.stringify(redactValue(opts.body)) : undefined, + }; +} +// One HTTP attempt. A timeout or caller abort is reported as "timed-out" and never retried: +// the request may already have reached the server, and re-sending it could duplicate a +// non-idempotent write (mem_save and friends carry no idempotency key). Any other rejection +// is a pre-send connection failure — the macOS wake-settle case the retry loop exists for. +async function attemptEngramFetch(url: string, init: RequestInit): Promise { + try { + const response = await fetch(url, { ...init, signal: AbortSignal.timeout(ENGRAM_FETCH_TIMEOUT_MS) }); + return { kind: "responded", response }; + } catch (error) { + return isTimeoutError(error) ? { kind: "timed-out" } : { kind: "unreachable" }; + } +} + +// Sends the request with bounded retries and exponential backoff. Only connection failures +// are retried: they never reached the server, and a hung server will not recover by being +// re-asked — the timeout case is terminal precisely because its outcome is unknown. +async function retryEngramFetch(url: string, init: RequestInit): Promise { + for (let attempt = 0; attempt < ENGRAM_FETCH_MAX_ATTEMPTS; attempt += 1) { + const outcome = await attemptEngramFetch(url, init); + if (outcome.kind !== "unreachable") return outcome; + if (attempt < ENGRAM_FETCH_MAX_ATTEMPTS - 1) await wait(ENGRAM_FETCH_BACKOFF_BASE_MS * 2 ** attempt); + } + return { kind: "unreachable" }; +} + +// Reads the response body as JSON, treating an unparseable or empty body as null (a valid +// result — an empty /search response is HTTP 200 with a null body) and preserving the HTTP +// error payload, message, and status in EngramHttpError. +async function decodeEngramResponse(res: Response): Promise { let data: unknown = null; try { data = await res.json(); } catch { data = null; } - if (!res.ok) { - const message = data && typeof data === "object" && "error" in data && typeof data.error === "string" - ? data.error - : `Engram request failed with HTTP ${res.status}`; - throw new EngramHttpError(message, res.status, data); + throw new EngramHttpError(httpErrorMessage(data, res.status), res.status, data); } - return data as TResponse; } +async function engramFetch(path: string, opts: FetchOptions = {}): Promise { + const method = opts.method ?? "GET"; + // This call's outcome supersedes any earlier one. A tool call can issue several fetches + // (mem_save creates the session, then writes the observation); without this reset a timeout + // on the first leg would mislabel an unrelated failure on the second as "may already have + // been applied", telling the agent not to retry a write that never left the machine. + lastFetchTimeoutMethod = undefined; + const outcome = await retryEngramFetch(`${ENGRAM_URL}${redactUrlPath(path)}`, engramRequestInit(method, opts)); + // A timeout is not the same as an unreachable server: the request may already have + // reached the server, so preserve the existing null fallthrough and record its method. + if (outcome.kind === "timed-out") { + lastFetchTimeoutMethod = method; + return null; + } + // A genuine transport failure (all retries exhausted) MUST surface as a thrown error so + // executeMemoryTool can report it instead of conflating it with a valid null response body. + if (outcome.kind === "unreachable") { + throw new Error(`gentle-engram could not reach the Engram HTTP server at ${ENGRAM_URL}. The Pi-native mem_* tools are registered, but the native memory provider is not currently responding. Run mem_doctor or restart Engram.`); + } + return decodeEngramResponse(outcome.response); +} + async function bestEffortEngramFetch(path: string, opts: FetchOptions = {}): Promise { try { return await engramFetch(path, opts); @@ -252,33 +286,52 @@ async function bestEffortEngramFetch(path: string, opts: Fe } } -function detectLocalConfigProject(cwd: string): CurrentProjectResponse | undefined { - let current = resolve(cwd || "."); - while (true) { - const configPath = `${current}/.engram/config.json`; - if (existsSync(configPath)) { - try { - const parsed = JSON.parse(readFileSync(configPath, "utf8")) as { project_name?: unknown }; - const projectName = typeof parsed.project_name === "string" ? parsed.project_name.trim() : ""; - if (projectName) { - return { - project: projectName, - project_source: "config", - project_path: current, - cwd, - warning: `Engram server at ${ENGRAM_URL} does not support /project/current; using ${configPath}. Upgrade or restart Engram for canonical project detection.`, - }; - } - return { - cwd, - error_hint: `${configPath} exists but project_name is missing or empty. Fix the config or pass project explicitly.`, - }; - } catch (error) { - const message = error instanceof Error ? error.message : String(error); - return { cwd, error_hint: `Could not read ${configPath}: ${message}` }; - } +function errorText(error: unknown): string { + return error instanceof Error ? error.message : String(error); +} + +// An empty cwd still means "here"; resolving once gives the upward walk a stable anchor. +function localConfigSearchRoot(cwd: string): string { + return resolve(cwd || "."); +} + +function readLocalConfigProjectName(configPath: string): string { + const parsed = JSON.parse(readFileSync(configPath, "utf8")) as { project_name?: unknown }; + return typeof parsed.project_name === "string" ? parsed.project_name.trim() : ""; +} + +function localConfigProjectResponse(projectName: string, configPath: string, dir: string, cwd: string): CurrentProjectResponse { + return { + project: projectName, + project_source: "config", + project_path: dir, + cwd, + warning: `Engram server at ${ENGRAM_URL} does not support /project/current; using ${configPath}. Upgrade or restart Engram for canonical project detection.`, + }; +} + +// Inspects one directory. Returns undefined only when no config file exists there, which is the +// sole signal to keep walking upward; every other outcome (valid project, invalid project_name, +// unreadable or malformed file) is reported for that directory and stops the walk. +function readLocalConfigProject(current: string, cwd: string): CurrentProjectResponse | undefined { + const configPath = `${current}/.engram/config.json`; + if (!existsSync(configPath)) return undefined; + try { + const projectName = readLocalConfigProjectName(configPath); + if (!projectName) { + return { cwd, error_hint: `${configPath} exists but project_name is missing or empty. Fix the config or pass project explicitly.` }; } + return localConfigProjectResponse(projectName, configPath, current, cwd); + } catch (error) { + return { cwd, error_hint: `Could not read ${configPath}: ${errorText(error)}` }; + } +} +function detectLocalConfigProject(cwd: string): CurrentProjectResponse | undefined { + let current = localConfigSearchRoot(cwd); + while (true) { + const response = readLocalConfigProject(current, cwd); + if (response) return response; const parent = dirname(current); if (parent === current) return undefined; current = parent; @@ -369,6 +422,12 @@ function errorStatusLabel(message: string): string { return "error"; } +function engramStatusLabel(): string { + // Reflects the reachability captured by initOnce. `undefined` (before first init + // completes) falls back to "ready" to preserve prior behavior; `false` is a real outage. + return engramReachable === false ? "offline" : "ready"; +} + function stripPrivateTags(str: string): string { return redactPrivateTags(str).trim(); } @@ -413,6 +472,7 @@ let directory = ""; let pendingRecoveryNotice: string | undefined; let projectResolutionError: string | undefined; let projectDetectionPending = false; +let engramReachable: boolean | undefined; const knownSessions = new Set(); const toolCounts = new Map(); @@ -482,10 +542,15 @@ async function initOnce(cwd: string): Promise { project = fallbackProjectName(cwd); const running = await isEngramRunning(); + let spawnedServer = false; if (!running && CONFIGURED_ENGRAM_URL === undefined) { await spawnDetached(ENGRAM_BIN, ["serve"]); await wait(500); + spawnedServer = true; } + // Capture real reachability so the status indicator reflects it instead of a + // hardcoded "ready". Re-probe only when we spawned the server; otherwise reuse `running`. + engramReachable = spawnedServer ? await isEngramRunning() : running; applyDetectedProject(await detectServerProject(cwd)); @@ -507,6 +572,18 @@ function getSessionId(ctx: SessionContext): string | undefined { return ctx.sessionManager.getSessionId(); } +// Shared session-event preamble: run one-time init, refresh project detection, and repaint +// the engram status indicator. ctx.ui is present at runtime even though SessionContext's +// type omits it (confirmed by probe); setStatus stays optional and guarded so a torn-down +// UI can never crash a handler. +async function initAndRepaintStatus(ctx: SessionContext): Promise { + await initOnce(ctx.cwd); + await refreshProjectDetection(ctx.cwd); + try { + (ctx as MemoryToolContext).ui?.setStatus?.("engram", `🧠 ${project} · ${engramStatusLabel()}`); + } catch {} +} + const optionalString = (description: string) => Type.Optional(Type.String({ description })); const optionalNumber = (description: string) => Type.Optional(Type.Number({ description })); const optionalBoolean = (description: string) => Type.Optional(Type.Boolean({ description })); @@ -648,159 +725,232 @@ function slugifyTopicKey(params: Record): string { return slug || "memory"; } +// Per-call resolution shared by the memory-tool handlers below: the raw params, the Pi +// session context, and the project/session values callMemoryTool derives once per call. +interface MemoryToolRequest { + params: Record; + ctx: SessionContext; + sessionId: string | undefined; + requestedProject: string | undefined; + activeProject: string; + activeSessionId: string; +} + +type MemoryToolHandler = (request: MemoryToolRequest) => unknown; + +function handleMemSearch({ params }: MemoryToolRequest): unknown { + return engramFetch(`/search${queryString({ + q: params.query, + type: params.type, + project: params.all_projects ? undefined : params.project, + scope: params.scope, + limit: params.limit, + match_mode: params.match_mode, + all_projects: params.all_projects, + })}`); +} + +function handleMemContext({ params }: MemoryToolRequest): unknown { + if (!params.project) requireResolvedProject(); + return engramFetch(`/context${queryString({ project: params.project || project, scope: params.scope })}`); +} + +function handleMemStats(): unknown { + return engramFetch("/stats"); +} + +function handleMemTimeline({ params }: MemoryToolRequest): unknown { + return engramFetch(`/timeline${queryString({ observation_id: params.observation_id, before: params.before, after: params.after, project: params.project })}`); +} + +function handleMemGetObservation({ params }: MemoryToolRequest): unknown { + return engramFetch(`/observations/${encodeURIComponent(String(params.id))}`); +} + +async function handleMemSave({ params, activeSessionId, activeProject, requestedProject }: MemoryToolRequest): Promise { + if (!requestedProject) requireResolvedProject(); + await ensureSession(activeSessionId, activeProject); + return engramFetch("/observations", { + method: "POST", + body: { + session_id: activeSessionId, + title: params.title, + content: params.content, + type: params.type || "manual", + project: activeProject, + scope: params.scope || "project", + topic_key: params.topic_key, + }, + }); +} + +function handleMemUpdate({ params }: MemoryToolRequest): unknown { + return engramFetch(`/observations/${encodeURIComponent(String(params.id))}`, { + method: "PATCH", + body: { + title: params.title, + content: params.content, + type: params.type, + scope: params.scope, + topic_key: params.topic_key, + }, + }); +} + +function handleMemDelete({ params }: MemoryToolRequest): unknown { + return engramFetch(`/observations/${encodeURIComponent(String(params.id))}${queryString({ hard: params.hard_delete })}`, { method: "DELETE" }); +} + +function handleMemSuggestTopicKey({ params }: MemoryToolRequest): unknown { + return { topic_key: slugifyTopicKey(params) }; +} + +async function handleMemSavePrompt({ params, activeSessionId, activeProject, requestedProject }: MemoryToolRequest): Promise { + if (!requestedProject) requireResolvedProject(); + await ensureSession(activeSessionId, activeProject); + return engramFetch("/prompts", { + method: "POST", + body: { session_id: activeSessionId, content: params.content, project: activeProject }, + }); +} + +async function handleMemSessionSummary({ params, activeSessionId, activeProject, requestedProject }: MemoryToolRequest): Promise { + if (!requestedProject) requireResolvedProject(); + await ensureSession(activeSessionId, activeProject); + return engramFetch("/observations", { + method: "POST", + body: { + session_id: activeSessionId, + type: "session_summary", + title: "Session summary", + content: params.content, + project: activeProject, + scope: "project", + }, + }); +} + +async function handleMemSessionStart({ params, ctx }: MemoryToolRequest): Promise { + requireResolvedProject(); + return engramFetch("/sessions", { + method: "POST", + body: { id: params.id, project, directory: params.directory || directory || ctx.cwd }, + }); +} + +function handleMemSessionEnd({ params }: MemoryToolRequest): unknown { + return engramFetch(`/sessions/${encodeURIComponent(String(params.id))}/end`, { + method: "POST", + body: { summary: params.summary || "" }, + }); +} + +async function handleMemCurrentProject({ params, ctx }: MemoryToolRequest): Promise { + const cwd = String(params.cwd || ctx.cwd); + try { + return await engramFetch(`/project/current${queryString({ cwd })}`); + } catch (error) { + if (error instanceof EngramHttpError && error.status === 404) { + return detectLocalConfigProject(cwd) || projectCurrentUnsupportedError(cwd); + } + throw error; + } +} + +function handleMemDoctor({ params, ctx }: MemoryToolRequest): unknown { + return engramFetch(`/doctor${queryString({ project: params.project, check: params.check, cwd: params.project ? undefined : ctx.cwd })}`); +} + +async function handleMemCapturePassive({ params, activeSessionId }: MemoryToolRequest): Promise { + requireResolvedProject(); + await ensureSession(activeSessionId); + return engramFetch("/observations/passive", { + method: "POST", + body: { + session_id: activeSessionId, + content: params.content, + project, + source: params.source || "pi-tool", + }, + }); +} + +function handleMemReview({ params }: MemoryToolRequest): unknown { + const action = String(params.action || "").trim(); + if (action === "list") { + return engramFetch(`/review${queryString({ project: params.project, limit: params.limit })}`); + } + if (action === "mark_reviewed") { + return engramFetch("/review/mark_reviewed", { + method: "POST", + body: { observation_id: params.observation_id || params.id }, + }); + } + throw new Error("action must be one of: list, mark_reviewed"); +} + +function handleMemJudge({ params, sessionId }: MemoryToolRequest): unknown { + return engramFetch("/conflicts/judge", { + method: "POST", + body: { + judgment_id: params.judgment_id, + relation: params.relation, + reason: params.reason, + evidence: params.evidence, + confidence: params.confidence, + session_id: params.session_id || sessionId, + }, + }); +} + +function handleMemCompare({ params }: MemoryToolRequest): unknown { + return engramFetch("/conflicts/compare", { + method: "POST", + body: { + memory_id_a: params.memory_id_a, + memory_id_b: params.memory_id_b, + relation: params.relation, + confidence: params.confidence, + reasoning: params.reasoning, + model: params.model, + }, + }); +} + +// Typed dispatch: one named handler per memory tool, mirroring ENGRAM_TOOLS. +// Map#get returns undefined for anything absent, so unknown tool names reach the +// unsupported-tool error exactly like the previous switch default. +const MEMORY_TOOL_HANDLERS = new Map([ + ["mem_search", handleMemSearch], + ["mem_context", handleMemContext], + ["mem_stats", handleMemStats], + ["mem_timeline", handleMemTimeline], + ["mem_get_observation", handleMemGetObservation], + ["mem_save", handleMemSave], + ["mem_update", handleMemUpdate], + ["mem_delete", handleMemDelete], + ["mem_suggest_topic_key", handleMemSuggestTopicKey], + ["mem_save_prompt", handleMemSavePrompt], + ["mem_session_summary", handleMemSessionSummary], + ["mem_session_start", handleMemSessionStart], + ["mem_session_end", handleMemSessionEnd], + ["mem_current_project", handleMemCurrentProject], + ["mem_doctor", handleMemDoctor], + ["mem_capture_passive", handleMemCapturePassive], + ["mem_review", handleMemReview], + ["mem_judge", handleMemJudge], + ["mem_compare", handleMemCompare], +]); + async function callMemoryTool(toolName: string, params: Record, ctx: SessionContext): Promise { const sessionId = getSessionId(ctx); const requestedProject = typeof params.project === "string" && params.project ? params.project : undefined; const activeProject = requestedProject || project; const activeSessionId = String(params.session_id || (requestedProject ? `manual-save-${requestedProject}` : sessionId) || `manual-save-${project}`); - switch (toolName) { - case "mem_search": - return engramFetch(`/search${queryString({ - q: params.query, - type: params.type, - project: params.all_projects ? undefined : params.project, - scope: params.scope, - limit: params.limit, - match_mode: params.match_mode, - all_projects: params.all_projects, - })}`); - case "mem_context": - if (!params.project) requireResolvedProject(); - return engramFetch(`/context${queryString({ project: params.project || project, scope: params.scope })}`); - case "mem_stats": - return engramFetch("/stats"); - case "mem_timeline": - return engramFetch(`/timeline${queryString({ observation_id: params.observation_id, before: params.before, after: params.after, project: params.project })}`); - case "mem_get_observation": - return engramFetch(`/observations/${encodeURIComponent(String(params.id))}`); - case "mem_save": - if (!requestedProject) requireResolvedProject(); - await ensureSession(activeSessionId, activeProject); - return engramFetch("/observations", { - method: "POST", - body: { - session_id: activeSessionId, - title: params.title, - content: params.content, - type: params.type || "manual", - project: activeProject, - scope: params.scope || "project", - topic_key: params.topic_key, - }, - }); - case "mem_update": - return engramFetch(`/observations/${encodeURIComponent(String(params.id))}`, { - method: "PATCH", - body: { - title: params.title, - content: params.content, - type: params.type, - scope: params.scope, - topic_key: params.topic_key, - }, - }); - case "mem_delete": - return engramFetch(`/observations/${encodeURIComponent(String(params.id))}${queryString({ hard: params.hard_delete })}`, { method: "DELETE" }); - case "mem_suggest_topic_key": - return { topic_key: slugifyTopicKey(params) }; - case "mem_save_prompt": - if (!requestedProject) requireResolvedProject(); - await ensureSession(activeSessionId, activeProject); - return engramFetch("/prompts", { - method: "POST", - body: { session_id: activeSessionId, content: params.content, project: activeProject }, - }); - case "mem_session_summary": - if (!requestedProject) requireResolvedProject(); - await ensureSession(activeSessionId, activeProject); - return engramFetch("/observations", { - method: "POST", - body: { - session_id: activeSessionId, - type: "session_summary", - title: "Session summary", - content: params.content, - project: activeProject, - scope: "project", - }, - }); - case "mem_session_start": - requireResolvedProject(); - return engramFetch("/sessions", { - method: "POST", - body: { id: params.id, project, directory: params.directory || directory || ctx.cwd }, - }); - case "mem_session_end": - return engramFetch(`/sessions/${encodeURIComponent(String(params.id))}/end`, { - method: "POST", - body: { summary: params.summary || "" }, - }); - case "mem_current_project": { - const cwd = String(params.cwd || ctx.cwd); - try { - return await engramFetch(`/project/current${queryString({ cwd })}`); - } catch (error) { - if (error instanceof EngramHttpError && error.status === 404) { - return detectLocalConfigProject(cwd) || projectCurrentUnsupportedError(cwd); - } - throw error; - } - } - case "mem_doctor": - return engramFetch(`/doctor${queryString({ project: params.project, check: params.check, cwd: params.project ? undefined : ctx.cwd })}`); - case "mem_capture_passive": - requireResolvedProject(); - await ensureSession(activeSessionId); - return engramFetch("/observations/passive", { - method: "POST", - body: { - session_id: activeSessionId, - content: params.content, - project, - source: params.source || "pi-tool", - }, - }); - case "mem_review": { - const action = String(params.action || "").trim(); - if (action === "list") { - return engramFetch(`/review${queryString({ project: params.project, limit: params.limit })}`); - } - if (action === "mark_reviewed") { - return engramFetch("/review/mark_reviewed", { - method: "POST", - body: { observation_id: params.observation_id || params.id }, - }); - } - throw new Error("action must be one of: list, mark_reviewed"); - } - case "mem_judge": - return engramFetch("/conflicts/judge", { - method: "POST", - body: { - judgment_id: params.judgment_id, - relation: params.relation, - reason: params.reason, - evidence: params.evidence, - confidence: params.confidence, - session_id: params.session_id || sessionId, - }, - }); - case "mem_compare": - return engramFetch("/conflicts/compare", { - method: "POST", - body: { - memory_id_a: params.memory_id_a, - memory_id_b: params.memory_id_b, - relation: params.relation, - confidence: params.confidence, - reasoning: params.reasoning, - model: params.model, - }, - }); - default: - throw new Error(`Unsupported Engram memory tool: ${toolName}`); - } + const handler = MEMORY_TOOL_HANDLERS.get(toolName); + if (!handler) throw new Error(`Unsupported Engram memory tool: ${toolName}`); + return handler({ params, ctx, sessionId, requestedProject, activeProject, activeSessionId }); } function unreachableMessage(timedOutMethod: string | undefined): string { @@ -813,6 +963,39 @@ function unreachableMessage(timedOutMethod: string | undefined): string { return `gentle-engram could not reach the Engram HTTP server at ${ENGRAM_URL}. The Pi-native mem_* tools are registered, but the native memory provider is not currently responding. Run mem_doctor or restart Engram.`; } +// A timed-out exchange never completed, so the tool reports the timeout as an outage instead of +// surfacing the null body, and the session is queued for self-heal recovery. +function timeoutToolResult(timedOutMethod: string, ctx: MemoryToolContext) { + const message = unreachableMessage(timedOutMethod); + const result = { content: [{ type: "text" as const, text: message }], details: { error: message }, isError: true }; + ctx.ui?.setStatus?.("engram", `🧠 ${project} · ${errorStatusLabel(message)}`); + scheduleEngramSelfHeal(ctx); + return result; +} + +// A completed response — including a valid `null` body — becomes the tool result and its compact +// status. A mem_doctor `{status: "error"}` payload keeps its content but is flagged as an error +// result so the UI shows the error status line for it. +function successToolResult(toolName: string, data: unknown, ctx: MemoryToolContext) { + const result = { content: [{ type: "text" as const, text: textResult(data) }], details: { data } }; + const doctorError = toolName === "mem_doctor" && data && typeof data === "object" && "status" in data && data.status === "error"; + const finalResult = doctorError ? { ...result, isError: true } : result; + ctx.ui?.setStatus?.("engram", `🧠 ${project} · ${compactResultStatus(toolName, finalResult)}`); + return finalResult; +} + +// A thrown error means the exchange failed outright: HTTP errors from a live server keep their +// status and payload for diagnosis, and only reachability failures queue self-heal recovery. +function errorToolResult(error: unknown, ctx: MemoryToolContext) { + const message = error instanceof Error ? error.message : String(error); + const details = error instanceof EngramHttpError + ? { error: message, http_status: error.status, data: error.data } + : { error: message }; + ctx.ui?.setStatus?.("engram", `🧠 ${project} · ${errorStatusLabel(message)}`); + if (!(error instanceof EngramHttpError)) scheduleEngramSelfHeal(ctx); + return { content: [{ type: "text" as const, text: message }], details, isError: true }; +} + async function executeMemoryTool(toolName: string, params: Record, ctx: MemoryToolContext) { await initOnce(ctx.cwd); await refreshProjectDetection(ctx.cwd); @@ -821,25 +1004,18 @@ async function executeMemoryTool(toolName: string, params: Record { - await initOnce(ctx.cwd); + // Show the engram status indicator at launch, before the first turn. + await initAndRepaintStatus(ctx); + if (!projectDetectionPending && !projectResolutionError) { + const sessionId = getSessionId(ctx); + if (sessionId) await ensureSessionBestEffort(sessionId); + } }); pi.on("session_shutdown", async (_event: unknown, ctx: SessionContext) => { @@ -880,8 +1062,8 @@ export default function registerEngram(pi: ExtensionAPI) { }); pi.on("session_compact", async (event: unknown, ctx: SessionContext) => { - await initOnce(ctx.cwd); - await refreshProjectDetection(ctx.cwd); + // Re-paint the engram status indicator after a context reload/compact. + await initAndRepaintStatus(ctx); if (projectDetectionPending || projectResolutionError) return; const sessionId = getSessionId(ctx); if (sessionId) await ensureSessionBestEffort(sessionId); @@ -907,8 +1089,9 @@ export default function registerEngram(pi: ExtensionAPI) { }); pi.on("before_agent_start", async (event: AgentStartEvent, ctx: SessionContext) => { - await initOnce(ctx.cwd); - await refreshProjectDetection(ctx.cwd); + // Paint the engram status indicator from the first turn (startup/idle), + // not only after a mem_* tool call. + await initAndRepaintStatus(ctx); const sessionId = getSessionId(ctx); let systemPrompt = event.systemPrompt.length > 0 ? `${event.systemPrompt}\n\n${MEMORY_INSTRUCTIONS}` : MEMORY_INSTRUCTIONS; diff --git a/plugin/pi/memory-tool-chrome.js b/plugin/pi/memory-tool-chrome.js index ea9b1a3e7..c07f8d417 100644 --- a/plugin/pi/memory-tool-chrome.js +++ b/plugin/pi/memory-tool-chrome.js @@ -1,155 +1,8 @@ -const TOOL_LABELS = { - mem_search: "search", - mem_save: "save", - mem_update: "update", - mem_delete: "delete", - mem_suggest_topic_key: "suggest topic", - mem_save_prompt: "save prompt", - mem_session_summary: "session summary", - mem_context: "context", - mem_stats: "stats", - mem_timeline: "timeline", - mem_get_observation: "get observation", - mem_session_start: "start session", - mem_session_end: "end session", - mem_current_project: "current project", - mem_doctor: "doctor", - mem_capture_passive: "capture passive", - mem_judge: "judge", - mem_compare: "compare", - mem_review: "review", -}; - -const ARG_KEYS = { - mem_search: ["query"], - mem_save: ["title", "type"], - mem_update: ["id", "title"], - mem_delete: ["id"], - mem_suggest_topic_key: ["title", "type"], - mem_save_prompt: ["content"], - mem_session_summary: ["content"], - mem_context: ["project", "scope"], - mem_stats: ["project"], - mem_timeline: ["observation_id"], - mem_get_observation: ["id"], - mem_session_start: ["id"], - mem_session_end: ["id"], - mem_current_project: ["cwd"], - mem_doctor: ["check", "project"], - mem_capture_passive: ["source", "content"], - mem_judge: ["judgment_id", "relation"], - mem_compare: ["memory_id_a", "memory_id_b"], - mem_review: ["action", "project", "limit", "observation_id", "id"], -}; - -export const SUPPORTED_MEMORY_TOOLS = Object.freeze(Object.keys(TOOL_LABELS)); - -export function humanToolName(toolName) { - return TOOL_LABELS[toolName] ?? toolName.replace(/^mem_/, "").replace(/_/g, " "); -} - -export function truncateText(value, max = 48) { - const text = String(value ?? "").replace(/\s+/g, " ").trim(); - if (text.length <= max) return text; - return `${text.slice(0, Math.max(0, max - 1))}…`; -} - -function quote(value) { - const text = truncateText(value); - return text ? `“${text}”` : ""; -} - -export function compactToolArg(toolName, args = {}) { - if (toolName === "mem_review") return compactReviewArg(args); - - const keys = ARG_KEYS[toolName] ?? []; - for (const key of keys) { - const value = args?.[key]; - if (value === undefined || value === null || value === "") continue; - if (key === "id" || key === "observation_id" || key === "memory_id_a" || key === "memory_id_b") return `#${value}`; - return quote(value); - } - return ""; -} - -function compactReviewArg(args = {}) { - const parts = []; - if (args.action !== undefined && args.action !== null && args.action !== "") parts.push(String(args.action)); - - const id = args.observation_id ?? args.id; - if (id !== undefined && id !== null && id !== "") parts.push(`#${id}`); - - if (args.project !== undefined && args.project !== null && args.project !== "") parts.push(quote(args.project)); - if (args.limit !== undefined && args.limit !== null && args.limit !== "") parts.push(`limit ${args.limit}`); - - return parts.join(" "); -} - -function firstTextContent(result) { - const block = result?.content?.find?.((entry) => entry?.type === "text" && typeof entry.text === "string"); - return block?.text ?? ""; -} - -function resultData(result) { - return result?.details?.data ?? result?.details ?? result; -} - -function countItems(value) { - if (Array.isArray(value)) return value.length; - if (Array.isArray(value?.results)) return value.results.length; - if (Array.isArray(value?.observations)) return value.observations.length; - if (Array.isArray(value?.sessions)) return value.sessions.length; - if (Array.isArray(value?.prompts)) return value.prompts.length; - if (typeof value?.count === "number") return value.count; - return undefined; -} - -export function compactResultStatus(toolName, result, options = {}) { - if (options.isPartial) return `${humanToolName(toolName)}…`; - if (options.isError || result?.isError) { - const text = truncateText(firstTextContent(result) || result?.details?.error || "error", 64); - return `✗ ${text}`; - } - - const data = resultData(result); - const count = countItems(data); - if (toolName === "mem_search") return `✓ ${count ?? 0} result${count === 1 ? "" : "s"}`; - if (toolName === "mem_context") return `✓ ${firstTextContent(result) || data?.context ? "loaded" : "empty"}`; - if (toolName === "mem_stats") return "✓ loaded"; - if (toolName === "mem_timeline") return `✓ ${count ?? "timeline"}`; - if (toolName === "mem_get_observation") return data?.id ? `✓ observation #${data.id}` : "✓ loaded"; - if (toolName === "mem_save" || toolName === "mem_session_summary") return data?.id ? `✓ saved #${data.id}` : "✓ saved"; - if (toolName === "mem_update") return data?.id ? `✓ updated #${data.id}` : "✓ updated"; - if (toolName === "mem_delete") return data?.id ? `✓ deleted #${data.id}` : "✓ deleted"; - if (toolName === "mem_suggest_topic_key") return data?.topic_key ? `✓ ${data.topic_key}` : "✓ suggested"; - if (toolName === "mem_save_prompt") return data?.id ? `✓ prompt #${data.id}` : "✓ prompt saved"; - if (toolName === "mem_session_start") return "✓ started"; - if (toolName === "mem_session_end") return "✓ ended"; - if (toolName === "mem_current_project") return data?.project ? `✓ ${data.project}` : "✓ detected"; - if (toolName === "mem_doctor") return data?.status ? `✓ ${data.status}` : "✓ checked"; - if (toolName === "mem_capture_passive") return `✓ captured ${data?.saved ?? count ?? 0}`; - if (toolName === "mem_judge") return data?.relation?.sync_id ? `✓ judged ${data.relation.sync_id}` : "✓ judged"; - if (toolName === "mem_compare") return data?.sync_id ? `✓ ${data.sync_id}` : "✓ compared"; - if (toolName === "mem_review") { - if (count !== undefined) return `✓ ${count} need${count === 1 ? "s" : ""} review`; - const id = data?.id ?? data?.observation_id ?? data?.observation?.id; - return id ? `✓ reviewed #${id}` : "✓ reviewed"; - } - return "✓ done"; -} - -export function renderCallText(toolName, args = {}) { - const arg = compactToolArg(toolName, args); - return `🧠 ${humanToolName(toolName)}${arg ? ` ${arg}` : ""} …`; -} - -export function renderResultText(toolName, result, options = {}) { - const status = compactResultStatus(toolName, result, options); - if (!options.expanded || options.isPartial) return `↳ ${status}`; - - const text = firstTextContent(result); - if (text) return `↳ ${status}\n\n${text}`; - - const data = resultData(result); - return `↳ ${status}\n\n${truncateText(JSON.stringify(data, null, 2), 2000)}`; -} +// Compact UI chrome for Engram memory tools — a compatibility facade. +// Tool labels, result status lines, and their shared text helpers live in +// memory-tool-status.js; call-argument formatting and the call/result renderers +// live in memory-tool-render.js. Everything is re-exported here so existing +// consumers (index.ts and the test suite) keep importing from one module. + +export { SUPPORTED_MEMORY_TOOLS, compactResultStatus, humanToolName } from "./memory-tool-status.js"; +export { compactToolArg, renderCallText, renderResultText } from "./memory-tool-render.js"; diff --git a/plugin/pi/memory-tool-render.js b/plugin/pi/memory-tool-render.js new file mode 100644 index 000000000..e91a3d6db --- /dev/null +++ b/plugin/pi/memory-tool-render.js @@ -0,0 +1,84 @@ +// Compact call rendering for Engram memory tools: turns a tool call's arguments +// into the short argument chip shown on the call line, and renders the call and +// result lines displayed in the Pi status area. Tool labels and result status +// lines live in memory-tool-status.js; memory-tool-chrome.js re-exports this +// module's public surface so existing consumers keep importing from one place. + +import { compactResultStatus, firstTextContent, humanToolName, resultData, truncateText } from "./memory-tool-status.js"; + +const ARG_KEYS = { + mem_search: ["query"], + mem_save: ["title", "type"], + mem_update: ["id", "title"], + mem_delete: ["id"], + mem_suggest_topic_key: ["title", "type"], + mem_save_prompt: ["content"], + mem_session_summary: ["content"], + mem_context: ["project", "scope"], + mem_stats: ["project"], + mem_timeline: ["observation_id"], + mem_get_observation: ["id"], + mem_session_start: ["id"], + mem_session_end: ["id"], + mem_current_project: ["cwd"], + mem_doctor: ["check", "project"], + mem_capture_passive: ["source", "content"], + mem_judge: ["judgment_id", "relation"], + mem_compare: ["memory_id_a", "memory_id_b"], + mem_review: ["action", "project", "limit", "observation_id", "id"], +}; + +// Argument keys whose values read best as a bare id reference (#42). +const ID_ARG_KEYS = new Set(["id", "observation_id", "memory_id_a", "memory_id_b"]); + +// An argument is shown only when the caller actually provided one. +function hasValue(value) { + return value !== undefined && value !== null && value !== ""; +} + +function quote(value) { + const text = truncateText(value); + return text ? `“${text}”` : ""; +} + +export function compactToolArg(toolName, args = {}) { + if (toolName === "mem_review") return compactReviewArg(args); + + const keys = ARG_KEYS[toolName] ?? []; + for (const key of keys) { + const value = args?.[key]; + if (!hasValue(value)) continue; + if (ID_ARG_KEYS.has(key)) return `#${value}`; + return quote(value); + } + return ""; +} + +function compactReviewArg(args = {}) { + const parts = []; + if (hasValue(args.action)) parts.push(String(args.action)); + + const id = args.observation_id ?? args.id; + if (hasValue(id)) parts.push(`#${id}`); + + if (hasValue(args.project)) parts.push(quote(args.project)); + if (hasValue(args.limit)) parts.push(`limit ${args.limit}`); + + return parts.join(" "); +} + +export function renderCallText(toolName, args = {}) { + const arg = compactToolArg(toolName, args); + return `🧠 ${humanToolName(toolName)}${arg ? ` ${arg}` : ""} …`; +} + +export function renderResultText(toolName, result, options = {}) { + const status = compactResultStatus(toolName, result, options); + if (!options.expanded || options.isPartial) return `↳ ${status}`; + + const text = firstTextContent(result); + if (text) return `↳ ${status}\n\n${text}`; + + const data = resultData(result); + return `↳ ${status}\n\n${truncateText(JSON.stringify(data, null, 2), 2000)}`; +} diff --git a/plugin/pi/memory-tool-status.js b/plugin/pi/memory-tool-status.js new file mode 100644 index 000000000..47ee7b4cd --- /dev/null +++ b/plugin/pi/memory-tool-status.js @@ -0,0 +1,118 @@ +// Compact status presentation for Engram memory tools: human tool labels, text +// truncation, and the per-tool result status lines. memory-tool-render.js keeps +// the call-argument formatting and call/result renderers; memory-tool-chrome.js +// re-exports both modules' public surface so existing consumers keep importing +// from one place. + +const TOOL_LABELS = { + mem_search: "search", + mem_save: "save", + mem_update: "update", + mem_delete: "delete", + mem_suggest_topic_key: "suggest topic", + mem_save_prompt: "save prompt", + mem_session_summary: "session summary", + mem_context: "context", + mem_stats: "stats", + mem_timeline: "timeline", + mem_get_observation: "get observation", + mem_session_start: "start session", + mem_session_end: "end session", + mem_current_project: "current project", + mem_doctor: "doctor", + mem_capture_passive: "capture passive", + mem_judge: "judge", + mem_compare: "compare", + mem_review: "review", +}; + +export const SUPPORTED_MEMORY_TOOLS = Object.freeze(Object.keys(TOOL_LABELS)); + +export function humanToolName(toolName) { + return TOOL_LABELS[toolName] ?? toolName.replace(/^mem_/, "").replace(/_/g, " "); +} + +export function truncateText(value, max = 48) { + const text = String(value ?? "").replace(/\s+/g, " ").trim(); + if (text.length <= max) return text; + return `${text.slice(0, Math.max(0, max - 1))}…`; +} + +export function firstTextContent(result) { + const block = result?.content?.find?.((entry) => entry?.type === "text" && typeof entry.text === "string"); + return block?.text ?? ""; +} + +export function resultData(result) { + return result?.details?.data ?? result?.details ?? result; +} + +// Envelope shapes that carry a countable item list, in resolution order. +const COUNT_FIELDS = ["results", "observations", "sessions", "prompts"]; + +function countItems(value) { + if (Array.isArray(value)) return value.length; + for (const field of COUNT_FIELDS) { + if (Array.isArray(value?.[field])) return value[field].length; + } + if (typeof value?.count === "number") return value.count; + return undefined; +} + +function partialStatus(toolName) { + return `${humanToolName(toolName)}…`; +} + +function errorStatus(result) { + const text = truncateText(firstTextContent(result) || result?.details?.error || "error", 64); + return `✗ ${text}`; +} + +// Saved-by-id shapes are shared by mem_save and mem_session_summary. +function savedStatus({ data }) { + return data?.id ? `✓ saved #${data.id}` : "✓ saved"; +} + +function reviewStatus({ data, count }) { + if (count !== undefined) return `✓ ${count} need${count === 1 ? "s" : ""} review`; + const id = data?.id ?? data?.observation_id ?? data?.observation?.id; + return id ? `✓ reviewed #${id}` : "✓ reviewed"; +} + +// One small formatter per tool: each receives { data, count, result } and returns the exact +// success line that tool previously produced inline in compactResultStatus. +const RESULT_STATUS_FORMATTERS = { + mem_search: ({ count }) => `✓ ${count ?? 0} result${count === 1 ? "" : "s"}`, + mem_context: ({ data, result }) => `✓ ${firstTextContent(result) || data?.context ? "loaded" : "empty"}`, + mem_stats: () => "✓ loaded", + mem_timeline: ({ count }) => `✓ ${count ?? "timeline"}`, + mem_get_observation: ({ data }) => (data?.id ? `✓ observation #${data.id}` : "✓ loaded"), + mem_save: savedStatus, + mem_session_summary: savedStatus, + mem_update: ({ data }) => (data?.id ? `✓ updated #${data.id}` : "✓ updated"), + mem_delete: ({ data }) => (data?.id ? `✓ deleted #${data.id}` : "✓ deleted"), + mem_suggest_topic_key: ({ data }) => (data?.topic_key ? `✓ ${data.topic_key}` : "✓ suggested"), + mem_save_prompt: ({ data }) => (data?.id ? `✓ prompt #${data.id}` : "✓ prompt saved"), + mem_session_start: () => "✓ started", + mem_session_end: () => "✓ ended", + mem_current_project: ({ data }) => (data?.project ? `✓ ${data.project}` : "✓ detected"), + mem_doctor: ({ data }) => (data?.status ? `✓ ${data.status}` : "✓ checked"), + mem_capture_passive: ({ data, count }) => `✓ captured ${data?.saved ?? count ?? 0}`, + mem_judge: ({ data }) => (data?.relation?.sync_id ? `✓ judged ${data.relation.sync_id}` : "✓ judged"), + mem_compare: ({ data }) => (data?.sync_id ? `✓ ${data.sync_id}` : "✓ compared"), + mem_review: reviewStatus, +}; + +function successStatus(toolName, result) { + const formatter = RESULT_STATUS_FORMATTERS[toolName]; + // Unknown tools (and prototype-chain keys) keep the generic fallback. + if (typeof formatter !== "function") return "✓ done"; + const data = resultData(result); + return formatter({ data, count: countItems(data), result }); +} + +export function compactResultStatus(toolName, result, options = {}) { + if (options.isPartial) return partialStatus(toolName); + if (options.isError || result?.isError) return errorStatus(result); + return successStatus(toolName, result); +} diff --git a/plugin/pi/package.json b/plugin/pi/package.json index 8937eabc1..21b30cfd0 100644 --- a/plugin/pi/package.json +++ b/plugin/pi/package.json @@ -45,6 +45,8 @@ "compaction-recovery.js", "index.ts", "memory-tool-chrome.js", + "memory-tool-render.js", + "memory-tool-status.js", "mcp-template.json", "private-redaction.js", "test/", diff --git a/plugin/pi/test/index-source.test.mjs b/plugin/pi/test/index-source.test.mjs index 4246eaae3..be1bbc2ab 100644 --- a/plugin/pi/test/index-source.test.mjs +++ b/plugin/pi/test/index-source.test.mjs @@ -1,6 +1,7 @@ import assert from "node:assert/strict"; import { readFileSync } from "node:fs"; import { test } from "node:test"; +import { compactResultStatus, humanToolName } from "../memory-tool-chrome.js"; const source = readFileSync(new URL("../index.ts", import.meta.url), "utf8"); @@ -29,11 +30,18 @@ function buildEngramFetchForTest({ timeoutMs = 3000, maxAttempts = 3, backoffBaseMs = 150, + redactUrlPath = (value) => value, + redactValue = (value) => value, } = {}) { - const body = extractFunctionBody("engramFetch", "{\n const method") - .replace("let res: Response | undefined;", "let res;") - .replace("let data: unknown = null;", "let data = null;") - .replace("return data as TResponse;", "return data;"); + // The harness extracts the real module-level helpers so the dynamic function under test runs + // the production algorithm, not a copy; TS-only syntax is stripped to keep plain JS valid. + const stripTypes = (body) => + body + .replace("let data: unknown = null;", "let data = null;") + .replace("return data as TResponse;", "return data;") + .replace("(data as { error?: unknown })", "data") + .replace("(data as { error: string })", "data") + .replace("decodeEngramResponse(", "decodeEngramResponse("); const factory = new Function( "fetch", "wait", @@ -55,9 +63,24 @@ function buildEngramFetchForTest({ function isTimeoutError(error) { ${extractFunctionBody("isTimeoutError", "{\n return error instanceof Error")} } + function httpErrorMessage(data, status) { + ${stripTypes(extractFunctionBody("httpErrorMessage", "{\n if (data &&"))} + } + function engramRequestInit(method, opts) { + ${extractFunctionBody("engramRequestInit", "{\n return {")} + } + async function attemptEngramFetch(url, init) { + ${extractFunctionBody("attemptEngramFetch", "{\n try {")} + } + async function retryEngramFetch(url, init) { + ${extractFunctionBody("retryEngramFetch", "{\n for (let attempt")} + } + async function decodeEngramResponse(res) { + ${stripTypes(extractFunctionBody("decodeEngramResponse", "{\n let data: unknown = null;"))} + } let lastFetchTimeoutMethod; const engramFetch = async function engramFetch(path, opts = {}) { - ${body} + ${stripTypes(extractFunctionBody("engramFetch", "{\n const method"))} }; return { engramFetch, timedOutMethod: () => lastFetchTimeoutMethod }; `, @@ -65,8 +88,8 @@ function buildEngramFetchForTest({ return factory( globalThis.fetch, wait, - (value) => value, - (value) => value, + redactUrlPath, + redactValue, "http://127.0.0.1:7437", timeoutMs, maxAttempts, @@ -103,6 +126,93 @@ function buildScheduleEngramSelfHealForTest({ waitUnref, isEngramRunning, maxAtt return factory(waitUnref, isEngramRunning, 1, maxAttempts); } +function buildExecuteMemoryToolForTest({ data = null, timedOutMethods = [] } = {}) { + // The harness extracts the real executeMemoryTool and the helpers it delegates to, so the + // tests drive the production control flow and formatting — compactResultStatus and + // humanToolName are imported from the real chrome module — not a copy; TS-only syntax is + // stripped to keep plain JS valid. + const stripTypes = (body) => + body + .replace(/ as const/g, "") + .replace(/\(data as ContextResponse\)/g, "data"); + const factory = new Function( + "initOnce", + "refreshProjectDetection", + "humanToolName", + "compactResultStatus", + "callMemoryTool", + "takeLastFetchTimeoutMethod", + "scheduleEngramSelfHeal", + "project", + "ENGRAM_URL", + "ENGRAM_FETCH_TIMEOUT_MS", + ` + class EngramHttpError extends Error { + constructor(message, status, data) { + super(message); + this.name = "EngramHttpError"; + this.status = status; + this.data = data; + } + } + function errorStatusLabel(message) { + ${extractFunctionBody("errorStatusLabel", "{\n if (/ambiguous project")} + } + function unreachableMessage(timedOutMethod) { + ${extractFunctionBody("unreachableMessage", "{\n if (timedOutMethod")} + } + function textResult(data) { + ${stripTypes(extractFunctionBody("textResult", "{\n if (typeof data === \"string\")"))} + } + function timeoutToolResult(timedOutMethod, ctx) { + ${stripTypes(extractFunctionBody("timeoutToolResult", "{\n const message = unreachableMessage"))} + } + function successToolResult(toolName, data, ctx) { + ${stripTypes(extractFunctionBody("successToolResult", "{\n const result = { content:"))} + } + function errorToolResult(error, ctx) { + ${stripTypes(extractFunctionBody("errorToolResult", "{\n const message = error instanceof Error"))} + } + async function executeMemoryTool(toolName, params, ctx) { + ${extractFunctionBody("executeMemoryTool", "{\n await initOnce")} + } + return { executeMemoryTool, EngramHttpError }; + `, + ); + + const events = []; + const statusCalls = []; + const selfHealContexts = []; + const methods = [...timedOutMethods]; + let runCall = async () => data; + const ctx = { + cwd: "/home/user/proj", + ui: { setStatus: (key, text) => { statusCalls.push([key, text]); events.push(`status:${text}`); } }, + }; + const harness = factory( + async (cwd) => { events.push(`initOnce:${cwd}`); }, + async (cwd) => { events.push(`refreshProjectDetection:${cwd}`); }, + humanToolName, + compactResultStatus, + (toolName, params, callCtx) => runCall(toolName, params, callCtx), + () => methods.shift(), + (healCtx) => { selfHealContexts.push(healCtx); events.push("self-heal"); }, + "test-project", + "http://127.0.0.1:7437", + 3000, + ); + return { + executeMemoryTool: harness.executeMemoryTool, + EngramHttpError: harness.EngramHttpError, + setCallMemoryTool: (fn) => { runCall = fn; }, + ctx, + statusCalls, + events, + selfHealContexts, + pendingTimeoutMethods: () => methods.length, + }; +} + function sessionCtx(id, sink) { return { sessionManager: { getSessionId: () => id }, @@ -110,15 +220,64 @@ function sessionCtx(id, sink) { }; } +function buildDetectLocalConfigProjectForTest({ files = {} } = {}) { + // The harness extracts the real config-detection helpers so the tests exercise the + // production upward-walk algorithm, not a copy; TS-only syntax is stripped to keep + // plain JS valid. `files` is a tiny in-memory filesystem keyed by absolute path; an + // Error value makes readFileSync fail like an unreadable file would. + const stripTypes = (body) => body.replace(" as { project_name?: unknown }", ""); + const factory = new Function( + "resolve", + "dirname", + "existsSync", + "readFileSync", + "ENGRAM_URL", + ` + function errorText(error) { + ${extractFunctionBody("errorText", "{\n return error instanceof Error")} + } + function localConfigSearchRoot(cwd) { + ${extractFunctionBody("localConfigSearchRoot", "{\n return resolve(cwd || \".\")")} + } + function readLocalConfigProjectName(configPath) { + ${stripTypes(extractFunctionBody("readLocalConfigProjectName", "{\n const parsed"))} + } + function localConfigProjectResponse(projectName, configPath, dir, cwd) { + ${extractFunctionBody("localConfigProjectResponse", "{\n return {")} + } + function readLocalConfigProject(current, cwd) { + ${extractFunctionBody("readLocalConfigProject", "{\n const configPath")} + } + function detectLocalConfigProject(cwd) { + ${extractFunctionBody("detectLocalConfigProject", "{\n let current")} + } + return detectLocalConfigProject; + `, + ); + const existsSync = (path) => files[path] !== undefined; + const readFileSync = (path) => { + const content = files[path]; + if (content === undefined) throw new Error(`ENOENT: no such file: ${path}`); + if (content instanceof Error) throw content; + return content; + }; + const resolvePath = (value) => (value === "." ? "/home/user/proj" : value); + const dirname = (path) => { + const index = path.lastIndexOf("/"); + return index <= 0 ? "/" : path.slice(0, index); + }; + return factory(resolvePath, dirname, existsSync, readFileSync, "http://127.0.0.1:7437"); +} + test("mem_session_summary accepts explicit project fallback", () => { assert.match(source, /mem_session_summary: Type\.Object\(\{[\s\S]*project: optionalString\("Optional project to use when automatic detection is unavailable"\)/); - assert.match(source, /case "mem_session_summary":[\s\S]*if \(!requestedProject\) requireResolvedProject\(\);[\s\S]*ensureSession\(activeSessionId, activeProject\)[\s\S]*project: activeProject/); + assert.match(source, /function handleMemSessionSummary[\s\S]*if \(!requestedProject\) requireResolvedProject\(\);[\s\S]*ensureSession\(activeSessionId, activeProject\)[\s\S]*project: activeProject/); }); test("mem_search exposes and forwards match_mode and all_projects", () => { assert.match(source, /mem_search: Type\.Object\(\{[\s\S]*all_projects: optionalBoolean\("Search across every project; when true project is ignored"\)/); assert.match(source, /mem_search: Type\.Object\(\{[\s\S]*match_mode: optionalString\("Match mode: all \(default\) or any for broader recall"\)/); - assert.match(source, /case "mem_search":[\s\S]*project: params\.all_projects \? undefined : params\.project[\s\S]*match_mode: params\.match_mode[\s\S]*all_projects: params\.all_projects/); + assert.match(source, /function handleMemSearch[\s\S]*project: params\.all_projects \? undefined : params\.project[\s\S]*match_mode: params\.match_mode[\s\S]*all_projects: params\.all_projects/); }); test("project detection 404 falls back to local config or diagnostic", () => { @@ -128,6 +287,90 @@ test("project detection 404 falls back to local config or diagnostic", () => { assert.match(source, /does not support \/project\/current/); }); +test("local config detection walks upward to the nearest .engram/config.json", () => { + const detect = buildDetectLocalConfigProjectForTest({ + files: { + "/home/user/proj/.engram/config.json": JSON.stringify({ project_name: "engram" }), + }, + }); + const response = detect("/home/user/proj/plugin/pi"); + assert.deepEqual(response, { + project: "engram", + project_source: "config", + project_path: "/home/user/proj", + cwd: "/home/user/proj/plugin/pi", + warning: + "Engram server at http://127.0.0.1:7437 does not support /project/current; using /home/user/proj/.engram/config.json. Upgrade or restart Engram for canonical project detection.", + }); +}); + +test("local config detection trims project_name and preserves an empty cwd verbatim", () => { + const detect = buildDetectLocalConfigProjectForTest({ + files: { + "/home/user/proj/.engram/config.json": JSON.stringify({ project_name: " engram \n" }), + }, + }); + const response = detect(""); + assert.equal(response.project, "engram"); + assert.equal(response.cwd, ""); + assert.equal(response.project_path, "/home/user/proj"); +}); + +test("a malformed config is reported where it was found and stops the walk", () => { + // An ancestor holds a perfectly valid config, but the malformed one found first wins: + // the walk must not silently continue upward past a broken directory config. + const detect = buildDetectLocalConfigProjectForTest({ + files: { + "/home/user/proj/.engram/config.json": "{ not valid json", + "/home/user/.engram/config.json": JSON.stringify({ project_name: "ancestor-project" }), + }, + }); + const response = detect("/home/user/proj"); + assert.deepEqual(response.cwd, "/home/user/proj"); + assert.match(response.error_hint, /^Could not read \/home\/user\/proj\/\.engram\/config\.json: /); + assert.equal(response.project, undefined); +}); + +test("an unreadable config file surfaces as a read failure, not a crash", () => { + const detect = buildDetectLocalConfigProjectForTest({ + files: { + "/home/user/proj/.engram/config.json": new Error("EACCES: permission denied"), + }, + }); + const response = detect("/home/user/proj"); + assert.deepEqual(response.cwd, "/home/user/proj"); + assert.match(response.error_hint, /^Could not read \/home\/user\/proj\/\.engram\/config\.json: EACCES: permission denied$/); +}); + +test("an existing config without a usable project_name reports the fix hint", () => { + const whitespace = buildDetectLocalConfigProjectForTest({ + files: { + "/home/user/proj/.engram/config.json": JSON.stringify({ project_name: " " }), + }, + })("/home/user/proj"); + assert.deepEqual(whitespace, { + cwd: "/home/user/proj", + error_hint: + "/home/user/proj/.engram/config.json exists but project_name is missing or empty. Fix the config or pass project explicitly.", + }); + + const nonString = buildDetectLocalConfigProjectForTest({ + files: { + "/home/user/proj/.engram/config.json": JSON.stringify({ project_name: 42 }), + }, + })("/home/user/proj"); + assert.deepEqual(nonString, { + cwd: "/home/user/proj", + error_hint: + "/home/user/proj/.engram/config.json exists but project_name is missing or empty. Fix the config or pass project explicitly.", + }); +}); + +test("with no config anywhere up to the filesystem root the walk returns undefined", () => { + const detect = buildDetectLocalConfigProjectForTest({ files: {} }); + assert.equal(detect("/home/user/proj/plugin/pi"), undefined); +}); + test("ambiguous_project error maps to actionable status label, not generic 'error'", () => { // The status bar must NOT show the generic 'error' label for ambiguous project conditions. // Instead it should show an actionable label such as 'ambiguous project'. @@ -173,7 +416,9 @@ test("native tool fetch backs off exponentially and attaches a per-request timeo const originalAbortSignalTimeout = AbortSignal.timeout; const waits = []; let observedTimeoutMs; + let timeoutSignalCalls = 0; AbortSignal.timeout = (ms) => { + timeoutSignalCalls += 1; observedTimeoutMs = ms; return originalAbortSignalTimeout(ms); }; @@ -201,6 +446,9 @@ test("native tool fetch backs off exponentially and attaches a per-request timeo assert.equal(calls, 3); assert.deepEqual(waits, [150, 300]); assert.equal(observedTimeoutMs, 2500); + // Every retry must build a fresh timeout signal so each attempt keeps the full budget + // instead of inheriting the milliseconds the previous attempt already spent. + assert.equal(timeoutSignalCalls, 3, "one fresh AbortSignal.timeout per attempt"); } finally { globalThis.fetch = originalFetch; AbortSignal.timeout = originalAbortSignalTimeout; @@ -247,12 +495,18 @@ test("a timeout resolves to null like any other failure, so callers keep their f test("a connection failure records no timeout method, so the generic message is used", async () => { const originalFetch = globalThis.fetch; + let calls = 0; globalThis.fetch = async () => { + calls += 1; throw new Error("connection refused"); }; try { const { engramFetch, timedOutMethod } = buildEngramFetchForTest(); - assert.equal(await engramFetch("/observations", { method: "POST", body: { title: "t" } }), null); + await assert.rejects( + () => engramFetch("/observations", { method: "POST", body: { title: "t" } }), + /could not reach the Engram HTTP server/, + ); + assert.equal(calls, 3, "every retry attempt is spent before giving up"); assert.equal(timedOutMethod(), undefined); } finally { globalThis.fetch = originalFetch; @@ -295,7 +549,7 @@ test("a session-creation timeout still lets the observation write through", asyn const originalFetch = globalThis.fetch; const paths = []; - globalThis.fetch = async (url, init) => { + globalThis.fetch = async (url, _init) => { const path = new URL(url).pathname; paths.push(path); if (path === "/sessions") { @@ -334,7 +588,10 @@ test("a timeout on the session leg does not mislabel an unrelated failure on the const { engramFetch, timedOutMethod } = buildEngramFetchForTest(); assert.equal(await engramFetch("/sessions", { method: "POST", body: { id: "s" } }), null); assert.equal(timedOutMethod(), "POST", "the session leg did time out"); - assert.equal(await engramFetch("/observations", { method: "POST", body: { title: "t" } }), null); + await assert.rejects( + () => engramFetch("/observations", { method: "POST", body: { title: "t" } }), + /could not reach the Engram HTTP server/, + ); assert.equal(timedOutMethod(), undefined, "the write leg's own failure must supersede the stale timeout"); } finally { globalThis.fetch = originalFetch; @@ -435,7 +692,7 @@ test("self-heal clears the stale status on every session that observed the outag test("a session that shuts down mid-outage is dropped instead of having its dead UI touched", async () => { const alive = []; const shutDown = []; - const { scheduleEngramSelfHeal, forgetSelfHealContext, trackedCount } = buildScheduleEngramSelfHealForTest({ + const { scheduleEngramSelfHeal, forgetSelfHealContext } = buildScheduleEngramSelfHealForTest({ waitUnref: () => Promise.resolve(), isEngramRunning: async () => true, }); @@ -478,6 +735,127 @@ test("only reachability failures schedule self-heal, HTTP errors from a live ser assert.match(source, /if \(!\(error instanceof EngramHttpError\)\) scheduleEngramSelfHeal\(ctx\);/); }); +test("init and project detection run before the first status update", async () => { + const { executeMemoryTool, ctx, events } = buildExecuteMemoryToolForTest({ data: { status: "ok" } }); + await executeMemoryTool("mem_doctor", {}, ctx); + assert.deepEqual(events, [ + "initOnce:/home/user/proj", + "refreshProjectDetection:/home/user/proj", + "status:🧠 test-project · doctor…", + "status:🧠 test-project · ✓ ok", + ]); +}); + +test("a timeout outranks the null body: exact warning, error status, and self-heal", async () => { + const message = + "gentle-engram timed out after 3000ms waiting for the Engram HTTP server at http://127.0.0.1:7437. " + + "The POST request may already have been applied — do NOT blindly retry it, or you may duplicate the write. " + + "Verify with mem_search or mem_doctor first."; + const { executeMemoryTool, ctx, statusCalls, selfHealContexts, pendingTimeoutMethods } = + buildExecuteMemoryToolForTest({ data: null, timedOutMethods: ["POST"] }); + const result = await executeMemoryTool("mem_save", {}, ctx); + assert.deepEqual(result, { content: [{ type: "text", text: message }], details: { error: message }, isError: true }); + assert.deepEqual(statusCalls, [ + ["engram", "🧠 test-project · save…"], + ["engram", "🧠 test-project · error"], + ]); + assert.deepEqual(selfHealContexts, [ctx]); + assert.equal(pendingTimeoutMethods(), 0, "the timeout marker is consumed exactly once"); +}); + +test("a valid null body stays a successful result and never schedules self-heal", async () => { + const { executeMemoryTool, ctx, statusCalls, selfHealContexts } = buildExecuteMemoryToolForTest({ data: null }); + const result = await executeMemoryTool("mem_search", {}, ctx); + assert.deepEqual(result, { content: [{ type: "text", text: "{}" }], details: { data: null } }); + assert.equal("isError" in result, false); + assert.deepEqual(statusCalls, [ + ["engram", "🧠 test-project · search…"], + ["engram", "🧠 test-project · ✓ 0 results"], + ]); + assert.deepEqual(selfHealContexts, []); +}); + +test("mem_doctor error payloads keep content and details but are flagged isError", async () => { + const payload = { status: "error", detail: "database locked" }; + const { executeMemoryTool, ctx, statusCalls, selfHealContexts } = buildExecuteMemoryToolForTest({ data: payload }); + const result = await executeMemoryTool("mem_doctor", {}, ctx); + assert.equal(result.isError, true); + assert.equal(result.content[0].text, JSON.stringify(payload, null, 2)); + assert.deepEqual(result.details, { data: payload }); + assert.match(statusCalls[1][1], /^🧠 test-project · ✗ /, "the compact status must show the error line, not ✓"); + assert.deepEqual(selfHealContexts, [], "a doctor-reported error is not an outage"); + + // The flag is doctor-specific: any other tool returning the same payload stays successful. + const other = buildExecuteMemoryToolForTest({ data: payload }); + const statsResult = await other.executeMemoryTool("mem_stats", {}, other.ctx); + assert.equal("isError" in statsResult, false); + assert.equal(other.statusCalls[1][1], "🧠 test-project · ✓ loaded"); +}); + +test("a completed tool call publishes the compact success status in production bytes", async () => { + const data = { project: "test-project", observations: 12 }; + const { executeMemoryTool, ctx, statusCalls, selfHealContexts } = buildExecuteMemoryToolForTest({ data }); + const result = await executeMemoryTool("mem_stats", {}, ctx); + assert.equal(result.content[0].text, JSON.stringify(data, null, 2)); + assert.deepEqual(result.details, { data }); + assert.equal("isError" in result, false); + assert.deepEqual(statusCalls, [ + ["engram", "🧠 test-project · stats…"], + ["engram", "🧠 test-project · ✓ loaded"], + ]); + assert.deepEqual(selfHealContexts, []); +}); + +test("a caught EngramHttpError keeps status and payload and does not schedule self-heal", async () => { + const { executeMemoryTool, EngramHttpError, setCallMemoryTool, ctx, statusCalls, selfHealContexts } = + buildExecuteMemoryToolForTest(); + setCallMemoryTool(async () => { + throw new EngramHttpError("server warming up", 503, { error: "server warming up" }); + }); + const result = await executeMemoryTool("mem_search", {}, ctx); + assert.deepEqual(result, { + content: [{ type: "text", text: "server warming up" }], + details: { error: "server warming up", http_status: 503, data: { error: "server warming up" } }, + isError: true, + }); + assert.deepEqual(statusCalls, [ + ["engram", "🧠 test-project · search…"], + ["engram", "🧠 test-project · error"], + ]); + assert.deepEqual(selfHealContexts, []); +}); + +test("a non-HTTP failure keeps the message and schedules self-heal; ambiguous projects get the actionable label", async () => { + const failure = buildExecuteMemoryToolForTest(); + failure.setCallMemoryTool(async () => { + throw new Error("could not reach the Engram HTTP server"); + }); + const result = await failure.executeMemoryTool("mem_save", {}, failure.ctx); + assert.deepEqual(result, { + content: [{ type: "text", text: "could not reach the Engram HTTP server" }], + details: { error: "could not reach the Engram HTTP server" }, + isError: true, + }); + assert.equal(failure.statusCalls[1][1], "🧠 test-project · error"); + assert.deepEqual(failure.selfHealContexts, [failure.ctx]); + + const ambiguous = buildExecuteMemoryToolForTest(); + ambiguous.setCallMemoryTool(async () => { + throw new Error("ambiguous project: /home/user/proj matches two configs"); + }); + await ambiguous.executeMemoryTool("mem_context", {}, ambiguous.ctx); + assert.equal(ambiguous.statusCalls[1][1], "🧠 test-project · ambiguous project"); + + // Non-Error throws convert through String() instead of crashing the catch path. + const thrown = buildExecuteMemoryToolForTest(); + thrown.setCallMemoryTool(async () => { + throw "plain string failure"; + }); + const converted = await thrown.executeMemoryTool("mem_stats", {}, thrown.ctx); + assert.equal(converted.content[0].text, "plain string failure"); + assert.deepEqual(converted.details, { error: "plain string failure" }); +}); + test("waitUnref schedules a background timer that does not keep the process alive", async () => { const body = extractFunctionBody("waitUnref", "{\n return new Promise"); const factory = new Function(` @@ -519,13 +897,109 @@ test("native tool fetch preserves HTTP error status", async () => { const { engramFetch } = buildEngramFetchForTest(); await assert.rejects( () => engramFetch("/search"), - (error) => error.name === "EngramHttpError" && error.status === 503 && error.message === "server warming up", + (error) => + error.name === "EngramHttpError" && + error.status === 503 && + error.message === "server warming up" && + error.data.error === "server warming up", ); } finally { globalThis.fetch = originalFetch; } }); +test("an HTTP error without a JSON error payload falls back to the status-line message", async () => { + const originalFetch = globalThis.fetch; + globalThis.fetch = async () => ({ + ok: false, + status: 500, + async json() { + throw new SyntaxError("Unexpected token in JSON"); + }, + }); + try { + const { engramFetch } = buildEngramFetchForTest(); + await assert.rejects( + () => engramFetch("/search"), + (error) => + error.name === "EngramHttpError" && + error.status === 500 && + error.message === "Engram request failed with HTTP 500" && + error.data === null, + ); + } finally { + globalThis.fetch = originalFetch; + } +}); + +test("an unparseable success body decodes to null, not an error", async () => { + // An empty /search response is HTTP 200 with a body that is not JSON; that must stay a + // valid null result exactly like a body that parses to null. + const originalFetch = globalThis.fetch; + globalThis.fetch = async () => ({ + ok: true, + status: 200, + async json() { + throw new SyntaxError("Unexpected end of JSON input"); + }, + }); + try { + const { engramFetch, timedOutMethod } = buildEngramFetchForTest(); + assert.equal(await engramFetch("/search"), null); + assert.equal(timedOutMethod(), undefined); + } finally { + globalThis.fetch = originalFetch; + } +}); + +test("a caller abort is treated as a timeout, not a retryable failure", async () => { + const originalFetch = globalThis.fetch; + let calls = 0; + globalThis.fetch = async () => { + calls += 1; + const abort = new Error("The operation was aborted"); + abort.name = "AbortError"; + throw abort; + }; + try { + const { engramFetch, timedOutMethod } = buildEngramFetchForTest(); + assert.equal(await engramFetch("/observations", { method: "POST", body: { title: "t" } }), null); + assert.equal(calls, 1, "an abort must not be retried either"); + assert.equal(timedOutMethod(), "POST"); + } finally { + globalThis.fetch = originalFetch; + } +}); + +test("the request carries the redacted URL and JSON body, and omits both when bodyless", async () => { + const originalFetch = globalThis.fetch; + const seen = []; + globalThis.fetch = async (url, init) => { + seen.push({ url, init }); + return { ok: true, async json() { return { status: "ok" }; } }; + }; + try { + const { engramFetch } = buildEngramFetchForTest({ + redactUrlPath: (path) => `/redacted${path}`, + redactValue: (value) => ({ ...value, redacted: true }), + }); + assert.deepEqual(await engramFetch("/observations", { method: "POST", body: { title: "t" } }), { status: "ok" }); + assert.deepEqual(await engramFetch("/health"), { status: "ok" }); + + const [write, read] = seen; + assert.equal(write.url, "http://127.0.0.1:7437/redacted/observations"); + assert.equal(write.init.method, "POST"); + assert.deepEqual(write.init.headers, { "Content-Type": "application/json" }); + assert.equal(write.init.body, JSON.stringify({ title: "t", redacted: true })); + assert.equal(read.url, "http://127.0.0.1:7437/redacted/health"); + assert.equal(read.init.method, "GET"); + assert.equal(read.init.headers, undefined, "a bodyless request sends no content-type"); + assert.equal(read.init.body, undefined); + } finally { + globalThis.fetch = originalFetch; + } +}); + test("native tool unavailable error names the Pi-native HTTP path", () => { assert.match(source, /gentle-engram could not reach the Engram HTTP server/); assert.match(source, /Pi-native mem_\* tools are registered/); @@ -537,8 +1011,25 @@ test("mem_review is registered as a Pi-native executable memory tool", () => { assert.match(source, /mem_review: Type\.Object\(\{[\s\S]*action: Type\.String\(\{ description: "Action: list \| mark_reviewed" \}\)/); assert.match(source, /mem_review: Type\.Object\(\{[\s\S]*observation_id: optionalNumber\("Observation id for action=mark_reviewed"\)/); assert.match(source, /mem_review: Type\.Object\(\{[\s\S]*id: optionalNumber\("Alias for observation_id"\)/); - assert.match(source, /case "mem_review":[\s\S]*action === "list"[\s\S]*engramFetch\(`\/review\$\{queryString\(\{ project: params\.project, limit: params\.limit \}\)\}`\)/); - assert.match(source, /case "mem_review":[\s\S]*action === "mark_reviewed"[\s\S]*engramFetch\("\/review\/mark_reviewed"/); - assert.match(source, /case "mem_review":[\s\S]*body: \{ observation_id: params\.observation_id \|\| params\.id \}/); + assert.match(source, /function handleMemReview[\s\S]*action === "list"[\s\S]*engramFetch\(`\/review\$\{queryString\(\{ project: params\.project, limit: params\.limit \}\)\}`\)/); + assert.match(source, /function handleMemReview[\s\S]*action === "mark_reviewed"[\s\S]*engramFetch\("\/review\/mark_reviewed"/); + assert.match(source, /function handleMemReview[\s\S]*body: \{ observation_id: params\.observation_id \|\| params\.id \}/); assert.match(source, /for \(const toolName of ENGRAM_TOOLS\)[\s\S]*executeMemoryTool\(toolName/); }); + +test("every registered memory tool maps to exactly one native dispatch handler", () => { + // The switch was extracted into MEMORY_TOOL_HANDLERS; this pins that no tool lost its + // dispatch entry and none gained a stray one during the refactor. + const toolsMatch = source.match(/const ENGRAM_TOOLS = \[([\s\S]*?)\] as const;/); + assert.ok(toolsMatch, "ENGRAM_TOOLS array not found"); + const tools = [...toolsMatch[1].matchAll(/"([^"]+)"/g)].map((match) => match[1]); + assert.ok(tools.length >= 19, `expected the full tool set, found ${tools.length}`); + + const handlersMatch = source.match(/const MEMORY_TOOL_HANDLERS = new Map\(\[([\s\S]*?)\]\);/); + assert.ok(handlersMatch, "MEMORY_TOOL_HANDLERS map not found"); + const mapped = [...handlersMatch[1].matchAll(/\["([^"]+)", \w+\]/g)].map((match) => match[1]); + + assert.deepEqual(new Set(mapped), new Set(tools)); + assert.equal(mapped.length, tools.length, "duplicate handler entries are not allowed"); + assert.match(source, /const handler = MEMORY_TOOL_HANDLERS\.get\(toolName\);[\s\S]*Unsupported Engram memory tool/); +}); diff --git a/plugin/pi/test/memory-tool-chrome.test.mjs b/plugin/pi/test/memory-tool-chrome.test.mjs index 226cd2b00..65f5ce2bb 100644 --- a/plugin/pi/test/memory-tool-chrome.test.mjs +++ b/plugin/pi/test/memory-tool-chrome.test.mjs @@ -1,5 +1,6 @@ import test from "node:test"; import assert from "node:assert/strict"; +import { readFileSync } from "node:fs"; import { SUPPORTED_MEMORY_TOOLS, compactResultStatus, @@ -9,6 +10,8 @@ import { renderResultText, } from "../memory-tool-chrome.js"; +const chromeSource = readFileSync(new URL("../memory-tool-chrome.js", import.meta.url), "utf8"); + const existingTools = [ "mem_search", "mem_save", @@ -39,6 +42,15 @@ test("supported memory tools all have chrome metadata", () => { } }); +test("chrome is a pure facade that re-exports the implementation modules", () => { + // Labels and result status lines live in memory-tool-status.js; call-argument formatting + // and the renderers live in memory-tool-render.js. The facade must forward those exact + // bindings instead of redefining them, so the modules can never drift apart. + assert.match(chromeSource, /export \{ SUPPORTED_MEMORY_TOOLS, compactResultStatus, humanToolName \} from "\.\/memory-tool-status\.js";/); + assert.match(chromeSource, /export \{ compactToolArg, renderCallText, renderResultText \} from "\.\/memory-tool-render\.js";/); + assert.doesNotMatch(chromeSource, /\bfunction\b/, "the facade must not define its own behavior"); +}); + test("compactToolArg prefers short meaningful identifiers", () => { assert.equal(compactToolArg("mem_search", { query: "auth model" }), "“auth model”"); assert.equal(compactToolArg("mem_save", { title: "Fixed the session recovery issue" }), "“Fixed the session recovery issue”"); @@ -73,6 +85,50 @@ test("compactResultStatus summarizes common Engram results", () => { assert.equal(compactResultStatus("mem_review", { details: { data: { id: 42, state: "active" } } }), "✓ reviewed #42"); }); +test("compactResultStatus preserves the exact status line for every tool", () => { + // Pins the per-tool formatter table: one representative result shape per tool, plus the + // generic fallback, so the table extraction cannot drift a single character. + const cases = [ + ["mem_search", { details: { data: { results: [{ id: 1 }] } } }, "✓ 1 result"], + ["mem_context", { details: { data: {} } }, "✓ empty"], + ["mem_stats", { details: { data: {} } }, "✓ loaded"], + ["mem_timeline", { details: { data: { observations: [{}, {}, {}] } } }, "✓ 3"], + ["mem_timeline", { details: { data: {} } }, "✓ timeline"], + ["mem_get_observation", { details: { data: { id: 11 } } }, "✓ observation #11"], + ["mem_get_observation", { details: { data: {} } }, "✓ loaded"], + ["mem_save", { details: { data: {} } }, "✓ saved"], + ["mem_session_summary", { details: { data: { id: 8 } } }, "✓ saved #8"], + ["mem_update", { details: { data: { id: 3 } } }, "✓ updated #3"], + ["mem_update", { details: { data: {} } }, "✓ updated"], + ["mem_delete", { details: { data: { id: 3 } } }, "✓ deleted #3"], + ["mem_delete", { details: { data: {} } }, "✓ deleted"], + ["mem_suggest_topic_key", { details: { data: {} } }, "✓ suggested"], + ["mem_save_prompt", { details: { data: { id: 9 } } }, "✓ prompt #9"], + ["mem_save_prompt", { details: { data: {} } }, "✓ prompt saved"], + ["mem_session_start", { details: { data: {} } }, "✓ started"], + ["mem_session_end", { details: { data: {} } }, "✓ ended"], + ["mem_current_project", { details: { data: {} } }, "✓ detected"], + ["mem_doctor", { details: { data: {} } }, "✓ checked"], + ["mem_capture_passive", { details: { data: {} } }, "✓ captured 0"], + ["mem_judge", { details: { data: {} } }, "✓ judged"], + ["mem_compare", { details: { data: {} } }, "✓ compared"], + ["mem_review", { details: { data: {} } }, "✓ reviewed"], + ["mem_unknown", { details: { data: { id: 1 } } }, "✓ done"], + ]; + for (const [tool, result, expected] of cases) { + assert.equal(compactResultStatus(tool, result), expected, `status for ${tool}`); + } +}); + +test("compactResultStatus error path prefers text content and truncates long errors", () => { + assert.equal(compactResultStatus("mem_save", { isError: true, content: [{ type: "text", text: "server exploded" }] }), "✗ server exploded"); + assert.equal(compactResultStatus("mem_save", { isError: true, details: { error: "boom" } }, {}), "✗ boom"); + assert.equal(compactResultStatus("mem_save", { isError: true }, {}), "✗ error"); + const long = compactResultStatus("mem_save", { isError: true, content: [{ type: "text", text: "e".repeat(120) }] }); + assert.ok(long.startsWith("✗ ") && long.length <= 66, "errors truncate to the 64-char budget plus the marker"); + assert.match(long, /…$/); +}); + test("renderResultText keeps collapsed output compact and expanded output detailed", () => { const result = { content: [{ type: "text", text: "full details\nwith more content" }],