Prevent duplicate active task submissions - #133
Conversation
tavateva
left a comment
There was a problem hiding this comment.
APPROVE — reviewed at head 14283b0.
Conformance check against the built-out #122 design (#122 (comment)), plus independent empirical verification.
Proposal conformance — all items delivered:
- Lifecycle state + immediate feedback:
OptionSubmitidle/starting/running states, disabled witharia-busy/aria-label;OptionsPanel'ssubmitLockref blocks re-entry synchronously on click. - EventSource lifecycle in
ServerContext: one active stream per URL (activeStreamsmap,closeStreambefore open),clearStreamnow actually closes the connection, provider- and page-level unmount cleanup, andlastEventIdtask filtering — the server already tags every event withid: task_id(verified inserver_api.py), and events failing the identity check are dropped. The stale-stream interleave from the issue is structurally gone. - Server single-active-task:
has_active_task409 gate on both POST endpoints;/delete/marks an enqueued taskcancelledand the worker skips it. The cancel-mark is race-safe: between the worker'squeue.get()returning and its status check there is no await point where a delete could interleave, so a delete either lands while the task is still queued (→ skipped) or afterstarted(→ 409). Running-task deletion is refused, with cooperative cancellation correctly left out of scope. - Restart sequencing: both pages now
awaitthe delete, surface its error as an alert, and abort the submit on failure. - Adjacent fixes: BackprojectPage clears
/backproject/(was/slice/); both SSE generators terminate onerror_event.
Empirical (local, at PR head):
- New
python/test/server/test_run_lifecycle.py: 3/3 pass — exercises the realcreate_apiand realprocess_requeststhrough ASGI transport (real seam, not stand-ins): duplicate POST → 409 until the active task finishes; running delete → 409; a cancelled queued task never reaches the handler; an error-status stream emits exactly oneerror_eventand terminates. - Full python suite: 171/172 — the single failure (
test_generate_tiff_write) is in files this PR doesn't touch; my ad-hoc venv carries a newer tifffile than the lock pin (SampleFormat tag-type change). Not attributable to the PR. npm run typecheck(node + web): clean. Targeted ESLint on the touched renderer files: 0 errors (the 2 prettier warnings in a wider sweep are pre-existing in untouched sibling components). Whitespace check clean; the pyproject change is exactly the declared dev-grouphttpxaddition.
Undeclared-decision sweep: clean. Active-task semantics (enqueued/started), the queued-cancel marker, running-delete refusal, error-stream termination, in-flight request dedup, and the no-JS-test-runner stance are each declared and grounded in the body.
Cost / ergonomics: the one visible behavior change — restarting a running task now refuses with an alert instead of silently stacking an invisible second run — is the designed honesty improvement from the issue, not a regression. Build/CI cost unchanged (dev-only dependency).
Verification depth: ran the lifecycle tests, full python suite, typecheck, and lint locally against the PR head; relied on CI for the Electron release builds and npm run test:release (all green).
Summary
Closes #122
Behavior and scope
enqueuedorstartedstate causes a409 Conflict.enqueuedtask marks it cancelled before removing it from the registry, so the queue worker skips it.startedtask returns409 Conflict; cooperative cancellation of a running pipeline remains out of scope.Starting…immediately, thenRunning…until the matching terminal stream event arrives.Validation
poetry run pytest— 172 passednpm run build— passed (the sandbox could not resolve Google Fonts, but the build completed)npm run test:release— 2 passedgit diff --check— passedpoetry check --lock— passed; Poetry emitted only the repository's pre-existing metadata deprecation warningsShared-basis audit
OptionsPanel/OptionSubmitform seam for immediate submission locking and feedback.ServerContextfetch and stream facilities rather than adding page-local request orEventSourcemanagers. Identical method+URL requests share one in-flight promise; each stream endpoint owns one currentEventSourceand accepts only events carrying its task ID.enqueuedorstartedfor the same task class; queued deletion uses a cancellation marker visible to the worker; running deletion is rejected because running-pipeline cancellation is explicitly outside this issue; terminal error events close streams just like completion events.httpxonly to the Python development dependency group to exercise the ASGI API and queue worker in regression tests.