From c1a42dce86cba23fb7ceb5994a83ddf82bdd754e Mon Sep 17 00:00:00 2001 From: Devin Robison Date: Wed, 29 Apr 2026 09:54:09 -0600 Subject: [PATCH] fix: enforce focus subagent scope (#73613) * fix: enforce focus subagent scope * docs: add changelog for focus scope fix --- CHANGELOG.md | 1 + .../reply/commands-subagents-focus.test.ts | 40 ++++++++++++++++++- .../commands-subagents-shared-focus.test.ts | 37 +++++++++++++++++ .../reply/commands-subagents/action-focus.ts | 18 ++++++++- .../reply/commands-subagents/shared.ts | 5 ++- 5 files changed, 97 insertions(+), 4 deletions(-) create mode 100644 src/auto-reply/reply/commands-subagents-shared-focus.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 1751858e2a7..05affa66a3b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -189,6 +189,7 @@ Docs: https://docs.openclaw.ai - Telegram/gateway: bound outbound Bot API calls and cache bundled plugin alias lookup so slow Telegram sends or WSL2 filesystem scans no longer wedge gateway replies. (#74210) Thanks @obviyus. - Configure/GitHub Copilot: reuse existing Copilot auth during configure and show the provider's manifest model catalog in the model picker. (#74276) Thanks @obviyus. - Configure/models: keep the model picker scoped to the selected manifest provider and enable its bundled plugin before catalog lookup, so choosing GitHub Copilot no longer falls back to Ollama or skips the catalog. (#74322) Thanks @obviyus. +- Auto-reply/subagents: reject `/focus` from leaf subagents and scope fallback target resolution to the requesting subagent's children, so subagents cannot bind conversations outside their control boundary. (#73613) Thanks @drobison00. ## 2026.4.27 diff --git a/src/auto-reply/reply/commands-subagents-focus.test.ts b/src/auto-reply/reply/commands-subagents-focus.test.ts index 0fd7f7c3672..4ec4df6230a 100644 --- a/src/auto-reply/reply/commands-subagents-focus.test.ts +++ b/src/auto-reply/reply/commands-subagents-focus.test.ts @@ -15,6 +15,7 @@ const hoisted = vi.hoisted(() => ({ readAcpSessionEntryMock: vi.fn(), resolveConversationBindingContextMock: vi.fn(), resolveFocusTargetSessionMock: vi.fn(), + resolveStoredSubagentCapabilitiesMock: vi.fn(), sessionBindingCapabilitiesMock: vi.fn(), sessionBindingBindMock: vi.fn(), sessionBindingResolveByConversationMock: vi.fn(), @@ -102,6 +103,11 @@ vi.mock("../../infra/outbound/session-binding-service.js", () => ({ getSessionBindingService: () => buildFocusSessionBindingService(), })); +vi.mock("../../agents/subagent-capabilities.js", () => ({ + resolveStoredSubagentCapabilities: (sessionKey: string, options: unknown) => + hoisted.resolveStoredSubagentCapabilitiesMock(sessionKey, options), +})); + vi.mock("./conversation-binding-input.js", () => ({ resolveConversationBindingContextFromAcpCommand: (params: unknown) => hoisted.resolveConversationBindingContextMock(params), @@ -195,6 +201,7 @@ function buildFocusContext(params?: { chatType?: string; senderId?: string; token?: string; + requesterKey?: string; }) { return { params: buildCommandParams({ @@ -203,7 +210,7 @@ function buildFocusContext(params?: { senderId: params?.senderId, }), handledPrefix: "/focus", - requesterKey: "agent:main:main", + requesterKey: params?.requesterKey ?? "agent:main:main", runs: [], restTokens: [params?.token ?? "codex-acp"], } satisfies Parameters[0]; @@ -224,6 +231,9 @@ function buildUnfocusContext(params?: { senderId?: string }) { describe("focus actions", () => { beforeEach(() => { vi.clearAllMocks(); + hoisted.resolveStoredSubagentCapabilitiesMock.mockReturnValue({ + controlScope: "children", + }); hoisted.sessionBindingCapabilitiesMock.mockReturnValue(createSessionBindingCapabilities()); hoisted.sessionBindingResolveByConversationMock.mockReturnValue(null); hoisted.resolveFocusTargetSessionMock.mockResolvedValue({ @@ -278,6 +288,11 @@ describe("focus actions", () => { expect(result.reply?.text).toContain("bound this conversation"); expect(result.reply?.text).toContain("(acp)"); + expect(hoisted.resolveFocusTargetSessionMock).toHaveBeenCalledWith( + expect.objectContaining({ + requesterKey: "agent:main:main", + }), + ); expect(hoisted.sessionBindingBindMock).toHaveBeenCalledWith( expect.objectContaining({ placement: "current", @@ -291,6 +306,29 @@ describe("focus actions", () => { ); }); + it("rejects /focus from a leaf subagent", async () => { + hoisted.resolveStoredSubagentCapabilitiesMock.mockReturnValue({ + controlScope: "none", + }); + hoisted.resolveConversationBindingContextMock.mockReturnValue({ + channel: THREAD_CHANNEL, + accountId: "default", + conversationId: "thread-1", + parentConversationId: "parent-1", + threadId: "thread-1", + }); + + const result = await handleSubagentsFocusAction( + buildFocusContext({ + requesterKey: "agent:main:subagent:leaf-a", + }), + ); + + expect(result.reply?.text).toContain("Leaf subagents cannot control other sessions."); + expect(hoisted.resolveFocusTargetSessionMock).not.toHaveBeenCalled(); + expect(hoisted.sessionBindingBindMock).not.toHaveBeenCalled(); + }); + it("binds topic-chat topics as current conversations", async () => { hoisted.resolveConversationBindingContextMock.mockReturnValue({ channel: TOPIC_CHANNEL, diff --git a/src/auto-reply/reply/commands-subagents-shared-focus.test.ts b/src/auto-reply/reply/commands-subagents-shared-focus.test.ts new file mode 100644 index 00000000000..7ddc164334c --- /dev/null +++ b/src/auto-reply/reply/commands-subagents-shared-focus.test.ts @@ -0,0 +1,37 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { resolveFocusTargetSession } from "./commands-subagents/shared.js"; + +const hoisted = vi.hoisted(() => ({ + callGatewayMock: vi.fn(), +})); + +vi.mock("../../gateway/call.js", () => ({ + callGateway: (params: unknown) => hoisted.callGatewayMock(params), +})); + +describe("resolveFocusTargetSession", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it("restricts gateway fallback resolution to a subagent requester's children", async () => { + hoisted.callGatewayMock.mockResolvedValue({ + key: "agent:main:subagent:child", + }); + + const result = await resolveFocusTargetSession({ + runs: [], + token: "child", + requesterKey: "agent:main:subagent:parent", + }); + + expect(result?.targetSessionKey).toBe("agent:main:subagent:child"); + expect(hoisted.callGatewayMock).toHaveBeenCalledWith({ + method: "sessions.resolve", + params: { + key: "child", + spawnedBy: "agent:main:subagent:parent", + }, + }); + }); +}); diff --git a/src/auto-reply/reply/commands-subagents/action-focus.ts b/src/auto-reply/reply/commands-subagents/action-focus.ts index 883d7af182f..528d27d0d21 100644 --- a/src/auto-reply/reply/commands-subagents/action-focus.ts +++ b/src/auto-reply/reply/commands-subagents/action-focus.ts @@ -21,7 +21,12 @@ import { getSessionBindingService } from "../../../infra/outbound/session-bindin import { normalizeOptionalString } from "../../../shared/string-coerce.js"; import type { CommandHandlerResult } from "../commands-types.js"; import { resolveConversationBindingContextFromAcpCommand } from "../conversation-binding-input.js"; -import { type SubagentsCommandContext, resolveFocusTargetSession, stopWithText } from "./shared.js"; +import { + type SubagentsCommandContext, + resolveCommandSubagentController, + resolveFocusTargetSession, + stopWithText, +} from "./shared.js"; type FocusBindingContext = { channel: string; @@ -71,6 +76,11 @@ export async function handleSubagentsFocusAction( return stopWithText("Usage: /focus "); } + const controller = resolveCommandSubagentController(params, ctx.requesterKey); + if (controller.controlScope !== "children") { + return stopWithText("⚠️ Leaf subagents cannot control other sessions."); + } + const bindingContext = resolveFocusBindingContext(params); if (!bindingContext) { return stopWithText("⚠️ /focus must be run inside a bindable conversation."); @@ -85,7 +95,11 @@ export async function handleSubagentsFocusAction( return stopWithText("⚠️ Conversation bindings are unavailable for this account."); } - const focusTarget = await resolveFocusTargetSession({ runs, token }); + const focusTarget = await resolveFocusTargetSession({ + runs, + token, + requesterKey: controller.controllerSessionKey, + }); if (!focusTarget) { return stopWithText(`⚠️ Unable to resolve focus target: ${token}`); } diff --git a/src/auto-reply/reply/commands-subagents/shared.ts b/src/auto-reply/reply/commands-subagents/shared.ts index 0fc0c0ced5f..64a687e4f2c 100644 --- a/src/auto-reply/reply/commands-subagents/shared.ts +++ b/src/auto-reply/reply/commands-subagents/shared.ts @@ -307,6 +307,7 @@ export type FocusTargetResolution = { export async function resolveFocusTargetSession(params: { runs: SubagentRunRecord[]; token: string; + requesterKey?: string; }): Promise { const subagentMatch = resolveSubagentTarget(params.runs, params.token); if (subagentMatch.entry) { @@ -326,6 +327,8 @@ export async function resolveFocusTargetSession(params: { } const attempts: Array> = []; + const requesterKey = normalizeOptionalString(params.requesterKey); + const spawnedBy = requesterKey && isSubagentSessionKey(requesterKey) ? requesterKey : undefined; attempts.push({ key: token }); if (looksLikeSessionId(token)) { attempts.push({ sessionId: token }); @@ -336,7 +339,7 @@ export async function resolveFocusTargetSession(params: { try { const resolved = await callGateway({ method: "sessions.resolve", - params: attempt, + params: spawnedBy ? { ...attempt, spawnedBy } : attempt, }); const key = normalizeOptionalString(resolved?.key) ?? ""; if (!key) {