Skip to content

Fix/embedding key resolution - #942

Closed
Noodle05 wants to merge 38 commits into
rohitg00:mainfrom
Noodle05:fix/embedding-key-resolution
Closed

Fix/embedding key resolution#942
Noodle05 wants to merge 38 commits into
rohitg00:mainfrom
Noodle05:fix/embedding-key-resolution

Conversation

@Noodle05

@Noodle05 Noodle05 commented Jun 16, 2026

Copy link
Copy Markdown

Summary by CodeRabbit

Release Notes

  • New Features

    • Docker containerization support with Docker Compose for simplified deployment and multi-architecture image builds.
    • MCP Stream HTTP server enabling streamable HTTP client connections with Bearer token authentication.
  • Documentation

    • Added Docker Compose setup guide in README with step-by-step instructions.
  • Configuration

    • Expanded environment configuration options for REST/MCP networking and viewer host settings.

Noodle05 and others added 30 commits June 14, 2026 23:52
- Install @modelcontextprotocol/node SDK for MCP Streamable HTTP transport
- Add @modelcontextprotocol/node to tsdown external array (ESM, not bundled)
- Add mcpPort field to AgentMemoryConfig type (defaults to restPort + 3)
- Add AGENTMEMORY_MCP_PORT env var support in loadConfig()

Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
Move handleToolsList() and handleToolCall() plus all their internal
helpers (validate, handleProxy, handleLocal, handleProxyGeneric,
normalizeList, parseLimit, textResponse, announceMode) from
src/mcp/standalone.ts into a new src/mcp/handler.ts shared module.

The extracted functions accept explicit sdk, kv, and config parameters
so they can be reused by both the existing stdio transport and the
upcoming Stream HTTP transport without code duplication.

standalone.ts retains:
- InMemoryKV instantiation
- createStdioTransport and the stdio message dispatch loop
- SIGINT/SIGTERM handlers
- Backward-compatible wrapper exports (old signatures) for tests

All 1410 non-integration tests pass; no behavioral changes.

Co-Authored-By: Claude <noreply@anthropic.com>
Implement JSON-RPC-over-HTTP MCP transport that coexists with existing stdio
transport. Uses Node.js http.createServer() with manual JSON-RPC handling,
Bearer auth via timingSafeCompare(), and session management via a Map.

- src/mcp/stream-http.ts: HTTP server with MCP JSON-RPC handling (initialize,
  tools/list, tools/call, notifications)
- test/mcp-stream-http.test.ts: 22 tests covering initialize, tools/list,
  tools/call, Bearer auth, JSON-RPC errors, session management
- test/mcp-transport-coexistence.test.ts: 5 tests verifying HTTP and stdio
  transports expose identical tool sets and handle notifications
- src/index.ts: Wire up MCP server on config.mcpPort (3114), add to boot log
  and graceful shutdown
- package.json: Add @cfworker/json-schema peer dependency

Port scheme: restPort + 3 (3114), configurable via AGENTMEMORY_MCP_PORT

Co-Authored-By: Claude <noreply@anthropic.com>
- Add session TTL with lastUsedAt tracking and periodic eviction (every 10 min)
- Fix session-not-found error code from -32001 to -32000 per contract
- Add explanation comment for manual JSON-RPC vs SDK transport
- Add 5MB request body size limit with 413 response
- Add HTTP method validation (POST/DELETE only, 405 for others)
- Add Content-Type validation (application/json required, 415 otherwise)

Co-Authored-By: Claude <noreply@anthropic.com>
- Fix JsonRpcMessage.id type to allow null (JSON-RPC notification compliant)
- Remove as cast in errorResponse() now that id type includes null
- Remove dead req.headers["Mcp-Session-Id"] branch (Node.js lowercases headers)
- DELETE handler now removes session from sessions Map
- Remove unused @cfworker/json-schema dependency
- Tighten parse error test to assert exactly -32700
- Add SDK bug reference URL in header comment
- Add safety guard comment for res.headersSent in body-too-large handler

Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
…cker

