Merge pull request #2152 from modelcontextprotocol/v2/chore/2129-use-session-ref
chore(web): collapse App.tsx's six session refs into useSessionRef
This commit is contained in:
commit
6c9c149406
4 changed files with 381 additions and 93 deletions
|
|
@ -79,6 +79,7 @@ import { usePaginatedListsOverride } from "./hooks/usePaginatedListsOverride";
|
||||||
import { useValueChange } from "./hooks/useValueChange";
|
import { useValueChange } from "./hooks/useValueChange";
|
||||||
import { useThemeToggle } from "./hooks/useThemeToggle";
|
import { useThemeToggle } from "./hooks/useThemeToggle";
|
||||||
import { useTabUiState } from "./hooks/useTabUiState";
|
import { useTabUiState } from "./hooks/useTabUiState";
|
||||||
|
import { useSessionRef } from "./hooks/useSessionRef";
|
||||||
import { useInspectorStores } from "./hooks/useInspectorStores";
|
import { useInspectorStores } from "./hooks/useInspectorStores";
|
||||||
import { useExportActions } from "./hooks/useExportActions";
|
import { useExportActions } from "./hooks/useExportActions";
|
||||||
import { useProgressToasts } from "./hooks/useProgressToasts";
|
import { useProgressToasts } from "./hooks/useProgressToasts";
|
||||||
|
|
@ -191,6 +192,7 @@ import { EMPTY_SETTINGS } from "./utils/serverSettingsDefaults";
|
||||||
import {
|
import {
|
||||||
isEmaStepUp,
|
isEmaStepUp,
|
||||||
isStepUpConfirmation,
|
isStepUpConfirmation,
|
||||||
|
type PendingStepUp,
|
||||||
type StepUpSource,
|
type StepUpSource,
|
||||||
} from "./utils/stepUp";
|
} from "./utils/stepUp";
|
||||||
import {
|
import {
|
||||||
|
|
@ -542,18 +544,10 @@ function App() {
|
||||||
togglePinProtocol,
|
togglePinProtocol,
|
||||||
resetTabUiState,
|
resetTabUiState,
|
||||||
} = useTabUiState();
|
} = useTabUiState();
|
||||||
const [pendingStepUp, setPendingStepUp] = useState<{
|
const [pendingStepUp, setPendingStepUp] = useState<PendingStepUp | null>(
|
||||||
challenge: AuthChallenge;
|
null,
|
||||||
authorizationUrl: URL;
|
);
|
||||||
serverId: string;
|
|
||||||
source: StepUpSource;
|
|
||||||
enterpriseManaged?: boolean;
|
|
||||||
} | null>(null);
|
|
||||||
const pendingStepUpRef = useRef(pendingStepUp);
|
|
||||||
const pendingStepUpRetryRef = useRef<(() => Promise<unknown>) | null>(null);
|
const pendingStepUpRetryRef = useRef<(() => Promise<unknown>) | null>(null);
|
||||||
useEffect(() => {
|
|
||||||
pendingStepUpRef.current = pendingStepUp;
|
|
||||||
}, [pendingStepUp]);
|
|
||||||
const [reAuthBanner, setReAuthBanner] = useState<{
|
const [reAuthBanner, setReAuthBanner] = useState<{
|
||||||
serverId: string;
|
serverId: string;
|
||||||
message: string;
|
message: string;
|
||||||
|
|
@ -575,27 +569,35 @@ function App() {
|
||||||
const [pendingReauth, setPendingReauth] = useState<PendingReauth | null>(
|
const [pendingReauth, setPendingReauth] = useState<PendingReauth | null>(
|
||||||
null,
|
null,
|
||||||
);
|
);
|
||||||
const pendingReauthRef = useRef(pendingReauth);
|
// One stable ref mirroring every session value a long-lived callback needs
|
||||||
useEffect(() => {
|
// to read *currently* rather than as of the render that created it. Declared
|
||||||
pendingReauthRef.current = pendingReauth;
|
// here — ahead of its first reader — because a hook return is not provably
|
||||||
}, [pendingReauth]);
|
// stable to `react-hooks/exhaustive-deps`, so consumers list it and would hit
|
||||||
|
// the temporal dead zone if it came later.
|
||||||
|
const sessionRef = useSessionRef({
|
||||||
|
activeServerId,
|
||||||
|
servers,
|
||||||
|
inspectorClient,
|
||||||
|
pendingStepUp,
|
||||||
|
pendingReauth,
|
||||||
|
});
|
||||||
const reauthResumeInProgressRef = useRef(false);
|
const reauthResumeInProgressRef = useRef(false);
|
||||||
const stepUpAuthorizeInProgressRef = useRef(false);
|
const stepUpAuthorizeInProgressRef = useRef(false);
|
||||||
|
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
const pending = pendingReauthRef.current;
|
const pending = sessionRef.current.pendingReauth;
|
||||||
if (pending && pending.serverId !== activeServerId) {
|
if (pending && pending.serverId !== activeServerId) {
|
||||||
setPendingReauth(null);
|
setPendingReauth(null);
|
||||||
}
|
}
|
||||||
const stepUp = pendingStepUpRef.current;
|
const stepUp = sessionRef.current.pendingStepUp;
|
||||||
if (stepUp && stepUp.serverId !== activeServerId) {
|
if (stepUp && stepUp.serverId !== activeServerId) {
|
||||||
setPendingStepUp(null);
|
setPendingStepUp(null);
|
||||||
}
|
}
|
||||||
}, [activeServerId]);
|
}, [sessionRef, activeServerId]);
|
||||||
|
|
||||||
const trySetPendingStepUp = useCallback(
|
const trySetPendingStepUp = useCallback(
|
||||||
(next: NonNullable<typeof pendingStepUp>): boolean => {
|
(next: NonNullable<typeof pendingStepUp>): boolean => {
|
||||||
if (pendingStepUpRef.current !== null) {
|
if (sessionRef.current.pendingStepUp !== null) {
|
||||||
notifications.show({
|
notifications.show({
|
||||||
title: "Step-up authorization in progress",
|
title: "Step-up authorization in progress",
|
||||||
message:
|
message:
|
||||||
|
|
@ -608,7 +610,7 @@ function App() {
|
||||||
setPendingStepUp(next);
|
setPendingStepUp(next);
|
||||||
return true;
|
return true;
|
||||||
},
|
},
|
||||||
[],
|
[sessionRef],
|
||||||
);
|
);
|
||||||
|
|
||||||
// Handshake telemetry. `connectStartRef` is set at the "connecting" edge
|
// Handshake telemetry. `connectStartRef` is set at the "connecting" edge
|
||||||
|
|
@ -860,7 +862,7 @@ function App() {
|
||||||
// leave the control reading Off while every modern request still carries
|
// leave the control reading Off while every modern request still carries
|
||||||
// the level — visible on the auth-recovery path, which reconnects the same
|
// the level — visible on the auth-recovery path, which reconnects the same
|
||||||
// client instance rather than rebuilding it (#1629, #1797).
|
// client instance rather than rebuilding it (#1629, #1797).
|
||||||
// `activeServerIdRef` is synced in a passive effect, so it still holds the
|
// The session ref is synced in a passive effect, so it still holds the
|
||||||
// outgoing server's id when this runs from `onDisconnect` — which is what
|
// outgoing server's id when this runs from `onDisconnect` — which is what
|
||||||
// lets the re-seed find its settings. Clearing that ref eagerly would take
|
// lets the re-seed find its settings. Clearing that ref eagerly would take
|
||||||
// the no-server branch below and silently drop this to Off.
|
// the no-server branch below and silently drop this to Off.
|
||||||
|
|
@ -868,8 +870,8 @@ function App() {
|
||||||
// is the common case (`mcp.json` written by hand, never opened in Server
|
// is the common case (`mcp.json` written by hand, never opened in Server
|
||||||
// Settings), and there the default is right — it is what the seed and the
|
// Settings), and there the default is right — it is what the seed and the
|
||||||
// client both use. Only "no server at all" means Off.
|
// client both use. Only "no server at all" means Off.
|
||||||
const activeServer = serversRef.current.find(
|
const activeServer = sessionRef.current.servers.find(
|
||||||
(s) => s.id === activeServerIdRef.current,
|
(s) => s.id === sessionRef.current.activeServerId,
|
||||||
);
|
);
|
||||||
setModernLogLevel(
|
setModernLogLevel(
|
||||||
activeServer
|
activeServer
|
||||||
|
|
@ -882,7 +884,7 @@ function App() {
|
||||||
// Remembered scroll offsets are session-scoped too — drop them so the next
|
// Remembered scroll offsets are session-scoped too — drop them so the next
|
||||||
// session's screens start at the top (#1417).
|
// session's screens start at the top (#1417).
|
||||||
clearScrollMemory();
|
clearScrollMemory();
|
||||||
}, [resetTabUiState, resetTaskProgress]);
|
}, [sessionRef, resetTabUiState, resetTaskProgress]);
|
||||||
|
|
||||||
// Reset activeServerId whenever the live session ends. Without this the
|
// Reset activeServerId whenever the live session ends. Without this the
|
||||||
// other ServerCards stay `inert` after disconnect — ServerCard dims any
|
// other ServerCards stay `inert` after disconnect — ServerCard dims any
|
||||||
|
|
@ -966,26 +968,6 @@ function App() {
|
||||||
activeServer,
|
activeServer,
|
||||||
]);
|
]);
|
||||||
|
|
||||||
// Mirror the active server's name into a ref so a mid-session failure toast
|
|
||||||
// can still name the server: a transport crash dispatches `disconnect`,
|
|
||||||
// which clears `activeServerId` (and thus `activeServer`) before the
|
|
||||||
// `lastError` effect below runs, so the ref is the only surviving handle.
|
|
||||||
const activeServerNameRef = useRef<string | undefined>(undefined);
|
|
||||||
const activeServerIdRef = useRef<string | undefined>(activeServerId);
|
|
||||||
const serversRef = useRef(servers);
|
|
||||||
// The live client, mirrored for the same reason: a settings write that
|
|
||||||
// outlives a disconnect/reconnect to the *same* server would otherwise push
|
|
||||||
// its value into the instance captured when it was issued — which has since
|
|
||||||
// been destroyed — while the replacement client, built from the stale list
|
|
||||||
// entry, keeps the value the write replaced (#2089).
|
|
||||||
const inspectorClientRef = useRef<InspectorClient | null>(inspectorClient);
|
|
||||||
useEffect(() => {
|
|
||||||
if (activeServer) activeServerNameRef.current = activeServer.name;
|
|
||||||
activeServerIdRef.current = activeServerId;
|
|
||||||
serversRef.current = servers;
|
|
||||||
inspectorClientRef.current = inspectorClient;
|
|
||||||
}, [activeServer, activeServerId, servers, inspectorClient]);
|
|
||||||
|
|
||||||
// Last connection-level error message, surfaced as `data-error-message` on
|
// Last connection-level error message, surfaced as `data-error-message` on
|
||||||
// the InspectorView header so an automated driver can read *why* a connect
|
// the InspectorView header so an automated driver can read *why* a connect
|
||||||
// failed without scraping a transient toast. Cleared on the next connect
|
// failed without scraping a transient toast. Cleared on the next connect
|
||||||
|
|
@ -1023,7 +1005,7 @@ function App() {
|
||||||
detail?: unknown,
|
detail?: unknown,
|
||||||
options?: { reason?: AuthChallengeReason },
|
options?: { reason?: AuthChallengeReason },
|
||||||
) => {
|
) => {
|
||||||
const server = serversRef.current.find((s) => s.id === serverId);
|
const server = sessionRef.current.servers.find((s) => s.id === serverId);
|
||||||
const message = reAuthBannerMessage({
|
const message = reAuthBannerMessage({
|
||||||
serverName: server?.name,
|
serverName: server?.name,
|
||||||
detail:
|
detail:
|
||||||
|
|
@ -1044,7 +1026,7 @@ function App() {
|
||||||
message,
|
message,
|
||||||
});
|
});
|
||||||
},
|
},
|
||||||
[],
|
[sessionRef],
|
||||||
);
|
);
|
||||||
|
|
||||||
// Surface a mid-session transport failure (stdio crash, SSE drop, HTTP 5xx)
|
// Surface a mid-session transport failure (stdio crash, SSE drop, HTTP 5xx)
|
||||||
|
|
@ -1054,13 +1036,13 @@ function App() {
|
||||||
// `lastError` clears at the next connecting edge, so each failure toasts once.
|
// `lastError` clears at the next connecting edge, so each failure toasts once.
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
if (!lastError) return;
|
if (!lastError) return;
|
||||||
const name = activeServerNameRef.current;
|
const name = sessionRef.current.activeServerName;
|
||||||
notifications.show({
|
notifications.show({
|
||||||
title: name ? `Connection to "${name}" lost` : "Connection lost",
|
title: name ? `Connection to "${name}" lost` : "Connection lost",
|
||||||
message: lastError,
|
message: lastError,
|
||||||
color: "red",
|
color: "red",
|
||||||
});
|
});
|
||||||
}, [lastError]);
|
}, [sessionRef, lastError]);
|
||||||
|
|
||||||
// `config.type` is optional in the schema (a bare `command: ...`
|
// `config.type` is optional in the schema (a bare `command: ...`
|
||||||
// entry implies stdio), so we materialize the default here rather
|
// entry implies stdio), so we materialize the default here rather
|
||||||
|
|
@ -1098,7 +1080,7 @@ function App() {
|
||||||
};
|
};
|
||||||
|
|
||||||
const onAmbientAuthChallenge = (): void => {
|
const onAmbientAuthChallenge = (): void => {
|
||||||
const name = activeServerNameRef.current;
|
const name = sessionRef.current.activeServerName;
|
||||||
notifications.show({
|
notifications.show({
|
||||||
title: name
|
title: name
|
||||||
? `Refreshing authorization for "${name}"`
|
? `Refreshing authorization for "${name}"`
|
||||||
|
|
@ -1123,7 +1105,7 @@ function App() {
|
||||||
onAmbientAuthChallenge,
|
onAmbientAuthChallenge,
|
||||||
);
|
);
|
||||||
};
|
};
|
||||||
}, [connectionStatus, inspectorClient]);
|
}, [sessionRef, connectionStatus, inspectorClient]);
|
||||||
|
|
||||||
const connectionInfoCanClearOAuth =
|
const connectionInfoCanClearOAuth =
|
||||||
connectionStatus === "connected" &&
|
connectionStatus === "connected" &&
|
||||||
|
|
@ -1176,7 +1158,7 @@ function App() {
|
||||||
// applies to the instance that exists now.
|
// applies to the instance that exists now.
|
||||||
const applyLiveServerSettings = useCallback(
|
const applyLiveServerSettings = useCallback(
|
||||||
(settings: InspectorServerSettings) => {
|
(settings: InspectorServerSettings) => {
|
||||||
const client = inspectorClientRef.current;
|
const client = sessionRef.current.inspectorClient;
|
||||||
if (!client) return;
|
if (!client) return;
|
||||||
// Settings the managed state reads at notification time
|
// Settings the managed state reads at notification time
|
||||||
// (auto-refresh-on-list-changed) take effect without a reconnect (#1444).
|
// (auto-refresh-on-list-changed) take effect without a reconnect (#1444).
|
||||||
|
|
@ -1200,7 +1182,7 @@ function App() {
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
},
|
},
|
||||||
[fetchLogRef],
|
[sessionRef, fetchLogRef],
|
||||||
);
|
);
|
||||||
|
|
||||||
// Flush the pre-redirect Network log to backend storage, keyed by the OAuth
|
// Flush the pre-redirect Network log to backend storage, keyed by the OAuth
|
||||||
|
|
@ -1253,7 +1235,7 @@ function App() {
|
||||||
client?: InspectorClient;
|
client?: InspectorClient;
|
||||||
}) => {
|
}) => {
|
||||||
setReAuthBanner(null);
|
setReAuthBanner(null);
|
||||||
const server = serversRef.current.find((s) => s.id === serverId);
|
const server = sessionRef.current.servers.find((s) => s.id === serverId);
|
||||||
const preRedirectToast = oauthPreRedirectToastCopy(authKind, {
|
const preRedirectToast = oauthPreRedirectToastCopy(authKind, {
|
||||||
serverName: server?.name,
|
serverName: server?.name,
|
||||||
enterpriseManaged: server?.settings?.enterpriseManaged,
|
enterpriseManaged: server?.settings?.enterpriseManaged,
|
||||||
|
|
@ -1284,7 +1266,7 @@ function App() {
|
||||||
});
|
});
|
||||||
void oauthClient?.beginInteractiveAuthorization(authorizationUrl);
|
void oauthClient?.beginInteractiveAuthorization(authorizationUrl);
|
||||||
},
|
},
|
||||||
[inspectorClient, activeTab, ui, onBeforeOAuthRedirect],
|
[sessionRef, inspectorClient, activeTab, ui, onBeforeOAuthRedirect],
|
||||||
);
|
);
|
||||||
|
|
||||||
const tryApplyStoredAuthRecovery = useCallback(
|
const tryApplyStoredAuthRecovery = useCallback(
|
||||||
|
|
@ -1320,7 +1302,7 @@ function App() {
|
||||||
authorizationUrl: URL;
|
authorizationUrl: URL;
|
||||||
source?: StepUpSource;
|
source?: StepUpSource;
|
||||||
}) => {
|
}) => {
|
||||||
const server = serversRef.current.find((s) => s.id === serverId);
|
const server = sessionRef.current.servers.find((s) => s.id === serverId);
|
||||||
if (isStepUpConfirmation(challenge, server)) {
|
if (isStepUpConfirmation(challenge, server)) {
|
||||||
trySetPendingStepUp({
|
trySetPendingStepUp({
|
||||||
challenge,
|
challenge,
|
||||||
|
|
@ -1338,28 +1320,31 @@ function App() {
|
||||||
recoverySource: source,
|
recoverySource: source,
|
||||||
});
|
});
|
||||||
},
|
},
|
||||||
[prepareOAuthRedirect, trySetPendingStepUp],
|
[sessionRef, prepareOAuthRedirect, trySetPendingStepUp],
|
||||||
);
|
);
|
||||||
|
|
||||||
const deferAmbientReauth = useCallback((pending: PendingReauth) => {
|
const deferAmbientReauth = useCallback(
|
||||||
if (pendingReauthRef.current) {
|
(pending: PendingReauth) => {
|
||||||
notifications.show({
|
if (sessionRef.current.pendingReauth) {
|
||||||
title: "Authorization update pending",
|
notifications.show({
|
||||||
message:
|
title: "Authorization update pending",
|
||||||
"A new authorization request replaced the previous deferred recovery.",
|
message:
|
||||||
color: "yellow",
|
"A new authorization request replaced the previous deferred recovery.",
|
||||||
autoClose: 5000,
|
color: "yellow",
|
||||||
});
|
autoClose: 5000,
|
||||||
} else {
|
});
|
||||||
notifications.show({
|
} else {
|
||||||
title: "Authorization pending",
|
notifications.show({
|
||||||
message: "Return to this tab to continue authorization.",
|
title: "Authorization pending",
|
||||||
color: "blue",
|
message: "Return to this tab to continue authorization.",
|
||||||
autoClose: 5000,
|
color: "blue",
|
||||||
});
|
autoClose: 5000,
|
||||||
}
|
});
|
||||||
setPendingReauth(pending);
|
}
|
||||||
}, []);
|
setPendingReauth(pending);
|
||||||
|
},
|
||||||
|
[sessionRef],
|
||||||
|
);
|
||||||
|
|
||||||
const handleCommandScopedAuthRecovery = useCallback(
|
const handleCommandScopedAuthRecovery = useCallback(
|
||||||
async (
|
async (
|
||||||
|
|
@ -1373,7 +1358,9 @@ function App() {
|
||||||
if (!inspectorClient) {
|
if (!inspectorClient) {
|
||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
const server = serversRef.current.find((s) => s.id === options.serverId);
|
const server = sessionRef.current.servers.find(
|
||||||
|
(s) => s.id === options.serverId,
|
||||||
|
);
|
||||||
const stepUp =
|
const stepUp =
|
||||||
error.emaStepUpConfirm ||
|
error.emaStepUpConfirm ||
|
||||||
isStepUpConfirmation(error.authChallenge, server);
|
isStepUpConfirmation(error.authChallenge, server);
|
||||||
|
|
@ -1418,6 +1405,7 @@ function App() {
|
||||||
return false;
|
return false;
|
||||||
},
|
},
|
||||||
[
|
[
|
||||||
|
sessionRef,
|
||||||
inspectorClient,
|
inspectorClient,
|
||||||
prepareOAuthRedirect,
|
prepareOAuthRedirect,
|
||||||
tryApplyStoredAuthRecovery,
|
tryApplyStoredAuthRecovery,
|
||||||
|
|
@ -1505,7 +1493,7 @@ function App() {
|
||||||
if (connectionStatus !== "connected") {
|
if (connectionStatus !== "connected") {
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
if (pending.serverId !== activeServerIdRef.current) {
|
if (pending.serverId !== sessionRef.current.activeServerId) {
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
@ -1564,6 +1552,7 @@ function App() {
|
||||||
}
|
}
|
||||||
},
|
},
|
||||||
[
|
[
|
||||||
|
sessionRef,
|
||||||
inspectorClient,
|
inspectorClient,
|
||||||
connectionStatus,
|
connectionStatus,
|
||||||
tryApplyStoredAuthRecovery,
|
tryApplyStoredAuthRecovery,
|
||||||
|
|
@ -1582,11 +1571,13 @@ function App() {
|
||||||
): void => {
|
): void => {
|
||||||
void (async () => {
|
void (async () => {
|
||||||
const { challenge, authorizationUrl } = event.detail;
|
const { challenge, authorizationUrl } = event.detail;
|
||||||
const serverId = activeServerIdRef.current;
|
const serverId = sessionRef.current.activeServerId;
|
||||||
if (!serverId) {
|
if (!serverId) {
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
const server = serversRef.current.find((s) => s.id === serverId);
|
const server = sessionRef.current.servers.find(
|
||||||
|
(s) => s.id === serverId,
|
||||||
|
);
|
||||||
const authKind: OAuthResumeAuthKind = isStepUpConfirmation(
|
const authKind: OAuthResumeAuthKind = isStepUpConfirmation(
|
||||||
challenge,
|
challenge,
|
||||||
server,
|
server,
|
||||||
|
|
@ -1629,6 +1620,7 @@ function App() {
|
||||||
);
|
);
|
||||||
};
|
};
|
||||||
}, [
|
}, [
|
||||||
|
sessionRef,
|
||||||
connectionStatus,
|
connectionStatus,
|
||||||
inspectorClient,
|
inspectorClient,
|
||||||
tryApplyStoredAuthRecovery,
|
tryApplyStoredAuthRecovery,
|
||||||
|
|
@ -1638,12 +1630,12 @@ function App() {
|
||||||
|
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
return onBrowserTabVisible(() => {
|
return onBrowserTabVisible(() => {
|
||||||
const pending = pendingReauthRef.current;
|
const pending = sessionRef.current.pendingReauth;
|
||||||
if (pending) {
|
if (pending) {
|
||||||
void resumePendingReauth(pending);
|
void resumePendingReauth(pending);
|
||||||
}
|
}
|
||||||
});
|
});
|
||||||
}, [resumePendingReauth]);
|
}, [sessionRef, resumePendingReauth]);
|
||||||
|
|
||||||
// Resume deferred background-tab recovery once the session reconnects.
|
// Resume deferred background-tab recovery once the session reconnects.
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
|
|
@ -1653,11 +1645,11 @@ function App() {
|
||||||
if (!isBrowserTabVisible()) {
|
if (!isBrowserTabVisible()) {
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
const pending = pendingReauthRef.current;
|
const pending = sessionRef.current.pendingReauth;
|
||||||
if (pending) {
|
if (pending) {
|
||||||
void resumePendingReauth(pending);
|
void resumePendingReauth(pending);
|
||||||
}
|
}
|
||||||
}, [connectionStatus, inspectorClient, resumePendingReauth]);
|
}, [sessionRef, connectionStatus, inspectorClient, resumePendingReauth]);
|
||||||
|
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
if (connectionStatus !== "connected" || !inspectorClient) {
|
if (connectionStatus !== "connected" || !inspectorClient) {
|
||||||
|
|
@ -1665,7 +1657,7 @@ function App() {
|
||||||
}
|
}
|
||||||
|
|
||||||
const onOAuthError = (event: TypedEvent<"oauthError">): void => {
|
const onOAuthError = (event: TypedEvent<"oauthError">): void => {
|
||||||
const serverId = activeServerIdRef.current;
|
const serverId = sessionRef.current.activeServerId;
|
||||||
if (!serverId) {
|
if (!serverId) {
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
@ -1676,7 +1668,7 @@ function App() {
|
||||||
return () => {
|
return () => {
|
||||||
inspectorClient.removeEventListener("oauthError", onOAuthError);
|
inspectorClient.removeEventListener("oauthError", onOAuthError);
|
||||||
};
|
};
|
||||||
}, [connectionStatus, inspectorClient, showReAuthBanner]);
|
}, [sessionRef, connectionStatus, inspectorClient, showReAuthBanner]);
|
||||||
|
|
||||||
// Detect an abandoned full-page OAuth redirect (snapshot left, no callback).
|
// Detect an abandoned full-page OAuth redirect (snapshot left, no callback).
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
|
|
@ -2183,7 +2175,7 @@ function App() {
|
||||||
// addServer/updateServer in the same async tick (e.g. the deep-link
|
// addServer/updateServer in the same async tick (e.g. the deep-link
|
||||||
// auto-connect IIFE) sees the freshly-mutated list, not the stale array
|
// auto-connect IIFE) sees the freshly-mutated list, not the stale array
|
||||||
// captured by this callback's closure.
|
// captured by this callback's closure.
|
||||||
const target = serversRef.current.find((s) => s.id === id);
|
const target = sessionRef.current.servers.find((s) => s.id === id);
|
||||||
if (!target) return;
|
if (!target) return;
|
||||||
|
|
||||||
// Always rebuild the InspectorClient on a (re)connect so the latest
|
// Always rebuild the InspectorClient on a (re)connect so the latest
|
||||||
|
|
@ -2344,6 +2336,7 @@ function App() {
|
||||||
}
|
}
|
||||||
},
|
},
|
||||||
[
|
[
|
||||||
|
sessionRef,
|
||||||
activeServerId,
|
activeServerId,
|
||||||
connectionStatus,
|
connectionStatus,
|
||||||
inspectorClient,
|
inspectorClient,
|
||||||
|
|
@ -2372,7 +2365,7 @@ function App() {
|
||||||
// 3. connect — row present AND its config already matches: connect.
|
// 3. connect — row present AND its config already matches: connect.
|
||||||
// Splitting update and connect across renders (rather than awaiting both in
|
// Splitting update and connect across renders (rather than awaiting both in
|
||||||
// one closure) is what makes the connect correct: `onToggleConnection` reads
|
// one closure) is what makes the connect correct: `onToggleConnection` reads
|
||||||
// the target from `serversRef`, which an earlier passive effect syncs from
|
// the target from the session ref, which an earlier passive effect syncs from
|
||||||
// `servers` — so connecting only once `servers` reflects the updated config
|
// `servers` — so connecting only once `servers` reflects the updated config
|
||||||
// guarantees the client is built from the fresh transport, not the stale one.
|
// guarantees the client is built from the fresh transport, not the stale one.
|
||||||
// The OAuth callback path takes precedence; a deep link on `/oauth/callback`
|
// The OAuth callback path takes precedence; a deep link on `/oauth/callback`
|
||||||
|
|
@ -2456,7 +2449,9 @@ function App() {
|
||||||
// toggle direction here.
|
// toggle direction here.
|
||||||
if (bannerKind === "lost_authorization_state") {
|
if (bannerKind === "lost_authorization_state") {
|
||||||
void (async () => {
|
void (async () => {
|
||||||
const server = serversRef.current.find((s) => s.id === serverId);
|
const server = sessionRef.current.servers.find(
|
||||||
|
(s) => s.id === serverId,
|
||||||
|
);
|
||||||
if (server) {
|
if (server) {
|
||||||
try {
|
try {
|
||||||
await clearServerOAuthState({
|
await clearServerOAuthState({
|
||||||
|
|
@ -2523,6 +2518,7 @@ function App() {
|
||||||
|
|
||||||
void onToggleConnection(serverId);
|
void onToggleConnection(serverId);
|
||||||
}, [
|
}, [
|
||||||
|
sessionRef,
|
||||||
reAuthBanner,
|
reAuthBanner,
|
||||||
activeServerId,
|
activeServerId,
|
||||||
connectionStatus,
|
connectionStatus,
|
||||||
|
|
@ -3136,7 +3132,7 @@ function App() {
|
||||||
activeServerId,
|
activeServerId,
|
||||||
next.paginatedLists ?? false,
|
next.paginatedLists ?? false,
|
||||||
);
|
);
|
||||||
if (activeServerIdRef.current === activeServerId) {
|
if (sessionRef.current.activeServerId === activeServerId) {
|
||||||
applyLiveServerSettings(next);
|
applyLiveServerSettings(next);
|
||||||
}
|
}
|
||||||
};
|
};
|
||||||
|
|
@ -3195,7 +3191,7 @@ function App() {
|
||||||
activeServerId,
|
activeServerId,
|
||||||
baseline.paginatedLists ?? false,
|
baseline.paginatedLists ?? false,
|
||||||
);
|
);
|
||||||
if (activeServerIdRef.current === activeServerId) {
|
if (sessionRef.current.activeServerId === activeServerId) {
|
||||||
applyLiveServerSettings(baseline);
|
applyLiveServerSettings(baseline);
|
||||||
}
|
}
|
||||||
notifications.show({
|
notifications.show({
|
||||||
|
|
@ -3206,6 +3202,7 @@ function App() {
|
||||||
});
|
});
|
||||||
},
|
},
|
||||||
[
|
[
|
||||||
|
sessionRef,
|
||||||
servers,
|
servers,
|
||||||
activeServerId,
|
activeServerId,
|
||||||
lastPersistedSettings,
|
lastPersistedSettings,
|
||||||
|
|
@ -3493,7 +3490,7 @@ function App() {
|
||||||
const settled = write.landed(value);
|
const settled = write.landed(value);
|
||||||
if (!settled) return;
|
if (!settled) return;
|
||||||
paginatedListsOverride.record(id, value.paginatedLists ?? false);
|
paginatedListsOverride.record(id, value.paginatedLists ?? false);
|
||||||
if (activeServerIdRef.current === id) {
|
if (sessionRef.current.activeServerId === id) {
|
||||||
applyLiveServerSettings(value);
|
applyLiveServerSettings(value);
|
||||||
}
|
}
|
||||||
};
|
};
|
||||||
|
|
@ -3522,7 +3519,7 @@ function App() {
|
||||||
const baseline = lastPersistedSettings.resolve(id);
|
const baseline = lastPersistedSettings.resolve(id);
|
||||||
if (baseline) {
|
if (baseline) {
|
||||||
paginatedListsOverride.record(id, baseline.paginatedLists ?? false);
|
paginatedListsOverride.record(id, baseline.paginatedLists ?? false);
|
||||||
if (activeServerIdRef.current === id) {
|
if (sessionRef.current.activeServerId === id) {
|
||||||
applyLiveServerSettings(baseline);
|
applyLiveServerSettings(baseline);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
@ -3909,7 +3906,7 @@ function App() {
|
||||||
};
|
};
|
||||||
|
|
||||||
const handleStepUpCancel = () => {
|
const handleStepUpCancel = () => {
|
||||||
const stepUp = pendingStepUpRef.current;
|
const stepUp = sessionRef.current.pendingStepUp;
|
||||||
setPendingStepUp(null);
|
setPendingStepUp(null);
|
||||||
pendingStepUpRetryRef.current = null;
|
pendingStepUpRetryRef.current = null;
|
||||||
if (!stepUp) {
|
if (!stepUp) {
|
||||||
|
|
|
||||||
197
clients/web/src/hooks/useSessionRef.test.tsx
Normal file
197
clients/web/src/hooks/useSessionRef.test.tsx
Normal file
|
|
@ -0,0 +1,197 @@
|
||||||
|
import { describe, it, expect } from "vitest";
|
||||||
|
import { InspectorClient } from "@inspector/core/mcp/index.js";
|
||||||
|
import type { ServerEntry } from "@inspector/core/mcp/types.js";
|
||||||
|
import { renderWithMantine } from "../test/renderWithMantine";
|
||||||
|
import type { PendingReauth } from "../utils/pendingReauth";
|
||||||
|
import type { PendingStepUp } from "../utils/stepUp";
|
||||||
|
import {
|
||||||
|
useSessionRef,
|
||||||
|
type SessionRef,
|
||||||
|
type SessionValues,
|
||||||
|
} from "./useSessionRef";
|
||||||
|
|
||||||
|
const entry = (id: string, name = id): ServerEntry => ({
|
||||||
|
id,
|
||||||
|
name,
|
||||||
|
config: { type: "stdio", command: "node" },
|
||||||
|
connection: { status: "disconnected" },
|
||||||
|
});
|
||||||
|
|
||||||
|
/**
|
||||||
|
* A distinct client identity per call. `Object.create` does not run the
|
||||||
|
* constructor, so this is a real `InspectorClient` prototype chain with no
|
||||||
|
* transport or environment behind it — which is all that is needed here: the
|
||||||
|
* hook only stores the reference, and every assertion compares identity rather
|
||||||
|
* than reading a property off it.
|
||||||
|
*/
|
||||||
|
const client = (): InspectorClient =>
|
||||||
|
Object.create(InspectorClient.prototype) as InspectorClient;
|
||||||
|
|
||||||
|
const stepUp = (serverId: string): PendingStepUp => ({
|
||||||
|
serverId,
|
||||||
|
challenge: { reason: "unauthorized" },
|
||||||
|
authorizationUrl: new URL("https://as.example/authorize"),
|
||||||
|
source: "tool",
|
||||||
|
});
|
||||||
|
|
||||||
|
const reauth = (serverId: string): PendingReauth => ({
|
||||||
|
serverId,
|
||||||
|
challenge: { reason: "unauthorized" },
|
||||||
|
authorizationUrl: new URL("https://as.example/authorize"),
|
||||||
|
authKind: "reauth",
|
||||||
|
source: "tool",
|
||||||
|
});
|
||||||
|
|
||||||
|
const values = (over: Partial<SessionValues> = {}): SessionValues => ({
|
||||||
|
activeServerId: undefined,
|
||||||
|
servers: [],
|
||||||
|
inspectorClient: null,
|
||||||
|
pendingStepUp: null,
|
||||||
|
pendingReauth: null,
|
||||||
|
...over,
|
||||||
|
});
|
||||||
|
|
||||||
|
interface Harness {
|
||||||
|
ref: () => SessionRef;
|
||||||
|
/** Re-render with a new set of session values, as App.tsx does each render. */
|
||||||
|
update: (next: SessionValues) => void;
|
||||||
|
}
|
||||||
|
|
||||||
|
function harness(initial: SessionValues): Harness {
|
||||||
|
let latest: SessionRef | undefined;
|
||||||
|
function Probe({ v }: { v: SessionValues }) {
|
||||||
|
latest = useSessionRef(v);
|
||||||
|
return null;
|
||||||
|
}
|
||||||
|
const { rerender } = renderWithMantine(<Probe v={initial} />);
|
||||||
|
return {
|
||||||
|
ref: () => {
|
||||||
|
if (!latest) throw new Error("hook did not render");
|
||||||
|
return latest;
|
||||||
|
},
|
||||||
|
update: (next) => rerender(<Probe v={next} />),
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
|
describe("useSessionRef", () => {
|
||||||
|
it("seeds the snapshot from the first render's values", () => {
|
||||||
|
const servers = [entry("a")];
|
||||||
|
const c = client();
|
||||||
|
const h = harness(
|
||||||
|
values({ activeServerId: "a", servers, inspectorClient: c }),
|
||||||
|
);
|
||||||
|
const snapshot = h.ref().current;
|
||||||
|
expect(snapshot.activeServerId).toBe("a");
|
||||||
|
expect(snapshot.servers).toBe(servers);
|
||||||
|
expect(snapshot.inspectorClient).toBe(c);
|
||||||
|
expect(snapshot.pendingStepUp).toBeNull();
|
||||||
|
expect(snapshot.pendingReauth).toBeNull();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("keeps one stable ref identity across renders", () => {
|
||||||
|
const h = harness(values());
|
||||||
|
const first = h.ref();
|
||||||
|
h.update(values({ activeServerId: "a", servers: [entry("a")] }));
|
||||||
|
expect(h.ref()).toBe(first);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("mirrors every field on a later render", () => {
|
||||||
|
const h = harness(values());
|
||||||
|
const servers = [entry("a"), entry("b")];
|
||||||
|
const c = client();
|
||||||
|
const up = stepUp("a");
|
||||||
|
const re = reauth("b");
|
||||||
|
h.update(
|
||||||
|
values({
|
||||||
|
activeServerId: "b",
|
||||||
|
servers,
|
||||||
|
inspectorClient: c,
|
||||||
|
pendingStepUp: up,
|
||||||
|
pendingReauth: re,
|
||||||
|
}),
|
||||||
|
);
|
||||||
|
expect(h.ref().current).toMatchObject({
|
||||||
|
activeServerId: "b",
|
||||||
|
servers,
|
||||||
|
inspectorClient: c,
|
||||||
|
pendingStepUp: up,
|
||||||
|
pendingReauth: re,
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
it("clears a value that becomes null or undefined", () => {
|
||||||
|
const h = harness(
|
||||||
|
values({
|
||||||
|
activeServerId: "a",
|
||||||
|
servers: [entry("a")],
|
||||||
|
inspectorClient: client(),
|
||||||
|
pendingStepUp: stepUp("a"),
|
||||||
|
pendingReauth: reauth("a"),
|
||||||
|
}),
|
||||||
|
);
|
||||||
|
h.update(values());
|
||||||
|
expect(h.ref().current).toMatchObject({
|
||||||
|
activeServerId: undefined,
|
||||||
|
servers: [],
|
||||||
|
inspectorClient: null,
|
||||||
|
pendingStepUp: null,
|
||||||
|
pendingReauth: null,
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
describe("activeServerName", () => {
|
||||||
|
it("stays undefined when no server was ever active", () => {
|
||||||
|
// It is derived in the sync effect rather than seeded, so it is blank
|
||||||
|
// until a render actually resolves an active entry.
|
||||||
|
const h = harness(values({ servers: [entry("a", "Alpha")] }));
|
||||||
|
expect(h.ref().current.activeServerName).toBeUndefined();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("is populated by the mount's own sync effect", () => {
|
||||||
|
const h = harness(
|
||||||
|
values({ activeServerId: "a", servers: [entry("a", "Alpha")] }),
|
||||||
|
);
|
||||||
|
expect(h.ref().current.activeServerName).toBe("Alpha");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("tracks the active server's name", () => {
|
||||||
|
const h = harness(values());
|
||||||
|
h.update(values({ activeServerId: "a", servers: [entry("a", "Alpha")] }));
|
||||||
|
expect(h.ref().current.activeServerName).toBe("Alpha");
|
||||||
|
h.update(
|
||||||
|
values({
|
||||||
|
activeServerId: "b",
|
||||||
|
servers: [entry("a", "Alpha"), entry("b", "Beta")],
|
||||||
|
}),
|
||||||
|
);
|
||||||
|
expect(h.ref().current.activeServerName).toBe("Beta");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("keeps the last active name when the active server goes away", () => {
|
||||||
|
// The case the stickiness exists for: a transport crash clears
|
||||||
|
// `activeServerId` before the failure toast is raised, and the name is
|
||||||
|
// the only surviving handle on which server died.
|
||||||
|
const h = harness(values());
|
||||||
|
h.update(values({ activeServerId: "a", servers: [entry("a", "Alpha")] }));
|
||||||
|
h.update(values({ servers: [entry("a", "Alpha")] }));
|
||||||
|
expect(h.ref().current.activeServerName).toBe("Alpha");
|
||||||
|
expect(h.ref().current.activeServerId).toBeUndefined();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("keeps the last active name when the id names no entry", () => {
|
||||||
|
const h = harness(values());
|
||||||
|
h.update(values({ activeServerId: "a", servers: [entry("a", "Alpha")] }));
|
||||||
|
h.update(values({ activeServerId: "gone", servers: [] }));
|
||||||
|
expect(h.ref().current.activeServerName).toBe("Alpha");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("follows a rename of the active server", () => {
|
||||||
|
const h = harness(values());
|
||||||
|
h.update(values({ activeServerId: "a", servers: [entry("a", "Alpha")] }));
|
||||||
|
h.update(
|
||||||
|
values({ activeServerId: "a", servers: [entry("a", "Renamed")] }),
|
||||||
|
);
|
||||||
|
expect(h.ref().current.activeServerName).toBe("Renamed");
|
||||||
|
});
|
||||||
|
});
|
||||||
|
});
|
||||||
80
clients/web/src/hooks/useSessionRef.ts
Normal file
80
clients/web/src/hooks/useSessionRef.ts
Normal file
|
|
@ -0,0 +1,80 @@
|
||||||
|
import { useEffect, useRef } from "react";
|
||||||
|
import type { RefObject } from "react";
|
||||||
|
import type { InspectorClient } from "@inspector/core/mcp/index.js";
|
||||||
|
import type { ServerEntry } from "@inspector/core/mcp/types.js";
|
||||||
|
import type { PendingReauth } from "../utils/pendingReauth";
|
||||||
|
import type { PendingStepUp } from "../utils/stepUp";
|
||||||
|
|
||||||
|
/**
|
||||||
|
* The session values App.tsx's long-lived callbacks read *currently* rather
|
||||||
|
* than as of the render that created them.
|
||||||
|
*
|
||||||
|
* `activeServerName` is the one field with a rule of its own — see the hook.
|
||||||
|
*/
|
||||||
|
export interface SessionSnapshot {
|
||||||
|
/** Name of the last server that was active; see `useSessionRef`. */
|
||||||
|
activeServerName: string | undefined;
|
||||||
|
activeServerId: string | undefined;
|
||||||
|
servers: ServerEntry[];
|
||||||
|
/**
|
||||||
|
* The live client. Mirrored because a settings write that outlives a
|
||||||
|
* disconnect/reconnect to the *same* server would otherwise push its value
|
||||||
|
* into the instance captured when it was issued — which has since been
|
||||||
|
* destroyed — while the replacement client, built from the stale list entry,
|
||||||
|
* keeps the value the write replaced (#2089).
|
||||||
|
*/
|
||||||
|
inspectorClient: InspectorClient | null;
|
||||||
|
pendingStepUp: PendingStepUp | null;
|
||||||
|
pendingReauth: PendingReauth | null;
|
||||||
|
}
|
||||||
|
|
||||||
|
/** What the caller supplies each render; `activeServerName` is derived. */
|
||||||
|
export type SessionValues = Omit<SessionSnapshot, "activeServerName">;
|
||||||
|
|
||||||
|
/** A stable handle onto the latest {@link SessionSnapshot}. */
|
||||||
|
export type SessionRef = RefObject<SessionSnapshot>;
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Mirrors the session values into a single stable ref, so a callback can read
|
||||||
|
* the current value without listing it as a dependency and being re-created on
|
||||||
|
* every change.
|
||||||
|
*
|
||||||
|
* This is the standard "latest ref" pattern, written once instead of once per
|
||||||
|
* value — App.tsx previously kept six such refs and three sync effects. It is
|
||||||
|
* **not** a store: nothing renders off it, nothing subscribes to it, and
|
||||||
|
* mutating it never schedules a render. It is also not the prop→state sync
|
||||||
|
* that AGENTS.md forbids, which is about `setState` in an effect; a ref write
|
||||||
|
* produces no render at all, which is the whole point.
|
||||||
|
*
|
||||||
|
* Because the ref identity never changes, a hook that takes it takes a
|
||||||
|
* *stable* dependency — which is what lets App.tsx's coupled clusters be
|
||||||
|
* extracted one at a time without each one dragging the others into its
|
||||||
|
* argument list.
|
||||||
|
*
|
||||||
|
* The single non-uniform field is `activeServerName`: it holds the name of the
|
||||||
|
* last server that *was* active and is never cleared. A transport crash
|
||||||
|
* dispatches `disconnect`, which clears `activeServerId` — and therefore the
|
||||||
|
* active entry — before the failure toast is raised, so the sticky name is the
|
||||||
|
* only surviving handle on which server just died.
|
||||||
|
*/
|
||||||
|
export function useSessionRef(values: SessionValues): SessionRef {
|
||||||
|
const ref = useRef<SessionSnapshot>({
|
||||||
|
activeServerName: undefined,
|
||||||
|
...values,
|
||||||
|
});
|
||||||
|
// Deliberately un-gated: this is a passive mirror, so re-assigning the same
|
||||||
|
// values on a render that changed none of them costs nothing and removes the
|
||||||
|
// dependency list as a place for a newly-added field to be forgotten.
|
||||||
|
useEffect(() => {
|
||||||
|
const activeServer = values.servers.find(
|
||||||
|
(s) => s.id === values.activeServerId,
|
||||||
|
);
|
||||||
|
if (activeServer) ref.current.activeServerName = activeServer.name;
|
||||||
|
ref.current.activeServerId = values.activeServerId;
|
||||||
|
ref.current.servers = values.servers;
|
||||||
|
ref.current.inspectorClient = values.inspectorClient;
|
||||||
|
ref.current.pendingStepUp = values.pendingStepUp;
|
||||||
|
ref.current.pendingReauth = values.pendingReauth;
|
||||||
|
});
|
||||||
|
return ref;
|
||||||
|
}
|
||||||
|
|
@ -29,3 +29,17 @@ export function isStepUpConfirmation(
|
||||||
|
|
||||||
/** Which in-flight action opened the step-up modal (for scoped cancel UX). */
|
/** Which in-flight action opened the step-up modal (for scoped cancel UX). */
|
||||||
export type StepUpSource = "tool" | "prompt" | "resource" | "ambient" | "app";
|
export type StepUpSource = "tool" | "prompt" | "resource" | "ambient" | "app";
|
||||||
|
|
||||||
|
/**
|
||||||
|
* The in-flight step-up prompt App.tsx holds while the user completes (or
|
||||||
|
* cancels) an interactive step-up authorization. Named here beside
|
||||||
|
* `StepUpSource` rather than inline in App.tsx so `useSessionRef`'s snapshot
|
||||||
|
* can name it without importing from the component that owns the state.
|
||||||
|
*/
|
||||||
|
export interface PendingStepUp {
|
||||||
|
challenge: AuthChallenge;
|
||||||
|
authorizationUrl: URL;
|
||||||
|
serverId: string;
|
||||||
|
source: StepUpSource;
|
||||||
|
enterpriseManaged?: boolean;
|
||||||
|
}
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue