From 1b251a6af1c715ad8f37ad77417f30e243e1c971 Mon Sep 17 00:00:00 2001 From: masatohoshino Date: Sun, 7 Jun 2026 23:38:22 +0900 Subject: [PATCH] fix(update): harden post-core finalize (service-env strip, no-op skip) - Strip gateway service identity (OPENCLAW_SERVICE_MARKER/KIND/PID) from the finalizer child so it is not mistaken for the managed service, matching the CLI post-core spawn. - Skip finalize for no-op git updates (unchanged SHA and version), mirroring the CLI resume gate, to avoid an unnecessary doctor/convergence run. --- src/infra/update-post-core-finalize.test.ts | 38 +++++++++++++++++- src/infra/update-post-core-finalize.ts | 44 ++++++++++++++++++--- 2 files changed, 75 insertions(+), 7 deletions(-) diff --git a/src/infra/update-post-core-finalize.test.ts b/src/infra/update-post-core-finalize.test.ts index b842671adda..10508becab2 100644 --- a/src/infra/update-post-core-finalize.test.ts +++ b/src/infra/update-post-core-finalize.test.ts @@ -41,6 +41,20 @@ describe("runPostCoreFinalizeAfterGatewayUpdate", () => { expect(spawnFinalize).not.toHaveBeenCalled(); }); + it("skips a no-op git update with no core change", async () => { + const spawnFinalize = vi.fn(); + const outcome = await runPostCoreFinalizeAfterGatewayUpdate({ + result: gitOkResult({ + before: { sha: "same", version: "2026.6.1" }, + after: { sha: "same", version: "2026.6.1" }, + }), + resolveEntrypoint: resolveEntrypointOk, + spawnFinalize, + }); + expect(outcome).toEqual({ status: "skipped", reason: "not-git-update" }); + expect(spawnFinalize).not.toHaveBeenCalled(); + }); + it("skips when no built entrypoint is found", async () => { const spawnFinalize = vi.fn(); const outcome = await runPostCoreFinalizeAfterGatewayUpdate({ @@ -63,7 +77,7 @@ describe("runPostCoreFinalizeAfterGatewayUpdate", () => { }); expect(outcome).toEqual({ status: "ok", entrypoint: ENTRYPOINT }); expect(spawnFinalize).toHaveBeenCalledTimes(1); - const call = spawnFinalize.mock.calls[0]![0]; + const call = spawnFinalize.mock.calls[0][0]; // Reconcile runs through the designed finalizer; never restarts (RPC owns restart). expect(call.argv).toEqual([ expect.any(String), @@ -82,6 +96,26 @@ describe("runPostCoreFinalizeAfterGatewayUpdate", () => { expect(call.env.OPENCLAW_COMPATIBILITY_HOST_VERSION).toBe("2026.6.1"); }); + it("strips the gateway service identity from the finalizer child env", async () => { + const spawnFinalize = vi.fn(async () => ({ code: 0 })); + await runPostCoreFinalizeAfterGatewayUpdate({ + result: gitOkResult(), + resolveEntrypoint: resolveEntrypointOk, + spawnFinalize, + env: { + PATH: "/usr/bin", + OPENCLAW_SERVICE_MARKER: "openclaw", + OPENCLAW_SERVICE_KIND: "gateway", + OPENCLAW_GATEWAY_SERVICE_PID: "4242", + }, + }); + const { env } = spawnFinalize.mock.calls[0][0]; + expect(env.PATH).toBe("/usr/bin"); + expect(env.OPENCLAW_SERVICE_MARKER).toBeUndefined(); + expect(env.OPENCLAW_SERVICE_KIND).toBeUndefined(); + expect(env.OPENCLAW_GATEWAY_SERVICE_PID).toBeUndefined(); + }); + it("omits channel/timeout flags when not provided", async () => { const spawnFinalize = vi.fn(async () => ({ code: 0 })); await runPostCoreFinalizeAfterGatewayUpdate({ @@ -89,7 +123,7 @@ describe("runPostCoreFinalizeAfterGatewayUpdate", () => { resolveEntrypoint: resolveEntrypointOk, spawnFinalize, }); - const argv = spawnFinalize.mock.calls[0]![0].argv; + const argv = spawnFinalize.mock.calls[0][0].argv; expect(argv).not.toContain("--channel"); expect(argv).not.toContain("--timeout"); }); diff --git a/src/infra/update-post-core-finalize.ts b/src/infra/update-post-core-finalize.ts index b3dc5348ab7..cad339af520 100644 --- a/src/infra/update-post-core-finalize.ts +++ b/src/infra/update-post-core-finalize.ts @@ -17,6 +17,7 @@ // and `runPostCorePluginConvergence`). Finalization never restarts, so the RPC // handler keeps ownership of the gateway restart. import path from "node:path"; +import { GATEWAY_SERVICE_RUNTIME_PID_ENV } from "../daemon/constants.js"; import { resolveGatewayInstallEntrypoint } from "../daemon/gateway-entrypoint.js"; import { runCommandWithTimeout } from "../process/exec.js"; import { resolveStableNodePath } from "./stable-node-path.js"; @@ -25,6 +26,22 @@ import type { UpdateRunResult } from "./update-runner.js"; const DEFAULT_FINALIZE_TIMEOUT_MS = 30 * 60_000; +// Strip the running gateway's service identity from the finalizer child so it is +// not mistaken for the managed service process (matches the CLI post-core spawn). +function buildFinalizeEnv( + baseEnv: NodeJS.ProcessEnv, + compatHostVersion?: string, +): NodeJS.ProcessEnv { + const env: NodeJS.ProcessEnv = { ...baseEnv }; + delete env.OPENCLAW_SERVICE_MARKER; + delete env.OPENCLAW_SERVICE_KIND; + delete env[GATEWAY_SERVICE_RUNTIME_PID_ENV]; + if (compatHostVersion) { + env.OPENCLAW_COMPATIBILITY_HOST_VERSION = compatHostVersion; + } + return env; +} + export type PostCoreFinalizeOutcome = | { status: "skipped"; reason: "not-git-update" | "entrypoint-missing" } | { status: "ok"; entrypoint: string } @@ -50,6 +67,25 @@ const defaultFinalizeSpawner: PostCoreFinalizeSpawner = async ({ argv, cwd, time return { code: res.code, ...(res.stderr ? { stderr: res.stderr } : {}) }; }; +function normalizeOptionalString(value: string | null | undefined): string | undefined { + const trimmed = value?.trim(); + return trimmed ? trimmed : undefined; +} + +// A no-op git update (same SHA and version) has nothing new to converge against, +// so skip finalize to avoid an unnecessary doctor/convergence run. Mirrors the +// CLI's `shouldResumePostCoreUpdateInFreshProcess` git resume gate. +function gitCoreChanged(result: UpdateRunResult): boolean { + const beforeSha = normalizeOptionalString(result.before?.sha); + const afterSha = normalizeOptionalString(result.after?.sha); + if (beforeSha && afterSha && beforeSha !== afterSha) { + return true; + } + const beforeVersion = normalizeOptionalString(result.before?.version); + const afterVersion = normalizeOptionalString(result.after?.version); + return Boolean(beforeVersion && afterVersion && beforeVersion !== afterVersion); +} + // Only git/source updates routed through `runGatewayUpdate` defer-and-drop // plugin convergence. Package-manager/global installs already converge because // the RPC routes them through `startManagedServiceUpdateHandoff`, which @@ -61,7 +97,8 @@ function isGitUpdateNeedingFinalize( result.status === "ok" && result.mode === "git" && typeof result.root === "string" && - result.root.length > 0 + result.root.length > 0 && + gitCoreChanged(result) ); } @@ -123,10 +160,7 @@ export async function runPostCoreFinalizeAfterGatewayUpdate(params: { // Pin the finalizer's host-compat resolution to the just-installed core // version so plugins reconcile against the new core, not the running process. const compatHostVersion = result.after?.version ?? undefined; - const baseEnv = params.env ?? process.env; - const env: NodeJS.ProcessEnv = compatHostVersion - ? { ...baseEnv, OPENCLAW_COMPATIBILITY_HOST_VERSION: compatHostVersion } - : { ...baseEnv }; + const env = buildFinalizeEnv(params.env ?? process.env, compatHostVersion); try { const spawnResult = await spawnFinalize({