diff --git a/server/src/__tests__/agent-shortname-collision.test.ts b/server/src/__tests__/agent-shortname-collision.test.ts new file mode 100644 index 00000000..69ef427a --- /dev/null +++ b/server/src/__tests__/agent-shortname-collision.test.ts @@ -0,0 +1,37 @@ +import { describe, expect, it } from "vitest"; +import { hasAgentShortnameCollision } from "../services/agents.ts"; + +describe("hasAgentShortnameCollision", () => { + it("detects collisions by normalized shortname", () => { + const collision = hasAgentShortnameCollision("Codex Coder", [ + { id: "a1", name: "codex-coder", status: "idle" }, + ]); + expect(collision).toBe(true); + }); + + it("ignores terminated agents", () => { + const collision = hasAgentShortnameCollision("Codex Coder", [ + { id: "a1", name: "codex-coder", status: "terminated" }, + ]); + expect(collision).toBe(false); + }); + + it("ignores the excluded agent id", () => { + const collision = hasAgentShortnameCollision( + "Codex Coder", + [ + { id: "a1", name: "codex-coder", status: "idle" }, + { id: "a2", name: "other-agent", status: "idle" }, + ], + { excludeAgentId: "a1" }, + ); + expect(collision).toBe(false); + }); + + it("does not collide when candidate has no shortname", () => { + const collision = hasAgentShortnameCollision("!!!", [ + { id: "a1", name: "codex-coder", status: "idle" }, + ]); + expect(collision).toBe(false); + }); +}); diff --git a/server/src/services/agents.ts b/server/src/services/agents.ts index 703a3f95..dd72b86f 100644 --- a/server/src/services/agents.ts +++ b/server/src/services/agents.ts @@ -51,6 +51,16 @@ interface UpdateAgentOptions { recordRevision?: RevisionMetadata; } +interface AgentShortnameRow { + id: string; + name: string; + status: string; +} + +interface AgentShortnameCollisionOptions { + excludeAgentId?: string | null; +} + function isPlainRecord(value: unknown): value is Record { return typeof value === "object" && value !== null && !Array.isArray(value); } @@ -140,6 +150,21 @@ function configPatchFromSnapshot(snapshot: unknown): Partial { + if (agent.status === "terminated") return false; + if (options?.excludeAgentId && agent.id === options.excludeAgentId) return false; + return normalizeAgentUrlKey(agent.name) === candidateShortname; + }); +} + export function agentService(db: Db) { function withUrlKey(row: T) { return { @@ -185,6 +210,31 @@ export function agentService(db: Db) { } } + async function assertCompanyShortnameAvailable( + companyId: string, + candidateName: string, + options?: AgentShortnameCollisionOptions, + ) { + const candidateShortname = normalizeAgentUrlKey(candidateName); + if (!candidateShortname) return; + + const existingAgents = await db + .select({ + id: agents.id, + name: agents.name, + status: agents.status, + }) + .from(agents) + .where(eq(agents.companyId, companyId)); + + const hasCollision = hasAgentShortnameCollision(candidateName, existingAgents, options); + if (hasCollision) { + throw conflict( + `Agent shortname '${candidateShortname}' is already in use in this company`, + ); + } + } + async function updateAgent( id: string, data: Partial, @@ -212,6 +262,14 @@ export function agentService(db: Db) { await assertNoCycle(id, data.reportsTo); } + if (data.name !== undefined) { + const previousShortname = normalizeAgentUrlKey(existing.name); + const nextShortname = normalizeAgentUrlKey(data.name); + if (previousShortname !== nextShortname) { + await assertCompanyShortnameAvailable(existing.companyId, data.name, { excludeAgentId: id }); + } + } + const normalizedPatch = { ...data } as Partial; if (data.permissions !== undefined) { const role = (data.role ?? existing.role) as string; @@ -267,6 +325,8 @@ export function agentService(db: Db) { await ensureManager(companyId, data.reportsTo); } + await assertCompanyShortnameAvailable(companyId, data.name); + const role = data.role ?? "general"; const normalizedPermissions = normalizeAgentPermissions(data.permissions, role); const created = await db