From 2275d388a77866b57d347b0a38cadce81c2e2e26 Mon Sep 17 00:00:00 2001 From: yanweidong Date: Tue, 21 Jul 2026 16:44:16 +0800 Subject: [PATCH] fix(api): harden response contracts --- apps/web_v1/scripts/api-client.test.mjs | 138 ++++++++++++++++++ apps/web_v1/scripts/structure-check.mjs | 9 ++ apps/web_v1/src/api/client.ts | 30 +++- apps/web_v1/src/api/inbox.ts | 1 + apps/web_v1/src/api/projects.ts | 4 +- apps/web_v1/src/api/search.ts | 2 +- apps/web_v1/src/pages/login.tsx | 6 +- backend/internal/logic/projects/dto.go | 4 +- .../internal/logic/projects/service_test.go | 2 +- backend/internal/logic/projects/workspace.go | 14 +- .../internal/logic/projects/workspace_test.go | 7 + 11 files changed, 191 insertions(+), 26 deletions(-) create mode 100644 apps/web_v1/scripts/api-client.test.mjs diff --git a/apps/web_v1/scripts/api-client.test.mjs b/apps/web_v1/scripts/api-client.test.mjs new file mode 100644 index 0000000..226ac3e --- /dev/null +++ b/apps/web_v1/scripts/api-client.test.mjs @@ -0,0 +1,138 @@ +import assert from 'node:assert/strict' +import { readFile } from 'node:fs/promises' +import test from 'node:test' +import ts from 'typescript' + +const clientPath = new URL('../src/api/client.ts', import.meta.url) +const source = (await readFile(clientPath, 'utf8')).replaceAll('import.meta.env', '{}') +const output = ts.transpileModule(source, { + compilerOptions: { + module: ts.ModuleKind.ESNext, + target: ts.ScriptTarget.ES2023, + }, + fileName: 'client.ts', +}).outputText +const client = await import(`data:text/javascript;base64,${Buffer.from(output).toString('base64')}`) +const { ApiError, apiRequest, normalizeBaseUrl, setApiBaseUrl } = client + +const originalFetch = globalThis.fetch + +test.beforeEach(() => { + setApiBaseUrl('http://api.test') +}) + +test.after(() => { + globalThis.fetch = originalFetch +}) + +test('empty 200 response rejects with a stable invalid_response ApiError', async () => { + globalThis.fetch = async () => new Response('', { status: 200 }) + + await assert.rejects(apiRequest('/empty'), invalidResponseError) +}) + +test('non-JSON 200 response rejects with a stable invalid_response ApiError', async () => { + globalThis.fetch = async () => new Response('bad gateway', { status: 200 }) + + await assert.rejects(apiRequest('/html'), invalidResponseError) +}) + +test('interrupted successful response body rejects with a stable invalid_response ApiError', async () => { + globalThis.fetch = async () => ({ + ok: true, + status: 200, + text: async () => { + throw new Error('read interrupted') + }, + }) + + await assert.rejects(apiRequest('/interrupted'), invalidResponseError) +}) + +test('204 response returns undefined without reading a body', async () => { + globalThis.fetch = async () => new Response(null, { status: 204 }) + + assert.equal(await apiRequest('/no-content'), undefined) +}) + +test('explicit void response returns undefined for a non-204 response', async () => { + globalThis.fetch = async () => new Response('ignored', { status: 200 }) + + assert.equal(await apiRequest('/accepted', { responseType: 'void' }), undefined) +}) + +test('network failure rejects with network_error ApiError', async () => { + globalThis.fetch = async () => { + throw new Error('socket closed') + } + + await assert.rejects(apiRequest('/network'), (error) => { + assert.equal(error instanceof ApiError, true) + assert.equal(error.status, 0) + assert.equal(error.code, 'network_error') + return true + }) +}) + +test('standard error envelope preserves status, code, and Chinese message', async () => { + globalThis.fetch = async () => Response.json( + { error: { code: 'invalid_request', message: '请求参数无效' } }, + { status: 400 }, + ) + + await assert.rejects(apiRequest('/standard-error'), (error) => { + assert.equal(error instanceof ApiError, true) + assert.equal(error.status, 400) + assert.equal(error.code, 'invalid_request') + assert.equal(error.message, '请求参数无效') + return true + }) +}) + +test('GET without a body does not send Content-Type', async () => { + let requestInit + globalThis.fetch = async (_url, init) => { + requestInit = init + return Response.json({ ok: true }) + } + + await apiRequest('/read') + + assert.equal(requestInit.headers['Content-Type'], undefined) +}) + +test('JSON body sends application/json Content-Type', async () => { + let requestInit + globalThis.fetch = async (_url, init) => { + requestInit = init + return Response.json({ ok: true }) + } + + await apiRequest('/write', { method: 'POST', body: { title: '测试' } }) + + assert.equal(requestInit.headers['Content-Type'], 'application/json') +}) + +test('FormData body leaves Content-Type unset so fetch provides the boundary', async () => { + let requestInit + globalThis.fetch = async (_url, init) => { + requestInit = init + return Response.json({ ok: true }) + } + + await apiRequest('/upload', { method: 'POST', body: new FormData() }) + + assert.equal(requestInit.headers['Content-Type'], undefined) +}) + +test('base URL normalizer trims whitespace and all trailing slashes', () => { + assert.equal(normalizeBaseUrl(' https://host.example/// '), 'https://host.example') +}) + +function invalidResponseError(error) { + assert.equal(error instanceof ApiError, true) + assert.equal(error.status, 200) + assert.equal(error.code, 'invalid_response') + assert.equal(error.message, '服务器返回了无法识别的数据') + return true +} diff --git a/apps/web_v1/scripts/structure-check.mjs b/apps/web_v1/scripts/structure-check.mjs index 58e40db..12bb6ef 100644 --- a/apps/web_v1/scripts/structure-check.mjs +++ b/apps/web_v1/scripts/structure-check.mjs @@ -23,6 +23,7 @@ const requiredFiles = [ 'src/api/mappers.tsx', 'src/api/search.ts', 'src/api/inbox.ts', + 'scripts/api-client.test.mjs', ] const failures = requiredFiles.filter((file) => !existsSync(file)).map((file) => `missing ${file}`) @@ -77,9 +78,17 @@ for (const match of apiSource.matchAll(/export type (\w+DTO)\s*=\s*\{([\s\S]*?)\ if (/^\s*[A-Z]\w*\s*:/m.test(body)) failures.push(`${name} fields must use camelCase`) if (/^\s*(?:id|\w+Id)\s*:\s*number\b/im.test(body)) failures.push(`${name} identities must be strings`) } +if (/\|\s*undefined/.test(projectsSource)) failures.push('project DTOs must not use undefined fields') const searchSource = readFileSync('src/api/search.ts', 'utf8') if (!searchSource.includes("'/api/v1/search'")) failures.push('search API must use /api/v1/search') +for (const unsupportedType of ["'source'", "'inbox'", "'ai_session'"]) { + if (searchSource.includes(unsupportedType)) failures.push(`search DTO contains unsupported result type ${unsupportedType}`) +} + +const loginSource = readFileSync('src/pages/login.tsx', 'utf8') +if (!loginSource.includes("import { normalizeBaseUrl } from '../api/client'")) failures.push('login must share the API base URL normalizer') +if (loginSource.includes('function normalizeBaseUrl')) failures.push('login must not define a second base URL normalizer') const inboxSource = readFileSync('src/api/inbox.ts', 'utf8') for (const path of ['/api/v1/projects/', '/api/v1/inbox/']) { diff --git a/apps/web_v1/src/api/client.ts b/apps/web_v1/src/api/client.ts index b0ff3d5..e3c1af2 100644 --- a/apps/web_v1/src/api/client.ts +++ b/apps/web_v1/src/api/client.ts @@ -7,6 +7,7 @@ type RequestOptions = { method?: string body?: unknown token?: string + responseType?: 'json' | 'void' } type ErrorEnvelope = { @@ -54,7 +55,7 @@ export async function apiRequest(path: string, options: RequestOptions = {}): response = await fetch(`${configuredBaseUrl}${path}`, { method: options.method ?? 'GET', headers: { - ...(isFormData ? {} : { 'Content-Type': 'application/json' }), + ...(options.body !== undefined && !isFormData ? { 'Content-Type': 'application/json' } : {}), ...(options.token ? { Authorization: `Bearer ${options.token}` } : {}), }, body: requestBody as BodyInit | undefined, @@ -75,15 +76,28 @@ export async function apiRequest(path: string, options: RequestOptions = {}): throw new ApiError(response.status, fallbackErrorCode(response.status), fallbackErrorMessage(response.status)) } - if (response.status === 204) { + if (response.status === 204 || options.responseType === 'void') { return undefined as T } - const text = await response.text() - if (text.trim() === '') { - return undefined as T + let text: string + try { + text = await response.text() + } catch { + throw invalidResponse(response.status) } - return JSON.parse(text) as T + if (text.trim() === '') { + throw invalidResponse(response.status) + } + try { + return JSON.parse(text) as T + } catch { + throw invalidResponse(response.status) + } +} + +function invalidResponse(status: number) { + return new ApiError(status, 'invalid_response', '服务器返回了无法识别的数据') } function isErrorEnvelope(value: unknown): value is ErrorEnvelope { @@ -119,7 +133,7 @@ function fallbackErrorMessage(status: number) { return '请求失败，请稍后重试' } -function normalizeBaseUrl(value: string) { +export function normalizeBaseUrl(value: string) { const trimmed = value.trim() - return trimmed.endsWith('/') ? trimmed.slice(0, -1) : trimmed + return trimmed.replace(/\/+$/, '') } diff --git a/apps/web_v1/src/api/inbox.ts b/apps/web_v1/src/api/inbox.ts index 6421d32..508dbd6 100644 --- a/apps/web_v1/src/api/inbox.ts +++ b/apps/web_v1/src/api/inbox.ts @@ -47,5 +47,6 @@ export async function confirmInboxItem(session: ApiSession, inboxId: string, sug method: 'POST', token: session.token, body: { suggestions }, + responseType: 'void', }) } diff --git a/apps/web_v1/src/api/projects.ts b/apps/web_v1/src/api/projects.ts index 500cc10..c77fa83 100644 --- a/apps/web_v1/src/api/projects.ts +++ b/apps/web_v1/src/api/projects.ts @@ -26,8 +26,8 @@ export type WorkspaceChannelDTO = { type: string title: string icon: string - count: number | undefined - url: string | undefined + count: number + url: string sortOrder: number } diff --git a/apps/web_v1/src/api/search.ts b/apps/web_v1/src/api/search.ts index 2aa402f..7463eed 100644 --- a/apps/web_v1/src/api/search.ts +++ b/apps/web_v1/src/api/search.ts @@ -1,7 +1,7 @@ import { apiRequest, type ApiSession } from './client' export type SearchResultDTO = { - type: 'project' | 'task' | 'note' | 'source' | 'inbox' | 'ai_session' + type: 'project' | 'task' | 'note' id: string projectId: string title: string diff --git a/apps/web_v1/src/pages/login.tsx b/apps/web_v1/src/pages/login.tsx index e2f5d4e..d726524 100644 --- a/apps/web_v1/src/pages/login.tsx +++ b/apps/web_v1/src/pages/login.tsx @@ -10,6 +10,7 @@ import { IconSafe, IconUserGroup, } from '@arco-design/web-react/icon' +import { normalizeBaseUrl } from '../api/client' const { Title, Text, Paragraph } = Typography @@ -129,11 +130,6 @@ async function checkServerStatus( } } -function normalizeBaseUrl(value: string) { - const trimmed = value.trim() - return trimmed.endsWith('/') ? trimmed.slice(0, -1) : trimmed -} - function ValuePoint({ icon, title, desc }: { icon: ReactElement; title: string; desc: string }) { return (
diff --git a/backend/internal/logic/projects/dto.go b/backend/internal/logic/projects/dto.go index 8968e53..261dd2b 100644 --- a/backend/internal/logic/projects/dto.go +++ b/backend/internal/logic/projects/dto.go @@ -81,8 +81,8 @@ type WorkspaceChannelDTO struct { Type string `json:"type"` Title string `json:"title"` Icon string `json:"icon"` - Count *int64 `json:"count,omitempty"` - URL string `json:"url,omitempty"` + Count int64 `json:"count"` + URL string `json:"url"` SortOrder int `json:"sortOrder"` } diff --git a/backend/internal/logic/projects/service_test.go b/backend/internal/logic/projects/service_test.go index 11b07e0..02564ef 100644 --- a/backend/internal/logic/projects/service_test.go +++ b/backend/internal/logic/projects/service_test.go @@ -171,7 +171,7 @@ func TestWorkspaceMatchesFrontendContract(t *testing.T) { require.Len(t, workspace.Channels, 7) require.Equal(t, "overview", workspace.Channels[0].Type) require.Equal(t, "inbox", workspace.Channels[1].Type) - require.Equal(t, int64(1), *workspace.Channels[1].Count) + require.Equal(t, int64(1), workspace.Channels[1].Count) require.Equal(t, "custom_link", workspace.Channels[6].Type) require.Equal(t, "Roadmap Board", workspace.Channels[6].Title) require.Equal(t, "https://example.com/roadmap-a1", workspace.Channels[6].URL) diff --git a/backend/internal/logic/projects/workspace.go b/backend/internal/logic/projects/workspace.go index 828bb25..e35e915 100644 --- a/backend/internal/logic/projects/workspace.go +++ b/backend/internal/logic/projects/workspace.go @@ -120,12 +120,12 @@ func (s *Service) workspaceTags(projectID uint) ([]WorkspaceTagDTO, error) { func (s *Service) workspaceChannels(projectID uint, projectIdentity string, counts workspaceCounts) ([]WorkspaceChannelDTO, error) { total := counts.inbox + counts.tasks + counts.aiSessions + counts.notesSources + counts.cronPlans channels := []WorkspaceChannelDTO{ - {ID: projectIdentity + ":overview", ProjectID: projectIdentity, Type: "overview", Title: "Overview", Icon: "home", Count: &total, SortOrder: 1}, - {ID: projectIdentity + ":inbox", ProjectID: projectIdentity, Type: "inbox", Title: "Message Flow", Icon: "mail", Count: &counts.inbox, SortOrder: 2}, - {ID: projectIdentity + ":tasks", ProjectID: projectIdentity, Type: "tasks", Title: "Work Plan", Icon: "list", Count: &counts.tasks, SortOrder: 3}, - {ID: projectIdentity + ":ai", ProjectID: projectIdentity, Type: "ai_sessions", Title: "AI Sessions", Icon: "sparkles", Count: &counts.aiSessions, SortOrder: 4}, - {ID: projectIdentity + ":notes", ProjectID: projectIdentity, Type: "notes_sources", Title: "Notes & Sources", Icon: "file", Count: &counts.notesSources, SortOrder: 5}, - {ID: projectIdentity + ":cron", ProjectID: projectIdentity, Type: "cron", Title: "Cron Plans", Icon: "clock", Count: &counts.cronPlans, SortOrder: 6}, + {ID: projectIdentity + ":overview", ProjectID: projectIdentity, Type: "overview", Title: "Overview", Icon: "home", Count: total, URL: "", SortOrder: 1}, + {ID: projectIdentity + ":inbox", ProjectID: projectIdentity, Type: "inbox", Title: "Message Flow", Icon: "mail", Count: counts.inbox, URL: "", SortOrder: 2}, + {ID: projectIdentity + ":tasks", ProjectID: projectIdentity, Type: "tasks", Title: "Work Plan", Icon: "list", Count: counts.tasks, URL: "", SortOrder: 3}, + {ID: projectIdentity + ":ai", ProjectID: projectIdentity, Type: "ai_sessions", Title: "AI Sessions", Icon: "sparkles", Count: counts.aiSessions, URL: "", SortOrder: 4}, + {ID: projectIdentity + ":notes", ProjectID: projectIdentity, Type: "notes_sources", Title: "Notes & Sources", Icon: "file", Count: counts.notesSources, URL: "", SortOrder: 5}, + {ID: projectIdentity + ":cron", ProjectID: projectIdentity, Type: "cron", Title: "Cron Plans", Icon: "clock", Count: counts.cronPlans, URL: "", SortOrder: 6}, } var customChannels []models.SenlinAgentProjectChannel if err := models.DBService.Where("project_id = ?", projectID).Order("sort_order asc, id asc").Find(&customChannels).Error; err != nil { @@ -134,7 +134,7 @@ func (s *Service) workspaceChannels(projectID uint, projectIdentity string, coun for _, channel := range customChannels { channels = append(channels, WorkspaceChannelDTO{ ID: channel.Identity, ProjectID: projectIdentity, Type: "custom_link", Title: channel.Title, - Icon: channel.Icon, URL: channel.URL, SortOrder: channel.SortOrder, + Icon: channel.Icon, Count: 0, URL: channel.URL, SortOrder: channel.SortOrder, }) } return channels, nil diff --git a/backend/internal/logic/projects/workspace_test.go b/backend/internal/logic/projects/workspace_test.go index 2312183..d305790 100644 --- a/backend/internal/logic/projects/workspace_test.go +++ b/backend/internal/logic/projects/workspace_test.go @@ -94,6 +94,13 @@ func TestWorkspaceUsesIdentitiesUTCTimesAndProjectScopedTags(t *testing.T) { require.NotContains(t, string(payload), "duration") } +func TestWorkspaceChannelDTOSerializesRequiredCountAndURL(t *testing.T) { + payload, err := json.Marshal(WorkspaceChannelDTO{}) + require.NoError(t, err) + require.Contains(t, string(payload), `"count":0`) + require.Contains(t, string(payload), `"url":""`) +} + func TestWorkspaceDoneTaskDoesNotInventCompletedAt(t *testing.T) { database := newTestDB(t) service := NewService()