feat(agent-workspaces): add remove button to workspace detail view#1174
Conversation
Allow users to remove a workspace directly from the detail page with a confirmation dialog, then navigate back to the workspace list. Closes kortex-hub#1164 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Marcel Bertagnini <mbertagn@redhat.com> Made-with: Cursor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds a "Remove Workspace" action to the workspace details view that opens a confirmation dialog, calls Changes
Sequence DiagramsequenceDiagram
actor User
participant AgentWorkspaceDetails
participant withConfirmation
participant window as WindowAPI
participant Router
User->>AgentWorkspaceDetails: Click "Remove Workspace"
AgentWorkspaceDetails->>withConfirmation: withConfirmation(handleRemove, text)
withConfirmation->>window: showMessageBox(confirmation)
window-->>withConfirmation: response (0=confirm, 1=cancel)
alt response = 0 (confirm)
withConfirmation->>window: removeAgentWorkspace(workspaceId)
window-->>withConfirmation: success / error
alt success
withConfirmation->>Router: goto('/agent-workspaces')
else error
withConfirmation-->>AgentWorkspaceDetails: log error
end
else response = 1 (cancel)
withConfirmation-->>AgentWorkspaceDetails: no-op
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/renderer/src/lib/agent-workspaces/AgentWorkspaceDetails.spec.ts (1)
160-193: Add a rejection-path test forremoveAgentWorkspacebefore navigating.Current coverage checks confirm/cancel, but not deletion failure. Please add a test asserting no navigation when
window.removeAgentWorkspacerejects.🧪 Suggested test case
+test('Expect not navigating when workspace removal fails', async () => { + vi.mocked(window.showMessageBox).mockResolvedValue({ response: 0 }); + vi.mocked(window.removeAgentWorkspace).mockRejectedValue(new Error('remove failed')); + const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); + + render(AgentWorkspaceDetails, { workspaceId: 'ws-1' }); + + await waitFor(() => { + expect(screen.getByRole('button', { name: 'Remove Workspace' })).toBeInTheDocument(); + }); + + await fireEvent.click(screen.getByRole('button', { name: 'Remove Workspace' })); + + await waitFor(() => { + expect(window.removeAgentWorkspace).toHaveBeenCalledWith('ws-1'); + }); + expect(router.goto).not.toHaveBeenCalled(); + expect(errorSpy).toHaveBeenCalled(); +});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/renderer/src/lib/agent-workspaces/AgentWorkspaceDetails.spec.ts` around lines 160 - 193, Add a test in AgentWorkspaceDetails.spec.ts that simulates window.showMessageBox resolving with the confirm response and then mocks window.removeAgentWorkspace to reject; render AgentWorkspaceDetails with workspaceId 'ws-1', click the "Remove Workspace" button (use fireEvent.click), await the removeAgentWorkspace rejection (e.g., by awaiting a waitFor that expects removeAgentWorkspace toHaveBeenCalled), and assert that router.goto was not called; reference the existing patterns for mocking window.showMessageBox, window.removeAgentWorkspace, and checking router.goto to place the new test alongside the confirm/cancel cases.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/renderer/src/lib/agent-workspaces/AgentWorkspaceDetails.svelte`:
- Around line 38-42: The confirmation callback in handleRemove currently
triggers removal and navigation without awaiting success and also gets invoked
on dialog rejection; change handleRemove to call withConfirmation so the
confirmed branch awaits window.removeAgentWorkspace(workspaceId) and only after
await resolves call router.goto('/agent-workspaces'), and ensure any errors from
removeAgentWorkspace are caught and logged/shown (e.g., catch and
processLogger/console.error) while not performing navigation; update usage or
signature of withConfirmation (or provide explicit onConfirm/onCancel callbacks)
so rejected dialog does not run the removal code.
---
Nitpick comments:
In `@packages/renderer/src/lib/agent-workspaces/AgentWorkspaceDetails.spec.ts`:
- Around line 160-193: Add a test in AgentWorkspaceDetails.spec.ts that
simulates window.showMessageBox resolving with the confirm response and then
mocks window.removeAgentWorkspace to reject; render AgentWorkspaceDetails with
workspaceId 'ws-1', click the "Remove Workspace" button (use fireEvent.click),
await the removeAgentWorkspace rejection (e.g., by awaiting a waitFor that
expects removeAgentWorkspace toHaveBeenCalled), and assert that router.goto was
not called; reference the existing patterns for mocking window.showMessageBox,
window.removeAgentWorkspace, and checking router.goto to place the new test
alongside the confirm/cancel cases.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4d14cbb8-8b48-4d58-a040-4b62209f3252
📒 Files selected for processing (2)
packages/renderer/src/lib/agent-workspaces/AgentWorkspaceDetails.spec.tspackages/renderer/src/lib/agent-workspaces/AgentWorkspaceDetails.svelte
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
…ul removal Await the removal IPC call before navigating, so the user stays on the detail page if the removal fails. Closes kortex-hub#1164 Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Marcel Bertagnini <mbertagn@redhat.com>
Allow users to remove a workspace directly from the detail page with a confirmation dialog, then navigate back to the workspace list.
Closes #1164
Made-with: Claude-4.6-opus-high