fix(mcp): inject default namespace for memwal_remember_bulk calls

[ducnmm]

Resolves #625 (and companion #611)

Summary

Includes memwal_remember_bulk in the set of NAMESPACE_TOOLS so the configured default namespace (--namespace / MEMWAL_NAMESPACE) is injected when an agent calls bulk remember without an explicit namespace.

Problem

Previously, NAMESPACE_TOOLS in packages/mcp/src/bridge.ts only included memwal_remember, memwal_recall, memwal_analyze, and memwal_restore. When an agent invoked memwal_remember_bulk without an explicit namespace, no namespace argument was injected and the batch was written into the relayer's fallback default namespace, making it unrecallable from the configured project namespace.

Changes

• Added "memwal_remember_bulk" to NAMESPACE_TOOLS in packages/mcp/src/bridge.ts.
• Added unit tests in packages/mcp/test/default-namespace.test.mjs to verify default namespace injection behavior for memwal_remember_bulk and other tools.

Verification

• pnpm --filter @mysten-incubation/memwal-mcp test (14/14 tests pass)

[nikola0x0]

Pre-approve before the two doc lists are updated — the fix itself is correct and I verified it end to end.

Root cause and shape are both right. services/server/scripts/mcp/tools/remember-bulk.ts:16-21 declares namespace as an optional top-level argument that the handler fans out across every fact (facts.map((text) => ({ text, namespace }))), so injecting at the top level rather than per-item is exactly what the relayer expects. NAMESPACE_TOOLS is the only namespace allowlist, so nothing else needs the same entry — though auth-required.ts:37 maintains its own hardcoded tool list and has drifted from the relayer in the same way (collapsed section below).

I also checked that the new tests are real regression guards rather than decoration: at 86c14a5, deleting the one-line Set addition and rebuilding fails 2 of them; with the fix, 14/14 pass. CI wiring is sound — .github/workflows/test.yml:78 runs the suite and the test script does tsc && first, so ../dist/bridge.js resolves under pnpm test. Worth flagging that this is the first static top-level import of a built module in the suite: three existing files spawn dist/bin/memwal-mcp.js as a subprocess and login-preflight.test.mjs:14 uses a cache-busting dynamic import, so a bare node --test or an editor runner hard-errors on this file where the others don't.

Majors (details inline)

• Two user-facing copies of the tool list are now stale. The JSDoc at bridge.ts:37-41 was updated, but docs/mcp/reference.md:142 and packages/mcp/README.md:61 still enumerate the old four — and the README ships to npm. This is the one thing I would fix before merge; two one-line edits, both inline.
• The tests do not pin the contract that actually prevents #625. Every assertion is on the returned object, while bridge.ts:764 discards the return value and depends purely on in-place mutation of msg for the forwarded and replayed-on-reconnect copies. A non-mutating refactor would keep all four tests green while reintroducing the bug. Fix inline on the test file.

Minor (2)

A vacuous tools/list assertion and two uncovered precedence branches (no-default, blank namespace). Both inline, both optional.

Pre-existing / out of diff — not this PR (4)

None of these are regressions from this change and none should block it. Flagging them because the first one is the same class of bug as #625 and is probably worth its own issue.

1. auth-required.ts:37 has the identical drift bug. The pre-login tools/list response hardcodes its own tool array, and it advertises only five: memwal_remember, memwal_recall, memwal_analyze, memwal_restore, memwal_login. Both memwal_remember_bulk and memwal_health are missing — same root cause as #625, a hand-maintained list that fell behind the relayer's registry. Impact is narrower than it first looks, since tools/list hot-hands-off to upstream once credentials exist, but capabilities: { tools: { listChanged: false } } (auth-required.ts:342) explicitly tells the client the list will never change, which invites caching the pre-login five. Worth reconciling against services/server/scripts/mcp/tools/ in a follow-up.

2. bridge.ts:797 reports every outbound failure as a parse error. The catch wraps the whole IIFE — JSON.parse, postMessage, and reconnect alike — and logs all of them as bridge.stdin_parse_failed with the offending line. So a relayer TCP reset mid-tool-call is surfaced as a stdin parse problem, and because no JSON-RPC error envelope is written back for msg.id, the client waits forever on a call that already died. Mislabelled event plus a silent drop.

3. Blank configured namespace is injected verbatim. MEMWAL_NAMESPACE=" " (easy to produce from a stray space in a JSON env block) passes the !namespace check at bridge.ts:62 and gets injected as-is, so memories land in a namespace named two spaces. Meanwhile the identical value arriving from the agent is treated as absent by the .trim() !== "" guard at line 73. Trimming and validating at resolution time in index.ts would make the two ends agree.

4. README.md:106 claims the package ships no test runner. It says "No automated test runner ships with this package (consistent with the rest of the monorepo)" and then gives a manual MCP-client procedure for verifying namespace injection. Already false before this PR — package.json defines test, five suites exist, and both test.yml:78 and release-mcp.yml:51 gate on them. Natural moment to replace that paragraph with pnpm --filter @mysten-incubation/memwal-mcp test, since this PR adds the sixth suite to exactly that feature area.

[ducnmm]

Updated! Addressed all review points:

• Added memwal_remember_bulk to the tool list in docs/mcp/reference.md and packages/mcp/README.md.
• Hardened unit tests in packages/mcp/test/default-namespace.test.mjs (asserted on in-place mutation of msg, corrected non-tool-call test fixture, and added coverage for no-default and blank namespace precedence rules).

All 16 MCP tests pass locally. Ready for merge!

[jessiemongeon1]

Style Guide Audit

All 1 file(s) pass the style guide audit.