fix: prevent exec approval resolve race
This commit is contained in:
@@ -105,6 +105,57 @@ describe("exec approval handlers", () => {
|
|||||||
expect(broadcasts.some((entry) => entry.event === "exec.approval.resolved")).toBe(true);
|
expect(broadcasts.some((entry) => entry.event === "exec.approval.resolved")).toBe(true);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("accepts resolve during broadcast", async () => {
|
||||||
|
const manager = new ExecApprovalManager();
|
||||||
|
const handlers = createExecApprovalHandlers(manager);
|
||||||
|
const respond = vi.fn();
|
||||||
|
const resolveRespond = vi.fn();
|
||||||
|
|
||||||
|
const resolveContext = {
|
||||||
|
broadcast: () => {},
|
||||||
|
};
|
||||||
|
|
||||||
|
const context = {
|
||||||
|
broadcast: (event: string, payload: unknown) => {
|
||||||
|
if (event !== "exec.approval.requested") return;
|
||||||
|
const id = (payload as { id?: string })?.id ?? "";
|
||||||
|
void handlers["exec.approval.resolve"]({
|
||||||
|
params: { id, decision: "allow-once" },
|
||||||
|
respond: resolveRespond,
|
||||||
|
context: resolveContext as unknown as Parameters<
|
||||||
|
(typeof handlers)["exec.approval.resolve"]
|
||||||
|
>[0]["context"],
|
||||||
|
client: { connect: { client: { id: "cli", displayName: "CLI" } } },
|
||||||
|
req: { id: "req-2", type: "req", method: "exec.approval.resolve" },
|
||||||
|
isWebchatConnect: noop,
|
||||||
|
});
|
||||||
|
},
|
||||||
|
};
|
||||||
|
|
||||||
|
await handlers["exec.approval.request"]({
|
||||||
|
params: {
|
||||||
|
command: "echo ok",
|
||||||
|
cwd: "/tmp",
|
||||||
|
host: "node",
|
||||||
|
timeoutMs: 2000,
|
||||||
|
},
|
||||||
|
respond,
|
||||||
|
context: context as unknown as Parameters<
|
||||||
|
(typeof handlers)["exec.approval.request"]
|
||||||
|
>[0]["context"],
|
||||||
|
client: null,
|
||||||
|
req: { id: "req-1", type: "req", method: "exec.approval.request" },
|
||||||
|
isWebchatConnect: noop,
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(resolveRespond).toHaveBeenCalledWith(true, { ok: true }, undefined);
|
||||||
|
expect(respond).toHaveBeenCalledWith(
|
||||||
|
true,
|
||||||
|
expect.objectContaining({ decision: "allow-once" }),
|
||||||
|
undefined,
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
it("accepts explicit approval ids", async () => {
|
it("accepts explicit approval ids", async () => {
|
||||||
const manager = new ExecApprovalManager();
|
const manager = new ExecApprovalManager();
|
||||||
const handlers = createExecApprovalHandlers(manager);
|
const handlers = createExecApprovalHandlers(manager);
|
||||||
|
|||||||
@@ -58,6 +58,7 @@ export function createExecApprovalHandlers(manager: ExecApprovalManager): Gatewa
|
|||||||
sessionKey: p.sessionKey ?? null,
|
sessionKey: p.sessionKey ?? null,
|
||||||
};
|
};
|
||||||
const record = manager.create(request, timeoutMs, explicitId);
|
const record = manager.create(request, timeoutMs, explicitId);
|
||||||
|
const decisionPromise = manager.waitForDecision(record, timeoutMs);
|
||||||
context.broadcast(
|
context.broadcast(
|
||||||
"exec.approval.requested",
|
"exec.approval.requested",
|
||||||
{
|
{
|
||||||
@@ -68,7 +69,7 @@ export function createExecApprovalHandlers(manager: ExecApprovalManager): Gatewa
|
|||||||
},
|
},
|
||||||
{ dropIfSlow: true },
|
{ dropIfSlow: true },
|
||||||
);
|
);
|
||||||
const decision = await manager.waitForDecision(record, timeoutMs);
|
const decision = await decisionPromise;
|
||||||
respond(
|
respond(
|
||||||
true,
|
true,
|
||||||
{
|
{
|
||||||
|
|||||||
Reference in New Issue
Block a user