feat(cron): token leak mitigation — instrumentation, max_tokens cap, background-review skip - #18255
feat(cron): token leak mitigation — instrumentation, max_tokens cap, background-review skip#182550xarkstar wants to merge 5 commits into
Conversation
382b6dd to
52766ea
Compare
52766ea to
0cf03ac
Compare
Cron agents on small/open-source models can fall into degenerate-loop hallucinations (e.g. "output is not output is not..." repetition trap) that produce 64KB+ of repetitive text before the API max-tokens limit fires. When combined with cron deliver mechanism, this gets split into 30+ Discord messages via Hermes scheduler — channel spam. Add HERMES_CRON_MAX_TOKENS env var that, when set, caps the AIAgent max_tokens for cron jobs. Default None preserves existing behavior. With HERMES_CRON_MAX_TOKENS=2048, worst-case hallucination is bounded to ~6000 chars (3 Discord messages) instead of 33+ message bursts. Real-world incident: BlueNode bot (Gemma 4 31B) produced 33+ chunk burst at 22:08 KST 2026-04-29 from a single repetition-trapped fire. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ntation Phase 0.5 of the Hermes Agent token leak mitigation plan: append a single JSONL line to ~/.hermes/cron/usage_audit.jsonl after every cron LLM invocation, capturing prompt/completion/total tokens, model, duration_ms, deliver target, and error (when raised). Read from agent.session_*_tokens which run_conversation already returns in its result dict. Without this, we have no measured baseline to attribute token deltas to subsequent mitigation phases. The plan's hard gate: observability lands before any mitigation phase. Writer NEVER raises — wrapped in a single try/except that logs a warning on any json.dumps / mkdir / open failure so an audit-log bug cannot break a cron job. Failure-path audit guard via locals() check covers exceptions that fire before the fire_id is assigned. No new dependencies, no new env vars (the plan rejected one in v2). Tests: 7 new unit tests in tests/cron/test_usage_audit_logger.py covering the success path, missing token info, swallowed writer exception, parent dir creation, multiple appends, and unicode preservation. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Phase 8 of the Hermes Agent token leak mitigation plan
(ralplan-hermes-token-leaks.md §3.9). Adds a boolean kwarg
`skip_background_review` (default False) to AIAgent.__init__ that
suppresses the end-of-turn _spawn_background_review fork.
Each background review fork instantiates a new AIAgent with its own
~15K input tokens + up to 8 LLM iterations, accumulating ~30K tokens
per event in the worst case. On cron sessions there is no
human-in-the-loop benefit from the review (no skill-creation pressure,
nobody curating MEMORY.md), so the cost is pure waste.
The end-of-turn guard now reads:
if (final_response and not interrupted
and not getattr(self, "skip_background_review", False)
and (_should_review_memory or _should_review_skills)):
skip_memory=True already disables the memory-review trigger; this
flag is the explicit single-switch off for both review paths.
Defaults to False, so behavior is unchanged for gateway/CLI callers
that omit the kwarg.
Tests: 5 new unit tests in tests/agent/test_skip_background_review.py
covering the default value, flag persistence, the gate short-circuit,
the gate fall-through, and a source-text assertion that the cron
scheduler sets the flag to True (separate commit).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…-presence
Phase 8 wire-in + Vector 8 doc comment from
ralplan-hermes-token-leaks.md.
(1) Phase 8 wire-in: cron AIAgent construction now passes
skip_background_review=True. This suppresses the end-of-turn
skill/memory review fork (~30K tokens/event, ≤30K typical and
≤150K worst-case daily on bluenode) which has no human-in-the-loop
value for cron sessions.
(2) Vector 8 doc comment: a one-line comment immediately above the
AIAgent(...) construction documenting the verified-negative
finding that title generation does not run on the cron path
(maybe_auto_title is gateway/CLI-side only). Future contributors
won't accidentally introduce title-gen here without realizing it
would add ~600-1000 tokens/fire on a path that explicitly opts out
of memory/review/title overhead.
No new tests required for the doc comment (no behavior change). The
skip_background_review wiring is covered by the existing source-text
assertion in tests/agent/test_skip_background_review.py.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Documents the four items intentionally deferred from ralplan-hermes-token-leaks v3 (2026-05-01) Wave 1 to give Wave 2 contributors a starting point with explicit gates and rationale: - Phase 4.3 — tool description compaction (gated on a 50-prompt tool-selection regression suite reporting ≤2% accuracy delta). - Phase 5 — SKILL.md content cache via provider cache_control breakpoints (blocked on bluenode running gemma-on-Nous-Portal which doesn't support breakpoints; revisit on Claude migration or Nous-Portal feature parity). - Phase 6 — precheck_command schema for cron (eliminates the cron-LLM-mandatory architecture; requires upstream RFC + cross-team approval, target 2026-05-15 RFC filing). - Vector 11 — conversation history compression on cron (Wave 3 target). Each entry records the gate, owner / target where known, and the specific blocker preventing immediate landing. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
0cf03ac to
bdb9fed
Compare
|
Rebased onto current main (2026-06-11). Notes on conflict resolution:
Related: #16530 is now closed — upstream's plugin replatform + |
teknium1
left a comment
There was a problem hiding this comment.
Thanks for isolating the cron review-suppression and audit work. The background-review premise remains present on current main: cron constructs AIAgent without a skip flag (cron/scheduler.py:3044-3075), while finalize_turn can still call _spawn_background_review (agent/turn_finalizer.py:454-480).
Problems
HERMES_CRON_MAX_TOKENSis a user-facingmax_tokensknob (cron/scheduler.py:1761-1768in this PR), which conflicts with the standing no-user-facing-max-tokens policy.- The audit path hardcodes
Path.home() / ".hermes"(cron/scheduler.py:224-225), bypassing the profile-aware_get_hermes_home()resolver used by the scheduler on current main (cron/scheduler.py:542-551). - The review-skip tests repeat the desired condition rather than exercise
finalize_turn(tests/agent/test_skip_background_review.py:64-76), so they would still pass if the production guard diverged.
Suggested changes
- Salvage the review-suppression and profile-safe auditing portions separately; remove the max-token-cap portion.
- Resolve the audit path through
_get_hermes_home()and cover a non-default profile. - Test the actual finalizer call path with
_spawn_background_reviewpatched.
Automated hermes-sweeper review.
| # hallucination is bounded to e.g. ~6000 chars (3 Discord messages) | ||
| # instead of 33+ message bursts via cron deliver mechanism. | ||
| # Set HERMES_CRON_MAX_TOKENS=2048 (or similar) to enable. | ||
| _cron_max_tokens_env = os.getenv("HERMES_CRON_MAX_TOKENS", "").strip() |
There was a problem hiding this comment.
Blocking: this introduces a user-facing max_tokens knob through HERMES_CRON_MAX_TOKENS. The standing policy disallows user-facing max-token configuration, so this portion needs to be removed rather than moved to another configuration surface.
| # Phase 0.5 token-leak instrumentation: per-fire usage audit log. | ||
| # Resolved lazily via Path.home() so test envs that override HOME work. | ||
| def _usage_audit_path() -> Path: | ||
| return Path.home() / ".hermes" / "cron" / "usage_audit.jsonl" |
There was a problem hiding this comment.
Use the scheduler's existing _get_hermes_home() resolver here. Hardcoding Path.home() / '.hermes' writes audit records outside the active non-default profile; current main documents _get_hermes_home() as the profile-scoped cron resolver at cron/scheduler.py:542-551.
|
|
||
| with patch.object(agent, "_spawn_background_review") as mock_spawn: | ||
| # This is the exact guard from run_agent.py end-of-turn block. | ||
| if ( |
There was a problem hiding this comment.
This duplicates the intended predicate instead of exercising agent.turn_finalizer.finalize_turn, so it cannot detect a production-site regression. Patch _spawn_background_review and invoke the actual finalizer with eligible review conditions.
- Fix _usage_audit_path() to use _get_hermes_home() instead of hardcoded Path.home() / '.hermes' (profile-safe resolution, sweeper finding) - Rewrite skip_background_review tests to exercise finalize_turn() directly instead of duplicating the guard expression (sweeper finding) - Fix response_silent audit field to use _is_cron_silence_response() instead of the buggy SILENT_MARKER substring check it was meant to replace (simplify-code review finding) - Remove dead 'model' in locals() guard — model is always in scope before the try block (simplify-code review finding) - Extract _stub_agent_for_finalize() helper to eliminate ~40 lines of copy-pasted agent stubbing in tests (simplify-code review finding) - Clean up 'Phase 0.5' instrumentation comments
|
Merged via #81254 (salvage). Your three commits cherry-picked with authorship preserved:
The Follow-up fixes applied on top:
Thanks for the contribution! |
- Fix _usage_audit_path() to use _get_hermes_home() instead of hardcoded Path.home() / '.hermes' (profile-safe resolution, sweeper finding) - Rewrite skip_background_review tests to exercise finalize_turn() directly instead of duplicating the guard expression (sweeper finding) - Fix response_silent audit field to use _is_cron_silence_response() instead of the buggy SILENT_MARKER substring check it was meant to replace (simplify-code review finding) - Remove dead 'model' in locals() guard — model is always in scope before the try block (simplify-code review finding) - Extract _stub_agent_for_finalize() helper to eliminate ~40 lines of copy-pasted agent stubbing in tests (simplify-code review finding) - Clean up 'Phase 0.5' instrumentation comments
- Fix _usage_audit_path() to use _get_hermes_home() instead of hardcoded Path.home() / '.hermes' (profile-safe resolution, sweeper finding) - Rewrite skip_background_review tests to exercise finalize_turn() directly instead of duplicating the guard expression (sweeper finding) - Fix response_silent audit field to use _is_cron_silence_response() instead of the buggy SILENT_MARKER substring check it was meant to replace (simplify-code review finding) - Remove dead 'model' in locals() guard — model is always in scope before the try block (simplify-code review finding) - Extract _stub_agent_for_finalize() helper to eliminate ~40 lines of copy-pasted agent stubbing in tests (simplify-code review finding) - Clean up 'Phase 0.5' instrumentation comments
- Fix _usage_audit_path() to use _get_hermes_home() instead of hardcoded Path.home() / '.hermes' (profile-safe resolution, sweeper finding) - Rewrite skip_background_review tests to exercise finalize_turn() directly instead of duplicating the guard expression (sweeper finding) - Fix response_silent audit field to use _is_cron_silence_response() instead of the buggy SILENT_MARKER substring check it was meant to replace (simplify-code review finding) - Remove dead 'model' in locals() guard — model is always in scope before the try block (simplify-code review finding) - Extract _stub_agent_for_finalize() helper to eliminate ~40 lines of copy-pasted agent stubbing in tests (simplify-code review finding) - Clean up 'Phase 0.5' instrumentation comments
- Fix _usage_audit_path() to use _get_hermes_home() instead of hardcoded Path.home() / '.hermes' (profile-safe resolution, sweeper finding) - Rewrite skip_background_review tests to exercise finalize_turn() directly instead of duplicating the guard expression (sweeper finding) - Fix response_silent audit field to use _is_cron_silence_response() instead of the buggy SILENT_MARKER substring check it was meant to replace (simplify-code review finding) - Remove dead 'model' in locals() guard — model is always in scope before the try block (simplify-code review finding) - Extract _stub_agent_for_finalize() helper to eliminate ~40 lines of copy-pasted agent stubbing in tests (simplify-code review finding) - Clean up 'Phase 0.5' instrumentation comments
- Fix _usage_audit_path() to use _get_hermes_home() instead of hardcoded Path.home() / '.hermes' (profile-safe resolution, sweeper finding) - Rewrite skip_background_review tests to exercise finalize_turn() directly instead of duplicating the guard expression (sweeper finding) - Fix response_silent audit field to use _is_cron_silence_response() instead of the buggy SILENT_MARKER substring check it was meant to replace (simplify-code review finding) - Remove dead 'model' in locals() guard — model is always in scope before the try block (simplify-code review finding) - Extract _stub_agent_for_finalize() helper to eliminate ~40 lines of copy-pasted agent stubbing in tests (simplify-code review finding) - Clean up 'Phase 0.5' instrumentation comments
Summary (TL;DR)
Three independently-revertable cron-path token-discipline fixes:
HERMES_CRON_MAX_TOKENSenv var caps LLM response on cron jobs. Mitigates degenerate-loop hallucinations on small/open-source models (single repetition trap producing 64KB+ output flooding chat with 33+ message bursts). DefaultNone= no cap (back-compat).usage_audit.jsonlwriter logs prompt/completion/total tokens, fire_id, deliver_target, response-silent flag, and duration to~/.hermes/cron/usage_audit.jsonlper cron fire. Append-only, fail-safe (writer exceptions swallowed). Provides ground-truth measurement for any future leak claim and a substrate for cost dashboards.skip_background_reviewAIAgent flag gates the_spawn_background_reviewfork (~15K input + up to 8 LLM iterations / event) on a single boolean. Cron setsTruesince cron sessions don't need skill-curation reviews; CLI / interactive paths default toFalseand behave unchanged.Plus a one-line code comment documenting that
maybe_auto_titleis gateway/CLI-only and never invoked on the cron path (verified via grep acrosscron/scheduler.py,run_agent.py).docs/wave-2-deferred.mdregisters items intentionally deferred from this wave: tool-description compaction (gated on a regression suite), provider-cache integration forSKILL.mdblocks, aprecheck_commandschema RFC, and streaming-aware history compression.Background
While operating a Discord bot on this gateway (cron
*/5recommend-sweep +*/15escalation-sweep + daily reminders), I measured cron path daily token spend at ~11M tokens/day input. Audit traced the dominant share to: tool-schema bloat per call (~12K tokens), no provider-side cache on non-Anthropic backends, and a degenerate-loop incident producing 64KB output. This PR addresses the directly fixable surface; the larger architectural items (cron LLM-mandatory,precheck_commandschema) are filed indocs/wave-2-deferred.mdfor follow-up RFC.Production deployment
This branch is currently the production runtime for BlueNode Hermes profile (OCI ARM64 Graviton) — operations Discord bot for Inha University's blockchain club. Token-discipline fixes here were diagnosed and validated against that workload (cron
*/5recommend-sweep, daily reminders, escalation handling).Test plan
tests/cron/test_usage_audit_logger.py— 9 tests covering JSONL schema, missing-token fallback, writer exception swallow, missing-parent-dir creationtests/agent/test_skip_background_review.py— 5 tests covering default-False, flag-persists, gate-short-circuits, gate-fires-when-unset, cron-source-asserts-Truepytest tests/cron/ tests/agent/green locally (13/13 new tests pass; 342/342 cron suite pass; no regressions)ruff check— no new warnings introduced (pre-existing E402 incron/scheduler.py:113is unchanged)upstream/mainHEAD44cdf555a(2026-05-10) — clean, MERGEABLEComplementary upstream change
Note: complementary to upstream
e5af1dd63 fix(review): tell background reviewer not to capture transient env failures as skills (#23004)— that PR keeps the reviewer smarter on non-cron paths; this PR skips the reviewer entirely on the cron path. Both can land independently in either order.Batch consideration
Logical mini-batch with #15508 (configurable background review routing) and #21511 (skip external memory providers in background review). All three touch
run_agent.pybackground-review path:These are compatible and can be reviewed/merged in any order; bundling them into a single "background review hardening" cycle could be efficient if the maintainer prefers.
Salvage-friendly
This PR is structured for salvage: each commit is independently revertable, no abstractions added, all touchpoints isolated to cron / agent. Happy to revise scope, rebase, or split commits on demand. Feedback welcome.
🤖 Generated with Claude Code