Fix routine coalescing for idle execution issues
Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
@@ -0,0 +1,6 @@
|
||||
DROP INDEX "issues_open_routine_execution_uq";--> statement-breakpoint
|
||||
CREATE UNIQUE INDEX "issues_open_routine_execution_uq" ON "issues" USING btree ("company_id","origin_kind","origin_id") WHERE "issues"."origin_kind" = 'routine_execution'
|
||||
and "issues"."origin_id" is not null
|
||||
and "issues"."hidden_at" is null
|
||||
and "issues"."execution_run_id" is not null
|
||||
and "issues"."status" in ('backlog', 'todo', 'in_progress', 'in_review', 'blocked');
|
||||
11393
packages/db/src/migrations/meta/0041_snapshot.json
Normal file
11393
packages/db/src/migrations/meta/0041_snapshot.json
Normal file
File diff suppressed because it is too large
Load Diff
@@ -288,6 +288,13 @@
|
||||
"when": 1773927102783,
|
||||
"tag": "0039_eager_shotgun",
|
||||
"breakpoints": true
|
||||
},
|
||||
{
|
||||
"idx": 41,
|
||||
"version": "7",
|
||||
"when": 1774008910991,
|
||||
"tag": "0041_reflective_captain_universe",
|
||||
"breakpoints": true
|
||||
}
|
||||
]
|
||||
}
|
||||
@@ -82,6 +82,7 @@ export const issues = pgTable(
|
||||
sql`${table.originKind} = 'routine_execution'
|
||||
and ${table.originId} is not null
|
||||
and ${table.hiddenAt} is null
|
||||
and ${table.executionRunId} is not null
|
||||
and ${table.status} in ('backlog', 'todo', 'in_progress', 'in_review', 'blocked')`,
|
||||
),
|
||||
}),
|
||||
|
||||
282
server/src/__tests__/routines-service.test.ts
Normal file
282
server/src/__tests__/routines-service.test.ts
Normal file
@@ -0,0 +1,282 @@
|
||||
import { randomUUID } from "node:crypto";
|
||||
import fs from "node:fs";
|
||||
import net from "node:net";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
import { eq } from "drizzle-orm";
|
||||
import { afterAll, afterEach, beforeAll, describe, expect, it } from "vitest";
|
||||
import {
|
||||
agents,
|
||||
applyPendingMigrations,
|
||||
companies,
|
||||
createDb,
|
||||
ensurePostgresDatabase,
|
||||
heartbeatRuns,
|
||||
issues,
|
||||
projects,
|
||||
routineRuns,
|
||||
routines,
|
||||
} from "@paperclipai/db";
|
||||
import { issueService } from "../services/issues.ts";
|
||||
import { routineService } from "../services/routines.ts";
|
||||
|
||||
type EmbeddedPostgresInstance = {
|
||||
initialise(): Promise<void>;
|
||||
start(): Promise<void>;
|
||||
stop(): Promise<void>;
|
||||
};
|
||||
|
||||
type EmbeddedPostgresCtor = new (opts: {
|
||||
databaseDir: string;
|
||||
user: string;
|
||||
password: string;
|
||||
port: number;
|
||||
persistent: boolean;
|
||||
initdbFlags?: string[];
|
||||
onLog?: (message: unknown) => void;
|
||||
onError?: (message: unknown) => void;
|
||||
}) => EmbeddedPostgresInstance;
|
||||
|
||||
async function getEmbeddedPostgresCtor(): Promise<EmbeddedPostgresCtor> {
|
||||
const mod = await import("embedded-postgres");
|
||||
return mod.default as EmbeddedPostgresCtor;
|
||||
}
|
||||
|
||||
async function getAvailablePort(): Promise<number> {
|
||||
return await new Promise((resolve, reject) => {
|
||||
const server = net.createServer();
|
||||
server.unref();
|
||||
server.on("error", reject);
|
||||
server.listen(0, "127.0.0.1", () => {
|
||||
const address = server.address();
|
||||
if (!address || typeof address === "string") {
|
||||
server.close(() => reject(new Error("Failed to allocate test port")));
|
||||
return;
|
||||
}
|
||||
const { port } = address;
|
||||
server.close((error) => {
|
||||
if (error) reject(error);
|
||||
else resolve(port);
|
||||
});
|
||||
});
|
||||
});
|
||||
}
|
||||
|
||||
async function startTempDatabase() {
|
||||
const dataDir = fs.mkdtempSync(path.join(os.tmpdir(), "paperclip-routines-service-"));
|
||||
const port = await getAvailablePort();
|
||||
const EmbeddedPostgres = await getEmbeddedPostgresCtor();
|
||||
const instance = new EmbeddedPostgres({
|
||||
databaseDir: dataDir,
|
||||
user: "paperclip",
|
||||
password: "paperclip",
|
||||
port,
|
||||
persistent: true,
|
||||
initdbFlags: ["--encoding=UTF8", "--locale=C"],
|
||||
onLog: () => {},
|
||||
onError: () => {},
|
||||
});
|
||||
await instance.initialise();
|
||||
await instance.start();
|
||||
|
||||
const adminConnectionString = `postgres://paperclip:paperclip@127.0.0.1:${port}/postgres`;
|
||||
await ensurePostgresDatabase(adminConnectionString, "paperclip");
|
||||
const connectionString = `postgres://paperclip:paperclip@127.0.0.1:${port}/paperclip`;
|
||||
await applyPendingMigrations(connectionString);
|
||||
return { connectionString, dataDir, instance };
|
||||
}
|
||||
|
||||
describe("routine service live-execution coalescing", () => {
|
||||
let db!: ReturnType<typeof createDb>;
|
||||
let instance: EmbeddedPostgresInstance | null = null;
|
||||
let dataDir = "";
|
||||
|
||||
beforeAll(async () => {
|
||||
const started = await startTempDatabase();
|
||||
db = createDb(started.connectionString);
|
||||
instance = started.instance;
|
||||
dataDir = started.dataDir;
|
||||
}, 20_000);
|
||||
|
||||
afterEach(async () => {
|
||||
await db.delete(routineRuns);
|
||||
await db.delete(routines);
|
||||
await db.delete(heartbeatRuns);
|
||||
await db.delete(issues);
|
||||
await db.delete(projects);
|
||||
await db.delete(agents);
|
||||
await db.delete(companies);
|
||||
});
|
||||
|
||||
afterAll(async () => {
|
||||
await instance?.stop();
|
||||
if (dataDir) {
|
||||
fs.rmSync(dataDir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
async function seedFixture() {
|
||||
const companyId = randomUUID();
|
||||
const agentId = randomUUID();
|
||||
const projectId = randomUUID();
|
||||
const issuePrefix = `T${companyId.replace(/-/g, "").slice(0, 6).toUpperCase()}`;
|
||||
|
||||
await db.insert(companies).values({
|
||||
id: companyId,
|
||||
name: "Paperclip",
|
||||
issuePrefix,
|
||||
requireBoardApprovalForNewAgents: false,
|
||||
});
|
||||
|
||||
await db.insert(agents).values({
|
||||
id: agentId,
|
||||
companyId,
|
||||
name: "CodexCoder",
|
||||
role: "engineer",
|
||||
status: "active",
|
||||
adapterType: "codex_local",
|
||||
adapterConfig: {},
|
||||
runtimeConfig: {},
|
||||
permissions: {},
|
||||
});
|
||||
|
||||
await db.insert(projects).values({
|
||||
id: projectId,
|
||||
companyId,
|
||||
name: "Routines",
|
||||
status: "in_progress",
|
||||
});
|
||||
|
||||
const svc = routineService(db);
|
||||
const issueSvc = issueService(db);
|
||||
const routine = await svc.create(
|
||||
companyId,
|
||||
{
|
||||
projectId,
|
||||
goalId: null,
|
||||
parentIssueId: null,
|
||||
title: "ascii frog",
|
||||
description: "Run the frog routine",
|
||||
assigneeAgentId: agentId,
|
||||
priority: "medium",
|
||||
status: "active",
|
||||
concurrencyPolicy: "coalesce_if_active",
|
||||
catchUpPolicy: "skip_missed",
|
||||
},
|
||||
{},
|
||||
);
|
||||
|
||||
return { companyId, agentId, issueSvc, projectId, routine, svc };
|
||||
}
|
||||
|
||||
it("creates a fresh execution issue when the previous routine issue is open but idle", async () => {
|
||||
const { companyId, issueSvc, routine, svc } = await seedFixture();
|
||||
const previousRunId = randomUUID();
|
||||
const previousIssue = await issueSvc.create(companyId, {
|
||||
projectId: routine.projectId,
|
||||
title: routine.title,
|
||||
description: routine.description,
|
||||
status: "todo",
|
||||
priority: routine.priority,
|
||||
assigneeAgentId: routine.assigneeAgentId,
|
||||
originKind: "routine_execution",
|
||||
originId: routine.id,
|
||||
originRunId: previousRunId,
|
||||
});
|
||||
|
||||
await db.insert(routineRuns).values({
|
||||
id: previousRunId,
|
||||
companyId,
|
||||
routineId: routine.id,
|
||||
triggerId: null,
|
||||
source: "manual",
|
||||
status: "issue_created",
|
||||
triggeredAt: new Date("2026-03-20T12:00:00.000Z"),
|
||||
linkedIssueId: previousIssue.id,
|
||||
completedAt: new Date("2026-03-20T12:00:00.000Z"),
|
||||
});
|
||||
|
||||
const detailBefore = await svc.getDetail(routine.id);
|
||||
expect(detailBefore?.activeIssue).toBeNull();
|
||||
|
||||
const run = await svc.runRoutine(routine.id, { source: "manual" });
|
||||
expect(run.status).toBe("issue_created");
|
||||
expect(run.linkedIssueId).not.toBe(previousIssue.id);
|
||||
|
||||
const routineIssues = await db
|
||||
.select({
|
||||
id: issues.id,
|
||||
originRunId: issues.originRunId,
|
||||
})
|
||||
.from(issues)
|
||||
.where(eq(issues.originId, routine.id));
|
||||
|
||||
expect(routineIssues).toHaveLength(2);
|
||||
expect(routineIssues.map((issue) => issue.id)).toContain(previousIssue.id);
|
||||
expect(routineIssues.map((issue) => issue.id)).toContain(run.linkedIssueId);
|
||||
});
|
||||
|
||||
it("coalesces only when the existing routine issue has a live execution run", async () => {
|
||||
const { agentId, companyId, issueSvc, routine, svc } = await seedFixture();
|
||||
const previousRunId = randomUUID();
|
||||
const liveHeartbeatRunId = randomUUID();
|
||||
const previousIssue = await issueSvc.create(companyId, {
|
||||
projectId: routine.projectId,
|
||||
title: routine.title,
|
||||
description: routine.description,
|
||||
status: "in_progress",
|
||||
priority: routine.priority,
|
||||
assigneeAgentId: routine.assigneeAgentId,
|
||||
originKind: "routine_execution",
|
||||
originId: routine.id,
|
||||
originRunId: previousRunId,
|
||||
});
|
||||
|
||||
await db.insert(routineRuns).values({
|
||||
id: previousRunId,
|
||||
companyId,
|
||||
routineId: routine.id,
|
||||
triggerId: null,
|
||||
source: "manual",
|
||||
status: "issue_created",
|
||||
triggeredAt: new Date("2026-03-20T12:00:00.000Z"),
|
||||
linkedIssueId: previousIssue.id,
|
||||
});
|
||||
|
||||
await db.insert(heartbeatRuns).values({
|
||||
id: liveHeartbeatRunId,
|
||||
companyId,
|
||||
agentId,
|
||||
invocationSource: "assignment",
|
||||
triggerDetail: "system",
|
||||
status: "running",
|
||||
contextSnapshot: { issueId: previousIssue.id },
|
||||
startedAt: new Date("2026-03-20T12:01:00.000Z"),
|
||||
});
|
||||
|
||||
await db
|
||||
.update(issues)
|
||||
.set({
|
||||
checkoutRunId: liveHeartbeatRunId,
|
||||
executionRunId: liveHeartbeatRunId,
|
||||
executionLockedAt: new Date("2026-03-20T12:01:00.000Z"),
|
||||
})
|
||||
.where(eq(issues.id, previousIssue.id));
|
||||
|
||||
const detailBefore = await svc.getDetail(routine.id);
|
||||
expect(detailBefore?.activeIssue?.id).toBe(previousIssue.id);
|
||||
|
||||
const run = await svc.runRoutine(routine.id, { source: "manual" });
|
||||
expect(run.status).toBe("coalesced");
|
||||
expect(run.linkedIssueId).toBe(previousIssue.id);
|
||||
expect(run.coalescedIntoRunId).toBe(previousRunId);
|
||||
|
||||
const routineIssues = await db
|
||||
.select({ id: issues.id })
|
||||
.from(issues)
|
||||
.where(eq(issues.originId, routine.id));
|
||||
|
||||
expect(routineIssues).toHaveLength(1);
|
||||
expect(routineIssues[0]?.id).toBe(previousIssue.id);
|
||||
});
|
||||
});
|
||||
@@ -1,10 +1,11 @@
|
||||
import crypto from "node:crypto";
|
||||
import { and, asc, desc, eq, inArray, isNotNull, isNull, lte, ne, or } from "drizzle-orm";
|
||||
import { and, asc, desc, eq, inArray, isNotNull, isNull, lte, ne, or, sql } from "drizzle-orm";
|
||||
import type { Db } from "@paperclipai/db";
|
||||
import {
|
||||
agents,
|
||||
companySecrets,
|
||||
goals,
|
||||
heartbeatRuns,
|
||||
issues,
|
||||
projects,
|
||||
routineRuns,
|
||||
@@ -30,6 +31,7 @@ import { secretService } from "./secrets.js";
|
||||
import { parseCron, validateCron } from "./cron.js";
|
||||
|
||||
const OPEN_ISSUE_STATUSES = ["backlog", "todo", "in_progress", "in_review", "blocked"];
|
||||
const LIVE_HEARTBEAT_RUN_STATUSES = ["queued", "running"];
|
||||
const TERMINAL_ISSUE_STATUSES = new Set(["done", "cancelled"]);
|
||||
const MAX_CATCH_UP_RUNS = 25;
|
||||
const WEEKDAY_INDEX: Record<string, number> = {
|
||||
@@ -119,8 +121,8 @@ function nextCronTickInTimeZone(expression: string, timeZone: string, after: Dat
|
||||
|
||||
function nextResultText(status: string, issueId?: string | null) {
|
||||
if (status === "issue_created" && issueId) return `Created execution issue ${issueId}`;
|
||||
if (status === "coalesced") return "Coalesced into an existing active execution issue";
|
||||
if (status === "skipped") return "Skipped because an active execution issue already exists";
|
||||
if (status === "coalesced") return "Coalesced into an existing live execution issue";
|
||||
if (status === "skipped") return "Skipped because a live execution issue already exists";
|
||||
if (status === "completed") return "Execution issue completed";
|
||||
if (status === "failed") return "Execution failed";
|
||||
return status;
|
||||
@@ -284,9 +286,9 @@ export function routineService(db: Db) {
|
||||
return map;
|
||||
}
|
||||
|
||||
async function listActiveIssueByRoutineIds(companyId: string, routineIds: string[]) {
|
||||
async function listLiveIssueByRoutineIds(companyId: string, routineIds: string[]) {
|
||||
if (routineIds.length === 0) return new Map<string, RoutineListItem["activeIssue"]>();
|
||||
const rows = await db
|
||||
const executionBoundRows = await db
|
||||
.selectDistinctOn([issues.originId], {
|
||||
originId: issues.originId,
|
||||
id: issues.id,
|
||||
@@ -297,6 +299,13 @@ export function routineService(db: Db) {
|
||||
updatedAt: issues.updatedAt,
|
||||
})
|
||||
.from(issues)
|
||||
.innerJoin(
|
||||
heartbeatRuns,
|
||||
and(
|
||||
eq(heartbeatRuns.id, issues.executionRunId),
|
||||
inArray(heartbeatRuns.status, LIVE_HEARTBEAT_RUN_STATUSES),
|
||||
),
|
||||
)
|
||||
.where(
|
||||
and(
|
||||
eq(issues.companyId, companyId),
|
||||
@@ -308,8 +317,52 @@ export function routineService(db: Db) {
|
||||
)
|
||||
.orderBy(issues.originId, desc(issues.updatedAt), desc(issues.createdAt));
|
||||
|
||||
const rowsByOriginId = new Map<string, (typeof executionBoundRows)[number]>();
|
||||
for (const row of executionBoundRows) {
|
||||
if (!row.originId) continue;
|
||||
rowsByOriginId.set(row.originId, row);
|
||||
}
|
||||
|
||||
const missingRoutineIds = routineIds.filter((routineId) => !rowsByOriginId.has(routineId));
|
||||
if (missingRoutineIds.length > 0) {
|
||||
const legacyRows = await db
|
||||
.selectDistinctOn([issues.originId], {
|
||||
originId: issues.originId,
|
||||
id: issues.id,
|
||||
identifier: issues.identifier,
|
||||
title: issues.title,
|
||||
status: issues.status,
|
||||
priority: issues.priority,
|
||||
updatedAt: issues.updatedAt,
|
||||
})
|
||||
.from(issues)
|
||||
.innerJoin(
|
||||
heartbeatRuns,
|
||||
and(
|
||||
eq(heartbeatRuns.companyId, issues.companyId),
|
||||
inArray(heartbeatRuns.status, LIVE_HEARTBEAT_RUN_STATUSES),
|
||||
sql`${heartbeatRuns.contextSnapshot} ->> 'issueId' = cast(${issues.id} as text)`,
|
||||
),
|
||||
)
|
||||
.where(
|
||||
and(
|
||||
eq(issues.companyId, companyId),
|
||||
eq(issues.originKind, "routine_execution"),
|
||||
inArray(issues.originId, missingRoutineIds),
|
||||
inArray(issues.status, OPEN_ISSUE_STATUSES),
|
||||
isNull(issues.hiddenAt),
|
||||
),
|
||||
)
|
||||
.orderBy(issues.originId, desc(issues.updatedAt), desc(issues.createdAt));
|
||||
|
||||
for (const row of legacyRows) {
|
||||
if (!row.originId) continue;
|
||||
rowsByOriginId.set(row.originId, row);
|
||||
}
|
||||
}
|
||||
|
||||
const map = new Map<string, RoutineListItem["activeIssue"]>();
|
||||
for (const row of rows) {
|
||||
for (const row of rowsByOriginId.values()) {
|
||||
if (!row.originId) continue;
|
||||
map.set(row.originId, {
|
||||
id: row.id,
|
||||
@@ -353,10 +406,17 @@ export function routineService(db: Db) {
|
||||
}
|
||||
}
|
||||
|
||||
async function findOpenExecutionIssue(routine: typeof routines.$inferSelect) {
|
||||
return db
|
||||
async function findLiveExecutionIssue(routine: typeof routines.$inferSelect) {
|
||||
const executionBoundIssue = await db
|
||||
.select()
|
||||
.from(issues)
|
||||
.innerJoin(
|
||||
heartbeatRuns,
|
||||
and(
|
||||
eq(heartbeatRuns.id, issues.executionRunId),
|
||||
inArray(heartbeatRuns.status, LIVE_HEARTBEAT_RUN_STATUSES),
|
||||
),
|
||||
)
|
||||
.where(
|
||||
and(
|
||||
eq(issues.companyId, routine.companyId),
|
||||
@@ -368,7 +428,32 @@ export function routineService(db: Db) {
|
||||
)
|
||||
.orderBy(desc(issues.updatedAt), desc(issues.createdAt))
|
||||
.limit(1)
|
||||
.then((rows) => rows[0] ?? null);
|
||||
.then((rows) => rows[0]?.issues ?? null);
|
||||
if (executionBoundIssue) return executionBoundIssue;
|
||||
|
||||
return db
|
||||
.select()
|
||||
.from(issues)
|
||||
.innerJoin(
|
||||
heartbeatRuns,
|
||||
and(
|
||||
eq(heartbeatRuns.companyId, issues.companyId),
|
||||
inArray(heartbeatRuns.status, LIVE_HEARTBEAT_RUN_STATUSES),
|
||||
sql`${heartbeatRuns.contextSnapshot} ->> 'issueId' = cast(${issues.id} as text)`,
|
||||
),
|
||||
)
|
||||
.where(
|
||||
and(
|
||||
eq(issues.companyId, routine.companyId),
|
||||
eq(issues.originKind, "routine_execution"),
|
||||
eq(issues.originId, routine.id),
|
||||
inArray(issues.status, OPEN_ISSUE_STATUSES),
|
||||
isNull(issues.hiddenAt),
|
||||
),
|
||||
)
|
||||
.orderBy(desc(issues.updatedAt), desc(issues.createdAt))
|
||||
.limit(1)
|
||||
.then((rows) => rows[0]?.issues ?? null);
|
||||
}
|
||||
|
||||
async function finalizeRun(runId: string, patch: Partial<typeof routineRuns.$inferInsert>) {
|
||||
@@ -460,7 +545,7 @@ export function routineService(db: Db) {
|
||||
: undefined;
|
||||
|
||||
try {
|
||||
const activeIssue = await findOpenExecutionIssue(input.routine);
|
||||
const activeIssue = await findLiveExecutionIssue(input.routine);
|
||||
if (activeIssue && input.routine.concurrencyPolicy !== "always_enqueue") {
|
||||
const status = input.routine.concurrencyPolicy === "skip_if_active" ? "skipped" : "coalesced";
|
||||
const updated = await finalizeRun(run.id, {
|
||||
@@ -507,7 +592,7 @@ export function routineService(db: Db) {
|
||||
throw error;
|
||||
}
|
||||
|
||||
const existingIssue = await findOpenExecutionIssue(input.routine);
|
||||
const existingIssue = await findLiveExecutionIssue(input.routine);
|
||||
if (!existingIssue) throw error;
|
||||
const status = input.routine.concurrencyPolicy === "skip_if_active" ? "skipped" : "coalesced";
|
||||
const updated = await finalizeRun(run.id, {
|
||||
@@ -572,7 +657,7 @@ export function routineService(db: Db) {
|
||||
const [triggersByRoutine, latestRunByRoutine, activeIssueByRoutine] = await Promise.all([
|
||||
listTriggersForRoutineIds(companyId, routineIds),
|
||||
listLatestRunByRoutineIds(companyId, routineIds),
|
||||
listActiveIssueByRoutineIds(companyId, routineIds),
|
||||
listLiveIssueByRoutineIds(companyId, routineIds),
|
||||
]);
|
||||
return rows.map((row) => ({
|
||||
...row,
|
||||
@@ -665,7 +750,7 @@ export function routineService(db: Db) {
|
||||
: null,
|
||||
})),
|
||||
),
|
||||
findOpenExecutionIssue(row),
|
||||
findLiveExecutionIssue(row),
|
||||
]);
|
||||
|
||||
return {
|
||||
|
||||
Reference in New Issue
Block a user