vrr/openclaw
2
0
Fork 0
mirror of https://github.com/openclaw/openclaw.git synced 2026-10-03 17:53:39 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 31

fix(update): retain ownership across pnpm package generations (#144778)

Browse source 
Related: #144667. Follow-up to #144713.

## What Problem This Solves

After pnpm replaces a package generation during an update across a state-schema version, the finalizer can fail with `Candidate executor binding does not match its parent`. A subsequent rollback can also fail service-ownership checks or restart without revalidating the restored service.

## Why This Change Was Made

Bind the finalizer to the activated package generation while retaining the original installation's recovery custody. Revalidate service ownership across rollback shutdown and after reinstalling the restored service, before restart. Preserve the receiver's existing live ownership checks.

The staging fix, tests and documentation from @steipete's merged #144713 are retained unchanged. This PR's remaining diff contains only the generation finalizer and rollback repair plus their tests.

## User Impact

An updater containing this repair can complete a pnpm package-generation change with a healthy target Gateway. A compatible post-activation failure can restore the previous healthy Gateway. Changed package or service owners still cause refusal.

## Evidence

Real public CLI proof used isolated Linux user services, published 2026.9.3 and 2026.9.4 packages, and distinct caller Node 24/service Node 26 runtimes:

| Case | Result |
| --- | --- |
| Original baseline, pnpm 12.1.0 | Staging refuses the live bin before activation; old Gateway stays healthy |
| Stage-only driver | Activated-generation finalizer binding fails; recovery failures retained |
| Corrected driver, pnpm 12.1.0 | Update succeeds; CLI and running Gateway confirm 2026.9.4 |
| Corrected driver, pnpm 11.24.0 | Update succeeds; CLI and running Gateway confirm 2026.9.4 |
| Controlled post-activation Doctor exit 73 | Failure retained; package, launcher and healthy 2026.9.3 Gateway restored |

- Caller PATH and pnpm configuration stayed unchanged. Staged validation completed before activation.
- Earlier live-bin refusal, npm 11.17.0 and Bun 1.4.2 controls retain their original tested identities with independent unchanged-behavior review.
- 357 distinct focused cases passed before main integration. The resolved integration passed 130 overlapping staging, executor, service-ancestry and rollback cases, focused lint and normal pre-commit checks.
- Independent source and public-behavior review completed. The real tested updater is `f8d27b2053ab`, built by [Package Acceptance](https://github.com/openclaw/openclaw/actions/runs/34573463335). It is not an exact build of the final integrated head. Source review qualified reuse for the recorded terminal path: equivalent staging environment, unchanged generation owners and published package bytes, inactive managed-descendant ancestry exemption, and equivalent canonical packaged plugin selection. The exact combined source passed the focused integration checks.

Proof uses the hoisted linker within pnpm's isolated `global/v11` projects. Default-linker service-root discovery remains a separate refusal. Healthy rollback retains an existing recommended service-PATH audit under custom `PNPM_HOME`; normal target update clears that recommendation.

This repair changes the updater that creates the grant. An older installed 2026.9.4 updater still creates an old-root grant when a later package relocates the generation across a schema version; the candidate receiver still refuses it. Those users need an updater containing this repair. Accepting legacy grants at the candidate receiver is separate compatibility work and is not claimed by this PR.

Thanks @xilopaint for the original report and @steipete for the landed staging fix and independent finalizer reproduction.

Co-authored-by: Ayaan Zaidi <hi@obviy.us>
This commit is contained in:
Ayaan Zaidi 2026-09-11 14:20:00 +05:30 • committed by GitHub
parent 6aab56d5c2
commit 68b7893b3c
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
10 changed files with 654 additions and 114 deletions
Download patch file Download diff file Expand all files Collapse all files

246
src/cli/update-cli/update-command-executor.test.ts
View file

@ -42,12 +42,12 @@ afterEach(() => {
vi.unstubAllEnvs();
});
function replaceOwner() {
function replaceOwner(installationRoot = root) {
const db = new DatabaseSync(path.join(temporary, "managed-update-handoffs.sqlite"));
try {
db.prepare("UPDATE managed_update_handoffs SET owner = ? WHERE install_root = ?").run(
"replacement",
root,
installationRoot,
);
} finally {
db.close();
@ -363,7 +363,7 @@ describe("candidate executor delegation", () => {
import {setTimeout} from "node:timers/promises";
import {withDelegatedUpdateCommandExecutor} from ${JSON.stringify(moduleUrl)};
const input=JSON.parse(fs.readFileSync(0,"utf8"));
await withDelegatedUpdateCommandExecutor(input.grant,input.grant.runId,input.grant.root,async (fence)=>{
await withDelegatedUpdateCommandExecutor(input.grant,input.grant.runId,input.root,async (fence)=>{
process.stdout.write("admitted\\n");
while(!fs.existsSync(input.proceed)) await setTimeout(10);
fence.assertCurrent();
@ -376,15 +376,25 @@ describe("candidate executor delegation", () => {
helper.unref();
});
`;
it.each([false, true])(
"retains parent exclusion through a real child (revoked=%s)",
async (revoked) => {
it.each([
{ changedRoot: false, revoked: false },
{ changedRoot: false, revoked: "candidate" },
{ changedRoot: true, revoked: false },
{ changedRoot: true, revoked: "candidate" },
{ changedRoot: true, revoked: "original" },
])(
"retains both installation owners through a real child ($changedRoot, $revoked)",
async ({ changedRoot, revoked }) => {
const candidateRoot = changedRoot ? path.join(root, "activated") : root;
if (changedRoot) {
fs.mkdirSync(candidateRoot);
}
const ready = createDeferred();
const proceed = path.join(root, "proceed");
const output = path.join(root, "effect");
const work = withUpdateCommandExecutor(randomUUID(), async (executor) => {
const fence = await executor.enter(root);
const pending = withUpdateCommandExecutorChild(fence, (grant, beforeInput) =>
const pending = withUpdateCommandExecutorChild(fence, candidateRoot, (grant, beforeInput) =>
runUtf8CommandWithTimeout(
[
process.execPath,
@ -395,7 +405,7 @@ describe("candidate executor delegation", () => {
program,
],
{
input: JSON.stringify({ grant, proceed, output }),
input: JSON.stringify({ grant, root: candidateRoot, proceed, output }),
beforeInput,
timeoutMs: 15_000,
killProcessTree: true,
@ -425,8 +435,11 @@ describe("candidate executor delegation", () => {
}
expect(store.release(primary.lease)).toBe(false);
expect(store.bind(primary.lease, process.pid)).toBeNull();
expect(store.acquire(candidateRoot, "other-candidate", { kind: "update" }).kind).toBe(
"busy",
);
if (revoked) {
replaceOwner();
replaceOwner(revoked === "original" ? root : candidateRoot);
}
} finally {
fs.writeFileSync(proceed, "continue");
@ -437,50 +450,79 @@ describe("candidate executor delegation", () => {
});
if (revoked) {
await expect(work).rejects.toThrow(/ownership|release/);
expect(fs.existsSync(output)).toBe(false);
// The shipped worker owns the activated generation; the parent still
// refuses completion if its independent recovery owner was replaced.
expect(fs.existsSync(output)).toBe(revoked === "original");
expect(
createManagedHandoffLeaseStore().read(revoked === "original" ? root : candidateRoot),
).toMatchObject({
kind: "current",
lease: { owner: "replacement" },
});
if (changedRoot && revoked === "candidate") {
expect(
createManagedHandoffLeaseStore().acquire(root, "next-original", { kind: "update" })
.kind,
).toBe("busy");
}
if (revoked === "original") {
expect(createManagedHandoffLeaseStore().read(candidateRoot)).toEqual({ kind: "absent" });
}
} else {
await work;
expect(fs.readFileSync(output, "utf8")).toBe("owned");
expect(createManagedHandoffLeaseStore().read(root)).toEqual({ kind: "absent" });
expect(createManagedHandoffLeaseStore().read(candidateRoot)).toEqual({ kind: "absent" });
}
},
);
it.skipIf(process.platform === "win32")(
"does not release installation ownership while a candidate descendant is alive",
async () => {
it.skipIf(process.platform === "win32").each([false, true])(
"does not release either installation while a candidate descendant is alive (changed root=%s)",
async (changedRoot) => {
const candidateRoot = changedRoot ? path.join(root, "activated") : root;
if (changedRoot) {
fs.mkdirSync(candidateRoot);
}
let descendant: number | undefined;
try {
await expect(
withUpdateCommandExecutor(randomUUID(), async (executor) => {
const fence = await executor.enter(root);
await withUpdateCommandExecutorChild(fence, async (grant, beforeInput) => {
const result = await runUtf8CommandWithTimeout(
[
process.execPath,
"-e",
`const fs=require('node:fs');const {spawn}=require('node:child_process');
await withUpdateCommandExecutorChild(
fence,
candidateRoot,
async (grant, beforeInput) => {
const result = await runUtf8CommandWithTimeout(
[
process.execPath,
"-e",
`const fs=require('node:fs');const {spawn}=require('node:child_process');
JSON.parse(fs.readFileSync(0,'utf8'));
const child=spawn(process.execPath,['-e',"setInterval(()=>{},1000);process.send('ready')"],{stdio:['ignore','ignore','ignore','ipc']});
child.once('message',()=>{process.stdout.write(String(child.pid));child.disconnect();child.unref();});`,
],
{
input: JSON.stringify(grant),
beforeInput,
killProcessTree: true,
timeoutMs: 15_000,
},
);
descendant = Number(result.stdout);
expect(result.code, result.stderr).toBe(0);
expect(Number.isSafeInteger(descendant) && descendant > 0).toBe(true);
process.kill(descendant, 0);
return result;
});
],
{
input: JSON.stringify(grant),
beforeInput,
killProcessTree: true,
timeoutMs: 15_000,
},
);
descendant = Number(result.stdout);
expect(result.code, result.stderr).toBe(0);
expect(Number.isSafeInteger(descendant) && descendant > 0).toBe(true);
process.kill(descendant, 0);
return result;
},
);
}),
).rejects.toThrow(/settled|release/);
const store = createManagedHandoffLeaseStore();
expect(store.acquire(root, "next-owner", { kind: "update" }).kind).toBe("busy");
expect(store.acquire(candidateRoot, "next-candidate", { kind: "update" }).kind).toBe(
"busy",
);
} finally {
if (descendant) {
process.kill(descendant, "SIGTERM");
@ -490,41 +532,115 @@ describe("candidate executor delegation", () => {
},
);
it("rejects a grant that does not match the stored parent generation", async () => {
const output = path.join(root, "effect");
await withUpdateCommandExecutor(randomUUID(), async (executor) => {
const fence = await executor.enter(root);
const result = await withUpdateCommandExecutorChild(fence, (grant, beforeInput) =>
runUtf8CommandWithTimeout(
[
process.execPath,
"--import",
path.resolve("scripts/tsx.mjs"),
"--input-type=module",
"-e",
program,
],
{
input: JSON.stringify({
grant: {
...grant,
parent: { ...grant.parent, updatedAt: grant.parent.updatedAt + 1 },
},
output,
}),
beforeInput,
timeoutMs: 15_000,
killProcessTree: true,
},
),
);
expect(result.code).not.toBe(0);
expect(result.stderr).toContain("does not match its parent");
it("does not expose a grant when another owner holds the activated installation", async () => {
const candidateRoot = path.join(root, "activated");
const output = path.join(root, "exposed-grant");
fs.mkdirSync(candidateRoot);
const store = createManagedHandoffLeaseStore();
const foreign = store.acquire(candidateRoot, "foreign-candidate", { kind: "update" });
assert(foreign.kind === "acquired", "Foreign candidate owner was not acquired");
try {
await expect(
withUpdateCommandExecutor(randomUUID(), async (executor) => {
const fence = await executor.enter(root);
await withUpdateCommandExecutorChild(fence, candidateRoot, async () => {
fs.writeFileSync(output, "exposed");
});
}),
).rejects.toThrow("owns the candidate installation");
expect(fs.existsSync(output)).toBe(false);
fence.assertCurrent();
});
expect(store.current(foreign.lease)).toBe(true);
expect(store.read(root)).toEqual({ kind: "absent" });
} finally {
expect(store.release(foreign.lease)).toBe(true);
}
});
it("withholds candidate input when the original owner changes before process binding", async () => {
const candidateRoot = path.join(root, "activated");
const output = path.join(root, "effect");
fs.mkdirSync(candidateRoot);
await expect(
withUpdateCommandExecutor(randomUUID(), async (executor) => {
const fence = await executor.enter(root);
await withUpdateCommandExecutorChild(fence, candidateRoot, (grant, beforeInput) =>
runUtf8CommandWithTimeout(
[
process.execPath,
"--import",
path.resolve("scripts/tsx.mjs"),
"--input-type=module",
"-e",
program,
],
{
input: JSON.stringify({ grant, root: candidateRoot, output }),
beforeInput: (pid) => {
replaceOwner();
beforeInput(pid);
},
timeoutMs: 15_000,
killProcessTree: true,
requireProcessTreeExtinction: true,
},
),
);
}),
).rejects.toThrow(/ownership|release/);
expect(fs.existsSync(output)).toBe(false);
expect(createManagedHandoffLeaseStore().read(root)).toMatchObject({
kind: "current",
lease: { owner: "replacement" },
});
expect(createManagedHandoffLeaseStore().read(candidateRoot)).toEqual({ kind: "absent" });
});
it.each([false, true])(
"rejects a changed parent grant and settles its child (changed root=%s)",
async (changedRoot) => {
const candidateRoot = changedRoot ? path.join(root, "activated") : root;
if (changedRoot) {
fs.mkdirSync(candidateRoot);
}
const output = path.join(root, "effect");
await withUpdateCommandExecutor(randomUUID(), async (executor) => {
const fence = await executor.enter(root);
const result = await withUpdateCommandExecutorChild(
fence,
candidateRoot,
(grant, beforeInput) =>
runUtf8CommandWithTimeout(
[
process.execPath,
"--import",
path.resolve("scripts/tsx.mjs"),
"--input-type=module",
"-e",
program,
],
{
input: JSON.stringify({
grant: {
...grant,
parent: { ...grant.parent, updatedAt: grant.parent.updatedAt + 1 },
},
output,
root: candidateRoot,
}),
beforeInput,
timeoutMs: 15_000,
killProcessTree: true,
},
),
);
expect(result.code).not.toBe(0);
expect(result.stderr).toContain("does not match its parent");
expect(fs.existsSync(output)).toBe(false);
fence.assertCurrent();
});
},
);
it.skipIf(process.platform === "win32")(
"retains a candidate group after both the updater and its direct child exit",
async () => {

98
src/cli/update-cli/update-command-executor.ts
View file

@ -63,18 +63,19 @@ type ChildOperation<T> = (
) => Promise<T>;
const childOwners = new WeakMap<
UpdateRecoveryFence,
<T>(operation: ChildOperation<T>) => Promise<T>
<T>(root: string, operation: ChildOperation<T>) => Promise<T>
>();
export async function withUpdateCommandExecutorChild<T>(
fence: UpdateRecoveryFence,
root: string,
operation: ChildOperation<T>,
): Promise<T> {
const owner = childOwners.get(fence);
if (!owner) {
throw new UpdateCommandRecoveryPendingError("Child continuation requires its live executor.");
}
return await owner(operation);
return await owner(root, operation);
}
/** A child owns its separate lease while the original installation lease remains
@ -190,7 +191,10 @@ export async function withUpdateCommandExecutor<T>(
const fence = { assertCurrent };
childOwners.set(
fence,
<ChildResult>(childOperation: ChildOperation<ChildResult>): Promise<ChildResult> => {
<ChildResult>(
root: string,
childOperation: ChildOperation<ChildResult>,
): Promise<ChildResult> => {
assertCurrent();
if (!childAdmissionOpen || !store || !lease || !databasePath) {
throw new UpdateCommandRecoveryPendingError("Child executor admission is closed.");
@ -198,40 +202,73 @@ export async function withUpdateCommandExecutor<T>(
preflightReleases.delete(fence);
const control = store;
const original = lease;
const acquired = control.acquire(
`${original.key}/.openclaw-update-child-${randomUUID()}`,
runId,
{ kind: "update" },
);
if (acquired.kind !== "acquired") {
throw new UpdateCommandRecoveryPendingError("Candidate lifetime could not be acquired.");
}
let childLease = acquired.lease;
const authority = captureUpdateCommandExecutorAuthority(fence);
const candidateRoot = resolveUpdateInstallRoot(root);
let candidateParent = original;
const children: ManagedHandoffLease[] = [];
let bound = false;
delegating = true;
const grant: UpdateCommandChildGrant = {
runId,
root: original.key,
databasePath,
parent: original,
childKey: childLease.key,
databaseIdentity: admittedAuthorities.get(fence),
const assertOwners = () => {
assertBase();
if (
!control.owns(candidateParent, "executor") ||
resolveUpdateInstallRoot(root) !== candidateParent.key
) {
throw new UpdateCommandRecoveryPendingError("Candidate installation ownership changed.");
}
};
const running = async () => {
let outcome: { result: ChildResult } | { error: Error };
try {
assertBase();
if (candidateRoot !== original.key) {
const acquired = control.acquire(candidateRoot, randomUUID(), { kind: "update" });
if (acquired.kind !== "acquired") {
throw new UpdateCommandRecoveryPendingError(
"Another update executor owns the candidate installation.",
);
}
candidateParent = acquired.lease;
}
assertOwners();
// The original child row keeps recovery exclusion after the updater dies.
// A replaced generation also needs the shipped worker's active-root binding.
const parents = candidateParent === original ? [original] : [original, candidateParent];
for (const parent of parents) {
const acquired = control.acquire(
`${parent.key}/.openclaw-update-child-${randomUUID()}`,
runId,
{ kind: "update" },
);
if (acquired.kind !== "acquired") {
throw new UpdateCommandRecoveryPendingError(
"Candidate lifetime could not be acquired.",
);
}
children.push(acquired.lease);
}
const grant: UpdateCommandChildGrant = {
runId,
root: candidateParent.key,
databasePath: authority.databasePath,
parent: candidateParent,
childKey: children[children.length - 1]!.key,
databaseIdentity: authority,
};
const result = await childOperation(grant, (pid) => {
assertBase();
assertOwners();
if (bound || pid === process.pid) {
throw new UpdateCommandRecoveryPendingError(
"Candidate process can be bound only once.",
);
}
const assigned = control.bind(childLease, pid);
if (!assigned) {
throw new UpdateCommandRecoveryPendingError("Candidate process binding failed.");
for (let index = 0; index < children.length; index++) {
const assigned = control.bind(children[index]!, pid);
if (!assigned) {
throw new UpdateCommandRecoveryPendingError("Candidate process binding failed.");
}
children[index] = assigned;
}
childLease = assigned;
bound = true;
});
if (!bound) {
@ -239,7 +276,7 @@ export async function withUpdateCommandExecutor<T>(
"Candidate continuation did not bind a process.",
);
}
assertBase();
assertOwners();
outcome = { result };
} catch (cause) {
outcome = {
@ -248,8 +285,15 @@ export async function withUpdateCommandExecutor<T>(
}
try {
// Release refuses a live child. Never reactivate the parent on timeout
// until the process owner has actually joined the candidate.
if (!control.release(childLease)) {
// until the process owner has actually joined the candidate. Keep the
// original child until active-generation cleanup is also confirmed.
if (children.length > 1 && !control.release(children[1]!)) {
throw new UpdateCommandRecoveryPendingError("Candidate executor has not settled.");
}
if (candidateParent !== original && !control.release(candidateParent)) {
throw new UpdateCommandRecoveryPendingError("Candidate installation release failed.");
}
if (children.length > 0 && !control.release(children[0]!)) {
throw new UpdateCommandRecoveryPendingError("Candidate executor has not settled.");
}
delegating = false;

2
src/cli/update-cli/update-command-migrated.ts
View file

@ -234,7 +234,7 @@ export async function continueMigratedUpdateInFreshProcess(
maxOutputBytes: 1024 * 1024,
});
const child = executorFence
? await withUpdateCommandExecutorChild(executorFence, runChild)
? await withUpdateCommandExecutorChild(executorFence, root, runChild)
: await runChild();
if (child.stdout) {
process.stdout.write(child.stdout);

24
src/cli/update-cli/update-command-post-update-recovery.test.ts
View file

@ -37,6 +37,14 @@ const mocks = vi.hoisted(() => ({
async () => "ok",
),
stopCandidate: vi.fn(),
revalidateService: vi.fn<
typeof import("./update-command-service-maintenance.js").revalidateManagedGatewayServiceAfterUpdate
>(async ({ root }) => ({
kind: "owned",
root,
fingerprint: "fixture",
refreshDefinition: false,
})),
restart:
vi.fn<
typeof import("./update-command-service.js").maybeRestartServiceAfterFailedMutableUpdate
@ -75,6 +83,10 @@ vi.mock("../../daemon/service.js", async (importOriginal) => ({
command: { programArguments: ["node", "/repo/dist/entry.js", "gateway"] },
}),
}));
vi.mock("./update-command-service-maintenance.js", async (importOriginal) => ({
...(await importOriginal<typeof import("./update-command-service-maintenance.js")>()),
revalidateManagedGatewayServiceAfterUpdate: mocks.revalidateService,
}));
vi.mock("./update-command-service.js", async (importOriginal) => ({
...(await importOriginal<typeof import("./update-command-service.js")>()),
maybeRestartServiceAfterFailedMutableUpdate: mocks.restart,
@ -82,12 +94,7 @@ vi.mock("./update-command-service.js", async (importOriginal) => ({
maybeRestartService: mocks.restartCandidate,
maybeStopManagedServiceBeforeMutableUpdate: mocks.stopCandidate,
resolveUpdatedGatewayRestartPort: async () => 19101,
revalidateManagedGatewayServiceAfterUpdate: async () => ({
kind: "owned",
root: "/repo",
fingerprint: "fixture",
refreshDefinition: false,
}),
revalidateManagedGatewayServiceAfterUpdate: mocks.revalidateService,
}));
vi.mock("./update-command-post-core.js", async (importOriginal) => ({
...(await importOriginal<typeof import("./update-command-post-core.js")>()),
@ -806,7 +813,10 @@ describe("failed package update recovery safety", () => {
return;
}
expect(failure.result).toMatchObject({ root: originalRoot, after: { version: "2026.9.1" } });
expect(mocks.restartCandidate.mock.lastCall?.[0]).toMatchObject({
expect(
mocks.restartCandidate.mock.lastCall?.[0],
JSON.stringify(failure.result),
).toMatchObject({
result: { root: originalRoot, after: { version: "2026.9.1" } },
});
expect(rollback).toHaveBeenCalledOnce();

31
src/cli/update-cli/update-command-rollback.test.ts
View file

@ -25,13 +25,23 @@ import { createWindowsTaskAutoStartRecovery } from "./update-command-windows-tas
const mocks = vi.hoisted(() => ({
stop: vi.fn(),
restart: vi.fn<typeof import("./update-command-service.js").maybeRestartService>(),
serviceState: vi.fn<typeof import("../../daemon/service.js").readGatewayServiceState>(),
revalidateService:
vi.fn<
typeof import("./update-command-service-maintenance.js").revalidateManagedGatewayServiceAfterUpdate
>(),
reachable: vi.fn(),
execSchtasks: vi.fn<typeof import("../../daemon/schtasks-exec.js").execSchtasks>(),
}));
vi.mock("../../daemon/schtasks-exec.js", () => ({ execSchtasks: mocks.execSchtasks }));
vi.mock("../../daemon/service.js", async (importOriginal) => ({
...(await importOriginal<typeof import("../../daemon/service.js")>()),
readGatewayServiceState: mocks.serviceState,
}));
vi.mock("./update-command-service-maintenance.js", async (importOriginal) => ({
...(await importOriginal<typeof import("./update-command-service-maintenance.js")>()),
createWindowsTaskAutoStartGuard: () => async () => {},
revalidateManagedGatewayServiceAfterUpdate: mocks.revalidateService,
}));
vi.mock("./update-command-service-command.js", async (importOriginal) => ({
...(await importOriginal<typeof import("./update-command-service-command.js")>()),
@ -97,6 +107,25 @@ describe("verified package rollback", () => {
`import ${JSON.stringify(pathToFileURL(path.resolve(worker)).href)};\n`,
);
vi.resetAllMocks();
mocks.serviceState.mockResolvedValue({
installed: true,
loadState: { status: "loaded" },
running: false,
env: {},
command: {
programArguments: [
process.execPath,
path.join(previousRoot, "dist", "index.js"),
"gateway",
],
},
});
mocks.revalidateService.mockResolvedValue({
kind: "owned",
root: previousRoot,
fingerprint: "fixture",
refreshDefinition: true,
});
mocks.reachable.mockResolvedValue({ reachable: true });
mocks.stop.mockResolvedValue({
stopped: true,
@ -667,7 +696,7 @@ describe("verified package rollback", () => {
resolveUpdateResultNextAction({ result: outcome.result, env: process.env }),
).toContain(configPath);
}
expect(outcome.rolledBack).toBe(restored);
expect(outcome.rolledBack, JSON.stringify(outcome)).toBe(restored);
expect(rollback, JSON.stringify(outcome)).toHaveBeenCalledTimes(
change === "none" ||
change === "readonly-config" ||

23
src/cli/update-cli/update-command-rollback.ts
View file

@ -10,6 +10,7 @@ import {
import { withConfigMutationLock } from "../../config/mutate.js";
import { resolveStateDir } from "../../config/paths.js";
import type { ConfigFileSnapshot } from "../../config/types.openclaw.js";
import { readGatewayServiceState, resolveGatewayService } from "../../daemon/service.js";
import { formatErrorMessage } from "../../infra/errors.js";
import type { PackageUpdateTransaction } from "../../infra/package-update-steps.js";
import { replaceFileAtomic } from "../../infra/replace-file.js";
@ -34,7 +35,11 @@ import {
import { readPackageUpdateIdentity } from "./update-command-package.js";
import { runUpdatedInstallGatewayCommand } from "./update-command-service-command.js";
import { withOwnedManagedUpdateEnv } from "./update-command-service-env.js";
import { createWindowsTaskAutoStartGuard } from "./update-command-service-maintenance.js";
import {
createWindowsTaskAutoStartGuard,
revalidateManagedGatewayServiceAfterUpdate,
} from "./update-command-service-maintenance.js";
import { assertGatewayServiceManagementAllowedForUpdate } from "./update-command-service-plan.js";
import {
maybeRestartService,
maybeResumeWindowsTaskAutoStartAfterPackageUpdate,
@ -241,6 +246,7 @@ export async function rollbackFailedUpdate(params: {
shouldRestart: true,
jsonMode: opts.json === true,
expectedService: before,
allowInstallRootChange: packageTransaction !== undefined,
timeoutMs: params.timeoutMs,
}),
);
@ -395,7 +401,7 @@ export async function rollbackFailedUpdate(params: {
assertCurrent();
// A failed candidate does not authorize its restart. The previous package's
// pre-activation verification authorizes restarting this schema-neutral restoration.
const verdict = stopped.serviceUpdateVerdict ?? before?.serviceUpdateVerdict;
let verdict = stopped.serviceUpdateVerdict ?? before?.serviceUpdateVerdict;
const nodeRunner = before?.serviceNodeRunner ?? params.nodeRunner;
if (verdict?.kind === "owned" && verdict.refreshDefinition) {
await runUpdatedInstallGatewayCommand(
@ -411,6 +417,19 @@ export async function rollbackFailedUpdate(params: {
},
"install",
);
const state = await readGatewayServiceState(resolveGatewayService(), {
env: recoveryEnv,
requireEffective: true,
requireLoadedCommand: true,
validateEnvBeforeStatusRead: assertGatewayServiceManagementAllowedForUpdate,
timeoutMs: params.timeoutMs,
});
assertCurrent();
verdict = await revalidateManagedGatewayServiceAfterUpdate({
state,
root: params.previousRoot,
preManagedServiceStop: stopped,
});
}
assertCurrent();
result.recovery = {

15
src/cli/update-cli/update-command-service-maintenance.ts
View file

@ -332,6 +332,7 @@ type ManagedServiceStopParams = {
PreManagedServiceStop,
"serviceEnv" | "serviceUpdateVerdict" | "serviceManagerUid"
>;
allowInstallRootChange?: boolean;
onStopped?: (state: PreManagedServiceStop) => void;
timeoutMs?: number;
};
@ -438,6 +439,7 @@ async function stopManagedServiceBeforeMutableUpdate(
root: params.root,
state: serviceState,
preManagedServiceStop: params.expectedService,
allowInstallRootChange: params.allowInstallRootChange,
});
assertCurrent();
if (params.phase) {
@ -573,18 +575,13 @@ async function stopManagedServiceBeforeMutableUpdate(
validateEnvBeforeStatusRead: assertGatewayServiceManagementAllowedForUpdate,
timeoutMs: params.timeoutMs,
});
await revalidateManagedGatewayServiceAfterUpdate({
const currentVerdict = await revalidateManagedGatewayServiceAfterUpdate({
state: currentState,
root: params.root,
preManagedServiceStop: {
serviceManagerUid: inspected.serviceManagerUid,
serviceEnv: serviceState.env,
serviceUpdateVerdict:
serviceUpdateVerdict.kind === "owned"
? { ...serviceUpdateVerdict, refreshDefinition: false }
: serviceUpdateVerdict,
},
preManagedServiceStop: inspected,
allowInstallRootChange: params.allowInstallRootChange,
});
assertGatewayServiceAdmissionUnchanged(inspected, currentVerdict);
assertCurrent();
const currentBlockMessage = await resolveAncestryBlock(currentState);
if (currentBlockMessage) {

321
src/cli/update-cli/update-command-service-rollback.test-support.ts Normal file
View file

@ -0,0 +1,321 @@
import fs from "node:fs/promises";
import path from "node:path";
import { expect, it, vi, type Mock } from "vitest";
import { readConfigFileSnapshot } from "../../config/config.js";
import { resolveStateDir } from "../../config/paths.js";
import { writePackageRoot } from "../../infra/package-update-steps.test-support.js";
import {
swapStagedPackageInstall,
type PackageUpdateTransaction,
} from "../../infra/package-update-swap.js";
import * as candidateState from "../../infra/update-candidate-state.js";
import type { ResolvedGlobalInstallTarget } from "../../infra/update-global.js";
import { prepareNativePackageStage } from "../../infra/update-native-package-stage.js";
import { VERSION } from "../../version.js";
import { rollbackFailedUpdate } from "./update-command-rollback.js";
import type { PreManagedServiceStop } from "./update-command-service-context-types.js";
import type { InstallRootTransitionFixture } from "./update-command-service-transition.test-support.js";
import { maybeStopManagedServiceBeforeMutableUpdate } from "./update-command-service.js";
export function registerPackageRootRollbackTests(
getFixture: () => InstallRootTransitionFixture & {
mocks: { managerUid: number | undefined; error: Mock };
},
) {
it.each([
"removed",
"retained",
"refreshed",
"running original",
"changed command",
"changed manager",
"unavailable manager",
"sealed definition",
"foreign command",
"changed manager during install",
"foreign command during install",
] as const)("rolls back a pnpm generation with %s service ownership", async (scenario) => {
const { root, run, mocks } = getFixture();
const changesDuringInstall =
scenario === "changed manager during install" ||
scenario === "foreign command during install";
// A removed group must not resolve to the fixture's enclosing package.
await fs.rm(path.join(root, "package.json"));
const globalRoot = path.join(root, "pnpm", "global", "v11");
const previousOwner = path.join(globalRoot, "previous");
const previousRoot = path.join(previousOwner, "node_modules", "openclaw");
const candidateRoot = path.join(globalRoot, "candidate", "node_modules", "openclaw");
const binDir = path.join(root, "bin");
await writePackageRoot(previousRoot, VERSION);
await fs.writeFile(
path.join(previousOwner, "package.json"),
JSON.stringify({ dependencies: { openclaw: VERSION } }),
);
await fs.symlink("previous", path.join(globalRoot, "active-openclaw"));
await fs.mkdir(binDir);
await fs.writeFile(path.join(binDir, "openclaw"), "previous launcher\n");
const command = {
programArguments: [
process.execPath,
path.join(previousRoot, "dist", "index.js"),
"gateway",
"--port",
"19305",
],
environment: { HOME: root },
};
mocks.command.mockResolvedValue(command);
mocks.capability.mockResolvedValue({ kind: "writable" });
// Keep real schema/config comparisons without starting a package worker in this service fixture.
vi.spyOn(candidateState, "readUpdateStateSchemaVersions").mockImplementation(
candidateState.readUpdateStateSchemaVersionsInProcess,
);
const configSnapshot = await readConfigFileSnapshot({ skipPluginValidation: true });
const schemaVersions = await candidateState.readUpdateStateSchemaVersions({
stateDir: resolveStateDir(run.env),
config: configSnapshot.sourceConfig,
env: run.env,
});
const installTarget: ResolvedGlobalInstallTarget = {
manager: "pnpm",
command: "pnpm",
globalRoot,
packageRoot: previousRoot,
pnpmIsolated: { layoutVersion: 11 },
};
const native = await prepareNativePackageStage({
installTarget,
packageName: "openclaw",
installSpec: "openclaw@9999.1.1",
globalBinDir: binDir,
env: {},
});
if (!native) {
throw new Error("native stage missing");
}
const stagedOwner = path.join(native.globalRoot, "candidate");
const stagedRoot = path.join(stagedOwner, "node_modules", "openclaw");
await writePackageRoot(stagedRoot, "9999.1.1");
await fs.writeFile(
path.join(stagedOwner, "package.json"),
JSON.stringify({ dependencies: { openclaw: "9999.1.1" } }),
);
await fs.unlink(path.join(native.globalRoot, "active-openclaw"));
await fs.symlink("candidate", path.join(native.globalRoot, "active-openclaw"));
if (scenario !== "retained") {
await fs.rm(path.join(native.globalRoot, "previous"), { recursive: true });
}
await fs.symlink(
path.relative(native.binDir, path.join(stagedRoot, "dist", "index.js")),
path.join(native.binDir, "openclaw"),
);
let before: PreManagedServiceStop | undefined;
let transaction: PackageUpdateTransaction | undefined;
const swap = await swapStagedPackageInstall({
stage: {
prefix: native.projectRoot,
layout: {
prefix: native.projectRoot,
globalRoot: native.globalRoot,
binDir: native.binDir,
},
packageRoot: stagedRoot,
installTarget: { ...installTarget, globalRoot: native.globalRoot, packageRoot: stagedRoot },
native,
},
installTarget,
packageName: "openclaw",
beforeActivate: async () => {
before = await maybeStopManagedServiceBeforeMutableUpdate({
root: previousRoot,
updateInstallKind: "package",
shouldRestart: true,
jsonMode: true,
updateRun: run,
});
},
onTransaction: (retained) => {
transaction = retained;
},
});
expect(swap.status).toBe("committed");
if (!before || !transaction) {
throw new Error("retained package and service ownership missing");
}
expect(before.stopped).toBe(true);
const refreshed = scenario === "refreshed" || changesDuringInstall;
mocks.running =
refreshed || ["running original", "changed command", "changed manager"].includes(scenario);
const currentCommand = refreshed
? {
...command,
programArguments: [
process.execPath,
path.join(candidateRoot, "dist", "index.js"),
"gateway",
"--port",
"19305",
],
}
: command;
const foreignRoot = path.join(root, "foreign");
if (scenario === "foreign command" || scenario === "foreign command during install") {
await writePackageRoot(foreignRoot, VERSION);
}
let reads = 0;
let changedDuringStop = false;
mocks.command.mockImplementation(async () => {
reads++;
if (reads === 2 && (scenario === "changed command" || scenario === "changed manager")) {
changedDuringStop = true;
}
if (scenario === "changed manager" && reads === 2) {
mocks.managerUid = 3002;
}
if (scenario === "foreign command") {
return {
...currentCommand,
programArguments: [
process.execPath,
path.join(foreignRoot, "dist", "index.js"),
"gateway",
],
};
}
return scenario === "changed command" && reads >= 2
? { ...currentCommand, programArguments: [...currentCommand.programArguments, "--verbose"] }
: currentCommand;
});
if (scenario === "unavailable manager") {
mocks.managerUid = undefined;
} else if (scenario === "sealed definition") {
mocks.capability.mockResolvedValue({ kind: "sealed", reason: "foreign-owner" });
}
mocks.configSnapshot.mockResolvedValue(undefined);
mocks.child.mockImplementation(async (argv) => {
expect(argv[1]).toBe(path.join(previousRoot, "dist", "index.js"));
expect(await fs.readFile(path.join(previousRoot, "package.json"), "utf8")).toContain(VERSION);
if (argv.includes("install")) {
mocks.command.mockResolvedValue(command);
if (scenario === "changed manager during install") {
mocks.managerUid = 3002;
} else if (scenario === "foreign command during install") {
mocks.command.mockResolvedValue({
...command,
programArguments: [
process.execPath,
path.join(foreignRoot, "dist", "index.js"),
"gateway",
],
});
}
} else if (argv.includes("restart")) {
mocks.running = true;
} else {
throw new Error("unexpected rollback subprocess");
}
return {
code: 0,
stdout: JSON.stringify({ action: "restart", ok: true, result: "restarted" }),
stderr: "",
signal: null,
killed: false,
termination: "exit",
};
});
const outcome = await rollbackFailedUpdate({
result: {
status: "error",
reason: "doctor-failed",
mode: "pnpm",
root: candidateRoot,
before: { version: VERSION },
after: { version: "9999.1.1" },
steps: [
{
name: "openclaw doctor",
command: "doctor",
cwd: candidateRoot,
durationMs: 1,
exitCode: 73,
},
],
durationMs: 1,
},
previousRoot,
packageTransaction: transaction,
schemaVersions,
previousVerified: true,
configSnapshot,
opts: { json: true, run },
preManagedServiceStop: before,
timeoutMs: 1000,
nodeRunner: process.execPath,
});
const refused = [
"changed command",
"changed manager",
"unavailable manager",
"sealed definition",
"foreign command",
].includes(scenario);
if (changesDuringInstall) {
expect(outcome.rolledBack).toBe(false);
expect(outcome.result).toMatchObject({
reason: "service-revalidation-failed",
root: previousRoot,
recovery: { packageRollbackVerified: true },
});
expect(await fs.readFile(path.join(previousRoot, "package.json"), "utf8")).toContain(VERSION);
expect(await fs.readFile(path.join(binDir, "openclaw"), "utf8")).toBe("previous launcher\n");
await expect(fs.stat(candidateRoot)).rejects.toMatchObject({ code: "ENOENT" });
expect(mocks.child).toHaveBeenCalledOnce();
expect(mocks.child.mock.calls[0]?.[0]).toContain("install");
expect(mocks.running).toBe(false);
expect(mocks.events.filter((event) => event === "native stop")).toHaveLength(2);
} else if (refused) {
if (scenario === "changed command" || scenario === "changed manager") {
expect(changedDuringStop).toBe(true);
}
expect(outcome.rolledBack).toBe(false);
expect(outcome.result.reason).toBe("service-revalidation-failed");
expect(await fs.readFile(path.join(candidateRoot, "package.json"), "utf8")).toContain(
"9999.1.1",
);
expect(
await fs.readFile(
path.join(
transaction.backupRoot,
"v11",
"previous",
"node_modules",
"openclaw",
"package.json",
),
"utf8",
),
).toContain(VERSION);
expect(mocks.events.filter((event) => event === "native stop")).toHaveLength(1);
expect(mocks.child).not.toHaveBeenCalled();
} else {
expect(
outcome.rolledBack,
JSON.stringify({ outcome, errors: mocks.error.mock.calls }, null, 2),
).toBe(true);
expect(outcome.result).toMatchObject({
status: "error",
reason: "doctor-failed",
root: previousRoot,
after: { version: VERSION },
recovery: { packageRollbackVerified: true, service: "healthy" },
});
expect(await fs.readFile(path.join(previousRoot, "package.json"), "utf8")).toContain(VERSION);
expect(await fs.readFile(path.join(binDir, "openclaw"), "utf8")).toBe("previous launcher\n");
expect(mocks.running).toBe(true);
expect(mocks.events.filter((event) => event === "native stop")).toHaveLength(
scenario === "refreshed" || scenario === "running original" ? 2 : 1,
);
await transaction.complete({ activationVerified: false }, () => {});
}
});
}

2
src/cli/update-cli/update-command-service-transition.test-support.ts
View file

@ -17,7 +17,7 @@ import {
revalidateManagedGatewayServiceAfterUpdate,
} from "./update-command-service.js";
type InstallRootTransitionFixture = {
export type InstallRootTransitionFixture = {
root: string;
run: NonNullable<UpdateCommandOptions["run"]>;
mocks: {

6
src/cli/update-cli/update-command-service.integration.test.ts
View file

@ -30,6 +30,7 @@ import {
registerRecoveryTests,
writeRecoveryConfig,
} from "./update-command-service-recovery.test-support.js";
import { registerPackageRootRollbackTests } from "./update-command-service-rollback.test-support.js";
import {
registerInstallRootTransitionTests,
registerPluginMaintenanceTests,
@ -51,6 +52,7 @@ const mocks = vi.hoisted(() => ({
terminateStale: vi.fn(async (pids: number[]) => pids),
running: true,
loaded: true,
managerUid: 2001 as number | undefined,
listenerPids: vi.fn(() => [4242]),
ports: vi.fn<typeof import("../../infra/ports-inspect.js").inspectPortUsage>(),
call: vi.fn<(opts: import("../../gateway/call.js").CallGatewayOptions) => Promise<unknown>>(),
@ -122,7 +124,7 @@ vi.mock("../../daemon/systemd.js", async (importOriginal) => ({
...(await importOriginal<typeof import("../../daemon/systemd.js")>()),
readSystemdServiceExecStart: mocks.command,
readSystemdServiceRuntime: async () => ({
systemd: { managerUid: 2001 },
systemd: { managerUid: mocks.managerUid },
status: mocks.running ? "running" : "stopped",
...(mocks.running ? { pid: 4242 } : {}),
}),
@ -214,6 +216,7 @@ beforeEach(async () => {
);
mocks.running = true;
mocks.loaded = true;
mocks.managerUid = 2001;
mocks.inLaunchd = false;
mocks.launchctl.mockImplementation(async () => {
throw new Error("Unexpected native control in fixture");
@ -569,6 +572,7 @@ describe("preserved update activation with real version guards", () => {
registerGenerationRecoveryTests(() => ({ root, configPath, mocks }));
registerInstallRootTransitionTests(() => ({ root, run, mocks }));
registerPackageRootRollbackTests(() => ({ root, run, mocks }));
it.each(["metadata", "profile", "unit"])(
"pins writable service identity across %s changes",