fix(redis): size cold-connection waits to survive a dead handshake by waleedlatif1 · Pull Request #7764 · simstudioai/sim

waleedlatif1 · 2026-09-11T06:02:17Z

Summary

Every readiness wait for a cold Redis connection was sized to a single deadline, so it expired while the first attempt was still being diagnosed — a configured retry could never be what rescued it
Adds coldConnectionBudgetMs beside the pinned client (lib/core/config/redis-budget.ts): a dead attempt is charged at the larger of what governs it — connectTimeout for a connect that never completes (destroyed outright), or the handshake's command deadlines plus disconnectTimeout, because after the ready check times out ioredis half-closes the socket and a wedged peer never answers with a FIN. Then the client's first reconnect delay, then a healthy attempt. Authenticated URLs cost one command deadline (a timed-out AUTH is fatal), unauthenticated two (CLIENT SETNAME/SETINFO settle before INFO starts); the budget takes the larger so it holds for either without parsing the URL
Execution-signal subscriber: readiness budget was the same constant as its commandTimeout (5s/5s). The command deadline stays at 5s — it also bounds the live SUBSCRIBE, and an initial subscribe that rejects fails the run — and the budget is derived in the constructor from the exact options the client is built with: 13.5s
Each waiter gets its own deadline. Waiters race the shared readiness signal against a per-waiter timer; the last to leave detaches the listeners, and a signal that settles clears itself. Before this branch the memoized promise shared one timer, so a late joiner inherited only the remainder
Shared client warm-up: budget was 10s against a 5s deadline — exactly the moment ioredis first tears a stalled socket down. Now 14.3s, derived from the client's own connect and disconnect deadlines and a pure sharedReconnectDelayMs that retryStrategy uses too
The signal subscriber is connected on intent. Constructing the hub is what connects, so executeWorkflowCore — the one path every execution shares — constructs it as its first statement, synchronously and never throwing, and the handshake overlaps the reads and preprocessing ahead of the subscribe. Only tasks that execute a workflow ever subscribe, and they are a minority of runs (over a week, 71% never do; one document-processing task is 61% alone), so neither the Trigger.dev init hook nor the Next server opens it eagerly

Verified against the installed ioredis

With a server that stalls only the first handshake and never sends a FIN, exact production options: unauthenticated, the socket closes at 12.0s (10s of command deadlines + 2s half-close) and a healthy connection #2 is ready at 12.5s; authenticated, at 7.0s / 7.5s. Today's subscriber budget (5000) fails at 5.0s on connection #1, matching the production failure; the derived 13.5s budget recovers both shapes.

What this is and is not

Not a strict improvement, stated precisely. In the failure shape observed in production — shared client healthy, subscriber handshake dead — a run now fails at ~13.5s instead of ~5s, and the same wait applies when Redis is simply unreachable. During a total outage the awaited warm-up costs 14.3s before the run starts and then either the shared client's 5s admission deadline or the 13.5s subscribe fails it: roughly 19–28s of worker hold versus ~15s before this PR. That is the whole regression surface, in exchange for recovering from every transient stall. The guarantee is one dead attempt from a fresh or previously-ready client; a wait that begins while the client is already deep in a reconnect loop faces larger delays this does not model. A widened wait also widens the window in which a short execution timeout pre-empts it and reports itself instead of the Redis failure.

On the healthy path nothing waits longer: the subscriber's ~40ms handshake now overlaps preprocessing instead of running inside run() before the engine starts, and the 71% of runs that never subscribe never open that connection.

Pub/Sub is at-most-once, so the run-fails-if-it-cannot-subscribe stance is kept deliberately — this change makes that subscription recoverable, it does not make it optional.

Type of Change

Bug fix

Testing

New tests: the budget helper (command deadlines plus half-close, the connect deadline when larger, the healthy-attempt allowance); the shared client's pure reconnect delay; the warm-up outlasts one dead attempt and still gives up past its budget; the subscriber waits exactly the budget derived from its own options; a subscribe joining another in-flight wait keeps its own full budget; waiter accounting stays exact when one waiter times out before the signal settles; connecting ahead of a subscription never throws on a misconfigured URL; executeWorkflowCore connects synchronously before its first await. Each verified to fail under a targeted sabotage on its intended assertion (dropping the half-close term, a shared timer, never detaching, the connect moved after the first await).

