From ea56b8c79538323875c00db2e7006b255f7cd494 Mon Sep 17 00:00:00 2001 From: Christophe Besson Date: Sun, 6 Sep 2026 17:48:36 +0200 Subject: fix(groups): finish Phase 1 — MNP root management, upload targets, eject state MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review of the Phase 1 commit found the RO/RW model sound but three paths unfinished, each of which broke the flow the phase exists to deliver. Plus 29 test failures it introduced and no coverage for anything it added. Uploads went to the wrong directory. The node read a `root` field on file_upload that no client ever sent, so every upload landed in the first writable root while the Files toolbar offered its button based on the root being browsed — with two writable roots, uploading from one wrote into the other. Files now names the root it is showing; Chat names one chosen in the shell (an operator-configured directory arrives in Phase 2); the node refuses an unknown name rather than falling back, and refuses read-only and ejected roots by code. Shared directories were unreachable on the web. The table read its roots only from the loopback API, which resolves to "not available" in a browser, so the section rendered for nobody there — while the Uploads controls it replaced had worked — and the transport.updateRoot/ejectRoot/plugRoot methods beside it were dead. MNP is now the path, loopback the fallback for a local node with no live connection, and adding a root over MNP takes a typed path since no web page can browse a remote disk. Ejecting updated nobody's screen. transport.js resolves an admin ack against the pending request and returns, which is right for every op whose caller knows the value it chose; the root acks carry state only the node can compute, so the operator who clicked Eject was the one client that never saw it happen. And the ejected flag reached roster.db but was never read back, so a restart undid it and the next scan read an empty mount point as an erased library. Also: the member-upload endpoint answered 200 and did nothing (removed); the wizard ignored the first root's RW switch; reload compared roots on name and path, so editing writable in node.toml did nothing; the table had no path column, which is the only thing separating two libraries sharing a basename; apps_enabled normalisation differed between the two sides of a signed subject. Tests: eject/plug, per-root upload refusal and the node.toml rewrite had no coverage at all. test_member_upload_policy.py is replaced by test_root_writable_policy.py — it tested a removed feature — and every property worth keeping from it moved rather than being dropped. Docs: draft-v6 structural decision 9 is annotated as superseded (the operator can no longer have a directory only they may write to — a real capability removed, flagged rather than hidden), the man page documents the root verb and the RO/RW fields, and refactor-groups.md §7b records what the plan got wrong. Suite: 41 failures before, 13 after — all 13 pre-existing on main. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_011pvMdvLBG92jyhvD5pD6us --- packages/meshbay-node/tests/test_node_status.py | 117 +++++++++++++++++++++--- 1 file changed, 106 insertions(+), 11 deletions(-) (limited to 'packages/meshbay-node/tests/test_node_status.py') diff --git a/packages/meshbay-node/tests/test_node_status.py b/packages/meshbay-node/tests/test_node_status.py index b56eb6e..091b1db 100644 --- a/packages/meshbay-node/tests/test_node_status.py +++ b/packages/meshbay-node/tests/test_node_status.py @@ -255,7 +255,7 @@ async def test_add_root_creates_directory_and_returns_info(tmp_path): from meshbay_node.config import NodeConfig, GroupConfig, RootSpec cfg = GroupConfig(id=GROUP, name="test", roots=[ - RootSpec(path=str(shared), name="shared", kind="generic", upload=True), + RootSpec(path=str(shared), name="shared", kind="generic", writable=True), ]) conf = tmp_path / "node.toml" @@ -295,7 +295,7 @@ async def test_remove_root_requires_at_least_one_remaining(tmp_path): from meshbay_node.config import GroupConfig, RootSpec, NodeConfig cfg = GroupConfig(id=GROUP, name="test", roots=[ - RootSpec(path=str(shared), name="shared", kind="generic", upload=True), + RootSpec(path=str(shared), name="shared", kind="generic", writable=True), ]) node_cfg = NodeConfig.__new__(NodeConfig) node_cfg.groups = [cfg] @@ -314,16 +314,22 @@ async def test_remove_root_requires_at_least_one_remaining(tmp_path): await ops.remove_root(state, GROUP, "shared") -async def test_remove_root_refuses_upload_root(tmp_path): - d1 = tmp_path / "uploads" +async def test_removing_a_writable_root_is_allowed(tmp_path): + """ + It used to be refused: with one designated upload root, removing it left + the group with nowhere to put an upload and no way to say so. Several roots + can be writable now, and a group with none is a valid read-only group — so + the refusal would be protecting a state that is no longer special. + """ + d1 = tmp_path / "incoming" d2 = tmp_path / "shared" d1.mkdir() d2.mkdir() from meshbay_node.config import GroupConfig, RootSpec, NodeConfig cfg = GroupConfig(id=GROUP, name="test", roots=[ - RootSpec(path=str(d1), name="uploads", kind="generic", upload=True), - RootSpec(path=str(d2), name="shared", kind="generic", upload=False), + RootSpec(path=str(d1), name="incoming", kind="generic", writable=True), + RootSpec(path=str(d2), name="shared", kind="generic", writable=False), ]) node_cfg = NodeConfig.__new__(NodeConfig) node_cfg.groups = [cfg] @@ -331,7 +337,7 @@ async def test_remove_root_refuses_upload_root(tmp_path): conf = tmp_path / "node.toml" conf.write_text( f'[[groups]]\nid = "{GROUP}"\nname = "test"\n\n' - f' [[groups.roots]]\n path = "{d1}"\n name = "uploads"\n upload = true\n\n' + f' [[groups.roots]]\n path = "{d1}"\n name = "incoming"\n writable = true\n\n' f' [[groups.roots]]\n path = "{d2}"\n name = "shared"\n') roots = RootSet.build([asdict(r) for r in cfg.roots]) index = GroupIndex(group_id=GROUP, sk_node=Ed25519PrivateKey.generate()) @@ -340,8 +346,97 @@ async def test_remove_root_refuses_upload_root(tmp_path): "config_path": str(conf), "groups_ctx": {GROUP: {"index": index, "roots": roots, "gek": b"\x01" * 32}}, } - with pytest.raises(ops.OpError, match="upload root"): - await ops.remove_root(state, GROUP, "uploads") + result = await ops.remove_root(state, GROUP, "incoming") + assert result["status"] == "removed" + assert [r["name"] for r in result["roots"]] == ["shared"] + assert conf.read_text().count("[[groups.roots]]") == 1 + + +async def test_update_root_rewrites_the_flags_in_node_toml(tmp_path): + """ + The flags live in the operator's config file, so they survive a restart — + and the file is hand-written and full of comments, so the change is a line + edit rather than a round trip through a TOML writer that would discard + every one of them. + """ + d1 = tmp_path / "media" + d1.mkdir() + + from meshbay_node.config import GroupConfig, RootSpec, NodeConfig + cfg = GroupConfig(id=GROUP, name="test", roots=[ + RootSpec(path=str(d1), name="media", kind="generic", writable=False), + ]) + node_cfg = NodeConfig.__new__(NodeConfig) + node_cfg.groups = [cfg] + + conf = tmp_path / "node.toml" + conf.write_text( + f'[[groups]]\nid = "{GROUP}"\nname = "test"\n\n' + f' [[groups.roots]]\n' + f' # the operator explained this one to themselves\n' + f' path = "{d1}"\n name = "media"\n') + roots = RootSet.build([asdict(r) for r in cfg.roots]) + index = GroupIndex(group_id=GROUP, sk_node=Ed25519PrivateKey.generate()) + state = { + "config": node_cfg, + "config_path": str(conf), + "groups_ctx": {GROUP: {"index": index, "roots": roots, "gek": b"\x01" * 32}}, + } + + result = await ops.update_root(state, GROUP, "media", + writable=True, removable=True) + assert result["status"] == "updated" + text = conf.read_text() + assert "writable = true" in text + assert "removable = true" in text + assert "the operator explained this one to themselves" in text, ( + "the config file was rewritten instead of edited") + + # And the live root set agrees immediately, without waiting for a reload: + # the loopback API reads it, and an operator who toggles a switch and sees + # it snap back assumes the change did not take. + assert roots.roots[0].writable is True + assert roots.roots[0].removable is True + + # A second call that changes nothing must not append a duplicate line. + await ops.update_root(state, GROUP, "media", writable=True, removable=True) + assert conf.read_text().count("writable =") == 1 + + +async def test_update_root_replaces_a_legacy_upload_line(tmp_path): + """ + A config written before the refactor says `upload = true`. Leaving it in + place next to a new `writable` line would give the file two answers, and + `RootSet.build` prefers `writable` — so the stale one would sit there + contradicting the running node for as long as anyone read it. + """ + d1 = tmp_path / "media" + d1.mkdir() + + from meshbay_node.config import GroupConfig, RootSpec, NodeConfig + cfg = GroupConfig(id=GROUP, name="test", roots=[ + RootSpec(path=str(d1), name="media", kind="generic", writable=True), + ]) + node_cfg = NodeConfig.__new__(NodeConfig) + node_cfg.groups = [cfg] + + conf = tmp_path / "node.toml" + conf.write_text( + f'[[groups]]\nid = "{GROUP}"\nname = "test"\n\n' + f' [[groups.roots]]\n path = "{d1}"\n name = "media"\n' + f' upload = true\n') + roots = RootSet.build([asdict(r) for r in cfg.roots]) + index = GroupIndex(group_id=GROUP, sk_node=Ed25519PrivateKey.generate()) + state = { + "config": node_cfg, + "config_path": str(conf), + "groups_ctx": {GROUP: {"index": index, "roots": roots, "gek": b"\x01" * 32}}, + } + + await ops.update_root(state, GROUP, "media", writable=False) + text = conf.read_text() + assert "upload = true" not in text + assert "writable = false" in text async def test_remove_root_succeeds_with_two_roots(tmp_path): @@ -352,8 +447,8 @@ async def test_remove_root_succeeds_with_two_roots(tmp_path): from meshbay_node.config import GroupConfig, RootSpec, NodeConfig cfg = GroupConfig(id=GROUP, name="test", roots=[ - RootSpec(path=str(d1), name="dir1", kind="generic", upload=True), - RootSpec(path=str(d2), name="dir2", kind="generic", upload=False), + RootSpec(path=str(d1), name="dir1", kind="generic", writable=True), + RootSpec(path=str(d2), name="dir2", kind="generic", writable=False), ]) node_cfg = NodeConfig.__new__(NodeConfig) node_cfg.groups = [cfg] -- cgit v1.2.3