fix(mcp): hydrate the profile the gateway runs, not just the enabled registry - #2805
Merged
Merged
Conversation
…registry
The GitHub MCP server returns 401 Bad credentials on the 4090 and the hydrator
reported nothing wrong, because it was looking at a different list of servers
than the gateway serves.
gateway runs docker mcp gateway run --profile pmoves_5090_web (25 servers)
hydrate read Docker's enabled-server registry file only
`github-official` is in the profile and not in that registry, so its secret was
never considered. Not pushed -- and not reported as a manual gap either, which
is the worse half: the tool's contract says unmanaged secrets are "reported by
name, never guessed", and this one was invisible to both paths. The server came
up with an empty GITHUB_PERSONAL_ACCESS_TOKEN and failed at call time instead of
at hydrate time.
load_required now unions both sources and dedups on (server, secret), so a
server in both is one requirement rather than two resolver calls -- the resolver
being the fragile part of this path.
Profiles come from `.mcp.json`, the gateway's actual invocation, NOT from the
tooling default. mcp-toolkit-connect.sh:31 and mcp-toolkit-bootstrap.sh:14-15
both default to `pmoves_5090_web` -- a profile named for one node -- so a
hardcoded default would report the same answer on every node regardless of what
that node actually started. With no `.mcp.json` the function returns [] and
main() says so on stderr: absent config is not a licence to guess.
Measured on this node, same command, before and after:
before 2 secrets (e2b, hostinger) -- github absent
after + github-official github.personal_access_token -> GITHUB_PERSONAL_ACCESS_TOKEN
+ dockerhub dockerhub.pat_token -> HUB_PAT_TOKEN
+ postman postman.postman-api-key -> POSTMAN_API_KEY
Three servers were silently unprovisioned, not one.
This makes the gap VISIBLE; it does not by itself fix the 401. `docker mcp
secret set` goes through the same resolver that is currently returning
deadline_exceeded on this node, so the documented order still holds: restart
Docker Desktop, confirm the resolver answers, then hydrate.
10 tests (was 6). The four new ones fail against the unfixed tool.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X9mpK1ZFjexjthZbvcb94f
Contributor
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b33366f526
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Found while confirming the profile-discovery fix against the live node. There
are two writers into the same Docker MCP secret store, keyed on the same strings
from opposite directions, with nothing holding them together:
scripts/mcp-toolkit-secrets-sync.sh env var -> docker secret name
config/docker_mcp_secret_map.yaml docker secret name -> env.shared key
Measured against `docker mcp profile show` on 2026-08-28, Docker's real names
are hostinger-mcp-server.api_token, dockerhub.pat_token,
github.personal_access_token and postman.postman-api-key. Two disagreed:
1. postman. The shell script wrote `postman.api_key`; the field really is
doubled, `postman.postman-api-key`. That writes a secret under a name no
server reads -- the sync reports success and postman still starts
unauthenticated. A silent success is worse than a failure here, because
nothing prompts anyone to look.
2. github. The map had no entry at all, so the hydrator fell back to the
INJECTED env-var name (GITHUB_PERSONAL_ACCESS_TOKEN) as the env.shared key.
The funnel does not call it that -- the shell script uses GITHUB_PAT /
GITHUB_TOKEN -- so it would have reported a gap under a key name that does
not exist, sending the operator to create the wrong variable.
Declared `null` (operator-provided) rather than guessing which manifest key
holds it: the file's own doctrine is that null means "named as a manual gap,
never auto-pushed", and promoting it to funnel-fed requires adding GITHUB_PAT
to the secrets manifest, which is operator lane. The comment says exactly
that.
The new test pins both files to each other, so a name added to one and not the
other fails instead of silently provisioning nothing. It fails against the
unfixed tree naming both offenders:
assert not {'github.personal_access_token', 'postman.api_key'}
After: every secret the gateway profile requires resolves to a DECLARED answer,
none to a guess --
dockerhub funnel key DOCKERHUB_PAT
github-official MANUAL GAP (operator-provided)
hostinger-mcp-server funnel key HOSTINGER_API_KEY
postman MANUAL GAP (operator-provided)
12 tests (was 6 before this branch).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X9mpK1ZFjexjthZbvcb94f
…tore Operator decision: github-official's PAT should flow through the funnel rather than be set by hand on each node. Checked the v2 CHIT manifest before changing anything, and the useful finding is that GITHUB_PAT was ALREADY fully funnel-managed -- `required: true`, `tier: agent`, routed to the generated env file, env.tier-agent, the GH secret, and the compose secret `pmoves_github_pat`. No manifest registration was needed; an earlier draft of this change added a redundant REGISTRY entry and it was reverted once the manifest was actually read. The gap is that the Docker MCP Toolkit keychain is a FOURTH destination, fed by nothing but this map. So the value reached every other consumer and the MCP server still came up unauthenticated -- which is why "the token is missing" looked wrong from every angle: it is present, everywhere the manifest routes it. Flipped from `null` (the conservative placeholder used before the manifest could be read) to `GITHUB_PAT`, so the hydrator pushes it instead of naming it a manual gap. Bundle impact, which is the reason to do it this way: a hand-set `docker mcp secret` lives only on the node that typed it. Routed through the funnel it travels in the CHIT bundle, so a rehydrated node comes up authenticated instead of repeating this diagnosis. Every secret the gateway profile requires now resolves to a declared answer: dockerhub dockerhub.pat_token FUNNEL key DOCKERHUB_PAT github-official github.personal_access_token FUNNEL key GITHUB_PAT hostinger-mcp-server hostinger-mcp-server.api_token FUNNEL key HOSTINGER_API_KEY postman postman.postman-api-key MANUAL GAP (operator-provided) postman stays `null` deliberately -- it is not in the secrets manifest, and declaring it funnel-fed would make the hydrator look for a key that does not exist. 12 tests pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X9mpK1ZFjexjthZbvcb94f
Codex P2, and it is the same hole as the original bug on the path the Makefile
actually documents. There are two supported ways to run the gateway and they
name the profile differently:
stdio root .mcp.json carries `--profile <p>`
SSE scripts/mcp-toolkit-gateway-listen.sh:58 reads PMOVES_MCP_PROFILE_ID
and passes it at :108
Discovery read only the first. On a host using
`make -C pmoves mcp-toolkit-gateway-start` there need be no root .mcp.json at
all, and the hydrate target passed neither --profile nor --mcp-json -- so it
resolved to an empty list and fell back to registry-only, leaving exactly the
profile-only servers uncovered that this branch exists to cover.
discover_profiles() now reads both, .mcp.json first: a config naming a running
gateway is stronger evidence than an exported shell default that may be left
over from a session which started nothing.
An UNSET PMOVES_MCP_PROFILE_ID yields nothing, deliberately. The listener script
defaults it to `pmoves_5090_web`, and mirroring that default here would
reintroduce the node-named guess this function exists to avoid -- every node
would report the 5090's profile whether or not it runs it. The stderr note now
names all three inputs so the fallback says what to do about it.
Also wired PROFILE=<id> through the make target, so the documented path can be
explicit rather than relying on discovery at all.
15 tests (was 12). The three new ones fail against the previous commit.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X9mpK1ZFjexjthZbvcb94f
POWERFULMOVES
added a commit
that referenced
this pull request
Aug 28, 2026
…tro-claim #2806 Three b850-claude lanes (docs/ytdlp-currency-reconcile #2793, docs/pmoves-yt-runbook #2801, fix/pmoves-yt-published-image #2803) were merged but still parsed as open by claim-collision-pre.py's own open_claims_in(). One RELEASE for docs/pmoves-yt-runbook already existed at line 2289 but the parser never saw it close: that RELEASE's prose backticks an unrelated branch name (`feat/upstream-sync-2026.07`, cited only to describe the PMOVES.YT submodule's pin) which the lane-matching regex picks up, switching the release from "closes everything" to "closes only named lanes" — narrowing itself without the author intending it. Documented as a durable grammar trap and closed with a supplementary RELEASE row rather than editing historical entries or the hook. Also verified the #2803 republish more strongly than the merged PR claimed possible: the merge-triggered integrations-ghcr.yml run's "Verify pushed image starts (runtime gate)" step succeeded against the real GHCR digest (still unreachable via `docker manifest inspect`/`gh api` from this node, but CI's own gate is authoritative). Also surfaced a separate, pre-existing gap: the same run is red end-to-end from 42 unaddressed HIGH-severity Trivy CVE findings, unrelated to and not caused by #2803 (baselined against the prior merge's identical failure). Retroactively CLAIMs PR #2806 (open, in flight via a delegated delivery-agent) since it and its merged sibling #2805 were pushed with no register entry — the unclaimed-work failure the steward role exists to catch. Register conflict resolved as UNION: git diff origin/main --numstat shows 15 insertions, 0 deletions on this file. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FkwiW3VY1xWmahTAtVioxz
POWERFULMOVES
added a commit
that referenced
this pull request
Aug 28, 2026
…tro-claim #2806 (#2808) Three b850-claude lanes (docs/ytdlp-currency-reconcile #2793, docs/pmoves-yt-runbook #2801, fix/pmoves-yt-published-image #2803) were merged but still parsed as open by claim-collision-pre.py's own open_claims_in(). One RELEASE for docs/pmoves-yt-runbook already existed at line 2289 but the parser never saw it close: that RELEASE's prose backticks an unrelated branch name (`feat/upstream-sync-2026.07`, cited only to describe the PMOVES.YT submodule's pin) which the lane-matching regex picks up, switching the release from "closes everything" to "closes only named lanes" — narrowing itself without the author intending it. Documented as a durable grammar trap and closed with a supplementary RELEASE row rather than editing historical entries or the hook. Also verified the #2803 republish more strongly than the merged PR claimed possible: the merge-triggered integrations-ghcr.yml run's "Verify pushed image starts (runtime gate)" step succeeded against the real GHCR digest (still unreachable via `docker manifest inspect`/`gh api` from this node, but CI's own gate is authoritative). Also surfaced a separate, pre-existing gap: the same run is red end-to-end from 42 unaddressed HIGH-severity Trivy CVE findings, unrelated to and not caused by #2803 (baselined against the prior merge's identical failure). Retroactively CLAIMs PR #2806 (open, in flight via a delegated delivery-agent) since it and its merged sibling #2805 were pushed with no register entry — the unclaimed-work failure the steward role exists to catch. Register conflict resolved as UNION: git diff origin/main --numstat shows 15 insertions, 0 deletions on this file. Claude-Session: https://claude.ai/code/session_01FkwiW3VY1xWmahTAtVioxz Co-authored-by: Claude Opus 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
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.
The GitHub MCP server returns 401 Bad credentials on the 4090, and the hydrator reported nothing wrong — because it reads a different list of servers than the gateway serves.
github-officialis in the profile and not in that registry, so its secret was never considered. Not pushed — and not reported as a gap either, which is the worse half: the tool's contract says unmanaged secrets are "reported by name, never guessed", and this one was invisible to both paths. The server started with an emptyGITHUB_PERSONAL_ACCESS_TOKENand failed at call time instead of at hydrate time.Measured, same command, this node
e2b,hostingergithub-official, +dockerhub, +postmanThree servers were silently unprovisioned, not one.
Why profiles come from
.mcp.jsonmcp-toolkit-connect.sh:31andmcp-toolkit-bootstrap.sh:14-15both default topmoves_5090_web— a profile named after one node. Trusting that default would report the same profile on every node regardless of what its gateway was actually started with, so discovery reads the gateway's real invocation. With no.mcp.jsonit returns[]and says so on stderr: absent config is not a licence to guess.load_requiredunions both sources and dedups on(server, secret), so a server in both is one requirement rather than two calls into the resolver — the fragile part of this path.What this does NOT do
It makes the gap visible; it does not by itself clear the 401.
docker mcp secret setgoes through the same resolver currently returningdeadline_exceededon this node, so the documented order still holds: restart Docker Desktop → confirm the resolver answers → hydrate.10 tests (was 6); the four new ones fail against the unfixed tool.
🤖 Generated with Claude Code
https://claude.ai/code/session_01X9mpK1ZFjexjthZbvcb94f