Skip to content

fix(goals): make the goal store's launch-home scope deliberate, not accidental - #320

Merged
OmarB97 merged 1 commit into
mainfrom
fix/goal-store-process-home-fork-20260802
Aug 2, 2026
Merged

fix(goals): make the goal store's launch-home scope deliberate, not accidental#320
OmarB97 merged 1 commit into
mainfrom
fix/goal-store-process-home-fork-20260802

Conversation

@OmarB97

@OmarB97 OmarB97 commented Aug 2, 2026

Copy link
Copy Markdown
Owner

What does this PR do?

hermes_cli/goals.py::_get_session_db cached one SessionDB per get_hermes_home() path, and its docstring claimed "We cache one instance per hermes_home path so profile switches still pick up the right DB."

That claim was false. It constructed SessionDB() with no argument, and SessionDB.__init__ (hermes_state.py:1121) falls back to DEFAULT_DB_PATH — a module-level constant evaluated at import time (hermes_state.py:206). Every SessionDB the goal store handed out pointed at whichever home was active when hermes_state was first imported, regardless of the live HERMES_HOME. The _DB_CACHE keying was dead code documenting behavior the process did not have.

import os, tempfile
os.environ["HERMES_HOME"] = tempfile.mkdtemp()
from hermes_cli import goals
db1 = goals._get_session_db()
os.environ["HERMES_HOME"] = tempfile.mkdtemp()
goals._DB_CACHE.clear()
db2 = goals._get_session_db()
print(db1.db_path, db2.db_path)  # identical

The tempting fix — make the store follow the active home — is the wrong one, and would break the goal loop.

A goal's writers and its reader sit on opposite sides of the per-turn HERMES_HOME binding:

site binds a profile home?
Writer session.create (spawn --goal, server.py:6639) No — computes profile_home at :6477, never binds it
Writer command.dispatch (/goal, server.py:13905) No — zero set_hermes_home_override / _profile_scoped in its body
Reader post-turn continuation hook (server.py:11124) Yes — inside set_hermes_home_override(profile_home) (bound :10801, reset :11301)

All three key on the same session_key. Today they agree only because the store ignores the home entirely. Make it follow the active home and, for any session running under a non-launch profile, /goal writes one database while the hook reads another — is_active() returns False and the Ralph loop silently never fires. That is the same stranded-loop failure mode #319 just fixed, arrived at from the other direction.

This is verified, not argued: temporarily flipping this function to get_hermes_home() fails the new test_reader_under_profile_override_still_sees_the_goal with AssertionError: goal store followed the per-turn override.

gateway/run.py has the same shape — _post_turn_goal_continuation (:14210) runs under _profile_runtime_scope, while _goal_still_active_for_session (:4930) does a "best-effort fresh DB check" from queue-drain code that does not.

So the scope stays process-global — but stated rather than inherited by accident. _get_session_db now resolves get_process_hermes_home() / "state.db" and passes it explicitly. That accessor exists for precisely this case: it reads the process env and deliberately ignores the context-local override. Production behavior is unchanged, because both gateways bind the profile through a contextvar, never through os.environ.

Two things improve as a result:

  1. The cache key is now the resolved path, so _DB_CACHE is live code instead of a comment about behavior that did not exist.
  2. The store no longer depends on when hermes_state was first imported. That was a real hazard, not a theoretical one — measured across a multi-file pytest run, goal rows landed in the first test's tmp home rather than the current test's, so per-test HERMES_HOME monkeypatching silently did not isolate them. The hermes_home fixture's comment in test_goals.py ("Bust the goal-module's DB cache for each test so it re-resolves HERMES_HOME") is now true.

Goals cannot collide across profiles either way, because session_id (the session key) is globally unique — which is why per-profile isolation buys nothing here.

Related Issue

