chore: prep Docker MCP Registry submission (roadmap 21.1) - #310
Merged
Conversation
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The JSON Schema → flat type mapping in
_resolve_typeis intentionally narrow (onlytypeandanyOf); consider failing loudly or logging when encountering unexpected shapes (e.g.,oneOf, nested arrays/objects) so future schema changes don’t silently downgrade to the fallbackstringtype. - The generator logic is embedded in a script and imported via
importlib.utilin the test; consider extracting the core functions (_resolve_type,_flatten_arguments, main generation routine) into a small importable module so both the script entrypoint and tests can use stable, public APIs instead of depending on a script path and private functions.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The JSON Schema → flat type mapping in `_resolve_type` is intentionally narrow (only `type` and `anyOf`); consider failing loudly or logging when encountering unexpected shapes (e.g., `oneOf`, nested arrays/objects) so future schema changes don’t silently downgrade to the fallback `string` type.
- The generator logic is embedded in a script and imported via `importlib.util` in the test; consider extracting the core functions (`_resolve_type`, `_flatten_arguments`, main generation routine) into a small importable module so both the script entrypoint and tests can use stable, public APIs instead of depending on a script path and private functions.
## Individual Comments
### Comment 1
<location path="packaging/docker-mcp-registry/generate_tools_json.py" line_range="67-68" />
<code_context>
+ ``[{"type": "string"}, {"type": "null"}]`` — the first non-null branch
+ wins), and schemas missing ``type`` entirely (Any-typed params).
+ """
+ if "type" in prop_schema:
+ return str(prop_schema["type"])
+ if "anyOf" in prop_schema:
+ for branch in prop_schema["anyOf"]:
</code_context>
<issue_to_address>
**issue (bug_risk):** Handle list-valued JSON Schema `type` more robustly.
When `type` is an array (e.g. `['string', 'null']`), `str(prop_schema['type'])` returns the list representation, which won’t match the registry’s expected simple type strings. Please special‑case list-valued `type` and select the first non-`null` entry, consistent with your `anyOf` handling, so optional fields still resolve to a usable single type.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
devopam
added a commit
that referenced
this pull request
Aug 14, 2026
Sourcery review on #310: prop_schema["type"] can legally be a list under JSON Schema 2020-12 (e.g. ["string", "null"]) — the old code did str() on it directly, which would have produced the Python list repr instead of a clean type string. Not currently triggered by any of MCPg's 254 tools (Pydantic emits anyOf for Optional[X] instead), but defensive since a future schema change could hit it silently. Mirrors the existing anyOf handling: first non-null entry wins. Added test_resolve_type_handles_every_json_schema_shape covering the shapes that aren't otherwise exercised via the real snapshot data. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PdN8kDW5Yc8rFb6scthKef
Adds packaging/docker-mcp-registry/ for the pending submission to
github.com/docker/mcp-registry:
- server.yaml draft (category=database, MCPG_DATABASE_URL as the sole
required secret, MCPG_ACCESS_MODE exposed read-only-default,
run.env forces MCPG_TRANSPORT=stdio).
- generate_tools_json.py derives the registry's tools.json bypass file
from tests/contract/tool_surface.snapshot.json. MCPg requires a live
MCPG_DATABASE_URL to start, so Docker's build sandbox can't run the
container to auto-discover tools the normal way; their CONTRIBUTING.md
offers tools.json as the documented bypass. Confirmed the real shape
({name, description, arguments: [{name, type, desc}]}, not MCP-native
inputSchema) against live registry entries before generating.
- tests/contract/test_docker_mcp_registry_tools_json.py guards the
checked-in tools.json against drift from the snapshot.
Also confirmed via a local Docker build + a real initialize/tools-list
stdio smoke test against a throwaway Postgres that run.env:
MCPG_TRANSPORT=stdio cleanly overrides the image's baked-in
streamable-http default — no Dockerfile change needed.
Not yet submitted: the fork + PR into docker/mcp-registry is a
separate, outward-facing step (roadmap 21.2) pending go-ahead.
Advances roadmap row: 21.1
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PdN8kDW5Yc8rFb6scthKef
Sourcery review on #310: prop_schema["type"] can legally be a list under JSON Schema 2020-12 (e.g. ["string", "null"]) — the old code did str() on it directly, which would have produced the Python list repr instead of a clean type string. Not currently triggered by any of MCPg's 254 tools (Pydantic emits anyOf for Optional[X] instead), but defensive since a future schema change could hit it silently. Mirrors the existing anyOf handling: first non-null entry wins. Added test_resolve_type_handles_every_json_schema_shape covering the shapes that aren't otherwise exercised via the real snapshot data. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PdN8kDW5Yc8rFb6scthKef
devopam
force-pushed
the
chore/docker-mcp-registry-prep
branch
from
August 14, 2026 12:06
79dc228 to
7499000
Compare
devopam
added a commit
that referenced
this pull request
Aug 14, 2026
Fork + PR sent: docker/mcp-registry#4689. Updates: - server.yaml: about.title changed MCPg -> PostgreSQL. Their validator (go run ./cmd/validate) rejects any title containing the literal substring "MCP" (case-sensitive) -- not documented anywhere, only caught by actually running their validator. Description still says "MCPg" freely; only the title field is checked. - server.yaml: source.commit re-pinned to main's tip at submission time (0ed672b, post-#309/#310) rather than the commit drafted alongside 21.1's tools.json generator. - feature-shortlist.md / CHANGELOG.md: mark 21.1/21.2 shipped, link the open PR, note 21.3 (254-tool surface size) as the next active work item. Advances roadmap row: 21.2 Claude-Session: https://claude.ai/code/session_01PdN8kDW5Yc8rFb6scthKef Co-authored-by: Claude Sonnet 5 <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.
Summary
Prep work for submitting MCPg to docker/mcp-registry — the registry backing Docker Desktop's MCP Toolkit /
docker mcpCLI. This PR is submission prep only (roadmap 21.1); the actual fork + PR intodocker/mcp-registryis a separate, outward-facing step (roadmap 21.2) pending explicit go-ahead, since it's a third-party public repo.Eligibility check (2026-08-14): no existing general-purpose PostgreSQL entry in their registry (only
prisma-postgres, which is Prisma-specific). MCPg clears every eligibility bar in theirCONTRIBUTING.md: MIT license, rootDockerfile, and a live-verifiedstdiotransport override.What's added
packaging/docker-mcp-registry/server.yaml— draft registry entry (categorydatabase,MCPG_DATABASE_URLas the sole required secret,MCPG_ACCESS_MODEexposed read-only-default,run.envforcesMCPG_TRANSPORT: stdio).packaging/docker-mcp-registry/generate_tools_json.py— generates the registry'stools.jsonbypass file fromtests/contract/tool_surface.snapshot.json. MCPg requires a liveMCPG_DATABASE_URLto start, so Docker's build sandbox can't run the container to auto-discover tools the normal way; their CONTRIBUTING.md documentstools.jsonas the bypass. Confirmed the real shape ({name, description, arguments: [{name, type, desc}]}— not MCP-nativeinputSchema) against live registry entries (servers/database-server,servers/context7) before generating, rather than guessing.tests/contract/test_docker_mcp_registry_tools_json.py— drift guard, per this repo's "generated beats hand-maintained" rule.Verification
Dockerfileand ran a realinitialize+tools/listJSON-RPC exchange over stdio against a throwaway Postgres container, with-e MCPG_TRANSPORT=stdio— confirmsrun.env: MCPG_TRANSPORT: stdiocleanly overrides the image's baked-instreamable-httpdefault. NoDockerfilechanges needed.python packaging/docker-mcp-registry/generate_tools_json.pyproduces all 254 entries; spot-checked simple and complex (nested object/array/anyOf-typed) tool schemas.pytest tests/contract/— 49 passed.pytest tests/unit— 2856 passed, 3 skipped.ruff check/ruff format --check/mypy src/mcpg— clean.Known gap (flagged, not blocking)
254 tools is unusually large for a single catalog entry; MCPg has no bucket/tool-filter env var today to ship a slimmer default surface (checked — doesn't exist). Tracked as roadmap 21.3; will address proactively in the actual registry PR description.
Advances roadmap row: 21.1
Summary by Sourcery
Prepare MCPg for submission to Docker’s MCP Registry by adding registry packaging metadata and a generated tools bypass file tied to the existing contract snapshot.
New Features:
Enhancements:
Tests: