From 0035170cfbfc9da5379fbd5df360bc30c633fb2a Mon Sep 17 00:00:00 2001 From: elkaix Date: Mon, 17 Aug 2026 01:49:46 -0400 Subject: [PATCH] fix(web): point provider calls at routes that exist The web client posted to POST /providers, DELETE /providers/{id} and POST /providers/{id}:refresh. None of those routes were ever registered, so adding an API key from the desktop app returned 404 and a new user could not configure a provider at all. Adding a provider now writes one POST /config patch carrying the provider, a model alias and default_model, which is what GET /auth needs before it reports ready. Refresh reads the real GET /providers/{id}. Removal cannot go through POST /config, because the config patch deep-merges and strips undefined, so a key can never be cleared. A removePythinkerProvider RPC already existed with no HTTP route; this adds DELETE /providers/{provider_id} wired to it, which also cleans up model aliases pointing at the removed provider. --- .changeset/web-provider-routes.md | 5 ++ apps/pythinker-web/src/api/daemon/client.ts | 40 ++++++--- .../test/daemon-contracts.test.ts | 83 +++++++++++++++++++ .../src/services/modelCatalog/modelCatalog.ts | 1 + .../modelCatalog/modelCatalogService.ts | 8 ++ .../test/services/config-service.test.ts | 57 +++++++++++++ .../services/model-catalog-service.test.ts | 13 +++ packages/server/src/routes/modelCatalog.ts | 44 ++++++++++ .../server/test/model-catalog.e2e.test.ts | 73 +++++++++++++++- 9 files changed, 309 insertions(+), 15 deletions(-) create mode 100644 .changeset/web-provider-routes.md create mode 100644 packages/agent-core/test/services/config-service.test.ts diff --git a/.changeset/web-provider-routes.md b/.changeset/web-provider-routes.md new file mode 100644 index 00000000..14c09b3f --- /dev/null +++ b/.changeset/web-provider-routes.md @@ -0,0 +1,5 @@ +--- +"@pymodel/pythinker-code": minor +--- + +Point the web provider calls at routes that exist. Adding a provider now writes through `POST /config`, refreshing reads `GET /providers/{id}`, and a new `DELETE /providers/{provider_id}` route removes a provider together with the model aliases that referenced it. diff --git a/apps/pythinker-web/src/api/daemon/client.ts b/apps/pythinker-web/src/api/daemon/client.ts index 07a54b8a..1cfcb079 100644 --- a/apps/pythinker-web/src/api/daemon/client.ts +++ b/apps/pythinker-web/src/api/daemon/client.ts @@ -1106,17 +1106,14 @@ export class DaemonPythinkerWebApi implements PythinkerWebApi { // ------------------------------------------------------------------------- // Models + Providers - // PRESUMED — not in current daemon docs; isolated here, swap when backend defines them. // ------------------------------------------------------------------------- async listModels(): Promise { - // PRESUMED endpoint: GET /v1/models → { items: WireModel[] } const data = await this.http.get<{ items: WireModel[] }>('/models'); return data.items.map(toAppModel); } async listProviders(): Promise { - // PRESUMED endpoint: GET /v1/providers → { items: WireProvider[] } const data = await this.http.get<{ items: WireProvider[] }>('/providers'); return data.items.map(toAppProvider); } @@ -1127,29 +1124,46 @@ export class DaemonPythinkerWebApi implements PythinkerWebApi { baseUrl?: string; defaultModel?: string; }): Promise { - // PRESUMED endpoint: POST /v1/providers → WireProvider - const body: Record = { type: input.type }; - if (input.apiKey !== undefined) body['api_key'] = input.apiKey; - if (input.baseUrl !== undefined) body['base_url'] = input.baseUrl; - if (input.defaultModel !== undefined) body['default_model'] = input.defaultModel; - const data = await this.http.post('/providers', body); + const providerId = input.type.replaceAll('_', '-'); + const modelId = input.defaultModel ?? providerId; + const modelAlias = `${providerId}/${modelId}`.replaceAll('_', '-'); + await this.http.post('/config', { + providers: { + [providerId]: { + type: input.type, + api_key: input.apiKey, + base_url: input.baseUrl, + default_model: input.defaultModel, + }, + }, + models: { + [modelAlias]: { + provider: providerId, + model: modelId, + max_context_size: 262_144, + }, + }, + default_model: modelAlias, + }); + const data = await this.http.get( + `/providers/${encodeURIComponent(providerId)}`, + ); return toAppProvider(data); } async deleteProvider(id: string): Promise<{ deleted: true }> { - // PRESUMED endpoint: DELETE /v1/providers/{id} → { deleted: true } return this.http.delete<{ deleted: true }>(`/providers/${encodeURIComponent(id)}`); } async refreshProvider(id: string): Promise { - // PRESUMED endpoint: POST /v1/providers/{id}:refresh → WireProvider - const data = await this.http.post( - `/providers/${encodeURIComponent(id)}:refresh`, + const data = await this.http.get( + `/providers/${encodeURIComponent(id)}`, ); return toAppProvider(data); } async refreshOAuthProviderModels(): Promise { + // No server route or core RPC currently backs this presumed endpoint. const data = await this.http.post('/providers:refresh_oauth'); return { changed: data.changed.map((item) => ({ diff --git a/apps/pythinker-web/test/daemon-contracts.test.ts b/apps/pythinker-web/test/daemon-contracts.test.ts index 5d7bdb6d..eeaf42ca 100644 --- a/apps/pythinker-web/test/daemon-contracts.test.ts +++ b/apps/pythinker-web/test/daemon-contracts.test.ts @@ -129,3 +129,86 @@ describe('dynamic workflow daemon contracts', () => { expect(dynamicClient.dynamicWorkflowMode.value).toBe(false); }); }); + +describe('provider daemon contracts', () => { + it('adds a provider through one config patch and reads it back', async () => { + const provider = { + id: 'openai-responses', + type: 'openai_responses', + base_url: 'https://api.example.test/v1', + default_model: 'gpt_5-mini', + has_api_key: true, + status: 'connected', + models: ['openai-responses/gpt-5-mini'], + }; + const fetchMock = vi.fn() + .mockResolvedValueOnce(okEnvelope({})) + .mockResolvedValueOnce(okEnvelope(provider)); + vi.stubGlobal('fetch', fetchMock); + + await expect(api().addProvider({ + type: 'openai_responses', + apiKey: 'sk-test', + baseUrl: 'https://api.example.test/v1', + defaultModel: 'gpt_5-mini', + })).resolves.toMatchObject({ id: 'openai-responses', defaultModel: 'gpt_5-mini' }); + + expect(fetchMock).toHaveBeenCalledTimes(2); + expect(fetchMock.mock.calls[0]![0]).toBe('http://example.test:58627/api/v1/config'); + expect(fetchMock.mock.calls[0]![1]).toMatchObject({ method: 'POST' }); + expect(JSON.parse((fetchMock.mock.calls[0]![1] as RequestInit).body as string)).toEqual({ + providers: { + 'openai-responses': { + type: 'openai_responses', + api_key: 'sk-test', + base_url: 'https://api.example.test/v1', + default_model: 'gpt_5-mini', + }, + }, + models: { + 'openai-responses/gpt-5-mini': { + provider: 'openai-responses', + model: 'gpt_5-mini', + max_context_size: 262_144, + }, + }, + default_model: 'openai-responses/gpt-5-mini', + }); + expect(fetchMock.mock.calls[1]![0]).toBe( + 'http://example.test:58627/api/v1/providers/openai-responses', + ); + expect(fetchMock.mock.calls[1]![1]).toMatchObject({ method: 'GET' }); + }); + + it('deletes a provider through the provider resource route', async () => { + const fetchMock = vi.fn().mockResolvedValueOnce(okEnvelope({ deleted: true })); + vi.stubGlobal('fetch', fetchMock); + + await expect(api().deleteProvider('openai/custom')).resolves.toEqual({ deleted: true }); + + expect(fetchMock.mock.calls[0]![0]).toBe( + 'http://example.test:58627/api/v1/providers/openai%2Fcustom', + ); + expect(fetchMock.mock.calls[0]![1]).toMatchObject({ method: 'DELETE' }); + expect((fetchMock.mock.calls[0]![1] as RequestInit).body).toBeUndefined(); + }); + + it('refreshes a provider by reading the provider resource', async () => { + const fetchMock = vi.fn().mockResolvedValueOnce(okEnvelope({ + id: 'openai', + type: 'openai', + has_api_key: true, + status: 'connected', + models: ['openai/gpt-5'], + })); + vi.stubGlobal('fetch', fetchMock); + + await expect(api().refreshProvider('openai')).resolves.toMatchObject({ id: 'openai' }); + + expect(fetchMock.mock.calls[0]![0]).toBe( + 'http://example.test:58627/api/v1/providers/openai', + ); + expect(fetchMock.mock.calls[0]![1]).toMatchObject({ method: 'GET' }); + expect((fetchMock.mock.calls[0]![1] as RequestInit).body).toBeUndefined(); + }); +}); diff --git a/packages/agent-core/src/services/modelCatalog/modelCatalog.ts b/packages/agent-core/src/services/modelCatalog/modelCatalog.ts index 0524cf62..054cfcc2 100644 --- a/packages/agent-core/src/services/modelCatalog/modelCatalog.ts +++ b/packages/agent-core/src/services/modelCatalog/modelCatalog.ts @@ -12,6 +12,7 @@ export interface IModelCatalogService { listModels(): Promise; listProviders(): Promise; getProvider(providerId: string): Promise; + removeProvider(providerId: string): Promise; setDefaultModel(modelId: string): Promise; } diff --git a/packages/agent-core/src/services/modelCatalog/modelCatalogService.ts b/packages/agent-core/src/services/modelCatalog/modelCatalogService.ts index 0e3513f1..9c4fd738 100644 --- a/packages/agent-core/src/services/modelCatalog/modelCatalogService.ts +++ b/packages/agent-core/src/services/modelCatalog/modelCatalogService.ts @@ -52,6 +52,14 @@ export class ModelCatalogService return this._provider(config, providerId, provider); } + async removeProvider(providerId: string): Promise { + const config = await this._readConfig(); + if (config.providers?.[providerId] === undefined) { + throw new ProviderNotFoundError(providerId); + } + await this.core.rpc.removePythinkerProvider({ providerId }); + } + async setDefaultModel(modelId: string): Promise { const config = await this._readConfig(); const alias = config.models?.[modelId]; diff --git a/packages/agent-core/test/services/config-service.test.ts b/packages/agent-core/test/services/config-service.test.ts new file mode 100644 index 00000000..f6c26952 --- /dev/null +++ b/packages/agent-core/test/services/config-service.test.ts @@ -0,0 +1,57 @@ +import { describe, expect, it, vi } from 'vitest'; + +import type { CoreRPC, PythinkerConfig } from '../../src'; +import { + ConfigService, + type ICoreProcessService, + type IEventService, +} from '../../src/services'; + +describe('ConfigService', () => { + it('converts underscores but preserves hyphens in nested record keys', async () => { + const setPythinkerConfig = vi.fn(async (patch: unknown) => patch as PythinkerConfig); + const core = { + rpc: { setPythinkerConfig } as unknown as CoreRPC, + } as ICoreProcessService; + const eventService = { publish: vi.fn() } as unknown as IEventService; + const service = new ConfigService(core, eventService); + + await service.set({ + providers: { + provider_with_underscore: { type: 'openai' }, + 'provider-with-hyphen': { type: 'openai' }, + }, + models: { + model_with_underscore: { + provider: 'provider_with_underscore', + model: 'model_with_underscore', + max_context_size: 1000, + }, + 'model-with-hyphen': { + provider: 'provider-with-hyphen', + model: 'model-with-hyphen', + max_context_size: 1000, + }, + }, + }); + + expect(setPythinkerConfig).toHaveBeenCalledWith({ + providers: { + providerWithUnderscore: { type: 'openai' }, + 'provider-with-hyphen': { type: 'openai' }, + }, + models: { + modelWithUnderscore: { + provider: 'provider_with_underscore', + model: 'model_with_underscore', + maxContextSize: 1000, + }, + 'model-with-hyphen': { + provider: 'provider-with-hyphen', + model: 'model-with-hyphen', + maxContextSize: 1000, + }, + }, + }); + }); +}); diff --git a/packages/agent-core/test/services/model-catalog-service.test.ts b/packages/agent-core/test/services/model-catalog-service.test.ts index 8534aae5..5e45ce37 100644 --- a/packages/agent-core/test/services/model-catalog-service.test.ts +++ b/packages/agent-core/test/services/model-catalog-service.test.ts @@ -252,4 +252,17 @@ describe('ModelCatalogService', () => { ); }); + it('removes an existing provider through core RPC', async () => { + const configRef = { current: catalogConfig() }; + const { core, removeCalls } = makeCore(configRef); + const svc = new ModelCatalogService(core); + + await expect(svc.removeProvider('pythinker')).resolves.toBeUndefined(); + expect(removeCalls).toEqual(['pythinker']); + await expect(svc.removeProvider('missing')).rejects.toBeInstanceOf( + ProviderNotFoundError, + ); + expect(removeCalls).toEqual(['pythinker']); + }); + }); diff --git a/packages/server/src/routes/modelCatalog.ts b/packages/server/src/routes/modelCatalog.ts index 2d964466..f1055b4d 100644 --- a/packages/server/src/routes/modelCatalog.ts +++ b/packages/server/src/routes/modelCatalog.ts @@ -30,6 +30,14 @@ interface ModelCatalogRouteHost { reply: { send(payload: unknown): unknown }, ) => Promise | void, ): unknown; + delete( + path: string, + options: { preHandler: unknown[]; schema?: Record }, + handler: ( + req: { id: string; params: unknown }, + reply: { send(payload: unknown): unknown }, + ) => Promise | void, + ): unknown; } const providerIdParamSchema = z.object({ @@ -40,6 +48,10 @@ const modelActionTailParamSchema = z.object({ tail: z.string().min(1), }); +const deleteProviderResponseSchema = z.object({ + deleted: z.literal(true), +}); + export function registerModelCatalogRoutes( app: ModelCatalogRouteHost, ix: IInstantiationService, @@ -161,6 +173,38 @@ export function registerModelCatalogRoutes( getProviderRoute.options, getProviderRoute.handler as Parameters[2], ); + + const deleteProviderRoute = defineRoute( + { + method: 'DELETE', + path: '/providers/{provider_id}', + params: providerIdParamSchema, + success: { data: deleteProviderResponseSchema }, + errors: { + [ErrorCode.VALIDATION_FAILED]: {}, + [ErrorCode.PROVIDER_NOT_FOUND]: {}, + }, + description: 'Delete a configured provider and its model aliases', + tags: ['providers'], + operationId: 'deleteProvider', + }, + async (req, reply) => { + try { + const { provider_id } = req.params; + await ix.invokeFunction((a) => + a.get(IModelCatalogService).removeProvider(provider_id), + ); + reply.send(okEnvelope({ deleted: true as const }, req.id)); + } catch (error) { + sendMappedError(reply, req.id, error); + } + }, + ); + app.delete( + deleteProviderRoute.path, + deleteProviderRoute.options, + deleteProviderRoute.handler as Parameters[2], + ); } function sendMappedError( diff --git a/packages/server/test/model-catalog.e2e.test.ts b/packages/server/test/model-catalog.e2e.test.ts index e183e44f..62e87de0 100644 --- a/packages/server/test/model-catalog.e2e.test.ts +++ b/packages/server/test/model-catalog.e2e.test.ts @@ -3,11 +3,12 @@ import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { pino } from 'pino'; -import { afterEach, beforeEach, describe, expect, it } from 'vitest'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; -import { IModelCatalogService, type IModelCatalogService as ModelCatalogServiceShape } from '@pymodel/agent-core'; +import type { IModelCatalogService as ModelCatalogServiceShape } from '@pymodel/agent-core'; import { IRestGateway, startServer, type RunningServer, type ServerStartOptions } from '../src'; +import { registerModelCatalogRoutes } from '../src/routes/modelCatalog'; let tmpDir: string; let lockPath: string; @@ -110,6 +111,46 @@ function seedCatalogConfig(): void { } describe('model/provider catalog routes', () => { + it('registers DELETE /providers/{provider_id} and removes through the service', async () => { + type DeleteHandler = ( + req: { id: string; params: { provider_id: string } }, + reply: { send(payload: unknown): unknown }, + ) => Promise | void; + let deleteHandler: DeleteHandler = () => { + throw new Error('delete route was not registered'); + }; + const app = { + get: vi.fn(), + post: vi.fn(), + delete: vi.fn((path: string, _options: unknown, handler: DeleteHandler) => { + expect(path).toBe('/providers/:provider_id'); + deleteHandler = handler; + }), + } as unknown as Parameters[0]; + const removeProvider = vi.fn(async () => undefined); + const service = { removeProvider } as unknown as ModelCatalogServiceShape; + const ix = { + invokeFunction: ( + fn: (accessor: { get(): ModelCatalogServiceShape }) => unknown, + ) => fn({ get: () => service }), + } as unknown as Parameters[1]; + registerModelCatalogRoutes(app, ix); + const send = vi.fn(); + + await deleteHandler( + { id: 'req_delete', params: { provider_id: 'openai' } }, + { send }, + ); + + expect(removeProvider).toHaveBeenCalledWith('openai'); + expect(send).toHaveBeenCalledWith({ + code: 0, + msg: 'success', + data: { deleted: true }, + request_id: 'req_delete', + }); + }); + it('lists configured models as selectable aliases', async () => { seedCatalogConfig(); const r = await bootDaemon(); @@ -210,6 +251,28 @@ describe('model/provider catalog routes', () => { expect(authEnv.data?.default_model).toBe('turbo'); }); + it('deletes a provider and its model aliases', async () => { + seedCatalogConfig(); + const r = await bootDaemon(); + + const removed = await appOf(r).inject({ + method: 'DELETE', + url: '/api/v1/providers/pythinker', + }); + expect(removed.statusCode).toBe(200); + expect(envelopeOf<{ deleted: true }>(removed.json()).data).toEqual({ deleted: true }); + + const provider = await appOf(r).inject({ + method: 'GET', + url: '/api/v1/providers/pythinker', + }); + expect(envelopeOf(provider.json()).code).toBe(40412); + + const models = await appOf(r).inject({ method: 'GET', url: '/api/v1/models' }); + expect(envelopeOf<{ items: Array<{ provider: string }> }>(models.json()).data?.items) + .toEqual([{ provider: 'openai', model: 'gpt4o', display_name: 'gpt-4o', max_context_size: 128000 }]); + }); + it('maps unknown provider and model ids to catalog not-found error codes', async () => { seedCatalogConfig(); const r = await bootDaemon(); @@ -220,6 +283,12 @@ describe('model/provider catalog routes', () => { }); expect(envelopeOf(provider.json()).code).toBe(40412); + const deleteProvider = await appOf(r).inject({ + method: 'DELETE', + url: '/api/v1/providers/missing', + }); + expect(envelopeOf(deleteProvider.json()).code).toBe(40412); + const model = await appOf(r).inject({ method: 'POST', url: '/api/v1/models/missing:set_default',