forked from molecule-ai/molecule-core
fix(onboarding): address Claude Code MCP onboarding friction (#2934)
Ryan's bug report (#2934) walked through ~45 min of debugging a stock external-runtime install. This PR fixes the four items he flagged that have a small surface, and stubs out the larger ones for follow-up. Fixed in this PR ================ #1 — Python floor disclosure (README in publish bundle) Add an explicit "Requires Python ≥3.11" section that calls out the cryptic "Could not find a version that satisfies the requirement" failure mode; recommend `pipx install` over `pip install` so the binary lands on PATH automatically; show the explicit `pip install --user` alternative with the PATH caveat. #3 — MOLECULE_WORKSPACE_TOKEN_FILE support (mcp_workspace_resolver.py) Add a third resolution step between the inline env var and the in-container CONFIGS_DIR fallback. Operators can write the bearer to a 0600 file (e.g. ~/.config/molecule/token) and point MOLECULE_WORKSPACE_TOKEN_FILE at it, keeping the secret out of ~/.zsh_history and out of plaintext in MCP-host configs like ~/.claude.json. Inline TOKEN still wins on conflict so rotation flows are predictable. README documents the safer option as the recommended path. 6 new tests pin every leg (file resolves, inline wins, missing/empty file falls through, blank env unset-equivalent, help text advertises it). #4 — Push delivery 3-condition gating (README in publish bundle) Document that real-time push on Claude Code requires (a) the server to declare experimental.claude/channel (we do), (b) the server to be marketplace-plugin-sourced (operators must scaffold their own until the official marketplace lands — see #2934 follow-up), and (c) the --dangerously-load-development-channels flag on the claude invocation. Until any of the three is in place, delivery silently falls back to poll mode with no diagnostic. The README now says all of this explicitly so a new operator doesn't grep the binary for channel_enable to figure it out. #8 — serverInfo.name mismatch (a2a_mcp_server.py) The server reported `serverInfo.name = "a2a-delegation"` while operators register it as `molecule` (the name in `claude mcp add molecule …`). Harmless on tool routing today but matters for any future Claude Code allowlist that gates push by hardcoded server name. Renamed to "molecule" with an inline comment explaining the invariant. Deferred (separate issues to track) =================================== #2 — covered transitively by #1's pipx recommendation; no separate fix. #5 — `moleculesai/claude-code-plugin` marketplace repo (substantial new repo work; the README references it as a documented follow-up). #6 — `molecule-mcp doctor` subcommand (substantial new CLI surface; mentioned in the README's push-vs-poll section as the planned diagnostic for silent push fallback). #7 — `--dangerously-load-development-channels` rename — not in our control; that's Claude Code's flag. Tests ===== 164/164 mcp_cli + a2a_mcp_server tests pass locally (WORKSPACE_ID=00000000-0000-0000-0000-000000000001 pytest …) including 6 new TestTokenFileEnv cases. Wheel builds successfully via scripts/build_runtime_package.py with the new README markers verified in the output. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Hongming Wang
parent
fc30b5c9de
commit
01deeb36cf
@ -289,10 +289,37 @@ directory** by the `publish-runtime` GitHub Actions workflow on every
|
||||
Operators running an agent outside the platform's container fleet
|
||||
(any runtime that supports MCP stdio — Claude Code, hermes, codex,
|
||||
etc.) can install this wheel and run the universal MCP server
|
||||
locally:
|
||||
locally.
|
||||
|
||||
### Requirements
|
||||
|
||||
* **Python ≥3.11.** The wheel sets `requires-python = ">=3.11"`. On
|
||||
older interpreters `pip install` returns the cryptic
|
||||
`Could not find a version that satisfies the requirement` — that
|
||||
message is pip filtering this wheel out, NOT the package missing
|
||||
from PyPI. Upgrade with `brew install python@3.12` /
|
||||
`apt install python3.12` / `pyenv install 3.12` first.
|
||||
* **`pipx` recommended over `pip`.** `pipx install` puts
|
||||
`molecule-mcp` on PATH automatically and isolates the runtime's
|
||||
deps from your system Python. Plain `pip install --user` works
|
||||
but the binary lands in `~/.local/bin` (Linux) or
|
||||
`~/Library/Python/3.X/bin` (macOS) which is often not on PATH on
|
||||
a fresh shell — `claude mcp add molecule -- molecule-mcp` then
|
||||
fails with "command not found" at first use.
|
||||
|
||||
### Install
|
||||
|
||||
```sh
|
||||
# Recommended:
|
||||
pipx install molecule-ai-workspace-runtime
|
||||
|
||||
# Alternative (manage PATH yourself):
|
||||
pip install --user molecule-ai-workspace-runtime
|
||||
```
|
||||
|
||||
### Run
|
||||
|
||||
```sh
|
||||
pip install molecule-ai-workspace-runtime
|
||||
WORKSPACE_ID=<uuid> \\
|
||||
PLATFORM_URL=https://<tenant>.staging.moleculesai.app \\
|
||||
MOLECULE_WORKSPACE_TOKEN=<bearer> \\
|
||||
@ -305,10 +332,64 @@ runtimes already get via the workspace's auto-spawned MCP. Register
|
||||
the binary in your agent's MCP config (e.g. Claude Code's
|
||||
`claude mcp add molecule -- molecule-mcp` with the env above).
|
||||
|
||||
### Keeping the token out of shell history
|
||||
|
||||
Inline `MOLECULE_WORKSPACE_TOKEN=<bearer>` ends up in `~/.zsh_history`
|
||||
and (when registered via `claude mcp add`) plaintext in
|
||||
`~/.claude.json`. To avoid that, write the token to a 0600 file and
|
||||
point `MOLECULE_WORKSPACE_TOKEN_FILE` at it:
|
||||
|
||||
```sh
|
||||
umask 077
|
||||
printf '%s' "<bearer>" > ~/.config/molecule/token
|
||||
WORKSPACE_ID=<uuid> \\
|
||||
PLATFORM_URL=https://<tenant>.staging.moleculesai.app \\
|
||||
MOLECULE_WORKSPACE_TOKEN_FILE=$HOME/.config/molecule/token \\
|
||||
molecule-mcp
|
||||
```
|
||||
|
||||
Token resolution order: `MOLECULE_WORKSPACE_TOKEN` (inline env) →
|
||||
`MOLECULE_WORKSPACE_TOKEN_FILE` (path) → `${CONFIGS_DIR}/.auth_token`
|
||||
(in-container default).
|
||||
|
||||
The token comes from the canvas → Tokens tab. Restarting an external
|
||||
workspace from the canvas no longer revokes the token (PR #2412), so
|
||||
operator tokens persist across status nudges.
|
||||
|
||||
### Push vs poll delivery (Claude Code specifics)
|
||||
|
||||
By default the inbox runs in **poll mode** — every turn the agent
|
||||
calls `wait_for_message`, which blocks up to ~60s on
|
||||
`/activity?since_id=…`. Real-time push delivery is also supported,
|
||||
but on Claude Code it requires THREE conditions, ALL of which must
|
||||
hold:
|
||||
|
||||
1. **The MCP server declares `experimental.claude/channel`** — this
|
||||
wheel does (see `_build_initialize_result`). Nothing for you to
|
||||
do.
|
||||
2. **Claude Code installs the server as a marketplace plugin** — a
|
||||
plain `claude mcp add molecule -- molecule-mcp` produces a
|
||||
non-plugin-sourced server, which Claude Code rejects with
|
||||
`channel_enable requires a marketplace plugin`. Until the
|
||||
official `moleculesai/claude-code-plugin` marketplace lands
|
||||
(issue #2934 follow-up), operators who want push must scaffold
|
||||
their own local marketplace under
|
||||
`~/.claude/marketplaces/molecule-local/` containing a
|
||||
`marketplace.json` + `plugin.json` that points at this wheel.
|
||||
3. **Claude Code is launched with the dev-channels flag** — pass
|
||||
`--dangerously-load-development-channels plugin:molecule@<marketplace>`
|
||||
on the `claude` invocation. Without this flag the channel
|
||||
capability is silently ignored.
|
||||
|
||||
Symptom of any condition failing: messages arrive but only via the
|
||||
poll path (every ~1–60s), not real-time. There's currently no
|
||||
diagnostic surfaced — `molecule-mcp doctor` (issue #2934 follow-up)
|
||||
is planned.
|
||||
|
||||
If you don't need real-time push, the default poll path works
|
||||
universally with no extra setup; both modes converge on the same
|
||||
`inbox_pop` ack so messages never duplicate.
|
||||
|
||||
See [`docs/workspace-runtime-package.md`](https://github.com/Molecule-AI/molecule-core/blob/main/docs/workspace-runtime-package.md)
|
||||
for the publish flow and architecture.
|
||||
"""
|
||||
|
||||
@ -425,7 +425,16 @@ def _build_initialize_result() -> dict:
|
||||
"tools": {"listChanged": False},
|
||||
"experimental": {"claude/channel": {}},
|
||||
},
|
||||
"serverInfo": {"name": "a2a-delegation", "version": "1.0.0"},
|
||||
# Identifier convention: this server is what users register with
|
||||
# `claude mcp add molecule -- molecule-mcp` (and similar across
|
||||
# other MCP hosts), so the canonical name is "molecule". Earlier
|
||||
# versions reported "a2a-delegation" — accurate to the original
|
||||
# purpose but a mismatch with how operators actually name it.
|
||||
# Mismatch is harmless on tool routing (all MCP hosts dispatch
|
||||
# by the user-supplied registration name, NOT serverInfo.name)
|
||||
# but matters for any future Claude Code allowlist that gates
|
||||
# channel push by hardcoded server name (issue #2934).
|
||||
"serverInfo": {"name": "molecule", "version": "1.0.0"},
|
||||
# Built per-call (not the module-level constant) so an operator
|
||||
# who sets MOLECULE_MCP_POLL_TIMEOUT_SECS after import — e.g.
|
||||
# via a wrapper script that exports then re-imports — sees
|
||||
|
||||
@ -35,9 +35,15 @@ def resolve_workspaces() -> tuple[list[tuple[str, str]], list[str]]:
|
||||
N workspaces). When set, ``WORKSPACE_ID`` / ``MOLECULE_WORKSPACE_TOKEN``
|
||||
are IGNORED — the JSON is the source of truth.
|
||||
|
||||
2. Single-workspace fallback — ``WORKSPACE_ID`` env var + token from
|
||||
``MOLECULE_WORKSPACE_TOKEN`` or ``${CONFIGS_DIR}/.auth_token``.
|
||||
This is the pre-existing path; back-compat exact.
|
||||
2. Single-workspace fallback — ``WORKSPACE_ID`` env var + token
|
||||
resolved in this order:
|
||||
a. ``MOLECULE_WORKSPACE_TOKEN`` (inline env — convenient but
|
||||
leaks into shell history + plaintext MCP-host config).
|
||||
b. ``MOLECULE_WORKSPACE_TOKEN_FILE`` (path to a file holding
|
||||
the token — operator can keep it 0600 in their home dir;
|
||||
survives shell-history scrubs).
|
||||
c. ``${CONFIGS_DIR}/.auth_token`` (in-container runtimes —
|
||||
the platform writes this on provision).
|
||||
|
||||
Returns ``(workspaces, errors)``:
|
||||
* ``workspaces``: list of ``(workspace_id, token)`` — non-empty
|
||||
@ -98,16 +104,47 @@ def resolve_workspaces() -> tuple[list[tuple[str, str]], list[str]]:
|
||||
wsid = os.environ.get("WORKSPACE_ID", "").strip()
|
||||
if not wsid:
|
||||
return [], ["WORKSPACE_ID (or MOLECULE_WORKSPACES) is required"]
|
||||
# Token resolution order (#2934): inline env → file path → CONFIGS_DIR
|
||||
# default. The file-path option exists so operators can keep the
|
||||
# bearer out of shell history and out of MCP-host config plaintext
|
||||
# (e.g. ~/.claude.json) — set MOLECULE_WORKSPACE_TOKEN_FILE to a
|
||||
# 0600 file containing the token. The CONFIGS_DIR/.auth_token
|
||||
# fallback predates this and stays for in-container runtimes.
|
||||
tok = os.environ.get("MOLECULE_WORKSPACE_TOKEN", "").strip()
|
||||
if not tok:
|
||||
tok = _read_token_from_file_env()
|
||||
if not tok:
|
||||
tok = read_token_file()
|
||||
if not tok:
|
||||
return [], [
|
||||
"MOLECULE_WORKSPACE_TOKEN (or CONFIGS_DIR/.auth_token) is required"
|
||||
"MOLECULE_WORKSPACE_TOKEN, MOLECULE_WORKSPACE_TOKEN_FILE, or "
|
||||
"CONFIGS_DIR/.auth_token is required"
|
||||
]
|
||||
return [(wsid, tok)], []
|
||||
|
||||
|
||||
def _read_token_from_file_env() -> str:
|
||||
"""Read the token from the file path in MOLECULE_WORKSPACE_TOKEN_FILE.
|
||||
|
||||
Returns "" on:
|
||||
- env var unset / blank
|
||||
- file not found, unreadable, or empty
|
||||
- any OSError on read
|
||||
|
||||
Empty-on-failure (rather than raising) lets the resolver fall through
|
||||
to the CONFIGS_DIR fallback. The caller surfaces the combined "no
|
||||
token" error if every source is empty.
|
||||
"""
|
||||
path = os.environ.get("MOLECULE_WORKSPACE_TOKEN_FILE", "").strip()
|
||||
if not path:
|
||||
return ""
|
||||
try:
|
||||
with open(path, encoding="utf-8") as fh:
|
||||
return fh.read().strip()
|
||||
except OSError:
|
||||
return ""
|
||||
|
||||
|
||||
def print_missing_env_help(missing: list[str], have_token_file: bool) -> None:
|
||||
print("molecule-mcp: missing required environment.\n", file=sys.stderr)
|
||||
print("Set the following before running molecule-mcp:", file=sys.stderr)
|
||||
@ -123,6 +160,16 @@ def print_missing_env_help(missing: list[str], have_token_file: bool) -> None:
|
||||
"(canvas → Tokens tab)",
|
||||
file=sys.stderr,
|
||||
)
|
||||
print(
|
||||
" OR set MOLECULE_WORKSPACE_TOKEN_FILE"
|
||||
" to a path that holds the token",
|
||||
file=sys.stderr,
|
||||
)
|
||||
print(
|
||||
" (keeps the secret out of shell"
|
||||
" history and MCP-host config plaintext)",
|
||||
file=sys.stderr,
|
||||
)
|
||||
print("", file=sys.stderr)
|
||||
print(f"Currently missing: {', '.join(missing)}", file=sys.stderr)
|
||||
|
||||
|
||||
@ -229,3 +229,87 @@ class TestResolveWorkspacesDirect:
|
||||
out, errors = mcp_workspace_resolver.resolve_workspaces()
|
||||
assert out == [("ws-a", "a"), ("ws-b", "b")]
|
||||
assert errors == []
|
||||
|
||||
|
||||
# ============== Token-from-file env var (issue #2934) ==============
|
||||
|
||||
class TestTokenFileEnv:
|
||||
"""``MOLECULE_WORKSPACE_TOKEN_FILE`` lets operators keep the bearer
|
||||
out of shell history and out of MCP-host config plaintext (e.g.
|
||||
~/.claude.json). Resolution order: inline TOKEN env > TOKEN_FILE
|
||||
env > ${CONFIGS_DIR}/.auth_token.
|
||||
"""
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _isolate(self, monkeypatch, tmp_path):
|
||||
for v in (
|
||||
"WORKSPACE_ID",
|
||||
"MOLECULE_WORKSPACE_TOKEN",
|
||||
"MOLECULE_WORKSPACE_TOKEN_FILE",
|
||||
"MOLECULE_WORKSPACES",
|
||||
):
|
||||
monkeypatch.delenv(v, raising=False)
|
||||
# Point CONFIGS_DIR at an empty tmp_path so the .auth_token
|
||||
# fallback returns "" — keeps the test cases unambiguous.
|
||||
monkeypatch.setenv("CONFIGS_DIR", str(tmp_path))
|
||||
yield tmp_path
|
||||
|
||||
def test_token_file_env_resolves(self, monkeypatch, tmp_path):
|
||||
token_path = tmp_path / "token.txt"
|
||||
token_path.write_text("file-tok-123\n") # trailing newline must strip
|
||||
monkeypatch.setenv("WORKSPACE_ID", "ws-1")
|
||||
monkeypatch.setenv("MOLECULE_WORKSPACE_TOKEN_FILE", str(token_path))
|
||||
out, errors = mcp_workspace_resolver.resolve_workspaces()
|
||||
assert out == [("ws-1", "file-tok-123")]
|
||||
assert errors == []
|
||||
|
||||
def test_inline_token_takes_precedence_over_file(self, monkeypatch, tmp_path):
|
||||
# If both env vars are set, inline wins — matches the docstring's
|
||||
# documented order. (Operators sometimes set both during a
|
||||
# rotation; we want predictable behavior.)
|
||||
token_path = tmp_path / "token.txt"
|
||||
token_path.write_text("file-tok")
|
||||
monkeypatch.setenv("WORKSPACE_ID", "ws-1")
|
||||
monkeypatch.setenv("MOLECULE_WORKSPACE_TOKEN", "inline-tok")
|
||||
monkeypatch.setenv("MOLECULE_WORKSPACE_TOKEN_FILE", str(token_path))
|
||||
out, _ = mcp_workspace_resolver.resolve_workspaces()
|
||||
assert out == [("ws-1", "inline-tok")]
|
||||
|
||||
def test_missing_file_falls_through_to_error(self, monkeypatch, tmp_path):
|
||||
# Pointed at a non-existent path — resolver should return the
|
||||
# combined "no token" error, NOT crash.
|
||||
monkeypatch.setenv("WORKSPACE_ID", "ws-1")
|
||||
monkeypatch.setenv(
|
||||
"MOLECULE_WORKSPACE_TOKEN_FILE", str(tmp_path / "does-not-exist")
|
||||
)
|
||||
out, errors = mcp_workspace_resolver.resolve_workspaces()
|
||||
assert out == []
|
||||
assert any("MOLECULE_WORKSPACE_TOKEN_FILE" in e for e in errors)
|
||||
|
||||
def test_empty_file_falls_through_to_error(self, monkeypatch, tmp_path):
|
||||
# File exists but is blank — same shape as no token at all.
|
||||
token_path = tmp_path / "empty.txt"
|
||||
token_path.write_text("")
|
||||
monkeypatch.setenv("WORKSPACE_ID", "ws-1")
|
||||
monkeypatch.setenv("MOLECULE_WORKSPACE_TOKEN_FILE", str(token_path))
|
||||
out, errors = mcp_workspace_resolver.resolve_workspaces()
|
||||
assert out == []
|
||||
assert errors # at least one combined error message
|
||||
|
||||
def test_blank_env_var_treated_as_unset(self, monkeypatch):
|
||||
# Empty string is treated as "not set" — common pitfall when
|
||||
# users export an unset shell var.
|
||||
monkeypatch.setenv("WORKSPACE_ID", "ws-1")
|
||||
monkeypatch.setenv("MOLECULE_WORKSPACE_TOKEN_FILE", "")
|
||||
out, errors = mcp_workspace_resolver.resolve_workspaces()
|
||||
assert out == []
|
||||
assert errors
|
||||
|
||||
def test_help_message_advertises_token_file(self, capsys):
|
||||
# Help text must mention TOKEN_FILE so a first-run operator
|
||||
# learns about the safer option without grepping the source.
|
||||
mcp_workspace_resolver.print_missing_env_help(
|
||||
["WORKSPACE_ID", "MOLECULE_WORKSPACE_TOKEN"], have_token_file=False
|
||||
)
|
||||
err = capsys.readouterr().err
|
||||
assert "MOLECULE_WORKSPACE_TOKEN_FILE" in err
|
||||
|
||||
Loading…
Reference in New Issue
Block a user