From 4a46cd6c95635c6a66a8683ce2c7c4babe61586d Mon Sep 17 00:00:00 2001
From: David Montero <davidmonterocrespo24@gmail.com>
Date: Fri, 19 Jun 2026 19:49:12 +0200
Subject: [PATCH] fix(simulator): wire colour change from UI was a no-op
 (recordUpdateWire applyNow)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

recordUpdateWire pushed its command with { applyNow: false }, so it recorded the
change for undo but never executed it. Its only callers (the wire colour palette
and the new right-click menu) pass the new colour and expect it applied — neither
pre-applies via the raw updateWire mutator. Net result: changing a wire colour
from the UI did nothing (only the 0-9/c/l/m/p/y keyboard shortcut, which calls
updateWire directly, worked). Drop applyNow:false so it applies like every other
record* command (recordRemoveWire etc.). Adds an undo/redo regression test.
---
 frontend/src/__tests__/undo-redo.test.ts | 16 ++++++++++++
 frontend/src/store/useSimulatorStore.ts  | 31 +++++++++++++-----------
 2 files changed, 33 insertions(+), 14 deletions(-)

diff --git a/frontend/src/__tests__/undo-redo.test.ts b/frontend/src/__tests__/undo-redo.test.ts
index bf9b2d3c..47402638 100644
--- a/frontend/src/__tests__/undo-redo.test.ts
+++ b/frontend/src/__tests__/undo-redo.test.ts
@@ -127,6 +127,22 @@ describe('recordRemoveComponent (cascade)', () => {
   });
 });
 
+describe('recordUpdateWire', () => {
+  it('applies the colour change immediately and is undoable / redoable', () => {
+    const s = useSimulatorStore.getState();
+    s.addWire(wire('w1', 'a', 'b')); // starts #22c55e
+    s.recordUpdateWire('w1', { color: '#22c55e' }, { color: '#cc0000' });
+    // Regression: recordUpdateWire used to pass applyNow:false, so the UI
+    // colour change (palette / right-click menu) recorded but never applied.
+    const c1 = () => useSimulatorStore.getState().wires.find((w) => w.id === 'w1')?.color;
+    expect(c1()).toBe('#cc0000');
+    s.undo();
+    expect(c1()).toBe('#22c55e');
+    s.redo();
+    expect(c1()).toBe('#cc0000');
+  });
+});
+
 describe('recordMove', () => {
   it('captures from/to, undo restores from, redo restores to', () => {
     const s = useSimulatorStore.getState();
diff --git a/frontend/src/store/useSimulatorStore.ts b/frontend/src/store/useSimulatorStore.ts
index 17d6089b..15753789 100644
--- a/frontend/src/store/useSimulatorStore.ts
+++ b/frontend/src/store/useSimulatorStore.ts
@@ -2776,20 +2776,23 @@ export const useSimulatorStore = create<SimulatorState>((set, get) => {
     },
 
     recordUpdateWire: (wireId, prev, next, description = 'Update wire') => {
-      get().pushCommand(
-        {
-          description,
-          execute: () =>
-            set((s) => ({
-              wires: s.wires.map((w) => (w.id === wireId ? { ...w, ...next } : w)),
-            })),
-          undo: () =>
-            set((s) => ({
-              wires: s.wires.map((w) => (w.id === wireId ? { ...w, ...prev } : w)),
-            })),
-        },
-        { applyNow: false },
-      );
+      // applyNow defaults to true: both callers (the wire color palette and the
+      // wire right-click menu) pass the new value and expect it applied — they
+      // do NOT pre-apply via the raw updateWire mutator. The old `applyNow:false`
+      // recorded the change for undo but never executed it, so changing a wire
+      // colour from the UI was a silent no-op (only the keyboard shortcut, which
+      // calls updateWire directly, actually worked).
+      get().pushCommand({
+        description,
+        execute: () =>
+          set((s) => ({
+            wires: s.wires.map((w) => (w.id === wireId ? { ...w, ...next } : w)),
+          })),
+        undo: () =>
+          set((s) => ({
+            wires: s.wires.map((w) => (w.id === wireId ? { ...w, ...prev } : w)),
+          })),
+      });
     },
 
     toggleSerialMonitor: () => set((s) => ({ serialMonitorOpen: !s.serialMonitorOpen })),