Files
novelly/docs/plans/api/mcp_http_merge_plan.md
James Wampler 897fb442a1 Unify MCP and agent tool surfaces onto one registry in NovelAgentToolset
Fixes NovelAgentService continuing a conversation under the wrong
novel's route, since FindConversationAsync matched by id alone. Then
extends NovelAgentToolset to all 45 tools the stdio MCP server offered
(tag/location CRUD, character relationships, arc-stage beat pinning,
question editing, cross-novel novel listing/creation), tagging each
with whether it needs an explicit novel scope so a later MCP adapter
can inject it. Renames the toolset's 33 existing schemas from
snake_case to camelCase to match .NET/REST convention, since nothing
external consumes them.

Lays the groundwork to serve this same registry over MCP at /mcp and
retire the separate stdio Novelly.Mcp project (docs/plans/api/mcp_http_merge_plan.md).
2026-08-21 10:52:50 -07:00

18 KiB

Merge the MCP server into Novelly.Api as Streamable HTTP /mcp

Context

Novelly has two duplicate tool surfaces that must be kept in sync by hand:

  • src/Novelly.Mcp/ — 45 tools as [McpServerTool] static methods, stdio-only, each calling the REST API back over HTTP via NovelApiClient. Built by no CI job, in no container, unknown to the AppHost. It only works if someone remembers to run scripts/publish-mcp.sh and re-point .mcp.json at the published binary. Zero tests.
  • src/Novelly.Api/Agent/NovelAgentToolset.cs — 33 tools for the embedded web agent, calling application services directly in-process.

The 33 are a strict subset of the 45, matching name-for-name. The gap is pure capability loss for the web agent, not a design distinction.

Two problems follow. The MCP server is unreachable from anything but a local stdio subprocess, so the QA deploy can't serve it without shipping a binary around. And every new capability has to be written twice, in two idioms, with nothing enforcing that they agree.

Outcome: one tool registry, called directly by both surfaces. The API serves MCP over Streamable HTTP at /mcp, so any MCP client reaches it over the network with an API key and no binary to distribute. The stdio project is deleted. The web agent gains all 12 tools it was missing.

Decisions taken

  • Delete src/Novelly.Mcp entirely. No stdio proxy is kept.
  • The web agent gets all 45 tools, including cross-novel list_novels and create_novel. Full parity, one list, no filtering.
  • camelCase argument names everywhere. MCP specifies nothing about argument naming — inputSchema is plain JSON Schema — so this is a free choice, and camelCase matches .NET convention, the REST API's JsonSerializerDefaults.Web output, and the current MCP surface. Existing MCP clients keep working unchanged. The cost lands on NovelAgentToolset's 33 hand-built schemas, which are today the only snake_case thing in the repo and must be rewritten.
  • Tool names stay snake_case (list_novels, get_novel_brief) — that part is genuine MCP convention, and both surfaces already agree on it.

Design

A single registry — the existing AgentTool shape in NovelAgentToolset, extended to all 45 tools — with two thin adapters over it.

MCP adapter. Register the SDK's dynamic-tools handlers rather than 45 attribute-decorated methods:

services.AddMcpServer(o => o.ServerInfo = new Implementation { Name = "novelly", Version = "1.0.0" })
    .WithHttpTransport()
    .WithListToolsHandler((request, ct) => ...)
    .WithCallToolHandler((request, ct) => ...);

Both handlers resolve request.Services!.GetRequiredService<NovelAgentToolset>() per request and delegate to a pure adapter class. This avoids fighting the SDK's schema inference (McpServerToolCreateOptions has no InputSchema property) and avoids hoisting the scoped, nine-dependency toolset into a static catalog, which WithTools(IEnumerable<McpServerTool>) would force.

Agent adapter. Unchanged — NovelAgentService.SendMessageAsynctoolset.ExecuteAsync(name, novelId, input, ct), with novelId still ambient from the route and never shown to the model.

Novel scoping, one entry / two shapes. AgentTool and AgentToolDefinition each gain a trailing bool RequiresNovelId = false, so the 33 existing construction sites keep compiling. The MCP adapter injects a required novelId property into the advertised schema for those tools and extracts it at call time; the agent adapter supplies it from the route. list_novels / create_novel need no scope at all.

The 11 currently novel-scoped tools are identifiable mechanically — grep -n 'async (novelId' src/Novelly.Api/Agent/NovelAgentToolset.cs: get_novel_brief, update_novel_brief, list_characters, create_character, list_tags, list_locations, list_chapters, create_chapter, get_character_beats, list_open_questions, raise_open_question. Of the 12 new tools, create_tag and create_location are novel-scoped; the other ten are child-id-scoped or unscoped.

