diff --git a/packages/core/sdk/src/executor.ts b/packages/core/sdk/src/executor.ts index 4a962c3ab..4b099bb57 100644 --- a/packages/core/sdk/src/executor.ts +++ b/packages/core/sdk/src/executor.ts @@ -1,4 +1,4 @@ -import { Effect, Inspectable, Layer, Option, Predicate, Schema } from "effect"; +import { Duration, Effect, Inspectable, Layer, Option, Predicate, Schema } from "effect"; import { FetchHttpClient, type HttpClient } from "effect/unstable/http"; import { fumadb } from "@executor-js/fumadb"; import { memoryAdapter } from "@executor-js/fumadb/adapters/memory"; @@ -1672,6 +1672,87 @@ export const createExecutor = ( + effect: Effect.Effect, + key: string, + operation: string, + ): Effect.Effect => + effect.pipe( + Effect.timeoutOrElse({ + duration: Duration.millis(CREDENTIAL_PROVIDER_TIMEOUT_MS), + orElse: () => + Effect.fail( + new StorageError({ + message: + `Credential provider "${key}" did not answer ${operation} within ` + + `${CREDENTIAL_PROVIDER_TIMEOUT_MS}ms. The store is unreachable or not responding; ` + + `the credential was not resolved.`, + cause: undefined, + }), + ), + }), + ); + + /** Wrap a provider so every call it exposes is bounded. + * + * Done once at the registration funnel rather than at each call site: every + * provider passes through here on its way in, rather than every call site + * remembering. The five methods `CredentialProvider` has today are named + * explicitly, so a sixth added to the interface must be added here too. + * Optional methods stay optional — a provider that cannot enumerate must not + * appear to. */ + const boundedProvider = (provider: CredentialProvider, key: string): CredentialProvider => { + // Wrapping must change neither how the provider's methods are CALLED nor what the + // object LOOKS like. + // + // Spreading would break the second: a spread copies only own ENUMERABLE properties, so + // everything on a class's prototype — its methods, and accessors like `writable` — is + // dropped silently. Nothing raises; the wrapper simply appears not to have the capability + // and the caller takes a path the provider meant to own. `Object.create` keeps the whole + // object reachable, including anything added to the interface later. + // + // Each bounded method is invoked ON the provider, which is the first half: a destructured + // binding called bare loses `this`, and a class-based provider throws TypeError on its + // first call. Every provider in this repo is an object literal and cannot notice either + // problem, but "wrap any provider" is the whole point of this funnel. + const bounded: Record = { + get: (id: ProviderItemId) => boundedCall(provider.get(id), key, "get"), + }; + if (provider.has) { + bounded.has = (id: ProviderItemId) => boundedCall(provider.has!(id), key, "has"); + } + if (provider.set) { + bounded.set = (id: ProviderItemId, value: string) => + boundedCall(provider.set!(id, value), key, "set"); + } + if (provider.delete) { + bounded.delete = (id: ProviderItemId) => boundedCall(provider.delete!(id), key, "delete"); + } + if (provider.list) { + bounded.list = () => boundedCall(provider.list!(), key, "list"); + } + return Object.assign(Object.create(provider) as CredentialProvider, bounded); + }; + const registerCredentialProvider = ( provider: CredentialProvider, sourceLabel: string, @@ -1685,7 +1766,7 @@ export const createExecutor = ({ + key: STORE, + writable: true, + get, + set: () => Effect.void, +}); + +const plugin = (provider: CredentialProvider) => + definePlugin(() => ({ + id: "acme" as const, + credentialProviders: [provider], + storage: () => ({}), + extension: (ctx) => ({ + seed: () => ctx.core.integrations.register({ slug: INTEG, description: "Acme", config: {} }), + read: () => ctx.connections.resolveValue({ owner: "org", integration: INTEG, name: CONN }), + }), + }))(); + +const executorWithConnection = (provider: CredentialProvider) => + Effect.gen(function* () { + const executor = yield* createExecutor( + makeTestConfig({ plugins: [plugin(provider)] as const }), + ); + yield* executor.acme.seed(); + yield* executor.connections.create({ + owner: "org", + name: CONN, + integration: INTEG, + template: AuthTemplateSlug.make("api_key"), + from: { provider: STORE, id: ProviderItemId.make("item-1") }, + }); + return executor; + }); + +describe("a credential provider that stops answering", () => { + it.effect("fails the resolution instead of hanging it", () => + Effect.gen(function* () { + const executor = yield* executorWithConnection(providerWith(() => Effect.never)); + + const fiber = yield* Effect.forkChild(Effect.exit(executor.acme.read())); + yield* TestClock.adjust(Duration.minutes(5)); + const exit = yield* Fiber.join(fiber); + + expect(Exit.isFailure(exit)).toBe(true); + }), + ); + + it.effect("names the provider and the operation, not just a failure", () => + Effect.gen(function* () { + // A bare timeout would leave an operator looking at whatever the caller was + // doing rather than at the store that stopped answering. + const executor = yield* executorWithConnection(providerWith(() => Effect.never)); + + const fiber = yield* Effect.forkChild(Effect.exit(executor.acme.read())); + yield* TestClock.adjust(Duration.minutes(5)); + const exit = yield* Fiber.join(fiber); + + expect(String(exit)).toContain("remote-store"); + expect(String(exit)).toContain("did not answer"); + // The operation, too — without this the test passes its own name by accident: + // the operation could drop out of the message entirely and nothing would notice. + expect(String(exit)).toContain("get"); + }), + ); + + it.effect("an object-literal provider stores a pasted value — the control", () => + Effect.gen(function* () { + // The control for the class case below: identical in every respect except that the + // provider is an object literal. Without it, a red class test could mean anything. + const items = new Map(); + const lit: CredentialProvider = { + key: STORE, + writable: true, + get: (id) => Effect.sync(() => items.get(String(id)) ?? null), + set: (id, value) => Effect.sync(() => void items.set(String(id), value)), + }; + const executor = yield* createExecutor(makeTestConfig({ plugins: [plugin(lit)] as const })); + yield* executor.acme.seed(); + yield* executor.connections.create({ + owner: "org", + name: CONN, + integration: INTEG, + template: AuthTemplateSlug.make("api_key"), + value: "tok", + }); + expect(yield* executor.acme.read()).toBe("tok"); + }), + ); + + it.effect("wraps a CLASS-based provider without breaking its methods", () => + Effect.gen(function* () { + // The wrapper must not change HOW a provider's own methods are called. `get` was + // invoked with its receiver (`provider.get(id)`) but the optional methods were + // destructured and called bare, which silently drops `this`. Every in-tree provider + // is an object literal and cannot notice; a provider written as a class — exactly + // what "wrap any provider" invites — throws TypeError on the first optional call. + class ClassProvider { + readonly key = STORE; + readonly writable = true; + private readonly items = new Map(); + get(id: ProviderItemId) { + return Effect.sync(() => this.items.get(String(id)) ?? null); + } + set(id: ProviderItemId, value: string) { + // `this` is the whole point: bare invocation makes this line throw. + return Effect.sync(() => void this.items.set(String(id), value)); + } + } + + const executor = yield* createExecutor( + makeTestConfig({ plugins: [plugin(new ClassProvider() as CredentialProvider)] as const }), + ); + yield* executor.acme.seed(); + yield* executor.connections.create({ + owner: "org", + name: CONN, + integration: INTEG, + template: AuthTemplateSlug.make("api_key"), + value: "tok", + }); + + expect(yield* executor.acme.read()).toBe("tok"); + }), + ); + + it.effect("keeps a capability the provider defines on its PROTOTYPE", () => + Effect.gen(function* () { + // The wrapper must not change the provider's SHAPE either. A spread copies only own + // ENUMERABLE properties, so anything on a class's prototype — every method, and any + // accessor like the `writable` below — is dropped silently. Nothing raises; the wrapper + // simply appears not to have it, and the caller takes a path the provider meant to own. + // Here that means `defaultWritableProvider` no longer sees a writable store, so creating + // a connection from a pasted value fails with no provider at all. + class PrototypeProvider { + readonly key = STORE; + private readonly items = new Map(); + // On the PROTOTYPE, not the instance — this is the property a spread loses. + get writable() { + return true; + } + get(id: ProviderItemId) { + return Effect.sync(() => this.items.get(String(id)) ?? null); + } + set(id: ProviderItemId, value: string) { + return Effect.sync(() => void this.items.set(String(id), value)); + } + } + + const executor = yield* createExecutor( + makeTestConfig({ + plugins: [plugin(new PrototypeProvider() as CredentialProvider)] as const, + }), + ); + yield* executor.acme.seed(); + yield* executor.connections.create({ + owner: "org", + name: CONN, + integration: INTEG, + template: AuthTemplateSlug.make("api_key"), + value: "tok", + }); + + expect(yield* executor.acme.read()).toBe("tok"); + }), + ); + + it.effect("still resolves normally when the provider answers", () => + Effect.gen(function* () { + // The control. A bound that refused everything would satisfy both assertions + // above while breaking every working deployment. + const executor = yield* executorWithConnection(providerWith(() => Effect.succeed("tok"))); + + expect(yield* executor.acme.read()).toBe("tok"); + }), + ); +});