From c84836d1794d884033da65cae50e311ca209436e Mon Sep 17 00:00:00 2001 From: Rhys Sullivan <39114868+RhysSullivan@users.noreply.github.com> Date: Mon, 31 Aug 2026 22:23:52 -0700 Subject: [PATCH 1/4] Stop rendering tool-sync failures as connection health --- packages/core/sdk/src/index.ts | 1 + packages/core/sdk/src/shared.ts | 1 + .../react/src/components/accounts-section.tsx | 12 +++++ .../src/lib/use-connection-health.test.ts | 54 ++++++++++++++++++- .../react/src/lib/use-connection-health.ts | 23 +++++++- 5 files changed, 88 insertions(+), 3 deletions(-) diff --git a/packages/core/sdk/src/index.ts b/packages/core/sdk/src/index.ts index 95fe72e8f3..ff3a27a180 100644 --- a/packages/core/sdk/src/index.ts +++ b/packages/core/sdk/src/index.ts @@ -134,6 +134,7 @@ export { pathNamesASecret, REDACTED_SAMPLE_VALUE, identityPathTier, + isToolSyncHealth, rankResponseSample, } from "./health-check"; diff --git a/packages/core/sdk/src/shared.ts b/packages/core/sdk/src/shared.ts index 86b6cd7d25..6b02d3e4f4 100644 --- a/packages/core/sdk/src/shared.ts +++ b/packages/core/sdk/src/shared.ts @@ -143,6 +143,7 @@ export { candidateIdentityTier, sortHealthCheckCandidatesByIdentity, identityPathTier, + isToolSyncHealth, rankResponseSample, } from "./health-check"; diff --git a/packages/react/src/components/accounts-section.tsx b/packages/react/src/components/accounts-section.tsx index b79a3c0c13..9362a4908e 100644 --- a/packages/react/src/components/accounts-section.tsx +++ b/packages/react/src/components/accounts-section.tsx @@ -4,6 +4,7 @@ import * as AsyncResult from "effect/unstable/reactivity/AsyncResult"; import * as Exit from "effect/Exit"; import { IntegrationSlug, + isToolSyncHealth, type Connection, type OAuthClientSummary, type Owner, @@ -172,6 +173,12 @@ function AccountRow(props: { const misconfigured = status === "misconfigured"; const needsHealthAttention = status === "expired" || status === "degraded"; const healthDetail = needsHealthAttention ? probe?.detail : undefined; + // A "Tool sync failing" stamp is about the CATALOG (tools may be stale or + // missing), not the credential — the health hook excludes it from `status`, + // so surface it as its own muted note instead of an unhealthy row. + const syncFailureDetail = isToolSyncHealth(connection.lastHealth) + ? connection.lastHealth?.detail + : undefined; const missingOAuthScopes = connection.missingOAuthScopes ?? []; const handleCheck = async () => { @@ -252,6 +259,11 @@ function AccountRow(props: { {healthDetail} ) : null} + {syncFailureDetail ? ( + + {syncFailureDetail} + + ) : null} {needsReconsent ? ( This connection wasn't granted all the access this integration now needs. diff --git a/packages/react/src/lib/use-connection-health.test.ts b/packages/react/src/lib/use-connection-health.test.ts index 89e5d08405..ad7f5080fa 100644 --- a/packages/react/src/lib/use-connection-health.test.ts +++ b/packages/react/src/lib/use-connection-health.test.ts @@ -1,7 +1,7 @@ import { describe, expect, it } from "@effect/vitest"; import type { HealthCheckResult } from "@executor-js/sdk/shared"; -import { HEALTH_REVALIDATE_MS, revalidateQuery } from "./use-connection-health"; +import { HEALTH_REVALIDATE_MS, presentableHealth, revalidateQuery } from "./use-connection-health"; const verdict = (status: HealthCheckResult["status"]): HealthCheckResult => ({ status, @@ -46,3 +46,55 @@ describe("revalidateQuery", () => { expect(windows, "only the healthy path is gated").toEqual([undefined, undefined, undefined]); }); }); + +// --------------------------------------------------------------------------- +// Tool-sync stamps are not connection health. `toolSyncHealth` writes a +// degraded verdict with the "Tool sync failing" detail prefix into +// `last_health` when catalog production fails; presenting that as the +// connection's health painted whole integration rows "Degraded" (one bad +// sweep stamps many connections at once) for credentials that were fine. +// --------------------------------------------------------------------------- + +describe("presentableHealth", () => { + it("passes genuine probe verdicts through untouched", () => { + const expired: HealthCheckResult = { + status: "expired", + checkedAt: Date.now(), + detail: "HTTP 401", + }; + expect(presentableHealth(expired), "a probe verdict presents as-is").toBe(expired); + }); + + it("hides a tool-sync stamp, old (detail-only) and new (reason) alike", () => { + const stamped: HealthCheckResult = { + status: "degraded", + checkedAt: Date.now(), + detail: "Tool sync failing: upstream returned HTTP 429", + reason: "tool_sync_failed", + }; + // Stamps written before `reason` existed carry only the detail prefix. + const legacy: HealthCheckResult = { + status: "degraded", + checkedAt: Date.now(), + detail: "Tool sync failing: plugin returned an incomplete tool catalog", + }; + expect(presentableHealth(stamped), "a sync stamp is not connection health").toBeNull(); + expect(presentableHealth(legacy), "pre-reason stamps hide the same way").toBeNull(); + }); + + it("treats missing verdicts as missing", () => { + expect(presentableHealth(null)).toBeNull(); + expect(presentableHealth(undefined)).toBeNull(); + }); + + it("does not hide a probe verdict whose upstream text merely mentions syncing", () => { + // The marker is the detail PREFIX, owned by the sync stamp writer — an + // upstream error that contains similar words elsewhere stays visible. + const probeVerdict: HealthCheckResult = { + status: "degraded", + checkedAt: Date.now(), + detail: "Health check request failed: upstream sync service unavailable", + }; + expect(presentableHealth(probeVerdict)).toBe(probeVerdict); + }); +}); diff --git a/packages/react/src/lib/use-connection-health.ts b/packages/react/src/lib/use-connection-health.ts index 9bfc1fe36a..ca43f6e570 100644 --- a/packages/react/src/lib/use-connection-health.ts +++ b/packages/react/src/lib/use-connection-health.ts @@ -9,6 +9,7 @@ import { useCallback, useContext, useEffect, useRef, useState } from "react"; import { RegistryContext, useAtomSet } from "@effect/atom-react"; import * as Exit from "effect/Exit"; +import { isToolSyncHealth } from "@executor-js/sdk/shared"; import type { Connection, HealthCheckResult, HealthStatus, Owner } from "@executor-js/sdk/shared"; import { checkConnectionHealth, connectionsOptimisticAtom } from "../api/atoms"; @@ -54,6 +55,17 @@ export const revalidateQuery = ( ): { readonly ifStaleMs?: number } => last?.status === "healthy" ? { ifStaleMs: HEALTH_REVALIDATE_MS } : {}; +/** The verdict a HEALTH surface may present, or null when the persisted + * verdict is a tool-sync stamp (`isToolSyncHealth`). A "Tool sync failing" + * verdict describes the CATALOG — tools may be stale or missing — not the + * credential, so rendering it as the connection's health painted whole rows + * "Degraded" (an entire integrations list, when one bad sweep stamped many + * connections at once) for connections whose credentials were fine. + * Surfaces show the stamp as its own muted note instead. */ +export const presentableHealth = ( + last: HealthCheckResult | null | undefined, +): HealthCheckResult | null => (last == null || isToolSyncHealth(last) ? null : last); + /** Identity of a persisted verdict, for detecting the reconnect transition. * An OAuth re-mint clears `last_health`, so a verdict giving way to `null` * means the grant was replaced and the row must re-probe even though it never @@ -115,7 +127,7 @@ export function useConnectionHealth(connection: Connection): { const doCheck = useAtomSet(checkConnectionHealth, { mode: "promiseExit" }); const invalidateConnections = useInvalidateConnections(); - const probe = freshestVerdict(liveProbe, connection.lastHealth); + const probe = freshestVerdict(liveProbe, presentableHealth(connection.lastHealth)); const status: HealthStatus = probe?.status ?? "unknown"; // Health checks are AUTOMATIC: loading the list revalidates any verdict @@ -137,6 +149,10 @@ export function useConnectionHealth(connection: Connection): { seenEpoch.current = epoch; if (!firstSight && !cleared) return; if (healthyAndFresh(last)) return; + // A tool-sync stamp still PROBES (it is non-healthy, and the probe is + // what discovers the credential's real state — a never-probed connection + // whose first sync failed must still surface Expired); it just never + // RENDERS as connection health (see `presentableHealth`). void doCheck({ params: connectionParams(connection), query: revalidateQuery(last), @@ -226,7 +242,10 @@ export function useConnectionsHealth( return useCallback( (connection: Connection) => - freshestVerdict(liveProbes.get(probeKey(connection)) ?? null, connection.lastHealth), + freshestVerdict( + liveProbes.get(probeKey(connection)) ?? null, + presentableHealth(connection.lastHealth), + ), [liveProbes], ); } From 09ce8135fc0208851d904f549bf1c2c2e1fe2540 Mon Sep 17 00:00:00 2001 From: Rhys Sullivan <39114868+RhysSullivan@users.noreply.github.com> Date: Mon, 31 Aug 2026 22:37:38 -0700 Subject: [PATCH 2/4] Carry the health reason to the admin plane so sync stamps read unchecked --- packages/core/api/src/admin/api.ts | 13 +++++++++- packages/core/api/src/admin/reads.ts | 13 +++++++--- .../react/src/lib/admin-users-display.test.ts | 18 +++++++++++++ packages/react/src/lib/admin-users-display.ts | 25 +++++++++++++++---- 4 files changed, 60 insertions(+), 9 deletions(-) diff --git a/packages/core/api/src/admin/api.ts b/packages/core/api/src/admin/api.ts index 69e76db94d..7b61df3cb2 100644 --- a/packages/core/api/src/admin/api.ts +++ b/packages/core/api/src/admin/api.ts @@ -36,7 +36,13 @@ import { HttpApi, HttpApiEndpoint, HttpApiGroup } from "effect/unstable/httpapi"; import { Schema } from "effect"; -import { ConnectionName, HealthStatus, IntegrationSlug, Owner } from "@executor-js/sdk/shared"; +import { + ConnectionName, + HealthCheckReason, + HealthStatus, + IntegrationSlug, + Owner, +} from "@executor-js/sdk/shared"; // --------------------------------------------------------------------------- // Errors @@ -150,6 +156,11 @@ export const AdminConnectionHealth = Schema.Struct({ /** Epoch ms the check ran, so an operator can tell a fresh verdict from a * stale one. */ checkedAt: Schema.Number, + /** Enumerable failure mechanism (`HealthCheckReason`). Carried so the view + * can tell a tool-sync stamp (`tool_sync_failed` — catalog state, not + * credential health) from a genuine probe verdict without the free-text + * `detail` this plane deliberately strips. */ + reason: Schema.optional(HealthCheckReason), }); /** diff --git a/packages/core/api/src/admin/reads.ts b/packages/core/api/src/admin/reads.ts index 96b6f2bbdc..627776de7a 100644 --- a/packages/core/api/src/admin/reads.ts +++ b/packages/core/api/src/admin/reads.ts @@ -183,12 +183,19 @@ const toUser = (subject: AdminSubject, identities: ReadonlyMap - health === null ? null : { status: health.status, checkedAt: health.checkedAt }; + health === null + ? null + : { + status: health.status, + checkedAt: health.checkedAt, + ...(health.reason !== undefined ? { reason: health.reason } : {}), + }; /** * `AdminConnection` → the public `AdminUserConnection` shape. diff --git a/packages/react/src/lib/admin-users-display.test.ts b/packages/react/src/lib/admin-users-display.test.ts index fad73a3396..23c5767774 100644 --- a/packages/react/src/lib/admin-users-display.test.ts +++ b/packages/react/src/lib/admin-users-display.test.ts @@ -174,6 +174,24 @@ describe("connectionHealthStatus", () => { it("a never-probed connection is unchecked, not healthy", () => { expect(connectionHealthStatus(connection({ lastHealth: null }))).toBe("unknown"); }); + + it("a tool-sync stamp is catalog state, not a degraded connection", () => { + // Same rule as the console's `presentableHealth`: one failed sync sweep + // must not paint an operator's view of a user's connections amber. + expect( + connectionHealthStatus( + connection({ lastHealth: { status: "degraded", reason: "tool_sync_failed" } }), + ), + ).toBe("unknown"); + }); + + it("a genuine degraded probe verdict still reads degraded", () => { + expect( + connectionHealthStatus( + connection({ lastHealth: { status: "degraded", reason: "probe_timeout" } }), + ), + ).toBe("degraded"); + }); }); describe("isConnectableIntegration", () => { diff --git a/packages/react/src/lib/admin-users-display.ts b/packages/react/src/lib/admin-users-display.ts index 41ba535640..4091df0cf8 100644 --- a/packages/react/src/lib/admin-users-display.ts +++ b/packages/react/src/lib/admin-users-display.ts @@ -1,4 +1,9 @@ -import type { HealthStatus, IntegrationSlug, Owner } from "@executor-js/sdk/shared"; +import type { + HealthCheckReason, + HealthStatus, + IntegrationSlug, + Owner, +} from "@executor-js/sdk/shared"; // --------------------------------------------------------------------------- // Pure display logic for the admin Users section. @@ -15,7 +20,10 @@ export interface AdminConnectionRow { readonly integration: IntegrationSlug; readonly name: string; readonly oauthScope: string | null; - readonly lastHealth: { readonly status: HealthStatus } | null; + readonly lastHealth: { + readonly status: HealthStatus; + readonly reason?: HealthCheckReason; + } | null; } // ── Last seen ─────────────────────────────────────────────────────────────── @@ -142,9 +150,16 @@ export const adminUserCopyableEmail = (user: AdminUserIdentityRow): string | nul // ── Connections ───────────────────────────────────────────────────────────── /** The health status a row displays. A connection that was never probed carries - * no verdict, which is `unknown` in the shared vocabulary. */ -export const connectionHealthStatus = (connection: AdminConnectionRow): HealthStatus => - connection.lastHealth?.status ?? "unknown"; + * no verdict, which is `unknown` in the shared vocabulary. A tool-sync stamp + * (`reason: "tool_sync_failed"`) is catalog state, not credential health — + * the same rule the console's `presentableHealth` applies — so it reads + * `unknown` here rather than painting the row degraded. */ +export const connectionHealthStatus = (connection: AdminConnectionRow): HealthStatus => { + const last = connection.lastHealth; + if (last === null) return "unknown"; + if (last.reason === "tool_sync_failed") return "unknown"; + return last.status; +}; /** The catalog row this view needs: the slug it marks, plus the `kind` that * says whether connecting is even a thing one can do to it. Structural so From 93405fd326674b68c1d8a5bc2402d075b2c73f08 Mon Sep 17 00:00:00 2001 From: Rhys Sullivan <39114868+RhysSullivan@users.noreply.github.com> Date: Mon, 31 Aug 2026 22:52:08 -0700 Subject: [PATCH 3/4] Keep sync stamps as the Tools tab's catalog signal; tolerant admin reason --- packages/core/api/src/admin/api.ts | 22 +++++++++---------- packages/react/src/lib/admin-users-display.ts | 12 +++++----- .../react/src/pages/integration-detail.tsx | 13 ++++++++++- 3 files changed, 27 insertions(+), 20 deletions(-) diff --git a/packages/core/api/src/admin/api.ts b/packages/core/api/src/admin/api.ts index 7b61df3cb2..534fbeff67 100644 --- a/packages/core/api/src/admin/api.ts +++ b/packages/core/api/src/admin/api.ts @@ -36,13 +36,7 @@ import { HttpApi, HttpApiEndpoint, HttpApiGroup } from "effect/unstable/httpapi"; import { Schema } from "effect"; -import { - ConnectionName, - HealthCheckReason, - HealthStatus, - IntegrationSlug, - Owner, -} from "@executor-js/sdk/shared"; +import { ConnectionName, HealthStatus, IntegrationSlug, Owner } from "@executor-js/sdk/shared"; // --------------------------------------------------------------------------- // Errors @@ -156,11 +150,15 @@ export const AdminConnectionHealth = Schema.Struct({ /** Epoch ms the check ran, so an operator can tell a fresh verdict from a * stale one. */ checkedAt: Schema.Number, - /** Enumerable failure mechanism (`HealthCheckReason`). Carried so the view - * can tell a tool-sync stamp (`tool_sync_failed` — catalog state, not - * credential health) from a genuine probe verdict without the free-text - * `detail` this plane deliberately strips. */ - reason: Schema.optional(HealthCheckReason), + /** Enumerable failure mechanism — a `HealthCheckReason` literal. Carried so + * the view can tell a tool-sync stamp (`tool_sync_failed` — catalog state, + * not credential health) from a genuine probe verdict without the free-text + * `detail` this plane deliberately strips. Typed as a plain string ON + * PURPOSE: the reason enum grows, and a closed set here would make a client + * built today fail the WHOLE admin response the first time one connection + * carries a literal it doesn't know. Clients treat unrecognized values as + * unclassified. */ + reason: Schema.optional(Schema.String), }); /** diff --git a/packages/react/src/lib/admin-users-display.ts b/packages/react/src/lib/admin-users-display.ts index 4091df0cf8..8a5e0bc59c 100644 --- a/packages/react/src/lib/admin-users-display.ts +++ b/packages/react/src/lib/admin-users-display.ts @@ -1,9 +1,4 @@ -import type { - HealthCheckReason, - HealthStatus, - IntegrationSlug, - Owner, -} from "@executor-js/sdk/shared"; +import type { HealthStatus, IntegrationSlug, Owner } from "@executor-js/sdk/shared"; // --------------------------------------------------------------------------- // Pure display logic for the admin Users section. @@ -22,7 +17,10 @@ export interface AdminConnectionRow { readonly oauthScope: string | null; readonly lastHealth: { readonly status: HealthStatus; - readonly reason?: HealthCheckReason; + /** A `HealthCheckReason` literal, tolerant of future additions (the admin + * wire types it as a plain string); unrecognized values read as + * unclassified. */ + readonly reason?: string; } | null; } diff --git a/packages/react/src/pages/integration-detail.tsx b/packages/react/src/pages/integration-detail.tsx index 0f2c0d1bd2..e67fe93514 100644 --- a/packages/react/src/pages/integration-detail.tsx +++ b/packages/react/src/pages/integration-detail.tsx @@ -11,6 +11,7 @@ import { IntegrationSlug, ToolAddress, effectivePolicyFromSorted, + isToolSyncHealth, type Connection, type Owner, } from "@executor-js/sdk/shared"; @@ -255,7 +256,17 @@ export function IntegrationDetailPage(props: { const healthProbeFor = useConnectionsHealth(integrationConnections); const toolsHealthIssue = useMemo(() => { const issues = integrationConnections - .map((connection) => ({ connection, probe: healthProbeFor(connection) })) + .map((connection) => ({ + connection, + // The health hook hides tool-sync stamps from CONNECTION-health + // display, but this consumer explains an empty CATALOG — and a + // "Tool sync failing" stamp is exactly that explanation (it carries + // the sync failure and drives the "Check and sync tools" recovery). + // Fall back to the raw persisted verdict when it is one. + probe: + healthProbeFor(connection) ?? + (isToolSyncHealth(connection.lastHealth) ? connection.lastHealth : null), + })) .filter( ( entry, From 526a0e9eeb4da725bf25e4c6386a7b2cb69be516 Mon Sep 17 00:00:00 2001 From: Rhys Sullivan <39114868+RhysSullivan@users.noreply.github.com> Date: Mon, 31 Aug 2026 23:11:30 -0700 Subject: [PATCH 4/4] Let expired probes outrank the catalog stamp; healthy probes never hide it --- .../react/src/pages/integration-detail.tsx | 29 ++++++++++++------- 1 file changed, 19 insertions(+), 10 deletions(-) diff --git a/packages/react/src/pages/integration-detail.tsx b/packages/react/src/pages/integration-detail.tsx index e67fe93514..84862a5dc5 100644 --- a/packages/react/src/pages/integration-detail.tsx +++ b/packages/react/src/pages/integration-detail.tsx @@ -256,17 +256,26 @@ export function IntegrationDetailPage(props: { const healthProbeFor = useConnectionsHealth(integrationConnections); const toolsHealthIssue = useMemo(() => { const issues = integrationConnections - .map((connection) => ({ - connection, + .map((connection) => { // The health hook hides tool-sync stamps from CONNECTION-health - // display, but this consumer explains an empty CATALOG — and a - // "Tool sync failing" stamp is exactly that explanation (it carries - // the sync failure and drives the "Check and sync tools" recovery). - // Fall back to the raw persisted verdict when it is one. - probe: - healthProbeFor(connection) ?? - (isToolSyncHealth(connection.lastHealth) ? connection.lastHealth : null), - })) + // display, but this consumer explains an empty CATALOG, where the + // "Tool sync failing" stamp is a real answer (it names the sync + // failure and drives the "Check and sync tools" recovery). Precedence: + // an expired/degraded PROBE verdict wins — a rejected credential + // names its own fix ("Connection rejected" → reconnect) — but a + // healthy or unknown probe must NOT hide the stamp: a working + // credential does not refute a broken catalog, and only a successful + // sync clears the stamp. + const probe = healthProbeFor(connection); + const catalogStamp = isToolSyncHealth(connection.lastHealth) ? connection.lastHealth : null; + return { + connection, + probe: + probe?.status === "expired" || probe?.status === "degraded" + ? probe + : (catalogStamp ?? probe), + }; + }) .filter( ( entry,