Skip to content

Add a scoped global command palette - #4334

Open
keppo-bot[bot] wants to merge 6 commits into
mainfrom
feat/scoped-command-palette
Open

Add a scoped global command palette#4334
keppo-bot[bot] wants to merge 6 commits into
mainfrom
feat/scoped-command-palette

Conversation

@keppo-bot

@keppo-bot keppo-bot Bot commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds a VS Code-style global command palette that makes Dyad navigation and configuration searchable from one place.

  • Opens chat-scoped search with Cmd/Ctrl+K, prefilled with chat: , and searches chat titles plus message content within the selected app.
  • Opens the unfiltered palette with Cmd/Ctrl+P or the title-bar Commands button.
  • Searches global settings, apps, navigation commands, and current-app configuration destinations, then navigates to and highlights the existing control.
  • Keeps app-specific results scoped to the currently selected app and exposes safe navigation/configuration entry points rather than destructive actions.
  • Blocks the shortcuts while confirmations or input-bearing dialogs are open; only disposable search dialogs opt into replacement.
  • Preserves Monaco and terminal-owned key chords before claiming global shortcuts.
  • Filters entitlement- and connection-gated settings consistently across search surfaces and surfaces an error when a destination cannot render.
  • Keeps palette filtering allocation-light by caching settings indexes and using a direct per-item scorer.
  • Opens configuration in the selected chat, or transparently creates a new chat instead of choosing an unrelated one.
  • Adds unit coverage for query parsing, ranking, and destination reveal behavior plus Playwright coverage for both shortcuts and configuration navigation.

Review in cubic

@keppo-bot
keppo-bot Bot requested a review from a team August 20, 2026 22:15

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

πŸ’‘ Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1ced497584

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with πŸ‘.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread src/components/CommandPalette.tsx Outdated

@dyad-assistant dyad-assistant Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Claude review: 5 inline finding(s).

Comment thread src/app/layout.tsx
Comment thread src/components/CommandPalette.tsx Outdated
Comment thread src/components/CommandPalette.tsx Outdated
Comment thread src/components/CommandPalette.tsx Outdated
Comment thread src/lib/commandPalette.ts
@dyad-assistant

Copy link
Copy Markdown
Contributor

πŸ” Dyadbot Code Review Summary

Verdict: πŸ€” NOT SURE - Potential issues
Recommendation: ready

The wiring is sound: CommandDialog already accepts filter, className and data-testid, the data hooks (useSearchApps, useSearchChats, useChats) are TanStack Query-based, previewModeAtom accepts "configure", ipc.chat.createChat returns a plain chatId, and both AppSearchDialog and ChatSearchDialog still use useEffect elsewhere so the removed shortcut effects leave no unused imports. Both dialogs also retain a visible button trigger (search-apps-button, search-chats-button), so removing their Cmd/Ctrl+K handlers does not strand them. No IPC surface, main-process, schema or agent-tool changes are in this PR, so the Electron boundary, DyadError and migration rules are not in play.

The issues below are all MEDIUM β€” mostly interaction-quality and consistency problems around the new global shortcut and the hand-rolled scoring/reveal helpers. None of them block merge. The diff is complete (not truncated).

Issues Summary

