mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-06 12:40:43 +00:00
fix: enforce focus subagent scope (#73613)
* fix: enforce focus subagent scope * docs: add changelog for focus scope fix
This commit is contained in:
@@ -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
|
||||
|
||||
|
||||
@@ -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<typeof handleSubagentsFocusAction>[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,
|
||||
|
||||
37
src/auto-reply/reply/commands-subagents-shared-focus.test.ts
Normal file
37
src/auto-reply/reply/commands-subagents-shared-focus.test.ts
Normal file
@@ -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",
|
||||
},
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -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 <subagent-label|session-key|session-id|session-label>");
|
||||
}
|
||||
|
||||
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}`);
|
||||
}
|
||||
|
||||
@@ -307,6 +307,7 @@ export type FocusTargetResolution = {
|
||||
export async function resolveFocusTargetSession(params: {
|
||||
runs: SubagentRunRecord[];
|
||||
token: string;
|
||||
requesterKey?: string;
|
||||
}): Promise<FocusTargetResolution | null> {
|
||||
const subagentMatch = resolveSubagentTarget(params.runs, params.token);
|
||||
if (subagentMatch.entry) {
|
||||
@@ -326,6 +327,8 @@ export async function resolveFocusTargetSession(params: {
|
||||
}
|
||||
|
||||
const attempts: Array<Record<string, string>> = [];
|
||||
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) {
|
||||
|
||||
Reference in New Issue
Block a user