Merge 9806f78139 into c9e3bb7ca4
This commit is contained in:
commit
ccef7fa494
|
|
@ -697,7 +697,9 @@ export interface IssueExecutionMonitorState {
|
|||
}
|
||||
|
||||
export interface IssueReviewRequest {
|
||||
instructions: string;
|
||||
/** Server-owned binding to the confirmation that gates this review. */
|
||||
id?: string;
|
||||
instructions?: string;
|
||||
}
|
||||
|
||||
export interface IssueExecutionState {
|
||||
|
|
@ -790,6 +792,7 @@ export interface Issue {
|
|||
workMode: IssueWorkMode;
|
||||
priority: IssuePriority;
|
||||
reviewPolicy: IssueReviewPolicy | null;
|
||||
reviewInteractionId?: string | null;
|
||||
assigneeAgentId: string | null;
|
||||
assigneeUserId: string | null;
|
||||
checkoutRunId: string | null;
|
||||
|
|
|
|||
|
|
@ -500,6 +500,14 @@ export const issueReviewRequestSchema = z
|
|||
})
|
||||
.strict();
|
||||
|
||||
const issueExecutionReviewRequestSchema = z
|
||||
.object({
|
||||
id: z.string().guid().optional(),
|
||||
instructions: z.string().trim().min(1).max(20000).optional(),
|
||||
})
|
||||
.strict()
|
||||
.refine((value) => value.id !== undefined || value.instructions !== undefined);
|
||||
|
||||
export const issueExecutionStateSchema = z.object({
|
||||
status: z.enum(ISSUE_EXECUTION_STATE_STATUSES),
|
||||
currentStageId: z.string().guid().nullable(),
|
||||
|
|
@ -507,7 +515,7 @@ export const issueExecutionStateSchema = z.object({
|
|||
currentStageType: z.enum(ISSUE_EXECUTION_STAGE_TYPES).nullable(),
|
||||
currentParticipant: issueExecutionStagePrincipalSchema.nullable(),
|
||||
returnAssignee: issueExecutionStagePrincipalSchema.nullable(),
|
||||
reviewRequest: issueReviewRequestSchema.nullable().optional().default(null),
|
||||
reviewRequest: issueExecutionReviewRequestSchema.nullable().optional().default(null),
|
||||
completedStageIds: z.array(z.string().guid()).default([]),
|
||||
lastDecisionId: z.string().guid().nullable(),
|
||||
lastDecisionOutcome: z.enum(ISSUE_EXECUTION_DECISION_OUTCOMES).nullable(),
|
||||
|
|
|
|||
|
|
@ -38,6 +38,9 @@ const mockAgentService = vi.hoisted(() => ({
|
|||
}));
|
||||
|
||||
const mockLogActivity = vi.hoisted(() => vi.fn(async () => undefined));
|
||||
const mockResolveIssueReviewRequester = vi.hoisted(() =>
|
||||
vi.fn(async () => null),
|
||||
);
|
||||
const mockTxInsertValues = vi.hoisted(() => vi.fn(async () => undefined));
|
||||
const mockTxInsert = vi.hoisted(() =>
|
||||
vi.fn(() => ({ values: mockTxInsertValues })),
|
||||
|
|
@ -149,6 +152,16 @@ vi.mock("../services/issues.js", () => ({
|
|||
issueService: () => mockIssueService,
|
||||
}));
|
||||
|
||||
vi.mock("../services/issue-review-policy.js", async (importOriginal) => {
|
||||
const orig = await importOriginal<
|
||||
typeof import("../services/issue-review-policy.js")
|
||||
>();
|
||||
return {
|
||||
...orig,
|
||||
resolveIssueReviewRequester: mockResolveIssueReviewRequester,
|
||||
};
|
||||
});
|
||||
|
||||
vi.mock("../services/routines.js", () => ({
|
||||
routineService: () => mockRoutineService,
|
||||
}));
|
||||
|
|
@ -338,6 +351,7 @@ describe.sequential("issue comment reopen routes", () => {
|
|||
mockAgentService.list.mockReset();
|
||||
mockAgentService.resolveByReference.mockReset();
|
||||
mockLogActivity.mockReset();
|
||||
mockResolveIssueReviewRequester.mockReset();
|
||||
mockFeedbackService.listIssueVotesForUser.mockReset();
|
||||
mockFeedbackService.saveIssueVote.mockReset();
|
||||
mockInstanceSettingsService.get.mockReset();
|
||||
|
|
@ -359,6 +373,7 @@ describe.sequential("issue comment reopen routes", () => {
|
|||
mockDbSelectOrderBy.mockReset();
|
||||
mockDb.transaction.mockReset();
|
||||
mockTxInsertValues.mockResolvedValue(undefined);
|
||||
mockResolveIssueReviewRequester.mockResolvedValue(null);
|
||||
mockTxInsert.mockImplementation(() => ({ values: mockTxInsertValues }));
|
||||
mockDbSelectOrderBy.mockResolvedValue([]);
|
||||
mockDbSelectWhere.mockImplementation(() => ({
|
||||
|
|
@ -3785,6 +3800,71 @@ describe.sequential("issue comment reopen routes", () => {
|
|||
});
|
||||
});
|
||||
|
||||
it("rejects an auto-approval comment while the designated confirmation is pending", async () => {
|
||||
const reviewerAgentId = "33333333-3333-4333-8333-333333333333";
|
||||
const returnAssigneeAgentId = "22222222-2222-4222-8222-222222222222";
|
||||
const interactionId = "44444444-4444-4444-8444-444444444444";
|
||||
const policy = await normalizePolicy({
|
||||
stages: [
|
||||
{
|
||||
id: "aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa",
|
||||
type: "review",
|
||||
participants: [{ type: "agent", agentId: reviewerAgentId }],
|
||||
},
|
||||
],
|
||||
})!;
|
||||
const issue = {
|
||||
...makeIssue("todo"),
|
||||
status: "in_review",
|
||||
assigneeAgentId: reviewerAgentId,
|
||||
executionPolicy: policy,
|
||||
executionState: {
|
||||
status: "pending",
|
||||
currentStageId: policy.stages[0].id,
|
||||
currentStageIndex: 0,
|
||||
currentStageType: "review",
|
||||
currentParticipant: { type: "agent", agentId: reviewerAgentId },
|
||||
returnAssignee: { type: "agent", agentId: returnAssigneeAgentId },
|
||||
completedStageIds: [],
|
||||
lastDecisionId: null,
|
||||
lastDecisionOutcome: null,
|
||||
},
|
||||
};
|
||||
mockIssueService.getById.mockResolvedValue(issue);
|
||||
mockResolveIssueReviewRequester.mockResolvedValue({
|
||||
type: "agent",
|
||||
id: returnAssigneeAgentId,
|
||||
reviewInteractionId: interactionId,
|
||||
});
|
||||
mockIssueThreadInteractionService.listForIssue.mockResolvedValue([{
|
||||
id: interactionId,
|
||||
kind: "request_confirmation",
|
||||
status: "pending",
|
||||
}]);
|
||||
|
||||
const res = await request(
|
||||
await installActor(createApp(), {
|
||||
type: "agent",
|
||||
agentId: reviewerAgentId,
|
||||
companyId: "company-1",
|
||||
source: "agent_key",
|
||||
runId: "run-review-pending-confirmation",
|
||||
}),
|
||||
)
|
||||
.post("/api/issues/11111111-1111-4111-8111-111111111111/comments")
|
||||
.send({ body: "## Review: APPROVED" });
|
||||
|
||||
expect(res.status).toBe(422);
|
||||
expect(res.body).toMatchObject({
|
||||
details: {
|
||||
code: "pending_review_confirmation",
|
||||
reviewInteractionId: interactionId,
|
||||
},
|
||||
});
|
||||
expect(mockIssueService.addComment).not.toHaveBeenCalled();
|
||||
expect(mockIssueService.update).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("rolls back the comment when the auto-approval status transition fails", async () => {
|
||||
const reviewerAgentId = "33333333-3333-4333-8333-333333333333";
|
||||
const policy = await normalizePolicy({
|
||||
|
|
|
|||
|
|
@ -60,6 +60,7 @@ const mockDb = vi.hoisted(() => ({
|
|||
}));
|
||||
|
||||
const mockLogActivity = vi.hoisted(() => vi.fn(async () => undefined));
|
||||
const mockResolveIssueReviewRequester = vi.hoisted(() => vi.fn(async () => null));
|
||||
const mockIssueThreadInteractionService = vi.hoisted(() => ({
|
||||
expirePendingInteractionsForTerminalIssue: vi.fn(async () => []),
|
||||
listForIssue: vi.fn(async () => []),
|
||||
|
|
@ -153,6 +154,14 @@ function registerModuleMocks() {
|
|||
}),
|
||||
workProductService: () => ({}),
|
||||
}));
|
||||
|
||||
vi.doMock("../services/issue-review-policy.js", async (importOriginal) => {
|
||||
const orig = await importOriginal<typeof import("../services/issue-review-policy.js")>();
|
||||
return {
|
||||
...orig,
|
||||
resolveIssueReviewRequester: mockResolveIssueReviewRequester,
|
||||
};
|
||||
});
|
||||
}
|
||||
|
||||
type TestActor =
|
||||
|
|
@ -198,6 +207,7 @@ describe("issue execution policy routes", () => {
|
|||
vi.doUnmock("../services/index.js");
|
||||
vi.doUnmock("../routes/issues.js");
|
||||
vi.doUnmock("../middleware/index.js");
|
||||
vi.doUnmock("../services/issue-review-policy.js");
|
||||
registerModuleMocks();
|
||||
vi.clearAllMocks();
|
||||
mockIssueService.assertCheckoutOwner.mockResolvedValue({ adoptedFromRunId: null });
|
||||
|
|
@ -207,6 +217,7 @@ describe("issue execution policy routes", () => {
|
|||
mockIssueService.listWakeableBlockedDependents.mockResolvedValue([]);
|
||||
mockIssueService.getWakeableParentAfterChildCompletion.mockResolvedValue(null);
|
||||
mockIssueThreadInteractionService.listForIssue.mockResolvedValue([]);
|
||||
mockResolveIssueReviewRequester.mockResolvedValue(null);
|
||||
mockIssueThreadInteractionService.expireRequestConfirmationsSupersededByComment.mockResolvedValue([]);
|
||||
mockIssueApprovalService.listApprovalsForIssue.mockResolvedValue([]);
|
||||
mockDbSelect.mockImplementation(() => ({ from: mockDbSelectFrom }));
|
||||
|
|
@ -556,7 +567,7 @@ describe("issue execution policy routes", () => {
|
|||
expect(activityTx).toBe(updateTx);
|
||||
});
|
||||
|
||||
it("rejects a review binding to a confirmation from another run", async () => {
|
||||
it("binds a confirmation created by the same agent in an earlier run", async () => {
|
||||
const issue = {
|
||||
id: "aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa",
|
||||
companyId: "company-1",
|
||||
|
|
@ -578,6 +589,60 @@ describe("issue execution policy routes", () => {
|
|||
sourceRunId: "44444444-4444-4444-8444-444444444444",
|
||||
payload: { version: 1, prompt: "Approve another run's request?" },
|
||||
}]);
|
||||
mockIssueService.update.mockImplementation(async (_id: string, patch: Record<string, unknown>) => ({
|
||||
...issue,
|
||||
...patch,
|
||||
changes: { status: { from: "todo", to: "in_review" } },
|
||||
updatedAt: new Date(),
|
||||
}));
|
||||
|
||||
const res = await request(await createApp({
|
||||
type: "agent",
|
||||
agentId: "33333333-3333-4333-8333-333333333333",
|
||||
companyId: "company-1",
|
||||
runId: "55555555-5555-4555-8555-555555555555",
|
||||
}))
|
||||
.patch("/api/issues/aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa")
|
||||
.send({
|
||||
status: "in_review",
|
||||
reviewInteractionId: "11111111-1111-4111-8111-111111111111",
|
||||
});
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.body.reviewInteractionId).toBe("11111111-1111-4111-8111-111111111111");
|
||||
expect(mockLogActivity).toHaveBeenCalledWith(
|
||||
expect.anything(),
|
||||
expect.objectContaining({
|
||||
details: expect.objectContaining({
|
||||
reviewInteractionId: "11111111-1111-4111-8111-111111111111",
|
||||
}),
|
||||
}),
|
||||
expect.any(Array),
|
||||
);
|
||||
});
|
||||
|
||||
it("rejects a review binding to a confirmation created by another agent", async () => {
|
||||
const issue = {
|
||||
id: "aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa",
|
||||
companyId: "company-1",
|
||||
status: "todo",
|
||||
assigneeAgentId: "33333333-3333-4333-8333-333333333333",
|
||||
assigneeUserId: null,
|
||||
createdByUserId: "local-board",
|
||||
identifier: "PAP-1004",
|
||||
title: "Pending confirmation",
|
||||
executionPolicy: null,
|
||||
executionState: null,
|
||||
};
|
||||
mockIssueService.getById.mockResolvedValue(issue);
|
||||
mockIssueThreadInteractionService.listForIssue.mockResolvedValue([{
|
||||
id: "11111111-1111-4111-8111-111111111111",
|
||||
kind: "request_confirmation",
|
||||
status: "pending",
|
||||
createdByAgentId: "44444444-4444-4444-8444-444444444444",
|
||||
sourceRunId: "44444444-4444-4444-8444-444444444444",
|
||||
payload: { version: 1, prompt: "Approve another agent's request?" },
|
||||
}]);
|
||||
|
||||
const res = await request(await createApp({
|
||||
type: "agent",
|
||||
|
|
@ -593,7 +658,7 @@ describe("issue execution policy routes", () => {
|
|||
|
||||
expect(res.status).toBe(422);
|
||||
expect(res.body).toMatchObject({
|
||||
error: expect.stringContaining("created by this agent run"),
|
||||
error: expect.stringContaining("created by this agent"),
|
||||
details: { code: "invalid_review_interaction" },
|
||||
});
|
||||
expect(mockIssueService.update).not.toHaveBeenCalled();
|
||||
|
|
@ -1203,4 +1268,334 @@ describe("issue execution policy routes", () => {
|
|||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("persists reviewInteractionId when entering execution review", async () => {
|
||||
const policy = normalizeIssueExecutionPolicy({
|
||||
stages: [
|
||||
{
|
||||
id: "11111111-1111-4111-8111-111111111111",
|
||||
type: "review",
|
||||
participants: [{ type: "agent", agentId: "44444444-4444-4444-8444-444444444444" }],
|
||||
},
|
||||
],
|
||||
})!;
|
||||
const issue = {
|
||||
id: "aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa",
|
||||
companyId: "company-1",
|
||||
status: "in_progress",
|
||||
assigneeAgentId: "33333333-3333-4333-8333-333333333333",
|
||||
assigneeUserId: null,
|
||||
createdByUserId: "local-board",
|
||||
identifier: "PAP-1100",
|
||||
title: "Plan confirmation during execution review",
|
||||
executionPolicy: policy,
|
||||
executionState: null,
|
||||
};
|
||||
mockIssueService.getById.mockResolvedValue(issue);
|
||||
mockIssueThreadInteractionService.listForIssue.mockResolvedValue([{
|
||||
id: "11111111-1111-4111-8111-111111111111",
|
||||
kind: "request_confirmation",
|
||||
status: "pending",
|
||||
createdByAgentId: "33333333-3333-4333-8333-333333333333",
|
||||
sourceRunId: "55555555-5555-4555-8555-555555555555",
|
||||
payload: { version: 1, prompt: "Approve this plan?" },
|
||||
}]);
|
||||
mockIssueService.update.mockImplementation(async (_id: string, patch: Record<string, unknown>) => ({
|
||||
...issue,
|
||||
...patch,
|
||||
changes: { status: { from: "in_progress", to: "in_review" } },
|
||||
updatedAt: new Date(),
|
||||
}));
|
||||
|
||||
const res = await request(await createApp({
|
||||
type: "agent",
|
||||
agentId: "33333333-3333-4333-8333-333333333333",
|
||||
companyId: "company-1",
|
||||
runId: "55555555-5555-4555-8555-555555555555",
|
||||
}))
|
||||
.patch("/api/issues/aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa")
|
||||
.send({
|
||||
status: "in_review",
|
||||
reviewInteractionId: "11111111-1111-4111-8111-111111111111",
|
||||
});
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.body.reviewInteractionId).toBe("11111111-1111-4111-8111-111111111111");
|
||||
expect(mockLogActivity).toHaveBeenCalledWith(
|
||||
expect.anything(),
|
||||
expect.objectContaining({
|
||||
action: "issue.updated",
|
||||
details: expect.objectContaining({
|
||||
reviewInteractionId: "11111111-1111-4111-8111-111111111111",
|
||||
}),
|
||||
}),
|
||||
expect.any(Array),
|
||||
);
|
||||
});
|
||||
|
||||
it("persists a cross-run reviewInteractionId on an active execution review", async () => {
|
||||
const policy = normalizeIssueExecutionPolicy({
|
||||
stages: [
|
||||
{
|
||||
id: "11111111-1111-4111-8111-111111111111",
|
||||
type: "review",
|
||||
participants: [{ type: "agent", agentId: "44444444-4444-4444-8444-444444444444" }],
|
||||
},
|
||||
],
|
||||
})!;
|
||||
const issue = {
|
||||
id: "aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa",
|
||||
companyId: "company-1",
|
||||
status: "in_review",
|
||||
assigneeAgentId: "44444444-4444-4444-8444-444444444444",
|
||||
assigneeUserId: null,
|
||||
createdByUserId: "local-board",
|
||||
identifier: "PAP-1101",
|
||||
title: "Rebind confirmation",
|
||||
executionPolicy: policy,
|
||||
executionState: {
|
||||
status: "pending",
|
||||
currentStageId: "11111111-1111-4111-8111-111111111111",
|
||||
currentStageIndex: 0,
|
||||
currentStageType: "review",
|
||||
currentParticipant: { type: "agent", agentId: "44444444-4444-4444-8444-444444444444" },
|
||||
returnAssignee: { type: "agent", agentId: "33333333-3333-4333-8333-333333333333" },
|
||||
completedStageIds: [],
|
||||
lastDecisionId: null,
|
||||
lastDecisionOutcome: null,
|
||||
},
|
||||
};
|
||||
mockIssueService.getById.mockResolvedValue(issue);
|
||||
mockIssueThreadInteractionService.listForIssue.mockResolvedValue([{
|
||||
id: "11111111-1111-4111-8111-111111111111",
|
||||
kind: "request_confirmation",
|
||||
status: "pending",
|
||||
createdByAgentId: "33333333-3333-4333-8333-333333333333",
|
||||
sourceRunId: "44444444-4444-4444-8444-444444444444",
|
||||
payload: { version: 1, prompt: "Approve this plan?" },
|
||||
}]);
|
||||
mockIssueService.update.mockImplementation(async (_id: string, patch: Record<string, unknown>) => ({
|
||||
...issue,
|
||||
...patch,
|
||||
changes: {},
|
||||
updatedAt: new Date(),
|
||||
}));
|
||||
|
||||
const res = await request(await createApp({
|
||||
type: "agent",
|
||||
agentId: "33333333-3333-4333-8333-333333333333",
|
||||
companyId: "company-1",
|
||||
runId: "55555555-5555-4555-8555-555555555555",
|
||||
}))
|
||||
.patch("/api/issues/aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa")
|
||||
.send({
|
||||
reviewInteractionId: "11111111-1111-4111-8111-111111111111",
|
||||
});
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.body.reviewInteractionId).toBe("11111111-1111-4111-8111-111111111111");
|
||||
expect(res.body.executionState.reviewRequest).toEqual({
|
||||
id: "11111111-1111-4111-8111-111111111111",
|
||||
});
|
||||
expect(mockIssueService.update).toHaveBeenCalledWith(
|
||||
"aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa",
|
||||
expect.objectContaining({
|
||||
executionState: expect.objectContaining({
|
||||
reviewRequest: {
|
||||
id: "11111111-1111-4111-8111-111111111111",
|
||||
},
|
||||
}),
|
||||
}),
|
||||
expect.anything(),
|
||||
);
|
||||
expect(mockLogActivity).toHaveBeenCalledWith(
|
||||
expect.anything(),
|
||||
expect.objectContaining({
|
||||
action: "issue.updated",
|
||||
details: expect.objectContaining({
|
||||
reviewInteractionId: "11111111-1111-4111-8111-111111111111",
|
||||
}),
|
||||
}),
|
||||
expect.any(Array),
|
||||
);
|
||||
});
|
||||
|
||||
it("rejects execution review completion while the designated confirmation is pending", async () => {
|
||||
const policy = normalizeIssueExecutionPolicy({
|
||||
stages: [
|
||||
{
|
||||
id: "11111111-1111-4111-8111-111111111111",
|
||||
type: "review",
|
||||
participants: [{ type: "agent", agentId: "44444444-4444-4444-8444-444444444444" }],
|
||||
},
|
||||
],
|
||||
})!;
|
||||
const issue = {
|
||||
id: "aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa",
|
||||
companyId: "company-1",
|
||||
status: "in_review",
|
||||
assigneeAgentId: "44444444-4444-4444-8444-444444444444",
|
||||
assigneeUserId: null,
|
||||
createdByUserId: "local-board",
|
||||
identifier: "PAP-1102",
|
||||
title: "Pending plan confirmation",
|
||||
executionPolicy: policy,
|
||||
executionState: {
|
||||
status: "pending",
|
||||
currentStageId: "11111111-1111-4111-8111-111111111111",
|
||||
currentStageIndex: 0,
|
||||
currentStageType: "review",
|
||||
currentParticipant: { type: "agent", agentId: "44444444-4444-4444-8444-444444444444" },
|
||||
returnAssignee: { type: "agent", agentId: "33333333-3333-4333-8333-333333333333" },
|
||||
completedStageIds: [],
|
||||
lastDecisionId: null,
|
||||
lastDecisionOutcome: null,
|
||||
},
|
||||
};
|
||||
mockIssueService.getById.mockResolvedValue(issue);
|
||||
mockIssueThreadInteractionService.listForIssue.mockResolvedValue([{
|
||||
id: "11111111-1111-4111-8111-111111111111",
|
||||
kind: "request_confirmation",
|
||||
status: "pending",
|
||||
createdByAgentId: "33333333-3333-4333-8333-333333333333",
|
||||
sourceRunId: "55555555-5555-4555-8555-555555555555",
|
||||
payload: { version: 1, prompt: "Approve this plan?" },
|
||||
}]);
|
||||
mockResolveIssueReviewRequester.mockResolvedValue({
|
||||
type: "agent",
|
||||
id: "33333333-3333-4333-8333-333333333333",
|
||||
reviewInteractionId: "11111111-1111-4111-8111-111111111111",
|
||||
});
|
||||
const reviewerRun = {
|
||||
id: "66666666-6666-4666-8666-666666666666",
|
||||
companyId: "company-1",
|
||||
agentId: "44444444-4444-4444-8444-444444444444",
|
||||
contextSnapshot: { issueId: "aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa" },
|
||||
permissions: null,
|
||||
};
|
||||
mockDbSelectWhere.mockImplementation(() => ({
|
||||
for: () => ({
|
||||
then: (onFulfilled: (rows: unknown[]) => unknown, onRejected?: (reason: unknown) => unknown) =>
|
||||
Promise.resolve([reviewerRun]).then(onFulfilled, onRejected),
|
||||
}),
|
||||
then: (onFulfilled: (rows: unknown[]) => unknown, onRejected?: (reason: unknown) => unknown) =>
|
||||
Promise.resolve([reviewerRun]).then(onFulfilled, onRejected),
|
||||
}));
|
||||
|
||||
const res = await request(await createApp({
|
||||
type: "agent",
|
||||
agentId: "44444444-4444-4444-8444-444444444444",
|
||||
companyId: "company-1",
|
||||
runId: "66666666-6666-4666-8666-666666666666",
|
||||
}))
|
||||
.patch("/api/issues/aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa")
|
||||
.send({
|
||||
status: "done",
|
||||
comment: "Approved: looks good.",
|
||||
});
|
||||
|
||||
expect(res.status).toBe(422);
|
||||
expect(res.body).toMatchObject({
|
||||
details: { code: "pending_review_confirmation" },
|
||||
});
|
||||
expect(mockIssueService.update).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("rejects a caller-supplied confirmation that differs from the persisted review binding", async () => {
|
||||
const policy = normalizeIssueExecutionPolicy({
|
||||
stages: [
|
||||
{
|
||||
id: "11111111-1111-4111-8111-111111111111",
|
||||
type: "review",
|
||||
participants: [{ type: "agent", agentId: "44444444-4444-4444-8444-444444444444" }],
|
||||
},
|
||||
],
|
||||
})!;
|
||||
const issue = {
|
||||
id: "aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa",
|
||||
companyId: "company-1",
|
||||
status: "in_review",
|
||||
assigneeAgentId: "44444444-4444-4444-8444-444444444444",
|
||||
assigneeUserId: null,
|
||||
createdByUserId: "local-board",
|
||||
identifier: "PAP-1103",
|
||||
title: "Persisted plan confirmation",
|
||||
executionPolicy: policy,
|
||||
executionState: {
|
||||
status: "pending",
|
||||
currentStageId: "11111111-1111-4111-8111-111111111111",
|
||||
currentStageIndex: 0,
|
||||
currentStageType: "review",
|
||||
currentParticipant: { type: "agent", agentId: "44444444-4444-4444-8444-444444444444" },
|
||||
returnAssignee: { type: "agent", agentId: "33333333-3333-4333-8333-333333333333" },
|
||||
completedStageIds: [],
|
||||
lastDecisionId: null,
|
||||
lastDecisionOutcome: null,
|
||||
},
|
||||
};
|
||||
const persistedInteractionId = "77777777-7777-4777-8777-777777777777";
|
||||
const suppliedInteractionId = "88888888-8888-4888-8888-888888888888";
|
||||
mockIssueService.getById.mockResolvedValue(issue);
|
||||
mockIssueThreadInteractionService.listForIssue.mockResolvedValue([
|
||||
{
|
||||
id: persistedInteractionId,
|
||||
kind: "request_confirmation",
|
||||
status: "pending",
|
||||
createdByAgentId: "33333333-3333-4333-8333-333333333333",
|
||||
sourceRunId: "55555555-5555-4555-8555-555555555555",
|
||||
payload: { version: 1, prompt: "Approve the persisted plan?" },
|
||||
},
|
||||
{
|
||||
id: suppliedInteractionId,
|
||||
kind: "request_confirmation",
|
||||
status: "pending",
|
||||
createdByAgentId: "44444444-4444-4444-8444-444444444444",
|
||||
sourceRunId: "66666666-6666-4666-8666-666666666666",
|
||||
payload: { version: 1, prompt: "Approve a replacement?" },
|
||||
},
|
||||
]);
|
||||
mockResolveIssueReviewRequester.mockResolvedValue({
|
||||
type: "agent",
|
||||
id: "33333333-3333-4333-8333-333333333333",
|
||||
reviewInteractionId: persistedInteractionId,
|
||||
});
|
||||
const reviewerRun = {
|
||||
id: "66666666-6666-4666-8666-666666666666",
|
||||
companyId: "company-1",
|
||||
agentId: "44444444-4444-4444-8444-444444444444",
|
||||
contextSnapshot: { issueId: "aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa" },
|
||||
permissions: null,
|
||||
};
|
||||
mockDbSelectWhere.mockImplementation(() => ({
|
||||
for: () => ({
|
||||
then: (onFulfilled: (rows: unknown[]) => unknown, onRejected?: (reason: unknown) => unknown) =>
|
||||
Promise.resolve([reviewerRun]).then(onFulfilled, onRejected),
|
||||
}),
|
||||
then: (onFulfilled: (rows: unknown[]) => unknown, onRejected?: (reason: unknown) => unknown) =>
|
||||
Promise.resolve([reviewerRun]).then(onFulfilled, onRejected),
|
||||
}));
|
||||
|
||||
const res = await request(await createApp({
|
||||
type: "agent",
|
||||
agentId: "44444444-4444-4444-8444-444444444444",
|
||||
companyId: "company-1",
|
||||
runId: "66666666-6666-4666-8666-666666666666",
|
||||
}))
|
||||
.patch("/api/issues/aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa")
|
||||
.send({
|
||||
status: "done",
|
||||
comment: "Approved: looks good.",
|
||||
reviewInteractionId: suppliedInteractionId,
|
||||
});
|
||||
|
||||
expect(res.status).toBe(422);
|
||||
expect(res.body).toMatchObject({
|
||||
details: {
|
||||
code: "review_interaction_binding_mismatch",
|
||||
reviewInteractionId: suppliedInteractionId,
|
||||
persistedReviewInteractionId: persistedInteractionId,
|
||||
},
|
||||
});
|
||||
expect(mockIssueService.update).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -159,6 +159,24 @@ describe("parseIssueExecutionState", () => {
|
|||
expect(state).not.toBeNull();
|
||||
expect(state!.status).toBe("pending");
|
||||
});
|
||||
|
||||
it("parses a server-owned confirmation binding without review instructions", () => {
|
||||
const interactionId = "bbbbbbbb-bbbb-4bbb-8bbb-bbbbbbbbbbbb";
|
||||
const state = parseIssueExecutionState({
|
||||
status: "pending",
|
||||
currentStageId: "aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa",
|
||||
currentStageIndex: 0,
|
||||
currentStageType: "review",
|
||||
currentParticipant: { type: "agent", agentId: qaAgentId },
|
||||
returnAssignee: { type: "agent", agentId: coderAgentId },
|
||||
reviewRequest: { id: interactionId },
|
||||
completedStageIds: [],
|
||||
lastDecisionId: null,
|
||||
lastDecisionOutcome: null,
|
||||
});
|
||||
|
||||
expect(state?.reviewRequest).toEqual({ id: interactionId });
|
||||
});
|
||||
});
|
||||
|
||||
describe("issue execution policy transitions", () => {
|
||||
|
|
|
|||
|
|
@ -2620,6 +2620,32 @@ describeEmbeddedPostgres("issueThreadInteractionService", () => {
|
|||
agentId,
|
||||
});
|
||||
|
||||
await db
|
||||
.update(issues)
|
||||
.set({
|
||||
executionState: {
|
||||
status: "pending",
|
||||
currentStageId: randomUUID(),
|
||||
currentStageIndex: 0,
|
||||
currentStageType: "review",
|
||||
currentParticipant: {
|
||||
type: "user",
|
||||
agentId: null,
|
||||
userId: "local-board",
|
||||
},
|
||||
returnAssignee: {
|
||||
type: "agent",
|
||||
agentId,
|
||||
userId: null,
|
||||
},
|
||||
reviewRequest: { id: created.id },
|
||||
completedStageIds: [],
|
||||
lastDecisionId: null,
|
||||
lastDecisionOutcome: null,
|
||||
},
|
||||
})
|
||||
.where(eq(issues.id, issueId));
|
||||
|
||||
const accepted = await interactionsSvc.acceptInteraction({
|
||||
id: issueId,
|
||||
companyId,
|
||||
|
|
@ -2642,6 +2668,14 @@ describeEmbeddedPostgres("issueThreadInteractionService", () => {
|
|||
status: "todo",
|
||||
assigneeAgentId: agentId,
|
||||
assigneeUserId: null,
|
||||
executionState: {
|
||||
returnAssignee: {
|
||||
type: "agent",
|
||||
agentId,
|
||||
userId: null,
|
||||
},
|
||||
reviewRequest: { id: created.id },
|
||||
},
|
||||
});
|
||||
|
||||
await db
|
||||
|
|
|
|||
|
|
@ -4497,7 +4497,6 @@ export function issueRoutes(
|
|||
actorType: "agent" | "user";
|
||||
actorId: string;
|
||||
actorAgentId?: string | null;
|
||||
actorRunId?: string | null;
|
||||
reviewInteractionId?: string;
|
||||
}) {
|
||||
const nextStatus = typeof input.updateFields.status === "string"
|
||||
|
|
@ -4506,7 +4505,18 @@ export function issueRoutes(
|
|||
// Conversations wait for the next message; successful run finalization owns
|
||||
// the waiting state. They do not need an execution-task review assignment.
|
||||
if (isConversation(input.existing) && !input.reviewInteractionId) return null;
|
||||
if (input.existing.status === "in_review" || nextStatus !== "in_review") return null;
|
||||
const stayingOrEnteringReview =
|
||||
nextStatus === "in_review" || input.existing.status === "in_review";
|
||||
if (!stayingOrEnteringReview) return null;
|
||||
// An explicit confirmation binding must persist even when the issue is
|
||||
// already in_review (execution-policy repair, follow-up PATCH). The
|
||||
// auto-detected review path still only applies when entering in_review.
|
||||
if (
|
||||
!input.reviewInteractionId &&
|
||||
(input.existing.status === "in_review" || nextStatus !== "in_review")
|
||||
) {
|
||||
return null;
|
||||
}
|
||||
if (input.actorType !== "agent" && !input.reviewInteractionId) return null;
|
||||
|
||||
const interactions = await issueThreadInteractionService(db).listForIssue(
|
||||
|
|
@ -4522,8 +4532,7 @@ export function issueRoutes(
|
|||
(interaction.kind === "request_confirmation" ||
|
||||
interaction.kind === "request_checkbox_confirmation") &&
|
||||
(input.actorType === "agent"
|
||||
? interaction.createdByAgentId === input.actorAgentId &&
|
||||
interaction.sourceRunId === input.actorRunId
|
||||
? interaction.createdByAgentId === input.actorAgentId
|
||||
: interaction.createdByUserId === input.actorId) &&
|
||||
!(
|
||||
interaction.kind === "request_confirmation" &&
|
||||
|
|
@ -4537,7 +4546,7 @@ export function issueRoutes(
|
|||
);
|
||||
if (!designatedReviewConfirmation) {
|
||||
const creatorDescription =
|
||||
input.actorType === "agent" ? "this agent run" : "this user";
|
||||
input.actorType === "agent" ? "this agent" : "this user";
|
||||
throw unprocessable(
|
||||
`reviewInteractionId must identify a pending non-tool confirmation created by ${creatorDescription}`,
|
||||
{
|
||||
|
|
@ -5104,6 +5113,48 @@ export function issueRoutes(
|
|||
return null;
|
||||
}
|
||||
|
||||
async function assertNoPendingDesignatedReviewConfirmation(input: {
|
||||
issue: Parameters<typeof resolveIssueReviewRequester>[1];
|
||||
requestedReviewInteractionId?: string;
|
||||
}) {
|
||||
const persistedReviewInteractionId = (
|
||||
await resolveIssueReviewRequester(db, input.issue)
|
||||
)?.reviewInteractionId;
|
||||
if (
|
||||
input.requestedReviewInteractionId &&
|
||||
input.requestedReviewInteractionId !== persistedReviewInteractionId
|
||||
) {
|
||||
throw unprocessable(
|
||||
"reviewInteractionId must match the persisted review confirmation binding",
|
||||
{
|
||||
code: "review_interaction_binding_mismatch",
|
||||
reviewInteractionId: input.requestedReviewInteractionId,
|
||||
persistedReviewInteractionId: persistedReviewInteractionId ?? null,
|
||||
},
|
||||
);
|
||||
}
|
||||
if (!persistedReviewInteractionId) return;
|
||||
|
||||
const pendingBoundConfirmation = (
|
||||
await issueThreadInteractionService(db).listForIssue(input.issue.id)
|
||||
).find(
|
||||
(interaction) =>
|
||||
interaction.id === persistedReviewInteractionId &&
|
||||
interaction.status === "pending" &&
|
||||
(interaction.kind === "request_confirmation" ||
|
||||
interaction.kind === "request_checkbox_confirmation"),
|
||||
);
|
||||
if (!pendingBoundConfirmation) return;
|
||||
|
||||
throw unprocessable(
|
||||
"Cannot complete execution review while the designated confirmation is still pending",
|
||||
{
|
||||
code: "pending_review_confirmation",
|
||||
reviewInteractionId: persistedReviewInteractionId,
|
||||
},
|
||||
);
|
||||
}
|
||||
|
||||
async function hasActiveCheckoutManagementOverride(
|
||||
actorAgentId: string,
|
||||
companyId: string,
|
||||
|
|
@ -8775,8 +8826,13 @@ export function issueRoutes(
|
|||
"Server-Timing",
|
||||
`paperclip_issue;dur=${(performance.now() - requestStartedAt).toFixed(1)}`,
|
||||
);
|
||||
const reviewRequester =
|
||||
issue.status === "in_review"
|
||||
? await resolveIssueReviewRequester(db, issue)
|
||||
: null;
|
||||
res.json({
|
||||
...issue,
|
||||
reviewInteractionId: reviewRequester?.reviewInteractionId ?? null,
|
||||
...inboxArchiveFields,
|
||||
goalId: goal?.id ?? issue.goalId,
|
||||
ancestors,
|
||||
|
|
@ -9242,7 +9298,6 @@ export function issueRoutes(
|
|||
actorType: actor.actorType,
|
||||
actorId: actor.actorId,
|
||||
actorAgentId: actor.agentId,
|
||||
actorRunId: actor.runId,
|
||||
});
|
||||
const executionPolicy = normalizeIssueExecutionPolicy(
|
||||
lockedIssue.executionPolicy ?? null,
|
||||
|
|
@ -13184,9 +13239,28 @@ export function issueRoutes(
|
|||
actorType: actor.actorType,
|
||||
actorId: actor.actorId,
|
||||
actorAgentId: actor.agentId,
|
||||
actorRunId: actor.runId,
|
||||
reviewInteractionId: requestedReviewInteractionId,
|
||||
});
|
||||
if (reviewInteractionId) {
|
||||
const executionState = parseIssueExecutionState(
|
||||
updateFields.executionState ?? existing.executionState,
|
||||
);
|
||||
if (executionState?.status === "pending") {
|
||||
updateFields.executionState = {
|
||||
...executionState,
|
||||
reviewRequest: {
|
||||
...(executionState.reviewRequest ?? {}),
|
||||
id: reviewInteractionId,
|
||||
},
|
||||
};
|
||||
}
|
||||
}
|
||||
if (transition.decision) {
|
||||
await assertNoPendingDesignatedReviewConfirmation({
|
||||
issue: existing,
|
||||
requestedReviewInteractionId,
|
||||
});
|
||||
}
|
||||
const enteringReviewRequested =
|
||||
existing.status !== "in_review" && updateFields.status === "in_review";
|
||||
const persistReviewActivityTransactionally =
|
||||
|
|
@ -13687,7 +13761,16 @@ export function issueRoutes(
|
|||
ReturnType<typeof issueReferencesSvc.listIssueReferenceSummary>
|
||||
>;
|
||||
referencedIssueIdentifiers?: string[];
|
||||
} = issue;
|
||||
reviewInteractionId?: string | null;
|
||||
} = {
|
||||
...issue,
|
||||
reviewInteractionId:
|
||||
reviewInteractionId ??
|
||||
(issue.status === "in_review"
|
||||
? (await resolveIssueReviewRequester(db, issue))?.reviewInteractionId ??
|
||||
null
|
||||
: null),
|
||||
};
|
||||
let updatedRelations: Awaited<
|
||||
ReturnType<typeof svc.getRelationSummaries>
|
||||
> | null = null;
|
||||
|
|
@ -17401,6 +17484,9 @@ export function issueRoutes(
|
|||
let comment: Awaited<ReturnType<typeof svc.addComment>>;
|
||||
let goalCommentSteered = false;
|
||||
if (shouldAutoApproveReviewComment) {
|
||||
await assertNoPendingDesignatedReviewConfirmation({
|
||||
issue: currentIssue,
|
||||
});
|
||||
const transition = applyIssueExecutionPolicyTransition({
|
||||
issue: currentIssue,
|
||||
policy: currentExecutionPolicy,
|
||||
|
|
|
|||
|
|
@ -48,6 +48,10 @@ export async function resolveIssueReviewRequester(
|
|||
AND ${activityLog.details} -> 'changes' -> 'status' ->> 'from' IS NOT NULL
|
||||
AND ${activityLog.details} -> 'changes' -> 'status' ->> 'from' <> 'in_review'
|
||||
)
|
||||
OR
|
||||
(
|
||||
${activityLog.details} ->> 'reviewInteractionId' IS NOT NULL
|
||||
)
|
||||
)`,
|
||||
))
|
||||
.orderBy(desc(activityLog.createdAt), desc(activityLog.id))
|
||||
|
|
|
|||
|
|
@ -396,3 +396,57 @@ describe("issueThreadInteractionService", () => {
|
|||
expect(state.toolActionRequestUpdates[0]).toMatchObject({ status: "expired", resolvedByUserId: "local-board" });
|
||||
});
|
||||
});
|
||||
|
||||
describe("shouldReturnAcceptedConfirmationToCreatorAgent", () => {
|
||||
it("returns an execution-review issue to the confirmation creator after accept", async () => {
|
||||
const { shouldReturnAcceptedConfirmationToCreatorAgent } = await import("./issue-thread-interactions.js");
|
||||
expect(shouldReturnAcceptedConfirmationToCreatorAgent({
|
||||
issue: {
|
||||
id: "issue-1",
|
||||
companyId: "company-1",
|
||||
status: "in_review",
|
||||
workMode: "standard",
|
||||
assigneeAgentId: "reviewer-agent",
|
||||
assigneeUserId: null,
|
||||
reviewPolicy: null,
|
||||
createdByAgentId: "creator-agent",
|
||||
createdByUserId: null,
|
||||
executionState: {
|
||||
status: "pending",
|
||||
returnAssignee: { type: "agent", agentId: "creator-agent" },
|
||||
},
|
||||
},
|
||||
current: {
|
||||
kind: "request_confirmation",
|
||||
createdByAgentId: "creator-agent",
|
||||
} as never,
|
||||
actor: { userId: "local-board" },
|
||||
})).toBe(true);
|
||||
});
|
||||
|
||||
it("does not return to the creator when a human still holds the in_review assignment", async () => {
|
||||
const { shouldReturnAcceptedConfirmationToCreatorAgent } = await import("./issue-thread-interactions.js");
|
||||
expect(shouldReturnAcceptedConfirmationToCreatorAgent({
|
||||
issue: {
|
||||
id: "issue-1",
|
||||
companyId: "company-1",
|
||||
status: "in_review",
|
||||
workMode: "standard",
|
||||
assigneeAgentId: "reviewer-agent",
|
||||
assigneeUserId: null,
|
||||
reviewPolicy: null,
|
||||
createdByAgentId: "creator-agent",
|
||||
createdByUserId: null,
|
||||
executionState: {
|
||||
status: "pending",
|
||||
returnAssignee: { type: "agent", agentId: "other-agent" },
|
||||
},
|
||||
},
|
||||
current: {
|
||||
kind: "request_confirmation",
|
||||
createdByAgentId: "creator-agent",
|
||||
} as never,
|
||||
actor: { userId: "local-board" },
|
||||
})).toBe(false);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -464,6 +464,7 @@ type IssueResolutionContext = {
|
|||
reviewPolicy: IssueReviewPolicy | null;
|
||||
createdByAgentId: string | null;
|
||||
createdByUserId: string | null;
|
||||
executionState?: unknown;
|
||||
};
|
||||
|
||||
async function assertRequestConfirmationResolutionAllowedUnderLock(
|
||||
|
|
@ -794,22 +795,35 @@ function interactionTerminalError(row: { status: string; result?: unknown }) {
|
|||
);
|
||||
}
|
||||
|
||||
function shouldReturnAcceptedConfirmationToCreatorAgent(args: {
|
||||
function executionReturnAssigneeAgentId(issue: IssueResolutionContext): string | null {
|
||||
const state = issue.executionState;
|
||||
if (!state || typeof state !== "object" || Array.isArray(state)) return null;
|
||||
const returnAssignee = (state as { returnAssignee?: { agentId?: unknown } }).returnAssignee;
|
||||
return typeof returnAssignee?.agentId === "string" ? returnAssignee.agentId : null;
|
||||
}
|
||||
|
||||
export function shouldReturnAcceptedConfirmationToCreatorAgent(args: {
|
||||
issue: IssueResolutionContext;
|
||||
current: IssueThreadInteractionRow;
|
||||
actor: InteractionActor;
|
||||
}) {
|
||||
if (!isRequestConfirmationLikeKind(args.current.kind)) return false;
|
||||
if (!args.current.createdByAgentId) return false;
|
||||
if (!args.actor.userId) return false;
|
||||
if (isTerminalIssueStatus(args.issue.status)) return false;
|
||||
const returnAssigneeAgentId = executionReturnAssigneeAgentId(args.issue);
|
||||
const creatorIsReturnAssignee =
|
||||
returnAssigneeAgentId === args.current.createdByAgentId;
|
||||
if (args.issue.assigneeAgentId) {
|
||||
return (
|
||||
args.issue.status === "in_review" &&
|
||||
args.issue.assigneeAgentId === args.current.createdByAgentId
|
||||
);
|
||||
if (args.issue.status !== "in_review") return false;
|
||||
if (args.issue.assigneeAgentId === args.current.createdByAgentId) {
|
||||
return Boolean(args.actor.userId);
|
||||
}
|
||||
// Execution review reassigned the issue away from the confirmation
|
||||
// creator. Accepting the card must wake that original executor, not
|
||||
// the current execution reviewer.
|
||||
return creatorIsReturnAssignee;
|
||||
}
|
||||
return Boolean(args.issue.assigneeUserId);
|
||||
return Boolean(args.issue.assigneeUserId) && Boolean(args.actor.userId);
|
||||
}
|
||||
|
||||
function shouldSupersedeInteractionOnUserComment(interaction: UserCommentSupersedableInteraction) {
|
||||
|
|
@ -2109,6 +2123,7 @@ export function issueThreadInteractionService(
|
|||
reviewPolicy: issues.reviewPolicy,
|
||||
createdByAgentId: issues.createdByAgentId,
|
||||
createdByUserId: issues.createdByUserId,
|
||||
executionState: issues.executionState,
|
||||
})
|
||||
.from(issues)
|
||||
.where(eq(issues.id, args.issue.id))
|
||||
|
|
@ -2381,6 +2396,7 @@ export function issueThreadInteractionService(
|
|||
reviewPolicy: issues.reviewPolicy,
|
||||
createdByAgentId: issues.createdByAgentId,
|
||||
createdByUserId: issues.createdByUserId,
|
||||
executionState: issues.executionState,
|
||||
})
|
||||
.from(issues)
|
||||
.where(eq(issues.id, args.issue.id))
|
||||
|
|
|
|||
Loading…
Reference in New Issue