fix(mcp-proxy): isolate MCP reload panics

[wklken]

Summary

• Skip malformed OpenAPI parameters whose parameter ref, parameter value, or schema ref is missing, while preserving resolved schemas and $ref schemas.
• Add per-server panic isolation in MCP reload apply so one bad server does not abort later server processing.
• Record prefetch goroutine panics on the matching server load result so reload error counts stay accurate.

Verification

• go test -mod=mod ./pkg/infra/proxy ./pkg/mcp: passed
• go test -mod=vendor ./pkg/mcp: passed after pre-commit formatting
• make dep: passed
• make init: passed
• make lint: passed, 0 issues
• make test: passed, Ginkgo ran 14 suites

Review

• Used small-step-refactoring constraints: tests first, key reload change in its own commit.
• Ran refactor audit call-site scans for changed/new symbols; no dead or proxy-only production leftovers found.
• Ran code-review-and-quality pass; no blocking findings.

[wklken]

review again

[wklken]

review again

[wklken]

PR #2959 Code Review 汇总报告

由 codex (gpt-5.4) + Claude Opus 4.7 双模型 review，主 agent 汇总整理。

Report

Critical Issues

无。

High Issues

无。

Medium Issues

• errorCount / skippedCount 语义变化可能影响外部监控 [claude]
updateMCPServer 返回值由 bool 改为 (bool, error) 后，原来计入 skippedCount 的场景（server 不存在、AddMCPServerFromOpenAPISpec 失败）现在计入 errorCount。从语义上这是合理的（这些确实是错误而非 skip），但若下游有 metrics/dashboards/alerting 直接基于 errorCount 的绝对值或增量做阈值告警，reload 后可能出现"错误数上升"的伪告警。建议在 PR 描述或 release note 中显式说明 stats 语义变化，或对触发告警的看板做同步调整。

Low Issues

• debug.Stack() 在 recover 之后调用，堆栈信息会被 unwind 截断 [claude]
buildReloadPanicReport 中的 debug.Stack() 在 recover() 之后执行时，返回的堆栈信息通常不包含 panic 触发点的完整调用链，对定位真正 root cause 帮助有限。可考虑在 recover 时同步捕获 runtime.Stack 或在 panic 现场做二次 wrap。非阻塞，可作为可观测性优化。

• recoverPrefetchPanic 使全局 sentry.CurrentHub() 兜底逻辑变哑 [claude]
GoroutineWithRecovery 自身已带 sentry 兜底 recover，内层 recoverPrefetchPanic 先 recover 后外层再也拿不到 panic 值，仅依赖 sentry.ReportToSentry 一条通路。若两个 sentry 通路 tag/fingerprint 不同，需确认无遗漏。风险低。

• applyServerChange 内 svr != nil 分支冗余 [claude]
  调用方 applyServerChanges 已在 result == nil || result.server == nil 时 continue，内部再次做 if svr != nil 略显防御性。不是 bug，可维护性上的小重复。

• buildReloadPanicReport 返回值使用不对称 [claude]
  四个返回值全部被 reportReloadPanic 消费，可读性上 seam 略生硬。若后续没有第二个消费者，可考虑合并或缩减返回值。

• 测试对全局 cache 单例的耦合 [claude]
  新增测试通过 cacheimpls.SetMCPServerPromptCache 替换全局 cache 单例，若测试并行运行需确认 Ginkgo suite 调度模型不会引入交叉污染。建议在 suite 中集中一次性 stub 该 cache。

Dismissed Findings

• "失败的 server 仍然加入 activeMcpServer，会不会导致脏实例残留" [codex]：这是有意设计——保留旧实例比误删在线实例更符合"panic 隔离"的目标。[claude] 也验证了此行为一致且符合测试期望。不成立。
• "prefetchServerConfigs() 内外两层 recover 是否重复" [codex]：外层 util.GoroutineWithRecovery() 负责通用 goroutine 防护；内层 recoverPrefetchPanic() 负责把 panic 归因到具体 server。职责不同，不重复。[claude] 已核对 defer 执行顺序（<-sem → recover → wg.Done()），semaphore 一定在 recover 前释放。不成立。
• "openapiSchemaRefHasSchema() 放宽为接受 $ref schema 会不会引入回归" [codex]：当前实现仍优先消费已解析的 Value，只有在存在 Ref 时才保留 $ref schema；配套测试也覆盖了已解析 ref 的场景。不成立。
• "remove tool 与 load spec 顺序调换可能引入回归" [claude]：新顺序为先 load 新 spec 再删除不再存在的 tool。若 load 失败会直接 return，旧 tool 保留；若 load 成功再按 svr.GetToolNames() 清理。此改动为正向改进，消除了"load 失败时已提前破坏 tool 集合"的坑。不成立。

Agent Agreement Summary

Issue	Codex	Claude	Verdict
errorCount/skippedCount 语义变化影响监控	否	是	Medium — 真实问题，建议在 PR 描述或 release note 中说明
debug.Stack() 在 recover 后堆栈截断	否	是	Low — 真实问题，非阻塞优化
recoverPrefetchPanic 使 sentry 兜底变哑	否	是	Low — 真实问题，风险低
applyServerChange 内 svr != nil 冗余	否	是	Low — 真实问题，可维护性小重复
buildReloadPanicReport 返回值不对称	否	是	Low — 真实问题，可读性优化
测试对全局 cache 耦合	否	是	Low — 真实问题，非阻塞
两层 recover 是否重复	是	否	误报 — 职责不同
activeMcpServer 脏实例残留	是	否	误报 — 有意设计
$ref schema 回归风险	是	否	误报 — 已覆盖
remove tool / load spec 顺序回归	否	否	误报 — 正向改进

合并建议：merge after fixes

建议在 PR 描述或 release note 中补充说明 errorCount/skippedCount 语义变化，避免下游监控产生伪告警。其余 Low 问题均为非阻塞优化项，可在后续迭代中处理。

---

由 codex (gpt-5.4) + Claude Opus 4.7 双模型 review，主 agent 汇总 | [from openclaw-internal]

@wklken 您的 PR 已完成 review，请查看以上意见。