Fix: fail closed after project reconcile errors by GOLDKUN · Pull Request #655 · chaitin/agent-compose

GOLDKUN · 2026-09-02T02:46:11Z

问题

Project Apply 会先持久化新 revision、agent 和 scheduler projection，再执行 scheduler reconcile。若后续 scheduler trigger 更新、启用或 controller refresh 失败，之前已成功启用的 project scheduler 可能继续运行，而 Apply 已返回失败。

影响

系统会同时存在“Apply 失败”与“部分新配置仍在运行”的状态：旧 scheduler 或已部分更新的 scheduler 可能执行不完整 revision，造成调度配置与持久化期望状态不一致。该问题属于状态机一致性缺陷。

修复内容

在 scheduler reconcile 失败后，对该 project 的所有已启用 scheduler 执行 fail-closed 补偿，将其禁用。
补偿后再次刷新 scheduler controller，确保内存中的调度状态与持久化禁用状态一致。
使用 errors.Join 保留原始 reconcile 错误及补偿/刷新错误，便于诊断。
增加成功补偿和多错误聚合的回归测试。

验证

go test ./internal/projects -run 'TestDisableProjectSchedulersAfterReconcileFailure' -count=1
git diff --check

说明：完整项目测试当前仍受 main 基线中 Protobuf 生成文件与 .proto 源码不同步影响，未在本 PR 中处理该独立问题。

monkeyscan · 2026-09-02T02:46:17Z

PR Title: Fix: fail closed after project reconcile errors

Commit: ad427fe

本次变更在 applyProject 的调度器协调失败路径上新增了 fail-closed 补偿机制：当 ReconcileSchedulers 返回错误时，调用新函数 disableProjectSchedulersAfterReconcileFailure，禁用该项目全部已启用调度器并刷新调度器控制器，避免调度器在未完整协调的新 revision 上继续运行；补偿错误通过 errors.Join 合并进原错误。新增测试文件用 mock store/validator 覆盖了辅助函数的禁用行为与错误合并。

整体评估：辅助函数本身的实现与错误处理正确，单元测试覆盖了主要正常/错误分支。但存在一个可用性回归风险——补偿触发条件过宽：applyProject 对 ReconcileSchedulers 的任何错误（包括第一个调度器写入前即失败的瞬态错误，以及最终控制器 Refresh 失败但 store 已完整落库的情况）都无条件禁用全部调度器，可能把瞬态故障放大为整个项目调度器停机。详见所提交的 finding。

GOLDKUN · 2026-09-02T04:54:17Z

已按最新评论修订 Apply 状态机：

ReconcileSchedulers 现在跟踪是否已经发生成功的持久化 mutation。
读取/校验阶段失败且未写入任何 scheduler 时，不触发 fail-closed 禁用。
最终 RefreshSchedulers 失败被标记为 refresh-only，不触发禁用，避免把瞬态内存刷新故障放大为项目停机。
仅在确认 scheduler store 已经部分写入且 reconcile 在后续阶段失败时执行 fail-closed 补偿。
增加“读取前失败不补偿”和“refresh-only 不补偿”的回归覆盖。

本地验证：go test ./internal/projects -run 'TestReconcileSchedulersFailureReportsCleanupAndPartialChanges|TestDisableProjectSchedulersAfterReconcileFailure' -count=1。

monkeyscan · 2026-09-02T04:54:22Z

PR Title: Fix: fail closed after project reconcile errors

Commit: 0691925

本次 PR 针对已确认的历史 finding（无条件 fail-closed 导致瞬态/未变更失败引发全项目调度器非必要停机）进行修复：将 controller.applyProject 中 reconcile 失败后的补偿从「任何错误都禁用项目全部调度器」收紧为「仅当 store 已发生实际变更且失败非 refresh 阶段时才 fail-closed」。

实现方式：ReconcileSchedulers 内部用 mutationTrackingSchedulerStore 包装 store，追踪三个写方法（UpsertProjectScheduler / SetProjectSchedulerEnabled / ReplaceSchedulerTriggers）的成功调用得到 storeChanged；新增 schedulerReconcileError 携带 err/storeChanged/refreshOnly 并在所有错误出口包装；新增 schedulerReconcileNeedsFailClosed 判定（errors.As 命中且 storeChanged && !refreshOnly）；controller.go 仅在该判定为真时调用 disableProjectSchedulersAfterReconcileFailure。

正确性分析：changed=false 表示失败发生在任何成功写入前，DB 保持变更前一致状态，不 fail-closed 安全；refreshOnly 仅在 ReconcileSchedulers 末尾触发，此时 store 已完整一致落库，仅控制器内存未同步，跳过 fail-closed 避免放大瞬态刷新失败。单测覆盖 read-before-mutation / trigger replacement / scheduler enable / controller refresh 四类失败下判定函数的布尔结果。

整体评估：修复方向正确、逻辑自洽，核心分支有单测覆盖，未发现高置信度新正确性缺陷。报告了一个低严重度测试覆盖缺口：多调度器「部分完整落库后再失败」的跨条目部分应用场景，以及 controller 集成层对条件化 fail-closed 是否被正确调用，均缺少回归测试。另建议团队确认 refresh 失败时 fail-open（DB 已落库新状态而运行态仍旧）这一安全姿态转变是否符合预期。

monkeyscan · 2026-09-02T04:59:13Z

-		{name: "scheduler enable", enableErr: enableErr, wantCleanup: true},
+		{name: "read before mutation", getErr: errors.New("read failed")},
+		{name: "trigger replacement", replaceErr: replaceErr, wantCleanup: true, wantFailClosed: true},
+		{name: "scheduler enable", enableErr: enableErr, wantCleanup: true, wantFailClosed: true},


条件化 fail-closed 缺少多调度器部分写入与控制器集成层测试，存在静默回归风险

本 PR 将 fail-closed 从『任意 reconcile 错误』收紧为『store 已变更且非 refresh 阶段』，这是修复历史 finding 的核心行为变化，直接影响部分写入后的数据一致性补偿是否触发。但当前新增测试仅覆盖单个调度器内的失败分支（read-before-mutation 验证 storeChanged=false 不 fail-closed，trigger replacement/scheduler enable 验证 storeChanged=true 时 fail-closed，controller refresh 验证 refreshOnly 不 fail-closed）。缺少两类覆盖：(1) 跨多个调度器场景——spec 中第一个调度器已完整落库（changed=true）后第二个调度器在读取/替换阶段失败，必须仍判定为需要 fail-closed；这是该判定存在的主要目的，一旦 mutationTrackingSchedulerStore 的 changed 追踪被误改成仅反映最后一次操作，或未来接口新增写方法而忘记包装，现有单调度器测试无法发现；(2) controller.applyProject 集成层没有任何测试验证 needsFailClosed=false 时 disableProjectSchedulersAfterReconcileFailure 不再被调用、needsFailClosed=true 时仍会被调用，该行为若被后续重构破坏只能靠人工回归发现。

Problem code:

Changed code at internal/projects/reconcile_scheduler_test.go:132-134

Recommendation:
建议补充：(1) 在 TestReconcileSchedulersFailureReportsCleanupAndPartialChanges 增加多调度器用例：请求包含两个 scheduler，第一个正常完成 upsert+replace+enable（使 trackedStore.changed=true），第二个在读取或替换阶段注入错误，断言 schedulerReconcileNeedsFailClosed(err)==true；(2) 增加 controller 层集成测试，mock store 与 refresh，分别验证 reconcile 失败且 needsFailClosed=false（如 read-before-mutation、refresh 失败）时 disableProjectSchedulersAfterReconcileFailure 不被调用、调度器保持原状态，以及 needsFailClosed=true 时该补偿被调用且所有 enabled 调度器被禁用。

GOLDKUN · 2026-09-02T17:32:15Z

已修复 mutation-unknown 状态：

mutation tracker 现在区分“已成功写入”和“写入结果不确定”。
UpsertProjectScheduler、SetProjectSchedulerEnabled、ReplaceSchedulerTriggers 返回错误时，均标记 mutation uncertain；因为底层写操作可能已提交，但写后读取/提交结果返回错误。
Apply 在非 refresh-only reconcile 错误中，只要存在成功 mutation 或 uncertain mutation，就继续 fail-closed 补偿。
最终 RefreshSchedulers 失败仍保持 refresh-only，不会因控制器刷新瞬态故障禁用全部调度器。
新增“scheduler 写入返回错误但可能已经落库”的回归场景。

本地验证：go test ./internal/projects -run 'TestReconcileSchedulersFailureReportsCleanupAndPartialChanges|TestDisableProjectSchedulersAfterReconcileFailure' -count=1 和 git diff --check 通过。

monkeyscan · 2026-09-02T17:32:23Z

PR Title: Fix: fail closed after project reconcile errors

Commit: 806efe6

