diff --git a/apps/macos/Sources/OpenClawProtocol/GatewayModels.swift b/apps/macos/Sources/OpenClawProtocol/GatewayModels.swift index 0b1d7b13e01..e01267c889f 100644 --- a/apps/macos/Sources/OpenClawProtocol/GatewayModels.swift +++ b/apps/macos/Sources/OpenClawProtocol/GatewayModels.swift @@ -1656,19 +1656,22 @@ public struct ConfigApplyParams: Codable, Sendable { public let sessionkey: String? public let note: String? public let restartdelayms: Int? + public let deliverycontext: [String: AnyCodable]? public init( raw: String, basehash: String?, sessionkey: String?, note: String?, - restartdelayms: Int?) + restartdelayms: Int?, + deliverycontext: [String: AnyCodable]?) { self.raw = raw self.basehash = basehash self.sessionkey = sessionkey self.note = note self.restartdelayms = restartdelayms + self.deliverycontext = deliverycontext } private enum CodingKeys: String, CodingKey { @@ -1677,6 +1680,7 @@ public struct ConfigApplyParams: Codable, Sendable { case sessionkey = "sessionKey" case note case restartdelayms = "restartDelayMs" + case deliverycontext = "deliveryContext" } } @@ -1686,19 +1690,22 @@ public struct ConfigPatchParams: Codable, Sendable { public let sessionkey: String? public let note: String? public let restartdelayms: Int? + public let deliverycontext: [String: AnyCodable]? public init( raw: String, basehash: String?, sessionkey: String?, note: String?, - restartdelayms: Int?) + restartdelayms: Int?, + deliverycontext: [String: AnyCodable]?) { self.raw = raw self.basehash = basehash self.sessionkey = sessionkey self.note = note self.restartdelayms = restartdelayms + self.deliverycontext = deliverycontext } private enum CodingKeys: String, CodingKey { @@ -1707,6 +1714,7 @@ public struct ConfigPatchParams: Codable, Sendable { case sessionkey = "sessionKey" case note case restartdelayms = "restartDelayMs" + case deliverycontext = "deliveryContext" } } @@ -3709,17 +3717,20 @@ public struct UpdateRunParams: Codable, Sendable { public let note: String? public let restartdelayms: Int? public let timeoutms: Int? + public let deliverycontext: [String: AnyCodable]? public init( sessionkey: String?, note: String?, restartdelayms: Int?, - timeoutms: Int?) + timeoutms: Int?, + deliverycontext: [String: AnyCodable]?) { self.sessionkey = sessionkey self.note = note self.restartdelayms = restartdelayms self.timeoutms = timeoutms + self.deliverycontext = deliverycontext } private enum CodingKeys: String, CodingKey { @@ -3727,6 +3738,7 @@ public struct UpdateRunParams: Codable, Sendable { case note case restartdelayms = "restartDelayMs" case timeoutms = "timeoutMs" + case deliverycontext = "deliveryContext" } } diff --git a/apps/shared/OpenClawKit/Sources/OpenClawProtocol/GatewayModels.swift b/apps/shared/OpenClawKit/Sources/OpenClawProtocol/GatewayModels.swift index 0b1d7b13e01..e01267c889f 100644 --- a/apps/shared/OpenClawKit/Sources/OpenClawProtocol/GatewayModels.swift +++ b/apps/shared/OpenClawKit/Sources/OpenClawProtocol/GatewayModels.swift @@ -1656,19 +1656,22 @@ public struct ConfigApplyParams: Codable, Sendable { public let sessionkey: String? public let note: String? public let restartdelayms: Int? + public let deliverycontext: [String: AnyCodable]? public init( raw: String, basehash: String?, sessionkey: String?, note: String?, - restartdelayms: Int?) + restartdelayms: Int?, + deliverycontext: [String: AnyCodable]?) { self.raw = raw self.basehash = basehash self.sessionkey = sessionkey self.note = note self.restartdelayms = restartdelayms + self.deliverycontext = deliverycontext } private enum CodingKeys: String, CodingKey { @@ -1677,6 +1680,7 @@ public struct ConfigApplyParams: Codable, Sendable { case sessionkey = "sessionKey" case note case restartdelayms = "restartDelayMs" + case deliverycontext = "deliveryContext" } } @@ -1686,19 +1690,22 @@ public struct ConfigPatchParams: Codable, Sendable { public let sessionkey: String? public let note: String? public let restartdelayms: Int? + public let deliverycontext: [String: AnyCodable]? public init( raw: String, basehash: String?, sessionkey: String?, note: String?, - restartdelayms: Int?) + restartdelayms: Int?, + deliverycontext: [String: AnyCodable]?) { self.raw = raw self.basehash = basehash self.sessionkey = sessionkey self.note = note self.restartdelayms = restartdelayms + self.deliverycontext = deliverycontext } private enum CodingKeys: String, CodingKey { @@ -1707,6 +1714,7 @@ public struct ConfigPatchParams: Codable, Sendable { case sessionkey = "sessionKey" case note case restartdelayms = "restartDelayMs" + case deliverycontext = "deliveryContext" } } @@ -3709,17 +3717,20 @@ public struct UpdateRunParams: Codable, Sendable { public let note: String? public let restartdelayms: Int? public let timeoutms: Int? + public let deliverycontext: [String: AnyCodable]? public init( sessionkey: String?, note: String?, restartdelayms: Int?, - timeoutms: Int?) + timeoutms: Int?, + deliverycontext: [String: AnyCodable]?) { self.sessionkey = sessionkey self.note = note self.restartdelayms = restartdelayms self.timeoutms = timeoutms + self.deliverycontext = deliverycontext } private enum CodingKeys: String, CodingKey { @@ -3727,6 +3738,7 @@ public struct UpdateRunParams: Codable, Sendable { case note case restartdelayms = "restartDelayMs" case timeoutms = "timeoutMs" + case deliverycontext = "deliveryContext" } } diff --git a/docs/reference/templates/AGENTS.md b/docs/reference/templates/AGENTS.md index 9375684b0dd..216a52cdbbe 100644 --- a/docs/reference/templates/AGENTS.md +++ b/docs/reference/templates/AGENTS.md @@ -214,6 +214,37 @@ Think of it like a human reviewing their journal and updating their mental model The goal: Be helpful without being annoying. Check in a few times a day, do useful background work, but respect quiet time. +## ๐Ÿ”„ Gateway Restarts โ€” Do It Right! + +**Never use `openclaw gateway restart` (CLI/shell).** It bypasses the restart sentinel, so you won't auto-resume or notify the user after restart. You'll just sit there silently until someone pings you. + +**Always restart via the gateway tool** (action=restart) or via `config.patch`/`config.apply` โ€” these write a sentinel file before restarting, which the new process consumes to wake you up and message the user automatically. + +```bash +# โœ… Correct: restart via gateway tool (action=restart, sessionKey, note) +# โœ… Correct: config.patch with a key that requires restart (writes sentinel automatically) + +# โŒ Wrong: openclaw gateway restart โ€” no sentinel, silent after restart +# โŒ Wrong: systemctl --user restart openclaw-gateway.service โ€” same problem +``` + +### Which config keys trigger a real restart vs dynamic reload? + +**Full process restart** (sentinel written, agent wakes up): + +- `gateway.*`, `discovery.*`, `plugins.*`, `canvasHost.*` +- Any unrecognized/new config key + +**Hot reload** (no restart, no sentinel needed): + +- `hooks.*`, `cron.*`, `browser.*`, `models.*`, `agents.defaults.heartbeat` + +**Dynamic no-op** (read on next access, no process action): + +- `messages.*`, `agents.*`, `tools.*`, `routing.*`, `session.*`, `skills.*`, `secrets.*`, `meta.*`, `identity.*`, `logging.*`, `ui.*` + +**Rule of thumb:** If you want a test restart, patch `discovery.mdns.mode` to its current value โ€” it's recognized as a restart-triggering key even if the value is unchanged. + ## Make It Yours This is a starting point. Add your own conventions, style, and rules as you figure out what works. diff --git a/docs/zh-CN/reference/AGENTS.default.md b/docs/zh-CN/reference/AGENTS.default.md index 84d4a01e21c..70e6688a374 100644 --- a/docs/zh-CN/reference/AGENTS.default.md +++ b/docs/zh-CN/reference/AGENTS.default.md @@ -120,6 +120,29 @@ git commit -m "Add Clawd workspace" - **bird** โ€” X/Twitter CLI๏ผŒๆ— ้œ€ๆต่งˆๅ™จๅณๅฏๅ‘ๆŽจใ€ๅ›žๅคใ€้˜…่ฏป่ฏ้ข˜ๅ’Œๆœ็ดขใ€‚ - **agent-tools** โ€” ็”จไบŽ่‡ชๅŠจๅŒ–ๅ’Œ่พ…ๅŠฉ่„šๆœฌ็š„ๅฎž็”จๅทฅๅ…ทๅŒ…ใ€‚ +## ๐Ÿ”„ ็ฝ‘ๅ…ณ้‡ๅฏ โ€” ๆญฃ็กฎๅšๆณ•๏ผ + +**ๆฐธ่ฟœไธ่ฆไฝฟ็”จ `openclaw gateway restart`๏ผˆCLI/shell๏ผ‰ใ€‚** ่ฟ™ไผš็ป•่ฟ‡้‡ๅฏๅ“จๅ…ตๆœบๅˆถ๏ผŒๅฏผ่‡ด้‡ๅฏๅŽไฝ ๆ— ๆณ•่‡ชๅŠจๆขๅค๏ผŒไนŸๆ— ๆณ•้€š็Ÿฅ็”จๆˆทใ€‚ไฝ ไผš้™้™ๅœฐ็ญ‰ๅพ…๏ผŒ็›ดๅˆฐๆœ‰ไบบ ping ไฝ ใ€‚ + +**ๅง‹็ปˆ้€š่ฟ‡ gateway ๅทฅๅ…ท**๏ผˆaction=restart๏ผ‰ๆˆ– `config.patch`/`config.apply` ่งฆๅ‘้‡ๅฏโ€”โ€”่ฟ™ไบ›ๆ–นๅผไผšๅœจ้‡ๅฏๅ‰ๅ†™ๅ…ฅๅ“จๅ…ตๆ–‡ไปถ๏ผŒๆ–ฐ่ฟ›็จ‹ๅฏๅŠจๅŽไผšๆถˆ่ดน่ฏฅๆ–‡ไปถไปฅๅ”ค้†’ไฝ ๅนถ่‡ชๅŠจ้€š็Ÿฅ็”จๆˆทใ€‚ + +### ๅ“ชไบ›้…็ฝฎ้”ฎไผš่งฆๅ‘็œŸๆญฃ็š„้‡ๅฏ๏ผŸ + +**ๅฎŒๆ•ด่ฟ›็จ‹้‡ๅฏ**๏ผˆๅ†™ๅ…ฅๅ“จๅ…ต๏ผŒไปฃ็†ๅ”ค้†’๏ผ‰๏ผš + +- `gateway.*`ใ€`discovery.*`ใ€`plugins.*`ใ€`canvasHost.*` +- ไปปไฝ•ๆ— ๆณ•่ฏ†ๅˆซ็š„ๆ–ฐ้…็ฝฎ้”ฎ + +**็ƒญ้‡่ฝฝ**๏ผˆๆ— ้œ€้‡ๅฏ๏ผŒๆ— ้œ€ๅ“จๅ…ต๏ผ‰๏ผš + +- `hooks.*`ใ€`cron.*`ใ€`browser.*`ใ€`models.*`ใ€`agents.defaults.heartbeat` + +**ๅŠจๆ€ๆ— ๆ“ไฝœ**๏ผˆไธ‹ๆฌก่ฎฟ้—ฎๆ—ถ่ฏปๅ–๏ผŒไธ่งฆๅ‘ไปปไฝ•่ฟ›็จ‹ๆ“ไฝœ๏ผ‰๏ผš + +- `messages.*`ใ€`agents.*`ใ€`tools.*`ใ€`routing.*`ใ€`session.*`ใ€`skills.*`ใ€`secrets.*`ใ€`meta.*` + +**็ป้ชŒๆณ•ๅˆ™๏ผš** ๅฆ‚ๆžœ้œ€่ฆๆต‹่ฏ•้‡ๅฏ๏ผŒๅฐ† `discovery.mdns.mode` ไฟฎๆ”นไธบๅฝ“ๅ‰ๅ€ผโ€”โ€”ๅณไฝฟๅ€ผๆœชๆ”นๅ˜๏ผŒๅฎƒไนŸไผš่งฆๅ‘้‡ๅฏๆต็จ‹ใ€‚ + ## ไฝฟ็”จ่ฏดๆ˜Ž - ่„šๆœฌ็ผ–ๅ†™ไผ˜ๅ…ˆไฝฟ็”จ `openclaw` CLI๏ผ›mac ๅบ”็”จๅค„็†ๆƒ้™ใ€‚ diff --git a/src/agents/openclaw-tools.ts b/src/agents/openclaw-tools.ts index de5e91fdf0c..631d59d8a2a 100644 --- a/src/agents/openclaw-tools.ts +++ b/src/agents/openclaw-tools.ts @@ -175,6 +175,10 @@ export function createOpenClawTools( ...(imageGenerateTool ? [imageGenerateTool] : []), createGatewayTool({ agentSessionKey: options?.agentSessionKey, + agentChannel: options?.agentChannel != null ? String(options.agentChannel) : undefined, + agentTo: options?.agentTo, + agentThreadId: options?.agentThreadId, + agentAccountId: options?.agentAccountId, config: options?.config, }), createAgentsListTool({ diff --git a/src/agents/tools/gateway-tool.test.ts b/src/agents/tools/gateway-tool.test.ts new file mode 100644 index 00000000000..7ecc8f6ca4f --- /dev/null +++ b/src/agents/tools/gateway-tool.test.ts @@ -0,0 +1,453 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; + +const mocks = vi.hoisted(() => ({ + isRestartEnabled: vi.fn(() => true), + resolveConfigSnapshotHash: vi.fn(() => undefined), + extractDeliveryInfo: vi.fn(() => ({ + deliveryContext: { + channel: "telegram", + to: "+19995550001", + accountId: undefined as string | undefined, + }, + threadId: undefined as string | undefined, + })), + writeRestartSentinel: vi.fn(async () => undefined), + scheduleGatewaySigusr1Restart: vi.fn(() => ({ ok: true })), + formatDoctorNonInteractiveHint: vi.fn(() => ""), + callGatewayTool: vi.fn(async () => ({})), + readGatewayCallOptions: vi.fn(() => ({})), + resolveGatewayTarget: vi.fn((): "local" | "remote" | undefined => undefined), +})); + +vi.mock("../../config/commands.js", () => ({ isRestartEnabled: mocks.isRestartEnabled })); +vi.mock("../../config/io.js", () => ({ + resolveConfigSnapshotHash: mocks.resolveConfigSnapshotHash, +})); +vi.mock("../../config/sessions.js", () => ({ + extractDeliveryInfo: mocks.extractDeliveryInfo, +})); +vi.mock("../../infra/restart-sentinel.js", () => ({ + writeRestartSentinel: mocks.writeRestartSentinel, + formatDoctorNonInteractiveHint: mocks.formatDoctorNonInteractiveHint, +})); +vi.mock("../../infra/restart.js", () => ({ + scheduleGatewaySigusr1Restart: mocks.scheduleGatewaySigusr1Restart, +})); +vi.mock("./gateway.js", () => ({ + callGatewayTool: mocks.callGatewayTool, + readGatewayCallOptions: mocks.readGatewayCallOptions, + resolveGatewayTarget: mocks.resolveGatewayTarget, +})); + +import { createGatewayTool } from "./gateway-tool.js"; + +async function execTool( + tool: ReturnType, + params: Record, +) { + return (tool as unknown as { execute: (id: string, args: unknown) => Promise }).execute( + "test-id", + params, + ); +} + +function getCallArg(mockFn: { mock: { calls: unknown[] } }, callIdx: number, argIdx: number): T { + const calls = mockFn.mock.calls as unknown[][]; + return calls[callIdx]?.[argIdx] as T; +} + +// โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€ +// Helpers to build common test fixtures +// โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€ + +function makePatchParams(overrides: Record = {}) { + return { + action: "config.patch", + raw: '{"key":"value"}', + baseHash: "abc123", + sessionKey: "agent:main:main", + note: "test patch", + ...overrides, + }; +} + +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(); + }); + + // โ”€โ”€ Happy path: full live context forwarded โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€ + + it("forwards liveDeliveryContext when agentChannel and agentTo are both present", async () => { + await execTool(makeTool(), makePatchParams()); + + const p = getCallArg>(mocks.callGatewayTool, 0, 2); + expect(p?.deliveryContext).toEqual({ + channel: "discord", + to: "123456789", + accountId: undefined, + threadId: undefined, + }); + }); + + it("includes agentAccountId in forwarded context when provided", async () => { + await execTool(makeTool({ agentAccountId: "acct-99" }), makePatchParams()); + + 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", + }); + + const p = getCallArg>(mocks.callGatewayTool, 0, 2); + expect(p?.deliveryContext).toBeDefined(); + expect((p?.deliveryContext as Record)?.channel).toBe("discord"); + }); + + 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: "webchat", to: "heartbeat", accountId: undefined }, + threadId: undefined, + }); + + await execTool(makeTool(), makePatchParams()); + + const p = getCallArg>(mocks.callGatewayTool, 0, 2); + expect((p?.deliveryContext as Record)?.channel).toBe("discord"); + expect((p?.deliveryContext as Record)?.to).toBe("123456789"); + }); + + // โ”€โ”€ Session key targeting: same-session โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€ + + it("forwards live context when sessionKey matches own session key exactly", async () => { + await execTool( + makeTool({ agentSessionKey: "agent:main:main" }), + makePatchParams({ sessionKey: "agent:main:main" }), + ); + + const p = getCallArg>(mocks.callGatewayTool, 0, 2); + expect(p?.deliveryContext).toBeDefined(); + }); + + 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"); + + await execTool(makeTool(), makePatchParams({ sessionKey: "agent:main:main" })); + + const p = getCallArg>(mocks.callGatewayTool, 0, 2); + expect(p?.deliveryContext).toBeUndefined(); + }); + + it("forwards deliveryContext when resolveGatewayTarget returns 'local' (loopback URL)", async () => { + mocks.readGatewayCallOptions.mockReturnValueOnce({ gatewayUrl: "ws://127.0.0.1:18789" }); + mocks.resolveGatewayTarget.mockReturnValueOnce("local"); + + await execTool( + makeTool(), + makePatchParams({ gatewayUrl: "ws://127.0.0.1:18789", sessionKey: "agent:main:main" }), + ); + + const p = getCallArg>(mocks.callGatewayTool, 0, 2); + expect(p?.deliveryContext).toBeDefined(); + expect((p?.deliveryContext as Record)?.channel).toBe("discord"); + }); + + it("forwards deliveryContext when resolveGatewayTarget returns undefined (default local)", async () => { + mocks.resolveGatewayTarget.mockReturnValueOnce(undefined); + + await execTool(makeTool(), makePatchParams({ sessionKey: "agent:main:main" })); + + 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("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, + }); + + await execTool(makeTool({ agentTo: undefined }), { action: "restart" }); + + const p = getCallArg<{ deliveryContext?: Record }>( + mocks.writeRestartSentinel, + 0, + 0, + ); + 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/agents/tools/gateway-tool.ts b/src/agents/tools/gateway-tool.ts index 33b8d86adcf..e49f73c57b5 100644 --- a/src/agents/tools/gateway-tool.ts +++ b/src/agents/tools/gateway-tool.ts @@ -12,7 +12,7 @@ import { scheduleGatewaySigusr1Restart } from "../../infra/restart.js"; import { createSubsystemLogger } from "../../logging/subsystem.js"; import { stringEnum } from "../schema/typebox.js"; import { type AnyAgentTool, jsonResult, readStringParam } from "./common.js"; -import { callGatewayTool, readGatewayCallOptions } from "./gateway.js"; +import { callGatewayTool, readGatewayCallOptions, resolveGatewayTarget } from "./gateway.js"; const log = createSubsystemLogger("gateway-tool"); @@ -69,6 +69,10 @@ const GatewayToolSchema = Type.Object({ export function createGatewayTool(opts?: { agentSessionKey?: string; + agentChannel?: string; + agentTo?: string; + agentThreadId?: string | number; + agentAccountId?: string; config?: OpenClawConfig; }): AnyAgentTool { return { @@ -76,7 +80,7 @@ export function createGatewayTool(opts?: { name: "gateway", ownerOnly: true, description: - "Restart, inspect a specific config schema path, apply config, or update the gateway in-place (SIGUSR1). Use config.schema.lookup with a targeted dot path before config edits. Use config.patch for safe partial config updates (merges with existing). Use config.apply only when replacing entire config. Both trigger restart after writing. Always pass a human-readable completion message via the `note` parameter so the system can deliver it to the user after restart.", + "Restart, inspect a specific config schema path, apply config, or update the gateway in-place (SIGUSR1). Use config.schema.lookup with a targeted dot path before config edits. Use config.patch for safe partial config updates (merges with existing). Use config.apply only when replacing entire config. Both trigger restart after writing. Always pass a human-readable completion message via the `note` parameter so the system can deliver it to the user after restart. IMPORTANT: Never use the `openclaw gateway restart` CLI command to restart โ€” it bypasses the restart sentinel so the agent will not auto-resume or notify the user after restart. Always restart via this tool (action=restart) or via config.patch/config.apply, which write the sentinel before restarting. Config keys under gateway.*, discovery.*, plugins.*, and canvasHost.* trigger a real process restart; keys under messages.*, agents.*, tools.*, hooks.*, and most others apply dynamically without a restart.", parameters: GatewayToolSchema, execute: async (_toolCallId, args) => { const params = args as Record; @@ -85,10 +89,11 @@ export function createGatewayTool(opts?: { if (!isRestartEnabled(opts?.config)) { throw new Error("Gateway restart is disabled (commands.restart=false)."); } - const sessionKey = + const explicitSessionKey = typeof params.sessionKey === "string" && params.sessionKey.trim() ? params.sessionKey.trim() - : opts?.agentSessionKey?.trim() || undefined; + : undefined; + const sessionKey = (explicitSessionKey ?? opts?.agentSessionKey?.trim()) || undefined; const delayMs = typeof params.delayMs === "number" && Number.isFinite(params.delayMs) ? Math.floor(params.delayMs) @@ -99,9 +104,74 @@ export function createGatewayTool(opts?: { : undefined; const note = typeof params.note === "string" && params.note.trim() ? params.note.trim() : undefined; - // Extract channel + threadId for routing after restart - // Supports both :thread: (most channels) and :topic: (Telegram) - const { deliveryContext, threadId } = extractDeliveryInfo(sessionKey); + // Prefer the live delivery context captured during the current agent + // run over extractDeliveryInfo() (which reads the persisted session + // store). The session store is frequently overwritten by heartbeat + // runs to { channel: "webchat", to: "heartbeat" }, causing the + // sentinel to write stale routing data that fails post-restart. + // See #18612. + // + // Only apply the live context when the restart targets this agent's + // own session. When an explicit sessionKey points to a different + // session, the live context belongs to the wrong session and would + // misroute the post-restart reply. Fall back to extractDeliveryInfo() + // so the server uses the correct routing for the target session. + // Canonicalize both keys before comparing so that aliases like "main" + // and "agent:main:main" are treated as the same session. Without this, + // an operator passing sessionKey="main" would be incorrectly treated as + // targeting a different session, suppressing live deliveryContext and + // falling back to the stale session store. See #18612. + const ownKey = opts?.agentSessionKey?.trim() || undefined; + const agentId = resolveAgentIdFromSessionKey(ownKey); + // Canonicalize each key using its OWN agentId โ€” not the current session's. + // If a non-default agent passes sessionKey="main", resolveAgentIdFromSessionKey + // returns DEFAULT_AGENT_ID ("main") so "main" โ†’ "agent:main:main". Using the + // current session's agentId instead would map "main" to the current agent's main + // session, falsely treating a cross-agent request as same-session. See #18612. + const canonicalizeOwn = (k: string) => + canonicalizeMainSessionAlias({ cfg: opts?.config, agentId, sessionKey: k }); + const canonicalizeTarget = (k: string) => + canonicalizeMainSessionAlias({ + cfg: opts?.config, + agentId: resolveAgentIdFromSessionKey(k), + sessionKey: k, + }); + const isTargetingOtherSession = + explicitSessionKey != null && + canonicalizeTarget(explicitSessionKey) !== (ownKey ? canonicalizeOwn(ownKey) : undefined); + // Only forward live context when both channel and to are present. + // Forwarding a partial context (channel without to) causes the server + // to write a sentinel without `to`, and scheduleRestartSentinelWake + // bails on `if (!channel || !to)`, silently degrading to a system + // event with no delivery/resume. See #18612. + const liveContext = + !isTargetingOtherSession && + opts?.agentChannel != null && + String(opts.agentChannel).trim() && + opts?.agentTo != null && + String(opts.agentTo).trim() + ? { + channel: String(opts.agentChannel).trim(), + to: String(opts.agentTo).trim(), + accountId: opts?.agentAccountId ?? undefined, + } + : undefined; + const extracted = extractDeliveryInfo(sessionKey); + const deliveryContext = + liveContext != null + ? { + ...liveContext, + accountId: liveContext.accountId ?? extracted.deliveryContext?.accountId, + } + : extracted.deliveryContext; + // Guard threadId with the same session check as deliveryContext. When + // targeting another session, opts.agentThreadId belongs to the current + // session's thread and must not be written into the sentinel โ€” it would + // cause scheduleRestartSentinelWake to deliver to the wrong thread. + const threadId = + !isTargetingOtherSession && opts?.agentThreadId != null + ? String(opts.agentThreadId) + : extracted.threadId; const payload: RestartSentinelPayload = { kind: "restart", status: "ok", @@ -133,22 +203,92 @@ export function createGatewayTool(opts?: { const gatewayOpts = readGatewayCallOptions(params); + // Build the live delivery context from the current agent run's routing + // fields. This is passed to server-side handlers so they can write an + // accurate sentinel without reading the (potentially stale) session + // store. The store is frequently overwritten by heartbeat runs to + // { channel: "webchat", to: "heartbeat" }. See #18612. + // + // Note: agentThreadId is intentionally excluded here. threadId is + // reliably derived server-side from the session key (via + // parseSessionThreadInfo), which encodes it as :thread:N or :topic:N. + // That parsing is not subject to heartbeat contamination, so there is + // no need to forward it through the RPC params. + // Only forward live context when both channel and to are present. + // Forwarding a partial context (channel without to) causes the server + // to prefer an incomplete deliveryContext over extractDeliveryInfo(), + // writing a sentinel without `to` that scheduleRestartSentinelWake + // rejects, silently degrading to a system event. See #18612. + // + // threadId is included so the server can use it for sessions where the + // session key is not :thread:-scoped (e.g. Slack replyToMode="all"), in + // which case the session-key-derived threadId would be empty. + const liveDeliveryContextForRpc = + opts?.agentChannel != null && + String(opts.agentChannel).trim() && + opts?.agentTo != null && + String(opts.agentTo).trim() + ? { + channel: String(opts.agentChannel).trim(), + to: String(opts.agentTo).trim(), + accountId: opts?.agentAccountId ?? undefined, + threadId: opts?.agentThreadId != null ? String(opts.agentThreadId) : undefined, + } + : undefined; + const resolveGatewayWriteMeta = (): { sessionKey: string | undefined; note: string | undefined; restartDelayMs: number | undefined; + deliveryContext: typeof liveDeliveryContextForRpc; } => { - const sessionKey = + const explicitSessionKey = typeof params.sessionKey === "string" && params.sessionKey.trim() ? params.sessionKey.trim() - : opts?.agentSessionKey?.trim() || undefined; + : undefined; + const sessionKey = (explicitSessionKey ?? opts?.agentSessionKey?.trim()) || undefined; const note = typeof params.note === "string" && params.note.trim() ? params.note.trim() : undefined; const restartDelayMs = typeof params.restartDelayMs === "number" && Number.isFinite(params.restartDelayMs) ? Math.floor(params.restartDelayMs) : undefined; - return { sessionKey, note, restartDelayMs }; + // Only forward live context when the target session is this agent's + // own session. Canonicalize both keys before comparing so that aliases + // like "main" and "agent:main:main" are treated as the same session. + // When an explicit sessionKey points to a different session, omit + // deliveryContext so the server falls back to extractDeliveryInfo(sessionKey). + const rpcOwnKey = opts?.agentSessionKey?.trim() || undefined; + const rpcAgentId = resolveAgentIdFromSessionKey(rpcOwnKey); + // Same cross-agent alias fix as the restart path: derive agentId from each key + // independently so that "main" resolves to the default agent, not the current one. + const rpcCanonicalizeOwn = (k: string) => + canonicalizeMainSessionAlias({ cfg: opts?.config, agentId: rpcAgentId, sessionKey: k }); + const rpcCanonicalizeTarget = (k: string) => + canonicalizeMainSessionAlias({ + cfg: opts?.config, + agentId: resolveAgentIdFromSessionKey(k), + sessionKey: k, + }); + const isTargetingOtherSession = + explicitSessionKey != null && + rpcCanonicalizeTarget(explicitSessionKey) !== + (rpcOwnKey ? rpcCanonicalizeOwn(rpcOwnKey) : undefined); + // Also omit when the call targets a remote gateway. The remote server's + // extractDeliveryInfo(sessionKey) is the authoritative source for that + // session's delivery route. Forwarding the local agent run's deliveryContext + // would write a sentinel with the wrong chat destination on the remote host, + // causing post-restart wake messages to be sent to the caller's chat instead + // of the session on the remote gateway. See #18612. + // Only suppress deliveryContext for truly remote gateways. A gatewayUrl + // override pointing to a local loopback address (127.0.0.1, localhost, + // [::1]) is still the local server and should forward context normally; + // treating it as remote would fall back to extractDeliveryInfo(sessionKey) + // and reintroduce the stale heartbeat routing this patch was meant to fix. + const isRemoteGateway = resolveGatewayTarget(gatewayOpts) === "remote"; + const deliveryContext = + isTargetingOtherSession || isRemoteGateway ? undefined : liveDeliveryContextForRpc; + return { sessionKey, note, restartDelayMs, deliveryContext }; }; const resolveConfigWriteParams = async (): Promise<{ @@ -157,6 +297,7 @@ export function createGatewayTool(opts?: { sessionKey: string | undefined; note: string | undefined; restartDelayMs: number | undefined; + deliveryContext: typeof liveDeliveryContextForRpc; }> => { const raw = readStringParam(params, "raw", { required: true }); let baseHash = readStringParam(params, "baseHash"); @@ -183,7 +324,7 @@ export function createGatewayTool(opts?: { return jsonResult({ ok: true, result }); } if (action === "config.apply") { - const { raw, baseHash, sessionKey, note, restartDelayMs } = + const { raw, baseHash, sessionKey, note, restartDelayMs, deliveryContext } = await resolveConfigWriteParams(); const result = await callGatewayTool("config.apply", gatewayOpts, { raw, @@ -191,11 +332,12 @@ export function createGatewayTool(opts?: { sessionKey, note, restartDelayMs, + deliveryContext, }); return jsonResult({ ok: true, result }); } if (action === "config.patch") { - const { raw, baseHash, sessionKey, note, restartDelayMs } = + const { raw, baseHash, sessionKey, note, restartDelayMs, deliveryContext } = await resolveConfigWriteParams(); const result = await callGatewayTool("config.patch", gatewayOpts, { raw, @@ -203,11 +345,12 @@ export function createGatewayTool(opts?: { sessionKey, note, restartDelayMs, + deliveryContext, }); return jsonResult({ ok: true, result }); } if (action === "update.run") { - const { sessionKey, note, restartDelayMs } = resolveGatewayWriteMeta(); + const { sessionKey, note, restartDelayMs, deliveryContext } = resolveGatewayWriteMeta(); const updateTimeoutMs = gatewayOpts.timeoutMs ?? DEFAULT_UPDATE_TIMEOUT_MS; const updateGatewayOpts = { ...gatewayOpts, @@ -217,6 +360,7 @@ export function createGatewayTool(opts?: { sessionKey, note, restartDelayMs, + deliveryContext, timeoutMs: updateTimeoutMs, }); return jsonResult({ ok: true, result }); diff --git a/src/agents/tools/gateway.test.ts b/src/agents/tools/gateway.test.ts index 5f768775432..9f0d16eb361 100644 --- a/src/agents/tools/gateway.test.ts +++ b/src/agents/tools/gateway.test.ts @@ -1,189 +1,267 @@ -import { afterAll, beforeEach, describe, expect, it, vi } from "vitest"; -import { callGatewayTool, resolveGatewayOptions } from "./gateway.js"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; -const callGatewayMock = vi.fn(); -const configState = vi.hoisted(() => ({ - value: {} as Record, +// โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€ +// Mocks +// โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€ + +const mocks = vi.hoisted(() => ({ + loadConfig: vi.fn(() => ({})), + resolveGatewayPort: vi.fn(() => 18789), })); + vi.mock("../../config/config.js", () => ({ - loadConfig: () => configState.value, - resolveGatewayPort: () => 18789, + loadConfig: mocks.loadConfig, + resolveGatewayPort: mocks.resolveGatewayPort, })); -vi.mock("../../gateway/call.js", () => ({ - callGateway: (...args: unknown[]) => callGatewayMock(...args), +vi.mock("../../gateway/call.js", () => ({})); +vi.mock("../../gateway/credentials.js", () => ({ + resolveGatewayCredentialsFromConfig: vi.fn(), + trimToUndefined: (v: unknown) => + typeof v === "string" && v.trim().length > 0 ? v.trim() : undefined, +})); +vi.mock("../../gateway/method-scopes.js", () => ({ + resolveLeastPrivilegeOperatorScopesForMethod: vi.fn(() => []), +})); +vi.mock("../../utils/message-channel.js", () => ({ + GATEWAY_CLIENT_MODES: { BACKEND: "backend" }, + GATEWAY_CLIENT_NAMES: { GATEWAY_CLIENT: "gateway-client" }, +})); +vi.mock("./common.js", () => ({ + readStringParam: vi.fn(), })); -describe("gateway tool defaults", () => { - const envSnapshot = { - openclaw: process.env.OPENCLAW_GATEWAY_TOKEN, - clawdbot: process.env.CLAWDBOT_GATEWAY_TOKEN, - }; +const { resolveGatewayTarget } = await import("./gateway.js"); +// โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€ +// Helpers +// โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€ + +function setConfig(overrides: Record) { + mocks.loadConfig.mockReturnValue(overrides); +} + +// โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€ +// Suite: resolveGatewayTarget โ€” env URL overrides and remote-mode fallback +// โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€ + +describe("resolveGatewayTarget โ€“ env URL override classification", () => { beforeEach(() => { - callGatewayMock.mockClear(); - configState.value = {}; - delete process.env.OPENCLAW_GATEWAY_TOKEN; - delete process.env.CLAWDBOT_GATEWAY_TOKEN; + vi.clearAllMocks(); + setConfig({}); + delete process.env.OPENCLAW_GATEWAY_URL; + delete process.env.CLAWDBOT_GATEWAY_URL; }); - afterAll(() => { - if (envSnapshot.openclaw === undefined) { - delete process.env.OPENCLAW_GATEWAY_TOKEN; - } else { - process.env.OPENCLAW_GATEWAY_TOKEN = envSnapshot.openclaw; - } - if (envSnapshot.clawdbot === undefined) { - delete process.env.CLAWDBOT_GATEWAY_TOKEN; - } else { - process.env.CLAWDBOT_GATEWAY_TOKEN = envSnapshot.clawdbot; - } + afterEach(() => { + delete process.env.OPENCLAW_GATEWAY_URL; + delete process.env.CLAWDBOT_GATEWAY_URL; }); - it("leaves url undefined so callGateway can use config", () => { - const opts = resolveGatewayOptions(); - expect(opts.url).toBeUndefined(); + it("returns undefined (local) with no overrides and default config", () => { + expect(resolveGatewayTarget()).toBeUndefined(); }); - it("accepts allowlisted gatewayUrl overrides (SSRF hardening)", async () => { - callGatewayMock.mockResolvedValueOnce({ ok: true }); - await callGatewayTool( - "health", - { gatewayUrl: "ws://127.0.0.1:18789", gatewayToken: "t", timeoutMs: 5000 }, - {}, - ); - expect(callGatewayMock).toHaveBeenCalledWith( - expect.objectContaining({ - url: "ws://127.0.0.1:18789", - token: "t", - timeoutMs: 5000, - scopes: ["operator.read"], - }), - ); - }); - - it("uses OPENCLAW_GATEWAY_TOKEN for allowlisted local overrides", () => { - process.env.OPENCLAW_GATEWAY_TOKEN = "env-token"; - const opts = resolveGatewayOptions({ gatewayUrl: "ws://127.0.0.1:18789" }); - expect(opts.url).toBe("ws://127.0.0.1:18789"); - expect(opts.token).toBe("env-token"); - }); - - it("falls back to config gateway.auth.token when env is unset for local overrides", () => { - configState.value = { - gateway: { - auth: { token: "config-token" }, - }, - }; - const opts = resolveGatewayOptions({ gatewayUrl: "ws://127.0.0.1:18789" }); - expect(opts.token).toBe("config-token"); - }); - - it("uses gateway.remote.token for allowlisted remote overrides", () => { - configState.value = { - gateway: { - remote: { - url: "wss://gateway.example", - token: "remote-token", - }, - }, - }; - const opts = resolveGatewayOptions({ gatewayUrl: "wss://gateway.example" }); - expect(opts.url).toBe("wss://gateway.example"); - expect(opts.token).toBe("remote-token"); - }); - - it("does not leak local env/config tokens to remote overrides", () => { - process.env.OPENCLAW_GATEWAY_TOKEN = "local-env-token"; - process.env.CLAWDBOT_GATEWAY_TOKEN = "legacy-env-token"; - configState.value = { - gateway: { - auth: { token: "local-config-token" }, - remote: { - url: "wss://gateway.example", - }, - }, - }; - const opts = resolveGatewayOptions({ gatewayUrl: "wss://gateway.example" }); - expect(opts.token).toBeUndefined(); - }); - - it("ignores unresolved local token SecretRef for strict remote overrides", () => { - configState.value = { - gateway: { - auth: { - mode: "token", - token: { source: "env", provider: "default", id: "MISSING_LOCAL_TOKEN" }, - }, - remote: { - url: "wss://gateway.example", - }, - }, - secrets: { - providers: { - default: { source: "env" }, - }, - }, - }; - const opts = resolveGatewayOptions({ gatewayUrl: "wss://gateway.example" }); - expect(opts.token).toBeUndefined(); - }); - - it("explicit gatewayToken overrides fallback token resolution", () => { - process.env.OPENCLAW_GATEWAY_TOKEN = "local-env-token"; - configState.value = { - gateway: { - remote: { - url: "wss://gateway.example", - token: "remote-token", - }, - }, - }; - const opts = resolveGatewayOptions({ - gatewayUrl: "wss://gateway.example", - gatewayToken: "explicit-token", + it("returns 'remote' when gateway.mode=remote AND gateway.remote.url is set", () => { + setConfig({ + gateway: { mode: "remote", remote: { url: "wss://remote.example.com" } }, }); - expect(opts.token).toBe("explicit-token"); + expect(resolveGatewayTarget()).toBe("remote"); }); - it("uses least-privilege write scope for write methods", async () => { - callGatewayMock.mockResolvedValueOnce({ ok: true }); - await callGatewayTool("wake", {}, { mode: "now", text: "hi" }); - expect(callGatewayMock).toHaveBeenCalledWith( - expect.objectContaining({ - method: "wake", - scopes: ["operator.write"], - }), - ); + it("returns undefined when gateway.mode=remote but gateway.remote.url is missing (callGateway falls back to local)", () => { + // This was the key regression: mode=remote without a url falls back to loopback, but the + // old code returned "remote", causing deliveryContext to be suppressed for a local call. + setConfig({ gateway: { mode: "remote" } }); + expect(resolveGatewayTarget()).toBeUndefined(); }); - it("uses admin scope only for admin methods", async () => { - callGatewayMock.mockResolvedValueOnce({ ok: true }); - await callGatewayTool("cron.add", {}, { id: "job-1" }); - expect(callGatewayMock).toHaveBeenCalledWith( - expect.objectContaining({ - method: "cron.add", - scopes: ["operator.admin"], - }), - ); + it("returns undefined (local) when gateway.mode=remote with a loopback remote.url (no tunnel evidence)", () => { + // A configured loopback remote.url (e.g. ws://127.0.0.1:18789) is indistinguishable + // from a local gateway on a custom port. Without a non-loopback URL proving SSH tunnel + // usage, classify as local so deliveryContext is preserved and post-restart wake + // messages are not misrouted via stale extractDeliveryInfo routing. + setConfig({ gateway: { mode: "remote", remote: { url: "ws://127.0.0.1:18789" } } }); + expect(resolveGatewayTarget()).toBeUndefined(); }); - it("default-denies unknown methods by sending no scopes", async () => { - callGatewayMock.mockResolvedValueOnce({ ok: true }); - await callGatewayTool("nonexistent.method", {}, {}); - expect(callGatewayMock).toHaveBeenCalledWith( - expect.objectContaining({ - method: "nonexistent.method", - scopes: [], - }), - ); + it("returns undefined when gateway.mode=remote but gateway.remote.url is empty string", () => { + setConfig({ gateway: { mode: "remote", remote: { url: " " } } }); + expect(resolveGatewayTarget()).toBeUndefined(); }); - it("rejects non-allowlisted overrides (SSRF hardening)", async () => { - await expect( - callGatewayTool("health", { gatewayUrl: "ws://127.0.0.1:8080", gatewayToken: "t" }, {}), - ).rejects.toThrow(/gatewayUrl override rejected/i); - await expect( - callGatewayTool("health", { gatewayUrl: "ws://169.254.169.254", gatewayToken: "t" }, {}), - ).rejects.toThrow(/gatewayUrl override rejected/i); + it("classifies OPENCLAW_GATEWAY_URL loopback env override as 'local' (no remote config)", () => { + process.env.OPENCLAW_GATEWAY_URL = "ws://127.0.0.1:18789"; + setConfig({}); + expect(resolveGatewayTarget()).toBe("local"); + }); + + it("classifies CLAWDBOT_GATEWAY_URL loopback env override as 'local' (no remote config)", () => { + process.env.CLAWDBOT_GATEWAY_URL = "ws://localhost:18789"; + setConfig({}); + expect(resolveGatewayTarget()).toBe("local"); + }); + + it("classifies loopback env URL as 'remote' when mode=remote with non-loopback remote URL (SSH tunnel, same port)", () => { + // Common SSH tunnel pattern: ssh -N -L 18789:remote-host:18789 + // OPENCLAW_GATEWAY_URL points to the local tunnel endpoint but gateway is remote. + process.env.OPENCLAW_GATEWAY_URL = "ws://127.0.0.1:18789"; + setConfig({ + gateway: { mode: "remote", remote: { url: "wss://remote.example.com" } }, + }); + expect(resolveGatewayTarget()).toBe("remote"); + }); + + it("classifies loopback env URL with different port as 'remote' when mode=remote with non-loopback remote URL (SSH tunnel, different port)", () => { + // SSH tunnel to a non-default port: ssh -N -L 9000:remote-host:9000 + process.env.OPENCLAW_GATEWAY_URL = "ws://127.0.0.1:9000"; + setConfig({ + gateway: { mode: "remote", remote: { url: "wss://remote.example.com" } }, + }); + expect(resolveGatewayTarget()).toBe("remote"); + }); + + it("classifies loopback env URL as 'local' when mode=remote but remote.url is absent (callGateway falls back to local)", () => { + process.env.OPENCLAW_GATEWAY_URL = "ws://127.0.0.1:18789"; + setConfig({ gateway: { mode: "remote" } }); + expect(resolveGatewayTarget()).toBe("local"); + }); + + it("classifies loopback env URL on non-local port as 'local' without remote config (local gateway on custom port)", () => { + // ws://127.0.0.1:9000 with no non-loopback remote URL configured: cannot prove this is + // an SSH tunnel โ€” it may simply be a local gateway on a custom port. Preserve "local" + // so deliveryContext is not suppressed and heartbeat wake-up routing stays correct. + process.env.OPENCLAW_GATEWAY_URL = "ws://127.0.0.1:9000"; + setConfig({}); + expect(resolveGatewayTarget()).toBe("local"); + }); + + it("classifies loopback env URL on non-local port as 'local' when mode=remote but no remote.url (no tunnel evidence)", () => { + // mode=remote without a non-loopback remote.url is insufficient to prove SSH tunnel; + // treat as local gateway on custom port. + process.env.OPENCLAW_GATEWAY_URL = "ws://127.0.0.1:9000"; + setConfig({ gateway: { mode: "remote" } }); + expect(resolveGatewayTarget()).toBe("local"); + }); + + it("classifies loopback env URL as 'local' when mode=remote but remote.url is a loopback address (local-only setup)", () => { + process.env.OPENCLAW_GATEWAY_URL = "ws://127.0.0.1:18789"; + setConfig({ gateway: { mode: "remote", remote: { url: "ws://127.0.0.1:18789" } } }); + expect(resolveGatewayTarget()).toBe("local"); + }); + + it("classifies OPENCLAW_GATEWAY_URL matching gateway.remote.url as 'remote'", () => { + process.env.OPENCLAW_GATEWAY_URL = "wss://remote.example.com"; + setConfig({ + gateway: { mode: "remote", remote: { url: "wss://remote.example.com" } }, + }); + expect(resolveGatewayTarget()).toBe("remote"); + }); + + it("falls through to config-based resolution when OPENCLAW_GATEWAY_URL is rejected (malformed)", () => { + process.env.OPENCLAW_GATEWAY_URL = "not-a-url"; + setConfig({ + gateway: { mode: "remote", remote: { url: "wss://remote.example.com" } }, + }); + // Falls through to config check: mode=remote + remote.url present โ†’ "remote" + expect(resolveGatewayTarget()).toBe("remote"); + }); + + it("classifies env-only remote URL (not matching gateway.remote.url) as 'remote'", () => { + // callGateway uses the env URL as-is even when validateGatewayUrlOverrideForAgentTools + // rejects it (different host than configured gateway.remote.url). Must not leak + // deliveryContext into a remote call by falling back to 'local'. + process.env.OPENCLAW_GATEWAY_URL = "wss://other-host.example.com"; + setConfig({ + gateway: { mode: "remote", remote: { url: "wss://remote.example.com" } }, + }); + expect(resolveGatewayTarget()).toBe("remote"); + }); + + it("classifies env-only remote URL with no configured gateway.remote.url as 'remote'", () => { + // callGateway picks up the env URL even when gateway.remote.url is absent. + process.env.OPENCLAW_GATEWAY_URL = "wss://remote.example.com"; + setConfig({}); + expect(resolveGatewayTarget()).toBe("remote"); + }); + + it("classifies env URL with /ws path (rejected by allowlist) as 'remote'", () => { + // URLs with non-root paths are rejected by validateGatewayUrlOverrideForAgentTools but + // callGateway/buildGatewayConnectionDetails still use them verbatim. Classify correctly. + process.env.OPENCLAW_GATEWAY_URL = "wss://remote.example.com/ws"; + setConfig({}); + expect(resolveGatewayTarget()).toBe("remote"); + }); + + it("classifies loopback env URL with /ws path (rejected by allowlist) as 'local'", () => { + // Even with a non-root path, loopback targets remain local. + process.env.OPENCLAW_GATEWAY_URL = "ws://127.0.0.1:18789/ws"; + setConfig({}); + expect(resolveGatewayTarget()).toBe("local"); + }); + + it("OPENCLAW_GATEWAY_URL takes precedence over env CLAWDBOT_GATEWAY_URL", () => { + process.env.OPENCLAW_GATEWAY_URL = "ws://127.0.0.1:18789"; + process.env.CLAWDBOT_GATEWAY_URL = "wss://remote.example.com"; + setConfig({ + gateway: { mode: "remote", remote: { url: "wss://remote.example.com" } }, + }); + // OPENCLAW_GATEWAY_URL wins (loopback); mode=remote with non-loopback remote.url + // means this loopback is an SSH tunnel โ†’ "remote" + expect(resolveGatewayTarget()).toBe("remote"); + }); + + it("OPENCLAW_GATEWAY_URL takes precedence over env CLAWDBOT_GATEWAY_URL (no remote config โ†’ 'local')", () => { + process.env.OPENCLAW_GATEWAY_URL = "ws://127.0.0.1:18789"; + process.env.CLAWDBOT_GATEWAY_URL = "wss://remote.example.com"; + setConfig({}); + // OPENCLAW_GATEWAY_URL wins (loopback), no remote config โ†’ "local" + expect(resolveGatewayTarget()).toBe("local"); + }); +}); + +describe("resolveGatewayTarget โ€“ explicit gatewayUrl override", () => { + beforeEach(() => { + vi.clearAllMocks(); + setConfig({}); + delete process.env.OPENCLAW_GATEWAY_URL; + delete process.env.CLAWDBOT_GATEWAY_URL; + }); + + it("returns 'local' for loopback explicit gatewayUrl (no remote config)", () => { + expect(resolveGatewayTarget({ gatewayUrl: "ws://127.0.0.1:18789" })).toBe("local"); + }); + + it("returns 'remote' for explicit remote gatewayUrl matching configured remote URL", () => { + setConfig({ + gateway: { mode: "remote", remote: { url: "wss://remote.example.com" } }, + }); + expect(resolveGatewayTarget({ gatewayUrl: "wss://remote.example.com" })).toBe("remote"); + }); + + it("returns 'remote' for loopback explicit gatewayUrl when mode=remote with non-loopback remote URL (SSH tunnel)", () => { + setConfig({ + gateway: { mode: "remote", remote: { url: "wss://remote.example.com" } }, + }); + expect(resolveGatewayTarget({ gatewayUrl: "ws://127.0.0.1:18789" })).toBe("remote"); + }); + + it("returns 'local' for loopback explicit gatewayUrl on non-local port without remote config (local gateway on custom port)", () => { + // ws://127.0.0.1:9000 with no non-loopback remote URL: cannot distinguish SSH tunnel + // from local gateway on a custom port โ€” preserve "local" so deliveryContext is kept. + setConfig({}); + expect(resolveGatewayTarget({ gatewayUrl: "ws://127.0.0.1:9000" })).toBe("local"); + }); + + it("returns 'local' for loopback explicit gatewayUrl on non-local port when mode=remote but no remote.url (no tunnel evidence)", () => { + // mode=remote without a non-loopback remote.url cannot prove SSH tunnel; treat as local. + setConfig({ gateway: { mode: "remote" } }); + expect(resolveGatewayTarget({ gatewayUrl: "ws://localhost:9000" })).toBe("local"); + }); + + it("returns 'remote' for loopback explicit gatewayUrl on non-local port when mode=remote with non-loopback remote.url (SSH tunnel)", () => { + // With a non-loopback remote URL configured, a custom-port loopback is unambiguously + // an SSH tunnel endpoint โ€” classify as "remote" to suppress deliveryContext. + setConfig({ gateway: { mode: "remote", remote: { url: "wss://remote.example.com" } } }); + expect(resolveGatewayTarget({ gatewayUrl: "ws://127.0.0.1:9000" })).toBe("remote"); }); }); diff --git a/src/agents/tools/gateway.ts b/src/agents/tools/gateway.ts index c31b7751e10..613f57343e3 100644 --- a/src/agents/tools/gateway.ts +++ b/src/agents/tools/gateway.ts @@ -13,7 +13,7 @@ export type GatewayCallOptions = { timeoutMs?: number; }; -type GatewayOverrideTarget = "local" | "remote"; +export type GatewayOverrideTarget = "local" | "remote"; export function readGatewayCallOptions(params: Record): GatewayCallOptions { return { @@ -53,6 +53,35 @@ function canonicalizeToolGatewayWsUrl(raw: string): { origin: string; key: strin return { origin, key }; } +/** + * Returns true when gateway.mode=remote is configured with a non-loopback remote URL. + * This indicates the user is connecting to a remote gateway, possibly via SSH port forwarding + * (ssh -N -L :remote-host:). In that case, a loopback gatewayUrl + * is a tunnel endpoint and should be classified as "remote" so deliveryContext is suppressed. + */ +function isNonLoopbackRemoteUrlConfigured(cfg: ReturnType): boolean { + if (cfg.gateway?.mode !== "remote") { + return false; + } + const remoteUrl = + typeof cfg.gateway?.remote?.url === "string" ? cfg.gateway.remote.url.trim() : ""; + if (!remoteUrl) { + return false; + } + try { + const parsed = new URL(remoteUrl); + const host = parsed.hostname.toLowerCase().replace(/^\[|\]$/g, ""); + return !(host === "127.0.0.1" || host === "localhost" || host === "::1"); + } catch { + return false; + } +} + +function isLoopbackHostname(hostname: string): boolean { + const h = hostname.toLowerCase().replace(/^\[|\]$/g, ""); + return h === "127.0.0.1" || h === "localhost" || h === "::1"; +} + function validateGatewayUrlOverrideForAgentTools(params: { cfg: ReturnType; urlOverride: string; @@ -82,11 +111,29 @@ function validateGatewayUrlOverrideForAgentTools(params: { const parsed = canonicalizeToolGatewayWsUrl(params.urlOverride); if (localAllowed.has(parsed.key)) { - return { url: parsed.origin, target: "local" }; + // A loopback URL on the configured port is normally the local gateway, but when + // gateway.mode=remote is configured with a non-loopback remote URL, the user is + // likely using SSH port forwarding (ssh -N -L ...) and this loopback is a tunnel + // endpoint pointing to a remote gateway. Classify as "remote" so deliveryContext + // is not forwarded to the remote server, which would misroute post-restart wake messages. + const target = isNonLoopbackRemoteUrlConfigured(cfg) ? "remote" : "local"; + return { url: parsed.origin, target }; } if (remoteKey && parsed.key === remoteKey) { return { url: parsed.origin, target: "remote" }; } + // Loopback URL on a non-configured port โ€” could be either: + // (a) An SSH tunnel endpoint (ssh -N -L :remote-host:) โ†’ "remote" + // (b) A local gateway running on a custom/non-default port โ†’ "local" + // We can only distinguish (a) from (b) when a non-loopback remote URL is configured: + // that proves gateway.mode=remote with an external host, so a loopback URL on any port + // must be a forwarded tunnel. Without that evidence, treat the loopback as local so that + // deliveryContext is not suppressed and heartbeat wake-up routing stays correct. + const urlForTunnelCheck = new URL(params.urlOverride.trim()); // already validated above + if (isLoopbackHostname(urlForTunnelCheck.hostname)) { + const target = isNonLoopbackRemoteUrlConfigured(cfg) ? "remote" : "local"; + return { url: parsed.origin, target }; + } throw new Error( [ "gatewayUrl override rejected.", @@ -113,6 +160,85 @@ function resolveGatewayOverrideToken(params: { }).token; } +/** + * Resolves whether a GatewayCallOptions points to a local or remote gateway. + * Returns "remote" when a remote gatewayUrl override is present, OR when + * gateway.mode=remote is configured with a gateway.remote.url set. + * Returns "local" for explicit loopback URL overrides (127.0.0.1, localhost, [::1]) + * UNLESS gateway.mode=remote is configured with a non-loopback remote URL, which indicates + * the loopback is an SSH tunnel endpoint โ€” in that case returns "remote". + * Returns undefined when no override is present and the effective target is the local gateway + * (including the gateway.mode=remote + missing gateway.remote.url fallback-to-local case). + * + * This mirrors the URL resolution path used by callGateway/buildGatewayConnectionDetails so + * that deliveryContext suppression decisions are based on the actual connection target, not just + * the configured mode. Mismatches fixed vs the previous version: + * 1. gateway.mode=remote without gateway.remote.url: callGateway falls back to local loopback; + * classifying that as "remote" would incorrectly suppress deliveryContext. + * 2. Env URL overrides (OPENCLAW_GATEWAY_URL / CLAWDBOT_GATEWAY_URL) are picked up by + * callGateway but were ignored here, causing incorrect local/remote classification. + * 3. Tunneled loopback URLs (ssh -N -L ...) when gateway.mode=remote with a non-loopback + * remote.url is configured: classifying as "local" would forward deliveryContext to the + * remote server, causing post-restart wake messages to be misrouted to the caller's chat. + * 4. Loopback URLs on a non-local port (ssh -N -L :...) with local mode or no remote + * URL configured: the non-local port cannot be the local gateway, so it must be a tunnel; + * classifying as "local" would forward deliveryContext to the remote server (misrouting). + */ +export function resolveGatewayTarget(opts?: GatewayCallOptions): GatewayOverrideTarget | undefined { + const cfg = loadConfig(); + if (trimToUndefined(opts?.gatewayUrl) === undefined) { + // No explicit gatewayUrl param โ€” mirror callGateway's resolution path. + // Check env URL overrides first (same precedence as buildGatewayConnectionDetails). + const envUrlOverride = + trimToUndefined(process.env.OPENCLAW_GATEWAY_URL) ?? + trimToUndefined(process.env.CLAWDBOT_GATEWAY_URL); + if (envUrlOverride !== undefined) { + try { + return validateGatewayUrlOverrideForAgentTools({ + cfg, + urlOverride: envUrlOverride, + }).target; + } catch { + // URL rejected by the agent-tools allowlist (e.g. non-loopback URL not matching + // gateway.remote.url, or URL with a non-root path like /ws). callGateway / + // buildGatewayConnectionDetails will still use this env URL as-is, so we must + // classify based on the actual target host โ€” not silently fall back to local. + try { + const parsed = new URL(envUrlOverride.trim()); + // Normalize IPv6 brackets: "[::1]" โ†’ "::1" + const host = parsed.hostname.toLowerCase().replace(/^\[|\]$/g, ""); + const isLoopback = host === "127.0.0.1" || host === "localhost" || host === "::1"; + if (isLoopback) { + // Classify as "remote" only when a non-loopback remote URL is configured, + // which proves the loopback is an SSH tunnel endpoint + // (ssh -N -L :remote-host:). Without that evidence + // a loopback URL on any port โ€” including a non-default port โ€” could be a + // local gateway on a custom port, so we preserve "local" classification to + // keep deliveryContext intact and avoid heartbeat-stale routing regressions. + if (isNonLoopbackRemoteUrlConfigured(cfg)) { + return "remote"; + } + return "local"; + } + return "remote"; + } catch { + // Truly malformed URL; callGateway will also fail. Fall through to config-based resolution. + } + } + } + // No env override. Classify as "remote" only when mode=remote is configured with a + // non-loopback remote URL. Loopback remote.url (e.g. ws://127.0.0.1:18789) is + // indistinguishable from a local gateway on a custom port; without a non-loopback + // URL proving SSH tunnel usage, treat it as local so deliveryContext is preserved + // and post-restart wake messages are not misrouted. + return isNonLoopbackRemoteUrlConfigured(cfg) ? "remote" : undefined; + } + return validateGatewayUrlOverrideForAgentTools({ + cfg, + urlOverride: String(opts?.gatewayUrl), + }).target; +} + export function resolveGatewayOptions(opts?: GatewayCallOptions) { const cfg = loadConfig(); const validatedOverride = diff --git a/src/gateway/protocol/schema/config.ts b/src/gateway/protocol/schema/config.ts index 9d0ec876668..7b97b6864e1 100644 --- a/src/gateway/protocol/schema/config.ts +++ b/src/gateway/protocol/schema/config.ts @@ -17,6 +17,22 @@ export const ConfigSetParamsSchema = Type.Object( { additionalProperties: false }, ); +// deliveryContext carries the live channel/to/accountId from the agent run so +// that the restart sentinel has accurate routing data even after heartbeats +// have overwritten the session store with { channel: "webchat", to: "heartbeat" }. +// Without this field, the additionalProperties: false constraint silently drops +// it and the sentinel falls back to stale session store data. See #18612. +const DeliveryContextSchema = Type.Optional( + Type.Object( + { + channel: Type.Optional(Type.String()), + to: Type.Optional(Type.String()), + accountId: Type.Optional(Type.String()), + }, + { additionalProperties: false }, + ), +); + const ConfigApplyLikeParamsSchema = Type.Object( { raw: NonEmptyString, @@ -24,6 +40,7 @@ const ConfigApplyLikeParamsSchema = Type.Object( sessionKey: Type.Optional(Type.String()), note: Type.Optional(Type.String()), restartDelayMs: Type.Optional(Type.Integer({ minimum: 0 })), + deliveryContext: DeliveryContextSchema, }, { additionalProperties: false }, ); @@ -46,6 +63,7 @@ export const UpdateRunParamsSchema = Type.Object( note: Type.Optional(Type.String()), restartDelayMs: Type.Optional(Type.Integer({ minimum: 0 })), timeoutMs: Type.Optional(Type.Integer({ minimum: 1 })), + deliveryContext: DeliveryContextSchema, }, { additionalProperties: false }, ); diff --git a/src/gateway/server-methods/config.ts b/src/gateway/server-methods/config.ts index 977a59f00b5..e3300d02045 100644 --- a/src/gateway/server-methods/config.ts +++ b/src/gateway/server-methods/config.ts @@ -52,7 +52,7 @@ import { validateConfigSetParams, } from "../protocol/index.js"; import { resolveBaseHashParam } from "./base-hash.js"; -import { parseRestartRequestParams } from "./restart-request.js"; +import { parseDeliveryContextFromParams, parseRestartRequestParams } from "./restart-request.js"; import type { GatewayRequestHandlers, RespondFn } from "./types.js"; import { assertValidParams } from "./validation.js"; @@ -194,9 +194,25 @@ function resolveConfigRestartRequest(params: unknown): { } { const { sessionKey, note, restartDelayMs } = parseRestartRequestParams(params); - // Extract deliveryContext + threadId for routing after restart - // Supports both :thread: (most channels) and :topic: (Telegram) - const { deliveryContext, threadId } = extractDeliveryInfo(sessionKey); + // For deliveryContext, prefer the live context passed by the client over + // extractDeliveryInfo(), which reads the persisted session store. Heartbeat + // runs overwrite the store to { channel: "webchat", to: "heartbeat" }, so + // reading it here would produce stale routing data. See #18612. + // + // For threadId, also prefer the client-forwarded value when present. The + // session-key-derived threadId is empty for Slack sessions where replies are + // threaded (replyToMode="all") but the session key is not :thread:-scoped. + const { deliveryContext: extractedDeliveryContext, threadId: extractedThreadId } = + extractDeliveryInfo(sessionKey); + const paramsDeliveryContext = parseDeliveryContextFromParams(params); + const deliveryContext = + paramsDeliveryContext != null + ? { + ...paramsDeliveryContext, + accountId: paramsDeliveryContext.accountId ?? extractedDeliveryContext?.accountId, + } + : extractedDeliveryContext; + const threadId = paramsDeliveryContext?.threadId ?? extractedThreadId; return { sessionKey, diff --git a/src/gateway/server-methods/restart-request.test.ts b/src/gateway/server-methods/restart-request.test.ts new file mode 100644 index 00000000000..41191d5a09c --- /dev/null +++ b/src/gateway/server-methods/restart-request.test.ts @@ -0,0 +1,207 @@ +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 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)", () => { + expect( + parseDeliveryContextFromParams({ deliveryContext: { channel: "discord" } }), + ).toBeUndefined(); + }); + + 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({ + deliveryContext: { channel: "discord", to: "123456789" }, + }), + ).toEqual({ channel: "discord", to: "123456789", accountId: undefined, threadId: undefined }); + }); + + 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: { + channel: "slack", + to: "C012AB3CD", + accountId: "acct-1", + threadId: "1234567890.123456", + }, + }), + ).toEqual({ + channel: "slack", + to: "C012AB3CD", + accountId: "acct-1", + threadId: "1234567890.123456", + }); + }); + + // โ”€โ”€ 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 ", + accountId: " acct ", + 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-methods/restart-request.ts b/src/gateway/server-methods/restart-request.ts index f8b2ddb8c0d..e480de31d9e 100644 --- a/src/gateway/server-methods/restart-request.ts +++ b/src/gateway/server-methods/restart-request.ts @@ -1,3 +1,42 @@ +/** + * Parse the live deliveryContext passed by gateway-tool clients. + * + * Clients capture delivery context from the active agent run and forward it + * so server-side handlers can write an accurate sentinel without reading the + * persisted session store, which heartbeat runs frequently overwrite to + * { channel: "webchat", to: "heartbeat" }. See #18612. + */ +export function parseDeliveryContextFromParams( + params: unknown, +): { channel?: string; to?: string; accountId?: string; threadId?: string } | undefined { + const raw = (params as { deliveryContext?: unknown }).deliveryContext; + if (!raw || typeof raw !== "object") { + return undefined; + } + const channel = + typeof (raw as { channel?: unknown }).channel === "string" + ? (raw as { channel: string }).channel.trim() || undefined + : undefined; + const to = + typeof (raw as { to?: unknown }).to === "string" + ? (raw as { to: string }).to.trim() || undefined + : undefined; + const accountId = + typeof (raw as { accountId?: unknown }).accountId === "string" + ? (raw as { accountId: string }).accountId.trim() || undefined + : undefined; + const threadId = + typeof (raw as { threadId?: unknown }).threadId === "string" + ? (raw as { threadId: string }).threadId.trim() || undefined + : undefined; + // Require both channel and to โ€” a partial context can overwrite a complete + // extracted route and produce a non-routable sentinel. See #18612. + if (!channel || !to) { + return undefined; + } + return { channel, to, accountId, threadId }; +} + export function parseRestartRequestParams(params: unknown): { sessionKey: string | undefined; note: string | undefined; diff --git a/src/gateway/server-methods/update.ts b/src/gateway/server-methods/update.ts index bf53e2a0227..ab9d72b69f6 100644 --- a/src/gateway/server-methods/update.ts +++ b/src/gateway/server-methods/update.ts @@ -11,7 +11,7 @@ import { normalizeUpdateChannel } from "../../infra/update-channels.js"; import { runGatewayUpdate } from "../../infra/update-runner.js"; import { formatControlPlaneActor, resolveControlPlaneActor } from "../control-plane-audit.js"; import { validateUpdateRunParams } from "../protocol/index.js"; -import { parseRestartRequestParams } from "./restart-request.js"; +import { parseDeliveryContextFromParams, parseRestartRequestParams } from "./restart-request.js"; import type { GatewayRequestHandlers } from "./types.js"; import { assertValidParams } from "./validation.js"; @@ -22,7 +22,24 @@ export const updateHandlers: GatewayRequestHandlers = { } const actor = resolveControlPlaneActor(client); const { sessionKey, note, restartDelayMs } = parseRestartRequestParams(params); - const { deliveryContext, threadId } = extractDeliveryInfo(sessionKey); + // Prefer live deliveryContext from params over extractDeliveryInfo() (see #18612). + // Also prefer threadId from params when present โ€” the session-key-derived value + // is empty for Slack sessions where replyToMode="all" but the key is not :thread:-scoped. + const { deliveryContext: extractedDeliveryContext, threadId: extractedThreadId } = + extractDeliveryInfo(sessionKey); + const paramsDeliveryContext = parseDeliveryContextFromParams(params); + // When live channel/to is present but accountId is missing (e.g. /tools/invoke without + // x-openclaw-account-id), fall back to the session-extracted account so the sentinel is + // not written without account context โ€” which would cause deliveries to use the channel + // default account and misroute in multi-account setups. See config.ts for the same pattern. + const deliveryContext = + paramsDeliveryContext != null + ? { + ...paramsDeliveryContext, + accountId: paramsDeliveryContext.accountId ?? extractedDeliveryContext?.accountId, + } + : extractedDeliveryContext; + const threadId = paramsDeliveryContext?.threadId ?? extractedThreadId; const timeoutMsRaw = (params as { timeoutMs?: unknown }).timeoutMs; const timeoutMs = typeof timeoutMsRaw === "number" && Number.isFinite(timeoutMsRaw) diff --git a/src/gateway/server-restart-sentinel.test.ts b/src/gateway/server-restart-sentinel.test.ts index 187698b06ed..d8fb903f161 100644 --- a/src/gateway/server-restart-sentinel.test.ts +++ b/src/gateway/server-restart-sentinel.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(() => ({ resolveSessionAgentId: vi.fn(() => "agent-from-key"), @@ -13,6 +13,10 @@ const mocks = vi.hoisted(() => ({ }, })), formatRestartSentinelMessage: vi.fn(() => "restart message"), + formatRestartSentinelUserMessage: vi.fn(() => "Gateway restarted successfully."), + formatRestartSentinelInternalContext: vi.fn( + () => "[Gateway restart context โ€” internal]\nkind: restart\nstatus: ok", + ), summarizeRestartSentinel: vi.fn(() => "restart summary"), resolveMainSessionKeyFromConfig: vi.fn(() => "agent:main:main"), parseSessionThreadInfo: vi.fn(() => ({ baseSessionKey: null, threadId: undefined })), @@ -25,8 +29,11 @@ const mocks = vi.hoisted(() => ({ })), normalizeChannelId: vi.fn((channel: string) => channel), resolveOutboundTarget: vi.fn(() => ({ ok: true as const, to: "+15550002" })), - deliverOutboundPayloads: vi.fn(async () => []), + deliverOutboundPayloads: vi.fn(async () => undefined), + buildOutboundSessionContext: vi.fn(() => ({ agentId: "main", sessionKey: "agent:main:main" })), + agentCommand: vi.fn(async () => undefined), enqueueSystemEvent: vi.fn(), + defaultRuntime: {}, })); vi.mock("../agents/agent-scope.js", () => ({ @@ -36,6 +43,8 @@ vi.mock("../agents/agent-scope.js", () => ({ vi.mock("../infra/restart-sentinel.js", () => ({ consumeRestartSentinel: mocks.consumeRestartSentinel, formatRestartSentinelMessage: mocks.formatRestartSentinelMessage, + formatRestartSentinelUserMessage: mocks.formatRestartSentinelUserMessage, + formatRestartSentinelInternalContext: mocks.formatRestartSentinelInternalContext, summarizeRestartSentinel: mocks.summarizeRestartSentinel, })); @@ -72,23 +81,284 @@ vi.mock("../infra/outbound/deliver.js", () => ({ deliverOutboundPayloads: mocks.deliverOutboundPayloads, })); +vi.mock("../infra/outbound/session-context.js", () => ({ + buildOutboundSessionContext: mocks.buildOutboundSessionContext, +})); + +vi.mock("../commands/agent.js", () => ({ + agentCommand: mocks.agentCommand, +})); + +vi.mock("../runtime.js", () => ({ + defaultRuntime: mocks.defaultRuntime, +})); + vi.mock("../infra/system-events.js", () => ({ enqueueSystemEvent: mocks.enqueueSystemEvent, })); const { scheduleRestartSentinelWake } = await import("./server-restart-sentinel.js"); -describe("scheduleRestartSentinelWake", () => { - it("forwards session context to outbound delivery", async () => { +// โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€ +// Suite 1 โ€” Agent resume flow (no direct channel delivery) +// โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€ + +describe("scheduleRestartSentinelWake โ€“ agent resume, no raw delivery", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it("resumes agent with internal context and never delivers raw sentinel fields to channel", async () => { await scheduleRestartSentinelWake({ deps: {} as never }); - expect(mocks.deliverOutboundPayloads).toHaveBeenCalledWith( + // Raw sentinel fields must NEVER go directly to the channel โ€” no deliverOutboundPayloads call. + expect(mocks.deliverOutboundPayloads).not.toHaveBeenCalled(); + + // Agent resume: summary as neutral wake prompt, full context in extraSystemPrompt only. + expect(mocks.agentCommand).toHaveBeenCalledWith( expect.objectContaining({ - channel: "whatsapp", + message: "restart summary", + extraSystemPrompt: expect.stringContaining("[Gateway restart context"), + sessionKey: "agent:main:main", to: "+15550002", - session: { key: "agent:main:main", agentId: "agent-from-key" }, + channel: "whatsapp", + deliver: true, + bestEffortDeliver: true, + messageChannel: "whatsapp", + accountId: "acct-2", }), + mocks.defaultRuntime, + {}, ); + + expect(mocks.enqueueSystemEvent).not.toHaveBeenCalled(); + }); + + 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 as never); + + await scheduleRestartSentinelWake({ deps: {} as never }); + + 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 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"), + } as never); + + await scheduleRestartSentinelWake({ deps: {} as never }); + + expect(mocks.agentCommand).not.toHaveBeenCalled(); + expect(mocks.enqueueSystemEvent).toHaveBeenCalledWith("Gateway restarted successfully.", { + sessionKey: "agent:main:main", + }); + }); + + 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 as never) // inner: sessionDeliveryContext merge + .mockReturnValueOnce({ to: "+15550002" } as never); // outer: sentinelContext wins, no channel + + await scheduleRestartSentinelWake({ deps: {} as never }); + + expect(mocks.agentCommand).not.toHaveBeenCalled(); + expect(mocks.enqueueSystemEvent).toHaveBeenCalledWith("Gateway restarted successfully.", { + 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 as never) + .mockReturnValueOnce({ channel: "whatsapp" } as never); + + await scheduleRestartSentinelWake({ deps: {} as never }); + + expect(mocks.agentCommand).not.toHaveBeenCalled(); + expect(mocks.enqueueSystemEvent).toHaveBeenCalledWith("Gateway restarted successfully.", { + sessionKey: "agent:main:main", + }); + }); + + it("falls back to enqueueSystemEvent (with summary + error) when agentCommand throws", async () => { + mocks.agentCommand.mockRejectedValueOnce(new Error("agent failed")); + + await scheduleRestartSentinelWake({ deps: {} as never }); + + // No direct delivery โ€” never. + expect(mocks.deliverOutboundPayloads).not.toHaveBeenCalled(); + // Fallback enqueues summary + error so the user isn't left silent. + expect(mocks.enqueueSystemEvent).toHaveBeenCalledWith( + expect.stringContaining("restart summary"), + { sessionKey: "agent:main:main" }, + ); + }); +}); + +// โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€ +// Suite 3 โ€” Thread routing (Slack vs non-Slack) +// โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€ + +describe("scheduleRestartSentinelWake โ€“ thread routing", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it("passes Slack threadId to agentCommand (Slack threading handled internally by agentCommand)", async () => { + mocks.consumeRestartSentinel.mockResolvedValueOnce({ + payload: { + sessionKey: "agent:main:main", + deliveryContext: { channel: "slack", to: "C012AB3CD", accountId: "acct-2" }, + threadId: "1234567890.123456", + }, + } as never); + mocks.normalizeChannelId.mockReturnValueOnce("slack"); + + await scheduleRestartSentinelWake({ deps: {} as never }); + + // No direct delivery โ€” agentCommand is the only delivery path. + expect(mocks.deliverOutboundPayloads).not.toHaveBeenCalled(); + const agentOpts = getArg>(mocks.agentCommand, 0); + expect(agentOpts.threadId).toBe("1234567890.123456"); + }); + + it("passes threadId directly for non-Slack channels", async () => { + mocks.consumeRestartSentinel.mockResolvedValueOnce({ + payload: { + sessionKey: "agent:main:main", + deliveryContext: { channel: "discord", to: "123456789", accountId: "acct-2" }, + threadId: "discord-thread-id", + }, + } as never); + + await scheduleRestartSentinelWake({ deps: {} as never }); + + expect(mocks.deliverOutboundPayloads).not.toHaveBeenCalled(); + const agentOpts = getArg>(mocks.agentCommand, 0); + expect(agentOpts.threadId).toBe("discord-thread-id"); + }); + + it("passes threadId to agentCommand for non-Slack threading", async () => { + mocks.consumeRestartSentinel.mockResolvedValueOnce({ + payload: { + sessionKey: "agent:main:main", + deliveryContext: { channel: "discord", to: "123456789", accountId: "acct-2" }, + threadId: "discord-thread-id", + }, + } as never); + + 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", accountId: "acct-2" }, + threadId: "sentinel-thread", + }, + } as never); + // parseSessionThreadInfo would derive a different threadId from the session key + mocks.parseSessionThreadInfo.mockReturnValueOnce({ + baseSessionKey: null, + threadId: "session-thread", + } as never); + + 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", + } as never); + + 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 + } as never); + mocks.deliveryContextFromSession.mockReturnValueOnce({ + channel: "telegram", + to: "+19990001", + } as never); + // Mock both merge calls: inner produces session ctx; outer passes it through + mocks.mergeDeliveryContext + .mockReturnValueOnce({ channel: "telegram", to: "+19990001" } as never) // inner + .mockReturnValueOnce({ channel: "telegram", to: "+19990001" } as never); // 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; +} diff --git a/src/gateway/server-restart-sentinel.ts b/src/gateway/server-restart-sentinel.ts index e6191942dba..95ce7d53049 100644 --- a/src/gateway/server-restart-sentinel.ts +++ b/src/gateway/server-restart-sentinel.ts @@ -1,28 +1,35 @@ import { resolveAnnounceTargetFromKey } from "../agents/tools/sessions-send-helpers.js"; import { normalizeChannelId } from "../channels/plugins/index.js"; import type { CliDeps } from "../cli/deps.js"; +import { agentCommand } from "../commands/agent.js"; import { resolveMainSessionKeyFromConfig } from "../config/sessions.js"; import { parseSessionThreadInfo } from "../config/sessions/delivery-info.js"; -import { deliverOutboundPayloads } from "../infra/outbound/deliver.js"; -import { buildOutboundSessionContext } from "../infra/outbound/session-context.js"; import { resolveOutboundTarget } from "../infra/outbound/targets.js"; import { consumeRestartSentinel, + formatRestartSentinelInternalContext, formatRestartSentinelMessage, + formatRestartSentinelUserMessage, summarizeRestartSentinel, } from "../infra/restart-sentinel.js"; import { enqueueSystemEvent } from "../infra/system-events.js"; +import { defaultRuntime } from "../runtime.js"; import { deliveryContextFromSession, mergeDeliveryContext } from "../utils/delivery-context.js"; import { loadSessionEntry } from "./session-utils.js"; -export async function scheduleRestartSentinelWake(_params: { deps: CliDeps }) { +export async function scheduleRestartSentinelWake(params: { deps: CliDeps }) { const sentinel = await consumeRestartSentinel(); if (!sentinel) { return; } const payload = sentinel.payload; const sessionKey = payload.sessionKey?.trim(); + // Raw diagnostic message (used for system events and enqueue fallbacks). const message = formatRestartSentinelMessage(payload); + // Human-friendly message for direct user delivery โ€” omits status prefix and doctorHint. + const userMessage = formatRestartSentinelUserMessage(payload); + // Full technical context injected into the agent's system prompt. + const internalContext = formatRestartSentinelInternalContext(payload); const summary = summarizeRestartSentinel(payload); if (!sessionKey) { @@ -54,7 +61,7 @@ export async function scheduleRestartSentinelWake(_params: { deps: CliDeps }) { const channel = channelRaw ? normalizeChannelId(channelRaw) : null; const to = origin?.to; if (!channel || !to) { - enqueueSystemEvent(message, { sessionKey }); + enqueueSystemEvent(userMessage, { sessionKey }); return; } @@ -66,7 +73,7 @@ export async function scheduleRestartSentinelWake(_params: { deps: CliDeps }) { mode: "implicit", }); if (!resolved.ok) { - enqueueSystemEvent(message, { sessionKey }); + enqueueSystemEvent(userMessage, { sessionKey }); return; } @@ -76,31 +83,44 @@ export async function scheduleRestartSentinelWake(_params: { deps: CliDeps }) { sessionThreadId ?? (origin?.threadId != null ? String(origin.threadId) : undefined); - // Slack uses replyToId (thread_ts) for threading, not threadId. - // The reply path does this mapping but deliverOutboundPayloads does not, - // so we must convert here to ensure post-restart notifications land in - // the originating Slack thread. See #17716. - const isSlack = channel === "slack"; - const replyToId = isSlack && threadId != null && threadId !== "" ? String(threadId) : undefined; - const resolvedThreadId = isSlack ? undefined : threadId; - const outboundSession = buildOutboundSessionContext({ - cfg, - sessionKey, - }); - + // Trigger an agent resume turn so the agent can compose a natural response and + // continue autonomously after restart. The restart context is injected via + // extraSystemPrompt โ€” the raw note, doctorHint, and status fields are NEVER + // sent directly to the channel. Only the agent's composed reply reaches the user. + // + // summary is used as the neutral wake prompt (e.g. "Gateway restart config-patch ok"). + // It is an internal technical label; the agent sees it but users do not โ€” only the + // agent's response is delivered. + // + // This is safe post-restart: scheduleRestartSentinelWake() runs in the new process + // with zero in-flight replies, so the pre-restart race condition (ab4a08a82) does + // not apply here. + // + // Explicitly set senderIsOwner: false. The restart wake runs in a new process after + // an operator-triggered restart, and we cannot reliably infer the original sender's + // authorization level. Defaulting to false prevents privilege escalation where any + // restarted session would inherit owner-level access. See #18612. try { - await deliverOutboundPayloads({ - cfg, - channel, - to: resolved.to, - accountId: origin?.accountId, - replyToId, - threadId: resolvedThreadId, - payloads: [{ text: message }], - session: outboundSession, - bestEffort: true, - }); + await agentCommand( + { + message: summary, + extraSystemPrompt: internalContext, + sessionKey, + to: resolved.to, + channel, + deliver: true, + bestEffortDeliver: true, + messageChannel: channel, + threadId, + accountId: origin?.accountId, + senderIsOwner: false, + }, + defaultRuntime, + params.deps, + ); } catch (err) { + // Agent failed โ€” fall back to a clean restart notice without raw sentinel fields + // so the user isn't left completely silent after a restart. enqueueSystemEvent(`${summary}\n${String(err)}`, { sessionKey }); } } diff --git a/src/infra/restart-sentinel.test.ts b/src/infra/restart-sentinel.test.ts index c28504685bb..e69cdfb5baf 100644 --- a/src/infra/restart-sentinel.test.ts +++ b/src/infra/restart-sentinel.test.ts @@ -5,8 +5,9 @@ import { afterEach, beforeEach, describe, expect, it } from "vitest"; import { captureEnv } from "../test-utils/env.js"; import { consumeRestartSentinel, - formatDoctorNonInteractiveHint, + formatRestartSentinelInternalContext, formatRestartSentinelMessage, + formatRestartSentinelUserMessage, readRestartSentinel, resolveRestartSentinelPath, summarizeRestartSentinel, @@ -160,6 +161,139 @@ describe("restart sentinel", () => { }); }); +describe("formatRestartSentinelUserMessage", () => { + it("returns generic success message regardless of note (note is internal only)", () => { + // The `note`/`message` field is an operator annotation โ€” it must never be surfaced + // directly to the user. Only the agent (via internalContext) should see it. + const payload = { + kind: "config-patch" as const, + status: "ok" as const, + ts: Date.now(), + message: "testing restart sentinel", + doctorHint: "Run: openclaw doctor --non-interactive", + }; + const result = formatRestartSentinelUserMessage(payload); + expect(result).toBe("Gateway restarted successfully."); + expect(result).not.toContain("testing restart sentinel"); + expect(result).not.toContain("config-patch"); + expect(result).not.toContain("doctor"); + }); + + it("returns generic success message when no note", () => { + const payload = { + kind: "update" as const, + status: "ok" as const, + ts: Date.now(), + }; + expect(formatRestartSentinelUserMessage(payload)).toBe("Gateway restarted successfully."); + }); + + it("returns generic failure message for error status (note is internal only)", () => { + // Raw note must not appear in user-facing fallback even on error. + const payload = { + kind: "config-apply" as const, + status: "error" as const, + ts: Date.now(), + message: "disk full", + }; + const result = formatRestartSentinelUserMessage(payload); + expect(result).toBe("Gateway restart failed."); + expect(result).not.toContain("disk full"); + }); + + it("returns generic failure message for error without note", () => { + const payload = { + kind: "restart" as const, + status: "error" as const, + ts: Date.now(), + }; + expect(formatRestartSentinelUserMessage(payload)).toBe("Gateway restart failed."); + }); + + it("returns skipped message for skipped status", () => { + const payload = { + kind: "update" as const, + status: "skipped" as const, + ts: Date.now(), + }; + expect(formatRestartSentinelUserMessage(payload)).toBe( + "Gateway restart skipped (no restart was performed).", + ); + }); + + it("returns skipped message for skipped status even with a note", () => { + const payload = { + kind: "update" as const, + status: "skipped" as const, + ts: Date.now(), + message: "update already up to date", + }; + const result = formatRestartSentinelUserMessage(payload); + expect(result).toBe("Gateway restart skipped (no restart was performed)."); + expect(result).not.toContain("already up to date"); + }); + + it("never includes doctorHint", () => { + const payload = { + kind: "config-patch" as const, + status: "ok" as const, + ts: Date.now(), + message: "applied config", + doctorHint: "Run: openclaw doctor --non-interactive", + }; + expect(formatRestartSentinelUserMessage(payload)).not.toContain("doctor"); + expect(formatRestartSentinelUserMessage(payload)).not.toContain("openclaw"); + }); +}); + +describe("formatRestartSentinelInternalContext", () => { + it("includes kind, status, note, and doctorHint", () => { + const payload = { + kind: "config-patch" as const, + status: "ok" as const, + ts: Date.now(), + message: "testing restart sentinel", + doctorHint: "Run: openclaw doctor --non-interactive", + stats: { mode: "gateway.config-patch", reason: "discovery.mdns.mode changed" }, + }; + const result = formatRestartSentinelInternalContext(payload); + expect(result).toContain("kind: config-patch"); + expect(result).toContain("status: ok"); + expect(result).toContain("note: testing restart sentinel"); + expect(result).toContain("hint: Run: openclaw doctor"); + expect(result).toContain("mode: gateway.config-patch"); + expect(result).toContain("reason: discovery.mdns.mode changed"); + expect(result).toContain("internal"); + }); + + it("omits empty optional fields", () => { + const payload = { + kind: "restart" as const, + status: "ok" as const, + ts: Date.now(), + }; + const result = formatRestartSentinelInternalContext(payload); + expect(result).not.toContain("note:"); + expect(result).not.toContain("hint:"); + expect(result).not.toContain("reason:"); + expect(result).not.toContain("mode:"); + }); + + it("omits reason when it duplicates note", () => { + const note = "Applying config changes"; + const payload = { + kind: "config-apply" as const, + status: "ok" as const, + ts: Date.now(), + message: note, + stats: { reason: note }, + }; + const result = formatRestartSentinelInternalContext(payload); + const noteOccurrences = result.split(note).length - 1; + expect(noteOccurrences).toBe(1); + }); +}); + describe("restart sentinel message dedup", () => { it("omits duplicate Reason: line when stats.reason matches message", () => { const payload = { diff --git a/src/infra/restart-sentinel.ts b/src/infra/restart-sentinel.ts index baf8168047d..c15e2aee7cb 100644 --- a/src/infra/restart-sentinel.ts +++ b/src/infra/restart-sentinel.ts @@ -127,6 +127,50 @@ export function formatRestartSentinelMessage(payload: RestartSentinelPayload): s return lines.join("\n"); } +/** + * Clean fallback message for system-event delivery when the agent cannot be woken. + * Never includes the raw `note`/`message` field โ€” that belongs in the agent's + * internal context only (see formatRestartSentinelInternalContext). The note is + * an operator annotation, not a user-facing string. + */ +export function formatRestartSentinelUserMessage(payload: RestartSentinelPayload): string { + if (payload.status === "error") { + return "Gateway restart failed."; + } + if (payload.status === "skipped") { + return "Gateway restart skipped (no restart was performed)."; + } + return "Gateway restarted successfully."; +} + +/** + * Full technical restart context injected into the agent's system prompt so + * it can reason about and respond to the restart without exposing raw + * diagnostic text directly in the user-facing chat message. + */ +export function formatRestartSentinelInternalContext(payload: RestartSentinelPayload): string { + const lines: string[] = [ + "[Gateway restart context โ€” internal, do not surface raw details to user]", + `kind: ${payload.kind}`, + `status: ${payload.status}`, + ]; + const note = payload.message?.trim(); + if (note) { + lines.push(`note: ${note}`); + } + const reason = payload.stats?.reason?.trim(); + if (reason && reason !== note) { + lines.push(`reason: ${reason}`); + } + if (payload.stats?.mode?.trim()) { + lines.push(`mode: ${payload.stats.mode.trim()}`); + } + if (payload.doctorHint?.trim()) { + lines.push(`hint: ${payload.doctorHint.trim()}`); + } + return lines.join("\n"); +} + export function summarizeRestartSentinel(payload: RestartSentinelPayload): string { const kind = payload.kind; const status = payload.status;