fix(node): remove --upload-dir rather than document it

Caught in review, and the review was right. The previous commit documented the
flag as deprecated so that `--help` and the man page would agree. That solved
the wrong problem: the flag contradicts the model this whole refactor exists to
establish, and the coherent answer was to delete it.

It wrote `upload_dir` into a *brand-new* `[[groups]]` block, and
`GroupConfig.__post_init__` reads that key by forcing every other root
read-only and appending that path as the one writable one. So
`group add --dir X --writable --upload-dir Y` silently made X read-only — two
mechanisms deciding which directories accept uploads, one of them invisible, in
a group created after the model that replaced it.

Gone from the CLI, from `ops.attach_group`, from the loopback API and from the
MNP `group_attach` payload, which now carries `writable` instead. The *read*
path in `config.py` is deliberately untouched: an existing node.toml using
`upload_dir` must keep working, and that is the only legitimate use left. The
man page says so under the config key, and no longer lists an option.

The test that guarded the deprecation wording now guards its absence — and
earned itself immediately by finding a `group add` usage string still offering
the flag.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011pvMdvLBG92jyhvD5pD6us

5 files changed, 45 insertions, 38 deletions

diff --git a/packages/meshbay-node/src/meshbay_node/daemon.py b/packages/meshbay-node/src/meshbay_node/daemon.py
index a9376e7..74fff4c 100644
--- a/packages/meshbay-node/src/meshbay_node/daemon.py
+++ b/packages/meshbay-node/src/meshbay_node/daemon.py
@@ -1783,9 +1783,6 @@ def main() -> None:
                         help="hub username, for init")
     parser.add_argument("--dir", default=None,
                         help="shared directory, for group add")
-    parser.add_argument("--upload-dir", default=None,
-                        help="deprecated: a second read-write root, for group "
-                             "add. Use `root add <path> --writable` instead")
     parser.add_argument("--yes", action="store_true",
                         help="skip the confirmation for destructive commands")
     parser.add_argument("--config", type=Path, default=None,
@@ -2571,11 +2568,13 @@ def main() -> None:
             print("usage: meshbay-node group list|add|remove <name>")
             sys.exit(1)
         if not args.target or not args.dir:
-            print("usage: meshbay-node group add <name> --dir <path> [--upload-dir <path>]")
+            print("usage: meshbay-node group add <name> --dir <path> "
+                  "[--no-writable]")
             print()
             print("The group must already exist on the hub and be yours. This")
-            print("only tells the node to host it, and picks the directory.")
-            print("--upload-dir sets a separate directory for uploaded files.")
+            print("only tells the node to host it, and picks its first")
+            print("directory, which accepts uploads unless --no-writable.")
+            print("Add more with:  meshbay-node root add <path> [--writable]")
             sys.exit(1)
 
         cfg = load_config(args.config or DEFAULT_CONFIG_PATH)
@@ -2586,17 +2585,10 @@ def main() -> None:
         writable = args.writable is not False
         body = {"name": args.target, "shared_dir": args.dir,
                 "writable": writable}
-        if args.upload_dir:
-            print("WARNING: --upload-dir is deprecated. The shared directory is "
-                  "read-write by default; use 'meshbay-node root add "
-                  "<path> --writable' for a second one.")
-            body["upload_dir"] = args.upload_dir
         out = _daemon_api(cfg, "/api/groups/attach", method="POST", body=body)
         print(f"{out['name']} ({out['group_id'][:8]}) added to {out['config']}")
         print(f"  shared_dir   {out['shared_dir']}"
               f"  ({'read-write' if writable else 'read-only'})")
-        if out.get("upload_dir"):
-            print(f"  upload_dir   {out['upload_dir']}")
         print()
         print("Tell the daemon to re-read its config, then give the group a key:")
         print("  meshbay-node reload")
diff --git a/packages/meshbay-node/src/meshbay_node/ops.py b/packages/meshbay-node/src/meshbay_node/ops.py
index 4c759c8..a10504e 100644
--- a/packages/meshbay-node/src/meshbay_node/ops.py
+++ b/packages/meshbay-node/src/meshbay_node/ops.py
@@ -394,7 +394,7 @@ async def list_groups(state: dict) -> dict:
 
 
 async def attach_group(state: dict, name: str, shared_dir: str,
-                       upload_dir: str = "", writable: bool = True) -> dict:
+                       writable: bool = True) -> dict:
     """
     Write a new [[groups]] block into node.toml.
 
@@ -439,15 +439,13 @@ async def attach_group(state: dict, name: str, shared_dir: str,
              f'name        = "{group["name"]}"\n'
              f'visibility  = "{group.get("visibility", "private")}"\n'
              f'join_policy = "{join_policy}"\n')
-    # Legacy: upload_dir becomes a second writable root
-    if upload_dir:
-        upload_path = Path(upload_dir).expanduser().resolve()
-        if upload_path != path.resolve():
-            try:
-                upload_path.mkdir(parents=True, exist_ok=True)
-            except OSError as e:
-                raise OpError(f"Cannot create {upload_path}: {e}") from e
-            block += f'upload_dir  = "{upload_path.as_posix()}"\n'
+    # No `upload_dir` here. `GroupConfig.__post_init__` still *reads* it, so an
+    # existing node.toml keeps working — but what it does on read is force every
+    # other root read-only and append that path as the one writable one, which
+    # is the model this refactor replaced. Writing it into a group created
+    # today would mean two mechanisms deciding the same thing, one of them
+    # invisible: `group add --dir X --writable --upload-dir Y` silently made X
+    # read-only. A second writable directory is `root add <path> --writable`.
     block += (f'\n  [[groups.roots]]\n'
               # Forward slashes: a Windows path in a TOML basic string is a
               # parse error (`\U`, `\a`, ... are escapes). pathlib reads `/`.
diff --git a/packages/meshbay-node/src/meshbay_node/transport/webrtc_server.py b/packages/meshbay-node/src/meshbay_node/transport/webrtc_server.py
index acdbe29..8e357c9 100644
--- a/packages/meshbay-node/src/meshbay_node/transport/webrtc_server.py
+++ b/packages/meshbay-node/src/meshbay_node/transport/webrtc_server.py
@@ -2483,11 +2483,14 @@ class WebRTCPeerSession:
         if not self._has_admin_authority():
             self._send({"type": "error", "detail": "No authorized key for this"})
             return
-        upload_dir = str(msg.get("upload_dir", "")).strip()
+        # `upload_dir` is not read here any more, and a client still sending it
+        # is ignored rather than obeyed: on load it forces every other root
+        # read-only, which is the model the RO/RW one replaced. A second
+        # writable directory is `root_add` with `writable`.
         self._issue_admin_challenge(
             OP_GROUP_ATTACH, name,
             payload={"name": name, "shared_dir": shared_dir,
-                     "upload_dir": upload_dir},
+                     "writable": bool(msg.get("writable", True))},
             group_id="")
 
     async def _admin_exec_group_attach(
@@ -2501,7 +2504,8 @@ class WebRTCPeerSession:
         p = pending.get("payload") or {}
         try:
             result = await self._run_op(
-                ops.attach_group, p["name"], p["shared_dir"], p.get("upload_dir", ""))
+                ops.attach_group, p["name"], p["shared_dir"],
+                writable=bool(p.get("writable", True)))
         except ops.OpError as e:
             self._send({"type": "error", "detail": e.message})
             return
diff --git a/packages/meshbay-node/src/meshbay_node/ui/app.py b/packages/meshbay-node/src/meshbay_node/ui/app.py
index 130c59e..fc6c04a 100644
--- a/packages/meshbay-node/src/meshbay_node/ui/app.py
+++ b/packages/meshbay-node/src/meshbay_node/ui/app.py
@@ -175,7 +175,6 @@ def create_ui_app(state: dict) -> FastAPI:
             state,
             (payload.get("name") or "").strip(),
             (payload.get("shared_dir") or "").strip(),
-            upload_dir=(payload.get("upload_dir") or "").strip(),
             writable=bool(payload.get("writable", True)),
         ))
         reload_fn = state.get("reload_fn")
diff --git a/packages/meshbay-node/tests/test_cli_dispatch.py b/packages/meshbay-node/tests/test_cli_dispatch.py
index 1a01eba..cf91564 100644
--- a/packages/meshbay-node/tests/test_cli_dispatch.py
+++ b/packages/meshbay-node/tests/test_cli_dispatch.py
@@ -261,18 +261,32 @@ def test_a_removed_verb_says_what_replaced_it():
         "the message does not name what replaced it")
 
 
-def test_the_help_does_not_offer_the_old_upload_directory_as_current():
+def test_there_is_no_way_to_create_a_group_in_the_old_shape():
     """
