Skip to content

fix(surface): keep waitForSignal's transport out of the authored gate on Node 22 - #682

Merged
miyaontherelay merged 2 commits into
mainfrom
ms-lead/long-signal-wait
Oct 22, 2026
Merged

miyaontherelay merged 2 commits into
mainfrom
ms-lead/long-signal-wait

Conversation

@miyaontherelay

@miyaontherelay miyaontherelay commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

Summary

A hosted waitForSignal that outlives one held 204 fails the run after the body has finished. Hosted run 4e320b7a-98bc-4d40-b44d-6a7f4355d5df waited 11 minutes, resumed when the signal landed, ran run-2 to completion, and then failed with:

operation_callback_failed: flow "hv54-signal" derived handler for run-1 (f.run) rejected: The operation was aborted due to timeout

The short case (aa813b41, signal posted 7 s in) passed because it never saw a 204. Native's onboarding waits for a person, often for minutes, so long waits need to work.

Root cause

  • Hosted sandboxes run the authored body on Node 22 (Cloud deploy/e2b/e2b.Dockerfile:24, deploy/daytona/Dockerfile.lite:6, infra/microsandbox-runtime-bridge.ts:23: node:22-bookworm-slim).
  • Node 22's bundled undici builds a body ReadableStream even for a 204. The JS Response.body is null, so nobody can read or cancel that stream. One of its internal promises stays pending forever, and it rejects with the request signal's reason if that signal aborts after the response.
  • holdSignalWait passed AbortSignal.timeout(30_000) straight to fetch. A held 204 returns after 25 s, and its deadline fires 5 s later.
  • The authored completion gate counts every promise created in the body's async context after an f.run as work derived from that step (packages/sdk/src/authored-promise-graph.ts:51). That includes undici's internals. A rejected one is reported as operation_callback_failed (authored-flow-lifecycle.ts:510, authored-flow-operation.ts:241). A pending one is reported as unsettled_derived_work (authored-flow-lifecycle.ts:497, authored-flow-operation.ts:250).
  • So on Node 22 any wait that sees one 204 fails. You get "aborted due to timeout" if the deadline fired before the body returned, and unsettled_derived_work if it had not yet fired. Both faces are reproduced below.

Why Node 22 differs

A probe tracked the promises created inside a single fetch that answered 204. Node 22.14.0 and 22.22.2 leave one pending promise from new ReadableStream in httpNetworkFetch, whether or not the body is consumed or cancelled. Node 24.22.0 and 26.5.0 create none. The same flow, run on unmodified 2.0.54, fails on Node 22.22.2 and completes on 24.22.0 and 26.5.0.

Fix (Surface only)

  1. Each held request runs in this module's load context, through an AsyncResource created at module load and scoped to holdSignalWait's request loop. The transport's internal promises are therefore never attributed to the flow author.
  2. Each request's signal is detached from the deadline and the caller's signal once the request settles. A deadline or a caller's AbortSignal.timeout that fires later can no longer reach a finished fetch.

No change to authored-promise-graph or to any SDK gate rule.

Design reasoning

  • The promise tracker exists to catch work the flow author started. waitForSignal's internal HTTP promises are not that.
  • The promise the body awaits, the helper's own, is still created and tracked in the authored context, and every request outcome reaches it. Transport errors, HTTP refusals and caller aborts still reject it.
  • The parallel certifier already keeps signal waits out of parallel flows.
  • The gate is unchanged. "retains a swallowed Promise.resolve callback failure before terminal success" stays green, which shows a swallowed failure in authored derived work is still caught.

Tests (Node 22.22.2, which CI uses)

