Skip to content

fix(execution): complete async and resume jobs on marked workflow user failures - #8380

Closed
waleedlatif1 wants to merge 1 commit into
stagingfrom
fix/async-jobs-complete-on-user-failures
Closed

waleedlatif1 wants to merge 1 commit into
stagingfrom
fix/async-jobs-complete-on-user-failures

Conversation

@waleedlatif1

Copy link
Copy Markdown
Collaborator

Summary

  • The workflow-execution and resume-execution Trigger.dev tasks re-threw every execution error, so a user's own workflow failure (their function code raising, a condition expression that doesn't parse) failed the run and paged #eng-errors. The existing finalized-by-core guard in workflow-execution also ran before core had recorded the failure, so it never applied
  • Failures stay faults by default. A job completes with success: false only when core recorded the failure and it was positively marked as the workflow's own at its source (markWorkflowUserFailure in executor/utils/errors.ts):
    • the Function route's 422 for a runtime/compile error in user or custom-tool code (the route keeps 5xx for its own faults), carried across executeTool's throw→result flattening as ToolResponse.workflowUserFailure
    • a condition expression that threw, or that the sandbox couldn't parse (per-branch fallback)
    • the HTTP block receiving a 4xx from the URL the workflow called
    • a missing required field, now a WorkflowValidationError attributed to its block
  • Child workflows carry the mark through their cause chain; custom blocks deliberately cut the chain at their trust boundary, so those still fault
  • Anything unmarked still faults and pages: database and other platform errors inside blocks, handler invariants, timeouts, preprocessing rejections, resume admission errors
  • /api/jobs/{id} and the queue-job status fallback project the completed failure result back to failed, so pollers still see a failed run
  • Supersedes fix(execution): complete background jobs on workflow failures, fault only on platform errors #8368, whose block-attribution rule could silence platform faults inside blocks. Webhook and schedule execution are intentionally unchanged: webhook runs inside idempotency with retryFailures: true (re-throwing would let a redelivery re-run a partly executed workflow) and schedule pages on nothing today, so paging by default there would add noise

Type of Change

  • Bug fix

Testing

  • Each marking site has a test, and each was confirmed to go red when its mark is removed. The two task-level tests fail on the pre-fix task code
  • Classifier covers late finalization, child-workflow wrapping, a marked failure core didn't record, and an unmarked recorded block failure (still faults)
  • Type-check, lint, all 51 audits and the full apps/sim suite pass, except 3 tests in lib/sim-search/live/application.test.ts that fail identically on current staging without this change

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

…r failures

The workflow-execution and resume-execution Trigger.dev tasks re-threw every
execution error, so a user's own workflow failure (their function code raising,
a condition expression that does not parse) failed the run and paged
#eng-errors. The existing finalized-by-core guard in workflow-execution also
ran before core had recorded the failure, so it never applied.

Failures stay faults by default. A job completes with success: false only when
core recorded the failure and it was positively marked as the workflow's own at
its source (markWorkflowUserFailure):
- the Function route's 422 for a runtime or compile error in user/custom-tool
  code, carried across executeTool's flattening as ToolResponse.workflowUserFailure
- a condition expression that threw or that the sandbox could not parse
- the HTTP block receiving a 4xx from the URL the workflow called
- a missing required field, now a WorkflowValidationError attributed to its block

Child workflows carry the mark through their cause chain. Anything unmarked,
including database and other platform errors inside blocks, still faults.
/api/jobs and the queue-job status fallback project the completed failure
result back to failed. Webhook and schedule execution are unchanged.
@vercel

vercel Bot commented Sep 28, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated
docs Skipped Skipped Sep 28, 2026 5:49pm UTC

Request Review

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

3 issues found across 24 files

Confidence score: 3/5

  • apps/sim/tools/index.ts classifies remote-sandbox provider_limit infrastructure failures as workflow-user failures, which can report infrastructure failures under the wrong category — preserve the provider-limit infrastructure classification.
  • apps/sim/background/resume-execution.ts returns before writing the terminal cell state on handled workflow failures, leaving table cells and groups stuck in their pre-resume state — write the cell terminal error state before returning.
  • apps/sim/executor/utils/errors.ts stops checking causes after ten links, so deeply nested child-workflow failures can be misclassified as job_fault and trigger paging instead of completing as workflow failures — ensure the marker is found through deeper cause chains.
Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="apps/sim/tools/index.ts">

