From 773af7df8e38860591a4e9dfa857e73ab41411b0 Mon Sep 17 00:00:00 2001 From: kevin9327 Date: Fri, 4 Sep 2026 07:21:18 +0900 Subject: [PATCH] Give a duplicated Bot in the box the instructions it was copied from #328 fixed the half of duplicate that a coworker with its own endpoint saw. This is the other half. A coworker that runs on this deployment's own Bot has no endpoint. Its configuration is `{ systemPrompt }`, and that prompt is the whole of what makes it that coworker. Duplicate rebuilt every copy from `source.endpoint` alone and wrote `type: "remote_ag_ui"` flat, so for one of these the endpoint read came back null, the copy fell through to the managed Bot, and the prompt was dropped on the floor. What comes back is the failure this repository already has a paragraph about. The copy looks identical on every screen and its entire instruction becomes standingRoleMessage - see the note above that function in copilot.ts, which names the compliance Bot that answered a filing question with invented thresholds because one sentence of role description was all that reached it. The default tenant package ships two built_in coworkers, General Assistant and Knowledge, and Knowledge's prompt is a careful do-not-fabricate instruction with the reasoning written out beside it in the package. Duplicate it and you get a coworker with the name, the title, the avatar, and none of that. The type is carried now, not only the configuration. A copy written as remote_ag_ui also could not be granted handoff for the rest of its life: agentRunsHere and botsReachableFrom both key on agents.type == "built_in", so the original could hand work on and its copy silently could not. And a built_in source no longer needs a managed Bot to fall back to. On a deployment with none, copying one used to be refused with advice to give the coworker an AG-UI endpoint it was never supposed to have - the same wrong refusal #328 removed for a coworker that brought its own endpoint. The decision is a pure function so it can be tested without a database. The stored row is read inside the transaction and after the access check, rather than widening AgentProfile: the DTO every surface receives should not start carrying a Bot's instructions. `auth` is still deliberately not copied. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 13 +++ server/src/agents/profile-store.ts | 99 +++++++++++++++++++++-- server/tests/duplicate-run.test.ts | 125 +++++++++++++++++++++++++++++ 3 files changed, 229 insertions(+), 8 deletions(-) create mode 100644 server/tests/duplicate-run.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 11fb9c67f..3d480188b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,19 @@ Newest first. `Unreleased` is what is on `main` and not yet tagged. ## Unreleased +### Duplicating a Bot in the box keeps its instructions + +A coworker that runs on this deployment's own Bot has no endpoint — it has a prompt, which is the +whole of what makes it that coworker. Duplicate rebuilt every copy from the endpoint alone, found +none, and fell back to the managed Bot with the prompt dropped, so the copy carried the name, the +title, the role and the avatar and none of the instructions. Its entire instruction became the one +sentence of role description, which is the shape behind the compliance answer this repository +already has a note about. The two coworkers the default package ships are both of this kind, and one +of them is a careful do-not-fabricate instruction. A copy now keeps the prompt and stays a Bot in the +box, which also means it can still be granted the right to hand work on — written as a hosted +coworker it could never hold that grant, however the original was set up — and copying one no longer +needs a managed Bot to fall back to. + ### Duplicating a coworker keeps the endpoint it was copied from Duplicate used to point every copy at this deployment's own managed Bot, whatever the coworker being diff --git a/server/src/agents/profile-store.ts b/server/src/agents/profile-store.ts index 315d6ee73..f96897043 100644 --- a/server/src/agents/profile-store.ts +++ b/server/src/agents/profile-store.ts @@ -183,6 +183,74 @@ function endpointOf(configuration: unknown): string | null { return typeof endpoint === "string" ? endpoint : null; } +/** + * The instruction a Bot in the box runs on, read back out of its stored configuration. + * + * The mirror of {@link endpointOf}, and needed for the same reason: a copy has to be made of what + * the original actually was, and for a `built_in` coworker the prompt IS the coworker. Trimmed and + * required to be non-empty, matching `registeredAgentFromRow`, which will not build a Bot from a + * blank one either. + */ +function systemPromptOf(configuration: unknown): string | null { + if (!configuration || typeof configuration !== "object") return null; + const prompt = (configuration as { systemPrompt?: unknown }).systemPrompt; + if (typeof prompt !== "string") return null; + const trimmed = prompt.trim(); + return trimmed.length > 0 ? trimmed : null; +} + +/** What a coworker is, and what it runs on: the two `agents` columns a copy has to reproduce. */ +export type AgentRun = { + type: "built_in" | "remote_ag_ui"; + configuration: Record; +}; + +/** + * What a duplicate runs on, decided from what the original ran on. + * + * WHY THIS IS NOT JUST THE ENDPOINT. Duplicate used to rebuild the copy from `source.endpoint` alone + * and write `type: "remote_ag_ui"` flat. #328 fixed the half of that a coworker with its own endpoint + * saw. The other half is a coworker that has no endpoint because it is not supposed to have one: a + * `built_in` Bot's configuration is `{ systemPrompt }`, so the endpoint read came back null, the copy + * fell through to the managed Bot, and the prompt was dropped on the floor. + * + * That copy is the failure this repository already has a paragraph about. It looks identical on every + * screen and its whole instruction becomes `standingRoleMessage` — see the note above that function + * in `copilot.ts`, which names the compliance Bot that answered a filing question with invented + * thresholds because one sentence of role description was all that reached it. The default tenant + * package ships two `built_in` coworkers, and one of them, `Knowledge`, is a careful + * do-not-fabricate instruction. Copy it and you get a coworker with the name, the title, the avatar, + * and none of that. + * + * The type is carried too, not only the configuration. A copy written as `remote_ag_ui` also cannot + * be granted handoff for the rest of its life: `agentRunsHere` and `botsReachableFrom` both key on + * `agents.type == "built_in"`, so the original may hand work on and its copy silently may not. + * + * `null` means there is nothing to run this copy on, which the caller turns into + * {@link ManagedAgentUnavailableError}. That can now only happen for a source that had neither an + * endpoint nor a prompt on a deployment with no managed Bot — never for a `built_in` source, which + * brings its own instruction and needs no managed Bot to fall back to. + * + * `auth` is deliberately not carried: it is a reference into the vault, and two coworkers sharing one + * credential would mean rotating either one's key silently changed the other's. + */ +export function runForDuplicate( + source: { type: "built_in" | "remote_ag_ui"; configuration: unknown }, + managed: Record | undefined, +): AgentRun | null { + const systemPrompt = systemPromptOf(source.configuration); + if (source.type === "built_in" && systemPrompt) { + return { type: "built_in", configuration: { systemPrompt } }; + } + + const endpoint = endpointOf(source.configuration); + if (endpoint) { + return { type: "remote_ag_ui", configuration: { endpoint } }; + } + + return managed ? { type: "remote_ag_ui", configuration: managed } : null; +} + async function findAccessibleProfile( executor: DatabaseExecutor, actor: AgentActor, @@ -437,20 +505,35 @@ export function createAgentProfileStore( const source = await findAccessibleProfile(transaction, actor, id); if (!source) throw new AgentNotFoundError(id); - // The endpoint alone: `auth` is a vault reference, and copying it shares one credential. - const configuration = source.endpoint - ? { endpoint: source.endpoint } - : managedConfiguration; - // After the source read, so a source with its own endpoint needs no managed Bot to fall back to. - if (!configuration) { + /* + * The stored row, because a profile does not carry what a copy has to reproduce. + * + * `AgentProfile` projects `endpoint` out of the configuration and nothing else, which is all + * an edit form needs and half of what this needs: a `built_in` coworker has no endpoint and + * a prompt instead. Read here rather than widened into the profile, so the DTO every surface + * gets does not start carrying a Bot's instructions. Inside the transaction, and after the + * access check, so this cannot read a row the caller may not see. + */ + const [stored] = await transaction + .select({ type: agents.type, configuration: agents.configuration }) + .from(agents) + .where(eq(agents.id, id)) + .limit(1); + if (!stored) throw new AgentNotFoundError(id); + + // `auth` is a vault reference and is deliberately not carried: see `runForDuplicate`. + const run = runForDuplicate(stored, managedConfiguration); + // After the source read, so a source that brings its own endpoint or its own prompt needs no + // managed Bot to fall back to. + if (!run) { throw new ManagedAgentUnavailableError(); } const duplicateId = newAgentId(); await transaction.insert(agents).values({ id: duplicateId, name: source.name, - type: "remote_ag_ui", - configuration, + type: run.type, + configuration: run.configuration, }); await transaction.insert(agentProfiles).values({ agentId: duplicateId, diff --git a/server/tests/duplicate-run.test.ts b/server/tests/duplicate-run.test.ts new file mode 100644 index 000000000..3d1d0c90b --- /dev/null +++ b/server/tests/duplicate-run.test.ts @@ -0,0 +1,125 @@ +import { describe, expect, test } from "bun:test"; +import { runForDuplicate } from "../src/agents/profile-store"; + +/** + * What a duplicated coworker runs on. + * + * A pure decision, tested without a database, because the failure it exists to stop is silent: the + * copy is created, appears on the roster, answers when asked, and is a different coworker from the + * one that was copied. + */ + +const managed = { endpoint: "http://managed.invalid/ag-ui" }; + +describe("what a copy runs on", () => { + /* + * The bug. A Bot in the box keeps its whole instruction in `configuration.systemPrompt` and has no + * endpoint at all, so an endpoint-only read came back empty, the copy fell through to the managed + * Bot, and the prompt went nowhere. The default tenant package ships two of these. + */ + test("keeps a Bot-in-the-box's prompt, and stays a Bot in the box", () => { + expect( + runForDuplicate( + { + type: "built_in", + configuration: { systemPrompt: "Never answer from memory." }, + }, + managed, + ), + ).toEqual({ + type: "built_in", + configuration: { systemPrompt: "Never answer from memory." }, + }); + }); + + test("needs no managed Bot to copy one that brought its own prompt", () => { + // The mirror of the endpoint case: a source that carries what it runs on does not fall back. + expect( + runForDuplicate( + { type: "built_in", configuration: { systemPrompt: "Be brief." } }, + undefined, + ), + ).toEqual({ + type: "built_in", + configuration: { systemPrompt: "Be brief." }, + }); + }); + + test("keeps the endpoint a hosted coworker was copied from", () => { + expect( + runForDuplicate( + { + type: "remote_ag_ui", + configuration: { endpoint: "https://theirs.invalid/ag-ui" }, + }, + managed, + ), + ).toEqual({ + type: "remote_ag_ui", + configuration: { endpoint: "https://theirs.invalid/ag-ui" }, + }); + }); + + test("never carries the key: two coworkers must not share one credential", () => { + expect( + runForDuplicate( + { + type: "remote_ag_ui", + configuration: { + endpoint: "https://theirs.invalid/ag-ui", + auth: { header: "Authorization", credentialId: "cred_1" }, + }, + }, + managed, + ), + ).toEqual({ + type: "remote_ag_ui", + configuration: { endpoint: "https://theirs.invalid/ag-ui" }, + }); + }); + + test("falls back to the managed Bot only when the source runs on nothing of its own", () => { + expect( + runForDuplicate({ type: "built_in", configuration: {} }, managed), + ).toEqual({ type: "remote_ag_ui", configuration: managed }); + }); + + test("has nowhere to put a copy of a coworker with nothing, and no managed Bot", () => { + // Null is what the caller turns into "give the coworker its own AG-UI endpoint". It must not be + // reachable for a source that had a prompt, which is what the second case above pins. + expect( + runForDuplicate({ type: "remote_ag_ui", configuration: {} }, undefined), + ).toBeNull(); + }); + + test("treats a blank prompt as no prompt, the way the runtime does", () => { + // `registeredAgentFromRow` refuses to build a Bot from a whitespace prompt, so copying one as + // `built_in` would produce a coworker that cannot be built at all. + expect( + runForDuplicate( + { type: "built_in", configuration: { systemPrompt: " " } }, + managed, + ), + ).toEqual({ type: "remote_ag_ui", configuration: managed }); + }); + + test("does not read a prompt off a coworker that runs somewhere else", () => { + // The type decides, not the presence of a key. A remote row carrying a stray `systemPrompt` is + // still a remote Bot, and copying it as built-in would move it into this process. + expect( + runForDuplicate( + { + type: "remote_ag_ui", + configuration: { + endpoint: "https://theirs.invalid/ag-ui", + systemPrompt: "ignored", + }, + }, + managed, + ), + ).toEqual({ + type: "remote_ag_ui", + configuration: { endpoint: "https://theirs.invalid/ag-ui" }, + }); + }); +});