From 0b32b95abee6355a5a7e98b6cac92f4b8e4c88b6 Mon Sep 17 00:00:00 2001 From: bboonnee Date: Mon, 3 Aug 2026 00:27:40 +0800 Subject: [PATCH] U-031 fix: worktree-isolate the PRD consistency check for self-target --- docs/development-plan.md | 1 + docs/sdd/unit_matrix.json | 2 +- src/e2e_pipeline.py | 55 ++++++++++++++++++++++++++++++++++++++++--------------- src/test_e2e_pipeline.py | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 96 insertions(+), 16 deletions(-) diff --git a/docs/development-plan.md b/docs/development-plan.md index 2678ef5..f4a993a 100644 --- a/docs/development-plan.md +++ b/docs/development-plan.md @@ -304,6 +304,7 @@ PH2-S1/S2/S3/S4 之间除标注的依赖外相互独立，可并行推进。 3. **REQ-06 / REQ-07 / REQ-14 三条 failure 用例共享的同一个真实缺口，是这次审计里最重要的发现**：`CorruptPersistedStateError`（REQ-06）、`ClassifierBlockedError`（REQ-07）、`UsageLimitWaitTimeoutError`（REQ-14）都被正确地抛出，也都正确地把各自的 `work_node_execution.status` 标成了 `blocked`（后者是 U-031 自己加的 catch-all），但从来没有任何代码把这个后果真正传导到 `task.status='blocked_pending_human'`——`work_node_run.py` 自己的 docstring 甚至早就写明"callers should route it"，但没有一个调用方真的这么做。这正是 U-032 的调度器该管的事：`e2e_pipeline.py` 拆出 `run_task_lifecycle()`（`run_end_to_end()` 变成"建 task + 调它"的两行包装，签名/行为不变），`scheduler.py` 新增 `_block_task()`（照抄 `budget_guardrail.check_budget()` 已有的 UPDATE + `task_status_changed` 事件写法，不是发明新模式），`run_goal_to_completion()` 自己建 task 后捕获这三种异常并正确路由；其余异常仍然原样冒泡，不吞掉。三个新测试类（`CorruptPersistedStateRoutingTest`/`ClassifierBlockedRoutingTest`/`UsageLimitWaitTimeoutRoutingTest`）逐一验证。 4. **REQ-02 boundary**（PRD 出现自相矛盾的场景假设应被拦截，触发 `reflecting` 而非直接放行）：这条此前完全没有实现——`artifact_write.write_artifact()` 一直无条件把 `validation_status` 写成 `'passed'`，`artifact_write.py` 自己的 docstring 也如实记录了这个缺口。因为这是全新的 LLM 判断能力而不是接线修复，就此询问用户是否现在补上，用户选择"现在就建"。新增 `work_graph_orchestrate.run_graph()` 的可选 `verify_fn` 钩子（每个节点跑完、下游节点开始前调用，命中失败时把 `work_graph_run.status` 写成 `reflecting`、对同一节点重新生成一次并再次校验，仍失败则写 `blocked` 并抛 `ConsistencyCheckFailedError`——`work_graph_run.status` 在此之前从未被任何代码真正写过，一直停在初始的 `'planning'`，这次是它第一次被真正用起来，但只做了 `verifying`/`reflecting`/`blocked` 三个和这条用例直接相关的状态，没有借机把 `running`/`merging` 那一整套图级状态机也补全——04-P 没具体描述过那部分，属于超出这次范围的新设计）；`e2e_pipeline._check_prd_consistency()` 是新增的一次真实 `agent(agentType:'planner', schema:...)` 调用，只挂在 PRD 节点上（`_prd_consistency_verify_fn`，其余节点直接放行不调用）。真实验证：`test_e2e_pipeline_e2e.RealPrdConsistencyCheckTest` 用一段刻意自相矛盾的撞墙规则（"必须立即结束" vs "可以穿墙不结束"）+一段真实一致的 PRD 各跑一次真实调用。**第一次真实跑就抓到一个真 bug**：模型把 `consistent` 字段返回成 JSON 字符串 `"false"` 而不是布尔值 `false`（即使 schema 里已经写了 `"consistent": {"type": "boolean"}`——05 §5.4.1.1 早就说过不能盲目信任 schema 真的会被强制），Python 里 `if "false":` 是真值，直接把"发现矛盾"误判成"通过"，是全场景里最隐蔽的一类 bug（mock 测试永远返回真的 Python bool，只有真实调用能暴露）。新增 `_coerce_bool()` 做防御性转换（bool 原样、字符串按大小写不敏感比较 "true"、其余走 `bool()`），修复后重跑真实测试通过；同时补了两个 mock 层回归测试锁死这个具体行为（`test_string_false_from_a_real_call_is_not_treated_as_truthy`/`test_string_true_from_a_real_call_is_treated_as_consistent`）。 5. **REQ-13 failure**：一个次要的测试质量问题（不是功能缺口）——`test_ws_nonexistent_node_rejected_before_upgrade` 只检查了 HTTP 状态行是 404，没检查响应体里的 `NODE_EXEC_NOT_FOUND`（生产代码本身是对的）。顺手加了 `_read_http_response_with_json_body` 辅助函数，把这条和同类的 `PROJECT_NOT_FOUND`（REQ-12 failure）测试都补上了 body 断言。 + 6. **真实 bug（2026-08-03，U-042 第四次真实自举压测发现）**：`_check_prd_consistency()` 直接对 `target_folder_path` 调用 `execute_work_graph()`，完全绕开了 `work_node_run.run_work_node()` 的 worktree 隔离机制——之前每一次真实调用这个函数都是对着一次性 target project 克隆做的，从没自我 targeting 过，所以从没暴露过问题。这次真实自举压测里，这个调用用 `agentType:'planner'` + `memory:project`，会真的往调用时的 cwd 写 `.claude/agent-memory/planner/MEMORY.md`——self-target 场景下 cwd 就是 `target_folder_path` 本身（主仓库），于是这次真实写入变成了主分支工作区里一处未提交的本地改动；紧接着 SDD 节点自己的 worktree 合并回主分支时，`git merge` 因为主分支工作区有未提交改动会覆盖同一个文件而直接报错退出（exit 2），SDD 节点的真实产出被卡在一个从未合并成功的 worktree 分支里。修复：`_check_prd_consistency()` 现在按 05 §5.8 同样的"self-target 无条件强制 worktree"规则，复用 `work_node_run.create_worktree`/`ensure_worktree_committed`/`merge_worktree`/`remove_worktree` 把这次调用也隔离到自己的 worktree 里，不再直接碰主仓库工作区；`paths.is_self_target()` 为 False 时（普通 target project）保持原样直接调用，不引入不必要的开销。新增两个回归测试锁死这个行为分支（self-target 走 worktree、非 self-target 不走）。清理过程中还顺手发现并合并了一个被卡住的真实 worktree 分支（SDD 节点当时的真实产出，合并冲突只是两条几乎一样的 memory 文案措辞不同，手工解决），以及三个此前测试遗留下来从未清理的孤儿 worktree（`git worktree remove`）。 - 全部 5 处修复后，`python3 -m unittest discover -s src` 380 个测试全绿（含新增的）；REQ-01~14 现在逐条都有真实覆盖，PH4 可以视为真正收口，PH5 排期不再依赖对这份回归的信任假设。 - 覆盖：01-M `work_graph_run`/`work_node_execution`（真正被生产代码写入，不再只是测试 fixture）；03-C `c_work_graph_plan`/`c_work_graph_execute`/`c_work_node_run`（首次真正接通）；REQ-01~14 全部 diff --git a/docs/sdd/unit_matrix.json b/docs/sdd/unit_matrix.json index a9e15d2..a8f680a 100644 --- a/docs/sdd/unit_matrix.json +++ b/docs/sdd/unit_matrix.json @@ -47,7 +47,7 @@ {"id": "U-029", "phase": "PH3", "sub_phase": "PH3-S3", "name": "v_skill_library + v_skill_audit_log（见 development-plan.md 2026-07-31 实现补充）", "classification": "Value Path", "depends_on": ["U-018"], "coverage": {"model": [], "view": ["v_skill_library", "v_skill_audit_log"], "controller": ["c_skill_diff", "c_skill_list", "c_skill_get", "c_skill_rollback"], "process": [], "requirement": []}, "done": "Skill 库与变更审计日志可查看；c_skill_rollback 首次接上 HTTP（skill_repo_path 可注入，测试绝不碰真实仓库）"}, {"id": "U-030", "phase": "PH3", "sub_phase": "PH3-S4", "name": "并发/预算/日志保留配置页（见 development-plan.md 2026-07-31 实现补充）", "classification": "Value Path", "depends_on": ["U-019", "U-021"], "coverage": {"model": ["org_concurrency_config"], "view": ["v_concurrency_settings", "v_budget_settings", "v_log_retention_settings"], "controller": ["c_concurrency_config_update", "c_budget_config_update", "c_log_retention_config_update"], "process": [], "requirement": []}, "done": "三个配置页可用且改动生效；PH3 至此全部完成"}, - {"id": "U-031", "phase": "PH4", "sub_phase": "PH4-S1", "name": "REQ-01~14 全量自动化回归（见 development-plan.md 2026-08-01/2026-08-02 实现补充）", "classification": "Risk Gate", "depends_on": ["U-014", "U-017", "U-020", "U-021", "U-022", "U-023", "U-024.5", "U-025", "U-026", "U-027", "U-028", "U-029", "U-030"], "coverage": {"model": [], "view": [], "controller": [], "process": [], "requirement": ["REQ-01", "REQ-02", "REQ-03", "REQ-04", "REQ-05", "REQ-06", "REQ-07", "REQ-08", "REQ-09", "REQ-10", "REQ-11", "REQ-12", "REQ-13", "REQ-14"]}, "done": "06 节全部 14 条 REQ 的全部用例通过一次完整回归；回归审计中发现 c_work_graph_plan/c_work_graph_execute/c_work_node_run 三个模块彼此隔离测试通过但从未被生产代码真正串联（唯一验证过的闭环 e2e_pipeline.py 绕开了三者，直接用 org_node_trigger），已按用户决策现场补上集成：新增 src/work_graph_orchestrate.py（拓扑排序 + 每节点一份独立动态工作流脚本调用 run_work_node），e2e_pipeline.py 的 plan_documents/implement_code 改为真调用 plan_work_graph→run_graph；真实 API 验证过程中发现并修复两个真实 bug（graph_manifest 的 id/node_id 字段名及 depends_on 缺失见 work_graph_plan._normalize_graph_manifest；run_work_node 缺 repo_path 导致 cwd=None）。**2026-08-02（PH5 开工前逐条复核）**：对照 06 节重新逐条核对后又发现并修复 5 处真实缺口——REQ-03 failure 缺失 'reflecting' 状态写入、REQ-05/09 的调度器场景此前从未真正测试过（U-032 建好后才补测）、REQ-06/07/14 三条 failure 共享的同一个缺口（CorruptPersistedStateError/ClassifierBlockedError/UsageLimitWaitTimeoutError 均未路由到 task.status=blocked_pending_human）、REQ-02 boundary（PRD 一致性校验）此前完全未实现现已用新的 verify_fn 机制补上、REQ-13 failure 一处测试断言过弱；全部详见 development-plan.md 对应实现补充，380 测试全绿，此时才是 06 节 REQ-01~14 真正逐条有覆盖"}, + {"id": "U-031", "phase": "PH4", "sub_phase": "PH4-S1", "name": "REQ-01~14 全量自动化回归（见 development-plan.md 2026-08-01/2026-08-02 实现补充）", "classification": "Risk Gate", "depends_on": ["U-014", "U-017", "U-020", "U-021", "U-022", "U-023", "U-024.5", "U-025", "U-026", "U-027", "U-028", "U-029", "U-030"], "coverage": {"model": [], "view": [], "controller": [], "process": [], "requirement": ["REQ-01", "REQ-02", "REQ-03", "REQ-04", "REQ-05", "REQ-06", "REQ-07", "REQ-08", "REQ-09", "REQ-10", "REQ-11", "REQ-12", "REQ-13", "REQ-14"]}, "done": "06 节全部 14 条 REQ 的全部用例通过一次完整回归；回归审计中发现 c_work_graph_plan/c_work_graph_execute/c_work_node_run 三个模块彼此隔离测试通过但从未被生产代码真正串联（唯一验证过的闭环 e2e_pipeline.py 绕开了三者，直接用 org_node_trigger），已按用户决策现场补上集成：新增 src/work_graph_orchestrate.py（拓扑排序 + 每节点一份独立动态工作流脚本调用 run_work_node），e2e_pipeline.py 的 plan_documents/implement_code 改为真调用 plan_work_graph→run_graph；真实 API 验证过程中发现并修复两个真实 bug（graph_manifest 的 id/node_id 字段名及 depends_on 缺失见 work_graph_plan._normalize_graph_manifest；run_work_node 缺 repo_path 导致 cwd=None）。**2026-08-02（PH5 开工前逐条复核）**：对照 06 节重新逐条核对后又发现并修复 5 处真实缺口——REQ-03 failure 缺失 'reflecting' 状态写入、REQ-05/09 的调度器场景此前从未真正测试过（U-032 建好后才补测）、REQ-06/07/14 三条 failure 共享的同一个缺口（CorruptPersistedStateError/ClassifierBlockedError/UsageLimitWaitTimeoutError 均未路由到 task.status=blocked_pending_human）、REQ-02 boundary（PRD 一致性校验）此前完全未实现现已用新的 verify_fn 机制补上、REQ-13 failure 一处测试断言过弱；全部详见 development-plan.md 对应实现补充，380 测试全绿，此时才是 06 节 REQ-01~14 真正逐条有覆盖。**2026-08-03（U-042 真实自举压测第四次尝试发现）**：`_check_prd_consistency()` 直接对 target_folder_path 调用未做 worktree 隔离，self-target 场景下会把 memory 写入落到主仓库工作区未提交状态，导致下一个节点的真实 worktree 合并失败；已改为按 05 §5.8 同样规则隔离到自己的 worktree，补充两个回归测试"}, {"id": "U-032", "phase": "PH4", "sub_phase": "PH4-S2", "name": "多 Goal 并发 + 长时间运行稳定性压测（见 development-plan.md 2026-08-02 实现补充）", "classification": "Risk Gate", "depends_on": ["U-031"], "coverage": {"model": ["org_concurrency_config"], "view": [], "controller": [], "process": [], "requirement": ["REQ-09"]}, "done": "并发数调高后多个独立 Goal 可同时稳定运行；发现并补上一个此前从未存在的结构性缺口——04-P/01-M/REQ-09 boundary 反复假设存在的自动调度器实际从未被建过，新增 src/scheduler.py（ThreadPoolExecutor + 每线程独立 sqlite 连接，避免复刻 U-027 的共享连接锁陷阱）真正实现，并顺带补上 e2e_pipeline.run_end_to_end() 自己文档过的缺口（PASS/FAIL 后从不调用 close_task_done/run_fix_loop，卡在 validating 不动）"}, {"id": "U-033", "phase": "PH4", "sub_phase": "PH4-S3", "name": "A-2 假设收口（见 development-plan.md 2026-08-02 实现补充）", "classification": "Risk Gate", "depends_on": ["U-004"], "coverage": {"model": [], "view": [], "controller": [], "process": [], "requirement": [], "assumption": ["A-2"]}, "done": "A-2 状态确认为 confirmed 或记录实际偏差；最终结论：维持 U-004 记录的\"部分成立\"，PH1~PH4 积累的真实运行数据未暴露新偏差，偏差范围到 U-004 为止已完全摸清（Coder SOUL.md 三处实质性重写），不改判 confirmed，正式收口"}, diff --git a/src/e2e_pipeline.py b/src/e2e_pipeline.py index a444d29..6f403f2 100644 --- a/src/e2e_pipeline.py +++ b/src/e2e_pipeline.py @@ -51,8 +51,8 @@ import subprocess import uuid from src import ( - artifact_write, org_node_lifecycle, org_node_trigger, task_create, - work_graph_execute, work_graph_orchestrate, work_graph_plan, + artifact_write, org_node_lifecycle, org_node_trigger, paths, task_create, + work_graph_execute, work_graph_orchestrate, work_graph_plan, work_node_run, ) _READY_PROMPT = "Confirm you're ready to begin. Take no other action yet." @@ -154,19 +154,44 @@ def _coerce_bool(value): def _check_prd_consistency(target_folder_path, prd_content, timeout=180): """REQ-02 boundary: one real agent() call reviewing the PRD's own - content for genuine internal contradiction. Returns (ok, issues).""" - script_name = "graphlemon-prd-consistency-" + uuid.uuid4().hex - prompt = _PRD_CONSISTENCY_PROMPT.format(prd_content=prd_content) - script_content = ( - "export const meta = {{ name: {name}, description: 'graphlemon PRD consistency check' }}\n\n" - "const result = await agent({prompt}, {{ agentType: 'planner', schema: {schema} }})\n\n" - "return result\n" - ).format(name=json.dumps(script_name), prompt=json.dumps(prompt), schema=json.dumps(_PRD_CONSISTENCY_SCHEMA)) - work_graph_execute.write_workflow_script(target_folder_path, script_name, script_content) - result = work_graph_execute.execute_work_graph( - target_folder_path, script_name, {}, _PRD_CONSISTENCY_SCHEMA, timeout=timeout - ) - return _coerce_bool(result["consistent"]), result.get("issues", []) + content for genuine internal contradiction. Returns (ok, issues). + + **Self-target worktree isolation (2026-08-03, real self-hosting run)**: + this call uses `agentType='planner'` with `memory: project`, which + writes a real, uncommitted change to `.claude/agent-memory/planner/ + MEMORY.md` in whatever `cwd` it runs with. Every prior real run of this + check only ever targeted a throwaway target-project clone, so calling + it directly against `target_folder_path` never caused a problem -- + until a genuine self-hosting run hit it: the memory write landed as an + uncommitted local change on the *main* checkout (bypassing + `work_node_run.run_work_node`'s worktree isolation entirely, since this + call never goes through it), which then made the *next* node's own + worktree merge fail with "local changes would be overwritten by + merge". 05 §5.8's unconditional self-target worktree rule has to apply + here too, not just to `run_work_node`'s own callers.""" + is_self = paths.is_self_target(target_folder_path) + node_exec_id = "prd-consistency-" + uuid.uuid4().hex[:12] + worktree_path = work_node_run.create_worktree(target_folder_path, node_exec_id) if is_self else None + cwd = worktree_path or target_folder_path + try: + script_name = "graphlemon-prd-consistency-" + uuid.uuid4().hex + prompt = _PRD_CONSISTENCY_PROMPT.format(prd_content=prd_content) + script_content = ( + "export const meta = {{ name: {name}, description: 'graphlemon PRD consistency check' }}\n\n" + "const result = await agent({prompt}, {{ agentType: 'planner', schema: {schema} }})\n\n" + "return result\n" + ).format(name=json.dumps(script_name), prompt=json.dumps(prompt), schema=json.dumps(_PRD_CONSISTENCY_SCHEMA)) + work_graph_execute.write_workflow_script(cwd, script_name, script_content) + result = work_graph_execute.execute_work_graph( + cwd, script_name, {}, _PRD_CONSISTENCY_SCHEMA, timeout=timeout + ) + if worktree_path: + work_node_run.ensure_worktree_committed(worktree_path, node_exec_id) + work_node_run.merge_worktree(target_folder_path, node_exec_id) + return _coerce_bool(result["consistent"]), result.get("issues", []) + finally: + if worktree_path: + work_node_run.remove_worktree(target_folder_path, worktree_path) def _prd_consistency_verify_fn(node_artifact_types, target_folder_path): diff --git a/src/test_e2e_pipeline.py b/src/test_e2e_pipeline.py index ecea726..9a94931 100644 --- a/src/test_e2e_pipeline.py +++ b/src/test_e2e_pipeline.py @@ -202,6 +202,60 @@ class PrdConsistencyVerifyFnTest(unittest.TestCase): self.assertEqual(args[0], "/tmp/proj-1") +class PrdConsistencySelfTargetWorktreeTest(unittest.TestCase): + """2026-08-03 regression: a real self-hosting run showed _check_prd_ + consistency() calling execute_work_graph() directly against + target_folder_path, bypassing worktree isolation entirely -- when + self-targeting, its own agentType='planner'+memory:project call writes + a real, uncommitted change straight to the main checkout's + .claude/agent-memory/planner/MEMORY.md, which then made the NEXT + node's own (correctly worktree-isolated) merge fail with "local + changes would be overwritten by merge". This call must isolate the + same way work_node_run.run_work_node() already does for every other + self-target call.""" + + def test_self_target_isolates_the_check_in_its_own_worktree(self): + node_artifact_types = {"prd": "prd"} + verify_fn = e2e_pipeline._prd_consistency_verify_fn(node_artifact_types, "/repo") + with patch.object(e2e_pipeline.paths, "is_self_target", return_value=True), \ + patch.object(e2e_pipeline.work_node_run, "create_worktree", return_value="/repo/.graphlemon-worktrees/x") as mock_create, \ + patch.object(e2e_pipeline.work_node_run, "ensure_worktree_committed") as mock_ensure, \ + patch.object(e2e_pipeline.work_node_run, "merge_worktree") as mock_merge, \ + patch.object(e2e_pipeline.work_node_run, "remove_worktree") as mock_remove, \ + patch.object(work_graph_execute, "write_workflow_script") as mock_write, \ + patch.object( + work_graph_execute, "execute_work_graph", + return_value={"consistent": True, "issues": []}, + ) as mock_execute: + ok, feedback = verify_fn({"node_id": "prd"}, {"content": "some prd"}) + + self.assertTrue(ok) + mock_create.assert_called_once_with("/repo", unittest.mock.ANY) + # both the script write and the real execute call must use the + # WORKTREE path as cwd, never the main checkout, when self-targeting + self.assertEqual(mock_write.call_args[0][0], "/repo/.graphlemon-worktrees/x") + self.assertEqual(mock_execute.call_args[0][0], "/repo/.graphlemon-worktrees/x") + mock_ensure.assert_called_once() + mock_merge.assert_called_once() + mock_remove.assert_called_once() + + def test_non_self_target_does_not_use_a_worktree_at_all(self): + node_artifact_types = {"prd": "prd"} + verify_fn = e2e_pipeline._prd_consistency_verify_fn(node_artifact_types, "/tmp/proj-1") + with patch.object(e2e_pipeline.paths, "is_self_target", return_value=False), \ + patch.object(e2e_pipeline.work_node_run, "create_worktree") as mock_create, \ + patch.object(work_graph_execute, "write_workflow_script") as mock_write, \ + patch.object( + work_graph_execute, "execute_work_graph", + return_value={"consistent": True, "issues": []}, + ) as mock_execute: + verify_fn({"node_id": "prd"}, {"content": "some prd"}) + + mock_create.assert_not_called() + self.assertEqual(mock_write.call_args[0][0], "/tmp/proj-1") + self.assertEqual(mock_execute.call_args[0][0], "/tmp/proj-1") + + class ValidateImplementationTest(unittest.TestCase): def test_parses_pass_verdict(self): with patch("src.org_node_trigger.trigger_org_node", return_value="VERDICT: PASS\nlooks good"): -- libgit2 0.21.4