mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-21 08:22:20 +02:00
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Operators open tasks to read progress and edit reviewers and approvers. > - Stored execution policies can contain a monitor and omit the stages array. > - These policies previously crashed the task properties panel. > - The policy reader now applies schema defaults through the fix in #15593. > - This pull request adds regression coverage for monitor-only policies with external service metadata. > - The tests check that the panel renders and participant edits keep the existing monitor. ## Linked Issues or Issue Description **What happened?** A task with a monitor-only execution policy could crash with `Cannot read properties of undefined (reading 'find')`. The stored policy omitted `stages`. **Expected behavior** The task shows no reviewers or approvers. Adding both kinds of participant keeps the monitor and its external service metadata. **Steps to reproduce** 1. Load a task whose execution policy contains an external service monitor and omits `stages`. 2. Render its properties panel and inspect both participant controls. 3. Add reviewers and approvers. Check that the resulting policy keeps the monitor. Refs #15593, which provides the policy normalization used by these tests. ## What Changed - Test that both stage types return empty participant lists for a monitor-only policy. - Test adding reviewers and approvers while keeping the external service monitor. - Test retaining a monitor when no review or approval stages exist. - Render the properties panel with omitted stages and check its participant and monitor controls. ## Verification - The helper and properties panel suites pass on current master with these tests: 108 tests. - `pnpm check:token-gates` passes. - All CI gates pass on the updated PR commit, including build, types, Runner verification, full test suites, browser suites, security scans, and canary checks. - Greptile gives commit `2634bd1d` 5/5 with no actionable findings or unresolved review threads. - The full local workspace suite was not repeated for this test-only rebase. CI verifies the full suite, build, types, Runner checks, browser suites, security scans, and canary run. ## Risks - Low risk. This change adds tests against the existing policy behavior. - The fixture uses example external references and test participant IDs. - Existing documentation remains accurate for this regression coverage. ## Model Used - OpenAI Codex, based on GPT-6, with tool use and code execution. The exact serving model ID and context window were not exposed in this session. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge Co-Authored-By: Paperclip <noreply@paperclip.ing>
186 lines
7.5 KiB
TypeScript
186 lines
7.5 KiB
TypeScript
import { afterEach, describe, expect, it, vi } from "vitest";
|
|
import { issueExecutionPolicySchema, type IssueExecutionPolicy } from "@paperclipai/shared";
|
|
import { buildExecutionPolicy, stageParticipantValues } from "./issue-execution-policy";
|
|
|
|
const AGENT_ID = "00000000-0000-4000-8000-000000000001";
|
|
const UUID_PATTERN = /^[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i;
|
|
const STAGE_ID = "00000000-0000-4000-8000-000000000002";
|
|
const PARTICIPANT_ID = "00000000-0000-4000-8000-000000000003";
|
|
|
|
describe("reading execution policy defaults", () => {
|
|
it.each([
|
|
{},
|
|
{ authorizationPolicy: { trustPreset: "standard" } },
|
|
{ monitor: { nextCheckAt: "2099-01-01T00:00:00.000Z" } },
|
|
{ stages: [{ type: "review" }] },
|
|
])("reads schema-valid omitted collections without changing the input: %j", (input) => {
|
|
expect(issueExecutionPolicySchema.safeParse(input).success).toBe(true);
|
|
const before = structuredClone(input);
|
|
expect(stageParticipantValues(input as IssueExecutionPolicy, "review")).toEqual([]);
|
|
expect(stageParticipantValues(input as IssueExecutionPolicy, "approval")).toEqual([]);
|
|
expect(input).toEqual(before);
|
|
});
|
|
|
|
it("retains configured reviewers when an unrelated stage omits participants", () => {
|
|
const policy = {
|
|
stages: [
|
|
{ type: "approval" },
|
|
{ id: STAGE_ID, type: "review", participants: [
|
|
{ id: PARTICIPANT_ID, type: "agent", agentId: AGENT_ID },
|
|
{ type: "user", userId: "board-reviewer" },
|
|
] },
|
|
],
|
|
} as IssueExecutionPolicy;
|
|
expect(stageParticipantValues(policy, "review")).toEqual([`agent:${AGENT_ID}`, "user:board-reviewer"]);
|
|
expect(stageParticipantValues(policy, "approval")).toEqual([]);
|
|
});
|
|
});
|
|
|
|
// Persisted monitor-only policies can omit fields supplied by schema defaults.
|
|
const monitorOnlyPolicy = {
|
|
monitor: {
|
|
nextCheckAt: "2026-10-08T16:00:00.000Z",
|
|
notes: "Check pull request",
|
|
scheduledBy: "assignee",
|
|
kind: "external_service",
|
|
serviceName: "github-pr",
|
|
externalRef: "https://github.com/example/repo/pull/1",
|
|
maxAttempts: 5,
|
|
},
|
|
} as IssueExecutionPolicy;
|
|
|
|
describe("stageParticipantValues", () => {
|
|
it.each(["review", "approval"] as const)("reads %s participants from a monitor-only policy", (stageType) => {
|
|
expect(issueExecutionPolicySchema.safeParse(monitorOnlyPolicy).success).toBe(true);
|
|
expect(stageParticipantValues(monitorOnlyPolicy, stageType)).toEqual([]);
|
|
});
|
|
});
|
|
|
|
describe("buildExecutionPolicy", () => {
|
|
afterEach(() => {
|
|
vi.unstubAllGlobals();
|
|
});
|
|
|
|
it("adds review and approval stages to a monitor-only policy without losing its monitor", () => {
|
|
const policy = buildExecutionPolicy({
|
|
existingPolicy: monitorOnlyPolicy,
|
|
reviewerValues: [`agent:${AGENT_ID}`],
|
|
approverValues: ["user:local-board"],
|
|
});
|
|
|
|
expect(issueExecutionPolicySchema.safeParse(policy).success).toBe(true);
|
|
expect(policy?.monitor).toEqual(monitorOnlyPolicy.monitor);
|
|
expect(stageParticipantValues(policy, "review")).toEqual([`agent:${AGENT_ID}`]);
|
|
expect(stageParticipantValues(policy, "approval")).toEqual(["user:local-board"]);
|
|
});
|
|
|
|
it("keeps a monitor-only policy when there are no review or approval stages", () => {
|
|
expect(buildExecutionPolicy({
|
|
existingPolicy: monitorOnlyPolicy,
|
|
reviewerValues: [],
|
|
approverValues: [],
|
|
})).toEqual({
|
|
mode: "normal",
|
|
commentRequired: true,
|
|
stages: [],
|
|
monitor: monitorOnlyPolicy.monitor,
|
|
});
|
|
});
|
|
|
|
it("generates schema-valid UUIDs when crypto.randomUUID is unavailable", () => {
|
|
vi.stubGlobal("crypto", {
|
|
getRandomValues: (bytes: Uint8Array) => {
|
|
for (let index = 0; index < bytes.length; index += 1) {
|
|
bytes[index] = index;
|
|
}
|
|
return bytes;
|
|
},
|
|
});
|
|
|
|
const policy = buildExecutionPolicy({
|
|
existingPolicy: null,
|
|
reviewerValues: [`agent:${AGENT_ID}`],
|
|
approverValues: ["user:local-board"],
|
|
});
|
|
|
|
expect(policy).not.toBeNull();
|
|
expect(issueExecutionPolicySchema.safeParse(policy).success).toBe(true);
|
|
expect(policy?.stages).toHaveLength(2);
|
|
|
|
for (const stage of policy?.stages ?? []) {
|
|
expect(stage.id).toMatch(UUID_PATTERN);
|
|
expect(stage.participants).toHaveLength(1);
|
|
expect(stage.participants[0]?.id).toMatch(UUID_PATTERN);
|
|
}
|
|
});
|
|
|
|
it("adds reviewers to a policy with omitted stages and retains other policy fields", () => {
|
|
const existingPolicy = {
|
|
authorizationPolicy: { trustPreset: "standard" },
|
|
maxReviewRounds: 4,
|
|
monitor: { nextCheckAt: "2099-01-01T00:00:00.000Z" },
|
|
} as IssueExecutionPolicy;
|
|
const before = structuredClone(existingPolicy);
|
|
const policy = buildExecutionPolicy({ existingPolicy, reviewerValues: [`agent:${AGENT_ID}`], approverValues: [] });
|
|
expect(issueExecutionPolicySchema.safeParse(policy).success).toBe(true);
|
|
expect(policy).toMatchObject(before);
|
|
expect(stageParticipantValues(policy, "review")).toEqual([`agent:${AGENT_ID}`]);
|
|
expect(existingPolicy).toEqual(before);
|
|
});
|
|
|
|
it("keeps stage and participant identities and governance fields when editing reviewers", () => {
|
|
const existingPolicy: IssueExecutionPolicy = {
|
|
mode: "normal", commentRequired: true, maxReviewRounds: 4,
|
|
authorizationPolicy: { trustPreset: "standard" },
|
|
stages: [{ id: STAGE_ID, type: "review", approvalsNeeded: 1, participants: [
|
|
{ id: PARTICIPANT_ID, type: "agent", agentId: AGENT_ID, userId: null },
|
|
] }],
|
|
};
|
|
const policy = buildExecutionPolicy({
|
|
existingPolicy, reviewerValues: [`agent:${AGENT_ID}`, "user:board-reviewer"], approverValues: [],
|
|
});
|
|
expect(policy).toMatchObject({
|
|
authorizationPolicy: existingPolicy.authorizationPolicy,
|
|
maxReviewRounds: 4,
|
|
stages: [{ id: STAGE_ID, participants: [
|
|
existingPolicy.stages[0].participants[0],
|
|
{ type: "user", userId: "board-reviewer" },
|
|
] }],
|
|
});
|
|
expect(issueExecutionPolicySchema.safeParse(policy).success).toBe(true);
|
|
});
|
|
|
|
it("does not remove authorization settings when the last reviewer is removed", () => {
|
|
const existingPolicy = {
|
|
mode: "normal", commentRequired: true, stages: [],
|
|
authorizationPolicy: { trustPreset: "standard" },
|
|
} as IssueExecutionPolicy;
|
|
expect(buildExecutionPolicy({ existingPolicy, reviewerValues: [], approverValues: [] }))
|
|
.toEqual(existingPolicy);
|
|
});
|
|
|
|
it.each([
|
|
{ maxReviewRounds: 4 },
|
|
{ maxReviewRounds: 4, stages: [{ id: STAGE_ID, type: "review", participants: [
|
|
{ id: PARTICIPANT_ID, type: "user", userId: "board-reviewer" },
|
|
] }] },
|
|
])("retains an independent review-round limit when no participants remain: %j", (existingPolicy) => {
|
|
expect(buildExecutionPolicy({ existingPolicy: existingPolicy as IssueExecutionPolicy, reviewerValues: [], approverValues: [] }))
|
|
.toEqual({ mode: "normal", commentRequired: true, stages: [], maxReviewRounds: 4 });
|
|
});
|
|
|
|
it("still clears a policy with no participants, monitor, or other settings", () => {
|
|
expect(buildExecutionPolicy({
|
|
existingPolicy: { mode: "normal", commentRequired: true, stages: [] },
|
|
reviewerValues: [], approverValues: [],
|
|
})).toBeNull();
|
|
});
|
|
|
|
it.each([{ stages: {} }, { stages: [null] }, { stages: [{ type: "review", participants: {} }] }])(
|
|
"refuses to manufacture a replacement for a malformed policy: %j", (input) => {
|
|
expect(() => buildExecutionPolicy({ existingPolicy: input as unknown as IssueExecutionPolicy,
|
|
reviewerValues: [], approverValues: [] })).toThrow();
|
|
},
|
|
);
|
|
});
|