feat(#90): promote FLAG_ENV_MIGRATIONS into a ServerSpec spine
CI / Lint (ruff) (pull_request) Successful in 7s
CI / Tests (py3.10 / ubuntu-latest) (pull_request) Successful in 11s
CI / Tests (py3.12 / windows-latest) (pull_request) Successful in 25s
CI / Tests (py3.12 / ubuntu-latest) (pull_request) Successful in 10s
CI / Tests (py3.13 / ubuntu-latest) (pull_request) Successful in 11s
CI / Catalog signature (pull_request) Successful in 6s

Introduce the server-package axis (orthogonal to ClientSpec): a frozen
ServerSpec dataclass + SERVER_SPECS registry that the sidecar (#91),
version-pin (#92) and permission (#93) work all hang off.

- ServerSpec carries env_flags / removed_flags / drift_flags / sidecar_paths
  / sidecar_doc_path / schema. FLAG_ENV_MIGRATIONS is now a derived view of
  the registry, so every existing reader and the migration functions keep the
  exact shape #89 shipped — the migration LOGIC is unchanged, only the DATA grew.
- resolve_server_spec(data) is the ServerSpec entry point; detect_migratable_package
  is a thin name-only wrapper over it (behaviour identical).
- Add the verified #4 follow-ups: --sudoPassword AND --suPassword now auto-migrate
  to SSH_MCP_SUDO_PASSWORD (two flags → one var; existing no-clobber handles it).
  --disableSudo stays warn-only (sudo is now a role/policy, no env replacement).
- Add drift_warnings(): --maxChars=none changed meaning (v1 silently capped at
  5000 chars). Warn-only, no auto-fix; matches inline and separate arg forms.
- Seed ssh-mcp's verified per-platform sidecar TOML paths and zod enums
  (auth/approvalMode/role/port) as data for later issues.

Pure core + tests, no GUI. One #89 test (sudoPassword now migratable) updated to
reflect the intended data growth, with a comment citing the verified facts.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
Cowork Supervisor
2026-08-12 02:54:42 -04:00
co-authored by Claude Opus 4.8
parent 436524bf00
commit e087107710
2 changed files with 246 additions and 38 deletions
+154 -35
View File
@@ -30,6 +30,7 @@ import tempfile
import threading import threading
import time import time
from dataclasses import dataclass from dataclasses import dataclass
from dataclasses import field as _field
from pathlib import Path from pathlib import Path
from typing import NamedTuple from typing import NamedTuple
from urllib.parse import urlparse from urllib.parse import urlparse
@@ -2218,46 +2219,116 @@ def _looks_like_multiple_args(a: str) -> bool:
# --------------------------------------------------------------------------- # # --------------------------------------------------------------------------- #
# Removed-flag → env migration # ServerSpec — the server-package axis (issue #90, epic #94)
# #
# Some MCP servers moved credential CLI flags into environment variables across # BCC already models *which host reads a config* (ClientSpec). ServerSpec is the
# a major version (secrets on the command line are visible to any local user in # orthogonal axis: *which server package a definition runs*. A given server has
# process listings). A config written for the old version then fails hard on the # both — Claude Desktop (client) running ssh-mcp (server), say. #89 shipped the
# new one — e.g. ssh-mcp v2 exits with "These flags were removed in v2: # seed of this axis as FLAG_ENV_MIGRATIONS (a per-npm-package registry of removed
# --password". This registry lets BCC recognise that shape, warn about it, and # credential flags); #90 promotes it into a richer struct the sidecar (#91),
# offer a one-click migration that lifts the value out of `args` into `env`. # version-pin (#92), permission (#93) and future schema/auth work all hang off.
# #
# Per package: # Non-reversing constraint (epic #94): the migration LOGIC below
# "env" removed flag -> env var that now supplies it. BCC auto-migrates # (migrate_removed_flags / detect_migratable_package / removed_flag_warnings) is
# these: the flag (and its value) leave `args`, the value lands in # unchanged — only the DATA grows. FLAG_ENV_MIGRATIONS is kept as a derived view
# `env` under the mapped name. # so every existing reader keeps seeing the same shape.
# "removed" flags that no longer exist but have no confirmed 1:1 env
# replacement (e.g. a boolean, or an env name not documented here).
# BCC only warns; the user resolves these by hand.
#
# Extending it: add a package key and its flag maps. Keep "env" limited to
# mappings whose env-var name is verified, so the auto-fix stays trustworthy.
# --------------------------------------------------------------------------- # # --------------------------------------------------------------------------- #
FLAG_ENV_MIGRATIONS: dict[str, dict[str, dict[str, str]]] = { @dataclass(frozen=True)
"ssh-mcp": { class ServerSpec:
"env": { """Everything BCC knows about one MCP *server package*.
Frozen so the module-level registry entries are effectively singletons.
Every collection field defaults empty, so a spec need only fill in the axes
that apply to its package. Fields:
* ``env_flags`` — removed CLI flags whose value now belongs in an env var.
Auto-migratable: ``migrate_removed_flags`` lifts the flag's value out of
``args`` into ``env`` under the mapped name. Two flags may map to one var
(ssh-mcp's ``--sudoPassword``/``--suPassword`` → ``SSH_MCP_SUDO_PASSWORD``);
the no-clobber logic already handles that collision.
* ``removed_flags`` — removed flags with no confirmed 1:1 env replacement
(e.g. a boolean policy toggle). Warn-only; the user resolves by hand.
* ``drift_flags`` — flags/values whose *meaning* changed across a major
version without being removed. Keyed by the exact ``flag=value`` token; the
value is the human explanation. Not auto-fixed — meaning drift needs a human.
* ``sidecar_paths`` — per-platform sidecar config location, VERIFIED from the
package source (not its docs). Keys are ``"darwin"``/``"win32"``/``"posix"``;
values are templates expanded against ``$HOME`` + environment (see #91).
Empty == the package has no sidecar. Consumed by #91/#93.
* ``sidecar_doc_path`` — a path the package's README documents but does NOT
actually read on some platform. Used by #91 to flag a file a user placed by
following the (wrong) docs, which the server silently never reads.
* ``schema`` — validation enums shipped by the package (zod). Seeded as data
now; becomes editor pick-lists later (#7).
"""
package: str
env_flags: dict[str, str] = _field(default_factory=dict)
removed_flags: dict[str, str] = _field(default_factory=dict)
drift_flags: dict[str, str] = _field(default_factory=dict)
sidecar_paths: dict[str, str] = _field(default_factory=dict)
sidecar_doc_path: str = ""
schema: dict = _field(default_factory=dict)
# The registry. Keyed by the normalised npm package name (see _normalize_pkg_token).
# Verified ssh-mcp facts (from the package source, NOT its README — the README's
# sidecar path is wrong on macOS/Windows):
SERVER_SPECS: dict[str, ServerSpec] = {
"ssh-mcp": ServerSpec(
package="ssh-mcp",
# Credential flags removed in v2 — secrets on argv are visible to any
# local user in process listings, so v2 reads them from the environment.
env_flags={
"--password": "SSH_MCP_PASSWORD", "--password": "SSH_MCP_PASSWORD",
"--sudoPassword": "SSH_MCP_SUDO_PASSWORD",
"--suPassword": "SSH_MCP_SUDO_PASSWORD",
}, },
"removed": { # --disableSudo is gone with no env replacement: sudo is now a role/policy.
"--sudoPassword": ( removed_flags={
"removed in ssh-mcp v2 — supply it via an environment variable "
"instead (see the ssh-mcp README's 'Migrating from v1' section)"
),
"--suPassword": (
"removed in ssh-mcp v2 — supply it via an environment variable "
"instead (see the ssh-mcp README's 'Migrating from v1' section)"
),
"--disableSudo": ( "--disableSudo": (
"removed in ssh-mcp v2 — sudo is now governed by the server " "removed in ssh-mcp v2 — sudo is now governed by the server "
"config/policy rather than a CLI flag" "config/policy (a role/permission) rather than a CLI flag"
), ),
}, },
}, # --maxChars=none changed meaning: v1 silently parsed "none" to a 5000-char
# cap (parseInt("none") || 5000), so a config still carrying it doesn't mean
# "unlimited" the way a user reading the flag name would assume.
drift_flags={
"--maxChars=none": (
"changed meaning across ssh-mcp versions — v1 silently parsed "
"'none' as a 5000-character cap (parseInt('none') || 5000), not "
'"unlimited". Confirm the value still means what you intend on the '
"version you run."
),
},
# Sidecar TOML path, verified from the package code (see #91). The README
# documents ~/.config on every platform, but the code resolves the OS's
# native app-data dir on macOS/Windows.
sidecar_paths={
"darwin": "~/Library/Application Support/ssh-mcp/config.toml",
"win32": "${APPDATA}/ssh-mcp/config.toml",
"posix": "${XDG_CONFIG_HOME:-~/.config}/ssh-mcp/config.toml",
},
sidecar_doc_path="~/.config/ssh-mcp/config.toml",
# zod enums shipped by ssh-mcp v2 (data now; pick-lists later — #7).
schema={
"auth": ["agent", "key", "password", "keychain"],
"approvalMode": ["auto", "ask-destructive", "ask-all", "deny"],
"role": ["viewer", "operator", "admin"],
"port": {"min": 1, "max": 65535},
},
),
}
# Backwards-compatible derived view of the registry: {pkg: {"env": ..., "removed": ...}}.
# SERVER_SPECS is the source of truth; this alias preserves the exact shape #89
# shipped so every existing reader of FLAG_ENV_MIGRATIONS (and the migration
# functions below) keeps working with no logic change.
FLAG_ENV_MIGRATIONS: dict[str, dict[str, dict[str, str]]] = {
pkg: {"env": dict(spec.env_flags), "removed": dict(spec.removed_flags)}
for pkg, spec in SERVER_SPECS.items()
} }
@@ -2282,11 +2353,13 @@ def _normalize_pkg_token(token: str) -> str:
return t return t
def detect_migratable_package(data: dict) -> str | None: def resolve_server_spec(data: dict) -> ServerSpec | None:
"""Return the FLAG_ENV_MIGRATIONS key this stdio server runs, or None. """Return the ServerSpec for the package a stdio server runs, or None.
Scans the command and every arg (so ``npx -y ssh-mcp`` and a direct Scans the command and every arg (so ``npx -y ssh-mcp`` and a direct
``command: ssh-mcp`` both resolve), matching on the normalised package name. ``command: ssh-mcp`` both resolve), matching on the normalised package name.
This is the ServerSpec-axis entry point; ``detect_migratable_package`` is a
thin name-only wrapper kept for the existing migration callers.
""" """
if not isinstance(data, dict): if not isinstance(data, dict):
return None return None
@@ -2294,11 +2367,21 @@ def detect_migratable_package(data: dict) -> str | None:
tokens.extend(data.get("args") or []) tokens.extend(data.get("args") or [])
for tok in tokens: for tok in tokens:
name = _normalize_pkg_token(tok) name = _normalize_pkg_token(tok)
if name in FLAG_ENV_MIGRATIONS: if name in SERVER_SPECS:
return name return SERVER_SPECS[name]
return None return None
def detect_migratable_package(data: dict) -> str | None:
"""Return the registry key for the package this stdio server runs, or None.
Scans the command and every arg (so ``npx -y ssh-mcp`` and a direct
``command: ssh-mcp`` both resolve), matching on the normalised package name.
"""
spec = resolve_server_spec(data)
return spec.package if spec else None
def migrate_removed_flags(data: dict) -> tuple[dict, list[str]]: def migrate_removed_flags(data: dict) -> tuple[dict, list[str]]:
"""Move known removed credential flags out of `args` and into `env`. """Move known removed credential flags out of `args` and into `env`.
@@ -2416,6 +2499,42 @@ def removed_flag_warning(data: dict) -> str | None:
return warnings[0] if warnings else None return warnings[0] if warnings else None
def drift_warnings(data: dict) -> list[str]:
"""Advisory lines for flags whose *meaning* changed across a major version.
Distinct from ``removed_flag_warnings``: these flags still exist and still
parse, so nothing errors — but the value no longer means what the user set
it to mean (e.g. ssh-mcp's ``--maxChars=none``, which v1 silently capped at
5000 chars). There is no safe auto-fix: only a human knows the intent, so
this is warn-only. Matches both the inline ``--flag=value`` form and the
separate ``--flag value`` form. Empty list == nothing to flag.
"""
spec = resolve_server_spec(data)
if spec is None or not spec.drift_flags:
return []
# Reconstruct the set of "flag=value" tokens present, collapsing the two argv
# spellings into one so a drift key of the form "--flag=value" matches both.
args = [str(a) for a in (data.get("args") or [])]
present: set[str] = set()
i, n = 0, len(args)
while i < n:
a = args[i]
if a.startswith("-") and "=" in a:
present.add(a) # inline: --flag=value
i += 1
continue
if a.startswith("-") and i + 1 < n and not args[i + 1].startswith("-"):
present.add(f"{a}={args[i + 1]}") # separate: --flag value
i += 2
continue
i += 1
out: list[str] = []
for token in sorted(spec.drift_flags):
if token in present:
out.append(f"{spec.package}: {token} {spec.drift_flags[token]}")
return out
# --------------------------------------------------------------------------- # # --------------------------------------------------------------------------- #
# Validation # Validation
# --------------------------------------------------------------------------- # # --------------------------------------------------------------------------- #
+92 -3
View File
@@ -3169,13 +3169,38 @@ def test_removed_flag_warnings_covers_migratable_and_manual():
assert "--password" in text assert "--password" in text
assert "sudoPassword" in text assert "sudoPassword" in text
assert "disableSudo" in text assert "disableSudo" in text
# migrate only touches the confirmed --password mapping # #90: --password AND --sudoPassword are now both auto-migratable (the
# verified ssh-mcp facts map sudo/su → SSH_MCP_SUDO_PASSWORD). The migration
# LOGIC is unchanged — only the registry data grew — so both move into env.
# --disableSudo has no env replacement (sudo is now a role/policy) → warn-only.
new, _ = c.migrate_removed_flags(data) new, _ = c.migrate_removed_flags(data)
assert new["env"] == {"SSH_MCP_PASSWORD": "p"} assert new["env"] == {"SSH_MCP_PASSWORD": "p", "SSH_MCP_SUDO_PASSWORD": "s"}
assert "--sudoPassword=s" in new["args"] assert "--sudoPassword=s" not in new["args"]
assert "--disableSudo" in new["args"] assert "--disableSudo" in new["args"]
def test_migrate_su_password_maps_to_sudo_env():
# --suPassword is the second flag that maps to the same SSH_MCP_SUDO_PASSWORD var.
data = {"command": "ssh-mcp", "args": ["--suPassword", "rootpw"]}
new, notes = c.migrate_removed_flags(data)
assert new["env"] == {"SSH_MCP_SUDO_PASSWORD": "rootpw"}
assert "args" not in new
assert notes
def test_migrate_sudo_and_su_two_flags_one_var_no_clobber():
# Both sudo flags map to one var; the no-clobber path keeps the first, drops
# the second (different value) with a note rather than silently overwriting.
data = {
"command": "ssh-mcp",
"args": ["--sudoPassword=first", "--suPassword=second"],
}
new, notes = c.migrate_removed_flags(data)
assert new["env"] == {"SSH_MCP_SUDO_PASSWORD": "first"}
assert "args" not in new
assert any("already set" in n for n in notes)
def test_removed_flag_warnings_empty_when_clean(): def test_removed_flag_warnings_empty_when_clean():
assert c.removed_flag_warnings({"command": "ssh-mcp", "args": ["--host=h"]}) == [] assert c.removed_flag_warnings({"command": "ssh-mcp", "args": ["--host=h"]}) == []
assert c.removed_flag_warning({"command": "ssh-mcp", "args": ["--host=h"]}) is None assert c.removed_flag_warning({"command": "ssh-mcp", "args": ["--host=h"]}) is None
@@ -3208,6 +3233,70 @@ def test_ssh_membermatters_end_to_end():
assert notes assert notes
# --------------------------------------------------------------------------- #
# ServerSpec spine (issue #90)
# --------------------------------------------------------------------------- #
def test_server_spec_registry_seeds_ssh_mcp():
spec = c.SERVER_SPECS["ssh-mcp"]
assert spec.package == "ssh-mcp"
# sudo/su both map to the one sudo env var (verified facts).
assert spec.env_flags["--sudoPassword"] == "SSH_MCP_SUDO_PASSWORD"
assert spec.env_flags["--suPassword"] == "SSH_MCP_SUDO_PASSWORD"
assert spec.env_flags["--password"] == "SSH_MCP_PASSWORD"
# disableSudo stays warn-only (no env replacement).
assert "--disableSudo" in spec.removed_flags
assert "--disableSudo" not in spec.env_flags
# Verified sidecar paths seeded per platform (README path differs → doc_path).
assert "ssh-mcp/config.toml" in spec.sidecar_paths["darwin"]
assert "Application Support" in spec.sidecar_paths["darwin"]
assert "APPDATA" in spec.sidecar_paths["win32"]
assert spec.sidecar_doc_path == "~/.config/ssh-mcp/config.toml"
# zod enums seeded as data (pick-lists later — #7).
assert spec.schema["auth"] == ["agent", "key", "password", "keychain"]
assert spec.schema["approvalMode"] == ["auto", "ask-destructive", "ask-all", "deny"]
assert spec.schema["role"] == ["viewer", "operator", "admin"]
assert spec.schema["port"] == {"min": 1, "max": 65535}
def test_flag_env_migrations_is_derived_from_server_specs():
# The legacy constant is now a derived view; it must mirror the registry.
assert set(c.FLAG_ENV_MIGRATIONS) == set(c.SERVER_SPECS)
assert c.FLAG_ENV_MIGRATIONS["ssh-mcp"]["env"] == c.SERVER_SPECS["ssh-mcp"].env_flags
assert c.FLAG_ENV_MIGRATIONS["ssh-mcp"]["removed"] == c.SERVER_SPECS["ssh-mcp"].removed_flags
def test_resolve_server_spec_matches_detect_migratable_package():
for data in (
{"command": "npx", "args": ["-y", "ssh-mcp"]},
{"command": "ssh-mcp", "args": []},
{"command": "/usr/local/bin/ssh-mcp", "args": []},
{"command": "npx", "args": ["some-other"]},
{},
{"url": "https://x"},
):
spec = c.resolve_server_spec(data)
pkg = c.detect_migratable_package(data)
assert (spec.package if spec else None) == pkg
def test_drift_warning_maxchars_none_inline_and_separate():
inline = {"command": "ssh-mcp", "args": ["--maxChars=none"]}
separate = {"command": "ssh-mcp", "args": ["--maxChars", "none"]}
for data in (inline, separate):
warnings = c.drift_warnings(data)
assert len(warnings) == 1
assert "--maxChars=none" in warnings[0]
assert "5000" in warnings[0]
def test_drift_warning_quiet_for_other_values_and_unknown_pkg():
# A real numeric cap is fine; only "none" drifts.
assert c.drift_warnings({"command": "ssh-mcp", "args": ["--maxChars=8000"]}) == []
assert c.drift_warnings({"command": "ssh-mcp", "args": ["--host=h"]}) == []
# Unknown package: nothing to say.
assert c.drift_warnings({"command": "npx", "args": ["other", "--maxChars=none"]}) == []
# --------------------------------------------------------------------------- # # --------------------------------------------------------------------------- #
# Move to environment variable (issue #83) # Move to environment variable (issue #83)
# --------------------------------------------------------------------------- # # --------------------------------------------------------------------------- #