diff --git a/scripts/build_runtime_package.py b/scripts/build_runtime_package.py index fc3006e0..eb5e1daa 100755 --- a/scripts/build_runtime_package.py +++ b/scripts/build_runtime_package.py @@ -374,8 +374,9 @@ hold: 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 + (tracking [#2936](https://github.com/Molecule-AI/molecule-core/issues/2936)), + 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 @@ -385,8 +386,9 @@ hold: 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. +diagnostic surfaced — `molecule-mcp doctor` (tracking +[#2937](https://github.com/Molecule-AI/molecule-core/issues/2937)) 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 diff --git a/workspace/mcp_workspace_resolver.py b/workspace/mcp_workspace_resolver.py index 051c64e4..9d41279b 100644 --- a/workspace/mcp_workspace_resolver.py +++ b/workspace/mcp_workspace_resolver.py @@ -112,7 +112,18 @@ def resolve_workspaces() -> tuple[list[tuple[str, str]], list[str]]: # 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() + tok, tf_err = _read_token_from_file_env() + if tf_err: + # Operator explicitly pointed TOKEN_FILE somewhere — surface + # the SPECIFIC failure (path doesn't exist, isn't readable, + # or holds a blank file) instead of falling through to the + # generic "set one of these three vars" message. Otherwise + # they get exactly the silent failure mode #2934 flagged + # ("a new user has no chance"). Skip the CONFIGS_DIR + # fallback in this case — the operator's intent is clearly + # to use the file path; deferring to a different source + # would mask their config error. + return [], [tf_err] if not tok: tok = read_token_file() if not tok: @@ -123,26 +134,62 @@ def resolve_workspaces() -> tuple[list[tuple[str, str]], list[str]]: return [(wsid, tok)], [] -def _read_token_from_file_env() -> str: +def _read_token_from_file_env() -> tuple[str, 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. + Returns ``(token, error)``: + * env var unset/blank → ``("", "")`` — caller falls through silently + to the next source; the operator didn't ask for this path. + * file open/read fails (missing, permission denied, decode error) + → ``("", "")`` — caller surfaces it directly. + The operator EXPLICITLY pointed at this path, so a generic + fallthrough error would mask their config bug (#2934). + * file is blank → ``("", "")`` — same reasoning. + * file read returns junk with internal whitespace/newlines (e.g. + a CSV cell, accidental multi-token paste) → ``("", "")`` + rather than concatenating into a malformed bearer that 401s + against the platform with no context. + * happy path → ``("", "")``. """ path = os.environ.get("MOLECULE_WORKSPACE_TOKEN_FILE", "").strip() if not path: - return "" + return "", "" try: with open(path, encoding="utf-8") as fh: - return fh.read().strip() - except OSError: - return "" + raw = fh.read() + except FileNotFoundError: + return "", ( + f"MOLECULE_WORKSPACE_TOKEN_FILE points to {path!r} which " + f"does not exist" + ) + except PermissionError: + return "", ( + f"MOLECULE_WORKSPACE_TOKEN_FILE={path!r} is not readable " + f"(permission denied)" + ) + except OSError as exc: + return "", ( + f"MOLECULE_WORKSPACE_TOKEN_FILE={path!r} could not be read: " + f"{exc}" + ) + except UnicodeDecodeError: + return "", ( + f"MOLECULE_WORKSPACE_TOKEN_FILE={path!r} is not valid UTF-8" + ) + tok = raw.strip() + if not tok: + return "", ( + f"MOLECULE_WORKSPACE_TOKEN_FILE={path!r} is empty" + ) + # Reject tokens with internal whitespace — a CSV cell or accidental + # multi-token paste would otherwise become a malformed bearer that + # 401s against the platform with no diagnostic. + if any(ch.isspace() for ch in tok): + return "", ( + f"MOLECULE_WORKSPACE_TOKEN_FILE={path!r} contains internal " + f"whitespace — expected a single token" + ) + return tok, "" def print_missing_env_help(missing: list[str], have_token_file: bool) -> None: diff --git a/workspace/tests/test_mcp_cli_split.py b/workspace/tests/test_mcp_cli_split.py index 34f87f34..868f772b 100644 --- a/workspace/tests/test_mcp_cli_split.py +++ b/workspace/tests/test_mcp_cli_split.py @@ -275,26 +275,68 @@ class TestTokenFileEnv: 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. + def test_missing_file_returns_specific_error(self, monkeypatch, tmp_path): + # Operator EXPLICITLY pointed TOKEN_FILE at a non-existent path — + # surface the SPECIFIC failure (not the generic "set one of these + # three vars" message). Otherwise they hit the silent failure mode + # #2934 flagged ("a new user has no chance"). + bad_path = tmp_path / "does-not-exist" monkeypatch.setenv("WORKSPACE_ID", "ws-1") - monkeypatch.setenv( - "MOLECULE_WORKSPACE_TOKEN_FILE", str(tmp_path / "does-not-exist") - ) + monkeypatch.setenv("MOLECULE_WORKSPACE_TOKEN_FILE", str(bad_path)) out, errors = mcp_workspace_resolver.resolve_workspaces() assert out == [] - assert any("MOLECULE_WORKSPACE_TOKEN_FILE" in e for e in errors) + assert len(errors) == 1 + assert "MOLECULE_WORKSPACE_TOKEN_FILE" in errors[0] + assert "does not exist" in errors[0] + assert str(bad_path) in errors[0] - def test_empty_file_falls_through_to_error(self, monkeypatch, tmp_path): - # File exists but is blank — same shape as no token at all. + def test_empty_file_returns_specific_error(self, monkeypatch, tmp_path): + # Blank file — operator's intent was clearly the file path, so a + # generic "no token" error would mask their config bug. 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 + assert len(errors) == 1 + assert "MOLECULE_WORKSPACE_TOKEN_FILE" in errors[0] + assert "is empty" in errors[0] + + def test_multi_line_file_rejected(self, monkeypatch, tmp_path): + # CSV cell or accidental multi-token paste — would otherwise become + # a malformed bearer that 401s against the platform with no + # diagnostic. Reject upfront with a specific error. + token_path = tmp_path / "junk.txt" + token_path.write_text("tok-a tok-b\n") + 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 len(errors) == 1 + assert "internal whitespace" in errors[0] + + def test_token_file_error_skips_configs_dir_fallback( + self, monkeypatch, tmp_path + ): + # When TOKEN_FILE is explicitly set but broken, do NOT fall through + # to a valid CONFIGS_DIR/.auth_token — the operator's intent is + # clearly to use the file path; deferring to a different source + # would mask their config error. + configs_dir = tmp_path / "configs" + configs_dir.mkdir() + (configs_dir / ".auth_token").write_text("configs-tok") + monkeypatch.setenv("CONFIGS_DIR", str(configs_dir)) + monkeypatch.setenv("WORKSPACE_ID", "ws-1") + monkeypatch.setenv( + "MOLECULE_WORKSPACE_TOKEN_FILE", str(tmp_path / "missing") + ) + out, errors = mcp_workspace_resolver.resolve_workspaces() + assert out == [] + # Specific TOKEN_FILE error — not the generic "no token" fallback + # and crucially not the silent success of using configs-tok. + assert len(errors) == 1 + assert "does not exist" in errors[0] def test_blank_env_var_treated_as_unset(self, monkeypatch): # Empty string is treated as "not set" — common pitfall when