fix(groups): finish Phase 1 — MNP root management, upload targets, eject state

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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011pvMdvLBG92jyhvD5pD6us

1 files changed, 64 insertions, 17 deletions

diff --git a/packages/meshbay-node/tests/test_roots.py b/packages/meshbay-node/tests/test_roots.py
index 1beb220..505091b 100644
--- a/packages/meshbay-node/tests/test_roots.py
+++ b/packages/meshbay-node/tests/test_roots.py
@@ -108,31 +108,61 @@ def test_a_sibling_with_a_shared_prefix_is_fine(tmp_path):
     assert roots.names == ["Media", "Media2"]
 
 
-# ── Uploads ──────────────────────────────────────────────────────────────────
+# ── Writable roots ───────────────────────────────────────────────────────────
 
-def test_a_single_root_receives_uploads_without_being_asked(tmp_path):
+def test_a_root_is_read_only_unless_it_says_otherwise(tmp_path):
+    """
+    The default is the safe one. An operator who shares a directory has not
+    thereby agreed to let anyone write into it, and the version of this that
+    guessed — one root, so it must be the upload target — meant adding a
+    second directory silently changed what the first one was.
+    """
     (tmp_path / "Media").mkdir()
     roots = RootSet.build([_spec(tmp_path / "Media")])
-    assert roots.upload_root is roots.roots[0]
+    assert roots.roots[0].writable is False
+    assert roots.writable_roots == []
+
+
+def test_several_roots_can_be_writable_at_once(tmp_path):
+    (tmp_path / "A").mkdir()
+    (tmp_path / "B").mkdir()
+    (tmp_path / "C").mkdir()
+    roots = RootSet.build([_spec(tmp_path / "A", writable=True),
+                           _spec(tmp_path / "B"),
+                           _spec(tmp_path / "C", writable=True)])
+    assert [r.name for r in roots.writable_roots] == ["A", "C"]
 
 
-def test_several_roots_and_no_designation_means_no_uploads(tmp_path):
+def test_a_fully_read_only_group_is_valid(tmp_path):
     """
-    Refused, never guessed: picking one would send a member's file to a disk the
-    operator did not intend, and that is discovered weeks later.
+    A group that only publishes is the point of the read-only model, not a
+    misconfiguration — build must not refuse it, and nothing downstream may
+    promote a root to writable to have somewhere to put an upload.
     """
     (tmp_path / "A").mkdir()
     (tmp_path / "B").mkdir()
     roots = RootSet.build([_spec(tmp_path / "A"), _spec(tmp_path / "B")])
-    assert roots.upload_root is None
+    assert roots.writable_roots == []
+    assert len(roots) == 2
 
 
-def test_two_upload_roots_are_refused(tmp_path):
-    (tmp_path / "A").mkdir()
-    (tmp_path / "B").mkdir()
-    with pytest.raises(RootError, match="exactly one"):
-        RootSet.build([_spec(tmp_path / "A", upload=True),
-                       _spec(tmp_path / "B", upload=True)])
+def test_the_old_upload_flag_still_reads_as_writable(tmp_path):
+    """A node.toml written before this refactor must not change meaning."""
+    (tmp_path / "Media").mkdir()
+    roots = RootSet.build([_spec(tmp_path / "Media", upload=True)])
+    assert roots.roots[0].writable is True
+    assert roots.describe()[0]["writable"] is True
+
+
+def test_writable_wins_over_a_leftover_upload_flag(tmp_path):
+    """
+    A config carrying both is one a migration touched. `writable` is the field
+    the operator's tooling writes now, so it is the one that decides — reading
+    the legacy field there would undo the migration on the next load.
+    """
+    (tmp_path / "Media").mkdir()
+    roots = RootSet.build([_spec(tmp_path / "Media", upload=True, writable=False)])
+    assert roots.roots[0].writable is False
 
 
 # ── Resolution ───────────────────────────────────────────────────────────────
@@ -236,18 +266,35 @@ def test_availability_follows_the_directory(tmp_path):
 def test_describe_reports_what_a_member_needs(tmp_path):
     (tmp_path / "Media").mkdir()
     (tmp_path / "Music").mkdir()
-    roots = RootSet.build([_spec(tmp_path / "Media", upload=True),
-                           _spec(tmp_path / "Music", kind="audio")])
+    roots = RootSet.build([_spec(tmp_path / "Media", writable=True),
+                           _spec(tmp_path / "Music", kind="audio",
+                                 removable=True)])
     described = roots.describe()
     assert described == [
-        {"name": "Media", "kind": "generic", "available": True, "upload": True},
-        {"name": "Music", "kind": "audio", "available": True, "upload": False},
+        {"name": "Media", "kind": "generic", "available": True,
+         "writable": True, "removable": False, "ejected": False,
+         "upload": True},
+        {"name": "Music", "kind": "audio", "available": True,
+         "writable": False, "removable": True, "ejected": False,
+         "upload": False},
     ]
     # Deliberately no paths: a member is told what exists and whether it is
     # readable, not where on the operator's disk it lives.
     assert not any("path" in d for d in described)
 
 
+def test_describe_still_carries_upload_for_mnp_1_0_clients(tmp_path):
+    """
+    `upload` is `writable` under its old name, kept because an MNP 1.0 client
+    reads no other field and would otherwise decide the group takes no uploads
+    at all. It is derived, never stored — the two can never disagree.
+    """
+    (tmp_path / "Media").mkdir()
+    roots = RootSet.build([_spec(tmp_path / "Media", writable=True)])
+    described = roots.describe()[0]
+    assert described["upload"] == described["writable"] is True
+
+
 # ── SAFE_UPLOAD_NAME ────────────────────────────────────────────────────────
 
 def test_safe_name_accepts_unicode_letters():