Merge pull request #690 from RhysSullivan/codex/real-protocol-testing-first-pass

Add real protocol testing fixtures

Author: RhysSullivan

.agents/skills/wrdn-effect-raw-fetch-boundary/SKILL.md

@@ -0,0 +1,58 @@
+---
+name: wrdn-effect-raw-fetch-boundary
+description: Route HTTP through Effect boundaries instead of raw fetch. Use when lint flags executor/no-raw-fetch or when adding networked protocol/provider code.
+allowed-tools: Read Grep Glob Bash
+---
+
+HTTP in core SDK and protocol plugins should go through Effect services so tests
+can replace real networks with local protocol fixtures or mock `HttpClient`
+layers.
+
+## Fix Shape
+
+- Prefer `effect/unstable/http` `HttpClient` and `HttpClientRequest` for
+  ordinary HTTP calls.
+- Accept a `Layer.Layer<HttpClient.HttpClient>` option on plugin/provider code
+  when callers need to inject a test client.
+- In tests, use real local servers plus `FetchHttpClient.layer` or a captured
+  `HttpClient` service from the test layer.
+- Do not add fetch-shaped abstractions in SDK or plugin seams. If a third-party
+  library truly only accepts `fetch`, keep the adapter in the owning package,
+  name the forced boundary explicitly, and delegate internally to Effect
+  `HttpClient`.
+- Do not type new protocol/plugin seams as `typeof globalThis.fetch`; keep the
+  ambient runtime boundary out of domain and test APIs.
+- Do not patch `globalThis.fetch`. Replace those tests with a local server,
+  `HttpClient` layer, or the approved Effect-backed adapter.
+- Do not add a broad allowlist entry unless the file is a platform entrypoint
+  or a temporary migration target.
+
+## Approved Boundaries
+
+- Worker/handler objects whose public API must expose a `fetch` method.
+- Test calls to a worker or Miniflare binding's `.fetch(...)` method.
+- Small adapters for libraries that only accept `fetch`, if the implementation
+  delegates to Effect `HttpClient`.
+- Browser UI event handlers may remain raw only until app-side boundaries are
+  classified; prefer SDK/client APIs where available.
+
+## Bad
+
+```ts
+const response = await fetch(url);
+```
+
+```ts
+const fetchImpl = options.fetch ?? globalThis.fetch;
+```
+
+## Good
+
+```ts
+const client = yield * HttpClient.HttpClient;
+const response = yield * client.execute(HttpClientRequest.get(url));
+```
+
+```ts
+const plugin = graphqlPlugin({ httpClientLayer: testHttpClientLayer });
+```

.oxlintrc.jsonc

@@ -17,6 +17,7 @@
     "executor/no-manual-tag-check": "error",
     "executor/no-promise-catch": "error",
     "executor/no-promise-reject": "error",
+    "executor/no-raw-fetch": "error",
     "executor/no-redundant-primitive-cast": "error",
     "executor/no-redundant-error-factory": "error",
     "executor/no-ts-nocheck": "error",

bun.lock

@@ -46,7 +46,7 @@ Testing services belong behind explicit `./testing` subpath exports:
 
 The plugin packages own the protocol details. Core SDK testing owns only boring
 Executor fixtures such as memory adapters, memory secrets, auto-accept
-elicitation, and fetch-shaped adapters that delegate to Effect HTTP.
+elicitation, and other Effect-native test primitives.
 
 Use `TestLayer` / `TestLayers` naming for realistic local test services.
 Reserve `LiveLayer` / `LiveLayers` for production wiring. These servers are
