mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-25 20:05:46 -06:00
fix(nodes): retry automatic placement across eligible devices (#128566)
* fix(nodes): harden automatic placement selection * test(ui): stabilize lazy module and geometry checks
This commit is contained in:
committed by
GitHub
parent
7916caab26
commit
082cdc8866
@@ -334,6 +334,7 @@ export const sessionDispatchHandlers: GatewayRequestHandlers = {
|
||||
dispatchTarget = destination.value;
|
||||
}
|
||||
if (
|
||||
!autoDevice &&
|
||||
dispatchTarget &&
|
||||
!(await validateDispatchExecutionMode({
|
||||
context,
|
||||
@@ -422,13 +423,15 @@ export const sessionDispatchHandlers: GatewayRequestHandlers = {
|
||||
return;
|
||||
}
|
||||
dispatchTarget = destination.value;
|
||||
}
|
||||
if (autoDevice) {
|
||||
const eligibility = await resolveDevicePlacementEligibility({
|
||||
environmentService: context.workerEnvironmentService,
|
||||
deviceId: dispatchTarget.deviceId!,
|
||||
deviceId: candidates[attempt]!,
|
||||
runtimeId: sessionRuntime,
|
||||
requirement: devicePlacement,
|
||||
config: cfg,
|
||||
currentNode: context.nodeRegistry.get(dispatchTarget.deviceId!),
|
||||
currentNode: context.nodeRegistry.get(candidates[attempt]!),
|
||||
});
|
||||
if (!eligibility.ok) {
|
||||
lastEligibilityError = eligibility.error;
|
||||
|
||||
@@ -39,6 +39,7 @@ import {
|
||||
getDispatchTestMocks,
|
||||
invokeSessionDispatch,
|
||||
makeDispatchTestContext,
|
||||
makeFailedPlacement,
|
||||
makeSessionTarget,
|
||||
} from "./sessions-dispatch.test-support.js";
|
||||
|
||||
@@ -281,6 +282,59 @@ describe("sessions.dispatch device targets", () => {
|
||||
);
|
||||
});
|
||||
|
||||
it.each([
|
||||
{ name: "disconnects", unavailableReason: "disconnected" as const },
|
||||
{ name: "fills its worker slots", unavailableReason: "at-capacity" as const },
|
||||
])(
|
||||
"tries the next host when the first $name before dispatch",
|
||||
async ({ unavailableReason }) => {
|
||||
useDeviceSession();
|
||||
const nodes = [connectedNode("first", 3), connectedNode("second", 2)];
|
||||
vi.spyOn(environmentMethods, "listGatewayEnvironments").mockResolvedValue(
|
||||
deviceEnvironments(nodes),
|
||||
);
|
||||
let firstChecks = 0;
|
||||
const workerEnvironmentService = {};
|
||||
bindDeviceWorkerAvailability(workerEnvironmentService, async (deviceId) => {
|
||||
if (deviceId === "first" && ++firstChecks >= 2) {
|
||||
return unavailableReason === "disconnected"
|
||||
? { available: false, unavailableReason }
|
||||
: { available: true, node: connectedNode(deviceId, 0) };
|
||||
}
|
||||
return { available: true, node: nodes.find((node) => node.nodeId === deviceId) };
|
||||
});
|
||||
const dispatch = vi.fn().mockResolvedValue(activeDevicePlacement("second"));
|
||||
|
||||
const respond = await invokeSessionDispatch(
|
||||
makeDispatchTestContext({
|
||||
nodeRegistry: {
|
||||
get: (deviceId: string) => nodes.find((node) => node.nodeId === deviceId),
|
||||
} as never,
|
||||
workerEnvironmentService: workerEnvironmentService as never,
|
||||
workerPlacementDispatchService: { dispatch },
|
||||
workerSessionPlacementService: { getMany: () => new Map() },
|
||||
}),
|
||||
{ autoDevice: true },
|
||||
);
|
||||
|
||||
expect(dispatch).toHaveBeenCalledOnce();
|
||||
expect(dispatch).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ profileId: "device:second", deviceId: "second" }),
|
||||
expect.any(Function),
|
||||
undefined,
|
||||
);
|
||||
expect(respond).toHaveBeenCalledWith(
|
||||
true,
|
||||
expect.objectContaining({
|
||||
placement: expect.objectContaining({
|
||||
runner: { kind: "device", status: "available", deviceId: "second" },
|
||||
}),
|
||||
}),
|
||||
undefined,
|
||||
);
|
||||
},
|
||||
);
|
||||
|
||||
it("redispatches to the next host when the first disappears at the inner eligibility fence", async () => {
|
||||
const root = await fs.mkdtemp(
|
||||
path.join(await fs.realpath(os.tmpdir()), "openclaw-session-auto-device-"),
|
||||
@@ -448,6 +502,50 @@ describe("sessions.dispatch device targets", () => {
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("never rotates to another host after an environment has been allocated", async () => {
|
||||
useDeviceSession();
|
||||
const nodes = [connectedNode("first", 3), connectedNode("second", 2)];
|
||||
vi.spyOn(environmentMethods, "listGatewayEnvironments").mockResolvedValue(
|
||||
deviceEnvironments(nodes),
|
||||
);
|
||||
let allocated = false;
|
||||
const workerEnvironmentService = {};
|
||||
bindDeviceWorkerAvailability(workerEnvironmentService, async (deviceId) =>
|
||||
allocated && deviceId === "first"
|
||||
? { available: false, unavailableReason: "disconnected" }
|
||||
: { available: true, node: nodes.find((node) => node.nodeId === deviceId) },
|
||||
);
|
||||
const dispatch = vi.fn(async () => {
|
||||
allocated = true;
|
||||
throw new Error("device worker node is not connected: first; reconnect it before retrying");
|
||||
});
|
||||
|
||||
const respond = await invokeSessionDispatch(
|
||||
makeDispatchTestContext({
|
||||
nodeRegistry: {
|
||||
get: (deviceId: string) => nodes.find((node) => node.nodeId === deviceId),
|
||||
} as never,
|
||||
workerEnvironmentService: workerEnvironmentService as never,
|
||||
workerPlacementDispatchService: { dispatch },
|
||||
workerSessionPlacementService: {
|
||||
getMany: () =>
|
||||
new Map(allocated ? [[dispatchTestSessionId, makeFailedPlacement()]] : []),
|
||||
} as never,
|
||||
}),
|
||||
{ autoDevice: true },
|
||||
);
|
||||
|
||||
expect(dispatch).toHaveBeenCalledOnce();
|
||||
expect(respond).toHaveBeenCalledWith(
|
||||
false,
|
||||
undefined,
|
||||
expect.objectContaining({
|
||||
code: ErrorCodes.UNAVAILABLE,
|
||||
message: expect.stringContaining("device worker node is not connected: first"),
|
||||
}),
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe("runtime-owned paired-node command authority", () => {
|
||||
|
||||
@@ -3,7 +3,9 @@ import type { OpenClawConfig } from "../../config/types.openclaw.js";
|
||||
import { isNodeCommandAllowed, resolveNodeCommandAllowlist } from "../node-command-policy.js";
|
||||
import { deviceUnavailableText, resolveDeviceWorkerAvailability } from "./device-provider.js";
|
||||
|
||||
type DevicePlacementEligibility = { ok: true } | { ok: false; error: string };
|
||||
type DevicePlacementEligibility =
|
||||
| { ok: true; availableSlots: number }
|
||||
| { ok: false; error: string };
|
||||
|
||||
export async function resolveDevicePlacementEligibility(params: {
|
||||
environmentService: object | undefined;
|
||||
@@ -70,5 +72,5 @@ export async function resolveDevicePlacementEligibility(params: {
|
||||
}),
|
||||
};
|
||||
}
|
||||
return { ok: true };
|
||||
return { ok: true, availableSlots: node.workerHost.capacity.available };
|
||||
}
|
||||
|
||||
@@ -111,6 +111,47 @@ describe("paired-device automatic placement selection", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("ranks current worker capacity instead of stale environment snapshots", async () => {
|
||||
const environments = [
|
||||
nodeEnvironment("alpha", 9),
|
||||
nodeEnvironment("bravo", 1),
|
||||
nodeEnvironment("charlie", 5),
|
||||
];
|
||||
const liveCapacity = new Map([
|
||||
["alpha", 2],
|
||||
["bravo", 4],
|
||||
["charlie", 2],
|
||||
]);
|
||||
const currentNodes = new Map(
|
||||
environments.map((environment) => {
|
||||
const node = nodeProof(environment);
|
||||
return [
|
||||
node.nodeId,
|
||||
{
|
||||
...node,
|
||||
workerHost: {
|
||||
...node.workerHost,
|
||||
capacity: { total: 9, available: liveCapacity.get(node.nodeId) ?? 0 },
|
||||
},
|
||||
},
|
||||
] as const;
|
||||
}),
|
||||
);
|
||||
|
||||
const result = await selectNodes(environments, {
|
||||
availability: async (deviceId) => ({ available: true, node: currentNodes.get(deviceId) }),
|
||||
});
|
||||
|
||||
expect(result).toEqual({
|
||||
ok: true,
|
||||
candidates: [
|
||||
{ deviceId: "bravo", availableSlots: 4 },
|
||||
{ deviceId: "alpha", availableSlots: 2 },
|
||||
{ deviceId: "charlie", availableSlots: 2 },
|
||||
],
|
||||
});
|
||||
});
|
||||
|
||||
it("breaks equal-capacity ties by device identity, independently of catalog order", async () => {
|
||||
const result = await selectNodes([
|
||||
nodeEnvironment("charlie", 2),
|
||||
|
||||
@@ -5,13 +5,8 @@ import type { NodeRegistry } from "../node-registry.js";
|
||||
import { resolveDevicePlacementEligibility } from "./device-placement-eligibility.js";
|
||||
import { deviceUnavailableText } from "./device-provider.js";
|
||||
|
||||
type DevicePlacementCandidate = {
|
||||
deviceId: string;
|
||||
availableSlots: number;
|
||||
};
|
||||
|
||||
type DevicePlacementSelection =
|
||||
| { ok: true; candidates: DevicePlacementCandidate[] }
|
||||
| { ok: true; candidates: { deviceId: string; availableSlots: number }[] }
|
||||
| { ok: false; error: string };
|
||||
|
||||
export async function selectDevicePlacementCandidates(params: {
|
||||
@@ -36,20 +31,18 @@ export async function selectDevicePlacementCandidates(params: {
|
||||
const outdated = nodes.find((node) =>
|
||||
node.issues?.some((issue) => issue.code === "update-required"),
|
||||
);
|
||||
const outdatedError =
|
||||
outdated &&
|
||||
deviceUnavailableText(outdated.id.slice("node:".length), {
|
||||
available: false,
|
||||
issue: outdated.issues?.[0],
|
||||
});
|
||||
const hosts = nodes.filter((node) => node.sessionHost === true);
|
||||
if (hosts.length === 0) {
|
||||
if (outdated) {
|
||||
return {
|
||||
ok: false,
|
||||
error: deviceUnavailableText(outdated.id.slice("node:".length), {
|
||||
available: false,
|
||||
issue: outdated.issues?.[0],
|
||||
}),
|
||||
};
|
||||
}
|
||||
return {
|
||||
ok: false,
|
||||
error:
|
||||
outdatedError ??
|
||||
"no paired session-host nodes are available; pair a node, enable session hosting, then retry",
|
||||
};
|
||||
}
|
||||
@@ -81,7 +74,9 @@ export async function selectDevicePlacementCandidates(params: {
|
||||
});
|
||||
return {
|
||||
deviceId,
|
||||
availableSlots: node.workerSlots?.available ?? 0,
|
||||
availableSlots: eligibility.ok
|
||||
? eligibility.availableSlots
|
||||
: (node.workerSlots?.available ?? 0),
|
||||
eligibility,
|
||||
};
|
||||
}),
|
||||
@@ -98,14 +93,8 @@ export async function selectDevicePlacementCandidates(params: {
|
||||
if (candidates.length > 0) {
|
||||
return { ok: true, candidates };
|
||||
}
|
||||
if (attempts.length === 0 && outdated) {
|
||||
return {
|
||||
ok: false,
|
||||
error: deviceUnavailableText(outdated.id.slice("node:".length), {
|
||||
available: false,
|
||||
issue: outdated.issues?.[0],
|
||||
}),
|
||||
};
|
||||
if (attempts.length === 0 && outdatedError) {
|
||||
return { ok: false, error: outdatedError };
|
||||
}
|
||||
const atCapacity =
|
||||
requirement.consumesWorkerSlot && attempts.every(({ availableSlots }) => availableSlots === 0);
|
||||
|
||||
@@ -7,6 +7,8 @@ import type { GatewaySessionRow } from "../api/types.ts";
|
||||
import type { RouteId } from "../app-routes.ts";
|
||||
import { createStorageMock } from "../test-helpers/storage.ts";
|
||||
import { resetAppHostTestGlobals } from "./app-host.test-support.ts";
|
||||
// This test owns shell panel routing, not lazy sidebar loading; settle that module at setup.
|
||||
import "../components/app-sidebar.ts";
|
||||
import "./app-host.ts";
|
||||
import type { ApplicationRuntime } from "./bootstrap.ts";
|
||||
import type { ApplicationContext } from "./context.ts";
|
||||
|
||||
@@ -8,13 +8,18 @@ import {
|
||||
} from "./new-session-page.test-support.ts";
|
||||
|
||||
const suite = createNewSessionPageE2eSuite();
|
||||
const deviceTargets = [
|
||||
{ name: "selected", value: "device:paired-runner", target: { deviceId: "paired-runner" } },
|
||||
{ name: "automatic", value: "auto-device", target: { autoDevice: true } },
|
||||
];
|
||||
|
||||
suite.define(() => {
|
||||
it("creates a managed session, dispatches the selected device, then sends the first turn", async () => {
|
||||
it.each(deviceTargets)("dispatches the $name device", async ({ value, target }) => {
|
||||
const context = await suite.browser.newContext({ locale: "en-US", serviceWorkers: "block" });
|
||||
const page = await context.newPage();
|
||||
const sessionKey = "agent:main:device-dispatch";
|
||||
const gateway = await installMockGateway(page, {
|
||||
operatorScopes: ["operator.read", "operator.write"],
|
||||
workspace: WORKSPACE,
|
||||
workspaceGit: true,
|
||||
methodResponses: {
|
||||
@@ -57,10 +62,7 @@ suite.define(() => {
|
||||
await page.goto(`${suite.server.baseUrl}new`);
|
||||
await gateway.waitForRequest("environments.list");
|
||||
await page.locator("#new-session-where-trigger").click();
|
||||
await page
|
||||
.locator("wa-popover.new-session-page__where-popover")
|
||||
.getByRole("button", { name: "Paired runner" })
|
||||
.click();
|
||||
await page.locator(`[data-value="${value}"]`).click();
|
||||
await page.locator(".new-session-page__message").fill("run on the paired device");
|
||||
expect(await page.locator('wa-dropdown-item[value="start-terminal"]').count()).toBe(0);
|
||||
await page.getByRole("button", { name: "Start session" }).click();
|
||||
@@ -78,7 +80,7 @@ suite.define(() => {
|
||||
expect(dispatch.params).toEqual({
|
||||
key: sessionKey,
|
||||
agentId: "main",
|
||||
deviceId: "paired-runner",
|
||||
...target,
|
||||
});
|
||||
const send = await gateway.waitForRequest("sessions.send");
|
||||
expect(send.params).toMatchObject({
|
||||
|
||||
@@ -5,7 +5,7 @@ import "../../styles/settings.css";
|
||||
import "../../styles/cron.css";
|
||||
|
||||
const hasBrowserLayout = !navigator.userAgent.toLowerCase().includes("jsdom");
|
||||
const alignmentTolerancePx = 1.25;
|
||||
const alignmentTolerancePx = 2;
|
||||
|
||||
afterEach(() => {
|
||||
document.body.replaceChildren();
|
||||
|
||||
@@ -119,7 +119,12 @@ export function resolveAutomaticDevicePlacementDisabledReason(
|
||||
.map((environment) => environment.id),
|
||||
);
|
||||
if (sessionHostIds.size === 0) {
|
||||
return t("newSession.noSessionHosts");
|
||||
const outdated = (environments ?? []).find((environment) =>
|
||||
environment.issues?.some((issue) => issue.code === "update-required"),
|
||||
);
|
||||
return outdated
|
||||
? unavailableReason(outdated, DEFAULT_DEVICE_PLACEMENT)
|
||||
: t("newSession.noSessionHosts");
|
||||
}
|
||||
return devices.some((device) => device.selectable)
|
||||
? undefined
|
||||
|
||||
@@ -189,7 +189,25 @@ describe("Where chip", () => {
|
||||
expect(emptyContainer.querySelector('[data-value="auto-device"]')).toBeNull();
|
||||
});
|
||||
|
||||
it("disables automatic selection with an actionable reason when no paired device hosts sessions", () => {
|
||||
it.each([
|
||||
{
|
||||
name: "no paired device hosts sessions",
|
||||
issues: undefined,
|
||||
reason: /no session hosts are paired/i,
|
||||
},
|
||||
{
|
||||
name: "a paired node must be updated before it can advertise session hosting",
|
||||
issues: [
|
||||
{
|
||||
code: "update-required",
|
||||
action: "update-and-reconnect",
|
||||
updateCommand: "openclaw update",
|
||||
headlessReconnectCommand: "openclaw node restart",
|
||||
} as const,
|
||||
],
|
||||
reason: /openclaw update.*openclaw node restart/i,
|
||||
},
|
||||
])("disables automatic selection with an actionable reason when $name", ({ issues, reason }) => {
|
||||
const state = resolveWhereChip({
|
||||
environments: [
|
||||
{
|
||||
@@ -198,6 +216,7 @@ describe("Where chip", () => {
|
||||
label: "MacBook",
|
||||
status: "available",
|
||||
sessionHost: false,
|
||||
...(issues ? { issues } : {}),
|
||||
},
|
||||
],
|
||||
cloudProfiles: [],
|
||||
@@ -231,7 +250,8 @@ describe("Where chip", () => {
|
||||
|
||||
const automatic = container.querySelector<HTMLButtonElement>('[data-value="auto-device"]');
|
||||
expect(automatic?.disabled).toBe(true);
|
||||
expect(automatic?.title).toMatch(/no session hosts are paired/i);
|
||||
expect(automatic?.title).toMatch(reason);
|
||||
expect(automatic?.textContent).toMatch(reason);
|
||||
});
|
||||
|
||||
it.each([
|
||||
|
||||
Reference in New Issue
Block a user