From 6dd93f934144dd994e73612fe5dabc6b45a22437 Mon Sep 17 00:00:00 2001 From: Rhys Sullivan <39114868+RhysSullivan@users.noreply.github.com> Date: Thu, 20 Aug 2026 09:46:21 -0700 Subject: [PATCH] Revalidate stale non-healthy connection verdicts on agent reads and successful invocations --- .changeset/health-verdict-revalidate.md | 11 ++ packages/core/sdk/src/connections.test.ts | 190 ++++++++++++++++++++++ packages/core/sdk/src/core-tools.ts | 61 ++++++- packages/core/sdk/src/executor.ts | 67 ++++++-- packages/core/sdk/src/health-check.ts | 9 + packages/core/sdk/src/plugin.ts | 11 ++ 6 files changed, 325 insertions(+), 24 deletions(-) create mode 100644 .changeset/health-verdict-revalidate.md diff --git a/.changeset/health-verdict-revalidate.md b/.changeset/health-verdict-revalidate.md new file mode 100644 index 0000000000..8d466e4972 --- /dev/null +++ b/.changeset/health-verdict-revalidate.md @@ -0,0 +1,11 @@ +--- +"@executor-js/sdk": patch +--- + +**Stale "unhealthy" verdicts no longer wait for a manual "Check now"** + +A connection's persisted health verdict was only ever re-checked from the web UI, so after one bad probe (a transient upstream error, a refresh that failed once) agents reading `connections.list` kept reporting "unhealthy, reconnect" for a connection that worked fine — invocation auto-refreshes OAuth tokens — until a human opened the page and clicked "Check now". + +Two repair paths make the verdict track reality on its own. The agent-facing `connections.list` now re-runs the same probe as "Check now" before reporting a non-healthy verdict older than a minute, so recovery shows on the next read while repeated lists collapse to one probe per window. And a successful tool invocation through a connection wearing a non-healthy verdict flips it back to healthy — real traffic is stronger evidence than any probe. Tool-sync failure verdicts and grants the authorization server has rejected as `invalid_grant` are deliberately left alone: the first is cleared only by a successful sync, and the second genuinely requires a reconnect. + +`PluginCtx.connections` gains `checkHealth`, the same probe-with-freshness-window the executor surface already exposed. diff --git a/packages/core/sdk/src/connections.test.ts b/packages/core/sdk/src/connections.test.ts index 7ed046f65b..274516ea67 100644 --- a/packages/core/sdk/src/connections.test.ts +++ b/packages/core/sdk/src/connections.test.ts @@ -14,6 +14,7 @@ import { createExecutor } from "./executor"; import { definePlugin } from "./plugin"; import type { CredentialProvider } from "./provider"; import { makeTestConfig, makeTestExecutor } from "./testing"; +import { ToolResult } from "./tool-result"; // removed: v1 connection-refresh lifecycle, ConnectionProvider.refresh, // SecretProvider, accessToken token-refresh + in-flight dedup tests — the v2 @@ -718,3 +719,192 @@ describe("execute over a connection", () => { }), ); }); + +// --------------------------------------------------------------------------- +// Sticky-verdict repair: agents read `lastHealth` through coreTools +// connections.list, and nothing else ever re-probes a persisted verdict, so a +// transient failure used to read as "unhealthy, reconnect" until a human +// clicked "Check now". These cover the two repair paths: read-time +// revalidation on the agent list, and heal-on-use from a successful +// invocation. +// --------------------------------------------------------------------------- + +const CORE_LIST = ToolAddress.make("executor.coreTools.connections.list"); +const STALE_MS = 5 * 60 * 1000; + +type ListedConnections = { + readonly connections: readonly { + readonly name: string; + readonly lastHealth: { readonly status: string; readonly detail?: string } | null; + }[]; +}; + +const makeHealthHarness = () => { + const counters = { probes: 0 }; + const plugin = definePlugin(() => ({ + id: "healthdemo" as const, + credentialProviders: [memoryProvider()], + storage: () => ({}), + resolveTools: () => + Effect.succeed({ tools: [{ name: ToolName.make("deploy"), description: "deploy" }] }), + invokeTool: ({ toolRow, credential, args }) => + Effect.succeed( + (args as { fail?: boolean }).fail === true + ? ToolResult.fail({ code: "upstream_error", message: "boom" }) + : { ran: toolRow.name, value: credential.value }, + ), + checkHealth: () => + Effect.sync(() => { + counters.probes += 1; + return { status: "healthy" as const, checkedAt: Date.now(), detail: "probe ok" }; + }), + extension: (ctx) => ({ + seed: () => + ctx.core.integrations.register({ slug: INTEG, description: "Vercel", config: {} }), + }), + }))(); + + return Effect.gen(function* () { + const config = makeTestConfig({ + plugins: [plugin] as const, + coreTools: { webBaseUrl: "http://localhost:3000" }, + }); + const executor = yield* createExecutor(config); + yield* executor.healthdemo.seed(); + yield* executor.connections.create({ + owner: "org", + name: ConnectionName.make("main"), + integration: INTEG, + template: TEMPLATE, + value: "secret-token", + }); + const stamp = (set: Record) => + Effect.promise(() => + config.db.updateMany("connection", { + where: (b) => b.and(b("integration", "=", String(INTEG)), b("name", "=", "main")), + set, + }), + ); + const persisted = () => + executor.connections.get({ + owner: "org", + integration: INTEG, + name: ConnectionName.make("main"), + }); + return { executor, counters, stamp, persisted } as const; + }); +}; + +describe("agent read revalidation (coreTools connections.list)", () => { + it.effect("re-probes a stale non-healthy verdict and reports + persists the fresh one", () => + Effect.gen(function* () { + const { executor, counters, stamp, persisted } = yield* makeHealthHarness(); + yield* stamp({ + last_health: { status: "expired", checkedAt: Date.now() - STALE_MS, detail: "HTTP 401" }, + }); + + const out = (yield* executor.execute(CORE_LIST, {})) as ListedConnections; + const listed = out.connections.find((c) => c.name === "main"); + expect(listed?.lastHealth?.status).toBe("healthy"); + expect(counters.probes).toBe(1); + + const row = yield* persisted(); + expect(row?.lastHealth?.status).toBe("healthy"); + }), + ); + + it.effect("serves a fresh non-healthy verdict without re-probing", () => + Effect.gen(function* () { + const { executor, counters, stamp } = yield* makeHealthHarness(); + yield* stamp({ + last_health: { status: "expired", checkedAt: Date.now(), detail: "HTTP 401" }, + }); + + const out = (yield* executor.execute(CORE_LIST, {})) as ListedConnections; + const listed = out.connections.find((c) => c.name === "main"); + expect(listed?.lastHealth?.status).toBe("expired"); + expect(counters.probes).toBe(0); + }), + ); + + it.effect("never probes a healthy verdict, however old", () => + Effect.gen(function* () { + const { executor, counters, stamp } = yield* makeHealthHarness(); + yield* stamp({ + last_health: { status: "healthy", checkedAt: Date.now() - STALE_MS, detail: "probe ok" }, + }); + + const out = (yield* executor.execute(CORE_LIST, {})) as ListedConnections; + const listed = out.connections.find((c) => c.name === "main"); + expect(listed?.lastHealth?.status).toBe("healthy"); + expect(counters.probes).toBe(0); + }), + ); + + it.effect("leaves tool-sync failure verdicts for sync to clear", () => + Effect.gen(function* () { + const { executor, counters, stamp } = yield* makeHealthHarness(); + const detail = "Tool sync failing: plugin returned an incomplete tool catalog"; + yield* stamp({ + last_health: { status: "degraded", checkedAt: Date.now() - STALE_MS, detail }, + }); + + const out = (yield* executor.execute(CORE_LIST, {})) as ListedConnections; + const listed = out.connections.find((c) => c.name === "main"); + expect(listed?.lastHealth?.detail).toBe(detail); + expect(counters.probes).toBe(0); + }), + ); +}); + +describe("heal-on-use", () => { + it.effect("a successful invocation flips a stale non-healthy verdict to healthy", () => + Effect.gen(function* () { + const { executor, stamp, persisted } = yield* makeHealthHarness(); + yield* stamp({ + last_health: { status: "expired", checkedAt: Date.now() - STALE_MS, detail: "HTTP 401" }, + }); + + yield* executor.execute(ToolAddress.make("tools.vercel.org.main.deploy"), {}); + + const row = yield* persisted(); + expect(row?.lastHealth).toMatchObject({ + status: "healthy", + detail: "Tool invocation succeeded.", + }); + }), + ); + + it.effect("an explicit tool failure does not heal", () => + Effect.gen(function* () { + const { executor, stamp, persisted } = yield* makeHealthHarness(); + yield* stamp({ + last_health: { status: "expired", checkedAt: Date.now() - STALE_MS, detail: "HTTP 401" }, + }); + + yield* executor.execute(ToolAddress.make("tools.vercel.org.main.deploy"), { fail: true }); + + const row = yield* persisted(); + expect(row?.lastHealth?.status).toBe("expired"); + }), + ); + + it.effect("a grant recorded invalid_grant-dead is not healed by a lingering token", () => + Effect.gen(function* () { + const { executor, stamp, persisted } = yield* makeHealthHarness(); + yield* stamp({ + provider_state: { oauthReauthRequiredAt: Date.now() }, + last_health: { + status: "expired", + checkedAt: Date.now() - STALE_MS, + detail: "invalid_grant", + }, + }); + + yield* executor.execute(ToolAddress.make("tools.vercel.org.main.deploy"), {}); + + const row = yield* persisted(); + expect(row?.lastHealth?.status).toBe("expired"); + }), + ); +}); diff --git a/packages/core/sdk/src/core-tools.ts b/packages/core/sdk/src/core-tools.ts index e6dc78ce77..f55a106f93 100644 --- a/packages/core/sdk/src/core-tools.ts +++ b/packages/core/sdk/src/core-tools.ts @@ -22,7 +22,7 @@ import { type Owner, } from "./ids"; import { definePlugin, tool, type StaticToolSchema } from "./plugin"; -import { HealthCheckResult } from "./health-check"; +import { HealthCheckResult, isToolSyncHealth } from "./health-check"; import { ToolPolicyActionSchema } from "./policies"; import type { Tool } from "./tool"; @@ -408,6 +408,26 @@ const connectionToListItem = (connection: Connection, verbose: boolean) => ({ ...(verbose ? { oauthScope: connection.oauthScope ?? null } : {}), }); +/** How long a non-healthy persisted verdict may be served to an agent before + * it is re-verified. Verdicts are sticky — nothing re-probes them between UI + * visits — so without read-time revalidation an agent keeps reporting + * "unhealthy, reconnect" for a connection that recovered long ago (or was + * never really down: invocation auto-refreshes OAuth tokens, so a stale + * "expired" verdict often describes a working connection). The window is + * short so recovery shows on the next read, but bounds repeated lists from + * hammering a genuinely-down upstream. Healthy verdicts are deliberately + * served as-is: they mislead no one into reconnect guidance, and the UI + * owns their background revalidation. */ +const NON_HEALTHY_REVALIDATE_MS = 60 * 1000; + +/** Whether an agent read must re-verify a persisted verdict before reporting + * it. Only probe-refutable non-healthy verdicts qualify: `unknown` and + * missing verdicts carry no reconnect implication, and a tool-sync failure + * verdict cannot be refuted by a credential probe (a successful sync clears + * it instead). */ +const needsAgentReadRevalidation = (last: HealthCheckResult | null | undefined): boolean => + (last?.status === "expired" || last?.status === "degraded") && !isToolSyncHealth(last); + const toolToOutput = (toolRow: Tool) => ({ address: String(toolRow.address), owner: toolRow.owner, @@ -593,20 +613,45 @@ export const coreToolsPlugin = definePlugin((options: CoreToolsPluginOptions = { inputSchema: ConnectionsListInputStd, outputSchema: ConnectionsListOutputStd, execute: (input: typeof ConnectionsListInput.Type, { ctx }) => - Effect.map( - ctx.connections.list({ + Effect.gen(function* () { + const connections = yield* ctx.connections.list({ integration: input.integration === undefined ? undefined : IntegrationSlug.make(input.integration), owner: input.owner === undefined ? undefined : (input.owner as Owner), - }), - (connections) => ({ - connections: connections.map((connection) => + }); + // Re-verify sticky non-healthy verdicts before reporting them + // (the same probe as the UI's "Check now", server-cached via + // `ifStaleMs` so repeated lists collapse to one probe per + // window). Quiet on probe failure: the persisted verdict is + // still the best known state, exactly like the UI surfaces. + const revalidated = yield* Effect.forEach( + connections, + (connection) => + needsAgentReadRevalidation(connection.lastHealth) + ? ctx.connections + .checkHealth( + { + owner: connection.owner, + integration: connection.integration, + name: connection.name, + }, + { ifStaleMs: NON_HEALTHY_REVALIDATE_MS }, + ) + .pipe( + Effect.map((health) => ({ ...connection, lastHealth: health })), + Effect.catch(() => Effect.succeed(connection)), + ) + : Effect.succeed(connection), + { concurrency: 4 }, + ); + return { + connections: revalidated.map((connection) => connectionToListItem(connection, input.verbose === true), ), - }), - ), + }; + }), }), tool({ name: "connections.create", diff --git a/packages/core/sdk/src/executor.ts b/packages/core/sdk/src/executor.ts index 04c742063d..eec91a99ec 100644 --- a/packages/core/sdk/src/executor.ts +++ b/packages/core/sdk/src/executor.ts @@ -27,7 +27,12 @@ import type { UpdateConnectionInput, ValidateConnectionInput, } from "./connection"; -import { HealthCheckResult, HealthCheckSpec } from "./health-check"; +import { + HealthCheckResult, + HealthCheckSpec, + isToolSyncHealth, + toolSyncHealthDetailPrefix, +} from "./health-check"; import type { HealthCheckCandidate } from "./health-check"; import { ARTIFACT_SUMMARY_COLUMNS, @@ -168,7 +173,7 @@ import { type OAuthEndpointUrlPolicy, } from "./oauth-helpers"; import { connectionIdentifier } from "./connection-name-identifier"; -import { annotateToolResultOutcome } from "./tool-result"; +import { annotateToolResultOutcome, isToolResult } from "./tool-result"; import { isUnauthorizedToolFailure } from "./auth-tool-failure"; const PLUGIN_STORAGE_DELETE_KEY_BATCH_SIZE = 90; @@ -2541,8 +2546,6 @@ export const createExecutor = ({ status: "degraded", checkedAt: Date.now(), @@ -2576,8 +2579,6 @@ export const createExecutor = => @@ -4541,16 +4571,20 @@ export const createExecutor = Effect.succeed(null)), - ); - if (!refreshed) return first; - yield* Effect.annotateCurrentSpan({ "executor.oauth.refresh.retried": true }); - return yield* invokeWith(refreshed); + const result = yield* Effect.gen(function* () { + if (!isUnauthorizedToolFailure(first)) return first; + const refreshed = yield* forceRefreshConnectionValues(connectionRow).pipe( + // A failed re-mint is not this call's failure to report: the upstream + // already produced an auth failure with recovery guidance, which is + // strictly more actionable than a refresh-plumbing error. Keep it. + Effect.catchTag("CredentialResolutionError", () => Effect.succeed(null)), + ); + if (!refreshed) return first; + yield* Effect.annotateCurrentSpan({ "executor.oauth.refresh.retried": true }); + return yield* invokeWith(refreshed); + }); + yield* healPersistedHealthOnUse(connectionRow, result); + return result; }).pipe( // Expected tool failures (`ToolResult.fail`) resolve through the // success channel, so the tracer alone would record them as healthy @@ -4709,6 +4743,7 @@ export const createExecutor = connectionsUpdate(ref, input), remove: (ref) => connectionsRemove(ref), refresh: (ref) => connectionsRefresh(ref), + checkHealth: (ref, options) => connectionCheckHealth(ref, options), markToolsStale: (ref) => connectionsMarkToolsStale(ref), resolveValue: (ref) => resolveConnectionValueByRef(ref), }, diff --git a/packages/core/sdk/src/health-check.ts b/packages/core/sdk/src/health-check.ts index d553f994fd..572a4b2074 100644 --- a/packages/core/sdk/src/health-check.ts +++ b/packages/core/sdk/src/health-check.ts @@ -84,6 +84,15 @@ export const HealthCheckResult = Schema.Struct({ }); export type HealthCheckResult = typeof HealthCheckResult.Type; +/** Detail prefix that marks a verdict as produced by tool-catalog sync, not a + * credential probe. Shared vocabulary: sync stamps it, and the surfaces that + * auto-revalidate verdicts skip these — a credential probe cannot refute a + * failed tool sync, and a later successful sync clears the verdict itself. */ +export const toolSyncHealthDetailPrefix = "Tool sync failing"; + +export const isToolSyncHealth = (result: HealthCheckResult | null | undefined): boolean => + result?.detail?.startsWith(toolSyncHealthDetailPrefix) === true; + // --------------------------------------------------------------------------- // HealthCheckCandidate: one operation the user can pick as the health check, // projected from the plugin's stored operations. The editor lists these ranked diff --git a/packages/core/sdk/src/plugin.ts b/packages/core/sdk/src/plugin.ts index 6d4dbc23b2..80bb847647 100644 --- a/packages/core/sdk/src/plugin.ts +++ b/packages/core/sdk/src/plugin.ts @@ -231,6 +231,17 @@ export interface PluginCtx { readonly Tool[], ConnectionNotFoundError | IntegrationNotFoundError | StorageFailure >; + /** Run the integration's declared health check against a saved connection + * and persist the verdict. `ifStaleMs` serves the persisted verdict when + * younger than that window, so concurrent readers collapse to one probe; + * omit it to always probe. */ + readonly checkHealth: ( + ref: ConnectionRef, + options?: { readonly ifStaleMs?: number }, + ) => Effect.Effect< + HealthCheckResult, + ConnectionNotFoundError | IntegrationNotFoundError | StorageFailure + >; /** Mark a connection's persisted tool catalog stale (clears its sync * stamp) without re-listing inline. The next tools read re-produces it. * For signals that arrive mid-invocation — e.g. an MCP server sending