From d6e1cc19a09df35988f6a90c226c94f2fdf6b209 Mon Sep 17 00:00:00 2001 From: Christophe Besson Date: Sun, 6 Sep 2026 22:20:46 +0200 Subject: fix(node): `root list` printed "?" for every path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `RootSet.describe()` feeds two audiences that want opposite things. The index payload goes to every member and has always deliberately carried no paths — a member is told what exists and whether it is readable, not that the library sits under someone's home directory. The loopback API answers the operator themselves, over a channel that already requires their machine and the run token, and the path is exactly what they asked for. The `root list` CLI I added in Phase 1 read `path` from the member form, so it printed a placeholder for every directory. Nothing caught it: the CLI reads a dict, the API returns a dict, and neither end states which keys it owes. `describe(with_paths=True)` is the operator's view and `list_groups` is the only caller. The shared-directories table has the same hole over MNP — the roots there come from the index payload — so its Path column now appears only when a path is actually present, rather than rendering a column of blanks. The test asserts both halves, because they pull opposite ways: one that only checked the operator sees paths would be satisfied by leaking them to every member. It reads the indexer's source for the member side, and compares the CLI's key reads against what the payload offers for the other — checked to fail in each direction independently. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_011pvMdvLBG92jyhvD5pD6us --- .../tests/test_root_paths_are_operator_only.py | 115 +++++++++++++++++++++ 1 file changed, 115 insertions(+) create mode 100644 packages/meshbay-node/tests/test_root_paths_are_operator_only.py (limited to 'packages/meshbay-node/tests/test_root_paths_are_operator_only.py') diff --git a/packages/meshbay-node/tests/test_root_paths_are_operator_only.py b/packages/meshbay-node/tests/test_root_paths_are_operator_only.py new file mode 100644 index 0000000..6ba2e08 --- /dev/null +++ b/packages/meshbay-node/tests/test_root_paths_are_operator_only.py @@ -0,0 +1,115 @@ +""" +Where a directory lives on the operator's disk is theirs, not the group's. + +`RootSet.describe()` feeds two very different audiences. The index payload goes +to every member, and has always deliberately carried no paths — a member is +told what exists and whether it is readable, not that the library sits in +`/media//BACKUP2`. The loopback API answers the operator +themselves, over a channel that already requires being on their machine with +the run token, where the path is exactly what they are asking for. + +`meshbay-node root list` printed `?` for every directory because it read a +field the member form omits. Nothing caught it: the CLI reads a dict, the +payload is a dict, and neither end says what keys it owes the other. + +Both halves matter and they pull opposite ways, so both are asserted here — a +test that only checked the operator gets paths would be satisfied by putting +them in the member payload too. +""" + +import inspect +import re +from pathlib import Path + +import pytest + +from meshbay_node import daemon as daemon_mod +from meshbay_node import ops +from meshbay_node.roots import RootSet + + +def _roots(tmp_path: Path) -> RootSet: + for name in ("Films", "Albums"): + (tmp_path / name).mkdir() + return RootSet.build([ + {"path": str(tmp_path / "Films"), "writable": True}, + {"path": str(tmp_path / "Albums"), "removable": True}, + ]) + + +# ── The member's half ──────────────────────────────────────────────────────── + +def test_the_default_form_carries_no_path(tmp_path): + described = _roots(tmp_path).describe() + assert described, "no roots described" + assert not any("path" in d for d in described), ( + "the index payload every member receives would carry the operator's " + "filesystem layout") + + +def test_the_default_form_still_says_what_a_member_needs(tmp_path): + """The counter-property: dropping the path must not drop the rest.""" + described = _roots(tmp_path).describe() + for d in described: + assert set(d) >= {"name", "kind", "available", "writable", + "removable", "ejected"} + + +def test_the_index_payload_is_built_without_paths(): + """ + Read from the source, because the alternative is asserting it about a + payload built by a test rather than by the node. + """ + from meshbay_node.indexer import indexer as indexer_mod + source = inspect.getsource(indexer_mod) + for call in re.findall(r"roots\.describe\([^)]*\)", source): + assert "with_paths" not in call, ( + f"the indexer builds the member-facing roots table as {call} — " + f"that payload goes to everyone in the group") + + +# ── The operator's half ────────────────────────────────────────────────────── + +def test_the_operator_form_carries_the_path(tmp_path): + described = _roots(tmp_path).describe(with_paths=True) + assert all(d.get("path") for d in described) + assert described[0]["path"] == str(tmp_path / "Films") + + +def test_the_loopback_api_asks_for_paths(): + """ + `list_groups` answers the operator's own channel, and the CLI's `root list` + prints what it returns. Asking for the member form there is what printed a + column of question marks. + """ + source = inspect.getsource(ops.list_groups) + assert "describe(with_paths=True)" in source, ( + "list_groups uses the member form, so every path it reports is missing") + + +def test_the_cli_only_reads_fields_the_payload_carries(): + """ + The gap this whole file exists for. The CLI reads a dict and the API + returns a dict; nothing between them says which keys are owed, so a name + that is simply absent prints as a placeholder and looks like a node + problem. + """ + source = inspect.getsource(daemon_mod.main) + start = source.index('if args.command == "root":') + block = source[start:source.index('if args.command == "operator":', start)] + + read = set(re.findall(r"r\.get\(['\"](\w+)['\"]", block)) + read |= set(re.findall(r"r\[['\"](\w+)['\"]\]", block)) + assert read, "the root CLI no longer reads the payload this way" + + class _Any: + path = Path("/tmp/x") + name = "x" + kind = "generic" + writable = removable = ejected = False + available = True + + offered = set(RootSet(roots=[_Any()]).describe(with_paths=True)[0]) + assert read <= offered, ( + f"the `root` CLI reads keys the loopback payload does not carry: " + f"{sorted(read - offered)}") -- cgit v1.2.3 From 13ccc4a16c604f46ed10b6c5dbfbc8fdd7d008c0 Mon Sep 17 00:00:00 2001 From: Christophe Besson Date: Sun, 6 Sep 2026 23:01:36 +0200 Subject: fix: a root change reaches every client without a page reload MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The directory table travelled on `index_sync` alone — a *full* index, which the node only ever sends on request. Every ongoing change went out as an `index_delta`, which carried files and nothing else. So the message that says "something changed" was the one message that could not say a root had. The acks hid it: `root_add_ack` and friends broadcast the new table to whoever is connected, so the common cases looked right. What that could not cover was the operator's own client, where the ack landed and was then overwritten — the table calls `onRefreshIndex` after an add, that fetch returns the set from *before* the node's reload (fire-and-forget, because a rescan is minutes on a real library), and `applyIndex` writes it over what the ack had just delivered. The new directory appeared for one paint and vanished. Two halves. `index_delta` now carries the roots table, sealed with the rest and identical to `index_sync`'s — additive, so a 1.0 client sees a field it does not read. And the table no longer refreshes the index after a root change: the ack gives it the new set immediately, and the delta the node pushes when the scan finishes gives it again, along with the files. The test that pins it uses an *eject* as its case, because an eject changes no file at all — the entries freeze — so its delta is empty of additions, deletions and updates. Without the table it says literally nothing, which is how a library disappearing from under a group went unannounced to everyone but whoever pressed the button. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_011pvMdvLBG92jyhvD5pD6us --- .../src/meshbay_hub/static/group-page.js | 7 ++ .../src/meshbay_hub/static/group-settings.js | 24 +++- packages/meshbay-node/src/meshbay_node/daemon.py | 3 +- .../src/meshbay_node/transport/wire.py | 12 +- .../tests/test_index_delta_carries_roots.py | 131 +++++++++++++++++++++ .../tests/test_root_paths_are_operator_only.py | 1 - 6 files changed, 169 insertions(+), 9 deletions(-) create mode 100644 packages/meshbay-node/tests/test_index_delta_carries_roots.py (limited to 'packages/meshbay-node/tests/test_root_paths_are_operator_only.py') diff --git a/packages/meshbay-hub/src/meshbay_hub/static/group-page.js b/packages/meshbay-hub/src/meshbay_hub/static/group-page.js index a1e6411..0a43724 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/group-page.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/group-page.js @@ -244,6 +244,13 @@ function GroupPage({ groupId, group, token, username, userId, userPrefs, // enrichment (duration/thumb_hash/display_title/...) arriving for a file // already in the table — same id, new fields (see group_index.py diff()). const applyIndexDelta = useCallback((deltaMsg) => { + // The roots table rides on the delta as of MNP 1.1. Before that it + // travelled only on a full index_sync, which is sent on request — so a + // root added, removed, ejected or plugged by anyone left every other + // client's directory table stale until they reloaded the page. + if (Array.isArray(deltaMsg.roots) && deltaMsg.roots.length) { + setNodeRoots(deltaMsg.roots); + } setEntries((prev) => { const deletions = new Set(deltaMsg.deletions || []); const kept = prev.filter((e) => !deletions.has(e.id)); diff --git a/packages/meshbay-hub/src/meshbay_hub/static/group-settings.js b/packages/meshbay-hub/src/meshbay_hub/static/group-settings.js index ce36a40..9646405 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/group-settings.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/group-settings.js @@ -42,7 +42,8 @@ import * as platform from './platform.js'; * nodeDetected — whether the loopback node API answers * readOnly — suppress every edit control * onRootsChange — called after a change, to re-read the loopback list - * onRefreshIndex — full index refresh, needed after an add or a remove + * onRefreshIndex — full index refresh. Not called after a root change: see + * `run()` for why the node's own push is what settles it * mode — "live" (default) or "local" * localRoots / onLocalRootsChange — the array, in "local" mode */ @@ -106,18 +107,29 @@ function SharedDirectoriesTable({ roots, groupId, transport, signFn, const rootUrl = (name, suffix = '') => '/api/groups/' + groupId + '/roots/' + encodeURIComponent(name) + suffix; - const run = useCallback(async (work, { refreshIndex = false } = {}) => { + // Deliberately no index refresh after a root change. + // + // Adding a root makes the node reload, which rescans — minutes on a real + // library — and the reload is fire-and-forget for that reason. Fetching the + // index in the moment after therefore returns the set from *before* it, and + // `applyIndex` writes that over the roots the ack had just delivered: the + // new directory appeared for one paint and vanished, which is what "it only + // shows up after a refresh" was. + // + // Nothing is lost by waiting. The ack carries the new table immediately, and + // the delta the node pushes when the scan finishes carries it again along + // with the files. + const run = useCallback(async (work) => { setBusy(true); setMsg(''); try { await work(); if (onRootsChange) await onRootsChange(); - if (refreshIndex && onRefreshIndex) await onRefreshIndex(); return true; } catch (err) { setMsg(platform.bridgeMessage(err)); return false; } finally { setBusy(false); } - }, [onRootsChange, onRefreshIndex]); + }, [onRootsChange]); const doUpdateRoot = useCallback(async (rootName, updates) => { if (isLocal) { @@ -171,7 +183,7 @@ function SharedDirectoriesTable({ roots, groupId, transport, signFn, await platform.node.call('DELETE', rootUrl(rootName)); await platform.node.call('POST', '/api/reload'); } else throw new Error(t('node.root_no_route')); - }, { refreshIndex: true }); + }); if (ok) setMsg(t('node.root_removed')); }, [isLocal, localRoots, onLocalRootsChange, overMnp, overLoopback, transport, groupId, signFn, run]); @@ -203,7 +215,7 @@ function SharedDirectoriesTable({ roots, groupId, transport, signFn, await platform.node.call('POST', '/api/reload'); await platform.watchIndexProgress(groupId, setIndexProgress); } else throw new Error(t('node.root_no_route')); - }, { refreshIndex: true }); + }); }, [isLocal, localRoots, onLocalRootsChange, overMnp, overLoopback, transport, groupId, signFn, run]); diff --git a/packages/meshbay-node/src/meshbay_node/daemon.py b/packages/meshbay-node/src/meshbay_node/daemon.py index 7637b41..028918d 100644 --- a/packages/meshbay-node/src/meshbay_node/daemon.py +++ b/packages/meshbay-node/src/meshbay_node/daemon.py @@ -1240,7 +1240,8 @@ class NodeDaemon: # node refuses every handshake while the GEK is None (NS8) — so this is # "nobody is listening", not a case to send in clear for. if peers and idx.gek: - msg = (index_delta_message(idx, delta) if delta is not None + msg = (index_delta_message(idx, delta, indexer.roots) + if delta is not None else index_sync_message(idx, indexer.roots)) pushed = 0 for session in peers: diff --git a/packages/meshbay-node/src/meshbay_node/transport/wire.py b/packages/meshbay-node/src/meshbay_node/transport/wire.py index c683204..6986b01 100644 --- a/packages/meshbay-node/src/meshbay_node/transport/wire.py +++ b/packages/meshbay-node/src/meshbay_node/transport/wire.py @@ -89,13 +89,21 @@ def index_sync_message(index, roots: RootSet | None) -> dict: } -def index_delta_message(index, delta) -> dict: +def index_delta_message(index, delta, roots=None) -> dict: """ One `index_delta` — what changed since the last thing this node broadcast. Built here rather than inline in the daemon, which is where it lived and which made it the third place an index message was constructed: precisely the drift that produced two `index_sync` encodings and two `file_chunk` encodings before it. + + `roots` rides along (MNP 1.1, additive — a 1.0 client ignores it). It used + to travel on `index_sync` alone, which is a *full* index and therefore only + ever sent on request. So a root added, removed, ejected or plugged left + every connected client's directory table stale until somebody reloaded the + page: the delta that told them something had changed was the one message + that could not say what. It is a handful of dicts, bounded by the number of + directories a group has, and it is sealed with the rest. """ payload = { "base_version": delta.base_version, @@ -104,6 +112,8 @@ def index_delta_message(index, delta) -> dict: "deletions": list(delta.deletions), "updates": [index_entry_wire(e) for e in delta.updates], } + if roots is not None: + payload["roots"] = roots.describe() return { "type": MNP.INDEX_DELTA, "v": MNP_VERSION, diff --git a/packages/meshbay-node/tests/test_index_delta_carries_roots.py b/packages/meshbay-node/tests/test_index_delta_carries_roots.py new file mode 100644 index 0000000..227f2d0 --- /dev/null +++ b/packages/meshbay-node/tests/test_index_delta_carries_roots.py @@ -0,0 +1,131 @@ +""" +The message that says something changed has to be able to say what. + +A group's directory table travelled on `index_sync` alone — a *full* index, +which the node only ever sends on request. Every ongoing change went out as an +`index_delta`, which carried files and nothing else. So a root added, removed, +ejected or plugged by the operator reached every other client's screen only +when somebody happened to reload the page. + +It was hidden by the acks: `root_add_ack` and friends broadcast the new table +to whoever was connected, so the common cases looked fine. What that could not +cover is a client connecting mid-change, one whose ack was lost, or — the one +that surfaced it — the operator's own client, where the ack landed and was then +overwritten by an index fetched before the node had rebuilt anything. + +Additive on the wire (MNP 1.1): a 1.0 client sees a field it does not read. +""" + +from pathlib import Path + +from cryptography.hazmat.primitives.asymmetric.ed25519 import Ed25519PrivateKey + +from meshbay_common.crypto import generate_gek +from meshbay_common.groupbox import PURPOSE_INDEX, unseal +from meshbay_node.indexer.group_index import GroupIndex +from meshbay_node.roots import RootSet +from meshbay_node.transport.wire import index_delta_message, index_sync_message + + +GROUP = "g" * 32 + + +def _roots(tmp_path: Path) -> RootSet: + for name in ("Films", "Albums"): + (tmp_path / name).mkdir() + return RootSet.build([ + {"path": str(tmp_path / "Films"), "writable": True}, + {"path": str(tmp_path / "Albums"), "removable": True}, + ]) + + +def _index() -> GroupIndex: + return GroupIndex(group_id=GROUP, sk_node=Ed25519PrivateKey.generate(), + gek=generate_gek()) + + +def _payload(msg: dict, index: GroupIndex) -> dict: + """What a member actually reads, through the seal rather than around it.""" + return unseal(index.gek, PURPOSE_INDEX, msg["type"], GROUP, msg) + + +class _Delta: + base_version = 1 + version = 2 + additions: list = [] + deletions: list = [] + updates: list = [] + + +def test_a_delta_carries_the_directory_table(tmp_path): + index = _index() + msg = index_delta_message(index, _Delta(), _roots(tmp_path)) + payload = _payload(msg, index) + + assert [r["name"] for r in payload["roots"]] == ["Films", "Albums"] + assert payload["roots"][0]["writable"] is True + assert payload["roots"][1]["removable"] is True + + +def test_the_table_is_sealed_with_the_rest(tmp_path): + """ + It is group content, not routing. Only `type`, `v` and `group_id` stay in + clear, because a receiver has to route and authenticate before it would + trust a decryption. + """ + index = _index() + msg = index_delta_message(index, _Delta(), _roots(tmp_path)) + assert set(msg) - {"type", "v", "group_id"}, "nothing was sealed" + assert "roots" not in msg, "the directory table is outside the envelope" + + +def test_a_delta_still_works_without_a_table(tmp_path): + """ + The argument is optional, so an older caller — or a path that has no root + set to hand — produces a message a client reads exactly as before. + """ + index = _index() + payload = _payload(index_delta_message(index, _Delta()), index) + assert "roots" not in payload + assert payload["version"] == 2 + + +def test_the_table_says_the_same_thing_on_both_messages(tmp_path): + """ + Two encodings of one idea is the drift `wire.py` exists to prevent — it + already happened twice, for `index_sync` and for `file_chunk`. + """ + index = _index() + roots = _roots(tmp_path) + delta = _payload(index_delta_message(index, _Delta(), roots), index) + sync = _payload(index_sync_message(index, roots), index) + assert delta["roots"] == sync["roots"] + + +def test_the_table_never_carries_a_path(tmp_path): + """ + This message goes to every member. Where a directory lives on the + operator's disk is theirs — see test_root_paths_are_operator_only.py. + """ + index = _index() + payload = _payload(index_delta_message(index, _Delta(), _roots(tmp_path)), + index) + assert not any("path" in r for r in payload["roots"]) + + +def test_an_ejected_root_is_visible_in_the_delta(tmp_path): + """ + The case this was written for. An eject changes no file — the entries + freeze — so the delta it produces is empty of additions, deletions and + updates. Without the table it says literally nothing, which is how a + library disappearing from under the group's feet went unannounced. + """ + index = _index() + roots = _roots(tmp_path) + roots.roots[1].ejected = True + roots.roots[1].available = False + + payload = _payload(index_delta_message(index, _Delta(), roots), index) + assert payload["additions"] == [] and payload["deletions"] == [] + assert payload["roots"][1]["ejected"] is True + assert payload["roots"][1]["available"] is False diff --git a/packages/meshbay-node/tests/test_root_paths_are_operator_only.py b/packages/meshbay-node/tests/test_root_paths_are_operator_only.py index 6ba2e08..080d4be 100644 --- a/packages/meshbay-node/tests/test_root_paths_are_operator_only.py +++ b/packages/meshbay-node/tests/test_root_paths_are_operator_only.py @@ -21,7 +21,6 @@ import inspect import re from pathlib import Path -import pytest from meshbay_node import daemon as daemon_mod from meshbay_node import ops -- cgit v1.2.3