feat(web): VS Code-style custom theme system - #2530
Conversation
The command-palette folder browser previously hid both kinds of directory references because `readdir` returns them with `lstat` semantics, leaving users unable to pick common locations like `~/Dropbox` (a symlink) or any Finder alias they had dragged into their home directory. - Follow symbolic links and show them when they point at a directory. - Detect modern bookmark-format (`book`) Finder aliases on macOS and resolve them in a single batched `osascript` call (30s timeout to cover the first-run TCC "control Finder" prompt). - Resolve an alias that appears in the middle of a typed path, so `~/my-alias/` lists the target directory. - Distinguish both kinds with `FolderSymlinkIcon`, use unique palette values for aliases that share a resolved target, and route clicks through `browseToPath` so the resolved path is navigated to directly. - Bound probe concurrency, harden the Effect.promise bodies, and use a NUL-delimited osascript output format so filenames containing tabs or newlines parse correctly.
filterBrowseEntries stripped only the `browse:` prefix and compared
the remainder to entry.fullPath. For Finder aliases, whose value is
`browse:alias:${name}:${fullPath}`, the comparison never matched and
highlightedBrowseEntry stayed null, so the alias target directory was
never prefetched on highlight.
Introduce a buildBrowseItemValue / parseBrowseItemValue helper pair,
use them on both ends so the format can't drift, and match aliases on
both name and fullPath so two aliases sharing a resolved target still
resolve to the correct row. Adds regression coverage in the logic
test suite.
- WorkspaceEntries: stat prefilter in isMacOSBookmarkAlias rejects files outside the plausible bookmark size range (< 4 B or > 64 KB) before opening, so ~/Downloads-style directories don't incur one file-open per entry just to check the magic bytes. - CommandPalette.logic: drop parseBrowseItemValue; filterBrowseEntries now recomputes buildBrowseItemValue per entry and compares. This makes the round-trip robust to filenames that contain `:` (POSIX allows it), where a naive first-colon split would misread the name/fullPath boundary.
# Conflicts: # apps/web/src/components/CommandPalette.logic.test.ts # apps/web/src/components/CommandPalette.tsx
Add a theme registry with built-in palettes, custom theme storage,
import/export, and an inline editor.
- New themes/ module: registry, transport, builtin, types
- applyThemeToDocument materializes Tailwind --alpha(...) -> color-mix
on emit so runtime <style> overrides don't collapse to white/black
- duplicateTheme of the default theme returns an empty patch so tokens
fall back through resolveTokens against the live default
- Settings: new Appearance page with a theme list panel
- Per-row Edit/Duplicate/Export/Copy JSON/Delete actions
- Inline editor with Form/JSON modes, light/dark variant tabs,
sticky header (Save/Cancel + Unsaved badge), and AlertDialog confirms
- Color mode selector moved here from General settings
- index.css: extract scrollbar/noise styles to CSS custom properties
- useTheme: track activeThemeId, sync across tabs
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
github-actions
Bot
added
size:XXL
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 3 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 1470200. Configure here.
| return () => { | ||
| applyThemeToDocument(source); | ||
| }; | ||
| }, [source]); |
There was a problem hiding this comment.
Saved previews are reverted
Medium Severity
ThemeEditor always reapplies source on unmount. After onSave applies the saved draft and closes the editor, this cleanup runs and restores the old theme, so same-id edits do not visibly take effect until a later reload or theme switch.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 1470200. Configure here.
| } | ||
| deleteCustomTheme(pendingDeleteId); | ||
| setPendingDeleteId(null); | ||
| refresh(); |
There was a problem hiding this comment.
Deleted active theme stays applied
Medium Severity
Deleting the active custom theme only updates storage through deleteCustomTheme. The current document is never switched back with setActiveTheme or applyThemeToDocument, so the deleted palette remains visible until another theme action or reload.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 1470200. Configure here.
| return Object.entries(tokens) | ||
| .filter(([, value]) => typeof value === "string" && value.length > 0) | ||
| .map(([name, value]) => `--${name}: ${value};`) | ||
| .join(" "); |
There was a problem hiding this comment.
Theme tokens bypass allow-list
Medium Severity
tokensToCss emits every imported token key and value directly into the runtime <style> tag. Since isValidTheme only checks that token values are strings, a crafted theme can inject arbitrary CSS instead of being limited to THEME_TOKEN_NAMES.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 1470200. Configure here.
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
| useEffect(() => { | ||
| return () => { | ||
| applyThemeToDocument(source); | ||
| }; | ||
| }, [source]); |
There was a problem hiding this comment.
🔴 Critical settings/ThemeEditor.tsx:154
The cleanup effect on lines 154-158 runs applyThemeToDocument(source) when ThemeEditor unmounts. When a user saves, AppearanceSettingsPanel calls handleSave, applies the draft theme, sets editingId to null, and unmounts this component. The cleanup then fires and immediately reverts the page to the original source theme, overwriting the just-saved changes. Consider only reverting to source on unmount when there are actual unsaved changes (e.g., by checking a ref or prop that indicates save was not triggered).
- useEffect(() => {
- return () => {
- applyThemeToDocument(source);
- };
- }, [source]);Also found in 1 other location(s)
apps/web/src/components/settings/AppearanceSettings.tsx:119
When editing a custom theme, if the user clicks on a different theme row to select it (line 235 calls
setActiveTheme), that theme becomes active and its styles are applied. However, if the user then cancels editing (line 119),handleCancelappliesediting.source(the original theme being edited) to the document, resulting in a mismatch where the visual theme doesn't match the selected active theme. The document will display the cancelled theme's styles while the UI shows a different theme as selected.
🤖 Copy this AI Prompt to have your agent fix this:
In file apps/web/src/components/settings/ThemeEditor.tsx around lines 154-158:
The cleanup effect on lines 154-158 runs `applyThemeToDocument(source)` when `ThemeEditor` unmounts. When a user saves, `AppearanceSettingsPanel` calls `handleSave`, applies the draft theme, sets `editingId` to null, and unmounts this component. The cleanup then fires and immediately reverts the page to the original `source` theme, overwriting the just-saved changes. Consider only reverting to source on unmount when there are actual unsaved changes (e.g., by checking a ref or prop that indicates save was not triggered).
Evidence trail:
ThemeEditor cleanup: apps/web/src/components/settings/ThemeEditor.tsx lines 154-158 (unconditional applyThemeToDocument(source) on unmount).
handleSave: apps/web/src/components/settings/AppearanceSettings.tsx lines 105-116 (applies draft then sets editingId to null).
Conditional rendering causing unmount: apps/web/src/components/settings/AppearanceSettings.tsx line 342 ({isEditing && editing ? <ThemeEditor ... /> : null}).
useTheme effect only depends on theme (light/dark): apps/web/src/hooks/useTheme.ts line 221 (useEffect(() => { applyTheme(theme); }, [theme])).
setActiveTheme: apps/web/src/hooks/useTheme.ts lines 212-217 (does not add an effect keyed on activeThemeId to re-apply theme post-cleanup).
Also found in 1 other location(s):
- apps/web/src/components/settings/AppearanceSettings.tsx:119 -- When editing a custom theme, if the user clicks on a different theme row to select it (line 235 calls `setActiveTheme`), that theme becomes active and its styles are applied. However, if the user then cancels editing (line 119), `handleCancel` applies `editing.source` (the original theme being edited) to the document, resulting in a mismatch where the visual theme doesn't match the selected active theme. The document will display the cancelled theme's styles while the UI shows a different theme as selected.
| export function themeFilename(theme: Pick<ThemeDefinition, "id" | "name">): string { | ||
| const base = (theme.name || theme.id).trim().replace(FILENAME_SAFE, "-").replace(/^-+|-+$/g, ""); | ||
| const slug = base.length > 0 ? base : theme.id || "theme"; | ||
| return `${slug}${THEME_FILE_EXTENSION}`; | ||
| } |
There was a problem hiding this comment.
🟢 Low themes/transport.ts:16
When base is empty after sanitization, the fallback theme.id || "theme" is used directly without sanitization. If theme.id contains filename-unsafe characters (e.g., foo/bar), the resulting filename includes those characters. For example, a theme with id: "foo/bar" and name: "..." produces foo/bar.t3theme.json, which violates the intended filename-safety of the function.
- const base = (theme.name || theme.id).trim().replace(FILENAME_SAFE, "-").replace(/^-+|-+$/g, "");
- const slug = base.length > 0 ? base : theme.id || "theme";
+ const base = (theme.name || theme.id).trim().replace(FILENAME_SAFE, "-").replace(/^-+|-+$/g, "");
+ const slug = base.length > 0 ? base : (theme.id || "theme").replace(FILENAME_SAFE, "-").replace(/^-+|-+$/g, "");🤖 Copy this AI Prompt to have your agent fix this:
In file apps/web/src/themes/transport.ts around lines 16-20:
When `base` is empty after sanitization, the fallback `theme.id || "theme"` is used directly without sanitization. If `theme.id` contains filename-unsafe characters (e.g., `foo/bar`), the resulting filename includes those characters. For example, a theme with `id: "foo/bar"` and `name: "..."` produces `foo/bar.t3theme.json`, which violates the intended filename-safety of the function.
Evidence trail:
apps/web/src/themes/transport.ts lines 15-21 (REVIEWED_COMMIT): `FILENAME_SAFE` regex and `themeFilename` function showing unsanitized fallback path. apps/web/src/themes/registry.ts lines 212-220 (REVIEWED_COMMIT): `isValidTheme` only validates id is a non-empty string, no character restriction. apps/web/src/themes/transport.ts lines 107-113 (REVIEWED_COMMIT): `downloadTheme` calls `themeFilename` to set `anchor.download`.
ApprovabilityVerdict: Needs human review 1 blocking correctness issue found. This PR introduces a complete VS Code-style custom theme system with 3000+ lines of new code, new settings routes, theme editor, and import/export functionality. Multiple unresolved review comments identify bugs (saved previews reverted, deleted themes staying applied, token allow-list bypass) that should be addressed before merging. You can customize Macroscope's approvability policy. Learn more. |
1 participant
Summary
localStorage, JSON import/export, and an inline form/JSON editor.AlertDialogfor discard/delete confirmations.What's in the PR
apps/web/src/themes/—registry,transport,builtin,types,README(full schema docs)apps/web/src/components/settings/AppearanceSettings.tsx+ThemeEditor.tsx— list panel and editorapps/web/src/hooks/useTheme.ts— tracksactiveThemeId, syncs across tabsapps/web/src/index.css— scrollbar / noise styles extracted to CSS custom properties so themes can override themroutes/settings.appearance.tsx+ nav entryNotable bug fix
Tailwind v4's
--alpha(<color> / <pct>%)is a build-time function — emitting it into a runtime<style>tag would resolve to nothing and collapse borders to solid white/black. Two-layer fix:duplicateTheme(DEFAULT_THEME)returns an empty patch — tokens fall back throughresolveTokensagainst the live default.applyThemeToDocumentruns resolved tokens throughmaterializeTokensbefore emit, converting any--alpha(...)to runtime-safecolor-mix(in srgb, ..., transparent).Test plan
bun run test— 1060/1060 across 99 files)bun run typecheck) — only pre-existing unrelatedSidebar.tsxSortableContext error--primary→ Save → reload, change persistsNote
Medium Risk
Medium risk: introduces new theme persistence/serialization logic in
localStorageand changes filesystem browsing to resolve symlinks and macOS Finder aliases viaosascript, which could affect directory listing behavior and performance/permissions on macOS.Overview
Adds a token-based theme system for the web app: built-in palettes plus custom themes persisted in
localStorage, applied via an injected style tag (themes/registry.ts), with validation and runtime conversion of Tailwind--alpha(...)values tocolor-mix(...).Introduces a new Settings → Appearance page (new route + sidebar entry) that lets users switch palettes and color mode, edit themes inline via a form/JSON editor, and import/export/copy/delete themes (
themes/transport.ts,ThemeEditor.tsx,AppearanceSettings.tsx), and updatesuseThemeto track/syncactiveThemeIdacross tabs.Updates filesystem browsing end-to-end to better handle non-directory entry types: server
WorkspaceEntries.browsenow follows directory symlinks and macOS bookmark aliases (batchedosascriptresolution with concurrency caps), surfacesisSymlink/isAliasonFilesystemBrowseEntry, and the command palette browse UI adjusts navigation/prefetch and item identity to handle aliases and shared targets.Reviewed by Cursor Bugbot for commit 1470200. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Add VS Code-style custom theme system with Appearance settings panel
themes/builtin.tsand a registry inthemes/registry.tsthat injects a single<style>tag of CSS variables for:rootand:root.dark.AppearanceSettingsPanelat/settings/appearancewhere users can switch themes, duplicate built-ins, edit custom themes with live preview (form or raw JSON), copy/export/import theme JSON files, and delete custom themes with confirmation.ThemeEditorwith per-token color validation, per-variant (light/dark) editing, and guarded save/cancel flows.useThemeto exposeactiveThemeIdandsetActiveTheme, and to react to cross-tablocalStoragechanges for active theme and custom themes.WorkspaceEntries.tsnow detects Finder bookmark alias files via magic bytes, resolves their targets via AppleScript, and exposesisSymlink/isAliasflags and resolvedfullPathvalues; the command palette uses these to navigate aliases correctly.osascriptper browse request; broken or slow Finder processes could delay directory listings.📊 Macroscope summarized 1470200. 17 files reviewed, 5 issues evaluated, 1 issue filtered, 2 comments posted
🗂️ Filtered Issues
apps/web/src/components/settings/AppearanceSettings.tsx — 0 comments posted, 2 evaluated, 1 filtered
setActiveTheme), that theme becomes active and its styles are applied. However, if the user then cancels editing (line 119),handleCancelappliesediting.source(the original theme being edited) to the document, resulting in a mismatch where the visual theme doesn't match the selected active theme. The document will display the cancelled theme's styles while the UI shows a different theme as selected. [ Cross-file consolidated ]