LGTM, how show case the fix in real life?
Do you have an example of behavior with/without the fix? Idk, maybe a clush command in tree mode, with a GW down? What would happen, the active commands would be wrong without the fix?

From AI review:

To address it cleanly, move gateway-routed ev_pickup emission from TreeWorker._execute_remote() / _copy_remote() to the point where the propagation channel actually sends the queued control message.

On top of the PR patch, I’d change PropagationChannel so shell() emits pickups only when the control message is really sent, then remove the early pickup loops from TreeWorker.

Rough patch shape:

diff --git a/lib/ClusterShell/Propagation.py b/lib/ClusterShell/Propagation.py
@@
-    def send_queued(self, ctl):
+    def _send_ctl(self, ctl, pickup_worker=None, pickup_nodes=None):
+        self.send(ctl)
+        if pickup_worker is not None:
+            for node in pickup_nodes:
+                pickup_worker._emit_pickup(node)
+
+    def send_queued(self, ctl, pickup_worker=None, pickup_nodes=None):
         """helper used to send a message, using msg queue if needed"""
         if self.setup and not self._sendq:
             # send now if channel is setup and sendq empty
-            self.send(ctl)
+            self._send_ctl(ctl, pickup_worker, pickup_nodes)
         else:
             self.logger.debug("send_queued: %d", len(self._sendq))
-            self._sendq.appendleft(ctl)
+            self._sendq.appendleft((ctl, pickup_worker, pickup_nodes))
 
     def send_dequeue(self):
         """helper used to send one queued message (if any)"""
         if self._sendq:
-            ctl = self._sendq.pop()
+            ctl, pickup_worker, pickup_nodes = self._sendq.pop()
             self.logger.debug("dequeuing sendq: %s", ctl)
-            self.send(ctl)
+            self._send_ctl(ctl, pickup_worker, pickup_nodes)
@@
-        self.send_queued(ctl)
+        self.send_queued(ctl, worker, nodes)

Then in TreeWorker, drop the new pickup loops added by the PR from both _copy_remote() and _execute_remote():

-        # Pickup is fired once the command is being sent to the gateway
-        # for each target (#594). On reroute, _launch() calls this method
-        # again so a fresh pickup is emitted per (re-)send.
-        for node in targets:
-            self._emit_pickup(node)

Same removal in _execute_remote().

One important adjustment: PropagationChannel.write() and set_write_eof() can keep calling send_queued(ctl) with no pickup args. That avoids changing their behavior.

This keeps the PR’s _emit_pickup() buffering/dedup logic intact, but moves the trigger to the channel layer, where we know whether the command left the local process instead of merely being queued for a gateway that may never come up.

Either clarify that this commit is improving but not fully fixing #594 or add the above logic?

@degremont excellent review and AI FTW. I have force-pushed a squashed commit that now does exactly that.

And by the way, I also found another simplification when iterating with my AI:

One step beyond the AI sketch: I removed _picked_up_nodes. With emission
gated by _send_ctl (only runs when setup=True) and reroute gated by
setup=False, the dedup is structurally unreachable. The 1:1 ev_pickup-
per-node invariant is held by the call sites; new
test_tree_run_gw2f1_no_double_pickup_under_reroute guards it.

Basically this means that with the existing code structure, _emit_pickup() is called at most once per node per worker lifetime. The set _picked_up_nodes is not needed anymore.

My AI warns that:

This is structural, not enforced by bytecode. It relies on three invariants holding:

1. Worker._on_start(key) is called at most once per (child, key) by the engine.
2. send_queued puts each CTL through _send_ctl exactly once (queue OR immediate, never both).
3. _relaunch only fires when setup=False (i.e., for CTLs that were never sent).

If a future refactor breaks any of those, the guarantee breaks. To catch that:

• tests/TreeWorkerTest.py::TreeWorkerGW2F1FTest::test_tree_run_gw2f1_no_double_pickup_under_reroute — upstream regression guard with a list-not-set comparison that catches duplicates.
• tests/local_tree_coverage/test_emit_pickup_at_most_once.py — local-only call-count probe that monkey-patches _emit_pickup directly.

I tested the new tests before(failed)/after(OK) so I believe they test what we want.

Finally, you asked:

how show case the fix in real life? Idk, maybe a clush command in tree mode, with a GW down?

The bug is not visible with clush itself (does not rely on ev_pickup, for example its progress counter is driven by ev_hup), but the problem is visible with a python program using ev_pickup.

For reference, the attached script cs_demo_594.py can be used to reproduce this, here are the results (this PR vs. master):

  Results

  On HEAD (commit 18533f0, deeper fix)

  Event sequence:
     0. start    None
     1. pickup   127.0.0.2
     2. hup      127.0.0.2
     3. routing  127.0.0.3
     4. pickup   127.0.0.3     ←  AFTER routing
     5. hup      127.0.0.3
     6. close    None

  rerouted target:    127.0.0.3
  routing event idx:  3
  pickup(127.0.0.3) idx: 4

  PASS: pickup fires AFTER routing (deeper fix in place)
  exit=0

  On master (commit c09f827, no PR #594 work)

  Event sequence:
     0. start    None
     1. pickup   127.0.0.2
     2. pickup   127.0.0.3     ←  fires eagerly, BEFORE the working gateway even sent
     3. hup      127.0.0.2
     4. routing  127.0.0.3
     5. hup      127.0.0.3
     6. close    None

  rerouted target:    127.0.0.3
  routing event idx:  4
  pickup(127.0.0.3) idx: 2

  FAIL: pickup fires BEFORE routing (bug present)
  exit=1

Please re-approve when you get a chance. Feels like a bit of a battle of AIs, haha. Thx!