Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 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
8 changes: 1 addition & 7 deletions packages/junior/src/chat/app/factory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,7 @@ import {
import { createJuniorRuntimeServices } from "@/chat/app/services";
import type { JuniorRuntimeServiceOverrides } from "@/chat/app/services";
import { coerceThreadConversationState } from "@/chat/state/conversation";
import {
logException,
logWarn,
resolveErrorReference,
withSpan,
} from "@/chat/logging";
import { logException, logWarn, withSpan } from "@/chat/logging";
import { createReplyToThread } from "@/chat/runtime/reply-executor";
import {
initializeAssistantThread as initializeAssistantThreadImpl,
Expand Down Expand Up @@ -66,7 +61,6 @@ export function createSlackRuntime(
assistantUserName: botConfig.userName,
modelId: botConfig.modelId,
now: options.now ?? (() => Date.now()),
getErrorReference: resolveErrorReference,
getThreadId,
getChannelId,
getRunId,
Expand Down
31 changes: 17 additions & 14 deletions packages/junior/src/chat/logging.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1414,8 +1414,8 @@ export function setSentryScopeContext(
// ---------------------------------------------------------------------------

export interface ErrorReference {
traceId: string;
eventId?: string;
eventId: string;
traceId?: string;
}

type SpanAttributePrimitive = string | number | boolean;
Expand Down Expand Up @@ -1657,23 +1657,26 @@ export function getActiveTraceId(): string | undefined {
}
}

/** Build a trace + event ID reference for error correlation. */
export function resolveErrorReference(eventId?: string): ErrorReference | null {
/** Resolve a Sentry error reference for user-facing error messages. */
export function resolveErrorReference(eventId: string): ErrorReference {
const traceId = getActiveTraceId();
if (!eventId && !traceId) {
return null;
}

if (!traceId) {
Comment thread
cursor[bot] marked this conversation as resolved.
return null;
}

return {
traceId,
...(eventId ? { eventId } : {}),
eventId,
...(traceId ? { traceId } : {}),
};
}

const TURN_FAILURE_RESPONSE_TEMPLATE =
"I ran into an internal error while processing that. Reference: `event_id={eventId}`.";

/** Build the static user-facing response for a failed turn. */
export function buildTurnFailureResponse(eventId: string | undefined): string {
if (!eventId) {
return TURN_FAILURE_RESPONSE_TEMPLATE.replace("{eventId}", "unknown");
}
return TURN_FAILURE_RESPONSE_TEMPLATE.replace("{eventId}", eventId);
}

// ---------------------------------------------------------------------------
// Gen-AI attribute serialization
// ---------------------------------------------------------------------------
Expand Down
9 changes: 0 additions & 9 deletions packages/junior/src/chat/respond-helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -227,15 +227,6 @@ export function encodeNonImageAttachmentForPrompt(attachment: {
].join("\n");
}

/** Build a user-facing message for execution failures. */
export function buildExecutionFailureMessage(toolErrorCount: number): string {
if (toolErrorCount > 0) {
return "I couldn't complete this because one or more required tools failed in this turn. I've logged the failure details.";
}

return "I couldn't complete this request in this turn due to an execution failure. I've logged the details for debugging.";
}

/** Type guard for Pi SDK tool result messages. */
export function isToolResultMessage(
value: unknown,
Expand Down
15 changes: 12 additions & 3 deletions packages/junior/src/chat/runtime/reply-executor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import type { SlackAdapter } from "@chat-adapter/slack";
import { botConfig } from "@/chat/config";
import { getSlackMessageTs } from "@/chat/slack/message";
import {
buildTurnFailureResponse,
logException,
logInfo,
logWarn,
Expand Down Expand Up @@ -302,7 +303,7 @@ export function createReplyToThread(deps: ReplyExecutorDeps) {
try {
const toolChannelId =
preparedState.artifacts.assistantContextChannelId ?? channelId;
const reply = await deps.services.generateAssistantReply(userText, {
let reply = await deps.services.generateAssistantReply(userText, {
requester: {
userId: message.author.userId,
userName: message.author.userName ?? fallbackIdentity?.userName,
Expand Down Expand Up @@ -398,16 +399,20 @@ export function createReplyToThread(deps: ReplyExecutorDeps) {
reply.diagnostics.errorMessage ??
"Provider error without explicit message",
);
logException(
const eventId = logException(
providerError,
"agent_turn_provider_error",
diagnosticsContext,
diagnosticsAttributes,
"Agent turn failed with provider error",
);
reply = {
...reply,
text: buildTurnFailureResponse(eventId),
};
} else if (reply.diagnostics.outcome !== "success") {
const failureReason = getExecutionFailureReason(reply);
logException(
const eventId = logException(
new Error(`Agent turn execution failure: ${failureReason}`),
"agent_turn_execution_failure",
diagnosticsContext,
Expand All @@ -417,6 +422,10 @@ export function createReplyToThread(deps: ReplyExecutorDeps) {
},
"Agent turn completed with execution failure",
);
reply = {
...reply,
text: buildTurnFailureResponse(eventId),
};
}

markConversationMessage(
Expand Down
22 changes: 3 additions & 19 deletions packages/junior/src/chat/runtime/slack-runtime.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import type { Message, Thread } from "chat";
import { getSubscribedReplyPreflightDecision } from "@/chat/services/subscribed-decision";
import { isRetryableTurnError } from "@/chat/runtime/turn";
import type { ErrorReference } from "@/chat/logging";
import { buildTurnFailureResponse } from "@/chat/logging";
import { getSlackErrorObservabilityAttributes } from "@/chat/runtime/thread-context";
import type { SubscribedReplyDecision } from "@/chat/services/subscribed-reply-policy";

Expand Down Expand Up @@ -89,7 +89,6 @@ export interface SlackTurnRuntimeDependencies<TPreparedState> {
) => void;
modelId: string;
now: () => number;
getErrorReference: (eventId?: string) => ErrorReference | null;
recordSkippedSubscribedMessage: (args: {
completedAtMs: number;
decision: SubscribedReplyDecision;
Expand Down Expand Up @@ -146,16 +145,6 @@ export interface SlackTurnRuntimeDependencies<TPreparedState> {
) => Promise<void>;
}

function buildFailureMessage(reference: ErrorReference | null): string {
if (!reference) {
return "I ran into an internal error while processing that. Please try again.";
}
if (reference.eventId) {
return `I ran into an internal error while processing that. Reference: \`event_id=${reference.eventId} trace_id=${reference.traceId}\`.`;
}
return `I ran into an internal error while processing that. Reference: \`trace_id=${reference.traceId}\`.`;
}

export interface SlackTurnRuntime<
_TPreparedState,
TAssistantEvent extends AssistantLifecycleEvent = AssistantLifecycleEvent,
Expand Down Expand Up @@ -215,14 +204,13 @@ export function createSlackTurnRuntime<

const postFallbackErrorReplyWithLogging = async (args: {
thread: Thread;
reference: ErrorReference | null;
errorContext: RuntimeLogContext;
eventId?: string;
eventId: string | undefined;
postFailureEventName: string;
postFailureBody: string;
}): Promise<void> => {
try {
await args.thread.post(buildFailureMessage(args.reference));
await args.thread.post(buildTurnFailureResponse(args.eventId));
} catch (postError) {
deps.logException(
postError,
Expand Down Expand Up @@ -336,10 +324,8 @@ export function createSlackTurnRuntime<
"onNewMention failed",
);
await hooks?.beforeFirstResponsePost?.();
const reference = deps.getErrorReference(eventId);
await postFallbackErrorReplyWithLogging({
thread,
reference,
errorContext,
eventId,
postFailureEventName: "mention_handler_failure_reply_post_failed",
Expand Down Expand Up @@ -492,10 +478,8 @@ export function createSlackTurnRuntime<
"onSubscribedMessage failed",
);
await hooks?.beforeFirstResponsePost?.();
const reference = deps.getErrorReference(eventId);
await postFallbackErrorReplyWithLogging({
thread,
reference,
errorContext,
eventId,
postFailureEventName:
Expand Down
3 changes: 1 addition & 2 deletions packages/junior/src/chat/services/turn-result.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import { isExplicitChannelPostIntent } from "@/chat/services/channel-intent";
import { enforceAttachmentClaimTruth } from "@/chat/services/attachment-claims";
import type { ThreadArtifactsState } from "@/chat/state/artifacts";
import {
buildExecutionFailureMessage,
extractAssistantText,
getTerminalAssistantMessages,
isAssistantMessage,
Expand Down Expand Up @@ -198,7 +197,7 @@ export function buildTurnResult(input: TurnResultInput): AssistantReply {
: sideEffectOnlySuccess
? "success"
: "execution_failure";
const fallbackText = buildExecutionFailureMessage(toolErrorCount);
const fallbackText = "";
Comment thread
cursor[bot] marked this conversation as resolved.
Outdated
Comment thread
cursor[bot] marked this conversation as resolved.
Outdated
const suppressReactionOnlyText =
reactionPerformed &&
!channelPostPerformed &&
Expand Down
16 changes: 2 additions & 14 deletions packages/junior/src/chat/slack/output.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import type { FileUpload, PostableMessage } from "chat";
import { logWarn } from "@/chat/logging";
import { renderSlackMrkdwn } from "@/chat/slack/mrkdwn";

const MAX_INLINE_CHARS = 2200;
Expand Down Expand Up @@ -338,20 +337,9 @@ export function buildSlackOutputMessage(
};
}

logWarn(
"slack_output_normalized_empty",
Comment thread
cursor[bot] marked this conversation as resolved.
{},
{
"app.output.original_length": text.length,
"app.output.parsed_length": normalized.length,
"app.output.file_count": fileCount,
},
"Slack output normalized to empty content",
throw new Error(
`Slack output normalized to empty content: original_length=${text.length} parsed_length=${normalized.length}`,
);
return {
markdown: "I couldn't produce a response.",
files,
};
}

return {
Expand Down
12 changes: 6 additions & 6 deletions packages/junior/tests/integration/slack/bot-handlers.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -484,12 +484,12 @@ describe("bot handlers (integration)", () => {
);

expect(thread.posts).toHaveLength(1);
expect(thread.posts[0]).toEqual(
expect.objectContaining({
markdown:
"Partial output...\n\n[Response interrupted before completion]",
}),
);
const postText =
typeof thread.posts[0] === "string"
? thread.posts[0]
: ((thread.posts[0] as { markdown?: string }).markdown ?? "");
expect(postText).toContain("I ran into an internal error");
expect(postText).toContain("event_id=");
});

it("emits assistant status updates in shared channel threads", async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -861,9 +861,7 @@ describe("generateAssistantReply progressive MCP loading", () => {
}),
);

expect(reply.text).toBe(
"I couldn't complete this request in this turn due to an execution failure. I've logged the details for debugging.",
);
expect(reply.text).toBe("");
expect(reply.diagnostics.outcome).toBe("execution_failure");
expect(reply.diagnostics.usedPrimaryText).toBe(false);
});
Expand Down
1 change: 0 additions & 1 deletion packages/junior/tests/unit/runtime/slack-runtime.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ describe("createSlackTurnRuntime", () => {
assistantUserName: "junior",
decideSubscribedReply,
getChannelId: () => "C123",
getErrorReference: () => null,
getPreparedConversationContext: () => "prior thread context",
getRunId: () => "run_123",
getThreadId: () => "thread_123",
Expand Down
18 changes: 6 additions & 12 deletions packages/junior/tests/unit/slack/slack-runtime.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ function createMockDeps(
assistantUserName: "test-bot",
modelId: "test-model",
now: () => 1700000000000,
getErrorReference: () => null,
getChannelId: (_thread, message) => message.threadId?.split(":")[1],
getThreadId: (_thread, message) => message.threadId,
getRunId: () => undefined,
Expand Down Expand Up @@ -77,7 +76,7 @@ describe("createSlackTurnRuntime", () => {
await runtime.handleNewMention(thread, message);

expect(thread.posts).toContain(
"I ran into an internal error while processing that. Please try again.",
"I ran into an internal error while processing that. Reference: `event_id=unknown`.",
);
});

Expand All @@ -97,7 +96,7 @@ describe("createSlackTurnRuntime", () => {
await runtime.handleNewMention(thread, message);

expect(thread.posts).toContain(
"I ran into an internal error while processing that. Please try again.",
"I ran into an internal error while processing that. Reference: `event_id=unknown`.",
);
});

Expand All @@ -107,10 +106,6 @@ describe("createSlackTurnRuntime", () => {
replyToThread: vi.fn().mockRejectedValue(replyError),
withSpan: vi.fn(async (_n, _o, _c, cb) => cb()),
logException: vi.fn(() => "evt_123"),
getErrorReference: () => ({
eventId: "evt_123",
traceId: "trace_ignored",
}),
});
const runtime = createSlackTurnRuntime<TestState>(deps);
const thread = createTestThread({});
Expand All @@ -119,17 +114,16 @@ describe("createSlackTurnRuntime", () => {
await runtime.handleNewMention(thread, message);

expect(thread.posts).toContain(
"I ran into an internal error while processing that. Reference: `event_id=evt_123 trace_id=trace_ignored`.",
"I ran into an internal error while processing that. Reference: `event_id=evt_123`.",
);
});

it("falls back to trace id when sentry event id is unavailable", async () => {
it("falls back to generic message 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),
getErrorReference: () => ({ traceId: "trace_123" }),
});
const runtime = createSlackTurnRuntime<TestState>(deps);
const thread = createTestThread({});
Expand All @@ -138,7 +132,7 @@ describe("createSlackTurnRuntime", () => {
await runtime.handleNewMention(thread, message);

expect(thread.posts).toContain(
"I ran into an internal error while processing that. Reference: `trace_id=trace_123`.",
"I ran into an internal error while processing that. Reference: `event_id=unknown`.",
);
});
});
Expand Down Expand Up @@ -384,7 +378,7 @@ describe("createSlackTurnRuntime", () => {
await runtime.handleSubscribedMessage(thread, message);

expect(thread.posts).toContain(
"I ran into an internal error while processing that. Please try again.",
"I ran into an internal error while processing that. Reference: `event_id=unknown`.",
);
});
});
Expand Down
8 changes: 2 additions & 6 deletions packages/junior/tests/unit/turn-result.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,7 @@ describe("buildTurnResult", () => {
thinkingSelection,
});

expect(reply.text).toBe(
"I couldn't complete this request in this turn due to an execution failure. I've logged the details for debugging.",
);
expect(reply.text).toBe("");
expect(reply.diagnostics.outcome).toBe("execution_failure");
});

Expand Down Expand Up @@ -73,9 +71,7 @@ describe("buildTurnResult", () => {
thinkingSelection,
});

expect(reply.text).toBe(
"I couldn't complete this request in this turn due to an execution failure. I've logged the details for debugging.",
);
expect(reply.text).toBe("");
expect(reply.diagnostics.outcome).toBe("execution_failure");
expect(reply.diagnostics.usedPrimaryText).toBe(false);
});
Expand Down
Loading