Skip to content

EPIC: MCP server support #789

Closed
Closed

Description

@mforce

Important

Amended 2026-10-06. This body predates #514's module contracts. Since it was written, #787 closed, #804 closed with its work split into #806 and #809, and #793 and #794 moved to the OAuth milestone. Read the re-evaluation before picking up a slice, then the re-check after milestone 10. OAuth (#788) has shipped. The body below is kept for history.

Note

Picking this up? Start here.

Blocked on #788 for most of it. Slices 1-2 can start now. Slices 3-6 cannot, because no client can
reach a tool until OAuth exists. Slice 6 also needs #787.

The design is finished and merged. Read
docs/plans/770-mcp-server/
in full before writing anything, especially 02-guards.md and 05-adversarial-review.md. The
design went through three rounds of adversarial review. They found six guards that could not go red
on their stated mutations, three of them in fixes for earlier rounds. The corrections are the
valuable part.

Two things will bite immediately if skipped:

  1. IdempotencyMiddleware 400s every MCP request, initialize included, because MCP sends
    everything as POST and no client knows this app's Idempotency-Key contract. Nothing works
    until /mcp is exempted. That is slice 1, and it blocks even a read-only demo.
  2. BodyReadingEndpointTests starts applying the moment /mcp is mapped. It needs
    ReadsRequestBodyAttribute or the suite goes red.

Do not re-derive the guard table from scratch. Six of its rows exist because a mutation was
stated, run, and found not to go red. Rewriting them from first principles loses that.


Expose a curated Cluckwork tool surface over the Model Context Protocol, so an MCP client can drive farm operations under the same account, role and flock-scope model a human uses.

Design: docs/plans/770-mcp-server/. #785 landed it and closed spike #770. Read 01-design.md before picking up any slice. 02-guards.md has the 28 guards with their red mutations.

Blocked on

Scope decisions already taken

What #788 changed about this epic

The identity bridge design in docs/plans/770-mcp-server/ still holds unchanged. McpCallContext reads the resolved ClaimsPrincipal and the scoped tenant, actor and flock-scope services. It does not care whether the caller presented a hand-rolled JWT or an OAuth reference token. The change is upstream of it. The pipeline must authenticate an OAuth token. The checks CredentialEpochMiddleware performs for JWTs (disabled user, suspended farm, must-change-password) must also run for OAuth callers, since that middleware sits on the JWT path.

Scopes are new: role ∩ scope, where scope can only subtract. There are two at launch, read farm data and record daily entries. They map onto the 6 read tools and 1 write tool below.

Slices

Ordered by dependency. Three L, four M, one S.

Wave 1: startable now. Neither slice depends on #788. One is pipeline work, the other is the identity
bridge, and neither cares how a caller authenticated.

Wave 2: needs OAuth

Wave 3: the tools, reads in parallel

Wave 4: closes the surface

Wave 5: audit follow-ups (2026-10-11)

Two read-only audits of the shipped surface found no tenant or authorization bypass. Fixes in #1225 (forged-bearer budget, Origin check, tool contracts). Remaining:

Deliberately not filed (edge cases, recorded here so they are not lost):

  • The write tool's idempotency key is per user, not per connected app. A collision needs one person's two apps to pick the same key; it refuses or replays and never double-writes.
  • Protected-resource metadata returns the SPA's HTML with 200 on a wrong Host, or when the proxy is outside RateLimiting:TrustedProxies. Only a misconfigured deployment hits it.
  • 401 challenges omit error="invalid_token" except on the switch-off path. MCP clients rediscover on any 401.
  • Malformed JSON gets -32600 instead of -32700: SDK bug, reported upstream as Streamable HTTP returns -32600 instead of -32700 for malformed JSON modelcontextprotocol/csharp-sdk#1918.

Packaging

