Fix the wrong Expired status and the refresh races behind it

.changeset/oauth-refresh-evidence.md

@@ -0,0 +1,18 @@
+---
+"@executor-js/sdk": patch
+"@executor-js/plugin-mcp": patch
+"@executor-js/plugin-openapi": patch
+"@executor-js/plugin-graphql": patch
+---
+
+Stop recording a permanent **Expired** verdict without evidence, and stop the refresh races that produced one.
+
+A refresher whose grant is refused now reads the stored refresh token again; when a peer instance rotated it during the request, the call adopts the access token that peer persisted and records nothing. Previously the loser of a concurrent refresh wrote `oauthReauthRequiredAt` onto a connection that still held a valid rotated refresh token, and every surface then answered `expired` without probing — no tool call could refresh it again, only a re-authorization recovered it. The record write is also skipped when `expires_at` moved forward during the grant, which is the same peer success read from the row.
+
+`isPermanentTokenRejection` no longer reads 408, 425, or 429 as a definitive refusal, so one rate-limited minute at a token endpoint no longer ends a grant; those statuses behave like a 5xx and the next call retries. A refresh response that omits `expires_in` (RFC 6749 makes it optional) no longer erases `expires_at`: the mint records the advertised lifetime in `provider_state.oauthTokenLifetimeMs` and a refresh derives the expiry from it, instead of disabling proactive refresh for the rest of the connection's life.
+
+`connections.checkHealth` re-mints once and probes again before it answers `expired` for an OAuth connection, so a revoked token, an idle timeout shorter than the advertised lifetime, or a null `expires_at` no longer shows a working connection as dead until a tool call heals it. The plugin is asked first with or without a declared health-check spec, so a plugin whose probe needs no spec (MCP lists tools) gives its OAuth connections a real verdict; only a plugin that answers `unknown` falls back to the credential-only verdict, and that verdict now reports `expired` when a credential value resolves to nothing.
+
+The MCP liveness probe takes the invocation pool's lease instead of dialling a second connection, bounded by the shared 15s discovery deadline; a probe of a stdio server no longer starts a second child process, which single-instance servers (Chrome DevTools MCP, Playwright MCP, `docker run -i`) refused — reporting a live, serving connection as broken on every page mount. A 403 scope shortfall on a probe reads `degraded` instead of `expired`, from either an RFC 6750 `WWW-Authenticate` challenge or a body marker. The GraphQL probe no longer reads a transport failure's prose as a dead credential (`connect EACCES: permission denied` on a socket is not an authentication verdict).
+
+The test authorization server's `/mcp` resource endpoint now speaks JSON-RPC honestly — the request's own id, an empty catalog for `tools/list`, silence for notifications — where the old canned reply used a fixed id and left every completed handshake waiting forever for its `tools/list` response.

packages/core/sdk/src/connections.test.ts

@@ -2575,6 +2575,11 @@ const makeHealthHarness = (options?: {
    *  `counters.probes`, so a Deferred-gated probe lets a test hold every
    *  in-flight health check open and count how many actually started. */
   readonly probe?: Effect.Effect<typeof HealthCheckResult.Type, unknown>;
+  /** Answer `unknown` when core passes no declared spec, the way the protocol
+   *  plugins do: they have no operation to dial, so core falls back to the
+   *  credential-only verdict. Without this the harness plugin answers every
+   *  check, and the fallback is unreachable. */
+  readonly declineWithoutSpec?: boolean;
 }) => {
   const counters = { probes: 0, resolves: 0 };
   const hooks = {
@@ -2660,9 +2665,16 @@ const makeHealthHarness = (options?: {
           ? ToolResult.fail({ code: "upstream_error", message: "boom" })
           : { ran: toolRow.name, value: credential.value },
       ),
-    checkHealth: () =>
+    checkHealth: ({ spec }) =>
       Effect.suspend(() => {
         counters.probes += 1;
+        if (options?.declineWithoutSpec === true && spec === undefined) {
+          return Effect.succeed({
+            status: "unknown" as const,
+            checkedAt: Date.now(),
+            detail: "No health check configured.",
+          });
+        }
         return (
           options?.probe ??
           Effect.succeed({ status: "healthy" as const, checkedAt: Date.now(), detail: "probe ok" })
@@ -3298,12 +3310,15 @@ describe("credential-only health path", () => {
       // parallel suite load the forked checks may not have finished their row
       // loads yet, and the counter reads 0.
       const entered = yield* Deferred.make<void>();
-      const { executor, counters, stamp, persisted, hooks } = yield* makeHealthHarness();
-      // No declared probe spec + an OAuth client on the row routes checkHealth
-      // down the credential-only path: the verdict is "the credential
-      // resolved", produced without invoking the plugin probe. That path runs
-      // behind the same in-flight gate as probing, so concurrent checks must
-      // collapse to ONE resolution.
+      const { executor, counters, stamp, persisted, hooks } = yield* makeHealthHarness({
+        declineWithoutSpec: true,
+      });
+      // No declared probe spec + an OAuth client on the row: the plugin is
+      // asked first, declines for want of an operation to dial, and core falls
+      // back to the credential-only verdict — "the credential resolved",
+      // produced from the SAME resolution the plugin's probe used, so nothing
+      // refreshes twice. That path runs behind the same in-flight gate as
+      // probing, so concurrent checks must collapse to ONE resolution.
       yield* stamp({ oauth_client: "acme", expires_at: null });
       hooks.onResolve = Deferred.succeed(entered, void 0).pipe(
         Effect.andThen(Deferred.await(gate)),
@@ -3329,7 +3344,10 @@ describe("credential-only health path", () => {
       expect(first.status).toBe("healthy");
       expect(second.status).toBe("healthy");
       expect(counters.resolves).toBe(1);
-      expect(counters.probes).toBe(0);
+      // Both checks shared ONE gate entry, so the plugin was asked once and
+      // declined once; the verdict both callers received is the fallback.
+      expect(counters.probes).toBe(1);
+      expect(first.detail).toBe("Credential resolved (no probe configured).");
 
       const row = yield* persisted();
       expect(row?.lastHealth?.status).toBe("healthy");