Skip to content

fix(memory-mcp): accept metadata on initialized notifications - #3111

Open
Fibilisim-Tekno wants to merge 1 commit into
affaan-m:mainfrom
Fibilisim-Tekno:fix/memory-initialized-metadata
Open

Fibilisim-Tekno wants to merge 1 commit into
affaan-m:mainfrom
Fibilisim-Tekno:fix/memory-initialized-metadata

Conversation

@Fibilisim-Tekno

Copy link
Copy Markdown

Summary

A client sending notifications/initialized with valid params: {"_meta": {}} never completes initialization: subsequent tools/list requests return -32002 Server is not initialized. The handler currently requires an empty params object, although the MCP schema permits optional _meta through NotificationParams.

Accept object-valued _meta on this notification while preserving validation of unknown fields, malformed metadata, and initialization ordering. This is a protocol-level reproduction; it does not claim a particular client currently emits this metadata.

Testing

  • Added six regression cases covering absent/empty params, empty/extension metadata, malformed metadata, and an early notification.
  • Before fix: 17 passed, 3 failed; both metadata regressions failed, plus the existing Windows symlink EPERM test.
  • After fix: 19 passed, 1 failed; only that same symlink EPERM test remains.
  • ESLint on both changed files and git diff --check: passed.
  • Full npm test was attempted but is not green: unrelated hook/Windows shell tests also fail. No full-suite success is claimed.

Only the notification handler and its tests are changed.

@ecc-tools

ecc-tools Bot commented Sep 13, 2026

Copy link
Copy Markdown
Contributor

ECC Tools / Security Evidence

Commit: f719f5c37b56c169373b0006bf1bad1296714be0

Security evidence gate passed (success)

No security-sensitive scanner-evidence gap detected.

Mode: enforce

Scanned 2 changed file(s). No missing scanner-evidence signal was detected.

Check publication was denied or unavailable. An app owner must enable Checks: read and write, and the installation owner must approve the updated permission.

@ecc-tools

ecc-tools Bot commented Sep 13, 2026

Copy link
Copy Markdown
Contributor

ECC Tools / PR Risk Taxonomy

Commit: f719f5c37b56c169373b0006bf1bad1296714be0

PR taxonomy review recommended (neutral)

Detected 1 PR taxonomy bucket(s): CI/CD Recommendation.

Scanned 2 changed file(s).

Roadmap taxonomy buckets:

CI/CD Recommendation

CI, dependency, coverage, and contract signals should be routed into follow-up checks or verification work.

Signals:

  • 1 CI or workflow path(s) changed

Paths:

  • tests/scripts/memory-mcp.test.js

Check publication was denied or unavailable. An app owner must enable Checks: read and write, and the installation owner must approve the updated permission.

@ecc-tools

ecc-tools Bot commented Sep 13, 2026

Copy link
Copy Markdown
Contributor

ECC Tools / Reference Set Readiness

Commit: f719f5c37b56c169373b0006bf1bad1296714be0

Reference set readiness gaps detected (neutral)

Reference evidence present for 1/7 areas (14%) across 2 changed file(s).

This check is based on files changed in this PR. Repository-level readiness is still reported by /ecc-tools analyze comments and generated manifests.

Area Status Evidence / Next Step
Deep analyzer corpus Missing Add analyzer fixture, golden, benchmark, or reference-set files that can catch analyzer regressions.
RAG/evaluator comparison Missing Add retrieval or evaluator reference-set comparison fixtures with expected ranking behavior.
PR salvage/review corpus Missing Add stale-PR, review-thread, reopen-flow, or salvage reference cases for queue cleanup automation.
Discussion triage corpus Missing Add public discussion triage fixtures, golden cases, or reference sets for informational, answered, and no-response classifications.
Harness compatibility Present tests/scripts/memory-mcp.test.js
Security evidence Missing Attach security evidence such as SBOMs, SARIF, audit reports, or AgentShield evidence packs.
CI failure-mode evidence Missing Add captured CI failure logs, dry-run fixtures, or troubleshooting docs for common workflow failure modes.

Check publication was denied or unavailable. An app owner must enable Checks: read and write, and the installation owner must approve the updated permission.

