mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-14 17:40:37 +02:00
fix: honor Stop revocation when persisting agent feedback
Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
1 parent
71e9bac392
commit
5209c3501d
2 files changed
+25
-1
No files matched your search
@@ -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()}`;
|
||||
|
||||
@@ -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)
|
||||
|
||||
Reference in new issue
Block a user