RomanNum3ral
/
paperclip
mirror of https://github.com/paperclipai/paperclip.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #11405 from paperclipai/fix/review-policy-verdict-enforcement 

Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
Dotta 2026-08-15 02:06:41 +00:00
parent ed8075b535 edb8083538
commit 57edb26db4
16 changed files with 796 additions and 119 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
doc/SPEC-implementation.md
View File

@ -258,6 +258,10 @@ Invariants:
- single assignee only
- task must trace to company goal chain via `goal_id`, `parent_id`, or project-goal linkage
- `in_progress` requires assignee
- an `in_review -> done | cancelled` verdict is authorized against the current review policy while the issue row is locked; a policy change in the same request or a concurrent request cannot relax that verdict gate
- accepting or rejecting the review-confirmation interaction locks the issue row before resolving the interaction and reauthorizes against the current review policy in that transaction
- while a restrictive review policy is stored, changing it requires an actor who is allowed by that row-locked policy
- the transition into `in_review` and its requester activity record commit atomically, including transitions without an explicit review-interaction binding
- terminal states: `done | cancelled`
## 7.7 `issue_comments`

3
server/src/__tests__/issue-activity-events-routes.test.ts
View File

@ -7,6 +7,7 @@ import { normalizeIssueExecutionPolicy } from "../services/issue-execution-polic
const mockIssueService = vi.hoisted(() => ({
getById: vi.fn(),
getByIdForUpdate: vi.fn(),
assertCheckoutOwner: vi.fn(),
update: vi.fn(),
addComment: vi.fn(),
@ -200,6 +201,7 @@ describe("issue activity event routes", () => {
registerModuleMocks();
vi.clearAllMocks();
mockIssueService.assertCheckoutOwner.mockResolvedValue({ adoptedFromRunId: null });
mockIssueService.getByIdForUpdate.mockImplementation(async () => mockIssueService.getById());
mockIssueService.findMentionedAgents.mockResolvedValue([]);
mockIssueService.getRelationSummaries.mockResolvedValue({ blockedBy: [], blocks: [] });
mockIssueService.listWakeableBlockedDependents.mockResolvedValue([]);
@ -555,6 +557,7 @@ describe("issue activity event routes", () => {
createdAt: new Date("2026-05-01T00:00:00.000Z"),
};
const dbMock = {
transaction: async (callback: (tx: unknown) => Promise<unknown>) => callback({}),
select: () => ({
from: () => ({
where: () => ({

10
server/src/__tests__/issue-agent-mutation-ownership-routes.test.ts
View File

@ -20,6 +20,7 @@ const mockIssueService = vi.hoisted(() => ({
getAttachmentById: vi.fn(),
getByIdentifier: vi.fn(),
getById: vi.fn(),
getByIdForUpdate: vi.fn(),
getComment: vi.fn(),
getDependencyReadiness: vi.fn(),
getRelationSummaries: vi.fn(),
@ -433,6 +434,7 @@ describe("agent issue mutation checkout ownership", () => {
mockIssueService.getAttachmentById.mockReset();
mockIssueService.getByIdentifier.mockReset();
mockIssueService.getById.mockReset();
mockIssueService.getByIdForUpdate.mockReset();
mockIssueService.getComment.mockReset();
mockIssueService.getDependencyReadiness.mockReset();
mockIssueService.getDependencyReadiness.mockResolvedValue({
@ -558,6 +560,7 @@ describe("agent issue mutation checkout ownership", () => {
mockAgentService.resolveByReference.mockResolvedValue({ ambiguous: false, agent: null });
mockCompanyService.getById.mockResolvedValue({ id: companyId, issuePrefix: "PAP" });
mockIssueService.getById.mockResolvedValue(makeIssue());
mockIssueService.getByIdForUpdate.mockImplementation(async () => mockIssueService.getById());
mockIssueService.getByIdentifier.mockResolvedValue(null);
mockIssueService.getComment.mockResolvedValue({
id: "comment-1",
@ -976,6 +979,7 @@ describe("agent issue mutation checkout ownership", () => {
expect(mockIssueService.update).toHaveBeenCalledWith(
issueId,
expect.objectContaining({ status: "done" }),
expect.anything(),
);
});
@ -1944,7 +1948,11 @@ describe("agent issue mutation checkout ownership", () => {
const res = await request(app).patch(`/api/issues/${issueId}`).send({ status: "in_review" });
expect(res.status, JSON.stringify(res.body)).toBe(200);
expect(mockIssueService.update).toHaveBeenCalledWith(issueId, expect.objectContaining({ status: "in_review" }));
expect(mockIssueService.update).toHaveBeenCalledWith(
issueId,
expect.objectContaining({ status: "in_review" }),
expect.anything(),
);
});
it("rejects stale watchdog source mutations when revalidation finds a live path", async () => {

3
server/src/__tests__/issue-comment-reopen-routes.test.ts
View File

@ -5,6 +5,7 @@ import { HttpError } from "../errors.js";
const mockIssueService = vi.hoisted(() => ({
getById: vi.fn(),
getByIdForUpdate: vi.fn(),
assertCheckoutOwner: vi.fn(),
update: vi.fn(),
addComment: vi.fn(),
@ -265,6 +266,7 @@ describe.sequential("issue comment reopen routes", () => {
beforeEach(() => {
vi.clearAllMocks();
mockIssueService.getById.mockReset();
mockIssueService.getByIdForUpdate.mockReset();
mockIssueService.assertCheckoutOwner.mockReset();
mockIssueService.update.mockReset();
mockIssueService.addComment.mockReset();
@ -316,6 +318,7 @@ describe.sequential("issue comment reopen routes", () => {
mockDbSelectFrom.mockImplementation(() => ({ where: mockDbSelectWhere }));
mockDbSelect.mockImplementation(() => ({ from: mockDbSelectFrom }));
mockDb.transaction.mockImplementation(async (fn: (tx: typeof mockTx) => Promise<unknown>) => fn(mockTx));
mockIssueService.getByIdForUpdate.mockImplementation(async () => mockIssueService.getById());
mockHeartbeatService.wakeup.mockResolvedValue(undefined);
mockHeartbeatService.reportRunActivity.mockResolvedValue(undefined);
mockHeartbeatService.getRun.mockResolvedValue(null);

3
server/src/__tests__/issue-dependency-wakeups-routes.test.ts
View File

@ -7,6 +7,7 @@ const mockFindExistingIssueBlockersResolvedWake = vi.hoisted(() => vi.fn(async (
const mockIssueService = vi.hoisted(() => ({
getAncestors: vi.fn(),
getById: vi.fn(),
getByIdForUpdate: vi.fn(),
getByIdentifier: vi.fn(async () => null),
getComment: vi.fn(),
getCommentCursor: vi.fn(),
@ -114,6 +115,7 @@ async function createApp() {
select: vi.fn(() => ({
from: vi.fn(() => query),
})),
transaction: async (callback: (tx: Record<string, never>) => Promise<unknown>) => callback({}),
};
const [{ issueRoutes }, { errorHandler }] = await Promise.all([
vi.importActual<typeof import("../routes/issues.js")>("../routes/issues.js"),
@ -145,6 +147,7 @@ describe("issue dependency wakeups in issue routes", () => {
vi.clearAllMocks();
mockFindExistingIssueBlockersResolvedWake.mockResolvedValue(null);
mockIssueService.getAncestors.mockResolvedValue([]);
mockIssueService.getByIdForUpdate.mockImplementation(async () => mockIssueService.getById());
mockIssueService.getComment.mockResolvedValue(null);
mockIssueService.getCommentCursor.mockResolvedValue({
totalComments: 0,

119
server/src/__tests__/issue-execution-policy-routes.test.ts
View File

@ -5,6 +5,7 @@ import { normalizeIssueExecutionPolicy } from "../services/issue-execution-polic
const mockIssueService = vi.hoisted(() => ({
getById: vi.fn(),
getByIdForUpdate: vi.fn(),
findOpenAncestorCreatedByAgent: vi.fn(async () => null),
assertCheckoutOwner: vi.fn(),
update: vi.fn(),
@ -190,6 +191,7 @@ describe("issue execution policy routes", () => {
registerModuleMocks();
vi.clearAllMocks();
mockIssueService.assertCheckoutOwner.mockResolvedValue({ adoptedFromRunId: null });
mockIssueService.getByIdForUpdate.mockImplementation(async () => mockIssueService.getById());
mockIssueService.findMentionedAgents.mockResolvedValue([]);
mockIssueService.getRelationSummaries.mockResolvedValue({ blockedBy: [], blocks: [] });
mockIssueService.listWakeableBlockedDependents.mockResolvedValue([]);
@ -250,6 +252,47 @@ describe("issue execution policy routes", () => {
mockAccessService.hasPermission.mockResolvedValue(false);
});
it("reauthorizes a terminal verdict against the review policy held under the update lock", async () => {
const issue = {
id: "aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa",
companyId: "company-1",
status: "in_review",
reviewPolicy: "anyone",
assigneeAgentId: "33333333-3333-4333-8333-333333333333",
assigneeUserId: null,
createdByUserId: "local-board",
identifier: "PAP-1002",
title: "Concurrent policy update",
executionPolicy: null,
executionState: null,
};
mockIssueService.getById.mockResolvedValue(issue);
mockIssueService.getByIdForUpdate.mockResolvedValue({
...issue,
reviewPolicy: "human_only",
});
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: "done" });
expect(res.status).toBe(403);
expect(res.body).toMatchObject({
details: {
code: "review_policy_denied",
policy: "human_only",
},
});
expect(mockDb.transaction).toHaveBeenCalled();
expect(mockIssueService.getByIdForUpdate).toHaveBeenCalled();
expect(mockIssueService.update).not.toHaveBeenCalled();
});
it("rejects an agent-authored in_review transition without a review path", async () => {
const issue = {
id: "aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa",
@ -326,6 +369,7 @@ describe("issue execution policy routes", () => {
expect(mockIssueService.update).toHaveBeenCalledWith(
"aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa",
expect.objectContaining({ status: "in_review" }),
expect.anything(),
);
expect(mockLogActivity).toHaveBeenCalledWith(
expect.anything(),
@ -333,7 +377,9 @@ describe("issue execution policy routes", () => {
action: "issue.updated",
details: expect.not.objectContaining({ reviewInteractionId: expect.anything() }),
}),
expect.any(Array),
);
expect(mockLogActivity.mock.calls[0]?.[0]).toBe(mockIssueService.update.mock.calls[0]?.[2]);
});
it("binds an explicitly designated same-run confirmation to the review transition", async () => {
@ -394,6 +440,63 @@ describe("issue execution policy routes", () => {
);
});
it("binds a user-designated confirmation to the review transition activity", async () => {
const issue = {
id: "aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa",
companyId: "company-1",
status: "todo",
assigneeAgentId: null,
assigneeUserId: "local-board",
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: null,
createdByUserId: "local-board",
sourceRunId: null,
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(),
}));
const res = await request(await createApp())
.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",
actorType: "user",
actorId: "local-board",
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",
@ -537,6 +640,7 @@ describe("issue execution policy routes", () => {
}),
}),
}),
expect.anything(),
);
});
@ -590,6 +694,7 @@ describe("issue execution policy routes", () => {
status: "in_review",
monitorNextCheckAt: new Date("2026-12-01T12:00:00.000Z"),
}),
expect.anything(),
);
});
@ -618,6 +723,18 @@ describe("issue execution policy routes", () => {
.send({ status: "in_review" });
expect(res.status).toBe(200);
expect(mockDb.transaction).toHaveBeenCalled();
expect(mockLogActivity).toHaveBeenCalledWith(
expect.anything(),
expect.objectContaining({
action: "issue.updated",
actorType: "user",
actorId: "local-board",
details: expect.objectContaining({ status: "in_review" }),
}),
expect.any(Array),
);
expect(mockLogActivity.mock.calls[0]?.[0]).toBe(mockIssueService.update.mock.calls[0]?.[2]);
expect(mockIssueThreadInteractionService.listForIssue).not.toHaveBeenCalled();
expect(mockIssueApprovalService.listApprovalsForIssue).not.toHaveBeenCalled();
});
@ -674,6 +791,7 @@ describe("issue execution policy routes", () => {
actorAgentId: null,
actorUserId: "local-board",
}),
expect.anything(),
);
expect(mockHeartbeatService.cancelRun).not.toHaveBeenCalled();
});
@ -730,6 +848,7 @@ describe("issue execution policy routes", () => {
actorAgentId: null,
actorUserId: "local-board",
}),
expect.anything(),
);
const updatePatch = mockIssueService.update.mock.calls[0]?.[1] as Record<string, unknown>;
expect(updatePatch.status).toBe("cancelled");

23
server/src/__tests__/issue-review-policy.test.ts
View File

@ -197,6 +197,29 @@ describeEmbeddedPostgres("issue review verdict policy", () => {
})).resolves.toBe(false);
});
it("classifies an unbound legacy confirmation only when the review requester created it", 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", _previous: { status: "in_progress" } },
});
await expect(isIssueReviewVerdictInteraction(db, {
issue: seeded.issue,
interaction: { id: "legacy-review", createdByAgentId: seeded.requesterAgentId },
})).resolves.toBe(true);
await expect(isIssueReviewVerdictInteraction(db, {
issue: seeded.issue,
interaction: { id: "unrelated-confirmation", createdByAgentId: seeded.peerAgentId },
})).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");

87
server/src/__tests__/issue-stalled-review-decision-routes.test.ts
View File

@ -355,7 +355,7 @@ describeEmbeddedPostgres("stalled review decision routes", () => {
code: "review_policy_denied",
policy: "human_only",
allowedActor: "authenticated_user_with_issue_write_access",
remediation: expect.stringContaining("authenticated user"),
remediation: "Have an authenticated user with issue write access submit the verdict.",
},
});
@ -366,7 +366,7 @@ describeEmbeddedPostgres("stalled review decision routes", () => {
expect(userVerdict.body).toMatchObject({ id: issueId, status: "cancelled" });
});
it("allows an agent writer to relax reviewPolicy in the verdict patch", async () => {
it("does not let an agent bypass human_only by relaxing reviewPolicy in the verdict patch", async () => {
const seeded = await seedCompany("RLP");
const issueId = await seedReview({
companyId: seeded.companyId,
@ -380,8 +380,87 @@ describeEmbeddedPostgres("stalled review decision routes", () => {
.patch(`/api/issues/${issueId}`)
.send({ status: "done", reviewPolicy: "anyone" });
expect(verdict.status, JSON.stringify(verdict.body)).toBe(200);
expect(verdict.body).toMatchObject({ id: issueId, status: "done", reviewPolicy: "anyone" });
expect(verdict.status).toBe(403);
expect(verdict.body).toMatchObject({
details: {
code: "review_policy_denied",
policy: "human_only",
allowedActor: "authenticated_user_with_issue_write_access",
remediation: "Have an authenticated user with issue write access submit the verdict.",
},
});
const [persisted] = await db.select({
status: issues.status,
reviewPolicy: issues.reviewPolicy,
}).from(issues).where(eq(issues.id, issueId));
expect(persisted).toEqual({ status: "in_review", reviewPolicy: "human_only" });
});
it("does not let the review requester bypass not_creator by relaxing reviewPolicy in the verdict patch", async () => {
const seeded = await seedCompany("RNC");
const issueId = await seedReview({
companyId: seeded.companyId,
assigneeAgentId: seeded.assigneeAgentId,
identifier: "RNC-1",
reviewPolicy: "not_creator",
});
await db.insert(activityLog).values({
companyId: seeded.companyId,
actorType: "agent",
actorId: seeded.assigneeAgentId,
agentId: seeded.assigneeAgentId,
action: "issue.updated",
entityType: "issue",
entityId: issueId,
details: { status: "in_review", _previous: { status: "in_progress" } },
});
const runId = await seedRun(seeded.companyId, seeded.assigneeAgentId, issueId);
const verdict = await request(app(agentActor(seeded.companyId, seeded.assigneeAgentId, runId)))
.patch(`/api/issues/${issueId}`)
.send({ status: "done", reviewPolicy: "anyone" });
expect(verdict.status).toBe(403);
expect(verdict.body).toMatchObject({
details: {
code: "review_policy_denied",
policy: "not_creator",
allowedActor: "writer_other_than_review_requester",
remediation: "Have another writer with issue write access submit the verdict.",
},
});
const [persisted] = await db.select({
status: issues.status,
reviewPolicy: issues.reviewPolicy,
}).from(issues).where(eq(issues.id, issueId));
expect(persisted).toEqual({ status: "in_review", reviewPolicy: "not_creator" });
});
it("does not let an excluded actor relax an existing review policy in a separate patch", async () => {
const seeded = await seedCompany("RSP");
const issueId = await seedReview({
companyId: seeded.companyId,
assigneeAgentId: seeded.assigneeAgentId,
identifier: "RSP-1",
reviewPolicy: "human_only",
});
const runId = await seedRun(seeded.companyId, seeded.assigneeAgentId, issueId);
const relaxation = await request(app(agentActor(seeded.companyId, seeded.assigneeAgentId, runId)))
.patch(`/api/issues/${issueId}`)
.send({ reviewPolicy: "anyone" });
expect(relaxation.status).toBe(403);
expect(relaxation.body).toMatchObject({
details: {
code: "review_policy_denied",
policy: "human_only",
},
});
const [persisted] = await db.select({ reviewPolicy: issues.reviewPolicy })
.from(issues)
.where(eq(issues.id, issueId));
expect(persisted).toEqual({ reviewPolicy: "human_only" });
});
it("enforces not_creator when accepting or rejecting pending review interactions", async () => {

2
server/src/__tests__/issue-telemetry-routes.test.ts
View File

@ -4,6 +4,7 @@ import { beforeEach, describe, expect, it, vi } from "vitest";
const mockIssueService = vi.hoisted(() => ({
getById: vi.fn(),
getByIdForUpdate: vi.fn(),
getWakeableParentAfterChildCompletion: vi.fn(),
listWakeableBlockedDependents: vi.fn(),
update: vi.fn(),
@ -157,6 +158,7 @@ describe("issue telemetry routes", () => {
vi.clearAllMocks();
mockGetTelemetryClient.mockReturnValue({ track: vi.fn() });
mockIssueService.getById.mockResolvedValue(makeIssue("todo"));
mockIssueService.getByIdForUpdate.mockImplementation(async () => mockIssueService.getById());
mockIssueService.getWakeableParentAfterChildCompletion.mockResolvedValue(null);
mockIssueService.listWakeableBlockedDependents.mockResolvedValue([]);
mockIssueService.update.mockImplementation(async (_id: string, patch: Record<string, unknown>) => ({

105
server/src/__tests__/issue-thread-interaction-routes.test.ts
View File

@ -1910,6 +1910,111 @@ describe.sequential("issue thread interaction routes", () => {
expect(mockInteractionService.acceptInteraction).not.toHaveBeenCalled();
});
it.each([
{ action: "accept", body: {} },
{ action: "reject", body: { reason: "Needs changes" } },
])("rejects an agent $action verdict under human_only when a user review transition omitted the interaction binding", async ({ action, body }) => {
mockReviewTransition.value = {
actorType: "user",
actorId: "local-board",
details: { status: "in_review", _previous: { status: "in_progress" } },
};
mockIssueService.getById.mockResolvedValueOnce(createIssue({
status: "in_review",
reviewPolicy: "human_only",
createdByAgentId: null,
createdByUserId: "local-board",
}));
mockInteractionService.getForIssue.mockResolvedValueOnce({
id: "interaction-user-review-unbound",
kind: "request_confirmation",
status: "pending",
createdByAgentId: null,
createdByUserId: "local-board",
sourceRunId: null,
requestedResolverPolicy: "board_or_agents",
effectiveResolverPolicy: "board_or_agents",
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-user-review-unbound/${action}`)
.send(body);
expect(res.status).toBe(403);
expect(res.body).toMatchObject({
details: {
code: "review_policy_denied",
policy: "human_only",
},
});
expect(mockInteractionService.acceptInteraction).not.toHaveBeenCalled();
expect(mockInteractionService.rejectInteraction).not.toHaveBeenCalled();
});
it("allows an unrelated agent-resolvable confirmation under a restrictive issue review policy", async () => {
mockReviewTransition.value = {
actorType: "user",
actorId: "local-board",
details: { status: "in_review", _previous: { status: "in_progress" } },
};
mockIssueService.getById.mockResolvedValueOnce(createIssue({
status: "in_review",
reviewPolicy: "human_only",
createdByAgentId: null,
createdByUserId: "local-board",
}));
mockInteractionService.getForIssue.mockResolvedValueOnce({
id: "interaction-unrelated",
kind: "request_confirmation",
status: "pending",
createdByAgentId: UNRELATED_AGENT_ID,
createdByUserId: null,
sourceRunId: "run-1",
requestedResolverPolicy: "board_or_agents",
effectiveResolverPolicy: "board_or_agents",
payload: { version: 1, prompt: "Confirm an independent action?" },
});
mockInteractionService.acceptInteraction.mockResolvedValueOnce({
interaction: {
id: "interaction-unrelated",
companyId: "company-1",
issueId: "aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa",
kind: "request_confirmation",
status: "accepted",
continuationPolicy: "none",
idempotencyKey: null,
sourceCommentId: null,
sourceRunId: "run-1",
payload: { version: 1, prompt: "Confirm an independent action?" },
result: { version: 1, outcome: "accepted" },
createdAt: "2026-04-20T12:00:00.000Z",
updatedAt: "2026-04-20T12:05:00.000Z",
resolvedAt: "2026-04-20T12:05:00.000Z",
},
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-unrelated/accept")
.send({});
expect(res.status, JSON.stringify(res.body)).toBe(200);
expect(mockInteractionService.acceptInteraction).toHaveBeenCalled();
});
it("allows only the addressed agent or board to resolve an addressed interaction", async () => {
const addressed = {
id: "interaction-addressed",

135
server/src/__tests__/issue-thread-interactions-service.test.ts
View File

@ -2,6 +2,7 @@ import { randomUUID } from "node:crypto";
import { eq } from "drizzle-orm";
import { afterAll, afterEach, beforeAll, describe, expect, it } from "vitest";
import {
activityLog,
agents,
companies,
createDb,
@ -48,6 +49,7 @@ describeEmbeddedPostgres("issueThreadInteractionService", () => {
afterEach(async () => {
await db.delete(issueThreadInteractions);
await db.delete(activityLog);
await db.delete(issueComments);
await db.delete(issueDocuments);
await db.delete(documentRevisions);
@ -100,6 +102,27 @@ describeEmbeddedPostgres("issueThreadInteractionService", () => {
return { companyId, goalId, issueId };
}
async function recordReviewTransition(args: {
companyId: string;
issueId: string;
interactionId: string;
actorId?: string;
}) {
await db.insert(activityLog).values({
companyId: args.companyId,
actorType: "user",
actorId: args.actorId ?? "local-board",
action: "issue.updated",
entityType: "issue",
entityId: args.issueId,
details: {
status: "in_review",
reviewInteractionId: args.interactionId,
_previous: { status: "in_progress" },
},
});
}
it("persists addressees without allowing them to bypass board-only governance", async () => {
const { companyId, issueId } = await seedConfirmationIssue("Agent-addressed interaction");
const creatorAgentId = randomUUID();
@ -1687,6 +1710,7 @@ describeEmbeddedPostgres("issueThreadInteractionService", () => {
}, {
userId: "local-board",
});
await recordReviewTransition({ companyId, issueId, interactionId: created.id });
const accepted = await interactionsSvc.acceptInteraction({
id: issueId,
@ -1707,10 +1731,113 @@ describeEmbeddedPostgres("issueThreadInteractionService", () => {
});
});
it.each(["accept", "reject"] as const)(
"revalidates review policy under the issue lock before interaction %s",
async (action) => {
const { companyId, goalId, issueId } = await seedConfirmationIssue(`Locked ${action} policy`);
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 created = await interactionsSvc.create({ id: issueId, companyId }, {
kind: "request_confirmation",
payload: { version: 1, prompt: "Approve this review?" },
}, {
userId: "local-board",
});
await db.update(issues)
.set({ status: "in_review", reviewPolicy: "anyone" })
.where(eq(issues.id, issueId));
await recordReviewTransition({ companyId, issueId, interactionId: created.id });
let releasePolicyLock!: () => void;
let policyLockReady!: () => void;
const holdPolicyLock = new Promise<void>((resolve) => {
releasePolicyLock = resolve;
});
const policyLocked = new Promise<void>((resolve) => {
policyLockReady = resolve;
});
const tightenPolicy = db.transaction(async (tx) => {
await tx.select({ id: issues.id })
.from(issues)
.where(eq(issues.id, issueId))
.for("update");
await tx.update(issues)
.set({ reviewPolicy: "human_only" })
.where(eq(issues.id, issueId));
policyLockReady();
await holdPolicyLock;
});
await policyLocked;
const actor = {
agentId: resolverAgentId,
runId: resolverRunId,
reviewVerdictAuthorized: true,
};
const verdict = action === "accept"
? interactionsSvc.acceptInteraction({
id: issueId,
companyId,
goalId,
projectId: null,
status: "in_review",
}, created.id, {}, actor)
: interactionsSvc.rejectInteraction({
id: issueId,
companyId,
status: "in_review",
}, created.id, { reason: "Needs changes" }, actor);
let verdictSettled = false;
void verdict.then(
() => { verdictSettled = true; },
() => { verdictSettled = true; },
);
const denied = expect(verdict).rejects.toMatchObject({
status: 403,
details: expect.objectContaining({
code: "review_policy_denied",
policy: "human_only",
}),
});
await new Promise((resolve) => setTimeout(resolve, 25));
expect(verdictSettled).toBe(false);
releasePolicyLock();
await tightenPolicy;
await denied;
const persisted = await db.select({ status: issueThreadInteractions.status })
.from(issueThreadInteractions)
.where(eq(issueThreadInteractions.id, created.id))
.then((rows) => rows[0]);
expect(persisted?.status).toBe("pending");
},
);
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.update(issues).set({ status: "in_review" }).where(eq(issues.id, issueId));
await db.insert(agents).values({
id: resolverAgentId,
companyId,
@ -1740,6 +1867,7 @@ describeEmbeddedPostgres("issueThreadInteractionService", () => {
await db.update(issueThreadInteractions)
.set({ createdByAgentId: resolverAgentId })
.where(eq(issueThreadInteractions.id, createdByResolver.id));
await recordReviewTransition({ companyId, issueId, interactionId: createdByResolver.id });
const createdBySameRun = await interactionsSvc.create({ id: issueId, companyId }, {
kind: "request_checkbox_confirmation",
@ -1763,6 +1891,13 @@ describeEmbeddedPostgres("issueThreadInteractionService", () => {
};
await expect(interactionsSvc.acceptInteraction(issue, createdByResolver.id, {}, actor))
.rejects.toThrow("Agents cannot resolve interactions they created");
await db.update(activityLog).set({
details: {
status: "in_review",
reviewInteractionId: createdBySameRun.id,
_previous: { status: "in_progress" },
},
}).where(eq(activityLog.entityId, issueId));
await expect(interactionsSvc.acceptInteraction(issue, createdBySameRun.id, {
selectedOptionIds: ["approve"],
}, actor)).rejects.toThrow("Agents cannot resolve interactions created by the same run");

6
server/src/__tests__/issue-update-comment-wakeup-routes.test.ts
View File

@ -8,6 +8,7 @@ const MENTIONED_AGENT_ID = "33333333-3333-4333-8333-333333333333";
const mockIssueService = vi.hoisted(() => ({
getById: vi.fn(),
getByIdForUpdate: vi.fn(),
update: vi.fn(),
addComment: vi.fn(),
findMentionedAgents: vi.fn(),
@ -192,7 +193,9 @@ async function createApp() {
};
next();
});
app.use("/api", issueRoutes({} as any, {} as any));
app.use("/api", issueRoutes({
transaction: async (callback: (tx: Record<string, never>) => Promise<unknown>) => callback({}),
} as any, {} as any));
app.use(errorHandler);
return app;
}
@ -227,6 +230,7 @@ describe("issue update comment wakeups", () => {
registerModuleMocks();
vi.clearAllMocks();
mockIssueService.findMentionedAgents.mockResolvedValue([]);
mockIssueService.getByIdForUpdate.mockImplementation(async () => mockIssueService.getById());
mockIssueService.getRelationSummaries.mockResolvedValue({ blockedBy: [], blocks: [] });
mockIssueService.listWakeableBlockedDependents.mockResolvedValue([]);
mockIssueService.getWakeableParentAfterChildCompletion.mockResolvedValue(null);

164
server/src/routes/issues.ts
View File

@ -3411,7 +3411,7 @@ export function issueRoutes(
);
}
async function assertAgentInReviewReviewPath(input: {
async function assertInReviewReviewPath(input: {
existing: {
id: string;
companyId: string;
@ -3421,7 +3421,8 @@ export function issueRoutes(
monitorNextCheckAt?: Date | null;
};
updateFields: Record<string, unknown>;
actorType: string;
actorType: "agent" | "user";
actorId: string;
actorAgentId?: string | null;
actorRunId?: string | null;
reviewInteractionId?: string;
@ -3429,7 +3430,8 @@ export function issueRoutes(
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 null;
if (input.existing.status === "in_review" || nextStatus !== "in_review") return null;
if (input.actorType !== "agent" && !input.reviewInteractionId) return null;
const interactions = await issueThreadInteractionService(db).listForIssue(input.existing.id);
const pendingInteractions = interactions.filter((interaction) => interaction.status === "pending");
@ -3437,8 +3439,12 @@ export function issueRoutes(
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
&& (
input.actorType === "agent"
? interaction.createdByAgentId === input.actorAgentId
&& interaction.sourceRunId === input.actorRunId
: interaction.createdByUserId === input.actorId
)
&& !(
interaction.kind === "request_confirmation"
&& interaction.payload
@ -3448,7 +3454,10 @@ export function issueRoutes(
)
);
if (!designatedReviewConfirmation) {
throw unprocessable("reviewInteractionId must identify a pending non-tool confirmation created by this agent run", {
const creatorDescription = input.actorType === "agent"
? "this agent run"
: "this user";
throw unprocessable(`reviewInteractionId must identify a pending non-tool confirmation created by ${creatorDescription}`, {
code: "invalid_review_interaction",
reviewInteractionId: input.reviewInteractionId,
});
@ -3456,6 +3465,8 @@ export function issueRoutes(
return designatedReviewConfirmation.id;
}
if (input.actorType !== "agent") return null;
const nextAssigneeUserId = input.updateFields.assigneeUserId === undefined
? input.existing.assigneeUserId
: input.updateFields.assigneeUserId;
@ -4139,9 +4150,13 @@ export function issueRoutes(
payload?: unknown;
},
) {
const isReviewConfirmationVerdict = await isPendingReviewConfirmationVerdict(issue, interaction);
if (req.actor.type !== "agent") {
assertBoard(req);
await assertPendingReviewInteractionVerdictAllowed(req, issue, interaction);
if (isReviewConfirmationVerdict) {
await assertPendingReviewInteractionVerdictAllowed(req, issue, interaction);
return "review_verdict" as const;
}
return "standard" as const;
}
const actorAgentId = req.actor.agentId;
@ -4154,7 +4169,6 @@ 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;
@ -4238,11 +4252,15 @@ export function issueRoutes(
createdByAgentId?: string | null;
createdByUserId?: string | null;
},
interaction: { status: string },
interaction: { kind: string; status: string },
) {
if (
issue.status !== "in_review"
|| interaction.status !== "pending"
|| (
interaction.kind !== "request_confirmation"
&& interaction.kind !== "request_checkbox_confirmation"
)
|| issue.reviewPolicy == null
|| issue.reviewPolicy === "anyone"
) return;
@ -6415,10 +6433,11 @@ export function issueRoutes(
? "owner_completed"
: outcome;
const updateFields = sourceIssueStatus ? { status: sourceIssueStatus } : {};
await assertAgentInReviewReviewPath({
await assertInReviewReviewPath({
existing,
updateFields,
actorType: req.actor.type,
actorType: actor.actorType,
actorId: actor.actorId,
actorAgentId: actor.agentId,
actorRunId: actor.runId,
});
@ -8764,19 +8783,25 @@ export function issueRoutes(
onBehalfOfUserId: _requestedOnBehalfOfUserId,
...updateFields
} = req.body;
const effectiveReviewPolicy = req.body.reviewPolicy === undefined
? existing.reviewPolicy
: req.body.reviewPolicy;
if (
const reviewPolicyChangeRequested =
req.body.reviewPolicy !== undefined
&& req.body.reviewPolicy !== existing.reviewPolicy;
const reviewVerdictRequested =
existing.status === "in_review"
&& (updateFields.status === "done" || updateFields.status === "cancelled")
&& effectiveReviewPolicy != null
&& effectiveReviewPolicy !== "anyone"
&& (updateFields.status === "done" || updateFields.status === "cancelled");
const reviewPolicySensitiveMutationRequested =
req.body.reviewPolicy !== undefined
|| updateFields.status === "done"
|| updateFields.status === "cancelled";
if (
(reviewVerdictRequested || reviewPolicyChangeRequested)
&& existing.reviewPolicy != null
&& existing.reviewPolicy !== "anyone"
) {
await assertIssueReviewVerdictActorAllowed(db, {
issue: existing,
actor: { type: actor.actorType, id: actor.actorId },
reviewPolicy: effectiveReviewPolicy,
reviewPolicy: existing.reviewPolicy,
});
}
const shouldCancelActiveRunForCancelledStatus =
@ -9097,14 +9122,19 @@ export function issueRoutes(
}
}
const reviewInteractionId = await assertAgentInReviewReviewPath({
const reviewInteractionId = await assertInReviewReviewPath({
existing,
updateFields,
actorType: req.actor.type,
actorType: actor.actorType,
actorId: actor.actorId,
actorAgentId: actor.agentId,
actorRunId: actor.runId,
reviewInteractionId: requestedReviewInteractionId,
});
const enteringReviewRequested =
existing.status !== "in_review" && updateFields.status === "in_review";
const persistReviewActivityTransactionally =
enteringReviewRequested || Boolean(reviewInteractionId);
const nextAssigneeAgentId =
updateFields.assigneeAgentId === undefined ? existing.assigneeAgentId : (updateFields.assigneeAgentId as string | null);
@ -9183,11 +9213,35 @@ export function issueRoutes(
? svc.update(id, issueUpdateData, db, postCommitActivityPublications)
: svc.update(id, issueUpdateData);
};
const persistBoundReviewActivity = async (
const assertLockedReviewPolicyAllowsMutation = async (
tx: Parameters<typeof svc.update>[2],
) => {
const lockedExisting = await svc.getByIdForUpdate(id, tx);
if (!lockedExisting) return false;
const lockedPolicyChangeRequested =
req.body.reviewPolicy !== undefined
&& req.body.reviewPolicy !== lockedExisting.reviewPolicy;
const lockedReviewVerdictRequested =
lockedExisting.status === "in_review"
&& (updateFields.status === "done" || updateFields.status === "cancelled");
if (
(lockedReviewVerdictRequested || lockedPolicyChangeRequested)
&& lockedExisting.reviewPolicy != null
&& lockedExisting.reviewPolicy !== "anyone"
) {
await assertIssueReviewVerdictActorAllowed(tx as unknown as Db, {
issue: lockedExisting,
actor: { type: actor.actorType, id: actor.actorId },
reviewPolicy: lockedExisting.reviewPolicy,
});
}
return true;
};
const persistReviewTransitionActivity = async (
tx: Parameters<typeof svc.update>[2],
updated: NonNullable<Awaited<ReturnType<typeof svc.update>>>,
) => {
if (!reviewInteractionId) return;
if (!persistReviewActivityTransactionally) return;
const changes = updated.changes ?? {};
const previous = Object.fromEntries(
Object.entries(changes).map(([key, change]) => [key, change.from]),
@ -9208,7 +9262,7 @@ export function issueRoutes(
identifier: updated.identifier,
authorizationReason: issueMutationAuthorizationReason,
changes,
reviewInteractionId,
...(reviewInteractionId ? { reviewInteractionId } : {}),
...(commentBody ? { source: "comment" } : {}),
...(resumeRequested === true ? { resumeIntent: true, followUpRequested: true } : {}),
...(interruptedRunId ? { interruptedRunId } : {}),
@ -9253,47 +9307,43 @@ export function issueRoutes(
generation: reopenedGeneration,
finalIssueStatus: () => issue?.status,
});
const decision = transition.decision && decisionId ? transition.decision : null;
const shouldUseTransactionalIssueUpdate =
Boolean(decision)
|| shouldRelayStop
|| persistReviewActivityTransactionally
|| reviewPolicySensitiveMutationRequested;
try {
if (transition.decision && decisionId) {
const decision = transition.decision;
if (shouldUseTransactionalIssueUpdate) {
issue = await db.transaction(async (tx) => {
if (
reviewPolicySensitiveMutationRequested
&& !(await assertLockedReviewPolicyAllowsMutation(tx))
) return null;
const updated = await updateIssue(tx);
if (!updated) return null;
await tx.insert(issueExecutionDecisions).values({
id: decisionId,
companyId: updated.companyId,
issueId: updated.id,
stageId: decision.stageId,
stageType: decision.stageType,
actorAgentId: actor.agentId ?? null,
actorUserId: actor.actorType === "user" ? actor.actorId : null,
outcome: decision.outcome,
body: decision.body,
createdByRunId: actor.runId ?? null,
});
if (decision && decisionId) {
await tx.insert(issueExecutionDecisions).values({
id: decisionId,
companyId: updated.companyId,
issueId: updated.id,
stageId: decision.stageId,
stageType: decision.stageType,
actorAgentId: actor.agentId ?? null,
actorUserId: actor.actorType === "user" ? actor.actorId : null,
outcome: decision.outcome,
body: decision.body,
createdByRunId: actor.runId ?? null,
});
}
if (shouldRelayStop) {
stopRelayResult.value = await svc.addStopRelayCommentIfNeeded(updated, tx);
}
await persistBoundReviewActivity(tx, updated);
await persistReviewTransitionActivity(tx, updated);
return updated;
});
} else if (shouldRelayStop) {
issue = await db.transaction(async (tx) => {
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 {
@ -9479,7 +9529,7 @@ export function issueRoutes(
activeRecoveryAction: null,
};
}
if (!reviewInteractionId) await logActivity(db, {
if (!persistReviewActivityTransactionally) await logActivity(db, {
companyId: issue.companyId,
actorType: actor.actorType,
actorId: actor.actorId,
@ -10633,7 +10683,7 @@ export function issueRoutes(
agentId: actor.agentId,
runId: actor.runId,
userId: actor.actorType === "user" ? actor.actorId : null,
...(actor.actorType === "agent" && resolutionAuthorization === "review_verdict"
...(resolutionAuthorization === "review_verdict"
? { reviewVerdictAuthorized: true }
: {}),
});
@ -10789,7 +10839,7 @@ export function issueRoutes(
agentId: actor.agentId,
runId: actor.runId,
userId: actor.actorType === "user" ? actor.actorId : null,
...(actor.actorType === "agent" && resolutionAuthorization === "review_verdict"
...(resolutionAuthorization === "review_verdict"
? { reviewVerdictAuthorized: true }
: {}),
});

18
server/src/services/issue-review-policy.ts
View File

@ -81,7 +81,17 @@ export async function isIssueReviewVerdictInteraction(
},
): Promise<boolean> {
const requester = await findReviewRequester(db, input.issue);
if (!requester?.reviewInteractionId || requester.reviewInteractionId !== input.interaction.id) return false;
if (!requester) return false;
if (requester.reviewInteractionId && requester.reviewInteractionId !== input.interaction.id) return false;
// Older review transitions did not persist the interaction binding. In that
// case, an unattributed confirmation is ambiguous and must fail closed.
// Confirmations attributed to an unrelated writer remain independently
// resolvable, while requester-created confirmations inherit the issue policy.
if (!requester.reviewInteractionId
&& !input.interaction.createdByAgentId
&& !input.interaction.createdByUserId) {
return true;
}
return requester.type === "agent"
? input.interaction.createdByAgentId === requester.id
: input.interaction.createdByUserId === requester.id;
@ -106,7 +116,7 @@ export async function assertIssueReviewVerdictActorAllowed(
code: "review_policy_denied",
policy,
allowedActor: "authenticated_user_with_issue_write_access",
remediation: "Have an authenticated user with issue write access submit the verdict, or change reviewPolicy to `anyone`.",
remediation: "Have an authenticated user with issue write access submit the verdict.",
},
);
}
@ -119,7 +129,7 @@ export async function assertIssueReviewVerdictActorAllowed(
code: "review_policy_denied",
policy,
allowedActor: "writer_other_than_review_requester",
remediation: "Change reviewPolicy to `anyone`, or move the issue out of and back into `in_review` to record a requester before another writer submits the verdict.",
remediation: "Move the issue out of and back into `in_review` to record a requester before another writer submits the verdict.",
},
);
}
@ -131,7 +141,7 @@ export async function assertIssueReviewVerdictActorAllowed(
code: "review_policy_denied",
policy,
allowedActor: "writer_other_than_review_requester",
remediation: "Have another writer with issue write access submit the verdict, or change reviewPolicy to `anyone`.",
remediation: "Have another writer with issue write access submit the verdict.",
},
);
}

224
server/src/services/issue-thread-interactions.ts
View File

@ -20,6 +20,7 @@ import type {
CancelIssueThreadInteraction,
CreateIssueThreadInteraction,
InteractionResolverGovernance,
IssueReviewPolicy,
IssueThreadInteraction,
IssueThreadInteractionKind,
IssueThreadInteractionResolverPolicy,
@ -59,6 +60,10 @@ import { conflict, forbidden, notFound, unprocessable } from "../errors.js";
import { getTelemetryClient } from "../telemetry.js";
import { logActivity } from "./activity-log.js";
import { evaluateAgentInvokabilityFromDb } from "./agent-invokability.js";
import {
assertIssueReviewVerdictActorAllowed,
isIssueReviewVerdictInteraction,
} from "./issue-review-policy.js";
import { issueService, runWorkspaceIsFinalized } from "./issues.js";
import {
createPullRequestMergeStateResolver,
@ -286,8 +291,48 @@ type IssueResolutionContext = {
status: string;
assigneeAgentId: string | null;
assigneeUserId: string | null;
reviewPolicy: IssueReviewPolicy | null;
createdByAgentId: string | null;
createdByUserId: string | null;
};
async function assertRequestConfirmationResolutionAllowedUnderLock(
tx: Db,
issue: IssueResolutionContext,
interaction: IssueThreadInteractionRow,
actor: InteractionActor,
) {
if (isTerminalIssueStatus(issue.status)) {
throw conflict("Interaction is no longer actionable because the issue is closed");
}
const isReviewVerdict = issue.status === "in_review"
&& isRequestConfirmationLikeKind(interaction.kind)
&& await isIssueReviewVerdictInteraction(tx, { issue, interaction });
if (!isReviewVerdict) {
assertAgentResolutionAllowed(interaction, {
...actor,
reviewVerdictAuthorized: false,
});
return;
}
if (actor.agentId) assertAgentInteractionActorAllowed(interaction, actor);
const verdictActor = actor.agentId
? { type: "agent" as const, id: actor.agentId }
: actor.userId
? { type: "user" as const, id: actor.userId }
: null;
if (!verdictActor) {
throw forbidden("A review verdict requires an authenticated agent or user");
}
await assertIssueReviewVerdictActorAllowed(tx, {
issue,
actor: verdictActor,
});
}
const REQUEST_CONFIRMATION_INTERACTION_KINDS = [
"request_confirmation",
"request_checkbox_confirmation",
@ -1330,17 +1375,62 @@ export function issueThreadInteractionService(db: Db, opts: IssueThreadInteracti
return { interaction: expired, continuationIssue: null };
}
const interaction = hydrateInteraction(args.current);
const selectedOptionIds =
interaction.kind === "request_checkbox_confirmation"
const now = new Date();
const result = await db.transaction(async (tx) => {
// Lock the issue before claiming the interaction. Policy mutations and
// review transitions use the same issue-row lock, so the authoritative
// review policy and requester are stable through the verdict write.
const issueContext = await tx
.select({
id: issues.id,
companyId: issues.companyId,
status: issues.status,
assigneeAgentId: issues.assigneeAgentId,
assigneeUserId: issues.assigneeUserId,
reviewPolicy: issues.reviewPolicy,
createdByAgentId: issues.createdByAgentId,
createdByUserId: issues.createdByUserId,
})
.from(issues)
.where(eq(issues.id, args.issue.id))
.for("update")
.then((rows: IssueResolutionContext[]) => rows[0] ?? null);
if (!issueContext || issueContext.companyId !== args.issue.companyId) {
throw notFound("Issue not found");
}
const lockedCurrent = await tx
.select()
.from(issueThreadInteractions)
.where(eq(issueThreadInteractions.id, args.current.id))
.for("update")
.then((rows) => rows[0] ?? null);
if (
!lockedCurrent
|| lockedCurrent.companyId !== args.issue.companyId
|| lockedCurrent.issueId !== args.issue.id
) {
throw notFound("Interaction not found");
}
if (lockedCurrent.status !== "pending") {
throw conflict("Interaction has already been resolved");
}
await assertRequestConfirmationResolutionAllowedUnderLock(
tx as unknown as Db,
issueContext,
lockedCurrent,
args.actor,
);
const interaction = hydrateInteraction(lockedCurrent);
const selectedOptionIds = interaction.kind === "request_checkbox_confirmation"
? resolveSelectedCheckboxConfirmationOptions({
interaction,
selectedOptionIds: args.input.selectedOptionIds,
})
: undefined;
const now = new Date();
const result = await db.transaction(async (tx) => {
const [updated] = await tx
.update(issueThreadInteractions)
.set({
@ -1357,7 +1447,7 @@ export function issueThreadInteractionService(db: Db, opts: IssueThreadInteracti
updatedAt: now,
})
.where(and(
eq(issueThreadInteractions.id, args.current.id),
eq(issueThreadInteractions.id, lockedCurrent.id),
eq(issueThreadInteractions.status, "pending"),
))
.returning();
@ -1366,32 +1456,16 @@ export function issueThreadInteractionService(db: Db, opts: IssueThreadInteracti
throw conflict("Interaction has already been resolved");
}
const issueContext = await tx
.select({
id: issues.id,
companyId: issues.companyId,
status: issues.status,
assigneeAgentId: issues.assigneeAgentId,
assigneeUserId: issues.assigneeUserId,
})
.from(issues)
.where(eq(issues.id, args.issue.id))
.then((rows: IssueResolutionContext[]) => rows[0] ?? null);
if (!issueContext || issueContext.companyId !== args.issue.companyId) {
throw notFound("Issue not found");
}
let continuationIssue: IssueWakeTarget | null = null;
if (shouldReturnAcceptedConfirmationToCreatorAgent({
issue: issueContext,
current: args.current,
current: lockedCurrent,
actor: args.actor,
})) {
const returnStatus = issueContext.status === "blocked" ? "blocked" : "todo";
const returnedIssue = await issueService(db).update(args.issue.id, {
status: returnStatus,
assigneeAgentId: args.current.createdByAgentId,
assigneeAgentId: lockedCurrent.createdByAgentId,
assigneeUserId: null,
actorAgentId: args.actor.agentId ?? null,
actorUserId: args.actor.userId ?? null,
@ -1420,12 +1494,12 @@ export function issueThreadInteractionService(db: Db, opts: IssueThreadInteracti
entityType: "issue",
entityId: args.issue.id,
details: {
interactionId: args.current.id,
interactionKind: args.current.kind,
interactionId: lockedCurrent.id,
interactionKind: lockedCurrent.kind,
interactionStatus: "accepted",
resolutionActorKind: "system",
requestedResolverPolicy: args.current.requestedResolverPolicy,
effectiveResolverPolicy: args.current.effectiveResolverPolicy,
requestedResolverPolicy: lockedCurrent.requestedResolverPolicy,
effectiveResolverPolicy: lockedCurrent.effectiveResolverPolicy,
...(args.actor.resolutionDetails ?? {}),
},
});
@ -1461,31 +1535,77 @@ export function issueThreadInteractionService(db: Db, opts: IssueThreadInteracti
}
const now = new Date();
const [updated] = await db
.update(issueThreadInteractions)
.set({
status: "rejected",
result: {
version: 1,
outcome: "rejected",
reason: reason || null,
},
resolvedByAgentId: args.actor.agentId ?? null,
resolvedByRunId: args.actor.runId ?? null,
resolvedByUserId: args.actor.userId ?? null,
resolvedAt: now,
updatedAt: now,
})
.where(and(
eq(issueThreadInteractions.id, args.current.id),
eq(issueThreadInteractions.status, "pending"),
))
.returning();
const updated = await db.transaction(async (tx) => {
const issueContext = await tx
.select({
id: issues.id,
companyId: issues.companyId,
status: issues.status,
assigneeAgentId: issues.assigneeAgentId,
assigneeUserId: issues.assigneeUserId,
reviewPolicy: issues.reviewPolicy,
createdByAgentId: issues.createdByAgentId,
createdByUserId: issues.createdByUserId,
})
.from(issues)
.where(eq(issues.id, args.issue.id))
.for("update")
.then((rows: IssueResolutionContext[]) => rows[0] ?? null);
if (!issueContext || issueContext.companyId !== args.issue.companyId) {
throw notFound("Issue not found");
}
const lockedCurrent = await tx
.select()
.from(issueThreadInteractions)
.where(eq(issueThreadInteractions.id, args.current.id))
.for("update")
.then((rows) => rows[0] ?? null);
if (
!lockedCurrent
|| lockedCurrent.companyId !== args.issue.companyId
|| lockedCurrent.issueId !== args.issue.id
) {
throw notFound("Interaction not found");
}
if (lockedCurrent.status !== "pending") {
throw conflict("Interaction has already been resolved");
}
await assertRequestConfirmationResolutionAllowedUnderLock(
tx as unknown as Db,
issueContext,
lockedCurrent,
args.actor,
);
const [resolved] = await tx
.update(issueThreadInteractions)
.set({
status: "rejected",
result: {
version: 1,
outcome: "rejected",
reason: reason || null,
},
resolvedByAgentId: args.actor.agentId ?? null,
resolvedByRunId: args.actor.runId ?? null,
resolvedByUserId: args.actor.userId ?? null,
resolvedAt: now,
updatedAt: now,
})
.where(and(
eq(issueThreadInteractions.id, lockedCurrent.id),
eq(issueThreadInteractions.status, "pending"),
))
.returning();
if (!resolved) {
throw conflict("Interaction has already been resolved");
}
await touchIssue(tx, args.issue.id);
return resolved;
});
if (!updated) {
throw conflict("Interaction has already been resolved");
}
await touchIssue(db, args.issue.id);
const rejected = hydrateInteraction(updated);
await emitInteractionResolvedTelemetry(db, rejected);
return rejected;

9
server/src/services/issues.ts
View File

@ -5893,6 +5893,15 @@ export function issueService(db: Db) {
return getIssueByUuid(id);
},
getByIdForUpdate: async (id: string, dbOrTx: any) => {
return dbOrTx
.select()
.from(issues)
.where(eq(issues.id, id))
.for("update")
.then((rows: Array<typeof issues.$inferSelect>) => rows[0] ?? null);
},
getByIdentifier: async (identifier: string) => {
return getIssueByIdentifier(identifier);
},