Skip to content

Commit b925f2e

Browse files
t3dotggclaude
andcommitted
fix(desktop): tell the renderer when the crop was dropped
A failed or timed out capturePage came back as a null screenshot, which the renderer could not tell apart from a comment-only pick, so the main hang this PR bounds produced no notice at all. The pick result now carries an optional screenshotFailed flag and the renderer shows its toast on either failure path. Co-Authored-By: Claude Fable 5.1 <[email protected]>
1 parent c92ac0c commit b925f2e

5 files changed

Lines changed: 57 additions & 6 deletions

File tree

‎apps/desktop/src/preview/Manager.test.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3207,6 +3207,7 @@ describe("PreviewManager", () => {
32073207
// which would leave the composer stuck on "Capturing…".
32083208
const result = yield* Fiber.join(pick);
32093209
expect(result?.annotation.screenshot).toBeNull();
3210+
expect(result?.screenshotFailed).toBe(true);
32103211
expect(result?.submission).toBe("send");
32113212
expect(webviewSend).toHaveBeenCalledWith("preview:annotation-captured");
32123213
}),

‎apps/desktop/src/preview/Manager.ts‎

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2464,11 +2464,21 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function
24642464
const submission = args[2] === "send" ? "send" : "attach";
24652465
runFork(
24662466
captureAnnotationScreenshot(tabId, wc, cropRect).pipe(
2467+
// The renderer cannot tell a dropped crop from a comment-only
2468+
// pick by the null alone, so a failed or timed-out capture is
2469+
// flagged on the result.
24672470
Effect.match({
2468-
onFailure: () => payload,
2469-
onSuccess: (screenshot) => ({ ...payload, screenshot }),
2471+
onFailure: (): PreviewAnnotationSubmissionResult => ({
2472+
annotation: payload,
2473+
submission,
2474+
screenshotFailed: true,
2475+
}),
2476+
onSuccess: (screenshot): PreviewAnnotationSubmissionResult =>
2477+
screenshot === null
2478+
? { annotation: payload, submission, screenshotFailed: true }
2479+
: { annotation: { ...payload, screenshot }, submission },
24702480
}),
2471-
Effect.flatMap((annotation) => {
2481+
Effect.flatMap((result) => {
24722482
// A capture that outlives its session must not touch the
24732483
// overlay: the preload tears down on the captured signal, and
24742484
// by now it may be running a newer pick.
@@ -2478,7 +2488,7 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function
24782488
() => {
24792489
if (!wc.isDestroyed()) wc.send(ANNOTATION_CAPTURED_CHANNEL);
24802490
},
2481-
).pipe(Effect.ignore, Effect.andThen(finishPick({ annotation, submission })));
2491+
).pipe(Effect.ignore, Effect.andThen(finishPick(result)));
24822492
}),
24832493
),
24842494
);

