From 1f95937dcf65eb4af53b176c8780b747a7c50da0 Mon Sep 17 00:00:00 2001 From: murashit Date: Sun, 28 Jun 2026 15:57:45 +0900 Subject: [PATCH] Preserve app-server metadata on probe failures --- src/app-server/query/cache.ts | 9 +++++--- .../chat/app-server/actions/metadata.ts | 16 +++++++++----- tests/app-server/query-cache.test.ts | 15 +++++++------ .../chat/app-server/actions/actions.test.ts | 22 +++++++++++++++++++ 4 files changed, 46 insertions(+), 16 deletions(-) diff --git a/src/app-server/query/cache.ts b/src/app-server/query/cache.ts index 5e74664b..d654dd01 100644 --- a/src/app-server/query/cache.ts +++ b/src/app-server/query/cache.ts @@ -188,13 +188,16 @@ export class AppServerQueryCache { writeAppServerMetadata(context: AppServerQueryContext, metadata: SharedServerMetadata): SharedServerMetadata | null { if (!appServerQueryContextIsComplete(context)) return null; + const previous = this.appServerMetadataSnapshot(context); + const probes = metadata.serverDiagnostics.probes; const next = cloneSharedServerMetadata({ ...metadata, - availableModels: - metadata.serverDiagnostics.probes["model/list"].status === "ok" ? metadata.availableModels : (this.modelsSnapshot(context) ?? []), + availableModels: probes["model/list"].status === "ok" ? metadata.availableModels : (this.modelsSnapshot(context) ?? []), + availableSkills: probes["skills/list"].status === "ok" ? metadata.availableSkills : (previous?.availableSkills ?? []), + rateLimit: probes["account/rateLimits/read"].status === "ok" ? metadata.rateLimit : (previous?.rateLimit ?? null), }); this.client.setQueryData(appServerMetadataQueryKey(context), cloneSharedServerMetadata(next)); - if (metadata.serverDiagnostics.probes["model/list"].status === "ok") { + if (probes["model/list"].status === "ok") { this.client.setQueryData(appServerModelsQueryKey(context), cloneModelMetadata(next.availableModels)); } return cloneSharedServerMetadata(next); diff --git a/src/features/chat/app-server/actions/metadata.ts b/src/features/chat/app-server/actions/metadata.ts index 62a28ebf..f86965fb 100644 --- a/src/features/chat/app-server/actions/metadata.ts +++ b/src/features/chat/app-server/actions/metadata.ts @@ -96,7 +96,7 @@ async function refreshSkillResource(host: ChatServerMetadataActionsHost, forceRe if (!metadata) return null; return { ...metadata, - availableSkills: skills.value, + ...(skills.probe.status === "ok" ? { availableSkills: skills.value } : {}), serverDiagnostics: diagnosticsWithProbe(cloneServerDiagnostics(metadata.serverDiagnostics), skills.probe), }; }); @@ -105,11 +105,15 @@ async function refreshSkillResource(host: ChatServerMetadataActionsHost, forceRe return next; } const diagnostics = diagnosticsWithProbe(currentMetadataDiagnostics(host), skills.probe); - host.stateStore.dispatch({ - type: "connection/metadata-applied", - availableSkills: skills.value, - serverDiagnostics: diagnostics, - }); + host.stateStore.dispatch( + skills.probe.status === "ok" + ? { + type: "connection/metadata-applied", + availableSkills: skills.value, + serverDiagnostics: diagnostics, + } + : { type: "connection/metadata-applied", serverDiagnostics: diagnostics }, + ); return null; } diff --git a/tests/app-server/query-cache.test.ts b/tests/app-server/query-cache.test.ts index bb31a8e6..5844b779 100644 --- a/tests/app-server/query-cache.test.ts +++ b/tests/app-server/query-cache.test.ts @@ -14,7 +14,7 @@ import { import type { SharedServerMetadata } from "../../src/domain/server/metadata"; describe("AppServerQueryCache", () => { - it("stores metadata snapshots without replacing failed resource values with stale values", () => { + it("preserves successful resource values when metadata probes fail", () => { const cache = new AppServerQueryCache(); const context = cacheContext(); const goodMetadata = metadata({ @@ -29,7 +29,7 @@ describe("AppServerQueryCache", () => { context, metadata({ availableModels: [modelMetadata("gpt-5.6")], - availableSkills: [skillMetadata("stale-skill")], + availableSkills: [skillMetadata("failed-skill")], rateLimit: rateLimit(90), skillsProbeStatus: "failed", rateLimitProbeStatus: "failed", @@ -37,8 +37,8 @@ describe("AppServerQueryCache", () => { ); expect(cache.appServerMetadataSnapshot(context)?.availableModels.map((model) => model.model)).toEqual(["gpt-5.6"]); - expect(cache.appServerMetadataSnapshot(context)?.availableSkills.map((skill) => skill.name)).toEqual(["stale-skill"]); - expect(cache.appServerMetadataSnapshot(context)?.rateLimit?.primary?.usedPercent).toBe(90); + expect(cache.appServerMetadataSnapshot(context)?.availableSkills.map((skill) => skill.name)).toEqual(["writer"]); + expect(cache.appServerMetadataSnapshot(context)?.rateLimit?.primary?.usedPercent).toBe(42); expect(cache.appServerMetadataSnapshot(context)?.serverDiagnostics.probes["skills/list"].status).toBe("failed"); expect(cache.appServerMetadataSnapshot(context)?.serverDiagnostics.probes["account/rateLimits/read"].status).toBe("failed"); @@ -47,7 +47,7 @@ describe("AppServerQueryCache", () => { expect(cache.appServerMetadataSnapshot(context)?.serverDiagnostics.probes["model/list"].status).toBe("failed"); }); - it("does not accept failed metadata model payloads as model cache truth", () => { + it("does not accept failed metadata resource payloads as cache truth", () => { const cache = new AppServerQueryCache(); const context = cacheContext(); @@ -58,6 +58,7 @@ describe("AppServerQueryCache", () => { availableSkills: [skillMetadata("writer")], rateLimit: rateLimit(90), modelProbeStatus: "failed", + skillsProbeStatus: "failed", rateLimitProbeStatus: "failed", }), ); @@ -66,8 +67,8 @@ describe("AppServerQueryCache", () => { expect(cached?.runtimeConfig).not.toBeNull(); expect(cached?.serverDiagnostics.probes["model/list"].status).toBe("failed"); expect(cached?.availableModels).toEqual([]); - expect(cached?.availableSkills.map((skill) => skill.name)).toEqual(["writer"]); - expect(cached?.rateLimit?.primary?.usedPercent).toBe(90); + expect(cached?.availableSkills).toEqual([]); + expect(cached?.rateLimit).toBeNull(); expect(cache.modelsSnapshot(context)).toBeNull(); }); diff --git a/tests/features/chat/app-server/actions/actions.test.ts b/tests/features/chat/app-server/actions/actions.test.ts index b73c5671..442187df 100644 --- a/tests/features/chat/app-server/actions/actions.test.ts +++ b/tests/features/chat/app-server/actions/actions.test.ts @@ -504,6 +504,28 @@ describe("chat app-server actions", () => { expect(updateAppServerMetadata).not.toHaveBeenCalled(); }); + it("keeps previous skills when sparse skill refresh fails", async () => { + let state = chatStateFixture(); + const previousSkills = [{ name: "writer", description: "", path: "/tmp/writer", enabled: true }]; + state = chatStateWith(state, { connection: { availableSkills: previousSkills } }); + const stateStore = createChatStateStore(state); + const listSkills = vi.fn().mockRejectedValue(new Error("offline")); + const client = requestClient({ "skills/list": listSkills }); + const controller = createChatServerMetadataActions({ + stateStore, + vaultPath: "/vault", + currentClient: () => client, + ...metadataCacheHost(), + refreshAppServerMetadata: async () => null, + }); + + await controller.applyAppServerResourceEvent({ type: "skills-changed", forceReload: true }); + + expect(listSkills).toHaveBeenCalledWith({ cwds: ["/vault"], forceReload: true }); + expect(stateStore.getState().connection.availableSkills).toEqual(previousSkills); + expect(stateStore.getState().connection.serverDiagnostics.probes["skills/list"]).toMatchObject({ status: "failed" }); + }); + it("publishes refreshed rate limits from sparse update notifications", async () => { const state = chatStateFixture(); const stateStore = createChatStateStore(state);