Code Review Summary

Verdict: Approved

Changes Overview

• Fixes Windows profile deletion reliability by stopping the pooled dashboard backend before removal
• Makes SOUL.md editable for default profile even in single-profile setups
• Surfaces a hard error when directory deletion fails instead of falsely reporting success

Analysis

Correctness

• The fix correctly addresses the Windows handle-exclusion issue where a running Hermes process keeps state.db/logs open
• The stopPoolBackend call + 300ms delay is a reasonable workaround
• The new RuntimeError when directory survives removal prevents false success reporting

Security

• No security concerns

Code Quality

• Excellent documentation in comments explaining the Windows handle issue and why the delay is needed
• The ProfilePill change (always show Manage) is appropriately justified with comments

Testing

• Good test coverage for the new RuntimeError behavior
• Test verifies both the exception and that the directory remains on disk

Reviewed by Hermes Agent

Thanks for this — the SOUL.md fix is still wanted, but main has moved since the approval and the PR now conflicts (CONFLICTING), so it needs a rebase and a trim before it can land. Most of Bug 2 is already handled upstream, and in a more robust way than this branch.

Bug 2, electron side: main already wires prepareProfileDeleteRequest() into the hermes:api handler (apps/desktop/electron/main.cjs). It deterministically waits for the target profile's backend to exit (teardownPoolBackendAndWait -> waitForBackendExit) rather than sleeping a fixed 300ms, also handles the primary/active profile (teardownPrimaryBackendAndWait), and validates the name against PROFILE_NAME_RE (the /^\/api\/profiles\/([^/]+)$/ regex here also matches a trailing query/hash). Re-adding the 300ms version would regress that path.

Bug 2, Python side: main's delete_profile already records remove_error and raises RuntimeError when removal fails, and cmd_profile already catches RuntimeError. The if profile_dir.exists(): raise added here overlaps with that and would double up. It does cover one extra case (rmtree returning without raising yet the dir surviving), but _make_writable re-raises on real failures, so that's mostly belt-and-suspenders.

Bug 1 is the part that's still genuinely unfixed on main: profile-switcher.tsx still gates the Manage overflow behind multiProfile, so a single-profile user can't reach the SOUL.md editor. That one-line ungating (plus the comment) is good as-is.

Could you rebase on main and reduce this to just the profile-switcher.tsx change? Drop the electron change (superseded) and the main.py/profiles.py changes (covered by the existing remove_error raise), unless you want to reconcile the exists() check into that existing path in a single spot. Happy to re-review once it's down to the SOUL.md fix. Not approving in the meantime since merging as-is would bring back the weaker delete handling.

Re-reviewed the trimmed PR — LGTM, approving.

Verified both "superseded by main" calls against origin/main:

• main.cjs — correct to drop. prepareProfileDeleteRequest + teardownPoolBackendAndWait already do a deterministic SIGTERM → await waitForBackendExit(...) (vs the original setTimeout(300ms)), plus active/primary-profile handling (primaryProfileKey() → switch to default + teardownPrimaryBackendAndWait) and PROFILE_NAME_RE validation. Strictly better than the removed change.
• profiles.py / main.py — also correct. profiles.py already accumulates remove_error and raises RuntimeError("Could not remove profile directory …") when the dir survives removal, with except RuntimeError handling downstream. The failed-deletion detection is covered.

Remaining change is a single commit touching only profile-switcher.tsx: ungating the Manage pill so a single-profile user can reach the manage overlay to edit the default profile's SOUL.md without creating a throwaway profile. multiProfile is still used for the named squares and the default↔all toggle, so nothing's orphaned. Comment accurately describes the new behavior.

Thanks for the clean rebase.