mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-25 09:41:04 +01:00
fix(recovery): make delegation race safe
Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
1 parent
1ca659d9e9
commit
dbbe8bdfb5
2 files changed
+95
-19
No files matched your search
@@ -1525,7 +1525,7 @@ describeEmbeddedPostgres("issue recovery actions", () => {
|
||||
await request(app)
|
||||
.post(`/api/issues/${sourceIssueId}/recovery-actions/delegate`)
|
||||
.send({ actionId: action.id, target: "ceo" })
|
||||
.expect(403);
|
||||
.expect(404);
|
||||
|
||||
const delegatedIssues = await db
|
||||
.select()
|
||||
@@ -1539,6 +1539,53 @@ describeEmbeddedPostgres("issue recovery actions", () => {
|
||||
expect(actionRow?.status).toBe("active");
|
||||
});
|
||||
|
||||
it("returns the delegated recovery result to concurrent callers", async () => {
|
||||
const { companyId, managerId, sourceIssueId } = await seedCompany();
|
||||
await seedCeo(companyId);
|
||||
const recoveryActionSvc = issueRecoveryActionService(db);
|
||||
const action = await recoveryActionSvc.upsertSourceScoped({
|
||||
companyId,
|
||||
sourceIssueId,
|
||||
kind: "workspace_validation",
|
||||
ownerType: "agent",
|
||||
ownerAgentId: managerId,
|
||||
cause: "workspace_validation_failed",
|
||||
fingerprint: "workspace-validation:concurrent-delegation",
|
||||
evidence: { reason: "git_worktree_branch_incoherence" },
|
||||
nextAction: "Repair the source issue workspace link before resuming.",
|
||||
wakePolicy: { type: "manual_repair_required" },
|
||||
});
|
||||
const app = createApp();
|
||||
|
||||
const responses = await Promise.all([
|
||||
request(app)
|
||||
.post(`/api/issues/${sourceIssueId}/recovery-actions/delegate`)
|
||||
.send({ actionId: action.id, target: "ceo" }),
|
||||
request(app)
|
||||
.post(`/api/issues/${sourceIssueId}/recovery-actions/delegate`)
|
||||
.send({ actionId: action.id, target: "ceo" }),
|
||||
]);
|
||||
|
||||
expect(responses.map((response) => response.status).sort()).toEqual([200, 201]);
|
||||
expect(responses[0].body.recoveryIssue.id).toBe(responses[1].body.recoveryIssue.id);
|
||||
expect(responses).toEqual(
|
||||
expect.arrayContaining([
|
||||
expect.objectContaining({
|
||||
body: expect.objectContaining({
|
||||
issue: expect.objectContaining({ activeRecoveryAction: null }),
|
||||
reused: true,
|
||||
}),
|
||||
}),
|
||||
]),
|
||||
);
|
||||
|
||||
const delegatedIssues = await db
|
||||
.select()
|
||||
.from(issues)
|
||||
.where(and(eq(issues.companyId, companyId), eq(issues.originKind, "delegated_recovery")));
|
||||
expect(delegatedIssues).toHaveLength(1);
|
||||
});
|
||||
|
||||
it("rejects peer-agent delegated recovery outside the recovery authority boundary", async () => {
|
||||
const { companyId, managerId, sourceIssueId } = await seedCompany();
|
||||
await seedCeo(companyId);
|
||||
|
||||
+47
-18
@@ -4374,16 +4374,28 @@ export function issueRoutes(
|
||||
}
|
||||
|
||||
function isDelegatedRecoveryIssueConflict(error: unknown) {
|
||||
const maybe = error as { code?: string; constraint?: string; constraint_name?: string; message?: string } | null;
|
||||
return Boolean(
|
||||
maybe &&
|
||||
maybe.code === "23505" &&
|
||||
(
|
||||
maybe.constraint === "issues_delegated_recovery_fingerprint_uq" ||
|
||||
maybe.constraint_name === "issues_delegated_recovery_fingerprint_uq" ||
|
||||
typeof maybe.message === "string" && maybe.message.includes("issues_delegated_recovery_fingerprint_uq")
|
||||
),
|
||||
);
|
||||
const constraintName = "issues_delegated_recovery_fingerprint_uq";
|
||||
const queue: unknown[] = [error];
|
||||
const messages: string[] = [];
|
||||
let hasUniqueCode = false;
|
||||
let hasConstraint = false;
|
||||
for (const candidate of queue) {
|
||||
if (!candidate || typeof candidate !== "object") continue;
|
||||
const typed = candidate as {
|
||||
code?: string;
|
||||
constraint?: string;
|
||||
constraint_name?: string;
|
||||
cause?: unknown;
|
||||
message?: string;
|
||||
};
|
||||
if (typed.code === "23505") hasUniqueCode = true;
|
||||
if (typed.constraint === constraintName || typed.constraint_name === constraintName) hasConstraint = true;
|
||||
if (typed.message) messages.push(typed.message);
|
||||
if (typed.cause) queue.push(typed.cause);
|
||||
}
|
||||
const message = messages.join("\n");
|
||||
return (hasUniqueCode || message.includes("duplicate key value violates unique constraint")) &&
|
||||
(hasConstraint || message.includes(constraintName));
|
||||
}
|
||||
|
||||
async function getDelegatedRecoveryIssue(companyId: string, fingerprint: string) {
|
||||
@@ -5728,12 +5740,8 @@ export function issueRoutes(
|
||||
validate(delegateIssueRecoveryActionSchema),
|
||||
async (req, res) => {
|
||||
const id = req.params.id as string;
|
||||
const existing = await svc.getById(id);
|
||||
if (!existing) {
|
||||
res.status(404).json({ error: "Issue not found" });
|
||||
return;
|
||||
}
|
||||
assertCompanyAccess(req, existing.companyId);
|
||||
const existing = await getAccessibleResource(req, res, svc.getById(id), "Issue not found");
|
||||
if (!existing) return;
|
||||
if (!(await assertAgentIssueMutationAllowed(req, res, existing))) return;
|
||||
|
||||
const { actionId, target } = req.body;
|
||||
@@ -5840,7 +5848,7 @@ export function issueRoutes(
|
||||
});
|
||||
if (!updatedSource) throw notFound("Issue not found");
|
||||
|
||||
const recoveryAction = await recoveryActionsSvc.resolveActiveForIssue({
|
||||
let recoveryAction = await recoveryActionsSvc.resolveActiveForIssue({
|
||||
companyId: existing.companyId,
|
||||
sourceIssueId: existing.id,
|
||||
actionId,
|
||||
@@ -5849,7 +5857,28 @@ export function issueRoutes(
|
||||
outcome: "delegated",
|
||||
resolutionNote: `Delegated to ${target} recovery issue ${recoveryIssue.identifier ?? recoveryIssue.id}.`,
|
||||
});
|
||||
if (!recoveryAction) throw notFound("Active recovery action not found");
|
||||
if (!recoveryAction) {
|
||||
const resolvedAction = await recoveryActionsSvc.getById(existing.companyId, actionId);
|
||||
if (
|
||||
resolvedAction?.sourceIssueId === existing.id &&
|
||||
resolvedAction.status === "resolved" &&
|
||||
resolvedAction.outcome === "delegated" &&
|
||||
resolvedAction.recoveryIssueId === recoveryIssue.id
|
||||
) {
|
||||
recoveryAction = resolvedAction;
|
||||
res.json({
|
||||
issue: {
|
||||
...updatedSource,
|
||||
activeRecoveryAction: null,
|
||||
},
|
||||
recoveryIssue,
|
||||
recoveryAction,
|
||||
reused: true,
|
||||
});
|
||||
return;
|
||||
}
|
||||
throw notFound("Active recovery action not found");
|
||||
}
|
||||
|
||||
await issueReferencesSvc.syncIssue(recoveryIssue.id);
|
||||
await externalObjectsSvc.syncIssueSafely(recoveryIssue.id);
|
||||
|
||||
Reference in new issue
Block a user