Skip to content

Commit b0127b9

Browse files
shakkernerdshakkernerd
committed
fix: harden npm update staging
1 parent 6985c67 commit b0127b9

5 files changed

Lines changed: 263 additions & 109 deletions

File tree

‎src/cli/update-cli/progress.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,9 @@ const STEP_LABELS: Record<string, string> = {
3030
"git rev-parse HEAD (after)": "Verifying update",
3131
"global update": "Updating via package manager",
3232
"global update (omit optional)": "Retrying update without optional deps",
33+
"global install stage": "Preparing staged package install",
34+
"global install verify": "Verifying global package",
35+
"global install swap": "Activating global package",
3336
"global install": "Installing global package",
3437
};
3538

‎src/infra/package-update-steps.test.ts‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -164,4 +164,33 @@ describe("runGlobalPackageUpdateSteps", () => {
164164
);
165165
});
166166
});
167+
168+
it("cleans the staged npm prefix when the install command throws", async () => {
169+
await withTempDir({ prefix: "openclaw-package-update-cleanup-" }, async (base) => {
170+
const prefix = path.join(base, "prefix");
171+
const globalRoot = path.join(prefix, "lib", "node_modules");
172+
const packageRoot = path.join(globalRoot, "openclaw");
173+
await writePackageRoot(packageRoot, "1.0.0");
174+
175+
let stagePrefix: string | undefined;
176+
await expect(
177+
runGlobalPackageUpdateSteps({
178+
installTarget: createNpmTarget(globalRoot),
179+
installSpec: "openclaw@2.0.0",
180+
packageName: "openclaw",
181+
packageRoot,
182+
runCommand: createRootRunner(globalRoot),
183+
runStep: async ({ argv }) => {
184+
const prefixIndex = argv.indexOf("--prefix");
185+
stagePrefix = argv[prefixIndex + 1];
186+
throw new Error("install crashed");
187+
},
188+
timeoutMs: 1000,
189+
}),
190+
).rejects.toThrow("install crashed");
191+
192+
expect(stagePrefix).toBeDefined();
193+
await expect(fs.access(stagePrefix ?? "")).rejects.toMatchObject({ code: "ENOENT" });
194+
});
195+
});
167196
});

‎src/infra/package-update-steps.ts‎

Lines changed: 163 additions & 101 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,39 @@ async function createStagedNpmInstall(
7171
};
7272
}
7373

