mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-10 12:37:55 +01:00
fix(activity): use authoritative changes for timeline assignments
Exercise the real task PATCH producer before timeline reads, and bind narrative attribution directly to the reassignment action. Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
1 parent
7b2a720986
commit
1eee137d16
6 files changed
+80
-10
No files matched your search
@@ -202,7 +202,7 @@ them.
|
||||
Evidence: `source-only` — [route](../../server/src/routes/companies.ts) and
|
||||
[assignment projection](../../server/src/services/work-timeline.ts).
|
||||
Ordinary `issue.updated` assignment fields preserve recorded human or agent
|
||||
attribution and time. Status-only, null and unchanged owners remain distinct.
|
||||
attribution and time. The saved PATCH change receipt excludes repeated owner fields. Status-only, null and unchanged owners remain distinct.
|
||||
[Local route tests](../../server/src/__tests__/work-timeline-service.test.ts)
|
||||
exercise these cases separately from live agent access.
|
||||
|
||||
|
||||
@@ -86,7 +86,7 @@ the routine rather than treating that component test as a reachable journey.
|
||||
|
||||
The company timeline keeps the actor, target worker and timestamp of recorded
|
||||
assignment changes, including ordinary `issue.updated` events. A recorded change
|
||||
replaces the inferred creation-time assignment to the current worker. Status-only
|
||||
replaces the inferred creation-time assignment to the current worker. The saved PATCH change receipt excludes repeated owner fields. Status-only
|
||||
updates and unchanged assignment fields do not count as handoffs. Null assignment
|
||||
fields do not create a target worker. [Timeline route regressions](../../server/src/__tests__/work-timeline-service.test.ts)
|
||||
cover these distinctions with a disposable database; they do not prove a live
|
||||
|
||||
@@ -1,9 +1,11 @@
|
||||
import { randomUUID } from "node:crypto";
|
||||
import express from "express";
|
||||
import { eq } from "drizzle-orm";
|
||||
import request from "supertest";
|
||||
import { afterAll, afterEach, beforeAll, describe, expect, it } from "vitest";
|
||||
import {
|
||||
activityLog,
|
||||
agentWakeupRequests,
|
||||
agents,
|
||||
approvals,
|
||||
authUsers,
|
||||
@@ -19,6 +21,7 @@ import {
|
||||
startEmbeddedPostgresTestDatabase,
|
||||
} from "./helpers/embedded-postgres.js";
|
||||
import { errorHandler } from "../middleware/index.js";
|
||||
import { issueRoutes } from "../routes/issues.js";
|
||||
import { companyRoutes } from "../routes/companies.js";
|
||||
import { normalizeTimelineWindow, workTimelineService } from "../services/work-timeline.js";
|
||||
|
||||
@@ -47,6 +50,7 @@ describeEmbeddedPostgres("work timeline aggregation", () => {
|
||||
await db.delete(approvals);
|
||||
await db.delete(heartbeatRuns);
|
||||
await db.delete(issues);
|
||||
await db.delete(agentWakeupRequests);
|
||||
await db.delete(agents);
|
||||
await db.delete(authUsers);
|
||||
await db.delete(companies);
|
||||
@@ -56,13 +60,13 @@ describeEmbeddedPostgres("work timeline aggregation", () => {
|
||||
await tempDb?.cleanup();
|
||||
});
|
||||
|
||||
function createApp() {
|
||||
function createApp(actorUserId = "local-board", includeIssues = false) {
|
||||
const app = express();
|
||||
app.use(express.json());
|
||||
app.use((req, _res, next) => {
|
||||
(req as any).actor = {
|
||||
type: "board",
|
||||
userId: "local-board",
|
||||
userId: actorUserId,
|
||||
companyIds: [],
|
||||
memberships: [],
|
||||
source: "local_implicit",
|
||||
@@ -71,6 +75,7 @@ describeEmbeddedPostgres("work timeline aggregation", () => {
|
||||
next();
|
||||
});
|
||||
app.use("/api/companies", companyRoutes(db, {} as any));
|
||||
if (includeIssues) app.use("/api", issueRoutes(db, {} as any));
|
||||
app.use(errorHandler);
|
||||
return app;
|
||||
}
|
||||
@@ -571,6 +576,50 @@ describeEmbeddedPostgres("work timeline aggregation", () => {
|
||||
}
|
||||
});
|
||||
|
||||
it("uses actual PATCH receipts to separate repeated owners from reassignment and unassignment", async () => {
|
||||
const { companyId, userId, agentAId, agentBId } = await seedBase();
|
||||
await db.update(agents).set({ status: "paused" }).where(eq(agents.companyId, companyId));
|
||||
const issueId = randomUUID();
|
||||
const createdAt = new Date(Date.now() - 60_000);
|
||||
await db.insert(issues).values({ id: issueId, companyId, title: "Producer receipt launch",
|
||||
status: "todo", priority: "medium", assigneeAgentId: agentAId, createdByUserId: userId, createdAt, updatedAt: createdAt });
|
||||
const app = createApp(userId, true);
|
||||
const repeated = await request(app).patch(`/api/issues/${issueId}`).send({ status: "backlog", assigneeAgentId: agentAId });
|
||||
expect(repeated.status, JSON.stringify(repeated.body)).toBe(200);
|
||||
const repeatedEvent = (await db.select().from(activityLog)).find(row => row.action === "issue.updated" && row.entityId === issueId)!;
|
||||
expect(repeatedEvent).toMatchObject({ actorType: "user", actorId: userId });
|
||||
const details = repeatedEvent.details as Record<string, any>;
|
||||
expect(details.assigneeAgentId).toBe(agentAId);
|
||||
expect(Object.keys(details.changes)).toContain("status");
|
||||
expect(details.changes).not.toHaveProperty("assigneeAgentId");
|
||||
expect(details._previous).not.toHaveProperty("assigneeAgentId");
|
||||
const timeline = async () => {
|
||||
const response = await request(app).get(`/api/companies/${companyId}/timeline`)
|
||||
.query({ issueId, from: new Date(createdAt.getTime() - 1000).toISOString(), to: new Date().toISOString() });
|
||||
expect(response.status, JSON.stringify(response.body)).toBe(200);
|
||||
return response.body;
|
||||
};
|
||||
const unchanged = await timeline();
|
||||
expect(unchanged.events.filter((event: { kind: string }) => event.kind === "assigned")).toEqual([]);
|
||||
expect(unchanged.edges.filter((edge: { kind: string }) => edge.kind === "assignment")).toEqual([
|
||||
{ fromActorId: `user:${userId}`, toActorId: `agent:${agentAId}`, issueId, at: createdAt.toISOString(), kind: "assignment" },
|
||||
]);
|
||||
const reassigned = await request(app).patch(`/api/issues/${issueId}`).send({ assigneeAgentId: agentBId });
|
||||
expect(reassigned.status, JSON.stringify(reassigned.body)).toBe(200);
|
||||
const changed = await timeline();
|
||||
expect(changed.events.filter((event: { kind: string }) => event.kind === "assigned")).toEqual([
|
||||
expect.objectContaining({ actorId: `user:${userId}`, issueId, kind: "assigned" }),
|
||||
]);
|
||||
expect(changed.edges.filter((edge: { kind: string }) => edge.kind === "assignment")).toEqual([
|
||||
expect.objectContaining({ fromActorId: `user:${userId}`, toActorId: `agent:${agentBId}`, issueId }),
|
||||
]);
|
||||
const unassigned = await request(app).patch(`/api/issues/${issueId}`).send({ assigneeAgentId: null });
|
||||
expect(unassigned.status, JSON.stringify(unassigned.body)).toBe(200);
|
||||
const cleared = await timeline();
|
||||
expect(cleared.edges.filter((edge: { kind: string }) => edge.kind === "assignment")).toEqual(changed.edges.filter((edge: { kind: string }) => edge.kind === "assignment"));
|
||||
expect(await db.select().from(heartbeatRuns)).toEqual([]);
|
||||
});
|
||||
|
||||
it("serves GET /api/companies/:companyId/timeline", async () => {
|
||||
const { companyId, agentAId } = await seedBase();
|
||||
const issueId = randomUUID();
|
||||
|
||||
@@ -735,11 +735,26 @@ export function workTimelineService(db: Db) {
|
||||
? row.details as Record<string, unknown> : {};
|
||||
const previous = details._previous && typeof details._previous === "object" && !Array.isArray(details._previous)
|
||||
? details._previous as Record<string, unknown> : {};
|
||||
const changed = ["assigneeAgentId", "assigneeUserId"].some((key) =>
|
||||
Object.hasOwn(details, key) && (!Object.hasOwn(previous, key) || details[key] !== previous[key]));
|
||||
const changes = row.action === "issue.updated" && details.changes && typeof details.changes === "object" && !Array.isArray(details.changes)
|
||||
? details.changes as Record<string, unknown> : null;
|
||||
const changeFor = (key: string) => {
|
||||
const value = changes?.[key];
|
||||
return value && typeof value === "object" && !Array.isArray(value) ? value as Record<string, unknown> : {};
|
||||
};
|
||||
// PATCH payload fields include repeated values; its saved change receipt
|
||||
// alone identifies actual changes. Older records retain their prior shape.
|
||||
const changed = ["assigneeAgentId", "assigneeUserId"].some((key) => {
|
||||
if (changes) {
|
||||
const change = changeFor(key);
|
||||
return Object.hasOwn(change, "from") && Object.hasOwn(change, "to") && change.from !== change.to;
|
||||
}
|
||||
return Object.hasOwn(details, key) && (!Object.hasOwn(previous, key) || details[key] !== previous[key]);
|
||||
});
|
||||
if (!row.action.includes("assign") && !(row.action === "issue.updated" && changed)) return [];
|
||||
const targetAgentId = readString(details.assigneeAgentId) ?? readString(details.toAgentId);
|
||||
const targetUserId = readString(details.assigneeUserId) ?? readString(details.toUserId);
|
||||
const targetAgentId = changes ? readString(changeFor("assigneeAgentId").to)
|
||||
: readString(details.assigneeAgentId) ?? readString(details.toAgentId);
|
||||
const targetUserId = changes ? readString(changeFor("assigneeUserId").to)
|
||||
: readString(details.assigneeUserId) ?? readString(details.toUserId);
|
||||
return [{ row, toActorId: targetAgentId ? actorId("agent", targetAgentId)
|
||||
: targetUserId ? actorId("user", targetUserId) : null }];
|
||||
});
|
||||
|
||||
@@ -13,11 +13,13 @@ export function truthfulTimelineAccount(input: {
|
||||
const failures = clauses.filter(({ text }) => mentions(text, firstWorker)
|
||||
&& /\b(?:verification|verify_launch_html|check|test|command)\b/i.test(text)
|
||||
&& /\b(?:fail(?:ed|ure|ing)?|exit(?:ed)?(?:\s+with)?(?:\s+(?:code|status))?\s*[:=]?\s*1)\b/i.test(text));
|
||||
const humanActors = ["you", "human", "person", "user", "operator", "board", input.personName].filter(Boolean).map(name => escape(name!)).join("|");
|
||||
const humanActors = ["you", "human operator", "board member", "human", "person", "user", "operator", "board", input.personName].filter(Boolean).map(name => escape(name!)).join("|");
|
||||
const humanActor = `\\b(?:${humanActors})\\b`;
|
||||
const assignment = "\\b(?:reassign(?:ed|ment)?|assign(?:ed|ment)?|hand(?:ed)?(?:\\s+\\w+){0,4}\\s+(?:off|over)|handed|handoff)\\b";
|
||||
const target = `\\bto\\s+${escape(repairWorker)}\\b`;
|
||||
const handoffToRepairWorker = new RegExp(`${humanActor}[^.!?\\n]{0,100}${assignment}[^.!?\\n]{0,60}${target}|${assignment}[^.!?\\n]{0,60}${target}[^.!?\\n]{0,40}\\bby\\s+${humanActor}`, "i");
|
||||
const actorAction = `${humanActor}(?:\\s+(?:then|later|now|personally|explicitly|actually|had|has|also)){0,3}\\s+${assignment}`;
|
||||
const passiveAction = `\\b(?:was|got|had been|has been)\\s+${assignment}`;
|
||||
const handoffToRepairWorker = new RegExp(`${actorAction}[^.!?\\n]{0,60}${target}|${passiveAction}[^.!?\\n]{0,60}${target}[^.!?\\n]{0,40}\\bby\\s+(?:(?:the|a|an)\\s+)?${humanActor}`, "i");
|
||||
const differentNamedPerson = (text: string) => {
|
||||
const namedRole = /^\s*(?:Then\s+)?([A-Z][\p{L}'-]+(?:\s+[A-Z][\p{L}'-]+)*)\s*,?\s*(?:the\s+)?(?:human|person|user|operator)\b/u.exec(text);
|
||||
return !!namedRole && !["The", "A", "An", input.personName].some(name => name?.toLowerCase() === namedRole[1]!.toLowerCase())
|
||||
|
||||
@@ -77,6 +77,7 @@ it.each([
|
||||
"Today (UTC): Morgan's verification failed and work paused. Avery handed it to Riley. Riley fixed the label and the command passed. Neither native run failed.",
|
||||
"Today (UTC): Morgan's verification failed and work paused. The task was reassigned to Riley by Avery. Riley fixed the label and the command passed. Did a native run fail? Both native runs succeeded.",
|
||||
"Today (UTC): Morgan's verification failed and work paused. You reassigned the task to Riley. Riley fixed the label and the command passed. If a native run failed, it would be recorded; both native runs succeeded.",
|
||||
"Today (UTC): Morgan's verification failed and work paused. The task was reassigned to Riley by the operator. Riley fixed the label and the command passed.",
|
||||
"Today (UTC): Overview: Riley completed the repair. First Morgan's verification failed and the task was blocked. Then the board reassigned the task to Riley. Riley fixed the input and the check passed.",
|
||||
])("accepts a complete attributed account regardless of incidental first mentions: %s", reply => {
|
||||
const o = fixture(); o.reply = reply; expect(gradeWorkTimeline(o).passed).toBe(true);
|
||||
@@ -91,6 +92,9 @@ it.each([
|
||||
"Today (UTC): Morgan's verification failed and work paused. Jordan reassigned the task to Riley. Riley repaired the label and the check passed.",
|
||||
"Today (UTC): Morgan's verification failed and work paused. Jordan the human reassigned the task to Riley. Riley repaired the label and the check passed.",
|
||||
"Today (UTC): Morgan's verification failed and work paused. Another person reassigned the task to Riley. Riley repaired the label and the check passed.",
|
||||
"Today (UTC): Morgan's verification failed and work paused. A human watched while Morgan reassigned the task to Riley. Riley fixed the label and the check passed.",
|
||||
"Today (UTC): Morgan's verification failed and work paused. You asked Morgan to reassign the task to Riley. Riley fixed the label and the check passed.",
|
||||
"Today (UTC): Morgan's verification failed and work paused. Morgan reassigned the task to Riley while watched by a human. Riley fixed the label and the check passed.",
|
||||
"Today (UTC): Morgan's native run failed and work paused. The human reassigned the task to Riley. Riley repaired the label and the check passed.",
|
||||
])("rejects missing, wrong or unordered consequential actors: %s", reply => {
|
||||
const o = fixture(); o.reply = reply; expect(gradeWorkTimeline(o).passed).toBe(false);
|
||||
|
||||
Reference in new issue
Block a user