<violation number="1" location="apps/sim/tools/index.ts:2805">
P1: This status check also marks remote-sandbox provider-limit failures as workflow-user failures. The sandbox contract identifies `providerFailure: 'provider_limit'` as an infrastructure outcome, but the Function route returns those errors as 422, causing this code to suppress paging for a platform fault. Preserve the provider-failure signal through the route and exclude it from user-fault marking.</violation>
</file>

<file name="apps/sim/background/resume-execution.ts">

<violation number="1" location="apps/sim/background/resume-execution.ts:239">
P1: This handled failure returns before the cell-context terminal writer runs. Marked workflow failures in table cells will therefore leave the cell/group stuck in its pre-resume state; write the cell terminal error state before returning this workflow-failure result.</violation>
</file>

<file name="apps/sim/executor/utils/errors.ts">

<violation number="1" location="apps/sim/executor/utils/errors.ts:161">
P2: `isWorkflowUserFailure` stops after `findCause` examines ten error links. A deeply nested child-workflow failure can therefore lose its marker and be classified as `job_fault`, paging instead of completing as a workflow failure; use a cycle-safe walk with a depth appropriate for the supported nesting limit.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Fix all with cubic | Re-trigger cubic

Comment thread apps/sim/tools/index.ts
{ status: response.status, statusText: response.statusText, data: errorData },
tool.errorExtractor
)
throw isFunctionOperation && response.status === FUNCTION_USER_CODE_FAILURE_STATUS

@cubic-dev-ai cubic-dev-ai Bot Sep 28, 2026 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1: This status check also marks remote-sandbox provider-limit failures as workflow-user failures. The sandbox contract identifies providerFailure: 'provider_limit' as an infrastructure outcome, but the Function route returns those errors as 422, causing this code to suppress paging for a platform fault. Preserve the provider-failure signal through the route and exclude it from user-fault marking.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/sim/tools/index.ts, line 2805:

<comment>This status check also marks remote-sandbox provider-limit failures as workflow-user failures. The sandbox contract identifies `providerFailure: 'provider_limit'` as an infrastructure outcome, but the Function route returns those errors as 422, causing this code to suppress paging for a platform fault. Preserve the provider-failure signal through the route and exclude it from user-fault marking.</comment>

