diff --git a/docs/agents.md b/docs/agents.md index 2520085c1..e6281ca0e 100644 --- a/docs/agents.md +++ b/docs/agents.md @@ -143,12 +143,58 @@ so the agent can finish and retire its Interest. Attention delivers the wake in the conversation where it called the tool: no message is sent, and the agent is told its text is data. It quotes the failed checks' names and the new comments, line by line, each clipped, with GitHub's links. Agents comment as the -`gh` account, as their owner does, so GitHub cannot tell them apart: a comment of -that account that starts with πŸ€–, as an agent's do, wakes nobody, and the owner's -other comments wake the agent. A watch is saved in Attention on the device and respects its on/off switch. Device-local GitHub watches saved before Attention are dropped, not imported: they name no Interest. It ends when the +`gh` account, as their owner does, so GitHub cannot tell them apart: by default +a comment of that account that starts with πŸ€–, as an agent's do, wakes nobody, +and the owner's other comments wake the agent. A watch is saved in Attention on the device and respects its on/off switch. Device-local GitHub watches saved before Attention are dropped, not imported: they name no Interest. It ends when the pull request merges or closes, after 20 wakes, after 30 minutes without a successful read, or when the agent calls it with `stop: true`. +Settings β†’ Plugins β†’ GitHub comment authors decides whose feedback wakes agents, +since anyone can comment on a public pull request. It holds rules, one per +line, saved on the device. Each starts with `allow` or `deny`, followed by any +of these conditions, all of which must hold: + +- `author:login`, an exact login, or `author:@me`, the signed-in `gh` account. + A bot's login ends in `[bot]`. +- `author:/pattern/`, a JavaScript regular expression the login must match. +- `org:name`, membership of a GitHub organization, named exactly. +- `body:/pattern/`, a regular expression the comment's first 1,000 characters + must match. That is the most of a comment a wake quotes. +- `prefix:text`, what the comment starts with, leading space aside. It runs to + the end of the line, spaces included, so it comes last. + +A bare `allow` or `deny`, with no conditions, matches everyone. + +Logins match case aside, so an author pattern always has the `i` flag; a +pattern may also use `m`, `s` and `u`. Patterns take JavaScript's syntax but not +backreferences, lookaround or escapes that mean something else alone, such as +`\c1`, `\U0041` or `\q`, and `src/bundled/github/pattern.ts` matches them +as an automaton, in time the text's length bounds, so a pattern like +`/^(a+)+$/` cannot stall the app on a hostile comment. The first rule that matches decides, and +When no rule matches decides the rest: Wake the agent or Don't wake it. +Saving compiles every pattern and rejects the rules, naming the line, when one +is not a rule, or when they name more organizations than one membership read +can ask about. Saved rules that no longer parse, such as a pattern with +lookaround an earlier build accepted, stay in Settings with their error, and +hold all feedback until they are fixed rather than falling back to the defaults. The defaults keep the behavior from before rules: agents' +comments wake nobody, and everyone else's wake the agent. + +```text +deny author:@me prefix:πŸ€– +``` + +with When no rule matches set to Wake the agent. Adding, say, +`allow org:block` and `allow author:copilot-pull-request-reviewer[bot]` and choosing Don't wake it +limits feedback to those. Membership is asked of `gh` only for organizations the rules +name, and kept for an hour; feedback whose author's membership cannot be read +waits for a later poll, until ten polls in a row fail for that login alone and +it counts as no member. While GitHub answers no membership read, not even about +the signed-in account, no failure counts, so an outage holds feedback rather +than letting it past an `org:` deny rule. A membership hidden from the `gh` account, or a +misspelled organization, counts as none, and bots and deleted accounts are in +no organization. Each poll reads the rules again, so a change applies to +running watches. CI and pull request state are not limited. + The Agents runner starts a mention run from a `p` tag, or from any new message by the other member of a one-to-one DM the agent is in; group DMs still need the mention. It also owns watches and timers, and runs attention work in Janet's lanes: each diff --git a/src/app/pages.integration.test.mjs b/src/app/pages.integration.test.mjs index 8431eede7..98fd08707 100644 --- a/src/app/pages.integration.test.mjs +++ b/src/app/pages.integration.test.mjs @@ -135,7 +135,13 @@ test("the app runtime exposes ready bundled pages and removes them on disable", .snapshot() .map((card) => card.pluginId) .sort(), - ["block.builderlab", "buzz.channels", "buzz.emoji", "buzz.pairing"], + [ + "block.builderlab", + "buzz.channels", + "buzz.emoji", + "buzz.github", + "buzz.pairing", + ], ); // The device-local template library remains reachable without a selected community. assert.equal( @@ -201,7 +207,13 @@ test("the app runtime exposes ready bundled pages and removes them on disable", .snapshot() .map((card) => card.pluginId) .sort(), - ["block.builderlab", "buzz.channels", "buzz.emoji", "buzz.pairing"], + [ + "block.builderlab", + "buzz.channels", + "buzz.emoji", + "buzz.github", + "buzz.pairing", + ], ); await vi.waitFor(() => diff --git a/src/bundled/github/AuthorSettings.test.tsx b/src/bundled/github/AuthorSettings.test.tsx new file mode 100644 index 000000000..5098ee604 --- /dev/null +++ b/src/bundled/github/AuthorSettings.test.tsx @@ -0,0 +1,85 @@ +// @vitest-environment jsdom +import "@testing-library/jest-dom/vitest"; +import { cleanup, render, screen } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { afterEach, expect, it } from "vitest"; +import { AuthorSettings } from "./AuthorSettings"; +import { DEFAULT_RULES, loadPolicy, POLICY_KEY } from "./authors"; + +afterEach(() => { + cleanup(); + localStorage.clear(); +}); + +it("shows the default rules, and saves only rules that parse", async () => { + const user = userEvent.setup(); + render( true} />); + expect(screen.getByRole("status")).toHaveTextContent( + "Comments from anyone no rule denies can wake your agents.", + ); + expect(screen.getByRole("radio", { name: "Wake the agent" })).toBeChecked(); + const rules = screen.getByRole("textbox", { name: /Rules/ }); + expect(rules).toHaveValue(DEFAULT_RULES); + const save = screen.getByRole("button", { name: "Save" }); + expect(save).toBeDisabled(); + await user.clear(rules); + await user.type(rules, "deny author:@me prefix:πŸ€–{Enter}allow author:/(/"); + await user.click(save); + expect(rules).toBeInvalid(); + expect(rules).toHaveAccessibleDescription( + expect.stringContaining("Line 2: Invalid regular expression"), + ); + expect(loadPolicy().text).toBe(DEFAULT_RULES); + // Editing checks the rules again, so restoring the saved ones clears it. + await user.clear(rules); + await user.type(rules, DEFAULT_RULES); + expect(rules).toBeValid(); + expect(screen.queryByRole("alert")).not.toBeInTheDocument(); + expect(save).toBeDisabled(); + await user.clear(rules); + await user.type( + rules, + "deny author:@me prefix:πŸ€–{Enter}allow org:block{Enter} allow author:copilot-pull-request-reviewer[[bot] ", + ); + await user.click(screen.getByRole("radio", { name: "Don’t wake it" })); + await user.click(save); + expect(loadPolicy().text).toBe( + "deny author:@me prefix:πŸ€–\nallow org:block\nallow author:copilot-pull-request-reviewer[bot]", + ); + expect(loadPolicy().fallback).toBe("deny"); + expect(rules).toHaveValue(loadPolicy().text); + expect(screen.queryByRole("status")).not.toBeInTheDocument(); + expect(screen.queryByRole("alert")).not.toBeInTheDocument(); + expect(save).toBeDisabled(); + await user.click(screen.getByRole("radio", { name: "Wake the agent" })); + expect(save).toBeEnabled(); + await user.click(save); + expect(loadPolicy().fallback).toBe("allow"); + expect(screen.getByRole("status")).toBeInTheDocument(); +}); + +it("shows saved rules that no longer parse, with why, until they are fixed", async () => { + const user = userEvent.setup(); + const text = "allow author:/^(?!eve$)[a-z]+$/"; + localStorage.setItem( + POLICY_KEY, + JSON.stringify({ version: 2, text, fallback: "deny" }), + ); + render( true} />); + const rules = screen.getByRole("textbox", { name: /Rules/ }); + expect(rules).toHaveValue(text); + expect(rules).toBeInvalid(); + expect(rules).toHaveAccessibleDescription( + expect.stringContaining("Line 1: a pattern's groups can only be"), + ); + expect(screen.getByRole("status")).toHaveTextContent( + "no comment or review wakes your agents", + ); + expect(screen.getByRole("radio", { name: "Don’t wake it" })).toBeChecked(); + await user.clear(rules); + await user.type(rules, "deny author:eve{Enter}allow author:/^[[a-z]+$/"); + await user.click(screen.getByRole("button", { name: "Save" })); + expect(loadPolicy().error).toBeUndefined(); + expect(rules).toBeValid(); + expect(screen.queryByRole("status")).not.toBeInTheDocument(); +}); diff --git a/src/bundled/github/AuthorSettings.tsx b/src/bundled/github/AuthorSettings.tsx new file mode 100644 index 000000000..9076b0be9 --- /dev/null +++ b/src/bundled/github/AuthorSettings.tsx @@ -0,0 +1,103 @@ +import { useState } from "react"; +import { Button } from "../../shared/design-system/ui/Button"; +import { Field } from "../../shared/design-system/ui/Field"; +import { Header } from "../../shared/design-system/ui/Header"; +import { Radio, RadioGroup } from "../../shared/design-system/ui/RadioGroup"; +import { SettingsGroup } from "../../shared/design-system/ui/SettingsGroup"; +import { Textarea } from "../../shared/design-system/ui/Textarea"; +import { loadPolicy, open, parse, savePolicy, type Rule } from "./authors"; + +/** Whose pull request comments and reviews wake agents watching it. */ +export function AuthorSettings({ active }: { active(): boolean }) { + const [saved, setSaved] = useState(loadPolicy); + const [draft, setDraft] = useState(saved.text); + const [fallback, setFallback] = useState(saved.fallback); + /** Why the rules could not be saved, apart from why they are not rules. */ + const [error, setError] = useState(""); + // After a Save, each edit checks the rules again. + const [checked, setChecked] = useState(saved.error !== undefined); + const parsed = parse(draft); + const dirty = + typeof parsed === "string" || + parsed.text !== saved.text || + fallback !== saved.fallback; + return ( +
+
+
{ + event.preventDefault(); + if (!active() || !dirty) return; + setChecked(true); + if (typeof parsed === "string") return; + try { + const policy = savePolicy(draft, fallback); + setSaved(policy); + setDraft(policy.text); + setChecked(false); + setError(""); + } catch (reason) { + setError(reason instanceof Error ? reason.message : String(reason)); + } + }} + > + + {saved.error !== undefined && ( +

+ Your saved rules no longer parse, so no comment or review wakes + your agents until you fix them. +

+ )} + {open(saved) && ( +

+ Comments from anyone no rule denies can wake your agents. To limit + them, allow only org:name or author:login, and don’t wake them + when no rule matches. +

+ )} + +