Skip to content

Commit b874ff6

Browse files
fix(web): SnapShots taken during a question attach to that question (#15538)
Co-authored-by: Julius Marminge <[email protected]>
1 parent bce474e commit b874ff6

4 files changed

Lines changed: 250 additions & 20 deletions

File tree

‎apps/web/src/components/chat/ChatComposer.tsx‎

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import { RefreshIcon } from "~/components/ui/refresh-icon";
1818
import {
1919
questionAttachmentDraftId,
2020
countQuestionAttachments,
21+
trackOpenQuestionAttachmentDraft,
2122
useQuestionAttachmentPreparation,
2223
changeQuestionAttachmentPreparation,
2324
} from "../../questionAttachments";
@@ -2041,6 +2042,33 @@ export const ChatComposer = memo(function ChatComposer(props: ChatComposerProps)
20412042
: null;
20422043
const attachmentDraftTarget = questionAttachmentTarget ?? composerDraftTarget;
20432044
const attachmentDraft = useComposerThreadDraft(attachmentDraftTarget);
2045+
const questionTakesSnapShots =
2046+
supportsQuestionAttachments &&
2047+
activePendingProgress?.activeQuestion?.allowCustomAnswer !== false &&
2048+
!activePendingIsResponding;
2049+
const questionRequest = pendingUserInputs[0];
2050+
// Publish eligibility before async captures can observe a question the UI has already closed.
2051+
useLayoutEffect(() => {
2052+
if (!questionAttachmentTarget || !questionRequest || !questionTakesSnapShots) return;
2053+
return trackOpenQuestionAttachmentDraft(routeThreadRef, {
2054+
draftId: questionAttachmentTarget,
2055+
requestKeys: questionRequest.questions.map((question) =>
2056+
questionAttachmentDraftId(
2057+
environmentId,
2058+
activeThreadId!,
2059+
questionRequest.requestId,
2060+
question.id,
2061+
),
2062+
),
2063+
});
2064+
}, [
2065+
activeThreadId,
2066+
environmentId,
2067+
questionAttachmentTarget,
2068+
questionRequest,
2069+
questionTakesSnapShots,
2070+
routeThreadRef,
2071+
]);
20442072
const attachmentTargetKey = composerTargetKey(attachmentDraftTarget);
20452073
// An import that finishes after a draft change must compare against the draft open *now*, not
20462074
// the one captured in the closure that started it.
@@ -2079,7 +2107,7 @@ export const ChatComposer = memo(function ChatComposer(props: ChatComposerProps)
20792107
pendingSnapShotIdSet,
20802108
uncommittedSnapShotIds,
20812109
standaloneComposerImages,
2082-
} = useComposerImageGroups(composerDraftTarget, composerImages, composerPreviewAnnotations);
2110+
} = useComposerImageGroups(attachmentDraftTarget, composerImages, composerPreviewAnnotations);
20832111
const nonPersistedComposerImageIds = attachmentDraft.nonPersistedImageIds;
20842112
const uploadsByImageId = useAttachmentUploadStore((state) => state.uploadsByImageId);
20852113
const openPrLink = useOpenPrLink(routeThreadRef);

‎apps/web/src/components/desktop/SnapShotCoordinator.browser.test.ts‎

Lines changed: 133 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import {
55
type DesktopSnapShotEvent,
66
DEFAULT_CLIENT_SETTINGS,
77
EnvironmentId,
8+
PROVIDER_SEND_TURN_MAX_ATTACHMENTS,
89
ProjectId,
910
ThreadId,
1011
} from "@t3tools/contracts";
@@ -18,11 +19,17 @@ import * as settings from "../../hooks/useSettings";
1819
import * as newThread from "../../hooks/useHandleNewThread";
1920
import * as snapShotSound from "../../lib/snapShotSound";
2021
import { toastManager } from "../ui/toast";
22+
import * as imageCompression from "../../lib/imageCompression";
23+
import {
24+
questionAttachmentDraftId,
25+
trackOpenQuestionAttachmentDraft,
26+
} from "../../questionAttachments";
2127
import {
2228
beginSnapShotAnimationWhenReady,
2329
deliverSnapShot,
2430
dismissFailedSnapShot,
2531
resolveExistingSnapShotTarget,
32+
resolveSnapShotAttachmentTarget,
2633
resolveSnapShotTargetOnce,
2734
resolveSnapShotDeliveryTarget,
2835
SnapShotCoordinator,
@@ -38,6 +45,7 @@ import {
3845
vi.mock(import("../../hooks/useSettings"), { spy: true });
3946
vi.mock(import("../../hooks/useHandleNewThread"), { spy: true });
4047
vi.mock(import("../../lib/snapShotSound"), { spy: true });
48+
vi.mock(import("../../lib/imageCompression"), { spy: true });
4149

4250
const storage = vi.hoisted(() => {
4351
const values = new Map<string, string>();
@@ -537,7 +545,7 @@ describe("window capture delivery", () => {
537545
}
538546
});
539547
});
540-
const delivery = deliverSnapShot(bridge, item, target);
548+
const delivery = deliverSnapShot(bridge, item, () => target);
541549
await persisted;
542550
await vi.advanceTimersByTimeAsync(0);
543551

@@ -605,6 +613,128 @@ describe("window capture target resolution", () => {
605613
});
606614
});
607615

