Guide plan revisions through the document concurrency guard
This commit is contained in:
parent
2f438a0ba0
commit
74f1914d55
|
|
@ -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");
|
||||
});
|
||||
|
|
|
|||
|
|
@ -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();
|
||||
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
});
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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()));
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue