fix(gateway): key the Honcho cache-busting memo on stat identity - #75812
fix(gateway): key the Honcho cache-busting memo on stat identity#75812bbasketballer75 wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Pull request overview
Improves gateway agent cache-busting for Honcho by ensuring honcho.json identity/config changes are detected even when filesystem mtime granularity is too coarse, and adds regression tests for the same-mtime/different-content case.
Changes:
- Update
GatewayRunner._HONCHO_CACHE_BUSTING_MEMOto key onhoncho.jsoncontent (SHA-256) in addition to stat fields. - Add gateway cache-busting tests, including a same-size/same-mtime rewrite scenario and provider gating for Honcho-only reads.
- (Unrelated scope) Includes Discord relay semantic thread-rename lane changes/removals that appear to break existing relay thread tests and should be reverted or handled in a dedicated PR.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
tests/gateway/test_agent_cache.py |
Adds/extends cache-busting tests for Honcho + general gateway signature behavior. |
gateway/run.py |
Changes Honcho cache-busting memo key to include file content hash; also contains unrelated Discord relay auto-thread rename logic changes. |
Suppressed comments (2)
gateway/run.py:18886
- This early return removes the bounded cache-polling behavior for relay-delivered Discord channel events. The relay adapter only learns the created thread_id/initial name from send-result feedback, and the auto-title thread can race that delivery; without polling, semantic renames can be skipped even though feedback arrives milliseconds later (and
tests/gateway/relay/test_relay_threads.py::test_title_rename_polls_feedback_that_arrives_lateexpects the poll behavior).
gateway/run.py:18923 - On relay title turns, returning when
_relay_auto_thread_info(source)isNoneat schedule time reintroduces the delivery/title race: the send-result feedback may not have populated the adapter cache yet. The semantic-rename lane should schedule on the shape of a relay Discord channel event and let the async rename method poll briefly for feedback.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Related to #46385: this extends the same Honcho invalidation family to equal-size rewrites. Please split out the unrelated Discord relay auto-thread rename reversion before review. |
teknium1
left a comment
There was a problem hiding this comment.
Thanks for isolating the same-mtime Honcho invalidation case. The current-main memo is indeed keyed only by path and mtime (gateway/run.py:21492).
Problems
gateway/run.py:21452reads and SHA-256-hashes the config before every memo lookup. This function feeds the per-turn agent-cache signature atgateway/run.py:4243-4248, so unchanged Honcho configurations lose the memo's no-I/O fast path. The maintainer resolution on #46385 explicitly declined content hashing here and identifiedst_sizeas the stat-only alternative.- This diff also reverts the relay rename behavior added in
4a8eeb5d1cd4(#75581). The changed gate requires send-result feedback before callback registration, while current main documents that the feedback arrives after registration (gateway/run.py:5343-5351) and polls for it atgateway/run.py:18906-18924.
Suggested changes
- Split the Discord relay reversion out and preserve #75581's eager registration/polling behavior.
- Re-scope the Honcho key to the maintainer-approved stat-only approach, with a regression test matching that guarantee.
Automated hermes-sweeper review.
6a058a4 to
f1e8672
Compare
|
@teknium1 @alt-glitch Both findings were right, and the branch is rebuilt in 1. The relay reversion is gone, and here's how it got in. I built the original branch with a whole-file 2. Re-scoped to the maintainer-approved stat-only key. I read the #46385 resolution — this branch had unknowingly re-proposed the exact content-hash design you declined there, per-lookup SHA-256 and all. Now: The tests match the narrowed guarantee rather than papering over it:
🤖 Addressed by Claude Code |
GottZ
left a comment
There was a problem hiding this comment.
This was generated by AI during triage.
Summary
Two PRs address the Honcho memo invalidation issue. #46385 covers equal-size, same-mtime rewrites through content hashing, while the revised #75812 deliberately narrows the fix to size-changing rewrites by adding st_size to the existing stat-based key without hot-path file reads.
Related pull requests
- #46385 [closed]
duplicate— (+71/-4) — closed design reference, not a reopening candidate: it detects equal-size rewrites with SHA-256 and adds a matching regression test, but the later blocking contributor decision rejected per-lookup hashing because it defeats the memo's no-I/O purpose. Despite the earlier hermes-sweeper keep_open verdict, the final diff retained that rejected cost and was therefore superseded by the stat-only approach in #75812. - #75812
related— (+159/-4) — keep open with a salvage path: retain the focused(path, st_mtime_ns, st_size)key and its tests for size-changing invalidation, memo reuse, the documented equal-size limitation, and stat failure. The current diff addresses the visible keep_open review by removing both SHA-256 reads and the unrelated #75581 relay reversion; it now implements the stat-only design requested by the blocking contributor feedback.
Duplicates
#46385 and #75812 target the same coarse-mtime stale-memo family, but they are not implementation duplicates: #46385 uses the rejected content-hash guarantee, whereas #75812 supersedes it with the narrower stat-only discriminator.
Suggested consolidation
Keep #75812 open with a salvage path limited to its stat-only memo-key change and focused regression coverage, pending re-review of the rebuilt diff. Keep #46385 closed as the rejected content-hashing predecessor rather than reopening it; no additional duplicate closure is needed because #46385 is already closed and #75812 carries the maintainer-approved implementation shape.
Complex graph
flowchart LR
classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
classDef best stroke-width:3px,stroke:#b45309
classDef target stroke-width:3px,stroke:#4338ca
subgraph Dup46385 ["PRs duplicating each other"]
P46385["PR #46385 (closed)"]
P75812["PR #75812 (open)"]
end
class P46385 closed
class P75812 open
class P75812 target
click P46385 "https://github.com/NousResearch/hermes-agent/pull/46385"
click P75812 "https://github.com/NousResearch/hermes-agent/pull/75812"
Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed (state tag in the node label).
Cross-PR triage: Reviewed 2 pull requests and 0 issues in this complex. Each diff was read against this issue; Assessment working set: 13 kB of PR diffs, 3 kB of issue/PR text, 7 kB of discussion (17 comments), 0 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.
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.
f1e8672 to
d14f44a
Compare
|
Rebased onto current Re-verified after the rebase: Worth noting this is not a theoretical race. On a Windows host it reproduces as a real intermittent test failure: |
The 2026-08-11 backup-ref restore re-introduced a fork-local sanitizer in plugins/memory/honcho that had already been retired on 2026-08-10. It is destructive: it reduced ordinary memo text to a NUL byte. Verified live before reverting -- "hermes has been flaky lately, can you look into it?" -> "\x00" Both files are now byte-identical to origin/main. The two upstream PRs that carry the parts worth keeping (NousResearch#75812 stat-identity keying, NousResearch#75813 Windows config-path normalization) remain open and are tracked in the carried-local-changes register.
Problem
GatewayRunner._HONCHO_CACHE_BUSTING_MEMOis keyed on(path, st_mtime_ns). On filesystems with coarse mtime granularity, an edit tohoncho.jsoncan land inside one mtime tick and keep serving the previously parsed identity config on the per-turn agent-cache signature path.Change
Add
st_sizeto the memo key. It comes from the singlestat()call the memo already makes, so any same-tick rewrite that changes the file's size is detected at zero added I/O on this hot path.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. Per the maintainer resolution on #46385, which declined content hashing here and named
st_sizeas the right-shaped discriminator. One test pins that documented tradeoff so a future change that silently adds per-lookup I/O surfaces as a deliberate decision.Tests (net-new; current
mainhas no memo coverage)stat()failure still returns a parsed configHistory
Supersedes the earlier content-hash version of this branch, which also accidentally reverted #75581's relay-rename behavior via a whole-file checkout across divergent bases. Both problems are gone: the diff is now hand-authored against current
mainand touches only the memo — 2 files, +159/−4, zero relay content.🤖 Generated with Claude Code