fix(plugins): gate hook callbacks by call identity, not by tool name alone

[deadczarvc]

Problem

_run_hook_callback_bounded keyed its busy gate on (hook_name, id(cb)) only. Two concurrent invocations of the same tool in the same session therefore collapsed into one key: the second invocation saw running and returned _HOOK_SKIPPED — as if a callback had timed out. For pre_tool_call a skip is not observational, it is a fail-closed block, so the tool call itself never ran. The gate silenced itself on an ordinary, healthy callback.

This is the concurrency half of #98382 ("Concurrent observer-hook invocations are dropped as if a callback had timed out"). It is independent of #107894, which releases the slot on timeout; this PR stops healthy concurrency from colliding in the first place.

Evidence (one busy profile, 2026-09-14)

• 3574 skipped after previous timeout or while still running lines and 0 timed out after lines in one hour — every skip came from the while still running branch, i.e. pure key collision, not slowness.
• Hook chain latency was not the factor: 0.76 s total for the whole pre_tool_call chain at idle, max 0.26 s under 6 concurrent invocations, zero hooks above threshold.
• id(cb) cannot discriminate: for shell hooks the dispatcher holds one closure per (event, matcher, command) for the life of the process, so id(cb) is constant.

Change

• The gate key becomes (hook_name, id(cb), <call identity>), where the call identity is tool_call_id, else turn_id, else None. All three are already in the payload the dispatcher receives — nothing new is plumbed, no new dependency.
• The suppression key stays coarse, (hook_name, id(cb)): a hung callback is a fact about the callback, so a timeout back-off must not be diluted to a single call, or a wedged hook would be restarted by every new call.
• api_request_id is deliberately not used: one API request carries many tool calls, which would re-collapse the keys.

Tests

tests/hermes_cli/test_plugins.py: two invariant tests, both proven to fail on the base revision.

1. test_concurrent_same_tool_calls_with_distinct_ids_both_run — two concurrent pre_tool_call invocations of the same tool with distinct tool_call_id must both run (base: the second is dropped).
2. test_repeated_same_call_identity_still_deduplicated — negative control: a repeat of the same call identity is still deduplicated, so a hung worker is never restarted by a repeat of the very same call.

tests/hermes_cli/test_plugins.py ................ 80 passed

Notes

• A timed-out invocation still leaves its gate entry behind (its call is over, so it blocks nothing new). Removing it belongs to fix(plugins): stop the hook slot from latching off for the life of the process #107894, which does exactly that under the same lock and token-identity rule; the two PRs compose without conflict.
• id(cb) remains in the key as a callback discriminator. Replacing it with a registration handle would be strictly better and is a separate change; it is not required to fix this collision.

[alt-glitch]

This was generated by AI during triage.

Related: competing fixes for #98382 with different mechanisms — #98385 (per-invocation workers), #107894 (timeout slot release), #109441 (per-session single-flight), #103119 (issue author). This PR keys the busy gate by call identity (tool_call_id/turn_id). Flagging the cluster so a maintainer can pick one approach.

[deadczarvc]

Follow-up on the same latch, one layer deeper: the gate's key is still id(cb).

callback_key = (hook_name, id(cb)) keys _hook_running_callbacks and
_hook_timeout_suppressed_until. A CPython address is reused after the object is collected, so a
fresh callback can inherit a dead one's latch and be reported as "still running" (or sit
inside a suppression window) for reasons that belonged to its predecessor. Observed live on a
fleet: one hook family skipped at ~1760/h while the process-wide gate looked healthy. A name alone
is not enough either — shell-hook callbacks are stubs that share a name, which is how the collision
was originally produced.

Proposed: key by the callback's registration slot (the dispatch loop already walks the list) +
its name, so the latch belongs to a registration, not to an address.

