From 4b293a8eac57a1c1d78e365d92ac72024677bdf4 Mon Sep 17 00:00:00 2001 From: bboonnee Date: Mon, 3 Aug 2026 01:01:50 +0800 Subject: [PATCH] U-041 fix: rollback path A now removes the candidate's worktree checkout --- docs/development-plan.md | 1 + docs/sdd/unit_matrix.json | 2 +- supervisor/deploy_rollback.py | 24 ++++++++++++++++++++---- supervisor/self_hosting_e2e.py | 1 + supervisor/test_deploy_rollback.py | 26 ++++++++++++++++++++++++++ 5 files changed, 49 insertions(+), 5 deletions(-) diff --git a/docs/development-plan.md b/docs/development-plan.md index 4f15c7c..235870d 100644 --- a/docs/development-plan.md +++ b/docs/development-plan.md @@ -394,6 +394,7 @@ PH2-S1/S2/S3/S4 之间除标注的依赖外相互独立，可并行推进。 - 依赖：U-038, U-040 - Done：两条回滚路径各自跑通一次；人为制造"旧版本也起不来"的场景能正确终态到 `blocked_pending_human` 而不是无限重试或静默失败；`git revert` 冲突场景（`supervisor-auto-rollback-design.md` §10 开放问题 3）有明确处理，不能是本 Unit 唯一遗留的未定义行为 - **实现补充（2026-08-02）——本 Unit 是 PH5 关键路径上明确标注返工风险最高的一环，按计划要求真的用人为制造的失败场景验证了兜底路径，没有假设"大概率不会发生"就跳过**：新增 `supervisor/deploy_rollback.py`。**设计上两处判断，都是按已有约定处理，不是新拍的**：① 没有一个统一的 `rollback(conn, deployment_id)` 总入口——`deployment` 表存了 `candidate_instance_dir`（U-037）但从没存过任何 PID 列，`deploy_stage.stage_deployment()`/`deploy_cutover.cutover()` 早就要求调用方直接把 pid 传进来（这些 pid 只在同一个 Deploy Supervisor 进程的生命周期内才有意义），`rollback_path_a`/`rollback_path_b` 延续同一约定，没有另造一套。② 03-C 的 `output_contract` 只写了 `{status: 'rolled_back'|'health_check_failed'|'rollback_failed'}`，从没出现过 `blocked_pending_human`，但 04-P 状态转移表的 `error_state` 列和 03-C 的 `errors` 描述都明确写"转 `blocked_pending_human`"——读作"`rollback_failed` 是错误名/返回值，真正持久化的 `deployment.status` 是 `blocked_pending_human`"，和 `deploy_stage.CheckoutFailedError`/`deploy_cutover.CutoverFailedError` 已经在用的同一个模式完全一致，不是新发明。`git revert` 冲突（本 Unit 自己点名要求处理的场景，design doc §10 开放问题 3）不开新分支，直接并入两条路径共用的"回滚本身失败"兜底，`git revert --abort` 收尾避免仓库卡在冲突中间态。 + - **真实 bug（2026-08-03，U-042 第七次真实自举压测第一次真正走到 health_check 环节）**：这是六次前置 bug 修完之后第一次真正让 `stage_deployment→run_health_check→rollback_path_a` 这条链路对着真实候选进程走一遍——候选健康检查真实失败，`rollback_path_a` 真实终止了候选进程、真实 `git revert` 了合并提交，全部正确；但候选自己的 `git worktree add --detach` 检出目录（`candidate_instance_dir`，U-037 就是为了追踪它才补的字段）从始至终没人清理，永久留在磁盘上、还继续注册在 `git worktree list` 里——一个确认失败的候选之后不会再被用到，路径 A 理应清掉它，和成功 cutover 后候选变成 `previous_instance_dir` 需要保留到监控窗口结束是两回事，不能用同一套"先留着"逻辑。修复：`rollback_path_a` 新增可选参数 `candidate_instance_dir`（默认 `None`，不影响任何未使用真实 worktree 检出的既有测试），提供时在终止进程之后、`git revert` 之前先 `git worktree remove --force`；`self_hosting_e2e.py` 把 `stage_deployment()` 自己返回值里早就有的 `candidate_instance_dir` 传过去。补了一个用真实 `git worktree add --detach` 检出目录的回归测试，验证目录和 `git worktree list` 注册都被清理干净。清理过程中顺带发现并杀掉了三个更早的 `test_deploy_rollback.py` 真实进程测试遗留下来、backing 目录早已不存在的孤儿进程。 - **真实验证，四个场景全部对着真实子进程/真实 git 仓库跑过，没有 mock 关键路径**：路径 A happy（真进程 `sleep 300` 模拟候选、真的删一次性 DB 文件、真实 `git revert` 落地、真的写一条 `bug_record`）；路径 A 失败（构造一个真实会冲突的 git 历史——v2 是待回滚的自我修改，v3 是之后touch 同一行的后续提交——`git revert` 真的冲突，验证 `blocked_pending_human` + 仓库被 `--abort` 清理干净、不留冲突中间态）；路径 B happy（真实 `git worktree add` 出一份"上一个已知良好版本"的检出目录，真实拉起一个"坏 Live"进程，真实停掉+复活旧版本+真实三项复核+真实 revert，复活的进程最后真的在验证用的端口上对外提供服务，不只是返回值对）；**路径 B 失败**（本 Unit 最该较真的场景）：`previous_instance_dir` 指向一个真实存在但完全没有 Python 包内容的空目录，真实拉起会真的失败/绑不上端口，验证最终落到 `blocked_pending_human` 而不是卡死重试或悄悄放过，且确认这次真的没有执行 `git revert`（提交内容原样保留，因为回滚在到达那一步之前就已经失败）。 - 覆盖：03-C `c_deploy_supervisor_rollback`；`git revert` 冲突场景（design doc §10 开放问题 3，已有明确处理） diff --git a/docs/sdd/unit_matrix.json b/docs/sdd/unit_matrix.json index 659d6dd..dce7fb8 100644 --- a/docs/sdd/unit_matrix.json +++ b/docs/sdd/unit_matrix.json @@ -58,7 +58,7 @@ {"id": "U-038", "phase": "PH5", "sub_phase": "PH5-S2", "name": "c_deploy_supervisor_health_check（见 development-plan.md 2026-08-02 实现补充）", "classification": "Risk Gate", "depends_on": ["U-037"], "coverage": {"model": ["deploy_health_signal"], "view": [], "controller": ["c_deploy_supervisor_health_check"], "process": [], "requirement": []}, "done": "process_alive/http_health/canary_task 三项检查全过 -> health_check_passed；任一失败 -> health_check_failed 且 Live 未受影响（supervisor/deploy_health_check.py，真实 HTTP+真实 project/task 创建验证过）；backend_server.py 新增 GET /api/health（真查一次 SELECT 1，不只是进程存活）"}, {"id": "U-039", "phase": "PH5", "sub_phase": "PH5-S2", "name": "c_deploy_supervisor_cutover（见 development-plan.md 2026-08-02 实现补充）", "classification": "Risk Gate", "depends_on": ["U-038"], "coverage": {"model": ["deployment"], "view": [], "controller": ["c_deploy_supervisor_cutover"], "process": [], "requirement": []}, "done": "健康检查通过后完成真实切换（supervisor/deploy_cutover.py，真实进程停止+重启验证过）；在途 work_node_execution 重启后可恢复（复用既有 REQ-06 机制）；previous_instance_dir 保留至监控窗口结束；新增 LIVE_PORT=8420 常量（05 §5.9 缺的一个字段）；真实测试发现并修复 deploy_supervisor.is_alive() 的僵尸进程判活缺陷（新增 reap_if_exited()）"}, {"id": "U-040", "phase": "PH5", "sub_phase": "PH5-S3", "name": "c_deploy_supervisor_monitor（见 development-plan.md 2026-08-02 实现补充）", "classification": "Risk Gate", "depends_on": ["U-039"], "coverage": {"model": ["deploy_health_signal"], "view": [], "controller": ["c_deploy_supervisor_monitor"], "process": [], "requirement": []}, "done": "四类信号采样正常（supervisor/deploy_monitor.py）；validator_approval_rate 偏离基线阈值时触发 anomaly_detected；approve 占比算法基于对 scheduler.finish_task_lifecycle 真实代码逻辑的核实（PASS 从不产生 bug_record，FAIL 必然产生），非字面 SDD 定义"}, - {"id": "U-041", "phase": "PH5", "sub_phase": "PH5-S3", "name": "c_deploy_supervisor_rollback（见 development-plan.md 2026-08-02 实现补充）", "classification": "Risk Gate", "depends_on": ["U-038", "U-040"], "coverage": {"model": ["deployment"], "view": [], "controller": ["c_deploy_supervisor_rollback"], "process": [], "requirement": []}, "done": "回滚路径 A/B 各跑通一次（supervisor/deploy_rollback.py，真实进程+真实 git 仓库验证）；旧版本也起不来时正确终态到 blocked_pending_human（真实场景验证，非假设）；git revert 冲突有明确处理（真实构造冲突历史验证过，含 --abort 清理）"}, + {"id": "U-041", "phase": "PH5", "sub_phase": "PH5-S3", "name": "c_deploy_supervisor_rollback（见 development-plan.md 2026-08-02/03 实现补充）", "classification": "Risk Gate", "depends_on": ["U-038", "U-040"], "coverage": {"model": ["deployment"], "view": [], "controller": ["c_deploy_supervisor_rollback"], "process": [], "requirement": []}, "done": "回滚路径 A/B 各跑通一次（supervisor/deploy_rollback.py，真实进程+真实 git 仓库验证）；旧版本也起不来时正确终态到 blocked_pending_human（真实场景验证，非假设）；git revert 冲突有明确处理（真实构造冲突历史验证过，含 --abort 清理）。2026-08-03：U-042 第七次真实压测第一次真正走到 health_check 环节，发现 rollback_path_a 从未清理候选的 git worktree 检出目录（永久留在磁盘+git worktree list），已修复并补真实回归测试"}, {"id": "U-042", "phase": "PH5", "sub_phase": "PH5-S3", "name": "自举闭环端到端验收（见 development-plan.md 2026-08-02 实现补充）", "classification": "Risk Gate", "depends_on": ["U-035", "U-041"], "coverage": {"model": [], "view": [], "controller": [], "process": [], "requirement": []}, "done": "真实复现 self-development-feasibility.md 9 步场景，全程无需人工评审代码内容（supervisor/self_hosting_e2e.py 编排层已建好）；真实双实例 E2E 用例（RUN_REAL_SELF_HOSTING_E2E=1，独立于常规 RUN_REAL_AGENT_E2E 的单独环境变量）默认跳过，需显式环境变量开启且需额外用户确认才真正执行；日常回归只跑 mock 版本（6 个编排逻辑测试，已跑通）"}, {"id": "U-043", "phase": "PH5", "sub_phase": "PH5-S4", "name": "控制台展示 deployment 状态与健康信号（见 development-plan.md 2026-08-02 实现补充）", "classification": "Value Path", "depends_on": ["U-024", "U-040"], "coverage": {"model": [], "view": ["v_deployment_status"], "controller": ["c_deployment_list", "c_deployment_current", "c_deployment_get"], "process": [], "requirement": []}, "done": "能看到当前 deployment 状态机所处状态与最近一次健康信号采样结果；02-V 从未定义过这个视图（PH5 后加），DOM/class 约定沿用现有页面；真实服务器 + urllib 验证过全部三条新路由"} ], diff --git a/supervisor/deploy_rollback.py b/supervisor/deploy_rollback.py index cdd88ab..2ba4e52 100644 --- a/supervisor/deploy_rollback.py +++ b/supervisor/deploy_rollback.py @@ -76,13 +76,24 @@ def _block(conn, deployment_id, reason): def rollback_path_a( conn, deployment_id, candidate_pid, candidate_db_path, git_commit_hash, - repo_root=None, description=None, task_id=None, stop_timeout=10, + repo_root=None, description=None, task_id=None, stop_timeout=10, candidate_instance_dir=None, ): """Pre-cutover rollback (design doc §7.1): Live was never touched, this is the cheapest path. Terminates the candidate, deletes its one-time DB - copy, `git revert`s the merge commit, and records a `bug_record( - root_cause_category=core_engine)` audit trail with the failing signals - as evidence. Returns {status: 'rolled_back'}.""" + copy, removes its worktree checkout, `git revert`s the merge commit, + and records a `bug_record(root_cause_category=core_engine)` audit trail + with the failing signals as evidence. Returns {status: 'rolled_back'}. + + **`candidate_instance_dir` cleanup (2026-08-03, real self-hosting run)**: + a real health-check failure exercised this path for the first time and + left the candidate's `git worktree add --detach` checkout permanently + on disk (and registered in `git worktree list`) -- a confirmed-bad + candidate is never needed again after path A, unlike a successful + cutover's candidate (which becomes `previous_instance_dir`, correctly + kept until the monitoring window closes). Optional (default None) so + existing tests that never used a real worktree checkout for their + candidate are unaffected; the real caller (self_hosting_e2e.py) now + passes it through from `stage_deployment()`'s own return value.""" repo_root = repo_root or paths.repo_root() description = description or "health check failed for deployment {}".format(deployment_id) @@ -90,6 +101,11 @@ def rollback_path_a( deploy_cutover._stop_process(candidate_pid, timeout=stop_timeout) if candidate_db_path and os.path.exists(candidate_db_path): os.remove(candidate_db_path) + if candidate_instance_dir: + subprocess.run( + ["git", "worktree", "remove", "--force", candidate_instance_dir], + cwd=repo_root, check=True, capture_output=True, text=True, + ) _git_revert(repo_root, git_commit_hash) except subprocess.CalledProcessError as e: _abort_revert_if_in_progress(repo_root) diff --git a/supervisor/self_hosting_e2e.py b/supervisor/self_hosting_e2e.py index 7637768..32967af 100644 --- a/supervisor/self_hosting_e2e.py +++ b/supervisor/self_hosting_e2e.py @@ -80,6 +80,7 @@ def run_self_hosting_closed_loop( conn, staged["deployment_id"], staged["candidate_pid"], candidate_db_path, merge_commit, repo_root=target_folder_path, description="health check failed: {}".format(health["signals"]), + candidate_instance_dir=staged["candidate_instance_dir"], ) return {"stage": "health_check", "outcome": "rolled_back_path_a", "deployment_id": staged["deployment_id"]} diff --git a/supervisor/test_deploy_rollback.py b/supervisor/test_deploy_rollback.py index bc47e74..48e6563 100644 --- a/supervisor/test_deploy_rollback.py +++ b/supervisor/test_deploy_rollback.py @@ -119,6 +119,32 @@ class RollbackPathAHappyPathTest(DeployRollbackTestBase): self.assertEqual(bug[1], "resolved") self.assertIn("canary_task failed", bug[2]) + def test_candidate_worktree_checkout_is_removed(self): + # 2026-08-03 regression: a real self-hosting run's health-check + # failure exercised this path for the first time and left the + # candidate's `git worktree add --detach` checkout permanently on + # disk (and still registered in `git worktree list`) -- confirmed- + # bad candidates are never needed again after path A. + candidate_dir = os.path.join(self.home_tmp, "candidate-checkout") + subprocess.run( + ["git", "worktree", "add", "--detach", candidate_dir, self.commit_v2], + cwd=self.repo_dir, check=True, capture_output=True, text=True, + ) + proc = subprocess.Popen(["sleep", "300"]) + self._cleanup_pids.append(proc.pid) + self._seed_deployment("dep-a3", "health_check_failed", git_commit_hash=self.commit_v2) + + deploy_rollback.rollback_path_a( + self.conn, "dep-a3", proc.pid, None, self.commit_v2, repo_root=self.repo_dir, + candidate_instance_dir=candidate_dir, + ) + + self.assertFalse(os.path.isdir(candidate_dir)) + worktrees = subprocess.run( + ["git", "worktree", "list"], cwd=self.repo_dir, check=True, capture_output=True, text=True + ).stdout + self.assertNotIn(candidate_dir, worktrees) + class RollbackPathAFailureTest(DeployRollbackTestBase): def test_git_revert_conflict_marks_blocked_pending_human_and_raises(self): -- libgit2 0.21.4