fix(interactions): enforce task access and current review policy on dismissal

This commit is contained in:
Dotta committed 2026-10-22 16:27:36 -07:00
1 parent e243cbb001
commit 9e1b4478b3
7 files changed
+77 -10

No files matched your search

+2 -1
View File
@@ -1891,7 +1891,8 @@ at their original position in the feed. Dismissing any composer request clears
its form and pending reminder, including completion reviews, confirmations,
checkbox choices, suggested tasks, item verdicts, and runtime requests. Saved
interactions persist `dismissedAt` and `dismissedByUserId` through a board-only,
company-scoped endpoint with resolver-audience checks and an activity record.
company-scoped endpoint with task-access, resolver-audience, and current review-policy
checks and an activity record. Review policy is checked under the task-row lock.
Dismissal never creates an answer, decision, execution, or continuation wake.
Raw runtime requests retain browser-local dismissal; reopening restores the
original request and draft. A failed save leaves the request open with an error.
@@ -134,11 +134,11 @@ API: `present` — POST /api/issues/{id}/interactions/{interactionId}/dismiss.
Agent access: `restricted` — board-only; an agent cannot dismiss its own pending request. Authorized agents can read dismissal in interaction/task context.
Policy: Task/company access and resolver audience apply. Saves dismissal under task/interaction locks without resolving, executing, or waking. Ordinary requests stop gating completion. Tool/secret/connection/document/company-policy and configured review gates retain authority, with no repeated reminder. Raw runtime dismissal remains a browser preference.
Policy: Task/company read access and resolver audience apply. Current issue review policy is rechecked under the task lock; an excluded review requester cannot defer the review. Saves dismissal under task/interaction locks without resolving, executing, or waking. Ordinary requests stop gating completion. Tool/secret/connection/document/company-policy and configured review gates retain authority, with no repeated reminder. Raw runtime dismissal remains a browser preference.
Evidence: `source-only` — [route](../../server/src/routes/issues.ts), [service](../../server/src/services/issue-thread-interactions.ts), [completion predicate](../../server/src/services/issue-question-context.ts), and [task thread](../../ui/src/components/TaskChatThread.tsx).
Verification: `not-run` — Dismiss, reload, reopen the original request; verify persisted dismissal with no answer, approval, or wake. Reject agents, foreign tasks, and wrong audiences. Check ordinary completion review and protected tool action separately.
Verification: `not-run` — Dismiss, reload, reopen the original request; verify persisted dismissal with no answer, approval, or wake. Reject agents, foreign or unreadable private tasks, wrong audiences, and review requesters excluded by current policy. Check ordinary completion review and protected tool action separately.
Gaps: Browser drafts and raw runtime request dismissal do not synchronize across devices.
@@ -261,11 +261,10 @@ function createIssue(overrides: Record<string, unknown> = {}) {
};
}
function loadAppModules() {
return Promise.all([
import("../routes/issues.js"),
import("../middleware/index.js"),
]);
async function loadAppModules() {
const routes = await import("../routes/issues.js");
const middleware = await import("../middleware/index.js");
return [routes, middleware] as const;
}
async function createApp(actor: Record<string, unknown> = {
@@ -629,6 +628,23 @@ describe("issue thread interaction routes", () => {
expect(mockInteractionService.dismissInteraction).not.toHaveBeenCalled();
});
it("hides private task requests from same-company members without task access before dismissal", async () => {
mockAccessDecide.mockResolvedValue({ allowed: false, action: "issue:read", reason: "deny_issue_private",
explanation: "Task access was revoked.",
});
const app = await createApp({ type: "board", userId: "same-company-member", companyIds: ["company-1"],
source: "session", isInstanceAdmin: false,
});
const result = await request(app)
.post("/api/issues/aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa/interactions/request-1/dismiss").send({});
expect(result.status).toBe(404);
expect(result.body).toEqual({ error: "Issue not found" });
expect(mockInteractionService.dismissInteraction).not.toHaveBeenCalled();
expect(mockInteractionService.getForIssue).not.toHaveBeenCalled();
expect(mockLogActivity).not.toHaveBeenCalled();
expect(mockHeartbeatService.wakeup).not.toHaveBeenCalled();
});
it("normalizes a canonical-only mixed question form at the HTTP boundary", async () => {
const questionSet = { schema: "paperclip.question_set.v1", questions: [
{ id: "repo", prompt: "Repository URL?", required: true, answerMode: "text" },
+1
View File
@@ -17691,6 +17691,7 @@ export function issueRoutes(
assertBoard(req);
const issue = await getAccessibleResource(req, res, svc.getById(req.params.id as string), "Issue not found");
if (!issue) return;
if (!(await assertIssueReadAllowed(req, res, issue))) return;
const actor = getActorInfo(req);
const interaction = await issueThreadInteractionService(db).dismissInteraction(
issue, req.params.interactionId as string,
@@ -5111,7 +5111,18 @@ export function issueThreadInteractionService(
return db.transaction(async (tx) => {
// Use the same lock order as resolution and completion. Dismissal only
// changes attention state; it never resolves or executes the request.
const [task] = await tx.select({ id: issues.id }).from(issues)
const [task] = await tx.select({
id: issues.id,
companyId: issues.companyId,
status: issues.status,
executionRunId: issues.executionRunId,
workMode: issues.workMode,
assigneeAgentId: issues.assigneeAgentId,
assigneeUserId: issues.assigneeUserId,
reviewPolicy: issues.reviewPolicy,
createdByAgentId: issues.createdByAgentId,
createdByUserId: issues.createdByUserId,
}).from(issues)
.where(and(eq(issues.id, issue.id), eq(issues.companyId, issue.companyId))).for("update");
if (!task) throw interactionNotFoundError();
const [current] = await tx.select().from(issueThreadInteractions).where(and(
@@ -5121,6 +5132,9 @@ export function issueThreadInteractionService(
if (!current) throw interactionNotFoundError();
assertInteractionResolutionAllowed(current, actor);
if (current.status !== "pending" || current.dismissedAt) return hydrateInteraction(current);
if (task.status === "in_review" && isRequestConfirmationLikeKind(current.kind)) {
await assertRequestConfirmationResolutionAllowedUnderLock(tx as unknown as Db, task, current, actor);
}
const now = new Date();
const [updated] = await tx.update(issueThreadInteractions)
.set({ dismissedAt: now, dismissedByUserId: actor.userId, updatedAt: now })
@@ -2,7 +2,7 @@ import { randomUUID } from "node:crypto";
import { eq, sql } from "drizzle-orm";
import { afterAll, beforeAll, describe, expect, it } from "vitest";
import {
agents, agentWakeupRequests, approvals, companies, completionContracts, createDb, heartbeatRuns, issueApprovals,
activityLog, agents, agentWakeupRequests, approvals, companies, completionContracts, createDb, heartbeatRuns, issueApprovals,
issueComments, issueQuestionResponseDeliveries, issueThreadInteractions, issueWorkProducts, issues, statusDecisions,
} from "@paperclipai/db";
import { startEmbeddedPostgresTestDatabase } from "../../__tests__/helpers/embedded-postgres.js";
@@ -120,6 +120,32 @@ describe("historical task questions", () => {
expect(feedback).not.toContain("Tell the user");
});
it.each(["request_confirmation", "request_checkbox_confirmation"])("enforces the current issue review policy when dismissing a %s", async kind => {
const f = await fixture();
await db.update(issues).set({ status: "in_review", reviewPolicy: "not_creator", createdByUserId: "original-author" })
.where(eq(issues.id, f.issueId));
await db.update(issueThreadInteractions).set({ kind, createdByAgentId: null, createdByUserId: "review-requester",
payload: kind === "request_confirmation"
? { version: 1, prompt: "Review completed work", target: { type: "custom", key: "native_completion_review" } }
: { version: 1, prompt: "Review completed work", options: [{ id: "verified", label: "Verified" }] },
}).where(eq(issueThreadInteractions.id, f.question.id));
await db.insert(activityLog).values({ companyId: f.companyId, actorType: "user", actorId: "review-requester",
action: "issue.updated", entityType: "issue", entityId: f.issueId,
details: { status: "in_review", _previous: { status: "in_progress" }, reviewInteractionId: f.question.id },
});
const service = issueThreadInteractionService(db);
const staleTask = { id: f.issueId, companyId: f.companyId, status: "in_progress", reviewPolicy: "anyone" };
await expect(service.dismissInteraction(staleTask, f.question.id, { userId: "review-requester" }))
.rejects.toMatchObject({ status: 403, details: { code: "review_policy_denied" } });
expect((await db.select().from(issueThreadInteractions).where(eq(issueThreadInteractions.id, f.question.id)))[0])
.toMatchObject({ dismissedAt: null, status: "pending", resolvedAt: null, result: null });
expect(await gate(f)).toEqual({ kind: "interaction", id: f.question.id });
expect(await service.dismissInteraction(staleTask, f.question.id, { userId: "original-author" }))
.toMatchObject({ dismissedByUserId: "original-author", status: "pending", resolvedAt: null, result: null });
expect(await gate(f)).toBeNull();
expect(await db.select().from(agentWakeupRequests).where(eq(agentWakeupRequests.companyId, f.companyId))).toEqual([]);
});
it("keeps a real current question gated without commanding another reminder", async () => {
const f = await fixture();
expect(await gate(f)).toEqual({ kind: "interaction", id: f.question.id });
+9
View File
@@ -69,6 +69,10 @@ const review = interactions.find(i => i.payload?.toolAction);
if (review?.status === "pending") { console.log("Still waiting for the existing review; no retry."); process.exit(0); }
if (review && review.status !== "pending") {
const result = review.result?.toolAction;
if (review.status === "accepted" && (!result || ["approved", "executing"].includes(result.status))) {
console.log("Review accepted; waiting for the stored execution result. No retry.");
process.exit(0);
}
const message = review.status === "rejected" ? "Review declined. No pages were read." : result?.status === "failed" ? "Read failed: " + result.errorMessage : ["expired", "cancelled"].includes(review.status) ? "The review expired or was cancelled. No pages were read." : describePages(result?.resultSummary);
await api("/issues/" + issueId + "/comments", "POST", { body: message });
await api("/issues/" + issueId, "PATCH", { status: "done" });
@@ -212,6 +216,11 @@ for (const journey of [
await expect(
page.getByRole("button", { name: "Approve & run", exact: true }),
).not.toBeVisible();
await page.reload();
await expect(page.getByRole("button", { name: "Review request", exact: true })).toBeVisible({ timeout: 30_000 });
await expect(
page.getByRole("button", { name: "Approve & run", exact: true }),
).not.toBeVisible();
await json(
await request.post(`/api/issues/${issue.id}/comments`, {
data: { body: "Keeping this review pending while I check." },