[pull] main from UsefulSoftwareCo:main

.changeset/oauth-popup-reserve-on-click.md

@@ -0,0 +1,9 @@
+---
+"@executor-js/react": patch
+---
+
+**A slow OAuth discovery no longer kills the connect with no popup and no error**
+
+The transparent connect flows opened the sign-in window only after their setup round trips had answered: DCR after probe and dynamic registration, CIMD after minting the client, reconnect after starting the session. `window.open` needs transient user activation, which browsers expire a few seconds after the click, so once the API was slow enough the browser refused the window and the connect ended with nothing on screen but the button returning to "Connect". Every MCP integration takes that path.
+
+The window is now claimed on the click itself and navigated when the authorization URL arrives, however long that takes, and it is closed again on the paths that end without signing in (failed probe, no registration endpoint, rejected registration, failed client mint) as well as on cancel and unmount. A window the browser does refuse is now reported instead of swallowed: the flows stop before their round trips, and the sign-in error renders above the dialog footer, where the automatic flows can actually show it, rather than inside a method tab panel they never render.

.github/workflows/ci.yml

@@ -10,6 +10,18 @@ concurrency:
   group: ci-${{ github.ref }}
   cancel-in-progress: true
 
+# CI is not a user. Without this, executor's own product telemetry reports to the
+# production PostHog project from every e2e shard: the analytics layer
+# (integration_added, execution_completed, artifact_*) and the
+# integrations-registry fetch, which lands as a `hit` event. Each shard is a
+# fresh container, so it mints a fresh anonymous id and arrives on a fresh
+# runner IP — inflating machine and user counts by roughly an order of
+# magnitude. Both opt-outs are the packages' documented CI/test hooks.
+env:
+  DO_NOT_TRACK: "1"
+  EXECUTOR_DISABLE_ANALYTICS: "1"
+  EXECUTOR_DISABLE_INTEGRATIONS_FETCH: "1"
+
 jobs:
   changes:
     name: Changed paths

.github/workflows/publish-desktop.yml

@@ -21,6 +21,18 @@ concurrency:
   group: publish-desktop-${{ github.ref }}
   cancel-in-progress: false
 
+# CI is not a user. Without this, executor's own product telemetry reports to the
+# production PostHog project from every e2e shard: the analytics layer
+# (integration_added, execution_completed, artifact_*) and the
+# integrations-registry fetch, which lands as a `hit` event. Each shard is a
+# fresh container, so it mints a fresh anonymous id and arrives on a fresh
+# runner IP — inflating machine and user counts by roughly an order of
+# magnitude. Both opt-outs are the packages' documented CI/test hooks.
+env:
+  DO_NOT_TRACK: "1"
+  EXECUTOR_DISABLE_ANALYTICS: "1"
+  EXECUTOR_DISABLE_INTEGRATIONS_FETCH: "1"
+
 jobs:
   build:
     permissions:

.github/workflows/publish-selfhost-docker.yml

@@ -16,6 +16,18 @@ concurrency:
   group: publish-selfhost-docker-${{ inputs.tag }}
   cancel-in-progress: false
 
+# CI is not a user. Without this, executor's own product telemetry reports to the
+# production PostHog project from every e2e shard: the analytics layer
+# (integration_added, execution_completed, artifact_*) and the
+# integrations-registry fetch, which lands as a `hit` event. Each shard is a
+# fresh container, so it mints a fresh anonymous id and arrives on a fresh
+# runner IP — inflating machine and user counts by roughly an order of
+# magnitude. Both opt-outs are the packages' documented CI/test hooks.
+env:
+  DO_NOT_TRACK: "1"
+  EXECUTOR_DISABLE_ANALYTICS: "1"
+  EXECUTOR_DISABLE_INTEGRATIONS_FETCH: "1"
+
 jobs:
   metadata:
     runs-on: blacksmith-4vcpu-ubuntu-2404

.github/workflows/release.yml

@@ -13,6 +13,18 @@ concurrency:
   group: release-${{ github.ref }}
   cancel-in-progress: false
 
+# CI is not a user. Without this, executor's own product telemetry reports to the
+# production PostHog project from every e2e shard: the analytics layer
+# (integration_added, execution_completed, artifact_*) and the
+# integrations-registry fetch, which lands as a `hit` event. Each shard is a
+# fresh container, so it mints a fresh anonymous id and arrives on a fresh
+# runner IP — inflating machine and user counts by roughly an order of
+# magnitude. Both opt-outs are the packages' documented CI/test hooks.
+env:
+  DO_NOT_TRACK: "1"
+  EXECUTOR_DISABLE_ANALYTICS: "1"
+  EXECUTOR_DISABLE_INTEGRATIONS_FETCH: "1"
+
 jobs:
   release:
     runs-on: ubuntu-latest

apps/cloud/scripts/start-closure.mjs

@@ -12,7 +12,10 @@
 //             cold isolate before any request is served.
 //   start   - the TanStack Start server graph, reached through the lazy
 //             `loadEntries` dynamic imports. Evaluated on the first request
-//             that enters the app.
+//             that enters the app - i.e. a page request.
+//   app     - the Effect app plane. `/api/*` dispatches at the Worker entry and
+//             skips Start entirely, so an API request evaluates this instead of
+//             `start`; reported separately because the two planes now diverge.
 //
 // Anything reachable only through a dynamic import is not counted: making a
 // heavy dependency lazy is exactly the outcome this rewards.
@@ -81,6 +84,9 @@ const startRoots = [...graph.get(ENTRY).dynamic].filter((f) =>
   /(start|router|tanstack)/.test(name(f)),
 );
 const start = closure(startRoots);
+const appRoots = [...graph.get(ENTRY).dynamic].filter((f) => /\/app-[A-Za-z0-9_-]+\.js$/.test(f));
+const app = closure([ENTRY, ...appRoots]);
+// The budget tracks the worst plane: whichever costs a cold isolate more.
 const evaluated = new Set([...startup, ...start]);
 
 const report = (label, files) => {
@@ -93,7 +99,10 @@ const report = (label, files) => {
 
 report("startup", startup);
 report("start", start);
-console.log(`\ntotal evaluated on a warm-path request: ${mb(bytes(evaluated))}`);
+console.log(`\npage request  (startup + start): ${mb(bytes(evaluated))}`);
+console.log(
+  `API request   (startup + app):   ${mb(bytes(app))}${appRoots.length ? "" : "  [no app chunk - /api still routes through Start]"}`,
+);
 const lazyOnly = [...graph.keys()].filter((f) => !evaluated.has(f));
 console.log(
   `deferred behind dynamic import:        ${mb(bytes(lazyOnly))}  (${lazyOnly.length} chunks)`,

apps/cloud/src/app-paths.test.ts

@@ -1,6 +1,6 @@
 import { describe, expect, it } from "@effect/vitest";
 
-import { isAppOwnedPath } from "./app-paths";
+import { isAppOwnedPath, servedByAppPlane } from "./app-paths";
 
 // Guards the start.ts dispatch decision: every surface the unified app handler
 // serves must be classified app-owned (forwarded to `app.handler`), and Start's
@@ -62,3 +62,39 @@ describe("isAppOwnedPath", () => {
     });
   }
 });
+
+describe("app-plane dispatch", () => {
+  // These two are the whole risk of dispatching `/api` before Start: both still
+  // return a response if routed early, just the wrong one, so nothing else would
+  // catch a regression here.
+  it("leaves the Sentry tunnel POST to Start's middleware", () => {
+    expect(servedByAppPlane("/api/sentry-tunnel", "POST")).toBe(false);
+    // Only the POST is claimed; anything else under that path is ordinary API.
+    expect(servedByAppPlane("/api/sentry-tunnel", "GET")).toBe(true);
+  });
+
+  it("leaves the OAuth callback to Start, for the signed-out redirect", () => {
+    expect(servedByAppPlane("/api/oauth/callback", "GET")).toBe(false);
+    expect(servedByAppPlane("/api/oauth/callback", "POST")).toBe(false);
+  });
+
+  const appPlane = [
+    "/api/connections",
+    "/api/tools",
+    "/api/integrations",
+    "/api/account/members",
+    "/api/docs",
+    "/api/billing/checkout",
+  ];
+  for (const pathname of appPlane) {
+    it(`serves ${pathname} without entering Start`, () => {
+      expect(servedByAppPlane(pathname, "GET")).toBe(true);
+    });
+  }
+
+  it("never claims a non-API path, however app-owned", () => {
+    expect(servedByAppPlane("/mcp", "POST")).toBe(false);
+    expect(servedByAppPlane("/", "GET")).toBe(false);
+    expect(servedByAppPlane("/.well-known/oauth-authorization-server", "GET")).toBe(false);
+  });
+});

apps/cloud/src/app-paths.ts

@@ -17,3 +17,31 @@ export const isApiPath = (pathname: string) => pathname === "/api" || pathname.s
 
 export const isAppOwnedPath = (pathname: string) =>
   isApiPath(pathname) || classifyMcpPath(pathname) !== null;
+
+// ---------------------------------------------------------------------------
+// Which plane serves an app-owned path: the Effect app directly, or TanStack
+// Start's middleware chain.
+//
+// Everything under `/api` is pure Effect and touches no part of the router,
+// React, or SSR — so `server.ts` dispatches it at the Worker entry and skips
+// Start's lazy `loadEntries` import entirely. Two paths must NOT take that
+// shortcut, because Start's request middleware claims them BEFORE the app
+// handler would ever see them:
+//
+//   POST /api/sentry-tunnel  - `sentryTunnelMiddleware` forwards the envelope
+//                              to Sentry; the app has no such route.
+//   /api/oauth/callback      - `oauthCallbackSignInMiddleware` redirects a
+//                              signed-out visitor to /login, and start.ts
+//                              rewrites the org-scoped `state` before handing
+//                              off. Routing it early would drop both.
+//
+// Getting this wrong is silent: the request still gets a response, just the
+// wrong one, which is why it is classified here and tested rather than being
+// an inline condition at the dispatch site.
+// ---------------------------------------------------------------------------
+
+export const isStartOwnedApiPath = (pathname: string, method: string): boolean =>
+  (pathname === "/api/sentry-tunnel" && method === "POST") || pathname === "/api/oauth/callback";
+
+export const servedByAppPlane = (pathname: string, method: string): boolean =>
+  isApiPath(pathname) && !isStartOwnedApiPath(pathname, method);

apps/cloud/src/engine/execution-rate-limit.node.test.ts