ModelContextProtocol.AspNetCore 2.2.0 ships a native net10.0 target (verified). The new package goes in Directory.Packages.props with a bare PackageReference. Commit the regenerated packages.lock.json files in the same commit, or CI fails NU1004 (#684 / #146).

Decide before the tool schemas ship

  • Decide how farm-controlled free text is presented to a model before the MCP tool surface exists #792, how farm-controlled free text is presented to a model. It is not a hard blocker on any one slice. But the mitigations are shape decisions (structured fields vs prose, whether free-text is returned by default, delimiting, tool ReadOnly annotations), and shape is expensive to change once clients depend on a published schema. Note the privilege gradient: Customer.Note is writable at SalesFlow (which includes a plain Worker) and readable through tools gated at AdminOnly. Decide during slices 4-5, not after.

Related, not blocking

Activity

  1. mforce commented on Sep 12, 2026

    @mforce
    OwnerAuthor

    Slice 1 changed shape before anyone started it. An adversarial review of the design (recorded in 05-adversarial-review.md, landed on #785) found a cheaper alternative the design never considered. It also found a gap in the port the design specified.

    Price the loopback shape before extracting #307. Instead of extracting the claim/execute/publish protocol into a port, the write tool can call the existing HTTP endpoint over loopback with the caller's bearer, a stable command JSON body and a tool-namespaced Idempotency-Key. That leaves #307 entirely untouched. The claim protocol, its /me user-scoping special case, its #269 single-attempt execution and its #345 re-entry skip all keep running as they do today, already guarded. The cost is one extra HTTP round trip per write, and a second place a bearer is presented.

    Slice 1 calls the extraction the largest risk in the epic and asks for an adversarial pass on the diff. The cheaper shape deserves pricing first. Extract #307 only if loopback loses on evidence.

    If the extraction is chosen anyway, the port needs an outcome contract. Commit eligibility today is an HTTP 2xx. Failure rolls back and releases the claim. An MCP tool call has no HTTP status, and the SDK converts exceptions into CallToolResult.IsError inside its own dispatch, so /error never sees them. A port that publishes on "the delegate returned normally" caches domain failures permanently. Record against a depleted flock, get FlockNotActive, cache it, and every identical retry replays that failure even after a manager reactivates the flock. The existing #307 suites cannot catch this, because they exercise a path that has an HTTP status.

    Three other slices gained requirements. All are now in the design and guards:

    • Slice 2: the identity bridge must constrain the dependency graph, not the tool's parameter list. A helper opening a second scope via IServiceScopeFactory defeats the parameter check, and the new scope's FlockScope is unresolved and therefore unrestricted.
    • Slice 3: /mcp needs an explicit body cap (WithMaxRequestBodyBytes) and a rate-limit policy on the shared IFixedWindowCounter. ReadsRequestBodyAttribute does not activate the Bound authentication request bodies and credential inputs before password hashing #309 cap. It only classifies binding errors. Security: rate limiting on anonymous auth endpoints (login/refresh) #143 is opt-in, and /mcp is one route carrying every operation.
    • Slices 4-6: bounded results with explicit completeness and continuation fields, and an ambiguity contract for name resolution, because duplicate flock names are legal.

    No new blockers. #788 remains the only one.

  2. mforce commented on Sep 12, 2026

    @mforce
    OwnerAuthor

    Prerequisites clarified. Both #787 and #788 are prerequisites, but at different scopes, and the difference is load-bearing.

    #788 blocks the whole epic

    Unchanged. MCP clients expect OAuth discovery and a refreshable token, and Cluckwork's refresh is an HttpOnly cookie. It does not change the design, but it decides whether anyone can practically connect. Building slices 1-8 before answering it risks shipping a surface nobody can reach.

    #787 blocks slice 6 (the write tool), not the epic

    RecordDailyEntry consults FlockScopeGuard's fail-open branch (if (!user.IsResolved) return Result.Success()). The read path does not consult it. Reads go through the FlockScope EF query filter instead. So the guard gates the write slice specifically.

    Why it is a genuine prerequisite for that slice rather than defence-in-depth theatre. The design closes the branch from MCP by construction, because McpCallContext throws before any tool body runs. But the adversarial review (finding 1, 05-adversarial-review.md) showed that guarantee has a hole. A tool injecting a helper that opens a second scope via IServiceScopeFactory leaves CurrentUserContext unresolved in that scope, which lands precisely on the fail-open line. Guard 7b is the primary fix. #787 is the backstop that makes the failure closed rather than silent if 7b is ever weakened or circumvented. A write path protected by exactly one mechanism is thin, given what that mechanism guards.

    Why it must not block slices 1-5. #787 flips an authorization default that affects both seeders, four one-shot CLI verbs and every non-HTTP caller. The guard's own comment says that change "deserves its own issue rather than a drive-by". Blocking the read slices on it would make MCP reads wait on unrelated seeder work for no safety gain, because the reads do not touch that branch.

    Revised blocking list

    Slice 6 is amended to read: blocked on #787; requires guard 7b (dependency-graph walk plus the causal second-scope test) as the primary control, with #787's fail-closed default as the backstop.

  3. mforce commented on Sep 12, 2026

    @mforce
    OwnerAuthor

    Epic re-sequenced after #788's decision. The blocker did not go away. It turned into a build.

    #788 asked whether MCP clients could authenticate at all. The answer is now: build an OAuth 2.1 authorization server with OpenIddict. That is a substantial piece of work in its own right (4 tables, a migration, a consent screen, two management screens, audit columns), so it is tracked there rather than in this epic.

    What that means for the ordering here

    What #788 adds to this epic's own work

    Three things land here rather than in #788, because they are MCP-shaped:

    1. The MCP SDK's McpAuthenticationHandler serves protected-resource metadata so assistants discover where to authenticate. That is the piece that makes a client connect by itself instead of being hand-configured.
    2. Scope enforcement on the tool surface. There are two scopes at launch, read farm data and record daily entries. AddAuthorizationFilters() already honours [Authorize] on tools and filters tools/list, so a read-only connection will not even see the write tool, rather than seeing it and being refused. That property comes free and is worth keeping.
    3. The checks CredentialEpochMiddleware does for JWTs must be re-done for OAuth callers. These are disabled user, suspended farm and must-change-password. That middleware sits on the JWT path, and an OAuth request bypasses it. Build an OAuth 2.1 authorization server (OpenIddict) so MCP clients can authenticate #788 records this. The wiring shows up in this epic's pipeline slice.

    What did NOT change

    The design in docs/plans/770-mcp-server/ stands as written. McpCallContext reads the resolved principal and scoped services, so it is indifferent to the authentication mechanism. The three adversarial review rounds, the guard table and the idempotency analysis all remain valid. None of them turned on JWTs specifically.

    Related issues updated in the same pass

  4. added
    epicPhase-level tracking issue
    epic-789MCP server support (#789)
    priority:tier3Real product weight, real cost
    and removed
    enhancementNew feature or request
    on Sep 13, 2026
  5. mforce commented on Sep 13, 2026

    @mforce
    OwnerAuthor

    Organizing note: neither this epic nor #788 is attached to a phase epic, and that is deliberate rather than an oversight.

    AGENTS.md says slice issues go on the phase epic's checklist, and the current phase is #15, Phase 1.5, "Egg product hardening". Its scope is legacy import, inventory reconciliation, alert centre, packaging inventory, additives/supplements, vaccination records, native-speaker es/tl review, deployment readiness, and Phase 1.1 carryover.

    MCP server support and an OAuth authorization server are none of those. Attaching them to #15 would make the phase epic mean something other than what it says. Phase epics are how work is navigated here, so a wrong association is worse than none.

    This needs a roadmap decision, not a labelling one. There are three plausible answers:

    1. Add them to Phase 1.5 anyway, treating it as the catch-all current phase. This is simple, but it dilutes what the phase means.
    2. A new phase. This is new capability rather than hardening of the egg product, which is what the numbered phases have marked.
    3. Leave both as free-standing epics, navigated by their own epic-788/epic-789 labels. That is the current state and it works. It just means they do not appear when someone walks the phase structure.

    I flagged this so whoever owns the roadmap decides, rather than an agent picking one by default.

  6. added this to the MCP server support milestone on Sep 13, 2026
  7. mforce commented on Sep 14, 2026

    @mforce
    OwnerAuthor

    Cross-epic note from #514's re-plan, 2026-09-14

    #514 (modular monolith) was re-planned on 2026-09-14. Two things touch this epic, and one of them is
    worth reading before #809 is picked up.

    1. A false premise in the design, affecting #809

    The design says the tool layer holds no validation, authorization or scope policy because "each
    already exists one layer down and is already guarded" (770-mcp-server/01-design.md:209).

    For flock name resolution that is false. IFlockRepository has no name lookup at all. The
    only name-to-flock resolution in src/ is SimulationDataSeeder.cs:796's
    FirstOrDefaultAsync(f => f.Name == name). That is the take-first-match behaviour guard 32 forbids and
    adversarial review finding 3 identified. So #809 must invent the ambiguity contract, and the design's
    own rule says it must not live in Mcp/.

    Grade names are fine. IX_EggGrades_AccountId_FarmId_LowerName is a unique index on lower(Name),
    so ambiguity cannot arise there. Flock names are explicitly not unique. Full detail and a
    suggested placement are on #809.

    Consider amending 01-design.md:112 to read "inject a feature contract where one exists, else
    a repository", so #807's author meets the rule before writing against IFlockRepository.

    2. An ordering request on #806, not a block

    #514 wants two size:S slices (1-2 days, no production code) to land before #806: a module ledger,
    and an adapter-tier row declaring Mcp/ a DirectRepository tier. Without them, #514's later adapter
    guard is red on this epic's merged code the day it is written, and the cheap fix would be to weaken
    the guard. Notes are on #806 and #807.

    Where #514 defers to this epic, for symmetry

    #514 is priority:tier4 with four workstreams ahead of it and is not asking this epic to wait.
    Nothing here is closed or reassigned.

  8. mforce commented on Oct 6, 2026

    @mforce
    OwnerAuthor

    Re-evaluation, 2026-10-06: what #514 and the merges since 2026-09-14 change

    The slices were written on 2026-09-13. Since then #514 (milestone "Modular monolith") closed, the code moved to package-by-module (#1089 to #1121), #787 closed and #852 shipped. I checked every code reference in docs/plans/770-mcp-server/ and in the slice bodies against main at 8cb14e67. The design still holds. These parts of it do not.

    Changed facts

    1. Tools can reach only module contracts. CW1004 has been a build error since feat(arch): make CW1004 a build error #1123. It rejects an adapter that names another module's type outside that module's Contracts namespace. AdapterReachRealTreeTests (ContractBypasses) also covers tier namespaces (docs/decisions/849-module-contract.md). A tool therefore calls I*Module and the lookup ports. It never injects a repository, handler or validator class. The plan's WaterTools(McpCallContext, IWaterUsageRepository) example fails twice; its replacement is IInventoryModule.ListWaterUsageAsync.

    2. The DirectRepository privilege on the Cluckwork.Api.Mcp tier row does nothing. No scanner reads it as an exemption. The row is now an [AdapterTier] attribute in src/Cluckwork.Domain/Common/Architecture/Modules/Platform.cs; module-ledger.json no longer exists. Its reason text ("MCP tool classes inject repositories by design") is wrong and belongs to MCP slice 3: map /mcp, wire RBAC and tools/list filtering #806 to fix.

    3. The contracts already cover nearly every planned tool:

      • flocks: IFlockModule.SearchAsync, IFlockLookup, GetBirdsRemovedAsync;
      • stock, lots and daily entries: IEggOperationsModule.GetStockByGradeAsync, ListLotsAsync, ListDailyEntriesAsync, GetDailyEntryAsync;
      • customers, orders and payments: ICommerceModule;
      • the write: IEggOperationsModule.RecordDailyEntryAsync with IValidator<RecordDailyEntryCommand>.

      There are two gaps, both in MCP slice 5: read tools — customers, orders, balances, with money rendered at the boundary #808. ICommerceModule.ListCustomerBalancesAsync has no paging and materializes every customer group (PaymentRepository.cs:43-63). PaymentDetails and CustomerBalance return raw minor units.

    4. FlockScopeGuard's fail-open branch grants account-wide flock access to an unresolved actor #787 is closed. FlockScopeGuard now refuses an unresolved actor before reading assignments, so MCP slice 6: write tool — record daily entry #809 no longer waits on it. The read gap the design records is unchanged: an unresolved FlockScope is still unrestricted (FlockScope.cs:22). McpCallContext's UserId != Guid.Empty check is still needed, because ResolveSystemActor still produces a resolved actor with no id and no roles.

    5. Flock name resolution exists. IFlockLookup.ResolveByNameAsync returns found, not found, or ambiguous with candidates ([C] #514 slice 10: Flock Management contract, eligibility and mortality ports #852). MCP does not need to invent the ambiguity contract.

    6. IFixedWindowCounter is internal to Infrastructure. The only public policy over it, DistributedIpFixedWindowPolicy, keys on IP. The /mcp rate limit needs a new public policy keyed on account or user.

    7. SDK version. The plan verified SDK internals against ModelContextProtocol.AspNetCore 2.2.0. The repo currently resolves only 1.3/1.4, transitively through Aspire in the AppHost. Re-check the plan's SDK facts against whatever version MCP slice 3: map /mcp, wire RBAC and tools/list filtering #806 adds.

    8. Unchanged: IdempotencyMiddleware still returns 400 on an /mcp initialize. No exemption or extracted protocol exists. Build an OAuth 2.1 authorization server (OpenIddict) so MCP clients can authenticate #788 still blocks MCP slice 3: map /mcp, wire RBAC and tools/list filtering #806 to MCP slice 6: write tool — record daily entry #809. HouseId still has no House model, and FlockDetails supplies it. Every cited path moved to Application/Modules/<Owner>/ or Api/Modules/<Owner>/; behaviour claims about tenant, scope, auth policies, body cap and page caps are still true.

    Changes to the slices

    Order

    1. Decide how farm-controlled free text is presented to a model before the MCP tool surface exists #792 can start now.
    2. MCP slice 2: the identity bridge (McpCallContext) and the dependency-graph walk #805 can start now.
    3. MCP slice 3: map /mcp, wire RBAC and tools/list filtering #806 to MCP slice 6: write tool — record daily entry #809 wait for Build an OAuth 2.1 authorization server (OpenIddict) so MCP clients can authenticate #788.
    4. MCP slice 7: [MirrorsRoute] parity guard — tool RBAC cannot drift from its route #810 and MCP slice 8: GLOSSARY, SPA Help and in-app glossary, in all three locales #811 close the milestone.

    Milestone 10 is being re-scoped in parallel, so #806's OAuth-facing items (protected-resource metadata, scope enforcement) may change with it.

  9. mforce commented on Oct 10, 2026

    @mforce
    OwnerAuthor

    Amendment: milestone 10 has shipped. This updates the OAuth dependencies in the October 6 re-evaluation. Source links pin main at bd4e5eca.

    The OAuth prerequisite is met. MCP still needs its endpoint and identity bridge, protected-resource discovery, tool execution and tools/list scope enforcement, idempotency, parity guards and MCP-specific documentation. The protocol-version decision also remains. #806 owns the endpoint integration; #809 must resolve the loopback authentication constraint; #811 owns the remaining documentation.

    Milestone 10 deliberately maps no /mcp and makes no ordinary REST route accept OAuth tokens. It provides authorization-server discovery, not MCP protected-resource discovery. The old "refreshable token" premise is replaced by tokens that last until revoked; no refresh-token grant exists. #806 must decide whether an additional account/user rate ceiling is needed. These limits do not complete the MCP acceptance criteria.

  10. 10 remaining items

  11. mforce commented on Oct 12, 2026

    @mforce
    OwnerAuthor

    Known P3 follow-ups (not filed)

    We found these during the MCP work. In each case the product still behaves correctly for users and nothing exposes data or grants access, so none has its own issue. If one starts to matter, file it from here.

    1. rpc-429-as-problem mutation anchor is broken. Its anchor in tools/oauth/mutation-check.sh matches nothing on main, so that mutant never runs. A regression in how that 429 response is shaped would go unnoticed.
    2. An aborted MCP POST is logged as a 500 at Error. When a client disconnects mid-request, the server logs a TaskCanceledException as a 500. This is log noise that can raise false alerts. It is the open question left on test(api): ignore late logs of earlier requests in McpToolErrorTests #1249.
    3. Dark-mode hover border on .glossary-jump a has low contrast (web/src/styles.css:2591). The link text stays readable.
    4. The REST sales-order list echoes an unknown status value (SaleEndpoints.cs:255). The value goes back only to the caller as JSON. The MCP tool stopped echoing it in fix(mcp): match flock names ignoring case, plain optional schema types, quiet idempotent replays #1237; REST could do the same for consistency.
    5. Farm suspension leaves app tokens valid in the database. They cannot be used: every request checks the farm status, and after reactivation the moved credential epoch refuses them. fix(api): revoke a connected app when the credential epoch supersedes it #1248 revokes each one when it is presented. Only database cleanup is missing.
    6. The live MCP test client has only some roles. Adding Manager, Sales and ReadOnly users to tools/verify/mcp/client.ts would let role checks run live. Integration tests already cover these roles.
  12. mforce commented on Oct 12, 2026

    @mforce
    OwnerAuthor

    MCP audit round 2 (main bae7db2f)

    Two auditors (Codex Astra and Fable) re-audited MCP after #1225, #1237, #1238, #1248 and #1249. They found no cross-tenant access, authorization bypass or incorrect write.

    P3 notes, not filed:

    • R2-02. An app whose token was issued across an epoch bump stays listed as connected. Its token is revoked on first use, but the approval stays Valid until the person disconnects the app.
    • R2-04. tools/oauth/mutation-check.sh has no mutants for the round-1 fixes: Origin check, failure budget, in-flight cap, bad-argument check, ILIKE lookup, stock tie-break and the ON CONFLICT claim.
  13. mforce commented on Oct 12, 2026

    @mforce
    OwnerAuthor

    All MCP slices, the two audit rounds and their fixes have merged (latest: #1260). Open P3 notes are in the comments above; upstream SDK reports are modelcontextprotocol/csharp-sdk#1918 and #1919.

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 layerepicPhase-level tracking issueepic-789MCP server support (#789)priority:tier3Real product weight, real cost

    Projects

    No projects

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions