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

1 files changed, 277 insertions, 0 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