Skip to content

Commit 706c4dc

Browse files
authored
Attach files to a message (#489)
Add staged uploads, file previews, bounded model inputs, and attachment cleanup. Preserve attachments across failed sends and verify durable sent state before model invocation.
1 parent e513aad commit 706c4dc

90 files changed

Lines changed: 37275 additions & 300 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎.gitignore‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,10 @@ docs/plans/
1313
!.env.example
1414
# Per-Bot egress proxies. Carries credentials in the URL, like .env does.
1515
egress.env
16-
node_modules/
16+
# No trailing slash, deliberately. With one, this pattern matches a DIRECTORY only, and a symlink
17+
# named node_modules is not a directory — so a sandbox that links its dependencies at the repo root
18+
# leaves three links git will happily commit. That happened.
19+
node_modules
1720
**/dist/
1821
app/src/lib/generated/application-config.ts
1922
.logs/

‎app/serve.ts‎

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -74,14 +74,33 @@ export function upstreamWebSocketHeaders(requestHeaders: Headers): Headers {
7474
return headers;
7575
}
7676

77+
/**
78+
* Bun's WebSocket client: the DOM one, plus the two things Bun adds and the browser does not.
79+
*
80+
* `bun-types` hands the global `WebSocket` over to `lib.dom` whenever DOM is in `lib`, and this
81+
* project has DOM in `lib` for the browser code that is the rest of `app/`. What is left describes
82+
* the BROWSER's client, which has no `terminate()` and takes subprotocols where Bun takes options.
83+
* This file only ever runs under Bun and uses both.
84+
*
85+
* Stated as an extension of the DOM type rather than by reaching for `Bun.WebSocket`, because that
86+
* interface is written for projects WITHOUT DOM in `lib` and degrades to `{}` here — its events
87+
* come back untyped, which costs more than the two members it would buy. Same socket either way;
88+
* this is a type-level correction, not a different client.
89+
*/
90+
export type BunWebSocket = WebSocket & { terminate(): void };
91+
export const BunWebSocket = globalThis.WebSocket as unknown as {
92+
new (url: string | URL, options?: { headers?: HeadersInit }): BunWebSocket;
93+
readonly OPEN: number;
94+
};
95+
7796
type WebSocketBridge = {
78-
upstream: WebSocket;
97+
upstream: BunWebSocket;
7998
attach: (downstream: ServerWebSocket<WebSocketBridge>) => void;
8099
dispose: () => void;
81100
};
82101

83102
/** Own the upstream before awaiting its handshake, including any immediate welcome frames. */
84-
function prepareWebSocketBridge(upstream: WebSocket, signal: AbortSignal) {
103+
function prepareWebSocketBridge(upstream: BunWebSocket, signal: AbortSignal) {
85104
upstream.binaryType = "arraybuffer";
86105
let downstream: ServerWebSocket<WebSocketBridge> | undefined;
87106
const pending: (string | ArrayBuffer)[] = [];
@@ -185,7 +204,7 @@ if (import.meta.main) {
185204
},
186205
message(ws, message) {
187206
const { upstream } = ws.data;
188-
if (upstream.readyState === WebSocket.OPEN) {
207+
if (upstream.readyState === BunWebSocket.OPEN) {
189208
upstream.send(message);
190209
} else {
191210
ws.close(1011, "Upstream connection closed");
@@ -202,13 +221,13 @@ if (import.meta.main) {
202221
if (isApiCall(url.pathname)) {
203222
const target = SERVER + url.pathname + url.search;
204223
if (request.headers.get("upgrade")?.toLowerCase() === "websocket") {
205-
const upstream = new WebSocket(target.replace(/^http/, "ws"), {
224+
const upstream = new BunWebSocket(target.replace(/^http/, "ws"), {
206225
headers: upstreamWebSocketHeaders(request.headers),
207226
});
208227
const bridge = prepareWebSocketBridge(upstream, request.signal);
209228
if (
210229
!(await bridge.opened) ||
211-
upstream.readyState !== WebSocket.OPEN
230+
upstream.readyState !== BunWebSocket.OPEN
212231
) {
213232
bridge.data.dispose();
214233
return new Response("Could not connect to the upstream WebSocket", {

‎app/src/components/channels/channel-chat.tsx‎

Lines changed: 183 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,13 @@
11
import type { Message } from "@ag-ui/core";
22
import {
3+
type Attachment,
34
UseAgentUpdate,
45
useAgent,
56
useCopilotKit,
67
} from "@copilotkit/react-core/v2";
78
import { useMutation, useQuery } from "@tanstack/react-query";
89
import { useCallback, useEffect, useRef, useState } from "react";
10+
import { attachmentModality } from "@/components/channels/chat-messages";
911
import { toAgentOptions } from "@/components/channels/composer";
1012
import { ConversationView } from "@/components/channels/conversation-view";
1113
import {
@@ -14,6 +16,7 @@ import {
1416
transcriptMessages,
1517
} from "@/components/channels/transcript-messages";
1618
import { agentListQueryOptions } from "@/lib/agents/queries";
19+
import { attachmentUrl } from "@/lib/channels/attachments";
1720
import {
1821
recordChannelActivityMutationOptions,
1922
setChannelBusy,
@@ -126,6 +129,95 @@ function mergeStoredMessages(local: Message[], stored: Message[]): Message[] {
126129
];
127130
}
128131

132+
/**
133+
* The uploaded id and filename an `Attachment` carries once it is `ready`, read from the
134+
* `metadata` the composer's `onUpload` stamped on it — see `composer/attachments.ts`. Not the
135+
* SDK's own `attachment.id`, which is a client-side handle for the upload placeholder rather than
136+
* the id this deployment stored the file under.
137+
*/
138+
function uploadedAttachment(attachment: Attachment): {
139+
attachmentId: string;
140+
filename?: string;
141+
} {
142+
const metadata = attachment.metadata as
143+
| { attachmentId?: unknown; filename?: unknown }
144+
| undefined;
145+
const attachmentId = metadata?.attachmentId;
146+
if (typeof attachmentId !== "string") {
147+
throw new Error("Attachment is missing its uploaded id.");
148+
}
149+
const filename =
150+
typeof metadata?.filename === "string" ? metadata.filename : undefined;
151+
return filename ? { attachmentId, filename } : { attachmentId };
152+
}
153+
154+
/**
155+
* One attachment, turned into the part shape a stored message carries.
156+
*
157+
* THE MODALITY COMES FROM THE BYTES, NOT FROM `attachment.type`, AND THIS IS THE ONLY PLACE IT CAN.
158+
*
159+
* `attachment.type` is the browser's claim, fixed before the upload and never reconciled with what
160+
* the file turned out to be. The server stopped trusting it — `resolvePart` decides an attachment's
161+
* modality with `classifyAttachment` on its own sniffed `mimeType` — but that correction lives on
162+
* the server and never comes back here. What this function writes IS the stored message, so a
163+
* `document` written here is what every later render of that message reads, for ever: a screenshot
164+
* whose part the browser mislabelled drew a grey file card over the picture, and the transcript's
165+
* document probe then paid a whole-file read per render for the privilege.
166+
*
167+
* Narrowed through the source union rather than read straight off, for the reason `parkedTiles` in
168+
* `chat-transcript.tsx` narrows the same field: a `data` source's `mimeType` is `file.type`, the
169+
* very claim being refused, and only a `url` source has been past the server. `attachmentModality`
170+
* falls back to the declared type when there is no corroborated one, so an attachment that somehow
171+
* arrives unuploaded is written exactly as it used to be.
172+
*
173+
* The comment this replaces said only "image" and "document" reach here because the composer's
174+
* upload config accepts no other kind of file. That reason is no longer true — the config's
175+
* `accept` is now the wildcard, and it is `screenPickedFiles` that holds the line. The conclusion still
176+
* holds; the justification had rotted, which is why the modality is now derived rather than cast.
177+
*/
178+
function toAttachmentPart(attachment: Attachment) {
179+
const { attachmentId, filename } = uploadedAttachment(attachment);
180+
const { source } = attachment;
181+
const mimeType =
182+
source.type === "url" && source.mimeType ? source.mimeType : undefined;
183+
return {
184+
type: attachmentModality(attachment.type, mimeType),
185+
source: { type: "url" as const, value: attachmentUrl(attachmentId) },
186+
metadata: filename ? { attachmentId, filename } : { attachmentId },
187+
};
188+
}
189+
190+
/**
191+
* A plain string when there is nothing attached, exactly as every message in every channel has
192+
* always been sent — never a single-element array wrapping the same text, which every existing
193+
* reader would take a different path for no gain. With attachments, the text goes first as its
194+
* own part and is left out entirely when empty, since an empty text part is noise the model has
195+
* to read past.
196+
*
197+
* Exported for the test that pins this wire format. Reaching it through `deliver`/`say` would mean
198+
* standing up `useAgent`'s runtime, the thread join and the ready/join gates around it just to
199+
* observe a pure string-in-object-out mapping — none of that machinery bears on what this function
200+
* decides, so a narrow export is the honest way to test the contract without restructuring the
201+
* module around a test.
202+
*/
203+
export function toMessageContent(
204+
trimmed: string,
205+
attachments: readonly Attachment[],
206+
) {
207+
if (attachments.length === 0) return trimmed;
208+
const refs = attachments.map(toAttachmentPart);
209+
return trimmed ? [{ type: "text" as const, text: trimmed }, ...refs] : refs;
210+
}
211+
212+
/** What the roster's "last thing said" reads when a message carried no caption. */
213+
function describeAttachments(attachments: readonly Attachment[]): string {
214+
if (attachments.length === 1) {
215+
const { filename } = uploadedAttachment(attachments[0]);
216+
return filename ? `Sent ${filename}` : "Sent an attachment";
217+
}
218+
return `Sent ${attachments.length} attachments`;
219+
}
220+
129221
/**
130222
* One channel's conversation with one coworker.
131223
*
@@ -408,6 +500,36 @@ export function ChannelChat({
408500
// Run failures arrive as events and are reported only for turns started in this mount.
409501
const [runError, setRunError] = useState<string | null>(null);
410502
const awaitingReply = useRef(false);
503+
/**
504+
* WHY THIS TURN ENDED WITHOUT AN ANSWER, KEPT WHERE `deliver` CAN STILL SEE IT — because the one
505+
* thing that knows is a subscriber, and the one thing that has to act on it is an `await`.
506+
*
507+
* `copilotkit.runAgent` DOES NOT REJECT ON A FAILED RUN. `CopilotKitCore.runAgent` catches
508+
* everything the agent throws, reports it through `emitError` as `AGENT_RUN_FAILED`, and returns
509+
* `{ result: undefined, newMessages: [] }` — a value indistinguishable from a run that finished
510+
* with nothing to say. So a gateway 503, a stream that dies, a model that refuses the request:
511+
* every one of them arrived here as a resolved promise, and `say` reported success for a turn
512+
* that never reached the server.
513+
*
514+
* WHAT THAT COST, WHICH IS THE REASON THIS EXISTS. `say` resolving is what every caller reads as
515+
* "it went". The composer clears the box and gives up the chips it was riding; the queue empties
516+
* into a draft nothing retries; `conversation-view.tsx` never runs either of the failure paths it
517+
* has written for exactly this. The person is left with the failed turn in the transcript and a
518+
* notice under it, the words unretryable, and the files behind them staged rows that nothing on
519+
* any screen points at any more. The notice is honest and everything under it was not.
520+
*
521+
* READ OFF THE SAME `fail` THE NOTICE IS, and deliberately not from a second subscription of its
522+
* own. `fail` already answers the one question a separate subscriber would get wrong: a turn the
523+
* PERSON stopped also reaches `onRunFailed`, with an abort, and `onStop` clears `awaitingReply`
524+
* before it — so Stop is not a failure here and nothing restores a draft somebody chose to end.
525+
*
526+
* ONE SLOT FOR ONE TURN AT A TIME, the same assumption `awaitingReply` beside it already makes.
527+
* Two overlapping turns — a component button pressed during a composer send — would have the
528+
* second clear the first's reason, which reports the earlier turn as successful. That is the
529+
* pre-existing shape of `awaitingReply`, not a new one, and narrowing it means giving a run a
530+
* handle that `copilotkit.runAgent` does not hand back.
531+
*/
532+
const turnFailure = useRef<string | null>(null);
411533
const assistantMessagesBeforeRun = useRef<Set<string>>(new Set());
412534

413535
/*
@@ -466,7 +588,11 @@ export function ChannelChat({
466588
* Everything `say` does once it has something worth sending, split out so the counter it is
467589
* wrapped in covers every way out of here, a throw included.
468590
*/
469-
const deliver = async (trimmed: string, skillInstructions: string[]) => {
591+
const deliver = async (
592+
trimmed: string,
593+
skillInstructions: string[],
594+
attachments: Attachment[],
595+
) => {
470596
// Wait briefly for the runtime agent instance before adding the message.
471597
if (!isReadyRef.current) {
472598
await Promise.race([
@@ -490,6 +616,7 @@ export function ChannelChat({
490616
const target = agentRef.current;
491617

492618
setRunError(null);
619+
turnFailure.current = null;
493620
assistantMessagesBeforeRun.current = new Set(
494621
target.messages
495622
.filter((message) => message.role === "assistant")
@@ -518,11 +645,11 @@ export function ChannelChat({
518645
}
519646

520647
target.addMessage({
521-
content: trimmed,
648+
content: toMessageContent(trimmed, attachments),
522649
id: newId(),
523650
role: "user",
524651
});
525-
report(trimmed, null);
652+
report(trimmed || describeAttachments(attachments), null);
526653

527654
// Providers reject later turns if prior tool calls have no result; repair before sending.
528655
const repaired = repairUnansweredToolCalls(target.messages);
@@ -536,6 +663,30 @@ export function ChannelChat({
536663
} finally {
537664
setRunsInFlight((count) => count - 1);
538665
}
666+
667+
/*
668+
* A TURN THAT DID NOT HAPPEN FAILS THE SEND, which is the only way anything upstream can tell.
669+
* See `turnFailure` for why the resolved promise above says nothing about that.
670+
*
671+
* AFTER the `finally`, not inside the `try`: the run is over either way, so the counter that
672+
* draws the Stop button must come down before this throws. Throwing from inside would leave
673+
* `runsInFlight` high for a run that has already ended.
674+
*
675+
* WHAT THE THROW REACHES, so it is clear this is a message and not a crash. The composer's
676+
* `catch` puts the words and the chips back; `conversation-view.tsx` puts a drained queue back
677+
* as retryable entries carrying their files. Nothing here reports the failure — `runError` was
678+
* already set from the same `fail` that set this, and the transcript already draws it — so this
679+
* adds a retry, not a second sentence.
680+
*
681+
* THE MESSAGE STAYS ON SCREEN. `deliver` added it above and nothing takes it away: it is what
682+
* the failed turn WAS, it is what the notice under it is about, and removing it would delete a
683+
* partial answer that a mid-stream failure had already produced. The restored draft beside it
684+
* is the retry, the same way a failed composer send has always put its words back while the
685+
* transcript kept the turn.
686+
*/
687+
if (turnFailure.current !== null) {
688+
throw new Error(turnFailure.current);
689+
}
539690
};
540691

541692
/**
@@ -546,17 +697,24 @@ export function ChannelChat({
546697
* keeping here rather than in the view: the view sees only the turns it started itself, and a
547698
* queue that drains on the wrong one of those posts a correction into the middle of an answer.
548699
*/
549-
const say = async (text: string, skillInstructions: string[] = []) => {
700+
const say = async (
701+
text: string,
702+
skillInstructions: string[] = [],
703+
attachments: Attachment[] = [],
704+
) => {
550705
const trimmed = text.trim();
551-
if (!trimmed) return;
706+
// A pasted screenshot with no caption is still a message to send: `canSendDraft` already
707+
// unlocks the button for exactly this case, so refusing it here would leave the button
708+
// enabled and inert.
709+
if (!trimmed && attachments.length === 0) return;
552710

553711
turnsRef.current += 1;
554712
setTurnsInFlight(turnsRef.current);
555713
if (turnsRef.current === 1) {
556714
void setChannelBusy({ channelId: channel.id, busy: true });
557715
}
558716
try {
559-
await deliver(trimmed, skillInstructions);
717+
await deliver(trimmed, skillInstructions, attachments);
560718
} finally {
561719
turnsRef.current -= 1;
562720
setTurnsInFlight(turnsRef.current);
@@ -572,6 +730,9 @@ export function ChannelChat({
572730
const fail = (message: string) => {
573731
if (!awaitingReply.current) return;
574732
awaitingReply.current = false;
733+
// Both halves of one fact: the sentence the transcript shows, and the reason `deliver` throws
734+
// so the draft behind the turn is restored rather than counted as sent. See `turnFailure`.
735+
turnFailure.current = message;
575736
setRunError(message);
576737
};
577738
const subscription = agent.subscribe?.({
@@ -604,9 +765,16 @@ export function ChannelChat({
604765

605766
/**
606767
* Component buttons speak as user turns without forcing every transcript card to re-render.
768+
*
769+
* The rejection is swallowed HERE rather than left to the void, and that is not a style choice:
770+
* `say` throws on a failed turn now (see `turnFailure`), and a voided promise with nothing on the
771+
* end of it is an unhandled rejection — in this repository's test runner, a failure attributed to
772+
* whichever test happened to be running when it surfaced. There is nothing to restore for this
773+
* caller either way: the words came from a button inside a rendered card, not from a box somebody
774+
* is still holding, and the failed turn is already reported by `runError` under the transcript.
607775
*/
608776
const askFromComponent = useCallback((text: string) => {
609-
void sayRef.current(text);
777+
void sayRef.current(text).catch(() => undefined);
610778
}, []);
611779

612780
/**
@@ -618,9 +786,12 @@ export function ChannelChat({
618786
if (!pending) return;
619787
seedRef.current = null;
620788

621-
void sayRef.current(
622-
typeof pending.content === "string" ? pending.content : "",
623-
);
789+
// Swallowed for the reason `askFromComponent` above records: `say` throws on a failed turn, and
790+
// the seed has no box to go back into — it was typed on a screen that has already navigated
791+
// away. The transcript keeps the seeded message and the notice under it says what happened.
792+
void sayRef
793+
.current(typeof pending.content === "string" ? pending.content : "")
794+
.catch(() => undefined);
624795

625796
// Keep `seed` in state; transcriptMessages gives it up once the agent holds a user turn.
626797
}, []);
@@ -629,6 +800,7 @@ export function ChannelChat({
629800
<ConversationProvider ask={askFromComponent}>
630801
<ConversationView
631802
agents={toAgentOptions(agentProfiles, channel.agentIds)}
803+
channelId={channel.id}
632804
/*
633805
* THE TURN, not the run. `say` waits for the runtime agent and the join before a run starts,
634806
* and `agent.isRunning` alone leaves that gap unmarked — which is the one moment the
@@ -677,7 +849,7 @@ export function ChannelChat({
677849
Boolean(instruction),
678850
);
679851

680-
await say(draft.text, skillInstructions);
852+
await say(draft.text, skillInstructions, draft.attachments);
681853
}}
682854
/**
683855
* Stop through the core so the abort signal reaches frontend tools; `say` repairs any

0 commit comments

Comments
 (0)