diff --git a/src/gateway/server-methods/server-methods.test.ts b/src/gateway/server-methods/server-methods.test.ts index 9fa3b49f2db..45b6cc0da57 100644 --- a/src/gateway/server-methods/server-methods.test.ts +++ b/src/gateway/server-methods/server-methods.test.ts @@ -41,6 +41,31 @@ function countMatching(items: readonly T[], predicate: (item: T) => boolean): return count; } +function expectRecordFields(record: unknown, expected: Record) { + expect(record).toBeDefined(); + const actual = record as Record; + for (const [key, value] of Object.entries(expected)) { + expect(actual[key]).toEqual(value); + } + return actual; +} + +function mockCallArg(mock: ReturnType, callIndex = 0, argIndex = 0) { + const call = mock.mock.calls[callIndex]; + if (!call) { + throw new Error(`Expected mock call ${callIndex}`); + } + return call[argIndex]; +} + +function lastMockCallArg(mock: ReturnType, argIndex = 0) { + const call = mock.mock.calls.at(-1); + if (!call) { + throw new Error("Expected mock call"); + } + return call[argIndex]; +} + describe("waitForAgentJob", () => { async function runLifecycleScenario(params: { runIdPrefix: string; @@ -84,7 +109,7 @@ describe("waitForAgentJob", () => { await vi.advanceTimersByTimeAsync(15_000); const snapshot = await snapshotPromise; - expect(snapshot).toMatchObject({ + expectRecordFields(snapshot, { status: "timeout", startedAt: 100, endedAt: 200, @@ -100,7 +125,7 @@ describe("waitForAgentJob", () => { startedAt: 300, endedAt: 400, }); - expect(snapshot).toMatchObject({ + expectRecordFields(snapshot, { status: "ok", startedAt: 300, endedAt: 400, @@ -131,7 +156,7 @@ describe("waitForAgentJob", () => { }); const snapshot = await waitPromise; - expect(snapshot).toMatchObject({ + expectRecordFields(snapshot, { status: "ok", startedAt: 500, endedAt: 700, @@ -162,7 +187,7 @@ describe("waitForAgentJob", () => { await vi.advanceTimersByTimeAsync(15_000); const snapshot = await waitPromise; - expect(snapshot).toMatchObject({ + expectRecordFields(snapshot, { status: "timeout", startedAt: 800, endedAt: 1_000, @@ -197,7 +222,7 @@ describe("waitForAgentJob", () => { await vi.advanceTimersByTimeAsync(15_000); const snapshot = await waitPromise; - expect(snapshot).toMatchObject({ + expectRecordFields(snapshot, { status: "error", startedAt: 1_100, endedAt: 1_300, @@ -934,13 +959,11 @@ describe("exec approval handlers", () => { nodeId: undefined, }, }); - expect(respond).toHaveBeenCalledWith( - false, - undefined, - expect.objectContaining({ - message: "nodeId is required for host=node", - }), - ); + expect(mockCallArg(respond)).toBe(false); + expect(mockCallArg(respond, 0, 1)).toBeUndefined(); + expectRecordFields(mockCallArg(respond, 0, 2), { + message: "nodeId is required for host=node", + }); }); it("rejects host=node approval requests without systemRunPlan", async () => { @@ -953,13 +976,11 @@ describe("exec approval handlers", () => { systemRunPlan: undefined, }, }); - expect(respond).toHaveBeenCalledWith( - false, - undefined, - expect.objectContaining({ - message: "systemRunPlan is required for host=node", - }), - ); + expect(mockCallArg(respond)).toBe(false); + expect(mockCallArg(respond, 0, 1)).toBeUndefined(); + expectRecordFields(mockCallArg(respond, 0, 2), { + message: "systemRunPlan is required for host=node", + }); }); it("rejects whitespace-only approval commands without trimming display text", async () => { @@ -975,13 +996,9 @@ describe("exec approval handlers", () => { systemRunPlan: undefined, }, }); - expect(respond).toHaveBeenCalledWith( - false, - undefined, - expect.objectContaining({ - message: "command is required", - }), - ); + expect(mockCallArg(respond)).toBe(false); + expect(mockCallArg(respond, 0, 1)).toBeUndefined(); + expectRecordFields(mockCallArg(respond, 0, 2), { message: "command is required" }); }); it("returns pending approval details for exec.approval.get", async () => { @@ -1008,18 +1025,17 @@ describe("exec approval handlers", () => { const getRespond = vi.fn(); await getExecApproval({ handlers, id, respond: getRespond }); - expect(getRespond).toHaveBeenCalledWith( - true, - expect.objectContaining({ - id, - commandText: "echo ok", - allowedDecisions: expect.arrayContaining(["allow-once", "allow-always", "deny"]), - host: "gateway", - nodeId: null, - agentId: null, - }), - undefined, - ); + expect(mockCallArg(getRespond)).toBe(true); + const approval = mockCallArg(getRespond, 0, 1) as Record; + expectRecordFields(approval, { + id, + commandText: "echo ok", + host: "gateway", + nodeId: null, + agentId: null, + }); + expect(approval.allowedDecisions).toEqual(["allow-once", "allow-always", "deny"]); + expect(mockCallArg(getRespond, 0, 2)).toBeUndefined(); const resolveRespond = vi.fn(); await resolveExecApproval({ @@ -1050,13 +1066,10 @@ describe("exec approval handlers", () => { const requested = broadcasts.find((entry) => entry.event === "exec.approval.requested"); const request = requested?.payload as { id?: string; request?: { commandAnalysis?: unknown } }; - expect(request.request?.commandAnalysis).toEqual( - expect.objectContaining({ - commandCount: 1, - riskKinds: expect.arrayContaining(["inline-eval"]), - warningLines: expect.arrayContaining(["Contains inline-eval: python3 -c"]), - }), - ); + const commandAnalysis = request.request?.commandAnalysis as Record; + expect(commandAnalysis.commandCount).toBe(1); + expect(commandAnalysis.riskKinds).toEqual(["inline-eval"]); + expect(commandAnalysis.warningLines).toEqual(["Contains inline-eval: python3 -c"]); const resolveRespond = vi.fn(); await resolveExecApproval({ @@ -1086,18 +1099,12 @@ describe("exec approval handlers", () => { const listRespond = vi.fn(); await listExecApprovals({ handlers, respond: listRespond }); - expect(listRespond).toHaveBeenCalledWith( - true, - expect.arrayContaining([ - expect.objectContaining({ - id: "approval-list-1", - request: expect.objectContaining({ - command: "echo ok", - }), - }), - ]), - undefined, - ); + expect(mockCallArg(listRespond)).toBe(true); + const approvals = mockCallArg(listRespond, 0, 1) as Array>; + const approval = approvals.find((entry) => entry.id === "approval-list-1"); + expectRecordFields(approval, { id: "approval-list-1" }); + expectRecordFields((approval as Record).request, { command: "echo ok" }); + expect(mockCallArg(listRespond, 0, 2)).toBeUndefined(); const resolveRespond = vi.fn(); await resolveExecApproval({ @@ -1132,14 +1139,12 @@ describe("exec approval handlers", () => { const getRespond = vi.fn(); await getExecApproval({ handlers, id: acceptedId as string, respond: getRespond }); - expect(getRespond).toHaveBeenCalledWith( - false, - undefined, - expect.objectContaining({ - code: "INVALID_REQUEST", - message: "unknown or expired approval id", - }), - ); + expect(mockCallArg(getRespond)).toBe(false); + expect(mockCallArg(getRespond, 0, 1)).toBeUndefined(); + expectRecordFields(mockCallArg(getRespond, 0, 2), { + code: "INVALID_REQUEST", + message: "unknown or expired approval id", + }); }); it("broadcasts request + resolve", async () => { @@ -1154,11 +1159,9 @@ describe("exec approval handlers", () => { const { id } = getRequestedExecApprovalPayload(broadcasts); - expect(respond).toHaveBeenCalledWith( - true, - expect.objectContaining({ status: "accepted", id }), - undefined, - ); + expect(mockCallArg(respond)).toBe(true); + expectRecordFields(mockCallArg(respond, 0, 1), { status: "accepted", id }); + expect(mockCallArg(respond, 0, 2)).toBeUndefined(); const resolveRespond = vi.fn(); await resolveExecApproval({ @@ -1171,11 +1174,9 @@ describe("exec approval handlers", () => { await requestPromise; expect(resolveRespond).toHaveBeenCalledWith(true, { ok: true }, undefined); - expect(respond).toHaveBeenCalledWith( - true, - expect.objectContaining({ id, decision: "allow-once" }), - undefined, - ); + expect(lastMockCallArg(respond)).toBe(true); + expectRecordFields(lastMockCallArg(respond, 1), { id, decision: "allow-once" }); + expect(lastMockCallArg(respond, 2)).toBeUndefined(); expect(broadcasts.map((entry) => entry.event)).toContain("exec.approval.resolved"); }); @@ -1225,14 +1226,11 @@ describe("exec approval handlers", () => { expect(countMatching(broadcasts, (entry) => entry.event === "exec.approval.resolved")).toBe( resolvedBroadcastCount, ); - expect(conflictingResolveRespond).toHaveBeenCalledWith( - false, - undefined, - expect.objectContaining({ - message: "approval already resolved", - details: expect.objectContaining({ reason: "APPROVAL_ALREADY_RESOLVED" }), - }), - ); + expect(mockCallArg(conflictingResolveRespond)).toBe(false); + expect(mockCallArg(conflictingResolveRespond, 0, 1)).toBeUndefined(); + const error = mockCallArg(conflictingResolveRespond, 0, 2) as Record; + expect(error.message).toBe("approval already resolved"); + expectRecordFields(error.details, { reason: "APPROVAL_ALREADY_RESOLVED" }); }); it("rejects allow-always when the request ask mode is always", async () => { @@ -1256,14 +1254,12 @@ describe("exec approval handlers", () => { context, }); - expect(resolveRespond).toHaveBeenCalledWith( - false, - undefined, - expect.objectContaining({ - message: - "allow-always is unavailable because the effective policy requires approval every time", - }), - ); + expect(mockCallArg(resolveRespond)).toBe(false); + expect(mockCallArg(resolveRespond, 0, 1)).toBeUndefined(); + expectRecordFields(mockCallArg(resolveRespond, 0, 2), { + message: + "allow-always is unavailable because the effective policy requires approval every time", + }); const denyRespond = vi.fn(); await resolveExecApproval({ @@ -1498,11 +1494,9 @@ describe("exec approval handlers", () => { }, }); const requested = broadcasts.find((entry) => entry.event === "exec.approval.requested"); - expect(requested).toEqual(expect.objectContaining({ event: "exec.approval.requested" })); + expectRecordFields(requested, { event: "exec.approval.requested" }); const request = (requested?.payload as { request?: Record })?.request ?? {}; - expect(request["commandAnalysis"]).toEqual( - expect.objectContaining({ commandCount: 1, nestedCommandCount: 0 }), - ); + expectRecordFields(request["commandAnalysis"], { commandCount: 1, nestedCommandCount: 0 }); expect(request["commandSpans"]).toEqual([ { startIndex: 0, endIndex: 2 }, { startIndex: 5, endIndex: 11 }, @@ -1562,11 +1556,9 @@ describe("exec approval handlers", () => { }); expect(resolveRespond).toHaveBeenCalledWith(true, { ok: true }, undefined); - expect(respond).toHaveBeenCalledWith( - true, - expect.objectContaining({ decision: "allow-once" }), - undefined, - ); + expect(lastMockCallArg(respond)).toBe(true); + expectRecordFields(lastMockCallArg(respond, 1), { decision: "allow-once" }); + expect(lastMockCallArg(respond, 2)).toBeUndefined(); }); it("accepts explicit approval ids", async () => { @@ -1592,11 +1584,12 @@ describe("exec approval handlers", () => { }); await requestPromise; - expect(respond).toHaveBeenCalledWith( - true, - expect.objectContaining({ id: "approval-123", decision: "allow-once" }), - undefined, - ); + expect(lastMockCallArg(respond)).toBe(true); + expectRecordFields(lastMockCallArg(respond, 1), { + id: "approval-123", + decision: "allow-once", + }); + expect(lastMockCallArg(respond, 2)).toBeUndefined(); expect(resolveRespond).toHaveBeenCalledWith(true, { ok: true }, undefined); }); @@ -1610,14 +1603,12 @@ describe("exec approval handlers", () => { params: { id: "plugin:approval-123", host: "gateway" }, }); - expect(respond).toHaveBeenCalledWith( - false, - undefined, - expect.objectContaining({ - code: "INVALID_REQUEST", - message: "approval ids starting with plugin: are reserved", - }), - ); + expect(mockCallArg(respond)).toBe(false); + expect(mockCallArg(respond, 0, 1)).toBeUndefined(); + expectRecordFields(mockCallArg(respond, 0, 2), { + code: "INVALID_REQUEST", + message: "approval ids starting with plugin: are reserved", + }); }); it("accepts unique short approval id prefixes", async () => { @@ -1666,13 +1657,11 @@ describe("exec approval handlers", () => { context, }); - expect(respond).toHaveBeenCalledWith( - false, - undefined, - expect.objectContaining({ - message: "ambiguous approval id prefix; use the full id", - }), - ); + expect(mockCallArg(respond)).toBe(false); + expect(mockCallArg(respond, 0, 1)).toBeUndefined(); + expectRecordFields(mockCallArg(respond, 0, 2), { + message: "ambiguous approval id prefix; use the full id", + }); }); it("returns deterministic unknown/expired message for missing approval ids", async () => { @@ -1685,15 +1674,14 @@ describe("exec approval handlers", () => { context, }); - expect(respond).toHaveBeenCalledWith( - false, - undefined, - expect.objectContaining({ - code: "INVALID_REQUEST", - message: "unknown or expired approval id", - details: expect.objectContaining({ reason: "APPROVAL_NOT_FOUND" }), - }), - ); + expect(mockCallArg(respond)).toBe(false); + expect(mockCallArg(respond, 0, 1)).toBeUndefined(); + const error = mockCallArg(respond, 0, 2) as Record; + expectRecordFields(error, { + code: "INVALID_REQUEST", + message: "unknown or expired approval id", + }); + expectRecordFields(error.details, { reason: "APPROVAL_NOT_FOUND" }); }); it("resolves only the targeted approval id when multiple requests are pending", async () => { @@ -1738,16 +1726,18 @@ describe("exec approval handlers", () => { await requestOne; await requestTwo; - expect(respondOne).toHaveBeenCalledWith( - true, - expect.objectContaining({ id: "approval-one", decision: "allow-once" }), - undefined, - ); - expect(respondTwo).toHaveBeenCalledWith( - true, - expect.objectContaining({ id: "approval-two", decision: null }), - undefined, - ); + expect(lastMockCallArg(respondOne)).toBe(true); + expectRecordFields(lastMockCallArg(respondOne, 1), { + id: "approval-one", + decision: "allow-once", + }); + expect(lastMockCallArg(respondOne, 2)).toBeUndefined(); + expect(lastMockCallArg(respondTwo)).toBe(true); + expectRecordFields(lastMockCallArg(respondTwo, 1), { + id: "approval-two", + decision: null, + }); + expect(lastMockCallArg(respondTwo, 2)).toBeUndefined(); }); it("forwards turn-source metadata to exec approval forwarding", async () => { @@ -1769,16 +1759,13 @@ describe("exec approval handlers", () => { }); await drainApprovalRequestTicks(); expect(forwarder.handleRequested).toHaveBeenCalledTimes(1); - expect(forwarder.handleRequested).toHaveBeenCalledWith( - expect.objectContaining({ - request: expect.objectContaining({ - turnSourceChannel: "whatsapp", - turnSourceTo: "+15555550123", - turnSourceAccountId: "work", - turnSourceThreadId: "1739201675.123", - }), - }), - ); + const forwarded = mockCallArg(forwarder.handleRequested) as Record; + expectRecordFields(forwarded.request, { + turnSourceChannel: "whatsapp", + turnSourceTo: "+15555550123", + turnSourceAccountId: "work", + turnSourceThreadId: "1739201675.123", + }); await vi.runOnlyPendingTimersAsync(); await requestPromise; @@ -1828,31 +1815,26 @@ describe("exec approval handlers", () => { await requestPromise; expect(resolveRespond).toHaveBeenCalledWith(true, { ok: true }, undefined); - expect(forwarder.handleResolved).toHaveBeenCalledWith( - expect.objectContaining({ - id: "approval-control-ui-multichannel", - decision: "allow-once", - request: expect.objectContaining({ - sessionKey: "agent:main:feishu:chat-123", - turnSourceChannel: "feishu", - turnSourceTo: "chat-123", - turnSourceAccountId: "work", - turnSourceThreadId: "thread-456", - }), - }), - ); - expect(broadcasts).toContainEqual( - expect.objectContaining({ - event: "exec.approval.resolved", - payload: expect.objectContaining({ - id: "approval-control-ui-multichannel", - request: expect.objectContaining({ - turnSourceChannel: "feishu", - turnSourceTo: "chat-123", - }), - }), - }), - ); + const resolved = mockCallArg(forwarder.handleResolved) as Record; + expectRecordFields(resolved, { + id: "approval-control-ui-multichannel", + decision: "allow-once", + }); + expectRecordFields(resolved.request, { + sessionKey: "agent:main:feishu:chat-123", + turnSourceChannel: "feishu", + turnSourceTo: "chat-123", + turnSourceAccountId: "work", + turnSourceThreadId: "thread-456", + }); + const resolvedBroadcast = broadcasts.find((entry) => entry.event === "exec.approval.resolved"); + expect(resolvedBroadcast?.event).toBe("exec.approval.resolved"); + const payload = resolvedBroadcast?.payload as Record; + expect(payload.id).toBe("approval-control-ui-multichannel"); + expectRecordFields(payload.request, { + turnSourceChannel: "feishu", + turnSourceTo: "chat-123", + }); }); it("fast-fails approvals when no approver clients and no forwarding targets", async () => { @@ -1869,11 +1851,12 @@ describe("exec approval handlers", () => { expect(forwarder.handleRequested).toHaveBeenCalledTimes(1); expect(expireSpy).toHaveBeenCalledWith("approval-no-approver", "no-approval-route"); - expect(respond).toHaveBeenCalledWith( - true, - expect.objectContaining({ id: "approval-no-approver", decision: null }), - undefined, - ); + expect(lastMockCallArg(respond)).toBe(true); + expectRecordFields(lastMockCallArg(respond, 1), { + id: "approval-no-approver", + decision: null, + }); + expect(lastMockCallArg(respond, 2)).toBeUndefined(); }); it("keeps approvals pending when iOS push delivery accepted the request", async () => { @@ -1900,17 +1883,16 @@ describe("exec approval handlers", () => { }); await vi.waitFor(() => { - expect(respond).toHaveBeenCalledWith( - true, - expect.objectContaining({ status: "accepted", id: "approval-ios-push" }), - undefined, - ); + expect(lastMockCallArg(respond)).toBe(true); + expectRecordFields(lastMockCallArg(respond, 1), { + status: "accepted", + id: "approval-ios-push", + }); + expect(lastMockCallArg(respond, 2)).toBeUndefined(); }); expect(forwarder.handleRequested).toHaveBeenCalledTimes(1); - expect(iosPushDelivery.handleRequested).toHaveBeenCalledWith( - expect.objectContaining({ id: "approval-ios-push" }), - ); + expectRecordFields(mockCallArg(iosPushDelivery.handleRequested), { id: "approval-ios-push" }); expect(expireSpy).not.toHaveBeenCalled(); manager.resolve("approval-ios-push", "allow-once"); @@ -1943,9 +1925,10 @@ describe("exec approval handlers", () => { await requestPromise; await vi.waitFor(() => { - expect(iosPushDelivery.handleResolved).toHaveBeenCalledWith( - expect.objectContaining({ id: "approval-ios-cleanup", decision: "allow-once" }), - ); + expectRecordFields(mockCallArg(iosPushDelivery.handleResolved), { + id: "approval-ios-cleanup", + decision: "allow-once", + }); }); }); @@ -1977,9 +1960,9 @@ describe("exec approval handlers", () => { await requestPromise; await vi.waitFor(() => { - expect(iosPushDelivery.handleExpired).toHaveBeenCalledWith( - expect.objectContaining({ id: "approval-ios-expire" }), - ); + expectRecordFields(mockCallArg(iosPushDelivery.handleExpired), { + id: "approval-ios-expire", + }); }); } finally { vi.useRealTimers(); @@ -2008,11 +1991,12 @@ describe("exec approval handlers", () => { }); await vi.waitFor(() => { - expect(respond).toHaveBeenCalledWith( - true, - expect.objectContaining({ status: "accepted", id: "approval-chat-route" }), - undefined, - ); + expect(lastMockCallArg(respond)).toBe(true); + expectRecordFields(lastMockCallArg(respond, 1), { + status: "accepted", + id: "approval-chat-route", + }); + expect(lastMockCallArg(respond, 2)).toBeUndefined(); }); expect(forwarder.handleRequested).toHaveBeenCalledTimes(1); @@ -2052,11 +2036,12 @@ describe("exec approval handlers", () => { await requestPromise; expect(resolveRespond).toHaveBeenCalledWith(true, { ok: true }, undefined); - expect(respond).toHaveBeenCalledWith( - true, - expect.objectContaining({ id: "approval-forwarded", decision: "allow-once" }), - undefined, - ); + expect(lastMockCallArg(respond)).toBe(true); + expectRecordFields(lastMockCallArg(respond, 1), { + id: "approval-forwarded", + decision: "allow-once", + }); + expect(lastMockCallArg(respond, 2)).toBeUndefined(); }); }); @@ -2267,7 +2252,9 @@ describe("gateway healthHandlers.health cache freshness", () => { includeSensitive: false, }); expect(getEventLoopHealth).not.toHaveBeenCalled(); - expect(respond).toHaveBeenCalledWith(true, expect.objectContaining({ eventLoop }), undefined); + expect(mockCallArg(respond)).toBe(true); + expectRecordFields(mockCallArg(respond, 0, 1), { eventLoop }); + expect(mockCallArg(respond, 0, 2)).toBeUndefined(); }); it("refreshes cached health when a runtime account is missing from the cached account summary", async () => { @@ -2381,14 +2368,12 @@ describe("logs.tail", () => { isWebchatConnect: logsNoop, }); - expect(respond).toHaveBeenCalledWith( - true, - expect.objectContaining({ - file: newer, - lines: ['{"msg":"new"}'], - }), - undefined, - ); + expect(mockCallArg(respond)).toBe(true); + expectRecordFields(mockCallArg(respond, 0, 1), { + file: newer, + lines: ['{"msg":"new"}'], + }); + expect(mockCallArg(respond, 0, 2)).toBeUndefined(); await fsPromises.rm(tempDir, { recursive: true, force: true }); }); @@ -2414,14 +2399,12 @@ describe("logs.tail", () => { isWebchatConnect: logsNoop, }); - expect(respond).toHaveBeenCalledWith( - true, - expect.objectContaining({ - file, - lines: ["starting gog gmail watch serve --token push-t…bbbb --hook-token hook-t…aaaa"], - }), - undefined, - ); + expect(mockCallArg(respond)).toBe(true); + expectRecordFields(mockCallArg(respond, 0, 1), { + file, + lines: ["starting gog gmail watch serve --token push-t…bbbb --hook-token hook-t…aaaa"], + }); + expect(mockCallArg(respond, 0, 2)).toBeUndefined(); await fsPromises.rm(tempDir, { recursive: true, force: true }); });