Skip to content

Commit 0035170

Browse files
committed
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.
1 parent 13a0621 commit 0035170

9 files changed

Lines changed: 309 additions & 15 deletions

File tree

.changeset/web-provider-routes.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@pymodel/pythinker-code": minor
3+
---
4+
5+
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.

apps/pythinker-web/src/api/daemon/client.ts

Lines changed: 27 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1106,17 +1106,14 @@ export class DaemonPythinkerWebApi implements PythinkerWebApi {
11061106

11071107
// -------------------------------------------------------------------------
11081108
// Models + Providers
1109-
// PRESUMED — not in current daemon docs; isolated here, swap when backend defines them.
11101109
// -------------------------------------------------------------------------
11111110

11121111
async listModels(): Promise<AppModel[]> {
1113-
// PRESUMED endpoint: GET /v1/models → { items: WireModel[] }
11141112
const data = await this.http.get<{ items: WireModel[] }>('/models');
11151113
return data.items.map(toAppModel);
11161114
}
11171115

11181116
async listProviders(): Promise<AppProvider[]> {
1119-
// PRESUMED endpoint: GET /v1/providers → { items: WireProvider[] }
11201117
const data = await this.http.get<{ items: WireProvider[] }>('/providers');
11211118
return data.items.map(toAppProvider);
11221119
}
@@ -1127,29 +1124,46 @@ export class DaemonPythinkerWebApi implements PythinkerWebApi {
11271124
baseUrl?: string;
11281125
defaultModel?: string;
11291126
}): Promise<AppProvider> {
1130-
// PRESUMED endpoint: POST /v1/providers → WireProvider
1131-
const body: Record<string, unknown> = { type: input.type };
1132-
if (input.apiKey !== undefined) body['api_key'] = input.apiKey;
1133-
if (input.baseUrl !== undefined) body['base_url'] = input.baseUrl;
1134-
if (input.defaultModel !== undefined) body['default_model'] = input.defaultModel;
1135-
const data = await this.http.post<WireProvider>('/providers', body);
1127+
const providerId = input.type.replaceAll('_', '-');
1128+
const modelId = input.defaultModel ?? providerId;
1129+
const modelAlias = `${providerId}/${modelId}`.replaceAll('_', '-');
1130+
await this.http.post('/config', {
1131+
providers: {
1132+
[providerId]: {
1133+
type: input.type,
1134+
api_key: input.apiKey,
1135+
base_url: input.baseUrl,
1136+
default_model: input.defaultModel,
1137+
},
1138+
},
1139+
models: {
1140+
[modelAlias]: {
1141+
provider: providerId,
1142+
model: modelId,
1143+
max_context_size: 262_144,
1144+
},
1145+
},
1146+
default_model: modelAlias,
1147+
});
1148+
const data = await this.http.get<WireProvider>(
1149+
`/providers/${encodeURIComponent(providerId)}`,
1150+
);
11361151
return toAppProvider(data);
11371152
}
11381153

11391154
async deleteProvider(id: string): Promise<{ deleted: true }> {
1140-
// PRESUMED endpoint: DELETE /v1/providers/{id} → { deleted: true }
11411155
return this.http.delete<{ deleted: true }>(`/providers/${encodeURIComponent(id)}`);
11421156
}
11431157

11441158
async refreshProvider(id: string): Promise<AppProvider> {
1145-
// PRESUMED endpoint: POST /v1/providers/{id}:refresh → WireProvider
1146-
const data = await this.http.post<WireProvider>(
1147-
`/providers/${encodeURIComponent(id)}:refresh`,
1159+
const data = await this.http.get<WireProvider>(
1160+
`/providers/${encodeURIComponent(id)}`,
11481161
);
11491162
return toAppProvider(data);
11501163
}
11511164

11521165
async refreshOAuthProviderModels(): Promise<ProviderRefreshResult> {
1166+
// No server route or core RPC currently backs this presumed endpoint.
11531167
const data = await this.http.post<WireProviderRefreshResult>('/providers:refresh_oauth');
11541168
return {
11551169
changed: data.changed.map((item) => ({

apps/pythinker-web/test/daemon-contracts.test.ts

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,3 +129,86 @@ describe('dynamic workflow daemon contracts', () => {
129129
expect(dynamicClient.dynamicWorkflowMode.value).toBe(false);
130130
});
131131
});
132+
133+
describe('provider daemon contracts', () => {
134+
it('adds a provider through one config patch and reads it back', async () => {
135+
const provider = {
136+
id: 'openai-responses',
137+
type: 'openai_responses',
138+
base_url: 'https://api.example.test/v1',
139+
default_model: 'gpt_5-mini',
140+
has_api_key: true,
141+
status: 'connected',
142+
models: ['openai-responses/gpt-5-mini'],
143+
};
144+
const fetchMock = vi.fn()
145+
.mockResolvedValueOnce(okEnvelope({}))
146+
.mockResolvedValueOnce(okEnvelope(provider));
147+
vi.stubGlobal('fetch', fetchMock);
148+
149+
await expect(api().addProvider({
150+
type: 'openai_responses',
151+
apiKey: 'sk-test',
152+
baseUrl: 'https://api.example.test/v1',
153+
defaultModel: 'gpt_5-mini',
154+
})).resolves.toMatchObject({ id: 'openai-responses', defaultModel: 'gpt_5-mini' });
155+
156+
expect(fetchMock).toHaveBeenCalledTimes(2);
157+
expect(fetchMock.mock.calls[0]![0]).toBe('http://example.test:58627/api/v1/config');
158+
expect(fetchMock.mock.calls[0]![1]).toMatchObject({ method: 'POST' });
159+
expect(JSON.parse((fetchMock.mock.calls[0]![1] as RequestInit).body as string)).toEqual({
160+
providers: {
161+
'openai-responses': {
162+
type: 'openai_responses',
163+
api_key: 'sk-test',
164+
base_url: 'https://api.example.test/v1',
165+
default_model: 'gpt_5-mini',
166+
},
167+
},
168+
models: {
169+
'openai-responses/gpt-5-mini': {
170+
provider: 'openai-responses',
171+
model: 'gpt_5-mini',
172+
max_context_size: 262_144,
173+
},
174+
},
175+
default_model: 'openai-responses/gpt-5-mini',
176+
});
177+
expect(fetchMock.mock.calls[1]![0]).toBe(
178+
'http://example.test:58627/api/v1/providers/openai-responses',
179+
);
180+
expect(fetchMock.mock.calls[1]![1]).toMatchObject({ method: 'GET' });
181+
});
182+
183+
it('deletes a provider through the provider resource route', async () => {
184+
const fetchMock = vi.fn().mockResolvedValueOnce(okEnvelope({ deleted: true }));
185+
vi.stubGlobal('fetch', fetchMock);
186+
187+
await expect(api().deleteProvider('openai/custom')).resolves.toEqual({ deleted: true });
188+
189+
expect(fetchMock.mock.calls[0]![0]).toBe(
190+
'http://example.test:58627/api/v1/providers/openai%2Fcustom',
191+
);
192+
expect(fetchMock.mock.calls[0]![1]).toMatchObject({ method: 'DELETE' });
193+
expect((fetchMock.mock.calls[0]![1] as RequestInit).body).toBeUndefined();
194+
});
195+
196+
it('refreshes a provider by reading the provider resource', async () => {
197+
const fetchMock = vi.fn().mockResolvedValueOnce(okEnvelope({
198+
id: 'openai',
199+
type: 'openai',
200+
has_api_key: true,
201+
status: 'connected',
202+
models: ['openai/gpt-5'],
203+
}));
204+
vi.stubGlobal('fetch', fetchMock);
205+
206+
await expect(api().refreshProvider('openai')).resolves.toMatchObject({ id: 'openai' });
207+
208+
expect(fetchMock.mock.calls[0]![0]).toBe(
209+
'http://example.test:58627/api/v1/providers/openai',
210+
);
211+
expect(fetchMock.mock.calls[0]![1]).toMatchObject({ method: 'GET' });
212+
expect((fetchMock.mock.calls[0]![1] as RequestInit).body).toBeUndefined();
213+
});
214+
});

packages/agent-core/src/services/modelCatalog/modelCatalog.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ export interface IModelCatalogService {
1212
listModels(): Promise<readonly ModelCatalogItem[]>;
1313
listProviders(): Promise<readonly ProviderCatalogItem[]>;
1414
getProvider(providerId: string): Promise<ProviderCatalogItem>;
15+
removeProvider(providerId: string): Promise<void>;
1516
setDefaultModel(modelId: string): Promise<SetDefaultModelResponse>;
1617
}
1718

packages/agent-core/src/services/modelCatalog/modelCatalogService.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,14 @@ export class ModelCatalogService
5252
return this._provider(config, providerId, provider);
5353
}
5454

55+
async removeProvider(providerId: string): Promise<void> {
56+
const config = await this._readConfig();
57+
if (config.providers?.[providerId] === undefined) {
58+
throw new ProviderNotFoundError(providerId);
59+
}
60+
await this.core.rpc.removePythinkerProvider({ providerId });
61+
}
62+
5563
async setDefaultModel(modelId: string): Promise<SetDefaultModelResponse> {
5664
const config = await this._readConfig();
5765
const alias = config.models?.[modelId];
Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
import { describe, expect, it, vi } from 'vitest';
2+
3+
import type { CoreRPC, PythinkerConfig } from '../../src';
4+
import {
5+
ConfigService,
6+
type ICoreProcessService,
7+
type IEventService,
8+
} from '../../src/services';
9+
10+
describe('ConfigService', () => {
11+
it('converts underscores but preserves hyphens in nested record keys', async () => {
12+
const setPythinkerConfig = vi.fn(async (patch: unknown) => patch as PythinkerConfig);
13+
const core = {
14+
rpc: { setPythinkerConfig } as unknown as CoreRPC,
15+
} as ICoreProcessService;
16+
const eventService = { publish: vi.fn() } as unknown as IEventService;
17+
const service = new ConfigService(core, eventService);
18+
19+
await service.set({
20+
providers: {
21+
provider_with_underscore: { type: 'openai' },
22+
'provider-with-hyphen': { type: 'openai' },
23+
},
24+
models: {
25+
model_with_underscore: {
26+
provider: 'provider_with_underscore',
27+
model: 'model_with_underscore',
28+
max_context_size: 1000,
29+
},
30+
'model-with-hyphen': {
31+
provider: 'provider-with-hyphen',
32+
model: 'model-with-hyphen',
33+
max_context_size: 1000,
34+
},
35+
},
36+
});
37+
38+
expect(setPythinkerConfig).toHaveBeenCalledWith({
39+
providers: {
40+
providerWithUnderscore: { type: 'openai' },
41+
'provider-with-hyphen': { type: 'openai' },
42+
},
43+
models: {
44+
modelWithUnderscore: {
45+
provider: 'provider_with_underscore',
46+
model: 'model_with_underscore',
47+
maxContextSize: 1000,
48+
},
49+
'model-with-hyphen': {
50+
provider: 'provider-with-hyphen',
51+
model: 'model-with-hyphen',
52+
maxContextSize: 1000,
53+
},
54+
},
55+
});
56+
});
57+
});

packages/agent-core/test/services/model-catalog-service.test.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -252,4 +252,17 @@ describe('ModelCatalogService', () => {
252252
);
253253
});
254254

255+
it('removes an existing provider through core RPC', async () => {
256+
const configRef = { current: catalogConfig() };
257+
const { core, removeCalls } = makeCore(configRef);
258+
const svc = new ModelCatalogService(core);
259+
260+
await expect(svc.removeProvider('pythinker')).resolves.toBeUndefined();
261+
expect(removeCalls).toEqual(['pythinker']);
262+
await expect(svc.removeProvider('missing')).rejects.toBeInstanceOf(
263+
ProviderNotFoundError,
264+
);
265+
expect(removeCalls).toEqual(['pythinker']);
266+
});
267+
255268
});

packages/server/src/routes/modelCatalog.ts

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,14 @@ interface ModelCatalogRouteHost {
3030
reply: { send(payload: unknown): unknown },
3131
) => Promise<void> | void,
3232
): unknown;
33+
delete(
34+
path: string,
35+
options: { preHandler: unknown[]; schema?: Record<string, unknown> },
36+
handler: (
37+
req: { id: string; params: unknown },
38+
reply: { send(payload: unknown): unknown },
39+
) => Promise<void> | void,
40+
): unknown;
3341
}
3442

