Skip to content

Commit 5607e44

Browse files
amknightamknight
committed
fix(update): pin package install to service root when nodes differ without owning npm
When the package root matches between shell and service but the node binaries differ, set honorPackageRoot so resolveGlobalInstallTarget pins the install to the service root's inferred global prefix. Without this, PATH-visible npm root -g could resolve to the switched Node-B prefix, silently moving the package away from the service unit's entrypoint. Also harden the Docker E2E script: validate the unit file was created before continuing, capture update exit codes, and exit non-zero when bug indicators are found.
1 parent e5b91b9 commit 5607e44

3 files changed

Lines changed: 146 additions & 5 deletions

File tree

‎scripts/e2e/multi-node-update-docker.sh‎

Lines changed: 40 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ docker_e2e_package_mount_args "$PACKAGE_TGZ"
3434

3535
echo "=== Running multi-node-update Docker E2E ==="
3636

37+
CONTAINER_EXIT=0
3738
docker_e2e_run_with_harness \
3839
-e COREPACK_ENABLE_DOWNLOAD_PROMPT=0 \
3940
-e CI=true \
@@ -209,6 +210,8 @@ echo "systemctl shim installed."
209210
# Now install the gateway service using node-A.
210211
echo "Installing gateway service..."
211212
mkdir -p "$(dirname "$GATEWAY_UNIT_PATH")"
213+
# gateway install may exit non-zero because our systemctl shim cannot fully
214+
# restart, but the unit file gets written before the restart step.
212215
openclaw gateway install --json >"$ARTIFACTS/gateway-install.json" 2>"$ARTIFACTS/gateway-install.err" || true
213216
214217
echo ""
@@ -222,11 +225,11 @@ if [ -f "$GATEWAY_UNIT_PATH" ]; then
222225
BAKED_NODE_BEFORE="$(echo "$EXEC_START_BEFORE" | sed "s/^ExecStart=//" | awk "{print \$1}")"
223226
echo "Baked node path BEFORE update: $BAKED_NODE_BEFORE"
224227
else
225-
echo "WARNING: Gateway unit file was not created at $GATEWAY_UNIT_PATH"
226-
ls -la "$(dirname "$GATEWAY_UNIT_PATH")/" 2>/dev/null || true
228+
echo "FAIL: Gateway unit file was not created at $GATEWAY_UNIT_PATH"
227229
echo "gateway install output:"
228230
cat "$ARTIFACTS/gateway-install.json" 2>/dev/null || true
229231
cat "$ARTIFACTS/gateway-install.err" 2>/dev/null || true
232+
exit 1
230233
fi
231234
232235
echo ""
@@ -251,14 +254,24 @@ echo "── Step 6: Run openclaw update (this is the bug) ──"
251254
# `gateway install --force` and bakes the current process.execPath
252255
# (now node-B) into the service unit. This is where the split happens.
253256
echo "Running openclaw update --yes --json..."
257+
UPDATE_EXIT=0
254258
openclaw update --yes --json \
255259
--tag /tmp/openclaw-current.tgz \
256-
>"$ARTIFACTS/update.json" 2>"$ARTIFACTS/update.err" || true
260+
>"$ARTIFACTS/update.json" 2>"$ARTIFACTS/update.err" || UPDATE_EXIT=$?
257261
258262
echo ""
263+
echo "Update exit code: $UPDATE_EXIT"
259264
echo "Update stderr (if any):"
260265
cat "$ARTIFACTS/update.err" 2>/dev/null | tail -10 || true
261266
267+
# The update may fail during restart (systemctl shim limitations) but it must
268+
# have at least attempted the package install. Check that it ran past early exit.
269+
if [ "$UPDATE_EXIT" -ne 0 ] && ! grep -q "gateway" "$ARTIFACTS/update.err" 2>/dev/null; then
270+
echo "FAIL: openclaw update failed before reaching the package install step"
271+
cat "$ARTIFACTS/update.err" 2>/dev/null || true
272+
exit 1
273+
fi
274+
262275
echo ""
263276
echo "── Step 7: Inspect the service unit AFTER update ──"
264277
@@ -354,7 +367,24 @@ echo "========================================"
354367
echo " Reproduction complete."
355368
echo " Artifacts saved to /tmp/artifacts/"
356369
echo "========================================"
357-
'
370+
371+
# ── Final exit code ──────────────────────────────────────────────────────────
372+
# Exit non-zero if any BUG was found, making this usable as a CI gate.
373+
EXIT_CODE=0
374+
if [ "$BAKED_NODE_AFTER" = "$NODE_B" ] && [ "$BAKED_NODE_BEFORE" != "$NODE_B" ]; then
375+
EXIT_CODE=1
376+
fi
377+
if [ -f "$NPM_PREFIX_B/lib/node_modules/openclaw/package.json" ]; then
378+
EXIT_CODE=1
379+
fi
380+
if [ -f "$GATEWAY_UNIT_PATH" ]; then
381+
ENTRYPOINT_PATH_CHECK="$(grep "^ExecStart=" "$GATEWAY_UNIT_PATH" | head -1 | sed "s/^ExecStart=//" | awk "{print \$2}")" || true
382+
if [ -n "$ENTRYPOINT_PATH_CHECK" ] && [ ! -f "$ENTRYPOINT_PATH_CHECK" ]; then
383+
EXIT_CODE=1
384+
fi
385+
fi
386+
exit $EXIT_CODE
387+
' || CONTAINER_EXIT=$?
358388

