From 32a86417d0edc3bf5c4cf859243c9e34b29bf1ef Mon Sep 17 00:00:00 2001 From: Christophe Besson Date: Thu, 10 Sep 2026 19:26:10 +0200 Subject: fix(node): the handshake ack dropped one app's directories MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The ack was assembled from its own tuple of application names, a copy of the daemon's `APP_DIR_KEYS`, and the two had drifted: the copy was missing `helloworld`. So the reference application — the one that exists to prove a new application needs no special-casing — was the single application whose configured folders never reached a client, which made the plugin claim false exactly where it is demonstrated. Fixed by removing the copy rather than syncing it. The ack now emits whatever `_directories` the group context carries, and `_app_directories_ctx` is the only thing that puts one there, so the two cannot disagree again. The transport names an application in one place, `ALLOWED_APPS`, which is enforcement rather than a directory list. The client had the same fault one layer up: `group-page.js` read three names by hand from the ack while the live-update path beside it was already generic. It derives the map from the ack's own keys now, so the fix reaches the settings pane instead of stopping at the wire. A first attempt moved the list to `roster.py`, where directory *storage* lives, and `test_helloworld_proves_the_plugin_claim.py` refused it: the roster, the ops, the config and the root set must name no application at all. That test is the architecture's own guard and it was right — the list belongs on the daemon, which is what wires a group's context, and everything downstream is derived from it. Two new tests, both verified to fail against the previous shape: the ack carries an application the node names nowhere else, and the ack keeps no list of its own. `test_the_lists_are_read_under_one_name_each` now asserts the shell names no application rather than that it names exactly three. Two stale comments went with it — the ack's, which described scalars removed in 07ff8b4, and the client's, which said those scalars still rode the wire for MNP 1.0 peers that can no longer connect. Full suite: 2258 passed, 4 skipped. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01YVoHVCcfBqud6ZjG4db3y7 --- .../meshbay-node/tests/test_app_directories.py | 66 +++++++++++++++++++++- 1 file changed, 64 insertions(+), 2 deletions(-) (limited to 'packages/meshbay-node/tests') diff --git a/packages/meshbay-node/tests/test_app_directories.py b/packages/meshbay-node/tests/test_app_directories.py index 69b3be5..726f5bd 100644 --- a/packages/meshbay-node/tests/test_app_directories.py +++ b/packages/meshbay-node/tests/test_app_directories.py @@ -14,8 +14,9 @@ Two properties are new rather than moved, and both matter more than the tidying: then matched no entry — an app showing an empty tab, with nothing to distinguish "misconfigured" from "no files yet". The moment of setting is the only one where the operator is present to be told; -* **the legacy scalar is derived, never stored.** `video_root` still rides on - the handshake ack for MNP 1.0 clients. Kept as a second stored value it would +* **a second name is derived, never stored.** `video_root` and friends are gone + from the wire entirely; `chat_directory` is the one that survives, and it is + computed from the list on every build. Kept as a second *stored* value it would drift from the list within one run — the shape of bug that reads as "it works after a restart". """ @@ -370,3 +371,64 @@ async def test_an_empty_set_is_signable(tmp_path): session = _handler_session(tmp_path, authorized=True) session._do_app_directories({"app": "video", "directories": []}) assert session.issued == [(OP_APP_DIRECTORIES, "video:")] + + +# ── One list of applications, not two ──────────────────────────────────────── + +async def test_the_ack_carries_every_app_the_context_knows_about(): + """ + The handshake ack emits whatever `_directories` the group context + holds, so an application cannot be configurable on the node and invisible on + the wire. + + There were two lists until 2026-09-10 — the daemon built the context from + one and the ack was assembled from a copy — and they had already drifted by + one entry. The application that entry belonged to is the reference app, + which exists precisely to prove that a new application needs no + special-casing; it was the single application whose directories never + reached a client, so the claim was false exactly where it is demonstrated. + + Deriving the ack from the context removes the second list rather than + syncing it, which is the only version of this that cannot drift again. + """ + session = WebRTCPeerSession.__new__(WebRTCPeerSession) + session._ctx = { + "video_directories": ["Media/Films"], + "photo_directories": [], + # An application nothing on the node names. It reaches the ack because + # the context carries it, which is the whole property. + "helloworld_directories": ["Media"], + # Not directories, and must not be swept in. + "chat_directory": "Media", + "chat_epoch": 3, + } + session._group_id = "" + + ack = session._app_directories_ack() + + assert ack == { + "video_directories": ["Media/Films"], + "photo_directories": [], + "helloworld_directories": ["Media"], + } + + +async def test_the_ack_has_no_list_of_applications_of_its_own(): + """ + `NodeDaemon.APP_DIR_KEYS` is the list, and the transport must not keep a + copy: a second list is a second thing to remember when an application is + added, and the one that is forgotten disagrees silently. + + The transport names an application in exactly one place — `ALLOWED_APPS`, + which is server-side enforcement rather than a directory list. + """ + from meshbay_node.daemon import NodeDaemon + + source = Path(WebRTCPeerSession.__module__.replace(".", "/")) + text = (Path(__file__).resolve().parents[2] / "meshbay-node" / "src" + / f"{source}.py").read_text(encoding="utf-8") + body = text[text.index("def _app_directories_ack"):] + body = body[:body.index("\n def ", 1)] + for app in NodeDaemon.APP_DIR_KEYS: + assert app not in body, ( + f"the ack names {app!r}; it must read the context's own keys") -- cgit v1.2.3