mirror of
https://gitcode.com/JianFeeeee/HomeAgent.git
synced 2026-11-06 17:27:31 +00:00
fix(agent): 插件崩溃熔断从不生效 —— 冷却锚点用了从未赋值的字段
## 缺陷（写上一个提交的判据时撞出来的）
recordCrash 标记 Unhealthy=true，但 isHealthy 的冷却判据是：
time.Since(r.LastReload) > reloadCooldown
而 LastReload **只在 markReloaded 里被赋值**（即"重载成功"之后）。
插件刚崩、从未重载时它是**零值**：
time.Since(零值) ≈ 2000 年 >> 30s
⇒ 判"冷却期早已过去" ⇒ 立刻把 Unhealthy 置回 false
实测（连打 3 次 recordCrash）：
crash #1 → needReload=false healthy=true
crash #2 → needReload=false healthy=true
crash #3 → needReload=true healthy=true ← 刚标完就自愈
## 影响
三处「插件不健康则跳过执行」全是**死分支**：
task.go:1065 stepToolBegin 中的跳过
task.go:1135 （上一提交刚补的那个分支的另一处同构）
toolbatch.go:155 并发批次中的跳过
即：一个正在崩溃循环的插件会被**持续调用**，每次调用再崩一次。
recordCrash 返回的 needReload=true 也没意义 —— 下一句 isHealthy 就说它健康了。
pendingReloads 用同一个零值字段 ⇒ autoReloadPlugins 每次都会尝试重载
**刚崩的**插件，很可能立刻再崩（崩溃循环）。
## 修法：冷却锚点改为 LastCrash
语义上「熔断」是"刚崩完先别用它，等 30s 再看" —— 观察窗口应锚在
**最近一次崩溃**，而不是"最近一次重载成功"。
isHealthy 锚点 LastReload → LastCrash
pendingReloads 锚点 LastReload → LastCrash
## 顺带删掉冗余
unhealthyPlugins() 零生产调用方（全仓 grep 只有定义本身）
record.FirstCrash 只写不读（写它的唯一目的是被下面这个判断用，
而那个判断早就改成只看 LastCrash 了）
## 判据（5 条，3 次变异全部实测变红）
TestCircuitBreakerActuallyOpens ★ 主判据
变异：锚点改回 LastReload ⇒ "★ 连续 3 次崩溃后插件仍被判健康"
TestCircuitBreakerWaitsForCooldownBeforeReload
变异：pendingReloads 锚点改回 LastReload ⇒
"★ 刚崩完就进了重载队列（应等冷却期）：[p]"
TestCircuitBreakerRecoversAfterCooldown
变异：删掉恢复分支 ⇒ "★ 冷却期过后仍未恢复健康 ——
插件会被永久停用（比不熔断更糟）"
TestCircuitBreakerUnknownPluginIsHealthy 从未崩过的插件不记账
TestCircuitBreakerWindowResets 窗口外的旧崩溃不该累加
## 判据本身的两个坑（已避开）
① 只断言 recordCrash 的返回值 —— 它本来就返回 true，测了等于没测。
必须断言 **isHealthy 的结论**，那才是被熔断逻辑消费的东西。
② 不测"冷却之后会恢复" —— 那样把代码改成恒 return false 也能过，
而那就成了永久禁用插件（比不熔断更糟）。
## 连带：deniedpair_test 的前置条件改回真实状态机
上一提交里，第二条判据只能**手工置 Unhealthy**（因为 recordCrash 标完
立刻被自愈撤销）。现在熔断真生效了，改回用 recordCrash 打满阈值 ——
判据依赖公开状态机而不是私有字段。
## 验证
go build ./... PASS
go vet ./... PASS
gofmt -l 干净
go test ./... -count=1 62 包全过，0 FAIL
干净检出（worktree） build + TestCircuitBreaker/TestDenied 全绿
This commit is contained in:
JianFeeeee
@ -36,7 +36,6 @@ import (
|
||||
"context"
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
agentAPI "github.com/JianFeeeee/HologramAgent/internal/agent/api"
|
||||
agentIO "github.com/JianFeeeee/HologramAgent/internal/agent/io"
|
||||
@ -180,14 +179,16 @@ func TestUnhealthyPluginSkipStillRecordsResult(t *testing.T) {
|
||||
// 这是**另一个独立缺陷**（插件崩溃熔断实际从不生效），不在本次修复范围
|
||||
// （本次只修 tool 配对）。这里直达状态，把判据聚焦在被测分支上；
|
||||
// 熔断那个缺陷应单独修并单独钉。
|
||||
// LastReload 必须设成"刚刚"，否则 isHealthy 的冷却判据
|
||||
//（time.Since(LastReload) > reloadCooldown）立刻判为已冷却 ⇒ 自愈。
|
||||
a.pluginHealth.mu.Lock()
|
||||
a.pluginHealth.records[pluginName] = &pluginHealthRecord{
|
||||
Unhealthy: true,
|
||||
LastReload: time.Now(), // 冷却期未过 ⇒ 熔断生效
|
||||
// ★ 走**真实状态机**，不直接改字段：
|
||||
// 现在 isHealthy 的冷却锚点是 LastCrash（见 plugin_health.go），
|
||||
// 连续 recordCrash 达到阈值就会进入真正的熔断态。
|
||||
//
|
||||
// 这条判据曾经只能手工置 Unhealthy —— 因为当时 recordCrash 标完
|
||||
// 立刻被 isHealthy 的自愈逻辑撤销（那是另一个缺陷，已单独修）。
|
||||
// 现在熔断真生效了，就用它自己的入口构造前置条件。
|
||||
for i := 0; i < maxPluginCrashes; i++ {
|
||||
a.pluginHealth.recordCrash(pluginName)
|
||||
}
|
||||
a.pluginHealth.mu.Unlock()
|
||||
if a.pluginHealth.isHealthy(pluginName) {
|
||||
t.Fatalf("★ 置 Unhealthy 后 %s 仍被判健康，判据前置条件不成立", pluginName)
|
||||
}
|
||||
|
||||
@ -19,7 +19,6 @@ type pluginHealthTracker struct {
|
||||
|
||||
type pluginHealthRecord struct {
|
||||
CrashCount int
|
||||
FirstCrash time.Time
|
||||
LastCrash time.Time
|
||||
Unhealthy bool
|
||||
LastReload time.Time
|
||||
@ -45,7 +44,6 @@ func (t *pluginHealthTracker) recordCrash(plugin string) bool {
|
||||
|
||||
if now.Sub(r.LastCrash) > crashWindow {
|
||||
r.CrashCount = 0
|
||||
r.FirstCrash = now
|
||||
}
|
||||
|
||||
r.CrashCount++
|
||||
@ -61,7 +59,24 @@ func (t *pluginHealthTracker) recordCrash(plugin string) bool {
|
||||
return false
|
||||
}
|
||||
|
||||
// isHealthy 检查插件是否健康；冷却期后自动恢复
|
||||
// isHealthy 检查插件是否健康。
|
||||
//
|
||||
// ## 冷却期必须从**最近一次崩溃**起算（2026-11-04 修的实测缺陷）
|
||||
//
|
||||
// 原实现是 `time.Since(r.LastReload) > reloadCooldown`，而 LastReload
|
||||
// **只在 markReloaded 里被赋值**（即只在"重载成功"之后）。于是：
|
||||
//
|
||||
// recordCrash 标记 Unhealthy=true
|
||||
// 但 LastReload 仍是零值
|
||||
// time.Since(零值) = 约 2000 年 ⇒ 远大于 30s
|
||||
// ⇒ 判定"冷却期早已过去" ⇒ 立刻置回 healthy
|
||||
//
|
||||
// 实测：连续 3 次 recordCrash 后 needReload=true、Unhealthy=true，
|
||||
// 紧接着 isHealthy 返回 **true**。即**崩溃熔断从不生效**，
|
||||
// task.go 与 toolbatch.go 里三处「插件不健康则跳过执行」全是死分支。
|
||||
//
|
||||
// 修法：冷却期以 LastCrash 为锚点 —— "刚崩完就先别用它，等 30s 再说"。
|
||||
// 这才是熔断的语义（原意是给插件一个恢复窗口，不是给重载计时的）。
|
||||
func (t *pluginHealthTracker) isHealthy(plugin string) bool {
|
||||
t.mu.Lock()
|
||||
defer t.mu.Unlock()
|
||||
@ -73,10 +88,11 @@ func (t *pluginHealthTracker) isHealthy(plugin string) bool {
|
||||
if !r.Unhealthy {
|
||||
return true
|
||||
}
|
||||
if time.Since(r.LastReload) > reloadCooldown {
|
||||
if time.Since(r.LastCrash) > reloadCooldown {
|
||||
r.Unhealthy = false
|
||||
r.CrashCount = 0
|
||||
log.Printf("[plugin] %s: cooldown passed, restored to healthy", plugin)
|
||||
log.Printf("[plugin] %s: cooldown passed (%v since last crash), restored to healthy",
|
||||
plugin, reloadCooldown)
|
||||
return true
|
||||
}
|
||||
return false
|
||||
@ -106,21 +122,10 @@ func (t *pluginHealthTracker) pendingReloads() []string {
|
||||
if !r.Unhealthy {
|
||||
continue
|
||||
}
|
||||
if now.Sub(r.LastReload) > reloadCooldown {
|
||||
result = append(result, name)
|
||||
}
|
||||
}
|
||||
return result
|
||||
}
|
||||
|
||||
// unhealthyPlugins 返回当前所有不健康的插件名
|
||||
func (t *pluginHealthTracker) unhealthyPlugins() []string {
|
||||
t.mu.Lock()
|
||||
defer t.mu.Unlock()
|
||||
|
||||
var result []string
|
||||
for name, r := range t.records {
|
||||
if r.Unhealthy {
|
||||
// 与 isHealthy 同一锚点（LastCrash）：刚崩完先等冷却，再尝试重载。
|
||||
// 原实现用 LastReload（零值 ⇒ 恒为真）⇒ 每次 autoReloadPlugins 都会
|
||||
// 尝试重载一个**刚崩的**插件，可能立刻再崩。
|
||||
if now.Sub(r.LastCrash) > reloadCooldown {
|
||||
result = append(result, name)
|
||||
}
|
||||
}
|
||||
|
||||
135
internal/agent/core/plugin_health_test.go
Normal file
135
internal/agent/core/plugin_health_test.go
Normal file
@ -0,0 +1,135 @@
|
||||
package core
|
||||
|
||||
// 判据：插件崩溃熔断**真的会熔断**。
|
||||
//
|
||||
// ## 守的是什么（2026-11-04 实测缺陷）
|
||||
//
|
||||
// 原实现：recordCrash 标记 Unhealthy=true，isHealthy 却用
|
||||
//
|
||||
// time.Since(r.LastReload) > reloadCooldown
|
||||
//
|
||||
// 而 LastReload **只在 markReloaded 里赋值**（重载成功后）。插件刚崩、
|
||||
// 从未重载时它是零值，time.Since(零值) ≈ 2000 年 ⇒ 远大于 30s
|
||||
// ⇒ 判"冷却期早已过去" ⇒ **立刻把 Unhealthy 置回 false**。
|
||||
//
|
||||
// 后果（这条比"少一个保护"严重）：
|
||||
//
|
||||
// agent 判定插件不健康 ⇒ 跳过执行、拉起重载 —— 这道保护**从不生效**
|
||||
// recordCrash 返回 needReload=true 也没用：下一句 isHealthy 就说它健康了
|
||||
//
|
||||
// 实测证据：连续 3 次 recordCrash 后 needReload=true、Unhealthy=true，
|
||||
// 紧接着 isHealthy 返回 true。
|
||||
//
|
||||
// ## 修法
|
||||
//
|
||||
// 冷却期锚点从 LastReload 改为 **LastCrash** —— "刚崩完先别用它，等 30s"。
|
||||
// 这才是熔断语义：给插件一个观察窗口，不是给重载计时。
|
||||
//
|
||||
// ## 这些判据最容易写废的地方
|
||||
//
|
||||
// ① 只断言 recordCrash 返回值 —— 它本来就返回 true，测了等于没测。
|
||||
// 必须断言 **isHealthy 的结论**，那才是被熔断逻辑消费的东西。
|
||||
// ② 不测"冷却之后会恢复" —— 那样把代码改成"永不恢复"也能过，
|
||||
// 而那就成了永久禁用插件。
|
||||
|
||||
import (
|
||||
"testing"
|
||||
"time"
|
||||
)
|
||||
|
||||
// TestCircuitBreakerActuallyOpens 是主判据。
|
||||
//
|
||||
// 变异：把 isHealthy 的锚点改回 r.LastReload ⇒ 本例红（立刻判健康）。
|
||||
func TestCircuitBreakerActuallyOpens(t *testing.T) {
|
||||
tr := newPluginHealthTracker()
|
||||
|
||||
for i := 0; i < maxPluginCrashes; i++ {
|
||||
tr.recordCrash("p")
|
||||
}
|
||||
|
||||
if tr.isHealthy("p") {
|
||||
t.Fatalf("★ 连续 %d 次崩溃后插件仍被判健康 ——\n"+
|
||||
"熔断从不生效，三处「插件不健康则跳过执行」全是死分支。\n"+
|
||||
"（原实现用 LastReload 作冷却锚点，零值让冷却显得早已过去）",
|
||||
maxPluginCrashes)
|
||||
}
|
||||
}
|
||||
|
||||
// TestCircuitBreakerWaitsForCooldownBeforeReload 钉住重载队列也要等冷却。
|
||||
//
|
||||
// 原实现同样用 LastReload（零值）⇒ 每次 autoReloadPlugins 都会尝试重载
|
||||
// 一个刚崩的插件，很可能立刻再崩（崩溃循环）。
|
||||
//
|
||||
// 变异：pendingReloads 锚点改回 LastReload ⇒ 本例红。
|
||||
func TestCircuitBreakerWaitsForCooldownBeforeReload(t *testing.T) {
|
||||
tr := newPluginHealthTracker()
|
||||
for i := 0; i < maxPluginCrashes; i++ {
|
||||
tr.recordCrash("p")
|
||||
}
|
||||
if got := tr.pendingReloads(); len(got) != 0 {
|
||||
t.Fatalf("★ 刚崩完就进了重载队列（应等冷却期）：%v", got)
|
||||
}
|
||||
}
|
||||
|
||||
// TestCircuitBreakerRecoversAfterCooldown 钉住"冷却之后要恢复"。
|
||||
//
|
||||
// 没有这条，把 isHealthy 改成恒返回 false 也能让主判据通过 ——
|
||||
// 那就成了永久禁用插件（比不熔断更糟：一个抖动过的插件永远被停用）。
|
||||
//
|
||||
// 变异：删掉 isHealthy 里的恢复分支（恒 return false）⇒ 本例红。
|
||||
func TestCircuitBreakerRecoversAfterCooldown(t *testing.T) {
|
||||
tr := newPluginHealthTracker()
|
||||
for i := 0; i < maxPluginCrashes; i++ {
|
||||
tr.recordCrash("p")
|
||||
}
|
||||
if tr.isHealthy("p") {
|
||||
t.Fatal("前置条件不成立：应当先处于熔断态")
|
||||
}
|
||||
|
||||
// 把最近崩溃时间往前拨，模拟"冷却期已过"。
|
||||
tr.mu.Lock()
|
||||
tr.records["p"].LastCrash = time.Now().Add(-2 * reloadCooldown)
|
||||
tr.mu.Unlock()
|
||||
|
||||
if !tr.isHealthy("p") {
|
||||
t.Fatal("★ 冷却期过后仍未恢复健康 —— 插件会被永久停用（比不熔断更糟）")
|
||||
}
|
||||
// 恢复后计数清零，下次崩溃重新从头计数。
|
||||
tr.mu.Lock()
|
||||
cnt := tr.records["p"].CrashCount
|
||||
tr.mu.Unlock()
|
||||
if cnt != 0 {
|
||||
t.Fatalf("★ 恢复后 CrashCount=%d，应清零（否则一次抖动就永久熔断）", cnt)
|
||||
}
|
||||
}
|
||||
|
||||
// TestCircuitBreakerUnknownPluginIsHealthy 钉住边界：从没崩过的插件不记账。
|
||||
func TestCircuitBreakerUnknownPluginIsHealthy(t *testing.T) {
|
||||
tr := newPluginHealthTracker()
|
||||
if !tr.isHealthy("never-seen") {
|
||||
t.Fatal("★ 从未崩溃的插件被判不健康")
|
||||
}
|
||||
if got := tr.pendingReloads(); len(got) != 0 {
|
||||
t.Fatalf("★ 空表却给出重载队列：%v", got)
|
||||
}
|
||||
}
|
||||
|
||||
// TestCircuitBreakerWindowResets 钉住崩溃窗口：稀疏的旧崩溃不该累加成熔断。
|
||||
func TestCircuitBreakerWindowResets(t *testing.T) {
|
||||
tr := newPluginHealthTracker()
|
||||
for i := 0; i < maxPluginCrashes-1; i++ {
|
||||
tr.recordCrash("p")
|
||||
}
|
||||
// 把上次崩溃拨到窗口之外：下一次崩溃应从 1 重新计数。
|
||||
tr.mu.Lock()
|
||||
tr.records["p"].LastCrash = time.Now().Add(-2 * crashWindow)
|
||||
tr.mu.Unlock()
|
||||
|
||||
need := tr.recordCrash("p")
|
||||
if need {
|
||||
t.Fatal("★ 窗口外的旧崩溃被累加进本次计数（应重新从 1 开始）")
|
||||
}
|
||||
if !tr.isHealthy("p") {
|
||||
t.Fatal("★ 窗口重置后不该处于熔断态")
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user