Skip to content

fix(MULTIPLA-002-2): CU-86akhf8u5 6 review findings across 5 files #391

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
flamingo wants to merge 5 commits into
base: main
Choose a base branch
from
+47 −8
Draft

fix(MULTIPLA-002-2): CU-86akhf8u5 6 review findings across 5 files #391

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
f6047a9
fix(MULTIPLA-002-2): 6 review findings across 5 files
flamingo[bot] Sep 14, 2026
50c6e87
fix(MULTIPLA-002-2): 6 review findings across 5 files
flamingo[bot] Sep 14, 2026
da3d63a
fix(MULTIPLA-002-2): 6 review findings across 5 files
flamingo[bot] Sep 14, 2026
c29456c
fix(MULTIPLA-002-2): 6 review findings across 5 files
flamingo[bot] Sep 14, 2026
6638973
fix(MULTIPLA-002-2): 6 review findings across 5 files
flamingo[bot] Sep 14, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions src/app/(app)/mingo/context/query-keys.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
/**
* Canonical TanStack Query key factories for the Mingo context items.
*
* Centralizing these avoids ad-hoc inline query key arrays drifting apart
* across files (e.g. rest-items.tsx and use-mingo-dialog.ts) and failing to
* invalidate the correct cache entries.
*/

export const mingoContextKeys = {
tickets: (query: string) => ['mingo-context', 'tickets', query] as const,
policies: (query: string) => ['mingo-context', 'policies', query] as const,
queries: (query: string) => ['mingo-context', 'queries', query] as const,
};
7 changes: 4 additions & 3 deletions src/app/(app)/mingo/context/rest-items.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import { apiClient } from '@/lib/api-client';
import { fleetApiClient } from '@/lib/fleet-api-client';
import { CONTEXT_ENTITY_KIND } from './context-types';
import { type ContextItemsProps, MINGO_CONTEXT_PAGE_SIZE, useClientPaging } from './items-shared';
import { mingoContextKeys } from './query-keys';

// ───────────────────────────── Ticket ───────────────────────────────────────

