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>

tests/test_plugin_src_route.py

@@ -138,3 +138,119 @@ def test_unready_plugin_src_is_404(client):
     c, _ = client
     plugins.LOADED_PLUGINS[0]["status"] = "installing"
     assert c.get(f"/api/plugins/{PLUGIN_ID}/src/main.js").status_code == 404
+
+
+# ── #879: the /g/<token>/ generation prefix ────────────────────────────────────
+#
+# A plugin ROLLBACK must actually re-evaluate a module plugin. 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. Busting the ENTRY url alone does not help — screen.js is a one-line
+# `import './src/main.js'`, and a relative specifier resolves against the base URL
+# with the QUERY DROPPED, so a ?v= token never reaches the graph.
+#
+# Hence a token in the PATH: every relative import inherits it, at every depth,
+# with no import-specifier rewriting. These routes must serve the SAME bytes and
+# keep the SAME containment.
+
+def test_generation_prefix_serves_identical_screen_js(client):
+    c, _ = client
+    plain = c.get(f"/api/plugins/{PLUGIN_ID}/screen.js")
+    gen = c.get(f"/api/plugins/{PLUGIN_ID}/g/7/screen.js")
+    assert gen.status_code == 200
+    assert gen.content == plain.content
+    assert "import './src/main.js'" in gen.text
+
+
+def test_generation_prefix_serves_the_whole_module_graph(client):
+    """The point of the path token: a relative import from a /g/7/ entry resolves
+    to a /g/7/ URL, so the graph is fetched fresh — not just the entry."""
+    c, _ = client
+    main = c.get(f"/api/plugins/{PLUGIN_ID}/g/7/src/main.js")
+    assert main.status_code == 200
+    assert main.text == c.get(f"/api/plugins/{PLUGIN_ID}/src/main.js").text
+    # and one level deeper, which is where a query-string token would already have
+    # been lost twice over
+    nested = c.get(f"/api/plugins/{PLUGIN_ID}/g/7/src/util/x.js")
+    assert nested.status_code == 200
+    assert "export const x = 42" in nested.text
+
+
+def test_generation_token_is_opaque(client):
+    """Any token serves the same bytes — it exists only to vary the URL."""
+    c, _ = client
+    a = c.get(f"/api/plugins/{PLUGIN_ID}/g/1/src/main.js")
+    b = c.get(f"/api/plugins/{PLUGIN_ID}/g/999999/src/main.js")
+    assert a.status_code == b.status_code == 200
+    assert a.text == b.text
+
+
+def test_generation_prefix_does_not_widen_containment(client):
+    """The token is never joined into a path, so containment must be EXACTLY what the
+    un-prefixed route already gives. Asserted as parity rather than as a flat 404:
+    `../screen.js` legitimately 200s on BOTH, because the URL normalises to
+    /api/plugins/<id>/screen.js before routing ever happens — it never leaves the
+    plugin dir. Pinning an absolute expectation here would have encoded my guess
+    about the existing route instead of testing the thing that matters, which is
+    that /g/ changes nothing."""
+    c, _ = client
+    for bad in ("../screen.js", "../../etc/passwd", "..%2f..%2fetc%2fpasswd",
+                "..%5c..%5cwindows%5cwin.ini", "/etc/passwd"):
+        plain = c.get(f"/api/plugins/{PLUGIN_ID}/src/{bad}")
+        gen = c.get(f"/api/plugins/{PLUGIN_ID}/g/1/src/{bad}")
+        assert gen.status_code == plain.status_code, f"/g/ diverged on {bad!r}"
+        assert gen.content == plain.content, f"/g/ served different bytes for {bad!r}"
+        assert "root:" not in gen.text and "[extensions]" not in gen.text
+
+    # and the real traversals are genuinely rejected, on both
+    for bad in ("../../etc/passwd", "..%2f..%2fetc%2fpasswd"):
+        assert c.get(f"/api/plugins/{PLUGIN_ID}/g/1/src/{bad}").status_code == 404
+
+
+def test_generation_prefix_404s_for_unknown_plugin(client):
+    c, _ = client
+    assert c.get("/api/plugins/nope/g/1/screen.js").status_code == 404
+    assert c.get("/api/plugins/nope/g/1/src/main.js").status_code == 404
+
+
+def test_generation_prefix_serves_ASSETS_too(client):
+    """Codex [P2] on the first cut of this fix, and it was right.
+
+    The path token shifts the BASE URL, so everything a module resolves relatively moves
+    with it — not just imports. `new URL('../assets/worklet.js', import.meta.url)` from
+    /api/plugins/<id>/g/1/src/main.js resolves to /api/plugins/<id>/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. Hence a path REWRITE, so every plugin route
+    — present and future — works under the prefix."""
+    c, _ = client
+    plain = c.get(f"/api/plugins/{PLUGIN_ID}/assets/worklet.js")
+    gen = c.get(f"/api/plugins/{PLUGIN_ID}/g/1/assets/worklet.js")
+    assert plain.status_code == 200
+    assert gen.status_code == 200, "an asset reached relatively from a reloaded module graph 404'd"
+    assert gen.content == plain.content
+
+
+def test_generation_prefix_covers_every_plugin_route(client):
+    """The rewrite is generic, so this holds for routes nobody thought about — which is
+    the point. Any plugin route added later works under /g/ with no extra wiring."""
+    c, _ = client
+    for route in ("screen.js", "src/main.js", "src/util/x.js", "src/theme.css",
+                  "assets/worklet.js", "settings.html"):
+        plain = c.get(f"/api/plugins/{PLUGIN_ID}/{route}")
+        gen = c.get(f"/api/plugins/{PLUGIN_ID}/g/42/{route}")
+        assert gen.status_code == plain.status_code, f"/g/ diverged on {route}"
+        assert gen.content == plain.content, f"/g/ served different bytes for {route}"
+
+
+def test_generation_prefix_handles_non_ascii_filenames(client):
+    """Codex [P3] on the second cut. A plugin file named e.g. src/工具.js is perfectly
+    valid, and the middleware must not 500 on it — which an eager
+    raw_path.encode("latin-1") did, making the prefixed route LESS capable than the
+    plain one. raw_path is informational; Starlette routes on scope["path"]."""
+    c, tmp = client
+    (tmp / "src" / "工具.js").write_text("export const t = 1;\n")
+    plain = c.get(f"/api/plugins/{PLUGIN_ID}/src/工具.js")
+    gen = c.get(f"/api/plugins/{PLUGIN_ID}/g/3/src/工具.js")
+    assert plain.status_code == 200
+    assert gen.status_code == 200, "non-ASCII module path 500'd or 404'd under /g/"
+    assert gen.content == plain.content