feat(server): log an activity row for each queued-comment queue mutation (#13159)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The server records actions that change issues and their queued comments > - The queued-comment edit, reorder, and discard routes changed queue state without activity rows > - Operators could not inspect these queue mutations in the activity feed > - This pull request adds one identifier-only activity row for each successful queue mutation > - The benefit is a durable audit trail with no comment text in the activity log ## Linked Issues or Issue Description **What existing behavior does this improve?** The queued-comment edit, reorder, and discard routes now record their successful mutations in the activity feed. **Subsystem affected** Cross-cutting (server and ui). **Current behavior** The three queue mutation routes change queued comments but do not write an activity row. The activity feed has no label for these actions. **Proposed behavior** Each successful route writes one activity row with the actor fields, entity fields, queue identifiers, and queue revision. The discard row also includes the cancelled run identifier. The activity feed shows a label for each action. **Reason and benefit** Operators need a durable record of queue changes. Identifier-only details support audit and troubleshooting without storing comment text. **Breaking changes** None. The routes keep their existing response and authorization behavior. **Additional context** Each mutation writes its activity row on the same locked transaction that applies the mutation, so the two commit or roll back together. The route publishes the live activity event only after that transaction commits. The separate comment-cancel route opts out of this write and keeps its existing single activity row. ## What Changed - Add activity rows for queued-comment edit, reorder, and discard mutations. - Include queue identifiers, revisions, ordered comment identifiers, and cancelled run identifiers as applicable. - Add activity-feed labels for the three new actions. - Add route and activity-format tests for the new behavior. - Write each activity row on the same transaction as the mutation it records, through a new port method that the adapter implements. - Keep the comment-delete route opted out of that write, so a cancellation does not log two rows. ## Verification - [x] `npx vitest run server/src/__tests__/issue-queued-comments-routes.test.ts` passes. - [x] `npx vitest run ui/src/lib/activity-format.test.ts` passes. - [x] `pnpm --filter @paperclipai/server typecheck` exits 0. - [x] `pnpm --filter @paperclipai/ui typecheck` exits 0. - [x] `node scripts/check-module-boundaries.mjs` passes. - [x] The full CI suite is green. ## Risks Low risk. The change adds activity rows after successful mutations and does not change route responses, authorization, or stored comment text. ## Model Used OpenAI Codex, GPT-5 Codex. The model used repository inspection, Git operations, and command execution. The context window and reasoning mode are not exposed by this runtime. ## 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>
This commit is contained in:
Nicky Leach
GitHub
parent
0d8bbf7cf4
commit
86c2e0ac4a
|
|
@ -251,6 +251,68 @@ describeEmbeddedPostgres("issue queued-comment routes", () => {
|
|||
expect(stale.body.details?.code).toBe("queued_comment_revision_conflict");
|
||||
});
|
||||
|
||||
it("writes one activity log row for each successful queue mutation, and none for a rejected one", async () => {
|
||||
const seeded = await seedQueue();
|
||||
const initial = await request(app(seeded.companyId))
|
||||
.get(`/api/issues/${seeded.issueId}/queued-comments`);
|
||||
|
||||
const edited = await request(app(seeded.companyId))
|
||||
.patch(`/api/issues/${seeded.issueId}/queued-comments/${seeded.commentIds[0]}`)
|
||||
.send({ queueId: seeded.wakeId, revision: initial.body.revision, body: "edited body" });
|
||||
expect(edited.status, JSON.stringify(edited.body)).toBe(200);
|
||||
|
||||
const staleEdit = await request(app(seeded.companyId))
|
||||
.patch(`/api/issues/${seeded.issueId}/queued-comments/${seeded.commentIds[0]}`)
|
||||
.send({ queueId: seeded.wakeId, revision: initial.body.revision, body: "stale" });
|
||||
expect(staleEdit.status).toBe(409);
|
||||
|
||||
const editRows = await db
|
||||
.select({ details: activityLog.details })
|
||||
.from(activityLog)
|
||||
.where(eq(activityLog.action, "issue.queued_comment_edited"));
|
||||
expect(editRows).toHaveLength(1);
|
||||
expect(editRows[0]?.details).toMatchObject({
|
||||
commentId: seeded.commentIds[0],
|
||||
queueId: seeded.wakeId,
|
||||
revision: edited.body.revision,
|
||||
});
|
||||
|
||||
const reordered = await request(app(seeded.companyId))
|
||||
.put(`/api/issues/${seeded.issueId}/queued-comments/order`)
|
||||
.send({
|
||||
queueId: seeded.wakeId,
|
||||
revision: edited.body.revision,
|
||||
orderedCommentIds: [...seeded.commentIds].reverse(),
|
||||
});
|
||||
expect(reordered.status, JSON.stringify(reordered.body)).toBe(200);
|
||||
const reorderRow = await db
|
||||
.select({ details: activityLog.details })
|
||||
.from(activityLog)
|
||||
.where(eq(activityLog.action, "issue.queued_comments_reordered"))
|
||||
.then((rows) => rows[0]);
|
||||
expect(reorderRow?.details).toMatchObject({
|
||||
queueId: seeded.wakeId,
|
||||
revision: reordered.body.revision,
|
||||
orderedCommentIds: [...seeded.commentIds].reverse(),
|
||||
});
|
||||
|
||||
const discarded = await request(app(seeded.companyId))
|
||||
.delete(`/api/issues/${seeded.issueId}/queued-comments/${seeded.commentIds[1]}`)
|
||||
.send({ queueId: seeded.wakeId, revision: reordered.body.revision });
|
||||
expect(discarded.status, JSON.stringify(discarded.body)).toBe(200);
|
||||
const discardRow = await db
|
||||
.select({ details: activityLog.details })
|
||||
.from(activityLog)
|
||||
.where(eq(activityLog.action, "issue.queued_comment_discarded"))
|
||||
.then((rows) => rows[0]);
|
||||
expect(discardRow?.details).toMatchObject({
|
||||
commentId: seeded.commentIds[1],
|
||||
queueId: seeded.wakeId,
|
||||
revision: discarded.body.revision,
|
||||
cancelledRunId: null,
|
||||
});
|
||||
});
|
||||
|
||||
it("preserves reordered messages across promotion and cancels the queued run after final trash", async () => {
|
||||
const seeded = await seedQueue();
|
||||
const initial = await request(app(seeded.companyId))
|
||||
|
|
@ -311,6 +373,21 @@ describeEmbeddedPostgres("issue queued-comment routes", () => {
|
|||
]);
|
||||
expect(queueRun?.status).toBe("cancelled");
|
||||
expect(storedIssue?.executionRunId).toBeNull();
|
||||
|
||||
// Two discards happen in this scenario (the first trash, then the final
|
||||
// one that empties the queue), so match the row by its own commentId
|
||||
// instead of assuming insertion order.
|
||||
const discardRows = await db
|
||||
.select({ details: activityLog.details })
|
||||
.from(activityLog)
|
||||
.where(eq(activityLog.action, "issue.queued_comment_discarded"));
|
||||
const finalDiscardRow = discardRows.find(
|
||||
(row) => (row.details as { commentId?: string } | null)?.commentId === seeded.commentIds[0],
|
||||
);
|
||||
expect(finalDiscardRow?.details).toMatchObject({
|
||||
commentId: seeded.commentIds[0],
|
||||
cancelledRunId: queueRunId,
|
||||
});
|
||||
});
|
||||
|
||||
it("keeps a mutation response's steering disposition in step with a fresh GET after promotion", async () => {
|
||||
|
|
|
|||
|
|
@ -2,7 +2,7 @@ import { randomUUID } from "node:crypto";
|
|||
import { eq } from "drizzle-orm";
|
||||
import { afterAll, afterEach, beforeAll, describe, expect, it } from "vitest";
|
||||
import type { Db } from "@paperclipai/db";
|
||||
import { agentWakeupRequests, agents, companies, createDb, heartbeatRuns, issueComments, issues } from "@paperclipai/db";
|
||||
import { activityLog, agentWakeupRequests, agents, companies, createDb, heartbeatRuns, issueComments, issues } from "@paperclipai/db";
|
||||
import {
|
||||
getEmbeddedPostgresTestSupport,
|
||||
startEmbeddedPostgresTestDatabase,
|
||||
|
|
@ -44,6 +44,9 @@ describeEmbeddedPostgres("queued-comment postgres adapter", () => {
|
|||
}, 20_000);
|
||||
|
||||
afterEach(async () => {
|
||||
// Deleted first: activity_log rows reference companies, agents, and
|
||||
// heartbeat_runs, and none of those foreign keys cascade.
|
||||
await db.delete(activityLog);
|
||||
await db.delete(issueComments);
|
||||
await db.delete(heartbeatRuns);
|
||||
await db.delete(agentWakeupRequests);
|
||||
|
|
@ -148,7 +151,7 @@ describeEmbeddedPostgres("queued-comment postgres adapter", () => {
|
|||
// context; that single value binds every read and write for the
|
||||
// whole transaction, so it alone must decide what is visible.
|
||||
issue: { id: issueId, companyId: otherCompanyId, assigneeAgentId: agentId, executionRunId: null },
|
||||
actor: { actorType: "user", actorId: "user-1", agentId: null },
|
||||
actor: { actorType: "user", actorId: "user-1", agentId: null, runId: null, agentApiKeyId: null },
|
||||
queueId: wakeId,
|
||||
},
|
||||
async () => {
|
||||
|
|
@ -176,7 +179,7 @@ describeEmbeddedPostgres("queued-comment postgres adapter", () => {
|
|||
issueLock.withLockedQueue(
|
||||
{
|
||||
issue: { id: issueId, companyId, assigneeAgentId: agentId, executionRunId: null },
|
||||
actor: { actorType: "user", actorId: "user-1", agentId: null },
|
||||
actor: { actorType: "user", actorId: "user-1", agentId: null, runId: null, agentApiKeyId: null },
|
||||
queueId: wakeId,
|
||||
},
|
||||
async (_locked, transaction) => {
|
||||
|
|
@ -216,7 +219,7 @@ describeEmbeddedPostgres("queued-comment postgres adapter", () => {
|
|||
const queue = await issueLock.withLockedQueue(
|
||||
{
|
||||
issue: { id: issueId, companyId, assigneeAgentId: agentId, executionRunId: null },
|
||||
actor: { actorType: "user", actorId: "user-1", agentId: null },
|
||||
actor: { actorType: "user", actorId: "user-1", agentId: null, runId: null, agentApiKeyId: null },
|
||||
queueId: wakeId,
|
||||
},
|
||||
async (locked, transaction) => {
|
||||
|
|
@ -231,7 +234,7 @@ describeEmbeddedPostgres("queued-comment postgres adapter", () => {
|
|||
await transaction.syncCommentReferences(commentId);
|
||||
return transaction.buildQueueSnapshot({
|
||||
issue: { id: issueId, companyId, assigneeAgentId: agentId, executionRunId: null },
|
||||
actor: { actorType: "user", actorId: "user-1", agentId: null },
|
||||
actor: { actorType: "user", actorId: "user-1", agentId: null, runId: null, agentApiKeyId: null },
|
||||
wake: locked.wake,
|
||||
state: locked.state,
|
||||
queueRun: locked.queueRun,
|
||||
|
|
@ -247,6 +250,54 @@ describeEmbeddedPostgres("queued-comment postgres adapter", () => {
|
|||
expect(commentRow?.body).toBe("edited body");
|
||||
});
|
||||
|
||||
it("rolls back an already-applied comment edit when its own activity insert fails", async () => {
|
||||
const companyId = await seedCompany();
|
||||
const agentId = await seedAgent({ companyId });
|
||||
const issueId = await seedIssue({ companyId, assigneeAgentId: agentId });
|
||||
const commentId = await seedComment({ companyId, issueId, authorUserId: "user-1" });
|
||||
const wakeId = await seedDeferredWake({ companyId, agentId, issueId, commentIds: [commentId] });
|
||||
|
||||
const issueLock = createQueuedCommentIssueLockWriter(db, noopDeps);
|
||||
const missingAgentId = randomUUID();
|
||||
|
||||
await expect(
|
||||
issueLock.withLockedQueue(
|
||||
{
|
||||
issue: { id: issueId, companyId, assigneeAgentId: agentId, executionRunId: null },
|
||||
actor: { actorType: "user", actorId: "user-1", agentId: null, runId: null, agentApiKeyId: null },
|
||||
queueId: wakeId,
|
||||
},
|
||||
async (_locked, transaction) => {
|
||||
await transaction.updateCommentBody({
|
||||
issueId,
|
||||
commentId,
|
||||
body: "edited body",
|
||||
updatedAt: new Date(),
|
||||
});
|
||||
// `agentId` carries a foreign key to `agents.id`; naming an agent
|
||||
// that was never seeded forces the activity insert to fail, which
|
||||
// must roll back the comment edit issued moments earlier on the
|
||||
// same transaction.
|
||||
await transaction.logActivity({
|
||||
actorType: "agent",
|
||||
actorId: missingAgentId,
|
||||
agentId: missingAgentId,
|
||||
runId: null,
|
||||
agentApiKeyId: null,
|
||||
action: "issue.queued_comment_edited",
|
||||
entityId: issueId,
|
||||
details: {},
|
||||
});
|
||||
},
|
||||
),
|
||||
).rejects.toMatchObject({ cause: { code: "23503" } });
|
||||
|
||||
const commentRow = (await db.select().from(issueComments).where(eq(issueComments.id, commentId)))[0];
|
||||
expect(commentRow?.body).toBe("queued message");
|
||||
const activityRows = await db.select().from(activityLog).where(eq(activityLog.entityId, issueId));
|
||||
expect(activityRows).toHaveLength(0);
|
||||
});
|
||||
|
||||
// Pins a fact the database itself cannot persist today: `runtime_mode` is
|
||||
// a NOT NULL column, so a real active run's own field is never null. The
|
||||
// port type allows it (`runtimeMode: string | null`), so this test builds
|
||||
|
|
@ -263,14 +314,14 @@ describeEmbeddedPostgres("queued-comment postgres adapter", () => {
|
|||
const queue = await issueLock.withLockedQueue(
|
||||
{
|
||||
issue: { id: issueId, companyId, assigneeAgentId: agentId, executionRunId: null },
|
||||
actor: { actorType: "user", actorId: "user-1", agentId: null },
|
||||
actor: { actorType: "user", actorId: "user-1", agentId: null, runId: null, agentApiKeyId: null },
|
||||
queueId: wakeId,
|
||||
},
|
||||
async (locked, transaction) => {
|
||||
expect(locked.state).toBe("deferred");
|
||||
return transaction.buildQueueSnapshot({
|
||||
issue: { id: issueId, companyId, assigneeAgentId: agentId, executionRunId: null },
|
||||
actor: { actorType: "user", actorId: "user-1", agentId: null },
|
||||
actor: { actorType: "user", actorId: "user-1", agentId: null, runId: null, agentApiKeyId: null },
|
||||
wake: locked.wake,
|
||||
state: "deferred",
|
||||
queueRun: null,
|
||||
|
|
|
|||
|
|
@ -9,11 +9,14 @@ import {
|
|||
withQueuedCommentIdsInRunContext,
|
||||
withQueuedCommentIdsInWakePayload,
|
||||
} from "../../../services/issue-queued-comment-queue.js";
|
||||
import { logActivity as persistActivityLogRow, type ActivityPublication } from "../../../services/activity-log.js";
|
||||
import { decideQueuedCommentWakeLookup } from "../domain/policy.js";
|
||||
import { parseObject, readNonEmptyString } from "../domain/values.js";
|
||||
import { QueuedCommentMutationError } from "../application/queued-comment-use-cases.js";
|
||||
import type {
|
||||
LockedQueuedCommentState,
|
||||
QueuedCommentActivityLogInput,
|
||||
QueuedCommentActivityPublication,
|
||||
QueuedCommentIssueLockWriter,
|
||||
QueuedCommentQueueTransaction,
|
||||
QueuedCommentRunRow,
|
||||
|
|
@ -175,6 +178,27 @@ function buildTransaction(tx: Db, companyId: string, deps: QueuedCommentQueuePos
|
|||
async syncCommentExternalObjectsSafely(commentId) {
|
||||
await deps.syncCommentExternalObjectsSafely(commentId, tx);
|
||||
},
|
||||
|
||||
async logActivity(input: QueuedCommentActivityLogInput): Promise<QueuedCommentActivityPublication> {
|
||||
const publications: ActivityPublication[] = [];
|
||||
await persistActivityLogRow(
|
||||
tx,
|
||||
{
|
||||
companyId,
|
||||
actorType: input.actorType,
|
||||
actorId: input.actorId,
|
||||
agentId: input.agentId,
|
||||
runId: input.runId,
|
||||
agentApiKeyId: input.agentApiKeyId,
|
||||
action: input.action,
|
||||
entityType: "issue",
|
||||
entityId: input.entityId,
|
||||
details: input.details,
|
||||
},
|
||||
publications,
|
||||
);
|
||||
return publications[0];
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -13,6 +13,33 @@ export type QueuedCommentActor = {
|
|||
actorId: string;
|
||||
/** Null for a user actor. */
|
||||
agentId: string | null;
|
||||
runId: string | null;
|
||||
agentApiKeyId: string | null;
|
||||
};
|
||||
|
||||
/** The fields a mutation needs to log its own activity row; entity type is always "issue". */
|
||||
export type QueuedCommentActivityLogInput = {
|
||||
actorType: "agent" | "user";
|
||||
actorId: string;
|
||||
agentId: string | null;
|
||||
runId: string | null;
|
||||
agentApiKeyId: string | null;
|
||||
action: string;
|
||||
entityId: string;
|
||||
details: Record<string, unknown>;
|
||||
};
|
||||
|
||||
/**
|
||||
* Structurally mirrors the server's `ActivityPublication` (from
|
||||
* `services/activity-log.ts`), which this layer cannot import by name --
|
||||
* the module boundary check forbids the application layer from importing
|
||||
* server services. The route casts this back to `ActivityPublication`
|
||||
* before calling `publishActivity`.
|
||||
*/
|
||||
export type QueuedCommentActivityPublication = {
|
||||
companyId: string;
|
||||
payload: Record<string, unknown>;
|
||||
pluginEvent: unknown;
|
||||
};
|
||||
|
||||
export type QueuedCommentIssueContext = {
|
||||
|
|
@ -121,6 +148,13 @@ export interface QueuedCommentQueueTransaction {
|
|||
syncCommentReferences(commentId: string): Promise<void>;
|
||||
deleteCommentReferenceSource(commentId: string): Promise<void>;
|
||||
syncCommentExternalObjectsSafely(commentId: string): Promise<void>;
|
||||
/**
|
||||
* Persists the activity row on this same transaction, so a mutation and
|
||||
* its audit record commit or roll back together. Returns the publication
|
||||
* for the caller to publish once the transaction has committed; this
|
||||
* write never publishes the live event itself.
|
||||
*/
|
||||
logActivity(input: QueuedCommentActivityLogInput): Promise<QueuedCommentActivityPublication>;
|
||||
}
|
||||
|
||||
export interface QueuedCommentIssueLockWriter {
|
||||
|
|
|
|||
|
|
@ -9,6 +9,7 @@ import {
|
|||
} from "./queued-comment-use-cases.js";
|
||||
import type {
|
||||
LockedQueuedCommentState,
|
||||
QueuedCommentActivityPublication,
|
||||
QueuedCommentActor,
|
||||
QueuedCommentEntrySnapshot,
|
||||
QueuedCommentIssueContext,
|
||||
|
|
@ -26,8 +27,24 @@ const ISSUE: QueuedCommentIssueContext = {
|
|||
executionRunId: null,
|
||||
};
|
||||
|
||||
const USER_ACTOR: QueuedCommentActor = { actorType: "user", actorId: "user-1", agentId: null };
|
||||
const AGENT_ACTOR: QueuedCommentActor = { actorType: "agent", actorId: "agent-1", agentId: "agent-1" };
|
||||
const USER_ACTOR: QueuedCommentActor = {
|
||||
actorType: "user",
|
||||
actorId: "user-1",
|
||||
agentId: null,
|
||||
runId: null,
|
||||
agentApiKeyId: null,
|
||||
};
|
||||
const AGENT_ACTOR: QueuedCommentActor = {
|
||||
actorType: "agent",
|
||||
actorId: "agent-1",
|
||||
agentId: "agent-1",
|
||||
runId: "run-1",
|
||||
agentApiKeyId: "api-key-1",
|
||||
};
|
||||
|
||||
function activityPublicationFixture(overrides: Partial<QueuedCommentActivityPublication> = {}): QueuedCommentActivityPublication {
|
||||
return { companyId: ISSUE.companyId, payload: {}, pluginEvent: null, ...overrides };
|
||||
}
|
||||
|
||||
function wakeRow(overrides: Partial<QueuedCommentWakeRow> = {}): QueuedCommentWakeRow {
|
||||
return { id: "wake-1", agentId: "agent-1", status: "deferred_issue_execution", runId: null, payload: {}, ...overrides };
|
||||
|
|
@ -103,6 +120,7 @@ function createFakeTransaction(overrides: Partial<QueuedCommentQueueTransaction>
|
|||
syncCommentReferences: vi.fn(async () => {}),
|
||||
deleteCommentReferenceSource: vi.fn(async () => {}),
|
||||
syncCommentExternalObjectsSafely: vi.fn(async () => {}),
|
||||
logActivity: vi.fn(async () => activityPublicationFixture()),
|
||||
...overrides,
|
||||
};
|
||||
}
|
||||
|
|
@ -135,7 +153,20 @@ describe("editQueuedComment", () => {
|
|||
);
|
||||
expect(transaction.syncCommentReferences).toHaveBeenCalledWith("comment-1");
|
||||
expect(transaction.syncCommentExternalObjectsSafely).toHaveBeenCalledWith("comment-1");
|
||||
expect(result).toEqual(queueSnapshot());
|
||||
expect(result.queue).toEqual(queueSnapshot());
|
||||
// Proves the activity write runs on the same locked transaction as the
|
||||
// mutation, not as a separate statement after it commits.
|
||||
expect(transaction.logActivity).toHaveBeenCalledWith({
|
||||
actorType: "user",
|
||||
actorId: "user-1",
|
||||
agentId: null,
|
||||
runId: null,
|
||||
agentApiKeyId: null,
|
||||
action: "issue.queued_comment_edited",
|
||||
entityId: "issue-1",
|
||||
details: { commentId: "comment-1", queueId: "wake-1", revision: "rev-1" },
|
||||
});
|
||||
expect(result.activityPublication).toEqual(activityPublicationFixture());
|
||||
});
|
||||
|
||||
it("rejects a stale queue id with queued_comment_stale_queue", async () => {
|
||||
|
|
@ -219,7 +250,7 @@ describe("reorderQueuedComments", () => {
|
|||
const transaction = createFakeTransaction();
|
||||
const reorderQueuedComments = createReorderQueuedComments({ issueLock: createFakeIssueLock(locked, transaction) });
|
||||
|
||||
await reorderQueuedComments({
|
||||
const result = await reorderQueuedComments({
|
||||
issue: ISSUE,
|
||||
actor: USER_ACTOR,
|
||||
queueId: "wake-1",
|
||||
|
|
@ -231,6 +262,19 @@ describe("reorderQueuedComments", () => {
|
|||
expect(transaction.updateWakeQueuedCommentIds).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ wakeId: "wake-1", ids: ["b", "a"] }),
|
||||
);
|
||||
// Proves the activity write runs on the same locked transaction as the
|
||||
// mutation, not as a separate statement after it commits.
|
||||
expect(transaction.logActivity).toHaveBeenCalledWith({
|
||||
actorType: "user",
|
||||
actorId: "user-1",
|
||||
agentId: null,
|
||||
runId: null,
|
||||
agentApiKeyId: null,
|
||||
action: "issue.queued_comments_reordered",
|
||||
entityId: "issue-1",
|
||||
details: { queueId: "wake-1", revision: "rev-1", orderedCommentIds: ["b", "a"] },
|
||||
});
|
||||
expect(result.activityPublication).toEqual(activityPublicationFixture());
|
||||
});
|
||||
|
||||
it("rejects an order that is not a permutation of the current queue", async () => {
|
||||
|
|
@ -266,6 +310,7 @@ describe("discardQueuedComment", () => {
|
|||
queueId: "wake-1",
|
||||
revision: "rev-1",
|
||||
now: new Date(),
|
||||
logActivity: true,
|
||||
});
|
||||
|
||||
expect(transaction.cancelWake).toHaveBeenCalledWith(expect.objectContaining({ wakeId: "wake-1" }));
|
||||
|
|
@ -274,6 +319,37 @@ describe("discardQueuedComment", () => {
|
|||
expect.objectContaining({ executionRunId: "run-1" }),
|
||||
);
|
||||
expect(result.cancelledRun).toEqual({ id: "run-1" });
|
||||
// Proves the activity write runs on the same locked transaction as the
|
||||
// mutation, and that the cancelled run's own id lands in its details.
|
||||
expect(transaction.logActivity).toHaveBeenCalledWith({
|
||||
actorType: "user",
|
||||
actorId: "user-1",
|
||||
agentId: null,
|
||||
runId: null,
|
||||
agentApiKeyId: null,
|
||||
action: "issue.queued_comment_discarded",
|
||||
entityId: "issue-1",
|
||||
details: { commentId: "comment-1", queueId: "wake-1", revision: "rev-1", cancelledRunId: "run-1" },
|
||||
});
|
||||
expect(result.activityPublication).toEqual(activityPublicationFixture());
|
||||
});
|
||||
|
||||
it("logs no activity row when the caller does not request one, matching the comment-delete route's cancellation call site", async () => {
|
||||
const locked = lockedState({ state: "queued", queueRun: runRow({ id: "run-1" }) });
|
||||
const transaction = createFakeTransaction();
|
||||
const discardQueuedComment = createDiscardQueuedComment({ issueLock: createFakeIssueLock(locked, transaction) });
|
||||
|
||||
const result = await discardQueuedComment({
|
||||
issue: ISSUE,
|
||||
actor: USER_ACTOR,
|
||||
commentId: "comment-1",
|
||||
queueId: "wake-1",
|
||||
revision: "rev-1",
|
||||
now: new Date(),
|
||||
});
|
||||
|
||||
expect(transaction.logActivity).not.toHaveBeenCalled();
|
||||
expect(result.activityPublication).toBeNull();
|
||||
});
|
||||
|
||||
it("rewrites the remaining ids when other queued comments are left", async () => {
|
||||
|
|
|
|||
|
|
@ -4,6 +4,7 @@ import {
|
|||
decideQueuedCommentReorder,
|
||||
} from "../domain/policy.js";
|
||||
import type {
|
||||
QueuedCommentActivityPublication,
|
||||
QueuedCommentActor,
|
||||
QueuedCommentIssueContext,
|
||||
QueuedCommentIssueLockWriter,
|
||||
|
|
@ -74,8 +75,13 @@ export type EditQueuedCommentInput = {
|
|||
now: Date;
|
||||
};
|
||||
|
||||
export type EditQueuedCommentResult = {
|
||||
queue: QueuedCommentQueueSnapshot;
|
||||
activityPublication: QueuedCommentActivityPublication;
|
||||
};
|
||||
|
||||
export function createEditQueuedComment(deps: { issueLock: QueuedCommentIssueLockWriter }) {
|
||||
return async function editQueuedComment(input: EditQueuedCommentInput): Promise<QueuedCommentQueueSnapshot> {
|
||||
return async function editQueuedComment(input: EditQueuedCommentInput): Promise<EditQueuedCommentResult> {
|
||||
return deps.issueLock.withLockedQueue(
|
||||
{ issue: input.issue, actor: input.actor, queueId: input.queueId },
|
||||
async (locked, tx) => {
|
||||
|
|
@ -108,7 +114,7 @@ export function createEditQueuedComment(deps: { issueLock: QueuedCommentIssueLoc
|
|||
updatedAt: input.now,
|
||||
});
|
||||
|
||||
return tx.buildQueueSnapshot({
|
||||
const queue = await tx.buildQueueSnapshot({
|
||||
issue: input.issue,
|
||||
actor: input.actor,
|
||||
wake: locked.wake,
|
||||
|
|
@ -116,6 +122,23 @@ export function createEditQueuedComment(deps: { issueLock: QueuedCommentIssueLoc
|
|||
queueRun: updatedQueueRun ?? locked.queueRun,
|
||||
activeRun: locked.activeRun,
|
||||
});
|
||||
|
||||
const activityPublication = await tx.logActivity({
|
||||
actorType: input.actor.actorType,
|
||||
actorId: input.actor.actorId,
|
||||
agentId: input.actor.agentId,
|
||||
runId: input.actor.runId,
|
||||
agentApiKeyId: input.actor.agentApiKeyId,
|
||||
action: "issue.queued_comment_edited",
|
||||
entityId: input.issue.id,
|
||||
details: {
|
||||
commentId: input.commentId,
|
||||
queueId: input.queueId,
|
||||
revision: queue.revision,
|
||||
},
|
||||
});
|
||||
|
||||
return { queue, activityPublication };
|
||||
},
|
||||
);
|
||||
};
|
||||
|
|
@ -130,8 +153,13 @@ export type ReorderQueuedCommentsInput = {
|
|||
now: Date;
|
||||
};
|
||||
|
||||
export type ReorderQueuedCommentsResult = {
|
||||
queue: QueuedCommentQueueSnapshot;
|
||||
activityPublication: QueuedCommentActivityPublication;
|
||||
};
|
||||
|
||||
export function createReorderQueuedComments(deps: { issueLock: QueuedCommentIssueLockWriter }) {
|
||||
return async function reorderQueuedComments(input: ReorderQueuedCommentsInput): Promise<QueuedCommentQueueSnapshot> {
|
||||
return async function reorderQueuedComments(input: ReorderQueuedCommentsInput): Promise<ReorderQueuedCommentsResult> {
|
||||
return deps.issueLock.withLockedQueue(
|
||||
{ issue: input.issue, actor: input.actor, queueId: input.queueId },
|
||||
async (locked, tx) => {
|
||||
|
|
@ -158,7 +186,7 @@ export function createReorderQueuedComments(deps: { issueLock: QueuedCommentIssu
|
|||
updatedAt: input.now,
|
||||
});
|
||||
|
||||
return tx.buildQueueSnapshot({
|
||||
const queue = await tx.buildQueueSnapshot({
|
||||
issue: input.issue,
|
||||
actor: input.actor,
|
||||
wake: updatedWake,
|
||||
|
|
@ -166,6 +194,23 @@ export function createReorderQueuedComments(deps: { issueLock: QueuedCommentIssu
|
|||
queueRun: updatedQueueRun ?? locked.queueRun,
|
||||
activeRun: locked.activeRun,
|
||||
});
|
||||
|
||||
const activityPublication = await tx.logActivity({
|
||||
actorType: input.actor.actorType,
|
||||
actorId: input.actor.actorId,
|
||||
agentId: input.actor.agentId,
|
||||
runId: input.actor.runId,
|
||||
agentApiKeyId: input.actor.agentApiKeyId,
|
||||
action: "issue.queued_comments_reordered",
|
||||
entityId: input.issue.id,
|
||||
details: {
|
||||
queueId: input.queueId,
|
||||
revision: queue.revision,
|
||||
orderedCommentIds: input.orderedCommentIds,
|
||||
},
|
||||
});
|
||||
|
||||
return { queue, activityPublication };
|
||||
},
|
||||
);
|
||||
};
|
||||
|
|
@ -179,6 +224,13 @@ export type DiscardQueuedCommentInput = {
|
|||
/** Skipped entirely when omitted, matching the comment-delete route's cancellation call site, which does not carry a revision. */
|
||||
revision?: string;
|
||||
now: Date;
|
||||
/**
|
||||
* Set only by the queue-discard route. The comment-delete route's
|
||||
* cancellation call site omits this: it already logs its own
|
||||
* `issue.comment_cancelled` row outside this use case, and this flag
|
||||
* would otherwise double-log that same discard.
|
||||
*/
|
||||
logActivity?: boolean;
|
||||
};
|
||||
|
||||
export type DiscardQueuedCommentResult = {
|
||||
|
|
@ -187,6 +239,8 @@ export type DiscardQueuedCommentResult = {
|
|||
queue: QueuedCommentQueueSnapshot;
|
||||
/** Set only when the discard emptied the queue and cancelled a queued run; the caller emits telemetry for it after the transaction commits. */
|
||||
cancelledRun: { id: string } | null;
|
||||
/** Set only when `input.logActivity` was true; the caller publishes it once the transaction commits. */
|
||||
activityPublication: QueuedCommentActivityPublication | null;
|
||||
};
|
||||
|
||||
export function createDiscardQueuedComment(deps: { issueLock: QueuedCommentIssueLockWriter }) {
|
||||
|
|
@ -278,7 +332,25 @@ export function createDiscardQueuedComment(deps: { issueLock: QueuedCommentIssue
|
|||
activeRun: locked.activeRun,
|
||||
});
|
||||
|
||||
return { deleted, queue, cancelledRun };
|
||||
const activityPublication = input.logActivity
|
||||
? await tx.logActivity({
|
||||
actorType: input.actor.actorType,
|
||||
actorId: input.actor.actorId,
|
||||
agentId: input.actor.agentId,
|
||||
runId: input.actor.runId,
|
||||
agentApiKeyId: input.actor.agentApiKeyId,
|
||||
action: "issue.queued_comment_discarded",
|
||||
entityId: input.issue.id,
|
||||
details: {
|
||||
commentId: input.commentId,
|
||||
queueId: input.queueId,
|
||||
revision: queue.revision,
|
||||
cancelledRunId: cancelledRun?.id ?? null,
|
||||
},
|
||||
})
|
||||
: null;
|
||||
|
||||
return { deleted, queue, cancelledRun, activityPublication };
|
||||
},
|
||||
);
|
||||
};
|
||||
|
|
|
|||
|
|
@ -44,10 +44,13 @@ export type {
|
|||
DiscardQueuedCommentInput,
|
||||
DiscardQueuedCommentResult,
|
||||
EditQueuedCommentInput,
|
||||
EditQueuedCommentResult,
|
||||
QueuedCommentMutationErrorCode,
|
||||
ReorderQueuedCommentsInput,
|
||||
ReorderQueuedCommentsResult,
|
||||
} from "./application/queued-comment-use-cases.js";
|
||||
export type {
|
||||
QueuedCommentActivityPublication,
|
||||
QueuedCommentActor,
|
||||
QueuedCommentIssueContext,
|
||||
QueuedCommentQueueSnapshot,
|
||||
|
|
|
|||
|
|
@ -15141,7 +15141,7 @@ export function issueRoutes(
|
|||
);
|
||||
if (!issue) return;
|
||||
const actor = getActorInfo(req);
|
||||
const queue = await runQueuedCommentMutation(() =>
|
||||
const { queue, activityPublication } = await runQueuedCommentMutation(() =>
|
||||
queuedCommentQueue.editQueuedComment({
|
||||
issue: buildQueuedCommentIssueContext(issue),
|
||||
actor,
|
||||
|
|
@ -15152,6 +15152,7 @@ export function issueRoutes(
|
|||
now: new Date(),
|
||||
}),
|
||||
);
|
||||
publishActivity(activityPublication as ActivityPublication);
|
||||
res.json(await runRedactions.redactForIssue(issue.companyId, issue.id, queue));
|
||||
},
|
||||
);
|
||||
|
|
@ -15171,7 +15172,7 @@ export function issueRoutes(
|
|||
);
|
||||
if (!issue) return;
|
||||
const actor = getActorInfo(req);
|
||||
const queue = await runQueuedCommentMutation(() =>
|
||||
const { queue, activityPublication } = await runQueuedCommentMutation(() =>
|
||||
queuedCommentQueue.reorderQueuedComments({
|
||||
issue: buildQueuedCommentIssueContext(issue),
|
||||
actor,
|
||||
|
|
@ -15181,6 +15182,7 @@ export function issueRoutes(
|
|||
now: new Date(),
|
||||
}),
|
||||
);
|
||||
publishActivity(activityPublication as ActivityPublication);
|
||||
res.json(await runRedactions.redactForIssue(issue.companyId, issue.id, queue));
|
||||
},
|
||||
);
|
||||
|
|
@ -15467,8 +15469,10 @@ export function issueRoutes(
|
|||
queueId: req.body.queueId,
|
||||
revision: req.body.revision,
|
||||
now: new Date(),
|
||||
logActivity: true,
|
||||
}),
|
||||
);
|
||||
publishActivity(result.activityPublication as ActivityPublication);
|
||||
// Telemetry is best-effort background work; it must not delay the
|
||||
// response with a slow lookup, so fire it and do not await it.
|
||||
if (result.cancelledRun) {
|
||||
|
|
|
|||
|
|
@ -66,6 +66,15 @@ describe("activity formatting", () => {
|
|||
expect(formatIssueActivityAction("issue.monitor_recovery_issue_created")).toBe("created a monitor recovery issue");
|
||||
});
|
||||
|
||||
it("labels each queued-comment queue mutation", () => {
|
||||
expect(formatActivityVerb("issue.queued_comment_edited")).toBe("edited a queued comment on");
|
||||
expect(formatActivityVerb("issue.queued_comments_reordered")).toBe("reordered queued comments on");
|
||||
expect(formatActivityVerb("issue.queued_comment_discarded")).toBe("discarded a queued comment on");
|
||||
expect(formatIssueActivityAction("issue.queued_comment_edited")).toBe("edited a queued comment");
|
||||
expect(formatIssueActivityAction("issue.queued_comments_reordered")).toBe("reordered queued comments");
|
||||
expect(formatIssueActivityAction("issue.queued_comment_discarded")).toBe("discarded a queued comment");
|
||||
});
|
||||
|
||||
// PAP-16506 P4: agents can now resolve an interaction, including a review of
|
||||
// their own work, so an outcome has to read as an outcome in the timeline
|
||||
// instead of leaking the raw action id.
|
||||
|
|
|
|||
|
|
@ -29,6 +29,9 @@ const ACTIVITY_ROW_VERBS: Record<string, string> = {
|
|||
"issue.released": "released",
|
||||
"issue.comment_added": "commented on",
|
||||
"issue.comment_cancelled": "cancelled a queued comment on",
|
||||
"issue.queued_comment_edited": "edited a queued comment on",
|
||||
"issue.queued_comments_reordered": "reordered queued comments on",
|
||||
"issue.queued_comment_discarded": "discarded a queued comment on",
|
||||
"issue.comment_deleted": "deleted a comment on",
|
||||
"issue.attachment_added": "attached file to",
|
||||
"issue.attachment_removed": "removed attachment from",
|
||||
|
|
@ -124,6 +127,9 @@ const ISSUE_ACTIVITY_LABELS: Record<string, string> = {
|
|||
"issue.released": "released the issue",
|
||||
"issue.comment_added": "added a comment",
|
||||
"issue.comment_cancelled": "cancelled a queued comment",
|
||||
"issue.queued_comment_edited": "edited a queued comment",
|
||||
"issue.queued_comments_reordered": "reordered queued comments",
|
||||
"issue.queued_comment_discarded": "discarded a queued comment",
|
||||
"issue.comment_deleted": "deleted a comment",
|
||||
"issue.feedback_vote_saved": "saved feedback on an AI output",
|
||||
"issue.attachment_added": "added an attachment",
|
||||
|
|
|
|||
Loading…
Reference in New Issue