fix: limit plan-to-auto transition to plan confirmation (#12695)
<!-- This pull request uses ASD-STE100 Simplified Technical English. --> ## 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 <noreply@paperclip.ing>
This commit is contained in:
parent
4b6de5327e
commit
8f9f850c20
|
|
@ -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`
|
||||
|
|
|
|||
|
|
@ -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 () => {
|
||||
|
|
|
|||
|
|
@ -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();
|
||||
|
|
|
|||
|
|
@ -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)) {
|
||||
|
|
|
|||
Loading…
Reference in New Issue