New tests:

  • Surface tests/signals.test.ts:
    • runs each held request outside the caller async context: 3 × 204, then 200.
    • detaches each settled request from its deadline and the caller signal.
    • still aborts an in-flight request on its deadline or the caller signal.
  • SDK tests/authored-flow-lifecycle-executor.test.ts, three cases of completes a signal wait after $rounds 204 round(s) with a $requestTimeoutMs ms request deadline. They use a fetch stub that does what Node 22's undici does: a pending internal promise per 204 that rejects on a later abort.
    • 1 round with a 5 ms deadline (the rejected face).
    • 1 round with a 60 000 ms deadline (the pending face).
    • 3 rounds with a 5 ms deadline.

Mutation: 4 tests fail when I revert the AsyncResource. I replaced requestContext.runInAsyncScope(() => fetch(...)) with a plain call (the release step kept), rebuilt, and ran:

$ (cd packages/surface && npx vitest run; cd ../sdk && npx vitest run tests/authored-flow-lifecycle-executor.test.ts)
   × @relayflows/surface/signals waitForSignal > runs each held request outside the caller async context 5ms
     → expected [ 'authored-body', …(3) ] to deeply equal [ undefined, undefined, …(2) ]
      Tests  1 failed | 82 passed (83)
   × authored flow lifecycle through the journal executor > completes a signal wait after 1 204 round(s) with a 5 ms request deadline 315ms
     → unsettled_derived_work: flow "long-signal-wait" completed while work derived from run-1 (f.run) was still in flight
   × authored flow lifecycle through the journal executor > completes a signal wait after 1 204 round(s) with a 60000 ms request deadline 309ms
     → unsettled_derived_work: flow "long-signal-wait" completed while work derived from run-1 (f.run) was still in flight
   × authored flow lifecycle through the journal executor > completes a signal wait after 3 204 round(s) with a 5 ms request deadline 1808ms
     → unsettled_derived_work: flow "long-signal-wait" completed while work derived from run-1 (f.run) was still in flight
      Tests  3 failed | 48 passed (51)

I restored the file (cmp against the saved copy: identical), rebuilt, and re-ran.

Against the published 2.0.54 Surface (before any change), the 1-round, 5 ms case failed with the exact production message: operation_callback_failed: flow "long-signal-wait" derived handler for run-1 (f.run) rejected: The operation was aborted due to timeout.

Full runs on Node 22.22.2, with the SDK resolving the local Surface (npm install ./packages/surface --prefix packages/sdk --no-save --ignore-scripts, as CI does):

$ (cd packages/surface && npm run build && npx tsc -p tsconfig.test.json && npx vitest run)
 Test Files  13 passed (13)
      Tests  83 passed (83)

$ (cd packages/sdk && npx vitest run --reporter=verbose tests/authored-flow-lifecycle-executor.test.ts)
 ✓ ... > retains a swallowed Promise.resolve callback failure before terminal success
 ✓ ... > retains root failure through a swallowed Promise.resolve callback chain
 ✓ ... > completes a signal wait after 1 204 round(s) with a 5 ms request deadline 309ms
 ✓ ... > completes a signal wait after 1 204 round(s) with a 60000 ms request deadline 304ms
 ✓ ... > completes a signal wait after 3 204 round(s) with a 5 ms request deadline 1811ms
      Tests  51 passed (51)

$ (cd packages/sdk && npx vitest run tests/authored-flow-lifecycle-executor.test.ts tests/authored-flow-operation.test.ts tests/authored-flow.test.ts tests/authored-parallel-body.test.ts tests/authored-parallel-eligibility.test.ts)
 ✓ tests/authored-parallel-body.test.ts (5 tests)
 ✓ tests/authored-parallel-eligibility.test.ts (13 tests)
 ✓ tests/authored-flow-operation.test.ts (24 tests)
 ✓ tests/authored-flow.test.ts (38 tests)
 ✓ tests/authored-flow-lifecycle-executor.test.ts (51 tests)

