Fix bulk-delete review findings: selection count, partial-failure refresh, API group (#871)

Three fixes for issues found in post-merge review of #322:

- Selection count mismatch: the bulk bar, select-all checkbox, and
  confirm dialog now all derive from checkedItems (checked rows still
  visible under current filters) — the same set the delete acts on.
  Previously the bar showed the raw checked-set size, which overstated
  the deletion scope when filters hid checked rows.

- Partial-failure refresh: bulk delete invalidates queries in onSettled
  instead of onSuccess, so resources deleted before a partial failure
  disappear from the table instead of lingering.

- API group disambiguation: DELETE /api/resources accepts ?group= and
  resolves via GetGVRWithGroup. Kind-only resolution picks whichever
  group discovery found first, so deletes from CRD tables whose kind
  collides with another group (Knative vs core Service, CAPI vs CNPG
  Cluster) could 404 or delete the same-named object in the wrong
  group. Both the single-resource and bulk delete paths now pass the
  table's group through.

Author: nadaverell

internal/server/server.go

@@ -2595,6 +2595,7 @@ func (s *Server) handleDeleteResource(w http.ResponseWriter, r *http.Request) {
 	namespace := chi.URLParam(r, "namespace")
 	name := chi.URLParam(r, "name")
 	force := r.URL.Query().Get("force") == "true"
+	group := r.URL.Query().Get("group")
 
 	auth.AuditLog(r, namespace, name)
 	client := s.getDynamicClientForRequest(r)
@@ -2604,6 +2605,7 @@ func (s *Server) handleDeleteResource(w http.ResponseWriter, r *http.Request) {
 	}
 	err := k8s.DeleteResourceWithClient(r.Context(), k8s.DeleteResourceOptions{
 		Kind:      kind,
+		Group:     group,
 		Namespace: namespace,
 		Name:      name,
 		Force:     force,

packages/k8s-ui/src/components/resources/ResourcesView.tsx

@@ -1808,7 +1808,7 @@ interface ResourcesViewProps {
    */
   onClearNamespaces?: () => void
   // Bulk operations
-  onBulkDelete?: (items: Array<{ kind: string; namespace: string; name: string }>, options?: { force?: boolean; onSuccess?: () => void }) => void
+  onBulkDelete?: (items: Array<{ kind: string; group?: string; namespace: string; name: string }>, options?: { force?: boolean; onSuccess?: () => void }) => void
   isBulkDeleting?: boolean
 }
 
@@ -3416,18 +3416,22 @@ export function ResourcesView({
     })
   }, [getResourceKey])
 
-  const toggleCheckAll = useCallback(() => {
-    setCheckedResources(prev => {
-      if (prev.size > 0 && prev.size === filteredResources.length) return new Set()
-      return new Set(filteredResources.map(getResourceKey))
-    })
-  }, [filteredResources, getResourceKey])
-
+  // Selection state is keyed by resource, but everything user-facing
+  // (count, select-all, the delete payload) derives from checkedItems —
+  // the checked ∩ currently-visible intersection. Rows checked and then
+  // hidden by a filter are excluded, so the bar never promises more
+  // deletions than the confirm dialog will perform.
   const checkedItems = useMemo(() => {
     if (checkedResources.size === 0) return []
     return filteredResources.filter(r => checkedResources.has(getResourceKey(r)))
   }, [filteredResources, checkedResources, getResourceKey])
 
+  const allVisibleChecked = filteredResources.length > 0 && checkedItems.length === filteredResources.length
+
+  const toggleCheckAll = useCallback(() => {
+    setCheckedResources(allVisibleChecked ? new Set() : new Set(filteredResources.map(getResourceKey)))
+  }, [allVisibleChecked, filteredResources, getResourceKey])
+
   const isCheckboxMode = onBulkDelete != null && bulkMode
 
   // Filter columns by visibility
@@ -4057,11 +4061,11 @@ export function ResourcesView({
         {isCheckboxMode && (
           <div className="flex items-center gap-3 px-4 py-2 bg-skyhook-500/10 border-b border-skyhook-400/20 shrink-0">
             <span className="text-sm font-medium text-theme-text-primary">
-              {checkedResources.size} selected
+              {checkedItems.length} selected
             </span>
             <button
               onClick={() => setShowBulkDeleteConfirm(true)}
-              disabled={checkedResources.size === 0}
+              disabled={checkedItems.length === 0}
               className="flex items-center gap-1.5 px-3 py-1.5 text-xs font-medium bg-red-600 hover:bg-red-700 disabled:opacity-50 disabled:pointer-events-none text-white rounded-lg transition-colors"
             >
               <Trash2 className="w-3.5 h-3.5" />
@@ -4185,11 +4189,11 @@ export function ResourcesView({
                     <th className="bg-theme-surface border-b border-theme-border w-10 text-center px-0 py-3">
                       <input
                         type="checkbox"
-                        checked={filteredResources.length > 0 && checkedResources.size === filteredResources.length}
-                        ref={(el) => { if (el) el.indeterminate = checkedResources.size > 0 && checkedResources.size < filteredResources.length }}
+                        checked={allVisibleChecked}
+                        ref={(el) => { if (el) el.indeterminate = checkedItems.length > 0 && checkedItems.length < filteredResources.length }}
                         onChange={toggleCheckAll}
                         className="w-3.5 h-3.5 rounded border-theme-border accent-skyhook-500 cursor-pointer"
-                        title={checkedResources.size > 0 ? 'Deselect all' : 'Select all'}
+                        title={checkedItems.length > 0 ? 'Deselect all' : 'Select all'}
                       />
                     </th>
                   )}
@@ -4425,6 +4429,7 @@ export function ResourcesView({
       onConfirm={() => {
         const items = checkedItems.map(r => ({
           kind: selectedKind.name,
+          group: selectedKind.group,
           namespace: r.metadata?.namespace || '',
           name: r.metadata?.name || '',
         }))

packages/k8s-ui/src/components/shared/ResourceActionsBar.tsx

@@ -65,7 +65,7 @@ interface ResourceActionsBarProps {
   renderPortForward?: (props: { type: 'pod' | 'service'; namespace: string; name: string; className?: string }) => React.ReactNode
 
   // Delete
-  onDelete?: (params: { kind: string; namespace: string; name: string; force: boolean }, callbacks?: { onSuccess?: () => void; onError?: (err: unknown) => void }) => void
+  onDelete?: (params: { kind: string; group?: string; namespace: string; name: string; force: boolean }, callbacks?: { onSuccess?: () => void; onError?: (err: unknown) => void }) => void
   isDeleting?: boolean
   cascadeDependents?: CascadeDependent[]
   cascadeLoading?: boolean
@@ -166,7 +166,7 @@ export function ResourceActionsBar({
 
   function handleDeleteConfirm(force: boolean) {
     onDelete?.(
-      { kind: resource.kind, namespace: resource.namespace, name: resource.name, force },
+      { kind: resource.kind, group: resource.group, namespace: resource.namespace, name: resource.name, force },
       {
         onSuccess: () => {
           setShowDeleteConfirm(false)

pkg/k8score/workload.go

@@ -41,6 +41,7 @@ type UpdateResourceOptions struct {
 // DeleteResourceOptions contains options for deleting a resource.
 type DeleteResourceOptions struct {
 	Kind      string
+	Group     string // API group, disambiguates kinds that collide across groups (e.g. Knative vs core Service)
 	Namespace string
 	Name      string
 	Force     bool // Force delete with grace period 0
@@ -378,7 +379,7 @@ func (m *WorkloadManager) DeleteResource(ctx context.Context, opts DeleteResourc
 		return fmt.Errorf("dynamic client not initialized")
 	}
 
-	gvr, ok := m.discovery.GetGVR(opts.Kind)
+	gvr, ok := m.discovery.GetGVRWithGroup(opts.Kind, opts.Group)
 	if !ok {
 		return fmt.Errorf("unknown resource kind: %s", opts.Kind)
 	}

web/src/api/client.ts

@@ -1699,8 +1699,11 @@ export function useDeleteResource() {
   const queryClient = useQueryClient()
 
   return useMutation({
-    mutationFn: async ({ kind, namespace, name, force }: { kind: string; namespace: string; name: string; force?: boolean }) => {
+    mutationFn: async ({ kind, group, namespace, name, force }: { kind: string; group?: string; namespace: string; name: string; force?: boolean }) => {
       const url = new URL(`${getApiBase()}/resources/${kind}/${namespace}/${name}`, window.location.origin)
+      if (group) {
+        url.searchParams.set('group', group)
+      }
       if (force) {
         url.searchParams.set('force', 'true')
       }
@@ -1729,10 +1732,11 @@ export function useBulkDeleteResources() {
   const queryClient = useQueryClient()
 
   return useMutation({
-    mutationFn: async ({ items, force }: { items: Array<{ kind: string; namespace: string; name: string }>; force?: boolean }) => {
+    mutationFn: async ({ items, force }: { items: Array<{ kind: string; group?: string; namespace: string; name: string }>; force?: boolean }) => {
       const results = await Promise.allSettled(
-        items.map(async ({ kind, namespace, name }) => {
+        items.map(async ({ kind, group, namespace, name }) => {
           const url = new URL(`${getApiBase()}/resources/${kind}/${namespace}/${name}`, window.location.origin)
+          if (group) url.searchParams.set('group', group)
           if (force) url.searchParams.set('force', 'true')
           const response = await apiFetch(url.toString(), { method: 'DELETE' })
           if (!response.ok) {
@@ -1752,7 +1756,9 @@ export function useBulkDeleteResources() {
       errorMessage: 'Failed to delete some resources',
       successMessage: 'Resources deleted',
     },
-    onSuccess: () => {
+    // onSettled, not onSuccess — a partial failure still deleted some
+    // resources, and the table must refetch to drop them.
+    onSettled: () => {
       queryClient.invalidateQueries({ queryKey: ['resources'] })
       queryClient.invalidateQueries({ queryKey: ['resource-counts'] })
       queryClient.invalidateQueries({ queryKey: ['topology'] })