mirror of
https://github.com/got-feedBack/feedBack.git
synced 2026-08-19 06:52:38 +00:00
feat(panes): mirrorGlobal, manifest-declared panes, and the plugin docs
Three things a plugin needs before it can actually use panes.
## mirrorGlobal — the camera-director problem
The 3D highways read their free camera from a plain global,
`window.__h3dCamCtl` (highway_3d/FREECAM_BRIDGE.md), once per frame in
_resolveFreeCam(). A camera panel in the main window just writes that
object and the camera moves. A panel in a POP-OUT window cannot:
window.__h3dCamCtl there is a different object in a different realm, and
writing it moves nothing.
So a pane declares one field — `mirrorGlobal: '__h3dCamCtl'` — and
pane-mirror.js (main realm, where the renderers live) copies that pane's
state onto the global whenever it changes. highway_3d, keys_highway_3d
and drum_highway_3d are NOT modified and do not know panes exist.
The rule that makes it work: MUTATE THE OBJECT, NEVER REPLACE IT. A
renderer may be holding the reference, and swapping in a new object would
leave it reading an orphan. Keys the pane doesn't set are left alone
rather than deleted — the global may carry a renderer's own bookkeeping.
Closing the pane deliberately leaves the global as-is: closing the camera
panel should not snap the camera back to a default, which is exactly what
happens today (nobody clears __h3dCamCtl).
## Manifest-declared panes
"panes": [{ "id": "camera_director", "title": "Camera Director",
"script": "panes/camera.js", "mirrorGlobal": "__h3dCamCtl" }]
Declaring a pane beats calling panes.register() from screen.js because it
becomes openable FROM THE RAIL OR THE TRAY WITHOUT THE PLUGIN'S SCREEN
EVER HAVING BEEN VISITED — core registers a stub from the manifest and
fetches the script only when the user opens it. A pane you can only reach
by first navigating to the screen it was meant to replace is not much of a
pane.
The script sets `window.feedBackPane_<id> = { mount, unmount }`, mirroring
the existing window.feedBackViz_<id> convention, and the SAME file is what
a pop-out window loads in its own realm.
`script` is validated as a relpath under the plugin's src/ and served
through the sandboxed /api/plugins/<id>/src/ route — the containment rule
`styles` already has for assets/. Traversal, absolute paths, drive letters,
backslashes and non-.js are rejected; a bad entry is dropped with a warning
rather than failing the whole plugin, because one malformed pane should not
cost the user a working plugin.
Note the projection is written TWICE — _nav_entry() and the /api/plugins
route re-project independently — so panes had to be added to both, plus the
pending branch (a pane can be opened while its plugin is still installing
deps; the script is fetched on open, not at discovery).
## docs/plugin-panes.md
The contract, and the one rule it all hangs on: mount(root, ctx) runs in a
realm that may not have the app in it. Everything comes through ctx, or the
pane works docked and silently dies popped out.
Verified: manifest validation rejects ../.., C:\, non-.js, dupes and
missing fields while passing a good entry; /api/plugins projects panes[] for
all 20 plugins. mirrorGlobal mutates the global IN PLACE — a reference held
the way _resolveFreeCam holds it sees the change, and a renderer's own field
on that object survives — both for a local write and for a write arriving
over the channel from a pop-out realm.
pytest: 2401 passed, 8 failed — all 8 reproduce on a clean main (including
the one in tests/test_plugins.py) and are unrelated.
Signed-off-by: topkoa <topkoa@gmail.com>
This commit is contained in:
@@ -277,6 +277,66 @@ def _normalize_string_list(value) -> list[str]:
|
||||
return [stripped for item in value if isinstance(item, str) and (stripped := item.strip())]
|
||||
|
||||
|
||||
def _normalize_panes(value, plugin_id: str) -> list[dict]:
|
||||
"""Validate a manifest's `panes` list (detachable panes — window.feedBack.panes).
|
||||
|
||||
Each entry: {id, title, icon?, script, defaultHost?, mirrorGlobal?, width?, height?}.
|
||||
|
||||
`script` is a relpath served through the sandboxed /api/plugins/<id>/src/
|
||||
route, so it MUST live under the plugin's src/ tree — the same containment
|
||||
rule `styles` has for assets/. A pane whose script could escape that route
|
||||
would be an arbitrary-file-read dressed up as a manifest key.
|
||||
|
||||
A bad entry is dropped with a warning rather than failing the whole plugin:
|
||||
one malformed pane should not cost the user their working plugin.
|
||||
"""
|
||||
if not isinstance(value, list):
|
||||
return []
|
||||
out: list[dict] = []
|
||||
seen: set[str] = set()
|
||||
for entry in value:
|
||||
if not isinstance(entry, dict):
|
||||
log.warning("[Plugin] %s: `panes` entry is not an object, skipping", plugin_id)
|
||||
continue
|
||||
pane_id = entry.get("id")
|
||||
script = entry.get("script")
|
||||
if not isinstance(pane_id, str) or not pane_id.strip():
|
||||
log.warning("[Plugin] %s: pane is missing `id`, skipping", plugin_id)
|
||||
continue
|
||||
pane_id = pane_id.strip()
|
||||
if not isinstance(script, str) or not script.strip():
|
||||
log.warning("[Plugin] %s: pane %r is missing `script`, skipping", plugin_id, pane_id)
|
||||
continue
|
||||
script = script.strip().lstrip("/")
|
||||
# Reject anything that could climb out of src/: absolute paths, drive
|
||||
# letters, backslashes, and .. segments — the same checks settings.server_files
|
||||
# applies to its relpaths.
|
||||
if (
|
||||
"\\" in script
|
||||
or ".." in script.split("/")
|
||||
or ":" in script
|
||||
or not script.endswith(".js")
|
||||
):
|
||||
log.warning("[Plugin] %s: pane %r has an unsafe `script` %r, skipping", plugin_id, pane_id, script)
|
||||
continue
|
||||
if pane_id in seen:
|
||||
log.warning("[Plugin] %s: duplicate pane id %r, skipping", plugin_id, pane_id)
|
||||
continue
|
||||
seen.add(pane_id)
|
||||
pane = {
|
||||
"id": pane_id,
|
||||
"title": entry.get("title") if isinstance(entry.get("title"), str) else pane_id,
|
||||
"icon": entry.get("icon") if isinstance(entry.get("icon"), str) else None,
|
||||
"script": script,
|
||||
"defaultHost": entry.get("defaultHost") if entry.get("defaultHost") in ("window", "dock") else None,
|
||||
"mirrorGlobal": entry.get("mirrorGlobal") if isinstance(entry.get("mirrorGlobal"), str) else None,
|
||||
"width": entry.get("width") if isinstance(entry.get("width"), int) else None,
|
||||
"height": entry.get("height") if isinstance(entry.get("height"), int) else None,
|
||||
}
|
||||
out.append(pane)
|
||||
return out
|
||||
|
||||
|
||||
def _normalize_manifest_mapping(value) -> dict:
|
||||
return value if isinstance(value, dict) else {}
|
||||
|
||||
@@ -1442,6 +1502,14 @@ def load_plugins(app: FastAPI, context: dict, progress_cb=None, route_setup_fn=N
|
||||
# client can build the asset URL without a second manifest read.
|
||||
"has_styles": bool(manifest.get("styles")),
|
||||
"styles": manifest.get("styles"),
|
||||
# Detachable panes (window.feedBack.panes). Declaring them in the
|
||||
# manifest — rather than calling panes.register() from screen.js —
|
||||
# is what lets a pane be OPENED FROM THE TRAY OR THE RAIL WITHOUT THE
|
||||
# PLUGIN'S SCREEN EVER HAVING BEEN VISITED: the host registers a stub
|
||||
# from this projection and only fetches the pane's script when the
|
||||
# user actually opens it. Validated (id + script required, script
|
||||
# sandboxed under src/) so one bad entry can't take the list down.
|
||||
"panes": _normalize_panes(manifest.get("panes"), plugin_id),
|
||||
"standards": _normalize_string_list(manifest.get("standards")),
|
||||
"capabilities": _validated_capabilities,
|
||||
"capability_validation_warnings": _capability_validation_warnings,
|
||||
@@ -2164,6 +2232,11 @@ def register_plugin_api(app: FastAPI):
|
||||
# _nav_entry() at discovery and carried through graduation.
|
||||
# The `.get()` fallbacks keep stubbed test entries (which build
|
||||
# rows directly without going through _nav_entry) working.
|
||||
# Detachable panes. Key-presence check (not truthiness) like
|
||||
# `capabilities` below: _nav_entry() may legitimately have
|
||||
# validated the list down to empty, and an `or` would wrongly
|
||||
# re-derive the unvalidated manifest value.
|
||||
"panes": p["panes"] if "panes" in p else _normalize_panes((p.get("_manifest") or {}).get("panes"), p.get("id", "plugin")),
|
||||
"standards": p.get("standards") or _normalize_string_list((p.get("_manifest") or {}).get("standards")),
|
||||
"capabilities": p["capabilities"] if "capabilities" in p else _normalize_manifest_mapping((p.get("_manifest") or {}).get("capabilities")),
|
||||
"capability_validation_warnings": p.get("capability_validation_warnings", []),
|
||||
@@ -2211,6 +2284,11 @@ def register_plugin_api(app: FastAPI):
|
||||
"has_tour": e.get("has_tour", False),
|
||||
"has_styles": e.get("has_styles", False),
|
||||
"styles": e.get("styles"),
|
||||
# Pending entries come from _nav_entry() too, so their panes are
|
||||
# already validated. Surfacing them here is what lets a pane be
|
||||
# opened while its plugin is still installing its dependencies —
|
||||
# the pane's script is fetched on open, not now.
|
||||
"panes": e.get("panes", []),
|
||||
# Pending entries are built from _nav_entry() too, so they
|
||||
# carry the same capability-pipelines.v1 metadata — surface it
|
||||
# so the Inspector can show still-installing plugins.
|
||||
|
||||
Reference in New Issue
Block a user