fix(ux): dev mode lockout warning + webchat 401 guidance + font preload trim#561
Conversation
hotplex-ai
left a comment
There was a problem hiding this comment.
Code Review — hotplex-ai
Verdict: APPROVE | P0:0 P1:0 P2:1 P3:0
P2 Findings
[P2] throwIfAuthError 仅覆盖 listSessions,其他 fetch 调用缺少一致 401 处理
webchat/lib/api/sessions.ts — createSession、deleteSession、getSessionHistory 同样使用 AUTH_HEADER 调用 gateway API,但未添加 throwIfAuthError。若用户 API key 不匹配,这些路径仍会抛出通用 Error 而非含操作指引的 AuthError。
建议:后续 PR 统一覆盖,或提取 authenticatedFetch wrapper(参考 admin-client.ts:67 的集中 401 处理模式)。
Summary
Go 侧 RemoveKey 变更正确:锁内计算 empty 布尔值、锁外调 slog.Warn,无竞态。devModeLocked 语义匹配(永久锁 + 无 key = 不可用状态)。字体 preload 移除已验证无副作用。AuthError class 和 catch 块 instanceof 排序正确。
aaa2960 to
cc21531
Compare
hotplex-ai
left a comment
There was a problem hiding this comment.
Code Review — hotplex-ai
Verdict: APPROVE | P0:0 P1:0 P2:1 P3:1
[P2] removeSession catch 未处理 AuthError,401 时用户无操作指引
webchat/lib/hooks/useSessions.ts:198 — deleteSession 已加 throwIfAuthError,但 removeSession 的 catch 块仍用 logger.error 静默处理。用户删除会话时遇到 401 得不到 "Check HOTPLEX_WEBCHAT_API_KEY" 提示,与 refreshSessions/createNewSession 的处理不一致。建议在 catch 中检测 AuthError 并调用 setError(e.message)。
[P3] SKILL.md 版本计数 "13 处" 应为 12
.agents/skills/hotplex-release/SKILL.md:534 — 实际枚举列表为 12 项(Go 2 + WebChat 1 + SDK 4 + 文档 3 + 基础设施 1 + Changelog 1 = 12),计数应修正为 "共 12 处"。
整体评价:三项改动(RemoveKey 日志、401 指引、字体瘦身)方向正确,Go 端锁外日志、TS 端 AuthError 类设计干净。P2 为一致性遗漏,建议合入前修复但不阻塞。
hotplex-ai
left a comment
There was a problem hiding this comment.
APPROVE — P2/P3 only, no blocking issues. See detailed review above.
49fa66a to
6e0cbce
Compare
hotplex-ai
left a comment
There was a problem hiding this comment.
Code Review — hotplex-ai
Verdict: APPROVE | P0:0 P1:0 P2:1 P3:0
Findings
[P2] Observability regression in removeSession catch block (webchat/lib/hooks/useSessions.ts:196-200)
Removing logger.error(...) and replacing with only setError(...) loses structured console logging for session deletion failures. These serve different purposes:
logger.error→ persistent console record for debuggingsetError→ transient UI state, lost on next state change
Suggest keeping both:
} catch (e) {
logger.error('Sessions', 'Failed to delete session', { error: String(e) });
setError(e instanceof AuthError ? e.message : (e instanceof Error ? e.message : 'Failed to delete session'));
refreshSessions();
}No P0/P1 issues found. Auth.go lock handling is correct (snapshot under lock, warn outside). throwIfAuthError pattern is sound. Font preload trim verified safe.
Observability regression from PR #561 — removing structured logging lost persistent console records for session deletion failures.
…ad trim (#560) - security: RemoveKey logs warning when all keys removed but dev mode locked - webchat: AuthError class with actionable 401 message (API key config guidance) - webchat: remove unused font weights (100/200/300) from preload, eliminating 9 console warnings
…etSessionHistory (#560)
….md count - P2: removeSession now surfaces AuthError (e.g. "Check HOTPLEX_WEBCHAT_API_KEY") via setError, consistent with refreshSessions/createNewSession - P3: hotplex-release SKILL.md version count corrected 13→12
Observability regression from PR #561 — removing structured logging lost persistent console records for session deletion failures.
bf89283 to
06b647f
Compare
hotplex-ai
left a comment
There was a problem hiding this comment.
Code Review — hotplex-ai
Verdict: APPROVE | P0:0 P1:0 P2:2 P3:2
P2:2
- WebSocket
UNAUTHORIZED错误仍使用通用消息 (webchat/lib/adapters/hotplex-runtime-adapter.ts:592-593) — REST API 已通过AuthError提供可操作的 401 指引,但 WebSocket 连接路径(主要数据通道)仍显示 "check your API key or connection settings",与 PR 目标(可操作的 401 指引)不一致。建议 follow-up 统一。 throwIfAuthError消息遗漏重启提示 (webchat/lib/api/sessions.ts:79) — 消息说 "remove gateway API keys to use dev mode",但仅删除 key 不会解锁 dev mode(devModeLocked永久锁)。需重启 gateway 才生效。建议改为 "remove gateway API keys and restart gateway"。
P3:2
- bare
slog.Warn无结构化字段 (internal/security/auth.go:149) — CLAUDE.md 要求 slog JSON handler + 结构化字段。同包apikey_resolver.go已有先例,且此为低频 warning 路径,可接受但属于 convention deviation。 - CI
FORCE_JAVASCRIPT_ACTIONS_TO_NODE24为 workflow 级全局 env (.github/workflows/ci.yml:33) — 影响所有 JS action,包括golangci-lint-action@v9.2.0。实际风险低(主流 action 已兼容 Node 24),但建议在 Node 24 成为默认后移除此 env var。
综合评价
三项 UX 改动实现质量良好:auth.go 锁内外分工正确(snapshot boolean + post-unlock IO),Webchat AuthError 覆盖全部 4 个 REST API call site,font preload 移除已验证无残留引用。无 P0/P1,可合并。
- P2: Unify WebSocket UNAUTHORIZED error message with actionable 401 guidance - P2: Add "restart gateway" hint to throwIfAuthError dev mode message - P3: Add structured field to slog.Warn in auth.go RemoveKey - P3: Add TODO comment for FORCE_JAVASCRIPT_ACTIONS_TO_NODE24 removal
hotplex-ai
left a comment
There was a problem hiding this comment.
Code Review — hotplex-ai
Verdict: ✅ APPROVE | P0:0 P1:0 P2:2 P3:0
Findings
[P2] removeSession catch 未处理 AuthError,与其他方法不一致 (useSessions.ts:197-199) — conf 92
refreshSessions(行 138)和 createNewSession(行 181)都优先检测 AuthError 并通过 setError() 展示给用户。但 removeSession 的 catch 块仅调用 logger.error + refreshSessions(),未调用 setError()。如果用户在删除 session 时遇到 401(如 key 被中途删除),UI 不会显示错误信息。建议对齐:
setError(e instanceof AuthError ? e.message : (e instanceof Error ? e.message : 'Failed to delete session'))[P2] 401 错误消息泄露内部实现细节 (sessions.ts:77) — conf 88
throwIfAuthError 构造的用户可见错误包含 HOTPLEX_WEBCHAT_API_KEY(内部环境变量名)、.env.local(配置文件路径)和 dev mode(内部运维概念)。这些信息有助于攻击者侦察。建议使用通用消息(如 Authentication failed — check your API key configuration),将详细指引放在开发者文档或服务端日志中。注意:这是 UX 可操作性 vs 安全性的权衡。
Reviewed by 4 parallel agents (Bug/Security/Concurrency, Compliance/DRY, History/Compat, SOLID/Perf). 15 raw findings → 2 after filtering.
| } catch (e) { | ||
| logger.error('Sessions', 'Failed to delete session', { error: String(e) }); | ||
| // Revert optimistic remove on failure | ||
| setError(e instanceof AuthError ? e.message : (e instanceof Error ? e.message : 'Failed to delete session')); |
There was a problem hiding this comment.
[P2] 此 catch 块未检测 AuthError。refreshSessions 和 createNewSession 都优先用 setError(e instanceof AuthError ? e.message : ...) 展示错误,但此处只 logger.error + refreshSessions()。建议对齐处理。
| function throwIfAuthError(prefix: string, status: number): never | void { | ||
| if (status === 401) { | ||
| throw new AuthError( | ||
| `${prefix} failed: 401 — API key mismatch. Check HOTPLEX_WEBCHAT_API_KEY in .env.local or remove gateway API keys and restart gateway to use dev mode.` |
There was a problem hiding this comment.
[P2] 401 消息暴露 HOTPLEX_WEBCHAT_API_KEY、.env.local、dev mode 等内部信息。考虑通用化:Authentication failed: 401. Check your API key configuration or contact your administrator.
Replace env var names and config paths with generic guidance to prevent information leakage. P2 from PR #561 review round 2.
Summary
Closes #560 — 三项新用户体验优化,确保 dev mode 闭环畅通。
P1: RemoveKey 清空后日志警告
internal/security/auth.go— 删除最后一个 API key 后,如果devModeLocked=true,打印slog.Warn提示用户重启 gateway 恢复匿名访问P2: Webchat 401 可操作提示
webchat/lib/api/sessions.ts— 新增AuthError类和throwIfAuthError,401 时抛出含操作指引的结构化错误webchat/lib/hooks/useSessions.ts—refreshSessions和createNewSession的 catch 块优先检测AuthError,显示可操作的配置提示P3: 字体 preload 瘦身
webchat/app/layout.tsx— 移除 Inter/Outfit/JetBrains Mono 的 100/200/300 字重(未使用),消除 9 条控制台 preload 警告Test plan
go test ./internal/security/通过cd webchat && npx next build通过