mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-10 12:37:55 +01:00
Address recovery action review feedback
This commit is contained in:
1 parent
b671cbda4b
commit
5b0a65980f
4 files changed
+122
-10
No files matched your search
@@ -25,6 +25,93 @@ import { recoveryService } from "../services/recovery/service.js";
|
||||
const embeddedPostgresSupport = await getEmbeddedPostgresTestSupport();
|
||||
const describeEmbeddedPostgres = embeddedPostgresSupport.supported ? describe : describe.skip;
|
||||
|
||||
function makeRecoveryActionRow(overrides: Record<string, unknown> = {}) {
|
||||
const now = new Date("2026-05-09T19:30:00.000Z");
|
||||
return {
|
||||
id: randomUUID(),
|
||||
companyId: "company-1",
|
||||
sourceIssueId: "source-1",
|
||||
recoveryIssueId: null,
|
||||
kind: "missing_disposition",
|
||||
status: "active",
|
||||
ownerType: "agent",
|
||||
ownerAgentId: "agent-1",
|
||||
ownerUserId: null,
|
||||
previousOwnerAgentId: null,
|
||||
returnOwnerAgentId: null,
|
||||
cause: "successful_run_missing_issue_disposition",
|
||||
fingerprint: "missing-disposition:fingerprint",
|
||||
evidence: {},
|
||||
nextAction: "Choose a valid issue disposition.",
|
||||
wakePolicy: null,
|
||||
monitorPolicy: null,
|
||||
attemptCount: 1,
|
||||
maxAttempts: null,
|
||||
timeoutAt: null,
|
||||
lastAttemptAt: now,
|
||||
outcome: null,
|
||||
resolutionNote: null,
|
||||
resolvedAt: null,
|
||||
createdAt: now,
|
||||
updatedAt: now,
|
||||
...overrides,
|
||||
};
|
||||
}
|
||||
|
||||
describe("issueRecoveryActionService", () => {
|
||||
it("does not reactivate an action resolved between the active read and update", async () => {
|
||||
const existingRow = makeRecoveryActionRow({ id: "existing-action", attemptCount: 1 });
|
||||
const createdRow = makeRecoveryActionRow({ id: "new-action", attemptCount: 1 });
|
||||
const selectResults = [[existingRow], []];
|
||||
|
||||
const makeSelectQuery = (rows: unknown[]) => ({
|
||||
from() {
|
||||
return this;
|
||||
},
|
||||
where() {
|
||||
return this;
|
||||
},
|
||||
orderBy() {
|
||||
return this;
|
||||
},
|
||||
limit() {
|
||||
return Promise.resolve(rows);
|
||||
},
|
||||
});
|
||||
|
||||
const fakeDb = {
|
||||
select: vi.fn(() => makeSelectQuery(selectResults.shift() ?? [])),
|
||||
update: vi.fn(() => ({
|
||||
set: vi.fn(() => ({
|
||||
where: vi.fn(() => ({
|
||||
returning: vi.fn(async () => []),
|
||||
})),
|
||||
})),
|
||||
})),
|
||||
insert: vi.fn(() => ({
|
||||
values: vi.fn(() => ({
|
||||
returning: vi.fn(async () => [createdRow]),
|
||||
})),
|
||||
})),
|
||||
};
|
||||
|
||||
const result = await issueRecoveryActionService(fakeDb as never).upsertSourceScoped({
|
||||
companyId: "company-1",
|
||||
sourceIssueId: "source-1",
|
||||
kind: "missing_disposition",
|
||||
ownerType: "agent",
|
||||
ownerAgentId: "agent-1",
|
||||
cause: "successful_run_missing_issue_disposition",
|
||||
fingerprint: "missing-disposition:fingerprint",
|
||||
nextAction: "Choose a valid issue disposition.",
|
||||
});
|
||||
|
||||
expect(result).toMatchObject({ id: "new-action", status: "active" });
|
||||
expect(fakeDb.update).toHaveBeenCalledTimes(1);
|
||||
expect(fakeDb.insert).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
});
|
||||
|
||||
if (!embeddedPostgresSupport.supported) {
|
||||
console.warn(
|
||||
`Skipping embedded Postgres issue recovery action tests on this host: ${embeddedPostgresSupport.reason ?? "unsupported environment"}`,
|
||||
|
||||
@@ -10,6 +10,7 @@ import type {
|
||||
} from "@paperclipai/shared";
|
||||
|
||||
const ACTIVE_RECOVERY_ACTION_STATUSES = ["active", "escalated"] as const satisfies readonly IssueRecoveryActionStatus[];
|
||||
const MAX_UPSERT_RETRIES = 3;
|
||||
|
||||
type IssueRecoveryActionRow = typeof issueRecoveryActions.$inferSelect;
|
||||
|
||||
@@ -128,7 +129,24 @@ export function issueRecoveryActionService(db: Db) {
|
||||
return result;
|
||||
}
|
||||
|
||||
async function upsertSourceScoped(input: UpsertIssueRecoveryActionInput): Promise<IssueRecoveryAction> {
|
||||
async function retryUpsertSourceScoped(
|
||||
input: UpsertIssueRecoveryActionInput,
|
||||
retryCount: number,
|
||||
error?: unknown,
|
||||
): Promise<IssueRecoveryAction> {
|
||||
if (retryCount >= MAX_UPSERT_RETRIES) {
|
||||
if (error) throw error;
|
||||
throw new Error(
|
||||
`Failed to upsert active recovery action for issue ${input.sourceIssueId} after ${MAX_UPSERT_RETRIES} retries`,
|
||||
);
|
||||
}
|
||||
return upsertSourceScoped(input, retryCount + 1);
|
||||
}
|
||||
|
||||
async function upsertSourceScoped(
|
||||
input: UpsertIssueRecoveryActionInput,
|
||||
retryCount = 0,
|
||||
): Promise<IssueRecoveryAction> {
|
||||
const existing = await getActiveForIssue(input.companyId, input.sourceIssueId);
|
||||
const now = new Date();
|
||||
const ownerType = input.ownerType ?? (input.ownerAgentId ? "agent" : "board");
|
||||
@@ -159,8 +177,16 @@ export function issueRecoveryActionService(db: Db) {
|
||||
resolvedAt: null,
|
||||
updatedAt: now,
|
||||
})
|
||||
.where(eq(issueRecoveryActions.id, existing.id))
|
||||
.where(
|
||||
and(
|
||||
eq(issueRecoveryActions.id, existing.id),
|
||||
inArray(issueRecoveryActions.status, [...ACTIVE_RECOVERY_ACTION_STATUSES]),
|
||||
),
|
||||
)
|
||||
.returning();
|
||||
if (!updated) {
|
||||
return retryUpsertSourceScoped(input, retryCount);
|
||||
}
|
||||
return toReadModel(updated!);
|
||||
}
|
||||
|
||||
@@ -193,9 +219,7 @@ export function issueRecoveryActionService(db: Db) {
|
||||
return toReadModel(created!);
|
||||
} catch (error) {
|
||||
if (!isUniqueRecoveryActionConflict(error)) throw error;
|
||||
const raced = await getActiveForIssue(input.companyId, input.sourceIssueId);
|
||||
if (!raced) throw error;
|
||||
return upsertSourceScoped(input);
|
||||
return retryUpsertSourceScoped(input, retryCount, error);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -132,7 +132,8 @@ describe("IssueRecoveryActionCard", () => {
|
||||
const section = node.querySelector("section[aria-label]");
|
||||
expect(section?.getAttribute("aria-label")).toBe("Recovery action: needed");
|
||||
expect(node.textContent).toContain("RECOVERY NEEDED");
|
||||
expect(node.textContent).toContain("missing_disposition");
|
||||
expect(node.textContent).toContain("Missing Disposition");
|
||||
expect(node.textContent).not.toContain("missing_disposition");
|
||||
expect(node.textContent).toContain("This issue's run finished, but no next step was chosen.");
|
||||
expect(node.textContent).toContain("ClaudeCoder");
|
||||
expect(node.textContent).toContain("CodexCoder");
|
||||
|
||||
@@ -42,10 +42,10 @@ export interface IssueRecoveryActionCardProps {
|
||||
}
|
||||
|
||||
const KIND_LABEL: Record<IssueRecoveryActionKind, string> = {
|
||||
missing_disposition: "missing_disposition",
|
||||
stranded_assigned_issue: "stranded_assigned_issue",
|
||||
active_run_watchdog: "active_run_watchdog",
|
||||
issue_graph_liveness: "issue_graph_liveness",
|
||||
missing_disposition: "Missing Disposition",
|
||||
stranded_assigned_issue: "Stranded Issue",
|
||||
active_run_watchdog: "Active Watchdog",
|
||||
issue_graph_liveness: "Graph Liveness",
|
||||
};
|
||||
|
||||
const KIND_HEADLINE: Record<IssueRecoveryActionKind, string> = {
|
||||
|
||||
Reference in new issue
Block a user