Arnike/paperclip
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

[codex] Revert PR #7678 (#7765)

Browse Source 
## Thinking Path

> - Paperclip is the open source app people use to manage AI agents for
work.
> - Issue comment wake handoffs are part of the control-plane execution
loop that decides when agents resume work after comments and issue
updates.
> - PR #7678 changed that wake handoff behavior in server issue routes,
heartbeat context, and related tests.
> - The change broke an important workflow after merge, so the safest
immediate fix is to restore the pre-#7678 wake behavior.
> - This pull request reverts the wake-handoff behavior from PR #7678
while keeping narrow review-requested safeguards that prevent known
runtime/test regressions.
> - The benefit is that Paperclip returns to the last known working wake
behavior without reintroducing avoidable UUID skill lookup and
annotation-resolution test gaps.

## Linked Issues or Issue Description

Refs: #7678

Bug context:
- What happened: PR #7678 was reported to have broken an important
Paperclip workflow after it merged.
- Expected behavior: Paperclip should preserve the prior issue comment
wake handoff behavior until a corrected change is ready.
- Steps to reproduce: Use the workflow affected by PR #7678's issue
comment wake handoff changes.
- Paperclip version/commit: `master` after merge commit
`4da79a88c67e54084d40bd18cada5ee5c8be23da`.
- Deployment mode: Paperclip control-plane server behavior.

## What Changed

- Reverted merge commit `4da79a88c67e54084d40bd18cada5ee5c8be23da` from
PR #7678 to restore pre-#7678 wake-handoff behavior.
- Preserved the safe accepted-plan routing check so `parseObject(...)`
is not used as a boolean.
- Preserved UUID filtering for run-scoped skill mentions so legacy
non-UUID skill IDs do not reach a Postgres UUID lookup.
- Restored the annotation thread-resolution test guard that verifies
resolving a thread does not wake the assignee.

## Verification

- `pnpm run preflight:workspace-links && NODE_ENV=test
PAPERCLIP_HOME=/tmp/... PAPERCLIP_INSTANCE_ID=pap10614-revert
TMPDIR=/tmp/... pnpm exec vitest run --project @paperclipai/server
--no-file-parallelism --maxWorkers=1
server/src/__tests__/document-annotation-routes.test.ts
server/src/__tests__/heartbeat-project-env.test.ts
server/src/__tests__/heartbeat-accepted-plan-workspace-refresh.test.ts
server/src/__tests__/heartbeat-context-summary.test.ts`
- Result: 4 test files passed, 26 tests passed.
- Earlier targeted revert verification also passed: 4 test files, 50
tests.

## Risks

- This intentionally restores behavior from before PR #7678, so intended
wake-handoff improvements from that PR are removed.
- The PR is no longer a byte-for-byte revert because Greptile identified
two narrow safeguards worth preserving.
- Low migration risk: no schema or dependency changes are included.
- Follow-up work may still be needed to reintroduce the desired wake
handoff behavior without the regression.

## Model Used

OpenAI Codex, GPT-5 coding agent in this Paperclip heartbeat, with
shell/tool execution and repository write access.

## 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 run tests locally and they pass
- [x] I have added or updated tests where applicable
- [x] If this change affects the UI, I have included before/after
screenshots
- [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
This commit is contained in:
Dotta
2026-06-08 10:35:58 -05:00
committed by GitHub
parent 76c88e5855
commit 7fb40264f8
8 changed files with 224 additions and 248 deletions
Download Patch File Download Diff File Expand all files Collapse all files
server/src/__tests__/document-annotation-routes.test.ts
+10 -2
View File
@@ -237,7 +237,7 @@ describe("document annotation routes", () => {
});
});
it("creates annotation threads, syncs references, logs activity, and does not wake the assignee", async () => {
it("creates annotation threads, syncs references, logs activity, and wakes the assignee", async () => {
mockIssueService.getById.mockResolvedValue({
id: issueId,
companyId,
@@ -261,7 +261,15 @@ describe("document annotation routes", () => {
expect(mockLogActivity).toHaveBeenCalledWith(expect.anything(), expect.objectContaining({
action: "issue.document_annotation_thread_created",
}));
expect(mockHeartbeatService.wakeup).not.toHaveBeenCalled();
expect(mockHeartbeatService.wakeup).toHaveBeenCalledWith(
"99999999-9999-4999-8999-999999999999",
expect.objectContaining({
payload: expect.objectContaining({
annotationThreadId: annotationThread.id,
annotationCommentId: annotationComment.id,
}),
}),
);
});
it("rejects agent cross-company annotation reads", async () => {
server/src/__tests__/heartbeat-accepted-plan-workspace-refresh.test.ts
-20
View File
@@ -737,17 +737,6 @@ describeEmbeddedPostgres("accepted plan workspace refresh", () => {
createdAt: new Date(),
updatedAt: new Date(),
});
const planCommentId = randomUUID();
await db.insert(issueComments).values({
id: planCommentId,
companyId,
issueId,
authorUserId: "board-user-1",
authorType: "user",
body: "Keep the accepted plan rollout split into separate child tasks.",
createdAt: new Date("2026-06-07T00:00:00.000Z"),
updatedAt: new Date("2026-06-07T00:00:00.000Z"),
});
await seedAcceptedPlanClaim({
companyId,
issueId,
@@ -814,14 +803,5 @@ describeEmbeddedPostgres("accepted plan workspace refresh", () => {
expect(adapterInput.runtime.sessionId).toBe("accepted-plan-retry-session");
expect(adapterInput.context.acceptedPlanWakeRouting).toBeUndefined();
expect(adapterInput.context.paperclipTaskMarkdown).toContain("Create child issues from the approved plan only");
expect(adapterInput.context.paperclipTaskMarkdown).toContain("Comments included with the confirmed plan:");
expect(adapterInput.context.paperclipTaskMarkdown).toContain(planCommentId);
expect(adapterInput.context.paperclipTaskMarkdown).toContain(
"Keep the accepted plan rollout split into separate child tasks.",
);
expect(adapterInput.context.paperclipWake).toEqual(expect.objectContaining({
commentIds: [planCommentId],
commentContextSource: "accepted_plan_confirmation",
}));
}, 20_000);
});
server/src/__tests__/heartbeat-context-summary.test.ts
-28
View File
@@ -72,34 +72,6 @@ describe("buildPaperclipTaskMarkdown", () => {
expect(acceptedConfirmation).not.toContain("- Work mode: \"planning\"");
});
it("includes confirmed-plan comments in accepted-plan continuation task context", () => {
const acceptedConfirmation = buildPaperclipTaskMarkdown({
issue: {
id: "issue-1",
identifier: "PAP-3404",
title: "Plan first",
workMode: "planning",
description: null,
},
interaction: {
kind: "request_confirmation",
status: "accepted",
},
acceptedPlanComments: [
{
id: "comment-1",
authorType: "user",
body: "Please keep the migration backwards compatible.",
},
],
});
expect(acceptedConfirmation).toContain("Create child issues from the approved plan only");
expect(acceptedConfirmation).toContain("Comments included with the confirmed plan:");
expect(acceptedConfirmation).toContain("Comment 1 - user - comment-1:");
expect(acceptedConfirmation).toContain("Please keep the migration backwards compatible.");
});
it("prefers ordinary comment planning guidance over stale accepted confirmation state", () => {
const commentWake = buildPaperclipTaskMarkdown({
issue: {
server/src/__tests__/issue-comment-redaction.test.ts
-63
View File
@@ -175,69 +175,6 @@ describeEmbeddedPostgres("deleted issue comment redaction", () => {
expect(JSON.stringify(wakePayload)).not.toContain("secret metadata");
});
it("adds recent issue comments to accepted-plan continuation wake payloads", async () => {
const { companyId, issueId } = await seedIssue();
const firstCommentId = randomUUID();
const secondCommentId = randomUUID();
const deletedCommentId = randomUUID();
await db.insert(issueComments).values([
{
id: firstCommentId,
companyId,
issueId,
authorUserId: "board-user-1",
body: "Keep the migration backwards compatible.",
createdAt: new Date("2026-06-03T12:00:00.000Z"),
},
{
id: deletedCommentId,
companyId,
issueId,
authorUserId: "board-user-1",
body: "Do not pass this deleted text downstream.",
deletedAt: new Date("2026-06-03T12:01:00.000Z"),
deletedByType: "user",
deletedByUserId: "board-user-1",
createdAt: new Date("2026-06-03T12:01:00.000Z"),
},
{
id: secondCommentId,
companyId,
issueId,
authorUserId: "board-user-1",
body: "Split the rollout into a verification child task.",
createdAt: new Date("2026-06-03T12:02:00.000Z"),
},
]);
const wakePayload = await buildPaperclipWakePayload({
db,
companyId,
contextSnapshot: {
issueId,
interactionKind: "request_confirmation",
interactionStatus: "accepted",
workspaceRefreshReason: "accepted_plan_confirmation",
},
issueSummary: {
id: issueId,
identifier: "RED-1",
title: "Deleted comment redaction",
status: "in_progress",
priority: "medium",
workMode: "planning",
},
});
expect(wakePayload?.commentContextSource).toBe("accepted_plan_confirmation");
expect(wakePayload?.commentIds).toEqual([firstCommentId, secondCommentId]);
expect(wakePayload?.comments.map((comment) => comment.body)).toEqual([
"Keep the migration backwards compatible.",
"Split the rollout into a verification child task.",
]);
expect(JSON.stringify(wakePayload)).not.toContain("Do not pass this deleted text downstream.");
});
it("excludes deleted comment bodies from company search", async () => {
const { companyId, issueId } = await seedIssue();
await db.insert(issueComments).values({
server/src/__tests__/issue-comment-reopen-routes.test.ts
+55 -10
View File
@@ -639,8 +639,20 @@ describe.sequential("issue comment reopen routes", () => {
}),
}),
);
await waitForWakeup(() => expect(mockIssueService.findMentionedAgents).toHaveBeenCalled());
expect(mockHeartbeatService.wakeup).not.toHaveBeenCalled();
await waitForWakeup(() => expect(mockHeartbeatService.wakeup).toHaveBeenCalledWith(
"22222222-2222-4222-8222-222222222222",
expect.objectContaining({
reason: "issue_commented",
payload: expect.objectContaining({
commentId: "comment-1",
mutation: "comment",
}),
contextSnapshot: expect.objectContaining({
wakeReason: "issue_commented",
source: "issue.comment",
}),
}),
));
});
it("does not move scheduled-retry issues to todo when POST comment retry cancellation fails", async () => {
@@ -689,8 +701,12 @@ describe.sequential("issue comment reopen routes", () => {
expect(mockIssueService.getCurrentScheduledRetry).toHaveBeenCalledWith("11111111-1111-4111-8111-111111111111");
expect(mockIssueService.update).not.toHaveBeenCalled();
expect(mockHeartbeatService.cancelRun).not.toHaveBeenCalled();
await waitForWakeup(() => expect(mockIssueService.findMentionedAgents).toHaveBeenCalled());
expect(mockHeartbeatService.wakeup).not.toHaveBeenCalled();
await waitForWakeup(() => expect(mockHeartbeatService.wakeup).toHaveBeenCalledWith(
"22222222-2222-4222-8222-222222222222",
expect.objectContaining({
reason: "issue_commented",
}),
));
});
it("passes validated comment presentation fields to trusted board comment writes", async () => {
@@ -792,8 +808,21 @@ describe.sequential("issue comment reopen routes", () => {
expect(res.status).toBe(201);
expect(mockIssueService.update).not.toHaveBeenCalled();
await waitForWakeup(() => expect(mockIssueService.findMentionedAgents).toHaveBeenCalled());
expect(mockHeartbeatService.wakeup).not.toHaveBeenCalled();
await waitForWakeup(() => expect(mockHeartbeatService.wakeup).toHaveBeenCalledWith(
"22222222-2222-4222-8222-222222222222",
expect.objectContaining({
reason: "issue_commented",
payload: expect.objectContaining({
commentId: "comment-1",
mutation: "comment",
}),
contextSnapshot: expect.objectContaining({
issueId: "11111111-1111-4111-8111-111111111111",
wakeCommentId: "comment-1",
wakeReason: "issue_commented",
}),
}),
));
});
it("does not implicitly reopen closed issues via POST comments when no agent is assigned", async () => {
@@ -888,8 +917,16 @@ describe.sequential("issue comment reopen routes", () => {
}),
);
expect(mockHeartbeatService.cancelRun).toHaveBeenCalledWith("retry-run-1");
await waitForWakeup(() => expect(mockIssueService.findMentionedAgents).toHaveBeenCalled());
expect(mockHeartbeatService.wakeup).not.toHaveBeenCalled();
await waitForWakeup(() => expect(mockHeartbeatService.wakeup).toHaveBeenCalledWith(
"22222222-2222-4222-8222-222222222222",
expect.objectContaining({
reason: "issue_commented",
payload: expect.objectContaining({
commentId: "comment-1",
mutation: "comment",
}),
}),
));
});
it("does not move scheduled-retry issues to todo when PATCH comment retry cancellation fails", async () => {
@@ -991,8 +1028,16 @@ describe.sequential("issue comment reopen routes", () => {
"11111111-1111-4111-8111-111111111111",
expect.objectContaining({ status: "todo" }),
);
await waitForWakeup(() => expect(mockIssueService.findMentionedAgents).toHaveBeenCalled());
expect(mockHeartbeatService.wakeup).not.toHaveBeenCalled();
await waitForWakeup(() => expect(mockHeartbeatService.wakeup).toHaveBeenCalledWith(
"22222222-2222-4222-8222-222222222222",
expect.objectContaining({
reason: "issue_commented",
payload: expect.objectContaining({
commentId: "comment-1",
mutation: "comment",
}),
}),
));
});
it("wakes the assignee when an assigned blocked issue moves back to todo", async () => {
server/src/__tests__/issue-update-comment-wakeup-routes.test.ts
+22 -6
View File
@@ -269,7 +269,7 @@ describe("issue update comment wakeups", () => {
);
});
it("does not wake the assignee on comment-only issue updates", async () => {
it("wakes the assignee on comment-only issue updates", async () => {
const existing = makeIssue({
assigneeAgentId: ASSIGNEE_AGENT_ID,
assigneeUserId: null,
@@ -292,10 +292,26 @@ describe("issue update comment wakeups", () => {
});
expect(res.status).toBe(200);
await vi.waitFor(() => expect(mockIssueService.findMentionedAgents).toHaveBeenCalledWith(
existing.companyId,
"please revise this",
));
expect(mockHeartbeatService.wakeup).not.toHaveBeenCalled();
expect(mockHeartbeatService.wakeup).toHaveBeenCalledTimes(1);
expect(mockHeartbeatService.wakeup).toHaveBeenCalledWith(
ASSIGNEE_AGENT_ID,
expect.objectContaining({
source: "automation",
reason: "issue_commented",
payload: expect.objectContaining({
issueId: existing.id,
commentId: "comment-2",
mutation: "comment",
}),
contextSnapshot: expect.objectContaining({
issueId: existing.id,
taskId: existing.id,
commentId: "comment-2",
wakeCommentId: "comment-2",
wakeReason: "issue_commented",
source: "issue.comment",
}),
}),
);
});
});