本次改动围绕调度器（scheduler）对账（reconcile）的 fail-closed 语义做加固：在 mutationTrackingSchedulerStore 中新增 uncertain 标志——当 UpsertProjectScheduler / SetProjectSchedulerEnabled / ReplaceSchedulerTriggers 任一写操作返回错误时置位；schedulerReconcileError 增加 storeMutationUncertain 字段并在所有错误出口透传；schedulerReconcileNeedsFailClosed 改为 storeChanged || storeMutationUncertain 时返回 true（refreshOnly 仍排除）。核心意图是：此前只有"确有成功写入"才 fail-closed，而"首次写入即失败、结果未知（如网络超时但实际已落库）"不会 fail-closed，可能让调用方误以为存储未变化而推进状态，导致半迁移漂移；改动后这类歧义写入也按 fail-closed 处理。测试补充了 upsertErr 注入与一条 "ambiguous scheduler write" 用例，验证无任何成功写入时写失败即 fail-closed。

整体评估：改动方向正确、范围聚焦，错误传播路径完整（两处变更出口及 refreshOnly 出口均已带上新字段），新增测试覆盖了主要的"首次 upsert 失败即 fail-closed"新行为，且 refreshOnly/仅读失败等旧语义保持不变。唯一顾虑是"uncertain"把所有写错误一律视为结果未知，可能与"确定性未落盘"错误（如 ErrNotFound、校验类错误）混淆，导致良性场景下过度 fail-closed（已作为低严重度发现提交）。

GOLDKUN · 2026-09-02T18:14:10Z

已处理最新 Review 建议：

SetProjectSchedulerEnabled(..., false) 返回 ErrNotFound 时，按“被删除的 Scheduler 已达到目标终态”处理，不再标记 mutation uncertain。
disableRemovedSchedulers 对已并发删除的 Scheduler 继续收敛并产生 Removed change，不再把良性竞态升级为全项目 fail-closed 停机。
增加多 Scheduler 回归测试：第一个 Scheduler 已完成写入，第二个 Scheduler 读取失败时，仍保留 partial mutation 的 fail-closed 信号。
保留真正写入结果不确定时的 fail-closed 语义。

本地相关测试与 git diff --check 通过。

monkeyscan · 2026-09-02T18:14:17Z

PR Title: Fix: fail closed after project reconcile errors

Commit: 0ece67d

本次改动修复 reconcile 流程中"禁用已移除调度器时遇到 ErrNotFound 被误判为 fail-closed"的问题（对应历史发现 f298986e）。

改动要点：

internal/projects/reconcile.go 的 mutationTrackingSchedulerStore.SetProjectSchedulerEnabled 中，当错误为 ErrNotFound 且本次操作为禁用（enabled=false）时不再置 uncertain=true，避免良性竞态（并发删除/已不存在）被当作"存储状态不确定"而 fail-closed。
disableRemovedSchedulers 中，SetProjectSchedulerEnabled(..., false) 返回 ErrNotFound 时视为已收敛：记录一条 ChangeActionRemoved 变更并 continue，而非返回错误；非 NotFound 错误仍按原路径返回错误并触发 fail-closed。
测试桩 schedulerReconcileStateStore 新增 setEnabledErr、getErrOnCall/getCalls 字段，并新增两个测试：一个验证跨调度器部分变更后发生读错误仍保留 fail-closed 信号；另一个验证移除已不存在的调度器被视为收敛（无错误）。

总体评估：修复方向正确，且与既有约定（ErrNotFound 表示确定性"不存在"）一致。disableRemovedSchedulers 的 ErrNotFound 特判与包装器的新条件相互配合，非 NotFound 写错误仍会置 uncertain 并 fail-closed，逻辑自洽。历史发现中描述的 disable-removed 良性竞态场景已被解决。主要遗留风险是：新增的错误分类边界（NotFound=收敛 vs 其他错误=fail-closed）只覆盖了 NotFound 一侧的测试，非 NotFound 的禁用移除错误路径缺少回归测试。

GOLDKUN · 2026-09-02T19:08:09Z

已修复 CI Lint 失败：

将 !(errors.Is(err, domain.ErrNotFound) && !enabled) 改写为等价的 De Morgan 表达式 !errors.Is(err, domain.ErrNotFound) || enabled。
保持原有语义：禁用不存在的 Scheduler 仍不标记 mutation uncertain，其他写错误继续 fail-closed。

提交：fix: satisfy scheduler reconcile lint。
相关 Apply reconcile 测试与 git diff --check 已通过，PR 已触发新一轮 CI。

