From fe513ffe5e76c698d72d153db618fbcc91bf910d Mon Sep 17 00:00:00 2001 From: Ayaan Zaidi Date: Fri, 3 Jul 2026 12:51:24 -0700 Subject: [PATCH] fix(agents): make cli binding tool-scope hash session-stable --- src/agents/cli-runner/prepare.test.ts | 125 ++++++++++++++++++++ src/agents/cli-runner/prepare.ts | 16 +-- src/gateway/tool-resolution.exclude.test.ts | 27 ++++- 3 files changed, 155 insertions(+), 13 deletions(-) diff --git a/src/agents/cli-runner/prepare.test.ts b/src/agents/cli-runner/prepare.test.ts index 4f094d4df5dd..d28bd3c49fe7 100644 --- a/src/agents/cli-runner/prepare.test.ts +++ b/src/agents/cli-runner/prepare.test.ts @@ -2117,6 +2117,131 @@ describe("shouldSkipLocalCliCredentialEpoch", () => { }, ); + it("reuses CLI session bindings across owner sender flips with stable prompt tool scope", async () => { + const { dir, sessionFile } = createSessionFile(); + try { + const getActiveMcpLoopbackRuntime = vi.fn(() => ({ + port: 31783, + ownerToken: "loopback-owner-token", + nonOwnerToken: "loopback-non-owner-token", + })); + const resolveMcpLoopbackScopedTools = vi.fn((scope: { senderIsOwner?: boolean }) => ({ + agentId: "main", + tools: [ + { + name: "message", + label: "Message", + description: "Send a message", + parameters: { type: "object", properties: {} }, + execute: vi.fn(), + }, + ...(scope.senderIsOwner === false + ? [] + : [ + { + name: "gateway", + label: "Gateway", + description: "Manage the gateway", + parameters: { type: "object", properties: {} }, + execute: vi.fn(), + }, + ]), + ], + })); + setCliRunnerPrepareTestDeps({ + getActiveMcpLoopbackRuntime, + resolveMcpLoopbackScopedTools, + }); + cliBackendsTesting.setDepsForTest({ + resolvePluginSetupCliBackend: () => undefined, + resolveRuntimeCliBackends: () => [ + { + id: "native-cli", + pluginId: "native-plugin", + bundleMcp: true, + bundleMcpMode: "claude-config-file", + config: { + command: "native-cli", + args: ["--print"], + systemPromptArg: "--system-prompt", + systemPromptWhen: "first", + output: "text", + input: "arg", + sessionMode: "existing", + }, + }, + ], + }); + const cliSessionBindingFacts = { + extraSystemPromptStatic: "group:telegram:group:message_tool_only", + sourceReplyDeliveryMode: "message_tool_only" as const, + }; + const first = await prepareCliRunContext({ + sessionId: "session-test", + sessionKey: "agent:main:telegram:group:chat123", + sessionFile, + workspaceDir: dir, + prompt: "first ask", + provider: "native-cli", + model: "test-model", + timeoutMs: 1_000, + runId: "run-test-owner-tool-scope-a", + extraSystemPrompt: "volatile owner turn", + currentMessageId: "owner-message", + senderIsOwner: true, + cliSessionBindingFacts, + config: createCliBackendConfig({ bundleMcp: true }), + }); + const second = await prepareCliRunContext({ + sessionId: "session-test", + sessionKey: "agent:main:telegram:group:chat123", + sessionFile, + workspaceDir: dir, + prompt: "second ask", + provider: "native-cli", + model: "test-model", + timeoutMs: 1_000, + runId: "run-test-owner-tool-scope-b", + extraSystemPrompt: "volatile non-owner turn", + currentMessageId: "non-owner-message", + senderIsOwner: false, + cliSessionBindingFacts, + cliSessionBinding: { + sessionId: "cli-session", + extraSystemPromptHash: first.extraSystemPromptHash, + messageToolPolicyHash: first.messageToolPolicyHash, + promptToolNamesHash: first.promptToolNamesHash, + cwdHash: hashCliSessionText(dir), + mcpConfigHash: first.preparedBackend.mcpConfigHash, + mcpResumeHash: first.preparedBackend.mcpResumeHash, + }, + config: createCliBackendConfig({ bundleMcp: true }), + }); + + expect(resolveMcpLoopbackScopedTools).toHaveBeenCalledTimes(2); + expect(resolveMcpLoopbackScopedTools).toHaveBeenNthCalledWith( + 1, + expect.objectContaining({ + senderIsOwner: undefined, + currentMessageId: undefined, + sourceReplyDeliveryMode: "message_tool_only", + }), + ); + expect(resolveMcpLoopbackScopedTools).toHaveBeenNthCalledWith( + 2, + expect.objectContaining({ + senderIsOwner: undefined, + currentMessageId: undefined, + sourceReplyDeliveryMode: "message_tool_only", + }), + ); + expect(second.promptToolNamesHash).toBe(first.promptToolNamesHash); + expect(second.reusableCliSession).toEqual({ sessionId: "cli-session" }); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + it("prepares raw-tail history for safe invalidations only when the backend opts in", async () => { const { dir, sessionFile } = createSessionFile(); appendTranscriptEntry(sessionFile, { diff --git a/src/agents/cli-runner/prepare.ts b/src/agents/cli-runner/prepare.ts index 08216bb1a7bb..f1f39c335a65 100644 --- a/src/agents/cli-runner/prepare.ts +++ b/src/agents/cli-runner/prepare.ts @@ -601,14 +601,16 @@ export async function prepareCliRunContext( sessionKey: params.sessionKey ?? "", messageProvider: params.messageChannel ?? params.messageProvider, currentChannelId: params.currentChannelId, - currentThreadTs: params.currentThreadTs, - currentMessageId: params.currentMessageId, - currentInboundAudio: params.currentInboundAudio, + // CLI binding hashes must use session-stable prompt facts. Per-sender + // and per-message scope stays in the runtime MCP env/list-call path. + currentThreadTs: undefined, + currentMessageId: undefined, + currentInboundAudio: undefined, accountId: params.agentAccountId, - inboundEventKind: params.currentInboundEventKind, - sourceReplyDeliveryMode: params.sourceReplyDeliveryMode, - requireExplicitMessageTarget, - senderIsOwner: params.senderIsOwner, + inboundEventKind: undefined, + sourceReplyDeliveryMode: bindingSourceReplyDeliveryMode, + requireExplicitMessageTarget: bindingRequireExplicitMessageTarget, + senderIsOwner: undefined, }).tools : []; const promptToolNamesHash = diff --git a/src/gateway/tool-resolution.exclude.test.ts b/src/gateway/tool-resolution.exclude.test.ts index 898365a2c413..8b74073a63c4 100644 --- a/src/gateway/tool-resolution.exclude.test.ts +++ b/src/gateway/tool-resolution.exclude.test.ts @@ -42,12 +42,12 @@ describe("resolveGatewayScopedTools excludeToolNames", () => { hoisted.createOpenClawToolsMock.mockClear(); }); - function readCreateToolsArgs(): { + function readCreateToolsArgs(index = 0): { cronCreatorToolAllowlist?: Array; inheritedToolDenylist?: string[]; pluginToolDenylist?: string[]; } { - const args = hoisted.createOpenClawToolsMock.mock.calls[0]?.[0]; + const args = hoisted.createOpenClawToolsMock.mock.calls[index]?.[0]; if (!args || typeof args !== "object") { throw new Error("expected createOpenClawTools args"); } @@ -77,8 +77,16 @@ describe("resolveGatewayScopedTools excludeToolNames", () => { expect(args.inheritedToolDenylist).toEqual([]); }); - it("filters owner-only core tools from non-owner loopback callers", () => { - const result = resolveGatewayScopedTools({ + it("keeps owner-only core tools visible only for owner loopback callers", () => { + const ownerResult = resolveGatewayScopedTools({ + cfg: { + gateway: { tools: { allow: ["gateway"] } }, + } as OpenClawConfig, + sessionKey: "agent:main:direct:test", + surface: "loopback", + senderIsOwner: true, + }); + const nonOwnerResult = resolveGatewayScopedTools({ cfg: { gateway: { tools: { allow: ["gateway"] } }, } as OpenClawConfig, @@ -87,8 +95,15 @@ describe("resolveGatewayScopedTools excludeToolNames", () => { senderIsOwner: false, }); - expect(result.tools.map((tool) => tool.name)).toEqual(["read", "sessions_spawn"]); - const args = readCreateToolsArgs(); + expect(ownerResult.tools.map((tool) => tool.name)).toEqual([ + "read", + "sessions_spawn", + "cron", + "gateway", + "nodes", + ]); + expect(nonOwnerResult.tools.map((tool) => tool.name)).toEqual(["read", "sessions_spawn"]); + const args = readCreateToolsArgs(1); expect(args.pluginToolDenylist).toEqual(["cron", "gateway", "nodes"]); expect(args.inheritedToolDenylist).toEqual(["cron", "gateway", "nodes"]); });