fix(gateway): detect stale sys.modules via expected exports (#17648 follow-up) - #19531
Conversation
0c22930 to
f080954
Compare
…usResearch#17648) - Add _loaded_modules_missing_expected_exports() for cfg_get / utils helpers - Run export check before mtime comparison so live-update mismatches restart - Log when mtime baseline is zero but boot time was captured - Extend stale-code self-check tests
f080954 to
26837b5
Compare
teknium1
left a comment
There was a problem hiding this comment.
Thanks for isolating the stale-module failure and adding focused coverage.
Problems
- Current main deliberately removed this exact per-message stale-code auto-restart mechanism in
f2332d4f1because it interrupted chats and dropped the triggering message. The current design instead records a boot fingerprint ingateway/run.py:20274-20279and limits the guard to model switching ingateway/slash_commands.py:57-83. - The proposed export list is incomplete:
tests/test_stale_utils_module_import.py:52-63reproduces the same failure when staleutilslacksenv_float, which is not checked by the new helper. The linked June 24 report also identifies stale imports fromtools.tool_backend_helpersandcron.jobs.
Suggested changes
- Re-scope any follow-up to current
gateway/code_skew.pybehavior without restoring automatic restarts that drop arbitrary inbound messages. - Cover representative lazy-import consumers rather than treating three exports as a complete stale-module contract.
Automated hermes-sweeper review.
| if utils_mod is not None: | ||
| if not hasattr(utils_mod, "atomic_replace"): | ||
| return True | ||
| if not hasattr(utils_mod, "base_url_host_matches"): |
There was a problem hiding this comment.
This fixed list is not a complete stale-module contract: current regression coverage at tests/test_stale_utils_module_import.py:52-63 reproduces the same ImportError when stale utils lacks env_float, which this probe does not check. The linked June 24 report also identifies missing exports outside utils.
|
Closing as superseded — the stale-code detection approach on main moved to git-revision fingerprinting (gateway/code_skew.py) instead of export-based sys.modules checks, and the auto-restart path this PR targeted was removed. The underlying gap (stale-module risk on regular agent turns, not just model switching) is worth tracking separately. |
What does this PR do?
After
hermes update, a gateway can keep running with old modules still insys.moduleswhile tools on disk import newer symbols. Users see import errors; session commands do not reload Python.#18409 restarts the gateway when key source files look newer than at startup (mtime check). This change adds a second condition: if
hermes_cli.configor the projectutilspackage is already loaded but is missingcfg_get,atomic_replace, orbase_url_host_matches, treat the process as stale and use the same restart path. It does not add broad try/except import fallbacks (#17935).Related Issue
Refs #17648. Follow-up to #18409.
Source branch
Fork: github.com/anuragbanerjee/hermes-agent, branch
fix/gateway-stale-sys-modules-exports. To check out locally:gh repo clone anuragbanerjee/hermes-agent -- -b fix/gateway-stale-sys-modules-exports(or add the fork as a remote and fetch that branch).Type of Change
Changes Made
gateway/run.py:_loaded_modules_missing_expected_exports(); call from_detect_stale_code(); warn when boot mtime baseline is zero.tests/gateway/test_stale_code_self_check.py: tests for the export-based path.How to Test
pytest tests/gateway/test_stale_code_self_check.py -vcfg_get.Checklist
Code
fix(scope):,feat(scope):, etc.)pytest tests/ -qand all tests passDocumentation & Housekeeping
docs/, docstrings) — or N/Acli-config.yaml.exampleif I added/changed config keys — or N/ACONTRIBUTING.mdorAGENTS.mdif I changed architecture or workflows — or N/AFor New Skills
N/A
Screenshots / Logs
N/A