@ecc-tools

ecc-tools Bot commented Sep 13, 2026

Copy link
Copy Markdown
Contributor

ECC Tools / Hosted Promotion Readiness

Commit: f719f5c37b56c169373b0006bf1bad1296714be0

Hosted promotion readiness passed (success)

No hosted promotion evidence gaps detected across 2 changed file(s); 0 corpus scenarios had matching evidence.

This check compares PR file changes against the evaluator/RAG promotion corpus in src/analyzers/fixtures/evaluator-rag-corpus.ts.
Hosted output scoring inspected 0 completed cached hosted job results.

No evaluator corpus scenarios matched this PR.

Check publication was denied or unavailable. An app owner must enable Checks: read and write, and the installation owner must approve the updated permission.

@coderabbitai

coderabbitai Bot commented Sep 13, 2026

Copy link
Copy Markdown
Contributor

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: a3535fb7-d8ab-47e1-a37e-a7a29aeec9ee

📥 Commits

Reviewing files that changed from the base of the PR and between 8321021 and f719f5c.

📒 Files selected for processing (2)
  • scripts/memory-mcp.mjs
  • tests/scripts/memory-mcp.test.js

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.

📜 Recent review details
⏰ Context from checks skipped due to timeout. (1)
  • GitHub Check: Greptile Review
🧰 Additional context used
📓 Path-based instructions (19)
Focus on command injection, unsafe subprocess usage, path traversal, SSRF, secret exposure, and missing tests for new CLI behavior.

⚙️ CodeRabbit configuration file

Files:

  • scripts/memory-mcp.mjs
Lightweight agents with frequent invocation Pair programming and code generation Worker agents in multi-agent systems Main development work Orchestrating multi-agent workflows Complex coding tasks Complex architectural decisions Maximum rea...

📄 CodeRabbit inference engine (.cursor/rules/common-performance.md)

Files:

  • tests/scripts/memory-mcp.test.js
  • scripts/memory-mcp.mjs
NEVER hardcode secrets in source code - ALWAYS use environment variables or a secret manager

📄 CodeRabbit inference engine (.cursor/rules/common-security.md)

Files:

  • tests/scripts/memory-mcp.test.js
No hardcoded secrets (API keys, passwords, tokens) - validate before any commit

📄 CodeRabbit inference engine (.cursor/rules/common-security.md)

Files:

  • tests/scripts/memory-mcp.test.js
Always create new objects, never mutate existing ones.

📄 CodeRabbit inference engine (.cursor/rules/common-coding-style.md)

Files:

  • tests/scripts/memory-mcp.test.js
Use parameterized queries to prevent SQL injection

📄 CodeRabbit inference engine (.cursor/rules/common-security.md)

Files:

  • tests/scripts/memory-mcp.test.js
Implement XSS prevention by sanitizing HTML output

📄 CodeRabbit inference engine (.cursor/rules/common-security.md)

Files:

  • tests/scripts/memory-mcp.test.js
All user inputs must be validated Enable CSRF protection on all state-changing endpoints Verify authentication and authorization for all protected endpoints Implement rate limiting on all endpoints to prevent abuse Ensure error messages do...

📄 CodeRabbit inference engine (.cursor/rules/common-security.md)

Files:

  • tests/scripts/memory-mcp.test.js
Write tests before implementation (test-driven development); target 80%+ coverage Achieve minimum 80% test coverage across all three layers: Unit, Integration, and E2E Use AAA structure (Arrange / Act / Assert) in tests with descriptive tes...

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Files:

  • tests/scripts/memory-mcp.test.js
Do not hardcode secrets, API keys, passwords, or tokens

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Files:

  • tests/scripts/memory-mcp.test.js
Always create new objects and never mutate in place; return new copies instead Keep files between 200–400 lines typical, with a maximum of 800 lines Extract helpers when a file exceeds 200 lines Handle errors explicitly at every level; neve...

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Files:

  • tests/scripts/memory-mcp.test.js
HTML output must be sanitized where applicable

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Files:

  • tests/scripts/memory-mcp.test.js
Auto-format JavaScript/TypeScript files using Prettier after edit Warn about `console.log` statements in edited files Check all modified files for `console.log` statements before session ends

