-
Notifications
You must be signed in to change notification settings - Fork 0
Fork-only: run the repo's gates on free runners #2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
0b24aec
7cc550f
f9c7999
d96c077
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,61 @@ | ||
| # FORK-ONLY. This file must never reach an upstream pull request. | ||
| # | ||
| # Upstream's ci.yml runs on `blacksmith-4vcpu-ubuntu-2404`, runners provisioned for the upstream org. | ||
| # On this fork nothing picks those jobs up: they sit `queued` with no runner, forever — no cost, and | ||
| # no signal either. That is why the sixteen PRs have never had a check. | ||
| # | ||
| # This runs the repo's OWN gates on `ubuntu-latest` instead. GitHub-hosted standard runners are free | ||
| # for public repositories with no minute cap, and this repository is public, so this costs nothing. | ||
| # Kept deliberately small for the same reason: the gates that catch real defects, and none of the | ||
| # e2e/deploy/docker jobs, which need secrets this fork does not have and would only fail slowly. | ||
| name: Fork gates | ||
|
|
||
| on: | ||
| pull_request: | ||
| workflow_dispatch: | ||
|
|
||
| concurrency: | ||
| group: fork-gates-${{ github.ref }} | ||
| cancel-in-progress: true | ||
|
|
||
| permissions: | ||
| contents: read | ||
|
|
||
| jobs: | ||
| gates: | ||
| name: Lint, format, typecheck, test | ||
| runs-on: ubuntu-latest | ||
| timeout-minutes: 25 | ||
| steps: | ||
| - uses: actions/checkout@v4 | ||
|
|
||
| - uses: oven-sh/setup-bun@v2 | ||
| with: | ||
| bun-version-file: package.json | ||
|
|
||
| - name: Install | ||
| run: bun install --frozen-lockfile | ||
|
|
||
| # Each gate runs even if an earlier one failed, so one run reports everything | ||
| # rather than hiding the second failure behind the first. | ||
| - name: Lint | ||
| id: lint | ||
| if: ${{ !cancelled() }} | ||
| run: bunx --bun oxlint -c .oxlintrc.jsonc . --deny-warnings | ||
|
|
||
| - name: Format | ||
| id: fmt | ||
| if: ${{ !cancelled() }} | ||
| run: bunx --bun oxfmt --check . | ||
|
|
||
| - name: Typecheck (core/sdk) | ||
| id: tc | ||
| if: ${{ !cancelled() }} | ||
| working-directory: packages/core/sdk | ||
| run: bunx --bun tsgo --noEmit | ||
|
|
||
| - name: Test (core/sdk) | ||
| id: test | ||
| if: ${{ !cancelled() }} | ||
| working-directory: packages/core/sdk | ||
| run: bun run test | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 = <const TPlugins extends readonly AnyPlugin[] = rea | |
| static: true, | ||
| }); | ||
|
|
||
| /** How long a credential provider gets to answer one call. | ||
| * | ||
| * A provider is frequently REMOTE — an HTTP secret store, or under sealed custody | ||
| * a vault that may live in another enclave — so "stopped answering" is one of its | ||
| * ordinary failure modes, not an exotic one. Without a bound, a vault that goes | ||
| * away does not fail a tool invocation, it hangs it, and nothing in the resulting | ||
| * silence names the provider. | ||
| * | ||
| * Generous on purpose: this is a backstop against a dead dependency, not a latency | ||
| * budget. A store legitimately slower than this is better served by the operator | ||
| * hearing about it than by the request waiting indefinitely. | ||
| * | ||
| * Executor already bounds its other remote calls this way — OAuth discovery, and | ||
| * the MCP plugin's probes. Credential resolution was the one that did not. */ | ||
| const CREDENTIAL_PROVIDER_TIMEOUT_MS = 30_000; | ||
|
|
||
| /** Bound one provider call, failing with an error that names the provider and the | ||
| * operation — so the diagnostic points at the store rather than at whatever the | ||
| * caller happened to be doing. */ | ||
| const boundedCall = <A>( | ||
| effect: Effect.Effect<A, StorageFailure>, | ||
| key: string, | ||
| operation: string, | ||
| ): Effect.Effect<A, StorageFailure> => | ||
| 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, | ||
|
Comment on lines
+1705
to
+1709
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win Use an operation-neutral timeout message. When 🤖 Prompt for AI Agents |
||
| }), | ||
| ), | ||
| }), | ||
| ); | ||
|
|
||
| /** 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<string, unknown> = { | ||
| 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); | ||
|
Comment on lines
+1723
to
+1753
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
# Inspect provider accessors that could depend on their original receiver.
rg -n -C 4 --glob '*.ts' 'get\s+(key|writable)\s*\(|#[A-Za-z_]' packages/core/sdk
# Confirm that the prototype-provider fixture exercises private-field access.
sed -n '162,201p' packages/core/sdk/src/provider-call-timeout.test.tsRepository: GeiserX/executor Length of output: 11110 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- executor definitions and wrapper use ---'
rg -n -C 8 'const boundedProvider|defaultWritableProvider|provider\.writable|provider\.key|boundedCall' packages/core/sdk/src/executor.ts
printf '%s\n' '--- credential provider declarations and implementations ---'
rg -n -C 6 'interface CredentialProvider|type CredentialProvider|writable|readonly key|private readonly' packages/core/sdk/src packages/core/sdk/test packages/core/sdk/*.ts 2>/dev/null || true
printf '%s\n' '--- relevant test block ---'
sed -n '150,215p' packages/core/sdk/src/provider-call-timeout.test.ts
printf '%s\n' '--- ECMAScript receiver probe ---'
node - <<'JS'
class Provider {
`#writable` = true;
`#key` = "store";
get writable() { return this.#writable; }
get key() { return this.#key; }
get() { return this.#key; }
}
const provider = new Provider();
const wrapped = Object.assign(Object.create(provider), {
get: () => provider.get(),
});
for (const property of ["writable", "key"]) {
try {
console.log(property, wrapped[property]);
} catch (error) {
console.log(property, error.name, error.message);
}
}
console.log("method", wrapped.get());
JSRepository: GeiserX/executor Length of output: 50372 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- complete runtime probe output ---'
node - <<'JS'
class Provider {
`#writable` = true;
`#key` = "store";
get writable() { return this.#writable; }
get key() { return this.#key; }
get() { return this.#key; }
}
const provider = new Provider();
const wrapped = Object.assign(Object.create(provider), {
get: () => provider.get(),
});
for (const property of ["writable", "key"]) {
try {
console.log(property, wrapped[property]);
} catch (error) {
console.log(property, error.name, error.message);
}
}
console.log("method", wrapped.get());
JS
printf '%s\n' '--- complete prototype-provider test ---'
sed -n '162,205p' packages/core/sdk/src/provider-call-timeout.test.ts
printf '%s\n' '--- all wrapper property reads ---'
rg -n -C 3 'credentialProviders\.get|provider\.(key|writable)|\.key|\.writable' packages/core/sdk/src/executor.ts | sed -n '1,180p'Repository: GeiserX/executor Length of output: 9365 Forward inherited accessors to the original provider.
🤖 Prompt for AI Agents |
||
| }; | ||
|
|
||
| const registerCredentialProvider = ( | ||
| provider: CredentialProvider, | ||
| sourceLabel: string, | ||
|
|
@@ -1685,7 +1766,7 @@ export const createExecutor = <const TPlugins extends readonly AnyPlugin[] = rea | |
| }), | ||
| ); | ||
| } | ||
| credentialProviders.set(key, provider); | ||
| credentialProviders.set(key, boundedProvider(provider, key)); | ||
| credentialProviderOrder.push(key); | ||
| return Effect.void; | ||
| }; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,212 @@ | ||
| // --------------------------------------------------------------------------- | ||
| // A credential provider that stops answering must fail the resolution, not hang it. | ||
| // | ||
| // A provider is frequently REMOTE — an HTTP secret store, or under sealed custody a | ||
| // vault that may live in another enclave — so "stopped answering" is one of its | ||
| // ordinary failure modes. Unbounded, a store that goes away does not fail a tool | ||
| // invocation, it hangs it, and nothing in the resulting silence names the provider. | ||
| // | ||
| // Executor already bounds its other remote calls this way (OAuth discovery, the MCP | ||
| // plugin's probes); credential resolution was the one that did not. | ||
| // | ||
| // Time is virtual here: the bound is deliberately generous, and a test that waited | ||
| // it out in real time would be a thirty-second test. TestClock is advanced past it | ||
| // instead — which is also why this uses `it.effect` rather than `it.live`. | ||
| // --------------------------------------------------------------------------- | ||
|
|
||
| import { describe, expect, it } from "@effect/vitest"; | ||
| import { Duration, Effect, Exit, Fiber } from "effect"; | ||
| import { TestClock } from "effect/testing"; | ||
|
|
||
| import { createExecutor } from "./executor"; | ||
| import { | ||
| AuthTemplateSlug, | ||
| ConnectionName, | ||
| IntegrationSlug, | ||
| ProviderItemId, | ||
| ProviderKey, | ||
| } from "./ids"; | ||
| import { definePlugin } from "./plugin"; | ||
| import type { CredentialProvider } from "./provider"; | ||
| import { makeTestConfig } from "./test-config"; | ||
|
|
||
| const STORE = ProviderKey.make("remote-store"); | ||
| const INTEG = IntegrationSlug.make("acme"); | ||
| const CONN = ConnectionName.make("main"); | ||
|
|
||
| const providerWith = (get: CredentialProvider["get"]): CredentialProvider => ({ | ||
| 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<string, string>(); | ||
| 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<string, string>(); | ||
| 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<string, string>(); | ||
| // 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"); | ||
| }), | ||
| ); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
Repository: GeiserX/executor
Length of output: 311
🏁 Script executed:
Repository: GeiserX/executor
Length of output: 2714
Enforce the fork-only restriction.
If this workflow reaches upstream,
gateswill run for upstream pull requests and manual dispatches. Add a job-level condition:Proposed fix
jobs: gates: + if: github.repository == 'GeiserX/executor' name: Lint, format, typecheck, test📝 Committable suggestion
🤖 Prompt for AI Agents