feat(assets): validate studio panel.jsonc at generate time (Asset Plugins P3, #577) - #588
Conversation
…gins P3, #577) Validate every `studio/*.panel.jsonc` a plugin (or a pack it bundles) ships, at `labelle generate` time, alongside the Phase-1/2 asset + pack validation — so the studio never renders an invalid panel from a generated project. A malformed panel is a build error with a file (and, for parse errors, line) location. The reference is the studio POC's zod schema (labelle-studio src/services/pluginPanels.ts, #73); src/panel_validate.zig is a faithful Zig port: strict top-level keys (id/title/icon/fields/actions), id identifier + non-empty title, per-type field branches (number/slider/select/text/toggle) with strict keys, and the semantic pass (duplicate names, min>max, default out of range, select default not in options). All problems for one file are collected and rendered as `<file>: <where>: <problem>` (the studio's rules sidecar shape). - src/panel_validate.zig (new): parser (JSONC strip + std.json) + schema + semantic validation; a bounded `studio/`-gated directory walk that discovers panels at a plugin root and inside its nested packs; validatePluginPanels drives it over cfg.plugins. - src/root.zig: call validatePluginPanels in generate, before any target is written; test aggregation + pub re-export. - Tests: 18 schema/semantic accept+reject cases plus 2 filesystem-discovery tests (studio-gating + no-panel no-op). Additive: a project with no panels is unaffected; an unresolvable plugin dir is skipped (other passes report a genuinely missing plugin). Deferred (pairs with engine#729): a `"target":"preview"` command naming a plugin-declared handler can't be cross-checked here yet — the engine declares handlers by runtime subscription to engine__editor_plugin_command, so no static handler-name list exists at generate time. The schema/semantic gate is the bulk of the acceptance. `zig build test` passes (the pre-existing flow_catalog sidecar failure is unrelated and present on origin/main). Closes #577 Claude-Session: https://claude.ai/code/session_017pW3ifKf9wgxNg4viy6okw
|
Warning Review limit reached
Next review available in: 23 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ea99158bbd
ℹ️ 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".
| for (cfg.plugins) |plugin| { | ||
| const plugin_dir = cache.resolvePlugin(allocator, plugin, game_dir) catch continue; | ||
| defer allocator.free(plugin_dir); | ||
| try walk(allocator, plugin_dir, plugin_dir, 0, &errors); |
There was a problem hiding this comment.
This recursively walks every directory below the plugin root, so any studio/*.panel.jsonc under an unused folder such as packs/experimental/studio/ or examples/studio/ is validated. Nested packs are only part of generation when they are declared in plugin.labelle's .packs list, so a plugin that keeps an undeclared/example pack with a draft or broken panel will now fail labelle generate even though that panel is not shipped or rendered. Validate the plugin-root studio/ directory and the same declared nested pack dirs that the pack discovery path registers, rather than every descendant studio directory.
Useful? React with 👍 / 👎.
The Phase-3 panel validator walked EVERY directory below a plugin root, so a `studio/*.panel.jsonc` in an UNDECLARED/example pack (e.g. `packs/experimental/studio/`, an `examples/` tree) was validated and could fail `labelle generate` even though that pack isn't shipped — nested packs are only part of generation when declared in `plugin.labelle`'s `.packs` list (codex review on #588). Replace the blanket recursive `walk` with a scoped discovery that mirrors the pack-discovery path (`generate_phases.discoverNestedPacks`): validate ONLY (a) the plugin-root `studio/` dir, and (b) the `studio/` dir of each pack the plugin DECLARES in `.packs`. `collectPluginPanelErrors` loads the plugin manifest (`plugin_manifest.loadOptional`) and scans exactly those units via a new non-recursive `scanStudioDir`; the undeclared/example descendants are never touched. Tests: swap the recursive-walk tests for `scanStudioDir` unit tests and add a declared-vs-undeclared scope test — a DECLARED pack's broken panel fails, an UNDECLARED pack's broken panel does NOT. `zig build test` passes (the pre-existing flow_catalog sidecar failure is unrelated). Claude-Session: https://claude.ai/code/session_017pW3ifKf9wgxNg4viy6okw
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 69fd572d78
ℹ️ 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".
| for (pmani.packs) |nested_name| { | ||
| const pack_dir = try std.fs.path.join(allocator, &.{ plugin_dir, "packs", nested_name }); | ||
| defer allocator.free(pack_dir); | ||
| try scanStudioDir(allocator, plugin_dir, pack_dir, errors); |
There was a problem hiding this comment.
When a plugin's .packs list names a directory that does not contain pack.labelle, generate_phases.discoverNestedPacks/loadNestedPackEntry tolerates that and does not register the pack, but this loop still scans <plugin>/packs/<name>/studio. In that case a draft or broken panel in a nested directory that generation skips will abort labelle generate; load/confirm the nested pack the same way as pack discovery before calling scanStudioDir.
Useful? React with 👍 / 👎.
| while (i + 1 < out.len and !(out[i] == '*' and out[i + 1] == '/')) : (i += 1) { | ||
| if (out[i] != '\n') out[i] = ' '; | ||
| } | ||
| if (i + 1 < out.len) { |
There was a problem hiding this comment.
If a panel ends with a valid descriptor followed by an unterminated block comment, e.g. { "id":"p", "title":"P" } /*, this loop blanks through EOF and never records that the comment was not closed, so the stripped buffer still parses and labelle generate accepts malformed JSONC that the studio parser can reject. Track whether the */ terminator was found and report it as invalid JSONC instead of silently accepting it.
Useful? React with 👍 / 👎.
1 participant
Phase 3 of the asset-plugins epic. Design: labelle-toolkit/labelle-engine#725 · RFC
RFC-ASSET-PLUGINS.mdrev 4.Validates every
studio/*.panel.jsonca plugin (or a pack it bundles) ships atlabelle generatetime, alongside the Phase-1/2 asset + pack validation — so the studio never renders an invalid panel from a generated project. A malformed panel is a build error with a file (and, for parse errors, line) location, not a studio-side surprise.What it does
src/panel_validate.zig(new) — a faithful Zig port of the studio POC's zod schema (labelle-studiosrc/services/pluginPanels.ts, feat(plugins): Controller discovery + ship_from_plugin + two-block scripts #73):id/title/icon/fields/actions;idan identifier, non-emptytitle, optional stringicon;typefield branches (number/slider/select/text/toggle) with strict per-branch keys and a non-emptyname;sliderrequires numericmin+max;selectrequires a non-emptyoptionsarray;label, identifiercommand,target∈ {preview,cli} (strict);min > max, adefaultoutsidemin..max, aselectdefaultnot among itsoptions;std.json, byte-offset-preserving so parse errors carry a line);studio/-gated directory walk that discovers panels at a plugin root and inside its nested packs;validatePluginPanelsdrives it overcfg.plugins.src/root.zig— callsvalidatePluginPanelsingenerate, before any target is written; test aggregation +pubre-export.All problems for one file are collected and rendered as
<file>: <where>: <problem>— the studio's rules-sidecar shape, because a panel author's error text is an agent's compiler output.Tests
18 schema/semantic accept+reject cases (a full well-formed panel with comments/trailing commas; bad id/title; strict-key violations; unknown type; slider min/max; range; select options; duplicate names; bad actions; multi-error collection) plus 2 filesystem-discovery tests (
studio/-gating + a no-panel no-op through a real tmpdir seam).zig build test→ all pass except the pre-existingflow_catalog.emitFlowCatalogSidecarfailure (unrelated, present onorigin/main).Additive / scope
A project with no panels is unaffected; an unresolvable plugin dir is skipped. Deferred (pairs with engine#729): a
"target":"preview"command naming a plugin-declared handler can't be cross-checked here yet — the engine declares handlers by runtime subscription toengine__editor_plugin_command, so no static handler-name list exists at generate time. The schema/semantic gate is the bulk of the acceptance.Studio-side consumption (labelle-studio #74/#75) is a separate parallel effort.
Closes #577
https://claude.ai/code/session_017pW3ifKf9wgxNg4viy6okw