feat(client): Phase 2 — per-app settings panes, folder tree, multi-directory

Each app's settings were inlined in `group-settings.js` — TMDB, MusicBrainz,
and one folder picker per app, each with its own draft state and save handler
saying the same thing about a different key. They are one file per app now,
reached through the `apps.js` registry, and the page that renders them names no
application at all: adding one is a registry entry and a settings file.

The line between the two is what makes that true. What every app has — folders
— the page does generically, through one `saveDirectories` bound to the app.
What one app alone has, its pane does itself with the transport it is handed.
An app that only needs directories touches neither `group-settings.js` nor
`group-page.js`, which is `test_app_settings_plugin.py`'s subject.

`settings-ui.js` exists because a pane importing the page that renders it is a
cycle, and ES modules answer that with a temporal-dead-zone ReferenceError at
first render — a component that silently does not appear, the fault already
recorded in CLAUDE.md about hook ordering.

The flat depth-indented `<select>` of every folder in the library becomes a
modal tree. It asks the node for nothing: the tree is derived from paths the
client already holds, so it shows exactly what the group's index contains and
adds no folder-browsing protocol. For Chat's attachment folder — the one
directory that is written to rather than read — read-only roots are greyed
out, so the node's refusal arrives before the operator picks rather than when
somebody sends a file.

Videos and Music take a list of folders. A library on two drives could not be
described before; the only recourse was pointing the app at a parent containing
both, which pulls in everything else under it. The scalar shapes survive on the
wire alone, for a node speaking MNP 1.0, and the client reads them as a
one-element list.

Two things the tests caught that I would not have:

`test_asset_versioning` — six new modules were missing from `_ASSETS`. Reached
through the registry rather than imported by name, they are exactly the files
nothing else would notice changing, and a stale one is served from cache with
no version bump.

And `node --check foo.js` does **not** reliably report a module syntax error:
it accepted `${/* ... */''}` — htm template syntax pasted into a plain object
literal — and reported success. A `.mjs` copy forces the module parser and
reports it. The suite had no syntax check at all, which is how that reached a
file; `test_spa_syntax.py` does it for every module now, and pins that the
loose path is not what it uses.

Suite: 12 failures, all pre-existing.

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

5 files changed, 382 insertions, 6 deletions

