Skip to content

Commit 29b411b

Browse files
committed
Refresh connection health after OAuth reconnect
1 parent 807a9ba commit 29b411b

2 files changed

Lines changed: 52 additions & 20 deletions

File tree

‎packages/core/sdk/src/executor.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2789,6 +2789,11 @@ export const createExecutor = <const TPlugins extends readonly AnyPlugin[] = rea
27892789
input.missingOAuthScopes && input.missingOAuthScopes.length > 0
27902790
? { missingOAuthScopes: input.missingOAuthScopes }
27912791
: null,
2792+
// A re-mint replaces the grant, so any persisted verdict describes
2793+
// a credential that no longer exists. Clear it rather than let a
2794+
// pre-reconnect "expired" outlive the reconnect; the next health
2795+
// check writes the verdict for the new grant.
2796+
last_health: null,
27922797
updated_at: now,
27932798
};
27942799
if (existing) {

‎packages/react/src/lib/use-connection-health.ts‎

Lines changed: 47 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,16 @@ const revalidateQuery = (
4242
): { readonly ifStaleMs?: number } =>
4343
last?.status === "healthy" ? { ifStaleMs: HEALTH_REVALIDATE_MS } : {};
4444

45+
/** Identity of a persisted verdict, for deciding when to revalidate again.
46+
* Revalidation is once per VERDICT, not once per mount: an OAuth re-mint
47+
* clears `last_health` and a probe from another surface lands a fresher one,
48+
* and both must re-trigger the background probe even though the row never
49+
* remounts (its React key is owner:integration:name, unchanged by a
50+
* reconnect). `null` is a real epoch — the never-checked / just-re-minted
51+
* state — distinct from the "never revalidated" sentinel `undefined`. */
52+
const verdictEpoch = (last: HealthCheckResult | null | undefined): number | null =>
53+
last?.checkedAt ?? null;
54+
4555
/**
4656
* Imperative invalidation of the connections cache for one owner. The server
4757
* persists every verdict on `last_health`, so after a check we must re-read the
@@ -81,24 +91,34 @@ export function useConnectionHealth(connection: Connection): {
8191
// Health checks are AUTOMATIC: loading the list revalidates any verdict
8292
// older than the freshness window (or never checked), stale-while-revalidate
8393
// style: the persisted verdict renders instantly, the probe corrects it in
84-
// place.
85-
const revalidated = useRef(false);
94+
// place. The guard is once per VERDICT (not per mount): the ref holds the
95+
// epoch this hook last acted on, and only a DIFFERENT epoch re-triggers —
96+
// that is how an OAuth reconnect (re-mint clears `last_health`, refetch
97+
// delivers a null epoch) gets its recovery probe without a page reload,
98+
// while the refetch echoing a probe's own persisted verdict matches the
99+
// epoch and stays quiet.
100+
// `undefined` = never revalidated (epochs are number | null, so it can
101+
// never collide with a real epoch).
102+
const revalidatedEpoch = useRef<number | null | undefined>(undefined);
86103
useEffect(() => {
87-
if (revalidated.current) return;
88104
const last = connection.lastHealth;
105+
const epoch = verdictEpoch(last);
106+
if (revalidatedEpoch.current === epoch) return;
107+
revalidatedEpoch.current = epoch;
89108
if (healthyAndFresh(last)) return;
90-
revalidated.current = true;
91109
void doCheck({
92110
params: connectionParams(connection),
93111
query: revalidateQuery(last),
94112
}).then((exit) => {
95113
// Background refresh: update the dot on success, stay quiet on failure
96-
// (the persisted verdict is still the best known state). Invalidate the
97-
// connections cache ONLY when the verdict actually changed: on the common
98-
// no-change reconfirm we skip it, so an automatic probe never churns the
99-
// cache (which would refetch connections, re-run this effect, and, but
100-
// for the once-per-mount ref guard, risk a probe loop).
114+
// (the persisted verdict is still the best known state). Adopt the
115+
// result's own epoch so the refetch below finds it already handled.
116+
// Invalidate the connections cache ONLY when the verdict actually
117+
// changed: on the common no-change reconfirm we skip it, so an automatic
118+
// probe never churns the cache (which would refetch connections, re-run
119+
// this effect, and, but for the epoch guard, risk a probe loop).
101120
if (!Exit.isSuccess(exit)) return;
121+
revalidatedEpoch.current = exit.value.checkedAt;
102122
setLiveProbe(exit.value);
103123
if (exit.value.status !== (last?.status ?? "unknown")) {
104124
invalidateConnections(connection.owner);
@@ -108,14 +128,17 @@ export function useConnectionHealth(connection: Connection): {
108128

109129
const runCheck = useCallback(async () => {
110130
// Manual "Check now": invalidate the connections cache unconditionally so
111-
// every surface picks up the freshly persisted verdict. Re-running this
112-
// effect after the refetch is harmless: the ref guard blocks a re-probe.
131+
// every surface picks up the freshly persisted verdict. Adopting the
132+
// result's epoch keeps the resulting refetch from re-probing.
113133
const exit = await doCheck({
114134
params: connectionParams(connection),
115135
query: {},
116136
reactivityKeys: connectionCheckKeys,
117137
});
118-
if (Exit.isSuccess(exit)) setLiveProbe(exit.value);
138+
if (Exit.isSuccess(exit)) {
139+
revalidatedEpoch.current = exit.value.checkedAt;
140+
setLiveProbe(exit.value);
141+
}
119142
return exit;
120143
}, [connection, doCheck]);
121144

@@ -140,25 +163,29 @@ export function useConnectionsHealth(
140163
const doCheck = useAtomSet(checkConnectionHealth, { mode: "promiseExit" });
141164
const invalidateConnections = useInvalidateConnections();
142165

143-
// Once per mount PER CONNECTION: the list streams in asynchronously, so the
144-
// effect re-runs as rows arrive; the key set keeps each row to one probe.
145-
const revalidated = useRef(new Set<string>());
166+
// Once per VERDICT per connection (same epoch guard as the single-connection
167+
// hook): the list streams in asynchronously, so the effect re-runs as rows
168+
// arrive; each row probes once per persisted-verdict epoch, and a re-minted
169+
// connection (epoch cleared to null) probes again without a remount.
170+
const revalidated = useRef(new Map<string, number | null>());
146171
useEffect(() => {
147172
for (const connection of connections) {
148173
const key = probeKey(connection);
149-
if (revalidated.current.has(key)) continue;
150174
const last = connection.lastHealth;
175+
const epoch = verdictEpoch(last);
176+
if (revalidated.current.has(key) && revalidated.current.get(key) === epoch) continue;
177+
revalidated.current.set(key, epoch);
151178
if (healthyAndFresh(last)) continue;
152-
revalidated.current.add(key);
153179
void doCheck({
154180
params: connectionParams(connection),
155181
query: revalidateQuery(last),
156182
}).then((exit) => {
157183
// Same automatic-path rule as the single-connection hook: reflect the
158-
// verdict, and invalidate the connections cache only when it changed so
159-
// an unchanged reconfirm never churns the cache (the per-key ref guard
160-
// already prevents a re-probe on the resulting re-render).
184+
// verdict, adopt its epoch so the refetch doesn't re-probe, and
185+
// invalidate the connections cache only when the verdict changed so an
186+
// unchanged reconfirm never churns the cache.
161187
if (!Exit.isSuccess(exit)) return;
188+
revalidated.current.set(key, exit.value.checkedAt);
162189
setLiveProbes((current) => new Map(current).set(key, exit.value));
163190
if (exit.value.status !== (last?.status ?? "unknown")) {
164191
invalidateConnections(connection.owner);

0 commit comments

Comments
 (0)