74+
async function prepareStagedNpmInstall(
75+
installTarget: ResolvedGlobalInstallTarget,
76+
packageName: string,
77+
): Promise<{
78+
stagedInstall: StagedNpmInstall | null;
79+
failedStep: PackageUpdateStepResult | null;
80+
}> {
81+
const startedAt = Date.now();
82+
try {
83+
return {
84+
stagedInstall: await createStagedNpmInstall(installTarget, packageName),
85+
failedStep: null,
86+
};
87+
} catch (err) {
88+
const targetLayout =
89+
installTarget.manager === "npm"
90+
? resolveNpmGlobalPrefixLayoutFromGlobalRoot(installTarget.globalRoot)
91+
: null;
92+
return {
93+
stagedInstall: null,
94+
failedStep: {
95+
name: "global install stage",
96+
command: "prepare staged npm install",
97+
cwd: targetLayout?.prefix ?? installTarget.globalRoot ?? process.cwd(),
98+
durationMs: Date.now() - startedAt,
99+
exitCode: 1,
100+
stdoutTail: null,
101+
stderrTail: formatError(err),
102+
},
103+
};
104+
}
105+
}
106+
74107
async function cleanupStagedNpmInstall(stage: StagedNpmInstall | null): Promise<void> {
75108
if (!stage) {
76109
return;
@@ -215,118 +248,147 @@ export async function runGlobalPackageUpdateSteps(params: {
215248
}> {
216249
const installCwd = params.installCwd === undefined ? {} : { cwd: params.installCwd };
217250
const installEnv = params.env === undefined ? {} : { env: params.env };
218-
let stagedInstall = await createStagedNpmInstall(params.installTarget, params.packageName);
219-
const updateStep = await params.runStep({
220-
name: "global update",
221-
argv: globalInstallArgs(
222-
params.installTarget,
223-
params.installSpec,
224-
undefined,
225-
stagedInstall?.prefix,
226-
),
227-
...installCwd,
228-
...installEnv,
229-
timeoutMs: params.timeoutMs,
230-
});
251+
let stagedInstall: StagedNpmInstall | null = null;
231252

232-
const steps = [updateStep];
233-
let finalInstallStep = updateStep;
234-
if (updateStep.exitCode !== 0) {
235-
await cleanupStagedNpmInstall(stagedInstall);
236-
stagedInstall = await createStagedNpmInstall(params.installTarget, params.packageName);
237-
const fallbackArgv = globalInstallFallbackArgs(
238-
params.installTarget,
239-
params.installSpec,
240-
undefined,
241-
stagedInstall?.prefix,
242-
);
243-
if (fallbackArgv) {
244-
const fallbackStep = await params.runStep({
245-
name: "global update (omit optional)",
246-
argv: fallbackArgv,
247-
...installCwd,
248-
...installEnv,
249-
timeoutMs: params.timeoutMs,
250-
});
251-
steps.push(fallbackStep);
252-
finalInstallStep = fallbackStep;
253-
} else {
253+
try {
254+
const preparedInstall = await prepareStagedNpmInstall(params.installTarget, params.packageName);
255+
stagedInstall = preparedInstall.stagedInstall;
256+
if (preparedInstall.failedStep) {
257+
return {
258+
steps: [preparedInstall.failedStep],
259+
verifiedPackageRoot: params.packageRoot ?? null,
260+
afterVersion: null,
261+
failedStep: preparedInstall.failedStep,
262+
};
263+
}
264+
265+
const updateStep = await params.runStep({
266+
name: "global update",
267+
argv: globalInstallArgs(
268+
params.installTarget,
269+
params.installSpec,
270+
undefined,
271+
stagedInstall?.prefix,
272+
),
273+
...installCwd,
274+
...installEnv,
275+
timeoutMs: params.timeoutMs,
276+
});
277+
278+
const steps = [updateStep];
279+
let finalInstallStep = updateStep;
280+
if (updateStep.exitCode !== 0) {
254281
await cleanupStagedNpmInstall(stagedInstall);
255282
stagedInstall = null;
283+
const preparedFallbackInstall = await prepareStagedNpmInstall(
284+
params.installTarget,
285+
params.packageName,
286+
);
287+
stagedInstall = preparedFallbackInstall.stagedInstall;
288+
if (preparedFallbackInstall.failedStep) {
289+
steps.push(preparedFallbackInstall.failedStep);
290+
return {
291+
steps,
292+
verifiedPackageRoot: params.packageRoot ?? null,
293+
afterVersion: null,
294+
failedStep: preparedFallbackInstall.failedStep,
295+
};
296+
}
297+
298+
const fallbackArgv = globalInstallFallbackArgs(
299+
params.installTarget,
300+
params.installSpec,
301+
undefined,
302+
stagedInstall?.prefix,
303+
);
304+
if (fallbackArgv) {
305+
const fallbackStep = await params.runStep({
306+
name: "global update (omit optional)",
307+
argv: fallbackArgv,
308+
...installCwd,
309+
...installEnv,
310+
timeoutMs: params.timeoutMs,
311+
});
312+
steps.push(fallbackStep);
313+
finalInstallStep = fallbackStep;
314+
} else {
315+
await cleanupStagedNpmInstall(stagedInstall);
316+
stagedInstall = null;
317+
}
256318
}
257-
}
258319

259-
let verifiedPackageRoot =
260-
stagedInstall?.packageRoot ??
261-
(
262-
await resolveGlobalInstallTarget({
263-
manager: params.installTarget,
264-
runCommand: params.runCommand,
265-
timeoutMs: params.timeoutMs,
266-
})
267-
).packageRoot ??
268-
params.packageRoot ??
269-
null;
320+
let verifiedPackageRoot =
321+
stagedInstall?.packageRoot ??
322+
(
323+
await resolveGlobalInstallTarget({
324+
manager: params.installTarget,
325+
runCommand: params.runCommand,
326+
timeoutMs: params.timeoutMs,
327+
})
328+
).packageRoot ??
329+
params.packageRoot ??
330+
null;
270331

271-
let afterVersion: string | null = null;
272-
if (finalInstallStep.exitCode === 0 && verifiedPackageRoot) {
273-
afterVersion = await readPackageVersion(verifiedPackageRoot);
274-
const expectedVersion = resolveExpectedInstalledVersionFromSpec(
275-
params.packageName,
276-
params.installSpec,
277-
);
278-
const verificationErrors = await collectInstalledGlobalPackageErrors({
279-
packageRoot: verifiedPackageRoot,
280-
expectedVersion,
281-
});
282-
if (verificationErrors.length > 0) {
283-
steps.push({
284-
name: "global install verify",
285-
command: `verify ${verifiedPackageRoot}`,
286-
cwd: verifiedPackageRoot,
287-
durationMs: 0,
288-
exitCode: 1,
289-
stderrTail: verificationErrors.join("\n"),
290-
stdoutTail: null,
332+
let afterVersion: string | null = null;
333+
if (finalInstallStep.exitCode === 0 && verifiedPackageRoot) {
334+
afterVersion = await readPackageVersion(verifiedPackageRoot);
335+
const expectedVersion = resolveExpectedInstalledVersionFromSpec(
336+
params.packageName,
337+
params.installSpec,
338+
);
339+
const verificationErrors = await collectInstalledGlobalPackageErrors({
340+
packageRoot: verifiedPackageRoot,
341+
expectedVersion,
291342
});
292-
}
343+
if (verificationErrors.length > 0) {
344+
steps.push({
345+
name: "global install verify",
346+
command: `verify ${verifiedPackageRoot}`,
347+
cwd: verifiedPackageRoot,
348+
durationMs: 0,
349+
exitCode: 1,
350+
stderrTail: verificationErrors.join("\n"),
351+
stdoutTail: null,
352+
});
353+
}
293354

294-
if (stagedInstall && verificationErrors.length === 0) {
295-
const swapStep = await swapStagedNpmInstall({
296-
stage: stagedInstall,
297-
installTarget: params.installTarget,
298-
packageName: params.packageName,
299-
});
300-
steps.push(swapStep);
301-
if (swapStep.exitCode === 0) {
302-
verifiedPackageRoot = params.installTarget.packageRoot ?? verifiedPackageRoot;
355+
if (stagedInstall && verificationErrors.length === 0) {
356+
const swapStep = await swapStagedNpmInstall({
357+
stage: stagedInstall,
358+
installTarget: params.installTarget,
359+
packageName: params.packageName,
360+
});
361+
steps.push(swapStep);
362+
if (swapStep.exitCode === 0) {
363+
verifiedPackageRoot = params.installTarget.packageRoot ?? verifiedPackageRoot;
364+
}
303365
}
304-
}
305366

306-
const failedVerifyOrSwap = steps.find(
307-
(step) =>
308-
(step.name === "global install verify" || step.name === "global install swap") &&
309-
step.exitCode !== 0,
310-
);
311-
const postVerifyStep = failedVerifyOrSwap
312-
? null
313-
: await params.postVerifyStep?.(verifiedPackageRoot);
314-
if (postVerifyStep) {
315-
steps.push(postVerifyStep);
367+
const failedVerifyOrSwap = steps.find(
368+
(step) =>
369+
(step.name === "global install verify" || step.name === "global install swap") &&
370+
step.exitCode !== 0,
371+
);
372+
const postVerifyStep = failedVerifyOrSwap
373+
? null
374+
: await params.postVerifyStep?.(verifiedPackageRoot);
375+
if (postVerifyStep) {
376+
steps.push(postVerifyStep);
377+
}
316378
}
317-
}
318-
319-
await cleanupStagedNpmInstall(stagedInstall);
320379

321-
const failedStep =
322-
finalInstallStep.exitCode !== 0
323-
? finalInstallStep
324-
: (steps.find((step) => step !== updateStep && step.exitCode !== 0) ?? null);
380+
const failedStep =
381+
finalInstallStep.exitCode !== 0
382+
? finalInstallStep
383+
: (steps.find((step) => step !== updateStep && step.exitCode !== 0) ?? null);
325384

326-
return {
327-
steps,
328-
verifiedPackageRoot,
329-
afterVersion,
330-
failedStep,
331-
};
385+
return {
386+
steps,
387+
verifiedPackageRoot,
388+
afterVersion,
389+
failedStep,
390+
};
391+
} finally {
392+
await cleanupStagedNpmInstall(stagedInstall);
393+
}
332394
}

0 commit comments

Comments
 (0)