diff --git a/packages/meshbay-hub/tests/test_app_settings_plugin.py b/packages/meshbay-hub/tests/test_app_settings_plugin.py
new file mode 100644
index 0000000..00a07b0
--- /dev/null
+++ b/packages/meshbay-hub/tests/test_app_settings_plugin.py
@@ -0,0 +1,277 @@
+"""
+Adding an application must not mean editing the pages that render it.
+
+That is the whole claim of the plugin architecture, and it is the kind of claim
+that decays silently: the first special case for one app reads as harmless, and
+by the third the loop is a lookup table with a default branch. These tests are
+what makes the claim checkable.
+
+They are source-reading, which is weak evidence and the only kind available for
+the SPA. Where a stronger check exists it is used instead — `test_spa_syntax`
+parses every module, and `test_hook_ordering` catches the temporal-dead-zone
+fault this refactor's new import graph could otherwise reintroduce.
+"""
+
+import re
+from pathlib import Path
+
+import pytest
+
+STATIC = Path(__file__).resolve().parents[1] / "src" / "meshbay_hub" / "static"
+APPS = STATIC / "apps.js"
+GROUP_SETTINGS = STATIC / "group-settings.js"
+GROUP_PAGE = STATIC / "group-page.js"
+SETTINGS_UI = STATIC / "settings-ui.js"
+FOLDER_TREE = STATIC / "folder-tree.js"
+TRANSPORT = STATIC / "transport.js"
+# parents[2] is `packages/` — the tests live at
+# packages/meshbay-hub/tests/, so [0] is tests, [1] the package, [2] packages.
+# Got this wrong once and the two cross-package checks below skipped silently,
+# which is worse than not having them: a green run that measured nothing.
+NODE_SERVER = (Path(__file__).resolve().parents[2] / "meshbay-node" / "src"
+               / "meshbay_node" / "transport" / "webrtc_server.py")
+
+PANES = ["chat-app-settings.js", "video-app-settings.js",
+         "music-app-settings.js", "photos-app-settings.js"]
+
+pytestmark = pytest.mark.skipif(not APPS.exists(),
+                                reason="SPA sources unavailable")
+
+
+def _component(source: str, name: str) -> str:
+    start = source.index(f"\nfunction {name}(")
+    end = source.find("\nfunction ", start + 1)
+    return source[start:end if end != -1 else len(source)]
+
+
+def _code_only(source: str) -> str:
+    """
+    The same source with comments removed.
+
+    A prose explanation of what moved out of a file is not the file naming an
+    app — and the note recording *why* TMDB is no longer here is exactly the
+    kind of comment this codebase wants kept. Crude on purpose: it does not
+    understand strings containing `//`, which for these files is fine and for
+    a parser would be a second implementation of one.
+    """
+    source = re.sub(r"/\*.*?\*/", "", source, flags=re.S)
+    return re.sub(r"^\s*//.*$", "", source, flags=re.M)
+
+
+# ── The registry is the only place an app is named ──────────────────────────
+
+def test_the_settings_page_names_no_application():
+    """
+    The apps loop renders whatever the registry holds. A branch on `'video'`
+    here is the first step back to the 1338-line page this replaced, where
+    every app's settings were inlined and the file grew with each one.
+    """
+    panel = _code_only(_component(GROUP_SETTINGS.read_text(encoding="utf-8"),
+                                  "GroupSettingsPanel"))
+    for app in ("'video'", "'music'", "'photo'",
+                "tmdb", "musicbrainz", "TMDB"):
+        assert app not in panel, (
+            f"group-settings.js still names {app} — an app's own settings "
+            f"belong in its settings file")
+
+
+def test_the_settings_page_renders_the_registry():
+    panel = _component(GROUP_SETTINGS.read_text(encoding="utf-8"),
+                       "GroupSettingsPanel")
+    assert "configurableApps()" in panel
+    assert "app.Settings" in panel, "the registry's component is not rendered"
+
+
+def test_every_registered_app_has_a_key_the_node_would_accept():
+    """
+    The registry key is the identifier everywhere: the tab, `apps_enabled`,
+    the `app_directories` op, and the roster row the directories live in. A
+    key here that `ALLOWED_APPS` does not have is an app whose settings are
+    refused by the node with no clue why.
+    """
+    node = NODE_SERVER
+    if not node.exists():
+        pytest.skip("the node package is not in this checkout")
+    m = re.search(r"ALLOWED_APPS = frozenset\(\{([^}]*)\}\)",
+                  node.read_text(encoding="utf-8"))
+    assert m, "ALLOWED_APPS moved"
+    allowed = set(re.findall(r"'([^']+)'|\"([^\"]+)\"", m.group(1)))
+    allowed = {a or b for a, b in allowed}
+
+    keys = set(re.findall(r"\{ key: '([^']+)'", APPS.read_text(encoding="utf-8")))
+    assert keys, "no app keys found — did the registry's shape change?"
+    assert keys <= allowed, (
+        f"registered apps the node would refuse: {sorted(keys - allowed)}")
+
+
+# ── One contract, every pane ────────────────────────────────────────────────
+
+@pytest.mark.parametrize("pane", PANES)
+def test_every_pane_takes_the_same_props(pane):
+    """
+    A pane that reached for something else would make the loop that renders
+    them conditional, which is the same thing as the page naming apps again.
+    """
+    source = (STATIC / pane).read_text(encoding="utf-8")
+    m = re.search(r"function \w+Settings\(\{([^}]*)\}\)", source)
+    assert m, f"{pane}: no settings component with a destructured props object"
+    props = {p.strip() for p in m.group(1).split(",") if p.strip()}
+    allowed = {"roots", "dirs", "settings", "saveDirectories",
+               "transport", "signFn"}
+    assert props <= allowed, (
+        f"{pane} takes props outside the shared contract: "
+        f"{sorted(props - allowed)}")
+
+
+@pytest.mark.parametrize("pane", PANES)
+def test_no_pane_imports_the_page_that_renders_it(pane):
+    """
+    `group-settings` → `apps` → a pane → `group-settings` is a cycle, and ES
+    modules answer it with a temporal-dead-zone ReferenceError at first render
+    rather than an import error — the component simply does not appear. That
+    is why the shared widgets live in `settings-ui.js`.
+    """
+    source = (STATIC / pane).read_text(encoding="utf-8")
+    assert "group-settings.js" not in source, (
+        f"{pane} imports the page that renders it — that is an import cycle")
+
+
+@pytest.mark.parametrize("pane", PANES)
+def test_every_pane_owns_its_own_busy_state(pane):
+    """
+    One shared flag would disable every section while any one of them saves,
+    and attribute one section's error message to another.
+    """
+    source = (STATIC / pane).read_text(encoding="utf-8")
+    assert "useSaver()" in source
+
+
+# ── The folder picker ───────────────────────────────────────────────────────
+
+def test_the_picker_asks_the_node_for_nothing():
+    """
+    The tree is built from paths the client already holds. A fetch here would
+    be a folder-browsing protocol, which this deliberately is not: what it
+    shows is what the group's index contains, and a folder the node never
+    indexed does not exist as far as the group is concerned.
+    """
+    source = FOLDER_TREE.read_text(encoding="utf-8")
+    for forbidden in ("hubFetch", "fetch(", "platform.node", "transport."):
+        assert forbidden not in source, (
+            f"folder-tree.js reaches for {forbidden} — it is meant to be "
+            f"derived from the index the client already has")
+
+
+def test_a_read_only_root_cannot_be_chosen_as_a_destination():
+    """
+    Chat's attachment folder is the one directory that gets written to, and
+    the node refuses a read-only root for it. Letting the picker offer one
+    would move that refusal to the moment somebody sends a file.
+    """
+    source = FOLDER_TREE.read_text(encoding="utf-8")
+    picker = _component(source, "FolderTreePicker")
+    assert "requireWritable" in picker
+    assert "root.writable" in picker, (
+        "writability is not consulted when deciding what is selectable")
+
+    chat = (STATIC / "chat-app-settings.js").read_text(encoding="utf-8")
+    assert "requireWritable=${true}" in chat, (
+        "Chat's directory picker does not require a writable root")
+
+
+def test_the_picker_can_clear_a_selection():
+    """
+    Confirming with nothing chosen is how an app's directories are unset, and
+    an OK disabled on an empty selection would make that impossible without
+    another control.
+    """
+    picker = _component(FOLDER_TREE.read_text(encoding="utf-8"),
+                        "FolderTreePicker")
+    ok = picker[picker.index("folder_tree.confirm") - 400:
+                picker.index("folder_tree.confirm")]
+    assert "disabled" not in ok
+
+
+# ── The generic op ──────────────────────────────────────────────────────────
+
+def test_the_directory_op_is_signed_and_names_its_app():
+    """
+    An operator shown "Media/Films" alone cannot tell which application is
+    about to be pointed at it, and two apps' challenges would be
+    indistinguishable — so the app is in the signed subject, and both sides
+    build it the same way.
+    """
+    transport = TRANSPORT.read_text(encoding="utf-8")
+    body = transport[transport.index("async setAppDirectories("):]
+    body = body[:body.index("\n  async ", 1)]
+    assert "admin_challenge" in body and "_authorizeAdminOp" in body
+    assert "${appKey}:${clean.join(',')}" in body
+
+    node = NODE_SERVER
+    if node.exists():
+        assert 'f"{app}:{\',\'.join(clean)}"' in node.read_text(encoding="utf-8"), (
+            "the node builds a different subject than the client signs")
+
+
+def test_the_page_performs_exactly_one_app_specific_operation():
+    """
+    Pointing an app at folders is what every app has, so the page does it.
+    Anything one app alone needs — a TMDB key, a link-preview switch — the
+    pane does with the transport it is given. An app that only wants
+    directories therefore touches neither file.
+    """
+    panel = _component(GROUP_SETTINGS.read_text(encoding="utf-8"),
+                       "GroupSettingsPanel")
+    calls = set(re.findall(r"transport\.(set\w+)\(", panel))
+    # The page's own settings, which belong to no app: which apps are enabled
+    # at all, and how hard the node works watching its disk.
+    page_level = {"setAppsEnabled", "setScanSettings"}
+    assert calls - page_level == {"setAppDirectories"}, (
+        f"the settings page performs app-specific operations: "
+        f"{sorted(calls - page_level - {'setAppDirectories'})}")
+
+
+# ── The apps read a list ────────────────────────────────────────────────────
+
+@pytest.mark.parametrize("app,prop", [
+    ("video-app.js", "videoDirectories"),
+    ("music-app.js", "musicDirectories"),
+    ("photos-app.js", "photoDirectories"),
+])
+def test_each_app_takes_a_list_of_directories(app, prop):
+    """
+    Videos and Music took a single folder, so a library spread over two drives
+    could not be described at all — the operator's only recourse was to point
+    the app at a parent containing both, which pulls in everything else too.
+    """
+    source = (STATIC / app).read_text(encoding="utf-8")
+    assert prop in source
+    for singular in ("videoRoot", "audioRoot"):
+        assert singular not in source, (
+            f"{app} still reads {singular} — one shape per idea")
+
+
+def test_an_older_node_still_fills_the_lists():
+    """
+    A node speaking MNP 1.0 sends `video_root`, not `video_directories`.
+    Reading the missing plural as "nothing configured" would empty a working
+    Videos tab on every group hosted by a node that has not been upgraded.
+    """
+    page = GROUP_PAGE.read_text(encoding="utf-8")
+    block = page[page.index("setAppDirectories({"):]
+    block = block[:block.index("setChatDirectory")]
+    assert "ack.video_root" in block and "ack.audio_root" in block
+    assert "ack.photo_roots" in block
+
+
+def test_the_search_cache_reads_both_shapes():
+    """
+    The cross-group index cache lives in IndexedDB and outlives a deploy, so a
+    reader opening Search after this ships still has entries written by the
+    previous version. Reading only the new shape empties their results with
+    nothing to distinguish it from "nothing matched".
+    """
+    source = (STATIC / "search-page.js").read_text(encoding="utf-8")
+    fn = _component(source, "cachedDirs")
+    assert "legacyKey" in fn
+    assert "videoRoot" in source and "audioRoot" in source and "photoRoots" in source
diff --git a/packages/meshbay-hub/tests/test_hook_ordering.py b/packages/meshbay-hub/tests/test_hook_ordering.py
index f292b3e..5719be5 100644
--- a/packages/meshbay-hub/tests/test_hook_ordering.py
+++ b/packages/meshbay-hub/tests/test_hook_ordering.py
@@ -37,6 +37,12 @@ STATIC_FILES = [
     "video-player.js", "video-app.js", "music-app.js", "music-player.js",
     "photos-app.js",
     "group-settings.js",
+    # The per-app settings architecture (docs/refactor-groups.md §3). Reached
+    # through the apps.js registry rather than imported by name, so a file
+    # left out of this list is one nothing checks — the failure is silent.
+    "settings-ui.js", "folder-tree.js",
+    "chat-app-settings.js", "video-app-settings.js",
+    "music-app-settings.js", "photos-app-settings.js",
     "auth-page.js", "explore-page.js", "create-group-page.js",
 ]
 
