fix(desktop): kill interpreter-exec'd profile backends on delete + refresh profile rail on focus - #88228
Merged
Conversation
Two independent bugs let a deleted profile reappear / leave orphaned resources on next launch: 1. hermes_cli/profiles.py's backend-process scanner required argv[0] to resolve to an executable literally named "hermes". Electron's pool-backend spawn resolves the hermes console-script shim's path and execs it via the interpreter directly (python3 /path/to/hermes ...), so argv[0] reports as "python3" and the scanner never matched the running backend -- delete removed the profile's files but left its live backend process running (still bound to a port via uvicorn), which accumulates across repeated delete/recreate cycles. 2. The desktop sidebar's ProfileRail only refreshed its cached profile list once, on mount, so a delete/create/rename from another surface (another window, or the CLI) left a stale ghost entry until something unrelated triggered a refetch. Note: a delete via this window's own Manage-Profiles view already refreshes the shared $profiles atom ProfileRail subscribes to (confirmed by reading refreshProfiles() and handleConfirmDelete()) -- this fix only covers the cross-window/cross- process staleness gap, not a duplicate of the already-merged #57329's Manage-Profiles rail-refresh work. Fix 1: recognize a python-interpreter argv[0] exec'ing a hermes-named console-script shim via argv[1]. Fix 2: refresh the profile list on window focus/visibilitychange, matching the existing pattern used elsewhere in the sidebar (sidebar/index.tsx, use-background-sync.ts, star-map.tsx, use-gateway-boot.ts all use the same focus+visibilitychange pattern). ## Related work already on main PR #57329 (merged) fixed the *headline* symptom from issue #52279 (deleted profile respawns) via a different, non-overlapping mechanism: routing profile-delete through the primary backend instead of spawning a fresh pool backend, plus a separate recreation guard in ensure_hermes_home() (#49435, merged) that makes a backend spawned into a deleted profile's directory raise FileNotFoundError instead of silently recreating it. This PR is NOT a duplicate of that fix. Verified: even with both of those merged, a backend process that survives because of gap #1 above still holds a bound port via uvicorn -- it just can no longer resurrect the profile directory. That's real resource-hygiene, not a symptom already covered. Gap #2 touches a different file/component (ProfileRail / profile-switcher.tsx) than #57329's rail-refresh half (which touched the Manage-Profiles view's own $profiles.ts / index.tsx) and covers a distinct staleness path (cross-window/cross-process, not same-window delete-then-refresh). Tests: tests/hermes_cli/test_profiles.py -- 156 passed (existing + regression coverage for the argv[0] python-interpreter detection case). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… scripts
External review (Fable) caught a real false-positive widening in the
original commit: the new argv[1] script-name check reused the loose
`script_name == "hermes" or script_name.startswith("hermes")` pattern
(copy-pasted from the exe_name check above it), but argv[1] can be ANY
user-invoked python script path when argv[0] is a bare interpreter --
unlike a directly-resolved executable name, where a false match on the
substring is rare. A user's own script named e.g. "hermes-notes.py" or
"hermes-unrelated-tool" run via `python3 <script>` would be misidentified
as the console-script shim and become killable by profile delete.
Match against the actual known console-script entry points instead
(pyproject.toml [project.scripts]: hermes, hermes-agent, hermes-acp),
stripping the script's extension before comparing.
Added 2 regression tests: one confirms the false-positive case is now
rejected (fails against the pre-fix loose-match code, confirmed via a
scripted revert), the other confirms the other two real entry points
(hermes-agent, hermes-acp) still match via the shebang-exec path.
Tests: tests/hermes_cli/test_profiles.py -- 158 passed (156 previous + 2
new).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… a tested hook Addresses review feedback from the hermes-sweeper (salvageability=high, keep_open): "The new focus/visibility listener behavior lacks a runtime UI regression test... no ProfileRail test." Rendering the full ProfileRail component for this would drag in drag-and-drop, dialogs, hotkeys, and i18n unrelated to what needs testing. Instead, extracted the focus/visibilitychange wiring into its own use-profile-rail-refresh-on-active hook, matching this exact directory's own established convention (use-profile-prewarm.ts is the same shape: a small side-effect hook pulled out of ProfileRail specifically so it's unit-testable in isolation). Added 6 tests covering exactly what the review asked for: refresh on mount, refresh on window focus, refresh on visibilitychange while visible, NO refresh on visibilitychange while hidden, listener cleanup on unmount, and no listener accumulation across repeated mount/unmount cycles. Verified the tests have real teeth: simulated the exact bug this PR originally fixed (dropped the cleanup return, leaving listeners attached after unmount) and confirmed 4 of 6 tests correctly fail against it -- including "no accumulate listeners" showing 7 calls instead of 1, the exact leaked-listener signature. Restored the real fix and all 6 pass. ProfileRail itself is otherwise unchanged in behavior -- this is a pure extraction (same effect, same dependencies, same cleanup), not a behavior change. Full sidebar test suite: 93 passed across 12 files (up from 87 across 11), 0 regressions. Python side unaffected: 158 passed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
alt-glitch
added
type/bug
This reverts commit a7ba91e.
Contributor
૮ >ﻌ< ა ci reviewran on 3ee22a6 — Revert "ci: touch ci.yml to force workflow re-parse"
|
8 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
hermes profile deletenow finds and terminates Desktop-spawned backends whose console-script shim was exec'd through the interpreter (python3 /path/to/hermes --profile X serve), and the Desktop profile rail refreshes when the window regains focus/visibility — so deleting a work profile no longer leaves a zombie backend running or a ghost entry in the sidebar.Salvage of #72152 by @adurham (3 commits cherry-picked onto current main, authorship preserved). Root cause: the backend scanner in
_profile_bound_backend_pids()requiredargv[0]to be ahermes-named executable; Electron'sfindOnPath('hermes')spawn reportsargv[0]=python3, so the scanner never matched and delete left the profile's live backend bound to its port. Separately,ProfileRailonly refreshed its cached profile list on mount.Changes
hermes_cli/profiles.py: recognize interpreter-exec'd shims —python*argv[0]+ a known console-script name (hermes/hermes-agent/hermes-acp) inargv[1]. Decoy scripts (hermes-notes.pyetc.) explicitly rejected.apps/desktop/src/app/chat/sidebar/: extracteduseProfileRailRefreshOnActivehook — refresh on mount + windowfocus+ visiblevisibilitychange, with listener cleanup.Validation
tests/hermes_cli/test_profiles.pycheck:lintpython3 <shim> --profile selena serveprocess, scanner from origin/mainhermes-notes.pyNOT matchedComplements #88120 (deletion gate / respawn race) — this covers the residual-process and stale-UI halves.
Closes #72152.
Infographic