Severity File Issue
🟑 MEDIUM src/app/layout.tsx:147 Global palette shortcut hijacks Ctrl+P/Ctrl+K from the terminal
🟑 MEDIUM src/components/CommandPalette.tsx:148 Chat results flash "No results found" during the debounce gap
🟑 MEDIUM src/components/CommandPalette.tsx:394 Chats with identical titles produce duplicate cmdk item values
🟑 MEDIUM src/components/CommandPalette.tsx:188 Substring-only scoring diverges from the existing Fuse search
🟑 MEDIUM src/lib/commandPalette.ts:40 revealCommandPaletteTarget duplicates useScrollAndNavigateTo
🟒 Low Priority Notes (8 items)
  • Unused reveal target id - id="app-config-database" is added to IntegrationSection, but no APP_DETAIL_TARGETS/CONFIGURE_TARGETS entry points at it (the database target uses app-setting-database on the app-details page) and nothing else in the repo references it. It is dead infrastructure that will bit-rot. (src/components/preview_panel/ConfigurePanel.tsx)
  • Triple chat-list invalidation - createChat calls invalidateChats() and then queryClient.invalidateQueries({ queryKey: queryKeys.chats.all }), which are the same operation; the createChat IPC contract already declares invalidates: () => [{ family: "chats" }]. Two of the three are redundant. (src/components/CommandPalette.tsx)
  • Silent reveal failure - revealCommandPaletteTarget returns false after ~1s with no toast or fallback, so if a destination never renders (e.g. a connector hidden by mutual exclusion) the user just lands on a page with no highlight and no explanation. (src/lib/commandPalette.ts)
  • app-config-commands missing in the early-return branch - AppCommandsSection only renders in ConfigurePanel's main return; the loading/error early return renders only the integration and env-vars cards, so the "Configure app commands" target can fail to resolve while the app query is still loading. (src/components/preview_panel/ConfigurePanel.tsx)
  • Optional onOpenCommandPalette - The prop is optional but the button always renders, so TitleBar usages that omit it (e.g. src/testing/hybrid_chat_harness.tsx) show an inert Commands button. Making the prop required, or hiding the button when it is absent, avoids a silently dead control. (src/app/TitleBar.tsx)
  • Shortcut hint without a label at mid widths - The kbd hint appears at @xl while the "Commands" label only appears at @3xl, so between those breakpoints the button reads as a magnifier icon plus a bare "Ctrl P" with no label. (src/app/TitleBar.tsx)
  • Highlight assertion races a self-removing class - toHaveClass(/settings-highlight/) targets a class removed on animationend after a 1.5s animation. It will usually pass, but it is an inherent flake source on slow CI. (e2e-tests/command_palette.spec.ts)
  • Fake timers not restored on failure - vi.useRealTimers() is only called at the end of the test body, so a failing assertion leaks fake timers into sibling tests in the file. Prefer afterEach. (src/lib/commandPalette.test.ts)

Generated by Dyadbot persona-based code review

@keppo-bot

keppo-bot Bot commented Aug 20, 2026

Copy link
Copy Markdown
Contributor Author

πŸ€– Claude Code Review Summary

PR Confidence: 4/5

All trusted review threads are resolved and local unit, type, build, and Electron E2E checks pass; the final CI run is still in progress.

Unresolved Threads

No unresolved threads

Resolved Threads

Issue Rationale Link
Settings destinations and ranking Removed orphaned and unavailable targets, including Pro-only and disconnected integrations, and shared the Settings page's cached Fuse ranking with the palette. Targets, Ranking, Sidebar, Integrations, Cache
Shortcut and modal safety Preserved terminal and Monaco bindings, made modal replacement opt-in, blocked input-bearing dialogs and destructive confirmations, and only registered coordination listeners for opted-in dialogs. Shortcut, Confirmation, Pending input, Monaco, Listener scope
Chat search stability and identity Added an explicit debounce loading state and unique cmdk values for chat/app results. Debounce, Identity
Shared target highlighting Extracted the scroll/highlight behavior so settings navigation and palette retries use one implementation. View
App-specific availability Hidden the mobile configuration command unless the selected app is confirmed to use Capacitor. View
Unavailable destination feedback Added a visible error when a palette destination fails to render after navigation. View
Stable E2E assertions Replaced animation-class checks with durable navigation and target-visibility assertions. View
Allocation-light command filtering Replaced per-item Fuse construction with a direct token/subsequence scorer. View
Explicit configuration chat Uses the selected chat when available and otherwise labels and creates a new chat instead of choosing an arbitrary one. View
Product Principle Suggestions

No suggestions


πŸ€– Generated by Claude Code

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

πŸ’‘ Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e76b5c0bff

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with πŸ‘.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread src/components/CommandPalette.tsx

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

πŸ’‘ Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9274dd8371

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with πŸ‘.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread src/lib/commandPalette.ts Outdated

@dyad-assistant dyad-assistant Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Claude review: 4 inline finding(s).

Comment thread src/components/SettingsList.tsx Outdated
Comment thread src/components/CommandPalette.tsx Outdated
Comment thread src/components/ui/dialog.tsx
Comment thread e2e-tests/command_palette.spec.ts Outdated
@dyad-assistant

Copy link
Copy Markdown
Contributor

πŸ” Dyadbot Code Review Summary

Verdict: πŸ€” NOT SURE - Potential issues
Recommendation: ready