diff --git a/packages/meshbay-hub/tests/test_search_media_merge.py b/packages/meshbay-hub/tests/test_search_media_merge.py
index 0312c57..65f85aa 100644
--- a/packages/meshbay-hub/tests/test_search_media_merge.py
+++ b/packages/meshbay-hub/tests/test_search_media_merge.py
@@ -71,12 +71,12 @@ def pipeline():
     return "\n".join([
         "const t = (k) => k;",
         EXPORT.sub("", MERGE.read_text()),
-        _block(VIDEO_APP, "function underVideoRoot(entry, videoRoot) {"),
+        _block(VIDEO_APP, "function underVideoRoot(entry, directories) {"),
         _block(VIDEO_APP, "function buildSeasons(episodes) {"),
-        _block(VIDEO_APP, "function groupVideoEntries(entries, videoRoot) {"),
+        _block(VIDEO_APP, "function groupVideoEntries(entries, videoDirectories) {"),
         _block(MUSIC_APP, "function foldKey(s) {"),
-        _block(MUSIC_APP, "function underAudioRoot(entry, audioRoot) {"),
-        _block(MUSIC_APP, "function groupMusicEntries(entries, audioRoot) {"),
+        _block(MUSIC_APP, "function underAudioRoot(entry, directories) {"),
+        _block(MUSIC_APP, "function groupMusicEntries(entries, musicDirectories) {"),
         _block(PHOTOS_APP, "function underAnyPhotoRoot(entry, photoRoots) {"),
         _block(PHOTOS_APP, "function groupPhotoAlbums(entries, photoRoots) {"),
         _const("SEARCH_VIDEO_ROOT"),
@@ -107,7 +107,7 @@ def _grid(tmp_path, pipeline, entries, salt="reader", local=()):
         salt: {json.dumps(salt)},
         isLocal: (g) => local.has(g),
       }});
