diff --git a/desktop/playwright.config.ts b/desktop/playwright.config.ts index 459fa75743..c2eec84a83 100644 --- a/desktop/playwright.config.ts +++ b/desktop/playwright.config.ts @@ -121,6 +121,7 @@ export default defineConfig({ "**/signout-confirmation.spec.ts", "**/agent-provider-dropdowns.spec.ts", "**/agent-lifecycle-feedback.spec.ts", + "**/agent-access-warning.spec.ts", "**/inbox-live-update.spec.ts", "**/mesh-compute.spec.ts", "**/observer-archive-policy.spec.ts", diff --git a/desktop/src/features/agents/AGENTS.md b/desktop/src/features/agents/AGENTS.md index 35ad4a63af..d9222c7032 100644 --- a/desktop/src/features/agents/AGENTS.md +++ b/desktop/src/features/agents/AGENTS.md @@ -114,6 +114,39 @@ with a TypeScript lookup table or an id comparison in a component. published or removed. A queued update must stay visibly queued, and the catalog itself must render only relay-confirmed publications — never an optimistic local persona. +11. **Shared agent access names the consequence where it is selected.** The + shared respond-to field shows a persistent warning whenever `anyone` **or** + `allowlist` is selected — both hand the host's access to someone other than + the owner, so both disclose it and only the audience phrase differs. This + covers persona-backed create and edit surfaces. Keep that disclosure in + the shared field instead of adding surface-specific flags. It renders + directly below the selector for `anyone` but *after* the people picker for + `allowlist`, so it never sits between the user and the selection they came + to make. The copy leads with the audience ("Anyone can use this agent to + access…") so it reads as a warning rather than an explanation, and stays one + sentence — don't split the mechanism into a second sentence. Both the machine + and the stakes it names come from `lib/agentAccessWarning.ts`, keyed on an + optional `runLocation`: instance surfaces resolve it from + `ManagedAgent.backend` via `runLocationForBackend`, and the create flow from + `WhereToRunDraft.runOn` via `runLocationForRunOn`. `AgentDialog` is the one + place that resolves it for dialog surfaces and publishes it through + `ui/AgentRunLocationContext.tsx`; the field reads that context and lets an + explicit `runLocation` prop win. Do **not** thread the value as a prop + through `AgentDefinitionDialog` / `AgentInstanceEditDialog` — both are + already over the 1000-line ceiling, and neither uses the value itself. + Surfaces rendered outside `AgentDialog` (e.g. `EditRespondToDialog`) pass the + prop directly. Local names "your + computer, including files, accounts, and connected tools"; remote names "the + server it runs on, including any accounts and tools available there" — + deliberately *not* the owner's files, which aren't theirs to describe on a + host they don't own. **An unknown location falls back to the local wording — + never hedge with "computer or server".** A remote host requires an + installed `buzz-backend-*` provider, and without one `WhereToRunSection` + never renders, so "server" would name a concept the owner has never been + shown; when it *is* remote they picked that host from the selector + themselves. Never synthesize a run location a surface doesn't have. Don't + expose `respond-to`, `allowlist`, Nostr, or harness jargon in primary UI + copy. ## The tests that enforce this @@ -128,6 +161,12 @@ with a TypeScript lookup table or an id comparison in a component. `isCacheableDiscoveryResponse`, `deriveModelDiscoveryPending`, `isSuccessfulEmptyDiscovery`. If the "reopen to retry" copy becomes inert again, these tests will catch it. +- `ui/respondToFieldContract.test.mjs` — plain-language mode labels, the + persistent warning contract for shared agent access, and its two render + positions (after the people picker for `allowlist`). +- `lib/agentAccessWarning.test.mjs` — every mode × run-location copy variant + plus both resolvers, including unknown-reads-as-local and + blank-`runOn`-is-not-a-provider. - `desktop/tests/e2e/onboarding-agent-defaults.spec.ts` — onboarding behavior acceptance coverage for readiness, failure states, defaults, navigation, successful-empty vs failed optional-model discovery, and persistence races. diff --git a/desktop/src/features/agents/lib/agentAccessWarning.test.mjs b/desktop/src/features/agents/lib/agentAccessWarning.test.mjs new file mode 100644 index 0000000000..0a3d13ce7b --- /dev/null +++ b/desktop/src/features/agents/lib/agentAccessWarning.test.mjs @@ -0,0 +1,89 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +import { + agentAccessWarningText, + runLocationForBackend, + runLocationForRunOn, +} from "./agentAccessWarning.ts"; + +test("only the modes that share access warn", () => { + assert.equal(agentAccessWarningText("owner-only", "local"), null); + assert.ok(agentAccessWarningText("anyone", "local")); + assert.ok(agentAccessWarningText("allowlist", "local")); +}); + +test("a local agent names this computer and what is reachable on it", () => { + assert.equal( + agentAccessWarningText("anyone", "local"), + "Anyone can use this agent to access your computer, including files, accounts, and connected tools.", + ); + assert.equal( + agentAccessWarningText("allowlist", "local"), + "Selected people can use this agent to access your computer, including files, accounts, and connected tools.", + ); +}); + +test("a provider-backed agent names the server, and not the owner's files", () => { + // A remote host's files aren't the owner's to describe, so the tail narrows + // to the accounts and tools provisioned there. + assert.equal( + agentAccessWarningText("anyone", "remote"), + "Anyone can use this agent to access the server it runs on, including any accounts and tools available there.", + ); + assert.equal( + agentAccessWarningText("allowlist", "remote"), + "Selected people can use this agent to access the server it runs on, including any accounts and tools available there.", + ); + assert.doesNotMatch( + agentAccessWarningText("anyone", "remote"), + /your computer/, + ); +}); + +test("an unknown run location reads as local, not as a hedge", () => { + // "computer or server" names a concept most owners have never been shown: + // the Run on selector only renders when a buzz-backend-* provider exists. + for (const unknown of [undefined, null]) { + assert.equal( + agentAccessWarningText("anyone", unknown), + "Anyone can use this agent to access your computer, including files, accounts, and connected tools.", + ); + } +}); + +test("every variant leads with the audience and stays jargon-free", () => { + for (const mode of ["anyone", "allowlist"]) { + for (const runLocation of [null, "local", "remote"]) { + const text = agentAccessWarningText(mode, runLocation); + assert.match( + text, + /^(Anyone|Selected people) can use this agent to access/, + ); + assert.doesNotMatch(text, /respond-to|allowlist|pubkey|Nostr|harness/i); + } + } +}); + +test("runLocationForBackend maps the backend union", () => { + assert.equal(runLocationForBackend({ type: "local" }), "local"); + assert.equal( + runLocationForBackend({ type: "provider", id: "blox", config: {} }), + "remote", + ); + assert.equal(runLocationForBackend(null), null); + assert.equal(runLocationForBackend(undefined), null); +}); + +test("runLocationForRunOn treats a provider id as remote", () => { + assert.equal(runLocationForRunOn("local"), "local"); + assert.equal(runLocationForRunOn("blox"), "remote"); +}); + +test("runLocationForRunOn treats a blank value as unknown", () => { + // `runOn` is typed `"local" | string`, so a blank must not read as a + // provider id and produce the server wording. + assert.equal(runLocationForRunOn(""), null); + assert.equal(runLocationForRunOn(null), null); + assert.equal(runLocationForRunOn(undefined), null); +}); diff --git a/desktop/src/features/agents/lib/agentAccessWarning.ts b/desktop/src/features/agents/lib/agentAccessWarning.ts new file mode 100644 index 0000000000..d98058f5bf --- /dev/null +++ b/desktop/src/features/agents/lib/agentAccessWarning.ts @@ -0,0 +1,63 @@ +import type { ManagedAgentBackend, RespondToMode } from "@/shared/api/types"; + +/** + * Where an agent's process runs, as far as the calling surface can tell. + * + * Deliberately coarser than `ManagedAgentBackend`: the warning copy only needs + * to know "this machine" vs "somewhere else", so surfaces resolve their own + * backend shape down to this before handing it over. `null` means the surface + * genuinely cannot tell — see `agentAccessWarningText` for how that is + * treated. + */ +export type AgentRunLocation = "local" | "remote"; + +/** Resolve a running agent's backend record. `null` when the backend is unknown. */ +export function runLocationForBackend( + backend: ManagedAgentBackend | null | undefined, +): AgentRunLocation | null { + if (!backend) return null; + return backend.type === "local" ? "local" : "remote"; +} + +/** + * Resolve the create flow's `WhereToRunDraft.runOn`, which is `"local"` or a + * discovered provider id. An empty string is treated as unknown rather than as + * a provider, since `runOn` is typed `"local" | string`. + */ +export function runLocationForRunOn( + runOn: string | null | undefined, +): AgentRunLocation | null { + if (!runOn) return null; + return runOn === "local" ? "local" : "remote"; +} + +/** + * Copy for the shared-access warning in the respond-to field, or `null` for + * modes that share nothing. + * + * Both `anyone` and `allowlist` hand the host's access to someone other than + * the owner, so both warn; only the audience phrase differs. + * + * An unknown run location falls back to the same "your computer" wording as + * `local` rather than hedging with "computer or server". A remote host is only + * reachable when a `buzz-backend-*` provider binary is installed — without one + * `WhereToRunSection`'s "Run on" selector never renders and every agent is + * local — so hedging would name a concept most owners have never been shown. + * When it *is* remote the owner picked that host from the selector + * deliberately, so naming a server is meaningful there. + */ +export function agentAccessWarningText( + mode: RespondToMode, + runLocation?: AgentRunLocation | null, +): string | null { + if (mode !== "anyone" && mode !== "allowlist") return null; + const audience = mode === "anyone" ? "Anyone" : "Selected people"; + // The two locations differ in more than the noun: a local agent reaches the + // owner's own files, while a remote host's files aren't theirs to describe — + // only the accounts and tools provisioned there. + const target = + runLocation === "remote" + ? "the server it runs on, including any accounts and tools available there" + : "your computer, including files, accounts, and connected tools"; + return `${audience} can use this agent to access ${target}.`; +} diff --git a/desktop/src/features/agents/ui/AgentDialog.tsx b/desktop/src/features/agents/ui/AgentDialog.tsx index f5be3cc7e8..dc608da489 100644 --- a/desktop/src/features/agents/ui/AgentDialog.tsx +++ b/desktop/src/features/agents/ui/AgentDialog.tsx @@ -6,6 +6,11 @@ import type { ManagedAgent, UpdatePersonaInput, } from "@/shared/api/types"; +import { + runLocationForBackend, + runLocationForRunOn, +} from "../lib/agentAccessWarning"; +import { AgentRunLocationProvider } from "./AgentRunLocationContext"; import type { BackendIntent } from "../lib/instanceInputForDefinition"; import type { AgentCreateIntent } from "./agentCreateIntent"; import type { EditAgentFocusTarget } from "@/features/agents/openEditAgentEvent"; @@ -89,17 +94,25 @@ type AgentDialogProps = export function AgentDialog(props: AgentDialogProps) { if (props.mode === "instance-edit") { return ( - + // A running instance knows its own backend, so the respond-to warning can + // name the machine it will actually run on. + + + ); } if (props.mode === "definition-edit") { + // A definition has no instance and no run draft, so the run location stays + // unknown and the warning uses its local-wording fallback. const { mode: _mode, ...definitionProps } = props; return ; } @@ -124,35 +137,39 @@ function AgentCreateDialogRouter({ const copy = createPersonaDialogState(); return ( - - } - createSubmitBlocked={!canSubmitWhereToRun(runDraft)} - description={copy.description} - error={definitionError} - initialValues={initialValues} - isPending={isDefinitionPending} - onOpenChange={onOpenChange} - onSubmit={async (input) => { - const submitted = await onSubmitDefinition( - input, - "definition_start", - resolveBackendIntent(runDraft), - ); - if (submitted) { - onOpenChange(false); + // The create flow is the one surface that knows where the agent will run, + // because it owns the "Run on" draft. + + } - }} - open - runtimes={runtimes} - runtimesLoading={runtimesLoading} - submitLabel={copy.submitLabel} - title={copy.title} - /> + createSubmitBlocked={!canSubmitWhereToRun(runDraft)} + description={copy.description} + error={definitionError} + initialValues={initialValues} + isPending={isDefinitionPending} + onOpenChange={onOpenChange} + onSubmit={async (input) => { + const submitted = await onSubmitDefinition( + input, + "definition_start", + resolveBackendIntent(runDraft), + ); + if (submitted) { + onOpenChange(false); + } + }} + open + runtimes={runtimes} + runtimesLoading={runtimesLoading} + submitLabel={copy.submitLabel} + title={copy.title} + /> + ); } diff --git a/desktop/src/features/agents/ui/AgentInstanceEditDialog.tsx b/desktop/src/features/agents/ui/AgentInstanceEditDialog.tsx index f3c410e2ff..79d1e9a790 100644 --- a/desktop/src/features/agents/ui/AgentInstanceEditDialog.tsx +++ b/desktop/src/features/agents/ui/AgentInstanceEditDialog.tsx @@ -936,7 +936,7 @@ export function AgentInstanceEditDialog({ - {/* Who can talk to this agent */} + {/* Who can send instructions */} ( + null, +); + +export function AgentRunLocationProvider({ + children, + runLocation, +}: { + children: React.ReactNode; + runLocation: AgentRunLocation | null; +}) { + return ( + + {children} + + ); +} + +export function useAgentRunLocation(): AgentRunLocation | null { + return React.useContext(AgentRunLocationContext); +} diff --git a/desktop/src/features/agents/ui/RespondToField.tsx b/desktop/src/features/agents/ui/RespondToField.tsx index d9773ad681..b32b9e0983 100644 --- a/desktop/src/features/agents/ui/RespondToField.tsx +++ b/desktop/src/features/agents/ui/RespondToField.tsx @@ -1,5 +1,5 @@ import * as React from "react"; -import { ChevronDown, Search, X } from "lucide-react"; +import { AlertTriangle, ChevronDown, Search, X } from "lucide-react"; import { mergeAllowlist, parsePubkeyInput, @@ -13,6 +13,11 @@ import { cn } from "@/shared/lib/cn"; import { Input } from "@/shared/ui/input"; import { Textarea } from "@/shared/ui/textarea"; import { UserAvatar } from "@/shared/ui/UserAvatar"; +import { + type AgentRunLocation, + agentAccessWarningText, +} from "@/features/agents/lib/agentAccessWarning"; +import { useAgentRunLocation } from "./AgentRunLocationContext"; import { PersonaDropdownField } from "./PersonaDropdownField"; import type { PersonaDropdownOption } from "./agentConfigOptions"; @@ -20,14 +25,25 @@ import type { PersonaDropdownOption } from "./agentConfigOptions"; * Inbound author gate UI for create/edit agent dialogs. * * Dropdown: - * - Owner only (default; matches `buzz-acp --respond-to=owner-only`) - * - Anyone (`--respond-to=anyone` — fully open bot) - * - Allowlist (`--respond-to=allowlist`, plus the chip list as - * `--respond-to-allowlist`) + * - Only me (default; maps to `buzz-acp --respond-to=owner-only`) + * - Anyone (`--respond-to=anyone` — fully open agent) + * - Selected people (`--respond-to=allowlist`, plus the selected pubkeys as + * `--respond-to-allowlist`) * * `nobody` is intentionally not surfaced — it pairs with a heartbeat-only * setup that has no meaningful GUI use case. * + * Anyone and Selected people both share the host's access with someone other + * than the owner, so both render the persistent warning; only the audience + * phrase differs. It leads with the audience so it reads as a warning rather + * than an explanation, and stays one sentence — Only me already owns the line + * below the control. + * + * Which machine and stakes it names follow the optional `runLocation` prop, and + * an unknown location falls back to the local wording rather than hedging with + * "computer or server" — see `lib/agentAccessWarning.ts` for the copy and the + * reasoning. + * * Validation is duplicated lightly here for inline UX feedback only; the * authoritative validator is `validate_respond_to_allowlist` in * `desktop/src-tauri/src/managed_agents/types.rs`. @@ -53,7 +69,7 @@ function formatSearchUserSecondary(user: UserSearchResult) { const RESPOND_TO_OPTIONS: PersonaDropdownOption[] = [ { label: "Only me (default)", value: "owner-only" }, { label: "Anyone", value: "anyone" }, - { label: "Allowlist", value: "allowlist" }, + { label: "Selected people", value: "allowlist" }, ]; export function CreateAgentRespondToField({ @@ -64,6 +80,7 @@ export function CreateAgentRespondToField({ ownerPubkey, disabled, variant, + runLocation, }: { mode: RespondToMode; allowlist: string[]; @@ -78,6 +95,12 @@ export function CreateAgentRespondToField({ disabled?: boolean; /** When "persona", uses PersonaDropdownField styling to match the persona dialog. */ variant?: "default" | "persona"; + /** + * Where the agent's process runs, when the surface can tell. Omit or pass + * `null` when it can't — the warning then uses the same "your computer" + * wording as a local agent rather than hedging. Never synthesize a value. + */ + runLocation?: AgentRunLocation | null; }) { const [query, setQuery] = React.useState(""); const [isDirectEntryOpen, setIsDirectEntryOpen] = React.useState(false); @@ -132,6 +155,33 @@ export function CreateAgentRespondToField({ const isPersonaVariant = variant === "persona"; + // An explicit prop wins; otherwise inherit from the dialog subtree. Surfaces + // inside AgentDialog get it from context (see AgentRunLocationContext for + // why), standalone ones like EditRespondToDialog pass the prop. + const inheritedRunLocation = useAgentRunLocation(); + const warningText = agentAccessWarningText( + mode, + runLocation ?? inheritedRunLocation, + ); + + // Rendered in two positions: directly below the selector for Anyone, but + // after the people picker for Selected people, so it never sits between the + // user and the selection they came here to make. + const accessWarning = warningText ? ( +
+
+ ) : null; + return (
{isPersonaVariant ? ( onModeChange(e.target.value as RespondToMode)} value={mode} > - - - + {RESPOND_TO_OPTIONS.map((option) => ( + + ))} )} - {!isPersonaVariant ? ( + {mode === "anyone" ? accessWarning : null} + {mode === "owner-only" ? (

- Controls which Nostr authors the agent listens to (@mentions, DMs, - thread replies). The agent's owner can always shut it down with - !shutdown. + Only you can send instructions.

) : null} {mode === "allowlist" ? ( @@ -202,6 +253,7 @@ export function CreateAgentRespondToField({ variant={isPersonaVariant ? "persona" : "default"} /> ) : null} + {mode === "allowlist" ? accessWarning : null}
); } @@ -269,7 +321,7 @@ function AllowlistPicker({ > {!isPersona ? (
- Allowed pubkeys + Selected people {allowlist.length} selected @@ -277,13 +329,13 @@ function AllowlistPicker({ ) : null} {!isPersona && ownerPubkey ? (

- Owner ( - ) is always implicitly allowed by the - harness — no need to add it here. + You ( + ) can always use this agent. You + don't need to add yourself.

) : !isPersona ? (

- The agent's owner is always implicitly allowed. + You can always use this agent.

) : null}
@@ -452,7 +504,7 @@ function AllowlistPicker({ onClick={onAddFromPaste} type="button" > - Add to allowlist + Add people
diff --git a/desktop/src/features/agents/ui/agentDialogRouting.test.mjs b/desktop/src/features/agents/ui/agentDialogRouting.test.mjs index 196dceeef0..15b6b7cf87 100644 --- a/desktop/src/features/agents/ui/agentDialogRouting.test.mjs +++ b/desktop/src/features/agents/ui/agentDialogRouting.test.mjs @@ -4,6 +4,7 @@ import test from "node:test"; import { AgentDialog } from "./AgentDialog.tsx"; import { AgentDefinitionDialog } from "./AgentDefinitionDialog.tsx"; import { AgentInstanceEditDialog } from "./AgentInstanceEditDialog.tsx"; +import { AgentRunLocationProvider } from "./AgentRunLocationContext.tsx"; // ── Phase 1B.3c routing pinning ───────────────────────────────────────────── // @@ -54,8 +55,13 @@ test("instance-edit routes to AgentInstanceEditDialog with its contract props", open: true, }); - assert.equal(element.type, AgentInstanceEditDialog); - assert.deepEqual(element.props, { + // The arm wraps the form in the run-location provider so the respond-to + // warning can name the machine without the value being threaded as a prop + // through AgentInstanceEditDialog (see AgentRunLocationContext for why). + assert.equal(element.type, AgentRunLocationProvider); + const form = element.props.children; + assert.equal(form.type, AgentInstanceEditDialog); + assert.deepEqual(form.props, { agent, onEditLinkedPersona: undefined, onOpenChange, @@ -65,6 +71,25 @@ test("instance-edit routes to AgentInstanceEditDialog with its contract props", }); }); +test("instance-edit publishes the run location resolved from the agent backend", () => { + const routeWithBackend = (backend) => + AgentDialog({ + mode: "instance-edit", + agent: { pubkey: "abc", name: "test-agent", backend }, + onOpenChange: noop, + onUpdated: noop, + open: true, + }).props.runLocation; + + assert.equal(routeWithBackend({ type: "local" }), "local"); + assert.equal( + routeWithBackend({ type: "provider", id: "blox", config: {} }), + "remote", + ); + // An agent with no backend record has an unknown location — never a guess. + assert.equal(routeWithBackend(undefined), null); +}); + test("create mode routes to the internal create router, not a form directly", () => { const element = AgentDialog({ mode: "definition", diff --git a/desktop/src/features/agents/ui/respondToFieldContract.test.mjs b/desktop/src/features/agents/ui/respondToFieldContract.test.mjs new file mode 100644 index 0000000000..c3efd34650 --- /dev/null +++ b/desktop/src/features/agents/ui/respondToFieldContract.test.mjs @@ -0,0 +1,67 @@ +import assert from "node:assert/strict"; +import { readFile } from "node:fs/promises"; +import test from "node:test"; + +const respondToFieldSource = await readFile( + new URL("./RespondToField.tsx", import.meta.url), + "utf8", +); + +/** + * Copy assertions run against this rather than the raw source: JSX text wraps + * wherever the formatter decides, and a sentence split across lines should not + * fail a copy test. + */ +const collapsedSource = respondToFieldSource.replace(/\s+/g, " "); + +for (const label of ["Only me (default)", "Selected people", "Anyone"]) { + test(`respond-to control uses the plain-language label: ${label}`, () => { + assert.ok(respondToFieldSource.includes(`label: "${label}"`)); + }); +} + +test("native and persona controls share one option list", () => { + assert.match( + respondToFieldSource, + / \([\s\S]*