Fix accepted-plan continuation review feedback

This commit is contained in:
Devin Foley committed 2026-05-28 19:32:50 -07:00
1 parent 3052b6c552
commit 61065ae380
8 files changed
+445 -25

No files matched your search

@@ -514,6 +514,164 @@ describeEmbeddedPostgres("accepted plan workspace refresh", () => {
expect(adapterInput.context.paperclipTaskMarkdown).not.toContain("Create child issues from the approved plan only");
}, 20_000);
it("guards cross-issue accepted-plan retries even when the waking issue is standard work mode", async () => {
const companyId = randomUUID();
const projectId = randomUUID();
const projectWorkspaceId = randomUUID();
const issueId = randomUUID();
const otherPlanningIssueId = randomUUID();
const agentId = randomUUID();
const repoRoot = await createGitRepo();
tempRoots.push(repoRoot);
await instanceSettingsService(db).updateExperimental({
enableIsolatedWorkspaces: false,
});
await db.insert(companies).values({
id: companyId,
name: "Acme",
issuePrefix: `T${companyId.replace(/-/g, "").slice(0, 6).toUpperCase()}`,
status: "active",
createdAt: new Date(),
updatedAt: new Date(),
});
await db.insert(projects).values({
id: projectId,
companyId,
name: "Accepted Plan Routing",
status: "active",
createdAt: new Date(),
updatedAt: new Date(),
});
await db.insert(projectWorkspaces).values({
id: projectWorkspaceId,
companyId,
projectId,
name: "Primary",
cwd: repoRoot,
isPrimary: true,
createdAt: new Date(),
updatedAt: new Date(),
});
await db.insert(agents).values({
id: agentId,
companyId,
name: "CodexCoder",
role: "engineer",
status: "idle",
adapterType: "codex_local",
adapterConfig: {},
runtimeConfig: {},
permissions: {},
createdAt: new Date(),
updatedAt: new Date(),
});
await db.insert(issues).values([
{
id: issueId,
companyId,
projectId,
projectWorkspaceId,
title: "Implementation wake after accepted plan",
status: "in_progress",
workMode: "standard",
priority: "medium",
assigneeAgentId: agentId,
identifier: "PAP-9401",
createdAt: new Date(),
updatedAt: new Date(),
},
{
id: otherPlanningIssueId,
companyId,
projectId,
projectWorkspaceId,
title: "Earlier accepted plan",
status: "in_progress",
workMode: "planning",
priority: "medium",
assigneeAgentId: agentId,
identifier: "PAP-9402",
createdAt: new Date(),
updatedAt: new Date(),
},
]);
await seedAcceptedPlanClaim({
companyId,
issueId: otherPlanningIssueId,
ownerAgentId: agentId,
status: "in_flight",
});
await db.insert(agentTaskSessions).values({
companyId,
agentId,
adapterType: "codex_local",
taskKey: issueId,
sessionParamsJson: {
sessionId: "stale-standard-cross-issue-session",
cwd: repoRoot,
},
sessionDisplayId: "stale-standard-cross-issue-session",
});
adapterExecute.mockImplementationOnce(async () => {
await db.update(issues).set({ status: "done", updatedAt: new Date() }).where(eq(issues.id, issueId));
return {
exitCode: 0,
signal: null,
timedOut: false,
sessionParams: { sessionId: "fresh-session" },
sessionDisplayId: "fresh-session",
summary: "Suppressed cross-issue accepted-plan continuation for a standard-work wake.",
provider: "test",
model: "test-model",
};
});
const heartbeat = heartbeatService(db);
const run = await heartbeat.wakeup(agentId, {
source: "automation",
triggerDetail: "system",
reason: "issue_commented",
payload: {
issueId,
interactionId: "interaction-standard-cross-issue",
interactionKind: "request_confirmation",
interactionStatus: "accepted",
mutation: "interaction",
},
contextSnapshot: {
issueId,
taskId: issueId,
wakeReason: "issue_commented",
interactionKind: "request_confirmation",
interactionStatus: "accepted",
forceFreshSession: true,
workspaceRefreshReason: "accepted_plan_confirmation",
},
});
expect(run).not.toBeNull();
await vi.waitFor(async () => {
const latest = await heartbeat.getRun(run!.id);
expect(latest?.status).toBe("succeeded");
}, { timeout: 10_000 });
expect(adapterExecute).toHaveBeenCalledTimes(1);
const adapterInput = adapterExecute.mock.calls[0]?.[0] as {
runtime: { sessionId: string | null; sessionParams: Record<string, unknown> | null };
context: Record<string, unknown>;
};
expect(adapterInput.runtime.sessionId).toBeNull();
expect(adapterInput.runtime.sessionParams).toBeNull();
expect(adapterInput.context.acceptedPlanWakeRouting).toEqual(expect.objectContaining({
reason: "other_issue_claim_in_flight",
otherActiveClaimIssueId: otherPlanningIssueId,
otherActiveClaimIdentifier: "PAP-9402",
}));
expect(adapterInput.context.paperclipTaskMarkdown).toContain("Issue: \"PAP-9401\"");
expect(adapterInput.context.paperclipTaskMarkdown).not.toContain("Create child issues from the approved plan only");
}, 20_000);
it("preserves accepted-plan continuation resume state when the wake issue owns the in-flight claim", async () => {
const companyId = randomUUID();
const projectId = randomUUID();
@@ -55,6 +55,23 @@ describe("buildPaperclipTaskMarkdown", () => {
expect(acceptedConfirmation).not.toContain("Make the plan only.");
});
it("adds accepted-plan continuation guidance for standard-work issues when the wake is flagged as a plan continuation", () => {
const acceptedConfirmation = buildPaperclipTaskMarkdown({
issue: {
id: "issue-2",
identifier: "PAP-415",
title: "Implement the fix",
workMode: "standard",
description: null,
},
acceptedPlanContinuation: true,
});
expect(acceptedConfirmation).toContain("Accepted plan directive:");
expect(acceptedConfirmation).toContain("Create child issues from the approved plan only");
expect(acceptedConfirmation).not.toContain("- Work mode: \"planning\"");
});
it("prefers ordinary comment planning guidance over stale accepted confirmation state", () => {
const commentWake = buildPaperclipTaskMarkdown({
issue: {
@@ -537,6 +537,14 @@ describe.sequential("issue thread interaction routes", () => {
payload: {
version: 1,
prompt: "Approve this plan?",
target: {
type: "issue_document",
issueId: "aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa",
documentId: "document-plan",
key: "plan",
revisionId: "revision-plan",
revisionNumber: 1,
},
},
result: {
version: 1,
@@ -572,6 +580,65 @@ describe.sequential("issue thread interaction routes", () => {
);
});
it("forces a fresh workspace-aware session when accepting a plan document confirmation on a standard-work issue", async () => {
mockIssueService.getById.mockResolvedValueOnce(createIssue({ workMode: "standard" }));
mockInteractionService.acceptInteraction.mockResolvedValueOnce({
interaction: {
id: "interaction-standard-plan",
companyId: "company-1",
issueId: "aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa",
kind: "request_confirmation",
status: "accepted",
continuationPolicy: "wake_assignee_on_accept",
idempotencyKey: "confirmation:issue:plan:revision-standard",
sourceCommentId: null,
sourceRunId: "run-standard-plan",
payload: {
version: 1,
prompt: "Approve this plan?",
target: {
type: "issue_document",
issueId: "aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa",
documentId: "document-plan",
key: "plan",
revisionId: "revision-standard",
revisionNumber: 2,
},
},
result: {
version: 1,
outcome: "accepted",
},
createdAt: "2026-04-20T12:00:00.000Z",
updatedAt: "2026-04-20T12:05:00.000Z",
resolvedAt: "2026-04-20T12:05:00.000Z",
},
createdIssues: [],
});
const app = await createApp();
const res = await request(app)
.post("/api/issues/aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa/interactions/interaction-standard-plan/accept")
.send({});
expect(res.status).toBe(200);
expect(mockHeartbeatService.wakeup).toHaveBeenCalledTimes(1);
expect(mockHeartbeatService.wakeup).toHaveBeenCalledWith(
ASSIGNEE_AGENT_ID,
expect.objectContaining({
reason: "issue_commented",
contextSnapshot: expect.objectContaining({
issueId: "aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa",
interactionId: "interaction-standard-plan",
interactionKind: "request_confirmation",
interactionStatus: "accepted",
forceFreshSession: true,
workspaceRefreshReason: "accepted_plan_confirmation",
}),
}),
);
});
it("wakes the returned agent when accepting an agent-authored confirmation from a board review assignee", async () => {
mockIssueService.getById.mockResolvedValueOnce(createIssue({
status: "in_review",
+108 -1
View File
@@ -3318,6 +3318,7 @@ describeEmbeddedPostgres("accepted plan decomposition", () => {
assigneeAgentId?: string;
sourceIssueId?: string;
issueTitle?: string;
workMode?: "planning" | "standard";
}) {
const companyId = args?.companyId ?? randomUUID();
const goalId = args?.goalId ?? randomUUID();
@@ -3362,7 +3363,7 @@ describeEmbeddedPostgres("accepted plan decomposition", () => {
title: args?.issueTitle ?? "Planning issue",
status: "in_progress",
priority: "medium",
workMode: "planning",
workMode: args?.workMode ?? "planning",
assigneeAgentId: assigneeAgentId,
});
await db.insert(documents).values({
@@ -3528,6 +3529,30 @@ describeEmbeddedPostgres("accepted plan decomposition", () => {
});
});
it("allows accepted-plan decomposition on a standard-work issue with an accepted plan document", async () => {
const { sourceIssueId, acceptedPlanRevisionId, assigneeAgentId } = await seedAcceptedPlanIssue({
workMode: "standard",
issueTitle: "Implement after planning",
});
const result = await svc.decomposeAcceptedPlan(sourceIssueId, {
acceptedPlanRevisionId,
children: [
{
title: "Implement the approved first slice",
status: "todo",
workMode: "standard",
priority: "medium",
},
],
actorAgentId: assigneeAgentId,
});
expect(result.childIssueIds).toHaveLength(1);
expect(result.newlyCreatedIssues).toHaveLength(1);
expect(result.decomposition.status).toBe("completed");
});
it("serializes concurrent accepted-plan retries for the same parent issue without duplicate children", async () => {
const { sourceIssueId, acceptedPlanRevisionId, assigneeAgentId } = await seedAcceptedPlanIssue();
const children = [
@@ -3795,6 +3820,88 @@ describeEmbeddedPostgres("accepted plan decomposition", () => {
expect(childrenRows.find((row) => row.id !== firstChildId)?.createdByAgentId).toBe(reassignedAgentId);
});
it("preserves the existing live claim owner when another actor resumes the same fingerprint", async () => {
const { companyId, sourceIssueId, acceptedPlanRevisionId, assigneeAgentId } = await seedAcceptedPlanIssue();
const competingAgentId = randomUUID();
const liveOwnerRunId = randomUUID();
const competingRunId = randomUUID();
await db.insert(agents).values({
id: competingAgentId,
companyId,
name: "SecondCoder",
role: "engineer",
status: "active",
adapterType: "codex_local",
adapterConfig: {},
runtimeConfig: {},
permissions: {},
});
await db.insert(heartbeatRuns).values([
{
id: liveOwnerRunId,
companyId,
agentId: assigneeAgentId,
status: "running",
invocationSource: "manual",
},
{
id: competingRunId,
companyId,
agentId: competingAgentId,
status: "running",
invocationSource: "manual",
},
]);
const children = [
{
title: "Keep the first created child",
status: "todo" as const,
workMode: "standard" as const,
priority: "medium" as const,
},
{
title: "Create the missing second child",
status: "todo" as const,
workMode: "standard" as const,
priority: "medium" as const,
},
];
const initial = await svc.decomposeAcceptedPlan(sourceIssueId, {
acceptedPlanRevisionId,
children,
actorAgentId: assigneeAgentId,
actorRunId: liveOwnerRunId,
});
const [firstChildId, secondChildId] = initial.childIssueIds;
const claim = await getAcceptedPlanClaim(sourceIssueId);
await db.delete(issues).where(eq(issues.id, secondChildId!));
await db
.update(issuePlanDecompositions)
.set({
status: "in_flight",
childIssueIds: [firstChildId!],
completedAt: null,
ownerAgentId: assigneeAgentId,
ownerRunId: liveOwnerRunId,
updatedAt: new Date(),
})
.where(eq(issuePlanDecompositions.id, claim!.id));
const retried = await svc.decomposeAcceptedPlan(sourceIssueId, {
acceptedPlanRevisionId,
children,
actorAgentId: competingAgentId,
actorRunId: competingRunId,
});
expect(retried.decomposition.status).toBe("completed");
expect(retried.decomposition.ownerAgentId).toBe(assigneeAgentId);
expect(retried.decomposition.ownerRunId).toBe(liveOwnerRunId);
});
it("lists persisted decompositions with child issue summaries", async () => {
const { sourceIssueId, acceptedPlanRevisionId, assigneeAgentId } = await seedAcceptedPlanIssue();
+4 -1
View File
@@ -100,6 +100,7 @@ import { assertEnvironmentSelectionForCompany } from "./environment-selection.js
import { executionWorkspaceService as executionWorkspaceServiceDirect } from "../services/execution-workspaces.js";
import { feedbackService } from "../services/feedback.js";
import { instanceSettingsService } from "../services/instance-settings.js";
import { readAcceptedPlanConfirmationTarget } from "../services/issues.js";
import { environmentService } from "../services/environments.js";
import { redactSensitiveText } from "../redaction.js";
import {
@@ -5264,10 +5265,12 @@ export function issueRoutes(
});
}
const acceptedPlanTarget = readAcceptedPlanConfirmationTarget(interaction.payload);
const acceptedPlanConfirmation =
interaction.kind === "request_confirmation" &&
interaction.status === "accepted" &&
issue.workMode === "planning";
acceptedPlanTarget?.issueId === issue.id &&
acceptedPlanTarget.key === "plan";
queueResolvedInteractionContinuationWakeup({
heartbeat,
issue: continuationWakeIssue,
+25 -5
View File
@@ -1947,11 +1947,11 @@ async function resolveAcceptedPlanWakeRoutingDecision(args: {
companyId: string;
agentId: string;
issueId: string | null;
issueWorkMode: string | null | undefined;
acceptedPlanContinuationWake: boolean;
contextSnapshot: Record<string, unknown>;
}): Promise<AcceptedPlanWakeRoutingDecision | null> {
if (args.issueId === null) return null;
if (args.issueWorkMode !== "planning") return null;
if (!args.acceptedPlanContinuationWake) return null;
const activeClaims = await args.db
.select({
@@ -2283,6 +2283,7 @@ export function buildPaperclipTaskMarkdown(input: {
kind?: string | null;
status?: string | null;
} | null;
acceptedPlanContinuation?: boolean;
}) {
const quoteTaskScalar = (value: string) => JSON.stringify(value);
const fenceTaskText = (value: string) => {
@@ -2297,8 +2298,11 @@ export function buildPaperclipTaskMarkdown(input: {
const wakeComment = input.wakeComment ?? null;
const acceptedPlanContinuation =
!wakeComment &&
input.interaction?.kind === "request_confirmation" &&
input.interaction.status === "accepted";
(input.acceptedPlanContinuation || (
input.interaction?.kind === "request_confirmation" &&
input.interaction.status === "accepted" &&
issue?.workMode === "planning"
));
if (!issue && !wakeComment) return null;
const lines = [
@@ -2324,6 +2328,12 @@ export function buildPaperclipTaskMarkdown(input: {
"Planning mode directive:",
directive,
);
} else if (acceptedPlanContinuation) {
lines.push(
"",
"Accepted plan directive:",
"Create child issues from the approved plan only. Do not write code or perform implementation work on the source issue.",
);
}
const description = issue.description?.trim();
if (description) {
@@ -7115,7 +7125,13 @@ export function heartbeatService(db: Db, options: HeartbeatServiceOptions = {})
companyId: agent.companyId,
agentId: agent.id,
issueId,
issueWorkMode: issueContext.workMode,
acceptedPlanContinuationWake:
readNonEmptyString(context.workspaceRefreshReason) === "accepted_plan_confirmation"
|| (
issueContext.workMode === "planning"
&& readNonEmptyString(context.interactionKind) === "request_confirmation"
&& readNonEmptyString(context.interactionStatus) === "accepted"
),
contextSnapshot: context,
})
: null;
@@ -7129,6 +7145,7 @@ export function heartbeatService(db: Db, options: HeartbeatServiceOptions = {})
};
if (acceptedPlanWakeRoutingDecision.suppressAcceptedContinuation) {
clearInteractionContinuationWakeContext(context);
delete context.workspaceRefreshReason;
}
} else {
delete context.acceptedPlanWakeRouting;
@@ -7232,6 +7249,9 @@ export function heartbeatService(db: Db, options: HeartbeatServiceOptions = {})
kind: readNonEmptyString(context.interactionKind),
status: readNonEmptyString(context.interactionStatus),
},
acceptedPlanContinuation:
readNonEmptyString(context.workspaceRefreshReason) === "accepted_plan_confirmation"
&& !parseObject(context.acceptedPlanWakeRouting),
});
if (issueRef) {
context.paperclipIssue = {
+59 -11
View File
@@ -301,6 +301,8 @@ function serializeAcceptedPlanDecomposition(
acceptedInteractionId: decomposition.acceptedInteractionId,
status: decomposition.status as AcceptedPlanDecomposition["status"],
requestFingerprint: decomposition.requestFingerprint,
// Intentionally omit requestedChildren here; the API only needs stable counts
// and child ids, while the durable table keeps the full child draft payload.
requestedChildCount: decomposition.requestedChildCount,
childIssueIds: normalizeIssuePlanDecompositionChildIds(decomposition.childIssueIds),
ownerAgentId: decomposition.ownerAgentId,
@@ -482,7 +484,7 @@ function normalizeIssuePlanDecompositionChildIds(value: unknown): string[] {
return value.filter((item): item is string => typeof item === "string" && item.length > 0);
}
function readAcceptedPlanConfirmationTarget(payload: unknown): {
export function readAcceptedPlanConfirmationTarget(payload: unknown): {
revisionId: string;
key: string;
issueId: string;
@@ -499,6 +501,46 @@ function readAcceptedPlanConfirmationTarget(payload: unknown): {
return { revisionId, key, issueId };
}
async function resolveAcceptedPlanClaimOwner(input: {
dbOrTx: Pick<Db, "select">;
claim: Pick<typeof issuePlanDecompositions.$inferSelect, "ownerAgentId" | "ownerUserId" | "ownerRunId">;
actorAgentId?: string | null;
actorUserId?: string | null;
actorRunId?: string | null;
}) {
const nextOwner = {
ownerAgentId: input.actorAgentId ?? null,
ownerUserId: input.actorUserId ?? null,
ownerRunId: input.actorRunId ?? null,
};
if (
input.claim.ownerAgentId === nextOwner.ownerAgentId
&& input.claim.ownerUserId === nextOwner.ownerUserId
&& input.claim.ownerRunId === nextOwner.ownerRunId
) {
return nextOwner;
}
if (!input.claim.ownerRunId) {
return nextOwner;
}
const existingOwnerRun = await input.dbOrTx
.select({ status: heartbeatRuns.status })
.from(heartbeatRuns)
.where(eq(heartbeatRuns.id, input.claim.ownerRunId))
.then((rows) => rows[0] ?? null);
if (existingOwnerRun && !TERMINAL_HEARTBEAT_RUN_STATUSES.has(existingOwnerRun.status)) {
return {
ownerAgentId: input.claim.ownerAgentId,
ownerUserId: input.claim.ownerUserId,
ownerRunId: input.claim.ownerRunId,
};
}
return nextOwner;
}
async function findAcceptedPlanDocumentInteraction(
dbOrTx: Pick<Db, "select">,
input: {
@@ -4226,15 +4268,11 @@ export function issueService(db: Db) {
companyId: issues.companyId,
projectId: issues.projectId,
goalId: issues.goalId,
workMode: issues.workMode,
})
.from(issues)
.where(eq(issues.id, sourceIssueId))
.then((rows) => rows[0] ?? null);
if (!sourceIssue) throw notFound("Source issue not found");
if (sourceIssue.workMode !== "planning") {
throw unprocessable("Accepted-plan decomposition is only available for planning issues");
}
const requestFingerprint = createAcceptedPlanDecompositionRequestFingerprint({
acceptedPlanRevisionId: data.acceptedPlanRevisionId,
@@ -4342,15 +4380,20 @@ export function issueService(db: Db) {
}
const completedAt = claim.completedAt ?? new Date();
const ownerPatch = await resolveAcceptedPlanClaimOwner({
dbOrTx: tx,
claim,
actorAgentId: data.actorAgentId,
actorUserId: data.actorUserId,
actorRunId: data.actorRunId,
});
const [completed] = await tx
.update(issuePlanDecompositions)
.set({
status: "completed",
childIssueIds: nextIds,
completedAt,
ownerAgentId: data.actorAgentId ?? null,
ownerUserId: data.actorUserId ?? null,
ownerRunId: data.actorRunId ?? null,
...ownerPatch,
updatedAt: completedAt,
})
.where(eq(issuePlanDecompositions.id, claim.id))
@@ -4371,15 +4414,20 @@ export function issueService(db: Db) {
const nextIds = [...existingChildIssueIds, createdChild.issue.id];
const now = new Date();
const nextStatus = nextIds.length === data.children.length ? "completed" : "in_flight";
const ownerPatch = await resolveAcceptedPlanClaimOwner({
dbOrTx: tx,
claim,
actorAgentId: data.actorAgentId,
actorUserId: data.actorUserId,
actorRunId: data.actorRunId,
});
const [updatedClaim] = await tx
.update(issuePlanDecompositions)
.set({
status: nextStatus,
childIssueIds: nextIds,
completedAt: nextStatus === "completed" ? now : null,
ownerAgentId: data.actorAgentId ?? null,
ownerUserId: data.actorUserId ?? null,
ownerRunId: data.actorRunId ?? null,
...ownerPatch,
updatedAt: now,
})
.where(eq(issuePlanDecompositions.id, claim.id))