Skip to content

Prevent duplicate active task submissions - #133

Merged
den-sq merged 1 commit into
mainfrom
codex/issue-122-disable-repeat-start
Aug 14, 2026
Merged

Prevent duplicate active task submissions#133
den-sq merged 1 commit into
mainfrom
codex/issue-122-disable-repeat-start

Conversation

@den-sq

@den-sq den-sq commented Aug 14, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • disable the Start control immediately while a slice or backprojection request is starting or active
  • deduplicate identical in-flight requests, own one task-filtered event stream per endpoint, and surface request errors
  • reject duplicate active tasks server-side, skip deleted queued work, and terminate SSE streams on both success and error
  • await deletion before restarting and fix backprojection cleanup to clear its own request state

Closes #122

Behavior and scope

  • A task of the same type in enqueued or started state causes a 409 Conflict.
  • Deleting an enqueued task marks it cancelled before removing it from the registry, so the queue worker skips it.
  • Deleting a started task returns 409 Conflict; cooperative cancellation of a running pipeline remains out of scope.
  • The renderer shows Starting… immediately, then Running… until the matching terminal stream event arrives.

Validation

  • poetry run pytest — 172 passed
  • npm run build — passed (the sandbox could not resolve Google Fonts, but the build completed)
  • npm run test:release — 2 passed
  • targeted ESLint, Prettier, autopep8, and git diff --check — passed
  • poetry check --lock — passed; Poetry emitted only the repository's pre-existing metadata deprecation warnings

Shared-basis audit

  • Inspected and reused the existing OptionsPanel/OptionSubmit form seam for immediate submission locking and feedback.
  • Extended the existing ServerContext fetch and stream facilities rather than adding page-local request or EventSource managers. Identical method+URL requests share one in-flight promise; each stream endpoint owns one current EventSource and accepts only events carrying its task ID.
  • Reused the FastAPI task registry as the authority for active-task conflicts and the existing queue worker as the cancellation enforcement point. No second task registry or scheduler was introduced.
  • Load-bearing decisions: active means enqueued or started for 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.
  • Added httpx only to the Python development dependency group to exercise the ASGI API and queue worker in regression tests.
  • The renderer has no JavaScript test runner today, so this PR does not introduce a parallel test stack; renderer coverage is typecheck, lint, formatting, and production build validation, while lifecycle behavior is covered at the backend boundary.

@den-sq
den-sq marked this pull request as ready for review August 14, 2026 18:56

@tavateva tavateva left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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: OptionSubmit idle/starting/running states, disabled with aria-busy/aria-label; OptionsPanel's submitLock ref blocks re-entry synchronously on click.
  • EventSource lifecycle in ServerContext: one active stream per URL (activeStreams map, closeStream before open), clearStream now actually closes the connection, provider- and page-level unmount cleanup, and lastEventId task filtering — the server already tags every event with id: task_id (verified in server_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_task 409 gate on both POST endpoints; /delete/ marks an enqueued task cancelled and the worker skips it. The cancel-mark is race-safe: between the worker's queue.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 after started (→ 409). Running-task deletion is refused, with cooperative cancellation correctly left out of scope.
  • Restart sequencing: both pages now await the delete, surface its error as an alert, and abort the submit on failure.
  • Adjacent fixes: BackprojectPage clears /backproject/ (was /slice/); both SSE generators terminate on error_event.

Empirical (local, at PR head):

  • New python/test/server/test_run_lifecycle.py: 3/3 pass — exercises the real create_api and real process_requests through 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 one error_event and 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-group httpx addition.

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).

@den-sq
den-sq merged commit 2db4b6b into main Aug 14, 2026
7 checks passed
@den-sq
den-sq deleted the codex/issue-122-disable-repeat-start branch August 14, 2026 19:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Ouroboros response to multiple button presses is Counter-intuitive.

2 participants