diff --git a/server/src/__tests__/agent-conversations.test.ts b/server/src/__tests__/agent-conversations.test.ts index edc861e41b..b7c23adfab 100644 --- a/server/src/__tests__/agent-conversations.test.ts +++ b/server/src/__tests__/agent-conversations.test.ts @@ -910,6 +910,9 @@ describe("chat prompt policy", () => { expect(prompt).toContain("Rejected plan review directive:"); expect(prompt).toContain("Add CHAT_REVIEW_MARKER and a validation step."); expect(prompt).toContain("not approval to implement or hand off execution tasks"); + expect(prompt).toContain("first GET /api/issues/{issueId}/documents/plan"); + expect(prompt).toContain("baseRevisionId set to that latestRevisionId"); + expect(prompt).toContain("Bind the new approval request to the revision returned by the successful update"); expect(prompt).not.toContain("Accepted chat plan directive:"); expect(prompt).not.toContain("stale-approved-plan"); }); diff --git a/server/src/__tests__/documents-service.test.ts b/server/src/__tests__/documents-service.test.ts index 44a6d3b184..92dd1f3217 100644 --- a/server/src/__tests__/documents-service.test.ts +++ b/server/src/__tests__/documents-service.test.ts @@ -113,6 +113,41 @@ describeEmbeddedPostgres("documentService system issue documents", () => { })); }); + it("explains the revision guard and rejects missing or stale update revisions without changing the document", async () => { + const { issueId } = await createIssueWithDocuments(); + const current = (await svc.getIssueDocumentByKey(issueId, "plan"))!; + const update = { + issueId, + key: "plan", + title: "Plan", + format: "markdown" as const, + body: "# Revised plan", + }; + + await expect(svc.upsertIssueDocument(update)).rejects.toMatchObject({ + status: 409, + message: expect.stringContaining("set baseRevisionId to that latestRevisionId"), + details: { currentRevisionId: current.latestRevisionId }, + }); + expect(await svc.getIssueDocumentByKey(issueId, "plan")).toMatchObject({ + body: current.body, + latestRevisionId: current.latestRevisionId, + }); + + const saved = await svc.upsertIssueDocument({ ...update, baseRevisionId: current.latestRevisionId }); + expect(saved.document.body).toBe(update.body); + expect(saved.document.latestRevisionNumber).toBe(current.latestRevisionNumber + 1); + await expect(svc.upsertIssueDocument({ + ...update, + body: "# Stale replacement", + baseRevisionId: current.latestRevisionId, + })).rejects.toMatchObject({ status: 409, message: "Document was updated by someone else" }); + expect(await svc.getIssueDocumentByKey(issueId, "plan")).toMatchObject({ + body: saved.document.body, + latestRevisionId: saved.document.latestRevisionId, + }); + }); + it("locks and unlocks issue documents", async () => { const { issueId } = await createIssueWithDocuments(); diff --git a/server/src/services/documents.ts b/server/src/services/documents.ts index b30f79b22b..9517456b53 100644 --- a/server/src/services/documents.ts +++ b/server/src/services/documents.ts @@ -351,7 +351,7 @@ export function documentService(db: Db) { } if (!input.baseRevisionId) { - throw conflict("Document update requires baseRevisionId", { + throw conflict("Document update requires baseRevisionId. GET the current document, read its body and latestRevisionId, then set baseRevisionId to that latestRevisionId when updating.", { currentRevisionId: existing.latestRevisionId, }); } diff --git a/server/src/services/heartbeat.ts b/server/src/services/heartbeat.ts index a38cbcf2a6..f721a09eb6 100644 --- a/server/src/services/heartbeat.ts +++ b/server/src/services/heartbeat.ts @@ -7791,6 +7791,7 @@ export function buildPaperclipTaskMarkdown(input: { "", "Rejected plan review directive:", "The user rejected the plan and requested changes. Revise the plan to address their feedback through the existing plan document and review workflow. In Ask mode, discuss the requested changes without mutating documents or tasks. This is not approval to implement or hand off execution tasks. Do not treat the issue's in_progress status as plan approval.", + "When revising the plan, first GET /api/issues/{issueId}/documents/plan and read its body and latestRevisionId. PUT the revised document to the same endpoint with baseRevisionId set to that latestRevisionId. An existing document requires this concurrency guard; do not omit it or blindly retry a stale revision. Bind the new approval request to the revision returned by the successful update.", ); if (input.planReview?.reason?.trim()) { lines.push("User's requested changes:", fenceTaskText(input.planReview.reason.trim())); diff --git a/skills/paperclip/SKILL.md b/skills/paperclip/SKILL.md index a4f917b65a..525e709c6c 100644 --- a/skills/paperclip/SKILL.md +++ b/skills/paperclip/SKILL.md @@ -599,7 +599,7 @@ PUT /api/issues/{issueId}/documents/plan } ``` -If `plan` already exists, fetch the current document first and send its latest `baseRevisionId` when you update it. +If `plan` already exists, first `GET /api/issues/{issueId}/documents/plan` and read its current body and `latestRevisionId`. Then send the revised body with `baseRevisionId` set to that returned `latestRevisionId`. The GET field is `latestRevisionId`; the PUT field is `baseRevisionId`. Omitting it on an update returns `409`. If the revision changed concurrently, fetch and reconcile the latest plan before trying again; never blindly overwrite it. ## Key Endpoints (Hot Routes)