feat: add packages/mex-mcp — MCP server for mex-agent#84
Conversation
Adds packages/mex-mcp with five MCP tools over stdio transport: - mex_check — runs runDriftCheck, returns DriftReport - mex_log — read/write via readEvents/appendEvent - mex_timeline — readEvents with kind + since filters - mex_heartbeat — checkHeartbeat, returns HeartbeatResult - mex_read_file — reads scaffold files relative to .mex/ All tools call the public mex-agent API directly (Option B, per mex-memory#81). mex_sync deferred until structured return shape is settled. Closes mex-memory#81
theDakshJaitly
left a comment
theDakshJaitly
left a comment
There was a problem hiding this comment.
Thanks for this — the shape is right (direct mex-agent imports, stdio, structured JSON) and matches the issue's design. No concerns about intent: I read all nine files, there are no network calls, lifecycle scripts, or anything unexpected. But as-is the package does not compile against the current mex-agent API, and there is one security issue to fix before merge. Details inline. Most of these are the call sites you flagged as guesses in the description, so this is just confirming the real signatures.
| ], | ||
| }; | ||
| } | ||
| const fullPath = join(config.scaffoldRoot, file); |
There was a problem hiding this comment.
Security: path traversal. file is caller-supplied and joined onto scaffoldRoot with no containment check, so file = "../../../../etc/passwd" escapes .mex/ and reads arbitrary host files. Since this is an MCP tool driven by an LLM (which may act on untrusted content) and projectRoot is also caller-controlled, this is effectively arbitrary file read. Please resolve and assert the target stays inside the scaffold, e.g.:
import { resolve, sep } from "node:path";
const base = resolve(config.scaffoldRoot);
const fullPath = resolve(base, file);
if (fullPath !== base && !fullPath.startsWith(base + sep)) {
return { content: [{ type: "text", text: JSON.stringify({ error: "Path escapes scaffold root", file }) }] };
}| ], | ||
| }; | ||
| } | ||
| await appendEvent(config, { kind: kind as EventKind, summary }); |
There was a problem hiding this comment.
This will not compile / behaves wrong. The real signature is appendEvent(config, message: string, opts?: LogOpts): EventEntry — synchronous, and the message is a positional string. Passing { kind, summary } as the message is a type error; at runtime message becomes the object and kind silently defaults to "note" (kind lives in opts). Correct form:
appendEvent(config, summary, { kind });(no await needed — it returns EventEntry, not a Promise.)
| await appendEvent(config, { kind: kind as EventKind, summary }); | ||
| return { content: [{ type: "text", text: JSON.stringify({ ok: true, kind, summary }) }] }; | ||
| } | ||
| const events = await readEvents(config, { limit }); |
There was a problem hiding this comment.
This will not compile. The real signature is readEvents(config): EventEntry[] — one argument, synchronous. Passing { limit } is a TS2554 ("expected 1 argument, got 2") and there is no LogOpts-style limit on this function. Because the arg is ignored, mex_log read would also return the entire unbounded log. Read all events and slice client-side if you want a limit:
const events = readEvents(config).slice(-limit);| }, | ||
| async ({ projectRoot }) => { | ||
| const root = projectRoot ?? process.cwd(); | ||
| const config = await findConfig(root); |
There was a problem hiding this comment.
findConfig is synchronous (findConfig(startDir?): MexConfig) and it throws when no .mex/ scaffold exists — it never returns null. So the if (!config) graceful branch below is dead code, and the no-config case throws uncaught out of the tool handler instead of returning the intended { error: "No mex config found" }. This same pattern is copied into all five tools (log, timeline, heartbeat, read-file). Suggest wrapping the lookup in try/catch and returning the error message as JSON:
let config;
try {
config = findConfig(root);
} catch (e) {
return { content: [{ type: "text", text: JSON.stringify({ error: (e as Error).message, projectRoot: root }) }] };
}| }; | ||
| } | ||
| // over-fetch to allow client-side filtering without multiple round-trips | ||
| let events = await readEvents(config, { limit: limit * 4 }); |
There was a problem hiding this comment.
Same readEvents signature issue as in log.ts — it takes only config and is synchronous, so { limit: limit * 4 } is a compile error and is ignored at runtime.
| events = events.filter((e) => new Date(e.timestamp).getTime() >= sinceMs); | ||
| } | ||
| return { | ||
| content: [{ type: "text", text: JSON.stringify(events.slice(0, limit), null, 2) }], |
There was a problem hiding this comment.
Logic: this returns the oldest events, not the most recent. readEvents returns entries in append order (oldest first), and there is no sort before slice(0, limit), so you get the oldest N — which contradicts the tool's "recent events" description. The CLI's runTimeline sorts descending by timestamp before slicing; mirror that:
events.sort((a, b) => b.timestamp.localeCompare(a.timestamp));
// ...then slice(0, limit)| }, | ||
| "dependencies": { | ||
| "@modelcontextprotocol/sdk": "^1.12.0", | ||
| "mex-agent": "workspace:*", |
There was a problem hiding this comment.
workspace:* will not resolve: the root package.json (which is mex-agent itself) has no workspaces field, and workspace:* is pnpm/yarn syntax while this repo uses npm. As written, npm install in this package fails. This is the open question from your description (items 1 and 4) — a maintainer decision, not a bug. Simplest path that matches the repo: add "workspaces": ["packages/*"] to the root package.json and change this to "mex-agent": "*" (npm workspace protocol).
…pace protocol - findConfig is sync and throws (never returns null) - wrap in try/catch across all five tools instead of dead if (!config) branches - appendEvent/readEvents are sync with (config, message, opts) / (config) signatures, not the (config, opts) shape used here - timeline: sort events descending by timestamp before slicing so "recent events" actually returns the most recent, not the oldest - read-file: resolve+contain file path inside scaffoldRoot to close a path traversal / arbitrary file read via the file argument - workspace:* is pnpm/yarn syntax; this repo uses npm - add workspaces: ["packages/*"] to root package.json and use "*" for the mex-agent dependency Verified: npm install resolves, tsc --noEmit clean, tsup build succeeds.
|
Pushed e953947 addressing every point in the review:
Verified locally: |
theDakshJaitly
left a comment
theDakshJaitly
left a comment
There was a problem hiding this comment.
Thanks for the fast turnaround — this is a big improvement. I cloned the head commit and ran the real toolchain to verify, rather than eyeballing. Confirmed fixed and build-verified:
- Path traversal in mex_read_file — the resolve +
startsWith(base + sep)guard correctly rejects../and absolute paths. - appendEvent / readEvents / findConfig call sites now match the real mex-agent API.
npm run build(tsup + DTS typecheck) passes clean; I sanity-checked it by reverting log.ts to the old calls and tsc immediately flagged TS2345/TS2554, so the green build is meaningful. - Timeline now sorts newest-first before slicing.
Two things remain — details inline. The workspace one is the important one: it installs green but does not actually do what the commit message says.
| }, | ||
| "dependencies": { | ||
| "@modelcontextprotocol/sdk": "^1.12.0", | ||
| "mex-agent": "*", |
There was a problem hiding this comment.
This installs green but does not actually link the local package — so the "npm workspace protocol" fix does not do what it claims. mex-agent is the workspace root, not a packages/* member, so npm cannot satisfy this sibling dependency from the workspace and silently pulls the published mex-agent from the registry instead. npm ls mex-agent after install:
mex-agent@0.6.1 /path/to/repo <- local workspace root
└─┬ mex-mcp@0.1.0 -> ./packages/mex-mcp
└── mex-agent@0.6.2 <- separate copy fetched from npm registry
So mex-mcp builds and runs against the published 0.6.2, not the local 0.6.1 source. It only compiles today because the published API still happens to match; the moment the local mex-agent API changes, mex-mcp keeps building against the stale registry copy until someone republishes — which is exactly the CLI/MCP drift #81 set out to eliminate.
The fix is structural: either move the core library into packages/mex-agent/ so it is a real workspace member that npm will symlink, or reference it explicitly with "mex-agent": "file:../..". Not a runtime blocker right now, but it should not merge advertised as a working workspace link when it is resolving from the registry.
| .optional() | ||
| .describe("Absolute path to the project root. Defaults to cwd."), | ||
| action: z.enum(["read", "write"]).default("read"), | ||
| kind: z |
There was a problem hiding this comment.
Minor: kind is a free z.string(), but on write it flows into appendEvent -> normalizeKind, which throws on any value outside EVENT_KINDS — and that throw is outside the try/catch above, so mex_log({ action: "write", kind: "foo" }) crashes the tool instead of returning a structured error. Since EVENT_KINDS is already imported, restrict the schema:
kind: z.enum(EVENT_KINDS).optional().describe(...)…enum - mex-agent is the workspace root, not a packages/* member, so "mex-agent": "*" silently resolved from the npm registry instead of the local source. Point at it directly with file:../.. so npm symlinks the real local package (verified: node_modules/mex-agent is now a link). - kind was an unvalidated z.string() that flows into appendEvent -> normalizeKind, which throws outside the try/catch on any value outside EVENT_KINDS, crashing the tool instead of returning a structured error. Restrict to z.enum(EVENT_KINDS).
|
Pushed 7d2b0e8 addressing both remaining points:
|
theDakshJaitly
left a comment
theDakshJaitly
left a comment
There was a problem hiding this comment.
Approving. Verified the latest commit by cloning the head and running the real toolchain end to end, not just reading the diff.
All prior findings resolved and build-verified:
- Security — mex_read_file path traversal:
resolve(base, file)+startsWith(base + sep)guard correctly rejects../and absolute paths. - API correctness — appendEvent / readEvents / findConfig call sites match the real mex-agent API; timeline sorts newest-first; findConfig wrapped in try/catch across all five tools.
npm run build(tsup + DTS typecheck) passes clean, and I confirmed the typecheck is meaningful by reverting to the old calls and watching tsc flag TS2345/TS2554. - Workspace link —
file:../..now produces a real local symlink (packages/mex-mcp/node_modules/mex-agent -> ../../..);npm lsshowsmex-agent@0.6.2 -> ./with no separate registry copy, and both typecheck and a runtimeimport('mex-agent')resolve against the local source. The CLI/MCP drift risk is closed. - log kind is now constrained with
z.enum(EVENT_KINDS).
Merge from main is clean and none of the earlier fixes regressed. Nice work iterating on this.
Follow-up (not blocking): mex_sync is still deferred, and a README section documenting the server + client config would be good to add before or shortly after merge.
2 participants
Follows up on #81 and your design guidance.
What this adds
packages/mex-mcp— a TypeScript MCP server over stdio that lets AI agents call mex as native tools.Five tools (v1 scope):
mex_checkrunDriftCheckDriftReport(score, issues, filesChecked)mex_logreadEvents/appendEvent{ ok }mex_timelinereadEvents+ client filtermex_heartbeatcheckHeartbeatHeartbeatResult(ok, staleFiles, memoryCleanupDue)mex_read_filemex_syncis deferred — leaving it for when the structured return shape is clear.Design choices (per your comment)
mex-agentpublic API — no subprocess wrappingmex_read_file)projectRootparam on every tool (defaults tocwd) so agents can target any projectWhat needs discussion / maintainer input
package.jsondoesn't haveworkspacesyet. Should I add"workspaces": ["packages/*"]here, or do you prefer a different approach?readEventsopts — I'm passing{ limit }toreadEventsbased on the exportedLogOptstype; if the signature is different let me know and I'll adjust.appendEventshape — calling asappendEvent(config, { kind, summary }); please correct if the actual signature differs.package.jsonusesworkspace:*(pnpm-style); adjust if you prefer npm workspaces syntax.Happy to co-develop this with you — treat this as a starting scaffold to iterate on.