monkeyscan · 2026-09-02T19:08:16Z

PR Title: Fix: fail closed after project reconcile errors

Commit: ec678da

改动位于 internal/projects/reconcile.go 的 mutationTrackingSchedulerStore.SetProjectSchedulerEnabled 包装器中，将错误处理分支条件从 !(errors.Is(err, domain.ErrNotFound) && !enabled) 重写为 !errors.Is(err, domain.ErrNotFound) || enabled。二者经德摩根定律（!(A && !enabled) ≡ !A || enabled）在四种真值组合下完全等价：仅当「enabled=false 且返回 ErrNotFound」（即禁用一个已不存在的调度器）时不置 uncertain，其余错误情形均置 uncertain。errors.Is 为纯函数、enabled 为局部布尔参数，不存在副作用差异或短路求值差异，故本改动为纯语义保持的化简重构，可读性更好，未引入任何行为回归。文件中 disableRemovedSchedulers 相关历史发现（非 NotFound 错误路径缺少回归测试）涉及本 diff 之外的代码，未被本次改动触及或暴露。总体评估：无正确性、安全或可靠性问题，无需提交新的评审发现。

GOLDKUN · 2026-09-02T19:33:11Z

已补充非 ErrNotFound 删除失败的回归测试：

SetProjectSchedulerEnabled(..., false) 返回普通写入错误时，reconcile 必须返回原始错误；
schedulerReconcileNeedsFailClosed(err) 必须为 true；
与 ErrNotFound 的已收敛分支形成明确对照。

提交：test: cover removed scheduler write failures。

monkeyscan · 2026-09-02T19:33:17Z

PR Title: Fix: fail closed after project reconcile errors

Commit: b099525

本次变更为 internal/projects/reconcile_scheduler_test.go 新增单元测试 TestReconcileSchedulersFailsClosedOnRemovedSchedulerWriteError，验证从 spec 移除的启用中调度器在禁用写入阶段返回非 ErrNotFound 错误时，ReconcileSchedulers 返回该错误且 schedulerReconcileNeedsFailClosed 为 true。经与 reconcile.go 实现核对：该错误经 mutationTrackingSchedulerStore 使 storeMutationUncertain=true，并随 schedulerReconcileError 透传，断言沿 Unwrap 链成立；与 ErrNotFound 视为已收敛的相邻测试互补，正确覆盖 disableRemovedSchedulers 路径的 fail-closed 分支。测试本身正确，无新增缺陷。仍需关注的是历史 finding 残留的控制器集成层测试缺口：applyProject 仅当 schedulerReconcileNeedsFailClosed(err) 为真时才调用 disableProjectSchedulersAfterReconcileFailure 执行补偿，该条件边界无集成测试保护，存在静默回归风险（详见所提交 finding）。

monkeyscan · 2026-09-02T19:37:00Z

+	if err == nil || !errors.Is(err, writeErr) || !schedulerReconcileNeedsFailClosed(err) {
+		t.Fatalf("removed scheduler write error = %v, want fail-closed error", err)
+	}
+}


控制器集成层仍缺 fail-closed 补偿触发测试：applyProject 仅当 needsFailClosed 为真时才调用 disableProjectSchedulersAfterReconcileFailure

本 diff 新增的 TestReconcileSchedulersFailsClosedOnRemovedSchedulerWriteError 是单元级测试，仅验证 ReconcileSchedulers 在移除调度器禁用写入失败（非 ErrNotFound）时返回 needsFailClosed=true 的错误，未触达 controller 集成边界。controller.go 的 applyProject 在 ReconcileSchedulers 返回错误后，仅当 schedulerReconcileNeedsFailClosed(err) 为真时才调用 disableProjectSchedulersAfterReconcileFailure 对悬空的已移除调度器做补偿关闭；该条件分支决定失败后补偿是否执行，直接关系数据一致性。但 internal/projects 下未见 applyProject/controller 集成测试覆盖两个方向：needsFailClosed=false 时不调用补偿、needsFailClosed=true 时必定调用补偿。若后续重构误删/反转该条件或改动 needsFailClosed 语义，仅靠现有单元测试无法发现，属于静默回归风险。此为历史 finding（多调度器部分写入与控制器集成层测试缺口）中仍残留的部分：多调度器部分写入场景已由 TestReconcileSchedulersPreservesPartialMutationSignalAcrossSchedulers 覆盖，本次新增也补充了移除路径的写错误单元覆盖，唯独控制器集成层仍无测试。

Problem code:

Changed code at internal/projects/reconcile_scheduler_test.go:140-153

Recommendation:
在 Controller.applyProject 层补充集成测试：构造一个令 ReconcileSchedulers 返回 needsFailClosed=true 的 store（例如移除调度器禁用写入返回非 ErrNotFound 错误，或调度器启用写入返回不确定错误），断言 ApplyProject 返回 Applied=false 且 disableProjectSchedulersAfterReconcileFailure 被调用；再构造 needsFailClosed=false 的失败（如 refresh 阶段失败或读前失败），断言补偿函数不被调用。通过注入 SetProjectSchedulerEnabled/UpsertProjectScheduler/ListProjectSchedulers 错误的伪 store 驱动两条分支即可。

winterfx

LGTM

GOLDKUN · 2026-09-03T11:59:37Z

当前唯一失败检查 Refresh Protobuf compatibility approval 已定位为仓库级 workflow 权限问题，不是本 PR 代码失败。

失败日志：

GH_TOKEN permissions: Actions: read
POST /actions/runs/<run_id>/rerun
Resource not accessible by integration (HTTP 403)

该 workflow 尝试重新运行 Protobuf compatibility，但自身 GITHUB_TOKEN 只有 actions: read。PR 的 Protobuf compatibility、Lint、Go tests、Coverage 和所有平台构建均已成功。

我尝试用 Fork 作者凭证手动 rerun，同样被 GitHub 拒绝：

Must have admin rights to Repository

需要仓库管理员执行其一：

在默认分支对应 workflow 的 permissions 中授予 actions: write；或
由管理员手动重新运行 Protobuf compatibility。

由于 pull_request_review 工作流从默认分支加载，在本 PR 分支修改 workflow 无法修复当前审批触发的 403。

fix: fail closed after project reconcile errors

ad427fe

monkeyscan Bot reviewed Sep 2, 2026

View reviewed changes

Comment thread internal/projects/controller.go

fix: limit project reconcile fail-closed compensation

0691925

monkeyscan Bot reviewed Sep 2, 2026

View reviewed changes

fix: fail closed on uncertain scheduler writes

806efe6

monkeyscan Bot reviewed Sep 2, 2026

View reviewed changes

Comment thread internal/projects/reconcile.go

fix: classify converged scheduler removals

0ece67d

monkeyscan Bot reviewed Sep 2, 2026

View reviewed changes

Comment thread internal/projects/reconcile.go

fix: satisfy scheduler reconcile lint

ec678da

test: cover removed scheduler write failures

b099525

monkeyscan Bot reviewed Sep 2, 2026

View reviewed changes

winterfx approved these changes Sep 3, 2026

View reviewed changes

Uh oh!

Conversation

GOLDKUN commented Sep 2, 2026

问题

影响

修复内容

验证

Uh oh!

monkeyscan Bot commented Sep 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

GOLDKUN commented Sep 2, 2026

Uh oh!

monkeyscan Bot commented Sep 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

monkeyscan Bot Sep 2, 2026

Choose a reason for hiding this comment

条件化 fail-closed 缺少多调度器部分写入与控制器集成层测试，存在静默回归风险

Uh oh!

GOLDKUN commented Sep 2, 2026

Uh oh!

monkeyscan Bot commented Sep 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

GOLDKUN commented Sep 2, 2026

Uh oh!

monkeyscan Bot commented Sep 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

GOLDKUN commented Sep 2, 2026

Uh oh!

monkeyscan Bot commented Sep 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

GOLDKUN commented Sep 2, 2026

Uh oh!

monkeyscan Bot commented Sep 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

monkeyscan Bot Sep 2, 2026

Choose a reason for hiding this comment

控制器集成层仍缺 fail-closed 补偿触发测试：applyProject 仅当 needsFailClosed 为真时才调用 disableProjectSchedulersAfterReconcileFailure

Uh oh!

winterfx left a comment

Choose a reason for hiding this comment

Uh oh!

GOLDKUN commented Sep 3, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

monkeyscan Bot commented Sep 2, 2026 •

edited

Loading

monkeyscan Bot commented Sep 2, 2026 •

edited

Loading

monkeyscan Bot commented Sep 2, 2026 •

edited

Loading

monkeyscan Bot commented Sep 2, 2026 •

edited

Loading

monkeyscan Bot commented Sep 2, 2026 •

edited

Loading

monkeyscan Bot commented Sep 2, 2026 •

edited

Loading