fix: QuickEntry agent outside-click + quarantine CLI full-suite cascade#2291
Conversation
Capture-phase outside mousedown and an open-token stop the agent portal from staying open or reopening after dismiss. Quarantine the full-suite shard-4 CLI package-lane cascade (extension-dist-barrel hookTimeout + lock-retry timeouts) on sight per the deletion ratchet.
📝 WalkthroughWalkthroughThe PR fixes Quick Add agent-picker dismissal by using capture-phase outside-click handling and tokens that reject stale agent-loading results. It also adds a changeset and expands CLI test quarantine records and exclusions for shard-4 failures. ChangesQuick Entry agent-picker dismissal
CLI test quarantine
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant QuickEntryBox
participant Document
participant fetchAgents
User->>QuickEntryBox: Open agent picker
QuickEntryBox->>fetchAgents: Fetch agents
QuickEntryBox->>Document: Register capture-phase mousedown listener
User->>Document: Click outside picker
Document->>QuickEntryBox: Close picker and invalidate token
fetchAgents-->>QuickEntryBox: Return agents
QuickEntryBox->>QuickEntryBox: Ignore stale result
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThis PR updates QuickEntry picker dismissal and quarantines unstable CLI suites. The main changes are:
Confidence Score: 4/5Pending agent requests can reopen dismissed UI and cross a project transition with stale data.
packages/dashboard/app/components/QuickEntryBox.tsx and its agent-picker tests
|
| Filename | Overview |
|---|---|
| packages/dashboard/app/components/QuickEntryBox.tsx | Adds capture-phase dismissal and request tokens, but several close and project-transition paths do not invalidate pending requests. |
| packages/dashboard/app/components/tests/QuickEntryBox.test.tsx | Improves outside-click timing but does not cover a response arriving after dismissal. |
| packages/cli/vitest.config.ts | Adds fourteen CLI test exclusions mirrored by the quarantine ledger. |
| scripts/lib/test-quarantine.json | Records the fourteen CLI quarantines with matching paths, reasons, and dates. |
| .changeset/quickentry-agent-outside-click.md | Adds a labeled patch changeset for the picker fix. |
Reviews (1): Last reviewed commit: "fix: close QuickEntry agent picker outsi..." | Re-trigger Greptile
| updateAgentPickerPosition(); | ||
| try { | ||
| const result = await fetchAgents(undefined, projectId); | ||
| if (openToken !== agentPickerOpenTokenRef.current) return; |
There was a problem hiding this comment.
Other Dismissals Keep Request Valid
When a fetch is pending, Escape, a second trigger click, or form reset closes the picker without changing agentPickerOpenTokenRef. The response therefore passes this check and reopens a picker the operator already dismissed.
Context Used: AGENTS.md (source)
| setAgents(result); | ||
| setAgentsProjectId(projectId); | ||
| setShowAgentPicker(true); |
There was a problem hiding this comment.
Project Switch Accepts Stale Agents
If projectId changes while this request is pending, the project-change effect closes and clears the picker but does not invalidate the token. The old response can then populate agents under the new project ID and reopen the picker with data from the previous project.
Context Used: AGENTS.md (source)
| outsideElement.dispatchEvent(new MouseEvent("mousedown", { bubbles: true, cancelable: true })); | ||
| await waitFor(() => { | ||
| expect(screen.queryByText("Select agent")).toBeNull(); | ||
| }); |
There was a problem hiding this comment.
Late Fetch Race Remains Untested
This test dismisses the picker only after the mocked agent request has resolved, so removing the new token guard would not make it fail. Use a deferred request, dismiss the loading picker, then resolve the request and verify that the portal stays closed.
Context Used: AGENTS.md (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/dashboard/app/components/QuickEntryBox.tsx (1)
559-577: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueStandardize capture-phase outside-click listeners across all pickers.
Using the capture phase (
true) successfully prevents nestedstopPropagationcalls from hiding outside clicks for the Agent picker. Consider standardizing this approach for the other pickers in this file (Node, Priority, Workflow, Models, Deps) to prevent similar dismissal bugs if their internal contents ever stop event propagation.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/dashboard/app/components/QuickEntryBox.tsx` around lines 559 - 577, Update the outside-click listeners for the Node, Priority, Workflow, Models, and Deps pickers to use capture-phase registration and matching cleanup, consistent with handleClickOutside for the Agent picker. Preserve each picker’s existing containment checks and dismissal behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/dashboard/app/components/QuickEntryBox.tsx`:
- Around line 194-195: Ensure every Agent picker dismissal invalidates pending
fetches, not only outside-click handling. Update the QuickEntryBox dismissal
flows—including Escape, the Agent toggle, and opening other pickers—to use a
shared closeAgentPicker helper or equivalent effect that increments
agentPickerOpenTokenRef alongside setShowAgentPicker(false), while preserving
the existing token guard.
---
Nitpick comments:
In `@packages/dashboard/app/components/QuickEntryBox.tsx`:
- Around line 559-577: Update the outside-click listeners for the Node,
Priority, Workflow, Models, and Deps pickers to use capture-phase registration
and matching cleanup, consistent with handleClickOutside for the Agent picker.
Preserve each picker’s existing containment checks and dismissal behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 3d2b51cc-533c-4a26-afb7-42b82b544f93
📒 Files selected for processing (5)
.changeset/quickentry-agent-outside-click.mdpackages/cli/vitest.config.tspackages/dashboard/app/components/QuickEntryBox.tsxpackages/dashboard/app/components/__tests__/QuickEntryBox.test.tsxscripts/lib/test-quarantine.json
| /** Bumps on open/close so a late fetchAgents resolution cannot re-open a dismissed picker. */ | ||
| const agentPickerOpenTokenRef = useRef(0); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Invalidate the token on all dismissal paths.
The token is successfully incremented on outside clicks, but other dismissal vectors (such as pressing the Escape key, clicking the Agent toggle button to close, or opening other pickers) call setShowAgentPicker(false) without bumping the token. If an agent fetch is in flight when the user closes the picker via these actions, the late resolution will still bypass the token guard and erroneously re-open the picker.
Consider adding a useEffect that bumps the token when the picker closes, or extract a closeAgentPicker helper that groups the state update and token increment, and use it across all dismissal paths.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/dashboard/app/components/QuickEntryBox.tsx` around lines 194 - 195,
Ensure every Agent picker dismissal invalidates pending fetches, not only
outside-click handling. Update the QuickEntryBox dismissal flows—including
Escape, the Agent toggle, and opening other pickers—to use a shared
closeAgentPicker helper or equivalent effect that increments
agentPickerOpenTokenRef alongside setShowAgentPicker(false), while preserving
the existing token guard.
## Summary Full Suite after #2291: shards 1–3 green; shard 4 failed CLI suites with: `Failed to resolve entry for package "@fusion-plugin-examples/claude-runtime"` from `dashboard/src/runtime-provider-probes.ts` under the CLI vitest package lane. - Add `plugins/fusion-plugin-claude-runtime/src/probes-entry.ts` (probe + model discovery only) - Alias `@fusion-plugin-examples/claude-runtime` to that entry in CLI vitest config (same class as Cursor/Grok/OMP source aliases, but avoids ACP index load) ## Test plan - [x] Local: vitest-workspace-resolution, task-steer, extension-task-tools - [ ] PR gate - [ ] Post-merge Full Suite green <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved CLI resolution for Claude runtime diagnostics and provider model discovery. * Prevented unnecessary runtime dependencies from affecting CLI test execution. * **Refactor** * Added a lightweight entry point for accessing Claude binary checks and provider model discovery. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Summary
Latest Full Suite after #2290 was green on shards 1–2 and nearly green on 3–4:
fetchAgentscannot re-open a dismissed picker@runfusion/fusionpackage-lane cascade (87 failures fromextension-dist-barrelhookTimeout + lock-retry timeouts under load) — quarantine the 14 observed files on sight (ledger + vitest exclude), no timeout appeasementAlso hardens the agent-picker outside-click test.
Test plan
Summary by CodeRabbit
Bug Fixes
Tests