4 Commits

Author	SHA1	Message	Date
Byron Gamatos	756588678b	fix(plugins): make a module plugin actually re-evaluate on reload (#879) (#897) ... A plugin reload silently did nothing for scriptType:"module" plugins. ES modules are evaluated ONCE PER URL PER DOCUMENT, so re-inserting a <script type="module"> whose src the module map has already seen fires `load` without re-running the body — and the loader then recorded the reload as applied. A no-op that reported success. THE ISSUE UNDERSTATES IT. #879 says "upgrades are fine — a new version yields a new URL". That is true of screen.js and FALSE of the plugin. I drove a real browser through install(1.0.0) -> upgrade(1.1.0) -> rollback(1.0.0), counting evaluations of src/main.js: ONE. Not three, not two. The upgrade re-runs the one-line screen.js shim at its new ?v= URL; the shim does `import './src/main.js'`; a relative specifier resolves against the base URL WITH THE QUERY DROPPED; that is the same URL as before; the module map hands back the already-evaluated v1.0.0 module. The plugin's own code never re-ran. Busting the entry point cannot fix this, whatever token you hang off it. So the token goes in the PATH: /api/plugins/<id>/g/<n>/screen.js. From there './src/main.js' resolves to /api/plugins/<id>/g/<n>/src/main.js — every relative import inherits it, at every depth, for free. No import-specifier rewriting (which could never see `import(expr)` anyway). Same browser drive after the fix: THREE evaluations. Keyed on the plugin ID, not id@version: EVERY re-load of a module plugin needs a fresh path, not just a rollback. First load keeps the stable ?v= URL, so the ETag/304 live-edit caching the R0 rails depend on is untouched. Classic-script plugins are not affected and never take a /g/ path. ━━━ A PATH REWRITE, NOT TWO MIRRORED ROUTES ━━━ Codex caught this, and it was right. The token shifts the BASE URL, so EVERYTHING the module graph resolves relatively moves with it — not only imports. `new URL('../assets/worklet.js', import.meta.url)` from /api/plugins/x/g/1/src/main.js resolves to /api/plugins/x/g/1/assets/worklet.js. Mirroring only screen.js and src/ would have fixed imports and 404'd every asset, worklet and wasm file the graph reaches — and would have broken again the next time someone added a plugin route. So the /g/<token> segment is STRIPPED BEFORE ROUTING. Every plugin route, present and future, works under the prefix with no extra wiring. The token is opaque and never joined into a filesystem path, so containment still rests entirely on the same safe_join. Codex then caught a [P3] in that: eagerly re-encoding raw_path with latin-1 raises UnicodeEncodeError on a valid plugin file like src/工具.js, 500ing a request the plain route serves fine. raw_path is informational and Starlette routes on scope["path"], so the mutation is simply gone — and leaving raw_path as the client sent it is more truthful for logs anyway. TESTS. tests/js/plugin_module_rollback.test.js (5) + 8 in test_plugin_src_route.py: identical bytes under the prefix, the whole graph one and two levels deep, ASSETS (the Codex [P2]), every plugin route, non-ASCII filenames (the [P3]), an opaque token, and containment asserted as PARITY with the un-prefixed route rather than a guessed 404 — `../screen.js` legitimately 200s on both, because the URL normalises before routing. All bite-tested: reverting the fix fails the rollback tests, disabling the rewrite fails the asset tests. Two harnesses re-anchored on `script.src = _pluginScriptUrl(` — the URL literal they keyed on now lives in the helper, further down the file, so their slice ran off the end. node 1045, pytest 2404, ESLint 0, Codex 0. Closes #879 Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-07-12 00:19:59 +02:00
Byron Gamatos	b5dd585d25	refactor(app): carve settings backup + plugin updates out of app.js (R3a) (#885) ... Two leaves, one PR. app.js 9,651 → 9,457. static/js/settings-io.js (155) — exportSettings + importSettings, the Settings backup bundle. Imports nothing. The two-phase rationale comment (server first and atomic; then a best-effort localStorage merge) is the contract and moved with the code. plugin-updates → INTO static/js/plugin-loader.js, not a module of its own. checkPluginUpdates + updatePlugin are plugin MANAGEMENT; they belong with the code that loads plugins. A new file for 50 lines would have been a file for its own sake. All four are inline handlers on the Settings screen and already in app.js's window contract, so app.js re-exposes the imported bindings unchanged. VERIFIED BY DRIVING BOTH FLOWS. A/B against origin/main in two browsers: * checkPluginUpdates() -> hits the API and settles the button back to "Check for Updates" — IDENTICAL * exportSettings() -> POSTs /api/settings/export and writes "Exported feedBack-settings…" to #backup-status — IDENTICAL (fetch intercepted so the assertion is on the real call, not a stub) * all four resolve on window — IDENTICAL * zero console/page errors either side Zero harnesses broke. pytest 2396, node 1038/1038, ESLint 0, tailwind clean, Codex 0. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-07-11 19:42:54 +02:00
Byron Gamatos	a222b45c02	refactor(app): carve the viz layer out of app.js — and delete the loader seam (R3a) (#880) ... static/js/viz.js (770 lines) — the viz picker, renderer selection, Auto-match, the WebGL2 probe, the 3D-promotion nag, the notation hints. Bodies VERBATIM. app.js 11,603 → 10,857. THE SEAM IS GONE. #878's plugin-loader needed configurePluginLoader({ populateVizPicker }) purely because _populateVizPicker lived in app.js and importing app.js would have closed a cycle. viz.js is a LEAF — it imports NOTHING — so plugin-loader now imports _populateVizPicker straight from it. The _host object, the configure function, its loud-default guard, and the wiring line in app.js are all deleted. The second carve simplifies the first. app.js -> { plugin-loader, viz } plugin-loader -> viz viz -> (nothing) NOT A PURE MOVE — one listener block had to be SPLIT. app.js had a single top-level `if (window.feedBack) { … }` registering four handlers, and only two were viz. song:loaded / arrangement:changed / song:ready (the mastery slider) stay in app.js and now call the imported _autoMatchViz / _maybeShowNotationViewHint. The viz:reverted handler MOVES, because it REASSIGNS _cancelPendingAutoLabel and an imported binding is read-only — `_cancelPendingAutoLabel = null` would throw if the listener stayed behind while the state moved. ORDER CHECKED, NOT ASSUMED: viz.js's song:ready listener now registers BEFORE app.js's own (imports evaluate first). Safe — _pendingPromotionNag is only ever set inside _populateVizPicker, which runs at boot/plugin-refresh, never from inside the other song:ready handler, so the two are independent. VERIFIED — the listeners are the risk here, so they were DRIVEN, not just booted. A/B against origin/main in two browsers: * viz picker: 6 options (auto|default|venue|drum_highway_3d|keys_highway_3d| highway_3d), selected highway_3d, Auto label — IDENTICAL. This alone proves plugin-loader's direct import of viz.js works. * emit('viz:reverted') -> picker resets to default, localStorage resets to default, the warning logs — IDENTICAL. The MOVED listener fires. * emit('song:ready') -> mastery slider enables, no throw — IDENTICAL. The SPLIT listener still does both halves. * plugin screens, module injections, 37 capability participants — IDENTICAL. * zero console/page errors on both. pytest 2396, node 1038/1038, ESLint 0, tailwind-fresh clean. no-cycle re-bitten on the 3-module graph (viz -> plugin-loader fails). Codex preflight raised a [P2] claiming viz.js's top-level bus guards would be false because "app.js only creates the event bus later" — FALSE POSITIVE. app.js does not create the bus; capabilities.js does, from its own <script type="module"> at index.html:122, and module scripts execute in document order, so the bus exists long before app.js's import graph evaluates. Instrumented the setter: by viz.js's turn `window.feedBack.on` is already a function, and the viz:reverted listener is provably attached (firing it resets the picker). The ordering is also enforced by test_app_shell_loads_capability_registry_before_app_runtime. Harnesses: 5 tests retargeted to viz.js across legacy_shim_hits, venue_scene_3d, venue_viz (each SPLIT — their non-viz tests still read app.js). Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-07-11 18:41:52 +02:00
Byron Gamatos	38772f604a	refactor(app): carve the plugin loader out of app.js into static/js/ (R3a) (#878) ... The first carve, and deliberately the riskiest: app.js IS the plugin loader (the R0 host rails), so it goes first while the module graph is still one edge deep. static/js/plugin-loader.js (829 lines) — bodies VERBATIM. app.js 12,217 → 11,439. Core's first `static/js/` module, exactly as constitution II anticipates. CLOSURE (measured with acorn, not regex — brace-matching stripped source drifted): the block at app.js:11246-12031 is contiguous and self-contained. It needs only TWO things from the rest of app.js, and exports only TWO: exports: loadPlugins (the window contract), bootstrapPluginsAndUi (boot) inbound: window.showScreen — already the public host contract (constitution II), so it is called through `window`, not re-coupled as an import _populateVizPicker — injected via configurePluginLoader() WHY A SEAM, NOT AN IMPORT. plugin-loader must not import app.js: app.js imports it, so that would close a cycle. I checked whether _populateVizPicker could just move into the module instead (which would delete the seam entirely) — it drags 9 further symbols (_canRun3D, _autoMatchViz, _showPromotionNag, …), i.e. a whole viz cluster. That is its own carve, so the seam stays. THE SEAM'S DEFAULT IS LOUD, ON PURPOSE. A no-op stub is the classic silent failure for this pattern (see the editor's setHostHooks trap, hit twice): drop the wiring call and the loader keeps working while the viz picker quietly stops refreshing — no test, no boot check says a word. The default now console.errors, so the smoke harness catches it. VERIFIED BY BITE TEST: removing configurePluginLoader() from app.js surfaces "[plugin-loader] host seam not configured" at boot. The seam IS exercised on the plugin-startup path, so an unwired hook cannot pass silently. no-cycle is now LIVE on core's own graph for the first time. eslint.config.js gains `static/app.js` + `static/js/**` to the module block — app.js now `import`s, so parsing it as a script would be a syntax error. VERIFIED BY BITE TEST: making plugin-loader import app.js back fails with "Dependency cycle detected". HARNESSES (the R3a note said budget one conversion per carve — it was five): retargeted capability_inspector_nav, plugin_hydration_wipe, plugin_loader_script_type, plugin_style_injection, legacy_shim_hits (SPLIT — one test needs the loader, one still needs app.js) + test_plugin_runtime_idempotence. legacy_shim_hits was missed by a symbol-name grep because it greps for a code STRING; only the failing run found it. test_capability_events' NEGATIVE asserts now span app.js + the loader — carving code out of app.js would otherwise make them vacuous instead of failing. VERIFIED: A/B against origin/main in two browsers — mounted plugin screens, 14 loaded plugin scripts, the 3 module plugins injected as <script type="module">, 37 capability participants, 14 shims, window.loadPlugins: IDENTICAL, zero console/page errors on both. /static/js/plugin-loader.js serves 200; R0 rails intact (src/main.js 200, conditional GET 304, script_type passthrough). pytest 2396, node 1032/1032, ESLint 0, Codex 0. Codex preflight caught a REAL [P1] first pass: static/js/plugin-loader.js was untracked, so a checkout would have served an app.js importing a nonexistent module — a failed static import kills the whole module and every window handler with it. Now tracked. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-07-11 18:18:00 +02:00