Skip to content

spike: assess adding MCP server support to Cluckwork (feasibility, pros/cons, phasing) #770

New issue
New issue
Closed
#784
#785
Closed
spike: assess adding MCP server support to Cluckwork (feasibility, pros/cons, phasing)#770
#784
#785
Labels
area:apiAPI/endpoint layerenhancementNew feature or request

Description

@mforce
mforce
opened on Sep 11, 2026

Research spike, not a slice. Documenting a feasibility read of adding Model Context Protocol (MCP) support so an AI assistant can drive Cluckwork's operations (record daily entry, list flocks/stock, run reports, etc.) as MCP tools. No code written; this records what an implementation would touch and what it would cost, so the work can be scoped and sliced deliberately.

Bottom line

Moderate. The tool surface is the easy part because the architecture is already the right shape — the hard part is bridging MCP's session to Cluckwork's per-request auth/tenant/actor context without reopening a closed defect. Rough shape: a read-only demo in days, a production-grade multi-tenant read+write server respecting every invariant in ~2–3 weeks, ~80% of it auth/tenancy/idempotency/audit plumbing and guards rather than tool code.

What "MCP support" means here

Most likely reading: expose a curated subset of Cluckwork operations as MCP tools over the official ModelContextProtocol C# SDK, so an MCP client can call them. Two transport shapes, with very different blast radius (see Cons #3).

Why the tool surface is genuinely easy

Cluckwork is layered DDD with a handler-per-feature Application layer invoked directly from thin endpoints — no MediatR (AGENTS.md → Application shape). An MCP tool is the same adapter an endpoint is: a thin wrapper that calls an existing handler + validator + repository. Domain logic is untouched; you add a second entry point beside the minimal-API one.

  • SDK drops into the existing ASP.NET Core host: builder.Services.AddMcpServer().WithHttpTransport().WithTools…() + app.MapMcp("/mcp"), a new group beside the ~20 MapGroup("/api/v1/…") calls in src/Cluckwork.Api/Program.cs:335+.
  • The Result / Result<T> pattern (Domain/Common) maps cleanly to MCP tool results.
  • Microsoft.AspNetCore.OpenApi is already referenced (src/Cluckwork.Api/Cluckwork.Api.csproj), so tool input schemas can lean on existing request DTOs.
  • 107 route registrations exist (grep -c over src/Cluckwork.Api/Endpoints/), but you'd expose a curated handful, not all of them.

Pros

