Skip to content

Commit 695be31

Browse files
committed
fix: preserve resumable auth and web batches
1 parent c55994d commit 695be31

9 files changed

Lines changed: 370 additions & 56 deletions

File tree

packages/junior/src/chat/runtime/slack-turn.ts

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -981,6 +981,7 @@ export function createSlackTurn(deps: SlackTurnDeps) {
981981
// Once model output is settled, later commit errors must not trigger a
982982
// second visible failure reply.
983983
let runResultHandled = false;
984+
let authPauseAccepted = false;
984985
let acceptedDeliveryId: string | undefined;
985986
let turnCompletionNotified = false;
986987
const recordDispatchOutcome = async (
@@ -1516,9 +1517,12 @@ export function createSlackTurn(deps: SlackTurnDeps) {
15161517
conversation: preparedState.conversation,
15171518
sessionId: turnId,
15181519
});
1519-
await persistThreadState(thread, {
1520-
conversation: preparedState.conversation,
1521-
});
1520+
authPauseAccepted = true;
1521+
await persistWithRetry(() =>
1522+
persistThreadState(thread, {
1523+
conversation: preparedState.conversation,
1524+
}),
1525+
);
15221526
persistedAtLeastOnce = true;
15231527
shouldPersistFailureState = false;
15241528
return;
@@ -1602,6 +1606,24 @@ export function createSlackTurn(deps: SlackTurnDeps) {
16021606
}
16031607
}
16041608
} catch (error) {
1609+
if (authPauseAccepted && !persistedAtLeastOnce) {
1610+
// The private auth link is already visible. Reconcile the paused
1611+
// state without turning a resumable Turn into a terminal failure.
1612+
logException(error, "slack.auth_pause_state_persist.failed");
1613+
try {
1614+
await persistThreadState(thread, {
1615+
conversation: preparedState.conversation,
1616+
});
1617+
persistedAtLeastOnce = true;
1618+
shouldPersistFailureState = false;
1619+
return;
1620+
} catch (recoveryError) {
1621+
logException(
1622+
recoveryError,
1623+
"slack.auth_pause_state_recovery.failed",
1624+
);
1625+
}
1626+
}
16051627
if (runResultHandled) {
16061628
// Errors after the completed run produced output or intentional
16071629
// silence (redundant-ack cleanup, completion callbacks) must not

packages/junior/src/chat/task-execution/web-cancellation.ts

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -92,18 +92,20 @@ export async function completeCancelledWebTurn(args: {
9292
sandboxRef?: SandboxRef;
9393
signal?: AbortSignal;
9494
turnId: string;
95-
userMessageId: string;
95+
userMessageIds: readonly string[];
9696
}): Promise<void> {
9797
await abandonTurnRecord({
9898
conversationId: args.conversationId,
9999
turnId: args.turnId,
100100
errorMessage: "Web Turn cancelled",
101101
});
102102
clearPendingAuth(args.conversation, args.turnId);
103-
markConversationMessage(args.conversation, args.userMessageId, {
104-
replied: false,
105-
skippedReason: "turn cancelled",
106-
});
103+
for (const userMessageId of args.userMessageIds) {
104+
markConversationMessage(args.conversation, userMessageId, {
105+
replied: false,
106+
skippedReason: "turn cancelled",
107+
});
108+
}
107109
markTurnClosed({
108110
conversation: args.conversation,
109111
nowMs: Date.now(),

packages/junior/src/chat/task-execution/web-work.ts

Lines changed: 119 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -206,6 +206,7 @@ export function createWebWorker(options: {
206206
let text: string;
207207
let turnId: string;
208208
let userMessageId: string;
209+
let userMessageIds: string[];
209210
let startedAtMs: number;
210211

211212
const storedConversation = await getConversationStore().get({
@@ -238,6 +239,7 @@ export function createWebWorker(options: {
238239

239240
if (resolved.kind === "mailbox") {
240241
const batch = mailboxBatch?.length ? mailboxBatch : resolved.batch;
242+
mailboxBatch = batch;
241243
const first = batch[0]!;
242244
text = batch
243245
.map((entry) => entry.message.input.text.trim())
@@ -246,6 +248,7 @@ export function createWebWorker(options: {
246248
actor = actorFromMetadata(first.metadata);
247249
turnId = webTurnIdForMessage(first.metadata.messageId);
248250
userMessageId = first.metadata.messageId;
251+
userMessageIds = batch.map((entry) => entry.metadata.messageId);
249252
startedAtMs = first.message.createdAtMs;
250253
} else {
251254
turnId = resolved.turnId;
@@ -266,6 +269,7 @@ export function createWebWorker(options: {
266269
});
267270
text = userMessage.text;
268271
userMessageId = userMessage.id;
272+
userMessageIds = [userMessage.id];
269273
startedAtMs = userMessage.createdAtMs;
270274
}
271275

@@ -292,6 +296,23 @@ export function createWebWorker(options: {
292296
...(actor.userName ? { userName: actor.userName } : {}),
293297
},
294298
async () => {
299+
const markUserMessages = (
300+
target: typeof conversation,
301+
patch: { replied?: boolean; skippedReason?: string },
302+
): void => {
303+
for (const messageId of userMessageIds) {
304+
markConversationMessage(target, messageId, patch);
305+
}
306+
};
307+
const buildInstructionContext = (): string | undefined => {
308+
const currentMessageIds = new Set(userMessageIds);
309+
return buildConversationContext({
310+
...conversation,
311+
messages: conversation.messages.filter(
312+
(message) => !currentMessageIds.has(message.id),
313+
),
314+
});
315+
};
295316
let acknowledged = isResume || context.attempt.messages.length === 0;
296317
const acknowledge = async (): Promise<void> => {
297318
if (acknowledged) {
@@ -370,33 +391,38 @@ export function createWebWorker(options: {
370391
conversationId: context.conversationId,
371392
});
372393
}
373-
upsertConversationMessage(conversation, {
374-
id: userMessageId,
375-
role: "user",
376-
text: normalizeConversationText(text),
377-
createdAtMs: startedAtMs,
378-
author: {
379-
...(actor.email ? { email: actor.email } : {}),
380-
...(actor.fullName ? { fullName: actor.fullName } : {}),
381-
userId: actor.userId,
382-
...(actor.userName ? { userName: actor.userName } : {}),
383-
},
384-
meta: {
385-
explicitMention: true,
386-
replied: false,
387-
source: "web",
388-
},
389-
});
394+
for (const entry of mailboxBatch!) {
395+
const messageActor = actorFromMetadata(entry.metadata);
396+
upsertConversationMessage(conversation, {
397+
id: entry.metadata.messageId,
398+
role: "user",
399+
text: normalizeConversationText(entry.message.input.text),
400+
createdAtMs: entry.message.createdAtMs,
401+
author: {
402+
...(messageActor.email ? { email: messageActor.email } : {}),
403+
...(messageActor.fullName
404+
? { fullName: messageActor.fullName }
405+
: {}),
406+
userId: messageActor.userId,
407+
...(messageActor.userName
408+
? { userName: messageActor.userName }
409+
: {}),
410+
},
411+
meta: {
412+
explicitMention: true,
413+
replied: false,
414+
source: "web",
415+
},
416+
});
417+
}
390418
await persistConversationMessages({
391419
conversation,
392420
conversationId: context.conversationId,
393421
});
394422
await lifecycle.start({
395423
conversationId: context.conversationId,
396424
createdAtMs: Date.now(),
397-
inputMessageIds: mailboxBatch!.map(
398-
(entry) => entry.metadata.messageId,
399-
),
425+
inputMessageIds: userMessageIds,
400426
surface: "api",
401427
turnId,
402428
});
@@ -409,6 +435,7 @@ export function createWebWorker(options: {
409435
let currentRunId: string | undefined;
410436
let failureCode: ConversationTurnFailureCode = "persistence_failed";
411437
let assistantMessageAccepted = false;
438+
let authPauseAccepted = false;
412439
let cancellationCleanupStarted = false;
413440
let terminalConversationStateCommitted = false;
414441
const completeCancelledTurn =
@@ -425,7 +452,7 @@ export function createWebWorker(options: {
425452
sandboxRef,
426453
signal: cancellationSignal,
427454
turnId,
428-
userMessageId,
455+
userMessageIds,
429456
});
430457
} catch (error) {
431458
if (hasLostTurnInputCommit(error)) {
@@ -458,6 +485,10 @@ export function createWebWorker(options: {
458485
text: replyText,
459486
userMessageId,
460487
});
488+
markUserMessages(conversation, {
489+
replied: true,
490+
skippedReason: undefined,
491+
});
461492
assistantMessageAccepted = true;
462493
try {
463494
await persistWithRetry(() =>
@@ -515,6 +546,10 @@ export function createWebWorker(options: {
515546
sessionId: turnId,
516547
userMessageId,
517548
});
549+
markUserMessages(completedState.conversation, {
550+
replied: true,
551+
skippedReason: undefined,
552+
});
518553
await persistThreadStateById(context.conversationId, {
519554
conversation: completedState.conversation,
520555
sandboxRef: result.sandboxRef ?? sandboxRef,
@@ -558,6 +593,12 @@ export function createWebWorker(options: {
558593
markConversationMessage,
559594
});
560595
}
596+
markUserMessages(
597+
conversation,
598+
turnHasReply(conversation, turnId)
599+
? { replied: true, skippedReason: undefined }
600+
: { replied: false, skippedReason: "reply failed" },
601+
);
561602
try {
562603
await persistThreadStateById(context.conversationId, {
563604
conversation,
@@ -594,9 +635,7 @@ export function createWebWorker(options: {
594635
runId: currentRunId,
595636
instruction: {
596637
text,
597-
context: buildConversationContext(conversation, {
598-
excludeMessageId: userMessageId,
599-
}),
638+
context: buildInstructionContext(),
600639
},
601640
history: piMessages,
602641
actor,
@@ -651,10 +690,17 @@ export function createWebWorker(options: {
651690
conversation,
652691
sessionId: turnId,
653692
});
654-
await persistThreadStateById(context.conversationId, {
655-
conversation,
656-
sandboxRef,
693+
markUserMessages(conversation, {
694+
replied: true,
695+
skippedReason: undefined,
657696
});
697+
authPauseAccepted = true;
698+
await persistWithRetry(() =>
699+
persistThreadStateById(context.conversationId, {
700+
conversation,
701+
sandboxRef,
702+
}),
703+
);
658704
if (cancellationSignal) {
659705
options.cancellation?.park(
660706
context.conversationId,
@@ -722,6 +768,46 @@ export function createWebWorker(options: {
722768
}
723769
return await completeCancelledTurn();
724770
}
771+
if (authPauseAccepted) {
772+
// The auth request is already visible. Reconcile the paused state
773+
// without turning a resumable Turn into a terminal failure.
774+
captureWebBoundaryFailure({
775+
conversationId: context.conversationId,
776+
error,
777+
failureCode: "persistence_failed",
778+
runId: currentRunId,
779+
turnId,
780+
});
781+
try {
782+
await persistThreadStateById(context.conversationId, {
783+
conversation,
784+
sandboxRef,
785+
});
786+
} catch (persistenceError) {
787+
if (context.attempt.isFinalAttempt) {
788+
finishCancellation();
789+
}
790+
throw new AggregateError(
791+
[error, persistenceError],
792+
"Web auth pause state could not be persisted",
793+
);
794+
}
795+
if (cancellationSignal) {
796+
options.cancellation?.park(
797+
context.conversationId,
798+
cancellationSignal,
799+
);
800+
}
801+
try {
802+
await acknowledge();
803+
} catch (acknowledgeError) {
804+
if (hasLostTurnInputCommit(acknowledgeError)) {
805+
return { status: "lost_lease" };
806+
}
807+
throw acknowledgeError;
808+
}
809+
return { status: "completed" };
810+
}
725811
if (!context.attempt.isFinalAttempt) {
726812
throw error;
727813
}
@@ -753,6 +839,12 @@ export function createWebWorker(options: {
753839
markConversationMessage,
754840
});
755841
}
842+
markUserMessages(
843+
conversation,
844+
turnHasReply(conversation, turnId) || userMessageHandled
845+
? { replied: true, skippedReason: undefined }
846+
: { replied: false, skippedReason: "reply failed" },
847+
);
756848
await persistThreadStateById(context.conversationId, {
757849
conversation,
758850
sandboxRef: initialSandboxRef ?? null,

packages/junior/tests/component/auth/mcp-auth-runtime-slack.test.ts

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -424,7 +424,7 @@ describe("mcp auth runtime slack integration", () => {
424424
process.env = { ...ORIGINAL_ENV };
425425
}, 45_000);
426426

427-
it("parks an MCP auth challenge from the real Slack runtime and resumes after OAuth callback", async () => {
427+
it("keeps a delivered MCP auth challenge resumable after a transient state failure", async () => {
428428
const threadId = "slack:C123:1700000000.001";
429429
const turnId = "turn_user-1";
430430
const { createTestChatRuntime } = chatRuntimeModule;
@@ -461,6 +461,22 @@ describe("mcp auth runtime slack integration", () => {
461461
},
462462
});
463463
await mirrorThreadStateToAdapter(thread);
464+
const stateAdapter = stateAdapterModule.getStateAdapter();
465+
const setState = stateAdapter.set.bind(stateAdapter);
466+
let authPauseStateFailures = 0;
467+
vi.spyOn(stateAdapter, "set").mockImplementation(
468+
async (key, value, ttlMs) => {
469+
if (
470+
authPauseStateFailures < 3 &&
471+
key === `thread-state:${threadId}` &&
472+
getCapturedSlackApiCalls("chat.postEphemeral").length > 0
473+
) {
474+
authPauseStateFailures += 1;
475+
throw new Error("thread state unavailable");
476+
}
477+
await setState(key, value, ttlMs);
478+
},
479+
);
464480

465481
await slackRuntime.handleNewMention(
466482
thread,
@@ -485,6 +501,7 @@ describe("mcp auth runtime slack integration", () => {
485501

486502
expect(agentProbe.promptCallCount).toBe(1);
487503
expect(agentProbe.continueCallCount).toBe(0);
504+
expect(authPauseStateFailures).toBe(3);
488505

489506
expect(getCapturedSlackApiCalls("chat.postEphemeral")).toEqual([
490507
expect.objectContaining({

0 commit comments

Comments
 (0)