diff --git a/server/src/__tests__/company-portability-import-batching.test.ts b/server/src/__tests__/company-portability-import-batching.test.ts index 7cc08ccbd1..4c32bfe5cd 100644 --- a/server/src/__tests__/company-portability-import-batching.test.ts +++ b/server/src/__tests__/company-portability-import-batching.test.ts @@ -7,17 +7,19 @@ import { issueAttachments, issueComments, issueDocuments, + issueInboxArchives, issueLabels, issueRelations, issueWorkProducts, issues, } from "@paperclipai/db"; -import { eq, sql } from "drizzle-orm"; +import { and, eq, sql } from "drizzle-orm"; import { getEmbeddedPostgresTestSupport, startEmbeddedPostgresTestDatabase, } from "./helpers/embedded-postgres.js"; import { companyPortabilityService } from "../services/company-portability.js"; +import { issueService } from "../services/issues.js"; import { workProductService } from "../services/work-products.js"; import type { ImportIssueWorkProductRow } from "../services/import-write-types.js"; import type { CompanyPortabilityFileEntry } from "@paperclipai/shared"; @@ -194,6 +196,35 @@ function buildSyntheticBundle(options: SyntheticBundleOptions): { return { rootPath: "batching-bench", files }; } +/** + * Build a bundle whose every task carries a *user*-authored comment. On import, + * user comments are re-attributed to the importing user, so every issue becomes + * "touched by me" — the exact shape that used to flood the inbox after an + * import. The inbox seeding is what must keep them hidden. + */ +function buildTouchedIssuesBundle(issueCount: number): { + rootPath: string; + files: Record; +} { + const files: Record = {}; + files["COMPANY.md"] = ['---', 'schema: "agentcompanies/v1"', 'name: "Inbox Flood Co"', '---', '', 'Synthetic import bundle.', ''].join("\n"); + + const tasks: Record = {}; + for (let i = 1; i <= issueCount; i += 1) { + const slug = `task-${String(i).padStart(4, "0")}`; + files[`tasks/${slug}/TASK.md`] = ['---', `name: "Task ${i}"`, "kind: task", '---', '', `Body for task ${i}.`, ''].join("\n"); + tasks[slug] = { + status: "todo", + priority: "medium", + comments: [{ body: `A human note on task ${i}.`, authorType: "user" }], + documents: [], + }; + } + + files[".paperclip.yaml"] = renderPaperclipYaml({ schemaVersion: 6, tasks }); + return { rootPath: "inbox-flood", files }; +} + describeEmbeddedPostgres("company import batches inserts", () => { let tempDb: Awaited> | null = null; let db!: ReturnType; @@ -270,6 +301,76 @@ describeEmbeddedPostgres("company import batches inserts", () => { expect(uniqueIdentifiers.size).toBe(issueCount); }); + it("keeps imported issues out of the importing user's inbox", async () => { + const importingUserId = "board-inbox-user"; + const bundle = buildTouchedIssuesBundle(3); + const portability = companyPortabilityService(db); + + const result = await portability.importBundle( + { + source: { type: "inline", rootPath: bundle.rootPath, files: bundle.files }, + include: { company: true, agents: false, projects: false, issues: true }, + target: { mode: "new_company", newCompanyName: "Inbox Flood Co" }, + collisionStrategy: "rename", + }, + importingUserId, + ); + const companyId = result.company.id; + + const importedIssues = await db + .select({ id: issues.id }) + .from(issues) + .where(eq(issues.companyId, companyId)); + expect(importedIssues.length).toBe(3); + const importedIds = new Set(importedIssues.map((row) => row.id)); + + // Every imported issue is seeded with a per-user inbox archive, attributed + // to the importing board user (not an agent). + const archives = await db + .select({ + issueId: issueInboxArchives.issueId, + archivedByActorType: issueInboxArchives.archivedByActorType, + }) + .from(issueInboxArchives) + .where(and( + eq(issueInboxArchives.companyId, companyId), + eq(issueInboxArchives.userId, importingUserId), + )); + expect(new Set(archives.map((row) => row.issueId))).toEqual(importedIds); + expect(archives.every((row) => row.archivedByActorType === "user")).toBe(true); + + const issuesSvc = issueService(db); + + // Sanity: the imported issues really are "touched by me" (their user comment + // was re-attributed to the importing user), so without the archive filter + // they would all flood the inbox. + const touched = await issuesSvc.list(companyId, { touchedByUserId: importingUserId }); + expect(new Set(touched.map((issue) => issue.id))).toEqual(importedIds); + + // The inbox "mine" query (touched AND not inbox-archived) returns none of + // them — the seeding is load-bearing. + const mineInbox = await issuesSvc.list(companyId, { + touchedByUserId: importingUserId, + inboxArchivedByUserId: importingUserId, + }); + expect(mineInbox.filter((issue) => importedIds.has(issue.id))).toEqual([]); + + // Normal (non-import) creation is unaffected: an issue the same user creates + // after the import is not archived and shows up in their inbox. + const fresh = await issuesSvc.create(companyId, { + title: "Freshly created work", + status: "todo", + priority: "medium", + createdByUserId: importingUserId, + }); + const mineAfterCreate = await issuesSvc.list(companyId, { + touchedByUserId: importingUserId, + inboxArchivedByUserId: importingUserId, + }); + expect(mineAfterCreate.map((issue) => issue.id)).toContain(fresh.id); + expect(mineAfterCreate.filter((issue) => importedIds.has(issue.id))).toEqual([]); + }); + it("rolls back the whole work-product batch when a later chunk fails", async () => { // Seed a real company + issue to hang work products off of. const bundle = buildSyntheticBundle({ issueCount: 1, commentsPerIssue: 0, documentsPerIssue: 0 }); diff --git a/server/src/__tests__/company-portability-routes.test.ts b/server/src/__tests__/company-portability-routes.test.ts index 2de71aba9d..9a06ba9f5a 100644 --- a/server/src/__tests__/company-portability-routes.test.ts +++ b/server/src/__tests__/company-portability-routes.test.ts @@ -790,7 +790,23 @@ describe.sequential("company portability routes", () => { }), })); - const nowSpy = vi.spyOn(Date, "now").mockReturnValue(Date.parse(succeeded.body.job.completedAt) + (5 * 60 * 1000) + 1); + const completedAtMs = Date.parse(succeeded.body.job.completedAt); + + // A completed job stays resolvable well past the old 5-minute retention so a + // user who steps away during a long import never polls into a false 404. + const withinWindow = vi.spyOn(Date, "now").mockReturnValue(completedAtMs + (5 * 60 * 1000) + 1); + try { + const stillThere = await request(app).get(accepted.body.statusUrl).set(cloudHeaders); + expect(stillThere.status).toBe(200); + expect(stillThere.body.job.status).toBe("succeeded"); + expect(stillThere.body.job.result.companyId).toBe(companyId); + } finally { + withinWindow.mockRestore(); + } + + // Only past the extended 60-minute window is the in-memory record finally + // dropped, and only then does the status route report the job as gone. + const nowSpy = vi.spyOn(Date, "now").mockReturnValue(completedAtMs + (60 * 60 * 1000) + 1); try { const expired = await request(app).get(accepted.body.statusUrl).set(cloudHeaders); expect(expired.status).toBe(404); diff --git a/server/src/__tests__/company-portability.test.ts b/server/src/__tests__/company-portability.test.ts index 6f93494ec8..47214b5829 100644 --- a/server/src/__tests__/company-portability.test.ts +++ b/server/src/__tests__/company-portability.test.ts @@ -48,6 +48,7 @@ const issueSvc = { listAttachments: vi.fn(), createAttachment: vi.fn(), importIssues: vi.fn(), + archiveImportedInbox: vi.fn(), addImportedComments: vi.fn(), addImportedAttachments: vi.fn(), }; diff --git a/server/src/routes/companies.ts b/server/src/routes/companies.ts index 8a9b38df8d..516ac95d29 100644 --- a/server/src/routes/companies.ts +++ b/server/src/routes/companies.ts @@ -191,7 +191,14 @@ export function companyRoutes(db: Db, storage?: StorageService) { const artifacts = companyArtifactsService(db, storage); const feedback = feedbackService(db); const importJobs = new Map(); - const importJobTerminalRetentionMs = 5 * 60 * 1000; + // Terminal jobs are retained in memory for an hour after they settle. A long + // import can outlast a user stepping away, and dropping the completion after + // only a few minutes made a later poll 404 — surfacing a finished, fully + // written import as a scary failure. An hour is long enough to cover a real + // "walk away and come back" gap while keeping the map bounded, and it needs + // no new persistence (jobs remain in-memory-only; a restart still 404s, which + // the client now treats as a soft success once it has seen the job running). + const importJobTerminalRetentionMs = 60 * 60 * 1000; function parseBooleanQuery(value: unknown) { return value === true || value === "true" || value === "1"; diff --git a/server/src/services/company-portability.ts b/server/src/services/company-portability.ts index c49c8d2bf4..eb7a448132 100644 --- a/server/src/services/company-portability.ts +++ b/server/src/services/company-portability.ts @@ -6041,6 +6041,19 @@ export function companyPortabilityService(db: Db, storage?: StorageService) { // handful per table. Empty buffers are skipped so an issues-free import // (e.g. routines only) issues no writes at all. if (issueRows.length > 0) await issues.importIssues(targetCompany.id, issueRows); + // Imported issues are historical work, not new inbox items. Seed a + // per-user inbox archive for the importing board user so a large import + // (a real 1,418-task company shipped every task to the inbox) does not + // flood it: the inbox "mine" query hides archived issues, and genuine + // new activity still resurfaces them. Agent/system imports (no board + // user) have no inbox to protect, so the seeding is skipped. + if (actorUserId && issueRows.length > 0) { + await issues.archiveImportedInbox( + targetCompany.id, + issueRows.map((row) => row.id), + actorUserId, + ); + } if (commentRows.length > 0) await issues.addImportedComments(commentRows); if (documentRows.length > 0) await documentsSvc.createIssueDocumentsForImport(documentRows); if (workProductRows.length > 0) await workProductsSvc.createManyForImport(workProductRows); diff --git a/server/src/services/issues.ts b/server/src/services/issues.ts index f30bc272f0..721d15deee 100644 --- a/server/src/services/issues.ts +++ b/server/src/services/issues.ts @@ -5220,6 +5220,40 @@ export function issueService(db: Db) { return row; }, + /** + * Seed inbox archives for a batch of freshly imported issues so a company + * import does not flood the importing user's inbox. Imported issues are + * historical work, not new inbox items, but the inbox "mine" query surfaces + * every touched-and-not-archived issue; a 1000-task import would otherwise + * bury the inbox. Seeding a per-user inbox archive keeps them hidden via + * `inboxVisibleForUserCondition`, while genuine new activity on an imported + * issue still resurfaces it. This runs only on import (mirroring how + * `pauseAutomations` threads an import-only suppression) and never touches + * normal issue creation. Rows carry the same "user"-attributed shape a + * manual inbox archive writes, batched to stay under Postgres bind limits. + */ + archiveImportedInbox: async ( + companyId: string, + issueIds: string[], + userId: string, + archivedAt: Date = new Date(), + ): Promise => { + if (issueIds.length === 0) return; + const now = new Date(); + const rows = issueIds.map((issueId) => ({ + companyId, + issueId, + userId, + archivedByActorType: "user" as const, + archivedByAgentId: null, + archivedByRunId: null, + archivedAt, + createdAt: now, + updatedAt: now, + })); + await insertRowsInChunks(db, issueInboxArchives, rows); + }, + unarchiveInbox: async (companyId: string, issueId: string, userId: string) => { const [row] = await db .delete(issueInboxArchives) diff --git a/ui/src/api/companies.ts b/ui/src/api/companies.ts index 04c7d09daa..45f5e77b27 100644 --- a/ui/src/api/companies.ts +++ b/ui/src/api/companies.ts @@ -46,6 +46,17 @@ export interface CompanyImportJobStatus { updatedAt?: string; completedAt?: string; error?: { message: string }; + /** + * Summary retained for every terminal job (board and cloud tenant); carries + * the imported company id so the page can navigate even when the full + * `importResult` is no longer available. + */ + result?: { + companyId: string; + agentCount?: number; + warningCount?: number; + companyAction?: unknown; + }; /** Board-created jobs carry the full result for parity with the sync response. */ importResult?: CompanyPortabilityImportResult; }; diff --git a/ui/src/pages/CompanyImport.test.tsx b/ui/src/pages/CompanyImport.test.tsx index 07ff1abf4c..ac531c8ab5 100644 --- a/ui/src/pages/CompanyImport.test.tsx +++ b/ui/src/pages/CompanyImport.test.tsx @@ -659,6 +659,87 @@ describe("CompanyImport", () => { expect(container.textContent).toContain("Import complete"); }); + it("treats a server-confirmed success without a full result as a soft success and refreshes the company list", async () => { + // The server reports the job `succeeded` but retains only the compact + // summary (a cloud tenant job, or a board job whose full in-memory result + // aged out): status is a confirmed success, `importResult` is gone, and the + // summary still carries the company id. That is a success we can no longer + // fully read — never a scary failure. + mockCompaniesApi.getImportJob.mockResolvedValue({ + job: { id: "job-1", status: "succeeded", result: { companyId: "company-2" } }, + }); + + const invalidateSpy = vi.spyOn(QueryClient.prototype, "invalidateQueries"); + try { + await renderPageAndImport(); + + // Success-leaning panel, not the failure panel. + expect(container.textContent).toContain("Import completed"); + expect(container.textContent).toContain("open it to view it"); + expect(container.textContent).not.toContain("Import failed"); + expect(mockPushToast).toHaveBeenCalledWith(expect.objectContaining({ tone: "success" })); + // The company list is refreshed so the new company appears in the switcher. + expect(invalidateSpy).toHaveBeenCalledWith(expect.objectContaining({ queryKey: ["companies"] })); + // The summary's company id still drives navigation into the import. + expect(mockSetSelectedCompanyId).toHaveBeenCalledWith("company-2"); + } finally { + invalidateSpy.mockRestore(); + } + }); + + it("surfaces a first-poll 404 as an error because the job never existed", async () => { + // A 404 on the very first poll — before the client ever saw the job + // running — means the id never existed. That stays a hard error. + mockCompaniesApi.getImportJob.mockRejectedValue( + new ApiError("Import job not found", 404, { error: "Import job not found" }), + ); + + await renderPage(); + await enterGithubUrl(); + await clickButton((text) => text === "Preview import"); + await clickButton((text) => text.startsWith("Import 3 file")); + await settle(); + + expect(container.textContent).toContain("Import failed:"); + expect(container.textContent).toContain("it may have restarted while the import ran"); + expect(container.textContent).not.toContain("Import completed"); + expect(mockPushToast).toHaveBeenCalledWith(expect.objectContaining({ tone: "error" })); + }); + + it("surfaces a running-then-gone job as an error because the server restarted mid-import", async () => { + // The first poll finds the job running; the next poll 404s. A running job is + // never dropped by the retention sweep (only settled jobs age out), so its + // disappearance means the server restarted mid-import — the import never + // reached a confirmed success and may not have finished. Report that + // honestly instead of masking a possibly-incomplete import as completed. + mockCompaniesApi.getImportJob + .mockResolvedValueOnce({ job: { id: "job-1", status: "running" } }) + .mockRejectedValue(new ApiError("Import job not found", 404, { error: "Import job not found" })); + + await renderPage(); + await enterGithubUrl(); + await clickButton((text) => text === "Preview import"); + await clickButton((text) => text.startsWith("Import 3 file")); + await settle(); + + expect(container.textContent).toContain("Import failed:"); + expect(container.textContent).toContain("it may have restarted while the import ran"); + expect(container.textContent).not.toContain("Import completed"); + expect(mockPushToast).toHaveBeenCalledWith(expect.objectContaining({ tone: "error" })); + }); + + it("refreshes the company list on a full import success", async () => { + const invalidateSpy = vi.spyOn(QueryClient.prototype, "invalidateQueries"); + try { + await renderPageAndImport(); + + expect(container.textContent).toContain("Import complete"); + expect(invalidateSpy).toHaveBeenCalledWith(expect.objectContaining({ queryKey: ["companies"] })); + } finally { + invalidateSpy.mockRestore(); + } + }); + it("resumes watching a stored import job on mount", async () => { // A previous page load persisted a running job; reloading must resume // watching it rather than showing the stale form. diff --git a/ui/src/pages/CompanyImport.tsx b/ui/src/pages/CompanyImport.tsx index d2921f6d99..5aecd4a437 100644 --- a/ui/src/pages/CompanyImport.tsx +++ b/ui/src/pages/CompanyImport.tsx @@ -709,17 +709,39 @@ function runningImportJobFromError(err: unknown): CompanyImportJobAccepted | nul }; } -/** Poll a job to a terminal state; resolves with the import result or throws the job's error. */ +/** + * Terminal outcome of watching an import job. + * + * - `completed` carries the full import result and drives the activation path. + * - `completed-expired` is a *server-confirmed* success whose full result we + * can no longer read: the server reported the job `succeeded` but retained + * only its compact summary — a cloud tenant job, or a board job whose full + * in-memory result aged out. The company id lets the page still navigate to + * the imported company. This is never a failure: the server confirmed the + * import succeeded before we stopped being able to read the full result. + */ +type CompanyImportWatchOutcome = + | { status: "completed"; result: CompanyPortabilityImportResult } + | { status: "completed-expired"; companyId: string | null }; + +/** Poll a job to a terminal state; resolves with the outcome or throws the job's error. */ async function watchImportJob( jobId: string, storageKey: string, -): Promise { +): Promise { for (;;) { let job: Awaited>["job"] | null = null; try { job = (await companiesApi.getImportJob(jobId)).job; } catch (err) { if (err instanceof ApiError && err.status === 404) { + // The server has no record of this job. The retention sweep only drops + // jobs that have already *settled* (a running job is never removed), so + // a 404 while we are still watching means the in-memory record was lost + // to a restart mid-import — the import never reached a confirmed + // `succeeded` state and may not have finished. Report that honestly + // rather than masking a possibly-incomplete import as a success; the + // refreshed company list lets the user confirm what actually landed. clearStoredImportJob(storageKey); throw new Error( "The server no longer reports this import job — it may have restarted while the import ran.", @@ -748,9 +770,15 @@ async function watchImportJob( if (job?.status === "succeeded") { clearStoredImportJob(storageKey); if (!job.importResult) { - throw new Error("The import finished, but its result is no longer available."); + // The server confirmed success but retained only the compact summary + // (a cloud tenant job, or a board job whose full result aged out of + // memory). Still a success — navigate by the summary's company id. + return { + status: "completed-expired", + companyId: job.result?.companyId ?? null, + }; } - return job.importResult; + return { status: "completed", result: job.importResult }; } if (job?.status === "failed") { clearStoredImportJob(storageKey); @@ -813,11 +841,16 @@ export function CompanyImport() { // Post-import success / activation state const [pauseAutomations, setPauseAutomations] = useState(true); - const [importOutcome, setImportOutcome] = useState<{ - result: CompanyPortabilityImportResult; - dashboardPath: string; - pausedAutomations: boolean; - } | null>(null); + const [importOutcome, setImportOutcome] = useState< + | { + kind: "full"; + result: CompanyPortabilityImportResult; + dashboardPath: string; + pausedAutomations: boolean; + } + | { kind: "expired" } + | null + >(null); const [activationChecked, setActivationChecked] = useState>(new Set()); const [activatedKeys, setActivatedKeys] = useState>(new Set()); const [activationFailures, setActivationFailures] = useState>({}); @@ -1042,9 +1075,36 @@ export function CompanyImport() { }); return watchImportJob(accepted.job.id, storageKey); }, - onSuccess: async (result, { previewForImport, pauseAutomations: submittedPauseAutomations }) => { + onSuccess: async (outcome, { previewForImport, pauseAutomations: submittedPauseAutomations }) => { setResumedWatchJobId(null); + // The company list powers the switcher; refresh it on every success path + // so the imported company appears immediately without a manual reload. await queryClient.invalidateQueries({ queryKey: queryKeys.companies.all }); + + if (outcome.status === "completed-expired") { + // The import finished and wrote all its data, but the job's result + // expired (or was never retained) before we could read it. This is a + // success, not a failure: surface it gently and let the refreshed + // switcher carry the user into the new company. + if (outcome.companyId) { + try { + const importedCompany = await companiesApi.get(outcome.companyId); + setSelectedCompanyId(importedCompany.id); + } catch { + // The company id may be unreadable (permissions, race); the + // refreshed company list still surfaces the import. + } + } + setImportOutcome({ kind: "expired" }); + pushToast({ + tone: "success", + title: "Import completed", + body: "Open the company to view it.", + }); + return; + } + + const result = outcome.result; const importedCompany = await companiesApi.get(result.company.id); const refreshedSession = currentUserId ? null @@ -1063,6 +1123,7 @@ export function CompanyImport() { setActivatedKeys(new Set()); setActivationFailures({}); setImportOutcome({ + kind: "full", result, dashboardPath: `/${importedCompany.issuePrefix}/dashboard`, pausedAutomations: submittedPauseAutomations, @@ -1301,7 +1362,7 @@ export function CompanyImport() { } async function handleActivateSelected() { - if (!importOutcome || isActivating) return; + if (!importOutcome || importOutcome.kind !== "full" || isActivating) return; setIsActivating(true); const nextActivated = new Set(activatedKeys); const nextFailures: Record = {}; @@ -1374,6 +1435,24 @@ export function CompanyImport() { : null; const selectedAction = selectedFile ? (actionMap.get(selectedFile) ?? null) : null; + if (importOutcome && importOutcome.kind === "expired") { + // Soft success: the import finished and wrote all its data, but the job's + // in-memory result expired before we could read it. Never a failure — the + // company list has been refreshed, so the imported company is available + // from the switcher. + return ( +
+
+

Import completed

+

+ The import finished and your company is ready. Its detailed summary is no + longer available, but the company has been added — open it to view it. +

+
+
+ ); + } + if (importOutcome) { const { result, dashboardPath } = importOutcome; const activationItems = importOutcome.pausedAutomations ? buildActivationItems(result) : [];