U-041 fix: rollback path A now removes the candidate's worktree checkout

The 7th real self-hosting attempt was the first to actually reach a real health check against a real candidate process -- it correctly failed and triggered rollback_path_a for real: candidate terminated, git revert landed cleanly. But the candidate's `git worktree add --detach` checkout (candidate_instance_dir, tracked since U-037 specifically to know where this lives) was never removed -- left permanently on disk and still registered in `git worktree list`. A confirmed-bad candidate is never needed again after path A, unlike a successful cutover's candidate (which correctly becomes previous_instance_dir, kept until the monitoring window closes) -- these are different lifecycles that shouldn't share the same "leave it" behavior. rollback_path_a gains an optional candidate_instance_dir parameter (default None, existing tests unaffected) and removes the worktree before git revert when provided. self_hosting_e2e.py now threads through the value stage_deployment() already returns. New regression test uses a real `git worktree add --detach` checkout and confirms both the directory and its git worktree registration are gone after rollback. Also cleaned up 3 orphaned processes left by earlier test_deploy_rollback.py runs (backing temp dirs already gone) and the real leftover candidate worktree from this run. 561 tests green (440 src + 121 supervisor). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

U-041 fix: rollback path A now removes the candidate's worktree checkout
The 7th real self-hosting attempt was the first to actually reach a real health check against a real candidate process -- it correctly failed and triggered rollback_path_a for real: candidate terminated, git revert landed cleanly. But the candidate's `git worktree add --detach` checkout (candidate_instance_dir, tracked since U-037 specifically to know where this lives) was never removed -- left permanently on disk and still registered in `git worktree list`. A confirmed-bad candidate is never needed again after path A, unlike a successful cutover's candidate (which correctly becomes previous_instance_dir, kept until the monitoring window closes) -- these are different lifecycles that shouldn't share the same "leave it" behavior. rollback_path_a gains an optional candidate_instance_dir parameter (default None, existing tests unaffected) and removes the worktree before git revert when provided. self_hosting_e2e.py now threads through the value stage_deployment() already returns. New regression test uses a real `git worktree add --detach` checkout and confirms both the directory and its git worktree registration are gone after rollback. Also cleaned up 3 orphaned processes left by earlier test_deploy_rollback.py runs (backing temp dirs already gone) and the real leftover candidate worktree from this run. 561 tests green (440 src + 121 supervisor). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
bboonnee
1 parent f220d91d
Showing 5 changed files with 49 additions and 5 deletions
docs/development-plan.md
docs/sdd/unit_matrix.json
supervisor/deploy_rollback.py
supervisor/self_hosting_e2e.py
supervisor/test_deploy_rollback.py
@@ -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，已有明确处理）
  
@@ -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 验证过全部三条新路由"}
   ],
@@ -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)
@@ -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"]}
  
@@ -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):