Skip to content

fix(desktop): identify foreign update blockers and close safe previews - #79018

Merged
teknium1 merged 3 commits into
NousResearch:mainfrom
ryankhart:feat/safe-preview-update-cleanup
Aug 16, 2026
Merged

teknium1 merged 3 commits into
NousResearch:mainfrom
ryankhart:feat/safe-preview-update-cleanup

Conversation

@ryankhart

@ryankhart ryankhart commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Summary

  • replace the generic Windows venv-blocker error with a dedicated blocker view for safe local previews and foreign processes
  • identify exact python -m http.server previews by label and port and offer Close previews and update
  • show foreign blockers by process name, PID, and a bounded, redacted command line without offering automatic termination
  • handle mixed blocker sets by stopping only verified safe previews, then rescanning while leaving foreign processes untouched
  • revalidate PID creation time and freshly classify the process’s current argv immediately before terminating a verified preview process tree
  • require a final clear scan before update handoff continues

Problem

Long-lived processes using the Hermes-managed venv can keep it open on Windows. This includes both disposable python -m http.server previews and unrelated user-owned Python scripts.

The updater correctly aborts rather than risk a partially updated environment, but the previous dialog described every blocker as another Hermes process and told users to close Hermes windows or terminals. That guidance was misleading and unactionable for foreign processes.

Closes #77422.

Behavior

  • Safe previews only: offer Close previews and update.
  • Foreign blockers only: identify the process and require the user to close it manually.
  • Mixed blockers: offer Close previews and check again. Only safe previews are stopped; foreign processes remain listed for manual action.

Safety boundary

Automatic termination remains restricted to blockers carrying scanner-issued local-preview metadata, safeToStop, and a process creation time.

Immediately before termination, the Python helper reconstructs the process, compares its creation time, and checks its current argv against the strict local-preview classifier. This protects against stale PIDs, PID reuse, truncated display command lines, and unrelated Python processes.

Foreign blocker command lines are diagnostic-only. Likely credentials are redacted, ambiguous sensitive command tails fail closed, and displayed command lines are bounded to 500 characters.

A final blocker scan must be clear before update handoff continues.

Scope

This addresses the blocker classification, diagnostics, and safe preview cleanup requested in #77422, including the user-owned Python watcher reproduction.

It does not broaden automatic termination to arbitrary Python processes. Atomic venv replacement or rollback after dependency-update failure remains out of scope and is related to #68760. The Hermes gateway exemption work in #78095 is related but separate.

Test plan

  • Electron blocker parsing/classification/termination tests (24 passed)
  • Focused Desktop blocker-view tests (6 passed)
  • Full Desktop UI suite (393 files, 3,412 tests)
  • Python scanner and termination tests (28 passed)
  • TypeScript typecheck
  • ESLint and Prettier on changed Desktop files
  • Ruff on changed Python files
  • Desktop production build

Localization note

Non-English strings were machine-assisted and have not been reviewed by native speakers. They follow the existing locale structure; corrections or a preference for English fallback are welcome.

Simulated screenshot

Safe preview blocker dialog

Rendered from the PR’s actual dialog component using mock blocker data; this was not captured during a live update. The screenshot shows the safe-preview-only state. Foreign-only and mixed states are covered by component tests.

@alt-glitch alt-glitch added type/bug Something isn't working comp/desktop Electron desktop app (apps/desktop/*) comp/cli CLI entry point, hermes_cli/, setup wizard platform/windows Native Windows-specific behavior or breakage area/install-update Installer, updater, packaging, wheels, doctor P2 Medium — degraded but workaround exists sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows labels Aug 5, 2026
@ryankhart
ryankhart marked this pull request as draft August 5, 2026 03:32
@ryankhart

Copy link
Copy Markdown
Contributor Author

Apologies, I should have had my AI agent wait for me to approve this first before publishing some minor personal information about me in the test code. I'm getting that fixed right now.

@ryankhart
ryankhart force-pushed the feat/safe-preview-update-cleanup branch from 9b44f30 to def9688 Compare August 5, 2026 03:44
@ryankhart

Copy link
Copy Markdown
Contributor Author