$ (cd packages/sdk && CARGO_TARGET_DIR=… RELAYFLOWD_BIN=…/debug/relayflowd TMPDIR=/private/tmp/lsw-tmp \
    npx vitest run tests/authored-parallel-agents.test.ts tests/authored-parallel-llm.test.ts tests/authored-parallel-run-live.test.ts)
 ✓ tests/authored-parallel-agents.test.ts (8 tests)
 ✓ tests/authored-parallel-llm.test.ts (6 tests)
 ✓ tests/authored-parallel-run-live.test.ts (42 tests | 7 skipped)
      Tests  49 passed | 7 skipped (56)

$ (cd packages/sdk && npm run typecheck:tests)                 # exit 0
$ (cd packages/surface && npm run typecheck:regressions)       # exit 0, HELPERS_GENERATED_OK

On the daemon-backed suites:

  • They need the kernel built by npm run test:prep (ops/cargo.sh, which uses a per-worktree target directory) and the SDK dist built.
  • My first run without CARGO_TARGET_DIR/RELAYFLOWD_BIN, without dist, and with a TMPDIR containing a space failed on those environment problems ("chain fixture daemon failed to start", "Cannot find module …/dist/cli.js", a %20 path). The run above fixes the environment and nothing else.

scripts/surface-package-gate.sh did not run locally. bun install --frozen-lockfile failed on this machine with "failed copying files from cache to destination" for 9 packages, before any test. CI runs it.

Real repro (Node 22.22.2, CLI built from this branch)

  • Setup: node packages/sdk/dist/cli.js run with a flow that calls waitForSignal at the hosted defaults (waitMs 25000, requestTimeoutMs 30000). It talks to a local HTTPS mock of Cloud's signal GET.
  • Long case: the mock holds 3 cycles of 25 s, each answered 204, then delivers the signal 5 s into the 4th hold (about 80 s in).
v22.22.2
5eee931d268b168d2defd88a37d95eeb605d5ccd
✓ run-1 (deterministic) 0.01s completionReason: success
✓ run-2 (deterministic) 0.02s completionReason: success
RUN 01M5EE5493EJGBEW9TTM541C5X completed (3 steps) completionReason: success
mock: +26.8s req 1 -> 204 · +51.8s req 2 -> 204 · +76.8s req 3 -> 204 · +81.8s req 4 -> 200 (signal delivered)
  • Short case: the signal is delivered 7 s into the first hold.
RUN 01M5EE8AVDHW61VCB4NQAA8S84 completed (3 steps) completionReason: success
mock: +9.0s req 1 -> 200 (signal delivered)
  • Control: the same long scenario on published relayflows@2.0.54, Node 22.22.2, reproduces the production failure:
✓ run-2 (deterministic) 0.01s completionReason: success
FAILED [protocol_error] relayflowd could not complete the run request: operation_callback_failed: flow "lsw-signal" derived handler for run-1 (f.run) rejected: The operation was aborted due to timeout

Not in this PR

packages/sdk/src/cloud-http.ts:205 (cloudFetch) uses the same deadline-on-fetch pattern. It is not changed here.