@@ -124,7 +124,6 @@ Suggested file layout:
 ```txt
 packages/core/sdk/src/testing.ts
 packages/core/sdk/src/testing/
-  fetch.ts
   memory-secrets.ts
 
 packages/plugins/openapi/src/testing/
@@ -191,9 +190,6 @@ Useful additions:
   in-test secret provider definitions.
 - `makeExecutorTestLayer(options)`, if repeated `createExecutor(makeTestConfig)`
   setup becomes noisy.
-- `makeFetchFromHttpClient` or `FetchTestAdapter`, for third-party APIs that
-  require a fetch-shaped callback while still routing requests through Effect's
-  HTTP client.
 - Small request/response capture primitives built on `Ref`, only if multiple
   plugins need the same shape.
 
@@ -243,8 +239,8 @@ Work:
 - Support a fresh `McpServer` factory per session, matching the current helper.
 - Keep malformed/non-MCP HTTP endpoints as explicit test layers for probe
   behavior.
-- Replace fetch-injected probe tests with real local HTTP servers and the
-  Effect-backed fetch adapter.
+- Replace fetch-injected probe tests with real local HTTP servers and
+  Effect-native HTTP boundaries.
 
 Representative scenarios:
 
@@ -261,10 +257,11 @@ Representative scenarios:
 GraphQL should gain a real executable schema server. This is the place where a
 new dependency may be justified.
 
-Preferred first dependency is the core `graphql` package if it is enough:
-define a schema, execute introspection and operations with `graphql(...)`, and
-serve the standard JSON-over-HTTP GraphQL endpoint through Effect/node HTTP.
-Use `graphql-yoga` only if we need a fuller server feature set later.
+Use `graphql-yoga` for the test server. The extra dependency is justified
+because the fixture needs to exercise a real JSON-over-HTTP GraphQL endpoint,
+not only direct `graphql(...)` execution. Keep Vitest configured to inline the
+Yoga/GraphQL dependency chain so executable schemas and server execution share
+one GraphQL module realm.
 
 Work:
 
@@ -292,19 +289,19 @@ Representative scenarios:
 ## Fetch Boundary
 
 The target rule is: product and plugin code should not call raw `fetch`
-directly. HTTP should go through Effect HTTP or a small approved adapter that
-turns an Effect `HttpClient` into a fetch-shaped function for libraries that
-force that API.
+directly. HTTP should go through Effect HTTP. If a third-party library truly
+forces a fetch-shaped callback, the adapter should live at that owning package's
+boundary rather than in shared SDK testing.
 
 Boundary files may exist, but they should be explicit and scarce. Examples:
 
-- a SDK adapter for `oauth4webapi` custom fetch
+- a core OAuth adapter for `oauth4webapi` custom fetch, if that library forces it
 - a MCP transport adapter if the upstream MCP SDK requires fetch-shaped input
 - platform entrypoints that must implement a `fetch(request, env, ctx)` method
 - test harnesses that intentionally call a Worker binding's `fetch` method
 
-Everything else should use Effect HTTP, Executor client APIs, or the approved
-adapter.
+Everything else should use Effect HTTP, Executor client APIs, or an explicit
+package-local boundary.
 
 Lint plan:
 
@@ -319,17 +316,17 @@ Lint plan:
 - Start with a narrow allowlist for known boundary files and remove entries as
   migrations land.
 - Include error messages that tell the author which Effect HTTP or approved
-  adapter to use.
+  package-local boundary to use.
 
-The lint rule should land after the first adapter exists. Otherwise every fix
-has to invent its own local escape hatch.
+The lint rule can land once the core test primitives and at least one real
+protocol fixture exist. Fetch-shaped adapters should be added only at forced
+library boundaries during migration.
 
 ## Migration Order
 
 1. Add `./testing` exports and SDK test primitives.
    - Expose `@executor-js/sdk/testing`.
    - Add memory secrets test support.
-   - Add the Effect HTTP to fetch-shaped adapter.
    - Keep `makeTestConfig` available from the root SDK.
 
 2. Build the GraphQL vertical slice.
@@ -344,8 +341,8 @@ has to invent its own local escape hatch.
    - Migrate current tests to import from the new public testing subpaths.
 
 4. Replace global fetch patching.
-   - Convert core OAuth discovery/helper tests to real local servers or the
-     Effect-backed fetch adapter.
+   - Convert core OAuth discovery/helper tests to real local servers or
+     package-local Effect HTTP boundaries.
    - Convert OpenAPI OAuth tests to the OAuth test server.
    - Convert MCP probe tests to real local HTTP server layers.
    - Convert Google Discovery tests using the same boundary pattern.
@@ -403,8 +400,8 @@ Turbo. Do not use `bun test`.
 
 - Should `@executor-js/sdk/testing` be a new subpath around the existing
   `src/testing.ts`, or should `src/testing.ts` become `src/testing/index.ts`?
-- Should GraphQL use only the `graphql` package, or should we adopt
-  `graphql-yoga` once tests need more HTTP behavior?
+- How much GraphQL HTTP behavior should live in the shared fixture versus
+  scenario-specific schemas in individual tests?
 - Which app-level raw fetch calls should remain approved platform boundaries
   versus being migrated to Effect HTTP?
 - Should the raw fetch lint rule become repo-wide immediately with allowlists,

notes/real-protocol-testing-plan.md

@@ -19,7 +19,8 @@
     ".": "./src/index.ts",
     "./core": "./src/index.ts",
     "./promise": "./src/promise.ts",
-    "./client": "./src/client.ts"
+    "./client": "./src/client.ts",
+    "./testing": "./src/testing.ts"
   },
   "publishConfig": {
     "access": "public",
@@ -41,6 +42,12 @@
           "types": "./dist/client.d.ts",
           "default": "./dist/client.js"
         }
+      },
+      "./testing": {
+        "import": {
+          "types": "./dist/testing.d.ts",
+          "default": "./dist/testing.js"
+        }
       }
     }
   },

packages/core/sdk/package.json

@@ -1,11 +1,14 @@
 import { makeMemoryAdapter } from "@executor-js/storage-core/testing/memory";
 
+import { Effect } from "effect";
+
 import { makeInMemoryBlobStore } from "./blob";
 import type { ExecutorConfig } from "./executor";
 import { collectSchemas } from "./executor";
 import { ScopeId } from "./ids";
-import type { AnyPlugin } from "./plugin";
+import { definePlugin, type AnyPlugin } from "./plugin";
 import { Scope } from "./scope";
+import type { SecretProvider } from "./secrets";
 
 // ---------------------------------------------------------------------------
 // makeTestConfig — build an ExecutorConfig backed by in-memory adapter +
@@ -42,3 +45,31 @@ export const makeTestConfig = <const TPlugins extends readonly AnyPlugin[] = []>
     onElicitation: "accept-all",
   };
 };
+
+export const memorySecretsPlugin = definePlugin(() => {
+  const store = new Map<string, string>();
+
+  const provider: SecretProvider = {
+    key: "memory",
+    writable: true,
+    get: (id, scope) => Effect.sync(() => store.get(`${scope}\u0000${id}`) ?? null),
+    set: (id, value, scope) =>
+      Effect.sync(() => {
+        store.set(`${scope}\u0000${id}`, value);
+      }),
+    delete: (id, scope) => Effect.sync(() => store.delete(`${scope}\u0000${id}`)),
+    list: () =>
+      Effect.sync(() =>
+        Array.from(store.keys()).map((key) => {
+          const name = key.split("\u0000", 2)[1] ?? key;
+          return { id: name, name };
+        }),
+      ),
+  };
+
+  return {
+    id: "memory-secrets" as const,
+    storage: () => ({}),
+    secretProviders: [provider],
+  };
+});

packages/core/sdk/src/testing.ts

@@ -5,6 +5,7 @@ export default defineConfig({
     index: "src/promise.ts",
     core: "src/index.ts",
     client: "src/client.ts",
+    testing: "src/testing.ts",
   },
   format: ["esm"],
   dts: false,

packages/core/sdk/tsup.config.ts

@@ -21,7 +21,8 @@
     "./api": "./src/api/index.ts",
     "./react": "./src/react/index.ts",
     "./presets": "./src/sdk/presets.ts",
-    "./client": "./src/react/plugin-client.tsx"
+    "./client": "./src/react/plugin-client.tsx",
+    "./testing": "./src/testing/index.ts"
   },
   "publishConfig": {
     "access": "public",
@@ -37,6 +38,12 @@
           "types": "./dist/sdk/index.d.ts",
           "default": "./dist/core.js"
         }
+      },
+      "./testing": {
+        "import": {
+          "types": "./dist/testing/index.d.ts",
+          "default": "./dist/testing.js"
+        }
       }
     }
   },
@@ -51,7 +58,9 @@
     "@effect/platform-node": "catalog:",
     "@executor-js/config": "workspace:*",
     "@executor-js/sdk": "workspace:*",
-    "effect": "catalog:"
+    "effect": "catalog:",
+    "graphql": "^16.12.0",
+    "graphql-yoga": "^5.17.0"
   },
   "devDependencies": {
     "@effect/atom-react": "catalog:",

packages/plugins/graphql/package.json

@@ -220,8 +220,9 @@ export const introspect = Effect.fn("GraphQL.introspect")(function* (
   );
 
   if (response.status !== 200) {
+    const body = yield* response.text.pipe(Effect.catch(() => Effect.succeed("<unreadable>")));
     return yield* new GraphqlIntrospectionError({
-      message: `Introspection failed with status ${response.status}`,
+      message: `Introspection failed with status ${response.status}: ${body.slice(0, 1_000)}`,
     });
   }