| author | |
| committer | |
| log | 19c97ea56d30b3a2de31a060f8f47d6b7404b78f |
| tree | 4915d07aca5d8acc8ade88e8a003c72e632781ce |
| parent | 2b745efe57cddc1753d3007d081c64bd20e3ab79 |
| signature | Signed by PGP key B5690EEEBB952194 |
Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>8 files changed, 495 insertions(+), 83 deletions(-)
apps/desktop/src/preview/Manager.test.ts+158| ... | @@ -3137,6 +3137,164 @@ describe("PreviewManager", () => { | ... | @@ -3137,6 +3137,164 @@ describe("PreviewManager", () => { |
| 3137 | ), | 3137 | ), |
| 3138 | ); | 3138 | ); |
| 3139 | 3139 | ||
| 3140 | effectIt.effect("settles the pick when the annotation screenshot never arrives", () => | ||
| 3141 | withManager((manager) => | ||
| 3142 | Effect.gen(function* () { | ||
| 3143 | let onPicked: ((event: unknown, ...args: unknown[]) => void) | undefined; | ||
| 3144 | fromId.mockReturnValue({ | ||
| 3145 | id: 42, | ||
| 3146 | isDestroyed: () => false, | ||
| 3147 | getType: () => "webview", | ||
| 3148 | getURL: () => "https://example.com", | ||
| 3149 | getTitle: () => "Example", | ||
| 3150 | isLoading: () => false, | ||
| 3151 | isFocused: () => true, | ||
| 3152 | getZoomFactor: () => 1, | ||
| 3153 | setZoomFactor: vi.fn(), | ||
| 3154 | setAudioMuted: vi.fn(), | ||
| 3155 | isCurrentlyAudible: () => false, | ||
| 3156 | on: vi.fn(), | ||
| 3157 | once: vi.fn(), | ||
| 3158 | off: vi.fn(), | ||
| 3159 | // A wedged compositor leaves `capturePage` pending forever. | ||
| 3160 | capturePage: vi.fn(() => new Promise(() => {})), | ||
| 3161 | ipc: { | ||
| 3162 | on: vi.fn((channel: string, listener: typeof onPicked) => { | ||
| 3163 | if (channel === "preview:element-picked") onPicked = listener; | ||
| 3164 | }), | ||
| 3165 | off: vi.fn(), | ||
| 3166 | removeListener: vi.fn(), | ||
| 3167 | }, | ||
| 3168 | send: webviewSend, | ||
| 3169 | navigationHistory: { canGoBack: () => false, canGoForward: () => false }, | ||
| 3170 | setWindowOpenHandler: vi.fn(), | ||
| 3171 | debugger: { | ||
| 3172 | isAttached: () => false, | ||
| 3173 | attach: vi.fn(), | ||
| 3174 | sendCommand: vi.fn(async () => undefined), | ||
| 3175 | on: vi.fn(), | ||
| 3176 | off: vi.fn(), | ||
| 3177 | }, | ||
| 3178 | } as never); | ||
| 3179 | |||
| 3180 | yield* manager.createTab("tab_1"); | ||
| 3181 | yield* manager.registerWebview("tab_1", 42); | ||
| 3182 | const pick = yield* manager.pickElement("tab_1").pipe(Effect.forkChild); | ||
| 3183 | yield* Effect.yieldNow; | ||
| 3184 | |||
| 3185 | onPicked?.( | ||
| 3186 | {}, | ||
| 3187 | { | ||
| 3188 | id: "annotation_1", | ||
| 3189 | pageUrl: "https://example.com", | ||
| 3190 | pageTitle: "Example", | ||
| 3191 | comment: "Tighten this spacing", | ||
| 3192 | elements: [], | ||
| 3193 | regions: [{ id: "region_1", rect: { x: 5, y: 6, width: 20, height: 30 } }], | ||
| 3194 | strokes: [], | ||
| 3195 | styleChanges: [], | ||
| 3196 | screenshot: null, | ||
| 3197 | createdAt: "2026-06-11T00:00:00.000Z", | ||
| 3198 | }, | ||
| 3199 | null, | ||
| 3200 | "send", | ||
| 3201 | ); | ||
| 3202 | yield* Effect.yieldNow; | ||
| 3203 | expect(pick.pollUnsafe()).toBeUndefined(); | ||
| 3204 | |||
| 3205 | yield* TestClock.adjust("6 seconds"); | ||
| 3206 | // The pick has to give up on the crop rather than strand the renderer, | ||
| 3207 | // which would leave the composer stuck on "Capturing…". | ||
| 3208 | const result = yield* Fiber.join(pick); | ||
| 3209 | expect(result?.annotation.screenshot).toBeNull(); | ||
| 3210 | expect(result?.screenshotFailed).toBe(true); | ||
| 3211 | expect(result?.submission).toBe("send"); | ||
| 3212 | expect(webviewSend).toHaveBeenCalledWith("preview:annotation-captured"); | ||
| 3213 | }), | ||
| 3214 | ), | ||
| 3215 | ); | ||
| 3216 | |||
| 3217 | effectIt.effect("a stale capture from a replaced pick never touches the next pick", () => | ||
| 3218 | withManager((manager) => | ||
| 3219 | Effect.gen(function* () { | ||
| 3220 | let onPicked: ((event: unknown, ...args: unknown[]) => void) | undefined; | ||
| 3221 | fromId.mockReturnValue({ | ||
| 3222 | id: 42, | ||
| 3223 | isDestroyed: () => false, | ||
| 3224 | getType: () => "webview", | ||
| 3225 | getURL: () => "https://example.com", | ||
| 3226 | getTitle: () => "Example", | ||
| 3227 | isLoading: () => false, | ||
| 3228 | isFocused: () => true, | ||
| 3229 | getZoomFactor: () => 1, | ||
| 3230 | setZoomFactor: vi.fn(), | ||
| 3231 | setAudioMuted: vi.fn(), | ||
| 3232 | isCurrentlyAudible: () => false, | ||
| 3233 | on: vi.fn(), | ||
| 3234 | once: vi.fn(), | ||
| 3235 | off: vi.fn(), | ||
| 3236 | capturePage: vi.fn(() => new Promise(() => {})), | ||
| 3237 | ipc: { | ||
| 3238 | on: vi.fn((channel: string, listener: typeof onPicked) => { | ||
| 3239 | if (channel === "preview:element-picked") onPicked = listener; | ||
| 3240 | }), | ||
| 3241 | off: vi.fn(), | ||
| 3242 | removeListener: vi.fn(), | ||
| 3243 | }, | ||
| 3244 | send: webviewSend, | ||
| 3245 | navigationHistory: { canGoBack: () => false, canGoForward: () => false }, | ||
| 3246 | setWindowOpenHandler: vi.fn(), | ||
| 3247 | debugger: { | ||
| 3248 | isAttached: () => false, | ||
| 3249 | attach: vi.fn(), | ||
| 3250 | sendCommand: vi.fn(async () => undefined), | ||
| 3251 | on: vi.fn(), | ||
| 3252 | off: vi.fn(), | ||
| 3253 | }, | ||
| 3254 | } as never); | ||
| 3255 | const annotation = { | ||
| 3256 | id: "annotation_1", | ||
| 3257 | pageUrl: "https://example.com", | ||
| 3258 | pageTitle: "Example", | ||
| 3259 | comment: "Tighten this spacing", | ||
| 3260 | elements: [], | ||
| 3261 | regions: [{ id: "region_1", rect: { x: 5, y: 6, width: 20, height: 30 } }], | ||
| 3262 | strokes: [], | ||
| 3263 | styleChanges: [], | ||
| 3264 | screenshot: null, | ||
| 3265 | createdAt: "2026-06-11T00:00:00.000Z", | ||
| 3266 | }; | ||
| 3267 | |||
| 3268 | yield* manager.createTab("tab_1"); | ||
| 3269 | yield* manager.registerWebview("tab_1", 42); | ||
| 3270 | const firstPick = yield* manager.pickElement("tab_1").pipe(Effect.forkChild); | ||
| 3271 | yield* Effect.yieldNow; | ||
| 3272 | // The first pick submits and its crop hangs. | ||
| 3273 | onPicked?.({}, annotation, null, "send"); | ||
| 3274 | yield* Effect.yieldNow; | ||
| 3275 | |||
| 3276 | // A second pick on the same tab replaces the first, which resumes null. | ||
| 3277 | const secondPick = yield* manager.pickElement("tab_1").pipe(Effect.forkChild); | ||
| 3278 | yield* Effect.yieldNow; | ||
| 3279 | expect(yield* Fiber.join(firstPick)).toBeNull(); | ||
| 3280 | webviewSend.mockClear(); | ||
| 3281 | |||
| 3282 | // The first pick's crop times out while the second pick is live. It | ||
| 3283 | // must not signal the overlay, which would tear down the second pick. | ||
| 3284 | yield* TestClock.adjust("6 seconds"); | ||
| 3285 | yield* Effect.yieldNow; | ||
| 3286 | expect(webviewSend).not.toHaveBeenCalledWith("preview:annotation-captured"); | ||
| 3287 | expect(secondPick.pollUnsafe()).toBeUndefined(); | ||
| 3288 | |||
| 3289 | onPicked?.({}, { ...annotation, id: "annotation_2" }, null, "attach"); | ||
| 3290 | yield* TestClock.adjust("6 seconds"); | ||
| 3291 | const result = yield* Fiber.join(secondPick); | ||
| 3292 | expect(result?.annotation.id).toBe("annotation_2"); | ||
| 3293 | expect(result?.submission).toBe("attach"); | ||
| 3294 | }), | ||
| 3295 | ), | ||
| 3296 | ); | ||
| 3297 | |||
| 3140 | effectIt.effect("navigates the guest history when the thumb-button ipc fires", () => | 3298 | effectIt.effect("navigates the guest history when the thumb-button ipc fires", () => |
| 3141 | withManager((manager) => | 3299 | withManager((manager) => |
| 3142 | Effect.gen(function* () { | 3300 | Effect.gen(function* () { |
apps/desktop/src/preview/Manager.ts+88-23| ... | @@ -306,13 +306,24 @@ const normalizeCaptureRect = (value: unknown): PreviewAnnotationRect | null => { | ... | @@ -306,13 +306,24 @@ const normalizeCaptureRect = (value: unknown): PreviewAnnotationRect | null => { |
| 306 | }; | 306 | }; |
| 307 | }; | 307 | }; |
| 308 | 308 | ||
| 309 | /** `capturePage` never settles when the guest's compositor is wedged. */ | ||
| 310 | const ANNOTATION_SCREENSHOT_TIMEOUT = "5 seconds"; | ||
| 311 | |||
| 312 | /** | ||
| 313 | * Crops the guest for a picked annotation. A stalled `capturePage` resolves to | ||
| 314 | * `null` after the timeout: the annotation is still sendable without its | ||
| 315 | * screenshot, and the pick session must settle either way. | ||
| 316 | */ | ||
| 309 | const captureAnnotationScreenshot = ( | 317 | const captureAnnotationScreenshot = ( |
| 310 | tabId: string, | 318 | tabId: string, |
| 311 | wc: Electron.WebContents, | 319 | wc: Electron.WebContents, |
| 312 | cropRect: PreviewAnnotationRect | null, | 320 | cropRect: PreviewAnnotationRect | null, |
| 313 | ): Effect.Effect<PreviewAnnotationPayload["screenshot"], PreviewManagerError> => | 321 | ): Effect.Effect<PreviewAnnotationPayload["screenshot"], PreviewManagerError> => |
| 314 | Effect.tryPromise({ | 322 | Effect.tryPromise({ |
| 315 | try: () => | 323 | // The unused abort signal is what makes this interruptible, and therefore |
| 324 | // what lets the timeout below fire. Drop the parameter and a stalled | ||
| 325 | // capture strands the pick session again. | ||
| 326 | try: (_signal) => | ||
| 316 | wc.capturePage( | 327 | wc.capturePage( |
| 317 | cropRect | 328 | cropRect |
| 318 | ? { | 329 | ? { |
| ... | @@ -331,7 +342,7 @@ const captureAnnotationScreenshot = ( | ... | @@ -331,7 +342,7 @@ const captureAnnotationScreenshot = ( |
| 331 | cause, | 342 | cause, |
| 332 | }), | 343 | }), |
| 333 | }).pipe( | 344 | }).pipe( |
| 334 | Effect.map((image) => { | 345 | Effect.map((image): PreviewAnnotationPayload["screenshot"] => { |
| 335 | const size = image.getSize(); | 346 | const size = image.getSize(); |
| 336 | return { | 347 | return { |
| 337 | dataUrl: image.toDataURL(), | 348 | dataUrl: image.toDataURL(), |
| ... | @@ -340,6 +351,15 @@ const captureAnnotationScreenshot = ( | ... | @@ -340,6 +351,15 @@ const captureAnnotationScreenshot = ( |
| 340 | cropRect: cropRect ?? { x: 0, y: 0, width: size.width, height: size.height }, | 351 | cropRect: cropRect ?? { x: 0, y: 0, width: size.width, height: size.height }, |
| 341 | }; | 352 | }; |
| 342 | }), | 353 | }), |
| 354 | Effect.timeoutOption(ANNOTATION_SCREENSHOT_TIMEOUT), | ||
| 355 | Effect.flatMap((screenshot) => | ||
| 356 | Option.isSome(screenshot) | ||
| 357 | ? Effect.succeed(screenshot.value) | ||
| 358 | : Effect.logWarning("preview annotation screenshot timed out").pipe( | ||
| 359 | Effect.annotateLogs({ tabId, webContentsId: wc.id }), | ||
| 360 | Effect.as(null), | ||
| 361 | ), | ||
| 362 | ), | ||
| 343 | ); | 363 | ); |
| 344 | 364 | ||
| 345 | const findZoomStep = (current: number): number => { | 365 | const findZoomStep = (current: number): number => { |
| ... | @@ -2370,30 +2390,52 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function | ... | @@ -2370,30 +2390,52 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function |
| 2370 | const annotationTheme = yield* Ref.get(annotationThemeRef); | 2390 | const annotationTheme = yield* Ref.get(annotationThemeRef); |
| 2371 | return yield* Effect.callback<PreviewAnnotationSubmissionResult | null, PreviewManagerError>( | 2391 | return yield* Effect.callback<PreviewAnnotationSubmissionResult | null, PreviewManagerError>( |
| 2372 | (resume) => { | 2392 | (resume) => { |
| 2393 | // Declared first so cleanup can check slot ownership by identity | ||
| 2394 | // without a type cycle through the cancel effect it builds. | ||
| 2395 | const session: PickSession = { cancel: Effect.suspend(() => cancelPickSession()) }; | ||
| 2373 | const cleanup = Effect.fn("PreviewManager.cleanupPickElement")(function* () { | 2396 | const cleanup = Effect.fn("PreviewManager.cleanupPickElement")(function* () { |
| 2374 | yield* attempt({ operation: "pickElement.cleanup", tabId, webContentsId: wc.id }, () => { | 2397 | yield* attempt({ operation: "pickElement.cleanup", tabId, webContentsId: wc.id }, () => { |
| 2375 | wc.ipc.removeListener(ELEMENT_PICKED_CHANNEL, onMessage); | 2398 | wc.ipc.removeListener(ELEMENT_PICKED_CHANNEL, onMessage); |
| 2376 | wc.off("destroyed", onDestroyed); | 2399 | wc.off("destroyed", onDestroyed); |
| 2377 | wc.off("did-start-navigation", onNavigated); | 2400 | wc.off("did-start-navigation", onNavigated); |
| 2378 | }).pipe(Effect.ignore); | 2401 | }).pipe(Effect.ignore); |
| 2402 | // Only drop the slot while it is still ours. A newer session may | ||
| 2403 | // already have swapped itself in before cancelling this one. | ||
| 2379 | yield* Ref.update(pickSessionsRef, (sessions) => | 2404 | yield* Ref.update(pickSessionsRef, (sessions) => |
| 2380 | replaceMap(sessions, (copy) => { | 2405 | sessions.get(tabId) === session |
| 2381 | copy.delete(tabId); | 2406 | ? replaceMap(sessions, (copy) => { |
| 2382 | }), | 2407 | copy.delete(tabId); |
| 2408 | }) | ||
| 2409 | : sessions, | ||
| 2383 | ); | 2410 | ); |
| 2384 | }); | 2411 | }); |
| 2385 | const settlePick = Effect.fn("PreviewManager.settlePickElement")(function* ( | 2412 | // Every exit from this session runs through `claimSettle`, so the |
| 2413 | // renderer's `pickElement` promise resolves exactly once. The previous | ||
| 2414 | // identity check let a cancelled or replaced session return without | ||
| 2415 | // resuming, which left the composer waiting forever. | ||
| 2416 | let settled = false; | ||
| 2417 | const claimSettle = (): boolean => { | ||
| 2418 | if (settled) return false; | ||
| 2419 | settled = true; | ||
| 2420 | return true; | ||
| 2421 | }; | ||
| 2422 | const finishPick = Effect.fn("PreviewManager.finishPickElement")(function* ( | ||
| 2386 | payload: PreviewAnnotationSubmissionResult | null, | 2423 | payload: PreviewAnnotationSubmissionResult | null, |
| 2387 | ) { | 2424 | ) { |
| 2388 | const active = (yield* Ref.get(pickSessionsRef)).get(tabId); | ||
| 2389 | if (!active || active.cancel !== cancel) return; | ||
| 2390 | yield* cleanup(); | 2425 | yield* cleanup(); |
| 2391 | resume(Effect.succeed(payload)); | 2426 | resume(Effect.succeed(payload)); |
| 2392 | }); | 2427 | }); |
| 2428 | const settlePick = Effect.fn("PreviewManager.settlePickElement")(function* ( | ||
| 2429 | payload: PreviewAnnotationSubmissionResult | null, | ||
| 2430 | ) { | ||
| 2431 | if (!claimSettle()) return; | ||
| 2432 | yield* finishPick(payload); | ||
| 2433 | }); | ||
| 2393 | const settle = (payload: PreviewAnnotationSubmissionResult | null) => { | 2434 | const settle = (payload: PreviewAnnotationSubmissionResult | null) => { |
| 2394 | runFork(settlePick(payload)); | 2435 | runFork(settlePick(payload)); |
| 2395 | }; | 2436 | }; |
| 2396 | const cancelPickSession = Effect.fn("PreviewManager.cancelPickSession")(function* () { | 2437 | const cancelPickSession = Effect.fn("PreviewManager.cancelPickSession")(function* () { |
| 2438 | if (!claimSettle()) return; | ||
| 2397 | yield* cleanup(); | 2439 | yield* cleanup(); |
| 2398 | const tabs = yield* SynchronizedRef.get(tabsRef); | 2440 | const tabs = yield* SynchronizedRef.get(tabsRef); |
| 2399 | const activeTab = tabs.get(tabId); | 2441 | const activeTab = tabs.get(tabId); |
| ... | @@ -2412,7 +2454,6 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function | ... | @@ -2412,7 +2454,6 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function |
| 2412 | } | 2454 | } |
| 2413 | resume(Effect.succeed(null)); | 2455 | resume(Effect.succeed(null)); |
| 2414 | }); | 2456 | }); |
| 2415 | const cancel = cancelPickSession(); | ||
| 2416 | const onMessage = (_event: Electron.IpcMainEvent, ...args: unknown[]): void => { | 2457 | const onMessage = (_event: Electron.IpcMainEvent, ...args: unknown[]): void => { |
| 2417 | const payload = args[0]; | 2458 | const payload = args[0]; |
| 2418 | if (!isPreviewAnnotationPayload(payload)) { | 2459 | if (!isPreviewAnnotationPayload(payload)) { |
| ... | @@ -2423,19 +2464,32 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function | ... | @@ -2423,19 +2464,32 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function |
| 2423 | const submission = args[2] === "send" ? "send" : "attach"; | 2464 | const submission = args[2] === "send" ? "send" : "attach"; |
| 2424 | runFork( | 2465 | runFork( |
| 2425 | captureAnnotationScreenshot(tabId, wc, cropRect).pipe( | 2466 | captureAnnotationScreenshot(tabId, wc, cropRect).pipe( |
| 2426 | Effect.matchEffect({ | 2467 | // The renderer cannot tell a dropped crop from a comment-only |
| 2427 | onFailure: () => Effect.sync(() => settle({ annotation: payload, submission })), | 2468 | // pick by the null alone, so a failed or timed-out capture is |
| 2428 | onSuccess: (screenshot) => | 2469 | // flagged on the result. |
| 2429 | Effect.sync(() => settle({ annotation: { ...payload, screenshot }, submission })), | 2470 | Effect.match({ |
| 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 }, | ||
| 2430 | }), | 2480 | }), |
| 2431 | Effect.ensuring( | 2481 | Effect.flatMap((result) => { |
| 2432 | attempt( | 2482 | // A capture that outlives its session must not touch the |
| 2483 | // overlay: the preload tears down on the captured signal, and | ||
| 2484 | // by now it may be running a newer pick. | ||
| 2485 | if (!claimSettle()) return Effect.void; | ||
| 2486 | return attempt( | ||
| 2433 | { operation: "pickElement.captureComplete", tabId, webContentsId: wc.id }, | 2487 | { operation: "pickElement.captureComplete", tabId, webContentsId: wc.id }, |
| 2434 | () => { | 2488 | () => { |
| 2435 | if (!wc.isDestroyed()) wc.send(ANNOTATION_CAPTURED_CHANNEL); | 2489 | if (!wc.isDestroyed()) wc.send(ANNOTATION_CAPTURED_CHANNEL); |
| 2436 | }, | 2490 | }, |
| 2437 | ).pipe(Effect.ignore), | 2491 | ).pipe(Effect.ignore, Effect.andThen(finishPick(result))); |
| 2438 | ), | 2492 | }), |
| 2439 | ), | 2493 | ), |
| 2440 | ); | 2494 | ); |
| 2441 | }; | 2495 | }; |
| ... | @@ -2449,6 +2503,21 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function | ... | @@ -2449,6 +2503,21 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function |
| 2449 | if (isMainFrame) settle(null); | 2503 | if (isMainFrame) settle(null); |
| 2450 | }; | 2504 | }; |
| 2451 | const registerPickElement = Effect.fn("PreviewManager.registerPickElement")(function* () { | 2505 | const registerPickElement = Effect.fn("PreviewManager.registerPickElement")(function* () { |
| 2506 | // Two picks on one tab can overlap. Swap this session in and cancel | ||
| 2507 | // the previous holder in one step, so no third pick can slip into an | ||
| 2508 | // empty slot in between and the session we push out still resumes | ||
| 2509 | // its renderer. | ||
| 2510 | const replaced = yield* Ref.modify(pickSessionsRef, (sessions) => [ | ||
| 2511 | sessions.get(tabId) ?? null, | ||
| 2512 | replaceMap(sessions, (copy) => { | ||
| 2513 | copy.set(tabId, session); | ||
| 2514 | }), | ||
| 2515 | ]); | ||
| 2516 | if (replaced) yield* replaced.cancel; | ||
| 2517 | // A newer pick may have cancelled this session while the previous | ||
| 2518 | // one was torn down. Cleanup already ran, so attaching listeners now | ||
| 2519 | // would leak them and start an overlay nobody is waiting on. | ||
| 2520 | if (settled) return; | ||
| 2452 | yield* attempt({ operation: "pickElement.register", tabId, webContentsId: wc.id }, () => { | 2521 | yield* attempt({ operation: "pickElement.register", tabId, webContentsId: wc.id }, () => { |
| 2453 | wc.ipc.on(ELEMENT_PICKED_CHANNEL, onMessage); | 2522 | wc.ipc.on(ELEMENT_PICKED_CHANNEL, onMessage); |
| 2454 | wc.once("destroyed", onDestroyed); | 2523 | wc.once("destroyed", onDestroyed); |
| ... | @@ -2456,21 +2525,17 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function | ... | @@ -2456,21 +2525,17 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function |
| 2456 | if (!wc.isFocused()) wc.focus(); | 2525 | if (!wc.isFocused()) wc.focus(); |
| 2457 | wc.send(START_PICK_CHANNEL, annotationTheme); | 2526 | wc.send(START_PICK_CHANNEL, annotationTheme); |
| 2458 | }); | 2527 | }); |
| 2459 | yield* Ref.update(pickSessionsRef, (sessions) => | ||
| 2460 | replaceMap(sessions, (copy) => { | ||
| 2461 | copy.set(tabId, { cancel }); | ||
| 2462 | }), | ||
| 2463 | ); | ||
| 2464 | }); | 2528 | }); |
| 2465 | runFork( | 2529 | runFork( |
| 2466 | registerPickElement().pipe( | 2530 | registerPickElement().pipe( |
| 2467 | Effect.catch((error: PreviewManagerError) => { | 2531 | Effect.catch((error: PreviewManagerError) => { |
| 2532 | if (!claimSettle()) return Effect.void; | ||
| 2468 | resume(Effect.fail(error)); | 2533 | resume(Effect.fail(error)); |
| 2469 | return cleanup(); | 2534 | return cleanup(); |
| 2470 | }), | 2535 | }), |
| 2471 | ), | 2536 | ), |
| 2472 | ); | 2537 | ); |
| 2473 | return cancel; | 2538 | return session.cancel; |
| 2474 | }, | 2539 | }, |
| 2475 | ); | 2540 | ); |
| 2476 | }); | 2541 | }); |
apps/desktop/src/preview/PickPreload.ts+105-44| ... | @@ -1,4 +1,4 @@ | ... | @@ -1,4 +1,4 @@ |
| 1 | // @effect-diagnostics globalDate:off - This isolated Electron preload does not run inside an Effect runtime. | 1 | // @effect-diagnostics globalDate:off globalTimers:off - This isolated Electron preload does not run inside an Effect runtime. |
| 2 | import { ipcRenderer } from "electron"; | 2 | import { ipcRenderer } from "electron"; |
| 3 | import { getElementContext } from "react-grab/primitives"; | 3 | import { getElementContext } from "react-grab/primitives"; |
| 4 | import type { | 4 | import type { |
| ... | @@ -30,6 +30,8 @@ const Z_INDEX_OVERLAY = 2147483646; | ... | @@ -30,6 +30,8 @@ const Z_INDEX_OVERLAY = 2147483646; |
| 30 | const PRIMARY = "var(--t3-primary)"; | 30 | const PRIMARY = "var(--t3-primary)"; |
| 31 | const PRIMARY_FILL = "color-mix(in srgb, var(--t3-primary) 10%, transparent)"; | 31 | const PRIMARY_FILL = "color-mix(in srgb, var(--t3-primary) 10%, transparent)"; |
| 32 | const MAX_MARQUEE_ELEMENTS = 20; | 32 | const MAX_MARQUEE_ELEMENTS = 20; |
| 33 | /** Upper bound on one element's React context lookup during submit. */ | ||
| 34 | const ELEMENT_CONTEXT_TIMEOUT_MS = 5_000; | ||
| 33 | const CONTENT_LAYER_Z_INDEX = 1; | 35 | const CONTENT_LAYER_Z_INDEX = 1; |
| 34 | const CHROME_LAYER_Z_INDEX = 10; | 36 | const CHROME_LAYER_Z_INDEX = 10; |
| 35 | 37 | ||
| ... | @@ -279,25 +281,67 @@ function toStackFrame(frame: { | ... | @@ -279,25 +281,67 @@ function toStackFrame(frame: { |
| 279 | }; | 281 | }; |
| 280 | } | 282 | } |
| 281 | 283 | ||
| 282 | async function captureElement(element: Element): Promise<PickedElementPayload | null> { | 284 | /** |
| 285 | * Resolves to `null` instead of hanging when `promise` outlives `millis`. | ||
| 286 | * `getElementContext` walks the inspected page's React internals, and some | ||
| 287 | * pages leave it pending forever. Without a bound, the whole submit chain | ||
| 288 | * stalls and the overlay sits on "Capturing…". | ||
| 289 | */ | ||
| 290 | function withCaptureTimeout<A>(promise: Promise<A>, millis: number): Promise<A | null> { | ||
| 291 | let timer: ReturnType<typeof setTimeout> | undefined; | ||
| 292 | return Promise.race([ | ||
| 293 | promise, | ||
| 294 | new Promise<null>((resolve) => { | ||
| 295 | timer = setTimeout(() => resolve(null), millis); | ||
| 296 | }), | ||
| 297 | ]).finally(() => clearTimeout(timer)); | ||
| 298 | } | ||
| 299 | |||
| 300 | /** Truncation for the DOM-only preview used when React context is unavailable. */ | ||
| 301 | const HTML_PREVIEW_MAX_CHARS = 500; | ||
| 302 | |||
| 303 | /** | ||
| 304 | * Describes a picked element. The React context lookup can stall or throw on | ||
| 305 | * some pages, so the element is never dropped: without context it still | ||
| 306 | * carries its tag, a short HTML preview, and its rect so the crop stays on the | ||
| 307 | * pick instead of falling back to the whole viewport. | ||
| 308 | */ | ||
| 309 | async function captureElement(element: Element): Promise<PickedElementPayload> { | ||
| 310 | const base = { | ||
| 311 | pageUrl: location.href, | ||
| 312 | pageTitle: document.title?.trim() || null, | ||
| 313 | tagName: element.tagName.toLowerCase(), | ||
| 314 | pickedAt: new Date().toISOString(), | ||
| 315 | }; | ||
| 283 | try { | 316 | try { |
| 284 | const context = await getElementContext(element); | 317 | const context = await withCaptureTimeout( |
| 285 | const stack = (context.stack ?? []).map(toStackFrame); | 318 | Promise.resolve(getElementContext(element)), |
| 286 | return { | 319 | ELEMENT_CONTEXT_TIMEOUT_MS, |
| 287 | pageUrl: location.href, | 320 | ); |
| 288 | pageTitle: document.title?.trim() || null, | 321 | if (context) { |
| 289 | tagName: element.tagName.toLowerCase(), | 322 | const stack = (context.stack ?? []).map(toStackFrame); |
| 290 | selector: context.selector, | 323 | return { |
| 291 | htmlPreview: context.htmlPreview ?? "", | 324 | ...base, |
| 292 | componentName: context.componentName, | 325 | selector: context.selector, |
| 293 | source: stack[0] ?? null, | 326 | htmlPreview: context.htmlPreview ?? "", |
| 294 | stack, | 327 | componentName: context.componentName, |
| 295 | styles: context.styles ?? "", | 328 | source: stack[0] ?? null, |
| 296 | pickedAt: new Date().toISOString(), | 329 | stack, |
| 297 | }; | 330 | styles: context.styles ?? "", |
| 331 | }; | ||
| 332 | } | ||
| 298 | } catch { | 333 | } catch { |
| 299 | return null; | 334 | // Fall through to the DOM-only payload. |
| 300 | } | 335 | } |
| 336 | return { | ||
| 337 | ...base, | ||
| 338 | selector: null, | ||
| 339 | htmlPreview: element.outerHTML.slice(0, HTML_PREVIEW_MAX_CHARS), | ||
| 340 | componentName: null, | ||
| 341 | source: null, | ||
| 342 | stack: [], | ||
| 343 | styles: "", | ||
| 344 | }; | ||
| 301 | } | 345 | } |
| 302 | 346 | ||
| 303 | function createButton(label: string, title: string): HTMLButtonElement { | 347 | function createButton(label: string, title: string): HTMLButtonElement { |
| ... | @@ -1225,12 +1269,20 @@ function startAnnotation(): void { | ... | @@ -1225,12 +1269,20 @@ function startAnnotation(): void { |
| 1225 | pendingCapture = true; | 1269 | pendingCapture = true; |
| 1226 | submit.disabled = true; | 1270 | submit.disabled = true; |
| 1227 | submit.textContent = "Capturing…"; | 1271 | submit.textContent = "Capturing…"; |
| 1272 | // Snapshot everything the annotation will carry before the capture runs. | ||
| 1273 | // The element context lookup can take up to its timeout, and the user can | ||
| 1274 | // keep editing meanwhile; the annotation must describe what they submitted. | ||
| 1275 | const submittedComment = comment.value.trim(); | ||
| 1276 | const submittedRegions = [...regions]; | ||
| 1277 | const submittedStrokes = [...strokes]; | ||
| 1278 | const submittedStyleChanges = Array.from(styleChanges.values(), (change) => ({ ...change })); | ||
| 1228 | void Promise.all( | 1279 | void Promise.all( |
| 1229 | Array.from(selected.values()).map(async (target) => { | 1280 | Array.from(selected.values()).map(async (target) => { |
| 1230 | const element = await captureElement(target.element); | 1281 | const element = await captureElement(target.element); |
| 1231 | if (!element) return null; | 1282 | for (const change of submittedStyleChanges) { |
| 1232 | for (const change of styleChanges.values()) { | 1283 | if (change.targetId === target.id && element.selector !== null) { |
| 1233 | if (change.targetId === target.id) change.selector = element.selector; | 1284 | change.selector = element.selector; |
| 1285 | } | ||
| 1234 | } | 1286 | } |
| 1235 | return { | 1287 | return { |
| 1236 | id: target.id, | 1288 | id: target.id, |
| ... | @@ -1238,30 +1290,39 @@ function startAnnotation(): void { | ... | @@ -1238,30 +1290,39 @@ function startAnnotation(): void { |
| 1238 | rect: rectFromDomRect(target.element.getBoundingClientRect()), | 1290 | rect: rectFromDomRect(target.element.getBoundingClientRect()), |
| 1239 | }; | 1291 | }; |
| 1240 | }), | 1292 | }), |
| 1241 | ).then((captured) => { | 1293 | ) |
| 1242 | const elements = captured.filter((target) => target !== null); | 1294 | .then((elements) => { |
| 1243 | const annotation: PreviewAnnotationPayload = { | 1295 | // The overlay may have been cancelled or replaced while the capture |
| 1244 | id: nextId("annotation"), | 1296 | // ran. A late submit must not deliver into the next pick's listener. |
| 1245 | pageUrl: location.href, | 1297 | if (finished) return; |
| 1246 | pageTitle: document.title?.trim() || null, | 1298 | const annotation: PreviewAnnotationPayload = { |
| 1247 | comment: comment.value.trim(), | 1299 | id: nextId("annotation"), |
| 1248 | elements, | 1300 | pageUrl: location.href, |
| 1249 | regions: [...regions], | 1301 | pageTitle: document.title?.trim() || null, |
| 1250 | strokes: [...strokes], | 1302 | comment: submittedComment, |
| 1251 | styleChanges: Array.from(styleChanges.values()), | 1303 | elements, |
| 1252 | screenshot: null, | 1304 | regions: submittedRegions, |
| 1253 | createdAt: new Date().toISOString(), | 1305 | strokes: submittedStrokes, |
| 1254 | }; | 1306 | styleChanges: submittedStyleChanges, |
| 1255 | editor.style.display = "none"; | 1307 | screenshot: null, |
| 1256 | toolbar.style.display = "none"; | 1308 | createdAt: new Date().toISOString(), |
| 1257 | hoverOutline.style.display = "none"; | 1309 | }; |
| 1258 | const screenshotRect = unionRects([ | 1310 | editor.style.display = "none"; |
| 1259 | ...elements.map((target) => target.rect), | 1311 | toolbar.style.display = "none"; |
| 1260 | ...regions.map((region) => region.rect), | 1312 | hoverOutline.style.display = "none"; |
| 1261 | ...strokes.map((stroke) => stroke.bounds), | 1313 | const screenshotRect = unionRects([ |
| 1262 | ]); | 1314 | ...elements.map((target) => target.rect), |
| 1263 | ipcRenderer.send(ELEMENT_PICKED_CHANNEL, annotation, screenshotRect, submission); | 1315 | ...submittedRegions.map((region) => region.rect), |
| 1264 | }); | 1316 | ...submittedStrokes.map((stroke) => stroke.bounds), |
| 1317 | ]); | ||
| 1318 | ipcRenderer.send(ELEMENT_PICKED_CHANNEL, annotation, screenshotRect, submission); | ||
| 1319 | }) | ||
| 1320 | .catch(() => { | ||
| 1321 | // Last resort. Main is waiting on this message, so hand it an empty | ||
| 1322 | // pick rather than leaving the button stuck on "Capturing…" and the | ||
| 1323 | // renderer's pick promise pending. teardown is a no-op once finished. | ||
| 1324 | teardown(true); | ||
| 1325 | }); | ||
| 1265 | }; | 1326 | }; |
| 1266 | submit.addEventListener("click", () => submitAnnotation("attach")); | 1327 | submit.addEventListener("click", () => submitAnnotation("attach")); |
| 1267 | root.addEventListener("keydown", (event) => { | 1328 | root.addEventListener("keydown", (event) => { |
apps/web/src/components/preview/PreviewView.test.tsx+45-6| ... | @@ -27,7 +27,7 @@ const mocks = vi.hoisted(() => ({ | ... | @@ -27,7 +27,7 @@ const mocks = vi.hoisted(() => ({ |
| 27 | openPictureInPicture: vi.fn(async (_tabId: string): Promise<void> => undefined), | 27 | openPictureInPicture: vi.fn(async (_tabId: string): Promise<void> => undefined), |
| 28 | closePictureInPicture: vi.fn(async (_tabId: string): Promise<void> => undefined), | 28 | closePictureInPicture: vi.fn(async (_tabId: string): Promise<void> => undefined), |
| 29 | pickElement: vi.fn(), | 29 | pickElement: vi.fn(), |
| 30 | previewAnnotationScreenshotFile: vi.fn(), | 30 | capturePreviewAnnotationScreenshot: vi.fn(), |
| 31 | addPreviewAnnotation: vi.fn(), | 31 | addPreviewAnnotation: vi.fn(), |
| 32 | addImage: vi.fn(), | 32 | addImage: vi.fn(), |
| 33 | toggleAnnotation: null as (() => void) | null, | 33 | toggleAnnotation: null as (() => void) | null, |
| ... | @@ -90,7 +90,7 @@ vi.mock("~/composerDraftStore", () => ({ | ... | @@ -90,7 +90,7 @@ vi.mock("~/composerDraftStore", () => ({ |
| 90 | })); | 90 | })); |
| 91 | 91 | ||
| 92 | vi.mock("~/lib/previewAnnotation", () => ({ | 92 | vi.mock("~/lib/previewAnnotation", () => ({ |
| 93 | previewAnnotationScreenshotFile: mocks.previewAnnotationScreenshotFile, | 93 | capturePreviewAnnotationScreenshot: mocks.capturePreviewAnnotationScreenshot, |
| 94 | })); | 94 | })); |
| 95 | 95 | ||
| 96 | vi.mock("~/localApi", () => ({ | 96 | vi.mock("~/localApi", () => ({ |
| ... | @@ -252,6 +252,7 @@ vi.mock("~/browser/BrowserSurfaceSlot", () => ({ BrowserSurfaceSlot: () => null | ... | @@ -252,6 +252,7 @@ vi.mock("~/browser/BrowserSurfaceSlot", () => ({ BrowserSurfaceSlot: () => null |
| 252 | vi.mock("./usePreviewSession", () => ({ usePreviewSession: vi.fn() })); | 252 | vi.mock("./usePreviewSession", () => ({ usePreviewSession: vi.fn() })); |
| 253 | 253 | ||
| 254 | import { PreviewView, previewProfileName } from "./PreviewView"; | 254 | import { PreviewView, previewProfileName } from "./PreviewView"; |
| 255 | import { toastManager } from "~/components/ui/toast"; | ||
| 255 | import { previewRuntimeTabId } from "~/browser/previewRuntimeTabId"; | 256 | import { previewRuntimeTabId } from "~/browser/previewRuntimeTabId"; |
| 256 | 257 | ||
| 257 | const TEST_THREAD_REF = { | 258 | const TEST_THREAD_REF = { |
| ... | @@ -339,8 +340,10 @@ describe("PreviewView navigation", () => { | ... | @@ -339,8 +340,10 @@ describe("PreviewView navigation", () => { |
| 339 | mocks.openPictureInPicture.mockClear(); | 340 | mocks.openPictureInPicture.mockClear(); |
| 340 | mocks.closePictureInPicture.mockClear(); | 341 | mocks.closePictureInPicture.mockClear(); |
| 341 | mocks.pickElement.mockReset(); | 342 | mocks.pickElement.mockReset(); |
| 342 | mocks.previewAnnotationScreenshotFile.mockReset(); | 343 | mocks.capturePreviewAnnotationScreenshot.mockReset(); |
| 344 | mocks.capturePreviewAnnotationScreenshot.mockResolvedValue({ status: "none" }); | ||
| 343 | mocks.addPreviewAnnotation.mockClear(); | 345 | mocks.addPreviewAnnotation.mockClear(); |
| 346 | vi.mocked(toastManager.add).mockClear(); | ||
| 344 | mocks.addImage.mockClear(); | 347 | mocks.addImage.mockClear(); |
| 345 | mocks.toggleAnnotation = null; | 348 | mocks.toggleAnnotation = null; |
| 346 | mocks.pictureInPicture = false; | 349 | mocks.pictureInPicture = false; |
| ... | @@ -551,7 +554,39 @@ describe("PreviewView navigation", () => { | ... | @@ -551,7 +554,39 @@ describe("PreviewView navigation", () => { |
| 551 | expect(mocks.addPreviewAnnotation).toHaveBeenCalledWith(TEST_THREAD_REF, annotation); | 554 | expect(mocks.addPreviewAnnotation).toHaveBeenCalledWith(TEST_THREAD_REF, annotation); |
| 552 | }); | 555 | }); |
| 553 | 556 | ||
| 554 | it("still sends when screenshot attachment conversion fails", async () => { | 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 | |||
| 589 | it("still sends when the picked element's crop cannot be captured", async () => { | ||
| 555 | const annotation = { | 590 | const annotation = { |
| 556 | id: "annotation-2", | 591 | id: "annotation-2", |
| 557 | pageUrl: "https://example.com/dashboard", | 592 | pageUrl: "https://example.com/dashboard", |
| ... | @@ -571,7 +606,7 @@ describe("PreviewView navigation", () => { | ... | @@ -571,7 +606,7 @@ describe("PreviewView navigation", () => { |
| 571 | }; | 606 | }; |
| 572 | const onSendAnnotation = vi.fn(); | 607 | const onSendAnnotation = vi.fn(); |
| 573 | mocks.pickElement.mockResolvedValue({ annotation, submission: "send" }); | 608 | mocks.pickElement.mockResolvedValue({ annotation, submission: "send" }); |
| 574 | mocks.previewAnnotationScreenshotFile.mockRejectedValue(new Error("conversion failed")); | 609 | mocks.capturePreviewAnnotationScreenshot.mockResolvedValue({ status: "failed" }); |
| 575 | 610 | ||
| 576 | renderToStaticMarkup( | 611 | renderToStaticMarkup( |
| 577 | <PreviewView | 612 | <PreviewView |
| ... | @@ -583,7 +618,11 @@ describe("PreviewView navigation", () => { | ... | @@ -583,7 +618,11 @@ describe("PreviewView navigation", () => { |
| 583 | ); | 618 | ); |
| 584 | mocks.toggleAnnotation?.(); | 619 | mocks.toggleAnnotation?.(); |
| 585 | 620 | ||
| 586 | await vi.waitFor(() => expect(onSendAnnotation).toHaveBeenCalledWith(annotation, null)); | 621 | // The forwarded and stored annotation both drop the screenshot, so the |
| 622 | // prompt does not claim a crop that was never attached. | ||
| 623 | const sent = { ...annotation, screenshot: null }; | ||
| 624 | await vi.waitFor(() => expect(onSendAnnotation).toHaveBeenCalledWith(sent, null)); | ||
| 625 | expect(mocks.addPreviewAnnotation).toHaveBeenCalledWith(TEST_THREAD_REF, sent); | ||
| 587 | expect(mocks.addImage).not.toHaveBeenCalled(); | 626 | expect(mocks.addImage).not.toHaveBeenCalled(); |
| 588 | }); | 627 | }); |
| 589 | }); | 628 | }); |
apps/web/src/components/preview/PreviewView.tsx+23-8| ... | @@ -20,7 +20,7 @@ import { | ... | @@ -20,7 +20,7 @@ import { |
| 20 | useThreadRecentHistory, | 20 | useThreadRecentHistory, |
| 21 | } from "~/browserHistoryStore"; | 21 | } from "~/browserHistoryStore"; |
| 22 | import { type ComposerImageAttachment, useComposerDraftStore } from "~/composerDraftStore"; | 22 | import { type ComposerImageAttachment, useComposerDraftStore } from "~/composerDraftStore"; |
| 23 | import { previewAnnotationScreenshotFile } from "~/lib/previewAnnotation"; | 23 | import { capturePreviewAnnotationScreenshot } from "~/lib/previewAnnotation"; |
| 24 | import { ensureLocalApi } from "~/localApi"; | 24 | import { ensureLocalApi } from "~/localApi"; |
| 25 | import { | 25 | import { |
| 26 | rememberPreviewUrl, | 26 | rememberPreviewUrl, |
| ... | @@ -579,15 +579,30 @@ export function PreviewView({ | ... | @@ -579,15 +579,30 @@ export function PreviewView({ |
| 579 | try { | 579 | try { |
| 580 | const result = await previewBridge.pickElement(runtimeTabId); | 580 | const result = await previewBridge.pickElement(runtimeTabId); |
| 581 | if (!result) return; | 581 | if (!result) return; |
| 582 | const { annotation, submission } = result; | 582 | const { annotation: picked, submission, screenshotFailed = false } = result; |
| 583 | // The structured annotation is still sendable when its optional crop | ||
| 584 | // stalls or fails, so tell the user what they lost and keep going | ||
| 585 | // instead of holding the composer for an attachment that never lands. | ||
| 586 | // The stored copy drops the screenshot on failure, otherwise the prompt | ||
| 587 | // would tell the agent a crop is attached when none was sent. | ||
| 588 | 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"; | ||
| 592 | const annotation = capture.status === "failed" ? { ...picked, screenshot: null } : picked; | ||
| 583 | addPreviewAnnotation(threadRef, annotation); | 593 | addPreviewAnnotation(threadRef, annotation); |
| 584 | let screenshotFile: File | null = null; | 594 | if (cropDropped) { |
| 585 | try { | 595 | toastManager.add( |
| 586 | screenshotFile = await previewAnnotationScreenshotFile(annotation); | 596 | stackedThreadToast({ |
| 587 | } catch { | 597 | type: "error", |
| 588 | // The structured annotation is still sendable when converting its | 598 | title: "Could not capture the picked element", |
| 589 | // optional screenshot into a composer attachment fails. | 599 | // The send path reports its own outcome, so only say what this |
| 600 | // handler knows: the crop was dropped. | ||
| 601 | description: "The annotation was kept without the screenshot.", | ||
| 602 | }), | ||
| 603 | ); | ||
| 590 | } | 604 | } |
| 605 | const screenshotFile = capture.status === "captured" ? capture.file : null; | ||
| 591 | const image = | 606 | const image = |
| 592 | screenshotFile && annotation.screenshot | 607 | screenshotFile && annotation.screenshot |
| 593 | ? ({ | 608 | ? ({ |
apps/web/src/lib/previewAnnotation.test.ts+35-1| ... | @@ -1,9 +1,10 @@ | ... | @@ -1,9 +1,10 @@ |
| 1 | import type { PreviewAnnotationPayload } from "@t3tools/contracts"; | 1 | import type { PreviewAnnotationPayload } from "@t3tools/contracts"; |
| 2 | import { describe, expect, it } from "vite-plus/test"; | 2 | import { afterEach, describe, expect, it, vi } from "vite-plus/test"; |
| 3 | 3 | ||
| 4 | import { | 4 | import { |
| 5 | appendPreviewAnnotationPrompt, | 5 | appendPreviewAnnotationPrompt, |
| 6 | buildPreviewAnnotationPrompt, | 6 | buildPreviewAnnotationPrompt, |
| 7 | capturePreviewAnnotationScreenshot, | ||
| 7 | extractTrailingPreviewAnnotation, | 8 | extractTrailingPreviewAnnotation, |
| 8 | } from "./previewAnnotation"; | 9 | } from "./previewAnnotation"; |
| 9 | 10 | ||
| ... | @@ -85,3 +86,36 @@ describe("preview annotations", () => { | ... | @@ -85,3 +86,36 @@ describe("preview annotations", () => { |
| 85 | expect(extractedFirst.promptText).toBe("Fix this"); | 86 | expect(extractedFirst.promptText).toBe("Fix this"); |
| 86 | }); | 87 | }); |
| 87 | }); | 88 | }); |
| 89 | |||
| 90 | describe("preview annotation capture", () => { | ||
| 91 | afterEach(() => { | ||
| 92 | vi.useRealTimers(); | ||
| 93 | vi.unstubAllGlobals(); | ||
| 94 | }); | ||
| 95 | |||
| 96 | it("returns the crop when the fetch resolves", async () => { | ||
| 97 | vi.stubGlobal("fetch", async () => new Response(new Blob(["png"], { type: "image/png" }))); | ||
| 98 | const capture = await capturePreviewAnnotationScreenshot(annotation); | ||
| 99 | expect(capture.status).toBe("captured"); | ||
| 100 | }); | ||
| 101 | |||
| 102 | it("reports none when the annotation carries no crop", async () => { | ||
| 103 | const capture = await capturePreviewAnnotationScreenshot({ ...annotation, screenshot: null }); | ||
| 104 | expect(capture).toEqual({ status: "none" }); | ||
| 105 | }); | ||
| 106 | |||
| 107 | it("fails instead of hanging when the crop never arrives", async () => { | ||
| 108 | vi.useFakeTimers(); | ||
| 109 | vi.stubGlobal("fetch", () => new Promise<Response>(() => {})); | ||
| 110 | const capturePromise = capturePreviewAnnotationScreenshot(annotation, 1_000); | ||
| 111 | await vi.advanceTimersByTimeAsync(1_000); | ||
| 112 | expect(await capturePromise).toEqual({ status: "failed" }); | ||
| 113 | }); | ||
| 114 | |||
| 115 | it("fails when the crop fetch throws", async () => { | ||
| 116 | vi.stubGlobal("fetch", async () => { | ||
| 117 | throw new Error("data url unreadable"); | ||
| 118 | }); | ||
| 119 | expect(await capturePreviewAnnotationScreenshot(annotation)).toEqual({ status: "failed" }); | ||
| 120 | }); | ||
| 121 | }); |
apps/web/src/lib/previewAnnotation.ts+38-1| ... | @@ -99,7 +99,7 @@ export function extractTrailingPreviewAnnotation(prompt: string): ExtractedPrevi | ... | @@ -99,7 +99,7 @@ export function extractTrailingPreviewAnnotation(prompt: string): ExtractedPrevi |
| 99 | }; | 99 | }; |
| 100 | } | 100 | } |
| 101 | 101 | ||
| 102 | export async function previewAnnotationScreenshotFile( | 102 | async function previewAnnotationScreenshotFile( |
| 103 | annotation: PreviewAnnotationPayload, | 103 | annotation: PreviewAnnotationPayload, |
| 104 | ): Promise<File | null> { | 104 | ): Promise<File | null> { |
| 105 | if (!annotation.screenshot) return null; | 105 | if (!annotation.screenshot) return null; |
| ... | @@ -109,3 +109,40 @@ export async function previewAnnotationScreenshotFile( | ... | @@ -109,3 +109,40 @@ export async function previewAnnotationScreenshotFile( |
| 109 | type: blob.type || "image/png", | 109 | type: blob.type || "image/png", |
| 110 | }); | 110 | }); |
| 111 | } | 111 | } |
| 112 | |||
| 113 | /** Upper bound on turning a picked element's crop into a composer attachment. */ | ||
| 114 | export const PREVIEW_ANNOTATION_CAPTURE_TIMEOUT_MS = 5_000; | ||
| 115 | |||
| 116 | export type PreviewAnnotationCapture = | ||
| 117 | /** The crop is ready to attach. */ | ||
| 118 | | { readonly status: "captured"; readonly file: File } | ||
| 119 | /** The pick carried no crop, which is normal for comment-only annotations. */ | ||
| 120 | | { readonly status: "none" } | ||
| 121 | /** The crop stalled or threw. Send the annotation without it. */ | ||
| 122 | | { readonly status: "failed" }; | ||
| 123 | |||
| 124 | /** | ||
| 125 | * Bounded wrapper around `previewAnnotationScreenshotFile`. The picker holds the | ||
| 126 | * composer while this runs, so it must always settle: a stalled crop resolves as | ||
| 127 | * `failed` instead of leaving the caller waiting. | ||
| 128 | */ | ||
| 129 | export async function capturePreviewAnnotationScreenshot( | ||
| 130 | annotation: PreviewAnnotationPayload, | ||
| 131 | timeoutMs: number = PREVIEW_ANNOTATION_CAPTURE_TIMEOUT_MS, | ||
| 132 | ): Promise<PreviewAnnotationCapture> { | ||
| 133 | if (!annotation.screenshot) return { status: "none" }; | ||
| 134 | let timer: ReturnType<typeof setTimeout> | undefined; | ||
| 135 | try { | ||
| 136 | const file = await Promise.race([ | ||
| 137 | previewAnnotationScreenshotFile(annotation), | ||
| 138 | new Promise<null>((resolve) => { | ||
| 139 | timer = setTimeout(() => resolve(null), timeoutMs); | ||
| 140 | }), | ||
| 141 | ]); | ||
| 142 | return file ? { status: "captured", file } : { status: "failed" }; | ||
| 143 | } catch { | ||
| 144 | return { status: "failed" }; | ||
| 145 | } finally { | ||
| 146 | clearTimeout(timer); | ||
| 147 | } | ||
| 148 | } |
packages/contracts/src/ipc.ts+3| ... | @@ -934,11 +934,14 @@ export const PreviewAnnotationSubmissionSchema: Schema.Codec<PreviewAnnotationSu | ... | @@ -934,11 +934,14 @@ export const PreviewAnnotationSubmissionSchema: Schema.Codec<PreviewAnnotationSu |
| 934 | export interface PreviewAnnotationSubmissionResult { | 934 | export interface PreviewAnnotationSubmissionResult { |
| 935 | annotation: PreviewAnnotationPayload; | 935 | annotation: PreviewAnnotationPayload; |
| 936 | submission: PreviewAnnotationSubmission; | 936 | submission: PreviewAnnotationSubmission; |
| 937 | /** The crop was requested but failed or timed out, so `annotation.screenshot` is null. */ | ||
| 938 | screenshotFailed?: boolean; | ||
| 937 | } | 939 | } |
| 938 | export const PreviewAnnotationSubmissionResultSchema: Schema.Codec<PreviewAnnotationSubmissionResult> = | 940 | export const PreviewAnnotationSubmissionResultSchema: Schema.Codec<PreviewAnnotationSubmissionResult> = |
| 939 | Schema.Struct({ | 941 | Schema.Struct({ |
| 940 | annotation: PreviewAnnotationPayloadSchema, | 942 | annotation: PreviewAnnotationPayloadSchema, |
| 941 | submission: PreviewAnnotationSubmissionSchema, | 943 | submission: PreviewAnnotationSubmissionSchema, |
| 944 | screenshotFailed: Schema.optionalKey(Schema.Boolean), | ||
| 942 | }); | 945 | }); |
| 943 | 946 | ||
| 944 | export const DesktopPreviewTabInputSchema = Schema.Struct({ | 947 | export const DesktopPreviewTabInputSchema = Schema.Struct({ |