gb_flea/paperclip
1
0
Fork 0
forked from farhoodlabs/paperclip
Code Pull Requests Activity

Merge pull request #3222 from paperclipai/pap-1266-issue-workflow

Browse Source 
feat(issue-ui): refine issue workflow surfaces and live updates
This commit is contained in:
Dotta
2026-04-09 14:52:16 -05:00
committed by GitHub
parent 6d63a4df45 4077ccd343
commit 0e87fdbe35
50 changed files with 2860 additions and 1206 deletions
Download Patch File Download Diff File Expand all files Collapse all files
server/src/__tests__/activity-routes.test.ts
+2 -1
View File
@@ -62,6 +62,7 @@ describe("activity routes", () => {
mockActivityService.runsForIssue.mockResolvedValue([
{
runId: "run-1",
adapterType: "codex_local",
},
]);
@@ -72,6 +73,6 @@ describe("activity routes", () => {
expect(mockIssueService.getByIdentifier).toHaveBeenCalledWith("PAP-475");
expect(mockIssueService.getById).not.toHaveBeenCalled();
expect(mockActivityService.runsForIssue).toHaveBeenCalledWith("company-1", "issue-uuid-1");
expect(res.body).toEqual([{ runId: "run-1" }]);
expect(res.body).toEqual([{ runId: "run-1", adapterType: "codex_local" }]);
});
});
server/src/__tests__/issue-execution-policy-routes.test.ts
+8 -69
View File
@@ -60,19 +60,17 @@ vi.mock("../services/index.js", () => ({
workProductService: () => ({}),
}));
function createApp(
actor: Record<string, unknown> = {
type: "board",
userId: "local-board",
companyIds: ["company-1"],
source: "local_implicit",
isInstanceAdmin: false,
},
) {
function createApp() {
const app = express();
app.use(express.json());
app.use((req, _res, next) => {
(req as any).actor = actor;
(req as any).actor = {
type: "board",
userId: "local-board",
companyIds: ["company-1"],
source: "local_implicit",
isInstanceAdmin: false,
};
next();
});
app.use("/api", issueRoutes({} as any, {} as any));
@@ -139,63 +137,4 @@ describe("issue execution policy routes", () => {
expect(updatePatch.executionState).toBeUndefined();
expect(mockHeartbeatService.wakeup).not.toHaveBeenCalled();
});
it("rejects agent stage advances from non-participants", async () => {
const reviewerAgentId = "33333333-3333-4333-8333-333333333333";
const approverAgentId = "44444444-4444-4444-8444-444444444444";
const executorAgentId = "22222222-2222-4222-8222-222222222222";
const policy = normalizeIssueExecutionPolicy({
stages: [
{
id: "11111111-1111-4111-8111-111111111111",
type: "review",
participants: [{ type: "agent", agentId: reviewerAgentId }],
},
{
id: "55555555-5555-4555-8555-555555555555",
type: "approval",
participants: [{ type: "agent", agentId: approverAgentId }],
},
],
})!;
const issue = {
id: "aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa",
companyId: "company-1",
status: "in_review",
assigneeAgentId: reviewerAgentId,
assigneeUserId: null,
createdByUserId: "local-board",
identifier: "PAP-1000",
title: "Execution policy guard",
executionPolicy: policy,
executionState: {
status: "pending",
currentStageId: "11111111-1111-4111-8111-111111111111",
currentStageIndex: 0,
currentStageType: "review",
currentParticipant: { type: "agent", agentId: reviewerAgentId },
returnAssignee: { type: "agent", agentId: executorAgentId },
completedStageIds: [],
lastDecisionId: null,
lastDecisionOutcome: null,
},
};
mockIssueService.getById.mockResolvedValue(issue);
const res = await request(
createApp({
type: "agent",
agentId: approverAgentId,
companyId: "company-1",
source: "api_key",
runId: "run-1",
}),
)
.patch("/api/issues/aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa")
.send({ status: "done", comment: "Skipping review." });
expect(res.status).toBe(403);
expect(res.body.error).toContain("active review participant");
expect(mockIssueService.update).not.toHaveBeenCalled();
});
});
server/src/__tests__/issue-execution-policy.test.ts
+26 -38
View File
@@ -413,45 +413,33 @@ describe("issue execution policy transitions", () => {
const policy = twoStagePolicy();
const reviewStageId = policy.stages[0].id;
it("non-participant stage updates are coerced back to the active stage", () => {
const result = applyIssueExecutionPolicyTransition({
issue: {
status: "in_review",
assigneeAgentId: qaAgentId,
assigneeUserId: null,
executionPolicy: policy,
executionState: {
status: "pending",
currentStageId: reviewStageId,
currentStageIndex: 0,
currentStageType: "review",
currentParticipant: { type: "agent", agentId: qaAgentId },
returnAssignee: { type: "agent", agentId: coderAgentId },
completedStageIds: [],
lastDecisionId: null,
lastDecisionOutcome: null,
it("non-participant cannot advance the active stage", () => {
expect(() =>
applyIssueExecutionPolicyTransition({
issue: {
status: "in_review",
assigneeAgentId: qaAgentId,
assigneeUserId: null,
executionPolicy: policy,
executionState: {
status: "pending",
currentStageId: reviewStageId,
currentStageIndex: 0,
currentStageType: "review",
currentParticipant: { type: "agent", agentId: qaAgentId },
returnAssignee: { type: "agent", agentId: coderAgentId },
completedStageIds: [],
lastDecisionId: null,
lastDecisionOutcome: null,
},
},
},
policy,
requestedStatus: "done",
requestedAssigneePatch: { assigneeUserId: boardUserId },
actor: { agentId: coderAgentId },
commentBody: "Trying to bypass review",
});
expect(result.patch).toMatchObject({
status: "in_review",
assigneeAgentId: qaAgentId,
assigneeUserId: null,
executionState: {
status: "pending",
currentStageId: reviewStageId,
currentStageType: "review",
currentParticipant: { type: "agent", agentId: qaAgentId },
returnAssignee: { type: "agent", agentId: coderAgentId },
},
});
expect(result.decision).toBeUndefined();
policy,
requestedStatus: "done",
requestedAssigneePatch: { assigneeUserId: boardUserId },
actor: { agentId: coderAgentId },
commentBody: "Trying to bypass review",
}),
).toThrow("Only the active reviewer or approver can advance");
});
it("non-participant can still post non-advancing updates", () => {
server/src/__tests__/issue-update-comment-wakeup-routes.test.ts
+202
View File
@@ -0,0 +1,202 @@
import express from "express";
import request from "supertest";
import { beforeEach, describe, expect, it, vi } from "vitest";
import { errorHandler } from "../middleware/index.js";
import { issueRoutes } from "../routes/issues.js";
const ASSIGNEE_AGENT_ID = "11111111-1111-4111-8111-111111111111";
const mockIssueService = vi.hoisted(() => ({
getById: vi.fn(),
update: vi.fn(),
addComment: vi.fn(),
findMentionedAgents: vi.fn(),
getRelationSummaries: vi.fn(),
listWakeableBlockedDependents: vi.fn(),
getWakeableParentAfterChildCompletion: vi.fn(),
}));
const mockHeartbeatService = vi.hoisted(() => ({
wakeup: vi.fn(async () => undefined),
reportRunActivity: vi.fn(async () => undefined),
getRun: vi.fn(async () => null),
getActiveRunForAgent: vi.fn(async () => null),
cancelRun: vi.fn(async () => null),
}));
vi.mock("../services/index.js", () => ({
accessService: () => ({
canUser: vi.fn(async () => true),
hasPermission: vi.fn(async () => true),
}),
agentService: () => ({
getById: vi.fn(async () => null),
}),
documentService: () => ({}),
executionWorkspaceService: () => ({}),
feedbackService: () => ({
listIssueVotesForUser: vi.fn(async () => []),
saveIssueVote: vi.fn(async () => ({ vote: null, consentEnabledNow: false, sharingEnabled: false })),
}),
goalService: () => ({}),
heartbeatService: () => mockHeartbeatService,
instanceSettingsService: () => ({
get: vi.fn(async () => ({
id: "instance-settings-1",
general: {
censorUsernameInLogs: false,
feedbackDataSharingPreference: "prompt",
},
})),
listCompanyIds: vi.fn(async () => ["company-1"]),
}),
issueApprovalService: () => ({}),
issueService: () => mockIssueService,
logActivity: vi.fn(async () => undefined),
projectService: () => ({}),
routineService: () => ({
syncRunStatusForIssue: vi.fn(async () => undefined),
}),
workProductService: () => ({}),
}));
function createApp() {
const app = express();
app.use(express.json());
app.use((req, _res, next) => {
(req as any).actor = {
type: "board",
userId: "local-board",
companyIds: ["company-1"],
source: "local_implicit",
isInstanceAdmin: false,
};
next();
});
app.use("/api", issueRoutes({} as any, {} as any));
app.use(errorHandler);
return app;
}
function makeIssue(overrides: Record<string, unknown> = {}) {
return {
id: "aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa",
companyId: "company-1",
status: "todo",
priority: "medium",
projectId: null,
goalId: null,
parentId: null,
assigneeAgentId: null,
assigneeUserId: "local-board",
createdByUserId: "local-board",
identifier: "PAP-999",
title: "Wake test",
executionPolicy: null,
executionState: null,
hiddenAt: null,
...overrides,
};
}
describe("issue update comment wakeups", () => {
beforeEach(() => {
vi.clearAllMocks();
mockIssueService.findMentionedAgents.mockResolvedValue([]);
mockIssueService.getRelationSummaries.mockResolvedValue({ blockedBy: [], blocks: [] });
mockIssueService.listWakeableBlockedDependents.mockResolvedValue([]);
mockIssueService.getWakeableParentAfterChildCompletion.mockResolvedValue(null);
});
it("includes the new comment in assignment wakes from issue updates", async () => {
const existing = makeIssue();
const updated = makeIssue({
assigneeAgentId: ASSIGNEE_AGENT_ID,
assigneeUserId: null,
});
mockIssueService.getById.mockResolvedValue(existing);
mockIssueService.update.mockResolvedValue(updated);
mockIssueService.addComment.mockResolvedValue({
id: "comment-1",
issueId: existing.id,
companyId: existing.companyId,
body: "write the whole thing",
});
const res = await request(createApp())
.patch(`/api/issues/${existing.id}`)
.send({
assigneeAgentId: ASSIGNEE_AGENT_ID,
assigneeUserId: null,
comment: "write the whole thing",
});
expect(res.status).toBe(200);
expect(mockHeartbeatService.wakeup).toHaveBeenCalledTimes(1);
expect(mockHeartbeatService.wakeup).toHaveBeenCalledWith(
ASSIGNEE_AGENT_ID,
expect.objectContaining({
source: "assignment",
reason: "issue_assigned",
payload: expect.objectContaining({
issueId: existing.id,
commentId: "comment-1",
mutation: "update",
}),
contextSnapshot: expect.objectContaining({
issueId: existing.id,
taskId: existing.id,
commentId: "comment-1",
wakeCommentId: "comment-1",
source: "issue.update",
}),
}),
);
});
it("wakes the assignee on comment-only issue updates", async () => {
const existing = makeIssue({
assigneeAgentId: ASSIGNEE_AGENT_ID,
assigneeUserId: null,
status: "in_progress",
});
const updated = { ...existing };
mockIssueService.getById.mockResolvedValue(existing);
mockIssueService.update.mockResolvedValue(updated);
mockIssueService.addComment.mockResolvedValue({
id: "comment-2",
issueId: existing.id,
companyId: existing.companyId,
body: "please revise this",
});
const res = await request(createApp())
.patch(`/api/issues/${existing.id}`)
.send({
comment: "please revise this",
});
expect(res.status).toBe(200);
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",
}),
}),
);
});
});
server/src/routes/issues.ts
+41 -33
View File
@@ -96,13 +96,6 @@ function executionPrincipalsEqual(
return left.type === "agent" ? left.agentId === right.agentId : left.userId === right.userId;
}
function executionParticipantMatchesAgent(
participant: ParsedExecutionState["currentParticipant"] | null,
agentId: string | null | undefined,
) {
return Boolean(agentId) && participant?.type === "agent" && participant.agentId === agentId;
}
function buildExecutionStageWakeContext(input: {
state: ParsedExecutionState;
wakeRole: ExecutionStageWakeContext["wakeRole"];
@@ -1386,14 +1379,10 @@ export function issueRoutes(
? (updateFields.executionPolicy as NormalizedExecutionPolicy | null)
: previousExecutionPolicy;
const requestedStatus = typeof updateFields.status === "string" ? updateFields.status : undefined;
const requestedAssigneePatchProvided =
req.body.assigneeAgentId !== undefined || req.body.assigneeUserId !== undefined;
const transition = applyIssueExecutionPolicyTransition({
issue: existing,
policy: nextExecutionPolicy,
requestedStatus,
requestedStatus: typeof updateFields.status === "string" ? updateFields.status : undefined,
requestedAssigneePatch: {
assigneeAgentId:
req.body.assigneeAgentId === undefined ? undefined : (req.body.assigneeAgentId as string | null),
@@ -1419,27 +1408,6 @@ export function issueRoutes(
}
Object.assign(updateFields, transition.patch);
const effectiveExecutionState = parseIssueExecutionState(
transition.patch.executionState !== undefined ? transition.patch.executionState : existing.executionState,
);
const isUnauthorizedAgentStageMutation =
req.actor.type === "agent" &&
req.actor.agentId &&
existing.status === "in_review" &&
transition.workflowControlledAssignment &&
!transition.decision &&
effectiveExecutionState?.status === "pending" &&
(
(requestedStatus !== undefined && requestedStatus !== "in_review") ||
requestedAssigneePatchProvided
) &&
!executionParticipantMatchesAgent(effectiveExecutionState.currentParticipant, req.actor.agentId);
if (isUnauthorizedAgentStageMutation) {
const stageLabel = effectiveExecutionState.currentStageType ?? "execution";
res.status(403).json({ error: `Only the active ${stageLabel} participant can update this stage` });
return;
}
const nextAssigneeAgentId =
updateFields.assigneeAgentId === undefined ? existing.assigneeAgentId : (updateFields.assigneeAgentId as string | null);
const nextAssigneeUserId =
@@ -1733,6 +1701,7 @@ export function issueRoutes(
reason: "issue_assigned",
payload: {
issueId: issue.id,
...(comment ? { commentId: comment.id } : {}),
mutation: "update",
...(interruptedRunId ? { interruptedRunId } : {}),
},
@@ -1740,6 +1709,13 @@ export function issueRoutes(
requestedByActorId: actor.actorId,
contextSnapshot: {
issueId: issue.id,
...(comment
? {
taskId: issue.id,
commentId: comment.id,
wakeCommentId: comment.id,
}
: {}),
source: "issue.update",
...(interruptedRunId ? { interruptedRunId } : {}),
},
@@ -1767,6 +1743,38 @@ export function issueRoutes(
}
if (commentBody && comment) {
const assigneeId = issue.assigneeAgentId;
const actorIsAgent = actor.actorType === "agent";
const selfComment = actorIsAgent && actor.actorId === assigneeId;
const skipAssigneeCommentWake = selfComment || isClosed;
if (assigneeId && !assigneeChanged && !skipAssigneeCommentWake) {
addWakeup(assigneeId, {
source: "automation",
triggerDetail: "system",
reason: reopened ? "issue_reopened_via_comment" : "issue_commented",
payload: {
issueId: id,
commentId: comment.id,
mutation: "comment",
...(reopened ? { reopenedFrom: reopenFromStatus } : {}),
...(interruptedRunId ? { interruptedRunId } : {}),
},
requestedByActorType: actor.actorType,
requestedByActorId: actor.actorId,
contextSnapshot: {
issueId: id,
taskId: id,
commentId: comment.id,
wakeCommentId: comment.id,
source: reopened ? "issue.comment.reopen" : "issue.comment",
wakeReason: reopened ? "issue_reopened_via_comment" : "issue_commented",
...(reopened ? { reopenedFrom: reopenFromStatus } : {}),
...(interruptedRunId ? { interruptedRunId } : {}),
},
});
}
let mentionedIds: string[] = [];
try {
mentionedIds = await svc.findMentionedAgents(issue.companyId, commentBody);
server/src/services/activity.ts
+10 -1
View File
@@ -1,6 +1,6 @@
import { and, desc, eq, isNull, or, sql } from "drizzle-orm";
import type { Db } from "@paperclipai/db";
import { activityLog, heartbeatRuns, issues } from "@paperclipai/db";
import { activityLog, agents, heartbeatRuns, issues } from "@paperclipai/db";
export interface ActivityFilters {
companyId: string;
@@ -66,14 +66,23 @@ export function activityService(db: Db) {
runId: heartbeatRuns.id,
status: heartbeatRuns.status,
agentId: heartbeatRuns.agentId,
adapterType: agents.adapterType,
startedAt: heartbeatRuns.startedAt,
finishedAt: heartbeatRuns.finishedAt,
createdAt: heartbeatRuns.createdAt,
invocationSource: heartbeatRuns.invocationSource,
usageJson: heartbeatRuns.usageJson,
resultJson: heartbeatRuns.resultJson,
logBytes: heartbeatRuns.logBytes,
})
.from(heartbeatRuns)
.innerJoin(
agents,
and(
eq(agents.id, heartbeatRuns.agentId),
eq(agents.companyId, heartbeatRuns.companyId),
),
)
.where(
and(
eq(heartbeatRuns.companyId, companyId),
server/src/services/heartbeat.ts
+23 -12
View File
@@ -707,6 +707,18 @@ export function shouldResetTaskSessionForWake(
return false;
}
function shouldRequireIssueCommentForWake(
contextSnapshot: Record<string, unknown> | null | undefined,
) {
const wakeReason = readNonEmptyString(contextSnapshot?.wakeReason);
return (
wakeReason === "issue_assigned" ||
wakeReason === "execution_review_requested" ||
wakeReason === "execution_approval_requested" ||
wakeReason === "execution_changes_requested"
);
}
export function formatRuntimeWorkspaceWarningLog(warning: string) {
return {
stream: "stdout" as const,
@@ -2011,18 +2023,6 @@ export function heartbeatService(db: Db) {
return { outcome: "not_applicable" as const, queuedRun: null };
}
const wakeReason = readNonEmptyString(contextSnapshot.wakeReason);
if (wakeReason === "issue_commented" || wakeReason === "issue_comment_mentioned" || wakeReason === "issue_reopened_via_comment") {
if (run.issueCommentStatus !== "not_applicable") {
await patchRunIssueCommentStatus(run.id, {
issueCommentStatus: "not_applicable",
issueCommentSatisfiedByCommentId: null,
issueCommentRetryQueuedAt: null,
});
}
return { outcome: "not_applicable" as const, queuedRun: null };
}
const postedComment = await findRunIssueComment(run.id, run.companyId, issueId);
if (postedComment) {
await patchRunIssueCommentStatus(run.id, {
@@ -2047,6 +2047,17 @@ export function heartbeatService(db: Db) {
return { outcome: "retry_exhausted" as const, queuedRun: null };
}
if (!shouldRequireIssueCommentForWake(contextSnapshot)) {
if (run.issueCommentStatus !== "not_applicable") {
await patchRunIssueCommentStatus(run.id, {
issueCommentStatus: "not_applicable",
issueCommentSatisfiedByCommentId: null,
issueCommentRetryQueuedAt: null,
});
}
return { outcome: "not_applicable" as const, queuedRun: null };
}
const queuedRun = await enqueueMissingIssueCommentRetry(run, agent, issueId);
if (queuedRun) {
await appendRunEvent(run, await nextRunEventSeq(run.id), {
server/src/services/issue-execution-policy.ts
+11 -5
View File
@@ -393,13 +393,19 @@ export function applyIssueExecutionPolicyTransition(input: TransitionInput): Tra
}
}
if (
const attemptedStageAdvance =
(requestedStatus !== undefined && requestedStatus !== "in_review") ||
(requestedAssigneePatchProvided && !principalsEqual(explicitAssignee, currentParticipant));
const stageStateDrifted =
input.issue.status !== "in_review" ||
!principalsEqual(currentAssignee, currentParticipant) ||
!principalsEqual(existingState?.currentParticipant ?? null, currentParticipant) ||
(requestedStatus !== undefined && requestedStatus !== "in_review") ||
(requestedAssigneePatchProvided && !principalsEqual(explicitAssignee, currentParticipant))
) {
!principalsEqual(existingState?.currentParticipant ?? null, currentParticipant);
if (attemptedStageAdvance && !stageStateDrifted) {
throw unprocessable("Only the active reviewer or approver can advance the current execution stage");
}
if (stageStateDrifted) {
buildPendingStagePatch({
patch,
previous: existingState,