Skip to content

fix(polling-tools): pass plan execution timeout to internal polling tool routes#4884

Merged
icecrasher321 merged 2 commits into
stagingfrom
fix/pass-plan-timeout-media
Jun 4, 2026
Merged

fix(polling-tools): pass plan execution timeout to internal polling tool routes#4884
icecrasher321 merged 2 commits into
stagingfrom
fix/pass-plan-timeout-media

Conversation

@icecrasher321
Copy link
Copy Markdown
Collaborator

Summary

Pass plan execution timeouts to internal tool routes with polling

Type of Change

  • Bug fix

Testing

Tested manually

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)

@vercel
Copy link
Copy Markdown

vercel Bot commented Jun 4, 2026

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

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped Jun 4, 2026 9:26pm

Request Review

@cursor
Copy link
Copy Markdown

cursor Bot commented Jun 4, 2026

PR Summary

Medium Risk
Increases serverless route duration and changes timeout/abort behavior for long STT, Textract, image, and video jobs; mis-sync between static maxDuration and getMaxExecutionTimeout() could cause premature cuts or over-long runs.

Overview
Aligns long-running media tool routes (image, STT, Textract, video) with the platform’s maximum plan execution window by raising Next.js maxDuration to 5400s (90 minutes, documented as mirroring getMaxExecutionTimeout()).

Polling and client timeouts no longer use the default free-tier sync limit: AssemblyAI STT and Textract async job polling, plus internal tool HTTP fetches in executeTool, now derive limits from getMaxExecutionTimeout() instead of DEFAULT_EXECUTION_TIMEOUT_MS.

Workflow cancellation is wired through tool execution: when no explicit signal is passed, executeTool uses executionContext.abortSignal, so plan timeouts and user cancellation can abort internal tool requests (including MCP and hosted-key retries).

Reviewed by Cursor Bugbot for commit d5ea6ab. Configure here.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Jun 4, 2026

Greptile Summary

This PR fixes polling-tool routes (image, video, STT, Textract) that could time out prematurely when running under a long-running plan execution. Two distinct fixes are bundled: the route maxDuration exports and polling maxAttempts bounds are raised to match the enterprise-async ceiling (getMaxExecutionTimeout() / 5400 s), and executeTool is updated to derive an effectiveSignal that falls back to executionContext.abortSignal so plan-level cancellation propagates to every internal-fetch call site.

  • Route duration alignment: image and video already polled up to 90 min but were capped at 10-min maxDuration; stt and textract polled only 5 min and are now raised in both maxDuration and maxAttempts.
  • Signal propagation fix: effectiveSignal is threaded through the direct call, the executeWithRetry factory, and the reacquireAfterRetriesExhausted factory, closing a gap where the execution-context abort signal was never forwarded to internal fetches.

Confidence Score: 5/5

Safe to merge — the changes close a real gap where plan-level cancellation was never forwarded to internal polling fetches, and align route maxDuration exports with already-existing polling ceilings.

The effectiveSignal plumbing is consistently applied at every call-site (direct, retry, reacquire), and the maxDuration / maxAttempts values are now in sync across all four routes. No logic regressions are introduced; the trade-offs flagged in previous threads have been accepted by the team.

No files require special attention.

Important Files Changed

Filename Overview
apps/sim/tools/index.ts Introduces effectiveSignal = signal ?? executionContext?.abortSignal and threads it through all executeToolRequest call-sites (direct, retry, reacquire); also raises the internal-fetch fallback timeout to getMaxExecutionTimeout() when any signal is present.
apps/sim/app/api/tools/image/route.ts Raises maxDuration from 600 s to 5400 s to match the pre-existing getMaxExecutionTimeout()-based polling loop; adds explanatory comment.
apps/sim/app/api/tools/video/route.ts Raises maxDuration from 600 s to 5400 s to align with the pre-existing getMaxExecutionTimeout()-based polling loops (5 providers); adds explanatory comment.
apps/sim/app/api/tools/stt/route.ts Replaces DEFAULT_EXECUTION_TIMEOUT_MS with getMaxExecutionTimeout() for AssemblyAI polling maxAttempts and raises maxDuration from 300 s to 5400 s.
apps/sim/app/api/tools/textract/parse/route.ts Replaces DEFAULT_EXECUTION_TIMEOUT_MS with getMaxExecutionTimeout() for Textract job polling and raises maxDuration from 300 s to 5400 s.

Sequence Diagram

sequenceDiagram
    participant Executor as Plan Executor
    participant ET as executeTool()
    participant ETR as executeToolRequest()
    participant Route as Polling Route(image/video/stt/textract)
    participant Provider as External Provider API

    Executor->>ET: "executeTool(toolId, params, {executionContext})"
    Note over ET: effectiveSignal = signal ?? executionContext.abortSignal
    ET->>ETR: executeToolRequest(..., effectiveSignal)
    Note over ETR: timeout = requestParams.timeout || (signal ? getMaxExecutionTimeout() : DEFAULT)
    ETR->>Route: "fetch(internalUrl, {signal: controller.signal})"
    Note over Route: maxDuration = 5400 s, maxAttempts = ceil(getMaxExecutionTimeout() / pollInterval)
    loop Poll until complete or maxAttempts
        Route->>Provider: status check
        Provider-->>Route: IN_PROGRESS / COMPLETED
    end
    Route-->>ETR: result
    ETR-->>ET: ToolResponse
    ET-->>Executor: ToolResponse

    alt Plan cancelled / timed out
        Executor->>ET: executionContext.abortSignal fires
        Note over ET: effectiveSignal aborts
        ETR-->>ET: AbortError (caller_aborted)
    end
Loading

Reviews (2): Last reviewed commit: "address comments" | Re-trigger Greptile

Comment thread apps/sim/tools/index.ts Outdated
Comment thread apps/sim/app/api/tools/stt/route.ts
@icecrasher321
Copy link
Copy Markdown
Collaborator Author

@greptile

@icecrasher321 icecrasher321 merged commit 71f6936 into staging Jun 4, 2026
13 checks passed
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