From fa240634838f8829653dd61944f4a206b3ca871a Mon Sep 17 00:00:00 2001 From: Paperclip Bot Date: Tue, 2 Jun 2026 08:01:26 +0000 Subject: [PATCH] Preserve Forgejo mappings across retry failures --- examples/first-dry-run-paperclip-issue.json | 93 +++++++++++++++++ src/paperclip-issue-sync.ts | 53 ++++++++-- src/persistence.ts | 51 +++++++++- src/types.ts | 2 +- tests/paperclip-issue-sync.spec.ts | 106 +++++++++++++++++++- 5 files changed, 291 insertions(+), 14 deletions(-) create mode 100644 examples/first-dry-run-paperclip-issue.json diff --git a/examples/first-dry-run-paperclip-issue.json b/examples/first-dry-run-paperclip-issue.json new file mode 100644 index 0000000..88c10a3 --- /dev/null +++ b/examples/first-dry-run-paperclip-issue.json @@ -0,0 +1,93 @@ +{ + "id": "issue-dry-run-1", + "companyId": "company-dry-run", + "projectId": "project-forgejo-sync", + "projectWorkspaceId": null, + "goalId": null, + "parentId": null, + "title": "Synthetic dry-run: reproduce outbound issue creation", + "description": "Summary:\nA controlled dry-run payload for the first Forgejo sync rehearsal.\n\nReproduction:\n1. Open the plugin test harness.\n2. Trigger issue sync for this payload.\n3. Confirm the outbound issue body contains the summary and attachment note.\n\nObserved:\nA Forgejo issue should be created exactly once for this Paperclip issue identifier.\n\nExpected:\nA retry should reuse the stored remote mapping instead of creating a second Forgejo issue.", + "status": "in_progress", + "workMode": "standard", + "priority": "medium", + "assigneeAgentId": null, + "assigneeUserId": null, + "checkoutRunId": null, + "executionRunId": null, + "executionAgentNameKey": null, + "executionLockedAt": null, + "createdByAgentId": null, + "createdByUserId": null, + "issueNumber": 9001, + "identifier": "PRIA-DRY-1", + "requestDepth": 0, + "billingCode": null, + "assigneeAdapterOverrides": null, + "executionWorkspaceId": null, + "executionWorkspacePreference": null, + "executionWorkspaceSettings": null, + "startedAt": null, + "completedAt": null, + "cancelledAt": null, + "hiddenAt": null, + "labels": [ + { + "id": "label-forgejo-sync", + "companyId": "company-dry-run", + "name": "forgejo-sync", + "color": "#000000", + "createdAt": "2026-06-02T00:00:00.000Z", + "updatedAt": "2026-06-02T00:00:00.000Z" + } + ], + "blockedBy": [], + "blocks": [], + "project": { + "id": "project-forgejo-sync", + "companyId": "company-dry-run", + "name": "Forgejo Issue Sync Plugin", + "description": null, + "status": "planned", + "primaryGoalId": null, + "createdAt": "2026-06-02T00:00:00.000Z", + "updatedAt": "2026-06-02T00:00:00.000Z" + }, + "goal": null, + "currentExecutionWorkspace": null, + "mentionedProjects": [], + "myLastTouchAt": null, + "lastExternalCommentAt": null, + "lastActivityAt": null, + "isUnreadForMe": false, + "createdAt": "2026-06-02T00:00:00.000Z", + "updatedAt": "2026-06-02T00:00:00.000Z", + "workProducts": [ + { + "id": "wp-dry-run-1", + "companyId": "company-dry-run", + "projectId": "project-forgejo-sync", + "issueId": "issue-dry-run-1", + "executionWorkspaceId": null, + "runtimeServiceId": null, + "type": "artifact", + "provider": "paperclip", + "externalId": "attachment-dry-run-1", + "title": "trace.log", + "url": "https://paperclip.example/artifacts/trace.log", + "status": "ready_for_review", + "reviewState": "none", + "isPrimary": false, + "healthStatus": "healthy", + "summary": null, + "metadata": { + "attachmentId": "attachment-dry-run-1", + "contentType": "text/plain", + "byteSize": 2048, + "originalFilename": "trace.log" + }, + "createdByRunId": null, + "createdAt": "2026-06-02T00:00:00.000Z", + "updatedAt": "2026-06-02T00:00:00.000Z" + } + ] +} diff --git a/src/paperclip-issue-sync.ts b/src/paperclip-issue-sync.ts index cfe844a..4dd2869 100644 --- a/src/paperclip-issue-sync.ts +++ b/src/paperclip-issue-sync.ts @@ -7,6 +7,8 @@ import { completeIssueMapping, enqueueManualReview, failIssueMapping, + recordRemoteIssueFailure, + recordRemoteIssueMapping, reserveIssueMapping } from "./persistence.js"; import type { @@ -31,21 +33,31 @@ type ArtifactWorkProduct = { type IssueSyncDependencies = { reserve: (ctx: PluginContext, draft: ReturnType) => Promise; createRemoteIssue: (ctx: PluginContext, payload: ForgejoIssuePayload) => Promise; - complete: ( + recordRemote: ( ctx: PluginContext, companyId: string, issueId: string, remoteIssue: ForgejoIssueRecord ) => Promise; + complete: (ctx: PluginContext, companyId: string, issueId: string) => Promise; fail: (ctx: PluginContext, companyId: string, issueId: string, errorMessage: string) => Promise; + failAfterRemote: ( + ctx: PluginContext, + companyId: string, + issueId: string, + remoteIssue: ForgejoIssueRecord, + errorMessage: string + ) => Promise; queueManualReview: (ctx: PluginContext, draft: ReturnType) => Promise; }; const defaultDependencies: IssueSyncDependencies = { reserve: reserveIssueMapping, createRemoteIssue: createForgejoIssue, + recordRemote: recordRemoteIssueMapping, complete: completeIssueMapping, fail: failIssueMapping, + failAfterRemote: recordRemoteIssueFailure, queueManualReview: enqueueManualReview }; @@ -157,17 +169,42 @@ export async function syncIssueToForgejo( const reservation = await deps.reserve(ctx, draft); if (reservation.kind === "existing") { - return reservation.mapping.syncStatus === "synced" ? "existing" : "skipped"; + if (reservation.mapping.syncStatus === "synced" || reservation.mapping.syncStatus === "remote_created") { + if ( + reservation.mapping.forgejoIssueId === null + || reservation.mapping.forgejoIssueNumber === null + || reservation.mapping.forgejoIssueUrl === null + || reservation.mapping.forgejoApiUrl === null + ) { + throw new Error("Existing remote-created mapping is missing Forgejo issue details."); + } + } else { + return "skipped"; + } } + let remoteIssue: ForgejoIssueRecord | null = + reservation.kind === "existing" + ? { + id: reservation.mapping.forgejoIssueId!, + number: reservation.mapping.forgejoIssueNumber!, + url: reservation.mapping.forgejoIssueUrl!, + apiUrl: reservation.mapping.forgejoApiUrl! + } + : null; + try { - const remoteIssue = await deps.createRemoteIssue(ctx, payload); - await deps.complete(ctx, issue.companyId, issue.id, remoteIssue); + if (!remoteIssue) { + remoteIssue = await deps.createRemoteIssue(ctx, payload); + await deps.recordRemote(ctx, issue.companyId, issue.id, remoteIssue); + } if (draft.manualReviewRequired) { await deps.queueManualReview(ctx, draft); } + await deps.complete(ctx, issue.companyId, issue.id); + await ctx.activity.log({ companyId: issue.companyId, entityType: "issue", @@ -181,10 +218,14 @@ export async function syncIssueToForgejo( } }); - return "created"; + return reservation.kind === "existing" ? "existing" : "created"; } catch (error) { const message = error instanceof Error ? error.message : String(error); - await deps.fail(ctx, issue.companyId, issue.id, message); + if (remoteIssue) { + await deps.failAfterRemote(ctx, issue.companyId, issue.id, remoteIssue, message); + } else { + await deps.fail(ctx, issue.companyId, issue.id, message); + } throw error; } } diff --git a/src/persistence.ts b/src/persistence.ts index e0d6344..44f3c40 100644 --- a/src/persistence.ts +++ b/src/persistence.ts @@ -12,6 +12,7 @@ function tableName(ctx: PluginContext, name: string): string { } function toIssueMappingRecord(row: Record): IssueMappingRecord { + const syncStatus = row.sync_status; return { companyId: String(row.company_id), paperclipIssueId: String(row.paperclip_issue_id), @@ -27,7 +28,12 @@ function toIssueMappingRecord(row: Record): IssueMappingRecord forgejoIssueNumber: typeof row.forgejo_issue_number === "number" ? row.forgejo_issue_number : null, forgejoIssueUrl: typeof row.forgejo_issue_url === "string" ? row.forgejo_issue_url : null, forgejoApiUrl: typeof row.forgejo_api_url === "string" ? row.forgejo_api_url : null, - syncStatus: row.sync_status === "pending" || row.sync_status === "failed" ? row.sync_status : "synced", + syncStatus: + syncStatus === "pending" + || syncStatus === "failed" + || syncStatus === "remote_created" + ? syncStatus + : "synced", lastError: typeof row.last_error === "string" ? row.last_error : null }; } @@ -86,7 +92,8 @@ export async function reserveIssueMapping(ctx: PluginContext, draft: IssueSyncDr updated_at = now() WHERE company_id = $1 AND paperclip_issue_id = $2 - AND sync_status = 'failed'`, + AND sync_status = 'failed' + AND forgejo_issue_id IS NULL`, baseParams ); if (retry.rowCount > 0) { @@ -100,7 +107,7 @@ export async function reserveIssueMapping(ctx: PluginContext, draft: IssueSyncDr return { kind: "existing", mapping: existing }; } -export async function completeIssueMapping( +export async function recordRemoteIssueMapping( ctx: PluginContext, companyId: string, paperclipIssueId: string, @@ -112,7 +119,7 @@ export async function completeIssueMapping( forgejo_issue_number = $4, forgejo_issue_url = $5, forgejo_api_url = $6, - sync_status = 'synced', + sync_status = 'remote_created', last_error = NULL, updated_at = now() WHERE company_id = $1 AND paperclip_issue_id = $2`, @@ -120,6 +127,21 @@ export async function completeIssueMapping( ); } +export async function completeIssueMapping( + ctx: PluginContext, + companyId: string, + paperclipIssueId: string +): Promise { + await ctx.db.execute( + `UPDATE ${tableName(ctx, "issue_mappings")} + SET sync_status = 'synced', + last_error = NULL, + updated_at = now() + WHERE company_id = $1 AND paperclip_issue_id = $2`, + [companyId, paperclipIssueId] + ); +} + export async function failIssueMapping( ctx: PluginContext, companyId: string, @@ -136,6 +158,27 @@ export async function failIssueMapping( ); } +export async function recordRemoteIssueFailure( + ctx: PluginContext, + companyId: string, + paperclipIssueId: string, + remoteIssue: ForgejoIssueRecord, + errorMessage: string +): Promise { + await ctx.db.execute( + `UPDATE ${tableName(ctx, "issue_mappings")} + SET forgejo_issue_id = $3, + forgejo_issue_number = $4, + forgejo_issue_url = $5, + forgejo_api_url = $6, + sync_status = 'remote_created', + last_error = $7, + updated_at = now() + WHERE company_id = $1 AND paperclip_issue_id = $2`, + [companyId, paperclipIssueId, remoteIssue.id, remoteIssue.number, remoteIssue.url, remoteIssue.apiUrl, errorMessage] + ); +} + export async function enqueueManualReview(ctx: PluginContext, candidate: IssueSyncDraft): Promise { await ctx.db.execute( `INSERT INTO ${tableName(ctx, "review_queue")} diff --git a/src/types.ts b/src/types.ts index 9bd7aec..d3a9f14 100644 --- a/src/types.ts +++ b/src/types.ts @@ -82,7 +82,7 @@ export type IssueMappingRecord = IssueSyncDraft & { forgejoIssueNumber: number | null; forgejoIssueUrl: string | null; forgejoApiUrl: string | null; - syncStatus: "pending" | "synced" | "failed"; + syncStatus: "pending" | "remote_created" | "synced" | "failed"; lastError: string | null; }; diff --git a/tests/paperclip-issue-sync.spec.ts b/tests/paperclip-issue-sync.spec.ts index 89f55ed..0d42df3 100644 --- a/tests/paperclip-issue-sync.spec.ts +++ b/tests/paperclip-issue-sync.spec.ts @@ -121,6 +121,7 @@ describe("paperclip issue sync", () => { expect(payload.body).toContain(ATTACHMENT_NOTE); expect(payload.body).toContain("trace.log | text/plain | 2.0 KiB"); expect(payload.body).toContain(""); + expect(payload.body).not.toContain("https://paperclip.example/artifacts/trace.log"); }); it("creates a Forgejo issue once and queues manual review when attachment context is required", async () => { @@ -163,8 +164,10 @@ describe("paperclip issue sync", () => { url: "https://forgejo.example/acme/repo/issues/33", apiUrl: "https://forgejo.example/api/v1/repos/acme/repo/issues/33" })); + const recordRemote = vi.fn(async () => undefined); const complete = vi.fn(async () => undefined); const fail = vi.fn(async () => undefined); + const failAfterRemote = vi.fn(async () => undefined); const queueManualReview = vi.fn(async () => undefined); const result = await syncIssueToForgejo( @@ -173,22 +176,25 @@ describe("paperclip issue sync", () => { activity: { log: activityLog } } as never, issue, - { reserve, createRemoteIssue, complete, fail, queueManualReview } + { reserve, createRemoteIssue, recordRemote, complete, fail, failAfterRemote, queueManualReview } ); expect(result).toBe("created"); expect(createRemoteIssue).toHaveBeenCalledOnce(); - expect(complete).toHaveBeenCalledWith(expect.anything(), "company-1", "issue-1", expect.objectContaining({ number: 33 })); + expect(recordRemote).toHaveBeenCalledWith(expect.anything(), "company-1", "issue-1", expect.objectContaining({ number: 33 })); + expect(complete).toHaveBeenCalledWith(expect.anything(), "company-1", "issue-1"); expect(queueManualReview).toHaveBeenCalledOnce(); expect(activityLog).toHaveBeenCalledWith(expect.objectContaining({ message: "Created Forgejo issue #33 for PRIA-17." })); expect(fail).not.toHaveBeenCalled(); + expect(failAfterRemote).not.toHaveBeenCalled(); }); it("skips remote creation when a synced mapping already exists", async () => { const issue = buildIssue(); const createRemoteIssue = vi.fn(); + const complete = vi.fn(async () => undefined); const result = await syncIssueToForgejo( { @@ -218,11 +224,105 @@ describe("paperclip issue sync", () => { lastError: null } }), - createRemoteIssue + createRemoteIssue, + complete, + failAfterRemote: vi.fn(async () => undefined) } ); expect(result).toBe("existing"); expect(createRemoteIssue).not.toHaveBeenCalled(); + expect(complete).toHaveBeenCalledOnce(); + }); + + it("does not create a duplicate Forgejo issue after remote success and local mapping failure", async () => { + const issue = buildIssue(); + const createdRemoteIssue = { + id: 101, + number: 33, + url: "https://forgejo.example/acme/repo/issues/33", + apiUrl: "https://forgejo.example/api/v1/repos/acme/repo/issues/33" + }; + const activityLog = vi.fn(async () => undefined); + const createRemoteIssue = vi.fn(async () => createdRemoteIssue); + const recordRemote = vi.fn(async () => { + throw new Error("failed to complete local mapping"); + }); + const failAfterRemote = vi.fn(async () => undefined); + + await expect(syncIssueToForgejo( + { + config: { get: async () => ({ forgejoOwner: "acme", forgejoRepo: "repo" }) }, + activity: { log: activityLog } + } as never, + issue, + { + reserve: async () => ({ kind: "reserved" as const }), + createRemoteIssue, + recordRemote, + complete: vi.fn(async () => undefined), + fail: vi.fn(async () => undefined), + failAfterRemote + } + )).rejects.toThrow("failed to complete local mapping"); + + expect(createRemoteIssue).toHaveBeenCalledTimes(1); + expect(failAfterRemote).toHaveBeenCalledWith( + expect.anything(), + "company-1", + "issue-1", + expect.objectContaining({ number: 33 }), + "failed to complete local mapping" + ); + + const retryCreateRemoteIssue = vi.fn(async () => { + throw new Error("should not create a duplicate remote issue"); + }); + const retryRecordRemote = vi.fn(async () => undefined); + const retryQueueManualReview = vi.fn(async () => undefined); + const retryComplete = vi.fn(async () => undefined); + + const result = await syncIssueToForgejo( + { + config: { get: async () => ({ forgejoOwner: "acme", forgejoRepo: "repo" }) }, + activity: { log: activityLog } + } as never, + issue, + { + reserve: async () => ({ + kind: "existing" as const, + mapping: { + companyId: "company-1", + paperclipIssueId: "issue-1", + repoOwner: "acme", + repoName: "repo", + dedupeKey: "paperclip-issue:company-1:issue-1", + sourceTitle: "[PRIA-17] Fix Forgejo sync", + sourceBody: "body", + attachmentMetadata: [], + manualReviewRequired: false, + reviewReasonCode: null, + forgejoIssueId: 101, + forgejoIssueNumber: 33, + forgejoIssueUrl: "https://forgejo.example/acme/repo/issues/33", + forgejoApiUrl: "https://forgejo.example/api/v1/repos/acme/repo/issues/33", + syncStatus: "remote_created", + lastError: "failed to complete local mapping" + } + }), + createRemoteIssue: retryCreateRemoteIssue, + recordRemote: retryRecordRemote, + complete: retryComplete, + fail: vi.fn(async () => undefined), + failAfterRemote: vi.fn(async () => undefined), + queueManualReview: retryQueueManualReview + } + ); + + expect(result).toBe("existing"); + expect(retryCreateRemoteIssue).not.toHaveBeenCalled(); + expect(retryRecordRemote).not.toHaveBeenCalled(); + expect(retryComplete).toHaveBeenCalledOnce(); + expect(retryQueueManualReview).not.toHaveBeenCalled(); }); });