Fun story how I came up with this (skip if you don't care): I had Hermes Desktop made a website for me and host the server as a preview for me. Days later, that server was still running, and I didn't know where it was the shut it down, but it was causing Hermes Desktop to complain at me when I tried to update. So, I started up a new Hermes thread, and asked why does this keep happening when I try to update? It did some investigating and presented me with several options, and this was the recommended option. And it worked! I updated and restarted, and then I was like...wait, if Hermes Agent modified core files of Hermes Desktop itself, and Hermes Desktop updates itself, does that overwrite our fix? I asked Hermes about that, it presented several options, and recommended submitting a PR so that it's fixed for everyone, not just me.

@ryankhart
ryankhart marked this pull request as ready for review August 5, 2026 04:03
@ryankhart

Copy link
Copy Markdown
Contributor Author

Possible duplicate of #78095

@spfcraze

spfcraze commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

This was generated by AI during triage.

Summary:
Closes #77422 will auto-close the issue when this merges, but the new safe-close flow covers only exact -m http.server previews — the thread's second reproduction, a user-owned fenbi_session_refresh.py watcher on the managed venv, is not a preview and still gets the unchanged generic blocker message.

Problems:

  • The close-and-update view is gated on every blocker being a safe local preview: updates-overlay.tsx renders it only when apply.blockers.every(blocker => blocker.kind === 'local-preview' && blocker.safeToStop), and the classifier grants that kind only to an exact -m http.server argv (_classify_local_preview_args requires -m as the first argument after the interpreter).
  • The issue thread's second reproduction — a user-owned fenbi_session_refresh.py watcher running from the managed venv (comment on Update aborted by a non-Hermes python -m http.server process inside the install dir; dialog gives misleading 'Close other Hermes windows' advice #77422) — is not a preview, so it classifies as kind: 'other' with safeToStop: false; with it present, applyUpdates returns the pre-existing formatBlockerMessage text, which this diff does not touch.
  • The issue body asks foreign blockers to be named explicitly and the generic "another Hermes process" guidance to appear only when the blocker is Hermes-owned; the watcher case still receives the unchanged generic text.

Solution:
Extend the blocked view to foreign non-preview blockers (name them as foreign with PID and command line), so the issue's second reproduction gets the distinct handling the issue asks for before Closes #77422 fires.


Checked against def9688 — the tip of feat/safe-preview-update-cleanup when this was written — and b3aa561, main at the same moment.

@ryankhart

Copy link
Copy Markdown
Contributor Author

Related to #78095, but not a duplicate. #78095 fixes long-command-line Hermes gateways being misclassified as blockers. This PR handles user-launched python -m http.server previews that are genuine blockers and adds a guarded stop-and-retry Desktop workflow. Both touch _scan_venv_blockers.py, so their changes may need to be integrated during rebase.

@ryankhart ryankhart changed the title fix(desktop): close safe preview blockers before update fix(desktop): identify foreign update blockers and close safe previews Aug 8, 2026
@ryankhart

ryankhart commented Aug 8, 2026

Copy link
Copy Markdown
Contributor Author

Thanks. This was a valid gap and is now addressed in commit 894d4a367.

Foreign blockers now receive a dedicated diagnostic view showing the process name, PID, and a bounded/redacted command line. They never receive an automatic termination action.

For mixed blocker sets, the UI offers Close previews and check again. That action remains restricted to scanner-classified safe local previews; unrelated Python processes are left untouched and listed for manual closure.

Coverage now includes foreign-only, mixed, safe-preview-only, and command-line redaction cases. The full UI suite passes with 393 files and 3,412 tests. I’ve also updated the PR description to clarify the broader #77422 coverage and the remaining out-of-scope venv rollback work.

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

Labels

area/install-update Installer, updater, packaging, wheels, doctor comp/cli CLI entry point, hermes_cli/, setup wizard comp/desktop Electron desktop app (apps/desktop/*) P2 Medium — degraded but workaround exists platform/windows Native Windows-specific behavior or breakage sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Update aborted by a non-Hermes python -m http.server process inside the install dir; dialog gives misleading 'Close other Hermes windows' advice

4 participants