Shared test infra changed (redis-config.mock.ts), so the full suite was run on the rebased tree: 50,151 tests pass in apps/sim, 381 in apps/realtime, 222 in packages/utils, 50 in packages/testing. bun run type-check clean in apps/sim, packages/utils, and packages/testing. bun run lint, docs-manifest:check, block-registry check, and all 46 audits in check:audits pass.

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 · 2026-09-11T06:02:21Z

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

1 Skipped Deployment

Project	Deployment	Actions	Updated
docs	Skipped		Sep 11, 2026 6:19pm UTC

greptile-apps · 2026-09-11T06:06:15Z

Confidence Score: 5/5

The PR appears safe to merge; no outstanding correctness, security, or repository-rule finding remains.

Summary

Derives readiness budgets from connect, command, disconnect, and reconnect timing.
Gives concurrent readiness waiters independent deadlines with shared, safely cleaned-up listeners.
Starts the execution-signal connection synchronously at workflow-execution intent.
Updates Redis test mocks and adds focused timing, reconnect, and waiter-accounting coverage.

Diagram

sequenceDiagram
    participant E as Workflow execution
    participant H as Execution signal hub
    participant R1 as Redis attempt 1
    participant R2 as Redis attempt 2
    E->>H: connectExecutionSignalHub()
    H->>R1: Begin cold connection
    R1--xH: Handshake stalls
    Note over H,R1: Command deadlines + disconnect timeout
    H->>R2: Retry after first reconnect delay
    R2-->>H: ready
    E->>H: Subscribe for execution signals
    H-->>E: Subscription acknowledged

_{Reviews (11) · Last reviewed commit: "fix(redis): connect the signal subscribe..."}

waleedlatif1 · 2026-09-11T06:09:06Z

@greptile

waleedlatif1 · 2026-09-11T06:09:07Z

@cubic-dev-ai review this PR

cubic-dev-ai · 2026-09-11T06:09:13Z

@waleedlatif1 I have started the AI code review. It will take a few minutes to complete.

waleedlatif1 · 2026-09-11T06:13:53Z

@greptile

waleedlatif1 · 2026-09-11T06:13:54Z

@cubic-dev-ai review this PR

cubic-dev-ai · 2026-09-11T06:13:59Z

@waleedlatif1 I have started the AI code review. It will take a few minutes to complete.

waleedlatif1 · 2026-09-11T06:53:36Z

@greptile

waleedlatif1 · 2026-09-11T06:53:38Z

@cubic-dev-ai review this PR

cubic-dev-ai · 2026-09-11T06:53:44Z

@waleedlatif1 I have started the AI code review. It will take a few minutes to complete.

waleedlatif1 · 2026-09-11T06:58:47Z

@greptile

waleedlatif1 · 2026-09-11T06:58:48Z

@cubic-dev-ai review this PR

cubic-dev-ai · 2026-09-11T06:58:53Z

@waleedlatif1 I have started the AI code review. It will take a few minutes to complete.

waleedlatif1 · 2026-09-11T07:13:46Z

@greptile

waleedlatif1 · 2026-09-11T07:13:48Z

@cubic-dev-ai review this PR

cubic-dev-ai · 2026-09-11T07:13:53Z

@waleedlatif1 I have started the AI code review. It will take a few minutes to complete.

waleedlatif1 · 2026-09-11T07:18:54Z

@greptile

waleedlatif1 · 2026-09-11T07:18:56Z

@cubic-dev-ai review this PR

cubic-dev-ai · 2026-09-11T07:19:02Z

@waleedlatif1 I have started the AI code review. It will take a few minutes to complete.

waleedlatif1 · 2026-09-11T07:21:07Z

@cubic-dev-ai review this PR

cubic-dev-ai · 2026-09-11T07:21:13Z

@waleedlatif1 I have started the AI code review. It will take a few minutes to complete.

waleedlatif1 · 2026-09-11T07:23:34Z

@greptile

waleedlatif1 · 2026-09-11T07:23:35Z

