fix(config): expand leading ~ in config paths (supersedes #301)#370
Open
cameroncooke wants to merge 8 commits intomainfrom
Open
fix(config): expand leading ~ in config paths (supersedes #301)#370cameroncooke wants to merge 8 commits intomainfrom
cameroncooke wants to merge 8 commits intomainfrom
Conversation
Move the tilde-expansion logic that previously lived as a private helper inside src/cli/commands/init.ts into a shared src/utils/expand-home.ts module so it can be reused by other call sites that need the same behavior. The shared version handles a bare '~', '~/' on POSIX, and '~\\' for Windows-style separators.
resolveEffectiveDerivedDataPath() previously fed any non-absolute input to path.resolve(process.cwd(), ...), which caused configured derivedDataPath values like '~/.foo/derivedData' to materialize as a literal '~' directory under the current working directory. Run the input through expandHomePrefix() before the absolute/relative check so tilde-prefixed paths resolve under the user's home directory as users expect. Add unit coverage for the absolute, relative, default, and tilde-expansion code paths.
normalizePathValue() resolved relative path values against cwd but left a literal '~' prefix in place, so derivedDataPath, projectPath, workspacePath, axePath, and the iOS/macOS template paths under both sessionDefaults and sessionDefaultsProfiles all leaked unexpanded tilde paths through to consumers. Apply expandHomePrefix() before the absolute/relative check inside normalizePathValue() so all callers (session defaults, profiles, and top-level path keys) benefit from a single expansion. Cover the behavior with unit tests for sessionDefaults, top-level path keys, and sessionDefaultsProfiles.
executeXcodeBuildCommand() resolves projectPath, workspacePath, and the derivedDataPath argument before invoking xcodebuild. The resolvePathFromCwd() helper passed non-absolute values straight to path.resolve(process.cwd(), ...), so callers that supplied a tilde path (for example, direct API callers without project-config normalization) ended up with a literal '~' segment under cwd. Run the input through expandHomePrefix() inside resolvePathFromCwd() so tilde-prefixed projectPath and workspacePath resolve under the home directory. Add a unit test that verifies tilde expansion of projectPath and derivedDataPath flows through to the xcodebuild command line.
4 tasks
commit: |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Consolidate the duplicated expand-tilde + resolve-cwd path helpers into a single canonical pair (expandHomePrefix, resolvePathFromCwd) in src/utils/path.ts. The helper accepts an optional cwd argument so the project-config call site (which passes its own cwd) shares the same function as the default-cwd callers. Drives out the cursor-bugbot-flagged inconsistency in app-path-resolver.ts: its private resolvePathFromCwd was missing the expandHomePrefix step, so xcodebuild -showBuildSettings received literal '~' paths even when executeXcodeBuildCommand expanded them for the build step. Both copies now resolve to the shared helper, eliminating the class of defect. Migrations: - Remove src/utils/expand-home.ts (contents moved into path.ts). - build-utils.ts, app-path-resolver.ts: drop private resolvePathFromCwd, import shared helper. app-path-resolver call sites pre-check optional params (matches build-utils style) so no overloads are needed. - derived-data-path.ts: keep public resolveEffectiveDerivedDataPath, body simplified to fallback-then-delegate. - project-config.ts: keep normalizePathValue, body simplified to keep tryFileUrlToPath step then delegate. - init.ts: delete the 1-line resolveDestinationPath forwarder, call sites use resolvePathFromCwd directly. Tests: - src/utils/__tests__/path.test.ts covers expandHomePrefix and the new resolvePathFromCwd (default cwd, explicit cwd, tilde, absolute, relative, ~user passthrough). - src/utils/__tests__/app-path-resolver.test.ts asserts the captured xcodebuild command sees expanded projectPath/workspacePath, locking in parity with the build-utils end-to-end test.
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 1535672. Configure here.
The unification commit (1535672) replaced init.ts's resolveDestinationPath wrapper, which did `path.resolve(expandHomePrefix(input))`. The shared helper kept absolute inputs verbatim, dropping the implicit normalization step that path.resolve() with one argument provides. That regressed init.ts's filesystem-root guard (`resolvedDest === path.parse(resolvedDest).root`), which only matches exact-root strings. Inputs like `--dest /foo/..` or `--dest /..` resolved to themselves, bypassing the guard, even though the kernel-level path collapses to `/`. Restore normalization at the helper layer by delegating unconditionally to `path.resolve(cwd, expanded)`. `path.resolve(cwd, X)` already produces the correct result for both absolute and relative `X`, and always normalizes, so the absolute/relative branch is redundant. The four other migrated call sites (build-utils, app-path-resolver, derived-data-path, project-config) gain a benign normalization improvement; only init.ts's guards depended on it for correctness. Tests added cover `/foo/..` -> `/` and `/a/b/../c` -> `/a/c`.
- Remove dead `~\` expansion branch and its test; only POSIX is supported. - Add overload so resolvePathFromCwd accepts string | undefined, dropping the ternary at app-path-resolver call sites. - Replace duplicate tilde handling in snapshot-tests/output-parsers with the shared expandHomePrefix helper. - Add defensive empty-input guard and clarified doc comments to expandHomePrefix; document that ~userName prefixes are not expanded. - Cover whitespace, multi-byte, and undefined cases in path.test.ts. - Note absolute-path normalization in the CHANGELOG fix entry.
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
1 participant
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.
Summary
Expands leading
~(and~//~\on Windows) prefixes in user-supplied config paths so values likederivedDataPath: "~/.foo/derivedData"resolve underhomedir()instead of creating a literal~directory under the project root.Fixes #283. Supersedes #301 by @trmquang93 — the original PR could not be cleanly rebased onto current
main(the targeted code inbuild-utils.tsanddevice/build-settings.tshas been refactored or removed since the PR was opened) and additionally bundled an unrelated #287/#300 commit. Reimplemented from scratch against currentmain, following the spirit of the original PR (single shared utility applied at every entry point).What changed
src/utils/expand-home.ts— single canonicalexpandHomePrefix()helper. Replaces the private duplicate insidesrc/cli/commands/init.ts.resolveEffectiveDerivedDataPath()insrc/utils/derived-data-path.tsnow expands tildes before the absolute/relative check.normalizePathValueinsrc/utils/project-config.tsnow expands tildes before the absolute/relative check. Single point of coverage forprojectPath,workspacePath,derivedDataPath,axePath,iosTemplatePath, andmacosTemplatePathacross bothsessionDefaultsandsessionDefaultsProfiles.resolvePathFromCwdinsrc/utils/build-utils.ts(still used for directprojectPath/workspacePathcallers that bypass project-config normalization) now also expands tildes.[Unreleased] / Fixedcrediting [Feature]: Expand ~ (tilde) in derived data path passed via config #283 and @trmquang93's original Expand tilde (~) in config paths like derivedDataPath #301.Ordering note: in
normalizePathValue,tryFileUrlToPathruns beforeexpandHomePrefix, sofile:URLs are not tilde-mangled.Test plan
npm run build— cleannpm run typecheck— cleannpm run lint— cleannpm run format:check— cleannpm test— 156 files, 1634 tests, all passingtest:snapshotandtest:smoke— skipped (require real device/simulator)Unit tests added at every layer:
src/utils/__tests__/expand-home.test.ts— utility behavior (~,~/...,~\\..., non-tilde absolute, non-tilde relative,~user"do not expand", embedded~, empty-string passthrough).src/utils/__tests__/derived-data-path.test.ts— tilde input resolves underhomedir().src/utils/__tests__/project-config.test.ts— tilde expansion across the path keys covered bynormalizePathValue.src/utils/__tests__/build-utils.test.ts— end-to-end throughexecuteXcodeBuildCommandconfirming bothprojectPathandderivedDataPathare tilde-expanded on the command line.