From d816eb80950384cb95b8bc9980e84bd42c0a2ab9 Mon Sep 17 00:00:00 2001 From: Devin Foley Date: Mon, 10 Aug 2026 17:00:32 -0700 Subject: [PATCH] fix(server): keep imported tasks quiescent under the productivity review sweep (#11191) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Company import brings a full company package — agents, tasks, routines — into an instance, with `pauseAutomations` promising a quiet landing > - The pause covers the imported entities, but the destination's own productivity-review sweep does not know the difference between imported rows and live work > - The importer stamps every imported in-progress task with `startedAt = now()`, so six hours later the sweep's long-active check fires on every one of them and floods the board with review tasks and agent wakeups > - This pull request stops fabricating `startedAt` on import and makes the sweep skip tasks whose assignee agent is paused > - The benefit is that an import lands quietly: no surprise review-task storm, and paused teams stay paused until the operator activates them ## Linked Issues or Issue Description **What happened?** After importing a company package with automations paused, a batch of "productivity review" tasks appeared roughly six hours later — one for every imported in-progress task — each with an owner-agent wakeup. The user described it as jarring and wasteful. Cause: `importIssues` fabricates `startedAt = now()` for imported in-progress rows, and `reconcileProductivityReviews` considers any assigned in-progress task without checking whether the assignee agent is paused, so its long-active-duration evidence (6 h threshold) trips on the fabricated timestamp. **Expected behavior** An import with paused automations must be quiescent: no destination sweep should generate work from imported rows until the operator unpauses the imported team. A paused agent must not accumulate review tasks it cannot act on. **Steps to reproduce** 1. Import a company package containing tasks with status `in_progress` assigned to agents, with "pause automations" enabled. 2. Wait for the productivity-review reconcile (runs at startup and on the heartbeat scheduler tick) more than six hours after the import. 3. Observe one new review task plus an owner wakeup per imported in-progress task. ## What Changed - `importIssues` no longer fabricates `startedAt` for imported `in_progress` rows; it inserts null (`server/src/services/issues.ts`). Audited every consumer of `issues.startedAt` — all are null-tolerant, and normal checkout/status-transition paths set the value when work really starts. - `reconcileProductivityReviews` skips candidates whose assignee agent is `paused`, counting them as skipped (`server/src/services/productivity-review.ts`). This is a general rule, not import-specific: a paused agent cannot act on a review. - Tests: paused-assignee candidate with an old `startedAt` creates no review, and creates one after unpausing; imported in-progress issue lands with null `startedAt` (embedded-Postgres import test); the pre-existing long-active regression test still passes. ## Verification - `pnpm vitest run server/src/__tests__/productivity-review-service.test.ts server/src/__tests__/company-portability-import-batching.test.ts` — 20 passed, 1 pre-existing opt-in benchmark skip. - `pnpm vitest run server/src/__tests__/company-portability.test.ts` — 78 passed. - `pnpm --filter @paperclipai/server typecheck` — clean. ## Risks - Behavior change beyond imports: tasks assigned to paused agents no longer receive productivity reviews anywhere. This is intended — the review would target an agent that cannot respond — and reviews resume on the first reconcile after unpausing. - Imported in-progress tasks now carry no `startedAt` until real work starts on the destination. The one sweep that read the fabricated value is the one this PR quiets; all other consumers fall back safely (audit in the commit body). - Low risk otherwise: no schema change, no API shape change. ## Model Used - Claude Fable 5 (`claude-fable-5`) via Claude Code CLI, with extended thinking and tool use (multi-agent implementation with independent verification). ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --- ...ompany-portability-import-batching.test.ts | 52 +++++++++++++++++++ .../productivity-review-service.test.ts | 22 ++++++++ server/src/services/issues.ts | 4 +- server/src/services/productivity-review.ts | 5 ++ 4 files changed, 82 insertions(+), 1 deletion(-) diff --git a/server/src/__tests__/company-portability-import-batching.test.ts b/server/src/__tests__/company-portability-import-batching.test.ts index 4c32bfe5cd..53ed945bc1 100644 --- a/server/src/__tests__/company-portability-import-batching.test.ts +++ b/server/src/__tests__/company-portability-import-batching.test.ts @@ -1,6 +1,8 @@ import { afterAll, beforeAll, beforeEach, describe, expect, it } from "vitest"; import { + agents, assets, + companies, createDb, documentRevisions, documents, @@ -371,6 +373,56 @@ describeEmbeddedPostgres("company import batches inserts", () => { expect(mineAfterCreate.filter((issue) => importedIds.has(issue.id))).toEqual([]); }); + it("imports in_progress issues with a null startedAt", async () => { + const companyId = randomUUID(); + const agentId = randomUUID(); + await db.insert(companies).values({ + id: companyId, + name: "Carried Over Co", + issuePrefix: "CAR", + requireBoardApprovalForNewAgents: false, + }); + // pauseAutomations imports assignees paused; paused agents keep assignments. + await db.insert(agents).values({ + id: agentId, + companyId, + name: "Carried Coder", + role: "engineer", + status: "paused", + adapterType: "codex_local", + adapterConfig: {}, + runtimeConfig: {}, + permissions: {}, + }); + + await issueService(db).importIssues(companyId, [{ + id: randomUUID(), + ref: "carried-over", + projectId: null, + projectWorkspaceId: null, + title: "Carried-over work", + description: null, + assigneeAgentId: agentId, + status: "in_progress", + priority: "medium", + billingCode: null, + assigneeAdapterOverrides: null, + executionWorkspaceSettings: null, + labelIds: [], + monitorNotes: null, + monitorScheduledBy: null, + }]); + + const [imported] = await db + .select({ status: issues.status, startedAt: issues.startedAt }) + .from(issues) + .where(eq(issues.companyId, companyId)); + expect(imported?.status).toBe("in_progress"); + // A fabricated import-time startedAt made carried-over work look hours + // stale to duration-based sweeps (e.g. the productivity review). + expect(imported?.startedAt).toBeNull(); + }); + 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__/productivity-review-service.test.ts b/server/src/__tests__/productivity-review-service.test.ts index 12b3bdd18c..a45e4502ed 100644 --- a/server/src/__tests__/productivity-review-service.test.ts +++ b/server/src/__tests__/productivity-review-service.test.ts @@ -505,6 +505,28 @@ describeEmbeddedPostgres("productivity review service", () => { expect(hold.held).toBe(false); }); + it("skips a long-active candidate while its assignee is paused and reviews it once unpaused", async () => { + const now = new Date("2026-04-28T12:00:00.000Z"); + const seeded = await seedAssignedIssue({ + status: "in_progress", + startedAt: new Date(now.getTime() - 7 * 60 * 60 * 1000), + }); + await db.update(agents).set({ status: "paused" }).where(eq(agents.id, seeded.coderId)); + const service = productivityReviewService(db); + + const pausedResult = await service.reconcileProductivityReviews({ now, companyId: seeded.companyId }); + + expect(pausedResult.created).toBe(0); + expect(pausedResult.skipped).toBe(1); + expect(await listProductivityReviews(seeded.companyId)).toHaveLength(0); + + await db.update(agents).set({ status: "idle" }).where(eq(agents.id, seeded.coderId)); + const unpausedResult = await service.reconcileProductivityReviews({ now, companyId: seeded.companyId }); + + expect(unpausedResult.created).toBe(1); + expect(await listProductivityReviews(seeded.companyId)).toHaveLength(1); + }); + it("creates a high-churn review even when every sampled run has a progress comment", async () => { const now = new Date("2026-04-28T12:00:00.000Z"); const seeded = await seedAssignedIssue(); diff --git a/server/src/services/issues.ts b/server/src/services/issues.ts index 347168911b..16dbd66207 100644 --- a/server/src/services/issues.ts +++ b/server/src/services/issues.ts @@ -7352,7 +7352,9 @@ export function issueService(db: Db) { responsibleUserId: null, requestDepth: clampIssueRequestDepth(undefined), originKind: "manual", - startedAt: row.status === "in_progress" ? new Date() : null, + // Imported in-progress work did not start at import time; fabricating + // startedAt here trips duration-based sweeps (e.g. productivity review). + startedAt: null, completedAt: row.status === "done" ? new Date() : null, cancelledAt: row.status === "cancelled" ? new Date() : null, monitorNotes: row.monitorNotes ?? null, diff --git a/server/src/services/productivity-review.ts b/server/src/services/productivity-review.ts index 199b122421..909682d675 100644 --- a/server/src/services/productivity-review.ts +++ b/server/src/services/productivity-review.ts @@ -896,6 +896,11 @@ export function productivityReviewService(db: Db, deps?: { enqueueWakeup?: Enque result.skipped += 1; continue; } + // A paused assignee cannot act on a review, so raising one only creates noise. + if (sourceAgent.status === "paused") { + result.skipped += 1; + continue; + } const evidence = await collectEvidence(candidate, sourceAgent, thresholds, now); if (!evidence) { result.skipped += 1;