[kirtimanmishrazipstack]

What

• GET execution/?stuck=true narrows the execution list to stuck runs, and each row gets stuck_since.
• GET execution/stuck-count/ returns the stuck runs per entity (API, ETL, TASK, WF) and the threshold.
• POST execution/abort/ stops up to 100 runs and returns one outcome per id: aborted, already_finished, not_permitted or failed.

Why

• The Frontend v1 executions page needs to show stuck runs and abort them (PB-T08).

How

• A run is stuck when it is PENDING or EXECUTING and neither the run nor its files have changed for longer than the threshold.
• Owners of the run's deployment, pipeline or workflow can abort it, and so can org admins and service accounts. Abort sets the run and its open file rows to STOPPED, writes the file counts, sets the pipeline's status, sends notifications and records an execution.aborted audit event.
• Abort stops a run between files; the file being processed finishes. Workers check the run's status before each file through a new internal workflow-execution/<id>/status/ endpoint, and stop only on STOPPED.
• Queued batches of a stopped run are skipped but still count down, and so does a batch that fails after the abort, so the callback runs. It keeps STOPPED, commits usage for completed files and cleans up the execution directory. On a STOPPED write the backend counts the file rows itself, whatever the worker sends. The backend refuses any other status over STOPPED on every run, including scheduled and API-triggered pipeline runs, which have no queue handle. The model does the same for backend writes, such as a staging or dispatch failure after a pending run is aborted.
• Abort reports the run itself, so a dead worker's run is still reported. A pipeline's status changes only if this is its newest run, and paused pipelines are reported too. The callback leaves both to the abort.
• A re-run ignores a stopped run's active-file markers.
• The contract is in backend/workflow_manager/execution/README.md.

Can this PR break any existing features. If yes, please list possible items. If no, please explain why. (PS: Admins do not merge the PR without this section filled)

• Runs nobody aborts behave as before, with two small changes. A failed batch now marks its run ERROR itself, with the same text and file cascade the barrier then repeats. A status write on a run without a queue handle now locks the row in update_status instead of in update_execution, still one lock per call. Without stuck, the list returns the same response (tested). Workers make one extra status call per file; if that API call fails, the file is processed as today. A run that hits ERROR mid-batch carries on as today.
• The internal execution payload gains successful_files and failed_files; workers read it leniently.
• These change, but only for STOPPED runs: a synchronous API deployment POST returns 422 instead of 200; GET applies its result filters; the callback keeps STOPPED, bills completed files, logs the saved file counts and leaves the pipeline status and notifications to the abort; a backend write keeps STOPPED and the abort's file counts; a run that finds no files leaves the pipeline status to the abort; stopped file rows show their error as the status message; the MCP getExecutionStatus tool filters a stopped run's result like GET, and the MCP instructions treat COMPLETED, ERROR and STOPPED as final.
• The public API spec now names STOPPED as a 422 outcome; only those strings changed. Regenerating the Python client and CLI is an optional follow-up.
• Known limits, in the README: a re-run right after an abort can process the file still finishing a second time. A dead worker's run gets no callback, so its execution directory and usage commit stay undone; the same goes for a run whose batch fails and then can't write its status, and for a workflow or ETL run aborted before a worker picks it up. An API caller can't get the result of the file still finishing, though it is billed.

Database Migrations

• None.

Env Config

• STUCK_EXECUTION_THRESHOLD_SECONDS (default 9000), also added to backend/sample.env.

Relevant Docs

• backend/workflow_manager/execution/README.md

Related Issues or PRs

• UN-4318, epic UN-4257
• UN-4366 is the planned consumer. The spec lists three abort outcomes; failed is a fourth.
• Replaces UN-4318 [FEAT] Show stuck executions and let owners abort running ones #2415 (closed), same branch and commits. The earlier review threads stay there.

Dependencies Versions

• None.

Notes on Testing

• Deploy the backend before the workers. A new worker talking to an old backend gets a 404 from the status check and keeps processing, as today, logging an ERROR and a WARNING per file. Expose abort to users only once the workers are updated too: an older worker's callback reports an aborted run with a completed file as completed.
• With Postgres and Redis up: backend (workflow_manager, api_v2, platform_audit, inbox, pipeline_v2, notification_v2, mcp_server) 917 passed, 26 skipped; workers 1787 passed, 3 skipped.
• Running-app test with the real backend, PG workers, Postgres, Redis and MinIO, with only the per-file tool step stubbed: API and ETL runs aborted mid-run, GET after an abort, a re-run before any worker picks up the stopped batch, an old run aborted while a newer one exists, a paused pipeline, a batch crashing after the abort, and an abort landing during the callback. The worker-killed-mid-file case was run on an earlier commit.
• Not tested live: a real tool or LLM run, usage billing (needs the cloud plugin) and actual webhook delivery (a notification row was confirmed queued).

Screenshots

Checklist

I have read and understood the Contribution Guidelines.

🤖 Generated with Claude Code

[kirtimanmishrazipstack]

Standardized review (review-deep, INITIAL)

Reviewed under unstract v2.1.80, review-deep. Mode: INITIAL — no prior review or inline comment from me on the thread. Diff 2043f0dc2..b441050d7 (HEAD exact), 35 files / 2951 lines, matches GitHub's count; base main fetched this run.

Verdict — APPROVE WITH NITS. There are no Critical or High findings. The six Mediums are real and worth fixing before you mark the PR ready.

Summary — Critical: 0 · High: 0 · Medium: 6 · Low: 6 · Lenses run: 19/19 (4 N/A, listed below)

Security deep-dive: done — 0 findings at confidence >= 8. The find step found no candidates, so no filter step was needed.
boundary-sweep: 12 sinks traced, clean

Findings are inline below.

Lens checklist

• Lens 1 — Spec & intent: Clean
• Lens 2 — Architectural fit: See findings #4
• Lens 3 — Correctness & edge cases: See findings #1, #2, #7
• Lens 4 — Security: Clean
• Lens 5 — Data integrity & migrations: See findings #1
• Lens 6 — Concurrency: See findings #2
• Lens 7 — API & contract compatibility: See findings #3, #10
• Lens 8 — Reliability & resilience: See findings #2
• Lens 9 — Performance & cost: See findings #4
• Lens 10 — Observability: See findings #7, #11
• Lens 11 — Operational safety: See findings #10
• Lens 12 — LLM/agent-specific: N/A — no model calls, prompts, tools or agent loops changed
• Lens 13 — Testing: See findings #5, #6
• Lens 14 — Dependencies & build: N/A — no manifest or lockfile changed
• Lens 15 — Code quality: See findings #12
• Lens 16 — Doc & comment accuracy: See findings #2, #8, #9
• Lens 17 — Cross-document rule consistency: See findings #3
• Lens 18 — On-prem deployment experience: N/A — no chart, on-prem values or schema, on-prem settings, build path or on-prem docs touched
• Lens 19 — Scaling & workload lifecycle: N/A — no concurrency, autoscaling, grace, broker, pool or PDB config touched

Who assessed what:

• Me: lenses 4, 5, 6, 8, 11, 12, 14, 17, 18 and 19, lens 7's consumer search, and lens 15's pinned gate.
• Lens 4: my own read, plus the security deep-dive and the boundary sweep.
• Lens 7 consumer search: the Python client and the public docs both already treat STOPPED as a final 422.
• Lens 17: the merge-result sweep ran against origin/main, which is 16 commits ahead. Its restatements of the terminal-skip and duplicate-callback rules still hold.
• Agents: code-reviewer, silent-failure-hunter, pr-test-analyzer, type-design-analyzer, comment-analyzer and the whole-diff cold read.

Open questions for the author

• On a STOPPED write, should the backend own the file counts (finding 1), or should the worker keep sending them?

Assumptions made

• Billing on a redelivered STOPPED callback is safe: the cloud commit_batch_subscription_usage claims and deletes each usage entry once. I checked this on unstract-cloud origin/main.
• All live executions use the PG queue, so queue_message_id is set once dispatched. The legacy unprotected path is the one the README already lists.
• No frontend calls the abort endpoint yet (UN-4366 is planned), which is what keeps finding 10 at Low.

cost: agents=7 wall=21:28 lenses-examined=15 of 19 files=35 lines=2951

Unanchored findings

