Skip to content

fix(profiles): ignore cron-only directory shells in profile discovery - #94840

Open
Nicolas-Formenton wants to merge 2 commits into
NousResearch:mainfrom
Nicolas-Formenton:fix/profile-discovery-ignore-runtime-shells
Open

Nicolas-Formenton wants to merge 2 commits into
NousResearch:mainfrom
Nicolas-Formenton:fix/profile-discovery-ignore-runtime-shells

Conversation

@Nicolas-Formenton

Copy link
Copy Markdown
Contributor

What does this PR do?

Stops runtime-only directory shells under profiles/ from being discovered as
real profiles. A deleted profile can be resurrected as an empty directory by
runtime subsystems — most visibly the cron ticker, which recreates
<profile>/cron/ to write ticker_heartbeat — and that shell then reappeared
in hermes profile list and was served again by multiplexed gateways.

list_profiles() and profiles_to_serve() treated any directory under
profiles/ as a valid profile. This PR requires at least one durable identity
marker before treating a directory as a profile:

  • config.yaml
  • .env
  • SOUL.md
  • profile.yaml
  • auth.json
  • state.db

Every one of these exists from profile creation onward, so real profiles are
unaffected; a cron-only shell fails the check and stays invisible. The check is
a bounded stat scan (≤6 stats per candidate), keeping profiles_to_serve's
documented cheap-scan contract intact.

Related upstream work in the same problem family: #94604 (ticker recreating
archived homes), #90141 (tombstones so logging cannot resurrect profiles),
#59554 (stale HERMES_HOME at startup), issue #69934 ("Ghost profiles. Profiles
re-appear after deleting"). Those address other resurrection paths; none
changes what counts as a discoverable profile, which is the gap this closes.

Related Issue

