fix(themes): sync custom theme selections portably by nisavid · Pull Request #9728 · warpdotdev/warp

nisavid · 2026-05-01T10:36:26Z

Summary

Store custom theme references under Warp's theme root as relative paths so Settings Sync does not publish machine-specific theme roots.
Resolve portable references and legacy macOS/Linux absolute theme-root paths against the local theme directory when reading synced settings.
Allow cloud sync for custom and Base16 theme selections only when every custom theme reference can be represented portably.

Fixes #9723.
Addresses the root cause behind #8988 and #6678.

Relationship to #9634

PR #9634 handles tilde expansion/contraction for home-relative paths. This PR is broader: it stores and resolves custom theme references relative to Warp's platform theme root, so the same selected theme can sync between macOS ~/.warp/themes and Linux XDG data theme roots.

Notes

Settings Sync still syncs only the custom theme reference. It does not upload, download, create, copy, or manage custom theme YAML files.

Test Plan

cargo test -p warp themes::theme::tests
cargo test -p warp settings::theme::tests
cargo fmt --check
cargo clippy -p warp --lib --tests -- -D warnings
env PATH=/tmp/codex-corepack-bin:/usr/local/sbin:/usr/local/bin:/usr/bin cargo clippy --workspace --all-targets --tests -- -D warnings
env PATH=/tmp/codex-corepack-bin:/usr/local/sbin:/usr/local/bin:/usr/bin cargo clippy --workspace --all-targets --all-features --tests -- -D warnings (blocked locally: --all-features enables release_bundle, but warp-channel-config is not installed on PATH)

Co-Authored-By: Oz [email protected]

CHANGELOG-BUG-FIX: Fixed custom theme selection sync for users who install the same custom themes on multiple machines or operating systems. Co-Authored-By: Oz <[email protected]> Co-authored-by: Codex <[email protected]>

Co-Authored-By: Oz <[email protected]> Co-authored-by: Codex <[email protected]>

cla-bot · 2026-05-01T10:36:30Z

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have the users @nisavid on file. In order for us to review and merge your code, each contributor must visit https://cla.warp.dev to read and agree to our CLA. Once you have done so, please comment @cla-bot check to trigger another check.

oz-for-oss · 2026-05-01T10:36:39Z

@nisavid

I'm starting a first review of this pull request.

You can view the conversation on Warp.

I completed the review and posted feedback on this pull request.

Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).

Powered by Oz

oz-for-oss

Overview

This PR makes custom theme selections portable by serializing theme-root paths relatively, resolving portable and legacy paths against the local theme root, and expanding syncability checks for custom/Base16 selections.

Concerns

Relative custom theme paths from synced settings are accepted without traversal validation, so .. components can resolve outside the intended theme root.

Security

The new relative-path resolution should reject or normalize traversal components before applying cloud/settings values to filesystem paths.

Verdict

Found: 0 critical, 1 important, 0 suggestions

Request changes

Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).

Powered by Oz

nisavid · 2026-05-01T10:52:37Z

@cla-bot check

cla-bot · 2026-05-01T10:52:42Z

The cla-bot has been summoned, and re-checked this pull request!

Co-Authored-By: Oz <[email protected]> Co-authored-by: Codex <[email protected]>

Co-authored-by: Codex <[email protected]>

nisavid · 2026-05-01T11:40:13Z

/oz-review

oz-for-oss · 2026-05-01T11:40:25Z

@nisavid

I'm re-reviewing this pull request in response to a review request.

You can view the conversation on Warp.

I reviewed this pull request and requested human review from: @lucieleblanc.

Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).

Powered by Oz

Oz no longer requests changes for this pull request after the latest automated review.

oz-for-oss

Overview

This PR stores custom theme selections as portable theme-root-relative references when possible, resolves those references back to the local theme directory when reading settings, and gates theme setting cloud sync on portable custom theme paths.

Concerns

No blocking correctness or security concerns found in the changed lines.

Verdict

Found: 0 critical, 0 important, 0 suggestions

Approve

Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).

Powered by Oz

rrroyal · 2026-05-01T14:48:31Z

+    relative_path_after_marker(&components, &[".warp", "themes"])
+        .or_else(|| relative_path_after_marker(&components, &["warp-terminal", "themes"]))


i'm pretty sure other warp channels use different paths, i.e. preview uses .warp-preview instead of .warp. does it also handle that case?

windows also uses different paths. maybe it would be better if it used platform-specific logic with conditional compilation instead of "hardcoding" paths here?

Good call. I agree the current suffix matching is the wrong middle ground.

A synced absolute theme path may have been written by a different platform, channel, user account, or data profile than the build now reading it. Current-platform conditional compilation can identify the current installation’s theme root, but it cannot reliably interpret every legacy absolute path that may arrive through sync.

I’m going to narrow this PR to the durable behavior:

safe relative paths resolve under the current themes_dir()

absolute paths under the current themes_dir() serialize as relative and are syncable

other absolute or foreign-looking paths are preserved and are not treated as syncable

That avoids an incomplete cross-platform/channel legacy migration. New saves get the portable representation. Existing foreign absolute selections keep their current behavior until the user reselects the theme and saves the portable form.

Co-Authored-By: Oz <[email protected]> Co-authored-by: Codex <[email protected]>

lucieleblanc

Thank you for the PR! Requesting changes because there's some Windows details we should iron out before this merges.

My main concern is cross-platform path separators. The changes + tests here assumes the filesystem accepts backslashes \ as a path separator.

For example: On Windows, a theme at C:\Users\…\themes\catppuccin\mocha.yml would be stripped to catppuccin\mocha.yml. When deserializing on a Unix system, PathBuf::from("catppuccin\\mocha.yml") reads the backslash as part of the filename, not a separator. The path becomes a single component, and theme_root.join(…) produces something like /home/user/.local/share/warp-terminal/themes/catppuccin\mocha.yml, which seems wrong.

I think the current setup happens to work when using Unix paths on Windows, because Windows accepts / as a separator. Unix systems don't accept \ as a separator though.

Maybe normalizing paths to use / during serialization, then deserializing into the native separator could work?

lucieleblanc · 2026-05-04T21:34:47Z

+#[test]
+fn custom_theme_absolute_parent_dir_path_under_theme_root_is_preserved() {
+    let root = PathBuf::from("/Users/example/.warp/themes");
+    let stored = root.join("../outside.yml");
+
+    assert_eq!(custom_theme_path_from_storage(&stored, &root), stored);
+}


This test seems like a duplicate of custom_theme_relative_parent_dir_path_is_preserved above?

Cleaned up the helper-level test names, made the absolute traversal case distinct, and added raw .. settings/serde ingress coverage.

lucieleblanc · 2026-05-04T21:37:45Z

