mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-18 19:04:45 +00:00
* fix: route subagent announce to originating parent session instead of channel-bound peer session When a subagent is spawned from agent:main:main while a Telegram DM is active, the completion announce was delivered to the parallel Telegram channel session instead of the originating parent. Two interacting bugs: 1. The spawn tool received the sandbox/policy session key (Telegram peer key) as the requester, instead of the real run session key. Fixed by passing runSessionKey to createSessionsSpawnTool so the registered requester points to the actual parent session. 2. resolveSubagentCompletionOrigin checked child session bindings before requester bindings. When both share the same channel+accountId (common for Telegram DMs), the child binding hijacked the delivery target. Fixed by checking requester binding first, with child as fallback. Fixes #80201 * fix: drop subagent_announce from mediated completion set The subagent_announce addition to AGENT_MEDIATED_COMPLETION_TOOLS was unrelated to the routing fix and could cause group/channel completions to fail silently when the subagent does not use the message tool. This should be addressed separately with proper message-tool-only guidance (tracked in #80223). * fix: separate sandbox policy from completion owner in sessions_spawn PR #80242 passed runSessionKey as agentSessionKey to createSessionsSpawnTool, which caused spawnSubagentDirect to use the run session key for sandbox policy checks (resolveSandboxRuntimeStatus). This could make a sandboxed channel run appear unsandboxed. Introduce completionOwnerKey as a separate field that is only used for registerSubagentRun routing (requesterSessionKey), keeping agentSessionKey for sandbox enforcement, callerDepth, activeChildren, and all other policy checks. * fix(agents): preserve subagent ownership routing --------- Co-authored-by: 忻役 <xinyi@mininglamp.com> Co-authored-by: Peter Steinberger <steipete@gmail.com>
467 lines
16 KiB
TypeScript
467 lines
16 KiB
TypeScript
import os from "node:os";
|
|
import { beforeAll, beforeEach, describe, expect, it, vi } from "vitest";
|
|
import {
|
|
createSubagentSpawnTestConfig,
|
|
expectPersistedRuntimeModel,
|
|
installSessionStoreCaptureMock,
|
|
loadSubagentSpawnModuleForTest,
|
|
} from "./subagent-spawn.test-helpers.js";
|
|
import { installAcceptedSubagentGatewayMock } from "./test-helpers/subagent-gateway.js";
|
|
|
|
const hoisted = vi.hoisted(() => ({
|
|
callGatewayMock: vi.fn(),
|
|
updateSessionStoreMock: vi.fn(),
|
|
pruneLegacyStoreKeysMock: vi.fn(),
|
|
registerSubagentRunMock: vi.fn(),
|
|
emitSessionLifecycleEventMock: vi.fn(),
|
|
resolveAgentConfigMock: vi.fn(),
|
|
configOverride: {} as Record<string, unknown>,
|
|
}));
|
|
|
|
let resetSubagentRegistryForTests: typeof import("./subagent-registry.js").resetSubagentRegistryForTests;
|
|
let spawnSubagentDirect: typeof import("./subagent-spawn.js").spawnSubagentDirect;
|
|
|
|
function createConfigOverride(overrides?: Record<string, unknown>) {
|
|
return createSubagentSpawnTestConfig(os.tmpdir(), {
|
|
agents: {
|
|
defaults: {
|
|
workspace: os.tmpdir(),
|
|
},
|
|
list: [
|
|
{
|
|
id: "main",
|
|
workspace: "/tmp/workspace-main",
|
|
},
|
|
],
|
|
},
|
|
...overrides,
|
|
});
|
|
}
|
|
|
|
function requireRecord(value: unknown): Record<string, unknown> {
|
|
if (!value || typeof value !== "object" || Array.isArray(value)) {
|
|
throw new Error("Expected a non-array record");
|
|
}
|
|
return value as Record<string, unknown>;
|
|
}
|
|
|
|
function gatewayRequestRecords(): Record<string, unknown>[] {
|
|
return hoisted.callGatewayMock.mock.calls.map((call) => requireRecord(call[0]));
|
|
}
|
|
|
|
function gatewayRequest(method: string): Record<string, unknown> {
|
|
const request = gatewayRequestRecords().find((entry) => entry.method === method);
|
|
return requireRecord(request);
|
|
}
|
|
|
|
function firstRegisteredSubagentRun(): Record<string, unknown> {
|
|
return requireRecord(hoisted.registerSubagentRunMock.mock.calls[0]?.[0]);
|
|
}
|
|
|
|
describe("spawnSubagentDirect seam flow", () => {
|
|
beforeAll(async () => {
|
|
({ resetSubagentRegistryForTests, spawnSubagentDirect } = await loadSubagentSpawnModuleForTest({
|
|
callGatewayMock: hoisted.callGatewayMock,
|
|
getRuntimeConfig: () => hoisted.configOverride,
|
|
updateSessionStoreMock: hoisted.updateSessionStoreMock,
|
|
pruneLegacyStoreKeysMock: hoisted.pruneLegacyStoreKeysMock,
|
|
registerSubagentRunMock: hoisted.registerSubagentRunMock,
|
|
emitSessionLifecycleEventMock: hoisted.emitSessionLifecycleEventMock,
|
|
resolveAgentConfig: hoisted.resolveAgentConfigMock,
|
|
resolveSubagentSpawnModelSelection: () => "openai-codex/gpt-5.4",
|
|
resolveSandboxRuntimeStatus: () => ({ sandboxed: false }),
|
|
sessionStorePath: "/tmp/subagent-spawn-session-store.json",
|
|
resetModules: false,
|
|
}));
|
|
});
|
|
|
|
beforeEach(() => {
|
|
resetSubagentRegistryForTests();
|
|
hoisted.callGatewayMock.mockReset();
|
|
hoisted.updateSessionStoreMock.mockReset();
|
|
hoisted.pruneLegacyStoreKeysMock.mockReset();
|
|
hoisted.registerSubagentRunMock.mockReset();
|
|
hoisted.emitSessionLifecycleEventMock.mockReset();
|
|
hoisted.resolveAgentConfigMock.mockReset();
|
|
hoisted.resolveAgentConfigMock.mockImplementation(
|
|
(cfg: { agents?: { list?: Array<{ id?: string }> } }, agentId: string) =>
|
|
cfg.agents?.list?.find((agent) => agent.id === agentId),
|
|
);
|
|
hoisted.configOverride = createConfigOverride();
|
|
installAcceptedSubagentGatewayMock(hoisted.callGatewayMock);
|
|
|
|
hoisted.updateSessionStoreMock.mockImplementation(
|
|
async (
|
|
_storePath: string,
|
|
mutator: (store: Record<string, Record<string, unknown>>) => unknown,
|
|
) => {
|
|
const store: Record<string, Record<string, unknown>> = {};
|
|
await mutator(store);
|
|
return store;
|
|
},
|
|
);
|
|
});
|
|
|
|
it("rejects explicit same-agent targets when allowAgents excludes the requester", async () => {
|
|
hoisted.configOverride = createConfigOverride({
|
|
agents: {
|
|
defaults: {
|
|
workspace: os.tmpdir(),
|
|
},
|
|
list: [
|
|
{
|
|
id: "task-manager",
|
|
workspace: "/tmp/workspace-task-manager",
|
|
subagents: {
|
|
allowAgents: ["planner"],
|
|
},
|
|
},
|
|
{
|
|
id: "planner",
|
|
workspace: "/tmp/workspace-planner",
|
|
},
|
|
],
|
|
},
|
|
});
|
|
|
|
const result = await spawnSubagentDirect(
|
|
{
|
|
task: "spawn myself explicitly",
|
|
agentId: "task-manager",
|
|
},
|
|
{
|
|
agentSessionKey: "agent:task-manager:main",
|
|
},
|
|
);
|
|
|
|
expect(result.status).toBe("forbidden");
|
|
expect(result.error).toBe("agentId is not allowed for sessions_spawn (allowed: planner)");
|
|
expect(gatewayRequestRecords().some((request) => request.method === "agent")).toBe(false);
|
|
});
|
|
|
|
it("allows omitted agentId to default to requester even when allowAgents excludes requester", async () => {
|
|
hoisted.configOverride = createConfigOverride({
|
|
agents: {
|
|
defaults: {
|
|
workspace: os.tmpdir(),
|
|
},
|
|
list: [
|
|
{
|
|
id: "task-manager",
|
|
workspace: "/tmp/workspace-task-manager",
|
|
subagents: {
|
|
allowAgents: ["planner"],
|
|
},
|
|
},
|
|
{
|
|
id: "planner",
|
|
workspace: "/tmp/workspace-planner",
|
|
},
|
|
],
|
|
},
|
|
});
|
|
|
|
const result = await spawnSubagentDirect(
|
|
{
|
|
task: "spawn default target",
|
|
},
|
|
{
|
|
agentSessionKey: "agent:task-manager:main",
|
|
},
|
|
);
|
|
|
|
expect(result.status).toBe("accepted");
|
|
expect(result.childSessionKey).toMatch(/^agent:task-manager:subagent:/);
|
|
});
|
|
|
|
it("accepts a spawned run across session patching, runtime-model persistence, registry registration, and lifecycle emission", async () => {
|
|
const operations: string[] = [];
|
|
let persistedStore: Record<string, Record<string, unknown>> | undefined;
|
|
|
|
hoisted.callGatewayMock.mockImplementation(async (request: { method?: string }) => {
|
|
operations.push(`gateway:${request.method ?? "unknown"}`);
|
|
if (request.method === "agent") {
|
|
return { runId: "run-1" };
|
|
}
|
|
if (request.method?.startsWith("sessions.")) {
|
|
return { ok: true };
|
|
}
|
|
return {};
|
|
});
|
|
installSessionStoreCaptureMock(hoisted.updateSessionStoreMock, {
|
|
operations,
|
|
onStore: (store) => {
|
|
persistedStore = store;
|
|
},
|
|
});
|
|
|
|
const result = await spawnSubagentDirect(
|
|
{
|
|
task: "inspect the spawn seam",
|
|
model: "openai-codex/gpt-5.4",
|
|
},
|
|
{
|
|
agentSessionKey: "agent:main:main",
|
|
agentChannel: "discord",
|
|
agentAccountId: "acct-1",
|
|
agentTo: "user-1",
|
|
agentThreadId: 42,
|
|
workspaceDir: "/tmp/requester-workspace",
|
|
},
|
|
);
|
|
|
|
expect(result.status).toBe("accepted");
|
|
expect(result.runId).toBe("run-1");
|
|
expect(result.mode).toBe("run");
|
|
expect(result.modelApplied).toBe(true);
|
|
expect(result.childSessionKey).toMatch(/^agent:main:subagent:/);
|
|
|
|
const childSessionKey = result.childSessionKey as string;
|
|
expect(hoisted.pruneLegacyStoreKeysMock).toHaveBeenCalledTimes(3);
|
|
expect(hoisted.updateSessionStoreMock).toHaveBeenCalledTimes(3);
|
|
const registerInput = firstRegisteredSubagentRun();
|
|
const requesterOrigin = requireRecord(registerInput.requesterOrigin);
|
|
expect(registerInput.runId).toBe("run-1");
|
|
expect(registerInput.childSessionKey).toBe(childSessionKey);
|
|
expect(registerInput.requesterSessionKey).toBe("agent:main:main");
|
|
expect(registerInput.requesterDisplayKey).toBe("agent:main:main");
|
|
expect(requesterOrigin.channel).toBe("discord");
|
|
expect(requesterOrigin.accountId).toBe("acct-1");
|
|
expect(requesterOrigin.to).toBe("user-1");
|
|
expect(requesterOrigin.threadId).toBe(42);
|
|
expect(registerInput.task).toBe("inspect the spawn seam");
|
|
expect(registerInput.cleanup).toBe("keep");
|
|
expect(registerInput.model).toBe("openai-codex/gpt-5.4");
|
|
expect(registerInput.workspaceDir).toBe("/tmp/requester-workspace");
|
|
expect(registerInput.expectsCompletionMessage).toBe(true);
|
|
expect(registerInput.spawnMode).toBe("run");
|
|
expect(hoisted.emitSessionLifecycleEventMock).toHaveBeenCalledWith({
|
|
sessionKey: childSessionKey,
|
|
reason: "create",
|
|
parentSessionKey: "agent:main:main",
|
|
label: undefined,
|
|
});
|
|
|
|
expectPersistedRuntimeModel({
|
|
persistedStore,
|
|
sessionKey: childSessionKey,
|
|
provider: "openai-codex",
|
|
model: "gpt-5.4",
|
|
overrideSource: "user",
|
|
});
|
|
expect(operations.indexOf("store:update")).toBeGreaterThan(-1);
|
|
expect(operations.indexOf("gateway:agent")).toBeGreaterThan(
|
|
operations.lastIndexOf("store:update"),
|
|
);
|
|
const agentRequest = gatewayRequest("agent");
|
|
const agentParams = requireRecord(agentRequest.params);
|
|
expect(agentParams.sessionKey).toBe(childSessionKey);
|
|
expect(agentParams.cleanupBundleMcpOnRunEnd).toBe(true);
|
|
});
|
|
|
|
it("keeps controller ownership separate from completion ownership", async () => {
|
|
await spawnSubagentDirect(
|
|
{
|
|
task: "background work",
|
|
},
|
|
{
|
|
agentSessionKey: "agent:main:telegram:default:direct:456",
|
|
completionOwnerKey: "agent:main:main",
|
|
agentChannel: "telegram",
|
|
agentAccountId: "default",
|
|
agentTo: "telegram:direct:456",
|
|
},
|
|
);
|
|
|
|
const registerInput = firstRegisteredSubagentRun();
|
|
expect(registerInput.controllerSessionKey).toBe("agent:main:telegram:default:direct:456");
|
|
expect(registerInput.requesterSessionKey).toBe("agent:main:main");
|
|
expect(registerInput.requesterDisplayKey).toBe("agent:main:main");
|
|
});
|
|
|
|
it("omits requesterOrigin threadId when no requester thread is provided", async () => {
|
|
hoisted.callGatewayMock.mockImplementation(async (request: { method?: string }) => {
|
|
if (request.method === "agent") {
|
|
return { runId: "run-1" };
|
|
}
|
|
if (request.method?.startsWith("sessions.")) {
|
|
return { ok: true };
|
|
}
|
|
return {};
|
|
});
|
|
installSessionStoreCaptureMock(hoisted.updateSessionStoreMock);
|
|
|
|
const result = await spawnSubagentDirect(
|
|
{
|
|
task: "inspect unthreaded spawn",
|
|
model: "openai-codex/gpt-5.4",
|
|
},
|
|
{
|
|
agentSessionKey: "agent:main:main",
|
|
agentChannel: "discord",
|
|
agentAccountId: "acct-1",
|
|
agentTo: "user-1",
|
|
},
|
|
);
|
|
|
|
expect(result.status).toBe("accepted");
|
|
const registerInput = firstRegisteredSubagentRun();
|
|
const requesterOrigin = requireRecord(registerInput.requesterOrigin);
|
|
expect(requesterOrigin.channel).toBe("discord");
|
|
expect(requesterOrigin.accountId).toBe("acct-1");
|
|
expect(requesterOrigin.to).toBe("user-1");
|
|
expect(requesterOrigin).not.toHaveProperty("threadId");
|
|
});
|
|
|
|
it("pins admin-only methods to operator.admin and preserves least-privilege for others (#59428)", async () => {
|
|
const capturedCalls: Array<{ method?: string; scopes?: string[] }> = [];
|
|
|
|
hoisted.callGatewayMock.mockImplementation(
|
|
async (request: { method?: string; scopes?: string[] }) => {
|
|
capturedCalls.push({ method: request.method, scopes: request.scopes });
|
|
if (request.method === "agent") {
|
|
return { runId: "run-1" };
|
|
}
|
|
if (request.method?.startsWith("sessions.")) {
|
|
return { ok: true };
|
|
}
|
|
return {};
|
|
},
|
|
);
|
|
installSessionStoreCaptureMock(hoisted.updateSessionStoreMock);
|
|
|
|
const result = await spawnSubagentDirect(
|
|
{
|
|
task: "verify per-method scope routing",
|
|
model: "openai-codex/gpt-5.4",
|
|
},
|
|
{
|
|
agentSessionKey: "agent:main:main",
|
|
agentChannel: "discord",
|
|
agentAccountId: "acct-1",
|
|
agentTo: "user-1",
|
|
workspaceDir: "/tmp/requester-workspace",
|
|
},
|
|
);
|
|
|
|
expect(result.status).toBe("accepted");
|
|
expect(capturedCalls.length).toBeGreaterThan(0);
|
|
|
|
for (const call of capturedCalls) {
|
|
if (call.method === "sessions.patch" || call.method === "sessions.delete") {
|
|
// Admin-only methods must be pinned to operator.admin.
|
|
expect(call.scopes).toEqual(["operator.admin"]);
|
|
} else {
|
|
// Non-admin methods (e.g. "agent") must NOT be forced to admin scope
|
|
// so the gateway preserves least-privilege and senderIsOwner stays false.
|
|
expect(call.scopes).toBeUndefined();
|
|
}
|
|
}
|
|
});
|
|
|
|
it("forwards normalized thinking to the agent run", async () => {
|
|
const calls: Array<{ method?: string; params?: unknown }> = [];
|
|
hoisted.callGatewayMock.mockImplementation(
|
|
async (request: { method?: string; params?: unknown }) => {
|
|
calls.push(request);
|
|
if (request.method === "agent") {
|
|
return { runId: "run-thinking", status: "accepted", acceptedAt: 1000 };
|
|
}
|
|
if (request.method?.startsWith("sessions.")) {
|
|
return { ok: true };
|
|
}
|
|
return {};
|
|
},
|
|
);
|
|
installSessionStoreCaptureMock(hoisted.updateSessionStoreMock);
|
|
|
|
const result = await spawnSubagentDirect(
|
|
{
|
|
task: "verify thinking forwarding",
|
|
thinking: "high",
|
|
},
|
|
{
|
|
agentSessionKey: "agent:main:main",
|
|
agentChannel: "discord",
|
|
},
|
|
);
|
|
|
|
expect(result.status).toBe("accepted");
|
|
const agentCall = calls.find((call) => call.method === "agent");
|
|
const params = requireRecord(agentCall?.params);
|
|
expect(params.thinking).toBe("high");
|
|
});
|
|
|
|
it("does not duplicate long subagent task text in the initial user message (#72019)", async () => {
|
|
const calls: Array<{ method?: string; params?: unknown }> = [];
|
|
hoisted.callGatewayMock.mockImplementation(
|
|
async (request: { method?: string; params?: unknown }) => {
|
|
calls.push(request);
|
|
if (request.method === "agent") {
|
|
return { runId: "run-no-dup", status: "accepted", acceptedAt: 1000 };
|
|
}
|
|
if (request.method?.startsWith("sessions.")) {
|
|
return { ok: true };
|
|
}
|
|
return {};
|
|
},
|
|
);
|
|
installSessionStoreCaptureMock(hoisted.updateSessionStoreMock);
|
|
|
|
const task = "UNIQUE_LONG_SUBAGENT_TASK_TOKEN\n keep indentation";
|
|
const result = await spawnSubagentDirect(
|
|
{
|
|
task,
|
|
},
|
|
{
|
|
agentSessionKey: "agent:main:main",
|
|
agentChannel: "discord",
|
|
},
|
|
);
|
|
|
|
expect(result.status).toBe("accepted");
|
|
const agentCall = calls.find((call) => call.method === "agent");
|
|
const params = agentCall?.params as { message?: string; extraSystemPrompt?: string };
|
|
expect(params.message).toContain("[Subagent Task]");
|
|
expect(params.message).toContain("UNIQUE_LONG_SUBAGENT_TASK_TOKEN");
|
|
expect(params.message).toContain(" keep indentation");
|
|
expect(params.message).not.toContain("**Your Role**");
|
|
expect(params.extraSystemPrompt).toBe("system-prompt");
|
|
});
|
|
|
|
it("returns an error when the initial child session patch is rejected", async () => {
|
|
hoisted.callGatewayMock.mockImplementation(
|
|
async (request: { method?: string; params?: unknown }) => {
|
|
if (request.method === "agent") {
|
|
return { runId: "run-1", status: "accepted", acceptedAt: 1000 };
|
|
}
|
|
if (request.method === "sessions.delete") {
|
|
return { ok: true };
|
|
}
|
|
return {};
|
|
},
|
|
);
|
|
hoisted.updateSessionStoreMock.mockRejectedValueOnce(new Error("invalid model: bad-model"));
|
|
|
|
const result = await spawnSubagentDirect(
|
|
{
|
|
task: "verify patch rejection",
|
|
model: "bad-model",
|
|
},
|
|
{
|
|
agentSessionKey: "agent:main:main",
|
|
agentChannel: "discord",
|
|
},
|
|
);
|
|
|
|
expect(result.status).toBe("error");
|
|
expect(result.childSessionKey).toMatch(/^agent:main:subagent:/);
|
|
expect(result.error ?? "").toContain("invalid model");
|
|
expect(
|
|
hoisted.callGatewayMock.mock.calls.some(
|
|
(call) => (call[0] as { method?: string }).method === "agent",
|
|
),
|
|
).toBe(false);
|
|
});
|
|
});
|