diff --git a/docs/api/issues.md b/docs/api/issues.md index a0bff74004..617b17f557 100644 --- a/docs/api/issues.md +++ b/docs/api/issues.md @@ -62,7 +62,7 @@ Headers: X-Paperclip-Run-Id: {runId} } ``` -The optional `comment` field adds a comment in the same call. +The optional `comment` field adds a comment in the same call. For execution-policy review or approval decisions, the decision comment must be included in this same `PATCH`; a prior `POST /api/issues/{issueId}/comments` does not satisfy the stage decision guard. Updatable fields: `title`, `description`, `status`, `priority`, `assigneeAgentId`, `projectId`, `goalId`, `parentId`, `billingCode`. diff --git a/docs/guides/agent-developer/comments-and-communication.md b/docs/guides/agent-developer/comments-and-communication.md index 23a77d455a..fe44cb78d8 100644 --- a/docs/guides/agent-developer/comments-and-communication.md +++ b/docs/guides/agent-developer/comments-and-communication.md @@ -19,6 +19,8 @@ PATCH /api/issues/{issueId} { "status": "done", "comment": "Implemented login endpoint with JWT auth." } ``` +When you are the active reviewer or approver for an execution-policy stage, include the decision rationale in this same `PATCH` request. A separate `POST /api/issues/{issueId}/comments` followed by a status-only `PATCH` does not advance the review/approval stage. + ## Comment Style Use concise markdown with: diff --git a/docs/guides/execution-policy.md b/docs/guides/execution-policy.md index f12af1cb13..030798b711 100644 --- a/docs/guides/execution-policy.md +++ b/docs/guides/execution-policy.md @@ -173,9 +173,26 @@ This prevents silent completions where an agent finishes work but leaves no trac - Only the **active reviewer/approver** (the `currentParticipant` in execution state) can advance or reject the current stage. - Non-participants who attempt to transition the issue receive a `422 Unprocessable Entity` error. - Both approvals and change requests **require a comment** — empty or whitespace-only comments are rejected. +- The decision comment must be included in the same `PATCH /api/issues/{issueId}` request that changes the status. A prior `POST /api/issues/{issueId}/comments` entry remains normal discussion and does not satisfy the review/approval decision guard. ## API Usage +### Recording review or approval decisions + +```bash +PATCH /api/issues/{issueId} +{ "status": "done", "comment": "Reviewer decision: approve - implementation and tests look good." } +``` + +To request changes, send the target non-`done` status and the explanation together: + +```bash +PATCH /api/issues/{issueId} +{ "status": "in_progress", "comment": "Reviewer decision: request changes - cover the retry edge case." } +``` + +Do not split a decision into `POST /comments` followed by a status-only `PATCH`; the runtime only records the decision from the `comment` field on the same status update. + ### Setting an execution policy on issue creation ```bash diff --git a/server/src/__tests__/issue-execution-policy.test.ts b/server/src/__tests__/issue-execution-policy.test.ts index b3f09d295b..d0e348b036 100644 --- a/server/src/__tests__/issue-execution-policy.test.ts +++ b/server/src/__tests__/issue-execution-policy.test.ts @@ -630,7 +630,7 @@ describe("issue execution policy transitions", () => { actor: { agentId: qaAgentId }, commentBody: "", }), - ).toThrow("requires a comment"); + ).toThrow(/Approving a review or approval stage requires a comment.*same PATCH request.*prior comments are not considered/); }); it("changes requested without comment throws", () => { @@ -659,7 +659,7 @@ describe("issue execution policy transitions", () => { actor: { agentId: qaAgentId }, commentBody: null, }), - ).toThrow("requires a comment"); + ).toThrow(/Requesting changes requires a comment.*same PATCH request.*prior comments are not considered/); }); it("whitespace-only comment is treated as empty", () => { diff --git a/server/src/services/issue-execution-policy.ts b/server/src/services/issue-execution-policy.ts index 9fb34e748b..ec04838fdd 100644 --- a/server/src/services/issue-execution-policy.ts +++ b/server/src/services/issue-execution-policy.ts @@ -63,6 +63,7 @@ const PENDING_STATUS: IssueExecutionState["status"] = "pending"; const CHANGES_REQUESTED_STATUS: IssueExecutionState["status"] = "changes_requested"; const MONITOR_INVALID_MESSAGE = "Monitor can only be scheduled on issues assigned to an agent in in_progress or in_review"; const MONITOR_BOUNDS_EXHAUSTED_MESSAGE = "Monitor bounds are already exhausted"; +const STAGE_DECISION_COMMENT_HINT = "Include the decision comment in the same PATCH request; prior comments are not considered."; export const REDACTED_ISSUE_MONITOR_EXTERNAL_REF = "[redacted]"; function normalizeMonitorNotes(notes: string | null | undefined) { @@ -708,7 +709,7 @@ function applyIssueExecutionStageTransition(input: TransitionInput): TransitionR if (principalsEqual(currentParticipant, actor)) { if (requestedStatus === "done") { if (!input.commentBody?.trim()) { - throw unprocessable("Approving a review or approval stage requires a comment"); + throw unprocessable(`Approving a review or approval stage requires a comment. ${STAGE_DECISION_COMMENT_HINT}`); } const approvedState = buildCompletedState(existingState, activeStage); // Only stages after the stage being approved are advance candidates. @@ -762,7 +763,7 @@ function applyIssueExecutionStageTransition(input: TransitionInput): TransitionR if (requestedStatus && requestedStatus !== "in_review") { if (!input.commentBody?.trim()) { - throw unprocessable("Requesting changes requires a comment"); + throw unprocessable(`Requesting changes requires a comment. ${STAGE_DECISION_COMMENT_HINT}`); } if (!existingState?.returnAssignee) { throw unprocessable("This execution stage has no return assignee");