Preserve Forgejo mappings across retry failures
This commit is contained in:
parent
b0c38705ce
commit
fa24063483
5 changed files with 291 additions and 14 deletions
93
examples/first-dry-run-paperclip-issue.json
Normal file
93
examples/first-dry-run-paperclip-issue.json
Normal file
|
|
@ -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"
|
||||
}
|
||||
]
|
||||
}
|
||||
|
|
@ -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<typeof buildIssueSyncDraft>) => Promise<IssueReservationResult>;
|
||||
createRemoteIssue: (ctx: PluginContext, payload: ForgejoIssuePayload) => Promise<ForgejoIssueRecord>;
|
||||
complete: (
|
||||
recordRemote: (
|
||||
ctx: PluginContext,
|
||||
companyId: string,
|
||||
issueId: string,
|
||||
remoteIssue: ForgejoIssueRecord
|
||||
) => Promise<void>;
|
||||
complete: (ctx: PluginContext, companyId: string, issueId: string) => Promise<void>;
|
||||
fail: (ctx: PluginContext, companyId: string, issueId: string, errorMessage: string) => Promise<void>;
|
||||
failAfterRemote: (
|
||||
ctx: PluginContext,
|
||||
companyId: string,
|
||||
issueId: string,
|
||||
remoteIssue: ForgejoIssueRecord,
|
||||
errorMessage: string
|
||||
) => Promise<void>;
|
||||
queueManualReview: (ctx: PluginContext, draft: ReturnType<typeof buildIssueSyncDraft>) => Promise<void>;
|
||||
};
|
||||
|
||||
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;
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -12,6 +12,7 @@ function tableName(ctx: PluginContext, name: string): string {
|
|||
}
|
||||
|
||||
function toIssueMappingRecord(row: Record<string, unknown>): 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<string, unknown>): 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<void> {
|
||||
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<void> {
|
||||
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<void> {
|
||||
await ctx.db.execute(
|
||||
`INSERT INTO ${tableName(ctx, "review_queue")}
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
};
|
||||
|
||||
|
|
|
|||
|
|
@ -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("<!-- paperclip-sync:company-1:issue-1 -->");
|
||||
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();
|
||||
});
|
||||
});
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue