From 5209c3501dd1eff41ab4a052eaecac44a1211a95 Mon Sep 17 00:00:00 2001 From: Dotta Date: Tue, 6 Oct 2026 13:04:59 -0500 Subject: [PATCH] fix: honor Stop revocation when persisting agent feedback Co-Authored-By: Paperclip --- .../agent-commentary.integration.test.ts | 24 ++++++++++++++++++- server/src/services/agent-commentary.ts | 2 ++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/server/src/__tests__/agent-commentary.integration.test.ts b/server/src/__tests__/agent-commentary.integration.test.ts index 175f158e2e..77f5956f01 100644 --- a/server/src/__tests__/agent-commentary.integration.test.ts +++ b/server/src/__tests__/agent-commentary.integration.test.ts @@ -7,7 +7,7 @@ import { fileURLToPath } from "node:url"; import { and, eq, sql } from "drizzle-orm"; import express from "express"; import request from "supertest"; -import { activityLog, agentApiKeys, agentCommentary, agents, heartbeatRuns, issues, nativeRunResults } from "@paperclipai/db"; +import { activityLog, agentApiKeys, agentCommentary, agents, authUsers, companyMemberships, heartbeatRuns, issues, nativeRunResults } from "@paperclipai/db"; import { afterAll, beforeAll, describe, expect, it } from "vitest"; import { createLocalAgentJwt } from "../agent-auth-jwt.js"; import { submitAgentCommentary } from "../services/agent-commentary.js"; @@ -141,6 +141,28 @@ describe("internal agent commentary through both transports", () => { expect(await rows(f)).toHaveLength(0); }); + it.each(["jwt", "api_key"])("rejects new feedback and replay with %s after Stop revokes a still-running legacy run", async (credential) => { + const f = await legacy(); + if (credential === "api_key") { + f.token = `pcp_${randomUUID()}`; + const responsibleUserId = randomUUID(); + await server.db.insert(authUsers).values({ id: responsibleUserId, name: "Feedback key owner", email: `${responsibleUserId}@fixture.invalid`, emailVerified: true, createdAt: new Date(), updatedAt: new Date() }); + await server.db.insert(companyMemberships).values({ companyId: f.companyId, principalType: "user", principalId: responsibleUserId, status: "active", membershipRole: "member" }); + await server.db.insert(agentApiKeys).values({ companyId: f.companyId, agentId: f.agentId, responsibleUserId, name: "feedback", keyHash: createHash("sha256").update(f.token).digest("hex") }); + } + expect((await post(f, { ...input, kind: "complaint" })).status).toBe(201); + await server.db.update(heartbeatRuns).set({ resultJson: { executionCancellation: { state: "requested" } } }).where(eq(heartbeatRuns.id, f.runId)); + for (const idempotencyKey of [input.idempotencyKey, "after-stop"]) { + const body = { ...input, kind: "complaint", idempotencyKey }; + expect((await post(f, body)).status).toBe(403); + // The persistence boundary must also reject a request whose middleware + // authenticated before Stop committed the revocation. + await expect(submitAgentCommentary(server.db, f, body)).rejects.toThrow(/no longer authorized/); + } + expect(await rows(f)).toHaveLength(1); + expect(await server.db.select().from(activityLog).where(and(eq(activityLog.runId, f.runId), eq(activityLog.action, "agent.commentary_submitted")))).toHaveLength(1); + }); + it("rejects revoked legacy credentials, replaced run ownership, and specialized review tools", async () => { const f = await legacy(); const token = `pcp_${randomUUID()}`; diff --git a/server/src/services/agent-commentary.ts b/server/src/services/agent-commentary.ts index 7afdc8bd0a..aac31ee9c1 100644 --- a/server/src/services/agent-commentary.ts +++ b/server/src/services/agent-commentary.ts @@ -6,6 +6,7 @@ import { badRequest, conflict, forbidden, HttpError } from "../errors.js"; import { redactSensitiveText } from "../redaction.js"; import { persistActivity, publishActivity } from "./activity-log.js"; import { createRunSecretRedactionRegistry } from "./run-secret-redaction.js"; +import { agentRunWritesRevoked } from "../agent-run-cancellation.js"; type CommentaryActor = { companyId: string; agentId: string; runId: string; agentApiKeyId?: string | null }; @@ -45,6 +46,7 @@ export async function submitAgentCommentary( .innerJoin(agents, and(eq(agents.id, heartbeatRuns.agentId), eq(agents.companyId, heartbeatRuns.companyId))) .where(predicate).for("update"); if (!context || context.run.status !== "running" + || agentRunWritesRevoked(context.run) || ["paused", "terminated", "pending_approval", "error"].includes(context.agent.status) || boundIssue(context.run) !== issueId || (issueId && !issue)