From ba824103c6e8af7488888b34c85f4150fe2c81ea Mon Sep 17 00:00:00 2001 From: Amp Date: Tue, 3 Nov 2026 10:49:22 +0000 Subject: [PATCH 1/2] fix(acp): keep v1 assistant images and standard tool images Preserve bounded raster content through the ACP parser and reuse chat attachment storage for v1 assistant images. Keep segmented image replies visible in web and mobile without changing text-only folds or v2 authoritative assistant updates. Written with Amp High in an Orb; the writing model name was not recorded. Amp-Thread-ID: T-01a1a0b3-0b97-77d4-9fbf-85ca74a651ca Co-authored-by: 5omeOtherGuy <268151429+5omeOtherGuy@users.noreply.github.com> --- apps/mobile/src/lib/threadActivity.test.ts | 54 ++++- apps/mobile/src/lib/threadActivity.ts | 25 +- apps/server/src/assets/ChatAttachments.ts | 79 ++++++ .../Adapters/AcpAdapterV2.test.ts | 226 +++++++++++++++++- apps/server/src/provider/ProviderHostLive.ts | 10 + apps/server/src/ws.ts | 81 +------ .../chat/MessagesTimeline.logic.test.ts | 83 ++++++- .../components/chat/MessagesTimeline.logic.ts | 17 +- .../components/chat/TimelineAssistantRows.tsx | 30 ++- packages/provider-acp/src/server/adapter.ts | 100 +++++++- .../provider-acp/src/server/runtimeModel.ts | 41 +++- .../provider-core/src/server/ProviderHost.ts | 5 + 12 files changed, 652 insertions(+), 99 deletions(-) create mode 100644 apps/server/src/assets/ChatAttachments.ts diff --git a/apps/mobile/src/lib/threadActivity.test.ts b/apps/mobile/src/lib/threadActivity.test.ts index 583cbe7098e15..6d0523814bf27 100644 --- a/apps/mobile/src/lib/threadActivity.test.ts +++ b/apps/mobile/src/lib/threadActivity.test.ts @@ -1,4 +1,5 @@ import { + ChatAttachmentId, MessageId, CheckpointId, CheckpointRef, @@ -810,12 +811,24 @@ describe("buildThreadFeed", () => { ); }); - it("folds progress messages while keeping the final assistant answer visible", () => { + it.each([false, true])("folds progress before a reply with images: %s", (hasImages) => { + const attachments = hasImages + ? [ + { + type: "image" as const, + id: ChatAttachmentId.make("reply-image"), + name: "Reply", + mimeType: "image/png", + sizeBytes: 401, + }, + ] + : []; const opening = { ...assistantMessage("2026-06-20T00:00:01.500Z"), id: TurnItemId.make("item-opening"), messageId: MessageId.make("message-opening"), text: "I will check the deployment configuration.", + attachments, }; const middle = { ...assistantMessage("2026-06-20T00:00:02.500Z"), @@ -823,12 +836,20 @@ describe("buildThreadFeed", () => { messageId: MessageId.make("message-middle"), text: "The configuration is valid; checking the build next.", }; + const image = { + ...assistantMessage("2026-06-20T00:00:02.750Z"), + id: TurnItemId.make("item-image"), + messageId: MessageId.make("message-image"), + text: hasImages ? "" : "Build is finishing.", + attachments, + }; const feed = buildThreadFeed([ projected(userMessage(), 0), projected(opening, 1), projected(command(), 2), projected(middle, 3), - projected(assistantMessage(), 4), + projected(image, 4), + projected(assistantMessage(), 5), ]); const latestRun = { runId, @@ -841,6 +862,7 @@ describe("buildThreadFeed", () => { expect(collapsed.map((entry) => entry.id)).toEqual([ "message-user", "run-fold:run-1", + ...(hasImages ? ["message-middle", "message-image"] : []), "message-assistant", ]); expect(collapsed[1]).toMatchObject({ @@ -857,6 +879,7 @@ describe("buildThreadFeed", () => { "activity-group", "message", "message", + "message", ]); const anchor = { ...command(), @@ -884,8 +907,35 @@ describe("buildThreadFeed", () => { "message-user", "message-opening", "message-middle", + "message-image", "message-assistant", ]); + + const steered = deriveThreadFeedPresentation( + buildThreadFeed([ + projected(userMessage(), 0), + projected(opening, 1), + projected( + { + ...userMessage("2026-06-20T00:00:02.000Z"), + id: TurnItemId.make("item-steer"), + messageId: MessageId.make("message-steer"), + inputIntent: "steer", + }, + 2, + ), + projected(middle, 3), + projected(image, 4), + projected(assistantMessage(), 5), + ]), + latestRun, + new Set(), + ); + expect( + steered.flatMap((entry) => + entry.type === "message" && entry.message.role === "assistant" ? [entry.id] : [], + ), + ).toEqual([...(hasImages ? ["message-middle", "message-image"] : []), "message-assistant"]); }); // regression: #21393 show plans and fold settled progress notes diff --git a/apps/mobile/src/lib/threadActivity.ts b/apps/mobile/src/lib/threadActivity.ts index 91681731aab16..545034781f678 100644 --- a/apps/mobile/src/lib/threadActivity.ts +++ b/apps/mobile/src/lib/threadActivity.ts @@ -1084,7 +1084,7 @@ function deriveThreadFeedRunFolds( } const groupsByRunId = new Map< RunId, - { entries: ThreadFeedEntry[]; startBoundary: string | null } + { entries: ThreadFeedEntry[]; startBoundary: string | null; responseBoundaryIndex: number } >(); let runlessKey: RunId | null = null; let responseKey: RunId | null = null; @@ -1094,6 +1094,8 @@ function deriveThreadFeedRunFolds( pendingUserBoundary = entry.message.createdAt; runlessKey = entry.message.runId == null ? RunId.make(`runless:${entry.id}`) : null; responseKey = entry.message.runId ?? runlessKey; + const group = responseKey === null ? undefined : groupsByRunId.get(responseKey); + if (group) group.responseBoundaryIndex = group.entries.length; continue; } const ownRunId = @@ -1112,7 +1114,7 @@ function deriveThreadFeedRunFolds( : ownRunId; let group = groupsByRunId.get(runId); if (!group) { - group = { entries: [], startBoundary: pendingUserBoundary }; + group = { entries: [], startBoundary: pendingUserBoundary, responseBoundaryIndex: 0 }; pendingUserBoundary = null; groupsByRunId.set(runId, group); } @@ -1148,11 +1150,26 @@ function deriveThreadFeedRunFolds( continue; } const terminalAssistantId = terminalAssistantMessageIdByRun.get(runId); + const terminalEntryIndex = terminalAssistantId + ? group.entries.findIndex((entry) => entry.id === terminalAssistantId) + : group.entries.length; + let responseStartIndex = terminalEntryIndex; + let responseHasImages = false; + // Inline images split one reply into consecutive text and attachment messages. + for (let index = terminalEntryIndex; index >= group.responseBoundaryIndex; index--) { + const entry = group.entries[index]; + if (entry?.type !== "message" || entry.message.role !== "assistant") break; + responseStartIndex = index; + if (entry.message.attachments.some((attachment) => attachment.type === "image")) { + responseHasImages = true; + } + } + if (!responseHasImages) responseStartIndex = terminalEntryIndex; const hiddenEntryIds = new Set( group.entries .filter( - (entry) => - entry.id !== terminalAssistantId && + (entry, index) => + (index < responseStartIndex || index > terminalEntryIndex) && entry.type !== "html-render" && entry.type !== "mcp-app" && !(entry.type === "activity-group" && isContextCompactionActivityGroup(entry)) && diff --git a/apps/server/src/assets/ChatAttachments.ts b/apps/server/src/assets/ChatAttachments.ts new file mode 100644 index 0000000000000..b619542b9a57f --- /dev/null +++ b/apps/server/src/assets/ChatAttachments.ts @@ -0,0 +1,79 @@ +import { + ChatAttachmentId, + PersistChatAttachmentsError, + type PersistChatAttachmentsInput, +} from "@t3tools/contracts"; +import * as Base64 from "effect/encoding/Base64"; +import * as Effect from "effect/Effect"; +import * as FileSystem from "effect/FileSystem"; +import * as Path from "effect/Path"; + +import { + attachmentRelativePath, + chatAttachmentStableKey, + createDeterministicAttachmentId, +} from "../attachmentStore.ts"; +import * as ServerConfig from "../config.ts"; +import { parseBase64DataUrl } from "../imageMime.ts"; + +export const persistChatAttachments = Effect.fn("ws.assets.persistChatAttachments")(function* ( + input: PersistChatAttachmentsInput, +) { + const config = yield* ServerConfig.ServerConfig; + const fileSystem = yield* FileSystem.FileSystem; + const path = yield* Path.Path; + return yield* Effect.forEach( + input.attachments.map((attachment, index) => ({ attachment, index })), + Effect.fn("ws.assets.persistChatAttachment")(function* ({ attachment, index }) { + const parsed = parseBase64DataUrl(attachment.dataUrl); + if (parsed === null || parsed.mimeType !== attachment.mimeType.toLowerCase()) { + return yield* new PersistChatAttachmentsError({ + message: `Attachment ${attachment.name} has an invalid image payload.`, + }); + } + const bytes = yield* Effect.fromResult(Base64.decode(parsed.base64)).pipe( + Effect.mapError( + (cause) => + new PersistChatAttachmentsError({ + message: `Attachment ${attachment.name} is not valid base64.`, + cause, + }), + ), + ); + if (bytes.byteLength !== attachment.sizeBytes) { + return yield* new PersistChatAttachmentsError({ + message: `Attachment ${attachment.name} size does not match its payload.`, + }); + } + const rawId = createDeterministicAttachmentId( + input.threadId, + chatAttachmentStableKey(input.messageId, index, bytes), + ); + if (rawId === null) { + return yield* new PersistChatAttachmentsError({ + message: "Could not allocate an attachment identifier.", + }); + } + const persisted = { + type: "image" as const, + id: ChatAttachmentId.make(rawId), + name: attachment.name, + mimeType: attachment.mimeType, + sizeBytes: attachment.sizeBytes, + }; + yield* fileSystem + .writeFile(path.join(config.attachmentsDir, attachmentRelativePath(persisted)!), bytes) + .pipe( + Effect.mapError( + (cause) => + new PersistChatAttachmentsError({ + message: `Could not persist attachment ${attachment.name}.`, + cause, + }), + ), + ); + return persisted; + }), + { concurrency: 2 }, + ); +}); diff --git a/apps/server/src/orchestration-v2/Adapters/AcpAdapterV2.test.ts b/apps/server/src/orchestration-v2/Adapters/AcpAdapterV2.test.ts index a58a83bffa349..d55555a856712 100644 --- a/apps/server/src/orchestration-v2/Adapters/AcpAdapterV2.test.ts +++ b/apps/server/src/orchestration-v2/Adapters/AcpAdapterV2.test.ts @@ -74,6 +74,9 @@ import { } from "@t3tools/provider-grok/testing"; import * as IdAllocator from "@t3tools/provider-core/server/IdAllocator"; import * as ProviderAdapter from "@t3tools/provider-core/server/ProviderAdapter"; +import * as ProviderHost from "@t3tools/provider-core/server/ProviderHost"; +import * as ServerConfig from "../../config.ts"; +import { persistChatAttachments } from "../../assets/ChatAttachments.ts"; import { makeTurnInput } from "./AcpAdapterV2.testkit.ts"; import type * as ProviderContinuationRequests from "@t3tools/provider-core/server/ProviderContinuationRequests"; import { @@ -2385,6 +2388,175 @@ describe("AcpAdapterV2", () => { }).pipe(Effect.provide(layerTest), Effect.scoped), ); + it.effect.each([ + { protocolVersion: 1, messageId: undefined }, + { protocolVersion: 1, messageId: "source-message" }, + { protocolVersion: 2, messageId: "source-message" }, + ] as const)( + "handles inline images under protocol $protocolVersion with messageId $messageId", + ({ protocolVersion, messageId }) => + Effect.gen(function* () { + const path = yield* Path.Path; + const config = yield* ServerConfig.ServerConfig; + const fileSystem = yield* FileSystem.FileSystem; + const host = yield* ProviderHost.ProviderHost; + const instanceId = ProviderInstanceId.make("acp-inline-images"); + const threadId = ThreadId.make("thread-acp-inline-images"); + type RuntimeService = AcpSessionRuntime.AcpSessionRuntime["Service"]; + let handler: Parameters[0] | undefined; + const png = + "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mP8/x8AAwMCAO+aY9sAAAAASUVORK5CYII="; + const image = { type: "image", mimeType: "image/png", data: png } as const; + const updates: Array = [ + ...( + [ + { type: "text", text: "before" }, + image, + { type: "text", text: "between" }, + { ...image, data: "", uri: `data:image/png;base64,${png}` }, + { type: "text", text: "after" }, + ] satisfies Array + ).map((content) => ({ + sessionUpdate: "agent_message_chunk" as const, + ...(messageId === undefined ? {} : { messageId }), + content, + })), + { + sessionUpdate: "tool_call", + toolCallId: "image-tool", + title: "Image tool", + kind: "other", + status: "inProgress", + }, + { + sessionUpdate: "tool_call_update", + toolCallId: "image-tool", + status: "completed", + content: [ + { type: "content", content: { type: "text", text: "tool caption" } }, + { type: "content", content: image }, + ], + }, + ]; + if (protocolVersion === 2) { + updates.push({ + sessionUpdate: "agent_message", + messageId: "source-message", + content: [{ type: "text", text: "authoritative mixed reply" }], + }); + } + const adapter = yield* makeAcpAdapterV2({ + instanceId, + selfInvocation: yield* resolveSelfInvocation(), + flavor: { + driver: ACP_TEST_DRIVER, + capabilities: AcpProviderCapabilitiesV2, + makeRuntime: makeMockRuntime({ + childProcessSpawner: yield* ChildProcessSpawner.ChildProcessSpawner, + mockAgentPath: yield* path.fromFileUrl( + new URL("../../../scripts/acp-mock-agent.ts", import.meta.url), + ), + wrapRuntime: (runtime) => ({ + ...runtime, + start: () => + runtime.start().pipe( + Effect.map((result) => ({ + ...result, + initializeResult: { ...result.initializeResult, protocolVersion }, + })), + ), + handleSessionUpdate: (next) => + Effect.sync(() => { + handler = next; + }).pipe(Effect.andThen(runtime.handleSessionUpdate(next))), + prompt: () => + Effect.gen(function* () { + assert.isDefined(handler); + for (const update of updates) + yield* handler!({ sessionId: "mock-session-1", update }); + return { stopReason: "end_turn" as const }; + }), + }), + }), + }, + }).pipe( + Effect.provideService(ProviderHost.ProviderHost, { + ...host, + persistChatAttachments: (input) => + persistChatAttachments(input).pipe( + Effect.provideService(ServerConfig.ServerConfig, config), + Effect.provideService(FileSystem.FileSystem, fileSystem), + Effect.provideService(Path.Path, path), + ), + }), + ); + const modelSelection = { instanceId, model: "default" }; + const runtimePolicy = ProviderAdapter.ProviderAdapterV2RuntimePolicy.make({ + runtimeMode: "full-access", + interactionMode: "default", + cwd: process.cwd(), + }); + const runtime = yield* adapter.openSession({ + threadId, + providerSessionId: ProviderSessionId.make("session-inline-images"), + modelSelection, + runtimePolicy, + }); + const providerThread = yield* runtime.ensureThread({ + threadId, + modelSelection, + runtimePolicy, + }); + yield* runtime.startTurn( + makeTurnInput({ + threadId, + providerThread, + instanceId, + runtimePolicy, + now: yield* DateTime.now, + }), + ); + const events = yield* runtime.events.pipe( + Stream.takeUntil((event) => event.type === "turn.terminal"), + Stream.runCollect, + ); + const items = new Map(); + for (const event of events) + if (event.type === "turn_item.updated") items.set(event.turnItem.id, event.turnItem); + const answers = [...items.values()].filter((item) => item.type === "assistant_message"); + assert.deepEqual( + answers.map((item) => item.text), + protocolVersion === 1 + ? ["before", "", "between", "", "after"] + : ["authoritative mixed reply"], + ); + const attachments = answers.flatMap((item) => item.attachments ?? []); + assert.lengthOf(attachments, protocolVersion === 1 ? 2 : 0); + if (protocolVersion === 1) assert.notEqual(attachments[0]?.id, attachments[1]?.id); + for (const attachment of attachments) { + const bytes = yield* fileSystem.readFile( + path.join(config.attachmentsDir, `${attachment.id}.png`), + ); + assert.equal(Buffer.from(bytes).toString("base64"), png); + assert.equal(bytes.byteLength, attachment.sizeBytes); + } + const tool = [...items.values()].find((item) => item.type === "dynamic_tool"); + assert.isTrue(tool?.type === "dynamic_tool"); + if (tool?.type === "dynamic_tool") + assert.deepEqual(tool.output, [{ type: "text", text: "tool caption" }, image]); + }).pipe( + Effect.provide( + Layer.mergeAll( + layerTest, + ServerConfig.layerTest(process.cwd(), { prefix: "t3-acp-inline-" }).pipe( + Layer.provide(NodeServices.layer), + ), + ), + ), + Effect.scoped, + ), + ); + it.effect("persists a bounded subset of streamed root and child tool updates", () => Effect.gen(function* () { const childProcessSpawner = yield* ChildProcessSpawner.ChildProcessSpawner; @@ -3273,6 +3445,47 @@ describe("AcpAdapterV2", () => { Effect.sync(() => { sessionUpdateHandler = handler; }).pipe(Effect.andThen(runtime.handleSessionUpdate(handler))), + prompt: (input) => + runtime.prompt(input).pipe( + Effect.tap(() => + Effect.gen(function* () { + const updates = [ + { + sessionUpdate: "agent_message_chunk", + messageId: "chunk-then-upsert", + content: { type: "text", text: "partial answer" }, + }, + { + sessionUpdate: "agent_thought_chunk", + content: { type: "text", text: "checking" }, + }, + { + sessionUpdate: "agent_message_chunk", + messageId: "chunk-then-upsert", + content: { type: "text", text: "continued answer" }, + }, + { + sessionUpdate: "agent_message", + messageId: "chunk-then-upsert", + content: [{ type: "text", text: "complete answer" }], + }, + { + sessionUpdate: "tool_call", + toolCallId: "opaque-result", + title: "Opaque result", + kind: "other", + status: "completed", + rawOutput: { + result: { content: [{ type: "text", text: "opaque" }] }, + extra: 1, + }, + }, + ] satisfies Array; + for (const update of updates) + yield* sessionUpdateHandler!({ sessionId: "mock-session-1", update }); + }), + ), + ), }), }), }, @@ -3345,8 +3558,19 @@ describe("AcpAdapterV2", () => { assert.isTrue(items.some((item) => item.type === "user_message" && item.text.length === 0)); assert.equal( items.filter((item) => item.type === "assistant_message").at(-1)?.text, - "authoritative answer", + "complete answer", + ); + const replies = new Map( + items.flatMap((item) => (item.type === "assistant_message" ? [[item.id, item.text]] : [])), + ); + assert.deepEqual([...replies.values()], ["authoritative answer", "complete answer"]); + const opaqueResult = items.find( + (item) => item.type === "dynamic_tool" && item.toolName === "Opaque result", ); + assert.deepEqual(opaqueResult?.type === "dynamic_tool" ? opaqueResult.output : undefined, { + result: { content: [{ type: "text", text: "opaque" }] }, + extra: 1, + }); assert.isTrue( items.some((item) => item.type === "reasoning" && item.text === "final thought"), ); diff --git a/apps/server/src/provider/ProviderHostLive.ts b/apps/server/src/provider/ProviderHostLive.ts index 6398210599b61..dfca52ec4bfc5 100644 --- a/apps/server/src/provider/ProviderHostLive.ts +++ b/apps/server/src/provider/ProviderHostLive.ts @@ -4,8 +4,10 @@ import * as Digest from "effect/crypto/Digest"; import * as Effect from "effect/Effect"; import * as FileSystem from "effect/FileSystem"; import * as Layer from "effect/Layer"; +import * as Path from "effect/Path"; import { resolveAttachmentPath } from "../attachmentStore.ts"; +import { persistChatAttachments } from "../assets/ChatAttachments.ts"; import * as ServerSecretStore from "../auth/ServerSecretStore.ts"; import * as BackgroundPolicy from "../background/BackgroundPolicy.ts"; import * as ServerConfig from "../config.ts"; @@ -22,6 +24,7 @@ export const layer = Layer.effect( const secrets = yield* ServerSecretStore.ServerSecretStore; const digest = yield* Digest.Digest; const fileSystem = yield* FileSystem.FileSystem; + const path = yield* Path.Path; return ProviderHost.ProviderHost.of({ paths: { cwd: config.cwd, @@ -39,6 +42,13 @@ export const layer = Layer.effect( shouldRunBackgroundWork: backgroundPolicy.shouldRunScopeWork, resolveAttachmentPath: (attachment) => resolveAttachmentPath({ attachmentsDir: config.attachmentsDir, attachment }), + persistChatAttachments: Effect.fn("ProviderHost.persistChatAttachments")((input) => + persistChatAttachments(input).pipe( + Effect.provideService(ServerConfig.ServerConfig, config), + Effect.provideService(FileSystem.FileSystem, fileSystem), + Effect.provideService(Path.Path, path), + ), + ), // DocumentText owns the path-resolution span. resolveExtractedTextPath: Effect.fnUntraced( function* (path) { diff --git a/apps/server/src/ws.ts b/apps/server/src/ws.ts index 492764c6ae5c8..c9dd64b648ef9 100644 --- a/apps/server/src/ws.ts +++ b/apps/server/src/ws.ts @@ -7,7 +7,6 @@ import * as PeerLinks from "./peer/PeerLinks.ts"; import * as DateTime from "effect/DateTime"; import * as Duration from "effect/Duration"; -import * as Base64 from "effect/encoding/Base64"; import * as Effect from "effect/Effect"; import * as Fiber from "effect/Fiber"; import * as FileSystem from "effect/FileSystem"; @@ -42,7 +41,6 @@ import { type OrchestrationV2Command, type GitActionProgressEvent, type GitManagerServiceError, - type MessageId, type AcpRegistryImportSessionInput, type AcpRegistryDeleteSessionInput, type AcpRegistryDisableProviderInput, @@ -87,8 +85,6 @@ import { AssetWorkspaceContextNotFoundError, AssetWorkspaceContextResolutionError, SOURCE_CONTROL_MEDIA_ASSET, - ChatAttachmentId, - PersistChatAttachmentsError, RpcClientId, EnvironmentAuthorizationError, type ProjectId, @@ -192,12 +188,7 @@ import * as DeviceService from "./device/DeviceService.ts"; import { remoteSshDeviceHosts } from "./device/localSshDeviceHost.ts"; import * as PreviewManager from "./preview/Manager.ts"; import { issueAssetUrl } from "./assets/AssetAccess.ts"; -import { - attachmentRelativePath, - chatAttachmentStableKey, - createDeterministicAttachmentId, -} from "./attachmentStore.ts"; -import { parseBase64DataUrl } from "./imageMime.ts"; +import { persistChatAttachments } from "./assets/ChatAttachments.ts"; import { deletePendingAttachment, issueAttachmentUploadUrl } from "./assets/AttachmentUpload.ts"; import * as PortScanner from "./preview/PortScanner.ts"; import * as WorkspaceEntries from "./workspace/WorkspaceEntries.ts"; @@ -428,76 +419,6 @@ function unexpectedCompatibilityError(error: never): never { throw new Error(`Unhandled compatibility error: ${String(error)}`); } -const persistChatAttachments = Effect.fn("ws.assets.persistChatAttachments")(function* (input: { - readonly threadId: ThreadId; - readonly messageId: MessageId; - readonly attachments: ReadonlyArray<{ - readonly type: "image"; - readonly name: string; - readonly mimeType: string; - readonly sizeBytes: number; - readonly dataUrl: string; - }>; -}) { - const config = yield* ServerConfig.ServerConfig; - const fileSystem = yield* FileSystem.FileSystem; - const path = yield* Path.Path; - return yield* Effect.forEach( - input.attachments.map((attachment, index) => ({ attachment, index })), - Effect.fn("ws.assets.persistChatAttachment")(function* ({ attachment, index }) { - const parsed = parseBase64DataUrl(attachment.dataUrl); - if (parsed === null || parsed.mimeType !== attachment.mimeType.toLowerCase()) { - return yield* new PersistChatAttachmentsError({ - message: `Attachment ${attachment.name} has an invalid image payload.`, - }); - } - const bytes = yield* Effect.fromResult(Base64.decode(parsed.base64)).pipe( - Effect.mapError( - (cause) => - new PersistChatAttachmentsError({ - message: `Attachment ${attachment.name} is not valid base64.`, - cause, - }), - ), - ); - if (bytes.byteLength !== attachment.sizeBytes) { - return yield* new PersistChatAttachmentsError({ - message: `Attachment ${attachment.name} size does not match its payload.`, - }); - } - const rawId = createDeterministicAttachmentId( - input.threadId, - chatAttachmentStableKey(input.messageId, index, bytes), - ); - if (rawId === null) { - return yield* new PersistChatAttachmentsError({ - message: "Could not allocate an attachment identifier.", - }); - } - const persisted = { - type: "image" as const, - id: ChatAttachmentId.make(rawId), - name: attachment.name, - mimeType: attachment.mimeType, - sizeBytes: attachment.sizeBytes, - }; - yield* fileSystem - .writeFile(path.join(config.attachmentsDir, attachmentRelativePath(persisted)!), bytes) - .pipe( - Effect.mapError( - (cause) => - new PersistChatAttachmentsError({ - message: `Could not persist attachment ${attachment.name}.`, - cause, - }), - ), - ); - return persisted; - }), - { concurrency: 2 }, - ); -}); - function projectEntriesFailureContext(error: WorkspaceEntries.WorkspaceEntriesError): { readonly failure: ProjectEntriesFailure; readonly normalizedCwd?: string; diff --git a/apps/web/src/components/chat/MessagesTimeline.logic.test.ts b/apps/web/src/components/chat/MessagesTimeline.logic.test.ts index 4c7eba620b5d1..3d8fa92a715c5 100644 --- a/apps/web/src/components/chat/MessagesTimeline.logic.test.ts +++ b/apps/web/src/components/chat/MessagesTimeline.logic.test.ts @@ -1,5 +1,6 @@ import { ThreadId, type WorktreeSetupSnapshot } from "@t3tools/contracts"; import { + ChatAttachmentId, CheckpointRef, NodeId, ProviderDriverKind, @@ -904,7 +905,7 @@ describe("deriveMessagesTimelineRows", () => { expect(assistantRow?.assistantTurnDiffSummary).toBe(assistantTurnDiffSummary); }); - it("folds the first assistant message and settled work before the terminal response", () => { + it.each([false, true])("folds work before a reply with images: %s", (hasImages) => { const timelineEntries = [ { id: "user-entry", @@ -929,6 +930,17 @@ describe("deriveMessagesTimelineRows", () => { role: "assistant" as const, text: "Synthetic deployment checklist\n1. Confirm the deployment is ready.", runId: "turn-1" as never, + attachments: hasImages + ? [ + { + type: "image" as const, + id: ChatAttachmentId.make("progress-image"), + name: "Progress", + mimeType: "image/png", + sizeBytes: 401, + }, + ] + : [], createdAt: "2026-01-01T00:00:05Z", updatedAt: "2026-01-01T00:00:06Z", streaming: false, @@ -946,6 +958,32 @@ describe("deriveMessagesTimelineRows", () => { tone: "tool" as const, }, }, + ...["before", "", "between", ""].map((text, index) => ({ + id: `assistant-part-${index}`, + kind: "message" as const, + createdAt: "2026-01-01T00:00:10Z", + message: { + id: MessageId.make(`assistant-part-${index}`), + role: "assistant" as const, + text: text || (hasImages ? "" : "Image follows"), + runId: RunId.make("turn-1"), + attachments: + hasImages && text.length === 0 + ? [ + { + type: "image" as const, + id: ChatAttachmentId.make(`reply-image-${index}`), + name: "Reply", + mimeType: "image/png", + sizeBytes: 401, + }, + ] + : [], + createdAt: "2026-01-01T00:00:10Z", + updatedAt: "2026-01-01T00:00:10Z", + streaming: false, + }, + })), { id: "assistant-final-entry", kind: "message" as const, @@ -980,6 +1018,9 @@ describe("deriveMessagesTimelineRows", () => { expect(collapsedRows.map((row) => row.id)).toEqual([ "user-entry", "turn-fold:turn-1", + ...(hasImages + ? ["assistant-part-0", "assistant-part-1", "assistant-part-2", "assistant-part-3"] + : []), "assistant-final-entry", ]); @@ -997,11 +1038,51 @@ describe("deriveMessagesTimelineRows", () => { "turn-fold:turn-1", "assistant-first-entry", "work-entry-1", + "assistant-part-0", + "assistant-part-1", + "assistant-part-2", + "assistant-part-3", "assistant-final-entry", ]); expect( expandedRows.find((row) => row.kind === "turn-fold" && row.expanded === true), ).toBeDefined(); + + const steeredRows = deriveMessagesTimelineRows({ + timelineEntries: timelineEntries.map((entry) => + entry.kind === "work" + ? { + kind: "message" as const, + id: "steer-entry", + createdAt: entry.createdAt, + message: { + id: MessageId.make("steer"), + role: "user" as const, + text: "Continue with the final reply", + inputIntent: "steer" as const, + runId: RunId.make("turn-1"), + createdAt: entry.createdAt, + updatedAt: entry.createdAt, + streaming: false, + }, + } + : entry, + ), + isWorking: false, + activeTurnStartedAt: null, + turnDiffSummaries: [], + supportsConversationRollback: false, + }); + expect( + steeredRows.flatMap((row) => + row.kind === "message" && row.message.role === "assistant" ? [row.id] : [], + ), + ).toEqual([ + ...(hasImages + ? ["assistant-part-0", "assistant-part-1", "assistant-part-2", "assistant-part-3"] + : []), + "assistant-final-entry", + ]); }); it.each([1, 2, 3])("folds %i completed activities after the terminal response", (count) => { diff --git a/apps/web/src/components/chat/MessagesTimeline.logic.ts b/apps/web/src/components/chat/MessagesTimeline.logic.ts index 2fe811db4d18f..f566098abe666 100644 --- a/apps/web/src/components/chat/MessagesTimeline.logic.ts +++ b/apps/web/src/components/chat/MessagesTimeline.logic.ts @@ -1120,8 +1120,23 @@ function deriveTurnFolds(input: { const terminalEntryIndex = group.terminalEntry ? group.entries.findIndex((entry) => entry.id === group.terminalEntry?.id) : group.entries.length; + const responseBoundaryIndex = + group.steerSegments.findLast(({ startIndex }) => startIndex <= terminalEntryIndex) + ?.startIndex ?? 0; + let responseStartIndex = terminalEntryIndex; + let responseHasImages = false; + // Inline images split one reply into consecutive text and attachment messages. + for (let index = terminalEntryIndex; index >= responseBoundaryIndex; index--) { + const entry = group.entries[index]; + if (entry?.kind !== "message" || entry.message.role !== "assistant") break; + responseStartIndex = index; + if (entry.message.attachments?.some((attachment) => attachment.type === "image")) { + responseHasImages = true; + } + } + if (!responseHasImages) responseStartIndex = terminalEntryIndex; for (const [index, entry] of group.entries.entries()) { - if (entry.id === group.terminalEntry?.id) { + if (index >= responseStartIndex && index <= terminalEntryIndex) { continue; } const isCompaction = timelineEntryIsCompaction(entry); diff --git a/apps/web/src/components/chat/TimelineAssistantRows.tsx b/apps/web/src/components/chat/TimelineAssistantRows.tsx index 458d84b9e982b..87435b2ebb837 100644 --- a/apps/web/src/components/chat/TimelineAssistantRows.tsx +++ b/apps/web/src/components/chat/TimelineAssistantRows.tsx @@ -2,7 +2,7 @@ import { shouldPreserveAssistantLineBreaks } from "@t3tools/shared/markdownPipel import { type RunId } from "@t3tools/contracts"; import { canForkProjectedAssistantItem } from "@t3tools/client-runtime/state/thread-workflows"; import { memo, use, useState } from "react"; -import { type ChatMessage, type TurnDiffSummary } from "../../types"; +import { isImageAttachment, type ChatMessage, type TurnDiffSummary } from "../../types"; import ChatMarkdown from "../ChatMarkdown"; import { GitForkIcon } from "lucide-react"; import { Button } from "../ui/button"; @@ -23,6 +23,10 @@ import { formatChatTimestampTooltip, formatDayAwareTimestamp } from "../../times import { useV2ItemSupport } from "../../state/v2ItemSupport"; import { TimelineRowCtx, TimelineRowActivityCtx, type TimelineRow } from "./timelineRowContext"; import { MESSAGE_HEADING_LEVEL, MessageAuthorHeading } from "./TimelineRowParts"; +import { GuestMessageAttachments } from "./GuestMessageAttachments"; +import { selectMessageImageResources } from "../../session-logic"; +import { useAssetUrls } from "../../assets/assetUrls"; +import { buildExpandedImagePreview } from "./ExpandedImagePreview"; export function AssistantTimelineRow({ row }: { row: Extract }) { const ctx = use(TimelineRowCtx); @@ -32,7 +36,13 @@ export function AssistantTimelineRow({ row }: { row: Extract [resource.attachmentId, urls[index]]), + ); return ( <> @@ -60,6 +70,22 @@ export function AssistantTimelineRow({ row }: { row: Extract +
+ urlsById.get(attachment.id) ?? undefined} + onOpenImage={(attachment) => { + const preview = buildExpandedImagePreview( + (row.message.attachments ?? []).filter(isImageAttachment).map((image) => { + const previewUrl = urlsById.get(image.id); + return previewUrl ? { ...image, previewUrl } : image; + }), + attachment.id, + ); + if (preview) ctx.onImageExpand(preview); + }} + /> +
{ctx.onOpenTurnDiff ? ( { + const record = unknownRecord(entry); + return record?.type === "content" ? record.content : entry; + }) + : content; +} + function acpMcpToolCallOutput(rawOutput: unknown): unknown { const record = unknownRecord(rawOutput); if (record === undefined) return rawOutput; @@ -1296,6 +1306,7 @@ interface ActiveTextSegment { readonly startedAt: DateTime.Utc; sourceMessageId: string | null; text: string; + attachments: ReadonlyArray; // The running node and first text have been emitted. emitted: boolean; // Text changed since the last emission; a delayed flush is scheduled. @@ -1325,6 +1336,7 @@ interface ActiveAcpTurn { readonly user: ActiveTextStream; readonly assistant: ActiveTextStream; readonly reasoning: ActiveTextStream; + readonly assistantImageSegments: Map; providerRetry?: | { readonly failure: OrchestrationV2ProviderFailure; @@ -2653,7 +2665,7 @@ export const makeAcpAdapterV2 = Effect.fn("makeAcpAdapterV2")(function* ( nodeId: messageNodeId, role: kind, text, - attachments: [], + attachments: segment.attachments, streaming: !completed, createdAt: segment.startedAt, updatedAt: now, @@ -2685,7 +2697,7 @@ export const makeAcpAdapterV2 = Effect.fn("makeAcpAdapterV2")(function* ( messageId, inputIntent: "turn_start", text, - attachments: [], + attachments: segment.attachments, }, } : { @@ -2709,6 +2721,7 @@ export const makeAcpAdapterV2 = Effect.fn("makeAcpAdapterV2")(function* ( type: "assistant_message", messageId, text, + attachments: segment.attachments, streaming: !completed, }, }, @@ -2783,8 +2796,9 @@ export const makeAcpAdapterV2 = Effect.fn("makeAcpAdapterV2")(function* ( kind: "user" | "assistant" | "reasoning", text: string, messageId?: string | null, + attachments: ReadonlyArray = [], ) { - if (text.length === 0) return; + if (text.length === 0 && attachments.length === 0) return; for (const other of ["user", "reasoning", "assistant"] as const) { if (other !== kind) yield* closeTextStream(context, other); } @@ -2800,14 +2814,22 @@ export const makeAcpAdapterV2 = Effect.fn("makeAcpAdapterV2")(function* ( } if (stream.current === null) { const now = yield* DateTime.now; + const messageItemId = `${context.nativeTurnId}:${kind}:message:${sourceMessageId}`; + const imageSegment = + kind === "assistant" && sourceMessageId !== null + ? (context.assistantImageSegments.get(sourceMessageId) ?? 0) + : 0; stream.current = { nativeItemId: sourceMessageId === null ? `${context.nativeTurnId}:${kind}:${stream.nextSegment}` - : `${context.nativeTurnId}:${kind}:message:${sourceMessageId}`, + : imageSegment > 0 + ? `${messageItemId}:${imageSegment}` + : messageItemId, startedAt: now, sourceMessageId, text: "", + attachments, emitted: false, dirty: false, }; @@ -2838,6 +2860,7 @@ export const makeAcpAdapterV2 = Effect.fn("makeAcpAdapterV2")(function* ( startedAt: now, sourceMessageId: messageId, text, + attachments: [], emitted: false, dirty: false, }; @@ -2848,6 +2871,66 @@ export const makeAcpAdapterV2 = Effect.fn("makeAcpAdapterV2")(function* ( yield* publishTextSegment(context, kind); }); + const appendAssistantContent = Effect.fnUntraced(function* ( + context: ActiveAcpTurn, + content: EffectAcpSchema.ContentBlock, + messageId?: string | null, + ) { + const image = acpContentBlockImage(content); + // V2 authoritative upserts cannot remove previously published image/text segments. + if ( + image !== undefined && + host.persistChatAttachments !== undefined && + (yield* Ref.get(activeSessionSetup))?.initializeResult.protocolVersion === 1 + ) { + const stored = yield* host + .persistChatAttachments({ + threadId: input.threadId, + messageId: providerMessageId( + `${context.nativeTurnId}:image:${context.assistant.nextSegment}`, + ), + attachments: [ + { + type: "image", + name: "ACP image", + mimeType: image.mimeType, + sizeBytes: image.sizeBytes, + dataUrl: `data:${image.mimeType};base64,${image.data}`, + }, + ], + }) + .pipe(Effect.result); + if (Result.isSuccess(stored)) { + const sourceMessageId = + messageId?.trim() || context.assistant.current?.sourceMessageId; + const imageSegment = sourceMessageId + ? (context.assistantImageSegments.get(sourceMessageId) ?? 0) + 1 + : 0; + yield* closeTextStreams(context); + if (sourceMessageId) + context.assistantImageSegments.set(sourceMessageId, imageSegment); + yield* appendText(context, "assistant", "", sourceMessageId, stored.success); + yield* closeTextStream(context, "assistant"); + if (sourceMessageId) + context.assistantImageSegments.set(sourceMessageId, imageSegment + 1); + return; + } + yield* appendText( + context, + "assistant", + `[Unsupported ACP image: ${stored.failure.message}]`, + messageId, + ); + return; + } + yield* appendText( + context, + "assistant", + acpContentBlockDisplayText(content) ?? "", + messageId, + ); + }); + const emitSubagentAssistant = Effect.fnUntraced(function* ( subagent: ActiveAcpSubagent, text: string, @@ -2976,7 +3059,8 @@ export const makeAcpAdapterV2 = Effect.fn("makeAcpAdapterV2")(function* ( ? (merged.title ?? merged.kind ?? "Tool") : `${mcpIdentity.server}.${mcpIdentity.tool}`, input: merged.data.rawInput ?? null, - output: merged.data.rawOutput ?? merged.data.content ?? null, + output: + merged.data.rawOutput ?? acpToolCallContentOutput(merged.data.content) ?? null, }, }); }); @@ -3931,7 +4015,8 @@ export const makeAcpAdapterV2 = Effect.fn("makeAcpAdapterV2")(function* ( updatedAt: now, } as const; const rawInput = toolCall.data.rawInput; - const rawOutput = toolCall.data.rawOutput ?? toolCall.data.content; + const rawOutput = + toolCall.data.rawOutput ?? acpToolCallContentOutput(toolCall.data.content); const changes = structuredFileChanges(toolCall); const path = changes[0]?.path ?? pathFromToolCall(toolCall); const diffText = @@ -5271,7 +5356,7 @@ export const makeAcpAdapterV2 = Effect.fn("makeAcpAdapterV2")(function* ( ) { context.earlyInjectedReportObserved = true; } - yield* appendText(context, "assistant", text, update.messageId); + yield* appendAssistantContent(context, update.content, update.messageId); } break; } @@ -7953,6 +8038,7 @@ export const makeAcpAdapterV2 = Effect.fn("makeAcpAdapterV2")(function* ( user: { current: null, nextSegment: 0 }, assistant: { current: null, nextSegment: 0 }, reasoning: { current: null, nextSegment: 0 }, + assistantImageSegments: new Map(), contextUsage: rememberedContextUsage ?? turnInput.providerThread.contextUsage ?? null, nativeMetadata: initialNativeMetadata, tools: new Map(), diff --git a/packages/provider-acp/src/server/runtimeModel.ts b/packages/provider-acp/src/server/runtimeModel.ts index bc418eefcb228..528a2b31c3fa5 100644 --- a/packages/provider-acp/src/server/runtimeModel.ts +++ b/packages/provider-acp/src/server/runtimeModel.ts @@ -4,6 +4,8 @@ import * as Effect from "effect/Effect"; import * as Option from "effect/Option"; import * as Ref from "effect/Ref"; import * as Schema from "effect/Schema"; +import * as Base64 from "effect/encoding/Base64"; +import * as Result from "effect/Result"; import * as EffectAcpSchema from "effect-acp/compat"; import * as EffectAcpSchemaV1 from "effect-acp/schema-v1"; import { @@ -11,6 +13,11 @@ import { mergeToolActivityData, } from "@t3tools/shared/toolActivity"; import { T3_MCP_TOOL_NAMES } from "@t3tools/shared/t3McpToolPresentation"; +import { + readToolOutputImage, + MAX_TOOL_OUTPUT_IMAGE_BASE64_LENGTH, +} from "@t3tools/shared/toolOutput"; +import { PROVIDER_SEND_TURN_MAX_IMAGE_BYTES } from "@t3tools/contracts"; import type { OrchestrationV2ProviderThreadNativeMetadata, ThreadTokenUsageSnapshot, @@ -314,6 +321,30 @@ const boundedContentUri = (value: string | null | undefined): string => { return uri.toLowerCase().startsWith("data:") ? "" : uri; }; +export function acpContentBlockImage(content: EffectAcpSchema.ContentBlock) { + if (content.type !== "image") return undefined; + const image = readToolOutputImage( + !content.data && content.uri?.startsWith("data:") ? { ...content, data: content.uri } : content, + ); + if (image?.data === undefined) return undefined; + const prefix = `data:${image.mimeType};base64,`; + const data = image.data.startsWith(prefix) ? image.data.slice(prefix.length) : image.data; + if (data.length === 0 || data.length > MAX_TOOL_OUTPUT_IMAGE_BASE64_LENGTH) return undefined; + const decoded = Base64.decode(data); + if ( + Result.isFailure(decoded) || + decoded.success.byteLength > PROVIDER_SEND_TURN_MAX_IMAGE_BYTES + ) { + return undefined; + } + return { + type: "image" as const, + mimeType: image.mimeType, + data, + sizeBytes: decoded.success.byteLength, + }; +} + export function acpContentBlockDisplayText( content: EffectAcpSchema.ContentBlock, ): string | undefined { @@ -356,7 +387,14 @@ function sanitizeAcpToolCallContent( ): ReadonlyArray { return content.map((entry) => { switch (entry.type) { - case "content": + case "content": { + const image = acpContentBlockImage(entry.content); + if (image !== undefined) { + return { + type: "content", + content: { type: "image", mimeType: image.mimeType, data: image.data }, + }; + } return { type: "content", content: { @@ -364,6 +402,7 @@ function sanitizeAcpToolCallContent( text: acpContentBlockDisplayText(entry.content) ?? "", }, }; + } case "diff": if ("changes" in entry) { return { diff --git a/packages/provider-core/src/server/ProviderHost.ts b/packages/provider-core/src/server/ProviderHost.ts index 298613bf13c91..fbdf514a81508 100644 --- a/packages/provider-core/src/server/ProviderHost.ts +++ b/packages/provider-core/src/server/ProviderHost.ts @@ -1,6 +1,8 @@ import type { BackgroundScope, ChatAttachment, + PersistChatAttachmentsInput, + PersistChatAttachmentsError, ServerSettings, ServerSettingsError, } from "@t3tools/contracts"; @@ -46,6 +48,9 @@ export class ProviderHost extends Context.Service< readonly shouldRunBackgroundWork: (scope: BackgroundScope) => Effect.Effect; /** Absolute attachment path, or null if the ID does not resolve inside attachmentsDir. */ readonly resolveAttachmentPath: (attachment: ChatAttachment) => string | null; + readonly persistChatAttachments?: ( + input: PersistChatAttachmentsInput, + ) => Effect.Effect, PersistChatAttachmentsError>; readonly resolveExtractedTextPath?: (path: string) => Effect.Effect; readonly credentials: ( namespace: string, From 8ff769f207f1f784bb9954f34b147e1b0a085d69 Mon Sep 17 00:00:00 2001 From: Amp Date: Tue, 3 Nov 2026 11:18:39 +0000 Subject: [PATCH 2/2] fix(server): avoid duplicate attachment persistence spans Delegate provider attachment persistence without opening a second span. The shared ChatAttachments operation retains its existing span and receives the same captured services. Written with Amp in an Orb. Co-authored-by: 5omeOtherGuy <268151429+5omeOtherGuy@users.noreply.github.com> Amp-Thread-ID: https://ampcode.com/threads/T-01a1a0b3-0b97-77d4-9fbf-85ca74a651ca --- apps/server/src/provider/ProviderHostLive.ts | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/apps/server/src/provider/ProviderHostLive.ts b/apps/server/src/provider/ProviderHostLive.ts index dfca52ec4bfc5..db3a14b02fb34 100644 --- a/apps/server/src/provider/ProviderHostLive.ts +++ b/apps/server/src/provider/ProviderHostLive.ts @@ -42,12 +42,14 @@ export const layer = Layer.effect( shouldRunBackgroundWork: backgroundPolicy.shouldRunScopeWork, resolveAttachmentPath: (attachment) => resolveAttachmentPath({ attachmentsDir: config.attachmentsDir, attachment }), - persistChatAttachments: Effect.fn("ProviderHost.persistChatAttachments")((input) => - persistChatAttachments(input).pipe( - Effect.provideService(ServerConfig.ServerConfig, config), - Effect.provideService(FileSystem.FileSystem, fileSystem), - Effect.provideService(Path.Path, path), - ), + // ChatAttachments owns the persistence span. + persistChatAttachments: Effect.fnUntraced( + function* (input) { + return yield* persistChatAttachments(input); + }, + Effect.provideService(ServerConfig.ServerConfig, config), + Effect.provideService(FileSystem.FileSystem, fileSystem), + Effect.provideService(Path.Path, path), ), // DocumentText owns the path-resolution span. resolveExtractedTextPath: Effect.fnUntraced(