Skip to content

Commit 74edaf7

Browse files
authored
Merge pull request #110 from httpsphl/feat/allow-always
feat: "Allow always" on a bot's requests, with the rules in its details
2 parents 25a332d + e7edbed commit 74edaf7

39 files changed

Lines changed: 1139 additions & 30 deletions
Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
// What the bot may do without asking (spec 10.1), in its details: each
2+
// "Allow always" the owner gave, with a button that makes the bot ask again.
3+
4+
import { X } from "lucide-react";
5+
import { useEffect, useState } from "react";
6+
import { useT } from "../../i18n";
7+
import type { AllowRule, Bot } from "../../lib/protocol.gen";
8+
import { useApi } from "../../store/context";
9+
import { Button } from "../../ui/Button";
10+
import { attempt } from "../../ui/toast";
11+
import { toolTitle } from "../chat/toolNames";
12+
13+
/** The bot's rules, read when shown and kept current by `bot.rules`. */
14+
function useAllowRules(botId: Bot["id"]): AllowRule[] | null {
15+
const api = useApi();
16+
const [rules, setRules] = useState<AllowRule[] | null>(null);
17+
useEffect(() => {
18+
let alive = true;
19+
setRules(null);
20+
const stop = api.subscribe((event) => {
21+
if (event.name === "bot.rules" && event.params.botId === botId) {
22+
setRules(event.params.rules);
23+
}
24+
});
25+
api.call("rules.list", { botId }).then(
26+
(listed) => alive && setRules((current) => current ?? listed),
27+
// An older daemon has no rules: the list stays empty.
28+
() => alive && setRules([]),
29+
);
30+
return () => {
31+
alive = false;
32+
stop();
33+
};
34+
}, [api, botId]);
35+
return rules;
36+
}
37+
38+
export function AllowRules({ bot }: { bot: Bot }) {
39+
const t = useT();
40+
const words = t.bots.details;
41+
const api = useApi();
42+
const rules = useAllowRules(bot.id);
43+
return (
44+
<div>
45+
<dt className="text-muted text-xs">{words.always}</dt>
46+
<dd className="mt-1">
47+
{rules !== null && rules.length === 0 && (
48+
<p className="text-ink-soft text-xs leading-relaxed">{words.alwaysNone(bot.name)}</p>
49+
)}
50+
{rules !== null && rules.length > 0 && (
51+
<ul className="flex flex-col gap-1.5">
52+
{rules.map((rule) => {
53+
const title = toolTitle(rule.scope.toolName, t.tools);
54+
const what = rule.scope.value ? `${title}: ${rule.scope.value}` : title;
55+
return (
56+
<li
57+
key={rule.id}
58+
className="flex items-start gap-2 rounded-lg border border-line bg-sunken py-1.5 pr-1 pl-2.5"
59+
>
60+
<span className="min-w-0 flex-1">
61+
<span className="block text-ink-soft text-xs">{title}</span>
62+
{rule.scope.value && (
63+
<span className="block break-all font-mono text-xs" data-selectable>
64+
{rule.scope.value}
65+
</span>
66+
)}
67+
</span>
68+
<Button
69+
variant="ghost"
70+
size="sm"
71+
icon={X}
72+
label={words.alwaysRemove(what)}
73+
onClick={() =>
74+
attempt(words.alwaysRemoveFailed, () =>
75+
api.call("rules.delete", { ruleId: rule.id }),
76+
)
77+
}
78+
/>
79+
</li>
80+
);
81+
})}
82+
</ul>
83+
)}
84+
</dd>
85+
</div>
86+
);
87+
}

‎app/src/features/bots/BotView.tsx‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import { BotRoutines } from "../routines/RoutineList";
2323
import { ScreensPanel } from "../screens/ScreensPanel";
2424
import { ShowScreen } from "../screens/showScreen";
2525
import { useScreens } from "../screens/useScreens";
26+
import { AllowRules } from "./AllowRules";
2627
import { BotHeader } from "./BotHeader";
2728
import { stateView } from "./BotStateBadge";
2829
import { useFollowBot } from "./useFollowBot";
@@ -287,6 +288,7 @@ function Details({ bot, onClose }: { bot: Bot; onClose(): void }) {
287288
{bot.instructions || words.noInstructions}
288289
</dd>
289290
</div>
291+
<AllowRules bot={bot} />
290292
</dl>
291293
</SidePanel>
292294
);

‎app/src/features/chat/ApprovalCard.tsx‎

Lines changed: 30 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
// A permission request (spec 10.1): what the bot wants to do, with Allow
22
// and Deny. Once answered it shrinks to one line.
33

4-
import { Ban, Check, Hand, TimerOff } from "lucide-react";
4+
import { Ban, Check, CheckCheck, Hand, TimerOff } from "lucide-react";
55
import { useId } from "react";
66
import { useT } from "../../i18n";
7-
import type { ApprovalItem, Bot } from "../../lib/protocol.gen";
7+
import type { AllowScope, ApprovalItem, Bot } from "../../lib/protocol.gen";
88
import { Button } from "../../ui/Button";
99
import { Details } from "../../ui/Details";
1010
import { HELP_TOOL, HelpCard } from "../browser/HelpCard";
@@ -78,6 +78,23 @@ export function ApprovalCard({ approval, bot }: { approval: ApprovalItem; bot: B
7878
}
7979
}
8080

