fix: sync configurator from SLASHED PR 460/461, close release-sync gap - #122
Merged
jackgranatowski merged 9 commits intoJun 30, 2026
Merged
Conversation
The configurator changes from SLASHED PR #460 (saved themes, motion panel overhaul) and #461 (base color palette, semantic preview) never made it into the vendored admin-app/src/ — two compounding bugs: 1. release.yml's "Commit framework sync" step runs build:admin-app (which syncs configurator/src/ fresh via its prebuild hook) but then checks out the default branch and only re-stages a fixed list of framework-CSS paths. admin-app/src/, .vendored-manifest.json, framework-css/, and the compiled assets/admin-app/ bundle were never in that list, so every release's sync was built into that release's zip and then silently discarded instead of being committed. Add those paths to the preserve/ restore/git-add steps so future syncs land for real. 2. .syncignore was still protecting App.svelte, PreviewPanel.svelte, StudioHeader.svelte, main.ts, vite-env.d.ts, and lib/persistence.ts against a divergence from SLASHED#443 that merged to main on 2026-06-28, two days before #460/#461. Removed the stale entries so these vendor normally. Re-ran the sync against current framework main to pull in both PRs, and fixed AppOverlay.svelte (plugin-specific, not vendored) to match the new onApplyTheme(overrides) contract now that ThemesPanel no longer passes a PresetTheme — it still imported the type #460 removed, which broke svelte-check. Rebuilt the admin-app bundle; verify-sync and the full test suite pass. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SQXr34nocCi1jrGcp5TPVm
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SQXr34nocCi1jrGcp5TPVm
|
Warning Review limit reached
Next review available in: 28 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: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (16)
📝 WalkthroughWalkthroughThis PR refactors the SLASHED admin-app configurator from preset-based theming to an overrides-snapshot model with localStorage-backed saved themes, rewrites the base color OKLCH ramp across CSS tokens and generated data, updates multiple panel UIs, rewrites the preview iframe skin, adds a reset confirmation dialog, and updates the release workflow/changelog/sync config. ChangesConfigurator theming and overrides refactor
Release workflow and sync configuration
Sequence Diagram(s)sequenceDiagram
participant User
participant ThemesPanel
participant AppSvelte
participant SavedThemesModule
participant PreviewPanel
User->>ThemesPanel: Click Save current as...
ThemesPanel->>SavedThemesModule: saveTheme(name, overrides)
SavedThemesModule->>SavedThemesModule: persist to localStorage
User->>ThemesPanel: Click Apply on saved theme
ThemesPanel->>AppSvelte: onApplyTheme(theme.overrides)
AppSvelte->>AppSvelte: setOverrides({...theme.overrides})
AppSvelte->>PreviewPanel: pass updated overrides
PreviewPanel->>PreviewPanel: withDerivedOverrides(overrides) + previewSkinCSS()
PreviewPanel-->>User: render updated iframe preview
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
Code Review by Qodo
Context used✅ Compliance rules (platform):
1 rule 1.
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
PR Summary by Qodofix: sync configurator from SLASHED PRs #460/#461, close release-sync gap
AI Description
Diagram
High-Level Assessment
Files changed (32)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
SLASHED-for-WP/admin-app/src/lib/persistence.ts (1)
66-71: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
getNumno longer rejects non-finite numbers.
isNaN(n)only catches actualNaN;parseFloatcan also returnInfinity/-Infinity(e.g. from an overflowing numeric override), which now passes through unguarded. Downstream,fmt()calls.toFixed(6)on it, which returns the literal string"Infinity"(confirmed:toFixednever throws on non-finite values, per MDN/ECMA-262) — producing invalid generated CSS like"Infinityrem"instead of falling back todef.🛠️ Proposed fix
function getNum(ov: Record<string, string>, key: string, def: number): number { const v = ov[key]; if (v === undefined) return def; const n = parseFloat(v); - return isNaN(n) ? def : n; + return Number.isFinite(n) ? n : def; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@SLASHED-for-WP/admin-app/src/lib/persistence.ts` around lines 66 - 71, `getNum` in `persistence.ts` only checks `isNaN`, so non-finite values like `Infinity` can still pass through and later break `fmt()` output. Update `getNum` to reject non-finite parsed numbers as well as `NaN`, and fall back to `def` whenever the override is not a finite number. Use the existing `getNum` helper and its callers in the persistence flow to keep invalid numeric overrides from propagating into CSS generation.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/release.yml:
- Around line 106-111: The restore step is asymmetric: `admin-app/src` is fully
wiped before copying, but `admin-app/framework-css/` and `assets/admin-app/` are
only overlaid, so deleted upstream files can linger as stale tracked files.
Update the release workflow restore logic around the `cp -a` steps to clear
those destination directories first, matching the `rm -rf` + recreate pattern
used for `admin-app/src`, then copy in the staged contents.
In `@SLASHED-for-WP/admin-app/src/App.svelte`:
- Around line 167-170: Restore the JSON import guard in App.svelte before
calling setOverrides: the current `typeof data === "object"` check in the file
import flow allows `null`, arrays, and other invalid shapes through. Update the
JSON parsing path so it only accepts a plain non-null object, matching the safer
import validation used by AppOverlay’s import logic, and only then call
`setOverrides`.
In `@SLASHED-for-WP/admin-app/src/components/panels/ColorsPanel.svelte`:
- Around line 298-305: Route the remaining source-token reads through
sourceValue so every endpoint uses the same precedence as edited colors. Update
the surface/text endpoint helpers in ColorsPanel.svelte that currently read
overrides or hardcoded defaults to call sourceValue with sourceByName for the
light/dark base sources and the matching neutral sources, so tokens with
synced/default values resolve consistently with swatches and live preview.
In `@SLASHED-for-WP/admin-app/src/components/panels/EffectsPanel.svelte`:
- Around line 283-292: The EffectsPanel.svelte input handler is checking
v.trim() but still passes the untrimmed value to onSet, so whitespace can be
saved in overrides. Update the oninput callback on the text input to trim the
value once and pass the trimmed string to onSet when it is non-empty, while
keeping onReset for empty input; use the existing overrides[t.token] binding and
onSet/onReset handlers as the anchors for the change.
In `@SLASHED-for-WP/admin-app/src/components/panels/MotionPanel.svelte`:
- Around line 39-43: Use $derived.by for staggerBase in MotionPanel so it
recomputes as a numeric value when overrides or scale change instead of
returning a stale function. Update the staggerBase declaration to derive the
number directly, and adjust any related call sites in MotionPanel that read
staggerBase so they treat it as a plain number rather than invoking it like a
function.
In `@SLASHED-for-WP/admin-app/src/components/panels/SpacingPanel.svelte`:
- Around line 55-61: The space scale preview in SpacingPanel.svelte is using the
wrong exponent offset, causing every bar in the SPACE_STEPS loop to render one
step too low. Update the offset calculation inside the SPACE_STEPS each block so
it matches the actual --sf-space-* mapping (with m at index 3), and verify the
rawRem/barWidth preview values now align with the real fluid scale for steps
like m and l.
In `@SLASHED-for-WP/admin-app/src/components/panels/ThemesPanel.svelte`:
- Around line 114-118: The swatch rendering in ThemesPanel.svelte is
interpolating persisted theme data directly into a raw style string, which can
turn malicious or malformed values into extra CSS declarations. Update the theme
preview block inside the Object.entries(theme.overrides) loop to avoid
string-based style interpolation for the swatch, and instead use a safe
property-level style binding or validate v as a real color before applying it.
Keep the fix localized to the k.includes("color") || k.includes("source") branch
so the rendered preview cannot trigger unintended CSS or outbound requests.
---
Outside diff comments:
In `@SLASHED-for-WP/admin-app/src/lib/persistence.ts`:
- Around line 66-71: `getNum` in `persistence.ts` only checks `isNaN`, so
non-finite values like `Infinity` can still pass through and later break `fmt()`
output. Update `getNum` to reject non-finite parsed numbers as well as `NaN`,
and fall back to `def` whenever the override is not a finite number. Use the
existing `getNum` helper and its callers in the persistence flow to keep invalid
numeric overrides from propagating into CSS generation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 701edf11-9046-46f7-be6c-46b298425741
📒 Files selected for processing (32)
.github/workflows/release.ymlCHANGELOG.mdSLASHED-for-WP/admin-app/.syncignoreSLASHED-for-WP/admin-app/.vendored-manifest.jsonSLASHED-for-WP/admin-app/framework-css/badges/slashed.full.cssSLASHED-for-WP/admin-app/framework-css/core/tokens.cssSLASHED-for-WP/admin-app/src/App.svelteSLASHED-for-WP/admin-app/src/AppOverlay.svelteSLASHED-for-WP/admin-app/src/components/DomainPanel.svelteSLASHED-for-WP/admin-app/src/components/inputs/PowerKnobRow.svelteSLASHED-for-WP/admin-app/src/components/inputs/StylePresetCards.svelteSLASHED-for-WP/admin-app/src/components/panels/BordersPanel.svelteSLASHED-for-WP/admin-app/src/components/panels/ColorsPanel.svelteSLASHED-for-WP/admin-app/src/components/panels/EffectsPanel.svelteSLASHED-for-WP/admin-app/src/components/panels/HomePanel.svelteSLASHED-for-WP/admin-app/src/components/panels/MacrosPanel.svelteSLASHED-for-WP/admin-app/src/components/panels/MotionPanel.svelteSLASHED-for-WP/admin-app/src/components/panels/ShadowsPanel.svelteSLASHED-for-WP/admin-app/src/components/panels/SpacingPanel.svelteSLASHED-for-WP/admin-app/src/components/panels/ThemesPanel.svelteSLASHED-for-WP/admin-app/src/components/panels/TypographyPanel.svelteSLASHED-for-WP/admin-app/src/components/shell/PreviewPanel.svelteSLASHED-for-WP/admin-app/src/components/shell/StudioHeader.svelteSLASHED-for-WP/admin-app/src/data/api-index.generated.jsonSLASHED-for-WP/admin-app/src/data/token-registry.generated.jsonSLASHED-for-WP/admin-app/src/lib/persistence.tsSLASHED-for-WP/admin-app/src/lib/savedThemes.tsSLASHED-for-WP/admin-app/src/lib/stylePresets.tsSLASHED-for-WP/admin-app/src/lib/themes.tsSLASHED-for-WP/admin-app/src/types.tsSLASHED-for-WP/assets/admin-app/app.cssSLASHED-for-WP/assets/admin-app/app.js
💤 Files with no reviewable changes (3)
- SLASHED-for-WP/admin-app/src/lib/stylePresets.ts
- SLASHED-for-WP/admin-app/src/lib/themes.ts
- SLASHED-for-WP/admin-app/src/components/inputs/StylePresetCards.svelte
…elease.yml cp -a src/. dest/ only adds/overwrites — it never removes files that no longer exist upstream. admin-app/src/ was already rm -rf'd before restore; apply the same wipe-then-copy to framework-css/ and assets/admin-app/ so deletions actually propagate to the default branch instead of leaving stale tracked files behind indefinitely. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SQXr34nocCi1jrGcp5TPVm
3 tasks
Re-syncs the configurator now that codeslash-dev/SLASHED#462 fixes the bugs CodeRabbit and Qodo found in this PR's vendored files: - App.svelte: null-safe JSON import (typeof null === "object" crash) - lib/persistence.ts: Number.isFinite guards against Infinity/-Infinity leaking into generated CSS - ColorsPanel.svelte: surface/text helpers now route through sourceValue()'s override precedence instead of reading overrides[...] directly - EffectsPanel.svelte: trim value before onSet - MotionPanel.svelte: staggerBase uses $derived.by so it actually recomputes - SpacingPanel.svelte: off-by-one exponent in the space-scale preview - ThemesPanel.svelte + swatch sites in ColorsPanel/EffectsPanel/ TokenRow/ColorInput/OklchColorDesk: style:background={value} instead of raw style={`background: ${value}`} string interpolation, closing the CSS-declaration-injection path the failing CodeQL check flagged These were pre-existing upstream bugs (not introduced by this PR's sync), fixed in SLASHED#462 since this repo has no CodeQL/CodeRabbit coverage of its own. verify-sync and the full test suite pass. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SQXr34nocCi1jrGcp5TPVm
Re-syncs after codeslash-dev/SLASHED#462 added a follow-up fix Qodo found on that PR: codec.ts's fa() interpolated override keys verbatim into generated CSS while only sanitizing values, letting a crafted override key (from imported JSON, the URL hash, localStorage, or WP hydration) break out of its CSS declaration in the live-preview <style> tag or exported stylesheet. Keys are now filtered to the --sf-[\w-]+ pattern every real token name matches. verify-sync and the full test suite (67/67) pass. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SQXr34nocCi1jrGcp5TPVm
…HED#462 Re-syncs two more CodeRabbit findings fixed upstream: - App.svelte: JSON import no longer wipes all overrides when the imported file has no string-valued keys - ColorsPanel.svelte: getDarkSurface() now resolves through the documented override -> loaded-token -> default precedence instead of reading overrides[...] directly, matching getLightSurface() verify-sync and the full test suite (67/67) pass. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SQXr34nocCi1jrGcp5TPVm
GitHub's CodeQL flagged the JSON-import handler as js/remote-property-injection: imported file keys flowed into Object.fromEntries() with only values filtered, so attacker-controlled property names from an untrusted JSON file ended up on the overrides object with no shape validation. - Re-syncs App.svelte's fix from SLASHED#462 (filters keys to --sf-[\w-]+ matching codec.fa()'s existing emission-time filter). - Applies the identical fix directly to AppOverlay.svelte, which has the same JSON-import pattern but is plugin-specific (not vendored, so it needs its own fix rather than a sync). verify-sync and the full test suite (67/67) pass. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SQXr34nocCi1jrGcp5TPVm
The CodeQL check has reported the same "8 high" alert count on every commit to PR #122 regardless of which source files actually changed — including after real fixes landed for the exact pattern one of those alerts named (js/remote-property-injection in handleImport()'s JSON import). That's because CodeQL was scanning the committed, minified assets/admin-app/app.js and bricks editor-app/app.js bundles: their line numbers shift on every rebuild, so alert fingerprinting can't match an alert to itself across commits and instead reports the same handful of flagged patterns as "new" every single push. GitHub's own UI can't even render a code preview for them ("snippet too large... may be minified"). Both directories are 100% build output (npm run build:apps); the actual hand-written source lives in admin-app/src/ and integrations/bricks/editor-app/src/, which remain fully scanned. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SQXr34nocCi1jrGcp5TPVm
The previous commit's top-level `paths-ignore` action input was silently
rejected — that input doesn't exist on this codeql-action version
('Unexpected input(s) paths-ignore', confirmed in the job log — the
action just warned and proceeded to scan everything, unfiltered, which
is why the alert count didn't change). paths-ignore must be nested
inside the inline `config` YAML input instead, which is how this
version of the action actually expects it.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SQXr34nocCi1jrGcp5TPVm
8 tasks
jackgranatowski
pushed a commit
that referenced
this pull request
Jul 1, 2026
Pulls in the App.svelte fix from codeslash-dev/SLASHED#464: the domain panel used w-full + shrink-0 in the same flex row as the icon nav rail, so it demanded 100% of the whole row's width instead of just the space left after the rail — overflowing the viewport by the rail's width with no scrollbar. Now flex-1 min-w-0 on mobile so it fills only what's actually left; desktop keeps its fixed 360px width via md:flex-none. Same root cause affects the WP admin Tokens page on mobile, independent of the earlier embedded-sizing fix (#122). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SQXr34nocCi1jrGcp5TPVm
8 tasks
This was referenced Jul 1, 2026
This was referenced Jul 8, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The configurator changes from framework PRs #460 (saved themes, motion panel overhaul) and #461 (base color palette, live semantic preview) merged cleanly into
codeslash-dev/SLASHEDbut never reached this repo's vendoredadmin-app/src/. Two compounding bugs in the sync pipeline:release.ymlsilently discarded every sync. The "Build admin-app" step runsnpm run build:admin-app, whoseprebuildhook (sync-core.mjs) correctly fetches the latestconfigurator/src/from GitHub on every release. But the next step ("Commit framework sync to default branch") doesgit checkout "$DEFAULT_BRANCH"and only re-stages a hardcoded list of framework-CSS paths (dist/,data/, the PHP*_CSS_REFfiles, etc.) —admin-app/src/,.vendored-manifest.json,framework-css/, and the compiledassets/admin-app/bundle were never on that list. So each release's sync was built into that release's zip and then thrown away instead of being committed to git. The repo's tracked vendored files were frozen at a one-off manual local sync from before either PR merged..syncignore. It was still protectingApp.svelte,PreviewPanel.svelte,StudioHeader.svelte,main.ts,vite-env.d.ts, andlib/persistence.tsagainst divergence fromSLASHED#443— a companion PR that merged tomainon 2026-06-28, two days before #460/#461. Those protections were obsolete and, left in place, would have skipped App.svelte's newhandleApplyTheme(overrides)signature while ThemesPanel.svelte (unprotected) called it with the new contract — a runtime breakage waiting to happen.Changes
.syncignoreentries.npm run syncagainst current frameworkmain, pulling in both PRs (savedThemes.ts, removal ofthemes.ts/stylePresets.ts/StylePresetCards.svelte, the colors panel base-ramp/semantic-preview work, mobile fold toggle, etc.).AppOverlay.svelte(plugin-specific, not vendored) — it still imported thePresetThemetype #460 removed and calledhandleApplyTheme(theme)with the old shape; updated to the new(overrides: Record<string,string>)contract to matchApp.svelte.release.ymlto preserve/restore/stageadmin-app/src/,.vendored-manifest.json,framework-css/, andassets/admin-app/across the branch checkout, so future releases actually commit what they sync instead of discarding it.assets/admin-app/app.js,app.css).Type
Checklist
feat:,fix:,docs:, …)npm testpasses (67/67)npm run lint— not run (no PHP/CSS changes in this PR)npm run verifypasses (version metadata in sync)admin-app/src/,framework-css/came fromnpm run sync)CHANGELOG.mdupdated under## [Unreleased]admin-appsource changed)Notes
svelte-checkis clean except a pre-existing, unrelated error inplugin-main.ts(allowImportingTsExtensions) — confirmed viagit stashthat it predates this change and is unaffected by it.Generated by Claude Code
Summary by CodeRabbit
New Features
Bug Fixes