Skip to content

Commit c823cf4

Browse files
committed
fix(hooks): persist tool stats from short-lived hook processes
PostToolUse hooks run in fresh Python processes that record exactly one tool call before exiting. With flush_interval=10, that single record_tool_call() never reached disk, leaving the Stop hook summary permanently stuck at "[CB] Xm | 0 tools | 0 errors" no matter how many tools were used in the session. - post-tool-use.py: explicit stats.flush() right after record for immediate visibility (statusline, Stop summary). - stats.py: track every live SessionStats in a module-level WeakSet and flush them via a single atexit handler. WeakSet keeps GC unchanged and avoids the per-instance handler leak that a naive atexit.register(self.method) would cause. - finalize() drops the instance from the WeakSet so the post-finalize flush sweep does not resurrect the cleaned-up file. - Module docstring documents the short-lived process pattern. - Regression tests cover: WeakSet membership, persistence via the module flush handler, finalize() removal, and the no-handler-leak invariant (20 instances must not register 20 handlers).
1 parent af0d12b commit c823cf4

3 files changed

Lines changed: 151 additions & 1 deletion

File tree

packages/claude-code-plugin/hooks/lib/stats.py

Lines changed: 57 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,25 @@
22
33
Tracks tool call count, tool names, errors, and session duration.
44
Uses fcntl.flock() for file-level locking on every IO operation.
5+
6+
Short-lived hook processes (e.g. PostToolUse) typically create a
7+
SessionStats, call record_tool_call() once, then exit. Because the
8+
default flush_interval is 10, that single call would never reach disk.
9+
Two safety nets cover this:
10+
11+
1. Callers in short-lived processes SHOULD call ``flush()`` explicitly
12+
right after recording, for immediate visibility (statusline, Stop
13+
hook summary, etc.).
14+
2. As a fallback, every live SessionStats instance is tracked in a
15+
module-level WeakSet and flushed once at interpreter exit by a single
16+
atexit handler. This avoids leaking a per-instance atexit handler and
17+
keeps GC behavior unchanged.
518
"""
19+
import atexit
620
import json
721
import os
822
import time
23+
import weakref
924
from typing import Any, Dict, List, Optional
1025

1126
try:
@@ -17,8 +32,38 @@
1732
DEFAULT_DATA_DIR = os.path.join(os.path.expanduser("~"), ".codingbuddy")
1833

1934

35+
# Track every live SessionStats instance with weak references so they
36+
# can be flushed at interpreter exit without preventing garbage
37+
# collection. Using a WeakSet means dead instances are removed
38+
# automatically; using a single module-level atexit handler avoids the
39+
# per-instance handler leak that would otherwise accumulate when many
40+
# SessionStats are created in one process.
41+
_live_instances: "weakref.WeakSet[SessionStats]" = weakref.WeakSet()
42+
43+
44+
def _flush_all_pending() -> None:
45+
"""Flush every live SessionStats instance.
46+
47+
Registered as a single atexit handler at module load time. Iterates
48+
a snapshot of the WeakSet so concurrent GC during iteration is safe.
49+
All exceptions are swallowed because atexit handlers must not raise
50+
during interpreter shutdown.
51+
"""
52+
for inst in list(_live_instances):
53+
try:
54+
inst.flush()
55+
except Exception:
56+
pass
57+
58+
59+
atexit.register(_flush_all_pending)
60+
61+
2062
class SessionStats:
21-
"""Track operational metrics for a Claude Code session."""
63+
"""Track operational metrics for a Claude Code session.
64+
65+
See module docstring for the short-lived process pattern.
66+
"""
2267

2368
def __init__(self, session_id: str, data_dir: Optional[str] = None, flush_interval: int = 10):
2469
"""Initialize stats tracker.
@@ -67,6 +112,13 @@ def __init__(self, session_id: str, data_dir: Optional[str] = None, flush_interv
67112
self._mem_tool_names: Dict[str, int] = {}
68113
self._mem_hook_timings: Dict[str, List[float]] = {}
69114

115+
# Add to the module-level WeakSet so the single atexit handler
116+
# can flush this instance on interpreter exit. WeakSet does not
117+
# prevent garbage collection; once the caller drops its last
118+
# reference (and finalize() has cleared mem state), the instance
119+
# disappears from the set automatically.
120+
_live_instances.add(self)
121+
70122
def record_hook_timing(self, hook_name: str, elapsed_ms: float) -> None:
71123
"""Record a hook execution timing in memory.
72124
@@ -205,6 +257,10 @@ def finalize(self) -> Dict[str, Any]:
205257
except OSError:
206258
pass
207259

260+
# Remove from the live set so the atexit flush does not resurrect
261+
# the file we just removed.
262+
_live_instances.discard(self)
263+
208264
return data
209265

210266
@staticmethod

packages/claude-code-plugin/hooks/post-tool-use.py

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,13 @@ def handle_post_tool_use(data: dict):
3434
stats = SessionStats(session_id=session_id)
3535
tool_name = data.get("tool_name", "unknown")
3636
stats.record_tool_call(tool_name, success=True)
37+
# PostToolUse runs in a fresh short-lived process per call. The
38+
# default flush_interval (10) means a single record_tool_call would
39+
# never reach disk. Flush immediately so subsequent reads (statusline,
40+
# Stop hook summary) see accurate counts. atexit in SessionStats is a
41+
# safety net, but explicit flush keeps the disk state observable
42+
# right after this hook exits.
43+
stats.flush()
3744
except Exception:
3845
pass # Never block tool execution
3946

packages/claude-code-plugin/tests/test_stats.py

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -207,6 +207,93 @@ def test_format_summary_uses_memory_data(self, data_dir):
207207
assert "Bash:2" in summary
208208

209209

210+
class TestShortLivedProcess:
211+
"""Regression tests for short-lived hook process pattern.
212+
213+
Each hook invocation (e.g. PostToolUse) is a fresh Python process that
214+
creates one SessionStats, calls record_tool_call() exactly once, then
215+
exits. With flush_interval > 1 and no explicit flush, the in-memory
216+
increment was lost — leaving disk counters stuck at 0 for the entire
217+
session. This produced statusline/stop-summary output like
218+
`[CB] Xm | 0 tools | 0 errors` even after many tool calls.
219+
220+
The library now tracks every live SessionStats in a module-level
221+
WeakSet and flushes them via a single atexit handler. These tests
222+
drive that handler directly (rather than calling
223+
``atexit._run_exitfuncs()``) so they do not interfere with other
224+
tests' atexit registrations.
225+
"""
226+
227+
def test_instance_added_to_live_set(self, data_dir):
228+
"""Newly constructed SessionStats must be tracked for atexit flush."""
229+
from stats import _live_instances
230+
231+
s = SessionStats(
232+
session_id="weakset-test", data_dir=data_dir, flush_interval=10
233+
)
234+
assert s in _live_instances
235+
236+
def test_record_persists_via_module_atexit_handler(self, data_dir):
237+
"""Records should reach disk via _flush_all_pending() when caller forgets."""
238+
from stats import _flush_all_pending
239+
240+
s1 = SessionStats(
241+
session_id="exit-flush-test", data_dir=data_dir, flush_interval=10
242+
)
243+
s1.record_tool_call("Bash")
244+
# Caller does NOT call flush() — simulates short-lived hook process.
245+
# Drive the same code path the atexit handler would run.
246+
_flush_all_pending()
247+
248+
# Fresh instance reads from disk only — must see the recorded call.
249+
s2 = SessionStats(
250+
session_id="exit-flush-test", data_dir=data_dir, flush_interval=10
251+
)
252+
on_disk = s2._locked_read()
253+
assert on_disk["tool_count"] == 1
254+
assert on_disk["tool_names"]["Bash"] == 1
255+
256+
def test_finalize_removes_instance_from_live_set(self, data_dir):
257+
"""finalize() removes the file AND drops the instance from the live set."""
258+
from stats import _flush_all_pending, _live_instances
259+
260+
s = SessionStats(
261+
session_id="finalize-weakset", data_dir=data_dir, flush_interval=10
262+
)
263+
s.record_tool_call("Bash")
264+
s.finalize()
265+
stats_file = os.path.join(data_dir, "finalize-weakset.json")
266+
assert not os.path.exists(stats_file)
267+
assert s not in _live_instances
268+
269+
# Subsequent flush sweeps must NOT recreate the file
270+
_flush_all_pending()
271+
assert not os.path.exists(stats_file)
272+
273+
def test_only_one_atexit_handler_regardless_of_instance_count(self, data_dir):
274+
"""Library must register a single atexit handler, not one per instance."""
275+
import atexit
276+
277+
# Snapshot atexit handler count, create many instances, snapshot again.
278+
# The implementation uses a module-level WeakSet + single handler,
279+
# so the count should not grow with instance creation.
280+
before = len(getattr(atexit, "_exithandlers", []))
281+
instances = [
282+
SessionStats(
283+
session_id=f"leak-test-{i}",
284+
data_dir=data_dir,
285+
flush_interval=10,
286+
)
287+
for i in range(20)
288+
]
289+
after = len(getattr(atexit, "_exithandlers", []))
290+
# Allow for at most the single module-level registration that may
291+
# have happened on first import in this test session.
292+
assert after - before <= 1
293+
# Keep references alive until the assertion runs
294+
assert len(instances) == 20
295+
296+
210297
class TestHookTimingIntegration:
211298
"""Tests for hook timing integration in SessionStats (#945)."""
212299

0 commit comments

Comments
 (0)