chore: review cleanup follow-up for #699 (#701)

Author: nadaverell

internal/helm/client.go

@@ -734,20 +734,19 @@ func parseManifestResources(manifest, defaultNamespace string) []OwnedResource {
 	manifests := releaseutil.SplitManifests(manifest)
 
 	for _, m := range manifests {
-		// Simple parsing - look for kind, apiVersion, name, and namespace
 		lines := strings.Split(m, "\n")
 		var kind, apiVersion, name, namespace string
 
+		// Take the first occurrence of each top-level field; nested specs
+		// (e.g. spec.template) can repeat the same keys with different values.
 		for _, line := range lines {
 			line = strings.TrimSpace(line)
-			if after, ok := strings.CutPrefix(line, "kind:"); ok {
-				kind = strings.TrimSpace(after)
-			} else if after, ok := strings.CutPrefix(line, "apiVersion:"); ok {
+			if after, ok := strings.CutPrefix(line, "kind:"); ok && kind == "" {
+				kind = strings.Trim(strings.TrimSpace(after), `"'`)
+			} else if after, ok := strings.CutPrefix(line, "apiVersion:"); ok && apiVersion == "" {
 				apiVersion = strings.Trim(strings.TrimSpace(after), `"'`)
 			} else if strings.HasPrefix(line, "name:") && name == "" {
-				// Only take first name (metadata.name, not container names etc)
 				name = strings.TrimSpace(strings.TrimPrefix(line, "name:"))
-				// Remove quotes if present
 				name = strings.Trim(name, `"'`)
 			} else if strings.HasPrefix(line, "namespace:") && namespace == "" {
 				namespace = strings.TrimSpace(strings.TrimPrefix(line, "namespace:"))

internal/timeline/sqlite_store.go

@@ -135,9 +135,8 @@ func (s *SQLiteStore) initSchema() error {
 		return err
 	}
 
-	// Additive migration: api_version column added to disambiguate CRD kind collisions
-	// on navigation. ALTER TABLE on an existing column errors with "duplicate column",
-	// so detect via PRAGMA before adding.
+	// SQLite's ALTER TABLE ADD COLUMN errors with "duplicate column" when the
+	// schema is already current; PRAGMA-detect before adding.
 	rows, err := s.db.Query("PRAGMA table_info(events)")
 	if err != nil {
 		return err

packages/k8s-ui/src/types/core.ts

@@ -237,7 +237,7 @@ export interface TimelineEvent {
 
   // Resource identity
   kind: string
-  apiVersion?: string // e.g. "apps/v1", "cluster.x-k8s.io/v1beta1" — empty for older backends
+  apiVersion?: string // e.g. "apps/v1", "cluster.x-k8s.io/v1beta1"
   namespace: string
   name: string
   uid?: string
@@ -508,7 +508,7 @@ export interface ChartDependency {
 
 export interface HelmOwnedResource {
   kind: string
-  apiVersion?: string // e.g. "apps/v1", "cluster.x-k8s.io/v1beta1" — empty for older backends
+  apiVersion?: string // e.g. "apps/v1", "cluster.x-k8s.io/v1beta1"
   name: string
   namespace: string
   status?: string   // Running, Pending, Failed, Active, etc.

packages/k8s-ui/src/utils/resource-hierarchy.ts

@@ -232,8 +232,8 @@ export function buildResourceHierarchy(options: HierarchyOptions): ResourceLane[
         childEventCount: 0,
       })
     } else {
-      // Older SQLite-stored events may lack apiVersion (column added by migration);
-      // pick up the group from any subsequent event that carries it.
+      // Stored events may lack apiVersion; upgrade the lane's group from any
+      // later event that carries it.
       if (!existing.group) {
         const fromEvent = apiVersionToGroup(event.apiVersion)
         if (fromEvent) existing.group = fromEvent

web/src/App.tsx

@@ -592,19 +592,11 @@ function AppInner() {
   }, forceNamespaceFilter, showPolicyEffect)
   const [reconnect, isReconnecting] = useRefreshAnimation(reconnectSSE)
 
-  // Engage SSE server-side filtering on large clusters and keep the filter
-  // in lockstep with the user's pick. Two failure modes this covers:
-  //   * Header switcher: SSE was already filtered (large-cluster picker
-  //     engaged forceNamespaceFilter once); switching namespaces only updated
-  //     `namespaces`, so SSE kept streaming the previous pick → blank graph
-  //     with stale sidebar counts.
-  //   * URL bookmark on a large cluster: forceNamespaceFilter starts
-  //     undefined, SSE opens with no filter, server replies
-  //     requiresNamespaceFilter=true. The LargeClusterPicker fallback only
-  //     renders when `namespaces` is empty, so a deep link with a namespace
-  //     showed "No resources found".
-  // Small clusters never see requiresNamespaceFilter and leave
-  // forceNamespaceFilter === undefined — frontend filtering is unaffected.
+  // On large clusters (where the server requires namespace filtering), keep
+  // SSE's server-side filter in lockstep with the user's namespace pick.
+  // Without this, header switches and deep-link loads can leave SSE filtered
+  // to a stale namespace while sidebar/topology show a different one. Small
+  // clusters never set forceNamespaceFilter and skip this path entirely.
   useEffect(() => {
     const isLarge = forceNamespaceFilter !== undefined || topology?.requiresNamespaceFilter === true
     if (!isLarge) return

web/src/components/workload/WorkloadView.tsx

@@ -57,12 +57,16 @@ export function WorkloadViewRoute({ onNavigateToResource }: WorkloadViewRoutePro
   const navigate = useNavigate()
   const [searchParams] = useSearchParams()
 
-  // Parse /workload/:kind/:ns/:name from pathname
+  // Parse /workload/:kind/:ns/:name from pathname. Segments are URL-encoded by
+  // buildWorkloadPath; names can also contain literal slashes (e.g. some CRD names),
+  // which survive encoding as %2F and reassemble correctly here.
   const parts = location.pathname.replace(/^\//, '').split('/')
-  // parts[0] = 'workload', parts[1] = kind, parts[2] = ns, parts[3+] = name (may contain slashes)
-  const kind = parts[1] || ''
-  const namespace = parts[2] || ''
-  const name = parts.slice(3).join('/') || ''
+  const decode = (s: string): string => {
+    try { return decodeURIComponent(s) } catch { return s }
+  }
+  const kind = decode(parts[1] ?? '')
+  const namespace = decode(parts[2] ?? '')
+  const name = parts.slice(3).map(decode).join('/')
   const group = searchParams.get('apiGroup') || ''
 
   if (!kind || !namespace || !name) {

web/src/utils/navigation.ts

@@ -10,7 +10,10 @@ export type { NavigateToResource } from '@skyhook-io/k8s-ui/utils/navigation'
  * query param so the WorkloadView can resolve CRDs with colliding kind names.
  */
 export function buildWorkloadPath(resource: SelectedResource): string {
-  const base = `/workload/${resource.kind}/${resource.namespace}/${resource.name}`
+  const kind = encodeURIComponent(resource.kind)
+  const namespace = encodeURIComponent(resource.namespace)
+  const name = encodeURIComponent(resource.name)
+  const base = `/workload/${kind}/${namespace}/${name}`
   return resource.group ? `${base}?apiGroup=${encodeURIComponent(resource.group)}` : base
 }