-    `--upload-dir` still works — an existing script passing it keeps working —
-    but the help has to say it is the old spelling, or it reads as the way to
-    do this.
+    `--upload-dir` is gone, and documenting it as deprecated was the wrong
+    answer — which is what it got at first.
+
+    It wrote `upload_dir` into a brand-new `[[groups]]` block, and
+    `GroupConfig.__post_init__` reads that by forcing *every other root
+    read-only* and appending that path as the one writable one. So
+    `group add --dir X --writable --upload-dir Y` silently made X read-only:
+    two mechanisms deciding which directories accept uploads, one of them
+    invisible, in a group created after the model that replaced it.
+
+    Reading it stays — an existing node.toml must keep working, and that is the
+    only legitimate use. Writing it does not.
     """
     import inspect
     source = inspect.getsource(daemon_mod.main)
-    # The whole call, not just its first string: an adjacent-literal help text
-    # is several strings, and matching only the first is how a test passes over
-    # the half that carries the meaning.
-    start = source.index('"--upload-dir"')
-    call = source[start:source.index("parser.add_argument", start + 1)]
-    assert "deprecated" in call.lower()
-    assert "root add" in call, "it does not name the replacement"
+    assert "--upload-dir" not in source, (
+        "the CLI can still create a group in the pre-RO/RW shape")
+
+    from meshbay_node import ops
+    params = inspect.signature(ops.attach_group).parameters
+    assert "upload_dir" not in params, (
+        "attach_group still writes the legacy key")
+
+    # The read path is deliberately untouched.
+    from meshbay_node.config import GroupConfig
+    assert "upload_dir" in inspect.getsource(GroupConfig), (
+        "an existing node.toml using upload_dir would stop working")