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 in @sim/utils/retry: a dead attempt is charged at the largest of what governs it — connectTimeout for a connect that never completes, one command deadline for an authenticated handshake (ioredis treats a timed-out AUTH as fatal), two for an unauthenticated one (CLIENT SETNAME/SETINFO settle by timing out before the INFO ready check starts its own) — then the client's first reconnect delay, then a healthy attempt. Taking the largest means it holds for either URL shape without parsing it
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 now derived in the constructor from the exact options the client is built with: 11.5s
Each waiter gets its own deadline. Waiters share one ready/end listener pair but run their own timers; a subscribe that joins an in-flight warm-up gets a full wait rather than the remainder of someone else's, and the signal is torn down when its last waiter gives up
Shared client warm-up: budget was 10s against a 5s deadline — exactly the moment ioredis first tears a stalled socket down — so in the case warming exists for, it gave up just before recovery could land. Now 12.3s, derived from the client's own connect deadline and a pure sharedReconnectDelayMs that retryStrategy uses too
The signal subscriber is warmed on intent. Only tasks that execute a workflow ever subscribe, and they are a minority of runs (over a week, 71% of runs never subscribe; one document-processing task is 61% alone). It is warmed as the first, non-awaited statement of executeWorkflowCore — the one path every execution shares — so the handshake overlaps the reads and preprocessing ahead of the subscribe. No task-id list: coverage follows from the funnel. 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, exact production options: an unauthenticated dead handshake tears down at 2×commandTimeout (10.0s at 5000, 6.0s at 3000); an authenticated one at 1× (5.0s) — the production shape. Either way, +500ms retry, then a healthy attempt lands on connection #2. Today's subscriber budget (5000) fails at 5.0s on connection #1, matching the production failure; the derived 11.5s budget recovers it.

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 ~11.5s instead of ~5s, and the same wait applies when Redis is simply unreachable. During a total outage the shared client's 5s deadline trips first in preprocessing, so the worst case is ~17s of worker hold before failing, 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.

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 (two command deadlines, 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 an in-flight warm-up keeps its own full budget while the warm-up's expires; warm() resolves true on ready and false (never rejects) on timeout; warmExecutionSignalHub reports not-warm instead of throwing on a misconfigured URL; executeWorkflowCore begins the warm-up synchronously before its first await. Each verified to fail under a targeted sabotage (dropping the connect term, hardcoding the budget, a shared timer, rethrow, uncaught construction, warm moved after the first await).

Shared test infra changed (redis-config.mock.ts) and @sim/utils gained an export, so the full suite was run: 50,143 tests pass in apps/sim, 381 in apps/realtime, 225 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)

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.

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 7:28am 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 issue remains.

Summary

Derives readiness budgets from each Redis client's connect timeout, command timeout, and first reconnect delay.
Gives concurrent signal-hub waiters independent deadlines while sharing readiness listeners safely.
Starts execution-signal warm-up at the workflow execution entry point without blocking preprocessing.
Adds focused coverage for stalled handshakes, waiter cleanup, warm-up behavior, and budget derivation.
The changes since the previous review replace prohibited any casts with repository-compliant typed fixtures.

Diagram

sequenceDiagram
    participant W as Workflow execution
    participant H as Execution signal hub
    participant R as Redis
    W->>H: warm() (fire-and-forget)
    H->>R: Begin cold connection
    R--xH: First handshake stalls
    Note over H,R: Connect/command deadline diagnoses failure
    H->>R: Reconnect after first retry delay
    R-->>H: ready
    W->>H: subscribe(executionId)
    H->>R: SUBSCRIBE
    R-->>H: Acknowledged

_{Reviews (10) · Last reviewed commit: "test(execution-core): type the warm-up t..."}

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.

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

@cubic-dev-ai review this PR

@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

@cubic-dev-ai review this PR

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

…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.

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

@cubic-dev-ai review this PR

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

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

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

@cubic-dev-ai review this PR

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

…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.

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

@cubic-dev-ai review this PR

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

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.

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

@cubic-dev-ai review this PR

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

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

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

@greptile

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

@cubic-dev-ai review this PR

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

@cubic-dev-ai review this PR

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

…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.

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

@cubic-dev-ai review this PR

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

…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 · 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

@cubic-dev-ai review this PR

@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

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

fa2da17

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

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

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

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

afca7b5

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

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

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

3df2838

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

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

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!

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 •

edited

Loading

vercel Bot commented Sep 11, 2026 •

edited

Loading

greptile-apps Bot commented Sep 11, 2026 •

edited

Loading