Skip to content

Commit e7eabc6

Browse files
committed
fix(update): retry post-install smoke check and surface verification failure
The smoke check ran once, immediately after the package-manager upgrade. Homebrew can report success moments before its opt launcher symlink is repointed at the new keg, so the probe exercised the old binary, reported the old version, and recorded a false VERIFICATION_FAILED — leaving the footer stuck on the stale update notice while the screen said "Updated successfully!". Retry the smoke check up to 3 times, 1s apart, off the event loop (asyncio.to_thread) so the probe can no longer block the shell either. When verification genuinely fails after all attempts, print the failure where the install success was shown so the on-screen outcome matches the recorded status.
1 parent 30d2cf3 commit e7eabc6

3 files changed

Lines changed: 89 additions & 6 deletions

File tree

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@ GitHub Releases page; `0.8.0` is the new starting line.
1515

1616
## Unreleased
1717

18+
- Retry the post-update smoke check briefly before recording `VERIFICATION_FAILED`, absorbing the Homebrew launcher-relink race that falsely failed successful upgrades, and print the verification failure on screen instead of leaving "Updated successfully!" as the last word when verification genuinely fails.
19+
1820
## 0.62.0 (2026-07-22)
1921

2022
- Treat the repository-root `.agents/` directory as local agent configuration and keep it out of version control.

src/pythinker_code/ui/shell/update_orchestrator.py

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
from __future__ import annotations
22

3+
import asyncio
34
import contextlib
45
import json
56
import os
@@ -408,7 +409,7 @@ async def run_update_job(
408409
append_update_log(message)
409410
_write_last_success(job_id=job_id, message=message)
410411
else:
411-
smoke_ok, smoke_message = run_post_install_smoke_check(
412+
smoke_ok, smoke_message = await _run_smoke_check_with_retry(
412413
target_version=_read_target_version()
413414
)
414415
append_update_log(smoke_message)
@@ -422,6 +423,13 @@ async def run_update_job(
422423
final_state = _result_state(reported_result)
423424
# Never promote a staged binary that can't even print --version.
424425
_finalize_native_staging(promote=False)
426+
# The install step has already printed its own success line;
427+
# leaving the screen at "Updated successfully!" while the
428+
# recorded state is VERIFICATION_FAILED would misreport the
429+
# outcome (the footer keeps the update notice for the same
430+
# reason). Surface the failure where the success was shown.
431+
if print_output:
432+
console.print(f"[{_get_tui_tokens().warning}]{message}[/]")
425433

426434
write_update_status(
427435
_new_status(
@@ -548,6 +556,35 @@ def _smoke_check_env() -> dict[str, str]:
548556
return env
549557

550558

559+
_SMOKE_CHECK_ATTEMPTS = 3
560+
_SMOKE_CHECK_RETRY_DELAY_SECONDS = 1.0
561+
562+
563+
async def _run_smoke_check_with_retry(target_version: str | None) -> tuple[bool, str]:
564+
"""Run the post-install smoke check, retrying transient failures.
565+
566+
Package managers can report success moments before the launcher they manage
567+
is repointed at the new install (observed with Homebrew's ``opt`` symlink):
568+
an immediate probe then exercises the OLD binary, reports the old version,
569+
and records a false ``VERIFICATION_FAILED``. A short retry window absorbs
570+
that race; a real bad install still fails every attempt. The subprocess
571+
probe runs off the event loop so a slow/hung binary cannot stall the shell.
572+
"""
573+
smoke_ok, smoke_message = False, "Smoke check did not run."
574+
for attempt in range(1, _SMOKE_CHECK_ATTEMPTS + 1):
575+
smoke_ok, smoke_message = await asyncio.to_thread(
576+
run_post_install_smoke_check, target_version=target_version
577+
)
578+
if smoke_ok or attempt == _SMOKE_CHECK_ATTEMPTS:
579+
break
580+
append_update_log(
581+
f"Smoke check attempt {attempt}/{_SMOKE_CHECK_ATTEMPTS} failed "
582+
f"({smoke_message}); retrying in {_SMOKE_CHECK_RETRY_DELAY_SECONDS:g}s..."
583+
)
584+
await asyncio.sleep(_SMOKE_CHECK_RETRY_DELAY_SECONDS)
585+
return smoke_ok, smoke_message
586+
587+
551588
def run_post_install_smoke_check(target_version: str | None = None) -> tuple[bool, str]:
552589
command = _smoke_check_command()
553590
try:

tests/ui_and_conv/test_update_orchestrator.py

Lines changed: 49 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -173,17 +173,23 @@ async def fake_do_update(
173173
return update.UpdateResult.UPDATED
174174

175175
monkeypatch.setattr(update, "do_update", fake_do_update)
176-
monkeypatch.setattr(
177-
orchestrator,
178-
"run_post_install_smoke_check",
179-
lambda **_kw: (False, "Smoke check failed: broken"),
180-
)
176+
monkeypatch.setattr(orchestrator, "_SMOKE_CHECK_RETRY_DELAY_SECONDS", 0.0)
177+
attempts = 0
178+
179+
def fake_smoke(**_kw):
180+
nonlocal attempts
181+
attempts += 1
182+
return (False, "Smoke check failed: broken")
183+
184+
monkeypatch.setattr(orchestrator, "run_post_install_smoke_check", fake_smoke)
181185

182186
result = await orchestrator.run_update_job(
183187
print_output=False, intent=update.UpdateIntent.INSTALL, source="test"
184188
)
185189

186190
assert result is update.UpdateResult.VERIFICATION_FAILED
191+
# A hard failure is retried before being reported — every attempt failed.
192+
assert attempts == orchestrator._SMOKE_CHECK_ATTEMPTS
187193
status = orchestrator.read_update_status()
188194
assert status is not None
189195
assert status.state is orchestrator.UpdateJobState.FAILED
@@ -192,6 +198,44 @@ async def fake_do_update(
192198
assert not orchestrator.UPDATE_LAST_SUCCESS_FILE.exists()
193199

194200

201+
@pytest.mark.asyncio
202+
async def test_update_job_smoke_check_retry_absorbs_launcher_relink_race(monkeypatch, tmp_path):
203+
"""A transiently stale launcher (e.g. brew's opt link mid-relink) must not
204+
record VERIFICATION_FAILED when a later attempt passes."""
205+
_isolate_update_files(monkeypatch, tmp_path)
206+
207+
async def fake_do_update(
208+
*, print_output: bool, intent: update.UpdateIntent, output_callback=None
209+
):
210+
return update.UpdateResult.UPDATED
211+
212+
monkeypatch.setattr(update, "do_update", fake_do_update)
213+
monkeypatch.setattr(orchestrator, "_SMOKE_CHECK_RETRY_DELAY_SECONDS", 0.0)
214+
attempts = 0
215+
216+
def fake_smoke(**_kw):
217+
nonlocal attempts
218+
attempts += 1
219+
if attempts == 1:
220+
return (False, "Smoke check reported 0.60.0, expected 0.62.0")
221+
return (True, "Smoke check passed: pythinker, version 0.62.0")
222+
223+
monkeypatch.setattr(orchestrator, "run_post_install_smoke_check", fake_smoke)
224+
225+
result = await orchestrator.run_update_job(
226+
print_output=False, intent=update.UpdateIntent.INSTALL, source="test"
227+
)
228+
229+
assert result is update.UpdateResult.UPDATED
230+
assert attempts == 2
231+
status = orchestrator.read_update_status()
232+
assert status is not None
233+
assert status.state is orchestrator.UpdateJobState.UPDATED
234+
assert orchestrator.UPDATE_LAST_SUCCESS_FILE.exists()
235+
log = "\n".join(orchestrator.read_update_log_tail())
236+
assert "retrying" in log
237+
238+
195239
@pytest.mark.asyncio
196240
async def test_run_update_prompt_routes_check_through_runner(monkeypatch):
197241
calls: list[update.UpdateIntent] = []

0 commit comments

Comments
 (0)