Skip to content

feat(mcp): add opt-in MCP server via additive @Mcp() decorators#256

Closed
tobiasstrebitzer wants to merge 8 commits into
rmyndharis:mainfrom
tobiasstrebitzer:feat/mcp
Closed

feat(mcp): add opt-in MCP server via additive @Mcp() decorators#256
tobiasstrebitzer wants to merge 8 commits into
rmyndharis:mainfrom
tobiasstrebitzer:feat/mcp

Conversation

@tobiasstrebitzer

Copy link
Copy Markdown
Contributor

Description

This adds an opt-in MCP server so AI agents (Claude, Cursor, etc.) can drive WhatsApp as first-class tools - list sessions, read/send messages, manage groups, webhooks, and more. It is delivered the way the maintainers asked for in the previous discussion: additively, on top of the existing NestJS controllers, without replacing them and without taking a core framework dependency.

Each tool-shaped route opts in with a single @Mcp() decorator. No controller is deleted, no REST route/verb/path changes, DTOs/services/guards/dashboard are untouched, and MCP is off by default (it only loads when MCP_ENABLED=true).

Type of Change

  • New feature

Checklist

  • Documentation updated (README)
  • Lint passes
  • Self-reviewed
  • No new tests (additive metadata only - see Testing)

Addressing risk and technical debt

This PR is designed specifically to resolve shortcomings of previous PR #230:

