Skip to content

Commit 7eff87e

Browse files
committed
fix: replace terminal-based npm install with server-side runtime.exec
- Auto npm install after package.json updates now uses runtime.exec() - Dependency validator npm install also uses runtime.exec() - Added retry logic (3 attempts with exponential backoff) - Bypasses interactive terminal entirely for reliability - Proper exit code handling and user alerts on failure
1 parent 3242c6b commit 7eff87e

1 file changed

Lines changed: 62 additions & 18 deletions

File tree

app/lib/runtime/action-runner.ts

Lines changed: 62 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import type { RuntimeProvider, DirEntry } from './runtime-provider';
1+
import type { RuntimeProvider, DirEntry, ProcessResult } from './runtime-provider';
22
import { path as nodePath, toRelativePath } from '~/utils/path';
33
import { atom, map, type MapStore } from 'nanostores';
44
import type {
@@ -656,19 +656,26 @@ export class ActionRunner {
656656
const hasNewDeps = Object.keys(newAllDeps).some((pkg) => !oldDepsSnapshot![pkg]);
657657

658658
if (hasNewDeps) {
659-
logger.info('package.json dependencies changed, running npm install...');
659+
logger.info('package.json dependencies changed, running npm install via runtime.exec...');
660660

661-
const shell = this.#shellTerminal();
662-
await shell.ready();
663-
await shell.executeCommand(this.runnerId.get(), 'npm install --legacy-peer-deps', () => {
664-
/* no-op: auto-install doesn't need abort */
665-
});
666-
logger.info('npm install completed after package.json update');
661+
const installResult = await this.#execNpmInstall(runtime);
662+
663+
if (installResult.exitCode === 0) {
664+
logger.info('npm install completed successfully after package.json update');
665+
} else {
666+
logger.warn('npm install exited with code', installResult.exitCode);
667+
logger.debug('npm install output:', installResult.output);
668+
669+
this.onAlert?.({
670+
type: 'warning',
671+
title: 'Dependency Install Warning',
672+
description: `npm install finished with warnings (exit code ${installResult.exitCode}). Some packages may not have installed correctly.`,
673+
});
674+
}
667675
}
668676
} catch (installError) {
669677
logger.error('Failed to auto-install after package.json update:', installError);
670678

671-
// Warn the user so they know to manually run npm install
672679
this.onAlert?.({
673680
type: 'warning',
674681
title: 'Dependency Install Failed',
@@ -682,6 +689,47 @@ export class ActionRunner {
682689
}
683690
}
684691

692+
/**
693+
* Run `npm install --legacy-peer-deps` via the server-side runtime exec API.
694+
*
695+
* This bypasses the interactive terminal entirely, using `child_process.exec`
696+
* on the server. Much more reliable than the marker-based DevonzShell approach
697+
* because it doesn't contend with the dev-server shell session.
698+
*
699+
* Retries up to 3 times with exponential backoff (1s, 2s, 4s) on failure.
700+
*/
701+
async #execNpmInstall(runtime: RuntimeProvider, retries = 3): Promise<ProcessResult> {
702+
for (let attempt = 1; attempt <= retries; attempt++) {
703+
try {
704+
const result = await runtime.exec('npm install --legacy-peer-deps');
705+
706+
if (result.exitCode === 0 || attempt === retries) {
707+
return result;
708+
}
709+
710+
// Non-zero exit — retry with backoff
711+
const delay = 1000 * 2 ** (attempt - 1); // 1s, 2s, 4s
712+
logger.warn(
713+
`npm install attempt ${attempt}/${retries} failed (exit ${result.exitCode}), retrying in ${delay}ms...`,
714+
);
715+
await new Promise((resolve) => setTimeout(resolve, delay));
716+
} catch (error) {
717+
if (attempt === retries) {
718+
logger.error(`npm install attempt ${attempt}/${retries} threw:`, error);
719+
720+
return { exitCode: 1, output: String(error) };
721+
}
722+
723+
const delay = 1000 * 2 ** (attempt - 1);
724+
logger.warn(`npm install attempt ${attempt}/${retries} threw, retrying in ${delay}ms...`, error);
725+
await new Promise((resolve) => setTimeout(resolve, delay));
726+
}
727+
}
728+
729+
// Unreachable, but satisfies TypeScript
730+
return { exitCode: 1, output: 'All retry attempts exhausted' };
731+
}
732+
685733
/**
686734
* Node.js built-in modules that should NOT be flagged as missing npm packages.
687735
*/
@@ -901,18 +949,14 @@ export class ActionRunner {
901949
await runtime.fs.writeFile('package.json', JSON.stringify(pkgJson, null, 2));
902950
logger.info('Updated package.json with missing dependencies');
903951

904-
// Run npm install to fetch the newly added packages
905-
const shell = this.#shellTerminal();
906-
await shell.ready();
907-
908-
const installResult = await shell.executeCommand(this.runnerId.get(), 'npm install --legacy-peer-deps', () => {
909-
/* no-op: dependency validation install doesn't need abort */
910-
});
952+
// Run npm install via runtime.exec — bypasses the terminal for reliability
953+
const installResult = await this.#execNpmInstall(runtime);
911954

912-
if (installResult?.exitCode === 0) {
955+
if (installResult.exitCode === 0) {
913956
logger.info('npm install completed successfully after dependency validation');
914957
} else {
915-
logger.warn('npm install had non-zero exit code after dependency validation:', installResult?.exitCode);
958+
logger.warn('npm install had non-zero exit code after dependency validation:', installResult.exitCode);
959+
logger.debug('npm install output:', installResult.output);
916960
}
917961
} else {
918962
logger.debug('Dependency validation passed: all imported packages are in package.json');

0 commit comments

Comments
 (0)