web: report a job finished only once everything finishing it is done #27

2026-10-10T23:29:07-07:00

dejima-agent commented

2026-10-10 23:29:07 -07:00

Summary generated by LLM agent

Fixes #18.

The job worker set status = "done" first, then removed the job's upload, and only set finished_at in a finally block after that. A client polling for done, whether the web UI or the tests' wait_status, could see a finished job with its upload still on disk and finished_at: null. That's why test_upload_dir_cleaned_up_after_success, test_job_has_timestamps and test_cover_upload_kept_after_success failed at random, and why CI kept going red for no reason.

Fix

The worker records the outcome (done, or failed with its error) and publishes status, error and finished_at together, under the lock, as its last step, after the upload cleanup. Cleanup moves inside the try. _cleanup_uploads never raises, so it can't fail a job that otherwise succeeded, and its docstring now says that instead of relying on the old ordering.

Testing

New regression test: upload cleanup is slowed by 0.3s, and a job seen as done must already have finished_at set and its upload removed. It fails every time against main and passes with this change.
The full test_web.py suite passed 5 runs in a row. It used to fail roughly once every two or three runs.
Full suite passes, except test_version (needs the package installed, as CI does).

**Summary generated by LLM agent** Fixes #18. The job worker set `status = "done"` first, then removed the job's upload, and only set `finished_at` in a `finally` block after that. A client polling for `done`, whether the web UI or the tests' `wait_status`, could see a finished job with its upload still on disk and `finished_at: null`. That's why `test_upload_dir_cleaned_up_after_success`, `test_job_has_timestamps` and `test_cover_upload_kept_after_success` failed at random, and why CI kept going red for no reason. ## Fix The worker records the outcome (`done`, or `failed` with its error) and publishes `status`, `error` and `finished_at` together, under the lock, as its last step, after the upload cleanup. Cleanup moves inside the `try`. `_cleanup_uploads` never raises, so it can't fail a job that otherwise succeeded, and its docstring now says that instead of relying on the old ordering. ## Testing - New regression test: upload cleanup is slowed by 0.3s, and a job seen as `done` must already have `finished_at` set and its upload removed. It fails every time against `main` and passes with this change. - The full `test_web.py` suite passed 5 runs in a row. It used to fail roughly once every two or three runs. - Full suite passes, except `test_version` (needs the package installed, as CI does).

dejima-agent added 1 commit 2026-10-10 23:29:08 -07:00

web: report a job finished only once everything finishing it is done

tests / test (pull_request_target) Successful in 3m47s

Details

da92e30ce8

The worker set status "done" before removing the job's upload and before
setting finished_at, so a client polling for done could see the upload
still there and finished_at null. Record the outcome and publish status,
error and finished_at together, under the lock, as the last step.

branderson approved these changes 2026-10-11 00:21:57 -07:00

branderson merged commit 0ffa5e3ad8 into main

2026-10-11 00:22:03 -07:00

branderson referenced this issue from a commit

2026-10-15 14:17:08 -07:00

Merge pull request 'web: report a job finished only once everything finishing it is done' (#27) from job-done-last into main

dejima-agent referenced this issue from a commit

2026-10-15 17:44:37 -07:00

Merge pull request 'web: report a job finished only once everything finishing it is done' (#27) from job-done-last into main

Sign in to join this conversation.