Skip to content

Commit 0f526e0

Browse files
committed
fix(cli): handle command args
Signed-off-by: Cory Rylan <crylan@nvidia.com>
1 parent 9280f22 commit 0f526e0

7 files changed

Lines changed: 242 additions & 46 deletions

File tree

projects/internals/tools/src/project/starters.test.ts

Lines changed: 55 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,22 @@
11
// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
22
// SPDX-License-Identifier: Apache-2.0
33

4-
import { describe, expect, it } from 'vitest';
5-
import { startersData, removeWireitScripts } from './starters.js';
4+
import type * as childProcess from 'node:child_process';
5+
import { beforeEach, describe, expect, it, vi } from 'vitest';
6+
import {
7+
startersData,
8+
createGitInitProcess,
9+
createStarterPaths,
10+
removeWireitScripts,
11+
startStarter
12+
} from './starters.js';
613
import { getNPMClient, isCommandAvailable, getPackageJson } from '../internal/node.js';
714

15+
vi.mock('node:child_process', async importOriginal => {
16+
const actual = await importOriginal<typeof childProcess>();
17+
return { ...actual, execFile: vi.fn(), execFileSync: vi.fn() };
18+
});
19+
820
describe('startersData', () => {
921
it('should provide a list of starter metadata', () => {
1022
expect(startersData).toBeDefined();
@@ -157,6 +169,17 @@ describe('removeWireitScripts', () => {
157169
});
158170
});
159171

172+
describe('createStarterPaths', () => {
173+
it('should create starter paths without shell command concatenation', () => {
174+
const outDir = '/tmp/starter output; echo bad';
175+
176+
expect(createStarterPaths('typescript', outDir)).toEqual({
177+
archivePath: '/tmp/starter output; echo bad/typescript.zip',
178+
extractedPath: '/tmp/starter output; echo bad/typescript'
179+
});
180+
});
181+
});
182+
160183
describe('getNPMClient', () => {
161184
it('should return the npm client', async () => {
162185
expect(await getNPMClient()).toBe('pnpm');
@@ -184,3 +207,33 @@ describe('getPackageJson', () => {
184207
expect(result.name).toBe('@internals/tools');
185208
});
186209
});
210+
211+
describe('startStarter', () => {
212+
beforeEach(() => {
213+
vi.clearAllMocks();
214+
});
215+
216+
it('should start the dev server without shell interpretation', async () => {
217+
const { execFileSync } = await import('node:child_process');
218+
const extractedPath = '/tmp/starter with spaces; echo bad';
219+
220+
await startStarter(extractedPath);
221+
222+
expect(execFileSync).toHaveBeenCalledWith('pnpm', ['run', 'dev'], { cwd: extractedPath, stdio: 'inherit' });
223+
});
224+
});
225+
226+
describe('createGitInitProcess', () => {
227+
beforeEach(() => {
228+
vi.clearAllMocks();
229+
});
230+
231+
it('should initialize git without shell interpretation', async () => {
232+
const { execFile } = await import('node:child_process');
233+
const extractedPath = '/tmp/starter with spaces; echo bad';
234+
235+
createGitInitProcess(extractedPath);
236+
237+
expect(execFile).toHaveBeenCalledWith('git', ['init', extractedPath]);
238+
});
239+
});

projects/internals/tools/src/project/starters.ts

Lines changed: 39 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
// SPDX-License-Identifier: Apache-2.0
33

44
import { basename, dirname, join, parse, resolve } from 'node:path';
5-
import { exec, execSync } from 'node:child_process';
5+
import { execFile, execFileSync } from 'node:child_process';
66
import { cwd } from 'node:process';
77

88
import { writeFile } from 'fs/promises';
@@ -37,6 +37,8 @@ export type Starter =
3737
| 'typescript'
3838
| 'vue';
3939

40+
type NPMClient = 'npm' | 'pnpm';
41+
4042
export const startersData = {
4143
angular: {
4244
zip: `${ELEMENTS_PAGES_BASE_URL}/starters/download/angular.zip`,
@@ -110,11 +112,11 @@ export const startersData = {
110112

111113
/* istanbul ignore next -- @preserve */
112114
export async function archiveStarter(projectDir: string, outDir: string) {
113-
const dist = `${outDir}/${projectDir}`;
115+
const dist = join(outDir, projectDir);
114116
await copyProject(projectDir);
115117
writeAllAgentConfigs(dist);
116118
const packageJSON = await exportPackageFromWorkspace(projectDir);
117-
await writeFile(`${dist}/package.json`, JSON.stringify(packageJSON, undefined, 2));
119+
await writeFile(join(dist, 'package.json'), JSON.stringify(packageJSON, undefined, 2));
118120
await zipProject(dist);
119121
}
120122

@@ -134,7 +136,7 @@ async function zipProject(outDir: string) {
134136
/* istanbul ignore next -- @preserve */
135137
function copyProject(projectDir: string) {
136138
const ignoreDirs = new Set(['dist', 'node_modules', '.wireit']);
137-
cpSync(projectDir, `dist/${projectDir}`, {
139+
cpSync(projectDir, join('dist', projectDir), {
138140
recursive: true,
139141
filter: src => !ignoreDirs.has(basename(src))
140142
});
@@ -179,8 +181,7 @@ export async function createStarter(starter: Starter, outDir: string = resolve(c
179181
if (!downloadPath) {
180182
throw new Error(`No download URL for starter "${starter}"`);
181183
}
182-
const archivePath = `${outDir}/${starter}.zip`;
183-
const extractedPath = `${outDir}/${starter}`;
184+
const { archivePath, extractedPath } = createStarterPaths(starter, outDir);
184185
await downloadStarter(downloadPath, archivePath);
185186
await extractStarter(archivePath, extractedPath);
186187
await setupStarterGit(extractedPath);
@@ -202,6 +203,13 @@ export async function createStarter(starter: Starter, outDir: string = resolve(c
202203
}
203204
}
204205

206+
export function createStarterPaths(starter: Starter, outDir: string) {
207+
return {
208+
archivePath: join(outDir, `${starter}.zip`),
209+
extractedPath: join(outDir, starter)
210+
};
211+
}
212+
205213
/* istanbul ignore next -- @preserve */
206214
async function downloadStarter(starterPath: string, outPath: string) {
207215
console.log('⏳ Downloading starter...');
@@ -225,7 +233,7 @@ async function setupStarterGit(extractedDir: string) {
225233
if (hasGit && !isGitRepository(extractedDir)) {
226234
console.log('🔄 Initializing git...');
227235
await new Promise<void>((resolve, reject) => {
228-
const child = exec(`cd ${extractedDir} && git init`);
236+
const child = createGitInitProcess(extractedDir);
229237
child.on('close', code => (code === 0 ? resolve() : reject(new Error(`git init exited with code ${code}`))));
230238
child.on('error', reject);
231239
});
@@ -234,6 +242,10 @@ async function setupStarterGit(extractedDir: string) {
234242
}
235243
}
236244

245+
export function createGitInitProcess(extractedDir: string) {
246+
return execFile('git', ['init', extractedDir]);
247+
}
248+
237249
/* istanbul ignore next -- @preserve */
238250
function isGitRepository(directoryPath: string) {
239251
// Check if .git directory exists directly in the given path
@@ -276,19 +288,17 @@ async function setupStarterNPM(extractedDir: string) {
276288

277289
/* istanbul ignore next -- @preserve */
278290
async function loginRegistry(extractedDir: string) {
279-
const npmClient = await getNPMClient();
291+
const npmClient = await getRequiredNPMClient();
280292
console.log('🔒 Logging in to registry...');
281-
execSync(`cd ${extractedDir} && ${npmClient} login`, {
282-
stdio: 'inherit'
283-
});
293+
execPackageManagerSync(npmClient, ['login'], extractedDir);
284294
}
285295

286296
/* istanbul ignore next -- @preserve */
287297
async function installFromRegistry(extractedDir: string) {
288-
const npmClient = await getNPMClient();
298+
const npmClient = await getRequiredNPMClient();
289299
console.log('📦 Installing dependencies...');
290300
await new Promise<void>((resolve, reject) => {
291-
const child = exec(`cd ${extractedDir} && ${npmClient} install`);
301+
const child = execPackageManager(npmClient, ['install'], extractedDir);
292302
child.on('close', code =>
293303
code === 0 ? resolve() : reject(new Error(`${npmClient} install exited with code ${code}`))
294304
);
@@ -303,9 +313,7 @@ export async function startStarter(extractedPath: string) {
303313
console.log('🚀 Starting project...');
304314

305315
try {
306-
execSync(`cd ${extractedPath} && ${npmClient} run dev`, {
307-
stdio: 'inherit'
308-
});
316+
execPackageManagerSync(npmClient, ['run', 'dev'], extractedPath);
309317
} catch (e) {
310318
if (e instanceof Error && 'signal' in e && e.signal === 'SIGINT') {
311319
console.log('\n👋 Stopped.');
@@ -316,6 +324,21 @@ export async function startStarter(extractedPath: string) {
316324
}
317325
}
318326

327+
async function getRequiredNPMClient() {
328+
const npmClient = await getNPMClient();
329+
if (npmClient === 'npm' || npmClient === 'pnpm') return npmClient;
330+
throw new Error('No supported package manager found.');
331+
}
332+
333+
function execPackageManager(npmClient: NPMClient, args: string[], cwd: string) {
334+
return npmClient === 'pnpm' ? execFile('pnpm', args, { cwd }) : execFile('npm', args, { cwd });
335+
}
336+
337+
function execPackageManagerSync(npmClient: NPMClient, args: string[], cwd: string) {
338+
const options = { cwd, stdio: 'inherit' as const };
339+
npmClient === 'pnpm' ? execFileSync('pnpm', args, options) : execFileSync('npm', args, options);
340+
}
341+
319342
export const claudeProjectSettings = {
320343
$schema: 'https://json.schemastore.org/claude-code-settings.json',
321344
permissions: {

projects/internals/tools/src/project/update.test.ts

Lines changed: 56 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ vi.mock('node:fs', async importOriginal => {
1212
});
1313

1414
vi.mock('node:child_process', () => ({
15-
execSync: vi.fn()
15+
execFileSync: vi.fn()
1616
}));
1717

1818
vi.mock('../internal/node.js', () => ({
@@ -341,8 +341,10 @@ describe('updatePackageJson', () => {
341341
});
342342

343343
describe('updateProject', () => {
344-
beforeEach(() => {
344+
beforeEach(async () => {
345345
vi.clearAllMocks();
346+
const { existsSync } = await import('node:fs');
347+
vi.mocked(existsSync).mockReturnValue(true);
346348
});
347349

348350
it('should return warning when no package.json exists', async () => {
@@ -372,4 +374,56 @@ describe('updateProject', () => {
372374
expect(result.dependencies.message).toContain('package.json');
373375
expect(result.dependencies.message).toContain('/some/cdn-only/project');
374376
});
377+
378+
it('should update dependencies without shell interpretation', async () => {
379+
const { writeFileSync } = await import('node:fs');
380+
const { execFileSync } = await import('node:child_process');
381+
const { ProjectsService } = await import('@internals/metadata');
382+
const { getLatestPublishedVersions } = await import('../api/utils.js');
383+
const { getNPMClient, getPackageJson } = await import('../internal/node.js');
384+
const cwd = '/tmp/project with spaces; echo bad';
385+
const packageJson = {
386+
dependencies: { '@nvidia-elements/core': '1.0.0' },
387+
devDependencies: {},
388+
peerDependencies: {}
389+
};
390+
391+
vi.mocked(getPackageJson).mockReturnValue(packageJson);
392+
vi.mocked(ProjectsService.getData).mockResolvedValue({ data: [{ changelog: 'core' }] });
393+
vi.mocked(getLatestPublishedVersions).mockResolvedValue(
394+
createMockElementVersions({ '@nvidia-elements/core': '2.0.0' })
395+
);
396+
vi.mocked(getNPMClient).mockResolvedValue('pnpm');
397+
398+
const result = await updateProject(cwd);
399+
400+
expect(result.dependencies.status).toBe('success');
401+
expect(writeFileSync).toHaveBeenCalledWith(`${cwd}/package.json`, expect.stringContaining('"2.0.0"'));
402+
expect(execFileSync).toHaveBeenCalledWith('pnpm', ['update', '@nvidia-elements/*', '@nvidia-elements/*'], { cwd });
403+
});
404+
405+
it('should not write package.json when no package manager exists', async () => {
406+
const { writeFileSync } = await import('node:fs');
407+
const { execFileSync } = await import('node:child_process');
408+
const { ProjectsService } = await import('@internals/metadata');
409+
const { getLatestPublishedVersions } = await import('../api/utils.js');
410+
const { getNPMClient, getPackageJson } = await import('../internal/node.js');
411+
412+
vi.mocked(getPackageJson).mockReturnValue({
413+
dependencies: { '@nvidia-elements/core': '1.0.0' },
414+
devDependencies: {},
415+
peerDependencies: {}
416+
});
417+
vi.mocked(ProjectsService.getData).mockResolvedValue({ data: [{ changelog: 'core' }] });
418+
vi.mocked(getLatestPublishedVersions).mockResolvedValue(
419+
createMockElementVersions({ '@nvidia-elements/core': '2.0.0' })
420+
);
421+
vi.mocked(getNPMClient).mockResolvedValue(null);
422+
423+
const result = await updateProject('/tmp/project');
424+
425+
expect(result.dependencies.status).toBe('danger');
426+
expect(writeFileSync).not.toHaveBeenCalled();
427+
expect(execFileSync).not.toHaveBeenCalled();
428+
});
375429
});

0 commit comments

Comments
 (0)