From 98e2acf32bcd44c82250f11f9551ec82dcc8eaf9 Mon Sep 17 00:00:00 2001 From: Mickael Farina Date: Sun, 3 May 2026 23:39:18 +0200 Subject: [PATCH] hotfix: stop trigger + shift_report notification spam MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two correlated bugs spotted in production at 21:31-21:45 on 2026-05-03: 1. clipboard_url_fetch was the only skill with SKILL_OBSERVATION_TRIGGER and was firing every ~7 min while the user was actively copying URLs (each new URL beat the 600s per-trigger cooldown). Each fire produced a `codec-triggers is asking a question` notification with `require_confirmation: True`. Spam was burying real notifications from the agent runner. Fix: comment out the SKILL_OBSERVATION_TRIGGER block. The skill itself stays callable via voice / chat / MCP — only the auto-fire path is muted. The Phase 2 Step 6 trigger system stays operational for future skills. Added a comment explaining how to re-enable. 2. shift_report.run_with_trigger_kind only called mark_fired_today() for non-manual fires (skills/shift_report.py:529). Result: anything calling shift_report.run() manually bypassed dedup AND left no timestamp, so consecutive manual calls always succeeded. Audit log showed 8 fires in 12 min (3 within 13ms of each other at 21:40 + 2 within 4ms at 21:45) — clear button-mash or polling-loop pattern. Fix: introduce a 5-min cooldown for manual fires, gated on last_fired_at + last_trigger_kind == "manual". Per-day dedup for idle/time fires unchanged. Always call mark_fired_today (manual path now writes timestamp so cooldown can read it). Tests: - test_manual_5min_cooldown_suppresses_repeats — verifies second immediate manual fire is suppressed - test_manual_cooldown_does_not_block_after_5min — seeds a 6-min-old fire, verifies new one goes through - All 22 prior shift_report tests still pass (1 pre-existing ModuleNotFoundError on pynput unchanged, unrelated to this PR) Open follow-ups (separate tasks): - Find what's calling shift_report.run() repeatedly (likely a chat handler or auto-polling tab; the cooldown is the safety net) - Add ~/.codec/triggers.json runtime mute config so users can toggle individual triggers without editing skill files Co-Authored-By: Claude Opus 4.7 (1M context) --- skills/clipboard_url_fetch.py | 35 +++++++++++++++++++++++------------ skills/shift_report.py | 32 +++++++++++++++++++++++++++++--- tests/test_shift_report.py | 31 +++++++++++++++++++++++++++++++ 3 files changed, 83 insertions(+), 15 deletions(-) diff --git a/skills/clipboard_url_fetch.py b/skills/clipboard_url_fetch.py index bbf3523..fb0ef5d 100644 --- a/skills/clipboard_url_fetch.py +++ b/skills/clipboard_url_fetch.py @@ -39,19 +39,30 @@ SKILL_MCP_EXPOSE = False # local-only; no value over MCP since clipboard isn't shared # ────────────────────────────────────────────────────────────────────────────── -# Phase 2 Step 6 declarative trigger. -# Fires when the clipboard preview contains an http(s) URL. -# Pattern is conservative: bounded character class, no whitespace, -# no quote chars (avoids accidentally matching JSON-encoded URLs as part -# of a larger blob). +# Phase 2 Step 6 declarative trigger — DISABLED BY DEFAULT. +# +# Auto-firing on every URL copied to clipboard turned out to be too noisy in +# practice — the average user copies several URLs per minute while working, +# and `require_confirmation: True` produced one ask_user notification per +# fresh URL despite the 10-min cooldown. The trigger was firing every ~7 min +# with each new URL, blocking the agent runner queue and burying real +# notifications under "codec-triggers is asking a question" spam. +# +# Manual paths still work. Say "fetch the link" / "summarize this URL" or +# call the skill via MCP / chat / voice — `run()` reads the clipboard at +# invocation time. The skill itself is fully functional; only the +# auto-fire-on-clipboard-pattern path is muted. +# +# To re-enable, uncomment the dict below. Future work: per-skill enable +# flag in `~/.codec/triggers.json` so users toggle without editing code. # ────────────────────────────────────────────────────────────────────────────── -SKILL_OBSERVATION_TRIGGER = { - "type": "clipboard_pattern", - "pattern": r"https?://[^\s<>'\"]+", - "cooldown_seconds": 600, # 10-min per-trigger cooldown - "require_confirmation": True, # ask user before fetch - "destructive": False, # read-only operation -} +# SKILL_OBSERVATION_TRIGGER = { +# "type": "clipboard_pattern", +# "pattern": r"https?://[^\s<>'\"]+", +# "cooldown_seconds": 600, # 10-min per-trigger cooldown +# "require_confirmation": True, # ask user before fetch +# "destructive": False, # read-only operation +# } import re diff --git a/skills/shift_report.py b/skills/shift_report.py index efbd8b4..f926a9b 100644 --- a/skills/shift_report.py +++ b/skills/shift_report.py @@ -490,13 +490,38 @@ def run(task: str = "", app: str = "", ctx: str = "") -> str: return run_with_trigger_kind(trigger_kind) +_MANUAL_COOLDOWN_SECONDS = 300 # 5 min — protects against runaway loops + + +def _manual_cooldown_active() -> bool: + """Returns True if a manual shift_report fired within the last + `_MANUAL_COOLDOWN_SECONDS` seconds. Prevents button-mash and + polling-loop spam from hammering the audit log.""" + state = _load_state() + last = state.get("last_fired_at") + if not last or state.get("last_trigger_kind") != "manual": + return False + try: + last_dt = datetime.fromisoformat(last.replace("Z", "+00:00")) + except (ValueError, TypeError): + return False + elapsed = (datetime.now(timezone.utc) - last_dt).total_seconds() + return elapsed < _MANUAL_COOLDOWN_SECONDS + + def run_with_trigger_kind(trigger_kind: str) -> str: """Internal entry — used by codec_observer when it knows the trigger - kind. Per-day dedup means time AND idle on the same day fire once.""" + kind. Per-day dedup means time AND idle on the same day fire once. + Manual fires bypass per-day dedup but are protected by a 5-min + cooldown so a button-mash or polling loop can't pile up reports.""" if not _enabled(): return "Shift report is disabled." if trigger_kind != "manual" and already_fired_today(): return f"Shift report already fired today ({trigger_kind} suppressed)." + if trigger_kind == "manual" and _manual_cooldown_active(): + return ("Shift report fired in the last 5 minutes — " + "suppressed to prevent loop spam. " + "Run again after the cooldown if you need a fresh one.") # Lazy-import codec_audit so the skill loads cleanly even in stripped envs try: @@ -526,8 +551,9 @@ def _log_event(*a, **kw): report = _assemble_shift_report(trigger_kind) nid = _post_notification(report) saved_path = _maybe_auto_save(report, cfg) - if trigger_kind != "manual": - mark_fired_today(trigger_kind) + # Always mark — manual fires need the timestamp for the 5-min cooldown + # check above; idle/time still need it for per-day dedup. + mark_fired_today(trigger_kind) duration_ms = (time.monotonic() - t0) * 1000.0 try: diff --git a/tests/test_shift_report.py b/tests/test_shift_report.py index 689ae31..d0806dc 100644 --- a/tests/test_shift_report.py +++ b/tests/test_shift_report.py @@ -250,6 +250,37 @@ def test_manual_trigger_bypasses_dedup(temp_state): assert len(notifs) == 1 +def test_manual_5min_cooldown_suppresses_repeats(temp_state): + """Two manual fires within 5 min — second is suppressed to prevent + button-mash / polling-loop spam. The audit-log loop spotted on + 2026-05-03 (8 fires in 12 min) motivated this floor.""" + # First manual fire — should succeed and set last_trigger_kind=manual + result1 = shift_report.run("shift report") + assert "Shift report posted" in result1 + # Second manual fire immediately after — suppressed + result2 = shift_report.run("shift report") + assert "last 5 minutes" in result2 + notifs = json.loads((temp_state / "notifications.json").read_text()) + assert len(notifs) == 1 # only the first fire created a notification + + +def test_manual_cooldown_does_not_block_after_5min(temp_state): + """Cooldown is time-based; if the last manual fire was >5 min ago, + a new manual fire goes through.""" + from datetime import datetime, timezone, timedelta + # Seed state with a manual fire from 6 minutes ago + old = (datetime.now(timezone.utc) - timedelta(minutes=6)).isoformat(timespec="seconds") + shift_report._save_state({ + "last_fired_date": shift_report._today_local_date(), + "last_fired_at": old, + "last_trigger_kind": "manual", + }) + # New manual fire should NOT be suppressed + assert shift_report._manual_cooldown_active() is False + result = shift_report.run("shift report") + assert "Shift report posted" in result + + # ───────────────────────────────────────────────────────────────────────────── # Kill switch + config (3) # ─────────────────────────────────────────────────────────────────────────────