From 595d4d988b3d606e34709cf37ffda7460d4cfe48 Mon Sep 17 00:00:00 2001 From: Dotta Date: Tue, 19 May 2026 15:52:31 -0500 Subject: [PATCH] Address Greptile runtime review concerns Co-Authored-By: Paperclip --- .../issue-thread-interactions-service.test.ts | 18 +++++++++++++++--- .../__tests__/plugin-lifecycle-restart.test.ts | 14 ++++++++++++++ .../src/services/issue-thread-interactions.ts | 10 +++++----- server/src/services/plugin-lifecycle.ts | 5 ++--- 4 files changed, 36 insertions(+), 11 deletions(-) diff --git a/server/src/__tests__/issue-thread-interactions-service.test.ts b/server/src/__tests__/issue-thread-interactions-service.test.ts index 5251f1fdb38..78ae5e0a2fa 100644 --- a/server/src/__tests__/issue-thread-interactions-service.test.ts +++ b/server/src/__tests__/issue-thread-interactions-service.test.ts @@ -816,7 +816,7 @@ describeEmbeddedPostgres("issueThreadInteractionService", () => { }); }); - it("expires request confirmations by default when a user comments after creation", async () => { + it("expires request confirmations opted into user-comment supersede after creation", async () => { const { companyId, issueId } = await seedConfirmationIssue(); const commentId = randomUUID(); @@ -828,6 +828,7 @@ describeEmbeddedPostgres("issueThreadInteractionService", () => { payload: { version: 1, prompt: "Proceed with the current draft?", + supersedeOnUserComment: true, }, }, { userId: "local-board", @@ -857,7 +858,7 @@ describeEmbeddedPostgres("issueThreadInteractionService", () => { }); }); - it("keeps request confirmations pending when user-comment supersede is explicitly disabled", async () => { + it("keeps request confirmations pending unless user-comment supersede is explicitly enabled", async () => { const { companyId, issueId } = await seedConfirmationIssue("Comment supersede opt-out"); await interactionsSvc.create({ @@ -868,7 +869,6 @@ describeEmbeddedPostgres("issueThreadInteractionService", () => { payload: { version: 1, prompt: "Proceed with the current draft?", - supersedeOnUserComment: false, }, }, { userId: "local-board", @@ -902,6 +902,7 @@ describeEmbeddedPostgres("issueThreadInteractionService", () => { payload: { version: 1, prompt: "Proceed with the current draft?", + supersedeOnUserComment: true, }, }, { userId: "local-board", @@ -957,6 +958,7 @@ describeEmbeddedPostgres("issueThreadInteractionService", () => { payload: { version: 1, prompt: "Proceed with the current draft?", + supersedeOnUserComment: true, }, }, { userId: "local-board", @@ -966,6 +968,16 @@ describeEmbeddedPostgres("issueThreadInteractionService", () => { .set({ createdAt, updatedAt: createdAt }) .where(eq(issueThreadInteractions.id, created.id)); + await db.insert(issueComments).values({ + id: randomUUID(), + companyId, + issueId, + authorAgentId: randomUUID(), + authorType: "agent", + body: "Agent-side progress note.", + createdAt: new Date("2026-05-18T12:00:30.000Z"), + updatedAt: new Date("2026-05-18T12:00:30.000Z"), + }); await db.insert(issueComments).values({ id: commentId, companyId, diff --git a/server/src/__tests__/plugin-lifecycle-restart.test.ts b/server/src/__tests__/plugin-lifecycle-restart.test.ts index d7731fde1fa..04026e6e59b 100644 --- a/server/src/__tests__/plugin-lifecycle-restart.test.ts +++ b/server/src/__tests__/plugin-lifecycle-restart.test.ts @@ -79,6 +79,10 @@ describe("pluginLifecycleManager.restartWorker", () => { {} as never, { loader: loader as PluginLoader, workerManager }, ); + const stopped = vi.fn(); + const started = vi.fn(); + lifecycle.on("plugin.worker_stopped", stopped); + lifecycle.on("plugin.worker_started", started); await lifecycle.restartWorker("plugin-1"); @@ -87,6 +91,8 @@ describe("pluginLifecycleManager.restartWorker", () => { // The bare worker handle should NOT be bounced — the loader handles // worker (re)start as part of activate. expect(handle.restart).not.toHaveBeenCalled(); + expect(stopped).not.toHaveBeenCalled(); + expect(started).not.toHaveBeenCalled(); }); it("falls back to bouncing the worker handle when the loader has no runtime services", async () => { @@ -105,11 +111,19 @@ describe("pluginLifecycleManager.restartWorker", () => { {} as never, { loader: loader as PluginLoader, workerManager }, ); + const stopped = vi.fn(); + const started = vi.fn(); + lifecycle.on("plugin.worker_stopped", stopped); + lifecycle.on("plugin.worker_started", started); await lifecycle.restartWorker("plugin-1"); expect(loader.unloadSingle).not.toHaveBeenCalled(); expect(loader.loadSingle).not.toHaveBeenCalled(); expect(handle.restart).toHaveBeenCalledTimes(1); + expect(stopped).toHaveBeenCalledTimes(1); + expect(stopped).toHaveBeenCalledWith({ pluginId: "plugin-1", pluginKey: "example.plugin" }); + expect(started).toHaveBeenCalledTimes(1); + expect(started).toHaveBeenCalledWith({ pluginId: "plugin-1", pluginKey: "example.plugin" }); }); }); diff --git a/server/src/services/issue-thread-interactions.ts b/server/src/services/issue-thread-interactions.ts index 98d9f153241..38e121dc95b 100644 --- a/server/src/services/issue-thread-interactions.ts +++ b/server/src/services/issue-thread-interactions.ts @@ -1,5 +1,5 @@ import { isDeepStrictEqual } from "node:util"; -import { and, asc, eq, inArray } from "drizzle-orm"; +import { and, asc, eq, inArray, isNotNull } from "drizzle-orm"; import type { Db } from "@paperclipai/db"; import { documents, @@ -159,7 +159,7 @@ function shouldReturnAcceptedConfirmationToCreatorAgent(args: { } function shouldSupersedeRequestConfirmationOnUserComment(interaction: RequestConfirmationInteraction) { - return interaction.payload.supersedeOnUserComment !== false; + return interaction.payload.supersedeOnUserComment === true; } function isCommentAtOrAfterInteraction(args: { @@ -1059,12 +1059,12 @@ export function issueThreadInteractionService(db: Db) { .where(and( eq(issueComments.companyId, issue.companyId), eq(issueComments.issueId, issue.id), + isNotNull(issueComments.authorUserId), )) .orderBy(asc(issueComments.createdAt)), ]); - const userComments = comments.filter((comment) => comment.authorUserId); - if (rows.length === 0 || userComments.length === 0) return []; + if (rows.length === 0 || comments.length === 0) return []; const now = new Date(); const expired: IssueThreadInteraction[] = []; @@ -1072,7 +1072,7 @@ export function issueThreadInteractionService(db: Db) { const interaction = hydrateInteraction(row) as RequestConfirmationInteraction; if (!shouldSupersedeRequestConfirmationOnUserComment(interaction)) continue; - const supersedingComment = userComments.find((comment) => isCommentAtOrAfterInteraction({ + const supersedingComment = comments.find((comment) => isCommentAtOrAfterInteraction({ commentCreatedAt: comment.createdAt, interactionCreatedAt: row.createdAt, })); diff --git a/server/src/services/plugin-lifecycle.ts b/server/src/services/plugin-lifecycle.ts index daee94173c2..24fa13e772a 100644 --- a/server/src/services/plugin-lifecycle.ts +++ b/server/src/services/plugin-lifecycle.ts @@ -810,11 +810,10 @@ export function pluginLifecycleManager( "plugin lifecycle: restarting worker (runtime services unavailable; skipping migration re-apply)", ); await handle.restart(); + emitDomain("plugin.worker_stopped", { pluginId, pluginKey: plugin.pluginKey }); + emitDomain("plugin.worker_started", { pluginId, pluginKey: plugin.pluginKey }); } - emitDomain("plugin.worker_stopped", { pluginId, pluginKey: plugin.pluginKey }); - emitDomain("plugin.worker_started", { pluginId, pluginKey: plugin.pluginKey }); - log.info( { pluginId, pluginKey: plugin.pluginKey }, "plugin lifecycle: plugin reloaded",