MCP overhaul: 2026-07-28 conformance, dual-era support, and Tasks - #107
Merged
Conversation
…d resources/read and prompts/get
Layer-1 method authorization was only performed inside the "Mcp-Session-Id
header present" branch of the POST /mcp/jsonrpc handler. A client that simply
omitted the session header took the other branch — a fresh unauthenticated
session was minted and the request dispatched with no authorization check. With
mcp.auth.enabled: true, this let an anonymous caller invoke any method,
including resources/read, and receive the resource's full query result.
Reproduced before the fix (mcp.auth.enabled, one mcp-resource):
resources/read, no Mcp-Session-Id, no Authorization
-> 200 {"result":{"contents":[{"text":"[{\"confidential\":\"TOP-SECRET...\"}]"}]}}
After: the same request returns {"code":-32001,"message":"Authentication
required for method: resources/read"}.
Changes:
- Authenticate and authorizeMethod on EVERY request, independent of any session
header (all methods except initialize). Sessions are now a legacy echo only,
never an authorization carrier.
- Add Layer-2 per-entity RBAC to resources/read and prompts/get, which had no
authorization behind the Layer-1 check. Adds optional allowed-roles to
mcp-resource and mcp-prompt with the same deny-by-default-under-auth semantics
as mcp-tool.allowed-roles; generalized MCPAuthorizationPolicy::authorizeRoles.
Backward compatibility: with mcp.auth.enabled: false (the default), authenticate
returns nullopt and authorizeMethod returns true, so the path is unchanged.
Auth failures still use -32001 in a 200 body for now; the 401/WWW-Authenticate
migration follows in a later commit.
Adds test/integration/test_mcp_auth_matrix.py (17 cases) covering the
no-session-header bypass across every method plus the resource/prompt RBAC
matrix. All existing MCP integration and unit suites stay green.
Every MCP tool parameter was advertised as {"type":"string"} regardless of its
validators, so a model calling a flAPI tool could not see that a parameter is an
integer with a range, a date, a uuid, an email, or an enum — information flAPI
already holds and already uses for prepared-statement binding.
New MCPSchemaBuilder::buildInputSchema maps ValidatorConfig to JSON Schema:
int->integer (+minimum/maximum), double->number, boolean->boolean,
date/time->string+format, uuid/email->string+format, enum->enum from
allowedValues, string->minLength/maxLength, regex->pattern; defaultValue->default.
Unset numeric bounds (stored as INT_MIN/INT_MAX sentinels) are suppressed rather
than emitted as noise.
Replaces the duplicated hand-built schema in both endpointToMCPToolDefinition
(mcp_route_handlers.cpp) and MCPToolHandler::getToolDefinition, which now both
delegate to the builder. Every property still carries `type` and `description`,
so the schema shape stays backward-compatible with existing clients.
Adds test/cpp/mcp_schema_builder_test.cpp (8 cases).
…redContent
Two spec-conformance fixes on the tools/call path.
isError: every tool-execution failure was returned as a JSON-RPC -32603, which
tells the model the call is unfixable. The spec separates protocol errors from
tool-execution errors so the model can self-correct the latter. Now:
- unknown tool -> JSON-RPC -32602 (protocol; unchanged category)
- RBAC denial -> JSON-RPC -32001 (protocol; 403 in a later commit)
- bad arguments / rate limit / SQL runtime error -> result with isError:true
The invalid-arguments path now surfaces the per-field validator messages (which
were previously discarded) so the model sees "customer_id: Integer is less than
the minimum allowed value" instead of a generic failure.
structuredContent: successful tool results now carry the row array as real JSON
({"rows":[...],"row_count":N}) alongside the existing text block, so a client
need not re-parse the stringified rows.
MCPToolExecutionResult gains a FailureKind; ContentResponse gains setError /
setStructuredContent. Adds test/cpp/mcp_content_types_test.cpp; updates the
rate-limit integration helper for the isError shape.
Note: outputSchema (deriving a JSON Schema from the DuckDB result column types)
is deferred; it needs QueryResult to expose column types and is tracked as
remaining C4 work.
This was referenced Aug 30, 2026
Fixes JSON-RPC handling and removes dead MCP code (research ISSUES #8, #10). Hygiene: - Notifications (a request with no `id`) now get HTTP 202 and no body, per JSON-RPC — previously flAPI replied with a spurious -32601 and id:null. - The response id is echoed verbatim from the request's raw JSON token, so large integer ids survive intact (the old std::stod round-trip mangled anything past 2^53) and string/number/null types are preserved. MCPRequest gains id_present / id_raw for this. - initialize now advertises listChanged: false for tools/resources/prompts — flAPI has no server->client notification transport and never emits list_changed, so advertising true was dishonest. Dead code removed: MCPServer ([[deprecated]], unused), MCPErrorBuilder (zero callers), MCPRequestValidator (unwired; its initialize check rejected all but 2024-11-05 and was a landmine), MCPStreamingResponse (never used), the sampling capability stubs (CAPABILITY_SAMPLING, MCPServerCapabilities::sampling, the "sampling" server-capability default). Their tests and CMake entries go too. Adds test/integration/test_mcp_protocol_hygiene.py; flips the sanctioned listChanged assertions in test_mcp_integration.py.
flAPI's MCP endpoint was not discoverable as an OAuth resource server: auth failures came back as a JSON-RPC -32001 inside an HTTP 200 with no WWW-Authenticate header, so a standard OAuth client (Claude, VS Code, Goose) could not learn how to authenticate and the browser flow could never start. - Serve GET /.well-known/oauth-protected-resource (RFC 9728) when an OIDC authorization server is configured: advertises `resource`, `authorization_servers` (the OIDC issuer), `bearer_methods_supported`, and optional `scopes_supported`. Returns 404 for basic/bearer auth (no discoverable authorization server). - Authentication failures now return HTTP 401 with `WWW-Authenticate: Bearer[ resource_metadata="<url>"]`; role/authorization denials return HTTP 403 with `error="insufficient_scope"` (RFC 6750). The metadata URL is derived from X-Forwarded-Proto/Host (or Host), honouring a reverse proxy, or an explicit `mcp.auth.canonical-resource-uri`. - MCPResponse gains http_status + www_authenticate; the last -32001 codes in the MCP route layer are gone. Config: `mcp.auth.canonical-resource-uri`, `mcp.auth.scopes-supported`. Sanctioned test updates: RBAC denials now assert 403 (was 200). New C5 assertions in test_mcp_auth_matrix.py.
…esources Three conformance items (research ISSUES #7, #9). Pagination: tools/list, resources/list, prompts/list (and the new resources/templates/list) support opaque cursor pagination via a new mcp.page-size config. Default 0 disables it — the whole list is returned with no nextCursor, exactly as before. A cursor is base64 {offset,gen}; the generation counter is bumped on every refreshMCPEntities() so a cursor minted before a config reload is rejected with -32602 rather than paging over a changed list. completion/complete now wraps its payload under a "completion" key ({"completion":{values,total,hasMore}}) as the spec requires. resources/templates/list is implemented (was a dead method), and mcp-resource gains an optional uri-template (e.g. flapi://customers/{id}). A templated resource appears in resources/templates/list, and resources/read matches the template, binds the {var} path segments into request params (validated and prepared-statement-bound like any field), and reports the concrete URI. Config: mcp.page-size, mcp-resource.uri-template (both default to prior behaviour). Adds test/integration/test_mcp_pagination.py.
Adds the stateless MCP 2026-07-28 path alongside the legacy initialize+session path, selected per-request by the presence of params._meta["io.modelcontextprotocol/protocolVersion"]. Legacy requests are untouched and byte-compatible; only modern requests see the new behaviour. Modern path: - server/discover (the initialize replacement): supportedVersions (2026-07-28 first), honest capabilities incl. the io.modelcontextprotocol/tasks extension, instructions, serverInfo. Publicly reachable (no auth) like initialize. - Per-request _meta preamble: required protocolVersion (unknown -> -32022 + supported list, HTTP 400) and clientCapabilities (missing -> -32602/400); optional logLevel and client extension ids (used later for Tasks gating). - Result envelope: resultType:"complete" and _meta.serverInfo on every result; ttlMs + cacheScope on cacheable results (server/discover public/1h, lists and resources/read private/5m). - Stateless: never mints or echoes Mcp-Session-Id and ignores an inbound one. - ping and logging/setLevel are removed on the modern path (-32601); GET on the endpoint returns 405. Legacy path keeps initialize, sessions, ping, logging/setLevel and the old envelope exactly. Adds MCP_SUPPORTED_VERSIONS and the 2026 _meta/error constants. New test/integration/test_mcp_modern_era.py (10 cases).
On the modern path the Streamable HTTP transport requires the request to be mirrored into headers so an edge proxy can route and rate-limit without parsing the JSON-RPC body: MCP-Protocol-Version, Mcp-Method, and Mcp-Name (for tools/call, prompts/get, resources/read). A missing required header or a mismatch against the body is -32020 HeaderMismatch / HTTP 400. New pure-function module mcp_header_validation: - decodeSentinel: decodes a `=?base64?<b64>?=` value (for non-ASCII header values), passing plain values through. - numericEquals: treats "42" and "42.0" as equal (a mirrored integer param may render differently in the header and body). - headerMatches: exact, then numeric, then sentinel comparison. Validation runs modern-era only, after the preamble checks; legacy requests never see it. The spec's value-encoding table is transcribed into test/cpp/mcp_header_validation_test.cpp; the modern integration suite sends the mirrors and covers missing/mismatched header cases.
Adds request[].mcp-header: naming a header suffix marks the parameter to be mirrored by the client into an Mcp-Param-<name> header, so an edge proxy can route and rate-limit per tenant/param without parsing the JSON-RPC body. The schema builder emits the "x-mcp-header" annotation on the property. Load-time enforcement (validateMcpHeaderAnnotations) fails the config if a header name has invalid token characters, collides case-insensitively with another field's, or looks like a secret (token/secret/password/key/…): a header value is visible to every intermediary and must never mirror a credential. Config: request[].mcp-header (default unset = no annotation, no change). Adds a schema-builder unit case.
Implements the io.modelcontextprotocol/tasks extension so a slow analytical
tool returns a taskId immediately instead of blocking the request connection
until a proxy times out.
- MCPTaskManager: a bounded worker pool + in-memory task store (thread-safe;
submit/get/cancel, TTL sweep, queue backpressure). A task carries its owning
principal and tasks/get re-checks ownership on every poll so a taskId is a
name, not a capability.
- tools/call runs a tool as a task only when (modern era) AND the client
declared the tasks capability AND the tool is configured async/async-after.
Otherwise it stays synchronous, so clients without the capability and all
legacy clients never see a task. `async: true` returns the task immediately;
`async-after-ms` runs synchronously up to a grace period then degrades to a
task. The result envelope reports resultType:"task" for a task handle.
- tasks/get and tasks/cancel RPCs. server/discover already advertises the
extension.
Config: mcp-tool.async / mcp-tool.async-after-ms; mcp.tasks.{workers,
queue-depth,default-ttl-ms,poll-interval-ms}.
Scope: the store is in-memory — tasks do not survive a process restart. A
restart-durable DuckLake-backed store is a planned follow-up; the API is shaped
to swap it in. Adds mcp_task_manager_test.cpp (5) and test_mcp_tasks.py (5).
Adds MCP_REFERENCE §11 covering the dual-era model (era selection via _meta, server/discover, the result envelope, mirrored headers, statelessness, OAuth discovery + 401/403, the Tasks extension, x-mcp-header, pagination, notifications, completion wrapper, resource templates) and updates the header to note flAPI is a dual-era server serving 2026-07-28 alongside the legacy protocol.
An earlier MCP commit inadvertently swept in an unrelated posthog-telemetry submodule pointer bump (pre-existing local dirty state) via git add -A. Restore it to main's pointer so this PR only touches the MCP layer.
…lication outputSchema (research ISSUES #5, deferred from C4): flAPI cannot know a parameterised query's result columns statically, so the outputSchema is derived from the first successful tools/call result (per-column JSON Schema types inferred from real values) and cached, then merged into tools/list as {rows: array<row>, row_count: integer}. The cache is cleared on refreshMCPEntities(). A tool that has not been called yet simply carries no outputSchema, which is honest. Cleanup: collapse the duplicated discovery methods to a single discoverMCPEntitiesImpl() (removes the const overload's const_cast and the dead getToolDefinitions()/getResourceDefinitions() accessors). Adds test/integration/test_mcp_output_schema.py.
Persists MCP tasks to a flapi_mcp_tasks table in the configured DuckDB so they
survive a process restart when duckdb.db_path is file-backed. Tasks are written
on creation and on every terminal transition; on startup the manager recovers
existing rows and marks any task left `working` (a crash mid-execution) as
`failed`. Durability is best-effort: a DB hiccup degrades to the in-memory task
without breaking execution, and with an in-memory database the table is simply
recreated empty each start.
MCPTaskManager gains an optional SqlExec callback (kept decoupled from
DatabaseManager); the route handler backs it with
DatabaseManager::executeQuery(sql, {}, with_pagination=false) — false so DDL/DML
and the recovery SELECT are never wrapped in a pagination subquery. Adds a
restart-recovery integration test; docs updated to describe real durability.
- Backward-compat: modern-only methods (server/discover, tasks/get, tasks/cancel) now return -32601 on the legacy path instead of executing, and legacy-only methods (initialize/ping/logging) return -32601 on the modern path. - Thread-safety: newTaskId() uses a thread_local RNG so concurrent submit() calls no longer race on a shared std::mt19937_64. - TTL on read: tasks/get now enforces the task TTL too (not just submit's sweep), so a terminal task's result stops being returned once past ttlMs even on an idle server. - Restart recovery: recovered tasks reconstruct created_at from the persisted updated_at_ms (aged by wall-clock elapsed) instead of resetting to now, so a restart no longer extends every task's TTL window. Cancellation remains cooperative (documented): a running SQL statement completes before the task is marked cancelled — true mid-query interruption via duckdb_interrupt is a tracked follow-up. Adds legacy-rejection tests.
… JSON-RPC errors Two regressions caught by the full integration suite: - The discoverMCPEntities consolidation dropped the flapi_* config-service tool merge block, so none of the config tools appeared in tools/list (breaking the flapii CLI). Restored the block in discoverMCPEntitiesImpl (and removed the early-return-on-no-endpoints so config tools load even with zero endpoints). - Revert config-tool execution failures back to JSON-RPC -32603 errors. isError is for data/endpoint tool execution the model can self-correct; the flapi_* management tools' tests (and semantics) expect a JSON-RPC error for a missing endpoint/template/cache target. Verified: all 43 test_mcp_config_tools.py cases pass against the release binary.
…eview) Correctness/security fixes from a second review pass: - Async RBAC bypass: tools/call on an async tool now runs the per-tool RBAC policy BEFORE submitting to the task queue, so a denied-but-authenticated caller gets a 403 (matching the sync path) instead of a taskId + a consumed queue slot. - inputSchema min:0/max:0: an explicit numeric bound of 0 was mistaken for "unset" and dropped from the schema; only the INT sentinel now means unset. - Pagination cursors are bound to their issuing list method (a tools/list cursor is rejected by resources/list) and validated for type/range (non-number or negative offset/gen -> -32602) instead of being coerced. - Task submit persists the initial `working` row BEFORE enqueue, closing a race where a fast worker could persist the terminal state first and get it clobbered back to `working`. - async-after grace loop breaks on any terminal state (not only Completed), so a fast-failing task returns promptly instead of waiting the full budget. - outputSchema types JSON numbers as `number` (not a per-row `integer` guess that a later fractional row would violate). - numericEquals rejects overflow/inf/nan (1e309 vs 2e309 no longer compare equal). - x-mcp-header secret check also inspects the mirrored FIELD name (a field named api_key can't be exposed through a header named Tenant). - mcp.tasks.default-ttl-ms <= 0 is clamped to the default so the task store is always reaped. Adds unit + integration coverage. Shutdown-hang on a long in-flight async query is tracked with the cancellation follow-up (#111).
jrosskopf
marked this pull request as ready for review
August 31, 2026 09:28
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Overhauls flAPI's MCP layer following two research analyses (
2026-08-24-flapi-mcp-2026-spec,2026-08-24-mcp-2026-07-28-flapi-erpl-adt) against the MCP2026-07-28spec. Delivered as ordered, independently-buildable commits in three tranches. Hard constraint: no existing MCP consumer breaks — legacyinitialize+Mcp-Session-Idclients on protocol versions2024-11-05…2025-11-25keep working; the modern stateless2026-07-28path is added alongside (dual-era).This is a draft / work in progress. Landed so far:
✅ C1 — SECURITY: MCP method-auth bypass + unguarded resources/read (
a38fdc2)mcp.auth.enabled: true, a client that omitted theMcp-Session-Idheader skipped Layer-1 method authorization entirely and could read anymcp-resource's full query result unauthenticated. Before:resources/readwith no session + no auth returned the payload with HTTP 200. After:-32001denial.authorizeMethodon every request, independent of the session header (sessions are now a legacy echo only). Added Layer-2 per-entity RBAC (allowed-roles) toresources/readandprompts/get, which previously had no authorization behind the Layer-1 check.mcp.auth.enabled: false(default) the path is unchanged.test/integration/test_mcp_auth_matrix.py(17 cases).✅ C3 — Typed
inputSchemafrom validators (599351c){"type":"string"}; nowMCPSchemaBuilderprojectsValidatorConfiginto JSON Schema (int/double/boolean/date/uuid/email/enum + min/max/regex/default). Raises first-call correctness for models. Schema shape stays backward-compatible (every property keepstype+description).test/cpp/mcp_schema_builder_test.cpp(8 cases).✅ C4 — isError + structuredContent (
4193224)isError:true(bad args carry the per-field validator message; rate limits carryretry_after_seconds) instead of an opaque JSON-RPC-32603; unknown tool →-32602; RBAC denial stays a protocol error. Successful results attachstructuredContent({rows, row_count}) alongside the text block.outputSchema(from DuckDB result column types) deferred — tracked as remaining C4 work.test/cpp/mcp_content_types_test.cpp; rate-limit integration helper updated for the isError shape.✅ C2 — Protocol hygiene + dead-code purge (
e12d838)listChanged: false(honest — no notification transport). Deleted MCPServer/MCPErrorBuilder/MCPRequestValidator/sampling stubs.test_mcp_protocol_hygiene.py.✅ C5 — OAuth discovery (RFC 9728) + 401/403 (
969d659)/.well-known/oauth-protected-resource(OIDC); auth failures → 401 +WWW-Authenticate; RBAC → 403insufficient_scope. No-32001left in the route layer.✅ C6 — Pagination, completion wrapper, resource templates (
ca24124)mcp.page-size, default off);completion/completewrapped undercompletion;resources/templates/list+mcp-resource.uri-templatewith path-param binding.test_mcp_pagination.py.Tranche 0 is complete except
outputSchema(per-column types can't be derived statically for parameterised SQL without executing; tracked in #108).Remaining (tracked, design in the research ISSUES.md / MIGRATION.md)
Tranche 0 (conformance): C2 dead-code purge + JSON-RPC id/notification hygiene · C4
isError:truefor tool failures +structuredContent/outputSchema· C5 RFC 9728 protected-resource metadata +401/WWW-Authenticate· C6 pagination + completion wrapper + resource templates.Tranche 1 (dual-era 2026-07-28): C7
server/discover+ per-request_meta+resultType+ttlMs/cacheScope+ session retirement · C8 mirrored-header validation (base64 sentinel) · C9x-mcp-header.Tranche 2 (new capability): C10–C12 Tasks extension (DuckLake-backed store, worker pool,
tasks/get/tasks/cancel,mcp-tool.async/async-after, capability-gated with synchronous fallback) · C13 docs.Tracked as: #108 (Tranche 0 remaining), #109 (Tranche 1 dual-era), #110 (Tranche 2 Tasks), DataZooDE/erpl-adt#50 (erpl-adt CORS).
Out of scope (per research): MCP Apps (belongs in peacock/triton), Skills-over-MCP (SEP-2640 in review), sampling/roots (deprecated). A separate issue will be filed on
erpl-adtfor itsAccess-Control-Allow-Origin: *transport gap.Test plan
make debugbuilds cleantools/listmake test-all(blocked locally by an unrelated DuckDB debug-build assertion booting theapi_configurationfixture; runs clean in CI)🤖 Generated with Claude Code
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.✅ Tranche 1 — dual-era MCP 2026-07-28 (#109)
d1ffcd2) —server/discover, per-request_metapreamble (unknown version → -32022, missing clientCapabilities → -32602), result envelope (resultType,_meta.serverInfo,ttlMs/cacheScope), stateless (no session),ping/logging/setLevelremoved on modern path,GET→ 405. Legacy path byte-compatible.test_mcp_modern_era.py.0264bfe) — mirrored-header validation (MCP-Protocol-Version/Mcp-Method/Mcp-Name, base64 sentinel, numeric equality) → -32020.mcp_header_validation_test.cpp.84f84ae) —x-mcp-headerparam mirroring + load-time secret/uniqueness enforcement.✅ Tranche 2 — Tasks extension (#110)
c66bd2f) —MCPTaskManager(worker pool + in-memory store),tools/callasync gating (modern ∧ client-declares-tasks ∧mcp-tool.async/async-after-ms),tasks/get/tasks/cancel, principal isolation,resultType:"task".mcp_task_manager_test.cpp+test_mcp_tasks.py. In-memory only — restart-durable DuckLake store is a follow-up.✅ Docs (
acf3f3a)Verification: full C++ suite 663/663; every MCP integration suite green (auth-matrix, rbac, rate-limit, dry-run, hygiene, pagination, modern-era, tasks). Legacy suites unchanged prove non-breakage.
Known remaining (small):
outputSchemaper-column types (needs executing the template; tracked in #108), a Tasks DuckLake durable store (in #110), and collapsing thediscoverMCPEntitiesconst/non-const overload (cosmetic).Follow-ups completed + review (2026-08-31)
outputSchema(learned from first call → merged into tools/list),discoverMCPEntitiescleanup, and durable Tasks store (DuckDB-backed, survives restart whenduckdb.db_pathis a file;working→failedrecovery). Commitsa180c75,869ca8d.ebb2df7) — legacy-path exposure of modern-only methods (→ -32601),newTaskIdRNG race (thread_local), TTL enforced ontasks/get, recovered-task TTL age reconstruction; cooperative cancellation documented (realduckdb_interruptinterruption tracked as MCP Tasks: interrupt in-flight queries on tasks/cancel (duckdb_interrupt) #111).aeb715f): the discovery consolidation had dropped theflapi_*config-tool registration and over-appliedisErrorto config-tool failures — both restored.Final verification: 216 C++ unit tests + 235 MCP integration tests pass against the release binary (standalone + shared-fixture legacy suites). Branch is 16 commits ahead of main.
Pending before merge:
/code-review ultra(user-triggered) and GitHub CI green.