Commit 8af3d4e4e060656bdb5fc7f0af012877e2463b71

Authored by bboonnee
1 parent afb478d5

U-013 fix: auto-commit uncommitted worktree changes before merge

A real self-hosting run (U-042, first real exercise of needs_worktree=True
since U-035 forces it unconditionally for self-target) exposed a real bug
RealGitWorktreeIsolationTest never caught: its own run_fn always committed
its edits, but a real Coder isn't instructed anywhere to git commit its own
changes. The Coder genuinely edited src/org_loop_route.py and verified it
in its own test_report -- but never committed, so merge_worktree()'s `git
merge` was a true no-op ("Already up to date"), and remove_worktree() then
silently discarded the dirty worktree along with the real work. code_diff
came out empty; the change never reached the branch at all.

Adds ensure_worktree_committed(): checks `git status --porcelain` in the
worktree before merging and auto-commits any dirty state found. A no-op
when the worktree is legitimately clean (e.g. a pure investigation task).
Same "判断用 Agent,机制用代码" principle already used elsewhere in this
codebase -- don't rely on persona prompt-engineering for a mechanical
correctness guarantee that the system itself should hold.

Two new regression tests: one reproducing the exact "edited but never
committed" scenario, one confirming a legitimately-clean worktree still
merges fine. 554 tests green (435 src + 119 supervisor).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
docs/development-plan.md
@@ -134,6 +134,7 @@ @@ -134,6 +134,7 @@
134 - Done(**2026-07-31 订正**:原来写"REQ-03 happy/boundary 通过",但 happy 用例("产出 code_diff 与 test_report...unit+integration test 全部通过")需要真实 Coder 会话写代码 + `c_artifact_write`(U-011 已记录的缺口,至今没有 Unit 认领),U-013 单靠 U-012 做不到——第五次出现同一类问题):REQ-03 **boundary** 通过(两个独立节点各自用 `git worktree add` 隔离,`run_fn` 各写一个文件并 commit,`merge --no-ff` 合并回主分支,`git log` 确认两次提交都在,且 `work_node_execution.git_worktree_ref` 各不相同);happy 用例随 `c_artifact_write` 落地后累计满足 134 - Done(**2026-07-31 订正**:原来写"REQ-03 happy/boundary 通过",但 happy 用例("产出 code_diff 与 test_report...unit+integration test 全部通过")需要真实 Coder 会话写代码 + `c_artifact_write`(U-011 已记录的缺口,至今没有 Unit 认领),U-013 单靠 U-012 做不到——第五次出现同一类问题):REQ-03 **boundary** 通过(两个独立节点各自用 `git worktree add` 隔离,`run_fn` 各写一个文件并 commit,`merge --no-ff` 合并回主分支,`git log` 确认两次提交都在,且 `work_node_execution.git_worktree_ref` 各不相同);happy 用例随 `c_artifact_write` 落地后累计满足
135 - **精度提升(2026-07-31,实现后补充)**:03-C §3.6 给出了 `USAGE_LIMIT_REACHED` 的精确探测正则 `/hit your (session|weekly|Opus) limit/i`,比 U-010/U-012 当时用的短语近似匹配更准——U-013 按这条正则实现,且真正写回 `work_node_execution.quota_wait_started_at`/`quota_limit_type`(U-010/U-012 阶段这两张表字段还没有真实归属)。`c_work_node_run` 是内部接口(03-C 原话"不对外暴露 HTTP,供 c_work_graph_execute 调用"),退避重试从 U-012 的整图级别下沉到这里的单节点级别 135 - **精度提升(2026-07-31,实现后补充)**:03-C §3.6 给出了 `USAGE_LIMIT_REACHED` 的精确探测正则 `/hit your (session|weekly|Opus) limit/i`,比 U-010/U-012 当时用的短语近似匹配更准——U-013 按这条正则实现,且真正写回 `work_node_execution.quota_wait_started_at`/`quota_limit_type`(U-010/U-012 阶段这两张表字段还没有真实归属)。`c_work_node_run` 是内部接口(03-C 原话"不对外暴露 HTTP,供 c_work_graph_execute 调用"),退避重试从 U-012 的整图级别下沉到这里的单节点级别
136 - 覆盖:01-M `work_node_execution`;03-C `c_work_node_run`;REQ-03(boundary) 136 - 覆盖:01-M `work_node_execution`;03-C `c_work_node_run`;REQ-03(boundary)
  137 + - **真实 bug(2026-08-02/03,U-042 第一次真实自举压测时发现)**:`RealGitWorktreeIsolationTest` 从 U-013 建成那天起,`run_fn` 一直是测试自己写的、写完文件后主动 `git add` + `git commit` 的假实现——这个盲区从没被真实 Coder 会话踩过,因为 PH1~PH4 所有真实调用都没启用 `needs_worktree=True`(development-plan.md U-031 的实现补充里已经记过这条),直到 U-035 让 self-target 场景无条件强制 worktree,U-042 第一次真实自举跑起来后才第一次真正踩中。真实现象:Coder 在 worktree 里确实编辑了文件,自己也验证过(`git status`/`git diff --stat` 都在它自己的 `test_report` 里回显了),但从未 `git commit`——Coder 人设里没有任何一处要求它提交自己的改动。`merge_worktree()` 只会 `git merge` 已提交的分支,worktree 分支上如果压根没有新提交,merge 就是个真正的空操作("Already up to date",不产生新 commit),紧接着 `remove_worktree()` 把这个还带着未提交改动的目录直接删掉——一次真实、已验证过的 Coder 改动就这样被无声吞掉,`implement_code()` 算出来的 `code_diff` 自然是空的。修复:新增 `work_node_run.ensure_worktree_committed()`,在 `merge_worktree()` 之前检查 worktree 是否有未提交改动(`git status --porcelain`),有则自动 `git add -A` + 提交一条 `graphlemon: auto-commit uncommitted worktree changes for <node_exec_id>`,worktree 本来就干净(比如纯排查、没有真的改代码)时什么也不做。新增两个回归测试:`test_uncommitted_worktree_changes_are_auto_committed_before_merge`(复现"改了但没提交"场景,断言改动最终真的落到主分支)、`test_clean_worktree_with_no_changes_is_still_a_valid_merge`(确认"确实没什么好改"这种合法结果不会被误伤)。这是"判断用 Agent,机制用代码"原则的又一次体现——不指望靠给人设加一条"记得 commit"的提示词来保证正确性,而是在系统层面兜底,人设漂移/遗忘不应该导致真实工作成果被无声丢弃。
137 138
138 ### PH1-S3 端到端验收(收口) 139 ### PH1-S3 端到端验收(收口)
139 140
docs/sdd/unit_matrix.json
@@ -25,7 +25,7 @@ @@ -25,7 +25,7 @@
25 {"id": "U-010", "phase": "PH1", "sub_phase": "PH1-S1", "name": "c_org_node_trigger 完整实现(三段前向状态迁移,见 development-plan.md 2026-07-31 范围说明)", "classification": "Value Path", "depends_on": ["U-005", "U-009"], "coverage": {"model": [], "view": [], "controller": ["c_org_node_trigger"], "process": [], "requirement": []}, "done": "三个 Org Agent 分别可被触发(真实 claude --agent 调用),task.status 按 queued→planning_brd_prd_sdd→coding→validating 正确流转;重试/路由/blocked_pending_human 不在本 Unit 范围"}, 25 {"id": "U-010", "phase": "PH1", "sub_phase": "PH1-S1", "name": "c_org_node_trigger 完整实现(三段前向状态迁移,见 development-plan.md 2026-07-31 范围说明)", "classification": "Value Path", "depends_on": ["U-005", "U-009"], "coverage": {"model": [], "view": [], "controller": ["c_org_node_trigger"], "process": [], "requirement": []}, "done": "三个 Org Agent 分别可被触发(真实 claude --agent 调用),task.status 按 queued→planning_brd_prd_sdd→coding→validating 正确流转;重试/路由/blocked_pending_human 不在本 Unit 范围"},
26 {"id": "U-011", "phase": "PH1", "sub_phase": "PH1-S2", "name": "c_work_graph_plan 实现(只规划不执行,见 development-plan.md 2026-07-31 订正)", "classification": "Value Path", "depends_on": ["U-004", "U-010"], "coverage": {"model": ["artifact", "work_graph_run"], "view": [], "controller": ["c_work_graph_plan"], "process": [], "requirement": ["REQ-02"]}, "done": "graph_manifest 通过结构校验(brd/prd/sdd/benchmark 四节点+依赖链),work_graph_run(status=planning) 落地;REQ-02 happy 用例整体随 U-012/U-013 累计满足。c_artifact_write(03-C §3.7)目前没有任何 Unit 认领,是已知缺口"}, 26 {"id": "U-011", "phase": "PH1", "sub_phase": "PH1-S2", "name": "c_work_graph_plan 实现(只规划不执行,见 development-plan.md 2026-07-31 订正)", "classification": "Value Path", "depends_on": ["U-004", "U-010"], "coverage": {"model": ["artifact", "work_graph_run"], "view": [], "controller": ["c_work_graph_plan"], "process": [], "requirement": ["REQ-02"]}, "done": "graph_manifest 通过结构校验(brd/prd/sdd/benchmark 四节点+依赖链),work_graph_run(status=planning) 落地;REQ-02 happy 用例整体随 U-012/U-013 累计满足。c_artifact_write(03-C §3.7)目前没有任何 Unit 认领,是已知缺口"},
27 {"id": "U-012", "phase": "PH1", "sub_phase": "PH1-S2", "name": "c_work_graph_execute 完整实现(见 development-plan.md 2026-07-31 范围说明)", "classification": "Value Path", "depends_on": ["U-006", "U-011"], "coverage": {"model": [], "view": [], "controller": ["c_work_graph_execute"], "process": ["p_work_graph_execution", "p_usage_limit_wait"], "requirement": ["REQ-07", "REQ-14"]}, "done": "REQ-07 boundary/failure 通过(happy 需 PH2 REQ-04/05/06 就绪后累计满足);REQ-14 happy 通过。退避重试是整图级别,非逐节点;CLASSIFIER_BLOCKED 用单次调用 permission_denials≥3 近似判定"}, 27 {"id": "U-012", "phase": "PH1", "sub_phase": "PH1-S2", "name": "c_work_graph_execute 完整实现(见 development-plan.md 2026-07-31 范围说明)", "classification": "Value Path", "depends_on": ["U-006", "U-011"], "coverage": {"model": [], "view": [], "controller": ["c_work_graph_execute"], "process": ["p_work_graph_execution", "p_usage_limit_wait"], "requirement": ["REQ-07", "REQ-14"]}, "done": "REQ-07 boundary/failure 通过(happy 需 PH2 REQ-04/05/06 就绪后累计满足);REQ-14 happy 通过。退避重试是整图级别,非逐节点;CLASSIFIER_BLOCKED 用单次调用 permission_denials≥3 近似判定"},
28 - {"id": "U-013", "phase": "PH1", "sub_phase": "PH1-S2", "name": "c_work_node_run + git worktree 隔离(见 development-plan.md 2026-07-31 订正)", "classification": "Value Path", "depends_on": ["U-012"], "coverage": {"model": ["work_node_execution"], "view": [], "controller": ["c_work_node_run"], "process": [], "requirement": ["REQ-03"]}, "done": "REQ-03 boundary 通过(真实 git worktree 隔离+merge,无冲突);happy 需 c_artifact_write 落地后累计满足"}, 28 + {"id": "U-013", "phase": "PH1", "sub_phase": "PH1-S2", "name": "c_work_node_run + git worktree 隔离(见 development-plan.md 2026-07-31 订正、2026-08-02/03 真实 bug 修复)", "classification": "Value Path", "depends_on": ["U-012"], "coverage": {"model": ["work_node_execution"], "view": [], "controller": ["c_work_node_run"], "process": [], "requirement": ["REQ-03"]}, "done": "REQ-03 boundary 通过(真实 git worktree 隔离+merge,无冲突);happy 需 c_artifact_write 落地后累计满足。2026-08-02/03:U-042 第一次真实自举压测发现 Coder 编辑 worktree 后从不 commit、merge_worktree 空操作、真实改动被 remove_worktree 静默吞掉的真实 bug,新增 ensure_worktree_committed() 兜底自动提交,已修复并补测试"},
29 {"id": "U-014", "phase": "PH1", "sub_phase": "PH1-S3", "name": "贪吃蛇场景端到端验收(含补建 c_artifact_write,见 development-plan.md 2026-07-31 实现补充)", "classification": "Value Path", "depends_on": ["U-011", "U-012", "U-013"], "coverage": {"model": ["artifact"], "view": [], "controller": ["c_artifact_write"], "process": [], "requirement": []}, "done": "架构文档 §11 验收标准第 1-3 条全部达成;plan_documents(Planner 四件套+血缘)真实验证过,Coder/Validator 全流程真实跑通待用户确认后执行"}, 29 {"id": "U-014", "phase": "PH1", "sub_phase": "PH1-S3", "name": "贪吃蛇场景端到端验收(含补建 c_artifact_write,见 development-plan.md 2026-07-31 实现补充)", "classification": "Value Path", "depends_on": ["U-011", "U-012", "U-013"], "coverage": {"model": ["artifact"], "view": [], "controller": ["c_artifact_write"], "process": [], "requirement": []}, "done": "架构文档 §11 验收标准第 1-3 条全部达成;plan_documents(Planner 四件套+血缘)真实验证过,Coder/Validator 全流程真实跑通待用户确认后执行"},
30 30
31 {"id": "U-015", "phase": "PH2", "sub_phase": "PH2-S1", "name": "p_validator_root_cause_trace 实现", "classification": "Risk Gate", "depends_on": ["U-013"], "coverage": {"model": ["bug_record"], "view": [], "controller": ["c_bug_record_create"], "process": ["p_validator_root_cause_trace"], "requirement": ["REQ-04"]}, "done": "REQ-04 happy/boundary/failure 通过"}, 31 {"id": "U-015", "phase": "PH2", "sub_phase": "PH2-S1", "name": "p_validator_root_cause_trace 实现", "classification": "Risk Gate", "depends_on": ["U-013"], "coverage": {"model": ["bug_record"], "view": [], "controller": ["c_bug_record_create"], "process": ["p_validator_root_cause_trace"], "requirement": ["REQ-04"]}, "done": "REQ-04 happy/boundary/failure 通过"},
src/test_work_node_run.py
@@ -381,6 +381,50 @@ class RealGitWorktreeIsolationTest(WorkNodeRunTestBase): @@ -381,6 +381,50 @@ class RealGitWorktreeIsolationTest(WorkNodeRunTestBase):
381 self.assertIn("module_a.txt", log.stdout) 381 self.assertIn("module_a.txt", log.stdout)
382 self.assertIn("module_b.txt", log.stdout) 382 self.assertIn("module_b.txt", log.stdout)
383 383
  384 + def _write_file_never_commit_run_fn(self, filename, content):
  385 + """Unlike _write_file_run_fn, deliberately edits and leaves the
  386 + worktree DIRTY -- reproducing a real Coder Work Node, which is
  387 + never instructed to `git commit` its own edits (found via a real
  388 + self-hosting run, 2026-08-02: a real, verified Coder edit was
  389 + silently discarded because nothing committed it before merge)."""
  390 + def run_fn(cwd, model=None):
  391 + with open(os.path.join(cwd, filename), "w") as f:
  392 + f.write(content)
  393 + return {"value": filename}
  394 +
  395 + return run_fn
  396 +
  397 + def test_uncommitted_worktree_changes_are_auto_committed_before_merge(self):
  398 + work_node_run.run_work_node(
  399 + self.conn, "node-1", {"goal": "x"}, _IN_SCHEMA, _OUT_SCHEMA,
  400 + run_fn=self._write_file_never_commit_run_fn("uncommitted.txt", "real edit, never committed"),
  401 + needs_worktree=True, repo_path=self.tmp.name,
  402 + )
  403 +
  404 + merged_path = os.path.join(self.tmp.name, "uncommitted.txt")
  405 + self.assertTrue(os.path.isfile(merged_path), "the Coder's edit must survive merge even if it never committed")
  406 + with open(merged_path) as f:
  407 + self.assertEqual(f.read(), "real edit, never committed")
  408 +
  409 + status = subprocess.run(
  410 + ["git", "status", "--porcelain"], cwd=self.tmp.name, check=True, capture_output=True, text=True
  411 + ).stdout
  412 + self.assertEqual(status.strip(), "", "main checkout must be clean after merge, not carrying a stray dirty file")
  413 +
  414 + def test_clean_worktree_with_no_changes_is_still_a_valid_merge(self):
  415 + # A node that legitimately makes no changes (e.g. a pure
  416 + # investigation, see the real self-hosting run this fix responds
  417 + # to) must not error just because there's nothing to auto-commit.
  418 + work_node_run.run_work_node(
  419 + self.conn, "node-1", {"goal": "x"}, _IN_SCHEMA, _OUT_SCHEMA,
  420 + run_fn=lambda cwd, model=None: {"value": "investigated, no changes needed"},
  421 + needs_worktree=True, repo_path=self.tmp.name,
  422 + )
  423 + status = subprocess.run(
  424 + ["git", "status", "--porcelain"], cwd=self.tmp.name, check=True, capture_output=True, text=True
  425 + ).stdout
  426 + self.assertEqual(status.strip(), "")
  427 +
384 428
385 class BreakpointRecoveryTest(WorkNodeRunTestBase): 429 class BreakpointRecoveryTest(WorkNodeRunTestBase):
386 """U-021 / REQ-06: run_work_node called again for a node_exec_id that was 430 """U-021 / REQ-06: run_work_node called again for a node_exec_id that was
src/work_node_run.py
@@ -117,6 +117,31 @@ def create_worktree(repo_path, node_exec_id): @@ -117,6 +117,31 @@ def create_worktree(repo_path, node_exec_id):
117 return worktree_path 117 return worktree_path
118 118
119 119
  120 +def ensure_worktree_committed(worktree_path, node_exec_id):
  121 + """A Coder Work Node is never actually required to `git commit` its own
  122 + edits before finishing -- nothing in its persona instructs it to, and
  123 + every prior test's `run_fn` happened to commit its own changes, so this
  124 + gap went unnoticed until a real self-hosting run (2026-08-02) exposed
  125 + it: the Coder made a real, verified edit, never committed it, and
  126 + merge_worktree() found nothing to merge (a no-op "Already up to date"
  127 + merge) -- remove_worktree() then silently discarded the dirty working
  128 + tree along with the real work. Auto-commits any uncommitted state left
  129 + in the worktree before it gets merged, so "the Coder said it edited a
  130 + file" and "the change actually lands" can't diverge silently. No-op
  131 + when the worktree is already clean (a node that legitimately made no
  132 + changes, e.g. a pure investigation)."""
  133 + status = subprocess.run(
  134 + ["git", "status", "--porcelain"], cwd=worktree_path, check=True, capture_output=True, text=True,
  135 + ).stdout
  136 + if not status.strip():
  137 + return
  138 + subprocess.run(["git", "add", "-A"], cwd=worktree_path, check=True, capture_output=True)
  139 + subprocess.run(
  140 + _prefix_git(["commit", "-m", "graphlemon: auto-commit uncommitted worktree changes for " + node_exec_id]),
  141 + cwd=worktree_path, check=True, capture_output=True, text=True,
  142 + )
  143 +
  144 +
120 def merge_worktree(repo_path, node_exec_id): 145 def merge_worktree(repo_path, node_exec_id):
121 """Merges the node's branch back into whichever branch is currently 146 """Merges the node's branch back into whichever branch is currently
122 checked out in the main worktree at repo_path.""" 147 checked out in the main worktree at repo_path."""
@@ -316,6 +341,7 @@ def run_work_node( @@ -316,6 +341,7 @@ def run_work_node(
316 ) 341 )
317 conn.commit() 342 conn.commit()
318 if worktree_path: 343 if worktree_path:
  344 + ensure_worktree_committed(worktree_path, node_exec_id)
319 merge_worktree(repo_path, node_exec_id) 345 merge_worktree(repo_path, node_exec_id)
320 remove_worktree(repo_path, worktree_path) # 03-C §3.6: "执行完成后清理 worktree" 346 remove_worktree(repo_path, worktree_path) # 03-C §3.6: "执行完成后清理 worktree"
321 return output_payload 347 return output_payload