@cubic-dev-ai review this PR

cubic-dev-ai · 2026-09-11T07:23:41Z

@waleedlatif1 I have started the AI code review. It will take a few minutes to complete.

waleedlatif1 · 2026-09-11T07:28:46Z

@greptile

waleedlatif1 · 2026-09-11T07:28:48Z

@cubic-dev-ai review this PR

cubic-dev-ai · 2026-09-11T07:28:55Z

@waleedlatif1 I have started the AI code review. It will take a few minutes to complete.

A dead Redis handshake costs two command deadlines, not one. ioredis sends CLIENT SETNAME/SETINFO on connect and dispatches the INFO ready check only once those settle — on a socket that never answers, they settle by timing out. Only then does the ready check start its own deadline, fail, and tear the socket down for retryStrategy to reconnect. Every readiness wait in the codebase was sized to a single deadline, so it expired while the first attempt was still being diagnosed and a configured retry could never be the thing that rescued it. Introduces `coldConnectionBudgetMs`, which derives a wait from the command deadline and the reconnect delays a caller's own retryStrategy would return, and states the 2x in one place. Both waits now derive from it: - The execution-signal subscriber's readiness budget was the same constant as its commandTimeout (5s and 5s), so ioredis's reconnect was decorative on the cold path that failed in production. It now has a 2s command deadline — this client only issues SUBSCRIBE/UNSUBSCRIBE, always after ready — and a budget with room for two dead attempts and a healthy one. - The shared client's warm-up budget was 10s against a 5s deadline, exactly the moment ioredis would first tear a stalled socket down, so on the case warming exists for it gave up just before recovery could land. Warms the execution-signal subscriber alongside the shared client at process start, in parallel, never throwing: it is the second connection a run opens and was paying its handshake inside its own readiness budget. The shared redis-config mock mirrors the pure budget helper, since consumers now evaluate it at module load and would otherwise fail to import under the global mock.

ioredis has one `commandTimeout` for the handshake commands and every live command on the socket, so tightening it to diagnose dead handshakes faster also cut the initial SUBSCRIBE from 5s to 2s — on the one path where a rejection fails the whole run. A ready-but-slow server delays SUBSCRIBE too. Restore the 5s tolerance that subscribe has always had and derive the readiness budget from it: one dead handshake recovers in ~10.5s, so an 11.5s budget still lets ioredis's reconnect rescue the case that failed. The local test mock now delegates to the shared, drift-guarded mirror of the budget arithmetic so the derived number under test is the one production derives.

…er start Only the tasks that execute a workflow ever subscribe to cancellation signals, and they are a minority of runs: over a week, 71% of Trigger.dev runs never subscribe, and a single document-processing task is 61% on its own. Warming the subscriber in the global init hook opened a TLS connection on every one of those runs that nothing would use. Warm it instead at `executeWorkflowCore` — the one path every execution shares — fire-and-forget as the first statement, so the handshake overlaps the custom-block read and preprocessing ahead of the cancellation subscribe rather than being paid inside that subscribe's readiness budget. No task-id list to keep in sync: coverage follows from the funnel. The subscribe and the warm-up share the hub's memoized readiness wait, so their budgets never compound within a run. The global init keeps warming only the shared client, which nearly every task uses; the Next server keeps warming both at boot, one connection per long-lived process.

The dynamic imports in the Redis warm-up block used relative paths; the file already resolves `@/` and the repository rule is absolute imports.

…the budget model Three corrections from an independent review of the branch. The shared readiness wait carried one timer, so a subscribe that joined an in-flight warm-up inherited only the remainder of that warm-up's budget — down to nothing — where before this branch it was guaranteed a full wait of its own. Waiters now share one ready/end listener pair but each runs its own deadline, and the signal is torn down when its last waiter gives up. The budget model claimed a dead handshake always costs two command deadlines. That is true only without a password: with one, ioredis treats a timed-out AUTH as fatal and tears the socket down after a single deadline, and a connect that never completes is bounded by connectTimeout, which the formula had no term for. `coldConnectionBudgetMs` now charges the largest of those so it holds for either URL shape, and moves to @sim/utils/retry: a pure helper evaluated at module load must not live in a module the test setup replaces wholesale, which had forced the shared mock to carry a verbatim mirror of the arithmetic. The subscriber derives its budget in its constructor from the exact options it is built with; the shared client's reconnect delay is a pure function used by both retryStrategy and the warm-up budget. The Next server no longer opens the signal subscriber at boot: the execution entry point already warms it on intent, and an eagerly opened subscriber that cannot reach Redis would reconnect on a five-second cadence in every idle replica for the life of the process.