616+
describe("open question delivery", () => {
617+
const threadRef = scopeThreadRef(environmentId, ThreadId.make("question-thread"));
618+
const first = questionAttachmentDraftId(environmentId, threadRef.threadId, "request-1", "q1");
619+
const second = questionAttachmentDraftId(environmentId, threadRef.threadId, "request-1", "q2");
620+
const request = { draftId: first, requestKeys: [first, second] };
621+
const capture = {
622+
id: "12345678-1234-1234-1234-123456789abc",
623+
name: "window.png",
624+
mimeType: "image/png" as const,
625+
sizeBytes: 3,
626+
dataUrl: "data:image/png;base64,AQID",
627+
source: {
628+
kind: "snap-shot" as const,
629+
capturedAt: "2026-09-01T00:00:00.000Z",
630+
appName: "Editor",
631+
windowTitle: "main.ts",
632+
},
633+
};
634+
const makeBridge = (readSnapShot: () => Promise<typeof capture> = async () => capture) => {
635+
const acknowledgeSnapShot = vi.fn(async () => undefined);
636+
const bridge = { readSnapShot, acknowledgeSnapShot } as unknown as DesktopSnapShotBridge;
637+
return { bridge, acknowledgeSnapShot };
638+
};
639+
640+
it("attaches a capture to the open question draft and leaves the thread draft alone", async () => {
641+
const pins = new Map();
642+
const untrack = trackOpenQuestionAttachmentDraft(threadRef, request);
643+
try {
644+
const { bridge, acknowledgeSnapShot } = makeBridge();
645+
await deliverSnapShot(bridge, capture, () =>
646+
resolveSnapShotAttachmentTarget(pins, capture.id, threadRef),
647+
);
648+
const store = useComposerDraftStore.getState();
649+
expect(store.getComposerDraft(first)?.images.map(({ id }) => id)).toEqual([capture.id]);
650+
expect(store.getComposerDraft(threadRef)?.images ?? []).toHaveLength(0);
651+
expect(acknowledgeSnapShot).toHaveBeenCalledExactlyOnceWith(capture.id);
652+
} finally {
653+
untrack();
654+
}
655+
});
656+
657+
it("falls back to the thread when a newer question replaces the pinned one during compression", async () => {
658+
const pins = new Map();
659+
const untrackFirst = trackOpenQuestionAttachmentDraft(threadRef, request);
660+
let untrackSecond = () => {};
661+
const compression = vi
662+
.spyOn(imageCompression, "compressImageToByteLimit")
663+
.mockImplementationOnce(async (file) => {
664+
untrackSecond = trackOpenQuestionAttachmentDraft(threadRef, {
665+
...request,
666+
draftId: second,
667+
});
668+
untrackFirst();
669+
return { ok: true, file, recompressed: false };
670+
});
671+
try {
672+
const { bridge, acknowledgeSnapShot } = makeBridge();
673+
const resolveTarget = () => resolveSnapShotAttachmentTarget(pins, capture.id, threadRef);
674+
expect(resolveTarget()).toBe(first);
675+
await deliverSnapShot(bridge, capture, resolveTarget);
676+
const store = useComposerDraftStore.getState();
677+
expect(store.getComposerDraft(first)?.images ?? []).toHaveLength(0);
678+
expect(store.getComposerDraft(second)?.images ?? []).toHaveLength(0);
679+
expect(store.getComposerDraft(threadRef)?.images.map(({ id }) => id)).toEqual([capture.id]);
680+
expect(acknowledgeSnapShot).toHaveBeenCalledExactlyOnceWith(capture.id);
681+
} finally {
682+
compression.mockRestore();
683+
untrackFirst();
684+
untrackSecond();
685+
}
686+
});
687+
688+
it("keeps a capture pinned before any question off a question that opens during the read", async () => {
689+
const pins = new Map();
690+
const resolveTarget = () => resolveSnapShotAttachmentTarget(pins, capture.id, threadRef);
691+
// The drain pins a capture it never saw requested before reading it.
692+
expect(resolveTarget()).toEqual(threadRef);
693+
let untrack = () => {};
694+
const { bridge, acknowledgeSnapShot } = makeBridge(async () => {
695+
untrack = trackOpenQuestionAttachmentDraft(threadRef, request);
696+
return capture;
697+
});
698+
try {
699+
await deliverSnapShot(bridge, capture, resolveTarget);
700+
const store = useComposerDraftStore.getState();
701+
expect(store.getComposerDraft(first)?.images ?? []).toHaveLength(0);
702+
expect(store.getComposerDraft(threadRef)?.images.map(({ id }) => id)).toEqual([capture.id]);
703+
expect(acknowledgeSnapShot).toHaveBeenCalledExactlyOnceWith(capture.id);
704+
} finally {
705+
untrack();
706+
}
707+
});
708+
709+
it("refuses a capture once the request's other questions hold the shared limit", async () => {
710+
useComposerDraftStore.getState().addImages(
711+
first,
712+
Array.from({ length: PROVIDER_SEND_TURN_MAX_ATTACHMENTS }, (_, index) => ({
713+
type: "image" as const,
714+
id: `staged-${index}`,
715+
name: `staged-${index}.png`,
716+
mimeType: "image/png",
717+
sizeBytes: 3,
718+
previewUrl: `data:image/png;base64,AQID${index}`,
719+
file: new File([new Uint8Array([1, 2, 3])], `staged-${index}.png`, { type: "image/png" }),
720+
})),
721+
);
722+
const untrack = trackOpenQuestionAttachmentDraft(threadRef, { ...request, draftId: second });
723+
try {
724+
const { bridge, acknowledgeSnapShot } = makeBridge();
725+
await expect(deliverSnapShot(bridge, capture, () => second)).rejects.toThrow(
726+
"Remove an attachment",
727+
);
728+
expect(useComposerDraftStore.getState().getComposerDraft(second)?.images ?? []).toHaveLength(
729+
0,
730+
);
731+
expect(acknowledgeSnapShot).not.toHaveBeenCalled();
732+
} finally {
733+
untrack();
734+
}
735+
});
736+
});
737+
608738
describe("durable snapshot delivery", () => {
609739
it.each([false, true])(
610740
"retains a capture on quota failure and retries without duplicates (staged: %s)",
@@ -643,15 +773,15 @@ describe("durable snapshot delivery", () => {
643773
});
644774
void store.syncPersistedAttachments(target, [capture]);
645775
}
646-
await expect(deliverSnapShot(bridge, capture, target)).rejects.toThrow(
776+
await expect(deliverSnapShot(bridge, capture, () => target)).rejects.toThrow(
647777
"could not be saved",
648778
);
649779
expect(acknowledgeSnapShot).not.toHaveBeenCalled();
650780
expect(
651781
useComposerDraftStore.getState().getComposerDraft(target)?.nonPersistedImageIds,
652782
).toContain(capture.id);
653783
storage.setItem.mockImplementation(write);
654-
await deliverSnapShot(bridge, capture, target);
784+
await deliverSnapShot(bridge, capture, () => target);
655785
expect(acknowledgeSnapShot).toHaveBeenCalledExactlyOnceWith(capture.id);
656786
expect(useComposerDraftStore.getState().getComposerDraft(target)?.images).toHaveLength(1);
657787
expect(

‎apps/web/src/components/desktop/SnapShotCoordinator.tsx‎

Lines changed: 55 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import {
22
type DesktopPendingSnapShot,
33
type DesktopSnapShotEvent,
4+
PROVIDER_SEND_TURN_MAX_ATTACHMENTS,
45
PROVIDER_SEND_TURN_MAX_IMAGE_BYTES,
56
type ScopedThreadRef,
67
} from "@t3tools/contracts";
@@ -33,6 +34,10 @@ import {
3334
getDesktopSnapShotBridge,
3435
type DesktopSnapShotBridge,
3536
} from "../../lib/desktopSnapShot";
37+
import {
38+
countQuestionRequestAttachments,
39+
openQuestionAttachmentDraft,
40+
} from "../../questionAttachments";
3641
import { readFileAsDataUrl } from "../ChatView.logic";
3742
import { stackedThreadToast, toastManager } from "../ui/toast";
3843

@@ -65,6 +70,18 @@ export function resolveExistingSnapShotTarget(
6570
: null;
6671
}
6772

73+
// Pin absence too: a later question must never claim an earlier capture.
74+
export function resolveSnapShotAttachmentTarget(
75+
pins: Map<string, DraftId | null>,
76+
id: string,
77+
target: CaptureTarget,
78+
): CaptureTarget {
79+
const open = typeof target === "string" ? null : openQuestionAttachmentDraft(target);
80+
if (!pins.has(id)) pins.set(id, open?.draftId ?? null);
81+
const pinned = pins.get(id);
82+
return pinned && pinned === open?.draftId ? pinned : target;
83+
}
84+
6885
const NEXT_PAINT_FALLBACK_MS = 100;
6986

7087
export async function beginSnapShotAnimationWhenReady(
@@ -141,7 +158,7 @@ async function afterNextPaint(): Promise<void> {
141158
export async function deliverSnapShot(
142159
bridge: DesktopSnapShotBridge,
143160
item: DesktopPendingSnapShot,
144-
target: CaptureTarget,
161+
resolveTarget: () => CaptureTarget,
145162
): Promise<void> {
146163
const store = useComposerDraftStore.getState();
147164
updateSnapShotAnimationSource(item.id, item.source);
@@ -155,22 +172,29 @@ export async function deliverSnapShot(
155172
const file = compressed.file;
156173
const source = resizeSnapShotSource(capture.source, compressed.imageSize);
157174
const dataUrl = compressed.recompressed ? await readFileAsDataUrl(file) : capture.dataUrl;
175+
const target = resolveTarget();
158176
const alreadyAttached =
159177
store.getComposerDraft(target)?.images.some(({ id }) => id === capture.id) ?? false;
160-
if (
161-
!alreadyAttached &&
162-
!store.addImage(target, {
163-
type: "image",
178+
if (!alreadyAttached) {
179+
const questionAttachmentCount =
180+
typeof target === "string" ? countQuestionRequestAttachments(target) : null;
181+
const image = {
182+
type: "image" as const,
164183
id: capture.id,
165184
name: file.name,
166185
mimeType: file.type,
167186
sizeBytes: file.size,
168187
previewUrl: dataUrl,
169188
file,
170189
source,
171-
})
172-
) {
173-
throw new Error("Remove an attachment, then try this capture again.");
190+
};
191+
// Only question drafts may bypass addImage's thread-session gate.
192+
const added =
193+
questionAttachmentCount === null
194+
? store.addImage(target, image)
195+
: questionAttachmentCount < PROVIDER_SEND_TURN_MAX_ATTACHMENTS &&
196+
store.addImages(target, [image]).includes(capture.id);
197+
if (!added) throw new Error("Remove an attachment, then try this capture again.");
174198
}
175199
const persisted: PersistedComposerImageAttachment = {
176200
id: capture.id,
@@ -216,6 +240,7 @@ export function SnapShotCoordinator() {
216240
const animateCaptures = useClientSettings((settings) => settings.snapShotAnimations);
217241
const enabled = useClientSettings((settings) => settings.snapShotEnabled);
218242
const captureTargetsRef = useRef(new Map<string, Promise<CaptureTarget | null>>());
243+
const questionPinsRef = useRef(new Map<string, DraftId | null>());
219244
const targetResolutionRef = useRef<Promise<CaptureTarget | null> | null>(null);
220245
const drainingRef = useRef<Promise<void> | null>(null);
221246
const rerunRequestedRef = useRef(false);
@@ -336,8 +361,13 @@ export function SnapShotCoordinator() {
336361
}
337362

338363
try {
339-
await deliverSnapShot(bridge, item, target);
364+
// Pending captures may have arrived without a requested event; pin before reading.
365+
resolveSnapShotAttachmentTarget(questionPinsRef.current, item.id, target);
366+
await deliverSnapShot(bridge, item, () =>
367+
resolveSnapShotAttachmentTarget(questionPinsRef.current, item.id, target),
368+
);
340369
captureTargetsRef.current.delete(item.id);
370+
questionPinsRef.current.delete(item.id);
341371
soundedCaptureIdsRef.current.delete(item.id);
342372
reportedCaptureFailuresRef.current.delete(item.id);
343373
} catch (error) {
@@ -387,22 +417,29 @@ export function SnapShotCoordinator() {
387417
const target = current ? resolveExistingSnapShotTarget(current, routeThreadRef) : null;
388418
// Creating a new draft would navigate the renderer before a self-capture finishes.
389419
if (target) {
420+
resolveSnapShotAttachmentTarget(questionPinsRef.current, event.id, target);
390421
void resolveSnapShotDeliveryTarget(captureTargetsRef.current, event.id, () =>
391422
Promise.resolve(target),
392423
);
424+
} else if (!questionPinsRef.current.has(event.id)) {
425+
questionPinsRef.current.set(event.id, null);
393426
}
394427
return;
395428
}
396429
case "started": {
397430
playCaptureSound(event.id);
431+
const target = resolveSnapShotDeliveryTarget(
432+
captureTargetsRef.current,
433+
event.id,
434+
resolveCaptureTarget,
435+
).then(
436+
(target) =>
437+
target && resolveSnapShotAttachmentTarget(questionPinsRef.current, event.id, target),
438+
);
398439
if (animateCaptures && !window.matchMedia("(prefers-reduced-motion: reduce)").matches) {
399440
void beginSnapShotAnimationWhenReady(
400441
event.id,
401-
resolveSnapShotDeliveryTarget(
402-
captureTargetsRef.current,
403-
event.id,
404-
resolveCaptureTarget,
405-
),
442+
target,
406443
pendingAnimationStartsRef.current,
407444
);
408445
}
@@ -412,7 +449,10 @@ export function SnapShotCoordinator() {
412449
void drain();
413450
return;
414451
case "failed": {
415-
if (event.id) captureTargetsRef.current.delete(event.id);
452+
if (event.id) {
453+
captureTargetsRef.current.delete(event.id);
454+
questionPinsRef.current.delete(event.id);
455+
}
416456
dismissFailedSnapShot(
417457
event.id,
418458
soundedCaptureIdsRef.current,

0 commit comments

Comments
 (0)