• backend/mcp_server/tools/execution.py:378 (outside the diff hunk): 3. [Medium] [Lens 7, 17] — The MCP getExecutionStatus tool was not updated for STOPPED, which the REST endpoints now handle
• Location: backend/mcp_server/tools/execution.py:378; tool text at backend/mcp_server/registry.py:269, :717, backend/mcp_server/tools/info.py:53.
• Made stale by: backend/api_v2/api_deployment_views.py:260-263 (GET filters STOPPED results), with abort.py now producing STOPPED for API runs.
• Failure mode: the MCP tool reads the same one-shot result store as the REST GET.
  • Filters. It applies process_completed_execution only for COMPLETED, so for STOPPED it returns the raw result: highlight_data even when the org has turned it off, plus extracted text, metadata and metrics without the include flags.
  • Polling. Its instructions say to poll "until COMPLETED or ERROR". The first poll consumes the STOPPED result; every later poll returns "already acknowledged".
• Fix: use (COMPLETED, STOPPED) at execution.py:378 and say "COMPLETED, ERROR or STOPPED" in the three instruction strings.
• Confidence: High. Verification: VERIFIED by reading both read paths in code.

[kirtimanmishrazipstack]

Replies for the review items that have no thread of their own.

3. The MCP getExecutionStatus tool was not updated for STOPPED. Real, fixed in d742282. The tool now applies process_completed_execution to a STOPPED result too (mcp_server/tools/execution.py:L379), so it filters the result the same way as the REST GET. The platform variant goes through the same function. The three instruction strings now say to poll until COMPLETED, ERROR or STOPPED. test_stopped_execution_is_enriched_too pins it and fails with STOPPED removed.

Open question: should the backend own a stopped run's file counts? Yes, done in d742282. A STOPPED write now counts the file rows itself and ignores the worker's numbers (see the reply on finding 1).

Merge-result sweep against main. Main is merged in (97ab0f0) with no conflicts. Backend 915 passed and workers 1781 passed on the merged branch.

[kirtimanmishrazipstack]

Standardized review (review-deep, FOLLOWUP)

Reviewed under unstract v2.1.80, review-deep. Mode: FOLLOWUP on my review at b441050d7. Diff da80ed2b4..d742282b8 (HEAD exact): 39 files and 3225 lines, which matches GitHub's count. Base main was fetched this run; the branch is current with it.

Verdict — REQUEST CHANGES. Prior finding #2 is still open and is now High. Its fix works for runs that have a queue_message_id. Scheduled and API-triggered pipeline runs never get one, and for those runs the fix now turns an aborted run into ERROR with no callback. The details are in the #2 thread. The other 11 prior findings are resolved or waived.

Scope change — YES. #2 was fixed a different way from the one reviewed (a write instead of a read), counts moved to the backend, and four MCP files came into scope. I re-ran all 19 lenses on d742282b8.