1. "Core-dependency commitment - this routes the entire public API through @silkweave/*."
I have resolved this, and REST is now served entirely by stock NestJS/Express, exactly as today - @silkweave/* is not on the request path. When MCP_ENABLED is unset (the default), SilkweaveModule is never registered: no /mcp mount, no Silkweave guards, no MCP routes. The dependency is opt-in and reversible (see Lock-in & reversibility). The public API is not coupled to Silkweave at the request-handling layer.

2. "Replacing vs. adding - the PR deletes every *.controller.ts."
With this new PR, nothing is deleted. Every @Controller stays exactly as it is. The change is purely additive: an import { Mcp } and one @Mcp() per exposed route. @Mcp() is a SetMetadata marker - inert at runtime unless MCP is enabled - so there is no "REST byte-for-byte unchanged" burden to verify: the REST routing is the original NestJS code, untouched. Reverting is mechanical (strip the decorators + the env-gated block).

3. "v0.2.0 just landed; the branch predates new endpoints."
This branch is based on current main (v0.2.3), post-v0.2.0. All the newer endpoints - templates, live chats, send-template, chat history, GET /infra/config, the tightened role guards - are present and exposed as MCP tools. No re-migration, no stale conflicts.

4. "MCP delivered as an additive module"
MCP is now delivered as an additive module that calls the existing services without replacing the controllers or taking a core framework dependency. Instead of a parallel layer that re-declares ~100 tools and re-calls services (permanent duplication + drift), it reflects the existing controller methods, so each tool's schema, validation, and @RequireRole stay defined in exactly one place - the controller.

How it works

@silkweave/nestjs ships a @Mcp() decorator and a SilkweaveModule. At boot (only when enabled), the module uses Nest's DiscoveryService to find @Mcp()-decorated routes across all controllers and exposes each as an MCP tool over a Streamable-HTTP transport at /mcp. The tool name, description, and input schema are derived from the decorators the route already has (@ApiOperation, @Param/@Query/@Body, class-validator, @nestjs/swagger).

// The ONLY change to a controller - add the decorator, leave everything else:
@Post('send-text')
@RequireRole(ApiKeyRole.OPERATOR)
@ApiOperation({ summary: 'Send a text message' })
@Mcp()                                   // ← exposes this exact route as an MCP tool
sendText(@Param('sessionId') sessionId: string, @Body() dto: SendTextDto) { ... }

Wiring stays in the existing lazy-require pattern (mirrors QUEUE_ENABLED), so exposing a module needs no app.module.ts or *.module.ts edits:

// app.module.ts - only constructed when MCP_ENABLED=true
SilkweaveModule.forRoot({
  silkweave: { name: 'openwa', description: '…', version: '0.2.3' },
  adapters: [mcp({ basePath: '/mcp' })],
  globalGuards: [ApiKeyGuard],   // run the app's API-key guard on tool calls
})

Auth & session scoping (fully enforced over MCP)

globalGuards: [ApiKeyGuard] runs the existing API-key guard on every tool call (Silkweave's raw routes don't pick up APP_GUARDs automatically, so it's opted in explicitly). Verified:

  • No / invalid key → tool call rejected (UnauthorizedException).
  • @RequireRole(...) enforced - e.g. a viewer key is blocked on operator/admin tools.
  • Per-key allowedSessions scoping enforced - a key scoped to one session can't act on another.
  • The throttler is intentionally excluded (it needs a writable HTTP response MCP doesn't provide).
  • IP-allowlisted keys won't authorize MCP calls (there's no client IP over MCP) - documented.

The login-only POST /api/auth/validate and the Prometheus metrics text endpoint are intentionally not exposed (not tool-shaped).

Lock-in & reversibility

  • Off by default. With MCP_ENABLED unset, there is zero MCP behavior - no /mcp, no Silkweave guards, no tool routes.
  • Not on the REST path. REST requests never traverse Silkweave; the proven NestJS layer is unchanged.
  • Mechanical to remove. Delete the env-gated block in app.module.ts and strip the @Mcp() decorators (a search-and-replace). No API surface, service, or dashboard change is entailed.
  • Honest cost when off: a decorated controller's top-level import { Mcp } loads the package at process start (so the decorator symbol resolves), but it does nothing - no routes, no guards, no transport - unless SilkweaveModule is registered.

What changed (and what didn't)

Changed (additive only)

  • @Mcp() added to 114 tool-shaped routes across the existing controllers; one import { Mcp } per controller.
  • app.module.ts: an opt-in, env-gated SilkweaveModule.forRoot(...) (same shape as the existing QUEUE_ENABLED block).
  • package.json: @silkweave/core + @silkweave/nestjs (^2.2.0) added as dependencies.
  • README.md: a short "MCP" section (enablement, /mcp, auth model, additive/off-by-default).
  • test/mocks/silkweave-nestjs.ts + a one-line jest moduleNameMapper (see Testing).

Deliberately unchanged

  • Controllers - every @Controller and route handler stays; nothing deleted.
  • REST contract - every route, verb, path param, validation, and error shape is the original NestJS code.
  • Services / business logic - zero changes.
  • Dashboard - untouched.
  • Auth & roles - ApiKeyGuard + @RequireRole are the same decorators, now also enforced over MCP.

Testing

Per the additive nature, this PR adds no new tests - @Mcp() is metadata read only when MCP loads. One small harness change was needed: three existing controller specs (health/infra/webhook) import their controllers, which now transitively import the ESM-only @silkweave/nestjs, which the CommonJS jest runner can't load. A single moduleNameMapper entry maps that import to a no-op Mcp stub so those specs load unchanged. Faithful, since @Mcp() is inert in unit tests.

Verification

  • npm run build, npm run lint, npm test (416 passing) - green.
  • Booting with MCP_ENABLED=true: tools/list returns 114 tools across all modules; with MCP unset, no /mcp route exists and REST is unchanged.
  • Auth: unauthenticated tool call rejected; @RequireRole and per-key allowedSessions scoping both enforced over MCP.

Disclosure

I'm the author of Silkweave (docs on Context7), MIT-licensed. In response to the previous review, this PR reduces it from a request-layer framework to a thin, optional, off-by-default add-on that is not on the REST path and is mechanical to remove. I'm actively maintaining Silkweave and happy to own any Silkweave/MCP-related issues. Happy to collaborate further on the tool surface, auth, and session-scoping shape.

References: silkweave.dev · Context7 docs

tobiasstrebitzer and others added 2 commits June 16, 2026 12:13
Expose existing NestJS controllers as Model Context Protocol (MCP) tools so
AI agents can drive WhatsApp as first-class tools — without replacing the
controllers or taking a core framework dependency.

The approach is purely additive: each tool-shaped route opts in with a
single @Mcp() decorator from @silkweave/nestjs. Controllers, REST paths/
verbs, DTOs, services, guards, and the dashboard are all unchanged. MCP is
off by default and only loaded when MCP_ENABLED=true, mirroring the existing
QUEUE_ENABLED lazy-require pattern in app.module.ts.

- 114 routes across the existing controllers decorated with @Mcp() (REST
  untouched).
- app.module.ts: opt-in SilkweaveModule.forRoot() mounts the MCP transport
  at /mcp and runs the global ApiKeyGuard on tool calls via globalGuards.
- Auth fully enforced over MCP: @RequireRole and per-key allowedSessions
  scoping both apply; throttler intentionally excluded.
- Skipped non-tool-shaped routes: Prometheus metrics text and the
  login-only POST /api/auth/validate.
- README: new "MCP" section (enablement, /mcp, auth model, additive nature).
- jest: moduleNameMapper stub for the ESM-only @silkweave/nestjs so the
  three controller specs that import their controllers still load under the
  CommonJS test runner.

Verified: build, lint, and unit tests green; booting with MCP_ENABLED
registers 114 tools; unauthenticated tool calls are rejected.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add an endpoint to delete a chat from the chat list (e.g. a group you
have left), removing it via the WhatsApp client.

Endpoint: POST /api/sessions/:id/chats/delete  { chatId }

- Adds deleteChat(chatId) to IWhatsAppEngine
- Implements it in the whatsapp-web-js adapter via chat.delete()
- Wires it through SessionService and SessionController
- Validates chatId as a WhatsApp JID via DeleteChatDto
- OPERATOR role required (mutation), mirroring markChatRead
- Adds unit tests mirroring sendSeen

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@tobiasstrebitzer

Copy link
Copy Markdown
Contributor Author

@rmyndharis keeping this PR's branch unchanged to keep it simple.

In case you'd prefer using the Plugin system, here's the PR:
tobiasstrebitzer#1

@tobiasstrebitzer

Copy link
Copy Markdown
Contributor Author

Test failed due to @silkweave/nestjs being ESM-only. e2e spec imports AppModule, which transitively loads session.controller.ts and its eager import { Mcp }. Jest's CommonJS runtime can't parse the package's .mjs import statement.

Fix: declare jest moduleNameMapper for @silkweave/nestjs to stub/mock.

Pushing a fix

The e2e suite imports AppModule, which transitively loads controllers
that eagerly import @silkweave/nestjs. That package is ESM-only and the
CommonJS Jest runtime cannot parse it. Map it to the existing no-op stub
(already wired into the unit jest config) so e2e can run.
… routes

Bump @silkweave/core, @silkweave/nestjs and the new optional @silkweave/mcp
peer to ^2.6.0. The mcp adapter moved to a subpath export in 2.5.0, so it is
now required from @silkweave/nestjs/mcp; eslint allows require() for the
ESM-only @silkweave packages.

2.6.0 added boot-time warnings for @Mcp() routes whose whole-body @Body()
param reflects no input fields (intersection/Partial/inline types erase to
Object). Replace those with proper DTO classes so the MCP tools expose real
input schemas:
- SettingsController.update -> UpdateSettingsDto
- InfraController.saveConfig/requestRestart/importData/importStorage

Move the data-migration row interfaces into infra/dto/migration.types.ts so
the controller and ImportDataDto share them without a circular import. REST
behaviour is unchanged.
@rmyndharis

Copy link
Copy Markdown
Owner

Thank you for the substantial work here — the additive @Mcp() reflection approach (single source of truth off the existing controllers, off-by-default) is genuinely elegant, and I appreciate the resubmission.

After review we're going to decline this as-is, on dependency-governance grounds rather than anything about the code quality. It adds @silkweave/* to production dependencies, which pulls in ~37 transitive packages — including a second HTTP framework (hono) and a second logger (pino) — and because every controller imports @Mcp() at module load, the framework loads on every boot regardless of MCP_ENABLED (the flag only gates the route, not the load). For something as load-bearing as the gateway core, we'd prefer to depend directly on the official @modelcontextprotocol/sdk behind a thin in-house adapter (Nest DiscoveryService), so the dependency surface stays auditable and the auth-over-MCP path (ApiKeyGuard + @RequireRole + per-key session scoping) is test-covered.

This isn't a no to MCP — it's a yes to MCP via a leaner integration. If you'd be open to an in-house-SDK variant, or want to take the plugin-system fork direction, I'd be glad to collaborate. Thank you again for the effort.

@rmyndharis rmyndharis closed this Jun 18, 2026
@tobiasstrebitzer

Copy link
Copy Markdown
Contributor Author

Thanks @rmyndharis, appreciate the thoughtful reply.

Happy to build this directly without silkweave as a dependency. Silkweave is built on the official SDK anyway, so I can take the same @Mcp() reflection approach (via Nest's DiscoveryService) and wire it straight onto the SDK. We'd also get the auth path (ApiKeyGuard + @RequireRole + per-key session scoping) test-covered.

Just a heads up on deps: hono and express aren't coming from silkweave, they're direct dependencies of @modelcontextprotocol/sdk itself, so they'll come in either way. The silkweave-specific extras (pino, some CLI helpers) drop out entirely with this approach.

Want me to open a fresh PR on main? Happy to put the adapter wherever fits best.

@rmyndharis

Copy link
Copy Markdown
Owner

Perfect — yes, please open a fresh PR against main. This is the direction we want.

And you're right about the deps — I'll correct the record: hono and express are direct dependencies of @modelcontextprotocol/sdk itself (I just checked 1.29.0), not something silkweave was adding. My decline mis-attributed that, apologies. Going straight to the official SDK keeps the integration lean — pino and the CLI extras drop out, and we're on the standard SDK surface.

A few things that would make this a clean merge:

  1. @Mcp() stays a pure SetMetadata marker (inert, zero runtime dep on the controllers), tools discovered via Nest DiscoveryService — exactly your approach.
  2. Truly off by default — gate the module, not just the route. The MCP module and any SDK transport instantiation should only be registered when MCP_ENABLED=true, so a default boot loads none of the SDK's server code.
  3. Mount on the existing server / single port. We moved to single-port deployment in v0.4.0 — please attach the MCP transport (e.g. StreamableHTTPServerTransport) to the existing Nest/Express instance rather than standing up @hono/node-server on a second port.
  4. Auth is the acceptance bar. Every MCP tool call must go through ApiKeyGuard + @RequireRole, with per-API-key session scoping enforced, and that path test-covered (unit + an e2e that proves an unscoped key can't reach another key's session).
  5. src/modules/mcp/ as a core module is the natural home for the DiscoveryService approach. REST untouched (as you already do). A short README/CHANGELOG note rounds it out.

Happy to collaborate on the auth-scoping design — that's the sensitive part. Thanks again for sticking with this and for the correction.

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.

2 participants