fix(cmdk): Filter out async resource actions with 0 results#112613
Merged
fix(cmdk): Filter out async resource actions with 0 results#112613
Conversation
…gistration Introduces a makeCollection<T>() factory in ui/collection.tsx that creates isolated, fully-typed collection instances. Each instance tracks a flat node map and a parent→children index, exposing a tree(rootKey?) API for structured traversal. A node becomes a group by wrapping its children in GroupContext.Provider — there is no separate group/item type distinction. Data fields are spread directly onto tree nodes so consumers can access them without any cast. ui/cmdk.tsx builds the CMDK-specific layer on top: CMDKCollection, CMDKGroup, and CMDKAction. Groups and actions share a single CMDKActionData union type. CMDK_PLAN.md tracks the remaining migration steps from the old reducer-based registration system. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Wire CMDKQueryContext and CMDKProvider so async CMDKGroup nodes can read
the current search query. Mount CMDKProvider inside CommandPaletteStateProvider
in the existing CommandPaletteProvider so the collection store is live for the
full app lifetime.
Add GlobalCommandPaletteActions component that registers all global actions
into the CMDK collection via JSX (CMDKGroup / CMDKAction) rather than the old
useCommandPaletteActionsRegister reducer hook. Both systems run in parallel
during this transition step.
Swap the CommandPalette UI to read from CMDKCollection.useStore() instead of
the old useCommandPaletteActions() reducer pipeline. Remove collectResourceActions,
asyncQueries, and asyncChildrenMap — async fetching is now handled inside
CMDKGroup before nodes appear in the tree. Rewrite scoreTree and flattenActions
to work with CollectionTreeNode<CMDKActionData> using direct field access.
Replace the linked-list navigation stack (which stored full action objects)
with CMDKNavStack which stores only the group key and label. Push action now
dispatches {key, label} instead of a full CommandPaletteActionWithKey.
Update modal.tsx, stories, analytics hook, and tests to use the new types.
Fix pre-existing duplicate Item declaration in commandPalette.tsx that caused
a Babel parse error in the test suite.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Remove CMDK_PLAN.md (internal planning doc, not production code) - Rename poc.spec.tsx → collection.spec.tsx (no longer a PoC) - Remove dead useGlobalCommandPaletteActions() call from navigation — commandPalette.tsx now reads from CMDKCollection.useStore(), so the old context registration had no effect - Rewrite stories demo to use CMDKAction/CMDKGroup JSX API; the old useCommandPaletteActionsRegister path fed CommandPaletteActionsContext which nothing reads anymore, so the demo was showing an empty palette Co-Authored-By: Claude Sonnet 4 <noreply@anthropic.com>
GlobalCommandPaletteActions covers everything the old hook registered. The hook had no remaining callers after the previous commit removed the navigation/index.tsx call site. Also cleans up cmdk.tsx comments and renames GroupContext to Context in the collection factory API. Co-Authored-By: Claude Sonnet 4 <noreply@anthropic.com>
The old context.tsx served as a registration hub for the reducer-based action system. With that system removed, it only wrapped two providers. Move CommandPaletteProvider directly into cmdk.tsx alongside the other palette primitives and delete the now-empty file. Also remove unused WithKey type variants from types.tsx. Co-Authored-By: Claude Sonnet 4 <noreply@anthropic.com>
Move GlobalCommandPaletteActions from globalActions.tsx into the ui/ directory alongside the other palette files, and render it inside the modal rather than in the navigation layout. The global actions now mount and unmount with the palette itself instead of running in the navigation tree at all times. CommandPalette stays a generic rendering component with no app-specific dependencies, keeping it fully testable in isolation. Co-Authored-By: Claude Sonnet 4 <noreply@anthropic.com>
Add a children prop so callers can pass CMDKGroup/CMDKAction trees directly to CommandPalette. The modal uses this to inject GlobalCommandPaletteActions, keeping the component itself free of app-specific dependencies. Co-Authored-By: Claude Sonnet 4 <noreply@anthropic.com>
CMDKAction now accepts LocationDescriptor for the to prop, so String() is no longer needed and was triggering a no-base-to-string lint error. Co-Authored-By: Claude Sonnet 4 <noreply@anthropic.com>
…onContext The modal renders outside SecondaryNavigationContextProvider, so useSecondaryNavigation() threw on open. Read the context directly and skip the sidebar toggle action when the context isn't available. Co-Authored-By: Claude Sonnet 4 <noreply@anthropic.com>
Moving it into the modal changed its React scope — the modal renders in a portal outside SecondaryNavigationContextProvider, causing a crash. Since CommandPaletteProvider is at the app root, CMDKCollection is shared across the whole tree. GlobalCommandPaletteActions can register from the navigation (where it has proper context) and the modal reads the same store. The children prop on CommandPalette is still useful for page-scoped actions. Co-Authored-By: Claude Sonnet 4 <noreply@anthropic.com>
…ing providers Add a test asserting that page slot actions appear before global actions in the command palette list. The test currently fails — it documents the known bug where collection insertion order (driven by React mount order) determines list position instead of slot priority. Also fixes two pre-existing issues uncovered while writing the test: - GlobalActionsComponent in the spec was missing CommandPaletteSlot.Provider, causing all existing tests to throw 'SlotContext not found' at render time. - GlobalCommandPaletteActions was registering actions directly into the CMDKCollection without going through a slot consumer. It now wraps its output in <CommandPaletteSlot name="global">, making all action registration slot-aware and setting up the path toward DOM-order-based priority sorting. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Documents the approach for fixing slot ordering in the command palette: store a ref to the outlet DOM element on each registered action node, then pre-sort the collection output via compareDocumentPosition before passing to flattenActions. Covers the ref-threading chain (shared SlotRefsContext → outlet populates → consumer wrapper injects → node stores), the new commandPaletteSlotRefs.tsx file needed to avoid circular imports, and all six files that need changing. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Pre-sort the root-level CMDK collection nodes by the DOM position of their slot outlet element before flattening, so that task > page > global priority is preserved regardless of React mount order. The slot library is extended with two additions: - SlotConsumer now provides OutletNameContext so portaled children see which slot they belong to (previously only the outlet provided it) - useSlotOutletRef() hook on SlotModule returns a stable RefObject whose .current is always the current slot's outlet element CommandPaletteSlot is extracted to commandPaletteSlot.tsx to avoid a circular import between cmdk.tsx (which needs the hook) and commandPalette.tsx (which imports the collection). CMDKAction and CMDKGroup call CommandPaletteSlot.useSlotOutletRef() and store the result as ref in their node data. presortBySlotRef in commandPalette.tsx then uses compareDocumentPosition on those elements to establish the correct order. Nodes outside any slot wrapper (ref is null) sort after all slotted nodes and preserve their relative order. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Extend CMDKActionData with a ref field that stores the slot outlet element. CMDKAction and CMDKGroup call useSlotOutletRef and include it in their registered node data. commandPalette.tsx applies presortBySlotRef to collection nodes before flattening, ordering by the outlet DOM position (task > page > global). Callers use CommandPaletteSlot directly — no wrapper components needed. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
CMDKGroup was overwriting any enabled field returned by the resource function with a static `!!resource` check. This meant the DSN lookup query always fired regardless of whether the query matched a DSN pattern. Fix CMDKGroup to respect the resource's own enabled field, then add `enabled: DSN_PATTERN.test(query)` to the DSN lookup resource so the API call only fires when the query looks like a DSN. Also remove an unused GlobalCommandPaletteActions import in navigation/index.tsx. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace the useCommandPaletteActions hook example with the new JSX component API using CMDKAction, CMDKGroup, and CommandPaletteProvider. Also adds a section documenting the async resource pattern. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…onent (#112563) ## Changes Merges `CMDKGroup` and `CMDKAction` into a single `CMDKAction` component. The distinction between the two was artificial — a node becomes a group simply by having children registered under it, which is already documented in the `CMDKActionData` type comment. Keeping them separate contradicted that stated design intent. The merged `CMDKAction` covers all four cases: | Usage | How | |-------|-----| | Navigation | `<CMDKAction display={...} to="/path/" />` | | Callback | `<CMDKAction display={...} onAction={fn} />` | | Group with children | `<CMDKAction display={...}><CMDKAction .../></CMDKAction>` | | Async resource group | `<CMDKAction display={...} resource={queryFn}>{data => ...}</CMDKAction>` | `CMDKActionDataTo.to` is widened from `string` to `LocationDescriptor` to match `CommandPaletteAsyncResult` and the existing callsites. Co-authored-by: Claude Sonnet 4 <noreply@anthropic.com>
Two small fixes for the JSX-powered command palette. **Empty groups are now omitted from the list.** A `CMDKGroup` that renders no children ends up with zero registered nodes in the collection. In browse mode, `flattenActions` was treating such nodes as leaf actions and showing them. These are headless section headers with nothing to show — they should be invisible. The fix skips any node that has no children and no executable action (`to` / `onAction`). **Scroll resets to the top when the query changes.** Typing a new search query now scrolls the results list back to the top. The tricky part: `ResultsList` is not the actual scroll element — `ListBox` creates its own inner `div` that it hands to TanStack Virtual as the scroll container. The fix adds a `scrollContainerRef` prop to `ListBox` that gets merged into that inner container's refs, then wires it up in the `onChange` handler. Stacked on #112262. Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
When a CMDKAction has both an onAction callback and children, selecting it now invokes the callback immediately and keeps the modal open so the user can choose a secondary action from the children. Previously, actions with children were treated purely as navigation groups — the onAction callback was silently ignored. Now the callback fires before the palette pushes into the child list. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The modal was calling the imported closeModal() from actionCreators/modal
directly, which goes straight to ModalStore.closeModal() and bypasses
GlobalModal's internal closeModal wrapper. That wrapper is the only place
options.onClose is invoked — which is the closeCommandPaletteModal callback
that dispatches {type: 'toggle modal'} to reset state.open.
With state.open stuck at true after an action closed the palette, the next
hotkey press would see open===true and treat it as a close request rather
than an open request, requiring a second press to actually reopen.
Fix by destructuring closeModal from ModalRenderProps and using that
instead of the imported version.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Instead of joining label, details, and keywords into a single string and running fzf once, score each field independently and return the best match. This has two benefits: - Avoids false cross-field subsequence matches that could arise when the tail of one field and the head of the next happen to satisfy a pattern across the join boundary. - Allows fzf's built-in exact-match bonus to fire naturally (e.g. when the query equals the label exactly), without needing a manual boost at the call site. Co-Authored-By: Claude Sonnet 4 <noreply@anthropic.com>
…coercion - Restore admin/superuser action group (Open _admin, Open org in _admin, Open Superuser Modal, Exit Superuser) for staff users; these existed in the old useGlobalCommandPaletteActions hook but were not ported to the new GlobalCommandPaletteActions component - Add missing IconList to DSN_ICONS so the third getDsnNavTargets result (Client Keys/DSN) renders with an icon - Remove String(action.to) coercion in modal.tsx and the test helper; both normalizeUrl and navigate already accept LocationDescriptor, and String() would corrupt object-form descriptors to "[object Object]" - Remove redundant CommandPaletteSlot.Provider from the test helper since CommandPaletteProvider already wraps children in one Co-Authored-By: Claude <noreply@anthropic.com>
Resource actions (those with a `resource` prop) are async group containers — they only make sense when they have children. When an async resource returns no results, the parent node should not appear in the list. Previously the browse-mode filter only ran on top-level nodes, so a resource action nested inside a group would still be pushed to the results list. The search-mode path had no guard at all, so a resource action whose label matched the query would show up as a (non-functional) clickable item. Fix both paths and add tests for top-level, nested, and search-mode variants of the 0-results case. Co-Authored-By: Claude <noreply@anthropic.com>
github-actions
bot
added
the
Scope: Frontend
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
…mode The matched-child filter in search mode only checked scores but not isEmptyResourceNode, so an async group container with 0 results nested inside a group (e.g. "Reverse DSN lookup" under "Navigate") would pass the filter and appear as a clickable action when the group label matched the query. Apply the same isEmptyResourceNode guard to the per-child filter that browse mode already uses, and add a regression test for the case. Co-Authored-By: Claude <noreply@anthropic.com>
rbro112
approved these changes
Apr 10, 2026
isEmptyResourceNode incorrectly classified nodes with both a prompt and a resource prop as empty (e.g. "Reverse DSN lookup"), causing them to be hidden in browse and search modes. Nodes with a prompt are actionable leaf items — clicking them pushes a nav context — so they should never be treated as empty placeholders. Also apply the isEmptyResourceNode check to top-level nodes in browse mode so that resource containers with 0 results are hidden there too, not just when nested inside a group. Co-Authored-By: Claude <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 9c5f53a. Configure here.
…urce nodes In browse mode, the section header was pushed to results unconditionally before iterating a group's children. If every child resolved to an empty resource node and was skipped, an orphaned, non-selectable header was left in the list. Collect visible children first and skip the group entirely when none remain, matching the guard already in place for search mode. Adds a regression test covering a group whose sole child is an empty resource node. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
wedamija
pushed a commit
that referenced
this pull request
Apr 13, 2026
Resource actions (those with a `resource` prop) are async group containers — they only make sense when they have children populated by async results. When a resource returns no results, the parent node should not appear in the list at all. Two bugs were present: 1. **Browse mode**: the empty-node filter only ran on top-level nodes. A resource action nested inside a group (the typical structure — e.g. "Reverse DSN lookup" under "Navigate") bypassed the check entirely and was pushed to the list as a clickable action with nothing to execute. 2. **Search mode**: no guard existed for resource leaf nodes, so a resource action whose label matched the query would appear as a clickable item even with 0 results. Both paths now apply the same condition: skip nodes that carry a `resource` key but have no children and no `to`/`onAction` of their own. Tests cover the top-level, nested, and search-mode variants. --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> Co-authored-by: OpenAI Codex <noreply@openai.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resource actions (those with a
resourceprop) are async group containers — they only make sense when they have children populated by async results. When a resource returns no results, the parent node should not appear in the list at all.Two bugs were present:
Browse mode: the empty-node filter only ran on top-level nodes. A resource action nested inside a group (the typical structure — e.g. "Reverse DSN lookup" under "Navigate") bypassed the check entirely and was pushed to the list as a clickable action with nothing to execute.
Search mode: no guard existed for resource leaf nodes, so a resource action whose label matched the query would appear as a clickable item even with 0 results.
Both paths now apply the same condition: skip nodes that carry a
resourcekey but have no children and noto/onActionof their own. Tests cover the top-level, nested, and search-mode variants.