Review follow-ups (a097d75)

  1. Refusal bodies are cancelled under the deadline (Greptile, also confirmed by the adversarial review).
    • On a non-2xx response, the unread body is cancelled inside the request context while the deadline and the caller's signal are still attached. Only then is the request released.
    • Real undici, Node 22.22.2, 503 headers with a stalled body, 2 s deadline: on 5eee931 the server never saw the connection close; on a097d75 it closed 63 ms after the request.
    • Test: cancels a stalled refusal body before detaching the request deadline. It fails under each of three reverts: no cancel, release before cancel, and cancel in the caller's context.
  2. Known limit: the request context pins the AsyncLocalStorage store active when the module is first evaluated.
    • A static import (the documented usage) loads the module before any flow body runs, so it captures no store.
    • If the module is first imported inside als.run(...) (for example a dynamic import from a body), that store is pinned for every later request in the process.
    • Creating the resource lazily does not help, because its first use is always inside a body. Node has no way to name an empty context without holding the storage instances themselves.
    • Documented in the code comment. Pinned by pins the AsyncLocalStorage store active when the module is first evaluated, which expects ['run-A', undefined].
  3. Timeout docs corrected. docs/SURFACE.md and the waitForSignal JSDoc recommended signal: AbortSignal.timeout(ms) to bound a wait. After an f.run, that fails the run whether or not the abort is caught. Racing the wait against a timer fails too. Checked with throwaway gate-level probes, not committed:
    PROBE caught   => failed operation_callback_failed: flow "bounded-wait-caught" derived handler for run-1 (f.run) rejected: The operation was aborted due to timeout
    PROBE uncaught => failed The operation was aborted due to timeout
    PROBE race     => failed unsettled_derived_work: flow "raced-wait" completed while work derived from run-1 (f.run) was still in flight
    
    The docs now say signal stops a run that has waited too long, and that there is no supported way yet to time a wait out and continue. This is a pre-existing gate behaviour, not changed here.

npm run check:docs in packages/surface reports AUTHORING.md drift both at this head and at 5eee931. The drift is the secrets?: string[] options, which predate this PR; I did not regenerate it here.

🤖 Generated with Claude Code


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need.


Agent Relay sessions

  • claude session a3247048 · opened via gh pr create · last active 2026-10-21

… on Node 22

A hosted waitForSignal that saw one held 204 failed the run after the body
finished: "derived handler for run-1 (f.run) rejected: The operation was
aborted due to timeout", or unsettled_derived_work when the request deadline
had not fired yet. Hosted sandboxes run the authored body on Node 22, whose
bundled undici builds an unreachable body stream even for a 204: its internal
promise stays pending, and the request deadline aborting after the response
rejects it. The authored completion gate counts every promise created in the
body's async context after an f.run as that step's derived work.

Run each held request in the module's load context (an AsyncResource), so the
transport's internal promises are not attributed to the author. The promise
the body awaits is still created and tracked in the authored context and
carries every request outcome. Each request's signal is also detached from
the deadline and the caller's signal once it settles. No SDK gate rule
changes; Node 24 and 26 never created the stream.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Oct 21, 2026 •

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: b6f455d4-cb3e-4e79-9022-19473eef5598
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@greptile-apps

greptile-apps Bot commented Oct 21, 2026 •

Copy link
Copy Markdown

RetriggerView in GreptileConfidence Score: 5/5

[High impact] The PR appears safe to merge; the previous error-body cleanup issue is fixed.

Summary

The PR keeps signal-wait HTTP work outside the authored completion gate and disconnects finished requests from later aborts.

  • Hosted signal waits finish after repeated empty responses.

Reviews (2) · Last reviewed commit: "fix(surface): cancel unread signal refus..." · Reviewed by Greptile

Comment thread packages/surface/src/signals.ts Outdated
… correct timeout docs

Review on #682:

- A non-2xx answer released the request before its body was read, so a
  server that sent error headers and stalled the body held the connection
  with no deadline (seen on real undici: the socket stayed open past the
  2 s deadline; it now closes on refusal). The unread body is cancelled in
  the request context, while the deadline still reaches the request, and
  only then released.
- The request context pins the AsyncLocalStorage stores active when the
  module is first evaluated. Documented as a known limit, with a test that
  pins it; a static import (the documented usage) captures none.
- docs/SURFACE.md and the waitForSignal JSDoc recommended bounding the wait
  with AbortSignal.timeout. After an f.run that fails the run whether the
  abort is caught or not (and racing a timer leaves the wait in flight), so
  the docs now say signal stops a run and there is no supported
  time-out-and-continue yet.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@miyaontherelay
miyaontherelay merged commit d982f45 into main Oct 22, 2026
6 checks passed
@miyaontherelay
miyaontherelay deleted the ms-lead/long-signal-wait branch October 22, 2026 04:38
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