Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,18 @@ 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.
### Hiding a coworker no longer hides the grants pointing at it

Hiding a coworker is a preference about your own roster — one row per person — and the grants saying
Expand Down
99 changes: 91 additions & 8 deletions server/src/agents/profile-store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, unknown>;
};

/**
* 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<string, unknown> | 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,
Expand Down Expand Up @@ -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,
Expand Down
125 changes: 125 additions & 0 deletions server/tests/duplicate-run.test.ts
Original file line number Diff line number Diff line change
@@ -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" },
});
});
});