Fixes the discovery half of the ghost-profile cluster (#69934); no dedicated
issue found for the cron-shell variant.

Type of Change

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

Changes Made

  • hermes_cli/profiles.py
    • Added _PROFILE_IDENTITY_MARKERS + _is_profile_directory().
    • list_profiles() and profiles_to_serve() use it instead of bare is_dir().
  • tests/hermes_cli/test_profiles.py
    • test_ignores_cron_only_profile_shells (list path).
    • test_on_ignores_cron_only_profile_shells (serve path).

How to Test

  1. Delete a profile, then let the desktop's cron ticker tick once over its home:
    profiles/<name>/cron/ticker_heartbeat reappears.
  2. hermes profile list before: the deleted name shows up again.
  3. After: it stays gone; real profiles are unchanged.
  4. pytest tests/hermes_cli/test_profiles.py -q → both new tests pass
    (2 pre-existing Windows-environment failures on this machine are identical
    with and without the patch; full file: 52 passed / 6 failed patched vs
    50 passed / 6 failed clean).

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature
  • I've run the relevant test suites and all tests pass
  • I've added tests for my changes (required for bug fixes)
  • I've tested on my platform: Windows 11

Documentation & Housekeeping

  • Documentation updates — N/A (internal discovery predicate)
  • Cross-platform impact considered — pure pathlib checks, no platform-specific calls

A deleted profile can be resurrected as an empty directory shell by
runtime subsystems (the cron ticker recreates <profile>/cron to write
ticker_heartbeat). list_profiles() and profiles_to_serve() treated any
directory under profiles/ as a valid profile, so the shell reappeared
in
 Profile          Model                        Gateway      Alias        Distribution
 ───────────────    ───────────────────────────    ───────────    ───────────    ────────────────────
 ◆default         stealth/ox-alpha             running      —            —
  edgezenn        tencent/hy3:free             stopped      edgezenn     —
  macos           stealth/ox-alpha             stopped      macos        —
  quant-hft       stealth/ox-alpha             stopped      quant-hft    —
  saf-auditor     gpt-5.6-sol                  stopped      saf-auditor  —
  saf-money       gpt-5.6-sol                  stopped      saf-money    —
  saf-scout       gpt-5.6-luna                 stopped      saf-scout    —
  saf-strategist  gpt-5.6-sol                  stopped      saf-strategist —
  saf-writer      gpt-5.6-terra                stopped      saf-writer   —
  vest            stealth/ox-alpha             stopped      —            —
  vestcardimpla   —                            stopped      —            —
  vestcardqa      —                            stopped      —            —
  vestcardreview  —                            stopped      —            — and was served again by a multiplexed gateway.

Both discovery paths now require at least one durable identity marker
(config.yaml, .env, SOUL.md, profile.yaml, auth.json, or state.db)
before treating a directory as a profile. A runtime-only shell fails
that check and stays invisible; a real profile is unaffected because
every one of those files exists from creation onward.

Adds regression tests for both list_profiles and
profiles_to_serve(multiplex=True).
@Nicolas-Formenton
Nicolas-Formenton force-pushed the fix/profile-discovery-ignore-runtime-shells branch from eddcafe to 8694b6f Compare August 25, 2026 15:26
@alt-glitch alt-glitch added type/bug Something isn't working comp/cli CLI entry point, hermes_cli/, setup wizard area/profiles Multi-profile isolation, HERMES_HOME scoping P2 Medium — degraded but workaround exists labels Aug 25, 2026
@Enough1122

Copy link
Copy Markdown
Contributor

AI code review — automated review for reference, author can ignore or act on any point.

This PR fixes a profile-discovery edge case where cron subsystems recreate a deleted profile's directory (e.g., for cron job state), causing an empty "ghost" directory to appear in list_profiles() and profiles_to_serve() as if it were a real profile. The fix introduces _is_profile_directory in hermes_cli/profiles.py:992 which checks for the presence of at least one "identity marker" file (config.yaml, .env, SOUL.md, profile.yaml, auth.json, state.db) before treating a directory as a profile. Both list_profiles and profiles_to_serve are updated to use the new check instead of the bare is_dir() test. Tests cover both code paths with a ghost directory containing only a cron/ subdirectory. Clean, minimal fix.

A few concerns:

  1. hermes_cli/profiles.py:982 — the _PROFILE_IDENTITY_MARKERS tuple is comprehensive but may be too strict for freshly created profiles. If create_profile hasn't been called yet (or was interrupted), a partially initialized profile directory might exist with only some markers — e.g., just config.yaml — and would correctly pass the check. But a profile that was created via an older version of Hermes that didn't write state.db or auth.json would need at least one of the other markers. This is fine as long as every profile creation path writes at least one marker file. Worth verifying that create_profile always writes at least one of these files atomically before the directory is considered "created."

  2. hermes_cli/profiles.py:992 — _is_profile_directory calls path.is_dir() and then up to 6 is_file() checks per directory. For a profiles root with many entries, this is O(N*M) filesystem stat calls. The previous code was O(N) with a single is_dir(). In practice the profiles root has few entries, so this is negligible — but if the markers list grows, consider short-circuiting after the first hit (which any() already does) and documenting the expected N.

  3. tests/hermes_cli/test_profiles.py:492 — the ghost directory test creates ghost/cron/ but doesn't test the case where a ghost directory contains a file that isn't a marker (e.g., a cron.log file or a .lock file). If a cron subsystem writes a .lock file into the profile directory, _is_profile_directory correctly rejects it (since .lock isn't in the markers), but there's no test for this scenario. Adding a test with a non-marker file would strengthen the regression guard.

  4. hermes_cli/profiles.py:1033 — the change from entry.is_dir() to _is_profile_directory(entry) in list_profiles changes the filtering but doesn't update the profiles_to_serve docstring or any user-facing documentation. If a user previously relied on seeing all directories (even empty ones) in hermes profile list, this is a behavior change. It's almost certainly an improvement, but worth noting in a changelog or release note.

…contributor email

- test_cron_profile_homes_follow_allowlist created its fixture profiles
  as bare directories; profile discovery now requires a durable identity
  marker, so the fixture writes an empty config.yaml (same contract a
  real profile satisfies).
- Add contributors/emails mapping for nicolas@users.noreply.github.com
  so the contributor attribution check passes.

This branch has not been deployed

No deployments
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 type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants