fix: portable paths in extra-manifest for cross-platform sync - #58
Merged
iHildy merged 5 commits intoAug 31, 2026
Merged
Conversation
Before this fix, extra-manifest.json stored absolute sourcePath values from the pushing machine (e.g. /Users/khangnghiem/.config/opencode/tui.json). On a different machine with a different home directory, the allowlist comparison would fail and files would be skipped during pull. Now: - writeExtraPathManifest stores sourcePath relative to configRoot when the path is inside configRoot (e.g. 'tui.json') - For paths inside homeDir but outside configRoot, stores with ~/ prefix (e.g. '~/.ssh/id_rsa') - For paths outside both, keeps absolute as fallback - applyExtraPaths resolves manifest sourcePaths against the local configRoot/homeDir, making pull work across machines and platforms This also fixes Windows compatibility where ~/ expands to USERPROFILE but opencode configDir uses APPDATA — relative paths resolve correctly against the local configRoot regardless of platform. SyncPlan now includes configRoot so it's available during pull. 14 new tests cover toPortablePath/fromPortablePath and cross-platform round-trips (macOS→Linux, macOS→Windows).
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces portable paths for sync manifests by adding toPortablePath and fromPortablePath functions and incorporating configRoot into the SyncPlan. The review feedback identifies significant cross-platform issues, particularly on Windows, regarding path separator consistency and case sensitivity. It is recommended to use POSIX separators for all portable paths and utilize path.relative to ensure robust path resolution across different operating systems.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
extra-manifest.jsonstored absolutesourcePathvalues from the machine that pushed. A second machine with a different home or OpenCode config root could not match those paths against its local allowlist, so configured extra files were skipped.This PR is intentionally separate from #72. PR #72 changes how relative
extraConfigPathsandextraSecretPathsare interpreted in local configuration. This PR only makes manifest paths portable after the sync plan has resolved them.Fix
/-separated relative paths..rather than an empty string.~/....repoPathand directory item separators.package-lock.json.Existing allowlist enforcement is unchanged. A manifest entry is still applied only when its resolved local destination exactly matches a configured extra path.
Verification
bun run checkbun test: 127 passed, 0 failedbun run buildthemes/night.jsonresolved into the POSIX config rootthemes/day.jsonresolved into the Windows APPDATA OpenCode config rootThe E2E emitted the existing duplicate-sandbox-skill warning and the known warning that the running machine-B config tree was not hot-reloaded even though its local sync checkout contained the second sentinel. Neither warning is caused by this change.
Merge order
Merge #72 first, then update this branch from
main. A synthetic merge shows the production code combines automatically; only the two appended test files need a mechanical conflict resolution. Keeping that order preserves the separate issue scopes while validating portable manifests on top of #72's local path resolution.