Skip to content

fix: simple application error dispaly #7779

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
wangdan-fit2cloud merged 1 commit into from
Nov 9, 2026
Merged

fix: simple application error dispaly #7779

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion ui/src/views/application-detail/setting/settings.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,14 @@ export type LongTermSettingData = Pick<

export type SelectedKnowledge = Partial<KnowledgeItem> & { id: string }

/** 后端空对象表示未配置 MCP，非空对象转为编辑器使用的 JSON 文本。 */
function normalizeMcpServers(value: unknown): string {
if (value == null) return ''
if (typeof value === 'string') return value
if (typeof value === 'object' && !Array.isArray(value) && Object.keys(value).length === 0) return ''
return JSON.stringify(value, null, 2) ?? ''
}

export function createSettings(detail?: ApplicationDetail): ApplicationSettings {
const defaults: ApplicationSettings = {
name: '',
Expand Down Expand Up @@ -120,7 +128,7 @@ export function createSettings(detail?: ApplicationDetail): ApplicationSettings
desc: detail.desc ?? '',
problem_optimization_prompt: detail.problem_optimization_prompt || DEFAULT_PROBLEM_OPTIMIZATION_PROMPT,
mcp_source: detail.mcp_source === 'custom' ? 'custom' : 'referencing',
mcp_servers: detail.mcp_servers ?? '',
mcp_servers: normalizeMcpServers(detail.mcp_servers),
mcp_output_enable: detail.mcp_output_enable ?? false,
model_id: detail.model ?? detail.model_id ?? '',
stt_model_id: detail.stt_model ?? detail.stt_model_id ?? '',

@shaohuzhang1 shaohuzhang1 Nov 9, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review of the diff

The change is small and mostly reasonable, but there are a few things worth flagging.

1. Unused null fallback on JSON.stringify

return JSON.stringify(value, null, 2) ?? ''

JSON.stringify only returns undefined when the input is undefined, a function, or a symbol — all of which are already excluded by the earlier typeof checks (or by value == null). Realistically this ?? '' is dead code. It's harmless but can be simplified:

return JSON.stringify(value, null, 2)

2. Behavior change for string values isn't validated

if (typeof value === 'string') return value

If the backend previously returned a string like "{}" (common serialized empty object), you'd now render "{}" in the editor instead of an empty string, whereas the object branch treats {} as unconfigured. This asymmetry could cause UX inconsistency depending on whether the backend sends a stringified or structured value. Consider normalizing string content too, e.g.:

if (typeof value === 'string') {
  const trimmed = value.trim()
  if (!trimmed || trimmed === '{}' || trimmed === 'null') return ''
  return value
}

Only do this if you're confident about the backend contract.

3. Non-plain objects

typeof value === 'object' && !Array.isArray(value) still passes for Date, Map, etc. Not an issue if the backend always sends plain JSON, but if you want to be strict:

Object.getPrototypeOf(value) === Object.prototype

4. Comment language mismatch

The new JSDoc is in Chinese while surrounding types (LongTermSettingData, SelectedKnowledge, ApplicationSettings) are English naming. Minor style inconsistency — align with repo conventions.

5. No type guard input schema

detail.mcp_servers appears typed as possibly-string already (given ?? '' before). If so, normalizeMcpServers is widening the actual runtime handling beyond what the type promises. That's fine defensively, but update the type of mcp_servers if it can actually be unknown | object at runtime — otherwise downstream code that assumes a string may still break.

6. Missing test for null

The function handles null/undefined via value == null, but there's no explicit test shown. Please ensure unit tests cover: null, undefined, '', plain string, {}, {a:1}, array, and primitive number/bool (which will hit JSON.stringify(123) → "123" — is that desired?). A primitive like true becoming "true" may be an unexpected pass-through — consider rejecting primitives other than string explicitly.

Suggested consolidated version

function normalizeMcpServers(value: unknown): string {
  if (value == null) return ''
  if (typeof value === 'string') return value.trim() === '{}' ? '' : value
  if (typeof value !== 'object' || Array.isArray(value)) return ''
  if (Object.keys(value as object).length === 0) return ''
  return JSON.stringify(value, null, 2) || ''
}

This drops the dead ?? '', ignores non-object primitives, and keeps the empty-object rule. Verify behavior with your existing consumers before adopting.

Expand Down
Loading