Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 18 additions & 2 deletions src/mcp/standalone.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,13 +64,21 @@ function normalizeList(value: unknown): string[] {

const DEFAULT_LIMIT = 10;
const MAX_LIMIT = 100;
type SearchFormat = "compact" | "full";

function parseLimit(raw: unknown, fallback = DEFAULT_LIMIT): number {
if (typeof raw !== "number" && typeof raw !== "string") return fallback;
const n = Number(raw);
if (!Number.isFinite(n) || n <= 0) return fallback;
return Math.min(Math.floor(n), MAX_LIMIT);
}

function parseSearchFormat(raw: unknown): SearchFormat | undefined {
if (typeof raw !== "string") return undefined;
const format = raw.trim().toLowerCase();
return format === "compact" || format === "full" ? format : undefined;
}
Comment on lines +67 to +80

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Enforce format at the boundary instead of silently dropping invalid values.

Line 130 currently accepts invalid format inputs by mapping them to undefined, which bypasses rejection. Also, the parser currently cannot represent "narrative" for recall requests.

Suggested fix
-type SearchFormat = "compact" | "full";
+type SearchFormat = "compact" | "full" | "narrative";

 function parseSearchFormat(raw: unknown): SearchFormat | undefined {
-  if (typeof raw !== "string") return undefined;
+  if (raw === undefined) return undefined;
+  if (typeof raw !== "string") return undefined;
   const format = raw.trim().toLowerCase();
-  return format === "compact" || format === "full" ? format : undefined;
+  return format === "compact" || format === "full" || format === "narrative"
+    ? format
+    : undefined;
 }

@@
-      v.format = parseSearchFormat(args["format"]);
+      v.format = parseSearchFormat(args["format"]);
+      if (args["format"] !== undefined && !v.format) {
+        throw new Error('format must be one of: "full", "compact", "narrative"');
+      }

Also applies to: 130-131

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/mcp/standalone.ts` around lines 67 - 80, The parseSearchFormat function
currently returns undefined for invalid inputs and doesn't accept the
"narrative" format; update the SearchFormat union to include "narrative" (or
create a separate RecallSearchFormat if domain-specific) and change
parseSearchFormat to validate strictly at the boundary by rejecting invalid
values (e.g., throw an Error or return a Result/Error object) instead of
returning undefined; keep the existing trimming/lowercasing and normalize
"compact", "full", and the new "narrative" values, and ensure callers of
parseSearchFormat (search code paths that currently treat undefined as “no-op”)
are updated to handle/reject the parse error accordingly (reference:
SearchFormat type and parseSearchFormat function).


function textResponse(payload: unknown, pretty = false): {
content: Array<{ type: string; text: string }>;
} {
Expand All @@ -89,6 +97,7 @@ interface Validated {
files?: string[];
query?: string;
limit?: number;
format?: SearchFormat;
memoryIds?: string[];
reason?: string;
}
Expand Down Expand Up @@ -118,6 +127,7 @@ function validate(toolName: string, args: Record<string, unknown>): Validated {
}
v.query = query.trim();
v.limit = parseLimit(args["limit"]);
v.format = parseSearchFormat(args["format"]);
return v;
}
case "memory_sessions": {
Expand Down Expand Up @@ -161,9 +171,14 @@ async function handleProxy(
}
case "memory_recall":
case "memory_smart_search": {
const body: { query?: string; limit?: number; format?: SearchFormat } = {
query: v.query,
limit: v.limit,
};
if (v.format) body.format = v.format;
const result = await handle.call("/agentmemory/smart-search", {
method: "POST",
body: JSON.stringify({ query: v.query, limit: v.limit }),
body: JSON.stringify(body),
Comment on lines 172 to +181

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

memory_recall is still proxied to the smart-search endpoint.

Line 179 routes both tools to /agentmemory/smart-search; this keeps recall on compact-search semantics and prevents dedicated recall behavior from /agentmemory/search.

Suggested fix
-    case "memory_recall":
-    case "memory_smart_search": {
+    case "memory_recall":
+    case "memory_smart_search": {
+      const endpoint =
+        v.tool === "memory_recall"
+          ? "/agentmemory/search"
+          : "/agentmemory/smart-search";
       const body: { query?: string; limit?: number; format?: SearchFormat } = {
         query: v.query,
         limit: v.limit,
       };
       if (v.format) body.format = v.format;
-      const result = await handle.call("/agentmemory/smart-search", {
+      const result = await handle.call(endpoint, {
         method: "POST",
         body: JSON.stringify(body),
       });
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/mcp/standalone.ts` around lines 172 - 181, The switch branch handling
"memory_recall" incorrectly calls the smart-search endpoint; update the
"memory_recall" case to call "/agentmemory/search" (leave "memory_smart_search"
calling "/agentmemory/smart-search"), i.e., change the handle.call target in the
"memory_recall" branch from "/agentmemory/smart-search" to "/agentmemory/search"
while keeping the same body construction (query, limit, optional format) and
response handling around result to preserve behavior.

});
return textResponse(result, true);
}
Expand Down Expand Up @@ -227,6 +242,7 @@ async function handleLocal(
case "memory_smart_search": {
const query = (v.query || "").toLowerCase();
const limit = v.limit ?? DEFAULT_LIMIT;
const format = v.format ?? "compact";
const all =
await kvInstance.list<Record<string, unknown>>("mem:memories");
const results = all
Expand All @@ -244,7 +260,7 @@ async function handleLocal(
return query.split(/\s+/).every((word) => text.includes(word));
})
.slice(0, limit);
return textResponse({ mode: "compact", results }, true);
return textResponse({ mode: format, format, results }, true);
}

