mirror of
https://github.com/openclaw/openclaw.git
synced 2026-04-12 09:41:11 +00:00
fix(exec): harden stale/replay/live requests
This commit is contained in:
@@ -381,16 +381,18 @@ implemented in `src/gateway/server-methods/*.ts`.
|
||||
|
||||
#### Approval families
|
||||
|
||||
- `exec.approval.request` and `exec.approval.resolve` cover one-shot exec
|
||||
approval requests.
|
||||
- `exec.approval.request`, `exec.approval.get`, `exec.approval.list`, and
|
||||
`exec.approval.resolve` cover one-shot exec approval requests plus pending
|
||||
approval lookup/replay.
|
||||
- `exec.approval.waitDecision` waits on one pending exec approval and returns
|
||||
the final decision (or `null` on timeout).
|
||||
- `exec.approvals.get` and `exec.approvals.set` manage gateway exec approval
|
||||
policy snapshots.
|
||||
- `exec.approvals.node.get` and `exec.approvals.node.set` manage node-local exec
|
||||
approval policy via node relay commands.
|
||||
- `plugin.approval.request`, `plugin.approval.waitDecision`, and
|
||||
`plugin.approval.resolve` cover plugin-defined approval flows.
|
||||
- `plugin.approval.request`, `plugin.approval.list`,
|
||||
`plugin.approval.waitDecision`, and `plugin.approval.resolve` cover
|
||||
plugin-defined approval flows.
|
||||
|
||||
#### Other major families
|
||||
|
||||
|
||||
@@ -491,6 +491,56 @@ describe("createExecApprovalChannelRuntime", () => {
|
||||
expect(deliverRequested).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("does not replay approvals after stop wins once hello is already complete", async () => {
|
||||
const replayDeferred = createDeferred<
|
||||
Array<{
|
||||
id: string;
|
||||
request: { command: string };
|
||||
createdAtMs: number;
|
||||
expiresAtMs: number;
|
||||
}>
|
||||
>();
|
||||
mockGatewayClientRequests.mockImplementation(async (method) => {
|
||||
if (method === "exec.approval.list") {
|
||||
return replayDeferred.promise;
|
||||
}
|
||||
return { ok: true };
|
||||
});
|
||||
const deliverRequested = vi.fn(async (request) => [{ id: request.id }]);
|
||||
const runtime = createExecApprovalChannelRuntime({
|
||||
label: "test/replay-stop-after-ready",
|
||||
clientDisplayName: "Test Replay Stop",
|
||||
cfg: {} as never,
|
||||
isConfigured: () => true,
|
||||
shouldHandle: () => true,
|
||||
deliverRequested,
|
||||
finalizeResolved: async () => undefined,
|
||||
});
|
||||
|
||||
const startPromise = runtime.start();
|
||||
await vi.waitFor(() => {
|
||||
expect(mockGatewayClientRequests).toHaveBeenCalledWith("exec.approval.list", {});
|
||||
});
|
||||
|
||||
const stopPromise = runtime.stop();
|
||||
replayDeferred.resolve([
|
||||
{
|
||||
id: "abc",
|
||||
request: {
|
||||
command: "echo abc",
|
||||
},
|
||||
createdAtMs: 1000,
|
||||
expiresAtMs: 2000,
|
||||
},
|
||||
]);
|
||||
|
||||
await startPromise;
|
||||
await stopPromise;
|
||||
|
||||
expect(deliverRequested).not.toHaveBeenCalled();
|
||||
expect(mockGatewayClientStops).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("clears pending state when delivery throws", async () => {
|
||||
const deliverRequested = vi
|
||||
.fn<() => Promise<Array<{ id: string }>>>()
|
||||
|
||||
@@ -89,6 +89,8 @@ export function createExecApprovalChannelRuntime<
|
||||
let shouldRun = false;
|
||||
let startPromise: Promise<void> | null = null;
|
||||
|
||||
const shouldKeepRunning = (): boolean => shouldRun;
|
||||
|
||||
const spawn = (label: string, promise: Promise<void>): void => {
|
||||
void promise.catch((err: unknown) => {
|
||||
const message = formatErrorMessage(err);
|
||||
@@ -96,6 +98,15 @@ export function createExecApprovalChannelRuntime<
|
||||
});
|
||||
};
|
||||
|
||||
const stopClientIfInactive = (client: GatewayClient): boolean => {
|
||||
if (shouldKeepRunning()) {
|
||||
return false;
|
||||
}
|
||||
gatewayClient = null;
|
||||
client.stop();
|
||||
return true;
|
||||
};
|
||||
|
||||
const clearPendingEntry = (
|
||||
approvalId: string,
|
||||
): PendingApprovalEntry<TPending, TRequest, TResolved> | null => {
|
||||
@@ -122,7 +133,13 @@ export function createExecApprovalChannelRuntime<
|
||||
});
|
||||
};
|
||||
|
||||
const handleRequested = async (request: TRequest): Promise<void> => {
|
||||
const handleRequested = async (
|
||||
request: TRequest,
|
||||
opts?: { ignoreIfInactive?: boolean },
|
||||
): Promise<void> => {
|
||||
if (opts?.ignoreIfInactive && !shouldKeepRunning()) {
|
||||
return;
|
||||
}
|
||||
if (!adapter.shouldHandle(request)) {
|
||||
return;
|
||||
}
|
||||
@@ -202,11 +219,17 @@ export function createExecApprovalChannelRuntime<
|
||||
|
||||
const handleGatewayEvent = (evt: EventFrame): void => {
|
||||
if (evt.event === "exec.approval.requested" && eventKinds.has("exec")) {
|
||||
spawn("error handling approval request", handleRequested(evt.payload as TRequest));
|
||||
spawn(
|
||||
"error handling approval request",
|
||||
handleRequested(evt.payload as TRequest, { ignoreIfInactive: true }),
|
||||
);
|
||||
return;
|
||||
}
|
||||
if (evt.event === "plugin.approval.requested" && eventKinds.has("plugin")) {
|
||||
spawn("error handling approval request", handleRequested(evt.payload as TRequest));
|
||||
spawn(
|
||||
"error handling approval request",
|
||||
handleRequested(evt.payload as TRequest, { ignoreIfInactive: true }),
|
||||
);
|
||||
return;
|
||||
}
|
||||
if (evt.event === "exec.approval.resolved" && eventKinds.has("exec")) {
|
||||
@@ -278,10 +301,22 @@ export function createExecApprovalChannelRuntime<
|
||||
try {
|
||||
client.start();
|
||||
await ready;
|
||||
if (stopClientIfInactive(client)) {
|
||||
return;
|
||||
}
|
||||
for (const method of resolveApprovalReplayMethods(eventKinds)) {
|
||||
if (stopClientIfInactive(client)) {
|
||||
return;
|
||||
}
|
||||
const pendingRequests = await client.request<Array<TRequest>>(method, {});
|
||||
if (stopClientIfInactive(client)) {
|
||||
return;
|
||||
}
|
||||
for (const request of pendingRequests) {
|
||||
await handleRequested(request);
|
||||
if (stopClientIfInactive(client)) {
|
||||
return;
|
||||
}
|
||||
await handleRequested(request, { ignoreIfInactive: true });
|
||||
}
|
||||
}
|
||||
started = true;
|
||||
|
||||
Reference in New Issue
Block a user