From 61d5aac4b21bdd241fa22ccd9ff0bbfab6a9af51 Mon Sep 17 00:00:00 2001 From: jackroberts236 <163729227+jackroberts236@users.noreply.github.com> Date: Wed, 7 Oct 2026 22:54:28 +0100 Subject: [PATCH 1/2] fix(desktop): prevent preview annotation upload collisions --- apps/desktop/src/preview/Manager.test.ts | 27 ++++++++++++++++++------ apps/desktop/src/preview/Manager.ts | 10 ++++++--- 2 files changed, 28 insertions(+), 9 deletions(-) diff --git a/apps/desktop/src/preview/Manager.test.ts b/apps/desktop/src/preview/Manager.test.ts index 11ee2d83e481..7c1692ba01f8 100644 --- a/apps/desktop/src/preview/Manager.test.ts +++ b/apps/desktop/src/preview/Manager.test.ts @@ -3937,7 +3937,7 @@ describe("PreviewManager", () => { ), ); - effectIt.effect("keeps annotation send grants isolated and updates an active pick", () => + effectIt.effect("keeps annotation IDs and send grants isolated across tabs", () => withManager((manager) => Effect.gen(function* () { const picked = new Map void>(); @@ -3985,6 +3985,8 @@ describe("PreviewManager", () => { screenshot: null, createdAt: "2026-09-05T00:00:00.000Z", }; + // Guest counters restart after navigation and collide across tabs. + const annotationIds = new Set(); for (const [tabId, id, nextGrant, expected] of [ ["tab_b", 43, undefined, "attach"], ["tab_a", 42, false, "attach"], @@ -3997,21 +3999,29 @@ describe("PreviewManager", () => { } picked.get(id)?.({}, annotation, null, "send"); const result = yield* Fiber.join(pick); - expect(result?.annotation.id).toBe(annotation.id); + expect(result).not.toBeNull(); + expect(annotationIds.has(result!.annotation.id)).toBe(false); + annotationIds.add(result!.annotation.id); + expect(result?.annotation.comment).toBe(annotation.comment); expect(result?.submission).toBe(expected); } yield* manager.registerWebview("tab_a", 44); const replacementPick = yield* manager.pickElement("tab_a").pipe(Effect.forkChild); yield* Effect.yieldNow; picked.get(44)?.({}, annotation, null, "send"); - expect((yield* Fiber.join(replacementPick))?.submission).toBe("send"); + const replacementResult = yield* Fiber.join(replacementPick); + expect(replacementResult?.submission).toBe("send"); + expect(annotationIds.has(replacementResult!.annotation.id)).toBe(false); + annotationIds.add(replacementResult!.annotation.id); yield* manager.closeTab("tab_a"); yield* manager.createTab("tab_a"); yield* manager.registerWebview("tab_a", 44); const reopenedPick = yield* manager.pickElement("tab_a").pipe(Effect.forkChild); yield* Effect.yieldNow; picked.get(44)?.({}, annotation, null, "send"); - expect((yield* Fiber.join(reopenedPick))?.submission).toBe("attach"); + const reopenedResult = yield* Fiber.join(reopenedPick); + expect(reopenedResult?.submission).toBe("attach"); + expect(annotationIds.has(reopenedResult!.annotation.id)).toBe(false); }), ), ); @@ -4168,10 +4178,15 @@ describe("PreviewManager", () => { expect(webviewSend).not.toHaveBeenCalledWith("preview:annotation-captured"); expect(secondPick.pollUnsafe()).toBeUndefined(); - onPicked?.({}, { ...annotation, id: "annotation_2" }, null, "attach"); + onPicked?.( + {}, + { ...annotation, id: "annotation_2", comment: "Second pick" }, + null, + "attach", + ); yield* TestClock.adjust("6 seconds"); const result = yield* Fiber.join(secondPick); - expect(result?.annotation.id).toBe("annotation_2"); + expect(result?.annotation.comment).toBe("Second pick"); expect(result?.submission).toBe("attach"); }), ), diff --git a/apps/desktop/src/preview/Manager.ts b/apps/desktop/src/preview/Manager.ts index f98694145b67..638bb2216b2f 100644 --- a/apps/desktop/src/preview/Manager.ts +++ b/apps/desktop/src/preview/Manager.ts @@ -2159,6 +2159,9 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function const wc = yield* requireWebContents(tabId); yield* cancelPickElement(tabId); const annotationTheme = yield* Ref.get(annotationThemeRef); + // Guest counters restart on navigation and collide across tabs. Annotation + // IDs also key draft screenshots and their cached uploads in the renderer. + const annotationId = yield* crypto.randomUUIDv4.pipe(Effect.orDie); return yield* Effect.callback( (resume) => { // Declared first so cleanup can check slot ownership by identity @@ -2231,6 +2234,7 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function settle(null); return; } + const annotation = { ...payload, id: annotationId }; const cropRect = normalizeCaptureRect(args[1]); const submission = args[2] === "send" && annotationSendEnabled.get(tabId) === true ? "send" : "attach"; @@ -2241,14 +2245,14 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function // flagged on the result. Effect.match({ onFailure: (): PreviewAnnotationSubmissionResult => ({ - annotation: payload, + annotation, submission, screenshotFailed: true, }), onSuccess: (screenshot): PreviewAnnotationSubmissionResult => screenshot === null - ? { annotation: payload, submission, screenshotFailed: true } - : { annotation: { ...payload, screenshot }, submission }, + ? { annotation, submission, screenshotFailed: true } + : { annotation: { ...annotation, screenshot }, submission }, }), Effect.flatMap((result) => { // A capture that outlives its session must not touch the From 1edc9025b78ace2757ba55a7a6f3bb8a1de19453 Mon Sep 17 00:00:00 2001 From: jackroberts236 <163729227+jackroberts236@users.noreply.github.com> Date: Wed, 7 Oct 2026 23:57:00 +0100 Subject: [PATCH 2/2] test(desktop): verify annotation IDs after capture timeout --- apps/desktop/src/preview/Manager.test.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/apps/desktop/src/preview/Manager.test.ts b/apps/desktop/src/preview/Manager.test.ts index 7c1692ba01f8..be7240c3da88 100644 --- a/apps/desktop/src/preview/Manager.test.ts +++ b/apps/desktop/src/preview/Manager.test.ts @@ -4188,6 +4188,8 @@ describe("PreviewManager", () => { const result = yield* Fiber.join(secondPick); expect(result?.annotation.comment).toBe("Second pick"); expect(result?.submission).toBe("attach"); + expect(result?.annotation.id).not.toBe(annotation.id); + expect(result?.annotation.id).not.toBe("annotation_2"); }), ), );