Reviewed all 22 changed files; the diff is complete (patchTruncated: false, diffTruncated: false), so confidence is good. This is a well-scoped renderer-only feature: no IPC contracts, main-process handlers, database schema, or agent tools are touched, so the Electron main/renderer boundary and DyadError conventions are unaffected. The palette is built on the existing Base UI Dialog wrapper plus the repo's existing cmdk command primitives, and IPC-backed data fetching (useSearchApps, useSearchChats, useChats, useQuery for isCapacitor) reuses established TanStack Query hooks and shared query keys. Verified that the removed disableShortcut prop has no remaining callers, that SETTING_IDS.nodeRuntime was unreferenced outside the index it was deleted from, that the six new requiresPro flags line up exactly with the Pro-gated toggles in SubagentSettings, and that the useEffect imports left behind in AppSearchDialog/ChatSearchDialog are still used.

No merge blockers found. The issues below are behavioral/consistency gaps rather than breakage.

Issues Summary

Severity File Issue
🟑 MEDIUM src/components/SettingsList.tsx:46 Settings sidebar search still offers Pro-only destinations that cannot render
🟑 MEDIUM src/components/CommandPalette.tsx:204 Reveal failures are silent when the destination element never renders
🟑 MEDIUM src/components/ui/dialog.tsx:21 Palette shortcut force-closes every dialog, discarding unsaved input
🟑 MEDIUM e2e-tests/command_palette.spec.ts:27 Highlight assertions race the 1.5s settings-highlight animation
🟒 Low Priority Notes (7 items)
  • Unused anchor id - id="app-config-database" is added to IntegrationSection, but no palette target references it; "Configure database integration" points at app-setting-database on the app-details page instead. Dead anchor. (src/components/preview_panel/ConfigurePanel.tsx)
  • Redundant invalidation in createChat - invalidateChats() already invalidates queryKeys.chats.all, so the following queryClient.invalidateQueries({ queryKey: queryKeys.chats.all }) is a duplicate. invalidateChats also returns void, so awaiting it does not actually wait for the refetch. (src/components/CommandPalette.tsx)
  • Inert Commands button in the test harness - onOpenCommandPalette is optional and src/testing/hybrid_chat_harness.tsx renders <TitleBar /> with no props, so the new "Commands" button renders there with an undefined handler. Consider making the prop required, or hiding the button when no handler is supplied. (src/app/TitleBar.tsx)
  • Silent shortcut suppression - when hasBlockingAlertDialogOpen() is true the handler has already called preventDefault() and then returns with no user-visible feedback, so the shortcut appears to do nothing. A brief hint would explain why. (src/app/layout.tsx)
  • Stale skipped test - e2e-tests/chat_search.spec.ts:88 (test.skip) still documents Ctrl+K opening the chat-search dialog, which this PR removes. It will not fail CI, but it now describes behavior that no longer exists. (e2e-tests/chat_search.spec.ts)
  • Per-item Fuse construction - scoreCommandPaletteItem builds a fresh single-document Fuse index for every non-matching item on every keystroke. Cheap per call, but it scales with app/chat result count; a shared index over the candidate list would avoid the churn. (src/lib/commandPalette.ts)
  • Focus after modal swap is untested - the new e2e asserts the palette becomes visible when Ctrl+K replaces an open app-search-dialog, but not that command-palette-input actually holds focus. Focus restoration from the closing dialog is the thing most likely to go wrong in that swap; an explicit focus assertion would cover it. (e2e-tests/command_palette.spec.ts)

One thing I could not verify from this checkout: node_modules is not installed, so I could not confirm that @base-ui/react@^1.2 exposes close() on Dialog.Root.Actions. If it does not, tsc would fail, so CI should catch it - flagging only because I could not check it directly.


Generated by Dyadbot persona-based code review

@github-actions github-actions Bot added the needs-human:review-issue ai agent flagged an issue that requires human review label Aug 20, 2026
@keppo-bot keppo-bot Bot removed the needs-human:review-issue ai agent flagged an issue that requires human review label Aug 21, 2026

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

πŸ’‘ Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c2d4ecfcd9

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with πŸ‘.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread src/components/CommandPalette.tsx

@dyad-assistant dyad-assistant Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Claude review: 5 inline finding(s).

