diff --git a/src/agents/tools/gateway-tool.test.ts b/src/agents/tools/gateway-tool.test.ts index 2e3cab0f520..5ea062a7ed5 100644 --- a/src/agents/tools/gateway-tool.test.ts +++ b/src/agents/tools/gateway-tool.test.ts @@ -1,4 +1,4 @@ -import { describe, expect, it, vi } from "vitest"; +import { beforeEach, describe, expect, it, vi } from "vitest"; const mocks = vi.hoisted(() => ({ isRestartEnabled: vi.fn(() => true), @@ -52,46 +52,54 @@ function getCallArg(mockFn: { mock: { calls: unknown[] } }, callIdx: number, return calls[callIdx]?.[argIdx] as T; } -describe("createGatewayTool – live delivery context guard", () => { - it("does not forward liveDeliveryContextForRpc when agentTo is missing", async () => { - mocks.callGatewayTool.mockClear(); - const tool = createGatewayTool({ - agentSessionKey: "agent:main:main", - agentChannel: "discord", - agentTo: undefined, // intentionally missing - }); +// ───────────────────────────────────────────────────────────────────────────── +// Helpers to build common test fixtures +// ───────────────────────────────────────────────────────────────────────────── - await execTool(tool, { - action: "config.patch", - raw: '{"key":"value"}', - baseHash: "abc123", - sessionKey: "agent:main:main", - note: "test patch", - }); +function makePatchParams(overrides: Record = {}) { + return { + action: "config.patch", + raw: '{"key":"value"}', + baseHash: "abc123", + sessionKey: "agent:main:main", + note: "test patch", + ...overrides, + }; +} - const forwardedParams = getCallArg>(mocks.callGatewayTool, 0, 2); - // deliveryContext should be undefined — falling back to server-side extractDeliveryInfo - expect(forwardedParams?.deliveryContext).toBeUndefined(); +function makeTool( + opts: { + agentSessionKey?: string; + agentChannel?: string; + agentTo?: string; + agentThreadId?: string; + agentAccountId?: string; + } = {}, +) { + return createGatewayTool({ + agentSessionKey: "agent:main:main", + agentChannel: "discord", + agentTo: "123456789", + ...opts, + }); +} + +// ───────────────────────────────────────────────────────────────────────────── +// Suite 1 — Live delivery context for RPC actions (config.apply / config.patch / update.run) +// ───────────────────────────────────────────────────────────────────────────── + +describe("createGatewayTool – RPC delivery context forwarding", () => { + beforeEach(() => { + vi.clearAllMocks(); }); - it("forwards liveDeliveryContextForRpc when both agentChannel and agentTo are present", async () => { - mocks.callGatewayTool.mockClear(); - const tool = createGatewayTool({ - agentSessionKey: "agent:main:main", - agentChannel: "discord", - agentTo: "123456789", - }); + // ── Happy path: full live context forwarded ────────────────────────────── - await execTool(tool, { - action: "config.patch", - raw: '{"key":"value"}', - baseHash: "abc123", - sessionKey: "agent:main:main", - note: "test patch", - }); + it("forwards liveDeliveryContext when agentChannel and agentTo are both present", async () => { + await execTool(makeTool(), makePatchParams()); - const forwardedParams = getCallArg>(mocks.callGatewayTool, 0, 2); - expect(forwardedParams?.deliveryContext).toEqual({ + const p = getCallArg>(mocks.callGatewayTool, 0, 2); + expect(p?.deliveryContext).toEqual({ channel: "discord", to: "123456789", accountId: undefined, @@ -99,207 +107,343 @@ describe("createGatewayTool – live delivery context guard", () => { }); }); - it("includes threadId in liveDeliveryContextForRpc when agentThreadId is present", async () => { - mocks.callGatewayTool.mockClear(); - const tool = createGatewayTool({ - agentSessionKey: "agent:main:main", - agentChannel: "slack", - agentTo: "C012AB3CD", - agentThreadId: "1234567890.123456", - }); + it("includes agentAccountId in forwarded context when provided", async () => { + await execTool(makeTool({ agentAccountId: "acct-99" }), makePatchParams()); - await execTool(tool, { - action: "config.patch", + const p = getCallArg>(mocks.callGatewayTool, 0, 2); + expect((p?.deliveryContext as Record)?.accountId).toBe("acct-99"); + }); + + it("includes agentThreadId in forwarded context when provided", async () => { + await execTool( + makeTool({ agentChannel: "slack", agentTo: "C012AB3CD", agentThreadId: "1234567890.123" }), + makePatchParams({ sessionKey: "agent:main:main" }), + ); + + const p = getCallArg>(mocks.callGatewayTool, 0, 2); + expect((p?.deliveryContext as Record)?.threadId).toBe("1234567890.123"); + expect((p?.deliveryContext as Record)?.channel).toBe("slack"); + }); + + it("forwards live context for config.apply as well as config.patch", async () => { + await execTool(makeTool(), { + action: "config.apply", raw: '{"key":"value"}', baseHash: "abc123", sessionKey: "agent:main:main", - note: "test patch", }); - const forwardedParams = getCallArg>(mocks.callGatewayTool, 0, 2); - expect(forwardedParams?.deliveryContext).toEqual({ - channel: "slack", - to: "C012AB3CD", - accountId: undefined, - threadId: "1234567890.123456", - }); + const p = getCallArg>(mocks.callGatewayTool, 0, 2); + expect(p?.deliveryContext).toBeDefined(); + expect((p?.deliveryContext as Record)?.channel).toBe("discord"); }); - it("does not forward live restart context when agentTo is missing", async () => { - mocks.writeRestartSentinel.mockClear(); + it("forwards live context for update.run", async () => { + await execTool(makeTool(), { + action: "update.run", + sessionKey: "agent:main:main", + }); + + const p = getCallArg>(mocks.callGatewayTool, 0, 2); + expect(p?.deliveryContext).toBeDefined(); + expect((p?.deliveryContext as Record)?.channel).toBe("discord"); + }); + + // ── Partial live context — must be suppressed ──────────────────────────── + + it("suppresses deliveryContext when agentTo is missing", async () => { + await execTool(makeTool({ agentTo: undefined }), makePatchParams()); + + const p = getCallArg>(mocks.callGatewayTool, 0, 2); + expect(p?.deliveryContext).toBeUndefined(); + }); + + it("suppresses deliveryContext when agentChannel is missing", async () => { + await execTool(makeTool({ agentChannel: undefined }), makePatchParams()); + + const p = getCallArg>(mocks.callGatewayTool, 0, 2); + expect(p?.deliveryContext).toBeUndefined(); + }); + + it("suppresses deliveryContext when agentChannel is an empty string", async () => { + await execTool(makeTool({ agentChannel: "" }), makePatchParams()); + + const p = getCallArg>(mocks.callGatewayTool, 0, 2); + expect(p?.deliveryContext).toBeUndefined(); + }); + + it("falls back to server extractDeliveryInfo when live context is suppressed", async () => { + // Confirm the RPC call still goes through — server side will use extractDeliveryInfo + await execTool(makeTool({ agentTo: undefined }), makePatchParams()); + + expect(mocks.callGatewayTool).toHaveBeenCalled(); + const p = getCallArg>(mocks.callGatewayTool, 0, 2); + expect(p?.deliveryContext).toBeUndefined(); + }); + + // ── Stale heartbeat override prevention ───────────────────────────────── + + it("overrides stale heartbeat deliveryContext from extractDeliveryInfo with live context", async () => { + // extractDeliveryInfo returning heartbeat sink — must not win over live context mocks.extractDeliveryInfo.mockReturnValueOnce({ - deliveryContext: { channel: "telegram", to: "+19995550001", accountId: undefined }, + deliveryContext: { channel: "webchat", to: "heartbeat", accountId: undefined }, threadId: undefined, }); - const tool = createGatewayTool({ - agentSessionKey: "agent:main:main", - agentChannel: "discord", - agentTo: undefined, // intentionally missing - }); + await execTool(makeTool(), makePatchParams()); - await execTool(tool, { action: "restart" }); - - const sentinelPayload = getCallArg<{ deliveryContext?: { channel?: string; to?: string } }>( - mocks.writeRestartSentinel, - 0, - 0, - ); - // Should fall back to extractDeliveryInfo() result, not the incomplete live context - expect(sentinelPayload?.deliveryContext?.channel).toBe("telegram"); - expect(sentinelPayload?.deliveryContext?.to).toBe("+19995550001"); + const p = getCallArg>(mocks.callGatewayTool, 0, 2); + expect((p?.deliveryContext as Record)?.channel).toBe("discord"); + expect((p?.deliveryContext as Record)?.to).toBe("123456789"); }); - it("uses live restart context when both agentChannel and agentTo are present", async () => { - mocks.writeRestartSentinel.mockClear(); + // ── Session key targeting: same-session ───────────────────────────────── - const tool = createGatewayTool({ - agentSessionKey: "agent:main:main", - agentChannel: "discord", - agentTo: "123456789", - }); - - await execTool(tool, { action: "restart" }); - - const sentinelPayload = getCallArg<{ deliveryContext?: { channel?: string; to?: string } }>( - mocks.writeRestartSentinel, - 0, - 0, + it("forwards live context when sessionKey matches own session key exactly", async () => { + await execTool( + makeTool({ agentSessionKey: "agent:main:main" }), + makePatchParams({ sessionKey: "agent:main:main" }), ); - expect(sentinelPayload?.deliveryContext?.channel).toBe("discord"); - expect(sentinelPayload?.deliveryContext?.to).toBe("123456789"); + + const p = getCallArg>(mocks.callGatewayTool, 0, 2); + expect(p?.deliveryContext).toBeDefined(); }); - it("does not forward live RPC delivery context when gateway.mode=remote is configured (no URL override)", async () => { - // When gateway.mode=remote is set in config, callGatewayTool() routes to - // gateway.remote.url without an explicit gatewayUrl param. resolveGatewayTarget - // must return "remote" in this case so deliveryContext is suppressed, preventing - // the remote sentinel from being stamped with the local chat route. - mocks.callGatewayTool.mockClear(); - // No gatewayUrl override — config-based remote mode + it("forwards live context when 'main' alias resolves to own default-agent session", async () => { + // agentSessionKey is "agent:main:main"; sessionKey "main" should canonicalize to the same + await execTool( + makeTool({ agentSessionKey: "agent:main:main" }), + makePatchParams({ sessionKey: "main" }), + ); + + const p = getCallArg>(mocks.callGatewayTool, 0, 2); + expect(p?.deliveryContext).toBeDefined(); + expect((p?.deliveryContext as Record)?.channel).toBe("discord"); + }); + + it("forwards live context when sessionKey is omitted (defaults to own session)", async () => { + await execTool(makeTool(), makePatchParams({ sessionKey: undefined })); + + const p = getCallArg>(mocks.callGatewayTool, 0, 2); + expect(p?.deliveryContext).toBeDefined(); + }); + + // ── Session key targeting: cross-session / cross-agent ─────────────────── + + it("suppresses deliveryContext when sessionKey targets a different session", async () => { + await execTool( + makeTool({ agentSessionKey: "agent:main:main" }), + makePatchParams({ sessionKey: "agent:other-claw:main" }), + ); + + const p = getCallArg>(mocks.callGatewayTool, 0, 2); + expect(p?.deliveryContext).toBeUndefined(); + }); + + it("suppresses deliveryContext when non-default agent passes sessionKey='main' (cross-agent alias)", async () => { + // "main" resolves to "agent:main:main" (default), not "agent:shopping-claw:main" + await execTool( + makeTool({ agentSessionKey: "agent:shopping-claw:main" }), + makePatchParams({ sessionKey: "main" }), + ); + + const p = getCallArg>(mocks.callGatewayTool, 0, 2); + expect(p?.deliveryContext).toBeUndefined(); + }); + + // ── Remote gateway targeting ───────────────────────────────────────────── + + it("suppresses deliveryContext when resolveGatewayTarget returns 'remote' (explicit URL)", async () => { + mocks.readGatewayCallOptions.mockReturnValueOnce({ gatewayUrl: "wss://remote.example.com" }); + mocks.resolveGatewayTarget.mockReturnValueOnce("remote"); + + await execTool( + makeTool(), + makePatchParams({ gatewayUrl: "wss://remote.example.com", sessionKey: "agent:main:main" }), + ); + + const p = getCallArg>(mocks.callGatewayTool, 0, 2); + expect(p?.deliveryContext).toBeUndefined(); + }); + + it("suppresses deliveryContext when resolveGatewayTarget returns 'remote' (config gateway.mode=remote)", async () => { mocks.readGatewayCallOptions.mockReturnValueOnce({}); mocks.resolveGatewayTarget.mockReturnValueOnce("remote"); - const tool = createGatewayTool({ - agentSessionKey: "agent:main:main", - agentChannel: "discord", - agentTo: "123456789", - }); - await execTool(tool, { - action: "config.patch", - raw: '{"key":"value"}', - baseHash: "abc123", - sessionKey: "agent:main:main", - note: "config-remote patch (no URL override)", - }); + await execTool(makeTool(), makePatchParams({ sessionKey: "agent:main:main" })); - const forwardedParams = getCallArg>(mocks.callGatewayTool, 0, 2); - expect(forwardedParams?.deliveryContext).toBeUndefined(); + const p = getCallArg>(mocks.callGatewayTool, 0, 2); + expect(p?.deliveryContext).toBeUndefined(); }); - it("does not forward live RPC delivery context when gatewayUrl targets a remote gateway", async () => { - // A remote gateway has its own extractDeliveryInfo(sessionKey) — forwarding - // the local agent's deliveryContext would write a sentinel with the wrong - // destination on the remote host. - mocks.callGatewayTool.mockClear(); - mocks.readGatewayCallOptions.mockReturnValueOnce({ gatewayUrl: "wss://remote-gw.example.com" }); - mocks.resolveGatewayTarget.mockReturnValueOnce("remote"); - const tool = createGatewayTool({ - agentSessionKey: "agent:main:main", - agentChannel: "discord", - agentTo: "123456789", - }); - - await execTool(tool, { - action: "config.patch", - raw: '{"key":"value"}', - baseHash: "abc123", - sessionKey: "agent:main:main", - note: "remote patch", - gatewayUrl: "wss://remote-gw.example.com", - }); - - const forwardedParams = getCallArg>(mocks.callGatewayTool, 0, 2); - expect(forwardedParams?.deliveryContext).toBeUndefined(); - }); - - it("forwards live RPC delivery context when gatewayUrl is a local loopback override", async () => { - // A gatewayUrl pointing to 127.0.0.1/localhost/[::1] is still the local server; - // deliveryContext must be forwarded so restart sentinels use the correct chat destination. - mocks.callGatewayTool.mockClear(); - mocks.readGatewayCallOptions.mockReturnValueOnce({ - gatewayUrl: "ws://127.0.0.1:18789", - }); + it("forwards deliveryContext when resolveGatewayTarget returns 'local' (loopback URL)", async () => { + mocks.readGatewayCallOptions.mockReturnValueOnce({ gatewayUrl: "ws://127.0.0.1:18789" }); mocks.resolveGatewayTarget.mockReturnValueOnce("local"); - const tool = createGatewayTool({ - agentSessionKey: "agent:main:main", - agentChannel: "discord", - agentTo: "123456789", - }); - await execTool(tool, { - action: "config.patch", - raw: '{"key":"value"}', - baseHash: "abc123", - sessionKey: "agent:main:main", - note: "local loopback patch", - gatewayUrl: "ws://127.0.0.1:18789", - }); + await execTool( + makeTool(), + makePatchParams({ gatewayUrl: "ws://127.0.0.1:18789", sessionKey: "agent:main:main" }), + ); - const forwardedParams = getCallArg>(mocks.callGatewayTool, 0, 2); - expect(forwardedParams?.deliveryContext).toEqual({ - channel: "discord", - to: "123456789", - accountId: undefined, - }); + const p = getCallArg>(mocks.callGatewayTool, 0, 2); + expect(p?.deliveryContext).toBeDefined(); + expect((p?.deliveryContext as Record)?.channel).toBe("discord"); }); - it("does not forward live RPC delivery context when a non-default agent passes sessionKey='main'", async () => { - // "main" resolves to "agent:main:main" (default agent), which differs from the - // current session "agent:shopping-claw:main". Live context must NOT be forwarded. - mocks.callGatewayTool.mockClear(); - const tool = createGatewayTool({ - agentSessionKey: "agent:shopping-claw:main", - agentChannel: "discord", - agentTo: "123456789", - }); + it("forwards deliveryContext when resolveGatewayTarget returns undefined (default local)", async () => { + mocks.resolveGatewayTarget.mockReturnValueOnce(undefined); - await execTool(tool, { - action: "config.patch", - raw: '{"key":"value"}', - baseHash: "abc123", - sessionKey: "main", // targets default agent — different from current shopping-claw session - note: "cross-agent patch", - }); + await execTool(makeTool(), makePatchParams({ sessionKey: "agent:main:main" })); - const forwardedParams = getCallArg>(mocks.callGatewayTool, 0, 2); - expect(forwardedParams?.deliveryContext).toBeUndefined(); + const p = getCallArg>(mocks.callGatewayTool, 0, 2); + expect(p?.deliveryContext).toBeDefined(); + }); +}); + +// ───────────────────────────────────────────────────────────────────────────── +// Suite 2 — Restart sentinel context (local restart action) +// ───────────────────────────────────────────────────────────────────────────── + +describe("createGatewayTool – restart sentinel delivery context", () => { + beforeEach(() => { + vi.clearAllMocks(); }); - it("does not forward live restart context when a non-default agent passes sessionKey='main'", async () => { - // "main" resolves to "agent:main:main" (default agent), which differs from the - // current session "agent:shopping-claw:main". Sentinel must not carry this session's thread. - mocks.writeRestartSentinel.mockClear(); + it("uses live context when both agentChannel and agentTo are present", async () => { + await execTool(makeTool(), { action: "restart" }); + + const p = getCallArg<{ deliveryContext?: Record }>( + mocks.writeRestartSentinel, + 0, + 0, + ); + expect(p?.deliveryContext?.channel).toBe("discord"); + expect(p?.deliveryContext?.to).toBe("123456789"); + }); + + it("falls back to extractDeliveryInfo when agentTo is missing", async () => { mocks.extractDeliveryInfo.mockReturnValueOnce({ deliveryContext: { channel: "telegram", to: "+19995550001", accountId: undefined }, threadId: undefined, }); - const tool = createGatewayTool({ - agentSessionKey: "agent:shopping-claw:main", - agentChannel: "discord", - agentTo: "123456789", - }); + await execTool(makeTool({ agentTo: undefined }), { action: "restart" }); - await execTool(tool, { action: "restart", sessionKey: "main" }); - - const sentinelPayload = getCallArg<{ deliveryContext?: { channel?: string; to?: string } }>( + const p = getCallArg<{ deliveryContext?: Record }>( mocks.writeRestartSentinel, 0, 0, ); - // Should fall back to extractDeliveryInfo() for the targeted session, not current session's live context - expect(sentinelPayload?.deliveryContext?.channel).toBe("telegram"); - expect(sentinelPayload?.deliveryContext?.to).toBe("+19995550001"); + expect(p?.deliveryContext?.channel).toBe("telegram"); + expect(p?.deliveryContext?.to).toBe("+19995550001"); + }); + + it("falls back to extractDeliveryInfo when agentChannel is missing", async () => { + mocks.extractDeliveryInfo.mockReturnValueOnce({ + deliveryContext: { channel: "whatsapp", to: "+10000000001", accountId: undefined }, + threadId: undefined, + }); + + await execTool(makeTool({ agentChannel: undefined }), { action: "restart" }); + + const p = getCallArg<{ deliveryContext?: Record }>( + mocks.writeRestartSentinel, + 0, + 0, + ); + expect(p?.deliveryContext?.channel).toBe("whatsapp"); + }); + + it("overrides stale heartbeat context from extractDeliveryInfo with live context", async () => { + mocks.extractDeliveryInfo.mockReturnValueOnce({ + deliveryContext: { channel: "webchat", to: "heartbeat", accountId: undefined }, + threadId: undefined, + }); + + await execTool(makeTool(), { action: "restart" }); + + const p = getCallArg<{ deliveryContext?: Record }>( + mocks.writeRestartSentinel, + 0, + 0, + ); + expect(p?.deliveryContext?.channel).toBe("discord"); + expect(p?.deliveryContext?.to).toBe("123456789"); + }); + + it("includes threadId in sentinel when agentThreadId is provided (same session)", async () => { + await execTool(makeTool({ agentThreadId: "ts.123456" }), { action: "restart" }); + + const p = getCallArg<{ threadId?: string }>(mocks.writeRestartSentinel, 0, 0); + expect(p?.threadId).toBe("ts.123456"); + }); + + it("uses extractDeliveryInfo threadId when targeting a different session", async () => { + mocks.extractDeliveryInfo.mockReturnValueOnce({ + deliveryContext: { channel: "telegram", to: "+19995550001", accountId: undefined }, + threadId: "extracted-thread", + }); + + await execTool(makeTool({ agentThreadId: "local-thread" }), { + action: "restart", + sessionKey: "agent:other-claw:main", + }); + + const p = getCallArg<{ threadId?: string }>(mocks.writeRestartSentinel, 0, 0); + expect(p?.threadId).toBe("extracted-thread"); + }); + + it("suppresses live context and uses extractDeliveryInfo when sessionKey targets another session", async () => { + mocks.extractDeliveryInfo.mockReturnValueOnce({ + deliveryContext: { channel: "signal", to: "+15550001", accountId: undefined }, + threadId: undefined, + }); + + await execTool(makeTool(), { action: "restart", sessionKey: "agent:other-agent:main" }); + + const p = getCallArg<{ deliveryContext?: Record }>( + mocks.writeRestartSentinel, + 0, + 0, + ); + expect(p?.deliveryContext?.channel).toBe("signal"); + expect(p?.deliveryContext?.to).toBe("+15550001"); + }); + + it("suppresses live context when non-default agent targets sessionKey='main' (cross-agent alias)", async () => { + mocks.extractDeliveryInfo.mockReturnValueOnce({ + deliveryContext: { channel: "telegram", to: "+19995550001", accountId: undefined }, + threadId: undefined, + }); + + await execTool(makeTool({ agentSessionKey: "agent:shopping-claw:main" }), { + action: "restart", + sessionKey: "main", // resolves to "agent:main:main" — different agent + }); + + const p = getCallArg<{ deliveryContext?: Record }>( + mocks.writeRestartSentinel, + 0, + 0, + ); + expect(p?.deliveryContext?.channel).toBe("telegram"); + expect(p?.deliveryContext?.to).toBe("+19995550001"); + }); + + it("sets status=ok and kind=restart on the sentinel payload", async () => { + await execTool(makeTool(), { action: "restart" }); + + const p = getCallArg<{ kind?: string; status?: string }>(mocks.writeRestartSentinel, 0, 0); + expect(p?.kind).toBe("restart"); + expect(p?.status).toBe("ok"); + }); + + it("includes sessionKey in sentinel payload", async () => { + await execTool(makeTool({ agentSessionKey: "agent:main:main" }), { + action: "restart", + }); + + const p = getCallArg<{ sessionKey?: string }>(mocks.writeRestartSentinel, 0, 0); + expect(p?.sessionKey).toBe("agent:main:main"); }); }); diff --git a/src/gateway/server-methods/restart-request.test.ts b/src/gateway/server-methods/restart-request.test.ts index 6273ee08815..41191d5a09c 100644 --- a/src/gateway/server-methods/restart-request.test.ts +++ b/src/gateway/server-methods/restart-request.test.ts @@ -1,27 +1,89 @@ import { describe, expect, it } from "vitest"; import { parseDeliveryContextFromParams } from "./restart-request.js"; +// ───────────────────────────────────────────────────────────────────────────── +// parseDeliveryContextFromParams +// Validates that only complete, routable delivery contexts are accepted +// and that partial or malformed inputs are rejected. +// ───────────────────────────────────────────────────────────────────────────── + describe("parseDeliveryContextFromParams", () => { + // ── No context present ──────────────────────────────────────────────────── + it("returns undefined when deliveryContext is absent", () => { expect(parseDeliveryContextFromParams({})).toBeUndefined(); }); - it("returns undefined when both channel and to are missing", () => { + it("returns undefined when deliveryContext is null", () => { + expect(parseDeliveryContextFromParams({ deliveryContext: null })).toBeUndefined(); + }); + + it("returns undefined when deliveryContext is a non-object (string)", () => { + expect(parseDeliveryContextFromParams({ deliveryContext: "discord" })).toBeUndefined(); + }); + + it("returns undefined when deliveryContext is a non-object (number)", () => { + expect(parseDeliveryContextFromParams({ deliveryContext: 42 })).toBeUndefined(); + }); + + // ── Partial context — must be rejected (prevents routing ambiguity) ─────── + + it("returns undefined when both channel and to are absent", () => { expect(parseDeliveryContextFromParams({ deliveryContext: {} })).toBeUndefined(); }); - it("returns undefined when only channel is present (partial context rejected)", () => { + it("returns undefined when only channel is present (partial context)", () => { expect( parseDeliveryContextFromParams({ deliveryContext: { channel: "discord" } }), ).toBeUndefined(); }); - it("returns undefined when only to is present (partial context rejected)", () => { + it("returns undefined when only to is present (partial context)", () => { expect( parseDeliveryContextFromParams({ deliveryContext: { to: "123456789" } }), ).toBeUndefined(); }); + it("returns undefined when channel is present but to is an empty string", () => { + expect( + parseDeliveryContextFromParams({ deliveryContext: { channel: "discord", to: "" } }), + ).toBeUndefined(); + }); + + it("returns undefined when to is present but channel is an empty string", () => { + expect( + parseDeliveryContextFromParams({ deliveryContext: { channel: "", to: "123456789" } }), + ).toBeUndefined(); + }); + + it("returns undefined when channel is whitespace-only", () => { + expect( + parseDeliveryContextFromParams({ deliveryContext: { channel: " ", to: "123456789" } }), + ).toBeUndefined(); + }); + + it("returns undefined when to is whitespace-only", () => { + expect( + parseDeliveryContextFromParams({ deliveryContext: { channel: "discord", to: " " } }), + ).toBeUndefined(); + }); + + // ── Non-string field types ──────────────────────────────────────────────── + + it("returns undefined when channel is a number (type coercion not allowed)", () => { + expect( + parseDeliveryContextFromParams({ deliveryContext: { channel: 42, to: "123" } }), + ).toBeUndefined(); + }); + + it("returns undefined when to is a boolean", () => { + expect( + parseDeliveryContextFromParams({ deliveryContext: { channel: "discord", to: true } }), + ).toBeUndefined(); + }); + + // ── Complete context ────────────────────────────────────────────────────── + it("returns full context when both channel and to are present", () => { expect( parseDeliveryContextFromParams({ @@ -30,7 +92,21 @@ describe("parseDeliveryContextFromParams", () => { ).toEqual({ channel: "discord", to: "123456789", accountId: undefined, threadId: undefined }); }); - it("includes accountId and threadId when present", () => { + it("includes accountId when present", () => { + const result = parseDeliveryContextFromParams({ + deliveryContext: { channel: "discord", to: "123456789", accountId: "acct-1" }, + }); + expect(result?.accountId).toBe("acct-1"); + }); + + it("includes threadId when present", () => { + const result = parseDeliveryContextFromParams({ + deliveryContext: { channel: "slack", to: "C012AB3CD", threadId: "1234567890.123456" }, + }); + expect(result?.threadId).toBe("1234567890.123456"); + }); + + it("includes all four fields when all are present", () => { expect( parseDeliveryContextFromParams({ deliveryContext: { @@ -48,11 +124,84 @@ describe("parseDeliveryContextFromParams", () => { }); }); - it("trims whitespace from all string fields", () => { + // ── Whitespace trimming ─────────────────────────────────────────────────── + + it("trims leading/trailing whitespace from channel", () => { + const result = parseDeliveryContextFromParams({ + deliveryContext: { channel: " discord ", to: "123456789" }, + }); + expect(result?.channel).toBe("discord"); + }); + + it("trims leading/trailing whitespace from to", () => { + const result = parseDeliveryContextFromParams({ + deliveryContext: { channel: "discord", to: " 123456789 " }, + }); + expect(result?.to).toBe("123456789"); + }); + + it("trims leading/trailing whitespace from threadId", () => { + const result = parseDeliveryContextFromParams({ + deliveryContext: { channel: "discord", to: "123", threadId: " ts.1 " }, + }); + expect(result?.threadId).toBe("ts.1"); + }); + + it("trims all string fields simultaneously", () => { expect( parseDeliveryContextFromParams({ - deliveryContext: { channel: " discord ", to: " 123 ", threadId: " ts.1 " }, + deliveryContext: { + channel: " discord ", + to: " 123 ", + accountId: " acct ", + threadId: " ts.1 ", + }, }), - ).toEqual({ channel: "discord", to: "123", accountId: undefined, threadId: "ts.1" }); + ).toEqual({ channel: "discord", to: "123", accountId: "acct", threadId: "ts.1" }); + }); + + // ── Optional fields absent / undefined ─────────────────────────────────── + + it("returns undefined for accountId when not provided", () => { + const result = parseDeliveryContextFromParams({ + deliveryContext: { channel: "discord", to: "123456789" }, + }); + expect(result?.accountId).toBeUndefined(); + }); + + it("returns undefined for threadId when not provided", () => { + const result = parseDeliveryContextFromParams({ + deliveryContext: { channel: "discord", to: "123456789" }, + }); + expect(result?.threadId).toBeUndefined(); + }); + + it("returns undefined for accountId when value is empty string after trim", () => { + const result = parseDeliveryContextFromParams({ + deliveryContext: { channel: "discord", to: "123456789", accountId: " " }, + }); + expect(result?.accountId).toBeUndefined(); + }); + + it("returns undefined for threadId when value is empty string after trim", () => { + const result = parseDeliveryContextFromParams({ + deliveryContext: { channel: "discord", to: "123456789", threadId: " " }, + }); + expect(result?.threadId).toBeUndefined(); + }); + + // ── Extra/unknown fields are ignored ───────────────────────────────────── + + it("ignores unknown extra fields in deliveryContext", () => { + const result = parseDeliveryContextFromParams({ + deliveryContext: { channel: "discord", to: "123456789", unknownField: "ignored" }, + }); + expect(result).toEqual({ + channel: "discord", + to: "123456789", + accountId: undefined, + threadId: undefined, + }); + expect((result as Record)?.unknownField).toBeUndefined(); }); }); diff --git a/src/gateway/server-restart-sentinel.test.ts b/src/gateway/server-restart-sentinel.test.ts index 24a00296189..99397d44677 100644 --- a/src/gateway/server-restart-sentinel.test.ts +++ b/src/gateway/server-restart-sentinel.test.ts @@ -93,15 +93,19 @@ vi.mock("../infra/system-events.js", () => ({ const { scheduleRestartSentinelWake } = await import("./server-restart-sentinel.js"); -describe("scheduleRestartSentinelWake", () => { +// ───────────────────────────────────────────────────────────────────────────── +// Suite 1 — Core two-step delivery + resume flow +// ───────────────────────────────────────────────────────────────────────────── + +describe("scheduleRestartSentinelWake – two-step delivery + resume", () => { beforeEach(() => { vi.clearAllMocks(); }); - it("delivers restart notice directly then resumes agent after restart", async () => { + it("delivers restart notice directly (model-independent) then resumes agent", async () => { await scheduleRestartSentinelWake({ deps: {} as never }); - // Step 1: deterministic delivery (model-independent) + // Step 1: deterministic delivery expect(mocks.deliverOutboundPayloads).toHaveBeenCalledWith( expect.objectContaining({ channel: "whatsapp", @@ -112,7 +116,7 @@ describe("scheduleRestartSentinelWake", () => { }), ); - // Step 2: agent resume turn + // Step 2: agent resume expect(mocks.agentCommand).toHaveBeenCalledWith( expect.objectContaining({ message: "restart message", @@ -128,26 +132,56 @@ describe("scheduleRestartSentinelWake", () => { {}, ); - // Verify delivery happened before resume - const deliverOrder = mocks.deliverOutboundPayloads.mock.invocationCallOrder[0]; - const agentOrder = mocks.agentCommand.mock.invocationCallOrder[0]; - expect(deliverOrder).toBeLessThan(agentOrder); - expect(mocks.enqueueSystemEvent).not.toHaveBeenCalled(); }); - it("falls back to enqueueSystemEvent when agentCommand throws", async () => { - mocks.agentCommand.mockRejectedValueOnce(new Error("agent failed")); + it("delivers notification before triggering agent resume (ordering guarantee)", async () => { + await scheduleRestartSentinelWake({ deps: {} as never }); + + const deliverOrder = mocks.deliverOutboundPayloads.mock.invocationCallOrder[0]; + const agentOrder = mocks.agentCommand.mock.invocationCallOrder[0]; + expect(deliverOrder).toBeLessThan(agentOrder); + }); + + it("passes senderIsOwner=false to agentCommand (no privilege escalation)", async () => { + await scheduleRestartSentinelWake({ deps: {} as never }); + + const opts = getArg>(mocks.agentCommand, 0); + expect(opts.senderIsOwner).toBe(false); + }); + + it("no-ops when there is no sentinel file", async () => { + mocks.consumeRestartSentinel.mockResolvedValueOnce(null); await scheduleRestartSentinelWake({ deps: {} as never }); - expect(mocks.enqueueSystemEvent).toHaveBeenCalledWith( - expect.stringContaining("restart summary"), - { sessionKey: "agent:main:main" }, - ); + expect(mocks.deliverOutboundPayloads).not.toHaveBeenCalled(); + expect(mocks.agentCommand).not.toHaveBeenCalled(); + expect(mocks.enqueueSystemEvent).not.toHaveBeenCalled(); + }); +}); + +// ───────────────────────────────────────────────────────────────────────────── +// Suite 2 — Fallback paths +// ───────────────────────────────────────────────────────────────────────────── + +describe("scheduleRestartSentinelWake – fallback to enqueueSystemEvent", () => { + beforeEach(() => { + vi.clearAllMocks(); }); - it("falls back to enqueueSystemEvent when channel cannot be resolved (no channel in origin)", async () => { + it("falls back to enqueueSystemEvent on main session key when sentinel has no sessionKey", async () => { + mocks.consumeRestartSentinel.mockResolvedValueOnce({ payload: { sessionKey: "" } } as never); + + await scheduleRestartSentinelWake({ deps: {} as never }); + + expect(mocks.enqueueSystemEvent).toHaveBeenCalledWith("restart message", { + sessionKey: "agent:main:main", + }); + expect(mocks.agentCommand).not.toHaveBeenCalled(); + }); + + it("falls back to enqueueSystemEvent when outbound target cannot be resolved", async () => { mocks.resolveOutboundTarget.mockReturnValueOnce({ ok: false, error: new Error("no-target"), @@ -161,13 +195,187 @@ describe("scheduleRestartSentinelWake", () => { }); }); - it("falls back to enqueueSystemEvent on main session key when sentinel has no sessionKey", async () => { - mocks.consumeRestartSentinel.mockResolvedValueOnce({ payload: { sessionKey: "" } } as never); + it("falls back to enqueueSystemEvent when channel is missing from merged delivery context", async () => { + // mergeDeliveryContext is called twice (inner + outer merge); mock the outer to drop channel + mocks.mergeDeliveryContext + .mockReturnValueOnce(undefined) // inner: sessionDeliveryContext merge + .mockReturnValueOnce({ to: "+15550002" }); // outer: sentinelContext wins, no channel await scheduleRestartSentinelWake({ deps: {} as never }); + expect(mocks.agentCommand).not.toHaveBeenCalled(); expect(mocks.enqueueSystemEvent).toHaveBeenCalledWith("restart message", { sessionKey: "agent:main:main", }); }); + + it("falls back to enqueueSystemEvent when to is missing from merged delivery context", async () => { + // Mock outer merge to return a context with no `to` + mocks.mergeDeliveryContext + .mockReturnValueOnce(undefined) + .mockReturnValueOnce({ channel: "whatsapp" }); + + await scheduleRestartSentinelWake({ deps: {} as never }); + + expect(mocks.agentCommand).not.toHaveBeenCalled(); + expect(mocks.enqueueSystemEvent).toHaveBeenCalledWith("restart message", { + sessionKey: "agent:main:main", + }); + }); + + it("falls back to enqueueSystemEvent (with error) when agentCommand throws", async () => { + mocks.agentCommand.mockRejectedValueOnce(new Error("agent failed")); + + await scheduleRestartSentinelWake({ deps: {} as never }); + + // Direct delivery should still have been attempted + expect(mocks.deliverOutboundPayloads).toHaveBeenCalled(); + // Then fallback for the resume + expect(mocks.enqueueSystemEvent).toHaveBeenCalledWith( + expect.stringContaining("restart summary"), + { sessionKey: "agent:main:main" }, + ); + }); + + it("still calls agentCommand even if deliverOutboundPayloads throws (bestEffort guard)", async () => { + // bestEffort: true means it shouldn't normally throw, but even if it does, resume continues + mocks.deliverOutboundPayloads.mockRejectedValueOnce(new Error("deliver failed")); + + await scheduleRestartSentinelWake({ deps: {} as never }); + + expect(mocks.agentCommand).toHaveBeenCalled(); + }); }); + +// ───────────────────────────────────────────────────────────────────────────── +// Suite 3 — Thread routing (Slack vs non-Slack) +// ───────────────────────────────────────────────────────────────────────────── + +describe("scheduleRestartSentinelWake – thread routing", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it("maps threadId to replyToId and clears threadId for Slack deliverOutboundPayloads", async () => { + mocks.consumeRestartSentinel.mockResolvedValueOnce({ + payload: { + sessionKey: "agent:main:main", + deliveryContext: { channel: "slack", to: "C012AB3CD", accountId: undefined }, + threadId: "1234567890.123456", + }, + }); + mocks.normalizeChannelId.mockReturnValueOnce("slack"); + + await scheduleRestartSentinelWake({ deps: {} as never }); + + const deliverArgs = getArg>(mocks.deliverOutboundPayloads, 0); + expect(deliverArgs.replyToId).toBe("1234567890.123456"); + expect(deliverArgs.threadId).toBeUndefined(); + }); + + it("passes threadId directly (not replyToId) for non-Slack channels", async () => { + mocks.consumeRestartSentinel.mockResolvedValueOnce({ + payload: { + sessionKey: "agent:main:main", + deliveryContext: { channel: "discord", to: "123456789", accountId: undefined }, + threadId: "discord-thread-id", + }, + }); + + await scheduleRestartSentinelWake({ deps: {} as never }); + + const deliverArgs = getArg>(mocks.deliverOutboundPayloads, 0); + expect(deliverArgs.threadId).toBe("discord-thread-id"); + expect(deliverArgs.replyToId).toBeUndefined(); + }); + + it("passes threadId to agentCommand for non-Slack threading", async () => { + mocks.consumeRestartSentinel.mockResolvedValueOnce({ + payload: { + sessionKey: "agent:main:main", + deliveryContext: { channel: "discord", to: "123456789", accountId: undefined }, + threadId: "discord-thread-id", + }, + }); + + await scheduleRestartSentinelWake({ deps: {} as never }); + + const agentOpts = getArg>(mocks.agentCommand, 0); + expect(agentOpts.threadId).toBe("discord-thread-id"); + }); + + it("sentinel payload threadId takes precedence over session-derived threadId", async () => { + mocks.consumeRestartSentinel.mockResolvedValueOnce({ + payload: { + sessionKey: "agent:main:main", + deliveryContext: { channel: "whatsapp", to: "+15550002" }, + threadId: "sentinel-thread", + }, + }); + // parseSessionThreadInfo would derive a different threadId from the session key + mocks.parseSessionThreadInfo.mockReturnValueOnce({ + baseSessionKey: null, + threadId: "session-thread", + }); + + await scheduleRestartSentinelWake({ deps: {} as never }); + + const agentOpts = getArg>(mocks.agentCommand, 0); + expect(agentOpts.threadId).toBe("sentinel-thread"); + }); +}); + +// ───────────────────────────────────────────────────────────────────────────── +// Suite 4 — Delivery context priority: sentinel > session store > parsed target +// ───────────────────────────────────────────────────────────────────────────── + +describe("scheduleRestartSentinelWake – delivery context priority", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it("prefers sentinel deliveryContext over session store (handles heartbeat-overwritten store)", async () => { + // Session store has been overwritten with heartbeat sink + mocks.deliveryContextFromSession.mockReturnValueOnce({ + channel: "webchat", + to: "heartbeat", + }); + + await scheduleRestartSentinelWake({ deps: {} as never }); + + // agentCommand should use the sentinel's whatsapp/+15550002, not webchat/heartbeat + const agentOpts = getArg>(mocks.agentCommand, 0); + expect(agentOpts.channel).toBe("whatsapp"); + expect(agentOpts.to).toBe("+15550002"); + }); + + it("falls back to session store when sentinel has no deliveryContext", async () => { + mocks.consumeRestartSentinel.mockResolvedValueOnce({ + payload: { sessionKey: "agent:main:main" }, // no deliveryContext + }); + mocks.deliveryContextFromSession.mockReturnValueOnce({ + channel: "telegram", + to: "+19990001", + }); + // Mock both merge calls: inner produces session ctx; outer passes it through + mocks.mergeDeliveryContext + .mockReturnValueOnce({ channel: "telegram", to: "+19990001" }) // inner + .mockReturnValueOnce({ channel: "telegram", to: "+19990001" }); // outer + // resolveOutboundTarget must reflect the session-store to value + mocks.resolveOutboundTarget.mockReturnValueOnce({ ok: true as const, to: "+19990001" }); + + await scheduleRestartSentinelWake({ deps: {} as never }); + + const agentOpts = getArg>(mocks.agentCommand, 0); + expect(agentOpts.channel).toBe("telegram"); + expect(agentOpts.to).toBe("+19990001"); + }); +}); + +// ───────────────────────────────────────────────────────────────────────────── +// Helpers +// ───────────────────────────────────────────────────────────────────────────── + +function getArg(mockFn: { mock: { calls: unknown[][] } }, argIdx: number): T { + return mockFn.mock.calls[0]?.[argIdx] as T; +}