[KoNit-K]

What does this PR do?

Fixes #106692. When Windows gateway relaunch fails liveness verification during fresh-checkout update completion, record the gateway restart as incomplete and fail the update instead of reporting the fleet healthy. The failed recovery attempt now disarms its resume_needed token, so the completion finally path and parent atexit fallback do not replay the same recovery attempt.

Related Issue

Fixes #106692

Type of Change

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

Changes Made

• hermes_cli/update_cmd_windows.py — preserve the incomplete restart outcome while disarming a failed Windows resume token.
• tests/hermes_cli/test_windows_update_restart_reconciliation.py — cover RuntimeError, no duplicate retry, and BaseException propagation.
• tests/hermes_cli/test_update_completion_process.py — exercise the fresh-checkout completion process path.

How to Test

• scripts/run_tests.sh tests/hermes_cli/test_windows_update_restart_reconciliation.py -q — 4 passed
• scripts/run_tests.sh tests/hermes_cli/test_update_completion_process.py -k completion_records_a_failed_windows_resume_without_replaying_it -vv — 1 passed
• scripts/run_tests.sh tests/hermes_cli/test_old_updater_takeover.py tests/hermes_cli/test_update_zip_completion.py -q — 35 and 23 passed

Evidence

• A failed Windows resume remains an incomplete gateway outcome and exits the update path with status 1.
• The completion token is cleared after that attempt, preventing the finally and atexit paths from retrying it.
• Six pre-existing completion boundary tests were blocked by the test environment's real Hermes Home I/O guard; this is recorded separately and is not reported as a passing result.

Checklist

Code

• ✅ I've read the Contributing Guide
• ✅ My commit messages follow Conventional Commits
• ✅ I searched for existing PRs to make sure this isn't a duplicate
• ✅ My PR contains only changes related to this fix
• ✅ I've run relevant tests locally (see How to Test)
• ✅ I've added tests for my changes
• ✅ I've tested on my platform: macOS

Documentation & Housekeeping

• ✅ Documentation update: N/A unless noted in Changes Made
• ✅ cli-config.yaml.example: N/A
• ✅ CONTRIBUTING.md or AGENTS.md: N/A
• ✅ Cross-platform impact considered
• ✅ Tool descriptions/schemas: N/A

[Enough1122]

AI code review — automated review for reference; please use your judgment.

PR #106702 — fix(update): soft-fail Windows gateway resume on already-up-to-date path

Verdict: Small, well-motivated robustness fix mirroring the pull path. Non-blocking notes only.

Summary: _finish_already_up_to_date called _resume_windows_gateways_after_update directly, so a Job-Object-teardown liveness miss (#48820) raised out of an otherwise successful checkout repair (#106692); the atexit handler then retried and emitted "Exception ignored in atexit". Now the resume is wrapped in try/except (warning printed, update outcome preserved) and resume_needed is cleared so the atexit callback becomes a no-op.

Findings (all Non-blocking):

• hermes_cli/update_cmd.py (diff L15-20): resume_needed is cleared even when the explicit resume succeeded. Correct — the work is done, and the atexit handler is a retry net, not a second resume. The tracking test (resume_attempts == [True, False], single warning) pins exactly-once semantics.
• Bare except Exception (diff L15) deliberately swallows resume errors into a printed warning, matching the pull path's _resume_windows_gateways_and_merge_outcome. Fail-open is right here (gateway liveness must not fail a good code update), and the exception text is preserved in the warning. BaseException (KeyboardInterrupt/SystemExit) still propagates — good.
• isinstance(_windows_gateway_resume, dict) guard (diff L19) before clearing: if the token is None/other, no clearing — confirm _resume_windows_gateways_after_update(None) itself is a safe no-op on that path (presumably; the non-Windows/None case flows through it today).
• Checkout-incomplete path still sys.exit(1) after the soft-failed resume (test L165-182): the fail-closed gate for a bad repair is preserved — resume soft-fail doesn't mask repair failure. Good.

Tests: New test_update_finish_gateway_soft_fail.py (+153) reproduces the #48820 liveness hole (launch OK, ready-poll empty) and asserts warning + no-raise, no double-fire, and preserved exit-1 on repair failure.

[KoNit-K]

Path C recheck of the Enough1122 AI review (all Non-blocking; no CHANGES_REQUESTED, no review threads). merge-tree --write-tree HEAD origin/main is clean; update_cmd.py on main only picked up an unrelated docs commit since this head. Fork CI workflows are action_required with zero jobs (label/approval gate), not a test failure.

Agreed — no code change:

1. Clearing resume_needed after a successful explicit resume is correct. The atexit handler in _cmd_update_impl is a retry net, not a second resume. Pinned by test_atexit_does_not_double_fire_resume_after_explicit_finish (resume_attempts == [True, False], single warning).

2. Bare except Exception matches _resume_windows_gateways_and_merge_outcome on the pull path. Fail-open is right: gateway liveness must not fail a good checkout repair. Exception text is preserved in the warning; BaseException (KeyboardInterrupt / SystemExit) still propagates.

3. _resume_windows_gateways_after_update(None) is a safe no-op. The function opens with if not token or not token.get("resume_needed"): return, so None (and any falsy token) returns immediately — no .get on None. _pause_windows_gateways_for_update returns None on non-Windows, and _cmd_update_impl only registers the atexit callback when the token is truthy (if _windows_gateway_resume:). The isinstance(..., dict) guard therefore only skips clearing when there is nothing to clear.

4. Checkout-incomplete still sys.exit(1) after the soft-failed resume. Fail-closed for a bad repair is unchanged; resume miss does not mask it. Pinned by test_checkout_incomplete_still_exits_after_soft_fail_resume.

Focused suite: scripts/run_tests.sh tests/hermes_cli/test_update_finish_gateway_soft_fail.py tests/hermes_cli/test_windows_gateway_job_teardown_48820.py tests/hermes_cli/test_update_handoff_exit.py -v — 3 files / 15 tests passed. Head remains 58e76a82a8. No commit/push; keeping #106702 open for maintainer review.

[Enough1122]

Ack — resume-once semantics, fail-open bare except, and None-safe token path accepted as designed; no-code-change agreed. Closed.

[KoNit-K]

Path C recheck after Enough1122 Ack (“no-code-change agreed. Closed.”).

No new actionable review: still no review threads, no CHANGES_REQUESTED, no test failures. Fork CI check-suites remain action_required with zero jobs (label/approval gate), not a code failure. merge-tree --write-tree HEAD origin/main vs 8d79c2ff57 is clean. Last comment was the Ack only — nothing to implement.

Independent delta vs current main is still unlanded:

• _finish_already_up_to_date on main still calls _resume_windows_gateways_after_update as a hard-fail (no try/except, no resume_needed clear).
• This branch still wraps that resume like the pull path, prints the recovery warning, and clears resume_needed so atexit does not double-fire.
• New test_update_finish_gateway_soft_fail.py is still unique to this PR.
• [Bug]: The hermes update never works directly #106692 is still open; no other open PR targets it.

Focused suite: scripts/run_tests.sh tests/hermes_cli/test_update_finish_gateway_soft_fail.py tests/hermes_cli/test_windows_gateway_job_teardown_48820.py tests/hermes_cli/test_update_handoff_exit.py -v — 3 files / 15 tests passed. Head remains 58e76a82a8. No commit/push; keeping #106702 open for maintainer review.

[KoNit-K]

@Enough1122 Following your review, I rebuilt this PR on current main in 8596e62. The earlier already-up-to-date call site is no longer the live completion path. The remaining root cause was that _resume_windows_gateways_and_merge_outcome recorded a Windows liveness RuntimeError as incomplete but left resume_needed set, so completion's finally retried the same recovery. The update now clears that token only after the caught operational failure; it retains incomplete, phase_errors, the receipt, and the exit-1 contract, so an unverified gateway is never reported healthy. BaseException still propagates. Verified: Windows reconciliation 4 passed; fresh-checkout completion regression 1 passed; related old-updater and ZIP suites 35 and 23 passed. Six pre-existing completion boundary tests remain blocked by the test environment's real Hermes Home I/O guard.

[KoNit-K]

Closing this older Windows-updater recovery patch to avoid maintaining a second, incompatible recovery policy.

On current main, merged #116804 already routes the "already up to date" Windows resume through the existing outcome handler and prevents the duplicate atexit callback. Merged #132386 subsequently establishes a different post-commit contract: a failed gateway restart is recorded as an owed, durable follow-up instead of making an otherwise committed update fail. This PR's remaining change clears resume_needed inside the exception handler and its tests expect a failure/exit-1 outcome. Retaining that behavior would work against the newer retry/receipt model rather than cleanly solving an outstanding integration gap.

This is a maintenance decision, not a claim that every Windows updater failure in #106692 has been solved. Any remaining regression should be reproduced against the current completion/recovery path and covered in that architecture, rather than by porting the stale token mutation. Thanks to the earlier reviewers and contributors.

References: #106692, #116804, #132386.