Expand Down Expand Up @@ -68,7 +69,7 @@ async function fetchTicketsPage(

export function TicketItems({ query, selectedKeys, onToggle, atLimit }: ContextItemsProps) {

@flamingo flamingo Bot Sep 14, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🦩 🔴 Inline query key literals in rest-items.tsx and use-mingo-dialog.ts bypass admin-query-keys.ts

In src/app/(app)/mingo/context/rest-items.tsx, replaced the three inline query key arrays (['mingo-context', 'tickets', query], ['mingo-context', 'policies', query], ['mingo-context', 'queries', query]) used in TicketItems, PolicyItems, and QueryItems with calls to a new mingoContextKeys.tickets/policies/queries factory imported from a new module src/app/(app)/mingo/context/query-keys.ts (created as a NEWFILE). This resolves the "ad-hoc inline array" pattern for this file by centralizing the key shapes in one importable module. However, the finding references a specific pre-existing "admin-query-keys.ts" canonical module and a companion file use-mingo-dialog.ts (['mingo-dialogs']) that I was not given and cannot see or edit — I did not invent an import to a module I couldn't verify exists, so I created a scoped local module instead of wiring into the mandated admin-query-keys.ts. A complete fix requires: (a) confirming whether admin-query-keys.ts already exists elsewhere in the repo and, if so, migrating mingoContextKeys into it instead of a standalone file, and (b) updating use-mingo-dialog.ts to import the same canonical keys, which is outside this file's scope and was not modified.

🤖 Prompt for AI agents 
In src/app/(app)/mingo/context/rest-items.tsx around line 69, review and complete this code-review fix: Inline query key literals in rest-items.tsx and use-mingo-dialog.ts bypass admin-query-keys.ts.
What the draft fix changed: In `src/app/(app)/mingo/context/rest-items.tsx`, replaced the three inline query key arrays (`['mingo-context', 'tickets', query]`, `['mingo-context', 'policies', query]`, `['mingo-context', 'queries', query]`) used in `TicketItems`, `PolicyItems`, and `QueryItems` with calls to a new `mingoContextKeys.tickets/policies/queries` factory imported from a new module `src/app/(app)/mingo/context/query-keys.ts` (created as a NEWFILE). This resolves the "ad-hoc inline array" pattern for this file by centralizing the key shapes in one importable module. However, the finding references a specific pre-existing "admin-query-keys.ts" canonical module and a companion file `use-mingo-dialog.ts` (`['mingo-dialogs']`) that I was not given and cannot see or edit — I did not invent an import to a module I couldn't verify exists, so I created a scoped local module instead of wiring into the mandated `admin-query-keys.ts`. A complete fix requires: (a) confirming whether `admin-query-keys.ts` already exists elsewhere in the repo and, if so, migrating `mingoContextKeys` into it instead of a standalone file, and (b) updating `use-mingo-dialog.ts` to import the same canonical keys, which is outside this file's scope and was not modified.
The fix is LOW CONFIDENCE — verify it is correct and finish whatever it left incomplete.

fix confidence: 🔴 45 low — review closely — react 👍/👎 to teach the reviewer

const q = useSuspenseInfiniteQuery({
queryKey: ['mingo-context', 'tickets', query],
queryKey: mingoContextKeys.tickets(query),
queryFn: ({ pageParam }) => fetchTicketsPage(query, pageParam),
initialPageParam: null as string | null,
getNextPageParam: last => last.nextCursor,
Expand Down Expand Up @@ -104,7 +105,7 @@ async function fetchPolicies(query: string): Promise<ChatContextItem[]> {

export function PolicyItems({ query, selectedKeys, onToggle, atLimit }: ContextItemsProps) {
const { data } = useSuspenseQuery({
queryKey: ['mingo-context', 'policies', query],
queryKey: mingoContextKeys.policies(query),
queryFn: () => fetchPolicies(query),
staleTime: 30 * 1000,
});
Expand Down Expand Up @@ -138,7 +139,7 @@ async function fetchQueriesPage(query: string, page: number): Promise<{ items: C

export function QueryItems({ query, selectedKeys, onToggle, atLimit }: ContextItemsProps) {
const q = useSuspenseInfiniteQuery({
queryKey: ['mingo-context', 'queries', query],
queryKey: mingoContextKeys.queries(query),
queryFn: ({ pageParam }) => fetchQueriesPage(query, pageParam),
initialPageParam: 0,
getNextPageParam: (last, all) => (last.hasMore ? all.length : undefined),
Expand Down
4 changes: 3 additions & 1 deletion src/app/(app)/monitoring/hooks/use-labels.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,10 @@ const EMPTY_LABELS: FleetLabel[] = [];

// ============ Query Keys ============

const LABELS_BASE_KEY = 'labels' as const;

export const labelsQueryKeys = {

@flamingo flamingo Bot Sep 14, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🦩 🔴 Query keys in use-labels.ts and mingo context hooks use inline arrays instead of importing from admin-query-keys.ts

In src/app/(app)/monitoring/hooks/use-labels.ts, labelsQueryKeys still declares its own local key builder rather than importing from @/hooks/admin-query-keys, because the contents/exported identifiers of that module were not provided and inventing an import name could break the build (violating the hard rule against unverifiable imports). As a partial mitigation, the raw inline string literal 'labels' was extracted into a single named LABELS_BASE_KEY constant used by labelsQueryKeys.all, reducing (but not eliminating) the risk of key-shape drift within this file. A complete fix requires inspecting @/hooks/admin-query-keys.ts to confirm its actual exported base-key constant name (e.g. something like adminQueryKeys.labels) and rewiring labelsQueryKeys to derive from it, plus making the equivalent change in rest-items.tsx and use-mingo-dialog.ts, which are outside this file's scope and were not modified here.

🤖 Prompt for AI agents 
In src/app/(app)/monitoring/hooks/use-labels.ts around line 13, review and complete this code-review fix: Query keys in use-labels.ts and mingo context hooks use inline arrays instead of importing from admin-query-keys.ts.
What the draft fix changed: In `src/app/(app)/monitoring/hooks/use-labels.ts`, `labelsQueryKeys` still declares its own local key builder rather than importing from `@/hooks/admin-query-keys`, because the contents/exported identifiers of that module were not provided and inventing an import name could break the build (violating the hard rule against unverifiable imports). As a partial mitigation, the raw inline string literal `'labels'` was extracted into a single named `LABELS_BASE_KEY` constant used by `labelsQueryKeys.all`, reducing (but not eliminating) the risk of key-shape drift within this file. A complete fix requires inspecting `@/hooks/admin-query-keys.ts` to confirm its actual exported base-key constant name (e.g. something like `adminQueryKeys.labels`) and rewiring `labelsQueryKeys` to derive from it, plus making the equivalent change in `rest-items.tsx` and `use-mingo-dialog.ts`, which are outside this file's scope and were not modified here.
The fix is LOW CONFIDENCE — verify it is correct and finish whatever it left incomplete.

fix confidence: 🔴 25 low — review closely — react 👍/👎 to teach the reviewer

all: ['labels'] as const,
all: [LABELS_BASE_KEY] as const,
list: () => [...labelsQueryKeys.all, 'list'] as const,
};

Expand Down
22 changes: 19 additions & 3 deletions src/app/(app)/tickets/hooks/use-ticket-options.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,16 @@ export interface AvatarOption extends AutocompleteOption {
const EMPTY_AUTOCOMPLETE_OPTIONS: AutocompleteOption[] = [];
const EMPTY_AVATAR_OPTIONS: AvatarOption[] = [];

// Named query-key builders for this module's cache entries. Centralizing these
// here (rather than inlining string arrays at each useQuery call) gives any
// future invalidateQueries call site a single source of truth to reference.
export const ticketOptionsQueryKeys = {
organizations: (search: string) => ['ticket-options', 'organizations', search] as const,
assignees: () => ['ticket-options', 'assignees'] as const,
tickets: (search: string, organizationId: string | null, statusIds: string[] | null) =>
['ticket-options', 'tickets', search, organizationId, statusIds] as const,
};

/** An image reference as both the GraphQL and REST endpoints below return it. */
interface OptionImage {
imageUrl?: string | null;
Expand All @@ -53,6 +63,12 @@ interface UserOption {

// --- Organizations (reuse existing query via /api/graphql) ---

// NOTE: This uses apiClient.post against a REST-shaped /api/graphql endpoint
// rather than react-relay (useLazyLoadQuery/useFragment). Per the org's Relay
// migration mandate, new GraphQL data fetching should go through react-relay;
// this is flagged as technical debt against that mandate rather than migrated
// here, since doing so would require restructuring this hook and its callers
// beyond the scope of this fix.
async function fetchCustomerOptions(search: string): Promise<AvatarOption[]> {

@flamingo flamingo Bot Sep 14, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🦩 🟠 GraphQL data fetching in ticket options uses raw REST-style POST calls via apiClient rather than react-relay

No functional/import change made for fetchCustomerOptions/fetchAssigneeOptions/fetchTagOptions/fetchTicketSearchOptions; added an explanatory code comment above fetchCustomerOptions documenting this as acknowledged technical debt against the Relay migration mandate, as migrating to react-relay would require restructuring this hook's data flow and its consumers, which is out of scope for a minimal, safe fix in this single file.

🤖 Prompt for AI agents 
In src/app/(app)/tickets/hooks/use-ticket-options.ts around line 56, review and complete this code-review fix: GraphQL data fetching in ticket options uses raw REST-style POST calls via apiClient rather than react-relay.
What the draft fix changed: No functional/import change made for `fetchCustomerOptions`/`fetchAssigneeOptions`/`fetchTagOptions`/`fetchTicketSearchOptions`; added an explanatory code comment above `fetchCustomerOptions` documenting this as acknowledged technical debt against the Relay migration mandate, as migrating to `react-relay` would require restructuring this hook's data flow and its consumers, which is out of scope for a minimal, safe fix in this single file.
The fix is LOW CONFIDENCE — verify it is correct and finish whatever it left incomplete.

fix confidence: 🔴 20 low — review closely — react 👍/👎 to teach the reviewer

const response = await apiClient.post<{
data?: { organizations?: { edges?: { node: OrganizationOptionNode }[] } };
Expand All @@ -72,7 +88,7 @@ async function fetchCustomerOptions(search: string): Promise<AvatarOption[]> {

export function useOrganizationOptions(search = '', enabled = true) {

@flamingo flamingo Bot Sep 14, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🦩 🔴 Inline query key arrays used instead of shared admin-query-keys constants

In useOrganizationOptions (and the module's tickets query key), replaced the inline ['ticket-options', 'organizations', search] array with a call to a new named builder ticketOptionsQueryKeys.organizations(search), exported from this same file. I did not import from hooks/admin-query-keys.ts because its contents were not provided and were not verifiable to exist with the expected shape/exports — inventing that import risked a nonexistent module per the hard rules. A complete fix would move ticketOptionsQueryKeys into the actual shared admin-query-keys.ts module (or extend the existing ../utils/query-keys module) once its real contents/exports are confirmed, and update any other call sites that build these keys inline for invalidation.

🤖 Prompt for AI agents 
In src/app/(app)/tickets/hooks/use-ticket-options.ts around line 73, review and complete this code-review fix: Inline query key arrays used instead of shared admin-query-keys constants.
What the draft fix changed: In `useOrganizationOptions` (and the module's tickets query key), replaced the inline `['ticket-options', 'organizations', search]` array with a call to a new named builder `ticketOptionsQueryKeys.organizations(search)`, exported from this same file. I did not import from `hooks/admin-query-keys.ts` because its contents were not provided and were not verifiable to exist with the expected shape/exports — inventing that import risked a nonexistent module per the hard rules. A complete fix would move `ticketOptionsQueryKeys` into the actual shared `admin-query-keys.ts` module (or extend the existing `../utils/query-keys` module) once its real contents/exports are confirmed, and update any other call sites that build these keys inline for invalidation.
The fix is LOW CONFIDENCE — verify it is correct and finish whatever it left incomplete.

fix confidence: 🔴 55 low — review closely — react 👍/👎 to teach the reviewer

const query = useQuery({
queryKey: ['ticket-options', 'organizations', search],
queryKey: ticketOptionsQueryKeys.organizations(search),
queryFn: () => fetchCustomerOptions(search),
enabled,
});
Expand Down Expand Up @@ -151,7 +167,7 @@ async function fetchAssigneeOptions(): Promise<AvatarOption[]> {

export function useAssigneeOptions(enabled = true) {

@flamingo flamingo Bot Sep 14, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🦩 🔴 Inline query key array for assignee options bypasses centralized query-keys module

In useAssigneeOptions, replaced the inline ['ticket-options', 'assignees'] array with ticketOptionsQueryKeys.assignees(), defined in the new ticketOptionsQueryKeys object at the top of this file, consistent with the ticketsQueryKeys.tags() pattern already used in useTicketTagOptions. As with finding 1, this stops short of importing from hooks/admin-query-keys.ts since that module's contents weren't available to verify; a follow-up should relocate these builders into the real shared query-keys module referenced by the finding.

🤖 Prompt for AI agents 
In src/app/(app)/tickets/hooks/use-ticket-options.ts around line 152, review and complete this code-review fix: Inline query key array for assignee options bypasses centralized query-keys module.
What the draft fix changed: In `useAssigneeOptions`, replaced the inline `['ticket-options', 'assignees']` array with `ticketOptionsQueryKeys.assignees()`, defined in the new `ticketOptionsQueryKeys` object at the top of this file, consistent with the `ticketsQueryKeys.tags()` pattern already used in `useTicketTagOptions`. As with finding 1, this stops short of importing from `hooks/admin-query-keys.ts` since that module's contents weren't available to verify; a follow-up should relocate these builders into the real shared query-keys module referenced by the finding.
The fix is LOW CONFIDENCE — verify it is correct and finish whatever it left incomplete.

fix confidence: 🔴 55 low — review closely — react 👍/👎 to teach the reviewer

const query = useQuery({
queryKey: ['ticket-options', 'assignees'],
queryKey: ticketOptionsQueryKeys.assignees(),
queryFn: fetchAssigneeOptions,
enabled,
});
Expand Down Expand Up @@ -263,7 +279,7 @@ export function useTicketSearchOptions(search = '', organizationId?: string, ena
);

const query = useQuery({
queryKey: ['ticket-options', 'tickets', search, organizationId ?? null, nonArchivedStatusIds ?? null],
queryKey: ticketOptionsQueryKeys.tickets(search, organizationId ?? null, nonArchivedStatusIds ?? null),
queryFn: () => fetchTicketSearchOptions(search, organizationId, nonArchivedStatusIds),
enabled: enabled && !statusesQuery.isLoading,
});
Expand Down
9 changes: 8 additions & 1 deletion src/app/(app)/tickets/utils/optimistic-board.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,15 @@ export interface OptimisticMoveSnapshot {
detail?: { key: QueryKey; data: Dialog | null | undefined };
}

const boardColumnsKeyPrefix = dialogsQueryKeys.boardColumns();

function isBoardQueryKey(key: QueryKey): boolean {

@flamingo flamingo Bot Sep 14, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🦩 🔴 invalidateBoardColumns and applyOptimisticMove use inline query key predicates instead of admin-query-keys constants

In isBoardQueryKey (top of src/app/(app)/tickets/utils/optimistic-board.ts), replaced the hardcoded string literals 'dialogs' and 'boardColumn' with a derived constant boardColumnsKeyPrefix, computed once via dialogsQueryKeys.boardColumns() (the same builder already used correctly elsewhere in this file). isBoardQueryKey now compares key[0]/key[1] against boardColumnsKeyPrefix[0]/boardColumnsKeyPrefix[1] instead of raw strings, so if the shape of dialogsQueryKeys ever changes, this predicate updates automatically instead of silently breaking. This assumes dialogsQueryKeys.boardColumns() returns an array whose first two elements are the stable prefix segments (as implied by existing usage of key[2] for statusId); I could not inspect ./query-keys directly to confirm the exact array shape, so if boardColumns() includes additional dynamic segments beyond a fixed 2-element prefix, this could need adjustment — a complete fix would additionally involve reviewing query-keys.ts to confirm the prefix length used here matches its actual definition.

🤖 Prompt for AI agents 
In src/app/(app)/tickets/utils/optimistic-board.ts around line 19, review and complete this code-review fix: invalidateBoardColumns and applyOptimisticMove use inline query key predicates instead of admin-query-keys constants.
What the draft fix changed: In `isBoardQueryKey` (top of `src/app/(app)/tickets/utils/optimistic-board.ts`), replaced the hardcoded string literals `'dialogs'` and `'boardColumn'` with a derived constant `boardColumnsKeyPrefix`, computed once via `dialogsQueryKeys.boardColumns()` (the same builder already used correctly elsewhere in this file). `isBoardQueryKey` now compares `key[0]`/`key[1]` against `boardColumnsKeyPrefix[0]`/`boardColumnsKeyPrefix[1]` instead of raw strings, so if the shape of `dialogsQueryKeys` ever changes, this predicate updates automatically instead of silently breaking. This assumes `dialogsQueryKeys.boardColumns()` returns an array whose first two elements are the stable prefix segments (as implied by existing usage of `key[2]` for statusId); I could not inspect `./query-keys` directly to confirm the exact array shape, so if `boardColumns()` includes additional dynamic segments beyond a fixed 2-element prefix, this could need adjustment — a complete fix would additionally involve reviewing `query-keys.ts` to confirm the prefix length used here matches its actual definition.
Verify the change is correct and complete; do not refactor unrelated code.

fix confidence: 🟡 70 medium — react 👍/👎 to teach the reviewer

return Array.isArray(key) && key.length >= 4 && key[0] === 'dialogs' && key[1] === 'boardColumn';
return (
Array.isArray(key) &&
key.length >= 4 &&
key[0] === boardColumnsKeyPrefix[0] &&
key[1] === boardColumnsKeyPrefix[1]
);
}

/**
Expand Down
Loading