@@ -0,0 +1,171 @@
+import { describe, expect, it } from "@effect/vitest";
+import { Data, Effect } from "effect";
+
+import type { ExecutionEngine } from "@executor-js/execution";
+
+import { makeExecutionRateLimiter } from "./execution-rate-limit";
+import { RATE_LIMIT_BLOCKED_MESSAGE } from "./execution-limit-messages";
+
+const ORG = "org_test";
+
+/** Stands in for whatever the real lookups fail with (Autumn down, DO down). */
+class UpstreamDownError extends Data.TaggedError("UpstreamDownError")<{
+  readonly which: string;
+}> {}
+
+// A stand-in engine: `execute` resolves to a marker, so a test tells an allowed
+// execution (marker) from a blocked one (the gate's error result) by which of
+// the two came back. A blocked decision never reaches the engine at all.
+const engineStub: ExecutionEngine = {
+  execute: () => Effect.succeed({ result: "ran" }),
+  executeWithPause: () => Effect.succeed({ status: "completed", result: { result: "ran" } }),
+  resume: () => Effect.succeed(null),
+  getPausedExecution: () => Effect.succeed(null),
+  pausedExecutionCount: () => Effect.succeed(0),
+  hasPausedExecutions: () => Effect.succeed(false),
+  getDescription: Effect.succeed("stub"),
+};
+
+/** Counter that hands out a caller-controlled sequence of counts. */
+const countingIncrement = (counts: ReadonlyArray<number>) => {
+  let calls = 0;
+  return () => Effect.succeed(counts[Math.min(calls++, counts.length - 1)] ?? 0);
+};
+
+const runExecute = (limiter: ReturnType<typeof makeExecutionRateLimiter>) =>
+  Effect.runPromise(
+    limiter.decorate(ORG, engineStub).execute("code", {
+      // Never invoked: the stub ignores it, and a blocked execution never runs.
+      onElicitation: () => Effect.die("elicitation is not exercised here"),
+    }),
+  );
+
+describe("execution rate limiter — paid exemption", () => {
+  it("allows executions under the cap without consulting the exemption", async () => {
+    let exemptionCalls = 0;
+    const limiter = makeExecutionRateLimiter(countingIncrement([1]), {
+      limit: 10,
+      isExempt: () => {
+        exemptionCalls += 1;
+        return Effect.succeed(false);
+      },
+    });
+
+    expect(await runExecute(limiter)).toMatchObject({ result: "ran" });
+    // The whole point of resolving lazily: the common path costs no lookup.
+    expect(exemptionCalls).toBe(0);
+  });
+
+  it("blocks a non-exempt org over the cap", async () => {
+    const limiter = makeExecutionRateLimiter(countingIncrement([11]), {
+      limit: 10,
+      isExempt: () => Effect.succeed(false),
+    });
+
+    expect(await runExecute(limiter)).toMatchObject({
+      result: null,
+      error: RATE_LIMIT_BLOCKED_MESSAGE,
+    });
+  });
+
+  it("allows an exempt org over the cap", async () => {
+    const limiter = makeExecutionRateLimiter(countingIncrement([11]), {
+      limit: 10,
+      isExempt: () => Effect.succeed(true),
+    });
+
+    expect(await runExecute(limiter)).toMatchObject({ result: "ran" });
+  });
+
+  it("caches the exemption so a paid org past the cap looks it up once", async () => {
+    let exemptionCalls = 0;
+    const limiter = makeExecutionRateLimiter(countingIncrement([11, 12, 13]), {
+      limit: 10,
+      exemptionTtlMs: 60_000,
+      now: () => 1_000,
+      isExempt: () => {
+        exemptionCalls += 1;
+        return Effect.succeed(true);
+      },
+    });
+
+    for (let i = 0; i < 3; i += 1)
+      expect(await runExecute(limiter)).toMatchObject({ result: "ran" });
+    expect(exemptionCalls).toBe(1);
+  });
+
+  it("re-resolves once the cached exemption expires", async () => {
+    let exemptionCalls = 0;
+    let clock = 1_000;
+    const limiter = makeExecutionRateLimiter(countingIncrement([11, 12]), {
+      limit: 10,
+      exemptionTtlMs: 1_000,
+      now: () => clock,
+      isExempt: () => {
+        exemptionCalls += 1;
+        return Effect.succeed(true);
+      },
+    });
+
+    await runExecute(limiter);
+    clock += 5_000;
+    await runExecute(limiter);
+    expect(exemptionCalls).toBe(2);
+  });
+
+  it("blocks when the exemption cannot be resolved and nothing is cached", async () => {
+    // Deliberately NOT fail-open: an unresolvable exemption during an Autumn
+    // outage must not switch the backstop off, which is the one scenario it
+    // exists to cover.
+    const limiter = makeExecutionRateLimiter(countingIncrement([11]), {
+      limit: 10,
+      isExempt: () => Effect.fail(new UpstreamDownError({ which: "autumn" })),
+    });
+
+    expect(await runExecute(limiter)).toMatchObject({
+      result: null,
+      error: RATE_LIMIT_BLOCKED_MESSAGE,
+    });
+  });
+
+  it("honours a stale exemption when a later lookup fails", async () => {
+    let clock = 1_000;
+    let shouldFail = false;
+    const limiter = makeExecutionRateLimiter(countingIncrement([11, 12]), {
+      limit: 10,
+      exemptionTtlMs: 1_000,
+      now: () => clock,
+      isExempt: () =>
+        shouldFail ? Effect.fail(new UpstreamDownError({ which: "autumn" })) : Effect.succeed(true),
+    });
+
+    expect(await runExecute(limiter)).toMatchObject({ result: "ran" });
+
+    // Cache expires and Autumn is now unreachable: the known-paid org keeps
+    // running rather than getting blocked mid-workload by a blip.
+    clock += 5_000;
+    shouldFail = true;
+    expect(await runExecute(limiter)).toMatchObject({ result: "ran" });
+  });
+
+  it("fails open when the counter itself is unreachable", async () => {
+    const limiter = makeExecutionRateLimiter(
+      () => Effect.fail(new UpstreamDownError({ which: "counter DO" })),
+      {
+        limit: 10,
+        isExempt: () => Effect.succeed(false),
+      },
+    );
+
+    expect(await runExecute(limiter)).toMatchObject({ result: "ran" });
+  });
+
+  it("applies the cap when no exemption predicate is wired", async () => {
+    const limiter = makeExecutionRateLimiter(countingIncrement([11]), { limit: 10 });
+
+    expect(await runExecute(limiter)).toMatchObject({
+      result: null,
+      error: RATE_LIMIT_BLOCKED_MESSAGE,
+    });
+  });
+});