fix: landing fixes for toolsBySender precedence (#1757) (thanks @adam91holt)
This commit is contained in:
parent
73c071d56d
commit
22610215bb
@ -6,6 +6,7 @@ Docs: https://docs.clawd.bot
|
||||
Status: unreleased.
|
||||
|
||||
### Changes
|
||||
- Tools: add per-sender group tool policies and fix precedence. (#1757) Thanks @adam91holt.
|
||||
- Agents: summarize dropped messages during compaction safeguard pruning. (#2509) Thanks @jogi47.
|
||||
- Skills: add multi-image input support to Nano Banana Pro skill. (#1958) Thanks @tyler6204.
|
||||
- Agents: honor tools.exec.safeBins in exec allowlist checks. (#2281)
|
||||
|
||||
@ -298,8 +298,12 @@ ack reaction after the bot replies.
|
||||
- `guilds."*"`: default per-guild settings applied when no explicit entry exists.
|
||||
- `guilds.<id>.slug`: optional friendly slug used for display names.
|
||||
- `guilds.<id>.users`: optional per-guild user allowlist (ids or names).
|
||||
- `guilds.<id>.tools`: optional per-guild tool policy overrides (`allow`/`deny`/`alsoAllow`) used when the channel override is missing.
|
||||
- `guilds.<id>.toolsBySender`: optional per-sender tool policy overrides at the guild level (applies when the channel override is missing; `"*"` wildcard supported).
|
||||
- `guilds.<id>.channels.<channel>.allow`: allow/deny the channel when `groupPolicy="allowlist"`.
|
||||
- `guilds.<id>.channels.<channel>.requireMention`: mention gating for the channel.
|
||||
- `guilds.<id>.channels.<channel>.tools`: optional per-channel tool policy overrides (`allow`/`deny`/`alsoAllow`).
|
||||
- `guilds.<id>.channels.<channel>.toolsBySender`: optional per-sender tool policy overrides within the channel (`"*"` wildcard supported).
|
||||
- `guilds.<id>.channels.<channel>.users`: optional per-channel user allowlist.
|
||||
- `guilds.<id>.channels.<channel>.skills`: skill filter (omit = all skills, empty = none).
|
||||
- `guilds.<id>.channels.<channel>.systemPrompt`: extra system prompt for the channel (combined with channel topic).
|
||||
|
||||
@ -421,8 +421,12 @@ Key settings (see `/gateway/configuration` for shared channel patterns):
|
||||
- `channels.msteams.replyStyle`: `thread | top-level` (see [Reply Style](#reply-style-threads-vs-posts)).
|
||||
- `channels.msteams.teams.<teamId>.replyStyle`: per-team override.
|
||||
- `channels.msteams.teams.<teamId>.requireMention`: per-team override.
|
||||
- `channels.msteams.teams.<teamId>.tools`: default per-team tool policy overrides (`allow`/`deny`/`alsoAllow`) used when a channel override is missing.
|
||||
- `channels.msteams.teams.<teamId>.toolsBySender`: default per-team per-sender tool policy overrides (`"*"` wildcard supported).
|
||||
- `channels.msteams.teams.<teamId>.channels.<conversationId>.replyStyle`: per-channel override.
|
||||
- `channels.msteams.teams.<teamId>.channels.<conversationId>.requireMention`: per-channel override.
|
||||
- `channels.msteams.teams.<teamId>.channels.<conversationId>.tools`: per-channel tool policy overrides (`allow`/`deny`/`alsoAllow`).
|
||||
- `channels.msteams.teams.<teamId>.channels.<conversationId>.toolsBySender`: per-channel per-sender tool policy overrides (`"*"` wildcard supported).
|
||||
- `channels.msteams.sharePointSiteId`: SharePoint site ID for file uploads in group chats/channels (see [Sending files in group chats](#sending-files-in-group-chats)).
|
||||
|
||||
## Routing & Sessions
|
||||
|
||||
@ -464,6 +464,8 @@ For fine-grained control, use these tags in agent responses:
|
||||
Channel options (`channels.slack.channels.<id>` or `channels.slack.channels.<name>`):
|
||||
- `allow`: allow/deny the channel when `groupPolicy="allowlist"`.
|
||||
- `requireMention`: mention gating for the channel.
|
||||
- `tools`: optional per-channel tool policy overrides (`allow`/`deny`/`alsoAllow`).
|
||||
- `toolsBySender`: optional per-sender tool policy overrides within the channel (keys are sender ids/@handles/emails; `"*"` wildcard supported).
|
||||
- `allowBots`: allow bot-authored messages in this channel (default: false).
|
||||
- `users`: optional per-channel user allowlist.
|
||||
- `skills`: skill filter (omit = all skills, empty = none).
|
||||
|
||||
@ -232,6 +232,42 @@ Notes:
|
||||
- Discord defaults live in `channels.discord.guilds."*"` (overridable per guild/channel).
|
||||
- Group history context is wrapped uniformly across channels and is **pending-only** (messages skipped due to mention gating); use `messages.groupChat.historyLimit` for the global default and `channels.<channel>.historyLimit` (or `channels.<channel>.accounts.*.historyLimit`) for overrides. Set `0` to disable.
|
||||
|
||||
## Group/channel tool restrictions (optional)
|
||||
Some channel configs support restricting which tools are available **inside a specific group/room/channel**.
|
||||
|
||||
- `tools`: allow/deny tools for the whole group.
|
||||
- `toolsBySender`: per-sender overrides within the group (keys are sender IDs/usernames/emails/phone numbers depending on the channel). Use `"*"` as a wildcard.
|
||||
|
||||
Resolution order (most specific wins):
|
||||
1) group/channel `toolsBySender` match
|
||||
2) group/channel `tools`
|
||||
3) default (`"*"`) `toolsBySender` match
|
||||
4) default (`"*"`) `tools`
|
||||
|
||||
Example (Telegram):
|
||||
|
||||
```json5
|
||||
{
|
||||
channels: {
|
||||
telegram: {
|
||||
groups: {
|
||||
"*": { tools: { deny: ["exec"] } },
|
||||
"-1001234567890": {
|
||||
tools: { deny: ["exec", "read", "write"] },
|
||||
toolsBySender: {
|
||||
"123456789": { alsoAllow: ["exec"] }
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
Notes:
|
||||
- Group/channel tool restrictions are applied in addition to global/agent tool policy (deny still wins).
|
||||
- Some channels use different nesting for rooms/channels (e.g., Discord `guilds.*.channels.*`, Slack `channels.*`, MS Teams `teams.*.channels.*`).
|
||||
|
||||
## Group allowlists
|
||||
When `channels.whatsapp.groups`, `channels.telegram.groups`, or `channels.imessage.groups` is configured, the keys act as a group allowlist. Use `"*"` to allow all groups while still setting default mention behavior.
|
||||
|
||||
|
||||
@ -20,8 +20,10 @@ describe("gateway tool", () => {
|
||||
vi.useFakeTimers();
|
||||
const kill = vi.spyOn(process, "kill").mockImplementation(() => true);
|
||||
const previousStateDir = process.env.CLAWDBOT_STATE_DIR;
|
||||
const previousProfile = process.env.CLAWDBOT_PROFILE;
|
||||
const stateDir = await fs.mkdtemp(path.join(os.tmpdir(), "clawdbot-test-"));
|
||||
process.env.CLAWDBOT_STATE_DIR = stateDir;
|
||||
process.env.CLAWDBOT_PROFILE = "isolated";
|
||||
|
||||
try {
|
||||
const tool = createClawdbotTools({
|
||||
@ -62,6 +64,11 @@ describe("gateway tool", () => {
|
||||
} else {
|
||||
process.env.CLAWDBOT_STATE_DIR = previousStateDir;
|
||||
}
|
||||
if (previousProfile === undefined) {
|
||||
delete process.env.CLAWDBOT_PROFILE;
|
||||
} else {
|
||||
process.env.CLAWDBOT_PROFILE = previousProfile;
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
|
||||
@ -14,7 +14,13 @@ function enhanceBrowserFetchError(url: string, err: unknown, timeoutMs: number):
|
||||
? "If this is a sandboxed session, ensure the sandbox browser is running and try again."
|
||||
: `Start (or restart) the Clawdbot gateway (Clawdbot.app menubar, or \`${formatCliCommand("clawdbot gateway")}\`) and try again.`;
|
||||
const msg = String(err);
|
||||
if (msg.toLowerCase().includes("timed out") || msg.toLowerCase().includes("timeout")) {
|
||||
const lower = msg.toLowerCase();
|
||||
if (
|
||||
lower.includes("timed out") ||
|
||||
lower.includes("timeout") ||
|
||||
lower.includes("aborted") ||
|
||||
lower.includes("aborterror")
|
||||
) {
|
||||
return new Error(
|
||||
`Can't reach the clawd browser control service (timed out after ${timeoutMs}ms). ${hint}`,
|
||||
);
|
||||
|
||||
@ -309,6 +309,9 @@ describe("backward compatibility (profile parameter)", () => {
|
||||
testPort = await getFreePort();
|
||||
_cdpBaseUrl = `http://127.0.0.1:${testPort + 1}`;
|
||||
|
||||
prevGatewayPort = process.env.CLAWDBOT_GATEWAY_PORT;
|
||||
process.env.CLAWDBOT_GATEWAY_PORT = String(testPort - 2);
|
||||
|
||||
vi.stubGlobal(
|
||||
"fetch",
|
||||
vi.fn(async (url: string) => {
|
||||
@ -344,6 +347,11 @@ describe("backward compatibility (profile parameter)", () => {
|
||||
afterEach(async () => {
|
||||
vi.unstubAllGlobals();
|
||||
vi.restoreAllMocks();
|
||||
if (prevGatewayPort === undefined) {
|
||||
delete process.env.CLAWDBOT_GATEWAY_PORT;
|
||||
} else {
|
||||
process.env.CLAWDBOT_GATEWAY_PORT = prevGatewayPort;
|
||||
}
|
||||
const { stopBrowserControlServer } = await import("./server.js");
|
||||
await stopBrowserControlServer();
|
||||
});
|
||||
|
||||
@ -286,6 +286,9 @@ describe("profile CRUD endpoints", () => {
|
||||
testPort = await getFreePort();
|
||||
_cdpBaseUrl = `http://127.0.0.1:${testPort + 1}`;
|
||||
|
||||
prevGatewayPort = process.env.CLAWDBOT_GATEWAY_PORT;
|
||||
process.env.CLAWDBOT_GATEWAY_PORT = String(testPort - 2);
|
||||
|
||||
vi.stubGlobal(
|
||||
"fetch",
|
||||
vi.fn(async (url: string) => {
|
||||
@ -299,6 +302,11 @@ describe("profile CRUD endpoints", () => {
|
||||
afterEach(async () => {
|
||||
vi.unstubAllGlobals();
|
||||
vi.restoreAllMocks();
|
||||
if (prevGatewayPort === undefined) {
|
||||
delete process.env.CLAWDBOT_GATEWAY_PORT;
|
||||
} else {
|
||||
process.env.CLAWDBOT_GATEWAY_PORT = prevGatewayPort;
|
||||
}
|
||||
const { stopBrowserControlServer } = await import("./server.js");
|
||||
await stopBrowserControlServer();
|
||||
});
|
||||
|
||||
@ -1 +1 @@
|
||||
6d63b866aa0e917b278c6bef42229e8cd1f43c8ba31c845a96b4d9d5ce780265
|
||||
2567ca5bbc065b922d96717a488d5db3120b5b033c5d0508682d1aa8fbba470a
|
||||
|
||||
@ -5,7 +5,10 @@ import {
|
||||
resolveToolsBySender,
|
||||
} from "../../config/group-policy.js";
|
||||
import type { DiscordConfig } from "../../config/types.js";
|
||||
import type { GroupToolPolicyConfig } from "../../config/types.tools.js";
|
||||
import type {
|
||||
GroupToolPolicyBySenderConfig,
|
||||
GroupToolPolicyConfig,
|
||||
} from "../../config/types.tools.js";
|
||||
import { resolveSlackAccount } from "../../slack/accounts.js";
|
||||
|
||||
type GroupMentionParams = {
|
||||
@ -331,7 +334,9 @@ export function resolveSlackGroupToolPolicy(
|
||||
channelName ?? "",
|
||||
normalizedName,
|
||||
].filter(Boolean);
|
||||
let matched: { tools?: GroupToolPolicyConfig } | undefined;
|
||||
let matched:
|
||||
| { tools?: GroupToolPolicyConfig; toolsBySender?: GroupToolPolicyBySenderConfig }
|
||||
| undefined;
|
||||
for (const candidate of candidates) {
|
||||
if (candidate && channels[candidate]) {
|
||||
matched = channels[candidate];
|
||||
|
||||
@ -1,17 +1,19 @@
|
||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
import { Command } from "commander";
|
||||
|
||||
const clientMocks = vi.hoisted(() => ({
|
||||
browserSnapshot: vi.fn(async () => ({
|
||||
const gatewayMocks = vi.hoisted(() => ({
|
||||
callGatewayFromCli: vi.fn(async () => ({
|
||||
ok: true,
|
||||
format: "ai",
|
||||
targetId: "t1",
|
||||
url: "https://example.com",
|
||||
snapshot: "ok",
|
||||
})),
|
||||
resolveBrowserControlUrl: vi.fn(() => "http://127.0.0.1:18791"),
|
||||
}));
|
||||
vi.mock("../browser/client.js", () => clientMocks);
|
||||
|
||||
vi.mock("./gateway-rpc.js", () => ({
|
||||
callGatewayFromCli: gatewayMocks.callGatewayFromCli,
|
||||
}));
|
||||
|
||||
const configMocks = vi.hoisted(() => ({
|
||||
loadConfig: vi.fn(() => ({ browser: {} })),
|
||||
@ -37,6 +39,7 @@ describe("browser cli snapshot defaults", () => {
|
||||
configMocks.loadConfig.mockReturnValue({
|
||||
browser: { snapshotDefaults: { mode: "efficient" } },
|
||||
});
|
||||
|
||||
const { registerBrowserInspectCommands } = await import("./browser-cli-inspect.js");
|
||||
const program = new Command();
|
||||
const browser = program.command("browser").option("--json", false);
|
||||
@ -44,12 +47,18 @@ describe("browser cli snapshot defaults", () => {
|
||||
|
||||
await program.parseAsync(["browser", "snapshot"], { from: "user" });
|
||||
|
||||
expect(clientMocks.browserSnapshot).toHaveBeenCalledWith(
|
||||
"http://127.0.0.1:18791",
|
||||
expect(gatewayMocks.callGatewayFromCli).toHaveBeenCalledWith(
|
||||
"browser.request",
|
||||
expect.any(Object),
|
||||
expect.objectContaining({
|
||||
format: "ai",
|
||||
mode: "efficient",
|
||||
method: "GET",
|
||||
path: "/snapshot",
|
||||
query: expect.objectContaining({
|
||||
format: "ai",
|
||||
mode: "efficient",
|
||||
}),
|
||||
}),
|
||||
expect.any(Object),
|
||||
);
|
||||
});
|
||||
|
||||
@ -57,13 +66,15 @@ describe("browser cli snapshot defaults", () => {
|
||||
configMocks.loadConfig.mockReturnValue({
|
||||
browser: { snapshotDefaults: { mode: "efficient" } },
|
||||
});
|
||||
clientMocks.browserSnapshot.mockResolvedValueOnce({
|
||||
|
||||
gatewayMocks.callGatewayFromCli.mockResolvedValueOnce({
|
||||
ok: true,
|
||||
format: "aria",
|
||||
targetId: "t1",
|
||||
url: "https://example.com",
|
||||
nodes: [],
|
||||
});
|
||||
|
||||
const { registerBrowserInspectCommands } = await import("./browser-cli-inspect.js");
|
||||
const program = new Command();
|
||||
const browser = program.command("browser").option("--json", false);
|
||||
@ -71,8 +82,9 @@ describe("browser cli snapshot defaults", () => {
|
||||
|
||||
await program.parseAsync(["browser", "snapshot", "--format", "aria"], { from: "user" });
|
||||
|
||||
expect(clientMocks.browserSnapshot).toHaveBeenCalled();
|
||||
const [, opts] = clientMocks.browserSnapshot.mock.calls.at(-1) ?? [];
|
||||
expect(opts?.mode).toBeUndefined();
|
||||
expect(gatewayMocks.callGatewayFromCli).toHaveBeenCalled();
|
||||
const [, , params] = gatewayMocks.callGatewayFromCli.mock.calls.at(-1) ?? [];
|
||||
expect(params).toBeTruthy();
|
||||
expect((params as { query?: Record<string, string> }).query?.mode).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
@ -1,4 +1,19 @@
|
||||
import { describe, expect, it, vi } from "vitest";
|
||||
import { afterAll, beforeAll, describe, expect, it, vi } from "vitest";
|
||||
|
||||
let previousProfile: string | undefined;
|
||||
|
||||
beforeAll(() => {
|
||||
previousProfile = process.env.CLAWDBOT_PROFILE;
|
||||
process.env.CLAWDBOT_PROFILE = "isolated";
|
||||
});
|
||||
|
||||
afterAll(() => {
|
||||
if (previousProfile === undefined) {
|
||||
delete process.env.CLAWDBOT_PROFILE;
|
||||
} else {
|
||||
process.env.CLAWDBOT_PROFILE = previousProfile;
|
||||
}
|
||||
});
|
||||
|
||||
const mocks = vi.hoisted(() => ({
|
||||
loadSessionStore: vi.fn().mockReturnValue({
|
||||
|
||||
@ -1,8 +1,22 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { afterAll, beforeAll, describe, expect, it } from "vitest";
|
||||
|
||||
import { buildPairingReply } from "./pairing-messages.js";
|
||||
|
||||
describe("buildPairingReply", () => {
|
||||
let previousProfile: string | undefined;
|
||||
|
||||
beforeAll(() => {
|
||||
previousProfile = process.env.CLAWDBOT_PROFILE;
|
||||
process.env.CLAWDBOT_PROFILE = "isolated";
|
||||
});
|
||||
|
||||
afterAll(() => {
|
||||
if (previousProfile === undefined) {
|
||||
delete process.env.CLAWDBOT_PROFILE;
|
||||
} else {
|
||||
process.env.CLAWDBOT_PROFILE = previousProfile;
|
||||
}
|
||||
});
|
||||
const cases = [
|
||||
{
|
||||
channel: "discord",
|
||||
|
||||
Loading…
Reference in New Issue
Block a user