Skip to content

Commit add655c

Browse files
authored
audit review clean (#95)
### Issue for this PR Closes # ### Type of change - [ ] Bug fix - [ ] New feature - [ ] Refactor / code improvement - [ ] Documentation ### What does this PR do? Please provide a description of the issue, the changes you made to fix it, and why they work. It is expected that you understand why your changes work and if you do not understand why at least say as much so a maintainer knows how much to value the PR. **If you paste a large clearly AI generated description here your PR may be IGNORED or CLOSED!** ### How did you verify your code works? ### Screenshots / recordings _If this is a UI change, please include a screenshot or recording._ ### Checklist - [ ] I have tested my changes locally - [ ] I have not included unrelated changes in this PR _If you do not follow this template your PR will be automatically rejected._
1 parent b080201 commit add655c

48 files changed

Lines changed: 1455 additions & 434 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

packages/app/src/components/dialog-custom-provider-form.ts

Lines changed: 38 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,16 @@ export type CustomProviderConfig = {
3434

3535
const npmForProtocol = (kind: ProviderProtocol | undefined) => (kind === "anthropic" ? ANTHROPIC : OPENAI_COMPATIBLE)
3636

37+
export function isAmbiguousProtocolError(error: unknown) {
38+
const message =
39+
error instanceof Error
40+
? error.message
41+
: error && typeof error === "object" && "message" in error && typeof error.message === "string"
42+
? error.message
43+
: ""
44+
return message.includes("Provider protocol is ambiguous")
45+
}
46+
3747
// Leading host labels that are generic service prefixes and make a poor provider id, so we skip past
3848
// them to reach the brand label (api.deepseek.com -> "deepseek", not "api"). Kept deliberately small:
3949
// only unambiguous service prefixes, never anything that could be a brand.
@@ -183,11 +193,14 @@ export function validateCustomProvider(input: ValidateArgs) {
183193
// Zero-config path: when the user leaves id/name blank we derive them from the URL, so those
184194
// fields are no longer required. Derivation needs a usable URL — if the URL itself is invalid we
185195
// skip it and let urlError drive the failure instead of emitting a spurious id/name error.
186-
const derived = !urlError && (!typedID || !typedName) ? deriveProviderIdentity({
187-
baseURL,
188-
existingProviderIDs: input.existingProviderIDs,
189-
disabledProviders: input.disabledProviders,
190-
}) : undefined
196+
const derived =
197+
!urlError && (!typedID || !typedName)
198+
? deriveProviderIdentity({
199+
baseURL,
200+
existingProviderIDs: input.existingProviderIDs,
201+
disabledProviders: input.disabledProviders,
202+
})
203+
: undefined
191204
const providerID = typedID || derived?.providerID || ""
192205
const name = typedName || derived?.name || ""
193206

@@ -232,8 +245,7 @@ export function validateCustomProvider(input: ValidateArgs) {
232245
const contextError = ctx && !/^\d+$/.test(ctx) ? input.t("provider.custom.error.context") : undefined
233246
return { id: idError, name: nameError, context: contextError }
234247
})
235-
const modelsValid =
236-
(discoveryMode || models.every((m) => !m.id && !m.name)) && models.every((m) => !m.context)
248+
const modelsValid = (discoveryMode || models.every((m) => !m.id && !m.name)) && models.every((m) => !m.context)
237249
const modelConfig = Object.fromEntries(
238250
input.form.models.map((m) => {
239251
const ctx = m.context.trim()
@@ -309,10 +321,9 @@ export function validateCustomProvider(input: ValidateArgs) {
309321
}
310322
}
311323

312-
// Build the dialog form state for editing an existing custom provider. Fields (URL/key/headers/name)
313-
// come from the raw config entry; model rows are seeded from the RESOLVED provider so the user sees the
314-
// actual context/reasoning/temperature values (a discovery provider has no models in config — its
315-
// specs only exist post-resolve). Each row is pre-filled so edits override just those fields.
324+
// Discovery results are runtime state, not durable config. In discovery mode only configured model
325+
// overrides become editable rows; otherwise merely opening and saving the dialog would snapshot every
326+
// cached model into config and keep revoked/removed models alive indefinitely.
316327
export function formStateFromProvider(input: {
317328
config: ProviderConfig
318329
resolved: ResolvedProvider | undefined
@@ -321,20 +332,24 @@ export function formStateFromProvider(input: {
321332
const headers = config.options?.headers
322333
const headerRows =
323334
headers && typeof headers === "object" && Object.keys(headers).length
324-
? Object.entries(headers as Record<string, string>).map(([key, value]) =>
325-
headerRow2(String(key), String(value)),
326-
)
335+
? Object.entries(headers as Record<string, string>).map(([key, value]) => headerRow2(String(key), String(value)))
327336
: [headerRow()]
328337

329-
const resolvedModels = resolved?.models ?? {}
330-
const modelRows = Object.entries(resolvedModels).map(([id, m]) =>
331-
modelRow({
332-
id,
333-
name: m.name || id,
334-
context: m.limit?.context ? String(m.limit.context) : "",
335-
reasoning: !!m.capabilities?.reasoning,
336-
temperature: !!m.capabilities?.temperature,
337-
}),
338+
const modelRows = Object.entries(config.discovery ? (config.models ?? {}) : (resolved?.models ?? {})).map(
339+
([id, configured]) => {
340+
const model = resolved?.models[id]
341+
return modelRow({
342+
id,
343+
name: configured.name || model?.name || id,
344+
context: configured.limit?.context
345+
? String(configured.limit.context)
346+
: model?.limit?.context
347+
? String(model.limit.context)
348+
: "",
349+
reasoning: configured.reasoning ?? !!model?.capabilities?.reasoning,
350+
temperature: configured.temperature ?? !!model?.capabilities?.temperature,
351+
})
352+
},
338353
)
339354

340355
return {

packages/app/src/components/dialog-custom-provider.test.ts

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,20 @@ import { describe, expect, test } from "bun:test"
22
import {
33
deriveProviderIdentity,
44
formStateFromProvider,
5+
isAmbiguousProtocolError,
56
modelRow,
67
validateCustomProvider,
78
} from "./dialog-custom-provider-form"
89

910
const t = (key: string) => key
1011

12+
test("recognizes protocol ambiguity from SDK and Error response shapes", () => {
13+
const message = "Provider protocol is ambiguous; select OpenAI-compatible or Anthropic explicitly"
14+
expect(isAmbiguousProtocolError(new Error(message))).toBe(true)
15+
expect(isAmbiguousProtocolError({ message })).toBe(true)
16+
expect(isAmbiguousProtocolError(new Error("HTTP 404"))).toBe(false)
17+
})
18+
1119
describe("validateCustomProvider", () => {
1220
test("builds trimmed config payload", () => {
1321
const result = validateCustomProvider({
@@ -290,6 +298,7 @@ describe("formStateFromProvider", () => {
290298
npm: "@ai-sdk/openai-compatible",
291299
discovery: true,
292300
options: { baseURL: "https://relay.example.com", apiKey: "secret", headers: { "X-Env": "prod" } },
301+
models: { "gpt-4o": { name: "GPT-4o" } },
293302
},
294303
resolved: {
295304
id: "relay",
@@ -337,6 +346,50 @@ describe("formStateFromProvider", () => {
337346
temperature: true,
338347
})
339348
})
349+
350+
test("does not turn discovered runtime models into durable overrides", () => {
351+
const form = formStateFromProvider({
352+
config: {
353+
name: "My Relay",
354+
npm: "@ai-sdk/openai-compatible",
355+
discovery: true,
356+
options: { baseURL: "https://relay.example.com", apiKey: "secret" },
357+
},
358+
resolved: {
359+
id: "relay",
360+
name: "My Relay",
361+
source: "custom",
362+
env: [],
363+
options: {},
364+
models: {
365+
stale: {
366+
id: "stale",
367+
providerID: "relay",
368+
api: { id: "stale", url: "", npm: "@ai-sdk/openai-compatible" },
369+
name: "Stale",
370+
capabilities: {
371+
temperature: false,
372+
reasoning: false,
373+
attachment: false,
374+
toolcall: true,
375+
input: { text: true, audio: false, image: false, video: false, pdf: false },
376+
output: { text: true, audio: false, image: false, video: false, pdf: false },
377+
interleaved: false,
378+
},
379+
cost: { input: 0, output: 0, cache: { read: 0, write: 0 } },
380+
limit: { context: 0, output: 0 },
381+
status: "active",
382+
options: {},
383+
headers: {},
384+
release_date: "",
385+
},
386+
},
387+
},
388+
})
389+
390+
expect(form.models).toHaveLength(1)
391+
expect(form.models[0]).toMatchObject({ id: "", name: "" })
392+
})
340393
})
341394

342395
describe("deriveProviderIdentity", () => {

packages/app/src/components/dialog-custom-provider.tsx

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import {
1818
deriveProviderIdentity,
1919
formStateFromProvider,
2020
headerRow,
21+
isAmbiguousProtocolError,
2122
modelRow,
2223
validateCustomProvider,
2324
} from "./dialog-custom-provider-form"
@@ -244,8 +245,10 @@ export function DialogCustomProvider(props: Props) {
244245
const choice = protocolChoice()
245246
// Defensive client-side timeout: the backend already caps its /models fetch, but guard the
246247
// whole round-trip too so a stalled request can never leave the submit button hung. On
247-
// timeout (or any error) we fall through to manual/validation handling instead of blocking.
248-
const res = await Promise.race([
248+
// A timeout or ordinary discovery failure falls through to manual validation. Protocol
249+
// ambiguity is different: continuing would either guess the wrong SDK or fail with no useful
250+
// feedback, so require an explicit protocol choice.
251+
const discovery = await Promise.race([
249252
serverSDK.client.provider.models
250253
.discover(
251254
{
@@ -259,7 +262,18 @@ export function DialogCustomProvider(props: Props) {
259262
)
260263
.then((res) => res.data),
261264
new Promise<undefined>((resolve) => setTimeout(() => resolve(undefined), 20_000)),
262-
]).catch(() => undefined)
265+
]).then(
266+
(data) => ({ data, error: undefined }),
267+
(error: unknown) => ({ data: undefined, error }),
268+
)
269+
if (choice === "auto" && isAmbiguousProtocolError(discovery.error)) {
270+
showToast({
271+
title: language.t("common.requestFailed"),
272+
description: language.t("provider.custom.error.protocol.ambiguous"),
273+
})
274+
return
275+
}
276+
const res = discovery.data
263277
if (res?.kind) setDetectedProtocol(res.kind)
264278
const discovered = res?.models ?? []
265279
if (discovered.length > 0 && !hasManualModels) {

packages/app/src/components/provider-model-refresh.test.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,11 @@ describe("provider model refresh", () => {
55
test("allows official, discovery, and legacy imported providers", () => {
66
expect(canRefreshProviderModels("openai", undefined)).toBe(true)
77
expect(canRefreshProviderModels("custom", { discovery: true })).toBe(true)
8+
expect(
9+
canRefreshProviderModels("grouped", {
10+
groups: { anthropic: { npm: "@ai-sdk/anthropic", discovery: true } },
11+
}),
12+
).toBe(true)
813
expect(
914
canRefreshProviderModels("mistral", {
1015
options: { baseURL: "https://api.mistral.ai/v1" },

packages/app/src/components/provider-model-refresh.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import type { ProviderConfig } from "@deepagent-code/sdk/v2"
44
export function canRefreshProviderModels(providerID: string, config: ProviderConfig | undefined) {
55
if (isOfficialProvider(providerID)) return true
66
if (config?.discovery === true) return true
7+
if (Object.values(config?.groups ?? {}).some((group) => group.discovery === true)) return true
78
return (
89
config?.npm === undefined &&
910
typeof config?.options?.baseURL === "string" &&

packages/app/src/i18n/en.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -247,6 +247,8 @@ export const dict = {
247247
"provider.custom.error.required": "Required",
248248
"provider.custom.error.duplicate": "Duplicate",
249249
"provider.custom.error.context": "Enter a positive whole number, or leave blank",
250+
"provider.custom.error.protocol.ambiguous":
251+
"This endpoint supports multiple authentication styles. Select OpenAI-compatible or Anthropic explicitly.",
250252

251253
"provider.disconnect.toast.disconnected.title": "{{provider}} disconnected",
252254
"provider.disconnect.toast.disconnected.description": "{{provider}} models are no longer available.",

packages/app/src/i18n/zh.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -261,6 +261,8 @@ export const dict = {
261261
"provider.custom.error.required": "必填",
262262
"provider.custom.error.duplicate": "重复",
263263
"provider.custom.error.context": "请输入正整数,或留空",
264+
"provider.custom.error.protocol.ambiguous":
265+
"该端点同时支持多种认证方式,无法安全地自动判断协议。请明确选择 OpenAI-compatible 或 Anthropic。",
264266

265267
"provider.disconnect.toast.disconnected.title": "{{provider}} 已断开连接",
266268
"provider.disconnect.toast.disconnected.description": "{{provider}} 模型已不再可用。",

packages/app/src/i18n/zht.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -208,6 +208,8 @@ export const dict = {
208208
"provider.custom.error.required": "必填",
209209
"provider.custom.error.duplicate": "重複",
210210
"provider.custom.error.context": "請輸入正整數,或留空",
211+
"provider.custom.error.protocol.ambiguous":
212+
"此端點同時支援多種驗證方式,無法安全地自動判斷協定。請明確選擇 OpenAI-compatible 或 Anthropic。",
211213

212214
"provider.disconnect.toast.disconnected.title": "{{provider}} 已中斷連線",
213215
"provider.disconnect.toast.disconnected.description": "{{provider}} 模型已不再可用。",

packages/core/src/deepagent/orchestration.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -306,7 +306,7 @@ export const buildOrchestrationSection = (mode: AgentMode): string | null => {
306306
"抑制信号(命中则本体做,禁止过度编排):单文件;机制已明确;纯机械改动(改名/typo/格式);用户要求快速/直接。",
307307
"",
308308
"关键判定(reviewer 的 verdict、研究结果的合并)走结构化结果:调 `task` 时传 `output_schema`(reviewer→ReviewResult,researcher→ResearchResult),不要依赖散文解析。",
309-
`扇出规模自控(宽松上限,非硬性):单次编排子 agent 总数控制在 ${DEFAULT_MAX_FANOUT} 个以内,单轮并行不超过 ${DEFAULT_MAX_CONCURRENCY} 个;确有必要可分多轮,但不要一次性发起远超此规模的 task。本轮的具体扇出建议数见对话末尾 <deepagent-round-context>。`,
309+
`扇出规模自控(宽松上限,非硬性):单次编排子 agent 总数控制在 ${DEFAULT_MAX_FANOUT} 个以内,单轮并行不超过 ${DEFAULT_MAX_CONCURRENCY} 个;确有必要可分多轮,但不要一次性发起远超此规模的 task。本轮的具体扇出建议数由系统通过 <deepagent-round-context> 提供。`,
310310
]
311311
if (mode === "ultra") {
312312
lines.push("当前为 ultra:默认倾向编排并可多轮迭代。")

0 commit comments

Comments
 (0)