fix(mcp): route memory_recall through full search#532
Open
Lubrsy706 wants to merge 2 commits into
Open
Conversation
Signed-off-by: Lubrsy706 <lubrsy706@gmail.com>
|
@Lubrsy706 is attempting to deploy a commit to the rohitg00's projects Team on Vercel. A member of the Team first needs to authorize it. |
Contributor
📝 WalkthroughWalkthroughRefactors memory_recall to validate token_budget, normalize format (full|compact|narrative), forward format/token_budget to the proxy (/agentmemory/search), add LocalMemory search helpers, implement format-aware local responses (compact list, narrative text, full observations), and update proxy and local tests. ChangesMemory_recall format and token_budget support
Sequence Diagram(s)sequenceDiagram
participant Client
participant MCP as MCP.handleProxy
participant AgentMemory as /agentmemory/search
Client->>MCP: memory_recall(query, limit, format, token_budget)
MCP->>AgentMemory: POST { query, limit, format, token_budget? }
AgentMemory-->>MCP: { format, text?, results }
MCP-->>Client: textResponse(proxyPayload, pretty=true)
sequenceDiagram
participant Client
participant MCPLocal as MCP.handleLocal
participant LocalMemory as searchLocalMemories
Client->>MCPLocal: memory_recall(query, limit, format)
MCPLocal->>LocalMemory: searchLocalMemories(queryWords, limit)
LocalMemory-->>MCPLocal: matched memories list
MCPLocal-->>Client: return compact list | narrative text | full observation objects
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
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 unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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.
Inline comments:
In `@src/mcp/standalone.ts`:
- Around line 185-199: The handler currently accepts arbitrary args["format"]
and sets v.format, which lets a memory_smart_search request forward unsupported
formats upstream; enforce that when v.kind === "memory_smart_search" you ignore
or override any provided format and set v.format = "compact" (or remove/omit
format) so only compact is used, and ensure args["format"] processing (the block
referencing args["format"] / v.format) does not apply for memory_smart_search;
reference the symbols v.kind, v.format, and args["format"] to locate and change
the logic.
- Around line 190-199: The current validation checks `budget > 0` then assigns
`v.tokenBudget = Math.floor(budget)`, which allows sub-unit inputs like 0.5 to
become 0; change both branches to compute the floored value first and only
assign if that floored value is > 0: for the numeric branch compute floorN =
Math.floor(budget) and set `v.tokenBudget = floorN` only when floorN > 0; for
the string branch parse to n, compute floorN = Math.floor(n) and assign only
when floorN > 0, ensuring `v.tokenBudget` never becomes 0 from fractional inputs
while still accepting valid positive integer budgets.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 870fd25d-7a78-4cc8-9e78-5e1e1ee2b223
📒 Files selected for processing (2)
src/mcp/standalone.tstest/mcp-standalone-proxy.test.ts
Comment on lines
+185
to
+199
| } else { | ||
| const fmt = args["format"]; | ||
| if (typeof fmt === "string" && fmt.trim()) { | ||
| v.format = fmt.trim().toLowerCase(); | ||
| } | ||
| const budget = args["token_budget"]; | ||
| if ( | ||
| typeof budget === "number" && | ||
| Number.isFinite(budget) && | ||
| budget > 0 | ||
| ) { | ||
| v.tokenBudget = Math.floor(budget); | ||
| } else if (typeof budget === "string" && budget.trim()) { | ||
| const n = Number(budget); | ||
| if (Number.isFinite(n) && n > 0) v.tokenBudget = Math.floor(n); |
Contributor
There was a problem hiding this comment.
Keep memory_smart_search compact-only.
Issue #507 says /agentmemory/smart-search does not support format, but this path accepts arbitrary values and Line 258 forwards them upstream. That creates a proxy-only failure mode while local fallback still stays compact.
Suggested fix
} else {
- const fmt = args["format"];
- if (typeof fmt === "string" && fmt.trim()) {
- v.format = fmt.trim().toLowerCase();
- }
const budget = args["token_budget"];
if (
typeof budget === "number" &&
@@
case "memory_smart_search": {
const body: Record<string, unknown> = { query: v.query, limit: v.limit };
- if (v.format != null) body["format"] = v.format;
if (v.tokenBudget != null) body["token_budget"] = v.tokenBudget;
const result = await handle.call("/agentmemory/smart-search", {
method: "POST",Also applies to: 257-259
🤖 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 185 - 199, The handler currently accepts
arbitrary args["format"] and sets v.format, which lets a memory_smart_search
request forward unsupported formats upstream; enforce that when v.kind ===
"memory_smart_search" you ignore or override any provided format and set
v.format = "compact" (or remove/omit format) so only compact is used, and ensure
args["format"] processing (the block referencing args["format"] / v.format) does
not apply for memory_smart_search; reference the symbols v.kind, v.format, and
args["format"] to locate and change the logic.
Comment on lines
+190
to
+199
| const budget = args["token_budget"]; | ||
| if ( | ||
| typeof budget === "number" && | ||
| Number.isFinite(budget) && | ||
| budget > 0 | ||
| ) { | ||
| v.tokenBudget = Math.floor(budget); | ||
| } else if (typeof budget === "string" && budget.trim()) { | ||
| const n = Number(budget); | ||
| if (Number.isFinite(n) && n > 0) v.tokenBudget = Math.floor(n); |
Contributor
There was a problem hiding this comment.
Ignore sub-unit token_budget values before flooring.
budget > 0 followed by Math.floor(...) turns values like 0.5 into 0, so this branch can still forward a non-positive token_budget.
Suggested fix
const budget = args["token_budget"];
- if (
- typeof budget === "number" &&
- Number.isFinite(budget) &&
- budget > 0
- ) {
- v.tokenBudget = Math.floor(budget);
- } else if (typeof budget === "string" && budget.trim()) {
- const n = Number(budget);
- if (Number.isFinite(n) && n > 0) v.tokenBudget = Math.floor(n);
- }
+ const n =
+ typeof budget === "number"
+ ? budget
+ : typeof budget === "string" && budget.trim()
+ ? Number(budget)
+ : undefined;
+ if (n !== undefined && Number.isFinite(n)) {
+ const floored = Math.floor(n);
+ if (floored > 0) v.tokenBudget = floored;
+ }🤖 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 190 - 199, The current validation checks
`budget > 0` then assigns `v.tokenBudget = Math.floor(budget)`, which allows
sub-unit inputs like 0.5 to become 0; change both branches to compute the
floored value first and only assign if that floored value is > 0: for the
numeric branch compute floorN = Math.floor(budget) and set `v.tokenBudget =
floorN` only when floorN > 0; for the string branch parse to n, compute floorN =
Math.floor(n) and assign only when floorN > 0, ensuring `v.tokenBudget` never
becomes 0 from fractional inputs while still accepting valid positive integer
budgets.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
memory_recallproxy calls to/agentmemory/searchinstead of/agentmemory/smart-searchformatandtoken_budgetsofull,compact, andnarrativerecall modes work as advertisedmemory_recallwith search-style responses while leavingmemory_smart_searchcompactFixes #507
Tests
npm test -- test/mcp-standalone-proxy.test.ts test/mcp-standalone.test.tsnpm run buildnpm testSummary by CodeRabbit
New Features
Behavior Changes
Tests