0.5.0: backward-compat + discoverability fixes for the generic authority tools#5
Merged
Conversation
…ity tools
Driven by an adopting host's parity review against the API contract the gem
replaced. All authority-path; the satellite path is untouched.
Discoverability:
- resource_schema surfaces custom filters under resource_filters (the
Resource#filter docs promised this; nothing delivered it)
- resources returns filterable + note alongside name/description
- list tool description documents the full filter grammar (bare equality,
comma/array IN sets, the "null" token, { op:, value: } conditions and
AND-ed ranges) plus resource-specific top-level filters
- tool descriptions/input schemas rewrite sibling-tool references to carry
config.generic_tool_name_prefix, so prose matches the advertised names
Filter semantics:
- an Array of scalars is an IN set (was stringified + comma-split into
fragments that silently matched nothing); same for { op: eq/in } values
- eq/in against "null"/null renders IS NULL, not IN (NULL)
- a JSON null filter value means IS NULL (was silently ignored)
- an Array mixing operator conditions and bare values raises InvalidParams
Ordering:
- non-numeric-PK resources order by created_at WITH the PK tiebreaker,
restoring the total order offset pagination needs
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…/gateway parity
Review findings on the 0.4.1 branch (3-agent adversarial pass), all fixed:
P1 — resources discovery resilience: one resource's failing lazy filterable
resolution no longer 500s the whole index (rescue per resource, key omitted)
and no longer poisons the allowlist (Resource#resolve_filterable_source! now
clears the source only AFTER a successful call, so a transient failure is
retried instead of silently resolving to {}).
Filter-value semantics, explicit over silent:
- IN-set elements must be non-null scalars; nil/Hash/nested-Array elements
raise InvalidParams (was: Hash => query-time TypeError, nil => the
never-matching IN (..., NULL)); the "null" token stays literal inside a
set and is documented as scalar-only
- null with a non-eq/in/not_eq operator raises InvalidParams (was: matches +
null LIKE-NULL'd to zero rows on the real path while the fake said all rows)
- an op-less Hash as a bare value raises InvalidParams
- { op: "eq", value: "" } now matches empty-string rows (documented)
Parity across serving paths:
- satellite generic tools get the same resources filterable+note output and
the same grammar/resource_filters descriptions as the authority tools
- the gateway aggregator rewrites backticked tool references in proxied
definitions into the <app>__ namespace (shared ToolReferenceRewriter),
so a proxied list no longer points clients at the upstream's bare names
Test hardening (review found the Arel branch had zero suite execution):
- filtering_arel_spec renders the REAL Arel SQL for the eq/in/null/array
matrix (IS NULL vs IN (NULL) pinned against Arel, not the fake)
- specs for the surviving mutations: in+null, {op: eq, value: null},
bare-value-first mixed arrays, PK-name-driven tiebreaker (dead fallbacks
removed), per-tool prefix rewrites, schema-walk value preservation,
raising-source retry, list-description grammar pinning
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The satellite resources tool's enriched output (filterable + note parity with the authority path) was only asserted indirectly; the end-to-end spec now pins the payload shape. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ompanion filters, full schema parity
For a host migrating an EXISTING MCP endpoint onto the gem whose clients hold
the pre-gem contract:
- config.session_key_prefix + session_payload_dumper/loader: keep the pre-gem
session namespace and wire format so old and new app versions SHARE live
sessions during a rolling deploy (no forced re-initialization); the
sliding-TTL bump re-writes the raw payload untouched
- config.bare_filter_value_semantics = :literal: bare values verbatim
("a,b" literal, "null" literal, "" matches empty strings, arrays incl.
nil elements get the adapter's native IN / OR-IS-NULL); :tokenized default
unchanged; operator conditions identical in both modes
- config.non_numeric_pk_order = :primary_key: preserve a pre-gem ORDER BY id
contract; :created_at default (with PK tiebreaker) unchanged
- Resource#filter_requirements (+ lazy callable form): companion-key
validation restores safe polymorphic-FK filtering (old message preserved:
"filter attribute X requires Y to also be provided"); resource_schema
advertises the requirement under relationships[].filter.requires
- resource_schema restores the remaining pre-gem keys: sparse_fieldsets,
filter_examples, nullable relationships[].resource (target_resource kept as
the resolved alias) and relationships[].filter {keys,type,operators,requires};
top-level nils compacted
- operators on ANY column type: uuid/enum/jsonb/... accept eq/in (was
"cannot be filtered with operators"), date accepts in again
- list input schemas declare additionalProperties: true explicitly
448 specs, rubocop + brakeman clean.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ct args, companion-skip guard
Findings from the final backwards-compat audit, all addressed:
- the authority list tool's served description now states the bare-value
grammar the host ACTUALLY configured: under :literal semantics the
comma/null tokenization bullet is swapped for the literal-matching one
(definition gained a config: kwarg; description_text hook on Tools::Base)
- tools/list advertises the generic tools in pre-gem alphabetical order
(get, list, resource_schema, resources); string/text operator lists keep
the pre-gem order (eq, in, not_eq, matches, does_not_match)
- get / resource_schema / resources reject arguments outside their input
schema with InvalidParams (pre-gem parity; account_id always tolerated —
the transport consumes it); list keeps accepting extras (custom filters)
- a companion key whose value the executor would SKIP (empty string under
:tokenized) no longer satisfies a filter_requirements pairing — the FK is
rejected instead of applied alone (the type-ambiguous WHERE the feature
exists to prevent)
- resource_filters entries keep nil type/description keys (pre-gem shape);
relationship filter_examples companion sample restored to "User"
- filter_requirements docs + spec fixture: the companion key must itself be
filterable, or the pairing is unsatisfiable while the schema advertises it
- CHANGELOG documents the one accepted op-path delta ({op: in, value: comma
string} now splits into an IN set; old matched the literal string)
454 specs, rubocop + brakeman clean.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…, truthful CHANGELOG
- resources tool tolerates ANY extra argument again: the pre-gem tool took
**params (tolerant), unlike get/resource_schema whose strict Ruby kwargs the
strict-args restoration correctly mirrors. CHANGELOG corrected accordingly.
- config.filter_operator_overrides: per-column-type operator-set overrides,
merged over OPERATORS_BY_TYPE and read by BOTH the schema advertisement and
the executor's validation (single source — they cannot disagree). Lets a
host preserve a pre-gem operator contract exactly (text/date = eq/in only),
where the gem's richer sets would otherwise change schema bytes on dozens of
attributes AND start accepting previously-rejected conditions. Gem defaults
unchanged (satellites keep their current sets).
- CHANGELOG: the {op: in, value: comma-string} note no longer claims a
single-element Array preserves the literal (elements are split too);
documents the actual escape hatch (bare value, literal on :literal hosts).
459 specs, rubocop + brakeman clean.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…rovider composition, adapter structs
Every authority host was hand-rolling the same four pieces; they are now
first-class gem API (host keeps only its VALUES):
- config.session_payload_key_map: declarative codec for the flat
renamed-keys legacy session format (unmapped keys pass through; stored
keys symbolized on read so string-keyed cache coders round-trip; explicit
dumper/loader still wins). Strictly more robust than hand-written lambdas.
- config.register_upstreams_from_env({key => env var}, env: ENV): resets the
registry first (reload-idempotent) and skips blank urls — the two gotchas
every gateway host re-discovers. env injectable for tests.
- config.tool_provider composes a default when unset: RegistryToolProvider
(only when resources are registered — a pure gateway still contributes
nothing) + config.extra_tool_providers, with bare tool classes wrapped in
the new Authority::SingleToolProvider. Explicit assignment still wins.
- McpToolkit::Serializer::AssociationDescriptor + TargetRef: the exported
structs for the association duck-type ResourceSchema/FieldSelection probe.
Plus a doc note that the data-path settings are read via the process-global
McpToolkit.config (per the final audit's informational finding).
474 specs, rubocop + brakeman clean.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- Version 0.4.1 -> 0.5.0. 0.4.1 was never released (0.4.0 is the last tag);
fold the whole cycle into 0.5.0.
- Engine ServerController is now built by role: an authority host that mounts
McpToolkit::Engine gets Authority::ControllerMethods (hand-rolled dispatcher,
gateway proxying, usage metering, rate limiting); a satellite keeps the
SDK-backed Transport path. An authority can now mount its whole transport with
one line instead of hand-drawing the four routes against a subclass of
Authority::ServerController (still supported for hosts that draw their own).
- Remove the session codec seams (session_key_prefix, session_payload_key_map,
session_payload_dumper/loader). They existed only for one host's legacy-session
migration, which now accepts the one-time 404 + client re-initialization at
cutover. Session reverts to the native mcp_toolkit:session: prefix and the
{created_at, data} wire format (the shipped 0.4.0 behavior).
465 specs, 0 failures; RuboCop clean.
…d IN sets
Security review (Codex) findings on the filter path:
- filter_operator_overrides accepted arbitrary method names, later
public_send to an Arel attribute with the request value passed through
verbatim (normalize_value's else branch). A host that configured e.g.
{ datetime: ["extract"] } opened an SQL-injection surface (Arel's Extract
interpolates its field raw). The default/intended { text: %w[eq in] } usage
was never vulnerable, but the API permitted the unsafe state.
Fix: the config writer now rejects any operator outside AREL_PREDICATIONS at
assignment (fail fast), and predicate_for refuses to public_send any
non-predication operator (defense-in-depth chokepoint).
- IN sets and ANDed condition arrays were unbounded. New config.max_filter_values
(default 500, nil disables) caps resolved IN-set size and per-attribute
condition count so a valid token can't emit an oversized clause/AST.
- The Arel spec's fake connection did not escape quotes, so it could not have
caught an escaping regression. Made it escape like a real adapter and added an
injection-safety regression describe (eq/bare/matches payloads stay inside
escaped literals; both operator guards asserted).
Full suite 476 examples, 0 failures; rubocop + brakeman clean.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…gateway + metering resilience, satellite superuser gate From the PR #5 security review: - HIGH (availability): unbounded JSON-RPC batch. Rate limiting is a per-HTTP-request before_action, so a batch fanned out unbounded work under one tick. New config.max_batch_size (default 50, nil disables); an over-size batch is rejected as a JSON-RPC error before any element runs. - The top-level list `ids` filter now honors config.max_filter_values — it built WHERE id IN (...) on its own path, bypassing the cap that bounds the per-attribute filters. - The dispatcher no longer relays an unexpected StandardError's message to the caller: it returns a generic "Internal error" (full detail still logged), so SQL text / internal class names / hostnames can't leak. - The gateway tools/list aggregator skips a malformed (non-Hash / name-less) upstream tool entry and wraps each upstream's processing, so one bad entry degrades that upstream instead of 500-ing the whole aggregated list. - Usage metering flush falls back to per-event writes when the batch write fails, so one un-persistable ("poison") event can't drop metering for a whole request's batch (a billing-evasion vector). - The satellite tool path (get/list/resource_schema/resources) now enforces Resource#superusers_only! — previously only the authority path did. get/list/ resource_schema refuse it for a non-superuser; resources hides it. (Was still account-scoped, so never cross-tenant.) Full suite 493 examples, 0 failures; rubocop + brakeman clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ort leak, AuthorityBase message leak, flush-fallback safety From the second (post-fix) security review: - MEDIUM auth-context injection: the dispatcher called tool.call(context:, **args) where a splatted keyword OVERRIDES the explicit one, so a caller-supplied `context` argument replaced the resolved Authority::Context with attacker JSON. Fail-closed for the gem's own tools, but the gem handed attacker-controlled data as `context` to arbitrary host tools. symbolized_arguments now strips the reserved `context` key. - MEDIUM upstream host:port leak (completes the earlier dispatcher error-hygiene fix): translate_upstream_call_error relayed error.message verbatim for a TRANSPORT failure — "Failed to open TCP connection to <host>:<port>". Now returns a generic error (proxy already logs the detail); a first-party upstream JSON-RPC error is still relayed verbatim. - LOW-MED Tools::AuthorityBase#execute relayed an unexpected exception's message to the caller (via InternalError → the dispatcher's Protocol::Error branch, which the catch-all fix didn't cover). Now generic "Internal error" + logs detail. - LOW-MED usage-flush per-event fallback could break the response: a misbehaving logger/error_reporter in flush_individually would escape into the after_action. Swallowed as a last resort, preserving the "metering never affects the response" invariant. Full suite 496 examples, 0 failures; rubocop + brakeman clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…r leak - Filtering#bare_value: enforce max_filter_values on an Array bare value before the semantics branch, so :literal no longer skips the cap and emits an unbounded IN (...). - AuthorityBase#execute: relay only Ruby's kwarg-binding ArgumentErrors (missing/unknown keyword, wrong arity); sanitize any other ArgumentError to a generic InternalError so business-logic messages can't leak. - Regression specs for both paths. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
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.
Backward-compatibility and discoverability fixes for the generic tools (
resources/resource_schema/get/list), driven by an adopting host's parity review against the API contract the gem replaced — plus a follow-up adversarial review pass whose findings are all fixed on this branch.Discoverability
resource_schemasurfaces custom filters underresource_filters([{name, type, description}]). They worked but were advertised nowhere.resourcesreturnsfilterable+notealongside name/description, so a resource's usage caveat surfaces at browse time. One resource's failing lazyfilterableresolution degrades to an omitted key (logged) instead of failing the whole discovery index, andResource's lazy source is now cleared only after a successful call, so a transient failure is retried instead of silently resolving the allowlist to{}.listtool description documents the full filter grammar — bare equality, comma/array IN sets, the"null"token,{ op:, value: }conditions, AND-ed ranges, resource-specific top-level filters.config.generic_tool_name_prefixis threaded into descriptions and input schemas (sharedToolReferenceRewriter), and the gateway aggregator applies the same rewrite with the upstream namespace, so a proxied<app>__listno longer points clients at the upstream's bare tool names.resourcesoutput and the same grammar/resource_filtersdescriptions.Filter semantics (explicit over silent — each previously returned a wrong or empty result)
to_s-ed and comma-split into fragments matching nothing); same for{ op: "eq"/"in" }values.eq/inagainst"null"/null rendersIS NULL, notIN (NULL)."null"token is scalar-only — SQLINcannot match NULL.eq/in/not_eqoperator raises InvalidParams (was:matches+ null LIKE-NULL'd to zero rows; comparisons silently matched nothing).{ op: "eq", value: "" }matches empty-string rows (was: nothing).Ordering
created_atwith the model's PK as tiebreaker (offset pagination needs the total order; bulk-inserted rows share timestamps).Verification
filtering_arel_specrenders actual Arel SQL for the eq/in/null/array matrix)in+null,{op: eq, value: null}, bare-first mixed arrays, PK-name-driven tiebreaker, per-tool prefix rewrites, schema-walk value preservation, raising-source retry, grammar pinning🤖 Generated with Claude Code
Host-compatibility seams (full pre-gem contract parity)
A follow-up backwards-compatibility audit against the pre-gem contract surfaced five remaining client-visible deltas; all are closed by config seams (gem defaults unchanged — satellites keep their current contract; the migrating host opts in):
session_key_prefix+session_payload_dumper/loader— keep the pre-gem session namespace and wire format, so live sessions survive the deploy and old/new instances share sessions during a rolling deploy (no forced re-initialization). The sliding-TTL bump rewrites the raw payload untouched.bare_filter_value_semantics = :literal— bare values verbatim ("a,b"one literal string,"null"the literal string,""matches empty strings, arrays incl. nil elements get the adapter's native IN / OR-IS-NULL). Default:tokenizedunchanged; operator conditions identical in both modes.non_numeric_pk_order = :primary_key— preserves a pre-gem ORDER BY id contract for uuid-PK resources; default:created_at+tiebreaker unchanged.Resource#filter_requirements— companion-key validation restores safe polymorphic-FK filtering ("filter attribute X requires Y to also be provided");resource_schemaadvertises the requirement underrelationships[].filter.requires.resource_schemafull shape parity — restoressparse_fieldsets,filter_examples, nullablerelationships[].resource(keepingtarget_resourceas the resolved alias) andrelationships[].filter(keys/type/operators/requires); top-level nils compacted.eq/infallback for uuid/enum/jsonb/...;dateacceptsinagain), and thelistinput schemas declareadditionalProperties: trueexplicitly.Verification: 448 gem examples / 0 failures; RuboCop + Brakeman clean; on the adopting host, 291 MCP specs green plus 25 runtime compat checks against real models (literal SQL, ORDER BY id, session round-trip in the old wire format in both directions).
Final-audit fixes (post-parity verification round)
A last independent audit against the pre-gem contract surfaced residual deltas; all fixed:
listdescription now states the bare-value grammar the host actually configured (:literalhosts no longer serve tokenization advice that would silently match nothing).get, list, resource_schema, resources(pre-gem alphabetical order); string/text operator lists keep the pre-gem order.get/resource_schema/resourcesreject unknown arguments with InvalidParams again (account_idtolerated);listkeeps accepting extras (custom filters).:tokenizedsemantics no longer bypassesfilter_requirementspairing.resource_filterskeeps niltype/descriptionkeys; relationshipfilter_examplescompanion sample is"User"(pre-gem values).{op: "in", value: "a,b"}now splits into an IN set (old matched the literal string).454 examples / 0 failures; RuboCop + Brakeman clean. Adopting host re-verified: 291 MCP specs + runtime checks per fix (including the host-side guests
fieldsregression guard).Host-boilerplate ports (final round)
A fifth independent audit confirmed full compatibility (modulo the documented list) and identified host plumbing every authority adopter would re-write; it is now first-class gem API:
config.session_payload_key_map— declarative legacy session codec (flat renamed-keys format; unmapped keys pass through; string-keyed cache rows symbolized on read). Replaces hand-written dumper/loader lambdas for the common case.config.register_upstreams_from_env(map, env: ENV)— reset-first (reload-idempotent) + blank-skip upstream registration from a{key => env var}map.config.tool_provider— RegistryToolProvider (only when resources are registered; pure gateways still contribute nothing) +config.extra_tool_providers, with bare tool classes wrapped in the newAuthority::SingleToolProvider.McpToolkit::Serializer::AssociationDescriptor/TargetRef— exported structs for the association duck-type, so serializer adapters stop re-deriving the field names.474 examples / 0 failures; RuboCop + Brakeman clean. The adopting host swapped onto all four (deleting 3 plumbing files) with 291 specs green and 10 runtime checks pinning wire-format and tools/list byte-parity across the swap.