mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-28 05:16:23 -06:00
fix(infra): drop in-flight approval delivery after onStopped
createChannelApprovalHandlerFromCapability shares a closure-scoped activeEntries Map across deliverTarget / finalizeResolved / finalizeExpired / onStopped, with no synchronization primitives in the file. deliverTarget's two awaits (transport.deliverPending then interactions.bindPending) bracket a read-modify-write on activeEntries; if onStopped clears the map between those awaits, the wrapped entry is inserted into an already-cleared map and never reaches unbindPending — the native side keeps its listener / channel binding open forever. Production-faithful e2e measured this 3/3 trials: bindPending=1, unbindPending=0 per request. Track a closure-scoped `stopped` flag set by onStopped, and have deliverTarget call unbindPending and bail to null on each await when stopped becomes true. nativeRuntime contracts (transport / interactions signatures) are untouched. AI-assisted: drafted with claude code (claude-opus-4-7).
This commit is contained in:
committed by
Peter Steinberger
parent
2c59ea8a2e
commit
06dfa6f160
@@ -364,4 +364,44 @@ describe("createLazyChannelApprovalNativeRuntimeAdapter", () => {
|
||||
expect(onDelivered).toHaveBeenCalledWith({ request: { id: "exec:1" } });
|
||||
expect(load).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("unbinds in-flight wrapped entry when stop() fires between deliverPending and bindPending", async () => {
|
||||
const deliverGate = { resolve: () => {}, promise: Promise.resolve() };
|
||||
const deliverPromise = new Promise<void>((resolve) => {
|
||||
deliverGate.resolve = resolve;
|
||||
});
|
||||
deliverGate.promise = deliverPromise;
|
||||
const deliverPending = vi.fn(async () => {
|
||||
await deliverPromise;
|
||||
return { messageId: "in-flight" };
|
||||
});
|
||||
const bindPending = vi.fn().mockResolvedValue({ bindingId: "bound-in-flight" });
|
||||
const unbindPending = vi.fn();
|
||||
|
||||
const runtime = await createTestApprovalHandler(
|
||||
makeNativeApprovalCapability({
|
||||
deliverPending,
|
||||
bindPending,
|
||||
unbindPending,
|
||||
}),
|
||||
);
|
||||
const approvalRuntime = expectApprovalRuntime(runtime);
|
||||
const request = makeExecApprovalRequest("exec:in-flight");
|
||||
|
||||
const inflight = approvalRuntime.handleRequested(request);
|
||||
await new Promise((r) => setTimeout(r, 0));
|
||||
|
||||
// stop() while deliverPending is parked — onStopped flips the closure flag.
|
||||
await approvalRuntime.stop();
|
||||
deliverGate.resolve();
|
||||
await inflight;
|
||||
|
||||
expect(unbindPending).toHaveBeenCalledTimes(1);
|
||||
const unbind = firstCallArg(unbindPending) as
|
||||
| { entry?: unknown; binding?: unknown; request?: unknown }
|
||||
| undefined;
|
||||
expect(unbind?.entry).toEqual({ messageId: "in-flight" });
|
||||
expect(unbind?.request).toBe(request);
|
||||
expect(bindPending).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -442,6 +442,7 @@ export async function createChannelApprovalHandlerFromCapability(params: {
|
||||
}
|
||||
const log = createSubsystemLogger(params.label);
|
||||
const activeEntries = new Map<string, ActiveApprovalEntries>();
|
||||
let stopped = false;
|
||||
const resolveApprovalKind =
|
||||
nativeRuntime.resolveApprovalKind ??
|
||||
((request: ApprovalRequest) =>
|
||||
@@ -514,6 +515,15 @@ export async function createChannelApprovalHandlerFromCapability(params: {
|
||||
if (!entry) {
|
||||
return null;
|
||||
}
|
||||
if (stopped) {
|
||||
await nativeRuntime.interactions?.unbindPending?.({
|
||||
...baseContext,
|
||||
entry,
|
||||
request,
|
||||
approvalKind,
|
||||
});
|
||||
return null;
|
||||
}
|
||||
const binding = await nativeRuntime.interactions?.bindPending?.({
|
||||
...baseContext,
|
||||
entry,
|
||||
@@ -522,6 +532,16 @@ export async function createChannelApprovalHandlerFromCapability(params: {
|
||||
view: pendingContent.view,
|
||||
pendingPayload: pendingContent.payload,
|
||||
});
|
||||
if (stopped) {
|
||||
await nativeRuntime.interactions?.unbindPending?.({
|
||||
...baseContext,
|
||||
entry,
|
||||
...(binding === undefined || binding === null ? {} : { binding }),
|
||||
request,
|
||||
approvalKind,
|
||||
});
|
||||
return null;
|
||||
}
|
||||
const wrapped: WrappedPendingEntry = {
|
||||
entry,
|
||||
...(binding === undefined || binding === null ? {} : { binding }),
|
||||
@@ -654,6 +674,7 @@ export async function createChannelApprovalHandlerFromCapability(params: {
|
||||
});
|
||||
},
|
||||
onStopped: async () => {
|
||||
stopped = true;
|
||||
if (activeEntries.size === 0) {
|
||||
activeEntries.clear();
|
||||
return;
|
||||
|
||||
Reference in New Issue
Block a user