diff --git a/src/agents/prepared-model-runtime-materializations.test.ts b/src/agents/prepared-model-runtime-materializations.test.ts new file mode 100644 index 000000000000..1b7d904d38ae --- /dev/null +++ b/src/agents/prepared-model-runtime-materializations.test.ts @@ -0,0 +1,106 @@ +import { afterEach, describe, expect, it } from "vitest"; +import { + clearAllRuntimeAuthMaterializations, + recordRuntimeAuthMaterialization, +} from "./auth-profiles/runtime-materializations.js"; +import { getPreparedModelRuntimeAuthMaterializations } from "./prepared-model-runtime-auth.js"; +import { registerPreparedRuntimeAuthMaterializationPublisher } from "./prepared-model-runtime-materializations.js"; +import type { + PreparedModelRuntimeOwner, + PreparedModelRuntimeSnapshot, +} from "./prepared-model-runtime.types.js"; + +function createOwner(params: { + agentId: string; + agentDir: string; + needsRefresh?: boolean; +}): PreparedModelRuntimeOwner { + const snapshot = { + agentId: params.agentId, + agentDir: params.agentDir, + config: {}, + authModes: {}, + activeProjectKeys: [], + allowGatewaySubagentBinding: true, + metadataSnapshot: { index: { plugins: [] }, plugins: [] }, + modelCatalog: { entries: [], routeVariants: [] }, + configuredRuntimeModels: [], + inlineProviderModels: [], + createStores: () => ({ authStorage: { getAll: () => ({}) }, modelRegistry: {} }), + } as unknown as PreparedModelRuntimeSnapshot; + return { + input: { agentId: params.agentId, agentDir: params.agentDir, config: {} }, + environmentFingerprint: "test-env", + catalogMode: "static", + provenance: "configured", + generation: 1, + needsRefresh: params.needsRefresh === true, + snapshot, + }; +} + +const materialization = { + provider: "openai", + modelId: "gpt-5.4", + modelApi: "openai-chatgpt-responses", + modelBaseUrl: "https://chatgpt.com/backend-api/codex", + requestTransportOverrides: "none" as const, + authMode: "oauth", + runtimeOwnerId: "codex", +}; + +afterEach(() => { + clearAllRuntimeAuthMaterializations(); +}); + +describe("prepared model runtime auth materialization publication", () => { + it("does not announce published while a sibling configured owner is stale", () => { + const main = createOwner({ agentId: "main", agentDir: "/tmp/configured-main" }); + const atlas = createOwner({ + agentId: "atlas", + agentDir: "/tmp/configured-atlas", + needsRefresh: true, + }); + const owners = new Map([ + ["main", main], + ["atlas", atlas], + ]); + const phases: string[] = []; + const unregister = registerPreparedRuntimeAuthMaterializationPublisher(owners, (event) => { + phases.push(event.phase); + }); + + expect( + recordRuntimeAuthMaterialization({ + ...materialization, + agentDir: "/tmp/configured-main", + }), + ).toBe(true); + expect(phases).toEqual([]); + expect(getPreparedModelRuntimeAuthMaterializations(main.snapshot!)).toEqual([ + expect.objectContaining({ + provider: "openai", + runtimeOwnerId: "codex", + }), + ]); + unregister(); + }); + + it("announces publication when every configured owner is request-visible", () => { + const main = createOwner({ agentId: "main", agentDir: "/tmp/configured-main" }); + const owners = new Map([["main", main]]); + const phases: string[] = []; + const unregister = registerPreparedRuntimeAuthMaterializationPublisher(owners, (event) => { + phases.push(event.phase); + }); + + expect( + recordRuntimeAuthMaterialization({ + ...materialization, + agentDir: "/tmp/configured-main", + }), + ).toBe(true); + expect(phases).toEqual(["invalidated", "published"]); + unregister(); + }); +}); diff --git a/src/agents/prepared-model-runtime-materializations.ts b/src/agents/prepared-model-runtime-materializations.ts index 6f9946c1377b..e40fb3aeab22 100644 --- a/src/agents/prepared-model-runtime-materializations.ts +++ b/src/agents/prepared-model-runtime-materializations.ts @@ -14,6 +14,20 @@ type MaterializationMutationEvent = { affectsInheritedStores: boolean; }; +function configuredOwnersAreRequestVisible( + owners: ReadonlyMap, +): boolean { + for (const owner of owners.values()) { + if (owner.provenance !== "configured") { + continue; + } + if (!owner.snapshot || owner.needsRefresh || owner.pending) { + return false; + } + } + return true; +} + export function registerPreparedRuntimeAuthMaterializationPublisher( owners: ReadonlyMap, notify: (event: { phase: "invalidated" | "published" }) => void, @@ -51,7 +65,6 @@ function publishPreparedRuntimeAuthMaterializations(params: { if (affectedOwners.length === 0) { return; } - params.onInvalidated(); const read = params.read ?? getPreparedRuntimeAuthMaterializations; for (const { owner, snapshot } of affectedOwners) { // A successful route only changes this bounded secret-free fact set. Rebuilding the model @@ -61,5 +74,12 @@ function publishPreparedRuntimeAuthMaterializations(params: { Object.freeze([...read(owner.input.agentDir)]), ); } + // Chat metadata treats published as "every configured owner is capturable". + // A bind on one agent must not announce while a sibling is stale or a replacement + // still holds needsRefresh; that refresh fail-closes the Control UI picker. + if (!configuredOwnersAreRequestVisible(params.owners)) { + return; + } + params.onInvalidated(); params.onPublished(); } diff --git a/src/gateway/server-methods/chat-metadata-runtime.test.ts b/src/gateway/server-methods/chat-metadata-runtime.test.ts index 5dd514c0c9f1..a83005be7d21 100644 --- a/src/gateway/server-methods/chat-metadata-runtime.test.ts +++ b/src/gateway/server-methods/chat-metadata-runtime.test.ts @@ -70,7 +70,7 @@ function createHarness( let pluginRegistryVersion = 1; let authStore: AuthProfileStore | undefined = { version: 1, profiles: {} }; let authStoreRevision = 1; - const getPreparedOwner = vi.fn(() => owner); + const getPreparedOwner = vi.fn((): PreparedModelRuntimeSnapshot | undefined => owner); const getPreparedAuthStore = vi.fn(() => authStore); const getAuthStoreRevision = vi.fn(() => authStoreRevision); const getSkillsVersion = vi.fn(() => skillsVersion); @@ -433,7 +433,7 @@ describe("gateway chat metadata runtime", () => { const harness = createHarness(undefined, { useDefaultProjection: true }); harness.setAuthStore({ version: 1, profiles: {} }); const preparedOwner = createOwner( - harness.getPreparedOwner().config, + harness.getPreparedOwner()!.config, "gpt-5.4", { openai: { @@ -701,6 +701,30 @@ describe("gateway chat metadata runtime", () => { expect(result).not.toBe(timedOut); }); + test("retries an unavailable owner on the next read once it is published again", async () => { + const harness = createHarness(); + await harness.runtime.refresh(); + + harness.getPreparedOwner.mockReturnValue(undefined); + await expect(harness.runtime.refresh()).rejects.toThrow( + 'prepared chat metadata owner is unavailable for agent "main"', + ); + await expect(harness.runtime.read({ agentId: "main" })).rejects.toThrow( + 'prepared chat metadata owner is unavailable for agent "main"', + ); + + const recovered = createOwner( + { agents: { list: [{ id: "main", default: true }] } }, + "recovered", + ); + harness.setOwner(recovered); + harness.getPreparedOwner.mockReturnValue(recovered); + + await expect(harness.runtime.read({ agentId: "main" })).resolves.toMatchObject({ + models: [expect.objectContaining({ id: "recovered" })], + }); + }); + test("rejects replacement waiters on failure and recovers on a later generation", async () => { const harness = createHarness(); await harness.runtime.refresh(); diff --git a/src/gateway/server-methods/chat-metadata-runtime.ts b/src/gateway/server-methods/chat-metadata-runtime.ts index 5aa63d6955a1..9485163b8b50 100644 --- a/src/gateway/server-methods/chat-metadata-runtime.ts +++ b/src/gateway/server-methods/chat-metadata-runtime.ts @@ -472,7 +472,10 @@ export function createGatewayChatMetadataRuntime(params: { continue; } let generation = current; - if (!generation && params.refreshOnRead) { + // Unavailable means the prepared owner was missing, not that publication failed. + // Retry capture so a later published owner is not hidden behind lastError. + const retryUnavailableOwner = lastError instanceof ChatMetadataSnapshotUnavailableError; + if (!generation && (params.refreshOnRead || retryUnavailableOwner)) { await refresh(); generation = current; } diff --git a/src/gateway/server.chat-metadata-boundary.test.ts b/src/gateway/server.chat-metadata-boundary.test.ts new file mode 100644 index 000000000000..f387e7b88159 --- /dev/null +++ b/src/gateway/server.chat-metadata-boundary.test.ts @@ -0,0 +1,177 @@ +import fs from "node:fs/promises"; +import path from "node:path"; +import { afterAll, beforeAll, beforeEach, expect, test, vi } from "vitest"; +import { + clearConfigCache, + clearRuntimeConfigSnapshot, + getRuntimeConfig, +} from "../config/config.js"; +import { captureEnv, setTestEnvValue } from "../test-utils/env.js"; +import { installGatewayTestHooks, rpcReq, startConnectedServerWithClient } from "./test-helpers.js"; + +installGatewayTestHooks({ scope: "suite" }); + +type ConnectedGateway = Awaited>; + +let gateway: ConnectedGateway | undefined; +let minimalGatewayEnv: ReturnType | undefined; + +function requireGateway(): ConnectedGateway { + if (!gateway) { + throw new Error("chat metadata Gateway is not ready"); + } + return gateway; +} + +beforeAll(async () => { + minimalGatewayEnv = captureEnv(["OPENCLAW_TEST_MINIMAL_GATEWAY"]); + // The production lifecycle has no refresh-on-read escape hatch. This must stay non-minimal, + // otherwise the old sticky behavior is hidden by the test-only lifecycle configuration. + setTestEnvValue("OPENCLAW_TEST_MINIMAL_GATEWAY", "0"); + await writeGatewayConfig(CHAT_METADATA_BOUNDARY_CONFIG); + gateway = await startConnectedServerWithClient(); + await gateway.server.startupSettled; +}, 60_000); + +beforeEach(async () => { + setTestEnvValue("OPENCLAW_TEST_MINIMAL_GATEWAY", "0"); + await writeGatewayConfig(CHAT_METADATA_BOUNDARY_CONFIG); + const { refreshPreparedModelRuntimeSnapshots } = + await import("../agents/prepared-model-runtime.js"); + await refreshPreparedModelRuntimeSnapshots(getRuntimeConfig(), { gatewayLifecycle: true }); + const ready = await rpcReq(requireGateway().ws, "chat.metadata", { agentId: "main" }); + expect(ready.ok, JSON.stringify(ready)).toBe(true); +}); + +afterAll(async () => { + if (gateway) { + gateway.ws.close(); + await gateway.server.close(); + gateway.envSnapshot.restore(); + } + clearConfigCache(); + minimalGatewayEnv?.restore(); +}); + +const CHAT_METADATA_BOUNDARY_CONFIG = { + agents: { + defaults: { + model: { primary: "openai/gpt-boundary" }, + models: { "openai/gpt-boundary": {} }, + }, + entries: { main: { default: true } }, + }, + models: { + providers: { + openai: { + baseUrl: "https://openai.example.com/v1", + models: [{ id: "gpt-boundary", name: "GPT Boundary" }], + }, + }, + }, +} as const; + +const CHAT_METADATA_MISSING_OWNER_CONFIG = { + ...CHAT_METADATA_BOUNDARY_CONFIG, + agents: { + ...CHAT_METADATA_BOUNDARY_CONFIG.agents, + entries: { + ...CHAT_METADATA_BOUNDARY_CONFIG.agents.entries, + missing: {}, + }, + }, +} as const; + +async function writeGatewayConfig( + config: Record, + options: { clearRuntimeSnapshot?: boolean } = {}, +) { + const configPath = process.env.OPENCLAW_CONFIG_PATH; + if (!configPath) { + throw new Error("OPENCLAW_CONFIG_PATH missing in gateway test environment"); + } + await fs.mkdir(path.dirname(configPath), { recursive: true }); + await fs.writeFile(configPath, JSON.stringify(config, null, 2), "utf-8"); + clearConfigCache(); + if (options.clearRuntimeSnapshot) { + clearRuntimeConfigSnapshot(); + } +} + +test("chat.metadata retries owner misses without broadly retrying cached failures", async () => { + const ws = requireGateway().ws; + const publicationEvents = await import("../agents/prepared-model-runtime.publication-events.js"); + const initial = await rpcReq(ws, "chat.metadata", { agentId: "main" }); + expect(initial.ok).toBe(true); + + // This is the lifecycle listener's real published catch-up. Config can expose an agent before + // its prepared owner exists, reproducing the stale publication announcement that wedged UI. + await writeGatewayConfig(CHAT_METADATA_MISSING_OWNER_CONFIG, { clearRuntimeSnapshot: true }); + publicationEvents.notifyPreparedModelRuntimePublication({ phase: "published" }); + let unavailable: Awaited> | undefined; + await vi.waitFor( + async () => { + unavailable = await rpcReq(ws, "chat.metadata", { agentId: "main" }); + expect(unavailable.ok).toBe(false); + }, + { interval: 1, timeout: 2_000 }, + ); + expect(unavailable).toMatchObject({ + ok: false, + error: { + code: "UNAVAILABLE", + message: expect.stringContaining("prepared chat metadata owner is unavailable"), + }, + }); + + await writeGatewayConfig(CHAT_METADATA_BOUNDARY_CONFIG, { clearRuntimeSnapshot: true }); + const recovered = await rpcReq<{ + models?: Array<{ id?: string; provider?: string }>; + }>(ws, "chat.metadata", { agentId: "main" }); + + // On the merge-base this remains false: readCurrent rethrows the cached unavailable error. + expect(recovered.ok).toBe(true); + expect(recovered.payload?.models).toEqual( + expect.arrayContaining([expect.objectContaining({ id: "gpt-boundary", provider: "openai" })]), + ); + + // Reset the published runtime between boundary cases without restarting the Gateway. + const { refreshPreparedModelRuntimeSnapshots } = + await import("../agents/prepared-model-runtime.js"); + await refreshPreparedModelRuntimeSnapshots(getRuntimeConfig(), { gatewayLifecycle: true }); + const reset = await rpcReq(ws, "chat.metadata", { agentId: "main" }); + expect(reset.ok).toBe(true); + + const modelsListResult = await import("./server-methods/models-list-result.js"); + const projectionFailure = new Error("configured model catalog unavailable"); + const projectionSpy = vi + .spyOn(modelsListResult, "buildModelsListResult") + .mockRejectedValue(projectionFailure); + + publicationEvents.notifyPreparedModelRuntimePublication({ phase: "invalidated" }); + publicationEvents.notifyPreparedModelRuntimePublication({ phase: "published" }); + await vi.waitFor(() => expect(projectionSpy).toHaveBeenCalled(), { + interval: 1, + timeout: 2_000, + }); + const projectionUnavailable = await rpcReq(ws, "chat.metadata", { agentId: "main" }); + expect(projectionUnavailable).toMatchObject({ + ok: false, + error: { + code: "UNAVAILABLE", + message: expect.stringContaining("configured model catalog unavailable"), + }, + }); + + projectionSpy.mockRestore(); + const stillUnavailable = await rpcReq(ws, "chat.metadata", { agentId: "main" }); + + // A broad retry would turn this into a false recovery and hide a genuinely broken catalog. + expect(stillUnavailable).toMatchObject({ + ok: false, + error: { + code: "UNAVAILABLE", + message: expect.stringContaining("configured model catalog unavailable"), + }, + }); +});