+#[test]
+fn theme_kind_syncs_custom_theme_under_theme_root() {
+    let setting = Theme::new(Some(custom(
+        crate::user_config::themes_dir().join("custom.yml"),


Style nit: please import user_config at the top of the file, instead of repeatedly qualifying inline in each test. Same goes in the other test file.

lucieleblanc · 2026-05-04T22:01:20Z

All of the tests in this file use hard-coded UNIX-style roots, so I'm not convinced this works cross-platform.

Could we add some #[cfg(windows)] tests that use a Windows theme root (for example, C:\Users\example\AppData\Roaming\warp\Warp\data\themes) as well?

Added #[cfg(windows)] coverage for a Windows theme root, slash-separated serialization/deserialization, syncability, and raw backslash/drive-prefix rejection.

Validate synced custom theme path strings before platform path parsing and serialize current theme-root paths with slash-separated portable components. Co-authored-by: Codex <[email protected]>

Add raw traversal inputs to custom theme settings ingress tests and rename helper-level tests so their names match the storage-helper layer they exercise. Co-authored-by: Codex <[email protected]>

nisavid · 2026-05-05T11:44:03Z

@lucieleblanc I pushed follow-ups addressing the review feedback.

The custom theme path storage now normalizes portable paths to /, validates raw stored strings before PathBuf parsing, and preserves/non-syncs ambiguous values like raw backslashes, .., and Windows drive-prefixed paths. I also added Windows-gated coverage, serde/SettingsValue ingress tests, and cleaned up the helper-level test names.

Could you take another look when you have a chance?

nisavid and others added 3 commits May 1, 2026 06:24

fix(themes): serialize custom theme references portably

5ad358b

CHANGELOG-BUG-FIX: Fixed custom theme selection sync for users who install the same custom themes on multiple machines or operating systems. Co-Authored-By: Oz <[email protected]> Co-authored-by: Codex <[email protected]>

fix(settings): sync portable custom theme selections

e376eae

CHANGELOG-BUG-FIX: Fixed custom theme selection sync for users who install the same custom themes on multiple machines or operating systems. Co-Authored-By: Oz <[email protected]> Co-authored-by: Codex <[email protected]>

style(theme): format portable custom theme sync changes

f999466

Co-Authored-By: Oz <[email protected]> Co-authored-by: Codex <[email protected]>

github-actions Bot added the external-contributor Indicates that a PR has been opened by someone outside the Warp team. label May 1, 2026

oz-for-oss Bot previously requested changes May 1, 2026

View reviewed changes

Comment thread app/src/themes/theme.rs Outdated

cla-bot Bot added the cla-signed label May 1, 2026

nisavid and others added 2 commits May 1, 2026 07:01

fix(themes): reject unsafe synced theme paths

f64a597

Co-Authored-By: Oz <[email protected]> Co-authored-by: Codex <[email protected]>

test(themes): cover unsafe theme root paths

519e36c

Co-authored-by: Codex <[email protected]>

oz-for-oss Bot reviewed May 1, 2026

View reviewed changes

oz-for-oss Bot requested a review from lucieleblanc May 1, 2026 11:43

rrroyal reviewed May 1, 2026

View reviewed changes

fix(themes): preserve foreign custom theme paths

e5d7a87

Co-Authored-By: Oz <[email protected]> Co-authored-by: Codex <[email protected]>

nisavid force-pushed the codex/portable-custom-theme-sync branch from 94f2f2e to e5d7a87 Compare May 1, 2026 16:17

lucieleblanc requested changes May 4, 2026

View reviewed changes

lucieleblanc mentioned this pull request May 4, 2026

fix(themes): Handle tilde in theme paths #9634

Open

2 tasks

nisavid and others added 2 commits May 5, 2026 06:10

fix(themes): normalize portable theme paths

668ce8b

Validate synced custom theme path strings before platform path parsing and serialize current theme-root paths with slash-separated portable components. Co-authored-by: Codex <[email protected]>

test(themes): cover raw traversal settings paths

88cbb1c

Add raw traversal inputs to custom theme settings ingress tests and rename helper-level tests so their names match the storage-helper layer they exercise. Co-authored-by: Codex <[email protected]>

		relative_path_after_marker(&components, &[".warp", "themes"])
		.or_else(\|\| relative_path_after_marker(&components, &["warp-terminal", "themes"]))

Conversation

nisavid commented May 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Relationship to #9634

Notes

Test Plan

Uh oh!

cla-bot Bot commented May 1, 2026

Uh oh!

oz-for-oss Bot commented May 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

oz-for-oss Bot left a comment

Choose a reason for hiding this comment

Overview

Concerns

Security

Verdict

Uh oh!

Uh oh!

nisavid commented May 1, 2026

Uh oh!

cla-bot Bot commented May 1, 2026

Uh oh!

nisavid commented May 1, 2026

Uh oh!

oz-for-oss Bot commented May 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

oz-for-oss Bot left a comment

Choose a reason for hiding this comment

Overview

Concerns

Verdict

Uh oh!

rrroyal May 1, 2026

Choose a reason for hiding this comment

Uh oh!

nisavid May 1, 2026

Choose a reason for hiding this comment

Uh oh!

lucieleblanc left a comment

Choose a reason for hiding this comment

Uh oh!

lucieleblanc May 4, 2026

Choose a reason for hiding this comment

Uh oh!

nisavid May 5, 2026

Choose a reason for hiding this comment

Uh oh!

lucieleblanc May 4, 2026

Choose a reason for hiding this comment

Uh oh!

lucieleblanc May 4, 2026

Choose a reason for hiding this comment

Uh oh!

nisavid May 5, 2026

Choose a reason for hiding this comment

Uh oh!

nisavid commented May 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

nisavid commented May 1, 2026 •

edited

Loading

oz-for-oss Bot commented May 1, 2026 •

edited

Loading

oz-for-oss Bot commented May 1, 2026 •

edited

Loading