feat(#102): core — stdlib TOML read/validate/surgical-write + atomic sidecar writer
The pure, fully-tested half of the in-app sidecar editor. No new runtime dependency: CI's test job installs only `pytest cryptography` (not requirements.txt) and the catalog-signature job imports bcc_core with only cryptography, and the workflow is off-limits — so a top-level TOML import is impossible and any pip TOML dep would leave this code untested/red on CI. - read_toml_section() — lenient, section-aware scalar reader (string/int/ float/bool); values it can't confidently decode are omitted (the writer preserves them regardless). - toml_sections() — ordered profile/section groups for the editor's picker. - set_toml_value() / update_toml() — SURGICAL writer: rewrites only the one key it's asked to, so comments, formatting, ordering and unknown keys/tables round-trip untouched (strictly safer than parse->dict->reserialize, which tomli-w wouldn't comment-preserve either). CRLF-preserving; correct scalar quoting/escaping; creates a missing section; deletes on value=None. - validate_sidecar_values() — enum/port validation against ServerSpec.schema for the MANAGED fields only; unknown keys pass through (preserved, never a save-blocker) — reconciles "reject out-of-enum" with "preserve unknown keys". - write_sidecar() — atomic temp-write + os.replace + rotating backup (reuses the extracted _atomic_write_text + _make_backup) then chmod 0600/0700 via #93's fix_permissions. Never routes through apply_servers (cardinal rule). - _make_backup() generalised to the file's own suffix (JSON naming unchanged), so a .toml sidecar and a .json config keep separate backup pools. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
ded1eef2dd
commit
c5a6bdd1d1
@@ -3745,6 +3745,177 @@ def test_sidecar_state_fingerprint_tracks_args_inert_flip():
|
||||
assert c.sidecar_state_changed(before, after)
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------- #
|
||||
# Sidecar TOML editing (issue #102)
|
||||
# --------------------------------------------------------------------------- #
|
||||
def test_read_toml_section_top_level_and_named():
|
||||
text = (
|
||||
"# a comment\n"
|
||||
'title = "root"\n'
|
||||
"port = 22\n"
|
||||
"enabled = true\n"
|
||||
"\n"
|
||||
"[server]\n"
|
||||
'host = "h"\n'
|
||||
"auth = 'key'\n" # literal string
|
||||
"weight = 1.5\n"
|
||||
)
|
||||
top = c.read_toml_section(text)
|
||||
assert top == {"title": "root", "port": 22, "enabled": True}
|
||||
srv = c.read_toml_section(text, "server")
|
||||
assert srv == {"host": "h", "auth": "key", "weight": 1.5}
|
||||
# An absent section reads as empty.
|
||||
assert c.read_toml_section(text, "nope") == {}
|
||||
|
||||
|
||||
def test_read_toml_section_skips_unparseable_values():
|
||||
text = '[server]\nhosts = [1, 2, 3]\ninline = {a = 1}\nname = "ok"\n'
|
||||
# Arrays / inline tables are omitted (surgical writer preserves them); the
|
||||
# simple scalar is read.
|
||||
assert c.read_toml_section(text, "server") == {"name": "ok"}
|
||||
|
||||
|
||||
def test_toml_sections_lists_groups_in_order():
|
||||
text = "top = 1\n[server]\nx=1\n[prod]\na=1\n[prod.auth]\nk=1\n[[hosts]]\nn=1\n"
|
||||
# top-level present -> "" first; [prod] and [prod.auth] collapse to one group.
|
||||
assert c.toml_sections(text) == ["", "server", "prod", "hosts"]
|
||||
assert c.toml_sections("[only]\nx=1\n") == ["only"]
|
||||
|
||||
|
||||
def test_set_toml_value_replaces_and_preserves_comment():
|
||||
text = '[server]\nport = 22 # ssh\nhost = "h"\n'
|
||||
out = c.set_toml_value(text, "port", 2222, section="server")
|
||||
assert "port = 2222 # ssh" in out
|
||||
# Nothing else changed.
|
||||
assert 'host = "h"' in out
|
||||
assert out.startswith("[server]\n")
|
||||
|
||||
|
||||
def test_set_toml_value_appends_missing_key_within_section():
|
||||
text = '[server]\nhost = "h"\n\n[other]\nx = 1\n'
|
||||
out = c.set_toml_value(text, "auth", "key", section="server")
|
||||
lines = out.splitlines()
|
||||
# The new key lands inside [server], before the blank line and [other].
|
||||
assert lines.index('auth = "key"') < lines.index("[other]")
|
||||
assert lines.index('auth = "key"') > lines.index("[server]")
|
||||
assert "[other]" in out and "x = 1" in out
|
||||
|
||||
|
||||
def test_set_toml_value_creates_missing_section():
|
||||
text = 'title = "root"\n'
|
||||
out = c.set_toml_value(text, "role", "admin", section="server")
|
||||
assert 'title = "root"' in out
|
||||
assert "[server]" in out
|
||||
assert 'role = "admin"' in out
|
||||
# And it round-trips back through the reader.
|
||||
assert c.read_toml_section(out, "server") == {"role": "admin"}
|
||||
|
||||
|
||||
def test_set_toml_value_top_level_key():
|
||||
text = "a = 1\n[server]\nb = 2\n"
|
||||
out = c.set_toml_value(text, "a", 5) # section=None -> top-level
|
||||
assert out.startswith("a = 5\n")
|
||||
# The [server] b is untouched.
|
||||
assert c.read_toml_section(out, "server") == {"b": 2}
|
||||
|
||||
|
||||
def test_set_toml_value_delete_key():
|
||||
text = '[server]\nhost = "h"\nport = 22\n'
|
||||
out = c.set_toml_value(text, "port", None, section="server")
|
||||
assert "port" not in out
|
||||
assert 'host = "h"' in out
|
||||
# Deleting an absent key is a no-op.
|
||||
assert c.set_toml_value(text, "ghost", None, section="server") == text
|
||||
|
||||
|
||||
def test_set_toml_value_preserves_crlf():
|
||||
text = "[server]\r\nport = 22\r\n"
|
||||
out = c.set_toml_value(text, "port", 23, section="server")
|
||||
assert "port = 23\r\n" in out
|
||||
assert "\r\n" in out
|
||||
|
||||
|
||||
def test_set_toml_value_escapes_strings():
|
||||
out = c.set_toml_value("", "path", 'C:\\a\\"b"', section="win")
|
||||
# Backslashes and quotes are escaped; it reads back to the exact original.
|
||||
assert c.read_toml_section(out, "win") == {"path": 'C:\\a\\"b"'}
|
||||
|
||||
|
||||
def test_update_toml_batch_and_roundtrip():
|
||||
text = '[server]\nhost = "h" # keep me\n'
|
||||
out = c.update_toml(text, {"host": "newhost", "port": 22, "auth": "key"}, section="server")
|
||||
assert c.read_toml_section(out, "server") == {"host": "newhost", "port": 22, "auth": "key"}
|
||||
# The comment on the pre-existing host line survives the value change.
|
||||
assert "# keep me" in out
|
||||
|
||||
|
||||
def test_validate_sidecar_values_enums_and_port():
|
||||
schema = c.SERVER_SPECS["ssh-mcp"].schema
|
||||
# All valid -> no problems.
|
||||
assert (
|
||||
c.validate_sidecar_values(
|
||||
{"auth": "key", "approvalMode": "ask-all", "role": "admin", "port": 22}, schema
|
||||
)
|
||||
== []
|
||||
)
|
||||
# Bad enum value.
|
||||
probs = c.validate_sidecar_values({"auth": "sshkey"}, schema)
|
||||
assert probs and "auth" in probs[0] and "sshkey" in probs[0]
|
||||
# Out-of-range port, and a bool is not a valid int port.
|
||||
assert c.validate_sidecar_values({"port": 70000}, schema)
|
||||
assert c.validate_sidecar_values({"port": True}, schema)
|
||||
assert c.validate_sidecar_values({"port": 1}, schema) == []
|
||||
|
||||
|
||||
def test_validate_sidecar_values_passes_through_unknown_keys():
|
||||
schema = c.SERVER_SPECS["ssh-mcp"].schema
|
||||
# An unmanaged key is preserved, never a save-blocker.
|
||||
assert c.validate_sidecar_values({"customThing": "whatever"}, schema) == []
|
||||
|
||||
|
||||
def test_write_sidecar_atomic_backup_and_chmod(tmp_path):
|
||||
p = tmp_path / "ssh-mcp" / "config.toml"
|
||||
p.parent.mkdir()
|
||||
p.write_text("[server]\nport = 22\n", encoding="utf-8")
|
||||
|
||||
chmods: list[tuple[str, int]] = []
|
||||
new_text = c.set_toml_value(p.read_text(encoding="utf-8"), "port", 2222, section="server")
|
||||
backup = c.write_sidecar(
|
||||
p,
|
||||
new_text,
|
||||
platform="linux",
|
||||
chmod=lambda path, mode: chmods.append((Path(path).as_posix(), mode)),
|
||||
)
|
||||
|
||||
# File updated atomically with the new content.
|
||||
assert c.read_toml_section(p.read_text(encoding="utf-8"), "server") == {"port": 2222}
|
||||
# A timestamped backup of the prior content was made, under .toml.
|
||||
assert backup is not None and backup.suffix == ".toml"
|
||||
assert c.read_toml_section(backup.read_text(encoding="utf-8"), "server") == {"port": 22}
|
||||
# Permissions tightened: file 0600, dir 0700 (via #93's fix_permissions).
|
||||
assert (p.as_posix(), 0o600) in chmods
|
||||
assert (p.parent.as_posix(), 0o700) in chmods
|
||||
|
||||
|
||||
def test_write_sidecar_new_file_no_backup(tmp_path):
|
||||
p = tmp_path / "config.toml"
|
||||
backup = c.write_sidecar(
|
||||
p, '[server]\nrole = "viewer"\n', platform="linux", chmod=lambda *_: None
|
||||
)
|
||||
assert backup is None # nothing pre-existing to back up
|
||||
assert p.read_text(encoding="utf-8").endswith("\n")
|
||||
assert c.read_toml_section(p.read_text(encoding="utf-8"), "server") == {"role": "viewer"}
|
||||
|
||||
|
||||
def test_write_sidecar_does_not_touch_apply_servers(tmp_path):
|
||||
# Guard the cardinal rule: write_sidecar writes ONLY the text it is given —
|
||||
# no mcpServers / _disabledMcpServers keys are introduced.
|
||||
p = tmp_path / "config.toml"
|
||||
c.write_sidecar(p, '[server]\nhost = "h"\n', platform="linux", chmod=lambda *_: None)
|
||||
body = p.read_text(encoding="utf-8")
|
||||
assert "mcpServers" not in body and "_disabledMcpServers" not in body
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------- #
|
||||
# Move to environment variable (issue #83)
|
||||
# --------------------------------------------------------------------------- #
|
||||
|
||||
Reference in New Issue
Block a user