📄 CodeRabbit inference engine (.cursor/rules/typescript-hooks.md)

Files:

  • tests/scripts/memory-mcp.test.js
Never hardcode secrets; always use environment variables for sensitive credentials like API keys Throw an error when required environment variables are not configured to fail fast and ensure security prerequisites are met

📄 CodeRabbit inference engine (.cursor/rules/typescript-security.md)

Files:

  • tests/scripts/memory-mcp.test.js
Use Playwright as the E2E testing framework for critical user flows in TypeScript/JavaScript

📄 CodeRabbit inference engine (.cursor/rules/typescript-testing.md)

Files:

  • tests/scripts/memory-mcp.test.js
Use spread operator for immutable updates in TypeScript/JavaScript instead of direct mutation Use async/await with try-catch for error handling in TypeScript/JavaScript Use Zod for schema-based input validation in TypeScript/JavaScript No c...

📄 CodeRabbit inference engine (.cursor/rules/typescript-coding-style.md)

Files:

  • tests/scripts/memory-mcp.test.js
Use the ApiResponse interface pattern with generic type parameter: `interface ApiResponse { success: boolean; data?: T; error?: string; meta?: { total: number; page: number; limit: number; } }` Implement custom React hooks following the...

📄 CodeRabbit inference engine (.cursor/rules/typescript-patterns.md)

Files:

  • tests/scripts/memory-mcp.test.js
Required environment variables must be validated at startup

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Files:

  • tests/scripts/memory-mcp.test.js
Use parameterized queries for all database writes (no string interpolation) Auth/authz must be checked server-side for every sensitive path Rate limiting must be applied to all public endpoints

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Files:

  • tests/scripts/memory-mcp.test.js
🔇 Additional comments (2)
scripts/memory-mcp.mjs (1)

386-386: LGTM!

Also applies to: 390-391

tests/scripts/memory-mcp.test.js (1)

807-833: LGTM!

Also applies to: 835-855, 857-875


📝 Summary

Summary by CodeRabbit

  • Bug Fixes

    • Initialization notifications can now include valid optional metadata.
    • Notifications with invalid metadata or unknown parameters continue to be rejected.
    • Initialization behavior remains unchanged for notifications received before the service is ready.
  • Tests

    • Added coverage for valid, empty, malformed, and extension metadata scenarios.

Walkthrough

The notifications/initialized handler now accepts optional record-valued _meta metadata. Tests cover valid metadata, invalid parameters, and notifications received before initialization.

Changes

Initialized notification metadata

Layer / File(s) Summary
Notification handler validation
scripts/memory-mcp.mjs
The handler accepts omitted parameters or a record-valued _meta key. Other parameters and non-record metadata do not initialize the server.
Metadata and lifecycle tests
tests/scripts/memory-mcp.test.js
Tests cover valid metadata, malformed parameters, unknown parameters, and pre-initialize notifications.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to f719f

The metadata support accepts valid object metadata while malformed and premature notifications remain rejected. No actionable merge risk remains.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the main change: accepting metadata on initialized memory MCP notifications.
Description check ✅ Passed The description directly explains the protocol issue, implementation, validation behavior, tests, and known test limitations.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@greptile-apps

greptile-apps Bot commented Sep 13, 2026

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 4/5

The change is not safe for direct POSIX use of the published entry-point file until its executable permission is restored; npm-generated command shims remain functional.

Findings

  1. P2 Restore executable permission
Fix with agent prompt
### Issue 1
scripts/memory-mcp.mjs:1
This CLI entry point is now committed and packed with mode `0644`, so directly running the checked-out or unpacked script on POSIX fails with `Permission denied` before Node starts. npm’s generated command shim restores executable permission during installation, but direct use of this entry point is broken. Restore mode `0755`; this non-blocking compatibility issue prevents direct script invocation.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Summary

  • The MCP initialization changes accept object-valued _meta parameters while preserving malformed-parameter and initialization-order checks. The memory MCP entry-point file lost its executable permission, so directly invoking the checked-out or unpacked script on POSIX fails before the server starts.

Reviews (1) · Last reviewed commit: "fix(memory-mcp): accept metadata on init..."

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant