From 8f9f850c2016151c02478dfcffd46d59a6b88493 Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Tue, 1 Sep 2026 17:18:42 -0500 Subject: [PATCH] fix: limit plan-to-auto transition to plan confirmation (#12695) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - The issue thread controls plan review and agent work modes. > - A user can accept a full plan or confirm a smaller checkbox action. > - Only full plan acceptance must start automatic agent work. > - The current transition did not check the interaction kind. > - This pull request limits the transition to an accepted plan confirmation. > - The benefit is a safe and clear start of agent work after plan approval. ## Linked Issues or Issue Description **What happened?** An accepted confirmation that targeted a plan could change an issue from planning mode to standard mode. This included a checkbox confirmation. A checkbox action is not approval of the full plan. **Expected behavior** Only acceptance of a current full-plan confirmation starts automatic agent work. Other interaction kinds and rejected confirmations keep the current work mode. **Steps to reproduce** 1. Put an issue in planning mode. 2. Create a checkbox confirmation that targets the current plan revision. 3. Accept the checkbox confirmation. 4. Observe that the issue enters standard mode before this fix. **Paperclip version or commit** The problem was present on `master` before this change. **Deployment mode** The problem is in the core server logic and is not deployment-specific. ## What Changed - Require a full `request_confirmation` interaction before plan acceptance starts automatic work. - Add service tests for acceptance, rejection, stale interaction kinds, and unchanged standard-mode behavior. - Check the route activity log for the planning-to-standard mode change. - Document the plan acceptance transition in the V1 contract. ## Verification - `pnpm exec vitest run server/src/__tests__/issue-thread-interactions-service.test.ts server/src/__tests__/issue-thread-interaction-routes.test.ts` passes 140 tests. - `pnpm -r typecheck` passes. - `pnpm build` passes. - `pnpm test:run` was also started. Unrelated workspace-runtime tests failed because fixed local runtime ports were occupied or offset on the shared host. The same failures reproduce alone. The changed test files pass alone. ## Risks - Risk is low. The change adds one interaction-kind guard to the existing transition. - A full accepted plan confirmation still changes planning mode to standard mode and an eligible review issue to todo in one transaction. - Checkbox confirmations, questions, rejection, and standard-mode issues keep their previous behavior. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used OpenAI Codex with GPT-5, reasoning, tool use, and code execution. The runtime does not expose the exact model suffix or context window. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge Co-authored-by: Paperclip --- doc/SPEC-implementation.md | 3 +- .../issue-thread-interaction-routes.test.ts | 11 ++ .../issue-thread-interactions-service.test.ts | 173 ++++++++++++++++++ .../src/services/issue-thread-interactions.ts | 3 +- 4 files changed, 188 insertions(+), 2 deletions(-) diff --git a/doc/SPEC-implementation.md b/doc/SPEC-implementation.md index 3e5c857105..3387102b93 100644 --- a/doc/SPEC-implementation.md +++ b/doc/SPEC-implementation.md @@ -240,7 +240,7 @@ Routine execution issues add a routine-scoped env overlay after project env and - `work_mode` text not null default `standard`; supported values: - `standard`: normal autonomous execution. Agents may investigate, edit files, create artifacts, and complete the task. - `ask`: answer-only execution. Agents may use tools for investigation or temporary scratch work, but the deliverable is an issue-thread answer; they must not write implementation code or produce an implementation plan. - - `planning`: plan-only execution. Agents create or revise the plan without implementation work; accepted-plan continuations remain planning-specific and create child issues from the approved plan. + - `planning`: plan-only execution. Agents create or revise the plan without implementation work. Accepting a fresh confirmation for the issue's current `plan` revision atomically changes this mode to `standard`, so the continuation may implement the approved plan on the source issue. - `billing_code` text null - `assignee_adapter_overrides` jsonb null - `execution_policy` jsonb null @@ -258,6 +258,7 @@ Invariants: - `in_progress` requires assignee - an `in_review -> done | cancelled` verdict is authorized against the current review policy while the issue row is locked; a policy change in the same request or a concurrent request cannot relax that verdict gate - accepting or rejecting the review-confirmation interaction locks the issue row before resolving the interaction and reauthorizes against the current review policy in that transaction +- accepting a fresh `request_confirmation` for the current issue's `plan` revision changes `work_mode = planning` to `work_mode = standard` in the same transaction as the accepted interaction; the existing agent-return transition also moves an eligible `in_review` issue to `todo` without changing its agent owner - while a restrictive review policy is stored, changing it requires an actor who is allowed by that row-locked policy - the transition into `in_review` and its requester activity record commit atomically, including transitions without an explicit review-interaction binding - terminal states: `done | cancelled` diff --git a/server/src/__tests__/issue-thread-interaction-routes.test.ts b/server/src/__tests__/issue-thread-interaction-routes.test.ts index a93bf5bf4b..3f8834475e 100644 --- a/server/src/__tests__/issue-thread-interaction-routes.test.ts +++ b/server/src/__tests__/issue-thread-interaction-routes.test.ts @@ -1744,6 +1744,17 @@ describe.sequential("issue thread interaction routes", () => { }), }), ); + expect(mockLogActivity).toHaveBeenCalledWith( + expect.anything(), + expect.objectContaining({ + action: "issue.updated", + details: expect.objectContaining({ + source: "request_confirmation_accept", + workMode: "standard", + _previous: expect.objectContaining({ workMode: "planning" }), + }), + }), + ); }); it("forces a fresh workspace-aware session when accepting a plan document confirmation on a standard-work issue", async () => { diff --git a/server/src/__tests__/issue-thread-interactions-service.test.ts b/server/src/__tests__/issue-thread-interactions-service.test.ts index 309bd8cb33..c6d2921fe4 100644 --- a/server/src/__tests__/issue-thread-interactions-service.test.ts +++ b/server/src/__tests__/issue-thread-interactions-service.test.ts @@ -102,6 +102,43 @@ describeEmbeddedPostgres("issueThreadInteractionService", () => { return { companyId, goalId, issueId }; } + async function attachPlanDocument(companyId: string, issueId: string) { + const documentId = randomUUID(); + const revisionId = randomUUID(); + await db.insert(documents).values({ + id: documentId, + companyId, + title: "Plan", + format: "markdown", + latestBody: "# Plan", + latestRevisionId: revisionId, + latestRevisionNumber: 1, + }); + await db.insert(issueDocuments).values({ + companyId, + issueId, + documentId, + key: "plan", + }); + await db.insert(documentRevisions).values({ + id: revisionId, + companyId, + documentId, + revisionNumber: 1, + title: "Plan", + format: "markdown", + body: "# Plan", + }); + return { + type: "issue_document" as const, + issueId, + documentId, + key: "plan", + revisionId, + revisionNumber: 1, + }; + } + async function recordReviewTransition(args: { companyId: string; issueId: string; @@ -2656,6 +2693,142 @@ describeEmbeddedPostgres("issueThreadInteractionService", () => { }); }); + it("atomically returns an accepted Plan-mode issue to its agent in Auto mode", async () => { + const { companyId, goalId, issueId } = await seedConfirmationIssue("Accept a plan into Auto mode"); + const agentId = randomUUID(); + await db.insert(agents).values({ + id: agentId, + companyId, + name: "Plan owner", + role: "engineer", + status: "active", + adapterType: "codex_local", + adapterConfig: {}, + runtimeConfig: {}, + permissions: {}, + }); + await db.update(issues).set({ + status: "in_review", + workMode: "planning", + assigneeAgentId: agentId, + }).where(eq(issues.id, issueId)); + const target = await attachPlanDocument(companyId, issueId); + const created = await interactionsSvc.create({ id: issueId, companyId }, { + kind: "request_confirmation", + continuationPolicy: "wake_assignee_on_accept", + payload: { version: 1, prompt: "Accept this plan?", target }, + }, { agentId }); + + const accepted = await interactionsSvc.acceptInteraction({ + id: issueId, + companyId, + goalId, + projectId: null, + }, created.id, {}, { userId: "local-board" }); + + expect(accepted.interaction).toMatchObject({ + id: created.id, + status: "accepted", + result: { outcome: "accepted" }, + }); + expect(accepted.continuationIssue).toEqual({ + id: issueId, + assigneeAgentId: agentId, + assigneeUserId: null, + status: "todo", + workMode: "standard", + }); + await expect(db.select().from(issues).where(eq(issues.id, issueId)).then((rows) => rows[0])).resolves.toMatchObject({ + status: "todo", + workMode: "standard", + assigneeAgentId: agentId, + assigneeUserId: null, + }); + }); + + it("keeps Plan mode for non-plan and checkbox confirmations", async () => { + const { companyId, goalId, issueId } = await seedConfirmationIssue("Do not auto-transition other confirmations"); + const agentId = randomUUID(); + await db.insert(agents).values({ + id: agentId, + companyId, + name: "Plan owner", + role: "engineer", + status: "active", + adapterType: "codex_local", + adapterConfig: {}, + runtimeConfig: {}, + permissions: {}, + }); + await db.update(issues).set({ + status: "in_review", + workMode: "planning", + assigneeAgentId: agentId, + }).where(eq(issues.id, issueId)); + + const nonPlan = await interactionsSvc.create({ id: issueId, companyId }, { + kind: "request_confirmation", + payload: { version: 1, prompt: "Accept this unrelated decision?" }, + }, { agentId }); + await interactionsSvc.acceptInteraction({ id: issueId, companyId, goalId, projectId: null }, nonPlan.id, {}, { + userId: "local-board", + }); + await expect(db.select().from(issues).where(eq(issues.id, issueId)).then((rows) => rows[0]?.workMode)) + .resolves.toBe("planning"); + + await db.update(issues).set({ status: "in_review" }).where(eq(issues.id, issueId)); + const target = await attachPlanDocument(companyId, issueId); + const checkbox = await interactionsSvc.create({ id: issueId, companyId }, { + kind: "request_checkbox_confirmation", + payload: { + version: 1, + prompt: "Select approved plan sections", + options: [{ id: "phase-1", label: "Phase 1" }], + target, + }, + }, { agentId }); + await interactionsSvc.acceptInteraction({ id: issueId, companyId, goalId, projectId: null }, checkbox.id, { + selectedOptionIds: ["phase-1"], + }, { userId: "local-board" }); + await expect(db.select().from(issues).where(eq(issues.id, issueId)).then((rows) => rows[0]?.workMode)) + .resolves.toBe("planning"); + }); + + it.each(["ask", "standard"] as const)("keeps %s mode when accepting a plan confirmation", async (workMode) => { + const { companyId, goalId, issueId } = await seedConfirmationIssue(`Keep ${workMode} mode`); + await db.update(issues).set({ workMode }).where(eq(issues.id, issueId)); + const target = await attachPlanDocument(companyId, issueId); + const created = await interactionsSvc.create({ id: issueId, companyId }, { + kind: "request_confirmation", + payload: { version: 1, prompt: "Accept this plan?", target }, + }, { userId: "local-board" }); + + await interactionsSvc.acceptInteraction({ id: issueId, companyId, goalId, projectId: null }, created.id, {}, { + userId: "local-board", + }); + + await expect(db.select().from(issues).where(eq(issues.id, issueId)).then((rows) => rows[0]?.workMode)) + .resolves.toBe(workMode); + }); + + it("keeps Plan mode when a plan confirmation is rejected", async () => { + const { companyId, issueId } = await seedConfirmationIssue("Reject a plan"); + await db.update(issues).set({ workMode: "planning" }).where(eq(issues.id, issueId)); + const target = await attachPlanDocument(companyId, issueId); + const created = await interactionsSvc.create({ id: issueId, companyId }, { + kind: "request_confirmation", + payload: { version: 1, prompt: "Accept this plan?", target }, + }, { userId: "local-board" }); + + const rejected = await interactionsSvc.rejectInteraction({ id: issueId, companyId }, created.id, { + reason: "Revise the plan", + }, { userId: "local-board" }); + + expect(rejected.status).toBe("rejected"); + await expect(db.select().from(issues).where(eq(issues.id, issueId)).then((rows) => rows[0]?.workMode)) + .resolves.toBe("planning"); + }); + it("expires request confirmations by default when a user comments after creation", async () => { const { companyId, issueId } = await seedConfirmationIssue(); const commentId = randomUUID(); diff --git a/server/src/services/issue-thread-interactions.ts b/server/src/services/issue-thread-interactions.ts index 107daa54c1..7b9ea530fa 100644 --- a/server/src/services/issue-thread-interactions.ts +++ b/server/src/services/issue-thread-interactions.ts @@ -1786,7 +1786,8 @@ export function issueThreadInteractionService(db: Db, opts: IssueThreadInteracti issueContext.id, ); const acceptedPlanStartsExecution = - acceptedPlanTarget?.issueId === issueContext.id + lockedCurrent.kind === "request_confirmation" + && acceptedPlanTarget?.issueId === issueContext.id && acceptedPlanTarget.key === "plan" && issueContext.workMode === "planning"; if (isNativeCompletionReview(lockedCurrent)) {