From 9ff7da5ec4d67e683b23517d2bb16fdd2fd84399 Mon Sep 17 00:00:00 2001 From: Renfei Zhou Date: Sat, 14 Nov 2026 13:35:08 +0000 Subject: [PATCH 1/2] test(server): cover one new main-list message per user message in Chat + Replies In Chat + Replies the agent may not send two messages in a row in the main list, but send_to_user accepts every new_sub_thread call. These cases fail on the current code: a second new message is accepted, and the tools' history read stops 120 messages back, before the main list's latest message. Co-Authored-By: Claude Opus 5.5 --- .../displayModeWrap/DisplayModeWrap.test.ts | 36 ++++++ .../SendToUserSubThreads.test.ts | 111 +++++++++++++++++- 2 files changed, 144 insertions(+), 3 deletions(-) diff --git a/apps/server/src/fork/displayModeWrap/DisplayModeWrap.test.ts b/apps/server/src/fork/displayModeWrap/DisplayModeWrap.test.ts index 592fcd132de64..03cd348ad15ee 100644 --- a/apps/server/src/fork/displayModeWrap/DisplayModeWrap.test.ts +++ b/apps/server/src/fork/displayModeWrap/DisplayModeWrap.test.ts @@ -54,6 +54,7 @@ const newId = ThreadId.make("thread:display-wrap:new"); const rewrittenId = ThreadId.make("thread:display-wrap:rewritten"); const latestId = ThreadId.make("thread:display-wrap:latest"); const ownCallId = ThreadId.make("thread:display-wrap:own-call"); +const mainFirstId = ThreadId.make("thread:display-wrap:main-first"); const switchId = ThreadId.make("thread:display-wrap:switch"); const repliesId = ThreadId.make("thread:display-wrap:replies"); const providerThreadId = ProviderThreadId.make("provider-thread:display-wrap"); @@ -658,6 +659,41 @@ it.effect("counts back from the caller's own item when it names one", () => }).pipe(Effect.provide(Layer.merge(store, wrapper))), ); +it.effect("reads back to the latest message of the main list when asked, past many replies", () => + Effect.gen(function* () { + const now = yield* DateTime.now; + const projections = yield* ProjectionStore.ProjectionStoreV2; + const wrap = yield* DisplayModeWrap.DisplayModeWrap; + const news = itemUpdated(now, mainFirstId, runId, 2, { type: "send_to_user", text: "News." }); + const replies = Array.from({ length: 125 }, (_, index) => + itemUpdated(now, mainFirstId, runId, index + 3, { + type: "user_message", + messageId: MessageId.make(`message:display-wrap:main-first:${index + 3}`), + text: `Reply ${index + 3}`, + inputIntent: "steer", + under: `item:${mainFirstId}:2`, + }), + ); + const events: ReadonlyArray = [ + threadCreated(now, mainFirstId, null), + runCreated(now, mainFirstId, runId, "running"), + itemUpdated(now, mainFirstId, runId, 1, { + type: "user_message", + messageId: MessageId.make("message:display-wrap:main-first:1"), + text: "Ship it.", + inputIntent: "turn_start", + }), + news, + ...replies, + ]; + yield* Effect.forEach(events, (event) => projections.apply(event), { discard: true }); + const first = (items: ReadonlyArray) => items[0]?.ordinal; + assert.strictEqual(first(yield* wrap.latestShownItems(mainFirstId, undefined, 120)), 8); + // The agent's news at 2 is the main list's latest message; the user's first message is not needed. + assert.strictEqual(first(yield* wrap.latestShownItems(mainFirstId, undefined, 120, true)), 2); + }).pipe(Effect.provide(Layer.merge(store, wrapper))), +); + it.effect("looks back past many of the user's messages to find the agent's earlier work", () => Effect.gen(function* () { const now = yield* DateTime.now; diff --git a/apps/server/src/fork/userMessageTools/SendToUserSubThreads.test.ts b/apps/server/src/fork/userMessageTools/SendToUserSubThreads.test.ts index d0a7764bf2099..86e01536f09d5 100644 --- a/apps/server/src/fork/userMessageTools/SendToUserSubThreads.test.ts +++ b/apps/server/src/fork/userMessageTools/SendToUserSubThreads.test.ts @@ -132,7 +132,11 @@ const layer = McpHttpServer.toolkitRegistration( ), Layer.provide( Layer.mock(DisplayModeWrap)({ - latestShownItems: () => Effect.succeed(history), + // As the real read: the latest `cores` messages, or back to the main list's latest message. + latestShownItems: (_threadId, _ownNativeId, cores, toMainListMessage) => + Effect.succeed( + cores === undefined || toMainListMessage === true ? history : history.slice(-cores), + ), forgetNotesAfterCompaction: () => (ingest) => ingest, }), ), @@ -203,7 +207,7 @@ it.effect("refuses a send that names neither a message to answer nor a new sub-t it.effect("shows a new message and returns the id that names it", () => Effect.gen(function* () { - setup(twoSubThreads()); + setup([...twoSubThreads(), typed("q3", "Anything else?")]); const result = yield* call("send_to_user", { text: "CI is red on main.", new_sub_thread: true, @@ -215,7 +219,9 @@ it.effect("shows a new message and returns the id that names it", () => messageId: shortId("CI is red on main."), }); expect(changes).toEqual([]); - // A second send with the same words, before the first is in the history, gets its own id. + // The same words again after the user writes in the main list, before the first send is in + // the history, get their own id. + setup([...twoSubThreads(), typed("q3", "Anything else?"), typed("q4", "And the docs?")]); const together = yield* call("send_to_user", { text: "CI is red on main.", new_sub_thread: true, @@ -224,6 +230,7 @@ it.effect("shows a new message and returns the id that names it", () => messageId: shortId("CI is red on main.\n1"), }); // The same words as an earlier first message get an id of their own, kept in the result. + setup([...twoSubThreads(), typed("q5", "And the tests?")]); const again = yield* call("send_to_user", { text: "The deploy is done.", new_sub_thread: true, @@ -236,6 +243,104 @@ it.effect("shows a new message and returns the id that names it", () => }).pipe(Effect.provide(layer)), ); +it.effect("takes one new message in the main list until the user sends one there", () => + Effect.gen(function* () { + const news2 = { text: "CI is red on main.", new_sub_thread: true }; + const taken = + "Refused: you already sent a new message to the main list, and the user has not written there since. You may not send two in a row there."; + const delivering = + "That message is still being delivered. To change it once it shows, call replace_send_to_user with sub_thread set to the id that its send returned."; + const unanswered = (...ids: ReadonlyArray) => [ + "Messages from the user with no answer yet, by id:", + ...ids.map(([id, text]) => `"${shortId(id)}": ${text}`), + ]; + const running = (item: OrchestrationV2TurnItem) => ({ ...item, status: "running" }) as const; + + // The user's replies under the agent's news are in its sub-thread, not in the main list. + setup(twoSubThreads()); + expect(refusalLines(yield* call("send_to_user", news2))).toEqual([ + taken, + "The user has replied under it, so it stays as it is.", + ...unanswered(["why", "Which build was deployed?"], ["q2", "And the lint job?"]), + DONE, + ]); + // However long the sub-thread under it grows. + setup([ + typed("q0", "Ship it."), + news("n0", "Shipped."), + ...Array.from({ length: 130 }, (_, index) => typed(`r${index}`, "And?", "n0")), + ]); + expect(refusalLines(yield* call("send_to_user", news2))[0]).toBe(taken); + setup([typed("q1", "Is the build green?"), news("n1", "The deploy is done.")]); + expect(refusalLines(yield* call("send_to_user", news2))).toEqual([ + taken, + "To change it, call replace_send_to_user with sub_thread set to its id:", + `"${shortId("The deploy is done.")}": The deploy is done.`, + ...unanswered(["q1", "Is the build green?"]), + DONE, + ]); + + // A send not stored yet holds the main list too. + setup([...twoSubThreads(), typed("q3", "Anything else?")]); + expect((yield* call("send_to_user", news2)).isError).not.toBe(true); + expect(refusalLines(yield* call("send_to_user", news2)).slice(0, 2)).toEqual([ + taken, + delivering, + ]); + + // A named call counts a new message still running before it, and not the agent's older news. + setup([ + news("n0", "The deploy is done."), + typed("q4", "Anything else?"), + running(news("s1", "Lint is red.")), + running(news("s2", "CI is red.")), + ]); + expect(refusalLines(yield* call("send_to_user", news2, repliesThread, "s2"))).toEqual([ + taken, + delivering, + ...unanswered(["q4", "Anything else?"]), + DONE, + ]); + expect((yield* call("send_to_user", news2, repliesThread, "s1")).isError).not.toBe(true); + // A running call that the handler refuses takes nothing. + setup([ + typed("q5", "Anything else?"), + running(sent("s3", { text: "Lint is red." })), + running(sent("s4", { text: "On it.", new_sub_thread: true, ack: true })), + running(news("s6", "a".repeat(1001))), + running(news("s7", "")), + running(news("s5", "CI is red.")), + ]); + expect((yield* call("send_to_user", news2, repliesThread, "s5")).isError).not.toBe(true); + + // An ack and an answer go into a sub-thread, and a replace adds no message. + setup([ + typed("q6", "Is the build green?"), + ackOf("k6", "Checking.", "q6"), + answer("a6", "Yes.", "q6"), + ]); + expect((yield* call("send_to_user", news2)).isError).not.toBe(true); + setup([ + news("n7", "The deploy is done."), + typed("q7", "Anything else?"), + { + ...sent("r7", { + sub_thread: shortId("The deploy is done."), + user_message: "", + old_text: "The deploy is done.", + new_text: "The deploy is done and checked.", + }), + toolName: "mcp__t3-code__replace_send_to_user", + }, + ]); + expect((yield* call("send_to_user", news2)).isError).not.toBe(true); + + // A message the user sends to the main list opens it again. + setup([...twoSubThreads(), typed("q8", "Anything else?")]); + expect((yield* call("send_to_user", news2)).isError).not.toBe(true); + }).pipe(Effect.provide(layer)), +); + it.effect("shows an answer to the latest user message of each sub-thread, in any order", () => Effect.gen(function* () { setup(twoSubThreads()); From d7515a189a0d2415dde9f451089273400847ad81 Mon Sep 17 00:00:00 2001 From: Renfei Zhou Date: Sat, 14 Nov 2026 13:35:52 +0000 Subject: [PATCH 2/2] fix(fork): one agent message in a row in the Chat + Replies main list The agent could send any number of new main-list messages in a row, and on desktop the hover Reply button under such a message was cut off. send_to_user now refuses a new main-list message while the agent's last one there has no user message after it, counting sends still running or not yet stored. User replies in sub-threads do not reopen the main list. The tools read history back to the main list's latest message, so a long sub-thread cannot hide it. The list clips each row, so the Reply button now sits inside its row: with a mouse its slot tops up the row's bottom padding to the button's height. Co-Authored-By: Claude Opus 5.5 --- .../fork/displayModeWrap/DisplayModeWrap.ts | 18 ++-- .../src/fork/userMessageTools/SendToUser.ts | 88 +++++++++++++++++-- .../src/components/chat/MessagesTimeline.tsx | 4 +- .../fork/subThreads/SubThreadReplyLine.tsx | 36 ++++++-- .../src/fork/displayModes/displayModes.ts | 5 ++ 5 files changed, 129 insertions(+), 22 deletions(-) diff --git a/apps/server/src/fork/displayModeWrap/DisplayModeWrap.ts b/apps/server/src/fork/displayModeWrap/DisplayModeWrap.ts index 3eb6456b0e840..f51a9c0c9accc 100644 --- a/apps/server/src/fork/displayModeWrap/DisplayModeWrap.ts +++ b/apps/server/src/fork/displayModeWrap/DisplayModeWrap.ts @@ -10,6 +10,7 @@ import { import { type DisplayMode, displayEventFromTurnItem, + isMainListEvent, isWebTypedMessage, type MessageOrigin, NORMAL_DISPLAY_MODE, @@ -90,12 +91,14 @@ export interface DisplayModeWrapShape { /** * The user messages and send calls that the screen shows, oldest first, in any state. Reads back * past the user's last typed message and at least `cores` messages (default WRAPPED_CORE_COUNT), - * counted from the item with `ownNativeId` back when given. + * counted from the item with `ownNativeId` back when given. With `toMainListMessage`, also past + * the latest message of the main list of a thread with sub-threads. */ readonly latestShownItems: ( threadId: ThreadId, ownNativeId?: string, cores?: number, + toMainListMessage?: boolean, ) => Effect.Effect< ReadonlyArray, SqlError.SqlError | Schema.SchemaError | ProjectionStore.ProjectionStoreV2Error @@ -198,13 +201,14 @@ export const layer: Layer.Layer< // Reads newest first from before `before`, or from the end, until `coreCount` messages // (not rewrites) and, without `before`, a typed message, counted from `ownNativeId` back when - // given. Visibility gets no items: they only decide interrupt results, which never enter the - // agent's transcript. + // given, and with `toMainListMessage`, a message of the main list. Visibility gets no items: + // they only decide interrupt results, which never enter the agent's transcript. const shownItems = Effect.fn("DisplayModeWrap.shownItems")(function* ( threadId: ThreadId, before: MessageId | null, ownNativeId?: string, coreCount: number = WRAPPED_CORE_COUNT, + toMainListMessage = false, ) { const pageRows = Math.max(PAGE_ROWS, Math.min(coreCount, 64)); let start = { ordinal: Number.MAX_SAFE_INTEGER, itemId: "" }; @@ -256,6 +260,7 @@ export const layer: Layer.Layer< let cores = 0; let passedTyped = before !== null; let passedOwn = ownNativeId === undefined; + let passedMainList = !toMainListMessage; for (const segment of segments) { let { ordinal, itemId } = segment.before; while (true) { @@ -281,7 +286,8 @@ export const layer: Layer.Layer< const event = displayEventFromTurnItem(item); if (event?.type === "user") passedTyped = true; if (event?.type === "user" || event?.type === "sent") cores++; - if (passedTyped && cores >= coreCount) return found.reverse(); + if (event !== null && isMainListEvent(event)) passedMainList = true; + if (passedTyped && passedMainList && cores >= coreCount) return found.reverse(); } const last = rows.at(-1); if (last === undefined || rows.length < pageRows) break; @@ -482,7 +488,9 @@ export const layer: Layer.Layer< const latestShownItems: DisplayModeWrapShape["latestShownItems"] = Effect.fn( "DisplayModeWrap.latestShownItems", - )((threadId, ownNativeId, cores) => shownItems(threadId, null, ownNativeId, cores)); + )((threadId, ownNativeId, cores, toMainListMessage) => + shownItems(threadId, null, ownNativeId, cores, toMainListMessage), + ); return DisplayModeWrap.of({ wrapUserText, diff --git a/apps/server/src/fork/userMessageTools/SendToUser.ts b/apps/server/src/fork/userMessageTools/SendToUser.ts index d1c296e7fc736..640684fdc912a 100644 --- a/apps/server/src/fork/userMessageTools/SendToUser.ts +++ b/apps/server/src/fork/userMessageTools/SendToUser.ts @@ -9,6 +9,7 @@ import { displayEventFromTurnItem, foldCores, freshFirstPointer, + isMainListEvent, messagePointer, recapLine, recapText, @@ -353,6 +354,63 @@ function answerCalls( ); } +/** Whether a send still running would be shown as a new message: the handler would accept it. */ +function takenAsNewMessage(item: OrchestrationV2TurnItem) { + const text = inputField(item, "text"); + return ( + inputField(item, "new_sub_thread") === true && + inputField(item, "ack") !== true && + messagePointer(inputField(item, "reply_to")) === undefined && + typeof text === "string" && + text !== "" && + jsonEscapedBytes(text) <= SEND_TO_USER_MAX_BYTES + ); +} + +/** + * The agent's messages in the main list since the user's latest message there: delivered ones, and + * with `named`, sends still running that the handler would take as a new message. + */ +function mainListSends(items: ReadonlyArray, named: boolean) { + const at = items.findLastIndex((item) => { + const event = displayEventFromTurnItem(item); + return event?.type === "user" && isMainListEvent(event); + }); + return { + anchor: items[at]?.id, + sends: replyCallsAfter(items, at, named, (item) => { + const event = displayEventFromTurnItem(item); + return event === null + ? takenAsNewMessage(item) + : event.type === "sent" && isMainListEvent(event); + }), + }; +} + +/** After a refused new message: how to change the agent's message in the main list, and what to answer. */ +function mainListTakenLines(cores: ReadonlyArray): ReadonlyArray { + const since = cores + .slice(cores.findLastIndex((core) => core.from === "user" && !core.reply) + 1) + .filter((core) => core.from === "agent" && !core.reply); + const open = since.filter( + (core) => !cores.some((reply) => reply.from === "user" && reply.first === core.id), + ); + return [ + ...(since.length === 0 + ? [ + "That message is still being delivered. To change it once it shows, call replace_send_to_user with sub_thread set to the id that its send returned.", + ] + : open.length === 0 + ? ["The user has replied under it, so it stays as it is."] + : [ + "To change it, call replace_send_to_user with sub_thread set to its id:", + ...open.map(listedLine), + ]), + ...unansweredLines(cores.slice(-SUB_THREAD_WINDOW_CORES)), + DONE, + ]; +} + const orchestrationError = (error: { readonly message: string }) => new OrchestratorMcpFailure({ code: "orchestration_error", message: error.message }); @@ -400,12 +458,15 @@ export const layerHandlers = McpToolAccess.toLayer( const holdKey = (threadId: ThreadId, userMessage: string | undefined) => `${threadId}\n${userMessage ?? ""}`; - const readThreadCores = Effect.fn("SendToUser.readThreadCores")(function* (threadId: ThreadId) { + const readThreadCores = Effect.fn("SendToUser.readThreadCores")(function* ( + threadId: ThreadId, + toMainListMessage: boolean, + ) { const client = yield* Effect.serviceOption(McpSchema.McpServerClient); const named = Option.getOrUndefined(client)?.requestMetadata?.["claudecode/toolUseId"]; const toolUseId = typeof named === "string" ? named : undefined; const shown = yield* (yield* DisplayModeWrap) - .latestShownItems(threadId, toolUseId, SUB_THREAD_WINDOW_CORES) + .latestShownItems(threadId, toolUseId, SUB_THREAD_WINDOW_CORES, toMainListMessage) .pipe(Effect.mapError(orchestrationError)); const items = itemsBefore(shown, toolUseId); // `cores` is what the agent saw when it made the call. `coresNow` has what the user typed since. @@ -433,8 +494,11 @@ export const layerHandlers = McpToolAccess.toLayer( readonly ack?: boolean | null | undefined; }, ) { - const { items, cores, named } = yield* readThreadCores(threadId); const replyTo = messagePointer(input.reply_to); + const { items, cores, named } = yield* readThreadCores( + threadId, + replyTo === undefined && input.new_sub_thread === true && input.ack !== true, + ); if ((replyTo !== undefined) === (input.new_sub_thread === true)) { return yield* subThreadRefusal( "invalid_request", @@ -456,8 +520,22 @@ export const layerHandlers = McpToolAccess.toLayer( ); } if (replyTo === undefined) { - // Two first messages sent together get two ids: the first id is not in the history yet. const now = yield* Clock.currentTimeMillis; + const main = mainListSends(items, named); + const mainKey = holdKey(threadId, `main\n${main.anchor ?? ""}`); + if (main.sends.length > 0 || holds.held(mainKey, named, now) !== "open") { + return yield* subThreadRefusal( + "invalid_request", + [ + "you already sent a new message to the main list, and the user has not written there since. You may not send two in a row there.", + ], + [], + mainListTakenLines(cores), + ); + } + holds.record(mainKey, named, "answer", "answered", now); + // A user message to the main list between two first messages can come before the first is + // in the history, so the ids given for a while count as taken. for (const [id, until] of givenIds) if (until <= now) givenIds.delete(id); const id = freshFirstPointer( new Set([ @@ -542,7 +620,7 @@ export const layerHandlers = McpToolAccess.toLayer( readonly new_text: string; }, ) { - const { items, cores, coresNow, named } = yield* readThreadCores(threadId); + const { items, cores, coresNow, named } = yield* readThreadCores(threadId, false); const replyTo = messagePointer(input.reply_to); const subThread = messagePointer(input.sub_thread); if ((replyTo === undefined) === (subThread === undefined)) { diff --git a/apps/web/src/components/chat/MessagesTimeline.tsx b/apps/web/src/components/chat/MessagesTimeline.tsx index 0b1c6d7723fa9..c75b0ac5637c9 100644 --- a/apps/web/src/components/chat/MessagesTimeline.tsx +++ b/apps/web/src/components/chat/MessagesTimeline.tsx @@ -1684,9 +1684,7 @@ const TimelineRowContent = memo(function TimelineRowContent({ row }: { row: Time ) : null} {row.kind === "assistant-meta" ? : null} - {row.kind === "message" ? ( - - ) : null} + {row.kind === "message" ? : null} {row.kind === "proposed-plan" ? : null} {row.kind === "html-render" ? : null} {row.kind === "viewed-images" ? : null} diff --git a/apps/web/src/fork/subThreads/SubThreadReplyLine.tsx b/apps/web/src/fork/subThreads/SubThreadReplyLine.tsx index a6c29252c63b1..8e2a20c48ef4c 100644 --- a/apps/web/src/fork/subThreads/SubThreadReplyLine.tsx +++ b/apps/web/src/fork/subThreads/SubThreadReplyLine.tsx @@ -2,7 +2,7 @@ import type { ScopedThreadRef } from "@t3tools/contracts"; import { ReplyIcon } from "lucide-react"; import { use } from "react"; -import { TimelineRowCtx } from "../../components/chat/timelineRowContext"; +import { type TimelineRow, TimelineRowCtx } from "../../components/chat/timelineRowContext"; import { RelativeTime } from "../../components/RelativeTime"; import { cn } from "../../lib/utils"; import { useThreadDisplayMode } from "../displayMode/useThreadDisplayMode"; @@ -91,26 +91,44 @@ function ReplyLine(props: { ); } +/** + * The Reply slot's height with a mouse: the button's 16px less the row's bottom padding, so the + * button stays inside the row, which the list clips. Mirrors TimelineRowContent's message padding: + * pb-1 when grouped with the next row, pb-2 for an agent message with no meta, else pb-4. + */ +const fineSlotHeight = (row: Extract) => + row.groupedWithNext + ? "h-3" + : row.message.role === "assistant" && !row.showAssistantMeta + ? "h-2" + : "h-0"; + /** * At the end of a first message's row that has no reply yet: a Reply button. With a mouse it shows - * while the pointer is on the row and takes no height, so a row keeps its size. On a touch screen - * it always shows and has its own height, so it stays clear of the next row. + * while the pointer is on the row, in the row's bottom padding and a slot that tops it up. On a touch + * screen it always shows and has its own height, so it stays clear of the next row. */ export function SubThreadReplyButton(props: { - readonly rowId: string; - readonly fromUser: boolean; + readonly row: Extract; }) { - const subThread = useRowSubThread(props.rowId); + const subThread = useRowSubThread(props.row.id); if (subThread === null || subThread.summary.replies > 0) return null; const { threadRef, summary } = subThread; + const fromUser = props.row.message.role === "user"; return ( -
+