From 96e29fbc5ba36dcc1cdd0875f8adf9c985e9182b Mon Sep 17 00:00:00 2001 From: chelsealong Date: Mon, 24 Aug 2026 04:04:48 +0000 Subject: [PATCH] fix(terminal): cap watch_patterns notifications over a process's lifetime MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per-session rate limiting only counts consecutive strike windows, so a pattern that recurs at a cadence just above WATCH_MIN_INTERVAL_SECONDS (e.g. a service restarted repeatedly over a day) never trips the existing strike-limit disable — each match lands in its own clean cooldown window. Every one of those matches still forces a full-context agent turn, which stalls the event loop on large sessions (#93513). Add WATCH_LIFETIME_MAX_HITS: once a session has delivered this many watch_match notifications over its whole life, disable watch_patterns and fall back to notify_on_complete, reusing the existing disable path. --- tests/tools/test_watch_patterns.py | 40 +++++++++++++++++++++++++ tools/process_registry.py | 47 ++++++++++++++++++++++++++++++ tools/terminal_tool.py | 2 +- 3 files changed, 88 insertions(+), 1 deletion(-) diff --git a/tests/tools/test_watch_patterns.py b/tests/tools/test_watch_patterns.py index 760e3ede6aa97..8ad936e91cee6 100644 --- a/tests/tools/test_watch_patterns.py +++ b/tests/tools/test_watch_patterns.py @@ -135,6 +135,46 @@ def test_suppressed_count_in_next_delivery(self, registry): assert session._watch_suppressed == 0 # reset after delivery +# ========================================================================= +# Lifetime cap: sparsely-spaced matches never trip the consecutive-strike +# counter, but must still stop eventually (#93513). +# ========================================================================= + +class TestLifetimeCap: + def test_sparse_matches_disable_after_lifetime_cap(self, registry): + """Matches spaced well past the cooldown never strike, but a service + restarted over and over must still get disabled instead of forcing a + full-context turn forever.""" + from tools.process_registry import WATCH_LIFETIME_MAX_HITS + + session = _make_session(watch_patterns=["Application started"]) + for i in range(WATCH_LIFETIME_MAX_HITS): + # Force the cooldown to have already expired, simulating a + # cleanly-spaced restart (no strikes should ever accumulate). + session._watch_cooldown_until = 0.0 + registry._check_watch_patterns(session, "Application started\n") + + assert session._watch_hits == WATCH_LIFETIME_MAX_HITS + assert session._watch_consecutive_strikes == 0 # never struck + assert session._watch_disabled is True + assert session.notify_on_complete is True + + events = [] + while not registry.completion_queue.empty(): + events.append(registry.completion_queue.get_nowait()) + match_events = [e for e in events if e["type"] == "watch_match"] + disabled_events = [e for e in events if e["type"] == "watch_disabled"] + assert len(match_events) == WATCH_LIFETIME_MAX_HITS + assert len(disabled_events) == 1 + assert "lifetime cap" in disabled_events[0]["message"] + + # One more restart after the cap: no further turns are forced. + session._watch_cooldown_until = 0.0 + registry._check_watch_patterns(session, "Application started\n") + assert registry.completion_queue.empty() + assert session._watch_hits == WATCH_LIFETIME_MAX_HITS + + # ========================================================================= # Checkpoint persistence # ========================================================================= diff --git a/tools/process_registry.py b/tools/process_registry.py index e275f418476a5..aa8523a1c8515 100644 --- a/tools/process_registry.py +++ b/tools/process_registry.py @@ -73,6 +73,16 @@ WATCH_MIN_INTERVAL_SECONDS = 15 # Minimum spacing between consecutive watch matches WATCH_STRIKE_LIMIT = 3 # Strikes in a row → disable watch + promote to notify_on_complete +# Lifetime cap — independent of the strike counter above. A process whose +# pattern recurs at a cadence just above WATCH_MIN_INTERVAL_SECONDS (e.g. a +# service restarted repeatedly over a day) never trips the consecutive-strike +# limit, since each match lands in its own clean cooldown window, yet still +# forces a full-context agent turn every single time (#93513). watch_patterns +# is documented as "ONLY for rare one-shot mid-process signals", so once a +# session has delivered this many matches over its whole life we disable it +# and fall back to notify_on_complete, same as the strike-limit path. +WATCH_LIFETIME_MAX_HITS = 8 + # Global circuit breaker — across all sessions. Secondary safety net so concurrent # siblings can't collectively flood the user even when each is under its own cap. WATCH_GLOBAL_MAX_PER_WINDOW = 15 @@ -526,6 +536,12 @@ def _check_watch_patterns(self, session: ProcessSession, new_text: str) -> None: disabled for this session and the session is promoted to notify_on_complete semantics — one notification when the process actually exits, no more mid-process spam. + + Independently, WATCH_LIFETIME_MAX_HITS caps the total number of + matches ever delivered for a session, so a pattern that keeps + recurring at a cadence just above the cooldown (e.g. a service + restarted repeatedly over a day) still gets disabled instead of + forcing a full-context agent turn indefinitely. """ if not session.watch_patterns or session._watch_disabled: return @@ -552,6 +568,7 @@ def _check_watch_patterns(self, session: ProcessSession, new_text: str) -> None: now = time.time() should_disable = False + lifetime_exhausted = False with session._lock: # Case 1: still inside the cooldown from the last emission. # Count this as a strike for the current window (only once per window) @@ -590,6 +607,13 @@ def _check_watch_patterns(self, session: ProcessSession, new_text: str) -> None: suppressed = session._watch_suppressed session._watch_suppressed = 0 return_early = False + # Lifetime cap: this match is delivered (it already earned it), + # but disable further ones regardless of how cleanly spaced + # they are — see WATCH_LIFETIME_MAX_HITS above. + lifetime_exhausted = session._watch_hits >= WATCH_LIFETIME_MAX_HITS + if lifetime_exhausted: + session._watch_disabled = True + session.notify_on_complete = True if return_early: if should_disable: @@ -645,6 +669,29 @@ def _check_watch_patterns(self, session: ProcessSession, new_text: str) -> None: _redact_process_result(notification) self.completion_queue.put(notification) + if lifetime_exhausted: + # Same "why things went quiet" summary as the strike-limit path, + # queued right after the final delivered match. + self.completion_queue.put({ + "session_id": session.id, + "session_key": session.session_key, + "command": session.command, + "type": "watch_disabled", + "suppressed": 0, + "platform": session.watcher_platform, + "chat_id": session.watcher_chat_id, + "user_id": session.watcher_user_id, + "user_name": session.watcher_user_name, + "thread_id": session.watcher_thread_id, + "message_id": session.watcher_message_id, + "message": ( + f"Watch patterns disabled for process {session.id} — " + f"reached the lifetime cap of {WATCH_LIFETIME_MAX_HITS} delivered " + f"matches. Falling back to notify_on_complete semantics; you'll get " + f"exactly one notification when the process exits." + ), + }) + def _global_watch_admit(self, now: float) -> bool: """Return True if this watch_match event is allowed through the global breaker. diff --git a/tools/terminal_tool.py b/tools/terminal_tool.py index 69089a3722d49..7d70f79000023 100644 --- a/tools/terminal_tool.py +++ b/tools/terminal_tool.py @@ -3971,7 +3971,7 @@ def check_terminal_requirements() -> bool: "watch_patterns": { "type": "array", "items": {"type": "string"}, - "description": "Strings to watch for in background output. ONLY for rare one-shot mid-process signals on processes that never exit (e.g. ['Application startup complete'] on a server). NOT for end-of-run markers (use notify_on_complete) and NOT for per-iteration patterns like 'ERROR' in loops — rate-limited to 1 notification/15s; repeated over-firing auto-disables it and falls back to notify-on-exit. When in doubt, use notify_on_complete. MUTUALLY EXCLUSIVE with notify_on_complete." + "description": "Strings to watch for in background output. ONLY for rare one-shot mid-process signals on processes that never exit (e.g. ['Application startup complete'] on a server). NOT for end-of-run markers (use notify_on_complete) and NOT for per-iteration patterns like 'ERROR' in loops — rate-limited to 1 notification/15s, capped at a small number of matches over the process's lifetime; over-firing auto-disables it and falls back to notify-on-exit. When in doubt, use notify_on_complete. MUTUALLY EXCLUSIVE with notify_on_complete." } }, "required": ["command"]