feat(server): allow agents to resolve review confirmations (#10939)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Issue reviews use thread confirmations to record explicit verdicts > - The server allowed users to resolve review confirmations but rejected all agent actors > - This one-way rule prevented an eligible agent reviewer from completing a review > - The existing review policy already defines which actor can submit a verdict > - This pull request applies that policy to agent confirmation verdicts on writable issues > - The benefit is a consistent review gate for users and agents with preserved audit attribution ## Linked Issues or Issue Description - Builds on: #10931 (merged into master before this PR) - Refs #8617 ## What Changed - Allow eligible agents to accept or reject pending review confirmations on issues they can write. - Allow a creator agent to withdraw its own pending review confirmation when the review policy permits it. - Reuse the review verdict policy check for users and agents. - Require an explicit, same-run review-confirmation binding so unrelated board-only confirmations stay protected. - Preserve board-only tool action confirmations and existing user attribution. - Add route and service tests for agent accept, reject, withdrawal, human-only denial, and user attribution. ## Verification - `pnpm exec vitest run packages/shared/src/validators/issue.test.ts server/src/__tests__/issue-execution-policy-routes.test.ts server/src/__tests__/issue-review-policy.test.ts server/src/__tests__/issue-thread-interaction-routes.test.ts server/src/__tests__/issue-thread-interactions-service.test.ts server/src/__tests__/issue-stalled-review-decision-routes.test.ts` (256 passed after rebasing onto master and the atomic binding fix) - `pnpm --filter @paperclipai/shared typecheck` - `pnpm --filter @paperclipai/server typecheck` - `pnpm -r typecheck` - `env -u AWS_ACCESS_KEY_ID -u AWS_SECRET_ACCESS_KEY pnpm test:run` - `pnpm build` ## Risks - The change expands who can resolve pending review confirmations. The existing issue write checks and review policy limit this access. - Tool action confirmations remain board-only. - The pull request depends on the review policy helper from #10931, which is now merged into master. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. The roadmap marks Agent Reviews and Approvals as shipped. This pull request fixes a narrow server behavior gap in that shipped capability. ## Model Used - OpenAI Codex, model `gpt-5.6-sol`, with reasoning, tool use, and code execution. The runtime does not expose the context window size. ## 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 not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [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 --------- Co-authored-by: Paperclip <noreply@paperclip.ing>
This commit is contained in:
Dotta
GitHub
parent
f554d67377
commit
814cb33676
|
|
@ -57,6 +57,13 @@ describe("issue validators", () => {
|
|||
expect(updateIssueSchema.safeParse({ reviewPolicy: "creator_only" }).success).toBe(false);
|
||||
});
|
||||
|
||||
it("accepts only UUID review interaction bindings on update", () => {
|
||||
expect(updateIssueSchema.parse({
|
||||
reviewInteractionId: "11111111-1111-4111-8111-111111111111",
|
||||
}).reviewInteractionId).toBe("11111111-1111-4111-8111-111111111111");
|
||||
expect(updateIssueSchema.safeParse({ reviewInteractionId: "interaction-1" }).success).toBe(false);
|
||||
});
|
||||
|
||||
it("normalizes JSON-escaped line breaks in issue descriptions", () => {
|
||||
const parsed = createIssueSchema.parse({
|
||||
title: "Follow up PR",
|
||||
|
|
|
|||
|
|
@ -545,6 +545,7 @@ export const updateIssueSchema = createIssueBaseSchema.omit({
|
|||
assigneeAgentId: z.string().trim().min(1).optional().nullable(),
|
||||
comment: multilineTextSchema.pipe(z.string().min(1)).optional(),
|
||||
onBehalfOfUserId: z.string().trim().min(1).optional().nullable(),
|
||||
reviewInteractionId: z.string().uuid().optional(),
|
||||
reviewRequest: issueReviewRequestSchema.optional().nullable(),
|
||||
reopen: z.boolean().optional(),
|
||||
resume: z.boolean().optional(),
|
||||
|
|
|
|||
|
|
@ -14,6 +14,7 @@ import {
|
|||
import {
|
||||
logActivity,
|
||||
resolveResponsibleUserIdForActivity,
|
||||
type ActivityPublication,
|
||||
type LogActivityInput,
|
||||
} from "../services/activity-log.js";
|
||||
import {
|
||||
|
|
@ -214,6 +215,7 @@ describeEmbeddedPostgres("logActivity responsible-user stamping", () => {
|
|||
responsibleUserId: "key-user",
|
||||
});
|
||||
|
||||
const postCommitPublications: ActivityPublication[] = [];
|
||||
await logActivity(db, activityInput({
|
||||
companyId,
|
||||
actorId: agentId,
|
||||
|
|
@ -221,7 +223,17 @@ describeEmbeddedPostgres("logActivity responsible-user stamping", () => {
|
|||
entityType: "agent",
|
||||
entityId: agentId,
|
||||
agentApiKeyId,
|
||||
}));
|
||||
}), postCommitPublications);
|
||||
|
||||
expect(postCommitPublications).toHaveLength(1);
|
||||
expect(postCommitPublications[0]).toMatchObject({
|
||||
companyId,
|
||||
payload: {
|
||||
action: "issue.updated",
|
||||
entityType: "agent",
|
||||
entityId: agentId,
|
||||
},
|
||||
});
|
||||
|
||||
const row = await db
|
||||
.select({ responsibleUserId: activityLog.responsibleUserId })
|
||||
|
|
|
|||
|
|
@ -74,6 +74,7 @@ const mockIssueThreadInteractionService = vi.hoisted(() => ({
|
|||
expirePendingInteractionsForTerminalIssue: vi.fn(async () => []),
|
||||
expireRequestConfirmationsSupersededByComment: vi.fn(async () => []),
|
||||
expireStaleRequestConfirmationsForIssueDocument: vi.fn(async () => []),
|
||||
listForIssue: vi.fn(async () => []),
|
||||
}));
|
||||
const mockIssueRecoveryActionService = vi.hoisted(() => ({
|
||||
getActiveForIssue: vi.fn(async () => null),
|
||||
|
|
@ -288,6 +289,7 @@ describe.sequential("issue comment reopen routes", () => {
|
|||
mockInstanceSettingsService.get.mockReset();
|
||||
mockInstanceSettingsService.listCompanyIds.mockReset();
|
||||
mockRoutineService.syncRunStatusForIssue.mockReset();
|
||||
mockIssueThreadInteractionService.listForIssue.mockReset();
|
||||
mockIssueRecoveryActionService.getActiveForIssue.mockReset();
|
||||
mockIssueTreeControlService.getActivePauseHoldGate.mockReset();
|
||||
mockExternalObjectService.syncCommentSafely.mockReset();
|
||||
|
|
@ -352,6 +354,7 @@ describe.sequential("issue comment reopen routes", () => {
|
|||
});
|
||||
mockInstanceSettingsService.listCompanyIds.mockResolvedValue(["company-1"]);
|
||||
mockRoutineService.syncRunStatusForIssue.mockResolvedValue(undefined);
|
||||
mockIssueThreadInteractionService.listForIssue.mockResolvedValue([]);
|
||||
mockIssueRecoveryActionService.getActiveForIssue.mockResolvedValue(null);
|
||||
mockIssueTreeControlService.getActivePauseHoldGate.mockResolvedValue(null);
|
||||
mockIssueService.addComment.mockResolvedValue({
|
||||
|
|
|
|||
|
|
@ -299,7 +299,13 @@ describe("issue execution policy routes", () => {
|
|||
};
|
||||
mockIssueService.getById.mockResolvedValue(issue);
|
||||
mockIssueThreadInteractionService.listForIssue.mockResolvedValue([
|
||||
{ id: "interaction-1", kind: "request_confirmation", status: "pending" },
|
||||
{
|
||||
id: "11111111-1111-4111-8111-111111111111",
|
||||
kind: "request_confirmation",
|
||||
status: "pending",
|
||||
createdByAgentId: "33333333-3333-4333-8333-333333333333",
|
||||
sourceRunId: "55555555-5555-4555-8555-555555555555",
|
||||
},
|
||||
]);
|
||||
mockIssueService.update.mockImplementation(async (_id: string, patch: Record<string, unknown>) => ({
|
||||
...issue,
|
||||
|
|
@ -321,6 +327,163 @@ describe("issue execution policy routes", () => {
|
|||
"aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa",
|
||||
expect.objectContaining({ status: "in_review" }),
|
||||
);
|
||||
expect(mockLogActivity).toHaveBeenCalledWith(
|
||||
expect.anything(),
|
||||
expect.objectContaining({
|
||||
action: "issue.updated",
|
||||
details: expect.not.objectContaining({ reviewInteractionId: expect.anything() }),
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("binds an explicitly designated same-run confirmation to the review transition", 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: "33333333-3333-4333-8333-333333333333",
|
||||
sourceRunId: "55555555-5555-4555-8555-555555555555",
|
||||
payload: { version: 1, prompt: "Approve this review?" },
|
||||
}]);
|
||||
mockIssueService.update.mockImplementation(async (_id: string, patch: Record<string, unknown>) => ({
|
||||
...issue,
|
||||
...patch,
|
||||
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(mockIssueService.update).toHaveBeenCalledWith(
|
||||
"aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa",
|
||||
expect.not.objectContaining({ reviewInteractionId: expect.anything() }),
|
||||
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("keeps a review transition and its confirmation binding in one rollback boundary", 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: "33333333-3333-4333-8333-333333333333",
|
||||
sourceRunId: "55555555-5555-4555-8555-555555555555",
|
||||
payload: { version: 1, prompt: "Approve this review?" },
|
||||
}]);
|
||||
mockIssueService.update.mockImplementation(async (_id: string, patch: Record<string, unknown>) => ({
|
||||
...issue,
|
||||
...patch,
|
||||
changes: { status: { from: "todo", to: "in_review" } },
|
||||
updatedAt: new Date(),
|
||||
}));
|
||||
mockLogActivity.mockRejectedValueOnce(new Error("activity insert failed"));
|
||||
|
||||
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(500);
|
||||
expect(mockDb.transaction).toHaveBeenCalled();
|
||||
const updateTx = mockIssueService.update.mock.calls[0]?.[2];
|
||||
const activityTx = mockLogActivity.mock.calls[0]?.[0];
|
||||
expect(activityTx).toBe(updateTx);
|
||||
});
|
||||
|
||||
it("rejects a review binding to a confirmation from another run", 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: "33333333-3333-4333-8333-333333333333",
|
||||
sourceRunId: "44444444-4444-4444-8444-444444444444",
|
||||
payload: { version: 1, prompt: "Approve another run's request?" },
|
||||
}]);
|
||||
|
||||
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(422);
|
||||
expect(res.body).toMatchObject({
|
||||
error: expect.stringContaining("created by this agent run"),
|
||||
details: { code: "invalid_review_interaction" },
|
||||
});
|
||||
expect(mockIssueService.update).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("allows an agent-authored in_review transition with a typed execution participant", async () => {
|
||||
|
|
|
|||
|
|
@ -2,7 +2,10 @@ import { randomUUID } from "node:crypto";
|
|||
import { afterAll, afterEach, beforeAll, describe, expect, it } from "vitest";
|
||||
import { activityLog, agents, companies, createDb, issues, type Db } from "@paperclipai/db";
|
||||
import { HttpError } from "../errors.js";
|
||||
import { assertIssueReviewVerdictActorAllowed } from "../services/issue-review-policy.js";
|
||||
import {
|
||||
assertIssueReviewVerdictActorAllowed,
|
||||
isIssueReviewVerdictInteraction,
|
||||
} from "../services/issue-review-policy.js";
|
||||
import {
|
||||
getEmbeddedPostgresTestSupport,
|
||||
startEmbeddedPostgresTestDatabase,
|
||||
|
|
@ -163,6 +166,37 @@ describeEmbeddedPostgres("issue review verdict policy", () => {
|
|||
})).resolves.toBeUndefined();
|
||||
});
|
||||
|
||||
it("classifies only confirmations created by the review requester as review verdicts", async () => {
|
||||
const seeded = await seedReview("not_creator");
|
||||
await db.insert(activityLog).values({
|
||||
companyId: seeded.companyId,
|
||||
actorType: "agent",
|
||||
actorId: seeded.requesterAgentId,
|
||||
agentId: seeded.requesterAgentId,
|
||||
action: "issue.updated",
|
||||
entityType: "issue",
|
||||
entityId: seeded.issue.id,
|
||||
details: {
|
||||
status: "in_review",
|
||||
reviewInteractionId: "review-confirmation",
|
||||
_previous: { status: "in_progress" },
|
||||
},
|
||||
});
|
||||
|
||||
await expect(isIssueReviewVerdictInteraction(db, {
|
||||
issue: seeded.issue,
|
||||
interaction: { id: "review-confirmation", createdByAgentId: seeded.requesterAgentId },
|
||||
})).resolves.toBe(true);
|
||||
await expect(isIssueReviewVerdictInteraction(db, {
|
||||
issue: seeded.issue,
|
||||
interaction: { id: "review-confirmation", createdByAgentId: seeded.peerAgentId },
|
||||
})).resolves.toBe(false);
|
||||
await expect(isIssueReviewVerdictInteraction(db, {
|
||||
issue: seeded.issue,
|
||||
interaction: { id: "requester-sibling", createdByAgentId: seeded.requesterAgentId },
|
||||
})).resolves.toBe(false);
|
||||
});
|
||||
|
||||
it("uses authenticated principal type for human_only", async () => {
|
||||
const seeded = await seedReview("human_only");
|
||||
expect(seeded.issue.reviewPolicy).toBe("human_only");
|
||||
|
|
|
|||
|
|
@ -34,6 +34,9 @@ const mockResolveTaskWatchdogMutationScope = vi.hoisted(() => vi.fn(async () =>
|
|||
const mockResolveCoreTrustPreset = vi.hoisted(() => vi.fn(() => ({ kind: "standard" })));
|
||||
|
||||
const mockLogActivity = vi.hoisted(() => vi.fn(async () => undefined));
|
||||
const mockReviewTransition = vi.hoisted(() => ({
|
||||
value: null as null | { actorType: string; actorId: string; details: Record<string, unknown> },
|
||||
}));
|
||||
const mockDbSelectWhere = vi.hoisted(() => vi.fn(() => ({
|
||||
then: (onFulfilled: (rows: unknown[]) => unknown, onRejected?: (reason: unknown) => unknown) =>
|
||||
Promise.resolve([{ companyId: "company-1", agentId: CREATED_AGENT_ID, contextSnapshot: null }]).then(
|
||||
|
|
@ -409,7 +412,11 @@ describe.sequential("issue thread interaction routes", () => {
|
|||
onFulfilled,
|
||||
onRejected,
|
||||
),
|
||||
orderBy: () => ({
|
||||
limit: () => Promise.resolve(mockReviewTransition.value ? [mockReviewTransition.value] : []),
|
||||
}),
|
||||
}));
|
||||
mockReviewTransition.value = null;
|
||||
});
|
||||
|
||||
it("lists and creates board-authored interactions", async () => {
|
||||
|
|
@ -762,6 +769,17 @@ describe.sequential("issue thread interaction routes", () => {
|
|||
});
|
||||
|
||||
it("allows the creator agent to withdraw and wakes a different assignee", async () => {
|
||||
mockIssueService.getById.mockResolvedValueOnce(createIssue({ status: "in_review", reviewPolicy: null }));
|
||||
mockInteractionService.getForIssue.mockResolvedValueOnce({
|
||||
id: "interaction-withdraw",
|
||||
kind: "request_confirmation",
|
||||
status: "pending",
|
||||
createdByAgentId: CREATED_AGENT_ID,
|
||||
sourceRunId: "run-1",
|
||||
requestedResolverPolicy: "board_only",
|
||||
effectiveResolverPolicy: "board_only",
|
||||
payload: { version: 1, prompt: "Proceed?" },
|
||||
});
|
||||
const app = await createApp({ type: "agent", agentId: CREATED_AGENT_ID, companyId: "company-1", runId: "run-1" });
|
||||
const res = await request(app)
|
||||
.post("/api/issues/aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa/interactions/interaction-withdraw/withdraw")
|
||||
|
|
@ -1661,6 +1679,300 @@ describe.sequential("issue thread interaction routes", () => {
|
|||
}));
|
||||
});
|
||||
|
||||
it("allows an agent to accept another agent's pending review confirmation by default", async () => {
|
||||
mockReviewTransition.value = {
|
||||
actorType: "agent",
|
||||
actorId: CREATED_AGENT_ID,
|
||||
details: { reviewInteractionId: "interaction-agent-review" },
|
||||
};
|
||||
mockIssueService.getById.mockResolvedValueOnce(createIssue({
|
||||
status: "in_review",
|
||||
reviewPolicy: null,
|
||||
createdByAgentId: CREATED_AGENT_ID,
|
||||
createdByUserId: null,
|
||||
}));
|
||||
mockInteractionService.getForIssue.mockResolvedValueOnce({
|
||||
id: "interaction-agent-review",
|
||||
kind: "request_confirmation",
|
||||
status: "pending",
|
||||
createdByAgentId: CREATED_AGENT_ID,
|
||||
sourceRunId: "run-1",
|
||||
requestedResolverPolicy: "board_only",
|
||||
effectiveResolverPolicy: "board_only",
|
||||
payload: { version: 1, prompt: "Approve this review?" },
|
||||
});
|
||||
mockInteractionService.acceptInteraction.mockResolvedValueOnce({
|
||||
interaction: {
|
||||
id: "interaction-agent-review",
|
||||
companyId: "company-1",
|
||||
issueId: "aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa",
|
||||
kind: "request_confirmation",
|
||||
status: "accepted",
|
||||
continuationPolicy: "none",
|
||||
requestedResolverPolicy: "board_only",
|
||||
effectiveResolverPolicy: "board_only",
|
||||
payload: { version: 1, prompt: "Approve this review?" },
|
||||
result: { version: 1, outcome: "accepted" },
|
||||
},
|
||||
createdIssues: [],
|
||||
});
|
||||
const app = await createApp({
|
||||
type: "agent",
|
||||
agentId: ASSIGNEE_AGENT_ID,
|
||||
companyId: "company-1",
|
||||
runId: "run-2",
|
||||
});
|
||||
|
||||
const res = await request(app)
|
||||
.post("/api/issues/aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa/interactions/interaction-agent-review/accept")
|
||||
.send({});
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
expect(mockInteractionService.acceptInteraction).toHaveBeenCalledWith(
|
||||
expect.anything(),
|
||||
"interaction-agent-review",
|
||||
{},
|
||||
{
|
||||
agentId: ASSIGNEE_AGENT_ID,
|
||||
runId: "run-2",
|
||||
userId: null,
|
||||
reviewVerdictAuthorized: true,
|
||||
},
|
||||
);
|
||||
expect(mockLogActivity).toHaveBeenCalledWith(expect.anything(), expect.objectContaining({
|
||||
actorType: "agent",
|
||||
agentId: ASSIGNEE_AGENT_ID,
|
||||
details: expect.objectContaining({ resolutionActorKind: "agent" }),
|
||||
}));
|
||||
});
|
||||
|
||||
it("allows an agent to reject a pending review confirmation by default", async () => {
|
||||
mockReviewTransition.value = {
|
||||
actorType: "agent",
|
||||
actorId: CREATED_AGENT_ID,
|
||||
details: { reviewInteractionId: "interaction-agent-reject" },
|
||||
};
|
||||
mockIssueService.getById.mockResolvedValueOnce(createIssue({
|
||||
status: "in_review",
|
||||
reviewPolicy: null,
|
||||
createdByAgentId: CREATED_AGENT_ID,
|
||||
createdByUserId: null,
|
||||
}));
|
||||
mockInteractionService.getForIssue.mockResolvedValueOnce({
|
||||
id: "interaction-agent-reject",
|
||||
kind: "request_confirmation",
|
||||
status: "pending",
|
||||
createdByAgentId: CREATED_AGENT_ID,
|
||||
sourceRunId: "run-1",
|
||||
requestedResolverPolicy: "board_only",
|
||||
effectiveResolverPolicy: "board_only",
|
||||
payload: { version: 1, prompt: "Approve this review?" },
|
||||
});
|
||||
const app = await createApp({
|
||||
type: "agent",
|
||||
agentId: ASSIGNEE_AGENT_ID,
|
||||
companyId: "company-1",
|
||||
runId: "run-2",
|
||||
});
|
||||
|
||||
const res = await request(app)
|
||||
.post("/api/issues/aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa/interactions/interaction-agent-reject/reject")
|
||||
.send({ reason: "Needs changes" });
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
expect(mockInteractionService.rejectInteraction).toHaveBeenCalledWith(
|
||||
expect.anything(),
|
||||
"interaction-agent-reject",
|
||||
{ reason: "Needs changes" },
|
||||
expect.objectContaining({
|
||||
agentId: ASSIGNEE_AGENT_ID,
|
||||
reviewVerdictAuthorized: true,
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("keeps an unrelated pending confirmation board-only on an in-review issue", async () => {
|
||||
mockReviewTransition.value = {
|
||||
actorType: "agent",
|
||||
actorId: CREATED_AGENT_ID,
|
||||
details: { reviewInteractionId: "interaction-agent-review" },
|
||||
};
|
||||
mockIssueService.getById.mockResolvedValueOnce(createIssue({
|
||||
status: "in_review",
|
||||
reviewPolicy: null,
|
||||
createdByAgentId: CREATED_AGENT_ID,
|
||||
createdByUserId: null,
|
||||
}));
|
||||
mockInteractionService.getForIssue.mockResolvedValueOnce({
|
||||
id: "interaction-unrelated-confirmation",
|
||||
kind: "request_confirmation",
|
||||
status: "pending",
|
||||
createdByAgentId: UNRELATED_AGENT_ID,
|
||||
sourceRunId: "run-1",
|
||||
requestedResolverPolicy: "board_only",
|
||||
effectiveResolverPolicy: "board_only",
|
||||
payload: { version: 1, prompt: "Approve an unrelated operation?" },
|
||||
});
|
||||
const app = await createApp({
|
||||
type: "agent",
|
||||
agentId: ASSIGNEE_AGENT_ID,
|
||||
companyId: "company-1",
|
||||
runId: "run-2",
|
||||
});
|
||||
|
||||
const res = await request(app)
|
||||
.post("/api/issues/aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa/interactions/interaction-unrelated-confirmation/accept")
|
||||
.send({});
|
||||
|
||||
expect(res.status).toBe(403);
|
||||
expect(res.body).toEqual({ error: "This issue-thread interaction is board-only" });
|
||||
expect(mockInteractionService.acceptInteraction).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("keeps a same-requester sibling confirmation board-only", async () => {
|
||||
mockReviewTransition.value = {
|
||||
actorType: "agent",
|
||||
actorId: CREATED_AGENT_ID,
|
||||
details: { reviewInteractionId: "interaction-agent-review" },
|
||||
};
|
||||
mockIssueService.getById.mockResolvedValueOnce(createIssue({
|
||||
status: "in_review",
|
||||
reviewPolicy: null,
|
||||
createdByAgentId: CREATED_AGENT_ID,
|
||||
createdByUserId: null,
|
||||
}));
|
||||
mockInteractionService.getForIssue.mockResolvedValueOnce({
|
||||
id: "interaction-requester-sibling",
|
||||
kind: "request_confirmation",
|
||||
status: "pending",
|
||||
createdByAgentId: CREATED_AGENT_ID,
|
||||
sourceRunId: "run-1",
|
||||
requestedResolverPolicy: "board_only",
|
||||
effectiveResolverPolicy: "board_only",
|
||||
payload: { version: 1, prompt: "Approve a different operation?" },
|
||||
});
|
||||
const app = await createApp({
|
||||
type: "agent",
|
||||
agentId: ASSIGNEE_AGENT_ID,
|
||||
companyId: "company-1",
|
||||
runId: "run-2",
|
||||
});
|
||||
|
||||
const res = await request(app)
|
||||
.post("/api/issues/aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa/interactions/interaction-requester-sibling/accept")
|
||||
.send({});
|
||||
|
||||
expect(res.status).toBe(403);
|
||||
expect(res.body).toEqual({ error: "This issue-thread interaction is board-only" });
|
||||
expect(mockInteractionService.acceptInteraction).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it.each([
|
||||
{
|
||||
name: "it addresses a different agent",
|
||||
requesterAgentId: CREATED_AGENT_ID,
|
||||
interaction: {
|
||||
addresseeAgentId: "agent-other-reviewer",
|
||||
createdByAgentId: CREATED_AGENT_ID,
|
||||
sourceRunId: "run-1",
|
||||
},
|
||||
error: "Only the addressed agent or a board user may resolve this issue-thread interaction",
|
||||
},
|
||||
{
|
||||
name: "the agent created it",
|
||||
requesterAgentId: ASSIGNEE_AGENT_ID,
|
||||
interaction: { createdByAgentId: ASSIGNEE_AGENT_ID, sourceRunId: "run-1" },
|
||||
error: "Agents cannot resolve interactions they created",
|
||||
},
|
||||
{
|
||||
name: "the same run created it",
|
||||
requesterAgentId: CREATED_AGENT_ID,
|
||||
interaction: { createdByAgentId: CREATED_AGENT_ID, sourceRunId: "run-2" },
|
||||
error: "Agents cannot resolve interactions created by the same run",
|
||||
},
|
||||
])("rejects an agent review verdict when $name", async ({ interaction, error, requesterAgentId }) => {
|
||||
mockReviewTransition.value = {
|
||||
actorType: "agent",
|
||||
actorId: requesterAgentId,
|
||||
details: { reviewInteractionId: "interaction-agent-review-scope" },
|
||||
};
|
||||
mockIssueService.getById.mockResolvedValueOnce(createIssue({
|
||||
status: "in_review",
|
||||
reviewPolicy: null,
|
||||
createdByAgentId: requesterAgentId,
|
||||
createdByUserId: null,
|
||||
}));
|
||||
mockInteractionService.getForIssue.mockResolvedValueOnce({
|
||||
id: "interaction-agent-review-scope",
|
||||
kind: "request_confirmation",
|
||||
status: "pending",
|
||||
requestedResolverPolicy: "board_only",
|
||||
effectiveResolverPolicy: "board_only",
|
||||
payload: { version: 1, prompt: "Approve this review?" },
|
||||
...interaction,
|
||||
});
|
||||
const app = await createApp({
|
||||
type: "agent",
|
||||
agentId: ASSIGNEE_AGENT_ID,
|
||||
companyId: "company-1",
|
||||
runId: "run-2",
|
||||
});
|
||||
|
||||
const res = await request(app)
|
||||
.post("/api/issues/aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa/interactions/interaction-agent-review-scope/accept")
|
||||
.send({});
|
||||
|
||||
expect(res.status).toBe(403);
|
||||
expect(res.body).toEqual({ error });
|
||||
expect(mockInteractionService.acceptInteraction).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("rejects an agent review-confirmation verdict under human_only with actionable copy", async () => {
|
||||
mockReviewTransition.value = {
|
||||
actorType: "agent",
|
||||
actorId: CREATED_AGENT_ID,
|
||||
details: { reviewInteractionId: "interaction-human-only" },
|
||||
};
|
||||
mockIssueService.getById.mockResolvedValueOnce(createIssue({
|
||||
status: "in_review",
|
||||
reviewPolicy: "human_only",
|
||||
createdByAgentId: CREATED_AGENT_ID,
|
||||
createdByUserId: null,
|
||||
}));
|
||||
mockInteractionService.getForIssue.mockResolvedValueOnce({
|
||||
id: "interaction-human-only",
|
||||
kind: "request_confirmation",
|
||||
status: "pending",
|
||||
createdByAgentId: CREATED_AGENT_ID,
|
||||
sourceRunId: "run-1",
|
||||
requestedResolverPolicy: "board_only",
|
||||
effectiveResolverPolicy: "board_only",
|
||||
payload: { version: 1, prompt: "Approve this review?" },
|
||||
});
|
||||
const app = await createApp({
|
||||
type: "agent",
|
||||
agentId: ASSIGNEE_AGENT_ID,
|
||||
companyId: "company-1",
|
||||
runId: "run-2",
|
||||
});
|
||||
|
||||
const res = await request(app)
|
||||
.post("/api/issues/aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa/interactions/interaction-human-only/accept")
|
||||
.send({});
|
||||
|
||||
expect(res.status).toBe(403);
|
||||
expect(res.body).toMatchObject({
|
||||
error: expect.stringContaining("only an authenticated user"),
|
||||
details: {
|
||||
code: "review_policy_denied",
|
||||
policy: "human_only",
|
||||
allowedActor: "authenticated_user_with_issue_write_access",
|
||||
remediation: expect.stringContaining("Have an authenticated user"),
|
||||
},
|
||||
});
|
||||
expect(mockInteractionService.acceptInteraction).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("allows only the addressed agent or board to resolve an addressed interaction", async () => {
|
||||
const addressed = {
|
||||
id: "interaction-addressed",
|
||||
|
|
|
|||
|
|
@ -1472,6 +1472,117 @@ describeEmbeddedPostgres("issueThreadInteractionService", () => {
|
|||
})).rejects.toThrow("A decline reason is required for this confirmation");
|
||||
});
|
||||
|
||||
it("records an authorized agent as the review-confirmation resolver", async () => {
|
||||
const { companyId, goalId, issueId } = await seedConfirmationIssue("Agent review verdict");
|
||||
const resolverAgentId = randomUUID();
|
||||
const resolverRunId = randomUUID();
|
||||
await db.update(issues).set({ status: "in_review" }).where(eq(issues.id, issueId));
|
||||
await db.insert(agents).values({
|
||||
id: resolverAgentId,
|
||||
companyId,
|
||||
name: "Review agent",
|
||||
role: "reviewer",
|
||||
status: "active",
|
||||
adapterType: "codex_local",
|
||||
adapterConfig: {},
|
||||
runtimeConfig: {},
|
||||
permissions: {},
|
||||
});
|
||||
await db.insert(heartbeatRuns).values({
|
||||
id: resolverRunId,
|
||||
companyId,
|
||||
agentId: resolverAgentId,
|
||||
invocationSource: "manual",
|
||||
status: "running",
|
||||
startedAt: new Date(),
|
||||
});
|
||||
const created = await interactionsSvc.create({ id: issueId, companyId }, {
|
||||
kind: "request_confirmation",
|
||||
payload: { version: 1, prompt: "Approve this review?" },
|
||||
}, {
|
||||
userId: "local-board",
|
||||
});
|
||||
|
||||
const accepted = await interactionsSvc.acceptInteraction({
|
||||
id: issueId,
|
||||
companyId,
|
||||
goalId,
|
||||
projectId: null,
|
||||
}, created.id, {}, {
|
||||
agentId: resolverAgentId,
|
||||
runId: resolverRunId,
|
||||
reviewVerdictAuthorized: true,
|
||||
});
|
||||
|
||||
expect(accepted.interaction).toMatchObject({
|
||||
status: "accepted",
|
||||
resolvedByAgentId: resolverAgentId,
|
||||
resolvedByRunId: resolverRunId,
|
||||
resolvedByUserId: null,
|
||||
});
|
||||
});
|
||||
|
||||
it("preserves creator and same-run guards for authorized agent review verdicts", async () => {
|
||||
const { companyId, goalId, issueId } = await seedConfirmationIssue("Guard agent review verdicts");
|
||||
const resolverAgentId = randomUUID();
|
||||
const resolverRunId = randomUUID();
|
||||
await db.insert(agents).values({
|
||||
id: resolverAgentId,
|
||||
companyId,
|
||||
name: "Review agent",
|
||||
role: "reviewer",
|
||||
status: "active",
|
||||
adapterType: "codex_local",
|
||||
adapterConfig: {},
|
||||
runtimeConfig: {},
|
||||
permissions: {},
|
||||
});
|
||||
await db.insert(heartbeatRuns).values({
|
||||
id: resolverRunId,
|
||||
companyId,
|
||||
agentId: resolverAgentId,
|
||||
invocationSource: "manual",
|
||||
status: "running",
|
||||
startedAt: new Date(),
|
||||
});
|
||||
|
||||
const createdByResolver = await interactionsSvc.create({ id: issueId, companyId }, {
|
||||
kind: "request_confirmation",
|
||||
payload: { version: 1, prompt: "Approve your own request?" },
|
||||
}, {
|
||||
userId: "local-board",
|
||||
});
|
||||
await db.update(issueThreadInteractions)
|
||||
.set({ createdByAgentId: resolverAgentId })
|
||||
.where(eq(issueThreadInteractions.id, createdByResolver.id));
|
||||
|
||||
const createdBySameRun = await interactionsSvc.create({ id: issueId, companyId }, {
|
||||
kind: "request_checkbox_confirmation",
|
||||
payload: {
|
||||
version: 1,
|
||||
prompt: "Approve the same run?",
|
||||
options: [{ id: "approve", label: "Approve" }],
|
||||
},
|
||||
}, {
|
||||
userId: "local-board",
|
||||
});
|
||||
await db.update(issueThreadInteractions)
|
||||
.set({ sourceRunId: resolverRunId })
|
||||
.where(eq(issueThreadInteractions.id, createdBySameRun.id));
|
||||
|
||||
const issue = { id: issueId, companyId, goalId, projectId: null };
|
||||
const actor = {
|
||||
agentId: resolverAgentId,
|
||||
runId: resolverRunId,
|
||||
reviewVerdictAuthorized: true,
|
||||
};
|
||||
await expect(interactionsSvc.acceptInteraction(issue, createdByResolver.id, {}, actor))
|
||||
.rejects.toThrow("Agents cannot resolve interactions they created");
|
||||
await expect(interactionsSvc.acceptInteraction(issue, createdBySameRun.id, {
|
||||
selectedOptionIds: ["approve"],
|
||||
}, actor)).rejects.toThrow("Agents cannot resolve interactions created by the same run");
|
||||
});
|
||||
|
||||
it("accepts request_checkbox_confirmation interactions with selected option ids", async () => {
|
||||
const { companyId, goalId, issueId } = await seedConfirmationIssue("Checkbox confirmation accept");
|
||||
|
||||
|
|
|
|||
|
|
@ -215,7 +215,10 @@ import {
|
|||
} from "../services/trust-preset-resolver.js";
|
||||
import { externalObjectService } from "../services/external-objects.js";
|
||||
import { deliverAgentUnblockNotification } from "../services/routable-blocked.js";
|
||||
import { assertIssueReviewVerdictActorAllowed } from "../services/issue-review-policy.js";
|
||||
import {
|
||||
assertIssueReviewVerdictActorAllowed,
|
||||
isIssueReviewVerdictInteraction,
|
||||
} from "../services/issue-review-policy.js";
|
||||
import {
|
||||
crossIssueInfluenceLimitError,
|
||||
crossIssueInfluenceRunContextError,
|
||||
|
|
@ -3362,34 +3365,61 @@ export function issueRoutes(
|
|||
};
|
||||
updateFields: Record<string, unknown>;
|
||||
actorType: string;
|
||||
actorAgentId?: string | null;
|
||||
actorRunId?: string | null;
|
||||
reviewInteractionId?: string;
|
||||
}) {
|
||||
const nextStatus = typeof input.updateFields.status === "string"
|
||||
? input.updateFields.status
|
||||
: input.existing.status;
|
||||
if (input.actorType !== "agent" || input.existing.status === "in_review" || nextStatus !== "in_review") return;
|
||||
if (input.actorType !== "agent" || input.existing.status === "in_review" || nextStatus !== "in_review") return null;
|
||||
|
||||
const interactions = await issueThreadInteractionService(db).listForIssue(input.existing.id);
|
||||
const pendingInteractions = interactions.filter((interaction) => interaction.status === "pending");
|
||||
if (input.reviewInteractionId) {
|
||||
const designatedReviewConfirmation = pendingInteractions.find((interaction) =>
|
||||
interaction.id === input.reviewInteractionId
|
||||
&& (interaction.kind === "request_confirmation" || interaction.kind === "request_checkbox_confirmation")
|
||||
&& interaction.createdByAgentId === input.actorAgentId
|
||||
&& interaction.sourceRunId === input.actorRunId
|
||||
&& !(
|
||||
interaction.kind === "request_confirmation"
|
||||
&& interaction.payload
|
||||
&& typeof interaction.payload === "object"
|
||||
&& "toolAction" in interaction.payload
|
||||
&& interaction.payload.toolAction !== undefined
|
||||
)
|
||||
);
|
||||
if (!designatedReviewConfirmation) {
|
||||
throw unprocessable("reviewInteractionId must identify a pending non-tool confirmation created by this agent run", {
|
||||
code: "invalid_review_interaction",
|
||||
reviewInteractionId: input.reviewInteractionId,
|
||||
});
|
||||
}
|
||||
return designatedReviewConfirmation.id;
|
||||
}
|
||||
|
||||
const nextAssigneeUserId = input.updateFields.assigneeUserId === undefined
|
||||
? input.existing.assigneeUserId
|
||||
: input.updateFields.assigneeUserId;
|
||||
if (typeof nextAssigneeUserId === "string" && nextAssigneeUserId.trim().length > 0) return;
|
||||
if (typeof nextAssigneeUserId === "string" && nextAssigneeUserId.trim().length > 0) return null;
|
||||
|
||||
const nextExecutionState = input.updateFields.executionState === undefined
|
||||
? input.existing.executionState
|
||||
: input.updateFields.executionState;
|
||||
if (hasExecutionParticipant(nextExecutionState)) return;
|
||||
if (hasExecutionParticipant(nextExecutionState)) return null;
|
||||
|
||||
const nextExecutionPolicy = input.updateFields.executionPolicy;
|
||||
if (hasScheduledMonitor({
|
||||
existingMonitorNextCheckAt: input.existing.monitorNextCheckAt ?? null,
|
||||
patchMonitorNextCheckAt: input.updateFields.monitorNextCheckAt,
|
||||
executionPolicy: nextExecutionPolicy,
|
||||
})) return;
|
||||
})) return null;
|
||||
|
||||
const interactions = await issueThreadInteractionService(db).listForIssue(input.existing.id);
|
||||
if (interactions.some((interaction) => interaction.status === "pending")) return;
|
||||
if (pendingInteractions.length > 0) return null;
|
||||
|
||||
const approvals = await issueApprovalsSvc.listApprovalsForIssue(input.existing.id);
|
||||
if (approvals.some((approval) => ACTIVE_REVIEW_APPROVAL_STATUSES.has(String(approval.status)))) return;
|
||||
if (approvals.some((approval) => ACTIVE_REVIEW_APPROVAL_STATUSES.has(String(approval.status)))) return null;
|
||||
|
||||
throw unprocessable(INVALID_AGENT_IN_REVIEW_DISPOSITION_MESSAGE, {
|
||||
code: "invalid_issue_disposition",
|
||||
|
|
@ -4039,17 +4069,21 @@ export function issueRoutes(
|
|||
res: Response,
|
||||
issue: Parameters<typeof assertAgentIssueMutationAllowed>[2],
|
||||
interaction: {
|
||||
id: string;
|
||||
createdByAgentId?: string | null;
|
||||
createdByUserId?: string | null;
|
||||
sourceRunId?: string | null;
|
||||
effectiveResolverPolicy: string;
|
||||
addresseeAgentId?: string | null;
|
||||
kind: string;
|
||||
status: string;
|
||||
payload?: unknown;
|
||||
},
|
||||
) {
|
||||
if (req.actor.type !== "agent") {
|
||||
assertBoard(req);
|
||||
return true;
|
||||
await assertPendingReviewInteractionVerdictAllowed(req, issue, interaction);
|
||||
return "standard" as const;
|
||||
}
|
||||
const actorAgentId = req.actor.agentId;
|
||||
const runId = requireAgentRunId(req, res);
|
||||
|
|
@ -4061,10 +4095,38 @@ export function issueRoutes(
|
|||
}
|
||||
if (await assertLowTrustControlPlaneDenied(req, res, issue.companyId, issue)) return false;
|
||||
if (!(await assertAgentIssueMutationAllowed(req, res, issue))) return false;
|
||||
const isReviewConfirmationVerdict = await isPendingReviewConfirmationVerdict(issue, interaction);
|
||||
const payload = interaction.payload && typeof interaction.payload === "object"
|
||||
? interaction.payload as { toolAction?: unknown }
|
||||
: null;
|
||||
if (interaction.kind === "request_confirmation" && payload?.toolAction !== undefined) {
|
||||
res.status(403).json({ error: "Tool-action confirmations are always board-only" });
|
||||
return false;
|
||||
}
|
||||
if (isReviewConfirmationVerdict) {
|
||||
if (!assertAgentInteractionActorAllowed(res, interaction, actorAgentId, runId)) return false;
|
||||
await assertPendingReviewInteractionVerdictAllowed(req, issue, interaction);
|
||||
return "review_verdict" as const;
|
||||
}
|
||||
if (interaction.effectiveResolverPolicy !== "board_or_agents") {
|
||||
res.status(403).json({ error: "This issue-thread interaction is board-only" });
|
||||
return false;
|
||||
}
|
||||
return assertAgentInteractionActorAllowed(res, interaction, actorAgentId, runId)
|
||||
? "standard" as const
|
||||
: false;
|
||||
}
|
||||
|
||||
function assertAgentInteractionActorAllowed(
|
||||
res: Response,
|
||||
interaction: {
|
||||
addresseeAgentId?: string | null;
|
||||
createdByAgentId?: string | null;
|
||||
sourceRunId?: string | null;
|
||||
},
|
||||
actorAgentId: string,
|
||||
runId: string,
|
||||
) {
|
||||
if (interaction.addresseeAgentId && interaction.addresseeAgentId !== actorAgentId) {
|
||||
res.status(403).json({ error: "Only the addressed agent or a board user may resolve this issue-thread interaction" });
|
||||
return false;
|
||||
|
|
@ -4077,16 +4139,36 @@ export function issueRoutes(
|
|||
res.status(403).json({ error: "Agents cannot resolve interactions created by the same run" });
|
||||
return false;
|
||||
}
|
||||
const payload = interaction.payload && typeof interaction.payload === "object"
|
||||
? interaction.payload as { toolAction?: unknown }
|
||||
: null;
|
||||
if (interaction.kind === "request_confirmation" && payload?.toolAction !== undefined) {
|
||||
res.status(403).json({ error: "Tool-action confirmations are always board-only" });
|
||||
return false;
|
||||
}
|
||||
return true;
|
||||
}
|
||||
|
||||
async function isPendingReviewConfirmationVerdict(
|
||||
issue: {
|
||||
id: string;
|
||||
companyId: string;
|
||||
status: string;
|
||||
createdByAgentId?: string | null;
|
||||
createdByUserId?: string | null;
|
||||
},
|
||||
interaction: {
|
||||
id: string;
|
||||
kind: string;
|
||||
status: string;
|
||||
createdByAgentId?: string | null;
|
||||
createdByUserId?: string | null;
|
||||
},
|
||||
) {
|
||||
if (
|
||||
issue.status !== "in_review"
|
||||
|| interaction.status !== "pending"
|
||||
|| (
|
||||
interaction.kind !== "request_confirmation"
|
||||
&& interaction.kind !== "request_checkbox_confirmation"
|
||||
)
|
||||
) return false;
|
||||
return isIssueReviewVerdictInteraction(db, { issue, interaction });
|
||||
}
|
||||
|
||||
async function assertPendingReviewInteractionVerdictAllowed(
|
||||
req: Request,
|
||||
issue: {
|
||||
|
|
@ -6118,6 +6200,8 @@ export function issueRoutes(
|
|||
existing,
|
||||
updateFields,
|
||||
actorType: req.actor.type,
|
||||
actorAgentId: actor.agentId,
|
||||
actorRunId: actor.runId,
|
||||
});
|
||||
|
||||
const actionStatus = outcome === "cancelled" ? "cancelled" : "resolved";
|
||||
|
|
@ -8376,6 +8460,7 @@ export function issueRoutes(
|
|||
: null;
|
||||
const {
|
||||
comment: commentBody,
|
||||
reviewInteractionId: requestedReviewInteractionId,
|
||||
reviewRequest,
|
||||
reopen: reopenRequested,
|
||||
resume: resumeRequested,
|
||||
|
|
@ -8719,10 +8804,13 @@ export function issueRoutes(
|
|||
}
|
||||
}
|
||||
|
||||
await assertAgentInReviewReviewPath({
|
||||
const reviewInteractionId = await assertAgentInReviewReviewPath({
|
||||
existing,
|
||||
updateFields,
|
||||
actorType: req.actor.type,
|
||||
actorAgentId: actor.agentId,
|
||||
actorRunId: actor.runId,
|
||||
reviewInteractionId: requestedReviewInteractionId,
|
||||
});
|
||||
|
||||
const nextAssigneeAgentId =
|
||||
|
|
@ -8802,6 +8890,39 @@ export function issueRoutes(
|
|||
? svc.update(id, issueUpdateData, db, postCommitActivityPublications)
|
||||
: svc.update(id, issueUpdateData);
|
||||
};
|
||||
const persistBoundReviewActivity = async (
|
||||
tx: Parameters<typeof svc.update>[2],
|
||||
updated: NonNullable<Awaited<ReturnType<typeof svc.update>>>,
|
||||
) => {
|
||||
if (!reviewInteractionId) return;
|
||||
const changes = updated.changes ?? {};
|
||||
const previous = Object.fromEntries(
|
||||
Object.entries(changes).map(([key, change]) => [key, change.from]),
|
||||
);
|
||||
await logActivity(tx as unknown as Db, {
|
||||
companyId: updated.companyId,
|
||||
actorType: actor.actorType,
|
||||
actorId: actor.actorId,
|
||||
agentId: actor.agentId,
|
||||
runId: actor.runId,
|
||||
agentApiKeyId: actor.agentApiKeyId,
|
||||
responsibleUserIdOverride: authenticatedActorResponsibleUserId(req),
|
||||
action: "issue.updated",
|
||||
entityType: "issue",
|
||||
entityId: updated.id,
|
||||
details: {
|
||||
...updateFields,
|
||||
identifier: updated.identifier,
|
||||
authorizationReason: issueMutationAuthorizationReason,
|
||||
changes,
|
||||
reviewInteractionId,
|
||||
...(commentBody ? { source: "comment" } : {}),
|
||||
...(resumeRequested === true ? { resumeIntent: true, followUpRequested: true } : {}),
|
||||
...(interruptedRunId ? { interruptedRunId } : {}),
|
||||
_previous: Object.keys(changes).length > 0 ? previous : undefined,
|
||||
},
|
||||
}, postCommitActivityPublications);
|
||||
};
|
||||
let issue: Awaited<ReturnType<typeof svc.update>>;
|
||||
try {
|
||||
if (transition.decision && decisionId) {
|
||||
|
|
@ -8827,6 +8948,8 @@ export function issueRoutes(
|
|||
stopRelayResult.value = await svc.addStopRelayCommentIfNeeded(updated, tx);
|
||||
}
|
||||
|
||||
await persistBoundReviewActivity(tx, updated);
|
||||
|
||||
return updated;
|
||||
});
|
||||
} else if (shouldRelayStop) {
|
||||
|
|
@ -8834,6 +8957,14 @@ export function issueRoutes(
|
|||
const updated = await updateIssue(tx);
|
||||
if (!updated) return null;
|
||||
stopRelayResult.value = await svc.addStopRelayCommentIfNeeded(updated, tx);
|
||||
await persistBoundReviewActivity(tx, updated);
|
||||
return updated;
|
||||
});
|
||||
} else if (reviewInteractionId) {
|
||||
issue = await db.transaction(async (tx) => {
|
||||
const updated = await updateIssue(tx);
|
||||
if (!updated) return null;
|
||||
await persistBoundReviewActivity(tx, updated);
|
||||
return updated;
|
||||
});
|
||||
} else {
|
||||
|
|
@ -9019,7 +9150,7 @@ export function issueRoutes(
|
|||
activeRecoveryAction: null,
|
||||
};
|
||||
}
|
||||
await logActivity(db, {
|
||||
if (!reviewInteractionId) await logActivity(db, {
|
||||
companyId: issue.companyId,
|
||||
actorType: actor.actorType,
|
||||
actorId: actor.actorId,
|
||||
|
|
@ -9035,6 +9166,7 @@ export function issueRoutes(
|
|||
identifier: issue.identifier,
|
||||
authorizationReason: issueMutationAuthorizationReason,
|
||||
changes: issueChanges,
|
||||
...(reviewInteractionId ? { reviewInteractionId } : {}),
|
||||
...(commentBody ? { source: "comment" } : {}),
|
||||
...(resumeRequested === true ? { resumeIntent: true, followUpRequested: true } : {}),
|
||||
...(reopened ? { reopened: true, reopenedFrom: reopenFromStatus } : {}),
|
||||
|
|
@ -10145,14 +10277,17 @@ export function issueRoutes(
|
|||
if (await rejectTaskWatchdogInteractionMutation(req, res, issue)) return;
|
||||
const interactionSvc = issueThreadInteractionService(db);
|
||||
const current = await interactionSvc.getForIssue(issue, interactionId);
|
||||
if (!(await assertIssueThreadInteractionResolutionAllowed(req, res, issue, current))) return;
|
||||
await assertPendingReviewInteractionVerdictAllowed(req, issue, current);
|
||||
const resolutionAuthorization = await assertIssueThreadInteractionResolutionAllowed(req, res, issue, current);
|
||||
if (!resolutionAuthorization) return;
|
||||
|
||||
const actor = getActorInfo(req);
|
||||
const { interaction, createdIssues, continuationIssue } = await interactionSvc.acceptInteraction(issue, interactionId, req.body, {
|
||||
agentId: actor.agentId,
|
||||
runId: actor.runId,
|
||||
userId: actor.actorType === "user" ? actor.actorId : null,
|
||||
...(actor.actorType === "agent" && resolutionAuthorization === "review_verdict"
|
||||
? { reviewVerdictAuthorized: true }
|
||||
: {}),
|
||||
});
|
||||
const toolAction = interaction.payload && typeof interaction.payload === "object"
|
||||
? (interaction.payload as { toolAction?: { actionRequestId?: unknown } }).toolAction
|
||||
|
|
@ -10298,14 +10433,17 @@ export function issueRoutes(
|
|||
if (await rejectTaskWatchdogInteractionMutation(req, res, issue)) return;
|
||||
const interactionSvc = issueThreadInteractionService(db);
|
||||
const current = await interactionSvc.getForIssue(issue, interactionId);
|
||||
if (!(await assertIssueThreadInteractionResolutionAllowed(req, res, issue, current))) return;
|
||||
await assertPendingReviewInteractionVerdictAllowed(req, issue, current);
|
||||
const resolutionAuthorization = await assertIssueThreadInteractionResolutionAllowed(req, res, issue, current);
|
||||
if (!resolutionAuthorization) return;
|
||||
|
||||
const actor = getActorInfo(req);
|
||||
const interaction = await interactionSvc.rejectInteraction(issue, interactionId, req.body, {
|
||||
agentId: actor.agentId,
|
||||
runId: actor.runId,
|
||||
userId: actor.actorType === "user" ? actor.actorId : null,
|
||||
...(actor.actorType === "agent" && resolutionAuthorization === "review_verdict"
|
||||
? { reviewVerdictAuthorized: true }
|
||||
: {}),
|
||||
});
|
||||
|
||||
await logActivity(db, {
|
||||
|
|
@ -10493,6 +10631,7 @@ export function issueRoutes(
|
|||
const interactionSvc = issueThreadInteractionService(db);
|
||||
const current = await interactionSvc.getForIssue(issue, interactionId);
|
||||
if (!(await assertIssueThreadInteractionWithdrawalAllowed(req, res, issue, current))) return;
|
||||
await assertPendingReviewInteractionVerdictAllowed(req, issue, current);
|
||||
|
||||
const actor = getActorInfo(req);
|
||||
const interaction = await interactionSvc.withdrawInteraction(issue, interactionId, req.body, {
|
||||
|
|
|
|||
|
|
@ -214,8 +214,16 @@ export async function persistActivity(db: Db, input: LogActivityInput) {
|
|||
};
|
||||
}
|
||||
|
||||
export async function logActivity(db: Db, input: LogActivityInput) {
|
||||
export async function logActivity(
|
||||
db: Db,
|
||||
input: LogActivityInput,
|
||||
postCommitPublications?: ActivityPublication[],
|
||||
) {
|
||||
const { activity, publication } = await persistActivity(db, input);
|
||||
publishActivity(publication);
|
||||
if (postCommitPublications) {
|
||||
postCommitPublications.push(publication);
|
||||
} else {
|
||||
publishActivity(publication);
|
||||
}
|
||||
return activity;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -8,6 +8,10 @@ export interface IssueReviewVerdictActor {
|
|||
id: string;
|
||||
}
|
||||
|
||||
interface IssueReviewRequester extends IssueReviewVerdictActor {
|
||||
reviewInteractionId: string | null;
|
||||
}
|
||||
|
||||
interface ReviewPolicyIssue {
|
||||
id: string;
|
||||
companyId: string;
|
||||
|
|
@ -19,11 +23,12 @@ interface ReviewPolicyIssue {
|
|||
async function findReviewRequester(
|
||||
db: Db,
|
||||
issue: ReviewPolicyIssue,
|
||||
): Promise<IssueReviewVerdictActor | null> {
|
||||
): Promise<IssueReviewRequester | null> {
|
||||
const transition = await db
|
||||
.select({
|
||||
actorType: activityLog.actorType,
|
||||
actorId: activityLog.actorId,
|
||||
details: activityLog.details,
|
||||
})
|
||||
.from(activityLog)
|
||||
.where(and(
|
||||
|
|
@ -50,17 +55,38 @@ async function findReviewRequester(
|
|||
.then((rows) => rows[0] ?? null);
|
||||
|
||||
if (transition?.actorType === "agent" || transition?.actorType === "user") {
|
||||
return { type: transition.actorType, id: transition.actorId };
|
||||
const reviewInteractionId = typeof transition.details?.reviewInteractionId === "string"
|
||||
? transition.details.reviewInteractionId
|
||||
: null;
|
||||
return { type: transition.actorType, id: transition.actorId, reviewInteractionId };
|
||||
}
|
||||
if (issue.createdByAgentId && !issue.createdByUserId) {
|
||||
return { type: "agent", id: issue.createdByAgentId };
|
||||
return { type: "agent", id: issue.createdByAgentId, reviewInteractionId: null };
|
||||
}
|
||||
if (issue.createdByUserId && !issue.createdByAgentId) {
|
||||
return { type: "user", id: issue.createdByUserId };
|
||||
return { type: "user", id: issue.createdByUserId, reviewInteractionId: null };
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
export async function isIssueReviewVerdictInteraction(
|
||||
db: Db,
|
||||
input: {
|
||||
issue: ReviewPolicyIssue;
|
||||
interaction: {
|
||||
id: string;
|
||||
createdByAgentId?: string | null;
|
||||
createdByUserId?: string | null;
|
||||
};
|
||||
},
|
||||
): Promise<boolean> {
|
||||
const requester = await findReviewRequester(db, input.issue);
|
||||
if (!requester?.reviewInteractionId || requester.reviewInteractionId !== input.interaction.id) return false;
|
||||
return requester.type === "agent"
|
||||
? input.interaction.createdByAgentId === requester.id
|
||||
: input.interaction.createdByUserId === requester.id;
|
||||
}
|
||||
|
||||
export async function assertIssueReviewVerdictActorAllowed(
|
||||
db: Db,
|
||||
input: {
|
||||
|
|
|
|||
|
|
@ -76,6 +76,7 @@ type InteractionActor = {
|
|||
runId?: string | null;
|
||||
userId?: string | null;
|
||||
systemId?: string | null;
|
||||
reviewVerdictAuthorized?: boolean;
|
||||
resolutionDetails?: Record<string, unknown>;
|
||||
};
|
||||
|
||||
|
|
@ -248,18 +249,6 @@ export function resolveInteractionPolicy(args: {
|
|||
function assertAgentResolutionAllowed(current: IssueThreadInteractionRow, actor: InteractionActor) {
|
||||
if (!actor.agentId) return;
|
||||
if (!actor.runId) throw forbidden("Agent run id required to resolve an issue-thread interaction");
|
||||
if (current.effectiveResolverPolicy !== "board_or_agents") {
|
||||
throw forbidden("This issue-thread interaction is board-only");
|
||||
}
|
||||
if (current.addresseeAgentId && current.addresseeAgentId !== actor.agentId) {
|
||||
throw forbidden("Only the addressed agent or a board user may resolve this issue-thread interaction");
|
||||
}
|
||||
if (current.createdByAgentId === actor.agentId) {
|
||||
throw forbidden("Agents cannot resolve interactions they created");
|
||||
}
|
||||
if (current.sourceRunId && current.sourceRunId === actor.runId) {
|
||||
throw forbidden("Agents cannot resolve interactions created by the same run");
|
||||
}
|
||||
if (
|
||||
current.kind === "request_confirmation"
|
||||
&& current.payload
|
||||
|
|
@ -269,6 +258,26 @@ function assertAgentResolutionAllowed(current: IssueThreadInteractionRow, actor:
|
|||
) {
|
||||
throw forbidden("Tool-action confirmations are always board-only");
|
||||
}
|
||||
if (actor.reviewVerdictAuthorized && isRequestConfirmationLikeKind(current.kind)) {
|
||||
assertAgentInteractionActorAllowed(current, actor);
|
||||
return;
|
||||
}
|
||||
if (current.effectiveResolverPolicy !== "board_or_agents") {
|
||||
throw forbidden("This issue-thread interaction is board-only");
|
||||
}
|
||||
assertAgentInteractionActorAllowed(current, actor);
|
||||
}
|
||||
|
||||
function assertAgentInteractionActorAllowed(current: IssueThreadInteractionRow, actor: InteractionActor) {
|
||||
if (current.addresseeAgentId && current.addresseeAgentId !== actor.agentId) {
|
||||
throw forbidden("Only the addressed agent or a board user may resolve this issue-thread interaction");
|
||||
}
|
||||
if (current.createdByAgentId === actor.agentId) {
|
||||
throw forbidden("Agents cannot resolve interactions they created");
|
||||
}
|
||||
if (current.sourceRunId && current.sourceRunId === actor.runId) {
|
||||
throw forbidden("Agents cannot resolve interactions created by the same run");
|
||||
}
|
||||
}
|
||||
|
||||
type IssueResolutionContext = {
|
||||
|
|
|
|||
|
|
@ -253,7 +253,7 @@ Key shared semantics:
|
|||
- **Supersede on user comment.** Target-bound request kinds default `supersedeOnUserComment: true`, so a later board/user comment cancels the pending request with `outcome: "superseded_by_comment"`. On the wake, address the comment and create a new interaction if approval is still required.
|
||||
- **Withdraw and terminal expiry.** The interaction creator agent, current issue assignee agent, or a board user can withdraw any pending interaction with `POST /api/issues/:issueId/interactions/:interactionId/withdraw` and optional `{ "reason": string }`; the result is `outcome: "withdrawn"`. Closing an issue as `done` or `cancelled` expires all remaining pending interactions with `outcome: "issue_closed"` and never wakes the closed issue.
|
||||
- **Idempotency.** Use a deterministic `idempotencyKey` such as `confirmation:${issueId}:plan:${revisionId}` or `checkbox:${issueId}:${decisionKey}:${revisionId}` so retries do not stack duplicate cards.
|
||||
- **Source issue posture.** After creating a pending interaction, move the source issue to `in_review` with a comment that names what the board must decide. The pending interaction is the explicit waiting path.
|
||||
- **Source issue posture.** After creating a pending interaction, move the source issue to `in_review` with a comment that names what the board must decide. When a `request_confirmation` or `request_checkbox_confirmation` is the issue review request, include its returned id as `reviewInteractionId` in that PATCH. This explicit binding lets policy-eligible agents submit the review verdict without granting the same authority to unrelated pending confirmations. The pending interaction is the explicit waiting path.
|
||||
|
||||
### Standalone Decisions
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue