Scope app-server diagnostics to their owning panel and thread

This commit is contained in:
murashit 2026-07-10 12:10:56 +09:00
parent 2ca7dc42d2
commit d28895b214
8 changed files with 237 additions and 27 deletions

View file

@ -1,6 +1,12 @@
import { QueryClient, QueryObserver, type QueryObserverResult } from "@tanstack/query-core";
import type { ModelMetadata } from "../../domain/catalog/metadata";
import { createServerDiagnostics, diagnosticProbeError, diagnosticProbeOk, diagnosticsWithProbe } from "../../domain/server/diagnostics";
import {
createServerDiagnostics,
diagnosticProbeError,
diagnosticProbeOk,
diagnosticsWithProbe,
metadataResourceDiagnostics,
} from "../../domain/server/diagnostics";
import type { SharedServerMetadata } from "../../domain/server/metadata";
import type { Thread } from "../../domain/threads/model";
import type { AppServerClient } from "../connection/client";
@ -390,6 +396,7 @@ function metadataWithLastKnownGood(
availablePermissionProfiles:
probes.permissionProfiles.status === "ok" ? metadata.availablePermissionProfiles : (previous?.availablePermissionProfiles ?? []),
rateLimit: probes.rateLimits.status === "ok" ? metadata.rateLimit : (previous?.rateLimit ?? null),
serverDiagnostics: metadataResourceDiagnostics(metadata.serverDiagnostics),
});
}

View file

@ -12,6 +12,8 @@ const DIAGNOSTIC_PROBE_DEFINITIONS = {
mcpServers: { label: "MCP servers" },
} as const;
const METADATA_RESOURCE_PROBE_IDS = ["models", "skills", "permissionProfiles", "rateLimits"] as const;
export type DiagnosticProbeId = keyof typeof DIAGNOSTIC_PROBE_DEFINITIONS;
type DiagnosticProbeStatus = "unknown" | "ok" | "failed";
@ -64,6 +66,14 @@ export function diagnosticsWithToolInventory(diagnostics: Diagnostics, toolInven
};
}
export function metadataResourceDiagnostics(diagnostics: Diagnostics): Diagnostics {
return diagnosticsWithMetadataResourceProbes(createServerDiagnostics(), diagnostics);
}
export function diagnosticsWithMetadataResourceProbes(diagnostics: Diagnostics, metadataDiagnostics: Diagnostics): Diagnostics {
return METADATA_RESOURCE_PROBE_IDS.reduce((current, id) => diagnosticsWithProbe(current, metadataDiagnostics.probes[id]), diagnostics);
}
function createDiagnosticProbeResult(id: DiagnosticProbeId): DiagnosticProbeResult {
return {
id,
@ -125,6 +135,20 @@ export function upsertMcpServerStatusDiagnostics(diagnostics: Diagnostics, serve
return next;
}
export function replaceMcpServerStatusDiagnostics(diagnostics: Diagnostics, servers: readonly McpServerStatusSummary[]): Diagnostics {
return {
...diagnostics,
mcpServers: servers
.map((server) =>
mergeMcpServerDiagnostic(
diagnostics.mcpServers.find((item) => item.name === server.name),
mcpServerDiagnosticFromStatus(server),
),
)
.sort((a, b) => a.name.localeCompare(b.name)),
};
}
export function shortDiagnosticErrorMessage(error: unknown, maxLength = 160): string {
const message = error instanceof Error ? error.message : String(error);
const compact = message.replace(/\s+/g, " ").trim() || "Codex app-server request failed.";

View file

@ -1,8 +1,9 @@
import {
cloneServerDiagnostics,
diagnosticsWithMetadataResourceProbes,
diagnosticsWithProbe,
diagnosticsWithToolInventory,
upsertMcpServerStatusDiagnostics,
replaceMcpServerStatusDiagnostics,
} from "../../../../domain/server/diagnostics";
import type { SharedServerMetadata } from "../../../../domain/server/metadata";
import type { ChatStateStore } from "../state/store";
@ -16,18 +17,23 @@ interface RefreshServerDiagnosticsOptions {
export interface ServerDiagnosticsActionsHost {
stateStore: ChatStateStore;
diagnosticsTransport: ServerDiagnosticsTransport;
updateAppServerMetadata: (updater: (metadata: SharedServerMetadata | null) => SharedServerMetadata | null) => SharedServerMetadata | null;
appServerMetadataSnapshot: () => SharedServerMetadata | null;
}
export interface ServerDiagnosticsActions {
refreshServerDiagnostics: (options?: RefreshServerDiagnosticsOptions) => Promise<void>;
invalidate(): void;
}
export function createServerDiagnosticsActions(host: ServerDiagnosticsActionsHost): ServerDiagnosticsActions {
let generation = 0;
return {
refreshServerDiagnostics: async (options) => {
await refreshServerDiagnostics(host, options);
const currentGeneration = ++generation;
await refreshServerDiagnostics(host, options, () => currentGeneration === generation);
},
invalidate: () => {
generation += 1;
},
};
}
@ -35,8 +41,9 @@ export function createServerDiagnosticsActions(host: ServerDiagnosticsActionsHos
async function refreshServerDiagnostics(
host: ServerDiagnosticsActionsHost,
options: RefreshServerDiagnosticsOptions = {},
isCurrent: () => boolean,
): Promise<boolean> {
const initialDiagnostics = currentMetadataDiagnostics(host);
const initialDiagnostics = currentPanelDiagnostics(host);
const state = host.stateStore.getState();
const activeThreadId = state.activeThread.id;
const metadataSnapshot = host.appServerMetadataSnapshot();
@ -55,9 +62,9 @@ async function refreshServerDiagnostics(
...(cachedSkillsProbe !== undefined ? { cachedSkillsProbe } : {}),
};
const snapshot = await host.diagnosticsTransport.readServerDiagnostics(request);
if (!snapshot) return false;
if (!snapshot || !isCurrent() || host.stateStore.getState().activeThread.id !== activeThreadId) return false;
let diagnostics = currentMetadataDiagnostics(host);
let diagnostics = currentPanelDiagnostics(host);
for (const probe of snapshot.resourceProbes) {
diagnostics = diagnosticsWithProbe(diagnostics, probe);
}
@ -65,16 +72,15 @@ async function refreshServerDiagnostics(
diagnostics = diagnosticsWithProbe(diagnostics, probe);
}
if (snapshot.toolInventory.mcpServerStatuses) {
diagnostics = upsertMcpServerStatusDiagnostics(diagnostics, snapshot.toolInventory.mcpServerStatuses);
diagnostics = replaceMcpServerStatusDiagnostics(diagnostics, snapshot.toolInventory.mcpServerStatuses);
}
diagnostics = diagnosticsWithToolInventory(diagnostics, snapshot.toolInventory.inventory);
host.updateAppServerMetadata((metadata) => (metadata ? { ...metadata, serverDiagnostics: diagnostics } : null));
host.stateStore.dispatch({ type: "connection/metadata-applied", serverDiagnostics: diagnostics });
return true;
}
function currentMetadataDiagnostics(host: ServerDiagnosticsActionsHost): SharedServerMetadata["serverDiagnostics"] {
return (
host.appServerMetadataSnapshot()?.serverDiagnostics ?? cloneServerDiagnostics(host.stateStore.getState().connection.serverDiagnostics)
);
function currentPanelDiagnostics(host: ServerDiagnosticsActionsHost): SharedServerMetadata["serverDiagnostics"] {
const current = cloneServerDiagnostics(host.stateStore.getState().connection.serverDiagnostics);
const metadata = host.appServerMetadataSnapshot();
return metadata ? diagnosticsWithMetadataResourceProbes(current, metadata.serverDiagnostics) : current;
}

View file

@ -1,4 +1,9 @@
import { cloneServerDiagnostics, diagnosticsWithProbe, upsertMcpServerDiagnostic } from "../../../../domain/server/diagnostics";
import {
cloneServerDiagnostics,
diagnosticsWithMetadataResourceProbes,
diagnosticsWithProbe,
upsertMcpServerDiagnostic,
} from "../../../../domain/server/diagnostics";
import type { McpServerStartupStatus } from "../../../../domain/server/mcp-status";
import type { SharedServerMetadata } from "../../../../domain/server/metadata";
import type { ChatStateStore } from "../state/store";
@ -54,6 +59,10 @@ async function applyAppServerResourceEvent(host: ServerMetadataActionsHost, even
}
function applyAppServerMetadata(host: ServerMetadataActionsHost, metadata: SharedServerMetadata): void {
const serverDiagnostics = diagnosticsWithMetadataResourceProbes(
cloneServerDiagnostics(host.stateStore.getState().connection.serverDiagnostics),
metadata.serverDiagnostics,
);
host.stateStore.dispatch({
type: "connection/metadata-applied",
runtimeConfig: metadata.runtimeConfig,
@ -61,7 +70,7 @@ function applyAppServerMetadata(host: ServerMetadataActionsHost, metadata: Share
availableSkills: metadata.availableSkills,
availablePermissionProfiles: metadata.availablePermissionProfiles,
rateLimit: metadata.rateLimit,
serverDiagnostics: metadata.serverDiagnostics,
serverDiagnostics,
});
}
@ -164,7 +173,6 @@ function applyMcpStartupStatusEvent(
toolCount: null,
message,
});
host.updateAppServerMetadata((metadata) => (metadata ? { ...metadata, serverDiagnostics: diagnostics } : null));
host.stateStore.dispatch({
type: "connection/metadata-applied",
serverDiagnostics: diagnostics,
@ -172,7 +180,7 @@ function applyMcpStartupStatusEvent(
}
function currentMetadataDiagnostics(host: ServerMetadataActionsHost): SharedServerMetadata["serverDiagnostics"] {
return (
host.appServerMetadataSnapshot()?.serverDiagnostics ?? cloneServerDiagnostics(host.stateStore.getState().connection.serverDiagnostics)
);
const current = cloneServerDiagnostics(host.stateStore.getState().connection.serverDiagnostics);
const metadata = host.appServerMetadataSnapshot();
return metadata ? diagnosticsWithMetadataResourceProbes(current, metadata.serverDiagnostics) : current;
}

View file

@ -57,7 +57,7 @@ export interface ChatPanelConnectionBundle {
sharedStateActions: {
applyAppServerMetadata: (metadata: SharedServerMetadata) => void;
};
clearServerRequestResponders: () => void;
invalidateConnectionScope: () => void;
refreshSharedThreads: () => Promise<void>;
}
@ -139,7 +139,6 @@ export function createConnectionBundle(
const serverDiagnostics = createServerDiagnosticsActions({
stateStore,
diagnosticsTransport: appServer.serverDiagnostics,
updateAppServerMetadata: (updater) => environment.plugin.appServerQueries.updateAppServerMetadata(updater),
appServerMetadataSnapshot: () => environment.plugin.appServerQueries.appServerMetadataSnapshot(),
});
const refreshSharedThreads = async (): Promise<void> => {
@ -212,6 +211,7 @@ export function createConnectionBundle(
},
onExit: () => {
serverRequestResponders.clear();
serverDiagnostics.invalidate();
handleChatConnectionExit(connectionExitHost);
},
}),
@ -266,8 +266,9 @@ export function createConnectionBundle(
serverMetadata.applyAppServerMetadata(metadata);
},
},
clearServerRequestResponders: () => {
invalidateConnectionScope: () => {
serverRequestResponders.clear();
serverDiagnostics.invalidate();
},
refreshSharedThreads,
};

View file

@ -191,7 +191,7 @@ export function createChatPanelSessionGraph(host: ChatPanelSessionGraphHost): Ch
host.deferredTasks.clearDiagnostics();
},
resetConnection: () => {
connectionBundle.clearServerRequestResponders();
connectionBundle.invalidateConnectionScope();
connection.resetConnection();
},
setStatus: (statusText, phase) => {
@ -274,7 +274,7 @@ export function createChatPanelSessionGraph(host: ChatPanelSessionGraphHost): Ch
refreshSharedThreads,
startNewThread: () => threadActions.navigation.startNewThread(),
dispose: () => {
connectionBundle.clearServerRequestResponders();
connectionBundle.invalidateConnectionScope();
shell.dispose();
composerController.dispose();
},

View file

@ -11,6 +11,8 @@ import {
diagnosticProbeError,
diagnosticProbeOk,
diagnosticsWithProbe,
diagnosticsWithToolInventory,
upsertMcpServerDiagnostic,
} from "../../src/domain/server/diagnostics";
import type { SharedServerMetadata } from "../../src/domain/server/metadata";
@ -130,6 +132,47 @@ describe("AppServerQueryCache", () => {
expect(cache.modelsSnapshot(context)).toBeNull();
});
it("keeps connection and thread diagnostics out of shared metadata snapshots", () => {
const cache = new AppServerQueryCache();
const context = cacheContext();
const polluted = metadata();
polluted.serverDiagnostics = diagnosticsWithToolInventory(
upsertMcpServerDiagnostic(polluted.serverDiagnostics, {
name: "github",
startupStatus: "ready",
authStatus: null,
toolCount: 1,
message: null,
}),
{
checkedAt: 1,
plugins: [],
pluginMarketplaceErrors: [],
pluginsError: null,
mcpServers: [],
mcpDiagnostics: [],
mcpError: null,
skills: [],
skillsError: null,
},
);
cache.writeAppServerMetadata(context, polluted);
expect(cache.appServerMetadataSnapshot(context)?.serverDiagnostics).toMatchObject({
mcpServers: [],
toolInventory: null,
probes: {
models: { status: "ok" },
skills: { status: "ok" },
permissionProfiles: { status: "ok" },
rateLimits: { status: "ok" },
plugins: { status: "unknown" },
mcpServers: { status: "unknown" },
},
});
});
it("does not share or store snapshots before the cache context is complete", async () => {
const cache = new AppServerQueryCache();
const context = cacheContext({ codexPath: "" });

View file

@ -9,6 +9,8 @@ import {
diagnosticProbeError,
diagnosticProbeOk,
diagnosticsWithProbe,
diagnosticsWithToolInventory,
upsertMcpServerDiagnostic,
} from "../../../../../src/domain/server/diagnostics";
import type { McpServerStatusSummary } from "../../../../../src/domain/server/mcp-status";
import type { SharedServerMetadata } from "../../../../../src/domain/server/metadata";
@ -18,6 +20,7 @@ import { createServerDiagnosticsActions } from "../../../../../src/features/chat
import { createServerMetadataActions } from "../../../../../src/features/chat/application/connection/server-metadata-actions";
import { createChatStateStore } from "../../../../../src/features/chat/application/state/store";
import { toolInventoryDiagnosticSections } from "../../../../../src/features/chat/presentation/runtime/tool-inventory-diagnostic-sections";
import { deferred } from "../../../../support/async";
import { chatStateFixture, chatStateWith } from "../../support/state";
describe("server metadata actions", () => {
@ -37,6 +40,60 @@ describe("server metadata actions", () => {
expect(stateStore.getState().connection.availableModels.map((model) => model.model)).toEqual(["gpt-5.1"]);
});
it("preserves panel-local tool diagnostics when applying shared metadata", async () => {
const localDiagnostics = diagnosticsWithToolInventory(
upsertMcpServerDiagnostic(createServerDiagnostics(), {
name: "github",
startupStatus: "ready",
authStatus: null,
toolCount: null,
message: null,
}),
toolInventory(),
);
const stateStore = createChatStateStore(chatStateFixture({ connection: { serverDiagnostics: localDiagnostics } }));
const metadata = serverMetadataFixture({
serverDiagnostics: diagnosticsWithProbe(createServerDiagnostics(), diagnosticProbeOk("models", "1 model", 2)),
});
const actions = createServerMetadataActions({
stateStore,
metadataResourceTransport: metadataResourceTransport(),
...metadataCacheHost(),
refreshAppServerMetadata: vi.fn().mockResolvedValue(metadata),
isStaleSharedQueryError: () => false,
});
await actions.refreshAppServerMetadata();
expect(stateStore.getState().connection.serverDiagnostics).toMatchObject({
probes: { models: { status: "ok", summary: "1 model" } },
mcpServers: [{ name: "github", startupStatus: "ready" }],
toolInventory: { checkedAt: 1 },
});
});
it("keeps MCP startup notifications out of shared metadata", async () => {
const stateStore = createChatStateStore(chatStateFixture());
const cache = { current: serverMetadataFixture() as SharedServerMetadata | null };
const actions = createServerMetadataActions({
stateStore,
metadataResourceTransport: metadataResourceTransport(),
...metadataCacheHost(cache),
refreshAppServerMetadata: vi.fn().mockResolvedValue(null),
isStaleSharedQueryError: () => false,
});
await actions.applyAppServerResourceEvent({
type: "mcp-startup-status-updated",
name: "github",
status: "ready",
message: null,
});
expect(stateStore.getState().connection.serverDiagnostics.mcpServers).toMatchObject([{ name: "github", startupStatus: "ready" }]);
expect(cache.current?.serverDiagnostics.mcpServers).toEqual([]);
});
it("ignores stale shared app-server metadata refreshes without applying state", async () => {
const stateStore = createChatStateStore(chatStateFixture());
const stale = new Error("stale");
@ -244,18 +301,15 @@ describe("server diagnostics actions", () => {
it("does not apply diagnostic probes when the transport returns no snapshot", async () => {
const stateStore = createChatStateStore(chatStateFixture());
const updateAppServerMetadata = vi.fn(() => null);
const diagnostics = createServerDiagnosticsActions({
stateStore,
diagnosticsTransport: { readServerDiagnostics: vi.fn().mockResolvedValue(null) },
appServerMetadataSnapshot: () => null,
updateAppServerMetadata,
});
await diagnostics.refreshServerDiagnostics({ appServerMetadataSnapshot: true });
expect(stateStore.getState().connection.serverDiagnostics.probes.mcpServers.status).toBe("unknown");
expect(updateAppServerMetadata).not.toHaveBeenCalled();
});
it("refreshes tool provider snapshots with cached MCP startup diagnostics", async () => {
@ -296,6 +350,73 @@ describe("server diagnostics actions", () => {
expect(sections.map((section) => section.title)).toEqual(["Plugins", "Tool providers", "Skills"]);
expect(toolProviderRows.map((row) => `${row.label}: ${row.value}`)).toEqual(["github: MCP server, ready, auth oAuth, 1 tool"]);
});
it("drops a diagnostics result when its active thread is no longer current", async () => {
const pending = deferred<ReturnType<typeof serverDiagnosticsSnapshot>>();
const stateStore = createChatStateStore(chatStateFixture({ activeThread: { id: "thread-1" } }));
const diagnostics = createServerDiagnosticsActions({
stateStore,
diagnosticsTransport: { readServerDiagnostics: vi.fn(() => pending.promise) },
...metadataCacheHost(),
});
const refreshing = diagnostics.refreshServerDiagnostics({ appServerMetadataSnapshot: true });
stateStore.dispatch({ type: "active-thread/cleared" });
pending.resolve(serverDiagnosticsSnapshot({ mcpServerStatuses: [mcpServerStatus()] }));
await refreshing;
expect(stateStore.getState().connection.serverDiagnostics.toolInventory).toBeNull();
expect(stateStore.getState().connection.serverDiagnostics.mcpServers).toEqual([]);
});
it("drops a diagnostics result after its connection scope is invalidated", async () => {
const pending = deferred<ReturnType<typeof serverDiagnosticsSnapshot>>();
const stateStore = createChatStateStore(chatStateFixture({ activeThread: { id: "thread-1" } }));
const diagnostics = createServerDiagnosticsActions({
stateStore,
diagnosticsTransport: { readServerDiagnostics: vi.fn(() => pending.promise) },
...metadataCacheHost(),
});
const refreshing = diagnostics.refreshServerDiagnostics({ appServerMetadataSnapshot: true });
diagnostics.invalidate();
pending.resolve(serverDiagnosticsSnapshot({ mcpServerStatuses: [mcpServerStatus()] }));
await refreshing;
expect(stateStore.getState().connection.serverDiagnostics.toolInventory).toBeNull();
expect(stateStore.getState().connection.serverDiagnostics.mcpServers).toEqual([]);
});
it("removes MCP providers missing from the latest thread-scoped inventory", async () => {
let initialDiagnostics = upsertMcpServerDiagnostic(createServerDiagnostics(), {
name: "github",
startupStatus: "ready",
authStatus: null,
toolCount: null,
message: null,
});
initialDiagnostics = upsertMcpServerDiagnostic(initialDiagnostics, {
name: "removed-provider",
startupStatus: "ready",
authStatus: null,
toolCount: null,
message: null,
});
const stateStore = createChatStateStore(
chatStateFixture({ activeThread: { id: "thread-1" }, connection: { serverDiagnostics: initialDiagnostics } }),
);
const diagnostics = createServerDiagnosticsActions({
stateStore,
diagnosticsTransport: {
readServerDiagnostics: vi.fn().mockResolvedValue(serverDiagnosticsSnapshot({ mcpServerStatuses: [mcpServerStatus()] })),
},
...metadataCacheHost(),
});
await diagnostics.refreshServerDiagnostics({ appServerMetadataSnapshot: true });
expect(stateStore.getState().connection.serverDiagnostics.mcpServers.map((server) => server.name)).toEqual(["github"]);
});
});
function metadataResourceTransport(overrides: Partial<MetadataResourceTransport> = {}): MetadataResourceTransport {