Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 0 additions & 7 deletions packages/junior/src/chat/app/factory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,6 @@ import {
getMessageActorIdentity,
} from "@/chat/services/message-actor-identity";
import { lookupSlackUser } from "@/chat/slack/user";
import { ConversationTurnLifecycleService } from "@/chat/conversations/turn-lifecycle";
import { getConversationEventStore } from "@/chat/db";
import type { ScheduleSessionCompletedPluginTasksOptions } from "@/chat/plugins/task-runner";
import {
createPausedTurns,
Expand Down Expand Up @@ -90,9 +88,6 @@ function upsertSkippedConversationMessage(

export function createSlackRuntime(options: CreateSlackRuntimeOptions) {
const services = createJuniorRuntimeServices(options.services);
const turnLifecycle = new ConversationTurnLifecycleService(
getConversationEventStore(),
);
const prepareTurnState = createPrepareTurnState({
compactConversationIfNeeded:
services.conversationMemory.compactConversationIfNeeded,
Expand Down Expand Up @@ -131,7 +126,6 @@ export function createSlackRuntime(options: CreateSlackRuntimeOptions) {
prepareTurnState,
resolveUserAttachments: services.visionContext.resolveUserAttachments,
sendPluginTask: options.sendPluginTask,
turnLifecycle,
});

const runtime = createSlackTurnRuntime<
Expand All @@ -143,7 +137,6 @@ export function createSlackRuntime(options: CreateSlackRuntimeOptions) {
getBotUserId: () => options.getSlackAdapter().botUserId,
modelId: defaultModelId(botConfig),
now: options.now ?? (() => Date.now()),
failConversationTurn: (input) => turnLifecycle.fail(input),
prepareTurnState,
persistPreparedState: async ({ thread, preparedState }) => {
await persistThreadState(thread, {
Expand Down
17 changes: 5 additions & 12 deletions packages/junior/src/chat/providers/slack/resume.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,7 @@ import {
finalizeFailedTurnReplyWithEvent,
requireTurnFailureEventId,
} from "@/chat/services/turn-failure-response";
import {
getTurnLifecycle,
type ConversationTurnLifecycle,
} from "@/chat/conversations/turn-lifecycle";
import { getTurnLifecycle } from "@/chat/conversations/turn-lifecycle";
import type { ConversationTurnFailureCode } from "@/chat/conversations/history";
import {
recordTurnSummary,
Expand Down Expand Up @@ -262,10 +259,10 @@ async function postResumeFailureReply(args: {
async function handleResumeFailure(args: {
error: unknown;
eventName: string;
turnLifecycle: ConversationTurnLifecycle;
failureCode: ConversationTurnFailureCode;
resume: ResumeSlackTurnArgs;
}): Promise<void> {
const turnLifecycle = getTurnLifecycle();
const capturedEventId = logException(args.error, args.eventName);
const eventId = requireTurnFailureEventId(capturedEventId, args.eventName);
let failureStatePersistError: unknown;
Expand All @@ -278,7 +275,7 @@ async function handleResumeFailure(args: {
{ "app.error.original_event_id": eventId },
);
try {
await args.turnLifecycle.fail({
await turnLifecycle.fail({
conversationId: args.resume.conversationId,
turnId: args.resume.turnId,
createdAtMs: Date.now(),
Expand Down Expand Up @@ -308,7 +305,7 @@ async function handleResumeFailure(args: {
{ "app.error.original_event_id": eventId },
);
try {
await args.turnLifecycle.fail({
await turnLifecycle.fail({
conversationId: args.resume.conversationId,
turnId: args.resume.turnId,
createdAtMs: Date.now(),
Expand All @@ -327,7 +324,7 @@ async function handleResumeFailure(args: {
if (failureStatePersistError) {
throw failureStatePersistError;
}
await args.turnLifecycle.fail({
await turnLifecycle.fail({
conversationId: args.resume.conversationId,
turnId: args.resume.turnId,
createdAtMs: Date.now(),
Expand Down Expand Up @@ -790,7 +787,6 @@ async function resumeSlackTurnInContext(
error: new Error("Resumed Run ended suspended without onSuspend"),
eventName: "slack.resume.turn.failed",
failureCode: "agent_run_failed",
turnLifecycle,
resume: runArgs,
});
};
Expand All @@ -803,7 +799,6 @@ async function resumeSlackTurnInContext(
),
eventName: "slack.resume.turn.failed",
failureCode: "agent_run_failed",
turnLifecycle,
resume: runArgs,
});
};
Expand Down Expand Up @@ -857,7 +852,6 @@ async function resumeSlackTurnInContext(
error: runError,
eventName: "slack.resume.turn.failed",
failureCode,
turnLifecycle,
resume: runArgs,
});
};
Expand Down Expand Up @@ -910,7 +904,6 @@ async function resumeSlackTurnInContext(
error: pauseError,
eventName: "slack.resume.pause_handler.failed",
failureCode: "persistence_failed",
turnLifecycle,
resume: runArgs,
});
return true;
Expand Down
9 changes: 6 additions & 3 deletions packages/junior/src/chat/providers/slack/runtime.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,10 @@ import {
} from "@/chat/runtime/turn-input";
import { getMessageActorIdentity } from "@/chat/services/message-actor-identity";
import { isResourceEventSlackMessage } from "@/chat/resource-events/actor";
import type { FailConversationTurnInput } from "@/chat/conversations/turn-lifecycle";
import {
getTurnLifecycle,
type FailConversationTurnInput,
} from "@/chat/conversations/turn-lifecycle";

export interface AssistantLifecycleEvent {
channelId: string;
Expand Down Expand Up @@ -140,7 +143,6 @@ export interface SlackTurnRuntimeDependencies<TPreparedState> {
threadId: string;
threadTs: string;
}) => Promise<void>;
failConversationTurn: (input: FailConversationTurnInput) => Promise<void>;
refreshAssistantThreadContext: (event: {
channelId: string;
sourceChannelId?: string;
Expand Down Expand Up @@ -363,6 +365,7 @@ export function createSlackTurnRuntime<
>(
deps: SlackTurnRuntimeDependencies<TPreparedState>,
): SlackTurnRuntime<TPreparedState, TAssistantEvent> {
const turnLifecycle = getTurnLifecycle();
const logContext = (args: {
channelId?: string;
actorId?: string;
Expand Down Expand Up @@ -400,7 +403,7 @@ export function createSlackTurnRuntime<
if (!conversationId) {
return;
}
await deps.failConversationTurn({
await turnLifecycle.fail({
conversationId,
createdAtMs: deps.now(),
eventId: args.eventId,
Expand Down
9 changes: 5 additions & 4 deletions packages/junior/src/chat/providers/slack/turn.ts
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ import {
} from "@/chat/pi/transcript";
import { requireSlackDestination } from "@/chat/destination";
import { persistConversationMessages } from "@/chat/conversations/messages";
import type { ConversationTurnLifecycle } from "@/chat/conversations/turn-lifecycle";
import { getTurnLifecycle } from "@/chat/conversations/turn-lifecycle";
import {
scheduleSessionCompletedPluginTasks,
type ScheduleSessionCompletedPluginTasksOptions,
Expand Down Expand Up @@ -249,7 +249,6 @@ interface SlackTurnDeps {
>;
prepareTurnState: (args: PrepareTurnStateInput) => Promise<PreparedTurnState>;
sendPluginTask?: ScheduleSessionCompletedPluginTasksOptions["send"];
turnLifecycle: Pick<ConversationTurnLifecycle, "fail" | "start">;
}

/** Return whether the Slack caller should publish destination output. */
Expand All @@ -259,6 +258,8 @@ function shouldPublishExternally(publishExternally?: boolean): boolean {

/** Build the Slack caller that prepares input and delivers output for a Turn. */
export function createSlackTurn(deps: SlackTurnDeps) {
const turnLifecycle = getTurnLifecycle();

return async function executeSlackTurn(
thread: Thread,
message: Message,
Expand Down Expand Up @@ -674,7 +675,7 @@ export function createSlackTurn(deps: SlackTurnDeps) {
nextTurnId: turnId,
});
if (conversationId && preparedState.userMessageId) {
await deps.turnLifecycle.start({
await turnLifecycle.start({
conversationId,
createdAtMs: Date.now(),
inputMessageIds: [
Expand Down Expand Up @@ -1318,7 +1319,7 @@ export function createSlackTurn(deps: SlackTurnDeps) {
conversation: preparedState.conversation,
});
if (conversationId) {
await deps.turnLifecycle.fail({
await turnLifecycle.fail({
conversationId,
createdAtMs: Date.now(),
...(authFailureEventId
Expand Down
5 changes: 5 additions & 0 deletions packages/junior/tests/integration/slack/bot-handlers.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -460,6 +460,11 @@ describe("bot handlers (integration)", () => {
failureCode: "model_execution_failed",
}),
]);
const failure = lifecycle[1]?.data;
if (failure?.type !== "turn_failed" || !failure.eventId) {
throw new Error("Expected a Turn failure event with an event ID");
}
expect(postIncludes(thread, `event_id=${failure.eventId}`)).toBe(true);
});

it("does not persist an assistant message when final Slack delivery fails", async () => {
Expand Down
45 changes: 23 additions & 22 deletions packages/junior/tests/unit/slack/slack-runtime.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { describe, expect, it, vi } from "vitest";
import { beforeEach, describe, expect, it, vi } from "vitest";
import {
createSlackTurnRuntime,
type SlackTurnRuntimeDependencies,
Expand All @@ -10,6 +10,12 @@ import {
createTestDestination,
} from "../../fixtures/slack-harness";

const failTurn = vi.hoisted(() => vi.fn(async () => undefined));

vi.mock("@/chat/conversations/turn-lifecycle", () => ({
getTurnLifecycle: () => ({ fail: failTurn }),
}));

interface TestState {
prepared: boolean;
}
Expand All @@ -25,7 +31,6 @@ function createMockDeps(
now: () => 1700000000000,
initializeAssistantThread: vi.fn().mockResolvedValue(undefined),
refreshAssistantThreadContext: vi.fn().mockResolvedValue(undefined),
failConversationTurn: vi.fn().mockResolvedValue(undefined),
onSubscribedMessageSkipped: vi.fn().mockResolvedValue(undefined),
recordSkippedSteeringMessage: vi.fn().mockResolvedValue(undefined),
recordSkippedSubscribedTurn: vi.fn().mockResolvedValue(undefined),
Expand All @@ -44,6 +49,10 @@ function createMockDeps(
}

describe("createSlackTurnRuntime", () => {
beforeEach(() => {
failTurn.mockClear();
});

describe("handleNewMention", () => {
it("subscribes thread and calls executeSlackTurn with explicitMention: true", async () => {
const deps = createMockDeps();
Expand Down Expand Up @@ -145,26 +154,18 @@ describe("createSlackTurnRuntime", () => {
isFinalAttempt,
});

const failureCalls = vi.mocked(deps.failConversationTurn).mock.calls;
const eventId = failureCalls[0]?.[0].eventId;
const expectedFailure = {
conversationId: message.threadId,
createdAtMs: 1700000000000,
eventId: expect.any(String),
failureCode: "agent_run_failed",
turnId: "turn_m-failed-turn",
};
expect(failureCalls).toEqual(
shouldPostFallback ? [[expectedFailure]] : [],
);
expect(thread.posts).toEqual(
shouldPostFallback
? [
"I ran into an internal error while processing that. " +
`Reference: \`event_id=${eventId}\`.`,
]
: [],
);
expect(failTurn).toHaveBeenCalledTimes(shouldPostFallback ? 1 : 0);
expect(thread.posts).toHaveLength(shouldPostFallback ? 1 : 0);
expect(
thread.posts.some((post) =>
typeof post === "string"
? post.includes(
"I ran into an internal error while processing that. " +
"Reference: `event_id=",
)
: false,
),
).toBe(shouldPostFallback);
},
);
});
Expand Down
Loading