diff --git a/apps/server/src/serverSettings.test.ts b/apps/server/src/serverSettings.test.ts index 3e809ef6cd4cc..fbea8cfa8b605 100644 --- a/apps/server/src/serverSettings.test.ts +++ b/apps/server/src/serverSettings.test.ts @@ -1,14 +1,17 @@ import * as NodeCrypto from "@effect/platform-node/NodeCrypto"; import * as NodeServices from "@effect/platform-node/NodeServices"; import { + clearRemovedFromProviderList, DEFAULT_SERVER_SETTINGS, globalIdleCompaction, + isRemovedDefaultProviderInstance, type ManagedPolicyDocument, ModelSelection, ProjectId, ProjectScript, ProviderDriverKind, ProviderInstanceId, + removedDefaultProviderInstance, resolveProviderInstanceEnabled, SecretProviderId, ServerSettings, @@ -1178,6 +1181,30 @@ it.layer(Layer.mergeAll(NodeServices.layer, NodeCrypto.layer))("server settings" }).pipe(Effect.provide(layerServerSettings())), ); + it.effect("keeps a removed default provider marked until it is edited", () => + Effect.gen(function* () { + const serverSettings = yield* ServerSettingsModule.ServerSettingsService; + const codexId = ProviderInstanceId.make("codex"); + const removed = removedDefaultProviderInstance(ProviderDriverKind.make("codex")); + + yield* serverSettings.updateProviderInstance({ + operation: "upsert", + instanceId: codexId, + instance: removed, + }); + const afterRemove = (yield* serverSettings.getSettings).providerInstances[codexId]!; + assert.isTrue(isRemovedDefaultProviderInstance(afterRemove)); + + yield* serverSettings.updateProviderInstance({ + operation: "upsert", + instanceId: codexId, + instance: clearRemovedFromProviderList({ ...afterRemove, enabled: true }), + }); + const reenabled = (yield* serverSettings.getSettings).providerInstances[codexId]!; + assert.isFalse(isRemovedDefaultProviderInstance({ ...reenabled, enabled: false })); + }).pipe(Effect.provide(layerServerSettings())), + ); + it.effect("reads settings secrets once per settings change", () => { let reads = 0; const layerSecret = Layer.effect( diff --git a/apps/web/src/components/settings/ProviderSettingsPanel.environment.browser.test.tsx b/apps/web/src/components/settings/ProviderSettingsPanel.environment.browser.test.tsx index ce0383e2105d5..abdb23d23c77e 100644 --- a/apps/web/src/components/settings/ProviderSettingsPanel.environment.browser.test.tsx +++ b/apps/web/src/components/settings/ProviderSettingsPanel.environment.browser.test.tsx @@ -4,6 +4,7 @@ import { EnvironmentId, ProviderDriverKind, ProviderInstanceId, + removedDefaultProviderInstance, type ProviderAuthState, type ServerProvider, type UnifiedSettings, @@ -360,7 +361,7 @@ describe("EnvironmentProviderSettings routing", () => { expect(resetMutation).toEqual({ operation: "upsert", instanceId: codexId, - instance: { driver: "codex", enabled: false, config: { enabled: false } }, + instance: removedDefaultProviderInstance(ProviderDriverKind.make("codex")), }); // Deleting touches only the instance; shared preferences stay untouched. expect(resetPatch ?? {}).toEqual({}); @@ -379,7 +380,7 @@ describe("EnvironmentProviderSettings routing", () => { expect(mutation).toEqual({ operation: "upsert", instanceId: codexId, - instance: { driver: "codex", enabled: false, config: { enabled: false } }, + instance: removedDefaultProviderInstance(ProviderDriverKind.make("codex")), }); state.settings = { @@ -547,7 +548,7 @@ describe("EnvironmentProviderSettings instance removal", () => { expect(state.mutateProviderInstance).toHaveBeenCalledWith({ operation: "upsert", instanceId: codexId, - instance: { driver: "codex", enabled: false, config: { enabled: false } }, + instance: removedDefaultProviderInstance(ProviderDriverKind.make("codex")), }), ); }); diff --git a/apps/web/src/components/settings/ProviderSettingsPanel.tsx b/apps/web/src/components/settings/ProviderSettingsPanel.tsx index c1723f8327696..129791395afb4 100644 --- a/apps/web/src/components/settings/ProviderSettingsPanel.tsx +++ b/apps/web/src/components/settings/ProviderSettingsPanel.tsx @@ -15,8 +15,10 @@ import { squashAtomCommandFailure, } from "@t3tools/client-runtime/state/runtime"; import { + clearRemovedFromProviderList, defaultInstanceIdForDriver, type EnvironmentId, + isRemovedDefaultProviderInstance, type AcpRegistryUrlAuthAction, PROVIDER_DISPLAY_NAMES, ProviderDriverKind, @@ -26,6 +28,7 @@ import { type ServerProvider, resolveEnvironmentMachineKind, resolveProjectProviderInstanceEnabled, + removedDefaultProviderInstance, resolveProviderInstanceEnabled, } from "@t3tools/contracts"; import { DEFAULT_UNIFIED_SETTINGS, type UnifiedSettings } from "@t3tools/contracts/settings"; @@ -168,25 +171,6 @@ const PROVIDER_SETTINGS = providerClients.definitions.map((definition) => ({ hasDefaultInstance: definition.hasDefaultInstance !== false, })); -/** - * The entry `deleteDefaultInstance` leaves for a built-in that runs unconfigured - * (Codex, Claude): disabled on the envelope and in its config, with nothing else - * set. Turning a provider off only sets the envelope flag, so it stays listed. - */ -const DELETED_DEFAULT_INSTANCE_CONFIG = { enabled: false } as const; - -function isDeletedDefaultInstance(instance: ProviderInstanceConfig): boolean { - const { driver: _driver, enabled, config, ...rest } = instance; - return ( - enabled === false && - Object.keys(rest).length === 0 && - typeof config === "object" && - config !== null && - Object.keys(config).length === 1 && - (config as { enabled?: unknown }).enabled === false - ); -} - function configuredBinaryPath(config: unknown): string { if (config === null || typeof config !== "object" || !("binaryPath" in config)) return ""; return typeof config.binaryPath === "string" ? config.binaryPath.trim() : ""; @@ -773,10 +757,10 @@ export function EnvironmentProviderSettings({ const hasDefaultSlot = providerSettings.hasDefaultInstance || explicitInstance !== undefined; // A slot is listed when the user configured it or it is live. An untouched, // disabled slot is a provider type the user never set up, and a deleted - // built-in (see `deleteDefaultInstance`) is one they removed; both stay out + // built-in (see `resetDefaultInstance`) is one they removed; both stay out // of the list until the + dialog adds them back. A deep-linked slot always shows. const isConfigured = - explicitInstance !== undefined && !isDeletedDefaultInstance(explicitInstance); + explicitInstance !== undefined && !isRemovedDefaultProviderInstance(explicitInstance); if ( hasDefaultSlot && (isConfigured || @@ -834,7 +818,7 @@ export function EnvironmentProviderSettings({ textGenerationModelSelection, }); const result = await persistProviderInstance( - { operation: "upsert", instanceId, instance: next }, + { operation: "upsert", instanceId, instance: clearRemovedFromProviderList(next) }, patch, ); if (result._tag === "Failure" && !isAtomCommandInterrupted(result)) { @@ -915,7 +899,7 @@ export function EnvironmentProviderSettings({ // Deleting a default slot drops it out of the provider list; it can be re-added // from the + dialog like any provider. A built-in that runs unconfigured (Codex, // Claude) would come straight back as enabled once its entry is gone, so it keeps - // the disabled marker entry `isDeletedDefaultInstance` recognizes instead. + // the disabled marker entry `removedDefaultProviderInstance` writes instead. const resetDefaultInstance = useStableCallback(async (driverKind: ProviderDriverKind) => { const instanceId = defaultInstanceIdForDriver(driverKind); const instance = settings.providerInstances?.[instanceId] ?? { driver: driverKind }; @@ -929,11 +913,7 @@ export function EnvironmentProviderSettings({ ? { operation: "upsert", instanceId, - instance: { - driver: driverKind, - enabled: false, - config: DELETED_DEFAULT_INSTANCE_CONFIG, - }, + instance: removedDefaultProviderInstance(driverKind), } : { operation: "remove", instanceId }, ); diff --git a/packages/contracts/src/providerInstance.ts b/packages/contracts/src/providerInstance.ts index 61fee930c3193..8271de9122346 100644 --- a/packages/contracts/src/providerInstance.ts +++ b/packages/contracts/src/providerInstance.ts @@ -82,6 +82,40 @@ export const ProviderInstanceConfig = Schema.Struct({ }); export type ProviderInstanceConfig = typeof ProviderInstanceConfig.Type; +// Not `config.enabled`: settings normalization folds that into the envelope and drops it. +const REMOVED_FROM_PROVIDER_LIST_KEY = "removedFromProviderList"; + +/** A removed built-in that runs unconfigured (Codex, Claude): kept as a disabled entry clients leave out of the provider list. */ +export const removedDefaultProviderInstance = ( + driver: ProviderDriverKind, +): ProviderInstanceConfig => ({ + driver, + enabled: false, + config: { [REMOVED_FROM_PROVIDER_LIST_KEY]: true }, +}); + +const removedMarkerConfig = (config: unknown): Record | undefined => + config !== null && + typeof config === "object" && + !Array.isArray(config) && + (config as Record)[REMOVED_FROM_PROVIDER_LIST_KEY] === true + ? (config as Record) + : undefined; + +export const isRemovedDefaultProviderInstance = (instance: ProviderInstanceConfig): boolean => + instance.enabled === false && removedMarkerConfig(instance.config) !== undefined; + +/** Any edit to a removed entry, such as turning it back on, makes it an ordinary listed instance. */ +export const clearRemovedFromProviderList = ( + instance: ProviderInstanceConfig, +): ProviderInstanceConfig => { + const config = removedMarkerConfig(instance.config); + if (config === undefined) return instance; + const { [REMOVED_FROM_PROVIDER_LIST_KEY]: _removed, ...rest } = config; + const { config: _config, ...envelope } = instance; + return Object.keys(rest).length > 0 ? { ...envelope, config: rest } : envelope; +}; + export const ProviderInstanceMutation = Schema.Union([ Schema.Struct({ operation: Schema.Literal("create"),