Conversation
tools/checkpoint_manager.py's CHECKPOINT_BASE and gateway/sticker_cache.py's CACHE_PATH are resolved once at import time via get_hermes_home(), which is a context-local ContextVar under the multiplexed gateway (multiple profiles sharing one process). Freezing the path at import time pins every later checkpoint/cache read-write to whichever profile's HERMES_HOME was active when the module was first imported -- the same bug class already fixed for cache dirs, skills_hub, rich_sent_store, and (this session) the OAuth/auth.json/ sessions.json paths. CheckpointManager is "owned by AIAgent" per-instance, but its methods read the frozen module constant directly instead of taking the store root from the instance, so a profile's CheckpointManager can read/write code-edit checkpoints into a different profile's store. Add a per-call resolver for each path, following the established "respect an existing test monkeypatch of the constant, otherwise re-resolve through get_hermes_home()" pattern so the extensive existing test seams in tests/tools/test_checkpoint_manager.py and tests/gateway/test_sticker_cache.py keep working unmodified.
tonydwb
left a comment
There was a problem hiding this comment.
LGTM — profile-scoped path resolution for sticker cache and checkpoint manager. The re-resolve-through-get_hermes_home() pattern correctly honors the active profile override under the multiplexed gateway. Test seam (monkeypatch detection) is preserved. The approach mirrors the existing cache-dir profile-isolation fix.
|
looks mergeable Security evidence:
The patch is focused, merges cleanly into current Signed: GPT-5.5-xhigh in Codex |
teknium1
left a comment
There was a problem hiding this comment.
Thanks for tracing this through the multiplexed profile runtime. The premise is confirmed on current main: get_hermes_home() honors the context-local override (hermes_constants.py:71-73), while CHECKPOINT_BASE and CACHE_PATH are frozen at import (tools/checkpoint_manager.py:72, gateway/sticker_cache.py:20). The proposed per-call resolution reaches the relevant checkpoint and sticker-cache sinks.
Problems
- The added tests currently assert resolver outputs only. They do not exercise a real checkpoint operation through
_take()/list_checkpoints()(tools/checkpoint_manager.py:693,875) or a sticker-cache write through_save_cache()(gateway/sticker_cache.py:39-50) after switching profile overrides.
Suggested changes
- Add two-profile end-to-end regressions that verify checkpoint and sticker-cache data are physically read/written under the active profile, while retaining the monkeypatch compatibility checks.
This is an automated hermes-sweeper review.
| active profile — otherwise one profile's CheckpointManager instance can | ||
| read/write code-edit checkpoints into a different profile's store under | ||
| the multiplexed gateway.""" | ||
|
|
There was a problem hiding this comment.
Please add a two-profile checkpoint operation test in addition to this resolver assertion: persist under profile A, switch the same manager to B, then verify B reads/writes only B's store. The affected production sinks are _take() and list_checkpoints().
… and schema paths under multiplex
Under `gateway.multiplex_profiles` one gateway process serves every profile
under ~/.hermes/profiles/NAME/; each routed turn runs with a context-local
HERMES_HOME override while `os.environ` still holds the DEFAULT profile's
values. Anything evaluated once at import, or memoised in a single unkeyed
module slot, therefore freezes the LAUNCH profile's value and leaks it into
every other profile's turns. This lands the tools-side half of that class:
- tools/process_registry.py, tools/environments/{modal,singularity}.py:
`_checkpoint_path()` / `_snapshot_store()` resolve `get_hermes_home()` at
call time (same seam as `tools/skills_tool._skills_dir`, so the existing
`monkeypatch.setattr(CHECKPOINT_PATH)` test sites keep working). Completes
the checkpoint_manager / sticker_cache half cherry-picked from #56315.
- plugins/platforms/feishu/feishu_comment_rules.py: `_MtimeCache` is now
path-keyed (accepts a Path or a zero-arg resolver, one (mtime, data) slot
per resolved path) with `invalidate()`; `_rules_file()` / `_pairing_file()`
resolve the routed profile's files. Proposed in #63962.
- tools/tool_output_limits.py, tools/browser_tool.py, tools/browser_camofox.py:
the process-lifetime config caches are dicts keyed by `hermes_home_key()`;
the `_X_resolved` flags and the lifecycle reset keep their shape.
tools/file_tools.py drops its private `file_read_max_chars` memo and reads
the already mtime+path-cached `load_config_readonly()`.
- hermes_time.py: `get_timezone_name()`; when `is_multiplex_active()` the
env `HERMES_TIMEZONE` (bridged from the default profile's config at gateway
startup) is ignored in favour of the routed profile's config.yaml. Both
sandbox TZ sites (code_execution_env/_tool) now use it.
- tools/cronjob_tools.py, tools/tts_tool.py, tools/skill_manager_tool.py:
the static schema text is profile-neutral and `dynamic_schema_overrides=`
rebuilds the `display_hermes_home()` / create-dir hint per
`get_definitions()`, so a routed profile's model sees its own paths.
Refs #95685.
Co-authored-by: Nathan Shan <nathanielcrush51@gmail.com>
(cherry picked from commit 6d3fc6b07b3155c6196b1fd61a829283f1d7855c)
… and schema paths under multiplex
Under `gateway.multiplex_profiles` one gateway process serves every profile
under ~/.hermes/profiles/NAME/; each routed turn runs with a context-local
HERMES_HOME override while `os.environ` still holds the DEFAULT profile's
values. Anything evaluated once at import, or memoised in a single unkeyed
module slot, therefore freezes the LAUNCH profile's value and leaks it into
every other profile's turns. This lands the tools-side half of that class:
- tools/process_registry.py, tools/environments/{modal,singularity}.py:
`_checkpoint_path()` / `_snapshot_store()` resolve `get_hermes_home()` at
call time (same seam as `tools/skills_tool._skills_dir`, so the existing
`monkeypatch.setattr(CHECKPOINT_PATH)` test sites keep working). Completes
the checkpoint_manager / sticker_cache half cherry-picked from #56315.
- plugins/platforms/feishu/feishu_comment_rules.py: `_MtimeCache` is now
path-keyed (accepts a Path or a zero-arg resolver, one (mtime, data) slot
per resolved path) with `invalidate()`; `_rules_file()` / `_pairing_file()`
resolve the routed profile's files. Proposed in #63962.
- tools/tool_output_limits.py, tools/browser_tool.py, tools/browser_camofox.py:
the process-lifetime config caches are dicts keyed by `hermes_home_key()`;
the `_X_resolved` flags and the lifecycle reset keep their shape.
tools/file_tools.py drops its private `file_read_max_chars` memo and reads
the already mtime+path-cached `load_config_readonly()`.
- hermes_time.py: `get_timezone_name()`; when `is_multiplex_active()` the
env `HERMES_TIMEZONE` (bridged from the default profile's config at gateway
startup) is ignored in favour of the routed profile's config.yaml. Both
sandbox TZ sites (code_execution_env/_tool) now use it.
- tools/cronjob_tools.py, tools/tts_tool.py, tools/skill_manager_tool.py:
the static schema text is profile-neutral and `dynamic_schema_overrides=`
rebuilds the `display_hermes_home()` / create-dir hint per
`get_definitions()`, so a routed profile's model sees its own paths.
Refs #95685.
Co-authored-by: Nathan Shan <nathanielcrush51@gmail.com>
(cherry picked from commit 6d3fc6b07b3155c6196b1fd61a829283f1d7855c)
… and schema paths under multiplex
Under `gateway.multiplex_profiles` one gateway process serves every profile
under ~/.hermes/profiles/NAME/; each routed turn runs with a context-local
HERMES_HOME override while `os.environ` still holds the DEFAULT profile's
values. Anything evaluated once at import, or memoised in a single unkeyed
module slot, therefore freezes the LAUNCH profile's value and leaks it into
every other profile's turns. This lands the tools-side half of that class:
- tools/process_registry.py, tools/environments/{modal,singularity}.py:
`_checkpoint_path()` / `_snapshot_store()` resolve `get_hermes_home()` at
call time (same seam as `tools/skills_tool._skills_dir`, so the existing
`monkeypatch.setattr(CHECKPOINT_PATH)` test sites keep working). Completes
the checkpoint_manager / sticker_cache half cherry-picked from #56315.
- plugins/platforms/feishu/feishu_comment_rules.py: `_MtimeCache` is now
path-keyed (accepts a Path or a zero-arg resolver, one (mtime, data) slot
per resolved path) with `invalidate()`; `_rules_file()` / `_pairing_file()`
resolve the routed profile's files. Proposed in #63962.
- tools/tool_output_limits.py, tools/browser_tool.py, tools/browser_camofox.py:
the process-lifetime config caches are dicts keyed by `hermes_home_key()`;
the `_X_resolved` flags and the lifecycle reset keep their shape.
tools/file_tools.py drops its private `file_read_max_chars` memo and reads
the already mtime+path-cached `load_config_readonly()`.
- hermes_time.py: `get_timezone_name()`; when `is_multiplex_active()` the
env `HERMES_TIMEZONE` (bridged from the default profile's config at gateway
startup) is ignored in favour of the routed profile's config.yaml. Both
sandbox TZ sites (code_execution_env/_tool) now use it.
- tools/cronjob_tools.py, tools/tts_tool.py, tools/skill_manager_tool.py:
the static schema text is profile-neutral and `dynamic_schema_overrides=`
rebuilds the `display_hermes_home()` / create-dir hint per
`get_definitions()`, so a routed profile's model sees its own paths.
Refs #95685.
Co-authored-by: Nathan Shan <nathanielcrush51@gmail.com>
(cherry picked from commit 6d3fc6b07b3155c6196b1fd61a829283f1d7855c)
|
Landed on |
Summary
tools/checkpoint_manager.py::CHECKPOINT_BASEandgateway/sticker_cache.py::CACHE_PATHare both resolved once at import time viaget_hermes_home(), which reads a context-localContextVar(_HERMES_HOME_OVERRIDE) set per-request under the multiplexed gateway (multiple profiles served from one process, e.g. the desktoptui_gateway). Freezing the resolved path at import time pins every later checkpoint/cache read-write to whichever profile'sHERMES_HOMEhappened to be active the first time the module was imported — the same bug class already fixed for cache dirs,tools/skills_hub.py,gateway/rich_sent_store.py, and (in a companion PR from this same audit) the Anthropic OAuth file / Nous Portalauth.json/sessions.jsonindex.CheckpointManageris documented as "owned by AIAgent" (one instance per agent), but its methods (list_checkpoints,diff,restore,_take) all read the frozen module constant directly instead of resolving through the instance — so under the multiplexed gateway, a profile'sCheckpointManagercan silently read/write the code-editing checkpoint (shadow-git snapshot) store of a different profile: wrong checkpoints listed, wrong snapshot restored, or a profile's file-edit history landing in another profile's store.gateway/sticker_cache.py's leak is lower-stakes (a cache of vision-generated Telegram sticker descriptions) but the identical bug shape — bundled here since it's a one-line-per-site version of the same fix.Changes
tools/checkpoint_manager.py: added_resolve_checkpoint_base(). Updated_store_path()'s default fallback and all 4CheckpointManagermethods that previously called_store_path(CHECKPOINT_BASE)directly (now_store_path(), relying on the updated default), plus the 5 module-level functions (prune_checkpoints,maybe_auto_prune_checkpoints,store_status,clear_all,clear_legacy) that usedcheckpoint_base or CHECKPOINT_BASE(nowcheckpoint_base or _resolve_checkpoint_base()). Preserves the existing test seam —tests/tools/test_checkpoint_manager.pyhas many call sites thatmonkeypatch.setattr("tools.checkpoint_manager.CHECKPOINT_BASE", ...), which the resolver still honors when the constant has been changed from its import-time default (same pattern asgateway/platforms/base.py::_resolve_cache_dir).gateway/sticker_cache.py: added_resolve_cache_path(), called from_load_cache()/_save_cache(). Same test-seam-preserving pattern (tests/gateway/test_sticker_cache.pypatchesCACHE_PATHdirectly).tests/test_profile_isolation_runtime.py(the existing profile-isolation regression suite): addedTestCheckpointManagerPathResolutionandTestStickerCachePathResolution, each proving the resolved path actually changes between two distinct profile overrides, plus "monkeypatched constant still wins" regression tests for both resolvers.Test plan
pytest tests/tools/test_checkpoint_manager.py tests/gateway/test_sticker_cache.py -q— 94 passedpytest tests/test_windows_subprocess_no_window_flags.py -q— 15 passed (sanity check, unaffected)pytest tests/test_profile_isolation_runtime.py -q— 15 passed (10 pre-existing + 5 new)ruff checkon all changed files — clean