feat(mcp/spark): canonical MCP inventory, OpenClaw scopes, and SPARK node matrix - #2126
Conversation
📝 WalkthroughWalkthroughAdds a canonical MCP inventory and generator, bootstraps MCP configurations across clients and node scopes, introduces validation and backup behavior, expands runtime MCP registrations, and adds a KiloCode TensorZero variant while removing a duplicate legacy declaration. ChangesMCP inventory and generation
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Bootstrap as mcp-bootstrap
participant Toolkit as MCP Toolkit
participant Generator as mcp_config_generator
participant Configs as Client and scope configs
Bootstrap->>Toolkit: bootstrap toolkit profile
Bootstrap->>Generator: generate canonical MCP configurations
Generator->>Configs: merge MCP servers and create backups
Bootstrap->>Configs: run configuration checks
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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: 7d13e26903
ℹ️ 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".
…tstrap - Add pmoves/config/mcp_inventory.json as single source of truth - Add pmoves/tools/mcp_config_generator.py to render client-native configs - Wire MCP bootstrap into pmoves/scripts/bootstrap-node.sh and Makefile - Add hermes-crush bootstrap script and make targets - Update Claude, Kimi, KiloCode, Hermes, Crush, and OpenCode configs - Add pytest suite for the generator - Add kilo parity check tooling - Docs updated in MCP_TOOLKIT.md
- Extend mcp_inventory.json with endpoint_prefix so fleet/local URLs resolve cleanly - Fix mcp_config_generator.py endpoint lookup to honor caller endpoint preference - Add pmoves/tools/bootstrap_openclaw_scopes.py with full/edge tier policy - Update all 8 pmoves/configs/claws/scopes/*.json configs - full nodes (4090, 5090, z890, kvm4-1, kvm4-2, nemotron-claw): full PMOVES MCP set - edge nodes (nemoclaw, kvm2): cipher + agent-zero + tailscale - preserve scope-specific MCPs (gpu-mesh, docker, zai-*, etc.) - Add openclaw-scope-bootstrap and openclaw-scope-check make targets - Extend test_mcp_config_generator.py with endpoint + scope tests (17 passing)
KVMs act as Tailscale exit nodes and connect to GPU-bearing nodes, so every KVM gets the full PMOVES MCP set per operator clarification. - Move kvm2 from edge to full tier in bootstrap_openclaw_scopes.py - Regenerate pmoves/configs/claws/scopes/kvm2.json with full PMOVES MCP set
…ensorZero TOML duplicates
… fallback 1. DEFAULT_OUTPUTS: add opencode entry (was missing — KeyError on --client all) 2. bootstrap-hermes-crush.sh: add export PYTHONPATH for python3 -m pmoves.tools.* 3. .kimi/mcp.json: SUPABASE_SERVICE_KEY -> canonical with fallback (same fixes as PR #2124, applied to this branch)
Addresses Codex P1, Codex P2, CodeRabbit Critical + Major findings:
P1 (security): mcp_config_generator._expand() now accepts
allow_os_environ flag. Tracked configs (repo-relative claude/kimi/
kilocode/opencode) generate with allow_os_environ=False, preserving
${VAR} placeholders instead of expanding real secret values from
os.environ. Only --set values and local configs (crush/hermes in
~/.config or ~/.hermes) expand from os.environ.
Critical (hermes): --client hermes now writes to a .mcp_snippet.json
sidecar instead of overwriting the real config.yaml. The bootstrap
script merges the snippet into config.yaml surgically via PyYAML.
Major (--client all): --client all with --output is now rejected
(exit 2) with a message requiring --output-dir.
P2 (PYTHONPATH): bootstrap-hermes-crush.sh already exports
PYTHONPATH at line 33 (was added in a prior commit).
Major (bootstrap fail): script now exits 1 on Crush generator failure
or missing PyYAML, instead of silently succeeding.
💘 Generated with Crush
Assisted-by: Crush:glm-5.2
2644966 to
2073863
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 18
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@kilo.json`:
- Around line 146-158: Update the pmoves-supabase configuration’s --apiKey value
to support the SUPABASE_SERVICE_ROLE_KEY fallback used by the corresponding Kimi
configuration, while retaining SUPABASE_SERVICE_KEY as the primary variable.
Preserve the existing PostgREST command and other arguments unchanged.
In `@pmoves/config/mcp_inventory.json`:
- Around line 78-88: Update the supabase-db configuration in mcp_inventory.json
and the corresponding generation logic in crush_configurator.py to use
--access-mode=restricted by default. Ensure generated client configurations
remain read-only, and only retain unrestricted access where it is explicitly
scoped to trusted clients.
In `@pmoves/docs/operations/MCP_TOOLKIT.md`:
- Line 236: Correct the target location statement in the MCP toolkit
documentation to identify pmoves/mk/mcp-toolkit.mk as the file defining the
targets and pmoves/Makefile as the file that includes it; leave the script
locations unchanged.
In `@pmoves/mk/kilo.mk`:
- Around line 100-107: Update the MCP parity check invocation in the Make target
to capture the exit status returned by pmoves.tools.kilo_parity_mcp_check rather
than assigning command-substitution stdout to mcp_gaps. Preserve the existing
gaps accumulation, blocked-item reporting, and final exit behavior.
In `@pmoves/mk/mcp-toolkit.mk`:
- Around line 6-8: Update the mutation warning comment near mcp-toolkit-connect
to include mcp-config-bootstrap, OpenCode/OpenClaw bootstrap, and Hermes/Crush
bootstrap as targets that modify client configuration files, and remove the
claim that mcp-toolkit-connect is the only mutating target while preserving the
explicit-authorization guidance.
- Around line 107-109: Update mcp-bootstrap in pmoves/mk/mcp-toolkit.mk to
preflight Toolkit availability, skip only when Toolkit is absent, and propagate
failures from profile import, network, or bootstrap scripts when it is
installed; do not use unconditional || true. In pmoves/Makefile lines 897-898,
call the corrected mcp-bootstrap umbrella target or apply the same
selective-skip behavior.
- Around line 123-153: Update the configuration validation recipe around the
existing file and grep checks to invoke openclaw-scope-check for generated
OpenClaw files and the repository’s canonical JSON validators for every
generated MCP configuration. Ensure validation fails on malformed JSON and
unresolved placeholders such as ${...} or your_*_here, while retaining
exhaustive checks for all generated scopes and canonical MCP entries.
In `@pmoves/scripts/bootstrap-hermes-crush.sh`:
- Line 79: Update the configuration merge in the mcp_servers setup to remove
previously managed PMOVES/Hermes entries before applying the current mcp_servers
inventory. Preserve unrelated user-defined entries, while replacing all
canonical inventory keys so renamed or removed servers do not remain.
In `@pmoves/scripts/bootstrap-node.sh`:
- Around line 243-248: Update the MCP bootstrap block in the node setup flow so
a nonzero result from the mcp-bootstrap target propagates as a failure instead
of allowing setup to continue successfully. Preserve the existing success
message, but replace the current warning-only else path with the script’s
established failure/exit behavior; only retain best-effort continuation if it is
explicitly enabled by an existing configuration option.
In `@pmoves/scripts/mcp-toolkit-bootstrap.sh`:
- Around line 70-74: Update the mcp-config-bootstrap invocation in the PMOVES
bootstrap flow to fail immediately when make returns a nonzero status. Remove
the warning-and-continue branch so canonical MCP generation errors propagate as
a bootstrap failure instead of reporting success with stale configurations.
In `@pmoves/tests/test_mcp_config_generator.py`:
- Around line 325-332: Update the scope test setup around scopes.main([]) to
restore both scopes.SCOPES_DIR and scopes.INVENTORY_PATH after each test,
including the additional affected test blocks. Prefer monkeypatch for both
module globals, or save their original values and restore them in the existing
finally cleanup.
In `@pmoves/tools/bootstrap_openclaw_scopes.py`:
- Around line 79-84: Disable OS-environment expansion in both tracked bootstrap
generator calls: update generate_for_client in
pmoves/tools/bootstrap_openclaw_scopes.py lines 79-84 and
pmoves/tools/bootstrap_opencode.py lines 37-43 to pass allow_os_environ=False,
preserving the existing inventory, endpoint, and context arguments.
- Around line 138-151: Update pmoves/tools/bootstrap_openclaw_scopes.py lines
138-151 to validate each PMOVES MCP entry’s complete definition against
canonical_scope_mcp_servers(), not just key presence; retain missing and
unexpected-entry reporting while detecting incorrect values. Update
pmoves/tools/kilo_parity_mcp_check.py lines 15-31 to compare generated KiloCode
MCP entries and their permissions against kilo.json, including configuration
values rather than only keys.
In `@pmoves/tools/crush_configurator.py`:
- Around line 196-223: Add TS_Z890 to the required_env declarations for the
remote pmoves-cipher and agent-zero MCPSpec entries, while leaving
pmoves-cipher-local dependent only on CIPHER_API_TOKEN. Ensure configurations
with missing TS_Z890 are not generated.
- Around line 242-260: Update the pmoves-supabase MCPSpec to use the MCPSpec
any-of environment requirement, requiring SUPABASE_SERVICE_ROLE_KEY or
SUPABASE_SERVICE_KEY before enabling the spec. Preserve the existing command,
arguments, timeout, and required_commands configuration.
- Around line 174-182: Update missing_envs to treat known placeholder-pattern
values, including your_*_here, as missing in addition to empty or absent values.
Add exhaustive placeholder validation within the existing _lookup_env-based
check, and return each affected key so configuration cannot pass with unresolved
placeholders.
In `@pmoves/tools/mcp_config_generator.py`:
- Around line 113-120: The recursive default expansion in `_expand` must
preserve the caller’s `allow_os_environ` value instead of re-enabling
process-environment lookups. Pass that policy through nested expansion so
tracked generation cannot resolve nested fallbacks from `os.environ` when
environment access is disabled.
- Around line 278-283: Update the stdio branch in render_crush() to include
spec.env in the generated entry, preserving the existing command and argument
rendering while ensuring environment variables are passed through to Crush stdio
servers.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 73ea5a99-dc07-4053-be4f-24cb5079546b
📒 Files selected for processing (43)
.claude/mcp.json.gitignore.kimi/mcp.jsonkilo.jsonpmoves/Makefilepmoves/config/mcp_inventory.jsonpmoves/config/profiles/hermes/4090.yamlpmoves/config/profiles/hermes/5090.yamlpmoves/config/profiles/hermes/b850.yamlpmoves/config/profiles/hermes/elder-melchor.yamlpmoves/config/profiles/hermes/kvm4-1.yamlpmoves/config/profiles/hermes/spark.yamlpmoves/config/profiles/hermes/z890.yamlpmoves/configs/claws/opencode-4090.jsonpmoves/configs/claws/opencode-5090.jsonpmoves/configs/claws/opencode-kvm2.jsonpmoves/configs/claws/opencode-kvm4-1.jsonpmoves/configs/claws/opencode-kvm4-2.jsonpmoves/configs/claws/opencode-nemoclaw.jsonpmoves/configs/claws/opencode-nemotron-claw.jsonpmoves/configs/claws/opencode-spark.jsonpmoves/configs/claws/scopes/4090.jsonpmoves/configs/claws/scopes/5090.jsonpmoves/configs/claws/scopes/kvm2.jsonpmoves/configs/claws/scopes/kvm4-1.jsonpmoves/configs/claws/scopes/kvm4-2.jsonpmoves/configs/claws/scopes/nemoclaw.jsonpmoves/configs/claws/scopes/nemotron-claw.jsonpmoves/configs/claws/scopes/spark.jsonpmoves/configs/claws/scopes/z890.jsonpmoves/docs/operations/MCP_TOOLKIT.mdpmoves/mk/kilo.mkpmoves/mk/mcp-toolkit.mkpmoves/scripts/bootstrap-hermes-crush.shpmoves/scripts/bootstrap-node.shpmoves/scripts/mcp-toolkit-bootstrap.shpmoves/tensorzero/config/tensorzero.tomlpmoves/tests/test_mcp_config_generator.pypmoves/tools/bootstrap_openclaw_scopes.pypmoves/tools/bootstrap_opencode.pypmoves/tools/crush_configurator.pypmoves/tools/kilo_parity_mcp_check.pypmoves/tools/mcp_config_generator.py
| "pmoves-supabase": { | ||
| "type": "local", | ||
| "command": [ | ||
| "npx", | ||
| "-y", | ||
| "@supabase/mcp-server-postgrest@0.1.1", | ||
| "--apiUrl", | ||
| "http://localhost:8000/rest/v1", | ||
| "--apiKey", | ||
| "${SUPABASE_SERVICE_KEY}", | ||
| "--schema", | ||
| "public" | ||
| ] |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Preserve the Supabase service-role-key fallback.
Line 155 accepts only SUPABASE_SERVICE_KEY, unlike the corresponding Kimi configuration. Environments exposing only SUPABASE_SERVICE_ROLE_KEY will fail authentication.
Proposed fix
- "${SUPABASE_SERVICE_KEY}",
+ "${SUPABASE_SERVICE_ROLE_KEY:-${SUPABASE_SERVICE_KEY}}",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "pmoves-supabase": { | |
| "type": "local", | |
| "command": [ | |
| "npx", | |
| "-y", | |
| "@supabase/mcp-server-postgrest@0.1.1", | |
| "--apiUrl", | |
| "http://localhost:8000/rest/v1", | |
| "--apiKey", | |
| "${SUPABASE_SERVICE_KEY}", | |
| "--schema", | |
| "public" | |
| ] | |
| "pmoves-supabase": { | |
| "type": "local", | |
| "command": [ | |
| "npx", | |
| "-y", | |
| "`@supabase/mcp-server-postgrest`@0.1.1", | |
| "--apiUrl", | |
| "http://localhost:8000/rest/v1", | |
| "--apiKey", | |
| "${SUPABASE_SERVICE_ROLE_KEY:-${SUPABASE_SERVICE_KEY}}", | |
| "--schema", | |
| "public" | |
| ] |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@kilo.json` around lines 146 - 158, Update the pmoves-supabase configuration’s
--apiKey value to support the SUPABASE_SERVICE_ROLE_KEY fallback used by the
corresponding Kimi configuration, while retaining SUPABASE_SERVICE_KEY as the
primary variable. Preserve the existing PostgREST command and other arguments
unchanged.
| "key": "supabase-db", | ||
| "description": "Supabase Postgres database", | ||
| "transport": "stdio", | ||
| "command": "uvx", | ||
| "args": [ | ||
| "postgres-mcp@0.3.0", | ||
| "--access-mode=unrestricted" | ||
| ], | ||
| "env": { | ||
| "DATABASE_URI": "${SUPABASE_DB_URI}" | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n -C3 -- '--access-mode=(unrestricted|restricted)' .Repository: POWERFULMOVES/PMOVES.AI
Length of output: 9458
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== mcp_inventory context =="
ast-grep outline pmoves/config/mcp_inventory.json || true
echo
echo "== related references =="
rg -n -C 2 -- 'mcp_inventory\.json|supabase-db|postgres-mcp@0\.3\.0|--access-mode=' pmoves .github README.md 2>/dev/null || true
echo
echo "== file sizes =="
wc -l pmoves/config/mcp_inventory.json pmoves/tools/crush_configurator.py 2>/dev/null || trueRepository: POWERFULMOVES/PMOVES.AI
Length of output: 50380
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== where mcp_inventory.json is consumed =="
rg -n -C 2 -- 'mcp_inventory\.json|mcp_inventory|inventory' pmoves .github README.md 2>/dev/null | head -n 200
echo
echo "== inspect the inventory file around the target entry =="
sed -n '60,100p' pmoves/config/mcp_inventory.json
echo
echo "== inspect crush_configurator around postgres-mcp entry =="
sed -n '240,290p' pmoves/tools/crush_configurator.pyRepository: POWERFULMOVES/PMOVES.AI
Length of output: 21640
Make the shared MCP DB default read-only pmoves/config/mcp_inventory.json and pmoves/tools/crush_configurator.py both pin supabase-db to --access-mode=unrestricted, so generated client configs still get write access. Switch the shared default to restricted, or scope the writable variant to trusted clients only.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pmoves/config/mcp_inventory.json` around lines 78 - 88, Update the
supabase-db configuration in mcp_inventory.json and the corresponding generation
logic in crush_configurator.py to use --access-mode=restricted by default.
Ensure generated client configurations remain read-only, and only retain
unrestricted access where it is explicitly scoped to trusted clients.
| | `make hermes-crush-bootstrap` | Updates Hermes Agent `~/.hermes/profiles/pmoves-hermes/config.yaml` and Crush CLI `~/.config/crush/crush.json` MCP sections from the inventory. | | ||
| | `make opencode-bootstrap` | Updates all `pmoves/configs/claws/opencode-*.json` node configs with canonical PMOVES MCPs (preserves existing zai/docker entries). | | ||
|
|
||
| Targets live in `pmoves/Makefile` (included from `pmoves/mk/mcp-toolkit.mk`). Scripts live in `pmoves/scripts/mcp-toolkit-*.sh` and `pmoves/scripts/bootstrap-hermes-crush.sh`. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Correct the target source location.
These targets are defined in pmoves/mk/mcp-toolkit.mk, which is included by pmoves/Makefile, not the reverse.
Proposed correction
-Targets live in `pmoves/Makefile` (included from `pmoves/mk/mcp-toolkit.mk`).
+Targets are defined in `pmoves/mk/mcp-toolkit.mk` and included by `pmoves/Makefile`.As per path instructions, “Check docs for operational accuracy.”
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Targets live in `pmoves/Makefile` (included from `pmoves/mk/mcp-toolkit.mk`). Scripts live in `pmoves/scripts/mcp-toolkit-*.sh` and `pmoves/scripts/bootstrap-hermes-crush.sh`. | |
| Targets are defined in `pmoves/mk/mcp-toolkit.mk` and included by `pmoves/Makefile`. Scripts live in `pmoves/scripts/mcp-toolkit-*.sh` and `pmoves/scripts/bootstrap-hermes-crush.sh`. |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pmoves/docs/operations/MCP_TOOLKIT.md` at line 236, Correct the target
location statement in the MCP toolkit documentation to identify
pmoves/mk/mcp-toolkit.mk as the file defining the targets and pmoves/Makefile as
the file that includes it; leave the script locations unchanged.
Source: Path instructions
| mcp_gaps=$$(PYTHONPATH="$(CURDIR)/.." $(PYTHON) -m pmoves.tools.kilo_parity_mcp_check); \ | ||
| gaps=$$((gaps + mcp_gaps)); \ | ||
| if [ $$blocked -gt 0 ]; then \ | ||
| echo "[*] Results: $$gaps gap(s) found, $$blocked item(s) blocked by platform (expected)"; \ | ||
| else \ | ||
| echo "[*] Results: $$gaps gap(s) found"; \ | ||
| fi; \ | ||
| if [ $$blocked -gt 0 ]; then \ | ||
| echo "[!] Hook implementation blocked — see .kilo/hooks/damage-control/README.md for activation plan"; \ | ||
| fi; \ | ||
| exit $$gaps |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Capture the checker’s exit status, not its stdout.
kilo_parity_mcp_check.main() returns the missing count as its exit status and prints only to stderr. Command substitution therefore leaves mcp_gaps empty, so MCP gaps are never counted.
Proposed fix
- mcp_gaps=$$(PYTHONPATH="$(CURDIR)/.." $(PYTHON) -m pmoves.tools.kilo_parity_mcp_check); \
+ PYTHONPATH="$(CURDIR)/.." $(PYTHON) -m pmoves.tools.kilo_parity_mcp_check; \
+ mcp_gaps=$$?; \
gaps=$$((gaps + mcp_gaps)); \📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| mcp_gaps=$$(PYTHONPATH="$(CURDIR)/.." $(PYTHON) -m pmoves.tools.kilo_parity_mcp_check); \ | |
| gaps=$$((gaps + mcp_gaps)); \ | |
| if [ $$blocked -gt 0 ]; then \ | |
| echo "[*] Results: $$gaps gap(s) found, $$blocked item(s) blocked by platform (expected)"; \ | |
| else \ | |
| echo "[*] Results: $$gaps gap(s) found"; \ | |
| fi; \ | |
| if [ $$blocked -gt 0 ]; then \ | |
| echo "[!] Hook implementation blocked — see .kilo/hooks/damage-control/README.md for activation plan"; \ | |
| fi; \ | |
| exit $$gaps | |
| mcp_gaps=$$(PYTHONPATH="$(CURDIR)/.." $(PYTHON) -m pmoves.tools.kilo_parity_mcp_check); \ | |
| gaps=$$((gaps + mcp_gaps)); \ | |
| if [ $$blocked -gt 0 ]; then \ | |
| echo "[*] Results: $$gaps gap(s) found, $$blocked item(s) blocked by platform (expected)"; \ | |
| else \ | |
| echo "[*] Results: $$gaps gap(s) found"; \ | |
| fi; \ | |
| exit $$gaps |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pmoves/mk/kilo.mk` around lines 100 - 107, Update the MCP parity check
invocation in the Make target to capture the exit status returned by
pmoves.tools.kilo_parity_mcp_check rather than assigning command-substitution
stdout to mcp_gaps. Preserve the existing gaps accumulation, blocked-item
reporting, and final exit behavior.
| # mcp-toolkit-connect is the only target that MUTATES a client's MCP config | ||
| # (writes .mcp.json at the repo root). It is gated on operator authorization — | ||
| # call explicitly, not as part of a wider chain. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Update the mutation warning for the new bootstrap targets.
mcp-config-bootstrap, OpenCode/OpenClaw bootstrap, and Hermes/Crush bootstrap also mutate client configuration files. The “only target” claim is no longer true.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pmoves/mk/mcp-toolkit.mk` around lines 6 - 8, Update the mutation warning
comment near mcp-toolkit-connect to include mcp-config-bootstrap,
OpenCode/OpenClaw bootstrap, and Hermes/Crush bootstrap as targets that modify
client configuration files, and remove the claim that mcp-toolkit-connect is the
only mutating target while preserving the explicit-authorization guidance.
| def missing_envs(self, env_cache: Dict[Path, Dict[str, str]]) -> List[str]: | ||
| keys = list(self.required_envs) | ||
| if self.required_env: | ||
| keys.append(self.required_env) | ||
| return [ | ||
| key | ||
| for key in keys | ||
| if not _lookup_env(key, env_cache) | ||
| ] |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Reject placeholder values as missing configuration.
missing_envs() only checks truthiness, so values such as your_*_here enable the MCP and produce a green configuration that cannot connect. Validate known placeholder patterns here and return those keys as missing.
As per path instructions, checks must be exhaustive and fail loudly on placeholders such as your_*_here. <path_instructions>
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pmoves/tools/crush_configurator.py` around lines 174 - 182, Update
missing_envs to treat known placeholder-pattern values, including your_*_here,
as missing in addition to empty or absent values. Add exhaustive placeholder
validation within the existing _lookup_env-based check, and return each affected
key so configuration cannot pass with unresolved placeholders.
Source: Path instructions
| MCPSpec( | ||
| key="pmoves-cipher", | ||
| config={ | ||
| "type": "sse", | ||
| "url": "http://${TS_Z890}:8105/mcp/sse", | ||
| "headers": {"Authorization": "Bearer ${CIPHER_API_TOKEN}"}, | ||
| "timeout": 30, | ||
| }, | ||
| required_env="CIPHER_API_TOKEN", | ||
| ), | ||
| MCPSpec( | ||
| key="pmoves-cipher-local", | ||
| config={ | ||
| "type": "sse", | ||
| "url": "http://localhost:8105/mcp/sse", | ||
| "headers": {"Authorization": "Bearer ${CIPHER_API_TOKEN}"}, | ||
| "timeout": 30, | ||
| }, | ||
| required_env="CIPHER_API_TOKEN", | ||
| ), | ||
| MCPSpec( | ||
| key="agent-zero", | ||
| config={ | ||
| "type": "http", | ||
| "url": "http://${TS_Z890}:8080/mcp", | ||
| "timeout": 30, | ||
| }, | ||
| ), |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Declare TS_Z890 as required for remote MCP endpoints.
Both URLs retain an unresolved ${TS_Z890} when the variable is absent, but Cipher only checks its token and Agent Zero checks nothing. Add TS_Z890 to their requirements so these servers are disabled instead of generated as unusable.
🧰 Tools
🪛 ast-grep (0.44.1)
[warning] 199-199: Do not make http calls without encryption
Context: "http://${TS_Z890}:8105/mcp/sse"
Note: [CWE-319] Cleartext Transmission of Sensitive Information.
(requests-http)
[warning] 219-219: Do not make http calls without encryption
Context: "http://${TS_Z890}:8080/mcp"
Note: [CWE-319] Cleartext Transmission of Sensitive Information.
(requests-http)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pmoves/tools/crush_configurator.py` around lines 196 - 223, Add TS_Z890 to
the required_env declarations for the remote pmoves-cipher and agent-zero
MCPSpec entries, while leaving pmoves-cipher-local dependent only on
CIPHER_API_TOKEN. Ensure configurations with missing TS_Z890 are not generated.
Source: Path instructions
| MCPSpec( | ||
| key="pmoves-supabase", | ||
| config={ | ||
| "type": "stdio", | ||
| "command": "npx", | ||
| "args": [ | ||
| "-y", | ||
| "@supabase/mcp-server-postgrest@0.1.1", | ||
| "--apiUrl", | ||
| "${SUPABASE_REST_URL:-http://localhost:8000/rest/v1}", | ||
| "--apiKey", | ||
| "${SUPABASE_SERVICE_ROLE_KEY:-${SUPABASE_SERVICE_KEY}}", | ||
| "--schema", | ||
| "public", | ||
| ], | ||
| "timeout": 60, | ||
| }, | ||
| required_commands=["npx"], | ||
| ), |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Disable Supabase unless either supported API key exists.
This spec remains enabled when both SUPABASE_SERVICE_ROLE_KEY and SUPABASE_SERVICE_KEY are missing. Extend MCPSpec with an “any-of” environment requirement and require at least one key.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pmoves/tools/crush_configurator.py` around lines 242 - 260, Update the
pmoves-supabase MCPSpec to use the MCPSpec any-of environment requirement,
requiring SUPABASE_SERVICE_ROLE_KEY or SUPABASE_SERVICE_KEY before enabling the
spec. Preserve the existing command, arguments, timeout, and required_commands
configuration.
Source: Path instructions
| val = env.get(var) | ||
| if not val and allow_os_environ: | ||
| val = os.environ.get(var) | ||
| if val: | ||
| result.append(val) | ||
| elif default: | ||
| # Recursively expand default so nested fallbacks resolve. | ||
| result.append(_expand(default, env)) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win
Preserve the no-environment policy during nested expansion.
The recursive call resets allow_os_environ=True. Thus tracked generation of ${SUPABASE_SERVICE_ROLE_KEY:-${SUPABASE_SERVICE_KEY}} can resolve the nested fallback from the process environment and commit the secret.
Proposed fix
- result.append(_expand(default, env))
+ result.append(
+ _expand(default, env, allow_os_environ=allow_os_environ)
+ )📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| val = env.get(var) | |
| if not val and allow_os_environ: | |
| val = os.environ.get(var) | |
| if val: | |
| result.append(val) | |
| elif default: | |
| # Recursively expand default so nested fallbacks resolve. | |
| result.append(_expand(default, env)) | |
| val = env.get(var) | |
| if not val and allow_os_environ: | |
| val = os.environ.get(var) | |
| if val: | |
| result.append(val) | |
| elif default: | |
| # Recursively expand default so nested fallbacks resolve. | |
| result.append( | |
| _expand(default, env, allow_os_environ=allow_os_environ) | |
| ) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pmoves/tools/mcp_config_generator.py` around lines 113 - 120, The recursive
default expansion in `_expand` must preserve the caller’s `allow_os_environ`
value instead of re-enabling process-environment lookups. Pass that policy
through nested expansion so tracked generation cannot resolve nested fallbacks
from `os.environ` when environment access is disabled.
Source: Path instructions
| elif spec.transport == "stdio": | ||
| command, args = _split_command_args(spec.command or "", spec.args) | ||
| entry["type"] = "stdio" | ||
| entry["command"] = command | ||
| entry["args"] = _render_args(args, context, **kw) | ||
| else: |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate relevant files =="
git ls-files | rg '(^|/)pmoves/tools/mcp_config_generator\.py$|(^|/)pmoves/.*(spec|mcp|config).*\.py$|(^|/)README\.md$'
echo "== outline generator =="
ast-grep outline pmoves/tools/mcp_config_generator.py --view expanded || true
echo "== relevant lines around target =="
sed -n '220,330p' pmoves/tools/mcp_config_generator.py | cat -n
echo "== search for env handling =="
rg -n "spec\.env|_render_env|\"env\"" pmoves/tools/mcp_config_generator.py pmoves -g '*.py' || trueRepository: POWERFULMOVES/PMOVES.AI
Length of output: 22946
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
import pathlib, re, json
path = pathlib.Path('pmoves/tools/mcp_config_generator.py')
print(path.exists(), path.stat().st_size if path.exists() else None)
PYRepository: POWERFULMOVES/PMOVES.AI
Length of output: 172
🌐 Web query:
Crush schema stdio env object official schema.json mcp env stdio server
💡 Result:
In the Model Context Protocol (MCP), the env field is a standard component within the configuration object for stdio servers [1][2][3]. It is used to define environment variables that the client should inject into the subprocess when spawning the MCP server [1][4][3]. Configuration Schema The schema for an MCP stdio server configuration generally includes the following fields [1][5][4]: - type: Must be set to "stdio" [1]. - command: The executable or path to the server command (e.g., "node", "python", "npx") [1][5][4]. - args: An optional array of strings representing command-line arguments [1][5][4]. - env: An optional object (dictionary) of key-value pairs representing custom environment variables [1][5][4]. Example Configuration A typical configuration in a client's settings (such as VS Code or Claude Desktop) looks like this [1][3]: { "servers": { "my-server": { "type": "stdio", "command": "npx", "args": ["-y", "my-mcp-server"], "env": { "API_KEY": "your-api-key-here" } } } } Important Implementation Details 1. Environment Merging: MCP SDK implementations (such as the Python SDK) do not always pass the entire host environment to the child process [6][4]. Instead, they often use a helper function (e.g., get_default_environment()) to merge a "safe" set of inherited variables (like PATH) with the custom variables provided in your env object [5][4][7]. 2. Security: By design, many implementations filter out potentially sensitive or dangerous environment variables by default to ensure the security of the subprocess execution [5][4]. 3. Client Responsibility: The env field is processed by the MCP client (the application launching the server), not the MCP protocol itself at the wire level [8][3]. Therefore, the specific schema and validation for env may vary slightly depending on the client application you are using [1][2][9]. While there is no single monolithic "official schema.json" that covers every client's implementation, the structure described above follows the established pattern used across major MCP SDKs and client configurations [1][5][3].
Citations:
- 1: https://github.com/microsoft/vscode-docs/blob/main/docs/copilot/reference/mcp-configuration.md
- 2: https://www.librechat.ai/docs/configuration/librechat_yaml/object_structure/mcp_servers
- 3: https://mcpcat.io/guides/building-stdio-mcp-server/
- 4: https://github.com/modelcontextprotocol/python-sdk/blob/babb477dffa33f46cdc886bc885eb1d521151430/src/mcp/client/stdio/__init__.py
- 5: https://py.sdk.modelcontextprotocol.io/v2/api/mcp/client/stdio/
- 6: MCP Client Environment Variables in StdioServerParameters modelcontextprotocol/python-sdk#99
- 7: https://github.com/modelcontextprotocol/python-sdk/blob/679b22970e12b8eec3897108ac7d6b0624809b9e/src/mcp/client/stdio/__init__.py
- 8: https://modelcontextprotocol.io/specification/draft/basic/transports/stdio
- 9: https://github.com/hackagadget/mcp-stdio-bridge/blob/main/docs/configuration.md
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== generator target section =="
sed -n '1,220p' pmoves/tools/mcp_config_generator.py | cat -n
echo "== crush renderer section =="
sed -n '240,290p' pmoves/tools/mcp_config_generator.py | cat -n
echo "== related tests =="
sed -n '1,360p' pmoves/tests/test_mcp_config_generator.py | cat -n | sed -n '1,340p'
echo "== inventory entries with stdio/env =="
python3 - <<'PY'
import json, pathlib
p = pathlib.Path('pmoves/config/mcp_inventory.json')
data = json.loads(p.read_text())
for server in data.get('servers', []):
if server.get('transport') == 'stdio' and server.get('env'):
print(server.get('key'), server.get('env'))
PYRepository: POWERFULMOVES/PMOVES.AI
Length of output: 29081
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
p = Path('pmoves/config/mcp_inventory.json')
print("exists", p.exists())
if p.exists():
import json
data = json.loads(p.read_text())
print("servers", len(data.get("servers", [])))
for s in data.get("servers", []):
if s.get("transport") == "stdio":
print(s.get("key"), "env_keys=", sorted((s.get("env") or {}).keys()))
PYRepository: POWERFULMOVES/PMOVES.AI
Length of output: 183
Pass spec.env into Crush stdio entries.
render_crush() drops spec.env, so any stdio server that needs credentials or connection settings will be generated without them.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pmoves/tools/mcp_config_generator.py` around lines 278 - 283, Update the
stdio branch in render_crush() to include spec.env in the generated entry,
preserving the existing command and argument rendering while ensuring
environment variables are passed through to Crush stdio servers.
2 participants
Summary
Bootstraps a canonical MCP inventory and generator, rolls the canonical PMOVES MCP set into OpenClaw scopes and OpenCode node configs, adds the SPARK (NVIDIA DGX) node to the matrix, and fixes a TensorZero TOML duplicate-key issue discovered during bring-up.
Commits
3ce7fad76feat(mcp): canonical MCP inventory + generator + full agent-stack bootstrap0cb095371feat(mcp): bootstrap canonical PMOVES MCPs into OpenClaw scopescd6616bf8fix(mcp): promote kvm2 OpenClaw scope to full tier7d13e2690feat(spark): add SPARK node to OpenClaw/OpenCode MCP matrix and fix TensorZero TOML duplicatesKey changes
pmoves/config/mcp_inventory.json: canonical inventory with endpoint prefixes (local/fleet) and defaults for Cipher / Agent Zero URLs.pmoves/tools/mcp_config_generator.py: renders client-native configs for Claude, Kimi, KiloCode, Crush, Hermes, and OpenClaw scopes.pmoves/tools/bootstrap_openclaw_scopes.py: idempotently updates per-node OpenClaw scopes using tier + endpoint policy.pmoves/tools/bootstrap_opencode.py: updates OpenCode node configs while preserving non-PMOVES MCPs.opencode-spark.json.[models.chat_zai_glm52]and[functions.coding_kilocode]keys inpmoves/tensorzero/config/tensorzero.toml.Verification
Notes
${TS_Z890})..pre-mcp-bootstrap.bakfiles andpmoves/docs/evidence/artifacts are left unstaged.Summary by CodeRabbit