‎apps/web/src/components/preview/PreviewView.test.tsx‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -252,6 +252,7 @@ vi.mock("~/browser/BrowserSurfaceSlot", () => ({ BrowserSurfaceSlot: () => null
252252
vi.mock("./usePreviewSession", () => ({ usePreviewSession: vi.fn() }));
253253

254254
import { PreviewView, previewProfileName } from "./PreviewView";
255+
import { toastManager } from "~/components/ui/toast";
255256
import { previewRuntimeTabId } from "~/browser/previewRuntimeTabId";
256257

257258
const TEST_THREAD_REF = {
@@ -342,6 +343,7 @@ describe("PreviewView navigation", () => {
342343
mocks.capturePreviewAnnotationScreenshot.mockReset();
343344
mocks.capturePreviewAnnotationScreenshot.mockResolvedValue({ status: "none" });
344345
mocks.addPreviewAnnotation.mockClear();
346+
vi.mocked(toastManager.add).mockClear();
345347
mocks.addImage.mockClear();
346348
mocks.toggleAnnotation = null;
347349
mocks.pictureInPicture = false;
@@ -552,6 +554,38 @@ describe("PreviewView navigation", () => {
552554
expect(mocks.addPreviewAnnotation).toHaveBeenCalledWith(TEST_THREAD_REF, annotation);
553555
});
554556

557+
it("warns when main dropped the crop before handing over the pick", async () => {
558+
const annotation = {
559+
id: "annotation-3",
560+
pageUrl: "https://example.com/dashboard",
561+
pageTitle: "Dashboard",
562+
comment: "Tighten this spacing",
563+
elements: [],
564+
regions: [],
565+
strokes: [],
566+
styleChanges: [],
567+
screenshot: null,
568+
createdAt: "2026-07-27T00:00:00.000Z",
569+
};
570+
const onSendAnnotation = vi.fn();
571+
mocks.pickElement.mockResolvedValue({ annotation, submission: "send", screenshotFailed: true });
572+
573+
renderToStaticMarkup(
574+
<PreviewView
575+
threadRef={TEST_THREAD_REF}
576+
tabId="tab-1"
577+
visible
578+
onSendAnnotation={onSendAnnotation}
579+
/>,
580+
);
581+
mocks.toggleAnnotation?.();
582+
583+
await vi.waitFor(() => expect(onSendAnnotation).toHaveBeenCalledWith(annotation, null));
584+
// A null screenshot alone looks like a comment-only pick; the flag is what
585+
// separates "no crop requested" from "crop lost to a timeout".
586+
expect(toastManager.add).toHaveBeenCalledTimes(1);
587+
});
588+
555589
it("still sends when the picked element's crop cannot be captured", async () => {
556590
const annotation = {
557591
id: "annotation-2",

‎apps/web/src/components/preview/PreviewView.tsx‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -579,16 +579,19 @@ export function PreviewView({
579579
try {
580580
const result = await previewBridge.pickElement(runtimeTabId);
581581
if (!result) return;
582-
const { annotation: picked, submission } = result;
582+
const { annotation: picked, submission, screenshotFailed = false } = result;
583583
// The structured annotation is still sendable when its optional crop
584584
// stalls or fails, so tell the user what they lost and keep going
585585
// instead of holding the composer for an attachment that never lands.
586586
// The stored copy drops the screenshot on failure, otherwise the prompt
587587
// would tell the agent a crop is attached when none was sent.
588588
const capture = await capturePreviewAnnotationScreenshot(picked);
589+
// Main reports a crop that failed or timed out on its side; the local
590+
// conversion can fail too. Either way the user should hear about it.
591+
const cropDropped = screenshotFailed || capture.status === "failed";
589592
const annotation = capture.status === "failed" ? { ...picked, screenshot: null } : picked;
590593
addPreviewAnnotation(threadRef, annotation);
591-
if (capture.status === "failed") {
594+
if (cropDropped) {
592595
toastManager.add(
593596
stackedThreadToast({
594597
type: "error",

‎packages/contracts/src/ipc.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -934,11 +934,14 @@ export const PreviewAnnotationSubmissionSchema: Schema.Codec<PreviewAnnotationSu
934934
export interface PreviewAnnotationSubmissionResult {
935935
annotation: PreviewAnnotationPayload;
936936
submission: PreviewAnnotationSubmission;
937+
/** The crop was requested but failed or timed out, so `annotation.screenshot` is null. */
938+
screenshotFailed?: boolean;
937939
}
938940
export const PreviewAnnotationSubmissionResultSchema: Schema.Codec<PreviewAnnotationSubmissionResult> =
939941
Schema.Struct({
940942
annotation: PreviewAnnotationPayloadSchema,
941943
submission: PreviewAnnotationSubmissionSchema,
944+
screenshotFailed: Schema.optionalKey(Schema.Boolean),
942945
});
943946

944947
export const DesktopPreviewTabInputSchema = Schema.Struct({

0 commit comments

Comments
 (0)