From c8048a0c8a4e0528542b697fa5fcc67ea0c45e89 Mon Sep 17 00:00:00 2001 From: Bohan Jiang <52446949+Bohan-J@users.noreply.github.com> Date: Wed, 19 Aug 2026 16:25:18 +0800 Subject: [PATCH] fix(issues): stop a comment create failing on sibling issue caches (MUL-6394) (#7211) `onIssueAuxiliaryRevision` scans issue caches by key PREFIX, but every prefix also covers siblings with a different shape: `tableAll` holds the grouped (infinite) and facet caches next to the row pages, `myAll` holds the assignee-grouped caches. Reading `data.rows` / `data.byStatus` off those threw "Cannot read properties of undefined (reading 'some')". `useCreateComment` calls it from onSuccess, so the throw rejected `mutateAsync` after the server had already created the comment and enqueued the agent task: the composer showed an error toast and skipped the timeline append, and the comment only appeared on reload. Every scan now goes through the shape-filtered helpers the cache coordinator already had (`bucketedListEntries` / `flatListEntries` / `tableRowEntries`, plus a new `issueArrayEntries`), so the check can't be forgotten at a new call site. Co-authored-by: J Co-authored-by: multica-agent --- packages/core/issues/cache-coordinator.ts | 30 ++++- packages/core/issues/mutations.test.tsx | 74 ++++++++++++ .../core/issues/revision-realtime.test.ts | 99 ++++++++++++++++ packages/core/issues/ws-updaters.ts | 109 +++++++----------- 4 files changed, 240 insertions(+), 72 deletions(-) diff --git a/packages/core/issues/cache-coordinator.ts b/packages/core/issues/cache-coordinator.ts index bf7f6f515..40c266977 100644 --- a/packages/core/issues/cache-coordinator.ts +++ b/packages/core/issues/cache-coordinator.ts @@ -122,7 +122,20 @@ function listContractFromKey(key: QueryKey): { }; } -function bucketedListEntries( +/** + * SHAPE-FILTERED CACHE SCANS. + * + * `getQueriesData` matches a key PREFIX, and every issue-surface prefix also + * covers sibling queries that hold a different shape: `myAll` covers the + * assignee-grouped caches, `tableAll` covers the grouped (infinite) and facet + * caches next to the row pages, `flatAll` covers the export window. Reading + * `data.rows` / `data.pages` / `data.byStatus` off those siblings throws + * ("Cannot read properties of undefined"), and inside a mutation's onSuccess + * that throw surfaces as a failed write the server already accepted + * (MUL-6394). Every scan goes through these helpers so the shape check can't + * be forgotten at a new call site. + */ +export function bucketedListEntries( qc: QueryClient, wsId: string, ): [QueryKey, ListIssuesCache][] { @@ -134,7 +147,7 @@ function bucketedListEntries( ); } -function flatListEntries( +export function flatListEntries( qc: QueryClient, wsId: string, ): [QueryKey, IssueFlatCache][] { @@ -146,7 +159,7 @@ function flatListEntries( ); } -function tableRowEntries( +export function tableRowEntries( qc: QueryClient, wsId: string, ): [QueryKey, IssueTableRowCache][] { @@ -160,6 +173,17 @@ function tableRowEntries( ); } +/** Caches under `prefix` that hold a plain `Issue[]` — per-parent children and + * the project Gantt list. */ +export function issueArrayEntries( + qc: QueryClient, + prefix: readonly unknown[], +): [QueryKey, Issue[]][] { + return qc + .getQueriesData({ queryKey: prefix }) + .filter((entry): entry is [QueryKey, Issue[]] => Array.isArray(entry[1])); +} + function flatContractFromKey(key: QueryKey): { scope: string | undefined; filter: IssueFlatFilter; diff --git a/packages/core/issues/mutations.test.tsx b/packages/core/issues/mutations.test.tsx index c36aa0524..28a466c8b 100644 --- a/packages/core/issues/mutations.test.tsx +++ b/packages/core/issues/mutations.test.tsx @@ -10,6 +10,7 @@ import { setApiInstance } from "../api"; import type { ApiClient } from "../api/client"; import { useBatchUpdateIssues, + useCreateComment, useDeleteComment, useResolveComment, useUpdateComment, @@ -948,3 +949,76 @@ describe("useResolveComment", () => { expect(resolvedIds(qc)).toEqual(["a1"]); }); }); + +// MUL-6394: posting a comment while the Table view's grouped/facet caches are +// loaded rejected `mutateAsync` with "Cannot read properties of undefined +// (reading 'some')" — the comment WAS created server-side (the agent task +// started), but the composer showed an error toast and never appended the +// entry, so it only appeared after a reload. +describe("useCreateComment — sibling caches under a shared key prefix", () => { + const ISSUE_ID = "issue-1"; + const tableQuery = { + scope: { kind: "workspace" }, + filters: {}, + sort: { field: "position", direction: "asc" }, + } as const; + + let qc: QueryClient; + + beforeEach(() => { + qc = new QueryClient({ defaultOptions: { queries: { retry: false } } }); + setApiInstance({ + createComment: vi.fn().mockResolvedValue({ + id: "comment-1", + issue_id: ISSUE_ID, + author_type: "member", + author_id: "user-1", + content: "hello", + type: "comment", + parent_id: null, + reactions: [], + attachments: [], + created_at: "2026-08-19T00:00:00Z", + updated_at: "2026-08-19T00:00:00Z", + resolved_at: null, + resolved_by_type: null, + resolved_by_id: null, + issue_revision: 7, + }), + } as unknown as ApiClient); + }); + + afterEach(() => { + qc.clear(); + vi.restoreAllMocks(); + }); + + it("appends the created comment even when non-row table caches are loaded", async () => { + qc.setQueryData(issueKeys.timeline(ISSUE_ID), []); + // Grouped rows are an infinite cache and facets a plain object — both live + // under the `table-query` prefix next to the row pages, and neither has a + // `rows` array. + qc.setQueryData(issueKeys.tableGroups(WS_ID, tableQuery, { kind: "status" }), { + pages: [ + { query_fingerprint: "sha256:groups", total: 0, groups: [], next_cursor: null }, + ], + pageParams: [null], + }); + qc.setQueryData( + issueKeys.tableFacets(WS_ID, { query: tableQuery, facets: [{ kind: "status" }] }), + { query_fingerprint: "sha256:facets", total: 0, facets: [] }, + ); + + const { result } = renderHook(() => useCreateComment(ISSUE_ID), { + wrapper: createWrapper(qc), + }); + + await act(async () => { + await result.current.mutateAsync({ content: "hello" }); + }); + + expect( + qc.getQueryData(issueKeys.timeline(ISSUE_ID))?.map((e) => e.id), + ).toEqual(["comment-1"]); + }); +}); diff --git a/packages/core/issues/revision-realtime.test.ts b/packages/core/issues/revision-realtime.test.ts index db238917f..35dedd2f9 100644 --- a/packages/core/issues/revision-realtime.test.ts +++ b/packages/core/issues/revision-realtime.test.ts @@ -220,3 +220,102 @@ describe("issue realtime revision admission", () => { ).toBe(true); }); }); + +// MUL-6394: `getQueriesData` matches a key PREFIX, so a scan over `tableAll` / +// `myAll` also sees sibling caches with a different shape. Reading `rows` / +// `byStatus` off them threw "Cannot read properties of undefined (reading +// 'some')" out of `useCreateComment`'s onSuccess — the comment was created, +// the UI reported a failure. +describe("auxiliary revision — sibling caches under a shared key prefix", () => { + const query = { + scope: { kind: "workspace" }, + filters: {}, + sort: { field: "position", direction: "asc" }, + } as const; + + const rowPageKey = [ + ...issueKeys.tableRows( + "ws-1", + query, + { kind: "none" }, + null, + false, + null, + ), + "page", + null, + ] as const; + + function seedSiblingCaches(qc: QueryClient) { + // Grouped rows are an infinite cache, facets a plain object; neither has + // a `rows` array, and both sit under the `table-query` prefix. + qc.setQueryData(issueKeys.tableGroups("ws-1", query, { kind: "status" }), { + pages: [ + { + query_fingerprint: "sha256:groups", + total: 1, + groups: [], + next_cursor: null, + }, + ], + pageParams: [null], + }); + qc.setQueryData( + issueKeys.tableFacets("ws-1", { query, facets: [{ kind: "status" }] }), + { query_fingerprint: "sha256:facets", total: 1, facets: [] }, + ); + // The assignee-grouped caches share the `my` prefix with the bucketed + // ones but carry no `byStatus`. + qc.setQueryData(issueKeys.myAssigneeGroups("ws-1", "assigned", {}), { + groups: [], + }); + } + + it("skips them instead of throwing, and still invalidates stale row pages", () => { + const qc = new QueryClient(); + seedSiblingCaches(qc); + qc.setQueryData(rowPageKey, { + query_fingerprint: "sha256:rows", + group_key: null, + parent_id: null, + total: 1, + rows: [{ issue: issue(2, "stale row"), direct_child_count: 0 }], + branch_total: 1, + next_cursor: null, + }); + + expect(() => + onIssueAuxiliaryRevision(qc, "ws-1", "issue-1", 3), + ).not.toThrow(); + + expect(qc.getQueryState(rowPageKey)?.isInvalidated).toBe(true); + expect( + qc.getQueryState( + issueKeys.tableGroups("ws-1", query, { kind: "status" }), + )?.isInvalidated, + ).toBe(false); + expect( + qc.getQueryState( + issueKeys.myAssigneeGroups("ws-1", "assigned", {}), + )?.isInvalidated, + ).toBe(false); + }); + + it("leaves an up-to-date row page alone", () => { + const qc = new QueryClient(); + seedSiblingCaches(qc); + qc.setQueryData(rowPageKey, { + query_fingerprint: "sha256:rows", + group_key: null, + parent_id: null, + total: 1, + rows: [{ issue: issue(3, "current row"), direct_child_count: 0 }], + branch_total: 1, + next_cursor: null, + }); + + onIssueAuxiliaryRevision(qc, "ws-1", "issue-1", 3); + + expect(qc.getQueryState(rowPageKey)?.isInvalidated).toBe(false); + }); +}); diff --git a/packages/core/issues/ws-updaters.ts b/packages/core/issues/ws-updaters.ts index 3b3107ac7..d5fc0f400 100644 --- a/packages/core/issues/ws-updaters.ts +++ b/packages/core/issues/ws-updaters.ts @@ -5,10 +5,14 @@ import { labelKeys } from "../labels/queries"; import { projectKeys } from "../projects/queries"; import { applyIssueChange, + bucketedListEntries, + flatListEntries, invalidateIssueDerivatives, invalidateLastActivitySortedIssueLists, invalidateStaleListKeys, invalidateUpdatedAtSortedIssueLists, + issueArrayEntries, + tableRowEntries, type IssueFlatCache, } from "./cache-coordinator"; import { @@ -128,10 +132,7 @@ function patchIssueInFlatCaches( patch: Partial, orderRevision?: number, ) { - for (const [key, data] of qc.getQueriesData({ - queryKey: issueKeys.flatAll(wsId), - })) { - if (!data?.pages) continue; + for (const [key, data] of flatListEntries(qc, wsId)) { qc.setQueryData(key, { ...data, pages: data.pages.map((page) => ({ @@ -154,10 +155,8 @@ function patchIssueInChildrenCaches( patch: Partial, orderRevision?: number, ) { - for (const [key, data] of qc.getQueriesData({ - queryKey: issueKeys.childrenAll(wsId), - })) { - if (!data || !data.some((child) => child.id === issueId)) continue; + for (const [key, data] of issueArrayEntries(qc, issueKeys.childrenAll(wsId))) { + if (!data.some((child) => child.id === issueId)) continue; qc.setQueryData( key, data.map((child) => @@ -176,17 +175,7 @@ function patchIssueInTableCaches( patch: Partial, orderRevision?: number, ) { - for (const [key, data] of qc.getQueriesData({ - queryKey: issueKeys.tableAll(wsId), - })) { - if ( - !data || - typeof data !== "object" || - !Array.isArray((data as IssueTableRowsResponse).rows) - ) { - continue; - } - const page = data as IssueTableRowsResponse; + for (const [key, page] of tableRowEntries(qc, wsId)) { if (!page.rows.some((row) => row.issue.id === issueId)) continue; qc.setQueryData(key, { ...page, @@ -209,18 +198,15 @@ function patchIssueSnapshot( patch: Partial, orderRevision?: number, ) { - for (const prefix of [issueKeys.list(wsId), issueKeys.myAll(wsId)]) { - for (const [key, data] of qc.getQueriesData({ queryKey: prefix })) { - if (!data) continue; - const location = findIssueLocation(data, issueId); - if ( - !location || - mergeIssuePatch(location.issue, patch, orderRevision) === location.issue - ) { - continue; - } - qc.setQueryData(key, patchIssueInBuckets(data, issueId, patch)); + for (const [key, data] of bucketedListEntries(qc, wsId)) { + const location = findIssueLocation(data, issueId); + if ( + !location || + mergeIssuePatch(location.issue, patch, orderRevision) === location.issue + ) { + continue; } + qc.setQueryData(key, patchIssueInBuckets(data, issueId, patch)); } patchIssueInFlatCaches(qc, wsId, issueId, patch, orderRevision); patchIssueInTableCaches(qc, wsId, issueId, patch, orderRevision); @@ -228,10 +214,10 @@ function patchIssueSnapshot( qc.setQueryData(issueKeys.detail(wsId, issueId), (old) => old ? mergeIssuePatch(old, patch, orderRevision) : old, ); - for (const [key, data] of qc.getQueriesData({ - queryKey: issueKeys.projectGanttAll(wsId), - })) { - if (!data) continue; + for (const [key, data] of issueArrayEntries( + qc, + issueKeys.projectGanttAll(wsId), + )) { qc.setQueryData( key, data.map((issue) => @@ -254,27 +240,20 @@ function freshestCachedIssueRevision( freshest = issue.revision; } }; - for (const prefix of [issueKeys.list(wsId), issueKeys.myAll(wsId)]) { - for (const [, data] of qc.getQueriesData({ queryKey: prefix })) { - consider(data ? findIssueLocation(data, issueId)?.issue : undefined); - } + for (const [, data] of bucketedListEntries(qc, wsId)) { + consider(findIssueLocation(data, issueId)?.issue); } - for (const [, data] of qc.getQueriesData({ - queryKey: issueKeys.flatAll(wsId), - })) { - for (const page of data?.pages ?? []) { + for (const [, data] of flatListEntries(qc, wsId)) { + for (const page of data.pages) { consider(page.issues.find((issue) => issue.id === issueId)); } } - for (const [, data] of qc.getQueriesData({ - queryKey: issueKeys.tableAll(wsId), - })) { - if (!data || !Array.isArray(data.rows)) continue; + for (const [, data] of tableRowEntries(qc, wsId)) { consider(data.rows.find((row) => row.issue.id === issueId)?.issue); } for (const prefix of [issueKeys.childrenAll(wsId), issueKeys.projectGanttAll(wsId)]) { - for (const [, data] of qc.getQueriesData({ queryKey: prefix })) { - consider(data?.find((issue) => issue.id === issueId)); + for (const [, data] of issueArrayEntries(qc, prefix)) { + consider(data.find((issue) => issue.id === issueId)); } } return freshest; @@ -307,30 +286,24 @@ function invalidateIssueOwnerProjectionsWhere( const detailKey = issueKeys.detail(wsId, issueId); if (shouldInvalidate(qc.getQueryData(detailKey))) invalidateExact(detailKey); - for (const prefix of [issueKeys.list(wsId), issueKeys.myAll(wsId)]) { - for (const [key, data] of qc.getQueriesData({ queryKey: prefix })) { - if (shouldInvalidate(data ? findIssueLocation(data, issueId)?.issue : undefined)) { - invalidateExact(key); - } - } - } - for (const [key, data] of qc.getQueriesData({ - queryKey: issueKeys.flatAll(wsId), - })) { - if (data?.pages.some((page) => shouldInvalidate(page.issues.find((issue) => issue.id === issueId)))) { + for (const [key, data] of bucketedListEntries(qc, wsId)) { + if (shouldInvalidate(findIssueLocation(data, issueId)?.issue)) { invalidateExact(key); } } - for (const [key, data] of qc.getQueriesData({ - queryKey: issueKeys.tableAll(wsId), - })) { - if (data?.rows.some((row) => row.issue.id === issueId && shouldInvalidate(row.issue))) { + for (const [key, data] of flatListEntries(qc, wsId)) { + if (data.pages.some((page) => shouldInvalidate(page.issues.find((issue) => issue.id === issueId)))) { + invalidateExact(key); + } + } + for (const [key, data] of tableRowEntries(qc, wsId)) { + if (data.rows.some((row) => row.issue.id === issueId && shouldInvalidate(row.issue))) { invalidateExact(key); } } for (const prefix of [issueKeys.childrenAll(wsId), issueKeys.projectGanttAll(wsId)]) { - for (const [key, data] of qc.getQueriesData({ queryKey: prefix })) { - if (data?.some((issue) => issue.id === issueId && shouldInvalidate(issue))) { + for (const [key, data] of issueArrayEntries(qc, prefix)) { + if (data.some((issue) => issue.id === issueId && shouldInvalidate(issue))) { invalidateExact(key); } } @@ -394,10 +367,8 @@ function findIssueInFlatCaches( wsId: string, issueId: string, ) { - for (const [, data] of qc.getQueriesData({ - queryKey: issueKeys.flatAll(wsId), - })) { - for (const page of data?.pages ?? []) { + for (const [, data] of flatListEntries(qc, wsId)) { + for (const page of data.pages) { const issue = page.issues.find((candidate) => candidate.id === issueId); if (issue) return issue; }