mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-21 16:27:18 +02:00
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Tasks let agents ask a person for input. > - The reply composer needs a useful default recipient. > - A recent comment does not identify the agent that assigned the task. > - The old fallback can select the person who is writing the reply. > - This change uses the latest agent assignment and keeps manual choices. ## Linked Issues or Issue Description **What happened?** When an agent assigns a task to a person, the reply composer can select that person or a different commenter. A data refresh can also replace a manual selection. **Expected behavior** For a task assigned to the current person, select the latest agent that assigned it to them. If no such assignment exists, use the latest other commenter. If neither exists, leave the recipient empty. Keep explicit choices during refreshes. **Steps to reproduce** 1. Have agent Alpha assign a task to a person. 2. Assign it to agent Beta, then have Beta assign it to the same person. 3. Open the task as that person. 4. Check that the reply composer selects Beta. 5. Select another recipient. Refresh task data. Check that the selection stays. Related work: #15091 shares task composer controls. This change fixes recipient defaults. ## What Changed - Read real assignment changes from REST and native runner task activity, in time order. - Record human assignee fields in native create and reassign activity. - Ignore unchanged assignment fields and assignments to other people. - Use the assigning agent before comment hints for human-owned tasks. - Connect the suggestion to current and classic task composers and pipeline replies. - Keep manual selections when suggestions change. - Add regression tests and document the fallback. - Use tasks with no assignee in the attachment receipt tests. This keeps the tests on the comment route. Dismiss announcements through the real UI. - Record human and agent assignees in REST creation receipts. Include child creation in the assignment history resolver. ## Verification - Focused tests cover assignment order, creation, other recipients, unchanged fields, fallback, and manual selections. - Browser fixture uses the real composer and assignment resolver. It selects Beta and keeps a manual human selection after history refresh. - Passed: 223 focused tests across four suites. - Passed: `pnpm --filter @paperclipai/ui typecheck`. - Passed: `pnpm --filter @paperclipai/ui build`. - Passed: `pnpm check:token-gates`. - The initial CI run passed full build, typecheck, Rust, unit tests and seven E2E shards. The attachment shard exposed tests that relied on the old self-selection default. The tests now select the recipient explicitly. - Native assignment regression tests and resolver tests pass: 40 tests. - Attachment E2E suite: 10 passed; one page-load timing failure passed on an isolated rerun. All 11 cases passed across the suite and rerun. - Previous full CI passed atcc7ed05. New CI must pass on the current head. - Latest local checks: 16 resolver tests and 7 workspace policy tests pass. REST creation audit tests cover both creation routes. - The latest browser check could not start. The UI build exceeded the local memory limit (exit 137). Full typecheck, tests, build, and browser checks will run in CI. ## Risks - Old activity rows without previous assignment values cannot prove a reassignment. They use the comment fallback. - An empty fallback requires the person to choose a recipient or use the existing no-assignee flow. - No database or API schema changes. ## Model Used OpenAI Codex, GPT-6. The session does not expose an exact model build or context window. Used code editing, shell tools, tests, and browser verification. ## 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 Final validation atf83f2f6: 55 checks passed. Two optional Storybook checks were skipped. All browser shards passed. Both review threads are resolved. Classic attachment tests use the real no-assignee confirmation.
222 lines
15 KiB
TypeScript
222 lines
15 KiB
TypeScript
import { randomUUID } from "node:crypto";
|
||
import { afterAll, beforeAll, describe, expect, it, vi } from "vitest";
|
||
import { eq, and } from "drizzle-orm";
|
||
import { activityLog, agents, authUsers, companyMemberships, companies, createDb, heartbeatRuns, issues, issueComments } from "@paperclipai/db";
|
||
import { startEmbeddedPostgresTestDatabase } from "../../__tests__/helpers/embedded-postgres.js";
|
||
import { issueService } from "../issues.js";
|
||
import { PaperclipRunnerToolAuthority } from "./paperclip-runner-tool-authority.js";
|
||
|
||
describe("runner task reassignment", () => {
|
||
let temporary: Awaited<ReturnType<typeof startEmbeddedPostgresTestDatabase>>;
|
||
let db: ReturnType<typeof createDb>;
|
||
beforeAll(async () => {
|
||
temporary = await startEmbeddedPostgresTestDatabase("runner-reassign-");
|
||
db = createDb(temporary.connectionString);
|
||
});
|
||
afterAll(async () => { await temporary?.cleanup(); });
|
||
|
||
async function fixture(status = "todo") {
|
||
const companyId = randomUUID(), agentId = randomUUID(), nextId = randomUUID(), issueId = randomUUID(), targetId = randomUUID(), runId = randomUUID();
|
||
await db.insert(companies).values({ id: companyId, name: "Reassignment", issuePrefix: `R${companyId.slice(0, 6)}` });
|
||
await db.insert(agents).values([
|
||
{ id: agentId, companyId, name: "Manager", role: "ceo", status: "active", adapterType: "paperclip_runner" },
|
||
{ id: nextId, companyId, name: "Engineer", role: "engineer", status: "active", adapterType: "paperclip_runner" },
|
||
]);
|
||
await db.insert(issues).values([
|
||
{ id: issueId, companyId, title: "Coordinate", status: "in_progress", assigneeAgentId: agentId },
|
||
{ id: targetId, companyId, title: "Existing work", status, assigneeAgentId: agentId, description: "Keep the existing plan and context" },
|
||
]);
|
||
await db.insert(heartbeatRuns).values({ id: runId, companyId, agentId, status: "running", runtimeMode: "native", nativeIssueId: issueId, invocationSource: "assignment", triggerDetail: "system", contextSnapshot: { issueId } });
|
||
await db.update(issues).set({ executionRunId: runId }).where(eq(issues.id, issueId));
|
||
const enqueueWakeup = vi.fn().mockResolvedValue({ id: randomUUID() });
|
||
const stopTaskForReassignment = vi.fn().mockResolvedValue(undefined);
|
||
const binding = { companyId, agentId, issueId, runId, enqueueWakeup, stopTaskForReassignment };
|
||
const authority = new PaperclipRunnerToolAuthority(db, binding);
|
||
const call = { tool: "reassign_task", callId: "handoff", arguments: { taskId: targetId, assigneeActorId: nextId, expectedAssigneeActorId: agentId, expectedStatusVersion: 0, reason: "Engineer owns implementation; retain the existing plan.", idempotencyKey: "handoff" } };
|
||
const target = async () => (await db.select().from(issues).where(eq(issues.id, targetId)))[0]!;
|
||
return { companyId, agentId, nextId, issueId, targetId, runId, authority, binding, call, target, enqueueWakeup, stopTaskForReassignment };
|
||
}
|
||
|
||
it("moves existing work once, persists context and audit, and dispatches a guarded wake", async () => {
|
||
const f = await fixture();
|
||
const receipt = await f.authority.execute(f.call);
|
||
expect(await f.target()).toMatchObject({ assigneeAgentId: f.nextId, status: "todo", statusVersion: 1, description: "Keep the existing plan and context" });
|
||
await expect(f.authority.execute({ ...f.call, callId: "retry" })).resolves.toEqual(receipt);
|
||
expect(f.stopTaskForReassignment).toHaveBeenCalledTimes(1);
|
||
expect(f.enqueueWakeup).toHaveBeenCalledWith(f.nextId, expect.objectContaining({ issueStateGuard: { statuses: ["todo"], assigneeAgentId: f.nextId, statusVersion: 1 } }));
|
||
expect(f.enqueueWakeup.mock.calls[0]![1].idempotencyKey).toEqual(f.enqueueWakeup.mock.calls[1]![1].idempotencyKey);
|
||
expect(await db.select().from(activityLog).where(and(eq(activityLog.companyId, f.companyId), eq(activityLog.action, "issue.reassigned")))).toHaveLength(1);
|
||
expect(await db.select().from(issueComments).where(eq(issueComments.issueId, f.targetId))).toHaveLength(1);
|
||
});
|
||
|
||
it("records human assignment fields for native creation and reassignment", async () => {
|
||
const f = await fixture();
|
||
const person = randomUUID();
|
||
await db.insert(authUsers).values({ id: person, name: "Reviewer", email: `${person}@example.test`, createdAt: new Date(), updatedAt: new Date() });
|
||
await db.insert(companyMemberships).values({ companyId: f.companyId, principalType: "user", principalId: person, status: "active", membershipRole: "operator" });
|
||
await f.authority.execute({ ...f.call, arguments: { ...f.call.arguments, assigneeActorId: null, assigneeUserId: person } });
|
||
const [reassigned] = await db.select().from(activityLog).where(and(eq(activityLog.entityId, f.targetId), eq(activityLog.action, "issue.reassigned")));
|
||
expect(reassigned).toMatchObject({ actorType: "agent", actorId: f.agentId, details: {
|
||
previousAssigneeAgentId: f.agentId, previousAssigneeUserId: null,
|
||
assigneeAgentId: null, assigneeUserId: person, changed: true,
|
||
} });
|
||
const created = await f.authority.execute({ tool: "create_task", callId: "human-child", arguments: {
|
||
title: "Review result", assigneeUserId: person, idempotencyKey: "human-child",
|
||
} }) as { task: { id: string } };
|
||
const [creation] = await db.select().from(activityLog).where(and(eq(activityLog.entityId, created.task.id), eq(activityLog.action, "issue.created")));
|
||
expect(creation).toMatchObject({ actorType: "agent", actorId: f.agentId, details: { assigneeAgentId: null, assigneeUserId: person } });
|
||
});
|
||
|
||
it.each(["blocked", "backlog"])("preserves %s and does not wake work prematurely", async status => {
|
||
const f = await fixture(status);
|
||
await f.authority.execute(f.call);
|
||
expect(await f.target()).toMatchObject({ assigneeAgentId: f.nextId, status });
|
||
expect(f.enqueueWakeup).not.toHaveBeenCalled();
|
||
});
|
||
|
||
it("replays across a replacement caller run without a second mutation", async () => {
|
||
const f = await fixture();
|
||
const receipt = await f.authority.execute(f.call);
|
||
const replacement = randomUUID();
|
||
await db.insert(heartbeatRuns).values({ id: replacement, companyId: f.companyId, agentId: f.agentId, nativeIssueId: f.issueId, runtimeMode: "native", status: "running", invocationSource: "assignment", triggerDetail: "system" });
|
||
await db.update(issues).set({ executionRunId: replacement }).where(eq(issues.id, f.issueId));
|
||
await expect(new PaperclipRunnerToolAuthority(db, { ...f.binding, runId: replacement }).execute(f.call)).resolves.toEqual(receipt);
|
||
expect((await f.target()).statusVersion).toBe(1);
|
||
});
|
||
|
||
it("repairs a dispatch failure by replaying the committed receipt", async () => {
|
||
const f = await fixture();
|
||
f.enqueueWakeup.mockRejectedValueOnce(new Error("queue unavailable"));
|
||
await expect(f.authority.execute(f.call)).rejects.toThrow("queue unavailable");
|
||
expect((await f.target()).assigneeAgentId).toBe(f.nextId);
|
||
await f.authority.execute(f.call);
|
||
expect(f.stopTaskForReassignment).toHaveBeenCalledTimes(1);
|
||
expect(f.enqueueWakeup).toHaveBeenCalledTimes(2);
|
||
});
|
||
|
||
it("rejects key reuse with different input and stale ownership/version", async () => {
|
||
const f = await fixture();
|
||
await f.authority.execute(f.call);
|
||
await expect(f.authority.execute({ ...f.call, arguments: { ...f.call.arguments, reason: "Changed intent" } })).rejects.toThrow("idempotency_conflict");
|
||
await expect(f.authority.execute({ ...f.call, arguments: { ...f.call.arguments, idempotencyKey: "stale" } })).rejects.toThrow("reassignment_conflict");
|
||
});
|
||
|
||
it("stops active execution before transferring ownership", async () => {
|
||
const f = await fixture("in_progress");
|
||
const oldRun = randomUUID();
|
||
await db.insert(heartbeatRuns).values({ id: oldRun, companyId: f.companyId, agentId: f.agentId, nativeIssueId: f.targetId, runtimeMode: "native", status: "running", invocationSource: "assignment", triggerDetail: "system" });
|
||
await db.update(issues).set({ executionRunId: oldRun, checkoutRunId: oldRun }).where(eq(issues.id, f.targetId));
|
||
f.stopTaskForReassignment.mockImplementationOnce(async () => {
|
||
expect((await f.target()).assigneeAgentId).toBe(f.agentId);
|
||
await db.update(heartbeatRuns).set({ status: "cancelled" }).where(eq(heartbeatRuns.id, oldRun));
|
||
});
|
||
await f.authority.execute(f.call);
|
||
expect(await f.target()).toMatchObject({ assigneeAgentId: f.nextId, status: "todo", executionRunId: null, checkoutRunId: null });
|
||
});
|
||
|
||
it("does not transfer ownership if stop fails or a new run races the stop", async () => {
|
||
const f = await fixture();
|
||
f.stopTaskForReassignment.mockRejectedValueOnce(new Error("stop failed"));
|
||
await expect(f.authority.execute(f.call)).rejects.toThrow("stop failed");
|
||
expect((await f.target()).assigneeAgentId).toBe(f.agentId);
|
||
f.stopTaskForReassignment.mockImplementationOnce(async () => {
|
||
const runId = randomUUID();
|
||
await db.insert(heartbeatRuns).values({ id: runId, companyId: f.companyId, agentId: f.agentId, status: "running", invocationSource: "assignment", triggerDetail: "system" });
|
||
await db.update(issues).set({ executionRunId: runId }).where(eq(issues.id, f.targetId));
|
||
});
|
||
await expect(f.authority.execute(f.call)).rejects.toThrow("reassignment_conflict");
|
||
expect(f.enqueueWakeup).not.toHaveBeenCalled();
|
||
});
|
||
|
||
it.each(["version", "caller", "stop-error"])("restores the prior owner after a cancelled handoff loses %s", async conflict => {
|
||
const f = await fixture("in_progress");
|
||
const oldRun = randomUUID();
|
||
await db.insert(heartbeatRuns).values({ id: oldRun, companyId: f.companyId, agentId: f.agentId,
|
||
nativeIssueId: f.targetId, status: "running", runtimeMode: "native", invocationSource: "assignment", triggerDetail: "system" });
|
||
await db.update(issues).set({ executionRunId: oldRun }).where(eq(issues.id, f.targetId));
|
||
f.stopTaskForReassignment.mockImplementationOnce(async () => {
|
||
await db.update(heartbeatRuns).set({ status: "cancelled", errorCode: "issue_reassigned" }).where(eq(heartbeatRuns.id, oldRun));
|
||
if (conflict === "version") await db.update(issues).set({ statusVersion: 1 }).where(eq(issues.id, f.targetId));
|
||
if (conflict === "caller") await db.update(heartbeatRuns).set({ status: "succeeded" }).where(eq(heartbeatRuns.id, f.runId));
|
||
if (conflict === "stop-error") throw new Error("stop acknowledgement failed after cancellation");
|
||
});
|
||
await expect(f.authority.execute(f.call)).rejects.toThrow();
|
||
expect((await f.target()).assigneeAgentId).toBe(f.agentId);
|
||
expect(f.enqueueWakeup).toHaveBeenCalledExactlyOnceWith(f.agentId, expect.objectContaining({
|
||
payload: { issueId: f.targetId, mutation: "reassign_task_rollback", interruptedRunId: oldRun },
|
||
issueStateGuard: { statuses: ["todo", "in_progress"], assigneeAgentId: f.agentId, statusVersion: conflict === "version" ? 1 : 0 },
|
||
}));
|
||
});
|
||
|
||
it.each(["owner", "closed", "new-run"])("does not compensate over a newer %s decision", async conflict => {
|
||
const f = await fixture("in_progress");
|
||
const oldRun = randomUUID();
|
||
await db.insert(heartbeatRuns).values({ id: oldRun, companyId: f.companyId, agentId: f.agentId,
|
||
nativeIssueId: f.targetId, status: "running", runtimeMode: "native", invocationSource: "assignment", triggerDetail: "system" });
|
||
await db.update(issues).set({ executionRunId: oldRun }).where(eq(issues.id, f.targetId));
|
||
f.stopTaskForReassignment.mockImplementationOnce(async () => {
|
||
await db.update(heartbeatRuns).set({ status: "cancelled", errorCode: "issue_reassigned" }).where(eq(heartbeatRuns.id, oldRun));
|
||
const newRun = randomUUID();
|
||
if (conflict === "new-run") await db.insert(heartbeatRuns).values({ id: newRun, companyId: f.companyId,
|
||
agentId: f.agentId, status: "running", invocationSource: "assignment", triggerDetail: "system" });
|
||
await db.update(issues).set(conflict === "owner" ? { assigneeAgentId: f.nextId, statusVersion: 1 }
|
||
: conflict === "closed" ? { status: "done" } : { executionRunId: newRun }).where(eq(issues.id, f.targetId));
|
||
});
|
||
await expect(f.authority.execute(f.call)).rejects.toThrow();
|
||
expect(f.enqueueWakeup).not.toHaveBeenCalled();
|
||
});
|
||
|
||
it.each(["done", "cancelled", "in_review"])("rejects %s before interrupting work", async status => {
|
||
const f = await fixture(status);
|
||
await expect(f.authority.execute(f.call)).rejects.toThrow("state_denied");
|
||
expect(f.stopTaskForReassignment).not.toHaveBeenCalled();
|
||
});
|
||
|
||
it("rejects conversations and the caller’s own task", async () => {
|
||
const f = await fixture();
|
||
await db.update(issues).set({ conversationAgentId: f.agentId, conversationUserId: "test-user", conversationState: "active", status: "in_progress" }).where(eq(issues.id, f.targetId));
|
||
await expect(f.authority.execute(f.call)).rejects.toThrow("conversation_locked");
|
||
await expect(f.authority.execute({ ...f.call, arguments: { ...f.call.arguments, taskId: f.issueId } })).rejects.toThrow("current_task");
|
||
expect(f.stopTaskForReassignment).not.toHaveBeenCalled();
|
||
});
|
||
|
||
it.each(["planning", "ask", "skill_test"])("denies reassignment from %s mode", async workMode => {
|
||
const f = await fixture();
|
||
await db.update(issues).set({ workMode }).where(eq(issues.id, f.issueId));
|
||
await expect(f.authority.execute(f.call)).rejects.toThrow("mode_denied");
|
||
});
|
||
|
||
it("denies foreign targets and unavailable assignees", async () => {
|
||
const f = await fixture(), foreign = await fixture();
|
||
await expect(f.authority.execute({ ...f.call, arguments: { ...f.call.arguments, taskId: foreign.targetId } })).rejects.toThrow("task_not_found");
|
||
await expect(f.authority.execute({ ...f.call, arguments: { ...f.call.arguments, assigneeActorId: foreign.agentId } })).rejects.toThrow();
|
||
await db.update(agents).set({ status: "pending_approval" }).where(eq(agents.id, f.nextId));
|
||
await expect(f.authority.execute(f.call)).rejects.toThrow();
|
||
expect(f.stopTaskForReassignment).not.toHaveBeenCalled();
|
||
});
|
||
it("rejects an ownership ABA race through the existing issue service", async () => {
|
||
const f = await fixture();
|
||
await issueService(db).update(f.targetId, { assigneeAgentId: f.nextId });
|
||
await issueService(db).update(f.targetId, { assigneeAgentId: f.agentId });
|
||
expect((await f.target()).statusVersion).toBe(2);
|
||
await expect(f.authority.execute(f.call)).rejects.toThrow("reassignment_conflict");
|
||
expect(f.stopTaskForReassignment).not.toHaveBeenCalled();
|
||
});
|
||
|
||
it("respects protected assignment policies instead of inheriting board authority", async () => {
|
||
const f = await fixture();
|
||
await db.update(agents).set({ permissions: { authorizationPolicy: { protectedAgent: { blockAssignment: true } } } }).where(eq(agents.id, f.nextId));
|
||
await expect(f.authority.execute(f.call)).rejects.toThrow();
|
||
expect((await f.target()).assigneeAgentId).toBe(f.agentId);
|
||
expect(f.stopTaskForReassignment).not.toHaveBeenCalled();
|
||
});
|
||
|
||
it("rejects writes after the caller loses its run binding", async () => {
|
||
const f = await fixture();
|
||
await db.update(heartbeatRuns).set({ status: "cancelled" }).where(eq(heartbeatRuns.id, f.runId));
|
||
await expect(f.authority.execute(f.call)).rejects.toThrow("binding_not_authorized");
|
||
expect((await f.target()).assigneeAgentId).toBe(f.agentId);
|
||
});
|
||
|
||
});
|