diff options
| author | Christophe Besson <cbesson@gmail.com> | 2026-09-07 00:58:37 +0200 |
|---|---|---|
| committer | Christophe Besson <cbesson@gmail.com> | 2026-09-07 00:58:37 +0200 |
| commit | 2d3cbdec301c592daa2faff8e1ca1cab155ebb58 (patch) | |
| tree | a4d543fa3f54d7bcb4c9147869e5a02cc58a0239 /packages/meshbay-node | |
| parent | a36080742287bad8f657f688d7fec0022dd696a1 (diff) | |
| download | meshbay-2d3cbdec301c592daa2faff8e1ca1cab155ebb58.tar.gz | |
chore(node): finish Phase 3 — CLI deprecations, Windows shapes, docs
`member upload` reached the generic usage line for the other `member` verbs —
"usage: meshbay-node member upload <username>" — which advertises a removed
feature and sends the operator looking for a username it would then reject. It
names `root set --writable` now, and the man page carries the same. Three lines
between an operator finding the replacement and concluding the CLI is broken.
`--upload-dir` still works, so an existing script keeps working, but its help
and the man page say it is the old spelling and name what replaced it.
The Windows pass (§4.4) is what can be checked from here, made checkable:
drive letters and UNC through `as_posix()` into TOML, a drive root having no
basename to derive a name from — sharing a whole drive is ordinary there — and
a case-insensitive collision, which on NTFS and exFAT is one directory indexed
as two roots. `PureWindowsPath` throughout, for the reason the backslash test
earlier this branch got wrong.
What it cannot check is written down rather than glossed: ReadDirectoryChangesW
dropping events, MAX_PATH, and whether an eject actually lets a drive be
removed. §7d says so, along with two things the plan never considered — the
RO/RW asymmetry in `_do_dir_delete`, and `index_delta` carrying roots but not
`dirs`.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011pvMdvLBG92jyhvD5pD6us
Diffstat (limited to 'packages/meshbay-node')
| -rw-r--r-- | packages/meshbay-node/src/meshbay_node/daemon.py | 23 | ||||
| -rw-r--r-- | packages/meshbay-node/tests/test_cli_dispatch.py | 43 | ||||
| -rw-r--r-- | packages/meshbay-node/tests/test_windows_root_shapes.py | 149 |
3 files changed, 214 insertions, 1 deletions
diff --git a/packages/meshbay-node/src/meshbay_node/daemon.py b/packages/meshbay-node/src/meshbay_node/daemon.py index bcc4f6d..a9376e7 100644 --- a/packages/meshbay-node/src/meshbay_node/daemon.py +++ b/packages/meshbay-node/src/meshbay_node/daemon.py @@ -1784,7 +1784,8 @@ def main() -> None: parser.add_argument("--dir", default=None, help="shared directory, for group add") parser.add_argument("--upload-dir", default=None, - help="separate upload directory, for group add") + 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, @@ -2133,6 +2134,26 @@ def main() -> None: f"expires {i['expires_at']}") return + # `member upload` is gone: whether uploads are accepted is `writable` + # on the root they would land in, not a per-group switch. Named + # explicitly rather than left to the usage line below, which offered a + # username for a verb that no longer takes one — an operator following + # it would have got "unknown subcommand" and no idea what replaced it. + if sub == "upload": + print("`member upload` is gone. Uploads are decided per directory " + "now:") + print() + print(" meshbay-node root list " + "# which are read-write") + print(" meshbay-node root set <name> --writable " + "# accept uploads there") + print(" meshbay-node root set <name> --no-writable # stop them") + print() + print("A group whose directories are all read-only accepts no " + "uploads at all,") + print("which is what turning the old switch off meant.") + sys.exit(1) + if not args.target: print(f"usage: meshbay-node member {sub} <username>") sys.exit(1) diff --git a/packages/meshbay-node/tests/test_cli_dispatch.py b/packages/meshbay-node/tests/test_cli_dispatch.py index 2ba251f..1a01eba 100644 --- a/packages/meshbay-node/tests/test_cli_dispatch.py +++ b/packages/meshbay-node/tests/test_cli_dispatch.py @@ -42,6 +42,9 @@ VERBS = [ ["member", "invite", "bob"], ["member", "revoke", "bob"], ["member", "unpin", "bob"], + # Removed, and it has to say so rather than offering a username for a verb + # that no longer takes one. + ["member", "upload"], ["operator", "pair"], ["file", "list"], ["file", "rm", "abc", "--yes"], @@ -233,3 +236,43 @@ def test_a_bare_invocation_with_no_config_yet_exits_cleanly(monkeypatch, tmp_pat assert "meshbay-node init" in capsys.readouterr().out assert not missing_config.parent.exists(), ( "a fresh, unprovisioned start must not create anything on disk") + + +def test_a_removed_verb_says_what_replaced_it(): + """ + `member upload` used to set a group-wide switch that no longer exists. It + reached the usage line for the *other* member verbs — "usage: meshbay-node + member upload <username>" — which advertises a removed feature and sends + the operator looking for a username it would then reject. + + Naming it costs three lines and is the difference between an operator + finding `root set --writable` and concluding the CLI is broken. + """ + import inspect + source = inspect.getsource(daemon_mod.main) + start = source.index('if args.command == "member":') + block = source[start:source.index('if args.command == "group":', start)] + + assert 'sub == "upload"' in block, ( + "`member upload` falls through to the generic usage line") + guidance = block[block.index('sub == "upload"'):] + guidance = guidance[:guidance.index("sys.exit")] + assert "root set" in guidance and "--writable" in guidance, ( + "the message does not name what replaced it") + + +def test_the_help_does_not_offer_the_old_upload_directory_as_current(): + """ + `--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. + """ + 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" diff --git a/packages/meshbay-node/tests/test_windows_root_shapes.py b/packages/meshbay-node/tests/test_windows_root_shapes.py new file mode 100644 index 0000000..5c5da25 --- /dev/null +++ b/packages/meshbay-node/tests/test_windows_root_shapes.py @@ -0,0 +1,149 @@ +""" +The root model against the shapes Windows produces. + +CLAUDE.md is explicit that exFAT/NTFS and Windows are the common case, not an +edge case: most operators are expected to share from an external drive on +Windows. The RO/RW refactor added two booleans and a config rewriter, and the +booleans are path-independent — but the rewriter, the name derivation and the +collision check all touch paths, and none of them has ever run on Windows here. + +What this can check without Windows is the *shape* work: drive letters through +`as_posix()`, a path with no basename to derive a name from, UNC, and a +case-insensitive collision. `PureWindowsPath` is used deliberately — the plain +`Path` on this machine is a `PosixPath`, where a backslash is an ordinary +filename character, which is the mistake that made +`test_a_backslash_path_written_into_node_toml_stays_parseable` fail everywhere +but the platform it was written for. + +What it cannot check is the filesystem itself: `ReadDirectoryChangesW` dropping +events under load, `MAX_PATH`, and whether an eject actually lets a drive be +removed. Those need a person with Windows, and §4.4 of the refactor plan is +where that is written down. +""" + +import os +import tempfile +import tomllib +from pathlib import Path, PureWindowsPath + +import pytest + +from meshbay_node.roots import RootError, RootSet, derive_name + +BS = chr(92) + + +# ── Paths into node.toml ───────────────────────────────────────────────────── + +@pytest.mark.parametrize("raw,expected", [ + (f"D:{BS}Movies", "D:/Movies"), + (f"E:{BS}Music{BS}Albums", "E:/Music/Albums"), + (f"C:{BS}Users{BS}alice{BS}Media", "C:/Users/alice/Media"), + (f"{BS}{BS}server{BS}share{BS}Media", "//server/share/Media"), +]) +def test_a_windows_path_survives_the_config_file(raw, expected): + """ + `ops` writes `as_posix()` into a TOML basic string, where a raw backslash + is an escape — `\\U` and `\\a` are the ones that bite — so the file would + not parse at all. pathlib reads the forward-slash form back on Windows. + """ + posix = PureWindowsPath(raw).as_posix() + assert posix == expected + parsed = tomllib.loads(f'path = "{posix}"\n') + assert parsed["path"] == expected + + +def test_a_raw_windows_path_would_not_parse(): + """The counter-property: without `as_posix()` there is no config file.""" + with pytest.raises(tomllib.TOMLDecodeError): + tomllib.loads(f'path = "C:{BS}Users{BS}alice{BS}Media"\n') + + +# ── Naming a drive ─────────────────────────────────────────────────────────── + +@pytest.mark.parametrize("raw,name", [ + (f"D:{BS}Movies", "Movies"), + (f"E:{BS}Music{BS}Albums", "Albums"), + (f"{BS}{BS}server{BS}share{BS}Media", "Media"), +]) +def test_a_name_is_derived_from_the_last_segment(raw, name): + assert PureWindowsPath(raw).name == name + + +@pytest.mark.parametrize("raw", [f"D:{BS}", f"E:{BS}", f"{BS}{BS}server{BS}share"]) +def test_a_drive_root_has_no_name_to_derive(raw): + """ + Sharing a whole drive is an ordinary thing to do on Windows and there is + nothing to call it, so the operator has to say. Refused with that as the + message rather than named "" or "D:". + """ + p = PureWindowsPath(raw) + if p.name: + pytest.skip(f"{raw!r} has a basename on this platform") + with pytest.raises(RootError, match="explicit"): + derive_name(p) + + +def test_naming_it_explicitly_works(): + with tempfile.TemporaryDirectory() as d: + roots = RootSet.build([{"path": d, "name": "Films"}]) + assert roots.names == ["Films"] + + +# ── Case, which Windows makes real ─────────────────────────────────────────── + +def test_two_roots_differing_only_in_case_are_refused(): + """ + On NTFS and exFAT `Movies` and `MOVIES` are the same directory to the + filesystem and two roots to a case-sensitive comparison — which would index + one tree twice, and make deleting a file from one copy break the other. + """ + with tempfile.TemporaryDirectory() as d: + os.makedirs(os.path.join(d, "Movies")) + os.makedirs(os.path.join(d, "other")) + with pytest.raises(RootError, match="regard to case"): + RootSet.build([ + {"path": os.path.join(d, "Movies")}, + {"path": os.path.join(d, "other"), "name": "MOVIES"}, + ]) + + +def test_a_root_is_found_by_name_without_regard_to_case(): + """ + What a client sends is what a person typed or a path it split, and on + Windows those disagree about case routinely. + """ + with tempfile.TemporaryDirectory() as d: + os.makedirs(os.path.join(d, "Movies")) + roots = RootSet.build([{"path": os.path.join(d, "Movies")}]) + for spelling in ("Movies", "movies", "MOVIES", "MoViEs"): + assert roots.by_name(spelling) is not None, spelling + + +# ── The two flags ──────────────────────────────────────────────────────────── + +def test_the_flags_do_not_touch_paths(): + """ + `writable` and `removable` are booleans and stay booleans on every + platform. Stated as a test because it is the reason the rest of the + refactor needed no Windows work: what did need it is above. + """ + with tempfile.TemporaryDirectory() as d: + os.makedirs(os.path.join(d, "USB")) + roots = RootSet.build([{"path": os.path.join(d, "USB"), + "writable": True, "removable": True}]) + described = roots.describe()[0] + assert described["writable"] is True + assert described["removable"] is True + assert "path" not in described + + +def test_an_ejected_removable_root_is_unavailable_wherever_it_runs(): + with tempfile.TemporaryDirectory() as d: + os.makedirs(os.path.join(d, "USB")) + roots = RootSet.build([{"path": os.path.join(d, "USB"), + "removable": True, "ejected": True}]) + assert roots.roots[0].available is False + assert Path(roots.roots[0].path).is_dir(), ( + "the directory is still there; `ejected` is the operator's answer, " + "not the filesystem's") |