mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-12 21:53:00 -06:00
fix(gateway): restore restart preflight compatibility (#121757)
* fix(gateway): restore restart preflight compatibility * ci(protocol): preserve restored method vintages * chore: defer compatibility note to release
This commit is contained in:
committed by
GitHub
parent
fd1b965f2b
commit
96b0cf95ef
@@ -405,6 +405,7 @@ enum class GatewayMethod(
|
||||
CronRun("cron.run"),
|
||||
CronRuns("cron.runs"),
|
||||
GatewayIdentityGet("gateway.identity.get"),
|
||||
GatewayRestartPreflight("gateway.restart.preflight"),
|
||||
GatewayRestartRequest("gateway.restart.request"),
|
||||
SystemPresence("system-presence"),
|
||||
SystemEvent("system-event"),
|
||||
|
||||
@@ -519,6 +519,7 @@ methods. Treat this as feature discovery, not a full enumeration of
|
||||
- `system-event` appends a system event and can update/broadcast presence context.
|
||||
- `last-heartbeat` returns the latest persisted heartbeat event.
|
||||
- `set-heartbeats` toggles heartbeat processing on the gateway.
|
||||
- `gateway.restart.preflight` is a deprecated, read-only compatibility preview of restart-specific active work. It does not close admission, create a suspension lease, or provide the atomic full-work fence of `gateway.suspend.prepare`; new restart flows should call `gateway.restart.request`.
|
||||
- `gateway.suspend.prepare` creates a short cooperative-suspension lease only when tracked Gateway work is idle. `gateway.suspend.status` checks that lease, and `gateway.suspend.resume` releases it after thaw or an aborted host operation.
|
||||
|
||||
</Accordion>
|
||||
|
||||
@@ -10,7 +10,12 @@ import { resolveRepoRoot } from "./lib/repo-root.mjs";
|
||||
const repoRoot = resolveRepoRoot(import.meta.url);
|
||||
const descriptorPath = "src/gateway/methods/core-descriptors.ts";
|
||||
|
||||
type MethodSpec = { line: number; name: string; since: string | undefined };
|
||||
type MethodSpec = {
|
||||
line: number;
|
||||
name: string;
|
||||
since: string | undefined;
|
||||
compatibilityRestored: boolean;
|
||||
};
|
||||
|
||||
function runGit(args: string[]): string {
|
||||
const result = spawnSync("git", args, { cwd: repoRoot, encoding: "utf8" });
|
||||
@@ -83,6 +88,20 @@ function stringProperty(object: ts.ObjectLiteralExpression, key: string): string
|
||||
return undefined;
|
||||
}
|
||||
|
||||
function trueProperty(object: ts.ObjectLiteralExpression, key: string): boolean {
|
||||
return object.properties.some((property) => {
|
||||
if (!ts.isPropertyAssignment(property)) {
|
||||
return false;
|
||||
}
|
||||
const propertyName = property.name;
|
||||
const name =
|
||||
ts.isIdentifier(propertyName) || ts.isStringLiteral(propertyName)
|
||||
? propertyName.text
|
||||
: undefined;
|
||||
return name === key && property.initializer.kind === ts.SyntaxKind.TrueKeyword;
|
||||
});
|
||||
}
|
||||
|
||||
function collectMethodSpec(
|
||||
element: ts.Expression,
|
||||
sourceFile: ts.SourceFile,
|
||||
@@ -96,7 +115,12 @@ function collectMethodSpec(
|
||||
`${fileName}:${line} core method spec names must be string literals so additions can be compared with origin/main.`,
|
||||
);
|
||||
}
|
||||
return { name, since: stringProperty(element, "since"), line };
|
||||
return {
|
||||
name,
|
||||
since: stringProperty(element, "since"),
|
||||
compatibilityRestored: trueProperty(element, "compatibilityRestored"),
|
||||
line,
|
||||
};
|
||||
}
|
||||
if (ts.isArrayLiteralExpression(element)) {
|
||||
const name = element.elements[0];
|
||||
@@ -106,7 +130,12 @@ function collectMethodSpec(
|
||||
`${fileName}:${line} core method spec rows must use string literal names and vintage metadata.`,
|
||||
);
|
||||
}
|
||||
return { name: name.text, since: since.text, line };
|
||||
const policy = element.elements[4];
|
||||
const compatibilityRestored =
|
||||
policy !== undefined && ts.isObjectLiteralExpression(policy)
|
||||
? trueProperty(policy, "compatibilityRestored")
|
||||
: false;
|
||||
return { name: name.text, since: since.text, compatibilityRestored, line };
|
||||
}
|
||||
throw new Error(
|
||||
`${fileName}:${line} core method specs must be inline object literals or labeled rows so vintage metadata can be enforced.`,
|
||||
@@ -166,22 +195,32 @@ try {
|
||||
collectMethodSpecs(baseSource, `${descriptorPath}@${mergeBase}`).map((s) => s.name),
|
||||
);
|
||||
const added = currentSpecs.filter((spec) => !baseNames.has(spec.name));
|
||||
const violations = added.filter((spec) => spec.since !== train);
|
||||
const restored = added.filter((spec) => spec.compatibilityRestored);
|
||||
const newMethods = added.filter((spec) => !spec.compatibilityRestored);
|
||||
// Restored shipped methods retain their historical vintage so discovery and
|
||||
// generated clients see the original availability contract, not a new API.
|
||||
const violations = added.filter((spec) =>
|
||||
spec.compatibilityRestored ? !spec.since?.startsWith("<=") : spec.since !== train,
|
||||
);
|
||||
|
||||
if (violations.length > 0) {
|
||||
console.error(`Protocol since guard failed for current train ${train}:`);
|
||||
for (const spec of violations) {
|
||||
const problem = spec.since
|
||||
? `has since ${JSON.stringify(spec.since)}`
|
||||
: "is missing since metadata";
|
||||
const problem = spec.compatibilityRestored
|
||||
? `is marked compatibilityRestored but has non-historical since ${JSON.stringify(spec.since)}`
|
||||
: spec.since
|
||||
? `has since ${JSON.stringify(spec.since)}`
|
||||
: "is missing since metadata";
|
||||
console.error(
|
||||
`- ${descriptorPath}:${spec.line} ${spec.name} ${problem}; add since: ${JSON.stringify(train)}.`,
|
||||
spec.compatibilityRestored
|
||||
? `- ${descriptorPath}:${spec.line} ${spec.name} ${problem}; restored compatibility methods must retain <= vintage metadata.`
|
||||
: `- ${descriptorPath}:${spec.line} ${spec.name} ${problem}; add since: ${JSON.stringify(train)}.`,
|
||||
);
|
||||
}
|
||||
process.exitCode = 1;
|
||||
} else {
|
||||
console.log(
|
||||
`protocol since guard passed: ${added.length} new core method${added.length === 1 ? "" : "s"} use train ${train}`,
|
||||
`protocol since guard passed: ${newMethods.length} new core method${newMethods.length === 1 ? "" : "s"} use train ${train}; ${restored.length} restored compatibility method${restored.length === 1 ? "" : "s"} retain historical vintage`,
|
||||
);
|
||||
}
|
||||
} catch (error) {
|
||||
|
||||
@@ -87,6 +87,7 @@ describe("method scope resolution", () => {
|
||||
["session.discussion.open", ["operator.write"]],
|
||||
["environments.status", ["operator.read"]],
|
||||
["diagnostics.stability", ["operator.read"]],
|
||||
["gateway.restart.preflight", ["operator.read"]],
|
||||
["skills.curator.status", ["operator.read"]],
|
||||
["hooks.status", ["operator.read"]],
|
||||
["skills.curator.pin", ["operator.admin"]],
|
||||
|
||||
@@ -16,12 +16,13 @@ type CoreGatewayMethodSpec = {
|
||||
advertise?: false;
|
||||
startup?: true;
|
||||
controlPlaneWrite?: true;
|
||||
compatibilityRestored?: true;
|
||||
};
|
||||
|
||||
type CoreGatewayMethodMetadata = Pick<CoreGatewayMethodSpec, "name" | "scope" | "since">;
|
||||
type CoreGatewayMethodPolicy = Pick<
|
||||
CoreGatewayMethodSpec,
|
||||
"advertise" | "startup" | "controlPlaneWrite"
|
||||
"advertise" | "startup" | "controlPlaneWrite" | "compatibilityRestored"
|
||||
>;
|
||||
type CoreGatewayMethodSpecRow = readonly [
|
||||
name: string,
|
||||
@@ -299,6 +300,15 @@ const CORE_GATEWAY_METHOD_SPECS = [
|
||||
["cron.run", "cron", "operator.admin", "<=2026.7"],
|
||||
["cron.runs", "cron", "operator.read", "<=2026.7"],
|
||||
["gateway.identity.get", "system", "operator.read", "<=2026.7"],
|
||||
// Deprecated read-only compatibility preview; new restart flows request the
|
||||
// restart directly, while atomic host suspension uses gateway.suspend.prepare.
|
||||
[
|
||||
"gateway.restart.preflight",
|
||||
"restart",
|
||||
"operator.read",
|
||||
"<=2026.7",
|
||||
{ compatibilityRestored: true },
|
||||
],
|
||||
["gateway.restart.request", "restart", "operator.admin", "<=2026.7", { controlPlaneWrite: true }],
|
||||
["system-presence", "system", "operator.read", "<=2026.7"],
|
||||
["system-event", "system", "operator.admin", "<=2026.7"],
|
||||
@@ -503,6 +513,9 @@ const CORE_GATEWAY_METHOD_SPEC_LIST: readonly CoreGatewayMethodSpec[] =
|
||||
if (normalizedPolicy?.controlPlaneWrite === true) {
|
||||
spec.controlPlaneWrite = true;
|
||||
}
|
||||
if (normalizedPolicy?.compatibilityRestored === true) {
|
||||
spec.compatibilityRestored = true;
|
||||
}
|
||||
return spec;
|
||||
});
|
||||
|
||||
|
||||
@@ -148,6 +148,25 @@ describe("listGatewayMethods", () => {
|
||||
expect(coreGatewayHandlers["update.hold"]).toBeTypeOf("function");
|
||||
});
|
||||
|
||||
it("keeps deprecated restart preflight compatibility read-only and advertised", () => {
|
||||
const methods = listGatewayMethods();
|
||||
const descriptor = createCoreGatewayMethodDescriptors(coreGatewayHandlers).find(
|
||||
(candidate) => candidate.name === "gateway.restart.preflight",
|
||||
);
|
||||
|
||||
expect(methods).toContain("gateway.restart.preflight");
|
||||
expect(methods.indexOf("gateway.restart.preflight")).toBe(
|
||||
methods.indexOf("gateway.restart.request") - 1,
|
||||
);
|
||||
expect(coreGatewayHandlers["gateway.restart.preflight"]).toBeTypeOf("function");
|
||||
expect(descriptor).toMatchObject({
|
||||
name: "gateway.restart.preflight",
|
||||
scope: "operator.read",
|
||||
since: "<=2026.7",
|
||||
});
|
||||
expect(descriptor?.controlPlaneWrite).toBeUndefined();
|
||||
});
|
||||
|
||||
it("does not advertise hidden core handlers", () => {
|
||||
const methods = listGatewayMethods();
|
||||
expect(methods).not.toContain("config.openFile");
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
// Restart method tests cover safe restart scheduling, deferral flags, and
|
||||
// response payloads returned by gateway.restart.request.
|
||||
// Restart method tests cover the read-only compatibility preview plus safe
|
||||
// restart scheduling, deferral flags, and request response payloads.
|
||||
|
||||
import { expectDefined } from "@openclaw/normalization-core";
|
||||
import { MAX_TIMER_TIMEOUT_MS } from "@openclaw/normalization-core/number-coercion";
|
||||
@@ -7,10 +7,12 @@ import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import { restartHandlers } from "./restart.js";
|
||||
|
||||
const requestSafeGatewayRestart = vi.hoisted(() => vi.fn());
|
||||
const createSafeGatewayRestartPreflight = vi.hoisted(() => vi.fn());
|
||||
const requestGatewayRestartWithSignalAdmission = vi.hoisted(() => vi.fn());
|
||||
const readActiveGatewayLockIdentity = vi.hoisted(() => vi.fn());
|
||||
|
||||
vi.mock("../../infra/restart-coordinator.js", () => ({
|
||||
createSafeGatewayRestartPreflight: () => createSafeGatewayRestartPreflight(),
|
||||
requestSafeGatewayRestart: (opts: unknown) => requestSafeGatewayRestart(opts),
|
||||
}));
|
||||
|
||||
@@ -38,6 +40,20 @@ function invokeRestartRequest(params: unknown) {
|
||||
).then(() => respond);
|
||||
}
|
||||
|
||||
function invokeRestartPreflight() {
|
||||
const respond = vi.fn();
|
||||
const handler = expectDefined(
|
||||
restartHandlers["gateway.restart.preflight"],
|
||||
'restartHandlers["gateway.restart.preflight"] test invariant',
|
||||
);
|
||||
return Promise.resolve(
|
||||
handler({
|
||||
respond,
|
||||
params: {},
|
||||
} as unknown as Parameters<typeof handler>[0]),
|
||||
).then(() => respond);
|
||||
}
|
||||
|
||||
function mockScheduledRestart(preflight: { safe: boolean; summary: string }) {
|
||||
requestSafeGatewayRestart.mockReturnValueOnce({
|
||||
ok: true,
|
||||
@@ -63,9 +79,10 @@ function expectRestartRequest(skipDeferral: boolean) {
|
||||
});
|
||||
}
|
||||
|
||||
describe("gateway.restart.request handler", () => {
|
||||
describe("gateway restart handlers", () => {
|
||||
beforeEach(() => {
|
||||
requestSafeGatewayRestart.mockClear();
|
||||
createSafeGatewayRestartPreflight.mockReset();
|
||||
requestGatewayRestartWithSignalAdmission.mockReset();
|
||||
requestGatewayRestartWithSignalAdmission.mockReturnValue({ status: "emitted" });
|
||||
readActiveGatewayLockIdentity.mockReset();
|
||||
@@ -77,6 +94,31 @@ describe("gateway.restart.request handler", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("keeps the deprecated read-only preflight response shape", async () => {
|
||||
const preflight = {
|
||||
safe: false,
|
||||
counts: {
|
||||
queueSize: 1,
|
||||
pendingReplies: 2,
|
||||
embeddedRuns: 3,
|
||||
cronRuns: 4,
|
||||
backgroundExecSessions: 5,
|
||||
rootRequests: 6,
|
||||
activeTasks: 7,
|
||||
totalActive: 28,
|
||||
},
|
||||
blockers: [{ kind: "queue", count: 1, message: "1 queued or active operation(s)" }],
|
||||
summary: "restart deferred: 1 queued or active operation(s)",
|
||||
};
|
||||
createSafeGatewayRestartPreflight.mockReturnValueOnce(preflight);
|
||||
|
||||
const respond = await invokeRestartPreflight();
|
||||
|
||||
expect(respond).toHaveBeenCalledWith(true, preflight);
|
||||
expect(requestSafeGatewayRestart).not.toHaveBeenCalled();
|
||||
expect(requestGatewayRestartWithSignalAdmission).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("defaults to skipDeferral: false when the param is absent", async () => {
|
||||
mockScheduledRestart({ safe: true, summary: "safe to restart now" });
|
||||
|
||||
|
||||
@@ -3,7 +3,10 @@ import { MAX_TIMER_TIMEOUT_MS } from "@openclaw/normalization-core/number-coerci
|
||||
import { truncateUtf16Safe } from "@openclaw/normalization-core/utf16-slice";
|
||||
import { ErrorCodes, errorShape } from "../../../packages/gateway-protocol/src/index.js";
|
||||
import { readActiveGatewayLockIdentity } from "../../infra/gateway-lock.js";
|
||||
import { requestSafeGatewayRestart } from "../../infra/restart-coordinator.js";
|
||||
import {
|
||||
createSafeGatewayRestartPreflight,
|
||||
requestSafeGatewayRestart,
|
||||
} from "../../infra/restart-coordinator.js";
|
||||
import type { GatewayRestartIntent } from "../../infra/restart-intent.js";
|
||||
import { requestGatewayRestartWithSignalAdmission } from "../../infra/restart.js";
|
||||
import type { GatewayRequestHandlers } from "./types.js";
|
||||
@@ -159,4 +162,9 @@ export const restartHandlers: GatewayRequestHandlers = {
|
||||
});
|
||||
respond(true, result);
|
||||
},
|
||||
// Deprecated compatibility preview for shipped read-only clients. This is
|
||||
// restart-specific information, not the atomic fence owned by suspend.prepare.
|
||||
"gateway.restart.preflight": async ({ respond }) => {
|
||||
respond(true, createSafeGatewayRestartPreflight());
|
||||
},
|
||||
};
|
||||
|
||||
@@ -4,7 +4,10 @@ import {
|
||||
resetGatewayWorkAdmission,
|
||||
tryBeginGatewayRootWorkAdmission,
|
||||
} from "../process/gateway-work-admission.js";
|
||||
import { requestSafeGatewayRestart } from "./restart-coordinator.js";
|
||||
import {
|
||||
createSafeGatewayRestartPreflight,
|
||||
requestSafeGatewayRestart,
|
||||
} from "./restart-coordinator.js";
|
||||
|
||||
const scheduleGatewaySigusr1Restart = vi.hoisted(() => vi.fn());
|
||||
|
||||
@@ -32,7 +35,7 @@ afterEach(() => {
|
||||
describe("safe gateway restart coordinator", () => {
|
||||
const requestPreflight = (
|
||||
inspect: NonNullable<Parameters<typeof requestSafeGatewayRestart>[0]>["inspect"],
|
||||
) => requestSafeGatewayRestart({ inspect }).preflight;
|
||||
) => createSafeGatewayRestartPreflight(inspect);
|
||||
|
||||
it("reports safe when no restart blockers are active", () => {
|
||||
const preflight = requestPreflight({
|
||||
|
||||
@@ -55,7 +55,7 @@ export type SafeGatewayRestartRequestResult = {
|
||||
restart: ScheduledRestart;
|
||||
};
|
||||
|
||||
function createSafeGatewayRestartPreflight(
|
||||
export function createSafeGatewayRestartPreflight(
|
||||
inspectors: Partial<SafeRestartInspectors> = {},
|
||||
): SafeGatewayRestartPreflight {
|
||||
const snapshot = createGatewayActiveWorkSnapshot({
|
||||
|
||||
@@ -782,12 +782,19 @@ function writeExecutable(filePath: string, lines: string[]): void {
|
||||
|
||||
function writeProtocolDescriptor(
|
||||
repo: string,
|
||||
additions: Array<{ name: string; since?: string }> = [],
|
||||
additions: Array<{
|
||||
name: string;
|
||||
since?: string;
|
||||
compatibilityRestored?: boolean;
|
||||
}> = [],
|
||||
): void {
|
||||
const rows = [{ name: "health", since: "2026.7" }, ...additions].map(({ name, since }) => {
|
||||
const sinceProperty = since === undefined ? "" : `, since: ${JSON.stringify(since)}`;
|
||||
return ` { name: ${JSON.stringify(name)}${sinceProperty} },`;
|
||||
});
|
||||
const rows = [{ name: "health", since: "2026.7" }, ...additions].map(
|
||||
({ name, since, compatibilityRestored }) => {
|
||||
const sinceProperty = since === undefined ? "" : `, since: ${JSON.stringify(since)}`;
|
||||
const compatibilityProperty = compatibilityRestored ? ", compatibilityRestored: true" : "";
|
||||
return ` { name: ${JSON.stringify(name)}${sinceProperty}${compatibilityProperty} },`;
|
||||
},
|
||||
);
|
||||
const descriptor = path.join(repo, "src/gateway/methods/core-descriptors.ts");
|
||||
mkdirSync(path.dirname(descriptor), { recursive: true });
|
||||
writeFileSync(
|
||||
@@ -827,6 +834,29 @@ function createQaProtocolTopology() {
|
||||
writeFileSync(path.join(origin, "main-tip.txt"), "later main tip\n");
|
||||
commitProtocolFixture(origin, "advance main");
|
||||
|
||||
runGit(origin, ["checkout", "-q", "-b", "compatibility/restore", mainBase]);
|
||||
writeProtocolDescriptor(origin, [
|
||||
{
|
||||
name: "gateway.restart.preflight",
|
||||
since: "<=2026.7",
|
||||
compatibilityRestored: true,
|
||||
},
|
||||
]);
|
||||
const compatibilityHead = commitProtocolFixture(origin, "restore compatibility method");
|
||||
|
||||
runGit(origin, ["checkout", "-q", "-b", "compatibility/invalid", mainBase]);
|
||||
writeProtocolDescriptor(origin, [
|
||||
{
|
||||
name: "gateway.restart.invalid",
|
||||
since: "2026.8",
|
||||
compatibilityRestored: true,
|
||||
},
|
||||
]);
|
||||
const invalidCompatibilityHead = commitProtocolFixture(
|
||||
origin,
|
||||
"mislabel new method as compatibility",
|
||||
);
|
||||
|
||||
runGit(origin, ["checkout", "-q", "-b", releaseBranch, mainBase]);
|
||||
writeProtocolDescriptor(origin, [{ name: "sessions.releaseOnly" }]);
|
||||
const releaseHead = commitProtocolFixture(origin, "add release protocol method");
|
||||
@@ -848,8 +878,10 @@ function createQaProtocolTopology() {
|
||||
|
||||
return {
|
||||
checkout,
|
||||
compatibilityHead,
|
||||
fakeBin,
|
||||
featureHead,
|
||||
invalidCompatibilityHead,
|
||||
mainBase,
|
||||
mainHead,
|
||||
mainReleaseTag,
|
||||
@@ -6068,6 +6100,24 @@ printf '%s\n' "\${CURL_SUCCESS_IP:-203.0.113.7}"
|
||||
expect(mainCheck.status, `${mainCheck.stdout}${mainCheck.stderr}`).toBe(0);
|
||||
expect(mainCheck.stdout).toContain("1 new core method");
|
||||
|
||||
runGit(topology.checkout, ["checkout", "-q", "--detach", topology.compatibilityHead]);
|
||||
const compatibilityCheck = runProtocolSinceFixture(topology.checkout, topology.mainBase);
|
||||
expect(
|
||||
compatibilityCheck.status,
|
||||
`${compatibilityCheck.stdout}${compatibilityCheck.stderr}`,
|
||||
).toBe(0);
|
||||
expect(compatibilityCheck.stdout).toContain("1 restored compatibility method");
|
||||
|
||||
runGit(topology.checkout, ["checkout", "-q", "--detach", topology.invalidCompatibilityHead]);
|
||||
const invalidCompatibilityCheck = runProtocolSinceFixture(
|
||||
topology.checkout,
|
||||
topology.mainBase,
|
||||
);
|
||||
expect(invalidCompatibilityCheck.status).not.toBe(0);
|
||||
expect(invalidCompatibilityCheck.stderr).toContain(
|
||||
"restored compatibility methods must retain <= vintage metadata",
|
||||
);
|
||||
|
||||
runGit(topology.checkout, ["checkout", "-q", "--detach", topology.releaseHead]);
|
||||
const releaseCheck = runProtocolSinceFixture(topology.checkout, topology.mainBase);
|
||||
expect(releaseCheck.status).not.toBe(0);
|
||||
|
||||
Reference in New Issue
Block a user