Skip to content

fix(profiles): reserve "main" + enforce _RESERVED_NAMES in create_profile - #17880

Closed
cola-runner wants to merge 1 commit into
NousResearch:mainfrom
cola-runner:codex/fix-profile-reserved-name-collision
Closed

cola-runner wants to merge 1 commit into
NousResearch:mainfrom
cola-runner:codex/fix-profile-reserved-name-collision

Conversation

@cola-runner

Copy link
Copy Markdown

Fixes #17879.

TL;DR

hermes profile create main succeeded and produced session keys with the agent:main:* prefix — the same prefix the default profile uses — silently re-creating the cross-profile collision that #12099 / the in-flight #12266 fix exists to eliminate. Same hole let root, hermes, test, tmp, sudo through. _RESERVED_NAMES was defined in hermes_cli/profiles.py but only consulted by check_alias_collision() (wrapper-script creation), not by profile creation itself.

This PR adds "main" to _RESERVED_NAMES and makes create_profile() reject any reserved name with a uniform message.

Why this is independent of #12266

I noticed this while reviewing #12266 (which is excellent and which I think should land — see #12102 close note). #12266 fixes the runtime behaviour of session-key construction so two profiles with different names don't collide. This PR fixes the input-validation layer so a user can't ask the runtime to construct a "main"-named profile in the first place. The two fixes are orthogonal and can land in either order:

Diff

hermes_cli/profiles.py            | +9 / -4
tests/hermes_cli/test_profiles.py | +53 / -0

Net 5 lines of production change.

Behavior matrix

Command Before this PR After this PR
hermes profile create coder ✓ created ✓ created (unchanged)
hermes profile create default ✗ "Cannot create… built-in" ✗ "…reserved. Reserved names: …"
hermes profile create main ✓ created (bug) ✗ "…reserved"
hermes profile create root ✓ created (bug) ✗ "…reserved"
hermes profile create hermes / test / tmp / sudo ✓ created (bug) ✗ "…reserved"
rename_profile, set_active_profile, etc. on a legacy "main" profile works works (validate_profile_name unchanged)

Scope — explicitly NOT changed

  • validate_profile_name() stays permissive. It's called by 8 non-creation paths (rename, delete, set_active, export, import, resolve_env, …). Tightening it would break any user who somehow already has a legacy "main" profile on disk — they should still be able to rename or delete it. Only creating a new reserved-name profile is what we reject.
  • No change to _resolve_session_key_prefix or build_session_key. That layer is fix(session): scope build_session_key by active profile, preserve default (#12099) #12266's scope and this PR doesn't touch it.
  • No migration of existing "main" profiles. Out of scope; a follow-up could detect and warn at startup, but the realistic exposure is near-zero (you'd have had to deliberately type hermes profile create main, which was undocumented).

Tests

tests/hermes_cli/test_profiles.py::TestReservedNames adds 9 cases:

  • Parametrised across all 7 reserved names (main, hermes, default, test, tmp, root, sudo) — each must raise ValueError with match="reserved".
  • Error message must name the offending value AND list hermes, default, main so the user can correct without reading source.
  • Reserved set drift canary: if someone later changes _RESERVED_NAMES, this test fails and forces an intentional update.
  • Bug-shaped check: failed creation must not leave an orphan directory.

All 102 existing tests/hermes_cli/test_profiles.py cases still pass on this branch (scripts/run_tests.sh tests/hermes_cli/test_profiles.py). The legacy test_default_raises_value_error continues to pass because the new error message still contains the literal string "default".

Pre-empted review questions

Q. Why is the new error message verbose?
The reserved set is small (7 names) and listing them is a lot cheaper than making the user grep profiles.py. If you'd rather a terser message, happy to drop the list.

Q. Why parametrise _RESERVED_NAMES in tests instead of hardcoding ["main"]?
The bug is that the frozenset existed but wasn't enforced. Parametrising every name pins all seven so a future contributor who adds a new reserved name automatically gets a passing test, and one who removes the enforcement gets a failing one.

Q. Should "default" be removed from _RESERVED_NAMES since validate_profile_name already special-cases it?
No — keeping "default" in _RESERVED_NAMES makes the creation-side rejection uniform (one code path, one message). The validate_profile_name special case is for the non-creation paths (rename, delete, etc.) where "default" is a valid alias for the built-in profile.

…file

Fixes #17879.

`hermes profile create main` succeeded today and produced session keys with
the `agent:main:*` prefix — the exact prefix the default profile uses —
re-introducing the cross-profile collision #12099 (and the in-flight #12266
fix) is meant to eliminate. The same hole let `root`, `hermes`, `test`,
`tmp`, and `sudo` through too: `_RESERVED_NAMES` was defined but only
consumed by `check_alias_collision()` (a wrapper-script-creation check),
not by `validate_profile_name()` or `create_profile()`.

Fix:
- Add "main" to `_RESERVED_NAMES`.
- Replace the one-name `if name == "default"` hardcode in `create_profile()`
  with a uniform `if name in _RESERVED_NAMES` check (the new error message
  still names the offending value and lists all reserved names).

`validate_profile_name()` is intentionally left permissive so existing
callers (`rename_profile`, `delete_profile`, `set_active_profile`,
`export_profile`, `import_profile`, `resolve_profile_env`) keep working
for any legacy `"main"` profile that may already exist on disk — only
*creation* of a new reserved-name profile is restricted.

Orthogonal to #12266: even after that PR lands, a user who runs
`hermes profile create main` still hits the original collision because
`_resolve_session_key_prefix("main")` evaluates to `"agent:main"`. This
PR closes that gap permanently.

Tests: `tests/hermes_cli/test_profiles.py::TestReservedNames` parametrises
all seven reserved names and verifies (a) each is rejected, (b) the error
message lists the offending name plus the full reserved set, (c) no
orphan directory is left behind. All 102 existing profile tests still pass.
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/cli CLI entry point, hermes_cli/, setup wizard labels Apr 30, 2026
@teknium1

Copy link
Copy Markdown
Collaborator

Thanks for identifying the namespace collision. The current-main premise is valid: create_profile() permits main after validation (hermes_cli/profiles.py:1031-1041), while gateway/session.py:865-867 maps both default and named main profiles to agent:main.

Problems

  • Adding main to _RESERVED_NAMES would not preserve legacy main profiles on current main. delete_profile() validates at hermes_cli/profiles.py:1472; rename_profile() validates both names at :2157-2158; and resolve_profile_env() validates at :2216.
  • The exact reserved-set assertion is a change-detector snapshot, contrary to AGENTS.md:1309-1355.
  • Creation also occurs through import (hermes_cli/profiles.py:2051-2081) and rename destinations (:2155-2179), so a create-only rule should cover those paths too.

Suggested changes

  • Introduce a new-profile-target validation path: reject main for create/import/rename destinations, but allow existing legacy main profiles to resolve, delete, and rename.
  • Keep behavioral rejection/no-orphan tests; replace the exact set snapshot with lifecycle compatibility coverage.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform area/profiles Multi-profile isolation, HERMES_HOME scoping labels Jul 12, 2026
@cola-runner cola-runner closed this by deleting the head repository Sep 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/profiles Multi-profile isolation, HERMES_HOME scoping comp/cli CLI entry point, hermes_cli/, setup wizard P2 Medium — degraded but workaround exists sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Profile creation accepts reserved names like 'main' (re-introduces #12099 in a corner case)

3 participants