81+
/** "Allow always", by what it would cover. */
82+
function alwaysLabel(
83+
scope: AllowScope,
84+
words: ReturnType<typeof useT>["chat"]["approval"]["always"],
85+
): string {
86+
switch (scope.kind) {
87+
case "command":
88+
return words.command;
89+
case "site":
90+
return words.site(scope.value);
91+
case "file":
92+
return words.file;
93+
default:
94+
return words.tool;
95+
}
96+
}
97+
8198
const BOX =
8299
"max-h-56 overflow-auto break-all rounded-lg border border-line bg-sunken px-2.5 py-1.5";
83100

@@ -158,10 +175,20 @@ function ToolApproval({ approval, bot }: { approval: ApprovalItem; bot: Bot }) {
158175
placeholder={t.chat.approval.notePlaceholder}
159176
className="mt-3 h-9 w-full rounded-xl border border-line-strong bg-canvas px-3 text-sm outline-none placeholder:text-muted focus:border-muted"
160177
/>
161-
<div className="mt-2.5 flex gap-2">
178+
<div className="mt-2.5 flex flex-wrap gap-2">
162179
<Button variant="primary" icon={Check} disabled={busy} onClick={() => answer(true)}>
163180
{t.chat.approval.allow}
164181
</Button>
182+
{approval.always && (
183+
<Button
184+
icon={CheckCheck}
185+
disabled={busy}
186+
title={t.chat.approval.always.hint(bot.name)}
187+
onClick={() => answer(true, undefined, true)}
188+
>
189+
{alwaysLabel(approval.always, t.chat.approval.always)}
190+
</Button>
191+
)}
165192
<Button variant="danger" icon={Ban} disabled={busy} onClick={() => answer(false)}>
166193
{t.chat.approval.deny}
167194
</Button>
Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
import { act, cleanup, fireEvent, screen, within } from "@testing-library/react";
2+
import { afterEach, describe, expect, test } from "vitest";
3+
import { FakeBotloft } from "../../lib/fake";
4+
import { crewOpened, openBot, renderApp } from "../../test/app";
5+
6+
afterEach(cleanup);
7+
8+
async function openScout() {
9+
const fake = new FakeBotloft();
10+
const ops = fake.addCrew("Ops");
11+
const scout = fake.addBot(ops.id, "Scout", "Finds sources");
12+
fake.setBotState(scout.id, "idle");
13+
renderApp(fake);
14+
await crewOpened("Ops");
15+
openBot("Scout");
16+
await screen.findByRole("list", { name: "Messages" });
17+
return { fake, scout };
18+
}
19+
20+
const details = () => screen.getByRole("complementary", { name: "About Scout" });
21+
22+
describe("Allow always", () => {
23+
test("allows the request for good and lists it in the bot's details, where it can go", async () => {
24+
const { fake, scout } = await openScout();
25+
act(() => {
26+
fake.chat.ask(scout.id, "Bash", "git status", '{"command":"git status"}');
27+
});
28+
const card = screen.getByRole("region", { name: "Scout asks to run a command" });
29+
const always = within(card).getByRole("button", { name: "Always allow this command" });
30+
expect(always.getAttribute("title")).toBe(
31+
"Scout will not ask for this again. You can undo it in Scout's details.",
32+
);
33+
fireEvent.click(always);
34+
expect(await screen.findByText("Allowed: run a command")).toBeDefined();
35+
expect(fake.calls.find((call) => call.method === "approvals.answer")?.params).toMatchObject({
36+
allow: true,
37+
always: true,
38+
});
39+
40+
fireEvent.click(screen.getByRole("button", { name: /^Show details/ }));
41+
const rule = await within(details()).findByText("git status");
42+
expect(rule).toBeDefined();
43+
fireEvent.click(
44+
within(details()).getByRole("button", { name: "Ask again: Run a command: git status" }),
45+
);
46+
expect(
47+
await within(details()).findByText(
48+
'Nothing yet. When Scout asks for something, "Always allow" puts it here.',
49+
),
50+
).toBeDefined();
51+
});
52+
53+
test("names the site, and is not offered for what cannot be allowed for good", async () => {
54+
const { fake, scout } = await openScout();
55+
act(() => {
56+
fake.chat.ask(
57+
scout.id,
58+
"WebFetch",
59+
"https://www.example.com/a",
60+
'{"url":"https://www.example.com/a"}',
61+
);
62+
});
63+
const page = screen.getByRole("region", { name: /^Scout asks to/ });
64+
expect(within(page).getByRole("button", { name: "Always allow example.com" })).toBeDefined();
65+
fireEvent.click(within(page).getByRole("button", { name: "Allow" }));
66+
await screen.findByText(/^Allowed:/);
67+
// A plain Allow keeps no rule.
68+
expect(fake.allow.rules).toEqual([]);
69+
70+
act(() => {
71+
fake.chat.ask(scout.id, "Glob", "*.md", '{"pattern":"*.md"}');
72+
});
73+
const glob = screen.getByRole("region", { name: /^Scout asks to/ });
74+
expect(within(glob).queryByRole("button", { name: /^Always allow/ })).toBeNull();
75+
});
76+
});

‎app/src/features/chat/useAnswer.ts‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,14 @@ import { attempt } from "../../ui/toast";
55

66
/**
77
* Answering a request; the note goes to the bot on a denial. `input` is a
8-
* bot suggestion as the owner changed it (spec 10.2).
8+
* bot suggestion as the owner changed it (spec 10.2); `always` allows the
9+
* request for good (spec 10.1).
910
*/
1011
export function useAnswer(approval: ApprovalItem, failed: { allow: string; deny: string }) {
1112
const api = useApi();
1213
const [note, setNote] = useState("");
1314
const [busy, setBusy] = useState(false);
14-
const answer = async (allow: boolean, input?: string) => {
15+
const answer = async (allow: boolean, input?: string, always = false) => {
1516
setBusy(true);
1617
const trimmed = note.trim();
1718
await attempt(allow ? failed.allow : failed.deny, () =>
@@ -20,6 +21,7 @@ export function useAnswer(approval: ApprovalItem, failed: { allow: string; deny:
2021
allow,
2122
...(allow || !trimmed ? {} : { note: trimmed }),
2223
...(allow && input !== undefined ? { input } : {}),
24+
...(allow && always ? { always } : {}),
2325
}),
2426
);
2527
setBusy(false);

‎app/src/i18n/en/bots.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,5 +116,10 @@ export const bots = {
116116
generation: (generation: number) => `generation ${generation}`,
117117
instructions: "Instructions",
118118
noInstructions: "None yet.",
119+
always: "Allowed without asking",
120+
alwaysNone: (bot: string) =>
121+
`Nothing yet. When ${bot} asks for something, "Always allow" puts it here.`,
122+
alwaysRemove: (what: string) => `Ask again: ${what}`,
123+
alwaysRemoveFailed: "Could not remove it",
119124
},
120125
};

‎app/src/i18n/en/chat.ts‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,15 @@ export const chat = {
8383
notePlaceholder: "Why not? Sent to the bot if you deny (optional)",
8484
allow: "Allow",
8585
deny: "Deny",
86+
/** "Allow always" (spec 10.1), by what it covers. */
87+
always: {
88+
command: "Always allow this command",
89+
site: (site: string) => `Always allow ${site}`,
90+
file: "Always allow this file",
91+
tool: "Always allow",
92+
hint: (bot: string) =>
93+
`${bot} will not ask for this again. You can undo it in ${bot}'s details.`,
94+
},
8695
allowFailed: "Could not allow it",
8796
denyFailed: "Could not deny it",
8897
allowed: (action: string) => `Allowed: ${action}`,

‎app/src/i18n/es/bots.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,5 +114,10 @@ export const bots: Messages["bots"] = {
114114
generation: (generation: number) => `generación ${generation}`,
115115
instructions: "Instrucciones",
116116
noInstructions: "Ninguna todavía.",
117+
always: "Permitido sin preguntar",
118+
alwaysNone: (bot: string) =>
119+
`Nada todavía. Cuando ${bot} pida algo, "Permitir siempre" lo pone aquí.`,
120+
alwaysRemove: (what: string) => `Volver a preguntar: ${what}`,
121+
alwaysRemoveFailed: "No se pudo quitar",
117122
},
118123
};

‎app/src/i18n/es/chat.ts‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,14 @@ export const chat: Messages["chat"] = {
8080
notePlaceholder: "¿Por qué no? Se envía al bot si deniegas (opcional)",
8181
allow: "Permitir",
8282
deny: "Denegar",
83+
always: {
84+
command: "Permitir siempre este comando",
85+
site: (site: string) => `Permitir siempre en ${site}`,
86+
file: "Permitir siempre este archivo",
87+
tool: "Permitir siempre",
88+
hint: (bot: string) =>
89+
`${bot} no volverá a pedir esto. Puedes deshacerlo en los detalles de ${bot}.`,
90+
},
8391
allowFailed: "No se pudo permitir",
8492
denyFailed: "No se pudo denegar",
8593
allowed: (action: string) => `Permitido: ${action}`,

‎app/src/i18n/pt-BR/bots.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,5 +113,10 @@ export const bots: Messages["bots"] = {
113113
generation: (generation: number) => `geração ${generation}`,
114114
instructions: "Instruções",
115115
noInstructions: "Nenhuma ainda.",
116+
always: "Permitido sem perguntar",
117+
alwaysNone: (bot: string) =>
118+
`Nada ainda. Quando ${bot} pedir algo, "Permitir sempre" coloca aqui.`,
119+
alwaysRemove: (what: string) => `Voltar a perguntar: ${what}`,
120+
alwaysRemoveFailed: "Não foi possível remover",
116121
},
117122
};

0 commit comments

Comments
 (0)