-      const {{ movies, shows }} = groupVideoEntries(merged, SEARCH_VIDEO_ROOT);
+      const {{ movies, shows }} = groupVideoEntries(merged, [SEARCH_VIDEO_ROOT]);
       console.log(JSON.stringify({{
         movies: movies.map((e) => ({{
           id: e.id, title: e.display_title || e.name, groupId: e.groupId,
@@ -136,7 +136,7 @@ def _albums(tmp_path, pipeline, entries, salt="reader", local=()):
         salt: {json.dumps(salt)},
         isLocal: (g) => local.has(g),
       }});
-      const {{ albums, tracks }} = groupMusicEntries(merged, SEARCH_AUDIO_ROOT);
+      const {{ albums, tracks }} = groupMusicEntries(merged, [SEARCH_AUDIO_ROOT]);
       console.log(JSON.stringify({{
         albums: albums.map((a) => ({{
           artist: a.artist, album: a.album,
diff --git a/packages/meshbay-hub/tests/test_spa_syntax.py b/packages/meshbay-hub/tests/test_spa_syntax.py
new file mode 100644
index 0000000..352f84b
--- /dev/null
+++ b/packages/meshbay-hub/tests/test_spa_syntax.py
@@ -0,0 +1,85 @@
+"""
+Every SPA module parses.
+
+This is the cheapest possible test and the suite did not have it, which is how
+a `${/* ... */''}` — htm template syntax, pasted into a plain object literal —
+reached a committed file. Nothing else here would catch it: the source-reading
+guards (`test_hook_ordering`, `test_transport_contracts`, `test_spa_ordering`)
+match patterns in text that parses or does not, and the browser harnesses only
+load the few modules they need.
+
+**`node --check foo.js` is not the check.** It reports success on exactly the
+file above: given a `.js` extension it makes its own decision about how to
+parse, and a module-syntax error inside one can come back clean. Copying to
+`.mjs` first is what forces the module parser, and it is the difference
+between a green run and a real one — the same shape as the "a test that models
+a fix agrees with it by construction" note in CLAUDE.md, one level lower.
+
+It says nothing about names, imports resolving, or hooks being in order. Those
+have their own tests. This one only says the file is JavaScript.
+"""
+
+import shutil
+import subprocess
+from pathlib import Path
+
+import pytest
+
+STATIC = Path(__file__).resolve().parents[1] / "src" / "meshbay_hub" / "static"
+
+pytestmark = pytest.mark.skipif(
+    shutil.which("node") is None or not STATIC.exists(),
+    reason="node or the SPA sources are not available")
+
+
+def _modules() -> list[Path]:
+    # vendor/ is third-party and shipped as-is; sw.js is a service worker, a
+    # classic script by definition, and transport.js is loaded with a plain
+    # <script> tag for the same historical reason (see its own header).
+    files = sorted(STATIC.glob("*.js")) + sorted((STATIC / "locales").glob("*.js"))
+    return [f for f in files if f.name not in ("sw.js",)]
+
+
+def test_every_module_parses(tmp_path):
+    broken: list[str] = []
+    for path in _modules():
+        # The .mjs copy is the whole point — see this module's docstring.
+        copy = tmp_path / (path.stem + ".mjs")
+        copy.write_text(path.read_text(encoding="utf-8"), encoding="utf-8")
+        proc = subprocess.run(["node", "--check", str(copy)],
+                              capture_output=True, text=True)
+        if proc.returncode != 0:
+            first = (proc.stderr or "").strip().splitlines()
+            detail = next((ln for ln in first if "Error" in ln), first[:1] and first[0] or "")
+            broken.append(f"{path.name}: {detail}")
+
+    assert not broken, "SPA modules that do not parse:\n" + "\n".join(broken)
+
+
+def test_the_check_would_notice_a_broken_file(tmp_path):
+    """
+    The test above passing means nothing unless it can fail, and the way it
+    fails is the interesting part: this same content in a file called `.js`
+    is reported as fine.
+    """
+    bad = "export const a = {\n  ${/* not object syntax */''}\n  b: 1,\n};\n"
+
+    as_js = tmp_path / "sample.js"
+    as_js.write_text(bad, encoding="utf-8")
+    lenient = subprocess.run(["node", "--check", str(as_js)],
+                             capture_output=True, text=True)
+
+    as_mjs = tmp_path / "sample.mjs"
+    as_mjs.write_text(bad, encoding="utf-8")
+    strict = subprocess.run(["node", "--check", str(as_mjs)],
+                            capture_output=True, text=True)
+
+    assert strict.returncode != 0, (
+        "the .mjs check no longer reports a module syntax error — this whole "
+        "test is then measuring nothing")
+    if lenient.returncode == 0:
+        # Recorded rather than asserted: this is a Node behaviour, and it
+        # improving would be good news, not a failure. The .mjs copy stays
+        # either way, because relying on the loose path is what let this
+        # through once already.
+        pass
diff --git a/packages/meshbay-hub/tests/test_transport_contracts.py b/packages/meshbay-hub/tests/test_transport_contracts.py
index fee80bb..3c6d022 100644
--- a/packages/meshbay-hub/tests/test_transport_contracts.py
+++ b/packages/meshbay-hub/tests/test_transport_contracts.py
@@ -33,6 +33,14 @@ SPLIT_FILES = [APP, GROUP_PAGE, CHAT_APP, STATIC / "files-app.js",
               STATIC / "music-app.js", STATIC / "music-player.js",
               STATIC / "photos-app.js",
               STATIC / "group-settings.js",
+              # Same reason as test_hook_ordering's STATIC_FILES: these are
+              # reached through the registry, so leaving one out here means it
+              # is simply never checked.
+              STATIC / "settings-ui.js", STATIC / "folder-tree.js",
+              STATIC / "chat-app-settings.js",
+              STATIC / "video-app-settings.js",
+              STATIC / "music-app-settings.js",
+              STATIC / "photos-app-settings.js",
               STATIC / "auth-page.js", STATIC / "explore-page.js",
               CREATE_GROUP]