Skip to content

Fix desktop slash commands#8341

Merged
DOsinga merged 4 commits intomainfrom
fix-desktop-slash-commands
Apr 6, 2026
Merged

Fix desktop slash commands#8341
DOsinga merged 4 commits intomainfrom
fix-desktop-slash-commands

Conversation

@DOsinga
Copy link
Copy Markdown
Collaborator

@DOsinga DOsinga commented Apr 6, 2026

Summary

Race condition that could make / commands show the results of the @ short cut if used first

Douwe Osinga added 2 commits April 3, 2026 17:57
The MentionPopover shared async state between @ (file scan) and /
(slash commands) modes, causing race conditions:

- Stale file items displayed when switching to slash command mode
  because items state was never cleared between mode switches
- Long-running scanFilesFromRoot() could complete after switching
  to slash command mode, overwriting command results with files
- No loading indicator shown while fetching slash commands

Fix by:
- Clearing items immediately when loading new data (setItems([]))
- Adding cancellation flag to prevent stale async results from
  overwriting current data
- Inlining the file scan into the unified useEffect with proper
  cancellation support
- Showing context-appropriate loading/empty messages for each mode

Signed-off-by: Douwe Osinga <douwe@squareup.com>
…isting

Vite's dependency pre-bundling was inlining a second copy of React into
the react-intl bundle. This happened because react-intl is hoisted to
the workspace root (ui/node_modules/react-intl/), and its
`import * as React from 'react'` resolved to ui/node_modules/react/
instead of ui/desktop/node_modules/react/. Two React instances at
runtime causes the 'Invalid hook call' error in IntlProvider.

Fix by adding resolve.alias entries in the Vite config that pin react
and react-dom to the desktop app's own node_modules copies, ensuring
a single React instance across all pre-bundled dependencies.

Signed-off-by: Douwe Osinga <douwe@squareup.com>
Copy link
Copy Markdown

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

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: 2094047d06

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +20 to +23
loadingCommands: {
id: 'mentionPopover.loadingCommands',
defaultMessage: 'Loading commands...',
},
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Sync i18n catalog after adding new message IDs

This commit introduces new defineMessages IDs (mentionPopover.loadingCommands here and mentionPopover.noCommandsFound below) but does not update ui/desktop/src/i18n/messages/en.json, so pnpm run i18n:check will fail because it diffs extracted messages against the committed catalog (ui/desktop/scripts/i18n-check.js), and CI runs that via pnpm run lint:check in .github/workflows/ci.yml. As written, this blocks the UI lint job and merge.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Member

@jamadeo jamadeo left a comment

Choose a reason for hiding this comment

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

out of scope here but we should do this in the backend and use ACP!

try re-creating node_modules before merging the vite config change

@DOsinga DOsinga added this pull request to the merge queue Apr 6, 2026
Merged via the queue into main with commit 771bc38 Apr 6, 2026
26 checks passed
@DOsinga DOsinga deleted the fix-desktop-slash-commands branch April 6, 2026 15:38
lifeizhou-ap added a commit that referenced this pull request Apr 7, 2026
* origin/main: (32 commits)
  docs: rework homepage and add aaif migration blog post (#8356)
  chore(aaif): rename a bunch of repository references (#8152)
  fix: use OPENAI_API_KEY secret for recipe security scanner (#8358)
  feat: configurable extension timeouts via ACP _meta and global default (#8295)
  fix: hide hidden extensions in UI (#8346)
  refactor: skills as its own platform ext (#8244)
  fix baseUrl (#8347)
  Fix desktop slash commands (#8341)
  fix(cli): display platform-correct secrets path in keyring config dialog (#8328)
  feat(acp): add reusable ACP provider controls (#8314)
  fix: resolve MDX compilation error in using-goosehints.md (#8332)
  fix: use v1beta1 API version for Google/MaaS models on GCP Vertex AI (#8278)
  docs: add MCP Roots guide (#8252)
  rust acp client for extension methods (#8227)
  fix: reconsolidate split tool-call messages to follow OpenAI format (#7921)
  fix: clean up MCP subprocesses after abrupt parent exit (#8242)
  build: raise default stack reserve to 8 MB (#8234)
  fix(config): honour GOOSE_DISABLE_KEYRING from config.yaml at startup (#8219)
  feat: add configurable fast_model for declarative providers (#8194)
  fix(authentication): Allow connecting to Oauth servers that use protected-resource fallback instead of the WWW-authenticate header (#8148)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants