fix(mcp): treat missing memory/sessions tables as empty memory#255
Conversation
On a fresh org no session has run yet, so the memory/sessions tables
don't exist (provisioning lives in the per-agent SessionStart hooks).
The MCP tools hivemind_search / hivemind_read / hivemind_index surfaced
the raw backend 400 ("relation \"memory\" does not exist") instead of
a clean empty result.
Classify the caught error with isMissingTableError and return the same
friendly empty-memory message the zero-rows path produces, plus a hint
that tables are created when the first agent session starts. No DDL
from the MCP server: it is a read-only path and a READ-role member
could not CREATE TABLE anyway.
Fixes #252
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughMCP handlers now detect backend "missing table" errors and return empty-result messages with a hint about tables being created during the first agent session. Changes apply to hivemind_search, hivemind_read, and hivemind_index and include tests for null content and fresh-org classification. ChangesFresh-org missing-table detection
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Coverage ReportScope: files changed in this PR. Enforced threshold: 90% per metric (per file via
File Coverage — 1 file changed
Generated for commit f244634. |
There was a problem hiding this comment.
🧹 Nitpick comments (4)
tests/claude-code/mcp-server.test.ts (4)
303-308: ⚡ Quick winAssert the complete expected message including the hint.
This test only verifies
"No summaries found."is present, but doesn't verify the fresh-org hint is included. This is inconsistent with the other tests in this suite and provides incomplete coverage.♻️ Proposed fix to assert the exact expected message
- expect(out.content[0].text).toContain("No summaries found."); + expect(out.content[0].text).toBe( + "No summaries found. Hivemind memory is empty — tables are created when the first agent session starts, and entries appear after it ends." + );As per coding guidelines:
tests/**: Prefer asserting on specific values (paths, messages) over generic substrings.🤖 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 `@tests/claude-code/mcp-server.test.ts` around lines 303 - 308, Update the test that mocks queryMock rejection and calls registeredTools.get("hivemind_index")!.handler to assert the full returned message (including the fresh-org hint) rather than only checking for the substring "No summaries found."; locate the test in mcp-server.test.ts (the it block that sets queryMock.mockRejectedValue(new Error('relation "sessions" does not exist')) and calls importServer() and the handler) and replace the partial expect with an assertion that the entire out.content[0].text equals (or contains) the exact expected message string including the hint so the test matches the other suite tests and provides complete coverage.Source: Coding guidelines
294-301: ⚡ Quick winPrefer exact message assertion over substring matching.
The test uses
toContain()to check for substrings. Per the coding guidelines, tests should prefer asserting on specific values rather than generic substrings.♻️ Proposed fix to assert the exact expected message
- expect(out.content[0].text).toContain("No content found at /summaries/alice/a.md"); - expect(out.content[0].text).toContain("first agent session"); - expect(out.content[0].text).not.toContain("Read failed"); + expect(out.content[0].text).toBe( + "No content found at /summaries/alice/a.md. Hivemind memory is empty — tables are created when the first agent session starts, and entries appear after it ends." + ); + expect(out.content[0].text).not.toContain("Read failed");As per coding guidelines:
tests/**: Prefer asserting on specific values (paths, messages) over generic substrings.🤖 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 `@tests/claude-code/mcp-server.test.ts` around lines 294 - 301, The test "hivemind_read: missing table → 'No content found' + fresh-org hint, no raw 400" currently uses substring assertions on out.content[0].text; change these to assert the exact expected message string (use a single equality assertion such as toEqual or toBe) for the entire text returned by registeredTools.get("hivemind_read")!.handler so the test verifies the full composed message (including "No content found at /summaries/alice/a.md" and the "first agent session" hint) and also assert that the exact message does not include "Read failed" (use exact inequality) while leaving queryMock.mockRejectedValue(missingTableErr) and importServer() unchanged.Source: Coding guidelines
275-283: ⚡ Quick winPrefer exact message assertion over substring matching.
The test uses
toContain()to check for substrings ("No summaries found."and"first agent session"). Per the coding guidelines, tests should prefer asserting on specific values rather than generic substrings.♻️ Proposed fix to assert the exact expected message
- expect(out.content[0].text).toContain("No summaries found."); - expect(out.content[0].text).toContain("first agent session"); - expect(out.content[0].text).not.toContain("Index failed"); - expect(out.content[0].text).not.toContain("400"); + expect(out.content[0].text).toBe( + "No summaries found. Hivemind memory is empty — tables are created when the first agent session starts, and entries appear after it ends." + ); + expect(out.content[0].text).not.toContain("Index failed"); + expect(out.content[0].text).not.toContain("400");As per coding guidelines:
tests/**: Prefer asserting on specific values (paths, messages) over generic substrings.🤖 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 `@tests/claude-code/mcp-server.test.ts` around lines 275 - 283, Replace the loose substring assertions with an exact equality check against the full expected message string produced by the hivemind_index handler: locate the test case (the it block) that calls registeredTools.get("hivemind_index")!.handler and replace the two expect(...).toContain(...) checks on out.content[0].text with a single expect(out.content[0].text).toEqual(<full_expected_message>) using the exact message the handler returns (include both the "No summaries found." sentence and the "first agent session" hint in the exact order/format). Keep the existing negative assertions (not toContain "Index failed" and not toContain "400") and do not change importServer or the queryMock setup.Source: Coding guidelines
285-292: ⚡ Quick winPrefer exact message assertion over substring matching.
The test uses
toContain()to check for substrings. Per the coding guidelines, tests should prefer asserting on specific values rather than generic substrings.♻️ Proposed fix to assert the exact expected message
- expect(out.content[0].text).toContain('No matches for "needle".'); - expect(out.content[0].text).toContain("first agent session"); - expect(out.content[0].text).not.toContain("Search failed"); + expect(out.content[0].text).toBe( + 'No matches for "needle". Hivemind memory is empty — tables are created when the first agent session starts, and entries appear after it ends.' + ); + expect(out.content[0].text).not.toContain("Search failed");As per coding guidelines:
tests/**: Prefer asserting on specific values (paths, messages) over generic substrings.🤖 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 `@tests/claude-code/mcp-server.test.ts` around lines 285 - 292, The test for hivemind_search uses substring assertions; change them to assert the exact expected output string. Call registeredTools.get("hivemind_search")!.handler({ query: "needle" }) to get the response and replace the three toContain checks with a single strict equality (or exact string) assertion against the full expected message that includes 'No matches for "needle".' and the "first agent session" hint, and ensure it does not equal or include the raw 400 error text; update references: searchDeeplakeTablesMock, missingTableErr, and the hivemind_search handler to construct the exact expected message used in the assertion.Source: Coding guidelines
🤖 Prompt for all review comments with 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.
Nitpick comments:
In `@tests/claude-code/mcp-server.test.ts`:
- Around line 303-308: Update the test that mocks queryMock rejection and calls
registeredTools.get("hivemind_index")!.handler to assert the full returned
message (including the fresh-org hint) rather than only checking for the
substring "No summaries found."; locate the test in mcp-server.test.ts (the it
block that sets queryMock.mockRejectedValue(new Error('relation "sessions" does
not exist')) and calls importServer() and the handler) and replace the partial
expect with an assertion that the entire out.content[0].text equals (or
contains) the exact expected message string including the hint so the test
matches the other suite tests and provides complete coverage.
- Around line 294-301: The test "hivemind_read: missing table → 'No content
found' + fresh-org hint, no raw 400" currently uses substring assertions on
out.content[0].text; change these to assert the exact expected message string
(use a single equality assertion such as toEqual or toBe) for the entire text
returned by registeredTools.get("hivemind_read")!.handler so the test verifies
the full composed message (including "No content found at /summaries/alice/a.md"
and the "first agent session" hint) and also assert that the exact message does
not include "Read failed" (use exact inequality) while leaving
queryMock.mockRejectedValue(missingTableErr) and importServer() unchanged.
- Around line 275-283: Replace the loose substring assertions with an exact
equality check against the full expected message string produced by the
hivemind_index handler: locate the test case (the it block) that calls
registeredTools.get("hivemind_index")!.handler and replace the two
expect(...).toContain(...) checks on out.content[0].text with a single
expect(out.content[0].text).toEqual(<full_expected_message>) using the exact
message the handler returns (include both the "No summaries found." sentence and
the "first agent session" hint in the exact order/format). Keep the existing
negative assertions (not toContain "Index failed" and not toContain "400") and
do not change importServer or the queryMock setup.
- Around line 285-292: The test for hivemind_search uses substring assertions;
change them to assert the exact expected output string. Call
registeredTools.get("hivemind_search")!.handler({ query: "needle" }) to get the
response and replace the three toContain checks with a single strict equality
(or exact string) assertion against the full expected message that includes 'No
matches for "needle".' and the "first agent session" hint, and ensure it does
not equal or include the raw 400 error text; update references:
searchDeeplakeTablesMock, missingTableErr, and the hivemind_search handler to
construct the exact expected message used in the assertion.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 29781e0f-3165-4ece-8aa9-6e7963fe01b9
📒 Files selected for processing (2)
src/mcp/server.tstests/claude-code/mcp-server.test.ts
CodeRabbit review on #255: the repo test guidelines prefer asserting specific values over generic substrings. Replace the toContain checks in the fresh-org block with exact toBe assertions on the full message (shared freshOrgHint constant).
Index and read map backend rows straight into the text the agent consumes; without the ?? fallbacks a NULL description or message::text would render the literal strings 'null'/'undefined' into the recall context. Cover both paths with exact-output assertions (branches 84.6% -> 94.2% on src/mcp/server.ts).
Summary
Fixes #252.
On a brand-new org no agent session has run yet, so the
memory/sessionstables don't exist — provisioning happens in the per-agent SessionStart hooks (src/hooks/session-start.tsand the hermes/cursor/codex variants), not at install time. The MCP server queries the tables directly, sohivemind_search/hivemind_read/hivemind_indexsurfaced the raw backend 400:Fix
In the three MCP tool handlers, classify the caught error with
isMissingTableError()(the same helpercommands/rules.tsandskillify/pull.tsalready use for this exact situation) and return the friendly empty result the zero-rows path produces, plus a hint:Intentionally no DDL from the MCP server: it is a read-only path, a READ-role org member couldn't
CREATE TABLEanyway, andcreateTableWithRetrycould block a tool call for ~17s. Tables keep being provisioned by the SessionStart hooks.Answers to the questions in #252
ensureSessionsTablebefore writing.Verification
api.deeplake.aiby running the built MCP server over raw stdio JSON-RPC (same probe as the issue) pointed at nonexistent tables: both tools returned the exact 400 from the issue. After the fix, the same probe returns the clean empty-memory message.pre_tool_callgrep fallback from the built bundle: on missing tables it logsfast-path failed, falling throughand stays silent (unchanged by this PR, by design).tests/claude-code/mcp-server.test.tsusing the exact backend error string (all three tools, bare-postgres wording variant, and a negative test that a missing column still surfaces raw). Full suite: 227 files / 4419 tests pass.Session Context
Session transcript
Summary by CodeRabbit
Bug Fixes
Tests