From 36ef1b76dce56a40650f890156eb137536ef015d Mon Sep 17 00:00:00 2001 From: Christophe Besson Date: Sun, 6 Sep 2026 19:11:05 +0200 Subject: fix(client): a successful root op must never blank the operator's table MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `ops.update_root` and `remove_root` returned `[]` when the group context had no live RootSet, and `group-page.js` accepted it: `if (msg.roots)` is true for an empty array. An op that succeeded would have emptied the shared-directories table, and "the node says this group has no directories" is not something the client can tell from "the node could not say". Both ends now refuse it — the node builds from config rather than answering empty, and the client requires a non-empty array. Found while adding `test_spa_imports.py`, which is the other half of this: it resolves every named import across the SPA against what the target actually exports. That failure has a shape nothing else here catches — no build step to fail, so the browser resolves the graph at load, finds a missing binding, and the page renders blank or the component just does not appear. `node --check` parses one file at a time and the source-reading guards look inside a file rather than between two. The settings split moved two shared components into a new module and rewired eight files to import them, which is exactly the change where a rename lands in one file and not the other. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_011pvMdvLBG92jyhvD5pD6us --- .../src/meshbay_hub/static/group-page.js | 6 +- packages/meshbay-hub/tests/test_spa_imports.py | 87 ++++++++++++++++++++++ packages/meshbay-node/src/meshbay_node/ops.py | 14 +++- 3 files changed, 104 insertions(+), 3 deletions(-) create mode 100644 packages/meshbay-hub/tests/test_spa_imports.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 747cb74..e762fc8 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/group-page.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/group-page.js @@ -374,7 +374,11 @@ function GroupPage({ groupId, group, token, username, userId, userPrefs, transport.onMusicbrainzEnabled = (enabled) => setMusicbrainzConfig((prev) => ({ ...(prev || {}), enabled })); transport.onRootsChanged = (msg) => { - if (msg.roots) { + // `msg.roots &&` would accept `[]`, and an empty array is truthy — + // so a node that could not describe its roots would blank the + // operator's table on an op that actually succeeded. A group always + // has at least one root, so nothing legitimate is dropped here. + if (Array.isArray(msg.roots) && msg.roots.length) { setNodeRoots(msg.roots); } }; diff --git a/packages/meshbay-hub/tests/test_spa_imports.py b/packages/meshbay-hub/tests/test_spa_imports.py new file mode 100644 index 0000000..230f33f --- /dev/null +++ b/packages/meshbay-hub/tests/test_spa_imports.py @@ -0,0 +1,87 @@ +""" +Every import in the SPA points at a file that exports what it names. + +The failure this catches has a distinctive shape: nothing errors at build time, +because there is no build; the browser resolves the module graph at load, finds +a binding that is not there, and the page renders blank or the component simply +does not appear. `node --check` cannot see it — it parses one file at a time — +and neither can the source-reading guards, which look inside a file rather than +between two. + +Written after the settings-page split, which moved two shared components into a +new module and rewired eight files to import them from there. That is exactly +the change where a rename lands in one file and not the other. + +It is a static check, not a load: it says the name is exported, not that the +value is what the caller expects. `test_spa_syntax` covers parsing; +`test_hook_ordering` covers the ordering fault that also presents as a missing +component. +""" + +import re +from pathlib import Path + +import pytest + +STATIC = Path(__file__).resolve().parents[1] / "src" / "meshbay_hub" / "static" + +pytestmark = pytest.mark.skipif(not STATIC.exists(), + reason="SPA sources unavailable") + +# `import { a, b as c } from './x.js';` / `import * as p from ...` / default. +IMPORT = re.compile( + r"^import\s+(?:\{([^}]*)\}|(\*\s+as\s+\w+)|(\w+))\s+from\s+'([^']+)';", re.M) +EXPORT_BLOCK = re.compile(r"^export\s+\{([^}]*)\};", re.M) +EXPORT_DECL = re.compile( + r"^export\s+(?:default\s+)?(?:async\s+)?(?:function|const|class|let|var)\s+(\w+)", + re.M) + + +def _exported(source: str) -> set[str]: + names = set(EXPORT_DECL.findall(source)) + for block in EXPORT_BLOCK.findall(source): + for raw in block.split(","): + name = raw.strip().split(" as ")[-1].strip() + if name: + names.add(name) + return names + + +def test_every_named_import_resolves(): + problems: list[str] = [] + for path in sorted(STATIC.glob("*.js")): + source = path.read_text(encoding="utf-8") + for names, star, default, spec in IMPORT.findall(source): + # vendor/ is third-party, bundled, and does not use a form this + # reads. Its exports are covered by the app failing to start. + if not spec.startswith("./") or "vendor/" in spec: + continue + target = (path.parent / spec[2:]).resolve() + if not target.exists(): + problems.append(f"{path.name}: imports {spec} — no such file") + continue + if star or default or not names.strip(): + continue + available = _exported(target.read_text(encoding="utf-8")) + for raw in names.split(","): + name = raw.strip().split(" as ")[0].strip() + if name and name not in available: + problems.append( + f"{path.name}: imports {{{name}}} from {spec}, " + f"which does not export it") + + assert not problems, "unresolved imports:\n" + "\n".join(problems) + + +def test_the_check_can_see_a_real_module(): + """ + Guard against the parser quietly matching nothing — a regex that stopped + finding imports would make the test above pass over an empty set, which + looks exactly like success. + """ + source = (STATIC / "apps.js").read_text(encoding="utf-8") + found = IMPORT.findall(source) + assert len(found) >= 5, ( + "the import pattern no longer matches apps.js — this test is then " + "checking nothing") + assert "configurableApps" in _exported(source) diff --git a/packages/meshbay-node/src/meshbay_node/ops.py b/packages/meshbay-node/src/meshbay_node/ops.py index f7f6190..11694c3 100644 --- a/packages/meshbay-node/src/meshbay_node/ops.py +++ b/packages/meshbay-node/src/meshbay_node/ops.py @@ -728,7 +728,12 @@ async def remove_root(state: dict, group_id: str, root_name: str) -> dict: live_roots.roots = [ r for r in live_roots.roots if fold(r.name) != target] - result_roots = live_roots.describe() if live_roots else [] + # Built from config when there is no live set, never returned empty: an + # empty list is a *valid answer* meaning "this group has no directories", + # and the client cannot tell it from "the node could not say". It would + # blank the operator's table on an op that succeeded. + result_roots = (live_roots.describe() if live_roots + else RootSet.build([asdict(r) for r in cfg.roots]).describe()) log.info("Root removed: %s from group %s", root_name, group_id[:8]) return {"status": "removed", "name": root_name, "group_id": group_id, @@ -788,7 +793,12 @@ async def update_root(state: dict, group_id: str, root_name: str, *, lr.removable = removable break - result_roots = live_roots.describe() if live_roots else [] + # Built from config when there is no live set, never returned empty: an + # empty list is a *valid answer* meaning "this group has no directories", + # and the client cannot tell it from "the node could not say". It would + # blank the operator's table on an op that succeeded. + result_roots = (live_roots.describe() if live_roots + else RootSet.build([asdict(r) for r in cfg.roots]).describe()) log.info("Root updated: %s (writable=%s, removable=%s) in group %s", root_name, match.writable, match.removable, group_id[:8]) -- cgit v1.2.3