From 73a654f2934e6b0cf8a6cfb0666e86f24933304f Mon Sep 17 00:00:00 2001 From: byrongamatos Date: Fri, 10 Jul 2026 14:43:40 +0200 Subject: [PATCH 1/2] feat(server): add appstate.py, the router seam (R3) Routes moving out of server.py need `meta_db` and friends, but must not `import server` -- that goes circular the moment server imports them back. server.py keeps CONSTRUCTING its singletons and now injects them once via `appstate.configure(...)`; routers read them back as module attributes at call time (`import appstate; appstate.meta_db`). The Python analogue of the frontend refactor's `configureX({...})` seams and of the plugin `setup(app, context)` contract: dependencies flow one way, server -> routers -> appstate. Two properties are load-bearing, both pinned by tests/test_appstate.py: 1. `import appstate` constructs nothing and touches no disk. This is why the ~49 test fixtures that `sys.modules.pop("server")` + re-import (to rebuild meta_db under a patched CONFIG_DIR) keep working UNTOUCHED. A singleton owned by appstate would survive that pop and go stale -- verified. 2. Reads must be late-bound. `from appstate import meta_db` freezes the binding and defeats both a later configure() and monkeypatch.setattr -- the same read-only-binding trap as ES imports. configure() raises on an unknown slot instead of silently creating a global nothing reads, and the suite asserts server ACTUALLY calls it. Negative-checked: dropping the configure() call fails exactly the two wiring tests while the other five stay green -- those five are the false-green a seam test must not be. The new suite imports server through an `isolated_server` fixture that patches CONFIG_DIR to tmp_path and closes both DB connections on teardown. An unguarded `import server` constructs MetadataDB + AudioEffectsMappingDB under the real `~/.local/share/feedback` (reproduced: running the file alone created web_library.db + audio_effects.db there). The full suite now leaves the real config dir untouched. Packaging: `COPY appstate.py /app/` plus a .dockerignore allowlist entry. That file opens with a blanket `*` exclusion, so root-level Python must be re-allowed explicitly -- without it the image build fails on the COPY. Verified against the real docker daemon (build context reaches /app/appstate.py). docker-compose.yml gains the dev bind-mount; docker-compose.nas.yml runs the baked image, so the COPY covers it. `routers/` will need the same two entries when it lands. Verified: pyflakes clean; pytest 2348 passed (2341 + 7 new); eslint 0 errors; boot smoke serves /api/version, /api/library, /api/audio-effects/mappings, and all three migrated plugins' src/ graphs, with `appstate.meta_db is server.meta_db` asserted against the live import. Co-Authored-By: Claude Opus 4.8 (1M context) --- .dockerignore | 4 ++ CHANGELOG.md | 23 ++++++++ Dockerfile | 3 + appstate.py | 72 ++++++++++++++++++++++++ docker-compose.yml | 1 + docs/size-exemptions.md | 4 +- server.py | 12 ++++ tests/test_appstate.py | 122 ++++++++++++++++++++++++++++++++++++++++ 8 files changed, 239 insertions(+), 2 deletions(-) create mode 100644 appstate.py create mode 100644 tests/test_appstate.py diff --git a/.dockerignore b/.dockerignore index 9ee9d00f..9ae2fafc 100644 --- a/.dockerignore +++ b/.dockerignore @@ -5,6 +5,10 @@ !dockerfile !requirements.txt !server.py +# The router seam server.py injects its singletons into (R3). Root-level Python +# that ships in the image must be re-allowed explicitly — this file starts with +# a blanket `*` exclusion. `routers/` needs the same treatment when it lands. +!appstate.py !main.py !VERSION !tailwind.config.js diff --git a/CHANGELOG.md b/CHANGELOG.md index 57b05162..5fcf5ccc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,29 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added +- **`appstate.py` — the router seam (R3).** Route modules moving out of `server.py` + need `meta_db` and friends but must not `import server`, or the import graph goes + circular the moment `server` imports them back. So `server.py` keeps *constructing* + its singletons and now **injects** them once — `appstate.configure(meta_db=…, + audio_effect_mappings=…)` — and a router reads them back as module attributes at call + time (`import appstate; appstate.meta_db.…`). This is the Python analogue of the + frontend refactor's injected `configureX({…})` seams and of the plugin + `setup(app, context)` contract: dependencies flow one way, `server → routers → + appstate`. Two properties are load-bearing and pinned by `tests/test_appstate.py`: + (1) `import appstate` constructs nothing and touches no disk, so the ~49 test fixtures + that `sys.modules.pop("server")` + re-import (to rebuild `meta_db` under a patched + `CONFIG_DIR`) keep working untouched — a singleton *owned* by `appstate` would survive + that pop and go stale; (2) reads must be late-bound (`appstate.meta_db`, never + `from appstate import meta_db`), since a `from` import freezes the binding and defeats + both a later `configure()` and `monkeypatch.setattr` — the same read-only-binding trap + as ES `import`. `configure()` rejects an unknown slot rather than silently creating a + global nothing reads, and the suite asserts `server` actually calls it (a seam whose + wiring can no-op undetected is worse than no seam). Ships in the image via + `Dockerfile` `COPY appstate.py /app/` plus a `.dockerignore` allowlist entry — that + file opens with a blanket `*` exclusion, so root-level Python must be re-allowed + explicitly or the build fails. + ### Changed - **`AudioEffectsMappingDB` moved out of `server.py` into `lib/audio_effects_db.py` (R3, move-only).** The core-owned song/tone → provider routing index follows diff --git a/Dockerfile b/Dockerfile index ebbd38f5..0c08d0ed 100644 --- a/Dockerfile +++ b/Dockerfile @@ -206,6 +206,9 @@ COPY --from=tailwind-builder /build/static/tailwind.min.css /app/static/tailwind # when a plugin is installed at runtime (see update_manager on-install hook). COPY tailwind.config.js /app/tailwind.config.js COPY server.py /app/ +# The router seam server.py injects its singletons into (R3). Root-level, like +# server.py, so `import appstate` resolves off PYTHONPATH=/app. +COPY appstate.py /app/ COPY main.py /app/ COPY VERSION /app/ # Built-in diagnostic sloppaks seeded into DLC_DIR/diagnostics-builtin/ at scan diff --git a/appstate.py b/appstate.py new file mode 100644 index 00000000..6c6c69ed --- /dev/null +++ b/appstate.py @@ -0,0 +1,72 @@ +"""Shared application state — the seam that lets route modules reach core +singletons without importing ``server``. + +``server.py`` is the host: it owns the FastAPI ``app``, constructs the DB +singletons, and runs the lifecycle. As routes move out into ``routers/`` (R3), +those modules need ``meta_db`` and friends — but they must not ``import +server``, or the import graph goes circular the moment ``server`` imports them +back. + +So ``server`` **injects** its singletons here once, at the point it builds them:: + + # server.py + meta_db = MetadataDB(CONFIG_DIR) + appstate.configure(meta_db=meta_db, ...) + +and a router reads them back as **module attributes, at call time**:: + + # routers/artists.py + import appstate + + @router.get("/api/artist/{name}/page") + def artist_page(name): + return appstate.meta_db.artist_page(name) + +This is the Python analogue of the injected `configureX({...})` seams the +frontend refactor uses (stems' ``configureStreaming``, studio's +``configureAudioGraph``, the editor's ``src/host.js``), and of the plugin +``setup(app, context)`` contract in Principle III: dependencies flow one way, +``server -> routers -> appstate``, and nothing imports back up. + +Two properties this shape buys, both load-bearing: + +* **``import appstate`` performs no IO and constructs nothing.** ``server`` + still owns construction, so the ~49 test fixtures that do + ``sys.modules.pop("server")`` + re-import (to rebuild ``meta_db`` under a + patched ``CONFIG_DIR``) keep working untouched — a singleton *owned* here + would survive that pop and go stale. +* **Reads are late-bound.** Routers must use ``appstate.meta_db``, never + ``from appstate import meta_db`` — a ``from`` import freezes the binding at + its current value, so a later ``configure()`` (or a + ``monkeypatch.setattr(appstate, "meta_db", fake)``) would not reach the + router. This is the same read-only-binding trap as ES ``import``. + +Defaults are ``None`` on purpose: they are inert but *type-honest*, so a router +that runs before ``configure()`` fails loudly on ``NoneType`` instead of +quietly operating on a stand-in. + +Slots are added here only when a router actually needs one — this is a seam, +not a grab-bag for everything in ``server.py``. +""" + +# The singletons routers may read. Every name here must also be a `_SLOTS` key. +meta_db = None +audio_effect_mappings = None + +# Declared up front so `configure()` can reject a typo'd or stale keyword +# instead of silently creating a new global that nothing ever reads. A seam +# whose wiring can no-op undetected is worse than no seam. +_SLOTS = frozenset({"meta_db", "audio_effect_mappings"}) + + +def configure(**kwargs) -> None: + """Publish `server`'s singletons into this module. Called once per + `server` import (and again on re-import), so it must be idempotent.""" + unknown = set(kwargs) - _SLOTS + if unknown: + raise TypeError( + f"appstate.configure() got unknown slot(s): {sorted(unknown)}. " + f"Known slots: {sorted(_SLOTS)}. Add the name to _SLOTS if a router " + f"genuinely needs it." + ) + globals().update(kwargs) diff --git a/docker-compose.yml b/docker-compose.yml index 19d931e0..8c8ad8e4 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -11,6 +11,7 @@ services: # Mount source for live reload during development - ./static:/app/static - ./server.py:/app/server.py + - ./appstate.py:/app/appstate.py - ./VERSION:/app/VERSION - ./ug_browser.py:/app/ug_browser.py - ./lib:/app/lib diff --git a/docs/size-exemptions.md b/docs/size-exemptions.md index f0682d53..777d8451 100644 --- a/docs/size-exemptions.md +++ b/docs/size-exemptions.md @@ -55,8 +55,8 @@ without a *signed* exemption" is unenforceable. ## Planned, NOT exempt (owned by split plans — listed so nothing falls between states) core `static/app.js` (11,852) · `static/highway.js` (4,168, whole file) · `server.py` -(9,433 — was 14,037; ratcheted by the R3 `MetadataDB` + `AudioEffectsMappingDB` -extractions) · +(9,445 — was 14,037; ratcheted by the R3 `MetadataDB` + `AudioEffectsMappingDB` +extractions, plus 12 lines for the `appstate.py` seam) · `lib/metadata_db.py` (4,373 — new in R3; the `MetadataDB` class alone is 4,018 lines and is a monolith in its own right, to be split per-table once the router train lands) · `static/v3/songs.js` (4,134) · `static/capabilities/audio-session.js` diff --git a/server.py b/server.py index 2c5d8b54..5b6bee28 100644 --- a/server.py +++ b/server.py @@ -66,6 +66,9 @@ # The audio-effect routing index. Same shape as metadata_db: the class lives in # its own module, the `audio_effect_mappings` singleton below stays here. from audio_effects_db import AudioEffectsMappingDB +# The router seam. Imported as a module (never `from appstate import ...`) so +# `appstate.configure(...)` below publishes into the same namespace routers read. +import appstate import sloppak as sloppak_mod import drums as drums_mod import notation as notation_mod @@ -353,6 +356,15 @@ def _env_flag(name: str) -> bool: meta_db = MetadataDB(CONFIG_DIR) audio_effect_mappings = AudioEffectsMappingDB(CONFIG_DIR) +# Publish the singletons to the router seam. server.py stays their owner — a +# `sys.modules.pop("server")` + re-import must keep rebuilding them under a +# patched CONFIG_DIR — and `routers/` read them back as `appstate.` at +# call time. See appstate.py for why the reads must be late-bound. +appstate.configure( + meta_db=meta_db, + audio_effect_mappings=audio_effect_mappings, +) + class LocalLibraryProvider: id = "local" diff --git a/tests/test_appstate.py b/tests/test_appstate.py new file mode 100644 index 00000000..d9fb5a54 --- /dev/null +++ b/tests/test_appstate.py @@ -0,0 +1,122 @@ +"""The router seam (`appstate.py`). + +The load-bearing assertion here is `test_server_wires_the_seam`: that `server` +actually calls `appstate.configure(...)`. Every other test in this file would +pass just fine against a seam nothing ever wires up — the same class of silent +no-op that bit the frontend refactor twice when a scripted `setHostHooks` edit +stopped matching its anchor. Unit tests cannot see wiring unless you make them +look at it. +""" + +import importlib +import sys + +import pytest + +import appstate + + +@pytest.fixture() +def isolated_server(tmp_path, monkeypatch): + """A freshly imported `server` bound to a throwaway CONFIG_DIR. + + Importing `server` constructs `MetadataDB` + `AudioEffectsMappingDB` at + module level, so it MUST be re-imported under a patched CONFIG_DIR — an + unguarded `import server` would create/mutate the developer's real + `~/.local/share/feedback` databases. Same idiom as the other ~49 + server-importing suites. + """ + monkeypatch.setenv("CONFIG_DIR", str(tmp_path)) + sys.modules.pop("server", None) + mod = importlib.import_module("server") + yield mod + conn = getattr(getattr(mod, "meta_db", None), "conn", None) + if conn is not None: + getattr(mod, "_join_background_db_threads", lambda: None)() + conn.close() + ae_conn = getattr(getattr(mod, "audio_effect_mappings", None), "conn", None) + if ae_conn is not None: + ae_conn.close() + # Leave no half-torn-down `server` behind: the next fixture re-imports it. + sys.modules.pop("server", None) + + +def test_import_is_side_effect_free(): + """`import appstate` must construct nothing and touch no disk. + + This is why the ~49 fixtures that `sys.modules.pop("server")` and re-import + (to rebuild `meta_db` under a patched CONFIG_DIR) keep working untouched: + server owns construction, appstate only mirrors it. A singleton *owned* + here would survive that pop and go stale. + """ + sys.modules.pop("appstate", None) + fresh = importlib.import_module("appstate") + try: + assert fresh.meta_db is None + assert fresh.audio_effect_mappings is None + finally: + sys.modules["appstate"] = appstate + + +def test_configure_publishes_known_slots(): + sentinel = object() + original = appstate.meta_db + try: + appstate.configure(meta_db=sentinel) + assert appstate.meta_db is sentinel + finally: + appstate.configure(meta_db=original) + + +def test_configure_is_idempotent(): + """server re-imports call configure() again; the last write must win.""" + original = appstate.meta_db + try: + appstate.configure(meta_db="first") + appstate.configure(meta_db="second") + assert appstate.meta_db == "second" + finally: + appstate.configure(meta_db=original) + + +def test_configure_rejects_an_unknown_slot(): + """A typo'd or stale keyword must raise, not silently create a global that + nothing reads. A seam whose wiring can no-op undetected is worse than none.""" + with pytest.raises(TypeError, match="unknown slot"): + appstate.configure(met_db="typo") + assert not hasattr(appstate, "met_db") + + +def test_late_bound_read_sees_a_later_configure(): + """Routers must read `appstate.meta_db`, never `from appstate import meta_db`. + This pins the property that makes that rule work.""" + def router_style_read(): + return appstate.meta_db # module attribute, resolved at call time + + original = appstate.meta_db + try: + appstate.configure(meta_db="before") + assert router_style_read() == "before" + appstate.configure(meta_db="after") + assert router_style_read() == "after" + finally: + appstate.configure(meta_db=original) + + +def test_server_wires_the_seam(isolated_server): + """The one that catches a dropped `appstate.configure(...)` call. + + Identity, not truthiness, so a stray re-assignment or a half-applied edit + fails here rather than in some router months later. + """ + assert appstate.meta_db is isolated_server.meta_db + assert appstate.audio_effect_mappings is isolated_server.audio_effect_mappings + assert appstate.meta_db is not None + + +def test_reimporting_server_republishes_the_fresh_singletons(isolated_server, tmp_path): + """The 49-fixture contract: re-importing server under a new CONFIG_DIR must + leave the seam pointing at the NEW meta_db, without those fixtures having to + know appstate exists.""" + assert appstate.meta_db is isolated_server.meta_db + assert str(tmp_path) in isolated_server.meta_db.db_path From f21b27196fc105d8b1bd9d87b52f721d206ff661 Mon Sep 17 00:00:00 2001 From: byrongamatos Date: Fri, 10 Jul 2026 14:59:41 +0200 Subject: [PATCH 2/2] =?UTF-8?q?test(appstate):=20address=20CodeRabbit=20?= =?UTF-8?q?=E2=80=94=20restore=20slots=20on=20teardown,=20really=20re-impo?= =?UTF-8?q?rt?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two real findings on #833, both fixed: (1) `isolated_server` closed server's DB connections but left `appstate.meta_db` and `appstate.audio_effect_mappings` published and pointing at the closed handles -- a live-looking, dead singleton for any later test. Teardown now snapshots and restores both slots. (2) `test_reimporting_server_republishes_the_fresh_singletons` never performed a second import: it only re-asserted what `test_server_wires_the_seam` already covers, so it could not detect the very staleness it names. (I introduced that regression while fixing Codex's CONFIG_DIR isolation finding.) It now pops `server`, re-imports under a SECOND CONFIG_DIR, and asserts the seam republishes -- `second_server.meta_db is not first_db` and `appstate.meta_db is second_server.meta_db`. Negative-checked both directions: simulating an appstate-OWNED singleton (configure() only-first-wins) now fails the re-import test, and dropping server's configure() call still fails exactly the two wiring tests. NB CodeRabbit's committable suggestion inserted the snapshot above the fixture docstring, which would have demoted it from __doc__; written by hand instead. pytest 2348 passed; the full suite leaves the real ~/.local/share/feedback untouched. Co-Authored-By: Claude Opus 4.8 (1M context) --- tests/test_appstate.py | 57 ++++++++++++++++++++++++++++++++---------- 1 file changed, 44 insertions(+), 13 deletions(-) diff --git a/tests/test_appstate.py b/tests/test_appstate.py index d9fb5a54..638ff42f 100644 --- a/tests/test_appstate.py +++ b/tests/test_appstate.py @@ -16,6 +16,16 @@ import appstate +def _close_server_dbs(mod): + conn = getattr(getattr(mod, "meta_db", None), "conn", None) + if conn is not None: + getattr(mod, "_join_background_db_threads", lambda: None)() + conn.close() + ae_conn = getattr(getattr(mod, "audio_effect_mappings", None), "conn", None) + if ae_conn is not None: + ae_conn.close() + + @pytest.fixture() def isolated_server(tmp_path, monkeypatch): """A freshly imported `server` bound to a throwaway CONFIG_DIR. @@ -25,20 +35,20 @@ def isolated_server(tmp_path, monkeypatch): unguarded `import server` would create/mutate the developer's real `~/.local/share/feedback` databases. Same idiom as the other ~49 server-importing suites. + + Teardown restores the appstate slots as well as closing the connections: + leaving `appstate.meta_db` published but pointing at a closed sqlite handle + would hand a later test (or router) a live-looking, dead singleton. """ + previous = (appstate.meta_db, appstate.audio_effect_mappings) monkeypatch.setenv("CONFIG_DIR", str(tmp_path)) sys.modules.pop("server", None) mod = importlib.import_module("server") yield mod - conn = getattr(getattr(mod, "meta_db", None), "conn", None) - if conn is not None: - getattr(mod, "_join_background_db_threads", lambda: None)() - conn.close() - ae_conn = getattr(getattr(mod, "audio_effect_mappings", None), "conn", None) - if ae_conn is not None: - ae_conn.close() + _close_server_dbs(mod) # Leave no half-torn-down `server` behind: the next fixture re-imports it. sys.modules.pop("server", None) + appstate.configure(meta_db=previous[0], audio_effect_mappings=previous[1]) def test_import_is_side_effect_free(): @@ -114,9 +124,30 @@ def test_server_wires_the_seam(isolated_server): assert appstate.meta_db is not None -def test_reimporting_server_republishes_the_fresh_singletons(isolated_server, tmp_path): - """The 49-fixture contract: re-importing server under a new CONFIG_DIR must - leave the seam pointing at the NEW meta_db, without those fixtures having to - know appstate exists.""" - assert appstate.meta_db is isolated_server.meta_db - assert str(tmp_path) in isolated_server.meta_db.db_path +def test_reimporting_server_republishes_the_fresh_singletons( + isolated_server, tmp_path, monkeypatch +): + """The 49-fixture contract, exercised end to end. + + Those fixtures `sys.modules.pop("server")` + re-import to rebuild `meta_db` + under a new CONFIG_DIR, and know nothing about appstate. So the seam must + re-publish on that second import. This is the test that would fail if + `appstate` ever *owned* the singletons: a module-level `meta_db` there + survives the pop and the assertions below would still see the FIRST DB. + """ + first_db = isolated_server.meta_db + assert appstate.meta_db is first_db + assert str(tmp_path) in first_db.db_path + + second_config = tmp_path / "second" + monkeypatch.setenv("CONFIG_DIR", str(second_config)) + sys.modules.pop("server", None) + second_server = importlib.import_module("server") + try: + assert second_server.meta_db is not first_db # genuinely rebuilt + assert str(second_config) in second_server.meta_db.db_path + assert appstate.meta_db is second_server.meta_db # ...and re-published + assert appstate.audio_effect_mappings is second_server.audio_effect_mappings + finally: + _close_server_dbs(second_server) + sys.modules.pop("server", None)