Skip to content

fix(update): don't report success when the Desktop rebuild failed (supersedes #88359, #87984) - #89838

Merged
OutThisLife merged 2 commits into
mainfrom
bb/update-desktop-rebuild-status
Aug 19, 2026
Merged

fix(update): don't report success when the Desktop rebuild failed (supersedes #88359, #87984)#89838
OutThisLife merged 2 commits into
mainfrom
bb/update-desktop-rebuild-status

Conversation

@OutThisLife

Copy link
Copy Markdown
Collaborator

What does this PR do?

Supersedes #88359 and #87984.

hermes update treated a failed Desktop pack as non-fatal: it printed an early warning, then still ended with ✓ Update complete!. Python moved on; Hermes.exe stayed on the previous build. That is the summary half of #88251 (the get-windows binding recovery is #88233).

_rebuild_desktop_after_update now returns False only when a rebuild was attempted and failed. Both the git and ZIP paths then print ⚠ Update partially complete instead of the success banner, and gateway mode writes 1 to .update_exit_code so /update watchers do not claim success.

The success-path output is unchanged when the Desktop rebuild succeeds or is not needed.

Credit: @JoaoMarcos44 (exit status / withhold the banner) and @liuhao1024 (return-value contract and tests).

Related Issue

Related to #88251. Supersedes #88359, #87984.

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✅ Tests (adding or improving test coverage)

Changes Made

  • hermes_cli/update_cmd.py: _rebuild_desktop_after_update() returns bool; shared _print_update_summary() and _write_gateway_update_exit_code().
  • Git, ZIP, and ZIP-fallback paths withhold ✓ Update complete! when the rebuild failed.
  • Gateway mode writes .update_exit_code=1 in that case.
  • tests/hermes_cli/test_update_desktop_stale_warning.py: rebuild contract, banner, combined node+desktop failure, exit-code file.

How to Test

  1. scripts/run_tests.sh tests/hermes_cli/test_update_desktop_stale_warning.py tests/hermes_cli/test_cmd_update.py tests/hermes_cli/test_update_version_report.py -q — 8 new tests plus existing update coverage.
  2. Force a desktop pack failure and run hermes update — the run should end with ⚠ Update partially complete and a retry hint, not ✓ Update complete!.

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 added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: macOS 15

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
  • I've updated tool descriptions/schemas if I changed tool behavior — or N/A

hermes update treated a failed desktop pack as non-fatal and still printed
✓ Update complete!, so Windows users kept running an old Hermes.exe after a
"successful" update. Withhold the success banner, surface the stale app in
the summary, and write .update_exit_code=1 for gateway watchers.

Supersedes #88359, #87984.

Co-authored-by: joaomarcos <joaomarcosdias444@gmail.com>
Co-authored-by: liuhao1024 <sunsky.lau@gmail.com>
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/cli CLI entry point, hermes_cli/, setup wizard comp/desktop Electron desktop app (apps/desktop/*) platform/windows Native Windows-specific behavior or breakage area/install-update Installer, updater, packaging, wheels, doctor sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows labels Aug 19, 2026
@OutThisLife
OutThisLife enabled auto-merge August 19, 2026 07:40
@OutThisLife

Copy link
Copy Markdown
Collaborator Author

Kicking CI: the ci.yml run died before scheduling any jobs (workflow-file-issue, 0 jobs, cannot retry). Reopening to retrigger.

auto-merge was automatically disabled August 19, 2026 07:40

Pull request was closed

@OutThisLife OutThisLife reopened this Aug 19, 2026
@OutThisLife
OutThisLife enabled auto-merge August 19, 2026 07:40
@OutThisLife
OutThisLife force-pushed the bb/update-desktop-rebuild-status branch from d8a4bbb to ff3dbed Compare August 19, 2026 07:42
@OutThisLife
OutThisLife merged commit d2433c1 into main Aug 19, 2026
25 of 45 checks passed
@OutThisLife
OutThisLife deleted the bb/update-desktop-rebuild-status branch August 19, 2026 07:44
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-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades 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.

2 participants