diff --git a/server/src/__tests__/company-portability.test.ts b/server/src/__tests__/company-portability.test.ts index 49edb4c881..c919bcb554 100644 --- a/server/src/__tests__/company-portability.test.ts +++ b/server/src/__tests__/company-portability.test.ts @@ -9,6 +9,10 @@ import type { CompanyPortabilityFileEntry } from "@paperclipai/shared"; const companySvc = { getById: vi.fn(), + // Async-empty default (not a bare vi.fn()): every new-company import reads + // the existing names for de-duplication, including describes that never + // touch this mock. + list: vi.fn(async () => []), create: vi.fn(), update: vi.fn(), }; @@ -154,7 +158,7 @@ vi.mock("../routes/org-chart-svg.js", () => ({ renderOrgChartPng: vi.fn(async () => Buffer.from("png")), })); -const { companyPortabilityService, parseGitHubSourceUrl, renderYamlBlock, renderFrontmatter } = await import("../services/company-portability.js"); +const { companyPortabilityService, dedupeImportedCompanyName, parseGitHubSourceUrl, renderYamlBlock, renderFrontmatter } = await import("../services/company-portability.js"); function asTextFile(entry: CompanyPortabilityFileEntry | undefined) { expect(typeof entry).toBe("string"); @@ -184,6 +188,7 @@ describe("company portability", () => { presentation: null, metadata: null, }); + companySvc.list.mockResolvedValue([]); companySvc.getById.mockResolvedValue({ id: "company-1", name: "Paperclip", @@ -2609,6 +2614,93 @@ describe("company portability", () => { ]); }); + it("suffixes a manifest-derived company name that collides with an existing company", async () => { + const portability = companyPortabilityService({} as any); + + companySvc.list.mockResolvedValue([ + { name: "Imported Paperclip" }, + // Case-insensitive: an existing "(2)" in any casing blocks that suffix. + { name: "imported paperclip (2)" }, + ]); + companySvc.create.mockResolvedValue({ + id: "company-imported", + name: "Imported Paperclip (3)", + }); + accessSvc.ensureMembership.mockResolvedValue(undefined); + + const files = { + "COMPANY.md": ["---", 'schema: "agentcompanies/v1"', 'name: "Imported Paperclip"', "---", ""].join("\n"), + }; + + await portability.importBundle({ + source: { type: "inline", rootPath: "paperclip-demo", files }, + include: { company: true, agents: false, projects: false, issues: false }, + // No newCompanyName: the manifest name is used and must be de-duplicated. + target: { mode: "new_company" }, + collisionStrategy: "rename", + }, "user-1"); + + expect(companySvc.create).toHaveBeenCalledWith(expect.objectContaining({ + name: "Imported Paperclip (3)", + })); + }); + + it("skips name de-duplication for agent-safe imports so collisions stay unobservable", async () => { + const portability = companyPortabilityService({} as any); + + companySvc.list.mockResolvedValue([{ name: "Imported Paperclip" }]); + companySvc.create.mockResolvedValue({ + id: "company-imported", + name: "Imported Paperclip", + }); + accessSvc.listActiveUserMemberships.mockResolvedValue([{ userId: "user-1" }]); + accessSvc.copyActiveUserMemberships.mockResolvedValue([]); + + const files = { + "COMPANY.md": ["---", 'schema: "agentcompanies/v1"', 'name: "Imported Paperclip"', "---", ""].join("\n"), + }; + + await portability.importBundle({ + source: { type: "inline", rootPath: "paperclip-demo", files }, + include: { company: true, agents: false, projects: false, issues: false }, + target: { mode: "new_company" }, + collisionStrategy: "rename", + }, "user-1", { mode: "agent_safe", sourceCompanyId: "company-1" }); + + // The instance-wide name list must never be consulted for a + // company-scoped agent, and no suffix may reflect a collision back. + expect(companySvc.list).not.toHaveBeenCalled(); + expect(companySvc.create).toHaveBeenCalledWith(expect.objectContaining({ + name: "Imported Paperclip", + })); + }); + + it("honors an explicitly typed company name even when it collides", async () => { + const portability = companyPortabilityService({} as any); + + companySvc.list.mockResolvedValue([{ name: "Imported Paperclip" }]); + companySvc.create.mockResolvedValue({ + id: "company-imported", + name: "Imported Paperclip", + }); + accessSvc.ensureMembership.mockResolvedValue(undefined); + + const files = { + "COMPANY.md": ["---", 'schema: "agentcompanies/v1"', 'name: "Imported Paperclip"', "---", ""].join("\n"), + }; + + await portability.importBundle({ + source: { type: "inline", rootPath: "paperclip-demo", files }, + include: { company: true, agents: false, projects: false, issues: false }, + target: { mode: "new_company", newCompanyName: "Imported Paperclip" }, + collisionStrategy: "rename", + }, "user-1"); + + expect(companySvc.create).toHaveBeenCalledWith(expect.objectContaining({ + name: "Imported Paperclip", + })); + }); + it("pauses imported agents and routines when pauseAutomations is requested", async () => { const portability = companyPortabilityService({} as any); @@ -5717,3 +5809,20 @@ describe("company portability", () => { expect(preview.plan.issuePlans).toHaveLength(0); }); }); + +describe("dedupeImportedCompanyName", () => { + it("returns the base name when nothing collides", () => { + expect(dedupeImportedCompanyName("Paperclip", ["Other Co"])).toBe("Paperclip"); + expect(dedupeImportedCompanyName("Paperclip", [])).toBe("Paperclip"); + }); + + it("suffixes past every taken candidate, case-insensitively", () => { + expect(dedupeImportedCompanyName("Paperclip", ["paperclip"])).toBe("Paperclip (2)"); + expect(dedupeImportedCompanyName("Paperclip", ["Paperclip", "Paperclip (2)"])).toBe("Paperclip (3)"); + expect(dedupeImportedCompanyName("Paperclip", ["PAPERCLIP", "paperclip (2)"])).toBe("Paperclip (3)"); + }); + + it("ignores surrounding whitespace in existing names", () => { + expect(dedupeImportedCompanyName("Paperclip", [" Paperclip "])).toBe("Paperclip (2)"); + }); +}); diff --git a/server/src/services/company-portability.ts b/server/src/services/company-portability.ts index a6df4e9ee1..85b61c9e46 100644 --- a/server/src/services/company-portability.ts +++ b/server/src/services/company-portability.ts @@ -229,6 +229,25 @@ function assertInlineSourceComplete(source: CompanyPortabilityImport["source"]) } } +/** + * Suffix a manifest-derived company name with " (2)", " (3)", … when it + * collides case-insensitively with an existing company, so repeat imports of + * the same package do not produce several identically named companies that + * only differ by issue prefix. Explicit user-typed names bypass this — they + * are the caller's deliberate choice. + */ +export function dedupeImportedCompanyName(baseName: string, existingNames: string[]): string { + const normalized = new Set(existingNames.map((name) => name.trim().toLowerCase())); + if (!normalized.has(baseName.trim().toLowerCase())) return baseName; + for (let suffix = 2; suffix < 10_000; suffix += 1) { + const candidate = `${baseName} (${suffix})`; + if (!normalized.has(candidate.toLowerCase())) return candidate; + } + // Pathological: thousands of identically named companies. Give up on the + // suffix rather than fail the import — names carry no uniqueness invariant. + return baseName; +} + function resolveSkillConflictStrategy(mode: ImportMode, collisionStrategy: CompanyPortabilityCollisionStrategy) { if (mode === "board_full") return collisionStrategy; return collisionStrategy === "skip" ? "skip" as const : "rename" as const; @@ -5241,11 +5260,21 @@ export function companyPortabilityService(db: Db, storage?: StorageService) { throw unprocessable("Safe new-company import requires at least one active user membership on the source company."); } } + const requestedCompanyName = asString(input.target.newCompanyName); + const manifestCompanyName = + sourceManifest.company?.name ?? sourceManifest.source?.companyName ?? "Imported Company"; + // De-duplicate only for board-driven imports. The lookup reads every + // company name in the instance, and reflecting a collision back through + // the numeric suffix would let a company-scoped agent (agent_safe mode) + // probe for the existence of company names outside its own company. const companyName = - asString(input.target.newCompanyName) ?? - sourceManifest.company?.name ?? - sourceManifest.source?.companyName ?? - "Imported Company"; + requestedCompanyName ?? + (mode === "agent_safe" + ? manifestCompanyName + : dedupeImportedCompanyName( + manifestCompanyName, + (await companies.list()).map((company) => company.name), + )); const created = await companies.create({ name: companyName, description: include.company ? (sourceManifest.company?.description ?? null) : null,