From 87bf430e7a5f6ffe61f4569bf70b82f0b95cffa1 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 1 Jul 2026 15:08:33 +0000 Subject: [PATCH 1/2] docs: add technical debt audit Full 9-dimension audit of core/, optional/, configurator/, scripts/, tests/, and docs/. 34 findings (0 Critical, 1 High, 15 Medium, 18 Low), verified against clean tsc/svelte-check/stylelint/madge/npm-audit baselines. Highest-impact items: zero test coverage on the configurator's interactive layer, deliberately obfuscated codec.ts naming, and duplicated parsing helpers across the doc-gen scripts. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_013cK9t8cQQn4DNweTjJzVZi --- docs/technical-debt-audit.md | 107 +++++++++++++++++++++++++++++++++++ 1 file changed, 107 insertions(+) create mode 100644 docs/technical-debt-audit.md diff --git a/docs/technical-debt-audit.md b/docs/technical-debt-audit.md new file mode 100644 index 00000000..d07bc574 --- /dev/null +++ b/docs/technical-debt-audit.md @@ -0,0 +1,107 @@ +# SLASHED — Technical Debt Audit + +**Date:** 2026-07-01 · **Version audited:** 0.6.32 · **Scope:** full repo (`core/`, `optional/`, `configurator/`, `scripts/`, `tests/`, `docs/`) + +This is a living audit. Re-run it periodically; mark resolved findings `RESOLVED` and tag new ones `NEW` rather than starting over. + +## Tooling baseline (ground truth before manual review) + +| Tool | Scope | Result | +|---|---|---| +| `npm audit` | root deps | 0 vulnerabilities | +| `stylelint` | `core/**/*.css`, `optional/**/*.css` | 0 errors | +| `tsc --noEmit` | `configurator/` | 0 errors | +| `svelte-check` | `configurator/` | 0 errors, 0 warnings (3557 files) | +| `madge --circular` | `configurator/src` | no circular deps | +| `depcheck` | `configurator/`, root | see findings SL-021/SL-022; root-level "unused" devDeps are false positives (commitlint/release-it/stylelint-config are CLI-hook/config-extends, not imports) | + +Mechanical gates are clean across the board. This audit therefore concentrates on what static tooling structurally cannot see: duplication, drift, contract looseness, silent failure paths, and documentation accuracy. + +## Executive summary + +- **0 Critical, 1 High, 15 Medium, 18 Low** findings (34 total). +- The framework CSS itself (`core/`, `optional/`) is disciplined — the worst issue there is a hand-duplicated dark-mode color formula between `tokens.css` and `themes.css` (SL-001). +- The **build/doc-gen scripts** (`scripts/*.js`) have real duplication: `stripComments`, `readValue`, and `readFile` helpers are copy-pasted across 4+ generator scripts with no shared `lib/` module (SL-007) — and none of the generated JSON is validated before being written, so a bad regex change could silently ship a truncated `token-registry.json`/`api-index.json` (SL-012). +- The **configurator app** is where the highest-severity finding lives: the entire interactive layer (`persistence.ts`, `previewResolver.svelte.ts`, every panel's slider/token-editing interaction) has **zero test coverage** (SL-023, High) — only the pure codec/data logic is tested. +- `configurator/src/lib/codec.ts` — the persistence/encode module that's the single most load-bearing file in the app — uses deliberately obfuscated 1-2 letter internal identifiers (`Wa`, `Ra`, `za`...) with readable names only aliased at the export boundary (SL-016). It's well-tested and defensively coded, just hard to read. +- Save failures in the configurator fail silently: a REST/network error during save only does `console.warn` and resets UI state to `'idle'` — indistinguishable from "never tried to save" (SL-018). +- `motion` (^12.23.24) is a fully unused dependency in `configurator/package.json` (SL-021); `lucide-svelte` is actively used but deprecated upstream in favor of `@lucide/svelte` (SL-022). +- Two documentation-drift findings actually matter for correctness of generated guidance: `docs/llm-guide.md` lists `optional/utilities.css` as if live when it's 100% commented-out stub (SL-030), and README's à-la-carte `` order example doesn't match the real cascade order in `core/layers.css` (SL-029). +- No security findings of note — no remote `url()` refs, no unsafe `{@html}`/`innerHTML` sinks beyond one safe mount-clear. + +## Architectural mental model + +SLASHED is a zero-runtime-dependency, cascade-`@layer`-based CSS framework. Source of truth lives in `core/` (required layers: tokens → reset → base → layout → macros → states → motion → accessibility → print) and `optional/` (forms, legacy shims, and a components/utilities layer that is real but almost entirely unimplemented — tracked in-repo as open work, not hidden debt). `scripts/*.js` is a set of independent Node CLI generators that derive `docs/api-index.json`, `token-registry.json`, and doc pages from the CSS source by regex-parsing comment banners and `@property` declarations — there is no shared parsing library between them, so each reimplements comment-stripping and value-scanning. `configurator/` is a standalone Svelte 5 + TypeScript SPA (also vendored into a sibling WordPress plugin repo) that lets users visually override tokens; it persists overrides via a compact custom binary codec (`codec.ts`) and either localStorage (standalone) or a WP REST endpoint (embedded mode). Tests are split between Playwright (browser-rendered CSS behavior) and Node's built-in test runner (data/script correctness) — a reasonable split that one misclassified file (`coverage.spec.js`) breaks. + +## Findings + +| ID | Category | File:Line | Severity | Effort | Description | Recommendation | +|---|---|---|---|---|---|---| +| SL-001 | Consistency rot | `core/themes.css:77-165`, `core/tokens.css:361-421` | Medium | M | Dark/light OKLCH derivation clamp formulas are hand-duplicated (once as `light-dark()` in tokens.css, once as flat `[data-theme]` rules in themes.css) with no shared source. | Extract repeated clamp expressions into intermediate custom properties referenced from both files, or cross-link with comments. | +| SL-002 | Performance | `core/accessibility.css:166-183` | Medium | M | `.sf-clickable-parent` combines 3 separate `:has()`-bearing selector branches, each forcing subtree re-evaluation on DOM mutation. | Add a doc note warning against applying the class to large (100+ node) grids; no rewrite needed, it's opt-in. | +| SL-003 | Type/contract | `core/tokens.css:107-115` | Low | S | `--sf-is-dark` is commented "do not set directly" while `--sf-is-active/-current/-pressed/-open` are "public hooks" — same naming pattern, opposite contracts. | Distinguish internal-only flags by naming convention, not prose alone. | +| SL-004 | Consistency rot | `core/layout.css` (lines 8,54,73,98,104,119,139,171,186,231,249,273) | Low | S | Section dividers are unlabeled blank lines, inconsistent with labeled sections in `tokens.css`/`macros.css`. | Add short section labels matching sibling files' style. | +| SL-005 | Consistency rot | `core/layout.css:370,398,466` | Low | S | Only one of four hardcoded-`@container`-breakpoint sites explains *why* (`var()` isn't allowed inside `@container` per spec). | State the constraint once near the top of the file; it applies framework-wide. | +| SL-006 | Architectural decay | `scripts/gen-api-index.js:1-859` | Medium | M | 859-line script mixes annotation loading, comment parsing, token/class extraction, Markdown rendering, and orchestration in one file. | Split extraction from rendering; JSDoc is already thorough so this is a readability/testability split, not a rewrite. | +| SL-007 | Architectural decay | `scripts/audit.js:42`, `scripts/check-macro-catalog.js:43`, `scripts/gen-token-index.js:42,48`, `scripts/gen-api-index.js:124,142,323`, `scripts/gen-token-reference.js:22`, `scripts/version-sync.js:19` | Medium | M | `stripComments`/`maskComments`, `readValue`, and `readFile` are each reimplemented 3-4 times with subtly different semantics; header comment in gen-api-index.js claims a "canonical parsing contract" that isn't actually shared. | Extract to a `scripts/lib/parse.js` module; a bugfix in one masking regex currently can't propagate. | +| SL-008 | Consistency rot | `scripts/*.js` repo-wide | Low | M | Error handling style varies: throw-on-missing, warn-and-default, `process.exit(1)`, and bare `catch {}` are all used with no documented convention. | Not urgent at this scale (each script is a single CI-gate job), but document the convention if the script count grows further. | +| SL-009 | Consistency rot | `scripts/audit.js:177,189`, `scripts/bundle.js:303`, `scripts/check-artifacts.js:20`, `scripts/changelog-release.js:15` | Low | S | Every script hand-rolls `process.argv` parsing with no shared helper or `--help`. | Fine at current flag-count; revisit if any script grows past ~2 flags. | +| SL-010 | Type/contract debt | `scripts/*.js` (module-wide) | Medium | L | No TypeScript; JSDoc typing exists only in `gen-api-index.js`. No runtime schema validation (zod/ajv) before `fs.writeFileSync` in `gen-api-index.js:847`, `gen-token-registry.js:133`, `gen-token-index.js:137`. | Add a minimal shape-check (even a hand-rolled assert) before writing generated JSON that downstream tooling trusts. | +| SL-011 | Dependency/config | `scripts/check-version-sync.js:20`, `scripts/bundle.js:24-25`, `scripts/check-token-registry.js:53` | Low | S | `SLASHED_ROOT`, `GITHUB_REF`/`GITHUB_REF_NAME`, `GITHUB_BASE_REF` env vars are read with no central documentation. | Note them in CLAUDE.md or a script-level comment explaining test-only vs CI-only usage. | +| SL-012 | Error handling | `scripts/gen-token-registry.js:133`, `scripts/gen-api-index.js:847` | Medium | M | No sanity check (e.g. "entry count didn't shrink drastically") before overwriting `token-registry.json`/`docs/api-index.json`, which `check-llm-guide.js` and the configurator sync trust as ground truth. | Add a basic regression guard (min entry count, or diff-size threshold) before write. | +| SL-013 | Security hygiene | `scripts/check-artifacts.js:23`, `scripts/changelog-release.js:54` | Low | S | `execSync(cmd, { shell: true })` where `cmd` is sourced from a repo-committed config file (`artifacts.json`) — not attacker input today, but the pattern grants full shell execution to any future entry added there. | No immediate action; note the pattern in a comment so future `artifacts.json` entries are added deliberately. | +| SL-014 | Documentation drift | `CLAUDE.md` "Key scripts" table vs `package.json:33-58` | Low | S | 16 of ~24 npm scripts (`watch`, `audit:check`, `gen:registry`, `check:macros`, `check:registry`, `docs:tokens/index/api/classes`, `lint:css(:fix)`, `pretest`, `test:unit`, `test:install`) are undocumented in CLAUDE.md, including two CI gates (`check:macros`, `check:registry`). | Add the missing rows, at minimum the CI-gate scripts. | +| SL-015 | Architectural decay | `configurator/src/components/panels/ColorsPanel.svelte:423,519,692`, `BordersPanel.svelte:340`, `AllTokensTab.svelte:33,41`, `GenericTokenPanel.svelte:29` | Low | S | The `name in overrides` / `overrides[name] !== undefined` override-check idiom is repeated ~15 times ad hoc instead of one shared helper. | Extract a one-line `isOverridden(overrides, name)` helper in `lib/`. | +| SL-016 | Type/contract debt | `configurator/src/lib/codec.ts:11-292` | Medium | M | Every internal symbol in the persistence/encode/CSS-export module uses cryptic 1-2 letter names (`Wa`, `Ra`, `za`, `Ba`, `Va`...), with readable aliases only exported at lines 283-292. Reads like committed minifier output. | Rename internal symbols to their alias names; keep the export surface identical. No behavior change. | +| SL-017 | Type/contract debt | `codec.ts:73,115,257,277`, `App.svelte:16`, `CheatsheetPanel.svelte:11,20,31` | Medium | S | `token-registry.generated.json`/`api-index.generated.json` are consumed via `any`-typed params/casts at 7 call sites rather than a shared interface. | Define a `TokenRegistry`/`ApiIndex` type once in `types.ts`; `SlashedToken` already exists as a base to extend. | +| SL-018 | Error handling | `configurator/src/App.svelte:109-112` | Medium | S | `handleSave` catches all save errors (network/REST failure) with only `console.warn`, resetting `saveState` to `'idle'` — UI can't distinguish "failed" from "never attempted." | Add an explicit `'error'` state to the `saveState` union and surface it in the header/status bar. | +| SL-019 | Consistency rot | `configurator/src/lib/persistence.ts:217-220` vs `savedThemes.ts:13-40` | Medium | S | `savedThemes.ts` validates parsed JSON shape before trusting it; `persistence.ts:loadInitialOverrides`'s standalone path does `JSON.parse(local)` and returns it directly as `Record` with no guard, feeding straight into override state and generated CSS. | Reuse a shape-guard (`isStringRecord`-style) in `persistence.ts` for defense-in-depth consistency. | +| SL-020 | Performance | `configurator/src/components/shell/PreviewPanel.svelte:515-548`, `lib/previewResolver.svelte.ts:70-75` | Medium | M | Every override change (including every keystroke/drag tick) regenerates full preview CSS and calls `bumpPreviewVersion()`, which clears the entire color-resolution cache — no debounce/throttle anywhere in the chain. Speculative (no reported perf complaint), but an unguarded hot path. | Consider rAF-coalescing `bumpPreviewVersion`/CSS injection if slider jank is ever reported. | +| SL-021 | Dependency debt | `configurator/package.json:16` | Low | S | `motion` (^12.23.24) has zero imports anywhere in `src/`. | Remove from `package.json`. | +| SL-022 | Dependency debt | `configurator/package.json:15` | Low | M | `lucide-svelte` (^0.468.0) is actively used in ~9 files but is deprecated upstream in favor of `@lucide/svelte`. | Migrate import paths in a dedicated PR (low risk, touches many files). | +| SL-023 | Test debt | `configurator/tests/*`, `configurator/tests-components/*` | **High** | L | All existing tests cover pure codec/data logic. Zero tests touch `persistence.ts` (save/load/error paths, WP-vs-standalone branching), `previewResolver.svelte.ts` (resolution/caching), or any panel's token-editing interaction. | Add component tests for `SliderRow`/`TokenRow` (bind/reset/raw-mode) and a unit test for `persistence.ts` covering both branch modes plus the malformed-localStorage case from SL-019. | +| SL-024 | Type/contract debt | `codec.ts:115,257,277` | Low | S | Options objects (`{ sanitize, isKnown }`) typed `any` instead of a small interface. | Define a `DecodeOptions` interface and use it at all 3 call sites. | +| SL-025 | Consistency rot | `configurator/src/lib/previewResolver.svelte.ts:34` | Low | S | The only module-level `.svelte.ts` reactive singleton in the app; every other cross-cutting concern is prop-drilled through `App.svelte`/`DomainPanel`. Two idioms for "shared state" with no documented rule for when to use which. | Document the convention so future additions don't diverge further. | +| SL-026 | Consistency rot | `codec.ts:14,17` | Low | S | Two separately-named constants (`Na`, `Ia`) share the identical value `65535` for overlapping purposes (max value length vs max id) — easy to drift if one changes. | Rename to intent-revealing names or comment why they're independent. | +| SL-027 | Architectural decay | `tests/coverage.spec.js:1-67` | Medium | S | File is `.spec.js` (Playwright-discovered, runs 3x across chromium/firefox/webkit) but never touches a browser — pure fs/regex work, tripling redundant CI work. | Rename to `.test.js`, run via `node --test`/`test:unit`. | +| SL-028 | Consistency rot | `package.json` scripts + `tests/*.{spec,test}.js` | Medium | S | The Playwright-vs-`node:test` split is mostly principled but SL-027 shows it's unenforced. | Add a one-line convention note in `tests/`. | +| SL-029 | Documentation drift | `README.md:41-53` vs `core/layers.css:6-20` | Medium | S | À-la-carte `` example lists `themes.css` right after `base.css`, but `layers.css`'s real `@layer` order puts `slashed.themes` after `slashed.utilities`/`slashed.states`. `` order is cosmetically misleading even though it's functionally harmless (layer order, not link order, controls the cascade). | Reorder the snippet to match `layers.css`, or add a note that link order doesn't matter once `layers.css` loads first. | +| SL-030 | Documentation drift | `docs/llm-guide.md:63` | Medium | S | Lists `optional/utilities.css # Utility classes` with no indication the file is a commented-out stub (0 live rules) — exactly the qualitative drift the `check:llm-guide` CI gate cannot catch (it only checks renamed/deleted tokens, per CLAUDE.md). An LLM using this guide would generate code with dead classes. | Add the same `(staged — commented out, not yet active)` annotation README.md already uses. | +| SL-031 | Documentation drift | `docs/migration.md:6` | Low | S | Header reads "SLASHED 0.6.25 → next" but current version is 0.6.32 (7 patch releases later) — these changes have shipped. | Rename header to the version range that actually shipped, consistent with versioned headers elsewhere in the same file. | +| SL-032 | Consistency rot | `tests/tier1-p2-*.js`, `tier1-p7-*.js`, `tier1-p8-*.js`, `tier1-p10-*.js` | Low | S | Numbered P2/P7/P8/P10 with no P1/P3-P6/P9 anywhere in the repo, and no comment explaining the scheme. | Add a short comment explaining the tier numbering, or renumber sequentially if the gaps are dead history. | +| SL-033 | Architectural decay | `tests/*.spec.js` (9+ files) | Low | S | Fixture loading is split between a shared `tests/fixture.html` and per-file inline `page.setContent(...)` boilerplate duplicated 3+ times with near-identical viewport/doctype/addStyleTag setup. | Extract a shared `renderWithBundle(page, html, bundle)` helper. | +| SL-034 | Test debt | `tests/typography.spec.js:54` and fluid-scale engine overall | Medium | S | The fluid clamp math (`core/tokens.css:120-141`) — flagged in CLAUDE.md as a core primitive — has only incidental coverage inside a typography test; no dedicated test file for the engine itself (space/display scales, arbitrary clamp recipes). | Low priority given partial coverage exists; add `fluid-scale.spec.js` if the engine grows more variants. | + +## Top 5 — if you fix nothing else, fix these + +1. **SL-023 — Test the configurator's interactive layer.** The entire save/load/preview path (the actual product surface most users touch) has zero coverage. Start with `persistence.ts`: a `vitest` unit test exercising `loadInitialOverrides`/`saveOverrides` in both standalone (localStorage) and WP-embedded (REST) modes, plus the malformed-localStorage edge case from SL-019. Then a component test for `SliderRow`/`TokenRow` covering bind → override-set → reset. +2. **SL-018 — Surface save failures to the user.** In `App.svelte:109-112`, add `saveState = 'error'` to the union type and render an inline error state in `StudioHeader`/status bar instead of silently reverting to `'idle'`. Small diff, directly prevents silent data loss for users. +3. **SL-007 — Deduplicate the script-generator parsing helpers.** Create `scripts/lib/parse.js` exporting `stripComments`, `readValue`, `readFile` once; update `audit.js`, `check-macro-catalog.js`, `gen-token-index.js`, `gen-api-index.js`, `gen-token-reference.js`, `version-sync.js` to import from it. This is the one change most likely to prevent a future silent-drift bug (SL-012 depends on the same surface). +4. **SL-016 — De-obfuscate `codec.ts`.** Pure rename, zero behavior change, but this file gates every save/load/share-URL in the app — the current naming makes it needlessly hard to review changes to security- and correctness-critical code. +5. **SL-030 — Fix `docs/llm-guide.md`'s utilities.css claim.** One-line annotation fix, but this is the authoritative LLM reference per CLAUDE.md — as written it will cause any LLM (including future Claude sessions) to generate code using classes that don't exist yet. + +## Quick wins (Low effort × Medium+ severity) + +- [ ] SL-018 — add `'error'` state to configurator save flow +- [ ] SL-027 — rename `coverage.spec.js` → `coverage.test.js`, move off Playwright +- [ ] SL-030 — annotate `optional/utilities.css` as staged in `docs/llm-guide.md` +- [ ] SL-029 — fix README's à-la-carte `` order example +- [ ] SL-021 — remove unused `motion` dependency +- [ ] SL-034 — (optional, low priority) add a dedicated fluid-scale test file + +## Things that look bad but are actually fine + +- **`optional/components.css`/`tokens.components.css` almost entirely commented out.** Confirmed tracked, active WIP (issue #384, "Components: first batch"). Spot-checked several token references inside the commented blocks (`--sf-radius-m`, `--sf-space-l`, `--sf-color-surface`, `--sf-shadow-s`) — all still live, not stale. The commenting itself is clean and consistent. +- **`core/tokens.css` at 1642 lines.** Large but flat — a mechanical list of `@property` registrations and derivations, not tangled logic. The maintainers already split layout/macro tokens into sibling files; what remains is legitimately one cohesive "base" token set. +- **30+ `!important` uses across `print.css`/`states.css`/`accessibility.css`.** Every instance reviewed is a deliberate override-proofing mechanism (print must beat screen styles, a11y must beat component authors, `.is-*` state utilities are documented as override-safe by design). No instance fights another `!important` within the same layer — not a specificity war. +- **`ColorsPanel.svelte` (1155 lines), `TypographyPanel.svelte` (1006), `LayoutPanel.svelte` (723).** Long, but reuse shared `SliderRow`/`TokenRow`/`ClampField` components heavily and have very few reactive (`$derived`/`$effect`) blocks relative to size — length comes from flat, repetitive per-domain markup (light/dark pairs × many color roles), not hidden complexity. +- **`codec.ts`'s obfuscated naming (SL-016).** Despite the readability issue, the module has thorough contract comments, defends every boundary (max compressed/decompressed byte limits, try/catch around inflate, id bounds-checking), and is the single most heavily-tested file in the app (`codec.test.js`, `css.test.js`, `share.test.js`). The debt is naming, not correctness. +- **depcheck flagging `tailwindcss`, `@commitlint/cli`, `@commitlint/config-conventional`, `@release-it/conventional-changelog`, `stylelint-config-standard` as unused.** All verified live — consumed via Vite plugin, git hooks, and config `extends`, not JS imports. Classic depcheck blind spot, not real debt. +- **`@framework-css/core` flagged by depcheck as a "missing dependency."** It's a Vite path alias (`vite.config.js:21`) to the sibling `core/` CSS directory, not an npm package. + +## Open questions for the maintainer + +1. Is the `themes.css`/`tokens.css` formula duplication (SL-001) intentional (section-level theming genuinely needs re-declaration) or an accident of incremental development? If intentional, it deserves a cross-reference comment at minimum. +2. Is there an appetite for a `scripts/lib/` shared module, or is the current copy-paste-per-generator style deliberate to keep each script single-file-portable? +3. Should `codec.ts`'s symbol names (SL-016) stay obfuscated for any reason (e.g. a historical minification-by-hand size optimization for a code-golf constraint), or is it safe to rename? +4. Is `optional/utilities.css` targeted for the same release as `optional/components.css` (issue #384), or on a separate timeline? That affects how urgently SL-030 should be fixed. From b97fd8dd8608c5f977a6e315c62c732c0df87cc2 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 1 Jul 2026 18:38:20 +0000 Subject: [PATCH 2/2] docs: add technical debt remediation plan Sequenced, multi-PR plan covering all 71 findings from both repos' audits: 7 themed PRs for SLASHED and ~13 for SLASHED-Plugins, grouped by risk/theme with explicit dependency ordering (framework fixes land first, then the plugin syncs the vendored copy). Cross-links the plan from each audit doc. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_013cK9t8cQQn4DNweTjJzVZi --- docs/technical-debt-audit.md | 2 + docs/technical-debt-remediation-plan.md | 248 ++++++++++++++++++++++++ 2 files changed, 250 insertions(+) create mode 100644 docs/technical-debt-remediation-plan.md diff --git a/docs/technical-debt-audit.md b/docs/technical-debt-audit.md index d07bc574..400a67fe 100644 --- a/docs/technical-debt-audit.md +++ b/docs/technical-debt-audit.md @@ -2,6 +2,8 @@ **Date:** 2026-07-01 · **Version audited:** 0.6.32 · **Scope:** full repo (`core/`, `optional/`, `configurator/`, `scripts/`, `tests/`, `docs/`) +See [`technical-debt-remediation-plan.md`](./technical-debt-remediation-plan.md) for the sequenced, multi-PR plan to fix every finding below. + This is a living audit. Re-run it periodically; mark resolved findings `RESOLVED` and tag new ones `NEW` rather than starting over. ## Tooling baseline (ground truth before manual review) diff --git a/docs/technical-debt-remediation-plan.md b/docs/technical-debt-remediation-plan.md new file mode 100644 index 00000000..9ba553e8 --- /dev/null +++ b/docs/technical-debt-remediation-plan.md @@ -0,0 +1,248 @@ +# Remediation plan — SLASHED + SLASHED-Plugins technical debt audits + +Companion plan to the audits: +- `docs/technical-debt-audit.md` (this repo) — 34 findings (SL-001..SL-034) +- `SLASHED-Plugins/docs/technical-debt-audit.md` — 37 findings (PL-001..PL-037) + +## Context + +A full 9-dimension technical-debt audit was completed and committed to both repos on branch `claude/codebase-audit-protocol-qr7xrn`: +- `/home/user/SLASHED/docs/technical-debt-audit.md` — 34 findings (SL-001..SL-034) +- `/home/user/SLASHED-Plugins/docs/technical-debt-audit.md` — 37 findings (PL-001..PL-037) + +The goal is to fix **all 71 findings**, delivered as **multiple themed PRs per repo** (not one giant PR, not one-commit-per-finding), with **SLASHED's framework-level fixes landed first**, then **SLASHED-Plugins syncs the vendored copy** (`npm run sync` pulls `SLASHED/configurator/src` into `SLASHED-for-WP/admin-app/src`) before Plugins-only work proceeds — except anything in Plugins that's fully independent of the vendored code (the Critical broken-CI-script bug, the Bricks/Gutenberg color-model drift bug, and the two `.syncignore`'d plugin-specific files), which ships immediately regardless of SLASHED's status. + +This plan was validated by a design-review pass that read the actual source files (not just the audit prose) and corrected several assumptions along the way — notably: `stripComments` vs `maskComments` in the scripts/ dedup work are **not** interchangeable (one preserves character offsets for line-number reporting, one doesn't); `npm run verify` in SLASHED-Plugins does something entirely unrelated to what the missing `npm run check` is documented to do, so PL-025 requires **authoring a new script**, not redirecting to an existing one; the target environment has no MySQL, so the new PHP test suite (PL-006) should use **Brain Monkey**, not the official WP core test scaffold. All three corrections are folded into the plan below. + +One finding, **PL-009** (`readme.txt`'s `Tested up to: 7.0`), is internally self-consistent and not provably wrong — it is **deferred as a maintainer question**, not auto-fixed in any PR. + +## Sequencing overview + +``` +SLASHED (parallel except noted): + PR1 (CSS cleanup), PR2 (docs/test hygiene), PR3 (scripts dedup) — independent, any order + PR4 (codec.ts readability) ─┐ parallel-safe (both touch App.svelte, non-overlapping lines) + PR5 (configurator UX/deps) ─┘ + PR6 (preview debounce) — after PR5 + PR7 (configurator test coverage) — after PR4, PR5, PR6 + +SLASHED-Plugins Wave 0 (ship immediately, no dependency on SLASHED): + PR-B1 (color-model -dark fix), PR-C1 (fix broken CI script + doc drift), + PR-C3 (plugin-specific frontend hardening), PR-C4 (wire playwright-admin.js into CI) + +SLASHED-Plugins Wave 1 (sync): + PR-SYNC — after SLASHED PR4 + PR5 + PR6 merge to main (NOT PR7 — its tests aren't vendored) + — benefits from PR-C1 already landed (reuses its new drift-check) + +SLASHED-Plugins Wave 2 (independent of sync, parallel with Wave 1 or after): + PR-A1 (PHP hardening), PR-A2 (PHP architecture, low priority), PR-A3 (PHPUnit suite), + PR-B2 (integrations hardening + Gutenberg README), PR-B3 (apply.js tests), + PR-C2 (sync mechanism hardening — after PR-C1, same file), PR-C5 (build-artifact policy) + +Deferred: PL-009 — resolve via a direct question to the maintainer, not a PR. +``` + +--- + +## SLASHED + +### PR1 — CSS source cleanup (SL-001..005) +Independent, any order, low risk. +- **SL-001**: don't unify the duplicated dark/light OKLCH formulas (`core/tokens.css:361-421` vs `core/themes.css:77-165`) without maintainer input on intent (see open question) — add a cross-link comment in both files for now. +- **SL-002**: doc-comment only, above `.sf-clickable-parent` (`core/accessibility.css:166-183`). +- **SL-003**: grep `core/**/*.css` and `configurator/src/**` for `var(--sf-is-dark)` reads before touching anything — prefer strengthening the "internal only" comment over renaming a public custom property. +- **SL-004/005**: mechanical — match `core/tokens.css`/`core/macros.css`'s existing section-comment style in `core/layout.css`; consolidate the repeated `@container` hardcoded-breakpoint explanation into one comment near the top of the file. + +**Verify:** `npm run lint:css && npm run build && npm test` — diff of `core/` should be comment-only. + +### PR2 — Docs & test hygiene (SL-014, 027-034) +Independent, parallel-safe with PR1. +- **SL-027/028**: rename `tests/coverage.spec.js` → `coverage.test.js` (confirm no Playwright fixtures used first), add to `pretest`/`test:unit` in `package.json` alongside existing `tier1-p*.test.js` entries, add a one-line convention note in `tests/`. +- **SL-029**: reorder README's à-la-carte `` example to match `core/layers.css:6-20`'s real `@layer` order. +- **SL-030**: annotate `docs/llm-guide.md:63`'s `optional/utilities.css` line with the same `(staged — commented out, not yet active)` wording README already uses. +- **SL-031**: retitle `docs/migration.md:6` to the version range that actually shipped, matching other headers in the file. +- **SL-032**: add one comment explaining the tier-numbering gaps in `tests/tier1-p*.js`; don't renumber. +- **SL-033**: diff the 3+ duplicated `page.setContent(...)` blocks character-by-character before extracting a shared `renderWithBundle(page, html, bundle)` helper — parameterize rather than force uniformity if they differ. +- **SL-034**: add a small `tests/fluid-scale.spec.js` (low priority, keep minimal). +- **SL-014**: add the 16 missing script rows to CLAUDE.md's "Key scripts" table, prioritizing the two CI gates (`check:macros`, `check:registry`). + +**Verify:** `npm run test:unit && npx playwright test && npm run check:llm-guide && npm test`. + +### PR3 — Build script hardening (SL-006..013) +Independent, **highest technical risk** in this wave. +- **Do SL-007 first**: extract `scripts/lib/parse.js`. Confirmed `stripComments` (used in `audit.js:42`, `check-macro-catalog.js:43`, `gen-token-index.js:42`) removes comments and collapses length, while `gen-api-index.js:124`'s `maskComments` blanks characters in place to **preserve offsets** for line-number reporting — export both as distinct named functions, do not merge them. Same caution applies to `readFile`/`readValue` variants across `audit.js`, `gen-api-index.js`, `gen-token-reference.js`, `version-sync.js` — diff behavior before consolidating any pair. +- **SL-006**: split `gen-api-index.js` (859 lines) along its existing section-comment boundaries into `scripts/lib/api-index/extract.js` + `render.js`, after SL-007's lib extraction so it isn't done twice. +- **SL-010 + SL-012**: add a minimal hand-rolled shape/size-sanity check before the three `fs.writeFileSync` sites (`gen-api-index.js:847`, `gen-token-registry.js:133`, `gen-token-index.js:137`) — no new dependency needed. +- **SL-008/009/011/013**: comment/doc-only additions — no restructuring (audit itself says "not urgent at this scale"). + +**Verify (this is the load-bearing check for this PR):** +```bash +cd /home/user/SLASHED +npm run docs && cp docs/api-index.json token-registry.json "$TMPDIR/audit-verify-before/" +# apply the refactor +npm run docs +diff "$TMPDIR/audit-verify-before/api-index.json" docs/api-index.json # must be empty +diff "$TMPDIR/audit-verify-before/token-registry.json" token-registry.json # must be empty +npm run check:macros && npm run check:registry && npm run check:llm-guide && npm run audit:check && npm test +``` +If either diff is non-empty, bisect by reverting one script's import at a time — regex changes here fail silently, not loudly. + +### PR4 — Configurator codec.ts readability & type-safety (SL-016, 017, 024, 026) +Parallel-safe with PR5. Must land before PR7. +- **SL-016/026**: rename internal 1-2 letter symbols (`Wa`, `Ra`, `za`...) to their already-existing readable names — confirmed present as export aliases at `configurator/src/lib/codec.ts:283-292` (`encode`, `decode`, `sanitizeValue`, `generateCSS`, `parseCSS`, `encodeOverrides`, `buildShareUrl`, `readShareFromHash`). **Keep the public export surface unchanged** — only rename internals. Use IDE scope-aware rename, not find/replace, to avoid touching string/wire-format literals. +- **SL-017/024**: add `TokenRegistry`/`ApiIndex`/`DecodeOptions` interfaces once in `configurator/src/lib/types.ts` (extending the existing `SlashedToken`), replace the 7 `any`-typed sites at `codec.ts:73,115,257,277`, `App.svelte:16`, `CheatsheetPanel.svelte:11,20,31`. + +**Verify:** `npx tsc --noEmit && npx svelte-check --tsconfig ./tsconfig.json && npm run test:unit` (codec.test.js/css.test.js/share.test.js must pass unchanged) — run incrementally per-rename, not just at the end. + +### PR5 — Configurator correctness, UX & dependency cleanup (SL-015, 018, 019, 021, 022, 025) +Parallel-safe with PR4. Before PR6/PR7. +- **SL-018** (top priority): in `configurator/src/App.svelte:109-112`, add `'error'` to the `saveState` union, set it in the catch instead of reverting to `'idle'`, and add a render branch in `StudioHeader`/status-bar mirroring its existing `'saved'` state. **Requires a manual `npm run dev` check** — a type-only union change passes `tsc` even if the UI has no render branch for it. +- **SL-019**: reuse `savedThemes.ts:13-40`'s existing shape-guard for `persistence.ts:217-220`'s unguarded `JSON.parse(local)` — import the guard, don't duplicate it. +- **SL-015**: extract `isOverridden(overrides, name)` into `lib/`, replace ~15 call sites across `ColorsPanel.svelte`, `BordersPanel.svelte`, `AllTokensTab.svelte`, `GenericTokenPanel.svelte` — confirm `name in overrides` and `overrides[name] !== undefined` are truly equivalent here (no key ever explicitly set to `undefined`) before collapsing both idioms into one helper. +- **SL-021**: re-grep for `motion` imports immediately before removing from `configurator/package.json`; `npm install` to update the lockfile. +- **SL-022**: migrate `lucide-svelte` → `@lucide/svelte` file-by-file (not one mechanical batch) across the ~9 confirmed files, building after each since the two packages' API surface isn't guaranteed identical. +- **SL-025**: doc-comment only, near `previewResolver.svelte.ts:34`'s module-level `$state` singleton. + +**Verify:** `npx tsc --noEmit && npx svelte-check ... && npm run test:unit && npm run test && npm run test:e2e && npm audit`, then `cd /home/user/SLASHED && npm run check:version` (CLAUDE.md requires this after touching `configurator/package.json`). + +### PR6 — Configurator preview debounce (SL-020) +After PR5. +- `previewResolver.svelte.ts` has an existing comment warning `bumpPreviewVersion()` must stay a pure write with no reactive read (avoids an `effect_update_depth` infinite loop) — any debounce must preserve this. Coalesce at the call site in `PreviewPanel.svelte:515-548` (rAF-coalesce-to-latest), not inside `previewResolver.svelte.ts` itself. +- Explicitly out of scope: `persistence.ts`'s separate `injectLivePreview` call path has no debounce either, but that's a different path — not fixed here (it's fixed independently in SLASHED-Plugins' PR-C3 for the frontend-overlay case). + +**Verify:** `tsc`/`svelte-check`/`test:unit`/`test:e2e`, plus **manual**: `npm run dev`, drag a slider rapidly, confirm smooth updates, no console errors about effect loops, and the settled value after release exactly matches the last dragged position (no dropped trailing frame). + +### PR7 — Configurator test coverage buildout (SL-023) +Last — after PR4, PR5, PR6. +- Tooling already present: `vitest`, `@testing-library/svelte`, `jest-dom`, `jsdom` are already in `configurator/package.json` devDependencies; a `test:components` script already exists (`configurator/tests-components/header.test.js` is the only current example — follow its pattern). +- Priority 1: `persistence.ts` — `loadInitialOverrides`/`saveOverrides` in both standalone (localStorage) and WP-embedded (REST) branches, plus the malformed-localStorage case (closes the loop on PR5's SL-019 guard). Follow `codec.test.js`/`css.test.js`'s existing structure. +- Priority 2: `previewResolver.svelte.ts` resolution/caching, including confirming PR6's debounce still invalidates `resolveCache` correctly without looping. +- Priority 3: component tests for `SliderRow`/`TokenRow` (bind → override-set → reset) via `@testing-library/svelte`; query by role/label, not CSS class. +- A save-error-state test closing PR5's SL-018 loop. + +**Verify:** `npm run test:unit && npm run test:components && npm run test && npx tsc --noEmit && npx svelte-check ...`. + +--- + +## SLASHED-Plugins + +### Wave 0 — ship immediately + +**PR-B1 — Fix color-model `-dark` filter drift + regression tests (PL-013, 014, 015)** +- `SLASHED-for-WP/integrations/bricks/editor-app/src/lib/color-model.js` (~line 140): change `if (suffix === 'light') return null;` to also check `'dark'`, matching Gutenberg's already-correct JS. Also port Gutenberg's `Array.isArray` guards (lines 377/380) into Bricks' `filterModel`, which is missing them. +- `SLASHED-for-WP/integrations/gutenberg/includes/class-presets.php` (~line 337): same fix, `'light' === $suffix` → also check `'dark'`. +- PL-015: add a cross-implementation regression test. Since PHP can't be imported into `node:test` directly and PR-A3 (PHPUnit) hasn't landed yet, shell out via `execSync('php -r ...')` against a shared fixture set for now; leave a comment noting eventual migration into the PHPUnit suite once PR-A3 exists. + +**Verify:** `node --test tests/color-model.test.js tests/*.test.js && composer phpstan && vendor/bin/phpcs ... && npm test`. + +**PR-C1 — Fix broken CI script + related doc drift (PL-025, 033, 036, 037)** +This is **not** a simple redirect. `scripts/verify-sync.js` (the closest existing script) only checks version-metadata consistency — a different concern from what README.md:74/CONTRIBUTING.md:34 claim `npm run check` does ("admin-app drift + cheatsheet coverage"). Git history confirms `check` never existed in this repo. +- Author a genuinely new `scripts/check.js` that runs, in sequence: `gen-class-hints.js --check` and `gen-variables-hints.js --check` (both already support a `--check` flag, confirmed), plus a new `--check`/`--dry-run` mode added to `admin-app/scripts/sync-core.mjs` that performs the same `.syncignore`-aware file resolution but diffs/hashes against the current `admin-app/src/` instead of writing, exiting non-zero on drift. +- Add `"check": "node scripts/check.js"` to root `package.json` — `framework-sync.yml:50` already calls it correctly, no workflow change needed once the script exists. +- **PL-036**: fix CLAUDE.md's `npm run sync` references to `npm --prefix SLASHED-for-WP/admin-app run sync` (the script only exists there, not at root). +- **PL-033**: trim `.syncignore`'s stale PR-#443 historical framing, same commit as PL-036. +- **PL-037**: add `docs/`, `tests/`, `SECURITY.md`, `CONTRIBUTING.md`, `phpstan.neon.dist`/`phpcs.xml.dist` to CLAUDE.md's structure diagram. + +**Verify:** `node scripts/check.js && SLASHED_FRAMEWORK_DIR=/home/user/SLASHED npm run check && npm test && npm run verify && npm run lint`. **Critical manual step**: hand-edit one non-`.syncignore`'d vendored file to introduce throwaway drift and confirm the new check catches it (exits non-zero) — this is the single most important verification here. Also confirm the `--check` mode of `sync-core.mjs` never writes to disk. + +**PR-C3 — Plugin-specific frontend hardening (PL-029, 030, 031)** +Both files are the only two `.syncignore` entries — zero interaction with the sync wave. +- **PL-029**: read `class-frontend-configurator.php:83-91`'s actual `esc_url_raw()` usage first, then add a same-origin/scheme check before `plugin-main.ts:36-51` injects `` from `window.slashedApp?.cssUrl` — don't over-restrict against a legitimate case the server already allows. +- **PL-030**: add a `.d.ts` ambient declaration for `window.slashedApp`, sourced field-for-field from the real PHP `wp_localize_script` payload. +- **PL-031**: `AppOverlay.svelte:106-113`'s `$effect` calling `injectLivePreview` on the live frontend has no debounce and is **not** covered by SLASHED's PR6 (different call path — `persistence.ts`'s `injectLivePreview`, not `PreviewPanel.svelte`'s). Add a local rAF-coalesce directly in this PR. + +**Verify:** `npx tsc --noEmit && npx svelte-check ... && npm run build:admin-app && npm test`, plus manual check on a live/local WP frontend: rapid override edits should produce exactly one eventual update per edit, no jank, no dropped edits. + +**PR-C4 — Wire `playwright-admin.js` into CI (PL-034)** +- Read the file first to confirm what it actually needs (likely `@playwright/test` + a built/running admin instance) before wiring it into `npm test`'s `node --test tests/*.test.js` glob (would break if forced in). Rename to `playwright-admin.test.js` and add a dedicated script/CI step if the infra is available; if it needs infra not present here or in CI, rename + document + leave a CI TODO rather than force a false-green wiring. + +**Verify:** `head -30 tests/playwright-admin.js` to confirm framework/deps, then attempt `npx playwright test tests/playwright-admin.test.js`; `npm test` must still pass unaffected. + +--- + +### Wave 1 — sync (after SLASHED PR4, PR5, PR6 merge — not PR7, since its new tests aren't vendored) + +**PR-SYNC — Sync vendored framework fixes** +1. `SLASHED_CONFIGURATOR_SRC=/path/to/updated/SLASHED/configurator/src npm --prefix SLASHED-for-WP/admin-app run sync`. +2. Confirm `.syncignore` respected (`git diff` shows zero changes to `plugin-main.ts`/`AppOverlay.svelte`). +3. **Manual cross-check required** (these two files consume vendored exports without being overwritten): confirm PR4's codec rename kept its export surface identical (tsc should show nothing); confirm `AppOverlay.svelte`/`plugin-main.ts` handle PR5's new `saveState: 'error'` gracefully if they branch on it; confirm PR6's debounce plus PR-C3's separate local debounce don't compound into excessive lag (manual check on live frontend). +4. Commit the vendored diff as its own commit referencing the upstream SLASHED commit/PR, matching this repo's existing "chore: sync framework..." convention. +5. Run PR-C1's new `npm run check` drift sub-check immediately after, to confirm the sync fully completed. + +**Verify:** `git status`/`git diff --stat` on `admin-app/src/`, `npm run build:apps`, `npm run check`, `npm test`, `npm run verify`, `cd SLASHED-for-WP/admin-app && npx tsc --noEmit`, plus manual: force a save failure (confirm new error UI) and rapid token edits on both admin UI and frontend overlay. + +--- + +### Wave 2 — Plugins-only, independent of sync + +**PR-A1 — PHP backend hardening (PL-002, 004, 005, 007, 008, 010, 012)** +- **PL-002**: return `WP_Error`/400 at `class-rest-controller.php:296-299` when *all* submitted overrides are rejected — this is the server-side half of SL-018's client-side error-state fix. +- **PL-004/005**: add `rest_sanitize_boolean` callbacks and `additionalProperties` shape constraints to the REST route schemas at `class-rest-controller.php:60-114` and `:107-110`, mirroring the existing `overrides` route's pattern. +- **PL-007**: wrap `build_family_scales()` in a transient keyed by the overrides hash, mirroring the existing CSS-parsing cache's transient pattern. +- **PL-008**: add `WP_DEBUG`-gated `error_log()` at `class-css-loader.php:57-58` and `class-inventory.php:744-761`. +- **PL-010**: extract the `html_font_size` allowlist to a `Slashed_Token_Store::ALLOWED_HTML_FONT_SIZES` constant, mirroring the existing `ALLOWED_CSS_BUNDLES` pattern; update both call sites. +- **PL-012**: leave `flush()` as intentional public API, note it as PR-A3's future test target. + +**Verify:** `composer phpstan && vendor/bin/phpcs && php -l && npm test`. PL-002 is the riskiest change here (a previously-silent-success path now returns 400) — double check no other caller relies on the old behavior. + +**PR-A2 — PHP backend architecture cleanup, low priority (PL-001, 003, 011)** +- **PL-001**: add internal section-comment banners to `class-color-resolver.php` (876 lines) instead of splitting it — audit's own recommendation is conditional on future growth. +- **PL-003**: type only methods actually touched by this PR and PR-A1 — not a big-bang pass across ~145 methods. Re-run `composer phpstan` after each newly-typed method, since new types can surface real bugs phpstan couldn't previously see. +- **PL-011**: one-line docblock cross-reference at `class-rest-controller.php:296`, no rename/extract. + +**Verify:** `composer phpstan && vendor/bin/phpcs && php -l ... && npm test`. + +**PR-A3 — PHPUnit test suite (PL-006, 035)** +Confirmed: no MySQL in this environment, no PHPUnit in `composer.json` today, PHP 8.4 CLI available. **Use Brain Monkey** (mocks WP core functions, no DB/bootstrap needed) rather than the official WP core test suite — matches the audit's priority targets, which are pure-logic-plus-WP-function-call surfaces. +- Add `phpunit/phpunit:^9` (PHP 7.4-compatible; v10+ drops 7.4) and `brain/monkey:^2` to `composer.json` `require-dev`. +- Add `phpunit.xml.dist` at repo root (alongside `phpstan.neon.dist`/`phpcs.xml.dist`), `tests/php/bootstrap.php` + a base TestCase wrapping `Brain\Monkey\setUp()`/`tearDown()`. +- Priority targets: `validate_override_value()`, `sanitize_rebemer_element_map()`, `Slashed_Token_Store` round-trips, and PL-035's `POST /wp-json/slashed/v1/tokens/validate` handler specifically (roadmap.md claims it's "shipped" with zero test evidence). +- Add a `composer test`/`vendor/bin/phpunit` entry point, kept separate from `npm test` (PHP and Node toolchains stay independently invoked). Add a CI step reusing `framework-sync.yml`'s existing `shivammathur/setup-php` pattern. +- Document Brain Monkey's mock-based scope/limits (no real DB/capability integration) in a short `tests/php/README.md` so future contributors know what the suite does and doesn't cover. + +**Verify:** `composer require --dev ...`, then `composer phpstan` (check whether `tests/` needs excluding from analysed paths), `vendor/bin/phpcs` (may need a tests-directory exclusion), `vendor/bin/phpunit`, `php -l` on all new test files. + +**PR-B2 — Integrations hardening + Gutenberg README (PL-017, 018, 019, 020, 021, 022)** +- **PL-017**: write `integrations/gutenberg/README.md` using Bricks' README as a template, adapted for Gutenberg's actually-different architecture (no REST routes, `wp_localize_script`/`theme.json`). +- **PL-018/019**: add `console.warn` on the identified silent-failure branches (`bricks-api.js:294-307`, `apply.js:112-118`, `panel.js:464-470`) without changing existing return/catch behavior. +- **PL-020**: comment only — no memoization until profiling shows it matters (per audit). +- **PL-021**: read what `class-rebemer-rest.php` (accepts `bricks_full_access` OR `manage_options`) and `class-fonts-rest.php` (requires `manage_options` only) actually gate before deciding which direction to align — this is a real access-control change, not cosmetic. Extract a shared `Slashed_Bricks_REST_Base::check_permissions()` regardless of the chosen direction. +- **PL-022**: extend `verify-sync.js`'s existing `inventory.json`-comparison pattern to also warn (not hard-fail) on `svelte`/`vite`/`svelte-check`/`typescript` version drift between `integrations/bricks/editor-app/package.json` and `admin-app/package.json`. + +**Verify:** `npm run lint:php && composer phpstan && vendor/bin/phpcs && npm --prefix SLASHED-for-WP/integrations/bricks/editor-app run build && npm test && npm run verify`. Manual: test both REST routes with a user having `bricks_full_access` but not `manage_options` (and vice versa) to confirm PL-021's chosen behavior matches intent. + +**PR-B3 — Bricks `apply.js` test coverage (PL-016)** +- Verify whether `applyToSubtree` performs live DOM mutations directly or only computes a plan before assuming full unit-testability — prioritize `buildPlan()`'s plan-generation logic (self-documented pure, highest complexity/blast-radius) regardless. Build representative Bricks element-tree fixtures, following `tests/color-model.test.js`'s `node:test` style. Keep any newly-exported helpers additive — don't restructure the module boundary as a side effect. + +**Verify:** `node --test tests/apply.test.js && npm test && npm --prefix SLASHED-for-WP/integrations/bricks/editor-app run build`. + +**PR-C2 — Vendoring/sync mechanism hardening (PL-024, 026, 027, 028, 032)** +Land after PR-C1 (both touch `sync-core.mjs` — avoid parallel edits to the same file; builds on PR-C1's new `--check` flag). +- **PL-024**: align `sync-configurator-core.yml`'s direct-push-to-main with `framework-sync.yml`'s PR-based model — re-read the workflow's own comments for deliberate rationale first; test via `workflow_dispatch` on a fork before trusting in production (a misconfigured PR step would silently stop the sync entirely, worse than today). +- **PL-026**: add `node:test` unit tests for `sync-core.mjs`'s file-selection logic, explicitly including a deliberately malicious `../`-containing input against the path-traversal/TOCTOU guards at lines 87-109/304-320. +- **PL-027**: differentiate 404 (file gone) vs 403 (rate-limited) log messages — logging change only, keep the existing fallback behavior. +- **PL-028**: replace rm-then-copy-in-place with copy-to-temp-dir-then-atomic-`renameSync`-swap — the one real correctness fix in this PR. Test explicitly: simulate a mid-copy failure and confirm `src/` is left in its original pre-sync state. +- **PL-032**: align `sync-core.mjs`'s import style with root scripts, folded in as a final mechanical pass. + +**Verify:** `node --test SLASHED-for-WP/admin-app/scripts/*.test.js`, `sync-core.mjs --check`, a full real sync on a throwaway branch, `npm test`, `npm run verify`. + +**PR-C5 — Build-artifact policy (PL-023)** +Implement only the near-term fix per explicit scoping — do **not** implement a release-time-build architectural change (that's a separate maintainer decision, restate it verbatim in the PR description as deferred). +- Create `.gitattributes` marking `SLASHED-for-WP/assets/admin-app/app.js`/`app.css` (and optionally `SLASHED-for-WP/dist/*.css`, same rationale) with `linguist-generated=true`, with a comment matching `.gitignore:11-13`'s existing tone. + +**Verify:** `git check-attr linguist-generated -- `, `npm run build:apps` (must be completely unaffected — diff-display-only change), `npm test`. + +--- + +## Deferred — not auto-fixed + +**PL-009** (`SLASHED-for-WP/readme.txt`'s `Tested up to: 7.0`) — internally consistent, not provably wrong, so no PR touches it. Resolve by asking the maintainer directly: *"Is `Tested up to: 7.0` accurate, or should it read the actual tested WP version?"* Once answered, fold the one-line fix into PR-C1's docs batch. + +Also unresolved without maintainer input, addressed with a placeholder mitigation only: **SL-001**'s duplicated dark-mode formula (cross-link comment added, not unified) pending confirmation of whether the duplication is intentional. + +## Overall rollout verification + +After each wave, in addition to the per-PR checks above: +- SLASHED: `npm run check:version` and `npm run check:llm-guide` before any commit that touched `configurator/package.json`, `core/*.css`, `optional/*.css`, or `token-registry.json` (CLAUDE.md CI gates). +- SLASHED-Plugins: `composer phpstan`, `vendor/bin/phpcs`, `npm test`, `npm run verify` after every PR touching PHP or the sync tooling. +- Before merging PR-SYNC specifically: full manual pass through the WP admin configurator UI and the frontend overlay (save/load, error state, rapid slider drags) since this is the one PR that crosses the repo boundary and the two `.syncignore`'d files can't be verified by the sync script alone.