Comment thread src/app/layout.tsx
Comment thread src/lib/settingsSearchIndex.ts Outdated
Comment thread src/lib/commandPalette.ts Outdated
Comment thread src/components/ui/dialog.tsx
Comment thread src/components/CommandPalette.tsx Outdated
@dyad-assistant

Copy link
Copy Markdown
Contributor

πŸ” Dyadbot Code Review Summary

Verdict: πŸ€” NOT SURE - Potential issues
Recommendation: ready

Reviewed PR #4334 (feat/scoped-command-palette) β€” 24 files, +1122/-130. The diff was complete (diffTruncated: false), so confidence in coverage is good.

The feature is well-scoped: no new IPC handlers, no main-process changes, no schema/migration surface, and no destructive commands are exposed β€” the palette only navigates and reveals existing controls. Renderer data fetching reuses existing TanStack Query hooks (useSearchApps, useSearchChats, useChats) and existing typed IPC contracts, and the UI is built on the Base UI-backed CommandDialog/Dialog wrappers rather than Radix. Loading ("Searching..."), empty, and "select an app" states are all handled in CommandEmpty, and reveal failures surface a toast.

I verified several things that could have been blockers and they check out: SETTING_IDS.nodeRuntime is only referenced inside settingsSearchIndex.ts (both the constant and the index entry are removed together, so nothing dangles); every remaining indexed setting has a matching rendered element in settings.tsx; the new requiresPro flags exactly match the isDyadProEnabled gate in SubagentSettings.tsx; ConfirmationDialog early-returns null when closed, so its new data-open marker cannot permanently block the palette; AppSearchDialog/ChatSearchDialog still use useEffect elsewhere, so removing the shortcut effects leaves no unused imports; and the only other Ctrl+K e2e assertion (chat_search.spec.ts) is already test.skip.

The issues below are all MEDIUM β€” none block merge.

Issues Summary

Severity File Issue
🟑 MEDIUM src/app/layout.tsx:154 Global palette shortcut only exempts the terminal, not the code editor
🟑 MEDIUM src/lib/settingsSearchIndex.ts:663 Settings Fuse index is rebuilt on every keystroke
🟑 MEDIUM src/lib/commandPalette.ts:37 Command palette filter builds a Fuse instance per item per keystroke
🟑 MEDIUM src/components/ui/dialog.tsx:41 Every Dialog gets an actionsRef and window listener it does not need
🟑 MEDIUM src/components/CommandPalette.tsx:180 App configuration commands depend on an arbitrary chat
🟒 Low Priority Notes (7 items)
  • Unused DOM id - id="app-config-database" is added to IntegrationSection, but no palette entry targets it (the database command points at app-setting-database on the app-details page). Either add the configure-panel entry or drop the id. (src/components/preview_panel/ConfigurePanel.tsx)
  • Duplicate cache invalidation - createChat calls invalidateChats() and then queryClient.invalidateQueries({ queryKey: queryKeys.chats.all }); invalidateChats already invalidates exactly that key. (src/components/CommandPalette.tsx)
  • Late reveal toast - revealCommandPaletteTarget polls for up to 1s (20 Γ— 50ms) with no cancellation, so "Couldn't open that setting" can appear a second after the user has already navigated elsewhere. Consider aborting when the route or selection changes. (src/lib/commandPalette.ts)
  • Re-pressing the shortcut clears the query - openCommandPalette always resets the query, so pressing Cmd/Ctrl+P while the palette is already open silently wipes what the user typed (the old Ctrl+K behavior was a toggle). Consider only resetting when the palette is currently closed. (src/app/layout.tsx)
  • Sheets are not treated as blocking - hasBlockingCommandPaletteDialogOpen matches dialog-content and alert-dialog-content, but Sheet renders data-slot="sheet-content", so the palette can stack a second modal on top of an open sheet. (src/lib/commandPalette.ts)
  • Accessible name differs from the visible label - the trigger's aria-label="Open command palette" overrides the visible "Commands" text, which breaks voice-control users saying "click Commands". Also, platform is null while useSystemPlatform resolves, so macOS briefly renders "Ctrl P" before "⌘P". (src/app/TitleBar.tsx)
  • No-op button in the test harness - onOpenCommandPalette is optional and the button always renders, so src/testing/hybrid_chat_harness.tsx (which mounts <TitleBar /> with no props) shows a dead Commands button. Making the prop required or rendering the button conditionally would keep the harness honest. (src/app/TitleBar.tsx)

