Skip to content

fix(mcp): local fallback exposes all 7 IMPLEMENTED_TOOLS, not 4 (#234) #283

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
rohitg00 merged 1 commit into from
May 11, 2026
Merged

fix(mcp): local fallback exposes all 7 IMPLEMENTED_TOOLS, not 4 (#234) #283

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
105 changes: 54 additions & 51 deletions src/mcp/standalone.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

import { InMemoryKV } from "./in-memory-kv.js";
import { createStdioTransport } from "./transport.js";
import { getVisibleTools } from "./tools-registry.js";
import { getAllTools } from "./tools-registry.js";
import { getStandalonePersistPath } from "../config.js";
import { VERSION } from "../version.js";
import { generateId } from "../state/schema.js";
Expand Down Expand Up @@ -354,6 +354,57 @@ export async function handleToolCall(
return handleLocal(validated, kvInstance);
}

export async function handleToolsList(): Promise<{ tools: unknown[] }> {
const debug = process.env["AGENTMEMORY_DEBUG"] === "1" || process.env["AGENTMEMORY_DEBUG"] === "true";
const handle = await resolveHandle();
announceMode(handle);
if (debug) {
process.stderr.write(
`[@agentmemory/mcp] tools/list: handle.mode=${handle.mode}${handle.mode === "proxy" ? ` baseUrl=${handle.baseUrl}` : ""}\n`,
);
}
if (handle.mode === "proxy") {
try {
const remote = (await handle.call("/agentmemory/mcp/tools", {
method: "GET",
})) as { tools?: unknown } | null;
if (debug) {
const shape = remote === null
? "null"
: typeof remote !== "object"
? typeof remote
: `keys=${Object.keys(remote as object).join(",")} toolsType=${Array.isArray((remote as { tools?: unknown }).tools) ? `array(len=${((remote as { tools: unknown[] }).tools).length})` : typeof (remote as { tools?: unknown }).tools}`;
process.stderr.write(
`[@agentmemory/mcp] tools/list: remote response shape: ${shape}\n`,
);
}
if (remote && Array.isArray(remote.tools)) {
if (debug) {
process.stderr.write(
`[@agentmemory/mcp] tools/list: returning ${remote.tools.length} tools from server\n`,
);
}
return { tools: remote.tools };
}
process.stderr.write(
`[@agentmemory/mcp] tools/list: server returned unexpected shape (no .tools array); falling back to local IMPLEMENTED_TOOLS list. Set AGENTMEMORY_DEBUG=1 to inspect response.\n`,
);
} catch (err) {
process.stderr.write(
`[@agentmemory/mcp] tools/list proxy failed: ${err instanceof Error ? err.message : String(err)}; falling back to local list\n`,
);
invalidateHandle();
}
}
const fallback = getAllTools().filter((t) => IMPLEMENTED_TOOLS.has(t.name));
if (debug) {
process.stderr.write(
`[@agentmemory/mcp] tools/list: returning ${fallback.length} local fallback tools (${fallback.map((t) => t.name).join(",")})\n`,
);
}
return { tools: fallback };
}

const transport = createStdioTransport(async (method, params) => {
switch (method) {
case "initialize":
Expand All @@ -369,56 +420,8 @@ const transport = createStdioTransport(async (method, params) => {
case "notifications/initialized":
return {};

case "tools/list": {
const debug = process.env["AGENTMEMORY_DEBUG"] === "1" || process.env["AGENTMEMORY_DEBUG"] === "true";
const handle = await resolveHandle();
announceMode(handle);
if (debug) {
process.stderr.write(
`[@agentmemory/mcp] tools/list: handle.mode=${handle.mode}${handle.mode === "proxy" ? ` baseUrl=${handle.baseUrl}` : ""}\n`,
);
}
if (handle.mode === "proxy") {
try {
const remote = (await handle.call("/agentmemory/mcp/tools", {
method: "GET",
})) as { tools?: unknown } | null;
if (debug) {
const shape = remote === null
? "null"
: typeof remote !== "object"
? typeof remote
: `keys=${Object.keys(remote as object).join(",")} toolsType=${Array.isArray((remote as { tools?: unknown }).tools) ? `array(len=${((remote as { tools: unknown[] }).tools).length})` : typeof (remote as { tools?: unknown }).tools}`;
process.stderr.write(
`[@agentmemory/mcp] tools/list: remote response shape: ${shape}\n`,
);
}
if (remote && Array.isArray(remote.tools)) {
if (debug) {
process.stderr.write(
`[@agentmemory/mcp] tools/list: returning ${remote.tools.length} tools from server\n`,
);
}
return { tools: remote.tools };
}
process.stderr.write(
`[@agentmemory/mcp] tools/list: server returned unexpected shape (no .tools array); falling back to local IMPLEMENTED_TOOLS list. Set AGENTMEMORY_DEBUG=1 to inspect response.\n`,
);
} catch (err) {
process.stderr.write(
`[@agentmemory/mcp] tools/list proxy failed: ${err instanceof Error ? err.message : String(err)}; falling back to local list\n`,
);
invalidateHandle();
}
}
const fallback = getVisibleTools().filter((t) => IMPLEMENTED_TOOLS.has(t.name));
if (debug) {
process.stderr.write(
`[@agentmemory/mcp] tools/list: returning ${fallback.length} local fallback tools (${fallback.map((t) => t.name).join(",")})\n`,
);
}
return { tools: fallback };
}
case "tools/list":
return handleToolsList();

case "tools/call": {
const toolName = params.name as string;
Expand Down
26 changes: 26 additions & 0 deletions test/mcp-standalone-proxy.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,32 @@ describe("@agentmemory/mcp standalone — server proxy (issue #159)", () => {
expect(joined).toMatch(/AGENTMEMORY_FORCE_PROXY/);
});

it("local fallback tools/list returns all 7 IMPLEMENTED_TOOLS regardless of AGENTMEMORY_TOOLS env (#234)", async () => {
const { handleToolsList } = await import("../src/mcp/standalone.js");
installFetch(() => {
throw new Error("ECONNREFUSED");
});
delete process.env["AGENTMEMORY_TOOLS"];
const before = await handleToolsList();
const beforeTools = before.tools as Array<{ name: string }>;
expect(beforeTools.map((t) => t.name).sort()).toEqual([
"memory_audit",
"memory_export",
"memory_governance_delete",
"memory_recall",
"memory_save",
"memory_sessions",
"memory_smart_search",
]);
expect(beforeTools).toHaveLength(7);

resetHandleForTests();
process.env["AGENTMEMORY_TOOLS"] = "core";
const core = await handleToolsList();
expect((core.tools as unknown[]).length).toBe(7);
delete process.env["AGENTMEMORY_TOOLS"];
});
Comment on lines +250 to +274
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot May 11, 2026

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

Ensure AGENTMEMORY_TOOLS is restored via finally for test isolation.

process.env["AGENTMEMORY_TOOLS"] is only cleaned up at the end of the happy path. If an assertion throws earlier, this can leak into later tests and create flakiness.

💡 Suggested patch 
 it("local fallback tools/list returns all 7 IMPLEMENTED_TOOLS regardless of AGENTMEMORY_TOOLS env (`#234`)", async () => {
+  const previousToolsEnv = process.env["AGENTMEMORY_TOOLS"];
   const { handleToolsList } = await import("../src/mcp/standalone.js");
   installFetch(() => {
     throw new Error("ECONNREFUSED");
   });
-  delete process.env["AGENTMEMORY_TOOLS"];
-  const before = await handleToolsList();
-  const beforeTools = before.tools as Array<{ name: string }>;
-  expect(beforeTools.map((t) => t.name).sort()).toEqual([
-    "memory_audit",
-    "memory_export",
-    "memory_governance_delete",
-    "memory_recall",
-    "memory_save",
-    "memory_sessions",
-    "memory_smart_search",
-  ]);
-  expect(beforeTools).toHaveLength(7);
-
-  resetHandleForTests();
-  process.env["AGENTMEMORY_TOOLS"] = "core";
-  const core = await handleToolsList();
-  expect((core.tools as unknown[]).length).toBe(7);
-  delete process.env["AGENTMEMORY_TOOLS"];
+  try {
+    delete process.env["AGENTMEMORY_TOOLS"];
+    const before = await handleToolsList();
+    const beforeTools = before.tools as Array<{ name: string }>;
+    expect(beforeTools.map((t) => t.name).sort()).toEqual([
+      "memory_audit",
+      "memory_export",
+      "memory_governance_delete",
+      "memory_recall",
+      "memory_save",
+      "memory_sessions",
+      "memory_smart_search",
+    ]);
+    expect(beforeTools).toHaveLength(7);
+
+    resetHandleForTests();
+    process.env["AGENTMEMORY_TOOLS"] = "core";
+    const core = await handleToolsList();
+    expect((core.tools as unknown[]).length).toBe(7);
+  } finally {
+    if (previousToolsEnv === undefined) {
+      delete process.env["AGENTMEMORY_TOOLS"];
+    } else {
+      process.env["AGENTMEMORY_TOOLS"] = previousToolsEnv;
+    }
+  }
 });
📝 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("local fallback tools/list returns all 7 IMPLEMENTED_TOOLS regardless of AGENTMEMORY_TOOLS env (#234)", async () => {
const { handleToolsList } = await import("../src/mcp/standalone.js");
installFetch(() => {
throw new Error("ECONNREFUSED");
});
delete process.env["AGENTMEMORY_TOOLS"];
const before = await handleToolsList();
const beforeTools = before.tools as Array<{ name: string }>;
expect(beforeTools.map((t) => t.name).sort()).toEqual([
"memory_audit",
"memory_export",
"memory_governance_delete",
"memory_recall",
"memory_save",
"memory_sessions",
"memory_smart_search",
]);
expect(beforeTools).toHaveLength(7);
resetHandleForTests();
process.env["AGENTMEMORY_TOOLS"] = "core";
const core = await handleToolsList();
expect((core.tools as unknown[]).length).toBe(7);
delete process.env["AGENTMEMORY_TOOLS"];
});
it("local fallback tools/list returns all 7 IMPLEMENTED_TOOLS regardless of AGENTMEMORY_TOOLS env (`#234`)", async () => {
const previousToolsEnv = process.env["AGENTMEMORY_TOOLS"];
const { handleToolsList } = await import("../src/mcp/standalone.js");
installFetch(() => {
throw new Error("ECONNREFUSED");
});
try {
delete process.env["AGENTMEMORY_TOOLS"];
const before = await handleToolsList();
const beforeTools = before.tools as Array<{ name: string }>;
expect(beforeTools.map((t) => t.name).sort()).toEqual([
"memory_audit",
"memory_export",
"memory_governance_delete",
"memory_recall",
"memory_save",
"memory_sessions",
"memory_smart_search",
]);
expect(beforeTools).toHaveLength(7);
resetHandleForTests();
process.env["AGENTMEMORY_TOOLS"] = "core";
const core = await handleToolsList();
expect((core.tools as unknown[]).length).toBe(7);
} finally {
if (previousToolsEnv === undefined) {
delete process.env["AGENTMEMORY_TOOLS"];
} else {
process.env["AGENTMEMORY_TOOLS"] = previousToolsEnv;
}
}
});
🤖 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-proxy.test.ts` around lines 250 - 274, The test leaks
process.env["AGENTMEMORY_TOOLS"] if an assertion fails; wrap the environment
mutation and cleanup around the handleToolsList calls in a try/finally so
AGENTMEMORY_TOOLS is always restored: save the original value into a variable,
set/delete process.env["AGENTMEMORY_TOOLS"] as needed before calling
handleToolsList (and resetHandleForTests()/installFetch() usage stays the same),
then restore the original value in a finally block to guarantee isolation for
subsequent tests.


it("AGENTMEMORY_PROBE_TIMEOUT_MS overrides the default probe timeout", async () => {
process.env["AGENTMEMORY_PROBE_TIMEOUT_MS"] = "50";
let probeStarted = 0;
Expand Down
Loading