fix(themes): sync custom theme selections portably#9728
fix(themes): sync custom theme selections portably#9728nisavid wants to merge 6 commits intowarpdotdev:masterfrom
Conversation
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 <oz-agent@warp.dev> Co-authored-by: Codex <noreply@openai.com>
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 <oz-agent@warp.dev> Co-authored-by: Codex <noreply@openai.com>
Co-Authored-By: Oz <oz-agent@warp.dev> Co-authored-by: Codex <noreply@openai.com>
|
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 |
|
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 Powered by Oz |
There was a problem hiding this comment.
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
|
@cla-bot check |
|
The cla-bot has been summoned, and re-checked this pull request! |
Co-Authored-By: Oz <oz-agent@warp.dev> Co-authored-by: Codex <noreply@openai.com>
Co-authored-by: Codex <noreply@openai.com>
|
/oz-review |
|
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 Powered by Oz |
Oz no longer requests changes for this pull request after the latest automated review.
There was a problem hiding this comment.
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
| relative_path_after_marker(&components, &[".warp", "themes"]) | ||
| .or_else(|| relative_path_after_marker(&components, &["warp-terminal", "themes"])) |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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 <oz-agent@warp.dev> Co-authored-by: Codex <noreply@openai.com>
94f2f2e to
e5d7a87
Compare
Summary
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
Co-Authored-By: Oz oz-agent@warp.dev