<file context>
@@ -2790,10 +2798,13 @@ async function executeDeclaredInternalOperation({
       { status: response.status, statusText: response.statusText, data: errorData },
       tool.errorExtractor
     )
+    throw isFunctionOperation && response.status === FUNCTION_USER_CODE_FAILURE_STATUS
+      ? markWorkflowUserFailure(error)
+      : error
</file context>
Fix with cubic

)
// The resumed run executes under its parent's id, and the manager settles
// its post-execution work before re-throwing.
if (classifySettledWorkflowJobFailure(error, parentExecutionId) === 'workflow_failure') {

@cubic-dev-ai cubic-dev-ai Bot Sep 28, 2026 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1: This handled failure returns before the cell-context terminal writer runs. Marked workflow failures in table cells will therefore leave the cell/group stuck in its pre-resume state; write the cell terminal error state before returning this workflow-failure result.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/sim/background/resume-execution.ts, line 239:

<comment>This handled failure returns before the cell-context terminal writer runs. Marked workflow failures in table cells will therefore leave the cell/group stuck in its pre-resume state; write the cell terminal error state before returning this workflow-failure result.</comment>

<file context>
@@ -230,6 +234,15 @@ export async function executeResumeJob(payload: ResumeExecutionPayload, signal?:
     )
+    // The resumed run executes under its parent's id, and the manager settles
+    // its post-execution work before re-throwing.
+    if (classifySettledWorkflowJobFailure(error, parentExecutionId) === 'workflow_failure') {
+      return {
+        ...buildWorkflowJobFailureResult({ error, workflowId, executionId: resumeExecutionId }),
</file context>
Fix with cubic

*/
export function isWorkflowUserFailure(error: unknown): boolean {
return (
findCause(

@cubic-dev-ai cubic-dev-ai Bot Sep 28, 2026 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2: isWorkflowUserFailure stops after findCause examines ten error links. A deeply nested child-workflow failure can therefore lose its marker and be classified as job_fault, paging instead of completing as a workflow failure; use a cycle-safe walk with a depth appropriate for the supported nesting limit.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/sim/executor/utils/errors.ts, line 161:

<comment>`isWorkflowUserFailure` stops after `findCause` examines ten error links. A deeply nested child-workflow failure can therefore lose its marker and be classified as `job_fault`, paging instead of completing as a workflow failure; use a cycle-safe walk with a depth appropriate for the supported nesting limit.</comment>

<file context>
@@ -141,6 +141,33 @@ function isRecordedThrown(value: unknown): value is object {
+ */
+export function isWorkflowUserFailure(error: unknown): boolean {
+  return (
+    findCause(
+      error,
+      (value): value is Error =>
</file context>
Fix with cubic

@greptile-apps

greptile-apps Bot commented Sep 28, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 4/5

[High risk] Changes how workflow execution failures are classified and reported.

The PR is not ready to merge until unavailable-sandbox failures remain job faults and the changed tests satisfy the repository’s testing requirement.

Findings

  1. P1 Sandbox failures marked as user errors ▶
  2. P2 Tests assert mock calls ▶

Summary

The PR marks selected workflow-origin errors, completes async and resume jobs when core records those failures, and projects their queue results as failed runs.

  • It also changes required-field validation to carry block attribution.
  • The Function 422 classification includes an unavailable-sandbox response, and changed tests contain prohibited mock-call assertions.
Diagram
%%{init: {'theme': 'neutral'}}%%
flowchart LR
  A[Function or other workflow block] --> B[Execution core]
  B --> C{Error marked and recorded?}
  C -->|Yes| D[Job completes with success false]
  C -->|No| E[Job faults]
  D --> F[Status readers project failed run]
Loading

Reviews (1) · Last reviewed commit: "fix(execution): complete async and resum..."

Comment thread apps/sim/tools/index.ts
Comment on lines +2805 to +2807
throw isFunctionOperation && response.status === FUNCTION_USER_CODE_FAILURE_STATUS
? markWorkflowUserFailure(error)
: error

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 Sandbox failures marked as user errors

When a Function block requests sandbox files on a deployment without a remote sandbox, the Function route returns 422 because the filesystem is unavailable. This check marks that response as a user-code failure. Once core records it, the queue job completes instead of faulting and alerting engineering. A 422 needs to be identified as a user runtime or compile error before it receives this mark.

Knowledge Base Used: Agent execution and sandbox tasks

Comment on lines +391 to 393
expect(loggingSessionMockFns.mockWaitForPostExecution).toHaveBeenCalled()
expect(mockWasExecutionFinalizedByCore).toHaveBeenCalledWith(rawError, 'execution-finalized')
expect(loggingSessionMockFns.mockSafeCompleteWithError).not.toHaveBeenCalled()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 Tests assert mock calls

These assertions check whether mocked functions were called. The repository’s testing directive prohibits mock-call assertions; the new workflow-execution test also uses them. Please verify the resulting behavior instead. This requirement must be satisfied before merging.

Context Used: CLAUDE.md (source)

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

@waleedlatif1
waleedlatif1 marked this pull request as draft September 28, 2026 18:02
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

Converting to draft: review found that the Function route's 422 is not a trustworthy user-code signal (isolated-vm bootstrap/runtime-binding failures, sim.values.read helper failures, E2B transport corruption and provider limits, and code-placeholder compiler bugs all surface as 422), and missing-required-field validation can fire because of a Sim-side registry change. Marking these as workflow user failures could silence platform faults. Not safe to merge as-is.

@waleedlatif1
waleedlatif1 deleted the fix/async-jobs-complete-on-user-failures branch September 29, 2026 04:46

This branch was previously deployed

1 inactive deployment
Preview — 11cd349a Deployed Sep 28, 2026 by vercel[bot]
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.

1 participant