diff --git a/src/server/core/domain/entities/provider-config.ts b/src/server/core/domain/entities/provider-config.ts index a8335cc76..138e6cbd3 100644 --- a/src/server/core/domain/entities/provider-config.ts +++ b/src/server/core/domain/entities/provider-config.ts @@ -5,6 +5,25 @@ * Non-sensitive data (API keys stored separately in keychain). */ +/** + * Durable record of why/when a provider was disconnected. + * + * Persisted on the provider entry (tombstone) when the provider was disconnected + * with a recorded reason — e.g. a permanent OAuth refresh failure — so the user + * can later see what happened and how to reconnect, instead of silently finding + * "no provider connected" with no explanation. + */ +export interface ProviderDisconnectInfo { + /** ISO timestamp of when the disconnect happened */ + readonly at: string + /** OAuth error code when available (e.g. invalid_grant, invalid_client) */ + readonly errorCode?: string + /** Human-readable reason (e.g. "OAuth token refresh failed") */ + readonly reason: string + /** HTTP status code when available (e.g. 400, 401) */ + readonly statusCode?: number +} + /** * Configuration for a single connected provider. */ @@ -21,6 +40,12 @@ export interface ConnectedProviderConfig { readonly connectedAt: string /** User's favorite models (for quick access) */ readonly favoriteModels: readonly string[] + /** + * Set when the provider was disconnected with a recorded reason. Its presence + * marks this entry as a disconnected tombstone — `isProviderConnected` returns + * false while it is set, and reconnecting clears it. + */ + readonly lastDisconnect?: ProviderDisconnectInfo /** OAuth account ID (e.g. ChatGPT-Account-Id for OpenAI) */ readonly oauthAccountId?: string /** Recently used models (last 10) */ @@ -131,9 +156,13 @@ export class ProviderConfig { /** * Check if a provider is connected. + * + * A disconnected tombstone (an entry retained only to record `lastDisconnect`) + * is NOT connected — it exists purely to surface why the provider dropped. */ public isProviderConnected(providerId: string): boolean { - return providerId in this.providers + const entry = this.providers[providerId] + return entry !== undefined && entry.lastDisconnect === undefined } /** @@ -249,11 +278,41 @@ export class ProviderConfig { /** * Create a new config with a provider disconnected. + * + * Without `details`, the provider entry is removed entirely (a clean manual + * disconnect). With `details`, the entry is retained as a tombstone carrying a + * `lastDisconnect` record so the reason survives to the providers view — used + * when the disconnect was involuntary (e.g. a permanent OAuth refresh failure). + * The active provider is cleared either way when it was the disconnected one. */ - public withProviderDisconnected(providerId: string): ProviderConfig { + public withProviderDisconnected( + providerId: string, + details?: {errorCode?: string; reason: string; statusCode?: number}, + ): ProviderConfig { + const newActiveProvider = this.activeProvider === providerId ? '' : this.activeProvider + const existingConfig = this.providers[providerId] + + // Record a tombstone (keep the entry) when a reason is supplied and the + // provider actually existed — so the disconnect is visible after the fact. + if (details && existingConfig) { + const lastDisconnect: ProviderDisconnectInfo = { + at: new Date().toISOString(), + errorCode: details.errorCode, + reason: details.reason, + statusCode: details.statusCode, + } + + return new ProviderConfig({ + activeProvider: newActiveProvider, + providers: { + ...this.providers, + [providerId]: {...existingConfig, lastDisconnect}, + }, + }) + } + // eslint-disable-next-line @typescript-eslint/no-unused-vars const {[providerId]: _removed, ...remainingProviders} = this.providers - const newActiveProvider = this.activeProvider === providerId ? '' : this.activeProvider return new ProviderConfig({ activeProvider: newActiveProvider, diff --git a/src/server/core/interfaces/i-provider-config-store.ts b/src/server/core/interfaces/i-provider-config-store.ts index 1be633da5..871378520 100644 --- a/src/server/core/interfaces/i-provider-config-store.ts +++ b/src/server/core/interfaces/i-provider-config-store.ts @@ -29,8 +29,15 @@ export interface IProviderConfigStore { * Removes a provider connection. * * @param providerId The provider ID to disconnect + * @param details Optional disconnect reason. When provided, the provider entry + * is retained as a tombstone carrying `lastDisconnect` (so the reason is + * visible in the providers view) instead of being removed entirely. Omit for + * a clean manual disconnect that fully removes the entry. */ - disconnectProvider: (providerId: string) => Promise + disconnectProvider: ( + providerId: string, + details?: {errorCode?: string; reason: string; statusCode?: number}, + ) => Promise /** * Gets the active model for a provider. diff --git a/src/server/infra/provider-oauth/token-refresh-manager.ts b/src/server/infra/provider-oauth/token-refresh-manager.ts index 5cf844167..bbed132d0 100644 --- a/src/server/infra/provider-oauth/token-refresh-manager.ts +++ b/src/server/infra/provider-oauth/token-refresh-manager.ts @@ -8,7 +8,7 @@ import type {ProviderTokenResponse, RefreshTokenExchangeParams, TokenRequestCont import {getProviderById} from '../../core/domain/entities/provider-registry.js' import {TransportDaemonEventNames} from '../../core/domain/transport/schemas.js' import {processLog} from '../../utils/process-logger.js' -import {isPermanentOAuthError} from './errors.js' +import {isPermanentOAuthError, ProviderTokenExchangeError} from './errors.js' import {exchangeRefreshToken as defaultExchangeRefreshToken} from './refresh-token-exchange.js' import {computeExpiresAt} from './types.js' @@ -111,9 +111,24 @@ export class TokenRefreshManager implements ITokenRefreshManager { this.deps.transport.broadcast(TransportDaemonEventNames.PROVIDER_UPDATED, {}) return true } catch (error) { - // 7. Permanent failure (token revoked, client invalid): disconnect provider, clean up + // 7. Permanent failure (token revoked, client invalid): disconnect provider, clean up. + // Record the reason durably (lastDisconnect tombstone) and log symmetrically + // with the transient branch so the disconnect leaves a visible trace instead + // of silently dropping the provider. if (isPermanentOAuthError(error)) { - await this.deps.providerConfigStore.disconnectProvider(providerId).catch(() => {}) + const statusCode = error instanceof ProviderTokenExchangeError ? error.statusCode : undefined + const errorCode = error instanceof ProviderTokenExchangeError ? error.errorCode : undefined + const detail = [statusCode ? `status ${statusCode}` : undefined, errorCode].filter(Boolean).join(', ') + + processLog( + `[TokenRefreshManager] Permanent refresh failure for ${providerId}${ + detail ? ` (${detail})` : '' + }: disconnecting provider`, + ) + + await this.deps.providerConfigStore + .disconnectProvider(providerId, {errorCode, reason: 'OAuth token refresh failed', statusCode}) + .catch(() => {}) await this.deps.providerOAuthTokenStore.delete(providerId).catch(() => {}) await this.deps.providerKeychainStore.deleteApiKey(providerId).catch(() => {}) this.deps.transport.broadcast(TransportDaemonEventNames.PROVIDER_UPDATED, {}) diff --git a/src/server/infra/provider/provider-config-resolver.ts b/src/server/infra/provider/provider-config-resolver.ts index 6790b00b9..159cda3fe 100644 --- a/src/server/infra/provider/provider-config-resolver.ts +++ b/src/server/infra/provider/provider-config-resolver.ts @@ -60,7 +60,12 @@ export async function clearStaleProviderConfig( })), ) - const staleProviderIds = results.filter(({accessible}) => !accessible).map(({providerId}) => providerId) + // Skip entries already tombstoned with a lastDisconnect reason: their + // credentials are intentionally gone, and re-disconnecting would drop the + // tombstone (deleting the entry) and erase the recorded disconnect reason. + const staleProviderIds = results + .filter(({accessible, providerId}) => !accessible && config.providers[providerId]?.lastDisconnect === undefined) + .map(({providerId}) => providerId) if (staleProviderIds.length === 0) return diff --git a/src/server/infra/storage/file-provider-config-store.ts b/src/server/infra/storage/file-provider-config-store.ts index b2819b772..3cf3ecf6a 100644 --- a/src/server/infra/storage/file-provider-config-store.ts +++ b/src/server/infra/storage/file-provider-config-store.ts @@ -74,11 +74,15 @@ export class FileProviderConfigStore implements IProviderConfigStore { } /** - * Removes a provider connection. + * Removes a provider connection. When `details` is supplied, the entry is + * retained as a `lastDisconnect` tombstone (see IProviderConfigStore). */ - public async disconnectProvider(providerId: string): Promise { + public async disconnectProvider( + providerId: string, + details?: {errorCode?: string; reason: string; statusCode?: number}, + ): Promise { const config = await this.read() - const newConfig = config.withProviderDisconnected(providerId) + const newConfig = config.withProviderDisconnected(providerId, details) await this.write(newConfig) } diff --git a/src/server/infra/transport/handlers/model-handler.ts b/src/server/infra/transport/handlers/model-handler.ts index d46983d79..edd7f23bb 100644 --- a/src/server/infra/transport/handlers/model-handler.ts +++ b/src/server/infra/transport/handlers/model-handler.ts @@ -147,7 +147,10 @@ export class ModelHandler { const config = await this.providerConfigStore.read() const providerConfig = config.providers[data.providerId] - if (!providerConfig) { + // A tombstoned entry (disconnected with a recorded reason, e.g. a permanent + // OAuth refresh failure) still exists in config.providers, so presence alone + // isn't enough — isProviderConnected() also rejects tombstones. + if (!providerConfig || !config.isProviderConnected(data.providerId)) { return { error: `Provider "${data.providerId}" is not connected`, success: false, diff --git a/src/server/infra/transport/handlers/provider-handler.ts b/src/server/infra/transport/handlers/provider-handler.ts index 92996e93d..71759b698 100644 --- a/src/server/infra/transport/handlers/provider-handler.ts +++ b/src/server/infra/transport/handlers/provider-handler.ts @@ -388,6 +388,7 @@ export class ProviderHandler { return false }), isCurrent: def.id === activeProviderId, + lastDisconnect: providerConfig?.lastDisconnect, name: def.name, oauthCallbackMode: def.oauth?.callbackMode, oauthLabel: def.oauth?.modes[0]?.label, @@ -409,6 +410,20 @@ export class ProviderHandler { return {error: BYTEROVER_AUTH_REQUIRED_MESSAGE, success: false} } + // byterover's gate is isByteRoverAuthSatisfied() above — it is authorized + // independently of config.providers (see resolveProviderConfig's early + // return for it), so it is exempt from the isProviderConnected check below. + if (data.providerId !== 'byterover') { + const config = await this.providerConfigStore.read() + // A tombstoned entry (disconnected with a recorded reason, e.g. a + // permanent OAuth refresh failure) still exists in config.providers — + // isProviderConnected() rejects it so SET_ACTIVE can't silently + // reactivate a provider whose credentials were deliberately dropped. + if (!config.isProviderConnected(data.providerId)) { + return {error: `Provider "${data.providerId}" is not connected`, success: false} + } + } + await this.providerConfigStore.setActiveProvider(data.providerId) this.transport.broadcast(TransportDaemonEventNames.PROVIDER_UPDATED, {}) return {success: true} diff --git a/src/shared/transport/types/dto.ts b/src/shared/transport/types/dto.ts index a4dee6bf3..927b25ed5 100644 --- a/src/shared/transport/types/dto.ts +++ b/src/shared/transport/types/dto.ts @@ -159,6 +159,13 @@ export interface ProviderDTO { id: string isConnected: boolean isCurrent: boolean + /** + * Present when the provider was disconnected with a recorded reason (e.g. a + * permanent OAuth refresh failure). Set alongside `isConnected: false` so the + * providers view can surface when/why it dropped and how to reconnect + * (`brv providers connect --oauth` for oauth authMethod, else without `--oauth`). + */ + lastDisconnect?: {at: string; errorCode?: string; reason: string; statusCode?: number} name: string oauthCallbackMode?: 'auto' | 'code-paste' oauthLabel?: string diff --git a/test/unit/core/domain/entities/provider-config.test.ts b/test/unit/core/domain/entities/provider-config.test.ts index 12acb5b8e..8ff8268ef 100644 --- a/test/unit/core/domain/entities/provider-config.test.ts +++ b/test/unit/core/domain/entities/provider-config.test.ts @@ -184,5 +184,95 @@ describe('ProviderConfig', () => { expect(disconnected.activeProvider).to.equal('anthropic') }) + + it('should retain a tombstone with lastDisconnect when details are provided', () => { + const config = ProviderConfig.createDefault() + .withProviderConnected('openai', {authMethod: 'oauth', oauthAccountId: 'acct_123'}) + .withActiveProvider('openai') + + const disconnected = config.withProviderDisconnected('openai', { + errorCode: 'invalid_grant', + reason: 'OAuth token refresh failed', + statusCode: 400, + }) + + // Entry is kept (exists in config) but reports as not connected + expect(disconnected.providers.openai).to.not.be.undefined + expect(disconnected.isProviderConnected('openai')).to.be.false + expect(disconnected.activeProvider).to.equal('') + + const {lastDisconnect} = disconnected.providers.openai + expect(lastDisconnect?.reason).to.equal('OAuth token refresh failed') + expect(lastDisconnect?.errorCode).to.equal('invalid_grant') + expect(lastDisconnect?.statusCode).to.equal(400) + expect(lastDisconnect?.at).to.be.a('string') + + if (!lastDisconnect) { + throw new Error('Expected lastDisconnect to be set') + } + + expect(Number.isNaN(Date.parse(lastDisconnect.at))).to.be.false + + // Last-known authMethod is preserved so the view can build the reconnect hint + expect(disconnected.providers.openai.authMethod).to.equal('oauth') + }) + + it('should record lastDisconnect without optional error/status fields', () => { + const config = ProviderConfig.createDefault().withProviderConnected('openrouter', {authMethod: 'api-key'}) + + const disconnected = config.withProviderDisconnected('openrouter', {reason: 'Manual disconnect'}) + + const {lastDisconnect} = disconnected.providers.openrouter + expect(lastDisconnect?.reason).to.equal('Manual disconnect') + expect(lastDisconnect?.errorCode).to.be.undefined + expect(lastDisconnect?.statusCode).to.be.undefined + }) + + it('should remove the entry entirely when details are omitted even if it existed', () => { + const config = ProviderConfig.createDefault().withProviderConnected('openai', {authMethod: 'oauth'}) + + const disconnected = config.withProviderDisconnected('openai') + + expect(disconnected.providers.openai).to.be.undefined + expect(disconnected.isProviderConnected('openai')).to.be.false + }) + + it('should not create a tombstone for a provider that was never connected', () => { + const config = ProviderConfig.createDefault() + + const disconnected = config.withProviderDisconnected('openai', {reason: 'OAuth token refresh failed'}) + + expect(disconnected.providers.openai).to.be.undefined + }) + + it('should clear lastDisconnect when the provider is reconnected', () => { + const disconnected = ProviderConfig.createDefault() + .withProviderConnected('openai', {authMethod: 'oauth'}) + .withProviderDisconnected('openai', {errorCode: 'invalid_grant', reason: 'OAuth token refresh failed'}) + + expect(disconnected.isProviderConnected('openai')).to.be.false + + const reconnected = disconnected.withProviderConnected('openai', {authMethod: 'oauth'}) + + expect(reconnected.isProviderConnected('openai')).to.be.true + expect(reconnected.providers.openai.lastDisconnect).to.be.undefined + }) + + it('should round-trip lastDisconnect through toJson/fromJson', () => { + const disconnected = ProviderConfig.createDefault() + .withProviderConnected('openai', {authMethod: 'oauth'}) + .withProviderDisconnected('openai', { + errorCode: 'invalid_grant', + reason: 'OAuth token refresh failed', + statusCode: 400, + }) + + const restored = ProviderConfig.fromJson(disconnected.toJson()) + + expect(restored.isProviderConnected('openai')).to.be.false + expect(restored.providers.openai.lastDisconnect?.reason).to.equal('OAuth token refresh failed') + expect(restored.providers.openai.lastDisconnect?.errorCode).to.equal('invalid_grant') + expect(restored.providers.openai.lastDisconnect?.statusCode).to.equal(400) + }) }) }) diff --git a/test/unit/infra/provider-oauth/token-refresh-manager.test.ts b/test/unit/infra/provider-oauth/token-refresh-manager.test.ts index 933f81955..271391731 100644 --- a/test/unit/infra/provider-oauth/token-refresh-manager.test.ts +++ b/test/unit/infra/provider-oauth/token-refresh-manager.test.ts @@ -186,6 +186,13 @@ describe('TokenRefreshManager', () => { expect(providerOAuthTokenStore.delete.calledWith('openai')).to.be.true expect(providerKeychainStore.deleteApiKey.calledWith('openai')).to.be.true expect(transport.broadcast.calledWith(TransportDaemonEventNames.PROVIDER_UPDATED, {})).to.be.true + + // Verify the disconnect reason/details were plumbed through so the drop is + // recorded durably (lastDisconnect) instead of vanishing silently. + const [disconnectedId, disconnectDetails] = providerConfigStore.disconnectProvider.firstCall.args + expect(disconnectedId).to.equal('openai') + expect(disconnectDetails).to.include({errorCode: 'invalid_grant', statusCode: 400}) + expect(disconnectDetails?.reason).to.equal('OAuth token refresh failed') }) it('should return true and keep credentials intact on transient refresh failure', async () => { @@ -319,6 +326,11 @@ describe('TokenRefreshManager', () => { expect(providerOAuthTokenStore.delete.calledWith('openai')).to.be.true expect(providerKeychainStore.deleteApiKey.calledWith('openai')).to.be.true expect(transport.broadcast.calledWith(TransportDaemonEventNames.PROVIDER_UPDATED, {})).to.be.true + + // Reason/details still plumbed through even when a later cleanup step fails. + const disconnectDetails = providerConfigStore.disconnectProvider.firstCall.args[1] + expect(disconnectDetails).to.include({errorCode: 'invalid_grant', statusCode: 401}) + expect(disconnectDetails?.reason).to.equal('OAuth token refresh failed') }) }) diff --git a/test/unit/infra/provider/provider-config-resolver.test.ts b/test/unit/infra/provider/provider-config-resolver.test.ts index 66afc603e..7e169e1c2 100644 --- a/test/unit/infra/provider/provider-config-resolver.test.ts +++ b/test/unit/infra/provider/provider-config-resolver.test.ts @@ -531,5 +531,33 @@ describe('provider-config-resolver', () => { expect(configStore.write.calledOnce).to.be.true }) + + it('should preserve an already-tombstoned provider instead of re-disconnecting it', async () => { + const {configStore, keychainStore} = createStubStores(sandbox) + // A provider that was disconnected with a recorded reason: its keychain key + // is intentionally gone, so it would otherwise look "stale" and be removed. + const tombstoned = ProviderConfig.createDefault() + .withProviderConnected('openai', {authMethod: 'oauth'}) + .withProviderDisconnected('openai', { + errorCode: 'invalid_grant', + reason: 'OAuth token refresh failed', + statusCode: 400, + }) + configStore.read.resolves(tombstoned) + keychainStore.getApiKey.resolves() + + const oauthTokenStore: SinonStubbedInstance = { + delete: sandbox.stub().resolves(), + get: sandbox.stub().resolves(), + has: sandbox.stub().resolves(false), + set: sandbox.stub().resolves(), + } as unknown as SinonStubbedInstance + + await clearStaleProviderConfig(configStore, keychainStore, oauthTokenStore) + + // Tombstone left untouched: no re-write, no re-delete — the reason survives + expect(configStore.write.notCalled).to.be.true + expect(oauthTokenStore.delete.notCalled).to.be.true + }) }) }) diff --git a/test/unit/infra/storage/file-provider-config-store.test.ts b/test/unit/infra/storage/file-provider-config-store.test.ts new file mode 100644 index 000000000..631d61958 --- /dev/null +++ b/test/unit/infra/storage/file-provider-config-store.test.ts @@ -0,0 +1,95 @@ +import {expect} from 'chai' +import {mkdir, rm, writeFile} from 'node:fs/promises' +import {tmpdir} from 'node:os' +import {join} from 'node:path' + +import type {FileProviderConfigStoreDeps} from '../../../../src/server/infra/storage/file-provider-config-store.js' + +import {FileProviderConfigStore} from '../../../../src/server/infra/storage/file-provider-config-store.js' + +describe('FileProviderConfigStore', () => { + let tempDir: string + let deps: FileProviderConfigStoreDeps + + beforeEach(async () => { + tempDir = join(tmpdir(), `brv-provider-config-${Date.now()}-${Math.random().toString(36).slice(2)}`) + await mkdir(tempDir, {recursive: true}) + + deps = { + getConfigDir: () => tempDir, + getConfigPath: () => join(tempDir, 'providers.json'), + } + }) + + afterEach(async () => { + try { + await rm(tempDir, {force: true, recursive: true}) + } catch { + // Ignore cleanup errors + } + }) + + describe('disconnectProvider with details', () => { + it('should round-trip a lastDisconnect tombstone through save/load', async () => { + const store = new FileProviderConfigStore(deps) + await store.connectProvider('openai', {authMethod: 'oauth', oauthAccountId: 'acct_123'}) + + await store.disconnectProvider('openai', { + errorCode: 'invalid_grant', + reason: 'OAuth token refresh failed', + statusCode: 400, + }) + + // Fresh store instance forces a read from disk (no cache reuse) + const reloaded = await new FileProviderConfigStore(deps).read() + + expect(reloaded.isProviderConnected('openai')).to.be.false + const entry = reloaded.providers.openai + expect(entry).to.not.be.undefined + expect(entry.lastDisconnect?.reason).to.equal('OAuth token refresh failed') + expect(entry.lastDisconnect?.errorCode).to.equal('invalid_grant') + expect(entry.lastDisconnect?.statusCode).to.equal(400) + expect(entry.lastDisconnect?.at).to.be.a('string') + // Last-known authMethod is preserved so a reconnect hint can be built + expect(entry.authMethod).to.equal('oauth') + }) + + it('should remove the entry entirely when details are omitted', async () => { + const store = new FileProviderConfigStore(deps) + await store.connectProvider('openai', {authMethod: 'oauth'}) + + await store.disconnectProvider('openai') + + const reloaded = await new FileProviderConfigStore(deps).read() + expect(reloaded.providers.openai).to.be.undefined + expect(reloaded.isProviderConnected('openai')).to.be.false + }) + }) + + describe('backward compatibility', () => { + it('should load an existing providers.json that has no lastDisconnect field', async () => { + const legacyConfig = { + activeProvider: 'openai', + providers: { + openai: { + activeModel: 'some-model', + authMethod: 'oauth', + connectedAt: '2025-01-01T00:00:00.000Z', + favoriteModels: [], + oauthAccountId: 'acct_legacy', + recentModels: [], + }, + }, + } + await writeFile(join(tempDir, 'providers.json'), JSON.stringify(legacyConfig, null, 2), 'utf8') + + const config = await new FileProviderConfigStore(deps).read() + + // Old file loads unchanged: provider is connected and has no tombstone + expect(config.isProviderConnected('openai')).to.be.true + expect(config.providers.openai.lastDisconnect).to.be.undefined + expect(config.providers.openai.authMethod).to.equal('oauth') + expect(config.activeProvider).to.equal('openai') + }) + }) +}) diff --git a/test/unit/infra/transport/handlers/model-handler.test.ts b/test/unit/infra/transport/handlers/model-handler.test.ts index 4c778a128..2ba460083 100644 --- a/test/unit/infra/transport/handlers/model-handler.test.ts +++ b/test/unit/infra/transport/handlers/model-handler.test.ts @@ -138,6 +138,52 @@ describe('ModelHandler', () => { expect(transport.broadcast.called).to.be.false }) + it('should reject a disconnected tombstone (permanent OAuth refresh failure) and not mutate the active provider', async () => { + // The entry still exists in config.providers (kept for lastDisconnect), + // so presence alone must not be enough to let SET_ACTIVE reactivate it. + providerConfigStore.read.resolves( + ProviderConfig.createDefault() + .withProviderConnected('openai', {authMethod: 'oauth'}) + .withProviderDisconnected('openai', { + errorCode: 'invalid_grant', + reason: 'OAuth token refresh failed', + statusCode: 400, + }), + ) + createHandler() + + const handler = transport._handlers.get(ModelEvents.SET_ACTIVE) + if (!handler) { + throw new Error('ModelEvents.SET_ACTIVE handler not registered') + } + + const result = await handler({modelId: 'some-model', providerId: 'openai'}, 'client-1') + + expect(result.success).to.be.false + expect(result.error).to.include('not connected') + expect(providerConfigStore.setActiveProvider.called).to.be.false + expect(providerConfigStore.setActiveModel.called).to.be.false + expect(transport.broadcast.called).to.be.false + }) + + it('should still succeed for a healthy (non-tombstoned) api-key-connected provider', async () => { + providerConfigStore.read.resolves( + ProviderConfig.createDefault().withProviderConnected('openrouter', {authMethod: 'api-key'}), + ) + createHandler() + + const handler = transport._handlers.get(ModelEvents.SET_ACTIVE) + if (!handler) { + throw new Error('ModelEvents.SET_ACTIVE handler not registered') + } + + const result = await handler({modelId: 'some-model', providerId: 'openrouter'}, 'client-1') + + expect(result).to.deep.equal({success: true}) + expect(providerConfigStore.setActiveProvider.calledWith('openrouter')).to.be.true + expect(providerConfigStore.setActiveModel.calledWith('openrouter', 'some-model')).to.be.true + }) + it('should return structured error when config store throws', async () => { providerConfigStore.read.rejects(new Error('Config file corrupted')) diff --git a/test/unit/infra/transport/handlers/provider-handler.test.ts b/test/unit/infra/transport/handlers/provider-handler.test.ts index b9765ee21..504a795e1 100644 --- a/test/unit/infra/transport/handlers/provider-handler.test.ts +++ b/test/unit/infra/transport/handlers/provider-handler.test.ts @@ -423,6 +423,9 @@ describe('ProviderHandler', () => { describe('provider:setActive', () => { it('should broadcast provider:updated after setting active provider', async () => { + providerConfigStore.read.resolves( + ProviderConfig.createDefault().withProviderConnected('openrouter', {authMethod: 'api-key'}), + ) createHandler() const handler = transport._handlers.get(ProviderEvents.SET_ACTIVE) @@ -434,6 +437,9 @@ describe('ProviderHandler', () => { }) it('should set active provider before broadcasting', async () => { + providerConfigStore.read.resolves( + ProviderConfig.createDefault().withProviderConnected('openrouter', {authMethod: 'api-key'}), + ) createHandler() const handler = transport._handlers.get(ProviderEvents.SET_ACTIVE) @@ -442,6 +448,50 @@ describe('ProviderHandler', () => { expect(providerConfigStore.setActiveProvider.calledWith('openrouter')).to.be.true expect(providerConfigStore.setActiveProvider.calledBefore(transport.broadcast)).to.be.true }) + + it('should reject a disconnected tombstone (permanent OAuth refresh failure) and not mutate the active provider', async () => { + // The entry still exists in config.providers (kept for lastDisconnect), + // so presence alone must not be enough to let SET_ACTIVE reactivate it. + providerConfigStore.read.resolves( + ProviderConfig.createDefault() + .withProviderConnected('openai', {authMethod: 'oauth'}) + .withProviderDisconnected('openai', { + errorCode: 'invalid_grant', + reason: 'OAuth token refresh failed', + statusCode: 400, + }), + ) + createHandler() + + const handler = transport._handlers.get(ProviderEvents.SET_ACTIVE) + if (!handler) { + throw new Error('ProviderEvents.SET_ACTIVE handler not registered') + } + + const result = await handler({providerId: 'openai'}, 'client-1') + + expect(result.success).to.be.false + expect(result.error).to.equal('Provider "openai" is not connected') + expect(providerConfigStore.setActiveProvider.called).to.be.false + expect(transport.broadcast.called).to.be.false + }) + + it('should still succeed for a healthy (non-tombstoned) connected provider', async () => { + providerConfigStore.read.resolves( + ProviderConfig.createDefault().withProviderConnected('openrouter', {authMethod: 'api-key'}), + ) + createHandler() + + const handler = transport._handlers.get(ProviderEvents.SET_ACTIVE) + if (!handler) { + throw new Error('ProviderEvents.SET_ACTIVE handler not registered') + } + + const result = await handler({providerId: 'openrouter'}, 'client-1') + + expect(result).to.deep.equal({success: true}) + expect(providerConfigStore.setActiveProvider.calledWith('openrouter')).to.be.true + }) }) describe('provider:getActive', () => { @@ -945,6 +995,53 @@ describe('ProviderHandler', () => { expect(openaiCompat?.isConnected).to.be.true expect(openaiCompat?.activeModel).to.be.undefined }) + + it('should surface lastDisconnect for a provider disconnected with a recorded reason', async () => { + const config = ProviderConfig.createDefault() + .withProviderConnected('openai', {authMethod: 'oauth', oauthAccountId: 'acct_123'}) + .withProviderDisconnected('openai', { + errorCode: 'invalid_grant', + reason: 'OAuth token refresh failed', + statusCode: 400, + }) + providerConfigStore.read.resolves(config) + providerConfigStore.isProviderConnected.withArgs('openai').resolves(false) + createHandler() + + const handler = transport._handlers.get(ProviderEvents.LIST) + if (!handler) { + throw new Error('ProviderEvents.LIST handler not registered') + } + + const result = await handler(undefined, 'client-1') + + const openaiProvider = result.providers.find((p: {id: string}) => p.id === 'openai') + // Not connected, but the drop reason is surfaced so the view can explain it + expect(openaiProvider?.isConnected).to.be.false + expect(openaiProvider?.lastDisconnect?.reason).to.equal('OAuth token refresh failed') + expect(openaiProvider?.lastDisconnect?.errorCode).to.equal('invalid_grant') + expect(openaiProvider?.lastDisconnect?.statusCode).to.equal(400) + // authMethod is retained so the reconnect hint can choose the --oauth form + expect(openaiProvider?.authMethod).to.equal('oauth') + }) + + it('should omit lastDisconnect for a normally connected provider', async () => { + const config = ProviderConfig.createDefault().withProviderConnected('openai', {authMethod: 'oauth'}) + providerConfigStore.read.resolves(config) + providerConfigStore.isProviderConnected.withArgs('openai').resolves(true) + createHandler() + + const handler = transport._handlers.get(ProviderEvents.LIST) + if (!handler) { + throw new Error('ProviderEvents.LIST handler not registered') + } + + const result = await handler(undefined, 'client-1') + + const openaiProvider = result.providers.find((p: {id: string}) => p.id === 'openai') + expect(openaiProvider?.isConnected).to.be.true + expect(openaiProvider?.lastDisconnect).to.be.undefined + }) }) describe('provider:cancelOAuth', () => {