fix: address code review findings (critical + high + medium)

Critical:
- retryOnBusy: return lastErr after exhausted retries instead of nil

High:
- Remove debug console.log from use-paired-devices.ts

Medium:
- ChannelGeneralTab: sync state from instance prop via useEffect
- i18n: add MsgCannotRemoveLastWriter key + translations (en/vi/zh)
- use-channel-crud: refetch after delete instead of optimistic remove
This commit is contained in:
viettranx committed 2026-03-27 20:53:05 +07:00
1 parent 5f5b0c45bd
commit 8b08dec04c
9 files changed
+27 -15

No files matched your search

+6 -5
View File
@@ -14,20 +14,21 @@ import (
// retryOnBusy retries fn up to 3 times on SQLITE_BUSY errors with 500ms delay.
func retryOnBusy(fn func() error) error {
var lastErr error
for attempt := 0; attempt < 3; attempt++ {
err := fn()
if err == nil {
lastErr = fn()
if lastErr == nil {
return nil
}
if !strings.Contains(err.Error(), "SQLITE_BUSY") && !strings.Contains(err.Error(), "database is locked") {
return err
if !strings.Contains(lastErr.Error(), "SQLITE_BUSY") && !strings.Contains(lastErr.Error(), "database is locked") {
return lastErr
}
if attempt < 2 {
slog.Warn("bootstrap: retrying after SQLITE_BUSY", "attempt", attempt+1)
time.Sleep(500 * time.Millisecond)
}
}
return nil // unreachable, but satisfies compiler
return lastErr
}
// SeedToStore seeds embedded templates into agent_context_files (agent-level).
+1 -1
View File
@@ -437,7 +437,7 @@ func (h *ChannelInstancesHandler) handleRemoveWriter(w http.ResponseWriter, r *h
}
}
if allowCount <= 1 {
writeError(w, http.StatusConflict, protocol.ErrFailedPrecondition, "cannot remove the last file writer")
writeError(w, http.StatusConflict, protocol.ErrFailedPrecondition, i18n.T(locale, i18n.MsgCannotRemoveLastWriter))
return
}
if err := h.configPermStore.Revoke(r.Context(), agentID, groupID, "file_writer", userID); err != nil {
+1
View File
@@ -95,6 +95,7 @@ func init() {
// Channels
MsgCannotDeleteDefaultInst: "cannot delete default channel instance",
MsgCannotRemoveLastWriter: "cannot remove the last file writer",
// Skills
MsgSkillsUpdateNotSupported: "skills.update not supported for file-based skills",
+1
View File
@@ -95,6 +95,7 @@ func init() {
// Channels
MsgCannotDeleteDefaultInst: "không thể xóa phiên bản channel mặc định",
MsgCannotRemoveLastWriter: "không thể xóa người quản lý cuối cùng",
// Skills
MsgSkillsUpdateNotSupported: "skills.update không được hỗ trợ với skill dựa trên tệp",
+1
View File
@@ -95,6 +95,7 @@ func init() {
// Channels
MsgCannotDeleteDefaultInst: "无法删除默认Channel实例",
MsgCannotRemoveLastWriter: "无法移除最后一个文件管理员",
// Skills
MsgSkillsUpdateNotSupported: "基于文件的Skill不支持 skills.update",
+1
View File
@@ -96,6 +96,7 @@ const (
// --- Channels ---
MsgCannotDeleteDefaultInst = "error.cannot_delete_default_inst" // "cannot delete default channel instance"
MsgCannotRemoveLastWriter = "error.cannot_remove_last_writer" // "cannot remove the last file writer"
// --- Skills ---
MsgSkillsUpdateNotSupported = "error.skills_update_not_supported" // "skills.update not supported for file-based skills"
@@ -1,4 +1,4 @@
import { useState, useCallback } from 'react'
import { useState, useCallback, useEffect } from 'react'
import { useTranslation } from 'react-i18next'
import { Switch } from '../../common/Switch'
import { Combobox } from '../../common/Combobox'
@@ -31,6 +31,16 @@ export function ChannelGeneralTab({ instance, agents, onUpdate }: ChannelGeneral
)
const [policyValues, setPolicyValues] = useState<Record<string, unknown>>(initialPolicyValues)
// Sync policy values when instance updates (e.g. after save)
useEffect(() => {
const config = (instance.config ?? {}) as Record<string, unknown>
const keys = ESSENTIAL_CONFIG_KEYS[instance.channel_type] ?? ESSENTIAL_CONFIG_KEYS._default ?? []
setPolicyValues(Object.fromEntries(keys.filter((k) => config[k] !== undefined).map((k) => [k, config[k]])))
setDisplayName(instance.display_name ?? '')
setAgentId(instance.agent_id)
setEnabled(instance.enabled)
}, [instance])
const handlePolicyChange = useCallback((key: string, value: unknown) => {
setPolicyValues((prev) => ({ ...prev, [key]: value }))
}, [])
@@ -56,13 +56,13 @@ export function useChannelCrud() {
const deleteInstance = useCallback(async (id: string) => {
try {
await getApiClient().delete(`/v1/channels/instances/${id}`)
setInstances((prev) => prev.filter((i) => i.id !== id))
await fetchInstances()
toast.success('Channel deleted')
} catch (err) {
toast.error('Failed to delete channel', (err as Error).message)
throw err
}
}, [])
}, [fetchInstances])
const telegramExists = instances.some((i) => i.channel_type === 'telegram')
const discordExists = instances.some((i) => i.channel_type === 'discord')
@@ -10,14 +10,11 @@ export function usePairedDevices() {
const fetchDevices = useCallback(async () => {
try {
const ws = getWsClient()
const raw = await ws.call('device.pair.list')
console.log('[paired-devices] raw response:', raw)
const res = raw as { pending: PendingPairing[]; paired: PairedDevice[] }
console.log('[paired-devices] pending:', res.pending?.length, 'paired:', res.paired?.length)
const res = (await ws.call('device.pair.list')) as { pending: PendingPairing[]; paired: PairedDevice[] }
setPendingPairings(res.pending ?? [])
setPairedDevices(res.paired ?? [])
} catch (err) {
console.error('[paired-devices] error:', err)
} catch {
// ignore — gateway may not be connected
} finally {
setLoading(false)
}