From 4ba6dc36c71f5a7e3db80fd4c18311b9c7ffb8dc Mon Sep 17 00:00:00 2001 From: Logan Johnson Date: Tue, 18 Aug 2026 20:44:24 +0000 Subject: [PATCH] fix(desktop): stabilize workstream PR subscriptions Signed-off-by: loganj Co-authored-by: Amp --- .../lib/workstreamPullRequestStatus.test.mjs | 45 +++++++++++++++++++ .../lib/workstreamPullRequestStatus.ts | 21 +++++++-- 2 files changed, 63 insertions(+), 3 deletions(-) diff --git a/desktop/src/features/workstream-board/lib/workstreamPullRequestStatus.test.mjs b/desktop/src/features/workstream-board/lib/workstreamPullRequestStatus.test.mjs index ec8c6e2f6f..67e0265928 100644 --- a/desktop/src/features/workstream-board/lib/workstreamPullRequestStatus.test.mjs +++ b/desktop/src/features/workstream-board/lib/workstreamPullRequestStatus.test.mjs @@ -236,6 +236,51 @@ test("workstream status hook preserves a synchronous live callback", async () => cleanup(); }); +test("workstream status hook keeps subscriptions stable across equivalent rerenders", async () => { + const { act, cleanup, renderHook } = await import("@testing-library/react"); + const { useWorkstreamPullRequestStatuses } = await import( + "./workstreamPullRequestStatus.ts" + ); + const reference = parseWorkstreamPullRequestReference( + "https://github.com/block/buzz/pull/42", + ); + let subscribes = 0; + let unsubscribes = 0; + const registry = { + subscribe: (_reference, listener) => { + subscribes += 1; + listener({ status: "loading" }); + return () => { + unsubscribes += 1; + }; + }, + }; + const { rerender, unmount } = renderHook( + ({ refs }) => useWorkstreamPullRequestStatuses(refs, registry), + { initialProps: { refs: [reference] } }, + ); + + await act(async () => { + rerender({ refs: [{ ...reference }] }); + rerender({ refs: [{ ...reference }] }); + }); + assert.equal(subscribes, 1); + assert.equal(unsubscribes, 0); + + const replacement = parseWorkstreamPullRequestReference( + "https://github.com/block/buzz/pull/43", + ); + await act(async () => { + rerender({ refs: [replacement] }); + }); + assert.equal(subscribes, 2); + assert.equal(unsubscribes, 1); + + unmount(); + assert.equal(unsubscribes, 2); + cleanup(); +}); + test("workstream status hook removes stale references and preserves unsupported state", async () => { const { act, cleanup, renderHook } = await import("@testing-library/react"); const { useWorkstreamPullRequestStatuses } = await import( diff --git a/desktop/src/features/workstream-board/lib/workstreamPullRequestStatus.ts b/desktop/src/features/workstream-board/lib/workstreamPullRequestStatus.ts index ed5e66b7a3..2d88617f58 100644 --- a/desktop/src/features/workstream-board/lib/workstreamPullRequestStatus.ts +++ b/desktop/src/features/workstream-board/lib/workstreamPullRequestStatus.ts @@ -519,11 +519,26 @@ export function useWorkstreamPullRequestStatuses( const [states, setStates] = React.useState< ReadonlyMap >(() => new Map()); - const referencesByIdentity = React.useMemo(() => { + // Cards rebuild parsed references on every render. Subscribe by their + // semantic identity instead of that transient array/object identity so a + // synchronous registry listener cannot cause an effect resubscribe loop. + const referenceIdentityKey = references + .map((reference) => reference.identity) + .sort() + .join("\u0000"); + const referencesByIdentityRef = React.useRef<{ + identityKey: string; + references: ReadonlyMap; + } | null>(null); + if (referencesByIdentityRef.current?.identityKey !== referenceIdentityKey) { const next = new Map(); for (const reference of references) next.set(reference.identity, reference); - return next; - }, [references]); + referencesByIdentityRef.current = { + identityKey: referenceIdentityKey, + references: next, + }; + } + const referencesByIdentity = referencesByIdentityRef.current.references; React.useEffect(() => { const initial = new Map();