Verified before planning

The design rests on SDK behaviour that build-and-test would not catch, so it was checked against a running probe app rather than inferred from docs:

  • WithListToolsHandler / WithCallToolHandler exist in ModelContextProtocol 2.1.0; Tool.InputSchema is a settable JsonElement whose setter validates exactly what JsonSchemaBuilder.Build() already emits.
  • capabilities.tools is advertised on initialize with handlers and no ToolCollection — this was the main open risk and it is closed.
  • request.Services is non-null and yields a fresh DI scope per request (three calls returned three distinct scope ids). This is what makes the scoped NovelDbContext and NovelAgentToolset correct here. HttpServerTransportOptions.Stateless defaults to true and PerSessionExecutionContext to false in 2.1.0, so no options need restating.
  • A hand-built JsonElement schema survives verbatim onto tools/list output, and IsError maps to result.isError rather than a JSON-RPC error object.
  • GET /mcp returns 405 in stateless mode. Harmless; clients only POST.

Chunks

Each builds, tests and commits independently.

Chunk 0 — Fix the cross-novel conversation leak

Independent; do it first to keep it out of the main diff.

NovelAgentService.FindConversationAsync(Guid conversationId, ...) matches on id alone, so a conversation belonging to novel X can be continued under novel Y's route, after which every tool call runs against Y with X's transcript. Add an optional novel filter, passed from SendMessageAsync only — GetConversationAsync/DeleteConversationAsync are reached via /api/conversations/{id}, which has no novel in the route, and keep passing null. Log the miss at Warning with {ConversationId}/{NovelId}.

Files: src/Novelly.Api/Agent/NovelAgentService.cs, tests/Novelly.Api.Tests/NovelAgentServiceTests.cs

Tests: Continuing_a_conversation_under_a_different_novel_is_rejected, plus Continuing_a_conversation_under_its_own_novel_still_works as the guard against over-tightening.

Chunk 1 — One registry, all 45 tools (no MCP wiring yet)