Generated by Dyadbot persona-based code review

@github-actions github-actions Bot added the needs-human:review-issue ai agent flagged an issue that requires human review label Aug 21, 2026
@keppo-bot keppo-bot Bot removed the needs-human:review-issue ai agent flagged an issue that requires human review label Aug 21, 2026

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

πŸ’‘ Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9573973b6c

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with πŸ‘.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment on lines +461 to +464
onSelect={() =>
closeAndRun(() =>
selectChat({ chatId: chat.id, appId: chat.appId }),
)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Discard chat results from the previously selected app

When switching from app A to app B after searching chats in A, useSearchChats retains A's results via keepPreviousData while B's request is pending. This item remains selectable and passes chat.appId to selectChat, so a quick selection unexpectedly navigates back to app A even though the palette is scoped to app B. Filter these results by selectedAppId or avoid retaining results across app-ID changes.

Useful? React with πŸ‘Β / πŸ‘Ž.

Comment on lines +392 to +394
onSelect={() =>
closeAndRun(() => openConfigureTarget(item.targetId))
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Keep the app-command target mounted during env failures

When environment-variable loading takes longer than the one-second reveal window or fails, ConfigurePanel.tsx returns from its loading/error branch before AppCommandsSection is mounted. This command is nevertheless always selectable, so navigation ends with β€œCouldn't open that setting” even though app-command configuration is independent of environment variables. Render the app-command section in that branch or only offer the command when its target can mount.

Useful? React with πŸ‘Β / πŸ‘Ž.

@dyad-assistant dyad-assistant Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Claude review: 2 inline finding(s).

if (setting.requiresPro && (!settings || !isDyadProEnabled(settings))) {
return false;
}
return (

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟑 MEDIUM

Disconnected integrations disappear from settings search

getAvailableSettings drops any entry with requiresConnection when the integration has no token, and SettingsList now searches that filtered list too. Unlike the Pro-gated entries (which genuinely do not render - SubagentSettings returns null without Pro), the GitHub/Vercel/Supabase/Neon sections in src/pages/settings.tsx render unconditionally with their Connect UI. So searching 'GitHub' in the Settings page sidebar returns nothing precisely when the user is not connected and most needs to find that section, which is a regression in the pre-existing settings search rather than only a new palette behavior.

πŸ’‘ Suggestion: Filter only on requiresPro (where the destination truly does not render) and keep requiresConnection entries searchable, or use requiresConnection just to de-rank them instead of removing them.

const openConfigureTarget = async (targetId: string) => {
if (!selectedAppId) return;
const chatId =
targetChatId !== null

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟑 MEDIUM

Palette can create a stray chat before the chat list loads

targetChatId comes from getSelectedChatId(chats, selectedChatId), and useChats returns [] while its query is in flight (for example right after switching the selected app, when the chats.list key for the new appId has no cached data). In that window a chat is open but targetChatId is null, so openConfigureTarget calls ipc.chat.createChat and navigates the user into a brand-new empty chat instead of reusing the one they were in, leaving an unwanted chat behind.

πŸ’‘ Suggestion: Take the loading flag from useChats and either wait for the list or fall back to selectedChatId while it is still loading, so a chat is only created when the loaded list confirms there is no selected chat.

@dyad-assistant

Copy link
Copy Markdown
Contributor

πŸ” Dyadbot Code Review Summary

Verdict: πŸ€” NOT SURE - Potential issues
Recommendation: ready

The command palette is well-factored: pure helpers (parseCommandPaletteQuery, scoreCommandPaletteItem, revealCommandPaletteTarget) are unit-tested, IPC data goes through existing TanStack Query hooks (useSearchApps, useSearchChats, useChats, plus the same queryKeys.appUpgrades.isCapacitor key CapacitorControls uses), dialogs stay on the Base UI wrappers, and the new E2E coverage is a single broad spec rather than many narrow ones. No main-process/IPC surface, no schema changes, so the DyadError/migration rules don't apply here. I verified that every id the palette targets exists (setting-* ids in settings.tsx/SubagentSettings.tsx, the new app-setting-*/app-config-* wrappers), that the removed SETTING_IDS.nodeRuntime was dead (no element rendered that id and nothing else referenced it), and that the duplicated app-config-environment-variables/app-config-commands ids sit on mutually exclusive render branches.

Two MEDIUM issues are worth a look; neither blocks merge.

Issues Summary

Severity File Issue
🟑 MEDIUM src/lib/settingsSearchIndex.ts:660 Disconnected integrations disappear from settings search
🟑 MEDIUM src/components/CommandPalette.tsx:206 Palette can create a stray chat before the chat list loads
🟒 Low Priority Notes (7 items)
  • Redundant chat invalidation - await invalidateChats() returns undefined (the hook doesn't return the invalidation promise) and already invalidates queryKeys.chats.all, so the immediately following await queryClient.invalidateQueries({ queryKey: queryKeys.chats.all }) duplicates it. Keep only the awaited one. (src/components/CommandPalette.tsx)
  • Dead branch in the scorer - candidate.startsWith(queryToken) || candidate.includes(queryToken) β€” includes subsumes startsWith, and both paths return the same score of 50, so prefix matches aren't actually ranked above mid-token matches. (src/lib/commandPalette.ts)
  • Sentinel text participates in matching - Item values embed #chat-<id> / #app-<id> for uniqueness, and scoreCommandPaletteItem substring-matches the whole value, so typing chat or app in the unscoped palette scores every chat/app row highly. Consider scoring the display text and keeping the sentinel out of the matched string. (src/components/CommandPalette.tsx)
  • Suppression contract is DOM-marker based and unenforced - Blocking depends on data-slot, data-open, and data-command-palette-dismissible, which is why the legacy ConfirmationDialog had to be hand-labeled data-slot="alert-dialog-content". Any future custom overlay that doesn't use the shared wrappers silently loses protection, and no test or lint catches it. The rules doc covers this, but a small unit test over the shared wrappers would be cheaper to keep honest. (src/components/ui/dialog.tsx)
  • Commands button is a no-op without the prop - onOpenCommandPalette is optional on TitleBar, so the new button renders but does nothing wherever TitleBar is used without it (e.g. src/testing/hybrid_chat_harness.tsx). Not user-facing today. (src/app/TitleBar.tsx)
  • Unverified: actionsRef.close() and controlled open state - AppSearchDialog/ChatSearchDialog are controlled by parent state (AppList, ChatList), so the imperative close must route through onOpenChange or the parent keeps open === true and its toggle button needs two clicks to reopen. The new E2E exercises the close path, but node_modules isn't available here so I couldn't confirm the Base UI 1.2 Root.Actions shape β€” worth a manual check. (src/components/ui/dialog.tsx)
  • Error copy is settings-specific and not actionable - "Couldn't open that setting. Please try again." is also shown for app-config destinations, and a retry hits the same 1s polling failure. (src/components/CommandPalette.tsx)

Generated by Dyadbot persona-based code review

@github-actions github-actions Bot added the needs-human:review-issue ai agent flagged an issue that requires human review label Aug 21, 2026
@github-actions

Copy link
Copy Markdown
Contributor

🎭 Playwright Test Results

❌ Some tests failed

OS Passed Failed Flaky Skipped
🍎 macOS 290 3 1 12

Summary: 290 passed, 3 failed, 1 flaky, 12 skipped

Failed Tests

🍎 macOS

  • local_agent_advanced.spec.ts > local-agent - mention apps
    • Error: expect(string).toMatchSnapshot(expected) failed
  • local_agent_auto.spec.ts > local-agent - auto model
    • Error: expect(string).toMatchSnapshot(expected) failed
  • local_agent_explore_code.spec.ts > local-agent - sub-agent tools replace root explore_code
    • Error: expect(string).toMatchSnapshot(expected) failed

πŸ“‹ Re-run Failing Tests (macOS)

Copy and paste to re-run all failing spec files locally:

npm run e2e \
  e2e-tests/local_agent_advanced.spec.ts \
  e2e-tests/local_agent_auto.spec.ts \
  e2e-tests/local_agent_explore_code.spec.ts

⚠️ Flaky Tests

🍎 macOS

  • problems.spec.ts > problems - manual edit (next.js) (passed after 1 retry)

πŸ“Š View full report

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-human:review-issue ai agent flagged an issue that requires human review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant