forked from farhoodlabs/paperclip
Track blocker and review activity events
This commit is contained in:
dotta
@@ -0,0 +1,244 @@
|
||||
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";
|
||||
import { normalizeIssueExecutionPolicy } from "../services/issue-execution-policy.ts";
|
||||
|
||||
const mockIssueService = vi.hoisted(() => ({
|
||||
getById: vi.fn(),
|
||||
assertCheckoutOwner: vi.fn(),
|
||||
update: vi.fn(),
|
||||
addComment: vi.fn(),
|
||||
findMentionedAgents: vi.fn(),
|
||||
getRelationSummaries: vi.fn(),
|
||||
listWakeableBlockedDependents: vi.fn(),
|
||||
getWakeableParentAfterChildCompletion: vi.fn(),
|
||||
}));
|
||||
|
||||
const mockLogActivity = vi.hoisted(() => vi.fn(async () => undefined));
|
||||
|
||||
vi.mock("../services/index.js", () => ({
|
||||
accessService: () => ({
|
||||
canUser: vi.fn(async () => false),
|
||||
hasPermission: vi.fn(async () => false),
|
||||
}),
|
||||
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: () => ({
|
||||
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),
|
||||
}),
|
||||
instanceSettingsService: () => ({
|
||||
get: vi.fn(async () => ({
|
||||
id: "instance-settings-1",
|
||||
general: {
|
||||
censorUsernameInLogs: false,
|
||||
feedbackDataSharingPreference: "prompt",
|
||||
},
|
||||
})),
|
||||
listCompanyIds: vi.fn(async () => ["company-1"]),
|
||||
}),
|
||||
issueApprovalService: () => ({}),
|
||||
issueService: () => mockIssueService,
|
||||
logActivity: mockLogActivity,
|
||||
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() {
|
||||
return {
|
||||
id: "11111111-1111-4111-8111-111111111111",
|
||||
companyId: "company-1",
|
||||
status: "todo",
|
||||
assigneeAgentId: "22222222-2222-4222-8222-222222222222",
|
||||
assigneeUserId: null,
|
||||
createdByUserId: "local-board",
|
||||
identifier: "PAP-580",
|
||||
title: "Activity event issue",
|
||||
executionPolicy: null,
|
||||
executionState: null,
|
||||
};
|
||||
}
|
||||
|
||||
describe("issue activity event routes", () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
mockIssueService.assertCheckoutOwner.mockResolvedValue({ adoptedFromRunId: null });
|
||||
mockIssueService.findMentionedAgents.mockResolvedValue([]);
|
||||
mockIssueService.getRelationSummaries.mockResolvedValue({ blockedBy: [], blocks: [] });
|
||||
mockIssueService.listWakeableBlockedDependents.mockResolvedValue([]);
|
||||
mockIssueService.getWakeableParentAfterChildCompletion.mockResolvedValue(null);
|
||||
});
|
||||
|
||||
it("logs blocker activity with added and removed issue summaries", async () => {
|
||||
const issue = makeIssue();
|
||||
mockIssueService.getById.mockResolvedValue(issue);
|
||||
mockIssueService.getRelationSummaries
|
||||
.mockResolvedValueOnce({
|
||||
blockedBy: [
|
||||
{
|
||||
id: "aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa",
|
||||
identifier: "PAP-10",
|
||||
title: "Old blocker",
|
||||
status: "todo",
|
||||
priority: "medium",
|
||||
assigneeAgentId: null,
|
||||
assigneeUserId: null,
|
||||
},
|
||||
],
|
||||
blocks: [],
|
||||
})
|
||||
.mockResolvedValueOnce({
|
||||
blockedBy: [
|
||||
{
|
||||
id: "bbbbbbbb-bbbb-4bbb-8bbb-bbbbbbbbbbbb",
|
||||
identifier: "PAP-11",
|
||||
title: "New blocker",
|
||||
status: "todo",
|
||||
priority: "medium",
|
||||
assigneeAgentId: null,
|
||||
assigneeUserId: null,
|
||||
},
|
||||
],
|
||||
blocks: [],
|
||||
});
|
||||
mockIssueService.update.mockImplementation(async (_id: string, patch: Record<string, unknown>) => ({
|
||||
...issue,
|
||||
...patch,
|
||||
updatedAt: new Date(),
|
||||
}));
|
||||
|
||||
const res = await request(createApp())
|
||||
.patch("/api/issues/11111111-1111-4111-8111-111111111111")
|
||||
.send({ blockedByIssueIds: ["bbbbbbbb-bbbb-4bbb-8bbb-bbbbbbbbbbbb"] });
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
expect(mockLogActivity).toHaveBeenCalledWith(
|
||||
expect.anything(),
|
||||
expect.objectContaining({
|
||||
action: "issue.blockers_updated",
|
||||
details: expect.objectContaining({
|
||||
addedBlockedByIssueIds: ["bbbbbbbb-bbbb-4bbb-8bbb-bbbbbbbbbbbb"],
|
||||
removedBlockedByIssueIds: ["aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa"],
|
||||
addedBlockedByIssues: [
|
||||
{
|
||||
id: "bbbbbbbb-bbbb-4bbb-8bbb-bbbbbbbbbbbb",
|
||||
identifier: "PAP-11",
|
||||
title: "New blocker",
|
||||
},
|
||||
],
|
||||
removedBlockedByIssues: [
|
||||
{
|
||||
id: "aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa",
|
||||
identifier: "PAP-10",
|
||||
title: "Old blocker",
|
||||
},
|
||||
],
|
||||
}),
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("logs explicit reviewer and approver activity when execution policy participants change", async () => {
|
||||
const existingPolicy = normalizeIssueExecutionPolicy({
|
||||
stages: [
|
||||
{
|
||||
id: "11111111-1111-4111-8111-111111111111",
|
||||
type: "review",
|
||||
participants: [{ type: "agent", agentId: "11111111-2222-4333-8444-555555555555" }],
|
||||
},
|
||||
{
|
||||
id: "22222222-2222-4222-8222-222222222222",
|
||||
type: "approval",
|
||||
participants: [{ type: "agent", agentId: "66666666-7777-4888-8999-aaaaaaaaaaaa" }],
|
||||
},
|
||||
],
|
||||
})!;
|
||||
const nextPolicy = normalizeIssueExecutionPolicy({
|
||||
stages: [
|
||||
{
|
||||
id: "11111111-1111-4111-8111-111111111111",
|
||||
type: "review",
|
||||
participants: [{ type: "agent", agentId: "bbbbbbbb-cccc-4ddd-8eee-ffffffffffff" }],
|
||||
},
|
||||
{
|
||||
id: "22222222-2222-4222-8222-222222222222",
|
||||
type: "approval",
|
||||
participants: [{ type: "user", userId: "local-board" }],
|
||||
},
|
||||
],
|
||||
})!;
|
||||
const issue = {
|
||||
...makeIssue(),
|
||||
executionPolicy: existingPolicy,
|
||||
};
|
||||
mockIssueService.getById.mockResolvedValue(issue);
|
||||
mockIssueService.update.mockImplementation(async (_id: string, patch: Record<string, unknown>) => ({
|
||||
...issue,
|
||||
...patch,
|
||||
executionPolicy: patch.executionPolicy,
|
||||
updatedAt: new Date(),
|
||||
}));
|
||||
|
||||
const res = await request(createApp())
|
||||
.patch("/api/issues/11111111-1111-4111-8111-111111111111")
|
||||
.send({ executionPolicy: nextPolicy });
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
expect(mockLogActivity).toHaveBeenCalledWith(
|
||||
expect.anything(),
|
||||
expect.objectContaining({
|
||||
action: "issue.reviewers_updated",
|
||||
details: expect.objectContaining({
|
||||
participants: [{ type: "agent", agentId: "bbbbbbbb-cccc-4ddd-8eee-ffffffffffff", userId: null }],
|
||||
addedParticipants: [{ type: "agent", agentId: "bbbbbbbb-cccc-4ddd-8eee-ffffffffffff", userId: null }],
|
||||
removedParticipants: [{ type: "agent", agentId: "11111111-2222-4333-8444-555555555555", userId: null }],
|
||||
}),
|
||||
}),
|
||||
);
|
||||
expect(mockLogActivity).toHaveBeenCalledWith(
|
||||
expect.anything(),
|
||||
expect.objectContaining({
|
||||
action: "issue.approvers_updated",
|
||||
details: expect.objectContaining({
|
||||
participants: [{ type: "user", agentId: null, userId: "local-board" }],
|
||||
addedParticipants: [{ type: "user", agentId: null, userId: "local-board" }],
|
||||
removedParticipants: [{ type: "agent", agentId: "66666666-7777-4888-8999-aaaaaaaaaaaa", userId: null }],
|
||||
}),
|
||||
}),
|
||||
);
|
||||
});
|
||||
});
|
||||
+122
-6
@@ -68,6 +68,16 @@ const updateIssueRouteSchema = updateIssueSchema.extend({
|
||||
});
|
||||
|
||||
type ParsedExecutionState = NonNullable<ReturnType<typeof parseIssueExecutionState>>;
|
||||
type NormalizedExecutionPolicy = NonNullable<ReturnType<typeof normalizeIssueExecutionPolicy>>;
|
||||
type ActivityIssueRelationSummary = {
|
||||
id: string;
|
||||
identifier: string | null;
|
||||
title: string;
|
||||
};
|
||||
type ActivityExecutionParticipant = Pick<
|
||||
NormalizedExecutionPolicy["stages"][number]["participants"][number],
|
||||
"type" | "agentId" | "userId"
|
||||
>;
|
||||
type ExecutionStageWakeContext = {
|
||||
wakeRole: "reviewer" | "approver" | "executor";
|
||||
stageId: string | null;
|
||||
@@ -102,6 +112,59 @@ function buildExecutionStageWakeContext(input: {
|
||||
};
|
||||
}
|
||||
|
||||
function summarizeIssueRelationForActivity(relation: {
|
||||
id: string;
|
||||
identifier: string | null;
|
||||
title: string;
|
||||
}): ActivityIssueRelationSummary {
|
||||
return {
|
||||
id: relation.id,
|
||||
identifier: relation.identifier,
|
||||
title: relation.title,
|
||||
};
|
||||
}
|
||||
|
||||
function activityExecutionParticipantKey(participant: ActivityExecutionParticipant): string {
|
||||
return participant.type === "agent" ? `agent:${participant.agentId}` : `user:${participant.userId}`;
|
||||
}
|
||||
|
||||
function summarizeExecutionParticipants(
|
||||
policy: NormalizedExecutionPolicy | null,
|
||||
stageType: NormalizedExecutionPolicy["stages"][number]["type"],
|
||||
): ActivityExecutionParticipant[] {
|
||||
const stage = policy?.stages.find((candidate) => candidate.type === stageType);
|
||||
return (
|
||||
stage?.participants.map((participant) => ({
|
||||
type: participant.type,
|
||||
agentId: participant.agentId ?? null,
|
||||
userId: participant.userId ?? null,
|
||||
})) ?? []
|
||||
);
|
||||
}
|
||||
|
||||
function diffExecutionParticipants(
|
||||
previousPolicy: NormalizedExecutionPolicy | null,
|
||||
nextPolicy: NormalizedExecutionPolicy | null,
|
||||
stageType: NormalizedExecutionPolicy["stages"][number]["type"],
|
||||
) {
|
||||
const previousParticipants = summarizeExecutionParticipants(previousPolicy, stageType);
|
||||
const nextParticipants = summarizeExecutionParticipants(nextPolicy, stageType);
|
||||
const previousByKey = new Map(previousParticipants.map((participant) => [
|
||||
activityExecutionParticipantKey(participant),
|
||||
participant,
|
||||
]));
|
||||
const nextByKey = new Map(nextParticipants.map((participant) => [
|
||||
activityExecutionParticipantKey(participant),
|
||||
participant,
|
||||
]));
|
||||
|
||||
return {
|
||||
participants: nextParticipants,
|
||||
addedParticipants: nextParticipants.filter((participant) => !previousByKey.has(activityExecutionParticipantKey(participant))),
|
||||
removedParticipants: previousParticipants.filter((participant) => !nextByKey.has(activityExecutionParticipantKey(participant))),
|
||||
};
|
||||
}
|
||||
|
||||
function buildExecutionStageWakeup(input: {
|
||||
issueId: string;
|
||||
previousState: ParsedExecutionState | null;
|
||||
@@ -1202,9 +1265,10 @@ export function issueRoutes(
|
||||
}
|
||||
|
||||
const actor = getActorInfo(req);
|
||||
const executionPolicy = normalizeIssueExecutionPolicy(req.body.executionPolicy);
|
||||
const issue = await svc.create(companyId, {
|
||||
...req.body,
|
||||
executionPolicy: normalizeIssueExecutionPolicy(req.body.executionPolicy),
|
||||
executionPolicy,
|
||||
createdByAgentId: actor.agentId,
|
||||
createdByUserId: actor.actorType === "user" ? actor.actorId : null,
|
||||
});
|
||||
@@ -1309,13 +1373,15 @@ export function issueRoutes(
|
||||
if (req.body.executionPolicy !== undefined) {
|
||||
updateFields.executionPolicy = normalizeIssueExecutionPolicy(req.body.executionPolicy);
|
||||
}
|
||||
const previousExecutionPolicy = normalizeIssueExecutionPolicy(existing.executionPolicy ?? null);
|
||||
const nextExecutionPolicy =
|
||||
updateFields.executionPolicy !== undefined
|
||||
? (updateFields.executionPolicy as NormalizedExecutionPolicy | null)
|
||||
: previousExecutionPolicy;
|
||||
|
||||
const transition = applyIssueExecutionPolicyTransition({
|
||||
issue: existing,
|
||||
policy:
|
||||
updateFields.executionPolicy !== undefined
|
||||
? (updateFields.executionPolicy as NonNullable<typeof updateFields.executionPolicy> | null)
|
||||
: normalizeIssueExecutionPolicy(existing.executionPolicy ?? null),
|
||||
policy: nextExecutionPolicy,
|
||||
requestedStatus: typeof updateFields.status === "string" ? updateFields.status : undefined,
|
||||
requestedAssigneePatch: {
|
||||
assigneeAgentId:
|
||||
@@ -1430,8 +1496,9 @@ export function issueRoutes(
|
||||
return;
|
||||
}
|
||||
let issueResponse: typeof issue & { blockedBy?: unknown; blocks?: unknown } = issue;
|
||||
let updatedRelations: Awaited<ReturnType<typeof svc.getRelationSummaries>> | null = null;
|
||||
if (issue && Array.isArray(req.body.blockedByIssueIds)) {
|
||||
const updatedRelations = await svc.getRelationSummaries(issue.id);
|
||||
updatedRelations = await svc.getRelationSummaries(issue.id);
|
||||
issueResponse = {
|
||||
...issue,
|
||||
blockedBy: updatedRelations.blockedBy,
|
||||
@@ -1488,6 +1555,8 @@ export function issueRoutes(
|
||||
const nextBlockedByIds = new Set(req.body.blockedByIssueIds as string[]);
|
||||
const addedBlockedByIssueIds = [...nextBlockedByIds].filter((candidate) => !previousBlockedByIds.has(candidate));
|
||||
const removedBlockedByIssueIds = [...previousBlockedByIds].filter((candidate) => !nextBlockedByIds.has(candidate));
|
||||
const nextBlockedByRelations = updatedRelations?.blockedBy ?? [];
|
||||
const previousBlockedByRelations = existingRelations?.blockedBy ?? [];
|
||||
if (addedBlockedByIssueIds.length > 0 || removedBlockedByIssueIds.length > 0) {
|
||||
await logActivity(db, {
|
||||
companyId: issue.companyId,
|
||||
@@ -1503,11 +1572,58 @@ export function issueRoutes(
|
||||
blockedByIssueIds: req.body.blockedByIssueIds,
|
||||
addedBlockedByIssueIds,
|
||||
removedBlockedByIssueIds,
|
||||
blockedByIssues: nextBlockedByRelations.map(summarizeIssueRelationForActivity),
|
||||
addedBlockedByIssues: nextBlockedByRelations
|
||||
.filter((relation) => addedBlockedByIssueIds.includes(relation.id))
|
||||
.map(summarizeIssueRelationForActivity),
|
||||
removedBlockedByIssues: previousBlockedByRelations
|
||||
.filter((relation) => removedBlockedByIssueIds.includes(relation.id))
|
||||
.map(summarizeIssueRelationForActivity),
|
||||
},
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
const reviewerChanges = diffExecutionParticipants(previousExecutionPolicy, nextExecutionPolicy, "review");
|
||||
if (reviewerChanges.addedParticipants.length > 0 || reviewerChanges.removedParticipants.length > 0) {
|
||||
await logActivity(db, {
|
||||
companyId: issue.companyId,
|
||||
actorType: actor.actorType,
|
||||
actorId: actor.actorId,
|
||||
agentId: actor.agentId,
|
||||
runId: actor.runId,
|
||||
action: "issue.reviewers_updated",
|
||||
entityType: "issue",
|
||||
entityId: issue.id,
|
||||
details: {
|
||||
identifier: issue.identifier,
|
||||
participants: reviewerChanges.participants,
|
||||
addedParticipants: reviewerChanges.addedParticipants,
|
||||
removedParticipants: reviewerChanges.removedParticipants,
|
||||
},
|
||||
});
|
||||
}
|
||||
|
||||
const approverChanges = diffExecutionParticipants(previousExecutionPolicy, nextExecutionPolicy, "approval");
|
||||
if (approverChanges.addedParticipants.length > 0 || approverChanges.removedParticipants.length > 0) {
|
||||
await logActivity(db, {
|
||||
companyId: issue.companyId,
|
||||
actorType: actor.actorType,
|
||||
actorId: actor.actorId,
|
||||
agentId: actor.agentId,
|
||||
runId: actor.runId,
|
||||
action: "issue.approvers_updated",
|
||||
entityType: "issue",
|
||||
entityId: issue.id,
|
||||
details: {
|
||||
identifier: issue.identifier,
|
||||
participants: approverChanges.participants,
|
||||
addedParticipants: approverChanges.addedParticipants,
|
||||
removedParticipants: approverChanges.removedParticipants,
|
||||
},
|
||||
});
|
||||
}
|
||||
|
||||
if (issue.status === "done" && existing.status !== "done") {
|
||||
const tc = getTelemetryClient();
|
||||
if (tc && actor.agentId) {
|
||||
|
||||
Reference in New Issue
Block a user