Delivers the parity decision on its own, verifiable through the existing web agent.

  • src/Novelly.Api/Agent/AgentContracts.cs — add RequiresNovelId to AgentToolDefinition. Safe: AnthropicAgentModelClient.ToSdkTool maps Name/Description/InputSchema explicitly, so the flag never reaches the model.
  • src/Novelly.Api/Agent/NovelAgentToolset.cs — add RequiresNovelId to AgentTool, set it on the 11 tools above, flow it into Definitions, and add the 12 new tools reusing the existing OrNotFound / DeletedOrNotFound / ToolNotFound idioms and the descriptions from the corresponding src/Novelly.Mcp/Tools/*.cs methods.
  • Rename the 33 existing schemas to camelCase in the same file — both the JsonSchemaBuilder property keys and the matching JsonInput lookup strings, which must stay in lockstep (.Str("character_id", …) / JsonInput.RequiredGuid(input, "character_id")"characterId"). Mechanical and contained to this one file, but it is the bulk of the chunk's diff and a mismatched pair fails silently as a missing argument rather than a compile error — so the per-tool tests below are what actually catch it. The 12 new tools are written camelCase from the start, matching the names their src/Novelly.Mcp/Tools/*.cs equivalents already used.

The 12 new tools and their existing service calls — no service-layer work is needed:

tool scope service call
list_novels none novels.ListAsync
create_novel none novels.CreateAsync
get_character child characters.GetAsync
relate_characters child characters.AddRelationshipAsync — note CreateRelationshipRequest's parameter order differs from the old MCP method's
set_arc_stage_beats child arcs.SetBeatsAsync
create_tag novel tags.CreateAsync
update_tag / delete_tag child tags.UpdateAsync / DeleteAsync
create_location novel locations.CreateAsync
update_location / delete_location child locations.UpdateAsync / DeleteAsync
update_open_question child questions.UpdateAsync

Guid-list arguments follow the existing JsonInput.Guids idiom used by reorder_beats.

Tests: new tests/Novelly.Api.Tests/NovelAgentToolsetTests.cs, driving ExecuteAsync directly in the style of ImportAgentToolsetTests.cs (ServiceTestFixture already wires every service the toolset needs). BDD names, one per new capability — e.g. Relating_two_characters_shows_the_pair_on_both_dossiers, Deleting_a_tag_leaves_the_characters_that_carried_it_alone, Updating_an_open_question_can_detach_it_from_its_chapter. Two structural tests carry the most weight:

  • The_toolset_offers_every_tool_the_stdio_server_offered — assert the 45 names against a hard-coded array. This is the anti-drift test.
  • Assert no RequiresNovelId tool's schema already declares novelId, since the MCP adapter injects it and a duplicate would be silent.

Runtime verify: AppHost up, open a novel's agent panel, ask it to create a tag and list tags; confirm in the Aspire trace.

Chunk 2 — Serve the registry at /mcp

  • src/Novelly.Api/Novelly.Api.csproj — add ModelContextProtocol.AspNetCore 2.1.0 (brings Core transitively; don't reference it directly). Not in the local NuGet cache — first restore needs network. Pin 2.1.0 to match the verified surface.
  • New feature folder src/Novelly.Api/Mcp/:
    • NovelMcpTools.cs — the adapter, as pure static methods: Describe(definitions) maps to Tool records, injecting novelId where RequiresNovelId; CallAsync(toolset, parameters, ct) serializes arguments to a JsonElement, extracts novelId when required (Guid.Empty otherwise), calls ExecuteAsync, and maps AgentToolResultCallToolResult. Catch the ArgumentException from a missing/malformed novelId and return it as IsError rather than letting it escape as a JSON-RPC error. Log {Tool} and {NovelId} only — never the arguments, which carry prose (what_happened, synopsis, notes).
    • McpEndpoints.csMapNovelMcp() calling app.MapMcp("/mcp"), matching the repo's Map*Endpoints convention.
  • src/Novelly.Api/Common/NovellyServiceRegistration.cs — the AddMcpServer(...) registration shown above.
  • src/Novelly.Api/Program.cs.MapNovelMcp() after UseAuthentication()/UseAuthorization().

Auth — the trap. Do not chain .RequireAuthorization() onto MapMcp. The parameterless overload applies the default policy, which authenticates IdentityConstants.ApplicationScheme only and would reject the API key. The fallback policy already registered in NovellyServiceRegistration lists both that scheme and ServiceApiKeyAuthenticationHandler.SchemeName, and applies to any endpoint carrying no authorization metadata — which MapMcp adds none of. /mcp inherits the right protection by doing nothing. If explicitness is wanted, register a named policy listing both schemes; never the parameterless call.

External clients send X-Novelly-Api-Key: <Auth:ServiceApiKey>, resolving to ServiceUser (Admin), which sees every novel. If Auth:ServiceApiKey is unset the service user is never seeded and every call 401s.

CORS needs no change — the origin-restricted default policy is irrelevant to non-browser clients, and stateless mode exposes no Mcp-Session-Id header to read.

Tests: new tests/Novelly.Api.Tests/NovelMcpToolsTests.cs, against the pure adapter methods — no live session, no WebApplicationFactory. Cover: all 45 advertised with unique names; novel-scoped tools declare a required novelId and child-id tools don't; list_novels needs none; every advertised schema is a valid type: object (what Tool.InputSchema's setter enforces, worth asserting before the SDK throws at startup); a missing novelId and a not-found id both come back as IsError results.

Deliberately not adding a WebApplicationFactory harness. None exists in the repo; adding one means an MVC.Testing reference, overriding the connection string, working around Program.cs's boot-time MigrateAsync + Environment.Exit(1), seeding the key, and parsing SSE. Its main payoff — proving the SDK wires up — is delivered more honestly by the curl walkthrough below, which exercises real Kestrel including auth, Serilog and the exception handler. Worth a separate chunk later if a regression harness is wanted.

Chunk 3 — Delete the stdio server

Only after Chunk 2 is verified, so there's never a window with no MCP surface.

Remove src/Novelly.Mcp/, scripts/publish-mcp.sh, the mcp-server/ publish output (gitignored; working-tree cleanup only), and the project line in Novelly.slnx. Verified as not referencing it: scripts/ci/build.sh (publishes the API by path), prepush.sh, test.sh, both Dockerfiles, the AppHost.

Docs to update:

  • README.md — fold the Novelly.Mcp stack-table row into the API's; rewrite "The MCP server" section (same 45 tools, now in-process at POST /mcp, no build step, X-Novelly-Api-Key auth). Note that MCP argument names are unchanged (camelCase), so existing clients need no edits, and that create_novel is owned by the signed-in user over the web agent but by the service user over MCP.
  • .mcp.json / .mcp.json.example — switch to type: "http", url: http://localhost:5080/mcp, with the key in headers.
  • .claude/agents/outline-importer.md — drop the published-binary requirement. Its tools: frontmatter is already stale (mcp__novelly__list_projects, get_project_brief, create_project exist in neither surface); fix to real names while here.
  • CLAUDE.md — the Structure list still calls src/Novelly.Mcp/ the "MCP stdio server" and Verifying still says "drive over stdio JSON-RPC". Propose these edits rather than slipping them in; CLAUDE.md is user-owned.

Leave docs/plans/api/users_and_roles_plan.md alone — historical.

Verify: dotnet build Novelly.slnx and ./scripts/ci/prepush.sh both pass.

Verification

Streamable HTTP needs Accept: application/json, text/event-stream and replies SSE-framed, so pipe through sed -n 's/^data: //p'.

dotnet user-secrets set Auth:ServiceApiKey devkey -p src/Novelly.Api
ASPNETCORE_URLS=http://localhost:5080 dotnet run --project src/Novelly.Api

MCP=http://localhost:5080/mcp
H=(-H "Content-Type: application/json" -H "Accept: application/json, text/event-stream" -H "X-Novelly-Api-Key: devkey")

# handshake — expect capabilities.tools present
curl -sS "${H[@]}" "$MCP" -d '{"jsonrpc":"2.0","id":1,"method":"initialize","params":{"protocolVersion":"2025-06-18","capabilities":{},"clientInfo":{"name":"curl","version":"1"}}}' | sed -n 's/^data: //p' | jq .
curl -sS "${H[@]}" -o /dev/null -w '%{http_code}\n' "$MCP" -d '{"jsonrpc":"2.0","method":"notifications/initialized"}'

# expect exactly 45
curl -sS "${H[@]}" "$MCP" -d '{"jsonrpc":"2.0","id":2,"method":"tools/list","params":{}}' | sed -n 's/^data: //p' | jq '.result.tools | length'

# novelId injection: present on list_tags, absent on delete_tag and list_novels
curl -sS "${H[@]}" "$MCP" -d '{"jsonrpc":"2.0","id":3,"method":"tools/list","params":{}}' | sed -n 's/^data: //p' \
  | jq '.result.tools[] | select(.name=="list_tags" or .name=="delete_tag" or .name=="list_novels") | {name, props:(.inputSchema.properties|keys), required:.inputSchema.required}'

# unscoped call, then a novel-scoped one (proves NovelUserContext resolved the service user in-handler)
curl -sS "${H[@]}" "$MCP" -d '{"jsonrpc":"2.0","id":4,"method":"tools/call","params":{"name":"list_novels","arguments":{}}}' | sed -n 's/^data: //p' | jq .
curl -sS "${H[@]}" "$MCP" -d '{"jsonrpc":"2.0","id":5,"method":"tools/call","params":{"name":"get_novel_brief","arguments":{"novelId":"<id from above>"}}}' | sed -n 's/^data: //p' | jq .

# error mapping — expect result.isError true, not a JSON-RPC error
curl -sS "${H[@]}" "$MCP" -d '{"jsonrpc":"2.0","id":6,"method":"tools/call","params":{"name":"list_tags","arguments":{}}}' | sed -n 's/^data: //p' | jq .

# auth — expect 401 with no key
curl -sS -o /dev/null -w '%{http_code}\n' -H "Content-Type: application/json" -H "Accept: application/json, text/event-stream" \
  "$MCP" -d '{"jsonrpc":"2.0","id":7,"method":"tools/list","params":{}}'

# one write, end to end
curl -sS "${H[@]}" "$MCP" -d '{"jsonrpc":"2.0","id":8,"method":"tools/call","params":{"name":"create_tag","arguments":{"novelId":"<id>","name":"Salt","color":"#9a4a2f"}}}' | sed -n 's/^data: //p' | jq .

Then reconnect a real client: rewrite .mcp.json to the type: "http" form and run /mcp in Claude Code to confirm 45 tools. Finally confirm the web agent still drives the same registry (Aspire up, agent panel, exercise one of the 12 new tools) and that the dashboard shows /mcp requests with no prose in the log lines.

Risks

  • ModelContextProtocol.AspNetCore is not cached locally. First restore needs network.
  • The web agent's argument names change (snake_case → camelCase) across all 33 existing tools. Nothing external consumes those schemas — they are built fresh per request and handed to the model each turn — so there is no compatibility surface, but a JsonSchemaBuilder key left out of step with its JsonInput lookup fails silently as a missing argument rather than a compile error. External MCP clients are unaffected: their argument names were already camelCase.
  • create_novel ownership differs by surface — signed-in user vs service user. Not a bug, but surprising; document it.
  • Not fixed here: child-id-scoped tools carry no novel context in either surface, so a caller holding a foreign beat/tag/question id can reach across novels, subject only to NovelAccessService. Pre-existing for 20+ tools and unchanged by this work. Worth a follow-up.
  • QA deploy: once merged, /mcp rides the existing API container and its already-exposed port. MCP_API_KEY must be set in Gitea for Auth__ServiceApiKey, or every MCP call 401s.