diff --git a/apps/desktop/electron/main.ts b/apps/desktop/electron/main.ts index 0846a0b6ce3e..2103a6ff6570 100644 --- a/apps/desktop/electron/main.ts +++ b/apps/desktop/electron/main.ts @@ -2028,6 +2028,36 @@ function directoryExists(filePath) { } } +// Identity markers for a real local profile. ``hermes profile create`` writes +// config.yaml; ``ensure_hermes_home()`` writes SOUL.md on first launch; the +// live runtime persists state.db. Any of these being on disk distinguishes a +// genuine profile from an empty shell the desktop cron multiplex ticker +// recreated via mkdir -p of profiles//cron/ (issue #95188 path A+C). +// +// We accept any of the three because the create path on older installs may +// only have written config.yaml, while a fresh install seeds SOUL.md first +// and config.yaml only on the next config load. +const LOCAL_PROFILE_IDENTITY_MARKERS = ['config.yaml', 'SOUL.md', 'state.db'] + +function hasLocalProfileIdentityMarker(hermesHome, profile) { + if (!hermesHome || !profile) { + return false + } + + const profileDir = path.join(hermesHome, 'profiles', profile) + if (!directoryExists(profileDir)) { + return false + } + + return LOCAL_PROFILE_IDENTITY_MARKERS.some(marker => { + try { + return fs.statSync(path.join(profileDir, marker)).isFile() + } catch { + return false + } + }) +} + // --- in-app update mutual exclusion (#50238) ------------------------------- // The Tauri updater writes HERMES_HOME/.hermes-update-in-progress for the whole // duration of an `--update` run (see update.rs UpdateMarkerGuard). If the user @@ -10766,8 +10796,18 @@ async function spawnPoolBackend(profile, entry, opts: { forceLocal?: boolean; po // here, and logging "Starting" first left an orphaned line with no READY // and no exit — the exact undiagnosable burst signature in remote-gateway // user bundles (Aug 2026, Dash's report). - assertLocalProfileCanStart(profile, profileDeletionGate, key => - directoryExists(path.join(HERMES_HOME, 'profiles', key)) + // + // #95188 path C: the bare ``directoryExists`` check accepts an empty + // shell, which the desktop cron multiplex ticker recreates every 60 s + // for a deleted profile. Reject unless a durable identity marker is on + // disk — without one, ``ensure_hermes_home()`` would rebuild the full + // profile tree (config.yaml, state.db, …) the moment we spawn a + // backend here. + assertLocalProfileCanStart( + profile, + profileDeletionGate, + key => directoryExists(path.join(HERMES_HOME, 'profiles', key)), + key => hasLocalProfileIdentityMarker(HERMES_HOME, key) ) rememberLog(`Starting Hermes backend for profile "${profile}" via ${backend.label}`) diff --git a/apps/desktop/electron/profile-delete-routing.test.ts b/apps/desktop/electron/profile-delete-routing.test.ts index a63e864bc962..5a7bcfd68839 100644 --- a/apps/desktop/electron/profile-delete-routing.test.ts +++ b/apps/desktop/electron/profile-delete-routing.test.ts @@ -144,6 +144,28 @@ test('assertLocalProfileCanStart rejects a delayed retry after the profile direc assert.doesNotThrow(() => assertLocalProfileCanStart('selena', gate, profile => profile === 'selena')) }) +test('assertLocalProfileCanStart rejects a cron-shell home with no durable identity marker (#95188 path C)', () => { + // The cron ticker in `hermes_cli/web_server._start_desktop_cron_ticker` + // recreates `profiles//cron/` every 60s tick. An empty shell is + // enough to satisfy the bare directoryExists check, so a stale backend + // retry after the delete can still pass the spawn guard and rebuild the + // full profile. The fix: the guard must also confirm a durable identity + // marker (config.yaml / SOUL.md / state.db etc.) is present. + const gate = new ProfileDeletionGate() + const profileDirectoryExists = () => true // shell exists + const profileIdentityMarkerPresent = (key: string) => key !== 'researcher' // deleted profile has no marker + + assert.throws( + () => assertLocalProfileCanStart('researcher', gate, profileDirectoryExists, profileIdentityMarkerPresent), + /Profile "researcher" no longer exists/ + ) + assert.doesNotThrow(() => + assertLocalProfileCanStart('worker', gate, profileDirectoryExists, profileIdentityMarkerPresent) + ) + // Default profile is exempt from the identity check. + assert.doesNotThrow(() => assertLocalProfileCanStart('default', gate, profileDirectoryExists, profileIdentityMarkerPresent)) +}) + test('localProfilePoolKeys returns every local process scope for one profile', () => { assert.deepEqual(localProfilePoolKeys('Selena'), ['selena', 'conn:local::selena']) assert.deepEqual(localProfilePoolKeys(''), []) diff --git a/apps/desktop/electron/profile-delete-routing.ts b/apps/desktop/electron/profile-delete-routing.ts index 39bada976a7c..fcddefd0d219 100644 --- a/apps/desktop/electron/profile-delete-routing.ts +++ b/apps/desktop/electron/profile-delete-routing.ts @@ -180,11 +180,26 @@ export class ProfileDeletionGate { * Validate the final boundary before spawning a local profile backend. The * deletion gate closes the in-flight race; the directory check rejects a * delayed renderer retry after the DELETE request has already completed. + * + * The third predicate — `profileIdentityMarkerPresent` — rejects a stale + * shell where only an empty directory remains (issue #95188 path C). The + * desktop cron multiplex ticker writes a heartbeat into each profile's + * ``cron/`` subdir every 60 s, which silently ``mkdir -p``s the parent + * profile home. A bare directory-existence check then lets a delayed + * renderer reconnect pass the guard, spawn a real backend, and have + * ``ensure_hermes_home()`` rebuild the full profile tree (config.yaml, + * state.db, models_dev_cache.json, …). Requiring a durable identity + * marker (e.g. ``config.yaml``) ensures the spawn only succeeds for a + * profile that is genuinely on disk. + * + * The default profile is exempt from the identity check: its home is + * managed by the install and may legitimately lack a config.yaml. */ export function assertLocalProfileCanStart( profile: unknown, gate: ProfileDeletionGate, - profileDirectoryExists: (profile: string) => boolean + profileDirectoryExists: (profile: string) => boolean, + profileIdentityMarkerPresent: (profile: string) => boolean = () => true ): void { const key = String(profile ?? '') .trim() @@ -192,8 +207,14 @@ export function assertLocalProfileCanStart( gate.assertCanStart(key) - if (key && key !== 'default' && !profileDirectoryExists(key)) { - throw new Error(`Profile "${key}" no longer exists.`) + if (key && key !== 'default') { + if (!profileDirectoryExists(key)) { + throw new Error(`Profile "${key}" no longer exists.`) + } + + if (!profileIdentityMarkerPresent(key)) { + throw new Error(`Profile "${key}" no longer exists.`) + } } } diff --git a/apps/desktop/src/app/profiles/delete-profile-dialog.tsx b/apps/desktop/src/app/profiles/delete-profile-dialog.tsx index 6f02d630b5aa..4cf231f3279c 100644 --- a/apps/desktop/src/app/profiles/delete-profile-dialog.tsx +++ b/apps/desktop/src/app/profiles/delete-profile-dialog.tsx @@ -2,6 +2,7 @@ import { ConfirmDialog } from '@/components/ui/confirm-dialog' import { deleteProfile } from '@/hermes' import { useI18n } from '@/i18n' import { retireLocalProfileGateways } from '@/store/gateway' +import { forgetLastProfileForAllConnections } from '@/store/connections' import { $activeGatewayProfile, normalizeProfileKey, selectProfile, setActiveProfile } from '@/store/profile' // Thin wrapper over ConfirmDialog: owns the deleteProfile call, inherits @@ -44,6 +45,14 @@ export function DeleteProfileDialog({ return } + // Always purge the renderer-side ``lastProfileByConnection`` cache + // for this profile name, regardless of which workspace the user + // initiated the delete from (#95188 path B). Without this, the + // next boot's ``selectConnection()`` calls ``ensureGatewayAgent()`` + // with the deleted profile name and the backend's + // ``ensure_hermes_home()`` recreates the whole profile tree. + forgetLastProfileForAllConnections(profile.name) + // Deleting the profile the live gateway is on strands it on a dead // backend. Capture that before the delete; reset *after* the host's // onDeleted refresh so our reset is the last write — a refreshActiveProfile diff --git a/apps/desktop/src/store/connections.test.ts b/apps/desktop/src/store/connections.test.ts index 59e840f16ddf..f2334904ffff 100644 --- a/apps/desktop/src/store/connections.test.ts +++ b/apps/desktop/src/store/connections.test.ts @@ -35,6 +35,7 @@ const { $activeConnectionId, $connectionsRegistry, $pendingConnectionId, + forgetLastProfileForAllConnections, initializeConnectionsRegistry, refreshConnectionsRegistry, _resetConnectionsForTests, @@ -292,3 +293,38 @@ describe('selectConnection', () => { expect($connection.get()?.mode).toBe('remote') }) }) + +describe('lastProfileByConnection cache invalidation', () => { + // Regression for issue #95188 path B: deleting a named profile from a + // workspace other than the active one must clear its entry in the + // renderer localStorage cache, otherwise the next boot calls + // ensureGatewayAgent() with the dead profile name and spawns a real + // backend, whose ensure_hermes_home() rebuilds the directory tree. + it('forgetLastProfileForAllConnections removes the deleted profile from every connection (#95188)', async () => { + // Seed: the cache records 'researcher' for two separate connections. + // (Local was last using 'researcher'; a remote 'homelab' connection + // also remembers the same profile.) + setConnectionsRegistry(registry) + $connection.set({ connectionId: 'homelab', mode: 'remote', profile: 'default', registryScoped: true }) + $activeGatewayProfile.set('researcher') + await selectConnection('local') + $activeGatewayProfile.set('researcher') + $connection.set({ connectionId: 'homelab', mode: 'remote', profile: 'default', registryScoped: true }) + await selectConnection('homelab') + + // Delete the 'researcher' profile from any workspace (NOT the active one). + // The renderer must purge every cached entry pointing at it. + forgetLastProfileForAllConnections('researcher') + + // Reboot: initializeConnectionsRegistry should now dial 'default' on + // both connections (not 'researcher'). Reset and let boot restore do it. + _resetConnectionsForTests() + $connection.set(null) + $activeGatewayProfile.set('default') + list.mockResolvedValueOnce({ ...registry, lastUsed: 'local', launchMode: 'last-used' }) + + await initializeConnectionsRegistry() + + expect(ensureGatewayAgent).toHaveBeenLastCalledWith('local', 'default') + }) +}) diff --git a/apps/desktop/src/store/connections.ts b/apps/desktop/src/store/connections.ts index 472f5b59ec51..78cdad603103 100644 --- a/apps/desktop/src/store/connections.ts +++ b/apps/desktop/src/store/connections.ts @@ -78,6 +78,48 @@ export function _resetConnectionsForTests(): void { $pendingConnectionId.set(null) } +/** + * Forget the cached last-used profile for every connection when the named + * profile has just been deleted. + * + * Regression for issue #95188 (path B): the renderer persisted + * ``hermes.desktop.lastProfileByConnection`` so the next boot could dial + * the user's preferred profile per source. The delete dialog only reset + * that key when the deleted profile was the active foreground one + * (``if (wasActive)``); deleting from another workspace left the key + * pointing at the dead profile forever. On the next restart + * ``selectConnection()`` called ``ensureGatewayAgent()`` with the deleted + * profile name, which spawned a full backend whose + * ``ensure_hermes_home()`` rebuilt the entire profile tree. + * + * The function is best-effort and idempotent: an entry that doesn't + * match is left alone, and the change is persisted via the existing + * ``$lastProfileByConnection`` subscriber (so callers don't need to + * touch ``localStorage`` themselves). + */ +export function forgetLastProfileForAllConnections(profile: string): void { + const target = normalizeProfileKey(profile) + + if (!target || target === 'default') { + return + } + + const current = $lastProfileByConnection.get() + const next: Record = {} + + for (const [connectionId, lastProfile] of Object.entries(current)) { + if (normalizeProfileKey(lastProfile) === target) { + continue + } + + next[connectionId] = lastProfile + } + + if (Object.keys(next).length !== Object.keys(current).length) { + $lastProfileByConnection.set(next) + } +} + export function setConnectionsRegistry(registry: DesktopConnectionsRegistry): void { $connectionsRegistry.set(registry) } diff --git a/cron/scheduler_provider.py b/cron/scheduler_provider.py index 8b5281fd6792..46cb8078b790 100644 --- a/cron/scheduler_provider.py +++ b/cron/scheduler_provider.py @@ -638,8 +638,28 @@ def _start_multiplex( agent execution to that profile's home — mirroring how ``_profile_runtime_scope`` scopes the multiplexed inbound path and ``web_server.py`` scopes per-profile cron API calls. + + Issue #95188 path A: ``profiles_to_serve()`` snapshots the active + profile homes at backend startup and that list is frozen for the + lifetime of the dashboard. ``hermes profile delete `` cleans + the directory tree, but the cron ticker still holds a reference to + it and the next ``record_ticker_heartbeat()`` call's + ``ensure_dirs()`` does ``mkdir -p profiles//cron/`` — + resurrecting an empty shell. The Electron spawn guard's bare + ``directoryExists`` check then passes, a stale renderer reconnect + spawns a backend, and ``ensure_hermes_home()`` rebuilds the full + profile (config.yaml, state.db, logs, …). + + The fix: per tick, skip any profile whose home is no longer on + disk — do not enter ``use_cron_store()``, do not record a + heartbeat, do not run ``cron.tick()``. The user's profile list + update on the next ``profiles_to_serve()`` call will eventually + reflect the deletion, and any profile they intentionally + recreate gets picked up on the next gateway restart. """ import logging + from pathlib import Path + from cron.scheduler import tick as cron_tick from cron.jobs import ( clear_ticker_error, @@ -656,9 +676,27 @@ def _start_multiplex( [p[0] if isinstance(p, tuple) else p for p in profile_homes], ) - # Recovery + initial heartbeat for every profile. + def _home_alive(home) -> bool: + # ``Path.is_dir()`` returns False for missing or non-directory + # paths without raising — exactly the predicate we need to + # distinguish a real profile home from a ticker-resurrected + # shell that no longer exists or was renamed. + try: + return Path(str(home)).is_dir() + except OSError: + return False + + # Recovery + initial heartbeat for every profile. Skip homes that + # were deleted between ``profiles_to_serve()`` and the first tick. for entry in profile_homes: home = entry[1] if isinstance(entry, tuple) else entry + if not _home_alive(home): + logger.info( + "Skipping initial heartbeat for missing profile home %s " + "(deleted before the ticker started; #95188)", + home, + ) + continue home_token = set_hermes_home_override(str(home)) try: with use_cron_store(home): @@ -683,6 +721,17 @@ def _start_multiplex( else: for entry in profile_homes: home = entry[1] if isinstance(entry, tuple) else entry + # Skip-and-don't-mkdir for a deleted profile home + # (#95188 path A): the heartbeat's ``ensure_dirs`` + # would otherwise ``mkdir -p`` the parent and + # resurrect the deleted profile. + if not _home_alive(home): + logger.debug( + "Skipping tick for missing profile home %s " + "(deleted since ticker startup; #95188)", + home, + ) + continue home_token = set_hermes_home_override(str(home)) try: with use_cron_store(home): @@ -703,9 +752,13 @@ def _start_multiplex( consecutive_failures = _note_tick_failure(e, consecutive_failures) else: _tick_error = None - # Record per-profile heartbeat after each tick cycle. + # Record per-profile heartbeat after each tick cycle. The same + # missing-home skip applies — a deleted profile must not get + # any on-disk artefact. for entry in profile_homes: home = entry[1] if isinstance(entry, tuple) else entry + if not _home_alive(home): + continue home_token = set_hermes_home_override(str(home)) try: with use_cron_store(home): diff --git a/tests/cron/test_scheduler_provider.py b/tests/cron/test_scheduler_provider.py index 552fbdf5a11d..b506ce7f9499 100644 --- a/tests/cron/test_scheduler_provider.py +++ b/tests/cron/test_scheduler_provider.py @@ -589,6 +589,87 @@ def _boom(provider, base_url): # ── Multiplex profiles: cron per secondary profile (issue #69377) ───────── +def test_multiplex_ticker_skips_missing_profile_home_and_does_not_resurrect_it(tmp_path, monkeypatch): + """Regression for #95188 path A. + + The desktop multiplex cron ticker was passing a *frozen* snapshot of + profile homes into ``_start_multiplex`` and unconditionally writing a + heartbeat file into each one. When a named profile was deleted via + ``hermes profile delete``, the ticker's next pass would silently + ``mkdir -p profiles//cron`` and recreate the empty directory + shell — defeating the Electron spawn guard's directoryExists check + and allowing a stale renderer reconnect to spawn a real backend, + whose ``ensure_hermes_home()`` rebuilt the full profile tree. + + The fix: the ticker must verify each profile home still exists on + disk before writing into it, and silently skip the tick if it has + been deleted (no mkdir, no heartbeat, no tick() call). + """ + from cron.scheduler_provider import InProcessCronScheduler + + # Two profile directories: "default" is intact, "researcher" was just + # deleted by the user — only an empty cron/ shell remains, the rest + # of the profile tree is gone (config.yaml, state.db, etc. all + # removed). This mirrors the post-delete state on disk. + p1 = tmp_path / "default" + p2 = tmp_path / "researcher" + (p1 / "cron").mkdir(parents=True) + (p1 / "config.yaml").write_text("profile: default\n") + # NOTE: p2 has NO config.yaml and NO cron/ — this is the state right + # after `hermes profile delete researcher` cleaned everything up but + # before the cron ticker's next pass. + + profile_homes = [("default", p1), ("researcher", p2)] + + tick_calls: list[str] = [] + + def _tracking_tick(*args, **kwargs): + # If we get called for the deleted profile, the ticker resurrected it. + tick_calls.append("tick") + return 0 + + stop = threading.Event() + prov = InProcessCronScheduler() + + import cron.jobs as cron_jobs + heartbeat_calls: list[str] = [] + original_record = cron_jobs.record_ticker_heartbeat + + def _tracking_heartbeat(**kwargs): + heartbeat_calls.append("hb") + return None + + monkeypatch.setattr(cron_jobs, "record_ticker_heartbeat", _tracking_heartbeat) + + with patch("cron.scheduler.tick", side_effect=_tracking_tick): + t = threading.Thread( + target=prov.start, + args=(stop,), + kwargs={"interval": 0, "profile_homes": profile_homes}, + daemon=True, + ) + t.start() + # Allow several tick cycles. + deadline = time.monotonic() + 2 + while time.monotonic() < deadline: + time.sleep(0.01) + stop.set() + t.join(timeout=5) + + assert not t.is_alive() + + # The deleted profile's home directory MUST NOT exist — the ticker + # must skip it entirely rather than mkdir its cron/ shell. + assert not p2.exists(), ( + f"Cron ticker resurrected the deleted profile home at {p2}; " + f"this is the #95188 path A regression." + ) + # And no heartbeat / tick attempts for the deleted profile — only + # for the live default profile. + assert "tick" in tick_calls # ticker ran (default profile ticked) + assert p1.exists() # default profile home still alive + + def test_multiplex_ticker_ticks_each_profile_once(tmp_path, monkeypatch): """The multiplex cron scheduler calls tick() once per profile home, scoped via use_cron_store, so secondary-profile jobs actually fire