diff --git a/cli/src/__tests__/company-import-transfer.test.ts b/cli/src/__tests__/company-import-transfer.test.ts index c8b560849d..37c0b150bc 100644 --- a/cli/src/__tests__/company-import-transfer.test.ts +++ b/cli/src/__tests__/company-import-transfer.test.ts @@ -320,6 +320,24 @@ describe("uploadCompanyImportTransfer", () => { await expect(uploadCompanyImportTransfer(api, zipBytes)).rejects.toThrow(/already imported/); expect(putRaw).not.toHaveBeenCalled(); }); + + it("names the company the completed transfer created", async () => { + const { api, putRaw } = fakeApi({ + post: vi.fn().mockResolvedValue({ + transferId: "transfer-1", + status: "completed", + alreadyCompleted: true, + totalParts: 2, + missingParts: [], + company: { id: "company-2", name: "Paperclip", issuePrefix: "PAPA" }, + }), + }); + + await expect(uploadCompanyImportTransfer(api, zipBytes)).rejects.toThrow( + /landed in the company "Paperclip" \(PAPA\)/, + ); + expect(putRaw).not.toHaveBeenCalled(); + }); }); describe("company import command over the chunked transfer path", () => { diff --git a/cli/src/commands/client/company.ts b/cli/src/commands/client/company.ts index e1e7bea44b..5deb822bd5 100644 --- a/cli/src/commands/client/company.ts +++ b/cli/src/commands/client/company.ts @@ -14,6 +14,7 @@ import type { CompanyPortabilityImportResult, } from "@paperclipai/shared"; import { + buildAlreadyImportedMessage, companyImportTransferApplyPath, companyImportTransferPartPath, companyImportTransferPreviewPath, @@ -1160,9 +1161,9 @@ export async function uploadCompanyImportTransfer( if (created.alreadyCompleted) { // The server keys transfers by content, and this exact zip already // finished an apply — its spooled parts are gone, so it cannot re-run. - throw new Error( - "This exact package was already imported by a completed transfer. Re-export the package to import it again.", - ); + // Name the company that apply created so the rejection points at the + // existing import instead of reading as data loss. + throw new Error(buildAlreadyImportedMessage(created.company)); } const missing = new Set(created.missingParts); let uploadedParts = manifest.parts.length - missing.size; diff --git a/packages/shared/src/company-import-transfer.test.ts b/packages/shared/src/company-import-transfer.test.ts new file mode 100644 index 0000000000..213169db79 --- /dev/null +++ b/packages/shared/src/company-import-transfer.test.ts @@ -0,0 +1,29 @@ +import { describe, expect, it } from "vitest"; +import { buildAlreadyImportedMessage } from "./company-import-transfer.js"; + +describe("buildAlreadyImportedMessage", () => { + it("names the landed company with its prefix", () => { + expect( + buildAlreadyImportedMessage({ id: "company-2", name: "Paperclip", issuePrefix: "PAPA" }), + ).toBe( + 'This exact package was already imported by a completed transfer. The earlier import landed in the company "Paperclip" (PAPA) — open it from the company switcher. Re-export the package to import it again.', + ); + }); + + it("falls back to the company id when the name is gone and omits an absent prefix", () => { + expect( + buildAlreadyImportedMessage({ id: "company-2", name: null, issuePrefix: null }), + ).toBe( + 'This exact package was already imported by a completed transfer. The earlier import landed in the company "company-2" — open it from the company switcher. Re-export the package to import it again.', + ); + }); + + it("keeps the original message when no company is known", () => { + expect(buildAlreadyImportedMessage(null)).toBe( + "This exact package was already imported by a completed transfer. Re-export the package to import it again.", + ); + expect(buildAlreadyImportedMessage(undefined)).toBe( + "This exact package was already imported by a completed transfer. Re-export the package to import it again.", + ); + }); +}); diff --git a/packages/shared/src/company-import-transfer.ts b/packages/shared/src/company-import-transfer.ts index f6778a1c9e..376482a8c2 100644 --- a/packages/shared/src/company-import-transfer.ts +++ b/packages/shared/src/company-import-transfer.ts @@ -78,6 +78,29 @@ export interface CompanyImportTransferCreated { alreadyCompleted: boolean; totalParts: number; missingParts: number[]; + /** + * Where the prior completed apply landed, so an alreadyCompleted rejection + * can point at the existing company instead of reading as data loss. + * Null/absent when the run's company link was never written or the company + * has since been deleted. + */ + company?: { id: string; name: string | null; issuePrefix: string | null } | null; +} + +/** + * Human-facing message for an `alreadyCompleted` declaration. Shared by the + * web and CLI clients so the copy (and the pointer to the landed company) + * stays identical everywhere the rejection surfaces. + */ +export function buildAlreadyImportedMessage( + company: CompanyImportTransferCreated["company"], +): string { + // "landed in", not "created": a completed transfer may have created a new + // company or merged into an existing one, and the caller cannot tell which. + const location = company + ? ` The earlier import landed in the company "${company.name ?? company.id}"${company.issuePrefix ? ` (${company.issuePrefix})` : ""} — open it from the company switcher.` + : ""; + return `This exact package was already imported by a completed transfer.${location} Re-export the package to import it again.`; } /** Response of the resume-polling GET for one transfer. */ diff --git a/server/src/__tests__/company-import-transfer-routes.test.ts b/server/src/__tests__/company-import-transfer-routes.test.ts index 994b7afe38..b1dd3f9d84 100644 --- a/server/src/__tests__/company-import-transfer-routes.test.ts +++ b/server/src/__tests__/company-import-transfer-routes.test.ts @@ -727,11 +727,28 @@ describeEmbeddedPostgres("company import transfer routes", () => { (await request(app).post(`/api/companies/import/transfers/${transferId}/apply`).send(importMeta)).status, ).toBe(200); + // The rejection names the company the completed apply created so the + // caller can find the earlier import instead of reading it as data loss. + mockCompanyService.getById.mockResolvedValue({ + id: companyId, + name: "Paperclip", + issuePrefix: "PAPA", + }); const redeclared = await request(app).post("/api/companies/import/transfers").send(body); expect(redeclared.status).toBe(200); expect(redeclared.body.transferId).toBe(transferId); expect(redeclared.body.alreadyCompleted).toBe(true); expect(redeclared.body.missingParts).toEqual([]); + expect(mockCompanyService.getById).toHaveBeenCalledWith(companyId); + expect(redeclared.body.company).toEqual({ id: companyId, name: "Paperclip", issuePrefix: "PAPA" }); + + // A company deleted since the apply (or an attach that never happened) + // degrades to company: null, never a 500. + mockCompanyService.getById.mockResolvedValue(null); + const redeclaredAfterDelete = await request(app).post("/api/companies/import/transfers").send(body); + expect(redeclaredAfterDelete.status).toBe(200); + expect(redeclaredAfterDelete.body.alreadyCompleted).toBe(true); + expect(redeclaredAfterDelete.body.company).toBeNull(); const reApplied = await request(app) .post(`/api/companies/import/transfers/${transferId}/apply`) diff --git a/server/src/routes/companies.ts b/server/src/routes/companies.ts index 7497da3617..6a929196e1 100644 --- a/server/src/routes/companies.ts +++ b/server/src/routes/companies.ts @@ -62,7 +62,7 @@ import { } from "../services/index.js"; import { isCloudManagedInstance } from "../services/cloud-instance.js"; import type { StorageService } from "../storage/types.js"; -import { assertBoard, assertCompanyAccess, assertInstanceAdmin, getActorInfo } from "./authz.js"; +import { assertBoard, assertCompanyAccess, assertInstanceAdmin, getActorInfo, hasCompanyAccess } from "./authz.js"; import { COMPANY_IMPORT_ROUTE_PATH } from "./company-import-paths.js"; // A company import can arrive one of two ways on the import + preview routes: @@ -729,12 +729,25 @@ export function companyRoutes(db: Db, storage?: StorageService, options?: Compan containerRef: { kind: "chunked_zip_upload" }, }); if (alreadyCompleted) { + // Tell the caller WHERE the prior apply landed. companyId on the run is + // written best-effort after apply and the company may have been deleted + // since, so this lookup is null-safe and the field stays optional. The + // actor key proves this caller ran the original import, but their + // access may have been revoked since — withhold the identity unless + // they can still reach the company today (hasCompanyAccess, so a + // revoked caller sees the same null as a deleted company). + const landedCompany = run.companyId && hasCompanyAccess(req, run.companyId) + ? await svc.getById(run.companyId) + : null; res.json({ transferId: run.id, status: "completed", alreadyCompleted: true, totalParts: declared.parts.length, missingParts: [], + company: landedCompany + ? { id: landedCompany.id, name: landedCompany.name ?? null, issuePrefix: landedCompany.issuePrefix ?? null } + : null, } satisfies CompanyImportTransferCreated); return; } diff --git a/ui/src/pages/CompanyImport.tsx b/ui/src/pages/CompanyImport.tsx index 4e86104af0..21b57eb9d4 100644 --- a/ui/src/pages/CompanyImport.tsx +++ b/ui/src/pages/CompanyImport.tsx @@ -52,7 +52,7 @@ import { } from "../components/FileTree"; import { readZipArchive } from "../lib/zip"; import { formatMegabytes } from "../lib/import-preflight"; -import type { CompanyImportTransferDeclaration } from "@paperclipai/shared/company-import-transfer"; +import { buildAlreadyImportedMessage, type CompanyImportTransferDeclaration } from "@paperclipai/shared/company-import-transfer"; import { CHUNKED_IMPORT_THRESHOLD_BYTES, IMPORT_TRANSFER_PART_ATTEMPTS, @@ -944,10 +944,10 @@ export function CompanyImport() { const created = await companiesApi.importTransferCreate(manifest); if (created.alreadyCompleted) { // The server keys transfers by content, and this exact zip already - // finished an apply — its parts are gone, so it cannot be re-run. - throw new Error( - "This exact package was already imported by a completed transfer. Re-export the package to import it again.", - ); + // finished an apply — its parts are gone, so it cannot be re-run. Name + // the company that apply created so this reads as "your import exists + // over there", not as data loss. + throw new Error(buildAlreadyImportedMessage(created.company)); } const missing = new Set(created.missingParts); let uploadedParts = manifest.parts.length - missing.size;