From 6d4950365b42d510bebb833ff4b096095eaf5bf2 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Tue, 18 Aug 2026 17:53:52 -0700 Subject: [PATCH 001/356] refactor(agents): consolidate session auth selection behind prepared-facts facade (#126084) * fix(subagents): drop dead announce gateway-context resolver plumbing #126062 left caller-supplied announce gateway-context resolvers behind after binding dispatch to the Gateway instance. Remove the dead plumbing that broke prod-types in gateway-scope lanes. * refactor(agents): consolidate session auth selection behind prepared-facts facade Completes #123894's redesign by deduplicating the four session-auth call-site preludes and deleting inference-runtime's duplicate route re-derivation. Co-authored-by: fuller-stack-dev <263060202+fuller-stack-dev@users.noreply.github.com> --------- Co-authored-by: fuller-stack-dev <263060202+fuller-stack-dev@users.noreply.github.com> --- .../session-override.selection.test.ts | 133 ++++++++++++++++++ .../session-override.test-support.ts | 25 ++-- .../auth-profiles/session-override.test.ts | 21 ++- src/agents/auth-profiles/session-override.ts | 111 ++++++++++++--- src/agents/btw.test.ts | 63 +++++---- src/agents/btw.ts | 21 +-- .../subagent-announce-delivery.test.ts | 3 - .../announce/subagent-announce-delivery.ts | 2 - .../subagent-announce-direct-delivery.ts | 1 - ...subagent-announce.requester-settle-wake.ts | 2 - .../subagents/announce/subagent-announce.ts | 2 - .../subagent-gateway-context-binding.test.ts | 14 -- ...ent-registry-lifecycle-announce-cleanup.ts | 2 - ...irective.directive-behavior.e2e-harness.ts | 6 +- ....directive.directive-behavior.e2e-mocks.ts | 5 +- src/auto-reply/reply.test-harness.ts | 2 +- .../reply/get-reply-run-admission.ts | 23 +-- .../reply/get-reply-run.media-only.test.ts | 74 +++------- ...ted-agent.auth-profile-propagation.test.ts | 8 +- ...d-agent.isolated-auth-session-flag.test.ts | 14 +- .../run-auth-profile.runtime.ts | 2 +- src/cron/isolated-agent/run-prepare.ts | 20 +-- .../run.auth-profile-cold-path.test.ts | 4 +- .../run.live-session-model-switch.test.ts | 20 ++- src/cron/isolated-agent/run.test-harness.ts | 8 +- .../inference-runtime.test.ts | 42 ++++-- .../worker-environments/inference-runtime.ts | 83 ++--------- src/plugins/runtime/gateway-request-scope.ts | 9 -- 28 files changed, 410 insertions(+), 310 deletions(-) create mode 100644 src/agents/auth-profiles/session-override.selection.test.ts diff --git a/src/agents/auth-profiles/session-override.selection.test.ts b/src/agents/auth-profiles/session-override.selection.test.ts new file mode 100644 index 000000000000..79474fbcc93f --- /dev/null +++ b/src/agents/auth-profiles/session-override.selection.test.ts @@ -0,0 +1,133 @@ +import { describe, expect, it } from "vitest"; +import type { SessionEntry } from "../../config/sessions/types.js"; +import type { OpenClawConfig } from "../../config/types.openclaw.js"; +import { + authStoreMocks, + createAuthStoreWithProfiles, + resolveSessionAuthSelection, + TEST_PRIMARY_PROFILE_ID, + TEST_SECONDARY_PROFILE_ID, + withAuthState, +} from "./session-override.test-support.js"; + +const OAUTH_PROFILE_ID = "openai:subscription"; +const SESSION_KEY = "agent:main:main"; + +function configureProfiles(): void { + authStoreMocks.state.hasSource = true; + authStoreMocks.state.store = createAuthStoreWithProfiles({ + profiles: { + [TEST_PRIMARY_PROFILE_ID]: { + type: "api_key", + provider: "openai", + key: "sk-primary", + }, + [TEST_SECONDARY_PROFILE_ID]: { + type: "api_key", + provider: "openai", + key: "sk-secondary", + }, + [OAUTH_PROFILE_ID]: { + type: "oauth", + provider: "openai", + access: "test-access", + refresh: "test-refresh", + expires: Date.now() + 60_000, + }, + }, + order: { openai: [TEST_PRIMARY_PROFILE_ID, TEST_SECONDARY_PROFILE_ID, OAUTH_PROFILE_ID] }, + }); +} + +async function select(params: { + agentDir: string; + sessionEntry: SessionEntry; + configuredProfileId?: string; + modelId?: string; +}) { + return await resolveSessionAuthSelection({ + cfg: {} as OpenClawConfig, + provider: "openai", + modelId: params.modelId ?? "gpt-5.6-sol", + ...(params.configuredProfileId ? { configuredProfileId: params.configuredProfileId } : {}), + agentDir: params.agentDir, + sessionEntry: params.sessionEntry, + sessionStore: { [SESSION_KEY]: params.sessionEntry }, + sessionKey: SESSION_KEY, + isNewSession: false, + }); +} + +describe("session auth selection prepared facts", () => { + it("returns prepared facts for a user pin", async () => { + await withAuthState(async (state) => { + configureProfiles(); + const sessionEntry: SessionEntry = { + sessionId: "s1", + updatedAt: 1, + authProfileOverride: TEST_PRIMARY_PROFILE_ID, + authProfileOverrideSource: "user", + }; + + await expect(select({ agentDir: state.agentDir(), sessionEntry })).resolves.toEqual({ + profileId: TEST_PRIMARY_PROFILE_ID, + source: "user", + routeRequirement: "api-key", + }); + }); + }); + + it("returns prepared facts after automatic rotation", async () => { + await withAuthState(async (state) => { + configureProfiles(); + const sessionEntry: SessionEntry = { + sessionId: "s1", + updatedAt: 1, + model: "gpt-5.6-sol", + compactionCount: 1, + authProfileOverride: TEST_PRIMARY_PROFILE_ID, + authProfileOverrideSource: "auto", + authProfileOverrideCompactionCount: 0, + }; + + await expect(select({ agentDir: state.agentDir(), sessionEntry })).resolves.toEqual({ + profileId: TEST_SECONDARY_PROFILE_ID, + source: "auto", + routeRequirement: "api-key", + }); + }); + }); + + it("uses only explicit configured-profile precedence", async () => { + await withAuthState(async (state) => { + configureProfiles(); + const sessionEntry: SessionEntry = { + sessionId: "s1", + updatedAt: 1, + compactionCount: 0, + authProfileOverride: TEST_PRIMARY_PROFILE_ID, + authProfileOverrideSource: "auto", + authProfileOverrideCompactionCount: 0, + }; + + await expect( + select({ + agentDir: state.agentDir(), + sessionEntry, + modelId: `gpt-5.6-sol@${OAUTH_PROFILE_ID}`, + }), + ).resolves.toMatchObject({ profileId: TEST_PRIMARY_PROFILE_ID, source: "auto" }); + await expect( + select({ + agentDir: state.agentDir(), + sessionEntry, + configuredProfileId: OAUTH_PROFILE_ID, + }), + ).resolves.toEqual({ + profileId: OAUTH_PROFILE_ID, + source: "user", + routeRequirement: "subscription", + }); + }); + }); +}); diff --git a/src/agents/auth-profiles/session-override.test-support.ts b/src/agents/auth-profiles/session-override.test-support.ts index 0079f0e3720f..dbc42dfc14ab 100644 --- a/src/agents/auth-profiles/session-override.test-support.ts +++ b/src/agents/auth-profiles/session-override.test-support.ts @@ -70,7 +70,7 @@ vi.mock("../../plugins/provider-model-routes.js", () => ({ resolveProviderModelRoutes: authStoreMocks.resolveProviderModelRoutes, })); -export const { clearSessionAuthProfileOverride, resolveSessionAuthProfileOverride } = +export const { clearSessionAuthProfileOverride, resolveSessionAuthSelection } = await import("./session-override.js"); export { authStoreMocks }; @@ -170,16 +170,19 @@ export async function resolveSession(params: { storePath?: string; isNewSession?: boolean; }): Promise { - return await resolveSessionAuthProfileOverride({ - cfg: params.cfg ?? ({} as OpenClawConfig), - provider: params.provider ?? "openai", - agentDir: params.agentDir, - sessionEntry: params.sessionEntry, - sessionStore: params.sessionStore, - sessionKey: params.sessionKey ?? "agent:main:main", - storePath: params.storePath, - isNewSession: params.isNewSession ?? false, - }); + return ( + await resolveSessionAuthSelection({ + cfg: params.cfg ?? ({} as OpenClawConfig), + provider: params.provider ?? "openai", + modelId: params.sessionEntry.model ?? "model-x", + agentDir: params.agentDir, + sessionEntry: params.sessionEntry, + sessionStore: params.sessionStore, + sessionKey: params.sessionKey ?? "agent:main:main", + storePath: params.storePath, + isNewSession: params.isNewSession ?? false, + }) + )?.profileId; } export function createAutomaticSessionEntry(overrides: Partial = {}): SessionEntry { diff --git a/src/agents/auth-profiles/session-override.test.ts b/src/agents/auth-profiles/session-override.test.ts index 6a4474c4d49a..f1efc1dd2c82 100644 --- a/src/agents/auth-profiles/session-override.test.ts +++ b/src/agents/auth-profiles/session-override.test.ts @@ -17,7 +17,6 @@ import { createAutomaticSessionEntry, prepareCooldownAuthState, resolveSession, - resolveSessionAuthProfileOverride, TEST_PRIMARY_PROFILE_ID, TEST_SECONDARY_PROFILE_ID, withAuthState, @@ -36,7 +35,7 @@ describe("resolveSessionAuthProfileOverride", () => { }; const sessionStore = { "agent:main:main": sessionEntry }; - const resolved = await resolveSessionAuthProfileOverride({ + const resolved = await resolveSession({ cfg: {} as OpenClawConfig, provider: "openrouter", agentDir, @@ -74,7 +73,7 @@ describe("resolveSessionAuthProfileOverride", () => { }; const sessionStore = { "agent:main:main": sessionEntry }; - const resolved = await resolveSessionAuthProfileOverride({ + const resolved = await resolveSession({ cfg: {} as OpenClawConfig, provider: "z.ai", agentDir, @@ -105,7 +104,7 @@ describe("resolveSessionAuthProfileOverride", () => { }; const sessionStore = { "agent:main:main": sessionEntry }; - const resolved = await resolveSessionAuthProfileOverride({ + const resolved = await resolveSession({ cfg: { models: { providers: { @@ -163,7 +162,7 @@ describe("resolveSessionAuthProfileOverride", () => { }; const sessionStore = { "agent:main:main": sessionEntry }; - const resolved = await resolveSessionAuthProfileOverride({ + const resolved = await resolveSession({ cfg: { models: { providers: { @@ -230,7 +229,7 @@ describe("resolveSessionAuthProfileOverride", () => { }; const sessionStore = { "agent:main:main": sessionEntry }; - const resolved = await resolveSessionAuthProfileOverride({ + const resolved = await resolveSession({ cfg: {} as OpenClawConfig, provider: "openai", agentDir, @@ -273,7 +272,7 @@ describe("resolveSessionAuthProfileOverride", () => { }; const sessionStore = { "agent:main:main": sessionEntry }; - const resolved = await resolveSessionAuthProfileOverride({ + const resolved = await resolveSession({ cfg: {} as OpenClawConfig, provider: "codex-cli", agentDir, @@ -315,10 +314,9 @@ describe("resolveSessionAuthProfileOverride", () => { }; const sessionStore = { "agent:main:main": sessionEntry }; - const resolved = await resolveSessionAuthProfileOverride({ + const resolved = await resolveSession({ cfg: {} as OpenClawConfig, provider: "openai", - acceptedProviderIds: ["openai"], agentDir, sessionEntry, sessionStore, @@ -363,10 +361,9 @@ describe("resolveSessionAuthProfileOverride", () => { }; const sessionStore = { "agent:main:main": sessionEntry }; - const resolved = await resolveSessionAuthProfileOverride({ + const resolved = await resolveSession({ cfg: {} as OpenClawConfig, provider: "openai", - acceptedProviderIds: ["openai"], agentDir, sessionEntry, sessionStore, @@ -415,7 +412,7 @@ describe("resolveSessionAuthProfileOverride", () => { }; const sessionStore = { "agent:main:main": sessionEntry }; - const resolved = await resolveSessionAuthProfileOverride({ + const resolved = await resolveSession({ cfg: {} as OpenClawConfig, provider: "openai", agentDir, diff --git a/src/agents/auth-profiles/session-override.ts b/src/agents/auth-profiles/session-override.ts index 28222ce2169c..c7496d5522c6 100644 --- a/src/agents/auth-profiles/session-override.ts +++ b/src/agents/auth-profiles/session-override.ts @@ -2,6 +2,7 @@ import { resolveSessionAuthProfileOverrideSource } from "../../config/sessions/auth-profile-override-provenance.js"; import type { SessionEntry } from "../../config/sessions/types.js"; import type { OpenClawConfig } from "../../config/types.openclaw.js"; +import type { ProviderModelRouteAuthRequirement } from "../../plugin-sdk/provider-model-types.js"; import { resolveProviderModelRoutes } from "../../plugins/provider-model-routes.js"; import { createLazyImportLoader } from "../../shared/lazy-promise.js"; import { @@ -15,6 +16,8 @@ import { isModelScopedCooldownReason, } from "../auth-profiles/usage-state.js"; import { isProfileInCooldown } from "../auth-profiles/usage.js"; +import { splitTrailingAuthProfile } from "../model-ref-profile.js"; +import { listOpenAIAuthProfileProvidersForAgentRuntime } from "../openai-routing.js"; import { resolveProviderModelRouteAuthRequirement } from "../provider-model-route-auth.js"; const sessionAccessorLoader = createLazyImportLoader( @@ -33,6 +36,21 @@ type SessionAuthProfileOverrideState = Pick< >; type SessionAuthProfileOverrideSnapshot = SessionAuthProfileOverrideState & Pick; +type SessionAuthProfileOverrideResult = { + profileId: string | undefined; + store: ReturnType | undefined; +}; + +function profileAuthRequirement(params: { + cfg: OpenClawConfig; + store: ReturnType | undefined; + profileId: string; +}): ProviderModelRouteAuthRequirement | undefined { + return resolveProviderModelRouteAuthRequirement( + params.store?.profiles[params.profileId]?.type ?? + params.cfg.auth?.profiles?.[params.profileId]?.mode, + ); +} function applySessionAuthProfileOverrideState( entry: SessionEntry, @@ -224,10 +242,10 @@ export async function clearSessionAuthProfileOverride(params: { }); } -/** Resolves and optionally rotates the session auth-profile override. */ -export async function resolveSessionAuthProfileOverride(params: { +async function resolveSessionAuthProfileOverride(params: { cfg: OpenClawConfig; provider: string; + modelId: string; agentDir: string; sessionEntry?: SessionEntry; sessionStore?: Record; @@ -235,7 +253,7 @@ export async function resolveSessionAuthProfileOverride(params: { storePath?: string; isNewSession: boolean; acceptedProviderIds?: string[]; -}): Promise { +}): Promise { const { cfg, provider, @@ -247,7 +265,7 @@ export async function resolveSessionAuthProfileOverride(params: { isNewSession, } = params; if (!sessionEntry || !sessionStore || !sessionKey) { - return sessionEntry?.authProfileOverride; + return { profileId: sessionEntry?.authProfileOverride, store: undefined }; } const hasConfiguredAuthProfiles = @@ -258,7 +276,7 @@ export async function resolveSessionAuthProfileOverride(params: { !hasConfiguredAuthProfiles && !hasAnyAuthProfileStoreSource(agentDir) ) { - return undefined; + return { profileId: undefined, store: undefined }; } const store = ensureAuthProfileStore(agentDir, { allowKeychainPrompt: false }); @@ -296,7 +314,7 @@ export async function resolveSessionAuthProfileOverride(params: { // Explicit user pins are strict until the profile disappears or changes provider. if (source === "user" && current) { - return current; + return { profileId: current, store }; } // Automatic pins must stay inside the currently configured rotation order. @@ -306,7 +324,7 @@ export async function resolveSessionAuthProfileOverride(params: { } if (order.length === 0) { - return undefined; + return { profileId: undefined, store }; } if (order.every((profileId) => isProfileGloballyInCooldown(store, profileId))) { @@ -331,13 +349,17 @@ export async function resolveSessionAuthProfileOverride(params: { }); const latestProfileId = latest?.authProfileOverride; const latestSource = resolveSessionAuthProfileOverrideSource(latest); - return latestProfileId && - latestSource === "user" && - isProfileForProvider({ cfg, providers, profileId: latestProfileId, store }) - ? latestProfileId - : undefined; + return { + profileId: + latestProfileId && + latestSource === "user" && + isProfileForProvider({ cfg, providers, profileId: latestProfileId, store }) + ? latestProfileId + : undefined, + store, + }; } - return undefined; + return { profileId: undefined, store }; } const isProfileUnavailableForSessionModel = (profileId: string) => @@ -352,19 +374,17 @@ export async function resolveSessionAuthProfileOverride(params: { Boolean(current) && !isNewSession && (currentUnavailable || compactionCount > storedCompaction); // Provider artifacts own persisted route stickiness; runtime planning owns cross-route failover. - const profileAuthRequirement = (profileId: string) => - resolveProviderModelRouteAuthRequirement( - store.profiles[profileId]?.type ?? cfg.auth?.profiles?.[profileId]?.mode, - ); const routeResolution = shouldRotateCurrent - ? resolveProviderModelRoutes({ provider, modelId: sessionEntry.model, config: cfg }) + ? resolveProviderModelRoutes({ provider, modelId: params.modelId, config: cfg }) : null; const currentAuthRequirement = current && routeResolution?.kind === "routes" && routeResolution.routes.length > 1 - ? profileAuthRequirement(current) + ? profileAuthRequirement({ cfg, store, profileId: current }) : undefined; const rotationOrder = currentAuthRequirement - ? order.filter((profileId) => profileAuthRequirement(profileId) === currentAuthRequirement) + ? order.filter( + (profileId) => profileAuthRequirement({ cfg, store, profileId }) === currentAuthRequirement, + ) : order; const pickAvailable = (active?: string) => { const startIndex = active ? rotationOrder.indexOf(active) : -1; @@ -385,7 +405,7 @@ export async function resolveSessionAuthProfileOverride(params: { } if (!next) { - return current; + return { profileId: current, store }; } const shouldPersist = next !== sessionEntry.authProfileOverride || @@ -405,5 +425,52 @@ export async function resolveSessionAuthProfileOverride(params: { }); } - return next; + return { profileId: next, store }; +} + +type SessionAuthSelection = { + profileId: string; + source: "auto" | "user"; + routeRequirement: ProviderModelRouteAuthRequirement | undefined; +}; + +/** Resolves the session credential and its prepared route facts. */ +export async function resolveSessionAuthSelection(params: { + cfg: OpenClawConfig; + provider: string; + modelId: string; + configuredProfileId?: string; + harnessRuntime?: string; + agentDir: string; + sessionEntry?: SessionEntry; + sessionStore?: Record; + sessionKey?: string; + storePath?: string; + isNewSession: boolean; +}): Promise { + const { profileId: rotatedProfileId, store } = await resolveSessionAuthProfileOverride({ + ...params, + modelId: splitTrailingAuthProfile(params.modelId).model, + acceptedProviderIds: listOpenAIAuthProfileProvidersForAgentRuntime({ + provider: params.provider, + harnessRuntime: params.harnessRuntime, + config: params.cfg, + }), + }); + const rotatedSource = rotatedProfileId + ? params.sessionEntry?.authProfileOverride?.trim() === rotatedProfileId + ? (resolveSessionAuthProfileOverrideSource(params.sessionEntry) ?? "auto") + : "auto" + : undefined; + const rotatedUserProfileId = rotatedSource === "user" ? rotatedProfileId : undefined; + const configuredProfileId = params.configuredProfileId?.trim() || undefined; + const profileId = rotatedUserProfileId ?? configuredProfileId ?? rotatedProfileId; + if (!profileId) { + return undefined; + } + return { + profileId, + source: rotatedUserProfileId || configuredProfileId ? "user" : (rotatedSource ?? "auto"), + routeRequirement: profileAuthRequirement({ cfg: params.cfg, store, profileId }), + }; } diff --git a/src/agents/btw.test.ts b/src/agents/btw.test.ts index cbcaf7a90d37..ec5563ce3f64 100644 --- a/src/agents/btw.test.ts +++ b/src/agents/btw.test.ts @@ -40,7 +40,7 @@ const ensureAuthProfileStoreWithoutExternalProfilesMock = vi.fn(); const resolveModelAsyncMock = vi.fn(); const getApiKeyForModelMock = vi.fn(); const requireApiKeyMock = vi.fn(); -const resolveSessionAuthProfileOverrideMock = vi.fn(); +const resolveSessionAuthSelectionMock = vi.fn(); const getActiveEmbeddedRunSnapshotMock = vi.fn(); const resolveSessionAgentIdMock = vi.fn(); const resolveSessionAgentIdsMock = vi.fn(); @@ -338,8 +338,7 @@ vi.mock("./embedded-agent-runner/stream-resolution.js", () => ({ })); vi.mock("./auth-profiles/session-override.js", () => ({ - resolveSessionAuthProfileOverride: (...args: unknown[]) => - resolveSessionAuthProfileOverrideMock(...args), + resolveSessionAuthSelection: (...args: unknown[]) => resolveSessionAuthSelectionMock(...args), })); vi.mock("../logging/diagnostic.js", () => ({ @@ -702,7 +701,7 @@ describe("runBtwSideQuestion", () => { ensureAuthProfileStoreWithoutExternalProfilesMock.mockReset(); getApiKeyForModelMock.mockReset(); requireApiKeyMock.mockReset(); - resolveSessionAuthProfileOverrideMock.mockReset(); + resolveSessionAuthSelectionMock.mockReset(); getActiveEmbeddedRunSnapshotMock.mockReset(); resolveSessionAgentIdMock.mockReset(); resolveSessionAgentIdsMock.mockReset(); @@ -773,7 +772,11 @@ describe("runBtwSideQuestion", () => { ...(params.profileId ? { profileId: params.profileId } : {}), })); requireApiKeyMock.mockReturnValue("secret"); - resolveSessionAuthProfileOverrideMock.mockResolvedValue("profile-1"); + resolveSessionAuthSelectionMock.mockResolvedValue({ + profileId: "profile-1", + source: "auto", + routeRequirement: undefined, + }); getActiveEmbeddedRunSnapshotMock.mockReturnValue(undefined); resolveSessionAgentIdMock.mockReturnValue("main"); resolveSessionAgentIdsMock.mockReturnValue({ defaultAgentId: "main", sessionAgentId: "main" }); @@ -996,7 +999,11 @@ describe("runBtwSideQuestion", () => { api: "openai-responses", baseUrl: "https://api.openai.com/v1", }); - resolveSessionAuthProfileOverrideMock.mockResolvedValue("openai:work"); + resolveSessionAuthSelectionMock.mockResolvedValue({ + profileId: "openai:work", + source: "auto", + routeRequirement: "subscription", + }); ensureAuthProfileStoreMock.mockReturnValue({ version: 1, profiles: { @@ -1134,7 +1141,7 @@ describe("runBtwSideQuestion", () => { }; resolveModelWithRegistryMock.mockReturnValue(subscriptionModel); resolveModelAsyncMock.mockResolvedValue({ model: subscriptionModel }); - resolveSessionAuthProfileOverrideMock.mockResolvedValue(undefined); + resolveSessionAuthSelectionMock.mockResolvedValue(undefined); ensureAuthProfileStoreMock.mockReturnValue({ version: 1, profiles: {} }); resolveProviderEntryApiKeyProfileReferenceMock.mockReturnValue({ kind: "literal" }); getApiKeyForModelMock.mockResolvedValue({ @@ -1185,7 +1192,7 @@ describe("runBtwSideQuestion", () => { }; resolveModelWithRegistryMock.mockReturnValue(platformModel); resolveModelAsyncMock.mockResolvedValue({ model: platformModel }); - resolveSessionAuthProfileOverrideMock.mockResolvedValue(undefined); + resolveSessionAuthSelectionMock.mockResolvedValue(undefined); ensureAuthProfileStoreMock.mockReturnValue({ version: 1, profiles: {} }); resolveProviderEntryApiKeyProfileReferenceMock.mockReturnValue({ kind: "literal" }); getApiKeyForModelMock.mockResolvedValue({ @@ -1247,7 +1254,7 @@ describe("runBtwSideQuestion", () => { baseUrl: "https://api.openai.com/v1", }; resolveModelWithRegistryMock.mockReturnValue(platformModel); - resolveSessionAuthProfileOverrideMock.mockResolvedValue(undefined); + resolveSessionAuthSelectionMock.mockResolvedValue(undefined); ensureAuthProfileStoreMock.mockReturnValue({ version: 1, profiles: {} }); getApiKeyForModelMock.mockResolvedValue({ apiKey: undefined, @@ -1318,7 +1325,7 @@ describe("runBtwSideQuestion", () => { }, order: { openai: ["openai:subscription", "openai:platform"] }, }); - resolveSessionAuthProfileOverrideMock.mockResolvedValue(undefined); + resolveSessionAuthSelectionMock.mockResolvedValue(undefined); resolveModelWithRegistryMock.mockReturnValue(platformModel); resolveModelAsyncMock.mockImplementation( async ( @@ -1429,7 +1436,11 @@ describe("runBtwSideQuestion", () => { api: "openai-responses", baseUrl: "https://api.openai.com/v1", }); - resolveSessionAuthProfileOverrideMock.mockResolvedValue("openai-codex:user@example.test"); + resolveSessionAuthSelectionMock.mockResolvedValue({ + profileId: "openai-codex:user@example.test", + source: "auto", + routeRequirement: "subscription", + }); ensureAuthProfileStoreMock.mockReturnValue({ version: 1, profiles: { @@ -1505,12 +1516,6 @@ describe("runBtwSideQuestion", () => { expect( Object.keys(sideQuestionParams.preparedRuntimeAuth?.authProfileStore?.profiles ?? {}), ).toEqual(["openai-codex:user@example.test"]); - const authArgs = mockArg(resolveSessionAuthProfileOverrideMock, 0, 0) as { - provider?: string; - acceptedProviderIds?: string[]; - }; - expect(authArgs.provider).toBe("openai"); - expect(authArgs.acceptedProviderIds).toEqual(["openai"]); expect(streamSimpleMock).not.toHaveBeenCalled(); expect(registerProviderStreamForModelMock).not.toHaveBeenCalled(); }); @@ -1811,13 +1816,17 @@ describe("runBtwSideQuestion", () => { authProfileOverrideSource: "auto", }); const sessionStore = { [DEFAULT_SESSION_KEY]: sessionEntry }; - resolveSessionAuthProfileOverrideMock.mockImplementation( + resolveSessionAuthSelectionMock.mockImplementation( async (params: { sessionEntry?: SessionEntry }) => { if (params.sessionEntry) { params.sessionEntry.authProfileOverride = "anthropic:api"; params.sessionEntry.authProfileOverrideSource = "auto"; } - return "anthropic:api"; + return { + profileId: "anthropic:api", + source: "auto", + routeRequirement: "api-key", + }; }, ); mockDoneAnswer("Generic fallback answer."); @@ -1846,7 +1855,7 @@ describe("runBtwSideQuestion", () => { expect(prepareParams.provider).toBe("claude-cli"); expect(prepareParams.executionMode).toBe("side-question"); expect(prepareParams.authProfileId).toBe("anthropic:auto-cli"); - expect(resolveSessionAuthProfileOverrideMock).not.toHaveBeenCalled(); + expect(resolveSessionAuthSelectionMock).not.toHaveBeenCalled(); expect(cleanup).toHaveBeenCalledTimes(1); expect(getApiKeyForModelMock).not.toHaveBeenCalled(); expect(streamSimpleMock).not.toHaveBeenCalled(); @@ -1879,7 +1888,7 @@ describe("runBtwSideQuestion", () => { profileId: "anthropic:claude-cli", }); requireApiKeyMock.mockReturnValueOnce("claude-cli-access"); - resolveSessionAuthProfileOverrideMock.mockResolvedValueOnce(undefined); + resolveSessionAuthSelectionMock.mockResolvedValueOnce(undefined); resolveModelAsyncMock.mockResolvedValueOnce({ model: { provider: DEFAULT_PROVIDER, @@ -1953,7 +1962,7 @@ describe("runBtwSideQuestion", () => { modelRegistry, }); ensureAuthProfileStoreWithoutExternalProfilesMock.mockReturnValue(authStore); - resolveSessionAuthProfileOverrideMock.mockResolvedValue(undefined); + resolveSessionAuthSelectionMock.mockResolvedValue(undefined); getApiKeyForModelMock.mockImplementation(async (authParams: { profileId?: string } = {}) => { if (authParams.profileId === "anthropic:primary") { throw new Error("primary credential resolution failed"); @@ -2072,7 +2081,7 @@ describe("runBtwSideQuestion", () => { }), ); ensureAuthProfileStoreMock.mockReturnValue(authStore); - resolveSessionAuthProfileOverrideMock.mockResolvedValue(undefined); + resolveSessionAuthSelectionMock.mockResolvedValue(undefined); getApiKeyForModelMock.mockImplementation(async (authParams: { profileId?: string } = {}) => { if (authParams.profileId === "openai:subscription") { throw new Error("subscription credential resolution failed"); @@ -2148,7 +2157,7 @@ describe("runBtwSideQuestion", () => { resolveModelWithRegistryMock.mockReturnValue(platformModel); resolveModelAsyncMock.mockResolvedValue({ model: platformModel }); ensureAuthProfileStoreMock.mockReturnValue(authStore); - resolveSessionAuthProfileOverrideMock.mockResolvedValue(undefined); + resolveSessionAuthSelectionMock.mockResolvedValue(undefined); resolveProviderEntryApiKeyProfileReferenceMock.mockReturnValue({ kind: "literal" }); getApiKeyForModelMock.mockImplementation( async (authParams: { profileId?: string; allowAuthProfileFallback?: boolean }) => { @@ -2233,7 +2242,11 @@ describe("runBtwSideQuestion", () => { profileId: "anthropic:api", }); requireApiKeyMock.mockReturnValueOnce("static-key"); - resolveSessionAuthProfileOverrideMock.mockResolvedValueOnce("anthropic:api"); + resolveSessionAuthSelectionMock.mockResolvedValueOnce({ + profileId: "anthropic:api", + source: "user", + routeRequirement: "api-key", + }); mockDoneAnswer("Static answer."); await runSideQuestion({ diff --git a/src/agents/btw.ts b/src/agents/btw.ts index 8f42c8dc8c20..3c4c66eef600 100644 --- a/src/agents/btw.ts +++ b/src/agents/btw.ts @@ -25,7 +25,7 @@ import { isModelSelectionLocked } from "../sessions/model-overrides.js"; import { prepareSystemAgentRunAdmission } from "./admitted-run-context.js"; import { resolveAgentWorkspaceDir, resolveSessionAgentId } from "./agent-scope.js"; import { resolveExternalCliAuthOverlayScopeFromSelection } from "./auth-profiles/external-cli-auth-selection.js"; -import { resolveSessionAuthProfileOverride } from "./auth-profiles/session-override.js"; +import { resolveSessionAuthSelection } from "./auth-profiles/session-override.js"; import type { AuthProfileStore } from "./auth-profiles/types.js"; import { readBtwTranscriptMessages, resolveBtwSessionTranscriptPath } from "./btw-transcript.js"; import { executePreparedCliRun } from "./cli-runner/execute.runtime.js"; @@ -63,10 +63,7 @@ import { isCliRuntimeAliasForProvider, resolveCliRuntimeExecutionProvider, } from "./model-runtime-aliases.js"; -import { - isOpenAIProvider, - listOpenAIAuthProfileProvidersForAgentRuntime, -} from "./openai-routing.js"; +import { isOpenAIProvider } from "./openai-routing.js"; import { loadPreparedModelRuntimeSnapshot, preparedModelRuntimeConfigsMatch, @@ -515,16 +512,11 @@ async function resolveRuntimeModel(params: { const runtimeProvider = model.provider; const runtimeModelId = model.id; - const acceptedProviderIds = listOpenAIAuthProfileProvidersForAgentRuntime({ - provider: runtimeProvider, - harnessRuntime: params.harnessId, - agentHarnessId: params.harnessId, - config: cfg, - }); - const authProfileId = await resolveSessionAuthProfileOverride({ + const authSelection = await resolveSessionAuthSelection({ cfg, provider: runtimeProvider, - acceptedProviderIds, + modelId: runtimeModelId, + harnessRuntime: params.harnessId, agentDir, sessionEntry: params.sessionEntry, sessionStore: params.sessionStore, @@ -532,7 +524,8 @@ async function resolveRuntimeModel(params: { storePath: params.storePath, isNewSession: params.isNewSession, }); - const authProfileIdSource = resolveReturnedAuthProfileSource(params.sessionEntry, authProfileId); + const authProfileId = authSelection?.profileId; + const authProfileIdSource = authSelection?.source; const authProfileStoreSelection = resolveBtwAuthProfileStore({ cfg, provider: runtimeProvider, diff --git a/src/agents/subagents/announce/subagent-announce-delivery.test.ts b/src/agents/subagents/announce/subagent-announce-delivery.test.ts index 4c2d06a311a1..f203a7a083cc 100644 --- a/src/agents/subagents/announce/subagent-announce-delivery.test.ts +++ b/src/agents/subagents/announce/subagent-announce-delivery.test.ts @@ -1894,8 +1894,6 @@ describe("deliverSubagentAnnouncement completion delivery", () => { getRuntimeConfig: () => ({}) as never, }); - const ownerContext = { owner: "gateway-a" } as never; - const resolveGatewayContext = () => ownerContext; const result = await deliverSubagentAnnouncement({ requesterSessionKey: "agent:main:slack:channel:C123:thread:171.222", targetRequesterSessionKey: "agent:main:slack:channel:C123:thread:171.222", @@ -1914,7 +1912,6 @@ describe("deliverSubagentAnnouncement completion delivery", () => { expectsCompletionMessage: true, bestEffortDeliver: true, directIdempotencyKey: "announce-local-dispatch", - resolveGatewayContext, }); expectDeliveryPath(result, "direct"); diff --git a/src/agents/subagents/announce/subagent-announce-delivery.ts b/src/agents/subagents/announce/subagent-announce-delivery.ts index efcf3b74d7fe..46fa16f07b29 100644 --- a/src/agents/subagents/announce/subagent-announce-delivery.ts +++ b/src/agents/subagents/announce/subagent-announce-delivery.ts @@ -99,7 +99,6 @@ export async function deliverSubagentAnnouncement(params: { directIdempotencyKey: string; onDeliveryResult?: (delivery: SubagentAnnounceDeliveryResult) => void; signal?: AbortSignal; - resolveGatewayContext?: import("../../../gateway/server-methods/types.js").GatewayContextResolver; }): Promise { const sourceOwnerChanged = () => params.isSourceSessionEffectsAllowed?.() === false; if (sourceOwnerChanged()) { @@ -259,7 +258,6 @@ export async function deliverSubagentAnnouncement(params: { onDeliveryResult: params.onDeliveryResult, signal: params.signal, bestEffortDeliver: params.bestEffortDeliver, - resolveGatewayContext: params.resolveGatewayContext, }); }, }); diff --git a/src/agents/subagents/announce/subagent-announce-direct-delivery.ts b/src/agents/subagents/announce/subagent-announce-direct-delivery.ts index 4d2cc77a0620..59b1571cd9df 100644 --- a/src/agents/subagents/announce/subagent-announce-direct-delivery.ts +++ b/src/agents/subagents/announce/subagent-announce-direct-delivery.ts @@ -108,7 +108,6 @@ export async function sendSubagentAnnounceDirectly(params: { requesterIsSubagent: boolean; onDeliveryResult?: (delivery: SubagentAnnounceDeliveryResult) => void; signal?: AbortSignal; - resolveGatewayContext?: import("../../../gateway/server-methods/types.js").GatewayContextResolver; }): Promise { if (params.signal?.aborted) { return { diff --git a/src/agents/subagents/announce/subagent-announce.requester-settle-wake.ts b/src/agents/subagents/announce/subagent-announce.requester-settle-wake.ts index 832d5f007beb..c0961b5e677e 100644 --- a/src/agents/subagents/announce/subagent-announce.requester-settle-wake.ts +++ b/src/agents/subagents/announce/subagent-announce.requester-settle-wake.ts @@ -7,7 +7,6 @@ import { SILENT_REPLY_TOKEN } from "../../../auto-reply/tokens.js"; import { getRuntimeConfig } from "../../../config/config.js"; import { logWarn } from "../../../logger.js"; -import { getSharedGatewayContextResolver } from "../../../plugins/runtime/gateway-request-scope.js"; import { isCronSessionKey } from "../../../sessions/session-key-utils.js"; import { type DeliveryContext, @@ -452,7 +451,6 @@ export async function maybeWakeRequesterAfterAllChildrenSettled(params: { attemptIndex === 0 ? wakeKeyBase : `${wakeKeyBase}:retry-${attemptIndex}`, ), signal: params.signal, - resolveGatewayContext: getSharedGatewayContextResolver(settledBatch), }); } catch (error) { // A transport exception can arrive after gateway admission. Replay the diff --git a/src/agents/subagents/announce/subagent-announce.ts b/src/agents/subagents/announce/subagent-announce.ts index 0472571d9479..9036078d1e1b 100644 --- a/src/agents/subagents/announce/subagent-announce.ts +++ b/src/agents/subagents/announce/subagent-announce.ts @@ -193,7 +193,6 @@ export async function runSubagentAnnounceFlow(params: { bestEffortDeliver?: boolean; onDeliveryResult?: (delivery: SubagentAnnounceDeliveryResult) => void; onBeforeDeleteChildSession?: () => boolean; - resolveGatewayContext?: import("../../../gateway/server-methods/types.js").GatewayContextResolver; }): Promise { let announceOutcome: SubagentAnnounceFlowOutcome = "retryable"; const expectsCompletionMessage = params.expectsCompletionMessage === true; @@ -590,7 +589,6 @@ export async function runSubagentAnnounceFlow(params: { directIdempotencyKey, onDeliveryResult: reportDeliveryResult, signal: params.signal, - resolveGatewayContext: params.resolveGatewayContext, }); reportDeliveryResult(delivery); announceOutcome = delivery.disposition ?? (delivery.delivered ? "delivered" : "retryable"); diff --git a/src/agents/subagents/registry/subagent-gateway-context-binding.test.ts b/src/agents/subagents/registry/subagent-gateway-context-binding.test.ts index cab49f4f1d85..6408b3553fea 100644 --- a/src/agents/subagents/registry/subagent-gateway-context-binding.test.ts +++ b/src/agents/subagents/registry/subagent-gateway-context-binding.test.ts @@ -2,7 +2,6 @@ import { describe, expect, it } from "vitest"; import { bindGatewayContextResolver, getGatewayContextResolver, - getSharedGatewayContextResolver, } from "../../../plugins/runtime/gateway-request-scope.js"; import { createSubagentRunRecord } from "../../subagent-test-fixtures.test-helpers.js"; @@ -20,17 +19,4 @@ describe("subagent Gateway context binding", () => { expect(getGatewayContextResolver(successor)?.()).toBe(context); expect(getGatewayContextResolver(restored)).toBeUndefined(); }); - - it("refuses to select one Gateway for a mixed-owner settle batch", () => { - const first = createSubagentRunRecord({ runId: "run-first" }); - const second = createSubagentRunRecord({ runId: "run-second" }); - const firstContext = { owner: "gateway-a" } as never; - const secondContext = { owner: "gateway-b" } as never; - bindGatewayContextResolver(first, () => firstContext); - bindGatewayContextResolver(second, () => secondContext); - - expect(getGatewayContextResolver(first)?.()).toBe(firstContext); - expect(getGatewayContextResolver(second)?.()).toBe(secondContext); - expect(getSharedGatewayContextResolver([first, second])).toBeUndefined(); - }); }); diff --git a/src/agents/subagents/registry/subagent-registry-lifecycle-announce-cleanup.ts b/src/agents/subagents/registry/subagent-registry-lifecycle-announce-cleanup.ts index dff2b6363d4d..c4dd4e124b04 100644 --- a/src/agents/subagents/registry/subagent-registry-lifecycle-announce-cleanup.ts +++ b/src/agents/subagents/registry/subagent-registry-lifecycle-announce-cleanup.ts @@ -1,4 +1,3 @@ -import { getGatewayContextResolver } from "../../../plugins/runtime/gateway-request-scope.js"; import { defaultRuntime } from "../../../runtime.js"; import { normalizeDeliveryContext } from "../../../utils/delivery-context.shared.js"; import { @@ -607,7 +606,6 @@ export const startSubagentAnnounceCleanupFlow = ( params.persist(runId); } }, - resolveGatewayContext: getGatewayContextResolver(entry), }; runDetachedCleanupAttempt(context, { runId, diff --git a/src/auto-reply/reply.directive.directive-behavior.e2e-harness.ts b/src/auto-reply/reply.directive.directive-behavior.e2e-harness.ts index c10077d601f7..24729b8b3753 100644 --- a/src/auto-reply/reply.directive.directive-behavior.e2e-harness.ts +++ b/src/auto-reply/reply.directive.directive-behavior.e2e-harness.ts @@ -14,7 +14,7 @@ import { compactEmbeddedAgentSessionMock, loadModelCatalogMock, resolveCommandSecretRefsViaGatewayMock, - resolveSessionAuthProfileOverrideMock, + resolveSessionAuthSelectionMock, runDirectiveBehaviorReplyAgent, runEmbeddedAgentMock, runDirectiveBehaviorPreparedReply, @@ -123,8 +123,8 @@ export function installDirectiveBehaviorE2EHooks() { })); clearSessionAuthProfileOverrideMock.mockReset(); clearSessionAuthProfileOverrideMock.mockResolvedValue(undefined); - resolveSessionAuthProfileOverrideMock.mockReset(); - resolveSessionAuthProfileOverrideMock.mockResolvedValue(undefined); + resolveSessionAuthSelectionMock.mockReset(); + resolveSessionAuthSelectionMock.mockResolvedValue(undefined); runReplyAgentMock.mockReset(); runReplyAgentMock.mockImplementation(runDirectiveBehaviorReplyAgent); runPreparedReplyMock.mockReset(); diff --git a/src/auto-reply/reply.directive.directive-behavior.e2e-mocks.ts b/src/auto-reply/reply.directive.directive-behavior.e2e-mocks.ts index 5ba4fbdc5429..81402ab873ff 100644 --- a/src/auto-reply/reply.directive.directive-behavior.e2e-mocks.ts +++ b/src/auto-reply/reply.directive.directive-behavior.e2e-mocks.ts @@ -6,7 +6,7 @@ export const compactEmbeddedAgentSessionMock: Mock = vi.fn(); export const loadModelCatalogMock: Mock = vi.fn(); export const resolveCommandSecretRefsViaGatewayMock: Mock = vi.fn(); export const clearSessionAuthProfileOverrideMock: Mock = vi.fn(); -export const resolveSessionAuthProfileOverrideMock: Mock = vi.fn(); +export const resolveSessionAuthSelectionMock: Mock = vi.fn(); function objectRecord(value: unknown): Record | undefined { return value && typeof value === "object" ? (value as Record) : undefined; @@ -127,8 +127,7 @@ vi.mock("../cli/command-secret-gateway.js", () => ({ vi.mock("../agents/auth-profiles/session-override.js", () => ({ clearSessionAuthProfileOverride: (...args: unknown[]) => clearSessionAuthProfileOverrideMock(...args), - resolveSessionAuthProfileOverride: (...args: unknown[]) => - resolveSessionAuthProfileOverrideMock(...args), + resolveSessionAuthSelection: (...args: unknown[]) => resolveSessionAuthSelectionMock(...args), })); vi.mock("../plugins/hook-runner-global.js", async (importOriginal) => { diff --git a/src/auto-reply/reply.test-harness.ts b/src/auto-reply/reply.test-harness.ts index 66b26235e927..f972dc5ddd4c 100644 --- a/src/auto-reply/reply.test-harness.ts +++ b/src/auto-reply/reply.test-harness.ts @@ -40,7 +40,7 @@ vi.mock("../agents/model-catalog.runtime.js", () => ({ vi.mock("../agents/auth-profiles/session-override.js", () => ({ clearSessionAuthProfileOverride: vi.fn(), - resolveSessionAuthProfileOverride: vi.fn().mockResolvedValue(undefined), + resolveSessionAuthSelection: vi.fn().mockResolvedValue(undefined), })); vi.mock("../commands-registry.runtime.js", () => ({ diff --git a/src/auto-reply/reply/get-reply-run-admission.ts b/src/auto-reply/reply/get-reply-run-admission.ts index 8f9130fbeffa..f257e058b499 100644 --- a/src/auto-reply/reply/get-reply-run-admission.ts +++ b/src/auto-reply/reply/get-reply-run-admission.ts @@ -2,9 +2,8 @@ import crypto from "node:crypto"; import path from "node:path"; import { normalizeOptionalString } from "@openclaw/normalization-core/string-coerce"; import { clearAutoFallbackPrimaryProbeSelection } from "../../agents/agent-scope.js"; -import { resolveSessionAuthProfileOverride } from "../../agents/auth-profiles/session-override.js"; +import { resolveSessionAuthSelection } from "../../agents/auth-profiles/session-override.js"; import { resolveAgentHarnessPolicy } from "../../agents/harness/policy.js"; -import { listOpenAIAuthProfileProvidersForAgentRuntime } from "../../agents/openai-routing.js"; import { hasResolvedThinkingCatalogEntry } from "../../agents/thinking-runtime.js"; import { resolveSessionAuthProfileOverrideSource } from "../../config/sessions/auth-profile-override-provenance.js"; import { formatSqliteSessionFileMarker } from "../../config/sessions/legacy-sqlite-marker.js"; @@ -407,14 +406,6 @@ export async function prepareReplyRunAdmission(context: PreparedReplyRunContext) agentId, sessionKey: context.runtimePolicySessionKey, }); - const resolveAcceptedAuthProfileProviders = () => - agentHarnessPolicy - ? listOpenAIAuthProfileProvidersForAgentRuntime({ - provider, - harnessRuntime: agentHarnessPolicy.runtime, - config: cfg, - }) - : [provider]; const resolveRuntimeAuthProfile = async () => { if (useFastReplyRuntime) { return { @@ -437,10 +428,11 @@ export async function prepareReplyRunAdmission(context: PreparedReplyRunContext) shouldUseEphemeralSession && authSessionEntry ? { [authSessionKey]: authSessionEntry } : sessionStore; - const resolvedAuthProfileId = await resolveSessionAuthProfileOverride({ + const selection = await resolveSessionAuthSelection({ cfg, provider, - acceptedProviderIds: resolveAcceptedAuthProfileProviders(), + modelId: model, + ...(agentHarnessPolicy ? { harnessRuntime: agentHarnessPolicy.runtime } : {}), agentDir, sessionEntry: authSessionEntry, sessionStore: authSessionStore, @@ -449,11 +441,8 @@ export async function prepareReplyRunAdmission(context: PreparedReplyRunContext) isNewSession, }); return { - authProfileId: resolvedAuthProfileId, - authProfileIdSource: - resolvedAuthProfileId && authSessionEntry?.authProfileOverride === resolvedAuthProfileId - ? resolveSessionAuthProfileOverrideSource(authSessionEntry) - : undefined, + authProfileId: selection?.profileId, + authProfileIdSource: selection?.source, }; }; let { authProfileId, authProfileIdSource } = await traceRunPhase( diff --git a/src/auto-reply/reply/get-reply-run.media-only.test.ts b/src/auto-reply/reply/get-reply-run.media-only.test.ts index 27c7d7c8e350..cce165330834 100644 --- a/src/auto-reply/reply/get-reply-run.media-only.test.ts +++ b/src/auto-reply/reply/get-reply-run.media-only.test.ts @@ -43,7 +43,7 @@ import { withReplySystemEventSessionKey } from "./system-event-session-key.js"; import { resolveTypingMode } from "./typing-mode.js"; vi.mock("../../agents/auth-profiles/session-override.js", () => ({ - resolveSessionAuthProfileOverride: vi.fn().mockResolvedValue(undefined), + resolveSessionAuthSelection: vi.fn().mockResolvedValue(undefined), })); vi.mock("../../agents/embedded-agent.runtime.js", () => ({ @@ -2072,11 +2072,11 @@ describe("runPreparedReply media-only handling", () => { }); it("does not register a reply operation before auth setup succeeds", async () => { - const { resolveSessionAuthProfileOverride } = + const { resolveSessionAuthSelection } = await import("../../agents/auth-profiles/session-override.js"); const sessionId = "reply-operation-auth-failure"; const activeBefore = getActiveReplyRunCount(); - vi.mocked(resolveSessionAuthProfileOverride).mockRejectedValueOnce(new Error("auth failed")); + vi.mocked(resolveSessionAuthSelection).mockRejectedValueOnce(new Error("auth failed")); await expect( runPrepared({ @@ -2525,7 +2525,7 @@ describe("runPreparedReply media-only handling", () => { }); it("rechecks same-session ownership after async prep before registering a new reply operation", async () => { - const { resolveSessionAuthProfileOverride } = + const { resolveSessionAuthSelection } = await import("../../agents/auth-profiles/session-override.js"); const queueSettings = await import("./queue/settings-runtime.js"); @@ -2534,7 +2534,7 @@ describe("runPreparedReply media-only handling", () => { resolveAuth = resolve; }); - vi.mocked(resolveSessionAuthProfileOverride).mockImplementationOnce( + vi.mocked(resolveSessionAuthSelection).mockImplementationOnce( async () => await authPromise.then(() => undefined), ); vi.mocked(queueSettings.resolveQueueSettings).mockReturnValueOnce({ mode: "interrupt" }); @@ -2699,54 +2699,8 @@ describe("runPreparedReply media-only handling", () => { } }); - it.each([ - { - name: "legacy source-less user", - authProfileOverride: "profile-legacy-user", - authProfileOverrideCompactionCount: undefined, - expectedSource: "user", - }, - { - name: "legacy marker-backed automatic", - authProfileOverride: "profile-legacy-auto", - authProfileOverrideCompactionCount: 0, - expectedSource: "auto", - }, - ] as const)( - "forwards $name auth provenance to the runner", - async ({ authProfileOverride, authProfileOverrideCompactionCount, expectedSource }) => { - const { resolveSessionAuthProfileOverride } = - await import("../../agents/auth-profiles/session-override.js"); - const sessionEntry: SessionEntry = { - sessionId: `session-${authProfileOverride}`, - updatedAt: 1, - authProfileOverride, - ...(authProfileOverrideCompactionCount === undefined - ? {} - : { authProfileOverrideCompactionCount }), - }; - vi.mocked(resolveSessionAuthProfileOverride).mockImplementationOnce( - async ({ sessionEntry: resolvedEntry }) => resolvedEntry?.authProfileOverride, - ); - - await runPreparedReply( - baseParams({ - isNewSession: false, - sessionId: sessionEntry.sessionId, - sessionEntry, - sessionStore: { "session-key": sessionEntry }, - }), - ); - - expect(requireLastRunReplyAgentCall().followupRun.run).toMatchObject({ - authProfileId: authProfileOverride, - authProfileIdSource: expectedSource, - }); - }, - ); - it("re-resolves auth profile after waiting for a prior run", async () => { - const { resolveSessionAuthProfileOverride } = + const { resolveSessionAuthSelection } = await import("../../agents/auth-profiles/session-override.js"); const queueSettings = await import("./queue/settings-runtime.js"); const sessionStore: Record = { @@ -2758,8 +2712,14 @@ describe("runPreparedReply media-only handling", () => { updatedAt: 1, }, }; - vi.mocked(resolveSessionAuthProfileOverride).mockImplementation(async ({ sessionEntry }) => { - return sessionEntry?.authProfileOverride; + vi.mocked(resolveSessionAuthSelection).mockImplementation(async ({ sessionEntry }) => { + return sessionEntry?.authProfileOverride + ? { + profileId: sessionEntry.authProfileOverride, + source: sessionEntry.authProfileOverrideSource ?? "user", + routeRequirement: undefined, + } + : undefined; }); vi.mocked(queueSettings.resolveQueueSettings).mockReturnValueOnce({ mode: "interrupt" }); const previousRun = createReplyOperation({ @@ -2788,11 +2748,11 @@ describe("runPreparedReply media-only handling", () => { await expect(runPromise).resolves.toEqual({ text: "ok" }); const call = requireLastRunReplyAgentCall(); expect(call?.followupRun.run.authProfileId).toBe("profile-after-wait"); - expect(vi.mocked(resolveSessionAuthProfileOverride)).toHaveBeenCalledTimes(1); + expect(vi.mocked(resolveSessionAuthSelection)).toHaveBeenCalledTimes(1); }); it("re-resolves same-session ownership after session-id rotation during async prep", async () => { - const { resolveSessionAuthProfileOverride } = + const { resolveSessionAuthSelection } = await import("../../agents/auth-profiles/session-override.js"); const queueSettings = await import("./queue/settings-runtime.js"); @@ -2808,7 +2768,7 @@ describe("runPreparedReply media-only handling", () => { }, }; - vi.mocked(resolveSessionAuthProfileOverride).mockImplementationOnce( + vi.mocked(resolveSessionAuthSelection).mockImplementationOnce( async () => await authPromise.then(() => undefined), ); vi.mocked(queueSettings.resolveQueueSettings).mockReturnValueOnce({ mode: "interrupt" }); diff --git a/src/cron/isolated-agent.auth-profile-propagation.test.ts b/src/cron/isolated-agent.auth-profile-propagation.test.ts index 8d60f11852a8..7e46e94e6a8f 100644 --- a/src/cron/isolated-agent.auth-profile-propagation.test.ts +++ b/src/cron/isolated-agent.auth-profile-propagation.test.ts @@ -10,7 +10,7 @@ import { loadRunCronIsolatedAgentTurn, mockRunCronFallbackPassthrough, resolveConfiguredModelRefMock, - resolveSessionAuthProfileOverrideMock, + resolveSessionAuthSelectionMock, runEmbeddedAgentMock, } from "./isolated-agent/run.test-harness.js"; @@ -57,7 +57,11 @@ describe("runCronIsolatedAgentTurn auth profile propagation (#20624, #90991)", ( provider: "openrouter", model: "moonshotai/kimi-k2.5", }); - resolveSessionAuthProfileOverrideMock.mockResolvedValue("openrouter:default"); + resolveSessionAuthSelectionMock.mockResolvedValue({ + profileId: "openrouter:default", + source: "auto", + routeRequirement: "api-key", + }); mockRunCronFallbackPassthrough(); const result = await runCronIsolatedAgentTurn( diff --git a/src/cron/isolated-agent.isolated-auth-session-flag.test.ts b/src/cron/isolated-agent.isolated-auth-session-flag.test.ts index 69d309fe4774..9f9caeb0840a 100644 --- a/src/cron/isolated-agent.isolated-auth-session-flag.test.ts +++ b/src/cron/isolated-agent.isolated-auth-session-flag.test.ts @@ -6,7 +6,7 @@ import { makeCronSession, resolveConfiguredModelRefMock, resolveCronSessionMock, - resolveSessionAuthProfileOverrideMock, + resolveSessionAuthSelectionMock, resetRunCronIsolatedAgentTurnHarness, restoreFastTestEnv, } from "./isolated-agent/run.test-harness.js"; @@ -51,7 +51,7 @@ function makeParams( }; } -describe("isolated cron resolveSessionAuthProfileOverride isNewSession (#62783)", () => { +describe("isolated cron auth selection isNewSession (#62783)", () => { let previousFastTestEnv: string | undefined; beforeEach(() => { @@ -72,7 +72,11 @@ describe("isolated cron resolveSessionAuthProfileOverride isNewSession (#62783)" }, }), ); - resolveSessionAuthProfileOverrideMock.mockResolvedValue("openrouter:default"); + resolveSessionAuthSelectionMock.mockResolvedValue({ + profileId: "openrouter:default", + source: "auto", + routeRequirement: "api-key", + }); }); afterEach(() => { @@ -82,11 +86,11 @@ describe("isolated cron resolveSessionAuthProfileOverride isNewSession (#62783)" it("passes isNewSession=false when sessionTarget is isolated", async () => { await runCronIsolatedAgentTurn(makeParams()); - const openRouterCall = resolveSessionAuthProfileOverrideMock.mock.calls.find( + const openRouterCall = resolveSessionAuthSelectionMock.mock.calls.find( (call) => call[0]?.provider === "openrouter", ); if (!openRouterCall) { - throw new Error("resolveSessionAuthProfileOverride was not called with provider openrouter"); + throw new Error("resolveSessionAuthSelection was not called with provider openrouter"); } expect(openRouterCall[0]?.isNewSession).toBe(false); }); diff --git a/src/cron/isolated-agent/run-auth-profile.runtime.ts b/src/cron/isolated-agent/run-auth-profile.runtime.ts index 08e1a37a37d4..e590a8843a77 100644 --- a/src/cron/isolated-agent/run-auth-profile.runtime.ts +++ b/src/cron/isolated-agent/run-auth-profile.runtime.ts @@ -1,2 +1,2 @@ // Runtime auth-profile seam for isolated cron agent runs. -export { resolveSessionAuthProfileOverride } from "../../agents/auth-profiles/session-override.js"; +export { resolveSessionAuthSelection } from "../../agents/auth-profiles/session-override.js"; diff --git a/src/cron/isolated-agent/run-prepare.ts b/src/cron/isolated-agent/run-prepare.ts index 1f50c5bd1788..8712f4a166ce 100644 --- a/src/cron/isolated-agent/run-prepare.ts +++ b/src/cron/isolated-agent/run-prepare.ts @@ -2,11 +2,9 @@ import { isDeepStrictEqual } from "node:util"; import { hasAnyAuthProfileStoreSource } from "../../agents/auth-profiles/source-check.js"; import { findModelInCatalog } from "../../agents/model-catalog-lookup.js"; -import { listOpenAIAuthProfileProvidersForAgentRuntime } from "../../agents/openai-routing.js"; import { loadAgentRuntimePluginRegistryHandle } from "../../agents/runtime-plugins.js"; import { resolveAgentModelPrimaryValue } from "../../config/model-input.js"; import type { SessionEntry } from "../../config/sessions.js"; -import { resolveSessionAuthProfileOverrideSource } from "../../config/sessions/auth-profile-override-provenance.js"; import { resolveSessionWorkStartError } from "../../config/sessions/lifecycle.js"; import type { AgentDefaultsConfig } from "../../config/types.agent-defaults.js"; import type { OpenClawConfig } from "../../config/types.openclaw.js"; @@ -601,22 +599,19 @@ export async function prepareCronRunContext(params: { }); const storedAuthProfileId = cronSession.sessionEntry.authProfileOverride?.trim(); const hasSessionAuthProfileOverride = Boolean(storedAuthProfileId); - const authProfileId = + const authSelection = !hasSessionAuthProfileOverride && !hasConfiguredAuthProfiles(cfgWithAgentDefaults) && !hasAnyAuthProfileStoreSource(agentDir) ? undefined : await ( await loadCronAuthProfileRuntime() - ).resolveSessionAuthProfileOverride({ + ).resolveSessionAuthSelection({ // Auth resolution may mutate session state; use the store/key persistence will write. cfg: cfgWithAgentDefaults, provider, - acceptedProviderIds: listOpenAIAuthProfileProvidersForAgentRuntime({ - provider, - harnessRuntime: effectiveAgentRuntime, - config: cfgWithAgentDefaults, - }), + modelId: model, + harnessRuntime: effectiveAgentRuntime, agentDir, sessionEntry: cronSession.sessionEntry, sessionStore: cronSession.store, @@ -624,6 +619,7 @@ export async function prepareCronRunContext(params: { storePath: cronSession.storePath, isNewSession: cronSession.isNewSession && input.job.sessionTarget !== "isolated", }); + const authProfileId = authSelection?.profileId; const liveSelection: CronLiveSelection = { provider, model, @@ -633,11 +629,7 @@ export async function prepareCronRunContext(params: { cfg: cfgWithAgentDefaults, }), authProfileId, - authProfileIdSource: authProfileId - ? authProfileId === storedAuthProfileId - ? resolveSessionAuthProfileOverrideSource(cronSession.sessionEntry) - : "auto" - : undefined, + authProfileIdSource: authSelection?.source, }; const runtimePluginCandidates = selectedPreflightCandidateIndex >= 0 diff --git a/src/cron/isolated-agent/run.auth-profile-cold-path.test.ts b/src/cron/isolated-agent/run.auth-profile-cold-path.test.ts index f21b3f431ff1..c297ffd9431a 100644 --- a/src/cron/isolated-agent/run.auth-profile-cold-path.test.ts +++ b/src/cron/isolated-agent/run.auth-profile-cold-path.test.ts @@ -10,7 +10,7 @@ vi.mock("../../agents/auth-profiles/source-check.js", () => ({ import { clearFastTestEnv, loadRunCronIsolatedAgentTurn, - resolveSessionAuthProfileOverrideMock, + resolveSessionAuthSelectionMock, resetRunCronIsolatedAgentTurnHarness, restoreFastTestEnv, } from "./run.test-harness.js"; @@ -58,6 +58,6 @@ describe("runCronIsolatedAgentTurn auth-profile cold path", () => { expect(result.status).toBe("ok"); expect(hasAnyAuthProfileStoreSourceMock).toHaveBeenCalledTimes(1); - expect(resolveSessionAuthProfileOverrideMock).not.toHaveBeenCalled(); + expect(resolveSessionAuthSelectionMock).not.toHaveBeenCalled(); }); }); diff --git a/src/cron/isolated-agent/run.live-session-model-switch.test.ts b/src/cron/isolated-agent/run.live-session-model-switch.test.ts index 62a13e11095a..a29a64dbda15 100644 --- a/src/cron/isolated-agent/run.live-session-model-switch.test.ts +++ b/src/cron/isolated-agent/run.live-session-model-switch.test.ts @@ -10,7 +10,7 @@ import { resolveAllowedModelRefMock, resolveConfiguredModelRefMock, resolveCronSessionMock, - resolveSessionAuthProfileOverrideMock, + resolveSessionAuthSelectionMock, resetRunCronIsolatedAgentTurnHarness, runEmbeddedAgentMock, runWithModelFallbackMock, @@ -193,7 +193,11 @@ describe("runCronIsolatedAgentTurn — LiveSessionModelSwitchError retry (#57206 }); it("propagates a legacy source-less user auth profile into the run", async () => { - resolveSessionAuthProfileOverrideMock.mockResolvedValue("profile-a"); + resolveSessionAuthSelectionMock.mockResolvedValue({ + profileId: "profile-a", + source: "user", + routeRequirement: undefined, + }); resolveCronSessionMock.mockReturnValue( makeCronSession({ sessionEntry: makeCronSessionEntry({ @@ -222,7 +226,11 @@ describe("runCronIsolatedAgentTurn — LiveSessionModelSwitchError retry (#57206 }); it("keeps a resolved fallback profile automatic when it differs from the stored pin", async () => { - resolveSessionAuthProfileOverrideMock.mockResolvedValue("profile-b"); + resolveSessionAuthSelectionMock.mockResolvedValue({ + profileId: "profile-b", + source: "auto", + routeRequirement: undefined, + }); resolveCronSessionMock.mockReturnValue( makeCronSession({ sessionEntry: makeCronSessionEntry({ @@ -251,7 +259,11 @@ describe("runCronIsolatedAgentTurn — LiveSessionModelSwitchError retry (#57206 }); it("retries with switched auth profile state from LiveSessionModelSwitchError", async () => { - resolveSessionAuthProfileOverrideMock.mockResolvedValue("profile-a"); + resolveSessionAuthSelectionMock.mockResolvedValue({ + profileId: "profile-a", + source: "auto", + routeRequirement: undefined, + }); const cronSession = makeCronSession({ sessionEntry: makeCronSessionEntry({ model: undefined, diff --git a/src/cron/isolated-agent/run.test-harness.ts b/src/cron/isolated-agent/run.test-harness.ts index 0d5505c8de85..27c382014ac6 100644 --- a/src/cron/isolated-agent/run.test-harness.ts +++ b/src/cron/isolated-agent/run.test-harness.ts @@ -91,7 +91,7 @@ export const resolveDeliveryTargetMock = createMock(); export const dispatchCronDeliveryMock = createMock(); export const queueCronMessageToolDeliveryAwarenessMock = createMock(); export const preflightCronModelProviderMock = createMock(); -export const resolveSessionAuthProfileOverrideMock = createMock(); +export const resolveSessionAuthSelectionMock = createMock(); export const resolveFastModeStateMock = createMock(); export const getChannelPluginMock = createMock(); export const retireSessionMcpRuntimeMock = createMock(); @@ -335,7 +335,7 @@ vi.mock("../../agents/model-runtime-aliases.js", () => ({ })); vi.mock("./run-auth-profile.runtime.js", () => ({ - resolveSessionAuthProfileOverride: resolveSessionAuthProfileOverrideMock, + resolveSessionAuthSelection: resolveSessionAuthSelectionMock, })); vi.mock("./run-embedded.runtime.js", () => ({ @@ -759,8 +759,8 @@ function resetRunOutcomeMocks(): void { queueCronMessageToolDeliveryAwarenessMock.mockResolvedValue(undefined); preflightCronModelProviderMock.mockReset(); preflightCronModelProviderMock.mockResolvedValue({ status: "available" }); - resolveSessionAuthProfileOverrideMock.mockReset(); - resolveSessionAuthProfileOverrideMock.mockResolvedValue(undefined); + resolveSessionAuthSelectionMock.mockReset(); + resolveSessionAuthSelectionMock.mockResolvedValue(undefined); } function resetRunSessionMocks(): void { diff --git a/src/gateway/worker-environments/inference-runtime.test.ts b/src/gateway/worker-environments/inference-runtime.test.ts index 96061305fa30..9ae09a59580f 100644 --- a/src/gateway/worker-environments/inference-runtime.test.ts +++ b/src/gateway/worker-environments/inference-runtime.test.ts @@ -4,6 +4,7 @@ import { validateWorkerInferenceTerminalOutcome, type WorkerInferenceStartParams, } from "../../../packages/gateway-protocol/src/schema/worker-inference.js"; +import type { resolveSessionAuthSelection } from "../../agents/auth-profiles/session-override.js"; import type { applyExtraParamsToAgent } from "../../agents/embedded-agent-runner/extra-params.js"; import type { resolveModelAsync } from "../../agents/embedded-agent-runner/model.js"; import type { resolveEmbeddedAgentStreamFn } from "../../agents/embedded-agent-runner/stream-resolution.js"; @@ -43,7 +44,7 @@ type Deps = { applyStreamPolicy: typeof applyExtraParamsToAgent; acquireRuntimeLease: typeof acquireAgentRunPreparedModelRuntime; prepareModel: typeof prepareSimpleCompletionModel; - resolveAuthProfileMode: () => string | undefined; + resolveSessionAuthSelection: typeof resolveSessionAuthSelection; resolveModel: typeof resolveModelAsync; resolveProviderStream: typeof registerProviderStreamForModel; resolveStream: typeof resolveEmbeddedAgentStreamFn; @@ -247,7 +248,15 @@ function setup( }, }; }); - const resolveAuthProfileMode = vi.fn(() => undefined); + const resolveAuthSelection = vi.fn(async () => + entry.authProfileOverride + ? { + profileId: entry.authProfileOverride, + source: entry.authProfileOverrideSource ?? "user", + routeRequirement: undefined, + } + : undefined, + ); const observedRegistry = () => getPluginRuntimeGenerationRegistry() ?? getActivePluginRegistry(); const stream = vi.fn(() => { options.observeStage?.("execution", observedRegistry()); @@ -288,10 +297,9 @@ function setup( })), acquireRuntimeLease, resolveDefaultModel: vi.fn(() => ({ provider: PROVIDER, model: MODEL })), - resolveSessionAuthProfile: vi.fn(async () => entry.authProfileOverride), + resolveSessionAuthSelection: resolveAuthSelection, resolveModel, prepareModel, - resolveAuthProfileMode, resolveProviderStream, resolveStream, applyStreamPolicy, @@ -307,7 +315,7 @@ function setup( acquireRuntimeLease, prepareModel, releaseRuntime, - resolveAuthProfileMode, + resolveAuthSelection, scope, stream, }; @@ -363,12 +371,20 @@ describe("worker inference provider runtime", () => { it("projects the gateway-owned auth profile onto the provider route", async () => { const oauthRuntime = setup(); - oauthRuntime.resolveAuthProfileMode.mockReturnValue("oauth"); + oauthRuntime.resolveAuthSelection.mockResolvedValue({ + profileId: PROFILE, + source: "user", + routeRequirement: "subscription", + }); await oauthRuntime.executor(params(request(), vi.fn())); const oauth = oauthRuntime.prepareModel.mock.calls[0]?.[0].cfg ?? {}; const apiKeyRuntime = setup(); - apiKeyRuntime.resolveAuthProfileMode.mockReturnValue("api_key"); + apiKeyRuntime.resolveAuthSelection.mockResolvedValue({ + profileId: PROFILE, + source: "user", + routeRequirement: "api-key", + }); await apiKeyRuntime.executor(params(request(), vi.fn())); const apiKey = apiKeyRuntime.prepareModel.mock.calls[0]?.[0].cfg ?? {}; @@ -386,7 +402,11 @@ describe("worker inference provider runtime", () => { it("prepares the selected model against its gateway-owned OAuth route", async () => { const runtime = setup(); - runtime.resolveAuthProfileMode.mockReturnValue("oauth"); + runtime.resolveAuthSelection.mockResolvedValue({ + profileId: PROFILE, + source: "user", + routeRequirement: "subscription", + }); await expect(runtime.executor(params(request(), vi.fn()))).resolves.toMatchObject({ type: "done", @@ -405,7 +425,11 @@ describe("worker inference provider runtime", () => { authProfileOverrideSource: "auto", authProfileOverrideCompactionCount: 1, }); - runtime.resolveAuthProfileMode.mockReturnValue("oauth"); + runtime.resolveAuthSelection.mockResolvedValue({ + profileId: PROFILE, + source: "auto", + routeRequirement: "subscription", + }); await expect(runtime.executor(params(request(), vi.fn()))).resolves.toMatchObject({ type: "done", diff --git a/src/gateway/worker-environments/inference-runtime.ts b/src/gateway/worker-environments/inference-runtime.ts index ababc26d8e41..18057824dea4 100644 --- a/src/gateway/worker-environments/inference-runtime.ts +++ b/src/gateway/worker-environments/inference-runtime.ts @@ -14,8 +14,7 @@ import { resolveAgentWorkspaceDir, resolveDefaultAgentId, } from "../../agents/agent-scope.js"; -import { resolveSessionAuthProfileOverride } from "../../agents/auth-profiles/session-override.js"; -import { ensureAuthProfileStore } from "../../agents/auth-profiles/store.js"; +import { resolveSessionAuthSelection } from "../../agents/auth-profiles/session-override.js"; import { applyExtraParamsToAgent } from "../../agents/embedded-agent-runner/extra-params.js"; import { resolveModelAsync } from "../../agents/embedded-agent-runner/model.js"; import { wrapStreamFnWithDiagnosticModelCallEvents } from "../../agents/embedded-agent-runner/run/attempt.model-diagnostic-events.js"; @@ -35,12 +34,10 @@ import { createModelVisibilityPolicy, RUNTIME_MODEL_VISIBILITY_NORMALIZATION, } from "../../agents/model-visibility-policy.js"; -import { listOpenAIAuthProfileProvidersForAgentRuntime } from "../../agents/openai-routing.js"; import { acquireAgentRunPreparedModelRuntime, type PreparedModelRuntimeSnapshot, } from "../../agents/prepared-model-runtime.js"; -import { resolveProviderModelRouteAuthRequirement } from "../../agents/provider-model-route-auth.js"; import { projectProviderModelRouteConfig } from "../../agents/provider-model-route.js"; import { registerProviderStreamForModel } from "../../agents/provider-stream.js"; import { @@ -49,7 +46,6 @@ import { } from "../../agents/simple-completion-runtime.js"; import { normalizeUsage, hasNonzeroUsage } from "../../agents/usage.js"; import { getRuntimeConfig } from "../../config/config.js"; -import { resolveSessionAuthProfileOverrideSource } from "../../config/sessions/auth-profile-override-provenance.js"; import type { OpenClawConfig } from "../../config/types.openclaw.js"; import { emitTrustedDiagnosticEvent, isDiagnosticsEnabled } from "../../infra/diagnostic-events.js"; import { resolveDiagnosticModelContentCapturePolicy } from "../../infra/diagnostic-llm-content.js"; @@ -106,8 +102,7 @@ type WorkerInferenceRuntimeDependencies = { ) => WorkerInferenceSessionTarget | undefined; acquireRuntimeLease: typeof acquireAgentRunPreparedModelRuntime; resolveDefaultModel: typeof resolveDefaultModelForAgent; - resolveSessionAuthProfile: typeof resolveSessionAuthProfileOverride; - resolveAuthProfileMode: typeof resolveWorkerInferenceAuthProfileMode; + resolveSessionAuthSelection: typeof resolveSessionAuthSelection; resolveModel: typeof resolveModelAsync; prepareModel: typeof prepareSimpleCompletionModel; resolveProviderStream: typeof registerProviderStreamForModel; @@ -118,22 +113,6 @@ type WorkerInferenceRuntimeDependencies = { recordUsage: (params: WorkerInferenceUsageParams) => void; }; -function resolveWorkerInferenceAuthProfileMode(params: { - config: OpenClawConfig; - agentDir: string; - profileId: string; -}): string | undefined { - const configuredMode = params.config.auth?.profiles?.[params.profileId]?.mode; - if (configuredMode) { - return configuredMode; - } - return ensureAuthProfileStore(params.agentDir, { - readOnly: true, - allowKeychainPrompt: false, - config: params.config, - }).profiles[params.profileId]?.type; -} - const ERROR_MESSAGES = { "model-not-approved": "Model is not approved for this agent.", "invalid-context": "Inference context is invalid.", @@ -348,8 +327,7 @@ const DEFAULT_DEPENDENCIES: WorkerInferenceRuntimeDependencies = { }, acquireRuntimeLease: acquireAgentRunPreparedModelRuntime, resolveDefaultModel: resolveDefaultModelForAgent, - resolveSessionAuthProfile: resolveSessionAuthProfileOverride, - resolveAuthProfileMode: resolveWorkerInferenceAuthProfileMode, + resolveSessionAuthSelection, resolveModel: resolveModelAsync, prepareModel: prepareSimpleCompletionModel, resolveProviderStream: registerProviderStreamForModel, @@ -360,19 +338,6 @@ const DEFAULT_DEPENDENCIES: WorkerInferenceRuntimeDependencies = { recordUsage: emitWorkerInferenceUsage, }; -function resolveReturnedProfileSource( - entry: WorkerInferenceSessionTarget["sessionEntry"], - profileId: string | undefined, -): "auto" | "user" | undefined { - if (!profileId) { - return undefined; - } - if (entry.authProfileOverride?.trim() !== profileId) { - return "auto"; - } - return resolveSessionAuthProfileOverrideSource(entry); -} - async function resolveApprovedModel(params: { config: OpenClawConfig; target: WorkerInferenceSessionTarget; @@ -492,14 +457,12 @@ async function resolveApprovedModel(params: { lifecycleConfig.plugins?.entries?.codex?.enabled === true ? harnessPolicy.runtime : undefined; - const sessionProfileId = await dependencies.resolveSessionAuthProfile({ + const sessionSelection = await dependencies.resolveSessionAuthSelection({ cfg: lifecycleConfig, provider: resolved.ref.provider, - acceptedProviderIds: listOpenAIAuthProfileProvidersForAgentRuntime({ - provider: resolved.ref.provider, - harnessRuntime: harnessPolicy.runtime, - config: lifecycleConfig, - }), + modelId: resolved.ref.model, + ...(configuredDefaultProfile ? { configuredProfileId: configuredDefaultProfile } : {}), + harnessRuntime: harnessPolicy.runtime, agentDir, sessionEntry: target.sessionEntry, sessionStore: target.sessionStore, @@ -507,28 +470,10 @@ async function resolveApprovedModel(params: { storePath: target.storePath, isNewSession: false, }); - const sessionProfileSource = resolveReturnedProfileSource( - target.sessionEntry, - sessionProfileId, - ); - const selectedProfile = - sessionProfileId && sessionProfileSource === "user" - ? { id: sessionProfileId, source: sessionProfileSource } - : configuredDefaultProfile - ? { id: configuredDefaultProfile, source: "user" as const } - : sessionProfileId - ? { id: sessionProfileId, source: sessionProfileSource } - : undefined; + const selectedProfileId = sessionSelection?.profileId; + const routeRequirement = sessionSelection?.routeRequirement; let modelConfig = lifecycleConfig; - const authMode = selectedProfile - ? dependencies.resolveAuthProfileMode({ - config: lifecycleConfig, - agentDir, - profileId: selectedProfile.id, - }) - : undefined; - const authRequirement = resolveProviderModelRouteAuthRequirement(authMode); - const routeResolution = authRequirement + const routeResolution = routeRequirement ? resolveProviderModelRoutes({ provider: resolved.ref.provider, modelId: resolved.ref.model, @@ -538,7 +483,7 @@ async function resolveApprovedModel(params: { const route = routeResolution?.kind === "routes" ? routeResolution.routes.find( - (candidate) => candidate.authRequirement === authRequirement, + (candidate) => candidate.authRequirement === routeRequirement, ) : undefined; if (route) { @@ -559,9 +504,9 @@ async function resolveApprovedModel(params: { provider: resolved.ref.provider, modelId: resolved.ref.model, agentDir, - ...(selectedProfile ? { profileId: selectedProfile.id } : {}), - ...(selectedProfile ? { preferredProfile: selectedProfile.id } : {}), - ...(selectedProfile ? { bindAuthOwner: true } : {}), + ...(selectedProfileId ? { profileId: selectedProfileId } : {}), + ...(selectedProfileId ? { preferredProfile: selectedProfileId } : {}), + ...(selectedProfileId ? { bindAuthOwner: true } : {}), allowMissingApiKeyModes: ["aws-sdk"], modelResolver: dependencies.resolveModel, preparedModelRuntime: runtimeSnapshot, diff --git a/src/plugins/runtime/gateway-request-scope.ts b/src/plugins/runtime/gateway-request-scope.ts index a9d63de1f297..ea5e4df3df4b 100644 --- a/src/plugins/runtime/gateway-request-scope.ts +++ b/src/plugins/runtime/gateway-request-scope.ts @@ -53,15 +53,6 @@ export const getGatewayContextResolver = (owner: object) => gatewayContextResolv export const clearGatewayContextResolver = (owner: object) => gatewayContextResolvers.delete(owner); -export function getSharedGatewayContextResolver( - owners: readonly object[], -): GatewayContextResolver | undefined { - const first = owners[0] ? gatewayContextResolvers.get(owners[0]) : undefined; - return first && owners.every((owner) => gatewayContextResolvers.get(owner) === first) - ? first - : undefined; -} - /** * Runs plugin gateway handlers with request-scoped context that runtime helpers can read. */ From 6ccc57b331ae03de5c5df61cf00208769c6a8267 Mon Sep 17 00:00:00 2001 From: Samuel Judson Date: Tue, 18 Aug 2026 21:00:58 -0400 Subject: [PATCH 002/356] fix: add ssrf protection to Beam fetches (#123848) * Add ssrf protection to Beam fetches. * Additional robustness following initial comments. * fix(beam): make redirect failures terminal * chore(plugin-sdk): refresh surface budget * docs(beam): define redirect restart behavior * docs(beam): align redirect config help * test(beam): cover warning before redirect block * fix(beam): always report terminal redirect blocks --------- Co-authored-by: joshavant <830519+joshavant@users.noreply.github.com> --- docs/plugins/beam.md | 6 +- docs/plugins/sdk-subpaths.md | 2 +- extensions/beam/openclaw.plugin.json | 2 +- extensions/beam/src/mirror.test.ts | 279 +++++++++++++++++++++++++- extensions/beam/src/mirror.ts | 57 +++++- scripts/plugin-sdk-surface-report.mts | 3 +- src/infra/net/fetch-guard.ts | 20 +- src/plugin-sdk/ssrf-runtime.ts | 2 +- 8 files changed, 351 insertions(+), 20 deletions(-) diff --git a/docs/plugins/beam.md b/docs/plugins/beam.md index 169f000e161e..0980fa768a87 100644 --- a/docs/plugins/beam.md +++ b/docs/plugins/beam.md @@ -140,7 +140,7 @@ Beam can also act as the sender: an opt-in mirror that continuously publishes th } ``` -- `endpoint` (required): the remote receiver URL. HTTPS is enforced for non-loopback hosts; plaintext `http://` is accepted only for `localhost`/`127.0.0.1`/`::1` development. +- `endpoint` (required): the final remote receiver URL. Redirect responses (301, 302, 303, 307, and 308) are not followed; configure the destination URL directly. After a redirect, repeated polls are suppressed for the current mirror service instance. A Gateway restart probes the configured endpoint once again so a receiver corrected at the same URL can recover. HTTPS is enforced for non-loopback hosts; plaintext `http://` is accepted only for `localhost`/`127.0.0.1`/`::1` development. - `token`: Gateway credential for the remote receiver, sent as `Authorization: Bearer`. Accepts a plain string or a secret reference; a configured-but-unresolved token pauses mirroring instead of sending unauthenticated requests. Deployments fronted by an identity-aware proxy need an ingress that accepts this bearer credential. - `catalogs` (required): the session catalog ids to mirror, as explicit per-catalog consent — an omitted or empty list mirrors nothing. The local `beam` receiver catalog is always excluded so two mirrored Gateways cannot re-mirror each other's rows. - `pollSeconds` (default 30, minimum 10): how often the mirror scans local catalogs. @@ -170,6 +170,10 @@ The mirror applies the same redaction contract as the beam skill before anything : The authenticated client exceeded the bounded request or concurrency limit. Retry after the current minute window. +`beam mirror upload blocked ... receiver returned redirect` + +: The configured mirror endpoint returned a redirect. Beam does not follow redirects and suppresses repeated attempts for the current service instance; set `mirror.endpoint` to the final receiver URL. A Gateway restart probes the configured endpoint once again. + ## Related - [Control UI](/web/control-ui) diff --git a/docs/plugins/sdk-subpaths.md b/docs/plugins/sdk-subpaths.md index 4aab3766115e..798715c09975 100644 --- a/docs/plugins/sdk-subpaths.md +++ b/docs/plugins/sdk-subpaths.md @@ -201,7 +201,7 @@ usage endpoint failed or returned no usable usage data. | `plugin-sdk/security-runtime` | Deprecated broad barrel for trust, DM gating, root-bounded file/path helpers including create-only writes, sync/async atomic file replacement, sibling temp writes, cross-device move fallback, private file-store helpers, symlink-parent guards, external-content, sensitive text redaction, constant-time secret comparison, and secret-collection helpers; prefer focused security/SSRF/secret subpaths | | `plugin-sdk/ssrf-policy` | Host allowlist and private-network SSRF policy helpers | | `plugin-sdk/ssrf-dispatcher` | Private-local after July 2026; Narrow pinned-dispatcher helpers without the broad infra runtime surface | - | `plugin-sdk/ssrf-runtime` | Pinned-dispatcher, SSRF-guarded fetch, SSRF error, SSRF policy helpers, and loopback/private host classification | + | `plugin-sdk/ssrf-runtime` | Pinned-dispatcher, SSRF-guarded fetch, `SsrFBlockedError` and `GuardedFetchRedirectError`, SSRF policy helpers, and loopback/private host classification | | `plugin-sdk/secret-input` | Secret input parsing helpers | | `plugin-sdk/secret-ref-readonly` | Closed available/missing/blocked resolution and provider-policy checks for read-only env SecretRefs | | `plugin-sdk/webhook-ingress` | Webhook request/target helpers and raw websocket/body coercion | diff --git a/extensions/beam/openclaw.plugin.json b/extensions/beam/openclaw.plugin.json index 885d6f54a3f9..6d59bd953010 100644 --- a/extensions/beam/openclaw.plugin.json +++ b/extensions/beam/openclaw.plugin.json @@ -7,7 +7,7 @@ "uiHints": { "mirror.endpoint": { "label": "Mirror Endpoint", - "help": "Remote Beam receiver URL, e.g. https://team.example.com/api/v1/beam/sessions." + "help": "Final Beam receiver URL, e.g. https://team.example.com/api/v1/beam/sessions. Redirects are not followed; retries pause until the service restarts or the endpoint changes." }, "mirror.token": { "label": "Mirror Token", diff --git a/extensions/beam/src/mirror.test.ts b/extensions/beam/src/mirror.test.ts index 83b6a69ecc70..1469c69322a1 100644 --- a/extensions/beam/src/mirror.test.ts +++ b/extensions/beam/src/mirror.test.ts @@ -1,3 +1,4 @@ +import { createServer, type IncomingMessage, type Server } from "node:http"; import type { PluginRuntime } from "openclaw/plugin-sdk/plugin-runtime"; import type { SessionCatalogHost, @@ -5,6 +6,7 @@ import type { SessionsCatalogReadResult, } from "openclaw/plugin-sdk/session-catalog"; import type { ActiveSessionCatalog } from "openclaw/plugin-sdk/session-catalog-runtime"; +import * as ssrfRuntime from "openclaw/plugin-sdk/ssrf-runtime"; import { describe, expect, it, vi } from "vitest"; import { beamMirrorId, @@ -92,7 +94,7 @@ function captureFetch( status = 200, onCancel?: () => void | Promise, ): typeof fetch { - return (async (url: unknown, init?: RequestInit) => { + return vi.fn(async (url: unknown, init?: RequestInit) => { const headers = (init?.headers ?? {}) as Record; sent.push({ url: String(url), @@ -105,11 +107,44 @@ function captureFetch( }) : "{}"; return new Response(body, { status }); - }) as typeof fetch; + }) as unknown as typeof fetch; } const silentLogger = { warn: () => {}, info: () => {} }; +async function listenOnLoopback(server: Server): Promise { + await new Promise((resolve, reject) => { + server.once("error", reject); + server.listen(0, "127.0.0.1", () => { + server.off("error", reject); + resolve(); + }); + }); + const address = server.address(); + if (!address || typeof address === "string") { + throw new Error("test server did not expose a TCP address"); + } + return `http://127.0.0.1:${address.port}`; +} + +async function closeTestServer(server: Server): Promise { + if (!server.listening) { + return; + } + server.closeAllConnections(); + await new Promise((resolve, reject) => { + server.close((error) => (error ? reject(error) : resolve())); + }); +} + +async function readRequestBody(req: IncomingMessage): Promise { + const chunks: Buffer[] = []; + for await (const chunk of req) { + chunks.push(Buffer.isBuffer(chunk) ? chunk : Buffer.from(chunk)); + } + return Buffer.concat(chunks).toString("utf8"); +} + describe("parseBeamMirrorConfig", () => { it("returns undefined without mirror config", () => { expect(parseBeamMirrorConfig({ plugins: { entries: { beam: { enabled: true } } } })).toBe( @@ -223,6 +258,205 @@ describe("fitBeamMirrorUpload", () => { }); describe("createBeamMirrorRunner", () => { + it("does not replay mirror uploads across redirects to another private origin", async () => { + const redirectedBodies: string[] = []; + const internalServer = createServer((req, res) => { + void readRequestBody(req).then( + (body) => { + redirectedBodies.push(body); + res.statusCode = 200; + res.end("ok"); + }, + (error: unknown) => { + res.destroy(error instanceof Error ? error : new Error(String(error))); + }, + ); + }); + const internalOrigin = await listenOnLoopback(internalServer); + const receiverBodies: string[] = []; + const receiverServer = createServer((req, res) => { + void readRequestBody(req).then( + (body) => { + receiverBodies.push(body); + res.statusCode = 307; + res.setHeader("Location", `${internalOrigin}/internal-action`); + res.end(); + }, + (error: unknown) => { + res.destroy(error instanceof Error ? error : new Error(String(error))); + }, + ); + }); + try { + const receiverOrigin = await listenOnLoopback(receiverServer); + const runner = createBeamMirrorRunner({ + runtime: fakeRuntime(mirrorConfig({ endpoint: `${receiverOrigin}/beam` })), + logger: silentLogger, + now: () => NOW, + listCatalogs: () => [ + fakeCatalog({ id: "claude", sessions: [{ threadId: "t1", recencyAt: NOW }] }), + ], + }); + + await runner.tick(); + + expect(receiverBodies).toHaveLength(1); + expect(receiverBodies[0]).toContain("Fix the flow."); + expect(redirectedBodies).toEqual([]); + } finally { + await Promise.all([closeTestServer(receiverServer), closeTestServer(internalServer)]); + } + }); + + it.each([ + { label: "301", status: 301, location: "/redirected?private=do-not-log" }, + { label: "302", status: 302, location: "/redirected?private=do-not-log" }, + { label: "303", status: 303, location: "/redirected?private=do-not-log" }, + { label: "307", status: 307, location: "/redirected?private=do-not-log" }, + { label: "308", status: 308, location: "/redirected?private=do-not-log" }, + { label: "307 without Location", status: 307, location: undefined }, + ])( + "blocks a $label redirect without retrying the configured endpoint", + async ({ status, location }) => { + const warnings: string[] = []; + const receiverBodies: string[] = []; + const redirectedBodies: string[] = []; + const server = createServer((req, res) => { + void readRequestBody(req).then( + (body) => { + if (req.url === "/redirected") { + redirectedBodies.push(body); + res.statusCode = 200; + res.end("ok"); + return; + } + receiverBodies.push(body); + res.statusCode = status; + if (location) { + res.setHeader("Location", location); + } + res.end(); + }, + (error: unknown) => { + res.destroy(error instanceof Error ? error : new Error(String(error))); + }, + ); + }); + try { + const origin = await listenOnLoopback(server); + const runner = createBeamMirrorRunner({ + runtime: fakeRuntime(mirrorConfig({ endpoint: `${origin}/beam` })), + logger: { warn: (message) => warnings.push(message), info: () => {} }, + now: () => NOW, + listCatalogs: () => [ + fakeCatalog({ id: "claude", sessions: [{ threadId: "t1", recencyAt: NOW }] }), + ], + }); + + await runner.tick(); + await runner.tick(); + + expect(receiverBodies).toHaveLength(1); + expect(redirectedBodies).toEqual([]); + expect(warnings).toEqual([ + `beam mirror upload blocked for claude: receiver returned redirect (${status}); redirects are not followed; configure the final endpoint`, + ]); + expect(warnings.join(" ")).not.toContain("do-not-log"); + } finally { + await closeTestServer(server); + } + }, + ); + + it("logs a terminal redirect block after a recent transient warning", async () => { + const warnings: string[] = []; + let requestCount = 0; + const server = createServer((req, res) => { + void readRequestBody(req).then( + () => { + requestCount += 1; + res.statusCode = requestCount === 1 ? 503 : 307; + res.end(); + }, + (error: unknown) => { + res.destroy(error instanceof Error ? error : new Error(String(error))); + }, + ); + }); + try { + const origin = await listenOnLoopback(server); + const runner = createBeamMirrorRunner({ + runtime: fakeRuntime(mirrorConfig({ endpoint: `${origin}/beam` })), + logger: { warn: (message) => warnings.push(message), info: () => {} }, + now: () => NOW, + listCatalogs: () => [ + fakeCatalog({ id: "claude", sessions: [{ threadId: "t1", recencyAt: NOW }] }), + ], + }); + + await runner.tick(); + await runner.tick(); + await runner.tick(); + + expect(requestCount).toBe(2); + expect(warnings).toEqual([ + "beam mirror upload failed (503) for claude", + "beam mirror upload blocked for claude: receiver returned redirect (307); redirects are not followed; configure the final endpoint", + ]); + } finally { + await closeTestServer(server); + } + }); + + it("rechecks once after runner restart and resumes after the endpoint changes", async () => { + const requests: string[] = []; + const server = createServer((req, res) => { + void readRequestBody(req).then( + () => { + requests.push(req.url ?? ""); + if (req.url === "/redirecting") { + res.statusCode = 307; + res.setHeader("Location", "/redirected"); + } else { + res.statusCode = 200; + } + res.end(); + }, + (error: unknown) => { + res.destroy(error instanceof Error ? error : new Error(String(error))); + }, + ); + }); + try { + const origin = await listenOnLoopback(server); + let endpoint = `${origin}/redirecting`; + const runtime = { + config: { current: () => mirrorConfig({ endpoint }) }, + } as unknown as PluginRuntime; + const createRunner = () => + createBeamMirrorRunner({ + runtime, + logger: silentLogger, + now: () => NOW, + listCatalogs: () => [ + fakeCatalog({ id: "claude", sessions: [{ threadId: "t1", recencyAt: NOW }] }), + ], + }); + const runner = createRunner(); + + await runner.tick(); + await runner.tick(); + const restartedRunner = createRunner(); + await restartedRunner.tick(); + endpoint = `${origin}/direct`; + await restartedRunner.tick(); + + expect(requests).toEqual(["/redirecting", "/redirecting", "/direct"]); + } finally { + await closeTestServer(server); + } + }); + it("uploads active local sessions and skips unchanged ones", async () => { const sent: SentRequest[] = []; const reads: string[] = []; @@ -299,6 +533,47 @@ describe("createBeamMirrorRunner", () => { expect(warnings).toEqual([]); }); + it("bounds guarded uploads and releases their response resources", async () => { + const cancel = vi.fn(); + const release = vi.fn(); + const response = new Response( + new ReadableStream({ + cancel, + }), + { status: 200 }, + ); + const guardedFetch = vi.spyOn(ssrfRuntime, "fetchWithSsrFGuard").mockResolvedValue({ + response, + finalUrl: "https://team.example/api/v1/beam/sessions", + release, + }); + const runner = createBeamMirrorRunner({ + runtime: fakeRuntime(mirrorConfig()), + logger: silentLogger, + now: () => NOW, + listCatalogs: () => [ + fakeCatalog({ id: "claude", sessions: [{ threadId: "t1", recencyAt: NOW }] }), + ], + }); + + try { + await runner.tick(); + + expect(guardedFetch).toHaveBeenCalledWith( + expect.objectContaining({ + url: "https://team.example/api/v1/beam/sessions", + timeoutMs: 15_000, + maxRedirects: 0, + policy: { allowedOrigins: ["https://team.example"] }, + }), + ); + expect(cancel).toHaveBeenCalledOnce(); + expect(release).toHaveBeenCalledOnce(); + } finally { + guardedFetch.mockRestore(); + } + }); + it("ignores idle sessions, node hosts, the beam catalog, and unlisted catalogs", async () => { const sent: SentRequest[] = []; const idle = fakeCatalog({ diff --git a/extensions/beam/src/mirror.ts b/extensions/beam/src/mirror.ts index 1ee582cd966c..b4c904f04469 100644 --- a/extensions/beam/src/mirror.ts +++ b/extensions/beam/src/mirror.ts @@ -11,6 +11,11 @@ import { listActiveSessionCatalogs, type ActiveSessionCatalog, } from "openclaw/plugin-sdk/session-catalog-runtime"; +import { + fetchWithSsrFGuard, + GuardedFetchRedirectError, + ssrfPolicyFromHttpBaseUrlAllowedOrigin, +} from "openclaw/plugin-sdk/ssrf-runtime"; import { isRecord } from "openclaw/plugin-sdk/string-coerce-runtime"; import { truncateUtf16Safe } from "openclaw/plugin-sdk/text-utility-runtime"; import { BEAM_MAX_BODY_BYTES, BEAM_MAX_ITEM_CHARS, BEAM_MAX_ITEMS } from "./types.js"; @@ -30,6 +35,7 @@ const MIRROR_MAX_SESSIONS = 32; const MIRROR_BODY_BUDGET_BYTES = BEAM_MAX_BODY_BYTES - 2_048; // One warning per source per interval keeps a broken endpoint from flooding logs. const MIRROR_WARN_INTERVAL_MS = 5 * 60_000; +const MIRROR_UPLOAD_TIMEOUT_MS = 15_000; type BeamMirrorConfig = { endpoint: string; @@ -268,11 +274,11 @@ export function createBeamMirrorRunner(params: { listCatalogs?: () => ActiveSessionCatalog[]; }): BeamMirrorRunner { const env = params.env ?? process.env; - const fetchFn = params.fetchFn ?? fetch; const now = params.now ?? Date.now; const listCatalogs = params.listCatalogs ?? listActiveSessionCatalogs; const tracked = new Map(); let lastWarnAt = 0; + let redirectBlockedEndpoint: string | undefined; let running = false; const warnThrottled = (message: string) => { @@ -287,14 +293,46 @@ export function createBeamMirrorRunner(params: { token: string | undefined, payload: BeamMirrorUpload, ): Promise => { - const response = await fetchFn(endpoint, { - method: "POST", - headers: { - "Content-Type": "application/json", - ...(token ? { Authorization: `Bearer ${token}` } : {}), - }, - body: JSON.stringify(payload), - }); + if (redirectBlockedEndpoint === endpoint) { + return false; + } + redirectBlockedEndpoint = undefined; + + let guarded: Awaited>; + try { + guarded = await fetchWithSsrFGuard({ + url: endpoint, + fetchImpl: params.fetchFn, + timeoutMs: MIRROR_UPLOAD_TIMEOUT_MS, + policy: ssrfPolicyFromHttpBaseUrlAllowedOrigin(endpoint), + auditContext: "beam.mirror_upload", + // Only the configured receiver can acknowledge delivery. Following a redirect + // could fingerprint a payload that the receiver never accepted. + maxRedirects: 0, + init: { + method: "POST", + headers: { + "Content-Type": "application/json", + ...(token ? { Authorization: `Bearer ${token}` } : {}), + }, + body: JSON.stringify(payload), + }, + }); + } catch (error) { + if (error instanceof GuardedFetchRedirectError) { + // Repeating the same poll cannot satisfy direct-only delivery. Hold this exact + // endpoint for this service instance; a fresh instance probes once so a receiver + // fixed in place can recover without a meaningless config change. + redirectBlockedEndpoint = endpoint; + params.logger.warn( + `beam mirror upload blocked for ${payload.source}: receiver returned redirect (${error.status}); redirects are not followed; configure the final endpoint`, + ); + return false; + } + throw error; + } + + const { response, release } = guarded; try { if (!response.ok) { warnThrottled(`beam mirror upload failed (${response.status}) for ${payload.source}`); @@ -305,6 +343,7 @@ export function createBeamMirrorRunner(params: { // The mirror uses only the status; cancel the ignored payload so slow // receiver responses cannot retain connection slots across poll retries. await response.body?.cancel().catch(() => undefined); + await release(); } }; diff --git a/scripts/plugin-sdk-surface-report.mts b/scripts/plugin-sdk-surface-report.mts index 3e4ba3b375e1..17358dabcb3a 100644 --- a/scripts/plugin-sdk-surface-report.mts +++ b/scripts/plugin-sdk-surface-report.mts @@ -301,7 +301,8 @@ export function readPluginSdkSurfaceBudgets(env: NodeJS.ProcessEnv = process.env // +2: shared delegation policy (mode resolver + section builder) so harness // runtimes render the same guidance instead of diverging prompt copies. // +1: shared harness visible-source-reply guidance. - 4335, + // +1: typed guarded-fetch redirect error for direct-only plugin delivery. + 4336, env, ), publicFunctionExports: readPluginSdkSurfaceBudgetEnv( diff --git a/src/infra/net/fetch-guard.ts b/src/infra/net/fetch-guard.ts index 22d8836b2494..285024cb9769 100644 --- a/src/infra/net/fetch-guard.ts +++ b/src/infra/net/fetch-guard.ts @@ -110,6 +110,18 @@ export type GuardedFetchResult = { dispatcherReused?: boolean; }; +export class GuardedFetchRedirectError extends Error { + readonly status: number; + readonly maxRedirects: number; + + constructor(params: { status: number; maxRedirects: number }) { + super(`Too many redirects (limit: ${params.maxRedirects})`); + this.name = "GuardedFetchRedirectError"; + this.status = params.status; + this.maxRedirects = params.maxRedirects; + } +} + type GuardedFetchInternalOptions = GuardedFetchOptions & { managedProxyBypass?: ConfiguredLocalOriginManagedProxyBypass; resolveDispatcherPolicy?: (url: URL) => PinnedDispatcherPolicy | undefined; @@ -721,14 +733,14 @@ async function fetchWithSsrFGuardInternal( }); if (isRedirectStatus(response.status)) { + redirectCount += 1; + if (redirectCount > maxRedirects) { + throw new GuardedFetchRedirectError({ status: response.status, maxRedirects }); + } const location = response.headers.get("location"); if (!location) { throw new Error(`Redirect missing location header (${response.status})`); } - redirectCount += 1; - if (redirectCount > maxRedirects) { - throw new Error(`Too many redirects (limit: ${maxRedirects})`); - } const nextParsedUrl = new URL(location, parsedUrl); const nextUrl = nextParsedUrl.toString(); const retainedAuthorization = resolveRetainedAuthorizationForRedirect({ diff --git a/src/plugin-sdk/ssrf-runtime.ts b/src/plugin-sdk/ssrf-runtime.ts index c73213fe661e..176570270105 100644 --- a/src/plugin-sdk/ssrf-runtime.ts +++ b/src/plugin-sdk/ssrf-runtime.ts @@ -16,7 +16,7 @@ export { type SsrFPolicy, } from "../infra/net/ssrf.js"; export { formatErrorMessage } from "../infra/errors.js"; -export { fetchWithSsrFGuard } from "../infra/net/fetch-guard.js"; +export { fetchWithSsrFGuard, GuardedFetchRedirectError } from "../infra/net/fetch-guard.js"; export { assertHttpUrlTargetsPrivateNetwork, buildHostnameAllowlistPolicyFromSuffixAllowlist, From 54ebb307cdfc28f8c232162179b9734b19662dbd Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Tue, 18 Aug 2026 18:10:45 -0700 Subject: [PATCH 003/356] fix(mcp): retire terminal transports and process trees (#126101) Close terminal SSE and stateful notification streams through the owning lifecycle, and reap stdio/QA process groups before exact authority is discarded.\n\nRefs #126098, #126099, #126100. --- src/agents/mcp-http-transport.test.ts | 70 +++++++++++++++++++ src/agents/mcp-http-transport.ts | 21 +++++- .../mcp-stdio-transport.process.test.ts | 37 ++++++++++ src/agents/mcp-stdio-transport.test.ts | 31 ++------ src/agents/mcp-stdio-transport.ts | 11 +-- .../gateway-node-mcp.test-support.test.ts | 31 ++++++++ .../runtime/gateway-node-mcp.test-support.ts | 43 ++++++++++-- 7 files changed, 202 insertions(+), 42 deletions(-) diff --git a/src/agents/mcp-http-transport.test.ts b/src/agents/mcp-http-transport.test.ts index b5b483f2c37f..030910308a43 100644 --- a/src/agents/mcp-http-transport.test.ts +++ b/src/agents/mcp-http-transport.test.ts @@ -75,6 +75,52 @@ describe("OpenClaw MCP HTTP lifecycle adapters", () => { await vi.waitFor(() => expect(onclose).toHaveBeenCalledOnce()); }); + it("closes an established legacy SSE transport after a terminal reconnect response", async () => { + let streamController: ReadableStreamDefaultController | undefined; + const encoder = new TextEncoder(); + const fetchMock = vi.fn(async (_input: RequestInfo | URL, init?: RequestInit) => { + const method = init?.method ?? "GET"; + if (method !== "GET") { + return new Response(null, { status: 202 }); + } + if (!streamController) { + return new Response( + new ReadableStream({ + start(controller) { + streamController = controller; + controller.enqueue( + encoder.encode("retry: 1\n\nevent: endpoint\ndata: /messages\n\n"), + ); + }, + }), + { headers: { "content-type": "text/event-stream" } }, + ); + } + return new Response(null, { status: 503, statusText: "Unavailable" }); + }); + const transport = new OpenClawSSEClientTransport(new URL("http://mcp.invalid/sse"), { + fetch: fetchMock, + eventSourceInit: { fetch: fetchMock }, + }); + const onclose = vi.fn(); + // MCP transports expose callback properties rather than EventTarget listeners. + // oxlint-disable-next-line unicorn/prefer-add-event-listener + transport.onclose = onclose; + + try { + await transport.start(); + streamController?.close(); + + await vi.waitFor(() => expect(onclose).toHaveBeenCalledOnce()); + await expect(transport.send({ jsonrpc: "2.0", id: 1, method: "tools/list" })).rejects.toThrow( + "closed", + ); + expect(fetchMock.mock.calls.filter((call) => call[1]?.method === "POST")).toHaveLength(0); + } finally { + await transport.close(); + } + }); + it("closes after Streamable notification retry exhaustion", async () => { let getCount = 0; const fetchMock = initializedFetch({ @@ -107,6 +153,30 @@ describe("OpenClaw MCP HTTP lifecycle adapters", () => { expect(fetchMock.mock.calls.filter((call) => call[1]?.method === "GET")).toHaveLength(3); }); + it("closes a stateful Streamable session when its initial notification GET expired", async () => { + const fetchMock = initializedFetch({ + onGet: () => new Response("Session not found", { status: 404, statusText: "Not Found" }), + }); + const transport = new OpenClawStreamableHTTPClientTransport(new URL("http://mcp.invalid/mcp"), { + fetch: fetchMock, + }); + const client = new Client({ name: "test", version: "1" }); + const onclose = vi.fn(); + // MCP clients expose callback properties rather than EventTarget listeners. + // oxlint-disable-next-line unicorn/prefer-add-event-listener + client.onclose = onclose; + + try { + await client.connect(transport); + + await vi.waitFor(() => expect(onclose).toHaveBeenCalledOnce()); + expect(transport.sessionId).toBe("session-1"); + expect(fetchMock.mock.calls.filter((call) => call[1]?.method === "GET")).toHaveLength(1); + } finally { + await disposeMcpClient({ client, transport, transportType: "streamable-http" }); + } + }); + it("sends stateful DELETE after failed initialization closed the SDK transport", async () => { const deleteRequests: RequestInit[] = []; const fetchMock = vi.fn(async (_input: RequestInfo | URL, init?: RequestInit) => { diff --git a/src/agents/mcp-http-transport.ts b/src/agents/mcp-http-transport.ts index 807745597309..c71100633563 100644 --- a/src/agents/mcp-http-transport.ts +++ b/src/agents/mcp-http-transport.ts @@ -58,7 +58,7 @@ export class OpenClawSSEClientTransport extends OpenClawMcpHttpTransport { // oxlint-disable-next-line unicorn/prefer-add-event-listener this.transport.onerror = (error) => { this.emitError(error); - if (error instanceof SseError && error.code === 204) { + if (error instanceof SseError && error.code !== undefined) { void this.close(); } }; @@ -75,6 +75,9 @@ export class OpenClawSSEClientTransport extends OpenClawMcpHttpTransport { } async send(message: JSONRPCMessage): Promise { + if (this.closed) { + throw new Error("MCP SSE transport is closed"); + } await this.transport.send(message); } @@ -94,6 +97,7 @@ export class OpenClawStreamableHTTPClientTransport extends OpenClawMcpHttpTransp private readonly url: URL; private readonly cleanupFetch: FetchLike; private readonly requestInit?: RequestInit; + private pendingExpiredNotificationGet = false; private terminatedSessionId?: string; constructor(url: URL, options: OpenClawStreamableHttpOptions = {}) { @@ -105,7 +109,11 @@ export class OpenClawStreamableHTTPClientTransport extends OpenClawMcpHttpTransp if (this.closed) { throw new Error("MCP Streamable HTTP transport is closed"); } - return await this.cleanupFetch(input, init); + const response = await this.cleanupFetch(input, init); + if (init?.method === "GET" && response.status === 404 && this.sessionId !== undefined) { + this.pendingExpiredNotificationGet = true; + } + return response; }; this.transport = new StreamableHTTPClientTransport(url, { ...options, @@ -136,7 +144,14 @@ export class OpenClawStreamableHTTPClientTransport extends OpenClawMcpHttpTransp return; } this.emitError(error); - if (STREAM_RETRY_EXHAUSTED_RE.test(error.message)) { + const sessionExpired = + this.pendingExpiredNotificationGet && + error instanceof StreamableHTTPError && + error.code === 404; + if (sessionExpired) { + this.pendingExpiredNotificationGet = false; + } + if (sessionExpired || STREAM_RETRY_EXHAUSTED_RE.test(error.message)) { void this.close(); } }; diff --git a/src/agents/mcp-stdio-transport.process.test.ts b/src/agents/mcp-stdio-transport.process.test.ts index 3b983c99a67a..d218baf0af5e 100644 --- a/src/agents/mcp-stdio-transport.process.test.ts +++ b/src/agents/mcp-stdio-transport.process.test.ts @@ -52,4 +52,41 @@ describe.skipIf(process.platform === "win32")("OpenClaw stdio process-group owne } }, ); + + it( + "kills same-group descendants after a graceful leader shutdown", + { timeout: 10_000 }, + async () => { + const root = tempDirs.make("mcp-stdio-graceful-descendant-"); + const serverPath = path.join(root, "leader.mjs"); + const descendantPidPath = path.join(root, "descendant.pid"); + await fs.writeFile( + serverPath, + `import {spawn} from "node:child_process"; import fs from "node:fs"; const child=spawn(process.execPath,["-e","setInterval(()=>{},1000)"],{stdio:"ignore"}); fs.writeFileSync(${JSON.stringify(descendantPidPath)},String(child.pid)); process.stdin.resume(); process.stdin.on("end",()=>process.exit(0));`, + "utf8", + ); + const transport = new OpenClawStdioClientTransport({ + command: process.execPath, + args: [serverPath], + stderr: "ignore", + }); + let descendantPid = 0; + try { + await transport.start(); + await vi.waitFor(async () => { + descendantPid = Number(await fs.readFile(descendantPidPath, "utf8")); + expect(isPidAlive(descendantPid)).toBe(true); + }); + + await transport.close(); + + await vi.waitFor(() => expect(isPidAlive(descendantPid)).toBe(false)); + } finally { + await transport.forceClose(); + if (descendantPid && isPidAlive(descendantPid)) { + process.kill(descendantPid, "SIGKILL"); + } + } + }, + ); }); diff --git a/src/agents/mcp-stdio-transport.test.ts b/src/agents/mcp-stdio-transport.test.ts index aa93535cd2aa..b15857b7d864 100644 --- a/src/agents/mcp-stdio-transport.test.ts +++ b/src/agents/mcp-stdio-transport.test.ts @@ -7,7 +7,6 @@ import { afterEach, describe, expect, it, vi } from "vitest"; import { OpenClawStdioClientTransport } from "./mcp-stdio-transport.js"; const spawnMock = vi.hoisted(() => vi.fn()); -const killProcessTreeMock = vi.hoisted(() => vi.fn()); const signalProcessTreeMock = vi.hoisted(() => vi.fn()); vi.mock("node:child_process", async () => ({ @@ -16,7 +15,6 @@ vi.mock("node:child_process", async () => ({ })); vi.mock("../process/kill-tree.js", () => ({ - killProcessTree: killProcessTreeMock, signalProcessTree: signalProcessTreeMock, })); @@ -35,7 +33,6 @@ describe("OpenClawStdioClientTransport", () => { vi.useRealTimers(); vi.restoreAllMocks(); spawnMock.mockReset(); - killProcessTreeMock.mockReset(); signalProcessTreeMock.mockReset(); }); @@ -113,14 +110,14 @@ describe("OpenClawStdioClientTransport", () => { const closing = transport.close(); await vi.advanceTimersByTimeAsync(2000); - expect(killProcessTreeMock).toHaveBeenCalledWith(4321, { detached: true }); + expect(signalProcessTreeMock).toHaveBeenCalledWith(4321, "SIGTERM", { detached: true }); child.exitCode = 0; child.emit("close", 0); await closing; }); - it("force-SIGKILLs synchronously when killProcessTree's grace expires (#86412)", async () => { + it("force-SIGKILLs synchronously when the owned process group outlives TERM", async () => { vi.useFakeTimers(); const child = new MockChildProcess(); spawnMock.mockReturnValue(child); @@ -132,11 +129,9 @@ describe("OpenClawStdioClientTransport", () => { const closing = transport.close(); await vi.advanceTimersByTimeAsync(2000); - expect(killProcessTreeMock).toHaveBeenCalledWith(4321, { detached: true }); - expect(signalProcessTreeMock).not.toHaveBeenCalled(); + expect(signalProcessTreeMock).toHaveBeenCalledWith(4321, "SIGTERM", { detached: true }); + expect(signalProcessTreeMock).not.toHaveBeenCalledWith(4321, "SIGKILL", { detached: true }); - // killProcessTree's SIGKILL is .unref()'d (#86412); close() force-SIGKILLs - // synchronously instead. await vi.advanceTimersByTimeAsync(2000); expect(signalProcessTreeMock).toHaveBeenCalledWith(4321, "SIGKILL", { detached: true }); @@ -168,24 +163,6 @@ describe("OpenClawStdioClientTransport", () => { expect(transport.pid).toBeNull(); }); - it("does not kill the process tree when graceful stdio close exits", async () => { - vi.useFakeTimers(); - const child = new MockChildProcess(); - spawnMock.mockReturnValue(child); - - const transport = new OpenClawStdioClientTransport({ command: "npx" }); - const started = transport.start(); - child.emit("spawn"); - await started; - - const closing = transport.close(); - child.exitCode = 0; - child.emit("close", 0); - await closing; - - expect(killProcessTreeMock).not.toHaveBeenCalled(); - }); - it("immediately kills the retained process group after the stdio leader exits", async () => { vi.useFakeTimers(); const child = new MockChildProcess(); diff --git a/src/agents/mcp-stdio-transport.ts b/src/agents/mcp-stdio-transport.ts index 9045b1139082..c6d9c23beb48 100644 --- a/src/agents/mcp-stdio-transport.ts +++ b/src/agents/mcp-stdio-transport.ts @@ -11,7 +11,7 @@ import type { Transport } from "@modelcontextprotocol/sdk/shared/transport.js"; import type { JSONRPCMessage } from "@modelcontextprotocol/sdk/types.js"; import { formatErrorMessage } from "../infra/errors.js"; import { mergeProcessEnv } from "../infra/process-env.js"; -import { killProcessTree, signalProcessTree } from "../process/kill-tree.js"; +import { signalProcessTree } from "../process/kill-tree.js"; import { prepareOomScoreAdjustedSpawn } from "../process/linux-oom-score.js"; type OpenClawStdioServerParameters = { @@ -96,11 +96,10 @@ export class OpenClawStdioClientTransport implements Transport { }); child.on("spawn", () => resolve()); child.on("close", () => { - const exitedUnexpectedly = this.process === child && this.closingProcess !== child; if (this.process === child) { this.process = undefined; } - if (exitedUnexpectedly && child.pid && this.ownedProcessGroupId === child.pid) { + if (child.pid && this.ownedProcessGroupId === child.pid) { // The leader still owns this PGID at close notification time. Kill any // descendants now so a retained numeric PGID can never outlive ownership. signalProcessTree(child.pid, "SIGKILL", { detached: true }); @@ -153,9 +152,6 @@ export class OpenClawStdioClientTransport implements Transport { const ownedProcessGroupId = this.ownedProcessGroupId; this.process = undefined; this.closingProcess = processToClose; - if (processToClose) { - this.closingProcess = processToClose; - } if (processToClose) { const closePromise = new Promise((resolve) => { processToClose.once("close", () => resolve()); @@ -167,10 +163,9 @@ export class OpenClawStdioClientTransport implements Transport { } await Promise.race([closePromise, delay(CLOSE_TIMEOUT_MS)]); if (processToClose.exitCode === null && processToClose.pid) { - killProcessTree(processToClose.pid, { detached: true }); + signalProcessTree(processToClose.pid, "SIGTERM", { detached: true }); await Promise.race([closePromise, delay(CLOSE_TIMEOUT_MS)]); if (processToClose.exitCode === null && processToClose.pid) { - // SIGKILL synchronously: killProcessTree's setTimeout is .unref()'d and races shutdown (#86412). signalProcessTree(processToClose.pid, "SIGKILL", { detached: true }); await Promise.race([closePromise, delay(SIGKILL_REAP_TIMEOUT_MS)]); } diff --git a/test/e2e/qa-lab/runtime/gateway-node-mcp.test-support.test.ts b/test/e2e/qa-lab/runtime/gateway-node-mcp.test-support.test.ts index fc9ed5b62439..e03f7b9eb1d6 100644 --- a/test/e2e/qa-lab/runtime/gateway-node-mcp.test-support.test.ts +++ b/test/e2e/qa-lab/runtime/gateway-node-mcp.test-support.test.ts @@ -8,6 +8,7 @@ import { parseNodeMcpTextRecord, processIsAlive, startHttpFixture, + stopChild, } from "./gateway-node-mcp.test-support.js"; const tempDirs = useAutoCleanupTempDirTracker(afterEach); @@ -50,4 +51,34 @@ describe("gateway node MCP fixture ownership", () => { } expect(processIsAlive(pid)).toBe(false); }); + + it("kills task-owned fixture descendants when stopping the captured root", async () => { + const root = tempDirs.make("mcp-fixture-descendant-cleanup-"); + const fixturePath = path.join(root, "fixture.mjs"); + const descendantPidPath = path.join(root, "descendant.pid"); + await fs.writeFile( + fixturePath, + `import {spawn} from "node:child_process"; import fs from "node:fs"; const child=spawn(process.execPath,["-e","setInterval(()=>{},1000)"],{stdio:"ignore"}); fs.writeFileSync(${JSON.stringify(descendantPidPath)},String(child.pid)); console.log(JSON.stringify({type:"openclaw-mcp-parity-ready",urls:{streamableHttp:"http://127.0.0.1/mcp",sse:"http://127.0.0.1/sse"}})); setInterval(()=>{},1000);`, + "utf8", + ); + + const fixture = await startHttpFixture({ + fixturePath, + labelPrefix: "node", + env: createChildEnv({ home: root, tempDir: os.tmpdir() }), + }); + const descendantPid = Number(await fs.readFile(descendantPidPath, "utf8")); + try { + expect(processIsAlive(descendantPid)).toBe(true); + + await stopChild(fixture); + + await vi.waitFor(() => expect(processIsAlive(descendantPid)).toBe(false), { timeout: 1_000 }); + } finally { + await stopChild(fixture); + if (processIsAlive(descendantPid)) { + process.kill(descendantPid, "SIGKILL"); + } + } + }); }); diff --git a/test/e2e/qa-lab/runtime/gateway-node-mcp.test-support.ts b/test/e2e/qa-lab/runtime/gateway-node-mcp.test-support.ts index 8a2809cb1369..e64505ca5940 100644 --- a/test/e2e/qa-lab/runtime/gateway-node-mcp.test-support.ts +++ b/test/e2e/qa-lab/runtime/gateway-node-mcp.test-support.ts @@ -9,6 +9,7 @@ import { expect, vi } from "vitest"; import type { startQaGatewayChild } from "../../../../extensions/qa-lab/api.js"; import type { NodePluginToolDescriptor } from "../../../../packages/gateway-protocol/src/schema/nodes.js"; import type { McpServerConfig } from "../../../../src/config/types.mcp.js"; +import { signalProcessTree } from "../../../../src/process/kill-tree.js"; export const TEST_TIMEOUT_MS = 180_000; const WAIT_TIMEOUT_MS = 30_000; @@ -25,6 +26,7 @@ export type CapturedChild = { child: ChildProcess; exited: Promise; logs: () => string; + signalTree: (signal: "SIGTERM" | "SIGKILL") => Promise; }; export type HttpFixture = CapturedChild & { pid: number; @@ -66,10 +68,41 @@ function captureChild(child: ChildProcess): CapturedChild { child.stderr?.on("data", (chunk: Buffer) => { stderr = (stderr + chunk.toString()).slice(-200_000); }); + const pid = child.pid; + let termPromise: Promise | undefined; + let killPromise: Promise | undefined; + const signalTree = (signal: "SIGTERM" | "SIGKILL") => { + const existing = signal === "SIGKILL" ? killPromise : termPromise; + if (existing) { + return existing; + } + const signaled = new Promise((resolve) => { + if (pid === undefined) { + resolve(); + return; + } + signalProcessTree(pid, signal, { + detached: process.platform !== "win32", + onComplete: resolve, + }); + }); + if (signal === "SIGKILL") { + killPromise = signaled; + } else { + termPromise = signaled; + } + return signaled; + }; + const exited = once(child, "exit").then(async () => { + // The root PID still identifies this task-owned tree at exit delivery. Reap + // descendants before any retained numeric process-group authority can age. + await signalTree("SIGKILL"); + }); return { child, - exited: once(child, "exit").then(() => {}), + exited, logs: () => `stdout:\n${stdout}\nstderr:\n${stderr}`, + signalTree, }; } @@ -107,6 +140,7 @@ export async function startHttpFixture(params: { const captured = captureChild( spawn(process.execPath, [params.fixturePath, "http", "--label-prefix", params.labelPrefix], { cwd: process.cwd(), + detached: process.platform !== "win32", env: params.env, stdio: ["ignore", "pipe", "pipe"], }), @@ -174,6 +208,7 @@ export function startNodeProcess(gatewayPort: number, nodeEnv: NodeJS.ProcessEnv ], { cwd: process.cwd(), + detached: process.platform !== "win32", env: nodeEnv, stdio: ["ignore", "pipe", "pipe"], }, @@ -186,15 +221,15 @@ export async function stopChild(captured: CapturedChild | undefined): Promise {}); return; } - captured.child.kill("SIGTERM"); + await captured.signalTree("SIGTERM"); const graceful = await Promise.race([ captured.exited.then(() => true), delay(10_000, false, { ref: false }), ]); if (!graceful) { - captured.child.kill("SIGKILL"); - await captured.exited; + await captured.signalTree("SIGKILL"); } + await captured.exited; } export function processIsAlive(pid: number): boolean { From 66c7d720bd88f11cc82a9e5405e0fb8716c674bf Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Tue, 18 Aug 2026 18:12:16 -0700 Subject: [PATCH 004/356] fix(models): seed configured provider model entries from catalog metadata (#126068) * fix(config): seed configured provider model entries from catalog metadata applyModelDefaults replaced omitted input/reasoning/cost/maxTokens on models.providers entries with generic defaults at config load, erasing the fact that the operator never authored them. An override entry that pins only sizing fields materialized as a text-only, non-reasoning, zero-cost model, which silently dropped vision-gated tools (most visibly the computer tool) for that model downstream. Seed omitted fields from the owning plugin manifest catalog row (same provider+model id, using the existing id-normalization policies) before falling back to generic defaults. Authored fields always win; entries with no catalog row behave exactly as before. * fix(config): preserve tiered pricing when seeding model cost defaults resolveModelCost keeps only flat per-token fields; carry an authored or catalog tieredPricing table through explicitly so cost defaulting does not silently discard it. --- src/config/defaults.test.ts | 198 ++++++++++++++++++++++++++++++++++++ src/config/defaults.ts | 138 +++++++++++++++++++++---- 2 files changed, 319 insertions(+), 17 deletions(-) diff --git a/src/config/defaults.test.ts b/src/config/defaults.test.ts index 2f06bbd6a56c..a7bfcbf9486f 100644 --- a/src/config/defaults.test.ts +++ b/src/config/defaults.test.ts @@ -138,3 +138,201 @@ describe("config defaults", () => { expect(next.agents?.defaults?.subagents?.maxConcurrent).toBe(DEFAULT_SUBAGENT_MAX_CONCURRENT); }); }); + +describe("applyModelDefaults catalog seeding", () => { + const catalogRegistry = { + plugins: [ + { + id: "openai", + modelCatalog: { + providers: { + openai: { + models: [ + { + id: "gpt-5.6-sol", + name: "GPT-5.6 Sol", + reasoning: true, + input: ["text", "image"], + contextWindow: 400_000, + contextTokens: 272_000, + maxTokens: 128_000, + cost: { input: 5, output: 30, cacheRead: 0.5, cacheWrite: 6.25 }, + thinkingLevelMap: { off: "none" }, + }, + ], + }, + }, + }, + }, + ], + // SAFETY: minimal manifest record carrying only the fields applyModelDefaults reads. + } as never; + + // Regression: an override entry pinning only sizing fields materialized as a + // text-only, non-reasoning, zero-cost model, silently dropping vision-gated + // tools (like `computer`) for that model downstream. + it("fills omitted fields from the owning catalog row before generic defaults", async () => { + const { applyModelDefaults } = await import("./defaults.js"); + const cfg = applyModelDefaults( + { + models: { + providers: { + openai: { + baseUrl: "https://api.openai.com/v1", + models: [ + // SAFETY: mirrors a real operator config entry that omits input/reasoning/cost. + { + id: "gpt-5.6-sol", + name: "GPT-5.6", + contextWindow: 1_050_000, + contextTokens: 922_000, + } as never, + ], + }, + }, + }, + }, + { manifestRegistry: catalogRegistry }, + ); + const model = expectDefined( + cfg.models?.providers?.openai?.models?.[0], + "materialized model entry", + ); + expect(model.input).toEqual(["text", "image"]); + expect(model.reasoning).toBe(true); + expect(model.cost).toEqual({ input: 5, output: 30, cacheRead: 0.5, cacheWrite: 6.25 }); + expect(model.maxTokens).toBe(128_000); + expect(model.thinkingLevelMap).toEqual({ off: "none" }); + // Authored fields stay authoritative. + expect(model.contextWindow).toBe(1_050_000); + expect(model.contextTokens).toBe(922_000); + expect(model.name).toBe("GPT-5.6"); + }); + + it("keeps authored metadata authoritative over the catalog row", async () => { + const { applyModelDefaults } = await import("./defaults.js"); + const cfg = applyModelDefaults( + { + models: { + providers: { + openai: { + baseUrl: "https://api.openai.com/v1", + models: [ + { + id: "gpt-5.6-sol", + name: "text-only override", + reasoning: false, + input: ["text"], + cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0 }, + contextWindow: 8_192, + maxTokens: 4_096, + }, + ], + }, + }, + }, + }, + { manifestRegistry: catalogRegistry }, + ); + const model = expectDefined( + cfg.models?.providers?.openai?.models?.[0], + "materialized model entry", + ); + expect(model.input).toEqual(["text"]); + expect(model.reasoning).toBe(false); + expect(model.maxTokens).toBe(4_096); + }); + + it("preserves catalog tiered pricing when flat cost fields are authored", async () => { + const { applyModelDefaults } = await import("./defaults.js"); + const tieredRegistry = { + plugins: [ + { + id: "openai", + modelCatalog: { + providers: { + openai: { + models: [ + { + id: "gpt-5.6-sol", + name: "GPT-5.6 Sol", + input: ["text", "image"], + cost: { + input: 5, + output: 30, + cacheRead: 0.5, + cacheWrite: 6.25, + tieredPricing: [ + { + input: 2.5, + output: 15, + cacheRead: 0.25, + cacheWrite: 3, + maxInputTokens: 200_000, + }, + ], + }, + }, + ], + }, + }, + }, + }, + ], + // SAFETY: minimal manifest record carrying only the fields applyModelDefaults reads. + } as never; + const cfg = applyModelDefaults( + { + models: { + providers: { + openai: { + baseUrl: "https://api.openai.com/v1", + models: [ + // SAFETY: mirrors an operator override with explicit flat cost only. + { + id: "gpt-5.6-sol", + name: "GPT-5.6", + cost: { input: 4, output: 24, cacheRead: 0.4, cacheWrite: 5 }, + } as never, + ], + }, + }, + }, + }, + { manifestRegistry: tieredRegistry }, + ); + const model = expectDefined( + cfg.models?.providers?.openai?.models?.[0], + "materialized model entry", + ); + expect(model.cost?.input).toBe(4); + expect(model.cost?.tieredPricing).toHaveLength(1); + expect(model.input).toEqual(["text", "image"]); + }); + + it("falls back to generic defaults when no catalog row matches", async () => { + const { applyModelDefaults } = await import("./defaults.js"); + const cfg = applyModelDefaults( + { + models: { + providers: { + custom: { + baseUrl: "https://custom.example.com/v1", + models: [ + // SAFETY: mirrors a real operator config entry that omits input/reasoning/cost. + { id: "house-model", name: "House Model" } as never, + ], + }, + }, + }, + }, + { manifestRegistry: catalogRegistry }, + ); + const model = expectDefined( + cfg.models?.providers?.custom?.models?.[0], + "materialized model entry", + ); + expect(model.input).toEqual(["text"]); + expect(model.reasoning).toBe(false); + }); +}); diff --git a/src/config/defaults.ts b/src/config/defaults.ts index 2f27a1d5e86b..f4cd93b4306e 100644 --- a/src/config/defaults.ts +++ b/src/config/defaults.ts @@ -157,6 +157,59 @@ export function applyTalkConfigNormalization(config: OpenClawConfig): OpenClawCo return normalizeTalkConfig(config); } +/** Catalog metadata eligible to fill fields the operator did not author. */ +type CatalogSeedModel = Pick< + ModelDefinitionConfig, + | "input" + | "reasoning" + | "cost" + | "contextWindow" + | "contextTokens" + | "maxTokens" + | "thinkingLevelMap" + | "compat" +>; + +/** + * Indexes plugin manifest catalog rows so configured model entries can inherit + * metadata the operator omitted. Without this, materialization would turn an + * override entry that pins only sizing fields into a text-only, non-reasoning, + * zero-cost model — silently dropping vision-gated tools downstream. + */ +function buildManifestCatalogModelLookup( + manifestRegistry: Pick | undefined, + policies: ReturnType | undefined, +): (providerId: string, modelId: string) => Partial | undefined { + const plugins = manifestRegistry?.plugins; + if (!plugins || plugins.length === 0) { + return () => undefined; + } + let index: Map> | undefined; + const keyFor = (providerId: string, modelId: string) => + normalizeProviderId(providerId) + + " " + + normalizeConfiguredProviderCatalogModelId(providerId, modelId, policies).toLowerCase(); + return (providerId, modelId) => { + if (!index) { + index = new Map(); + for (const plugin of plugins) { + for (const [catalogProviderId, provider] of Object.entries( + plugin.modelCatalog?.providers ?? {}, + )) { + for (const model of provider.models) { + const key = keyFor(catalogProviderId, model.id); + if (!index.has(key)) { + // SAFETY: ModelCatalogModel's seed fields are a structural subset of ModelDefinitionConfig; only the picked metadata fields are read from this entry. + index.set(key, model as Partial); + } + } + } + } + } + return index.get(keyFor(providerId, modelId)); + }; +} + export function applyModelDefaults( cfg: OpenClawConfig, options: ProviderPolicyDefaultsOptions = {}, @@ -170,6 +223,10 @@ export function applyModelDefaults( const modelIdNormalizationPolicies = manifestRegistry ? collectManifestModelIdNormalizationPolicies(manifestRegistry.plugins) : undefined; + const resolveCatalogModel = buildManifestCatalogModelLookup( + manifestRegistry, + modelIdNormalizationPolicies, + ); const nextProviders = { ...providerConfig }; for (const [providerId, provider] of Object.entries(providerConfig)) { const normalizedProvider = normalizeProviderConfigForConfigDefaults({ @@ -206,33 +263,58 @@ export function applyModelDefaults( modelMutated = true; } - const reasoning = typeof raw.reasoning === "boolean" ? raw.reasoning : false; + // Config entries are overrides, not full definitions: authored fields + // win, the owning catalog row fills omitted fields, and only then do + // generic defaults apply. Defaulting straight past the catalog would + // erase field absence (for example turning an entry that pins only + // contextWindow into a text-only model, dropping vision-gated tools). + const catalogModel = resolveCatalogModel(providerId, id); + const reasoning = + typeof raw.reasoning === "boolean" ? raw.reasoning : (catalogModel?.reasoning ?? false); if (raw.reasoning !== reasoning) { modelMutated = true; } - const input = raw.input ?? [...DEFAULT_MODEL_INPUT]; + const input = raw.input ?? catalogModel?.input ?? [...DEFAULT_MODEL_INPUT]; if (raw.input === undefined) { modelMutated = true; } - const cost = resolveModelCost(raw.cost); + const cost = resolveModelCost( + raw.cost || catalogModel?.cost ? { ...catalogModel?.cost, ...raw.cost } : undefined, + ); + // resolveModelCost keeps only the flat per-token fields; carry tiered + // pricing through explicitly so an authored or catalog tier table is + // not silently discarded when other cost fields are defaulted. + const tieredPricing = raw.cost?.tieredPricing ?? catalogModel?.cost?.tieredPricing; + if (tieredPricing) { + cost.tieredPricing = tieredPricing; + } const costMutated = !raw.cost || raw.cost.input !== cost.input || raw.cost.output !== cost.output || raw.cost.cacheRead !== cost.cacheRead || - raw.cost.cacheWrite !== cost.cacheWrite; + raw.cost.cacheWrite !== cost.cacheWrite || + raw.cost.tieredPricing !== cost.tieredPricing; if (costMutated) { modelMutated = true; } - const contextWindow = isPositiveNumber(raw.contextWindow) ? raw.contextWindow : undefined; + const contextWindow = isPositiveNumber(raw.contextWindow) + ? raw.contextWindow + : isPositiveNumber(catalogModel?.contextWindow) + ? catalogModel.contextWindow + : undefined; if (raw.contextWindow !== contextWindow) { modelMutated = true; } - const contextTokens = isPositiveNumber(raw.contextTokens) ? raw.contextTokens : undefined; + const contextTokens = isPositiveNumber(raw.contextTokens) + ? raw.contextTokens + : isPositiveNumber(catalogModel?.contextTokens) + ? catalogModel.contextTokens + : undefined; if (raw.contextTokens !== contextTokens) { modelMutated = true; } @@ -242,7 +324,11 @@ export function applyModelDefaults( providerMaxTokens ?? DEFAULT_MODEL_MAX_TOKENS, maxTokenContextWindow, ); - const rawMaxTokens = isPositiveNumber(raw.maxTokens) ? raw.maxTokens : defaultMaxTokens; + const rawMaxTokens = isPositiveNumber(raw.maxTokens) + ? raw.maxTokens + : isPositiveNumber(catalogModel?.maxTokens) + ? catalogModel.maxTokens + : defaultMaxTokens; const maxTokens = resolveNormalizedProviderModelMaxTokens({ providerId, modelId: id, @@ -257,20 +343,38 @@ export function applyModelDefaults( modelMutated = true; } + const thinkingLevelMap = + raw.thinkingLevelMap === undefined && catalogModel?.thinkingLevelMap !== undefined + ? catalogModel.thinkingLevelMap + : undefined; + const compat = + raw.compat === undefined && catalogModel?.compat !== undefined + ? catalogModel.compat + : undefined; + if (thinkingLevelMap !== undefined || compat !== undefined) { + modelMutated = true; + } + if (!modelMutated) { return model; } providerMutated = true; - return Object.assign({}, raw, { - id, - reasoning, - input, - cost, - contextWindow, - contextTokens, - maxTokens, - api, - }) as ModelDefinitionConfig; + return Object.assign( + {}, + raw, + { + id, + reasoning, + input, + cost, + contextWindow, + contextTokens, + maxTokens, + api, + }, + thinkingLevelMap !== undefined ? { thinkingLevelMap } : {}, + compat !== undefined ? { compat } : {}, + ) as ModelDefinitionConfig; }); if (!providerMutated) { From f3c076cacbe69f213668edfc7bd8479ca4bfbcca Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Tue, 18 Aug 2026 18:14:25 -0700 Subject: [PATCH 005/356] test(release): remove release-check helper barrel (#126105) --- scripts/release-check.ts | 5 ----- test/release-check.test.ts | 14 ++------------ 2 files changed, 2 insertions(+), 17 deletions(-) diff --git a/scripts/release-check.ts b/scripts/release-check.ts index 994d8f46aa71..88de32fe248d 100755 --- a/scripts/release-check.ts +++ b/scripts/release-check.ts @@ -64,9 +64,6 @@ type ReleaseCheckExecOptions = ExecFileSyncOptions & { windowsVerbatimArguments?: boolean; }; -export { collectBundledExtensionManifestErrors } from "./lib/bundled-extension-manifest.ts"; -export { packageNameFromSpecifier } from "./lib/plugin-package-dependencies.mts"; - export const RELEASE_CHECK_LOCAL_PACKAGE_TARBALL_DIR_ENV = "OPENCLAW_RELEASE_CHECK_LOCAL_PACKAGE_TARBALL_DIR"; @@ -1159,8 +1156,6 @@ export function collectForbiddenPackContentPaths( .toSorted((left, right) => left.localeCompare(right)); } -export { collectPackUnpackedSizeErrors } from "./lib/npm-pack-budget.mts"; - function extractTag(item: string, tag: string): string | null { const escapedTag = escapeRegExp(tag); const regex = new RegExp(`<${escapedTag}>([^<]+)`); diff --git a/test/release-check.test.ts b/test/release-check.test.ts index 8df84136c409..ab6e7e1538de 100644 --- a/test/release-check.test.ts +++ b/test/release-check.test.ts @@ -4,8 +4,10 @@ import { tmpdir } from "node:os"; import { dirname, join, resolve as resolvePath, win32 } from "node:path"; import { bundledDistPluginFile, bundledPluginFile } from "openclaw/plugin-sdk/test-fixtures"; import { describe, expect, it } from "vitest"; +import { collectBundledExtensionManifestErrors } from "../scripts/lib/bundled-extension-manifest.ts"; import { listBundledPluginPackArtifacts } from "../scripts/lib/bundled-plugin-build-entries.mjs"; import { resolveNpmJsonEntries } from "../scripts/lib/npm-json-output.mts"; +import { collectPackUnpackedSizeErrors } from "../scripts/lib/npm-pack-budget.mts"; import { LOCAL_BUILD_METADATA_DIST_PATHS, PACKAGE_DIST_INVENTORY_RELATIVE_PATH, @@ -25,13 +27,11 @@ import { } from "../scripts/openclaw-npm-postpublish-verify.ts"; import { collectAppcastSparkleVersionErrors, - collectBundledExtensionManifestErrors, collectCriticalPluginSdkEntrypointSizeErrors, collectForbiddenPackContentPaths, collectForbiddenPackPaths, collectMissingPackPaths, collectSkillShellScriptExecutableErrors, - collectPackUnpackedSizeErrors, collectPackedInstalledPackageVerificationErrors, createPackedPluginSdkTypescriptSmokeProject, createPackedCompletionSmokeEnv, @@ -41,7 +41,6 @@ import { PACKED_BUNDLED_RUNTIME_DEPS_REPAIR_ARGS, PACKED_CLI_SMOKE_COMMANDS, PACKED_COMPLETION_SMOKE_ARGS, - packageNameFromSpecifier, resolvePackedTarballPath, resolveReleaseNpmCommand, resolveMissingPackBuildHint, @@ -392,15 +391,6 @@ describe("collectBundledExtensionManifestErrors", () => { }); describe("bundled plugin package dependency checks", () => { - it("maps package names from import specifiers", () => { - expect(packageNameFromSpecifier("@larksuiteoapi/node-sdk/subpath")).toBe( - "@larksuiteoapi/node-sdk", - ); - expect(packageNameFromSpecifier("grammy/web")).toBe("grammy"); - expect(packageNameFromSpecifier("node:fs")).toBeNull(); - expect(packageNameFromSpecifier("./local")).toBeNull(); - }); - it("does not require root deps for root chunks sourced from the owning installed plugin", () => { const tempRoot = mkdtempSync(join(tmpdir(), "openclaw-root-owned-installed-")); From 61eb7b932b91702b24e6207a6d8726e5fa891e3e Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Tue, 18 Aug 2026 18:18:53 -0700 Subject: [PATCH 006/356] fix(agents): keep guided auth atomic through creation (#126096) * fix(agents): make guided auth creation atomic * fix(auth): keep staged agent workspace explicit * test(auth): use managed temp directory * fix(gateway): finish detached lifecycle dispatch migration * fix(auth): carry staged workspace through providers * test(auth): align mocks with batch persistence --- extensions/cloudflare-ai-gateway/index.ts | 1 + extensions/lmstudio/index.ts | 1 + extensions/lmstudio/src/setup.ts | 2 + extensions/microsoft-foundry/auth.ts | 1 + extensions/ollama/index.ts | 1 + extensions/ollama/src/setup.runtime.ts | 4 + extensions/pixverse/index.test.ts | 2 + extensions/pixverse/onboard.ts | 1 + extensions/xiaomi/index.ts | 1 + extensions/zai/index.ts | 1 + src/agents/agent-create.test.ts | 54 ++++ src/agents/agent-create.ts | 20 ++ src/agents/auth-profiles.ts | 1 + .../upsert-with-lock.sqlite.test.ts | 196 +++++++++++++ src/agents/auth-profiles/upsert-with-lock.ts | 162 ++++++++++- src/commands/agents.add.test.ts | 258 ++++++++++++++++-- src/commands/agents.commands.add.ts | 106 ++++--- .../auth-choice.apply.plugin-provider.test.ts | 60 ++-- src/commands/auth-choice.test.ts | 17 ++ src/plugins/provider-api-key-auth.ts | 1 + src/plugins/provider-auth-choice.ts | 64 +---- src/plugins/provider-auth-input.test.ts | 9 +- src/plugins/provider-auth-input.ts | 11 +- 23 files changed, 807 insertions(+), 167 deletions(-) create mode 100644 src/agents/auth-profiles/upsert-with-lock.sqlite.test.ts diff --git a/extensions/cloudflare-ai-gateway/index.ts b/extensions/cloudflare-ai-gateway/index.ts index 95bc00078040..5a1665aeee58 100644 --- a/extensions/cloudflare-ai-gateway/index.ts +++ b/extensions/cloudflare-ai-gateway/index.ts @@ -99,6 +99,7 @@ export default definePluginEntry({ ? (ctx.secretInputMode ?? "plaintext") : ctx.secretInputMode, config: ctx.config, + workspaceDir: ctx.workspaceDir, expectedProviders: [PROVIDER_ID], provider: PROVIDER_ID, envLabel: PROVIDER_ENV_VAR, diff --git a/extensions/lmstudio/index.ts b/extensions/lmstudio/index.ts index d7d2c8aa6d66..c0dcc500eb30 100644 --- a/extensions/lmstudio/index.ts +++ b/extensions/lmstudio/index.ts @@ -190,6 +190,7 @@ export default definePluginEntry({ return await providerSetup.promptAndConfigureLmstudioInteractive({ config: ctx.config, agentDir: ctx.agentDir, + workspaceDir: ctx.workspaceDir, prompter: ctx.prompter, secretInputMode: ctx.secretInputMode, allowSecretRefPrompt: ctx.allowSecretRefPrompt, diff --git a/extensions/lmstudio/src/setup.ts b/extensions/lmstudio/src/setup.ts index 74ae9adeba2c..c88e9fd8ba80 100644 --- a/extensions/lmstudio/src/setup.ts +++ b/extensions/lmstudio/src/setup.ts @@ -508,6 +508,7 @@ export async function prepareAppGuidedLmstudioSetup( export async function promptAndConfigureLmstudioInteractive(params: { config: OpenClawConfig; agentDir?: string; + workspaceDir?: string; prompter?: WizardPrompter; secretInputMode?: SecretInputMode; allowSecretRefPrompt?: boolean; @@ -541,6 +542,7 @@ export async function promptAndConfigureLmstudioInteractive(params: { : params.prompter ? await ensureApiKeyFromEnvOrPrompt({ config: params.config, + workspaceDir: params.workspaceDir, provider: PROVIDER_ID, envLabel: LMSTUDIO_DEFAULT_API_KEY_ENV_VAR, promptMessage: `${LMSTUDIO_PROVIDER_LABEL} API key`, diff --git a/extensions/microsoft-foundry/auth.ts b/extensions/microsoft-foundry/auth.ts index 92eb69c0b356..16e6d863653c 100644 --- a/extensions/microsoft-foundry/auth.ts +++ b/extensions/microsoft-foundry/auth.ts @@ -235,6 +235,7 @@ export const apiKeyAuthMethod: ProviderAuthMethod = { ? (ctx.secretInputMode ?? "plaintext") : ctx.secretInputMode, config: ctx.config, + workspaceDir: ctx.workspaceDir, expectedProviders: [PROVIDER_ID], provider: PROVIDER_ID, envLabel: "AZURE_OPENAI_API_KEY", diff --git a/extensions/ollama/index.ts b/extensions/ollama/index.ts index a4be634634fb..9fb7b6c74ab4 100644 --- a/extensions/ollama/index.ts +++ b/extensions/ollama/index.ts @@ -1050,6 +1050,7 @@ export default definePluginEntry({ const result = await promptAndConfigureOllama({ cfg: ctx.config, env: ctx.env, + workspaceDir: ctx.workspaceDir, opts: ctx.opts as Record | undefined, prompter: ctx.prompter, ...(ctx.signal ? { signal: ctx.signal } : {}), diff --git a/extensions/ollama/src/setup.runtime.ts b/extensions/ollama/src/setup.runtime.ts index 506e7a203c70..cfe08eb0d6ce 100644 --- a/extensions/ollama/src/setup.runtime.ts +++ b/extensions/ollama/src/setup.runtime.ts @@ -147,6 +147,7 @@ export async function checkOllamaCloudAuth( async function promptForOllamaCloudCredential(params: { cfg: OpenClawConfig; env?: NodeJS.ProcessEnv; + workspaceDir?: string; opts?: Record; prompter: WizardPrompter; secretInputMode?: SecretInputMode; @@ -169,6 +170,7 @@ async function promptForOllamaCloudCredential(params: { : params.secretInputMode, config: params.cfg, env: params.env, + workspaceDir: params.workspaceDir, expectedProviders: ["ollama"], provider: "ollama", envLabel: "OLLAMA_API_KEY", @@ -383,6 +385,7 @@ async function promptAndConfigureHostBackedOllama(params: { export async function promptAndConfigureOllama(params: { cfg: OpenClawConfig; env?: NodeJS.ProcessEnv; + workspaceDir?: string; opts?: Record; prompter: WizardPrompter; secretInputMode?: SecretInputMode; @@ -405,6 +408,7 @@ export async function promptAndConfigureOllama(params: { const { credential, credentialMode, discoveryApiKey } = await promptForOllamaCloudCredential({ cfg: params.cfg, env: params.env, + workspaceDir: params.workspaceDir, opts: params.opts, prompter: params.prompter, secretInputMode: params.secretInputMode, diff --git a/extensions/pixverse/index.test.ts b/extensions/pixverse/index.test.ts index a6845526786a..073426c6c116 100644 --- a/extensions/pixverse/index.test.ts +++ b/extensions/pixverse/index.test.ts @@ -24,6 +24,7 @@ function registerPixVerseProvider() { function createRuntimeContext( region: "international" | "cn", config: Record = { + agents: { entries: { main: {}, work: { workspace: "/tmp/pixverse-workspace" } } }, models: { providers: { pixverse: { @@ -42,6 +43,7 @@ function createRuntimeContext( const ctx = { config, env: {}, + workspaceDir: "/tmp/pixverse-workspace", prompter: { intro: vi.fn(), outro: vi.fn(), diff --git a/extensions/pixverse/onboard.ts b/extensions/pixverse/onboard.ts index 83b904b9039b..75b452e1c852 100644 --- a/extensions/pixverse/onboard.ts +++ b/extensions/pixverse/onboard.ts @@ -144,6 +144,7 @@ async function runPixVerseApiKeyAuth(ctx: ProviderAuthContext): Promise { expect(mocks.ensureAgentWorkspace).toHaveBeenCalledOnce(); }); + it("prepares staged config effects after setup and immediately before publication", async () => { + mocks.ensureAgentWorkspace.mockResolvedValue({ + dir: "/tmp/default-researcher", + bootstrapPending: false, + }); + const prepareConfigCommit = vi.fn(async () => { + expect(mocks.ensureAgentWorkspace).toHaveBeenCalledOnce(); + expect(mocks.mkdir).toHaveBeenCalledOnce(); + expect(mocks.rootWrite).toHaveBeenCalledOnce(); + expect(mocks.persisted).not.toHaveProperty("agents"); + }); + + await createAgent({ name: "researcher", prepareConfigCommit }); + + expect(prepareConfigCommit).toHaveBeenCalledOnce(); + expect(mocks.persisted).toHaveProperty("agents.entries.researcher"); + }); + + it("rolls staged config effects back once when config publication fails", async () => { + const rollback = vi.fn(); + const prepareConfigCommit = vi.fn(async () => rollback); + mocks.transformConfigFileWithRetry.mockImplementationOnce(async ({ transform }) => { + await transform(structuredClone(mocks.config), { + snapshot: { exists: false }, + previousHash: null, + }); + throw new Error("injected config commit failure"); + }); + + await expect(createAgent({ name: "researcher", prepareConfigCommit })).rejects.toThrow( + "injected config commit failure", + ); + + expect(prepareConfigCommit).toHaveBeenCalledOnce(); + expect(rollback).toHaveBeenCalledOnce(); + }); + + it("does not roll staged config effects back after config publication", async () => { + const rollback = vi.fn(); + mocks.recordAgentProvenance.mockImplementationOnce(() => { + throw new Error("injected provenance failure"); + }); + + await expect( + createAgent({ + name: "researcher", + prepareConfigCommit: async () => rollback, + }), + ).rejects.toThrow("injected provenance failure"); + + expect(mocks.persisted).toHaveProperty("agents.entries.researcher"); + expect(rollback).not.toHaveBeenCalled(); + }); + it("keeps the template identity while bootstrap is pending", async () => { await createAgent({ name: "researcher" }); diff --git a/src/agents/agent-create.ts b/src/agents/agent-create.ts index 7c7eff62b477..b9f36ac9ba30 100644 --- a/src/agents/agent-create.ts +++ b/src/agents/agent-create.ts @@ -66,6 +66,7 @@ type CreateError = { type CreateAgentResult = (CreateAgentSuccess & { config: OpenClawConfig }) | CreateError; type AgentEntryConfig = NonNullable["entries"]>[string]; type CreateAgentEntry = AgentEntryConfig & { id: string }; +type ConfigCommitRollback = () => void | Promise; type CreateAgentParams = { name?: string; @@ -87,6 +88,8 @@ type CreateAgentParams = { skipOptionalBootstrapFiles?: OptionalBootstrapFileName[]; bindingSpecs?: string[]; transformConfig?: typeof transformConfigFileWithRetry; + /** Prepare guided staged state at the last reversible edge before config publication. */ + prepareConfigCommit?: () => Promise; provenance?: { createdVia: AgentCreatedVia; creatorAgentId?: string }; }; @@ -255,6 +258,7 @@ export async function createAgent(params: CreateAgentParams): Promise { @@ -436,6 +440,10 @@ export async function createAgent(params: CreateAgentParams): Promise Promise): Promise { + const root = tempDirs.make("openclaw-auth-batch-"); + const agentDir = path.join(root, "agents", "work", "agent"); + fs.mkdirSync(agentDir, { recursive: true }); + try { + await withEnvAsync( + { OPENCLAW_STATE_DIR: root, OPENCLAW_AGENT_DIR: agentDir }, + async () => await run(agentDir), + ); + } finally { + closeOpenClawAgentDatabasesForTest(); + closeOpenClawStateDatabaseForTest(); + } +} + +describe("auth profile batch persistence", () => { + it("conditionally rolls a portable profile batch and its order back to absence", async () => { + await withAgentDir(async (agentDir) => { + const noOp = await persistAuthProfileBatch({ agentDir, profiles: [] }); + noOp.rollback(); + expect(fs.existsSync(resolveAuthProfileDatabasePath(agentDir))).toBe(false); + + const receipt = await persistAuthProfileBatch({ + agentDir, + profiles: [ + profile("openai:primary", " sk-primary "), + { + profileId: "openai:backup", + credential: { type: "token", provider: "openai", token: " backup-token " }, + }, + ], + order: { openai: ["openai:primary", "openai:backup"] }, + }); + + expect(loadPersistedAuthProfileStore(agentDir)).toMatchObject({ + profiles: { + "openai:primary": { key: "sk-primary" }, + "openai:backup": { token: "backup-token" }, + }, + order: { openai: ["openai:primary", "openai:backup"] }, + }); + + receipt.rollback(); + receipt.rollback(); + + expect(loadPersistedAuthProfileStore(agentDir)).toBeNull(); + expect(inspectPersistedAuthProfileStoreRaw(agentDir).status).toBe("missing"); + expect(inspectPersistedAuthProfileStateRaw(agentDir).status).toBe("missing"); + }); + }); + + it("removes only owned profiles and introduced order ids", async () => { + await withAgentDir(async (agentDir) => { + saveAuthProfileStore( + { + version: 1, + profiles: { + "openai:existing": apiKey("sk-existing"), + }, + order: { openai: ["openai:existing"] }, + }, + agentDir, + ); + const receipt = await persistAuthProfileBatch({ + agentDir, + profiles: [profile("openai:primary", "sk-attempt"), profile("openai:backup", "sk-backup")], + order: { openai: ["openai:primary", "openai:backup"] }, + }); + await updateAuthProfileStoreWithLock({ + agentDir, + saveOptions: { filterExternalAuthProfiles: false, syncExternalCli: false }, + updater: (store) => { + store.profiles["openai:primary"] = apiKey("sk-newer"); + store.profiles["openai:concurrent"] = apiKey("sk-unrelated"); + store.order = { + openai: ["openai:primary", "openai:backup", "openai:concurrent", "openai:existing"], + }; + return true; + }, + }); + + receipt.rollback(); + + expect(loadPersistedAuthProfileStore(agentDir)).toMatchObject({ + profiles: { + "openai:primary": { key: "sk-newer" }, + "openai:existing": { key: "sk-existing" }, + "openai:concurrent": { key: "sk-unrelated" }, + }, + order: { + openai: ["openai:primary", "openai:concurrent", "openai:existing"], + }, + }); + expect(loadPersistedAuthProfileStore(agentDir)?.profiles["openai:backup"]).toBeUndefined(); + }); + }); + + it("does not claim skipped non-replacing profiles or their order entries", async () => { + await withAgentDir(async (agentDir) => { + saveAuthProfileStore( + { + version: 1, + profiles: { + "openai:existing": apiKey("sk-existing"), + "openai:conflict": apiKey("sk-concurrent"), + }, + order: { openai: ["openai:existing"] }, + }, + agentDir, + ); + const receipt = await persistAuthProfileBatch({ + agentDir, + profiles: [ + { ...profile("openai:conflict", "sk-portable"), replaceExisting: false }, + { ...profile("openai:portable", "sk-portable"), replaceExisting: false }, + ], + order: { openai: ["openai:conflict", "openai:portable"] }, + }); + + expect(loadPersistedAuthProfileStore(agentDir)).toMatchObject({ + profiles: { + "openai:conflict": { key: "sk-concurrent" }, + "openai:portable": { key: "sk-portable" }, + }, + order: { openai: ["openai:existing", "openai:portable"] }, + }); + + receipt.rollback(); + + expect(loadPersistedAuthProfileStore(agentDir)).toMatchObject({ + profiles: { + "openai:existing": { key: "sk-existing" }, + "openai:conflict": { key: "sk-concurrent" }, + }, + order: { openai: ["openai:existing"] }, + }); + expect(loadPersistedAuthProfileStore(agentDir)?.profiles["openai:portable"]).toBeUndefined(); + }); + }); + + it("leaves no partial profile batch when the SQLite state write fails", async () => { + await withAgentDir(async (agentDir) => { + const database = openOpenClawAgentDatabase({ + agentId: "work", + path: resolveAuthProfileDatabasePath(agentDir), + }); + database.db.exec(` + CREATE TRIGGER reject_auth_profile_batch_state + BEFORE INSERT ON auth_profile_state + BEGIN + SELECT RAISE(ABORT, 'injected auth batch state failure'); + END; + `); + + await expect( + persistAuthProfileBatch({ + agentDir, + profiles: [profile("openai:first", "sk-first"), profile("openai:second", "sk-second")], + order: { openai: ["openai:first", "openai:second"] }, + }), + ).rejects.toThrow("injected auth batch state failure"); + + expect(loadPersistedAuthProfileStore(agentDir)).toBeNull(); + }); + }); +}); diff --git a/src/agents/auth-profiles/upsert-with-lock.ts b/src/agents/auth-profiles/upsert-with-lock.ts index a6652212dcf6..dcf74a7538a9 100644 --- a/src/agents/auth-profiles/upsert-with-lock.ts +++ b/src/agents/auth-profiles/upsert-with-lock.ts @@ -1,12 +1,162 @@ -/** - * Locked auth profile upsert helper. - * Normalizes literal secrets before persistence and routes all writes through - * the shared SQLite lock to avoid racing concurrent auth updates. - */ +/** Locked auth profile writes and attempt-scoped compensation. */ +import { isDeepStrictEqual } from "node:util"; +import { AUTH_STORE_VERSION } from "./constants.js"; import { normalizeAuthProfileCredential } from "./credential-normalize.js"; -import { updateAuthProfileStoreWithLock } from "./store.js"; +import { loadPersistedAuthProfileStore } from "./persisted.js"; +import { + deletePersistedAuthProfileStoreRaw, + inspectPersistedAuthProfileStateRaw, + inspectPersistedAuthProfileStoreRaw, + runAuthProfileWriteTransaction, + writePersistedAuthProfileStateRaw, +} from "./sqlite.js"; +import { buildPersistedAuthProfileState } from "./state.js"; +import { saveAuthProfileStore, updateAuthProfileStoreWithLock } from "./store.js"; import type { AuthProfileCredential, AuthProfileStore } from "./types.js"; +type PersistAuthProfileBatchParams = { + profiles: readonly { + profileId: string; + credential: AuthProfileCredential; + replaceExisting?: boolean; + }[]; + order?: Readonly>; + agentDir?: string; + stateDir?: string; +}; + +/** Atomically persists a batch and returns conditional attempt-scoped compensation. */ +export async function persistAuthProfileBatch( + params: PersistAuthProfileBatchParams, +): Promise<{ rollback: () => void }> { + const profiles = new Map( + params.profiles.map(({ profileId, credential, replaceExisting }) => [ + profileId, + { + credential: normalizeAuthProfileCredential(credential), + replaceExisting: replaceExisting !== false, + }, + ]), + ); + if (profiles.size === 0) { + return { rollback() {} }; + } + + const previousProfiles = new Map(); + const previousOrder = new Map(); + const appliedProfiles = new Map(); + let storeWasAbsent = false; + let stateWasAbsent = false; + runAuthProfileWriteTransaction( + params.agentDir, + (database) => { + storeWasAbsent = + inspectPersistedAuthProfileStoreRaw(params.agentDir, database).status === "missing"; + stateWasAbsent = + inspectPersistedAuthProfileStateRaw(params.agentDir, database).status === "missing"; + const next = + loadPersistedAuthProfileStore(params.agentDir, { database }) ?? + ({ version: AUTH_STORE_VERSION, profiles: {} } satisfies AuthProfileStore); + for (const [profileId, entry] of profiles) { + if (!entry.replaceExisting && Object.hasOwn(next.profiles, profileId)) { + continue; + } + previousProfiles.set(profileId, next.profiles[profileId]); + next.profiles[profileId] = entry.credential; + appliedProfiles.set(profileId, entry.credential); + } + for (const [provider, profileIds] of Object.entries(params.order ?? {})) { + previousOrder.set(provider, next.order?.[provider]); + const existing = next.order?.[provider] ?? []; + const additions = [...new Set(profileIds)].filter( + (profileId) => appliedProfiles.has(profileId) && !existing.includes(profileId), + ); + if (additions.length > 0) { + next.order = { ...next.order, [provider]: [...existing, ...additions] }; + } + } + if (appliedProfiles.size > 0) { + saveAuthProfileStore( + next, + params.agentDir, + { filterExternalAuthProfiles: false, syncExternalCli: false }, + database, + ); + } + }, + { stateDir: params.stateDir }, + ); + + let rolledBack = false; + return { + rollback: () => { + if (rolledBack) { + return; + } + runAuthProfileWriteTransaction( + params.agentDir, + (database) => { + const current = loadPersistedAuthProfileStore(params.agentDir, { database }); + if (!current) { + return; + } + const ownedProfiles = new Set(); + for (const [profileId, credential] of appliedProfiles) { + if (!isDeepStrictEqual(current.profiles[profileId], credential)) { + continue; + } + ownedProfiles.add(profileId); + const previous = previousProfiles.get(profileId); + if (previous) { + current.profiles[profileId] = previous; + } else { + delete current.profiles[profileId]; + } + } + for (const [provider, profileIds] of Object.entries(params.order ?? {})) { + const existing = current.order?.[provider]; + if (!existing) { + continue; + } + const preexisting = new Set(previousOrder.get(provider) ?? []); + const introduced = new Set( + profileIds.filter((profileId) => !preexisting.has(profileId)), + ); + const remaining = existing.filter( + (profileId) => !introduced.has(profileId) || !ownedProfiles.has(profileId), + ); + if (remaining.length === existing.length) { + continue; + } + if (remaining.length > 0) { + current.order = { ...current.order, [provider]: remaining }; + } else if (current.order) { + delete current.order[provider]; + if (Object.keys(current.order).length === 0) { + delete current.order; + } + } + } + saveAuthProfileStore( + current, + params.agentDir, + { filterExternalAuthProfiles: false, syncExternalCli: false }, + database, + ); + if (storeWasAbsent && Object.keys(current.profiles).length === 0) { + deletePersistedAuthProfileStoreRaw(params.agentDir, database); + } + if (stateWasAbsent && buildPersistedAuthProfileState(current) === null) { + writePersistedAuthProfileStateRaw(null, params.agentDir, database); + } + }, + { stateDir: params.stateDir }, + ); + rolledBack = true; + }, + }; +} + /** Upserts an auth profile under the store lock, returning null on store write failure. */ export async function upsertAuthProfileWithLock(params: { profileId: string; diff --git a/src/commands/agents.add.test.ts b/src/commands/agents.add.test.ts index be66be32afed..f6cc842ecc9f 100644 --- a/src/commands/agents.add.test.ts +++ b/src/commands/agents.add.test.ts @@ -7,7 +7,7 @@ import { AUTH_STORE_VERSION } from "../agents/auth-profiles/constants.js"; import { loadPersistedAuthProfileStore } from "../agents/auth-profiles/persisted.js"; import { resolveAuthProfileDatabasePath } from "../agents/auth-profiles/sqlite.js"; import { saveAuthProfileStore } from "../agents/auth-profiles/store.js"; -import type { AuthProfileStore } from "../agents/auth-profiles/types.js"; +import type { AuthProfileCredential, AuthProfileStore } from "../agents/auth-profiles/types.js"; import type { ChannelOnboardingPostWriteHook } from "../channels/plugins/setup-wizard-types.js"; import { formatCliCommand } from "../cli/command-format.js"; import { writeConfigMachineState } from "../state/config-machine-state.js"; @@ -20,7 +20,7 @@ import { baseConfigSnapshot, createTestRuntime } from "./test-runtime-config-hel type SetupChannels = typeof import("./onboard-channels.js").setupChannels; type EnsureWorkspaceAndSessions = typeof import("./onboard-helpers.js").ensureWorkspaceAndSessions; -type ApplyAuthChoice = typeof import("./auth-choice.js").applyAuthChoice; +type PrepareAuthChoice = typeof import("./auth-choice.js").prepareAuthChoice; const readConfigFileSnapshotMock = vi.hoisted(() => vi.fn()); const writeConfigFileMock = vi.hoisted(() => vi.fn().mockResolvedValue(undefined)); @@ -79,13 +79,33 @@ const transformConfigWithPendingPluginInstallsMock = vi.hoisted(() => const wizardMocks = vi.hoisted(() => ({ createClackPrompter: vi.fn(), })); +const pluginLifecycleMocks = vi.hoisted(() => { + const state = { active: false }; + return { + state, + withPluginLifecycleLease: vi.fn(async (_options, run: () => Promise) => { + state.active = true; + try { + return await run(); + } finally { + state.active = false; + } + }), + }; +}); const terminalMocks = vi.hoisted(() => ({ isTerminalInteractive: vi.fn(() => true), })); const authChoiceMocks = vi.hoisted(() => ({ - applyAuthChoice: vi.fn(), + prepareAuthChoice: vi.fn(), warnIfModelConfigLooksOff: vi.fn(async () => {}), })); +const authProfileMocks = vi.hoisted(() => ({ + persistBatch: vi.fn(), +})); +const authPromptMocks = vi.hoisted(() => ({ + promptAuthChoiceGrouped: vi.fn(async () => "fixture-auth"), +})); const onboardChannelsMocks = vi.hoisted(() => ({ setupChannels: vi.fn(async (config) => config), })); @@ -122,16 +142,28 @@ vi.mock("../wizard/clack-prompter.js", () => ({ createClackPrompter: wizardMocks.createClackPrompter, })); +vi.mock("../plugins/plugin-lifecycle-lease.js", () => ({ + withPluginLifecycleLease: pluginLifecycleMocks.withPluginLifecycleLease, +})); + vi.mock("../cli/terminal-interactivity.js", async (importOriginal) => ({ ...(await importOriginal()), isTerminalInteractive: terminalMocks.isTerminalInteractive, })); vi.mock("./auth-choice.js", () => ({ - applyAuthChoice: authChoiceMocks.applyAuthChoice, + prepareAuthChoice: authChoiceMocks.prepareAuthChoice, warnIfModelConfigLooksOff: authChoiceMocks.warnIfModelConfigLooksOff, })); +vi.mock("./auth-choice-prompt.js", () => ({ + promptAuthChoiceGrouped: authPromptMocks.promptAuthChoiceGrouped, +})); + +vi.mock("../agents/auth-profiles/upsert-with-lock.js", () => ({ + persistAuthProfileBatch: authProfileMocks.persistBatch, +})); + vi.mock("./onboard-channels.js", () => ({ setupChannels: onboardChannelsMocks.setupChannels, })); @@ -143,6 +175,10 @@ vi.mock("./onboard-helpers.js", () => ({ import { WizardCancelledError } from "../wizard/prompts.js"; import { agentsAddCommand } from "./agents.commands.add.js"; +const { persistAuthProfileBatch } = await vi.importActual< + typeof import("../agents/auth-profiles/upsert-with-lock.js") +>("../agents/auth-profiles/upsert-with-lock.js"); + const runtime = createTestRuntime(); const RESERVED_SYSTEM_AGENT_IDS_FOR_TEST = ["openclaw", "crestodian"] as const; // reserved ids @@ -174,6 +210,7 @@ describe("agents add command", () => { entry?: { id: string; name?: string; workspace?: string; agentDir?: string }; bindingSpecs?: string[]; stagedConfig?: Record; + prepareConfigCommit?: () => Promise<(() => void | Promise) | void>; }) => { const name = params.name ?? params.entry?.name ?? params.entry?.id ?? ""; const agentId = (params.entry?.id ?? name).toLowerCase(); @@ -187,6 +224,7 @@ describe("agents add command", () => { match: { channel: params.bindingSpecs[0].split(":")[0] }, } : undefined; + await params.prepareConfigCommit?.(); return { status: "created" as const, agentId, @@ -210,9 +248,13 @@ describe("agents add command", () => { }, ); wizardMocks.createClackPrompter.mockClear(); + pluginLifecycleMocks.withPluginLifecycleLease.mockClear(); + pluginLifecycleMocks.state.active = false; terminalMocks.isTerminalInteractive.mockReset().mockReturnValue(true); - authChoiceMocks.applyAuthChoice.mockClear(); + authChoiceMocks.prepareAuthChoice.mockReset(); authChoiceMocks.warnIfModelConfigLooksOff.mockClear(); + authPromptMocks.promptAuthChoiceGrouped.mockClear(); + authProfileMocks.persistBatch.mockReset().mockImplementation(persistAuthProfileBatch); onboardChannelsMocks.setupChannels.mockClear(); onboardHelpersMocks.ensureWorkspaceAndSessions.mockClear(); runtime.log.mockClear(); @@ -265,6 +307,24 @@ describe("agents add command", () => { return wizard; } + function stageGuidedAuth( + profiles: Array<{ profileId: string; credential: AuthProfileCredential }> = [ + { + profileId: "openai:primary", + credential: { type: "api_key", provider: "openai", key: "sk-primary" }, + }, + ], + ): void { + authChoiceMocks.prepareAuthChoice.mockImplementationOnce(async ({ config }) => ({ + config: { + ...config, + auth: { profiles: { "openai:primary": { provider: "openai", mode: "api_key" } } }, + }, + authProfiles: profiles, + persistAuthProfiles: async () => {}, + })); + } + function stageChannelPostWriteHook(run: ChannelOnboardingPostWriteHook["run"]): void { onboardChannelsMocks.setupChannels.mockImplementationOnce( async (config, _runtime, _prompter, options) => { @@ -523,7 +583,7 @@ describe("agents add command", () => { expect(checkAgentCreationGateMock).toHaveBeenCalledWith("main"); expect(prompter.outro).toHaveBeenCalledWith("Run openclaw doctor --fix, then retry."); expect(prompter.text).not.toHaveBeenCalled(); - expect(authChoiceMocks.applyAuthChoice).not.toHaveBeenCalled(); + expect(authChoiceMocks.prepareAuthChoice).not.toHaveBeenCalled(); expect(createAgentMock).not.toHaveBeenCalled(); }); @@ -628,7 +688,25 @@ describe("agents add command", () => { }); }); - it("keeps guided auth written after portable copy acceptance when finalizing the copy", async () => { + it("does not persist prepared provider auth when a later prompt is cancelled", async () => { + await withAgentsAddStateRoot("openclaw-agents-add-auth-cancel-provider-", async (root) => { + const agentDir = path.join(root, "agents", "work", "agent"); + const workspaceDir = path.join(root, "workspace-work"); + setConfigSnapshot({ agents: { list: [{ id: "main", default: true }] } }); + useFreshAgentWizard({ workspaceDir, confirmValues: [true] }); + stageGuidedAuth(); + authChoiceMocks.warnIfModelConfigLooksOff.mockRejectedValueOnce(new WizardCancelledError()); + + await agentsAddCommand({}, runtime); + + expect(loadPersistedAuthProfileStore(agentDir)).toBeNull(); + expect(authProfileMocks.persistBatch).not.toHaveBeenCalled(); + expect(createAgentMock).not.toHaveBeenCalled(); + expect(runtime.exit).toHaveBeenCalledWith(1); + }); + }); + + it("keeps guided auth while applying portable profiles without overwriting", async () => { await withAgentsAddStateRoot("openclaw-agents-add-auth-guided-", async (root) => { const destAgentDir = path.join(root, "agents", "work", "agent"); const workspaceDir = path.join(root, "workspace-work"); @@ -655,28 +733,16 @@ describe("agents add command", () => { selectValues: ["openai", "openai-api-key"], }); wizardMocks.createClackPrompter.mockReturnValue(wizard.prompter); - authChoiceMocks.applyAuthChoice.mockImplementationOnce(async ({ config, agentDir }) => { - saveAuthProfileStore( - { - version: AUTH_STORE_VERSION, - profiles: { - "openai:api-key": { - type: "api_key", - provider: "openai", - key: "guided-wins", - }, - "openai:guided": { - type: "api_key", - provider: "openai", - key: "guided-retained", - }, - }, - order: { openai: ["openai:guided", "openai:api-key"] }, - }, - agentDir, - ); - return { config, retrySelection: false }; - }); + stageGuidedAuth([ + { + profileId: "openai:api-key", + credential: { type: "api_key", provider: "openai", key: "guided-wins" }, + }, + { + profileId: "openai:guided", + credential: { type: "api_key", provider: "openai", key: "guided-retained" }, + }, + ]); await agentsAddCommand({}, runtime); @@ -686,7 +752,7 @@ describe("agents add command", () => { "openai:portable": { key: "portable-retained" }, "openai:guided": { key: "guided-retained" }, }); - expect(persisted?.order?.openai).toEqual(["openai:guided", "openai:api-key"]); + expect(persisted?.order?.openai).toEqual(["openai:api-key", "openai:portable"]); expect(wizard.note).toHaveBeenCalledWith( 'Copied 2 portable auth profiles from "main".', "Auth profiles", @@ -694,6 +760,138 @@ describe("agents add command", () => { }); }); + it("persists staged provider auth only at the agent config commit edge", async () => { + await withAgentsAddStateRoot("openclaw-agents-add-auth-create-", async (root) => { + const agentDir = path.join(root, "agents", "work", "agent"); + const workspaceDir = path.join(root, "workspace-work"); + setConfigSnapshot({ agents: { list: [{ id: "main", default: true }] } }); + useFreshAgentWizard({ workspaceDir, confirmValues: [true] }); + stageGuidedAuth(); + createAgentMock.mockImplementationOnce( + async (params: { + stagedConfig?: Record; + prepareConfigCommit?: () => Promise<(() => void | Promise) | void>; + }) => { + expect(pluginLifecycleMocks.state.active).toBe(true); + expect(authProfileMocks.persistBatch).not.toHaveBeenCalled(); + await params.prepareConfigCommit?.(); + return { + status: "created" as const, + agentId: "work", + name: "work", + workspace: workspaceDir, + agentDir, + bootstrapPending: true, + config: params.stagedConfig ?? {}, + }; + }, + ); + + await agentsAddCommand({}, runtime); + + expect(loadPersistedAuthProfileStore(agentDir)?.profiles["openai:primary"]).toMatchObject({ + key: "sk-primary", + }); + expect(authProfileMocks.persistBatch).toHaveBeenCalledOnce(); + expect(authChoiceMocks.warnIfModelConfigLooksOff).toHaveBeenCalledWith( + expect.any(Object), + expect.any(Object), + expect.objectContaining({ + pendingAuthProfiles: [expect.objectContaining({ profileId: "openai:primary" })], + }), + ); + expect(createAgentMock).toHaveBeenCalledWith( + expect.objectContaining({ + stagedConfig: expect.objectContaining({ auth: expect.any(Object) }), + prepareConfigCommit: expect.any(Function), + }), + ); + }); + }); + + it("publishes no agent when staged provider auth cannot persist atomically", async () => { + await withAgentsAddStateRoot("openclaw-agents-add-auth-persist-failure-", async (root) => { + const agentDir = path.join(root, "agents", "work", "agent"); + const workspaceDir = path.join(root, "workspace-work"); + const profiles = ["first", "second"].map((name) => ({ + profileId: `openai:${name}`, + credential: { type: "api_key" as const, provider: "openai", key: `sk-${name}` }, + })); + setConfigSnapshot({ agents: { list: [{ id: "main", default: true }] } }); + useFreshAgentWizard({ workspaceDir, confirmValues: [true] }); + stageGuidedAuth(profiles); + authProfileMocks.persistBatch.mockRejectedValueOnce( + new Error("injected auth batch persistence failure"), + ); + + await expect(agentsAddCommand({}, runtime)).rejects.toThrow( + "injected auth batch persistence failure", + ); + + expect(loadPersistedAuthProfileStore(agentDir)).toBeNull(); + expect(createAgentMock).toHaveBeenCalledOnce(); + expect(commitConfigWithPendingPluginInstallsMock).not.toHaveBeenCalled(); + expect(writeConfigFileMock).not.toHaveBeenCalled(); + }); + }); + + it("retains existing-agent auth after config publication when later output fails", async () => { + await withAgentsAddStateRoot("openclaw-agents-add-auth-existing-", async (root) => { + const agentDir = path.join(root, "agents", "work", "agent"); + const workspaceDir = path.join(root, "workspace-work"); + setConfigSnapshot({ + agents: { entries: { work: { id: "work", workspace: workspaceDir, agentDir } } }, + }); + const wizard = createQueuedWizardPrompter({ + textValues: [workspaceDir], + confirmValues: [true, true], + }); + wizardMocks.createClackPrompter.mockReturnValue(wizard.prompter); + stageGuidedAuth(); + wizard.outro.mockRejectedValueOnce(new Error("injected late failure")); + + await expect(agentsAddCommand({ name: "work" }, runtime)).rejects.toThrow( + "injected late failure", + ); + + expect(loadPersistedAuthProfileStore(agentDir)?.profiles["openai:primary"]).toMatchObject({ + key: "sk-primary", + }); + expect(commitConfigWithPendingPluginInstallsMock).toHaveBeenCalledWith( + expect.objectContaining({ + nextConfig: expect.objectContaining({ auth: expect.any(Object) }), + }), + ); + expect(createAgentMock).not.toHaveBeenCalled(); + }); + }); + + it("rolls existing-agent auth back when config publication fails", async () => { + await withAgentsAddStateRoot("openclaw-agents-add-auth-existing-rollback-", async (root) => { + const agentDir = path.join(root, "agents", "work", "agent"); + const workspaceDir = path.join(root, "workspace-work"); + setConfigSnapshot({ + agents: { entries: { work: { id: "work", workspace: workspaceDir, agentDir } } }, + }); + const wizard = createQueuedWizardPrompter({ + textValues: [workspaceDir], + confirmValues: [true, true], + }); + wizardMocks.createClackPrompter.mockReturnValue(wizard.prompter); + stageGuidedAuth(); + commitConfigWithPendingPluginInstallsMock.mockRejectedValueOnce( + new Error("injected config publication failure"), + ); + + await expect(agentsAddCommand({ name: "work" }, runtime)).rejects.toThrow( + "injected config publication failure", + ); + + expect(loadPersistedAuthProfileStore(agentDir)).toBeNull(); + expect(createAgentMock).not.toHaveBeenCalled(); + }); + }); + it("runs channel post-write hooks only after fresh agent creation", async () => { const hook = vi.fn(async () => {}); setConfigSnapshot({ agents: { list: [{ id: "main", default: true }] } }); diff --git a/src/commands/agents.commands.add.ts b/src/commands/agents.commands.add.ts index e2c1683719cc..61214663cd08 100644 --- a/src/commands/agents.commands.add.ts +++ b/src/commands/agents.commands.add.ts @@ -1,5 +1,4 @@ // Implements `openclaw agents add`, including config mutation, workspace setup, auth copy, and route binding setup. -import fs from "node:fs/promises"; import path from "node:path"; import { normalizeLowercaseStringOrEmpty, @@ -18,22 +17,17 @@ import { import { buildPortableAuthProfileStoreForAgentCopy, ensureAuthProfileStore, + persistAuthProfileBatch, type AuthProfileStore, } from "../agents/auth-profiles.js"; import { AuthProfileStoreUnreadableError } from "../agents/auth-profiles/legacy-source-diagnostic.js"; -import { - loadPersistedAuthProfileStore, - mergeAuthProfileStores, -} from "../agents/auth-profiles/persisted.js"; +import { loadPersistedAuthProfileStore } from "../agents/auth-profiles/persisted.js"; import { resolveSharedMainAuthAgentDir } from "../agents/auth-profiles/shared-main-dir.js"; import { inspectPersistedAuthProfileStoreRaw, resolveAuthProfileDatabasePath, } from "../agents/auth-profiles/sqlite.js"; -import { - loadAuthProfileStoreWithoutExternalProfiles, - saveAuthProfileStore, -} from "../agents/auth-profiles/store.js"; +import { loadAuthProfileStoreWithoutExternalProfiles } from "../agents/auth-profiles/store.js"; import { formatCliCommand } from "../cli/command-format.js"; import { isTerminalInteractive } from "../cli/terminal-interactivity.js"; import { logConfigUpdated } from "../config/logging.js"; @@ -51,7 +45,7 @@ import { WizardCancelledError } from "../wizard/prompts.js"; import { applyAgentBindings, buildChannelBindings, describeBinding } from "./agents.bindings.js"; import { applyAgentConfig, listAgentEntries } from "./agents.config.js"; import { promptAuthChoiceGrouped } from "./auth-choice-prompt.js"; -import { applyAuthChoice, warnIfModelConfigLooksOff } from "./auth-choice.js"; +import { prepareAuthChoice, warnIfModelConfigLooksOff } from "./auth-choice.js"; import { requireValidConfigFileSnapshot } from "./config-validation.js"; import { ensureOnboardingAgentWorkspace, @@ -296,7 +290,11 @@ export async function agentsAddCommand( workspace: workspaceDir, agentDir, }); - let finalizePortableAuthCopy: (() => Promise) | undefined; + const stagedAuthProfiles: Array< + Parameters[0]["profiles"][number] + > = []; + let stagedAuthOrder: AuthProfileStore["order"]; + let reportPortableAuthCopy: (() => Promise) | undefined; const defaultAgentId = resolveDefaultAgentId(cfg); if (defaultAgentId !== agentId) { @@ -333,7 +331,6 @@ export async function agentsAddCommand( initialValue: false, }); if (shouldCopy) { - const portableStore = portable.store; const copiedProfileIds = portable.copiedProfileIds; const copiedOAuthProfileIds = copiedProfileIds.filter( (profileId) => sourceStore.profiles[profileId]?.type === "oauth", @@ -341,16 +338,11 @@ export async function agentsAddCommand( const sourceAgentId = defaultAgentId; const sourceInheritedMain = sourceIsInheritedMain; const destinationAgentDir = agentDir; - finalizePortableAuthCopy = async () => { - await fs.mkdir(destinationAgentDir, { recursive: true }); - const destinationStore = loadPersistedAuthProfileStore(destinationAgentDir); - const storeToPersist = destinationStore - ? mergeAuthProfileStores(portableStore, destinationStore) - : portableStore; - saveAuthProfileStore(storeToPersist, destinationAgentDir, { - filterExternalAuthProfiles: false, - syncExternalCli: false, - }); + for (const [profileId, credential] of Object.entries(portable.store.profiles)) { + stagedAuthProfiles.push({ profileId, credential, replaceExisting: false }); + } + stagedAuthOrder = portable.store.order; + reportPortableAuthCopy = async () => { const persisted = loadPersistedAuthProfileStore(destinationAgentDir); const persistedIds = new Set(Object.keys(persisted?.profiles ?? {})); const copiedCount = copiedProfileIds.filter((profileId) => @@ -371,7 +363,7 @@ export async function agentsAddCommand( } else if (skippedOAuthProfiles) { const sourceAgentId = defaultAgentId; const sourceInheritedMain = sourceIsInheritedMain; - finalizePortableAuthCopy = async () => { + reportPortableAuthCopy = async () => { await prompter.note( formatSkippedOAuthProfilesMessage(sourceAgentId, sourceInheritedMain), "Auth profiles", @@ -388,6 +380,8 @@ export async function agentsAddCommand( if (wantsAuth) { const authStore = ensureAuthProfileStore(agentDir, { allowKeychainPrompt: false, + readOnly: true, + syncExternalCli: false, }); while (true) { const authChoice = await promptAuthChoiceGrouped({ @@ -397,7 +391,7 @@ export async function agentsAddCommand( config: nextConfig, }); - const authResult = await applyAuthChoice({ + const authResult = await prepareAuthChoice({ authChoice, config: nextConfig, prompter, @@ -410,6 +404,7 @@ export async function agentsAddCommand( if (authResult.retrySelection) { continue; } + stagedAuthProfiles.push(...authResult.authProfiles); if (authResult.agentModelOverride) { nextConfig = applyAgentConfig(nextConfig, { agentId, @@ -423,6 +418,10 @@ export async function agentsAddCommand( await warnIfModelConfigLooksOff(nextConfig, prompter, { agentId, agentDir, + pendingAuthProfiles: stagedAuthProfiles.map(({ profileId, credential }) => ({ + profileId, + credential, + })), validateCatalog: false, }); @@ -479,6 +478,20 @@ export async function agentsAddCommand( } } + const stagedEntry = existingAgent + ? undefined + : listAgentEntries(nextConfig).find( + (candidate) => normalizeAgentId(candidate.id) === agentId, + ); + const stagedAuthBatch = + stagedAuthProfiles.length > 0 + ? { + profiles: stagedAuthProfiles, + ...(stagedAuthOrder ? { order: stagedAuthOrder } : {}), + agentDir, + } + : undefined; + let payload: { agentId: string; name: string; workspace: string; agentDir: string }; if (existingAgent) { const target = resolveOnboardingAgentTarget(nextConfig, agentId); @@ -486,13 +499,21 @@ export async function agentsAddCommand( skipBootstrap: Boolean(nextConfig.agents?.defaults?.skipBootstrap), skipOptionalBootstrapFiles: nextConfig.agents?.defaults?.skipOptionalBootstrapFiles, }); - nextConfig = await channelSetup.commit(nextConfig, async (configToCommit) => { - const committed = await commitConfigWithPendingPluginInstalls({ - nextConfig: configToCommit, - ...(baseHash !== undefined ? { baseHash } : {}), + const authPersistence = stagedAuthBatch + ? await persistAuthProfileBatch(stagedAuthBatch) + : undefined; + try { + nextConfig = await channelSetup.commit(nextConfig, async (configToCommit) => { + const committed = await commitConfigWithPendingPluginInstalls({ + nextConfig: configToCommit, + ...(baseHash !== undefined ? { baseHash } : {}), + }); + return committed.config; }); - return committed.config; - }); + } catch (error) { + authPersistence?.rollback(); + throw error; + } payload = { agentId: target.agentId, name: agentName, @@ -500,17 +521,22 @@ export async function agentsAddCommand( agentDir: target.agentDir, }; } else { - const entry = listAgentEntries(nextConfig).find( - (candidate) => normalizeAgentId(candidate.id) === agentId, - ); - if (!entry) { + if (!stagedEntry) { throw new Error(`staged agent "${agentId}" is missing from config`); } - const created = await createAgent({ - entry: { ...entry, id: agentId }, - expectedConfigHash: baseHash ?? null, - stagedConfig: nextConfig, - transformConfig: transformConfigWithPendingPluginInstalls, + const created = await withPluginLifecycleLease({}, async () => { + return await createAgent({ + entry: { ...stagedEntry, id: agentId }, + expectedConfigHash: baseHash ?? null, + stagedConfig: nextConfig, + transformConfig: transformConfigWithPendingPluginInstalls, + ...(stagedAuthBatch + ? { + prepareConfigCommit: async () => + (await persistAuthProfileBatch(stagedAuthBatch)).rollback, + } + : {}), + }); }); if (created.status === "error") { await prompter.outro(created.message); @@ -525,7 +551,7 @@ export async function agentsAddCommand( }; await channelSetup.runPostWriteHooks(nextConfig); } - await finalizePortableAuthCopy?.(); + await reportPortableAuthCopy?.(); if (!opts.json) { logConfigUpdated(runtime); } diff --git a/src/commands/auth-choice.apply.plugin-provider.test.ts b/src/commands/auth-choice.apply.plugin-provider.test.ts index 09ef4f47d5b4..d2b5056b6cec 100644 --- a/src/commands/auth-choice.apply.plugin-provider.test.ts +++ b/src/commands/auth-choice.apply.plugin-provider.test.ts @@ -1,6 +1,7 @@ // Auth-choice plugin provider tests cover loaded provider setup, plugin install, and credential routing. import { expectDefined } from "@openclaw/normalization-core"; import { beforeEach, describe, expect, it, vi } from "vitest"; +import type { AuthProfileCredential } from "../agents/auth-profiles/types.js"; import { applyAuthChoiceLoadedPluginProvider, prepareAuthChoiceLoadedPluginProvider, @@ -42,11 +43,9 @@ vi.mock("../plugins/provider-auth-choices.js", () => ({ resolveManifestProviderAuthChoice, })); -const upsertAuthProfile = vi.hoisted(() => vi.fn(() => ({ version: 1, profiles: {} }))); +const persistAuthProfileBatch = vi.hoisted(() => vi.fn(async () => {})); vi.mock("../agents/auth-profiles.js", () => ({ - upsertAuthProfile, - upsertAuthProfileWithLock: upsertAuthProfile, - upsertAuthProfileWithLockOrThrow: upsertAuthProfile, + persistAuthProfileBatch, })); const resolveDefaultAgentId = vi.hoisted(() => vi.fn(() => "default")); @@ -110,6 +109,15 @@ const LOCAL_API_KEY = "local-provider-key"; const LOCAL_DEFAULT_MODEL = `${LOCAL_PROVIDER_ID}/demo-model`; const EXISTING_DEFAULT_MODEL = "amazon-bedrock/anthropic.claude-3-5-sonnet-20241022-v2:0"; +function expectPersistedProfile(profileId: string, credential: AuthProfileCredential): void { + expect(persistAuthProfileBatch).toHaveBeenCalledWith( + expect.objectContaining({ + profiles: [{ profileId, credential }], + agentDir: "/tmp/agent", + }), + ); +} + function buildProvider(): ProviderPlugin { return { id: LOCAL_PROVIDER_ID, @@ -254,7 +262,7 @@ describe("applyAuthChoiceLoadedPluginProvider", () => { }, }, ]); - expect(upsertAuthProfile).not.toHaveBeenCalled(); + expect(persistAuthProfileBatch).not.toHaveBeenCalled(); await prepared?.persistAuthProfiles([ { @@ -268,15 +276,11 @@ describe("applyAuthChoiceLoadedPluginProvider", () => { ]); await prepared?.persistAuthProfiles(); - expect(upsertAuthProfile).toHaveBeenCalledOnce(); - expect(upsertAuthProfile).toHaveBeenCalledWith({ - profileId: LOCAL_PROFILE_ID, - credential: { - type: "api_key", - provider: LOCAL_PROVIDER_ID, - key: "test-key", - }, - agentDir: "/tmp/agent", + expect(persistAuthProfileBatch).toHaveBeenCalledOnce(); + expectPersistedProfile(LOCAL_PROFILE_ID, { + type: "api_key", + provider: LOCAL_PROVIDER_ID, + key: "test-key", }); }); @@ -376,14 +380,10 @@ describe("applyAuthChoiceLoadedPluginProvider", () => { expect(result?.config.models?.providers?.["remote-alpha"]?.models?.[0]?.input).toContain( "image", ); - expect(upsertAuthProfile).toHaveBeenCalledWith({ - profileId: "remote-alpha:default", - credential: { - type: "api_key", - provider: "remote-alpha", - key: "sk-remote-alpha-test", - }, - agentDir: "/tmp/agent", + expectPersistedProfile("remote-alpha:default", { + type: "api_key", + provider: "remote-alpha", + key: "sk-remote-alpha-test", }); expect(runProviderModelSelectedHook).not.toHaveBeenCalled(); }); @@ -401,14 +401,10 @@ describe("applyAuthChoiceLoadedPluginProvider", () => { expect(result?.config.agents?.defaults?.model).toEqual({ primary: LOCAL_DEFAULT_MODEL, }); - expect(upsertAuthProfile).toHaveBeenCalledWith({ - profileId: LOCAL_PROFILE_ID, - credential: { - type: "api_key", - provider: LOCAL_PROVIDER_ID, - key: LOCAL_API_KEY, - }, - agentDir: "/tmp/agent", + expectPersistedProfile(LOCAL_PROFILE_ID, { + type: "api_key", + provider: LOCAL_PROVIDER_ID, + key: LOCAL_API_KEY, }); expect(runProviderModelSelectedHook).toHaveBeenCalledOnce(); const [hookParams] = runProviderModelSelectedHook.mock @@ -621,6 +617,7 @@ describe("applyAuthChoiceLoadedPluginProvider", () => { }, }, }, + env: { OPENCLAW_STATE_DIR: "/tmp/openclaw-state" }, runtime: {} as ApplyAuthChoiceParams["runtime"], prompter: { note, @@ -645,6 +642,9 @@ describe("applyAuthChoiceLoadedPluginProvider", () => { "Detected local provider runtime.\nPulled model metadata.", "Provider notes", ); + expect(persistAuthProfileBatch).toHaveBeenCalledWith( + expect.objectContaining({ stateDir: "/tmp/openclaw-state" }), + ); expect(events).toEqual(["note", "lock"]); }); diff --git a/src/commands/auth-choice.test.ts b/src/commands/auth-choice.test.ts index 56bbba7337aa..e01b63d63b73 100644 --- a/src/commands/auth-choice.test.ts +++ b/src/commands/auth-choice.test.ts @@ -185,6 +185,23 @@ function seedTestAuthProfile(params: { } vi.mock("../agents/auth-profiles.js", () => ({ + persistAuthProfileBatch: async (params: { + profiles: readonly { + profileId: string; + credential: StoredAuthProfile; + replaceExisting?: boolean; + }[]; + agentDir?: string; + }) => { + for (const profile of params.profiles) { + const existing = readTestAuthProfileStore(params.agentDir).profiles[profile.profileId]; + if (profile.replaceExisting === false && existing) { + continue; + } + seedTestAuthProfile({ ...profile, agentDir: params.agentDir }); + } + return { rollback() {} }; + }, upsertAuthProfile: (params: { profileId: string; credential: StoredAuthProfile; diff --git a/src/plugins/provider-api-key-auth.ts b/src/plugins/provider-api-key-auth.ts index dcf9742b7aec..9333e0259ce9 100644 --- a/src/plugins/provider-api-key-auth.ts +++ b/src/plugins/provider-api-key-auth.ts @@ -165,6 +165,7 @@ export function createProviderApiKeyAuthMethod( : ctx.secretInputMode, config: ctx.config, env: ctx.env, + workspaceDir: ctx.workspaceDir, expectedProviders: params.expectedProviders ?? [params.providerId], provider: params.providerId, envLabel: params.envVar, diff --git a/src/plugins/provider-auth-choice.ts b/src/plugins/provider-auth-choice.ts index 9cdd5385684b..451a258e692c 100644 --- a/src/plugins/provider-auth-choice.ts +++ b/src/plugins/provider-auth-choice.ts @@ -5,7 +5,7 @@ import { resolveAgentDir, resolveAgentWorkspaceDir, } from "../agents/agent-scope.js"; -import { upsertAuthProfileWithLockOrThrow } from "../agents/auth-profiles.js"; +import { persistAuthProfileBatch } from "../agents/auth-profiles.js"; import { formatLiteralProviderPrefixedModelRef } from "../agents/model-ref-shared.js"; import { resolveDefaultAgentWorkspaceDir } from "../agents/workspace.js"; import { normalizeAgentModelRefForConfig } from "../config/model-input.js"; @@ -330,52 +330,12 @@ export async function runProviderPluginAuthMethod(params: { allowSecretRefPrompt?: boolean; opts?: Partial; }): Promise<{ config: OpenClawConfig; defaultModel?: string }> { - const agentId = params.agentId ?? resolveDefaultAgentId(params.config); - const agentDir = params.agentDir ?? resolveAgentDir(params.config, agentId); - const workspaceDir = - params.workspaceDir ?? - resolveAgentWorkspaceDir(params.config, agentId) ?? - resolveDefaultAgentWorkspaceDir(); - const result = await runProviderPluginAuthMethodUnpersisted({ - config: params.config, - env: params.env, - runtime: params.runtime, - prompter: params.prompter, - method: params.method, - agentDir, - workspaceDir, - ...(params.signal ? { signal: params.signal } : {}), - ...(params.isRemote !== undefined ? { isRemote: params.isRemote } : {}), - secretInputMode: params.secretInputMode, - allowSecretRefPrompt: params.allowSecretRefPrompt, - opts: params.opts, - }); - - if (params.emitNotes !== false && result.notes && result.notes.length > 0) { - await params.prompter.note(result.notes.join("\n"), "Provider notes"); - } - - await params.beforePersistentEffect?.(); - for (const profile of result.profiles) { - await upsertAuthProfileWithLockOrThrow({ - profileId: profile.profileId, - credential: profile.credential, - agentDir, - }); - } - - const nextConfig = applyProviderPluginAuthMethodResultConfig({ - config: params.config, - result, - }); - - const defaultModel = result.defaultModel - ? normalizeAgentModelRefForConfig(result.defaultModel) - : undefined; + const prepared = await prepareProviderPluginAuthMethod(params); + await prepared.persistAuthProfiles(); return { - config: nextConfig, - ...(defaultModel ? { defaultModel } : {}), + config: prepared.config, + ...(prepared.defaultModel ? { defaultModel: prepared.defaultModel } : {}), }; } @@ -426,15 +386,11 @@ async function prepareProviderPluginAuthMethod( return; } await params.beforePersistentEffect?.(); - for (const profile of profiles) { - const { profileId, credential } = profile; - await upsertAuthProfileWithLockOrThrow({ - profileId, - credential, - agentDir, - stateDir: params.env?.OPENCLAW_STATE_DIR, - }); - } + await persistAuthProfileBatch({ + profiles, + agentDir, + stateDir: params.env?.OPENCLAW_STATE_DIR, + }); profilesPersisted = true; }; diff --git a/src/plugins/provider-auth-input.test.ts b/src/plugins/provider-auth-input.test.ts index 4baaa86c9f7b..b63169296821 100644 --- a/src/plugins/provider-auth-input.test.ts +++ b/src/plugins/provider-auth-input.test.ts @@ -100,6 +100,7 @@ function currentMinimaxTestEnv(): NodeJS.ProcessEnv { async function ensureMinimaxApiKey(params: { config?: Parameters[0]["config"]; env?: Parameters[0]["env"]; + workspaceDir?: string; confirm: WizardPrompter["confirm"]; note?: WizardPrompter["note"]; select?: WizardPrompter["select"]; @@ -110,6 +111,7 @@ async function ensureMinimaxApiKey(params: { return await ensureMinimaxApiKeyInternal({ config: params.config, env: params.env ?? currentMinimaxTestEnv(), + workspaceDir: params.workspaceDir, prompter: createPrompter({ confirm: params.confirm, note: params.note, @@ -124,6 +126,7 @@ async function ensureMinimaxApiKey(params: { async function ensureMinimaxApiKeyInternal(params: { config?: Parameters[0]["config"]; env?: Parameters[0]["env"]; + workspaceDir?: string; prompter: WizardPrompter; secretInputMode?: Parameters[0]["secretInputMode"]; setCredential: Parameters[0]["setCredential"]; @@ -131,6 +134,7 @@ async function ensureMinimaxApiKeyInternal(params: { return await ensureApiKeyFromEnvOrPrompt({ config: params.config ?? {}, env: params.env, + workspaceDir: params.workspaceDir, provider: "minimax", envLabel: "MINIMAX_API_KEY", promptMessage: "Enter key", @@ -250,10 +254,10 @@ describe("validateApiKeyInput", () => { }); describe("ensureApiKeyFromEnvOrPrompt", () => { - it("resolves environment auth using the same config and workspace as provider runtime", async () => { + it("uses the prepared workspace when staged config has no default agent", async () => { const workspaceDir = "/tmp/openclaw-provider-workspace"; const config: OpenClawConfig = { - agents: { defaults: { workspace: workspaceDir } }, + agents: { entries: { main: {}, work: { workspace: workspaceDir } } }, plugins: { entries: { minimax: { enabled: true } } }, }; const env = { MINIMAX_API_KEY: "workspace-env-key" } as NodeJS.ProcessEnv; @@ -262,6 +266,7 @@ describe("ensureApiKeyFromEnvOrPrompt", () => { const result = await ensureMinimaxApiKey({ config, env, + workspaceDir, confirm, text, setCredential, diff --git a/src/plugins/provider-auth-input.ts b/src/plugins/provider-auth-input.ts index 2c5edcacc707..58feca9210a9 100644 --- a/src/plugins/provider-auth-input.ts +++ b/src/plugins/provider-auth-input.ts @@ -137,6 +137,7 @@ export async function ensureApiKeyFromOptionEnvOrPrompt(params: { secretInputMode?: SecretInputMode; config: OpenClawConfig; env?: NodeJS.ProcessEnv; + workspaceDir?: string; expectedProviders: string[]; provider: string; envLabel: string; @@ -168,6 +169,7 @@ export async function ensureApiKeyFromOptionEnvOrPrompt(params: { return await ensureApiKeyFromEnvOrPrompt({ config: params.config, env: params.env, + workspaceDir: params.workspaceDir, provider: params.provider, envLabel: params.envLabel, promptMessage: params.promptMessage, @@ -183,6 +185,7 @@ export async function ensureApiKeyFromOptionEnvOrPrompt(params: { export async function ensureApiKeyFromEnvOrPrompt(params: { config: OpenClawConfig; env?: NodeJS.ProcessEnv; + workspaceDir?: string; provider: string; envLabel: string; promptMessage: string; @@ -201,11 +204,9 @@ export async function ensureApiKeyFromEnvOrPrompt(params: { // runtime; dropping the staged config silently changes credential ownership. const envKey = resolveEnvApiKey(params.provider, env, { config: params.config, - workspaceDir: resolveAgentWorkspaceDir( - params.config, - resolveDefaultAgentId(params.config), - env, - ), + workspaceDir: + params.workspaceDir ?? + resolveAgentWorkspaceDir(params.config, resolveDefaultAgentId(params.config), env), }); if (selectedMode === "ref") { From f4faa0a1b00073d41f95ad405c4f96987d7632d9 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Tue, 18 Aug 2026 18:26:14 -0700 Subject: [PATCH 007/356] fix(status): show unavailable telemetry diagnostics (#126092) --- src/commands/status-all/diagnosis.test.ts | 62 ++++++++++++++--- src/commands/status-all/diagnosis.ts | 76 +++++++++++++++------ src/commands/status-all/report-data.test.ts | 4 +- src/commands/status-all/report-data.ts | 5 +- src/commands/status-runtime-shared.test.ts | 29 ++++++-- src/commands/status-runtime-shared.ts | 14 ++-- 6 files changed, 145 insertions(+), 45 deletions(-) diff --git a/src/commands/status-all/diagnosis.test.ts b/src/commands/status-all/diagnosis.test.ts index ca9700d430a6..c3465d5ded1d 100644 --- a/src/commands/status-all/diagnosis.test.ts +++ b/src/commands/status-all/diagnosis.test.ts @@ -51,6 +51,10 @@ function createProgressReporter(): ProgressReporter { }; } +function availableDiagnostics(value: unknown) { + return { ok: true as const, value }; +} + function createBaseParams( listeners: NonNullable["listeners"], ): DiagnosisParams { @@ -271,7 +275,7 @@ describe("status-all diagnosis port checks", () => { it("summarizes inbound delivery telemetry proof counters", async () => { const params = createBaseParams([]); params.gatewayReachable = true; - params.deliveryDiagnostics = { + params.deliveryDiagnostics = availableDiagnostics({ summary: { byType: { "message.received": 2, @@ -282,7 +286,7 @@ describe("status-all diagnosis port checks", () => { }, }, events: [{ type: "session.turn.created", ts: Date.now() - 60_000 }], - }; + }); await appendStatusAllDiagnosis(params); @@ -295,7 +299,7 @@ describe("status-all diagnosis port checks", () => { it("renders the shared redacted telemetry exporter summary", async () => { const params = createBaseParams([]); - params.exporterDiagnostics = { + params.exporterDiagnostics = availableDiagnostics({ events: [ { seq: 1, @@ -311,7 +315,7 @@ describe("status-all diagnosis port checks", () => { error: "raw failure", }, ], - }; + }); await appendStatusAllDiagnosis(params); @@ -325,10 +329,40 @@ describe("status-all diagnosis port checks", () => { expect(output).not.toContain("raw failure"); }); + it("renders failed diagnostics as unavailable instead of empty", async () => { + const params = createBaseParams([]); + params.gatewayReachable = true; + const failedProbe = { + ok: false as const, + error: + "Error: diagnostics probe timed out at wss://probe-user:probe-pass@gateway.example/socket?token=probe-secret", + }; + params.deliveryDiagnostics = failedProbe; + params.exporterDiagnostics = failedProbe; + + await appendStatusAllDiagnosis(params); + + const output = params.lines.join("\n"); + expect(output).toContain("! Telemetry exporters: unavailable"); + expect(output).toContain( + "Exporter diagnostics failed: Error: diagnostics probe timed out at wss://***:***@gateway.example/socket?token=***", + ); + expect(output).toContain("Retry: openclaw gateway stability --type telemetry.exporter"); + expect(output).toContain("! Inbound delivery telemetry: unavailable"); + expect(output).toContain( + "Delivery diagnostics failed: Error: diagnostics probe timed out at wss://***:***@gateway.example/socket?token=***", + ); + expect(output).toContain("Retry: openclaw gateway stability"); + expect(output).not.toContain("received 0 · dispatch 0/0 · turns 0 · processed 0"); + expect(output).not.toContain("probe-user"); + expect(output).not.toContain("probe-pass"); + expect(output).not.toContain("probe-secret"); + }); + it("keeps handled terminal delivery paths healthy without dispatch starts", async () => { const params = createBaseParams([]); params.gatewayReachable = true; - params.deliveryDiagnostics = { + params.deliveryDiagnostics = availableDiagnostics({ summary: { byType: { "message.received": 1, @@ -339,7 +373,7 @@ describe("status-all diagnosis port checks", () => { }, }, events: [{ type: "message.processed", ts: Date.now() - 30_000 }], - }; + }); await appendStatusAllDiagnosis(params); @@ -353,7 +387,7 @@ describe("status-all diagnosis port checks", () => { it("keeps handled terminal dispatches healthy without agent turns", async () => { const params = createBaseParams([]); params.gatewayReachable = true; - params.deliveryDiagnostics = { + params.deliveryDiagnostics = availableDiagnostics({ summary: { byType: { "message.received": 1, @@ -364,7 +398,7 @@ describe("status-all diagnosis port checks", () => { }, }, events: [{ type: "message.processed", ts: Date.now() - 30_000 }], - }; + }); await appendStatusAllDiagnosis(params); @@ -378,7 +412,7 @@ describe("status-all diagnosis port checks", () => { it("warns when received messages never reach agent turn creation", async () => { const params = createBaseParams([]); params.gatewayReachable = true; - params.deliveryDiagnostics = { + params.deliveryDiagnostics = availableDiagnostics({ summary: { byType: { "message.received": 3, @@ -389,7 +423,7 @@ describe("status-all diagnosis port checks", () => { }, }, events: [{ type: "message.dispatch.started", ts: Date.now() - 120_000 }], - }; + }); await appendStatusAllDiagnosis(params); @@ -421,6 +455,12 @@ describe("status-all diagnosis port checks", () => { ].join("\n"), }; params.gatewayReachable = true; + const failedProbe = { + ok: false as const, + error: "Error: diagnostics probe unavailable in node-only mode", + }; + params.deliveryDiagnostics = failedProbe; + params.exporterDiagnostics = failedProbe; await appendStatusAllDiagnosis(params); @@ -432,6 +472,8 @@ describe("status-all diagnosis port checks", () => { expect(output).not.toContain("Channel issues skipped (gateway unreachable)"); expect(output).not.toContain("Gateway health:"); expect(output).not.toContain("Inbound delivery telemetry: unavailable"); + expect(output).not.toContain("Telemetry exporters: unavailable"); + expect(output).not.toContain("Retry: openclaw gateway stability"); }); it("does not read or display stale stderr tails on Darwin", async () => { diff --git a/src/commands/status-all/diagnosis.ts b/src/commands/status-all/diagnosis.ts index 1167a390990d..0d9afff4fb03 100644 --- a/src/commands/status-all/diagnosis.ts +++ b/src/commands/status-all/diagnosis.ts @@ -1,7 +1,9 @@ // Appends the read-only diagnosis section for `openclaw status --all`. // Every line that can include logs, config, or connection details is redacted before display. +import { redactSensitiveUrlLikeString } from "@openclaw/net-policy/redact-sensitive-url"; import { normalizeOptionalString } from "@openclaw/normalization-core/string-coerce"; +import { sanitizeTerminalText } from "../../../packages/terminal-core/src/safe-text.js"; import type { ProgressReporter } from "../../cli/progress.js"; import { formatConfigIssueLine } from "../../config/issue-format.js"; import { @@ -24,6 +26,7 @@ import { formatPluginCompatibilityNotice, type PluginCompatibilityNotice, } from "../../plugins/status.js"; +import type { StatusGatewayDiagnosticsResult } from "../status-runtime-shared.ts"; import { formatUpdateRestartActionLines, formatUpdateRestartStatusValue, @@ -157,8 +160,8 @@ export async function appendStatusAllDiagnosis(params: { pluginCompatibility: PluginCompatibilityNotice[]; channelsStatus: unknown; channelIssues: ChannelIssueLike[]; - deliveryDiagnostics: unknown; - exporterDiagnostics: unknown; + deliveryDiagnostics: StatusGatewayDiagnosticsResult | null; + exporterDiagnostics: StatusGatewayDiagnosticsResult | null; agentStatus?: AgentStatusLike; gatewayReachable: boolean; health: unknown; @@ -171,6 +174,17 @@ export async function appendStatusAllDiagnosis(params: { const colored = status === "ok" ? ok(label) : status === "warn" ? warn(label) : fail(label); lines.push(`${icon} ${colored}`); }; + const emitUnavailableDiagnostics = (diagnostic: { + label: string; + detail: string; + retry: string; + }) => { + emitCheck(`${diagnostic.label}: unavailable`, "warn"); + lines.push( + ` ${muted(sanitizeTerminalText(redactStatusSecrets(redactSensitiveUrlLikeString(diagnostic.detail))))}`, + ); + lines.push(` ${muted(`Retry: ${diagnostic.retry}`)}`); + }; lines.push(""); lines.push(muted("Gateway connection details:")); @@ -339,33 +353,41 @@ export async function appendStatusAllDiagnosis(params: { } } - const exporterSummary = formatTelemetryExporterSummary(params.exporterDiagnostics); - if (exporterSummary) { - emitCheck(exporterSummary.title, exporterSummary.status); - for (const line of exporterSummary.lines) { - lines.push(` ${muted(line)}`); + if (!params.nodeOnlyGateway && params.exporterDiagnostics) { + if (params.exporterDiagnostics.ok) { + const exporterSummary = formatTelemetryExporterSummary(params.exporterDiagnostics.value); + if (exporterSummary) { + emitCheck(exporterSummary.title, exporterSummary.status); + for (const line of exporterSummary.lines) { + lines.push(` ${muted(line)}`); + } + } + } else { + emitUnavailableDiagnostics({ + label: "Telemetry exporters", + detail: `Exporter diagnostics failed: ${params.exporterDiagnostics.error}`, + retry: "openclaw gateway stability --type telemetry.exporter", + }); } } - if (params.deliveryDiagnostics != null) { - if (isDeliveryDiagnosticsLike(params.deliveryDiagnostics)) { - const received = countDeliveryEvent(params.deliveryDiagnostics, "message.received"); - const dispatchStarted = countDeliveryEvent( - params.deliveryDiagnostics, - "message.dispatch.started", - ); + if (!params.nodeOnlyGateway && params.deliveryDiagnostics?.ok) { + if (isDeliveryDiagnosticsLike(params.deliveryDiagnostics.value)) { + const deliveryDiagnostics = params.deliveryDiagnostics.value; + const received = countDeliveryEvent(deliveryDiagnostics, "message.received"); + const dispatchStarted = countDeliveryEvent(deliveryDiagnostics, "message.dispatch.started"); const dispatchCompleted = countDeliveryEvent( - params.deliveryDiagnostics, + deliveryDiagnostics, "message.dispatch.completed", ); - const turnsCreated = countDeliveryEvent(params.deliveryDiagnostics, "session.turn.created"); - const processed = countDeliveryEvent(params.deliveryDiagnostics, "message.processed"); + const turnsCreated = countDeliveryEvent(deliveryDiagnostics, "session.turn.created"); + const processed = countDeliveryEvent(deliveryDiagnostics, "message.processed"); const hasReceivedWithoutDispatch = received > 0 && dispatchStarted === 0 && processed === 0; const hasDispatchWithoutTurn = dispatchStarted > 0 && turnsCreated === 0 && processed < dispatchStarted; const dispatchGap = dispatchStarted - dispatchCompleted; const hasDispatchGap = dispatchGap >= 2; - const latestAgeMs = latestDeliveryEventAgeMs(params.deliveryDiagnostics); + const latestAgeMs = latestDeliveryEventAgeMs(deliveryDiagnostics); emitCheck( `Inbound delivery telemetry: received ${received} · dispatch ${dispatchStarted}/${dispatchCompleted} · turns ${turnsCreated} · processed ${processed}`, hasReceivedWithoutDispatch || hasDispatchWithoutTurn || hasDispatchGap ? "warn" : "ok", @@ -389,10 +411,22 @@ export async function appendStatusAllDiagnosis(params: { ); } } else { - emitCheck("Inbound delivery telemetry: unavailable", "warn"); + emitUnavailableDiagnostics({ + label: "Inbound delivery telemetry", + detail: "Delivery diagnostics returned an invalid response.", + retry: "openclaw gateway stability", + }); } - } else if (params.gatewayReachable && !params.nodeOnlyGateway) { - emitCheck("Inbound delivery telemetry: unavailable", "warn"); + } else if ( + !params.nodeOnlyGateway && + params.deliveryDiagnostics && + !params.deliveryDiagnostics.ok + ) { + emitUnavailableDiagnostics({ + label: "Inbound delivery telemetry", + detail: `Delivery diagnostics failed: ${params.deliveryDiagnostics.error}`, + retry: "openclaw gateway stability", + }); } params.progress.setLabel("Reading logs…"); diff --git a/src/commands/status-all/report-data.test.ts b/src/commands/status-all/report-data.test.ts index a8093336adf0..356d9e270cdf 100644 --- a/src/commands/status-all/report-data.test.ts +++ b/src/commands/status-all/report-data.test.ts @@ -5,7 +5,7 @@ const mocks = vi.hoisted(() => ({ readConfigFileSnapshot: vi.fn(async () => ({ path: "/tmp/openclaw.json" })), inspectPortUsage: vi.fn(async () => null), resolveGatewayBindHost: vi.fn(async () => "127.0.0.1"), - resolveStatusGatewayDiagnosticsSafe: vi.fn(async () => null), + resolveStatusGatewayDiagnosticsSafe: vi.fn(async () => ({ ok: true, value: {} })), resolveStatusGatewayHealthSafe: vi.fn(async () => undefined), resolveNodeExecEligibility: vi.fn(() => ({ canExec: false })), loadExecApprovalsReadOnly: vi.fn(() => ({ version: 1, agents: {} })), @@ -63,7 +63,7 @@ import { buildStatusAllReportData } from "./report-data.js"; describe("buildStatusAllReportData", () => { beforeEach(() => { vi.clearAllMocks(); - mocks.resolveStatusGatewayDiagnosticsSafe.mockResolvedValue(null); + mocks.resolveStatusGatewayDiagnosticsSafe.mockResolvedValue({ ok: true, value: {} }); mocks.resolveStatusGatewayHealthSafe.mockResolvedValue(undefined); }); diff --git a/src/commands/status-all/report-data.ts b/src/commands/status-all/report-data.ts index 618269b9538b..374c06e8b197 100644 --- a/src/commands/status-all/report-data.ts +++ b/src/commands/status-all/report-data.ts @@ -20,6 +20,7 @@ import { import { resolveStatusGatewayDiagnosticsSafe, resolveStatusGatewayHealthSafe, + type StatusGatewayDiagnosticsResult, type resolveStatusServiceSummaries, } from "../status-runtime-shared.ts"; import { formatUpdateRestartStatusValue } from "../status-update-restart.ts"; @@ -81,8 +82,8 @@ async function resolveStatusAllLocalDiagnosis(params: { agentStatus: StatusScanOverviewResult["agentStatus"]; gatewayReachable: boolean; health: StatusGatewayHealthSafe | undefined; - deliveryDiagnostics: unknown; - exporterDiagnostics: unknown; + deliveryDiagnostics: StatusGatewayDiagnosticsResult | null; + exporterDiagnostics: StatusGatewayDiagnosticsResult | null; nodeOnlyGateway: NodeOnlyGatewayInfo | null; }; }> { diff --git a/src/commands/status-runtime-shared.test.ts b/src/commands/status-runtime-shared.test.ts index dcc5784d8780..332a26db759b 100644 --- a/src/commands/status-runtime-shared.test.ts +++ b/src/commands/status-runtime-shared.test.ts @@ -448,12 +448,14 @@ describe("status-runtime-shared", () => { }); it("requests the typed exporter stability projection", async () => { - await resolveStatusGatewayDiagnosticsSafe({ - config: { gateway: {} }, - timeoutMs: 4321, - gatewayReachable: true, - type: "telemetry.exporter", - }); + await expect( + resolveStatusGatewayDiagnosticsSafe({ + config: { gateway: {} }, + timeoutMs: 4321, + gatewayReachable: true, + type: "telemetry.exporter", + }), + ).resolves.toEqual({ ok: true, value: { ok: true } }); expect(mocks.callGateway).toHaveBeenCalledWith({ method: "diagnostics.stability", @@ -463,6 +465,21 @@ describe("status-runtime-shared", () => { }); }); + it("preserves failed gateway diagnostics as a typed result", async () => { + mocks.callGateway.mockRejectedValueOnce(new Error("diagnostics probe timed out")); + + await expect( + resolveStatusGatewayDiagnosticsSafe({ + config: { gateway: {} }, + timeoutMs: 4321, + gatewayReachable: true, + }), + ).resolves.toEqual({ + ok: false, + error: "Error: diagnostics probe timed out", + }); + }); + it("resolves daemon summaries together", async () => { await expect(resolveStatusServiceSummaries()).resolves.toEqual([ { label: "LaunchAgent" }, diff --git a/src/commands/status-runtime-shared.ts b/src/commands/status-runtime-shared.ts index 32a6f5757a85..37a24a6ce585 100644 --- a/src/commands/status-runtime-shared.ts +++ b/src/commands/status-runtime-shared.ts @@ -1,6 +1,7 @@ // Shared runtime probes used by status text and JSON commands. // Heavy modules stay lazily loaded so fast status output avoids security/provider/gateway costs. +import type { Result } from "@openclaw/normalization-core/result"; import { listAgentIds, resolveSystemAgentTargetAgentId } from "../agents/agent-scope-config.js"; import { resolveAgentDir } from "../agents/agent-scope.js"; import { resolveAgentHarnessPolicy } from "../agents/harness/policy.js"; @@ -205,7 +206,9 @@ export async function resolveStatusGatewayHealthSafe(params: { }).catch((err: unknown) => ({ error: String(err) })); } -/** Reads gateway delivery diagnostics when reachable, returning null on failures. */ +export type StatusGatewayDiagnosticsResult = Result; + +/** Reads gateway diagnostics while preserving whether data or an unavailable outcome was observed. */ export async function resolveStatusGatewayDiagnosticsSafe(params: { config: OpenClawConfig; timeoutMs?: number; @@ -216,9 +219,9 @@ export async function resolveStatusGatewayDiagnosticsSafe(params: { token?: string; password?: string; }; -}) { +}): Promise { if (!params.gatewayReachable) { - return null; + return { ok: false, error: "gateway unreachable" }; } const { callGateway } = await loadGatewayCallModule(); return await callGateway({ @@ -227,7 +230,10 @@ export async function resolveStatusGatewayDiagnosticsSafe(params: { timeoutMs: params.timeoutMs, config: params.config, ...params.callOverrides, - }).catch(() => null); + }).then( + (value) => ({ ok: true, value }), + (error: unknown) => ({ ok: false, error: String(error) }), + ); } /** Reads the most recent gateway heartbeat only when the gateway probe succeeded. */ From fea2198319403415aba20798637a7dc6ffb725fe Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Tue, 18 Aug 2026 18:29:49 -0700 Subject: [PATCH 008/356] fix(ui): keep New Session folder picker usable while menus close (#126032) * fix(ui): track overlapping place popover hides * test(ui): stabilize activity grouping across timezones --- .../activity-session-feed.capture.e2e.test.ts | 9 ++++++++- .../new-session/draft-place-browser.test.ts | 17 +++++++++++++++++ ui/src/pages/new-session/draft-place-browser.ts | 15 +++++++-------- 3 files changed, 32 insertions(+), 9 deletions(-) diff --git a/ui/src/e2e/activity-session-feed.capture.e2e.test.ts b/ui/src/e2e/activity-session-feed.capture.e2e.test.ts index f4b13bf46568..4a0864997fcf 100644 --- a/ui/src/e2e/activity-session-feed.capture.e2e.test.ts +++ b/ui/src/e2e/activity-session-feed.capture.e2e.test.ts @@ -25,7 +25,14 @@ suite.define(() => { viewport: { height: 900, width: 1280 }, }, async ({ page }) => { - const now = Date.now(); + const current = new Date(); + // Keep the automation fixtures on one local calendar day in every timezone. + const now = new Date( + current.getFullYear(), + current.getMonth(), + current.getDate() - 1, + 12, + ).getTime(); const releaseKey = "agent:main:release-readiness"; const designKey = "agent:main:design-review"; await installMockGateway(page, { diff --git a/ui/src/pages/new-session/draft-place-browser.test.ts b/ui/src/pages/new-session/draft-place-browser.test.ts index 5c2bc5705c83..8b292f49b739 100644 --- a/ui/src/pages/new-session/draft-place-browser.test.ts +++ b/ui/src/pages/new-session/draft-place-browser.test.ts @@ -90,6 +90,23 @@ function createBrowser(request: (method: string) => Promise, data?: New } describe("DraftPlaceBrowser", () => { + it("tracks overlapping popover hides independently", () => { + const { browser } = createBrowser(async () => ({})); + + browser.onPopoverHide("project"); + browser.onPopoverHide("where"); + + expect(browser.popoverHiding("project")).toBe(true); + expect(browser.popoverHiding("where")).toBe(true); + + browser.onPopoverAfterHide("project"); + expect(browser.popoverHiding("project")).toBe(false); + expect(browser.popoverHiding("where")).toBe(true); + + browser.onPopoverAfterHide("where"); + expect(browser.popoverHiding("where")).toBe(false); + }); + it.each([ ["the Gateway omits recents", async () => ({ projects: [] })], [ diff --git a/ui/src/pages/new-session/draft-place-browser.ts b/ui/src/pages/new-session/draft-place-browser.ts index c8cd40476c94..cca058ed7281 100644 --- a/ui/src/pages/new-session/draft-place-browser.ts +++ b/ui/src/pages/new-session/draft-place-browser.ts @@ -59,7 +59,8 @@ export class DraftPlaceBrowser { private browserProjectPathValue: string | null = null; private browserRegisteringValue = false; private openPopoverValue: DraftPickerKind | null = null; - private hidingPopoverValue: DraftPickerKind | null = null; + // Independent hide animations can overlap; keep every trigger fenced until its own completes. + private readonly hidingPopovers = new Set(); // Live head input; absolute paths stay applicable even without fs.listDir. private browserPathDraftValue = ""; private browserRequestToken = 0; @@ -217,7 +218,7 @@ export class DraftPlaceBrowser { } popoverHiding(kind: DraftPickerKind): boolean { - return this.hidingPopoverValue === kind; + return this.hidingPopovers.has(kind); } popoverCallbacks(kind: DraftPickerKind) { @@ -524,7 +525,7 @@ export class DraftPlaceBrowser { if (this.openPopoverValue === kind) { this.openPopoverValue = null; } - this.hidingPopoverValue = kind; + this.hidingPopovers.add(kind); if (kind === "project") { this.showRoot(); } else { @@ -533,15 +534,13 @@ export class DraftPlaceBrowser { } onPopoverAfterHide(kind: DraftPickerKind) { - if (this.hidingPopoverValue === kind) { - this.hidingPopoverValue = null; - } + this.hidingPopovers.delete(kind); this.restorePopoverTrigger(`new-session-${kind}-trigger`, `.new-session-page__${kind}-popover`); this.callbacks.requestUpdate(); } guardPopoverTransition(event: Event, kind: DraftPickerKind) { - if (this.hidingPopoverValue !== kind) { + if (!this.hidingPopovers.has(kind)) { return; } event.preventDefault(); @@ -549,7 +548,7 @@ export class DraftPlaceBrowser { } clearPopoverHiding() { - this.hidingPopoverValue = null; + this.hidingPopovers.clear(); this.callbacks.requestUpdate(); } From a480d0347fa8c0bc5c8a8aa35c4b4d364484410b Mon Sep 17 00:00:00 2001 From: ClawSweeper Date: Tue, 18 Aug 2026 18:41:35 -0700 Subject: [PATCH 009/356] feat(sessions): expose sidebar category controls (#126074) * feat(sessions): expose sidebar category controls * fix(sessions): make category controls explicit * test(sessions): update list description fixture --------- Co-authored-by: fuller-stack-dev <263060202+fuller-stack-dev@users.noreply.github.com> --- docs/concepts/session-tool.md | 10 +-- docs/tools/subagents.md | 5 +- src/agents/tool-description-presets.test.ts | 2 +- src/agents/tool-description-presets.ts | 4 +- src/agents/tool-schema-hints.ts | 2 +- src/agents/tools/sessions-helpers.ts | 2 + src/agents/tools/sessions-list-tool.test.ts | 18 +++-- src/agents/tools/sessions-list-tool.ts | 3 + src/agents/tools/sessions-spawn-tool.test.ts | 41 +++++++++++ src/agents/tools/sessions-spawn-visible.ts | 11 +++ .../tools/sessions-tool.sidebar.test.ts | 73 +++++++++++++++++++ src/agents/tools/sessions-tool.test.ts | 50 ++----------- src/agents/tools/sessions-tool.ts | 17 ++++- .../codex-dynamic-tools.discord-group.json | 6 +- .../codex-dynamic-tools.telegram-direct.json | 8 +- .../discord-group-codex-message-tool.md | 8 +- .../telegram-direct-codex-message-tool.md | 8 +- .../telegram-heartbeat-codex-tool.md | 8 +- 18 files changed, 199 insertions(+), 77 deletions(-) create mode 100644 src/agents/tools/sessions-tool.sidebar.test.ts diff --git a/docs/concepts/session-tool.md b/docs/concepts/session-tool.md index 72c640abd6c4..4d15b93898d5 100644 --- a/docs/concepts/session-tool.md +++ b/docs/concepts/session-tool.md @@ -32,7 +32,7 @@ Group, provider, sandbox, and per-agent policies can still remove those tools af ## Listing and reading sessions -`sessions_list` returns focused discovery rows: session key, durable session ID, agent, kind, channel, label/title/preview fields, parent and child relationships, last update, archive/pin state, state version, model, context/total token counts, run status, and whether the last run aborted. Filter by `kinds` (array; accepted values: `main`, `group`, `cron`, `hook`, `node`, `other`), exact `label`, exact `agentId`, `search` text, or recency (`activeMinutes`). Active sessions are returned by default; pass `archived: true` to inspect archived sessions instead. Set `includeDerivedTitles`, `includeLastMessage`, or `messageLimit` (capped at 20) when you need mailbox-style triage: a visibility-scoped derived title, a last-message preview snippet, or bounded recent messages on each row. Use the returned `sessionId` as `expectedSessionId` when the `sessions` tool archives, restores, or deletes another session; this prevents a stale key from targeting a replacement. Delivery routing, other internal IDs, per-run timings/settings, cost estimates, and transcript paths remain omitted; use `session_status`, conversation tools, and `sessions_history` for those owner-specific details. Derived titles and previews are produced only for sessions the caller can already see under the configured session tool visibility policy, so unrelated sessions stay hidden. When visibility is restricted, `sessions_list` returns optional `visibility` metadata showing the effective mode and a warning that results may be scope-limited. +`sessions_list` returns focused discovery rows: session key, durable session ID, agent, kind, channel, label/title/preview fields, sidebar category, parent and child relationships, last update, archive/pin state, state version, model, context/total token counts, run status, and whether the last run aborted. Filter by `kinds` (array; accepted values: `main`, `group`, `cron`, `hook`, `node`, `other`), exact `label`, exact `agentId`, `search` text, or recency (`activeMinutes`). Active sessions are returned by default; pass `archived: true` to inspect archived sessions instead. Set `includeDerivedTitles`, `includeLastMessage`, or `messageLimit` (capped at 20) when you need mailbox-style triage: a visibility-scoped derived title, a last-message preview snippet, or bounded recent messages on each row. Use the returned `sessionId` as `expectedSessionId` when the `sessions` tool archives, restores, or deletes another session; this prevents a stale key from targeting a replacement. Delivery routing, other internal IDs, per-run timings/settings, cost estimates, and transcript paths remain omitted; use `session_status`, conversation tools, and `sessions_history` for those owner-specific details. Derived titles and previews are produced only for sessions the caller can already see under the configured session tool visibility policy, so unrelated sessions stay hidden. When visibility is restricted, `sessions_list` returns optional `visibility` metadata showing the effective mode and a warning that results may be scope-limited. `sessions_history` fetches the conversation transcript for a specific session. By default, tool results are excluded; pass `includeTools: true` to see them. Use `limit` for the newest bounded tail. Pass `offset: 0` when you need pagination metadata, then pass returned `nextOffset` values to page backward through older OpenClaw transcript windows without reading raw transcript files. Explicit offset pages do not merge external CLI fallback imports; use the default newest-tail view (no `offset`) when you need that merged display history. @@ -60,13 +60,13 @@ Use [`sessions_search`](/concepts/session-search) for exact full-text recall acr The owner-gated `sessions` tool exposes bounded self-service surfaces: -- `action: "patch"` changes the current session by default, or another visible session selected by `sessionKey`. It can set the label, persistent sidebar `icon`, pin/archive state, model, and thinking level. The icon must be one emoji grapheme or one of the named icons `braces`, `book`, `monitor`, `bot`, `kanban`, and `coins`; pass an empty string to clear it. The Control UI picker also accepts a custom emoji and shows the macOS (Control-Command-Space) or Windows (Windows-period) system emoji picker shortcut. Archiving or restoring another session requires its `sessions_list` `sessionId` as `expectedSessionId`. +- `action: "patch"` changes the current session by default, or another visible session selected by `sessionKey`. It can set the label, persistent sidebar `icon`, sidebar `category`, pin/archive state, model, and thinking level. Pass `null` or an empty string to clear `category`; assigning a category adds it to the catalog on first use. The icon must be one emoji grapheme or one of the named icons `braces`, `book`, `monitor`, `bot`, `kanban`, and `coins`; pass an empty string to clear it. The Control UI picker also accepts a custom emoji and shows the macOS (Control-Command-Space) or Windows (Windows-period) system emoji picker shortcut. Archiving or restoring another session requires its `sessions_list` `sessionId` as `expectedSessionId`. - `action: "reset"` resets another visible session selected by `sessionKey`. - `action: "delete"` first archives and then deletes the exact same generation of another visible session selected by `sessionKey`. By default its transcript is retained as a deleted archive; pass `deleteTranscript: false` to leave the transcript state untouched. Resetting or deleting the session currently running the tool is rejected. - `action: "assign_owner"` hands session responsibility to a person or agent. Pass `ownerType` (`"human"` or `"agent"`) and `ownerId`; the target is the current session by default, or another visible session via `sessionKey`. Agent owner ids must name a configured agent. The assignment records who reassigned it and when, and the Control UI reflects the new owner immediately. Ownership is display and responsibility, not access control; see [Multi-user mode](/concepts/multi-user). -- `group_list`, `group_set`, `group_rename`, and `group_delete` manage the global ordered session-group catalog. `group_set` replaces the ordered name list rather than patching one entry. +- `group_list`, `group_set`, `group_rename`, and `group_delete` manage the global ordered session-group catalog. `group_set` replaces the ordered name list rather than assigning a session; use `action: "patch"` with `category` for membership. -Use `sessions_spawn` with `visible: true` to create a persistent dashboard session. This keeps session creation on the controlled spawn path, which enforces the parent's tool policy, sandbox, concurrency limits, and run timeout. +Use `sessions_spawn` with `visible: true` to create a persistent dashboard session. Pass `category` to place it in a sidebar group atomically; omit `category` or pass an empty string to leave it ungrouped. This keeps session creation on the controlled spawn path, which enforces the parent's tool policy, sandbox, concurrency limits, and run timeout. An agent-selected model patch stays reversible until that selection completes a successful run. If the selected model is definitively unusable because of authentication, billing, or model-not-found failure, OpenClaw restores the previous model and writes a visible system note. Transient rate-limit, overload, timeout, network, and server failures do not undo the selection. @@ -131,7 +131,7 @@ Key options: - `thread: true` to bind the spawn to a chat thread (Discord, Slack, etc.). - `sandbox: "require"` to enforce sandboxing on the child. - `context: "fork"` for native sub-agents when the child needs the current requester transcript; omit it or use `context: "isolated"` for a clean child. `context: "fork"` is only valid with `runtime: "subagent"`. Thread-bound native sub-agents default to `context: "fork"` unless `threadBindings.defaultSpawnContext` says otherwise. -- `visible: true` to create a persistent dashboard session instead of a hidden sub-agent session. Visible spawns support an explicit model, working directory, same-agent transcript fork, and an optional [managed worktree](/concepts/managed-worktrees); see [Sub-agents](/tools/subagents#tool-parameters) for the exact compatibility limits. The accepted result is a receipt: it includes the child session key, run id, a Control UI `sessionUrl` (omitted when the Control UI is disabled), and an `owner` record naming the requesting agent. When acknowledging the spawn in a channel, put the session URL on the first line and `Owner: