Skip to content

Commit 4d72f2e

Browse files
committed
fix(update): record terminal status when the update job is cancelled
Cancellation lands on the job's await points and skipped the failure handler, releasing the update lock while the status file still said RUNNING. Catch CancelledError, write a terminal FAILED status, and re-raise.
1 parent 92e95af commit 4d72f2e

2 files changed

Lines changed: 63 additions & 0 deletions

File tree

src/pythinker_code/ui/shell/update_orchestrator.py

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -443,6 +443,27 @@ async def run_update_job(
443443
)
444444
)
445445
return reported_result
446+
except asyncio.CancelledError:
447+
# Cancellation lands on the await points (do_update, the smoke-check
448+
# thread, retry sleeps) and would otherwise skip the failure handler
449+
# below, releasing the lock with the job still recorded as RUNNING —
450+
# a stale "in progress" status with no process behind it. Record a
451+
# terminal state, then propagate. The smoke-check subprocess is not
452+
# interrupted mid-flight, but its own timeout bounds it.
453+
message = "Update job cancelled."
454+
append_update_log(message)
455+
write_update_status(
456+
_new_status(
457+
job_id=job_id,
458+
state=UpdateJobState.FAILED,
459+
source=source,
460+
started_at=started_at,
461+
finished_at=time.time(),
462+
result=UpdateResult.FAILED.name,
463+
message=message,
464+
)
465+
)
466+
raise
446467
except Exception as exc:
447468
message = f"Update failed: {exc}"
448469
append_update_log(message)

tests/ui_and_conv/test_update_orchestrator.py

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -236,6 +236,48 @@ def fake_smoke(**_kw):
236236
assert "retrying" in log
237237

238238

239+
@pytest.mark.asyncio
240+
async def test_update_job_cancellation_records_terminal_status_and_releases_lock(
241+
monkeypatch, tmp_path
242+
):
243+
"""Cancelling the job mid-await must not leave a stale RUNNING status."""
244+
import asyncio
245+
246+
_isolate_update_files(monkeypatch, tmp_path)
247+
248+
async def fake_do_update(
249+
*, print_output: bool, intent: update.UpdateIntent, output_callback=None
250+
):
251+
return update.UpdateResult.UPDATED
252+
253+
monkeypatch.setattr(update, "do_update", fake_do_update)
254+
255+
started = asyncio.Event()
256+
257+
async def hanging_smoke_check(**_kw):
258+
started.set()
259+
await asyncio.sleep(60)
260+
return (True, "unreachable")
261+
262+
monkeypatch.setattr(orchestrator, "_run_smoke_check_with_retry", hanging_smoke_check)
263+
264+
task = asyncio.create_task(
265+
orchestrator.run_update_job(
266+
print_output=False, intent=update.UpdateIntent.INSTALL, source="test"
267+
)
268+
)
269+
await started.wait()
270+
task.cancel()
271+
with pytest.raises(asyncio.CancelledError):
272+
await task
273+
274+
assert not orchestrator.UPDATE_LOCK_FILE.exists()
275+
status = orchestrator.read_update_status()
276+
assert status is not None
277+
assert status.state is orchestrator.UpdateJobState.FAILED
278+
assert "cancelled" in (status.message or "").lower()
279+
280+
239281
@pytest.mark.asyncio
240282
async def test_run_update_prompt_routes_check_through_runner(monkeypatch):
241283
calls: list[update.UpdateIntent] = []

0 commit comments

Comments
 (0)