feat(mcp): gateway preflight + KVM-primary topology in the runbook - #2806
Merged
Merged
Conversation
`make -C pmoves mcp-toolkit-gateway-start` verified that Docker was present and
the profile imported, then started a gateway. It never checked the SECRET
RESOLVER. On 2026-08-28 the resolver was wedged and the gateway started anyway,
serving github-official with an empty token; every call returned 401, and
nothing at start time said why. The failure surfaced at call time, in a
different tool, to a different person.
Note the asymmetry this closes: the COMPOSE gateway already fails closed
(`up-mcp-gateway` depends on `mcp-gateway-preflight`, which requires the
network and a running BoTZ bridge). The Toolkit gateway had no equivalent.
Written as a Python tool, not the shell script I first drafted, because the
repo already has this shape and the shell version could not be tested here:
- tools/docker_host_policy_check.py is the pattern -- a `_docker()` helper
raising Unmeasured, monkeypatched in tests, with "could not measure exits 3,
NOT 0" as stated doctrine. Copied rather than reinvented.
- a PATH-stubbed `docker` cannot work on Windows: Git Bash resolves `docker`
to `docker.exe` and never sees an extensionless stub. My first draft
"passed" against the REAL daemon's live profile list instead of its fixture,
which is precisely the decorative-green this repo keeps finding. I had also
invented a PMOVES_DOCKER_BIN seam for it; no such convention existed.
- Python runs on every node class including the arm64 ones (SPARK, Jetson,
KVMs), where a Git-Bash-shaped test would not.
Exit codes follow the house doctrine: 0 ready, 1 measured a problem, 3 could not
measure (NOT a pass), 4 usage. The listener treats non-zero as ADVISORY -- a
wedged resolver breaks only credentialed servers, and refusing to start 25
because 4 cannot authenticate is worse than the problem -- with
PMOVES_MCP_STRICT=1 to fail closed for CI and unattended bring-up.
Verified live against this node's actually-wedged resolver, exit 1:
WARN: the Docker MCP secret resolver is NOT answering.
hostinger-mcp-server needs hostinger-mcp-server.api_token
dockerhub needs dockerhub.pat_token
github-official needs github.personal_access_token
postman needs postman.postman-api-key
9 tests, stubbed, no daemon: healthy passes, wedged exits 1 and names the
servers, a secretless server is NOT listed, a `secret ls` failure is a FINDING
rather than an absence, a missing profile and a missing docker are both exit 3,
and a wiring assertion fails if the listener stops calling it.
force_utf8_stdio() from _secrets_common, because the recovery line's em-dash
came out as a replacement char on the cp1252 console.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X9mpK1ZFjexjthZbvcb94f
The runbook documented the per-node Docker Desktop bootstrap as though it were
the whole story. Operator correction 2026-08-28: the KVM-hosted gateway is
PRIMARY so every node can reach one endpoint, and Desktop is the fallback. The
doc now says which is which, and section 0 is deliberately first so nobody
reaches the Desktop bootstrap thinking it is the target state.
New section 0 — topology:
- KVM gateway: PRIMARY, and NOT YET DEPLOYED. Verified 2026-08-27 across 25+
ports on kvm2 / kvm4-1 / kvm4-2: no MCP server runs on any KVM today.
- Docker Desktop Toolkit: fallback and workstation dev — needs a GUI session
and carries a per-node secret keychain that is a per-node failure point.
- Hostinger / Cloudflare / Tailscale: reachable directly by CLI and MCP,
dependent on neither gateway.
Placement carries the measured capacity rather than a preference: kvm4-1 is the
best fit (8C/16GB, ~6GB headroom, already the API/agent tier), kvm2 suits light
public SSE behind its nginx (~7.5GB free), and kvm4-2 must take nothing until
its over-subscription is fixed (~29GB of declared limits on a 16GB host). The
substrate to clone is #2665's Docker MCP Gateway pipeline (:8189, --static); the
BoTZ :8052 draft is superseded for federation, being K8s-only.
Section 0.1 records that the fleet bootstraps to `pmoves_5090_web` — a profile
named for one workstation — because that name reads as intent to the next person
and is not. It works; it is just misnamed, and a fleet bundle belongs with the
fleet gateway.
New section 5.1 — resolver recovery. There was NO runbook entry for this at all:
searching the repo for `deadline_exceeded` / `ResolverService` found only two
code comments and an unrelated SDK. Now recorded with the distinguishing
symptom (four of five `docker mcp` subsystems answer, only `secret ls` times
out), both occurrences (5090 2026-08-17, 4090 2026-08-28), the fact that the
4090 case SURVIVED a Desktop restart so a restart alone is not the fix, and an
ordered recovery ending in a funnel re-push that rotates nothing.
Two things it is careful to say: the Cloudflare 13 are OAuth-mediated and stay
unauthenticated after any hydrate, which is expected rather than a regression;
and a "funnel gap" can mean a stale `env.shared` aggregate rather than an absent
value, since GITHUB_PAT materializes into env.tier-agent — pointing the hydrator
at the tier file tells those apart, and only a value missing from both needs
secrets-rotate.
Verified: the doc's own stated reconcile signal holds (live profile 25 servers,
section 2 claims 25), and no IP addresses were introduced.
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 |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![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: ef9f555bd1
ℹ️ 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".
…emory
The identity half was already wired -- claude-pmoves.sh resolves node 4090 ->
identity claude_4090 and, correctly, puts it in --append-system-prompt because
"exported variables do not reach the model's context". Cipher had no equivalent,
and Cipher is the half that fails silently.
An MCP server that never connects contributes no tools. A session with no memory
therefore looks exactly like a session with memory and nothing to recall, and
the agent cannot tell which it is. Every recalled fact is simply missing and
nothing says so.
cipher_preflight.py answers it before launch and the launcher appends the
verdict to the system prompt, so the documented fallback ("check Cipher at
startup; fall back to auto-memory if down") is something the agent can actually
act on rather than a rule it cannot evaluate.
It reports WHICH endpoint answered, not merely that one did. The roster carries
a fleet cipher via ${TS_Z890} and a local one, and #2792 exists because it once
carried only the fleet entry -- "memory that silently wasn't there" whenever
Z890 was unreachable. Measured on this node right now:
cipher DOWN pmoves-cipher ${TS_Z890} unresolved (tailnet helper not run)
cipher OK pmoves-cipher-local http://localhost:8105/mcp/sse -> 200
Exit 0, and the prompt names `pmoves-cipher-local` -- so "memory is up" cannot
quietly mean "someone else's memory is up".
SSE is why this could not be a naive health check. `/mcp/sse` never closes its
body: measured on the 4090, a check that waits for completion returns HTTP 200
after exactly the timeout, because the deadline ended it rather than the server.
A 10s budget reads a healthy Cipher as a 10s hang. This reads the status line
and stops.
Exit codes follow docker_host_policy_check.py: 0 reachable, 1 every endpoint
reached and none answered, 3 could not measure (no cipher entry at all) -- NOT a
pass. The launcher NEVER blocks on any of them: a session without memory is
degraded, not unusable, and refusing to launch would be worse than launching
informed.
10 tests, network stubbed: an unexpanded ${TS_Z890} is reported rather than
dialled as a hostname, one endpoint up is enough, an underscore-prefixed entry
is skipped (that is the repo's real off-switch, and the legacy python wrapper is
a broken duplicate), a 404 is distinguished from absence because something IS
listening, and a wiring assertion fails if the launcher stops calling it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X9mpK1ZFjexjthZbvcb94f
Three review findings on #2806, all the same shape: a check reporting success while doing nothing. 1. `check()` computed `servers_requiring_secrets(profile)` — exactly the list that matters — and then set `ok` from `resolver_healthy()` alone, never consulting it. A resolver that answers over a store where a secret was never hydrated (or was later wiped) returned ok:true, strict startup and CI passed, and the server still came up uncredentialed and 401'd at call time: the precise failure the module docstring says it exists to catch. Readiness is now the conjunction. Coverage is read from `docker mcp secret ls --json`, not the table form — the vendor writer (cmd/docker-mcp/commands/secret.go) prints the table via `PrettyPrintTable(rows, []int{40, 120})` with NO header and a 40-column TRUNCATION over namespace-qualified IDs, so comparing against it yields a confident wrong "missing" verdict. A store that cannot be enumerated raises Unmeasured (exit 3), never an empty list of problems; a store enumerated and short a required secret is a measured failure (exit 1), named by server and secret NAME. 2. The listener invoked bare `python3`, which exits 127 on nodes where Python is `py -3` or lives only in .venv-pmoves, and fails on hosts whose system Python lacks PyYAML although the canonical venv has it. Non-strict startup then warned and launched the gateway unchecked; strict rejected a healthy gateway. It now sources pm-python.sh — the ONE discovery, shared with claude-pmoves.sh and crush-pmoves — with the `yaml` probe, and maps "could not run the preflight at all" to exit 3 rather than sharing the exit-1 bucket with real findings. A missing preflight tool is likewise exit 3; that branch used to start unchecked even under PMOVES_MCP_STRICT=1. 3. CodeQL 376 (py/clear-text-logging-sensitive-data) is a false positive: vendor pkg/catalog/types.go defines `Secret{Name, Env}` with no value field, so `docker mcp profile show` has nothing to serialise a value into, and `secret ls` is value-free at both verbosities. Values are fetched by the gateway from the ResolverService at container start and never transit this tool. Dismissed with that justification. A runtime name-shape guard (`_reportable_secret_name`, mirroring emit_local_env._reportable_name) is added at the sink regardless, covering the one input the vendor schema does not constrain — a hand-authored profile — in both text and --json output. Tests: +25 (9 -> 34). All 25 fail against 2631efa and pass here. Four are behavioural rather than grep-the-script-body, exercising the listener's exit-code contract against a stubbed `docker` (skipped on Windows, where a PATH stub is bypassed). The `secret ls` fixture was corrected: it carried a `NAME` header the vendor never emits. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FkwiW3VY1xWmahTAtVioxz
The resolver-wedged and secret-missing paths each carried their own copy of
the server/secret-name print loop. One writer means one guard to audit, one
format for operators to learn, and no way for the two paths to drift — and it
drops the CodeQL sink count for py/clear-text-logging-sensitive-data from 3
back to 2 (the remaining pair being this writer and the --json dump).
Both are the same established false positive as alert 376: vendor
docker/mcp-gateway pkg/catalog/types.go defines Secret{Name, Env} with no value
field, so a secret VALUE cannot reach either sink through
`docker mcp profile show`.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FkwiW3VY1xWmahTAtVioxz
Owner
Author
Independent review — Control Body, second passFull report retained at Reviewer wrote neither commit. Read-only worktree at Verdict: APPROVE-WITH-NITS
Negative control (the evidence that makes 443 new test lines mean anything): new tests against the reverted module →
Two suspicions I raised that were closed by execution, not argument
The stderr leak path — closed from vendor source
Safe as written. Honest bound stated: audited CodeQL 376's dismissal independently confirmed against P2 —
|
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
5 tasks
POWERFULMOVES
added a commit
that referenced
this pull request
Aug 28, 2026
…support Two Codex findings on #2810, both correct. P1 -- no TTL. AGENTS.md requires branch + scope + TTL and this row carried branch and scope. Added, as an explicit instant. Noted without deflection that the same omission was caught on #2809 an hour earlier, which makes this the second instance in one session of the very convention whose 7% compliance rate that PR documented. P2 -- the row asserted all 20 merges were "bypassed every time through ADMIN_REVIEW_BYPASS". That was not measured. What was measured is that no approving review existed on any of them; the bypass PATH was inferred from knowing the path this session used, and then written as if checked. This is the inferring-a-referent-and-treating-it-as-checked failure recorded as this identity's most frequent one, committed inside a claim about governance being asserted rather than verified. Narrowed to the demonstrated claim, and the attempt to support the stronger one is itself the better finding: * ADMIN_REVIEW_BYPASS is confirmed for exactly one merge, #2806, run here * mergedBy is POWERFULMOVES on all of them -- an account, not a path * gh run list --workflow=pr-closeout.yml returns ZERO runs So the guarded, fail-closed target left no trace for any of the 20. There is no audit trail separating "merged through pr-closeout with a named reviewing body" from "merged by any other admin path". A bypass that cannot be distinguished from a different bypass cannot be made auditable by requiring it to name the reviewing body -- the record has to exist before it can carry a name. That constraint lands on this lane's own deliverable and would have been discovered late, during design, rather than now. 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>
POWERFULMOVES
added a commit
that referenced
this pull request
Aug 28, 2026
* docs(agnote): claim review-gate topology lane Branch protection's required-approval gate has a measured 0% pass rate and 100% bypass rate (20/20 merged PRs, #2782-#2806, 0 approvals, author=POWERFULMOVES) because every PR in this fleet is authored by the same account and GitHub forbids self-approval. Files one CLAIM in the register scoping investigation + a design-doc proposal, signed via the pmoves-chit-sign skill. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * docs(agnote): add the TTL, and retract a mechanism claim I could not support Two Codex findings on #2810, both correct. P1 -- no TTL. AGENTS.md requires branch + scope + TTL and this row carried branch and scope. Added, as an explicit instant. Noted without deflection that the same omission was caught on #2809 an hour earlier, which makes this the second instance in one session of the very convention whose 7% compliance rate that PR documented. P2 -- the row asserted all 20 merges were "bypassed every time through ADMIN_REVIEW_BYPASS". That was not measured. What was measured is that no approving review existed on any of them; the bypass PATH was inferred from knowing the path this session used, and then written as if checked. This is the inferring-a-referent-and-treating-it-as-checked failure recorded as this identity's most frequent one, committed inside a claim about governance being asserted rather than verified. Narrowed to the demonstrated claim, and the attempt to support the stronger one is itself the better finding: * ADMIN_REVIEW_BYPASS is confirmed for exactly one merge, #2806, run here * mergedBy is POWERFULMOVES on all of them -- an account, not a path * gh run list --workflow=pr-closeout.yml returns ZERO runs So the guarded, fail-closed target left no trace for any of the 20. There is no audit trail separating "merged through pr-closeout with a named reviewing body" from "merged by any other admin path". A bypass that cannot be distinguished from a different bypass cannot be made auditable by requiring it to name the reviewing body -- the record has to exist before it can carry a name. That constraint lands on this lane's own deliverable and would have been discovered late, during design, rather than now. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FkwiW3VY1xWmahTAtVioxz --------- 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
2 participants
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.
Two halves of one problem:
mcp-toolkit-gateway-startcould not tell you it was about to serve 401s, and the runbook described the fallback as though it were the target.The check
make -C pmoves mcp-toolkit-gateway-startverified Docker was present and the profile imported, then started a gateway. It never checked the secret resolver. On 2026-08-28 the resolver was wedged and the gateway started anyway — servinggithub-officialwith an empty token. Every call returned 401, and nothing at start time said why.That closes a real asymmetry: the compose gateway already fails closed (
up-mcp-gateway→mcp-gateway-preflight). The Toolkit gateway had no equivalent.Verified live against this node's actually-wedged resolver, exit 1:
Why Python, not the shell script I first wrote
tools/docker_host_policy_check.pyis the existing pattern — a_docker()helper raisingUnmeasured, monkeypatched in tests, with "could not measure exits 3, NOT 0" as stated doctrine. Copied rather than reinvented.dockercannot work on Windows: Git Bash resolvesdocker→docker.exeand never sees an extensionless stub. My first draft "passed" against the real daemon's live profile list instead of its fixture — precisely the decorative-green this repo keeps finding. I had also invented aPMOVES_DOCKER_BINseam; no such convention existed.Advisory by default — a wedged resolver breaks only credentialed servers, and refusing to start 25 because 4 cannot authenticate is worse than the problem.
PMOVES_MCP_STRICT=1fails closed for CI.9 tests, stubbed, no daemon — including that a
secret lsfailure is a finding not an absence, and a wiring assertion that fails if the listener stops calling it.The runbook
Sections 2–4 documented the Desktop bootstrap as the whole story. Per operator direction, KVM is primary so every node reaches one endpoint; Desktop is fallback.
Placement carries measured capacity: kvm4-1 best fit, kvm2 for light public SSE, kvm4-2 takes nothing until its ~29GB-limits-on-16GB over-subscription is fixed.
§5.1 resolver recovery — there was no runbook entry at all. Searching the repo found only two code comments and an unrelated SDK. Now recorded with the distinguishing symptom (4 of 5
docker mcpsubsystems answer, onlysecret lstimes out), both occurrences, the fact the 4090 case survived a Desktop restart, and an ordered recovery that rotates nothing.§0.1 records that the fleet bootstraps to
pmoves_5090_web— a profile named for one workstation. It works; it's misnamed, and a fleet bundle belongs with the fleet gateway.Doc's own reconcile signal verified (live 25 servers = §2's claim). No IPs introduced.
🤖 Generated with Claude Code
https://claude.ai/code/session_01X9mpK1ZFjexjthZbvcb94f