Skip to content

fix(sdk): bind embedded transport at request time - #46971

Open
kitlangton wants to merge 1 commit into
v2from
sdk-transport
Open

fix(sdk): bind embedded transport at request time#46971
kitlangton wants to merge 1 commit into
v2from
sdk-transport

Conversation

@kitlangton

Copy link
Copy Markdown
Contributor

Why

An Effect application that supplies FetchHttpClient.Fetch can accidentally redirect embedded SDK requests away from their in-process host. Even client.health.get() uses the caller's Fetch instead of the embedded router. The Core test harness's network guard exposed this in all three Effect instance tests.

Providing Fetch while constructing FetchHttpClient.layer is not sufficient: Effect resolves that reference when each request executes, and the caller's context can override the captured value.

What Changes

Bind the owning host's Fetch around the HTTP response effect for every SDK request using HttpClient.transformResponse and Effect.provideService.

  • Before: a caller-side Fetch override receives requests for http://opencode.local, causing failures or sending them to the wrong transport.
  • After: SDK requests always use their embedded host, while the caller's surrounding Fetch remains unchanged.
  • The same binding covers ordinary requests and stream establishment, including calls made through OpenCode.layer.
  • Typed API errors and host shutdown still use the existing client and transport paths. The network guard is unchanged.

Scope

This changes the shared Effect SDK constructor, also used by the Effect Workerd entrypoint. It does not change generated clients, public APIs, provider HTTP configuration, or Promise SDK behavior. The Slack consumer upgrade remains a follow-up after a published SDK contains the fix.

Verification

Using Bun 1.4.0:

cd packages/sdk
bun run test test/transport.test.ts
bun run test test/transport.test.ts test/instances.test.ts test/instances-effect.test.ts test/instances-lifecycle.test.ts
bun typecheck
bun run test
cd ../..
bunx prettier --check packages/sdk/src/effect/opencode.ts packages/sdk/test/transport.test.ts
bunx oxlint packages/sdk/src/effect/opencode.ts packages/sdk/test/transport.test.ts
git diff --check
  • New regression tests: 2 failed before the fix, 2 passed after. They exercise both create and layer with a conflicting Fetch at construction and call time, real router requests, durable event reads, live event subscription, typed not-found errors, and calls after host closure. No global Fetch mutation or network listener is used.
  • Focused transport/instance suite: 11 passed, including all three previously failing Effect instance tests.
  • SDK typecheck, formatting, lint, and whitespace checks passed.
  • The unmodified pre-push workspace typecheck passed: 33 successful tasks, 24 cached. No hooks were skipped.
  • Broader SDK suite: 29 passed, 2 failed. The remaining known failures are the existing plugin-backed web-search test (websearch_provider_not_found) and Promise event cancellation expectation (iterator completion instead of a transport error); neither test was changed here.
  • No native Workerd or Slack deployment test was performed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant