fix: palette-add mints unique node ids against live topology

src/App.tsx

@@ -39,7 +39,7 @@ import { Palette } from './components/Palette';
 import { Glossary } from './components/Glossary';
 import { Shortcuts } from './components/Shortcuts';
 import { Examples } from './components/Examples';
-import { cloneSubgraph, isTopology, selectionSubgraph } from './clipboard';
+import { cloneSubgraph, freshId, isTopology, selectionSubgraph } from './clipboard';
 import type { ClipboardSubgraph } from './clipboard';
 import {
   NOTE_DEFAULT_WIDTH,
@@ -1478,15 +1478,14 @@ export default function App() {
 
   const handleAddNode = useCallback(
     (kind: NodeKind, x: number, y: number) => {
+      const t = topoLiveRef.current;
       const node = makeNode(kind, x, y);
+      node.id = freshId(kind, new Set(t.nodes.map((n) => n.id)));
       history.commit('add', snapRef.current);
-      applyTopology({
-        ...topology,
-        nodes: [...topology.nodes, node],
-      });
+      applyTopology({ ...t, nodes: [...t.nodes, node] });
       setSelectedIds(new Set([node.id]));
     },
-    [applyTopology, topology, history],
+    [applyTopology, history],
   );
 
   /**

src/clipboard.test.ts

@@ -2,6 +2,7 @@ import { describe, expect, it } from 'vitest';
 import {
   buildClipboardText,
   cloneSubgraph,
+  freshId,
   isTopology,
   parseClipboardText,
   selectionSubgraph,
@@ -194,3 +195,58 @@ describe('cloneSubgraph', () => {
     expect(clones.nodes.map((n) => n.x)).toEqual(sub.nodes.map((n) => n.x));
   });
 });
+
+describe('freshId (palette-add dedup, issue #46)', () => {
+  it('two mints of the same kind against an existing topology produce unique ids', () => {
+    const existing: Topology = {
+      nodes: [
+        { ...node('client', 0, 0), kind: 'client', id: 'client' },
+        { ...node('service-1'), kind: 'service', id: 'service-1' },
+        { ...node('db', 480, 0), kind: 'db', id: 'db' },
+      ],
+      edges: [],
+    };
+
+    const used = new Set(existing.nodes.map((n) => n.id));
+    const id1 = freshId('cache', used);
+    const id2 = freshId('cache', used);
+
+    expect(id1).not.toBe(id2);
+    expect(new Set([...existing.nodes.map((n) => n.id), id1, id2]).size).toBe(5);
+  });
+
+  it('skips ids already present in the topology', () => {
+    const existing: Topology = {
+      nodes: [node('cache-1', 0, 0)],
+      edges: [],
+    };
+    // Simulate: topology already has cache-1 (from an earlier add).
+    // A second palette-add of cache must NOT mint cache-1 again.
+    const used = new Set(existing.nodes.map((n) => n.id));
+    const id = freshId('cache', used);
+    expect(id).toBe('cache-2');
+  });
+
+  it('result passes isTopology (no duplicate ids)', () => {
+    const base: Topology = {
+      nodes: [
+        { ...node('client', 0, 0), kind: 'client', id: 'client' },
+        { ...node('service-1'), kind: 'service', id: 'service-1' },
+        { ...node('db', 480, 0), kind: 'db', id: 'db' },
+      ],
+      edges: [],
+    };
+    const used = new Set(base.nodes.map((n) => n.id));
+    const id1 = freshId('cache', used);
+    const id2 = freshId('cache', used);
+    const after: Topology = {
+      nodes: [
+        ...base.nodes,
+        { ...node(id1, 240, 100), kind: 'cache', id: id1 },
+        { ...node(id2, 240, 200), kind: 'cache', id: id2 },
+      ],
+      edges: [],
+    };
+    expect(isTopology(after)).toBe(true);
+  });
+});

src/clipboard.ts

@@ -186,7 +186,7 @@ export function parseClipboardText(text: string): ClipboardSubgraph | null {
  * the clipboard may come from another tab whose counter this session never
  * saw, and a collision would silently merge two different nodes.
  */
-function freshId(kind: NodeKind, used: Set<string>): string {
+export function freshId(kind: NodeKind, used: Set<string>): string {
   let n = 1;
   while (used.has(`${kind}-${n}`)) n += 1;
   const id = `${kind}-${n}`;

src/handleAddNode.wiring.test.ts

@@ -0,0 +1,128 @@
+import { describe, expect, it } from 'vitest';
+import { freshId, isTopology } from './clipboard';
+import { makeNode } from './sim/presets';
+import type { Topology } from './sim/types';
+
+/**
+ * Guards the wiring between handleAddNode and the live topology mirror.
+ *
+ * Issue #46: `makeNode` uses a module counter that resets on load, so two
+ * palette-adds of the same kind produce duplicate ids. The fix mints ids
+ * via `freshId` against `topoLiveRef.current`. A stale-closure bug (reading
+ * the React `topology` binding instead of the live ref) causes two rapid
+ * adds to drop the first node, because the second add spreads the same
+ * pre-add snapshot.
+ *
+ * The source assertion catches regressions that the behavioral test cannot:
+ * a refactor could reintroduce the stale closure while keeping the id mint,
+ * and the behavioral test (which cannot simulate React scheduling) would
+ * still pass.
+ */
+
+const SOURCES = import.meta.glob('./App.tsx', {
+  eager: true,
+  query: '?raw',
+  import: 'default',
+}) as Record<string, string>;
+
+const APP_SRC = Object.values(SOURCES)[0] ?? '';
+
+/**
+ * Extract the handleAddNode callback body from App.tsx source.
+ *
+ * Finds `const handleAddNode = useCallback(` and captures everything up to
+ * the matching dep array closing paren.
+ */
+function extractHandleAddNode(src: string): string {
+  const marker = 'const handleAddNode = useCallback(';
+  const start = src.indexOf(marker);
+  if (start === -1) return '';
+  let depth = 0;
+  let i = start + marker.length;
+  for (; i < src.length; i++) {
+    if (src[i] === '(') depth++;
+    if (src[i] === ')') {
+      if (depth === 0) break;
+      depth--;
+    }
+  }
+  return src.slice(start, i + 1);
+}
+
+describe('handleAddNode wiring (issue #46)', () => {
+  const body = extractHandleAddNode(APP_SRC);
+
+  it('finds handleAddNode in App.tsx', () => {
+    expect(body.length).toBeGreaterThan(0);
+  });
+
+  it('builds the next topology from topoLiveRef.current, not the React topology binding', () => {
+    expect(body).toContain('topoLiveRef.current');
+    expect(body).not.toMatch(/\.\.\.\s*topology\b/);
+    expect(body).not.toMatch(/topology\.nodes/);
+  });
+
+  it('mints the id via freshId', () => {
+    expect(body).toContain('freshId');
+  });
+
+  it('does not list topology in its dependency array', () => {
+    const depsMatch = body.match(/\},\s*\[([^\]]*)\]/);
+    expect(depsMatch).not.toBeNull();
+    const deps = depsMatch![1]!;
+    expect(deps).not.toMatch(/\btopology\b/);
+  });
+});
+
+/**
+ * Behavioral test: two sequential palette-adds against the default preset
+ * (ids: client, api, db) with no intervening React render. Simulates the
+ * actual #46 path as a pure function.
+ */
+
+function simulateAddNode(
+  topo: Topology,
+  kind: Parameters<typeof makeNode>[0],
+  x: number,
+  y: number,
+): { topology: Topology; nodeId: string } {
+  const node = makeNode(kind, x, y);
+  node.id = freshId(kind, new Set(topo.nodes.map((n) => n.id)));
+  const next: Topology = { ...topo, nodes: [...topo.nodes, node] };
+  return { topology: next, nodeId: node.id };
+}
+
+describe('two sequential palette-adds (issue #46 behavioral)', () => {
+  const DEFAULT_PRESET: Topology = {
+    nodes: [
+      makeNode('client', 0, 0),
+      makeNode('service', 240, 0),
+      makeNode('db', 480, 0),
+    ],
+    edges: [
+      { id: 'client-1->service-2', from: 'client-1', to: 'service-2', weight: 1 },
+      { id: 'service-2->db-3', from: 'service-2', to: 'db-3', weight: 1 },
+    ],
+  };
+
+  it('both nodes survive, ids are unique, isTopology accepts the result', () => {
+    const after1 = simulateAddNode(DEFAULT_PRESET, 'cache', 240, 240);
+    const after2 = simulateAddNode(after1.topology, 'cache', 240, 360);
+
+    expect(after2.topology.nodes).toHaveLength(5);
+    expect(after1.nodeId).not.toBe(after2.nodeId);
+
+    const allIds = after2.topology.nodes.map((n) => n.id);
+    expect(new Set(allIds).size).toBe(allIds.length);
+
+    expect(isTopology(after2.topology)).toBe(true);
+  });
+
+  it('second add does not drop the first (stale-closure regression)', () => {
+    const after1 = simulateAddNode(DEFAULT_PRESET, 'cache', 240, 240);
+    const after2 = simulateAddNode(after1.topology, 'cache', 240, 360);
+
+    expect(after2.topology.nodes.some((n) => n.id === after1.nodeId)).toBe(true);
+    expect(after2.topology.nodes.some((n) => n.id === after2.nodeId)).toBe(true);
+  });
+});