case "memory_sessions": {
Expand Down
46 changes: 46 additions & 0 deletions test/mcp-standalone.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,52 @@ describe("handleToolCall", () => {
expect(parsed.results[0].content).toBe("TypeScript is great");
});

it("memory_recall preserves a safe format option in local fallback (#440)", async () => {
const kv = new InMemoryKV();
await handleToolCall("memory_save", { content: "TypeScript is great" }, kv);

const result = await handleToolCall(
"memory_recall",
{ query: "typescript", format: "full" },
kv,
);

const parsed = JSON.parse(result.content[0].text);
expect(parsed.format).toBe("full");
expect(parsed.mode).toBe("full");
expect(parsed.results[0].content).toBe("TypeScript is great");
});

it("memory_recall and memory_smart_search forward safe format to smart-search proxy (#440)", async () => {
const proxyProbe = vi.fn(async () => ({ ok: true, status: 200 }));
setLivezProbe(proxyProbe);

const proxyFetch = vi.fn(async () =>
new Response(JSON.stringify({ results: [] }), {
status: 200,
headers: { "content-type": "application/json" },
}),
);
(globalThis as { fetch: typeof fetch }).fetch = proxyFetch as unknown as typeof fetch;

for (const toolName of ["memory_recall", "memory_smart_search"]) {
resetHandleForTests();
setLivezProbe(proxyProbe);
await handleToolCall(
toolName,
{ query: "typescript", limit: 3, format: "full" },
new InMemoryKV(),
);
}

expect(proxyFetch).toHaveBeenCalledTimes(2);
for (const [, init] of proxyFetch.mock.calls) {
expect((init as RequestInit).method).toBe("POST");
const body = JSON.parse((init as RequestInit).body as string);
expect(body).toEqual({ query: "typescript", limit: 3, format: "full" });
}
});
Comment on lines +241 to +269

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Add endpoint assertions so this test catches recall routing regressions.

This test verifies request body only; it won’t fail if memory_recall is sent to the wrong endpoint. Assert URL/path per call (/agentmemory/search vs /agentmemory/smart-search) to lock the contract.

Suggested assertion addition
     expect(proxyFetch).toHaveBeenCalledTimes(2);
+    expect(String(proxyFetch.mock.calls[0][0])).toContain("/agentmemory/search");
+    expect(String(proxyFetch.mock.calls[1][0])).toContain("/agentmemory/smart-search");
     for (const [, init] of proxyFetch.mock.calls) {
       expect((init as RequestInit).method).toBe("POST");
       const body = JSON.parse((init as RequestInit).body as string);
       expect(body).toEqual({ query: "typescript", limit: 3, format: "full" });
     }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
it("memory_recall and memory_smart_search forward safe format to smart-search proxy (#440)", async () => {
const proxyProbe = vi.fn(async () => ({ ok: true, status: 200 }));
setLivezProbe(proxyProbe);
const proxyFetch = vi.fn(async () =>
new Response(JSON.stringify({ results: [] }), {
status: 200,
headers: { "content-type": "application/json" },
}),
);
(globalThis as { fetch: typeof fetch }).fetch = proxyFetch as unknown as typeof fetch;
for (const toolName of ["memory_recall", "memory_smart_search"]) {
resetHandleForTests();
setLivezProbe(proxyProbe);
await handleToolCall(
toolName,
{ query: "typescript", limit: 3, format: "full" },
new InMemoryKV(),
);
}
expect(proxyFetch).toHaveBeenCalledTimes(2);
for (const [, init] of proxyFetch.mock.calls) {
expect((init as RequestInit).method).toBe("POST");
const body = JSON.parse((init as RequestInit).body as string);
expect(body).toEqual({ query: "typescript", limit: 3, format: "full" });
}
});
it("memory_recall and memory_smart_search forward safe format to smart-search proxy (`#440`)", async () => {
const proxyProbe = vi.fn(async () => ({ ok: true, status: 200 }));
setLivezProbe(proxyProbe);
const proxyFetch = vi.fn(async () =>
new Response(JSON.stringify({ results: [] }), {
status: 200,
headers: { "content-type": "application/json" },
}),
);
(globalThis as { fetch: typeof fetch }).fetch = proxyFetch as unknown as typeof fetch;
for (const toolName of ["memory_recall", "memory_smart_search"]) {
resetHandleForTests();
setLivezProbe(proxyProbe);
await handleToolCall(
toolName,
{ query: "typescript", limit: 3, format: "full" },
new InMemoryKV(),
);
}
expect(proxyFetch).toHaveBeenCalledTimes(2);
expect(String(proxyFetch.mock.calls[0][0])).toContain("/agentmemory/search");
expect(String(proxyFetch.mock.calls[1][0])).toContain("/agentmemory/smart-search");
for (const [, init] of proxyFetch.mock.calls) {
expect((init as RequestInit).method).toBe("POST");
const body = JSON.parse((init as RequestInit).body as string);
expect(body).toEqual({ query: "typescript", limit: 3, format: "full" });
}
});
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/mcp-standalone.test.ts` around lines 241 - 269, The test currently only
validates request bodies but not endpoints, so add assertions on the first
argument of each proxyFetch call to ensure routing is correct; after calling
handleToolCall for "memory_recall" and "memory_smart_search" (functions
referenced) inspect proxyFetch.mock.calls and assert the URL/path (first call
equals "/agentmemory/search" or full expected URL for memory_recall and second
call equals "/agentmemory/smart-search" for memory_smart_search) before checking
the RequestInit body, so the test fails if the tools are routed to the wrong
proxy endpoint.


it("memory_save accepts concepts/files as arrays (plugin skill format, #139)", async () => {
const kv = new InMemoryKV();
const result = await handleToolCall(
Expand Down