fix(worker): complete successful operations - #2608
Merged
benfrank241 merged 1 commit intoJul 8, 2026
Merged
Conversation
3 tasks
nicoloboschi
added a commit
that referenced
this pull request
Jul 20, 2026
execute_task completes an operation via _mark_operation_completed / _mark_operation_completed_and_fire_webhook, both of which wrapped the status='completed' commit in one transaction with fallible side-effects (webhook outbox insert, parent aggregation) and swallowed every exception. A hiccup in either rolled the completion back and dropped the error, leaving the operation stuck in 'processing' forever while the log already said the work was done (#2601). PR #2608 added a poller-side backstop that unstuck the row but silently lost the consolidation webhook. - On failure of the atomic outbox transaction, fall back to a completion-only commit and fire the consolidation webhook best-effort (non-transactional) instead of losing both. Happy path keeps the transactional-outbox guarantee; the failure path degrades to completed + delivered rather than stuck + lost. The best-effort fire only runs when the fallback actually transitioned the row, so there is no duplicate delivery. - Guard every completion UPDATE on `status NOT IN ('completed','failed', 'cancelled')` so an already-terminal row is never re-terminalized: keeps the engine idempotent with the poller backstop (#2608) and avoids double parent aggregation, while still completing pending/processing rows. Adds fast DB-free regression tests (fake connections) covering the happy path (no double-fire), the webhook-failure fallback, and the terminal-row no-op guard.
nicoloboschi
added a commit
that referenced
this pull request
Jul 20, 2026
…ts (#2823) execute_task completes an operation via _mark_operation_completed / _mark_operation_completed_and_fire_webhook, both of which wrapped the status='completed' commit in one transaction with fallible side-effects (webhook outbox insert, parent aggregation) and swallowed every exception. A hiccup in either rolled the completion back and dropped the error, leaving the operation stuck in 'processing' forever while the log already said the work was done (#2601). PR #2608 added a poller-side backstop that unstuck the row but silently lost the consolidation webhook. - On failure of the atomic outbox transaction, fall back to a completion-only commit and fire the consolidation webhook best-effort (non-transactional) instead of losing both. Happy path keeps the transactional-outbox guarantee; the failure path degrades to completed + delivered rather than stuck + lost. The best-effort fire only runs when the fallback actually transitioned the row, so there is no duplicate delivery. - Guard every completion UPDATE on `status NOT IN ('completed','failed', 'cancelled')` so an already-terminal row is never re-terminalized: keeps the engine idempotent with the poller backstop (#2608) and avoids double parent aggregation, while still completing pending/processing rows. Adds fast DB-free regression tests (fake connections) covering the happy path (no double-fire), the webhook-failure fallback, and the terminal-row no-op guard.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
_mark_completed()status-safe so it only updates rows still inprocessingFixes #2601.
Notes
The completion update is intentionally guarded with
status = 'processing'. If the executor already wrote a terminal state, for example a deterministic failure handled insideMemoryEngine.execute_task, the poller will not overwrite that row as completed.Testing
python3 -m py_compile hindsight-api-slim/hindsight_api/worker/poller.py hindsight-api-slim/tests/test_worker.pyuvx ruff@0.14.9 format --check hindsight-api-slim/hindsight_api/worker/poller.py hindsight-api-slim/tests/test_worker.pyuvx ruff@0.14.9 check --select F hindsight-api-slim/hindsight_api/worker/poller.py hindsight-api-slim/tests/test_worker.pygit diff --checkI attempted the focused pytest selection, but this sparse local checkout is missing runtime dependencies before the tests run (
hindsight_api.engine, thenasyncpg; the project environment also pulls packages that are not available here on macOS Python 3.14). The added tests use fake async connection objects for the new status guard and should run in the normal project test environment.