diff --git a/CHANGELOG.md b/CHANGELOG.md index 3400b3e7..776c5e1d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,33 @@ Newest first. `Unreleased` is what is on `main` and not yet tagged. ## Unreleased +### One Bot can hand work to another, and reach a person when no Bot will do + +A Bot asked something it is not the right Bot for can now put the question to one that is. The +addressed Bot answers **as itself, in its own conversation**, with its own tools and its own +knowledge. The asking Bot does not relay text on its behalf, so what you read is the answer that +Bot actually gave rather than another Bot's summary of it. The asking conversation records that the +question was put and to whom. A Bot that judges no other Bot will do can instead reach the person +who asked it. + +**No Bot may address any other until an administrator says so.** Which Bot may reach which is an +ordinary grant, made per Bot, and a Bot with no grant is told it cannot rather than quietly trying. +A Bot addressed by a name two Bots answer to is refused and both are named, because picking one +would be a guess about which colleague a person meant. + +Two ceilings, because a Bot deciding to ask another Bot is a Bot deciding to spend a run: +`BOT_HANDOFF_MAX_DEPTH` is how many Bots deep a chain may go and defaults to `1`, and **`0` switches +the capability off entirely**: the tool is not offered rather than offered and refused. +`BOT_HANDOFF_MAX_PER_RUN` is how many Bots one run may address and defaults to `3`. The Helm chart +takes the same two as `config.handoff.maxDepth` and `config.handoff.maxPerRun`. + +A hop that fails is reported back by the Bot that asked, after its attempts are spent, rather than +leaving the person watching a conversation that never finishes. One rough edge to know about: a hop +that is retried leaves one "asked" line per attempt in the addressed Bot's own transcript, so a hop +that took three attempts reads there as having been asked three times. + +No new tables: this uses the work queue that already fires the culler. + ### The framework Bot answers on 5.6-tier models, and can be told how hard to think Pointing `BOT_MODEL` at a `gpt-5.6-*` model gave a Bot that started, reported healthy, and then said @@ -194,8 +221,8 @@ A Helm chart under `charts/openbot`, Bots and all, and the fixes that installing up. Proven on a real EKS cluster: five workloads, replicas across two nodes, EBS volumes bound, and a Bot opening a real page from inside AWS with the decision in the audit trail. -One chart, four targets: EKS, GKE, AKS and somebody's own cluster, with nothing but values between -them. There is no cloud branching in any template. Every place the clouds genuinely differ is a +One chart, five targets: EKS with a shared browser, EKS with a computer for each Bot, GKE, AKS and +somebody's own cluster, with nothing but values between them. There is no cloud branching in any template. Every place the clouds genuinely differ is a value whose default is what a plain self-hosted cluster does: the cluster's own default StorageClass, no RuntimeClass, a plain Kubernetes Secret, an Ingress. Identity is one `serviceAccount.annotations` map, which is all IRSA, Workload Identity and AKS workload identity are. Secrets are a plain Secret diff --git a/app/src/components/agents/agent-profile.tsx b/app/src/components/agents/agent-profile.tsx index 3e3c0e9d..f74555d6 100644 --- a/app/src/components/agents/agent-profile.tsx +++ b/app/src/components/agents/agent-profile.tsx @@ -4,6 +4,7 @@ import { type ReactNode, useState } from "react"; import { AbstractAvatar } from "@/components/agents/abstract-avatar"; import { AgentFields } from "@/components/agents/agent-fields"; import { CallbackTokenPanel } from "@/components/agents/callback-token-panel"; +import { HandoffPanel } from "@/components/agents/handoff-panel"; import { Button } from "@/components/ui/button"; import { Separator } from "@/components/ui/separator"; import { Skeleton } from "@/components/ui/skeleton"; @@ -154,6 +155,13 @@ export function AgentProfile({ agentId }: { agentId: string }) { /> ) : null} + {/* + * Not while editing, for the same reason the panel above is not: the form owns the screen, and + * these switches write immediately rather than on save, which would make one half of an open + * form apply and the other half not. + */} + {isEditing ? null : } + {actionError ? (

{actionError.message} diff --git a/app/src/components/agents/handoff-panel.tsx b/app/src/components/agents/handoff-panel.tsx new file mode 100644 index 00000000..bc299f93 --- /dev/null +++ b/app/src/components/agents/handoff-panel.tsx @@ -0,0 +1,111 @@ +import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query"; +import { AbstractAvatar } from "@/components/agents/abstract-avatar"; +import { Switch } from "@/components/ui/switch"; +import { setHandoffGrantMutationOptions } from "@/lib/agents/mutations"; +import { + agentHandoffQueryOptions, + agentListQueryOptions, +} from "@/lib/agents/queries"; + +/** + * Which Bots this one may hand work to. + * + * On the Bot's own screen rather than in the connector catalogue: a catalogue entry has a fixed list + * of tools somebody else maintains, and the Bots a deployment has are whatever was made here. It is + * also the question a person asks while looking at a Bot, not while looking at a vendor. + * + * DIRECTIONAL, and said so on the screen, because the pair is the one thing about this that is easy + * to get backwards: this is who this Bot may ask, not who may ask it. + */ +export function HandoffPanel({ agentId }: { agentId: string }) { + const queryClient = useQueryClient(); + const handoff = useQuery(agentHandoffQueryOptions(agentId)); + const agents = useQuery(agentListQueryOptions()); + const setGrant = useMutation(setHandoffGrantMutationOptions(queryClient)); + + if (handoff.isPending || !handoff.data) return null; + const { enabled, canGrant, reachable } = handoff.data; + + /* + * A Bot may not be granted itself, and the server refuses it, so it is not offered here either. + * Hidden Bots are already absent from this list. + */ + const others = (agents.data ?? []).filter( + (candidate) => candidate.id !== agentId, + ); + + // Nothing to say to somebody who cannot change it and has nothing to read. + if (!canGrant && reachable.length === 0) return null; + + return ( +

+

+ Bots it may ask +

+ +

+ {enabled + ? "Work this Bot cannot do itself, it may hand to one of these. The Bot it asks answers in its own conversation, as itself." + : "Handing work between Bots is switched off for this deployment, so none of these takes effect until it is switched back on."} +