- T039: Add "stream-http" connection category to ConnectAdapter types
- T040: Add AGENTMEMORY_MCP_HTTP_BLOCK constant and HTTP transport comments
  in CLI connect adapters for Stream HTTP URL-based connections
- T041: Add AGENTMEMORY_MCP_PORT entry to .env.example ports section
- T042: Add Docker quickstart section to README.md
- T045: Add mcpPort to startup banner alongside REST API and MCP surface lines

Co-Authored-By: Claude <noreply@anthropic.com>
- Remove @modelcontextprotocol/node from package.json dependencies
- Remove @modelcontextprotocol/node from tsdown.config.ts external array
- Update stream-http.ts comment to remove SDK revisit suggestion
- Update spec.md assumptions: Stream HTTP uses hand-rolled JSON-RPC
  server on port 3114 (separate from REST on 3111)
- 1450 tests pass (6 pre-existing environmental failures in worktree)

Co-Authored-By: Claude <noreply@anthropic.com>
Merge the multi-container Docker setup into a single image:
- Dockerfile: 3-stage build (iii binary + TypeScript build + runtime)
- entrypoint.sh: based on deploy/coolify upstream, starts iii-engine in
  background then agentmemory in foreground with graceful shutdown trap
- docker-compose.yml: simplified to single agentmemory service
- iii-config.docker.yaml: removed (entrypoint generates config dynamically)

Builds agentmemory from local TypeScript source (not npm registry).
Minimal diff vs deploy/coolify upstream — only source-build vs
npm-install differences.

Co-Authored-By: Claude <noreply@anthropic.com>
…xample

These env vars are needed for the single-image Docker setup.
AGENTMEMORY_SECRET_FILE points to the persisted HMAC secret file.
AGENTMEMORY_REST_HOST sets the REST upstream host for the viewer proxy.

Co-Authored-By: Claude <noreply@anthropic.com>
- Fix 'iii run' → 'iii' (no 'run' subcommand in iii CLI)
- Fix 'SIGTERM SIGINT' → 'TERM INT' (POSIX sh requires no SIG prefix)
- Verified: container starts, health check passes, viewer responds,
  HMAC persists across restarts, no regressions

Co-Authored-By: Claude <noreply@anthropic.com>
Keep the static config file as reference/documentation of expected
iii-engine worker configuration. The entrypoint still generates the
runtime config dynamically (matching deploy/coolify pattern), but the
static file stays in the repo per the minimal-changes principle.

Co-Authored-By: Claude <noreply@anthropic.com>
Previously restored from fork main (which had extra port 3114 CORS and
missing iii-exec). Now matches rohitg00/agentmemory upstream exactly.

The runtime config is still generated by entrypoint.sh — this file stays
as-is per upstream.

Co-Authored-By: Claude <noreply@anthropic.com>
Noodle05 and others added 8 commits June 15, 2026 14:19
Co-Authored-By: Claude <noreply@anthropic.com>
Previously iii-engine ran as root (only agentmemory used gosu).
Now both processes drop to node:node, matching the docker-compose
user: 65532:65532 pattern from the original iii-engine container.

Co-Authored-By: Claude <noreply@anthropic.com>
feat: consolidate iii-engine + agentmemory into single Docker image
- Disable ci.yml and publish.yml auto-triggers (kept for reference,
  manual dispatch still available)
- Add docker-build-publish.yml: multi-arch (amd64/arm64) build and
  push to ghcr.io on release or workflow_dispatch

Co-Authored-By: Claude <noreply@anthropic.com>
chore: disable existing workflows, add docker build+publish
package-lock.json is gitignored — generate it with
npm install --package-lock-only before npm ci, matching
the pattern used in ci.yml and publish.yml workflows.

Co-Authored-By: Claude <noreply@anthropic.com>
The runtime stage npm ci must use --legacy-peer-deps to match
the lockfile generated by npm install --package-lock-only in
the build stage. Without it, npm ci rejects the lockfile as
out of sync and reports missing packages.

