From 1eee137d16eeb13b59773adf691a2d0a3de755b2 Mon Sep 17 00:00:00 2001 From: Dotta Date: Tue, 3 Nov 2026 07:51:06 -0600 Subject: [PATCH] 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 --- maps/agent-access-map/activity.md | 2 +- maps/feature-map/activity.md | 2 +- .../__tests__/work-timeline-service.test.ts | 53 ++++++++++++++++++- server/src/services/work-timeline.ts | 23 ++++++-- .../internal-timeline-narrative.ts | 6 ++- ...internal-timeline-native-milestone.test.ts | 4 ++ 6 files changed, 80 insertions(+), 10 deletions(-) diff --git a/maps/agent-access-map/activity.md b/maps/agent-access-map/activity.md index 5348a3feadf..12e6401b7dc 100644 --- a/maps/agent-access-map/activity.md +++ b/maps/agent-access-map/activity.md @@ -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. diff --git a/maps/feature-map/activity.md b/maps/feature-map/activity.md index 7f99db28099..e74689d2082 100644 --- a/maps/feature-map/activity.md +++ b/maps/feature-map/activity.md @@ -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 diff --git a/server/src/__tests__/work-timeline-service.test.ts b/server/src/__tests__/work-timeline-service.test.ts index 97927abff12..01bfee2df5e 100644 --- a/server/src/__tests__/work-timeline-service.test.ts +++ b/server/src/__tests__/work-timeline-service.test.ts @@ -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; + 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(); diff --git a/server/src/services/work-timeline.ts b/server/src/services/work-timeline.ts index 17adfed4788..816933a9050 100644 --- a/server/src/services/work-timeline.ts +++ b/server/src/services/work-timeline.ts @@ -735,11 +735,26 @@ export function workTimelineService(db: Db) { ? row.details as Record : {}; const previous = details._previous && typeof details._previous === "object" && !Array.isArray(details._previous) ? details._previous as Record : {}; - 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 : null; + const changeFor = (key: string) => { + const value = changes?.[key]; + return value && typeof value === "object" && !Array.isArray(value) ? value as Record : {}; + }; + // 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 }]; }); diff --git a/tests/runner-e2e/chat-catalog-domains/internal-timeline-narrative.ts b/tests/runner-e2e/chat-catalog-domains/internal-timeline-narrative.ts index 54fd34d9e03..5ef9d3963e2 100644 --- a/tests/runner-e2e/chat-catalog-domains/internal-timeline-narrative.ts +++ b/tests/runner-e2e/chat-catalog-domains/internal-timeline-narrative.ts @@ -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()) diff --git a/tests/runner-e2e/chat-catalog-domains/internal-timeline-native-milestone.test.ts b/tests/runner-e2e/chat-catalog-domains/internal-timeline-native-milestone.test.ts index 1f7c45d1374..84b0cdd90b0 100644 --- a/tests/runner-e2e/chat-catalog-domains/internal-timeline-native-milestone.test.ts +++ b/tests/runner-e2e/chat-catalog-domains/internal-timeline-native-milestone.test.ts @@ -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);