Skip to content

Commit 1535672

Browse files
committed
refactor(utils): unify path resolution helpers in src/utils/path.ts
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.
1 parent f98824a commit 1535672

10 files changed

Lines changed: 201 additions & 103 deletions

File tree

src/cli/commands/init.ts

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import * as os from 'node:os';
55
import * as clack from '@clack/prompts';
66
import { getResourceRoot } from '../../core/resource-root.ts';
77
import { createPrompter, isInteractiveTTY, type Prompter } from '../interactive/prompts.ts';
8-
import { expandHomePrefix } from '../../utils/expand-home.ts';
8+
import { resolvePathFromCwd } from '../../utils/path.ts';
99

1010
type SkillType = 'mcp' | 'cli';
1111

@@ -73,10 +73,6 @@ function readSkillContent(skillType: SkillType): string {
7373
return fs.readFileSync(sourcePath, 'utf8');
7474
}
7575

76-
function resolveDestinationPath(inputPath: string): string {
77-
return path.resolve(expandHomePrefix(inputPath));
78-
}
79-
8076
async function promptConfirm(question: string): Promise<boolean> {
8177
if (!isInteractiveTTY()) {
8278
return false;
@@ -205,7 +201,7 @@ function resolveTargets(
205201
operation: 'install' | 'uninstall',
206202
): ClientInfo[] {
207203
if (destFlag) {
208-
const resolvedDest = resolveDestinationPath(destFlag);
204+
const resolvedDest = resolvePathFromCwd(destFlag);
209205
if (resolvedDest === path.parse(resolvedDest).root) {
210206
throw new Error(
211207
'Refusing to use filesystem root as skills destination. Use a dedicated directory.',
@@ -350,7 +346,7 @@ async function collectInitSelection(
350346
}
351347

352348
if (destProvided) {
353-
const resolvedDest = resolveDestinationPath(argv.dest!);
349+
const resolvedDest = resolvePathFromCwd(argv.dest!);
354350
if (resolvedDest === path.parse(resolvedDest).root) {
355351
throw new Error(
356352
'Refusing to use filesystem root as skills destination. Use a dedicated directory.',
@@ -432,7 +428,7 @@ async function promptCustomPath(): Promise<string> {
432428
message: 'Enter the destination directory path:',
433429
validate: (value: string | undefined) => {
434430
if (!value?.trim()) return 'Path cannot be empty.';
435-
const resolved = resolveDestinationPath(value);
431+
const resolved = resolvePathFromCwd(value);
436432
if (resolved === path.parse(resolved).root) {
437433
return 'Refusing to use filesystem root. Use a dedicated directory.';
438434
}
@@ -445,7 +441,7 @@ async function promptCustomPath(): Promise<string> {
445441
throw new Error('Operation cancelled.');
446442
}
447443

448-
return resolveDestinationPath(result as string);
444+
return resolvePathFromCwd(result as string);
449445
}
450446

451447
export function registerInitCommand(app: Argv, ctx?: { workspaceRoot: string }): void {
Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
import { describe, expect, it } from 'vitest';
2+
import path from 'node:path';
3+
import { homedir } from 'node:os';
4+
import { createMockExecutor } from '../../test-utils/mock-executors.ts';
5+
import { resolveAppPathFromBuildSettings } from '../app-path-resolver.ts';
6+
import { XcodePlatform } from '../../types/common.ts';
7+
8+
describe('resolveAppPathFromBuildSettings', () => {
9+
it('expands tilde-prefixed projectPath when invoking xcodebuild', async () => {
10+
let capturedCommand: string[] | undefined;
11+
12+
const mockExecutor = createMockExecutor({
13+
success: true,
14+
output:
15+
'BUILT_PRODUCTS_DIR = /Build/Products/Debug-iphonesimulator\nFULL_PRODUCT_NAME = App.app\n',
16+
exitCode: 0,
17+
onExecute: (command) => {
18+
capturedCommand = command;
19+
},
20+
});
21+
22+
await resolveAppPathFromBuildSettings(
23+
{
24+
projectPath: '~/Code/App.xcodeproj',
25+
scheme: 'App',
26+
platform: XcodePlatform.iOSSimulator,
27+
},
28+
mockExecutor,
29+
);
30+
31+
const expected = path.join(homedir(), 'Code/App.xcodeproj');
32+
expect(capturedCommand).toBeDefined();
33+
expect(capturedCommand).toContain(expected);
34+
expect(capturedCommand).not.toContain('~/Code/App.xcodeproj');
35+
});
36+
37+
it('expands tilde-prefixed workspacePath when invoking xcodebuild', async () => {
38+
let capturedCommand: string[] | undefined;
39+
40+
const mockExecutor = createMockExecutor({
41+
success: true,
42+
output:
43+
'BUILT_PRODUCTS_DIR = /Build/Products/Debug-iphonesimulator\nFULL_PRODUCT_NAME = App.app\n',
44+
exitCode: 0,
45+
onExecute: (command) => {
46+
capturedCommand = command;
47+
},
48+
});
49+
50+
await resolveAppPathFromBuildSettings(
51+
{
52+
workspacePath: '~/Code/App.xcworkspace',
53+
scheme: 'App',
54+
platform: XcodePlatform.iOSSimulator,
55+
},
56+
mockExecutor,
57+
);
58+
59+
const expected = path.join(homedir(), 'Code/App.xcworkspace');
60+
expect(capturedCommand).toBeDefined();
61+
expect(capturedCommand).toContain(expected);
62+
});
63+
64+
it('leaves absolute paths unchanged', async () => {
65+
let capturedCommand: string[] | undefined;
66+
67+
const mockExecutor = createMockExecutor({
68+
success: true,
69+
output:
70+
'BUILT_PRODUCTS_DIR = /Build/Products/Debug-iphonesimulator\nFULL_PRODUCT_NAME = App.app\n',
71+
exitCode: 0,
72+
onExecute: (command) => {
73+
capturedCommand = command;
74+
},
75+
});
76+
77+
await resolveAppPathFromBuildSettings(
78+
{
79+
projectPath: '/abs/path/App.xcodeproj',
80+
scheme: 'App',
81+
platform: XcodePlatform.iOSSimulator,
82+
},
83+
mockExecutor,
84+
);
85+
86+
expect(capturedCommand).toContain('/abs/path/App.xcodeproj');
87+
});
88+
});

src/utils/__tests__/expand-home.test.ts

Lines changed: 0 additions & 38 deletions
This file was deleted.

src/utils/__tests__/path.test.ts

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
import { describe, expect, it } from 'vitest';
2+
import path from 'node:path';
3+
import { homedir } from 'node:os';
4+
import { expandHomePrefix, resolvePathFromCwd } from '../path.ts';
5+
6+
describe('expandHomePrefix', () => {
7+
it('expands a bare ~ to the home directory', () => {
8+
expect(expandHomePrefix('~')).toBe(homedir());
9+
});
10+
11+
it('expands a leading ~/ to the home directory', () => {
12+
expect(expandHomePrefix('~/foo/bar')).toBe(path.join(homedir(), 'foo/bar'));
13+
});
14+
15+
it('expands a leading ~\\ on Windows-style separators', () => {
16+
expect(expandHomePrefix('~\\foo\\bar')).toBe(path.join(homedir(), 'foo\\bar'));
17+
});
18+
19+
it('returns absolute paths unchanged', () => {
20+
expect(expandHomePrefix('/absolute/path')).toBe('/absolute/path');
21+
});
22+
23+
it('returns relative paths unchanged', () => {
24+
expect(expandHomePrefix('relative/path')).toBe('relative/path');
25+
});
26+
27+
it('does not expand ~user style prefixes', () => {
28+
expect(expandHomePrefix('~other/foo')).toBe('~other/foo');
29+
});
30+
31+
it('does not expand ~ embedded later in the path', () => {
32+
expect(expandHomePrefix('foo/~/bar')).toBe('foo/~/bar');
33+
});
34+
35+
it('returns an empty string unchanged', () => {
36+
expect(expandHomePrefix('')).toBe('');
37+
});
38+
});
39+
40+
describe('resolvePathFromCwd', () => {
41+
it('expands a bare ~ to the home directory', () => {
42+
expect(resolvePathFromCwd('~')).toBe(homedir());
43+
});
44+
45+
it('expands a leading ~/ under the home directory', () => {
46+
expect(resolvePathFromCwd('~/.foo/derivedData')).toBe(path.join(homedir(), '.foo/derivedData'));
47+
});
48+
49+
it('returns absolute paths unchanged', () => {
50+
expect(resolvePathFromCwd('/abs/path')).toBe('/abs/path');
51+
});
52+
53+
it('resolves relative paths against process.cwd() by default', () => {
54+
expect(resolvePathFromCwd('rel/path')).toBe(path.resolve(process.cwd(), 'rel/path'));
55+
});
56+
57+
it('resolves relative paths against an explicit cwd when provided', () => {
58+
expect(resolvePathFromCwd('rel/path', '/some/base')).toBe(
59+
path.resolve('/some/base', 'rel/path'),
60+
);
61+
});
62+
63+
it('does not resolve absolute paths against an explicit cwd', () => {
64+
expect(resolvePathFromCwd('/abs/path', '/some/base')).toBe('/abs/path');
65+
});
66+
67+
it('does not expand ~user style prefixes', () => {
68+
expect(resolvePathFromCwd('~other/foo')).toBe(path.resolve(process.cwd(), '~other/foo'));
69+
});
70+
});

src/utils/app-path-resolver.ts

Lines changed: 3 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2,18 +2,7 @@ import path from 'node:path';
22
import type { XcodePlatform } from '../types/common.ts';
33
import type { CommandExecutor } from './command.ts';
44
import { resolveEffectiveDerivedDataPath } from './derived-data-path.ts';
5-
6-
function resolvePathFromCwd(pathValue?: string): string | undefined {
7-
if (!pathValue) {
8-
return undefined;
9-
}
10-
11-
if (path.isAbsolute(pathValue)) {
12-
return pathValue;
13-
}
14-
15-
return path.resolve(process.cwd(), pathValue);
16-
}
5+
import { resolvePathFromCwd } from './path.ts';
176

187
export function getBuildSettingsDestination(platform: XcodePlatform, deviceId?: string): string {
198
if (deviceId) {
@@ -57,8 +46,8 @@ export async function resolveAppPathFromBuildSettings(
5746
): Promise<string> {
5847
const command = ['xcodebuild', '-showBuildSettings'];
5948

60-
const workspacePath = resolvePathFromCwd(params.workspacePath);
61-
const projectPath = resolvePathFromCwd(params.projectPath);
49+
const workspacePath = params.workspacePath ? resolvePathFromCwd(params.workspacePath) : undefined;
50+
const projectPath = params.projectPath ? resolvePathFromCwd(params.projectPath) : undefined;
6251
const derivedDataPath = resolveEffectiveDerivedDataPath(params.derivedDataPath);
6352

6453
let projectDir: string | undefined;

src/utils/build-utils.ts

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import {
1313
import path from 'path';
1414
import os from 'node:os';
1515
import { resolveEffectiveDerivedDataPath } from './derived-data-path.ts';
16-
import { expandHomePrefix } from './expand-home.ts';
16+
import { resolvePathFromCwd } from './path.ts';
1717
import type { XcodebuildPipeline } from './xcodebuild-pipeline.ts';
1818
import { createNoticeFragment } from './xcodebuild-output.ts';
1919

@@ -22,14 +22,6 @@ export interface BuildCommandResult {
2222
isError?: boolean;
2323
}
2424

25-
function resolvePathFromCwd(pathValue: string): string {
26-
const expanded = expandHomePrefix(pathValue);
27-
if (path.isAbsolute(expanded)) {
28-
return expanded;
29-
}
30-
return path.resolve(process.cwd(), expanded);
31-
}
32-
3325
function getDefaultSwiftPackageCachePath(): string {
3426
return path.join(os.homedir(), 'Library', 'Caches', 'org.swift.swiftpm');
3527
}

src/utils/derived-data-path.ts

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,9 @@
1-
import * as path from 'node:path';
21
import { DERIVED_DATA_DIR } from './log-paths.ts';
3-
import { expandHomePrefix } from './expand-home.ts';
2+
import { resolvePathFromCwd } from './path.ts';
43

54
export function resolveEffectiveDerivedDataPath(input?: string): string {
65
if (!input || input.trim().length === 0) {
76
return DERIVED_DATA_DIR;
87
}
9-
const expanded = expandHomePrefix(input);
10-
if (path.isAbsolute(expanded)) {
11-
return expanded;
12-
}
13-
return path.resolve(process.cwd(), expanded);
8+
return resolvePathFromCwd(input);
149
}

src/utils/expand-home.ts

Lines changed: 0 additions & 18 deletions
This file was deleted.

src/utils/path.ts

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
import path from 'node:path';
2+
import { homedir } from 'node:os';
3+
4+
/**
5+
* Expand a leading ~ or ~/ (or ~\ on Windows) prefix to the user's home directory.
6+
* Returns the path unchanged if it does not start with ~ or starts with ~userName.
7+
*/
8+
export function expandHomePrefix(inputPath: string): string {
9+
if (inputPath === '~') {
10+
return homedir();
11+
}
12+
13+
if (inputPath.startsWith('~/') || inputPath.startsWith('~\\')) {
14+
return path.join(homedir(), inputPath.slice(2));
15+
}
16+
17+
return inputPath;
18+
}
19+
20+
/**
21+
* Resolve a user-supplied path: expand ~ then return as-is when absolute,
22+
* otherwise resolve against `cwd` (defaults to process.cwd()).
23+
*/
24+
export function resolvePathFromCwd(pathValue: string, cwd: string = process.cwd()): string {
25+
const expanded = expandHomePrefix(pathValue);
26+
if (path.isAbsolute(expanded)) {
27+
return expanded;
28+
}
29+
return path.resolve(cwd, expanded);
30+
}

0 commit comments

Comments
 (0)