Commit ebfcc4375a182a2332f58f79848ddf07439d89ba
1 parent
b6423cff
U-013 follow-up: drop the exclude pathspec, it was itself a hard error
Attempt 10 crashed immediately in the fix committed last: git add -A -- . ':!.graphlemon-worktrees' throws "paths are ignored by one of your .gitignore files" (exit 1, not just advisory text -- confirmed even with advice.addIgnoredFile=false suppressed) the moment that path is ALSO gitignored, which it now always is. Reproduced locally. Plain `git add -A` (no explicit pathspec) already skips gitignored paths correctly on its own -- the explicit exclude was redundant now that .gitignore covers it, and actively conflicting. Simplified ensure_worktree_committed() back down. Also added the same .gitignore entry to test_work_node_run.py's _init_git_repo() fixture so real-worktree tests match production config instead of coincidentally not hitting this. 562 tests green (441 src + 121 supervisor). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Showing
3 changed files
with
22 additions
and
10 deletions
docs/development-plan.md
| @@ -137,6 +137,7 @@ | @@ -137,6 +137,7 @@ | ||
| 137 | - 覆盖:01-M `work_node_execution`;03-C `c_work_node_run`;REQ-03(boundary) | 137 | - 覆盖:01-M `work_node_execution`;03-C `c_work_node_run`;REQ-03(boundary) |
| 138 | - **真实 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"的提示词来保证正确性,而是在系统层面兜底,人设漂移/遗忘不应该导致真实工作成果被无声丢弃。 | 138 | - **真实 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"的提示词来保证正确性,而是在系统层面兜底,人设漂移/遗忘不应该导致真实工作成果被无声丢弃。 |
| 139 | - **真实 bug 的第二层(2026-08-03,U-042 第八/九次真实压测发现,比上面这条更深一层)**:`ensure_worktree_committed()` 只兜住了 worktree 自己的未提交改动,但 git worktree 的设计本身是**和主仓库共享同一个项目身份**(同一个 `.git`,只是另一份检出)——一次真实压测发现,`claude -p` 的 `memory: project` 写入并不总是遵守调用时传入的 `cwd`,而是可能直接落在**主仓库自己的检出目录**上,和当时具体是哪个节点、哪次调用完全无关(不是 `_check_prd_consistency` 一个函数的问题,把它单独包一层 worktree 只是挪动了"这次踩中的是哪次调用",没有从根上解决)。后果:主仓库工作区多出一处未提交的 memory 文件改动,下一个节点自己的、原本完全正常的 worktree merge 会因为"主分支有本地改动会被覆盖"直接报错退出(`git merge` exit 2)。修复:`ensure_worktree_committed()` 从只处理 worktree 自身,扩展成同时也对 `repo_path`(主仓库检出目录)本身跑一遍同样的"有脏改动就自动提交"逻辑,在每次 `merge_worktree()` 之前调用两次——不管是哪次真实调用造成的漂移、原因是什么,统一兜底,而不是针对某一个特定调用点单独打补丁。修复过程中顺带发现并解决了这条兜底自己引入的一个新问题:对主仓库跑 `git add -A` 时,如果某个节点的 worktree 检出目录(`.graphlemon-worktrees/<node_exec_id>/`)当时还没被 `remove_worktree()` 清理,会被当成一个类似 git link 的条目一并加进去,紧接着目录被删掉后 `git status` 就会显示成一条诡异的"D"(删除)——两处修复:`.graphlemon-worktrees/` 加进 `.gitignore`(这是长期正确性),`ensure_worktree_committed()` 自己的 `git add -A`/`git status --porcelain` 也显式排除这个路径(`-- . ':!.graphlemon-worktrees'`),不依赖调用方仓库一定配置了对的 `.gitignore`。新增回归测试 `test_drift_on_main_checkout_is_auto_committed_before_merge`,模拟"改动落在主仓库而不是 worktree"这个具体场景,并验证接下来一个节点的合并不受影响。 | 139 | - **真实 bug 的第二层(2026-08-03,U-042 第八/九次真实压测发现,比上面这条更深一层)**:`ensure_worktree_committed()` 只兜住了 worktree 自己的未提交改动,但 git worktree 的设计本身是**和主仓库共享同一个项目身份**(同一个 `.git`,只是另一份检出)——一次真实压测发现,`claude -p` 的 `memory: project` 写入并不总是遵守调用时传入的 `cwd`,而是可能直接落在**主仓库自己的检出目录**上,和当时具体是哪个节点、哪次调用完全无关(不是 `_check_prd_consistency` 一个函数的问题,把它单独包一层 worktree 只是挪动了"这次踩中的是哪次调用",没有从根上解决)。后果:主仓库工作区多出一处未提交的 memory 文件改动,下一个节点自己的、原本完全正常的 worktree merge 会因为"主分支有本地改动会被覆盖"直接报错退出(`git merge` exit 2)。修复:`ensure_worktree_committed()` 从只处理 worktree 自身,扩展成同时也对 `repo_path`(主仓库检出目录)本身跑一遍同样的"有脏改动就自动提交"逻辑,在每次 `merge_worktree()` 之前调用两次——不管是哪次真实调用造成的漂移、原因是什么,统一兜底,而不是针对某一个特定调用点单独打补丁。修复过程中顺带发现并解决了这条兜底自己引入的一个新问题:对主仓库跑 `git add -A` 时,如果某个节点的 worktree 检出目录(`.graphlemon-worktrees/<node_exec_id>/`)当时还没被 `remove_worktree()` 清理,会被当成一个类似 git link 的条目一并加进去,紧接着目录被删掉后 `git status` 就会显示成一条诡异的"D"(删除)——两处修复:`.graphlemon-worktrees/` 加进 `.gitignore`(这是长期正确性),`ensure_worktree_committed()` 自己的 `git add -A`/`git status --porcelain` 也显式排除这个路径(`-- . ':!.graphlemon-worktrees'`),不依赖调用方仓库一定配置了对的 `.gitignore`。新增回归测试 `test_drift_on_main_checkout_is_auto_committed_before_merge`,模拟"改动落在主仓库而不是 worktree"这个具体场景,并验证接下来一个节点的合并不受影响。 |
| 140 | + - **上面这条修复自己的一个真 bug(2026-08-03,U-042 第十次真实压测立刻发现)**:`-- . ':!.graphlemon-worktrees'` 这个显式排除 pathspec,一旦该路径**同时**已经在 `.gitignore` 里(这次修复自己刚加的),`git add -A` 会直接报错退出("paths are ignored by one of your .gitignore files",即使配置 `advice.addIgnoredFile=false` 抑制提示文案,退出码依然是 1,不是单纯的提示)——本地复现过,是真的会失败,不是巧合。既然 `.gitignore` 已经能让普通的 `git add -A`(不带任何显式 pathspec)正确跳过这个目录,显式排除反而是多余且互相冲突的,直接删掉,`ensure_worktree_committed()` 恢复成最简单的 `git add -A`。同时把 `test_work_node_run.py` 的 `_init_git_repo()` 测试夹具也补上同一条 `.gitignore` 规则,让所有真实用到 `git worktree add` 的测试仓库和生产环境配置保持一致,而不是靠巧合"测试仓库正好没触发这条冲突"。 | ||
| 140 | 141 | ||
| 141 | ### PH1-S3 端到端验收(收口) | 142 | ### PH1-S3 端到端验收(收口) |
| 142 | 143 |
src/test_work_node_run.py
| @@ -23,7 +23,12 @@ def _init_git_repo(path): | @@ -23,7 +23,12 @@ def _init_git_repo(path): | ||
| 23 | subprocess.run(["git", "init"], cwd=path, check=True, capture_output=True) | 23 | subprocess.run(["git", "init"], cwd=path, check=True, capture_output=True) |
| 24 | with open(os.path.join(path, "README.md"), "w") as f: | 24 | with open(os.path.join(path, "README.md"), "w") as f: |
| 25 | f.write("seed\n") | 25 | f.write("seed\n") |
| 26 | - subprocess.run(["git", "add", "README.md"], cwd=path, check=True, capture_output=True) | 26 | + # matches graphlemon's own .gitignore (2026-08-03) -- without this, |
| 27 | + # ensure_worktree_committed's plain `git add -A` on repo_path picks up | ||
| 28 | + # a still-present worktree checkout as a tracked gitlink-like entry. | ||
| 29 | + with open(os.path.join(path, ".gitignore"), "w") as f: | ||
| 30 | + f.write(".graphlemon-worktrees/\n") | ||
| 31 | + subprocess.run(["git", "add", "README.md", ".gitignore"], cwd=path, check=True, capture_output=True) | ||
| 27 | subprocess.run( | 32 | subprocess.run( |
| 28 | ["git"] + work_node_run._GIT_IDENTITY + ["commit", "-m", "seed"], | 33 | ["git"] + work_node_run._GIT_IDENTITY + ["commit", "-m", "seed"], |
| 29 | cwd=path, check=True, capture_output=True, | 34 | cwd=path, check=True, capture_output=True, |
src/work_node_run.py
| @@ -144,19 +144,25 @@ def ensure_worktree_committed(path, node_exec_id): | @@ -144,19 +144,25 @@ def ensure_worktree_committed(path, node_exec_id): | ||
| 144 | why -- `path` being a worktree or the main checkout doesn't change what | 144 | why -- `path` being a worktree or the main checkout doesn't change what |
| 145 | "auto-commit any dirty state here" means.""" | 145 | "auto-commit any dirty state here" means.""" |
| 146 | # `.graphlemon-worktrees/` (create_worktree's own checkout location) | 146 | # `.graphlemon-worktrees/` (create_worktree's own checkout location) |
| 147 | - # lives INSIDE repo_path while a node is running -- excluded from both | ||
| 148 | - # the dirty-check and the add, not just relying on the caller repo | ||
| 149 | - # having a matching .gitignore (2026-08-03: without this, `git add -A` | ||
| 150 | - # on repo_path picked up the still-present worktree checkout itself as | ||
| 151 | - # a tracked gitlink-like entry, which then showed as "deleted" the | ||
| 152 | - # moment remove_worktree() ran right after). | ||
| 153 | - exclude_worktrees = ["--", ".", ":!.graphlemon-worktrees"] | 147 | + # lives INSIDE repo_path while a node is running -- without excluding |
| 148 | + # it, `git add -A` on repo_path picked up the still-present worktree | ||
| 149 | + # checkout itself as a tracked gitlink-like entry, which then showed as | ||
| 150 | + # "deleted" the moment remove_worktree() ran right after (2026-08-03). | ||
| 151 | + # An explicit `:!.graphlemon-worktrees` pathspec exclude was tried | ||
| 152 | + # first but is itself a hard error ("paths are ignored by one of your | ||
| 153 | + # .gitignore files", exit 1, even with the advice hint suppressed) the | ||
| 154 | + # moment the path is ALSO gitignored -- which it now always is (see | ||
| 155 | + # .gitignore). Plain `git add -A` with no explicit pathspec already | ||
| 156 | + # skips gitignored paths correctly on its own; the caller's repo just | ||
| 157 | + # needs `.graphlemon-worktrees/` in its own `.gitignore` for this to | ||
| 158 | + # hold (graphlemon's own repo has it; test fixtures that use a real | ||
| 159 | + # worktree checkout add the same entry in their own setUp). | ||
| 154 | status = subprocess.run( | 160 | status = subprocess.run( |
| 155 | - ["git", "status", "--porcelain"] + exclude_worktrees, cwd=path, check=True, capture_output=True, text=True, | 161 | + ["git", "status", "--porcelain"], cwd=path, check=True, capture_output=True, text=True, |
| 156 | ).stdout | 162 | ).stdout |
| 157 | if not status.strip(): | 163 | if not status.strip(): |
| 158 | return | 164 | return |
| 159 | - subprocess.run(["git", "add", "-A"] + exclude_worktrees, cwd=path, check=True, capture_output=True) | 165 | + subprocess.run(["git", "add", "-A"], cwd=path, check=True, capture_output=True) |
| 160 | subprocess.run( | 166 | subprocess.run( |
| 161 | _prefix_git(["commit", "-m", "graphlemon: auto-commit uncommitted changes for " + node_exec_id]), | 167 | _prefix_git(["commit", "-m", "graphlemon: auto-commit uncommitted changes for " + node_exec_id]), |
| 162 | cwd=path, check=True, capture_output=True, text=True, | 168 | cwd=path, check=True, capture_output=True, text=True, |