Skip to content

fix(desktop): skip ensureBackend after profile-delete teardown to prevent respawn loop (#52279) - #52301

Closed
liuhao1024 wants to merge 1 commit into
NousResearch:mainfrom
liuhao1024:fix/desktop-profile-delete-respawn
Closed

fix(desktop): skip ensureBackend after profile-delete teardown to prevent respawn loop (#52279)#52301
liuhao1024 wants to merge 1 commit into
NousResearch:mainfrom
liuhao1024:fix/desktop-profile-delete-respawn

Conversation

@liuhao1024

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes a respawn loop where deleting a non-default profile via the Desktop UI causes the profile directory to be recreated indefinitely, accumulating zombie backend processes.

Related Issue

Fixes #52279

Type of Change

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

Changes Made

  • apps/desktop/electron/main.cjs: Make prepareProfileDeleteRequest return the torn-down profile name. In the hermes:api IPC handler, use this return value to route the DELETE request to the primary backend instead of spawning a fresh pool backend for the just-deleted profile.
  • apps/desktop/electron/profile-delete-respawn.test.cjs: Structural tests verifying prepareProfileDeleteRequest returns the profile name and the IPC handler uses it to skip ensureBackend.

How to Test

  1. Create a non-default profile via the Desktop UI or hermes profile create test-profile
  2. Delete the profile via the Desktop UI
  3. Verify the profile directory is removed and does not reappear after Desktop restart
  4. Verify no zombie hermes dashboard --profile test-profile processes remain (ps aux | grep "hermes.*dashboard.*profile")
  5. Run node --test apps/desktop/electron/profile-delete-respawn.test.cjs — both tests should pass

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run node --test apps/desktop/electron/profile-delete-respawn.test.cjs and all tests pass
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: macOS

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — or N/A
  • I've updated cli-config.yaml.example if I added/changed config keys — or N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A
  • I've updated tool descriptions/schemas if I changed tool behavior — or N/A

Code Intelligence

  • Analyzed: apps/desktop/electron/main.cjs (prepareProfileDeleteRequest, ensureBackend, hermes:api handler)
  • Blast radius: LOW — affects only profile-delete IPC path in Desktop; no change to CLI, gateway, or non-desktop flows
  • Related patterns: teardownPoolBackendAndWait lifecycle, ensure_hermes_home() directory creation, backendPool management

…vent respawn loop

When the renderer sends a DELETE /api/profiles/{name} request, the IPC
handler tears down the profile's pool backend (or primary backend) via
prepareProfileDeleteRequest.  However, the very next line calls
ensureBackend(profile), which spawns a fresh pool backend for the just-
deleted profile.  The new backend's startup path calls ensure_hermes_home(),
which recreates the profile directory — defeating the deletion and leaving
the process as a zombie.

On the next Desktop restart the cycle repeats: the profile directory exists,
the Desktop spawns a backend, the backend recreates the directory after
deletion, and PIDs accumulate indefinitely.

Fix: make prepareProfileDeleteRequest return the torn-down profile name.
The IPC handler uses this to route the DELETE to the primary backend
instead of spawning a new pool backend for the deleted profile.

Fixes NousResearch#52279

@tonydwb tonydwb left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review Summary

Verdict: Approved

Well-scoped bug fix for a respawn loop in profile deletion. The fix correctly routes profile-delete requests to the primary backend instead of spawning a fresh pool backend. Good documentation in code comments explaining the root cause. Dedicated test file with structural tests verifies the fix.

Changes:

  • apps/desktop/electron/main.cjs: Returns torn-down profile name from prepareProfileDeleteRequest, routes to primary backend after delete
  • apps/desktop/electron/profile-delete-respawn.test.cjs: New test file with structural tests

Clean, minimal fix with clear root cause analysis. LGTM.

Reviewed by Hermes Agent

@OutThisLife

Copy link
Copy Markdown
Collaborator

Superseded by #57329, which carries your commit as-is (cherry-picked, authorship + Co-authored-by preserved).

Your diagnosis was exactly right: a pool backend spawned for the just-deleted profile calls ensure_hermes_home() and recreates the directory, defeating the delete. Routing the delete request to the primary backend is the correct Desktop-side fix, and it pairs cleanly with the CLI recreation guard (#49435) landing separately. I folded it together with the rail-refresh fix (#49335) so the two Desktop halves of #47368 review and land as one behavior. Closes #52279 moves to #57329. Thanks @liuhao1024!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/desktop Electron desktop app (apps/desktop/*) P3 Low — cosmetic, nice to have type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Deleted profile respawns indefinitely, accumulating zombie backend processes

4 participants