Open after this round — Critical: 0 · High: 1 · Medium: 3 · Low: 4 (the High is prior #2; #13–#19 are new)

Security deep-dive: done — 0 findings at confidence >= 8. The find step returned no candidates.
boundary-sweep: 7 sinks traced, clean

Prior findings status

• #1 → RESOLVED, #2 → PARTIALLY RESOLVED (raised to High), #3 → RESOLVED, #4 → WAIVED, #5 to #12 → RESOLVED. Each status is in its own thread. #3 was in the last review's body and is fixed at tools/execution.py:380-383, registry.py:269-270 and :718, and info.py:53.

New findings #13–#18 are inline below.

Unanchored findings

• unstract-docs:docs/unstract_platform/mcp/mcp_server.md:129 (another repo): #19 [Low] [Lens 7]. The public MCP doc still says to poll "until the status is COMPLETED or ERROR". This PR makes STOPPED a final status of getExecutionStatus, so add it there in a docs PR.
• The PR description says "Runs nobody aborts behave as before". After d742282b8, every failed batch also makes an extra ERROR write (#13).

Lens checklist

• Lens 1 — Spec & intent: Clean
• Lens 2 — Architectural fit: See findings #13
• Lens 3 — Correctness & edge cases: See findings #2, #13, #16
• Lens 4 — Security: Clean
• Lens 5 — Data integrity & migrations: See findings #2, #13
• Lens 6 — Concurrency: See findings #2
• Lens 7 — API & contract compatibility: See findings #19
• Lens 8 — Reliability & resilience: See findings #2, #13
• Lens 9 — Performance & cost: Clean
• Lens 10 — Observability: See findings #16
• Lens 11 — Operational safety: Clean
• Lens 12 — LLM/agent-specific: See findings #14
• Lens 13 — Testing: See findings #15, #17
• Lens 14 — Dependencies & build: N/A — no manifest or lockfile changed
• Lens 15 — Code quality: Clean
• Lens 16 — Doc & comment accuracy: See findings #2, #14, #18
• Lens 17 — Cross-document rule consistency: Clean
• Lens 18 — On-prem deployment experience: N/A — no chart, on-prem values or schema, on-prem settings, build path or on-prem docs touched
• Lens 19 — Scaling & workload lifecycle: N/A — no concurrency, autoscaling, grace, broker, pool or PDB config touched

Who assessed what:

• Me:
  • lenses 5, 6, 8, 11, 14, 17, 18 and 19;
  • lens 7's consumer search (unstract-cloud, unstract-python-client and unstract-docs);
  • lens 15's pinned gate (ruff 0.3.4, check-only, clean);
  • lens 17's merge-result sweep.
• Lens 4: my read, the security deep-dive and the boundary sweep.
• Agents: code-reviewer, silent-failure-hunter, pr-test-analyzer (the test runner), type-design-analyzer, comment-analyzer and the whole-diff cold read.
• Tests: with Postgres and Redis both up, backend 167 passed and workers 64 passed.

Open questions / assumptions

• I assume per-file errors stay caught in FileProcessor.process_file, so a batch raises only on setup or API failures. That keeps #2 at High rather than Critical.
• My last review assumed every live run has a queue_message_id. That was wrong: scheduled and API-triggered pipeline runs do not.

cost: agents=7 wall=23:52 lenses-examined=16 of 19 files=39 lines=3225

[kirtimanmishrazipstack]

Replies for the follow-up review items that have no thread of their own.

19. The public MCP doc still says to poll until COMPLETED or ERROR. Real, but out of scope here: the page is in unstract-docs (docs/unstract_platform/mcp/mcp_server.md:129), a different repo. It needs STOPPED added once this ships. The server's own instructions in this PR already name all three (info.py:L50, registry.py:L717, platform.py:L181).

"Runs nobody aborts behave as before" in the description. Corrected in the description. It now says a failed batch marks its run ERROR itself with the same text and file cascade the barrier then repeats (cfeb9f4), and that a status write on a run without a queue handle now takes its row lock in update_status.

[kirtimanmishrazipstack]

pre-commit.ci run

[kirtimanmishrazipstack]

Standardized review (review-deep, FOLLOWUP)

Reviewed under unstract v2.1.80, review-deep. Mode: FOLLOWUP on my review at d742282b8. Diff da80ed2b4..cfeb9f438 (HEAD exact): 41 files and 3438 lines, which matches GitHub's count. Base main was fetched this run; the branch is current with it.

Verdict — APPROVE WITH NITS. Prior #2 (High) is resolved: every worker status write now goes through the STOPPED guard, and its test fails without the guard. All other prior findings are resolved or waived. This round adds 3 Mediums and 3 Lows; none blocks merge.

Summary — Critical: 0 · High: 0 · Medium: 3 · Low: 3 · Lenses run: 19/19 (3 N/A, listed below)

Prior context fetched — followup-fetch.sh exited 0. The "since" boundary is d742282b8; one commit since, cfeb9f438.

Scope change — NO. The commit touches only files reviewed before, follows the fix suggested for #2, adds no migration, dependency, endpoint or auth surface, and widens rather than weakens its tests.

Security deep-dive: done — 0 findings at confidence >= 8. The find step returned no candidates.
boundary-sweep: 6 sinks traced, clean

Prior findings status

• #1 [Medium] → RESOLVED (unchanged since last round).
• #2 [High] No STOPPED guard on runs without queue_message_id → RESOLVED. update_status locks every row and refuses a different status over STOPPED (internal_views.py:592-614, :888-899). All 20 worker calls of update_workflow_execution_status post there. test_later_writes_cannot_undo_the_abort fails when a row without a handle loses the protection (mutation run). Backend code that writes through the model directly is a separate gap: #20.
• #3 → RESOLVED (unchanged). #4 → WAIVED (unchanged). #5 to #12 → RESOLVED (unchanged).
• #13 [Medium] → RESOLVED. The failed batch's write now matches the barrier's text and cascade (file_processing/tasks.py:366-373, pg_barrier.py:1304, :1395). The test fails if either is reverted.
• #14 [Medium] → RESOLVED. info.py:50, registry.py:717 and platform.py:181 treat COMPLETED, ERROR and STOPPED as final, which matches execute_workflow_async.
• #15 [Medium] → RESOLVED. The test expects (3, 3, 0) and fails both when the recount is replaced by the worker's numbers and when it is skipped.
• #16 [Low] → RESOLVED. _final_log_counts (callback/tasks.py:1958) is used at :2006 and :2079, and the payload carries the counts (internal_serializers.py:99-100). Its test covers only the helper: #22.
• #17 [Low] → RESOLVED. Dropping the STOPPED check makes the completed case fail.
• #18 [Low] → RESOLVED. README :140 and :191-192.
• #19 [Low] MCP doc in unstract-docs → WAIVED. I accept the reply (#2452 (comment)): the page is in another repo and changes once this ships. It still says "until the status is COMPLETED or ERROR" (docs/unstract_platform/mcp/mcp_server.md:129 at 456369ba3), so keep it on the ticket's follow-ups.

New findings #20 to #25 are inline below.

Lens checklist

• Lens 1 — Spec & intent: Clean
• Lens 2 — Architectural fit: See findings #20
• Lens 3 — Correctness & edge cases: See findings #20, #21
• Lens 4 — Security: Clean
• Lens 5 — Data integrity & migrations: See findings #20
• Lens 6 — Concurrency: See findings #20, #21
• Lens 7 — API & contract compatibility: Clean
• Lens 8 — Reliability & resilience: See findings #23
• Lens 9 — Performance & cost: Clean
• Lens 10 — Observability: Clean
• Lens 11 — Operational safety: Clean
• Lens 12 — LLM/agent-specific: Clean
• Lens 13 — Testing: See findings #22
• Lens 14 — Dependencies & build: N/A — no manifest or lockfile changed
• Lens 15 — Code quality: See findings #25
• Lens 16 — Doc & comment accuracy: See findings #21, #23, #24
• Lens 17 — Cross-document rule consistency: Clean
• Lens 18 — On-prem deployment experience: N/A — no chart, on-prem values or schema, on-prem settings, build path or on-prem docs touched
• Lens 19 — Scaling & workload lifecycle: N/A — no concurrency, autoscaling, grace, broker, pool or PDB config touched

Who assessed what:

• Me: lenses 5, 6, 8, 11, 12, 14, 17, 18 and 19; lens 7's consumer search (unstract-cloud at 35067152d has no caller of update_status or the execution payload; unstract-docs at 456369ba3 is #19); lens 15's pinned gate (ruff 0.3.4 check and format --check, clean on all 28 Python files); lens 17's merge-result sweep (exit 1, every hit classified, none stale).
• Lens 4: my read, the security deep-dive and the boundary sweep.
• Agents: code-reviewer, silent-failure-hunter, pr-test-analyzer (the test runner), type-design-analyzer, comment-analyzer and the whole-diff cold read.
• Tests: with Postgres and Redis confirmed up: backend 478 passed (plus 35 spec tests), workers 1784 passed, 3 skipped (none in this diff's scope). Six mutations were run on copies; the two that survived are #22.

Open questions / assumptions

• Not raised again: when a stopped run's batch fails and its own ERROR write fails too, the barrier's identical write is refused, but the barrier ignores the refusal and tears down, so the callback never runs (reproduced with mocks). README :140 and :191-192 document this, and I accepted that last round. Having the barrier read the refusal itself would close it and remove the second ERROR write every failed batch now makes.
• I assume #20's window is short, because staging and dispatch usually take seconds. Slow staging of large uploads would make it more likely.

cost: agents=7 wall=33:47 lenses-examined=16 of 19 files=41 lines=3438

[sonarqubecloud]

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[kirtimanmishrazipstack]

Replies for the follow-up review's items that have no thread.

#19, the MCP doc in unstract-docs. It goes on the ticket's follow-ups with the final Jira update: once this ships, docs/unstract_platform/mcp/mcp_server.md:129 should add STOPPED to the statuses that end polling.

Open question: the barrier reading the refusal itself. Unchanged in e0006b5. A run whose failed batch can't write its status stays a documented limit (README L191-197). The second ERROR write per failed batch is the same write repeated, so it changes nothing.

The PR description now covers e0006b5: backend writes keep STOPPED, a run with no files leaves the pipeline status to the abort, the new limit, and backend 917 / workers 1787 passed.