No issue filed — found by inspection of the _DB_CACHE docstring.

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

  • hermes_cli/goals.py_get_session_db resolves get_process_hermes_home() / "state.db" and passes it to SessionDB(db_path=...) instead of relying on the import-time DEFAULT_DB_PATH fallback. _DB_CACHE is keyed by resolved path. The docstring now states the process-global scope, names the writer/reader split that requires it, and records why the explicit path replaces the import-time constant.
  • tests/hermes_cli/test_goals.py — new TestGoalStoreHomeScope:
    • test_reader_under_profile_override_still_sees_the_goal — writes a goal unbound (as session.create / command.dispatch do), reads it back inside set_hermes_home_override(other_profile), asserts the goal survives. This is the guard: it fails if anyone makes the store follow the active home.
    • test_db_path_is_the_process_home_not_an_import_time_constant — the DB follows the process HERMES_HOME rather than freezing at first import.

How to Test

  1. Run the reproduction above against main — the two paths print identical. On this branch they differ.
  2. Prove the guard bites: change get_process_hermes_home to get_hermes_home on both lines in _get_session_db, then run pytest tests/hermes_cli/test_goals.py::TestGoalStoreHomeScope -qtest_reader_under_profile_override_still_sees_the_goal fails with assert None is not None. Revert.
  3. Full goal surface, CI-shaped runner:
scripts/run_tests.sh tests/hermes_cli/test_goals.py tests/tui_gateway/test_goal_session_scope.py tests/tui_gateway/test_goal_turn_timeout.py tests/agent/test_compression_rotation_state.py tests/tui_gateway/test_goal_command.py tests/gateway/test_goal_status_notice.py tests/gateway/test_goal_max_turns_config.py tests/gateway/test_goal_verdict_send.py tests/cli/test_cli_goal_interrupt.py tests/hermes_cli/test_kanban_goal_mode.py tests/hermes_cli/test_desktop_spawn.py -q

11 files, 241 tests passed, 0 failed.

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass — not run in full. The 11 goal-related files above are green (241/241). The full suite is not green on this macOS host on main either (Linux-only tooling), so a full-suite claim would be noise.
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: macOS 15 (Darwin 25.6.0), Python 3.13

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — the _get_session_db docstring is the substance of this change
  • I've updated cli-config.yaml.example if I added/changed config keys — N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — N/A
  • I've considered cross-platform impact (Windows, macOS) — get_process_hermes_home already handles the platform-native default; no path handling added
  • I've updated tool descriptions/schemas if I changed tool behavior — N/A

Screenshots / Logs

=== Summary: 11 files, 241 tests passed, 0 failed (100% complete) in 25.2s (4 workers) ===

Guard proven to bite (implementation temporarily flipped to get_hermes_home()):

>       assert loaded is not None, "goal store followed the per-turn override"
E       AssertionError: goal store followed the per-turn override
E       assert None is not None
FAILED tests/hermes_cli/test_goals.py::TestGoalStoreHomeScope::test_reader_under_profile_override_still_sees_the_goal

Follow-up (not in this PR)

command.dispatch binds no profile home at all, so in app-global remote mode every slash command resolves config, plugins, skills and quick-commands against the gateway's launch profile even for sessions running under another one. Some of that may be intentional (cf. get_process_hermes_home's "dashboard-owned assets" doctrine); some may not. It needs per-command judgment and has wide blast radius, so it is deliberately out of scope here. Note that /goal must be excluded from any such change — this store depends on command.dispatch staying unbound, which is why TestGoalStoreHomeScope exists.

🤖 Generated with Claude Code

…ccidental

`goals._get_session_db` cached one SessionDB per `get_hermes_home()` path and
its docstring claimed "we cache one instance per hermes_home path so profile
switches still pick up the right DB." That claim was false. It built
`SessionDB()` with no argument, and `SessionDB.__init__` falls back to
`hermes_state.DEFAULT_DB_PATH` — a module-level constant evaluated at import
time. Every SessionDB the goal store handed out pointed at whichever home was
active when `hermes_state` first got imported, so the cache key was dead code
documenting behavior the process did not have:

    os.environ["HERMES_HOME"] = tempfile.mkdtemp()
    db1 = goals._get_session_db()
    os.environ["HERMES_HOME"] = tempfile.mkdtemp()
    goals._DB_CACHE.clear()
    db2 = goals._get_session_db()
    # db1.db_path == db2.db_path

