Skip to content

Commit 0addf5c

Browse files
committed
fix(threads): stop turn/steer fallback to turn/start
1 parent 28acc72 commit 0addf5c

3 files changed

Lines changed: 35 additions & 174 deletions

File tree

src/features/threads/hooks/useThreadMessaging.test.tsx

Lines changed: 13 additions & 95 deletions
Original file line numberDiff line numberDiff line change
@@ -204,6 +204,7 @@ describe("useThreadMessaging telemetry", () => {
204204
});
205205

206206
it("uses turn/steer when steer mode is enabled and an active turn is present", async () => {
207+
const dispatch = vi.fn();
207208
const { result } = renderHook(() =>
208209
useThreadMessaging({
209210
activeWorkspace: workspace,
@@ -229,7 +230,7 @@ describe("useThreadMessaging telemetry", () => {
229230
},
230231
rateLimitsByWorkspace: {},
231232
pendingInterruptsRef: { current: new Set<string>() },
232-
dispatch: vi.fn(),
233+
dispatch,
233234
getCustomName: vi.fn(() => undefined),
234235
markProcessing: vi.fn(),
235236
markReviewing: vi.fn(),
@@ -263,14 +264,15 @@ describe("useThreadMessaging telemetry", () => {
263264
[],
264265
);
265266
expect(sendUserMessageService).not.toHaveBeenCalled();
267+
expect(dispatch).not.toHaveBeenCalledWith(
268+
expect.objectContaining({ type: "upsertItem" }),
269+
);
266270
});
267271

268-
it("falls back to turn/start when turn/steer is unsupported and remembers fallback", async () => {
272+
it("does not fall back to turn/start when turn/steer fails", async () => {
273+
const pushThreadErrorMessage = vi.fn();
269274
vi.mocked(steerTurnService).mockResolvedValueOnce({
270-
error: {
271-
message:
272-
"Invalid request: unknown variant `turn/steer`, expected one of `turn/start`, `turn/interrupt`",
273-
},
275+
error: { message: "no active turn to steer" },
274276
} as unknown as Awaited<ReturnType<typeof steerTurnService>>);
275277

276278
const { result } = renderHook(() =>
@@ -306,89 +308,7 @@ describe("useThreadMessaging telemetry", () => {
306308
recordThreadActivity: vi.fn(),
307309
safeMessageActivity: vi.fn(),
308310
onDebug: vi.fn(),
309-
pushThreadErrorMessage: vi.fn(),
310-
ensureThreadForActiveWorkspace: vi.fn(async () => "thread-1"),
311-
ensureThreadForWorkspace: vi.fn(async () => "thread-1"),
312-
refreshThread: vi.fn(async () => null),
313-
forkThreadForWorkspace: vi.fn(async () => null),
314-
updateThreadParent: vi.fn(),
315-
}),
316-
);
317-
318-
await act(async () => {
319-
await result.current.sendUserMessageToThread(
320-
workspace,
321-
"thread-1",
322-
"fallback once",
323-
[],
324-
);
325-
});
326-
await act(async () => {
327-
await result.current.sendUserMessageToThread(
328-
workspace,
329-
"thread-1",
330-
"fallback twice",
331-
[],
332-
);
333-
});
334-
335-
expect(steerTurnService).toHaveBeenCalledTimes(1);
336-
expect(sendUserMessageService).toHaveBeenCalledTimes(2);
337-
expect(sendUserMessageService).toHaveBeenNthCalledWith(
338-
1,
339-
"ws-1",
340-
"thread-1",
341-
"fallback once",
342-
expect.any(Object),
343-
);
344-
expect(sendUserMessageService).toHaveBeenNthCalledWith(
345-
2,
346-
"ws-1",
347-
"thread-1",
348-
"fallback twice",
349-
expect.any(Object),
350-
);
351-
});
352-
353-
it("falls back to turn/start when remote reports unknown method turn_steer", async () => {
354-
vi.mocked(steerTurnService).mockRejectedValueOnce(
355-
new Error("unknown method: turn_steer"),
356-
);
357-
358-
const { result } = renderHook(() =>
359-
useThreadMessaging({
360-
activeWorkspace: workspace,
361-
activeThreadId: "thread-1",
362-
accessMode: "current",
363-
model: null,
364-
effort: null,
365-
collaborationMode: null,
366-
reviewDeliveryMode: "inline",
367-
steerEnabled: true,
368-
customPrompts: [],
369-
threadStatusById: {
370-
"thread-1": {
371-
isProcessing: true,
372-
isReviewing: false,
373-
hasUnread: false,
374-
processingStartedAt: 0,
375-
lastDurationMs: null,
376-
},
377-
},
378-
activeTurnIdByThread: {
379-
"thread-1": "turn-1",
380-
},
381-
rateLimitsByWorkspace: {},
382-
pendingInterruptsRef: { current: new Set<string>() },
383-
dispatch: vi.fn(),
384-
getCustomName: vi.fn(() => undefined),
385-
markProcessing: vi.fn(),
386-
markReviewing: vi.fn(),
387-
setActiveTurnId: vi.fn(),
388-
recordThreadActivity: vi.fn(),
389-
safeMessageActivity: vi.fn(),
390-
onDebug: vi.fn(),
391-
pushThreadErrorMessage: vi.fn(),
311+
pushThreadErrorMessage,
392312
ensureThreadForActiveWorkspace: vi.fn(async () => "thread-1"),
393313
ensureThreadForWorkspace: vi.fn(async () => "thread-1"),
394314
refreshThread: vi.fn(async () => null),
@@ -401,18 +321,16 @@ describe("useThreadMessaging telemetry", () => {
401321
await result.current.sendUserMessageToThread(
402322
workspace,
403323
"thread-1",
404-
"fallback remote method",
324+
"steer should fail",
405325
[],
406326
);
407327
});
408328

409329
expect(steerTurnService).toHaveBeenCalledTimes(1);
410-
expect(sendUserMessageService).toHaveBeenCalledTimes(1);
411-
expect(sendUserMessageService).toHaveBeenCalledWith(
412-
"ws-1",
330+
expect(sendUserMessageService).not.toHaveBeenCalled();
331+
expect(pushThreadErrorMessage).toHaveBeenCalledWith(
413332
"thread-1",
414-
"fallback remote method",
415-
expect.any(Object),
333+
"Turn steer failed: no active turn to steer",
416334
);
417335
});
418336
});

src/features/threads/hooks/useThreadMessaging.ts

Lines changed: 22 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { useCallback, useRef } from "react";
1+
import { useCallback } from "react";
22
import type { Dispatch, MutableRefObject } from "react";
33
import * as Sentry from "@sentry/react";
44
import type {
@@ -26,7 +26,6 @@ import {
2626
extractRpcErrorMessage,
2727
parseReviewTarget,
2828
} from "@threads/utils/threadNormalize";
29-
import { isUnsupportedTurnSteerError } from "@threads/utils/threadRpc";
3029
import type { ThreadAction, ThreadState } from "./useThreadsReducer";
3130
import { useReviewPrompt } from "./useReviewPrompt";
3231
import { formatRelativeTime } from "@utils/time";
@@ -113,8 +112,6 @@ export function useThreadMessaging({
113112
updateThreadParent,
114113
registerDetachedReviewChild,
115114
}: UseThreadMessagingOptions) {
116-
const steerSupportedByWorkspaceRef = useRef<Record<string, boolean>>({});
117-
118115
const sendMessageToThread = useCallback(
119116
async (
120117
workspace: WorkspaceInfo,
@@ -157,29 +154,8 @@ export function useThreadMessaging({
157154

158155
const isProcessing = threadStatusById[threadId]?.isProcessing ?? false;
159156
const activeTurnId = activeTurnIdByThread[threadId] ?? null;
160-
const steerSupported = steerSupportedByWorkspaceRef.current[workspace.id] !== false;
161157
const shouldSteer =
162-
isProcessing && steerEnabled && Boolean(activeTurnId) && steerSupported;
163-
if (isProcessing && steerEnabled) {
164-
const optimisticText = finalText;
165-
if (optimisticText || images.length > 0) {
166-
dispatch({
167-
type: "upsertItem",
168-
workspaceId: workspace.id,
169-
threadId,
170-
item: {
171-
id: `optimistic-user-${Date.now()}-${Math.random()
172-
.toString(36)
173-
.slice(2, 8)}`,
174-
kind: "message",
175-
role: "user",
176-
text: optimisticText,
177-
images: images.length > 0 ? images : undefined,
178-
},
179-
hasCustomName: Boolean(getCustomName(workspace.id, threadId)),
180-
});
181-
}
182-
}
158+
isProcessing && steerEnabled && Boolean(activeTurnId);
183159
Sentry.metrics.count("prompt_sent", 1, {
184160
attributes: {
185161
workspace_id: workspace.id,
@@ -217,7 +193,7 @@ export function useThreadMessaging({
217193
collaborationMode: sanitizedCollaborationMode,
218194
},
219195
});
220-
let requestMode: "start" | "steer" = shouldSteer ? "steer" : "start";
196+
const requestMode: "start" | "steer" = shouldSteer ? "steer" : "start";
221197
try {
222198
const startTurn = () => {
223199
const payload: {
@@ -245,49 +221,26 @@ export function useThreadMessaging({
245221
);
246222
};
247223

248-
let response: Record<string, unknown>;
249-
if (shouldSteer) {
250-
try {
251-
response = (await (appMentions.length > 0
252-
? steerTurnService(
253-
workspace.id,
254-
threadId,
255-
activeTurnId ?? "",
256-
finalText,
257-
images,
258-
appMentions,
259-
)
260-
: steerTurnService(
261-
workspace.id,
262-
threadId,
263-
activeTurnId ?? "",
264-
finalText,
265-
images,
266-
))) as Record<string, unknown>;
267-
} catch (error) {
268-
const message = error instanceof Error ? error.message : String(error);
269-
if (!isUnsupportedTurnSteerError(message)) {
270-
throw error;
271-
}
272-
steerSupportedByWorkspaceRef.current[workspace.id] = false;
273-
requestMode = "start";
274-
response = (await startTurn()) as Record<string, unknown>;
275-
}
276-
} else {
277-
response = (await startTurn()) as Record<string, unknown>;
278-
}
224+
const response: Record<string, unknown> = shouldSteer
225+
? (await (appMentions.length > 0
226+
? steerTurnService(
227+
workspace.id,
228+
threadId,
229+
activeTurnId ?? "",
230+
finalText,
231+
images,
232+
appMentions,
233+
)
234+
: steerTurnService(
235+
workspace.id,
236+
threadId,
237+
activeTurnId ?? "",
238+
finalText,
239+
images,
240+
))) as Record<string, unknown>
241+
: (await startTurn()) as Record<string, unknown>;
279242

280-
let rpcError = extractRpcErrorMessage(response);
281-
if (
282-
rpcError
283-
&& requestMode === "steer"
284-
&& isUnsupportedTurnSteerError(rpcError)
285-
) {
286-
steerSupportedByWorkspaceRef.current[workspace.id] = false;
287-
requestMode = "start";
288-
response = (await startTurn()) as Record<string, unknown>;
289-
rpcError = extractRpcErrorMessage(response);
290-
}
243+
const rpcError = extractRpcErrorMessage(response);
291244

292245
onDebug?.({
293246
id: `${Date.now()}-${requestMode === "steer" ? "server-turn-steer" : "server-turn-start"}`,

src/features/threads/utils/threadRpc.ts

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -61,13 +61,3 @@ export function getResumedActiveTurnId(thread: Record<string, unknown>): string
6161
}
6262
return null;
6363
}
64-
65-
export function isUnsupportedTurnSteerError(message: string): boolean {
66-
const normalized = message.toLowerCase();
67-
const mentionsSteerMethod =
68-
normalized.includes("turn/steer") || normalized.includes("turn_steer");
69-
return normalized.includes("unknown variant `turn/steer`")
70-
|| normalized.includes("unknown variant \"turn/steer\"")
71-
|| (normalized.includes("unknown request") && mentionsSteerMethod)
72-
|| (normalized.includes("unknown method") && mentionsSteerMethod);
73-
}

0 commit comments

Comments
 (0)