3543
const providerIdParamSchema = z.object({
@@ -40,6 +48,10 @@ const modelActionTailParamSchema = z.object({
4048
tail: z.string().min(1),
4149
});
4250

51+
const deleteProviderResponseSchema = z.object({
52+
deleted: z.literal(true),
53+
});
54+
4355
export function registerModelCatalogRoutes(
4456
app: ModelCatalogRouteHost,
4557
ix: IInstantiationService,
@@ -161,6 +173,38 @@ export function registerModelCatalogRoutes(
161173
getProviderRoute.options,
162174
getProviderRoute.handler as Parameters<ModelCatalogRouteHost['get']>[2],
163175
);
176+
177+
const deleteProviderRoute = defineRoute(
178+
{
179+
method: 'DELETE',
180+
path: '/providers/{provider_id}',
181+
params: providerIdParamSchema,
182+
success: { data: deleteProviderResponseSchema },
183+
errors: {
184+
[ErrorCode.VALIDATION_FAILED]: {},
185+
[ErrorCode.PROVIDER_NOT_FOUND]: {},
186+
},
187+
description: 'Delete a configured provider and its model aliases',
188+
tags: ['providers'],
189+
operationId: 'deleteProvider',
190+
},
191+
async (req, reply) => {
192+
try {
193+
const { provider_id } = req.params;
194+
await ix.invokeFunction((a) =>
195+
a.get(IModelCatalogService).removeProvider(provider_id),
196+
);
197+
reply.send(okEnvelope({ deleted: true as const }, req.id));
198+
} catch (error) {
199+
sendMappedError(reply, req.id, error);
200+
}
201+
},
202+
);
203+
app.delete(
204+
deleteProviderRoute.path,
205+
deleteProviderRoute.options,
206+
deleteProviderRoute.handler as Parameters<ModelCatalogRouteHost['delete']>[2],
207+
);
164208
}
165209

166210
function sendMappedError(

0 commit comments

Comments
 (0)