Co-Authored-By: Claude <noreply@anthropic.com>
The createEmbeddingProvider factory was passing OPENAI_API_KEY
explicitly to the OpenAIEmbeddingProvider constructor, which
short-circuited the constructor's own fallback chain
(OPENAI_EMBEDDING_API_KEY -> OPENAI_API_KEY). When OPENAI_API_KEY
points at a non-OpenAI provider (e.g. DeepSeek) but
OPENAI_EMBEDDING_API_KEY holds a valid OpenAI key, embeddings
failed with 401 because the wrong key was sent to api.openai.com.

Fix: let the constructor resolve the key itself — it already
has the correct priority order.

Co-Authored-By: Claude <noreply@anthropic.com>
@vercel

vercel Bot commented Jun 16, 2026

Copy link
Copy Markdown

@Noodle05 is attempting to deploy a commit to the rohitg00's projects Team on Vercel.

A member of the Team first needs to authorize it.

@Noodle05 Noodle05 closed this Jun 16, 2026
@coderabbitai

coderabbitai Bot commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 6cca7cc9-7759-4376-b16c-52205d3d9309

📥 Commits

Reviewing files that changed from the base of the PR and between f6f9e3c and 558b85a.

📒 Files selected for processing (22)
  • .dockerignore
  • .env.example
  • .github/workflows/ci.yml
  • .github/workflows/docker-build-publish.yml
  • .github/workflows/publish.yml
  • Dockerfile
  • README.md
  • docker-compose.yml
  • entrypoint.sh
  • src/cli/connect/types.ts
  • src/cli/connect/util.ts
  • src/config.ts
  • src/index.ts
  • src/mcp/handler.ts
  • src/mcp/standalone.ts
  • src/mcp/stream-http.ts
  • src/providers/embedding/index.ts
  • src/types.ts
  • src/viewer/server.ts
  • test/mcp-stream-http.test.ts
  • test/mcp-transport-coexistence.test.ts
  • test/multi-instance-port.test.ts

📝 Walkthrough

Walkthrough

Adds a hand-rolled MCP Streamable HTTP server (stream-http.ts) on a configurable mcpPort, extracts shared MCP tool-handling logic from standalone.ts into handler.ts, wires the new server into startup/shutdown in index.ts, and extends AgentMemoryConfig with mcpPort. Also introduces complete Docker deployment infrastructure: multi-stage Dockerfile, entrypoint.sh with HMAC-secret bootstrapping, updated docker-compose.yml, a new GHCR publish workflow, and a README Docker section.

Changes

MCP Streamable HTTP Transport and Docker Deployment