A waiter that timed out was still subscribed to the shared readiness signal, so when that signal later settled for another waiter its cleanup ran a second time. The waiter count drifted negative, the last-waiter teardown could never match again, and the settled signal stayed memoized — a later subscribe during a reconnect would have observed a readiness that had already passed. Each waiter now leaves exactly once, and a settling signal clears itself from the memo so the next waiter observes the connection afresh regardless of the count.

A reconnect also re-subscribes surviving channels, so the new subscription is identified by its channel rather than by SUBSCRIBE having been issued once.

…er must perform The settle path clears the memo on its own, so a drifted waiter count only shows when a later lone waiter times out and fails to tear the signal down. Exercise that path directly; the test now fails without the per-waiter guard.

…ting to any The partial fixtures are widened through unknown to the real ExecutionSnapshot and LoggingSession types, as the repository's TypeScript rule prescribes.

…budget model Findings from an independent review of the finished branch. Constructing the execution-signal hub is what connects — ioredis dials in its constructor — so the warm-up that observed that handshake and returned a boolean its only caller discarded is gone. The execution entry point now constructs the hub, synchronously and never throwing, and the subscribe that follows waits on its own budget. With no observer to accommodate, the waiter machinery collapses to a race between the shared readiness signal and a per-waiter deadline, with a per-signal count so the last waiter detaches. The budget's dead-attempt term was still short for an unanswered handshake: after the ready check times out, ioredis half-closes the socket and destroys it only after `disconnectTimeout` when a wedged peer never answers with a FIN. Measured against such a peer: an unauthenticated connection reconnects at 12.5s, past the 11.5s the budget allowed. The term is now `max(connectTimeout, 2 * commandTimeout + disconnectTimeout)`, with the disconnect deadline stated in the shared connection defaults so the budget derives from it. The model is specific to ioredis 5's handshake sequence, so it lives beside the pinned client in `lib/core/config/redis-budget.ts` — a module the global test mock does not replace — rather than in a generic package. Corrects two doc claims: no caller warms the subscriber at server boot, and a late joiner never had a full wait of its own before this branch either — the memoized promise shared its single timer.

waleedlatif1 · 2026-09-11T18:19:27Z

@greptile

waleedlatif1 · 2026-09-11T18:19:28Z

@cubic-dev-ai review this PR

cubic-dev-ai · 2026-09-11T18:19:33Z

@waleedlatif1 I have started the AI code review. It will take a few minutes to complete.

vercel Bot temporarily deployed to Preview September 11, 2026 06:02 Inactive

greptile-apps Bot reviewed Sep 11, 2026

View reviewed changes

Comment thread apps/sim/lib/execution/execution-signal.ts Outdated

vercel Bot temporarily deployed to Preview September 11, 2026 06:09 Inactive

greptile-apps Bot reviewed Sep 11, 2026

View reviewed changes

Comment thread apps/sim/lib/core/config/redis.ts Outdated

vercel Bot temporarily deployed to Preview September 11, 2026 06:53 Inactive

greptile-apps Bot reviewed Sep 11, 2026

View reviewed changes

Comment thread apps/sim/instrumentation-node.ts Outdated

vercel Bot temporarily deployed to Preview September 11, 2026 06:58 Inactive

vercel Bot deployed to Preview September 11, 2026 07:15 View deployment

greptile-apps Bot reviewed Sep 11, 2026

View reviewed changes

Comment thread apps/sim/lib/execution/execution-signal.ts Outdated

vercel Bot temporarily deployed to Preview September 11, 2026 07:18 Inactive

vercel Bot temporarily deployed to Preview September 11, 2026 07:21 Inactive