359389
echo ""
360390
echo "=== Artifacts ==="
@@ -366,3 +396,9 @@ if [ -f "$ARTIFACT_DIR/run.log" ]; then
366396
echo "=== Key results ==="
367397
grep -E "^(BUG|FIXED|OK|CHANGED|WARNING)" "$ARTIFACT_DIR/run.log" || echo "(no key results found)"
368398
fi
399+
400+
if [ "$CONTAINER_EXIT" -ne 0 ]; then
401+
echo ""
402+
echo "FAIL: Docker container exited with code $CONTAINER_EXIT"
403+
fi
404+
exit "$CONTAINER_EXIT"

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

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3197,6 +3197,110 @@ describe("update-cli", () => {
31973197
expect(serviceInstallCall?.[0][0]).toBe(serviceNode);
31983198
});
31993199

3200+
it("pins package install to the service root when nodes differ and no owning npm exists at the prefix", async () => {
3201+
const servicePrefix = await createTrackedTempDir("openclaw-no-npm-prefix-");
3202+
const nodeModules = path.join(servicePrefix, "lib", "node_modules");
3203+
const root = path.join(nodeModules, "openclaw");
3204+
const serviceNode = path.join(servicePrefix, "bin", "node");
3205+
const entrypoint = path.join(root, "dist", "index.js");
3206+
// Create the node binary but intentionally do NOT create <prefix>/bin/npm
3207+
// so resolvePreferredNpmCommand returns null and the PATH npm is used.
3208+
await fs.mkdir(path.dirname(entrypoint), { recursive: true });
3209+
await fs.mkdir(path.dirname(serviceNode), { recursive: true });
3210+
await fs.writeFile(serviceNode, "", "utf-8");
3211+
// No npm binary at servicePrefix/bin/npm!
3212+
await fs.writeFile(
3213+
path.join(root, "package.json"),
3214+
JSON.stringify({ name: "openclaw", version: "2026.5.18" }),
3215+
"utf-8",
3216+
);
3217+
await fs.writeFile(entrypoint, "", "utf-8");
3218+
await writePackageDistInventory(root);
3219+
mockPackageInstallStatus(root);
3220+
serviceReadCommand.mockResolvedValue({
3221+
programArguments: [serviceNode, entrypoint, "gateway"],
3222+
});
3223+
serviceLoaded.mockResolvedValue(true);
3224+
pathExists.mockImplementation(async (candidate: string) => {
3225+
try {
3226+
await fs.access(candidate);
3227+
return true;
3228+
} catch {
3229+
return false;
3230+
}
3231+
});
3232+
// The PATH npm returns a DIFFERENT global root (simulates Node-B's npm).
3233+
const nodeBGlobalRoot = path.join(
3234+
await createTrackedTempDir("node-b-global-"),
3235+
"lib",
3236+
"node_modules",
3237+
);
3238+
await fs.mkdir(nodeBGlobalRoot, { recursive: true });
3239+
vi.mocked(runCommandWithTimeout).mockImplementation(async (argv) => {
3240+
if (Array.isArray(argv) && argv[0] === serviceNode && argv[1] === "--version") {
3241+
return {
3242+
stdout: "v24.14.0\n",
3243+
stderr: "",
3244+
code: 0,
3245+
signal: null,
3246+
killed: false,
3247+
termination: "exit",
3248+
};
3249+
}
3250+
if (Array.isArray(argv) && argv[0] === "npm" && argv[1] === "root" && argv[2] === "-g") {
3251+
// PATH npm returns Node-B's root, NOT the service root.
3252+
return {
3253+
stdout: `${nodeBGlobalRoot}\n`,
3254+
stderr: "",
3255+
code: 0,
3256+
signal: null,
3257+
killed: false,
3258+
termination: "exit",
3259+
};
3260+
}
3261+
if (Array.isArray(argv) && argv[0] === "npm" && argv[1] === "i") {
3262+
// Install step: create the expected package structure at the target.
3263+
const prefixIdx = argv.indexOf("--prefix");
3264+
const stagePrefix = prefixIdx >= 0 ? argv[prefixIdx + 1] : undefined;
3265+
const stageRoot = stagePrefix
3266+
? path.join(stagePrefix, "lib", "node_modules", "openclaw")
3267+
: root;
3268+
const stageEntryPoint = path.join(stageRoot, "dist", "index.js");
3269+
await fs.mkdir(path.dirname(stageEntryPoint), { recursive: true });
3270+
await fs.writeFile(
3271+
path.join(stageRoot, "package.json"),
3272+
JSON.stringify({ name: "openclaw", version: "2026.5.20" }),
3273+
"utf-8",
3274+
);
3275+
await fs.writeFile(stageEntryPoint, "export {};\n", "utf-8");
3276+
await writePackageDistInventory(stageRoot);
3277+
}
3278+
return {
3279+
stdout: "",
3280+
stderr: "",
3281+
code: 0,
3282+
signal: null,
3283+
killed: false,
3284+
termination: "exit",
3285+
};
3286+
});
3287+
3288+
await updateCommand({ yes: true });
3289+
3290+
// The install command must use --prefix pointing to a location within
3291+
// the service root's prefix tree, NOT Node-B's global root.
3292+
const installCall = packageInstallCommandCall();
3293+
expect(installCall).toBeDefined();
3294+
const installArgv = installCall![0];
3295+
const prefixIdx = installArgv.indexOf("--prefix");
3296+
expect(prefixIdx).toBeGreaterThan(-1);
3297+
// Staging prefix should be under the service prefix, not Node-B's.
3298+
expect(installArgv[prefixIdx + 1]).toContain(servicePrefix);
3299+
expect(installArgv[prefixIdx + 1]).not.toContain(nodeBGlobalRoot);
3300+
// Follow-up commands use the service node.
3301+
expect(doctorCommandCall()?.[0][0]).toBe(serviceNode);
3302+
});
3303+
32003304
it("repairs legacy config before persisting a requested update channel", async () => {
32013305
const tempDir = createCaseDir("openclaw-update");
32023306
mockPackageInstallStatus(tempDir);

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3254,7 +3254,8 @@ export async function updateCommand(opts: UpdateCommandOptions): Promise<void> {
32543254
jsonMode: Boolean(opts.json),
32553255
managedServiceEnv: prePackageServiceStop?.serviceEnv,
32563256
invocationCwd,
3257-
honorPackageRoot: managedServiceRootRedirect !== null,
3257+
honorPackageRoot:
3258+
managedServiceRootRedirect !== null || managedServiceNodeRunner !== undefined,
32583259
nodeRunner: managedServiceNodeRunner,
32593260
})
32603261
: await runGitUpdate({

0 commit comments

Comments
 (0)