Pro Why it holds
No domain rewrite Tools call the same handlers endpoints already call; the egg-loop state machine and aggregates are untouched.
Validation/Result reuse FluentValidation validators (*Validator) and the Result pattern are reused as-is — no parallel validation path.
HTTP transport adds no scale-out risk An authenticated HTTP endpoint adds no AddHostedService and no in-memory limiter, so it does not touch the single-serving-instance invariant (#271).
RBAC model already exists AuthPolicies (Owner/Manager/Sales/Worker/ReadOnly tiers, AuthPolicies.cs) can gate tools exactly as it gates endpoints via RequireAuthorization.
Read-first is a small, low-risk first slice Read tools over existing read handlers, behind the existing auth pipeline, are demoable in days.

Cons / the real work (each maps to a closed defect the repo has already paid for)

  1. Auth + tenancy bridge — the big one. Every request resolves TenantContext from the JWT account_id claim and it is single-assignment per request (TenantResolutionMiddleware, Program.cs:317; EPIC: Phase 1.6 — Multi-farm tenancy #530). ICurrentUser's roles feed FlockScopeGuard — an authorization input, not a label (fix(seed): seeded audit events carry "(unresolved)" as the actor, now visible on record History columns #500). Cluckwork auth is asymmetric-JWT + rotating refresh + farm-code login + credential-epoch revocation with a fresh DB read per request that must not be cached (Credential epoch: per-request revocation check, deployed inert ahead of the user-admin mutations #364, CredentialEpochMiddleware, Program.cs:322). Every tool call must reconstruct a resolved tenant + real actor. Get it wrong and you reopen the multi-tenancy holes (EPIC: Phase 1.6 — Multi-farm tenancy #530/Write guard trusts OriginalValue as DB provenance; detached Update/Remove can bypass the tenant theft check #562/AccountId must be a non-nullable Guid: the write guard and the #562 token walk are both fail-open for any other shape #673). This is most of the effort.

  2. Writes carry more baggage than reads. Write endpoints require an Idempotency-Key (IdempotencyMiddleware, Program.cs:332); mutations must obey Version++ and the AccountId concurrency token, and nothing writes an audit event without a resolved actor (fix(seed): seeded audit events carry "(unresolved)" as the actor, now visible on record History columns #500). Write tools must mint idempotency keys and run under a real authenticated actor. → scope reads first, writes as a deliberate second phase.

  3. Transport choice drives blast radius.

  4. Repo hygiene tax (small each, unavoidable). New NuGet package → Directory.Packages.props + regenerated packages.lock.json in the same commit (chore: adopt Central Package Management so a version bump is one file, not eight #684), passes the vuln gate (CI: dependency-vulnerability and SAST gates #146, --locked-mode). Host-agnostic: no provider names in code/config/docs. Any user-visible behavior updates specs/product/GLOSSARY.md + SPA Help in the same PR. New behavior needs guards written to the repo's guard bar.

  5. Tool-surface curation is a product decision, not a mechanical one. 107 endpoints ≠ 107 tools. Which operations an AI agent should be able to invoke — and at which RBAC tier — needs deciding, not auto-generating.

Rough effort

Scope Effort
Read-only HTTP MCP, dev-auth, few tools 1–3 days
Production-grade: multi-tenant auth bridge (#364/#530) + curated read tools + guards ~1 week
Add write tools (idempotency, audit actor, concurrency, per-tool RBAC) +~1 week
stdio transport / desktop path (new process role #347, sim harness #370, AppHost #565) +several days

Recommendation

HTTP streamable transport, read-only first, reusing existing Application handlers, with the MCP endpoint behind the same JWT auth + TenantResolutionMiddleware + CredentialEpochMiddleware pipeline the REST API uses. That path adds capability without touching the single-serving-instance (#271), process-role (#347), or write-contract (#394) invariants. Add writes as a separate phase once the auth bridge is proven and guarded.

Decisions to make before any code

  1. Transport: HTTP streamable (recommended) vs stdio (desktop client). Drives blast radius per Con Bump to v4.4: README, CHANGELOG, and spec version line #3.
  2. Auth model: how an MCP session authenticates — reuse the existing JWT bearer (client passes a Cluckwork token) vs an MCP-native OAuth 2.1 flow bridged to Cluckwork identity. Reusing the JWT bearer is the smaller first step.
  3. Tool surface + RBAC: which operations are exposed and at which AuthPolicies tier.
  4. Read-only vs read+write for phase 1.
  5. Version compatibility: pin the ModelContextProtocol SDK against .NET 10 before committing (not yet verified in this spike).

Repo rules this touches

Activity

  1. mforce
    added
    enhancementNew feature or request
    area:apiAPI/endpoint layer
    on Sep 11, 2026

  2. mforce commented on Sep 12, 2026

    @mforce
    mforce
    on Sep 12, 2026
    OwnerAuthor

    Design landed in #785 (docs/plans/770-mcp-server/). No code written, per this issue's framing.

    The spike's headline read holds: the tool surface is genuinely the easy part, the architecture is the right shape, and the auth/tenant bridge is most of the work. Four of its specific premises did not survive verification, though, and two of them change the phasing. Recording them here so the issue and the design do not disagree.

    1. The DI-scope hazard is real, but the setting worth guarding is not the obvious one. McpServerOptions.ScopeRequests does default to true, so a fresh scope per tool call is the documented behaviour — but under SessionMode.Stateless the SDK overrides it per request:

    // StreamableHttpHandler.CreateSessionAsync, v2.2.0 L554-558
if (serveStatelessly)
{
    mcpServerServices = context.RequestServices;
    mcpServerOptions.ScopeRequests = false;
}

    So setting ScopeRequests = false ourselves is redundant, and a guard on it is a false alarm — its mutation reddens the test while the product stays safe. The real regression is a later change to SessionMode; under StatefulForInitializeClients the scope is per-request mixed, which is the worst shape to debug.

    2. The idempotency problem is bigger than key collision, and it blocks the read-only demo. This issue frames writes as carrying the idempotency baggage and recommends scoping reads first. But IdempotencyMiddleware triggers on all of POST/PUT/PATCH/DELETE, /mcp is not exempt, and an authenticated MCP call resolves a tenant — so with no Idempotency-Key header it returns 400 on every MCP request, initialize and tools/list included. MCP does not function at all until /mcp is exempted. The "read-only HTTP MCP, dev-auth, few tools — 1–3 days" row needs the exemption in it, and the exemption means touching #307's middleware.

    3. #271 is touched — inertly, but the walk that concluded otherwise is unsound. The Pros table says HTTP transport "adds no AddHostedService and no in-memory limiter". WithHttpTransport in fact registers three singletons and IdleTrackingBackgroundService as a hosted service, unconditionally. The conclusion survives — StartAsync returns early under Stateless, so it is registered but inert, and the blocker list is not extended — but AGENTS.md's "walk every AddSingleton/AddHostedService under src/" cannot see a package registration by construction. Filed as #786.

    4. Open decision 5 is answered. ModelContextProtocol.AspNetCore 2.2.0 ships a native lib/net10.0 target. Compatible.

    Also worth knowing before anyone picks this up: all three independent design candidates set PerSessionExecutionContext, which is [Obsolete] with a DiagnosticId at 2.2.0 — with TreatWarningsAsErrors=true that is a build error, and since it is only read in the stateful branch the fix is deletion, not suppression.

    Decisions taken

    • Transport: HTTP streamable, per this issue's recommendation.
    • Tool surface: flock / stock / daily-entry reads, customer / order / payment reads, and one write — record daily entry. Owner's call. The write is what makes this the full bridge rather than the read-only first slice recommended above, so the "+~1 week" write row is in scope from the start.

    Decisions still open

    For whoever implements this

    The largest risk is not the tool code. It is extracting #307's claim/execute/publish protocol out of IdempotencyMiddleware so /mcp can key idempotency by tool name instead of URL path. AtomicIdempotencyProtocolTests and the other #307 suites must pass unedited — an edit to one of them is a stop-and-review, not a fix. docs/plans/770-mcp-server/02-guards.md has the 28 guards with their red mutations, including three traps the candidates fell into.

  3. mforce
    added 2 commits that reference this issue on Sep 12, 2026

    docs(agents): walk into package registrations when re-deriving #271 (#…

    5fab974

    docs(mcp): record the MCP server design and what the spike got wrong

    8778df3
  4. mforce
    closed this as completedin #785on Sep 12, 2026
  5. mforce
    added a commit that references this issue on Sep 12, 2026

    chore(main): release 0.1.1 (#784)

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    area:apiAPI/endpoint layerenhancementNew feature or request

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions