From 7a4815556823b34cf8a589301872ea0516f11e64 Mon Sep 17 00:00:00 2001 From: chenchen Date: Thu, 4 Jun 2026 14:24:25 +0800 Subject: [PATCH] fix(agent): apply review findings (backend data-integrity + frontend error UX) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two review agents (backend + frontend) audited the template-agent feature. Real findings fixed (verified the one false-positive — okOrThrow already reads the top-level success because the response interceptor returns response, not response.data): Backend: - buildAgentEnvFromBindings now filters status='active' so a revoked/disabled binding can't still have its KV secret resolved + injected. - same-type binding env collision (e.g. two git repos -> GIT_REPO_URL) now errors instead of silently overwriting. - refreshAgentStatus uses a field-level Updates (status/updated_at) instead of Save(full row) to avoid clobbering a concurrent stop/delete. - tests added: same-type collision rejected, inactive binding rejected. Frontend: - deploy/stop/delete now pass noBusinessError so the global interceptor stops double-toasting the raw English backend message; okOrThrow+onError give one friendly Chinese error. Extracted okOrThrow/noBusinessError into lib/api.ts. - resources page revoke + create + KV-secret-write now check success (okOrThrow) so a failed unbind / credential write no longer falsely reports success. - clipboard copy wrapped in try/catch (no false "copied" on failure). - resources group label shows Chinese resource-type names. Go + frontend builds/tests green. Co-Authored-By: Claude Opus 4.8 --- heicode/controller/agent_template_env.go | 16 ++++-- heicode/controller/agent_template_handlers.go | 12 +++-- heicode/controller/agent_template_test.go | 20 ++++++++ heicode/web/default/dist/index.html | 2 +- .../deploy-agent/deploy-agent-page.tsx | 50 +++++++++---------- .../src/features/resources/resources-page.tsx | 29 +++++++++-- heicode/web/default/src/lib/api.ts | 37 ++++++++++++++ 7 files changed, 128 insertions(+), 38 deletions(-) diff --git a/heicode/controller/agent_template_env.go b/heicode/controller/agent_template_env.go index 4f5a2788..61a67301 100644 --- a/heicode/controller/agent_template_env.go +++ b/heicode/controller/agent_template_env.go @@ -154,8 +154,10 @@ func buildAgentEnvFromBindings(userID int, bindingIDs []int) (map[string]string, for _, id := range bindingIDs { var binding model.ResourceBinding - if err := model.DB.Where("id = ? AND user_id = ?", id, userID).First(&binding).Error; err != nil { - return nil, fmt.Errorf("resource binding %d not found for user", id) + // Only active bindings — a revoked/disabled resource must not be resolved + // (its KV secret would otherwise still be read and injected). + if err := model.DB.Where("id = ? AND user_id = ? AND status = ?", id, userID, "active").First(&binding).Error; err != nil { + return nil, fmt.Errorf("resource binding %d not found or not active", id) } spec := builtinEnvSpec(binding.ResourceType, binding.Provider) @@ -196,9 +198,15 @@ func buildAgentEnvFromBindings(userID int, bindingIDs []int) (map[string]string, } if value, ok := src[f.Key]; ok { str := fmt.Sprintf("%v", value) - if strings.TrimSpace(str) != "" { - env[f.Env] = str + if strings.TrimSpace(str) == "" { + continue } + // Two bindings of the same type produce the same env name; refuse + // rather than silently let the later one overwrite the earlier. + if prev, dup := env[f.Env]; dup && prev != str { + return nil, fmt.Errorf("环境变量 %s 冲突：同类型资源（如两个 git/数据库）只能挂一个", f.Env) + } + env[f.Env] = str } } } diff --git a/heicode/controller/agent_template_handlers.go b/heicode/controller/agent_template_handlers.go index 746c3b15..fc078900 100644 --- a/heicode/controller/agent_template_handlers.go +++ b/heicode/controller/agent_template_handlers.go @@ -183,11 +183,17 @@ func refreshAgentStatus(c *gin.Context, row *model.AgentDeployment) { if err != nil || strings.TrimSpace(status) == "" || status == row.Status { return } + now := agentNow() + nowMs := time.Now().UnixMilli() row.Status = status - row.UpdatedAtText = agentNow() - row.UpdatedAtMs = time.Now().UnixMilli() + row.UpdatedAtText = now + row.UpdatedAtMs = nowMs + // Field-level update (not Save of the whole row) so a concurrent stop/delete + // or other column write isn't clobbered by a stale full-row save. if model.DB != nil { - _ = model.DB.Save(row).Error + _ = model.DB.Model(&model.AgentDeployment{}). + Where("deployment_id = ?", row.DeploymentID). + Updates(map[string]any{"status": status, "updated_at_text": now, "updated_at_ms": nowMs}).Error } } diff --git a/heicode/controller/agent_template_test.go b/heicode/controller/agent_template_test.go index 84fe3864..1ac54b31 100644 --- a/heicode/controller/agent_template_test.go +++ b/heicode/controller/agent_template_test.go @@ -79,6 +79,26 @@ func TestBuildAgentEnvFromBindings_GitNamesProviderAgnostic(t *testing.T) { } } +func TestBuildAgentEnvFromBindings_SameTypeCollisionRejected(t *testing.T) { + setupResourceControllerTestDB(t) + b1 := model.ResourceBinding{UserId: 7, Name: "g1", ResourceType: "git", Provider: "github", Status: "active", Metadata: `{"repo_url":"https://x/a"}`} + b2 := model.ResourceBinding{UserId: 7, Name: "g2", ResourceType: "git", Provider: "gitea", Status: "active", Metadata: `{"repo_url":"https://y/b"}`} + require.NoError(t, model.DB.Create(&b1).Error) + require.NoError(t, model.DB.Create(&b2).Error) + + _, err := buildAgentEnvFromBindings(7, []int{b1.Id, b2.Id}) // both set GIT_REPO_URL + require.Error(t, err) +} + +func TestBuildAgentEnvFromBindings_InactiveRejected(t *testing.T) { + setupResourceControllerTestDB(t) + b := model.ResourceBinding{UserId: 7, Name: "x", ResourceType: "blob", Provider: "blob", Status: "revoked", Metadata: `{"account":"a"}`} + require.NoError(t, model.DB.Create(&b).Error) + + _, err := buildAgentEnvFromBindings(7, []int{b.Id}) // not active + require.Error(t, err) +} + func TestBuildAgentEnvFromBindings_OwnershipEnforced(t *testing.T) { setupResourceControllerTestDB(t) b := model.ResourceBinding{UserId: 7, Name: "x", ResourceType: "blob", Provider: "blob", Metadata: `{}`} diff --git a/heicode/web/default/dist/index.html b/heicode/web/default/dist/index.html index d51fe906..d36de8fc 100644 --- a/heicode/web/default/dist/index.html +++ b/heicode/web/default/dist/index.html @@ -20,7 +20,7 @@ - +
diff --git a/heicode/web/default/src/features/deploy-agent/deploy-agent-page.tsx b/heicode/web/default/src/features/deploy-agent/deploy-agent-page.tsx index d9a6f4f1..fc9b2d34 100644 --- a/heicode/web/default/src/features/deploy-agent/deploy-agent-page.tsx +++ b/heicode/web/default/src/features/deploy-agent/deploy-agent-page.tsx @@ -13,7 +13,7 @@ import { Trash2, } from 'lucide-react' import { toast } from 'sonner' -import { api } from '@/lib/api' +import { api, noBusinessError, okOrThrow } from '@/lib/api' import { cn } from '@/lib/utils' import { Button } from '@/components/ui/button' import { Skeleton } from '@/components/ui/skeleton' @@ -87,23 +87,10 @@ async function listAgents(): Promise { return res.data?.data?.items ?? [] } -// The backend returns HTTP 200 with {success:false, error:{code,message}} for -// business failures (new-api convention), so axios resolves — we must check the -// envelope ourselves and throw, otherwise a failed call looks like a success. -type Envelope = { - data?: { success?: boolean; message?: string; error?: { code?: string; message?: string } } -} const FRIENDLY: Record = { RUNTIME_UNAVAILABLE: '云端服务暂时不可用，请稍后再试', RESOURCE_BINDING_INVALID: '所选资源无效，请重新选择', } -function okOrThrow(res: Envelope) { - const d = res?.data - if (d && d.success === false) { - const code = d.error?.code ?? '' - throw new Error(FRIENDLY[code] || d.error?.message || d.message || '操作失败') - } -} export function DeploySubAgentPage() { const qc = useQueryClient() @@ -129,11 +116,12 @@ export function DeploySubAgentPage() { const deploy = useMutation({ mutationFn: async () => { - const res = await api.post('/api/heicode/agents', { - template_id: selectedTemplate, - binding_ids: selectedBindings, - }) - okOrThrow(res) + const res = await api.post( + '/api/heicode/agents', + { template_id: selectedTemplate, binding_ids: selectedBindings }, + noBusinessError + ) + okOrThrow(res, FRIENDLY) }, onSuccess: () => { toast.success('Agent 已开始部署') @@ -330,7 +318,13 @@ function AgentRow({ const stop = useMutation({ mutationFn: async () => { - okOrThrow(await api.post(`/api/heicode/agents/${agent.agent_id}/stop`)) + okOrThrow( + await api.post( + `/api/heicode/agents/${agent.agent_id}/stop`, + undefined, + noBusinessError + ) + ) }, onSuccess: () => { toast.success('已停止') @@ -340,7 +334,9 @@ function AgentRow({ }) const del = useMutation({ mutationFn: async () => { - okOrThrow(await api.delete(`/api/heicode/agents/${agent.agent_id}`)) + okOrThrow( + await api.delete(`/api/heicode/agents/${agent.agent_id}`, noBusinessError) + ) }, onSuccess: () => { toast.success('已删除') @@ -349,10 +345,14 @@ function AgentRow({ onError: (e) => toast.error(e instanceof Error ? e.message : '操作失败'), }) - const copy = () => { + const copy = async () => { if (!agent.subdomain) return - void navigator.clipboard.writeText(agent.subdomain) - toast.success('已复制访问地址') + try { + await navigator.clipboard.writeText(agent.subdomain) + toast.success('已复制访问地址') + } catch { + toast.error('复制失败，请手动复制地址') + } } return ( @@ -374,7 +374,7 @@ function AgentRow({

-