+ + {setGrant.error ? ( +

+ {setGrant.error.message} +

+ ) : null} + + {others.length === 0 ? ( +

+ There is no other Bot here to hand work to. +

+ ) : ( + + )} + + {canGrant ? null : ( +

+ An administrator decides which Bots may be asked. +

+ )} +
+ ); +} diff --git a/app/src/lib/agents/mutations.ts b/app/src/lib/agents/mutations.ts index b0b00436..64114638 100644 --- a/app/src/lib/agents/mutations.ts +++ b/app/src/lib/agents/mutations.ts @@ -113,3 +113,39 @@ export function revokeCallbackTokenMutationOptions(queryClient: QueryClient) { onSuccess: () => invalidateAgents(queryClient), }); } + +/** + * Whether one Bot may hand work to another. + * + * The same `plugin_grants` write every other grant makes, with `kind: "bot"`, so the audit row and + * the refusals are the ones already in place: an administrator only, never a Bot on itself, and + * never onto a Bot that does not exist. + * + * DIRECTIONAL, and the two ids are easy to swap: `agentId` is the Bot doing the asking and `ref` is + * the Bot it may reach. Granted the other way round it reads as working and hands over nothing. + */ +export function setHandoffGrantMutationOptions(queryClient: QueryClient) { + return mutationOptions({ + mutationFn: async (variables: { + /** The Bot doing the asking. */ + agentId: string; + /** The Bot it may reach. */ + ref: string; + granted: boolean; + }) => { + if (variables.granted) { + await client("/api/plugins/grants", { + method: "POST", + body: { kind: "bot", ref: variables.ref, agentId: variables.agentId }, + fallback: FALLBACK, + }); + return; + } + await client( + `/api/plugins/grants?kind=bot&ref=${encodeURIComponent(variables.ref)}&agentId=${encodeURIComponent(variables.agentId)}`, + { method: "DELETE", fallback: FALLBACK }, + ); + }, + onSuccess: () => invalidateAgents(queryClient), + }); +} diff --git a/app/src/lib/agents/queries.ts b/app/src/lib/agents/queries.ts index ce276d69..eadc2e20 100644 --- a/app/src/lib/agents/queries.ts +++ b/app/src/lib/agents/queries.ts @@ -43,6 +43,22 @@ export const agentKeys = { all: ["agents"] as const, list: (hidden = false) => ["agents", "list", { hidden }] as const, detail: (agentId: string) => ["agents", "detail", agentId] as const, + handoff: (agentId: string) => ["agents", "handoff", agentId] as const, +}; + +/** Which Bots one Bot may hand work to, and whether this deployment lets it. */ +export type HandoffGrants = { + /** + * Whether the capability is switched on at all. + * + * Separate from the grants because the two fail differently: with this false, a grant is a row + * nothing will ever read, so the screen says so rather than offering a switch wired to nothing. + */ + enabled: boolean; + /** Whether the signed-in person may change any of it. Granting is an administrator's. */ + canGrant: boolean; + /** Bot ids this Bot may address today. */ + reachable: string[]; }; export function agentListQueryOptions(hidden = false) { @@ -65,6 +81,16 @@ export function agentQueryOptions(agentId: string) { }); } +export function agentHandoffQueryOptions(agentId: string) { + return queryOptions({ + queryKey: agentKeys.handoff(agentId), + queryFn: (): Promise => + client(`/api/agents/${agentId}/handoff`, "handoff", { + fallback: "Could not load which Bots this one may ask", + }), + }); +} + /** What the server said when it tried the endpoint. */ export type ConnectionVerdict = | { ok: true; events: string[] } diff --git a/app/src/lib/copilot/repair-history.ts b/app/src/lib/copilot/repair-history.ts index ad8f3e66..d8a04a3a 100644 --- a/app/src/lib/copilot/repair-history.ts +++ b/app/src/lib/copilot/repair-history.ts @@ -16,9 +16,29 @@ function isToolResult(message: Message): message is Message & ToolResult { } /** - * The same messages, with a result inserted for any tool call that has none. + * The same messages, with every tool call answered by a result that FOLLOWS it. * * Returns the original array when no repair is needed. + * + * POSITION IS THE WHOLE POINT, and it was the half this function did not check. It decided which + * calls were answered by collecting every tool result in the array regardless of where it sat, so a + * result stored BEFORE its own call marked that call answered and the array was returned untouched. + * A provider matches a result to the call above it, so what it saw was a call with nothing after it: + * `AI_MissingToolResultsError`, thrown while converting the prompt, which fails the whole + * conversation rather than one turn. Every later message in that channel then failed the same way, + * including "what is 2 plus 2" — the conversation was dead for good and said only that a tool result + * was missing. + * + * Threads really are stored that way. Read back from the platform after a Bot handed work to another + * Bot, the result sat three messages ahead of the call that produced it, and the same inversion held + * for `ask_person` and for an MCP tool call; only the computer's own tools came back in order. So + * this is not a hypothetical ordering: it is what a transcript looks like after using the features + * this release is named for. + * + * An early result is MOVED rather than replaced, because it is the real one — "Handed to Knowledge" + * says more than this function's apology ever could. Only a call with no result anywhere gets the + * sentence below. A result whose call never appears at all is dropped: it answers nothing, and a + * provider rejects it for the same reason it rejects the mirror image. */ export function repairUnansweredToolCalls( messages: ReadonlyArray, @@ -29,9 +49,28 @@ export function repairUnansweredToolCalls( */ mintId: () => string = newId, ): ReadonlyArray { + const called = new Set(); + /** Answered where a provider can see it: by a result later in the array than the call. */ const answered = new Set(); + /** A real result that arrived before its own call, kept so it can be put back in the right place. */ + const early = new Map(); + /** Results that answer nothing where they sit, and so must not be sent where they sit. */ + const misplaced = new Set(); + for (const message of messages) { - if (isToolResult(message)) answered.add(message.toolCallId); + if (message.role === "assistant") { + for (const call of message.toolCalls ?? []) called.add(call.id); + continue; + } + if (!isToolResult(message)) continue; + const id = message.toolCallId; + if (called.has(id) && !answered.has(id)) { + answered.add(id); + continue; + } + // Before its call, or a second result for a call already answered, or answering no call at all. + if (!early.has(id)) early.set(id, message); + misplaced.add(message); } const missing = messages.some( @@ -39,26 +78,32 @@ export function repairUnansweredToolCalls( message.role === "assistant" && (message.toolCalls ?? []).some((call) => !answered.has(call.id)), ); - if (!missing) return messages; + if (!missing && misplaced.size === 0) return messages; const repaired: Message[] = []; + const filled = new Set(answered); for (const message of messages) { + if (misplaced.has(message)) continue; repaired.push(message); if (message.role !== "assistant") continue; for (const call of message.toolCalls ?? []) { - if (answered.has(call.id)) continue; + if (filled.has(call.id)) continue; // Immediately after the assistant message that made the call, and before any later message: // OpenAI requires the results to follow their calls, and some providers require the order to // match the `tool_calls` array as well. - repaired.push({ - id: mintId(), - role: "tool", - toolCallId: call.id, - content: UNANSWERED, - } as Message); + const moved = early.get(call.id); + repaired.push( + moved ?? + ({ + id: mintId(), + role: "tool", + toolCallId: call.id, + content: UNANSWERED, + } as Message), + ); // A duplicated call id may only receive one repair result. - answered.add(call.id); + filled.add(call.id); } } diff --git a/app/src/routes/_authed/_app/agents/index.tsx b/app/src/routes/_authed/_app/agents/index.tsx index 118ea7f4..aa4b4216 100644 --- a/app/src/routes/_authed/_app/agents/index.tsx +++ b/app/src/routes/_authed/_app/agents/index.tsx @@ -25,6 +25,15 @@ export const Route = createFileRoute("/_authed/_app/agents/")({ component: AgentsScreen, }); +/* + * The roster wraps on the width it actually has, not on the window's. + * + * A card is a fixed 144px, so four fixed columns overlap the moment the column they sit in is + * narrower than the card. That is not a narrow-window case: opening the detail pane takes the width + * out of this column at any window size, so the cards behind an open Bot overlapped each other on a + * perfectly ordinary screen. `auto-fill` tracks the container instead, which is the thing that + * actually changed. + */ function AgentsScreen() { const { new: isCreating, agent: selectedAgentId } = Route.useSearch(); const navigate = Route.useNavigate(); @@ -66,7 +75,7 @@ function AgentsScreen() {
{!!mine?.length && ( -
+
{mine.map((agent, index) => { return ( @@ -91,7 +100,7 @@ function AgentsScreen() {

Explore agents

-
+
{!!explore?.length && explore.map((agent, index) => { return ( diff --git a/app/src/routes/_authed/_app/bot.tsx b/app/src/routes/_authed/_app/bot.tsx index b29f1322..7eed66aa 100644 --- a/app/src/routes/_authed/_app/bot.tsx +++ b/app/src/routes/_authed/_app/bot.tsx @@ -31,7 +31,8 @@ function RouteComponent() { const { agent } = Route.useSearch(); const { data: agents, isPending } = useQuery(agentListQueryOptions()); const agentId = agent ?? agents?.[0]?.id; - const known = agents?.some((candidate) => candidate.id === agentId) ?? false; + const bot = agents?.find((candidate) => candidate.id === agentId); + const known = bot !== undefined; if (isPending) return null; if (!agentId || !known) { @@ -50,10 +51,10 @@ function RouteComponent() { * Keyed on the Bot, so the hooks below never see it change under them. They cannot be called * conditionally, and the guards above return before any of them run. */ - return ; + return ; } -function BotChat({ agentId }: { agentId: string }) { +function BotChat({ agentId, name }: { agentId: string; name: string }) { // Tool calls here act on this Bot's own computer. useActiveBot(agentId); /* @@ -77,7 +78,13 @@ function BotChat({ agentId }: { agentId: string }) {
-

Browser Bot

+ {/* + * The Bot this screen is actually showing. A name written into the markup is wrong on + * every deployment whose package did not happen to use it, which is the same defect the + * route default above was fixed for: this screen called whichever Bot you opened + * "Browser Bot", including the one named something else two lines of state away. + */} +

{name}

{/* * Labelled rather than the bare icon button the sidebar uses for its own "start * something new" control: that one opens an empty screen, but this one throws away diff --git a/app/tests/repair-history.test.ts b/app/tests/repair-history.test.ts index 3ee972cf..f4030c16 100644 --- a/app/tests/repair-history.test.ts +++ b/app/tests/repair-history.test.ts @@ -114,6 +114,62 @@ describe("repairing a history before it is sent", () => { expect(results).toHaveLength(1); }); + /* + * The shape a real thread came back in after one Bot handed work to another: the result three + * messages AHEAD of the call that produced it. Collecting results without regard to position + * called that call answered, returned the array untouched, and the provider then refused the whole + * conversation because nothing followed the call. + */ + test("a result stored before its own call is moved after it", () => { + const messages = [ + { id: "u1", role: "user", content: "ask the other Bot" }, + { + id: "t1", + role: "tool", + toolCallId: "c1", + content: "Handed to Knowledge.", + }, + { id: "a1", role: "assistant", content: "I asked it." }, + { + id: "a2", + role: "assistant", + content: "", + toolCalls: [call("c1", "message_bot")], + }, + { id: "u2", role: "user", content: "what is 2 plus 2" }, + ] as Message[]; + + const repaired = repairUnansweredToolCalls(messages, ids); + + expect(repaired.map((message) => message.role)).toEqual([ + "user", + "assistant", + "assistant", + "tool", + "user", + ]); + const result = repaired[3] as Message & { toolCallId: string }; + expect(result.toolCallId).toBe("c1"); + // The real result is moved, not thrown away and apologised for. + expect(result.content).toBe("Handed to Knowledge."); + }); + + test("a result whose call never appears is dropped", () => { + const messages = [ + { id: "u1", role: "user", content: "hello" }, + { id: "t1", role: "tool", toolCallId: "nobody", content: "orphan" }, + { id: "a1", role: "assistant", content: "hi" }, + ] as Message[]; + + const repaired = repairUnansweredToolCalls(messages, ids); + + // A result answering no call is refused by a provider for the mirror-image reason. + expect(repaired.map((message) => message.role)).toEqual([ + "user", + "assistant", + ]); + }); + test("a conversation with no tool calls at all is untouched", () => { const messages = [ { id: "u1", role: "user", content: "hello" }, diff --git a/charts/openbot/README.md b/charts/openbot/README.md index 9b8f99aa..d778f1f6 100644 --- a/charts/openbot/README.md +++ b/charts/openbot/README.md @@ -1,8 +1,109 @@ # OpenBot on Kubernetes -Runs OpenBot on any Kubernetes cluster: EKS, GKE, AKS, or your own. One chart, four targets, and the +Runs OpenBot on any Kubernetes cluster: EKS, GKE, AKS, or your own. One chart, five targets, and the only difference between them is values. +## What a cluster needs first + +Four things this chart assumes and does not create. + +**An image the cluster can pull.** A release publishes `ghcr.io/copilotkit/openbot:vX.Y.Z` +publicly, and that tag is what `image.tag` wants. It is built for **`linux/amd64` only**, so an +arm64 node group (Graviton on EKS, Tau T2A on GKE, Ampere on AKS) cannot run it: the pods sit in +`ImagePullBackOff`, which is the same thing a wrong tag or a missing pull secret looks like, so the +node pool being the wrong shape is the last thing anybody checks. Either run amd64 nodes, or build the image for the +architecture you have and push it somewhere the cluster can reach. Check before assuming: + +```sh +docker manifest inspect ghcr.io/copilotkit/openbot:v0.0.4 | grep architecture +``` + +**Intelligence credentials.** OpenBot requires CopilotKit Intelligence and the chart refuses to +install without `secrets.intelligenceApiKey` and `secrets.licenseToken`. Both come from the CLI, on +any machine with a browser: + +```sh +npx --yes copilotkit@latest login # browser sign-in +npx --yes copilotkit@latest project select # prints the cpk-... runtime key +npx --yes copilotkit@latest license --print # prints the licence token +``` + +`--print` rather than `--write` here: `--write` puts the token in a local `.env`, which is what a +laptop wants and not what you are about to paste into a Secret. The free plan is enough to install. + +**A default StorageClass**, or a named one. Both a Bot's computer and the bundled database ask for +a volume, and a fresh cluster often has no class marked default. See +[Check for a default StorageClass first](#check-for-a-default-storageclass-first), which is the +single most common reason a first install comes up with a pod stuck `Pending` and nothing saying +why. + +**A database, and the Secret that names it**, unless you are using the bundled one. The chart reads +a URL out of a Secret you make; it never writes your database credentials into a values file: + +```sh +kubectl create namespace openbot +kubectl -n openbot create secret generic openbot-database \ + --from-literal=database-url='postgresql://USER:PASSWORD@HOST:5432/openbot?sslmode=require' +``` + +Then `--set database.existingSecret=openbot-database`. The key must be `database-url`, or name a +different one with `database.existingSecretKey`. See +[Your own database](#your-own-database-which-is-what-a-real-deployment-uses) for `sslmode` and the +`vector` extension, both of which a managed database will otherwise fail on in a way that names the +wrong problem. + +### A cluster from nothing, on EKS + +The three above, as one config and two commands. `eksctl` creates `gp2` and does not mark it +default, and the provisioner it names is the in-tree one current Kubernetes no longer has, so the +StorageClass below is not optional. + +```yaml +# cluster.yaml +apiVersion: eksctl.io/v1alpha5 +kind: ClusterConfig +metadata: { name: openbot, region: us-east-2, version: "1.34" } +iam: { withOIDC: true } +addons: + - name: vpc-cni + - name: coredns + - name: kube-proxy + - name: metrics-server + # Last to be created, because it needs the OIDC provider that needs the control plane. Let it + # finish; creating the same addon by hand while this is running fails the cluster create. + - name: aws-ebs-csi-driver + wellKnownPolicies: { ebsCSIController: true } +managedNodeGroups: + - name: workers + # amd64: the published image has no arm64 variant. See the image note above. + instanceType: t3.large + desiredCapacity: 2 + minSize: 2 + maxSize: 4 + volumeSize: 60 + volumeType: gp3 +``` + +```sh +eksctl create cluster -f cluster.yaml + +kubectl apply -f - <<'EOF' +apiVersion: storage.k8s.io/v1 +kind: StorageClass +metadata: + name: gp3 + annotations: { storageclass.kubernetes.io/is-default-class: "true" } +provisioner: ebs.csi.aws.com +volumeBindingMode: WaitForFirstConsumer +allowVolumeExpansion: true +parameters: { type: gp3 } +EOF +``` + +The database goes in the same VPC, in the private subnets, with a security group admitting 5432 +from the cluster's own security group. `aws eks describe-cluster` names both. Keep it +`--no-publicly-accessible`: the only thing that needs to reach it is in the cluster. + ## Install The bundled database and one administrator, which is the shortest thing that works: @@ -60,7 +161,7 @@ user and not the actual problem. creates it and a later one drops it again. On a managed database, create it once as the administrative role; `CREATE EXTENSION IF NOT EXISTS` then passes for an ordinary user. -## The four targets +## The five targets `ci/` holds a values file per target, and each is the shortest thing that expresses what is different about that cluster: @@ -69,6 +170,7 @@ about that cluster: | --- | --- | | `self-hosted-values.yaml` | Nothing turned on. If this file needs to grow, a default is wrong. | | `eks-values.yaml` | IRSA, Secrets Manager, ALB, zone spread, autoscaling. | +| `eks-sandbox-values.yaml` | The same, with a computer each rather than one shared browser. `shared` and `sandbox` render a different Deployment, different RBAC and a different pod template, so a target that renders only one checks half the chart. | | `gke-values.yaml` | Workload Identity, Secret Manager, Gateway API instead of an Ingress. | | `aks-values.yaml` | Workload identity, Key Vault, the AKS web app routing class. | @@ -260,5 +362,8 @@ install rather than found as a browser that fails on every page. Migrations run as a `pre-install,pre-upgrade` Job, so no replica ever serves in front of a schema it has not seen. An init container would mean every replica racing to migrate the same database. -Use `helm upgrade --install --atomic` so a failed upgrade rolls back rather than leaving half a -rollout. +Roll back a failed upgrade rather than leaving half a rollout: `helm upgrade --install --atomic` on +Helm 3, and `--rollback-on-failure` on Helm 4, which renamed the flag. Helm 4 still accepts +`--atomic` on `upgrade` as a deprecated alias and prints a warning, so the Helm 3 spelling keeps +working on both today; it is `helm install --atomic` that Helm 4 removed outright, which is one more +reason this is written as `upgrade --install`. diff --git a/charts/openbot/ci/aks-values.yaml b/charts/openbot/ci/aks-values.yaml index de50c308..1058f8c3 100644 --- a/charts/openbot/ci/aks-values.yaml +++ b/charts/openbot/ci/aks-values.yaml @@ -2,8 +2,8 @@ config: initialAdminEmails: admin@example.com intelligence: - apiUrl: https://api.cloud.copilotkit.ai - gatewayWsUrl: wss://gateway.cloud.copilotkit.ai + apiUrl: https://api.intelligence.copilotkit.ai + gatewayWsUrl: wss://realtime.intelligence.copilotkit.ai auth: google: clientId: example.apps.googleusercontent.com diff --git a/charts/openbot/ci/eks-sandbox-values.yaml b/charts/openbot/ci/eks-sandbox-values.yaml index 2a7335a1..95d860a7 100644 --- a/charts/openbot/ci/eks-sandbox-values.yaml +++ b/charts/openbot/ci/eks-sandbox-values.yaml @@ -20,8 +20,8 @@ config: initialAdminEmails: admin@example.com intelligence: - apiUrl: https://api.cloud.copilotkit.ai - gatewayWsUrl: wss://gateway.cloud.copilotkit.ai + apiUrl: https://api.intelligence.copilotkit.ai + gatewayWsUrl: wss://realtime.intelligence.copilotkit.ai auth: google: clientId: example.apps.googleusercontent.com diff --git a/charts/openbot/ci/eks-values.yaml b/charts/openbot/ci/eks-values.yaml index 737a3c60..460caaa9 100644 --- a/charts/openbot/ci/eks-values.yaml +++ b/charts/openbot/ci/eks-values.yaml @@ -18,8 +18,8 @@ config: initialAdminEmails: admin@example.com intelligence: - apiUrl: https://api.cloud.copilotkit.ai - gatewayWsUrl: wss://gateway.cloud.copilotkit.ai + apiUrl: https://api.intelligence.copilotkit.ai + gatewayWsUrl: wss://realtime.intelligence.copilotkit.ai auth: google: clientId: example.apps.googleusercontent.com diff --git a/charts/openbot/ci/gke-values.yaml b/charts/openbot/ci/gke-values.yaml index b2531e0e..701e9141 100644 --- a/charts/openbot/ci/gke-values.yaml +++ b/charts/openbot/ci/gke-values.yaml @@ -2,8 +2,8 @@ config: initialAdminEmails: admin@example.com intelligence: - apiUrl: https://api.cloud.copilotkit.ai - gatewayWsUrl: wss://gateway.cloud.copilotkit.ai + apiUrl: https://api.intelligence.copilotkit.ai + gatewayWsUrl: wss://realtime.intelligence.copilotkit.ai auth: google: clientId: example.apps.googleusercontent.com diff --git a/charts/openbot/ci/self-hosted-values.yaml b/charts/openbot/ci/self-hosted-values.yaml index 688d4570..228dc70e 100644 --- a/charts/openbot/ci/self-hosted-values.yaml +++ b/charts/openbot/ci/self-hosted-values.yaml @@ -6,8 +6,8 @@ config: initialAdminEmails: admin@example.com intelligence: - apiUrl: https://api.cloud.copilotkit.ai - gatewayWsUrl: wss://gateway.cloud.copilotkit.ai + apiUrl: https://api.intelligence.copilotkit.ai + gatewayWsUrl: wss://realtime.intelligence.copilotkit.ai auth: google: clientId: example.apps.googleusercontent.com diff --git a/charts/openbot/values.yaml b/charts/openbot/values.yaml index aeb379cf..717bbb46 100644 --- a/charts/openbot/values.yaml +++ b/charts/openbot/values.yaml @@ -359,6 +359,9 @@ secrets: modelApiKey: "" computerToken: "" supervisorToken: "" + # Both from the CopilotKit CLI: `npx --yes copilotkit@latest project select` prints the runtime + # key, and `npx --yes copilotkit@latest license --print` prints the licence. The chart refuses to + # install without them, because there is no mode where this runs with Intelligence missing. intelligenceApiKey: "" licenseToken: "" # Sent to `config.managedAgent.url` on every call. Required when that url is set. diff --git a/docs/README.md b/docs/README.md index 9ef2cf6e..6b7c6668 100644 --- a/docs/README.md +++ b/docs/README.md @@ -11,6 +11,7 @@ Start with the root [README](../README.md), then use these references: - [Google Drive](plugins/google-drive.md) - [Notion](plugins/notion.md) - [Deployment](deployment.md): the container, what is in the image, minimum sizes, and the platform notes. +- [Kubernetes](../charts/openbot/README.md): the Helm chart, what a cluster needs before it, and the values that differ per cloud. - [Releasing](releasing.md): how a release is proposed, reviewed and published. Do not include credential values, customer data, transcripts, or local-only notes in public docs. diff --git a/docs/configuration.md b/docs/configuration.md index 1593ca5d..e9ec67d1 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -204,7 +204,14 @@ Both refuse rather than truncate, and both are refused at start-up if they are n zero or more: a deployment that typed `two` and silently got the default would believe it had set a cap. -Which Bots may address which is a grant, not a variable. It is made per Bot like any other grant. +Which Bots may address which is a grant, not a variable, and no Bot may address any other until one +is made. It is made on the Bot's own screen: open it from **Agents**, and switch on each Bot under +**Bots it may ask**. The pair is directional: that list is who this Bot may ask, not who may ask it, +so letting them ask each other is two switches. Only an administrator may change it; anyone who can +see the Bot can read it. + +With both caps above at zero the screen says the capability is switched off, because a grant made +then is a row nothing will read. ## Computer and supervisor diff --git a/docs/deployment.md b/docs/deployment.md index da5b23ee..827731a9 100644 --- a/docs/deployment.md +++ b/docs/deployment.md @@ -55,7 +55,7 @@ Measured on the real image, one Bot, arm64. | --- | --- | --- | --- | | Memory | 409 MB idle, 498 MB after three page loads, 548 MB after a snapshot | **2 GB** | **4 GB** | | vCPU | 3 to 6 percent at rest, bursty while a page renders | **1** | **2** | -| Disk | 5.3 GB image | **8 GB** | 10 GB with room for `/workspace` | +| Disk | 1.4 GB image | **4 GB** | 8 GB with room for `/workspace` | **Why 2 GB when it measures at 550 MB.** That figure is one Bot with one page open. Every additional concurrent page is roughly another 100 to 200 MB, and Playwright's own guidance is to allow about @@ -132,6 +132,12 @@ in ECR, and is what AWS points App Runner users at now that App Runner takes no Plain ECS on Fargate behind an ALB is the answer if you want task definitions and fine-grained IAM. No shared-memory configuration is needed or possible. +**Kubernetes.** Everything above describes one container run by hand. A cluster is the other shape, +and it is the only one that gives a Bot a computer of its own, runs the routines schedule without +something outside the container, and scales the API past a single replica. That is the Helm chart: +[charts/openbot/README.md](../charts/openbot/README.md), which covers EKS, GKE, AKS and a plain +self-hosted cluster from the same templates. + **Azure Container Apps.** Managed ingress with TLS and custom domains. Note the **240-second request timeout**: the live screen holds a long connection, so expect it to reconnect. Concurrent WebSockets are capped at 350 per instance on the basic tier. @@ -141,8 +147,8 @@ which makes them the shortest path from nothing to a running deployment. ## Known costs -**The image is 5.3 GB**, most of it the Playwright base, which ships Firefox and WebKit alongside the -Chromium we use. Deleting them afterwards does not help, because the bytes still ship in the layer +**The image is 1.4 GB**, and 595 MB of that is Firefox and WebKit, which the Playwright base ships +alongside the Chromium we use and nothing here ever launches. Deleting them afterwards does not help, because the bytes still ship in the layer below. Building Chromium-only onto a slim base would cut this substantially and is not done yet. **A strict content-security-policy needs a hash or a nonce.** `app/index.html` runs a small inline diff --git a/scripts/check-new-values-keys.ts b/scripts/check-new-values-keys.ts index 678db3f3..f87538c1 100644 --- a/scripts/check-new-values-keys.ts +++ b/scripts/check-new-values-keys.ts @@ -15,7 +15,7 @@ * * bun scripts/check-new-values-keys.ts [--since v0.0.4] */ -import { parse } from "yaml"; +import { parse, parseAllDocuments } from "yaml"; const [valuesFile, ...rest] = process.argv.slice(2); if (!valuesFile) { @@ -246,9 +246,10 @@ function renderedValue(out: string, variable: string): string | undefined { * runs in a job with no Helm — and a test that shells out to a binary which is not there returns * undefined rather than failing. */ -const chartValues = parse( - await Bun.file("charts/openbot/values.yaml").text(), -) as { config?: { handoff?: Record } }; +const rawChartValues = await Bun.file("charts/openbot/values.yaml").text(); +const chartValues = parse(rawChartValues) as { + config?: { handoff?: Record }; +}; const absent = render(["--set", "config.handoff=null"]); for (const { path, variable } of offSwitches) { const leaf = path.slice(path.lastIndexOf(".") + 1); @@ -263,6 +264,130 @@ for (const { path, variable } of offSwitches) { console.log(`${variable} falls back to ${got}, as values.yaml says`); } } +/** + * What has to be switched on for the workload carrying this fallback to render at all. + * + * The culler only needs the sandbox mode it belongs to. Routines additionally need the credential + * the worker presents, and WHERE that lives differs per target: three of the five read secrets from + * a cloud store, where naming `secrets.workerSharedSecret` is not enough and `externalSecrets.data` + * has to name it too. Appended after whatever the target already declares rather than turning the + * store off, so this still renders the target as shipped. + */ +const targetValues = parse(await Bun.file(valuesFile).text()) as { + externalSecrets?: { enabled?: boolean; data?: unknown[] }; +}; +function enableFor(component: string): string[] { + if (component === "culler") return ["--set", "computers.mode=sandbox"]; + const on = ["--set", "routines.enabled=true"]; + if (!targetValues.externalSecrets?.enabled) { + return [ + ...on, + "--set-string", + "secrets.workerSharedSecret=for-rendering-only", + ]; + } + const next = targetValues.externalSecrets.data?.length ?? 0; + return [ + ...on, + "--set", + `externalSecrets.data[${next}].secretKey=worker-shared-secret`, + "--set", + `externalSecrets.data[${next}].remoteRef.key=openbot/worker-shared-secret`, + ]; +} + +/** One step of a dotted path through parsed YAML, without asserting a shape it may not have. */ +function at(value: unknown, key: string): unknown { + return value !== null && typeof value === "object" + ? (value as Record)[key] + : undefined; +} + +/** What sits at a dotted path, or undefined if any step of it is missing. */ +function valueAt(root: unknown, path: readonly string[]): unknown { + return path.reduce((here, key) => at(here, key), root); +} + +/* + * The same assertion for the fallbacks that are not env vars. + * + * `offSwitches` above reads a rendered `name:`/`value:` pair, so it can only see a fallback that + * reaches a container's environment. Two do not: the routines schedule and the culler's deadline are + * fields on a CronJob. They were left unchecked as "no drift test yet" and the routines schedule had + * already been wrong once — `* * * * *`, five times more often than anything documents — which is + * the whole argument for asserting it rather than trusting the two comments that describe it. + * + * Nulling the LEAF, not the parent, because that is the shape `--reuse-values` actually produces + * here: an existing release carries `routines.enabled` from the release that introduced it and + * simply has no `schedule` key, so nulling the parent would delete the feature and render nothing + * to assert against. + */ +const fieldFallbacks: ReadonlyArray<{ + /** The values key, as `--set` names it. */ + path: string; + /** The component label on the workload that carries the field. */ + component: string; + /** Where the field sits on the rendered resource. */ + field: readonly string[]; +}> = [ + { + path: "routines.schedule", + component: "routines", + field: ["spec", "schedule"], + }, + { + path: "computers.sandbox.culler.activeDeadlineSeconds", + component: "culler", + field: ["spec", "jobTemplate", "spec", "activeDeadlineSeconds"], + }, +]; + +const chartValuesTree = parse(rawChartValues) as unknown; + +for (const { path, component, field } of fieldFallbacks) { + const documented = valueAt(chartValuesTree, path.split(".")); + const attempt = render([...enableFor(component), "--set", `${path}=null`]); + if (!attempt.ok) { + console.error( + `::error::The chart failed to render with ${path} absent: ${attempt.err.trim().split("\n")[0]}`, + ); + bad += 1; + continue; + } + /* + * Found by its component label rather than by name, because a name is the release name plus a + * suffix and this check would then be pinned to both. + */ + const carriers = parseAllDocuments(attempt.out) + .map((document) => document.toJS() as unknown) + .filter( + (resource) => + valueAt(resource, [ + "metadata", + "labels", + "app.kubernetes.io/component", + ]) === component, + ); + if (carriers.length !== 1) { + console.error( + `::error::Rendering with ${path} absent produced ${carriers.length} workloads labelled ${component}, not one.`, + ); + bad += 1; + continue; + } + const got = valueAt(carriers[0], field); + if (String(got) !== String(documented)) { + console.error( + `::error::With ${path} absent, the ${component} workload rendered ${JSON.stringify(got)} but values.yaml documents ${JSON.stringify(documented)}.`, + ); + bad += 1; + continue; + } + console.log( + `${path} falls back to ${JSON.stringify(got)}, as values.yaml says`, + ); +} + for (const { path, variable } of offSwitches) { const attempt = render(["--set", `${path}=0`]); if (!attempt.ok) { diff --git a/server/src/agents/handoff-runner.ts b/server/src/agents/handoff-runner.ts index 8d57fec3..1950e322 100644 --- a/server/src/agents/handoff-runner.ts +++ b/server/src/agents/handoff-runner.ts @@ -355,6 +355,9 @@ export function createHandoffRunner(options: { targetId: work.toBotId, ...(work.actorId ? { actorUserId: work.actorId } : {}), payload: { + // See the same key on `agent.handoff_offered`: the Audit screen's Bot column reads + // `payload.bot`, so a row without it names no Bot. + bot: work.fromBotId, from: work.fromBotId, to: work.toBotId, run: work.runId, diff --git a/server/src/agents/handoff.ts b/server/src/agents/handoff.ts index ccb1c75e..0e23818c 100644 --- a/server/src/agents/handoff.ts +++ b/server/src/agents/handoff.ts @@ -383,6 +383,11 @@ export function createHandoffDesk(options: { targetId: found.id, ...(from.actorId ? { actorUserId: from.actorId } : {}), payload: { + // The Bot that did this, under the key the Audit screen reads for its Bot column. `from` + // below says the same thing and is what the payload is read by, but the screen renders + // `payload.bot` and nothing else, so without this the two handoff rows are the only Bot + // actions on a screen headed "Every action a Bot took" that name no Bot. + bot: from.botId, from: from.botId, to: found.id, run: from.runId, diff --git a/server/src/agents/routes.ts b/server/src/agents/routes.ts index 50cb872c..060e77af 100644 --- a/server/src/agents/routes.ts +++ b/server/src/agents/routes.ts @@ -142,6 +142,23 @@ export function createAgentRoutes( * address. A hosted deployment sets this and leaves the other off. */ allowedHosts: ReadonlySet = new Set(), + /** + * Which Bots a Bot may hand work to, for the screen that grants it. + * + * A named object rather than another positional argument: every parameter above this one is + * optional, so a misplaced one typechecks and silently does nothing, and this list is already at + * the length where that stops being hypothetical. + * + * Absent in a deployment with no plugin store, which is a deployment where no Bot may address any + * other. The screen is then told the capability is off rather than shown a control that grants + * nothing. + */ + handoff?: { + /** Whether the deployment's own caps leave the capability switched on at all. */ + enabled: boolean; + /** The Bots this one may address today, read per call so a revoked grant stops showing. */ + reachableFrom: (agentId: string) => Promise; + }, ) { const routes = new Hono<{ Variables: AppVariables }>(); @@ -440,6 +457,37 @@ export function createAgentRoutes( } }); + /** + * Which Bots this Bot may hand work to. + * + * On the Bot's own screen rather than under the connector catalogue, because it is a fact about + * this Bot and not about a vendor: the catalogue's entries have a fixed list of tools, and the + * Bots a deployment has are whatever somebody made. + * + * `enabled` is reported separately from the grants, because the two fail differently. A grant with + * the capability switched off is a row in the database that will never be read, and a screen that + * offered it without saying so would be a switch wired to nothing. + */ + routes.get("/:agentId/handoff", requireUser, async (context) => { + const agentId = context.req.param("agentId"); + try { + // Asked of the store, so a Bot somebody may not see is "not found" here as everywhere else, + // rather than a list of who it can reach. + const agent = await store.get(context.var.actor, agentId); + if (!agent) return context.json({ error: "Agent not found." }, 404); + return context.json({ + handoff: { + enabled: handoff?.enabled ?? false, + // Granting is an administrator's, the same as it is on every other grant. + canGrant: context.var.actor.role === "admin", + reachable: handoff ? await handoff.reachableFrom(agentId) : [], + }, + }); + } catch (error) { + return mapStoreError(context, error); + } + }); + return routes; } diff --git a/server/src/app.ts b/server/src/app.ts index c802dcff..20561446 100644 --- a/server/src/app.ts +++ b/server/src/app.ts @@ -764,6 +764,21 @@ export function createApp( // Addresses this deployment named, which is how a hosted one reaches an agent on its own // network without dropping the floor for everything else. config.agentEndpointAllowedHosts, + /* + * What the Bot's own screen needs to show, and change, which Bots it may hand work to. + * + * Read per request rather than captured, for the reason the desk reads it per hop: a grant + * made a minute ago counts and one revoked a minute ago stops counting. Absent with no + * plugin store, which is a deployment where no Bot may address any other. + */ + pluginStore + ? { + enabled: + config.handoff.maxDepth > 0 && config.handoff.maxPerRun > 0, + reachableFrom: (agentId) => + pluginStore.botsReachableFrom(agentId), + } + : undefined, ), ); // Choosing a coworker for an untagged message needs the same permission-filtered roster the diff --git a/server/tests/agent-handoff-runner.test.ts b/server/tests/agent-handoff-runner.test.ts index b8252c05..fa7c726e 100644 --- a/server/tests/agent-handoff-runner.test.ts +++ b/server/tests/agent-handoff-runner.test.ts @@ -34,6 +34,10 @@ function runner(options?: { }) { const calls: Array<{ verb: string; key: string; owner?: string }> = []; const events: string[] = []; + const written: Array<{ + eventType: string; + payload: Record; + }> = []; const delivered: Array<{ message: string; assertion: string }> = []; const offered: HandoffWork[] = []; @@ -66,12 +70,17 @@ function runner(options?: { const auditStore: AuditStore = { insert: async (event) => { events.push(event.eventType); + written.push({ + eventType: event.eventType, + payload: (event.payload ?? {}) as Record, + }); }, }; return { calls, events, + written, delivered, offered, runner: createHandoffRunner({ @@ -167,6 +176,26 @@ describe("delivering a hop", () => { expect(events).toContain("agent.handoff_delivered"); }); + /* + * The Audit screen's Bot column reads `payload.bot` and renders a dash without it, so a delivery + * that names the Bot only under `from` is a row saying a handoff happened and not who did it. + * Its sibling `agent.handoff_offered` is asserted the same way in `agent-handoff.test.ts`. + */ + test("a delivery names the Bot that handed the work over", async () => { + const { runner: sweep, written } = runner(); + + await sweep.sweep(); + + const delivery = written.find( + (event) => event.eventType === "agent.handoff_delivered", + ); + expect(delivery?.payload).toMatchObject({ + bot: WORK.fromBotId, + from: WORK.fromBotId, + to: WORK.toBotId, + }); + }); + /* Releasing an unusable row would put it back on the queue for ever. */ test("a row that is not a hop is finished rather than released", async () => { const { runner: sweep, calls } = runner({ diff --git a/server/tests/agent-handoff.test.ts b/server/tests/agent-handoff.test.ts index 0aebe548..da3abae6 100644 --- a/server/tests/agent-handoff.test.ts +++ b/server/tests/agent-handoff.test.ts @@ -317,6 +317,13 @@ describe("handing work to another Bot", () => { from: "assistant", to: "researcher", run: "run-1", + /* + * The Audit screen's Bot column reads `payload.bot` and nothing else, so a row without it + * renders a dash. Every other Bot action sets it — `agent.escalated` one file over does — + * and these two did not, which made the handoff the only thing on a screen headed "Every + * action a Bot took" that named no Bot. Asserted rather than left to the reader of a payload. + */ + bot: "assistant", }); const refused = desk({ granted: false }); diff --git a/server/tests/agent-routes.test.ts b/server/tests/agent-routes.test.ts index 323d860a..73fc40af 100644 --- a/server/tests/agent-routes.test.ts +++ b/server/tests/agent-routes.test.ts @@ -607,3 +607,117 @@ describe("agent route composition", () => { expect(response.status).toBe(404); }); }); + +/* + * The screen that grants one Bot the right to address another reads this, so what it renders is + * decided here rather than in the browser: whether the capability is on at all, and whether the + * person looking may change any of it. + */ +describe("which Bots a Bot may hand work to", () => { + const admin = { + id: "admin-1", + email: "a@openbot.test", + role: "admin", + } as const; + + function appWith( + handoff: Parameters[5], + who: { id: string; email: string; role: "admin" | "user" } = admin, + ) { + const app = new Hono<{ Variables: AppVariables }>(); + const asWho: MiddlewareHandler<{ Variables: AppVariables }> = async ( + context, + next, + ) => { + context.set("actor", who); + await next(); + }; + app.route( + "/", + createAgentRoutes( + fakeStore(), + asWho, + false, + undefined, + new Set(), + handoff, + ), + ); + return app; + } + + test("reports the grants and that an administrator may change them", async () => { + const app = appWith({ + enabled: true, + reachableFrom: async () => ["knowledge"], + }); + + const body = (await json( + await app.request("/general-assistant/handoff"), + )) as { + handoff: { enabled: boolean; canGrant: boolean; reachable: string[] }; + }; + + expect(body.handoff).toEqual({ + enabled: true, + canGrant: true, + reachable: ["knowledge"], + }); + }); + + test("somebody who is not an administrator may read it and not change it", async () => { + const app = appWith( + { enabled: true, reachableFrom: async () => ["knowledge"] }, + actor, + ); + + const body = (await json( + await app.request("/general-assistant/handoff"), + )) as { + handoff: { canGrant: boolean }; + }; + + expect(body.handoff.canGrant).toBe(false); + }); + + /* + * A deployment with the caps at zero, or with no plugin store to read a grant from, has the + * capability switched off. Reported rather than left to the screen to infer, because a switch + * wired to nothing is the thing this says out loud. + */ + test("says the capability is off when nothing can grant it", async () => { + const app = appWith(undefined); + + const body = (await json( + await app.request("/general-assistant/handoff"), + )) as { + handoff: { enabled: boolean; reachable: string[] }; + }; + + expect(body.handoff.enabled).toBe(false); + expect(body.handoff.reachable).toEqual([]); + }); + + test("a Bot the person may not see is not found, rather than described", async () => { + const app = new Hono<{ Variables: AppVariables }>(); + app.route( + "/", + createAgentRoutes( + fakeStore({ + async get() { + return null; + }, + }), + requireUser, + false, + undefined, + new Set(), + { enabled: true, reachableFrom: async () => ["knowledge"] }, + ), + ); + + const response = await app.request("/somebody-elses/handoff"); + + expect(response.status).toBe(404); + }); +});