Add PrefabAppConfig for customizable Prefab tool setup#3648
Conversation
PrefabAppConfig auto-wires the Prefab renderer URI and merges renderer CSP with user-provided CSP domains. Fills the gap between app=True (zero config, no customization) and app=AppConfig(...) (full manual control, no auto-wiring). Also auto-registers the renderer resource when any tool points to the Prefab renderer URI, fixing #3623.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 08f4d8610c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| merged = ResourceCSP( | ||
| connect_domains=_merge_domains( | ||
| renderer_csp.get("connect_domains"), | ||
| user_csp.connect_domains, | ||
| ), |
There was a problem hiding this comment.
Preserve custom CSP directives when merging renderer defaults
PrefabAppConfig rebuilds ResourceCSP from a fixed subset of fields, which drops any extra directives users pass (for example future-spec keys like workerDomains). ResourceCSP is configured with extra="allow" and existing tests rely on unknown directives round-tripping, so this introduces a regression specifically for PrefabAppConfig users who add non-core CSP keys.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 95bf66fd32
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| elif isinstance(ui, dict) and ui.get("resourceUri") == PREFAB_RENDERER_URI: | ||
| # PrefabAppConfig or manual config pointing to the Prefab renderer — | ||
| # ensure the renderer resource is registered (CSP already set by caller) | ||
| _ensure_prefab_renderer(provider) |
There was a problem hiding this comment.
Skip prefab auto-registration when renderer URI is versioned
This new branch now auto-calls _ensure_prefab_renderer for any tool whose ui.resourceUri is ui://prefab/renderer.html, but _ensure_prefab_renderer only checks the unversioned key (resource:ui://prefab/renderer.html@). In setups that already register a versioned renderer resource at the same URI, adding such a tool now raises ValueError from version-mixing (LocalProvider._check_version_mixing), so app=AppConfig(resource_uri="ui://prefab/renderer.html") can fail even though the renderer is already present. This regression is triggered specifically when users version their renderer resource.
Useful? React with 👍 / 👎.
Closes #3623.
The gap:
app=Trueauto-wires everything but isn't customizable.app=AppConfig(...)is fully customizable but doesn't auto-wire the renderer or CSP. If you need one extra CSP domain, you fall off a cliff.PrefabAppConfigfills the middle: it auto-sets the renderer URI, merges the renderer's CSP with any additional domains you provide, and the renderer resource is auto-registered.Also fixes the renderer resource not being registered when using
AppConfigwith the Prefab renderer URI directly — any tool pointing toui://prefab/renderer.htmlnow triggers auto-registration.