fix: enforce group tool policy inheritance for subagents (#1557) (thanks @adam91holt)
This commit is contained in:
@@ -31,6 +31,12 @@ export function createClawdbotTools(options?: {
|
||||
agentTo?: string;
|
||||
/** Thread/topic identifier for routing replies to the originating thread. */
|
||||
agentThreadId?: string | number;
|
||||
/** Group id for channel-level tool policy inheritance. */
|
||||
agentGroupId?: string | null;
|
||||
/** Group channel label for channel-level tool policy inheritance. */
|
||||
agentGroupChannel?: string | null;
|
||||
/** Group space label for channel-level tool policy inheritance. */
|
||||
agentGroupSpace?: string | null;
|
||||
agentDir?: string;
|
||||
sandboxRoot?: string;
|
||||
workspaceDir?: string;
|
||||
@@ -114,6 +120,9 @@ export function createClawdbotTools(options?: {
|
||||
agentAccountId: options?.agentAccountId,
|
||||
agentTo: options?.agentTo,
|
||||
agentThreadId: options?.agentThreadId,
|
||||
agentGroupId: options?.agentGroupId,
|
||||
agentGroupChannel: options?.agentGroupChannel,
|
||||
agentGroupSpace: options?.agentGroupSpace,
|
||||
sandboxed: options?.sandboxed,
|
||||
}),
|
||||
createSessionStatusTool({
|
||||
|
||||
@@ -73,6 +73,14 @@ export async function compactEmbeddedPiSession(params: {
|
||||
messageChannel?: string;
|
||||
messageProvider?: string;
|
||||
agentAccountId?: string;
|
||||
/** Group id for channel-level tool policy resolution. */
|
||||
groupId?: string | null;
|
||||
/** Group channel label (e.g. #general) for channel-level tool policy resolution. */
|
||||
groupChannel?: string | null;
|
||||
/** Group space label (e.g. guild/team id) for channel-level tool policy resolution. */
|
||||
groupSpace?: string | null;
|
||||
/** Parent session key for subagent policy inheritance. */
|
||||
spawnedBy?: string | null;
|
||||
sessionFile: string;
|
||||
workspaceDir: string;
|
||||
agentDir?: string;
|
||||
@@ -207,6 +215,10 @@ export async function compactEmbeddedPiSession(params: {
|
||||
messageProvider: params.messageChannel ?? params.messageProvider,
|
||||
agentAccountId: params.agentAccountId,
|
||||
sessionKey: params.sessionKey ?? params.sessionId,
|
||||
groupId: params.groupId,
|
||||
groupChannel: params.groupChannel,
|
||||
groupSpace: params.groupSpace,
|
||||
spawnedBy: params.spawnedBy,
|
||||
agentDir,
|
||||
workspaceDir: effectiveWorkspace,
|
||||
config: params.config,
|
||||
|
||||
@@ -267,6 +267,7 @@ export async function runEmbeddedPiAgent(
|
||||
groupId: params.groupId,
|
||||
groupChannel: params.groupChannel,
|
||||
groupSpace: params.groupSpace,
|
||||
spawnedBy: params.spawnedBy,
|
||||
currentChannelId: params.currentChannelId,
|
||||
currentThreadTs: params.currentThreadTs,
|
||||
replyToMode: params.replyToMode,
|
||||
|
||||
@@ -211,6 +211,7 @@ export async function runEmbeddedAttempt(
|
||||
groupId: params.groupId,
|
||||
groupChannel: params.groupChannel,
|
||||
groupSpace: params.groupSpace,
|
||||
spawnedBy: params.spawnedBy,
|
||||
sessionKey: params.sessionKey ?? params.sessionId,
|
||||
agentDir,
|
||||
workspaceDir: effectiveWorkspace,
|
||||
|
||||
@@ -33,6 +33,8 @@ export type RunEmbeddedPiAgentParams = {
|
||||
groupChannel?: string | null;
|
||||
/** Group space label (e.g. guild/team id) for channel-level tool policy resolution. */
|
||||
groupSpace?: string | null;
|
||||
/** Parent session key for subagent policy inheritance. */
|
||||
spawnedBy?: string | null;
|
||||
/** Current channel ID for auto-threading (Slack). */
|
||||
currentChannelId?: string;
|
||||
/** Current thread timestamp for auto-threading (Slack). */
|
||||
|
||||
@@ -29,6 +29,8 @@ export type EmbeddedRunAttemptParams = {
|
||||
groupChannel?: string | null;
|
||||
/** Group space label (e.g. guild/team id) for channel-level tool policy resolution. */
|
||||
groupSpace?: string | null;
|
||||
/** Parent session key for subagent policy inheritance. */
|
||||
spawnedBy?: string | null;
|
||||
currentChannelId?: string;
|
||||
currentThreadTs?: string;
|
||||
replyToMode?: "off" | "first" | "all";
|
||||
|
||||
@@ -295,6 +295,31 @@ describe("Agent-specific tool filtering", () => {
|
||||
expect(names).not.toContain("exec");
|
||||
});
|
||||
|
||||
it("should inherit group tool policy for subagents from spawnedBy session keys", () => {
|
||||
const cfg: ClawdbotConfig = {
|
||||
channels: {
|
||||
whatsapp: {
|
||||
groups: {
|
||||
trusted: {
|
||||
tools: { allow: ["read"] },
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
};
|
||||
|
||||
const tools = createClawdbotCodingTools({
|
||||
config: cfg,
|
||||
sessionKey: "agent:main:subagent:test",
|
||||
spawnedBy: "agent:main:whatsapp:group:trusted",
|
||||
workspaceDir: "/tmp/test-subagent-group",
|
||||
agentDir: "/tmp/agent-subagent",
|
||||
});
|
||||
const names = tools.map((t) => t.name);
|
||||
expect(names).toContain("read");
|
||||
expect(names).not.toContain("exec");
|
||||
});
|
||||
|
||||
it("should apply global tool policy before agent-specific policy", () => {
|
||||
const cfg: ClawdbotConfig = {
|
||||
tools: {
|
||||
|
||||
@@ -120,7 +120,10 @@ function resolveGroupContextFromSessionKey(sessionKey?: string | null): {
|
||||
if (!raw) return {};
|
||||
const base = resolveThreadParentSessionKey(raw) ?? raw;
|
||||
const parts = base.split(":").filter(Boolean);
|
||||
const body = parts[0] === "agent" ? parts.slice(2) : parts;
|
||||
let body = parts[0] === "agent" ? parts.slice(2) : parts;
|
||||
if (body[0] === "subagent") {
|
||||
body = body.slice(1);
|
||||
}
|
||||
if (body.length < 3) return {};
|
||||
const [channel, kind, ...rest] = body;
|
||||
if (kind !== "group" && kind !== "channel") return {};
|
||||
@@ -198,6 +201,7 @@ export function resolveEffectiveToolPolicy(params: {
|
||||
export function resolveGroupToolPolicy(params: {
|
||||
config?: ClawdbotConfig;
|
||||
sessionKey?: string;
|
||||
spawnedBy?: string | null;
|
||||
messageProvider?: string;
|
||||
groupId?: string | null;
|
||||
groupChannel?: string | null;
|
||||
@@ -206,9 +210,10 @@ export function resolveGroupToolPolicy(params: {
|
||||
}): SandboxToolPolicy | undefined {
|
||||
if (!params.config) return undefined;
|
||||
const sessionContext = resolveGroupContextFromSessionKey(params.sessionKey);
|
||||
const groupId = params.groupId ?? sessionContext.groupId;
|
||||
const spawnedContext = resolveGroupContextFromSessionKey(params.spawnedBy);
|
||||
const groupId = params.groupId ?? sessionContext.groupId ?? spawnedContext.groupId;
|
||||
if (!groupId) return undefined;
|
||||
const channelRaw = params.messageProvider ?? sessionContext.channel;
|
||||
const channelRaw = params.messageProvider ?? sessionContext.channel ?? spawnedContext.channel;
|
||||
const channel = normalizeMessageChannel(channelRaw);
|
||||
if (!channel) return undefined;
|
||||
let dock;
|
||||
|
||||
@@ -135,6 +135,8 @@ export function createClawdbotCodingTools(options?: {
|
||||
groupChannel?: string | null;
|
||||
/** Group space label (e.g. guild/team id) for channel-level tool policy resolution. */
|
||||
groupSpace?: string | null;
|
||||
/** Parent session key for subagent group policy inheritance. */
|
||||
spawnedBy?: string | null;
|
||||
/** Reply-to mode for Slack auto-threading. */
|
||||
replyToMode?: "off" | "first" | "all";
|
||||
/** Mutable ref to track if a reply was sent (for "first" mode). */
|
||||
@@ -161,6 +163,7 @@ export function createClawdbotCodingTools(options?: {
|
||||
const groupPolicy = resolveGroupToolPolicy({
|
||||
config: options?.config,
|
||||
sessionKey: options?.sessionKey,
|
||||
spawnedBy: options?.spawnedBy,
|
||||
messageProvider: options?.messageProvider,
|
||||
groupId: options?.groupId,
|
||||
groupChannel: options?.groupChannel,
|
||||
@@ -290,6 +293,9 @@ export function createClawdbotCodingTools(options?: {
|
||||
agentAccountId: options?.agentAccountId,
|
||||
agentTo: options?.messageTo,
|
||||
agentThreadId: options?.messageThreadId,
|
||||
agentGroupId: options?.groupId ?? null,
|
||||
agentGroupChannel: options?.groupChannel ?? null,
|
||||
agentGroupSpace: options?.groupSpace ?? null,
|
||||
agentDir: options?.agentDir,
|
||||
sandboxRoot,
|
||||
workspaceDir: options?.workspaceDir,
|
||||
|
||||
@@ -63,6 +63,9 @@ export function createSessionsSpawnTool(opts?: {
|
||||
agentAccountId?: string;
|
||||
agentTo?: string;
|
||||
agentThreadId?: string | number;
|
||||
agentGroupId?: string | null;
|
||||
agentGroupChannel?: string | null;
|
||||
agentGroupSpace?: string | null;
|
||||
sandboxed?: boolean;
|
||||
}): AnyAgentTool {
|
||||
return {
|
||||
@@ -153,7 +156,7 @@ export function createSessionsSpawnTool(opts?: {
|
||||
}
|
||||
}
|
||||
const childSessionKey = `agent:${targetAgentId}:subagent:${crypto.randomUUID()}`;
|
||||
const shouldPatchSpawnedBy = opts?.sandboxed === true;
|
||||
const spawnedByKey = requesterInternalKey;
|
||||
const targetAgentConfig = resolveAgentConfig(cfg, targetAgentId);
|
||||
const resolvedModel =
|
||||
normalizeModelSelection(modelOverride) ??
|
||||
@@ -219,7 +222,10 @@ export function createSessionsSpawnTool(opts?: {
|
||||
thinking: thinkingOverride,
|
||||
timeout: runTimeoutSeconds > 0 ? runTimeoutSeconds : undefined,
|
||||
label: label || undefined,
|
||||
spawnedBy: shouldPatchSpawnedBy ? requesterInternalKey : undefined,
|
||||
spawnedBy: spawnedByKey,
|
||||
groupId: opts?.agentGroupId ?? undefined,
|
||||
groupChannel: opts?.agentGroupChannel ?? undefined,
|
||||
groupSpace: opts?.agentGroupSpace ?? undefined,
|
||||
},
|
||||
timeoutMs: 10_000,
|
||||
})) as { runId?: string };
|
||||
|
||||
@@ -67,6 +67,10 @@ export const handleCompactCommand: CommandHandler = async (params) => {
|
||||
sessionId,
|
||||
sessionKey: params.sessionKey,
|
||||
messageChannel: params.command.channel,
|
||||
groupId: params.sessionEntry.groupId,
|
||||
groupChannel: params.sessionEntry.groupChannel,
|
||||
groupSpace: params.sessionEntry.space,
|
||||
spawnedBy: params.sessionEntry.spawnedBy,
|
||||
sessionFile: resolveSessionFilePath(sessionId, params.sessionEntry),
|
||||
workspaceDir: params.workspaceDir,
|
||||
config: params.cfg,
|
||||
|
||||
@@ -81,6 +81,10 @@ async function resolveContextReport(
|
||||
workspaceDir,
|
||||
sessionKey: params.sessionKey,
|
||||
messageProvider: params.command.channel,
|
||||
groupId: params.sessionEntry?.groupId ?? undefined,
|
||||
groupChannel: params.sessionEntry?.groupChannel ?? undefined,
|
||||
groupSpace: params.sessionEntry?.space ?? undefined,
|
||||
spawnedBy: params.sessionEntry?.spawnedBy ?? undefined,
|
||||
modelProvider: params.provider,
|
||||
modelId: params.model,
|
||||
});
|
||||
|
||||
@@ -377,6 +377,7 @@ export async function agentCommand(
|
||||
runContext.messageChannel,
|
||||
opts.replyChannel ?? opts.channel,
|
||||
);
|
||||
const spawnedBy = opts.spawnedBy ?? sessionEntry?.spawnedBy;
|
||||
const fallbackResult = await runWithModelFallback({
|
||||
cfg,
|
||||
provider,
|
||||
@@ -412,6 +413,10 @@ export async function agentCommand(
|
||||
agentAccountId: runContext.accountId,
|
||||
messageTo: opts.replyTo ?? opts.to,
|
||||
messageThreadId: opts.threadId,
|
||||
groupId: runContext.groupId,
|
||||
groupChannel: runContext.groupChannel,
|
||||
groupSpace: runContext.groupSpace,
|
||||
spawnedBy,
|
||||
currentChannelId: runContext.currentChannelId,
|
||||
currentThreadTs: runContext.currentThreadTs,
|
||||
replyToMode: runContext.replyToMode,
|
||||
|
||||
@@ -14,6 +14,15 @@ export function resolveAgentRunContext(opts: AgentCommandOpts): AgentRunContext
|
||||
const normalizedAccountId = normalizeAccountId(merged.accountId ?? opts.accountId);
|
||||
if (normalizedAccountId) merged.accountId = normalizedAccountId;
|
||||
|
||||
const groupId = (merged.groupId ?? opts.groupId)?.toString().trim();
|
||||
if (groupId) merged.groupId = groupId;
|
||||
|
||||
const groupChannel = (merged.groupChannel ?? opts.groupChannel)?.toString().trim();
|
||||
if (groupChannel) merged.groupChannel = groupChannel;
|
||||
|
||||
const groupSpace = (merged.groupSpace ?? opts.groupSpace)?.toString().trim();
|
||||
if (groupSpace) merged.groupSpace = groupSpace;
|
||||
|
||||
if (
|
||||
merged.currentThreadTs == null &&
|
||||
opts.threadId != null &&
|
||||
|
||||
@@ -17,6 +17,9 @@ export type AgentStreamParams = {
|
||||
export type AgentRunContext = {
|
||||
messageChannel?: string;
|
||||
accountId?: string;
|
||||
groupId?: string | null;
|
||||
groupChannel?: string | null;
|
||||
groupSpace?: string | null;
|
||||
currentChannelId?: string;
|
||||
currentThreadTs?: string;
|
||||
replyToMode?: "off" | "first" | "all";
|
||||
@@ -55,6 +58,14 @@ export type AgentCommandOpts = {
|
||||
accountId?: string;
|
||||
/** Context for embedded run routing (channel/account/thread). */
|
||||
runContext?: AgentRunContext;
|
||||
/** Group id for channel-level tool policy resolution. */
|
||||
groupId?: string | null;
|
||||
/** Group channel label for channel-level tool policy resolution. */
|
||||
groupChannel?: string | null;
|
||||
/** Group space label for channel-level tool policy resolution. */
|
||||
groupSpace?: string | null;
|
||||
/** Parent session key for subagent policy inheritance. */
|
||||
spawnedBy?: string | null;
|
||||
deliveryTargetMode?: ChannelOutboundTargetMode;
|
||||
bestEffortDeliver?: boolean;
|
||||
abortSignal?: AbortSignal;
|
||||
|
||||
@@ -59,6 +59,9 @@ export const AgentParamsSchema = Type.Object(
|
||||
accountId: Type.Optional(Type.String()),
|
||||
replyAccountId: Type.Optional(Type.String()),
|
||||
threadId: Type.Optional(Type.String()),
|
||||
groupId: Type.Optional(Type.String()),
|
||||
groupChannel: Type.Optional(Type.String()),
|
||||
groupSpace: Type.Optional(Type.String()),
|
||||
timeout: Type.Optional(Type.Integer({ minimum: 0 })),
|
||||
lane: Type.Optional(Type.String()),
|
||||
extraSystemPrompt: Type.Optional(Type.String()),
|
||||
|
||||
@@ -76,6 +76,9 @@ export const agentHandlers: GatewayRequestHandlers = {
|
||||
accountId?: string;
|
||||
replyAccountId?: string;
|
||||
threadId?: string;
|
||||
groupId?: string;
|
||||
groupChannel?: string;
|
||||
groupSpace?: string;
|
||||
lane?: string;
|
||||
extraSystemPrompt?: string;
|
||||
idempotencyKey: string;
|
||||
@@ -85,6 +88,15 @@ export const agentHandlers: GatewayRequestHandlers = {
|
||||
};
|
||||
const cfg = loadConfig();
|
||||
const idem = request.idempotencyKey;
|
||||
const groupIdRaw = typeof request.groupId === "string" ? request.groupId.trim() : "";
|
||||
const groupChannelRaw =
|
||||
typeof request.groupChannel === "string" ? request.groupChannel.trim() : "";
|
||||
const groupSpaceRaw = typeof request.groupSpace === "string" ? request.groupSpace.trim() : "";
|
||||
let resolvedGroupId: string | undefined = groupIdRaw || undefined;
|
||||
let resolvedGroupChannel: string | undefined = groupChannelRaw || undefined;
|
||||
let resolvedGroupSpace: string | undefined = groupSpaceRaw || undefined;
|
||||
let spawnedByValue =
|
||||
typeof request.spawnedBy === "string" ? request.spawnedBy.trim() : undefined;
|
||||
const cached = context.dedupe.get(`agent:${idem}`);
|
||||
if (cached) {
|
||||
respond(cached.ok, cached.payload, cached.error, {
|
||||
@@ -198,7 +210,25 @@ export const agentHandlers: GatewayRequestHandlers = {
|
||||
const now = Date.now();
|
||||
const sessionId = entry?.sessionId ?? randomUUID();
|
||||
const labelValue = request.label?.trim() || entry?.label;
|
||||
const spawnedByValue = request.spawnedBy?.trim() || entry?.spawnedBy;
|
||||
spawnedByValue = spawnedByValue || entry?.spawnedBy;
|
||||
let inheritedGroup:
|
||||
| { groupId?: string; groupChannel?: string; groupSpace?: string }
|
||||
| undefined;
|
||||
if (spawnedByValue && (!resolvedGroupId || !resolvedGroupChannel || !resolvedGroupSpace)) {
|
||||
try {
|
||||
const parentEntry = loadSessionEntry(spawnedByValue)?.entry;
|
||||
inheritedGroup = {
|
||||
groupId: parentEntry?.groupId,
|
||||
groupChannel: parentEntry?.groupChannel,
|
||||
groupSpace: parentEntry?.space,
|
||||
};
|
||||
} catch {
|
||||
inheritedGroup = undefined;
|
||||
}
|
||||
}
|
||||
resolvedGroupId = resolvedGroupId || inheritedGroup?.groupId;
|
||||
resolvedGroupChannel = resolvedGroupChannel || inheritedGroup?.groupChannel;
|
||||
resolvedGroupSpace = resolvedGroupSpace || inheritedGroup?.groupSpace;
|
||||
const deliveryFields = normalizeSessionDeliveryFields(entry);
|
||||
const nextEntry: SessionEntry = {
|
||||
sessionId,
|
||||
@@ -217,6 +247,10 @@ export const agentHandlers: GatewayRequestHandlers = {
|
||||
providerOverride: entry?.providerOverride,
|
||||
label: labelValue,
|
||||
spawnedBy: spawnedByValue,
|
||||
channel: entry?.channel ?? request.channel?.trim(),
|
||||
groupId: resolvedGroupId ?? entry?.groupId,
|
||||
groupChannel: resolvedGroupChannel ?? entry?.groupChannel,
|
||||
space: resolvedGroupSpace ?? entry?.space,
|
||||
};
|
||||
sessionEntry = nextEntry;
|
||||
const sendPolicy = resolveSendPolicy({
|
||||
@@ -326,8 +360,15 @@ export const agentHandlers: GatewayRequestHandlers = {
|
||||
runContext: {
|
||||
messageChannel: resolvedChannel,
|
||||
accountId: resolvedAccountId,
|
||||
groupId: resolvedGroupId,
|
||||
groupChannel: resolvedGroupChannel,
|
||||
groupSpace: resolvedGroupSpace,
|
||||
currentThreadTs: resolvedThreadId != null ? String(resolvedThreadId) : undefined,
|
||||
},
|
||||
groupId: resolvedGroupId,
|
||||
groupChannel: resolvedGroupChannel,
|
||||
groupSpace: resolvedGroupSpace,
|
||||
spawnedBy: spawnedByValue,
|
||||
timeout: request.timeout?.toString(),
|
||||
bestEffortDeliver,
|
||||
messageChannel: resolvedChannel,
|
||||
|
||||
Reference in New Issue
Block a user