Skip to content

fix(gateway): harden Honcho cache memo invalidation - #46385

Closed
lkz-de wants to merge 1 commit into
NousResearch:mainfrom
lkz-de:pr/honcho-memo-size-key
Closed

fix(gateway): harden Honcho cache memo invalidation#46385
lkz-de wants to merge 1 commit into
NousResearch:mainfrom
lkz-de:pr/honcho-memo-size-key

Conversation

@lkz-de

@lkz-de lkz-de commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

Summary

Harden the Honcho cache-busting memo key so coarse-mtime rewrites of honcho.json cannot reuse stale parsed identity state.

Why

On coarse-mtime filesystems, rapid rewrites of honcho.json can keep the same st_mtime_ns while changing contents. The earlier size-only approach still missed equal-size rewrites. Using a SHA-256 content hash as a discriminator closes that remaining stale-config path.

Scope

  • memo/cache invalidation only
  • no Honcho config schema changes

Test plan

  • pytest tests/gateway/test_agent_cache.py
    • includes a regression that forces equal st_mtime_ns, equal file size, changed honcho.json content, and asserts the Honcho parse is refreshed
    • 79 passed locally against the current head

Related

@alt-glitch alt-glitch added type/bug Something isn't working comp/gateway Gateway runner, session dispatch, delivery tool/memory Memory tool and memory providers comp/plugins Plugin system and bundled plugins P3 Low — cosmetic, nice to have labels Jun 15, 2026

@tonydwb tonydwb left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review Summary

Verdict: Approved

Fixes Honcho cache memo invalidation: the cache key now includes a component that changes when the config path changes, ensuring cached values don't persist across config reloads.


Reviewed by Hermes Agent

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for isolating this to the Honcho memo key. The current-head premise is real: gateway/run.py:15777-15794 keys the memo only by (path, st_mtime_ns), so a coarse-mtime rewrite can reuse the prior parsed identity mapping.

Problems

  • Adding st_size detects only size-changing rewrites. Equal-size changes to values consumed at gateway/run.py:15787-15792 remain indistinguishable when mtime is unchanged, so the general stale-config failure remains possible.
  • The PR diff has no test. tests/gateway/test_agent_cache.py:349-388 covers a normal rewrite but does not force equal mtime or assert size-key invalidation.

Suggested changes

  • Use a discriminator that covers equal-size content changes, or narrow the stated guarantee to size-changing edits.
  • Add a regression test with controlled equal st_mtime_ns values and changed config content; assert the parser/cache result is refreshed.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state sweeper:risk-caching Sweeper risk: may break/degrade prompt caching or cache-key stability (invariant) sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 14, 2026
@lkz-de
lkz-de force-pushed the pr/honcho-memo-size-key branch from 3ff05c9 to 2a267da Compare July 15, 2026 09:53
@lkz-de

lkz-de commented Jul 15, 2026

Copy link
Copy Markdown
Contributor Author

Thanks — I pushed 2a267da7, which addresses both issues from your review.

The memo key now includes a SHA-256 content-hash discriminator alongside st_mtime_ns / st_size, so equal-size rewrites of honcho.json no longer reuse stale parsed state when st_mtime_ns is unchanged.

I also added a regression in tests/gateway/test_agent_cache.py that forces equal st_mtime_ns, equal file size, changed config content, and asserts the Honcho parse is refreshed.

@teknium1

Copy link
Copy Markdown
Contributor

Closing after review against current main.

The memo this hardens exists to avoid I/O on the gateway's hot agent-cache path — and adding a full SHA256 of honcho.json to the key means every lookup now reads and hashes the file, which defeats the memo's purpose. The edge it protects against (a same-size rewrite landing inside one mtime tick on a coarse-mtime filesystem) is real but vanishingly rare, and its worst outcome is one stale cache generation until the next change.

If this ever bites someone in practice, the right-shaped fix is size-only: st_size comes free from the stat() call the memo already makes, catching most same-tick rewrites with zero added I/O. Content hashing on the hot path is the part we're declining.

Thanks for the well-tested submission @lkz-de — the failure mode analysis was solid.

@teknium1 teknium1 closed this Jul 17, 2026
@teknium1 teknium1 added the area/memory Memory subsystem: store, providers, sync, background reviews label Jul 19, 2026
bbasketballer75 added a commit to bbasketballer75/hermes-agent that referenced this pull request Aug 10, 2026
The memo was keyed on (path, st_mtime_ns), so an edit to honcho.json
landing inside one mtime tick on a coarse-mtime filesystem kept serving
the previously parsed identity config. Add st_size to the key — it comes
from the single stat() call the memo already makes, so any same-tick
rewrite that changes the file's size is now detected at zero added I/O
on this hot path (it feeds the per-turn agent-cache signature).

Deliberately NOT content-hashed. An equal-size rewrite inside one mtime
tick can still reuse stale parsed state for one cache generation; that
edge is vanishingly rare and self-heals on the next change. Hashing the
file on every lookup would defeat the memo's no-I/O purpose — this is
the exact design the maintainer resolution on NousResearch#46385 declined, naming
st_size as the right-shaped discriminator. A test pins the documented
tradeoff so a future change that silently adds per-lookup I/O surfaces
as a deliberate decision rather than an accident.

Tests cover: same-mtime size-changing rewrite invalidates (verified to
fail against the old key), identical stat reuses the memo without
re-parsing, the equal-size edge stays memoized by design, and stat
failure still returns a parsed config.

Supersedes the earlier content-hash version of this branch, which also
accidentally reverted NousResearch#75581's relay rename behavior via a whole-file
checkout across divergent bases — this rewrite touches only the memo.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/memory Memory subsystem: store, providers, sync, background reviews comp/gateway Gateway runner, session dispatch, delivery comp/plugins Plugin system and bundled plugins P3 Low — cosmetic, nice to have sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-caching Sweeper risk: may break/degrade prompt caching or cache-key stability (invariant) sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state tool/memory Memory tool and memory providers type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants