From 56479bd0f8ce4dcf8fcc8af2b7526b1f9df05fda Mon Sep 17 00:00:00 2001 From: David Cramer Date: Sun, 10 May 2026 09:40:10 -0700 Subject: [PATCH] test(slack): Trim runtime unit coverage Remove Slack runtime unit tests that duplicated integration coverage or asserted observability details. Keep the unit suite focused on local runtime handoff and ordering contracts. Co-Authored-By: GPT-5 Codex --- .../tests/unit/runtime/slack-runtime.test.ts | 87 ------ .../tests/unit/slack/slack-runtime.test.ts | 254 +----------------- 2 files changed, 4 insertions(+), 337 deletions(-) delete mode 100644 packages/junior/tests/unit/runtime/slack-runtime.test.ts diff --git a/packages/junior/tests/unit/runtime/slack-runtime.test.ts b/packages/junior/tests/unit/runtime/slack-runtime.test.ts deleted file mode 100644 index 27e1eecf..00000000 --- a/packages/junior/tests/unit/runtime/slack-runtime.test.ts +++ /dev/null @@ -1,87 +0,0 @@ -import { describe, expect, it, vi } from "vitest"; -import { createSlackTurnRuntime } from "@/chat/runtime/slack-runtime"; - -describe("createSlackTurnRuntime", () => { - it("runs subscribed-thread routing and preparation inside the turn span", async () => { - let insideSpan = false; - - const withSpan = vi.fn( - async ( - _name: string, - _op: string, - _context: Record, - callback: () => Promise, - ) => { - insideSpan = true; - try { - await callback(); - } finally { - insideSpan = false; - } - }, - ); - - const prepareTurnState = vi.fn(async () => { - expect(insideSpan).toBe(true); - return { prepared: true } as const; - }); - const decideSubscribedReply = vi.fn(async () => { - expect(insideSpan).toBe(true); - return { - shouldReply: false, - reason: "side_conversation", - }; - }); - - const runtime = createSlackTurnRuntime({ - assistantUserName: "junior", - decideSubscribedReply, - getChannelId: () => "C123", - getPreparedConversationContext: () => "prior thread context", - getRunId: () => "run_123", - getThreadId: () => "thread_123", - initializeAssistantThread: vi.fn(), - logException: vi.fn(), - logWarn: vi.fn(), - modelId: "openai/gpt-4o-mini", - now: () => 1, - onSubscribedMessageSkipped: vi.fn(async () => {}), - persistPreparedState: vi.fn(async () => {}), - prepareTurnState, - recordSkippedSubscribedMessage: vi.fn(async () => {}), - refreshAssistantThreadContext: vi.fn(), - replyToThread: vi.fn(async () => {}), - stripLeadingBotMention: (text: string) => text, - withSpan, - }); - - await runtime.handleSubscribedMessage( - { - post: vi.fn(), - unsubscribe: vi.fn(), - } as any, - { - attachments: [], - author: { - userId: "U123", - userName: "alice", - }, - isMention: false, - text: "can you take a look at this?", - } as any, - ); - - expect(withSpan).toHaveBeenCalledTimes(1); - expect(withSpan.mock.calls[0]?.[2]).toEqual( - expect.objectContaining({ - conversationId: "thread_123", - slackChannelId: "C123", - slackThreadId: "thread_123", - slackUserId: "U123", - runId: "run_123", - }), - ); - expect(prepareTurnState).toHaveBeenCalledTimes(1); - expect(decideSubscribedReply).toHaveBeenCalledTimes(1); - }); -}); diff --git a/packages/junior/tests/unit/slack/slack-runtime.test.ts b/packages/junior/tests/unit/slack/slack-runtime.test.ts index 7e1efefe..89518453 100644 --- a/packages/junior/tests/unit/slack/slack-runtime.test.ts +++ b/packages/junior/tests/unit/slack/slack-runtime.test.ts @@ -1,5 +1,4 @@ import { describe, expect, it, vi } from "vitest"; -import type { Attachment } from "chat"; import { createSlackTurnRuntime, type SlackTurnRuntimeDependencies, @@ -12,7 +11,6 @@ import { interface TestState { prepared: boolean; - conversationContext?: string; } function createMockDeps( @@ -62,82 +60,10 @@ describe("createSlackTurnRuntime", () => { explicitMention: true, }); }); - - it("posts a safe error when replyToThread fails", async () => { - const replyError = new Error("reply failed"); - const deps = createMockDeps({ - replyToThread: vi.fn().mockRejectedValue(replyError), - withSpan: vi.fn(async (_n, _o, _c, cb) => cb()), - }); - const runtime = createSlackTurnRuntime(deps); - const thread = createTestThread({}); - const message = createTestMessage({}); - - await runtime.handleNewMention(thread, message); - - expect(thread.posts).toContain( - "I ran into an internal error while processing that. Reference: `event_id=evt_test`.", - ); - }); - - it("posts a safe error when subscribe fails", async () => { - const subscribeError = new Error("subscribe failed"); - const deps = createMockDeps({ - withSpan: vi.fn(async (_n, _o, _c, cb) => cb()), - }); - const runtime = createSlackTurnRuntime(deps); - const thread = createTestThread({}); - // Override subscribe to throw - thread.subscribe = async () => { - throw subscribeError; - }; - const message = createTestMessage({}); - - await runtime.handleNewMention(thread, message); - - expect(thread.posts).toContain( - "I ran into an internal error while processing that. Reference: `event_id=evt_test`.", - ); - }); - - it("includes sentry event id when available", async () => { - const replyError = new Error("reply failed"); - const deps = createMockDeps({ - replyToThread: vi.fn().mockRejectedValue(replyError), - withSpan: vi.fn(async (_n, _o, _c, cb) => cb()), - logException: vi.fn(() => "evt_123"), - }); - const runtime = createSlackTurnRuntime(deps); - const thread = createTestThread({}); - const message = createTestMessage({}); - - await runtime.handleNewMention(thread, message); - - expect(thread.posts).toContain( - "I ran into an internal error while processing that. Reference: `event_id=evt_123`.", - ); - }); - - it("fails closed when sentry capture returns no event id", async () => { - const replyError = new Error("reply failed"); - const deps = createMockDeps({ - replyToThread: vi.fn().mockRejectedValue(replyError), - withSpan: vi.fn(async (_n, _o, _c, cb) => cb()), - logException: vi.fn(() => undefined), - }); - const runtime = createSlackTurnRuntime(deps); - const thread = createTestThread({}); - const message = createTestMessage({}); - - await expect(runtime.handleNewMention(thread, message)).rejects.toThrow( - "Sentry did not return an event ID for mention_handler_failed", - ); - expect(thread.posts).toHaveLength(0); - }); }); describe("handleSubscribedMessage", () => { - it("calls prepareTurnState → persistPreparedState → shouldReply → replyToThread in order", async () => { + it("calls prepareTurnState → persistPreparedState → decideSubscribedReply → replyToThread in order", async () => { const callOrder: string[] = []; const deps = createMockDeps({ prepareTurnState: vi.fn(async () => { @@ -148,7 +74,7 @@ describe("createSlackTurnRuntime", () => { callOrder.push("persistPreparedState"); }), decideSubscribedReply: vi.fn(async () => { - callOrder.push("shouldReply"); + callOrder.push("decideSubscribedReply"); return { shouldReply: true, reason: "test" }; }), replyToThread: vi.fn(async () => { @@ -165,7 +91,7 @@ describe("createSlackTurnRuntime", () => { expect(callOrder).toEqual([ "prepareTurnState", "persistPreparedState", - "shouldReply", + "decideSubscribedReply", "replyToThread", ]); }); @@ -193,100 +119,7 @@ describe("createSlackTurnRuntime", () => { ); }); - it("when shouldReply: false, skips replyToThread", async () => { - const deps = createMockDeps({ - decideSubscribedReply: vi.fn(async () => ({ - shouldReply: false, - reason: "passive conversation", - })), - }); - const runtime = createSlackTurnRuntime(deps); - const thread = createTestThread({}); - const message = createTestMessage({}); - - await runtime.handleSubscribedMessage(thread, message); - - expect(deps.replyToThread).not.toHaveBeenCalled(); - expect(deps.recordSkippedSubscribedMessage).not.toHaveBeenCalled(); - expect(deps.onSubscribedMessageSkipped).toHaveBeenCalledWith( - expect.objectContaining({ - thread, - message, - decision: { shouldReply: false, reason: "passive conversation" }, - completedAtMs: 1700000000000, - }), - ); - }); - - it("preflight-skips messages addressed to another party before preparing turn state", async () => { - const deps = createMockDeps(); - const runtime = createSlackTurnRuntime(deps); - const thread = createTestThread({}); - const message = createTestMessage({ - text: "@Cursor can you take this one?", - isMention: false, - }); - - await runtime.handleSubscribedMessage(thread, message); - - expect(deps.prepareTurnState).not.toHaveBeenCalled(); - expect(deps.persistPreparedState).not.toHaveBeenCalled(); - expect(deps.decideSubscribedReply).not.toHaveBeenCalled(); - expect(deps.replyToThread).not.toHaveBeenCalled(); - expect(deps.onSubscribedMessageSkipped).toHaveBeenCalledWith( - expect.objectContaining({ - thread, - message, - decision: { - shouldReply: false, - reason: "directed_to_other_party:named_mention:Cursor", - }, - completedAtMs: 1700000000000, - }), - ); - expect(deps.recordSkippedSubscribedMessage).toHaveBeenCalledWith( - expect.objectContaining({ - thread, - message, - userText: "@Cursor can you take this one?", - decision: { - shouldReply: false, - reason: "directed_to_other_party:named_mention:Cursor", - }, - completedAtMs: 1700000000000, - }), - ); - }); - - it("unsubscribes when subscribed-thread routing returns thread opt-out", async () => { - const deps = createMockDeps({ - decideSubscribedReply: vi.fn(async () => ({ - shouldReply: false, - shouldUnsubscribe: true, - reason: "thread_opt_out:user asked junior to stop participating", - })), - }); - const runtime = createSlackTurnRuntime(deps); - const thread = createTestThread({}); - await thread.subscribe(); - const message = createTestMessage({ - text: "<@U123> leave this thread alone", - isMention: true, - }); - - await runtime.handleSubscribedMessage(thread, message); - - expect(thread.subscribed).toBe(false); - expect(deps.prepareTurnState).toHaveBeenCalled(); - expect(deps.persistPreparedState).toHaveBeenCalled(); - expect(deps.decideSubscribedReply).toHaveBeenCalled(); - expect(deps.replyToThread).not.toHaveBeenCalled(); - expect(thread.posts).toEqual([ - "Understood. I'll stay out of this thread unless someone @mentions me again.", - ]); - }); - - it("passes conversationContext from getPreparedConversationContext to shouldReply", async () => { + it("passes conversationContext from getPreparedConversationContext to decideSubscribedReply", async () => { const deps = createMockDeps({ getPreparedConversationContext: vi.fn(() => "some context"), withSpan: vi.fn(async (_n, _o, _c, cb) => cb()), @@ -301,85 +134,6 @@ describe("createSlackTurnRuntime", () => { expect.objectContaining({ conversationContext: "some context" }), ); }); - - it("passes explicitMention: true for classifier-approved subscribed mentions", async () => { - const deps = createMockDeps({ - decideSubscribedReply: vi.fn(async () => ({ - shouldReply: true, - reason: "llm_classifier:follow_up_question", - })), - withSpan: vi.fn(async (_n, _o, _c, cb) => cb()), - }); - const runtime = createSlackTurnRuntime(deps); - const thread = createTestThread({}); - const message = createTestMessage({ isMention: true }); - - await runtime.handleSubscribedMessage(thread, message); - - expect(deps.replyToThread).toHaveBeenCalledWith(thread, message, { - explicitMention: true, - preparedState: { prepared: true }, - }); - }); - - it("passes hasAttachments: true when message has attachments", async () => { - const deps = createMockDeps({ - decideSubscribedReply: vi.fn(async (args) => ({ - shouldReply: Boolean(args.hasAttachments), - reason: args.hasAttachments ? "attachment" : "empty message", - })), - withSpan: vi.fn(async (_n, _o, _c, cb) => cb()), - }); - const runtime = createSlackTurnRuntime(deps); - const thread = createTestThread({}); - const message = createTestMessage({ - text: "", - attachments: [ - { - type: "image", - url: "https://example.com/img.png", - } satisfies Attachment, - ], - }); - - await runtime.handleSubscribedMessage(thread, message); - - expect(deps.decideSubscribedReply).toHaveBeenCalledWith( - expect.objectContaining({ hasAttachments: true }), - ); - expect(deps.replyToThread).toHaveBeenCalled(); - }); - - it("passes hasAttachments: false when message has no attachments", async () => { - const deps = createMockDeps({ - withSpan: vi.fn(async (_n, _o, _c, cb) => cb()), - }); - const runtime = createSlackTurnRuntime(deps); - const thread = createTestThread({}); - const message = createTestMessage({ text: "hello" }); - - await runtime.handleSubscribedMessage(thread, message); - - expect(deps.decideSubscribedReply).toHaveBeenCalledWith( - expect.objectContaining({ hasAttachments: false }), - ); - }); - - it("on failure, posts a safe error message", async () => { - const err = new Error("handler boom"); - const deps = createMockDeps({ - prepareTurnState: vi.fn().mockRejectedValue(err), - }); - const runtime = createSlackTurnRuntime(deps); - const thread = createTestThread({}); - const message = createTestMessage({}); - - await runtime.handleSubscribedMessage(thread, message); - - expect(thread.posts).toContain( - "I ran into an internal error while processing that. Reference: `event_id=evt_test`.", - ); - }); }); describe("handleAssistantThreadStarted", () => {