diff --git a/hermes_cli/plugins_dispatch.py b/hermes_cli/plugins_dispatch.py
index aca55c5e..9423cf49 100644
--- a/hermes_cli/plugins_dispatch.py
+++ b/hermes_cli/plugins_dispatch.py
@@ -187,10 +187,10 @@ class PluginDispatchMixin:
         timeout = _resolve_hook_callback_timeout()
         use_timeout = _hook_uses_callback_timeout(hook_name, timeout)
         fail_closed = hook_name in _HOOK_TIMEOUT_FAIL_CLOSED_HOOKS
-        for cb in self._hooks.get(hook_name, []):
+        for slot, cb in enumerate(self._hooks.get(hook_name, [])):
             try:
                 if use_timeout:
-                    ret = self._run_hook_callback_bounded(hook_name, cb, kwargs, timeout)
+                    ret = self._run_hook_callback_bounded(hook_name, cb, kwargs, timeout, slot)
                     if ret is _HOOK_SKIPPED:
                         if fail_closed:  # policy hook: fail closed with a block directive
                             results.append({"action": "block", "message": _PRE_TOOL_CALL_TIMEOUT_BLOCK_MESSAGE})
@@ -205,13 +205,19 @@ class PluginDispatchMixin:
         return results
 
     def _run_hook_callback_bounded(
-        self, hook_name: str, cb: Callable, kwargs: Dict[str, Any], timeout: float
+        self, hook_name: str, cb: Callable, kwargs: Dict[str, Any], timeout: float,
+        slot: int = -1,
     ) -> Any:
         """Run one callback on a daemon worker with a wall-clock cap; ``_HOOK_SKIPPED`` when
         suppressed, still running, timed out (worker abandoned, never joined), or the worker
         could not be started. Exceptions propagate."""
         callback_name = getattr(cb, "__name__", repr(cb))
-        callback_key = (hook_name, id(cb))
+        # Key by the callback registration slot + name, never by id(cb): a CPython address is
+        # reused after the object is collected, so a fresh callback can inherit a dead one
+        # latch (suppression window / in-flight token) and be skipped for reasons that
+        # belonged to its predecessor. Slots are stable under append (plugins register by
+        # appending); the name disambiguates a slot shifted by a removal.
+        callback_key = (hook_name, slot, callback_name)
         token = object()
         with self._hook_timeout_lock:
             suppressed_until = self._hook_timeout_suppressed_until.get(callback_key)

py_compile clean; the invariant worth pinning: two different callbacks under one hook name, the
first timing out — the second must still launch.

[deadczarvc]

Correction to my own follow-up above, after re-reading the code.

I overstated one part. The gate key is no longer (hook_name, id(cb)) — this PR itself widened it to
(hook_name, id(cb), tool, session, call_identity). The address-keyed map that remains is
suppression_key alone (_hook_timeout_suppressed_until). So scoping the gate back to a registration slot
would undo the call-identity fix this PR makes.

Narrower proposal, consistent with the "suppression stays coarse" invariant: key
_hook_timeout_suppressed_until by registration slot + name instead of a Python address. Same coarse
scope (a fact about the callback, not about one call), but no latch inherited from a collected predecessor.

The reuse concern itself is verified, not theoretical: re_register_config_hooks()
(agent/shell_hooks.py, #60036 / PR #60267) clears the idempotence set and re-runs register_from_config(),
which builds fresh closures — so an address freed by the previous generation of stubs can be handed to a
new one, and id(cb) is not stable across a plugin force-reload.

[kvnloo]

Exact-head pick-one cross-link on #110470 head 53b3dac.

CHECK vs KEEP #111177 head 9524343 (plugin hook call-identity gate). Close as salvaged.

Abort if head drifts.

[kvnloo]

Exact-head pick-one cross-link on #110470 head 53b3dac.

CHECK vs KEEP #111177 head 9524343 (plugin hook call-identity gate). Close as salvaged.

Abort if head drifts.

[kshitijk4poor]

Thanks @deadczarvc — this landed on main via #111177 (rebase-merged, your commit cherry-picked with authorship intact; head 73f808e47f). Closing this one in favour of the merged stack.