vercel Bot temporarily deployed to Preview September 11, 2026 07:23 Inactive

greptile-apps Bot reviewed Sep 11, 2026

View reviewed changes

Comment thread apps/sim/lib/workflows/executor/execution-core.test.ts

greptile-apps Bot reviewed Sep 11, 2026

View reviewed changes

Comment thread apps/sim/lib/workflows/executor/execution-core.test.ts

vercel Bot temporarily deployed to Preview September 11, 2026 07:28 Inactive

waleedlatif1 added 10 commits September 11, 2026 11:12

chore(instrumentation): import the warm-ups through the path alias

c64d1c2

The dynamic imports in the Redis warm-up block used relative paths; the file already resolves `@/` and the repository rule is absolute imports.

test(redis): assert the fresh subscribe by channel, not by call count

84116a8

A reconnect also re-subscribes surviving channels, so the new subscription is identified by its channel rather than by SUBSCRIBE having been issued once.

test(execution-core): type the warm-up test's fixtures instead of cas…

39e039a

…ting to any The partial fixtures are widened through unknown to the real ExecutionSnapshot and LoggingSession types, as the repository's TypeScript rule prescribes.

waleedlatif1 force-pushed the fix/redis-cold-connection-budget branch from 3df2838 to b91b90a Compare September 11, 2026 18:19

vercel Bot temporarily deployed to Preview September 11, 2026 18:19 Inactive

waleedlatif1 merged commit 2be2670 into staging Sep 11, 2026
33 of 34 checks passed

waleedlatif1 deleted the fix/redis-cold-connection-budget branch September 11, 2026 20:24

Uh oh!

Conversation

waleedlatif1 commented Sep 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Verified against the installed ioredis

What this is and is not

Type of Change

Testing

Checklist

Uh oh!

vercel Bot commented Sep 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

greptile-apps Bot commented Sep 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Confidence Score: 5/5

Summary

Diagram

Uh oh!

Uh oh!

waleedlatif1 commented Sep 11, 2026

Uh oh!

waleedlatif1 commented Sep 11, 2026

Uh oh!

cubic-dev-ai Bot commented Sep 11, 2026

Uh oh!

Uh oh!

waleedlatif1 commented Sep 11, 2026

Uh oh!

waleedlatif1 commented Sep 11, 2026

Uh oh!

cubic-dev-ai Bot commented Sep 11, 2026

Uh oh!

waleedlatif1 commented Sep 11, 2026

Uh oh!

waleedlatif1 commented Sep 11, 2026

Uh oh!

cubic-dev-ai Bot commented Sep 11, 2026

Uh oh!

Uh oh!

waleedlatif1 commented Sep 11, 2026

Uh oh!

waleedlatif1 commented Sep 11, 2026

Uh oh!

cubic-dev-ai Bot commented Sep 11, 2026

Uh oh!

waleedlatif1 commented Sep 11, 2026

Uh oh!

waleedlatif1 commented Sep 11, 2026

Uh oh!

cubic-dev-ai Bot commented Sep 11, 2026

Uh oh!

Uh oh!

waleedlatif1 commented Sep 11, 2026

Uh oh!

waleedlatif1 commented Sep 11, 2026

Uh oh!

cubic-dev-ai Bot commented Sep 11, 2026

Uh oh!

waleedlatif1 commented Sep 11, 2026

Uh oh!

cubic-dev-ai Bot commented Sep 11, 2026

Uh oh!

waleedlatif1 commented Sep 11, 2026

Uh oh!

waleedlatif1 commented Sep 11, 2026

Uh oh!

cubic-dev-ai Bot commented Sep 11, 2026

Uh oh!

Uh oh!

Uh oh!

waleedlatif1 commented Sep 11, 2026

Uh oh!

waleedlatif1 commented Sep 11, 2026

Uh oh!

cubic-dev-ai Bot commented Sep 11, 2026

Uh oh!

waleedlatif1 commented Sep 11, 2026

Uh oh!

waleedlatif1 commented Sep 11, 2026 •

edited

Loading

vercel Bot commented Sep 11, 2026 •

edited

Loading

greptile-apps Bot commented Sep 11, 2026 •

edited

Loading