From 60a726883be85c174e88ae832ef95f3c0f084ec9 Mon Sep 17 00:00:00 2001 From: Adam Malczewski Date: Fri, 4 Sep 2026 00:51:47 +0900 Subject: fix(plugins): make groupByTier duplicate-key-proof; normalize tiers at the boundary MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PluginTable.svelte renders a keyed each ({#each groups as group (group.tier)}); groupByTier's old "rest" bucket for unknown tiers fell back to key "standard", producing a DUPLICATE standard group (and thus an each_key_duplicate crash in Svelte 5, blanking both manager tables) whenever tierless rows were present alongside canonical standard rows. groupByTier now buckets through a Map keyed by the NORMALIZED tier — each canonical tier can yield at most one group, so duplicate keys are impossible by construction, and no row is ever dropped. Unknown/missing tiers fold into the standard bucket rather than forming a separate trailing group. The normalization (normalizeTier, in types.ts next to tierLabel/TIER_ORDER) also runs at the pluginNodes boundary, so every node's tier is canonical before any tier-keyed consumer sees it — including tierLabel's exhaustive switch, which rendered an empty badge for out-of-contract values. Normalizing into an existing bucket (option a) was chosen over a synthetic "unknown" group because it needs no new UI vocabulary and keeps every consumer total. Tests: all-canonical order, unknown tier ("kernel"), and missing tier each assert no duplicate keys, no dropped rows, and the right group/row counts from the Loaded-table view; the boundary test pins pluginNodes' canonical output. Plugins-feature vitest green (95 tests); svelte-check, biome, and vite build green. --- src/features/plugins/logic/view-model.test.ts | 102 +++++++++++++++++++++++++- 1 file changed, 100 insertions(+), 2 deletions(-) (limited to 'src/features/plugins/logic/view-model.test.ts') diff --git a/src/features/plugins/logic/view-model.test.ts b/src/features/plugins/logic/view-model.test.ts index c4a4547..680ba94 100644 --- a/src/features/plugins/logic/view-model.test.ts +++ b/src/features/plugins/logic/view-model.test.ts @@ -1,6 +1,6 @@ -import type { PluginListResponse } from "@dispatch/transport-contract"; +import type { PluginListResponse, PluginTier } from "@dispatch/transport-contract"; import { describe, expect, it } from "vitest"; -import { emptyListResponse } from "./test-fixtures"; +import { emptyListResponse, node } from "./test-fixtures"; import { groupByTier, pluginNodes, splitSections } from "./view-model"; function listResponse(overrides: Partial = {}): PluginListResponse { @@ -103,6 +103,55 @@ describe("pluginNodes", () => { it("handles the empty response", () => { expect(pluginNodes(emptyListResponse())).toEqual([]); }); + + it("normalizes a missing/unknown tier at the boundary so node tiers are canonical", () => { + // Contract-violation casts: the wire type pins three tier values, but the + // view-model must survive a backend that emits a tierless or out-of- + // contract row (the original duplicate-key crash trigger). + const response = listResponse({ + loaded: [ + { + id: "kernel", + name: "Kernel", + version: "1.0.0", + tier: "core", + trust: "bundled", + locked: true, + dependsOn: [], + }, + { + id: "tierless", + name: "Tierless", + version: "1.0.0", + tier: undefined as unknown as PluginTier, + trust: "external", + locked: false, + dependsOn: [], + }, + { + id: "kernelish", + name: "Kernelish", + version: "1.0.0", + tier: "kernel" as unknown as PluginTier, + trust: "external", + locked: false, + dependsOn: [], + }, + ], + }); + const nodes = pluginNodes(response); + expect(nodes.find((n) => n.id === "tierless")?.tier).toBe("standard"); + expect(nodes.find((n) => n.id === "kernelish")?.tier).toBe("standard"); + + // The Loaded table's grouping: exactly two groups (core + standard), no + // duplicate keys, every row present. + const groups = groupByTier(nodes.filter((n) => n.section === "loaded")); + const keys = groups.map((g) => g.tier); + expect(new Set(keys).size).toBe(keys.length); + expect(keys).toEqual(["core", "standard"]); + expect(groups[0]?.rows.map((r) => r.id)).toEqual(["kernel"]); + expect(groups[1]?.rows.map((r) => r.id)).toEqual(["kernelish", "tierless"]); + }); }); describe("groupByTier", () => { @@ -117,6 +166,55 @@ describe("groupByTier", () => { const groups = groupByTier([pluginNodes(listResponse())[2] ?? []].flat()); expect(groups.map((g) => g.tier)).toEqual(["standard"]); }); + + it("emits one group per canonical tier, all three present", () => { + const groups = groupByTier([ + node({ id: "s", tier: "standard" }), + node({ id: "c", tier: "core" }), + node({ id: "p", tier: "providers" }), + ]); + expect(groups.map((g) => g.tier)).toEqual(["core", "providers", "standard"]); + expect(groups.flatMap((g) => g.rows.map((r) => r.id))).toEqual(["c", "p", "s"]); + }); + + it("never emits duplicate group keys — an unknown tier folds into standard", () => { + // Contract violation cast: "kernel" is NOT a tier (the kernel hosts + // plugins, it is not one). groupByTier must stay total anyway. + const groups = groupByTier([ + node({ id: "s-row", tier: "standard" }), + node({ id: "k-row", tier: "kernel" as PluginTier }), + ]); + const keys = groups.map((g) => g.tier); + // No duplicates (the keyed each `(group.tier)` crashes on them)… + expect(new Set(keys).size).toBe(keys.length); + // …and the unknown-tier row is NOT dropped — it merges into standard. + expect(keys).toEqual(["standard"]); + expect(groups[0]?.rows.map((r) => r.id)).toEqual(["k-row", "s-row"]); + }); + + it("never emits duplicate group keys — a missing tier folds into standard", () => { + const groups = groupByTier([ + node({ id: "s-row", tier: "standard" }), + node({ id: "m-row", tier: undefined as unknown as PluginTier }), + ]); + const keys = groups.map((g) => g.tier); + expect(new Set(keys).size).toBe(keys.length); + expect(keys).toEqual(["standard"]); + expect(groups[0]?.rows.map((r) => r.id)).toEqual(["m-row", "s-row"]); + }); + + it("keeps unknown/missing tiers out of the canonical groups' identity (no second standard group)", () => { + // The old implementation pushed a trailing "rest" group keyed + // `rest[0]?.tier ?? "standard"` — a DUPLICATE standard key when canonical + // standard rows existed (the headless-Chromium each_key_duplicate crash). + const groups = groupByTier([ + node({ id: "a", tier: "standard" }), + node({ id: "b", tier: "bogus" as PluginTier }), + node({ id: "c", tier: undefined as unknown as PluginTier }), + ]); + expect(groups.map((g) => g.tier)).toEqual(["standard"]); + expect(groups[0]?.rows).toHaveLength(3); + }); }); describe("splitSections", () => { -- cgit v1.2.3