fix: resolve event dispatch divergence in ObserverManager and add tests

src/main/java/org/tomitribe/pixie/observer/ObserverManager.java

@@ -40,7 +40,6 @@
 import java.util.Map;
 import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
-import java.util.concurrent.CopyOnWriteArrayList;
 import java.util.concurrent.atomic.AtomicReference;
 import java.util.function.Consumer;
 import java.util.logging.Level;
@@ -59,7 +58,6 @@ protected Set<Invocation> initialValue() {
     private static final AtomicReference<Logger> LOGGER = new AtomicReference<>();
     private final Set<Observer> observers = new LinkedHashSet<>();
     private final Map<Class, Invocation> methods = new ConcurrentHashMap<>();
-    private final List<ConsumerReference> references = new CopyOnWriteArrayList<>();
 
 
     public boolean addObserver(final Object observer) {
@@ -71,7 +69,6 @@ public boolean addObserver(final Object observer) {
             final Observer wrapper = new Observer(observer);
             if (wrapper.hasObserverMethods() && observers.add(wrapper)) {
                 methods.clear();
-                references.stream().forEach(ConsumerReference::clear);
                 fireEvent(new ObserverAdded(observer));
                 return true;
             } else {
@@ -89,7 +86,6 @@ public boolean removeObserver(final Object observer) {
         try {
             if (observers.remove(new Observer(observer))) {
                 methods.clear();
-                references.stream().forEach(ConsumerReference::clear);
                 fireEvent(new ObserverRemoved(observer));
                 return true;
             } else {
@@ -114,15 +110,12 @@ public <E> E fireEvent(final E event) {
 
     public <E> Consumer<E> consumersOf(final Class<E> eventClass) {
         if (eventClass == null) throw new IllegalArgumentException("eventClass cannot be null");
-        final ConsumerReference e = new ConsumerReference(eventClass);
-        references.add(e);
-        return e;
+        return new ConsumerReference(eventClass);
     }
 
     private class ConsumerReference<E> implements Consumer<E> {
 
         private final Class<E> type;
-        private final AtomicReference<Invocation> invocation = new AtomicReference<>();
 
         private ConsumerReference(final Class<E> type) {
             if (type == null) throw new IllegalArgumentException("type cannot be null");
@@ -131,32 +124,21 @@ private ConsumerReference(final Class<E> type) {
 
         @Override
         public void accept(final E e) {
-            try {
-                getInvocation().invoke(e);
-
-            } finally {
-                seen.remove();
+            if (e != null && !type.isInstance(e)) {
+                throw new IllegalArgumentException(
+                        "event " + e.getClass().getName() + " is not a " + type.getName());
             }
-        }
-
-        private Invocation getInvocation() {
-            return this.invocation.updateAndGet(this::resolve);
-        }
-
-        private Invocation resolve(final Invocation invocation) {
-            if (invocation != null) return invocation;
-            return ObserverManager.this.getInvocation(type);
-        }
-
-        private void clear() {
-            invocation.set(null);
+            // Dispatch on the runtime type, exactly like fireEvent, so observers registered
+            // on a subtype of the static type T are reached. The static type is kept only as
+            // a compile-time bound and the runtime assertion above.
+            fireEvent(e);
         }
 
         @Override
         public String toString() {
             return "ConsumerReference{" +
                     "type=" + type.getName() +
-                    "} " + getInvocation();
+                    "} " + ObserverManager.this.getInvocation(type);
         }
     }
 

src/test/java/org/tomitribe/pixie/EventDispatchDivergenceTest.java

@@ -0,0 +1,156 @@
+/*
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.tomitribe.pixie;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+import java.util.function.Consumer;
+
+/**
+ * Reproduces the divergence described in HANDOFF-event-dispatch-divergence.md.
+ *
+ * Publishing the SAME event object reaches DIFFERENT observers depending on how
+ * it is published:
+ *
+ *   - system.fireEvent(event)          -> dispatches on event.getClass() (runtime type)
+ *   - @Event Consumer<T> bus; bus.accept(event) -> dispatches on the static generic T
+ *
+ * A Consumer<Object> therefore starts its up-walk at Object and never reaches an
+ * observer registered under a subtype such as PatchCreated.
+ *
+ * These tests assert the EXPECTED (consistent) behaviour: both publish paths should
+ * dispatch on the event's runtime type. They FAIL today, pinning the bug.
+ *
+ * The event hierarchy also has a SIBLING subtype PatchUpdated (both extend PatchChanged).
+ * Publishing a PatchCreated must never reach an observer registered only on PatchUpdated:
+ * dispatch walks UP the hierarchy (to supertypes), never sideways to siblings nor down to
+ * subtypes. This guards against the "over-deliver" fix the handoff warns against.
+ */
+public class EventDispatchDivergenceTest extends Assert {
+
+    /** Control: fireEvent dispatches on the runtime type, so the concrete observer fires. */
+    @Test
+    public void fireEventReachesConcreteObserver() {
+        final System system = System.builder()
+                .definition(Probe.class)
+                .definition(Logger.class)
+                .definition(Publisher.class)
+                .build();
+        final Probe probe = system.get(Probe.class);
+        final Logger logger = system.get(Logger.class);
+        final Publisher publisher = system.get(Publisher.class);
+        final int anyBefore = probe.any;
+        final int loggerBefore = logger.any;
+
+        publisher.fireEvent(new PatchCreated());
+
+        assertEquals("concrete observer should fire via fireEvent", 1, probe.created);
+        assertEquals("sibling observer must not fire for a PatchCreated", 0, probe.updated);
+        assertEquals("Object observer in the SAME object is shadowed by the more-specific onCreated",
+                anyBefore, probe.any);
+        assertEquals("Object observer in a SEPARATE component should still fire",
+                loggerBefore + 1, logger.any);
+    }
+
+    /**
+     * Bug: publishing a PatchCreated through an injected Consumer<Object> dispatches on
+     * the static type Object, so the @Observes PatchCreated observer is never invoked.
+     *
+     * Expected behaviour (asserted here) mirrors fireEvent: the concrete observer fires.
+     * FAILS today (probe.created == 0).
+     */
+    @Test
+    public void consumerOfObjectReachesConcreteObserver() {
+        final System system = System.builder()
+                .definition(Probe.class)
+                .definition(Logger.class)
+                .definition(Publisher.class)
+                .build();
+        final Probe probe = system.get(Probe.class);
+        final Logger logger = system.get(Logger.class);
+        final Publisher publisher = system.get(Publisher.class);
+        final int anyBefore = probe.any;
+        final int loggerBefore = logger.any;
+
+        publisher.accept(new PatchCreated());
+
+        assertEquals("concrete observer should fire via Consumer<Object>", 1, probe.created);
+        assertEquals("sibling observer must not fire for a PatchCreated", 0, probe.updated);
+        assertEquals("Object observer in the SAME object is shadowed by the more-specific onCreated",
+                anyBefore, probe.any);
+        assertEquals("Object observer in a SEPARATE component should still fire",
+                loggerBefore + 1, logger.any);
+    }
+
+    // --- event hierarchy: PatchCreated extends PatchChanged (mirrors harminie) ---
+
+    public static class PatchChanged {
+    }
+
+    public static class PatchCreated extends PatchChanged {
+    }
+
+    public static class PatchUpdated extends PatchChanged {
+    }
+
+    public static class Probe {
+        private int created;
+        private int updated;
+        private int any;
+
+        public void onCreated(@Observes final PatchCreated e) {
+            created++;
+        }
+
+        public void onUpdated(@Observes final PatchUpdated e) {
+            updated++;
+        }
+
+        public void onAny(@Observes final Object e) {
+            any++;
+        }
+    }
+
+    /**
+     * A SEPARATE observer (distinct from Probe) that catches every event via @Observes Object.
+     * Because most-specific matching is per-observer, this one is NOT shadowed by Probe.onCreated:
+     * a distinct observer contributes its own best match, so it should fire for a PatchCreated too.
+     */
+    public static class Logger {
+        private int any;
+
+        public void onAny(@Observes final Object e) {
+            any++;
+        }
+    }
+
+    public static class Publisher {
+        private final Consumer<Object> bus;
+        private final System system;
+
+        public Publisher(@Event final Consumer<Object> bus, @Component @Param("system") final System system) {
+            this.bus = bus;
+            this.system = system;
+        }
+
+        public void accept(final Object e) {
+            bus.accept(e);
+        }
+
+        public void fireEvent(final Object e) {
+            system.fireEvent(e);
+        }
+    }
+}