4 Commits (8ffdca363642be5f40bbefad8a2b9dbb41786c8c)

Author	SHA1	Message	Date
BoulderBadgeDad	ffb9249ded	Fix: mirrored playlist action buttons dead when name has an apostrophe ... A mirrored playlist named with an apostrophe (e.g. "Road trip-The Rolfe's") rendered dead action buttons. _escAttr HTML-escapes ' to &#39;, but it was used to inject the name into a single-quoted JS string inside an inline onclick. The HTML parser decodes &#39; back to a bare ' BEFORE the JS parser runs, producing an unterminated string literal -> SyntaxError -> the whole handler fails to compile. Two symptoms (both reproduced with the real name + the literal line-524 onclick template): clicking the X delete never ran event.stopPropagation(), so the click bubbled to the card and opened the track preview instead; and the preview's "Delete Mirror" silently did nothing (no DELETE request, no log). Plain names ("Classic Rock") were unaffected, which is why it looked intermittent. Add a dedicated _escJs() that backslash-escapes the JS metacharacters (\, ') first, then HTML-escapes the attribute-breaking chars - correct for a single-quoted JS string inside a double-quoted HTML attribute. Convert all 16 inline-onclick string-argument sites to it: mirrored card (clear/Auto-Sync/ link/delete) and preview modal, plus the same latent bug in pool Fix Match / Rematch, group bulk-toggle/rename/delete, and automation history/group/delete. Genuine HTML-attribute usages (class/value/data-*/title/option) stay on _escAttr where it is correct. Tests: tests/static/test_stats_automations_esc.mjs extracts the real _escJs/ _escAttr from source and asserts apostrophe + quote/backslash/&/<> names round-trip through HTML+JS decoding, documents that _escAttr throws a SyntaxError for the apostrophe case while _escJs compiles clean, and pins wolf39's exact name. pytest shim tests/test_stats_automations_esc_js.py runs it under node --test (skips if node<22 / absent).	4 weeks ago
Broque Thomas	698c21c3ce	Auto-Sync Weekly Board: weekday schedules in the UI (PR 3/4) ... PR 3 of the schedule-types feature — see ``memory/project_auto_sync_schedule_types.md``. Backend ``next_run_at`` + ``weekly_time`` trigger handler landed in PRs 1-2. This PR exposes them in the Auto-Sync manager so users can finally schedule playlists by day-of-week + time instead of only hourly intervals. **UI layout:** The Auto-Sync modal grows a ``Weekly Board`` tab between ``Hourly Board`` (renamed from ``Schedule Board``) and ``Automation Pipelines``. Same sidebar (mirrored playlists grouped by source, with filter). Main panel is 7 day columns Mon-Sun instead of 10 hour buckets. Drag a playlist onto a day column → creates a single-day weekly schedule at the default time (09:00 in the browser's IANA tz from ``Intl.DateTimeFormat().resolvedOptions().timeZone``). Click any scheduled card → opens an editor popover for time, multi-day toggles, tz override, and unschedule. Multi-day schedules render under every matching column (Mon-Wed-Fri schedule appears as three cards, one per column) — matches how users think about "this playlist runs on Mon AND Wed AND Fri". **Mutual exclusion:** one schedule per playlist. The save path on either tab deletes any existing schedule of the OTHER kind before installing the new one. Backend can technically run both as two separate automation rows, but two cards under the same playlist would surprise users and the engine has no merge semantic for "daily-and-hourly". **Pure-function helpers** (testable via node:test, matching the existing ``tests/static/test_auto_sync.mjs`` pattern): - ``detectBrowserTimezone()`` — Intl tz with UTC fallback for browsers where Intl is absent. - ``autoSyncWeeklyTrigger({time, days, tz})`` — defensive payload builder: garbage time → 09:00, unrecognised days dropped, missing tz → browser tz. - ``autoSyncWeeklyFromTrigger(config)`` — inverse parser with the same defensive shape. Empty days expands to every weekday (matches ``next_run_at`` engine semantic). Returns null for non-object configs so ``buildAutoSyncScheduleState`` can route broken rows to automationPipelines instead of silently bucketing them as every-day weekly. - ``autoSyncWeeklyLabel(parsed)`` — sorted "Mon, Wed, Fri @ 09:00" / collapses to "Daily @ HH:MM" for full-week / "Unscheduled" for null. Canonical Mon-Sun ordering regardless of input order. **Tests:** 26 new node:test cases across ``detectBrowserTimezone`` x1, ``autoSyncWeeklyTrigger`` x6, ``autoSyncWeeklyFromTrigger`` x6, ``autoSyncWeeklyLabel`` x5, and ``buildAutoSyncScheduleState`` weekly bucketing x5 (covering owned weekly_time → weeklySchedules, hourly stays in playlistSchedules, non-owned falls through to automationPipelines, legacy-named auto-sync rows still recognised, garbage trigger_config falls through). All 62 node:test cases pass; 261 across the automation pytest suite still green (zero regression on PRs 1-2's plumbing). Python wrapper at ``tests/test_auto_sync_js.py`` shells out cleanly. **CSS** (themed to the existing Auto-Sync gradient + accent variables): - 7-column grid for the weekly board, narrower than the 10 hour-bucket layout. - Editor popover with backdrop-blur, accent-tinted save / delete buttons, hover states that pick up the user's accent color. - ``scheduled-elsewhere`` state for playlists with an hourly schedule visible on the weekly board (dashed border + opacity) so the user knows a drop will replace, not stack. **WHATS_NEW entry** under 2.6.3 unreleased — first user-visible slice of the schedule-types feature. PR 4 (Monthly UI tab) deferred until weekly proves wanted.	4 weeks ago
Broque Thomas	a65ba7e6a3	Add node:test contract for auto-sync.js helpers ... Cin review: no JS tests covered the autoSync* helpers, so the timezone fix shipped without a regression test in the layer where the bug actually lived. New `tests/static/test_auto_sync.mjs` runs under `node --test` (built-in runner, no extra deps) and pins: - `autoSyncTriggerForHours` / `autoSyncHoursFromTrigger` round-trip for 1h, 4h, 12h, 24h, 48h, 168h. Catches off-by-one in the day vs hour conversion that backs the schedule board's drag-drop. - `autoSyncBucketLabel` / `autoSyncIntervalLabel` formatting + pluralization. - `autoSyncSourceLabel` known + unknown + falsy. - Predicates (`autoSyncCanSchedulePlaylist`, `autoSyncIsPipelineAutomation`, `autoSyncPlaylistIdFromAutomation`, `autoSyncIsScheduleOwned`) including the `owned_by`-flag / legacy-name-prefix split from the previous commit. - `buildAutoSyncScheduleState` partitions board-owned schedules from custom pipelines correctly. - `autoSyncNextRunLabel` parses the naive UTC timestamp as UTC, not local — exactly the regression that took an hour to diagnose this session. Includes a past-time check ("due now") and a multi-day case. - `getMirroredSourceRef` source_ref/description-URL/source_playlist_id resolution order. Cross-realm note: vm-sandbox return values fail `deepStrictEqual` against host-realm objects even when shape matches, so a small `deepShapeEqual` helper round-trips through JSON for structural comparison. The `_autoParseUTC` stub mirrors the real implementation in stats-automations.js so the timezone test exercises both files end to end. `tests/test_auto_sync_js.py` is the pytest shim — shells out to `node --test` and surfaces failures inline. Skips cleanly when node isn't on PATH or is older than 22, matching the existing discover-section-controller test pattern. Also updated SPLIT_MODULES in tests/test_script_split_integrity.py to include the new auto-sync.js — the onclick-coverage check was failing because `openAutoSyncScheduleModal` (referenced from index.html via the Sync page button) now lives in a module the integrity scanner wasn't searching. 39 new JS test cases, all green via `node --test` and via the pytest wrapper.	1 month ago
Broque Thomas	c557d9196e	Discover controller — Cin pre-review polish ... Three changes tightening the controller before opening the PR. DROP MAGIC `extractItems` DEFAULTS Controller used to auto-pull `data.items` / `data.albums` / `data.artists` / `data.tracks` / `data.results` when no extractor was supplied. Removed the fallback chain — every section now MUST provide an explicit `extractItems(data) => array`. Validated at register-time so misuse fails immediately, not silently on first load against an endpoint that happened to return two arrays. Cin standard: explicit > implicit. Magic key-grabbing could pick the wrong one in edge cases (e.g. an endpoint returning both `data.albums` and `data.results` would have grabbed albums when the section actually wanted results). All 10 existing controller call sites already passed explicit extractors, so no migration churn — this is purely tightening the contract for future sections. REPLACE `renderItems` NULL-RETURN CONVENTION WITH `manualDom: true` Your Albums and similar sections that delegate to existing renderers that target a CHILD element of `contentEl` used to signal "leave the container alone" by returning null/undefined from `renderItems`. That convention is easy to confuse with an accidental missing-return error. Replaced with an explicit `manualDom: true` config flag. Renderer is still called for its side-effects, controller just skips the innerHTML swap. Clearer intent at the call site. Updated `loadYourAlbums` to use the new flag. PIN THE CONTROLLER CONTRACT WITH JS TESTS Added `tests/static/test_discover_section_controller.mjs` — 32 tests covering the controller's lifecycle contract: - Config validation (every required field, mutual exclusivity of fetchUrl/data, type checks on contentEl) - Happy-path fetch → parse → render - Empty state (default empty render, hideWhenEmpty + sectionEl, success=false treated as empty, custom isSuccess override) - Stale state (fires when isStale returns true, wins over empty, custom renderStale override) - Error state (HTTP non-ok, fetch throws, showErrorToast fires window.showToast, default off doesn't fire) - No-fetch `data:` mode (value + function form, doesn't call fetch) - manualDom mode (skips innerHTML swap, still calls renderer) - Callable `fetchUrl` (resolved at load time, refresh re-resolves) - Load coalescing (concurrent loads share one fetch) - Refresh bypasses coalescing (re-fires fetch every call) - Hook error containment (throwing renderer/onSuccess hooks don't crash the controller) Runs via Node's stable built-in `--test` runner — no package.json, no jest/vitest dependency, no compile step. Just `node --test`. Pytest wrapper at `tests/test_discover_section_controller_js.py` shells out to node and asserts clean exit, so the JS tests fail the regular pytest sweep if the controller contract drifts. Skipped gracefully when node isn't available or is < 22. Closes the "controller is a contract, pin it at the test boundary" gap that Cin would have flagged on review. VERIFICATION - 2205/2205 full pytest suite green (was 2204 + 1 new wrapper) - 32/32 `node --test` pass on the controller test file directly - ruff clean - node --check clean on all touched JS files	2 months ago