Layer / File(s) Summary
mcpPort config contract and derivation
src/types.ts, src/config.ts
AgentMemoryConfig gains mcpPort: number; loadConfig() derives it from AGENTMEMORY_MCP_PORT or restPort + 3.
Shared MCP tool handler extraction
src/mcp/handler.ts, src/mcp/standalone.ts
handler.ts (442 lines) defines IMPLEMENTED_TOOLS, per-tool argument validation, handleProxy REST dispatch, handleLocal InMemoryKV fallback, handleProxyGeneric, and exported handleToolCall/handleToolsList. standalone.ts is gutted (−422 lines) and now delegates to handler.ts.
MCP Streamable HTTP server
src/mcp/stream-http.ts
New 331-line hand-rolled JSON-RPC-over-HTTP server with bounded body reading (5 MB), BodyTooLargeError, session map with periodic idle eviction, timing-safe Bearer auth, DELETE session support, and routing for initialize/tools/list/tools/call.
Server startup, shutdown, and CLI wiring
src/index.ts, src/cli/connect/types.ts, src/cli/connect/util.ts, src/viewer/server.ts, src/providers/embedding/index.ts
index.ts starts mcpServer on config.mcpPort and awaits mcpServer.shutdown() in teardown. ConnectAdapter.category gains "stream-http". AGENTMEMORY_MCP_HTTP_BLOCK exports the Streamable HTTP client config. Viewer REST upstream host becomes env-configurable; embedding provider drops explicit API key arg.
Docker build, entrypoint, and Compose
Dockerfile, entrypoint.sh, docker-compose.yml, .dockerignore, .env.example
Multi-stage Dockerfile copies iii binary and builds via node:22-slim. entrypoint.sh prepares /data, rewrites iii-config.yaml, generates/persists HMAC secret on first boot, starts iii in background, and runs agentmemory in foreground under tini. docker-compose.yml replaces iii-init/iii-engine with a single agentmemory service with healthcheck. .env.example documents new env vars.
CI/CD workflow changes
.github/workflows/docker-build-publish.yml, .github/workflows/ci.yml, .github/workflows/publish.yml
Adds docker-build-publish.yml for multi-arch GHCR image on release/dispatch. Removes auto-triggers from ci.yml (push/PR) and publish.yml (release); both now require manual dispatch.
MCP stream HTTP and transport coexistence tests
test/mcp-stream-http.test.ts, test/mcp-transport-coexistence.test.ts, test/multi-instance-port.test.ts
937-line test suite covers initialize, tools/list, tools/call, Bearer auth, JSON-RPC errors, session management, notifications, HTTP method/content-type validation, payload size limits, DELETE, and session eviction. Coexistence tests validate identical tool sets between HTTP and direct handler (T016) and notification handling (T017). Port tests extend to assert mcpPort derivation and pinning.
README Docker section
README.md
Adds Docker subsection with Compose commands, HMAC secret extraction, Streamable HTTP MCP client config snippet, stdio shim guidance, and quickstart link.

Sequence Diagram(s)

sequenceDiagram
  participant Client as MCP Client
  participant StreamHTTP as stream-http.ts server
  participant Handler as handler.ts
  participant KV as InMemoryKV / REST proxy

  rect rgba(70, 130, 180, 0.5)
    note over Client, StreamHTTP: initialize
    Client->>StreamHTTP: POST /mcp {method: "initialize"}
    StreamHTTP->>StreamHTTP: create session, set lastUsedAt
    StreamHTTP-->>Client: JSON-RPC result + Mcp-Session-Id
  end

  rect rgba(34, 139, 34, 0.5)
    note over Client, KV: tools/call
    Client->>StreamHTTP: POST /mcp {method: "tools/call", Mcp-Session-Id}
    StreamHTTP->>StreamHTTP: validate session + update lastUsedAt
    StreamHTTP->>Handler: handleToolCall(toolName, args, sdk, kv)
    Handler->>KV: proxy REST or local InMemoryKV
    KV-->>Handler: result
    Handler-->>StreamHTTP: content array
    StreamHTTP-->>Client: JSON-RPC result
  end

  rect rgba(178, 34, 34, 0.5)
    note over Client, StreamHTTP: session deletion
    Client->>StreamHTTP: DELETE /mcp (Mcp-Session-Id)
    StreamHTTP->>StreamHTTP: delete session from map
    StreamHTTP-->>Client: 200
  end
Loading

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~120 minutes

Possibly related PRs

  • rohitg00/agentmemory#503: Changes OpenAIEmbeddingProvider constructor to resolve the API key internally, directly related to this PR's removal of the explicit getEnvVar("OPENAI_API_KEY") argument in createEmbeddingProvider().
  • rohitg00/agentmemory#815: Modifies loadConfig() port-derivation logic and test/multi-instance-port.test.ts, the same files this PR extends to add mcpPort derivation.
  • rohitg00/agentmemory#887: Modifies ConnectAdapter.category in src/cli/connect/types.ts, the same field this PR extends with the "stream-http" value.

Suggested reviewers

  • rohitg00

Poem

🐇 Hop, hop — the stream runs free,
JSON-RPC dancing over HTTP!
Sessions born and secrets grown,
A Docker nest I call my home.
tini guards with gentle care,
While MCP tools float through the air. ✨

✨ 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 and usage tips.

@Noodle05
Noodle05 deleted the fix/embedding-key-resolution branch June 16, 2026 01:55
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