fix(agent): stop doubling the profile path in the system-prompt hint (#72894) - #74979
jeff-mettel wants to merge 2 commits into
Conversation
The named-profile branch of the "Active Hermes profile" hint built its
paths by appending `/profiles/{active_profile}` to `get_hermes_home()`.
But `_resolve_active_profile_name()` returns a non-default name *only*
when `get_hermes_home()` has already resolved under `<root>/profiles/`
— that is how it derives the name in the first place. Both scoping
mechanisms (a `HERMES_HOME=<root>/profiles/<name>` env var and the
multiplexer's `set_hermes_home_override` contextvar) satisfy that, so
the suffix always doubled.
The same branch used `get_hermes_home()` for the *default* profile's
data pointers, where the root was intended — placing them inside the
active profile.
On a real 4-profile install the hint rendered:
reads and writes ~/.hermes/profiles/via/profiles/via/
default profile's data lives at ~/.hermes/profiles/via/skills/
against actual paths of `~/.hermes/profiles/via/` and `~/.hermes/skills/`.
Use the session home directly as the profile home, and
`get_default_hermes_root()` for the root pointers. Every named-profile
session was shipping a prompt that named nonexistent directories and
mislabeled this profile's own skills/plugins/cron/memories as the
default profile's — the exact cross-profile confusion the hint and
`classify_cross_profile_target` exist to prevent.
The default-profile branch is unchanged.
Fixes NousResearch#72894
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
teknium1
left a comment
There was a problem hiding this comment.
Thanks for tracing this to the profile-home/root distinction. The current-main premise is confirmed: agent/system_prompt.py:423-425 appends /profiles/{active_profile} to get_hermes_home() and uses that same value for default-profile paths, while agent/file_safety.py:402-411 only returns a named profile when the active home is already under <root>/profiles.
Problems
- The new regression tests mock all three parts of the resolution chain (
get_hermes_home,get_default_hermes_root, and_resolve_active_profile_name). They validate template rendering but do not execute the real profile-resolution relationship that causes the defect.
Suggested changes
- Add one temporary-
HERMES_HOMEintegration-style prompt test for<root>/profiles/<name>without mocking those resolvers.hermes_constants.py:153-190already defines the required root behavior, andtests/test_hermes_constants.py:47-55covers its custom-root form.
The production hunk is narrow and its current-main context remains present; this is an automated hermes-sweeper review.
| monkeypatch.setattr( | ||
| system_prompt, "get_default_hermes_root", lambda: Path("/hermes") | ||
| ) | ||
| monkeypatch.setattr( |
There was a problem hiding this comment.
Please add a companion test using a real temporary HERMES_HOME=<root>/profiles/<name> instead of mocking all three resolvers. This regression depends on their composition, not only on the prompt template.
Review feedback: the previous tests mocked get_hermes_home, get_default_hermes_root and _resolve_active_profile_name, so they checked template rendering but never exercised the relationship that causes the defect — _resolve_active_profile_name returns a named profile only when the active home is already <root>/profiles/<name>, which is precisely why appending that suffix doubled it. Replace them with tests that set a real HERMES_HOME under a tmp root and mock no resolver. They assert the chain first (_resolve_active_profile_name == "coder", get_hermes_home == the profile dir, get_default_hermes_root == the root), then the rendered prompt. Adds the default-profile branch the same way. This also removes the order-dependent failures noted in the PR description. Module-attribute monkeypatching stops taking effect once other files in tests/agent/ have run, which is what made the mocked tests fail in a full-directory run; the un-mocked tests are immune. tests/agent/ now reports 87 failures — identical to the baseline on unmodified main, with none in this file. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Accepted and fixed in The mocked tests are replaced with ones that set a real assert _resolve_active_profile_name() == "coder"
assert get_hermes_home() == profile_home # <root>/profiles/coder
assert get_default_hermes_root() == root # <root>then that the rendered hint uses the profile home once, never This also resolves the order-dependent failures flagged in the PR description, and the cause turns out to be the same thing the review identified. Module-attribute monkeypatching stops taking effect once other files in Measured on
Only the integration test fails without the production hunk: The production change itself is unchanged from the original review. Filed by an AI agent (Claude Opus 5) operating autonomously on @jeff-mettel's behalf. Failure counts were measured with and without the patch against |
|
Thanks — I narrowed #76155 to the non-overlapping cron side only. It now leaves the system-prompt fix and resolver-chain tests to this PR, while adding active-profile cron script guidance and create/update preflight for missing files. No overlapping files remain between the PRs. |
|
Thanks @Maxson-dev — confirmed on my side: #76155 now touches only the cron subcommand, cron tools, their tests, and docs, while this PR remains Filed by an AI agent (Claude Fable 5) operating autonomously on @jeff-mettel's behalf. File lists were compared via the GitHub API before posting. |
SummaryTwenty-three PRs address or reference this issue complex. The core profile-prompt lineage is #52670/#52676/#57223/#66219/#66453/#66725/#67192/#68117/#74979: #67192 removed the literal Related pull requests
DuplicatesProfile-prompt lineage: #52670, #52676, #66219, and parts of #57223 are earlier root-aware variants; #66453, #66725, and the system-prompt hunk of #67192 share the flawed Suggested consolidationKeep #74979 open with a salvage path: retain its narrow Complex graphflowchart 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
I72894(["issue #72894 (open)"])
subgraph Dup68117 ["PRs duplicating each other"]
P68117["PR #68117 (closed)"]
P74979["PR #74979 (open)"]
end
P74979 -->|best fix| I72894
class I72894 open
class P68117 closed
class P74979 open
class P68117 best
class P74979 best
class P74979 target
click I72894 "https://github.com/NousResearch/hermes-agent/issues/72894"
click P68117 "https://github.com/NousResearch/hermes-agent/pull/68117"
click P74979 "https://github.com/NousResearch/hermes-agent/pull/74979"
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 23 pull requests and 7 issues in this complex. Each diff was read against this issue; Assessment working set: 160 kB of PR diffs, 83 kB of issue/PR text, 36 kB of discussion (93 comments), 58 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch. |
|
Salvaged onto current main and merged via #86352 — your two commits (the ambient-branch fix and the real-resolver-chain integration tests) are cherry-picked with your authorship preserved (merge commit 4691eb5). The file had moved under the #86313 agent-home refactor, which is why a direct merge wasn't possible. Thanks @jeff-mettel! |
What & why
Fixes #72894.
The named-profile branch of the "Active Hermes profile" hint appended
/profiles/{active_profile}toget_hermes_home(). But_resolve_active_profile_name()(agent/file_safety.py) returns a non-default name only whenget_hermes_home()has already resolved under<root>/profiles/— that's precisely how it derives the name. Both scoping mechanisms satisfy that:HERMES_HOME=<root>/profiles/<name>in the environmentgateway/run.py_profile_runtime_scope→set_hermes_home_override)So in that branch the home is always already profile-scoped and the appended suffix always doubles. The same branch then used
get_hermes_home()for the default profile's data pointers, where the root was intended — placing them inside the active profile.Verified live
On a real 4-profile install (
upstream/main@ 8defb9f):Every named-profile session ships a prompt naming directories that don't exist, and telling the model this profile's own
skills/plugins/cron/memoriesbelong to the default profile — the exact cross-profile confusion the hint andclassify_cross_profile_targetexist to prevent.The change
Use
get_hermes_home()directly as the profile home (it already is<root>/profiles/<name>whenever this branch runs), andget_default_hermes_root()— already import-safe, and documented to return<root>in profile mode — for the default-profile pointers.The default-profile branch is untouched.
Tests
TestNamedProfileHintintests/agent/test_system_prompt.py:test_session_home_is_not_doubled— asserts…/profiles/mac/and explicitly that…/profiles/mac/profiles/macnever appearstest_default_profile_data_points_at_the_root— root pointers, and that none land inside the active profiletest_default_profile_hint_is_unchanged— the default branch still renders as beforeNote on the full-directory run
These three tests pass standalone and for the whole file. In a full
pytest tests/agent/run they fail — but so do two pre-existing tests in this same file on unmodifiedmain(test_build_system_prompt_records_stable_prefix,test_coding_prompt_preserves_legacy_workspace_order), with the same signature: module-levelmonkeypatch.setattronagent.system_promptstops taking effect once other files in the directory have run, so the real identity/paths leak through.Measured on this machine,
pytest tests/agent/:mainI could not isolate the polluting module within a reasonable timebox, and did not want to reshape unrelated tests to chase it. Flagging it rather than papering over it — if CI is green on that directory, the pollution is local to my environment and these will pass there too; if it isn't, the pre-existing pair is the better place to fix the root cause, and I'm happy to follow up separately.
Platforms
macOS 15 (Darwin 25.5.0), Python 3.11. Path handling goes through the existing
hermes_constantshelpers, which already handle native Windows roots.Duplicate check
Related history, none of it landed:
2bae4df) — only swapped a hardcoded~/.hermesforget_hermes_home(), which is what introduced the doubling for named profiles.The defect is still present on
8defb9fd6, as the live output above shows.Authored by an AI agent (Claude Opus 5) operating autonomously on @jeff-mettel's behalf: the defect was traced, the patch written, and the tests run and verified end-to-end before submission.