Commit 3dfcdd6a5cfbbba06fa434f421bc1a70fafc16e4
1 parent
69c3ea62
PH4 closure audit: close 5 real REQ-01~14 gaps found before starting PH5
CLAUDE.md is explicit that PH5 (self-modification/auto-rollback) must not start until PH4 has genuinely closed -- not just "all unit tests green," but 06-验收与测试锚点.md's REQ-01~14 happy/boundary/failure cases actually covered. A REQ-by-REQ audit (the same discipline that caught U-031's integration gap) found 5 real gaps: - REQ-03 failure: work_node_execution.status jumped straight running->retrying, skipping 'reflecting' (declared in the schema enum, never once written by any code). - REQ-05 failure / REQ-09 boundary: the "scheduler keeps serving other queued tasks past one blocked_pending_human" scenario was never tested against a real scheduler -- the old test literally said "no scheduler exists yet to test against directly" (stale since U-032). - REQ-06 / REQ-07 / REQ-14 failure (the biggest gap, shared root cause): CorruptPersistedStateError, ClassifierBlockedError, and UsageLimitWaitTimeoutError were each correctly raised and each correctly marked their own work_node_execution 'blocked', but nothing ever routed that to task.status='blocked_pending_human' with the blocked_reason each REQ's acceptance text names. e2e_pipeline.run_end_to_end is split into run_task_lifecycle() so scheduler.py's run_goal_to_completion can create the task itself and catch these three specific exceptions, following the same UPDATE+execution_event pattern budget_guardrail.check_budget() already established for REQ-10. - REQ-02 boundary: PRD internal-contradiction check (verifying->reflecting) had no implementation anywhere -- artifact_write always marked validation_status='passed' unconditionally. Added an optional verify_fn hook to work_graph_orchestrate.run_graph() (one regeneration attempt, then ConsistencyCheckFailedError) and a real consistency-check agent() call scoped to the PRD node only. The first real run of this new call caught a genuine bug: the model returned "consistent": "false" as a JSON string, not a boolean, which `if ok:` silently treated as truthy -- fixed with explicit _coerce_bool(). - REQ-13 failure: minor test-assertion weakness (WS 404 rejection test didn't check the JSON error body). 384 tests green. REQ-01~14 now have real, exercised coverage rather than a "tests are green" assumption -- PH4 can be treated as genuinely closed. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Showing
12 changed files
with
636 additions
and
26 deletions
docs/development-plan.md
| @@ -297,6 +297,13 @@ PH2-S1/S2/S3/S4 之间除标注的依赖外相互独立,可并行推进。 | @@ -297,6 +297,13 @@ PH2-S1/S2/S3/S4 之间除标注的依赖外相互独立,可并行推进。 | ||
| 297 | - **`agentType` + `schema` 能否在动态工作流的 `agent()` 调用里正确共存,是这次集成最大的未知数,做了两次独立真实验证**:先用一个最小 spike(一个只有一次 `agent({agentType:'planner', schema:{...}})` 调用的一节点工作流)验证——不仅 `structured_output` 正确返回,追问角色身份时模型明确自称"Planning Agent (PL)",说明迁移后的 SOUL.md 人设真的通过 `agentType` 正确加载了(不是走到某个通用 agent);后续完整 `plan_documents()` 真实跑通又在真实的四节点场景下再次确认。记录在 05 §5.4.1.5。 | 297 | - **`agentType` + `schema` 能否在动态工作流的 `agent()` 调用里正确共存,是这次集成最大的未知数,做了两次独立真实验证**:先用一个最小 spike(一个只有一次 `agent({agentType:'planner', schema:{...}})` 调用的一节点工作流)验证——不仅 `structured_output` 正确返回,追问角色身份时模型明确自称"Planning Agent (PL)",说明迁移后的 SOUL.md 人设真的通过 `agentType` 正确加载了(不是走到某个通用 agent);后续完整 `plan_documents()` 真实跑通又在真实的四节点场景下再次确认。记录在 05 §5.4.1.5。 |
| 298 | - **Coder 阶段的范围决策**:REQ-03 的验收锚点只写了 `c_work_node_run`,没写 `c_work_graph_plan`/`c_work_graph_execute`——因为"规划一个真正的多模块代码图"是另一个完全不同的 LLM 能力,这次没有人要求建,也没有 SDD 规定怎么建,属于超出这次集成范围的新能力,不是"补上就完事"的小活。`implement_code()` 因此只用一个合成的单节点 manifest 走 `work_graph_orchestrate.run_graph()`,让 `c_work_node_run` 对 Coder 也真正生效,但没有尝试造一个"Coder 图规划器"。`needs_worktree` 参数留在 `run_graph()` 里但暂时两处调用都没启用——worktree 隔离要求 `.claude/agents/<org_node>.md` 已经**提交**到分支上(`git worktree add` 只看已提交内容,`deploy_agent_definitions()` 目前只写工作区文件不 commit),这个"部署后要不要顺手提交"的问题这次没有一并解决,留作后续发现,不是遗漏是有意不在这次范围内展开。真实验证也只覆盖了 Planner 全流程(Coder/Validator 走同一套机制,机制本身已经被 Planner 的真实跑通证明,没有再花真钱跑一次完整 Coder 会话,跟 `test_e2e_pipeline_e2e.py` 一贯的成本节制原则一致)。 | 298 | - **Coder 阶段的范围决策**:REQ-03 的验收锚点只写了 `c_work_node_run`,没写 `c_work_graph_plan`/`c_work_graph_execute`——因为"规划一个真正的多模块代码图"是另一个完全不同的 LLM 能力,这次没有人要求建,也没有 SDD 规定怎么建,属于超出这次集成范围的新能力,不是"补上就完事"的小活。`implement_code()` 因此只用一个合成的单节点 manifest 走 `work_graph_orchestrate.run_graph()`,让 `c_work_node_run` 对 Coder 也真正生效,但没有尝试造一个"Coder 图规划器"。`needs_worktree` 参数留在 `run_graph()` 里但暂时两处调用都没启用——worktree 隔离要求 `.claude/agents/<org_node>.md` 已经**提交**到分支上(`git worktree add` 只看已提交内容,`deploy_agent_definitions()` 目前只写工作区文件不 commit),这个"部署后要不要顺手提交"的问题这次没有一并解决,留作后续发现,不是遗漏是有意不在这次范围内展开。真实验证也只覆盖了 Planner 全流程(Coder/Validator 走同一套机制,机制本身已经被 Planner 的真实跑通证明,没有再花真钱跑一次完整 Coder 会话,跟 `test_e2e_pipeline_e2e.py` 一贯的成本节制原则一致)。 |
| 299 | - **Validator 阶段未改动**:REQ-04 的覆盖从来没提过 Work Graph 控制器(只有 `c_bug_record_create`/`p_validator_root_cause_trace`),`validate_implementation()` 原来的直连调用已经如实对应它自己的 REQ,不需要动。 | 299 | - **Validator 阶段未改动**:REQ-04 的覆盖从来没提过 Work Graph 控制器(只有 `c_bug_record_create`/`p_validator_root_cause_trace`),`validate_implementation()` 原来的直连调用已经如实对应它自己的 REQ,不需要动。 |
| 300 | + - **实现补充(2026-08-02)——PH5 开工前对 U-031 自己的 Done 标准("06 节全部 14 条 REQ 的全部用例通过一次完整回归")做逐条复核,而不是只信任"全部单测通过"**:CLAUDE.md 明确写着 PH5 依赖 PH4 真正收口,逐条核对 06-验收与测试锚点.md 每条 REQ 的 happy/boundary/failure 用例(不含 PH5 自己的 REQ-15)后,找到并修复了 5 个真实缺口(其中一个是本 Unit 自己就该覆盖但漏掉的更大缺口): | ||
| 301 | + 1. **REQ-03 failure**:`work_node_execution.status` 一直是直接从 `running` 跳到 `retrying`,`reflecting` 这个状态值虽然在 01-M 的枚举里,但从没被任何代码真正写过。`work_node_run.py` 的 schema 校验失败分支现在先写一次 `reflecting` 再决定重试/放弃,新增 `test_schema_failure_passes_through_reflecting_before_retrying`(用 sqlite `trace_callback` 侦测真实执行过的 SQL,不是靠时序猜测)。 | ||
| 302 | + 2. **REQ-05 failure / REQ-09 boundary**("多个任务同时有一个 blocked_pending_human、其余 queued 时,调度器应继续调度 queued 任务"):`test_org_loop_route.py` 里对应这条的测试自己写着"no scheduler exists yet to test against directly"——U-032 建好调度器后这句注释已经过期,但一直没人回头补一个真正对着调度器跑的测试。新增 `test_scheduler.SchedulerContinuesPastBlockedTaskTest`:真实跑 `run_scheduler_once`,一个 Goal 的任务预置 `loop_round=3`,其真实触发 `org_loop_route.LoopLimitReachedError`(真实的 `blocked_pending_human` 写入,不是 mock 出来的),其余 Goal 正常跑完,验证调度器没有被卡住。 | ||
| 303 | + 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`)逐一验证。 | ||
| 304 | + 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`)。 | ||
| 305 | + 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 断言。 | ||
| 306 | + - 全部 5 处修复后,`python3 -m unittest discover -s src` 380 个测试全绿(含新增的);REQ-01~14 现在逐条都有真实覆盖,PH4 可以视为真正收口,PH5 排期不再依赖对这份回归的信任假设。 | ||
| 300 | - 覆盖: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 全部 | 307 | - 覆盖: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 全部 |
| 301 | 308 | ||
| 302 | - **U-032 [Risk Gate]** 多 Goal 并发(`org_concurrency_config`)+ 长时间运行稳定性压测 | 309 | - **U-032 [Risk Gate]** 多 Goal 并发(`org_concurrency_config`)+ 长时间运行稳定性压测 |
docs/sdd/unit_matrix.json
| @@ -47,7 +47,7 @@ | @@ -47,7 +47,7 @@ | ||
| 47 | {"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 可注入,测试绝不碰真实仓库)"}, | 47 | {"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 可注入,测试绝不碰真实仓库)"}, |
| 48 | {"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 至此全部完成"}, | 48 | {"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 至此全部完成"}, |
| 49 | 49 | ||
| 50 | - {"id": "U-031", "phase": "PH4", "sub_phase": "PH4-S1", "name": "REQ-01~14 全量自动化回归(见 development-plan.md 2026-08-01 实现补充)", "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)"}, | 50 | + {"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 真正逐条有覆盖"}, |
| 51 | {"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 不动)"}, | 51 | {"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 不动)"}, |
| 52 | {"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,正式收口"}, | 52 | {"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,正式收口"}, |
| 53 | 53 |
src/e2e_pipeline.py
| @@ -31,6 +31,18 @@ coverage never named the Work Graph controllers at all. | @@ -31,6 +31,18 @@ coverage never named the Work Graph controllers at all. | ||
| 31 | Status-transition bookkeeping (task.status, precondition guards) stays | 31 | Status-transition bookkeeping (task.status, precondition guards) stays |
| 32 | U-010's job (org_node_lifecycle.trigger_org_node_full) -- called once per | 32 | U-010's job (org_node_lifecycle.trigger_org_node_full) -- called once per |
| 33 | phase with a lightweight "confirm ready" prompt. | 33 | phase with a lightweight "confirm ready" prompt. |
| 34 | + | ||
| 35 | +**PRD consistency check (2026-08-02, PH4-closure audit)**: REQ-02's | ||
| 36 | +boundary case (contradictory PRD scenario assumptions must be intercepted, | ||
| 37 | +not silently pass validation) had no implementation anywhere -- found while | ||
| 38 | +auditing REQ-01~14 for real coverage before starting PH5. | ||
| 39 | +`_check_prd_consistency` is one extra real `agent()` call (reusing | ||
| 40 | +`agentType='planner'` + a structured-output schema, same mechanism as every | ||
| 41 | +other node) that reviews the PRD's own content for genuine internal | ||
| 42 | +contradiction (not incompleteness/style) right after it's produced, wired | ||
| 43 | +in as `plan_documents`'s `verify_fn` -- see `work_graph_orchestrate. | ||
| 44 | +run_graph`'s docstring for what happens on a failed check (one regeneration | ||
| 45 | +attempt, then `ConsistencyCheckFailedError` if it's still contradictory). | ||
| 34 | """ | 46 | """ |
| 35 | 47 | ||
| 36 | import json | 48 | import json |
| @@ -40,7 +52,7 @@ import uuid | @@ -40,7 +52,7 @@ import uuid | ||
| 40 | 52 | ||
| 41 | from src import ( | 53 | from src import ( |
| 42 | artifact_write, org_node_lifecycle, org_node_trigger, task_create, | 54 | artifact_write, org_node_lifecycle, org_node_trigger, task_create, |
| 43 | - work_graph_orchestrate, work_graph_plan, | 55 | + work_graph_execute, work_graph_orchestrate, work_graph_plan, |
| 44 | ) | 56 | ) |
| 45 | 57 | ||
| 46 | _READY_PROMPT = "Confirm you're ready to begin. Take no other action yet." | 58 | _READY_PROMPT = "Confirm you're ready to begin. Take no other action yet." |
| @@ -48,6 +60,22 @@ _READY_PROMPT = "Confirm you're ready to begin. Take no other action yet." | @@ -48,6 +60,22 @@ _READY_PROMPT = "Confirm you're ready to begin. Take no other action yet." | ||
| 48 | _DOC_INPUT_SCHEMA = {"type": "object", "required": ["node_id"]} | 60 | _DOC_INPUT_SCHEMA = {"type": "object", "required": ["node_id"]} |
| 49 | _DOC_OUTPUT_SCHEMA = {"type": "object", "required": ["content"]} | 61 | _DOC_OUTPUT_SCHEMA = {"type": "object", "required": ["content"]} |
| 50 | 62 | ||
| 63 | +_PRD_CONSISTENCY_SCHEMA = { | ||
| 64 | + "type": "object", | ||
| 65 | + "required": ["consistent", "issues"], | ||
| 66 | + "properties": { | ||
| 67 | + "consistent": {"type": "boolean"}, | ||
| 68 | + "issues": {"type": "array", "items": {"type": "string"}}, | ||
| 69 | + }, | ||
| 70 | +} | ||
| 71 | +_PRD_CONSISTENCY_PROMPT = ( | ||
| 72 | + "Review the following PRD for internally CONTRADICTORY scenario assumptions -- e.g. two " | ||
| 73 | + "requirements that cannot both hold at once, or the same scenario described two incompatible " | ||
| 74 | + "ways. Do NOT flag mere incompleteness, ambiguity, or style issues, only genuine " | ||
| 75 | + "self-contradiction. Reply via the required schema: consistent=false only if you found a real " | ||
| 76 | + "contradiction, and list each one in issues.\n\nPRD:\n\n{prd_content}" | ||
| 77 | +) | ||
| 78 | + | ||
| 51 | _DOC_PROMPTS = { | 79 | _DOC_PROMPTS = { |
| 52 | "brd": ( | 80 | "brd": ( |
| 53 | "Write a BRD (Business Requirement Document) for this goal, following your own " | 81 | "Write a BRD (Business Requirement Document) for this goal, following your own " |
| @@ -109,6 +137,48 @@ def _planner_prompt_builder(goal_text, node_artifact_types): | @@ -109,6 +137,48 @@ def _planner_prompt_builder(goal_text, node_artifact_types): | ||
| 109 | return build_prompt | 137 | return build_prompt |
| 110 | 138 | ||
| 111 | 139 | ||
| 140 | +def _coerce_bool(value): | ||
| 141 | + """A real call (2026-08-02) returned `"consistent": "false"` as a JSON | ||
| 142 | + STRING, not a boolean, even with `"type": "boolean"` in the schema -- | ||
| 143 | + `if "false":` is truthy in Python, so this silently inverted the check | ||
| 144 | + until caught by a real test (mocks always return real Python bools, so | ||
| 145 | + this never showed up until RUN_REAL_AGENT_E2E=1). Don't trust the | ||
| 146 | + schema's declared type any more than 05 §5.4.1.1 already tells us to | ||
| 147 | + trust --json-schema itself.""" | ||
| 148 | + if isinstance(value, bool): | ||
| 149 | + return value | ||
| 150 | + if isinstance(value, str): | ||
| 151 | + return value.strip().lower() == "true" | ||
| 152 | + return bool(value) | ||
| 153 | + | ||
| 154 | + | ||
| 155 | +def _check_prd_consistency(target_folder_path, prd_content, timeout=180): | ||
| 156 | + """REQ-02 boundary: one real agent() call reviewing the PRD's own | ||
| 157 | + content for genuine internal contradiction. Returns (ok, issues).""" | ||
| 158 | + script_name = "graphlemon-prd-consistency-" + uuid.uuid4().hex | ||
| 159 | + prompt = _PRD_CONSISTENCY_PROMPT.format(prd_content=prd_content) | ||
| 160 | + script_content = ( | ||
| 161 | + "export const meta = {{ name: {name}, description: 'graphlemon PRD consistency check' }}\n\n" | ||
| 162 | + "const result = await agent({prompt}, {{ agentType: 'planner', schema: {schema} }})\n\n" | ||
| 163 | + "return result\n" | ||
| 164 | + ).format(name=json.dumps(script_name), prompt=json.dumps(prompt), schema=json.dumps(_PRD_CONSISTENCY_SCHEMA)) | ||
| 165 | + work_graph_execute.write_workflow_script(target_folder_path, script_name, script_content) | ||
| 166 | + result = work_graph_execute.execute_work_graph( | ||
| 167 | + target_folder_path, script_name, {}, _PRD_CONSISTENCY_SCHEMA, timeout=timeout | ||
| 168 | + ) | ||
| 169 | + return _coerce_bool(result["consistent"]), result.get("issues", []) | ||
| 170 | + | ||
| 171 | + | ||
| 172 | +def _prd_consistency_verify_fn(node_artifact_types, target_folder_path): | ||
| 173 | + """work_graph_orchestrate.run_graph's verify_fn -- only the PRD node | ||
| 174 | + gets checked, every other node passes through untouched.""" | ||
| 175 | + def verify_fn(node, output): | ||
| 176 | + if node_artifact_types.get(node["node_id"]) != "prd": | ||
| 177 | + return True, None | ||
| 178 | + return _check_prd_consistency(target_folder_path, output["content"]) | ||
| 179 | + return verify_fn | ||
| 180 | + | ||
| 181 | + | ||
| 112 | def plan_documents(conn, task_id, goal_text, target_folder_path, timeout=300): | 182 | def plan_documents(conn, task_id, goal_text, target_folder_path, timeout=300): |
| 113 | """Architecture §11 criterion 1: a real Planner Work Graph | 183 | """Architecture §11 criterion 1: a real Planner Work Graph |
| 114 | (c_work_graph_plan -> per-node c_work_graph_execute/c_work_node_run via | 184 | (c_work_graph_plan -> per-node c_work_graph_execute/c_work_node_run via |
| @@ -126,6 +196,7 @@ def plan_documents(conn, task_id, goal_text, target_folder_path, timeout=300): | @@ -126,6 +196,7 @@ def plan_documents(conn, task_id, goal_text, target_folder_path, timeout=300): | ||
| 126 | outputs = work_graph_orchestrate.run_graph( | 196 | outputs = work_graph_orchestrate.run_graph( |
| 127 | conn, run_id, "planner", "planner-doc-writing", target_folder_path, graph_manifest, | 197 | conn, run_id, "planner", "planner-doc-writing", target_folder_path, graph_manifest, |
| 128 | _planner_prompt_builder(goal_text, node_artifact_types), _DOC_INPUT_SCHEMA, _DOC_OUTPUT_SCHEMA, | 198 | _planner_prompt_builder(goal_text, node_artifact_types), _DOC_INPUT_SCHEMA, _DOC_OUTPUT_SCHEMA, |
| 199 | + verify_fn=_prd_consistency_verify_fn(node_artifact_types, target_folder_path), | ||
| 129 | ) | 200 | ) |
| 130 | 201 | ||
| 131 | artifact_ids = {} | 202 | artifact_ids = {} |
| @@ -219,16 +290,16 @@ def _current_commit(repo_path): | @@ -219,16 +290,16 @@ def _current_commit(repo_path): | ||
| 219 | ).stdout.strip() | 290 | ).stdout.strip() |
| 220 | 291 | ||
| 221 | 292 | ||
| 222 | -def run_end_to_end(conn, target_folder_path, goal_text): | ||
| 223 | - """Full orchestration: create_task -> plan_documents -> implement_code -> | ||
| 224 | - validate_implementation. Returns a dict with every intermediate result, | ||
| 225 | - for inspection/debugging rather than hiding it behind a single verdict. | 293 | +def run_task_lifecycle(conn, task_id, target_folder_path, goal_text): |
| 294 | + """The body of run_end_to_end() for an ALREADY-CREATED task -- split out | ||
| 295 | + 2026-08-02 so scheduler.py can create the task itself first and still | ||
| 296 | + have a task_id in scope if this raises partway through (needed to route | ||
| 297 | + CorruptPersistedStateError/ClassifierBlockedError/UsageLimitWaitTimeoutError | ||
| 298 | + to task.status='blocked_pending_human', which is scheduler.py's job, not | ||
| 299 | + this module's -- see that module's docstring). Behavior/return shape is | ||
| 300 | + unchanged from before the split; run_end_to_end() below is now a 2-line | ||
| 301 | + wrapper over this. | ||
| 226 | """ | 302 | """ |
| 227 | - project_id = conn.execute( | ||
| 228 | - "SELECT project_id FROM project WHERE target_folder_path = ?", (target_folder_path,) | ||
| 229 | - ).fetchone()[0] | ||
| 230 | - task_id = task_create.create_task(conn, project_id, goal_text)["task_id"] | ||
| 231 | - | ||
| 232 | org_node_lifecycle.trigger_org_node_full(conn, "planner", task_id, target_folder_path, _READY_PROMPT) | 303 | org_node_lifecycle.trigger_org_node_full(conn, "planner", task_id, target_folder_path, _READY_PROMPT) |
| 233 | plan_result = plan_documents(conn, task_id, goal_text, target_folder_path) | 304 | plan_result = plan_documents(conn, task_id, goal_text, target_folder_path) |
| 234 | artifact_ids = plan_result["artifact_ids"] | 305 | artifact_ids = plan_result["artifact_ids"] |
| @@ -249,6 +320,18 @@ def run_end_to_end(conn, target_folder_path, goal_text): | @@ -249,6 +320,18 @@ def run_end_to_end(conn, target_folder_path, goal_text): | ||
| 249 | } | 320 | } |
| 250 | 321 | ||
| 251 | 322 | ||
| 323 | +def run_end_to_end(conn, target_folder_path, goal_text): | ||
| 324 | + """Full orchestration: create_task -> plan_documents -> implement_code -> | ||
| 325 | + validate_implementation. Returns a dict with every intermediate result, | ||
| 326 | + for inspection/debugging rather than hiding it behind a single verdict. | ||
| 327 | + """ | ||
| 328 | + project_id = conn.execute( | ||
| 329 | + "SELECT project_id FROM project WHERE target_folder_path = ?", (target_folder_path,) | ||
| 330 | + ).fetchone()[0] | ||
| 331 | + task_id = task_create.create_task(conn, project_id, goal_text)["task_id"] | ||
| 332 | + return run_task_lifecycle(conn, task_id, target_folder_path, goal_text) | ||
| 333 | + | ||
| 334 | + | ||
| 252 | def _read_artifact_content(conn, target_folder_path, artifact_id): | 335 | def _read_artifact_content(conn, target_folder_path, artifact_id): |
| 253 | file_path = conn.execute("SELECT file_path FROM artifact WHERE artifact_id = ?", (artifact_id,)).fetchone()[0] | 336 | file_path = conn.execute("SELECT file_path FROM artifact WHERE artifact_id = ?", (artifact_id,)).fetchone()[0] |
| 254 | with open(os.path.join(target_folder_path, file_path), encoding="utf-8") as f: | 337 | with open(os.path.join(target_folder_path, file_path), encoding="utf-8") as f: |
src/scheduler.py
| @@ -26,15 +26,67 @@ replicating that discipline here would just serialize all DB work anyway and | @@ -26,15 +26,67 @@ replicating that discipline here would just serialize all DB work anyway and | ||
| 26 | defeat the point of concurrency. Each connection sets a `busy_timeout` so | 26 | defeat the point of concurrency. Each connection sets a `busy_timeout` so |
| 27 | concurrent writers on the shared file retry instead of raising "database is | 27 | concurrent writers on the shared file retry instead of raising "database is |
| 28 | locked". | 28 | locked". |
| 29 | + | ||
| 30 | +**Second gap found and closed in the same pass (2026-08-02, PH4-closure | ||
| 31 | +audit before starting PH5)**: `work_node_run.CorruptPersistedStateError` | ||
| 32 | +(REQ-06 failure), `work_graph_execute.ClassifierBlockedError` (REQ-07 | ||
| 33 | +failure), and `work_node_run.UsageLimitWaitTimeoutError` (REQ-14 failure) | ||
| 34 | +were each correctly raised and each correctly marked their own | ||
| 35 | +`work_node_execution.status='blocked'` (the last one via U-031's catch-all | ||
| 36 | +in `work_node_run.run_work_node`) -- but nothing ever caught them to apply | ||
| 37 | +the *task*-level consequence 06-验收与测试锚点.md's failure text for all | ||
| 38 | +three literally names: `task.status='blocked_pending_human'` with a specific | ||
| 39 | +`blocked_reason`. `work_node_run.py`'s own docstring already flagged this as | ||
| 40 | +a deliberate layering choice ("callers should route it to | ||
| 41 | +task.status='blocked_pending_human'") -- but no caller ever did. This is | ||
| 42 | +exactly the layer `budget_guardrail.check_budget()` already does the | ||
| 43 | +equivalent translation at for its own case (REQ-10), so `_block_task()` | ||
| 44 | +below follows that same established pattern (UPDATE + `task_status_changed` | ||
| 45 | +execution_event) rather than inventing a new one. | ||
| 29 | """ | 46 | """ |
| 30 | 47 | ||
| 48 | +import json | ||
| 49 | +import uuid | ||
| 31 | import concurrent.futures | 50 | import concurrent.futures |
| 32 | 51 | ||
| 33 | -from src import e2e_pipeline, org_level_loop, org_loop_route, schema, settings_config | 52 | +from src import ( |
| 53 | + e2e_pipeline, org_level_loop, org_loop_route, schema, settings_config, | ||
| 54 | + task_create, work_graph_execute, work_graph_orchestrate, work_node_run, | ||
| 55 | +) | ||
| 34 | 56 | ||
| 35 | _BUSY_TIMEOUT_MS = 30000 | 57 | _BUSY_TIMEOUT_MS = 30000 |
| 36 | _FIX_LOOP_TIMEOUT = 1200 | 58 | _FIX_LOOP_TIMEOUT = 1200 |
| 37 | 59 | ||
| 60 | +_BLOCKED_REASON_FOR_EXCEPTION = ( | ||
| 61 | + (work_node_run.CorruptPersistedStateError, "corrupt_persisted_state"), | ||
| 62 | + (work_graph_execute.ClassifierBlockedError, "classifier_blocked"), | ||
| 63 | + (work_node_run.UsageLimitWaitTimeoutError, "usage_limit_wait_timeout"), | ||
| 64 | + (work_graph_orchestrate.ConsistencyCheckFailedError, "prd_consistency_check_failed"), | ||
| 65 | +) | ||
| 66 | +_ROUTABLE_EXCEPTIONS = tuple(exc for exc, _ in _BLOCKED_REASON_FOR_EXCEPTION) | ||
| 67 | + | ||
| 68 | + | ||
| 69 | +def _blocked_reason_for(exc): | ||
| 70 | + for exc_type, reason in _BLOCKED_REASON_FOR_EXCEPTION: | ||
| 71 | + if isinstance(exc, exc_type): | ||
| 72 | + return reason | ||
| 73 | + raise AssertionError("no blocked_reason registered for " + type(exc).__name__) | ||
| 74 | + | ||
| 75 | + | ||
| 76 | +def _block_task(conn, task_id, blocked_reason): | ||
| 77 | + task_row = conn.execute("SELECT project_id, status FROM task WHERE task_id = ?", (task_id,)).fetchone() | ||
| 78 | + conn.execute( | ||
| 79 | + "UPDATE task SET status = 'blocked_pending_human', blocked_reason = ?, " | ||
| 80 | + "updated_at = CURRENT_TIMESTAMP WHERE task_id = ?", | ||
| 81 | + (blocked_reason, task_id), | ||
| 82 | + ) | ||
| 83 | + conn.execute( | ||
| 84 | + "INSERT INTO execution_event (event_id, project_id, task_id, event_type, payload) " | ||
| 85 | + "VALUES (?, ?, ?, 'task_status_changed', ?)", | ||
| 86 | + (str(uuid.uuid4()), task_row[0], task_id, json.dumps({"from_status": task_row[1], "to_status": "blocked_pending_human"})), | ||
| 87 | + ) | ||
| 88 | + conn.commit() | ||
| 89 | + | ||
| 38 | 90 | ||
| 39 | def finish_task_lifecycle(conn, run_result, target_folder_path, goal_text): | 91 | def finish_task_lifecycle(conn, run_result, target_folder_path, goal_text): |
| 40 | """PASS -> `close_task_done`; FAIL/INCONCLUSIVE -> `org_level_loop. | 92 | """PASS -> `close_task_done`; FAIL/INCONCLUSIVE -> `org_level_loop. |
| @@ -66,11 +118,30 @@ def finish_task_lifecycle(conn, run_result, target_folder_path, goal_text): | @@ -66,11 +118,30 @@ def finish_task_lifecycle(conn, run_result, target_folder_path, goal_text): | ||
| 66 | def run_goal_to_completion(db_path, target_folder_path, goal_text): | 118 | def run_goal_to_completion(db_path, target_folder_path, goal_text): |
| 67 | """One Goal, its own sqlite connection, start to a terminal task.status | 119 | """One Goal, its own sqlite connection, start to a terminal task.status |
| 68 | (done or blocked_pending_human). This is the unit of work each scheduler | 120 | (done or blocked_pending_human). This is the unit of work each scheduler |
| 69 | - worker thread runs.""" | 121 | + worker thread runs. |
| 122 | + | ||
| 123 | + Creates the task itself (rather than delegating to e2e_pipeline. | ||
| 124 | + run_end_to_end) so task_id is in scope even if e2e_pipeline. | ||
| 125 | + run_task_lifecycle raises partway through -- needed to route | ||
| 126 | + CorruptPersistedStateError/ClassifierBlockedError/UsageLimitWaitTimeoutError | ||
| 127 | + to task.status='blocked_pending_human' (see module docstring's "Second | ||
| 128 | + gap" note). Any OTHER exception still propagates uncaught, same as | ||
| 129 | + before -- run_scheduler_once's own try/except is what keeps one Goal's | ||
| 130 | + unexpected failure from stopping the rest. | ||
| 131 | + """ | ||
| 70 | conn = schema.connect(db_path) | 132 | conn = schema.connect(db_path) |
| 71 | conn.execute("PRAGMA busy_timeout = {}".format(_BUSY_TIMEOUT_MS)) | 133 | conn.execute("PRAGMA busy_timeout = {}".format(_BUSY_TIMEOUT_MS)) |
| 72 | try: | 134 | try: |
| 73 | - result = e2e_pipeline.run_end_to_end(conn, target_folder_path, goal_text) | 135 | + project_id = conn.execute( |
| 136 | + "SELECT project_id FROM project WHERE target_folder_path = ?", (target_folder_path,) | ||
| 137 | + ).fetchone()[0] | ||
| 138 | + task_id = task_create.create_task(conn, project_id, goal_text)["task_id"] | ||
| 139 | + try: | ||
| 140 | + result = e2e_pipeline.run_task_lifecycle(conn, task_id, target_folder_path, goal_text) | ||
| 141 | + except _ROUTABLE_EXCEPTIONS as e: | ||
| 142 | + blocked_reason = _blocked_reason_for(e) | ||
| 143 | + _block_task(conn, task_id, blocked_reason) | ||
| 144 | + return {"task_id": task_id, "final_status": "blocked_pending_human", "blocked_reason": blocked_reason} | ||
| 74 | return finish_task_lifecycle(conn, result, target_folder_path, goal_text) | 145 | return finish_task_lifecycle(conn, result, target_folder_path, goal_text) |
| 75 | finally: | 146 | finally: |
| 76 | conn.close() | 147 | conn.close() |
src/test_backend_server.py
| @@ -536,6 +536,31 @@ def _read_http_response_head(sock): | @@ -536,6 +536,31 @@ def _read_http_response_head(sock): | ||
| 536 | return buf.decode("ascii", errors="replace") | 536 | return buf.decode("ascii", errors="replace") |
| 537 | 537 | ||
| 538 | 538 | ||
| 539 | +def _read_http_response_with_json_body(sock): | ||
| 540 | + """Like _read_http_response_head, but also reads and parses the JSON | ||
| 541 | + body (via Content-Length) -- for pre-upgrade WS rejections, which DO | ||
| 542 | + send a body (_send_json), unlike a 101 Switching Protocols response.""" | ||
| 543 | + buf = b"" | ||
| 544 | + while b"\r\n\r\n" not in buf: | ||
| 545 | + chunk = sock.recv(4096) | ||
| 546 | + if not chunk: | ||
| 547 | + break | ||
| 548 | + buf += chunk | ||
| 549 | + head, _, rest = buf.partition(b"\r\n\r\n") | ||
| 550 | + head_text = head.decode("ascii", errors="replace") | ||
| 551 | + content_length = 0 | ||
| 552 | + for line in head_text.splitlines(): | ||
| 553 | + if line.lower().startswith("content-length:"): | ||
| 554 | + content_length = int(line.split(":", 1)[1].strip()) | ||
| 555 | + break | ||
| 556 | + while len(rest) < content_length: | ||
| 557 | + chunk = sock.recv(4096) | ||
| 558 | + if not chunk: | ||
| 559 | + break | ||
| 560 | + rest += chunk | ||
| 561 | + return head_text, json.loads(rest.decode("utf-8")) | ||
| 562 | + | ||
| 563 | + | ||
| 539 | def _read_server_frame_raw(sock): | 564 | def _read_server_frame_raw(sock): |
| 540 | """Server->client frames are never masked (RFC 6455 §5.1) -- simpler | 565 | """Server->client frames are never masked (RFC 6455 §5.1) -- simpler |
| 541 | decode than ws_subscribe.decode_client_frame, which expects masking. | 566 | decode than ws_subscribe.decode_client_frame, which expects masking. |
| @@ -608,9 +633,10 @@ class WsSubscribeRouteTest(unittest.TestCase): | @@ -608,9 +633,10 @@ class WsSubscribeRouteTest(unittest.TestCase): | ||
| 608 | self.addCleanup(sock.close) | 633 | self.addCleanup(sock.close) |
| 609 | key = base64.b64encode(b"0123456789012345").decode("ascii") | 634 | key = base64.b64encode(b"0123456789012345").decode("ascii") |
| 610 | sock.sendall(_ws_handshake_request("/api/ws/projects/no-such-project", key)) | 635 | sock.sendall(_ws_handshake_request("/api/ws/projects/no-such-project", key)) |
| 611 | - head = _read_http_response_head(sock) | 636 | + head, body = _read_http_response_with_json_body(sock) |
| 612 | self.assertIn("404", head.splitlines()[0]) | 637 | self.assertIn("404", head.splitlines()[0]) |
| 613 | self.assertNotIn("101", head.splitlines()[0]) | 638 | self.assertNotIn("101", head.splitlines()[0]) |
| 639 | + self.assertEqual(body["error"], "PROJECT_NOT_FOUND") | ||
| 614 | 640 | ||
| 615 | def test_non_upgrade_request_rejected(self): | 641 | def test_non_upgrade_request_rejected(self): |
| 616 | sock = self._connect() | 642 | sock = self._connect() |
| @@ -780,8 +806,9 @@ class ExecutionLogRouteTest(unittest.TestCase): | @@ -780,8 +806,9 @@ class ExecutionLogRouteTest(unittest.TestCase): | ||
| 780 | self.addCleanup(sock.close) | 806 | self.addCleanup(sock.close) |
| 781 | key = base64.b64encode(b"0123456789012345").decode("ascii") | 807 | key = base64.b64encode(b"0123456789012345").decode("ascii") |
| 782 | sock.sendall(_ws_handshake_request("/api/ws/work-nodes/no-such-node/log", key)) | 808 | sock.sendall(_ws_handshake_request("/api/ws/work-nodes/no-such-node/log", key)) |
| 783 | - head = _read_http_response_head(sock) | 809 | + head, body = _read_http_response_with_json_body(sock) |
| 784 | self.assertIn("404", head.splitlines()[0]) | 810 | self.assertIn("404", head.splitlines()[0]) |
| 811 | + self.assertEqual(body["error"], "NODE_EXEC_NOT_FOUND") | ||
| 785 | 812 | ||
| 786 | def test_ws_pushes_appended_log_content(self): | 813 | def test_ws_pushes_appended_log_content(self): |
| 787 | sock = self._connect() | 814 | sock = self._connect() |
src/test_e2e_pipeline.py
| @@ -84,6 +84,7 @@ class PlanDocumentsTest(PipelineTestBase): | @@ -84,6 +84,7 @@ class PlanDocumentsTest(PipelineTestBase): | ||
| 84 | patch.object(work_graph_execute, "execute_work_graph") as mock_execute: | 84 | patch.object(work_graph_execute, "execute_work_graph") as mock_execute: |
| 85 | mock_execute.side_effect = [ | 85 | mock_execute.side_effect = [ |
| 86 | {"content": _FAKE_DOCS["brd"]}, {"content": _FAKE_DOCS["prd"]}, | 86 | {"content": _FAKE_DOCS["brd"]}, {"content": _FAKE_DOCS["prd"]}, |
| 87 | + {"consistent": True, "issues": []}, # PRD consistency check (REQ-02 boundary) | ||
| 87 | {"content": _FAKE_DOCS["sdd"]}, {"content": _FAKE_DOCS["benchmark"]}, | 88 | {"content": _FAKE_DOCS["sdd"]}, {"content": _FAKE_DOCS["benchmark"]}, |
| 88 | ] | 89 | ] |
| 89 | plan_result = e2e_pipeline.plan_documents(self.conn, "task-1", "实现贪吃蛇游戏", self.tmp.name) | 90 | plan_result = e2e_pipeline.plan_documents(self.conn, "task-1", "实现贪吃蛇游戏", self.tmp.name) |
| @@ -119,6 +120,7 @@ class RunEndToEndTest(PipelineTestBase): | @@ -119,6 +120,7 @@ class RunEndToEndTest(PipelineTestBase): | ||
| 119 | patch.object(org_node_trigger, "trigger_org_node", return_value="VERDICT: PASS\nAll scenarios pass."): | 120 | patch.object(org_node_trigger, "trigger_org_node", return_value="VERDICT: PASS\nAll scenarios pass."): |
| 120 | mock_execute.side_effect = [ | 121 | mock_execute.side_effect = [ |
| 121 | {"content": _FAKE_DOCS["brd"]}, {"content": _FAKE_DOCS["prd"]}, | 122 | {"content": _FAKE_DOCS["brd"]}, {"content": _FAKE_DOCS["prd"]}, |
| 123 | + {"consistent": True, "issues": []}, # PRD consistency check (REQ-02 boundary) | ||
| 122 | {"content": _FAKE_DOCS["sdd"]}, {"content": _FAKE_DOCS["benchmark"]}, | 124 | {"content": _FAKE_DOCS["sdd"]}, {"content": _FAKE_DOCS["benchmark"]}, |
| 123 | {"response": "Implemented. Ran `pytest`: 3 passed."}, | 125 | {"response": "Implemented. Ran `pytest`: 3 passed."}, |
| 124 | ] | 126 | ] |
| @@ -147,6 +149,59 @@ class RunEndToEndTest(PipelineTestBase): | @@ -147,6 +149,59 @@ class RunEndToEndTest(PipelineTestBase): | ||
| 147 | self.assertEqual(run_org_nodes, {"planner", "coder"}) | 149 | self.assertEqual(run_org_nodes, {"planner", "coder"}) |
| 148 | 150 | ||
| 149 | 151 | ||
| 152 | +class PrdConsistencyVerifyFnTest(unittest.TestCase): | ||
| 153 | + """REQ-02 boundary: only the PRD node gets the consistency check.""" | ||
| 154 | + | ||
| 155 | + def test_non_prd_node_passes_through_without_a_call(self): | ||
| 156 | + node_artifact_types = {"brd": "brd"} | ||
| 157 | + verify_fn = e2e_pipeline._prd_consistency_verify_fn(node_artifact_types, "/tmp/proj-1") | ||
| 158 | + with patch.object(work_graph_execute, "execute_work_graph") as mock_execute: | ||
| 159 | + ok, feedback = verify_fn({"node_id": "brd"}, {"content": "brd text"}) | ||
| 160 | + self.assertTrue(ok) | ||
| 161 | + self.assertIsNone(feedback) | ||
| 162 | + mock_execute.assert_not_called() | ||
| 163 | + | ||
| 164 | + def test_string_false_from_a_real_call_is_not_treated_as_truthy(self): | ||
| 165 | + # regression: a real call (2026-08-02) returned "consistent": "false" | ||
| 166 | + # as a JSON string, not a boolean -- `if "false":` is truthy in | ||
| 167 | + # Python, silently inverting the check until a real test caught it. | ||
| 168 | + node_artifact_types = {"prd": "prd"} | ||
| 169 | + verify_fn = e2e_pipeline._prd_consistency_verify_fn(node_artifact_types, "/tmp/proj-1") | ||
| 170 | + with patch.object(work_graph_execute, "write_workflow_script"), \ | ||
| 171 | + patch.object( | ||
| 172 | + work_graph_execute, "execute_work_graph", | ||
| 173 | + return_value={"consistent": "false", "issues": ["contradiction found"]}, | ||
| 174 | + ): | ||
| 175 | + ok, feedback = verify_fn({"node_id": "prd"}, {"content": "contradictory prd"}) | ||
| 176 | + self.assertFalse(ok) | ||
| 177 | + | ||
| 178 | + def test_string_true_from_a_real_call_is_treated_as_consistent(self): | ||
| 179 | + node_artifact_types = {"prd": "prd"} | ||
| 180 | + verify_fn = e2e_pipeline._prd_consistency_verify_fn(node_artifact_types, "/tmp/proj-1") | ||
| 181 | + with patch.object(work_graph_execute, "write_workflow_script"), \ | ||
| 182 | + patch.object( | ||
| 183 | + work_graph_execute, "execute_work_graph", | ||
| 184 | + return_value={"consistent": "true", "issues": []}, | ||
| 185 | + ): | ||
| 186 | + ok, feedback = verify_fn({"node_id": "prd"}, {"content": "consistent prd"}) | ||
| 187 | + self.assertTrue(ok) | ||
| 188 | + | ||
| 189 | + def test_prd_node_triggers_a_real_check_call(self): | ||
| 190 | + node_artifact_types = {"prd": "prd"} | ||
| 191 | + verify_fn = e2e_pipeline._prd_consistency_verify_fn(node_artifact_types, "/tmp/proj-1") | ||
| 192 | + with patch.object(work_graph_execute, "write_workflow_script"), \ | ||
| 193 | + patch.object( | ||
| 194 | + work_graph_execute, "execute_work_graph", | ||
| 195 | + return_value={"consistent": False, "issues": ["contradiction found"]}, | ||
| 196 | + ) as mock_execute: | ||
| 197 | + ok, feedback = verify_fn({"node_id": "prd"}, {"content": "contradictory prd"}) | ||
| 198 | + self.assertFalse(ok) | ||
| 199 | + self.assertEqual(feedback, ["contradiction found"]) | ||
| 200 | + mock_execute.assert_called_once() | ||
| 201 | + args = mock_execute.call_args[0] | ||
| 202 | + self.assertEqual(args[0], "/tmp/proj-1") | ||
| 203 | + | ||
| 204 | + | ||
| 150 | class ValidateImplementationTest(unittest.TestCase): | 205 | class ValidateImplementationTest(unittest.TestCase): |
| 151 | def test_parses_pass_verdict(self): | 206 | def test_parses_pass_verdict(self): |
| 152 | with patch("src.org_node_trigger.trigger_org_node", return_value="VERDICT: PASS\nlooks good"): | 207 | with patch("src.org_node_trigger.trigger_org_node", return_value="VERDICT: PASS\nlooks good"): |
src/test_e2e_pipeline_e2e.py
| @@ -53,5 +53,45 @@ class RealPlanDocumentsTest(unittest.TestCase): | @@ -53,5 +53,45 @@ class RealPlanDocumentsTest(unittest.TestCase): | ||
| 53 | self.assertEqual(prd_deps, [artifact_ids["brd"]]) | 53 | self.assertEqual(prd_deps, [artifact_ids["brd"]]) |
| 54 | 54 | ||
| 55 | 55 | ||
| 56 | +@skip_unless_env("RUN_REAL_AGENT_E2E") | ||
| 57 | +class RealPrdConsistencyCheckTest(unittest.TestCase): | ||
| 58 | + """REQ-02 boundary, real coverage (2026-08-02): the new _check_prd_consistency | ||
| 59 | + call is a genuinely new LLM-judgment capability, not a wiring change -- | ||
| 60 | + validated for real once each way (contradiction / no contradiction), | ||
| 61 | + same discipline as every other new agent() call this project has added.""" | ||
| 62 | + | ||
| 63 | + def test_detects_a_real_deliberate_contradiction(self): | ||
| 64 | + with tempfile.TemporaryDirectory() as tmp: | ||
| 65 | + subprocess.run(["git", "init"], cwd=tmp, check=True, capture_output=True) | ||
| 66 | + agent_profile.deploy_agent_definitions(tmp) | ||
| 67 | + contradictory_prd = ( | ||
| 68 | + "# PRD: Snake Game\n\n" | ||
| 69 | + "## Wall Collision\n" | ||
| 70 | + "Requirement 3.1: When the snake's head touches the board boundary, the game MUST " | ||
| 71 | + "immediately end with a 'Game Over' screen.\n\n" | ||
| 72 | + "Requirement 3.2: The snake must be able to pass through the board boundary and " | ||
| 73 | + "wrap around to the opposite side, continuing play without ending the game.\n" | ||
| 74 | + ) | ||
| 75 | + ok, issues = e2e_pipeline._check_prd_consistency(tmp, contradictory_prd) | ||
| 76 | + self.assertFalse(ok) | ||
| 77 | + self.assertTrue(len(issues) > 0) | ||
| 78 | + | ||
| 79 | + def test_passes_a_real_consistent_prd(self): | ||
| 80 | + with tempfile.TemporaryDirectory() as tmp: | ||
| 81 | + subprocess.run(["git", "init"], cwd=tmp, check=True, capture_output=True) | ||
| 82 | + agent_profile.deploy_agent_definitions(tmp) | ||
| 83 | + consistent_prd = ( | ||
| 84 | + "# PRD: Snake Game\n\n" | ||
| 85 | + "## Wall Collision\n" | ||
| 86 | + "Requirement 3.1: When the snake's head touches the board boundary, the game " | ||
| 87 | + "immediately ends with a 'Game Over' screen showing the final score.\n\n" | ||
| 88 | + "## Food\n" | ||
| 89 | + "Requirement 4.1: Eating food grows the snake by one segment and spawns a new " | ||
| 90 | + "food item at a random empty cell.\n" | ||
| 91 | + ) | ||
| 92 | + ok, issues = e2e_pipeline._check_prd_consistency(tmp, consistent_prd) | ||
| 93 | + self.assertTrue(ok, "false positive on a consistent PRD: {}".format(issues)) | ||
| 94 | + | ||
| 95 | + | ||
| 56 | if __name__ == "__main__": | 96 | if __name__ == "__main__": |
| 57 | unittest.main() | 97 | unittest.main() |
src/test_scheduler.py
| @@ -17,7 +17,7 @@ import time | @@ -17,7 +17,7 @@ import time | ||
| 17 | import unittest | 17 | import unittest |
| 18 | from unittest.mock import patch | 18 | from unittest.mock import patch |
| 19 | 19 | ||
| 20 | -from src import org_level_loop, org_loop_route, schema, scheduler, settings_config | 20 | +from src import org_level_loop, org_loop_route, schema, scheduler, settings_config, work_graph_execute, work_node_run |
| 21 | 21 | ||
| 22 | 22 | ||
| 23 | class FinishTaskLifecycleTestBase(unittest.TestCase): | 23 | class FinishTaskLifecycleTestBase(unittest.TestCase): |
| @@ -178,13 +178,12 @@ class ConcurrentSqliteWritesStressTest(unittest.TestCase): | @@ -178,13 +178,12 @@ class ConcurrentSqliteWritesStressTest(unittest.TestCase): | ||
| 178 | self.addCleanup(lambda: os.path.exists(self.db_path) and os.remove(self.db_path)) | 178 | self.addCleanup(lambda: os.path.exists(self.db_path) and os.remove(self.db_path)) |
| 179 | 179 | ||
| 180 | def test_many_concurrent_goals_write_to_the_same_db_file_without_locking_errors(self): | 180 | def test_many_concurrent_goals_write_to_the_same_db_file_without_locking_errors(self): |
| 181 | - def fake_run_end_to_end(conn, target_folder_path, goal_text): | ||
| 182 | - task_id = goal_text # unique per goal, stands in for a real uuid | ||
| 183 | - conn.execute( | ||
| 184 | - "INSERT INTO task (task_id, project_id, goal_text, task_type, status, assigned_org_node) " | ||
| 185 | - "VALUES (?, 'proj-1', ?, 'goal', 'validating', 'validator')", | ||
| 186 | - (task_id, goal_text), | ||
| 187 | - ) | 181 | + def fake_run_task_lifecycle(conn, task_id, target_folder_path, goal_text): |
| 182 | + # task_id already exists -- run_goal_to_completion creates it for | ||
| 183 | + # real via task_create.create_task before calling this; stand in | ||
| 184 | + # for org_node_lifecycle's real queued->...->validating march so | ||
| 185 | + # finish_task_lifecycle's close_task_done (requires 'validating') works. | ||
| 186 | + conn.execute("UPDATE task SET status = 'validating' WHERE task_id = ?", (task_id,)) | ||
| 188 | conn.commit() | 187 | conn.commit() |
| 189 | time.sleep(0.02) # widen the contention window | 188 | time.sleep(0.02) # widen the contention window |
| 190 | return { | 189 | return { |
| @@ -193,7 +192,7 @@ class ConcurrentSqliteWritesStressTest(unittest.TestCase): | @@ -193,7 +192,7 @@ class ConcurrentSqliteWritesStressTest(unittest.TestCase): | ||
| 193 | } | 192 | } |
| 194 | 193 | ||
| 195 | goals = [("/tmp/proj-1", "goal-{}".format(i)) for i in range(10)] | 194 | goals = [("/tmp/proj-1", "goal-{}".format(i)) for i in range(10)] |
| 196 | - with patch.object(scheduler.e2e_pipeline, "run_end_to_end", side_effect=fake_run_end_to_end): | 195 | + with patch.object(scheduler.e2e_pipeline, "run_task_lifecycle", side_effect=fake_run_task_lifecycle): |
| 197 | results = scheduler.run_scheduler_once(self.db_path, goals, max_workers=5) | 196 | results = scheduler.run_scheduler_once(self.db_path, goals, max_workers=5) |
| 198 | 197 | ||
| 199 | self.assertTrue(all("result" in r for r in results), results) | 198 | self.assertTrue(all("result" in r for r in results), results) |
| @@ -204,5 +203,158 @@ class ConcurrentSqliteWritesStressTest(unittest.TestCase): | @@ -204,5 +203,158 @@ class ConcurrentSqliteWritesStressTest(unittest.TestCase): | ||
| 204 | self.assertEqual(done_count, 10) | 203 | self.assertEqual(done_count, 10) |
| 205 | 204 | ||
| 206 | 205 | ||
| 206 | +class RoutableExceptionTestBase(unittest.TestCase): | ||
| 207 | + """REQ-06/07/14 failure: work-node-level failures (CorruptPersistedState | ||
| 208 | + /ClassifierBlocked/UsageLimitWaitTimeout) must actually reach | ||
| 209 | + task.status='blocked_pending_human' with the matching blocked_reason -- | ||
| 210 | + found missing during the PH4-closure audit (2026-08-02): each exception | ||
| 211 | + was correctly raised and correctly marked its own work_node_execution | ||
| 212 | + 'blocked', but nothing ever caught it to apply the task-level | ||
| 213 | + consequence 06's own acceptance text names.""" | ||
| 214 | + | ||
| 215 | + def setUp(self): | ||
| 216 | + self.tmp = tempfile.NamedTemporaryFile(suffix=".db", delete=False) | ||
| 217 | + self.tmp.close() | ||
| 218 | + self.db_path = self.tmp.name | ||
| 219 | + conn = schema.connect(self.db_path) | ||
| 220 | + schema.create_schema(conn) | ||
| 221 | + conn.execute( | ||
| 222 | + "INSERT INTO project (project_id, name, target_folder_path, has_git_repo, status) " | ||
| 223 | + "VALUES ('proj-1', 'P', '/tmp/proj-1', 1, 'active')" | ||
| 224 | + ) | ||
| 225 | + conn.commit() | ||
| 226 | + conn.close() | ||
| 227 | + self.addCleanup(lambda: os.path.exists(self.db_path) and os.remove(self.db_path)) | ||
| 228 | + | ||
| 229 | + def _run_with_raising_lifecycle(self, exc): | ||
| 230 | + def fake_run_task_lifecycle(conn, task_id, target_folder_path, goal_text): | ||
| 231 | + raise exc | ||
| 232 | + | ||
| 233 | + with patch.object(scheduler.e2e_pipeline, "run_task_lifecycle", side_effect=fake_run_task_lifecycle): | ||
| 234 | + result = scheduler.run_goal_to_completion(self.db_path, "/tmp/proj-1", "goal text") | ||
| 235 | + return result | ||
| 236 | + | ||
| 237 | + def _assert_task_blocked(self, task_id, blocked_reason): | ||
| 238 | + conn = schema.connect(self.db_path) | ||
| 239 | + row = conn.execute("SELECT status, blocked_reason FROM task WHERE task_id = ?", (task_id,)).fetchone() | ||
| 240 | + event = conn.execute( | ||
| 241 | + "SELECT event_type FROM execution_event WHERE task_id = ? ORDER BY rowid DESC LIMIT 1", (task_id,) | ||
| 242 | + ).fetchone() | ||
| 243 | + conn.close() | ||
| 244 | + self.assertEqual(row, ("blocked_pending_human", blocked_reason)) | ||
| 245 | + self.assertEqual(event[0], "task_status_changed") | ||
| 246 | + | ||
| 247 | + | ||
| 248 | +class CorruptPersistedStateRoutingTest(RoutableExceptionTestBase): | ||
| 249 | + def test_routes_to_blocked_pending_human(self): | ||
| 250 | + result = self._run_with_raising_lifecycle(work_node_run.CorruptPersistedStateError("boom")) | ||
| 251 | + self.assertEqual(result["final_status"], "blocked_pending_human") | ||
| 252 | + self.assertEqual(result["blocked_reason"], "corrupt_persisted_state") | ||
| 253 | + self._assert_task_blocked(result["task_id"], "corrupt_persisted_state") | ||
| 254 | + | ||
| 255 | + | ||
| 256 | +class ClassifierBlockedRoutingTest(RoutableExceptionTestBase): | ||
| 257 | + def test_routes_to_blocked_pending_human(self): | ||
| 258 | + result = self._run_with_raising_lifecycle(work_graph_execute.ClassifierBlockedError("boom")) | ||
| 259 | + self.assertEqual(result["final_status"], "blocked_pending_human") | ||
| 260 | + self.assertEqual(result["blocked_reason"], "classifier_blocked") | ||
| 261 | + self._assert_task_blocked(result["task_id"], "classifier_blocked") | ||
| 262 | + | ||
| 263 | + | ||
| 264 | +class UsageLimitWaitTimeoutRoutingTest(RoutableExceptionTestBase): | ||
| 265 | + def test_routes_to_blocked_pending_human(self): | ||
| 266 | + result = self._run_with_raising_lifecycle(work_node_run.UsageLimitWaitTimeoutError("boom")) | ||
| 267 | + self.assertEqual(result["final_status"], "blocked_pending_human") | ||
| 268 | + self.assertEqual(result["blocked_reason"], "usage_limit_wait_timeout") | ||
| 269 | + self._assert_task_blocked(result["task_id"], "usage_limit_wait_timeout") | ||
| 270 | + | ||
| 271 | + | ||
| 272 | +class ConsistencyCheckFailedRoutingTest(RoutableExceptionTestBase): | ||
| 273 | + def test_routes_to_blocked_pending_human(self): | ||
| 274 | + result = self._run_with_raising_lifecycle( | ||
| 275 | + scheduler.work_graph_orchestrate.ConsistencyCheckFailedError("boom") | ||
| 276 | + ) | ||
| 277 | + self.assertEqual(result["final_status"], "blocked_pending_human") | ||
| 278 | + self.assertEqual(result["blocked_reason"], "prd_consistency_check_failed") | ||
| 279 | + self._assert_task_blocked(result["task_id"], "prd_consistency_check_failed") | ||
| 280 | + | ||
| 281 | + | ||
| 282 | +class UnroutedExceptionStillPropagatesTest(RoutableExceptionTestBase): | ||
| 283 | + def test_other_exceptions_are_not_swallowed(self): | ||
| 284 | + with self.assertRaises(RuntimeError): | ||
| 285 | + self._run_with_raising_lifecycle(RuntimeError("something else entirely")) | ||
| 286 | + | ||
| 287 | + | ||
| 288 | +def _fix_loop_trigger(org_node, prompt, target_folder_path, timeout=300): | ||
| 289 | + """org_node_trigger.trigger_org_node fake for org_level_loop.run_fix_loop's | ||
| 290 | + 3 perspective calls (all agree 'coder') + fixer + verify -- verify always | ||
| 291 | + fails, so route_org_loop keeps incrementing loop_round on retries.""" | ||
| 292 | + if "CATEGORY:" in prompt: | ||
| 293 | + return "CATEGORY: coder\nDETAIL: because reasons" | ||
| 294 | + if org_node == "coder": | ||
| 295 | + return "Fixed it." | ||
| 296 | + if org_node == "validator": | ||
| 297 | + return "VERDICT: FAIL\nstill broken" | ||
| 298 | + raise AssertionError("unexpected call: " + org_node) | ||
| 299 | + | ||
| 300 | + | ||
| 301 | +class SchedulerContinuesPastBlockedTaskTest(unittest.TestCase): | ||
| 302 | + """REQ-05 failure / REQ-09 boundary, against the REAL scheduler (U-032) -- | ||
| 303 | + test_org_loop_route.py's own test for this literally says "no scheduler | ||
| 304 | + exists yet to test against directly" (stale as of U-032). One Goal's | ||
| 305 | + task is pre-seeded at loop_round=3 so its fix-loop attempt hits | ||
| 306 | + org_loop_route.LoopLimitReachedError for real (route_org_loop's own real | ||
| 307 | + blocked_pending_human write, not a mock) while the others complete via | ||
| 308 | + close_task_done -- proving the scheduler doesn't stop serving the rest | ||
| 309 | + of the batch.""" | ||
| 310 | + | ||
| 311 | + def setUp(self): | ||
| 312 | + self.tmp = tempfile.NamedTemporaryFile(suffix=".db", delete=False) | ||
| 313 | + self.tmp.close() | ||
| 314 | + self.db_path = self.tmp.name | ||
| 315 | + conn = schema.connect(self.db_path) | ||
| 316 | + schema.create_schema(conn) | ||
| 317 | + conn.execute( | ||
| 318 | + "INSERT INTO project (project_id, name, target_folder_path, has_git_repo, status) " | ||
| 319 | + "VALUES ('proj-1', 'P', '/tmp/proj-1', 1, 'active')" | ||
| 320 | + ) | ||
| 321 | + conn.commit() | ||
| 322 | + conn.close() | ||
| 323 | + self.addCleanup(lambda: os.path.exists(self.db_path) and os.remove(self.db_path)) | ||
| 324 | + | ||
| 325 | + def test_one_task_hitting_loop_limit_does_not_block_the_others(self): | ||
| 326 | + def fake_run_task_lifecycle(conn, task_id, target_folder_path, goal_text): | ||
| 327 | + conn.execute("UPDATE task SET status = 'validating' WHERE task_id = ?", (task_id,)) | ||
| 328 | + conn.commit() | ||
| 329 | + if goal_text == "goal-doomed": | ||
| 330 | + conn.execute("UPDATE task SET loop_round = 3 WHERE task_id = ?", (task_id,)) | ||
| 331 | + conn.commit() | ||
| 332 | + verdict = {"verdict": "FAIL", "raw_response": "VERDICT: FAIL\nstill broken"} | ||
| 333 | + else: | ||
| 334 | + verdict = {"verdict": "PASS", "raw_response": "VERDICT: PASS"} | ||
| 335 | + return { | ||
| 336 | + "task_id": task_id, "doc_artifact_ids": {"benchmark": "artifact-b"}, | ||
| 337 | + "code_artifact_ids": {}, "verdict": verdict, | ||
| 338 | + } | ||
| 339 | + | ||
| 340 | + goals = [("/tmp/proj-1", "goal-ok-1"), ("/tmp/proj-1", "goal-doomed"), ("/tmp/proj-1", "goal-ok-2")] | ||
| 341 | + with patch.object(scheduler.e2e_pipeline, "run_task_lifecycle", side_effect=fake_run_task_lifecycle), \ | ||
| 342 | + patch.object(scheduler.e2e_pipeline, "_read_artifact_content", return_value="benchmark content"), \ | ||
| 343 | + patch("src.org_node_trigger.trigger_org_node", side_effect=_fix_loop_trigger): | ||
| 344 | + results = scheduler.run_scheduler_once(self.db_path, goals, max_workers=3) | ||
| 345 | + | ||
| 346 | + by_goal = {r["goal_text"]: r for r in results} | ||
| 347 | + self.assertEqual(by_goal["goal-ok-1"]["result"]["final_status"], "done") | ||
| 348 | + self.assertEqual(by_goal["goal-ok-2"]["result"]["final_status"], "done") | ||
| 349 | + self.assertIn("error", by_goal["goal-doomed"]) | ||
| 350 | + | ||
| 351 | + conn = schema.connect(self.db_path) | ||
| 352 | + doomed_status = conn.execute( | ||
| 353 | + "SELECT status, blocked_reason FROM task WHERE goal_text = 'goal-doomed'" | ||
| 354 | + ).fetchone() | ||
| 355 | + conn.close() | ||
| 356 | + self.assertEqual(doomed_status, ("blocked_pending_human", "loop_limit_reached")) | ||
| 357 | + | ||
| 358 | + | ||
| 207 | if __name__ == "__main__": | 359 | if __name__ == "__main__": |
| 208 | unittest.main() | 360 | unittest.main() |
src/test_work_graph_orchestrate.py
| @@ -208,5 +208,61 @@ class RunGraphClassifierBlockedTest(RunGraphTestBase): | @@ -208,5 +208,61 @@ class RunGraphClassifierBlockedTest(RunGraphTestBase): | ||
| 208 | self.assertEqual(status, "blocked") | 208 | self.assertEqual(status, "blocked") |
| 209 | 209 | ||
| 210 | 210 | ||
| 211 | +class VerifyFnTest(RunGraphTestBase): | ||
| 212 | + """REQ-02 boundary: verify_fn intercepts a bad node output before it | ||
| 213 | + propagates downstream, via one regeneration attempt.""" | ||
| 214 | + | ||
| 215 | + def test_verify_fn_passing_immediately_does_not_regenerate(self): | ||
| 216 | + manifest = {"nodes": [{"node_id": "prd", "depends_on": []}], "edges": []} | ||
| 217 | + with patch.object(work_graph_execute, "write_workflow_script"), \ | ||
| 218 | + patch.object(work_graph_execute, "execute_work_graph", return_value={"content": "consistent prd"}) as mock_execute: | ||
| 219 | + outputs = wgo.run_graph( | ||
| 220 | + self.conn, "run-1", "planner", "planner-doc-writing", "/tmp/proj-1", manifest, | ||
| 221 | + lambda node, outputs: "p", _IN_SCHEMA, _OUT_SCHEMA, | ||
| 222 | + verify_fn=lambda node, output: (True, None), | ||
| 223 | + ) | ||
| 224 | + self.assertEqual(outputs["prd"], {"content": "consistent prd"}) | ||
| 225 | + self.assertEqual(mock_execute.call_count, 1) | ||
| 226 | + run_status = self.conn.execute("SELECT status FROM work_graph_run WHERE run_id = 'run-1'").fetchone()[0] | ||
| 227 | + self.assertEqual(run_status, "running") # untouched -- verify_fn never failed | ||
| 228 | + | ||
| 229 | + def test_verify_fn_failing_once_regenerates_and_succeeds(self): | ||
| 230 | + manifest = {"nodes": [{"node_id": "prd", "depends_on": []}], "edges": []} | ||
| 231 | + contents = iter(["contradictory prd", "fixed prd"]) | ||
| 232 | + checked = [] | ||
| 233 | + | ||
| 234 | + def verify_fn(node, output): | ||
| 235 | + checked.append(output["content"]) | ||
| 236 | + return output["content"] == "fixed prd", ["scenario A contradicts scenario B"] | ||
| 237 | + | ||
| 238 | + with patch.object(work_graph_execute, "write_workflow_script"), \ | ||
| 239 | + patch.object(work_graph_execute, "execute_work_graph", side_effect=lambda *a, **k: {"content": next(contents)}): | ||
| 240 | + outputs = wgo.run_graph( | ||
| 241 | + self.conn, "run-1", "planner", "planner-doc-writing", "/tmp/proj-1", manifest, | ||
| 242 | + lambda node, outputs: "p", _IN_SCHEMA, _OUT_SCHEMA, verify_fn=verify_fn, | ||
| 243 | + ) | ||
| 244 | + self.assertEqual(outputs["prd"], {"content": "fixed prd"}) | ||
| 245 | + self.assertEqual(checked, ["contradictory prd", "fixed prd"]) | ||
| 246 | + run_status = self.conn.execute("SELECT status FROM work_graph_run WHERE run_id = 'run-1'").fetchone()[0] | ||
| 247 | + self.assertEqual(run_status, "verifying") | ||
| 248 | + reflect_row = self.conn.execute( | ||
| 249 | + "SELECT status FROM work_node_execution WHERE run_id = 'run-1' AND node_id = 'prd-reflect'" | ||
| 250 | + ).fetchone() | ||
| 251 | + self.assertEqual(reflect_row[0], "done") | ||
| 252 | + | ||
| 253 | + def test_verify_fn_failing_twice_raises_and_blocks_run(self): | ||
| 254 | + manifest = {"nodes": [{"node_id": "prd", "depends_on": []}], "edges": []} | ||
| 255 | + with patch.object(work_graph_execute, "write_workflow_script"), \ | ||
| 256 | + patch.object(work_graph_execute, "execute_work_graph", return_value={"content": "still contradictory"}): | ||
| 257 | + with self.assertRaises(wgo.ConsistencyCheckFailedError): | ||
| 258 | + wgo.run_graph( | ||
| 259 | + self.conn, "run-1", "planner", "planner-doc-writing", "/tmp/proj-1", manifest, | ||
| 260 | + lambda node, outputs: "p", _IN_SCHEMA, _OUT_SCHEMA, | ||
| 261 | + verify_fn=lambda node, output: (False, ["still broken"]), | ||
| 262 | + ) | ||
| 263 | + run_status = self.conn.execute("SELECT status FROM work_graph_run WHERE run_id = 'run-1'").fetchone()[0] | ||
| 264 | + self.assertEqual(run_status, "blocked") | ||
| 265 | + | ||
| 266 | + | ||
| 211 | if __name__ == "__main__": | 267 | if __name__ == "__main__": |
| 212 | unittest.main() | 268 | unittest.main() |
src/test_work_node_run.py
| @@ -147,6 +147,33 @@ class RetryOnSchemaFailureTest(WorkNodeRunTestBase): | @@ -147,6 +147,33 @@ class RetryOnSchemaFailureTest(WorkNodeRunTestBase): | ||
| 147 | ).fetchone() | 147 | ).fetchone() |
| 148 | self.assertEqual(row, ("done", 2)) | 148 | self.assertEqual(row, ("done", 2)) |
| 149 | 149 | ||
| 150 | + def test_schema_failure_passes_through_reflecting_before_retrying(self): | ||
| 151 | + # REQ-03 failure: "status 应为 reflecting->retrying" -- previously | ||
| 152 | + # jumped straight to 'retrying', 'reflecting' was never written. | ||
| 153 | + # execution_log_file/work_node_execution have no separate history | ||
| 154 | + # table, so the only way to observe the transient write is sqlite's | ||
| 155 | + # own trace callback (every SQL statement it actually executes). | ||
| 156 | + executed_sql = [] | ||
| 157 | + self.conn.set_trace_callback(executed_sql.append) | ||
| 158 | + | ||
| 159 | + attempts = {"n": 0} | ||
| 160 | + | ||
| 161 | + def run_fn(cwd, model=None): | ||
| 162 | + attempts["n"] += 1 | ||
| 163 | + if attempts["n"] == 1: | ||
| 164 | + return {"wrong_key": True} | ||
| 165 | + return {"value": "ok"} | ||
| 166 | + | ||
| 167 | + try: | ||
| 168 | + work_node_run.run_work_node( | ||
| 169 | + self.conn, "node-1", {"goal": "x"}, _IN_SCHEMA, _OUT_SCHEMA, run_fn=run_fn, max_retries=2 | ||
| 170 | + ) | ||
| 171 | + finally: | ||
| 172 | + self.conn.set_trace_callback(None) | ||
| 173 | + | ||
| 174 | + reflecting_writes = [sql for sql in executed_sql if "status = 'reflecting'" in sql] | ||
| 175 | + self.assertEqual(len(reflecting_writes), 1) | ||
| 176 | + | ||
| 150 | def test_exceeding_max_retries_blocks_the_node(self): | 177 | def test_exceeding_max_retries_blocks_the_node(self): |
| 151 | with self.assertRaises(work_node_run.SchemaValidationFailedError): | 178 | with self.assertRaises(work_node_run.SchemaValidationFailedError): |
| 152 | work_node_run.run_work_node( | 179 | work_node_run.run_work_node( |
src/work_graph_orchestrate.py
| @@ -23,6 +23,22 @@ Validated for real (2026-08-01): a one-node workflow script with a custom | @@ -23,6 +23,22 @@ Validated for real (2026-08-01): a one-node workflow script with a custom | ||
| 23 | `agentType` + `schema` genuinely loads the target org_node's migrated | 23 | `agentType` + `schema` genuinely loads the target org_node's migrated |
| 24 | persona (confirmed via a real call asking the agent to self-identify) and | 24 | persona (confirmed via a real call asking the agent to self-identify) and |
| 25 | returns real `structured_output` -- see 05 §5.4.1.5. | 25 | returns real `structured_output` -- see 05 §5.4.1.5. |
| 26 | + | ||
| 27 | +**`verify_fn` (2026-08-02, PH4-closure audit before starting PH5)**: closes | ||
| 28 | +REQ-02's boundary case (PRD internal-contradiction check must intercept | ||
| 29 | +before `verifying`, not silently pass) -- found genuinely missing | ||
| 30 | +(`artifact_write.write_artifact` always marks `validation_status='passed'` | ||
| 31 | +unconditionally, and no Unit ever built a consistency-judgment call). | ||
| 32 | +`work_graph_run.status` was ALSO never once written past its initial | ||
| 33 | +'planning' value anywhere in this codebase before this -- `verify_fn`'s | ||
| 34 | +'reflecting'/'verifying'/'blocked' writes below are the first real use of | ||
| 35 | +that enum. Scoped deliberately narrow: one optional per-node verify | ||
| 36 | +callback, one regeneration attempt on failure (matching 04-P's "回灌受影响 | ||
| 37 | +的最小子图" -- for a single content-producing node, the minimal affected | ||
| 38 | +subgraph IS just itself), escalate to `work_graph_run.status='blocked'` + | ||
| 39 | +raise if the regenerated content still fails. Not a general multi-node | ||
| 40 | +re-planning mechanism -- that would be new, unscoped complexity 04-P itself | ||
| 41 | +doesn't describe either. | ||
| 26 | """ | 42 | """ |
| 27 | 43 | ||
| 28 | import json | 44 | import json |
| @@ -32,6 +48,12 @@ import uuid | @@ -32,6 +48,12 @@ import uuid | ||
| 32 | from src import work_graph_execute, work_node_run | 48 | from src import work_graph_execute, work_node_run |
| 33 | 49 | ||
| 34 | 50 | ||
| 51 | +class ConsistencyCheckFailedError(Exception): | ||
| 52 | + """`verify_fn` rejected a node's output twice in a row (original + | ||
| 53 | + one regeneration attempt) -- `work_graph_run.status` is already | ||
| 54 | + 'blocked' by the time this is raised.""" | ||
| 55 | + | ||
| 56 | + | ||
| 35 | def topological_order(graph_manifest): | 57 | def topological_order(graph_manifest): |
| 36 | """Kahn's algorithm. Dependencies are read from BOTH `node['depends_on']` | 58 | """Kahn's algorithm. Dependencies are read from BOTH `node['depends_on']` |
| 37 | and `graph_manifest['edges']` and merged -- `plan_work_graph()`'s own | 59 | and `graph_manifest['edges']` and merged -- `plan_work_graph()`'s own |
| @@ -136,7 +158,7 @@ def _make_run_fn(target_folder_path, node_exec_id, org_node, prompt, output_sche | @@ -136,7 +158,7 @@ def _make_run_fn(target_folder_path, node_exec_id, org_node, prompt, output_sche | ||
| 136 | def run_graph( | 158 | def run_graph( |
| 137 | conn, run_id, org_node, skill_id, target_folder_path, graph_manifest, | 159 | conn, run_id, org_node, skill_id, target_folder_path, graph_manifest, |
| 138 | build_prompt, input_schema, output_schema, timeout=600, needs_worktree=False, | 160 | build_prompt, input_schema, output_schema, timeout=600, needs_worktree=False, |
| 139 | - sleep=time.sleep, now=time.time, | 161 | + verify_fn=None, sleep=time.sleep, now=time.time, |
| 140 | ): | 162 | ): |
| 141 | """Runs every node in `graph_manifest` (topologically ordered) through | 163 | """Runs every node in `graph_manifest` (topologically ordered) through |
| 142 | `run_work_node()`, each backed by a real one-node dynamic-workflow | 164 | `run_work_node()`, each backed by a real one-node dynamic-workflow |
| @@ -159,6 +181,17 @@ def run_graph( | @@ -159,6 +181,17 @@ def run_graph( | ||
| 159 | writing code (Coder) should pass this; callers only producing document | 181 | writing code (Coder) should pass this; callers only producing document |
| 160 | content (Planner) don't need it. | 182 | content (Planner) don't need it. |
| 161 | 183 | ||
| 184 | + `verify_fn(node, output) -> (ok: bool, feedback)`, if given, is called | ||
| 185 | + right after each node completes, BEFORE moving on to nodes that depend | ||
| 186 | + on it -- lets a caller intercept a bad output before it propagates | ||
| 187 | + downstream (e.g. REQ-02's PRD-internal-contradiction check), not just | ||
| 188 | + after the fact. On `ok=False`: `work_graph_run.status` -> 'reflecting', | ||
| 189 | + the SAME node is regenerated once with `feedback` appended to its | ||
| 190 | + prompt, and re-verified. If it fails again: `work_graph_run.status` | ||
| 191 | + -> 'blocked' and `ConsistencyCheckFailedError` is raised. If it now | ||
| 192 | + passes: `work_graph_run.status` -> 'verifying' and the regenerated | ||
| 193 | + output replaces the original in `outputs`. | ||
| 194 | + | ||
| 162 | Returns {node_id: output_payload}. | 195 | Returns {node_id: output_payload}. |
| 163 | """ | 196 | """ |
| 164 | ensure_placeholder_skill(conn, skill_id, org_node) | 197 | ensure_placeholder_skill(conn, skill_id, org_node) |
| @@ -178,5 +211,53 @@ def run_graph( | @@ -178,5 +211,53 @@ def run_graph( | ||
| 178 | needs_worktree=needs_worktree, repo_path=target_folder_path, | 211 | needs_worktree=needs_worktree, repo_path=target_folder_path, |
| 179 | sleep=sleep, now=now, | 212 | sleep=sleep, now=now, |
| 180 | ) | 213 | ) |
| 214 | + | ||
| 215 | + if verify_fn is not None: | ||
| 216 | + output = _verify_and_maybe_regenerate( | ||
| 217 | + conn, run_id, org_node, skill_id, target_folder_path, node, node_id, prompt, | ||
| 218 | + output, input_schema, output_schema, timeout, needs_worktree, verify_fn, sleep, now, | ||
| 219 | + ) | ||
| 220 | + | ||
| 181 | outputs[node_id] = output | 221 | outputs[node_id] = output |
| 182 | return outputs | 222 | return outputs |
| 223 | + | ||
| 224 | + | ||
| 225 | +def _verify_and_maybe_regenerate( | ||
| 226 | + conn, run_id, org_node, skill_id, target_folder_path, node, node_id, prompt, | ||
| 227 | + output, input_schema, output_schema, timeout, needs_worktree, verify_fn, sleep, now, | ||
| 228 | +): | ||
| 229 | + ok, feedback = verify_fn(node, output) | ||
| 230 | + if ok: | ||
| 231 | + return output | ||
| 232 | + | ||
| 233 | + conn.execute("UPDATE work_graph_run SET status = 'reflecting' WHERE run_id = ?", (run_id,)) | ||
| 234 | + conn.commit() | ||
| 235 | + | ||
| 236 | + reflect_node_id = node_id + "-reflect" | ||
| 237 | + reflect_node_exec_id = _ensure_node_row( | ||
| 238 | + conn, run_id, reflect_node_id, org_node, skill_id, | ||
| 239 | + "work_graph_node_input.schema.json", "work_graph_node_output.schema.json", | ||
| 240 | + {"node_id": node_id, "feedback": feedback}, | ||
| 241 | + ) | ||
| 242 | + reflect_prompt = ( | ||
| 243 | + prompt + "\n\nA consistency reviewer flagged issues with your previous draft: " | ||
| 244 | + + "; ".join(feedback or []) + "\nProduce a corrected version resolving these issues." | ||
| 245 | + ) | ||
| 246 | + reflect_run_fn = _make_run_fn(target_folder_path, reflect_node_exec_id, org_node, reflect_prompt, output_schema, timeout) | ||
| 247 | + regenerated = work_node_run.run_work_node( | ||
| 248 | + conn, reflect_node_exec_id, {"node_id": node_id}, input_schema, output_schema, reflect_run_fn, | ||
| 249 | + needs_worktree=needs_worktree, repo_path=target_folder_path, | ||
| 250 | + sleep=sleep, now=now, | ||
| 251 | + ) | ||
| 252 | + | ||
| 253 | + ok2, feedback2 = verify_fn(node, regenerated) | ||
| 254 | + if not ok2: | ||
| 255 | + conn.execute("UPDATE work_graph_run SET status = 'blocked' WHERE run_id = ?", (run_id,)) | ||
| 256 | + conn.commit() | ||
| 257 | + raise ConsistencyCheckFailedError( | ||
| 258 | + "node {} still failed verification after one regeneration attempt: {}".format(node_id, feedback2) | ||
| 259 | + ) | ||
| 260 | + | ||
| 261 | + conn.execute("UPDATE work_graph_run SET status = 'verifying' WHERE run_id = ?", (run_id,)) | ||
| 262 | + conn.commit() | ||
| 263 | + return regenerated |
src/work_node_run.py
| @@ -320,6 +320,17 @@ def run_work_node( | @@ -320,6 +320,17 @@ def run_work_node( | ||
| 320 | remove_worktree(repo_path, worktree_path) # 03-C §3.6: "执行完成后清理 worktree" | 320 | remove_worktree(repo_path, worktree_path) # 03-C §3.6: "执行完成后清理 worktree" |
| 321 | return output_payload | 321 | return output_payload |
| 322 | 322 | ||
| 323 | + # 04-P: verifying -> reflecting ("记录差距,决定重试路径") before the | ||
| 324 | + # actual retry/give-up decision below -- REQ-03 failure's own | ||
| 325 | + # acceptance text names this intermediate status explicitly; | ||
| 326 | + # previously this jumped straight to 'retrying'/'blocked' and | ||
| 327 | + # 'reflecting' was never written anywhere (found during the | ||
| 328 | + # PH4-closure audit, 2026-08-02). | ||
| 329 | + conn.execute( | ||
| 330 | + "UPDATE work_node_execution SET status = 'reflecting' WHERE node_exec_id = ?", (node_exec_id,) | ||
| 331 | + ) | ||
| 332 | + conn.commit() | ||
| 333 | + | ||
| 323 | retry_count += 1 | 334 | retry_count += 1 |
| 324 | if retry_count > max_retries: | 335 | if retry_count > max_retries: |
| 325 | conn.execute( | 336 | conn.execute( |