mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-01 15:35:12 +01:00
fix(feedback): handle paused delivery assignees
Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
1 parent
ea34e9b25b
commit
c26ff70959
12 files changed
+297
-5
No files matched your search
@@ -0,0 +1,168 @@
|
||||
import { randomUUID } from "node:crypto";
|
||||
import express from "express";
|
||||
import request from "supertest";
|
||||
import { afterAll, afterEach, beforeAll, describe, expect, it } from "vitest";
|
||||
import {
|
||||
activityLog,
|
||||
agents,
|
||||
agentWakeupRequests,
|
||||
companies,
|
||||
createDb,
|
||||
issueRecoveryActions,
|
||||
issues,
|
||||
} from "@paperclipai/db";
|
||||
import {
|
||||
getEmbeddedPostgresTestSupport,
|
||||
startEmbeddedPostgresTestDatabase,
|
||||
} from "./helpers/embedded-postgres.js";
|
||||
import { errorHandler } from "../middleware/index.js";
|
||||
import { issueRoutes } from "../routes/issues.js";
|
||||
import {
|
||||
FEEDBACK_DELIVERY_RECOVERY_ACTION_KIND,
|
||||
FEEDBACK_DELIVERY_RECOVERY_CAUSE,
|
||||
} from "../services/recovery/feedback-delivery.js";
|
||||
|
||||
const embeddedPostgresSupport = await getEmbeddedPostgresTestSupport();
|
||||
const describeEmbeddedPostgres = embeddedPostgresSupport.supported ? describe : describe.skip;
|
||||
|
||||
if (!embeddedPostgresSupport.supported) {
|
||||
console.warn(
|
||||
`Skipping embedded Postgres feedback-delivery retry route tests on this host: ${
|
||||
embeddedPostgresSupport.reason ?? "unsupported environment"
|
||||
}`,
|
||||
);
|
||||
}
|
||||
|
||||
describeEmbeddedPostgres("issue feedback-delivery retry routes", () => {
|
||||
let db!: ReturnType<typeof createDb>;
|
||||
let tempDb: Awaited<ReturnType<typeof startEmbeddedPostgresTestDatabase>> | null = null;
|
||||
|
||||
beforeAll(async () => {
|
||||
tempDb = await startEmbeddedPostgresTestDatabase("paperclip-feedback-delivery-retry-routes-");
|
||||
db = createDb(tempDb.connectionString);
|
||||
}, 20_000);
|
||||
|
||||
afterEach(async () => {
|
||||
await db.delete(issueRecoveryActions);
|
||||
await db.delete(activityLog);
|
||||
await db.delete(issues);
|
||||
await db.delete(agentWakeupRequests);
|
||||
await db.delete(agents);
|
||||
await db.delete(companies);
|
||||
});
|
||||
|
||||
afterAll(async () => {
|
||||
await tempDb?.cleanup();
|
||||
});
|
||||
|
||||
function createApp(companyId: string) {
|
||||
const app = express();
|
||||
app.use(express.json());
|
||||
app.use((req, _res, next) => {
|
||||
req.actor = {
|
||||
type: "board",
|
||||
userId: "board-user",
|
||||
companyIds: [companyId],
|
||||
memberships: [{ companyId, membershipRole: "admin", status: "active" }],
|
||||
isInstanceAdmin: false,
|
||||
source: "session",
|
||||
};
|
||||
next();
|
||||
});
|
||||
app.use("/api", issueRoutes(db, {} as any));
|
||||
app.use(errorHandler);
|
||||
return app;
|
||||
}
|
||||
|
||||
async function seedExhaustedDelivery(input: { agentStatus?: string; withAgent?: boolean } = {}) {
|
||||
const companyId = randomUUID();
|
||||
const issueId = randomUUID();
|
||||
const agentId = input.withAgent === false ? null : randomUUID();
|
||||
const rootWakeupRequestId = randomUUID();
|
||||
const sourceCommentId = randomUUID();
|
||||
const issuePrefix = `F${companyId.replace(/-/g, "").slice(0, 6).toUpperCase()}`;
|
||||
|
||||
await db.insert(companies).values({
|
||||
id: companyId,
|
||||
name: "Paperclip",
|
||||
issuePrefix,
|
||||
requireBoardApprovalForNewAgents: false,
|
||||
});
|
||||
if (agentId) {
|
||||
await db.insert(agents).values({
|
||||
id: agentId,
|
||||
companyId,
|
||||
name: "CodexCoder",
|
||||
role: "engineer",
|
||||
status: input.agentStatus ?? "paused",
|
||||
adapterType: "codex_local",
|
||||
adapterConfig: {},
|
||||
runtimeConfig: {},
|
||||
permissions: {},
|
||||
});
|
||||
}
|
||||
await db.insert(issues).values({
|
||||
id: issueId,
|
||||
companyId,
|
||||
title: "Feedback delivery exhausted",
|
||||
status: "in_review",
|
||||
priority: "medium",
|
||||
assigneeAgentId: agentId,
|
||||
issueNumber: 1,
|
||||
identifier: `${issuePrefix}-1`,
|
||||
});
|
||||
await db.insert(issueRecoveryActions).values({
|
||||
companyId,
|
||||
sourceIssueId: issueId,
|
||||
kind: FEEDBACK_DELIVERY_RECOVERY_ACTION_KIND,
|
||||
status: "active",
|
||||
ownerType: agentId ? "agent" : "board",
|
||||
ownerAgentId: agentId,
|
||||
previousOwnerAgentId: agentId,
|
||||
returnOwnerAgentId: agentId,
|
||||
cause: FEEDBACK_DELIVERY_RECOVERY_CAUSE,
|
||||
fingerprint: `feedback_delivery:${companyId}:${issueId}:${agentId ?? "missing"}:${rootWakeupRequestId}`,
|
||||
evidence: {
|
||||
rootWakeupRequestId,
|
||||
outstandingCommentIds: [sourceCommentId],
|
||||
},
|
||||
nextAction: "Deliver the outstanding feedback",
|
||||
attemptCount: 1,
|
||||
maxAttempts: 1,
|
||||
});
|
||||
|
||||
return { companyId, issueId };
|
||||
}
|
||||
|
||||
it("returns a paused reason instead of leaking the wakeup 409", async () => {
|
||||
const { companyId, issueId } = await seedExhaustedDelivery({ agentStatus: "paused" });
|
||||
|
||||
const res = await request(createApp(companyId))
|
||||
.post(`/api/issues/${issueId}/feedback-delivery/retry`)
|
||||
.send({});
|
||||
|
||||
expect(res.status, JSON.stringify(res.body)).toBe(200);
|
||||
expect(res.body).toMatchObject({
|
||||
outcome: "no_invokable_assignee",
|
||||
reason: "paused",
|
||||
message: "This delivery's agent is paused. Unpause the agent before retrying.",
|
||||
});
|
||||
expect(await db.select().from(agentWakeupRequests)).toHaveLength(0);
|
||||
expect(await db.select().from(activityLog)).toHaveLength(0);
|
||||
});
|
||||
|
||||
it("distinguishes a missing assignee from a paused one", async () => {
|
||||
const { companyId, issueId } = await seedExhaustedDelivery({ withAgent: false });
|
||||
|
||||
const res = await request(createApp(companyId))
|
||||
.post(`/api/issues/${issueId}/feedback-delivery/retry`)
|
||||
.send({});
|
||||
|
||||
expect(res.status, JSON.stringify(res.body)).toBe(200);
|
||||
expect(res.body).toMatchObject({
|
||||
outcome: "no_invokable_assignee",
|
||||
reason: "missing",
|
||||
message: "This delivery has no agent to retry against. Reassign the task first.",
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -113,6 +113,7 @@ import {
|
||||
FEEDBACK_DELIVERY_RETRY_WAKE_REASON,
|
||||
FEEDBACK_DELIVERY_WAKE_COMMENT_IDS_KEY,
|
||||
} from "../services/recovery/feedback-delivery.js";
|
||||
import { evaluateAgentInvokabilityFromDb } from "../services/agent-invokability.js";
|
||||
import { getTelemetryClient } from "../telemetry.js";
|
||||
import { isUniqueViolation } from "../db-errors.js";
|
||||
import type { StorageService } from "../storage/types.js";
|
||||
@@ -10826,6 +10827,7 @@ export function issueRoutes(
|
||||
const states = await readStates();
|
||||
res.json({
|
||||
outcome: "not_exhausted",
|
||||
reason: null,
|
||||
message: "This task has no exhausted feedback delivery to retry.",
|
||||
feedbackDelivery: states.values().next().value ?? null,
|
||||
} satisfies IssueFeedbackDeliveryRetryResponse);
|
||||
@@ -10849,12 +10851,40 @@ export function issueRoutes(
|
||||
if (!targetAgentId || !rootWakeupRequestId) {
|
||||
res.json({
|
||||
outcome: "no_invokable_assignee",
|
||||
reason: "missing",
|
||||
message: "This delivery has no agent to retry against. Reassign the task first.",
|
||||
feedbackDelivery: (await readStates()).get(outstandingCommentIds[0] ?? "") ?? null,
|
||||
} satisfies IssueFeedbackDeliveryRetryResponse);
|
||||
return;
|
||||
}
|
||||
|
||||
const targetAgent = await db
|
||||
.select({
|
||||
id: agents.id,
|
||||
companyId: agents.companyId,
|
||||
name: agents.name,
|
||||
reportsTo: agents.reportsTo,
|
||||
status: agents.status,
|
||||
})
|
||||
.from(agents)
|
||||
.where(and(eq(agents.id, targetAgentId), eq(agents.companyId, issue.companyId)))
|
||||
.then((rows) => rows[0] ?? null);
|
||||
const invokability = await evaluateAgentInvokabilityFromDb(db, targetAgent);
|
||||
if (!invokability.invokable) {
|
||||
const message = invokability.reason === "paused"
|
||||
? "This delivery's agent is paused. Unpause the agent before retrying."
|
||||
: invokability.reason === "pending_approval"
|
||||
? "This delivery's agent is waiting for approval. Approve or reassign the agent before retrying."
|
||||
: "This delivery has no invokable agent to retry against. Reassign the task first.";
|
||||
res.json({
|
||||
outcome: "no_invokable_assignee",
|
||||
reason: invokability.reason,
|
||||
message,
|
||||
feedbackDelivery: (await readStates()).get(outstandingCommentIds[0] ?? "") ?? null,
|
||||
} satisfies IssueFeedbackDeliveryRetryResponse);
|
||||
return;
|
||||
}
|
||||
|
||||
const statesBefore = await readStates();
|
||||
const alreadyInFlight = outstandingCommentIds.some(
|
||||
(commentId) => statesBefore.get(commentId)?.retryInFlight === true,
|
||||
@@ -10862,6 +10892,7 @@ export function issueRoutes(
|
||||
if (alreadyInFlight) {
|
||||
res.json({
|
||||
outcome: "already_queued",
|
||||
reason: null,
|
||||
message: "A delivery retry is already queued.",
|
||||
feedbackDelivery: statesBefore.get(outstandingCommentIds[0] ?? "") ?? null,
|
||||
} satisfies IssueFeedbackDeliveryRetryResponse);
|
||||
@@ -10919,6 +10950,7 @@ export function issueRoutes(
|
||||
const projected = latestCommentId ? statesAfter.get(latestCommentId) ?? null : null;
|
||||
res.json({
|
||||
outcome: "queued",
|
||||
reason: null,
|
||||
message: "Retrying feedback delivery.",
|
||||
// Deferred behind a live run still reads as in-flight to the operator, so
|
||||
// fall back to a synthetic in-flight projection rather than reporting a
|
||||
|
||||
Reference in new issue
Block a user