Skip to content

fix(db-part-3): bound cross-request shared promises against pool wedge#5021

Merged
icecrasher321 merged 2 commits into
stagingfrom
improvement/db-pattern-3
Jun 13, 2026
Merged

fix(db-part-3): bound cross-request shared promises against pool wedge#5021
icecrasher321 merged 2 commits into
stagingfrom
improvement/db-pattern-3

Conversation

@icecrasher321

Copy link
Copy Markdown
Collaborator

Summary

  • coalesceLocally: 30s settle deadline with evict-before-reject and
    identity-checked eviction; defer fn() so a sync throw can't hit the TDZ
  • oauth refresh: 15s fetch timeout + null-on-timeout, preserving the
    null-on-failure contract across all ~70 call sites
  • mcp oauth refresh: bound the in-process queue wait (90s) so a stalled link
    can't pile up callers; never aborts a running fn (would orphan the client)
  • function execute: evict the cached typescript import on rejection (was
    poisoned forever); vanta: 15s token-exchange timeout
  • webhooks: document the shouldDeleteWebhook pre-transaction pool contract

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

vercel Bot commented Jun 13, 2026

Copy link
Copy Markdown

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

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped Jun 13, 2026 6:38am

Request Review

@cursor

cursor Bot commented Jun 13, 2026

Copy link
Copy Markdown

PR Summary

Medium Risk
Touches OAuth refresh, MCP credential serialization, and in-process coalescing—important auth paths—but changes are defensive timeouts with tests and preserved null-on-failure OAuth behavior.

Overview
Adds timeouts and eviction so hung async work cannot block every future caller on the same key (a DB pool wedge risk).

coalesceLocally now races the producer against a 30s settle deadline, evicts the in-flight entry before rejecting with CoalesceSettleTimeoutError, and defers fn() to a microtask so synchronous throws do not hit a TDZ. OAuth coalesced refresh catches those failures and still returns null, keeping the existing failure contract for callers.

OAuth token refresh and Vanta token exchange use AbortSignal.timeout(15s) on the token fetch. MCP OAuth refresh lock bounds only the in-process queue wait (90s)—queued callers reject without running fn; a running refresh is not aborted. Function execute clears the cached typescript dynamic import promise on rejection so a failed load is not cached forever.

Webhook undeploy paths gain docs that shouldDeleteWebhook must run outside an open transaction (pool nesting); behavior is unchanged.

Reviewed by Cursor Bugbot for commit 3331547. Configure here.

@greptile-apps

greptile-apps Bot commented Jun 13, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR adds bounded settle deadlines to all cross-request shared promises to prevent hung producers from wedging callers indefinitely until process restart.

  • coalesceLocally gains a 30s settle deadline with CoalesceSettleTimeoutError, identity-checked eviction (so a late-resolving fn() cannot evict a successor), and a microtask-deferred fn() call to avoid a TDZ ReferenceError when fn throws synchronously.
  • withMcpOauthRefreshLock bounds the in-process queue wait to 90s via Promise.race([prevSettled, queueDeadline]); the running caller's own fn() is intentionally unbounded to avoid orphaning a connected McpClient.
  • refreshOAuthToken and exchangeVantaToken each add AbortSignal.timeout (15s) on their fetch calls; timeout errors propagate through existing catch paths that already return null.
  • loadTypeScriptModule nulls out the cached promise on rejection so subsequent calls retry the import instead of permanently re-joining a rejected promise.

Confidence Score: 4/5

Safe to merge once the hung-chain accumulation in withMcpOauthRefreshLock is understood and accepted as a known trade-off.

All five fixes are correctly implemented and the new test coverage is comprehensive. The one gap worth attention is in withMcpOauthRefreshLock: if prev (the original hung link) never settles, the chain of unresolved promise links accumulates in inflightChains for the row until the process restarts. In normal operation the inner SDK/HTTP/Redis timeouts prevent this, but it is an implicit invariant with no explicit guard.

apps/sim/lib/mcp/oauth/storage.ts — specifically the inflightChains growth path when prev is permanently hung.

Important Files Changed

Filename Overview
apps/sim/lib/concurrency/singleflight.ts Adds a 30s settle deadline with CoalesceSettleTimeoutError, identity-checked eviction, and a microtask-deferred fn() call to avoid TDZ. Logic is correct: evict-before-reject prevents new joiners from latching on; clearTimeout in the IIFE finally properly cleans up when fn() wins the race.
apps/sim/lib/mcp/oauth/storage.ts Bounds the in-process queue wait to 90s via a queueTimedOut flag and Promise.race. Logic is correct for the single-threaded event loop. Gap: if prev never settles, next never settles, and inflightChains accumulates one entry per subsequent caller for the lifetime of the hung link.
apps/sim/app/api/auth/oauth/utils.ts Wraps coalesceLocally in a try/catch to absorb CoalesceSettleTimeoutError and return null, preserving the null-on-failure contract across ~70 call sites.
apps/sim/app/api/function/execute/route.ts Evicts the poisoned typescriptModulePromise on rejection so subsequent calls retry the import instead of re-joining a permanently-rejected promise. Correct fix.
apps/sim/tools/vanta/utils.ts Adds AbortSignal.timeout(15s) to the Vanta token-exchange fetch. On timeout, the finally block removes the in-flight entry from vantaTokenExchanges so subsequent callers create a fresh exchange.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[coalesceLocally called] --> B{Entry in inflight map?}
    B -->|yes| C[Return existing promise - join producer]
    B -->|no| D[Create async IIFE + settle timer]
    D --> E[Promise.race: IIFE vs timer]
    E --> F[inflight.set key to guarded]
    F --> G[Return guarded to all callers]
    G --> H{Who settles first?}
    H -->|fn resolves| I[finally: clearTimeout + evict identity-check]
    I --> J[guarded resolves - all joiners get value]
    H -->|30s timer fires| K[evict before reject - clears inflight entry]
    K --> L[guarded rejects with CoalesceSettleTimeoutError]
    L --> M[All joiners get error - next caller mints fresh entry]
    G2[withMcpOauthRefreshLock caller] --> Q[Chain next off prevSettled]
    Q --> R[inflightChains.set key to next]
    R --> S[await Promise.race prevSettled vs queueDeadline]
    S -->|prevSettled wins| T[clearTimeout - return next - fn runs]
    S -->|90s queueDeadline fires| U[queueTimedOut=true - throw error]
    U --> V[next rejects with abandoned message when prev settles]
Loading

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

Comment thread apps/sim/app/api/auth/oauth/utils.ts
Comment thread apps/sim/lib/mcp/oauth/storage.ts Outdated
@icecrasher321

Copy link
Copy Markdown
Collaborator Author

@greptile

@icecrasher321 icecrasher321 merged commit 9a4c9d2 into staging Jun 13, 2026
14 checks passed
@waleedlatif1 waleedlatif1 deleted the improvement/db-pattern-3 branch June 13, 2026 16:14
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