mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-11 05:11:49 +01:00
Consume Reflection Coach change approvals
Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
1 parent
cbd9015791
commit
ed39a0c504
2 files changed
+120
-1
No files matched your search
@@ -1,5 +1,6 @@
|
||||
import { randomUUID } from "node:crypto";
|
||||
import { afterAll, afterEach, beforeAll, describe, expect, it } from "vitest";
|
||||
import { eq } from "drizzle-orm";
|
||||
import {
|
||||
agents,
|
||||
companies,
|
||||
@@ -133,6 +134,50 @@ describeEmbeddedPostgres("changeConsentGateService", () => {
|
||||
});
|
||||
|
||||
it("allows a previous-run accepted interaction with a displayed diff for the bound target", async () => {
|
||||
const { companyId, coachId, sourceRunId, proposalIssueId, targetKey } = await seedGateFixture();
|
||||
const interactionId = randomUUID();
|
||||
await db.insert(issueThreadInteractions).values({
|
||||
id: interactionId,
|
||||
companyId,
|
||||
issueId: proposalIssueId,
|
||||
kind: "request_confirmation",
|
||||
status: "accepted",
|
||||
continuationPolicy: "wake_assignee_on_accept",
|
||||
sourceRunId,
|
||||
createdByAgentId: coachId,
|
||||
payload: {
|
||||
version: 1,
|
||||
prompt: "Apply this Reflection Coach skill diff?",
|
||||
detailsMarkdown: "```diff\n+Tighten the workflow.\n```",
|
||||
target: { type: "custom", key: targetKey, revisionId: "proposal-v1" },
|
||||
},
|
||||
result: { version: 1, outcome: "accepted" },
|
||||
resolvedByUserId: "board-user",
|
||||
resolvedAt: new Date(),
|
||||
});
|
||||
const actorRunId = randomUUID();
|
||||
|
||||
await expect(changeConsentGateService(db).assertConsented({
|
||||
companyId,
|
||||
actorAgentId: coachId,
|
||||
actorRunId,
|
||||
targetKeys: [targetKey],
|
||||
})).resolves.toBe(true);
|
||||
|
||||
const [stored] = await db
|
||||
.select({ result: issueThreadInteractions.result })
|
||||
.from(issueThreadInteractions)
|
||||
.where(eq(issueThreadInteractions.id, interactionId));
|
||||
|
||||
expect(stored?.result).toMatchObject({
|
||||
consumedByRunId: actorRunId,
|
||||
outcome: "accepted",
|
||||
version: 1,
|
||||
});
|
||||
expect((stored?.result as { consumedAt?: unknown } | undefined)?.consumedAt).toEqual(expect.any(String));
|
||||
});
|
||||
|
||||
it("rejects reusing an accepted interaction after it is consumed by a mutation", async () => {
|
||||
const { companyId, coachId, sourceRunId, proposalIssueId, targetKey } = await seedGateFixture();
|
||||
await db.insert(issueThreadInteractions).values({
|
||||
id: randomUUID(),
|
||||
@@ -160,6 +205,16 @@ describeEmbeddedPostgres("changeConsentGateService", () => {
|
||||
actorRunId: randomUUID(),
|
||||
targetKeys: [targetKey],
|
||||
})).resolves.toBe(true);
|
||||
|
||||
await expect(changeConsentGateService(db).assertConsented({
|
||||
companyId,
|
||||
actorAgentId: coachId,
|
||||
actorRunId: randomUUID(),
|
||||
targetKeys: [targetKey],
|
||||
})).rejects.toMatchObject({
|
||||
status: 403,
|
||||
details: { code: "reflection_coach_mutation_gate_required" },
|
||||
});
|
||||
});
|
||||
|
||||
it("allows legacy Reflection Coach target keys for durable accepted interactions", async () => {
|
||||
|
||||
@@ -6,6 +6,11 @@ import { forbidden } from "../errors.js";
|
||||
|
||||
export const AGENT_PROFILE_CHANGE_CONSENT_FIELDS = ["name", "role", "title", "capabilities"] as const;
|
||||
|
||||
type ConsumedRequestConfirmationResult = RequestConfirmationResult & {
|
||||
consumedAt?: string | null;
|
||||
consumedByRunId?: string | null;
|
||||
};
|
||||
|
||||
export function agentInstructionsChangeTargetKey(agentId: string) {
|
||||
return `agent:${agentId}:instructions`;
|
||||
}
|
||||
@@ -47,6 +52,23 @@ function payloadHasDisplayedDiff(payload: RequestConfirmationPayload) {
|
||||
return /(^|\n)[+-][^\n]+/.test(details);
|
||||
}
|
||||
|
||||
function requestConfirmationResultConsumed(result: RequestConfirmationResult | null) {
|
||||
const consumed = result as ConsumedRequestConfirmationResult | null;
|
||||
return Boolean(readNonEmptyString(consumed?.consumedByRunId) || readNonEmptyString(consumed?.consumedAt));
|
||||
}
|
||||
|
||||
function markRequestConfirmationResultConsumed(
|
||||
result: RequestConfirmationResult,
|
||||
actorRunId: string,
|
||||
consumedAt: Date,
|
||||
): ConsumedRequestConfirmationResult {
|
||||
return {
|
||||
...result,
|
||||
consumedAt: consumedAt.toISOString(),
|
||||
consumedByRunId: actorRunId,
|
||||
};
|
||||
}
|
||||
|
||||
function legacyTargetKeysFor(targetKey: string) {
|
||||
if (targetKey.startsWith("agent:") && targetKey.endsWith(":instructions")) {
|
||||
const agentId = targetKey.slice("agent:".length, -":instructions".length);
|
||||
@@ -146,6 +168,7 @@ export function changeConsentGateService(db: Db) {
|
||||
return payload.target?.type === "custom"
|
||||
&& queryTargetKeys.includes(payload.target.key)
|
||||
&& result?.outcome === "accepted"
|
||||
&& !requestConfirmationResultConsumed(result)
|
||||
&& payloadHasDisplayedDiff(payload)
|
||||
&& Boolean(row.sourceRunId)
|
||||
&& row.sourceRunId !== actorRunId;
|
||||
@@ -154,7 +177,48 @@ export function changeConsentGateService(db: Db) {
|
||||
if (!accepted) {
|
||||
throw forbidden(
|
||||
"Reflection Coach mutations require an accepted request_confirmation with a displayed diff for this target, "
|
||||
+ "created in a previous run.",
|
||||
+ "created in a previous run and not already consumed.",
|
||||
{
|
||||
code: "reflection_coach_mutation_gate_required",
|
||||
targetKeys,
|
||||
},
|
||||
);
|
||||
}
|
||||
|
||||
const acceptedResult = accepted.result as RequestConfirmationResult | null;
|
||||
if (!acceptedResult) {
|
||||
throw forbidden(
|
||||
"Reflection Coach mutations require an accepted request_confirmation with a displayed diff for this target, "
|
||||
+ "created in a previous run and not already consumed.",
|
||||
{
|
||||
code: "reflection_coach_mutation_gate_required",
|
||||
targetKeys,
|
||||
},
|
||||
);
|
||||
}
|
||||
|
||||
const now = new Date();
|
||||
const [consumed] = await db
|
||||
.update(issueThreadInteractions)
|
||||
.set({
|
||||
result: markRequestConfirmationResultConsumed(acceptedResult, actorRunId, now),
|
||||
updatedAt: now,
|
||||
})
|
||||
.where(and(
|
||||
eq(issueThreadInteractions.id, accepted.id),
|
||||
eq(issueThreadInteractions.companyId, input.companyId),
|
||||
eq(issueThreadInteractions.createdByAgentId, actorAgentId),
|
||||
eq(issueThreadInteractions.kind, "request_confirmation"),
|
||||
eq(issueThreadInteractions.status, "accepted"),
|
||||
sql`${issueThreadInteractions.result}->>'outcome' = 'accepted'`,
|
||||
sql`coalesce(${issueThreadInteractions.result}->>'consumedByRunId', ${issueThreadInteractions.result}->>'consumedAt') is null`,
|
||||
))
|
||||
.returning({ id: issueThreadInteractions.id });
|
||||
|
||||
if (!consumed) {
|
||||
throw forbidden(
|
||||
"Reflection Coach mutations require an accepted request_confirmation with a displayed diff for this target, "
|
||||
+ "created in a previous run and not already consumed.",
|
||||
{
|
||||
code: "reflection_coach_mutation_gate_required",
|
||||
targetKeys,
|
||||
|
||||
Reference in new issue
Block a user