fix(server): keep imported tasks quiescent under the productivity review sweep (#11191)
## 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
This commit is contained in:
parent
8f7b8b3fda
commit
d816eb8095
|
|
@ -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 });
|
||||
|
|
|
|||
|
|
@ -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();
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
|
|
|
|||
Loading…
Reference in New Issue