The tempting fix — follow the active home — is the wrong one, and would break
the goal loop. A goal's writers and its reader sit on opposite sides of the
per-turn HERMES_HOME binding. In `tui_gateway/server.py` the desktop gateway
writes goals from `session.create` (`spawn --goal`, which computes
`profile_home` but never binds it) and from `command.dispatch` (`/goal`, which
binds no profile home either, and which `/goal` reaches deliberately rather
than the slash-worker subprocess — see `_PENDING_INPUT_COMMANDS`). It reads
them back in the post-turn continuation hook, which DOES run inside
`set_hermes_home_override(profile_home)`. Make the store follow the active home
and, for any session under a non-launch profile, `/goal` writes one database
while the hook reads another, `is_active()` returns False, and the Ralph loop
silently never fires — the same stranded-loop failure #319 just fixed, arrived
at from the other direction. Verified: flipping this function to
`get_hermes_home()` fails the new `test_reader_under_profile_override_still_
sees_the_goal` with `loaded is None`.

So the scope stays process-global — but stated rather than inherited by
accident. `_get_session_db` now resolves `get_process_hermes_home() /
"state.db"` and passes it explicitly. `get_process_hermes_home` is the existing
accessor for exactly this: it reads the process env and ignores the
context-local override. Production behavior is unchanged, because both gateways
bind the profile through a contextvar and never through `os.environ`.

Two things improve as a result. The cache key is now the resolved path, so it
is live code instead of a comment about behavior that did not exist. And the
store no longer depends on *when* `hermes_state` was first imported — a real
hazard: measured across a multi-file pytest run, goal rows landed in the FIRST
test's tmp home rather than the current test's, so per-test `HERMES_HOME`
monkeypatching silently did not isolate them.

Goals cannot collide across profiles either way, because `session_id` (the
session key) is globally unique.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@OmarB97
OmarB97 merged commit 8bdc37d into main Aug 2, 2026
31 checks passed
@OmarB97
OmarB97 deleted the fix/goal-store-process-home-fork-20260802 branch August 2, 2026 15:20
OmarB97 added a commit that referenced this pull request Aug 2, 2026
…equest (#331)

Plugin discovery is lazy, and several tools call `_ensure_plugins_discovered()`
from inside an agent turn — `tts_tool`, `web_tools`, `video_generation_tool`,
`browser_tool`. The gateway's turn handler binds
`set_hermes_home_override(session_profile_home)` for the duration of a turn, and
`discover_and_load` resolved its user plugin dir with `get_hermes_home()`.

So whichever session happened to trigger the FIRST discovery decided the whole
process's plugin registry, permanently — the singleton never rescans. In a
backend serving several profiles the launch profile could lose its own plugins
AND inherit another profile's, depending only on which chat ran a
plugin-touching tool first. Reproduced with two homes: discovery fired under a
profile override, and afterwards the launch profile's own `/launch-cmd` was
gone while the other profile's `/profile-cmd` answered for it.

Bind the process home around the sweep. That covers the user plugin dir, the
enabled/disabled allow-lists (`load_config` follows the override too, so
changing only the directory would have scanned one home while gating on
another), and anything a plugin's own `register()` reads — one consistent scope
instead of a mix.

The process home is the only coherent scope here: plugins register
process-global capabilities (tools, hooks, middleware, platforms, the context
engine) that every session on the backend shares, and `PluginManager` has no
lock, so re-scoping per request would mean clearing registrations out from
under live turns. Same rationale as the dashboard's user plugin dir in
`hermes_cli/web_server.py` and the goal store in #320.

This does NOT give non-launch profiles their own plugins when one backend
serves many — that needs a per-profile registry and is left alone deliberately.
It makes the existing scope deterministic and correct for the per-profile
deployments that are the norm: a dedicated backend or slash worker exports
HERMES_HOME at spawn, so the process home IS that profile's home there.

Co-authored-by: Omar Baradei <omar@kostudios.io>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant